CI: fold PG12+ update-to-current check into test, rename legacy job for clarity

[jnasbyupgrade]

extension-update-test's PG12+ leg (CREATE EXTENSION at 0.2.2, ALTER EXTENSION UPDATE to current, structural diff against a fresh install, then the full suite) used to run as its own separate matrix job. It's now folded into the test job's existing step instead, since that job already has a container/checkout/install running for the same PostgreSQL majors — a separate job was paying for all of that again for no added coverage. The old job now only covers PG10's legacy pre-0.2.2 install/update-script checks, its one remaining reason to exist, so it's renamed legacy-extension-update-test to say so.

What changed

• test job: added a step running bin/test_existing update-scenario cat_tools_update 0.2.2 right after make verify-results. Its comment explains why this isn't equivalent to make test-update — update-scenario additionally plants and proves the dependency guard and structurally diffs the result against a fresh install before running the suite.
• extension-update-test → legacy-extension-update-test: renamed (job key and display name), and its PG12+ matrix leg dropped entirely. It now runs on a single fixed major (PG10, via needs.changes.outputs.legacy_pg — single source of truth, not a hardcoded '10') with no strategy: matrix: at all, and the now-dead if: matrix.pg != '10' / == '10' guards are gone.
• changes job: removed the update_pg output/derivation — its only consumer is gone.
• Comment updates to describe the rename and fold: the test and legacy-extension-update-test entries in the "Test strategy" summary, the cross-reference in pg-tle-test's own comment, and CLAUDE.md's "CI jobs" section (which described the pre-fold matrix).
• No coverage lost: the PG12+ check still runs on the exact same 7 majors it always did (moved, not removed); the PG10 legacy checks are byte-for-byte unchanged; every other job is untouched.

Background

This (plus #77, already merged) is a clean rebuild, on current master, of the schema-independent CI-fold value that used to be PR #75 — split at the maintainer's request into a docs-only PR (#77) and this functional-only one, since mixing them muddied review. #75 itself is now superseded/closed; see its closing comment. That work in turn traces back to PR #54 (closed without merging: its actual subject, a TEST_SCHEMA test-harness dimension, turned out to be a non-starter since cat_tools' control file pins relocatable = false / schema = 'cat_tools').

Test plan

• make check-relkind-source && make verify-results && bin/test_existing update-scenario cat_tools_update 0.2.2 passes against a scratch cluster, mirroring the new CI step: no database-name collision, dependency-guard proof fires, structural diff reports identical, full suite passes
• make lint clean
• CI green

[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: 46b4bffb-3bf0-4c4d-b418-be89a2020449

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 36s —— View job

---

Claude finished the review

Todo list

• Gather context (read PR diff, CLAUDE.md, full .github/workflows/ci.yml)
• Understand the request (code review of the CI workflow restructuring)
• Review the diff for correctness (job gating, matrix→scalar conversion, output wiring, coverage parity)
• Post review feedback

Review

I traced through the whole workflow (not just the diff) to check the restructuring is coherent. The core mechanics check out:

• changes job: update_pg output/derivation is fully removed, and I confirmed no remaining reference to it anywhere else in the file (grep -n update_pg finds nothing).
• test job: the new bin/test_existing update-scenario cat_tools_update 0.2.2 step runs after make verify-results in the same run: block. GitHub's default bash shell runs with -e, so if verify-results fails, the update-scenario command never executes — the per-major gating that mattered from the old design is preserved even though the step moved.
• make verify-results depends on $(TEST_DEPS), which includes install (pgxntool/base.mk:374), so cat_tools is already installed by the time bin/test_existing update-scenario runs — no missing make install step was needed in test.
• extension-update-test: strategy: matrix is gone, matrix.pg no longer appears anywhere in that job (confirmed via grep), and the now-dead if: matrix.pg != '10' / == '10' guards were correctly dropped along with the steps that only applied to the other leg.
• The job's name: field uses ${{ needs.changes.outputs.legacy_pg }} directly (not env.LEGACY_PG) — correct, since the needs context (unlike job-level env) is valid in jobs.<id>.name.
• Coverage math: old extension-update-test ran 8 legs (supported_pg 12–18 + legacy_pg 10). New split is 7 legs' worth of update-to-current checks folded into test's existing 7-major matrix, plus 1 fixed PG10 leg in extension-update-test — same 8 total, nothing dropped.
• One subtle, intentional-looking behavior change worth being aware of (not a bug): previously, extension-update-test's PG12+ legs were gated on the entire test job succeeding (all matrix legs green) before any update check ran. Now that the update check is a step inside test's own per-major leg, a failure on PG18's fresh-install no longer blocks the update check on PG14 from running. That's a reasonable side effect of the fold, and consistent with how test's matrix already behaves per-major elsewhere in this workflow, but flagging it since it's a real (if minor) semantic shift from the old gating.

One nit: the "Derive the supported-PostgreSQL-major lists" step's comment block (.github/workflows/ci.yml:194-198, untouched by this diff) still says "the fresh-install test matrix, the extension-update-test matrix and the stepwise climb" — but this PR removes extension-update-test's strategy: matrix: entirely, so calling it "the extension-update-test matrix" is now stale. Minor, since the underlying point (single source of truth for PG versions) still holds, but worth a follow-up tweak since this PR is the one that made it stale.

Everything else — the changes/test/extension-update-test/all-checks-passed wiring, the reformatted "Test strategy" comment block, and the pg-tle-test cross-reference update — is internally consistent and matches the PR description. No functional issues found.

[github-actions]

Claude finished @jnasbyupgrade's task in 2m 48s —— View job

---

Tasks

• Gather context (PR diff, CLAUDE.md, full .github/workflows/ci.yml)
• Review the workflow restructuring for correctness (job gating, matrix→scalar conversion, output wiring, coverage parity)
• Post review feedback

Review

I re-verified the full workflow against the PR description (not just the diff). The mechanics are sound:

• changes job: update_pg output is fully gone and no reference to it remains anywhere in the file (checked via grep for update_pg/matrix.pg).
• extension-update-test: strategy: matrix is removed, matrix.pg doesn't appear anywhere in that job, and it correctly uses needs.changes.outputs.legacy_pg (single source of truth) instead of a hardcoded '10', including in the job-level name: — valid, since needs context is available at that scope.
• test job: the new bin/test_existing update-scenario cat_tools_update 0.2.2 step runs in the same run: block right after make verify-results. Default bash -e means a verify-results failure stops the script before the update-scenario line runs, so per-major gating is preserved even though the check moved into the matrix leg. Confirmed bin/test_existing has an update-scenario subcommand (bin/test_existing:251,405), and make verify-results depends on $(TEST_DEPS) (includes install), so cat_tools is already installed before this step runs — no missing setup.
• Coverage math checks out: old extension-update-test ran 8 legs (7 supported majors + PG10 legacy); new split is the same 7 update-to-current checks folded into test's matrix + 1 fixed PG10 leg — nothing dropped.
• No merge-conflict markers or YAML structural issues from the docs-test-strategy-format merge commit.

Still-open nit from the earlier pass on this PR: the "Derive the supported-PostgreSQL-major lists" step's comment (.github/workflows/ci.yml:196) still reads "...the fresh-install test matrix, the extension-update-test matrix and the stepwise climb..." — but this PR removes extension-update-test's strategy: matrix: entirely, so "the extension-update-test matrix" is stale phrasing now. Same finding as before; doesn't look like it's been addressed yet. Fix this →

One additional observation (not a bug, worth being aware of): folding the update-to-current check into test's per-major step means a fresh-install failure and an update-path failure on the same PG major now surface under a single check name (e.g. "🐘 PostgreSQL 14"), where they previously had distinct job/check names. This is a reasonable tradeoff for avoiding a duplicate container/checkout, but it does mean the Checks tab alone can no longer distinguish "fresh install broke" from "update path broke" for a given major — you'd need to open the step logs. Likely acceptable given the PR's stated goal, just flagging the tradeoff explicitly.

Everything else — the changes/test/extension-update-test/all-checks-passed wiring, the reformatted "Test strategy" comment block, and the pg-tle-test cross-reference cleanup — is internally consistent and matches the PR description. No functional issues found.

[github-actions]

Claude finished @jnasbyupgrade's task in 2m 21s —— View job

---

Tasks

• Gather context (PR diff, CLAUDE.md, full .github/workflows/ci.yml)
• Review the workflow restructuring for correctness
• Post review feedback

Review

This is the third pass on this PR (two earlier automated reviews already ran on prior pushes). The one nit those raised — stale "extension-update-test matrix" wording in the changes job's "Derive the supported-PostgreSQL-major lists" comment — has since been fixed in 720b1f2; I confirmed the comment now reads `test`/`pg-tle-test` matrix, `extension-update-test`'s single legacy major, and the stepwise climb (ci.yml:196-197).

Re-verified the mechanics from scratch against the current file, not just the diff:

• update_pg output/derivation is fully gone from the changes job, and grepping the whole file for update_pg/matrix.pg turns up nothing left in extension-update-test — no dangling references.
• test job (ci.yml:340-384): the new bin/test_existing update-scenario cat_tools_update 0.2.2 step runs in the same run: block right after make verify-results. Default bash -e means a verify-results failure stops the script before reaching the update-scenario line, preserving the old per-major gating even though the check moved into the matrix leg.
• Confirmed make verify-results genuinely depends on $(TEST_DEPS) (pgxntool/base.mk:417, verify-results: $(TEST_DEPS), distinct from the recipe-only rule at line 385) — TEST_DEPS includes install/installcheck (base.mk:374), so cat_tools is already installed on disk before the new step runs. No missing setup.
• Confirmed bin/test_existing has an update-scenario subcommand (bin/test_existing:251,405) matching the new step's invocation.
• extension-update-test (ci.yml:711-772): strategy: matrix is gone entirely, it correctly derives its single major from needs.changes.outputs.legacy_pg (including in the job-level name:, where needs context — unlike job-level env — is valid), and the dead if: matrix.pg != '10' / == '10' guards are removed along with the steps that only applied to the other leg.
• all-checks-passed's needs: list (ci.yml:1028) still matches the full set of defined jobs exactly (verified by extracting both lists) — nothing was orphaned by the restructuring.
• Coverage math holds: old extension-update-test ran 8 legs (7 supported majors + PG10 legacy); new split is the same 7 update-to-current checks folded into test's existing matrix + 1 fixed PG10 leg — nothing dropped, nothing duplicated.

Everything is internally consistent and matches the PR description. No functional issues found — this looks ready to merge (pending #77 landing first, per the PR description, so the diff shrinks to just the functional commit).

[github-actions]

Claude finished @jnasbyupgrade's task in 2m 36s —— View job

---

Review

This is a follow-up pass after the latest push (f208428), which addressed the one nit raised in the earlier three review rounds on this PR (stale "extension-update-test matrix" wording). Re-verified the full diff (git diff ceb4cec..HEAD -- .github/workflows/ci.yml, isolating this PR's own two commits from the already-merged #77 base) against the live file, not just the patch:

• Gather context (PR diff isolated to this PR's own commits, CLAUDE.md, full current ci.yml)
• Re-verify the mechanics (job gating, matrix→scalar conversion, output wiring, coverage parity)
• Check the previously-flagged nit
• Post review feedback

Previously-flagged nit: fixed. The "Derive the supported-PostgreSQL-major lists" comment (ci.yml:194-198) now correctly reads "the test/pg-tle-test matrix, extension-update-test's single legacy major, and the stepwise climb" — no longer stale.

Mechanics re-confirmed:

• changes job: update_pg output/derivation fully removed; no remaining reference anywhere (update_pg/matrix.pg grep is clean outside the jobs that still legitimately use matrix.pg).
• test job (ci.yml:340-384): the new bin/test_existing update-scenario cat_tools_update 0.2.2 step runs in the same run: block right after make verify-results. Default bash -e means a verify-results failure aborts before reaching the update-scenario line, so the old per-major gating is preserved even though the check moved into the matrix leg. make verify-results depends on $(TEST_DEPS) (includes install), so cat_tools is already installed before this step — no missing setup.
• extension-update-test (ci.yml:711-772): strategy: matrix is gone entirely, correctly derives its single major from needs.changes.outputs.legacy_pg (including in the job-level name:, where the needs context — unlike job-level env — is actually valid per GitHub's context-availability rules), and the dead if: matrix.pg != '10' / == '10' guards are removed along with the steps that only applied to the other leg.
• all-checks-passed's needs: list (ci.yml:1028) still matches the full set of defined jobs exactly, and the job self-verifies this at runtime (ci.yml:1033-1051) — nothing orphaned by the restructuring.
• Coverage math holds: old extension-update-test ran 8 legs (7 supported majors + PG10 legacy); new split is the same 7 update-to-current checks folded into test's existing matrix + 1 fixed PG10 leg — nothing dropped, nothing duplicated.

No functional issues found. This looks ready to merge.

[github-actions]

Claude finished @jnasbyupgrade's task in 2m 4s —— View job

---

Tasks

• Gather context (isolate this push's diff: git show f516e88, full current ci.yml, bin/test_existing, CLAUDE.md)
• Review the new commit (f516e88, comment-only) for accuracy against the code it describes
• Cross-check the comment change against the other comment block it's referenced from
• Post review feedback

Review

This push (f516e88) is comment-only — it rewrites the test job's update-to-current step comment (ci.yml:356-379) to explain why bin/test_existing update-scenario isn't just make test-update, and drops the "verified locally before folding it in" paragraph per CLAUDE.md's guidance not to memorialize routine pre-commit verification.

I checked the new comment's claims against bin/test_existing's actual behavior:

• The order described (create DB + extension at 0.2.2 → plant guard → update to current → assert_matches_fresh structural diff → run suite) matches update_scenario() exactly (bin/test_existing:250-258).
• assert_matches_fresh is a real function (bin/test_existing:205) that itself calls bin/structural_diff compare, matching the comment's "(assert_matches_fresh, via bin/structural_diff)".
• The make test-update contrast (install 0.2.2 → ALTER EXTENSION UPDATE → run the suite, all inside pg_regress's own throwaway db) matches CLAUDE.md's description of TEST_LOAD_SOURCE=update mode and test/install/*.sql running once inside the same pg_regress invocation.

Bug: stale cross-reference left dangling by this commit. The "Test strategy" summary block still says, at ci.yml:258-259:

...for no added coverage. See that step's own comment for what was verified locally before folding it in.

But f516e88 removed the entire "Verified locally before folding it in" paragraph from that step's comment (ci.yml:356-379) — there's nothing there anymore to point to. This is the same class of staleness the earlier review rounds on this PR caught and got fixed (the "extension-update-test matrix" wording in f208428), just introduced fresh by this latest push. Fix this →

Everything else re-checked clean: no other reference to "verified locally" remains anywhere in the file, the rest of the "Test strategy" block and the other job comments are unaffected by this push, and the mechanics validated in the three earlier review rounds on this PR (job gating, matrix→scalar conversion, output wiring, coverage parity) are untouched by this comment-only commit.

[github-actions]

Claude finished @jnasbyupgrade's task in 1m 56s —— View job

---

Tasks

• Gather context (latest push f9a7d44, full current ci.yml, prior review history on this PR)
• Review the latest commit and check the previously-flagged stale cross-reference
• Post review feedback

Review

This push (f9a7d44) is comment-only and does exactly one thing: it removes the dangling cross-reference flagged in the previous review round. The test job's "Test strategy" summary used to end with "See that step's own comment for what was verified locally before folding it in" — but f516e88 (the prior commit) had already deleted that paragraph from the step's own comment, leaving the sentence pointing at nothing. f9a7d44 drops it cleanly (ci.yml:255-258), and I confirmed grep -n "verified locally" now returns no matches anywhere in the file — nothing else is left dangling.

Re-verified the full file for consistency, not just this diff:

• The two comment blocks that reference the fold (test job's own step comment at ci.yml:356-379, and the "Test strategy" summary at ci.yml:255-258) no longer duplicate content or cross-reference something removed — the summary states the fold briefly, the step comment carries the "why not make test-update" detail. Consistent with CLAUDE.md's "never repeat the same comment verbatim" and "explain at the first mention" guidance.
• update_pg/matrix.pg: no stale references remain — matrix.pg only appears in test and pg-tle-test, which legitimately keep their matrices; extension-update-test (ci.yml:705-766) has none.
• extension-update-test's job-level name: and env.LEGACY_PG correctly derive from needs.changes.outputs.legacy_pg (single source of truth), with no hardcoded '10'.
• all-checks-passed's needs: list (ci.yml:1022) matches the full set of defined jobs exactly (changes, test, lint, verify-cancel-on-close-coupling, pg-upgrade-test, pg-upgrade-stepwise, extension-update-test, pg-tle-test, pg-tle-upgrade-test) — nothing orphaned.
• The "Derive the supported-PostgreSQL-major lists" comment (ci.yml:196-197, fixed by f208428 earlier in this PR) still correctly reads "the test/pg-tle-test matrix, extension-update-test's single legacy major, and the stepwise climb."

No functional issues found, and no remaining staleness. This looks ready to merge (pending #77 already landed, per branch history it's in the log as ceb4cec).

[github-actions]

Claude finished @jnasbyupgrade's task in 2m 20s —— View job

---

Tasks

• Gather context (latest push e8a3bfa, full current ci.yml/CLAUDE.md, prior review history on this PR)
• Review the latest commit (rename extension-update-test → legacy-extension-update-test) for correctness
• Post review feedback

Review

This push (e8a3bfa) is a pure rename + doc-catchup commit: extension-update-test → legacy-extension-update-test, now that its only remaining purpose (after the earlier fold in 47836c0) is the PG10-only legacy pre-0.2.2 update-script path.

Verified against the committed state (git show HEAD:..., not just the diff):

• ci.yml: every occurrence of the old name is renamed — job key (ci.yml:705), job-level name: display string, all-checks-passed's needs: list (ci.yml:1022), and all 6 comment references (ci.yml:196, 215, 255, 269, 356, 375). grep -n "extension-update-test" on the committed file turns up only the new legacy-extension-update-test spelling — no stray old-name reference left anywhere in the repo.
• all-checks-passed's job list still matches its own dynamic self-check (ci.yml:1027-1045, which derives DEFINED from the workflow's actual job keys rather than a hardcoded list) — nothing to go stale there regardless of renames.
• pg-tle-test's cross-reference (ci.yml:879-882) already says "the test job" (updated in an earlier commit of this PR, f516e88/f9a7d44 era) — correctly does not need touching by this rename.
• CLAUDE.md: the "CI jobs" section (CLAUDE.md:147-157) is rewritten to split into a test-job bullet (widest update path, PG12+) and a legacy-extension-update-test bullet (PG10-only pre-0.2.2 scripts) — this section had actually been left describing the pre-fold matrix since 47836c0 landed, so this commit is also closing a doc gap that predates the rename itself, per its own commit message. The docs_only-skip parenthetical (CLAUDE.md:7) is updated to the new name too, and it's still accurate: legacy-extension-update-test's if: condition (ci.yml:707) does gate on needs.changes.outputs.docs_only != 'true'.
• No other committed file references the old name (grep -r across the repo for both extension-update-test and the old display string Extension update test on PostgreSQL returns only these two files).

One non-blocking heads-up, outside the diff: renaming a job's name: changes the GitHub Checks display string. If branch protection on master has a required status check configured against the literal old name ("⬆️ Extension update test on PostgreSQL …"), it'll need updating on GitHub's side after this merges — not something visible in-repo, just flagging since it's the real-world consequence of this kind of rename.

No functional issues found. This looks ready to merge (per the PR description, pending #77 already landed — confirmed in the log as ceb4cec).