[Kiran01bm]

The catch-up keeps a delivered change buffered until the source's own snapshot shows its transaction committed, so a change the walsender sent before the committing backend made it visible — a synchronous-commit wait — is never discarded or completed from a read that cannot see it.

Why

PostgreSQL writes a transaction's commit record to the WAL, then waits for any synchronous standby, and only then marks the transaction visible to new snapshots. The walsender decodes the commit as soon as it is flushed, so under synchronous_commit = on (or remote_write / remote_apply) with a slow or absent standby, the stream delivers a change whose transaction no snapshot on the source shows yet — for as long as the wait lasts. Until now CO-4 recorded this as the one Open window in the copy/apply ordering: a drain in that window could discard an uncut key's change (the copier's later chunk read would not see it either, if it ran before visibility) or complete a moved image from a source row that does not yet show the move. The engine already judges an in-flight chunk by holding its changes back; the same rule closes this window once the drain knows which delivered transactions the server has made visible.

What

• pkg/decode: every ChangeEvent carries Xid, the transaction's id from its BEGIN.
• pkg/applier/snapshot.go: Snapshot (xmin, xmax, in-progress xids, parsed from pg_current_snapshot()), ReadSnapshot on the pool, Visible(xid) widening the stream's 32-bit xid to the 64-bit id nearest xmax, AllVisible for a drain under a lock every writer has already released; the zero Snapshot shows nothing visible.
• Buffer: each Entry carries the xid of the latest event merged into it (a key move stamps both halves, a delete its marker, the flush's replacement marker the image it replaced). Drain(pos, snap) treats an entry whose xid the snapshot does not show committed as in flight — kept buffered, counted in Batch.Invisible / Status.Invisible, its pair deferred with it under the existing three-way rule.
• Catchup: reads the snapshot on the pool before the Position read, and only when the buffer holds anything; visibility is monotone, so a change held this cycle is judged again next cycle.
• pkg/schemachange/runner_drain.go flushAll: drains under AllVisible — the swap's ACCESS EXCLUSIVE lock cannot be granted while a writer still holds its locks, and a writer holds them until its commit is visible.
• Tests: Snapshot parsing and Visible at xmin/xmax/in-progress/wraparound (unit); Drain holding an invisible entry and its pair (unit); TestCatchupWaitsForACommitTheServerHasNotMadeVisible (integration, PG 14–18): a server with synchronous_standby_names naming a standby that never connects, a writer under synchronous_commit = on whose UPDATE is delivered and stays buffered with Invisible == 1, nothing applied, the slot unconfirmed past it; pg_cancel_backend ends the wait and the change lands and converges.
• Docs: CO-4 in docs/invariants.md moves the window from Open to Enforced and adds the test obligation; docs/copy-and-swap-design.md package map; docs/schemabot-integration.md no longer calls the window open.

Not in this PR: the primary-key move / unique-value collision CO-4 still lists as open belongs to #239.

Before / after

Table orders(id bigint PRIMARY KEY, amount integer), every key already landed in the shadow. A writer session with synchronous_commit = on runs UPDATE orders SET amount = 99 WHERE id = 2 while the configured standby never acknowledges, so the backend sits in its commit wait. The WAL is flushed, so the walsender delivers the UPDATE; no snapshot on the source shows the transaction yet.

Before
┌────────────────────────────────────┐    ┌──────────────────────────────────────┐
│ stream delivers UPDATE id=2        │    │ source, as any snapshot sees it:     │
│ (xid 731, commit flushed)          │    │ id=2 amount=10 — xid 731 not visible │
├────────────────────────────────────┤    ├──────────────────────────────────────┤
│ Drain judges id=2 against Position │    │ a completion or chunk read taken now │
│ only: landed → flush now           │───▶│ returns amount=10 for id=2           │
└────────────────────────────────────┘    └──────────────────────────────────────┘
          │
          ▼
a whole image flushes correctly; a marker-bearing image completed from this
read, or an uncut key's change discarded for a chunk read taken in the gap,
lands as amount=10 — the change is lost, and the window is recorded as open.

After
┌────────────────────────────────────┐    ┌──────────────────────────────────────┐
│ stream delivers UPDATE id=2        │    │ pg_current_snapshot() on the pool:   │
│ (xid 731, commit flushed)          │    │ 700:731: — 731 is at xmax, not yet   │
├────────────────────────────────────┤    │ complete                             │
│ Drain: entry xid 731 not visible → │    ├──────────────────────────────────────┤
│ in flight; Batch.Invisible = 1;    │    │ Applied 0, Buffered 1, Invisible 1,  │
│ nothing flushed, slot unconfirmed  │    │ Confirmed < Delivered                │
│ past it                            │    │                                      │
├────────────────────────────────────┤    ├──────────────────────────────────────┤
│ standby answers (or the wait is    │    │ next snapshot: 700:732: — 731 is     │
│ cancelled): backend makes xid 731  │───▶│ complete; Drain flushes id=2; the    │
│ visible, releases its locks        │    │ shadow reads amount=99; converged    │
└────────────────────────────────────┘    └──────────────────────────────────────┘

Under the swap's lock the same drain runs with AllVisible: the lock was granted only after every writer released its locks, which happens after its commit is visible, so there is nothing left to wait for.

🤖 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-24T18:08:50.126622Z We couldn’t complete this review. Comment @codex review to try again.	37050c1	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 performed by Jash's AI agent at head 37050c168e0e6784f2caddbde0aa0af6481ff264.

Verdict: no actionable correctness defect found in the reviewed delta; verification is limited below. Reading the source snapshot before the copier's position preserves the ordering needed to discard an uncut key, while an invisible transaction now stays buffered and continues to bound confirmation. The swap uses the explicit all-visible mode only at its documented writer-excluding lock boundary.

Findings

No new findings. The decoder stamps events from BEGIN; inserts, overlays, move halves, deletes, and replacement delete markers retain the latest transaction ID. The drain treats invisible entries as in flight before pair deferral, and the zero snapshot keeps entries buffered. Snapshot tests cover both horizons, in-progress transactions, epoch boundaries, and malformed input. The blocked synchronous-commit integration test asserts buffering, zero application, and confirmation behind delivery before cancellation, then convergence afterward. Existing tests retain their assertions while explicitly opting into all-visible mode.

Verified and limitations

Read the description and complete PR diff, including snapshot parsing/widening, transaction propagation, drain pairing, confirmation-related assertions, and the swap call sites; all 16 reported checks succeeded, including build, unit tests, PostgreSQL 14–18 suites, Supabase compatibility, and the built-artifact smoke test. No internal links or details found in the reviewed title/body/diff or this comment. No local build or tests were run. Direct GitHub API reads failed, so current-head repository guidance, surrounding source, and inline review threads were unavailable; I could not independently verify the unchanged pair-deferral, confirmation, or lock-acquisition implementations. The primary-key/unique-value collision described as remaining open is an existing limitation outside this delta.

This review was generated by Codex (GPT-6.1 Sol).

[JashLal]

🤖 Product review at 37050c1, performed by Jash's AI agent.

OSS adoption: This makes synchronous-replication deployments safer without adding configuration. The Invisible status counter distinguishes waiting for source commit visibility from waiting for a copier chunk, and the snapshot API documents the ordering and the restricted use of AllVisible. No adoption blocker found in the reviewed change.

SchemaBot integration: The wait stays inside the catch-up loop, preserving buffered changes and withholding confirmation rather than reporting successful application. Snapshot-read errors propagate with source-table context; malformed snapshots retain the typed invariant error. Consumers can inspect Status.Invisible without parsing error text. No new integration blocker found in this delta; unchanged consumer wiring was unavailable for inspection.

This review was generated by Codex (GPT-6.1 Sol).

[JashLal]

🤖 Approving on Jash’s behalf after the reviews at 37050c1 above. No new defect was established in the changed visibility and buffering logic; the review discloses remaining source and local verification limits.

[Kiran01bm]

🤖 Adversarial review response — created by Kiran's code review agent (Amp, Claude Opus 4.6) — pull/241

No defects were found, so nothing changes in this PR. Two caveats are real follow-ups outside its scope.

#	Finding	Status	Explanation
C2-F1	Consumer wiring for Status.Invisible was not inspected	deferred	Tracked as an internal follow-up: Status.Invisible is visible only on applier.Catchup.Status(). schemachange.Runner and progress.Work do not expose it, so if a synchronous standby stays stuck, awaitCaughtUp fails with a bare context error that does not say what it was waiting on.
C1-F2	The primary-key move / unique-value collision in CO-4 is still open	deferred	Tracked as an internal follow-up: #239 handles it. The collision exists without this change and is unrelated to the commit-visibility window.

• C1-F1 (pair deferral, confirmation, and lock acquisition could not be checked) and the verification limits in review 5640877415: no action. Checked against the code: Drain marks an invisible entry in flight before deferPairs. Both halves of a key move carry the same xid. Invisible entries call trackOldest, so confirmBound stays below them. PostgreSQL releases a writer's locks only after the commit becomes visible, which is what makes AllVisible sound under the swap lock.
• The "Findings: none" and "Verified" sections of C1, and the OSS-adoption and integration confirmations in C2: no action.

Source: block/pg-sprite#241, review comments 6391741479 and 6391742898 and review 5640877415 at head 37050c16