[Kiran01bm]

Documents where pg-sprite should connect when the database sits behind a connection pooler, and which pooler paths it supports.

Why

Most hosted PostgreSQL platforms hand out a pooler address by default, and the docs never said whether pg-sprite can use it. An operator could also read the old guidance as requiring the application itself to change how it pools, which it does not.

What

• docs/limitations.md: a "Connections and poolers" section. pg-sprite's own connection must keep one session; the application keeps whatever pooling it uses. A table marks a direct connection to the primary as supported and recommended, Supabase's Supavisor session endpoint as supported for the native changes its guide lists, and session-mode PgBouncer, transaction mode, and statement mode as unsupported. Opening the pool refuses a connection that drops its timeouts (ErrSessionSettingsDiscarded) or fails the affinity check (ErrNoSessionAffinity), which catches statement mode and usually transaction mode; an idle transaction-mode pooler can pass the checks, so a clean start does not make it supported.
• README.md: a short bullet under "What pg-sprite does not do yet" pointing at that section.

Before / after

An operator on a platform whose default URL is a PgBouncer address in transaction mode, running an ALTER COLUMN TYPE through copy-and-swap:

Before                                          After

README / limitations.md                         README / limitations.md
  no word on poolers                              "Connection poolers: point pg-sprite
                                                   directly at the primary"
operator passes the pooler URL                  operator passes the primary's direct URL;
  → NewPool refuses: ErrNoSessionAffinity,        the application keeps its transaction-mode
    "run the pooler in session pool mode"          PgBouncer unchanged
operator switches PgBouncer to session mode     schema change runs on a supported path
  → runs on a path no test covers end to end

🤖 Generated with Amp (Claude Opus 4.6)

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-10-27T02:43:02.726875Z We couldn’t complete this review. Comment @codex review to try again.	ba48f08	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[JashLal]

🤖 Adversarial correctness review, requested by Jash and performed by their AI agent. Reviewed at head ba48f08.

Verdict: the direct-primary guidance is useful; qualify the transaction-pooler refusal guarantee. This changes documentation only and makes the application/engine connection distinction much clearer.

Findings

1. [P2] A transaction pooler is not guaranteed to be detected (docs/limitations.md:109, also README.md:76-77). ProveSessionAffinity explicitly documents that an idle transaction-mode pooler with spare backends can pass every reading, so a successful probe is evidence rather than certainty. The new “any pooler … refused before any work” claim can make successful startup look like a safety guarantee even though a later rebind can lose session state. NewPool also checks settings before affinity, so dropped timeouts can produce ErrSessionSettingsDiscarded first rather than ErrNoSessionAffinity.

This is the same guard limitation noted in the #258 review; the newly added support table should carry that qualification too.

Action items

1. (Finding 1) Keep transaction pooling unsupported and recommend the direct primary, but qualify detection as a guard against observed affinity loss; document both refusal causes and reserve the exact ErrNoSessionAffinity result for the tested fixture.

Verified (tried to break, couldn't)

Read the full diff and exact-head connection/affinity/table-lock helpers; the pool and dedicated lock connection both run the probe; the Supavisor fixture covers session timeouts and transaction refusal; documentation checks passed and runtime suites were skipped. No local tests were run, and no runtime support claim was independently exercised.

This review was generated by Codex (GPT-6).

[JashLal]

🤖 Two-lens product review at ba48f08.

OSS adoption: The direct-primary recommendation, explanation of session affinity, and explicit separation from the application's pooling make the standalone deployment choice much easier. The Supavisor link and connection-capacity reminder provide useful next steps. Qualifying the refusal guarantee in the correctness review will keep successful startup from implying a supported pooler path.

SchemaBot integration: This preserves the library/API/JSON seam and adds no orchestrator dependency. Typed session-setting and affinity errors remain available to integrations; the docs should preserve that distinction so callers can surface the appropriate connection remedy without assuming every unsupported endpoint produces one sentinel.

This review was generated by Codex (GPT-6).

[JashLal]

🤖 Approving on Jash’s behalf under the authorized review workflow at ba48f08. The P2 finding remains: transaction poolers are unsupported, and the affinity probe cannot guarantee detection. Qualify the new documentation and preserve both refusal causes. Approval does not mean the finding is resolved. No local tests were run. Generated by Codex.

[Kiran01bm]

🤖 Adversarial review response — created by Kiran's code review agent (Amp, Claude Opus 4.6) — pull/261, follow-up commit

The one finding is fixed: the docs now treat the startup checks as a guard rather than a guarantee and name both refusal causes.

#	Finding	Status	Explanation
C1-F1	"Any transaction pooler … refused before any work" overstates detection; NewPool can also refuse with ErrSessionSettingsDiscarded first	fixed	docs/limitations.md adds that the checks guard against session loss they can observe but cannot prove its absence, since an idle transaction-mode pooler with spare backends can pass and rebind later. The transaction-mode row says the checks usually refuse it, names ErrSessionSettingsDiscarded (timeouts dropped) and ErrNoSessionAffinity (connection moved), reserves the exact ErrNoSessionAffinity result for the tested PgBouncer and Supavisor fixtures, and says a pooler that passes is still unsupported. The statement-mode row now records its refusal with ErrNoSessionAffinity (TestNewPoolRefusesStatementPooling). The README bullet drops the unconditional claim. PR body updated to match.
C2	Keep both typed causes visible so callers can surface the right remedy	fixed	Same fix as C1-F1: both sentinels are named, with when each is returned.
C3	Qualify the documentation and preserve both refusal causes	fixed	Same fix as C1-F1.

The "Verified" notes in C1 and the adoption note in C2 need no action.

Source: block/pg-sprite#261, review comments 6433948287 and 6433948606 and review 5662302050 at head ba48f08f; fixes in the follow-up commit