Skip to content

fix(gateway): bound failed completion notification retries - #138947

Closed
KoNit-K wants to merge 1 commit into
NousResearch:mainfrom
KoNit-K:fix/138940-bound-completion-retries
Closed

KoNit-K wants to merge 1 commit into
NousResearch:mainfrom
KoNit-K:fix/138940-bound-completion-retries

Conversation

@KoNit-K

@KoNit-K KoNit-K commented Oct 15, 2026

Copy link
Copy Markdown

What does this PR do?

Background-process completion notifications no longer retry forever when the gateway cannot admit their synthetic event. The process watcher now makes at most three admission attempts and doubles the retry delay up to 60 seconds. Successful admissions and the existing completion delivery path are unchanged.

Related Issue

Fixes #138940

Type of Change

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

Changes Made

  • gateway/run_notifications.py — bounds refused completion admissions and adds capped exponential retry delays for exited background processes.
  • tests/gateway/test_background_process_notifications.py — proves that failed admissions stop after the third attempt instead of polling an exited process indefinitely.

How to Test

  • scripts/run_tests.sh tests/gateway/test_background_process_notifications.py -q — 31 passed.
  • $HERMES_PYTHON -m ruff check gateway/run_notifications.py tests/gateway/test_background_process_notifications.py — passed.

Evidence

  • BEFORE RED: the new refused-admission case made 4 delivery attempts; its assertion expected 3.
  • AFTER GREEN: scripts/run_tests.sh tests/gateway/test_background_process_notifications.py -q completed with 31 passed.
  • CONTROL: Ruff passed for both changed Python files, and successful completion admission remains on the original one-attempt path.

Checklist

Code

  • I've read the Contributing Guide.
  • My commit message follows Conventional Commits.
  • I searched for existing PRs to make sure this is not a duplicate.
  • My PR contains only changes related to this fix.
  • I've run relevant tests locally (see How to Test).
  • I've added a regression test for the retry limit.
  • I've tested on my platform: macOS.

Documentation & Housekeeping

  • Documentation update: N/A.
  • cli-config.yaml.example: N/A.
  • CONTRIBUTING.md or AGENTS.md: N/A.
  • Cross-platform impact considered; retry timing uses asyncio and is platform-neutral.
  • Tool descriptions/schemas: N/A.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery tool/terminal Terminal execution and process management sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Oct 15, 2026
kshitijk4poor added a commit that referenced this pull request Oct 15, 2026
…n sends the plain notice

The retry cap from #138947 gave up about 35 s after exit and
then broke out of the watcher, skipping the plain-text completion notice.
A temporary refusal (adapter reconnecting, store draining or busy, a
preflight "retry" verdict) therefore lost a completion that main would
have delivered on the next tick, and nothing told the user.

Retry eight times with capped exponential backoff via the existing
jittered_backoff helper (5, 10, 20, 40, then 60 s: about five minutes at
the default 5 s interval), then fall through to the plain completion
notice the watcher already sends when agent notify is off.
@kshitijk4poor

Copy link
Copy Markdown

Thanks @KoNit-K. Your commit is carried into #139026 with your authorship (fcb7baa5c0), so I'm closing this in favour of it.

Why this PR couldn't land on its own: the refusal behind #138940 is always the same admission_conflict (the synthetic wake clears source.message_id, while Discord and Telegram keep it on the committed source). Capping retries stopped the loop, but the completion was still never delivered. A temporary refusal (adapter reconnecting, store draining) also got dropped after about 35 s, where main would have delivered it on the next tick. #139026 adds the root-cause fix from #138967, raises the cap to 8 retries with backoff, and falls back to the plain completion notice instead of break.

#139026 is armed. I'll post the merge SHA here once it lands, and reopen this if it doesn't.

kshitijk4poor added a commit that referenced this pull request Oct 15, 2026
…n sends the plain notice

The retry cap from #138947 gave up about 35 s after exit and
then broke out of the watcher, skipping the plain-text completion notice.
A temporary refusal (adapter reconnecting, store draining or busy, a
preflight "retry" verdict) therefore lost a completion that main would
have delivered on the next tick, and nothing told the user.

Retry eight times with capped exponential backoff via the existing
jittered_backoff helper (5, 10, 20, 40, then 60 s: about five minutes at
the default 5 s interval), then fall through to the plain completion
notice the watcher already sends when agent notify is off.
@kshitijk4poor

Copy link
Copy Markdown

Landed on main via #139026 (merge head 2bc1edbc5e). Thanks again.

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 tool/terminal Terminal execution and process management type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Background process completion notifications fail delivery to Discord and spin in infinite retry injection loop

3 participants