Static check that the update script covers this cycle's install-script changes

.github/workflows/ci.yml

@@ -394,6 +394,10 @@ jobs:
     steps:
       - name: Check out the repo
         uses: actions/checkout@v7
+      - name: Test bin/
+        # First, before anything that uses bin/: a bug in the tooling should be
+        # reported as a bug in the tooling, not as a finding against the SQL.
+        run: make test-bin
       - name: Lint SQL
         # CRITICAL: call `make lint` directly, not some other path (a script, a
         # different target, etc). lint.mk's vendored include is guarded on
@@ -404,6 +408,12 @@ jobs:
         # wrapper that swallows that exit code or calls a different entry point
         # would defeat this safety net.
         run: make lint
+      - name: Check the update script covers this cycle's install-script changes
+        # Static and database-free (see bin/update_lint_textfirst's header),
+        # which is why it rides this job rather than the pgxn-tools container.
+        # A step, not a job: all-checks-passed hard-fails when the workflow's
+        # job-key set differs from its `needs:`.
+        run: make update-lint
 
   # cancel-on-close.yml cancels in-flight CI/claude-review runs on PR close by
   # joining the SAME concurrency group strings this workflow and

CLAUDE.md

@@ -55,6 +55,13 @@ documented in `../ai/CLAUDE.md` or pgxntool's own docs.
   `.sql` directly (historical exception; see `sql/.gitignore`).
 - Never hand-edit a generated `.sql` file — edit the `.sql.in` (or, for
   the base file, `sql/cat_tools.sql.in`) and regenerate.
+- **A change to `sql/cat_tools.sql.in` must also extend the update
+  script `sql/cat_tools--<last-released>--stable.sql.in`**, so an
+  existing install reaches the same objects. `make update-lint` checks
+  that statically (no database) and runs in CI's `lint` job; see
+  `bin/update_lint_textfirst`'s header for what it catches, what it
+  cannot, and the `-- update-lint: ok /REGEX/ reason` escape hatch for
+  deliberate divergence.
 
 See [`../ai/CLAUDE.md`](../ai/CLAUDE.md) for the general (pgxntool-level)
 rules this preprocessing sits on top of: why version-specific install and

Makefile

@@ -105,3 +105,25 @@ clean_old_version:
 # `.vendor/linter/sql/bin/sql-lint sql/cat_tools--0.3.0.sql.in`.
 LINT_TARGETS = sql/cat_tools.sql.in test/
 include lint.mk
+
+# Static check that this cycle's update script accounts for every change to the
+# install script -- no database, no SQL parser. See bin/update_lint_textfirst's
+# header for what it does and does not catch.
+#
+# Deliberately NOT tied to `lint` in either direction. lint.mk's include is
+# guarded on $(wildcard .git), so `lint` does not exist as a target in a
+# released tarball and `make lint` fails loudly there; naming it as a
+# prerequisite would define it with no recipe and turn that loud failure into a
+# silent pass.
+.PHONY: update-lint
+update-lint:
+	bin/update_lint_textfirst
+
+# Everything in bin/ that the rest of the suite leans on -- bin/test_existing
+# and bin/structural_diff drive the pgTAP runs, bin/update_lint_textfirst drives
+# `update-lint` above -- so run this before anything that uses them: a broken
+# tool otherwise reports as a broken extension. prove takes the directory, so a
+# new bin/test/*.t needs no edit here. Test::Harness is core Perl; no setup.
+.PHONY: test-bin
+test-bin:
+	prove bin/test/

bin/test/textfirst.t

@@ -0,0 +1,226 @@
+#!/usr/bin/env perl
+#
+# One case per rule bin/update_lint_textfirst implements, plus the three things
+# only the real tree can prove: that the current development pair is clean, that
+# the ALTER DEFAULT PRIVILEGES rule reproduces the historical bug it was written
+# for, and that preprocessing erases sql.mk's " VERSIONED FILE!" tag. Kept
+# deliberately small -- a checker whose test suite dwarfs it has stopped being
+# the cheap option. Run from the repo root:
+#
+#     prove bin/test/textfirst.t
+
+use strict;
+use warnings;
+use Test::More tests => 32;
+use File::Temp qw(tempdir);
+
+my $PROG = 'bin/update_lint_textfirst';
+my $DIR  = tempdir(CLEANUP => 1);
+
+# Write an OLD/NEW/UPDATE trio and run the linter over it.
+sub run_trio {
+    my (%f) = @_;
+    my @p;
+    for my $k (qw(old new update)) {
+        my $p = "$DIR/$k.sql";
+        open my $fh, '>', $p or die $!;
+        print $fh $f{$k} // '';
+        close $fh;
+        push @p, $p;
+    }
+    my $out = qx{$^X $PROG @p 2>&1};
+    return ($? >> 8, $out);
+}
+
+# --- the splitter -----------------------------------------------------------
+
+{
+    # A `;` inside a string, a quoted identifier, a comment and a dollar-quoted
+    # body must not end a statement. If any did, NEW would hold extra
+    # statements OLD lacks and they would be reported as added.
+    my $sql = <<'SQL';
+SELECT 'a;b';
+SELECT "we;ird";
+SELECT 1; -- trailing ; comment
+CREATE FUNCTION f() RETURNS int LANGUAGE plpgsql AS $body$
+BEGIN
+  /* nested /* block ; comment */ still in here ; */
+  RETURN 1;
+END
+$body$;
+SQL
+    my ($rc, $out) = run_trio(old => $sql, new => $sql, update => '');
+    is $rc, 0, 'identical files are clean';
+    like $out, qr/old \S+ \(4 statements\)/, 'four top-level statements found';
+}
+
+# --- rule 4: substring against the whole update file ------------------------
+
+{
+    my ($rc, $out) = run_trio(
+        old    => "SELECT 1;\n",
+        new    => "SELECT 1;\nCREATE TABLE t (a int);\n",
+        # Wrapped in a DO block, so it is not a top-level statement over here.
+        update => "DO \$\$ BEGIN EXECUTE 'CREATE TABLE t (a int)'; END \$\$;\n",
+    );
+    is $rc, 0, 'a copy buried in a DO block satisfies the added statement';
+    like $out, qr/added 1 \(matched 1,/, '... and is counted as matched';
+}
+
+{
+    my ($rc, $out) = run_trio(
+        old    => "SELECT 1;\n",
+        new    => "SELECT 1;\nCREATE TABLE t (a int);\n",
+        update => "SELECT 2;\n",
+    );
+    is $rc, 1, 'an added statement with no copy fails';
+    like $out, qr/CREATE TABLE t \(a int\)/, '... and is named in the finding';
+}
+
+# --- enum labels, which are out of scope ------------------------------------
+
+{
+    # The pgTAP suite asserts enum contents against the catalog across fresh,
+    # updated and pg_upgraded databases, so this does not second-guess it.
+    my ($rc, $out) = run_trio(
+        old    => "CREATE TYPE e AS ENUM ('a', 'b');\n",
+        new    => "CREATE TYPE e AS ENUM ('a', 'b', 'c');\n",
+        update => "SELECT 1;\n",
+    );
+    is $rc, 0, 'a changed enum on a pre-existing type is exempt';
+    like $out, qr/enum 1,/, '... and the exemption is reported, not silent';
+}
+
+{
+    # Only CHANGED enums are exempt. A brand-new one copies into the update
+    # script verbatim, so the ordinary rule has to keep applying to it.
+    my ($rc, $out) = run_trio(
+        old    => "SELECT 1;\n",
+        new    => "SELECT 1;\nCREATE TYPE e AS ENUM ('a');\n",
+        update => "SELECT 2;\n",
+    );
+    is $rc, 1, 'a brand-new enum type is still checked for a copy';
+    like $out, qr/CREATE TYPE e AS ENUM/, '... and is named in the finding';
+}
+
+# --- scaffolding ------------------------------------------------------------
+
+{
+    # __cat_tools is created and dropped inside the install script, so its
+    # contents cannot differ between a fresh and an updated database.
+    my ($rc, $out) = run_trio(
+        old    => "CREATE FUNCTION __cat_tools.helper() RETURNS void LANGUAGE sql AS 'SELECT';\n",
+        new    => "CREATE FUNCTION __cat_tools.helper(a int) RETURNS void LANGUAGE sql AS 'SELECT';\n",
+        update => "SELECT 1;\n",
+    );
+    is $rc, 0, 'a changed scaffolding definition is exempt';
+    like $out, qr/scaffolding 1,/, '... and the exemption is reported, not silent';
+}
+
+{
+    my ($rc, $out) = run_trio(
+        old    => "SELECT 1;\n",
+        new    => "SELECT 1;\nSELECT __cat_tools.create_function('cat_tools.f');\n",
+        update => "SELECT 2;\n",
+    );
+    is $rc, 1, 'a CALL to scaffolding is still checked -- it creates a real object';
+    like $out, qr/create_function\('cat_tools\.f'\)/, '... and is named in the finding';
+}
+
+# --- the escape hatch -------------------------------------------------------
+
+{
+    my ($rc, $out) = run_trio(
+        old    => "SELECT 1;\n",
+        new    => "SELECT 1;\nCREATE TABLE t (a int);\n",
+        update => "-- update-lint: ok /CREATE TABLE t/ hand-rewritten below\n"
+                . "CREATE TABLE t (a int NOT NULL);\n",
+    );
+    is $rc, 0, 'a waiver suppresses the finding';
+    like $out, qr/waived .*hand-rewritten below/, '... and prints its reason';
+}
+
+{
+    my ($rc, $out) = run_trio(
+        old    => "SELECT 1;\n",
+        new    => "SELECT 1;\n",
+        update => "-- update-lint: ok /nothing matches this/ stale\n",
+    );
+    is $rc, 1, 'a waiver that matches nothing fails';
+    like $out, qr{stale waiver /nothing matches this/}, '... and says which one';
+}
+
+{
+    # Whitespace closes the regex, not the first `/`. Closing at the first one
+    # would waive /a/ here -- far wider than written, and silently.
+    my ($rc, $out) = run_trio(
+        old    => "SELECT 1;\n",
+        new    => "SELECT 1;\nSELECT a/b;\n",
+        update => "-- update-lint: ok /a/b/ division is fine\n",
+    );
+    is $rc, 0, 'a waiver regex may contain a slash';
+    like $out, qr/waived .*division is fine/, '... and keeps the whole reason';
+}
+
+# A typo must not degrade into "no waiver at all". Both of these would otherwise
+# leave the author staring at a finding they believe they already waived.
+for my $bad (
+    ['-- update-lint: ok /CREATE TABLE t/',             'a waiver with no reason'],
+    ['-- update-lint: okay /CREATE TABLE t/ mistyped',  'a mistyped waiver keyword'],
+) {
+    my ($line, $desc) = @$bad;
+    my ($rc, $out) = run_trio(
+        old    => "SELECT 1;\n",
+        new    => "SELECT 1;\nCREATE TABLE t (a int);\n",
+        update => "$line\n",
+    );
+    is $rc, 2, "$desc is a usage error, not a silent no-op";
+    like $out, qr/malformed waiver/, '... naming it as malformed';
+}
+
+# --- ALTER DEFAULT PRIVILEGES -----------------------------------------------
+
+{
+    # Only one ADP shape is understood. A different one must say so rather than
+    # pass as "nothing to see" -- a copy in the update script is not enough,
+    # since ADP does not reach objects that already exist.
+    my $adp = 'ALTER DEFAULT PRIVILEGES FOR ROLE r IN SCHEMA s GRANT USAGE ON TYPES TO u;';
+    my ($rc, $out) = run_trio(
+        old    => "SELECT 1;\n",
+        new    => "SELECT 1;\n$adp\n",
+        update => "$adp\n",
+    );
+    is $rc, 1, 'an ALTER DEFAULT PRIVILEGES form the rule cannot read fails';
+    like $out, qr/unrecognized ALTER DEFAULT PRIVILEGES form/, '... saying so, not skipping it';
+}
+
+# --- against the real tree --------------------------------------------------
+
+SKIP: {
+    skip 'run from the repo root', 6 unless -e 'sql/cat_tools--0.2.1.sql.in';
+
+    # No arguments at all: the pair every SQL-touching PR is judged on. It has
+    # to be clean, or the CI step this drives is useless from the day it lands.
+    my $out = qx{$^X $PROG 2>&1};
+    is $? >> 8, 0, 'the current development pair is clean';
+    like $out, qr{new sql/cat_tools\.sql\.in\b}, '... comparing against the base install script';
+    like $out, qr{update sql/cat_tools--\S+--stable\.sql\.in}, '... via this cycle\'s update script';
+
+    # A released install script is a copy of the base file with sql.mk's
+    # " VERSIONED FILE!" tag added to every @generated@ marker. One of those
+    # markers sits inside a dollar-quoted function body where no comment strip
+    # can reach it, so the pair above is only clean if preprocessing erases the
+    # difference.
+    unlike $out, qr/create_function/, '... with no @generated@ tag false positive';
+
+    $out = qx{$^X $PROG --versions 0.2.1 0.2.2 2>&1};
+    my @gaps = $out =~ /^\S+: TYPE (\S+) predates/mg;
+    is_deeply [sort @gaps], [sort qw(
+        cat_tools.constraint_type cat_tools.procedure_type cat_tools.relation_type
+        cat_tools.relation_relkind cat_tools.object_type
+    )], 'the five enum types that never got GRANT USAGE are flagged';
+
+    # ADP was already in force across this pair, so nothing predates it.
+    $out = qx{$^X $PROG --versions 0.2.2 0.2.3 2>&1};
+    unlike $out, qr/predates/, 'an ADP present in both installs raises nothing';
+}