Careful, correctness-focused rework of the CDC ack machinery. The DeferredAck split (SnapshotCopyBarrier / Stream{MayDefer,RequireDurable}) and WatcherReceiver::{Fast,Strict} cleanly encode the durability contract that was missing before. The unreachable! guards on the snapshot barrier — plus the explicit #[should_panic] test — are the kind of belt-and-braces I like to see on a fix of this class.

Verdict: COMMENT — the design is right and the tests are substantial. Three items should be resolved before merge (see below); once they are, this is an approve.

Blocking-ish

1. CI is red on main check-run. 5937a719 fails the pre-commit run --all-files step (clippy/fmt). Needs a green build before merge.

2. New integration tests are not #[ignore]d, but the older CDC tests are. Every pre-existing test_cdc_* in crates/adapters/src/integrated/postgres/test.rs has #[ignore] — the new ones (test_cdc_ft_streaming_ack_latency_and_slot_advance, test_cdc_ft_stream_ack_flushes_below_min_batch, test_cdc_ft_accepted_streaming_events_replay_after_restart, both test_cdc_discard_*_on_startup, test_cdc_ft_terminal_catchup_ack_waits_for_checkpoint, test_cdc_ft_snapshot_barrier_covers_buffered_tail) do not. Two concerns:

• These tests hit a live Postgres with wal_level=logical (the CI job sets POSTGRES_URL, but pre-existing tests are still ignored — was that just historical, or is logical not guaranteed?). If Postgres isn't logical everywhere, this breaks non-CI cargo test.
• The tests carry bounded sleeps up to 5s and a NEGATIVE_OBSERVATION_MS = 10 * STREAMING_ACK_HOLD_MS window, run on a shared runner with sidecars. PR_REVIEW_RULES.md §4 (flakiness zero tolerance): please confirm these are stable across many CI runs, or make them #[ignore] like their siblings and run them in a separate manual/nightly lane.

3. Unbounded rollback loop in discard_matching_table_errors.

for table_id in errored_table_ids {
    loop {  // no iteration bound
        let Some(state) = store.get_table_state(table_id).await? else { break; };
        if !should_discard_table_error(&state, discard_all) { break; }
        store.rollback_table_state(table_id).await ...
    }
}

The comment says "Errored states can stack, so keep rolling back until something else surfaces." Fair intent, but if a race or bug re-inserts the sentinel error faster than we can drain it, startup blocks forever with no diagnostic. NASA Power of Ten #2 (bounded loops). Cap the inner loop (say, 32) and log a warning if the cap trips.

Design / correctness — verified

• Snapshot terminal barrier is queued behind all snapshot data before being completed (FelderaDestination::ack_snapshot_copy + DeferredAck::complete()), and cannot be Accepted (enforced statically). test_cdc_ft_snapshot_barrier_covers_buffered_tail restarts with send_snapshot: true and asserts the whole snapshot re-emerges from checkpoint — exactly the property the PR title promises. Good.
• Streaming acks: MayDefer → Accepted after streaming_ack_hold_ms; RequireDurable never times out. Terminal catchup batch is RequireDurable, so etl cannot advance sync_done/ready until Feldera checkpoints. test_cdc_ft_terminal_catchup_ack_waits_for_checkpoint covers this.
• FT strict mode watches total_checkpointed_steps, non-FT fast mode watches total_completed_steps. Slot never advances past durable data under FT. Correct.
• Shutdown ordering: shutdown_etl_pipeline() fires before abort_completion_watcher() on Terminated and TableErrorMonitor paths, so etl sees shutdown before it sees dropped acks and won't persist the sentinel error. That's the "fix shutdown" half of the title — it holds.
• Backwards compatibility: ack semantics change silently, but the old behavior was unsafe (ack could race checkpoint). A compat flag would perpetuate the bug; the right knob is streaming_ack_hold_ms, which is well-documented.

Config surface — good

streaming_ack_hold_ms = 2000, discard_shutdown_errors = true, discard_table_errors = false. Defaults are durability-first. Validator rejects streaming_ack_hold_ms = 0 with a clear message. All three appear in openapi.json and the docs page. discard_table_errors doc calls out the primary-key requirement — nice touch.

Docs

postgresql-cdc.md is clear and mostly follows ENGLISH.md. Two small suggestions:

• The streaming_ack_hold_ms table cell is very long. Consider moving the terminal-batches / snapshot-catchup nuance into the "Resume behavior" section and leaving a one-sentence summary in the table.
• "Do not manually delete only the replication slot or only the persisted etl state" — good warning, but consider linking it to a Troubleshooting section (even a stub) since operators tend to reach for exactly that when things wedge.

Commits

• 5ad7d13c fix(postgres-cdc): gate acks on durability; fix shutdown — imperative, signed-off, body explains the why. No AI trailers. Good.
• 5937a719 [ci] apply automatic fixes — signed-off by feldera-bot. Fine.

Nits

• PendingAcks::step_at_flush comment: "The flushed data lands in the next step" — could be explicit that "next" means the completion frontier must exceed step_at_flush, i.e. total_completed_steps ≥ step_at_flush + 1.
• The #[cfg(test)] DeferredAck::Test variant is a nice testability affordance, but its stream: bool, may_defer: bool combo can encode illegal states (stream=false, may_defer=true). Not user-facing, so fine, but a small TestAckKind enum would be tidier.

Nothing here is a hard block on the fix itself — it's the CI red, the ignore/flakiness question, and the bounded loop. Address those and I'll approve.

Maybe some of this is standard terminology, but I don't know this terminology. I think you need to explain better and have some sections with definitions and a description of the entities involved and how the ensemble works.

Re-review of 19b8d33 (squashed to a single commit since my 2026-07-16 COMMENT).

Blocker follow-up:

1. Unbounded rollback loop — addressed. discard_matching_table_errors now bounds each table at MAX_ERROR_ROLLBACKS_PER_TABLE and falls back to update_table_state(TableState::Init) (re-copy from scratch) with a warn! when the cap is hit. Exactly the shape I asked for. Good.
2. #[ignore] inconsistency on new CDC integration tests — not addressed here (the new test_cdc_ft_streaming_ack_latency_and_slot_advance, test_cdc_ft_stream_ack_flushes_below_min_batch, test_cdc_ft_accepted_streaming_events_replay_after_restart, test_cdc_ft_terminal_catchup_ack_waits_for_checkpoint, test_cdc_ft_snapshot_barrier_covers_buffered_tail all run in CI while the older sibling tests are #[ignore]d). If this is intentional because CI's Postgres is guaranteed wal_level=logical now, please add a one-line comment where the ignored tests live noting the divergence. Not a merge blocker on its own — deferring to yours + swanandx's judgement on flakiness risk.
3. CI still red on 19b8d33. The main check job (run 29598068886, job 87943212985) fails at step 11 ("Don't build the webconsole for rust checks"). That was already red on 5937a71 and the squash didn't clear it. Please rebase / fix the lint or fmt before merge — CI green is a hard rule.

The durability contract itself looks unchanged from the version I already read through carefully: DeferredAck::{SnapshotCopyBarrier, Stream {durability}, Test}, WatcherReceiver::{Fast,Strict}, queued_acks: AtomicUsize gating request_step(), shutdown_etl_pipeline() before dropping pending acks, discard_shutdown_errors (default true) vs discard_table_errors (default false, opt-in destructive resnap). The crown-jewel test_cdc_ft_snapshot_barrier_covers_buffered_tail is still there. Config surface (streaming_ack_hold_ms: u64 = 2000, validator rejects 0) is documented in postgresql-cdc.md and lands in openapi.json.

Commit message imperative + explains why + signed-off, no AI trailers. Good.

Verdict: COMMENT — approving as soon as CI is green. The rollback-loop fix is exactly what I wanted; the only remaining hard blocker is CI red. Item (2) can ride if you're confident.