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

[don-petry]

Increment 5 of #668 (Layer 0 of the design) — pure, unit-tested decision cores + the codified standard. This is where most correctness bugs get caught before a version exists — cheaper and broader than any canary-time signal.

Context / what's already done

#680/#682 set the precedent: criterion #2 of pr-auto-review's readiness gate (all required checks passing) was extracted into .github/scripts/pr-auto-review/lib/ready-check.sh (pr_auto_review_checks_ready), with bats in test/workflows/pr-auto-review/ gated by the existing PR Auto-Review Tests workflow. Increment 5 finishes the job for pr-auto-review and writes the standard.

Scope

1. Extract the remaining readiness criteria into the pure lib

Today the check-and-dispatch job in .github/workflows/pr-auto-review-reusable.yml still has criteria #1 (PR open + not draft), #3 (effective review decision ≠ CHANGES_REQUESTED), and #4 (no unresolved review threads) as inline bash. Move their decision logic into pure functions in ready-check.sh (e.g. pr_auto_review_ready <state> <is_draft> <checks_json> <required_json> <self_check> <review_decision> <unresolved_count> returning the ready/skip verdict and the decision class), so the workflow only does I/O (gather PR state, checks, review decision, unresolved-thread count via gh/GraphQL) and calls the pure core.

• Preserve the decision=<class> outputs the job emits (skip-draft, skip-checks-pending, skip-changes-requested, skip-unresolved-threads, dispatched) — Layer 2 decision-telemetry (increment 4) consumes them. The pure function should RETURN the class; the glue echoes it to $GITHUB_OUTPUT.
• Keep pr_auto_review_checks_ready (from feat: implement issue #680 — pr-auto-review ready-check counts non-required/cancelled checks → false 'not passing', blocks auto-dispatch #682) as-is or fold it into the unified core — no behaviour change to the required-checks gate.

2. Decision-matrix bats (test/workflows/pr-auto-review/)

Cover the full matrix, one point per branch, plus combinations that exercise precedence (e.g. draft + CHANGES_REQUESTED → whichever the gate checks first):

• open+non-draft, all-required-green, no CHANGES_REQUESTED, 0 unresolved → ready=true / dispatched
• draft → skip-draft; closed → not-ready
• a required check pending/failing → skip-checks-pending (a NON-required failing/cancelled check must NOT block — the pr-auto-review ready-check counts non-required/cancelled checks → false 'not passing', blocks auto-dispatch #680 fix; pin it)
• CHANGES_REQUESTED → skip-changes-requested
• ≥1 unresolved thread → skip-unresolved-threads
• no checks at all → skip-checks-pending

3. Codify the standard

Add to AGENTS.md (and/or standards/agent-standards.md) a short standard: "A decision-making reusable keeps its decision logic in a pure, side-effect-free, unit-tested script (scripts/**), with the workflow as thin I/O glue. Gate the script with bats in CI on any PR that changes the reusable." Cite the canary engine (scripts/lib/canary-rollout.sh + tests/canary_rollout.bats, now CI-gated via #685) and pr-auto-review's ready-check.sh as the exemplars. Note the payoff: correctness bugs caught pre-merge, whole decision matrix covered vs one fixture point, zero canary-time cost.

Out of scope

• Extending the pattern to other agents' reusables (auto-rebase already follows it; dependabot-automerge/dev-lead are follow-ups, not this issue).
• Increment 6 (optional SUSPECT→PRE_EXISTING auto-downgrade) — deferred per design until SUSPECT toil appears.

Acceptance

• Criteria Addressing PR comments #1/Add coding standards & principles section to AGENTS.md #3/Add workflow, environment, and orchestration guidance #4 decision logic lives in ready-check.sh pure functions; check-and-dispatch is I/O glue that calls them.
• decision=<class> outputs unchanged (Layer 2 telemetry still works); required-vs-non-required gate (pr-auto-review ready-check counts non-required/cancelled checks → false 'not passing', blocks auto-dispatch #680) behaviour unchanged.
• Full decision-matrix bats green under the PR Auto-Review Tests workflow.
• Standard codified in AGENTS.md / agent-standards.md with the two exemplars.
• shellcheck clean.

[don-petry]

Dev-Lead Implementation Plan

Issue: #686 — canary #668 increment 5: Layer 0 — pure decision cores + codified "decision logic in a tested script" standard

Scope

Extract readiness criteria #1 (open/not-draft), #3 (review decision) and #4 (unresolved threads) out of the inline bash in pr-auto-review-reusable.yml into a single pure, unit-tested decision core in ready-check.sh; make the workflow thin I/O glue; cover the full decision matrix with bats; and codify the "decision logic lives in a pure, tested script" standard.

Implementation Checklist

• Add pr_auto_review_ready <state> <is_draft> <checks_json> <required_json> <self_check> <review_decision> <unresolved_count> to .github/scripts/pr-auto-review/lib/ready-check.sh — returns the decision class (skip-draft/skip-checks-pending/skip-changes-requested/skip-unresolved-threads/dispatched) on stdout, exit 0 iff ready. Delegates the required-checks gate to the existing pr_auto_review_checks_ready (issue pr-auto-review ready-check counts non-required/cancelled checks → false 'not passing', blocks auto-dispatch #680 behaviour unchanged).
• Rewire check-and-dispatch in .github/workflows/pr-auto-review-reusable.yml: the criteria step gathers PR state/checks/required-contexts/self-check/review-decision/unresolved-count via gh+GraphQL, then calls the pure core once and echoes ready= and decision=<class> to $GITHUB_OUTPUT. Decision-telemetry step names unchanged.
• Update .github/scripts/pr-auto-review/README.md with the new function contract.
• Add the standard to AGENTS.md and standards/agent-standards.md, citing scripts/lib/canary-rollout.sh+tests/canary_rollout.bats and ready-check.sh as exemplars.

Tests to Write

• test/workflows/pr-auto-review/ready.bats — full decision matrix for pr_auto_review_ready:
  • open+non-draft, all-required-green, no CHANGES_REQUESTED, 0 unresolved → dispatched (ready)
  • draft → skip-draft; closed → not ready (skip-draft)
  • required check pending/failing → skip-checks-pending; NON-required failing/cancelled must NOT block (pins pr-auto-review ready-check counts non-required/cancelled checks → false 'not passing', blocks auto-dispatch #680)
  • CHANGES_REQUESTED → skip-changes-requested
  • ≥1 unresolved thread → skip-unresolved-threads
  • no checks at all → skip-checks-pending
  • precedence: draft + CHANGES_REQUESTED → skip-draft (gate order)

Test Command

bats --print-output-on-failure test/workflows/pr-auto-review/ and shellcheck -x .github/scripts/pr-auto-review/lib/ready-check.sh

[don-petry]

Dev-Lead: Implementation Complete

Plan Execution

• Added unified pure core pr_auto_review_ready to .github/scripts/pr-auto-review/lib/ready-check.sh — folds criteria Addressing PR comments #1/Add multi-agent isolation strategy using git worktrees #2/Add coding standards & principles section to AGENTS.md #3/Add workflow, environment, and orchestration guidance #4, returns the decision class, exit 0 iff ready. Delegates the required-checks gate verbatim to pr_auto_review_checks_ready (issue pr-auto-review ready-check counts non-required/cancelled checks → false 'not passing', blocks auto-dispatch #680 behaviour unchanged).
• Rewired check-and-dispatch in pr-auto-review-reusable.yml to thin I/O glue: gathers PR state/checks/required-contexts/self-check/review-decision/unresolved-count, calls the pure core once, echoes ready= + decision=<class>. Decision-telemetry step names unchanged.
• Added full decision-matrix bats in test/workflows/pr-auto-review/ready.bats (16 cases incl. precedence + the pr-auto-review ready-check counts non-required/cancelled checks → false 'not passing', blocks auto-dispatch #680 non-required-advisory pin).
• Documented the new contract in .github/scripts/pr-auto-review/README.md.
• Codified the standard in AGENTS.md and standards/agent-standards.md, citing canary-rollout.sh+canary_rollout.bats (ci: add canary-rollout-tests.yml — enforce the engine's bats suite (CI gap from #613/#624) #685) and ready-check.sh as exemplars.

Test Results

$ bats test/workflows/pr-auto-review/
36 tests, 0 failures  (20 existing + 16 new decision-matrix cases)

$ shellcheck -x .github/scripts/pr-auto-review/lib/ready-check.sh
clean

$ bash .dev-lead/scripts/dev-lead-lint.sh
[lint] shellcheck on 21 file(s)... OK
[lint] validating .github/CODEOWNERS... OK
[lint] all checks passed

Decision matrix covered: dispatched (happy path + empty review decision); skip-draft (draft, closed, merged, and draft+CHANGES_REQUESTED precedence); skip-checks-pending (required pending/failing/missing, no checks at all, empty payload — with a NON-required cancelled/failing advisory pinned as non-blocking, #680); skip-changes-requested; skip-unresolved-threads; plus gate-order precedence between #2/#3/#4 and SELF_CHECK exclusion.

Files Changed

• .github/scripts/pr-auto-review/lib/ready-check.sh: new pure pr_auto_review_ready core (delegates to existing pr_auto_review_checks_ready).
• .github/workflows/pr-auto-review-reusable.yml: criteria step now gathers I/O and calls the pure core; decision=<class> outputs preserved.
• test/workflows/pr-auto-review/ready.bats: new decision-matrix suite.
• .github/scripts/pr-auto-review/README.md: documents pr_auto_review_ready + class table.
• AGENTS.md, standards/agent-standards.md: "decision logic in a pure, tested script" standard with the two exemplars.

Notes

• decision=<class> outputs are byte-for-byte the same set of classes Layer 2 telemetry consumes; required-vs-non-required gate (pr-auto-review ready-check counts non-required/cancelled checks → false 'not passing', blocks auto-dispatch #680) is unchanged (delegated, not reimplemented).
• Behaviour-preserving refactor for the workflow glue, so the TDD red phase was driven by the new pure-core bats (which failed with command not found before implementation, then passed) rather than a workflow-level test.
• Minor operational change: the glue now gathers all PR facts before deciding (single call to the core) instead of short-circuiting mid-gather; the emitted ready/decision outputs are identical. Out-of-scope items (other agents' reusables, increment 6) left untouched.