Skip to content

fix(webhook): disable async delivery for single-shot HTTP ingress - #69166

Open
Bartok9 wants to merge 3 commits into
NousResearch:mainfrom
Bartok9:fix/69145-webhook-stateless-async-delivery
Open

fix(webhook): disable async delivery for single-shot HTTP ingress#69166
Bartok9 wants to merge 3 commits into
NousResearch:mainfrom
Bartok9:fix/69145-webhook-stateless-async-delivery

Conversation

@Bartok9

@Bartok9 Bartok9 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #69145

Webhook (and MS Graph webhook) one-shot sessions still strand background delegate_task results: the parent turn ends, the session is marked ended, and the #55578 fail-closed path drops async completion injection. #66617 fixed the same class of bug for hermes -z and cron via declare_stateless_channel() / async_delivery=False; the gateway path already propagates each adapter's supports_async_delivery flag in _set_session_env, but webhook left the default True.

Motivation / Root cause

  • Stateless HTTP ingress is one inbound → one response (same contract as API server).
  • Top-level delegation is often forced background=True.
  • After the parent finish_reason=stop, completions cannot safely re-enter an ended session.

Fix

Set supports_async_delivery = False on:

  • gateway/platforms/webhook.pyWebhookAdapter
  • gateway/platforms/msgraph_webhook.pyMSGraphWebhookAdapter

APIServerAdapter already does this. Tools that consult async_delivery_supported() then fall back to synchronous inline delegation (existing #53027 / #63142 / #66617 path).

Tests

  • Extended tests/gateway/test_async_delivery_capability.py::TestAdapterCapabilityFlag::test_api_server_false to assert webhook + msgraph flags.

Verification

pytest tests/gateway/test_async_delivery_capability.py -q

15 passed.

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

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #69159 and #69145. Both open PRs set WebhookAdapter to disable async delivery for finite sessions; this patch also applies the capability flag to MSGraphWebhookAdapter. Maintainers can choose whether to consolidate the overlapping webhook hunk.

@Bartok9

Bartok9 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @alt-glitch — good catch on the overlap. This PR's distinct value is extending the capability flag to MSGraphWebhookAdapter, which #69159 and #69145 don't touch. Happy to defer to maintainers on consolidation: if you'd prefer to land the shared webhook hunk in one of those, I can rebase this down to just the MSGraph-adapter delta so there's no duplicated change. Just say the word.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing the generic webhook lifecycle. The WebhookAdapter portion addresses a verified current-main gap: it inherits supports_async_delivery=True while its on_processing_complete closes each delivery session (gateway/platforms/webhook.py:936-1016), and GatewayRunner._set_session_env propagates that capability to delegate_task (gateway/run.py:18013-18029).

Problems

  • MSGraphWebhookAdapter is not shown to have the same lifecycle. It uses a stable subscriptionId chat id (gateway/platforms/msgraph_webhook.py:391-405) and schedules notifications through a pipeline or handle_message (gateway/platforms/msgraph_webhook.py:437-453); unlike generic webhook, it has no on_processing_complete session-close override. Marking it non-delivering would force synchronous delegation without a demonstrated stranded-completion path.
  • The added test checks only class attributes. It does not verify the gateway binding that makes the flag effective.

Suggested changes

  • Scope this PR to WebhookAdapter; investigate MS Graph separately if a concrete ended-session reproduction exists.
  • Add a GatewayRunner._set_session_env regression test for webhook binding, with a supported-delivery control.

Automated hermes-sweeper review.

@alt-glitch alt-glitch added the needs-decision Awaiting maintainer decision before any implementation label Jul 30, 2026
@Bartok9

Bartok9 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @teknium1 — applied both suggestions:

  • Scoped to WebhookAdapter. Dropped the MSGraphWebhookAdapter flag; you're right that it has no demonstrated ended-session path — stable subscriptionId chat id and no on_processing_complete session-close override, so marking it non-delivering would force sync delegation without a real stranded-completion repro. Will investigate MS Graph separately only if a concrete reproduction shows up.
  • Real gateway-binding test. Replaced the class-attribute-only check with a GatewayRunner._set_session_env regression test that verifies the webhook inbound actually binds async_delivery=False through the live adapter lookup, plus a delivering-platform (telegram) control that binds True on the same path — so the webhook False is a real signal, not a blanket off.

Green locally (pytest tests/gateway/test_async_delivery_capability.py → 17 passed).

Bartok9 added 3 commits July 30, 2026 01:01
Closes NousResearch#69145

Root cause: NousResearch#66617 bound declare_stateless_channel for hermes -z and
cron so delegate_task runs inline when the channel cannot wake after
the turn. Webhook (and msgraph_webhook) keep supports_async_delivery=
True by default; gateway marks the parent session ended after one
response, then NousResearch#55578 drops ASYNC DELEGATION BATCH COMPLETE injections.

Fix: set supports_async_delivery=False on WebhookAdapter and
MSGraphWebhookAdapter so run.py _set_session_env propagates the
capability like APIServerAdapter.

Verification: pytest tests/gateway/test_async_delivery_capability.py -q
…n test

Address hermes-sweeper review:
- Drop the MSGraphWebhookAdapter flag; it has no demonstrated ended-session
  stranded-completion path (stable subscriptionId chat id, no
  on_processing_complete session-close override). Investigate separately if a
  concrete reproduction appears.
- Replace class-attribute-only assertion with a _set_session_env regression
  test that verifies the gateway actually binds async_delivery=False for a
  webhook inbound, plus a delivering-platform (telegram) control that binds
  True through the same path.
@Bartok9
Bartok9 force-pushed the fix/69145-webhook-stateless-async-delivery branch from ba365b3 to 59b09c7 Compare July 30, 2026 05:02
@alt-glitch alt-glitch added duplicate This issue or pull request already exists and removed needs-decision Awaiting maintainer decision before any implementation labels Jul 30, 2026
@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
@alt-glitch alt-glitch added duplicate This issue or pull request already exists and removed sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades duplicate This issue or pull request already exists labels Jul 30, 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 duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists platform/webhook Webhook / API server 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 tool/delegate Subagent delegation type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: gateway/webhook one-shot sessions still strand background delegations — #66617's stateless-channel fix doesn't cover the gateway runner

3 participants