Skip to content

feat(gateway,skills): unified skill trigger framework with Discord components - #16530

Closed
0xarkstar wants to merge 20 commits into
NousResearch:mainfrom
0xarkstar:feat/unified-trigger-framework
Closed

feat(gateway,skills): unified skill trigger framework with Discord components#16530
0xarkstar wants to merge 20 commits into
NousResearch:mainfrom
0xarkstar:feat/unified-trigger-framework

Conversation

@0xarkstar

@0xarkstar 0xarkstar commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Unified skill trigger framework — resolver + reactions + mentions + Feishu

Status update (2026-05-19)

Rebased onto current upstream/main HEAD 16abb74ea (post-Atropos-removal + post-clarify-buttons-salvage). 20 commits, 132/132 trigger framework tests green.

Discord button wiring deferred to PR #19413 (TerminalSausage, "interactive components — buttons, select menus, REST + WebSocket paths"), which the maintainer is salvaging onto current main. To avoid an on_interaction handler conflict (discord.py allows only one registered handler per event) and reduce review surface, this PR's button-specific wiring was removed in the "refactor: defer Discord button wiring to PR #19413" commit.

The metadata.hermes.triggers.button schema and the resolver's button matcher are retained as framework extension points so a follow-up PR can connect PR #19413's component dispatch into resolve_event_skills('button', payload, skills) — completing the round-trip skill declaration → component emission → click → skill dispatch.

Adjacent upstream landing — clarify-as-buttons: PR #19249 (LeonSGP43) was salvage-merged into main on 2026-05-13 as commit 1dca6a696 (also tracked as PR #25485). That work is complementary to this PR — it adds a Discord-specific ClarifyChoiceView for the existing clarify tool; this PR provides the cross-adapter resolver substrate that lets any skill declare button/reaction/mention triggers via frontmatter. No code overlap (verified during rebase).

TL;DR

opt-in metadata.hermes.triggers schema in skill frontmatter + adapter-agnostic event resolver + Discord composition handler that routes inbound reactions and mentions to skills declaring matching triggers + 2 outbound reaction tools. Feishu reaction routing uplifted to the same resolver with broadcast fallback for skills without explicit triggers. Matrix is left for follow-up.

Closes: #3879 (skill trigger keywords RFC) · #19111 (Discord clarify-as-buttons — partial; button emission via #19413)
Closes (partial): #22365 (Discord reaction emoji customization — outbound lifecycle config separately tracked in #22100)
Supersedes: #8379 (Discord inbound reaction routing — generalized in this PR)
Defers to #19413: button emission + interaction dispatch (this PR retains the resolver-side button trigger as a bridge hook point)
Issue: #503 Platform-Native Rich Interactions RFC — Discord reaction/mention lanes addressed; button lane addressed by #19413; Telegram/Slack/Matrix lanes remain open as follow-ups.

Production deployment

This branch is currently the production runtime for BlueNode (Hermes profile on OCI ARM64 Graviton) — operations Discord bot for Inha University's blockchain club. Continuously deployed since 2026-04-27 (~22 days at time of writing).

Multiple commits emerged directly from operating that bot:

  • cache-first reaction author lookup — added after observing fetch_message round-trips dominating reaction event latency
  • reactions config bridge fix — added after dict-form yaml discord.reactions: {inbound_routing: true} was silently disabling routing in production
  • event counters for filter decisions — production observability
  • mention routing wiring — production use case of skill resolution on @mentions

Architecture (in one diagram)

graph LR
  A[skill frontmatter<br/>metadata.hermes.triggers] -->|skill_utils.py<br/>parser| B[skill_resolver.py<br/>resolve_event_skills]
  D[on_raw_reaction_*<br/>discord.py] -->|reaction| H[discord_interactions.py<br/>handler]
  E[_handle_message<br/>discord.py] -->|mention| H
  F[_handle_reaction_event<br/>feishu.py] -->|reaction| B
  H --> B
  B --> G[adapter.handle_message<br/>auto_skill=&lt;matched&gt;]
  P[PR #19413<br/>component dispatch] -.->|future bridge| B
Loading

The resolver is gateway/skill_resolver.py (pure function, no self, no platform imports). Adapters call resolve_event_skills(event_type, payload, skills) and dispatch on the result. Adding a new adapter = wire its event handler to the same resolver call — no resolver changes needed.

Defensive note (mirrors review feedback on PR #19413)

This PR's reacted-message cache lookup (_connection._messages) is a discord.py internal — wrapped in try/except (AttributeError, KeyError, TypeError) so a minor-version rename falls through to fetch_message silently rather than breaking reaction routing. Pattern preemptively applied per maintainer's #19413 review #2 (which flagged _view_store._views for the same defensive treatment).

Concrete changes

Frontmatter parser (agent/skill_utils.py)
Adapter-agnostic resolver (gateway/skill_resolver.py, new)
Discord interactions handler (gateway/platforms/discord_interactions.py, new)
  • DiscordInteractionsHandler — composition handler. Receives on_raw_reaction_add/remove and handle_inbound_mention calls from the Discord adapter.
  • handle_inbound_reaction(payload, action): cache-first author lookup (with defensive try/except on discord.py internals) → resolver dispatch.
  • handle_inbound_mention(message, normalized_text): routes mentions through the resolver; caller can early-return when no match.
  • make_skill_custom_id / is_skill_custom_id: canonical skill_<name>_<action> helpers retained as the bridge's hook points for the future PR connecting feat(discord): interactive components — buttons, select menus, REST + WebSocket paths #19413's component dispatch to this resolver.
Discord adapter wiring (gateway/platforms/discord.py)
  • __init__: instantiate DiscordInteractionsHandler with _build_skill_provider().
  • Intents: conditional intents.reactions = True opt-in via config.extra.reactions.inbound_routing (default false).
  • connect(): register on_raw_reaction_add/remove ONLY if inbound_routing flag is True. Mention routing fires from _handle_message (already in the normal message path).
  • Button on_interaction registration removed in this PR — that surface is owned by PR feat(discord): interactive components — buttons, select menus, REST + WebSocket paths #19413.
  • Existing slash commands, ExecApprovalView, UpdatePromptView, ModelPickerView all UNTOUCHED.
Feishu BC (gateway/platforms/feishu.py)
  • _handle_reaction_event calls resolve_event_skills('reaction', payload, skills) BEFORE building the synthetic text event.
  • BC fork: matched → matched dispatch · empty + no triggers exist anywhere → broadcast (preserves existing behavior) · empty + triggers exist → skip.
Outbound reaction tools
  • tools/discord_reaction_tool.pydiscord_add_reaction + discord_remove_reaction. Async, lazy import discord, structured error returns. _remove_reaction passes client.user so it removes the bot's own reaction (not other users'). Wraps discord.NotFound / Forbidden / HTTPException / generic Exception structurally.

Note: discord_send_button_message was removed; outbound button emission is provided by PR #19413's send_message tool extension.

Latent bug fix: dict-form discord.reactions config bridge

The yaml form discord.reactions: {enabled: true, inbound_routing: true} was previously stringified by gateway/config.py (str(reactions).lower() → garbage env var) and never bridged to PlatformConfig.extra, silently disabling both outbound 👀✅❌ emoji feedback AND the new inbound routing. Fix splits cleanly:

  • bool form → legacy DISCORD_REACTIONS env var
  • dict form → extra.reactions (Track 1 routing)

Backward-compat preserved for both.

Testing

pytest tests/agent/test_skill_utils_triggers.py \
       tests/gateway/test_skill_resolver.py \
       tests/gateway/test_discord_interactions.py \
       tests/gateway/test_discord_inbound_reactions.py \
       tests/gateway/test_feishu_reactions_bc.py \
       tests/tools/test_discord_reaction_tool.py \
       tests/tools/test_registry.py \
       tests/gateway/test_discord_event_counters.py \
       tests/gateway/test_reaction_cache_first.py \
       tests/gateway/test_mention_routing.py \
       -n auto
# 132/132 passed

Both -n auto (parallel) and -n 0 (serial) modes pass.

Broader regression check: pytest tests/gateway/ tests/tools/ — env-only failures (missing optional psutil, faster_whisper, kittentts deps), confirmed identical on plain upstream/main.

Backward compatibility

  • Skills WITHOUT triggers: frontmatter are parsed unchanged.
  • Feishu broadcast fallback preserved for the no-explicit-triggers corpus.
  • intents.reactions = True is opt-in via config.extra.reactions.inbound_routing (default false).
  • No new slash commands registered.
  • Existing internal Views (ExecApprovalView, UpdatePromptView, ModelPickerView) keep their own dispatch.

Salvage-friendly + split offer

This PR is structured for salvage: each commit is independently revertable, the resolver is decoupled from adapters, and I'm happy to rewrite/rescope on maintainer feedback within 24h.

If preferred, I can split this PR further into 2 smaller PRs:

  • Core foundation (~600 LoC): skill_utils.py parser + skill_resolver.py + their tests
  • Discord + Feishu integration (~900 LoC): handler + adapter wiring + reaction tools + Feishu BC + config bridge fix + docs

Let me know which approach is easier to land.

Status

🤖 Generated with Claude Code

@alt-glitch alt-glitch added type/feature New feature or request P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/skills Skills system (list, view, manage) platform/discord Discord bot adapter labels Apr 27, 2026
@0xarkstar

Copy link
Copy Markdown
Contributor Author

Friendly nudge — this PR has been open for a few days without review. Happy to address any maintainer feedback or rebase if upstream has moved.

For context, I've also opened a complementary PR #18255 (cron token-leak mitigation) which is independent of this one — it does not touch the unified trigger framework files and can be reviewed/merged in either order.

Thanks for the maintenance work on this project!

@0xarkstar
0xarkstar force-pushed the feat/unified-trigger-framework branch 4 times, most recently from c4dd08f to bfa985c Compare May 11, 2026 19:03
0xarkstar and others added 19 commits May 19, 2026 01:06
Extend agent/skill_utils.py with extract_skill_triggers (Schema α —
type-keyed dict), derive_implicit_triggers (slash from legacy
slash_command field), and get_skill_triggers (combined accessor).

Schema:
  metadata.hermes.triggers:
    mention: { regex, channel_filter }
    slash:   { name }
    button:  { custom_id_pattern }
    reaction: { emoji, channel_filter, age_limit }
    cron:    { schedule }

Backward compatibility: skills without triggers field continue to
work via the existing prompt-builder injection path. Skills with a
legacy metadata.hermes.slash_command field automatically get an
implicit slash trigger derived from that field.

Pure functions, fully unit-tested. 18 test cases cover: missing
metadata, missing triggers, all 5 trigger types, scalar shorthand,
malformed YAML, derive rule edge cases, combined accessor.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New module gateway/skill_resolver.py with:
- resolve_event_skills(event_type, payload, skills) -> List[str]
  Pure function; matches button/reaction/mention/slash payloads
  against skill triggers. Returns matching skill names.
- has_explicit_triggers(skills) -> bool
  Signal for adapter BC fallbacks (e.g., Feishu broadcast).
- snapshot_skills() -> List[SkillEntry]
  Lazy walker shared across Discord and Feishu adapter wrappers.

Adapter-agnostic — no platform imports, no self.config access. The
resolver receives a list of (skill_name, frontmatter, triggers) tuples
plus a payload dict and returns matched names.

Matchers:
  button:   custom_id pattern via fnmatch + optional channel_filter
  reaction: exact emoji match + optional channel_filter + optional
            age_limit (units: s/m/h/d/w)
  mention:  regex search against payload['text'] + channel_filter
  slash:    exact name match
  cron:     returns [] (handled by cron registrar; schema-forward)

20 test cases cover all matchers, malformed entries, unknown event
types, channel filters, age limits, and the has_explicit_triggers
helper used by Feishu BC.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New module gateway/platforms/discord_interactions.py with:
- DiscordInteractionsHandler — composition handler injected into
  DiscordAdapter. Receives button interactions and (when opt-in
  flag is set) raw reaction events. Built around callable injection
  for skill enumeration — no fabricated _available_skills attribute.
- SkillButtonView(discord.ui.View) — convenience subclass that
  delegates @discord.ui.button callbacks to the handler. Custom_id
  shape: 'skill_<skill_name>_<action>' (matched via fnmatch).
- make_skill_custom_id / is_skill_custom_id — canonical helpers.

Composition over inheritance: the handler does NOT subclass
DiscordAdapter. Adapter instantiates the handler in __init__ and
calls into it from connect()-time event handlers.

discord.ui.View precedence: discord.py 2.7+ routes View callbacks
BEFORE the global on_interaction event. Skills using raw
discord.ui.View subclasses bypass the resolver intentionally —
internal Hermes views (ExecApprovalView, UpdatePromptView,
ModelPickerView) keep their existing in-process callbacks.

Tests: 16 unit cases (custom_id helpers, cache, payload builders,
button dispatch, SkillButtonView construction) +
8 reaction integration cases (add/remove paths, self-reaction
filter, multi-skill, provider exception handling, DM vs guild
source typing).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Minimal diff (~89 LoC) to gateway/platforms/discord.py:

- __init__: instantiate self._interactions = DiscordInteractionsHandler
  with skill_provider=snapshot_skills (lazy walker, no cached
  attribute on adapter).
- connect() intents block: opt-in 'reactions' intent gated behind
  config.extra.reactions.inbound_routing (default false). Avoids
  forcing existing deployments to re-handshake with Discord. Flag is
  read once at adapter init / connect()-time; runtime change requires
  a bot restart for handlers to bind.
- connect() event registration: global on_interaction handler that
  filters out non-component types and View-bound interactions
  (custom_id namespace check), then delegates skill-prefixed clicks
  to handler.handle_skill_button_interaction. Inbound raw reaction
  handlers (on_raw_reaction_add / on_raw_reaction_remove) registered
  ONLY when inbound_routing is true.

Existing slash command registration (lines ~1622-1710),
ExecApprovalView, UpdatePromptView, ModelPickerView are untouched —
discord.py 2.7+ View dispatch order means custom Views own their
own callbacks and bypass the resolver intentionally.

_build_skill_provider() helper delegates to the shared
gateway.skill_resolver.snapshot_skills walker so the same lazy
enumeration is used by both Discord and Feishu adapters.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Refactor _handle_reaction_event to call resolve_event_skills BEFORE
building the synthetic text event. BC fork (preserves pre-framework
behavior for skills that have not opted into the new schema):

  - resolver returns ≥1 match → dispatch with auto_skill=<matched>
    (NEW behavior, performant per-skill routing)
  - resolver returns [] AND no skill in corpus has explicit triggers
    → fall through to broadcast (LEGACY behavior preserved; existing
    Feishu deployments without triggers field continue to receive
    'reaction:added:EMOJI' synthetic events on every loaded skill)
  - resolver returns [] AND ≥1 skill has explicit triggers → skip
    (opt-in semantics: corpus declared what it wants, do not
    surprise it with broadcast)

Existing _FEISHU_ACK_EMOJI skip and bot/app sender_type filter run
upstream of the resolver and are unchanged.

_reaction_skill_snapshot() instance method delegates to the shared
gateway.skill_resolver.snapshot_skills() walker so Discord and
Feishu use the same lazy enumeration.

Tests: 6 BC contract cases — matched corpus dispatches only matched,
legacy-only corpus falls back to broadcast, opt-in corpus skips
unmatched, mixed legacy+opt-in dispatches only matched, ACK emoji
filter is upstream, empty corpus falls back to broadcast.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- CONTRIBUTING.md: new section under 'Adding a Skill' covering the
  metadata.hermes.triggers schema (Schema α — type-keyed dict) with
  3 worked examples (mention/button/reaction). Documents BC: skills
  without triggers field continue to work; skills with legacy
  metadata.hermes.slash_command field auto-derive an implicit slash
  trigger.
- AGENTS.md: cross-adapter event routing flow diagram added under
  Skills section, with adapter coverage matrix (Discord:
  buttons + opt-in reactions, Feishu: reactions with broadcast
  fallback, Matrix: stub-only, deferred).
- docs/migration/triggers-v1.md: full migration guide alongside the
  existing openclaw.md. Covers schema, opt-in flow for Discord
  inbound reactions (config.extra.reactions.inbound_routing flag,
  bot restart required for runtime changes), Discord buttons via
  SkillButtonView helper, Feishu BC fallback rules, BC contract
  summary, resolver implementation reference, testing notes, and
  follow-ups (Matrix uplift, Slack components, cron registrar).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Lets LLM-based skills emit Discord button messages with skill-routed
custom_ids. Tool wraps SkillButtonView (introduced in this PR) so skills
can use the unified trigger framework for 1-tap UX:

    skill calls tool with buttons spec
      → tool builds SkillButtonView with custom_id "skill_<name>_<action>"
      → message sent with view attached
      → user clicks button
      → discord.py routes to View callback
      → callback delegates to DiscordInteractionsHandler
      → resolver matches skill via triggers.button.custom_id_pattern
      → skill invoked with auto_skill set

Closes the LLM ↔ Discord components gap that Schema α alone cannot
bridge: schemas declare WHICH events skills want, this tool lets
skills EMIT button-bearing messages.

- tools/discord_button_tool.py — new outbound tool
- tests/tools/test_discord_button_message.py — 10 test cases
- gateway/platforms/discord_interactions.py — SkillButtonView accepts
  per-button style via button_styles kwarg (backward-compatible)
- CONTRIBUTING.md, docs/migration/triggers-v1.md — usage docs

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…Config.extra

The unified trigger framework's opt-in inbound reaction routing
(`config.extra.reactions.inbound_routing: true`) was unreachable: the
config layer only forwarded `discord.reactions` to the legacy
`DISCORD_REACTIONS` env var (a boolean toggle for outbound emoji
feedback) and never bridged dict-form reactions into PlatformConfig
extra. Result: `self.config.extra.get('reactions')` was always None,
`_reactions_inbound` was always False, and on_raw_reaction_add never
got registered — even though discord.py's default Intents include
reactions and the events were arriving at the WS layer.

Fix splits the two forms cleanly:

  - dict form (`{enabled: true, inbound_routing: true}`) →
    bridged into platforms.discord.extra.reactions, where the adapter
    reads `inbound_routing` and registers handlers.

  - bool form (`true|false`) → translated to DISCORD_REACTIONS env
    var as before, controlling legacy outbound 👀/✅/❌ processing
    feedback.

Without the second guard, dict-form would also serialize as a string
into DISCORD_REACTIONS, coercing to "false" via the truthy-set check
in `_reactions_enabled` and silently disabling outbound emojis.

Verified end-to-end on a live OCI deployment: ✅ reaction add/remove
now reach the registered handler, resolver matches the trigger, and
the dispatch skill is invoked with auto_skill set.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…on outbound tools

Closes the inbound/outbound asymmetry left after commits 1-8 of the
unified trigger framework. Skills can now react on Discord messages
programmatically: pre-attach a ✅ reaction to a freshly-sent bot message
so the user completes the action with a single tap, and clean up
reactions when an action expires or is reversed.

* `discord_add_reaction(channel_id, message_id, emoji)` — bot reacts.
* `discord_remove_reaction(channel_id, message_id, emoji)` — bot
  removes its own reaction (passes `client.user` to discord.py).

Symmetric to the existing `discord_send_button_message` tool: lazy
`discord` import, `_get_discord_adapter` helper, structured error
returns for `discord.NotFound` / `Forbidden` / `HTTPException` /
unexpected exceptions, registered into the `discord` toolset with
`is_async=True`. Both tools share `_resolve_message`, validation,
and availability checks.

12 unit tests at `tests/tools/test_discord_reaction_tool.py` cover
both happy paths, all four input-validation paths, adapter not
initialized, channel not found, message not found, Forbidden, and
non-numeric snowflake. Idempotent `_ensure_discord_mock` helper that
guarantees `discord.NotFound` / `Forbidden` / `HTTPException` are real
Exception classes regardless of whether other test modules already
populated `sys.modules["discord"]` with a bare MagicMock.

* tests/tools/test_discord_reaction_tool.py: 12 passed (parallel + serial)
* tests/tools/test_discord_button_message.py: 10 passed (no regression)
* tests/gateway/test_skill_resolver.py: 20 passed (no regression)
* tests/gateway/test_discord_interactions.py: 16 passed (no regression)
* tests/gateway/test_discord_inbound_reactions.py: 8 passed (no regression)
* tests/agent/test_skill_utils_triggers.py: 18 passed (no regression)
* Combined: 84 passed in 0.22s (-n 0) / 0.87s (-n auto)

Backward compatible: nothing else in the gateway or agent loop
references these tools; skills opt in by listing them in their
`requires_toolsets` and the LLM calling them via the registry.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ion outbound tools

Adds a new sub-section to the migration guide covering the outbound
reaction-emit tools shipped in commit 9, mirroring the existing
discord_send_button_message section style. Documents:

* The asymmetry-closure intent: framework already routes inbound
  reactions to skills via `triggers.reaction`; these tools give skills
  the matching outbound surface.
* JSON tool-call examples + return shape for both `discord_add_reaction`
  and `discord_remove_reaction`.
* Clarification that `discord_remove_reaction` removes the bot's own
  reaction (passes `client.user` to discord.py) and is not a generic
  admin operation.
* The 1-tap UX pattern: send a message, capture its `message_id`, then
  pre-attach `✅` so users complete the action with a single tap on the
  existing reaction (which routes back via `triggers.reaction.emoji`).
* The add+remove timing race: discord.py processes events on the gateway
  WS, so a remove issued within ms of an add may surface `NotFound`. The
  tool wraps errors structurally (never raises), so callers should
  expect this case in rapid-toggle flows. Addresses architect's commit-9
  next-pass note.

Also extends the top-level "What changed" summary to enumerate all
three outbound tools (`discord_send_button_message`, `discord_add_reaction`,
`discord_remove_reaction`) so the migration guide TL;DR matches the
shipped surface.

Docs-only — no Python files touched, no tests changed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the incorrect `discord_send_message` reference with the actual
registered tool surface: the action-based `discord` tool exposes a
`send_message` action via `_CORE_ACTIONS` (`tools/discord_tool.py:925`),
there is no separate `discord_send_message` tool. Caught during the
ralph cycle's deslop pass after architect noted the cross-reference was
unverified.

Single-line phrasing change in `docs/migration/triggers-v1.md:220-226`,
no behavior or schema impact.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the still-incorrect `discord` tool's `send_message` action
with the actual surface: the cross-platform `send_message` tool
registered under the `messaging` toolset
(`tools/send_message_tool.py:1574`).

The `discord` tool's `_CORE_ACTIONS` contains only
`fetch_messages`/`search_members`/`create_thread`, and the action
manifest at `tools/discord_tool.py:_ACTION_MANIFEST` has no
`send_message` action. Outbound message emission for skills runs
through the messaging toolset's tool, not the platform-specific
`discord` tool. Apologies for the iteration; the fix is now
consistent with what the registry actually exposes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…r button + reaction tools

Addresses architect's commit-9 next-pass note #1: the fetch_channel
fallback in both `_send_button_message` and `_resolve_message` masked
`discord.Forbidden` (bot lacks VIEW_CHANNEL permission) under a generic
"channel not found or bot cannot access it" message. The fetch_message
block in `_resolve_message` already differentiated Forbidden/NotFound
via isinstance; this commit extends the same pattern to fetch_channel
in both tools so users see specific errors:

* `Forbidden` → "Bot lacks VIEW_CHANNEL permission for channel <id>."
* `NotFound`  → "Channel <id> does not exist or bot is not in its guild."
* Otherwise   → existing generic "not found or bot cannot access" message
  (preserves backward compat for callers that grep on "not found").

The fall-through path uses defensive `try: import discord` /
`except ImportError: pass` so the module remains importable when
discord.py is absent.

Tests:

* `test_add_reaction_channel_forbidden` (reaction tool) — fetch_channel
  raises discord.Forbidden, asserts "permission" or "VIEW_CHANNEL"
  substring in the error.
* `test_channel_forbidden_returns_perm_error` (button tool) — same
  shape for the button outbound tool.
* Existing `test_channel_not_found_returns_error` tests continue
  asserting the generic fallback (raise generic Exception).

Mock helpers tightened: both test files now use stricter type checks
(`isinstance(getattr(mod, "ui", None), SimpleNamespace) and
isinstance(<that>.View, type)`) instead of `hasattr`, because
`MagicMock()` returns truthy auto-attrs for any name. The previous
`hasattr` checks would skip ui/ButtonStyle setup if a bare-MagicMock
discord module had been installed by another test, leaving the
`SkillButtonView` subclass with a MagicMock parent that produced empty
`children`. Now both files agree on the same idempotent helper shape.

Test totals (Track 1 + reaction tool combined):
* 86 passed (parallel + serial), no xdist races
* +2 new Forbidden-path tests
* No regressions in adjacent suites (gateway, agent, skill_resolver,
  discord_interactions)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes ~140 lines of near-identical _ensure_discord_mock duplication
between tests/tools/test_discord_button_message.py and
tests/tools/test_discord_reaction_tool.py. Both files now rely on a
shared helper at tests/tools/conftest.py that pytest auto-loads at
collection time before any test module in the same directory imports.

The reason the duplication existed in the first place is that
tests/gateway/conftest.py builds its own discord mock with
`discord_mod = MagicMock()` (fresh) and overwrites sys.modules,
clobbering attributes set by an earlier conftest. To make the two
conftests cooperate regardless of xdist worker collection order,
this commit also adds three lines to tests/gateway/conftest.py
(`Forbidden`, `NotFound`, `HTTPException` as real Exception
subclasses), so production isinstance checks in
tools/discord_button_tool.py and tools/discord_reaction_tool.py
work whichever conftest's mock ends up in sys.modules at runtime.

Verification:

* tests/tools/test_discord_button_message.py: 11 passed (parallel + serial)
* tests/tools/test_discord_reaction_tool.py: 13 passed (parallel + serial)
* Combined: 24 passed (parallel + serial)
* Full Track-1 suite (button + reaction + skill_resolver +
  discord_interactions + discord_inbound_reactions +
  skill_utils_triggers): 86 passed (parallel + serial)
* Broader gateway-discord subset (5 test files): 92 passed (no
  regressions in adjacent gateway tests)
* Net diff: -156 + 13 + 100 = -43 lines

Single source of truth for the discord mock shape, eliminating the
drift risk noted in cycle-3 architect review of commit 9.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 0.5 token-leak instrumentation: per-decision counters for inbound
Discord events. Three counters wired now to handle_inbound_reaction's
existing decision branches (skipped_non_bot_author / skipped_no_match /
invoked); two mention counters pre-declared but not yet wired (the
mention hook does not yet exist — PR-B will add it).

Without baseline counts, we cannot tell whether a future filter change
"reduced reaction-driven LLM invocations 80%" or "broke the path
entirely." This is the metric needed to attribute Phase 1+ token deltas.

Single-event-loop access (discord.py runs all event handlers on one
loop) means a plain dict is safe — no asyncio.Lock needed.
get_event_counters() returns a dict() copy so callers cannot mutate the
live counters.

Tests: 7 new unit tests in tests/gateway/test_discord_event_counters.py
covering counter shape, snapshot immutability, all three reaction
decision branches, accumulation across calls, and the mention-counters-
remain-zero contract.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…h_message

Use client._connection._messages deque iteration as a zero-cost lookup
before falling back to channel.fetch_message API call. Reduces Discord
rate-limit pressure on reaction events.

Only reactions on the bot's own messages route to the resolver; the new
filter sits after the existing reactor-user (Vector 2) check and reuses
the discord.reactions.skipped_non_bot_author counter from Phase 0.5.

discord.py exposes recent-message cache as a Deque[Message] (or None
when max_messages=None) at client._connection._messages — iterate with
next((m for m in cache if m.id == ...)), don't call .get() on a deque.
Falls through to fetch_message on cache miss, None cache, empty deque,
or missing _messages attr; fetch_message exceptions early-return safely
without bumping the author-filter counter.

Tests: 7 new unit tests in tests/gateway/test_reaction_cache_first.py
covering cache hit (bot/human), cache miss (success/raise), cache None,
empty deque, and missing-attribute fallback. Existing event-counter
test fixture seeds a bot-authored Message in the cache deque so the
Phase 0.5 counter tests stay green through the new filter.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors handle_inbound_reaction: builds a mention payload (text +
channel name/id), runs resolve_event_skills, increments the pre-declared
discord.mentions.{skipped_no_match,invoked} counters, and returns the
matched skill names to the caller. Wiring into _handle_message lands in
the next commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nbound_routing config gate

Vector 3 token-leak fix. When ``discord.mentions.inbound_routing`` is True
and the inbound message is an actual @mention of the bot, route through
the new ``handle_inbound_mention`` resolver hook BEFORE the legacy
``handle_message`` LLM invoke:

- Match → dispatch with ``auto_skill=<matched names>`` (skips
  general-purpose reasoning).
- No match + explicit triggers configured → early-return (saves the
  ~14.5K input tokens / mention cited in plan §3.3).
- No match + no explicit triggers OR feature flag off → fall through
  to legacy path unchanged (back-compat).

Resolver call is wrapped in try/except — any failure falls through to
legacy as a fail-safe. Default ``inbound_routing=False`` preserves the
current production behavior on every adapter that has not opted in.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ger paths

Pins the PR-B contract for ``handle_inbound_mention`` and the
``_handle_message`` dispatch decision:

test_discord_event_counters.py — three new ``handle_inbound_mention``
unit tests (no-match counter, match counter + return value, fail-safe
when skill_provider raises). Renames the legacy "unwired" class to
``TestMentionCountersUnaffectedByReactions`` since the counters are now
wired but reaction-only flows must still leave them at zero.

test_mention_routing.py — new file modeling the dispatch decision in
``DiscordAdapter._handle_message`` (line 3649). Covers the five contract
branches called out in plan §3.3 (inbound_routing=False default, match,
no-match-with-explicit-triggers early-return, no-match-without-triggers
legacy fall-through, skill_provider raises) plus channel_filter
match/exclusion. Uses the same composition primitives the wiring uses
(``has_explicit_triggers`` + the resolver hook) so tests stay decoupled
from a live Discord client.

All 18 new tests + 24 existing event-counter / interactions tests pass.
The pre-existing reaction-handler test regressions on this branch are
unrelated to PR-B (verified against pristine HEAD).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…active components)

PR NousResearch#19413 (TerminalSausage, "interactive components — buttons, select
menus, REST + WebSocket paths") is being salvaged by maintainer onto
current main. That PR owns Discord button emission + interaction
dispatch via the existing send_message tool extended with a `components`
schema. To avoid an on_interaction handler conflict (discord.py allows
only one registered handler per event) and reduce review surface, this
PR's button-specific wiring is removed.

Removed:
- tools/discord_button_tool.py (outbound discord_send_button_message tool)
- tests/tools/test_discord_button_message.py
- SkillButtonView class in gateway/platforms/discord_interactions.py
- handle_skill_button_interaction method
- _dispatch_synthetic method (button-only)
- _build_button_payload method (button-only)
- on_interaction event registration in gateway/platforms/discord.py
- Button-specific tests in tests/gateway/test_discord_interactions.py
- Button outbound sections in docs/migration/triggers-v1.md
- Button send/receive sections in CONTRIBUTING.md
- discord_button_tool entry in tests/tools/test_registry.py

Retained as framework extension points:
- metadata.hermes.triggers.button schema (agent/skill_utils.py)
- button event matcher in gateway/skill_resolver.py
- make_skill_custom_id / is_skill_custom_id / SKILL_CUSTOM_ID_PREFIX
  helpers in gateway/platforms/discord_interactions.py
- button trigger example in CONTRIBUTING.md (now annotated as bridge target)

Rationale: a follow-up PR can plug PR NousResearch#19413's component dispatch into
resolve_event_skills('button', payload, skills) via the retained helpers,
completing the round-trip from skill declaration → component emission →
click → skill dispatch.

This PR now covers: skill_resolver + frontmatter parser + Discord inbound
reaction routing + Discord mention routing + outbound reaction tools
(discord_add_reaction / discord_remove_reaction) + Feishu BC integration.

Tests: 132 pass across the trigger framework + reaction tools +
event counters + cache-first + mention routing suites.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@0xarkstar
0xarkstar force-pushed the feat/unified-trigger-framework branch from a731f36 to 27af45c Compare May 18, 2026 16:08
@0xarkstar

Copy link
Copy Markdown
Contributor Author

Closing this PR. Upstream has since shipped native equivalents of what this PR set out to do, and the code it patches has been replatformed:

  1. Discord was replatformed to a plugin architecture (plugins/platforms/discord/), removing gateway/platforms/discord.py / discord_interactions.py that this PR builds on — the diff can no longer be rebased onto current main (same situation that closed feat: add Gemma 4 tool call parser #7449).
  2. Channel→skill routing now exists natively: resolve_channel_skills() in gateway/platforms/base.py with channel_skill_bindings config (see tests/gateway/test_discord_channel_skills.py), covering the auto-skill dispatch use case this PR's mention/reaction trigger routing targeted.
  3. Discord components (buttons / clarify flows) also landed natively (tests/gateway/test_discord_clarify_buttons.py, tools/slash_confirm.py).

The remaining piece not covered upstream — cron token-leak mitigation — lives in #18255, which I've rebased onto current main and keeps standing on its own.

Thanks for the earlier triage; happy to re-open a focused follow-up if a gap in the native trigger handling turns up.

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

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/discord Discord bot adapter tool/skills Skills system (list, view, manage) type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants