Merge pull request #2 from Sea-Haven-Industries/fix/ci-status-honesty

fix: never report an unread CI signal as missing CI

.github/workflows/review-pr.yml

@@ -115,6 +115,7 @@ jobs:
           # installation is later granted broader permissions.
           permission-contents: read
           permission-pull-requests: read
+          permission-checks: read
           permission-metadata: read
 
       - name: Resolve frontend PR head

README.md

@@ -61,24 +61,31 @@ missing or rejected.
 
 ## GitHub App
 
-The App must have **exactly** these permissions and nothing else, installed on
-**only** `shoc-frontend-new` and `shoc-backend`:
+The App (`shoc-review-runner`) must have **exactly** these permissions and
+nothing else, installed on **only** `shoc-frontend-new` and `shoc-backend`:
 
 - Repository permissions: Contents **Read-only**, Pull requests **Read-only**,
-  Metadata **Read-only**.
+  Checks **Read-only**, Metadata **Read-only**.
+
+Checks read is what lets the runner report the reviewed PR's own CI status.
+Without it the runner records that signal as `unknown` rather than guessing.
 
 Verify after installing (and re-verify when the App changes):
 
 ```sh
-APP_INSTALLS=$(gh api /orgs/Sea-Haven-Industries/installations --jq \
-  '.installations[] | select(.app_slug=="<app-slug>")')
-echo "$APP_INSTALLS" | jq '{permissions, repository_selection}'
-# permissions must be exactly {contents: "read", pull_requests: "read", metadata: "read"}
-gh api "/user/installations/$(echo "$APP_INSTALLS" | jq -r .id)/repositories" \
-  --jq '.repositories[].full_name'
-# must list exactly the two product repos
+gh api /orgs/Sea-Haven-Industries/installations \
+  --jq '.installations[] | select(.app_slug=="shoc-review-runner")
+        | {permissions, repository_selection}'
+# permissions must be exactly contents/pull_requests/checks/metadata = "read",
+# and repository_selection must be "selected"
 ```
 
+Listing the selected repositories requires the installation token itself
+(`GET /installation/repositories`), which only exists inside a workflow run.
+In practice the token mint is the check: `actions/create-github-app-token`
+requests `shoc-frontend-new,shoc-backend` explicitly and fails the run if the
+App is not installed on both.
+
 ## Security architecture
 
 Reviewing a PR means building it, and building it means executing code the PR

scripts/generate-evidence.sh

@@ -76,7 +76,7 @@ cat >"$EVIDENCE_FILE" <<EOF
 - Backend PR CI status: $(pr_field backend '.ci_status')
 - Frontend PR mergeable: $(pr_field frontend '.mergeable // "unknown"')
 - Backend PR mergeable: $(pr_field backend '.mergeable // "unknown"')
-- CI statuses come from the PR head's check-runs; red/pending/missing is a governance signal, not silently omitted.
+- CI statuses come from the PR head's check-runs. "missing" means the API returned zero checks; "unknown" means the runner could not read them and is NOT evidence that CI is absent or failing. Do not treat "unknown" as a governance problem with the PR.
 
 ## Stack Status
 - Frontend parent: NOT_RUN (stacked-PR resolution is Phase 4)

scripts/resolve-pr-head.sh

@@ -27,14 +27,28 @@ head_repo="$(jq -r '.head.repo.full_name // empty' <<<"$pr_json")"
 
 short_sha="${head_sha:0:7}"
 
-# CI status on the exact head: green / red / pending / missing (never omitted).
-checks_json="$(gh api "repos/$repo/commits/$head_sha/check-runs" --jq '{total: .total_count, runs: [.check_runs[] | {name, status, conclusion}]}' 2>/dev/null)" \
-  || checks_json='{"total":0,"runs":[]}'
-ci_status="$(jq -r '
-  if .total == 0 then "missing"
-  elif ([.runs[] | select(.status != "completed")] | length) > 0 then "pending"
-  elif ([.runs[] | select(.conclusion != "success" and .conclusion != "neutral" and .conclusion != "skipped")] | length) > 0 then "red"
-  else "green" end' <<<"$checks_json")"
+# CI status on the exact head: green / red / pending / missing / unknown.
+#
+# A failed API call must NEVER be reported as "missing": that would state as a
+# fact ("this PR has no CI") what is actually an unread signal, which is the
+# exact failure mode this runner exists to prevent. Reading check-runs needs the
+# App's `checks: read` permission; without it the call 403s and the honest
+# answer is "unknown".
+checks_err="$ARTIFACTS_DIR/$side-checks-error.txt"
+if checks_json="$(gh api "repos/$repo/commits/$head_sha/check-runs" \
+     --jq '{total: .total_count, runs: [.check_runs[] | {name, status, conclusion}]}' 2>"$checks_err")"; then
+  ci_status="$(jq -r '
+    if .total == 0 then "missing"
+    elif ([.runs[] | select(.status != "completed")] | length) > 0 then "pending"
+    elif ([.runs[] | select(.conclusion != "success" and .conclusion != "neutral" and .conclusion != "skipped")] | length) > 0 then "red"
+    else "green" end' <<<"$checks_json")"
+  rm -f "$checks_err"
+else
+  reason="$(tr -d '\000-\037' <"$checks_err" | cut -c1-160)"
+  log "WARNING: could not read check-runs for $repo@$short_sha: $reason"
+  ci_status="unknown (check-runs unreadable; the review App likely lacks 'checks: read')"
+  checks_json='{"total":0,"runs":[],"unreadable":true}'
+fi
 
 jq -n \
   --arg side "$side" --arg repo "$repo" --argjson pr "$pr" \

skills/pr-review/SKILL.md

@@ -52,6 +52,11 @@ Sort every problem before writing the review:
 * **Environment limitation** — a gate the runner could not execute (see evidence
   "Runtime Limitations"). State it; never convert it into a pass or a code defect.
 
+A signal the runner could not read is not a finding. If a governance signal is
+recorded as `unknown`, say nothing about it or note only that it was unavailable
+to the runner — never report it as missing, failing, or a reason to withhold
+approval. Only `missing`, `red`, or `pending` are statements about the PR.
+
 ## Verdict discipline
 
 * `APPROVE` only when every required gate in the evidence is PASS or