fix(gh): shepherd works only on approved PRs, unless asked for all #465

Activity

1. brianespinosa

   added

   plugin:ghThe gh pluginThe gh plugin

   on Oct 19, 2026
2. brianespinosa

   added the

   Bug

   issue type on Oct 19, 2026
3. brianespinosa

   changed the title [-]fix(gh): shepherd skips a PR that lacks a required approval[/-] [+]fix(gh): shepherd works only on approved PRs, unless asked for all[/+] on Oct 19, 2026
4. brianespinosa commented on Oct 19, 2026

   

   brianespinosa

   on Oct 19, 2026

   ContributorAuthor

   More actions

   Sequencing (sweep)

   Unblocked set: #465 only. No blockers, nothing blocked. No open pull request touches these files. #463, #464 and #466 touch the same plugin but are in no round.

   The defect holds on main (b01a286): statusDocument (plugins/gh/src/shepherd.ts, after :480) makes a target from two tests only: not a draft, and auto-merge on. No review state is read. PULL_REQUEST_FIELDS (lib/gh.ts:956) carries reviewDecision, and no review list.

   Hidden dependencies, checked against the code:

   • Approvals need a new field or a new read. gh pr list --json offers latestReviews (one review per author, with its state). Adding it to PULL_REQUEST_FIELDS changes what shepherd-status and every shepherd-pr action read. A per-PR GraphQL read is the other way.
   • The ruleset read exists for checks only. requiredContexts (lib/gh.ts:2807) reads rules/branches/{branch}, and requiredContextsOf (:1937) keeps only required_status_checks. The pull_request rule (required_approving_review_count, require_code_owner_review) needs a parser of its own, or a wider result. The read is per base branch. baseRefName is in the field list since fix(gh): shepherd rebases a behind PR before it waits on CI #462.
   • A code owner approval cannot be proven from these reads. Proving one needs CODEOWNERS parsed and team membership read. The issue allows "at least one APPROVED review" in its place.
   • The bje enterprise "Protect main" ruleset requires 0 approvals (~/Code/@bje/CLAUDE.md, Enterprise rulesets). Under "approved only" as the default, a solo-maintained repo (Dependabot PRs, no reviewer) has no targets without --all. This is a question for the question phase: an approval always, or only when the branch rules ask for one.
   • shepherd-status parses its own argv. It refuses more than two arguments as "one label per run" (shepherd-status.ts:124). --all needs a flag parse that keeps that refusal for a second label.
   • The output contract is frozen by tests/plugins/gh/fixtures/shepherd-status-output.json and the shepherd capture rules (README-shepherd.md). A new key for the left-out pull requests is a capture edit or a divergence row.
   • lib/gh.ts changes go through node scripts/sync-lib.ts to the three copies. A new GhClient method needs harness/gh.ts support.

   File map (one stream):

   File	Region	Change
   lib/gh.ts (+ 3 synced copies)	PULL_REQUEST_FIELDS; the rules read	review list; the pull_request rule
   plugins/gh/src/shepherd.ts	statusDocument, summary, PullSummary	approval test; the left-out list; an approval field
   plugins/gh/src/subcommands/shepherd-status.ts	argv, the rules read, HELP	--all; base-branch rules
   plugins/gh/src/subcommands/shepherd-pr.ts	state	approval still current
   plugins/gh/skills/shepherd/SKILL.md	argument-hint, steps 2, 3 and 5	--all; report the left-out PRs
   plugins/gh/agents/shepherd-prs.md	step 1 or 2	skip a PR that lost its approval, without --all
   plugins/gh/README.md	shepherd args table (:152), shepherd-status reference (:237)
   docs/flows/gh/shepherd/_skill-flow.md, shepherd-status-flow.md, shepherd-pr-flow.md		approval edge
   harness/gh.ts, harness/README.md		only if a new client method
   tests/lib/gh.test.ts, tests/plugins/gh/shepherd.test.ts, tests/plugins/gh/subcommands/shepherd-status.test.ts, shepherd-pr.actions.test.ts, .test-support.ts
   tests/plugins/gh/fixtures/shepherd-status-output.json, shepherd-pr-output.json, divergences-shepherd.json, README-shepherd.md		the output change
   tests/repo/gh-docs.test.ts		pins for the skill and agent rules
   plugins/gh/.claude-plugin/plugin.json	version	0.12.0 to 0.13.0; this stream owns it

   Comments only: plugins/workstreams/src/subcommands/pr-state.ts (it also calls requiredContexts), if the rules read changes shape.

   Shape: one pull request. One issue. The skill's --all and the script's filter cannot merge apart.

   Size:

   • Build: opus. It is a new filter over two new reads, a frozen capture contract, and 100% coverage. That is about 20 files, like fix(gh): shepherd rebases a behind PR before it waits on CI #462.
   • Review: opus. Two rounds are likely, because this is new behaviour. Expect the review at about half of the round's tokens. Round 23 spent 46% on review.
   • Session effort: medium.

   Unlocks: nothing in the tracker.

   Recorded by workstreams:sweep 2026-10-19.
5. brianespinosa commented on Oct 19, 2026

   

   brianespinosa

   on Oct 19, 2026

   ContributorAuthor

   More actions

   Rulings (question phase)

   Asked

   1. The branch rules decide. Without --all, a pull request is left out only when its base branch requires an approval and the pull request does not have one. "Requires" means any of these:

      • the ruleset's pull_request rule has required_approving_review_count above 0, or require_code_owner_review true;
      • reviewDecision is REVIEW_REQUIRED or CHANGES_REQUESTED.

      A repository that requires no review is shepherded as today, with no --all. This replaces the issue's line "at least one APPROVED review" as the default.

   Ruled (each one is cheap to reverse)

   2. When an approval is met.

      • The current APPROVED reviews number at least required_approving_review_count.
      • A code owner rule takes at least one APPROVED review. A code owner approval cannot be proven without parsing CODEOWNERS and reading team membership.
      • reviewDecision APPROVED is met.
      • CHANGES_REQUESTED, from reviewDecision or from a current review, is never met.

      For: one read per base branch, no CODEOWNERS parser. Against: an approval from a person who is not a code owner counts. GitHub then still blocks the merge, and the worker skips it on BLOCKED, as today.

   3. Reviews come from latestReviews in PULL_REQUEST_FIELDS (lib/gh.ts:956), one entry per author. For: no extra call per pull request. Against: every shepherd read carries the field.

   4. The rule comes from a new GhClient method. It reads rules/branches/{branch} and returns the pull_request rule's two parameters. requiredContexts and its caller pr-state stay unchanged. shepherd-status reads it once for each distinct base branch. For: no change to a contract workstreams uses. Against: a second parser over one endpoint.

   5. A rules read that fails does not leave a pull request out. It falls back to reviewDecision alone, and stderr says why. A plan-limit answer means no ruleset, as requiredContexts already treats it (lib/gh.ts:2810). For: a failed read never empties the queue. Against: a PR that needs an approval can get through on a failed read. That is today's behaviour, no worse.

   6. The output.

      • shepherd-status gains a key, needs_approval: the pull requests left out, each with number, title, author and the reason.
      • Each pr in shepherd-pr gains approval: met, missing or not_required, with null when unread.
      • Record the change as a capture edit, as fix(gh): shepherd resolves conflicted auto-merge pull requests #444 and fix(gh): shepherd rebases a behind PR before it waits on CI #462 did.

   7. --all is the flag name, on /gh:shepherd and on shepherd-status. It works with one label and with --dry-run. Any other flag is refused, and a second label is still refused.

   8. The worker gets the flag. SKILL.md step 3 tells the agent whether --all is on. Without it, state with approval: missing is a skip ("needs an approval") before any rebase or wait.

   9. Shape: one layer, one pull request. It bumps the gh plugin from 0.12.0 to 0.13.0.

   Recorded by workstreams:question 2026-10-19.
6. brianespinosa commented on Oct 19, 2026

   

   brianespinosa

   on Oct 19, 2026

   ContributorAuthor

   More actions

   Round 24 plan, approved 2026-10-19T21:59Z.

   Issues: #465. Sweep: issuecomment-6278294598. Rulings: issuecomment-6278313158.

   Layer 1: #465

   • Branch: wktr-465-shepherd-approved, base main
   • Build model: opus. Review model: opus; code-reviewer, silent-failure-hunter, pr-test-analyzer, comment-analyzer, type-design-analyzer
   • Version lines: plugins/gh/.claude-plugin/plugin.json (0.12.0 to 0.13.0)
   • File set:
     • lib/gh.ts (latestReviews in PULL_REQUEST_FIELDS; a new rules method for the pull_request rule) and its synced copies in plugins/{gh,audit,workstreams}/src/lib/gh.ts
     • plugins/gh/src/shepherd.ts (the approval verdict, PullSummary.approval, needs_approval)
     • plugins/gh/src/subcommands/shepherd-status.ts (--all, one rules read for each base, the fallback, HELP)
     • plugins/gh/src/subcommands/shepherd-pr.ts (state reports approval, HELP)
     • plugins/gh/skills/shepherd/SKILL.md, plugins/gh/agents/shepherd-prs.md, plugins/gh/README.md
     • docs/flows/gh/shepherd/_skill-flow.md, shepherd-status-flow.md, shepherd-pr-flow.md
     • harness/gh.ts, harness/README.md, tests/harness/gh.test.ts
     • tests/lib/gh.test.ts, tests/plugins/gh/shepherd.test.ts, tests/plugins/gh/subcommands/shepherd-status.test.ts, tests/plugins/gh/subcommands/shepherd-pr.*.test.ts, shepherd-pr.test-support.ts, tests/repo/gh-docs.test.ts
     • The captures tests/plugins/gh/fixtures/shepherd-status-output.json, shepherd-pr-output.json, README-shepherd.md
     • plugins/gh/.claude-plugin/plugin.json
   • Out of scope: requesting reviews, CODEOWNERS parsing, fix(gh): shepherd-status measures ci_seconds from every workflow, not CI #463, feat(gh): shepherd adds new auto-merge PRs to its queue during a run #464, fix(gh): shepherd skips a PR whose CI is still making progress #466

   Done

   pr-state gives done for the layer.

   Review shape

   The review state machine in skills/review/SKILL.md, with the model and reviewers above.

   Estimate

   • Build: medium, about 22 files, like fix(gh): shepherd rebases a behind PR before it waits on CI #462.
   • One review round costs about the build, and two cost about twice the build.
   • Round 2 is likely, because this is new behaviour.
   • Session effort: medium.

   Recorded by workstreams:question 2026-10-19.
7. brianespinosa commented on Oct 19, 2026

   

   brianespinosa

   on Oct 19, 2026

   ContributorAuthor

   More actions

   Mid-round ruling 10 (Round 24, answered by the person):

   Field evidence: sm-incubator/beta-recognition#2816 was current, green, approved and on auto-merge, and still BLOCKED. The base branch's pull_request rule had required_review_thread_resolution: true, and the pull request had 5 unresolved review threads. reviewDecision was empty. It merged within 20 s of the threads being resolved.

   Chosen: #465's check covers every pull_request rule parameter that a green pull request can still fail. That is approvals, code owner review, review thread resolution, and approval of the last push.

   • required_review_thread_resolution true: a pull request with an unresolved review thread is left out ("has unresolved review threads"). Count the threads with the GraphQL read that viewPullRequestDetail already does, or one like it, and only when the rule asks for it.
   • require_last_push_approval true: the approval must be on the current head. latestReviews carries each review's commit. An approval on an older commit is missing.
   • The left-out key is named not_ready, not needs_approval. Each entry carries its reason or reasons.
   • Each pr in shepherd-pr carries approval and threads (resolved, unresolved, not_required, or null when unread). Without --all, the agent skips a pull request that has either blocker, with that reason.
   • A read that fails does not leave a pull request out (ruling 5).

   For: one rules read, one exclusion list, one review. Against: a larger pull request, and one GraphQL read for each pull request whose rule asks for thread resolution.

   Recorded by workstreams:start 2026-10-19.
8. brianespinosa commented on Oct 19, 2026

   

   brianespinosa

   on Oct 19, 2026

   ContributorAuthor

   More actions

   Mid-round ruling 11 and 12 (Round 24, cheap to reverse):

   11. PULL_REQUEST_FIELDS carries reviews, not latestReviews. This revises ruling 3. gh's latestReviews JSON gives commit.oid as empty on every entry (chore(deps): bump the codeql-actions group with 3 updates cli/cli#14751, #14750, gh 2.102.0). The last-push rule (ruling 10) needs each review's commit. For: the real oid, with no extra call. Against: every review, with its body, rides on each shepherd read. To reverse: a per-PR GraphQL read for only the PRs whose rule asks for approval of the last push.
   12. reviewDecision REVIEW_REQUIRED gives missing, even when the count of approvals is met. This narrows ruling 2. For: it is GitHub's own verdict, and without it, "REVIEW_REQUIRED and no rule" would count 0 of 0 as met. Against: a stale reviewDecision can hold back a PR that the ruleset would pass. To reverse: let the count win when a ruleset was read.

   Also passed to review round 1: the build return names a gap in the agent prose. The changed and ok:false paths re-read state and go back to step 2, so the step 1 approval and threads check is skipped on a re-read.

   Recorded by workstreams:start 2026-10-19.
9. brianespinosa commented on Oct 19, 2026

   

   brianespinosa

   on Oct 19, 2026

   ContributorAuthor

   More actions

   Mid-round ruling 13 (Round 24, cheap to reverse):

   Review round 1 found that rulings 2 and 10 conflict in one case: reviewDecision is APPROVED, the ruleset asks for approval of the last push, and the only approval is on an older commit.

   Chosen: ruling 2 holds, and a decision of APPROVED is met. #468 already behaves this way, with a doc comment and a pin. For: GitHub computed that verdict, and it is the mirror of ruling 12, which lets REVIEW_REQUIRED win. A ruleset-only requirement usually leaves reviewDecision empty, so the count path decides there. Against: if GitHub reports APPROVED and the ruleset still blocks on the last push, the shepherd spends CI on the pull request, and settle then reports it blocked. To reverse: let the count path decide whenever a ruleset was read with lastPush true. That is one branch in approvalOf and one test.

   Recorded by workstreams:start 2026-10-19.
10. brianespinosa commented on Oct 19, 2026

    

    brianespinosa

    on Oct 19, 2026

    ContributorAuthor

    More actions

    Round 24 end, 2026-10-19.

    1. State

    • fix(gh): shepherd works only on approved PRs, unless asked for all #468 is done at fba0281. CI is green, and no thread is open. It is 0 commits behind main, and a merge with main is clean. It is still a draft.
    • Merge sequence: fix(gh): shepherd works only on approved PRs, unless asked for all #468 alone. The person marks it ready and merges it.

    2. Decisions the model made
    The person confirmed all of these.

    • Ruling 11: the field is reviews, not latestReviews.
    • Ruling 12: REVIEW_REQUIRED gives missing.
    • Ruling 13: APPROVED is met under a last-push rule.
    • Threads are read under --all too, and only when the rule asks for it.
    • No state read follows a rebase. settle catches an approval that the rebase made stale.

    3. Defects from before the round: not opened. The person declined the issue for the unpaginated rules/branches read.

    4. Scope changes for other issues: none.

    5. Other items: the person marks #468 ready and merges it. Then /gh:sync removes the worktree and the branch.

    6. Cost

    Agent	PR / round	Model	Tokens	Tool uses	Duration
    Stream, layer 1 (ruling 10 included)	#468 build	opus	298,278	123	29.1 min
    Review round 1	#468 R1	opus	220,501	70	30.7 min
    Total			518,779	193	59.8 agent min

    • The round took about 61 minutes, from the first dispatch (about 22:02Z) to 23:03Z. No notification records idle time.
    • fix(gh): shepherd works only on approved PRs, unless asked for all #468 changes +1,901 and -159 lines.
    • The review used 42% of the tokens. Round 1 changed no logic, so there was no round 2.

    Facts for the record

    • Ruling 10 was added mid-build, from field evidence. beta-recognition#2816 was approved and green, and stayed BLOCKED on required_review_thread_resolution.
    • No real APPROVED review exists in this repository. The reviews shape was checked against COMMENTED reviews on fix(gh): shepherd rebases a behind PR before it waits on CI #462 and fix(gh): prove stacked, rebased and moved merges in sync #451.
    • The not_ready path has no live evidence yet, because this repository has no open auto-merge pull request.

    Result

    • Done: the decisions are recorded, and the round end is recorded.
    • Not done: the pagination issue, which the person declined.
    • Not done, and the person's to do: mark fix(gh): shepherd works only on approved PRs, unless asked for all #468 ready and merge it.

    Recorded by workstreams:start 2026-10-19.
11. brianespinosa

    closed this as completedin #468on Oct 20, 2026
12. brianespinosa

    added a commit that references this issue on Nov 13, 2026

    feat(shepherd): read the merge queue and report it in shepherd-status (…

    ...

    0655833