Skip to content

fix(discord): schedule approval boxes on the gateway loop - #74471

Draft
peacockesq wants to merge 2 commits into
NousResearch:mainfrom
peacockesq:fix/74470-discord-approval-gateway-loop
Draft

fix(discord): schedule approval boxes on the gateway loop#74471
peacockesq wants to merge 2 commits into
NousResearch:mainfrom
peacockesq:fix/74470-discord-approval-gateway-loop

Conversation

@peacockesq

Copy link
Copy Markdown

Closes #74470.

What changed

  • restores the Discord deliverable approval-box tool and three-button component view after the platform-plugin migration
  • stores the Discord adapter's owning gateway event loop
  • schedules approval delivery on that loop instead of _run_async's private worker loop
  • persists pending approval records and disables controls after the first authorized decision
  • includes the Google Drive review URL in both plain content and the embed

Root cause

discord.py owns an aiohttp session bound to the gateway event loop. The synchronous tool handler used _run_async, which ran channel.send() on a different event loop and produced Timeout context manager should be used inside a task.

Verification

  • venv/bin/python -m py_compile tools/discord_approval_box_tool.py plugins/platforms/discord/adapter.py
  • venv/bin/python -m pytest tests/tools/test_discord_approval_box_tool.py -o addopts='' -q — 3 passed
  • venv/bin/python -m pytest tests/agent/test_async_utils.py -o addopts='' -q — 6 passed

Live Discord verification will be recorded after the local gateway restart.

@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins comp/tools Tool registry, model_tools, toolsets platform/discord Discord bot adapter P3 Low — cosmetic, nice to have sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 30, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the focused gateway-loop repair. The requested approval-box symbols are still absent from current main, so this is not stale, but two changes are needed before this can safely land.

Problems

  • tools/discord_approval_box_tool.py:145 always reads runner.adapters[Platform.DISCORD]. In multiplex mode, secondary adapters live in _profile_adapters; gateway/authz_mixin.py:71-97 deliberately fails closed rather than falling back to the default bot. Resolve using HERMES_SESSION_PROFILE and the profile-aware adapter path, with regression coverage for a secondary profile.
  • The toolsets.py change places a Discord-only, live-gateway-only capability in _HERMES_CORE_TOOLS. Since all CLI and messaging bundles inherit that list (toolsets.py:31-82, 435-465), expose it only through the Discord platform bundle instead.

Suggested changes

  • Add a multiplex-profile routing test and a schema-scope test alongside the existing loop scheduling coverage.

Automated hermes-sweeper review.

from gateway.config import Platform
runner = _gateway_runner_ref()
adapter = runner.adapters.get(Platform("discord")) if runner is not None else None
except Exception:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

runner.adapters is only the active/default adapter map in multiplex mode. Read HERMES_SESSION_PROFILE and use the profile-aware resolver (which fails closed if that profile lacks Discord), otherwise a secondary profile can send this approval through the default profile's bot.

Comment thread toolsets.py
# Approval-gated external delivery on Discord (available only when a
# Discord bot is configured; check_fn keeps it off other installations).
"discord_approval_box",
]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep this Discord-only live-adapter tool out of _HERMES_CORE_TOOLS: every CLI and messaging platform inherits that schema. Add it to the hermes-discord platform bundle instead, so non-Discord sessions never advertise a tool they cannot execute.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Two PRs address #74470. #74471 directly restores deliverable approval boxes and schedules their send on the Discord gateway-owned loop, while #74493 adds generic loop-scheduling infrastructure without wiring the reported deliverable-approval call site and also contains unrelated changes.

Related pull requests

Duplicates

#74471 and #74493 target the same Discord aiohttp event-loop failure; #74493 is the broader partial duplicate because it does not connect its scheduling helper to the deliverable approval-box call site addressed by #74471.

Suggested consolidation

Keep #74471 open with a salvage path: retain its concrete owning-loop scheduling, approval persistence, three-button rendering, and focused tests, while requiring profile-aware fail-closed Discord adapter resolution and Discord-only tool exposure with regression coverage, as specified by the keep_open contributor review. Keep #74493 closed as a duplicate of #74471 because its relevant infrastructure is not wired into the affected call path and its branch includes unrelated changes.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I74470(["issue #74470 (open)"])
    subgraph Dup74471 ["PRs duplicating each other"]
        P74471["PR #74471 (open)"]
        P74493["PR #74493 (closed)"]
    end
    P74471 -->|best fix| I74470
    class I74470 open
    class P74471 open
    class P74493 closed
    class P74471 best
    class P74471 target
    click I74470 "https://github.com/NousResearch/hermes-agent/issues/74470"
    click P74471 "https://github.com/NousResearch/hermes-agent/pull/74471"
    click P74493 "https://github.com/NousResearch/hermes-agent/pull/74493"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 2 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 46 kB of PR diffs, 4 kB of issue/PR text, 2 kB of discussion (4 comments), 5 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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

Labels

comp/plugins Plugin system and bundled plugins comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have platform/discord Discord bot adapter 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-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Discord approval boxes use the wrong asyncio event loop

4 participants