Skip to content

feat(feishu): opt-in approval mention notifications on exec prompts - #66780

Open
9lie wants to merge 1 commit into
NousResearch:mainfrom
9lie:feat/feishu-approval-mentions
Open

feat(feishu): opt-in approval mention notifications on exec prompts#66780
9lie wants to merge 1 commit into
NousResearch:mainfrom
9lie:feat/feishu-approval-mentions

Conversation

@9lie

@9lie 9lie commented Jul 18, 2026

Copy link
Copy Markdown

What does this PR do?

Adds an opt-in Feishu (Lark) approval mention feature. When feishu.approval_mentions is enabled in config.yaml (under gateway.platforms.feishu.extra), dangerous-command approval cards prepend <at user_id="ou_xxx"> for each configured admin so they receive a Feishu mention notification instead of silently waiting.

The feature is default-off, only mentions admins with valid ou_ open_ids (Feishu <at> tag requires open_id format), and is implemented entirely within the Feishu adapter -- no gateway/run.py changes.

Related Issue

No issue linked. Duplicate/overlap search performed:

  • No existing Feishu issue or PR for approval mention notifications.
  • Discord already has this feature: discord.approval_mentions (commit e0176cb, PR feat: optionally mention Discord approval owners #39719). This PR adds the same capability for Feishu, mirroring the config name, default, and adapter-internal approach.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • plugins/platforms/feishu/adapter.py:
    • Added approval_mentions: bool = False field to FeishuAdapterSettings
    • _load_settings parses extra.get("approval_mentions") (also supports FEISHU_APPROVAL_MENTIONS env var)
    • _apply_settings propagates the setting to the adapter instance
    • send_exec_approval builds a <at user_id="ou_xxx"> mention prefix for each admin with a valid ou_ open_id, prepended to the card markdown before the code block
  • tests/gateway/test_feishu_approval_buttons.py:
    • 6 new tests covering: mention rendering (enabled/disabled), no-admin noop, config propagation (_load_settings + _apply_settings), default-off behavior

How to Test

  1. Run tests:

    uv run --with pytest --with pytest-asyncio --with pytest-xdist \
      pytest tests/gateway/test_feishu_approval_buttons.py -v

    All 11 tests pass (5 existing + 6 new).

  2. Manual test on Feishu:

    • Set approval_mentions: true under gateway.platforms.feishu.extra in config.yaml
    • Configure at least one admin with an ou_ open_id
    • Trigger a dangerous command (e.g., git reset --hard in a test repo)
    • Verify the approval card @-mentions you in Feishu

Checklist

Code

  • I have read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this is not a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I have run pytest tests/gateway/test_feishu_approval_buttons.py -v and all tests pass
  • I have added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I have tested on my platform: macOS (Apple Silicon), Feishu WebSocket gateway

Documentation & Housekeeping

  • I have updated relevant documentation (README, docs/, docstrings) - or N/A
  • I have updated cli-config.yaml.example if I added/changed config keys - or N/A: Feishu has no existing config block in cli-config.yaml.example; Discord's approval_mentions (commit e0176cb) also does not appear there
  • I have updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows - or N/A: no architecture or workflow changes
  • I have considered cross-platform impact (Windows, macOS) per the compatibility guide - or N/A
  • I have updated tool descriptions/schemas if I changed tool behavior - or N/A: no tool schema or LLM-facing tool description changes

Alignment with Discord

Aspect Discord (commit e0176cb) Feishu (this PR)
Config name discord.approval_mentions feishu.approval_mentions
Default off off
@ target allowlist numeric user IDs admins with ou_ open_ids
Mention position content prefix card markdown prefix
Mention syntax <@uid> <at user_id="ou_xxx">
Notification scoping AllowedMentions (users only) N/A (<at> is inherently targeted)
gateway/run.py changes none none

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins platform/feishu Feishu / Lark adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 18, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to merged #60493, which implemented the analogous default-off Discord approval mentions. This PR ports the capability to Feishu.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

{
"event": "COMMENT",

Code Review Summary\n\nVerdict: Approved\n\nLooks good. No obvious issues found.\n\n---\nReviewed by Hermes Agent",

"comments": []
}

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused Feishu port. The runtime path is sound: the adapter initializes from config.extra at plugins/platforms/feishu/adapter.py:1444-1445, and the proposed mention targets come from the existing admin set used for approval authorization (plugins/platforms/feishu/adapter.py:2694-2702). Current remote main still has no equivalent feature (send_exec_approval renders the unprefixed markdown at plugins/platforms/feishu/adapter.py:1979-2051).

Problems

  • The PR introduces approval_mentions and FEISHU_APPROVAL_MENTIONS without documenting either. The Feishu guide already documents admins and platforms.feishu.extra at website/docs/user-guide/messaging/feishu.md:491-529, plus its supported environment variables at :539-566.

Suggested changes

  • Add a default-off approval_mentions example beside the existing Feishu admins configuration, explain that only ou_ admin IDs are mentioned, and document the environment fallback if it remains supported.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 18, 2026
Add feishu.approval_mentions config (default off). When enabled,
exec-approval cards prepend <at user_id="ou_xxx"> for each configured
admin open_id, so they receive a Feishu mention notification instead of
silently waiting.

Mirrors discord.approval_mentions (commit e0176cb):
- Default off, opt-in via config.yaml
- Only mentions admins with valid ou_ open_ids
- Mention prepended to card markdown, before the code block
- No gateway/run.py changes (adapter-internal, like Discord approach)

Tests: 11 total (5 existing + 6 new covering mention rendering, config
propagation, and edge cases).
@9lie
9lie force-pushed the feat/feishu-approval-mentions branch from b1905da to af8190d Compare July 19, 2026 02:07
@9lie

9lie commented Jul 19, 2026

Copy link
Copy Markdown
Author

Documentation added per review feedback: approval_mentions and FEISHU_APPROVAL_MENTIONS are now documented in both English (website/docs/user-guide/messaging/feishu.md) and Chinese (website/i18n/zh-Hans/.../feishu.md), covering the YAML config example, a feature explanation section under Interactive Card Actions, and the environment variable table.

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 P3 Low — cosmetic, nice to have platform/feishu Feishu / Lark adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants