feat(discord): add /kill and /stop to interrupt a running agent - #253
feat(discord): add /kill and /stop to interrupt a running agent#253tbertran wants to merge 5 commits into
Conversation
Telegram gained /kill in moazbuilds#184 but Discord never got the equivalent, so a Discord user has no way to stop a run in flight — the only recourse is restarting the daemon. Wire the existing killActive() into the Discord message handler, dispatched before skill routing so it takes effect immediately rather than queueing behind the run it is meant to cancel. /stop is accepted as an alias. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
TerrysPOV
left a comment
There was a problem hiding this comment.
Solid, well-scoped change and the mechanics check out — /kill and /stop are dispatched before skill routing and runUserMessage, so they take effect immediately rather than queueing behind the run they cancel; the dispatch sits behind the auth gate (unauthorized users can't kill runs); sendMessage(token, channelId, …) is correct for Discord; and it reuses the existing transport-agnostic killActive() with fork runs correctly excluded. It merges clean against master.
Three things before this can ship:
-
Please demonstrate
/killend-to-end on a live Discord daemon before merge. You've flagged yourself that it hasn't been exercised yet — and an interrupt control is exactly the kind of thing that has to be shown working, not inferred from parity with the Telegram path. A short confirmation (or a clip) that a/killin a real channel actually stops an in-flight run is the bar here. -
Missing version bump.
discord.tsis shipped plugin content, soplugin-version-guard/marketplace-version-guardrequire a bump — but no.claude-plugin/*.jsonis touched. Runbun run bump:plugin-version+bun run bump:marketplace-versionand target 1.0.44 (master is at 1.0.43). -
Please note the current blast radius in the PR (and ideally the code comment):
killActive()is global — it kills every in-flight main run across all Discord channels/threads, not just the caller's. On a multi-channel server a/killin one channel cancels everyone's runs, and combined with the channel-scoped allowlist (#248) a user scoped to one channel can kill runs in channels they aren't authorized for. This matches Telegram's existing/killcontract, so it's not a regression and I'm not asking you to fix it here — but it should be documented, and the thread-scoped fix tracked separately (see the same concern raised on #237).
claudeclaw/src/commands/discord.ts
Lines 979 to 983 in 3cd8bad
…st radius Addresses review feedback on moazbuilds#253: - plugin-version-guard / marketplace-version-guard require a bump for shipped plugin content; upstream master already consumed 1.0.44, so this lands on 1.0.45. - Note the blast radius of killActive() at the call site. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
killActive() terminates every in-flight main run across all channels and threads. Matches Telegram's existing contract (moazbuilds#184), not a regression; thread-scoped kill tracked separately (moazbuilds#237). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks for the review — all three addressed. 1. 2. Version bumped — to 1.0.45, not 1.0.44. Between your review and this push, upstream 3. Blast radius documented. Added a "Known limitation: kill is global, not caller-scoped" section to the PR description covering the cross-channel kill, the interaction with the channel-scoped allowlist (#248), that it matches Telegram's existing contract (#184) and so is not a regression, and that thread-scoped kill is tracked separately (#237). Also noted at the call site in Two other things worth knowing:
Test results after the merge: 131 pass, 6 fail. The same 6 fail on a clean CI note: Separately, |
Problem
Telegram gained
/killin #184 (feat(telegram): live streaming, /verbose tool display, /fork, /kill), but Discord never got the equivalent.killActive()and themainActiveProcsregistry already exist inrunner.tsand are transport-agnostic — Discord just never imported them. The result is that a Discord user has no way to stop a run in flight. If the agent goes off the rails, the only recourse is killing the daemon process.Change
Wire the existing
killActive()into the Discord message handler:/kill(and/stopas an alias) kills all main-queueclaudesubprocesses and confirms in-channel.runUserMessage()— so it takes effect right away instead of queueing behind the run it is meant to cancel. This works because Discord's gateway message handler is event-driven and runs concurrently with an in-flight run.mainActiveProcsdeliberately excludes them, per the existing contract inrunner.ts.Known limitation: kill is global, not caller-scoped
killActive()terminates every in-flight main run across all Discord channels and threads, not just the caller's. Two consequences worth stating plainly:/killin one channel cancels runs other people started elsewhere.This matches Telegram's existing
/killcontract (#184), so it is not a regression introduced here, and this PR deliberately does not change it. Thread/channel-scoped kill is tracked separately — the same concern is raised on #237. The limitation is also documented at the call site.Version
plugin.jsonandmarketplace.jsonbumped to 1.0.45. The review asked for 1.0.44, but upstreammasterconsumed 1.0.44 via a merged PR in the interim, so 1.0.45 is the next free version.Also included
chore(discord): update fallback model to claude-sonnet-5— an unrelated one-line model-string update that was sitting in the same working tree. Kept as its own commit so it can be dropped independently if you would rather it ship separately.Testing
bun build src/index.ts— clean.bun test— 131 pass, 6 fail. The same 6 fail on a cleanmasteron this machine (pre-existing:plugins.test.ts, andjobs.test.tsassertingtoContain("/.claude/")against Windows backslash paths). No change in results./killtyped in a real channel against an in-flight run returnedKilled active agent.and the run stopped. The terminated subprocess then surfacedError (exit 1): Unknown errorin-channel — that is the killed process's abnormal exit travelling the normal error path, and is corroborating evidence that the process really was terminated rather than allowed to finish. Suppressing that message when the exit was a deliberate kill is worth a follow-up.Branch is synced with upstream
mastervia a merge commit (no force-push).🤖 Generated with Claude Code