fix(inbox): bound ACK confirms and current-socket prefix admission

[yuezengwu]

Summary

• [PERF-015][High] Inbox ACK confirmation retries never stop #1678: Confirmed Client ACKs send at most twice on the same socket (initial + one 3s retry). Hitting a second timeout or the 1024 pending-ACK cap fail-closes once (1011 / inbox ack timeout), rejects every pending ACK, and floors reconnect at RECONNECT_MAX_MS. Socket close now clears pending ACKs so they cannot flush after the next agent:bound.
• [PERF-007][High] ACK-through is not bound to a delivery attempt #1670: ACK-through requires an in-process snapshot of the current socket's in-flight notify ids for that chat. The snapshot is the complete per-chat bucket already bounded by maxInFlightPerAgentChat (schema 1–1024), not a hardcoded 8. Any delivered / reset-pending notify row that would become acked must be in that snapshot; otherwise the transaction returns not_found_or_not_bound and leaves recovery/redelivery to bind-reset. Duplicate already_acked still succeeds without a snapshot, but no longer drains new silent rows.
• Lost confirm after commit: Fail-close closes the socket before inbox:recover can send, and an already-acked row is not redelivered. Chats whose ACK-through failed keep a narrow rebind-reconcile mark; AgentSlot calls reconcileAckSettlementAfterBind on this agent's agent:bound (not via runtime-proof or Reset-fence sets). unackedOutstanding === 0 is Server-authoritative settlement and clears ledger/debt; omitted or > 0 stays conservative. A failed recover keeps the mark for the next bind.
• No schema, migration, WS field, capability, lease/generation, or SessionManager change. Old Client / new Server and the reverse stay rolling-compatible. Fail-safe is reject + redelivery, not a mis-ACK.

Fixes #1678
Fixes #1670

Test plan

• Client Vitest: first timeout retries once with the same ref; second timeout sends no third frame, rejects all pending ACKs, closes once; cap hits the same fail-close; accepted on send 1/2 does not close; ordinary close does not flush old ACKs after rebind; legacy fire-and-forget unchanged
• Client recover settlement: same-socket unackedOutstanding: 0 still clears ledger; production fail-close timing (first recover rejects as socket unavailable, then rebind reconciliation returns 0) clears ledger/debt/activation without a second provider/ACK; 1 and omitted retain terminal and converge on redelivery re-ACK
• AgentSlot agent:bound wiring: this agent triggers ACK settlement reconcile; other agents do not; runtime-proof recovery is not reused
• Server service Vitest: full current prefix ACKs; missing earlier committable row rejects with DB unchanged; reset + redelivery + rebuilt snapshot ACKs; already_acked ignores snapshot and does not drain leftover silent rows; authorized notify commit still drains silent
• WS integration: current-socket delivery ACKs and frees the in-flight slot; leftover delivered / reset-pending prefix not in the current socket set is rejected
• Fake coordinator harness: routeHarness inbox cap override 12, mock 9 deliveries, ACK snapshot is the complete 9 ids (no second live Fastify app)
• ws-client-edge ACK spies: delivered entry snapshot is [entryId]; synthetic ids with no in-flight owner snapshot []
• pnpm check (changed files clean; repo has pre-existing warnings)
• pnpm typecheck
• QA live: two branches in packages/qa/cases/cross-surface/inbox-ack-confirm-timeout-and-bind-reset.md
  • response-loss (request committed): row stays acked, no redelivery / no second provider entry; first recover may fail on the dying socket; after agent:bound recover unackedOutstanding === 0 Client local unsettled/recovery state clears
  • request-loss: bind-reset/redelivery, later ACK only the current-socket delivery set
    Both: initial + one retry, one 1011 close, no third same-socket ACK

[baixiaohang]

Recommendation: request changes

• Rationale: The bounded retry path cannot converge when the ACK commits but its confirmation is lost, and the server snapshot rejects valid prefixes when the supported per-chat window is configured above eight.

Risk level: A

• Path baseline: includes packages/client/** -> A
• Semantic lift: touches WebSocket ACK/recovery and inbox state core logic; no downgrade from A

PR summary

• Author / repo: yuezengwu / agent-team-foundation/first-tree
• Problem: Prevent a non-responsive ACK-confirm path from retrying forever, and prevent one socket from acknowledging inbox work outside its own delivery set.
• Approach: Bound confirmed ACKs to two sends followed by reconnect recovery, and require every committable notify row in an ACK-through prefix to appear in the current socket's in-flight snapshot.
• Impacted modules: Client WebSocket connection, Server inbox delivery coordinator/service, regression tests, and cross-surface QA case

Review findings
❌ 1. The lost-confirm branch has no convergence path after the Server has already committed the ACK. Both ACK sends can reach ackThroughEntryIdForBoundAgents, commit the row, and lose only their inbox:ack:accepted responses; the second timeout then rejects the Client promise, so InboxDeliveryCoordinator retains the terminal entry as recovery debt. The next bind cannot redeliver that row because bind reset only moves delivered rows and this row is already acked. The added live case expects the row to be unacked/redelivered after dropping only the confirm frame, which is therefore impossible. Reconcile authoritative Server settlement after rebind (the protocol already exposes unackedOutstanding, and exact settled ids are another option) and cover both request-loss and response-loss branches. [R4/R5 / packages/client/src/runtime/client-connection.ts:995]

❌ 2. The current-socket snapshot hard-codes eight ids even though maxInFlightPerAgentChat is a supported 1..1024 configuration and the repository explicitly tests an override of 12. With a legal window above eight, an ACK-through for a longer terminal prefix sees more than eight committable rows but receives only the first eight snapshot ids, so it is rejected as not_found_or_not_bound even though every row came from this socket; recovery can repeat the same failure. Derive the bound from the configured window (or pass the complete already-bounded bucket) and add a cap-above-eight ACK-through regression. [R1/R5 / packages/server/src/api/agent/ws-client/inbox-delivery.ts:157]

Action taken

• Submitted request changes

[baixiaohang]

Recommendation: request changes

• Rationale: The two original design findings are resolved on this head, but the new cap-above-eight regression does not type-check, so the PR is not buildable yet.

Risk level: A

• Path baseline: includes packages/client/** -> A
• Semantic lift: WebSocket ACK/recovery and inbox state core logic; no downgrade from A

PR summary

• Author / repo: yuezengwu / agent-team-foundation/first-tree
• Problem: Bound a non-responsive ACK-confirm path so it cannot retry forever, while preventing a socket from acknowledging inbox work outside its own delivery set.
• Approach: Send confirmed ACKs at most twice before a controlled reconnect, and admit ACK-through only when every committable notify row belongs to the current socket's complete per-chat in-flight set.
• Impacted modules: Client WebSocket connection, Server inbox delivery coordinator/service, regression tests, and cross-surface QA case

Review findings
❌ 1. The newly added cap-12 WS regression fails the repository TypeScript check: inside the finally block, ws is declared as WebSocket | undefined, and the callback at line 669 closes over it after the if (ws) narrowing, so TypeScript reports TS18048: 'ws' is possibly 'undefined'. Capture the narrowed socket in a local constant or otherwise keep the callback on a definitely assigned value. This is directly confirmed by the current Lint & Type Check CI failure. [packages/server/src/__tests__/ws-inbox-delivery-order.test.ts:669]

✅ The previous hard-coded eight-entry snapshot is fixed by passing the complete bucket already bounded by maxInFlightPerAgentChat, with a nine-entry/cap-12 regression.

✅ The updated confirm-drop QA split now matches the inbox contract: a committed ACK remains settled and must not redeliver the provider-entered turn, while an uncommitted request remains recovery debt.

Action taken

• Submitted request changes

[baixiaohang]

Recommendation: request changes

• Rationale: The ACK/recovery design is now sound, but the current head still has stale WS edge assertions for the new required snapshot argument, and the Server suite has repeatably exhausted its 4 GB Vitest heap before it can report the test failures.

Risk level: A

• Path baseline: includes packages/client/** -> A
• Semantic lift: touches WebSocket ACK/recovery and inbox state core logic; no downgrade from A

PR summary

• Author / repo: yuezengwu / agent-team-foundation/first-tree
• Problem: Stop unbounded ACK-confirm retries without losing committed work, and prevent a socket from acknowledging inbox work outside its own deliveries.
• Approach: Bound confirmed ACK sends and reconnect on exhaustion; admit ACK-through only for the current socket's complete per-chat in-flight set; reconcile a lost confirmation only when Server recovery authoritatively reports no unacked notify rows.
• Impacted modules: Client connection and delivery coordinator, Server inbox delivery coordinator/service, WS regression tests, and cross-surface QA

Review findings
❌ 1. createInboxDeliveryCoordinator now always calls ackEntryByIdForBoundAgents(db, entryId, inboxIds, currentSocketDeliveredEntryIds), but three ws-client-edge assertions still require the old exact three-argument call. The spies hide this from TypeScript, while Vitest's toHaveBeenCalledWith will reject the additional argument once this file completes. The delivered-entry case should assert [entryId]; the two synthetic ids with no current in-flight owner should assert []. [packages/server/src/__tests__/ws-client-edge.test.ts:837, :1319, :1342]

❌ 2. The required Server job has now failed three times on this PR line (head 0569c87, head eb565fc, and the failed-job rerun) with the same ws-client-edge worker reaching the approximately 4 GB V8 heap limit, followed by ERR_IPC_CHANNEL_CLOSED. This is repeatable rather than a one-off runner failure and currently masks the ordinary Vitest report. Resolve or isolate the retained state/resource growth and produce one green full Server run before approval.

✅ The two prior design blockers are resolved: the socket snapshot uses the complete configured per-chat bucket, and unackedOutstanding === 0 is a conservative Server-authoritative settlement proof while omitted/non-zero results retain recovery debt.

Action taken

• Submitted request changes; approval remains withheld despite authorization because the exact approved precondition (green CI) was not met.

[baixiaohang]

Recommendation: request changes

• Rationale: The rebind settlement path now covers the fail-close timing correctly, but the two build blockers from the prior review remain unresolved on this head.

Risk level: A

• Path baseline: includes packages/client/** -> A
• Semantic lift: touches WebSocket ACK/recovery and inbox state core logic; no downgrade from A

PR summary

• Author / repo: yuezengwu / agent-team-foundation/first-tree
• Problem: Stop unbounded ACK-confirm retries without losing committed work, and prevent a socket from acknowledging inbox work outside its own deliveries.
• Approach: Bound confirmed ACK sends and reconnect on exhaustion; admit ACK-through only for the current socket's complete per-chat in-flight set; retain a narrow ACK-settlement marker and reconcile it after this agent rebinds.
• Impacted modules: Client connection, AgentSlot and delivery coordinator; Server inbox delivery coordinator/service; WS regressions and cross-surface QA

Review findings
❌ 1. The three stale ws-client-edge assertions reported on the prior head are unchanged. Production always supplies the fourth current-socket snapshot argument, while the tests still require exact three-argument calls at lines 837, 1319, and 1342. Assert [entryId] for the delivered-entry case and [] for the two synthetic ids with no current in-flight owner. [packages/server/src/__tests__/ws-client-edge.test.ts:837, :1319, :1342]

❌ 2. Test Server failed again on head 76d737a: the ws-client-edge worker reached approximately 4 GB and terminated with FATAL ERROR: Reached heap limit, followed by ERR_IPC_CHANNEL_CLOSED. This is now the fourth same-signature failure on the PR line and still masks the ordinary assertion report. Resolve or isolate the retained state/resource growth and provide one green full Server run.

✅ The new rebind reconciliation is appropriately scoped: ACK-confirm failures retain a coordinator-local marker, only this agent's agent:bound triggers it, an unavailable dying socket leaves the marker retryable, and unackedOutstanding === 0 remains the only Server-authoritative settlement proof.

Action taken

• Submitted request changes; approval remains withheld because required Server CI is still red and the known stale assertions remain.

[baixiaohang]

Recommendation: approve

• Rationale: Both ACK/recovery correctness issues are closed on this exact head, the implementation preserves the Server inbox as the durable source of truth, and all CI checks pass.

Risk level: A

• Path baseline: includes packages/client/** -> A
• Semantic lift: touches WebSocket ACK/recovery and inbox state core logic; no downgrade from A

PR summary

• Author / repo: yuezengwu / agent-team-foundation/first-tree
• Problem: Prevent confirmed ACKs from retrying indefinitely while ensuring a rebound socket cannot acknowledge inbox work outside its own delivery set.
• Approach: Bound confirmed ACK sends and reconnect on exhaustion; admit ACK-through only for the current socket's complete per-chat in-flight set; retain a coordinator-local settlement marker and reconcile it after this agent rebinds, accepting only unackedOutstanding === 0 as authoritative proof of settlement.
• Impacted modules: Client connection, AgentSlot and delivery coordinator; Server inbox delivery coordinator/service; WS regressions and cross-surface QA

Review findings
✅ The current-socket snapshot uses the complete configured bucket and covers a cap-above-eight case without introducing another live Server fixture.

✅ Request-loss and response-loss branches both converge without re-entering provider-visible work or silently acknowledging rows outside the current socket.

✅ Rebind reconciliation remains narrowly owned by the delivery coordinator; persistent inbox state remains Server-authoritative and no schema, lease, generation, or SessionManager coupling is introduced.

✅ The stale WS expectations are fixed, the previously repeatable Server heap failure is resolved, and the exact head is fully green.

Action taken

• Approved exact head d613b691d9be5da163da9890680a277c68040f5f.