Skip to content

feat(discord): add message edit and delete admin actions - #15864

Open
StatPan wants to merge 1 commit into
NousResearch:mainfrom
StatPan:feat/discord-message-admin
Open

feat(discord): add message edit and delete admin actions#15864
StatPan wants to merge 1 commit into
NousResearch:mainfrom
StatPan:feat/discord-message-admin

Conversation

@StatPan

@StatPan StatPan commented Apr 26, 2026

Copy link
Copy Markdown

Summary

  • add discord_admin.edit_message(channel_id, message_id, content)
  • add discord_admin.delete_message(channel_id, message_id)
  • wire both through the existing Discord action manifest, schema, handler defaults, required-parameter validation, and 403 hints
  • add tests for action calls, registry handler content propagation, admin/core schema split, validation, and 403 enrichment

Part of #1559.

Test Plan

  • python3 -m pytest tests/tools/test_discord_tool.py -q -o addopts=

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have platform/discord Discord bot adapter comp/tools Tool registry, model_tools, toolsets labels Apr 26, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the complete action wiring and focused tests. The edit_message capability is still absent from the current Discord admin manifest (tools/discord_tool.py:633-697) and is worth salvaging.

Problems

  • delete_message already landed on current main in 8b32a9d0f1705a126d838e2ecac173de7960b87a; its implementation is at tools/discord_tool.py:581-584, with existing validation and tests. The PR's duplicate delete implementation conflicts with that current code and changes its response shape.
  • The action list documented for discord.server_actions at hermes_cli/config.py:2447-2449 should include both existing delete_message and proposed edit_message.

Suggested changes

  • Salvage the edit-only wiring and tests onto current main while preserving the landed deletion behavior.
  • Extend the discord.server_actions action-list comment with both message actions.

Automated hermes-sweeper review.

@teknium1 teknium1 added 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 sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026
@StatPan
StatPan force-pushed the feat/discord-message-admin branch from a3a8bc4 to 1abfc7a Compare July 13, 2026 07:49

StatPan commented Jul 13, 2026

Copy link
Copy Markdown
Author

Thanks — updated this against current main. Kept the existing delete_message behavior and tests intact; the PR now adds only edit_message and updates the server_actions list. scripts/run_tests.sh tests/tools/test_discord_tool.py passes.

StatPan commented Jul 13, 2026

Copy link
Copy Markdown
Author

@teknium1 This is ready for another look when you have a moment — thanks.

@GottZ GottZ 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.

This was generated by AI during triage.

Summary

Three PRs address Discord message mutation: #19052 added deletion, #21197 merged that deletion as a salvage, and the current #15864 diff adds the still-missing edit action while preserving the landed deletion behavior.

Related pull requests

  • #15864 related — (+113/-2) — merge: The current diff adds edit_message end to end—PATCH implementation, manifest/schema wiring, required-content validation, registry propagation, 403 guidance, configuration documentation, and focused tests—without replacing the deletion behavior already on main. This follows the keep_open review on #15864, including its request to salvage only editing and document both actions.
  • #19052 [closed] related — (+31/-2) — superseded duplicate: This closed PR implemented delete_message across the handler, action manifest, validation, 403 guidance, and tests, but its change was subsequently salvaged and merged through #21197; it remains relevant as the original implementation and attribution source.
  • #21197 [merged] related — (+32/-2) — merged reference implementation: This merged PR carries essentially the full #19052 deletion diff plus release attribution, establishing delete_message on main and resolving the deletion portion that the earlier version of #15864 overlapped.

Duplicates

#19052 and #21197 contain essentially the same delete_message implementation; #21197 is the merged salvage of #19052.

Suggested consolidation

Merge #15864 for the non-duplicative edit_message capability, consistent with its keep_open review and updated edit-only diff. Keep #19052 closed as superseded by merged #21197; no further duplicate PR needs closing.

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

@StatPan
StatPan force-pushed the feat/discord-message-admin branch from 1abfc7a to 89f2da9 Compare August 2, 2026 00:43

StatPan commented Aug 2, 2026

Copy link
Copy Markdown
Author

@teknium1 Updated against current main as requested: preserved the landed delete_message behavior, added only edit_message, and updated the discord.server_actions documentation. scripts/run_tests.sh tests/tools/test_discord_tool.py passes (45 tests). Thanks for the review.

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

Labels

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-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