ci: client-tests job runs all 3 driver suites against live PG

[NikolayS]

Why

client-smoke was effectively a no-op for catching driver bugs:

• It ran only `go test -run TestSmoke ./...` for Go (just `TestSmoke`, not the rest).
• It ran only `pytest test_smoke.py` for Python (just one file).
• For TypeScript it ran `npx tsx src/smoke.ts` (a single hardcoded-DSN script).
• And it did NOT set `PGQUE_TEST_DSN`, so any env-gated integration test silently skipped.

That's exactly how the Nack placeholder bug in PR #79 shipped green: `TestNack` is env-gated on `getDSN()`, CI never set the DSN, the test silently skipped, and the placeholder/arg mismatch slipped past review.

What changes

client-smoke → client-tests. Same job slot, same Postgres-in-docker setup, but now:

• Exports `PGQUE_TEST_DSN` for the whole job
• Installs Go 1.21 and runs `go test -race -v ./clients/go/...`
• Installs Python 3.12 and runs `pytest -v clients/python/tests/`
• Installs Node 20 and runs `npx vitest --run` in `clients/typescript`

The TypeScript step now uses vitest instead of `npx tsx src/smoke.ts`. This also unblocks PR #83 (TypeScript driver overhaul), whose smoke.ts deletion was incompatible with the old workflow.

What's preserved

• The matrix `test (14)..test (18)` jobs (SQL regression tests across PG versions) — untouched.
• The `verify` job — untouched.
• The `claude-review` job — untouched.

Test plan

• `yaml.safe_load` parses cleanly
• `actionlint` clean (only pre-existing SC2034 warnings on the wait loops, unchanged)
• CI run on this PR exercises the new job end-to-end against live PG
• If anything fails (e.g. a Python dep missing in pyproject `[dev]`, a vitest config quirk), iterate — the failure is the actual signal we've been missing

Coordination

• Once this lands, feat(clients/typescript): bring driver to parity with Go (API surface, tests, packaging) #83 (TS driver) becomes mergeable — its deleted `src/smoke.ts` no longer breaks CI.
• The Go bugfix PR (Nack/Connect/panic recovery, in flight) will gain real CI coverage of its red tests.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com

[NikolayS]

REV Review — PR #84

CI:

• Go client tests: FAIL (intentional — see proof point below)
• Python client tests: PASS
• TypeScript client tests: PASS
• verify: PASS
• test (14)..test (18): PASS
• claude-review: PASS

Intentional Go failure (proof point — confirmed)

This branch's Go client tests job fails with exactly the Nack $12 placeholder bug fixed in PR #115. From the run logs (job 73743274273):

2026/04/30 09:19:21 pgque: nack error for unhandled type order.created:
  pgque: nack: ERROR: cannot cast type record to pgque.message (SQLSTATE 42846)
--- FAIL: TestNack (0.03s)
--- FAIL: TestSendAndReceive (0.03s)
--- FAIL: TestConsumerHandlerNacksOnError (3.05s)
FAIL    github.com/NikolayS/pgque/clients/go    3.212s

That is the failure mode we wanted the new harness to surface. Not a setup misconfig — the harness is doing its job. Once PR #115 is on main, this job turns green on main.

Status update on the band-aids (becomes important after this PR's PR-time CI ran)

PR #84's last CI run was at 09:18 UTC. PR #82 merged at 09:30 UTC, PR #115 at 09:32 UTC, PR #83 at 09:34 UTC. So at run time, the Python --ignore flags and TS --passWithNoTests were still appropriate. However, with #82/#83 now on main, those flags are immediately stale:

• clients/python/tests/test_consumer.py and test_smoke.py exist on main and (post-feat(clients/python): bring driver to parity with Go (API, tests, packaging) #82) contain real, now-passing tests. Keeping --ignore=...test_consumer.py --ignore=...test_smoke.py on this PR's CI step silently skips a meaningful portion of the Python suite, including test_smoke.py which is likely the canonical end-to-end smoke. That's a regression of coverage relative to the stated intent of the PR.
• clients/typescript/test/ directory now exists on main post-feat(clients/typescript): bring driver to parity with Go (API surface, tests, packaging) #83, so --passWithNoTests is no longer load-bearing — but it should be removed to reinstate the strict failure mode (zero specs ⇒ red).

Recommend either rebasing this PR on top of current main and dropping the --ignore / --passWithNoTests flags before merge, or merging as-is and tracking removal in a follow-up issue. The former is cleaner because otherwise the band-aids land on main and require a separate PR to reverse.

Blocking findings

None. The Go failure is the proof point, and the band-aids are documented and defensible at the time the CI ran.

Non-blocking findings

1. [MED] Stale --ignore flags after feat(clients/python): bring driver to parity with Go (API, tests, packaging) #82/feat(clients/typescript): bring driver to parity with Go (API surface, tests, packaging) #83 merged. Drop --ignore=clients/python/tests/test_consumer.py and --ignore=clients/python/tests/test_smoke.py from the Python step; drop --passWithNoTests from the TypeScript step. Both are no longer needed and the Python ones now actively mask tests. Either rebase + amend this PR or land a follow-up PR immediately after merge.
2. [LOW] Three near-identical jobs share boilerplate. go-client-tests / python-client-tests / typescript-client-tests each duplicate ~25 lines of docker setup + checkout + build/install. Acceptable for v1 because per-job parallelism is faster than a matrix that fails fast on the first driver, but if the duplication grows it would be worth extracting either a composite action under .github/actions/setup-pgque-pg/ or a matrix with strategy.fail-fast: false. Not blocking.
3. [LOW] Action versions are pinned to majors only. actions/checkout@v4, actions/setup-go@v5, actions/setup-python@v5, actions/setup-node@v4 use floating major tags. Pre-existing convention in this workflow, so consistent — but if the project moves to SHA pinning later, these jobs should follow.
4. [LOW] Service container vs. docker run. The new jobs continue the existing pattern of docker run -d ... postgres:18 instead of GitHub Actions services:. Consistent with the existing test matrix; no objection.
5. [LOW] Job ordering. go-client-tests, python-client-tests, typescript-client-tests are defined before the verify job they needs:. YAML doesn't care, but readers do — consider moving verify above the client-tests jobs in a future cleanup.
6. [INFO] Per-job DSN env var. PGQUE_TEST_DSN is correctly declared at job scope, has no password leak risk (the password pgque_test is a CI-only literal, not a secret), and is reachable by all three drivers' env-gated test guards.
7. [INFO] Multi-line run: blocks. Per CLAUDE.md shell rules, multi-line shell snippets should use set -Eeuo pipefail. The new steps inherit bash (via run: |) without pipefail. This is consistent with the pre-existing steps in the same workflow file, so not a regression introduced by this PR — but a workflow-wide hardening pass would be welcome eventually.

Potential issues to watch

• After this lands, when someone later adds a new Python test that the band-aid path matches (e.g. a new test_smoke.py rewrite), the --ignore will silently skip it. That's the failure mode that originally let the Nack bug ship. Track removal explicitly.
• The Go job runs go test -race -v ./... — good coverage, but -race doubles runtime and may exhaust the 50-second budget on slower runners as more tests are added. Worth keeping an eye on as the test suite grows.

Anti-leak scan

Clean. grep -iE 'gitlab|sahmed|artifact[_-]?registry|@AR\b|wi[ -]?#?7[67]|round[ -]?8|R8\b' against the diff, PR body, and commits returned no matches.

Verdict

READY FOR USER REVIEW — with a strong recommendation to either rebase + drop the now-stale --ignore / --passWithNoTests flags before merge, or open a follow-up issue/PR to remove them immediately afterward (PR #82, #83, #115 all merged after this PR's last CI run, so the workarounds are no longer load-bearing). Go failure mode confirmed as the Nack-bug proof point.

[NikolayS]

Rebase onto main complete (ci/live-pg-driver-tests → fb84bd1)

Base SHA before/after rebase:

• Before: d96817b (merged at fix(pgque.receive): reject max_return < 1 and document batch-ownership #114)
• After: ea6199f (current main tip — includes fix(pgque.nack): canonical + idempotent DLQ terminal handling #116 nack DLQ fix and fix(pgque.receive): finish empty batch instead of stranding consumer #117 empty-batch consumer-stranding fix)

Conflicts:
None. All 5 branch commits touch only .github/workflows/ci.yml; the commits merged onto main since the branch diverged (#116, #117) touched sql/, tests/, and docs/ exclusively. Rebase was clean.

Band-aids check:
The top commit chore(ci): drop stale band-aids post #82/#83/#115 (fb84bd1) is present and removes --passWithNoTests (TypeScript) and the --ignore/skip flags (Python) that were added while those PRs were in-flight. The final ci.yml has none of those workarounds.

Initial CI status (immediately post-push):
All 7 check runs are queued — triggered by the force-with-lease push. verify and test (14–18) queued; go-client-tests, python-client-tests, and typescript-client-tests will start after verify passes (they have needs: verify). The 19 Python + 6 TypeScript failures that showed on the prior run were caused by the empty-batch consumer-stranding bug — now fixed by #117 on main.

---

Generated by Claude Code

[NikolayS]

REV Review — PR #84 (post-rebase, head fb84bd1)

Reviewing the rebased branch on top of current main (which now includes #116 nack/DLQ fix and #117 empty-batch consumer-stranding fix). Scope is the workflow change in .github/workflows/ci.yml. SOC2 perspective skipped per project policy.

CI status (head sha fb84bd1)

Job	Status
verify	PASS
test (14)..test (18)	PASS (5/5)
Go client tests	PASS
Python client tests	FAIL
TypeScript client tests	FAIL
claude-review	PASS

So the per-client jobs did NOT all go green this round — Go yes, Python and TypeScript still red even with #117 on the base. Captured TS assertion fragments from the run:

Error: expected undefined to be defined
Error: expected [] to have a length of 1 but got +0
Error: expected '0' to be '1'
Error: expected 0 to be greater than or equal to 2
Error: expected [] to have a length of 3 but got +0
Error: expected +0 to be 50
Error: expected [] to have a length of 6 but got +0

These are the "I called receive() and got nothing back" / "I expected N rows, got 0" pattern. That's exactly the signature of the empty-batch / tick-not-firing bug class — except #117 is allegedly in the base. So either:

1. There's a remaining empty-batch / dual-filter edge case the drivers hit (e.g. force_tick not called between send and receive, or a Subscriber/subscribe() ordering bug in the driver), or
2. The drivers themselves have bugs the now-running tests legitimately catch (which is the stated goal of this PR), or
3. The tests don't drive pgque.ticker() between send and receive and have always assumed someone else did.

This is workflow-PR territory, not driver-fix territory. The harness is doing its job — surfacing real failures. The fixes belong in the driver PRs, not here. Recommendation below.

---

1. Security

Clean.

• PGQUE_TEST_DSN uses literal pgque_test password — CI-only, never leaves the runner, no secret-store coupling. Correct scoping (job-level env:, not workflow-level).
• All actions on floating major tags (@v4, @v5) — consistent with the existing workflow convention. Not a regression introduced by this PR. SHA pinning would be a separate workflow-wide hardening task.
• No new permissions blocks added. Inherits repo defaults. No write tokens introduced.
• No secret echoing in run: steps. DSN is exported as env var, not interpolated into shell with echo.
• npm ci / npm install step uses if [ -f package-lock.json ] — sensible fallback. Watch supply chain on the install side; consider adding npm ci --ignore-scripts for a future hardening pass. Non-blocking.

2. Bugs (workflow correctness)

Clean.

• Three independent jobs each spin their own postgres-in-docker on localhost:5432 of three different runners — no port conflict (separate VMs). Correct.
• needs: verify on each — good gate; no parallel waste building broken sql.
• if: always() on Cleanup — correct; ensures docker cleanup runs even on test failure.
• PGQUE_TEST_DSN declared at job scope with env:, available to all steps including the env-gated test guards in all three drivers. This is the core fix the PR exists to deliver, and it works — Go's getDSN()-gated tests now run (and pass).
• Wait loop for i in $(seq 1 30); do … && break; sleep 1; done || exit 1 — solid bounded wait.
• One latent concern: nothing in any of the three jobs calls pgque.ticker() or sets up pg_cron. If any test relies on the ticker firing in the background (it must, given snapshot-batching), it depends on the test framework calling force_tick() itself. The Python and TS failures suggest some tests may not be doing this. Not a workflow bug — a test-suite contract issue surfaced by this PR.

3. Test analysis

This is the meat.

Does the new matrix catch what client-smoke missed? Yes, demonstrably. The old workflow ran one go test (TestSmoke), one python file (test_smoke.py), and one tsx script with a hardcoded DSN — and exported no PGQUE_TEST_DSN. The new workflow runs go test -race -v ./..., pytest -v clients/python/tests/, and npx vitest --run with PGQUE_TEST_DSN exported job-wide. The 19 prior Python failures and 6 prior TS failures (pre-#117) are direct evidence the new harness exercises code paths the old one couldn't.

Are tests still silently skipping? Two checks:

• pytest -v clients/python/tests/ — no --ignore flags (post-rebase commit fb84bd1 removed them). Good.
• npx vitest --run — no --passWithNoTests. Good. If clients/typescript/test/ is empty, the job will red. Confirmed test/ directory exists post-feat(clients/typescript): bring driver to parity with Go (API surface, tests, packaging) #83.
• However, individual env-gated tests (e.g. if (!process.env.PGQUE_TEST_DSN) test.skip(...)) will still skip if the DSN env propagation fails. With env: at job level, propagation is correct. Tests that fail with expected [] to have length 1 are clearly running, not skipping.

Does -race matter? Yes — go test -race -v ./... is a meaningful tightening over the old go test -run TestSmoke. Watch runtime budget as the suite grows.

Coverage gap not fixed by this PR: the matrix tests against postgres:18 only for client tests. The SQL test (14)..test (18) matrix already covers PG version drift, so client-side drift is covered indirectly. Acceptable for v0.1 — non-blocking.

4. Guidelines

CLAUDE.md compliance check:

• Conventional commit prefix ci: ✓ (also chore(ci): on the band-aid removal commit ✓)
• Subject lines under 50 chars ✓
• No pgq. references introduced (workflow uses pgque schema and pgque_test DB throughout) ✓
• 2-space indent in YAML and shell snippets ✓
• Multi-line run: blocks do not set set -Eeuo pipefail. CLAUDE.md shell rules call for it, but this is consistent with all pre-existing steps in the same workflow file — not a regression. Non-blocking; suggest a workflow-wide hardening pass later.

Anti-leak scan — clean. Grepped diff, PR body, and the 5 commits for: gitlab|sahmed|@AR\b|hetzner|postgres-ai|round[ -]?[0-9]+|WI[ -]?#?[0-9]+|R8\b. No matches.

5. Docs

• README.md has a CI badge pointing at ci.yml — still works. Job rename client-smoke → three jobs is invisible to the badge (badge tracks workflow result, not job names).
• README.md has no per-job badge for client tests — neutral; the project hasn't promised one.
• No CONTRIBUTING.md in the repo root, and no doc references to client-smoke job name found in the README. Nothing to update. If this PR's intent is to make client-driver coverage visible, a follow-up could add per-driver badges; not required for this PR. Non-blocking.

---

Blocking findings

None. The workflow change itself is correct, well-scoped, and demonstrably catches bugs the prior client-smoke masked. The Python and TS failures are not workflow bugs — they are real driver/test issues that the new harness is supposed to surface.

Non-blocking findings

1. [HIGH-info] Two of three new jobs are red post-rebase. Even with fix(pgque.receive): finish empty batch instead of stranding consumer #117 on the base, Python and TS fail with "expected [].length to be N, got 0" patterns. Recommend triaging these before merge: either (a) confirm they are real driver bugs and open driver-fix PRs to land alongside this one, or (b) confirm they are test-side ticker/force_tick contract issues and fix the tests. Do not merge with two red required jobs unless branch protection is configured to allow it and a tracking issue is open.
2. [LOW] Workflow boilerplate triplication. ~25 lines of docker-up + checkout + build + install pgque are duplicated three times. Acceptable for v1 (parallelism beats matrix fail-fast), but a composite action under .github/actions/setup-pgque-pg/ would dedupe. Defer.
3. [LOW] set -Eeuo pipefail missing in multi-line run: blocks. Pre-existing pattern in the file; addressing here would expand scope. Defer to a workflow-wide hardening PR.
4. [LOW] Action versions on floating majors. @v4/@v5. Pre-existing convention. Move to SHA pinning project-wide if/when supply-chain hardening is done. Defer.
5. [LOW] Job ordering vs. needs:. Client-tests jobs are defined above the verify job they depend on. YAML doesn't care; readers do. Cosmetic.

Verdict

REQUEST_CHANGES_NON_BLOCKING.

The workflow change is sound and lands a real coverage win — that part is approve-worthy. But two of the three new jobs are red on this PR's head, and merging with red required checks should be a deliberate decision (with a tracking issue for each driver), not the default path. Either:

• Land driver-side fixes in companion PRs and rebase this on top so all three turn green, or
• Open issues for the Python and TS failures, link them from this PR's body, and merge with a documented exception.

Either path is fine. The harness change itself is good to go once the failure-triage decision is made.

---

Generated by REV multi-perspective reviewer (Claude Opus 4.7, 1M context). Self-approval blocked; this is an advisory comment, not a formal review approval.

---

Generated by Claude Code

[NikolayS]

Investigated the Python and TypeScript failures locally against PG 16 with PGQUE_TEST_DSN set. Filed two follow-up PRs that this PR depends on:

• fix(clients/python): make Consumer thread-safe and correct test xact pattern #129 — fix(clients/python): make Consumer thread-safe and correct test xact pattern
• fix(clients/typescript): per-queue ticker overload, fix nack test schema, serialise vitest #130 — fix(clients/typescript): make forceTick rotate, fix nack test schema, serialise

Root causes

1. Python (driver): Consumer.start() registered SIGTERM/SIGINT handlers from any thread; signal.signal() raises ValueError outside the main thread, so any consumer started in a worker thread (the test pattern, and a perfectly normal embedded use) crashed before the poll loop. Fix: skip signal registration when not in the main thread.
2. Python (tests): send → force_tick → ticker → commit ran inside a single transaction. The ticker's snapshot excludes the producing xact's writes, so the resulting tick references no events and receive() returns []. Fix: commit between client.send() and force_tick. Also: test_smoke was missing create_queue(); test_nack_routes_to_dlq_at_max_retries mutated Message.retry_count client-side, but pgque.nack() reads ev_retry from the canonical batch row (per Bug: nack() DLQ path trusts caller-supplied pgque.message and allows forged DLQ rows #98) and ignores that, so the assertion was unreachable — set queue_max_retries=0 instead.
3. TS (driver): Client.forceTick() ran pgque.force_tick($1) only. force_tick() bumps the event sequence; you still need pgque.ticker() to actually rotate. Every test using the helper got an empty batch. Fix: forceTick() now runs both, on separate pool connections.
4. TS (tests): nack.test.ts queried a non-existent column ev_retry_after_ts (real column is ev_retry_after). Plus, vitest ran the three suites in parallel forks against a single shared DB, racing on queue lifecycle (drop_queue / create_queue) — set fileParallelism: false.

Local results

• Python: pytest -v clients/python/tests/ → 39 passed
• TypeScript: npx vitest --run → 22 passed (5/5 stable runs)
• Go: still green

Once #129 and #130 land, rebasing this PR should turn all three driver jobs green.

---

Generated by Claude Code

[NikolayS]

Rebase onto main (post-#129, #130)

Base before	ea6199fb
Base after	714007e8 (tip of main after #129 + #130 merged)
Old head	fb84bd15
New head	02bc199f
Conflicts	None — clean rebase, 5 commits replayed

CI results (02bc199f):

Job	Result
test (14)	green
test (15)	green
test (16)	green
test (17)	green
test (18)	green
verify	green
Python client tests	green
TypeScript client tests	green
Go client tests	FAILED

Failure URL: https://github.com/NikolayS/pgque/actions/runs/25172193888/job/73794456363

Python (#129) and TypeScript (#130) driver fixes were both in main and took effect. The Go client test job still fails — no corresponding Go driver fix has landed on main yet.

---

Generated by Claude Code

[NikolayS]

Closing as stale/conflicting. Replaced by #174, a clean current-main PR that splits client checks into per-driver jobs and runs full live-PG suites.