fix(send-idem): reject non-finite dedup ttl

[NikolayS]

Summary

pgque.send_idem() validated the dedup TTL with only a > 0 check. On
PostgreSQL 17+, interval 'infinity' passes that check, creating a dedup
key that never expires and is never reaped by maint_idem — unbounded
pgque.idem table growth.

This adds an isfinite() guard to the TTL validation (same pattern used by
claim_slot's lease TTL). All send_idem overloads reduce to the text
overload, so the single guard covers both the text and jsonb variants.

The guard makes the documented contract — a dedup TTL must be a positive
finite interval — actually enforced.

Change

• devel/sql/pgque-api/send_idem.sql: guard becomes
  i_ttl is null or i_ttl <= interval '0' or not isfinite(i_ttl), with a
  clear message: dedup ttl must be a positive finite interval, got %.
• Regenerated the devel assembly (devel/sql/pgque.sql, pgque-tle.sql).
• tests/test_send_idem.sql: new red/green test. Gated to PG 17+, where
  infinite intervals exist (older servers reject the literal at parse time).

Validation

Fresh install of devel/sql/pgque.sql on PostgreSQL 18.3.

Red (before the fix): the new test fails — the infinite TTL is accepted.

NOTICE: PASS: jsonb overload dedups like the text overload
ERROR: infinite dedup ttl must be rejected naming the requirement, got: send_idem(infinity ttl) should fail

Green (after the fix):

NOTICE: PASS: send_idem rejects a non-finite dedup ttl

Both suites green on a fresh install:

• tests/test_send_idem.sql — 13/13 PASS
• tests/acceptance/us13_producer_idempotency.sql — US-13: PASSED

Commands:

PAGER=cat psql --no-psqlrc --single-transaction -d DB -f devel/sql/pgque.sql
PAGER=cat psql --no-psqlrc -v ON_ERROR_STOP=1 -d DB -f tests/test_send_idem.sql
PAGER=cat psql --no-psqlrc -v ON_ERROR_STOP=1 -d DB -f tests/acceptance/us13_producer_idempotency.sql

🤖 Generated with Claude Code

[NikolayS]

samorev Code Review Report

• PR: fix(send-idem): reject non-finite dedup ttl #350 - fix(send-idem): reject non-finite dedup ttl
• Author: @NikolayS
• AI-Assisted: Yes

Pipeline	Coverage
PASS	Not reported

---

REVIEW FINDINGS (5)

HIGH MR/PR state - Review target is draft

The review target is still marked as draft.
Fix: Mark it ready for review before merge.

LOW [guidelines] The error string was rewritten for all three validation branches, not just the new non-finite one, so send_idem(ttl => null) and send_idem(ttl => '0') now raise a different message than before. Any in-repo assertion, doc table, or client matching the old 'ttl must be a positive interval' silently breaks. Validation in the description covers only tests/test_send_idem.sql and tests/acceptance/us13_producer_idempotency.sql, not the full suite.

- raise exception 'ttl must be a positive interval'; → + raise exception 'dedup ttl must be a positive finite interval, got %', i_ttl; (guard covers i_ttl is null and i_ttl <= interval '0' too)
Fix: grep -rn "must be a positive interval" tests/ docs/ devel/ and update any stale matcher; run the whole test suite, not the two files, before merge. If the old string is part of a documented contract, keep it as a prefix (e.g. ttl must be a positive interval (finite), got %).

LOW [tests] The PR's central safety argument — "all send_idem overloads reduce to the text overload, so the single guard covers both text and jsonb" — is asserted but not pinned by a test. The new case exercises only the 5-arg text overload. If a later change inlines or duplicates validation in the jsonb overload, the infinity hole reopens on that path with no failing test.

perform 1 from pgque.send_idem('test_idem', 'migrate', '{}', 'inf:k1', 'infinity'::interval); — single overload; the file already has a jsonb overload dedups like the text overload case, so the jsonb entry point is otherwise covered.
Fix: Add the mirror case against the jsonb overload ('{}'::jsonb, key inf:k2) inside the same PG 17+ gate — two extra lines, and it makes the delegation claim self-enforcing.

LOW [tests] On PG < 17 the new test emits a NOTICE: SKIP and returns, but the suite still reports success and the description's "13/13 PASS" count. A skip is indistinguishable from a pass to the harness, so a future regression that removed the guard would look green on any pre-17 CI leg. (No coverage loss today, since infinite intervals can't be constructed there — the issue is the reporting.)

if v_pg < 170000 then raise notice 'SKIP: infinite intervals require PG 17+ (server %)', v_pg; return; end if;
Fix: If the suite has a skip/pass accounting convention, use it; otherwise assert the guard itself unconditionally on all versions (e.g. that a NULL/zero ttl is rejected with the finite-naming message) and keep only the 'infinity'::interval literal behind the version gate.

LOW [bugs] Unverified from the diff — the guard is placed in the send_idem API body, so any other writer of pgque.idem rows (a batch producer entry point, an internal helper the overloads share, or a queue-level default dedup TTL settable via config) would still be able to persist an expires_at of infinity that maint_idem never reaps. Same question for sibling interval parameters not touched here (delay, retention).

validation is inline in the function body, immediately before select q.queue_id, q.queue_extra_maint; only claim_slot is cited as already having the isfinite pattern.
Fix: grep -n "interval '0'" devel/sql/pgque-api/*.sql and confirm every interval validator that feeds a stored expires_at/deadline also has isfinite; if a batch or config path writes pgque.idem, either route it through the same validation or add the guard there.

---

Summary

Area	Findings	Potential	Filtered
CI/Pipeline	0	0	0
Security	0	0	0
Bugs	0	1	0
Tests	0	2	0
Guidelines	0	1	0
Docs	0	0	0
Metadata	1	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/PgQue
number=350
target=github:NikolayS/PgQue#350
state=OPEN
draft=true
diff_lines=88
diff_added=38
diff_removed=6
diff_bytes=3815
comments_count=0
commits_count=1
ci_status=success
ci_summary=total=17 success=17 failure=0 pending=0 other=0
prompt=.claude/commands/review-mr.md
blocking=false
posted_by=gh
no_comment=false
live_posting=posted

---

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