From c17b275d961f5022cfc948bff15abf4bcdb40268 Mon Sep 17 00:00:00 2001 From: jmeridth Date: Fri, 8 May 2026 09:39:53 -0500 Subject: [PATCH 1/2] feat: add ignore_drafts option to filter draft PRs from inbox MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What Adds an `ignore_drafts` config field (default false) and a matching `--ignore-drafts` flag on `zen inbox`. When enabled, draft PRs are filtered out of inbox results, watch-daemon poll output, and the MCP inbox tool. The filter is applied at the GitHub search layer: `draft:false` for GraphQL queries and `--draft=false` for `gh pr list`. ## Why Avoid spending review attention on PRs that aren't ready for review. The CLI flag overrides the persisted config in either direction for single-run experiments. ## Notes - The setting is honored by the watch daemon and MCP zen_inbox tool, not just zen inbox — a user enabling this will also stop receiving daemon notifications for drafts. Consistent on purpose, but worth flagging. - Default behavior is unchanged for existing configs (drafts still shown). - The flag uses cmd.Flags().Changed("ignore-drafts") so it only overrides config when explicitly set; bare zen inbox always uses config. - --draft=false is a documented gh pr list flag; no new dependency. Signed-off-by: jmeridth --- README.md | 11 +++-- cmd/inbox.go | 39 +++++++++------- cmd/watch.go | 2 +- docs/configuration.md | 6 +++ internal/config/config.go | 17 +++---- internal/config/config_test.go | 55 +++++++++++++++++++++++ internal/github/queries.go | 67 +++++++++++++++++++--------- internal/github/queries_test.go | 79 +++++++++++++++++++++++++++++++-- internal/mcp/tools.go | 2 +- 9 files changed, 225 insertions(+), 53 deletions(-) diff --git a/README.md b/README.md index 641d150..9d0450f 100644 --- a/README.md +++ b/README.md @@ -45,12 +45,15 @@ When a PR shows up in your inbox, `zen review ` opens that PR in a new t - **Open PRs touching paths you watch** — for staying aware of areas you care about. ```bash -zen inbox # everything, filtered by configured authors -zen inbox --all # from all authors -zen inbox --path pkg/sts # PRs touching specific paths -zen inbox --repo other-repo # different repo +zen inbox # everything, filtered by configured authors +zen inbox --all # from all authors +zen inbox --path pkg/sts # PRs touching specific paths +zen inbox --repo other-repo # different repo +zen inbox --ignore-drafts # skip drafts on this run (or set ignore_drafts: true in config) ``` +Drafts are shown by default. To skip drafts permanently, set `ignore_drafts: true` in your config — see [docs/configuration.md](docs/configuration.md). The `--ignore-drafts` flag overrides the config for a single run. + Example output: ``` diff --git a/cmd/inbox.go b/cmd/inbox.go index 796553e..baa516a 100644 --- a/cmd/inbox.go +++ b/cmd/inbox.go @@ -20,11 +20,12 @@ var inboxCmd = &cobra.Command{ } var ( - inboxRepo string - inboxAuthors string - inboxAll bool - inboxPathFilter string - inboxLimit int + inboxRepo string + inboxAuthors string + inboxAll bool + inboxPathFilter string + inboxLimit int + inboxIgnoreDrafts bool ) func init() { @@ -33,6 +34,7 @@ func init() { inboxCmd.Flags().BoolVar(&inboxAll, "all", false, "Show from all authors") inboxCmd.Flags().StringVarP(&inboxPathFilter, "path", "p", "", "List PRs touching files under DIR") inboxCmd.Flags().IntVar(&inboxLimit, "limit", 100, "Max PRs to scan when using --path") + inboxCmd.Flags().BoolVar(&inboxIgnoreDrafts, "ignore-drafts", false, "Skip draft PRs (overrides config when set)") rootCmd.AddCommand(inboxCmd) } @@ -46,7 +48,7 @@ type InboxPR struct { MatchedCount int `json:"matched_count,omitempty"` } -func runInbox(_ *cobra.Command, _ []string) error { +func runInbox(cmd *cobra.Command, _ []string) error { repos := []string{inboxRepo} if inboxRepo == "" { repos = cfg.RepoNames() @@ -60,6 +62,11 @@ func runInbox(_ *cobra.Command, _ []string) error { authors = nil } + ignoreDrafts := cfg.IgnoreDrafts + if cmd.Flags().Changed("ignore-drafts") { + ignoreDrafts = inboxIgnoreDrafts + } + // Cache current user once for all repos. ctx := context.Background() currentUser, _ := ghpkg.GetCurrentUser(ctx) @@ -70,7 +77,7 @@ func runInbox(_ *cobra.Command, _ []string) error { hasResults := false for _, repo := range repos { - found, err := runInboxForRepo(repo, authors, currentUser) + found, err := runInboxForRepo(repo, authors, currentUser, ignoreDrafts) if err != nil { return err } @@ -100,14 +107,14 @@ func runInbox(_ *cobra.Command, _ []string) error { return nil } -func runInboxForRepo(repo string, authors []string, currentUser string) (bool, error) { +func runInboxForRepo(repo string, authors []string, currentUser string, ignoreDrafts bool) (bool, error) { ctx := context.Background() fullRepo := cfg.RepoFullName(repo) localPRs := getLocalPRNumbers(repo) hasResults := false if inboxPathFilter != "" { - prs, err := fetchPRsByPath(ctx, fullRepo, inboxPathFilter, authors) + prs, err := fetchPRsByPath(ctx, fullRepo, inboxPathFilter, authors, ignoreDrafts) if err != nil { return false, err } @@ -124,11 +131,11 @@ func runInboxForRepo(repo string, authors []string, currentUser string) (bool, e g, gctx := errgroup.WithContext(ctx) g.Go(func() error { - reviews, reviewsErr = ghpkg.GetReviewRequests(gctx, fullRepo) + reviews, reviewsErr = ghpkg.GetReviewRequests(gctx, fullRepo, ignoreDrafts) return nil }) g.Go(func() error { - approved, approvedErr = ghpkg.GetApprovedUnmerged(gctx, fullRepo) + approved, approvedErr = ghpkg.GetApprovedUnmerged(gctx, fullRepo, ignoreDrafts) return nil }) _ = g.Wait() @@ -150,7 +157,7 @@ func runInboxForRepo(repo string, authors []string, currentUser string) (bool, e } if len(cfg.WatchPaths) > 0 { - watched, others, err := fetchOpenPRs(ctx, fullRepo, currentUser) + watched, others, err := fetchOpenPRs(ctx, fullRepo, currentUser, ignoreDrafts) if err == nil { if len(watched) > 0 { hasResults = true @@ -216,10 +223,10 @@ func filterLocalPRs(prs []InboxPR, local map[int]bool) []InboxPR { return pending } -func fetchPRsByPath(ctx context.Context, fullRepo, pathPrefix string, authors []string) ([]InboxPR, error) { +func fetchPRsByPath(ctx context.Context, fullRepo, pathPrefix string, authors []string, ignoreDrafts bool) ([]InboxPR, error) { pathPrefix = strings.TrimSuffix(pathPrefix, "/") - prs, err := ghpkg.ListOpenPRs(ctx, fullRepo, inboxLimit) + prs, err := ghpkg.ListOpenPRs(ctx, fullRepo, inboxLimit, ignoreDrafts) if err != nil { return nil, err } @@ -289,8 +296,8 @@ func fetchPRsByPath(ctx context.Context, fullRepo, pathPrefix string, authors [] // fetchOpenPRs splits recent open PRs into two groups: those touching watched // paths and all others. The current user's PRs are excluded from both. -func fetchOpenPRs(ctx context.Context, fullRepo string, currentUser string) ([]InboxPR, []InboxPR, error) { - prs, err := ghpkg.ListOpenPRs(ctx, fullRepo, 30) +func fetchOpenPRs(ctx context.Context, fullRepo string, currentUser string, ignoreDrafts bool) ([]InboxPR, []InboxPR, error) { + prs, err := ghpkg.ListOpenPRs(ctx, fullRepo, 30, ignoreDrafts) if err != nil { return nil, nil, err } diff --git a/cmd/watch.go b/cmd/watch.go index 82d4a44..be706c2 100644 --- a/cmd/watch.go +++ b/cmd/watch.go @@ -406,7 +406,7 @@ func saveState(seenPRs map[string]bool, prCount int) { } func pollOnce(ctx context.Context, seenPRs map[string]bool, queue workqueue.Interface, rec *reconciler.SetupReconciler) { - reviews, err := ghpkg.GetReviewRequests(ctx, "chainguard-dev/mono") + reviews, err := ghpkg.GetReviewRequests(ctx, "chainguard-dev/mono", cfg.IgnoreDrafts) if err != nil { fmt.Printf("[%s] Error fetching reviews: %v\n", time.Now().Format(time.RFC3339), err) return diff --git a/docs/configuration.md b/docs/configuration.md index 9271850..289d7f5 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -37,6 +37,12 @@ terminal: iterm # or "ghostty" # If unset, falls back to `git config user.name` (spaces → hyphens), then no prefix. branch_prefix: mgreau +# Skip draft PRs in `zen inbox`, watch notifications, and the MCP inbox tool. +# Defaults to false (drafts are shown). Set to true to skip drafts so you don't +# review something that isn't ready. Override on a single run with +# `--ignore-drafts=false`. +ignore_drafts: true + watch: dispatch_interval: "10s" # How often to process queued work cleanup_interval: "1h" # How often to scan for merged PRs diff --git a/internal/config/config.go b/internal/config/config.go index f55c60f..ab9e891 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -14,14 +14,15 @@ import ( // Config holds the complete zen configuration. type Config struct { - Repos map[string]RepoConfig `yaml:"repos"` - WatchPaths []string `yaml:"watch_paths"` - Authors []string `yaml:"authors"` - PollInterval string `yaml:"poll_interval"` - ClaudeBin string `yaml:"claude_bin"` - Terminal string `yaml:"terminal"` // "iterm" or "ghostty" - BranchPrefix string `yaml:"branch_prefix"` - Watch WatchConfig `yaml:"watch"` + Repos map[string]RepoConfig `yaml:"repos"` + WatchPaths []string `yaml:"watch_paths"` + Authors []string `yaml:"authors"` + PollInterval string `yaml:"poll_interval"` + ClaudeBin string `yaml:"claude_bin"` + Terminal string `yaml:"terminal"` // "iterm" or "ghostty" + BranchPrefix string `yaml:"branch_prefix"` + IgnoreDrafts bool `yaml:"ignore_drafts"` + Watch WatchConfig `yaml:"watch"` } // WatchConfig holds configuration for the watch daemon's workqueue behavior. diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 482a6b4..54f423a 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -213,6 +213,61 @@ func TestExpandPaths(t *testing.T) { } } +func TestLoadIgnoreDrafts(t *testing.T) { + tests := []struct { + name string + yaml string + want bool + }{ + { + name: "omitted defaults to false", + yaml: `repos: + m: + full_name: o/m + base_path: /tmp/m +`, + want: false, + }, + { + name: "explicit true", + yaml: `repos: + m: + full_name: o/m + base_path: /tmp/m +ignore_drafts: true +`, + want: true, + }, + { + name: "explicit false", + yaml: `repos: + m: + full_name: o/m + base_path: /tmp/m +ignore_drafts: false +`, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + zenDir := filepath.Join(tmpDir, ".zen") + os.MkdirAll(zenDir, 0o755) + os.WriteFile(filepath.Join(zenDir, "config.yaml"), []byte(tt.yaml), 0o644) + + cfg, err := Load() + if err != nil { + t.Fatalf("Load() error: %v", err) + } + if cfg.IgnoreDrafts != tt.want { + t.Errorf("IgnoreDrafts = %v, want %v", cfg.IgnoreDrafts, tt.want) + } + }) + } +} + func TestWatchConfigDefaults(t *testing.T) { w := WatchConfig{} diff --git a/internal/github/queries.go b/internal/github/queries.go index 09bd6fe..cfbd11f 100644 --- a/internal/github/queries.go +++ b/internal/github/queries.go @@ -72,9 +72,40 @@ func GetCurrentUser(ctx context.Context) (string, error) { return strings.TrimSpace(string(out)), nil } +// buildReviewRequestQueries returns the two GitHub search query strings used +// by GetReviewRequests: requested-reviews and re-review queries. +func buildReviewRequestQueries(repoFilter string, ignoreDrafts bool) (string, string) { + repoClause := "" + if repoFilter != "" { + repoClause = " repo:" + repoFilter + } + draftClause := "" + if ignoreDrafts { + draftClause = " draft:false" + } + q1 := fmt.Sprintf("is:pr is:open review-requested:@me%s%s", repoClause, draftClause) + q2 := fmt.Sprintf("is:pr is:open reviewed-by:@me review:required%s%s", repoClause, draftClause) + return q1, q2 +} + +// buildApprovedUnmergedQuery returns the GitHub search query string for +// GetApprovedUnmerged. +func buildApprovedUnmergedQuery(repoFilter string, ignoreDrafts bool) string { + repoClause := "" + if repoFilter != "" { + repoClause = " repo:" + repoFilter + } + draftClause := "" + if ignoreDrafts { + draftClause = " draft:false" + } + return fmt.Sprintf("is:pr is:open author:@me review:approved%s%s", repoClause, draftClause) +} + // GetReviewRequests fetches PRs where the user is a requested reviewer, -// including re-reviews. Uses GraphQL via `gh api graphql`. -func GetReviewRequests(ctx context.Context, repoFilter string) ([]ReviewRequest, error) { +// including re-reviews. Uses GraphQL via `gh api graphql`. When ignoreDrafts +// is true, draft PRs are filtered out at the GitHub search layer. +func GetReviewRequests(ctx context.Context, repoFilter string, ignoreDrafts bool) ([]ReviewRequest, error) { ctx, cancel := withTimeout(ctx) defer cancel() query := `query($q1: String!, $q2: String!) { @@ -104,13 +135,7 @@ func GetReviewRequests(ctx context.Context, repoFilter string) ([]ReviewRequest, } }` - repoClause := "" - if repoFilter != "" { - repoClause = " repo:" + repoFilter - } - - q1 := fmt.Sprintf("is:pr is:open review-requested:@me%s", repoClause) - q2 := fmt.Sprintf("is:pr is:open reviewed-by:@me review:required%s", repoClause) + q1, q2 := buildReviewRequestQueries(repoFilter, ignoreDrafts) cmd := exec.CommandContext(ctx, "gh", "api", "graphql", "-f", "query="+query, @@ -157,7 +182,8 @@ func GetReviewRequests(ctx context.Context, repoFilter string) ([]ReviewRequest, } // GetApprovedUnmerged fetches the user's own PRs that are approved but not yet merged. -func GetApprovedUnmerged(ctx context.Context, repoFilter string) ([]ApprovedPR, error) { +// When ignoreDrafts is true, draft PRs are excluded at the GitHub search layer. +func GetApprovedUnmerged(ctx context.Context, repoFilter string, ignoreDrafts bool) ([]ApprovedPR, error) { ctx, cancel := withTimeout(ctx) defer cancel() query := `query($q: String!) { @@ -176,12 +202,7 @@ func GetApprovedUnmerged(ctx context.Context, repoFilter string) ([]ApprovedPR, } }` - repoClause := "" - if repoFilter != "" { - repoClause = " repo:" + repoFilter - } - - q := fmt.Sprintf("is:pr is:open author:@me review:approved%s", repoClause) + q := buildApprovedUnmergedQuery(repoFilter, ignoreDrafts) cmd := exec.CommandContext(ctx, "gh", "api", "graphql", "-f", "query="+query, @@ -215,16 +236,22 @@ func GetApprovedUnmerged(ctx context.Context, repoFilter string) ([]ApprovedPR, return filtered, nil } -// ListOpenPRs lists open PRs for a repository using `gh pr list`. -func ListOpenPRs(ctx context.Context, fullRepo string, limit int) ([]ReviewRequest, error) { +// ListOpenPRs lists open PRs for a repository using `gh pr list`. When +// ignoreDrafts is true, drafts are excluded via `--draft=false`. +func ListOpenPRs(ctx context.Context, fullRepo string, limit int, ignoreDrafts bool) ([]ReviewRequest, error) { ctx, cancel := withTimeout(ctx) defer cancel() - cmd := exec.CommandContext(ctx, "gh", "pr", "list", + args := []string{ + "pr", "list", "-R", fullRepo, "--state", "open", "--limit", fmt.Sprintf("%d", limit), "--json", "number,title,author,createdAt,url", - ) + } + if ignoreDrafts { + args = append(args, "--draft=false") + } + cmd := exec.CommandContext(ctx, "gh", args...) out, err := cmd.Output() if err != nil { if ctx.Err() == context.DeadlineExceeded { diff --git a/internal/github/queries_test.go b/internal/github/queries_test.go index 913f698..d38e291 100644 --- a/internal/github/queries_test.go +++ b/internal/github/queries_test.go @@ -55,7 +55,7 @@ func TestGetReviewRequests_timeoutError(t *testing.T) { ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second)) defer cancel() - _, err := GetReviewRequests(ctx, "") + _, err := GetReviewRequests(ctx, "", false) if err == nil { t.Fatal("expected error from expired context") } @@ -68,7 +68,7 @@ func TestGetApprovedUnmerged_timeoutError(t *testing.T) { ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second)) defer cancel() - _, err := GetApprovedUnmerged(ctx, "") + _, err := GetApprovedUnmerged(ctx, "", false) if err == nil { t.Fatal("expected error from expired context") } @@ -81,7 +81,7 @@ func TestListOpenPRs_timeoutError(t *testing.T) { ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second)) defer cancel() - _, err := ListOpenPRs(ctx, "owner/repo", 10) + _, err := ListOpenPRs(ctx, "owner/repo", 10, false) if err == nil { t.Fatal("expected error from expired context") } @@ -89,3 +89,76 @@ func TestListOpenPRs_timeoutError(t *testing.T) { t.Fatalf("expected timeout error message, got: %s", err) } } + +func TestBuildReviewRequestQueries(t *testing.T) { + tests := []struct { + name string + repoFilter string + ignoreDrafts bool + wantQ1 string + wantQ2 string + }{ + { + name: "no repo, drafts allowed", + wantQ1: "is:pr is:open review-requested:@me", + wantQ2: "is:pr is:open reviewed-by:@me review:required", + }, + { + name: "no repo, drafts excluded", + ignoreDrafts: true, + wantQ1: "is:pr is:open review-requested:@me draft:false", + wantQ2: "is:pr is:open reviewed-by:@me review:required draft:false", + }, + { + name: "repo + drafts excluded", + repoFilter: "owner/repo", + ignoreDrafts: true, + wantQ1: "is:pr is:open review-requested:@me repo:owner/repo draft:false", + wantQ2: "is:pr is:open reviewed-by:@me review:required repo:owner/repo draft:false", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotQ1, gotQ2 := buildReviewRequestQueries(tt.repoFilter, tt.ignoreDrafts) + if gotQ1 != tt.wantQ1 { + t.Errorf("q1 = %q, want %q", gotQ1, tt.wantQ1) + } + if gotQ2 != tt.wantQ2 { + t.Errorf("q2 = %q, want %q", gotQ2, tt.wantQ2) + } + }) + } +} + +func TestBuildApprovedUnmergedQuery(t *testing.T) { + tests := []struct { + name string + repoFilter string + ignoreDrafts bool + want string + }{ + { + name: "no repo, drafts allowed", + want: "is:pr is:open author:@me review:approved", + }, + { + name: "drafts excluded", + ignoreDrafts: true, + want: "is:pr is:open author:@me review:approved draft:false", + }, + { + name: "repo + drafts excluded", + repoFilter: "owner/repo", + ignoreDrafts: true, + want: "is:pr is:open author:@me review:approved repo:owner/repo draft:false", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := buildApprovedUnmergedQuery(tt.repoFilter, tt.ignoreDrafts) + if got != tt.want { + t.Errorf("got %q, want %q", got, tt.want) + } + }) + } +} diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index 7c1b620..ebe8e66 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -36,7 +36,7 @@ func (s *Server) handleInbox(ctx context.Context, req mcpgo.CallToolRequest) (*m repoFilter = s.cfg.RepoFullName(repoShort) } - reviews, err := ghpkg.GetReviewRequests(ctx, repoFilter) + reviews, err := ghpkg.GetReviewRequests(ctx, repoFilter, s.cfg.IgnoreDrafts) if err != nil { return mcpgo.NewToolResultError("failed to fetch review requests: " + err.Error()), nil } From 8c18b2678c1c882b8e55656f78c5c71800c5d6e7 Mon Sep 17 00:00:00 2001 From: jmeridth Date: Mon, 11 May 2026 10:29:07 -0400 Subject: [PATCH 2/2] docs: note zen_inbox MCP tool has no per-call ignore_drafts override ## What Add a callout to docs/mcp.md clarifying that the zen_inbox MCP tool reads ignore_drafts from config only, with no per-call parameter override. ## Why The CLI accepts --ignore-drafts as a per-run flag while the MCP handler reads s.cfg.IgnoreDrafts directly. A reader coming from the CLI docs could reasonably expect the same toggle on the tool call and be surprised when it isn't there. ## Notes - Documentation only, no behavior change. - If a per-call override is added later, this note should be removed rather than amended. Signed-off-by: jmeridth --- docs/mcp.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/mcp.md b/docs/mcp.md index 258cff3..0235a32 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -27,3 +27,5 @@ claude mcp add --scope user zen -- zen mcp serve | `zen_config_repos` | Configured repositories | | `zen_review` | Create a worktree for a PR (auto-detects repo, injects context) | | `zen_review_resume` | Get worktree path and sessions for an existing PR review | + +> **Note:** `zen_inbox` uses the `ignore_drafts` setting from your config. Unlike the `zen inbox` CLI, there is no per-call override. Change the config (see [docs/configuration.md](configuration.md)) to toggle draft filtering for MCP callers.