Skip to content

ci: switch doc-skip from workflow paths-ignore to a job-level gate#39

Merged
jnasbyupgrade merged 5 commits into
Postgres-Extensions:masterfrom
jnasbyupgrade:ci-per-commit-docs-gate
Jul 20, 2026
Merged

ci: switch doc-skip from workflow paths-ignore to a job-level gate#39
jnasbyupgrade merged 5 commits into
Postgres-Extensions:masterfrom
jnasbyupgrade:ci-per-commit-docs-gate

Conversation

@jnasbyupgrade

@jnasbyupgrade jnasbyupgrade commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Replace the workflow-level paths-ignore doc-skip with a job-level gate. A cheap changes job computes the actual per-push changed-file set and exposes docs_only; the heavy test, pg-upgrade-test, and extension-update-test jobs run only when it is false. This skips the matrix for a doc-only commit per-commit (not just when an entire PR is docs-only), and because the workflow always triggers, the required all-checks-passed job always reports — avoiding the "stuck pending" hazard that trigger-level path filtering has with a required status check.

Also repairs claude-review, which is currently failing on every fork PR: actions/checkout's floating major tags recently gained a guard that refuses to check out fork-PR code under pull_request_target, so claude-code-review.yml's checkout now errors. This adds allow-unsafe-pr-checkout: true to that step (the job is the vetted case — gated to the trusted fork owner, read-only, workflow sourced from master), restoring the review. Also bumps actions/checkout v4->v6 in the Claude workflows (the v4 pin uses the deprecated Node 20 runtime) and updates the stale CLAUDE.md CI note to describe the new gate.

Details

  • changes job: fetch-depth: 0; BASE/HEAD from event.before/event.after (push, and PR synchronize) else PR base.sha/head.sha; fails safe to docs_only=false (run the full matrix) on a missing/all-zero BASE or a failed git diff; sets docs_only=true only when every changed file matches \.(md|asc)$.
  • Heavy jobs gated via needs: [changes] + if: needs.changes.outputs.docs_only != 'true'; all-checks-passed adds changes to needs (its pass/fail logic is unchanged — it already treats skipped as success).
  • claude-review's existing cost gate already treats skipped siblings as clean (its bad-check set is only failure|cancelled|timed_out|action_required|stale), so the review still runs on doc-only changes — no gate-logic change needed.
  • claude-code-review.yml's fork-checkout safety rationale now lives in one place: the owner-only if: on the claude-review job carries the security-critical statement that this gate is what makes allow-unsafe-pr-checkout acceptable, and the comment at allow-unsafe-pr-checkout: true itself just points back to that gate instead of repeating a list of secondary reasons (persist-credentials, read-only, workflow-sourced-from-master) that could go stale independently of the actual control.

Note: the checkout bump and the fork-checkout fix can't be observed on this PR's own run — claude-code-review.yml runs the base-branch (master) copy under pull_request_target, so both only take effect once this merges (which is also why claude-review stays red on this PR itself). master currently has no branch protection, so nothing is blocked today; the gate change also preempts the stuck-Pending hazard for whenever a required check is added. This branch has been rebased twice onto master as it moved (most recently past #42's relkind fix) to keep the diff limited to the workflow files and CLAUDE.md.

@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown

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: 5ef303ad-10b8-41c3-abe0-73c35d866fff

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
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

jnasbyupgrade added a commit to jnasbyupgrade/cat_tools that referenced this pull request Jul 18, 2026
Doc-only commit to HISTORY.asc to prove, on a live CI run, that the
job-level docs-only gate correctly skips the heavy matrix jobs (test,
pg-upgrade-test, extension-update-test) while all-checks-passed still
runs and reports green. Kept in the branch intentionally as evidence;
can be squashed/dropped before merge.
@jnasbyupgrade
jnasbyupgrade force-pushed the ci-per-commit-docs-gate branch from 34892ba to 27e64df Compare July 18, 2026 21:03
jnasbyupgrade added a commit to jnasbyupgrade/cat_tools that referenced this pull request Jul 20, 2026
…hs-ignore

The GitHub CI note still described workflow-level paths-ignore, which
this PR (Postgres-Extensions#39) replaces with a per-push changes/docs_only job-level
gate. Update the note so it reflects the mechanism actually in ci.yml.
@jnasbyupgrade
jnasbyupgrade force-pushed the ci-per-commit-docs-gate branch from 3dc7d91 to 4b4a85d Compare July 20, 2026 19:56
jnasbyupgrade added a commit to jnasbyupgrade/cat_tools that referenced this pull request Jul 20, 2026
…hs-ignore

The GitHub CI note still described workflow-level paths-ignore, which
this PR (Postgres-Extensions#39) replaces with a per-push changes/docs_only job-level
gate. Update the note so it reflects the mechanism actually in ci.yml.
@jnasbyupgrade
jnasbyupgrade force-pushed the ci-per-commit-docs-gate branch from 4b4a85d to 38bbb94 Compare July 20, 2026 19:59
paths-ignore on the workflow trigger only skips CI when the entire
push/PR diff is docs-only; for a PR that also touches code, every
commit runs the full matrix even if that commit is doc-only. It's also
the classic "stuck Pending" hazard for a required status check: if the
whole workflow (including all-checks-passed) never runs, the required
check never reports.

Add a cheap `changes` job that computes the per-push changed file set
(github.event.before/after for push and PR synchronize; PR base/head
sha otherwise) and sets docs_only only when every changed file matches
.md/.asc, failing safe (docs_only=false) on any diff failure or
ambiguous base. Gate `test`, `pg-upgrade-test`, and
`extension-update-test` on `needs.changes.outputs.docs_only != 'true'`,
and add `changes` to all-checks-passed's needs so it keeps running and
reporting on every commit.
actions/checkout@v4 pins to the deprecated Node 20 runtime, which
GitHub Actions warns about on every run. ci.yml already uses @v6;
align claude-code-review.yml and claude.yml so all workflows are on
the same, non-deprecated major version.
…st_target job

actions/checkout added a guard that refuses to check out fork PR heads
under pull_request_target/workflow_run by default (a "pwn request"
mitigation), plus an allow-unsafe-pr-checkout escape hatch for the
reviewed cases where that is intentional. This job is exactly that
case: it already gates on the trusted fork owner, checks out read-only
(persist-credentials: false), and the workflow file itself always
comes from master under pull_request_target, so a PR can't alter the
job that reviews it.

The guard/input shipped in actions/checkout v7.0.0 and has since been
backported to the v4/v5/v6 floating major tags too (confirmed directly
against actions/checkout's live action.yml on each tag), so no
major-version bump beyond the existing @v6 is needed to use it here.
claude.yml is unaffected: it doesn't trigger on
pull_request_target/workflow_run and its checkout step doesn't target
a fork PR head, so it isn't subject to this guard.
…hs-ignore

The GitHub CI note still described workflow-level paths-ignore, which
this PR (Postgres-Extensions#39) replaces with a per-push changes/docs_only job-level
gate. Update the note so it reflects the mechanism actually in ci.yml.
…ces it

The allow-unsafe-pr-checkout comment listed several reasons the fork
checkout was "safe" (owner gate, persist-credentials: false, read-only,
workflow sourced from master). A laundry list like that rots: any one
item can change later while the comment keeps reassuring, and the
actual enforcing control is easy to miss among the secondary points.

Move the security-critical statement to the owner-only `if:` on the
claude-review job, since that gate is the one thing that makes the
fork checkout acceptable at all: it stops arbitrary fork code from
being checked out in the trusted pull_request_target context (a "pwn
request"). The allow-unsafe-pr-checkout comment now just states it's
unsafe by default and points to that gate, instead of re-deriving the
safety case inline.
@jnasbyupgrade
jnasbyupgrade force-pushed the ci-per-commit-docs-gate branch from ab21c4c to da986fe Compare July 20, 2026 22:34
@jnasbyupgrade
jnasbyupgrade merged commit 7b23d1a into Postgres-Extensions:master Jul 20, 2026
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant