fix(shipping-pr): wait for missing required checks

plugins/skill-set/commands/pr/ship.md

@@ -9,10 +9,10 @@ Supported flags and defaults:
 
 - `--max-cycles N` — resolver attempts, default 3
 - `--ci-timeout MIN` — current-HEAD CI deadline, default 30
-- `--review-timeout MIN` — current-HEAD automated-review deadline, default 10
+- `--review-timeout MIN` — deprecated compatibility input with no completion-gate effect
 - `--no-create` — fail when the current branch has no PR
-- `--required-only=BOOL` — select required checks only, default true
+- `--required-only=BOOL` — enforce effective required checks only; false additionally selects observed optional checks, default true
 
-Reviewer detection is always automatic for CodeRabbit, Claude, and `chatgpt-codex-connector`; no adapter flag is accepted. Invoking this command authorizes one automatic initial commit containing the complete inspected working-tree scope, publication of existing and resolver commits, PR creation when needed, and gated resolution feedback. Do not ask again before a normal commit or push; stop only for ambiguous fix decisions, stale/diverged state, or a failed publication gate.
+Reviewer detection is automatic and reporting-only for CodeRabbit, Claude, and `chatgpt-codex-connector`; no adapter flag is accepted. Only effective required check contexts gate review completion. Invoking this command authorizes one automatic initial commit containing the complete inspected working-tree scope, publication of existing and resolver commits, PR creation when needed, and gated resolution feedback. Do not ask again before a normal commit or push; stop only for ambiguous fix decisions, stale/diverged state, or a failed publication gate.
 
 The workflow keeps state in the repository's Git common directory and rejects concurrent active runs. It binds publication to the live PR head repository/ref and the selected remote's canonical push URL, including fork remotes. Resume existing state instead of starting a second loop. Never include post-inspection changes, publish partial resolver work, force-push, merge, pull, or rebase.

plugins/skill-set/evals/shipping-pr/state-machine-happy-path/fixtures/mock-gh

@@ -31,6 +31,16 @@ if [[ ${1:-} == pr && ${2:-} == checks ]]; then
   exit 0
 fi
 
+if [[ ${1:-} == api && ${2:-} == repos/*/rules/branches/* ]]; then
+  printf '%s\n' '[{"type":"required_status_checks","parameters":{"required_status_checks":[{"context":"verify","integration_id":null}]}}]'
+  exit 0
+fi
+
+if [[ ${1:-} == api && ${2:-} == repos/*/branches/*/protection/required_status_checks* ]]; then
+  printf '%s\n' 'gh: Branch not protected (HTTP 404)' >&2
+  exit 1
+fi
+
 if [[ ${1:-} == api && ${2:-} == graphql ]]; then
   graphql_id=
   graphql_body=

plugins/skill-set/skills/shipping-pr/SKILL.md

@@ -29,17 +29,17 @@ Do not use for:
 |---|---:|---|
 | `--max-cycles` | 3 | Maximum resolver attempts |
 | `--ci-timeout` | 30 minutes | Current-HEAD check deadline |
-| `--review-timeout` | 10 minutes | Persisted compatibility deadline; reviewer telemetry never delays a settled PR |
+| `--review-timeout` | 10 minutes | Deprecated compatibility input; it does not gate completion |
 | `--no-create` | off | Refuse to create a missing PR |
-| `--required-only` | true | Select required checks only |
+| `--required-only` | true | Enforce effective required checks only; `false` additionally selects observed optional checks |
 
 ## Common Scenarios
 
 - “Ship this PR and keep fixing blockers” runs the full resumable loop with the default limits.
 - “Ship this branch even though it is behind main” commits and publishes the current branch state, then lets the normal blocker cycle resolve any base conflict.
 - “Keep resolving without extra checkouts” reconciles and fixes the PR in the currently checked-out worktree and branch.
 - “Resume PR 42” loads the active run and branches on its persisted status/publication phase without starting a duplicate resolver.
-- “Is this PR truly clean?” snapshots the same HEAD across checks, paginated threads, mergeability, and auto-detected reviewer telemetry. Pending reviewer telemetry is reported without delaying a settled PR.
+- “Is this PR truly clean?” snapshots the same HEAD across effective required checks, paginated threads, and mergeability. Reviewer telemetry is reported without affecting the verdict.
 
 ## Workflow
 
@@ -74,11 +74,10 @@ Convert minute flags to seconds, then run:
   --pr "$PR" --repo "$REPO" \
   --max-cycles "$MAX_CYCLES" \
   --ci-timeout-seconds "$CI_TIMEOUT_SECONDS" \
-  --review-timeout-seconds "$REVIEW_TIMEOUT_SECONDS" \
   --required-only "$REQUIRED_ONLY"
 ```
 
-Reviewer discovery is always automatic. The runner detects CodeRabbit, Claude, and `chatgpt-codex-connector` from recent merged-PR activity, then incorporates current-PR evidence on every snapshot for reporting and blocker fingerprints. Do not ask the user to select an adapter or pass reviewer-specific flags. Reviewer telemetry is not a separate completion gate: once selected checks are settled and no actionable review thread remains, an absent, pending, or failed reviewer signal must not keep the run polling or make it time out. A reviewer check still participates through the normal selected-check gate, and reviewer feedback still participates through review threads.
+Reviewer discovery is always automatic and reporting-only. The runner detects CodeRabbit, Claude, and `chatgpt-codex-connector` from recent merged-PR activity, but their absent, pending, or failed telemetry never affects status, deadlines, or blocker fingerprints. Do not ask the user to select an adapter or pass reviewer-specific flags. A review-related status or check gates completion only when it is an effective required context; `--required-only false` intentionally broadens the selected set to every observed check. Existing actionable review threads still participate through the review-thread gate.
 
 Use `--resume` only when the runner reports an active run. State lives under the repository's Git common directory, so linked worktrees share one lock and one run. Branch on the returned status: snapshot only `polling`; handle `blocked`, `awaiting_user`, and `resolving` before polling again. A resumed `resolving` run must use its recorded `resolution` metadata, including `decision_requirements` and `decisions`, and must never dispatch a duplicate resolver. Resume a journaled `prepared`, `gate_passed`, or `commenting` publication by calling `publish` with the unchanged files; the runner reconciles the remote HEAD and hidden comment marker. If its phase is `pending`, report the recorded worktree/branch and treat the interrupted attempt as `partial-failure`. A resumed `awaiting_user` run remains paused with its saved result and recovery paths until the user explicitly decides every recorded ID.
 
@@ -150,13 +149,15 @@ Every transition supplies `--from`, `--to`, and `--expected-run-id`. Treat a com
 Declare clean only when one snapshot confirms all of the following for the same HEAD at query start and finish:
 
 - no merge conflict;
-- every selected check is `pass` or `skipping`;
+- every effective required check context is present and `pass` or `skipping`;
+- every additionally selected optional check is `pass` or `skipping`;
 - no fail, cancel, pending, or timeout result;
+- GitHub mergeability is known;
 - no unresolved actionable review thread;
 
-Auto-detected reviewer signals are telemetry only. Their absence, pending state, or standalone failure does not override this verdict; checks and actionable review threads remain authoritative.
+An effective required context that has not appeared on the current HEAD is `pending`, not absent from the verdict. Auto-detected reviewer signals are telemetry only and never override this rule; a reviewer affects the verdict only through a required check context or an existing actionable thread. `mergeStateStatus=BLOCKED` is reported but does not independently gate completion because it can include approval requirements outside the required check set.
 
-If HEAD changes, discard the old results. The runner resets the check deadline, review deadline, and check-registration grace before snapshotting the new HEAD.
+If HEAD changes, discard the old results. The runner resets the check deadline and check-registration grace before snapshotting the new HEAD.
 
 ## Output
 

plugins/skill-set/skills/shipping-pr/reference/polling.md

@@ -15,17 +15,17 @@ If the stored HEAD changed before the snapshot but the two current reads agree,
 
 ## Checks
 
-The runner calls `gh pr checks --json bucket,name,state,link,workflow` and optionally `--required`. Buckets are classified as:
+The runner queries all active rules that apply to the base branch, including inherited organization rulesets, and unions their required status-check contexts with legacy branch-protection contexts. It then calls `gh pr checks --json bucket,name,state,link,workflow` and optionally `--required`. A configured required context that is missing from the current HEAD is synthesized as `pending`. Buckets are classified as:
 
 | Bucket | Meaning |
 |---|---|
 | `pass`, `skipping` | Satisfied |
 | `fail`, `cancel` | Blocked |
 | `pending` or unknown | Polling until the CI deadline |
 
-Pending at the deadline becomes `timed_out`. For a new HEAD, zero selected checks remain `polling` for a 60-second registration grace so a fresh push cannot appear clean before workflows register. After that grace, a repository with genuinely no selected checks may satisfy the check condition. If checks were observed and later disappear, polling continues until the CI deadline.
+Pending or missing required contexts at the deadline become `timed_out`. For a new HEAD with no configured or observed selected checks, zero selected checks remain `polling` for a 60-second registration grace so a fresh push cannot appear clean before workflows register. After that grace, a repository with genuinely no selected checks may satisfy the check condition. If checks were observed and later disappear, polling continues until the CI deadline.
 
-Signal-gated review workflows, including `karrot-emu/signal-gated-review-action`, require no reviewer adapter. Treat the workflow job (for example, `Signal-Gated PR Review / review`) as a normal selected check and its inline findings as review threads. With the default `--required-only true`, configure that job as a required check on every target branch that must wait for it; otherwise use `--required-only false` intentionally to observe all checks.
+Signal-gated review workflows require no reviewer adapter. A review workflow gates completion only through the exact status or check context configured as required on the base branch. Do not wait for a workflow job, reviewer identity, review object, reaction, or comment merely because it exists. With `--required-only false`, all currently observed checks are intentionally selected in addition to the required set.
 
 ## Review Threads
 
@@ -37,16 +37,16 @@ Any unresolved actionable thread makes the snapshot `blocked`.
 
 ## Automated Reviewers
 
-Reviewer discovery is always `auto`; there is no adapter-selection flag. Initialization detects CodeRabbit, Claude, and `chatgpt-codex-connector` from authors and apps found in the ten most recent merged PRs. Every snapshot unions that history with current-PR commit statuses, check-runs, reviews, comments, and reactions, so the report and blocker fingerprint include reviewer telemetry without requiring every repository to run those reviewers.
+Reviewer discovery is always `auto`; there is no adapter-selection flag. Initialization detects CodeRabbit, Claude, and `chatgpt-codex-connector` from authors and apps found in the ten most recent merged PRs. Every snapshot unions that history with current-PR commit statuses, check-runs, reviews, comments, and reactions for reporting only.
 
-CodeRabbit telemetry comes from its current-HEAD commit status or check-run. Claude telemetry comes from its current-HEAD check-run, status, or review. Codex telemetry comes from a current-HEAD review or, before any resolver push, the connector's `+1` reaction. These signals are observational and are never independently required. Their absence, pending state, or standalone failure cannot keep a snapshot `polling`, make it `timed_out`, or make it `blocked` after selected checks have settled and no actionable review thread remains. A telemetry query or normalization failure is reported as `telemetry_available:false` with `unavailable` provider states and does not change the snapshot verdict.
+CodeRabbit telemetry comes from its current-HEAD commit status or check-run. Claude telemetry comes from its current-HEAD check-run, status, or review. Codex telemetry comes from a current-HEAD review or, before any resolver push, the connector's `+1` reaction. These signals are observational and are never independently required. Their absence, pending state, standalone failure, or change cannot affect polling, timeout, blocking, clean, or stalled decisions. A telemetry query or normalization failure is reported as `telemetry_available:false` with `unavailable` provider states.
 
-Reviewer results that appear as selected PR checks still use the normal check classification. Actionable comments from every provider still use review-thread state. This makes checks and unresolved conversations authoritative while preserving reviewer evidence for reports and change fingerprints.
+Reviewer results that match effective required contexts use the normal check classification. Actionable comments from every provider still use review-thread state. In the default required-only mode, optional reviewer evidence remains report-only; `--required-only false` intentionally makes every observed check part of the verdict.
 
 ## Mergeability
 
-`CONFLICTING` or `DIRTY` is blocked. An unknown mergeability result is polling, never clean.
+`CONFLICTING` or `DIRTY` is blocked. Unknown mergeability is polling and becomes `timed_out` at the current-HEAD check deadline when no actionable blocker is available. `mergeStateStatus=BLOCKED` is reported but does not gate the verdict because it can represent approval requirements outside the required status-check set.
 
 ## Fingerprint
 
-The blocker fingerprint covers the observed HEAD; normalized check identities, states, and buckets; conflict state; unresolved thread IDs and latest-comment content; and all active reviewer states. After a resolver returns to polling, only a fresh snapshot can declare `stalled`, and only when both HEAD and fingerprint remain unchanged.
+The blocker fingerprint covers the observed HEAD and base branch; effective required contexts; normalized check identities, states, and buckets; conflict or unknown-mergeability state; and unresolved thread IDs and latest-comment content. Reviewer telemetry and non-gating `mergeStateStatus` values never affect the fingerprint. After a resolver returns to polling, only a fresh snapshot can declare `stalled`, and only when both HEAD and fingerprint remain unchanged.