Repository navigation
fix(sdk): recover backlog past exhausted failed messages - #730
bandzalkin wants to merge 1 commit into
Conversation
bandzalkin
left a comment
There was a problem hiding this comment.
Architectural review → fix round
Scope: Reviewed #730 from merge base 8921c8ce8e5b78d4264a5e286880df87c74dc8a9 through correction commit 49d3dff9179bb92bfa08ecbe07b816ed7f75d447, including message lifecycle, execution, regressions, operator docs, and changelog. Outcome: recover later actionable work without acknowledging an exhausted failed head.
Constraints: Platform delivery state remains authoritative; local retry exhaustion must not erase failure evidence. MessageLifecycle owns REST pagination and ordering, ExecutionContext owns eligibility, claims, stop/play, and WebSocket handoff. An incomplete listing is not proof of a drained backlog. Existing cursor and legacy-page compatibility stays intact.
Decisions: Keep the existing recovery owners. Replace three status scans and synthesized timestamp sorting with one fully paginated, oldest-first unprocessed server stream and stable deduplication. This avoids status-transition omissions and invented ordering without adding another queue or retry policy. Stop before claiming each snapshot row and hand back to the WebSocket queue at its marker; preserve later reconnect synchronization.
Findings and proof: B1 marker overrun, B2 stop-during-listing, and B3 status/order loss were confirmed, corrected, and re-probed. A reconnect regression exposed during correction was also fixed. No in-scope blocker remains. Fresh local gate: 6,327 passed / 684 skipped; focused execution suite 151 passed; all pre-commit hooks passed; both lock checks passed; baseline E2E collection 526 items / 1 skipped; markdown snippets 23 passed. Standalone runtime/REST smoke preserved A → new-user → B, executed nothing while stopped, and retained status-transition and missing-timestamp order.
Architectural verdict: APPROVED for #730 independently. No new dependency, durable format, or policy owner. Approval assumes the existing REST listing contract. Combined integration with #729 remains HELD on the separately demonstrated ACP effect-commit finding; this is not remote CI or merge approval.
Squashed from band-ai#730 at fb7403a, resolved over band-ai#753: a locally skipped /next head now hands the drain to the actionable snapshot, so band-ai#753's re-offer guard is a backstop and its wedge tests assert one /next poll; the sync point still accepts a no-progress result but only once the message is terminal. Upstream-PR: band-ai#730
Squashed from band-ai#730 at fb7403a, resolved over band-ai#753: a locally skipped /next head now hands the drain to the actionable snapshot, so band-ai#753's re-offer guard is a backstop and its wedge tests assert one /next poll; the sync point still accepts a no-progress result but only once the message is terminal. Upstream-PR: band-ai#730
Squashed from band-ai#730 at fb7403a, resolved over band-ai#753: a locally skipped /next head now hands the drain to the actionable snapshot, so band-ai#753's re-offer guard is a backstop and its wedge tests assert one /next poll; the sync point still accepts a no-progress result but only once the message is terminal. Upstream-PR: band-ai#730
…unning the turn Squashed from band-ai#760 at 896d186, resolved over band-ai#730: the fork's stale recovery already drains one actionable snapshot that ends once the room is stopped, so band-ai#760's sweep and sync guards are dropped and its stale-claim test feeds that snapshot. Upstream-PR: band-ai#760
Squashed from band-ai#730 at fb7403a, resolved over band-ai#753: a locally skipped /next head now hands the drain to the actionable snapshot, so band-ai#753's re-offer guard is a backstop and its wedge tests assert one /next poll; the sync point still accepts a no-progress result but only once the message is terminal. Upstream-PR: band-ai#730
…unning the turn Squashed from band-ai#760 at 896d186, resolved over band-ai#730: the fork's stale recovery already drains one actionable snapshot that ends once the room is stopped, so band-ai#760's sweep and sync guards are dropped and its stale-claim test feeds that snapshot. Upstream-PR: band-ai#760
Squashed from band-ai#730 at fb7403a, resolved over band-ai#753: a locally skipped /next head now hands the drain to the actionable snapshot, so band-ai#753's re-offer guard is a backstop and its wedge tests assert one /next poll; the sync point still accepts a no-progress result but only once the message is terminal. Upstream-PR: band-ai#730
…unning the turn Squashed from band-ai#760 at 896d186, resolved over band-ai#730: the fork's stale recovery already drains one actionable snapshot that ends once the room is stopped, so band-ai#760's sweep and sync guards are dropped and its stale-claim test feeds that snapshot. Upstream-PR: band-ai#760
Squashed from band-ai#730 at fb7403a, resolved over band-ai#753: a locally skipped /next head now hands the drain to the actionable snapshot, so band-ai#753's re-offer guard is a backstop and its wedge tests assert one /next poll; the sync point still accepts a no-progress result but only once the message is terminal. Upstream-PR: band-ai#730
…unning the turn Squashed from band-ai#760 at 896d186, resolved over band-ai#730: the fork's stale recovery already drains one actionable snapshot that ends once the room is stopped, so band-ai#760's sweep and sync guards are dropped and its stale-claim test feeds that snapshot. Upstream-PR: band-ai#760
Squashed from band-ai#730 at fb7403a, resolved over band-ai#753: a locally skipped /next head now hands the drain to the actionable snapshot, so band-ai#753's re-offer guard is a backstop and its wedge tests assert one /next poll; the sync point still accepts a no-progress result but only once the message is terminal. Upstream-PR: band-ai#730
…unning the turn Squashed from band-ai#760 at 896d186, resolved over band-ai#730: the fork's stale recovery already drains one actionable snapshot that ends once the room is stopped, so band-ai#760's sweep and sync guards are dropped and its stale-claim test feeds that snapshot. Upstream-PR: band-ai#760
Squashed from band-ai#730 at fb7403a, resolved over band-ai#753: a locally skipped /next head now hands the drain to the actionable snapshot, so band-ai#753's re-offer guard is a backstop and its wedge tests assert one /next poll; the sync point still accepts a no-progress result but only once the message is terminal. Upstream-PR: band-ai#730
…unning the turn Squashed from band-ai#760 at 896d186, resolved over band-ai#730: the fork's stale recovery already drains one actionable snapshot that ends once the room is stopped, so band-ai#760's sweep and sync guards are dropped and its stale-claim test feeds that snapshot. Upstream-PR: band-ai#760
Squashed from band-ai#730 at fb7403a, resolved over band-ai#753: a locally skipped /next head now hands the drain to the actionable snapshot, so band-ai#753's re-offer guard is a backstop and its wedge tests assert one /next poll; the sync point still accepts a no-progress result but only once the message is terminal. Upstream-PR: band-ai#730
…unning the turn Squashed from band-ai#760 at 896d186, resolved over band-ai#730: the fork's stale recovery already drains one actionable snapshot that ends once the room is stopped, so band-ai#760's sweep and sync guards are dropped and its stale-claim test feeds that snapshot. Upstream-PR: band-ai#760
Squashed from band-ai#730 at fb7403a, resolved over band-ai#753: a locally skipped /next head now hands the drain to the actionable snapshot, so band-ai#753's re-offer guard is a backstop and its wedge tests assert one /next poll; the sync point still accepts a no-progress result but only once the message is terminal. Upstream-PR: band-ai#730
…unning the turn Squashed from band-ai#760 at 896d186, resolved over band-ai#730: the fork's stale recovery already drains one actionable snapshot that ends once the room is stopped, so band-ai#760's sweep and sync guards are dropped and its stale-claim test feeds that snapshot. Upstream-PR: band-ai#760
fb7403a to
adebc7d
Compare
Squashed from band-ai#730 at adebc7d; preserves band-ai#753 terminal sync handling and removes a duplicate recovery wiring assertion in favor of behavioral recovery coverage. Upstream-PR: band-ai#730
Squashed from band-ai#730 at adebc7d; preserves band-ai#753 terminal sync handling and removes a duplicate recovery wiring assertion in favor of behavioral recovery coverage. Upstream-PR: band-ai#730
Squashed from band-ai#730 at adebc7d; preserves band-ai#753 terminal sync handling and removes a duplicate recovery wiring assertion in favor of behavioral recovery coverage. Upstream-PR: band-ai#730
Squashed from band-ai#730 at adebc7d; preserves band-ai#753 terminal sync handling and removes a duplicate recovery wiring assertion in favor of behavioral recovery coverage. Upstream-PR: band-ai#730
Squashed from band-ai#730 at adebc7d; preserves band-ai#753 terminal sync handling and removes a duplicate recovery wiring assertion in favor of behavioral recovery coverage. Upstream-PR: band-ai#730
Squashed from band-ai#730 at adebc7d; preserves band-ai#753 terminal sync handling and removes a duplicate recovery wiring assertion in favor of behavioral recovery coverage. Upstream-PR: band-ai#730
AlexanderZ-Band
left a comment
There was a problem hiding this comment.
Current-main follow-up
The underlying head-of-line failure remains on current main: /next continues to return the retained, locally exhausted failed head; startup stops at it, and idle reconciliation reports BLOCKED rather than listing later actionable rows. Later deliveries therefore remain unrecoverable through REST after a missed WebSocket delivery.
This PRs core contract is still needed: preserve the failed head and its diagnostics, but snapshot the complete ordered actionable stream before any status transition, then process eligible later rows without overtaking retryable or in-flight work.
The implementation cannot merge unchanged: GitHub now reports a conflict with main, whose reconciliation model has since acquired ResyncOutcome, deferred-head handling, and cursor-page APIs. The fix should be rebuilt on current main and retain this PRs regression coverage rather than resolving the old diff mechanically.
Refresh #730 onto current upstream. Collect complete unfiltered chronological pages before claims; retain failed heads, eligible delivery order, terminal WebSocket handoff and current recovery/admission ownership. Source-Base: 55c70f2 Source-Head: adebc7d Applied-Over: cd6bde7 Conflict-Notes: keep authoritative /next and stopped-room 204 gating, no filtered processing pre-sweep; reuse shared nonrunnable predicate instead of #753 NO_PROGRESS bookkeeping; fold 0537758 snapshot revision fence into explicit recovery outcomes; preserve accepted claim, pending input, acknowledgement, #776 STOP and TurnTask custody. CHANGELOG.md excluded. Verification: parent assembled HTTP-boundary/custody proof 589 passed; #761 plus refreshed #730 covers #753 no-spin behavior. Source hunk ledger retained in u3-preparation/fork. Upstream-PR: #730
adebc7d to
41e9389
Compare
Source-Base: 97f8330 Source-Head: 41e9389 Applied-Over: 97f8330 Conflict-Notes: None; exact source delta, excluding CHANGELOG.md and FORK-CHANGES.md Upstream-PR: band-ai#730
Source-Base: 97f8330 Source-Head: 41e9389 Applied-Over: 97f8330 Conflict-Notes: None; exact source delta, excluding CHANGELOG.md and FORK-CHANGES.md Upstream-PR: band-ai#730
Source-Base: 97f8330 Source-Head: 41e9389 Applied-Over: f807d62 Conflict-Notes: Over band-ai#789/band-ai#775: resync entry keeps upstream _drain_synthetic() and drops previous_id as the source does; _recover_actionable_backlog matches TurnOutcome.NOT_ACCEPTED/RETRY_LATER (renamed from BacklogProcessResult), and PAUSED settles behind the existing _stopped check like upstream's /next loop. Over band-ai#804: SessionConfig imported from band.runtime.sessionconfig; resync kit/conftest keep ReconciliationPeer beside BacklogApi. Excludes CHANGELOG.md and FORK-CHANGES.md Fork fix beyond the PR head: the in-snapshot synthetic handoff uses the entry check's rule (not during bootstrap, not while an older message is deferred), so neither bootstrap nor idle recovery stalls on a queued synthetic turn; see FORK-CHANGES.md. A deferral is released whenever its row is locally completed or permanently failed (stop/control fences, processed-ack retry), at every pass start and after every processed row. Upstream-PR: band-ai#730
Source-Base: 97f8330 Source-Head: 41e9389 Applied-Over: d7ab342 Conflict-Notes: Over band-ai#789/band-ai#775: resync entry keeps upstream _drain_synthetic() and drops previous_id as the source does; _recover_actionable_backlog matches TurnOutcome.NOT_ACCEPTED/RETRY_LATER (renamed from BacklogProcessResult), and PAUSED settles behind the existing _stopped check like upstream's /next loop. Over band-ai#804: SessionConfig imported from band.runtime.sessionconfig; resync kit/conftest keep ReconciliationPeer beside BacklogApi. Excludes CHANGELOG.md and FORK-CHANGES.md Fork fix beyond the PR head: the in-snapshot synthetic handoff uses the entry check's rule (not during bootstrap, not while an older message is deferred), so neither bootstrap nor idle recovery stalls on a queued synthetic turn; see FORK-CHANGES.md. A deferral is released whenever its row is locally completed or permanently failed (stop/control fences, processed-ack retry), at every pass start and after every processed row. Upstream-PR: band-ai#730
Source-Base: 97f8330 Source-Head: 41e9389 Applied-Over: d7ab342 Conflict-Notes: Over band-ai#789/band-ai#775: resync entry keeps upstream _drain_synthetic() and drops previous_id as the source does; _recover_actionable_backlog matches TurnOutcome.NOT_ACCEPTED/RETRY_LATER (renamed from BacklogProcessResult), and PAUSED settles behind the existing _stopped check like upstream's /next loop. Over band-ai#804: SessionConfig imported from band.runtime.sessionconfig; resync kit/conftest keep ReconciliationPeer beside BacklogApi. Excludes CHANGELOG.md and FORK-CHANGES.md Fork fix beyond the PR head: the in-snapshot synthetic handoff uses the entry check's rule (not during bootstrap, not while an older message is deferred), so neither bootstrap nor idle recovery stalls on a queued synthetic turn; see FORK-CHANGES.md. A deferral is released whenever its row is locally completed or permanently failed (stop/control fences, processed-ack retry), at every pass start and after every processed row. Upstream-PR: band-ai#730
A /next head this agent can never run (completed locally, permanently failed, already processed or its own echo) hands recovery to one complete chronological snapshot instead of re-polling the same head, so eligible work behind it is neither hidden nor overtaken. Queued local (synthetic) turns keep their place: snapshot recovery yields to them only outside bootstrap and once no older deferred message is outstanding, and a deferral ends as soon as its row is settled locally (completed or permanently failed), wherever that happens: under a STOP, PLAY or INTERRUPT fence, or through a processed-ack retry.
41e9389 to
b915882
Compare
|
Rebased onto current main ( |
Source-Base: d7ab342 Source-Head: b915882 Applied-Over: 051e81f Upstream-PR: band-ai#730
Source-Base: d7ab342 Source-Head: b915882 Applied-Over: 051e81f Upstream-PR: band-ai#730
Source-Base: d7ab342 Source-Head: b915882 Upstream-PR: band-ai#730
Rebased onto main
d7ab3429cbdb(2026-10-15)Rebased onto current main (after #709, #804, #789/#775, #796 and #802) as one commit; head
b9158829eb142311a7ba99761665135f9fc74679. Ported onto #789/#775'sTurnOutcomeand #804's module layout. Adds a fix for a stall the previous head had: snapshot recovery now yields to queued local turns only outside bootstrap and once no older deferred message is outstanding, and a deferral ends as soon as its row is settled locally (including under a STOP/PLAY/INTERRUPT fence or via a processed-ack retry); regressions intests/runtime/resync/test_actionable_recovery.py. Also carries the deferred-local-event stopped-room probe, itsdocs/websocket-events.mdnote, and a bounded watchdog-cleanup test fixture.CHANGELOG.mdedits are dropped: Release Please generates it.Verified at the new head (macOS, Python 3.13):
uv sync --all-packages --locked --extra dev, all pre-commit hooks, and the fullpytest -q: 8628 passed, 738 skipped. Verification notes further down refer to the previous head.Summary
A retained skipped or retry-exhausted
/nexthead must not starve later eligible deliveries. Refresh this fix onto current upstreamd7ab3429cbdb5381b9ca572cf7eac210a8196052, preserving the current reconciliation model and the failed head's diagnostics rather than acknowledging it as processed.Changes
/next, overtaking retryable/in-flight work, or introducing a preliminary processing-status sweep.Testing
Fresh current-main affected suite: 491 passed, 1 warning, exit 0. Scope:
tests/runtime/resync,executioninterrupt,rooms, execution/working-state/shutdown, platform link and session sync/shutdown; includes all 67 actionable-recovery cases.uv sync --all-packages --locked --extra devand all five pre-commit hooks passed. The published head adds only the required changelog entry to tested code head2c15fb08a18af243232a96fd5243fda57fca0aac.Local checks are not remote CI or live-room/native provider receipts.
Commit range and merge order
Complete prerequisite range:
d7ab3429cbdb5381b9ca572cf7eac210a8196052..b9158829eb142311a7ba99761665135f9fc74679.This fix is independent of the ACP and OMP prerequisites and can merge independently. The separate resolver stacks retain their own prerequisite-first order. Related: #709.