fix: backport receive safety to 0.2.1 - #367
Conversation
Refs #365. Reject oversized plain and cooperative batches, correct documentation, and retain the 0.2 stable feature set.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Refs #365. Backport complete-batch retry assertions.
samorev Code Review ReportReviewed head:
BLOCKING ISSUES (4)MEDIUM [docs] The v0.2.0 → v0.2.1 upgrade note says to "re-run the installer", but the pg_tle installer can't do that.
MEDIUM [bugs] Every overflow raises the generic
LOW [tests] The "Exactly N" section of
LOW [docs] The reference tells users they can "use the low-level full-batch interface" but never names it (for example
Summary
Note:
Review metadataContested / dropped
samorev-assisted review (AI analysis by Tanya301/samorev) |
Refs #365. Register a function-only update edge, classify overflow as SQLSTATE54000, and tighten regression assertions.
samorev Code Review Report — full AI reviewReviewed head:
Verdict: ❌ CHANGES REQUESTED — 2 blocking issues. This is not a PASS. BLOCKING ISSUES (2)MEDIUM
MEDIUM
NON-BLOCKING (1)LOW
POTENTIAL ISSUES (12)These have moderate confidence (4–7/10). Review them manually; some may be false positives. MEDIUM
MEDIUM
LOW
LOW
LOW
LOW
LOW
LOW
INFO
INFO
INFO
INFO Overall - The dollar-quote collision guard in
Summary
Note: "Findings" are high-confidence (8–10/10) issues, "Potential" are moderate-confidence (4–7/10) and "Filtered" are 0–3/10. Duplicate findings from different agents were merged into one row at the highest validated confidence. The table counts each consolidated finding once, under its primary area. Filtered (1): Guidelines — "PR bundles fix + pg_tle update path + version bump". Issue #365 explicitly requires the backport, release prep, and installation/upgrade tests in this change, so the bundling is in scope (validated 3/10). Issue #365 requirements check
Validation of the round-2 deterministic CLI gate (
|
Resolve review findings on SDK documentation and default ceilings. Refs #365.
samorev Code Review Report — round 3 (delta review)Reviewed head: Scope: delta review. This round covers the changes from the previous full-source review at
Verdict: ✅ PASSED. No blocking findings at This is a review verdict, not merge approval. One merge prerequisite is still open: neither the PR description nor CLAUDE.md step 3 is satisfied yet, because the real v0.2.0 upgrade/reapply evidence has not been posted on the PR. CI does not cover the plain-SQL v0.2.0→HEAD path (see Potential #1). Prior blocking findings (round 2 at
|
| # | Round-2 finding | Status | Evidence |
|---|---|---|---|
| B1 | MEDIUM docs/upgrading.md: the default ceiling of 100 is undersized and the docs gave no pre-upgrade warning |
✅ Resolved (core). Leftovers are non-blocking. | Done: • docs/upgrading.md:23-30 warns before upgrade. It names the SQL default (100) against the ticker threshold (500), the direct Python/TS receive and Go ReceiveCoop defaults, says a same-ceiling retry cannot progress, says to alert on SQLSTATE 54000 and lag, and says the high-level consumers are unaffected.• docs/reference.md:125 adds "Choose a ceiling explicitly", and the example changes from 100 to 1000 marked as illustrative.• reference.md:181 adds a note on the cooperative default.All claims were checked against source: receive.sql:30, sql/pgque.sql:110, client.py:222,396, client.ts:158,409, options.go:119, and the consumer defaults pgque.go:384, consumer.py:26, consumer.ts:12. The coop consumers pass the integer maximum (consumer.go:121, consumer.py:250, consumer.ts:85). Go Receive has no default, so leaving it out is correct.Leftovers: docs/pgq-concepts.md:67 (Non-blocking #1) and the demo examples (Potential #2). |
| B2 | MEDIUM: stale truncation wording in the TS README, client.ts, types.ts, Go options.go and Python client.py |
✅ Resolved | All listed strings now describe complete-batch-or-54000. Also fixed: the Go pgque.go Receive/ReceiveCoop comments, the Go/TS README coop rows, and the old-server wording in the Python/Go READMEs. A repo-wide grep finds no "up to max", "rows beyond", "skipped after ack", "drop rows", "row cap" or "match ticker_max_count" wording left in clients/, docs/ or README. One exception: the Python README coop example still uses max_messages=100 with no note (Potential #2). |
| — | Round-2 non-blocking: the PR description cited a partition test that doesn't exist | ✅ Resolved | The description now says "Partitioned receive is absent from the 0.2 line; its audit and tests apply only to development PR #366." |
| — | Round-2 non-blocking: v0.2.0 upgrade evidence not posted | ❌ Still open | No evidence comment on the PR. The only comments are the Codex usage-limit notice and the samorev round 1/2 reports. |
CLI gate report (pr-367-samorev-gate.md, run against dc0b698): validation
- MEDIUM [tests] "no CI test for plain-SQL v0.2.0 → HEAD reinstall over a populated database". The CLI gate marked this BLOCKING. Confirmed as a real gap, but scored at 7/10 (potential), not blocking.
.github/workflows/ci.ymlat head has onlyupgrade v0.1.0 to HEAD(git show v0.1.0:sql/pgque.sql, line 126) and the pg_tle 0.2.0 update test (line 224). Nothing runsv0.2.0:sql/pgque.sql.- The Bug Hunter and Test Analyzer each validated it independently at 7/10. Mitigation: the v0.1 job already exercises a populated reinstall, and the 0.2.0→0.2.1 SQL delta is function-only
create or replace. Round 2 verified that onlyreceive,receive_coopandversionchange. The delta does not touchsql/orbuild/. - Posted manual evidence would also satisfy CLAUDE.md step 3. See Potential Set up PgQ git submodule (vendor/pgq/) #1.
- Defender-dropped "resource-safe" wording item: agree with the drop. The delta keeps the wording, and it still describes the operator's choice of ceiling, not bounded server materialization.
- Defender-dropped
when othersintest_api_receive: agree with the drop. A related, separate weakness intest_receive_overflow.sqlis carried as Potential Add pgque-specific schema additions (config table, roles, queue_max_retries) #3.
NON-BLOCKING (1)
LOW docs/pgq-concepts.md:67 - ticker_max_count is still described as "force tick at N events (batch-size cap)". (confidence 8/10; Bugs 8 + Docs 7; pre-existing, in #365 scope)
- This directly contradicts what this PR now says everywhere else:
reference.md:135,examples.md:70,three-latencies.md:47,tick-frequency.md:67and every client doc ("Ticker thresholds do not cap batch size").- With the overflow guard in place, a reader who sizes
max_returntoticker_max_countfrom this page gets SQLSTATE 54000 during bursts.Suggestion: Reword to "event-count threshold for creating a tick; not a batch-size cap", matching
tick-frequency.md:67.
POTENTIAL ISSUES (9)
These have moderate confidence (4–7/10). Review them manually; some may be false positives.
MEDIUM .github/workflows/ci.yml - No CI job reinstalls plain-SQL v0.2.0 → HEAD over a populated database. (confidence 7/10; Tests 7 + Bugs 7; CLI gate's blocking item; carried forward and still unresolved)
Suggestion: Add a v0.2.0 variant of the upgrade job:
- install
v0.2.0:sql/pgque.sql;- create an active batch of 3 events plus a coop member;
- reapply HEAD;
- assert
version()='0.2.1', unchanged subscription rows,receive(...,2)→ 54000,receive(...,3)→ 3 events, andtest_upgrade_grants.sql.Posting manual v0.2.0 reapply evidence on the PR is in any case required before merge (CLAUDE.md step 3).
LOW README.md:314,330,405,423, docs/tutorial.md:102,147,168,189,228,260, docs/examples.md:46,47,59,80, clients/python/README.md:73,144 - The most-copied snippets still pass 100 with no caveat. (Docs reported this at 8; the orchestrator validated it at 6 and downgraded it from MEDIUM; leftover of B1, pre-existing)
- Every one of these calls was checked: each is a forced-tick, one-event demo, so
100is correct where it appears.examples.md:70already carries a complete-batch caveat after the exactly-once pattern.- The risk is copy-paste into production. The Python README coop example (
max_messages=100) and the README TS snippet have no nearby note. Python README line 73 still says "per-receivelimit".Suggestion: Add a single line after the first README and tutorial receive, e.g. "100 is a demo ceiling; size it for your bursts (see reference: Choose a ceiling explicitly)". Reword Python README line 73 to "complete-batch safety ceiling".
LOW tests/test_receive_overflow.sql:109,334 - Two overflow assertions accept any error. (confidence 6/10; Tests; carried forward)
The already-allocated-batch and stale-takeover cases use
when others then v_raised := truewith no SQLSTATE check, confirmed by direct inspection. There is also still no coop exactly-N, NULL or INT_MAX case.Suggestion: Use
when sqlstate '54000'and add at least a coop exactly-N case.
LOW docs/upgrading.md:48-51 + sql/pgque-tle.sql:7277-7280 - The new pg_tle origin paragraph is accurate but only partly actionable. (confidence 6/10; Docs; partially resolves a round-2 potential)
- The claim "rejected without changing the installation" is correct: the raise happens inside the DO block before any pg_tle change.
- However, the wrapper keys on
pgtle.available_extensions().default_version, and the doc gives no query to check it.- "Plan and test a separate migration" gives no procedure, and the exception text still does not point to the docs.
Suggestion: Add
select default_version from pgtle.available_extensions() where name = 'pgque';, and point the exception text atdocs/upgrading.md.
LOW tests/test_tle_upgrade_v0_2.sql - The unsupported-origin and already-registered-path branches of the wrapper are untested. (confidence 5/10; Tests; carried forward)
The delta's new claim "Other origins are rejected without changing the installation" raises the value of a test.
Suggestion: Add a scratch-database case asserting that the raise leaves
extversionand the pg_tle registration unchanged.
LOW clients/typescript/test/ - There is no TypeScript overflow/54000 integration test, unlike Go and Python. (confidence 5/10; Tests; carried forward)
LOW docs/tutorial.md:247,285 - The receive(...,1) limit 1 then nack+ack pattern is presented as "the natural pattern". (confidence 5/10; Bugs; pre-existing)
It only works because each tutorial batch holds one event. Copied code now raises 54000 on a batch of more than one event. Before this change it dropped the rest on ack.
Suggestion: Note the single-event assumption.
LOW clients/go/concurrency_test.go:48-49 - The new comment says "maximum producer count", but the ceiling 2*expected is sized against the total event count (goroutines * perGoroutine). (confidence 5/10; Bugs; new in delta, comment only)
Suggestion: "…knows its total event count, so use a ceiling above it."
LOW sql/pgque-tle.sql:7292-7294 / README.md:219-234 - Two round-2 potential items are still unresolved. (confidence 5/10; Docs; carried forward)
The final
\echois vague and prints on every path, and the README pg_tle section does not link todocs/upgrading.md.
INFO (Guidelines, confidence 4–5; not counted above):
docs/upgrading.md:19,33: "rather than returning a truncated result" and "now causes an error" are history framing (CLAUDE.md Documentation). This is acceptable inside the per-version upgrade section..github/workflows/ci.yml:6,8: the short-lived branchrelease-0-2-1is hardcoded in the triggers. Remove it after the release.- Commit
dc0b698has a body line of about 78 characters, over the 72-character limit indevelopment__git-commit-standards.mdc. Do not amend.
Security (confidence 4, carried forward, still present): build/transform.sh:1003-1008: the dollar-quote collision guard scans pgque.sql but not the generated pg_tle update body. Not exploitable today, because the body is extracted from the same sources. Defense-in-depth fix: repeat the grep -qF check on ${PGTLE_UPDATE_FILE}.
Summary
| Area | Findings | Potential | Filtered |
|---|---|---|---|
| CI/Pipeline | 0 | 0 | 0 |
| Security | 0 | 1 | 0 |
| Bugs | 1 (merged w/ Docs) | 2 | 0 |
| Tests | 0 | 4 | 0 |
| Guidelines | 0 | 3 (INFO) | 0 |
| Docs | 0 | 3 | 0 |
| Metadata | 0 (prior item resolved; evidence item tracked as merge prerequisite) | 0 | 0 |
Note: "Findings" are high-confidence (8–10/10) issues, "Potential" are moderate (4–7/10) and "Filtered" are 0–3/10. Duplicate findings from different agents were merged at the highest validated confidence and are counted once, under their primary area.
Specialized agent results (all 5 completed on dc0b698)
Agent 1 — Security Reviewer (opus): NO_FINDINGS at confidence 4+ in the delta; 1 carried-forward LOW(4)
- No executable change in the delta:
git diff -U0 -won the.go/.py/.tsfiles leaves only comment, JSDoc or docstring lines.- Signatures, defaults, queries and the
n <= 0panics are unchanged. - In
concurrency_test.goonly the comment changed.
- No SQL/build changes in the delta:
build/andsql/are untouched (git diff --statis empty), so SECURITY DEFINER,search_path, grants and the generated artifacts are as reviewed in round 2. - Delta content:
- No secrets or DSNs.
- The only literal change is example
100→1000. - The doc guidance is safe: it says not to drop a populated extension, to never ack after 54000, and it adds no advice to pass NULL to disable the ceiling.
- Prior finding: the dollar-quote guard gap is still present at
build/transform.sh:1003-1008(LOW 4, missing guard, not a live exploit).
Agent 2 — Bug Hunter (opus): 5 findings (MEDIUM 7, LOW 8, LOW 6, LOW 5, LOW 5)
- Findings:
- MEDIUM (7)
ci.yml: no plain-SQL v0.2.0→HEAD test. This is the gate item, unresolved. - LOW (8)
docs/pgq-concepts.md:67: "batch-size cap". - LOW (6): uncaveated
100examples in README, tutorial, examples and the Python README. - LOW (5)
docs/tutorial.md:247,285: thelimit 1pattern. - LOW (5)
concurrency_test.go:48: comment says "producer count" where it means event count.
- MEDIUM (7)
- Prior status:
- B1: partially resolved; the core warning is done.
- B2: resolved.
run_all.sql\echo: resolved.- pg_tle origin documentation: resolved, but the exception text has no pointer.
- Gate CI item: unresolved.
- Checked clean:
- Delta byte-identical to
git diff 22e0b240 dc0b698(12 files, +75/−61). - All new numeric and default claims match source.
- 54000 errcode is present in both functions in every built artifact.
- "No partial result": PL/pgSQL
return nextbuffers rows in a tuplestore, so the raise aborts the statement before any row is sent. - "Same ceiling cannot make progress" holds.
- The wrapper rejects other origins before any pg_tle change.
- Below threshold: role pre-creation runs before the check, a no-op on a populated install; an unregistered (NULL) origin goes to
install_extension.
- Delta byte-identical to
Agent 3 — Test Analyzer (sonnet): 5 findings (MEDIUM 7, LOW 6, LOW 5, LOW 5, LOW 5)
- Findings:
- MEDIUM (7): no CI v0.2.0 plain-SQL reinstall. Validated as the gate item; the agent rated it MEDIUM rather than a hard block given the v0.1 reinstall job and the function-only delta.
- LOW (6)
test_receive_overflow.sql:109,334:when otherswithout a SQLSTATE check. - LOW (5): no coop exactly-N/NULL/INT_MAX cases.
- LOW (5): no TS 54000 test.
- LOW (5): untested wrapper branches, now more relevant because of the new doc claim.
- Delta:
- The
run_all.sql\echois correct and closes the prior item. - The
concurrency_test.gochange is comment-only. - No new-in-delta test findings.
- The
Agent 4 — Guidelines Checker (sonnet): 3 INFO (confidence 4–5)
- Rules read from local postgres-ai rules (
/pgai-rules/rules):development__core-principles,development__ai-coding-guidelines,development__git-commit-standards,writing__professional-communication,writing__terminology-consistency,writing__title-capitalization(plusdevelopment__db-sql-style-guide; the delta has no SQL logic). Also PgQueCLAUDE.md. - Findings:
- INFO (5):
dc0b698body line about 78 characters, over 72. - INFO (5):
upgrading.md:19,33history framing; tolerated in the per-version section. - INFO (4):
release-0-2-1hardcoded in the CI triggers.
- INFO (5):
- Closed:
run_all.sql\echo.- "Older servers can truncate" framing in the Python and Go READMEs.
- Checked clean:
- Terminology is consistent across the Go, Python, TS and docs ("complete-batch safety ceiling", "SQLSTATE
54000", "resource-safe larger ceiling"). - No PR or issue numbers in the docs.
- Sentence-case headings.
- SOC2 ignored per CLAUDE.md.
- Terminology is consistent across the Go, Python, TS and docs ("complete-batch safety ceiling", "SQLSTATE
Agent 5 — Docs Reviewer (sonnet): 5 findings (MEDIUM 8 → validated 6 by orchestrator, LOW 7, LOW 6, LOW 5, INFO 5)
- Findings:
- MEDIUM (8): the README, tutorial, examples and Python README still show
100without a caveat. The orchestrator downgraded this to LOW/6 potential after checking that every listed call is a one-event demo, and thatexamples.md:70already carries the complete-batch caveat. - LOW (7)
pgq-concepts.md:67: "batch-size cap". Merged with the Bug Hunter finding at 8. - LOW (6)
upgrading.md:48-51: accurate, but gives no check query and no exception pointer. - LOW (5): the pg_tle
\echoand the missing README link are unresolved. - INFO (5):
upgrading.mdhistory wording.
- MEDIUM (8): the README, tutorial, examples and Python README still show
- Prior status:
- B1: partially resolved; the core is done and the leftovers are above.
- B2: resolved, except for the Python README coop example.
- Checked clean:
- Every new delta sentence was checked against source defaults and
receive.sql:53-54. - No stale truncation wording in the client docs,
reference.md,upgrading.mdorthree-latencies.md.
- Every new delta sentence was checked against source defaults and
Not applicable:
- Agent 6 (Sqitch): postgres-ai/platform only.
- Agent 7 (Visual): the PR touches no UI or visual paths, so
IS_VISUAL=0. The GitHub file list was not truncated (28/28 files).
Coverage and limitations
- Newly reviewed in this round:
- The complete delta
22e0b240..dc0b698: 12 files, +75/−61, 338 diff lines, read in full by every agent. It was confirmed byte-identical togit diff. - Source files inspected directly at
dc0b698wherever a claim needed checking:- the SQL defaults and the 54000 raise sites;
- the client defaults and consumer paths;
- the pg_tle wrapper;
ci.yml;- the overflow tests;
- every remaining
receive(...)example in the README, tutorial and examples.
- No reviewer worked from clipped diff context.
- The complete delta
- Carried forward from round 2 (full-source multi-agent review at
22e0b24, saved atpr-367-samorev-round2.md) — the delta does not touchsql/orbuild/(verified with an emptygit diff --stat), so the round-2 runtime-relevant results stay valid:- generated
sql/pgque.sql,sql/pgque-tle.sqlandpgque--0.2.0--0.2.1.sqlreproduce byte-for-byte frombuild/transform.sh; - the v0.2.0→head function delta is only
receive,receive_coopandversion; - all wrapper branches were read and checked;
- SECURITY DEFINER
search_pathand grants were checked; - SRF and rollback semantics were checked.
- generated
- Runtime: this review did not run the SQL, pg_tle or client test suites locally. Runtime behavior rests on the 28/28 green CI checks on the exact head
dc0b698. CI does not cover the plain-SQL v0.2.0→HEAD reinstall over populated state, and neither does this review (Potential Set up PgQ git submodule (vendor/pgq/) #1). No posted manual evidence covers it yet either. - Validation (Step 5.5): done by the orchestrator, which checked each finding against source and merged cross-agent duplicates. It did not use separate validator subagents.
- Org rules: loaded locally from
pgai-rules/rules. - SOC2: skipped, as PgQue CLAUDE.md requires.
- Completeness: all 5 specialized reviewers completed. None timed out or was incomplete.
samorev-assisted review (AI analysis by Tanya301/samorev); full multi-agent /review-mr delta run, round 3. Exit status: 0 (no blocking findings). This is not an approval and nothing was merged.
Clarify ticker thresholds and consumer comments. Refs #365.
samorev Code Review Report — round 4 (delta review)Reviewed head: Scope: delta review. This round covers only the delta from the previous reviewed head
Verdict: ✅ PASSED. There are no blocking findings at This is a review verdict, not merge approval. The gates below are unchanged and still apply:
Delta reviewed (
|
| File | Change | Verified against source at head |
|---|---|---|
clients/go/concurrency_test.go:49 |
Comment: "maximum producer count" → "maximum message count" | Correct. expected := goroutines * perGoroutine (10×20 = 200 events). The ceiling is 2*expected. The test logic is unchanged (git diff -w -U0). |
clients/python/README.md:73 |
"controls the per-receive limit" → "sets the complete-batch safety ceiling for each receive" |
Correct. consumer.py:26 sets _DEFAULT_MAX_MESSAGES = 2_147_483_647. It is passed to receive/receive_coop at consumer.py:250,255, and the wording matches the client.py docstrings. |
clients/typescript/src/consumer.ts:9-10 |
JSDoc rationale: "so a subsequent pgque.ack(batch_id) does not strand events" → "so ordinary bursts do not exceed a smaller receive ceiling (SQLSTATE 54000)" |
Correct. The old rationale was stale: under complete-batch-or-54000, ack can no longer strand unseen events. DEFAULT_MAX_MESSAGES is unchanged and used at lines 46, 85 and 88. |
docs/pgq-concepts.md:67 |
"force tick at N events (batch-size cap)" → "event-count threshold for creating a tick; not a batch-size cap" | Correct. Matches tick-frequency.md:67, reference.md:135, three-latencies.md:47 and examples.md:70. |
No executable code changed. git diff --stat dc0b698 034f3f6 -- sql build is empty, and .github/workflows/ci.yml and tests/ are untouched.
Prior findings: status at 034f3f6
| Prior item | Round-3 status | Status now |
|---|---|---|
| Round-2 B1 (MEDIUM, default ceiling 100 undersized, no pre-upgrade warning) | ✅ Resolved (core) | ✅ Still resolved. Nothing in the delta reverts it. |
| Round-2 B2 (MEDIUM, stale truncation wording in clients) | ✅ Resolved | ✅ Still resolved. Head-tree grep: "batch-size cap" appears only in negated form, and "producer count" has no hits. "per-receive limit" appears only in the untracked, gitignored clients/python/pgque_py.egg-info/PKG-INFO (.gitignore:5). |
Round-3 Non-blocking LOW(8) docs/pgq-concepts.md:67 "batch-size cap" |
Open | ✅ Resolved, with wording that matches the suggestion. |
Round-3 Potential LOW(5) concurrency_test.go:48-49 "producer count" |
Open | ✅ Resolved |
Round-3 Potential LOW(6) 100 demo examples / Python README line 73 "per-receive limit" |
Open | ◐ Partially resolved. Python README line 73 is reworded. The uncaveated 100 demo snippets in README/tutorial/examples carry forward (Potential #2). |
| Round-3 Potential MEDIUM(7) no CI plain-SQL v0.2.0→HEAD reinstall | Open | Carried forward, unchanged (Potential #1) |
| All other round-3 potentials (tests, pg_tle docs, tutorial pattern, transform.sh guard) | Open | Carried forward, unchanged. Those files are not in the delta. |
| Round-2/3 metadata: v0.2.0 upgrade evidence not posted | ❌ Open | ❌ Still open. This is a merge prerequisite (see above). |
There were no blocking findings in round 3, so no blocking item needed re-validation beyond round-2 B1/B2, which remain resolved.
POTENTIAL ISSUES (8, all carried forward; none new in this delta)
These have moderate confidence (4–7/10). Review them manually. Full text for each is in the round-3 report on this PR.
- MEDIUM
.github/workflows/ci.yml: no CI job reinstalls plain-SQL v0.2.0 → HEAD over a populated database. (7/10; Tests + Bugs; unchanged) Posting manual v0.2.0 reapply evidence on the PR is required before merge in any case. - LOW
README.md:314,330,405,423,docs/tutorial.md:102,147,168,189,228,260,docs/examples.md: one-event demo snippets pass100with no "demo ceiling" caveat. (5–6/10; Docs + Bugs) The Python README line 73 part of this item is now resolved. - LOW
tests/test_receive_overflow.sql:109,334:when otherswithout a SQLSTATE54000check, and there is no coop exactly-N/NULL/INT_MAX case. (6/10; Tests) - LOW
docs/upgrading.md:48-51+sql/pgque-tle.sqlexception text: nopgtle.available_extensions()check query, and no pointer to the docs. (6/10; Docs) - LOW
tests/test_tle_upgrade_v0_2.sql: the unsupported-origin and already-registered wrapper branches are untested. (5/10; Tests) - LOW
clients/typescript/test/: no TypeScript 54000 integration test. (5/10; Tests) - LOW
docs/tutorial.md:247,285: thereceive(...,1) limit 1pattern relies on single-event batches. (5/10; Bugs) - LOW
sql/pgque-tle.sql:7292-7294/README.mdpg_tle section: the final\echois vague, and the pg_tle section does not linkdocs/upgrading.md. (5/10; Docs)
Security (carried forward, confidence 4): build/transform.sh:1003-1008: the dollar-quote collision guard does not re-scan the generated pg_tle update body. This is defense in depth and not exploitable today. The file is unchanged in this delta.
INFO (Guidelines, carried forward):
docs/upgrading.md:19,33uses history framing. This is tolerated in the per-version section..github/workflows/ci.yml:6,8hardcodesrelease-0-2-1in its triggers. Remove it after the release.- Commit
dc0b698has a body line of about 78 characters. Do not amend. The new commit034f3f6complies: a 40-character subject and a 59-character body.
Filtered (1):
- The Docs reviewer flagged INFO(4) on
clients/go/concurrency_test.go:253-256, where the test comment says the old hardcodedReceive(...,100)was "stranding any extras". - The orchestrator validated it at 3/10 and filtered it. The comment is explicitly historical ("prior to WithMaxMessages") and accurately describes pre-fix server behavior. The Bug Hunter independently judged it accurate history. It is also pre-existing and outside the delta.
Summary
| Area | Findings | Potential | Filtered |
|---|---|---|---|
| CI/Pipeline | 0 | 0 | 0 |
| Security | 0 | 1 (carried) | 0 |
| Bugs | 0 | 1 (carried) | 0 |
| Tests | 0 | 4 (carried) | 0 |
| Guidelines | 0 | 3 INFO (carried) | 0 |
| Docs | 0 | 3 (carried) | 1 |
| Metadata | 0 (v0.2.0 evidence tracked as a merge prerequisite) | 0 | 0 |
New-in-delta findings: 0. The round-3 non-blocking finding and one round-3 potential are resolved. A second round-3 potential is partially resolved.
Specialized agent results (all 5 completed on 034f3f6)
Agent 1 — Security Reviewer (opus): NO_FINDINGS in the delta; 1 carried-forward LOW(4). Complete.
git diff -w -U0shows only Go comment, JSDoc and Markdown lines.DEFAULT_MAX_MESSAGES = 2_147_483_647is untouched (checked in source).sql/andbuild/are untouched, so the round-2/3 runtime-relevant security results carry forward.- No secrets in the diff (grep for password/secret/token/key).
- No unsafe guidance: no advice to use NULL or unlimited ceilings, and no advice to ack after 54000. The new wording strengthens the "not a batch-size cap" guidance.
- Carried forward:
build/transform.sh:1003-1008dollar-quote guard gap, LOW(4). The file is unchanged.
Agent 2 — Bug Hunter (opus): NO_FINDINGS in the delta. Complete for the delta scope.
- Each of the 4 changed lines was verified against source at head:
- Go: 200 events with a ceiling of 400.
- Python:
consumer.py:26,250,255and theclient.py:238,411docstrings. - TypeScript:
consumer.ts:46,85,88andclient.ts:153("returns no partial result and fails"). - pgq-concepts: consistent with tick-frequency/reference/three-latencies/examples.
- Head-tree grep:
- "batch-size cap" appears only in negated form.
- "producer count": no hits.
- "per-
receivelimit" appears only in the untracked egg-info. - "strand" appears only in a historical Go test comment at line 255, outside the delta and accurate as history.
- Prior items:
- pgq-concepts LOW(8): resolved.
- concurrency_test LOW(5): resolved.
100examples LOW(6): partially resolved (Python README fixed).- Tutorial LOW(5): carried forward.
- CI v0.2.0 MEDIUM(7): carried forward.
- Round-2 B1/B2: not reverted.
- Noted limits: it did not re-audit unchanged code, did not open the round-3 file (prior context was supplied in its prompt), and ran no tests.
Agent 3 — Test Analyzer (sonnet): NO_FINDINGS in the delta; 5 carried-forward potentials. Complete.
- The Go test change is comment-only (
git diff -w -U0), and the test logic is unchanged. - No
tests/,sql/, build or CI files changed, so no behavior change needs test updates. - Prior items, all unchanged and carried forward:
- CI v0.2.0 plain-SQL reinstall, MEDIUM(7)
test_receive_overflow.sql:109,334when others, LOW(6)- coop exactly-N/NULL/INT_MAX, LOW(5)
- TS 54000 test, LOW(5)
- pg_tle wrapper branches, LOW(5)
Agent 4 — Guidelines Checker (sonnet): NO_FINDINGS in the delta; 3 carried-forward INFO. Complete for the delta.
- Rules read (locally from
pgai-rules/rules): PgQueCLAUDE.md(Documentation and Git Commits sections),development__git-commit-standards,writing__terminology-consistency. - Rules not opened in full:
writing__professional-communication,writing__title-capitalization,development__core-principles,development__ai-coding-guidelines. The agent judged that none of them applies to 5 lines of prose and comments with no headings. This is a disclosed scope limit, not an incomplete run. - Commit
034f3f6: the subject is 40 characters, Conventional Commitsdocs:, present tense. The body is 59 characters withRefs #365. Both comply, and the orchestrator re-measured them independently. - Terminology: "complete-batch safety ceiling" and "SQLSTATE 54000" are consistent across clients and docs. No PR/issue numbers or version tags were added to the docs, and no history framing was added.
- Python README line 73 is 92 characters, longer than its neighbors. No rule sets a Markdown line width, so this is not a finding.
- Prior INFO items carried forward:
dc0b698body length (do not amend),upgrading.mdhistory framing, andrelease-0-2-1CI trigger. - SOC2: ignored per PgQue CLAUDE.md.
Agent 5 — Docs Reviewer (sonnet): no new findings above the filter; 1 INFO(4) filtered by the orchestrator; carried-forward LOWs. Complete.
- All 5 changed lines are accurate at head. The removed TS "strand events" rationale was stale under complete-batch-or-54000, and removing it loses no information.
- Grep found no live doc that contradicts complete-batch-or-54000.
upgrading.md:19"truncated result" is correct history. The TRUNCATE-rotation andedge_test.gotruncation hits are unrelated. The stalePKG-INFOis an untracked, gitignored build artifact. - INFO(4)
concurrency_test.go:253-256"stranding any extras": filtered by the orchestrator. It is a historical comment, accurate for pre-fix servers, pre-existing and outside the delta. - Prior items:
- pgq-concepts LOW(8): resolved.
- Python README line 73: resolved.
- concurrency_test "producer count": resolved.
100README/tutorial examples: carried forward.upgrading.md:48-51check query/exception pointer: carried forward.- pg_tle
\echoand README link: carried forward.
Not applicable:
- Agent 6 (Sqitch): postgres-ai/platform only.
- Agent 7 (Visual): the delta touches no UI or visual paths (
IS_VISUAL=0). The 4-file delta list is complete and not truncated.
Coverage and limitations
- Newly reviewed in this round:
- The complete delta
dc0b698..034f3f6: 1 commit, 4 files, +5/−5. It was read in full by all 5 reviewers fromgit diff, with no clipped diff context. - Source inspected directly at
034f3f6wherever a claim needed checking: Pythonconsumer.py/client.py, TSconsumer.ts/client.ts, the Go test, the tick and threshold docs,.gitignore, andbuild/transform.sh. - The PR fix: reject truncated receive batches #366 body was checked for the tag-match gate text.
- The complete delta
- Carried forward from round 3 (delta review at
dc0b698,pr-367-samorev-round3.md) and round 2 (full-source multi-agent review at22e0b24,pr-367-samorev-round2.md). These results stay valid becausesql/,build/,tests/and.github/are untouched in this delta (verified with an emptygit diff --stat):- generated
sql/pgque.sql,sql/pgque-tle.sqlandpgque--0.2.0--0.2.1.sqlreproduce byte-for-byte frombuild/transform.sh; - the v0.2.0→head function delta is only
receive,receive_coopandversion; - the pg_tle wrapper branches were checked;
- SECURITY DEFINER
search_pathand grants were checked; - SRF and rollback semantics were checked;
- the round-3 doc/default claims were verified against source.
- generated
- Not re-audited: unchanged code outside the delta, beyond the targeted greps and source checks listed above.
- Runtime: this review ran no SQL, pg_tle or client test suites locally. Runtime behavior rests on the 28/28 green CI check runs on the exact head
034f3f6and on the carried-forward round-2 analysis. Neither CI nor this review covers the plain-SQL v0.2.0→HEAD reinstall over populated state (Potential Set up PgQ git submodule (vendor/pgq/) #1), and no manual evidence for it has been posted yet. - Validation (Step 5.5): done by the orchestrator, which checked each finding against source and merged cross-agent duplicates. It did not use separate validator subagents.
- Org rules: loaded locally from
pgai-rules/rules. The Guidelines reviewer's rule subset is disclosed above. - SOC2: skipped, as PgQue CLAUDE.md requires.
- Completeness: all 5 specialized reviewers completed. None timed out or was incomplete.
samorev-assisted review (AI analysis by Tanya301/samorev); multi-agent /review-mr delta run, round 4, reviewed head 034f3f6b353caeedd00d3f8cb1041cde58520692. Exit status: 0 (no blocking findings). This is not an approval. Nothing was approved, merged, committed or pushed.
Post-review real-test evidenceExact reviewed/tested head: Environment: isolated Postgres 18.6 containers, no published ports, localhost listening, connection logging enabled. pg_tle upgrade uses v1.5.2. Commands and resultsFrom the candidate checkout, the regression harness executes: psql -X -v ON_ERROR_STOP=1 -f sql/pgque.sql
psql -X -v ON_ERROR_STOP=1 -f tests/run_all.sql
psql -X -v ON_ERROR_STOP=1 -f tests/acceptance/run_acceptance.sql
psql -X -v ON_ERROR_STOP=1 -f sql/pgque.sql -f tests/test_install_idempotency.sqlOutputs: Additional executable harnesses (all exit 0):
The upgrade harnesses use the maintenance candidate; its three frozen release SQL files are byte-identical to this PR's frozen files. No throughput benchmark or client package publication is claimed. Non-blocking review dispositions
Evidence integrityLogs retained in Release sequencing remains: merge maintenance, verify exact merge CI, publish and verify v0.2.1, prove frozen artifacts match the tag, then merge development. |
Published-tag gate: PASS
At development PR #366 head git diff --exit-code v0.2.1 -- sql/pgque.sql sql/pgque-tle.sql sql/pgque-tle-updates/pgque--0.2.0--0.2.1.sqlExit 0, no diff. The frozen SQL/tag-match release-operator gate is satisfied. A fresh detached checkout of the published tag also passed (all exit 0):
Output markers: No client packages were published. Real pagination remains tracked separately in #364. |
Summary
Refs #365. Backport the complete-batch-or-error receive fix to the latest stable release, v0.2.0, and prepare v0.2.1. This branch deliberately excludes v0.3 alpha features.
Real pagination remains separate in #364.
Verification
Commands for a fresh database:
Required before merge/tag: full existing PG14–18 CI and integration/client jobs, exact-head SamoRev review, actual v0.2.0 upgrade/reapply with retained queue state and complete event recovery. Evidence will be posted on this PR. No package publishing or benchmarks required for the SQL-only fix.
Intentional scope
The owner requested fail-closed overflow, preserving existing ceiling defaults/signatures. An undersized consumer must stop with an observable error until its operator selects a sufficient resource-safe ceiling; automatic ceiling enlargement is deliberately out of scope. Real pagination remains #364. Overflow uses SQLSTATE 54000 so clients can classify the resource-limit condition without matching message text.
Both plain SQL and pg_tle populated-upgrade paths must preserve queue data before this maintenance release is published. The release tag will be cut from this maintenance line, not from v0.3 development.
pg_tle update verification
The v0.2.0→v0.2.1 wrapper now registers a function-only update edge via the documented pg_tle
install_update_pathAPI and sets the new default version. PostgreSQL can create the default version through the existing base install plus that edge; a redundant standaloneinstall_extension_version_sqlregistration is not required. This was executed on pg_tle v1.5.2: both an in-place populated ALTER EXTENSION UPDATE and a fresh CREATE EXTENSION after registration pass. The regression also reruns wrapper registration, preserves active subscription state, and verifies both plain/cooperative overflow return SQLSTATE54000 followed by successful full recovery. The same test is wired into CI, with tagged v0.2.0 fetched via full checkout history.The overflow SQLSTATE change was red/green tested (old P0001 fails the new assertion). Partitioned receive is absent from the 0.2 line; its audit and tests apply only to development PR #366. INT_MAX now explicitly checks zero returned rows and no active batch.
Draft GitHub release notes
The following notes will be published with the immutable maintenance tag after all gates pass.
Complete-batch receive safety
PgQue v0.2.1 is a maintenance release backported to v0.2.0. It does not include the v0.3 alpha features.
pgque.receive()andpgque.receive_coop()now return the complete batch or raise an error when the batch exceedsmax_return. Exactlymax_returnevents succeeds; an additional event raises before the call can return a successful partial result. The failed statement rolls back its consumer allocation/ownership changes. Overflow uses SQLSTATE54000(program_limit_exceeded) and includes recovery guidance.This fixes a data-loss footgun: earlier receives could return only part of a batch, while
ack(batch_id)finished the entire batch and skipped unreturned events.ack()remains a whole-batch acknowledgment; applications must process every event before calling it.Compatibility and recovery
Before upgrading: the default SQL ceiling is 100, below the default ticker event-count threshold of 500. Ordinary batches can exceed 100. Direct Python/TypeScript receives and Go
ReceiveCoopalso default to 100; callers relying on these defaults must pass a sufficient resource-safe ceiling before upgrading. Alert on SQLSTATE54000and growing consumer lag. High-level consumer loops default to the Postgres integer maximum.Older servers can silently truncate receive results; install this server update before relying on overflow protection.
ticker_max_countis a tick-trigger threshold, not a maximum batch size. Repeated receives with an undersized ceiling are not pagination. Monitor overflow errors and consumer lag.Install or upgrade the plain SQL installation
Use the installer from this tag, as its schema owner or a superuser:
For a v0.2.0 plain SQL installation, this replaces functions while preserving queues, consumer positions, active batches, retry/DLQ rows and queue configuration. Reapplication is supported.
Upgrade a pg_tle installation
Register the new version and its non-destructive update path, then update the installed extension:
psql -v ON_ERROR_STOP=1 -d mydb -f sql/pgque-tle.sql psql -v ON_ERROR_STOP=1 -d mydb -c "alter extension pgque update to '0.2.1';"The update replaces only the receive functions and version function. Do not uninstall a populated extension to apply this fix. The wrapper also supports fresh installations.
Verification
Release-gate evidence and SamoRev review are posted on the maintenance PR. The tests cover N-1/N/N+1, integer-maximum ceilings, complete retry payloads, consumer-state rollback, cooperative stale-worker takeover, fresh installation and populated v0.2.0 upgrade/reapplication.
Fixes #365.