fix(server): make inbox ACK a delta commit instead of a full-prefix scan

[bestony]

Fixes #1671 (PERF-008).

Problem

ackThroughEntryIdForBoundAgents selected and locked every notify row of the (inbox, chat) partition from the beginning through the ACK cursor — including already-acked rows that are never collected — and materialized them in JavaScript just to compute the committable delta. Duplicate ACKs repeated the same scan. Per-ACK cost was O(chat history), i.e. O(N²) work over a chat's lifetime, with lock contention widening as history grew.

Fix

The commit is now a delta scan: only rows still in flight are ever read or locked, and acked history is never touched.

• Gap probe — locks at most one never-delivered pending row (status = 'pending' AND delivered_at IS NULL). FOR UPDATE preserves the old blocking semantics: if a concurrent claim holds that row mid-flight, we wait and PG re-evaluates the predicate on the committed version, so a row that just became delivered stops counting as a gap instead of racing the ACK.
• Promotion — two targeted UPDATEs (delivered rows, and recovery-reset rows identified by their retained deliveredAt) merged into a single ascending ackedEntryIds delta. Result contract (disposition, ackedCount, throughEntry) is unchanged.
• High-water ledger for free — acked is terminal and deliveredAt is never cleared once set (recovery resets status only), so id <= cursor AND status <> 'acked' is exactly the uncommitted delta and no row can re-enter the gap set behind a committed cursor; bigserial ids keep late inserts strictly above it. A duplicate ACK therefore finds an empty delta instead of rescanning the prefix.
• Matching partial index — idx_inbox_unacked_cursor on (inbox_id, chat_id, notify, id) WHERE status IN ('pending','delivered') serves every ACK statement (gap probe, both promotions, silent drain), the prefix-claim scan, and delivered-reset recovery scans. The index only contains in-flight rows, so it stays small and hot regardless of how much acked history a chat accumulates. Migration 0091_short_johnny_storm.sql.

Benchmark (large histories)

Disposable Postgres 16, one chat, in-flight delta of 2 delivered + 1 recovery-reset + 3 silent rows, 50 iterations, per-ACK latency:

acked history	old p50	old p95	new p50	new p95
1,000	3.46 ms	4.99 ms	2.12 ms	3.05 ms
10,000	18.30 ms	21.63 ms	1.98 ms	2.62 ms
100,000	170.69 ms	199.51 ms	2.92 ms	8.03 ms

Old cost grows linearly with history (→ quadratic lifetime work); new cost is flat at the in-flight window (~58× at 100k).

Tests

• All existing ACK/delivery/recovery semantics tests pass unchanged (packages/server: 253 files / 2853 tests green).
• New: delivered + recovery-reset rows interleaved in one ACK come back as a single ordered delta with accepted_from_pending; committed history is neither re-reported nor has its ackedAt rewritten by later ACKs.
• New index-shape pin: idx_inbox_unacked_cursor exists with the exact partial predicate, and an EXPLAIN-based regression test (acked-heavy distribution, sibling indexes dropped inside a rolled-back transaction, seq scans priced out) fails if any ACK statement's predicate drifts outside the partial index.
• pnpm check and pnpm typecheck clean on touched files (pre-existing unrelated lint findings in packages/client/apps/cli remain untouched).

QA

This touches the WS/inbox path, so formal QA is warranted per repo policy — matching case: packages/qa/cases/cross-surface/authenticated-ws-inbox-delivery.md (delivery + ACK-through drain after session start). Behavior contract of inbox:ack frames is intentionally unchanged.

[baixiaohang]

Recommendation: request changes

• Rationale: the split promotion can miss a recovery-reset row that is reclaimed during the ACK transaction, breaking the contiguous-prefix contract; the large-table index migration also needs a production-safe rollout path.

Risk level: B-high

• Path baseline: packages/server/** plus a database index migration -> B-high
• Semantic lift: none

PR summary

• Author / repo: bestony / agent-team-foundation/first-tree
• Problem: ACK-through currently rescans and locks the complete notify history for a chat, so long-lived chats pay history-sized work and increasingly broad lock contention on every ACK.
• Approach: replace the prefix materialization with an in-flight gap probe and targeted promotions, use terminal acked rows as the commit ledger, and add a partial index containing only pending/delivered rows.
• Impacted modules: server inbox ACK/recovery service, inbox schema and migration, WS delivery tests, index-plan regression tests.

Review findings

❌ 1. The two promotion statements do not remain atomic with the allowed pending -> delivered reclaim transition. A recovery-reset row (status = 'pending', delivered_at IS NOT NULL) is intentionally excluded from the gap lock. If a claim changes an earlier such row to delivered after the delivered promotion at lines 644-648 and before the reset-pending promotion at lines 649-653, it matches neither statement. The transaction can then ACK a later cursor and return success while the earlier row remains delivered, persisting the exact non-contiguous prefix this helper is meant to prevent. Please keep both committable states under one locking/update guard (or otherwise prevent the reclaim transition from crossing the two statements) and add a deterministic ACK-vs-reclaim regression test. [R4 / packages/server/src/services/inbox.ts:644]

❌ 2. The migration builds this index with plain CREATE INDEX and no IF NOT EXISTS. Drizzle applies migrations transactionally, so production cannot prebuild it with CREATE INDEX CONCURRENTLY; applying the migration directly will scan the full, write-heavy inbox_entries table while blocking fan-out/ACK writes. That is especially risky for the large histories this fix targets. Please provide the repository's established concurrent-prebuild + idempotent migration path (see 0064_superb_betty_brant.sql) or an equivalent production-safe rollout. [R5 / packages/server/drizzle/0091_short_johnny_storm.sql:1]

✅ 3. The delta predicates and EXPLAIN-based index-shape pin clearly protect the intended “acked history is never rescanned” performance property once the concurrency and rollout issues are resolved.

Action taken

• Submitted request changes.

[yuezengwu]

Review summary

The goal is to reduce ACK-through from a full historical prefix scan to an in-flight delta commit. The core change replaces prefix materialization with a pending-gap probe plus targeted promotions, and adds a partial (inbox_id, chat_id, notify, id) index over pending/delivered rows. The direction and index-shape regression coverage are sound, but this head has two blockers.

1. The split promotion can violate the contiguous-prefix contract under a concurrent reclaim. A recovery-reset row (status = 'pending', delivered_at IS NOT NULL) is intentionally excluded from the gap lock. If another claimant moves an earlier such row to delivered after the delivered update at packages/server/src/services/inbox.ts:644-648 and before the reset-pending update at :649-653, it matches neither statement. The ACK can then commit a later cursor and return success while the earlier row remains delivered. Please place both committable states under one atomic locking/update guard, or otherwise fence the pending -> delivered transition, and add a deterministic ACK-vs-reclaim regression test.

2. The new large-table index lacks the repository's production-safe rollout path. packages/server/drizzle/0091_short_johnny_storm.sql:1 uses plain CREATE INDEX without IF NOT EXISTS. Because Drizzle runs migrations transactionally, operators cannot prebuild the index concurrently and let the migration adopt it; applying this migration directly can block writes while scanning the write-heavy inbox table. Please follow the established concurrent-prebuild plus idempotent migration pattern in 0064_superb_betty_brant.sql (or provide an equivalent safe rollout). This database change should receive explicit maintainer/operations review before release.

I reviewed the diff and surrounding ACK/delivery/recovery paths. Per review scope, I did not run tests or formal QA.