feat: support PostgreSQL 19 beta 2

[NikolayS]

Summary

• release postgres_dba 2026.8.1 with exact PostgreSQL 19 beta 2 coverage
• execute all 32 non-interactive reports with ON_ERROR_STOP as a non-superuser pg_monitor member with the required object grants
• preload pg_stat_statements and functionally test both interactive role reports
• verify generated credentials with password authentication, password rotation, and old-password rejection
• retain PostgreSQL 13–18 coverage, assert every matrix server version, and harden container isolation/cleanup
• preserve the cumulative 7.0 release history and document the move to CalVer

Verification

• PostgreSQL 13, 14, 15, 16, 17, 18, and exact 19beta2 matrix jobs
• 32 non-interactive reports with SQL errors promoted to job failures
• create-role and alter-role workflows with role state, verifier, and authentication assertions
• 4 targeted regression assertions
• local exact-PG19beta2 authentication and rotation probe
• actionlint 1.7.7, YAML parser, and git diff --check

Relates to #96

[tembo]

The wide and normal mode tests above use if ! output=$(... 2>&1) to check the psql exit code and capture stderr. The four regression tests below don't follow the same pattern — they only check the output content and ignore the exit code. For three of the four tests, stderr isn't captured either, so any psql error message won't appear in the failure output, making CI failures harder to debug.

Consider wrapping these in the same if ! pattern so SQL errors (caught by ON_ERROR_STOP) surface explicitly. Same applies to the x1_alignment_padding, a1_activity, and i3_non_indexed_fks tests below.

Suggested change

	output=$(PGOPTIONS='-c postgres_dba.wide=off' PAGER='cat' psql \
	--no-psqlrc \
	--set=ON_ERROR_STOP=1 \
	--set=postgres_dba_interactive_mode=false \
	--host=localhost \
	--username=dba_user \
	--dbname=test \
	--file=warmup.psql \
	--file=sql/0_node.sql)
	if [[ "${output}" != *Primary* ]]; then
	echo "Role test failed: ${output}" >&2
	exit 1
	fi
	if ! output=$(PGOPTIONS='-c postgres_dba.wide=off' PAGER='cat' psql \
	--no-psqlrc \
	--set=ON_ERROR_STOP=1 \
	--set=postgres_dba_interactive_mode=false \
	--host=localhost \
	--username=dba_user \
	--dbname=test \
	--file=warmup.psql \
	--file=sql/0_node.sql 2>&1); then
	echo "Role test failed: ${output}" >&2
	exit 1
	fi
	if [[ "${output}" != *Primary* ]]; then
	echo "Role test failed: ${output}" >&2
	exit 1
	fi

[NikolayS]

samorev Code Review Report

• PR: feat: support PostgreSQL 19 beta 2 #97 - feat: support PostgreSQL 19 beta 2
• Author: @NikolayS
• AI-Assisted: Yes

Pipeline	Coverage
FAIL	Not reported

---

BLOCKING ISSUES (11)

CRITICAL CI/Pipeline - Pipeline status is failure

Provider CI reported status failure.
Fix: Fix failing checks and rerun review.

HIGH [bugs] Wide mode is almost certainly not enabled — the psql variable postgres_dba_wide was replaced with a server-side GUC postgres_dba.wide, which no report can read. The "wide" and "normal" steps then run byte-identical tests, so wide-mode coverage (and the RELEASE_NOTES claim of fixing it) is illusory. The smoking gun is the inconsistency inside the same command: postgres_dba_interactive_mode is still passed as a psql variable via --set, while wide moved to PGOPTIONS; the name also changed (_ → .) and the values changed (true/false → on/off). A custom dotted GUC is accepted silently by the server as a placeholder, so nothing errors — it just does nothing.

if ! output=$(PGOPTIONS='-c postgres_dba.wide=on' PAGER='cat' psql --no-psqlrc --set=ON_ERROR_STOP=1 --set=postgres_dba_interactive_mode=false ... (wide step) vs the same command with postgres_dba.wide=off (normal step); removed lines echo "\set postgres_dba_wide true" > ~/.psqlrc
Fix: Drop PGOPTIONS and pass the toggle the same way as interactive mode: --set=postgres_dba_wide=true in the wide step and --set=postgres_dba_wide=false in the normal/regression steps. To prove the two modes actually differ, assert that a known wide-only column appears in wide output and is absent in normal output — otherwise this regression can silently return. (The original ~/.psqlrc approach was indeed dead because every psql call passes --no-psqlrc; that part of the diagnosis is correct, only the remedy is wrong.)

MEDIUM [bugs] In the i3 regression test, psql's stderr is captured into $output and the assignment is unguarded. GitHub Actions runs run: blocks under bash -e, so when ON_ERROR_STOP=1 makes psql exit non-zero the step aborts at the assignment and $output is never printed — a SQL failure in i3 produces a red job with zero diagnostics. The wide/normal loops avoid this by wrapping in if ! output=$(...); this step does not.

output=$(PGOPTIONS='-c postgres_dba.wide=off' PAGER='cat' psql ... --file=sql/i3_non_indexed_fks.sql 2>&1) followed by if [[ "${output}" != *fk_child* || ... ]]
Fix: Guard it like the loops: if ! output=$(psql ... 2>&1); then echo "${output}" >&2; exit 1; fi, then run the content assertions. Apply the same pattern to the 0_node / x1 / a1 assignments so a psql failure prints its stderr before exiting.

MEDIUM [security] The failure-path redaction for the role reports is a guessed regex. sed -E 's/(password: ).*/\1[redacted]/' only matches a lowercase, literal password: prefix on a single line. postgres_dba's r1/r2 emit the generated password inside psql output (a result table / notice), which will not match that pattern — so on any failure the plaintext generated password is written verbatim to a public CI log. The step also pulls pg_authid.rolpassword hashes into shell variables, which land in the log if anyone enables step debug (ACTIONS_STEP_DEBUG) or adds set -x.

echo "${output}" | sed -E 's/(password: ).*/\1[redacted]/' >&2 (both the r1 and r2 failure branches)
Fix: Don't rely on pattern-matching the report's prose. Either redact by whitelist (print only lines you know are safe), or avoid the leak entirely: compare md5(rolpassword) server-side and never bring the hash to the shell, e.g. select rolpassword is distinct from :'old_hash' computed in SQL, or store md5(rolpassword) in the shell vars instead of the SCRAM verifier. At minimum make the redaction case-insensitive and anchor-free (sed -E 's/[Pp]assword[^[:alnum:]]*[^[:space:]]+/password [redacted]/g') and mask the captured hashes with ::add-mask::.

MEDIUM [docs] RELEASE_NOTES.md is rewritten in place rather than prepended to — the entire 7.0 section (~70 lines) is deleted. That erases the only record of the c1–c4 corruption checks, m1/s3/t2, the b6→m1 / c1→p1 / p1→x1 renames, and the s1/s2/i3/m1/i2 bug fixes. The rename table in particular is migration-critical for existing users, and this PR (a PG19 compatibility + CI change) has no reason to remove it.

-# postgres_dba 7.0 … -- Added PAGER=cat to prevent pager hangs replaced wholesale by +# postgres_dba 2026.8.1
Fix: Keep RELEASE_NOTES.md cumulative: add the 2026.8.1 section above the existing 7.0 section and restore the deleted content unchanged.

LOW [docs] The version scheme silently changes from semver (7.0) to CalVer (2026.8.1) with no explanation anywhere, and no other file is updated to match. README and CLAUDE.md were touched in this PR for the version matrix but say nothing about the release identifier, so any reader mapping "7.0" to a release now finds nothing.

-# postgres_dba 7.0 → +# postgres_dba 2026.8.1; README/CLAUDE.md diffs change only the PG version range
Fix: Either keep semver (7.1) for a CI/compat release, or state the scheme switch explicitly in the release notes and update any other place carrying the version.

LOW [tests] The server-version assertion is asymmetric — it only fires for 19beta2. For 13–18 the job prints the version and accepts anything, so a wrong/floating image tag (e.g. postgres:17 silently serving a different major, or a tag that resolves unexpectedly) passes unnoticed. The new assertion exists precisely because that class of mistake matters.

if [[ '${{ matrix.postgres-version }}' == '19beta2' && "${server_version}" != 19beta2* ]]; then
Fix: Assert for every matrix entry: if [[ "${server_version}" != "${{ matrix.postgres-version }}"* ]]; then echo "Expected PostgreSQL ${{ matrix.postgres-version }}, got ${server_version}" >&2; exit 1; fi — the prefix match already works for both 18.x and 19beta2.

LOW [tests] The regression assertions were weakened when | grep <field> was dropped: the substring match now runs against the whole report output rather than the targeted line. [[ "${output}" != *User* ]] for a1_activity is tautological — "User" is a column header in that report, so the check passes even if the report returns zero rows or the wrong data. *Primary* for 0_node can likewise match anywhere in the output rather than the Role line.

output=$(... --file=sql/a1_activity.sql) / if [[ "${output}" != *User* ]]; then
Fix: Restore line-scoping and assert on the value, not the label — e.g. role_line=$(printf '%s\n' "${output}" | grep -E '^\s*Role') then check it contains Primary; for a1, assert on a row that must exist (e.g. the current dba_user session) rather than the header.

LOW [tests] The new r1 test asserts only that rolpassword is not null, never that the password the report printed actually works. If r1 generated one password, displayed it, and stored a different one (the exact failure mode worth covering for a "create user with random password" report), the test still passes. Same for r2: it asserts the stored hash changed, not that the newly reported password authenticates.

if [[ "${role_state}" != 'f|t|t' ]]; then and || "${old_password}" == "${new_password}"
Fix: Capture the generated password from the report output and assert it authenticates, e.g. PGPASSWORD="${pw}" psql --host=localhost --username=ci_test_role --dbname=test -c 'select 1' (requires an md5/scram entry rather than trust for that role, or a second listener); keep the password out of the log via ::add-mask::.

LOW [bugs] The hand-rolled container has a fixed name and no teardown. Nothing stops or removes postgres-dba-test, and there is no if: always() cleanup step, so a leaked container blocks every subsequent job on a self-hosted or reused runner with "name already in use" — and docker run failing that way aborts the job before any test runs. The previous services: block was managed and cleaned up by the runner.

docker run --detach --name postgres-dba-test ... with no corresponding docker rm
Fix: Add --rm where possible and a trailing - name: Stop PostgreSQL step with if: always() running docker rm --force postgres-dba-test, or make the name unique per job (postgres-dba-test-${{ matrix.postgres-version }}).

LOW [security] The container publishes a trust-authenticated superuser Postgres on all interfaces of the runner (--publish 5432:5432 binds 0.0.0.0), combined with POSTGRES_HOST_AUTH_METHOD=trust. On a shared or self-hosted runner any co-located process or network peer gets unauthenticated superuser access for the job's lifetime. The rewrite from services: is the natural point to tighten this.

--env POSTGRES_HOST_AUTH_METHOD=trust \ --publish 5432:5432 \
Fix: Bind loopback only: --publish 127.0.0.1:5432:5432. Tests already connect via --host=localhost, so nothing else changes.

---

Summary

Area	Findings	Potential	Filtered
CI/Pipeline	1	0	0
Security	0	2	0
Bugs	0	3	0
Tests	0	3	0
Guidelines	0	0	0
Docs	1	1	0
Metadata	0	0	0

Note:

• Findings: High-confidence issues (8-10/10) - blocking or non-blocking per severity
• Potential: Medium-confidence issues (4-7/10) - review manually
• Filtered: Low-confidence issues (0-3/10) - excluded as likely false positives

Review metadata

provider=github
kind=pr
project=NikolayS/postgres_dba
number=97
target=github:NikolayS/postgres_dba#97
state=OPEN
draft=false
diff_lines=530
diff_added=266
diff_removed=171
diff_bytes=23024
comments_count=0
commits_count=1
ci_status=failure
ci_summary=total=10 success=7 failure=1 pending=0 other=2
prompt=.claude/commands/review-mr.md
blocking=true
posted_by=gh
no_comment=false
live_posting=posted

---

samorev-assisted review (AI analysis by Tanya301/samorev)

[tembo]

This step only exercises normal mode. warmup.psql reads postgres_dba.wide as a server-side GUC (current_setting('postgres_dba.wide', true)), not a psql variable, so --set won't enable wide mode. Consider wrapping this loop in an outer for mode in normal wide iteration and setting PGOPTIONS='-c postgres_dba.wide=on' for the wide pass to catch wide-mode regressions in CI.

[NikolayS]

samorev Code Review Report

• PR: NikolayS/postgres_dba#97 — feat: support PostgreSQL 19 beta 2
• Author: @NikolayS
• Branch: release-2026.8.1-pg19beta2 (HEAD e7517b8)
• AI-Assisted: Yes
• Re-review: yes — 3 new commits (fd42084, b47e2a7, e7517b8) landed after the 2026-08-13 review

CI: test (13 … 19beta2) — all 7 matrix jobs pass. claude-review — fail, but the cause is
Either ANTHROPIC_API_KEY, CLAUDE_CODE_OAUTH_TOKEN, ... is required (empty repo secret in
.github/workflows/claude-code-review.yml, a file this PR does not touch; it has failed on every commit
of this PR including the first). Treated as environmental, not a defect in this change.

---

Prior review reconciliation

Verified as genuinely fixed: loopback-only publish, if: always() cleanup, per-matrix version assertion,
RELEASE_NOTES 7.0 history (confirmed byte-identical and preserved as a suffix), line-scoped and
value-based regression assertions, real r1/r2 functional tests with password auth + rotation +
old-password rejection, ::add-mask::.

Two prior findings were wrong and are withdrawn:

• The prior HIGH on wide mode claimed PGOPTIONS='-c postgres_dba.wide=on' was the wrong mechanism.
  It was the right one — warmup.psql:29 reads the server GUC via
  current_setting('postgres_dba.wide', true) = 'on'. Moot regardless: postgres_dba_wide is consumed
  by no report in sql/, so wide mode is inert and dropping the wide step loses nothing.
• The prior MEDIUM on password redaction (sed on password: ) is obsolete — the redaction approach was
  replaced with capture + ::add-mask::, and mask ordering is correct on every path.

---

BLOCKING ISSUES (1)

MEDIUM .github/workflows/test.yml:305 — unguarded grep in a command substitution under bash -e makes the error handler unreachable (confidence: 9/10)

role_line=$(printf '%s\n' "${output}" | grep -E '^[[:space:]]*Role[[:space:]]*\|')
if [[ "${role_line}" != *Primary* ]]; then
  echo "Role test failed: ${output}" >&2

run: blocks execute under bash --noprofile --norc -e -o pipefail. If 0_node.sql ever stops emitting
a Role row (catalog change on a new PG major, output-format change, empty result), grep exits 1, the
assignment inherits that status, and set -e kills the step at line 305 — so lines 306-309, the only
place the actual report output is printed, never run. The job dies with a bare
"Process completed with exit code 1" and zero evidence, on the newest-PG-major failure mode this PR exists
to catch. Verified locally: the equivalent snippet exits 1 without reaching the handler.
This is the one remaining instance of the exact pattern this PR fixed in the four psql call sites.
Fix: role_line=$(... | grep -E '...' || true), then test [[ -z "${role_line}" || "${role_line}" != *Primary* ]].

---

NON-BLOCKING (4)

LOW .github/workflows/test.yml:277-286 — old-password rejection passes on any nonzero psql exit (confidence: 8/10)

>/dev/null 2>&1 discards the reason, so connection-refused, a dead server, a no pg_hba.conf entry
misconfiguration, or a mistyped flag all satisfy the assertion identically to a real auth rejection.
This is the single check protecting the release notes' "rejection of the previous password" claim.
Suggestion: capture stderr and require *'password authentication failed'*, failing with the captured
text otherwise. Both passwords are already ::add-mask::-ed by this point, so printing it is safe.

LOW .github/workflows/test.yml:371 — if: always() cleanup destroys the server log before it can be read (confidence: 8/10)

docker logs postgres-dba-test is invoked in exactly one place (line 52, the readiness-timeout path).
Any later failure — report error, backend crash, auth problem — is followed immediately by
docker rm --force, discarding all server-side ERROR/FATAL/PANIC lines and leaving only psql's
client-side message. Most costly for 19beta2, where server-side breakage is the likely failure mode.
Suggestion: add a preceding if: failure() step running docker logs postgres-dba-test, or make the
cleanup docker logs postgres-dba-test || true before the docker rm.

LOW RELEASE_NOTES.md:8 — "pg_monitor-only user" overstates the privileges actually exercised (confidence: 8/10)

dba_user is granted pg_monitor plus connect, usage on schema public, and
grant select on all tables in schema public (test.yml:111-115). pg_monitor alone does not grant SELECT
on user tables, so those extra grants are load-bearing for x1/i3/a1 to run at all. Flagged independently by
two reviewers. The grants are unchanged from the pre-PR workflow — only the claim is new.
Suggestion: drop "-only", e.g. "with a non-superuser pg_monitor member holding the minimal
CONNECT/USAGE/SELECT grants the reports require".

INFO .github/workflows/test.yml:107-111 — inconsistent SQL formatting inside the new heredoc (confidence: 8/10)

insert into fk_child splits select / from onto their own root-keyword lines (matching align1/align2),
but the adjacent insert into fk_parent keeps select ... from ... on one line despite identical structure.
Suggestion: put from generate_series(1, 100000) i; on its own line, per the SQL style guide's
"root keywords on their own line".

---

POTENTIAL ISSUES (8)

Moderate confidence (4-7/10) — review manually, some may be acceptable trade-offs.

MEDIUM .github/workflows/test.yml:189-197 — sh -c pg_hba rewrite has no set -e; a failed cat clobbers the file (confidence: 6/10)

{ printf ...; cat "${hba}"; } > "${hba}.new" then mv. If cat fails (unset/changed PGDATA, permissions,
a future image layout), ${hba}.new already exists containing only the one md5 line, and mv installs it as
the entire pg_hba.conf. mv succeeds, so docker exec exits 0 and the breakage surfaces later as a
confusing no pg_hba.conf entry for host ... user "postgres".
Suggestion: set -eu at the top of the sh -c script, plus a [ -s "${hba}.new" ] guard before mv.

MEDIUM .github/workflows/test.yml:126-141 — the 32-report loop asserts only that psql exited 0 (confidence: 6/10)

Most relevant to s1/s2/s3, whose real execution via the newly added
shared_preload_libraries=pg_stat_statements is a headline claim of this release.
s1_pg_stat_statements_top_total.sql is a bare aggregate with no GROUP BY, so it returns one row of NULLs
and exits 0 whether or not the view captured anything — a PG19 column/catalog rename or a preload that
silently didn't take effect would pass CI.
Suggestion: for the pg_stat_statements-backed reports, assert on content (nonzero calls / non-empty row),
not just exit status.

MEDIUM .github/workflows/test.yml:152, 226 — r1/r2 exercised with exactly one input combination (confidence: 6/10)

Both use printf 'ci_test_role\n0\n1\n' (not-superuser, login). The is_superuser=1 and login=0 branches
that append superuser / login to the generated SQL, and the empty-username exception path, are never
executed. A broken or inverted superuser branch would not be caught by a PR whose stated goal is to
"functionally test both interactive role reports".
Suggestion: add one more invocation with is_superuser=1 and/or login=0 and assert pg_authid reflects it.

MEDIUM .github/workflows/test.yml:226-247 — r2 never re-checks rolsuper/rolcanlogin after ALTER (confidence: 5/10)

r1 checks rolsuper, rolcanlogin, rolpassword is not null (line 173-187); r2 checks only rotation and
connectivity. alter_user_with_random_password.psql re-applies both flags on every run, so a regression that
always grants superuser or drops LOGIN would go undetected — the connectivity check only proves the role can
still log in.
Suggestion: mirror the r1 pg_authid assertion after the r2 run.

LOW .github/workflows/test.yml:198-212 — pg_reload_conf() is not awaited, so the positive auth assertion can be vacuous (confidence: 6/10)

pg_reload_conf() only signals the postmaster. If the reload hasn't landed when the next connection opens,
that connection still matches the entrypoint's trust rule and succeeds regardless of the password — proving
nothing about md5/SCRAM. Blast radius is limited: the later negative test would catch a rewrite that never took
effect, so the realistic symptom is flakiness rather than a silent pass. Flagged independently by two reviewers.
Suggestion: after the reload, poll until a deliberately wrong password is rejected, then run the positive check.

LOW .github/workflows/test.yml:65 — version guard is an unanchored prefix match (confidence: 5/10)

[[ "${server_version}" != '<matrix>'* ]] pins only the major for 13-18: '18' would accept 18beta1,
18rc1, or 18devel. 19beta2 is the only entry where the prefix is the complete string — so the natural
follow-up (flipping '19beta2' to '19' at GA) silently reintroduces the gap.
Suggestion: compare server_version_num / 10000 against the numeric major, and require an exact match for
pre-release entries.

MEDIUM roles/create_user_with_random_password.psql:43, roles/alter_user_with_random_password.psql — passwords generated with non-cryptographic random() (confidence: 6/10)

Pre-existing; not introduced by this PR and outside its diff. Raised because this PR is what adds CI that
certifies both generators and advertises "verify generated credentials" — the new tests validate mechanics,
not entropy. PostgreSQL's random() is documented as not cryptographically secure; the per-backend PRNG is
seeded from PID XOR session start time, so the nominal ~92-bit password has far less real entropy. Same line:
int4() rounds rather than truncates, so j can reach 54 (out of range → wasted iteration) and j=0 occurs
at half rate.
Suggestion: track separately from this PR. gen_random_uuid() (PG13+, backed by pg_strong_random) or
pgcrypto's gen_random_bytes(); use floor() instead of int4().

INFO .github/workflows/test.yml:130, 189, 227 — three load-bearing, non-obvious choices carry no comment (confidence: 6/10)

(a) line 227's PGOPTIONS='-c client_min_messages=debug' is required — alter_user_with_random_password.psql
emits the new password via raise debug, not raise info; without it the extraction silently finds nothing.
(b) line 189's pg_hba md5 rule is what makes the auth assertions meaningful at all, since the container runs
POSTGRES_HOST_AUTH_METHOD=trust — and md5 specifically, because it is the only method that works for both
PG13 (md5) and PG14+ (SCRAM, auto-negotiated). (c) line 130's r1_*/r2_* skip is unexplained at the point it appears.
Suggestion: one-line comment at each.

---

Summary

Area	Findings	Potential	Filtered
CI	0	0	1
Security	1	2	0
Bugs	2	2	2
Tests	0	3	0
Guidelines	1	0	1
Docs	1	1	0
Total	5	8	4

Findings = 5 → 1 blocking + 4 non-blocking, matching the section headers above.

Filtered (checked and ruled out, not reported as issues):

• claude-review CI failure — environmental (empty repo secret), not introduced by this PR.
• docker rm --force on a missing container was suspected of reddening the if: always() step —
  empirically verified to exit 0 on modern Docker, so it does not.
• Wide-mode coverage "loss" — postgres_dba_wide is consumed by no report; the step was inert.
• RELEASE_NOTES.md:3 "13–19beta2" vs line 7 "19 beta 2" — the compact form matches the existing 7.0
  summary line (line 32, "13–18"), so it follows convention rather than breaking it.

Also verified clean and deliberately not flagged: mask ordering on every path; no pg_stat_statements
leak of the CREATE/ALTER ROLE text (nested dynamic SQL in a DO block, and
pg_stat_statements.track defaults to top); no ${{ }} injection path (matrix values are file-local
literals); heredoc de-indentation; the readiness loop; md5 correctness across PG13 vs PG14+; and the
x1/a1 regression assertions, which are genuinely load-bearing rather than tautological.

Result: FAILED — 1 blocking issue.

---

samorev-assisted review (AI analysis by postgres-ai/rev)

[NikolayS]

samorev Code Review Report

• PR: NikolayS/postgres_dba#97 — feat: support PostgreSQL 19 beta 2
• Author: @NikolayS
• Branch: release-2026.8.1-pg19beta2 (HEAD a2bba1e)
• AI-Assisted: Yes
• Re-review: yes — a2bba1e landed after the 2026-08-14 03:59 review, so this pass re-reviews that delta plus the full change (per PRINCIPLES pg_stat_statements report for older (<9.5) Postgres versions #1: a fix commit invalidates the prior approval)

CI: test (13 … 19beta2) — all 7 matrix jobs pass. claude-review — fail, cause is
Either ANTHROPIC_API_KEY, CLAUDE_CODE_OAUTH_TOKEN, ... is required (empty repo secret consumed by
.github/workflows/claude-code-review.yml, a file this PR does not touch). It failed identically on the
PR's first commit 2b4493c. Environmental/repo-config, not a defect in this change — not counted as blocking,
but the empty secret is worth fixing separately.

---

Prior review reconciliation

Every finding from the previous pass is fixed in a2bba1e, verified against the file at HEAD:

Prior finding	Status
MEDIUM unguarded grep makes the role-test error handler unreachable	fixed — || true + -z guard (L373-379); pipefail no longer kills the step before the diagnostic prints
LOW old-password rejection passed on any nonzero psql exit	fixed — stderr captured, password authentication failed for user "ci_test_role" now required (L340-354)
LOW if: always() cleanup destroyed the server log	fixed — Print PostgreSQL logs on failure step added ahead of cleanup (L439-441)
LOW RELEASE_NOTES "pg_monitor-only user" overstated privileges	fixed — reworded to "non-superuser pg_monitor member holding the minimal … grants"
INFO inconsistent SQL formatting in the fk_parent insert	fixed — from moved to its own root-keyword line (L113-115)
potential sh -c HBA rewrite had no set -e	fixed — set -eu + test -s guard (L212-222)
potential r2 never re-checked rolsuper/rolcanlogin	fixed — altered_role_state assertion added (L299-313); see NON-BLOCKING note below on what it actually proves
potential pg_stat_statements reports only asserted exit 0	substantially fixed — pgss_rows == 0 assertion proves the view is loaded and populated (L133-145)

Also independently re-verified: postgres_dba_wide is set in warmup.psql:29 and consumed by no file in
sql/, so dropping the wide-mode CI step loses no coverage. Conversely, the old ~/.psqlrc +
--no-psqlrc combination meant postgres_dba_interactive_mode was never actually applied before this PR —
--set=postgres_dba_interactive_mode=false is a real fix, not just a rewrite. 34 reports − r1/r2 = the 32
non-interactive reports claimed. RELEASE_NOTES 7.0 history preserved verbatim.

---

NON-BLOCKING (2)

LOW .github/workflows/test.yml:247-250 — auth-rule readiness loop discards its own diagnostic (confidence: 8/10)

if auth_error=$(PGPASSWORD='deliberately-wrong' psql ... 2>&1); then :
elif [[ "${auth_error}" == *'password authentication failed for user "ci_test_role"'* ]]; then
...
if [[ "${auth_rule_ready}" != true ]]; then
  echo 'Password-authentication rule did not become active' >&2

${auth_error} holds psql's real message and is then thrown away. If the probe never sees the expected
text — HBA rewrite landed wrong, no pg_hba.conf entry, server gone, or a future PG dropping md5
support — the step dies after 3 s with one context-free line. There is no secret to protect here:
the password is the literal deliberately-wrong, and both generated passwords are ::add-mask::-ed
by this point. This is the one path in the new step that still swallows its diagnostic, in the commit
titled "preserve CI failure diagnostics".
Suggestion: echo "${auth_error}" >&2 before the exit 1.

INFO PR description — still carries the "pg_monitor-only" wording that a2bba1e corrected (confidence: 8/10)

Bullet 2 reads "execute all 32 non-interactive reports with ON_ERROR_STOP as a pg_monitor-only user".
RELEASE_NOTES was changed in a2bba1e to drop "-only" precisely because dba_user also holds
connect, usage on schema public, and select on all tables (test.yml:93-95, 122) — load-bearing
grants without which x1/i3/a1 cannot run.
Suggestion: mirror the RELEASE_NOTES wording in the PR body so the merge commit message is accurate.

---

POTENTIAL ISSUES (2)

Moderate confidence (4-7/10) — review manually, may be acceptable trade-offs.

LOW .github/workflows/test.yml:65-74 — the beta version assertion depends on the Debian vendor suffix (confidence: 7/10)

else expected_prefix="${expected_version} "   # note the trailing space

Verified locally on postgres:19beta2: show server_version returns
19beta2 (Debian 19~beta2-1.pgdg13+1) — the trailing space matches only because that vendor suffix
exists. An image that reports a bare 19beta2 (different base, or a packaging change between beta2 and
GA) makes this assertion fail on a correct server, turning the "assert every matrix job runs its expected
version" guard into a false alarm on the one matrix entry it was added for. The numeric branch (13.)
has no such dependency — it genuinely needs the dot to avoid matching 130.x.
Suggestion: accept an exact match too, e.g.
if [[ "${server_version}" != "${expected_version}" && "${server_version}" != "${expected_prefix}"* ]].

LOW .github/workflows/test.yml:174, 273, 299-313 — r1/r2 exercised with one input combination, and the r2 flag assertion is degenerate (confidence: 7/10)

Both reports are driven with printf 'ci_test_role\n0\n1\n' (not-superuser, login). Consequence for r2,
verified against roles/alter_user_with_random_password.psql:49-58: is_superuser=0 makes the superuser
case yield an empty suffix, so r2 executes alter role ci_test_role — confirmed on PG19beta2 to be a
valid no-op, not an error. login=1 then re-applies LOGIN, which r1 already set. So the new
altered_role_state != 'f|t' check passes without either flag branch having done anything: an inverted or
broken superuser branch would still read f|t. r1's superuser branch and its empty-username raise exception path are likewise never reached.
Suggestion: add one more r2 invocation with is_superuser=1 / login=0 and assert pg_authid
flips accordingly — that is what makes the altered_role_state assertion load-bearing.

---

Summary

Area	Findings	Potential	Filtered
CI	0 (1 environmental)	0	1
Security	0	0	0
Bugs	0	1	2
Tests	1	1	0
Guidelines	0	0	0
Docs	1	0	0

Filtered (checked, disproven — recorded so they are not re-raised):

• docker rm --force on a missing container was suspected to fail the if: always() cleanup step; measured
  exit status is 0, so the unguarded docker rm is correct as written.
• alter role <name> with no options was suspected to be a syntax error in the is_superuser=0 path;
  confirmed on PG19beta2 to be accepted as a no-op.
• The show server_version probe (L56-63) omits ON_ERROR_STOP unlike every other psql call, but an error
  there yields an empty server_version that the very next prefix check rejects loudly — self-correcting.

No blocking issues. Test-quality gaps only; the change is sound, and the PostgreSQL 19 beta 2 coverage it
claims is real and verified by 7/7 green matrix jobs.

Result: PASSED

---

samorev-assisted review (AI analysis by postgres-ai/rev)

[tembo]

If the psql query fails (e.g., pg_stat_statements view inaccessible), pgss_rows will contain the error message text instead of a number. Bash then evaluates (( pgss_rows == 0 )) as a syntax error (exit code 1 = false), so the if block is skipped entirely — the health check is silently bypassed and the test continues without verifying pg_stat_statements is working.

Wrapping the command substitution in an if ! guard catches the psql failure explicitly:

Suggested change

	pgss_rows=$(PAGER='cat' psql \
	--no-psqlrc \
	--set=ON_ERROR_STOP=1 \
	--host=localhost \
	--username=dba_user \
	--dbname=test \
	--tuples-only \
	--no-align \
	--command='select count(*) from pg_stat_statements;')
	if (( pgss_rows == 0 )); then
	echo 'pg_stat_statements did not capture any statements' >&2
	exit 1
	fi
	if ! pgss_rows=$(PAGER='cat' psql \
	--no-psqlrc \
	--set=ON_ERROR_STOP=1 \
	--host=localhost \
	--username=dba_user \
	--dbname=test \
	--tuples-only \
	--no-align \
	--command='select count(*) from pg_stat_statements;'); then
	echo 'pg_stat_statements query failed' >&2
	exit 1
	fi
	if (( pgss_rows == 0 )); then
	echo 'pg_stat_statements did not capture any statements' >&2
	exit 1
	fi