From dc7a4e3842a439483b80c59adc87e5ca4916ac02 Mon Sep 17 00:00:00 2001 From: Julian Blacher Date: Sat, 17 Oct 2026 17:19:30 -0300 Subject: [PATCH 1/2] feat: add `prw mcp`, an MCP server for AI coding agents Agents that open PRs need to check and wait for CI and read which checks failed. Expose prw's CI aggregation as Model Context Protocol tools over stdio: - get_ci_status: state, check counts, failed checks with links. - wait_for_ci: blocks until CI finishes (default 10m, max 1h), fails fast, follows new pushes, emits notifications/progress when the call has a progressToken, and honors notifications/cancelled (no response is sent for a cancelled request, per the spec). A timeout is a normal result (pending, timed_out=true) so the agent can call again. - list_watched, watch_pr, unwatch_pr: manage the watch list via config.Update, so a running `prw run` picks changes up. The protocol subset (initialize with version negotiation, ping, tools/list, tools/call) is implemented directly: the official Go SDK needs Go 1.25 and the module targets 1.22. Tool calls run concurrently so pings and cancellations are handled during a long wait; on stdin EOF in-flight calls finish before exit. One GitHub client is shared so the ETag cache keeps repeated polling free. Verified end-to-end over stdio against the real API, with the token resolved from `gh auth token`. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + README.md | 32 ++ TASKS.md | 3 +- cmd/prw/mcp.go | 711 ++++++++++++++++++++++++++++++++++++++++++++ cmd/prw/mcp_test.go | 518 ++++++++++++++++++++++++++++++++ 5 files changed, 1264 insertions(+), 1 deletion(-) create mode 100644 cmd/prw/mcp.go create mode 100644 cmd/prw/mcp_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 7002679..dd2d690 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `prw run` now notifies once (`merged` or `closed`) and automatically stops watching a PR once it is merged or closed, instead of polling it forever. This notification always fires, regardless of `--on`/`notification_filter`. ### Added +- `prw mcp`: a Model Context Protocol server over stdio for AI coding agents, with tools `get_ci_status`, `wait_for_ci` (progress notifications, cancellation, fail-fast), `list_watched`, `watch_pr` and `unwatch_pr`. Implemented without new dependencies (the official Go SDK needs Go 1.25). - Conditional requests (ETags): repeated polls of unchanged resources are answered with `304 Not Modified`, which doesn't count against the authenticated rate limit. Measured against the real API: after the first poll, 5 further polls of a PR (15 requests) consumed 0 quota. Previously each watched PR cost 3 requests per poll (~540/hour at the default 20s interval), so about 9 PRs exhausted the 5,000/hour limit. - Rate-limit handling: 403/429 rate-limit responses are told apart from missing-permission 403s. `prw run` stops the current round and pauses until the reset time; `prw wait` waits for the reset (still bounded by `--timeout`) instead of giving up. - `prw wait `: blocks until the PR's CI finishes and exits `0` (success or merged), `1` (failed), `2` (timeout), `3` (other error) or `130` (interrupted). Fails fast on the first failed check, follows new pushes, retries transient API errors, and prints a JSON result with `--json`. Progress goes to stderr. diff --git a/README.md b/README.md index 1527673..4d5f6ff 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,7 @@ The tool is deliberately minimal. It does one thing well: watch PRs and tell you ## Features - **`prw wait`**: block until a PR's CI finishes and exit 0/1/2 — for scripts, pipelines and AI coding agents +- **MCP server** (`prw mcp`): lets Claude Code, Claude Desktop, Cursor and other agents check and wait for PR CI as tools - **Auto-cleanup**: merged/closed PRs are notified once and dropped from the watch list by `prw run` - **Watch multiple PRs** from different repositories simultaneously - **Instant notifications** when CI status changes (pending, success, failure, error) @@ -251,6 +252,37 @@ With `--json`, stdout is a single object: `state` is one of `success`, `failure`, `error`, `pending` (with `timed_out` or `interrupted` set), `merged`, or `closed`. +## Using prw from AI agents: `prw mcp` + +`prw mcp` runs a [Model Context Protocol](https://modelcontextprotocol.io) server over stdio, so coding agents can check a PR's CI and wait for it after pushing — and read which checks failed — without screen-scraping `gh` output. + +**Claude Code:** + +```bash +claude mcp add prw -- prw mcp +``` + +**Claude Desktop, Cursor and other clients** (add to the client's `mcpServers` JSON config): + +```json +{ + "mcpServers": { + "prw": { "command": "prw", "args": ["mcp"] } + } +} +``` + +Tools: + +| Tool | What it does | +|------|--------------| +| `get_ci_status` | Current state of a PR (`success`, `failure`, `error`, `pending`, `merged`, `closed`), check counts, failed checks with links. Returns immediately. | +| `wait_for_ci` | Blocks until CI finishes (`timeout_seconds` default 600, max 3600). Fails fast on the first failed check, follows new pushes, sends progress notifications. On timeout it returns `pending` with `timed_out: true` so the agent can call it again. | +| `list_watched` | PRs on the local watch list with their last known state. | +| `watch_pr` / `unwatch_pr` | Add or remove a PR from the watch list (a running `prw run` picks it up). | + +Results include a one-line summary for the model plus the full data as JSON (and as `structuredContent`). Authentication works like every other command — with `gh auth login` there's nothing to configure. All tools share one GitHub client, so the ETag cache keeps repeated polling free. + ## Configuration Configuration is stored in `~/.prw/config.json`. You can manage settings via the `config` subcommand. diff --git a/TASKS.md b/TASKS.md index 36974eb..d4ef03c 100644 --- a/TASKS.md +++ b/TASKS.md @@ -86,7 +86,8 @@ Legend — Impact: 1–5 · Effort: S (≤½ day), M (1–2 days), L (3+ days) - 🟡 #8 `gh` extension distribution — code part done (token from `GH_TOKEN` / `gh auth token`, tested with a fake gh binary and end-to-end). Remaining: publishing as `gh-prw` needs a repo named `gh-prw` (new repo or rename) + precompiled release assets named `gh-prw--` — owner decision. - ✅ #13 `prw wait ` — done (exit codes 0/1/2/3/130, fail-fast, follows new pushes, retries transient errors, `--json`; verified live against devblac/prw#5 and real process exit codes) - ✅ #9 API efficiency — done (ETag/If-None-Match cache; measured live: 15 steady-state requests = 0 quota; rate-limit detection + pause in `run`, wait-for-reset in `wait`). GraphQL batching not needed for now. -- Next: #14 MCP server (builds on `waitForCI`) +- ✅ #14 MCP server — done (`prw mcp`: 5 tools, progress + cancellation, protocol 2025-06-18/2025-03-26/2024-11-05; verified end-to-end over stdio against the real API with the token from `gh auth token`). Not yet tried inside a real MCP client. +- Next: #10 `prw watch --mine` / `--review-requested` ## Planned diff --git a/cmd/prw/mcp.go b/cmd/prw/mcp.go new file mode 100644 index 0000000..44d7240 --- /dev/null +++ b/cmd/prw/mcp.go @@ -0,0 +1,711 @@ +package main + +import ( + "bufio" + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "os" + "os/signal" + "strings" + "sync" + "syscall" + "time" + + "github.com/spf13/cobra" + + "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/github" + "github.com/devblac/prw/internal/version" +) + +// prw speaks the Model Context Protocol over stdio: newline-delimited JSON-RPC 2.0 +// messages on stdin/stdout, logs on stderr. Only the parts an MCP tool server needs +// are implemented (initialize, ping, tools/list, tools/call, progress, cancellation); +// the official Go SDK would require raising the module's Go version. + +// mcpProtocolVersions are the protocol revisions this server can speak, newest first. +var mcpProtocolVersions = []string{"2025-06-18", "2025-03-26", "2024-11-05"} + +const ( + rpcParseError = -32700 + rpcInvalidRequest = -32600 + rpcMethodNotFound = -32601 + rpcInvalidParams = -32602 + + mcpDefaultWaitTimeout = 10 * time.Minute + mcpMaxWaitTimeout = time.Hour + mcpDefaultWaitInterval = 15 * time.Second + mcpMinWaitInterval = 5 * time.Second + + mcpInstructions = `prw reports the CI status of GitHub pull requests, aggregating GitHub Actions check runs and legacy commit statuses. +Use get_ci_status for a one-off check, and wait_for_ci after pushing to a PR to block until CI finishes (it fails fast and lists failed checks with links). +States: success, failure, error, pending, merged, closed.` +) + +func init() { + rootCmd.AddCommand(mcpCmd) +} + +var mcpCmd = &cobra.Command{ + Use: "mcp", + Short: "Run an MCP server on stdio so AI agents can check and wait for PR CI", + Long: `Run a Model Context Protocol (MCP) server over stdin/stdout. + +AI coding agents (Claude Code, Claude Desktop, Cursor, ...) can then call these tools: + get_ci_status current CI state of a PR, with failed checks and links + wait_for_ci block until a PR's CI finishes (fails fast on the first failed check) + list_watched PRs on prw's watch list and their last known state + watch_pr add a PR to the watch list (picked up by a running 'prw run') + unwatch_pr remove a PR from the watch list + +Authentication is the same as for every other command (gh auth login, GITHUB_TOKEN, ...).`, + Example: ` # Claude Code + claude mcp add prw -- prw mcp + + # Claude Desktop / Cursor (mcpServers in the client's JSON config) + "prw": { "command": "prw", "args": ["mcp"] }`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + defer stop() + return newMCPServer(os.Stdin, os.Stdout, os.Stderr, defaultMCPClient).Serve(ctx) + }, +} + +// mcpClient is what the MCP tools need from GitHub. +type mcpClient interface { + ciClient +} + +func defaultMCPClient() (mcpClient, error) { + cfg, err := config.Load() + if err != nil { + return nil, fmt.Errorf("failed to load config: %w", err) + } + token := cfg.GetToken() + if token == "" { + return nil, errMissingToken + } + return newGitHubClient(token), nil +} + +type rpcRequest struct { + JSONRPC string `json:"jsonrpc"` + ID json.RawMessage `json:"id,omitempty"` + Method string `json:"method,omitempty"` + Params json.RawMessage `json:"params,omitempty"` +} + +type rpcError struct { + Code int `json:"code"` + Message string `json:"message"` +} + +type rpcResponse struct { + JSONRPC string `json:"jsonrpc"` + ID json.RawMessage `json:"id"` + Result interface{} `json:"result,omitempty"` + Error *rpcError `json:"error,omitempty"` +} + +type rpcNotification struct { + JSONRPC string `json:"jsonrpc"` + Method string `json:"method"` + Params interface{} `json:"params,omitempty"` +} + +type mcpServer struct { + in io.Reader + out io.Writer + logOut io.Writer + + writeMu sync.Mutex + + clientFactory func() (mcpClient, error) + clientMu sync.Mutex + client mcpClient // created on first use and shared, so the ETag cache is too + + inflightMu sync.Mutex + inflight map[string]context.CancelFunc + calls sync.WaitGroup +} + +func newMCPServer(in io.Reader, out, logOut io.Writer, clientFactory func() (mcpClient, error)) *mcpServer { + return &mcpServer{ + in: in, + out: out, + logOut: logOut, + clientFactory: clientFactory, + inflight: make(map[string]context.CancelFunc), + } +} + +// Serve handles messages until stdin is closed or ctx is done. Tool calls run +// concurrently so that pings and cancellations are handled during a long wait. +func (s *mcpServer) Serve(ctx context.Context) error { + ctx, cancel := context.WithCancel(ctx) + defer func() { + cancel() + s.calls.Wait() + }() + + lines := make(chan []byte) + readErr := make(chan error, 1) + go func() { + scanner := bufio.NewScanner(s.in) + scanner.Buffer(make([]byte, 0, 64*1024), 10*1024*1024) + for scanner.Scan() { + line := append([]byte(nil), scanner.Bytes()...) + select { + case lines <- line: + case <-ctx.Done(): + return + } + } + readErr <- scanner.Err() + }() + + for { + select { + case <-ctx.Done(): + return nil + case err := <-readErr: + // stdin closed (err is nil on EOF): the client is shutting down. Let calls in + // flight finish and answer; a client that can't wait sends SIGTERM, which + // cancels ctx. + finished := make(chan struct{}) + go func() { + s.calls.Wait() + close(finished) + }() + select { + case <-finished: + case <-ctx.Done(): + } + return err + case line := <-lines: + s.handleLine(ctx, line) + } + } +} + +func (s *mcpServer) handleLine(ctx context.Context, line []byte) { + line = bytes.TrimSpace(line) + if len(line) == 0 { + return + } + if line[0] == '[' { + s.respondError(json.RawMessage("null"), rpcInvalidRequest, "batch requests are not supported") + return + } + + var req rpcRequest + if err := json.Unmarshal(line, &req); err != nil { + s.respondError(json.RawMessage("null"), rpcParseError, "parse error: "+err.Error()) + return + } + if req.Method == "" { + return // a response to a request we never send; ignore + } + if len(req.ID) == 0 || string(req.ID) == "null" { + s.handleNotification(req) + return + } + + switch req.Method { + case "initialize": + s.respond(req.ID, s.initializeResult(req.Params)) + case "ping": + s.respond(req.ID, struct{}{}) + case "tools/list": + s.respond(req.ID, map[string]interface{}{"tools": mcpTools}) + case "tools/call": + s.startToolCall(ctx, req) + default: + s.respondError(req.ID, rpcMethodNotFound, "method not found: "+req.Method) + } +} + +func (s *mcpServer) handleNotification(req rpcRequest) { + if req.Method != "notifications/cancelled" { + return // notifications/initialized and others need no action + } + var params struct { + RequestID json.RawMessage `json:"requestId"` + } + if json.Unmarshal(req.Params, ¶ms) != nil { + return + } + s.inflightMu.Lock() + cancel := s.inflight[string(params.RequestID)] + s.inflightMu.Unlock() + if cancel != nil { + cancel() + } +} + +func (s *mcpServer) initializeResult(params json.RawMessage) interface{} { + var p struct { + ProtocolVersion string `json:"protocolVersion"` + } + _ = json.Unmarshal(params, &p) + negotiated := mcpProtocolVersions[0] + for _, v := range mcpProtocolVersions { + if v == p.ProtocolVersion { + negotiated = v + } + } + return map[string]interface{}{ + "protocolVersion": negotiated, + "capabilities": map[string]interface{}{"tools": map[string]interface{}{}}, + "serverInfo": map[string]string{"name": "prw", "version": version.Version}, + "instructions": mcpInstructions, + } +} + +func (s *mcpServer) startToolCall(ctx context.Context, req rpcRequest) { + var params struct { + Name string `json:"name"` + Arguments json.RawMessage `json:"arguments"` + Meta struct { + ProgressToken json.RawMessage `json:"progressToken"` + } `json:"_meta"` + } + if err := json.Unmarshal(req.Params, ¶ms); err != nil || params.Name == "" { + s.respondError(req.ID, rpcInvalidParams, "tools/call needs a tool name") + return + } + handler, ok := mcpToolHandlers[params.Name] + if !ok { + s.respondError(req.ID, rpcInvalidParams, "unknown tool: "+params.Name) + return + } + + callCtx, cancel := context.WithCancel(ctx) + key := string(req.ID) + s.inflightMu.Lock() + s.inflight[key] = cancel + s.inflightMu.Unlock() + + s.calls.Add(1) + go func() { + defer s.calls.Done() + defer func() { + s.inflightMu.Lock() + delete(s.inflight, key) + s.inflightMu.Unlock() + cancel() + }() + + var progress io.Writer = io.Discard + if len(params.Meta.ProgressToken) > 0 && string(params.Meta.ProgressToken) != "null" { + progress = &mcpProgressWriter{server: s, token: params.Meta.ProgressToken} + } + + result := handler(callCtx, s, params.Arguments, progress) + if callCtx.Err() != nil { + // Cancelled by the client (or the server is shutting down): per the spec the + // receiver should not answer a cancelled request. + return + } + s.respond(req.ID, result) + }() +} + +// githubClient returns the shared GitHub client, creating it on first use so a missing +// token is reported as a tool error instead of preventing the server from starting. +func (s *mcpServer) githubClient() (mcpClient, error) { + s.clientMu.Lock() + defer s.clientMu.Unlock() + if s.client != nil { + return s.client, nil + } + c, err := s.clientFactory() + if err != nil { + return nil, err + } + s.client = c + return c, nil +} + +func (s *mcpServer) respond(id json.RawMessage, result interface{}) { + s.write(rpcResponse{JSONRPC: "2.0", ID: id, Result: result}) +} + +func (s *mcpServer) respondError(id json.RawMessage, code int, message string) { + s.write(rpcResponse{JSONRPC: "2.0", ID: id, Error: &rpcError{Code: code, Message: message}}) +} + +func (s *mcpServer) write(msg interface{}) { + data, err := json.Marshal(msg) + if err != nil { + fmt.Fprintf(s.logOut, "prw mcp: failed to encode message: %v\n", err) + return + } + s.writeMu.Lock() + defer s.writeMu.Unlock() + if _, err := s.out.Write(append(data, '\n')); err != nil { + fmt.Fprintf(s.logOut, "prw mcp: failed to write message: %v\n", err) + } +} + +// mcpProgressWriter turns each line waitForCI prints into a notifications/progress +// message for the tool call that asked for progress. +type mcpProgressWriter struct { + server *mcpServer + token json.RawMessage + buf []byte + count int +} + +func (w *mcpProgressWriter) Write(p []byte) (int, error) { + w.buf = append(w.buf, p...) + for { + i := bytes.IndexByte(w.buf, '\n') + if i < 0 { + return len(p), nil + } + line := strings.TrimSpace(string(w.buf[:i])) + w.buf = w.buf[i+1:] + if line == "" { + continue + } + w.count++ + w.server.write(rpcNotification{ + JSONRPC: "2.0", + Method: "notifications/progress", + Params: map[string]interface{}{ + "progressToken": w.token, + "progress": w.count, + "message": line, + }, + }) + } +} + +// --- tools --- + +type mcpToolHandler func(ctx context.Context, s *mcpServer, args json.RawMessage, progress io.Writer) mcpToolResult + +type mcpToolResult struct { + Content []mcpContent `json:"content"` + StructuredContent interface{} `json:"structuredContent,omitempty"` + IsError bool `json:"isError,omitempty"` +} + +type mcpContent struct { + Type string `json:"type"` + Text string `json:"text"` +} + +func toolError(format string, args ...interface{}) mcpToolResult { + return mcpToolResult{Content: []mcpContent{{Type: "text", Text: fmt.Sprintf(format, args...)}}, IsError: true} +} + +// toolOK returns a short human summary followed by the JSON data, plus the data as +// structuredContent for clients that support it. +func toolOK(summary string, data interface{}) mcpToolResult { + encoded, err := json.MarshalIndent(data, "", " ") + if err != nil { + return toolError("failed to encode result: %v", err) + } + return mcpToolResult{ + Content: []mcpContent{{Type: "text", Text: summary + "\n\n" + string(encoded)}}, + StructuredContent: data, + } +} + +var prURLSchema = map[string]interface{}{ + "type": "string", + "description": "GitHub pull request URL, e.g. https://github.com/owner/repo/pull/123", +} + +var mcpTools = []map[string]interface{}{ + { + "name": "get_ci_status", + "title": "Get PR CI status", + "description": "Get the current CI status of a GitHub pull request: overall state (success, failure, error, pending, merged, closed), check counts, and failed checks with links. Aggregates GitHub Actions check runs and legacy commit statuses. Returns immediately.", + "inputSchema": map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{"pr_url": prURLSchema}, + "required": []string{"pr_url"}, + "additionalProperties": false, + }, + "annotations": map[string]interface{}{"readOnlyHint": true, "openWorldHint": true}, + }, + { + "name": "wait_for_ci", + "title": "Wait for PR CI", + "description": "Block until a GitHub pull request's CI finishes, then return the result. Fails fast: returns as soon as any check fails, with the failed checks and links. Follows new pushes to the PR. If the timeout is reached first, returns state 'pending' with timed_out=true (call again to keep waiting). A merge while waiting returns state 'merged'.", + "inputSchema": map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{ + "pr_url": prURLSchema, + "timeout_seconds": map[string]interface{}{ + "type": "integer", + "description": "Give up after this many seconds (default 600, max 3600).", + "minimum": 1, + "maximum": int(mcpMaxWaitTimeout / time.Second), + }, + "interval_seconds": map[string]interface{}{ + "type": "integer", + "description": "Seconds between polls (default 15, min 5).", + "minimum": int(mcpMinWaitInterval / time.Second), + }, + }, + "required": []string{"pr_url"}, + "additionalProperties": false, + }, + "annotations": map[string]interface{}{"readOnlyHint": true, "openWorldHint": true}, + }, + { + "name": "list_watched", + "title": "List watched PRs", + "description": "List the pull requests on prw's local watch list with their last known CI state. Reads local state only; use get_ci_status for fresh data.", + "inputSchema": map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{}, + "additionalProperties": false, + }, + "annotations": map[string]interface{}{"readOnlyHint": true, "openWorldHint": false}, + }, + { + "name": "watch_pr", + "title": "Watch a PR", + "description": "Add a GitHub pull request to prw's watch list so a running 'prw run' sends desktop/Slack/Discord notifications when its CI status changes. Idempotent.", + "inputSchema": map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{"pr_url": prURLSchema}, + "required": []string{"pr_url"}, + "additionalProperties": false, + }, + "annotations": map[string]interface{}{"readOnlyHint": false, "destructiveHint": false, "idempotentHint": true, "openWorldHint": true}, + }, + { + "name": "unwatch_pr", + "title": "Stop watching a PR", + "description": "Remove a GitHub pull request from prw's watch list. Idempotent.", + "inputSchema": map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{"pr_url": prURLSchema}, + "required": []string{"pr_url"}, + "additionalProperties": false, + }, + "annotations": map[string]interface{}{"readOnlyHint": false, "destructiveHint": true, "idempotentHint": true, "openWorldHint": false}, + }, +} + +var mcpToolHandlers map[string]mcpToolHandler + +func init() { + // Assigned in init to avoid an initialization cycle (handlers reference mcpServer). + mcpToolHandlers = map[string]mcpToolHandler{ + "get_ci_status": toolGetCIStatus, + "wait_for_ci": toolWaitForCI, + "list_watched": toolListWatched, + "watch_pr": toolWatchPR, + "unwatch_pr": toolUnwatchPR, + } +} + +type prURLArgs struct { + PRURL string `json:"pr_url"` +} + +func parsePRArg(args json.RawMessage) (owner, repo string, number int, err error) { + var a prURLArgs + if len(args) > 0 { + if err := json.Unmarshal(args, &a); err != nil { + return "", "", 0, fmt.Errorf("invalid arguments: %v", err) + } + } + if strings.TrimSpace(a.PRURL) == "" { + return "", "", 0, errors.New("pr_url is required") + } + owner, repo, number, err = github.ParsePRURL(a.PRURL) + if err != nil { + return "", "", 0, fmt.Errorf("invalid pr_url %q: %v", a.PRURL, err) + } + return owner, repo, number, nil +} + +func ciSummary(r *waitResult) string { + pr := fmt.Sprintf("%s/%s#%d", r.Owner, r.Repo, r.Number) + switch { + case r.TimedOut: + return fmt.Sprintf("Timed out after %s: CI of %s is still pending (%d of %d check(s) running). Call wait_for_ci again to keep waiting.", + formatElapsed(r.ElapsedSeconds), pr, r.PendingChecks, r.TotalChecks) + case r.State == "merged" || r.State == "closed": + return fmt.Sprintf("%s was %s.", pr, r.State) + case r.State == "pending": + return fmt.Sprintf("CI of %s is pending: %s.", pr, pendingSummary(r)) + case r.State == "success": + return fmt.Sprintf("CI of %s passed (%d check(s)).", pr, r.TotalChecks) + } + names := make([]string, len(r.FailedChecks)) + for i, c := range r.FailedChecks { + names[i] = c.Name + } + summary := fmt.Sprintf("CI of %s finished with %s.", pr, r.State) + if len(names) > 0 { + summary += " Failed: " + strings.Join(names, ", ") + "." + } + return summary +} + +func toolGetCIStatus(ctx context.Context, s *mcpServer, args json.RawMessage, _ io.Writer) mcpToolResult { + owner, repo, number, err := parsePRArg(args) + if err != nil { + return toolError("%v", err) + } + client, err := s.githubClient() + if err != nil { + return toolError("%v", err) + } + result := &waitResult{Owner: owner, Repo: repo, Number: number, URL: github.FormatPRURL(owner, repo, number)} + if _, err := pollOnce(client, result); err != nil { + return toolError("failed to get CI status of %s/%s#%d: %v", owner, repo, number, err) + } + return toolOK(ciSummary(result), result) +} + +func toolWaitForCI(ctx context.Context, s *mcpServer, args json.RawMessage, progress io.Writer) mcpToolResult { + owner, repo, number, err := parsePRArg(args) + if err != nil { + return toolError("%v", err) + } + var opts struct { + TimeoutSeconds int `json:"timeout_seconds"` + IntervalSeconds int `json:"interval_seconds"` + } + if err := json.Unmarshal(args, &opts); err != nil { + return toolError("invalid arguments: %v", err) + } + timeout := mcpDefaultWaitTimeout + if opts.TimeoutSeconds != 0 { + if opts.TimeoutSeconds < 0 { + return toolError("timeout_seconds must be positive") + } + timeout = time.Duration(opts.TimeoutSeconds) * time.Second + } + if timeout > mcpMaxWaitTimeout { + timeout = mcpMaxWaitTimeout + } + interval := mcpDefaultWaitInterval + if opts.IntervalSeconds != 0 { + interval = time.Duration(opts.IntervalSeconds) * time.Second + } + if interval < mcpMinWaitInterval { + interval = mcpMinWaitInterval + } + + client, err := s.githubClient() + if err != nil { + return toolError("%v", err) + } + + waitCtx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + result, err := waitForCI(waitCtx, client, owner, repo, number, interval, progress) + if err != nil { + return toolError("failed waiting for CI of %s/%s#%d: %v", owner, repo, number, err) + } + return toolOK(ciSummary(result), result) +} + +type watchedPROutput struct { + URL string `json:"url"` + Owner string `json:"owner"` + Repo string `json:"repo"` + Number int `json:"number"` + Title string `json:"title,omitempty"` + LastState string `json:"last_known_state"` + LastChecked *time.Time `json:"last_checked,omitempty"` +} + +func toolListWatched(ctx context.Context, s *mcpServer, _ json.RawMessage, _ io.Writer) mcpToolResult { + cfg, err := config.Load() + if err != nil { + return toolError("failed to load config: %v", err) + } + prs := make([]watchedPROutput, 0, len(cfg.WatchedPRs)) + for _, pr := range cfg.WatchedPRs { + state := github.NormalizeState(pr.LastKnownState) + if state == "" { + state = "unknown" + } + out := watchedPROutput{ + URL: github.FormatPRURL(pr.Owner, pr.Repo, pr.Number), Owner: pr.Owner, Repo: pr.Repo, + Number: pr.Number, Title: pr.Title, LastState: state, + } + if !pr.LastChecked.IsZero() { + t := pr.LastChecked + out.LastChecked = &t + } + prs = append(prs, out) + } + summary := fmt.Sprintf("%d PR(s) on the watch list.", len(prs)) + if len(prs) == 0 { + summary = "No PRs on the watch list. Add one with watch_pr." + } + return toolOK(summary, map[string]interface{}{"watched_prs": prs}) +} + +func toolWatchPR(ctx context.Context, s *mcpServer, args json.RawMessage, _ io.Writer) mcpToolResult { + owner, repo, number, err := parsePRArg(args) + if err != nil { + return toolError("%v", err) + } + client, err := s.githubClient() + if err != nil { + return toolError("%v", err) + } + pr, err := client.GetPullRequest(owner, repo, number) + if err != nil { + return toolError("failed to fetch %s/%s#%d: %v", owner, repo, number, err) + } + + added := false + if err := config.Update(func(c *config.Config) error { + added = c.AddPR(config.WatchedPR{Owner: owner, Repo: repo, Number: number, Title: pr.Title}) + if !added { + return config.ErrNoChanges + } + return nil + }); err != nil { + return toolError("failed to save config: %v", err) + } + + summary := fmt.Sprintf("Now watching %s/%s#%d - %s. A running 'prw run' picks it up on its next poll.", owner, repo, number, pr.Title) + if !added { + summary = fmt.Sprintf("%s/%s#%d was already being watched.", owner, repo, number) + } + return toolOK(summary, map[string]interface{}{"url": github.FormatPRURL(owner, repo, number), "title": pr.Title, "added": added}) +} + +func toolUnwatchPR(ctx context.Context, s *mcpServer, args json.RawMessage, _ io.Writer) mcpToolResult { + owner, repo, number, err := parsePRArg(args) + if err != nil { + return toolError("%v", err) + } + removed := false + if err := config.Update(func(c *config.Config) error { + removed = c.RemovePR(owner, repo, number) + if !removed { + return config.ErrNoChanges + } + return nil + }); err != nil { + return toolError("failed to update config: %v", err) + } + + summary := fmt.Sprintf("Stopped watching %s/%s#%d.", owner, repo, number) + if !removed { + summary = fmt.Sprintf("%s/%s#%d was not being watched.", owner, repo, number) + } + return toolOK(summary, map[string]interface{}{"url": github.FormatPRURL(owner, repo, number), "removed": removed}) +} diff --git a/cmd/prw/mcp_test.go b/cmd/prw/mcp_test.go new file mode 100644 index 0000000..fffea80 --- /dev/null +++ b/cmd/prw/mcp_test.go @@ -0,0 +1,518 @@ +package main + +import ( + "bufio" + "context" + "encoding/json" + "fmt" + "io" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/github" +) + +// mcpHarness runs an mcpServer over in-memory pipes and lets a test exchange messages. +type mcpHarness struct { + t *testing.T + in *io.PipeWriter + out *bufio.Scanner + done chan error + cancel context.CancelFunc + pending []map[string]interface{} // messages read while waiting for something else + mu sync.Mutex +} + +func startMCP(t *testing.T, factory func() (mcpClient, error)) *mcpHarness { + t.Helper() + path := filepath.Join(t.TempDir(), ".prw", "config.json") + oldPath := config.ConfigPath + config.ConfigPath = func() (string, error) { return path, nil } + t.Cleanup(func() { config.ConfigPath = oldPath }) + + inR, inW := io.Pipe() + outR, outW := io.Pipe() + ctx, cancel := context.WithCancel(context.Background()) + h := &mcpHarness{t: t, in: inW, out: bufio.NewScanner(outR), done: make(chan error, 1), cancel: cancel} + h.out.Buffer(make([]byte, 0, 64*1024), 10*1024*1024) + + srv := newMCPServer(inR, outW, io.Discard, factory) + go func() { + h.done <- srv.Serve(ctx) + outW.Close() + }() + t.Cleanup(func() { + cancel() + inW.Close() + select { + case <-h.done: + case <-time.After(5 * time.Second): + t.Error("server did not stop") + } + }) + return h +} + +func (h *mcpHarness) send(msg string) { + h.t.Helper() + if _, err := io.WriteString(h.in, msg+"\n"); err != nil { + h.t.Fatalf("write: %v", err) + } +} + +func (h *mcpHarness) readMessage() map[string]interface{} { + h.t.Helper() + type res struct { + m map[string]interface{} + err error + } + ch := make(chan res, 1) + go func() { + if !h.out.Scan() { + ch <- res{err: fmt.Errorf("stream closed: %v", h.out.Err())} + return + } + var m map[string]interface{} + err := json.Unmarshal(h.out.Bytes(), &m) + ch <- res{m, err} + }() + select { + case r := <-ch: + if r.err != nil { + h.t.Fatalf("read: %v", r.err) + } + return r.m + case <-time.After(5 * time.Second): + h.t.Fatal("timed out waiting for a message") + } + return nil +} + +// response reads messages until the response with the given id, keeping others. +func (h *mcpHarness) response(id int) map[string]interface{} { + h.t.Helper() + for i, m := range h.pending { + if v, ok := m["id"].(float64); ok && int(v) == id { + h.pending = append(h.pending[:i], h.pending[i+1:]...) + return m + } + } + for { + m := h.readMessage() + if v, ok := m["id"].(float64); ok && int(v) == id { + return m + } + h.pending = append(h.pending, m) + } +} + +func (h *mcpHarness) call(id int, method string, params interface{}) map[string]interface{} { + h.t.Helper() + p, _ := json.Marshal(params) + h.send(fmt.Sprintf(`{"jsonrpc":"2.0","id":%d,"method":%q,"params":%s}`, id, method, p)) + return h.response(id) +} + +func (h *mcpHarness) initialize() { + h.t.Helper() + resp := h.call(1, "initialize", map[string]interface{}{ + "protocolVersion": "2025-06-18", + "capabilities": map[string]interface{}{}, + "clientInfo": map[string]string{"name": "test", "version": "1"}, + }) + if resp["error"] != nil { + h.t.Fatalf("initialize failed: %v", resp["error"]) + } + h.send(`{"jsonrpc":"2.0","method":"notifications/initialized"}`) +} + +// toolResult extracts the tool result and its text from a tools/call response. +func toolResultOf(t *testing.T, resp map[string]interface{}) (result map[string]interface{}, text string) { + t.Helper() + if resp["error"] != nil { + t.Fatalf("unexpected JSON-RPC error: %v", resp["error"]) + } + result = resp["result"].(map[string]interface{}) + content := result["content"].([]interface{}) + if len(content) != 1 { + t.Fatalf("expected 1 content item, got %v", content) + } + item := content[0].(map[string]interface{}) + if item["type"] != "text" { + t.Fatalf("content type = %v", item["type"]) + } + return result, item["text"].(string) +} + +// setSleep replaces sleepFunc until the end of the test. Call it before startMCP: +// cleanups run in reverse order, so the original is restored only after the server +// (and any tool call still reading sleepFunc) has stopped. +func setSleep(t *testing.T, fn func(context.Context, time.Duration)) { + t.Helper() + old := sleepFunc + sleepFunc = fn + t.Cleanup(func() { sleepFunc = old }) +} + +func staticClient(c mcpClient) func() (mcpClient, error) { + return func() (mcpClient, error) { return c, nil } +} + +func successClient() *scriptedClient { + return &scriptedClient{steps: []pollStep{{ + pr: openPR("abc1234"), + runs: []github.CheckRun{run("Test", "completed", "success"), run("Build", "completed", "success")}, + }}} +} + +func TestMCP_Initialize(t *testing.T) { + h := startMCP(t, staticClient(successClient())) + + resp := h.call(1, "initialize", map[string]interface{}{"protocolVersion": "2025-03-26", "capabilities": map[string]interface{}{}}) + result := resp["result"].(map[string]interface{}) + if result["protocolVersion"] != "2025-03-26" { + t.Errorf("must echo a supported requested version, got %v", result["protocolVersion"]) + } + if _, ok := result["capabilities"].(map[string]interface{})["tools"]; !ok { + t.Errorf("tools capability missing: %v", result["capabilities"]) + } + if info := result["serverInfo"].(map[string]interface{}); info["name"] != "prw" { + t.Errorf("serverInfo = %v", info) + } + if result["instructions"] == "" { + t.Error("instructions should be set") + } + + unknown := h.call(2, "initialize", map[string]interface{}{"protocolVersion": "1999-01-01"}) + if v := unknown["result"].(map[string]interface{})["protocolVersion"]; v != mcpProtocolVersions[0] { + t.Errorf("unsupported version should get the latest supported, got %v", v) + } +} + +func TestMCP_ToolsList(t *testing.T) { + h := startMCP(t, staticClient(successClient())) + h.initialize() + + resp := h.call(2, "tools/list", map[string]interface{}{}) + tools := resp["result"].(map[string]interface{})["tools"].([]interface{}) + names := map[string]bool{} + for _, raw := range tools { + tool := raw.(map[string]interface{}) + name := tool["name"].(string) + names[name] = true + if tool["description"] == "" { + t.Errorf("%s has no description", name) + } + schema := tool["inputSchema"].(map[string]interface{}) + if schema["type"] != "object" { + t.Errorf("%s inputSchema type = %v", name, schema["type"]) + } + if _, ok := mcpToolHandlers[name]; !ok { + t.Errorf("%s is listed but has no handler", name) + } + } + for _, want := range []string{"get_ci_status", "wait_for_ci", "list_watched", "watch_pr", "unwatch_pr"} { + if !names[want] { + t.Errorf("tool %s missing from tools/list", want) + } + } + if len(names) != len(mcpToolHandlers) { + t.Errorf("%d tools listed, %d handlers", len(names), len(mcpToolHandlers)) + } +} + +func TestMCP_PingAndUnknownMethod(t *testing.T) { + h := startMCP(t, staticClient(successClient())) + + if resp := h.call(1, "ping", nil); resp["error"] != nil || resp["result"] == nil { + t.Errorf("ping response = %v", resp) + } + resp := h.call(2, "resources/list", map[string]interface{}{}) + if code := resp["error"].(map[string]interface{})["code"]; code != float64(rpcMethodNotFound) { + t.Errorf("error code = %v, want %d", code, rpcMethodNotFound) + } +} + +func TestMCP_MalformedInput(t *testing.T) { + h := startMCP(t, staticClient(successClient())) + + h.send(`{not json`) + if code := h.readMessage()["error"].(map[string]interface{})["code"]; code != float64(rpcParseError) { + t.Errorf("parse error code = %v", code) + } + h.send(`[{"jsonrpc":"2.0","id":1,"method":"ping"}]`) + if code := h.readMessage()["error"].(map[string]interface{})["code"]; code != float64(rpcInvalidRequest) { + t.Errorf("batch error code = %v", code) + } + h.send(``) // blank lines are ignored + if resp := h.call(3, "tools/call", map[string]interface{}{"name": "no_such_tool"}); resp["error"].(map[string]interface{})["code"] != float64(rpcInvalidParams) { + t.Errorf("unknown tool: %v", resp) + } + if resp := h.call(4, "tools/call", map[string]interface{}{}); resp["error"].(map[string]interface{})["code"] != float64(rpcInvalidParams) { + t.Errorf("missing name: %v", resp) + } +} + +func TestMCP_GetCIStatus(t *testing.T) { + h := startMCP(t, staticClient(&scriptedClient{steps: []pollStep{{ + pr: openPR("abc1234"), + runs: []github.CheckRun{run("Lint", "completed", "failure"), run("Test", "in_progress", "")}, + }}})) + h.initialize() + + result, text := toolResultOf(t, h.call(2, "tools/call", map[string]interface{}{ + "name": "get_ci_status", "arguments": map[string]string{"pr_url": "https://github.com/owner/repo/pull/1"}, + })) + if result["isError"] == true { + t.Fatalf("unexpected tool error: %s", text) + } + if !strings.HasPrefix(text, "CI of owner/repo#1 finished with failure. Failed: Lint.") { + t.Errorf("summary = %q", strings.SplitN(text, "\n", 2)[0]) + } + structured := result["structuredContent"].(map[string]interface{}) + if structured["state"] != "failure" || structured["total_checks"] != float64(2) || structured["pending_checks"] != float64(1) { + t.Errorf("structuredContent = %v", structured) + } + failed := structured["failed_checks"].([]interface{}) + if len(failed) != 1 || failed[0].(map[string]interface{})["url"] != "https://github.com/o/r/runs/Lint" { + t.Errorf("failed_checks = %v", failed) + } +} + +func TestMCP_ToolErrorsAreReportedAsIsError(t *testing.T) { + tests := []struct { + name string + factory func() (mcpClient, error) + tool string + args interface{} + want string + }{ + {"missing pr_url", staticClient(successClient()), "get_ci_status", map[string]string{}, "pr_url is required"}, + {"invalid pr_url", staticClient(successClient()), "wait_for_ci", map[string]string{"pr_url": "nope"}, "invalid pr_url"}, + {"wrong argument type", staticClient(successClient()), "wait_for_ci", map[string]interface{}{"pr_url": "https://github.com/o/r/pull/1", "timeout_seconds": "soon"}, "invalid arguments"}, + {"missing token", func() (mcpClient, error) { return nil, errMissingToken }, "get_ci_status", map[string]string{"pr_url": "https://github.com/o/r/pull/1"}, "gh auth login"}, + {"API error", staticClient(&scriptedClient{steps: []pollStep{{prErr: &github.APIError{StatusCode: 404, Body: "Not Found"}}}}), "get_ci_status", map[string]string{"pr_url": "https://github.com/o/r/pull/1"}, "404"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := startMCP(t, tt.factory) + h.initialize() + result, text := toolResultOf(t, h.call(2, "tools/call", map[string]interface{}{"name": tt.tool, "arguments": tt.args})) + if result["isError"] != true { + t.Errorf("expected isError, got %v", result) + } + if !strings.Contains(text, tt.want) { + t.Errorf("text %q should contain %q", text, tt.want) + } + }) + } +} + +func TestMCP_WaitForCI_WithProgress(t *testing.T) { + noSleep(t) + h := startMCP(t, staticClient(&scriptedClient{steps: []pollStep{ + {pr: openPR("abc1234"), runs: []github.CheckRun{run("Test", "in_progress", "")}}, + {pr: openPR("abc1234"), runs: []github.CheckRun{run("Test", "completed", "success")}}, + }})) + h.initialize() + + resp := h.call(2, "tools/call", map[string]interface{}{ + "name": "wait_for_ci", + "arguments": map[string]interface{}{"pr_url": "https://github.com/owner/repo/pull/1", "timeout_seconds": 60}, + "_meta": map[string]interface{}{"progressToken": "tok-1"}, + }) + result, text := toolResultOf(t, resp) + if result["isError"] == true || result["structuredContent"].(map[string]interface{})["state"] != "success" { + t.Fatalf("unexpected result: %s", text) + } + + var progress []map[string]interface{} + for _, m := range h.pending { + if m["method"] == "notifications/progress" { + progress = append(progress, m["params"].(map[string]interface{})) + } + } + if len(progress) != 1 { + t.Fatalf("expected 1 progress notification, got %d: %v", len(progress), h.pending) + } + if progress[0]["progressToken"] != "tok-1" || progress[0]["progress"] != float64(1) || + !strings.Contains(progress[0]["message"].(string), "1 of 1 check(s) still running") { + t.Errorf("progress = %v", progress[0]) + } +} + +func TestMCP_WaitForCI_NoProgressWithoutToken(t *testing.T) { + noSleep(t) + h := startMCP(t, staticClient(&scriptedClient{steps: []pollStep{ + {pr: openPR("abc1234"), runs: []github.CheckRun{run("Test", "in_progress", "")}}, + {pr: openPR("abc1234"), runs: []github.CheckRun{run("Test", "completed", "success")}}, + }})) + h.initialize() + + h.call(2, "tools/call", map[string]interface{}{"name": "wait_for_ci", "arguments": map[string]string{"pr_url": "https://github.com/o/r/pull/1"}}) + if len(h.pending) != 0 { + t.Errorf("no notifications expected without a progressToken, got %v", h.pending) + } +} + +func TestMCP_WaitForCI_Timeout(t *testing.T) { + setSleep(t, func(ctx context.Context, d time.Duration) { <-ctx.Done() }) + + h := startMCP(t, staticClient(&scriptedClient{steps: []pollStep{{pr: openPR("sha"), runs: []github.CheckRun{run("Test", "queued", "")}}}})) + h.initialize() + + result, text := toolResultOf(t, h.call(2, "tools/call", map[string]interface{}{ + "name": "wait_for_ci", "arguments": map[string]interface{}{"pr_url": "https://github.com/o/r/pull/1", "timeout_seconds": 1}, + })) + if result["isError"] == true { + t.Fatalf("a timeout is a normal result, not a tool error: %s", text) + } + if s := result["structuredContent"].(map[string]interface{}); s["timed_out"] != true || s["state"] != "pending" { + t.Errorf("structuredContent = %v", s) + } + if !strings.Contains(text, "Call wait_for_ci again") { + t.Errorf("summary should tell the agent how to continue: %q", text) + } +} + +func TestMCP_PingWhileWaitingAndCancellation(t *testing.T) { + setSleep(t, func(ctx context.Context, d time.Duration) { <-ctx.Done() }) // wait until cancelled + + h := startMCP(t, staticClient(&scriptedClient{steps: []pollStep{{pr: openPR("sha"), runs: []github.CheckRun{run("Test", "queued", "")}}}})) + h.initialize() + + h.send(`{"jsonrpc":"2.0","id":7,"method":"tools/call","params":{"name":"wait_for_ci","arguments":{"pr_url":"https://github.com/o/r/pull/1"}}}`) + + // The long call must not block other requests. + if resp := h.call(8, "ping", nil); resp["result"] == nil { + t.Fatalf("ping during wait_for_ci failed: %v", resp) + } + + h.send(`{"jsonrpc":"2.0","method":"notifications/cancelled","params":{"requestId":7,"reason":"user aborted"}}`) + + // No response must be sent for the cancelled request; the next ping proves the + // server is alive and that nothing for id 7 arrived before it. + if resp := h.call(9, "ping", nil); resp["result"] == nil { + t.Fatalf("ping after cancel failed: %v", resp) + } + time.Sleep(100 * time.Millisecond) + h.send(`{"jsonrpc":"2.0","id":10,"method":"ping"}`) + h.response(10) + for _, m := range h.pending { + if m["id"] == float64(7) { + t.Errorf("a cancelled request must not get a response: %v", m) + } + } +} + +func TestMCP_FinishesInFlightCallsWhenStdinCloses(t *testing.T) { + release := make(chan struct{}) + setSleep(t, func(ctx context.Context, d time.Duration) { <-release }) + + h := startMCP(t, staticClient(&scriptedClient{steps: []pollStep{ + {pr: openPR("sha"), runs: []github.CheckRun{run("Test", "queued", "")}}, + {pr: openPR("sha"), runs: []github.CheckRun{run("Test", "completed", "success")}}, + }})) + h.initialize() + + h.send(`{"jsonrpc":"2.0","id":5,"method":"tools/call","params":{"name":"wait_for_ci","arguments":{"pr_url":"https://github.com/o/r/pull/1"}}}`) + time.Sleep(50 * time.Millisecond) + h.in.Close() + close(release) + + result, _ := toolResultOf(t, h.response(5)) + if result["structuredContent"].(map[string]interface{})["state"] != "success" { + t.Errorf("in-flight call should complete after stdin closes: %v", result) + } + select { + case err := <-h.done: + if err != nil { + t.Errorf("Serve returned %v", err) + } + h.done <- nil // let the cleanup see it + case <-time.After(5 * time.Second): + t.Fatal("server did not exit after stdin closed") + } +} + +func TestMCP_WatchListTools(t *testing.T) { + pr := openPR("sha") + pr.Number, pr.Title = 42, "Ship it" + client := &scriptedClient{steps: []pollStep{{pr: pr}}} + h := startMCP(t, staticClient(client)) + h.initialize() + url := map[string]string{"pr_url": "https://github.com/owner/repo/pull/42"} + + _, text := toolResultOf(t, h.call(2, "tools/call", map[string]interface{}{"name": "list_watched", "arguments": map[string]string{}})) + if !strings.HasPrefix(text, "No PRs on the watch list") { + t.Errorf("empty list summary = %q", text) + } + + result, text := toolResultOf(t, h.call(3, "tools/call", map[string]interface{}{"name": "watch_pr", "arguments": url})) + if result["structuredContent"].(map[string]interface{})["added"] != true || !strings.Contains(text, "Ship it") { + t.Errorf("watch_pr: %s", text) + } + result, _ = toolResultOf(t, h.call(4, "tools/call", map[string]interface{}{"name": "watch_pr", "arguments": url})) + if result["structuredContent"].(map[string]interface{})["added"] != false { + t.Error("watch_pr must be idempotent") + } + + result, _ = toolResultOf(t, h.call(5, "tools/call", map[string]interface{}{"name": "list_watched"})) + prs := result["structuredContent"].(map[string]interface{})["watched_prs"].([]interface{}) + if len(prs) != 1 { + t.Fatalf("watched_prs = %v", prs) + } + if p := prs[0].(map[string]interface{}); p["url"] != "https://github.com/owner/repo/pull/42" || p["title"] != "Ship it" || p["last_known_state"] != "unknown" { + t.Errorf("watched PR = %v", p) + } + + loaded, _ := config.Load() + if len(loaded.WatchedPRs) != 1 || loaded.WatchedPRs[0].Number != 42 { + t.Errorf("watch_pr must persist to the config file: %+v", loaded.WatchedPRs) + } + + result, _ = toolResultOf(t, h.call(6, "tools/call", map[string]interface{}{"name": "unwatch_pr", "arguments": url})) + if result["structuredContent"].(map[string]interface{})["removed"] != true { + t.Error("unwatch_pr should remove the PR") + } + result, _ = toolResultOf(t, h.call(7, "tools/call", map[string]interface{}{"name": "unwatch_pr", "arguments": url})) + if result["structuredContent"].(map[string]interface{})["removed"] != false { + t.Error("unwatch_pr must be idempotent") + } +} + +func TestMCP_SharesOneGitHubClient(t *testing.T) { + created := 0 + h := startMCP(t, func() (mcpClient, error) { + created++ + return successClient(), nil + }) + h.initialize() + for id := 2; id <= 4; id++ { + h.call(id, "tools/call", map[string]interface{}{"name": "get_ci_status", "arguments": map[string]string{"pr_url": "https://github.com/o/r/pull/1"}}) + } + if created != 1 { + t.Errorf("client created %d times; it must be shared so the ETag cache is reused", created) + } +} + +func TestCISummary(t *testing.T) { + tests := []struct { + r waitResult + want string + }{ + {waitResult{Owner: "o", Repo: "r", Number: 1, State: "success", TotalChecks: 3}, "CI of o/r#1 passed (3 check(s))."}, + {waitResult{Owner: "o", Repo: "r", Number: 1, State: "error"}, "CI of o/r#1 finished with error."}, + {waitResult{Owner: "o", Repo: "r", Number: 1, State: "merged"}, "o/r#1 was merged."}, + {waitResult{Owner: "o", Repo: "r", Number: 1, State: "pending", SHA: "abcdef123", TotalChecks: 2, PendingChecks: 1}, "CI of o/r#1 is pending: 1 of 2 check(s) still running on abcdef1."}, + } + for _, tt := range tests { + if got := ciSummary(&tt.r); got != tt.want { + t.Errorf("ciSummary(%s) = %q, want %q", tt.r.State, got, tt.want) + } + } +} From 8cbc891856378036b5b0b670c04d99af1fbf0f80 Mon Sep 17 00:00:00 2001 From: Julian Blacher Date: Sat, 17 Oct 2026 19:01:20 -0300 Subject: [PATCH 2/2] feat: auto-watch your PRs and review requests (`prw watch --mine`) Copying PR URLs into `prw watch` one by one was the main friction in day-to-day use. `prw watch --mine` and `prw watch --review-requested` subscribe to a GitHub search (author:@me / review-requested:@me, open, non-archived) instead of a fixed list: - Matches are added right away and tagged with their source; `prw run` (and `run --once`) re-runs the searches every 5 minutes, picking up subscriptions enabled while it is running. - Review-request PRs are dropped when the request goes away; "mine" PRs are left for `prw run` to notify about and drop when merged or closed. Incomplete or failed searches never remove anything. - PRs watched by hand are never touched; watching an auto-added PR by hand claims it. `prw unwatch --mine` disables the source and removes only the PRs it added. - `prw list` shows auto-watch and an ADDED BY column (`source` in --json); `prw config show` shows auto_watch. Also compare owner/repo case-insensitively, as GitHub does, so search results can't duplicate PRs watched with different casing. Verified live: 18 open PRs across 7 repos found and CI-checked, then cleanly removed by `unwatch --mine`. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 + README.md | 10 + TASKS.md | 4 +- cmd/prw/autowatch_test.go | 268 +++++++++++++++++++++++ cmd/prw/main.go | 338 +++++++++++++++++++++++------ internal/config/autowatch_test.go | 85 ++++++++ internal/config/config.go | 89 +++++++- internal/github/client.go | 69 ++++++ internal/github/search_test.go | 117 ++++++++++ internal/watcher/autowatch.go | 157 ++++++++++++++ internal/watcher/autowatch_test.go | 303 ++++++++++++++++++++++++++ internal/watcher/watcher.go | 27 ++- 12 files changed, 1392 insertions(+), 77 deletions(-) create mode 100644 cmd/prw/autowatch_test.go create mode 100644 internal/config/autowatch_test.go create mode 100644 internal/github/search_test.go create mode 100644 internal/watcher/autowatch.go create mode 100644 internal/watcher/autowatch_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index dd2d690..b989c4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `prw run` now notifies once (`merged` or `closed`) and automatically stops watching a PR once it is merged or closed, instead of polling it forever. This notification always fires, regardless of `--on`/`notification_filter`. ### Added +- Auto-watch: `prw watch --mine` follows all your open PRs and `prw watch --review-requested` the PRs where your review is requested, across every repository. Matches are added immediately; `prw run` refreshes the searches every 5 minutes. Review-request PRs are dropped when the request goes away; PRs watched by hand are never touched by it. `prw unwatch --mine` / `--review-requested` turns it off and removes the PRs it added. `prw list` gains an `ADDED BY` column (and `source` in `--json`); `prw config show` shows `auto_watch`. - `prw mcp`: a Model Context Protocol server over stdio for AI coding agents, with tools `get_ci_status`, `wait_for_ci` (progress notifications, cancellation, fail-fast), `list_watched`, `watch_pr` and `unwatch_pr`. Implemented without new dependencies (the official Go SDK needs Go 1.25). - Conditional requests (ETags): repeated polls of unchanged resources are answered with `304 Not Modified`, which doesn't count against the authenticated rate limit. Measured against the real API: after the first poll, 5 further polls of a PR (15 requests) consumed 0 quota. Previously each watched PR cost 3 requests per poll (~540/hour at the default 20s interval), so about 9 PRs exhausted the 5,000/hour limit. - Rate-limit handling: 403/429 rate-limit responses are told apart from missing-permission 403s. `prw run` stops the current round and pauses until the reset time; `prw wait` waits for the reset (still bounded by `--timeout`) instead of giving up. @@ -19,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Integration tests against the real GitHub API (`make integration`, build tag `integration`), run nightly by a new `Integration` workflow. They fail against the old status-only logic. ### Fixed +- PR owner and repository names are compared case-insensitively (as on GitHub), so `DevBlac/PRW#5` and `devblac/prw#5` are no longer two separate watch-list entries. - Requests without a token no longer send an empty `Authorization: Bearer ` header, which GitHub rejects with 401 even for public repositories. - `MultiNotifier` (used whenever more than one notification channel is enabled) stopped at the first failing notifier. A down webhook silently suppressed the console and native notifications that were supposed to run after it; now every notifier always runs and their errors are combined. - The README's "Prebuilt binaries" install snippet was pinned to `v0.2.0`, two releases behind the current `v0.3.0`. It now resolves the actual latest release tag first. diff --git a/README.md b/README.md index 4d5f6ff..669de8f 100644 --- a/README.md +++ b/README.md @@ -33,6 +33,7 @@ The tool is deliberately minimal. It does one thing well: watch PRs and tell you - **`prw wait`**: block until a PR's CI finishes and exit 0/1/2 — for scripts, pipelines and AI coding agents - **MCP server** (`prw mcp`): lets Claude Code, Claude Desktop, Cursor and other agents check and wait for PR CI as tools +- **Auto-watch**: `prw watch --mine` / `--review-requested` follow your PRs and review requests across all repositories, no URLs needed - **Auto-cleanup**: merged/closed PRs are notified once and dropped from the watch list by `prw run` - **Watch multiple PRs** from different repositories simultaneously - **Instant notifications** when CI status changes (pending, success, failure, error) @@ -143,6 +144,15 @@ prw watch https://github.com/owner/repo/pull/123 The tool will fetch the PR details and add it to your watch list. +**Or let prw find your PRs** — no URLs to copy: + +```bash +prw watch --mine # all your open PRs, in every repository +prw watch --review-requested # open PRs where your review is requested (incl. your teams) +``` + +These follow a GitHub search instead of a fixed list: matching PRs are added right away, and `prw run` searches again every 5 minutes, so new PRs show up by themselves. Merged or closed PRs are dropped after their final notification, and review-request PRs are also dropped once your review is no longer requested. `prw list` shows which PRs were added automatically (`ADDED BY`). Undo with `prw unwatch --mine` / `--review-requested`, which removes only the PRs that search added — PRs you watched by hand are kept. + ### 3. Start the watcher ```bash diff --git a/TASKS.md b/TASKS.md index d4ef03c..fdbec24 100644 --- a/TASKS.md +++ b/TASKS.md @@ -87,7 +87,9 @@ Legend — Impact: 1–5 · Effort: S (≤½ day), M (1–2 days), L (3+ days) - ✅ #13 `prw wait ` — done (exit codes 0/1/2/3/130, fail-fast, follows new pushes, retries transient errors, `--json`; verified live against devblac/prw#5 and real process exit codes) - ✅ #9 API efficiency — done (ETag/If-None-Match cache; measured live: 15 steady-state requests = 0 quota; rate-limit detection + pause in `run`, wait-for-reset in `wait`). GraphQL batching not needed for now. - ✅ #14 MCP server — done (`prw mcp`: 5 tools, progress + cancellation, protocol 2025-06-18/2025-03-26/2024-11-05; verified end-to-end over stdio against the real API with the token from `gh auth token`). Not yet tried inside a real MCP client. -- Next: #10 `prw watch --mine` / `--review-requested` +- ✅ #10 `prw watch --mine` / `--review-requested` — done (search-backed subscriptions refreshed every 5 min by `prw run`; verified live: 18 open PRs across 7 repos found, CI checked, cleanly removed by `unwatch --mine`) +- 🆕 #18 **Report "no CI" instead of `pending` forever** (found while testing #10: 14 of 18 real PRs have zero checks — repos without CI, or stacked PRs whose base isn't `main`). Today `prw wait` / `wait_for_ci` would wait until the timeout on them. Needs a grace period after the last push (CI takes a moment to register checks) before concluding "no CI". Impact 4, effort S–M. +- Next: #18, then #11 review & mergeability events ## Planned diff --git a/cmd/prw/autowatch_test.go b/cmd/prw/autowatch_test.go new file mode 100644 index 0000000..860334c --- /dev/null +++ b/cmd/prw/autowatch_test.go @@ -0,0 +1,268 @@ +package main + +import ( + "fmt" + "net/http" + "net/http/httptest" + "path/filepath" + "strings" + "testing" + + "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/github" +) + +// setupAutoWatchCmd points commands at a temp config (with a token) and a fake GitHub +// whose search returns mine/reviewRequested PR numbers in owner/repo. +func setupAutoWatchCmd(t *testing.T, mine, reviewRequested []int) *int { + t.Helper() + path := filepath.Join(t.TempDir(), ".prw", "config.json") + oldPath := config.ConfigPath + config.ConfigPath = func() (string, error) { return path, nil } + t.Cleanup(func() { config.ConfigPath = oldPath }) + cfg := config.DefaultConfig() + cfg.GitHubToken = "test-token" + if err := cfg.Save(); err != nil { + t.Fatal(err) + } + + searches := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/search/issues": + searches++ + numbers := mine + if strings.Contains(r.URL.Query().Get("q"), "review-requested:@me") { + numbers = reviewRequested + } + items := make([]string, len(numbers)) + for i, n := range numbers { + items[i] = fmt.Sprintf(`{"number":%d,"title":"PR %d","html_url":"https://github.com/owner/repo/pull/%d","pull_request":{}}`, n, n, n) + } + fmt.Fprintf(w, `{"total_count":%d,"items":[%s]}`, len(items), strings.Join(items, ",")) + case strings.HasPrefix(r.URL.Path, "/repos/owner/repo/pulls/"): + n := strings.TrimPrefix(r.URL.Path, "/repos/owner/repo/pulls/") + fmt.Fprintf(w, `{"number":%s,"title":"PR %s","state":"open","head":{"sha":"sha%s"}}`, n, n, n) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(server.Close) + oldClient := newGitHubClient + newGitHubClient = func(token string) *github.Client { + c := github.NewClient(token) + c.BaseURL = server.URL + c.HTTPClient = server.Client() + return c + } + t.Cleanup(func() { newGitHubClient = oldClient }) + + resetFlags := func() { + watchMine, watchReviewRequested, unwatchMine, unwatchReviewRequested = false, false, false, false + } + resetFlags() + t.Cleanup(resetFlags) + return &searches +} + +func watchListSummary(t *testing.T) string { + t.Helper() + cfg, err := config.Load() + if err != nil { + t.Fatal(err) + } + var parts []string + for _, pr := range cfg.WatchedPRs { + src := pr.Source + if src == "" { + src = "manual" + } + parts = append(parts, fmt.Sprintf("%d:%s", pr.Number, src)) + } + return strings.Join(parts, " ") +} + +func TestWatchCmd_MineAddsSubscriptionAndCurrentPRs(t *testing.T) { + searches := setupAutoWatchCmd(t, []int{1, 2}, nil) + watchMine = true + + out, err := captureStdout(func() error { return watchCmd.RunE(watchCmd, nil) }) + if err != nil { + t.Fatalf("watch --mine: %v", err) + } + for _, want := range []string{"Now auto-watching your open PRs.", "+ owner/repo#1 - PR 1", "+ owner/repo#2 - PR 2", "searches again every 5 minutes"} { + if !strings.Contains(out, want) { + t.Errorf("output missing %q:\n%s", want, out) + } + } + cfg, _ := config.Load() + if !cfg.HasAutoWatch(config.AutoWatchMine) || cfg.HasAutoWatch(config.AutoWatchReviewRequested) { + t.Errorf("AutoWatch = %v", cfg.AutoWatch) + } + if got := watchListSummary(t); got != "1:mine 2:mine" { + t.Errorf("watch list = %s", got) + } + + // Running it again is idempotent. + out, err = captureStdout(func() error { return watchCmd.RunE(watchCmd, nil) }) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out, "Already auto-watching your open PRs.") || !strings.Contains(out, "No new PRs to add right now.") { + t.Errorf("second run output:\n%s", out) + } + if *searches != 2 { + t.Errorf("searches = %d", *searches) + } +} + +func TestWatchCmd_BothSourcesAndAURL(t *testing.T) { + setupAutoWatchCmd(t, []int{1}, []int{2}) + watchMine, watchReviewRequested = true, true + + if _, err := captureStdout(func() error { + return watchCmd.RunE(watchCmd, []string{"https://github.com/owner/repo/pull/9"}) + }); err != nil { + t.Fatal(err) + } + if got := watchListSummary(t); got != "9:manual 1:mine 2:review-requested" { + t.Errorf("watch list = %s", got) + } +} + +func TestWatchCmd_ByHandClaimsAnAutoWatchedPR(t *testing.T) { + setupAutoWatchCmd(t, []int{1}, nil) + watchMine = true + captureStdout(func() error { return watchCmd.RunE(watchCmd, nil) }) + watchMine = false + + out, err := captureStdout(func() error { return watchCmd.RunE(watchCmd, []string{"https://github.com/owner/repo/pull/1"}) }) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out, "already being watched") { + t.Errorf("output: %s", out) + } + if got := watchListSummary(t); got != "1:manual" { + t.Errorf("watching by hand must claim the PR, got %s", got) + } + + // ... so turning auto-watch off keeps it. + unwatchMine = true + if _, err := captureStdout(func() error { return unwatchCmd.RunE(unwatchCmd, nil) }); err != nil { + t.Fatal(err) + } + if got := watchListSummary(t); got != "1:manual" { + t.Errorf("watch list after unwatch --mine = %s", got) + } +} + +func TestUnwatchCmd_MineRemovesSubscriptionAndItsPRs(t *testing.T) { + setupAutoWatchCmd(t, []int{1, 2}, []int{3}) + watchMine, watchReviewRequested = true, true + captureStdout(func() error { return watchCmd.RunE(watchCmd, []string{"https://github.com/owner/repo/pull/9"}) }) + + unwatchMine = true + out, err := captureStdout(func() error { return unwatchCmd.RunE(unwatchCmd, nil) }) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out, "Stopped auto-watching your open PRs; removed 2 PR(s) it had added.") { + t.Errorf("output: %s", out) + } + cfg, _ := config.Load() + if cfg.HasAutoWatch(config.AutoWatchMine) || !cfg.HasAutoWatch(config.AutoWatchReviewRequested) { + t.Errorf("AutoWatch = %v", cfg.AutoWatch) + } + if got := watchListSummary(t); got != "9:manual 3:review-requested" { + t.Errorf("watch list = %s", got) + } + + out, _ = captureStdout(func() error { return unwatchCmd.RunE(unwatchCmd, nil) }) + if !strings.Contains(out, "Was not auto-watching your open PRs.") { + t.Errorf("second unwatch --mine output: %s", out) + } +} + +func TestWatchCmd_Args(t *testing.T) { + setupAutoWatchCmd(t, nil, nil) + + if err := watchCmd.Args(watchCmd, nil); err == nil || !strings.Contains(err.Error(), "--mine") { + t.Errorf("no URL and no flag must fail with a hint, got %v", err) + } + if err := watchCmd.Args(watchCmd, []string{"a", "b"}); err == nil { + t.Error("two URLs must fail") + } + if err := watchCmd.Args(watchCmd, []string{"https://github.com/o/r/pull/1"}); err != nil { + t.Errorf("one URL: %v", err) + } + watchMine = true + if err := watchCmd.Args(watchCmd, nil); err != nil { + t.Errorf("--mine without URL: %v", err) + } + if err := watchCmd.Args(watchCmd, []string{"a", "b"}); err == nil { + t.Error("--mine with two URLs must fail") + } + + if err := unwatchCmd.Args(unwatchCmd, nil); err == nil { + t.Error("unwatch with nothing must fail") + } + unwatchReviewRequested = true + if err := unwatchCmd.Args(unwatchCmd, nil); err != nil { + t.Errorf("unwatch --review-requested: %v", err) + } +} + +func TestWatchCmd_MineNeedsToken(t *testing.T) { + setupAutoWatchCmd(t, nil, nil) + config.Update(func(c *config.Config) error { c.GitHubToken = ""; return nil }) + t.Setenv("GITHUB_TOKEN", "") + watchMine = true + + if err := watchCmd.RunE(watchCmd, nil); err != errMissingToken { + t.Errorf("err = %v, want errMissingToken", err) + } + if cfg, _ := config.Load(); len(cfg.AutoWatch) != 0 { + t.Error("auto-watch must not be enabled without a token") + } +} + +func TestListAndConfigShow_ShowAutoWatch(t *testing.T) { + setupAutoWatchCmd(t, []int{1}, nil) + watchMine = true + captureStdout(func() error { return watchCmd.RunE(watchCmd, []string{"https://github.com/owner/repo/pull/9"}) }) + + out, err := captureStdout(func() error { return listCmd.RunE(listCmd, nil) }) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out, "Auto-watching your open PRs (--mine).") || !strings.Contains(out, "ADDED BY") { + t.Errorf("list output:\n%s", out) + } + lines := strings.Split(out, "\n") + var row1, row9 string + for _, l := range lines { + if strings.Contains(l, "#1 ") { + row1 = l + } + if strings.Contains(l, "#9 ") { + row9 = l + } + } + if !strings.Contains(row1, "--mine") || !strings.Contains(row9, "you") { + t.Errorf("ADDED BY column wrong:\n%s", out) + } + + oldJSON := listJSON + listJSON = true + defer func() { listJSON = oldJSON }() + out, _ = captureStdout(func() error { return listCmd.RunE(listCmd, nil) }) + if !strings.Contains(out, `"source": "mine"`) { + t.Errorf("JSON list should include the source:\n%s", out) + } + + out, _ = captureStdout(func() error { return configShowCmd.RunE(configShowCmd, nil) }) + if !strings.Contains(out, "auto_watch: mine") { + t.Errorf("config show output:\n%s", out) + } +} diff --git a/cmd/prw/main.go b/cmd/prw/main.go index 911a5be..a7a7665 100644 --- a/cmd/prw/main.go +++ b/cmd/prw/main.go @@ -8,6 +8,7 @@ import ( "os" "os/signal" "strconv" + "strings" "syscall" "text/tabwriter" "time" @@ -52,6 +53,10 @@ func init() { rootCmd.AddCommand(completionCmd) listCmd.Flags().BoolVar(&listJSON, "json", false, "output watched PRs as JSON") + watchCmd.Flags().BoolVar(&watchMine, "mine", false, "automatically watch all your open PRs") + watchCmd.Flags().BoolVar(&watchReviewRequested, "review-requested", false, "automatically watch open PRs where your review is requested") + unwatchCmd.Flags().BoolVar(&unwatchMine, "mine", false, "stop auto-watching your open PRs and remove the PRs it added") + unwatchCmd.Flags().BoolVar(&unwatchReviewRequested, "review-requested", false, "stop auto-watching review requests and remove the PRs it added") runCmd.Flags().StringVar(¬ifyFilter, "on", "", "notify on: change, fail, or success") runCmd.Flags().BoolVar(&runOnce, "once", false, "check watched PRs once and exit") runCmd.Flags().BoolVar(¬ifyNative, "notify-native", false, "enable native OS notifications (macOS/Linux/Windows)") @@ -70,63 +75,187 @@ var errMissingToken = errors.New("missing GITHUB_TOKEN; run 'gh auth login', set // newGitHubClient allows tests to inject a custom GitHub client. var newGitHubClient = github.NewClient -var watchCmd = &cobra.Command{ - Use: "watch ", - Short: "Add a PR to the watch list", - Args: cobra.ExactArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { - prURL := args[0] +var ( + watchMine bool + watchReviewRequested bool + unwatchMine bool + unwatchReviewRequested bool +) - owner, repo, number, err := github.ParsePRURL(prURL) - if err != nil { - return fmt.Errorf("invalid PR URL: %w", err) - } +// autoWatchFlags returns the auto-watch sources selected by a command's flags. +func autoWatchFlags(mine, reviewRequested bool) []string { + var sources []string + if mine { + sources = append(sources, config.AutoWatchMine) + } + if reviewRequested { + sources = append(sources, config.AutoWatchReviewRequested) + } + return sources +} - cfg, err := config.Load() - if err != nil { - return fmt.Errorf("failed to load config: %w", err) +// prURLOrAutoWatchArgs requires a PR URL unless an auto-watch flag is set. +func prURLOrAutoWatchArgs(flagsSet func() bool) cobra.PositionalArgs { + return func(cmd *cobra.Command, args []string) error { + if flagsSet() { + return cobra.MaximumNArgs(1)(cmd, args) } - - token := cfg.GetToken() - if token == "" { - return errMissingToken + if len(args) != 1 { + return fmt.Errorf("expected a PR URL (or --mine / --review-requested), got %d argument(s)", len(args)) } + return nil + } +} - // Try to fetch the PR to validate it exists and get title - client := newGitHubClient(token) - pr, err := client.GetPullRequest(owner, repo, number) - if err != nil { - return fmt.Errorf("failed to fetch PR: %w", err) +var watchCmd = &cobra.Command{ + Use: "watch [PR_URL]", + Short: "Add a PR to the watch list, or follow your PRs automatically", + Long: `Add a pull request to the watch list. + +With --mine or --review-requested, prw instead follows a GitHub search and keeps +the watch list up to date by itself: matching PRs are added right away, and +'prw run' refreshes the search every 5 minutes. + + --mine your open PRs (author:@me), in every repository + --review-requested open PRs where your review is requested (incl. your teams) + +PRs added by --review-requested are dropped again once your review is no longer +requested. Merged or closed PRs are always dropped by 'prw run' after it notifies. +Undo with 'prw unwatch --mine' / 'prw unwatch --review-requested'.`, + Example: ` prw watch https://github.com/owner/repo/pull/123 + prw watch --mine + prw watch --mine --review-requested`, + Args: prURLOrAutoWatchArgs(func() bool { return watchMine || watchReviewRequested }), + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) == 1 { + if err := watchSinglePR(args[0]); err != nil { + return err + } } - - watchedPR := config.WatchedPR{ - Owner: owner, - Repo: repo, - Number: number, - Title: pr.Title, + if sources := autoWatchFlags(watchMine, watchReviewRequested); len(sources) > 0 { + return enableAutoWatch(sources) } + return nil + }, +} - // Re-read the config under a lock instead of saving the copy loaded before the - // network call: a running `prw run` may have updated the file in the meantime. - added := false - if err := config.Update(func(c *config.Config) error { - added = c.AddPR(watchedPR) - if !added { - return config.ErrNoChanges - } - return nil - }); err != nil { - return fmt.Errorf("failed to save config: %w", err) - } +func watchSinglePR(prURL string) error { + owner, repo, number, err := github.ParsePRURL(prURL) + if err != nil { + return fmt.Errorf("invalid PR URL: %w", err) + } + + cfg, err := config.Load() + if err != nil { + return fmt.Errorf("failed to load config: %w", err) + } + + token := cfg.GetToken() + if token == "" { + return errMissingToken + } + + // Try to fetch the PR to validate it exists and get title + client := newGitHubClient(token) + pr, err := client.GetPullRequest(owner, repo, number) + if err != nil { + return fmt.Errorf("failed to fetch PR: %w", err) + } + + watchedPR := config.WatchedPR{ + Owner: owner, + Repo: repo, + Number: number, + Title: pr.Title, + } + // Re-read the config under a lock instead of saving the copy loaded before the + // network call: a running `prw run` may have updated the file in the meantime. + added, claimed := false, false + if err := config.Update(func(c *config.Config) error { + added = c.AddPR(watchedPR) if !added { - fmt.Printf("PR %s/%s#%d is already being watched.\n", owner, repo, number) - return nil + // Watching by hand a PR that auto-watch added keeps it even if that + // auto-watch is turned off later. + claimed = c.MarkManual(owner, repo, number) + } + if !added && !claimed { + return config.ErrNoChanges } + return nil + }); err != nil { + return fmt.Errorf("failed to save config: %w", err) + } - fmt.Printf("Now watching: %s/%s#%d - %s\n", owner, repo, number, pr.Title) + if !added { + fmt.Printf("PR %s/%s#%d is already being watched.\n", owner, repo, number) return nil - }, + } + + fmt.Printf("Now watching: %s/%s#%d - %s\n", owner, repo, number, pr.Title) + return nil +} + +func enableAutoWatch(sources []string) error { + cfg, err := config.Load() + if err != nil { + return fmt.Errorf("failed to load config: %w", err) + } + token := cfg.GetToken() + if token == "" { + return errMissingToken + } + + var enabled []string + if err := config.Update(func(c *config.Config) error { + for _, source := range sources { + if c.EnableAutoWatch(source) { + enabled = append(enabled, source) + } + } + if len(enabled) == 0 { + return config.ErrNoChanges + } + return nil + }); err != nil { + return fmt.Errorf("failed to save config: %w", err) + } + for _, source := range sources { + state := "Now auto-watching" + if !contains(enabled, source) { + state = "Already auto-watching" + } + fmt.Printf("%s %s.\n", state, watcher.AutoWatchDescription[source]) + } + + // Run the searches now so the user sees the result immediately. + res, err := watcher.SyncAutoWatch(newGitHubClient(token), sources) + for _, warning := range res.Warnings { + fmt.Printf("Warning: %s\n", warning) + } + if err != nil { + return fmt.Errorf("auto-watch is enabled, but searching GitHub failed (prw run will retry): %w", err) + } + if len(res.Added) == 0 { + fmt.Println("No new PRs to add right now.") + } + for _, pr := range res.Added { + fmt.Printf(" + %s/%s#%d - %s\n", pr.Owner, pr.Repo, pr.Number, pr.Title) + } + for _, pr := range res.Removed { + fmt.Printf(" - %s/%s#%d (review no longer requested)\n", pr.Owner, pr.Repo, pr.Number) + } + fmt.Println("'prw run' keeps this list up to date (searches again every 5 minutes).") + return nil +} + +func contains(list []string, s string) bool { + for _, v := range list { + if v == s { + return true + } + } + return false } var listCmd = &cobra.Command{ @@ -142,14 +271,20 @@ var listCmd = &cobra.Command{ return outputJSONList(cfg.WatchedPRs) } + for _, source := range cfg.AutoWatch { + fmt.Printf("Auto-watching %s (--%s).\n", watcher.AutoWatchDescription[source], source) + } if len(cfg.WatchedPRs) == 0 { fmt.Println("No PRs being watched.") return nil } + if len(cfg.AutoWatch) > 0 { + fmt.Println() + } w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) - fmt.Fprintln(w, "REPO\tPR\tSTATUS\tLAST CHECKED\tTITLE") - fmt.Fprintln(w, "----\t--\t------\t------------\t-----") + fmt.Fprintln(w, "REPO\tPR\tSTATUS\tLAST CHECKED\tADDED BY\tTITLE") + fmt.Fprintln(w, "----\t--\t------\t------------\t--------\t-----") for _, pr := range cfg.WatchedPRs { repo := fmt.Sprintf("%s/%s", pr.Owner, pr.Repo) @@ -166,7 +301,11 @@ var listCmd = &cobra.Command{ if len(title) > 50 { title = title[:47] + "..." } - fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", repo, prNum, status, lastChecked, title) + addedBy := "you" + if pr.Source != "" { + addedBy = "--" + pr.Source + } + fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\n", repo, prNum, status, lastChecked, addedBy, title) } w.Flush() @@ -175,36 +314,89 @@ var listCmd = &cobra.Command{ } var unwatchCmd = &cobra.Command{ - Use: "unwatch ", - Short: "Remove a PR from the watch list", - Args: cobra.ExactArgs(1), + Use: "unwatch [PR_URL]", + Short: "Remove a PR from the watch list, or stop following your PRs", + Long: `Remove a pull request from the watch list. + +With --mine or --review-requested, stop that auto-watch and remove the PRs it +added. PRs you added by hand with 'prw watch ' are kept. + +Unwatching a single PR that auto-watch added only lasts until the next search +finds it again; use --mine / --review-requested to stop following those PRs.`, + Example: ` prw unwatch https://github.com/owner/repo/pull/123 + prw unwatch --review-requested`, + Args: prURLOrAutoWatchArgs(func() bool { return unwatchMine || unwatchReviewRequested }), RunE: func(cmd *cobra.Command, args []string) error { - prURL := args[0] - - owner, repo, number, err := github.ParsePRURL(prURL) - if err != nil { - return fmt.Errorf("invalid PR URL: %w", err) - } - - removed := false - if err := config.Update(func(c *config.Config) error { - removed = c.RemovePR(owner, repo, number) - if !removed { - return config.ErrNoChanges + if len(args) == 1 { + if err := unwatchSinglePR(args[0]); err != nil { + return err } - return nil - }); err != nil { - return fmt.Errorf("failed to update config: %w", err) } + if sources := autoWatchFlags(unwatchMine, unwatchReviewRequested); len(sources) > 0 { + return disableAutoWatch(sources) + } + return nil + }, +} +func unwatchSinglePR(prURL string) error { + owner, repo, number, err := github.ParsePRURL(prURL) + if err != nil { + return fmt.Errorf("invalid PR URL: %w", err) + } + + removed := false + if err := config.Update(func(c *config.Config) error { + removed = c.RemovePR(owner, repo, number) if !removed { - fmt.Printf("PR %s/%s#%d is not being watched.\n", owner, repo, number) - return nil + return config.ErrNoChanges } + return nil + }); err != nil { + return fmt.Errorf("failed to update config: %w", err) + } - fmt.Printf("Stopped watching: %s/%s#%d\n", owner, repo, number) + if !removed { + fmt.Printf("PR %s/%s#%d is not being watched.\n", owner, repo, number) return nil - }, + } + + fmt.Printf("Stopped watching: %s/%s#%d\n", owner, repo, number) + return nil +} + +func disableAutoWatch(sources []string) error { + type outcome struct { + wasOn bool + removed []config.WatchedPR + } + results := map[string]outcome{} + if err := config.Update(func(c *config.Config) error { + changed := false + for _, source := range sources { + wasOn, removed := c.DisableAutoWatch(source) + results[source] = outcome{wasOn, removed} + if wasOn || len(removed) > 0 { + changed = true + } + } + if !changed { + return config.ErrNoChanges + } + return nil + }); err != nil { + return fmt.Errorf("failed to update config: %w", err) + } + + for _, source := range sources { + r := results[source] + if !r.wasOn && len(r.removed) == 0 { + fmt.Printf("Was not auto-watching %s.\n", watcher.AutoWatchDescription[source]) + continue + } + fmt.Printf("Stopped auto-watching %s; removed %d PR(s) it had added.\n", watcher.AutoWatchDescription[source], len(r.removed)) + } + return nil } var runCmd = &cobra.Command{ @@ -296,6 +488,12 @@ var configShowCmd = &cobra.Command{ _, tokenSource := cfg.ResolveToken() fmt.Printf("github_token: %s\n", tokenSource) + autoWatch := "off" + if len(cfg.AutoWatch) > 0 { + autoWatch = strings.Join(cfg.AutoWatch, ", ") + } + fmt.Printf("auto_watch: %s\n", autoWatch) + fmt.Printf("\nWatched PRs: %d\n", len(cfg.WatchedPRs)) return nil @@ -424,6 +622,7 @@ type listPROutput struct { Status string `json:"status"` LastChecked *time.Time `json:"last_checked,omitempty"` Title string `json:"title,omitempty"` + Source string `json:"source,omitempty"` // auto-watch source that added it; empty if added by hand } func outputJSONList(prs []config.WatchedPR) error { @@ -447,6 +646,7 @@ func outputJSONList(prs []config.WatchedPR) error { Status: status, LastChecked: lastChecked, Title: pr.Title, + Source: pr.Source, }) } diff --git a/internal/config/autowatch_test.go b/internal/config/autowatch_test.go new file mode 100644 index 0000000..5c75e96 --- /dev/null +++ b/internal/config/autowatch_test.go @@ -0,0 +1,85 @@ +package config + +import ( + "encoding/json" + "strings" + "testing" +) + +func TestEnableAndDisableAutoWatch(t *testing.T) { + c := &Config{} + if !c.EnableAutoWatch(AutoWatchMine) || c.EnableAutoWatch(AutoWatchMine) { + t.Fatal("EnableAutoWatch must report the first enable only") + } + c.EnableAutoWatch(AutoWatchReviewRequested) + if !c.HasAutoWatch(AutoWatchMine) || !c.HasAutoWatch(AutoWatchReviewRequested) { + t.Fatalf("AutoWatch = %v", c.AutoWatch) + } + + c.WatchedPRs = []WatchedPR{ + {Owner: "o", Repo: "r", Number: 1, Source: AutoWatchMine}, + {Owner: "o", Repo: "r", Number: 2}, + {Owner: "o", Repo: "r", Number: 3, Source: AutoWatchReviewRequested}, + {Owner: "o", Repo: "r", Number: 4, Source: AutoWatchMine}, + } + wasOn, removed := c.DisableAutoWatch(AutoWatchMine) + if !wasOn || len(removed) != 2 || removed[0].Number != 1 || removed[1].Number != 4 { + t.Errorf("DisableAutoWatch(mine) = %v, %+v", wasOn, removed) + } + if len(c.WatchedPRs) != 2 || c.WatchedPRs[0].Number != 2 || c.WatchedPRs[1].Number != 3 { + t.Errorf("manual and other-source PRs must be kept: %+v", c.WatchedPRs) + } + if c.HasAutoWatch(AutoWatchMine) || !c.HasAutoWatch(AutoWatchReviewRequested) { + t.Errorf("AutoWatch = %v", c.AutoWatch) + } + + if wasOn, removed := c.DisableAutoWatch(AutoWatchMine); wasOn || len(removed) != 0 { + t.Error("disabling twice must be a no-op") + } + c.DisableAutoWatch(AutoWatchReviewRequested) + if c.AutoWatch != nil { + t.Errorf("AutoWatch should be nil when empty so it's omitted from JSON, got %#v", c.AutoWatch) + } +} + +func TestMarkManual(t *testing.T) { + c := &Config{WatchedPRs: []WatchedPR{{Owner: "o", Repo: "r", Number: 1, Source: AutoWatchMine}, {Owner: "o", Repo: "r", Number: 2}}} + if !c.MarkManual("O", "R", 1) || c.WatchedPRs[0].Source != "" { + t.Errorf("MarkManual should clear the source: %+v", c.WatchedPRs[0]) + } + if c.MarkManual("o", "r", 1) || c.MarkManual("o", "r", 2) || c.MarkManual("o", "r", 99) { + t.Error("MarkManual must report false when nothing changes") + } +} + +func TestPRMatchingIsCaseInsensitive(t *testing.T) { + c := &Config{} + c.AddPR(WatchedPR{Owner: "DevBlac", Repo: "PRW", Number: 5}) + if c.AddPR(WatchedPR{Owner: "devblac", Repo: "prw", Number: 5}) { + t.Error("owner/repo differing only in case must not create a duplicate") + } + if !c.ApplyPRState(WatchedPR{Owner: "devblac", Repo: "prw", Number: 5, LastKnownState: "success"}) || c.WatchedPRs[0].LastKnownState != "success" { + t.Error("ApplyPRState must match case-insensitively") + } + if !c.RemovePR("DEVBLAC", "prw", 5) || len(c.WatchedPRs) != 0 { + t.Error("RemovePR must match case-insensitively") + } +} + +func TestAutoWatchFieldsAreOmittedWhenUnused(t *testing.T) { + data, err := json.Marshal(&Config{WatchedPRs: []WatchedPR{{Owner: "o", Repo: "r", Number: 1}}}) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(data), "auto_watch") || strings.Contains(string(data), "source") { + t.Errorf("existing configs must not gain new keys: %s", data) + } +} + +func TestApplyPRStateKeepsSource(t *testing.T) { + c := &Config{WatchedPRs: []WatchedPR{{Owner: "o", Repo: "r", Number: 1, Source: AutoWatchMine}}} + c.ApplyPRState(WatchedPR{Owner: "o", Repo: "r", Number: 1, LastKnownState: "failure"}) + if c.WatchedPRs[0].Source != AutoWatchMine { + t.Error("saving polling state must not drop the auto-watch source") + } +} diff --git a/internal/config/config.go b/internal/config/config.go index 4a76593..fb78d24 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -28,10 +28,70 @@ type Config struct { NotificationFilter string `json:"notification_filter,omitempty"` NotificationNative bool `json:"notification_native,omitempty"` + // AutoWatch lists the searches (AutoWatchMine, AutoWatchReviewRequested) whose + // matching PRs are added to the watch list automatically. + AutoWatch []string `json:"auto_watch,omitempty"` + // Watched PRs WatchedPRs []WatchedPR `json:"watched_prs"` } +// Auto-watch sources. +const ( + AutoWatchMine = "mine" + AutoWatchReviewRequested = "review-requested" +) + +// AutoWatchSources are the valid auto-watch sources, in display order. +var AutoWatchSources = []string{AutoWatchMine, AutoWatchReviewRequested} + +// HasAutoWatch reports whether source is enabled. +func (c *Config) HasAutoWatch(source string) bool { + for _, s := range c.AutoWatch { + if s == source { + return true + } + } + return false +} + +// EnableAutoWatch turns source on and reports whether it was off. +func (c *Config) EnableAutoWatch(source string) bool { + if c.HasAutoWatch(source) { + return false + } + c.AutoWatch = append(c.AutoWatch, source) + return true +} + +// DisableAutoWatch turns source off and removes the PRs it added (PRs watched by hand +// are kept). It returns whether the source was on and the PRs removed. +func (c *Config) DisableAutoWatch(source string) (wasOn bool, removed []WatchedPR) { + kept := c.AutoWatch[:0] + for _, s := range c.AutoWatch { + if s == source { + wasOn = true + continue + } + kept = append(kept, s) + } + c.AutoWatch = kept + if len(c.AutoWatch) == 0 { + c.AutoWatch = nil + } + + prs := c.WatchedPRs[:0] + for _, pr := range c.WatchedPRs { + if pr.Source == source { + removed = append(removed, pr) + continue + } + prs = append(prs, pr) + } + c.WatchedPRs = prs + return wasOn, removed +} + // WatchedPR represents a pull request being watched. type WatchedPR struct { Owner string `json:"owner"` @@ -41,6 +101,9 @@ type WatchedPR struct { LastKnownState string `json:"last_known_state,omitempty"` LastChecked time.Time `json:"last_checked,omitempty"` Title string `json:"title,omitempty"` + // Source is the auto-watch source that added this PR, or empty if it was added by + // hand with `prw watch `. + Source string `json:"source,omitempty"` } // DefaultConfig returns a config with sensible defaults. @@ -213,7 +276,7 @@ func acquireLock(configPath string) (release func(), err error) { func (c *Config) ApplyPRState(pr WatchedPR) bool { for i := range c.WatchedPRs { w := &c.WatchedPRs[i] - if w.Owner == pr.Owner && w.Repo == pr.Repo && w.Number == pr.Number { + if samePR(*w, pr.Owner, pr.Repo, pr.Number) { w.LastKnownSHA = pr.LastKnownSHA w.LastKnownState = pr.LastKnownState w.LastChecked = pr.LastChecked @@ -262,10 +325,28 @@ func RemovePRs(prs []WatchedPR) error { }) } +// samePR reports whether pr is owner/repo#number. GitHub owner and repository names +// are case-insensitive, so "DevBlac/PRW" and "devblac/prw" are the same repository. +func samePR(pr WatchedPR, owner, repo string, number int) bool { + return pr.Number == number && strings.EqualFold(pr.Owner, owner) && strings.EqualFold(pr.Repo, repo) +} + +// MarkManual clears the auto-watch Source of a watched PR, so that disabling that +// auto-watch source no longer removes it. It reports whether anything changed. +func (c *Config) MarkManual(owner, repo string, number int) bool { + for i := range c.WatchedPRs { + if samePR(c.WatchedPRs[i], owner, repo, number) && c.WatchedPRs[i].Source != "" { + c.WatchedPRs[i].Source = "" + return true + } + } + return false +} + // AddPR adds a PR to the watched list if not already present. func (c *Config) AddPR(pr WatchedPR) bool { for _, existing := range c.WatchedPRs { - if existing.Owner == pr.Owner && existing.Repo == pr.Repo && existing.Number == pr.Number { + if samePR(existing, pr.Owner, pr.Repo, pr.Number) { return false } } @@ -276,7 +357,7 @@ func (c *Config) AddPR(pr WatchedPR) bool { // RemovePR removes a PR from the watched list. func (c *Config) RemovePR(owner, repo string, number int) bool { for i, pr := range c.WatchedPRs { - if pr.Owner == owner && pr.Repo == repo && pr.Number == number { + if samePR(pr, owner, repo, number) { c.WatchedPRs = append(c.WatchedPRs[:i], c.WatchedPRs[i+1:]...) return true } @@ -287,7 +368,7 @@ func (c *Config) RemovePR(owner, repo string, number int) bool { // UpdatePR updates the state of a watched PR. func (c *Config) UpdatePR(owner, repo string, number int, sha, state string) { for i := range c.WatchedPRs { - if c.WatchedPRs[i].Owner == owner && c.WatchedPRs[i].Repo == repo && c.WatchedPRs[i].Number == number { + if samePR(c.WatchedPRs[i], owner, repo, number) { c.WatchedPRs[i].LastKnownSHA = sha c.WatchedPRs[i].LastKnownState = state c.WatchedPRs[i].LastChecked = time.Now() diff --git a/internal/github/client.go b/internal/github/client.go index 11012cd..2a771c3 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -232,6 +232,75 @@ func (c *Client) GetCheckRuns(owner, repo, ref string) ([]CheckRun, error) { return runs, nil } +// SearchResultPR is a pull request returned by SearchPullRequests. +type SearchResultPR struct { + Owner string + Repo string + Number int + Title string +} + +// SearchResult holds the pull requests matching a search query. +type SearchResult struct { + PRs []SearchResultPR + // Total is how many items GitHub says match; it can exceed len(PRs) when the + // results were capped. + Total int + // Incomplete is set when GitHub timed out before finding every match, or when the + // results were capped, so absence from PRs doesn't prove a PR stopped matching. + Incomplete bool +} + +const ( + searchPerPage = 100 + maxSearchPages = 3 +) + +type searchIssuesResponse struct { + TotalCount int `json:"total_count"` + IncompleteResults bool `json:"incomplete_results"` + Items []struct { + Number int `json:"number"` + Title string `json:"title"` + HTMLURL string `json:"html_url"` + PullRequest *struct{} `json:"pull_request"` + } `json:"items"` +} + +// SearchPullRequests runs a GitHub issue search (e.g. "is:pr is:open author:@me") and +// returns the pull requests it finds, up to 300. Qualifiers like @me need a token. +func (c *Client) SearchPullRequests(query string) (*SearchResult, error) { + result := &SearchResult{} + for page := 1; page <= maxSearchPages; page++ { + path := fmt.Sprintf("/search/issues?q=%s&per_page=%d&page=%d", url.QueryEscape(query), searchPerPage, page) + var resp searchIssuesResponse + if err := c.getJSON(path, &resp); err != nil { + return nil, err + } + result.Total = resp.TotalCount + if resp.IncompleteResults { + result.Incomplete = true + } + for _, item := range resp.Items { + if item.PullRequest == nil { + continue // an issue, not a PR + } + owner, repo, number, err := ParsePRURL(item.HTMLURL) + if err != nil { + continue + } + result.PRs = append(result.PRs, SearchResultPR{Owner: owner, Repo: repo, Number: number, Title: item.Title}) + } + if len(resp.Items) < searchPerPage || page*searchPerPage >= resp.TotalCount { + return result, nil + } + } + if result.Total > len(result.PRs) { + result.Incomplete = true + } + return result, nil +} + // FetchCIStatus fetches commit statuses and check runs for a commit and aggregates them. func FetchCIStatus(c CIFetcher, owner, repo, ref string) (*CIStatus, error) { status, err := c.GetCombinedStatus(owner, repo, ref) diff --git a/internal/github/search_test.go b/internal/github/search_test.go new file mode 100644 index 0000000..3fcc3aa --- /dev/null +++ b/internal/github/search_test.go @@ -0,0 +1,117 @@ +package github + +import ( + "fmt" + "net/http" + "strconv" + "strings" + "testing" +) + +func searchItem(owner, repo string, number int, isPR bool) string { + pr := "" + if isPR { + pr = `,"pull_request":{"url":"x"}` + } + return fmt.Sprintf(`{"number":%d,"title":"%s %d","html_url":"https://github.com/%s/%s/pull/%d"%s}`, number, repo, number, owner, repo, number, pr) +} + +func TestSearchPullRequests_ParsesResultsAndSkipsIssues(t *testing.T) { + var gotQuery string + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/search/issues" { + t.Errorf("path = %s", r.URL.Path) + } + gotQuery = r.URL.Query().Get("q") + fmt.Fprintf(w, `{"total_count":3,"incomplete_results":false,"items":[%s,%s,%s]}`, + searchItem("Alice", "api", 1, true), searchItem("bob", "web", 2, false), searchItem("bob", "web", 3, true)) + }) + + res, err := c.SearchPullRequests("is:pr is:open author:@me") + if err != nil { + t.Fatal(err) + } + if gotQuery != "is:pr is:open author:@me" { + t.Errorf("query sent = %q (must round-trip through URL encoding)", gotQuery) + } + if len(res.PRs) != 2 || res.Incomplete { + t.Fatalf("result = %+v", res) + } + if p := res.PRs[0]; p.Owner != "Alice" || p.Repo != "api" || p.Number != 1 || p.Title != "api 1" { + t.Errorf("first PR = %+v", p) + } + if p := res.PRs[1]; p.Number != 3 { + t.Errorf("an issue must be skipped, got %+v", p) + } +} + +func TestSearchPullRequests_Paginates(t *testing.T) { + var pages []string + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + page := r.URL.Query().Get("page") + pages = append(pages, page) + n, _ := strconv.Atoi(page) + count := searchPerPage + if n == 2 { + count = 5 + } + items := make([]string, count) + for i := range items { + items[i] = searchItem("o", "r", n*1000+i, true) + } + fmt.Fprintf(w, `{"total_count":%d,"items":[%s]}`, searchPerPage+5, strings.Join(items, ",")) + }) + + res, err := c.SearchPullRequests("q") + if err != nil { + t.Fatal(err) + } + if len(res.PRs) != searchPerPage+5 || len(pages) != 2 || res.Incomplete { + t.Errorf("got %d PRs over pages %v, incomplete=%v", len(res.PRs), pages, res.Incomplete) + } +} + +func TestSearchPullRequests_CapsAndMarksIncomplete(t *testing.T) { + requests := 0 + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + requests++ + items := make([]string, searchPerPage) + for i := range items { + items[i] = searchItem("o", "r", requests*1000+i, true) + } + fmt.Fprintf(w, `{"total_count":5000,"items":[%s]}`, strings.Join(items, ",")) + }) + + res, err := c.SearchPullRequests("q") + if err != nil { + t.Fatal(err) + } + if requests != maxSearchPages || len(res.PRs) != maxSearchPages*searchPerPage { + t.Errorf("requests=%d PRs=%d", requests, len(res.PRs)) + } + if !res.Incomplete || res.Total != 5000 { + t.Errorf("capped results must be marked incomplete: %+v", res.Total) + } +} + +func TestSearchPullRequests_IncompleteFromGitHub(t *testing.T) { + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintf(w, `{"total_count":1,"incomplete_results":true,"items":[%s]}`, searchItem("o", "r", 1, true)) + }) + res, err := c.SearchPullRequests("q") + if err != nil { + t.Fatal(err) + } + if !res.Incomplete { + t.Error("incomplete_results from GitHub must be propagated") + } +} + +func TestSearchPullRequests_Error(t *testing.T) { + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + http.Error(w, `{"message":"Validation Failed"}`, http.StatusUnprocessableEntity) + }) + if _, err := c.SearchPullRequests("q"); err == nil || !strings.Contains(err.Error(), "422") { + t.Errorf("expected a 422 error, got %v", err) + } +} diff --git a/internal/watcher/autowatch.go b/internal/watcher/autowatch.go new file mode 100644 index 0000000..297fdd5 --- /dev/null +++ b/internal/watcher/autowatch.go @@ -0,0 +1,157 @@ +package watcher + +import ( + "fmt" + "strings" + "time" + + "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/github" +) + +// AutoWatchInterval is how often `prw run` refreshes the auto-watch searches. Search +// results change slowly and the search API has its own, lower rate limit (30/min), so +// this is much longer than the CI poll interval. +const AutoWatchInterval = 5 * time.Minute + +// autoWatchQueries maps each auto-watch source to its GitHub search query. +var autoWatchQueries = map[string]string{ + config.AutoWatchMine: "is:pr is:open archived:false author:@me", + config.AutoWatchReviewRequested: "is:pr is:open archived:false review-requested:@me", +} + +// AutoWatchDescription is a human-readable name for each source. +var AutoWatchDescription = map[string]string{ + config.AutoWatchMine: "your open PRs", + config.AutoWatchReviewRequested: "PRs where your review is requested", +} + +// Searcher runs GitHub pull request searches. +type Searcher interface { + SearchPullRequests(query string) (*github.SearchResult, error) +} + +// SyncResult reports what SyncAutoWatch changed. +type SyncResult struct { + Added []config.WatchedPR + Removed []config.WatchedPR + // Warnings are non-fatal problems, e.g. a search that failed or was truncated. + Warnings []string +} + +// SyncAutoWatch runs the search of every source in sources and updates the watch list: +// matching PRs that aren't watched yet are added (tagged with their source), and PRs +// added for "review-requested" that no longer match are removed, since the request +// was withdrawn or answered. PRs added for "mine" are not removed when they stop +// matching: that means they were merged or closed, and `prw run` should still notify +// about it before dropping them. PRs watched by hand are never touched. +// +// It returns an error only if every search failed; other problems are Warnings. +func SyncAutoWatch(client Searcher, sources []string) (*SyncResult, error) { + result := &SyncResult{} + found := map[string]*github.SearchResult{} + var lastErr error + + for _, source := range sources { + query, ok := autoWatchQueries[source] + if !ok { + result.Warnings = append(result.Warnings, fmt.Sprintf("unknown auto-watch source %q ignored", source)) + continue + } + res, err := client.SearchPullRequests(query) + if err != nil { + lastErr = err + result.Warnings = append(result.Warnings, fmt.Sprintf("search for %s failed: %v", AutoWatchDescription[source], err)) + continue + } + if res.Incomplete { + result.Warnings = append(result.Warnings, fmt.Sprintf("search for %s returned %d of %d results; PRs that no longer match are kept", + AutoWatchDescription[source], len(res.PRs), res.Total)) + } + found[source] = res + } + if len(found) == 0 && lastErr != nil { + return result, lastErr + } + + err := config.Update(func(c *config.Config) error { + changed := false + for _, source := range sources { + res, ok := found[source] + if !ok || !c.HasAutoWatch(source) { + continue // search failed, or the source was disabled meanwhile + } + + matches := make(map[string]bool, len(res.PRs)) + for _, pr := range res.PRs { + matches[prKey(pr.Owner, pr.Repo, pr.Number)] = true + added := config.WatchedPR{Owner: pr.Owner, Repo: pr.Repo, Number: pr.Number, Title: pr.Title, Source: source} + if c.AddPR(added) { + result.Added = append(result.Added, added) + changed = true + } + } + + if source != config.AutoWatchReviewRequested || res.Incomplete { + continue + } + kept := c.WatchedPRs[:0] + for _, pr := range c.WatchedPRs { + if pr.Source == source && !matches[prKey(pr.Owner, pr.Repo, pr.Number)] { + result.Removed = append(result.Removed, pr) + changed = true + continue + } + kept = append(kept, pr) + } + c.WatchedPRs = kept + } + if !changed { + return config.ErrNoChanges + } + return nil + }) + if err != nil { + return result, fmt.Errorf("failed to update watch list: %w", err) + } + return result, nil +} + +func prKey(owner, repo string, number int) string { + return strings.ToLower(owner) + "/" + strings.ToLower(repo) + "#" + fmt.Sprint(number) +} + +// syncAutoWatch refreshes the auto-watch searches during `prw run` and reports changes. +func (w *Watcher) syncAutoWatch() { + sources := w.config.AutoWatch + if fresh, err := config.Load(); err == nil { + sources = fresh.AutoWatch // `prw watch --mine` may have run since startup + } + if len(sources) == 0 { + return + } + w.lastAutoWatch = w.now() + + searcher, ok := w.client.(Searcher) + if !ok { + return + } + res, err := SyncAutoWatch(searcher, sources) + for _, warning := range res.Warnings { + fmt.Printf("Warning: auto-watch: %s\n", warning) + } + if err != nil { + // Per-search failures (including rate limits) are already in Warnings; this is + // either "every search failed" or a config write error. + if limited, _ := github.IsRateLimited(err); !limited { + fmt.Printf("Warning: auto-watch failed: %v\n", err) + } + return + } + for _, pr := range res.Added { + fmt.Printf("➕ Auto-watching %s/%s#%d - %s (%s)\n", pr.Owner, pr.Repo, pr.Number, pr.Title, AutoWatchDescription[pr.Source]) + } + for _, pr := range res.Removed { + fmt.Printf("➖ No longer watching %s/%s#%d - review no longer requested\n", pr.Owner, pr.Repo, pr.Number) + } +} diff --git a/internal/watcher/autowatch_test.go b/internal/watcher/autowatch_test.go new file mode 100644 index 0000000..08fbe63 --- /dev/null +++ b/internal/watcher/autowatch_test.go @@ -0,0 +1,303 @@ +package watcher + +import ( + "context" + "errors" + "fmt" + "sort" + "strings" + "sync" + "testing" + "time" + + "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/github" +) + +// fakeSearcher answers SearchPullRequests by source (looked up from the query). +type fakeSearcher struct { + mockGitHubClient + results map[string]*github.SearchResult + errs map[string]error + searches int +} + +func (f *fakeSearcher) SearchPullRequests(query string) (*github.SearchResult, error) { + f.searches++ + for source, q := range autoWatchQueries { + if q == query { + if err := f.errs[source]; err != nil { + return nil, err + } + if res := f.results[source]; res != nil { + return res, nil + } + return &github.SearchResult{}, nil + } + } + return nil, fmt.Errorf("unexpected query %q", query) +} + +func found(numbers ...int) *github.SearchResult { + res := &github.SearchResult{Total: len(numbers)} + for _, n := range numbers { + res.PRs = append(res.PRs, github.SearchResultPR{Owner: "owner", Repo: "repo", Number: n, Title: fmt.Sprintf("PR %d", n)}) + } + return res +} + +func saveConfig(t *testing.T, cfg *config.Config) { + t.Helper() + if err := cfg.Save(); err != nil { + t.Fatal(err) + } +} + +func watchList(t *testing.T) string { + t.Helper() + cfg, err := config.Load() + if err != nil { + t.Fatal(err) + } + var items []string + for _, pr := range cfg.WatchedPRs { + src := pr.Source + if src == "" { + src = "manual" + } + items = append(items, fmt.Sprintf("%d:%s", pr.Number, src)) + } + sort.Strings(items) + return strings.Join(items, " ") +} + +func TestSyncAutoWatch_AddsMatchesTaggedWithSource(t *testing.T) { + useTempConfig(t) + saveConfig(t, &config.Config{ + AutoWatch: []string{config.AutoWatchMine, config.AutoWatchReviewRequested}, + WatchedPRs: []config.WatchedPR{{Owner: "owner", Repo: "repo", Number: 1}}, // watched by hand + }) + s := &fakeSearcher{results: map[string]*github.SearchResult{ + config.AutoWatchMine: found(1, 2), + config.AutoWatchReviewRequested: found(3), + }} + + res, err := SyncAutoWatch(s, []string{config.AutoWatchMine, config.AutoWatchReviewRequested}) + if err != nil { + t.Fatal(err) + } + if len(res.Added) != 2 || len(res.Removed) != 0 { + t.Errorf("added=%+v removed=%+v", res.Added, res.Removed) + } + if got := watchList(t); got != "1:manual 2:mine 3:review-requested" { + t.Errorf("watch list = %s (a PR watched by hand must stay manual)", got) + } + cfg, _ := config.Load() + for _, pr := range cfg.WatchedPRs { + if pr.Number == 2 && pr.Title != "PR 2" { + t.Errorf("title should come from the search result, got %q", pr.Title) + } + } +} + +func TestSyncAutoWatch_DropsWithdrawnReviewRequestsButKeepsMine(t *testing.T) { + useTempConfig(t) + saveConfig(t, &config.Config{ + AutoWatch: []string{config.AutoWatchMine, config.AutoWatchReviewRequested}, + WatchedPRs: []config.WatchedPR{ + {Owner: "owner", Repo: "repo", Number: 10, Source: config.AutoWatchMine}, // merged: no longer matches + {Owner: "owner", Repo: "repo", Number: 20, Source: config.AutoWatchReviewRequested}, // review withdrawn + {Owner: "owner", Repo: "repo", Number: 21, Source: config.AutoWatchReviewRequested}, // still requested + {Owner: "owner", Repo: "repo", Number: 30}, // manual + }, + }) + s := &fakeSearcher{results: map[string]*github.SearchResult{ + config.AutoWatchMine: found(), + config.AutoWatchReviewRequested: found(21), + }} + + res, err := SyncAutoWatch(s, []string{config.AutoWatchMine, config.AutoWatchReviewRequested}) + if err != nil { + t.Fatal(err) + } + if len(res.Removed) != 1 || res.Removed[0].Number != 20 { + t.Errorf("removed = %+v", res.Removed) + } + if got := watchList(t); got != "10:mine 21:review-requested 30:manual" { + t.Errorf("watch list = %s (mine PRs are left for prw run to notify and drop)", got) + } +} + +func TestSyncAutoWatch_IncompleteResultsNeverRemove(t *testing.T) { + useTempConfig(t) + saveConfig(t, &config.Config{ + AutoWatch: []string{config.AutoWatchReviewRequested}, + WatchedPRs: []config.WatchedPR{{Owner: "owner", Repo: "repo", Number: 20, Source: config.AutoWatchReviewRequested}}, + }) + partial := found(21) + partial.Incomplete, partial.Total = true, 2 + s := &fakeSearcher{results: map[string]*github.SearchResult{config.AutoWatchReviewRequested: partial}} + + res, err := SyncAutoWatch(s, []string{config.AutoWatchReviewRequested}) + if err != nil { + t.Fatal(err) + } + if got := watchList(t); got != "20:review-requested 21:review-requested" { + t.Errorf("watch list = %s", got) + } + if len(res.Warnings) != 1 || !strings.Contains(res.Warnings[0], "returned 1 of 2") { + t.Errorf("warnings = %v", res.Warnings) + } +} + +func TestSyncAutoWatch_OneFailedSearchDoesNotBlockTheOther(t *testing.T) { + useTempConfig(t) + saveConfig(t, &config.Config{ + AutoWatch: []string{config.AutoWatchMine, config.AutoWatchReviewRequested}, + WatchedPRs: []config.WatchedPR{{Owner: "owner", Repo: "repo", Number: 20, Source: config.AutoWatchReviewRequested}}, + }) + s := &fakeSearcher{ + results: map[string]*github.SearchResult{config.AutoWatchMine: found(1)}, + errs: map[string]error{config.AutoWatchReviewRequested: errors.New("boom")}, + } + + res, err := SyncAutoWatch(s, []string{config.AutoWatchMine, config.AutoWatchReviewRequested}) + if err != nil { + t.Fatalf("a partial failure is a warning, got %v", err) + } + if got := watchList(t); got != "1:mine 20:review-requested" { + t.Errorf("watch list = %s (failed search must not remove its PRs)", got) + } + if len(res.Warnings) != 1 || !strings.Contains(res.Warnings[0], "boom") { + t.Errorf("warnings = %v", res.Warnings) + } +} + +func TestSyncAutoWatch_AllSearchesFailing(t *testing.T) { + useTempConfig(t) + saveConfig(t, &config.Config{AutoWatch: []string{config.AutoWatchMine}}) + boom := errors.New("network down") + s := &fakeSearcher{errs: map[string]error{config.AutoWatchMine: boom}} + + if _, err := SyncAutoWatch(s, []string{config.AutoWatchMine}); !errors.Is(err, boom) { + t.Errorf("err = %v, want the search error", err) + } +} + +func TestSyncAutoWatch_SkipsSourceDisabledMeanwhile(t *testing.T) { + useTempConfig(t) + saveConfig(t, &config.Config{}) // `prw unwatch --mine` ran after the caller read the config + s := &fakeSearcher{results: map[string]*github.SearchResult{config.AutoWatchMine: found(1)}} + + res, err := SyncAutoWatch(s, []string{config.AutoWatchMine}) + if err != nil { + t.Fatal(err) + } + if len(res.Added) != 0 || watchList(t) != "" { + t.Errorf("a disabled source must not add PRs: %+v / %s", res.Added, watchList(t)) + } +} + +func TestSyncAutoWatch_UnknownSource(t *testing.T) { + useTempConfig(t) + saveConfig(t, &config.Config{AutoWatch: []string{"bogus"}}) + res, err := SyncAutoWatch(&fakeSearcher{}, []string{"bogus"}) + if err != nil { + t.Fatal(err) + } + if len(res.Warnings) != 1 || !strings.Contains(res.Warnings[0], "unknown auto-watch source") { + t.Errorf("warnings = %v", res.Warnings) + } +} + +func TestRun_SyncsOnStartupEvenWithEmptyWatchList(t *testing.T) { + useTempConfig(t) + cfg := &config.Config{PollIntervalSeconds: 60, AutoWatch: []string{config.AutoWatchMine}} + saveConfig(t, cfg) + s := &fakeSearcher{results: map[string]*github.SearchResult{config.AutoWatchMine: found(7)}} + s.prs = map[string]*github.PullRequest{"owner/repo/7": newPR(7, "sha7")} + s.statuses = map[string]*github.CombinedStatus{"sha7": {State: "success", TotalCount: 1}} + + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + New(s, cfg, &mockNotifier{}).Run(ctx) + + if s.searches != 1 { + t.Errorf("searches = %d, want 1 on startup", s.searches) + } + loaded, _ := config.Load() + if len(loaded.WatchedPRs) != 1 || loaded.WatchedPRs[0].LastKnownState != "success" { + t.Errorf("the auto-watched PR should be added and checked on startup: %+v", loaded.WatchedPRs) + } +} + +func TestRun_RefreshesSearchesOnlyEveryAutoWatchInterval(t *testing.T) { + useTempConfig(t) + cfg := &config.Config{PollIntervalSeconds: 1, AutoWatch: []string{config.AutoWatchMine}} + saveConfig(t, cfg) + s := &fakeSearcher{} + w := New(s, cfg, &mockNotifier{}) + + start := time.Now() + var mu sync.Mutex + clock := start + w.now = func() time.Time { + mu.Lock() + defer mu.Unlock() + return clock + } + + ctx, cancel := context.WithTimeout(context.Background(), 2500*time.Millisecond) + defer cancel() + go func() { + // After ~1.5s (startup + one tick), jump the clock past the interval. + time.Sleep(1500 * time.Millisecond) + mu.Lock() + clock = start.Add(AutoWatchInterval + time.Second) + mu.Unlock() + }() + w.Run(ctx) + + if s.searches != 2 { + t.Errorf("searches = %d, want 2 (startup + once after the interval elapsed)", s.searches) + } +} + +func TestRunOnce_SyncsAutoWatch(t *testing.T) { + useTempConfig(t) + cfg := &config.Config{AutoWatch: []string{config.AutoWatchReviewRequested}} + saveConfig(t, cfg) + s := &fakeSearcher{results: map[string]*github.SearchResult{config.AutoWatchReviewRequested: found(3)}} + s.prs = map[string]*github.PullRequest{"owner/repo/3": newPR(3, "sha3")} + + if err := New(s, cfg, &mockNotifier{}).RunOnce(context.Background()); err != nil { + t.Fatal(err) + } + if got := watchList(t); got != "3:review-requested" { + t.Errorf("watch list = %s", got) + } +} + +func TestRun_PicksUpAutoWatchEnabledWhileRunning(t *testing.T) { + useTempConfig(t) + cfg := &config.Config{PollIntervalSeconds: 1, WatchedPRs: []config.WatchedPR{watched(1, "success")}} + saveConfig(t, cfg) + s := &fakeSearcher{results: map[string]*github.SearchResult{config.AutoWatchMine: found(2)}} + s.prs = map[string]*github.PullRequest{"owner/repo/1": newPR(1, "sha1"), "owner/repo/2": newPR(2, "sha2")} + + ctx, cancel := context.WithTimeout(context.Background(), 1500*time.Millisecond) + defer cancel() + go func() { + time.Sleep(300 * time.Millisecond) + // `prw watch --mine` in another terminal. + config.Update(func(c *config.Config) error { c.EnableAutoWatch(config.AutoWatchMine); return nil }) + }() + New(s, cfg, &mockNotifier{}).Run(ctx) + + if s.searches != 1 { + t.Errorf("searches = %d, want 1 after auto-watch was enabled", s.searches) + } + if got := watchList(t); got != "1:manual 2:mine" { + t.Errorf("watch list = %s", got) + } +} diff --git a/internal/watcher/watcher.go b/internal/watcher/watcher.go index 81f29d2..1dc9ada 100644 --- a/internal/watcher/watcher.go +++ b/internal/watcher/watcher.go @@ -34,8 +34,12 @@ type Watcher struct { pausedUntil time.Time // now is the clock; tests replace it. now func() time.Time + // lastAutoWatch is when the auto-watch searches last ran. + lastAutoWatch time.Time } +const noPRsMessage = "No PRs being watched. Add some with 'prw watch ', or 'prw watch --mine' to follow your open PRs." + // defaultRateLimitPause is used when GitHub doesn't say when the limit resets. const defaultRateLimitPause = time.Minute @@ -56,12 +60,19 @@ func (w *Watcher) Run(ctx context.Context) error { defer ticker.Stop() fmt.Printf("Starting watcher with %d second poll interval...\n", w.config.PollIntervalSeconds) - if len(w.config.WatchedPRs) == 0 { - fmt.Println("No PRs being watched. Add some with 'prw watch '.") + if len(w.config.WatchedPRs) == 0 && len(w.config.AutoWatch) == 0 { + fmt.Println(noPRsMessage) return nil } + for _, source := range w.config.AutoWatch { + fmt.Printf("Auto-watching %s (refreshed every %s).\n", AutoWatchDescription[source], AutoWatchInterval) + } // Check immediately on startup + if len(w.config.AutoWatch) > 0 { + w.syncAutoWatch() + w.reloadWatchedPRs() + } w.checkAllPRs() for { @@ -74,6 +85,10 @@ func (w *Watcher) Run(ctx context.Context) error { continue // rate limited; the pause message was printed when it started } w.reloadWatchedPRs() + if len(w.config.AutoWatch) > 0 && w.now().Sub(w.lastAutoWatch) >= AutoWatchInterval { + w.syncAutoWatch() + w.reloadWatchedPRs() + } w.checkAllPRs() } } @@ -91,13 +106,19 @@ func (w *Watcher) reloadWatchedPRs() { fmt.Printf("Watch list changed: now watching %d PR(s).\n", len(fresh.WatchedPRs)) } w.config.WatchedPRs = fresh.WatchedPRs + // `prw watch --mine` / `prw unwatch --mine` may have changed the searches too. + w.config.AutoWatch = fresh.AutoWatch } // RunOnce checks all watched PRs a single time and returns. func (w *Watcher) RunOnce(ctx context.Context) error { fmt.Printf("Running one-time check with %d second poll interval...\n", w.config.PollIntervalSeconds) + if len(w.config.AutoWatch) > 0 { + w.syncAutoWatch() + w.reloadWatchedPRs() + } if len(w.config.WatchedPRs) == 0 { - fmt.Println("No PRs being watched. Add some with 'prw watch '.") + fmt.Println(noPRsMessage) return nil } w.checkAllPRs()