Skip to content

feat(gateway): opt-in Discord requester mention on approval prompts - #21528

Closed
donovan-yohan wants to merge 2 commits into
NousResearch:mainfrom
donovan-yohan:feat/discord-mention-approval-requester
Closed

feat(gateway): opt-in Discord requester mention on approval prompts#21528
donovan-yohan wants to merge 2 commits into
NousResearch:mainfrom
donovan-yohan:feat/discord-mention-approval-requester

Conversation

@donovan-yohan

@donovan-yohan donovan-yohan commented May 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds an opt-in Discord setting, discord.mention_exec_approval / DISCORD_MENTION_EXEC_APPROVAL, default false.

When enabled, dangerous-command approval prompts on Discord prepend <@user_id> command approval needed so the original requester gets notified while the agent is blocked waiting for approval.

The mention is constrained to a digit-only requester ID and the outgoing approval message uses per-message AllowedMentions restricted to that single user. @everyone, @here, role pings, and replied-user pings remain disabled.

Configuration

discord:
  mention_exec_approval: true
DISCORD_MENTION_EXEC_APPROVAL=true

If both are present, the environment variable wins. Blank or invalid env values stay disabled.

Implementation

  • Reads the YAML setting into Discord platform extra; does not seed process env from YAML, so gateway config reloads cannot get stuck behind a self-written env var.
  • Adds approval-only metadata containing requester_user_id for Discord human requesters.
  • Keeps normal status/progress metadata generic (thread_id, Telegram DM topic fallback, Feishu reply metadata).
  • send_exec_approval() adds mention content only when the setting is enabled and the requester ID is digit-only.

Verification

  • ulimit -n 4096 && ./scripts/run_tests.sh tests/gateway/test_config.py tests/gateway/test_discord_send.py tests/gateway/test_status_thread_metadata.py -q — 87 passed
  • ulimit -n 4096 && ./scripts/run_tests.sh tests/gateway/test_tts_media_routing.py -q — 6 passed
  • git diff --check upstream/main...HEAD
  • conflict-marker scan over changed files

Full tests/gateway -q was also attempted locally: the PR-relevant suites passed, while the run still had unrelated existing/baseline failures around Platform.GOOGLE_CHAT plugin enum registration plus two parallel timing failures that passed when rerun in isolation.

Risk

Low: default off, requester IDs are validated before mention construction, and the mention allowlist is scoped per approval message.

Copilot AI review requested due to automatic review settings May 7, 2026 21:41

Copilot AI 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.

Pull request overview

Adds an opt-in Discord feature to @mention the original requester when a dangerous-command execution approval prompt is posted, improving responsiveness (especially for mobile users) while keeping mentions tightly scoped via per-message allowed_mentions.

Changes:

  • Introduces discord.mention_exec_approval / DISCORD_MENTION_EXEC_APPROVAL (default false) and wires it through config/env bridging.
  • Refactors status/approval metadata construction into gateway.run._build_status_thread_metadata, adding Discord requester fields while preserving Feishu reply-in-thread behavior.
  • Updates Discord adapter send_exec_approval() to conditionally prepend a validated <@user_id> mention and pin allowed_mentions to that single user; adds/extends tests and documentation.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
gateway/run.py Extracts status/approval metadata builder and enriches metadata for Discord/Feishu.
gateway/platforms/discord.py Adds config reader + mention/allowed_mentions behavior in exec approval sends.
gateway/config.py Bridges discord.mention_exec_approval from YAML to adapter extra and env var.
tests/gateway/test_status_thread_metadata.py Adds focused unit tests for the new metadata builder behavior.
tests/gateway/test_discord_send.py Adds tests covering ping-enabled/disabled paths and invalid requester ID handling.
tests/gateway/test_config.py Adds tests for YAML→env bridging and env precedence.
tests/gateway/conftest.py Extends the shared Discord mock to support embed fields and allowed-mentions assertions.
website/docs/user-guide/messaging/discord.md Documents the new Discord setting and behavior.
website/docs/reference/environment-variables.md Documents the new env var and links to the Discord docs section.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread gateway/platforms/discord.py Outdated
Comment thread gateway/platforms/discord.py Outdated
Comment thread website/docs/user-guide/messaging/discord.md Outdated
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery platform/discord Discord bot adapter labels May 7, 2026
@donovan-yohan
donovan-yohan force-pushed the feat/discord-mention-approval-requester branch from 8c7d7f5 to 50d4d66 Compare May 10, 2026 14:34
@donovan-yohan

Copy link
Copy Markdown
Contributor Author

Rebased/resolved conflicts against current main and force-updated the PR branch.

Follow-up fixes included:

  • preserved upstream Telegram DM topic fallback metadata during conflict resolution
  • fixed DISCORD_MENTION_EXEC_APPROVAL precedence so an explicit env var overrides YAML
  • tightened default-off boolean parsing so blank/invalid values do not enable pings
  • kept Discord approval send tests robust when ExecApprovalView is absent under faked discord imports
  • fixed the post-stream media helper fallback surfaced by the full gateway suite

Verification:

  • ulimit -n 4096 && pytest tests/gateway/test_discord_send.py tests/gateway/test_status_thread_metadata.py tests/gateway/test_config.py tests/gateway/test_tts_media_routing.py -q — 85 passed
  • ulimit -n 4096 && pytest tests/gateway -q — 5243 passed, 10 skipped
  • git diff --check nous/main...HEAD
  • conflict-marker scan clean

Current head: 50d4d6662c300a65c4266f3ba49baa4be1785176

Copilot AI 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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@donovan-yohan
donovan-yohan force-pushed the feat/discord-mention-approval-requester branch from 50d4d66 to 7e0749c Compare May 11, 2026 15:12
@donovan-yohan
donovan-yohan requested a review from Copilot May 11, 2026 23:15

Copilot AI 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.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Comment thread gateway/config.py Outdated
@donovan-yohan
donovan-yohan force-pushed the feat/discord-mention-approval-requester branch from 7e0749c to 570b69f Compare May 16, 2026 06:28
@donovan-yohan

Copy link
Copy Markdown
Contributor Author

Rebased against current main and force-updated the PR branch.

Follow-up cleanup:

  • resolved merge conflicts while preserving upstream Discord thread mention gating, Telegram DM topic fallback, and Feishu topic reply metadata
  • removed the YAML→env self-seeding path for DISCORD_MENTION_EXEC_APPROVAL so config reloads cannot get stuck behind a process env var written by the loader
  • kept requester metadata approval-only and reduced it to requester_user_id
  • removed noisy inline rationale comments / misleading AllowedMentions wording
  • dropped an unrelated post-stream media helper fallback from this PR

Verification:

  • ulimit -n 4096 && ./scripts/run_tests.sh tests/gateway/test_config.py tests/gateway/test_discord_send.py tests/gateway/test_status_thread_metadata.py -q — 87 passed
  • ulimit -n 4096 && ./scripts/run_tests.sh tests/gateway/test_tts_media_routing.py -q — 6 passed
  • git diff --check upstream/main...HEAD
  • conflict-marker scan over changed files

GitHub now reports mergeable: MERGEABLE at head 570b69f577bb88928ea703cd8d82c73ffc6ce68e.

Add opt-in Discord requester mentions for dangerous command approval prompts, with scoped allowed_mentions, config/env bridging, docs, and gateway tests.
@donovan-yohan
donovan-yohan force-pushed the feat/discord-mention-approval-requester branch from 570b69f to 06028cb Compare May 19, 2026 00:33
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 P3 Low — cosmetic, nice to have platform/discord Discord bot adapter type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants