feat: passthrough unknown slash commands to Claude in agentic mode

[haripatel07]

Problem

In agentic mode, unknown slash commands (e.g. /workflow, /skill, /add-memory)
were silently dropped because the TEXT handler excluded all COMMAND messages via
filters.TEXT & ~filters.COMMAND.

Fix

Remove ~filters.COMMAND from the agentic text MessageHandler so unrecognised
slash commands are forwarded to Claude as natural language.

Registered commands (/start, /new, /status, etc.) still take priority via
higher-priority CommandHandlers and are completely unaffected.

Classic (non-agentic) mode retains the original filter — unknown commands are
not forwarded there.

Change

src/bot/orchestrator.py line ~321:

# Before
filters.TEXT & ~filters.COMMAND

# After  
filters.TEXT  # unknown slash commands pass through to Claude in agentic mode

Related Issue

Closes #129

Type of Change

• Bug fix / feature (restoring expected agentic behaviour)

Testing

• 491 passed, 0 failed
• No regressions

[Copilot]

Pull request overview

This PR removes the ~filters.COMMAND exclusion from the agentic mode text MessageHandler filter, intending to allow unknown slash commands (e.g., /workflow, /skill) to pass through to Claude as natural language text.

Changes:

• Removed ~filters.COMMAND from the filters.TEXT filter in the agentic MessageHandler registration in _register_agentic_handlers.

---

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

[Copilot]

Bug: Removing ~filters.COMMAND will cause registered slash commands (e.g. /start, /new, /status, /verbose, /repo, /restart) to be processed twice — once by the CommandHandler in the default group (0) and again by agentic_text in group 10.

In python-telegram-bot, handler groups are independent: a match in group 0 does not prevent handlers in group 10 from also firing. Only ApplicationHandlerStop can stop cross-group propagation, and it is not raised anywhere in this codebase.

This means typing /start will both run agentic_start and send "/start" to Claude as a text message.

To fix this while still passing through unknown slash commands, you should keep ~filters.COMMAND but add a fallback MessageHandler with filters.COMMAND in the same group (10) that only fires for unrecognised commands. Alternatively, you could move the CommandHandlers into group 10 as well (so they share the same group as the text handler and block it), or raise ApplicationHandlerStop in each command handler to prevent further group processing.

[FridayOpenClawBot]

PR Review
Reviewed head: f74608038780d1e6b674469d821feb58603b9642

Summary

• 1-line change: removes ~filters.COMMAND from the agentic MessageHandler so slash commands reach Claude in agentic mode
• Addresses issue feat: Allow unknown slash commands to passthrough to Claude Code in agentic mode #129
• PR feat: forward unknown /commands to Claude as natural language #120 (by @inhyoe) implements the same feature — not a true duplicate though; see Issues below

What looks good

• The motivation is correct: unknown slash commands (e.g. /workflow, /skill) should reach Claude in agentic mode
• Classic mode is untouched
• Group=10 priority ordering is the right instinct
• Tests pass

Issues / questions

1. [Blocker] src/bot/orchestrator.py — filters.TEXT in PTB v20+ does match command messages. message.text is non-empty for /start, /new, etc., so filters.TEXT returns True for them. PTB only stops cross-group propagation if a handler raises ApplicationHandlerStop. Since the existing CommandHandlers don't do that, removing ~filters.COMMAND means every registered command (/start, /new, /status, /verbose, /repo) will be handled twice: once by its CommandHandler (group ≤ 0) and again by agentic_text (group 10). This is a functional regression, not just dead code.

2. [Blocker] src/bot/orchestrator.py — the correct fix is to add a separate MessageHandler(filters.COMMAND, ...) at group=10 — but PTB doesn't stop propagation automatically across groups either. The real solution requires either (a) raising ApplicationHandlerStop in each registered CommandHandler, or (b) adding a catch-all COMMAND handler in group=10 that explicitly checks whether the command is in the known-commands list before forwarding to Claude.

3. [Important] Duplicate risk — PR feat: forward unknown /commands to Claude as natural language #120 ("forward unknown /commands to Claude as natural language", branch fix/frozen-message-text-assignment) targets the same issue. It takes a different approach: adds a dedicated filters.COMMAND handler at group=10 and uses an override_text parameter instead of mutating update.message.text (which is frozen in PTB v20+). This PR feat: passthrough unknown slash commands to Claude in agentic mode #131 doesn't address the frozen-text issue — if agentic_text reads update.message.text directly, it should still work for forwarding, but worth verifying. Recommend reconciling with PR feat: forward unknown /commands to Claude as natural language #120 before merging either.

4. [Nit] The inline comment # unknown slash commands pass through to Claude in agentic mode is misleading once you know filters.TEXT also passes known commands through — update once the blocker is fixed.

Suggested tests

• Test that sending /start in agentic mode does NOT result in agentic_text being called (no double-dispatch)
• Test that /unknown_command arg1 arg2 reaches agentic_text with the full text intact
• Test that classic mode is unaffected

Verdict
⚠️ Merge after fixes — the intent is right and the feature is useful, but the current implementation causes double-dispatch of all registered commands. Also recommend reconciling with PR #120 to avoid duplicate work.

— Friday, AI assistant to @RichardAtCT

[FridayOpenClawBot]

PR Review
Reviewed head: db9a0765f781e24894586a74aa94686afa57c8cc

Summary

• Fixes the double-fire regression from the previous commit (f746080) by restoring filters.TEXT & ~filters.COMMAND for the text handler and adding a separate MessageHandler(filters.COMMAND, ...) in group 10 to catch unregistered slash commands
• _handle_unknown_command does a Python-level guard against KNOWN_COMMANDS and delegates to agentic_text for anything not recognised
• Closes feat: Allow unknown slash commands to passthrough to Claude Code in agentic mode #129; overlaps significantly with PR feat: forward unknown /commands to Claude as natural language #120

What looks good

• The two-handler approach is clean and correct — group 0 CommandHandlers still take priority, group 10 catches the rest
• Command parsing (split()[0].lstrip("/").split("@")[0].lower()) correctly handles bot-suffixed commands like /help@mybot
• Comment block clearly explains the group priority logic, which will help future readers
• Handler count test updated to match the new shape

Issues / questions

1. [Important] src/bot/orchestrator.py — KNOWN_COMMANDS: frozenset = frozenset(...) will fail mypy strict. Should be KNOWN_COMMANDS: ClassVar[frozenset[str]] = frozenset(...) (with from typing import ClassVar). The CLAUDE.md notes this class variable convention explicitly, and disallow_untyped_defs = true means CI will flag it.

2. [Important] src/bot/orchestrator.py — KNOWN_COMMANDS is a hardcoded list that will silently drift whenever a new CommandHandler is registered. If someone adds /debug or /config later, it'll double-fire with no warning. Consider deriving it dynamically from the registered handlers at startup (e.g. collect command strings from app.handlers[0]) or at least add a comment directing maintainers to keep it in sync.

3. [Important] Conflicts with PR feat: forward unknown /commands to Claude as natural language #120 ("feat: forward unknown /commands to Claude as natural language") which solves the same problem. One of them needs to be closed. If this PR is the chosen approach, feat: forward unknown /commands to Claude as natural language #120 should be explicitly closed/superseded before merge to avoid confusion.

4. [Nit] tests/unit/test_orchestrator.py — The only new test assertion is a handler count check (== 5). There's no test verifying that a known command is not forwarded to Claude, or that an unknown command is forwarded. A routing test with a mocked agentic_text would give real confidence in the guard logic and prevent future regressions.

Suggested tests (if needed)

• test_known_command_not_forwarded_to_claude: mock agentic_text, send /help, assert it was NOT called
• test_unknown_command_forwarded_to_claude: mock agentic_text, send /workflow, assert it WAS called once
• test_bot_suffixed_command_parsed_correctly: send /help@mybot, verify it resolves to "help" and is blocked

Verdict
⚠️ Merge after fixes — the routing logic is sound, but the mypy type annotation and the KNOWN_COMMANDS drift risk are worth addressing before landing. Also close or supersede #120 explicitly.

— Friday, AI assistant to @RichardAtCT

[FridayOpenClawBot]

PR Review
Reviewed head: 22d16ee92d4ee85e39add26a83658f85a4359260

(Incremental review from db9a0765 — previous head)

Summary

• Addresses all three [Important] items raised in the last review
• Replaces the hardcoded KNOWN_COMMANDS class variable with a dynamically derived self._known_commands instance attribute, built from the registered handler list at startup
• Adds ClassVar to the typing import (fixing the mypy annotation)
• Adds three routing tests covering the known/unknown/bot-suffixed cases

What looks good

• Dynamic derivation (frozenset(cmd for cmd, _ in handlers)) is the right call — no more drift when commands are added
• ClassVar import and annotation are correct per CLAUDE.md's mypy strict config
• The three new tests (test_known_command_not_forwarded_to_claude, test_unknown_command_forwarded_to_claude, test_bot_suffixed_command_not_forwarded) are exactly what I requested and test the right paths
• Initialisation inside _register_agentic_handlers means _known_commands is always available before any handler fires

Issues / questions

1. [Important] PR feat: forward unknown /commands to Claude as natural language #120 (fix/frozen-message-text-assignment) is still open and targets the same feature. It should be explicitly closed or superseded before this merges — otherwise two competing implementations will coexist in the PR list indefinitely. If this PR is the chosen approach, a close-with-comment on feat: forward unknown /commands to Claude as natural language #120 pointing here would be the clean move.

2. [Nit] src/bot/orchestrator.py — self._known_commands is set inside _register_agentic_handlers, which means if _handle_unknown_command is somehow invoked before that method runs (e.g. in tests that don't call register_handlers), it'll raise AttributeError. A safe default (self._known_commands: frozenset[str] = frozenset()) in __init__ would guard against this. Low risk given normal usage, but worth noting.

Verdict
✅ Ready to merge — all previous blockers and important items are resolved. Close #120 alongside this and it's clean.

— Friday, AI assistant to @RichardAtCT

[FridayOpenClawBot]

PR Review
Reviewed head: 60d794ecb22babd16503dc3e4b67f45d87919b22

(Incremental review from 22d16ee9 — previous head)

Summary

• Single defensive fix: initialises _known_commands to frozenset() in __init__ so that _handle_unknown_command never raises AttributeError if somehow called before _register_agentic_handlers runs.

What looks good

• Correct fix: the empty-frozenset default is safe — worst case it means every command is treated as "unknown" until registration completes, which is practically unreachable at runtime.
• Type annotation on the __init__ assignment is consistent with the one in _register_agentic_handlers.

Issues / questions

1. [Nit] src/bot/orchestrator.py — The type annotation frozenset[str] is repeated on both the __init__ assignment and the _register_agentic_handlers assignment. The second one is redundant (mypy already knows the type from the first). Not a problem, but slightly noisy.

2. [Important] PR feat: forward unknown /commands to Claude as natural language #120 (fix/frozen-message-text-assignment) is also open and titled "feat: forward unknown /commands to Claude as natural language" — it appears to address the same problem from a different angle. Worth confirming one supersedes the other before merging, to avoid a merge conflict or partial revert.

Verdict
⚠️ Merge after fixes — the incremental change itself is fine; just confirm the conflict with PR #120 is resolved first.

— Friday, AI assistant to @RichardAtCT