Skip to content

fix(gateway): keep owed delivery obligations under max-row prune#71111

Open
fangliquanflq wants to merge 1 commit into
NousResearch:mainfrom
fangliquanflq:fix/delivery-ledger-max-rows-preserve-undelivered
Open

fix(gateway): keep owed delivery obligations under max-row prune#71111
fangliquanflq wants to merge 1 commit into
NousResearch:mainfrom
fangliquanflq:fix/delivery-ledger-max-rows-preserve-undelivered

Conversation

@fangliquanflq

Copy link
Copy Markdown

What does this PR do?

Stops the delivery-obligation ledger from hard-deleting still-owed final replies when the row-count cap is exceeded. Count pressure now only drops terminal delivered / abandoned rows, matching time-based retention, so backlog pressure cannot silently erase recoverables.

Bug Cause

gateway/delivery_ledger._prune applies a max-row DELETE after the time cutoff. The subquery ordered delivered=0, abandoned=1, then ELSE=2. Under a backlog of only (or mostly) pending / attempting / failed rows, excess deleted the oldest owed obligations permanently. Those rows never reach sweep_recoverable, so users lose finals the ledger was meant to protect. Time retention already scoped to terminal states; the count path did not. Existing tests only covered the time cutoff, not _MAX_ROWS.

Reproduction Steps

  1. Point HERMES_HOME at an empty temp dir and import gateway.delivery_ledger.
  2. Call record_obligation more than _MAX_ROWS times with distinct pending ids (each call triggers _prune).
  3. Query delivery_obligations: total is capped at _MAX_ROWS, and the oldest pending id is gone.

Expected: owed (pending / attempting / failed) rows survive count pressure (or at least are abandoned for inspection, not hard-deleted).
Before fix: oldest owed rows disappear with no abandoned transition.

Fix

Scope the max-row DELETE to state IN ('delivered', 'abandoned'), preferring delivered before abandoned by updated age. Owed rows may temporarily exceed _MAX_ROWS until delivered or until dead-owner sweep transitions poison/stale rows to abandoned. Added TestPrune coverage for all-pending overflow and mixed terminal + owed (including abandoned).

Related Issue

No issue - discovered via bug-hunter finding BUG-GDL-2.

Type of Change

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

Changes Made

  • gateway/delivery_ledger.py - max-row prune deletes only terminal rows; document soft cap for owed backlog
  • tests/gateway/test_delivery_ledger.py - assert owed rows survive count cap; terminal rows (delivered + abandoned) are preferred victims

How to Test

  1. Manual: insert more than _MAX_ROWS pending obligations via record_obligation; confirm oldest pending ids remain and only terminal rows are dropped when mixed.
  2. Automated (already run locally, 31 passed):
scripts/run_tests.sh tests/gateway/test_delivery_ledger.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 - N/A
  • I've updated cli-config.yaml.example if I added/changed config keys - N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows - N/A
  • I've considered cross-platform impact - N/A (SQLite prune SQL only)
  • I've updated tool descriptions/schemas if I changed tool behavior - N/A

Count-cap prune deleted oldest pending/attempting/failed rows when the
ledger exceeded _MAX_ROWS, permanently dropping undelivered finals under
backlog pressure. Scope the DELETE to delivered/abandoned only.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 25, 2026
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:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants