ci: skip Postgres test matrix on docs-only pushes

[jnasbyupgrade]

Summary

• Adds a resolve job to ci.yml that detects doc-only pushes/PRs (via git diff on push/PR SHAs) and gates the PostgreSQL test matrix (test) on it.
• Pattern is ported from Postgres-Extensions/pgxntool-test and pgxntool (job name/output naming: resolve / doc-only), combined with the push+PR git diff handling used in pg_count_nulls/cat_tools.
• .github/* files never count as doc-only even if their extension matches, so workflow-file changes always run tests.
• claude-code-review.yml/claude.yml are untouched — CLAUDE.md/README changes still get code review, only the expensive Postgres matrix is skipped.

Test plan

• python3 -c "import yaml; yaml.safe_load(open('.github/workflows/ci.yml'))" — valid YAML
• Open this PR and confirm the resolve job runs and test matrix is gated correctly (this PR itself touches .github/workflows/ci.yml, so it should NOT be treated as doc-only — good live test of the .github/* exclusion)
• Push a docs-only commit (e.g. touch README.md) on a follow-up branch/PR and confirm the test matrix legs report as skipped, not pending

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6291a9b0-0c95-4113-a799-cc527b999fe2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Claude finished @jnasbyupgrade's task in 3m 28s —— View job

---

Code Review: ci: skip Postgres test matrix on docs-only pushes

• Gather context (PR description, changed files)
• Read .github/workflows/ci.yml diff in full
• Validate resolve job logic (doc-only detection, .github/* exclusion)
• Validate gating of test matrix on resolve output
• Post review feedback

Summary

Overall the doc-only detection script itself is careful and well-reasoned (defaults doc-only=false up front, bails safely on unresolvable SHAs/failed diffs, excludes .github/*, handles the push-vs-PR-vs-first-run SHA cases distinctly). One issue undermines the stated fail-safe guarantee at the job level, though.

Finding: the test job's if: doesn't actually fail open if resolve fails

.github/workflows/ci.yml:81-83:

test:
  needs: resolve
  if: needs.resolve.outputs.doc-only != 'true'

The script's comment says the design is "fail safe to running the full matrix" — doc-only=false is set as the very first thing the step does, so any edge case inside the script degrades to running tests. But that fail-safe only covers failures inside the "Check doc-only" step. If the resolve job fails before or outside that step — e.g. actions/checkout@v4 hits a transient network error, a runner is lost, or the step itself errors before the first echo runs — the resolve job's overall status is failure, and needs.resolve.outputs.doc-only is never set (empty string).

The problem: GitHub Actions implicitly ANDs success() onto a job's if: whenever that job has needs and the if: condition doesn't itself reference success(), failure(), cancelled(), or always(). So if: needs.resolve.outputs.doc-only != 'true' really evaluates as if: success() && needs.resolve.outputs.doc-only != 'true'. When resolve fails, success() is false, and test is skipped entirely — not run with the full matrix. That's the opposite of "fail safe to running the full matrix," and it's a real risk for exactly the class of failures (checkout flakiness, runner issues) this design set out to be robust against, since a skipped required check can look identical to "nothing to test" rather than a build problem.

Suggested fix — make the if: explicitly override the implicit success gate, e.g.:

if: ${{ !cancelled() && needs.resolve.outputs.doc-only != 'true' }}

With this, if resolve fails, its doc-only output stays unset/empty, != 'true' is true, and test still runs the full matrix — matching the intended fail-safe. (Using !cancelled() rather than plain always() avoids forcing test to run if the workflow itself was cancelled.)

Fix this →

Everything else checked out

• .github/* exclusion: correctly implemented as the first case pattern, checked before the doc-extension patterns, so workflow changes always run tests.
• BASE/HEAD resolution: correctly distinguishes pull_request synchronize (uses before/after from the payload, which are documented fields for that action) from first-run PR events (falls back to base.sha/head.sha) from plain push events (before/after). No unhandled event/action combination given this workflow only triggers on [push, pull_request] with default activity types (opened/synchronize/reopened).
• Diff failure handling: git diff --name-only "$BASE" "$HEAD" || echo __DIFF_FAILED__ combined with the all-zero-SHA / empty-SHA guard correctly degrades to doc-only=false rather than erroring out, including the force-push case where before may reference now-unreachable history.
• No injection risk: only SHAs and GitHub-controlled action enum values are interpolated into the run: script — no PR title/branch-name style attacker-controlled strings.
• YAML validity already confirmed via the PR's own test plan (yaml.safe_load).

[jnasbyupgrade]

Closing as superseded: master already has an equivalent, more mature mechanism for this — the changes job (docs_only output), already wired into gating pg-upgrade-test (the job actually expensive enough to be worth gating). This PR's resolve/doc-only job predates pg-upgrade-test's existence entirely (this branch was cut from a much older point in history) and doesn't gate it, so merging this would add a second, parallel, redundant doc-detection mechanism rather than improve anything.

For the record: the AI review's finding on this PR (implicit success() AND on the test job's needs-based if:, causing it to fail closed instead of the intended fail-open on a resolve failure) was technically correct — genuinely worth knowing for future workflow edits in this repo — it's just moot for this specific PR given the above.