Skip to content

fix(gateway): label redelivery after mid-recovery crash - #70700

Open
fangliquanflq wants to merge 5 commits into
NousResearch:mainfrom
fangliquanflq:fix/delivery-ledger-recovery-marker
Open

fangliquanflq wants to merge 5 commits into
NousResearch:mainfrom
fangliquanflq:fix/delivery-ledger-recovery-marker

Conversation

@fangliquanflq

@fangliquanflq fangliquanflq commented Jul 24, 2026 •

Copy link
Copy Markdown

What does this PR do?

Fixes honest at-least-once labeling for delivery-ledger recovery: after a pending obligation is claimed and the gateway crashes mid-send, the next reclaim now carries RECOVERED_MARKER instead of silently resending.

Bug Cause

sweep_recoverable claimed dead-owner rows by restamping owner/attempts but left state='pending', and needs_marker was only state != "pending". _redeliver_pending_obligations also never called mark_attempting before adapter.send. A crash after the first recovery send could therefore reclaim the same row again with needs_marker=False.

Reproduction Steps

  1. Leave an orphaned pending delivery obligation in state.db.
  2. Boot the gateway so sweep_recoverable claims it and starts adapter.send without the recovered marker.
  3. Kill the process after the platform accepts the message but before mark_delivered.
  4. Boot again and observe the second redelivery.

Expected: second redelivery is prefixed with RECOVERED_MARKER.
Before fix: second redelivery is plain text (needs_marker=False, state still pending).

Fix

  • Claim UPDATE sets state='attempting'.
  • needs_marker is true when state != "pending" or pre-claim attempts >= 1 (covers stuck pre-fix rows).
  • Recovery path calls mark_attempting immediately before adapter.send.
  • Tests cover reclaim-after-crash, stuck pending+attempts, and second-boot gateway redelivery.

Related Issue

No issue

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • gateway/delivery_ledger.py - claim transitions to attempting; needs_marker honors prior attempts
  • gateway/run.py - call mark_attempting before recovery adapter.send
  • tests/gateway/test_delivery_ledger.py - reclaim / stuck-pending / second-boot marker coverage

How to Test

  1. Manual: orphan a pending obligation, claim via sweep, orphan again, reclaim - second claim must set needs_marker=True.
  2. Automated (already run locally, 40 passed):
scripts/run_tests.sh \
  tests/gateway/test_delivery_ledger.py \
  tests/gateway/test_delivery_ledger_producer.py \
  -q

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature
  • I've run scripts/run_tests.sh on relevant tests and they pass
  • I've added tests for my changes
  • I've tested on my platform: Windows 11

Documentation & Housekeeping

  • I've updated relevant documentation - or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys - or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows - or N/A
  • I've considered cross-platform impact - or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior - or N/A

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 24, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused recovery-state fix. The premise is confirmed on current main: gateway/delivery_ledger.py:287-304 reclaims a dead-owner row by updating ownership and attempts but leaves state='pending', so its marker decision remains false; gateway/run.py:9934-9939 then sends without a recovery-side mark_attempting. The normal producer already establishes the intended contract at gateway/platforms/base.py:6051-6066 by recording and marking attempting before its send.

The proposed state transition, prior-attempt marker fallback, and pre-send checkpoint directly address that gap. The added tests cover both the existing-row compatibility case and the second-recovery marker behavior. The patch adds no tool, configuration, cache, or message-alternation surface.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 30, 2026
Claiming a pending obligation now stamps attempting and treats prior attempts as needing the recovered marker, so a crash mid-redelivery cannot silently duplicate a platform-ACK'd send.
@fangliquanflq
fangliquanflq force-pushed the fix/delivery-ledger-recovery-marker branch from 57eb6a3 to 544c7b9 Compare July 31, 2026 06:59

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants