Commit Graph

6 Commits (af7dceaf77bbcd5dcafe4f65982c7cd7df5f4c4a)

Author SHA1 Message Date
ooiuuii 87be36c240 fix(discord): bound component labels by UTF-16 units 2026-07-07 02:40:12 -07:00
Kenny John Jacob bce1e36b57 fix(discord): unwrap dict choices + soft-boundary truncate clarify buttons
Two bugs surfaced from production usage in #37134:

1. Dict choices rendered as Python repr. LLMs sometimes emit
   [{"description": "..."}] instead of bare strings; the old
   str(c).strip() coercion turned the whole dict into
   "{'description': '...'}" on the button label.

   Fix: add a _flatten_choice helper that unwraps dicts against
   the canonical LLM tool-call user-facing keys (label, description,
   text, title) in that order. Dicts with none of those keys are
   dropped. The "name" and "value" keys are deliberately NOT in the
   priority list — they're Discord-component-shaped fields that
   could appear in dicts that aren't meant to be choices (a
   developer-error wiring that passes a Button-shaped object);
   picking them would leak raw enum values or 4-char model
   identifiers onto user-facing buttons.

2. Mid-word truncation on long button labels. The old
   choice[:72] + "..." cut at position 72, mid-word. Worse, the
   three-char ellipsis ate into the 80-char Discord label cap,
   leaving only 75 chars of body.

   Fix: budget-aware cut strategy with three tiers:
     a. Last space in the trailing half of the budget (word boundary).
     b. Last soft boundary (- , . )) in the trailing half — used
        only when no word boundary exists.
     c. Hard cut at the budget limit (last resort).
   Use single U+2026 (…) to fit the cap. Cut AT soft boundaries
   (inclusive) so the label ends on the boundary char rather than
   on the alpha char that followed it.

Tests:
- test_unwraps_dict_choices_to_description: reproduces the
  screenshot in #37134, asserts the Python repr is gone.
- test_unwrap_prefers_description_over_name_in_multi_key_dict:
  regression guard for the name-key order in the unwrap list.
- test_unwrap_prefers_label_over_description: regression guard
  for label winning over description.
- test_unwrap_does_not_pick_value_or_name_alone: regression
  guard for the "name"/"value" fields being absent.
- test_truncates_long_choice_label: 200-char input, asserts
  total <= 80 and U+2026.
- test_truncates_long_choice_label_breaks_on_word_boundary:
  asserts the cut is on a space, not mid-word.
- test_truncates_long_no_space_choice_on_soft_boundary:
  adversarial input where position 76 is mid-word alpha, asserts
  the renderer falls back to a soft boundary.

Parity: telegram clarify suite (12 tests) still passes; the
helper is a Discord adapter local, not shared with the gateway.

Follow-up: gateway/platforms/telegram.py has the same str(c).strip()
pattern in its own send_clarify and will need a similar fix
(separate PR to keep this diff reviewable).

Fixes #37134
2026-06-19 06:31:08 -07:00
Teknium 9dbad1990b
test(discord): align clarify/model-picker tests with fail-closed component auth (#41338)
Three gateway tests broke on main after the component-auth security
hardening (test_discord_component_auth.py) made empty Discord component
allowlists fail-closed: a view built with allowed_user_ids=set() now
rejects every click instead of allowing anyone.

The clarify and model-picker BEHAVIOR tests still constructed their views
with an empty allowlist and expected the click to succeed — a stale
assumption from before the hardening. Fixed by giving each view an
allowlist containing the clicking user (the interaction's own id), which
is the realistic shape and what the security model requires.

Production code unchanged — this only updates the test fixtures to match
the intended (and separately pinned) fail-closed contract. The security
regression suite and these behavior suites now both pass.

Fixes:
- test_discord_clarify_buttons.py: test_choice_falls_back_to_label_text_when_entry_missing, test_other_flips_entry_to_awaiting_text
- test_discord_model_picker.py: test_model_picker_clears_controls_before_running_switch_callback
2026-06-07 08:27:40 -07:00
kshitijk4poor 66827f8947 chore: prune unused imports and duplicate import redefinitions
Remove unused imports (F401) and duplicate/shadowed import
redefinitions (F811) across the codebase using ruff's safe
autofixes. No behavioral changes -- imports only.

- ~1400 safe autofixes applied across 644 files (net -1072 lines)
- __init__.py re-exports preserved (excluded from F401 removal so
  public re-export surfaces stay intact)
- Re-exports that are imported or monkeypatched by tests but look
  unused in their defining module are kept with explicit # noqa:
  F401 (gateway/run.py load_dotenv; run_agent re-exports from
  agent.message_sanitization, agent.context_compressor,
  agent.retry_utils, agent.prompt_builder, agent.process_bootstrap,
  agent.codex_responses_adapter)
- Unsafe F841 (unused-variable) fixes deliberately skipped -- those
  can change behavior when the RHS has side effects
- ruff lints remain disabled in pyproject.toml (only PLW1514 is
  selected); this is a one-time cleanup, not a config change

Verification:
- python -m compileall: clean
- pytest --collect-only: all 27161 tests collect (zero import errors)
- core entry points import clean (run_agent, model_tools, cli,
  toolsets, hermes_state, batch_runner, gateway)
- static scan: every name any test imports directly from an edited
  module still resolves
2026-05-28 22:26:25 -07:00
kshitijk4poor cc8e5ec2af refactor(gateway): migrate Discord adapter to bundled plugin (full Teams parity)
First migration of an existing built-in platform adapter to the plugin
system established by IRC / Teams / LINE / Google Chat. Closes #24325;
advances the umbrella refactor in #3823.

Matches Teams' shape exactly — adapter under ``plugins/platforms/discord/``
with the standard ``__init__.py`` / ``adapter.py`` / ``plugin.yaml``
shell, ``register(ctx)`` entry point, **no back-compat shim** at the old
import path, and full parity for the four hooks Teams uses plus the
``apply_yaml_config_fn`` hook that landed in #25443 (the Discord plugin
is the first consumer of that hook):

* ``standalone_sender_fn`` — out-of-process cron delivery via REST API
* ``setup_fn`` — interactive ``hermes setup gateway`` wizard
* ``apply_yaml_config_fn`` — translate ``config.yaml`` ``discord:`` keys
  into ``DISCORD_*`` env vars (replaces the hardcoded block in
  ``gateway/config.py``)
* ``is_connected`` — declares connection state from ``DISCORD_BOT_TOKEN``
* ``check_fn`` — lazy-installs ``discord.py`` on demand
* plus ``allowed_users_env``, ``allow_all_env``, ``cron_deliver_env_var``,
  ``max_message_length``, ``emoji``, ``required_env``, ``install_hint``

* ``gateway/platforms/discord.py`` (5,101 LOC) →
  ``plugins/platforms/discord/adapter.py`` (git rename, R090).
* New ``plugins/platforms/discord/{__init__.py, plugin.yaml}`` with
  ``requires_env`` / ``optional_env`` declarations.
* Append ``register(ctx)`` block + new hook implementations
  (``_standalone_send``, ``interactive_setup``, ``_apply_yaml_config``,
  ``_clean_discord_user_ids``, ``_is_connected``, ``_build_adapter``,
  plus helpers ``_DISCORD_CHANNEL_TYPE_PROBE_CACHE`` etc.) to the
  adapter.

* Replace the ``Platform.DISCORD elif`` branch in
  ``GatewayRunner._create_adapter()`` (−9 LOC) with a generic post-creation
  hook (+6 LOC) in the registry path: any plugin adapter that declares a
  ``gateway_runner`` attribute now gets it auto-injected. Webhook's
  built-in branch is unchanged (it doesn't go through the registry path).

* Move ``_send_discord`` (190 LOC) and helpers
  (``_DISCORD_CHANNEL_TYPE_PROBE_CACHE``, ``_remember_channel_is_forum``,
  ``_probe_is_forum_cached``, ``_derive_forum_thread_name``) from
  ``tools/send_message_tool.py`` into the plugin as ``_standalone_send``.
* Wire via ``standalone_sender_fn=_standalone_send`` (Teams pattern; same
  gap fixed in #21804 for other plugin platforms).
* Replace the Discord ``elif`` in ``tools/send_message_tool.py``
  ``_send_to_platform`` with a 10-line registry-hook dispatch.
* Drop the ``DiscordAdapter`` import and the
  ``Platform.DISCORD: DiscordAdapter.MAX_MESSAGE_LENGTH`` ``_MAX_LENGTHS``
  entry — the registry's ``max_message_length=2000`` covers it.

* Move ``_setup_discord`` and ``_clean_discord_user_ids`` (68 LOC) from
  ``hermes_cli/setup.py`` into the plugin as ``interactive_setup``.
* Wire via ``setup_fn=interactive_setup``.  CLI helpers (``prompt``,
  ``print_info``, etc.) are lazy-imported so the plugin's module-load
  surface stays minimal.
* Remove ``"discord": _s._setup_discord`` from
  ``hermes_cli/gateway.py::_builtin_setup_fn``.
* Remove the entire 32-line ``_PLATFORMS["discord"]`` static dict entry —
  Discord's setup metadata is now discovered dynamically via
  ``_all_platforms()`` from the registry entry.

* Move the 59-line ``discord_cfg`` YAML→env bridge from
  ``gateway/config.py::load_gateway_config()`` into the plugin as
  ``_apply_yaml_config``.  Covers ``require_mention``,
  ``thread_require_mention``, ``free_response_channels``, ``auto_thread``,
  ``reactions``, ``ignored_channels``, ``allowed_channels``,
  ``no_thread_channels``, ``allow_mentions.{everyone,roles,users,
  replied_user}``, and ``reply_to_mode`` (including the YAML 1.1
  ``off``-as-False coercion and the ``extra.reply_to_mode`` fallback).
* Wire via ``apply_yaml_config_fn=_apply_yaml_config``.
* The hook runs BEFORE ``_apply_env_overrides`` and after the generic
  shared-key loop, exactly as documented in
  ``website/docs/developer-guide/adding-platform-adapters.md``.
* Behavior is preserved exactly — every assignment still uses
  ``not os.getenv(...)`` guards so env vars take precedence over YAML.

All 78 references to the old import path are rewritten — no back-compat
shim:

* 51 ``from gateway.platforms.discord import X`` →
  ``from plugins.platforms.discord.adapter import X``
* 5 ``import gateway.platforms.discord as discord_platform`` →
  ``import plugins.platforms.discord.adapter as discord_platform``
* 1 ``from gateway.platforms import discord as discord_mod`` →
  ``from plugins.platforms.discord import adapter as discord_mod``
* 21 ``mock.patch("gateway.platforms.discord.X")`` strings →
  ``mock.patch("plugins.platforms.discord.adapter.X")``
* 1 docstring reference in ``hermes_cli/commands.py``
* 1 import in ``tools/send_message_tool.py`` (now removed entirely)

The import-safety test in ``tests/gateway/test_discord_imports.py`` is
updated to purge the new canonical module name from ``sys.modules``.

**38 files changed, +621 / −473** — net positive due to the YAML hook
implementation (89 new LOC in the plugin trading for 59 deleted in core),
but every line moved has a clear plugin home now.  The git rename is
detected at R090 because the adapter gained ~340 LOC of moved-in hook
implementations (``_standalone_send`` + ``interactive_setup`` +
``_apply_yaml_config`` + helpers).

* All 568 Discord-specific tests pass across 25 ``test_discord_*.py``
  files plus voice/send/text-batching/reload-skills/stream-consumer/
  integration tests.
* All 147 tests in the YAML-touching subset
  (``test_discord_reply_mode``, ``test_discord_free_response``,
  ``test_discord_allowed_channels``, ``test_discord_allowed_mentions``,
  ``test_discord_channel_controls``, ``test_discord_reactions``,
  ``test_discord_thread_persistence``, ``test_runtime_footer``) pass —
  this is the strongest signal that the YAML→env hook behaves
  identically to the legacy block.
* Broader gateway/cron/integration sweep (1297 tests) introduces zero
  new failures vs ``main``.  Pre-existing failures in
  ``tests/gateway/test_tts_media_routing.py`` and
  ``tests/e2e/test_platform_commands.py`` reproduce identically on the
  unchanged ``main`` revision.
* Plugin discovery sanity check confirms Discord registers alongside the
  other four platform plugins:

    Registered platforms: ['discord', 'google_chat', 'irc', 'line', 'teams']

These Discord-shaped tendrils in core were **deliberately not moved** —
they are generic platform-registry concerns affecting every platform,
not Discord-specific:

* ``gateway/config.py:1205`` ``DISCORD_BOT_TOKEN → config.token`` env
  enablement — same shape Telegram has.  The existing
  ``env_enablement_fn`` registry hook only seeds ``extra``, not
  ``.token``, so it can't replace this without an adapter refactor to
  read from ``extra["bot_token"]``.
* ``gateway/run.py`` voice-mode hooks
  (``self.adapters.get(Platform.DISCORD)`` for
  ``start_voice_mode``/``stop_voice_mode``), role-based auth,
  ``DISCORD_ALLOW_BOTS`` branch in ``_is_user_authorized``,
  ``_UPDATE_ALLOWED_PLATFORMS`` frozenset, and the per-platform
  allowlist maps — generic platform-registry concerns.
* ``Platform.DISCORD`` enum literal — stable identifier used as dict
  keys throughout the codebase; removing it is a separate refactor with
  no real benefit.
* ``tools/discord_tool.py`` and ``tools/environments/local.py`` —
  first-class agent tools and env-passthrough config, neither is the
  gateway adapter.

Each of these is worth its own scoping issue when the time comes.
2026-05-22 14:21:41 -07:00
teknium1 1dca6a6960 feat(discord): render clarify choices as buttons
Brings Discord to parity with Telegram on the clarify tool's interactive
UX. Overrides BasePlatformAdapter.send_clarify on DiscordAdapter to attach
a button view when choices are present.

  - ClarifyChoiceView: one discord.ui.Button per choice (max 24, Discord's
    25-component view cap leaves one slot for Other) plus a final
    'Other (type answer)' button.
  - Numeric click -> tools.clarify_gateway.resolve_gateway_clarify(
    clarify_id, choice_text) using the canonical choice text from the
    gateway entry (falls back to the button label if the entry vanished).
  - Other click -> tools.clarify_gateway.mark_awaiting_text(clarify_id) so
    the gateway's text-intercept captures the next user message in this
    session as the response.
  - Auth via the shared _component_check_auth helper (same OR-semantics as
    ExecApprovalView / SlashConfirmView / UpdatePromptView / ModelPickerView).
  - Open-ended (no choices) path renders the prompt as a plain embed and
    relies on the existing text-intercept resolution.
  - Single-use: first valid click disables every button and updates the
    embed footer with who answered and what they chose.

No changes to BasePlatformAdapter.send_clarify or the gateway's
clarify_callback wiring -- the existing scaffolding already drives all
adapters; Discord just inherits the default text fallback today and gains
buttons by virtue of this override.

Test conftest extended: _FakeEmbed gains add_field() / set_footer() stubs
so tests can construct embedded views without monkey-patching per-test.

Original PR: #19249 by @LeonSGP43. This is a reshape of the contributor's
work onto current main's clarify infrastructure (clarify_id + entry-based
resolution shared with Telegram, instead of a parallel on_answer-closure
mechanism). The button view structure and UX shape are preserved.

Tests: 14 new tests in tests/gateway/test_discord_clarify_buttons.py.
391/391 existing Discord gateway tests still pass.

Co-authored-by: LeonSGP43 <cine.dreamer.one@gmail.com>
2026-05-14 07:26:43 -07:00