[Kiran01bm]

The copier runs a chunk again, after a bounded wait, when its insert collides with a shadow row on a unique index or exclusion constraint, instead of stopping the copy on the first 23505.

Why

The shadow is built LIKE … INCLUDING ALL, so every unique index on the source exists on the shadow while the copy runs. The copy and the catch-up write the shadow at the same time: a source transaction that moves a unique value from a landed row to a row in a chunk still in flight leaves the shadow holding the value under the landed key until the applier flushes that change. If the chunk's insert reaches the shadow first, the server refuses the whole chunk with 23505, and until now the copy ended there — a correct, converging race ended the schema change, and the convergence tests for unique-key swaps had to start their load only after the copy had landed every key.

The resolution is the copier's, not the catch-up's: each chunk attempt is its own transaction with its own snapshot, so once the flush has committed, a fresh attempt reads the moved rows as the source now holds them and lands.

What

• pkg/copier/retry.go: copyUntilClear runs copyChunk until it commits; on 23505 / 23P01 (matched by SQLSTATE) it counts the collision, waits CollisionBackoff × attempt (context-aware), and tries again; after CollisionAttempts collisions it returns the server's *pgconn.PgError wrapped with the chunk bounds and the attempt count, chunk left in flight. Any other error is the first attempt's error, unretried. Collisions() exposes the count.
• Options.CollisionAttempts / Options.CollisionBackoff (defaults 10 and 250 ms; validate refuses attempts below one and a negative backoff).
• Chunk-time feedback to the chunker is the committed attempt's duration, not the sum of the attempts.
• Tests: SQLSTATE classification and option defaults/validation (unit); a copier integration test that holds a colliding shadow row across several attempts, then removes it and sees the chunk land, and one where the row outlasts every attempt and the copy stops with the server's error, no refusal cause; TestCatchupConvergesUnderUniqueKeyMovesDuringTheCopy in pkg/applier, unique-key swaps running from before the first chunk is claimed until after the copy, converging; and TestCatchupConvergesWhenTheCopierCollidesWithAnUnflushedMove, which forces the copier/applier race on every run: with the catch-up's flushes paused, a source transaction moves a unique value from a landed key to a key not yet copied, the copier's chunk collides, the flushes resume, the retried chunk lands, and the shadow converges.
• Docs: CO-4 in docs/invariants.md marks the copier's side Enforced and narrows the Open text to the one case this does not resolve (below); docs/copy-and-swap-design.md package map and pkg/copier/doc.go.

Not in this PR: a primary-key move whose row also carries a unique secondary value, from a landed key to a key in flight. The drain's pair rule defers the old key's delete until the chunk lands, and the chunk cannot land while the old key's row holds the value, so the two wait on each other until the attempts run out. TestCatchupConvergesUnderKeyMoves keeps its load after the copy and says why; this is tracked as an internal follow-up.

Before / after

Table seats(id bigint PRIMARY KEY, slot integer UNIQUE). The copier is reading chunk [1, 100]; key 2000 has already landed with slot = 15. A source transaction moves slot 15 from row 2000 to row 42 (and gives 2000 a fresh slot), committing while the chunk is in flight. Row 2000's half of the change is for a landed key, so the applier can flush it; row 42's half waits for its chunk.

Before
┌─────────────────────────────┐    ┌──────────────────────────────────────┐
│ copier, chunk [1, 100]      │    │ shadow                               │
│ reads source: row 42 has    │    │ row 2000: slot = 15 (flush not yet   │
│ slot = 15                   │    │ committed)                           │
│ INSERT … ON CONFLICT (id)   │───▶│ unique index on slot refuses:        │
│ DO NOTHING                  │    │ SQLSTATE 23505                       │
└─────────────────────────────┘    └──────────────────────────────────────┘
          │
          ▼
copy stops: "duplicate key value violates unique constraint seats_slot_key";
the chunk stays in flight and the schema change ends here.

After
┌─────────────────────────────┐    ┌──────────────────────────────────────┐
│ attempt 1: same insert      │───▶│ row 2000 still slot = 15 → 23505     │
│ collision counted; wait     │    │                                      │
│ 250 ms                      │    │ applier flushes row 2000 → slot = 77 │
├─────────────────────────────┤    ├──────────────────────────────────────┤
│ attempt 2: new transaction, │    │ slot 15 is free; chunk [1, 100]      │
│ new snapshot, re-reads the  │───▶│ lands with row 42 at slot = 15       │
│ source                      │    │                                      │
└─────────────────────────────┘    └──────────────────────────────────────┘
          │
          ▼
chunk landed; row 42 now reads as landed and its own change flushes on the
next cycle. Had the row kept slot 15 across all 10 attempts, the copy would
stop with the same 23505, wrapped: "chunk [1, 100] collided with a shadow
row on each of 10 attempts; the row holding its value was not removed
between them".

🤖 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-27T20:45:02.766099Z We couldn’t complete this review. Comment @codex review to try again.	8d35e2e	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. Reviewed at head 8d35e2e.

Verdict: no correctness blocker identified in the diff; one optional coverage improvement. The retry remains scoped to the two collision SQLSTATEs, preserves the server error on exhaustion, and lands a chunk only after success. The documented primary-key-move limitation remains explicit.

Findings

1. (optional coverage) The new load test can pass without a collision. pkg/applier/catchup_integration_test.go:474 logs Collisions() but does not establish that the applier/copier race occurred. The dedicated copier test forces 23505, while the convergence test currently verifies general concurrent-load convergence.

Action items

1. (Finding 1, optional) Add a synchronized applier/copier case that forces a landed-row/in-flight-chunk collision, then asserts a collision and final convergence. Avoid making the probabilistic load test flaky with an unconditional positive-count assertion.

Verified (tried to break, couldn't)

Reviewed the full eight-file diff against the exact live head/file metadata; checked SQLSTATE classification, attempt count, context-aware wait, error wrapping, position/row-count updates and successful-attempt timing; no test removals or weakened assertions; GitHub reports all 16 checks successful. Exact-head surrounding source and AGENTS.md were unavailable in this environment, so no local build/tests or helper-source verification were performed.

This review was generated by Codex.

[JashLal]

🤖 Two-lens review at 8d35e2e, performed by Jash's AI agent.

OSS adoption ease: Automatic bounded retries improve the default experience for unique-value moves. The final error retains the database detail plus chunk bounds and attempt count; the remaining primary-key-move limitation is documented. No adoption blocker found in the diff.

SchemaBot integration: %w preserves errors.As access to *pgconn.PgError, and the public attempt/backoff options and concurrent-safe collision count provide usable library seams. I could not inspect the consuming engine at its current head, so this assesses the API change rather than proving end-to-end integration.

This review was generated by Codex.

[JashLal]

🤖 Approving on Jash's behalf after the review at 8d35e2e. No concrete correctness blocker found in the full diff; the optional deterministic inter-component collision test is recorded in the review. Exact-head unchanged helpers and repository instructions were inaccessible, so no local build or tests ran. Approval does not certify those unverified paths. Generated by Codex.

[Kiran01bm]

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

The one finding is fixed: a new convergence test forces the copier/applier collision on every run instead of leaving it to the load's timing.

#	Finding	Status	Explanation
C1-F1	The load test can pass without a collision; add a synchronized applier/copier case	fixed	New TestCatchupConvergesWhenTheCopierCollidesWithAnUnflushedMove: the catch-up's flushes are paused, and once key 10 lands a source transaction moves uniq 10 to key 1000. The chunk carrying key 1000 then collides on every run. The test waits for Collisions() >= 1, resumes the flushes, and asserts the copy completes and the shadow converges. It fails when the copier stops retrying collisions. The load-driven test keeps logging Collisions() without asserting a count, as the review advised.
C2	Two-lens notes: retries improve the default, error detail and %w are preserved	no action	Confirmations only.
Review	Approval	no action	—

Source: block/pg-sprite#239, review comments 6454179618 and 6454181892 and review 5674719763 at head 8d35e2e1; fixes in the follow-up commit.