Skip to content

feat: migrate PR fetch to GraphQL to expose required checks (+ review decision, PR number) #59

Description

@davidtaing

Problem

PR enrichment uses gh pr list --json number,headRefName,isCrossRepository,statusCheckRollup
(ghFetchPRChecks in checks.go). That canned JSON shape can't express:

What's possible

A custom gh api graphql query returns all of it in one call. Confirmed against
PR #56:

ci                 isRequired=true
build (ubuntu)     isRequired=false
build (macos)      isRequired=false
changes            isRequired=false
CodeRabbit         isRequired=false

statusCheckRollup.contexts nodes expose isRequired(pullRequestNumber: N) on
both CheckRun and StatusContext; the same query also yields reviewDecision
and number.

Proposed

Replace the gh pr list --json call in ghFetchPRChecks with a gh api graphql
query returning, per open PR: number, headRefName, isCrossRepository,
reviewDecision, and the rollup contexts with isRequired. Carry the new fields
through prStatus / Worktree:

  • Check grows a Required bool.
  • prStatus (and Worktree) gain Number int and ReviewDecision string.

This is foundational — it unblocks three things at once:

  1. Required-check marker in --checks (this issue's headline).
  2. PR number display.
  3. reviewDecision for the feat: desktop notifications for PR review + CI state changes (watch mode) #55 notification tiers.

Display (the design fork — why hitl)

How to mark a required check in the --checks rows — pick one, keep it lean; the
rollup glyph stays as-is:

  • bold the required rows, or
  • a * suffix / req tag, or
  • a --required-only filter that hides optional checks.

Design notes

  • Repo resolution is the main wrinkle. gh pr list resolves the repo from
    cmd.Dir (the worktree path). gh api graphql does not substitute
    {owner}/{repo}, so we need owner/name explicitly — e.g. one cached
    gh repo view --json owner,name per repo dir, or parse the remote.
  • Keep all gating/caching/cap unchanged: filter-gated, 5-project cap, 15s cache,
    hardenedGitEnv, 4s timeout. Still one subprocess per repo.
  • isRequired takes the PR number, which the query has in scope per node.
  • Preserve current normalisation: checkStateOf, cross-repo skip,
    empty-rollup → neutral.
  • Tests mirror checks_test.go: unmarshal a sample GraphQL payload into the same
    prStatus/Check shapes; add a required-flag case.

Cut-line / slicing

Could split into: (a) an afk-able fetch migration that exposes the fields with
no display change (output stays byte-identical), then (b) the required-check
display and (c) PR number as small follow-ups.

Note

The PR number is already in today's JSON and merely discarded, so it can ship
as a tiny standalone change independent of this migration — this issue just also
surfaces it through the new path.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requesthitlNeeds a human (decision/design/review)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions