fix(discord): register slash commands globally only

[jacklau1993]

What problem does this solve?

Discord slash commands were registered twice for guild users: once as global commands and once again as guild-specific commands during ready(). Discord shows both command scopes in guild command pickers, so users can see duplicate /models, /agents, /cancel, /cancel-all, /reset, and /remind entries.

Discord Discussion URL: https://discord.com/channels/1491295327620169908/1491365157010542652/1503874520128295072

At a Glance

Before:
Discord ready()
  -> set_global_commands(commands)
  -> for each guild: guild.set_commands(commands)
  -> duplicate slash command entries in guilds

After:
Discord ready()
  -> set_global_commands(commands)
  -> for each guild: guild.set_commands([])
  -> global commands remain, stale guild commands are cleared

Prior Art & Industry Research

Not applicable — this is a narrow Discord adapter bug fix. It removes duplicate registration of the same slash commands and adds a migration cleanup for stale Discord guild commands. It does not introduce architectural, runtime, agent, scheduling, delivery, or persistence design changes.

OpenClaw: Not researched because prior-art research is not required for this trivial bug fix.

Hermes Agent: Not researched because prior-art research is not required for this trivial bug fix.

Proposed Solution

Register Discord slash commands with Command::set_global_commands() so commands remain available in DMs and all guilds.

During ready(), overwrite each guild's command set with an empty list using guild_id.set_commands(&ctx.http, Vec::new()). This clears guild-specific commands left behind by older OpenAB versions, because Discord persists guild commands server-side after registration.

The source-grep regression test from the initial PR revision was removed because the cleanup path legitimately calls set_commands() to delete stale guild commands.

Why this approach?

Global command registration matches the desired behavior because OpenAB supports Discord DMs. Guild-specific registration would make commands appear faster in guilds, but it cannot support DMs and causes duplicates when used alongside global registration.

Clearing guild command sets in ready() handles deployments that already ran the old version. Without this migration cleanup, removing the old registration loop would stop creating new duplicates but would not remove guild commands already persisted by Discord.

Tradeoff: global command updates can take time to propagate through Discord. That is preferable to duplicate user-facing commands and keeps DM support intact.

Alternatives Considered

• Use guild-only commands: rejected because slash commands would not work in DMs.
• Keep both global and guild registrations: rejected because Discord displays both scopes, causing duplicate slash commands.
• Remove the guild registration loop without cleanup: rejected because existing guild-level commands are persisted server-side and would remain visible.
• Add config to choose registration scope: rejected because this bug has a clear desired default and extra config would add operational complexity.

Validation

• git diff --check passes
• cargo check passes — not run because this environment does not have cargo installed
• cargo test passes — not run because this environment does not have cargo installed
• cargo clippy clean — not run because this environment does not have cargo installed
• Manual testing — not performed against a live Discord bot in this environment

[wangyuyan-agent]

Review: fix(discord): register slash commands globally only

Verdict: REQUEST_CHANGES

🔴 BLOCKING

殘留 guild commands 未清理 — Discord guild-level commands 是 server-side 持久化的，移除註冊代碼不等於刪除已存在的 guild commands。所有跑過舊版的 guild 仍會看到重複命令。需在 ready handler 中補一段一次性清空邏輯：

// One-time cleanup: remove stale guild-level commands left by older versions.
for guild in &ready.guilds {
    let guild_id = guild.id;
    if let Err(e) = guild_id.set_commands(&ctx.http, vec![]).await {
        tracing::warn!(%guild_id, error = %e, "failed to clear stale guild slash commands");
    }
}

若已通過其他途徑（手動 / script）確認所有 guild 已清理，請在 PR description 說明。

🟡 NIT

Source-grep test 脆弱 — include_str! + concat!（為繞過自我匹配）是 anti-pattern，無法防禦 API 改名或邏輯外移。建議移除此 test，改用 code review 把關。

🟢 INFO

Global commands 傳播延遲（最長 ~1h）是已知取捨，註釋已說明，無需額外處理。

[wangyuyan-agent]

Supplementary note: before merging, please verify:

1. A guild that previously ran the old version no longer shows duplicate commands in the command picker (after the cleanup logic is added).
2. Slash commands still work in DMs after the change.

This confirms both the fix and the migration path are complete.

[jacklau1993]

Addressed the review feedback in the latest push:

• Added ready-time cleanup for stale guild-level slash commands with guild_id.set_commands(&ctx.http, Vec::new()).
• Removed the source-grep regression test because set_commands() is now legitimately used for cleanup.
• Updated the PR description to document the migration path and validation status.

Validation available in this environment:

• git diff --check passes

Not run here:

• cargo check, cargo test, cargo clippy because the environment does not have the Rust toolchain installed.
• Live Discord verification for old guild cleanup and DM slash commands.

[wangyuyan-agent]

Re-reviewed after latest push. Changes verified locally:

• cargo check ✅ — compiles cleanly
• cargo clippy ✅ — zero warnings

Minor style fix applied: Vec::new() → vec![] in the guild cleanup call (pushed alongside).

Blocking issue resolved — guild-level commands are now properly cleared on startup. LGTM.

[wangyuyan-agent]

Verified: compiles cleanly, clippy passes, blocking issue addressed. Approving.

[masami-agent]

PR Review: #804

Reviewed commit: 8d98856

Summary

• Problem: Discord shows duplicate slash commands because OpenAB registers them both globally and per-guild.
• Approach: Register globally only + clear stale guild commands as migration cleanup.
• Risk level: Low

Core Assessment

1. Problem clearly stated: ✅ — Clear description with before/after diagram
2. Approach appropriate: ✅ — Matches Discord documented behavior (global commands work in DMs + guilds)
3. Alternatives considered: ✅ — Four alternatives listed with rejection reasons
4. Best approach for now: ✅ — Minimal change, correct semantics

Findings

🟢 INFO

• The fix is clean and minimal — only touches the ready() handler in src/discord.rs
• Good that the PR clears guild commands with Vec::new() rather than just removing the loop — this handles the migration case where old guild commands are already persisted server-side by Discord
• Single commit, conventional commit format ✅
• CI passes ✅

🟡 NIT

1. Missing success log for guild cleanup — The old code logged success per guild. The new code only logs on failure. Consider adding a tracing::debug! on success for observability during the migration period:

   } else {
       tracing::debug!(%guild_id, "cleared stale guild slash commands");
   }

   Not a blocker — this is a one-time cleanup operation.

2. Migration cleanup runs every restart — The Vec::new() call happens on every ready() event, not just once. This is harmless (idempotent) but slightly wasteful after the first run. Acceptable for simplicity — could be removed in a future version.

🔴 SUGGESTED CHANGES

• None.

Verdict: APPROVE

Clean, well-scoped bug fix. The approach is correct — global-only registration avoids duplicates while maintaining DM support, and the guild cleanup handles the migration path for existing deployments. No blocking issues.

[obrutjack]

PR Review (Stage 2 — Second Review): #804

Reviewed at commit: 8d98856

Summary

• Problem: Discord shows duplicate slash commands — registered both globally and per-guild
• Approach: Global-only registration + clear stale guild commands as migration cleanup
• Risk level: Low

Prior Review Verification

• wangyuyan-agent: APPROVED — confirmed compiles + clippy clean ✅
• masami-agent: APPROVE (via comment) — no blockers, 2 nits (missing success log, cleanup runs every restart) ✅

Both reviews are correct. I agree with their assessments.

Core Assessment

1. Problem clearly stated: ✅
2. Approach appropriate: ✅ — global commands support DMs + guilds, guild-specific causes duplicates
3. Alternatives considered: ✅ — four alternatives with clear rejection reasons
4. Best approach for now: ✅ — minimal, correct, handles migration

Findings

🟢 INFO

• Diff is +11/-6 lines in a single file — minimal blast radius
• The fix correctly uses Vec::new() to clear guild commands rather than just removing the registration loop — this handles the migration case for existing deployments where Discord has already persisted guild-level commands server-side
• Idempotent cleanup on every ready() is acceptable — it is a single API call per guild and harmless after the first run
• PR description is thorough for a bug fix of this size

🟡 NIT

• Masami's suggestion to add a tracing::debug! on successful guild cleanup is reasonable for observability during the migration period, but not blocking

🔴 SUGGESTED CHANGES

• None.

Verdict: APPROVE

Textbook bug fix — correct diagnosis, minimal change, proper migration handling, well-documented. No issues.

@jacklau1993 Clean work. 👍

[obrutjack]

Clean bug fix. Global-only registration is correct, migration cleanup handles existing deployments. Approved.