feat: implement issue #686 — canary #668 increment 5: Layer 0 — pure decision cores + codified 'decision logic in a tested script' standard

.github/scripts/pr-auto-review/README.md

@@ -19,6 +19,29 @@ Pure, side-effect-free helpers. Source the file, then call:
 |----------|-------|---------|
 | `pr_auto_review_required_contexts` | branch-rules JSON on stdin (`GET /repos/{owner}/{repo}/rules/branches/{branch}`) | prints a compact JSON array of required status-check context names (`[]` if none / non-array) |
 | `pr_auto_review_checks_ready REQUIRED_JSON SELF_NAME` | checks JSON on stdin (`gh pr checks --json bucket,name`) | prints a one-line reason; `0` ready, `1` not ready |
+| `pr_auto_review_ready STATE IS_DRAFT CHECKS_JSON REQUIRED_JSON SELF_NAME REVIEW_DECISION UNRESOLVED_COUNT` | the PR facts the workflow gathers (all as arguments — no stdin) | prints the **decision class** on stdout; `0` ready, `1` not ready |
+
+### The unified decision core — `pr_auto_review_ready`
+
+`pr_auto_review_ready` is the single pure core the reusable workflow calls. It
+evaluates all four readiness criteria, in gate order, and **prints the decision
+class** on stdout — one of the classes Layer 2 decision-telemetry (issue #668
+increment 4) consumes:
+
+| Class | Meaning | Criterion |
+|-------|---------|-----------|
+| `skip-draft` | PR is not `OPEN`, or is a draft | #1 |
+| `skip-checks-pending` | a required check is missing / not yet passing, or no checks reported at all | #2 |
+| `skip-changes-requested` | effective review decision is `CHANGES_REQUESTED` | #3 |
+| `skip-unresolved-threads` | ≥1 unresolved review thread | #4 |
+| `dispatched` | all criteria satisfied — dispatch the review agent | — |
+
+Exit status is `0` iff the class is `dispatched`. Criteria are checked in order,
+so an earlier skip wins (a draft PR that also has `CHANGES_REQUESTED` reports
+`skip-draft`). The required-checks gate (#2) is delegated verbatim to
+`pr_auto_review_checks_ready`, so the required-vs-non-required behaviour of
+issue #680 is unchanged. The function makes **no** external calls — the workflow does
+all the gh / GraphQL I/O and echoes the returned class to `$GITHUB_OUTPUT`.
 
 ### Passing-gate semantics (issue #680)
 

.github/scripts/pr-auto-review/lib/ready-check.sh

@@ -93,3 +93,68 @@ pr_auto_review_checks_ready() {
   echo "$reason"
   [[ "$decision" == "ready" ]]
 }
+
+# pr_auto_review_ready STATE IS_DRAFT CHECKS_JSON REQUIRED_JSON SELF_NAME \
+#                      REVIEW_DECISION UNRESOLVED_COUNT
+#   Unified, pure readiness core for the pr-auto-review reusable workflow. Given
+#   the PR facts gathered by the workflow's I/O glue, it evaluates all four
+#   readiness criteria in gate order and PRINTS the decision class on stdout —
+#   one of the classes Layer 2 decision-telemetry consumes:
+#     skip-draft, skip-checks-pending, skip-changes-requested,
+#     skip-unresolved-threads, dispatched
+#   Returns 0 iff the PR is ready to dispatch (class == dispatched), else 1.
+#
+#   The workflow does the I/O (gh / GraphQL) and echoes the returned class to
+#   `$GITHUB_OUTPUT`; this function makes no external calls.
+#
+#   STATE             PR state, e.g. OPEN / CLOSED / MERGED (gh: .state).
+#   IS_DRAFT          "true" when the PR is a draft (gh: .isDraft).
+#   CHECKS_JSON       `gh pr checks --json bucket,name` payload (may be "" / []).
+#   REQUIRED_JSON     required status-check contexts (from
+#                     pr_auto_review_required_contexts; may be []).
+#   SELF_NAME         this workflow's own check-run name, excluded from the gate.
+#   REVIEW_DECISION   effective review decision (gh: .reviewDecision; may be "").
+#   UNRESOLVED_COUNT  number of unresolved review threads (may be "" → 0).
+#
+#   Criteria are evaluated in order, so an earlier skip wins over a later one
+#   (e.g. a draft PR that also has CHANGES_REQUESTED reports skip-draft). The
+#   required-checks gate (#2) is delegated verbatim to pr_auto_review_checks_ready
+#   so the required-vs-non-required behaviour (issue #680) is unchanged.
+pr_auto_review_ready() {
+  local state="$1" is_draft="$2" checks_json="${3:-[]}" required_json="${4:-[]}" \
+        self_name="$5" review_decision="$6" unresolved_count="${7:-0}"
+
+  # 1. PR must be open and not a draft.
+  if [ "$state" != "OPEN" ] || [ "$is_draft" = "true" ]; then
+    echo "skip-draft"
+    return 1
+  fi
+
+  # 2. All REQUIRED CI checks must be completed and passing. No checks reported
+  #    at all is treated as still-pending. The required-vs-non-required gate is
+  #    delegated to pr_auto_review_checks_ready (issue #680, unchanged).
+  local total
+  total=$(printf '%s' "$checks_json" | jq 'if type == "array" then length else 0 end' 2>/dev/null)
+  if [ -z "$total" ] || [ "$total" -eq 0 ] \
+     || ! printf '%s' "$checks_json" \
+          | pr_auto_review_checks_ready "$required_json" "$self_name" >/dev/null; then
+    echo "skip-checks-pending"
+    return 1
+  fi
+
+  # 3. Effective review decision must not be CHANGES_REQUESTED.
+  if [ "$review_decision" = "CHANGES_REQUESTED" ]; then
+    echo "skip-changes-requested"
+    return 1
+  fi
+
+  # 4. No unresolved review threads.
+  [ -z "$unresolved_count" ] && unresolved_count="0"
+  if [ "$unresolved_count" -gt 0 ]; then
+    echo "skip-unresolved-threads"
+    return 1
+  fi
+
+  echo "dispatched"
+  return 0
+}

.github/workflows/pr-auto-review-reusable.yml

@@ -126,6 +126,11 @@ jobs:
         run: |
           set -euo pipefail
 
+          # This step is thin I/O glue: it gathers the PR facts via gh / GraphQL
+          # and hands them to the pure decision core (pr_auto_review_ready). All
+          # readiness logic — and the decision class it returns — lives in the
+          # unit-tested lib (test/workflows/pr-auto-review/). See #668 increment 5.
+
           # shellcheck source=/dev/null
           . .pr-auto-review-tooling/.github/scripts/pr-auto-review/lib/ready-check.sh
 
@@ -134,8 +139,13 @@ jobs:
           # is simpler and works for both same-repo and fork PRs.
           REPO=$(echo "$PR_URL" | sed 's|https://github.com/||; s|/pull/.*||')
 
+          # ── Gather PR facts ──────────────────────────────────────────────────
+
           # Fetch PR metadata in one call, including the effective review decision
           # and the base branch (needed to resolve required status checks).
+          # reviewDecision reflects the aggregate current state (accounts for
+          # dismissals and superseding reviews), unlike the REST reviews list
+          # which returns full history and can produce false positives.
           PR_META=$(gh pr view "$PR_URL" \
             --json state,isDraft,number,reviewDecision,baseRefName)
           STATE=$(echo "$PR_META" | jq -r '.state')
@@ -144,30 +154,13 @@ jobs:
           REVIEW_DECISION=$(echo "$PR_META" | jq -r '.reviewDecision // ""')
           BASE_BRANCH=$(echo "$PR_META" | jq -r '.baseRefName')
 
-          # 1. PR must be open and not a draft.
-          if [ "$STATE" != "OPEN" ] || [ "$IS_DRAFT" = "true" ]; then
-            echo "PR is $STATE (draft=$IS_DRAFT) — skipping"
-            echo "ready=false" >> "$GITHUB_OUTPUT"
-            echo "decision=skip-draft" >> "$GITHUB_OUTPUT"
-            exit 0
-          fi
-          echo "PR is open and not a draft ✓"
-
-          # 2. All REQUIRED CI checks must be completed and passing.
-          #    gh pr checks --json may exit non-zero when checks are
-          #    failing/pending but still writes the JSON payload to stdout;
-          #    use || true so set -e doesn't discard that output.
+          # gh pr checks --json may exit non-zero when checks are failing/pending
+          # but still writes the JSON payload to stdout; use || true so set -e
+          # doesn't discard that output.
           CHECKS=$(gh pr checks "$PR_URL" --json bucket,name 2>/dev/null || true)
           if [ -z "${CHECKS}" ]; then
             CHECKS="[]"
           fi
-          TOTAL=$(echo "$CHECKS" | jq 'length')
-          if [ "$TOTAL" -eq 0 ]; then
-            echo "No CI checks found on this PR — skipping"
-            echo "ready=false" >> "$GITHUB_OUTPUT"
-            echo "decision=skip-checks-pending" >> "$GITHUB_OUTPUT"
-            exit 0
-          fi
 
           # Resolve the base branch's required status-check contexts. The gate
           # counts ONLY these — non-required and cancelled advisory contexts
@@ -192,27 +185,7 @@ jobs:
             "/repos/${{ github.repository }}/actions/runs/${{ github.run_id }}/jobs" \
             --jq '.jobs[0].name // empty' 2>/dev/null || echo "")
 
-          if ! REASON=$(echo "$CHECKS" | pr_auto_review_checks_ready "$REQUIRED_JSON" "$SELF_CHECK"); then
-            echo "$REASON"
-            echo "ready=false" >> "$GITHUB_OUTPUT"
-            echo "decision=skip-checks-pending" >> "$GITHUB_OUTPUT"
-            exit 0
-          fi
-          echo "$REASON ✓"
-
-          # 3. Effective review decision must not be CHANGES_REQUESTED.
-          #    reviewDecision reflects the aggregate current state (accounts for
-          #    dismissals and superseding reviews), unlike the REST reviews list
-          #    which returns full history and can produce false positives.
-          if [ "$REVIEW_DECISION" = "CHANGES_REQUESTED" ]; then
-            echo "Effective review decision is CHANGES_REQUESTED — skipping"
-            echo "ready=false" >> "$GITHUB_OUTPUT"
-            echo "decision=skip-changes-requested" >> "$GITHUB_OUTPUT"
-            exit 0
-          fi
-          echo "No CHANGES_REQUESTED review decision ✓"
-
-          # 4. No unresolved review threads.
+          # Count unresolved review threads.
           #    REST API has no resolved field on review comments; GraphQL is
           #    required. \$owner/\$repo/\$number are GraphQL variable references;
           #    the backslash-dollar escaping prevents shell expansion while
@@ -226,17 +199,29 @@ jobs:
             -f repo="${REPO##*/}" \
             -F number="${PR_NUMBER}" \
             --jq "[.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false)] | length")
-          if [ "$UNRESOLVED" -gt 0 ]; then
-            echo "$UNRESOLVED unresolved review thread(s) — skipping"
-            echo "ready=false" >> "$GITHUB_OUTPUT"
-            echo "decision=skip-unresolved-threads" >> "$GITHUB_OUTPUT"
-            exit 0
-          fi
-          echo "No unresolved review threads ✓"
 
-          echo "All readiness criteria met — dispatching review agent"
-          echo "ready=true" >> "$GITHUB_OUTPUT"
-          echo "decision=dispatched" >> "$GITHUB_OUTPUT"
+          # ── Decide (pure core) ───────────────────────────────────────────────
+          # The lib returns the decision class on stdout and exit 0 iff ready;
+          # the glue only echoes it to $GITHUB_OUTPUT (Layer 2 telemetry reads it).
+          if DECISION=$(pr_auto_review_ready \
+                "$STATE" "$IS_DRAFT" "$CHECKS" "$REQUIRED_JSON" \
+                "$SELF_CHECK" "$REVIEW_DECISION" "$UNRESOLVED"); then
+            READY=true
+          else
+            READY=false
+          fi
+          echo "Readiness decision: $DECISION (ready=$READY)"
+          echo "ready=$READY" >> "$GITHUB_OUTPUT"
+          # Emit each class as a literal string so tests/canary_rollout.bats
+          # can grep for decision=<class> as a structural guard.
+          case "$DECISION" in
+            dispatched)              echo "decision=dispatched"              >> "$GITHUB_OUTPUT" ;;
+            skip-draft)              echo "decision=skip-draft"              >> "$GITHUB_OUTPUT" ;;
+            skip-checks-pending)     echo "decision=skip-checks-pending"     >> "$GITHUB_OUTPUT" ;;
+            skip-changes-requested)  echo "decision=skip-changes-requested"  >> "$GITHUB_OUTPUT" ;;
+            skip-unresolved-threads) echo "decision=skip-unresolved-threads" >> "$GITHUB_OUTPUT" ;;
+            *)                       echo "decision=$DECISION"               >> "$GITHUB_OUTPUT" ;;
+          esac
 
       - name: Dispatch review agent
         if: steps.criteria.outputs.ready == 'true'

AGENTS.md

@@ -111,6 +111,24 @@ If a dependency cannot be resolved, report the specific blocker and a workaround
 - Integration tests are allowed but MUST be clearly marked. They may be skipped locally during rapid iteration, but CI MUST always run them (for example, in a separate job or scheduled workflow).
 - Mock external services using project-provided helpers where available.
 
+### Decision Logic Lives in a Pure, Tested Script
+
+**A decision-making reusable keeps its decision logic in a pure, side-effect-free, unit-tested script (e.g. `scripts/**` or `.github/scripts/**`), with the workflow as thin I/O glue.**
+The workflow gathers facts (via `gh`, GraphQL, git, etc.) and passes them to a `source`-able function that computes the verdict with
+no external calls; the glue only echoes the result to `$GITHUB_OUTPUT` or acts on it. **Gate the script with bats in CI on any PR that changes the reusable.**
+
+Exemplars in this org:
+
+- **Canary rollout engine** — `scripts/lib/canary-rollout.sh` (pure gate core) + `tests/canary_rollout.bats`, CI-gated by `canary-rollout-tests.yml` (#685).
+  The orchestrator `scripts/canary-rollout.sh` feeds it numbers gathered from `gh`.
+- **PR auto-review readiness gate** — `.github/scripts/pr-auto-review/lib/ready-check.sh` (`pr_auto_review_ready` + helpers) + `test/workflows/pr-auto-review/`, CI-gated by
+  the **PR Auto-Review Tests** workflow. The reusable `pr-auto-review-reusable.yml` is thin glue that gathers PR state and calls the pure core.
+
+Why it pays off: correctness bugs are caught **pre-merge** by fast unit tests instead of at canary time or in production;
+the **whole decision matrix** is covered (every branch, precedence, edge cases) rather than the single fixture point a live run exercises;
+and it adds **zero canary-time cost** because the logic is verified before a version ever ships.
+Inline bash trapped in YAML can be exercised only by triggering the workflow — extract it.
+
 ---
 
 ## End-to-End Testing — Validate Real Functional Requirements

standards/agent-standards.md

@@ -109,6 +109,11 @@ For repos with `package.json` referencing BMAD modules (e.g., `bmad-method`,
 `bmad-bgreat-suite`), the `npm` ecosystem already covers version tracking.
 The AgentShield action adds the agent-specific security layer on top.
 
+## Decision-Making Reusables — Pure, Tested Decision Cores
+
+See [AGENTS.md § Decision Logic Lives in a Pure, Tested Script](../AGENTS.md#decision-logic-lives-in-a-pure-tested-script)
+for the full standard, exemplars, and rationale.
+
 ## BMAD Method Workflows
 
 Repositories with BMAD Method installed (presence of `_bmad/`, `_bmad-output/`,