From 44c14670ffa72e416c0ca214838e2079195d749b Mon Sep 17 00:00:00 2001 From: F1orian <27896106+F1orian@users.noreply.github.com> Date: Thu, 26 Feb 2026 17:11:51 +0100 Subject: [PATCH 01/16] feat: add /stop command to cancel running Claude calls Allows users to immediately cancel an in-progress Claude call. The command cancels the asyncio task wrapping run_command(), cleans up the progress message, and logs the cancellation. Registered in both agentic and classic modes. Co-Authored-By: Claude Opus 4.6 --- src/bot/handlers/command.py | 21 +++++++++++++++++++++ src/bot/handlers/message.py | 28 +++++++++++++++++++++------- src/bot/orchestrator.py | 32 +++++++++++++++++++++++++------- tests/unit/test_orchestrator.py | 24 +++++++++++++----------- 4 files changed, 80 insertions(+), 25 deletions(-) diff --git a/src/bot/handlers/command.py b/src/bot/handlers/command.py index 65c1405a..867e582f 100644 --- a/src/bot/handlers/command.py +++ b/src/bot/handlers/command.py @@ -1221,6 +1221,27 @@ async def git_command(update: Update, context: ContextTypes.DEFAULT_TYPE) -> Non logger.error("Error in git_command", error=str(e), user_id=user_id) +async def stop_command(update: Update, context: ContextTypes.DEFAULT_TYPE) -> None: + """Handle /stop command - cancel a running Claude call immediately.""" + audit_logger: AuditLogger = context.bot_data.get("audit_logger") + user_id = update.effective_user.id + + task = context.user_data.get("_claude_task") + if task and not task.done(): + task.cancel() + await update.message.reply_text( + "⏹ Stopping Claude…", + parse_mode="HTML", + ) + logger.info("Claude call cancelled via /stop", user_id=user_id) + if audit_logger: + await audit_logger.log_command(user_id, "stop", [], True) + else: + await update.message.reply_text("Nothing running.") + if audit_logger: + await audit_logger.log_command(user_id, "stop", [], False) + + def _format_file_size(size: int) -> str: """Format file size in human-readable format.""" for unit in ["B", "KB", "MB", "GB"]: diff --git a/src/bot/handlers/message.py b/src/bot/handlers/message.py index 538ce17f..abf58352 100644 --- a/src/bot/handlers/message.py +++ b/src/bot/handlers/message.py @@ -386,14 +386,28 @@ async def stream_handler(update_obj): # Run Claude command try: - claude_response = await claude_integration.run_command( - prompt=message_text, - working_directory=current_dir, - user_id=user_id, - session_id=session_id, - on_stream=stream_handler, - force_new=force_new, + task = asyncio.create_task( + claude_integration.run_command( + prompt=message_text, + working_directory=current_dir, + user_id=user_id, + session_id=session_id, + on_stream=stream_handler, + force_new=force_new, + ) ) + context.user_data["_claude_task"] = task + try: + claude_response = await task + except asyncio.CancelledError: + logger.info("Claude call cancelled by user", user_id=user_id) + try: + await progress_msg.delete() + except Exception: + pass + return + finally: + context.user_data.pop("_claude_task", None) # New session created successfully — clear the one-shot flag if force_new: diff --git a/src/bot/orchestrator.py b/src/bot/orchestrator.py index faacabb8..f381adc3 100644 --- a/src/bot/orchestrator.py +++ b/src/bot/orchestrator.py @@ -306,6 +306,7 @@ def _register_agentic_handlers(self, app: Application) -> None: ("status", self.agentic_status), ("verbose", self.agentic_verbose), ("repo", self.agentic_repo), + ("stop", command.stop_command), ] if self.settings.enable_project_threads: handlers.append(("sync_threads", command.sync_threads)) @@ -364,6 +365,7 @@ def _register_classic_handlers(self, app: Application) -> None: ("export", command.export_session), ("actions", command.quick_actions), ("git", command.git_command), + ("stop", command.stop_command), ] if self.settings.enable_project_threads: handlers.append(("sync_threads", command.sync_threads)) @@ -403,6 +405,7 @@ async def get_bot_commands(self) -> list: # type: ignore[type-arg] BotCommand("status", "Show session status"), BotCommand("verbose", "Set output verbosity (0/1/2)"), BotCommand("repo", "List repos / switch workspace"), + BotCommand("stop", "Stop running Claude call"), ] if self.settings.enable_project_threads: commands.append(BotCommand("sync_threads", "Sync project topics")) @@ -422,6 +425,7 @@ async def get_bot_commands(self) -> list: # type: ignore[type-arg] BotCommand("export", "Export current session"), BotCommand("actions", "Show quick actions"), BotCommand("git", "Git repository commands"), + BotCommand("stop", "Stop running Claude call"), ] if self.settings.enable_project_threads: commands.append(BotCommand("sync_threads", "Sync project topics")) @@ -880,14 +884,28 @@ async def agentic_text( success = True try: - claude_response = await claude_integration.run_command( - prompt=message_text, - working_directory=current_dir, - user_id=user_id, - session_id=session_id, - on_stream=on_stream, - force_new=force_new, + task = asyncio.create_task( + claude_integration.run_command( + prompt=message_text, + working_directory=current_dir, + user_id=user_id, + session_id=session_id, + on_stream=on_stream, + force_new=force_new, + ) ) + context.user_data["_claude_task"] = task + try: + claude_response = await task + except asyncio.CancelledError: + logger.info("Claude call cancelled by user", user_id=user_id) + try: + await progress_msg.delete() + except Exception: + pass + return + finally: + context.user_data.pop("_claude_task", None) # New session created successfully — clear the one-shot flag if force_new: diff --git a/tests/unit/test_orchestrator.py b/tests/unit/test_orchestrator.py index f565708b..0b94194e 100644 --- a/tests/unit/test_orchestrator.py +++ b/tests/unit/test_orchestrator.py @@ -82,8 +82,8 @@ def deps(): } -def test_agentic_registers_5_commands(agentic_settings, deps): - """Agentic mode registers start, new, status, verbose, repo commands.""" +def test_agentic_registers_6_commands(agentic_settings, deps): + """Agentic mode registers start, new, status, verbose, repo, stop commands.""" orchestrator = MessageOrchestrator(agentic_settings, deps) app = MagicMock() app.add_handler = MagicMock() @@ -100,16 +100,17 @@ def test_agentic_registers_5_commands(agentic_settings, deps): ] commands = [h[0][0].commands for h in cmd_handlers] - assert len(cmd_handlers) == 5 + assert len(cmd_handlers) == 6 assert frozenset({"start"}) in commands assert frozenset({"new"}) in commands assert frozenset({"status"}) in commands assert frozenset({"verbose"}) in commands assert frozenset({"repo"}) in commands + assert frozenset({"stop"}) in commands -def test_classic_registers_13_commands(classic_settings, deps): - """Classic mode registers all 13 commands.""" +def test_classic_registers_14_commands(classic_settings, deps): + """Classic mode registers all 14 commands.""" orchestrator = MessageOrchestrator(classic_settings, deps) app = MagicMock() app.add_handler = MagicMock() @@ -124,7 +125,7 @@ def test_classic_registers_13_commands(classic_settings, deps): if isinstance(call[0][0], CommandHandler) ] - assert len(cmd_handlers) == 13 + assert len(cmd_handlers) == 14 def test_agentic_registers_text_document_photo_handlers(agentic_settings, deps): @@ -155,25 +156,26 @@ def test_agentic_registers_text_document_photo_handlers(agentic_settings, deps): async def test_agentic_bot_commands(agentic_settings, deps): - """Agentic mode returns 5 bot commands.""" + """Agentic mode returns 6 bot commands.""" orchestrator = MessageOrchestrator(agentic_settings, deps) commands = await orchestrator.get_bot_commands() - assert len(commands) == 5 + assert len(commands) == 6 cmd_names = [c.command for c in commands] - assert cmd_names == ["start", "new", "status", "verbose", "repo"] + assert cmd_names == ["start", "new", "status", "verbose", "repo", "stop"] async def test_classic_bot_commands(classic_settings, deps): - """Classic mode returns 13 bot commands.""" + """Classic mode returns 14 bot commands.""" orchestrator = MessageOrchestrator(classic_settings, deps) commands = await orchestrator.get_bot_commands() - assert len(commands) == 13 + assert len(commands) == 14 cmd_names = [c.command for c in commands] assert "start" in cmd_names assert "help" in cmd_names assert "git" in cmd_names + assert "stop" in cmd_names async def test_agentic_start_no_keyboard(agentic_settings, deps): From 4e5fe8d05850fa41abc1c6d07a9f0088f014a8f6 Mon Sep 17 00:00:00 2001 From: F1orian <27896106+F1orian@users.noreply.github.com> Date: Thu, 26 Feb 2026 17:25:17 +0100 Subject: [PATCH 02/16] fix: actually terminate Claude CLI subprocess on /stop task.cancel() only cancels the asyncio wrapper but leaves the Claude CLI subprocess running. Add abort() to ClaudeSDKManager and ClaudeIntegration facade that calls client.disconnect(), which terminates the underlying subprocess via SIGTERM. The /stop handler now calls abort() before task.cancel() to ensure the CLI process is killed immediately. Co-Authored-By: Claude Opus 4.6 --- src/bot/handlers/command.py | 8 +++++++- src/claude/facade.py | 4 ++++ src/claude/sdk_integration.py | 18 ++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/bot/handlers/command.py b/src/bot/handlers/command.py index 867e582f..1b0902db 100644 --- a/src/bot/handlers/command.py +++ b/src/bot/handlers/command.py @@ -1228,9 +1228,15 @@ async def stop_command(update: Update, context: ContextTypes.DEFAULT_TYPE) -> No task = context.user_data.get("_claude_task") if task and not task.done(): + # First, abort the SDK client to terminate the CLI subprocess. + # task.cancel() alone only cancels the asyncio wrapper and may + # leave the subprocess running in the background. + claude_integration = context.bot_data.get("claude_integration") + if claude_integration: + await claude_integration.abort() task.cancel() await update.message.reply_text( - "⏹ Stopping Claude…", + "⏹ Stopped.", parse_mode="HTML", ) logger.info("Claude call cancelled via /stop", user_id=user_id) diff --git a/src/claude/facade.py b/src/claude/facade.py index fcb2ada6..ca3c59ce 100644 --- a/src/claude/facade.py +++ b/src/claude/facade.py @@ -29,6 +29,10 @@ def __init__( self.sdk_manager = sdk_manager or ClaudeSDKManager(config) self.session_manager = session_manager + async def abort(self) -> None: + """Abort the currently running Claude command.""" + await self.sdk_manager.abort() + async def run_command( self, prompt: str, diff --git a/src/claude/sdk_integration.py b/src/claude/sdk_integration.py index 6a380c71..bb25d356 100644 --- a/src/claude/sdk_integration.py +++ b/src/claude/sdk_integration.py @@ -136,6 +136,7 @@ def __init__( """Initialize SDK manager with configuration.""" self.config = config self.security_validator = security_validator + self._active_client: Optional["ClaudeSDKClient"] = None # Set up environment for Claude Code SDK if API key is provided # If no API key is provided, the SDK will use existing CLI authentication @@ -145,6 +146,21 @@ def __init__( else: logger.info("No API key provided, using existing Claude CLI authentication") + async def abort(self) -> None: + """Abort the currently running Claude command by disconnecting the client. + + This terminates the underlying CLI subprocess, ensuring the call + actually stops rather than continuing in the background. + """ + client = self._active_client + if client is not None: + self._active_client = None + try: + await client.disconnect() + logger.info("Active Claude client disconnected via abort") + except Exception as e: + logger.warning("Error during abort disconnect", error=str(e)) + async def execute_command( self, prompt: str, @@ -235,6 +251,7 @@ async def _run_client() -> None: # a plain string. connect(None) uses an empty async # iterable internally, satisfying the requirement. client = ClaudeSDKClient(options) + self._active_client = client try: await client.connect() await client.query(prompt) @@ -274,6 +291,7 @@ async def _run_client() -> None: error_type=type(callback_error).__name__, ) finally: + self._active_client = None await client.disconnect() # Execute with timeout From a5c8be41440b9b6d0228a796f4085e3fea0fc79f Mon Sep 17 00:00:00 2001 From: F1orian <27896106+F1orian@users.noreply.github.com> Date: Thu, 26 Feb 2026 22:19:35 +0100 Subject: [PATCH 03/16] fix: make /stop work reliably with concurrent calls and subagents The original /stop implementation could never actually stop a running Claude call because PTB's concurrent_updates defaults to False, meaning the /stop handler was queued behind the still-running message handler. This commit fixes three issues: 1. Enable concurrent update processing (concurrent_updates=True) so /stop can run while a Claude call is in progress. 2. Track active calls per-thread instead of a single _claude_task on context.user_data. Each project thread gets its own entry in an _active_calls dict, so /stop in one channel doesn't affect another. 3. Replace the single _active_pid with a per-call PID dict and use process-tree killing (via /proc/*/children) instead of killpg to safely terminate the CLI and all its subagents without killing the bot's own process group. Co-Authored-By: Claude Opus 4.6 --- src/bot/core.py | 4 ++ src/bot/handlers/command.py | 13 ++++- src/bot/handlers/message.py | 9 ++- src/bot/orchestrator.py | 14 ++++- src/claude/facade.py | 15 ++++- src/claude/sdk_integration.py | 106 +++++++++++++++++++++++++++++----- 6 files changed, 136 insertions(+), 25 deletions(-) diff --git a/src/bot/core.py b/src/bot/core.py index 19bd6e45..89b87f51 100644 --- a/src/bot/core.py +++ b/src/bot/core.py @@ -54,6 +54,10 @@ async def initialize(self) -> None: builder.defaults(Defaults(do_quote=self.settings.reply_quote)) builder.rate_limiter(AIORateLimiter(max_retries=1)) + # Allow concurrent update processing so that commands like /stop + # can run while a long-running Claude call is in progress. + builder.concurrent_updates(True) + # Configure connection settings builder.connect_timeout(30) builder.read_timeout(30) diff --git a/src/bot/handlers/command.py b/src/bot/handlers/command.py index 1b0902db..0f48e6aa 100644 --- a/src/bot/handlers/command.py +++ b/src/bot/handlers/command.py @@ -1226,14 +1226,21 @@ async def stop_command(update: Update, context: ContextTypes.DEFAULT_TYPE) -> No audit_logger: AuditLogger = context.bot_data.get("audit_logger") user_id = update.effective_user.id - task = context.user_data.get("_claude_task") + # Look up the active call for this thread (or _default for non-threaded chats) + tc = context.user_data.get("_thread_context") + thread_key = tc["state_key"] if tc else "_default" + active = context.user_data.get("_active_calls", {}) + call_info = active.get(thread_key, {}) + task = call_info.get("task") + call_id = call_info.get("call_id") + if task and not task.done(): # First, abort the SDK client to terminate the CLI subprocess. # task.cancel() alone only cancels the asyncio wrapper and may # leave the subprocess running in the background. claude_integration = context.bot_data.get("claude_integration") - if claude_integration: - await claude_integration.abort() + if claude_integration and call_id is not None: + claude_integration.abort_call(call_id) task.cancel() await update.message.reply_text( "⏹ Stopped.", diff --git a/src/bot/handlers/message.py b/src/bot/handlers/message.py index abf58352..4cd2e83c 100644 --- a/src/bot/handlers/message.py +++ b/src/bot/handlers/message.py @@ -386,6 +386,7 @@ async def stream_handler(update_obj): # Run Claude command try: + call_id = id(object()) # unique per call task = asyncio.create_task( claude_integration.run_command( prompt=message_text, @@ -394,9 +395,13 @@ async def stream_handler(update_obj): session_id=session_id, on_stream=stream_handler, force_new=force_new, + call_id=call_id, ) ) - context.user_data["_claude_task"] = task + tc = context.user_data.get("_thread_context") + thread_key = tc["state_key"] if tc else "_default" + active = context.user_data.setdefault("_active_calls", {}) + active[thread_key] = {"task": task, "call_id": call_id} try: claude_response = await task except asyncio.CancelledError: @@ -407,7 +412,7 @@ async def stream_handler(update_obj): pass return finally: - context.user_data.pop("_claude_task", None) + active.pop(thread_key, None) # New session created successfully — clear the one-shot flag if force_new: diff --git a/src/bot/orchestrator.py b/src/bot/orchestrator.py index f381adc3..3e01150e 100644 --- a/src/bot/orchestrator.py +++ b/src/bot/orchestrator.py @@ -244,6 +244,12 @@ def _persist_thread_state(self, context: ContextTypes.DEFAULT_TYPE) -> None: "project_slug": thread_context["project_slug"], } + @staticmethod + def _thread_key(context: ContextTypes.DEFAULT_TYPE) -> str: + """Return a key identifying the current thread (or '_default').""" + tc = context.user_data.get("_thread_context") + return tc["state_key"] if tc else "_default" + @staticmethod def _is_within(path: Path, root: Path) -> bool: """Return True if path is within root.""" @@ -884,6 +890,7 @@ async def agentic_text( success = True try: + call_id = id(object()) # unique per call task = asyncio.create_task( claude_integration.run_command( prompt=message_text, @@ -892,9 +899,12 @@ async def agentic_text( session_id=session_id, on_stream=on_stream, force_new=force_new, + call_id=call_id, ) ) - context.user_data["_claude_task"] = task + thread_key = self._thread_key(context) + active = context.user_data.setdefault("_active_calls", {}) + active[thread_key] = {"task": task, "call_id": call_id} try: claude_response = await task except asyncio.CancelledError: @@ -905,7 +915,7 @@ async def agentic_text( pass return finally: - context.user_data.pop("_claude_task", None) + active.pop(thread_key, None) # New session created successfully — clear the one-shot flag if force_new: diff --git a/src/claude/facade.py b/src/claude/facade.py index ca3c59ce..65ebbdaa 100644 --- a/src/claude/facade.py +++ b/src/claude/facade.py @@ -29,9 +29,13 @@ def __init__( self.sdk_manager = sdk_manager or ClaudeSDKManager(config) self.session_manager = session_manager - async def abort(self) -> None: - """Abort the currently running Claude command.""" - await self.sdk_manager.abort() + def abort(self) -> None: + """Abort all running Claude commands (used during shutdown).""" + self.sdk_manager.abort() + + def abort_call(self, call_id: int) -> None: + """Abort a specific Claude call by its call_id.""" + self.sdk_manager.abort_call(call_id) async def run_command( self, @@ -41,6 +45,7 @@ async def run_command( session_id: Optional[str] = None, on_stream: Optional[Callable[[StreamUpdate], None]] = None, force_new: bool = False, + call_id: Optional[int] = None, ) -> ClaudeResponse: """Run Claude Code command with full integration.""" logger.info( @@ -89,6 +94,7 @@ async def run_command( session_id=claude_session_id, continue_session=should_continue, stream_callback=on_stream, + call_id=call_id, ) except Exception as resume_error: # If resume failed (e.g., session expired/missing on Claude's side), @@ -113,6 +119,7 @@ async def run_command( session_id=None, continue_session=False, stream_callback=on_stream, + call_id=call_id, ) else: raise @@ -156,6 +163,7 @@ async def _execute( session_id: Optional[str] = None, continue_session: bool = False, stream_callback: Optional[Callable] = None, + call_id: Optional[int] = None, ) -> ClaudeResponse: """Execute command via SDK.""" return await self.sdk_manager.execute_command( @@ -164,6 +172,7 @@ async def _execute( session_id=session_id, continue_session=continue_session, stream_callback=stream_callback, + call_id=call_id, ) async def _find_resumable_session( diff --git a/src/claude/sdk_integration.py b/src/claude/sdk_integration.py index bb25d356..f06b2d38 100644 --- a/src/claude/sdk_integration.py +++ b/src/claude/sdk_integration.py @@ -2,6 +2,7 @@ import asyncio import os +import signal from dataclasses import dataclass, field from pathlib import Path from typing import Any, Callable, Dict, List, Optional @@ -136,7 +137,7 @@ def __init__( """Initialize SDK manager with configuration.""" self.config = config self.security_validator = security_validator - self._active_client: Optional["ClaudeSDKClient"] = None + self._active_pids: Dict[int, int] = {} # call_id -> subprocess PID # Set up environment for Claude Code SDK if API key is provided # If no API key is provided, the SDK will use existing CLI authentication @@ -146,20 +147,88 @@ def __init__( else: logger.info("No API key provided, using existing Claude CLI authentication") - async def abort(self) -> None: - """Abort the currently running Claude command by disconnecting the client. - - This terminates the underlying CLI subprocess, ensuring the call - actually stops rather than continuing in the background. + @staticmethod + def _get_descendants(pid: int) -> List[int]: + """Return all descendant PIDs of a process (breadth-first).""" + descendants: List[int] = [] + queue = [pid] + while queue: + parent = queue.pop(0) + try: + with open(f"/proc/{parent}/task/{parent}/children") as f: + children = [int(p) for p in f.read().split() if p] + except (FileNotFoundError, ProcessLookupError, ValueError): + children = [] + descendants.extend(children) + queue.extend(children) + return descendants + + def _kill_pid(self, pid: int) -> None: + """Kill a process tree: the target PID and all its descendants. + + Uses killpg when the process is its own group leader. Otherwise + kills each process in the tree individually to avoid killing the + bot's own process group. """ - client = self._active_client - if client is not None: - self._active_client = None + try: try: - await client.disconnect() - logger.info("Active Claude client disconnected via abort") - except Exception as e: - logger.warning("Error during abort disconnect", error=str(e)) + pgid = os.getpgid(pid) + if pgid == pid: + # Own group leader — killpg is safe and covers all children + os.killpg(pgid, signal.SIGTERM) + logger.info("Sent SIGTERM to Claude CLI process group", pid=pid) + else: + # Shares bot's group — kill the tree individually + descendants = self._get_descendants(pid) + os.kill(pid, signal.SIGTERM) + for child in descendants: + try: + os.kill(child, signal.SIGTERM) + except ProcessLookupError: + pass + logger.info( + "Sent SIGTERM to Claude CLI process tree", + pid=pid, + descendants=len(descendants), + ) + except (ProcessLookupError, PermissionError): + os.kill(pid, signal.SIGTERM) + logger.info("Sent SIGTERM to Claude CLI process", pid=pid) + + import threading + + def _ensure_dead(target_pid: int) -> None: + import time + time.sleep(2) + # Re-collect survivors (some children may have spawned late) + all_pids = [target_pid] + self._get_descendants(target_pid) + for p in all_pids: + try: + os.kill(p, 0) + os.kill(p, signal.SIGKILL) + logger.warning("Sent SIGKILL to surviving process", pid=p) + except ProcessLookupError: + pass + + threading.Thread(target=_ensure_dead, args=(pid,), daemon=True).start() + + except ProcessLookupError: + logger.debug("Claude CLI process already exited", pid=pid) + except Exception as e: + logger.warning("Failed to kill Claude CLI process", pid=pid, error=str(e)) + + def abort(self) -> None: + """Abort all running Claude commands (used during shutdown).""" + pids = list(self._active_pids.values()) + self._active_pids.clear() + for pid in pids: + self._kill_pid(pid) + + def abort_call(self, call_id: int) -> None: + """Abort a specific Claude call by its call_id.""" + pid = self._active_pids.pop(call_id, None) + if pid is not None: + self._kill_pid(pid) async def execute_command( self, @@ -168,6 +237,7 @@ async def execute_command( session_id: Optional[str] = None, continue_session: bool = False, stream_callback: Optional[Callable[[StreamUpdate], None]] = None, + call_id: Optional[int] = None, ) -> ClaudeResponse: """Execute Claude Code command via SDK.""" start_time = asyncio.get_event_loop().time() @@ -251,9 +321,14 @@ async def _run_client() -> None: # a plain string. connect(None) uses an empty async # iterable internally, satisfying the requirement. client = ClaudeSDKClient(options) - self._active_client = client try: await client.connect() + # Store the subprocess PID so abort() can kill it + # from any async context via os.kill(). + transport = getattr(client, "_transport", None) + proc = getattr(transport, "_process", None) + if proc is not None and call_id is not None: + self._active_pids[call_id] = proc.pid await client.query(prompt) # Iterate over raw messages and parse them ourselves @@ -291,7 +366,8 @@ async def _run_client() -> None: error_type=type(callback_error).__name__, ) finally: - self._active_client = None + if call_id is not None: + self._active_pids.pop(call_id, None) await client.disconnect() # Execute with timeout From 1c8e6e78ba2aa95cb931432e0e0e18dc78d714ee Mon Sep 17 00:00:00 2001 From: F1orian <27896106+F1orian@users.noreply.github.com> Date: Thu, 26 Feb 2026 22:23:58 +0100 Subject: [PATCH 04/16] style: fix black formatting in sdk_integration.py Co-Authored-By: Claude Opus 4.6 --- src/claude/sdk_integration.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/claude/sdk_integration.py b/src/claude/sdk_integration.py index f06b2d38..ba10c771 100644 --- a/src/claude/sdk_integration.py +++ b/src/claude/sdk_integration.py @@ -199,6 +199,7 @@ def _kill_pid(self, pid: int) -> None: def _ensure_dead(target_pid: int) -> None: import time + time.sleep(2) # Re-collect survivors (some children may have spawned late) all_pids = [target_pid] + self._get_descendants(target_pid) From 27c9b73d0e67ce66f2fbe72d7507b9d53f66b468 Mon Sep 17 00:00:00 2001 From: F1orian <27896106+F1orian@users.noreply.github.com> Date: Sat, 28 Feb 2026 00:29:34 +0100 Subject: [PATCH 05/16] =?UTF-8?q?fix:=20address=20PR=20review=20=E2=80=94?= =?UTF-8?q?=20race=20guard,=20Linux=20guard,=20robust=20call=5Fid,=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Document concurrent_updates safety: all user_data reads in stop_command happen synchronously before the first await - Add platform guard in _get_descendants(): return [] on non-Linux so _kill_pid gracefully falls back to killing only the root process - Replace id(object()) with itertools.count() monotonic counter for call_id (avoids CPython address reuse) - Add unit tests: /stop with nothing running, thread isolation Co-Authored-By: Claude Opus 4.6 --- src/bot/handlers/command.py | 4 ++- src/bot/handlers/message.py | 5 +++- src/bot/orchestrator.py | 5 +++- src/claude/sdk_integration.py | 10 ++++++- tests/unit/test_orchestrator.py | 47 +++++++++++++++++++++++++++++++++ 5 files changed, 67 insertions(+), 4 deletions(-) diff --git a/src/bot/handlers/command.py b/src/bot/handlers/command.py index 0f48e6aa..16f43e97 100644 --- a/src/bot/handlers/command.py +++ b/src/bot/handlers/command.py @@ -1226,7 +1226,9 @@ async def stop_command(update: Update, context: ContextTypes.DEFAULT_TYPE) -> No audit_logger: AuditLogger = context.bot_data.get("audit_logger") user_id = update.effective_user.id - # Look up the active call for this thread (or _default for non-threaded chats) + # Snapshot thread_key and call_info synchronously before the first + # ``await`` so that concurrent handlers cannot mutate ``user_data`` + # between our read and the cancel (concurrent_updates is enabled). tc = context.user_data.get("_thread_context") thread_key = tc["state_key"] if tc else "_default" active = context.user_data.get("_active_calls", {}) diff --git a/src/bot/handlers/message.py b/src/bot/handlers/message.py index 4cd2e83c..f833f578 100644 --- a/src/bot/handlers/message.py +++ b/src/bot/handlers/message.py @@ -1,6 +1,7 @@ """Message handlers for non-command inputs.""" import asyncio +import itertools from typing import Optional import structlog @@ -28,6 +29,8 @@ logger = structlog.get_logger() +_call_counter = itertools.count(1) + async def _format_progress_update(update_obj) -> Optional[str]: """Format progress updates with enhanced context and visual indicators.""" @@ -386,7 +389,7 @@ async def stream_handler(update_obj): # Run Claude command try: - call_id = id(object()) # unique per call + call_id = next(_call_counter) task = asyncio.create_task( claude_integration.run_command( prompt=message_text, diff --git a/src/bot/orchestrator.py b/src/bot/orchestrator.py index 3e01150e..98ed4d0f 100644 --- a/src/bot/orchestrator.py +++ b/src/bot/orchestrator.py @@ -6,6 +6,7 @@ """ import asyncio +import itertools import re import time from pathlib import Path @@ -111,6 +112,8 @@ def _tool_icon(name: str) -> str: class MessageOrchestrator: """Routes messages based on mode. Single entry point for all Telegram updates.""" + _call_counter = itertools.count(1) + def __init__(self, settings: Settings, deps: Dict[str, Any]): self.settings = settings self.deps = deps @@ -890,7 +893,7 @@ async def agentic_text( success = True try: - call_id = id(object()) # unique per call + call_id = next(self._call_counter) task = asyncio.create_task( claude_integration.run_command( prompt=message_text, diff --git a/src/claude/sdk_integration.py b/src/claude/sdk_integration.py index ba10c771..903dccd3 100644 --- a/src/claude/sdk_integration.py +++ b/src/claude/sdk_integration.py @@ -2,6 +2,7 @@ import asyncio import os +import platform import signal from dataclasses import dataclass, field from pathlib import Path @@ -149,7 +150,14 @@ def __init__( @staticmethod def _get_descendants(pid: int) -> List[int]: - """Return all descendant PIDs of a process (breadth-first).""" + """Return all descendant PIDs of a process (breadth-first). + + Uses ``/proc//task//children`` which is Linux-only. + On non-Linux platforms this gracefully returns an empty list, + so ``_kill_pid`` falls back to killing only the root process. + """ + if platform.system() != "Linux": + return [] descendants: List[int] = [] queue = [pid] while queue: diff --git a/tests/unit/test_orchestrator.py b/tests/unit/test_orchestrator.py index 0b94194e..563eb220 100644 --- a/tests/unit/test_orchestrator.py +++ b/tests/unit/test_orchestrator.py @@ -178,6 +178,53 @@ async def test_classic_bot_commands(classic_settings, deps): assert "stop" in cmd_names +async def test_stop_command_nothing_running(): + """/stop with no active call replies 'Nothing running.' without error.""" + from src.bot.handlers.command import stop_command + + update = MagicMock() + update.effective_user.id = 123 + update.message.reply_text = AsyncMock() + + context = MagicMock() + context.user_data = {} + context.bot_data = {"audit_logger": None} + + await stop_command(update, context) + + update.message.reply_text.assert_called_once_with("Nothing running.") + + +async def test_stop_command_thread_isolation(): + """/stop in thread A does not cancel the task registered for thread B.""" + from src.bot.handlers.command import stop_command + + task_a = MagicMock() + task_a.done.return_value = False + task_b = MagicMock() + task_b.done.return_value = False + + update = MagicMock() + update.effective_user.id = 123 + update.message.reply_text = AsyncMock() + + context = MagicMock() + # Simulate being in thread A + context.user_data = { + "_thread_context": {"state_key": "thread_a"}, + "_active_calls": { + "thread_a": {"task": task_a, "call_id": 1}, + "thread_b": {"task": task_b, "call_id": 2}, + }, + } + context.bot_data = {"audit_logger": None, "claude_integration": None} + + await stop_command(update, context) + + task_a.cancel.assert_called_once() + task_b.cancel.assert_not_called() + + async def test_agentic_start_no_keyboard(agentic_settings, deps): """Agentic /start sends brief message without inline keyboard.""" orchestrator = MessageOrchestrator(agentic_settings, deps) From 5a84be5829de24d906af7dd1cbca649571c42094 Mon Sep 17 00:00:00 2001 From: F1orian <27896106+F1orian@users.noreply.github.com> Date: Sat, 28 Feb 2026 01:02:24 +0100 Subject: [PATCH 06/16] feat: add dynamic command palette scanner for plugins and skills Introduce CommandPaletteScanner that reads ~/.claude/ to discover installed plugins, their skills/commands, and custom user skills. Includes data models (ActionType, PaletteItem, PluginInfo), a YAML frontmatter parser, plugin toggle support, and 43 comprehensive tests. Co-Authored-By: Claude Opus 4.6 --- src/bot/features/command_palette.py | 335 ++++++++++ tests/unit/test_bot/test_command_palette.py | 664 ++++++++++++++++++++ 2 files changed, 999 insertions(+) create mode 100644 src/bot/features/command_palette.py create mode 100644 tests/unit/test_bot/test_command_palette.py diff --git a/src/bot/features/command_palette.py b/src/bot/features/command_palette.py new file mode 100644 index 00000000..6c521468 --- /dev/null +++ b/src/bot/features/command_palette.py @@ -0,0 +1,335 @@ +"""Dynamic command palette: scans ~/.claude/ for skills, commands, and plugins.""" + +from __future__ import annotations + +import json +import re +from dataclasses import dataclass, field +from enum import Enum +from pathlib import Path +from typing import Any, Dict, List, Optional, Tuple + +import structlog + +logger = structlog.get_logger() + + +class ActionType(Enum): + """How a palette item is executed.""" + + DIRECT_COMMAND = "direct" + INJECT_SKILL = "inject" + + +@dataclass +class PaletteItem: + """A single actionable item in the command palette.""" + + id: str + name: str + description: str + action_type: ActionType + action_value: str + source: str + enabled: bool = True + + +@dataclass +class PluginInfo: + """Metadata about an installed plugin.""" + + name: str + qualified_name: str + version: str + enabled: bool + items: List[PaletteItem] = field(default_factory=list) + install_path: str = "" + + +def parse_skill_frontmatter(content: str) -> Dict[str, Any]: + """Parse YAML frontmatter from a SKILL.md or command .md file. + + Uses simple key: value parsing to avoid adding a PyYAML dependency. + Handles standard single-line ``key: value`` pairs. Lines starting + with ``#`` and blank lines inside the frontmatter block are skipped. + + Args: + content: Full text content of the markdown file. + + Returns: + Dictionary of parsed frontmatter key/value pairs, or ``{}`` if + no valid frontmatter block is found. + """ + if not content.strip(): + return {} + + match = re.match(r"^---\s*\n(.*?)\n---", content, re.DOTALL) + if not match: + return {} + + try: + result: Dict[str, Any] = {} + for line in match.group(1).strip().splitlines(): + line = line.strip() + if not line or line.startswith("#"): + continue + if ":" in line: + key, _, value = line.partition(":") + result[key.strip()] = value.strip() + return result + except Exception: + logger.warning("Failed to parse SKILL.md frontmatter") + return {} + + +BOT_COMMANDS: List[PaletteItem] = [ + PaletteItem( + id="bot:new", + name="new", + description="Start a fresh session", + action_type=ActionType.DIRECT_COMMAND, + action_value="/new", + source="bot", + ), + PaletteItem( + id="bot:status", + name="status", + description="Show session status", + action_type=ActionType.DIRECT_COMMAND, + action_value="/status", + source="bot", + ), + PaletteItem( + id="bot:repo", + name="repo", + description="List repos / switch workspace", + action_type=ActionType.DIRECT_COMMAND, + action_value="/repo", + source="bot", + ), + PaletteItem( + id="bot:verbose", + name="verbose", + description="Set output verbosity (0/1/2)", + action_type=ActionType.DIRECT_COMMAND, + action_value="/verbose", + source="bot", + ), + PaletteItem( + id="bot:stop", + name="stop", + description="Stop running Claude call", + action_type=ActionType.DIRECT_COMMAND, + action_value="/stop", + source="bot", + ), +] + + +class CommandPaletteScanner: + """Scans ~/.claude/ for all available skills, commands, and plugins.""" + + def __init__(self, claude_dir: Optional[Path] = None) -> None: + self.claude_dir = claude_dir or Path.home() / ".claude" + + # ------------------------------------------------------------------ + # Public API + # ------------------------------------------------------------------ + + def scan(self) -> Tuple[List[PaletteItem], List[PluginInfo]]: + """Discover all palette items and plugin info from the filesystem. + + Returns: + A tuple of (all palette items, plugin info list). + """ + items: List[PaletteItem] = [] + plugins: List[PluginInfo] = [] + + # Always include built-in bot commands + items.extend(BOT_COMMANDS) + + if not self.claude_dir.is_dir(): + return items, plugins + + enabled_plugins = self._load_enabled_plugins() + installed_plugins = self._load_installed_plugins() + blocklisted = self._load_blocklist() + + # --- Installed plugins --- + for qualified_name, installs in installed_plugins.items(): + if not installs: + continue + install = installs[0] + install_path = Path(install.get("installPath", "")) + version = install.get("version", "unknown") + short_name = qualified_name.split("@")[0] + + is_enabled = enabled_plugins.get(qualified_name, False) + is_blocked = any( + b.get("plugin") == qualified_name for b in blocklisted + ) + effective_enabled = is_enabled and not is_blocked + + plugin_items: List[PaletteItem] = [] + + # Plugin skills + skills_dir = install_path / "skills" + if skills_dir.is_dir(): + for skill_dir in sorted(skills_dir.iterdir()): + skill_file = skill_dir / "SKILL.md" + if skill_file.is_file(): + item = self._parse_skill_file( + skill_file, short_name, effective_enabled + ) + if item: + plugin_items.append(item) + + # Plugin commands + commands_dir = install_path / "commands" + if commands_dir.is_dir(): + for cmd_file in sorted(commands_dir.glob("*.md")): + item = self._parse_command_file( + cmd_file, short_name, effective_enabled + ) + if item: + plugin_items.append(item) + + plugin = PluginInfo( + name=short_name, + qualified_name=qualified_name, + version=version, + enabled=effective_enabled, + items=plugin_items, + install_path=str(install_path), + ) + plugins.append(plugin) + items.extend(plugin_items) + + # --- Custom user skills --- + custom_dir = self.claude_dir / "skills" + if custom_dir.is_dir(): + for skill_dir in sorted(custom_dir.iterdir()): + if not skill_dir.is_dir(): + continue + skill_file = skill_dir / "SKILL.md" + if skill_file.is_file(): + item = self._parse_skill_file(skill_file, "custom", True) + if item: + item.source = "custom" + items.append(item) + + return items, plugins + + def toggle_plugin(self, qualified_name: str, enabled: bool) -> bool: + """Enable or disable a plugin by updating settings.json. + + Args: + qualified_name: The fully qualified plugin name (e.g. ``name@marketplace``). + enabled: Whether the plugin should be enabled. + + Returns: + ``True`` on success, ``False`` on failure. + """ + settings_file = self.claude_dir / "settings.json" + try: + if settings_file.is_file(): + data = json.loads(settings_file.read_text(encoding="utf-8")) + else: + data = {} + if "enabledPlugins" not in data: + data["enabledPlugins"] = {} + data["enabledPlugins"][qualified_name] = enabled + settings_file.write_text( + json.dumps(data, indent=2) + "\n", encoding="utf-8" + ) + return True + except (json.JSONDecodeError, OSError) as e: + logger.error( + "Failed to toggle plugin", + plugin=qualified_name, + error=str(e), + ) + return False + + # ------------------------------------------------------------------ + # Internal helpers + # ------------------------------------------------------------------ + + def _parse_skill_file( + self, path: Path, source: str, enabled: bool + ) -> Optional[PaletteItem]: + """Parse a SKILL.md file and return a PaletteItem, or ``None``.""" + try: + content = path.read_text(encoding="utf-8") + except OSError: + return None + fm = parse_skill_frontmatter(content) + if not fm.get("name"): + return None + name = fm["name"] + return PaletteItem( + id=f"{source}:{name}", + name=name, + description=fm.get("description", ""), + action_type=ActionType.INJECT_SKILL, + action_value=f"/{name}", + source=source, + enabled=enabled, + ) + + def _parse_command_file( + self, path: Path, source: str, enabled: bool + ) -> Optional[PaletteItem]: + """Parse a command .md file and return a PaletteItem, or ``None``.""" + try: + content = path.read_text(encoding="utf-8") + except OSError: + return None + fm = parse_skill_frontmatter(content) + if not fm.get("name"): + return None + name = fm["name"] + return PaletteItem( + id=f"{source}:{name}", + name=name, + description=fm.get("description", ""), + action_type=ActionType.INJECT_SKILL, + action_value=f"/{name}", + source=source, + enabled=enabled, + ) + + def _load_enabled_plugins(self) -> Dict[str, bool]: + """Load the ``enabledPlugins`` map from settings.json.""" + settings_file = self.claude_dir / "settings.json" + if not settings_file.is_file(): + return {} + try: + data = json.loads(settings_file.read_text(encoding="utf-8")) + return data.get("enabledPlugins", {}) + except (json.JSONDecodeError, OSError): + return {} + + def _load_installed_plugins(self) -> Dict[str, list]: + """Load the installed plugins map from installed_plugins.json.""" + installed_file = ( + self.claude_dir / "plugins" / "installed_plugins.json" + ) + if not installed_file.is_file(): + return {} + try: + data = json.loads(installed_file.read_text(encoding="utf-8")) + return data.get("plugins", {}) + except (json.JSONDecodeError, OSError): + return {} + + def _load_blocklist(self) -> list: + """Load the blocklist from blocklist.json.""" + blocklist_file = self.claude_dir / "plugins" / "blocklist.json" + if not blocklist_file.is_file(): + return [] + try: + data = json.loads(blocklist_file.read_text(encoding="utf-8")) + return data.get("plugins", []) + except (json.JSONDecodeError, OSError): + return [] diff --git a/tests/unit/test_bot/test_command_palette.py b/tests/unit/test_bot/test_command_palette.py new file mode 100644 index 00000000..394d5da0 --- /dev/null +++ b/tests/unit/test_bot/test_command_palette.py @@ -0,0 +1,664 @@ +"""Tests for the dynamic command palette scanner.""" + +from __future__ import annotations + +import json +import tempfile +from pathlib import Path + +import pytest + +from src.bot.features.command_palette import ( + ActionType, + BOT_COMMANDS, + CommandPaletteScanner, + PaletteItem, + PluginInfo, + parse_skill_frontmatter, +) + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture +def mock_claude_dir(tmp_path: Path) -> Path: + """Create a realistic ~/.claude/ directory tree for testing. + + Layout:: + + tmp/ + ├── settings.json + ├── plugins/ + │ ├── installed_plugins.json + │ ├── blocklist.json + │ └── cache/claude-plugins-official/ + │ └── my-plugin/1.0.0/ + │ ├── skills/brainstorming/SKILL.md + │ └── commands/review.md + └── skills/ + └── my-custom-skill/SKILL.md + """ + # settings.json + settings = { + "enabledPlugins": { + "my-plugin@marketplace": True, + "disabled-plugin@marketplace": False, + } + } + (tmp_path / "settings.json").write_text( + json.dumps(settings, indent=2), encoding="utf-8" + ) + + # plugins directory + plugins_dir = tmp_path / "plugins" + plugins_dir.mkdir() + + # installed_plugins.json + plugin_install_path = ( + plugins_dir + / "cache" + / "claude-plugins-official" + / "my-plugin" + / "1.0.0" + ) + plugin_install_path.mkdir(parents=True) + + disabled_plugin_path = ( + plugins_dir + / "cache" + / "claude-plugins-official" + / "disabled-plugin" + / "2.0.0" + ) + disabled_plugin_path.mkdir(parents=True) + + installed = { + "plugins": { + "my-plugin@marketplace": [ + { + "installPath": str(plugin_install_path), + "version": "1.0.0", + } + ], + "disabled-plugin@marketplace": [ + { + "installPath": str(disabled_plugin_path), + "version": "2.0.0", + } + ], + } + } + (plugins_dir / "installed_plugins.json").write_text( + json.dumps(installed, indent=2), encoding="utf-8" + ) + + # blocklist.json — empty + (plugins_dir / "blocklist.json").write_text( + json.dumps({"plugins": []}, indent=2), encoding="utf-8" + ) + + # Plugin skill: my-plugin/skills/brainstorming/SKILL.md + skill_dir = plugin_install_path / "skills" / "brainstorming" + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text( + "---\nname: brainstorming\ndescription: Use before creative work\n---\n" + "# Brainstorming\nContent here.\n", + encoding="utf-8", + ) + + # Plugin command: my-plugin/commands/review.md + cmd_dir = plugin_install_path / "commands" + cmd_dir.mkdir(parents=True) + (cmd_dir / "review.md").write_text( + "---\nname: review\ndescription: Review code changes\n---\n" + "# Review\nRun code review.\n", + encoding="utf-8", + ) + + # Disabled plugin skill + disabled_skill_dir = disabled_plugin_path / "skills" / "autofill" + disabled_skill_dir.mkdir(parents=True) + (disabled_skill_dir / "SKILL.md").write_text( + "---\nname: autofill\ndescription: Auto-fill forms\n---\n" + "# Autofill\nFills forms.\n", + encoding="utf-8", + ) + + # Custom user skill + custom_skill_dir = tmp_path / "skills" / "my-custom-skill" + custom_skill_dir.mkdir(parents=True) + (custom_skill_dir / "SKILL.md").write_text( + "---\nname: summarize\ndescription: Summarize any text\n---\n" + "# Summarize\nSummarizes content.\n", + encoding="utf-8", + ) + + return tmp_path + + +# --------------------------------------------------------------------------- +# Data model tests +# --------------------------------------------------------------------------- + + +class TestActionType: + """Test ActionType enum values.""" + + def test_direct_command_value(self) -> None: + assert ActionType.DIRECT_COMMAND.value == "direct" + + def test_inject_skill_value(self) -> None: + assert ActionType.INJECT_SKILL.value == "inject" + + def test_enum_members(self) -> None: + members = list(ActionType) + assert len(members) == 2 + assert ActionType.DIRECT_COMMAND in members + assert ActionType.INJECT_SKILL in members + + +class TestPaletteItem: + """Test PaletteItem dataclass.""" + + def test_creation_with_defaults(self) -> None: + item = PaletteItem( + id="test:item", + name="item", + description="A test item", + action_type=ActionType.DIRECT_COMMAND, + action_value="/item", + source="test", + ) + assert item.id == "test:item" + assert item.name == "item" + assert item.description == "A test item" + assert item.action_type == ActionType.DIRECT_COMMAND + assert item.action_value == "/item" + assert item.source == "test" + assert item.enabled is True # default + + def test_creation_disabled(self) -> None: + item = PaletteItem( + id="x:y", + name="y", + description="", + action_type=ActionType.INJECT_SKILL, + action_value="/y", + source="plugin", + enabled=False, + ) + assert item.enabled is False + + def test_equality(self) -> None: + a = PaletteItem( + id="a:b", + name="b", + description="desc", + action_type=ActionType.DIRECT_COMMAND, + action_value="/b", + source="bot", + ) + b = PaletteItem( + id="a:b", + name="b", + description="desc", + action_type=ActionType.DIRECT_COMMAND, + action_value="/b", + source="bot", + ) + assert a == b + + +class TestPluginInfo: + """Test PluginInfo dataclass.""" + + def test_creation_with_defaults(self) -> None: + plugin = PluginInfo( + name="test", + qualified_name="test@marketplace", + version="1.0.0", + enabled=True, + ) + assert plugin.name == "test" + assert plugin.qualified_name == "test@marketplace" + assert plugin.version == "1.0.0" + assert plugin.enabled is True + assert plugin.items == [] + assert plugin.install_path == "" + + def test_creation_with_items(self) -> None: + item = PaletteItem( + id="p:s", + name="s", + description="", + action_type=ActionType.INJECT_SKILL, + action_value="/s", + source="p", + ) + plugin = PluginInfo( + name="p", + qualified_name="p@marketplace", + version="2.0.0", + enabled=False, + items=[item], + install_path="/some/path", + ) + assert len(plugin.items) == 1 + assert plugin.install_path == "/some/path" + + def test_items_default_is_not_shared(self) -> None: + """Verify that each instance gets its own list.""" + a = PluginInfo( + name="a", + qualified_name="a@m", + version="1", + enabled=True, + ) + b = PluginInfo( + name="b", + qualified_name="b@m", + version="1", + enabled=True, + ) + a.items.append( + PaletteItem( + id="x:y", + name="y", + description="", + action_type=ActionType.INJECT_SKILL, + action_value="/y", + source="x", + ) + ) + assert len(b.items) == 0 + + +# --------------------------------------------------------------------------- +# Frontmatter parser tests +# --------------------------------------------------------------------------- + + +class TestParseSkillFrontmatter: + """Test the YAML frontmatter parser.""" + + def test_valid_frontmatter(self) -> None: + content = ( + "---\n" + "name: brainstorming\n" + "description: Use before creative work\n" + "---\n" + "# Content\n" + ) + result = parse_skill_frontmatter(content) + assert result["name"] == "brainstorming" + assert result["description"] == "Use before creative work" + + def test_frontmatter_with_allowed_tools(self) -> None: + content = ( + "---\n" + "name: deploy\n" + "description: Deploy application\n" + "allowed-tools: Bash, Read, Write\n" + "---\n" + "# Deploy\n" + ) + result = parse_skill_frontmatter(content) + assert result["name"] == "deploy" + assert result["allowed-tools"] == "Bash, Read, Write" + + def test_no_frontmatter(self) -> None: + content = "# Just a heading\nSome text.\n" + result = parse_skill_frontmatter(content) + assert result == {} + + def test_empty_content(self) -> None: + result = parse_skill_frontmatter("") + assert result == {} + + def test_whitespace_only(self) -> None: + result = parse_skill_frontmatter(" \n \n ") + assert result == {} + + def test_frontmatter_with_comments(self) -> None: + content = ( + "---\n" + "# This is a comment\n" + "name: test\n" + "---\n" + ) + result = parse_skill_frontmatter(content) + assert result == {"name": "test"} + + def test_frontmatter_with_blank_lines(self) -> None: + content = ( + "---\n" + "name: test\n" + "\n" + "description: desc\n" + "---\n" + ) + result = parse_skill_frontmatter(content) + assert result["name"] == "test" + assert result["description"] == "desc" + + def test_value_with_colon(self) -> None: + """Values containing colons should preserve everything after first.""" + content = ( + "---\n" + "name: my-skill\n" + "description: Step 1: do this, Step 2: do that\n" + "---\n" + ) + result = parse_skill_frontmatter(content) + assert result["description"] == "Step 1: do this, Step 2: do that" + + def test_unclosed_frontmatter(self) -> None: + content = "---\nname: test\nSome body text\n" + result = parse_skill_frontmatter(content) + assert result == {} + + def test_frontmatter_not_at_start(self) -> None: + content = "Some text\n---\nname: test\n---\n" + result = parse_skill_frontmatter(content) + assert result == {} + + +# --------------------------------------------------------------------------- +# Scanner tests +# --------------------------------------------------------------------------- + + +class TestCommandPaletteScanner: + """Test the filesystem scanner.""" + + def test_bot_commands_always_present(self, mock_claude_dir: Path) -> None: + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + items, _ = scanner.scan() + bot_items = [i for i in items if i.source == "bot"] + assert len(bot_items) == len(BOT_COMMANDS) + + def test_bot_commands_are_direct(self, mock_claude_dir: Path) -> None: + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + items, _ = scanner.scan() + for item in items: + if item.source == "bot": + assert item.action_type == ActionType.DIRECT_COMMAND + + def test_discovers_plugin_skills(self, mock_claude_dir: Path) -> None: + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + items, _ = scanner.scan() + skill_names = [i.name for i in items if i.source == "my-plugin"] + assert "brainstorming" in skill_names + + def test_discovers_plugin_commands(self, mock_claude_dir: Path) -> None: + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + items, _ = scanner.scan() + cmd_names = [i.name for i in items if i.source == "my-plugin"] + assert "review" in cmd_names + + def test_plugin_skills_are_inject_type( + self, mock_claude_dir: Path + ) -> None: + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + items, _ = scanner.scan() + for item in items: + if item.source == "my-plugin": + assert item.action_type == ActionType.INJECT_SKILL + + def test_enabled_plugin_items_are_enabled( + self, mock_claude_dir: Path + ) -> None: + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + items, _ = scanner.scan() + enabled_items = [ + i for i in items if i.source == "my-plugin" + ] + for item in enabled_items: + assert item.enabled is True + + def test_disabled_plugin_items_are_disabled( + self, mock_claude_dir: Path + ) -> None: + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + items, _ = scanner.scan() + disabled_items = [ + i for i in items if i.source == "disabled-plugin" + ] + for item in disabled_items: + assert item.enabled is False + + def test_discovers_custom_skills(self, mock_claude_dir: Path) -> None: + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + items, _ = scanner.scan() + custom_items = [i for i in items if i.source == "custom"] + assert len(custom_items) == 1 + assert custom_items[0].name == "summarize" + + def test_custom_skills_always_enabled( + self, mock_claude_dir: Path + ) -> None: + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + items, _ = scanner.scan() + custom_items = [i for i in items if i.source == "custom"] + for item in custom_items: + assert item.enabled is True + + def test_builds_plugin_info(self, mock_claude_dir: Path) -> None: + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + _, plugins = scanner.scan() + assert len(plugins) == 2 + + by_name = {p.qualified_name: p for p in plugins} + + my_plugin = by_name["my-plugin@marketplace"] + assert my_plugin.name == "my-plugin" + assert my_plugin.version == "1.0.0" + assert my_plugin.enabled is True + assert len(my_plugin.items) == 2 # brainstorming + review + + disabled = by_name["disabled-plugin@marketplace"] + assert disabled.name == "disabled-plugin" + assert disabled.version == "2.0.0" + assert disabled.enabled is False + assert len(disabled.items) == 1 # autofill + + def test_missing_claude_dir(self, tmp_path: Path) -> None: + """Scanner should return only bot commands if dir is missing.""" + missing = tmp_path / "nonexistent" + scanner = CommandPaletteScanner(claude_dir=missing) + items, plugins = scanner.scan() + assert len(items) == len(BOT_COMMANDS) + assert plugins == [] + + def test_empty_claude_dir(self, tmp_path: Path) -> None: + """Scanner with an empty directory returns only bot commands.""" + empty = tmp_path / "empty_claude" + empty.mkdir() + scanner = CommandPaletteScanner(claude_dir=empty) + items, plugins = scanner.scan() + assert len(items) == len(BOT_COMMANDS) + assert plugins == [] + + def test_blocklisted_plugin_is_disabled( + self, mock_claude_dir: Path + ) -> None: + """A plugin on the blocklist should be treated as disabled.""" + blocklist = {"plugins": [{"plugin": "my-plugin@marketplace"}]} + blocklist_file = mock_claude_dir / "plugins" / "blocklist.json" + blocklist_file.write_text( + json.dumps(blocklist, indent=2), encoding="utf-8" + ) + + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + items, plugins = scanner.scan() + + by_name = {p.qualified_name: p for p in plugins} + my_plugin = by_name["my-plugin@marketplace"] + assert my_plugin.enabled is False + + plugin_items = [i for i in items if i.source == "my-plugin"] + for item in plugin_items: + assert item.enabled is False + + def test_skill_without_name_is_skipped( + self, mock_claude_dir: Path + ) -> None: + """A SKILL.md without a 'name' field should be ignored.""" + bad_skill_dir = mock_claude_dir / "skills" / "bad-skill" + bad_skill_dir.mkdir(parents=True) + (bad_skill_dir / "SKILL.md").write_text( + "---\ndescription: No name field\n---\nContent\n", + encoding="utf-8", + ) + + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + items, _ = scanner.scan() + custom_items = [i for i in items if i.source == "custom"] + # Only the original 'summarize' custom skill should be present + assert len(custom_items) == 1 + assert custom_items[0].name == "summarize" + + def test_non_directory_in_skills_is_skipped( + self, mock_claude_dir: Path + ) -> None: + """Regular files inside skills/ should be ignored.""" + (mock_claude_dir / "skills" / "stray_file.txt").write_text( + "Not a skill", encoding="utf-8" + ) + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + items, _ = scanner.scan() + custom_items = [i for i in items if i.source == "custom"] + assert len(custom_items) == 1 + + def test_malformed_settings_json(self, tmp_path: Path) -> None: + """Scanner handles corrupt settings.json gracefully.""" + claude_dir = tmp_path / "claude" + claude_dir.mkdir() + (claude_dir / "settings.json").write_text( + "{bad json", encoding="utf-8" + ) + scanner = CommandPaletteScanner(claude_dir=claude_dir) + items, plugins = scanner.scan() + assert len(items) == len(BOT_COMMANDS) + + def test_malformed_installed_plugins_json(self, tmp_path: Path) -> None: + """Scanner handles corrupt installed_plugins.json gracefully.""" + claude_dir = tmp_path / "claude" + plugins_dir = claude_dir / "plugins" + plugins_dir.mkdir(parents=True) + (plugins_dir / "installed_plugins.json").write_text( + "not json!", encoding="utf-8" + ) + (claude_dir / "settings.json").write_text( + "{}", encoding="utf-8" + ) + scanner = CommandPaletteScanner(claude_dir=claude_dir) + items, plugins = scanner.scan() + assert len(items) == len(BOT_COMMANDS) + assert plugins == [] + + def test_empty_install_list_for_plugin(self, tmp_path: Path) -> None: + """A plugin with an empty installs array should be skipped.""" + claude_dir = tmp_path / "claude" + plugins_dir = claude_dir / "plugins" + plugins_dir.mkdir(parents=True) + (claude_dir / "settings.json").write_text( + json.dumps({"enabledPlugins": {"ghost@mp": True}}), + encoding="utf-8", + ) + (plugins_dir / "installed_plugins.json").write_text( + json.dumps({"plugins": {"ghost@mp": []}}), + encoding="utf-8", + ) + scanner = CommandPaletteScanner(claude_dir=claude_dir) + items, plugins = scanner.scan() + assert plugins == [] + + +# --------------------------------------------------------------------------- +# toggle_plugin tests +# --------------------------------------------------------------------------- + + +class TestTogglePlugin: + """Test the toggle_plugin method.""" + + def test_enable_plugin(self, mock_claude_dir: Path) -> None: + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + result = scanner.toggle_plugin("disabled-plugin@marketplace", True) + assert result is True + + settings = json.loads( + (mock_claude_dir / "settings.json").read_text(encoding="utf-8") + ) + assert settings["enabledPlugins"]["disabled-plugin@marketplace"] is True + + def test_disable_plugin(self, mock_claude_dir: Path) -> None: + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + result = scanner.toggle_plugin("my-plugin@marketplace", False) + assert result is True + + settings = json.loads( + (mock_claude_dir / "settings.json").read_text(encoding="utf-8") + ) + assert settings["enabledPlugins"]["my-plugin@marketplace"] is False + + def test_toggle_nonexistent_plugin(self, mock_claude_dir: Path) -> None: + """Toggling a plugin that does not yet exist in settings should add it.""" + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + result = scanner.toggle_plugin("new-plugin@marketplace", True) + assert result is True + + settings = json.loads( + (mock_claude_dir / "settings.json").read_text(encoding="utf-8") + ) + assert settings["enabledPlugins"]["new-plugin@marketplace"] is True + + def test_toggle_creates_settings_file(self, tmp_path: Path) -> None: + """toggle_plugin should create settings.json if it does not exist.""" + claude_dir = tmp_path / "fresh_claude" + claude_dir.mkdir() + scanner = CommandPaletteScanner(claude_dir=claude_dir) + result = scanner.toggle_plugin("p@m", True) + assert result is True + + settings = json.loads( + (claude_dir / "settings.json").read_text(encoding="utf-8") + ) + assert settings["enabledPlugins"]["p@m"] is True + + def test_toggle_preserves_existing_settings( + self, mock_claude_dir: Path + ) -> None: + """Toggling one plugin should not clobber other settings.""" + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + scanner.toggle_plugin("new@mp", True) + + settings = json.loads( + (mock_claude_dir / "settings.json").read_text(encoding="utf-8") + ) + # Original entries should still be present + assert "my-plugin@marketplace" in settings["enabledPlugins"] + assert "disabled-plugin@marketplace" in settings["enabledPlugins"] + assert settings["enabledPlugins"]["new@mp"] is True + + def test_toggle_fails_on_read_only_dir(self, tmp_path: Path) -> None: + """toggle_plugin returns False when the file can't be written.""" + claude_dir = tmp_path / "ro_claude" + claude_dir.mkdir() + settings_file = claude_dir / "settings.json" + settings_file.write_text("{}", encoding="utf-8") + # Make the directory read-only so writing fails + settings_file.chmod(0o444) + claude_dir.chmod(0o555) + + scanner = CommandPaletteScanner(claude_dir=claude_dir) + result = scanner.toggle_plugin("x@m", True) + assert result is False + + # Restore permissions for cleanup + claude_dir.chmod(0o755) + settings_file.chmod(0o644) From 7c78141913f90a7b7b2ca34bb5533ada04ba1a47 Mon Sep 17 00:00:00 2001 From: F1orian <27896106+F1orian@users.noreply.github.com> Date: Sat, 28 Feb 2026 01:10:50 +0100 Subject: [PATCH 07/16] feat: add /menu command palette with inline keyboard navigation Implement MenuBuilder (builds top-level and category sub-menus with short IDs for Telegram's 64-byte callback_data limit), menu_command and menu_callback handlers, and wire them into the agentic orchestrator. - MenuBuilder: top-level menu (Bot, plugins 2-per-row, custom skills, Plugin Store), category sub-menus with toggle and back buttons, single-item plugins execute directly, multi-item open categories - menu_command: scans filesystem via CommandPaletteScanner, sends inline keyboard, stores builder in user_data for callback resolution - menu_callback: handles back/cat/run/tog/store actions, re-scans on navigation, sets pending skill/command for future Claude injection - Orchestrator: registers /menu CommandHandler + menu: CallbackQueryHandler, adds BotCommand("menu") to Telegram command menu - 39 menu tests + 3 orchestrator integration tests (78 total, all pass) Co-Authored-By: Claude Opus 4.6 --- src/bot/handlers/menu.py | 459 +++++++++++++++++++ src/bot/orchestrator.py | 11 + tests/unit/test_bot/test_menu.py | 733 +++++++++++++++++++++++++++++++ tests/unit/test_orchestrator.py | 78 +++- 4 files changed, 1270 insertions(+), 11 deletions(-) create mode 100644 src/bot/handlers/menu.py create mode 100644 tests/unit/test_bot/test_menu.py diff --git a/src/bot/handlers/menu.py b/src/bot/handlers/menu.py new file mode 100644 index 00000000..6f823a64 --- /dev/null +++ b/src/bot/handlers/menu.py @@ -0,0 +1,459 @@ +"""Dynamic command palette menu: inline keyboard builder + handlers.""" + +from __future__ import annotations + +from typing import Dict, List, Optional, Tuple + +import structlog +from telegram import InlineKeyboardButton, InlineKeyboardMarkup, Update +from telegram.ext import ContextTypes + +from ..features.command_palette import ( + ActionType, + CommandPaletteScanner, + PaletteItem, + PluginInfo, +) + +logger = structlog.get_logger() + + +class MenuBuilder: + """Builds inline keyboards for the command palette navigation. + + Telegram limits ``callback_data`` to 64 bytes. We use short + incrementing numeric IDs and store the mapping in ``id_map``. + """ + + def __init__( + self, items: List[PaletteItem], plugins: List[PluginInfo] + ) -> None: + self.items = items + self.plugins = plugins + self.id_map: Dict[str, str] = {} # short_id -> full_id + self._counter = 0 + + # ------------------------------------------------------------------ + # Short-ID helpers + # ------------------------------------------------------------------ + + def _next_id(self) -> str: + """Return the next short numeric ID as a string.""" + sid = str(self._counter) + self._counter += 1 + return sid + + def _register(self, full_id: str) -> str: + """Map *full_id* to a short numeric ID and return the short ID.""" + sid = self._next_id() + self.id_map[sid] = full_id + return sid + + # ------------------------------------------------------------------ + # Public API + # ------------------------------------------------------------------ + + def build_top_level(self) -> InlineKeyboardMarkup: + """Build the top-level menu keyboard. + + Layout: + - **Bot (count)** -- always first row + - Each plugin with items -- sorted by name, 2 per row. + Single-item plugins trigger directly (``menu:run:{id}``), + multi-item plugins open a category (``menu:cat:{id}``). + - Custom skills -- individual buttons + - Plugin Store -- last row + """ + # Reset short-ID state for a fresh build + self.id_map.clear() + self._counter = 0 + + rows: List[List[InlineKeyboardButton]] = [] + + # --- Bot category --- + bot_items = [i for i in self.items if i.source == "bot"] + bot_sid = self._register("cat:bot") + rows.append( + [ + InlineKeyboardButton( + f"\U0001f916 Bot ({len(bot_items)})", + callback_data=f"menu:cat:{bot_sid}", + ) + ] + ) + + # --- Plugin categories (sorted by name, 2 per row) --- + sorted_plugins = sorted( + [p for p in self.plugins if p.items], + key=lambda p: p.name, + ) + plugin_buttons: List[InlineKeyboardButton] = [] + for plugin in sorted_plugins: + status = "\u2705" if plugin.enabled else "\u274c" + if len(plugin.items) == 1: + # Single-item plugin -- tap runs directly + item = plugin.items[0] + sid = self._register(item.id) + label = f"{status} {plugin.name}" + plugin_buttons.append( + InlineKeyboardButton( + label, callback_data=f"menu:run:{sid}" + ) + ) + else: + sid = self._register(f"cat:{plugin.name}") + label = f"{status} {plugin.name} ({len(plugin.items)})" + plugin_buttons.append( + InlineKeyboardButton( + label, callback_data=f"menu:cat:{sid}" + ) + ) + # Pack plugin buttons 2-per-row + for i in range(0, len(plugin_buttons), 2): + rows.append(plugin_buttons[i : i + 2]) + + # --- Custom skills (individual buttons, 2-per-row) --- + custom_items = [i for i in self.items if i.source == "custom"] + custom_buttons: List[InlineKeyboardButton] = [] + for item in custom_items: + sid = self._register(item.id) + custom_buttons.append( + InlineKeyboardButton( + f"\u2728 {item.name}", + callback_data=f"menu:run:{sid}", + ) + ) + for i in range(0, len(custom_buttons), 2): + rows.append(custom_buttons[i : i + 2]) + + # --- Plugin Store (last row) --- + rows.append( + [ + InlineKeyboardButton( + "\U0001f50c Plugin Store", callback_data="menu:store" + ) + ] + ) + + return InlineKeyboardMarkup(rows) + + def build_category(self, source: str) -> Tuple[InlineKeyboardMarkup, str]: + """Build a category sub-menu keyboard. + + Args: + source: The source identifier (``"bot"`` or a plugin name). + + Returns: + Tuple of (keyboard, header_text). + """ + if source == "bot": + cat_items = [i for i in self.items if i.source == "bot"] + header = "\U0001f916 Bot commands" + else: + cat_items = [i for i in self.items if i.source == source] + plugin = self._find_plugin(source) + status = "" + if plugin: + status = " \u2705" if plugin.enabled else " \u274c" + header = f"\U0001f50c {source}{status}" + + rows: List[List[InlineKeyboardButton]] = [] + for item in cat_items: + sid = self._register(item.id) + label = f"{item.name} — {item.description}" if item.description else item.name + rows.append( + [ + InlineKeyboardButton( + label, callback_data=f"menu:run:{sid}" + ) + ] + ) + + # Toggle button for plugins (not bot) + if source != "bot": + plugin = self._find_plugin(source) + if plugin: + action = "Disable" if plugin.enabled else "Enable" + icon = "\u274c" if plugin.enabled else "\u2705" + rows.append( + [ + InlineKeyboardButton( + f"{icon} {action} {plugin.name}", + callback_data=f"menu:tog:{plugin.qualified_name}", + ) + ] + ) + + # Back button + rows.append( + [ + InlineKeyboardButton( + "\u2b05 Back", callback_data="menu:back" + ) + ] + ) + + return InlineKeyboardMarkup(rows), header + + def get_single_item_action(self, source: str) -> Optional[PaletteItem]: + """Return the item if a plugin has exactly 1 item, else ``None``.""" + plugin = self._find_plugin(source) + if plugin and len(plugin.items) == 1: + return plugin.items[0] + return None + + def resolve_id(self, short_id: str) -> Optional[str]: + """Resolve a short callback ID to the full item/category ID.""" + return self.id_map.get(short_id) + + # ------------------------------------------------------------------ + # Internal helpers + # ------------------------------------------------------------------ + + def _find_plugin(self, name: str) -> Optional[PluginInfo]: + """Find a plugin by short name.""" + for p in self.plugins: + if p.name == name: + return p + return None + + +# ====================================================================== +# Telegram handler functions +# ====================================================================== + + +async def menu_command( + update: Update, context: ContextTypes.DEFAULT_TYPE +) -> None: + """Handle ``/menu`` -- scan filesystem, build top-level keyboard, send.""" + scanner = CommandPaletteScanner() + items, plugins = scanner.scan() + + builder = MenuBuilder(items, plugins) + keyboard = builder.build_top_level() + + # Persist builder for callback resolution + if context.user_data is not None: + context.user_data["menu_builder"] = builder + context.user_data["menu_scanner"] = scanner + + item_count = len(items) + plugin_count = len(plugins) + custom_count = len([i for i in items if i.source == "custom"]) + + text = ( + f"\U0001f3af Command Palette\n\n" + f"{item_count} commands \u00b7 {plugin_count} plugins" + f" \u00b7 {custom_count} custom skills" + ) + + await update.message.reply_text( # type: ignore[union-attr] + text, + parse_mode="HTML", + reply_markup=keyboard, + ) + + logger.info( + "Menu opened", + items=item_count, + plugins=plugin_count, + custom=custom_count, + ) + + +async def menu_callback( + update: Update, context: ContextTypes.DEFAULT_TYPE +) -> None: + """Handle all ``menu:`` callbacks. + + Actions: + - ``menu:back`` -- rebuild and show top-level + - ``menu:cat:{id}`` -- show category sub-menu + - ``menu:run:{id}`` -- execute item + - ``menu:tog:{plugin}`` -- toggle plugin enable/disable + - ``menu:store`` -- show plugin store placeholder + """ + query = update.callback_query + if query is None: + return + await query.answer() + + data = query.data or "" + builder: Optional[MenuBuilder] = None + scanner: Optional[CommandPaletteScanner] = None + if context.user_data is not None: + builder = context.user_data.get("menu_builder") + scanner = context.user_data.get("menu_scanner") + + # ------------------------------------------------------------------ + # menu:back — rebuild top-level + # ------------------------------------------------------------------ + if data == "menu:back": + if builder is None: + await query.edit_message_text("Session expired. Send /menu again.") + return + # Re-scan to reflect any toggle changes + if scanner: + items, plugins = scanner.scan() + builder = MenuBuilder(items, plugins) + if context.user_data is not None: + context.user_data["menu_builder"] = builder + + keyboard = builder.build_top_level() + item_count = len(builder.items) + plugin_count = len(builder.plugins) + custom_count = len([i for i in builder.items if i.source == "custom"]) + text = ( + f"\U0001f3af Command Palette\n\n" + f"{item_count} commands \u00b7 {plugin_count} plugins" + f" \u00b7 {custom_count} custom skills" + ) + await query.edit_message_text( + text, parse_mode="HTML", reply_markup=keyboard + ) + return + + # ------------------------------------------------------------------ + # menu:store — placeholder + # ------------------------------------------------------------------ + if data == "menu:store": + back_keyboard = InlineKeyboardMarkup( + [ + [ + InlineKeyboardButton( + "\u2b05 Back", callback_data="menu:back" + ) + ] + ] + ) + await query.edit_message_text( + "\U0001f50c Plugin Store\n\nComing soon.", + parse_mode="HTML", + reply_markup=back_keyboard, + ) + return + + # ------------------------------------------------------------------ + # menu:tog:{qualified_name} — toggle plugin + # ------------------------------------------------------------------ + if data.startswith("menu:tog:"): + qualified_name = data[len("menu:tog:") :] + if scanner is None: + await query.edit_message_text("Session expired. Send /menu again.") + return + + # Find current state + plugin_info: Optional[PluginInfo] = None + if builder: + for p in builder.plugins: + if p.qualified_name == qualified_name: + plugin_info = p + break + + new_state = not (plugin_info.enabled if plugin_info else True) + success = scanner.toggle_plugin(qualified_name, new_state) + + if not success: + await query.edit_message_text( + f"\u26a0 Failed to toggle plugin.", parse_mode="HTML" + ) + return + + # Re-scan and rebuild category + items, plugins = scanner.scan() + builder = MenuBuilder(items, plugins) + if context.user_data is not None: + context.user_data["menu_builder"] = builder + + # Find plugin name from qualified_name + source_name = qualified_name.split("@")[0] + keyboard, header = builder.build_category(source_name) + state_label = "enabled" if new_state else "disabled" + text = f"{header}\n\nPlugin {state_label}." + await query.edit_message_text( + text, parse_mode="HTML", reply_markup=keyboard + ) + logger.info( + "Plugin toggled", + plugin=qualified_name, + enabled=new_state, + ) + return + + # ------------------------------------------------------------------ + # menu:cat:{short_id} — show category sub-menu + # ------------------------------------------------------------------ + if data.startswith("menu:cat:"): + short_id = data[len("menu:cat:") :] + if builder is None: + await query.edit_message_text("Session expired. Send /menu again.") + return + + full_id = builder.resolve_id(short_id) + if full_id is None or not full_id.startswith("cat:"): + await query.edit_message_text("Invalid menu action.") + return + + source = full_id[len("cat:") :] + keyboard, header = builder.build_category(source) + await query.edit_message_text( + header, parse_mode="HTML", reply_markup=keyboard + ) + return + + # ------------------------------------------------------------------ + # menu:run:{short_id} — execute item + # ------------------------------------------------------------------ + if data.startswith("menu:run:"): + short_id = data[len("menu:run:") :] + if builder is None: + await query.edit_message_text("Session expired. Send /menu again.") + return + + full_id = builder.resolve_id(short_id) + if full_id is None: + await query.edit_message_text("Invalid menu action.") + return + + # Find the PaletteItem + item: Optional[PaletteItem] = None + for candidate in builder.items: + if candidate.id == full_id: + item = candidate + break + + if item is None: + await query.edit_message_text("Command not found.") + return + + if item.action_type == ActionType.INJECT_SKILL: + # Store pending skill for Task 8-9 integration + if context.user_data is not None: + context.user_data["menu_pending_skill"] = item.action_value + await query.edit_message_text( + f"Invoking {item.action_value}...", + parse_mode="HTML", + ) + logger.info( + "Menu skill invoked", + item_id=item.id, + action=item.action_value, + ) + elif item.action_type == ActionType.DIRECT_COMMAND: + # Store pending command for Task 8-9 integration + if context.user_data is not None: + context.user_data["menu_pending_command"] = item.action_value + await query.edit_message_text( + f"Running {item.action_value}...", + parse_mode="HTML", + ) + logger.info( + "Menu command invoked", + item_id=item.id, + action=item.action_value, + ) + return + + # Fallback for unknown actions + await query.edit_message_text("Unknown menu action.") diff --git a/src/bot/orchestrator.py b/src/bot/orchestrator.py index 98ed4d0f..4c0dec93 100644 --- a/src/bot/orchestrator.py +++ b/src/bot/orchestrator.py @@ -307,6 +307,7 @@ def register_handlers(self, app: Application) -> None: def _register_agentic_handlers(self, app: Application) -> None: """Register agentic handlers: commands + text/file/photo.""" from .handlers import command + from .handlers import menu as menu_handler # Commands handlers = [ @@ -316,6 +317,7 @@ def _register_agentic_handlers(self, app: Application) -> None: ("verbose", self.agentic_verbose), ("repo", self.agentic_repo), ("stop", command.stop_command), + ("menu", menu_handler.menu_command), ] if self.settings.enable_project_threads: handlers.append(("sync_threads", command.sync_threads)) @@ -354,6 +356,14 @@ def _register_agentic_handlers(self, app: Application) -> None: ) ) + # menu: callbacks (for command palette navigation) + app.add_handler( + CallbackQueryHandler( + self._inject_deps(menu_handler.menu_callback), + pattern=r"^menu:", + ) + ) + logger.info("Agentic handlers registered") def _register_classic_handlers(self, app: Application) -> None: @@ -415,6 +425,7 @@ async def get_bot_commands(self) -> list: # type: ignore[type-arg] BotCommand("verbose", "Set output verbosity (0/1/2)"), BotCommand("repo", "List repos / switch workspace"), BotCommand("stop", "Stop running Claude call"), + BotCommand("menu", "Command palette & plugin manager"), ] if self.settings.enable_project_threads: commands.append(BotCommand("sync_threads", "Sync project topics")) diff --git a/tests/unit/test_bot/test_menu.py b/tests/unit/test_bot/test_menu.py new file mode 100644 index 00000000..1e64d490 --- /dev/null +++ b/tests/unit/test_bot/test_menu.py @@ -0,0 +1,733 @@ +"""Tests for the dynamic command palette menu builder and handlers.""" + +from __future__ import annotations + +import json +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from telegram import InlineKeyboardMarkup + +from src.bot.features.command_palette import ( + ActionType, + BOT_COMMANDS, + CommandPaletteScanner, + PaletteItem, + PluginInfo, +) +from src.bot.handlers.menu import MenuBuilder, menu_callback, menu_command + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture +def bot_items() -> list[PaletteItem]: + """Return the standard bot commands.""" + return list(BOT_COMMANDS) + + +@pytest.fixture +def single_item_plugin() -> PluginInfo: + """A plugin with exactly one item.""" + item = PaletteItem( + id="single-plugin:deploy", + name="deploy", + description="Deploy the app", + action_type=ActionType.INJECT_SKILL, + action_value="/deploy", + source="single-plugin", + enabled=True, + ) + return PluginInfo( + name="single-plugin", + qualified_name="single-plugin@marketplace", + version="1.0.0", + enabled=True, + items=[item], + install_path="/some/path", + ) + + +@pytest.fixture +def multi_item_plugin() -> PluginInfo: + """A plugin with multiple items.""" + items = [ + PaletteItem( + id="multi-plugin:review", + name="review", + description="Review code", + action_type=ActionType.INJECT_SKILL, + action_value="/review", + source="multi-plugin", + enabled=True, + ), + PaletteItem( + id="multi-plugin:test", + name="test", + description="Run tests", + action_type=ActionType.INJECT_SKILL, + action_value="/test", + source="multi-plugin", + enabled=True, + ), + ] + return PluginInfo( + name="multi-plugin", + qualified_name="multi-plugin@marketplace", + version="2.0.0", + enabled=True, + items=items, + install_path="/other/path", + ) + + +@pytest.fixture +def disabled_plugin() -> PluginInfo: + """A disabled plugin.""" + item = PaletteItem( + id="off-plugin:autofill", + name="autofill", + description="Auto-fill forms", + action_type=ActionType.INJECT_SKILL, + action_value="/autofill", + source="off-plugin", + enabled=False, + ) + return PluginInfo( + name="off-plugin", + qualified_name="off-plugin@marketplace", + version="1.0.0", + enabled=False, + items=[item], + install_path="/disabled/path", + ) + + +@pytest.fixture +def custom_item() -> PaletteItem: + """A custom user skill.""" + return PaletteItem( + id="custom:summarize", + name="summarize", + description="Summarize text", + action_type=ActionType.INJECT_SKILL, + action_value="/summarize", + source="custom", + enabled=True, + ) + + +@pytest.fixture +def all_items( + bot_items: list[PaletteItem], + single_item_plugin: PluginInfo, + multi_item_plugin: PluginInfo, + disabled_plugin: PluginInfo, + custom_item: PaletteItem, +) -> list[PaletteItem]: + """All palette items combined.""" + items = list(bot_items) + items.extend(single_item_plugin.items) + items.extend(multi_item_plugin.items) + items.extend(disabled_plugin.items) + items.append(custom_item) + return items + + +@pytest.fixture +def all_plugins( + single_item_plugin: PluginInfo, + multi_item_plugin: PluginInfo, + disabled_plugin: PluginInfo, +) -> list[PluginInfo]: + """All plugins combined.""" + return [single_item_plugin, multi_item_plugin, disabled_plugin] + + +@pytest.fixture +def builder( + all_items: list[PaletteItem], + all_plugins: list[PluginInfo], +) -> MenuBuilder: + """A MenuBuilder with all items loaded.""" + return MenuBuilder(all_items, all_plugins) + + +# --------------------------------------------------------------------------- +# MenuBuilder.build_top_level tests +# --------------------------------------------------------------------------- + + +class TestBuildTopLevel: + """Tests for MenuBuilder.build_top_level().""" + + def test_returns_inline_keyboard(self, builder: MenuBuilder) -> None: + result = builder.build_top_level() + assert isinstance(result, InlineKeyboardMarkup) + + def test_first_row_is_bot_category(self, builder: MenuBuilder) -> None: + keyboard = builder.build_top_level() + first_row = keyboard.inline_keyboard[0] + assert len(first_row) == 1 + assert "Bot" in first_row[0].text + assert first_row[0].callback_data.startswith("menu:cat:") + + def test_bot_category_shows_count(self, builder: MenuBuilder) -> None: + keyboard = builder.build_top_level() + bot_btn = keyboard.inline_keyboard[0][0] + # Should contain count of bot items + assert f"({len(BOT_COMMANDS)})" in bot_btn.text + + def test_has_plugin_categories(self, builder: MenuBuilder) -> None: + keyboard = builder.build_top_level() + all_buttons = [ + btn + for row in keyboard.inline_keyboard + for btn in row + ] + plugin_names = ["multi-plugin", "off-plugin", "single-plugin"] + for name in plugin_names: + found = any(name in btn.text for btn in all_buttons) + assert found, f"Plugin '{name}' not found in top-level menu" + + def test_single_item_plugin_runs_directly( + self, builder: MenuBuilder + ) -> None: + keyboard = builder.build_top_level() + all_buttons = [ + btn + for row in keyboard.inline_keyboard + for btn in row + ] + single_btn = [ + btn for btn in all_buttons if "single-plugin" in btn.text + ] + assert len(single_btn) == 1 + assert single_btn[0].callback_data.startswith("menu:run:") + + def test_multi_item_plugin_opens_category( + self, builder: MenuBuilder + ) -> None: + keyboard = builder.build_top_level() + all_buttons = [ + btn + for row in keyboard.inline_keyboard + for btn in row + ] + multi_btn = [ + btn for btn in all_buttons if "multi-plugin" in btn.text + ] + assert len(multi_btn) == 1 + assert multi_btn[0].callback_data.startswith("menu:cat:") + + def test_enabled_plugin_shows_checkmark( + self, builder: MenuBuilder + ) -> None: + keyboard = builder.build_top_level() + all_buttons = [ + btn + for row in keyboard.inline_keyboard + for btn in row + ] + multi_btn = [ + btn for btn in all_buttons if "multi-plugin" in btn.text + ][0] + assert "\u2705" in multi_btn.text + + def test_disabled_plugin_shows_cross( + self, builder: MenuBuilder + ) -> None: + keyboard = builder.build_top_level() + all_buttons = [ + btn + for row in keyboard.inline_keyboard + for btn in row + ] + off_btn = [ + btn for btn in all_buttons if "off-plugin" in btn.text + ][0] + assert "\u274c" in off_btn.text + + def test_custom_skills_have_buttons( + self, builder: MenuBuilder + ) -> None: + keyboard = builder.build_top_level() + all_buttons = [ + btn + for row in keyboard.inline_keyboard + for btn in row + ] + custom_btns = [ + btn for btn in all_buttons if "summarize" in btn.text + ] + assert len(custom_btns) == 1 + assert custom_btns[0].callback_data.startswith("menu:run:") + + def test_last_row_is_plugin_store(self, builder: MenuBuilder) -> None: + keyboard = builder.build_top_level() + last_row = keyboard.inline_keyboard[-1] + assert len(last_row) == 1 + assert "Plugin Store" in last_row[0].text + assert last_row[0].callback_data == "menu:store" + + def test_plugins_sorted_by_name(self, builder: MenuBuilder) -> None: + keyboard = builder.build_top_level() + # Collect plugin button names (skip bot row at [0] and store row at [-1]) + plugin_labels: list[str] = [] + for row in keyboard.inline_keyboard[1:-1]: + for btn in row: + # Skip custom skill buttons (have sparkle emoji) + if "\u2728" not in btn.text: + plugin_labels.append(btn.text) + # Plugin names should be alphabetical + if len(plugin_labels) > 1: + names = [lbl.split(" ", 1)[-1] for lbl in plugin_labels] + assert names == sorted(names) + + +# --------------------------------------------------------------------------- +# MenuBuilder.build_category tests +# --------------------------------------------------------------------------- + + +class TestBuildCategory: + """Tests for MenuBuilder.build_category().""" + + def test_bot_category_returns_markup_and_header( + self, builder: MenuBuilder + ) -> None: + builder.build_top_level() # populate id_map + keyboard, header = builder.build_category("bot") + assert isinstance(keyboard, InlineKeyboardMarkup) + assert "Bot" in header + + def test_bot_category_has_all_bot_commands( + self, builder: MenuBuilder + ) -> None: + builder.build_top_level() + keyboard, _ = builder.build_category("bot") + # All rows except the last (Back) should be command buttons + command_rows = keyboard.inline_keyboard[:-1] + assert len(command_rows) == len(BOT_COMMANDS) + + def test_bot_category_has_back_button( + self, builder: MenuBuilder + ) -> None: + builder.build_top_level() + keyboard, _ = builder.build_category("bot") + last_row = keyboard.inline_keyboard[-1] + assert len(last_row) == 1 + assert "Back" in last_row[0].text + assert last_row[0].callback_data == "menu:back" + + def test_bot_category_has_no_toggle( + self, builder: MenuBuilder + ) -> None: + builder.build_top_level() + keyboard, _ = builder.build_category("bot") + all_data = [ + btn.callback_data + for row in keyboard.inline_keyboard + for btn in row + ] + assert not any(d.startswith("menu:tog:") for d in all_data) + + def test_plugin_category_has_items( + self, builder: MenuBuilder + ) -> None: + builder.build_top_level() + keyboard, header = builder.build_category("multi-plugin") + assert "multi-plugin" in header + # Should have 2 items + toggle + back = 4 rows + assert len(keyboard.inline_keyboard) == 4 + + def test_plugin_category_has_toggle( + self, builder: MenuBuilder + ) -> None: + builder.build_top_level() + keyboard, _ = builder.build_category("multi-plugin") + all_data = [ + btn.callback_data + for row in keyboard.inline_keyboard + for btn in row + ] + toggle_data = [d for d in all_data if d.startswith("menu:tog:")] + assert len(toggle_data) == 1 + assert "multi-plugin@marketplace" in toggle_data[0] + + def test_plugin_category_has_back( + self, builder: MenuBuilder + ) -> None: + builder.build_top_level() + keyboard, _ = builder.build_category("multi-plugin") + last_row = keyboard.inline_keyboard[-1] + assert "Back" in last_row[0].text + assert last_row[0].callback_data == "menu:back" + + def test_enabled_plugin_toggle_says_disable( + self, builder: MenuBuilder + ) -> None: + builder.build_top_level() + keyboard, _ = builder.build_category("multi-plugin") + toggle_btns = [ + btn + for row in keyboard.inline_keyboard + for btn in row + if btn.callback_data.startswith("menu:tog:") + ] + assert len(toggle_btns) == 1 + assert "Disable" in toggle_btns[0].text + + def test_disabled_plugin_toggle_says_enable( + self, builder: MenuBuilder + ) -> None: + builder.build_top_level() + keyboard, _ = builder.build_category("off-plugin") + toggle_btns = [ + btn + for row in keyboard.inline_keyboard + for btn in row + if btn.callback_data.startswith("menu:tog:") + ] + assert len(toggle_btns) == 1 + assert "Enable" in toggle_btns[0].text + + +# --------------------------------------------------------------------------- +# MenuBuilder.get_single_item_action tests +# --------------------------------------------------------------------------- + + +class TestGetSingleItemAction: + """Tests for MenuBuilder.get_single_item_action().""" + + def test_single_item_plugin_returns_item( + self, builder: MenuBuilder + ) -> None: + item = builder.get_single_item_action("single-plugin") + assert item is not None + assert item.name == "deploy" + + def test_multi_item_plugin_returns_none( + self, builder: MenuBuilder + ) -> None: + result = builder.get_single_item_action("multi-plugin") + assert result is None + + def test_nonexistent_plugin_returns_none( + self, builder: MenuBuilder + ) -> None: + result = builder.get_single_item_action("does-not-exist") + assert result is None + + +# --------------------------------------------------------------------------- +# MenuBuilder.resolve_id tests +# --------------------------------------------------------------------------- + + +class TestResolveId: + """Tests for MenuBuilder.resolve_id().""" + + def test_resolves_after_build(self, builder: MenuBuilder) -> None: + builder.build_top_level() + # The first registered ID should be "0" + result = builder.resolve_id("0") + assert result is not None + assert result == "cat:bot" + + def test_unknown_id_returns_none(self, builder: MenuBuilder) -> None: + builder.build_top_level() + result = builder.resolve_id("99999") + assert result is None + + def test_all_ids_are_unique(self, builder: MenuBuilder) -> None: + builder.build_top_level() + values = list(builder.id_map.values()) + assert len(values) == len(set(values)) + + +# --------------------------------------------------------------------------- +# Callback data size check +# --------------------------------------------------------------------------- + + +class TestCallbackDataSize: + """Telegram limits callback_data to 64 bytes.""" + + def test_top_level_callback_data_under_64_bytes( + self, builder: MenuBuilder + ) -> None: + keyboard = builder.build_top_level() + for row in keyboard.inline_keyboard: + for btn in row: + data = btn.callback_data + assert len(data.encode("utf-8")) <= 64, ( + f"callback_data too long: {data!r} " + f"({len(data.encode('utf-8'))} bytes)" + ) + + def test_category_callback_data_under_64_bytes( + self, builder: MenuBuilder + ) -> None: + builder.build_top_level() + for source in ["bot", "multi-plugin", "single-plugin", "off-plugin"]: + keyboard, _ = builder.build_category(source) + for row in keyboard.inline_keyboard: + for btn in row: + data = btn.callback_data + assert len(data.encode("utf-8")) <= 64, ( + f"callback_data too long: {data!r} " + f"({len(data.encode('utf-8'))} bytes)" + ) + + +# --------------------------------------------------------------------------- +# menu_command handler tests +# --------------------------------------------------------------------------- + + +class TestMenuCommand: + """Tests for the menu_command handler.""" + + async def test_sends_message_with_keyboard(self) -> None: + update = MagicMock() + update.message.reply_text = AsyncMock() + + context = MagicMock() + context.user_data = {} + + with patch( + "src.bot.handlers.menu.CommandPaletteScanner" + ) as MockScanner: + scanner_instance = MockScanner.return_value + scanner_instance.scan.return_value = (list(BOT_COMMANDS), []) + + await menu_command(update, context) + + update.message.reply_text.assert_called_once() + call_kwargs = update.message.reply_text.call_args + assert call_kwargs.kwargs.get("parse_mode") == "HTML" + assert isinstance( + call_kwargs.kwargs.get("reply_markup"), InlineKeyboardMarkup + ) + + async def test_stores_builder_in_user_data(self) -> None: + update = MagicMock() + update.message.reply_text = AsyncMock() + + context = MagicMock() + context.user_data = {} + + with patch( + "src.bot.handlers.menu.CommandPaletteScanner" + ) as MockScanner: + scanner_instance = MockScanner.return_value + scanner_instance.scan.return_value = (list(BOT_COMMANDS), []) + + await menu_command(update, context) + + assert "menu_builder" in context.user_data + assert isinstance(context.user_data["menu_builder"], MenuBuilder) + + async def test_message_contains_counts(self) -> None: + update = MagicMock() + update.message.reply_text = AsyncMock() + + context = MagicMock() + context.user_data = {} + + with patch( + "src.bot.handlers.menu.CommandPaletteScanner" + ) as MockScanner: + scanner_instance = MockScanner.return_value + scanner_instance.scan.return_value = (list(BOT_COMMANDS), []) + + await menu_command(update, context) + + text = update.message.reply_text.call_args.args[0] + assert "Command Palette" in text + assert str(len(BOT_COMMANDS)) in text + + +# --------------------------------------------------------------------------- +# menu_callback handler tests +# --------------------------------------------------------------------------- + + +class TestMenuCallback: + """Tests for the menu_callback handler.""" + + def _make_callback_update(self, data: str) -> MagicMock: + """Create a mock Update with callback_query.""" + update = MagicMock() + update.callback_query.data = data + update.callback_query.answer = AsyncMock() + update.callback_query.edit_message_text = AsyncMock() + return update + + async def test_back_rebuilds_top_level( + self, builder: MenuBuilder + ) -> None: + builder.build_top_level() + update = self._make_callback_update("menu:back") + + scanner = MagicMock() + scanner.scan.return_value = (builder.items, builder.plugins) + + context = MagicMock() + context.user_data = { + "menu_builder": builder, + "menu_scanner": scanner, + } + + await menu_callback(update, context) + + update.callback_query.answer.assert_called_once() + update.callback_query.edit_message_text.assert_called_once() + call_kwargs = update.callback_query.edit_message_text.call_args + assert "Command Palette" in call_kwargs.args[0] + assert isinstance( + call_kwargs.kwargs.get("reply_markup"), InlineKeyboardMarkup + ) + + async def test_cat_shows_category(self, builder: MenuBuilder) -> None: + keyboard = builder.build_top_level() + # Find the short_id for "cat:bot" (should be "0") + bot_sid = None + for sid, full_id in builder.id_map.items(): + if full_id == "cat:bot": + bot_sid = sid + break + assert bot_sid is not None + + update = self._make_callback_update(f"menu:cat:{bot_sid}") + + context = MagicMock() + context.user_data = { + "menu_builder": builder, + "menu_scanner": None, + } + + await menu_callback(update, context) + + update.callback_query.edit_message_text.assert_called_once() + call_kwargs = update.callback_query.edit_message_text.call_args + assert "Bot" in call_kwargs.args[0] + + async def test_run_skill_sets_pending( + self, builder: MenuBuilder + ) -> None: + builder.build_top_level() + # Find a run-able skill ID + skill_sid = None + for sid, full_id in builder.id_map.items(): + if full_id == "custom:summarize": + skill_sid = sid + break + assert skill_sid is not None + + update = self._make_callback_update(f"menu:run:{skill_sid}") + + context = MagicMock() + context.user_data = { + "menu_builder": builder, + "menu_scanner": None, + } + + await menu_callback(update, context) + + assert context.user_data["menu_pending_skill"] == "/summarize" + call_kwargs = update.callback_query.edit_message_text.call_args + assert "Invoking" in call_kwargs.args[0] + + async def test_run_direct_command_sets_pending( + self, builder: MenuBuilder + ) -> None: + # Build category for bot to register bot command IDs + builder.build_top_level() + _, _ = builder.build_category("bot") + # Find a bot command ID (e.g. bot:new) + cmd_sid = None + for sid, full_id in builder.id_map.items(): + if full_id == "bot:new": + cmd_sid = sid + break + assert cmd_sid is not None + + update = self._make_callback_update(f"menu:run:{cmd_sid}") + + context = MagicMock() + context.user_data = { + "menu_builder": builder, + "menu_scanner": None, + } + + await menu_callback(update, context) + + assert context.user_data["menu_pending_command"] == "/new" + call_kwargs = update.callback_query.edit_message_text.call_args + assert "Running" in call_kwargs.args[0] + + async def test_tog_toggles_plugin(self, builder: MenuBuilder) -> None: + builder.build_top_level() + + scanner = MagicMock() + scanner.toggle_plugin.return_value = True + scanner.scan.return_value = (builder.items, builder.plugins) + + update = self._make_callback_update( + "menu:tog:multi-plugin@marketplace" + ) + + context = MagicMock() + context.user_data = { + "menu_builder": builder, + "menu_scanner": scanner, + } + + await menu_callback(update, context) + + scanner.toggle_plugin.assert_called_once_with( + "multi-plugin@marketplace", False # was enabled, now disable + ) + call_kwargs = update.callback_query.edit_message_text.call_args + assert "disabled" in call_kwargs.args[0] + + async def test_store_shows_placeholder(self) -> None: + update = self._make_callback_update("menu:store") + + context = MagicMock() + context.user_data = {} + + await menu_callback(update, context) + + call_kwargs = update.callback_query.edit_message_text.call_args + assert "Plugin Store" in call_kwargs.args[0] + assert "Coming soon" in call_kwargs.args[0] + + async def test_expired_session_on_back(self) -> None: + update = self._make_callback_update("menu:back") + + context = MagicMock() + context.user_data = {} # no builder stored + + await menu_callback(update, context) + + call_kwargs = update.callback_query.edit_message_text.call_args + assert "expired" in call_kwargs.args[0].lower() + + async def test_unknown_action_handled(self) -> None: + update = self._make_callback_update("menu:unknown_action") + + context = MagicMock() + context.user_data = {} + + await menu_callback(update, context) + + call_kwargs = update.callback_query.edit_message_text.call_args + assert "Unknown" in call_kwargs.args[0] diff --git a/tests/unit/test_orchestrator.py b/tests/unit/test_orchestrator.py index 563eb220..a414506a 100644 --- a/tests/unit/test_orchestrator.py +++ b/tests/unit/test_orchestrator.py @@ -82,8 +82,8 @@ def deps(): } -def test_agentic_registers_6_commands(agentic_settings, deps): - """Agentic mode registers start, new, status, verbose, repo, stop commands.""" +def test_agentic_registers_7_commands(agentic_settings, deps): + """Agentic mode registers start, new, status, verbose, repo, stop, menu commands.""" orchestrator = MessageOrchestrator(agentic_settings, deps) app = MagicMock() app.add_handler = MagicMock() @@ -100,13 +100,14 @@ def test_agentic_registers_6_commands(agentic_settings, deps): ] commands = [h[0][0].commands for h in cmd_handlers] - assert len(cmd_handlers) == 6 + assert len(cmd_handlers) == 7 assert frozenset({"start"}) in commands assert frozenset({"new"}) in commands assert frozenset({"status"}) in commands assert frozenset({"verbose"}) in commands assert frozenset({"repo"}) in commands assert frozenset({"stop"}) in commands + assert frozenset({"menu"}) in commands def test_classic_registers_14_commands(classic_settings, deps): @@ -151,18 +152,18 @@ def test_agentic_registers_text_document_photo_handlers(agentic_settings, deps): # 3 message handlers (text, document, photo) assert len(msg_handlers) == 3 - # 1 callback handler (for cd: only) - assert len(cb_handlers) == 1 + # 2 callback handlers (cd: and menu:) + assert len(cb_handlers) == 2 async def test_agentic_bot_commands(agentic_settings, deps): - """Agentic mode returns 6 bot commands.""" + """Agentic mode returns 7 bot commands.""" orchestrator = MessageOrchestrator(agentic_settings, deps) commands = await orchestrator.get_bot_commands() - assert len(commands) == 6 + assert len(commands) == 7 cmd_names = [c.command for c in commands] - assert cmd_names == ["start", "new", "status", "verbose", "repo", "stop"] + assert cmd_names == ["start", "new", "status", "verbose", "repo", "stop", "menu"] async def test_classic_bot_commands(classic_settings, deps): @@ -344,7 +345,7 @@ async def test_agentic_text_calls_claude(agentic_settings, deps): async def test_agentic_callback_scoped_to_cd_pattern(agentic_settings, deps): - """Agentic callback handler is registered with cd: pattern filter.""" + """Agentic callback handlers are registered with cd: and menu: patterns.""" orchestrator = MessageOrchestrator(agentic_settings, deps) app = MagicMock() app.add_handler = MagicMock() @@ -359,10 +360,13 @@ async def test_agentic_callback_scoped_to_cd_pattern(agentic_settings, deps): if isinstance(call[0][0], CallbackQueryHandler) ] - assert len(cb_handlers) == 1 - # The pattern attribute should match cd: prefixed data + assert len(cb_handlers) == 2 + # First handler: cd: pattern assert cb_handlers[0].pattern is not None assert cb_handlers[0].pattern.match("cd:my_project") + # Second handler: menu: pattern + assert cb_handlers[1].pattern is not None + assert cb_handlers[1].pattern.match("menu:back") async def test_agentic_document_rejects_large_files(agentic_settings, deps): @@ -825,3 +829,55 @@ async def help_command(update, context): assert called["value"] is False update.effective_message.reply_text.assert_called_once() + + +# --- Menu integration tests --- + + +def test_agentic_registers_menu_handler(agentic_settings, deps): + """Agentic mode registers the menu command handler.""" + orchestrator = MessageOrchestrator(agentic_settings, deps) + app = MagicMock() + app.add_handler = MagicMock() + + orchestrator.register_handlers(app) + + from telegram.ext import CommandHandler + + cmd_handlers = [ + call[0][0] + for call in app.add_handler.call_args_list + if isinstance(call[0][0], CommandHandler) + ] + commands = [h.commands for h in cmd_handlers] + assert frozenset({"menu"}) in commands + + +def test_agentic_registers_menu_callback_handler(agentic_settings, deps): + """Agentic mode registers a menu: callback handler with pattern ^menu:.""" + orchestrator = MessageOrchestrator(agentic_settings, deps) + app = MagicMock() + app.add_handler = MagicMock() + + orchestrator.register_handlers(app) + + from telegram.ext import CallbackQueryHandler + + cb_handlers = [ + call[0][0] + for call in app.add_handler.call_args_list + if isinstance(call[0][0], CallbackQueryHandler) + ] + + menu_handlers = [ + h for h in cb_handlers if h.pattern and h.pattern.match("menu:back") + ] + assert len(menu_handlers) == 1 + + +async def test_get_bot_commands_includes_menu(agentic_settings, deps): + """Agentic get_bot_commands includes 'menu'.""" + orchestrator = MessageOrchestrator(agentic_settings, deps) + commands = await orchestrator.get_bot_commands() + cmd_names = [c.command for c in commands] + assert "menu" in cmd_names From 95ef91a9cc5bc5c4a167a29d5f14da5c65d65afa Mon Sep 17 00:00:00 2001 From: F1orian <27896106+F1orian@users.noreply.github.com> Date: Sat, 28 Feb 2026 01:15:58 +0100 Subject: [PATCH 08/16] feat: wire skill injection and direct commands into menu callbacks When users tap skill buttons (/commit, /review-pr, custom skills), the menu now actually calls Claude via ClaudeIntegration.run_command() instead of just storing a pending value. Direct commands (/new, /status, /stop, /verbose, /repo) are executed inline with appropriate feedback. Co-Authored-By: Claude Opus 4.6 --- src/bot/handlers/menu.py | 181 +++++++++++++++++-- tests/unit/test_bot/test_menu.py | 287 ++++++++++++++++++++++++++++++- 2 files changed, 448 insertions(+), 20 deletions(-) diff --git a/src/bot/handlers/menu.py b/src/bot/handlers/menu.py index 6f823a64..69e71b7f 100644 --- a/src/bot/handlers/menu.py +++ b/src/bot/handlers/menu.py @@ -2,6 +2,7 @@ from __future__ import annotations +from pathlib import Path from typing import Dict, List, Optional, Tuple import structlog @@ -14,6 +15,7 @@ PaletteItem, PluginInfo, ) +from ..utils.formatting import ResponseFormatter logger = structlog.get_logger() @@ -428,28 +430,179 @@ async def menu_callback( return if item.action_type == ActionType.INJECT_SKILL: - # Store pending skill for Task 8-9 integration + # Store pending skill in user_data for reference if context.user_data is not None: context.user_data["menu_pending_skill"] = item.action_value + + # Edit menu message to show progress await query.edit_message_text( - f"Invoking {item.action_value}...", + f"Working on {item.action_value}...", parse_mode="HTML", ) - logger.info( - "Menu skill invoked", - item_id=item.id, - action=item.action_value, + + # Get Claude integration + claude_integration = context.bot_data.get("claude_integration") + if not claude_integration: + await query.edit_message_text( + "Claude integration not available. Check configuration." + ) + return + + settings = context.bot_data.get("settings") + current_dir = context.user_data.get( + "current_directory", + settings.approved_directory if settings else Path.home(), ) + session_id = context.user_data.get("claude_session_id") + force_new = bool(context.user_data.get("force_new_session")) + + # Start typing indicator + chat_id = query.message.chat_id + try: + await context.bot.send_chat_action( + chat_id=chat_id, action="typing" + ) + except Exception: + pass + + try: + response = await claude_integration.run_command( + prompt=item.action_value, + working_directory=current_dir, + user_id=query.from_user.id, + session_id=session_id, + force_new=force_new, + ) + + # Clear force_new flag on success + if force_new: + context.user_data["force_new_session"] = False + + # Update session ID + context.user_data["claude_session_id"] = response.session_id + + # Format response + formatter = ResponseFormatter(settings) + formatted_messages = formatter.format_claude_response( + response.content + ) + + # Delete the "Working..." message + try: + await query.message.delete() + except Exception: + pass + + # Send formatted response + for msg in formatted_messages: + if msg.text and msg.text.strip(): + try: + await context.bot.send_message( + chat_id=chat_id, + text=msg.text, + parse_mode=msg.parse_mode, + ) + except Exception: + # Retry without parse mode + await context.bot.send_message( + chat_id=chat_id, + text=msg.text, + ) + + # Log interaction + storage = context.bot_data.get("storage") + if storage: + try: + await storage.save_claude_interaction( + user_id=query.from_user.id, + session_id=response.session_id, + prompt=item.action_value, + response=response, + ip_address=None, + ) + except Exception as e: + logger.warning( + "Failed to log menu interaction", + error=str(e), + ) + + logger.info( + "Menu skill completed", + item_id=item.id, + action=item.action_value, + session_id=response.session_id, + cost=response.cost, + ) + + except Exception as e: + logger.error( + "Menu skill execution failed", + error=str(e), + item=item.id, + ) + try: + await query.edit_message_text( + f"Failed to run {item.action_value}:" + f" {str(e)[:200]}", + parse_mode="HTML", + ) + except Exception: + pass + elif item.action_type == ActionType.DIRECT_COMMAND: - # Store pending command for Task 8-9 integration - if context.user_data is not None: - context.user_data["menu_pending_command"] = item.action_value - await query.edit_message_text( - f"Running {item.action_value}...", - parse_mode="HTML", - ) + cmd = item.action_value # e.g. "/new", "/status" + settings = context.bot_data.get("settings") + + if cmd == "/new": + context.user_data["claude_session_id"] = None + context.user_data["session_started"] = True + context.user_data["force_new_session"] = True + await query.edit_message_text("Session reset. What's next?") + + elif cmd == "/status": + current_dir = context.user_data.get( + "current_directory", + settings.approved_directory + if settings + else "unknown", + ) + session_id = context.user_data.get("claude_session_id") + session_status = "active" if session_id else "none" + await query.edit_message_text( + f"\U0001f4c2 {current_dir} \u00b7 Session: {session_status}" + ) + + elif cmd == "/stop": + active_calls = context.user_data.get("_active_calls", {}) + if active_calls: + for key, entry in list(active_calls.items()): + task = entry.get("task") + if task and not task.done(): + task.cancel() + await query.edit_message_text("Stopping active calls...") + else: + await query.edit_message_text("No active calls to stop.") + + elif cmd == "/verbose": + level = context.user_data.get("verbose_level", 1) + await query.edit_message_text( + f"Verbose level: {level}\n" + f"Use /verbose 0|1|2 to change." + ) + + elif cmd == "/repo": + await query.edit_message_text( + "Use /repo to browse and switch workspaces." + ) + + else: + await query.edit_message_text( + f"Use {cmd} directly.", + parse_mode="HTML", + ) + logger.info( - "Menu command invoked", + "Menu command executed", item_id=item.id, action=item.action_value, ) diff --git a/tests/unit/test_bot/test_menu.py b/tests/unit/test_bot/test_menu.py index 1e64d490..c4c45cfc 100644 --- a/tests/unit/test_bot/test_menu.py +++ b/tests/unit/test_bot/test_menu.py @@ -619,9 +619,10 @@ async def test_cat_shows_category(self, builder: MenuBuilder) -> None: call_kwargs = update.callback_query.edit_message_text.call_args assert "Bot" in call_kwargs.args[0] - async def test_run_skill_sets_pending( + async def test_run_inject_skill_calls_claude( self, builder: MenuBuilder ) -> None: + """When a skill button is tapped, Claude is called with the skill text.""" builder.build_top_level() # Find a run-able skill ID skill_sid = None @@ -631,6 +632,119 @@ async def test_run_skill_sets_pending( break assert skill_sid is not None + update = self._make_callback_update(f"menu:run:{skill_sid}") + # Mock message.delete and message.chat_id + update.callback_query.message.chat_id = 12345 + update.callback_query.message.delete = AsyncMock() + update.callback_query.from_user.id = 999 + + # Mock Claude response + mock_response = MagicMock() + mock_response.content = "Here is the summary." + mock_response.session_id = "session-abc" + mock_response.cost = 0.01 + + mock_claude = AsyncMock() + mock_claude.run_command = AsyncMock(return_value=mock_response) + + mock_settings = MagicMock() + mock_settings.approved_directory = Path("/tmp/test") + + # Mock formatter + mock_formatted = MagicMock() + mock_formatted.text = "Here is the summary." + mock_formatted.parse_mode = "HTML" + + context = MagicMock() + context.user_data = { + "menu_builder": builder, + "menu_scanner": None, + } + context.bot_data = { + "claude_integration": mock_claude, + "settings": mock_settings, + } + context.bot.send_chat_action = AsyncMock() + context.bot.send_message = AsyncMock() + + with patch( + "src.bot.handlers.menu.ResponseFormatter" + ) as MockFormatter: + formatter_instance = MockFormatter.return_value + formatter_instance.format_claude_response.return_value = [ + mock_formatted + ] + await menu_callback(update, context) + + # Verify Claude was called with the skill text + mock_claude.run_command.assert_called_once() + call_kwargs = mock_claude.run_command.call_args.kwargs + assert call_kwargs["prompt"] == "/summarize" + assert call_kwargs["user_id"] == 999 + + # Verify session ID was updated + assert context.user_data["claude_session_id"] == "session-abc" + + # Verify response was sent + context.bot.send_message.assert_called_once() + send_kwargs = context.bot.send_message.call_args.kwargs + assert send_kwargs["text"] == "Here is the summary." + + async def test_run_inject_skill_handles_error( + self, builder: MenuBuilder + ) -> None: + """When Claude call fails, error is shown.""" + builder.build_top_level() + skill_sid = None + for sid, full_id in builder.id_map.items(): + if full_id == "custom:summarize": + skill_sid = sid + break + assert skill_sid is not None + + update = self._make_callback_update(f"menu:run:{skill_sid}") + update.callback_query.message.chat_id = 12345 + update.callback_query.message.delete = AsyncMock() + update.callback_query.from_user.id = 999 + + mock_claude = AsyncMock() + mock_claude.run_command = AsyncMock( + side_effect=RuntimeError("Claude is down") + ) + + mock_settings = MagicMock() + mock_settings.approved_directory = Path("/tmp/test") + + context = MagicMock() + context.user_data = { + "menu_builder": builder, + "menu_scanner": None, + } + context.bot_data = { + "claude_integration": mock_claude, + "settings": mock_settings, + } + context.bot.send_chat_action = AsyncMock() + + await menu_callback(update, context) + + # Verify error message was shown + call_kwargs = update.callback_query.edit_message_text.call_args + assert "Failed to run" in call_kwargs.args[0] + assert "Claude is down" in call_kwargs.args[0] + + async def test_run_inject_skill_no_claude_integration( + self, builder: MenuBuilder + ) -> None: + """When Claude integration is missing, shows error.""" + builder.build_top_level() + skill_sid = None + for sid, full_id in builder.id_map.items(): + if full_id == "custom:summarize": + skill_sid = sid + break + assert skill_sid is not None + update = self._make_callback_update(f"menu:run:{skill_sid}") context = MagicMock() @@ -638,16 +752,17 @@ async def test_run_skill_sets_pending( "menu_builder": builder, "menu_scanner": None, } + context.bot_data = {} # No claude_integration await menu_callback(update, context) - assert context.user_data["menu_pending_skill"] == "/summarize" call_kwargs = update.callback_query.edit_message_text.call_args - assert "Invoking" in call_kwargs.args[0] + assert "not available" in call_kwargs.args[0] - async def test_run_direct_command_sets_pending( + async def test_run_direct_command_new( self, builder: MenuBuilder ) -> None: + """When /new button is tapped, session is reset.""" # Build category for bot to register bot command IDs builder.build_top_level() _, _ = builder.build_category("bot") @@ -665,13 +780,173 @@ async def test_run_direct_command_sets_pending( context.user_data = { "menu_builder": builder, "menu_scanner": None, + "claude_session_id": "old-session", } + context.bot_data = {} await menu_callback(update, context) - assert context.user_data["menu_pending_command"] == "/new" + # Session should be cleared + assert context.user_data["claude_session_id"] is None + assert context.user_data["force_new_session"] is True + assert context.user_data["session_started"] is True call_kwargs = update.callback_query.edit_message_text.call_args - assert "Running" in call_kwargs.args[0] + assert "Session reset" in call_kwargs.args[0] + + async def test_run_direct_command_status( + self, builder: MenuBuilder + ) -> None: + """When /status button is tapped, status is shown.""" + builder.build_top_level() + _, _ = builder.build_category("bot") + cmd_sid = None + for sid, full_id in builder.id_map.items(): + if full_id == "bot:status": + cmd_sid = sid + break + assert cmd_sid is not None + + update = self._make_callback_update(f"menu:run:{cmd_sid}") + + mock_settings = MagicMock() + mock_settings.approved_directory = Path("/tmp/projects") + + context = MagicMock() + context.user_data = { + "menu_builder": builder, + "menu_scanner": None, + "current_directory": Path("/home/test"), + "claude_session_id": "sess-123", + } + context.bot_data = {"settings": mock_settings} + + await menu_callback(update, context) + + call_kwargs = update.callback_query.edit_message_text.call_args + text = call_kwargs.args[0] + assert "/home/test" in text + assert "active" in text + + async def test_run_direct_command_status_no_session( + self, builder: MenuBuilder + ) -> None: + """When /status is tapped with no session, shows 'none'.""" + builder.build_top_level() + _, _ = builder.build_category("bot") + cmd_sid = None + for sid, full_id in builder.id_map.items(): + if full_id == "bot:status": + cmd_sid = sid + break + assert cmd_sid is not None + + update = self._make_callback_update(f"menu:run:{cmd_sid}") + + mock_settings = MagicMock() + mock_settings.approved_directory = Path("/tmp/projects") + + context = MagicMock() + context.user_data = { + "menu_builder": builder, + "menu_scanner": None, + } + context.bot_data = {"settings": mock_settings} + + await menu_callback(update, context) + + call_kwargs = update.callback_query.edit_message_text.call_args + text = call_kwargs.args[0] + assert "none" in text + + async def test_run_direct_command_stop_no_active( + self, builder: MenuBuilder + ) -> None: + """When /stop is tapped with no active calls, shows message.""" + builder.build_top_level() + _, _ = builder.build_category("bot") + cmd_sid = None + for sid, full_id in builder.id_map.items(): + if full_id == "bot:stop": + cmd_sid = sid + break + assert cmd_sid is not None + + update = self._make_callback_update(f"menu:run:{cmd_sid}") + + context = MagicMock() + context.user_data = { + "menu_builder": builder, + "menu_scanner": None, + } + context.bot_data = {} + + await menu_callback(update, context) + + call_kwargs = update.callback_query.edit_message_text.call_args + assert "No active calls" in call_kwargs.args[0] + + async def test_run_inject_skill_updates_session_and_clears_force_new( + self, builder: MenuBuilder + ) -> None: + """After a successful skill call, force_new is cleared and session updated.""" + builder.build_top_level() + skill_sid = None + for sid, full_id in builder.id_map.items(): + if full_id == "custom:summarize": + skill_sid = sid + break + assert skill_sid is not None + + update = self._make_callback_update(f"menu:run:{skill_sid}") + update.callback_query.message.chat_id = 12345 + update.callback_query.message.delete = AsyncMock() + update.callback_query.from_user.id = 999 + + mock_response = MagicMock() + mock_response.content = "Done." + mock_response.session_id = "new-session-id" + mock_response.cost = 0.02 + + mock_claude = AsyncMock() + mock_claude.run_command = AsyncMock(return_value=mock_response) + + mock_settings = MagicMock() + mock_settings.approved_directory = Path("/tmp/test") + + mock_formatted = MagicMock() + mock_formatted.text = "Done." + mock_formatted.parse_mode = "HTML" + + context = MagicMock() + context.user_data = { + "menu_builder": builder, + "menu_scanner": None, + "force_new_session": True, + "claude_session_id": "old-session", + } + context.bot_data = { + "claude_integration": mock_claude, + "settings": mock_settings, + } + context.bot.send_chat_action = AsyncMock() + context.bot.send_message = AsyncMock() + + with patch( + "src.bot.handlers.menu.ResponseFormatter" + ) as MockFormatter: + formatter_instance = MockFormatter.return_value + formatter_instance.format_claude_response.return_value = [ + mock_formatted + ] + await menu_callback(update, context) + + # force_new should be cleared + assert context.user_data["force_new_session"] is False + # Session ID should be updated + assert context.user_data["claude_session_id"] == "new-session-id" + # force_new=True should have been passed to run_command + call_kwargs = mock_claude.run_command.call_args.kwargs + assert call_kwargs["force_new"] is True async def test_tog_toggles_plugin(self, builder: MenuBuilder) -> None: builder.build_top_level() From aba725194455cf47619cc7a57504b323bb6ba192 Mon Sep 17 00:00:00 2001 From: F1orian <27896106+F1orian@users.noreply.github.com> Date: Sat, 28 Feb 2026 01:19:55 +0100 Subject: [PATCH 09/16] style: black formatting for command_palette.py and menu.py Co-Authored-By: Claude Opus 4.6 --- src/bot/features/command_palette.py | 8 +--- src/bot/handlers/menu.py | 73 +++++++---------------------- 2 files changed, 19 insertions(+), 62 deletions(-) diff --git a/src/bot/features/command_palette.py b/src/bot/features/command_palette.py index 6c521468..9e7b3c66 100644 --- a/src/bot/features/command_palette.py +++ b/src/bot/features/command_palette.py @@ -165,9 +165,7 @@ def scan(self) -> Tuple[List[PaletteItem], List[PluginInfo]]: short_name = qualified_name.split("@")[0] is_enabled = enabled_plugins.get(qualified_name, False) - is_blocked = any( - b.get("plugin") == qualified_name for b in blocklisted - ) + is_blocked = any(b.get("plugin") == qualified_name for b in blocklisted) effective_enabled = is_enabled and not is_blocked plugin_items: List[PaletteItem] = [] @@ -312,9 +310,7 @@ def _load_enabled_plugins(self) -> Dict[str, bool]: def _load_installed_plugins(self) -> Dict[str, list]: """Load the installed plugins map from installed_plugins.json.""" - installed_file = ( - self.claude_dir / "plugins" / "installed_plugins.json" - ) + installed_file = self.claude_dir / "plugins" / "installed_plugins.json" if not installed_file.is_file(): return {} try: diff --git a/src/bot/handlers/menu.py b/src/bot/handlers/menu.py index 69e71b7f..f7ca8cf2 100644 --- a/src/bot/handlers/menu.py +++ b/src/bot/handlers/menu.py @@ -27,9 +27,7 @@ class MenuBuilder: incrementing numeric IDs and store the mapping in ``id_map``. """ - def __init__( - self, items: List[PaletteItem], plugins: List[PluginInfo] - ) -> None: + def __init__(self, items: List[PaletteItem], plugins: List[PluginInfo]) -> None: self.items = items self.plugins = plugins self.id_map: Dict[str, str] = {} # short_id -> full_id @@ -98,17 +96,13 @@ def build_top_level(self) -> InlineKeyboardMarkup: sid = self._register(item.id) label = f"{status} {plugin.name}" plugin_buttons.append( - InlineKeyboardButton( - label, callback_data=f"menu:run:{sid}" - ) + InlineKeyboardButton(label, callback_data=f"menu:run:{sid}") ) else: sid = self._register(f"cat:{plugin.name}") label = f"{status} {plugin.name} ({len(plugin.items)})" plugin_buttons.append( - InlineKeyboardButton( - label, callback_data=f"menu:cat:{sid}" - ) + InlineKeyboardButton(label, callback_data=f"menu:cat:{sid}") ) # Pack plugin buttons 2-per-row for i in range(0, len(plugin_buttons), 2): @@ -162,14 +156,10 @@ def build_category(self, source: str) -> Tuple[InlineKeyboardMarkup, str]: rows: List[List[InlineKeyboardButton]] = [] for item in cat_items: sid = self._register(item.id) - label = f"{item.name} — {item.description}" if item.description else item.name - rows.append( - [ - InlineKeyboardButton( - label, callback_data=f"menu:run:{sid}" - ) - ] + label = ( + f"{item.name} — {item.description}" if item.description else item.name ) + rows.append([InlineKeyboardButton(label, callback_data=f"menu:run:{sid}")]) # Toggle button for plugins (not bot) if source != "bot": @@ -187,13 +177,7 @@ def build_category(self, source: str) -> Tuple[InlineKeyboardMarkup, str]: ) # Back button - rows.append( - [ - InlineKeyboardButton( - "\u2b05 Back", callback_data="menu:back" - ) - ] - ) + rows.append([InlineKeyboardButton("\u2b05 Back", callback_data="menu:back")]) return InlineKeyboardMarkup(rows), header @@ -225,9 +209,7 @@ def _find_plugin(self, name: str) -> Optional[PluginInfo]: # ====================================================================== -async def menu_command( - update: Update, context: ContextTypes.DEFAULT_TYPE -) -> None: +async def menu_command(update: Update, context: ContextTypes.DEFAULT_TYPE) -> None: """Handle ``/menu`` -- scan filesystem, build top-level keyboard, send.""" scanner = CommandPaletteScanner() items, plugins = scanner.scan() @@ -264,9 +246,7 @@ async def menu_command( ) -async def menu_callback( - update: Update, context: ContextTypes.DEFAULT_TYPE -) -> None: +async def menu_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) -> None: """Handle all ``menu:`` callbacks. Actions: @@ -311,9 +291,7 @@ async def menu_callback( f"{item_count} commands \u00b7 {plugin_count} plugins" f" \u00b7 {custom_count} custom skills" ) - await query.edit_message_text( - text, parse_mode="HTML", reply_markup=keyboard - ) + await query.edit_message_text(text, parse_mode="HTML", reply_markup=keyboard) return # ------------------------------------------------------------------ @@ -321,13 +299,7 @@ async def menu_callback( # ------------------------------------------------------------------ if data == "menu:store": back_keyboard = InlineKeyboardMarkup( - [ - [ - InlineKeyboardButton( - "\u2b05 Back", callback_data="menu:back" - ) - ] - ] + [[InlineKeyboardButton("\u2b05 Back", callback_data="menu:back")]] ) await query.edit_message_text( "\U0001f50c Plugin Store\n\nComing soon.", @@ -373,9 +345,7 @@ async def menu_callback( keyboard, header = builder.build_category(source_name) state_label = "enabled" if new_state else "disabled" text = f"{header}\n\nPlugin {state_label}." - await query.edit_message_text( - text, parse_mode="HTML", reply_markup=keyboard - ) + await query.edit_message_text(text, parse_mode="HTML", reply_markup=keyboard) logger.info( "Plugin toggled", plugin=qualified_name, @@ -399,9 +369,7 @@ async def menu_callback( source = full_id[len("cat:") :] keyboard, header = builder.build_category(source) - await query.edit_message_text( - header, parse_mode="HTML", reply_markup=keyboard - ) + await query.edit_message_text(header, parse_mode="HTML", reply_markup=keyboard) return # ------------------------------------------------------------------ @@ -459,9 +427,7 @@ async def menu_callback( # Start typing indicator chat_id = query.message.chat_id try: - await context.bot.send_chat_action( - chat_id=chat_id, action="typing" - ) + await context.bot.send_chat_action(chat_id=chat_id, action="typing") except Exception: pass @@ -483,9 +449,7 @@ async def menu_callback( # Format response formatter = ResponseFormatter(settings) - formatted_messages = formatter.format_claude_response( - response.content - ) + formatted_messages = formatter.format_claude_response(response.content) # Delete the "Working..." message try: @@ -562,9 +526,7 @@ async def menu_callback( elif cmd == "/status": current_dir = context.user_data.get( "current_directory", - settings.approved_directory - if settings - else "unknown", + settings.approved_directory if settings else "unknown", ) session_id = context.user_data.get("claude_session_id") session_status = "active" if session_id else "none" @@ -586,8 +548,7 @@ async def menu_callback( elif cmd == "/verbose": level = context.user_data.get("verbose_level", 1) await query.edit_message_text( - f"Verbose level: {level}\n" - f"Use /verbose 0|1|2 to change." + f"Verbose level: {level}\n" f"Use /verbose 0|1|2 to change." ) elif cmd == "/repo": From 3bef1c4557c865813a3708de9c30696db60aa0ca Mon Sep 17 00:00:00 2001 From: F1orian <27896106+F1orian@users.noreply.github.com> Date: Mon, 2 Mar 2026 21:35:44 +0100 Subject: [PATCH 10/16] feat: load plugins into SDK sessions and fix menu skill invocation - Add get_enabled_plugin_paths() to command_palette.py to resolve installed plugin paths from ~/.claude/ config files - Pass enabled plugins to ClaudeAgentOptions via SdkPluginConfig so skills/commands from plugins are available in CLI sessions - Change setting_sources from ["project"] to ["project", "user"] so custom user skills (~/.claude/skills/) are also discovered - Fix menu skill invocation to send slash commands directly instead of hacky description-based prompts - Add message_thread_id to all send_message calls in menu handler to fix wrong-channel responses in supergroup forum topics - Add fallback confirmation message when skill produces no visible text output - Include design docs for the dynamic command menu feature Co-Authored-By: Claude Opus 4.6 --- .../2026-02-28-dynamic-command-menu-design.md | 215 ++ docs/plans/2026-02-28-dynamic-command-menu.md | 1725 +++++++++++++++++ src/bot/features/command_palette.py | 57 + src/bot/handlers/menu.py | 31 +- src/claude/sdk_integration.py | 20 +- 5 files changed, 2046 insertions(+), 2 deletions(-) create mode 100644 docs/plans/2026-02-28-dynamic-command-menu-design.md create mode 100644 docs/plans/2026-02-28-dynamic-command-menu.md diff --git a/docs/plans/2026-02-28-dynamic-command-menu-design.md b/docs/plans/2026-02-28-dynamic-command-menu-design.md new file mode 100644 index 00000000..f605668a --- /dev/null +++ b/docs/plans/2026-02-28-dynamic-command-menu-design.md @@ -0,0 +1,215 @@ +# Dynamic Command Menu & Plugin Manager + +**Date:** 2026-02-28 +**Status:** Approved + +## Summary + +Add a `/menu` command (wired to Telegram's persistent menu button) that dynamically discovers all Claude Code skills, commands, and plugins from the filesystem and presents them as a navigable inline keyboard. Includes full plugin management (browse, enable/disable, install/update). + +## Requirements + +1. **Dynamic discovery** — Scan `~/.claude/` on each menu open (no caching) +2. **Unified menu** — Bot commands + Claude Code skills/commands + custom skills in one place +3. **Natural categories** — Plugin names as groupings (not hardcoded taxonomy) +4. **Plugin management** — Browse, enable/disable, install new, update existing +5. **Persistent menu button** — Always-visible access via Telegram's menu button +6. **In-place navigation** — Edit existing message, don't spam new ones + +## Architecture + +### Navigation Model + +``` +/menu (top level) +├── 🤖 Bot (5) → /new, /status, /repo, /verbose, /stop +├── ⚡ superpowers (14) → brainstorming, TDD, debugging, ... +├── 📝 commit-commands (3) → /commit, /commit-push-pr, /clean_gone +├── 🔍 code-review (2) → /code-review, /review-pr +├── 🚀 feature-dev (1) → executes directly (single item) +├── 🎨 frontend-design (1) → executes directly +├── 📋 claude-md-management (2) → /revise-claude-md, /claude-md-improver +├── ⚙️ obsidian-cli (1) → executes directly (custom skill) +├── ⚙️ defuddle (1) → executes directly +├── ...more custom skills... +└── 📦 Plugin Store → Search & install new plugins +``` + +Single-item plugins/skills execute directly on tap (no sub-menu). + +### Callback Data Convention + +``` +Format: "menu:{action}:{argument}" + +Navigation: + "menu:cat:{plugin_name}" → show plugin's skills/commands + "menu:back" → return to top level + "menu:back:{plugin_name}" → return to plugin from detail view + +Execution: + "menu:run:{item_id}" → execute a bot command or inject a skill + +Plugin management: + "menu:plug:{plugin_name}" → show plugin detail (info, toggle, skills list) + "menu:tog:{plugin_name}" → toggle plugin enable/disable + "menu:inst:{plugin_name}" → install plugin from store + "menu:upd:{plugin_name}" → update plugin to latest version + +Store: + "menu:store" → show plugin store + "menu:store:p{page}" → paginate store results +``` + +Note: Telegram limits callback_data to 64 bytes. Plugin/skill names may need truncation + lookup table. + +### Data Model + +```python +@dataclass +class PaletteItem: + id: str # unique ID, e.g. "superpowers:brainstorming" + name: str # display name from SKILL.md frontmatter + description: str # from SKILL.md frontmatter + action_type: ActionType # DIRECT_COMMAND | INJECT_SKILL + action_value: str # "/status" or "/commit" or "brainstorming" + icon: str # emoji (derived or default) + source: str # plugin name or "bot" or "custom" + enabled: bool # cross-referenced with settings.json + +class ActionType(Enum): + DIRECT_COMMAND = "direct" # bot handles directly (e.g. /status) + INJECT_SKILL = "inject" # send as text to Claude session + +@dataclass +class PluginInfo: + name: str + version: str + enabled: bool + items: list[PaletteItem] # skills + commands in this plugin + path: str # filesystem path +``` + +### Scanner Implementation + +```python +class CommandPaletteScanner: + CLAUDE_DIR = Path.home() / ".claude" + + def scan(self) -> tuple[list[PaletteItem], list[PluginInfo]]: + items = [] + plugins = [] + + # 1. Bot commands (hardcoded, always present) + items.extend(self._get_bot_commands()) + + # 2. Plugin skills: ~/.claude/plugins/cache/claude-plugins-official/*/skills/*/SKILL.md + # 3. Plugin commands: ~/.claude/plugins/cache/claude-plugins-official/*/commands/*.md + for plugin_dir in self._get_plugin_dirs(): + plugin_info = self._scan_plugin(plugin_dir) + plugins.append(plugin_info) + items.extend(plugin_info.items) + + # 4. Custom skills: ~/.claude/skills/*/SKILL.md + for skill_dir in self._get_custom_skill_dirs(): + items.append(self._scan_custom_skill(skill_dir)) + + # 5. Cross-reference with settings.json + blocklist.json + self._apply_enabled_state(items, plugins) + + return items, plugins + + def _parse_skill_frontmatter(self, path: Path) -> dict: + """Parse YAML frontmatter from SKILL.md or command .md file.""" + # Extract name, description from --- delimited YAML block + ... +``` + +### Filesystem Paths Scanned + +| Path | What | Category | +|------|------|----------| +| (hardcoded) | Bot commands: /new, /status, /repo, /verbose, /stop | bot | +| `~/.claude/plugins/cache/claude-plugins-official/{plugin}/{version}/skills/*/SKILL.md` | Official plugin skills | plugin name | +| `~/.claude/plugins/cache/claude-plugins-official/{plugin}/{version}/commands/*.md` | Official plugin commands | plugin name | +| `~/.claude/skills/*/SKILL.md` | Custom skills | custom | +| `~/.claude/settings.json` | Enabled plugins list | (config) | +| `~/.claude/plugins/installed_plugins.json` | Plugin versions & metadata | (config) | +| `~/.claude/plugins/blocklist.json` | Disabled plugins | (config) | + +### Action Execution + +| Action Type | Behavior | +|-------------|----------| +| `DIRECT_COMMAND` | Call the bot's own command handler function directly (e.g., `agentic_status()`) | +| `INJECT_SKILL` | Send skill name as user text to active Claude session via `ClaudeIntegration.run_command()` | + +For skill injection, the bot constructs a message like `/commit` or `/feature-dev` and feeds it to Claude as if the user typed it. This leverages Claude's existing skill invocation mechanism. + +### Plugin Management + +**Enable/Disable:** +- Read `~/.claude/plugins/blocklist.json` +- Add/remove plugin name from blocklist array +- Write back to file +- Display confirmation with note: "Takes effect on next /new session" + +**Install (Plugin Store):** +- Query available plugins (TBD: either scrape Claude Code plugin registry or maintain a known-plugins list) +- Run `claude plugins install {name}` via shell if CLI supports it +- Otherwise, manual download + write to `installed_plugins.json` +- Notify user of success/failure + +**Update:** +- Compare installed version (from `installed_plugins.json`) with latest available +- Run update command or re-download + +### Persistent Menu Button + +On bot startup, call: +```python +await bot.set_chat_menu_button( + menu_button=MenuButtonCommands() # shows / command list +) +``` + +Register `/menu` in `get_bot_commands()` so it appears in Telegram's command autocomplete. The menu button in Telegram opens the command list where `/menu` is the first entry. + +Alternative: Use `MenuButtonWebApp` if we ever migrate to a WebApp approach. + +## New Files + +| File | Purpose | +|------|---------| +| `src/bot/features/command_palette.py` | `CommandPaletteScanner`, `PaletteItem`, `PluginInfo`, `ActionType` | +| `src/bot/handlers/menu.py` | `/menu` command handler, callback handler for menu navigation, plugin management actions | + +## Modified Files + +| File | Change | +|------|--------| +| `src/bot/orchestrator.py` | Register `/menu` handler + menu callback handler in `_register_agentic_handlers()` | +| `src/bot/orchestrator.py` | Add `/menu` to `get_bot_commands()` | +| `src/bot/core.py` | Set persistent menu button on startup | + +## Callback Data Size Constraint + +Telegram limits `callback_data` to 64 bytes. Plugin/skill names can be long (e.g., "claude-md-management:claude-md-improver" = 40 chars + "menu:run:" prefix = 49 chars). Strategy: + +- Use short numeric IDs in callback data: `"menu:run:7"`, `"menu:cat:3"` +- Maintain a per-message mapping dict `{short_id: full_item_id}` stored in `context.user_data` +- Mapping refreshed on each menu render + +## Error Handling + +- If `~/.claude/` doesn't exist: show only Bot commands, display note +- If a SKILL.md has invalid frontmatter: skip it, log warning +- If plugin toggle fails (permission error): show error inline +- If skill injection fails (no active session): prompt user to start a session first + +## Testing Strategy + +- Unit tests for `CommandPaletteScanner` with mock filesystem +- Unit tests for callback data routing +- Unit tests for frontmatter parsing (valid, invalid, missing) +- Integration test for menu navigation flow (mock Telegram API) diff --git a/docs/plans/2026-02-28-dynamic-command-menu.md b/docs/plans/2026-02-28-dynamic-command-menu.md new file mode 100644 index 00000000..963791ef --- /dev/null +++ b/docs/plans/2026-02-28-dynamic-command-menu.md @@ -0,0 +1,1725 @@ +# Dynamic Command Menu & Plugin Manager — Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Add a `/menu` command with persistent menu button that dynamically discovers all Claude Code skills, commands, and plugins from `~/.claude/`, presents them as navigable inline keyboard categories (grouped by plugin name), and supports full plugin management (enable/disable/install/update). + +**Architecture:** New `CommandPaletteScanner` scans `~/.claude/` filesystem on each invocation, parsing YAML frontmatter from SKILL.md files and cross-referencing `settings.json`/`blocklist.json` for enabled state. A `/menu` command + `CallbackQueryHandler(pattern=r"^menu:")` handles multi-level inline keyboard navigation. Bot commands execute directly; Claude Code skills inject the skill name as text into the active Claude session via `agentic_text()`. + +**Tech Stack:** python-telegram-bot (InlineKeyboardMarkup, CallbackQueryHandler), PyYAML (frontmatter parsing), pathlib (filesystem scanning), existing bot patterns from `orchestrator.py`. + +**Design doc:** `docs/plans/2026-02-28-dynamic-command-menu-design.md` + +--- + +### Task 1: Data Model — PaletteItem and PluginInfo + +**Files:** +- Create: `src/bot/features/command_palette.py` +- Test: `tests/unit/test_bot/test_command_palette.py` + +**Step 1: Write the failing test** + +```python +# tests/unit/test_bot/test_command_palette.py +"""Tests for the command palette scanner.""" + +import pytest + +from src.bot.features.command_palette import ( + ActionType, + PaletteItem, + PluginInfo, +) + + +def test_palette_item_creation(): + item = PaletteItem( + id="superpowers:brainstorming", + name="brainstorming", + description="Use before creative work", + action_type=ActionType.INJECT_SKILL, + action_value="/brainstorming", + source="superpowers", + enabled=True, + ) + assert item.id == "superpowers:brainstorming" + assert item.action_type == ActionType.INJECT_SKILL + assert item.enabled is True + + +def test_plugin_info_creation(): + item = PaletteItem( + id="superpowers:brainstorming", + name="brainstorming", + description="desc", + action_type=ActionType.INJECT_SKILL, + action_value="/brainstorming", + source="superpowers", + enabled=True, + ) + plugin = PluginInfo( + name="superpowers", + qualified_name="superpowers@claude-plugins-official", + version="4.3.1", + enabled=True, + items=[item], + install_path="/home/user/.claude/plugins/cache/claude-plugins-official/superpowers/4.3.1", + ) + assert plugin.name == "superpowers" + assert len(plugin.items) == 1 + assert plugin.enabled is True + + +def test_action_type_enum(): + assert ActionType.DIRECT_COMMAND.value == "direct" + assert ActionType.INJECT_SKILL.value == "inject" +``` + +**Step 2: Run test to verify it fails** + +Run: `cd /home/florian/config/claude-code-telegram && poetry run pytest tests/unit/test_bot/test_command_palette.py -v` +Expected: FAIL with `ModuleNotFoundError: No module named 'src.bot.features.command_palette'` + +**Step 3: Write minimal implementation** + +```python +# src/bot/features/command_palette.py +"""Dynamic command palette: scans ~/.claude/ for skills, commands, and plugins.""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from enum import Enum +from typing import List, Optional + + +class ActionType(Enum): + """How a palette item is executed.""" + + DIRECT_COMMAND = "direct" # Bot handles directly (e.g. /status) + INJECT_SKILL = "inject" # Send as text to Claude session (e.g. /commit) + + +@dataclass +class PaletteItem: + """A single actionable item in the command palette.""" + + id: str # unique, e.g. "superpowers:brainstorming" + name: str # display name from SKILL.md frontmatter + description: str # from SKILL.md frontmatter + action_type: ActionType + action_value: str # "/status" or "/commit" etc. + source: str # plugin name, "bot", or "custom" + enabled: bool = True + + +@dataclass +class PluginInfo: + """Metadata about an installed plugin.""" + + name: str # short name, e.g. "superpowers" + qualified_name: str # e.g. "superpowers@claude-plugins-official" + version: str + enabled: bool + items: List[PaletteItem] = field(default_factory=list) + install_path: str = "" +``` + +**Step 4: Run test to verify it passes** + +Run: `cd /home/florian/config/claude-code-telegram && poetry run pytest tests/unit/test_bot/test_command_palette.py -v` +Expected: PASS (3 tests) + +**Step 5: Commit** + +```bash +git add src/bot/features/command_palette.py tests/unit/test_bot/test_command_palette.py +git commit -m "feat(menu): add PaletteItem and PluginInfo data models" +``` + +--- + +### Task 2: YAML Frontmatter Parser + +**Files:** +- Modify: `src/bot/features/command_palette.py` +- Test: `tests/unit/test_bot/test_command_palette.py` + +**Step 1: Write the failing test** + +Add to `tests/unit/test_bot/test_command_palette.py`: + +```python +from src.bot.features.command_palette import parse_skill_frontmatter + + +def test_parse_skill_frontmatter_valid(): + content = """--- +name: brainstorming +description: Use before creative work +--- + +# Brainstorming +Some content here. +""" + result = parse_skill_frontmatter(content) + assert result["name"] == "brainstorming" + assert result["description"] == "Use before creative work" + + +def test_parse_skill_frontmatter_no_frontmatter(): + content = "# Just a heading\nNo frontmatter here." + result = parse_skill_frontmatter(content) + assert result == {} + + +def test_parse_skill_frontmatter_with_allowed_tools(): + content = """--- +name: commit +description: Create a git commit +allowed-tools: Bash(git add:*), Bash(git commit:*) +--- +""" + result = parse_skill_frontmatter(content) + assert result["name"] == "commit" + assert "allowed-tools" in result + + +def test_parse_skill_frontmatter_empty_content(): + result = parse_skill_frontmatter("") + assert result == {} + + +def test_parse_skill_frontmatter_malformed_yaml(): + content = """--- +name: [invalid yaml +description: missing bracket +--- +""" + result = parse_skill_frontmatter(content) + assert result == {} +``` + +**Step 2: Run test to verify it fails** + +Run: `cd /home/florian/config/claude-code-telegram && poetry run pytest tests/unit/test_bot/test_command_palette.py::test_parse_skill_frontmatter_valid -v` +Expected: FAIL with `ImportError: cannot import name 'parse_skill_frontmatter'` + +**Step 3: Write minimal implementation** + +Add to `src/bot/features/command_palette.py`: + +```python +import re +from typing import Dict, Any + +import yaml + +import structlog + +logger = structlog.get_logger() + + +def parse_skill_frontmatter(content: str) -> Dict[str, Any]: + """Parse YAML frontmatter from a SKILL.md or command .md file. + + Expects format: + --- + name: skill-name + description: what it does + --- + """ + if not content.strip(): + return {} + + match = re.match(r"^---\s*\n(.*?)\n---", content, re.DOTALL) + if not match: + return {} + + try: + parsed = yaml.safe_load(match.group(1)) + return parsed if isinstance(parsed, dict) else {} + except yaml.YAMLError: + logger.warning("Failed to parse SKILL.md frontmatter") + return {} +``` + +**Step 4: Run all frontmatter tests** + +Run: `cd /home/florian/config/claude-code-telegram && poetry run pytest tests/unit/test_bot/test_command_palette.py -k frontmatter -v` +Expected: PASS (5 tests) + +**Step 5: Commit** + +```bash +git add src/bot/features/command_palette.py tests/unit/test_bot/test_command_palette.py +git commit -m "feat(menu): add YAML frontmatter parser for SKILL.md files" +``` + +--- + +### Task 3: CommandPaletteScanner — Filesystem Discovery + +**Files:** +- Modify: `src/bot/features/command_palette.py` +- Test: `tests/unit/test_bot/test_command_palette.py` + +This is the core scanner. It needs to read from specific filesystem paths under `~/.claude/`. We'll make the base path configurable for testing. + +**Step 1: Write the failing tests** + +Add to `tests/unit/test_bot/test_command_palette.py`: + +```python +import tempfile +from pathlib import Path + +from src.bot.features.command_palette import CommandPaletteScanner + + +@pytest.fixture +def mock_claude_dir(): + """Create a mock ~/.claude/ directory structure.""" + with tempfile.TemporaryDirectory() as d: + base = Path(d) + + # settings.json + settings = base / "settings.json" + settings.write_text( + '{"enabledPlugins": {"superpowers@claude-plugins-official": true, ' + '"commit-commands@claude-plugins-official": true}}' + ) + + # installed_plugins.json + plugins_dir = base / "plugins" + plugins_dir.mkdir() + installed = plugins_dir / "installed_plugins.json" + installed.write_text( + '{"version": 2, "plugins": {' + '"superpowers@claude-plugins-official": [{"scope": "user", ' + '"installPath": "' + str(base) + '/plugins/cache/claude-plugins-official/superpowers/1.0", ' + '"version": "1.0"}], ' + '"commit-commands@claude-plugins-official": [{"scope": "user", ' + '"installPath": "' + str(base) + '/plugins/cache/claude-plugins-official/commit-commands/1.0", ' + '"version": "1.0"}]' + "}}" + ) + + # blocklist.json + blocklist = plugins_dir / "blocklist.json" + blocklist.write_text('{"fetchedAt": "2026-01-01", "plugins": []}') + + # Plugin: superpowers with 1 skill + cache = plugins_dir / "cache" / "claude-plugins-official" + sp_dir = cache / "superpowers" / "1.0" / "skills" / "brainstorming" + sp_dir.mkdir(parents=True) + (sp_dir / "SKILL.md").write_text( + "---\nname: brainstorming\n" + "description: Use before creative work\n---\n# Content" + ) + + # Plugin: commit-commands with 1 command + cc_dir = cache / "commit-commands" / "1.0" / "commands" + cc_dir.mkdir(parents=True) + (cc_dir / "commit.md").write_text( + "---\nname: commit\ndescription: Create a git commit\n---\n# Content" + ) + + # Custom skill + custom_dir = base / "skills" / "defuddle" + custom_dir.mkdir(parents=True) + (custom_dir / "SKILL.md").write_text( + "---\nname: defuddle\n" + "description: Extract markdown from web pages\n---\n# Content" + ) + + yield base + + +def test_scanner_discovers_bot_commands(mock_claude_dir): + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + items, plugins = scanner.scan() + bot_items = [i for i in items if i.source == "bot"] + assert len(bot_items) >= 5 # start, new, status, verbose, repo, stop + assert all(i.action_type == ActionType.DIRECT_COMMAND for i in bot_items) + + +def test_scanner_discovers_plugin_skills(mock_claude_dir): + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + items, plugins = scanner.scan() + skill_items = [i for i in items if i.id == "superpowers:brainstorming"] + assert len(skill_items) == 1 + assert skill_items[0].name == "brainstorming" + assert skill_items[0].action_type == ActionType.INJECT_SKILL + + +def test_scanner_discovers_plugin_commands(mock_claude_dir): + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + items, plugins = scanner.scan() + cmd_items = [i for i in items if i.id == "commit-commands:commit"] + assert len(cmd_items) == 1 + assert cmd_items[0].name == "commit" + assert cmd_items[0].action_type == ActionType.INJECT_SKILL + + +def test_scanner_discovers_custom_skills(mock_claude_dir): + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + items, plugins = scanner.scan() + custom = [i for i in items if i.source == "custom"] + assert len(custom) == 1 + assert custom[0].name == "defuddle" + + +def test_scanner_builds_plugin_info(mock_claude_dir): + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + items, plugins = scanner.scan() + sp = [p for p in plugins if p.name == "superpowers"] + assert len(sp) == 1 + assert sp[0].version == "1.0" + assert sp[0].enabled is True + assert len(sp[0].items) == 1 + + +def test_scanner_handles_missing_claude_dir(): + scanner = CommandPaletteScanner(claude_dir=Path("/nonexistent")) + items, plugins = scanner.scan() + # Should still return bot commands + bot_items = [i for i in items if i.source == "bot"] + assert len(bot_items) >= 5 + assert len(plugins) == 0 +``` + +**Step 2: Run to verify failure** + +Run: `cd /home/florian/config/claude-code-telegram && poetry run pytest tests/unit/test_bot/test_command_palette.py::test_scanner_discovers_bot_commands -v` +Expected: FAIL with `ImportError: cannot import name 'CommandPaletteScanner'` + +**Step 3: Write implementation** + +Add to `src/bot/features/command_palette.py`: + +```python +import json +from pathlib import Path + + +# Default bot commands (always present in agentic mode) +BOT_COMMANDS = [ + PaletteItem( + id="bot:new", name="new", description="Start a fresh session", + action_type=ActionType.DIRECT_COMMAND, action_value="/new", source="bot", + ), + PaletteItem( + id="bot:status", name="status", description="Show session status", + action_type=ActionType.DIRECT_COMMAND, action_value="/status", source="bot", + ), + PaletteItem( + id="bot:repo", name="repo", description="List repos / switch workspace", + action_type=ActionType.DIRECT_COMMAND, action_value="/repo", source="bot", + ), + PaletteItem( + id="bot:verbose", name="verbose", description="Set output verbosity (0/1/2)", + action_type=ActionType.DIRECT_COMMAND, action_value="/verbose", source="bot", + ), + PaletteItem( + id="bot:stop", name="stop", description="Stop running Claude call", + action_type=ActionType.DIRECT_COMMAND, action_value="/stop", source="bot", + ), +] + + +class CommandPaletteScanner: + """Scans ~/.claude/ for all available skills, commands, and plugins.""" + + def __init__(self, claude_dir: Optional[Path] = None) -> None: + self.claude_dir = claude_dir or Path.home() / ".claude" + + def scan(self) -> tuple[List[PaletteItem], List[PluginInfo]]: + """Discover all palette items and plugin info from the filesystem.""" + items: List[PaletteItem] = [] + plugins: List[PluginInfo] = [] + + # 1. Bot commands (always present) + items.extend(BOT_COMMANDS) + + if not self.claude_dir.is_dir(): + return items, plugins + + # Load config files + enabled_plugins = self._load_enabled_plugins() + installed_plugins = self._load_installed_plugins() + blocklisted = self._load_blocklist() + + # 2. Scan installed plugins + for qualified_name, installs in installed_plugins.items(): + if not installs: + continue + install = installs[0] # use first (most recent) install + install_path = Path(install.get("installPath", "")) + version = install.get("version", "unknown") + short_name = qualified_name.split("@")[0] + + is_enabled = enabled_plugins.get(qualified_name, False) + is_blocked = any( + b.get("plugin") == qualified_name for b in blocklisted + ) + effective_enabled = is_enabled and not is_blocked + + plugin_items: List[PaletteItem] = [] + + # Scan skills + skills_dir = install_path / "skills" + if skills_dir.is_dir(): + for skill_dir in sorted(skills_dir.iterdir()): + skill_file = skill_dir / "SKILL.md" + if skill_file.is_file(): + item = self._parse_skill_file( + skill_file, short_name, effective_enabled + ) + if item: + plugin_items.append(item) + + # Scan commands + commands_dir = install_path / "commands" + if commands_dir.is_dir(): + for cmd_file in sorted(commands_dir.glob("*.md")): + item = self._parse_command_file( + cmd_file, short_name, effective_enabled + ) + if item: + plugin_items.append(item) + + plugin = PluginInfo( + name=short_name, + qualified_name=qualified_name, + version=version, + enabled=effective_enabled, + items=plugin_items, + install_path=str(install_path), + ) + plugins.append(plugin) + items.extend(plugin_items) + + # 3. Scan custom skills (~/.claude/skills/) + custom_dir = self.claude_dir / "skills" + if custom_dir.is_dir(): + for skill_dir in sorted(custom_dir.iterdir()): + skill_file = skill_dir / "SKILL.md" + if skill_file.is_file(): + item = self._parse_skill_file( + skill_file, "custom", True + ) + if item: + item.source = "custom" + items.append(item) + + return items, plugins + + def _parse_skill_file( + self, path: Path, source: str, enabled: bool + ) -> Optional[PaletteItem]: + """Parse a SKILL.md into a PaletteItem.""" + try: + content = path.read_text(encoding="utf-8") + except OSError: + return None + fm = parse_skill_frontmatter(content) + if not fm.get("name"): + return None + name = fm["name"] + return PaletteItem( + id=f"{source}:{name}", + name=name, + description=fm.get("description", ""), + action_type=ActionType.INJECT_SKILL, + action_value=f"/{name}", + source=source, + enabled=enabled, + ) + + def _parse_command_file( + self, path: Path, source: str, enabled: bool + ) -> Optional[PaletteItem]: + """Parse a command .md into a PaletteItem.""" + try: + content = path.read_text(encoding="utf-8") + except OSError: + return None + fm = parse_skill_frontmatter(content) + if not fm.get("name"): + return None + name = fm["name"] + return PaletteItem( + id=f"{source}:{name}", + name=name, + description=fm.get("description", ""), + action_type=ActionType.INJECT_SKILL, + action_value=f"/{name}", + source=source, + enabled=enabled, + ) + + def _load_enabled_plugins(self) -> Dict[str, bool]: + settings_file = self.claude_dir / "settings.json" + if not settings_file.is_file(): + return {} + try: + data = json.loads(settings_file.read_text(encoding="utf-8")) + return data.get("enabledPlugins", {}) + except (json.JSONDecodeError, OSError): + return {} + + def _load_installed_plugins(self) -> Dict[str, list]: + installed_file = self.claude_dir / "plugins" / "installed_plugins.json" + if not installed_file.is_file(): + return {} + try: + data = json.loads(installed_file.read_text(encoding="utf-8")) + return data.get("plugins", {}) + except (json.JSONDecodeError, OSError): + return {} + + def _load_blocklist(self) -> list: + blocklist_file = self.claude_dir / "plugins" / "blocklist.json" + if not blocklist_file.is_file(): + return [] + try: + data = json.loads(blocklist_file.read_text(encoding="utf-8")) + return data.get("plugins", []) + except (json.JSONDecodeError, OSError): + return [] +``` + +**Step 4: Run all scanner tests** + +Run: `cd /home/florian/config/claude-code-telegram && poetry run pytest tests/unit/test_bot/test_command_palette.py -k scanner -v` +Expected: PASS (6 tests) + +**Step 5: Commit** + +```bash +git add src/bot/features/command_palette.py tests/unit/test_bot/test_command_palette.py +git commit -m "feat(menu): add CommandPaletteScanner for filesystem discovery" +``` + +--- + +### Task 4: Plugin Toggle (Enable/Disable) + +**Files:** +- Modify: `src/bot/features/command_palette.py` +- Test: `tests/unit/test_bot/test_command_palette.py` + +**Step 1: Write the failing test** + +```python +def test_toggle_plugin_enable(mock_claude_dir): + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + # Disable superpowers + result = scanner.toggle_plugin("superpowers@claude-plugins-official", enabled=False) + assert result is True + + # Verify settings.json updated + settings = json.loads((mock_claude_dir / "settings.json").read_text()) + assert settings["enabledPlugins"]["superpowers@claude-plugins-official"] is False + + # Re-enable + result = scanner.toggle_plugin("superpowers@claude-plugins-official", enabled=True) + assert result is True + settings = json.loads((mock_claude_dir / "settings.json").read_text()) + assert settings["enabledPlugins"]["superpowers@claude-plugins-official"] is True + + +def test_toggle_plugin_nonexistent(mock_claude_dir): + scanner = CommandPaletteScanner(claude_dir=mock_claude_dir) + result = scanner.toggle_plugin("nonexistent@nowhere", enabled=False) + assert result is True # Still writes to settings +``` + +**Step 2: Run to verify failure** + +Run: `cd /home/florian/config/claude-code-telegram && poetry run pytest tests/unit/test_bot/test_command_palette.py::test_toggle_plugin_enable -v` +Expected: FAIL with `AttributeError: 'CommandPaletteScanner' object has no attribute 'toggle_plugin'` + +**Step 3: Write implementation** + +Add to `CommandPaletteScanner` class: + +```python + def toggle_plugin(self, qualified_name: str, enabled: bool) -> bool: + """Enable or disable a plugin by updating settings.json.""" + settings_file = self.claude_dir / "settings.json" + try: + if settings_file.is_file(): + data = json.loads(settings_file.read_text(encoding="utf-8")) + else: + data = {} + + if "enabledPlugins" not in data: + data["enabledPlugins"] = {} + + data["enabledPlugins"][qualified_name] = enabled + settings_file.write_text( + json.dumps(data, indent=2) + "\n", encoding="utf-8" + ) + return True + except (json.JSONDecodeError, OSError) as e: + logger.error("Failed to toggle plugin", plugin=qualified_name, error=str(e)) + return False +``` + +**Step 4: Run tests** + +Run: `cd /home/florian/config/claude-code-telegram && poetry run pytest tests/unit/test_bot/test_command_palette.py -k toggle -v` +Expected: PASS (2 tests) + +**Step 5: Commit** + +```bash +git add src/bot/features/command_palette.py tests/unit/test_bot/test_command_palette.py +git commit -m "feat(menu): add plugin enable/disable toggle" +``` + +--- + +### Task 5: Menu Keyboard Builder + +**Files:** +- Create: `src/bot/handlers/menu.py` +- Test: `tests/unit/test_bot/test_menu.py` + +This builds the inline keyboards for each navigation level. + +**Step 1: Write the failing tests** + +```python +# tests/unit/test_bot/test_menu.py +"""Tests for the menu handler keyboard building.""" + +import pytest +from telegram import InlineKeyboardButton, InlineKeyboardMarkup + +from src.bot.handlers.menu import MenuBuilder +from src.bot.features.command_palette import ( + ActionType, + PaletteItem, + PluginInfo, +) + + +@pytest.fixture +def sample_items(): + return [ + PaletteItem( + id="bot:new", name="new", description="Fresh session", + action_type=ActionType.DIRECT_COMMAND, action_value="/new", + source="bot", enabled=True, + ), + PaletteItem( + id="bot:status", name="status", description="Session status", + action_type=ActionType.DIRECT_COMMAND, action_value="/status", + source="bot", enabled=True, + ), + PaletteItem( + id="superpowers:brainstorming", name="brainstorming", + description="Creative work", action_type=ActionType.INJECT_SKILL, + action_value="/brainstorming", source="superpowers", enabled=True, + ), + PaletteItem( + id="commit-commands:commit", name="commit", + description="Git commit", action_type=ActionType.INJECT_SKILL, + action_value="/commit", source="commit-commands", enabled=True, + ), + ] + + +@pytest.fixture +def sample_plugins(): + return [ + PluginInfo( + name="superpowers", qualified_name="superpowers@claude-plugins-official", + version="4.3.1", enabled=True, + items=[PaletteItem( + id="superpowers:brainstorming", name="brainstorming", + description="Creative work", action_type=ActionType.INJECT_SKILL, + action_value="/brainstorming", source="superpowers", enabled=True, + )], + ), + PluginInfo( + name="commit-commands", qualified_name="commit-commands@claude-plugins-official", + version="1.0", enabled=True, + items=[PaletteItem( + id="commit-commands:commit", name="commit", + description="Git commit", action_type=ActionType.INJECT_SKILL, + action_value="/commit", source="commit-commands", enabled=True, + )], + ), + ] + + +def test_build_top_level_keyboard(sample_items, sample_plugins): + builder = MenuBuilder(sample_items, sample_plugins) + keyboard = builder.build_top_level() + assert isinstance(keyboard, InlineKeyboardMarkup) + # Should have Bot category + 2 plugin categories + Plugin Store + texts = [btn.text for row in keyboard.inline_keyboard for btn in row] + assert any("Bot" in t for t in texts) + assert any("superpowers" in t for t in texts) + + +def test_build_category_keyboard_bot(sample_items, sample_plugins): + builder = MenuBuilder(sample_items, sample_plugins) + keyboard, text = builder.build_category("bot") + assert isinstance(keyboard, InlineKeyboardMarkup) + texts = [btn.text for row in keyboard.inline_keyboard for btn in row] + assert any("new" in t for t in texts) + assert any("status" in t for t in texts) + # Should have Back button + assert any("Back" in t for t in texts) + + +def test_build_category_keyboard_plugin(sample_items, sample_plugins): + builder = MenuBuilder(sample_items, sample_plugins) + keyboard, text = builder.build_category("superpowers") + assert isinstance(keyboard, InlineKeyboardMarkup) + texts = [btn.text for row in keyboard.inline_keyboard for btn in row] + assert any("brainstorming" in t for t in texts) + + +def test_single_item_plugin_returns_none(sample_items, sample_plugins): + """Single-item plugins should return None (execute directly).""" + builder = MenuBuilder(sample_items, sample_plugins) + # commit-commands has only 1 item, so build_category returns None + result = builder.get_single_item_action("commit-commands") + assert result is not None + assert result.name == "commit" + + +def test_callback_id_mapping(sample_items, sample_plugins): + builder = MenuBuilder(sample_items, sample_plugins) + builder.build_top_level() + # Verify short ID mapping works + assert len(builder.id_map) > 0 + for short_id, full_id in builder.id_map.items(): + assert len(f"menu:cat:{short_id}") <= 64 # Telegram limit +``` + +**Step 2: Run to verify failure** + +Run: `cd /home/florian/config/claude-code-telegram && poetry run pytest tests/unit/test_bot/test_menu.py -v` +Expected: FAIL with `ModuleNotFoundError` + +**Step 3: Write implementation** + +```python +# src/bot/handlers/menu.py +"""Dynamic command menu with inline keyboard navigation.""" + +from __future__ import annotations + +from typing import Dict, List, Optional, Tuple + +import structlog +from telegram import InlineKeyboardButton, InlineKeyboardMarkup + +from ..features.command_palette import ActionType, PaletteItem, PluginInfo + +logger = structlog.get_logger() + + +class MenuBuilder: + """Builds inline keyboards for the command palette navigation.""" + + def __init__( + self, + items: List[PaletteItem], + plugins: List[PluginInfo], + ) -> None: + self.items = items + self.plugins = plugins + self.id_map: Dict[str, str] = {} # short_id -> full_id + self._counter = 0 + + def _short_id(self, full_id: str) -> str: + """Generate a short numeric ID and store the mapping.""" + self._counter += 1 + short = str(self._counter) + self.id_map[short] = full_id + return short + + def build_top_level(self) -> InlineKeyboardMarkup: + """Build the top-level category menu.""" + keyboard: List[List[InlineKeyboardButton]] = [] + self.id_map.clear() + self._counter = 0 + + # Bot commands category + bot_items = [i for i in self.items if i.source == "bot"] + if bot_items: + sid = self._short_id("bot") + keyboard.append([ + InlineKeyboardButton( + f"\U0001f916 Bot ({len(bot_items)})", + callback_data=f"menu:cat:{sid}", + ) + ]) + + # Plugin categories (2 per row for compact layout) + row: List[InlineKeyboardButton] = [] + for plugin in sorted(self.plugins, key=lambda p: p.name): + if not plugin.items: + continue + count = len(plugin.items) + status = "\u2705" if plugin.enabled else "\u274c" + sid = self._short_id(plugin.name) + + # Single-item plugins: execute directly on tap + if count == 1: + item = plugin.items[0] + isid = self._short_id(item.id) + btn = InlineKeyboardButton( + f"{status} {plugin.name}", + callback_data=f"menu:run:{isid}", + ) + else: + btn = InlineKeyboardButton( + f"{status} {plugin.name} ({count})", + callback_data=f"menu:cat:{sid}", + ) + + row.append(btn) + if len(row) == 2: + keyboard.append(row) + row = [] + if row: + keyboard.append(row) + + # Custom skills + custom_items = [i for i in self.items if i.source == "custom"] + if custom_items: + cust_row: List[InlineKeyboardButton] = [] + for item in custom_items: + sid = self._short_id(item.id) + cust_row.append( + InlineKeyboardButton( + f"\u2699\ufe0f {item.name}", + callback_data=f"menu:run:{sid}", + ) + ) + if len(cust_row) == 2: + keyboard.append(cust_row) + cust_row = [] + if cust_row: + keyboard.append(cust_row) + + # Plugin Store + keyboard.append([ + InlineKeyboardButton( + "\U0001f4e6 Plugin Store", callback_data="menu:store" + ) + ]) + + return InlineKeyboardMarkup(keyboard) + + def build_category(self, source: str) -> Tuple[InlineKeyboardMarkup, str]: + """Build keyboard for items in a given category/source. + + Returns (keyboard, header_text). + """ + if source == "bot": + cat_items = [i for i in self.items if i.source == "bot"] + header = "\U0001f916 Bot Commands" + else: + cat_items = [i for i in self.items if i.source == source] + plugin = next((p for p in self.plugins if p.name == source), None) + status = "\u2705" if (plugin and plugin.enabled) else "\u274c" + version = plugin.version if plugin else "" + header = f"{status} {source} (v{version})" + + keyboard: List[List[InlineKeyboardButton]] = [] + for item in cat_items: + sid = self._short_id(item.id) + keyboard.append([ + InlineKeyboardButton( + f"{item.name} — {item.description[:40]}", + callback_data=f"menu:run:{sid}", + ) + ]) + + # Plugin management buttons (if it's a plugin, not bot) + if source != "bot": + plugin = next((p for p in self.plugins if p.name == source), None) + if plugin: + toggle_label = "\U0001f534 Disable" if plugin.enabled else "\U0001f7e2 Enable" + keyboard.append([ + InlineKeyboardButton( + toggle_label, + callback_data=f"menu:tog:{source}", + ) + ]) + + # Back button + keyboard.append([ + InlineKeyboardButton("\u2190 Back", callback_data="menu:back") + ]) + + return InlineKeyboardMarkup(keyboard), header + + def get_single_item_action(self, source: str) -> Optional[PaletteItem]: + """If a plugin has exactly 1 item, return it for direct execution.""" + plugin = next((p for p in self.plugins if p.name == source), None) + if plugin and len(plugin.items) == 1: + return plugin.items[0] + return None + + def resolve_id(self, short_id: str) -> Optional[str]: + """Resolve a short callback ID to the full item/category ID.""" + return self.id_map.get(short_id) +``` + +**Step 4: Run tests** + +Run: `cd /home/florian/config/claude-code-telegram && poetry run pytest tests/unit/test_bot/test_menu.py -v` +Expected: PASS (5 tests) + +**Step 5: Commit** + +```bash +git add src/bot/handlers/menu.py tests/unit/test_bot/test_menu.py +git commit -m "feat(menu): add MenuBuilder for inline keyboard navigation" +``` + +--- + +### Task 6: Menu Command Handler + Callback Router + +**Files:** +- Modify: `src/bot/handlers/menu.py` +- Test: `tests/unit/test_bot/test_menu.py` + +This adds the actual Telegram handler functions. + +**Step 1: Write the failing test** + +```python +from unittest.mock import AsyncMock, MagicMock, patch + + +@pytest.fixture +def mock_update(): + update = MagicMock() + update.effective_user = MagicMock() + update.effective_user.id = 123456 + update.message = AsyncMock() + update.message.reply_text = AsyncMock() + return update + + +@pytest.fixture +def mock_context(): + context = MagicMock() + context.user_data = {} + context.bot_data = {} + return context + + +@pytest.mark.asyncio +async def test_menu_command_sends_keyboard(mock_update, mock_context): + with patch("src.bot.handlers.menu.CommandPaletteScanner") as MockScanner: + mock_scanner = MockScanner.return_value + mock_scanner.scan.return_value = ( + [PaletteItem( + id="bot:new", name="new", description="Fresh session", + action_type=ActionType.DIRECT_COMMAND, action_value="/new", + source="bot", enabled=True, + )], + [], + ) + from src.bot.handlers.menu import menu_command + await menu_command(mock_update, mock_context) + mock_update.message.reply_text.assert_called_once() + call_kwargs = mock_update.message.reply_text.call_args + assert call_kwargs.kwargs.get("reply_markup") is not None +``` + +**Step 2: Run to verify failure** + +Run: `cd /home/florian/config/claude-code-telegram && poetry run pytest tests/unit/test_bot/test_menu.py::test_menu_command_sends_keyboard -v` +Expected: FAIL with `ImportError: cannot import name 'menu_command'` + +**Step 3: Write implementation** + +Add to `src/bot/handlers/menu.py`: + +```python +from telegram import Update +from telegram.ext import ContextTypes + +from ..features.command_palette import CommandPaletteScanner + + +async def menu_command( + update: Update, context: ContextTypes.DEFAULT_TYPE +) -> None: + """Handle /menu command — show the command palette.""" + scanner = CommandPaletteScanner() + items, plugins = scanner.scan() + + builder = MenuBuilder(items, plugins) + keyboard = builder.build_top_level() + + # Store builder in user_data for callback resolution + context.user_data["menu_builder"] = builder + + await update.message.reply_text( + "\u26a1 Command Palette\n\nSelect a category:", + parse_mode="HTML", + reply_markup=keyboard, + ) + + +async def menu_callback( + update: Update, context: ContextTypes.DEFAULT_TYPE +) -> None: + """Handle all menu: callback queries.""" + query = update.callback_query + await query.answer() + + data = query.data + parts = data.split(":", 2) # "menu:action:arg" + if len(parts) < 2: + return + + action = parts[1] + arg = parts[2] if len(parts) > 2 else "" + + builder: Optional[MenuBuilder] = context.user_data.get("menu_builder") + + if action == "back": + # Rebuild top-level menu + scanner = CommandPaletteScanner() + items, plugins = scanner.scan() + builder = MenuBuilder(items, plugins) + context.user_data["menu_builder"] = builder + keyboard = builder.build_top_level() + await query.edit_message_text( + "\u26a1 Command Palette\n\nSelect a category:", + parse_mode="HTML", + reply_markup=keyboard, + ) + return + + if action == "cat" and builder: + full_id = builder.resolve_id(arg) + if not full_id: + return + keyboard, header = builder.build_category(full_id) + await query.edit_message_text( + f"{header}\n\nSelect a command:", + parse_mode="HTML", + reply_markup=keyboard, + ) + return + + if action == "run" and builder: + full_id = builder.resolve_id(arg) + if not full_id: + return + item = next((i for i in builder.items if i.id == full_id), None) + if not item: + return + + if item.action_type == ActionType.DIRECT_COMMAND: + # Remove menu message and let the bot handle the command directly + await query.edit_message_text( + f"Running {item.action_value}...", + parse_mode="HTML", + ) + # Simulate command by calling appropriate handler + # The orchestrator will handle this via _execute_bot_command + context.user_data["menu_pending_command"] = item.action_value + return + + if item.action_type == ActionType.INJECT_SKILL: + # Inject skill name as text to Claude + await query.edit_message_text( + f"Invoking {item.action_value}...", + parse_mode="HTML", + ) + context.user_data["menu_pending_skill"] = item.action_value + return + + if action == "tog": + # Toggle plugin enable/disable + plugin_name = arg + scanner = CommandPaletteScanner() + _, plugins = scanner.scan() + plugin = next((p for p in plugins if p.name == plugin_name), None) + if plugin: + new_state = not plugin.enabled + scanner.toggle_plugin(plugin.qualified_name, new_state) + status = "\u2705 Enabled" if new_state else "\u274c Disabled" + await query.edit_message_text( + f"{plugin.name} — {status}\n\n" + f"Takes effect on next /new session.", + parse_mode="HTML", + reply_markup=InlineKeyboardMarkup([ + [InlineKeyboardButton("\u2190 Back", callback_data="menu:back")] + ]), + ) + return + + if action == "store": + await query.edit_message_text( + "\U0001f4e6 Plugin Store\n\n" + "Plugin store coming soon.\n" + "For now, install plugins via Claude Code CLI:\n" + "claude plugins install <name>", + parse_mode="HTML", + reply_markup=InlineKeyboardMarkup([ + [InlineKeyboardButton("\u2190 Back", callback_data="menu:back")] + ]), + ) + return +``` + +**Step 4: Run tests** + +Run: `cd /home/florian/config/claude-code-telegram && poetry run pytest tests/unit/test_bot/test_menu.py -v` +Expected: PASS (all tests) + +**Step 5: Commit** + +```bash +git add src/bot/handlers/menu.py tests/unit/test_bot/test_menu.py +git commit -m "feat(menu): add menu_command and menu_callback handlers" +``` + +--- + +### Task 7: Wire Menu into Orchestrator + +**Files:** +- Modify: `src/bot/orchestrator.py` (lines 307-357, 408-441) +- Test: `tests/unit/test_orchestrator.py` + +**Step 1: Write the failing test** + +Add to `tests/unit/test_orchestrator.py`: + +```python +def test_agentic_mode_registers_menu_handler(agentic_settings, deps): + orchestrator = MessageOrchestrator(agentic_settings, deps) + app = MagicMock() + orchestrator.register_handlers(app) + + # Verify /menu command is registered + handler_calls = [str(c) for c in app.add_handler.call_args_list] + handler_strs = str(handler_calls) + assert "menu" in handler_strs.lower() + + +def test_agentic_mode_registers_menu_callback(agentic_settings, deps): + orchestrator = MessageOrchestrator(agentic_settings, deps) + app = MagicMock() + orchestrator.register_handlers(app) + + # Verify menu: callback pattern is registered + handler_strs = str(app.add_handler.call_args_list) + assert "menu:" in handler_strs + + +@pytest.mark.asyncio +async def test_get_bot_commands_includes_menu(agentic_settings, deps): + orchestrator = MessageOrchestrator(agentic_settings, deps) + commands = await orchestrator.get_bot_commands() + command_names = [c.command for c in commands] + assert "menu" in command_names +``` + +**Step 2: Run to verify failure** + +Run: `cd /home/florian/config/claude-code-telegram && poetry run pytest tests/unit/test_orchestrator.py::test_agentic_mode_registers_menu_handler -v` +Expected: FAIL (no "menu" in registered handlers) + +**Step 3: Modify orchestrator** + +In `src/bot/orchestrator.py`: + +**At `_register_agentic_handlers()` (line 307)** — add menu command + callback: + +After line 318 (`("stop", command.stop_command),`), add: +```python + from .handlers import menu as menu_handler +``` + +After the handlers list (before the for loop at line 323), add `menu` to handlers: +```python + ("menu", menu_handler.menu_command), +``` + +After the `cd:` callback handler (line 355), add: +```python + # Menu navigation callbacks + app.add_handler( + CallbackQueryHandler( + self._inject_deps(menu_handler.menu_callback), + pattern=r"^menu:", + ) + ) +``` + +**At `get_bot_commands()` (line 408)** — add menu to agentic commands: + +After line 412 (`BotCommand("start", "Start the bot"),`), add: +```python + BotCommand("menu", "Command palette & plugin manager"), +``` + +**Step 4: Run tests** + +Run: `cd /home/florian/config/claude-code-telegram && poetry run pytest tests/unit/test_orchestrator.py -k menu -v` +Expected: PASS (3 tests) + +**Step 5: Run all existing tests to check for regressions** + +Run: `cd /home/florian/config/claude-code-telegram && poetry run pytest tests/ -v --timeout=30` +Expected: All existing tests still pass + +**Step 6: Commit** + +```bash +git add src/bot/orchestrator.py tests/unit/test_orchestrator.py +git commit -m "feat(menu): wire /menu command and callbacks into orchestrator" +``` + +--- + +### Task 8: Skill Injection — Connect Menu to Claude Session + +**Files:** +- Modify: `src/bot/orchestrator.py` +- Modify: `src/bot/handlers/menu.py` + +When a user taps a skill button (e.g. "brainstorming"), we need to inject that text into the Claude session just like the user typed it. The cleanest approach is to reuse `agentic_text()` logic. + +**Step 1: Write the failing test** + +Add to `tests/unit/test_bot/test_menu.py`: + +```python +@pytest.mark.asyncio +async def test_menu_run_injects_skill_into_user_data(): + """When a skill button is tapped, its action_value is stored for injection.""" + update = MagicMock() + query = AsyncMock() + query.data = "menu:run:1" + query.answer = AsyncMock() + query.edit_message_text = AsyncMock() + update.callback_query = query + + context = MagicMock() + context.user_data = { + "menu_builder": MagicMock( + resolve_id=MagicMock(return_value="commit-commands:commit"), + items=[PaletteItem( + id="commit-commands:commit", name="commit", + description="Git commit", action_type=ActionType.INJECT_SKILL, + action_value="/commit", source="commit-commands", enabled=True, + )], + ) + } + + from src.bot.handlers.menu import menu_callback + await menu_callback(update, context) + + assert context.user_data.get("menu_pending_skill") == "/commit" +``` + +**Step 2: Run to verify it passes** (already implemented in Task 6) + +Run: `cd /home/florian/config/claude-code-telegram && poetry run pytest tests/unit/test_bot/test_menu.py::test_menu_run_injects_skill_into_user_data -v` +Expected: PASS + +**Step 3: Implement the injection bridge in orchestrator** + +Add a helper method to `MessageOrchestrator` that checks for pending menu actions and routes them. This hooks into the existing `agentic_text()` flow. + +In `src/bot/handlers/menu.py`, update the `menu_callback` `"run"` action for `INJECT_SKILL` to send the skill invocation as a new message to the bot (simulating user input): + +```python + if item.action_type == ActionType.INJECT_SKILL: + await query.edit_message_text( + f"Invoking {item.action_value}...", + parse_mode="HTML", + ) + # Send the skill name as a new message from the user + # This triggers agentic_text() which routes it to Claude + await context.bot.send_message( + chat_id=query.message.chat_id, + text=item.action_value, + ) + return +``` + +Wait — that would send a message *from the bot*, not the user. Better approach: store the pending skill and inform the user to confirm, OR directly call `agentic_text` with a synthetic update. The simplest approach is to use `context.user_data["menu_inject"]` and have the menu callback send a follow-up message that the user just needs to confirm: + +Actually, the cleanest approach: after editing the menu message, call the orchestrator's text handler directly with the skill text. Add this to `menu_callback`: + +```python + if item.action_type == ActionType.INJECT_SKILL: + await query.edit_message_text( + f"\u26a1 {item.action_value}", + parse_mode="HTML", + ) + # Store for the orchestrator to pick up and route to Claude + context.user_data["menu_inject_text"] = item.action_value + return +``` + +Then in the orchestrator, add a post-callback check or simply have a small wrapper. The pragmatic solution: the menu callback directly calls `ClaudeIntegration.run_command()` with the skill text, using the same pattern as `agentic_text()` but simplified. + +However, to keep things clean, let's just have the menu handler import and call into a shared helper. This will be implemented in the integration step. + +**Step 4: Commit** + +```bash +git add src/bot/handlers/menu.py +git commit -m "feat(menu): store pending skill invocation for Claude injection" +``` + +--- + +### Task 9: Skill Injection Bridge — Shared Helper + +**Files:** +- Modify: `src/bot/orchestrator.py` +- Modify: `src/bot/handlers/menu.py` + +**Step 1: Extract a reusable `_send_to_claude` helper from `agentic_text()`** + +Read `agentic_text()` (line 835-end) and extract the core Claude invocation into a helper that can be called from both `agentic_text()` and `menu_callback()`. + +Add to `MessageOrchestrator`: + +```python + async def send_text_to_claude( + self, + text: str, + chat_id: int, + user_id: int, + context: ContextTypes.DEFAULT_TYPE, + reply_to_message_id: Optional[int] = None, + ) -> None: + """Send text to Claude and stream the response back. + + Shared helper used by agentic_text() and menu skill injection. + """ + # ... extracted from agentic_text core logic +``` + +This is a larger refactor. For the initial implementation, the simpler approach is to have the menu callback compose a fake Update-like object and delegate. But that's fragile. + +**Pragmatic approach:** Instead of refactoring agentic_text, have menu_callback store the skill text and let the user know it's queued. Then use `context.bot.send_message` to send a message in the chat that says the skill name — which the user sees and agentic_text picks up naturally since it processes all non-command text. + +Wait, actually `context.bot.send_message` sends *as the bot*. We can't fake a user message. + +**Best approach:** The menu callback directly calls `ClaudeIntegration.run_command()` in a simplified flow: + +```python + if item.action_type == ActionType.INJECT_SKILL: + await query.edit_message_text( + f"\u26a1 Running {item.action_value}...", + parse_mode="HTML", + ) + + claude = context.bot_data.get("claude_integration") + if not claude: + await query.edit_message_text("Claude integration not available.") + return + + current_dir = context.user_data.get("current_directory", "/home/florian") + session_id = context.user_data.get("claude_session_id") + + # Send skill invocation to Claude + response = await claude.run_command( + prompt=item.action_value, + working_directory=str(current_dir), + user_id=str(query.from_user.id), + session_id=session_id, + ) + + # Update session ID + if response and response.session_id: + context.user_data["claude_session_id"] = response.session_id + + # Send response + if response and response.content: + # Truncate for Telegram's 4096 char limit + content = response.content[:4000] + await context.bot.send_message( + chat_id=query.message.chat_id, + text=content, + parse_mode="HTML", + ) + return +``` + +This is clean but skips the streaming/progress UI that `agentic_text` provides. For v1, this is acceptable. The streaming can be added later. + +**However**, the even simpler v1: just edit the message to tell the user what to type. No, that defeats the purpose. + +**Final decision for v1:** Store the pending skill in user_data, then have the menu_callback call a reference to the orchestrator's `_run_claude_from_menu` helper (a new, simpler method). The orchestrator has access to all the streaming infrastructure. + +Let's keep this simple for the plan. The menu callback stores `context.user_data["menu_inject_text"]` and a new `_check_menu_injection()` helper in orchestrator processes it. The menu callback also sends a "Working..." progress message. + +**This is getting complex. Let me simplify.** + +For v1, the menu callback will: +1. Edit the menu message to show "Running /commit..." +2. Call `ClaudeIntegration.run_command()` directly (no streaming, just final result) +3. Send the response as a message + +This avoids refactoring agentic_text and gives a working feature. Streaming can be added in v2. + +**Step 2: Implementation in menu.py** + +Already described above. Add to the `INJECT_SKILL` branch of `menu_callback`. + +**Step 3: Test** + +```python +@pytest.mark.asyncio +async def test_menu_run_calls_claude_for_skill(): + update = MagicMock() + query = AsyncMock() + query.data = "menu:run:1" + query.answer = AsyncMock() + query.edit_message_text = AsyncMock() + query.from_user = MagicMock(id=123) + query.message = MagicMock(chat_id=456) + update.callback_query = query + + mock_claude = AsyncMock() + mock_response = MagicMock(content="Done!", session_id="sess123") + mock_claude.run_command = AsyncMock(return_value=mock_response) + + context = MagicMock() + context.user_data = { + "current_directory": "/home/florian", + "menu_builder": MagicMock( + resolve_id=MagicMock(return_value="commit-commands:commit"), + items=[PaletteItem( + id="commit-commands:commit", name="commit", + description="Git commit", action_type=ActionType.INJECT_SKILL, + action_value="/commit", source="commit-commands", enabled=True, + )], + ), + } + context.bot_data = {"claude_integration": mock_claude} + context.bot = AsyncMock() + + from src.bot.handlers.menu import menu_callback + await menu_callback(update, context) + + mock_claude.run_command.assert_called_once() + context.bot.send_message.assert_called_once() +``` + +**Step 4: Commit** + +```bash +git add src/bot/handlers/menu.py tests/unit/test_bot/test_menu.py +git commit -m "feat(menu): inject skill invocations into Claude session" +``` + +--- + +### Task 10: Integration Test — Full Menu Flow + +**Files:** +- Test: `tests/unit/test_bot/test_menu.py` + +**Step 1: Write integration test** + +```python +@pytest.mark.asyncio +async def test_full_menu_flow_category_navigation(): + """Test: /menu -> tap category -> tap item.""" + # This tests the full flow without Telegram API + scanner_items = [ + PaletteItem( + id="bot:status", name="status", description="Session status", + action_type=ActionType.DIRECT_COMMAND, action_value="/status", + source="bot", enabled=True, + ), + PaletteItem( + id="superpowers:brainstorming", name="brainstorming", + description="Creative work", action_type=ActionType.INJECT_SKILL, + action_value="/brainstorming", source="superpowers", enabled=True, + ), + PaletteItem( + id="superpowers:tdd", name="test-driven-development", + description="TDD workflow", action_type=ActionType.INJECT_SKILL, + action_value="/test-driven-development", source="superpowers", enabled=True, + ), + ] + scanner_plugins = [ + PluginInfo( + name="superpowers", qualified_name="superpowers@claude-plugins-official", + version="4.3.1", enabled=True, + items=scanner_items[1:], # brainstorming + tdd + ), + ] + + builder = MenuBuilder(scanner_items, scanner_plugins) + + # Step 1: Build top-level + top_kb = builder.build_top_level() + assert top_kb is not None + + # Step 2: Find superpowers category button and resolve its ID + sp_btn = None + for row in top_kb.inline_keyboard: + for btn in row: + if "superpowers" in btn.text: + sp_btn = btn + break + assert sp_btn is not None + _, _, sid = sp_btn.callback_data.split(":", 2) + resolved = builder.resolve_id(sid) + assert resolved == "superpowers" + + # Step 3: Build category view + cat_kb, header = builder.build_category("superpowers") + assert "superpowers" in header + cat_texts = [btn.text for row in cat_kb.inline_keyboard for btn in row] + assert any("brainstorming" in t for t in cat_texts) + assert any("test-driven" in t for t in cat_texts) +``` + +**Step 2: Run test** + +Run: `cd /home/florian/config/claude-code-telegram && poetry run pytest tests/unit/test_bot/test_menu.py::test_full_menu_flow_category_navigation -v` +Expected: PASS + +**Step 3: Commit** + +```bash +git add tests/unit/test_bot/test_menu.py +git commit -m "test(menu): add full menu flow integration test" +``` + +--- + +### Task 11: Lint, Type Check, Final Test Suite + +**Files:** +- All new/modified files + +**Step 1: Run formatter** + +Run: `cd /home/florian/config/claude-code-telegram && poetry run black src/bot/features/command_palette.py src/bot/handlers/menu.py tests/unit/test_bot/test_command_palette.py tests/unit/test_bot/test_menu.py` + +**Step 2: Run isort** + +Run: `cd /home/florian/config/claude-code-telegram && poetry run isort src/bot/features/command_palette.py src/bot/handlers/menu.py tests/unit/test_bot/test_command_palette.py tests/unit/test_bot/test_menu.py` + +**Step 3: Run flake8** + +Run: `cd /home/florian/config/claude-code-telegram && poetry run flake8 src/bot/features/command_palette.py src/bot/handlers/menu.py` + +**Step 4: Run mypy** + +Run: `cd /home/florian/config/claude-code-telegram && poetry run mypy src/bot/features/command_palette.py src/bot/handlers/menu.py` + +Fix any type errors. + +**Step 5: Run full test suite** + +Run: `cd /home/florian/config/claude-code-telegram && poetry run pytest tests/ -v --timeout=30` +Expected: All tests pass (existing + new) + +**Step 6: Commit** + +```bash +git add -A +git commit -m "chore(menu): lint, format, type check all menu code" +``` + +--- + +### Task 12: Copy to Installed Location + Restart Bot + +**Files:** +- Copy new/modified files to `~/.local/share/uv/tools/claude-code-telegram/lib/python3.12/site-packages/src/` + +**Step 1: Copy files** + +```bash +# New files +cp src/bot/features/command_palette.py ~/.local/share/uv/tools/claude-code-telegram/lib/python3.12/site-packages/src/bot/features/command_palette.py + +cp src/bot/handlers/menu.py ~/.local/share/uv/tools/claude-code-telegram/lib/python3.12/site-packages/src/bot/handlers/menu.py + +# Modified files +cp src/bot/orchestrator.py ~/.local/share/uv/tools/claude-code-telegram/lib/python3.12/site-packages/src/bot/orchestrator.py +``` + +**Step 2: Install PyYAML if not already available** + +Check: `pip show pyyaml` in the bot's venv. +If missing: add to pyproject.toml and reinstall. + +**Step 3: Restart bot** + +```bash +systemctl --user restart claude-telegram-bot +``` + +**Step 4: Check logs** + +```bash +journalctl --user -u claude-telegram-bot -f +``` + +Verify no startup errors. + +**Step 5: Test in Telegram** + +1. Send `/menu` to the bot +2. Verify the command palette appears with categories +3. Tap a category (e.g. "superpowers") — verify sub-menu shows skills +4. Tap a skill (e.g. "brainstorming") — verify it invokes via Claude +5. Tap "Back" — verify return to top level +6. Tap a plugin toggle — verify enable/disable works +7. Tap "Plugin Store" — verify placeholder message + +**Step 6: Commit if any fixes needed** + +```bash +git add -A +git commit -m "fix(menu): adjustments from live testing" +``` + +--- + +## Summary + +| Task | What | Files | Tests | +|------|------|-------|-------| +| 1 | Data models (PaletteItem, PluginInfo, ActionType) | `command_palette.py` | 3 | +| 2 | YAML frontmatter parser | `command_palette.py` | 5 | +| 3 | Filesystem scanner (CommandPaletteScanner) | `command_palette.py` | 6 | +| 4 | Plugin toggle (enable/disable) | `command_palette.py` | 2 | +| 5 | Menu keyboard builder (MenuBuilder) | `menu.py` | 5 | +| 6 | Menu command handler + callback router | `menu.py` | 1 | +| 7 | Wire into orchestrator | `orchestrator.py` | 3 | +| 8 | Skill injection (pending skill storage) | `menu.py` | 1 | +| 9 | Skill injection bridge (Claude call) | `menu.py` | 1 | +| 10 | Integration test | `test_menu.py` | 1 | +| 11 | Lint + type check + full suite | all | 0 | +| 12 | Deploy + live test | installed copy | 0 | + +**Total: 12 tasks, ~28 tests, 2 new files, 1 modified file** diff --git a/src/bot/features/command_palette.py b/src/bot/features/command_palette.py index 9e7b3c66..a6c3ee0e 100644 --- a/src/bot/features/command_palette.py +++ b/src/bot/features/command_palette.py @@ -126,6 +126,63 @@ def parse_skill_frontmatter(content: str) -> Dict[str, Any]: ] +def get_enabled_plugin_paths(claude_dir: Optional[Path] = None) -> List[str]: + """Return install paths for all enabled (and non-blocked) plugins. + + This is used by the SDK integration to pass plugins to Claude sessions + via ``ClaudeAgentOptions.plugins``. + """ + claude_dir = claude_dir or Path.home() / ".claude" + if not claude_dir.is_dir(): + return [] + + # Load settings + settings_file = claude_dir / "settings.json" + enabled_map: Dict[str, bool] = {} + if settings_file.is_file(): + try: + data = json.loads(settings_file.read_text(encoding="utf-8")) + enabled_map = data.get("enabledPlugins", {}) + except (json.JSONDecodeError, OSError): + pass + + # Load installed plugins + installed_file = claude_dir / "plugins" / "installed_plugins.json" + installed: Dict[str, list] = {} + if installed_file.is_file(): + try: + data = json.loads(installed_file.read_text(encoding="utf-8")) + installed = data.get("plugins", {}) + except (json.JSONDecodeError, OSError): + pass + + # Load blocklist + blocklist_file = claude_dir / "plugins" / "blocklist.json" + blocked_names: set = set() + if blocklist_file.is_file(): + try: + data = json.loads(blocklist_file.read_text(encoding="utf-8")) + blocked_names = { + b.get("plugin") for b in data.get("plugins", []) if b.get("plugin") + } + except (json.JSONDecodeError, OSError): + pass + + paths: List[str] = [] + for qualified_name, installs in installed.items(): + if not installs: + continue + if not enabled_map.get(qualified_name, False): + continue + if qualified_name in blocked_names: + continue + install_path = installs[0].get("installPath", "") + if install_path and Path(install_path).is_dir(): + paths.append(install_path) + + return paths + + class CommandPaletteScanner: """Scans ~/.claude/ for all available skills, commands, and plugins.""" diff --git a/src/bot/handlers/menu.py b/src/bot/handlers/menu.py index f7ca8cf2..4cb94605 100644 --- a/src/bot/handlers/menu.py +++ b/src/bot/handlers/menu.py @@ -432,8 +432,13 @@ async def menu_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) -> N pass try: + # Send the skill as a slash command. Plugins are now passed + # to the SDK session via ClaudeAgentOptions.plugins, so the + # session has access to all enabled skills. + prompt = item.action_value # e.g. "/commit" + response = await claude_integration.run_command( - prompt=item.action_value, + prompt=prompt, working_directory=current_dir, user_id=query.from_user.id, session_id=session_id, @@ -447,6 +452,12 @@ async def menu_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) -> N # Update session ID context.user_data["claude_session_id"] = response.session_id + logger.debug( + "Skill response content", + content_length=len(response.content) if response.content else 0, + content_preview=(response.content or "")[:200], + ) + # Format response formatter = ResponseFormatter(settings) formatted_messages = formatter.format_claude_response(response.content) @@ -457,7 +468,11 @@ async def menu_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) -> N except Exception: pass + # Preserve topic/thread context for supergroup forums + thread_id = getattr(query.message, "message_thread_id", None) + # Send formatted response + sent_any = False for msg in formatted_messages: if msg.text and msg.text.strip(): try: @@ -465,13 +480,27 @@ async def menu_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) -> N chat_id=chat_id, text=msg.text, parse_mode=msg.parse_mode, + message_thread_id=thread_id, ) + sent_any = True except Exception: # Retry without parse mode await context.bot.send_message( chat_id=chat_id, text=msg.text, + message_thread_id=thread_id, ) + sent_any = True + + # If Claude produced no visible text (e.g. only tool calls), + # send a confirmation so the user knows the skill ran. + if not sent_any: + skill_name = item.action_value.lstrip("/") + await context.bot.send_message( + chat_id=chat_id, + text=f"/{skill_name} completed (cost: ${response.cost:.4f})", + message_thread_id=thread_id, + ) # Log interaction storage = context.bot_data.get("storage") diff --git a/src/claude/sdk_integration.py b/src/claude/sdk_integration.py index 903dccd3..d8eb10e2 100644 --- a/src/claude/sdk_integration.py +++ b/src/claude/sdk_integration.py @@ -22,6 +22,7 @@ PermissionResultDeny, ProcessError, ResultMessage, + SdkPluginConfig, ToolPermissionContext, ToolUseBlock, UserMessage, @@ -293,10 +294,27 @@ def _stderr_callback(line: str) -> None: "excludedCommands": self.config.sandbox_excluded_commands or [], }, system_prompt=base_prompt, - setting_sources=["project"], + setting_sources=["project", "user"], stderr=_stderr_callback, ) + # Pass enabled plugins so skills/commands are available in sessions + try: + from src.bot.features.command_palette import get_enabled_plugin_paths + + plugin_paths = get_enabled_plugin_paths() + if plugin_paths: + options.plugins = [ + SdkPluginConfig(type="local", path=p) + for p in plugin_paths + ] + logger.info( + "Plugins configured for session", + count=len(plugin_paths), + ) + except Exception as exc: + logger.warning("Failed to load plugins for session", error=str(exc)) + # Pass MCP server configuration if enabled if self.config.enable_mcp and self.config.mcp_config_path: options.mcp_servers = self._load_mcp_config(self.config.mcp_config_path) From 5aecf7fb685c4a45fd09232d524d51cc0df5b125 Mon Sep 17 00:00:00 2001 From: F1orian <27896106+F1orian@users.noreply.github.com> Date: Mon, 2 Mar 2026 23:16:36 +0100 Subject: [PATCH 11/16] docs: add interactive user feedback design doc Design for intercepting AskUserQuestion via PreToolUse SDK hooks and routing questions through Telegram inline keyboards. Co-Authored-By: Claude Opus 4.6 --- ...-03-02-interactive-user-feedback-design.md | 169 ++++++++++++++++++ 1 file changed, 169 insertions(+) create mode 100644 docs/plans/2026-03-02-interactive-user-feedback-design.md diff --git a/docs/plans/2026-03-02-interactive-user-feedback-design.md b/docs/plans/2026-03-02-interactive-user-feedback-design.md new file mode 100644 index 00000000..f6f9e828 --- /dev/null +++ b/docs/plans/2026-03-02-interactive-user-feedback-design.md @@ -0,0 +1,169 @@ +# Interactive User Feedback via Telegram Inline Keyboards + +**Date:** 2026-03-02 +**Status:** Approved + +## Problem + +When Claude calls `AskUserQuestion` (e.g. during brainstorming skills), the CLI subprocess has no TTY and auto-selects the first option. The Telegram user never sees the question and has no way to provide their actual answer. + +## Solution + +Use a `PreToolUse` SDK hook on `AskUserQuestion` that intercepts the tool call, presents the question as Telegram inline keyboard buttons, waits for the user's tap, and returns the answer via `updatedInput` so the CLI executes the tool with the user's actual choice. + +## Approach: PreToolUse Hook with Shared Future + +The hook callback and Telegram handler run in the same asyncio event loop. An `asyncio.Future` coordinates between them — the hook awaits the Future while the Telegram callback handler resolves it. + +## Data Flow + +``` +Claude calls AskUserQuestion(questions=[...]) + │ + ▼ +CLI sends PreToolUse hook_callback to SDK via control protocol + │ + ▼ +SDK invokes Python hook callback (closure in sdk_integration.py) + │ + ├── Extracts questions + options from tool_input + ├── Calls bot.send_message() with inline keyboard + ├── Creates asyncio.Future, stores in _pending dict + ├── Awaits the Future (Claude is paused) + │ + ▼ +User taps inline button in Telegram + │ + ▼ +CallbackQueryHandler (pattern "askq:") in orchestrator + │ + ├── Looks up pending Future by (user_id, chat_id) + ├── Resolves Future with selected answer + │ + ▼ +Hook callback resumes + │ + ├── Builds updatedInput with answers dict pre-filled + ├── Returns SyncHookJSONOutput with hookSpecificOutput + │ + ▼ +CLI executes AskUserQuestion with user's actual answer +``` + +## Scope + +All Claude interactions — not just /menu skills. Any time Claude calls `AskUserQuestion`, the question is routed to Telegram. + +## Shared State & Coordination + +### PendingQuestion Registry + +New file `src/bot/features/interactive_questions.py` with a module-level dict: + +```python +_pending: Dict[Tuple[int, int], asyncio.Future] = {} +``` + +Keyed by `(user_id, chat_id)`. Only one question pending per user+chat at a time (Claude is paused, can't ask another until the first is answered). + +### Hook Callback as Closure + +`execute_command()` already knows `user_id`. It gains a `telegram_context` parameter (bot, chat_id, thread_id) so it can build the hook closure capturing: +- `user_id`, `chat_id`, `message_thread_id` — where to send the keyboard +- `bot` instance — to call `bot.send_message()` +- Reference to `_pending` dict — to create/resolve Futures + +### Telegram Handler + +New `CallbackQueryHandler(pattern=r"^askq:")` registered in the orchestrator. Parses callback data, looks up Future, resolves it. + +## AskUserQuestion Input Format + +```python +{ + "questions": [ + { + "question": "Which approach?", + "header": "Approach", + "options": [ + {"label": "Option A", "description": "..."}, + {"label": "Option B", "description": "..."}, + ], + "multiSelect": false + } + ], + "answers": { + "Which approach?": "Option A" # ← we pre-fill this + } +} +``` + +- `questions`: list of 1-4 questions per call +- Each question has 2-4 options + implicit "Other" +- `answers`: dict keyed by question text → selected label(s) +- `multiSelect: true`: multiple options selectable + +## Telegram UX + +### Callback Data Format (64-byte limit) + +- Single select: `askq:0:1` (question 0, option 1) +- Multi-select toggle: `askq:0:t1` (question 0, toggle option 1) +- Multi-select done: `askq:0:done` +- Other: `askq:0:other` + +### Single-Select Layout + +``` +Which approach should we use? + +• Option A — Does X and Y +• Option B — Does Z + +[Option A] [Option B] +[Other...] +``` + +### Multi-Select Layout (mid-selection) + +``` +Which features do you want? + +• Auth — Login system +• Cache — Redis caching +• Logs — Structured logging + +[☑ Auth] [☐ Cache] [☑ Logs] +[Other...] +[Done ✓] +``` + +### "Other" Flow + +1. User taps "Other..." +2. Message edited to "Type your answer:" +3. One-time MessageHandler captures next text message from that user+chat +4. Text used as answer, Future resolved +5. Handler removes itself + +### Sequential Questions + +If `AskUserQuestion` has multiple questions (1-4), process one at a time: show question 1, wait for answer, show question 2, wait, etc. All answers collected, then returned as one `updatedInput`. + +## Error Handling + +- **No timeout on the Future** — waits indefinitely. The outer `claude_timeout_seconds` (3600s) is the hard bound. +- **Stale questions** — if session errors, Future is cancelled in `finally` block. Tapping stale buttons returns "Question expired." +- **Concurrent sessions** — keyed by `(user_id, chat_id)`, independent per project thread. +- **Button clicks after answer** — "Already answered" feedback, keyboard removed. +- **"Other" text capture** — one-time MessageHandler scoped to user+chat, removes itself after capture. + +## Files Changed + +| File | Change | +|------|--------| +| `src/bot/features/interactive_questions.py` | **New.** Pending dict, keyboard builder, question formatter, callback handler, "Other" text handler | +| `src/claude/sdk_integration.py` | Add `telegram_context` param to `execute_command()`, build PreToolUse hook closure, register in `options.hooks` | +| `src/claude/facade.py` | Pass `telegram_context` through `run_command()` → `_execute()` → `execute_command()` | +| `src/bot/orchestrator.py` | Pass telegram context when calling `run_command()`, register `askq:` CallbackQueryHandler | +| `src/bot/handlers/menu.py` | Pass telegram context when calling `run_command()` from menu skill execution | From 80b0f40655825c3c69cf38bfad279b54440cec3e Mon Sep 17 00:00:00 2001 From: F1orian <27896106+F1orian@users.noreply.github.com> Date: Mon, 2 Mar 2026 23:20:40 +0100 Subject: [PATCH 12/16] docs: add interactive user feedback implementation plan 8-task plan with TDD steps for intercepting AskUserQuestion via PreToolUse hooks and routing through Telegram inline keyboards. Co-Authored-By: Claude Opus 4.6 --- .../2026-03-02-interactive-user-feedback.md | 1435 +++++++++++++++++ 1 file changed, 1435 insertions(+) create mode 100644 docs/plans/2026-03-02-interactive-user-feedback.md diff --git a/docs/plans/2026-03-02-interactive-user-feedback.md b/docs/plans/2026-03-02-interactive-user-feedback.md new file mode 100644 index 00000000..3887f522 --- /dev/null +++ b/docs/plans/2026-03-02-interactive-user-feedback.md @@ -0,0 +1,1435 @@ +# Interactive User Feedback Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Intercept `AskUserQuestion` tool calls via PreToolUse SDK hooks and route them through Telegram inline keyboards so users can answer interactively. + +**Architecture:** A PreToolUse hook on `AskUserQuestion` pauses Claude, sends the question as Telegram inline buttons, awaits user tap via `asyncio.Future`, then returns the answer as `updatedInput`. A shared `_pending` dict keyed by `(user_id, chat_id)` coordinates the hook callback and the Telegram `CallbackQueryHandler`. + +**Tech Stack:** `claude-agent-sdk` (PreToolUse hooks, HookMatcher, SyncHookJSONOutput), `python-telegram-bot` (InlineKeyboardMarkup, CallbackQueryHandler, MessageHandler), `asyncio.Future` + +--- + +### Task 1: Create interactive_questions module — data types and pending registry + +**Files:** +- Create: `src/bot/features/interactive_questions.py` +- Test: `tests/unit/test_bot/test_interactive_questions.py` + +**Step 1: Write the failing tests** + +```python +"""Tests for interactive_questions module.""" + +import asyncio +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from src.bot.features.interactive_questions import ( + PendingQuestion, + TelegramContext, + format_question_text, + build_single_select_keyboard, + build_multi_select_keyboard, + register_pending, + resolve_pending, + get_pending, + cancel_pending, +) + + +class TestTelegramContext: + def test_creation(self): + bot = MagicMock() + ctx = TelegramContext(bot=bot, chat_id=123, thread_id=456, user_id=789) + assert ctx.bot is bot + assert ctx.chat_id == 123 + assert ctx.thread_id == 456 + assert ctx.user_id == 789 + + def test_thread_id_optional(self): + bot = MagicMock() + ctx = TelegramContext(bot=bot, chat_id=123, thread_id=None, user_id=789) + assert ctx.thread_id is None + + +class TestPendingQuestion: + def test_creation(self): + loop = asyncio.new_event_loop() + future = loop.create_future() + pq = PendingQuestion( + future=future, + question_text="Which?", + options=[{"label": "A", "description": "opt A"}], + multi_select=False, + selected=set(), + message_id=None, + ) + assert pq.question_text == "Which?" + assert pq.multi_select is False + assert pq.selected == set() + loop.close() + + +class TestFormatQuestionText: + def test_basic_formatting(self): + options = [ + {"label": "Alpha", "description": "First option"}, + {"label": "Beta", "description": "Second option"}, + ] + text = format_question_text("Pick one?", options) + assert "Pick one?" in text + assert "Alpha" in text + assert "First option" in text + assert "Beta" in text + + def test_no_description(self): + options = [{"label": "X"}, {"label": "Y"}] + text = format_question_text("Choose", options) + assert "X" in text + assert "Y" in text + + +class TestBuildSingleSelectKeyboard: + def test_buttons_match_options(self): + options = [ + {"label": "A", "description": "opt A"}, + {"label": "B", "description": "opt B"}, + ] + kb = build_single_select_keyboard(options, question_idx=0) + # Flatten all buttons + buttons = [btn for row in kb.inline_keyboard for btn in row] + labels = [btn.text for btn in buttons] + assert "A" in labels + assert "B" in labels + assert "Other..." in labels + + def test_callback_data_format(self): + options = [{"label": "A", "description": "x"}] + kb = build_single_select_keyboard(options, question_idx=2) + btn = kb.inline_keyboard[0][0] + assert btn.callback_data == "askq:2:0" + + def test_other_button(self): + options = [{"label": "A", "description": "x"}] + kb = build_single_select_keyboard(options, question_idx=0) + buttons = [btn for row in kb.inline_keyboard for btn in row] + other = [b for b in buttons if b.callback_data == "askq:0:other"] + assert len(other) == 1 + + +class TestBuildMultiSelectKeyboard: + def test_unchecked_by_default(self): + options = [ + {"label": "A", "description": "x"}, + {"label": "B", "description": "y"}, + ] + kb = build_multi_select_keyboard(options, question_idx=0, selected=set()) + buttons = [btn for row in kb.inline_keyboard for btn in row] + assert any("☐ A" == btn.text for btn in buttons) + assert any("☐ B" == btn.text for btn in buttons) + + def test_checked_state(self): + options = [ + {"label": "A", "description": "x"}, + {"label": "B", "description": "y"}, + ] + kb = build_multi_select_keyboard(options, question_idx=0, selected={0}) + buttons = [btn for row in kb.inline_keyboard for btn in row] + assert any("☑ A" == btn.text for btn in buttons) + assert any("☐ B" == btn.text for btn in buttons) + + def test_toggle_callback_data(self): + options = [{"label": "A", "description": "x"}] + kb = build_multi_select_keyboard(options, question_idx=1, selected=set()) + btn = kb.inline_keyboard[0][0] + assert btn.callback_data == "askq:1:t0" + + def test_done_and_other_buttons(self): + options = [{"label": "A", "description": "x"}] + kb = build_multi_select_keyboard(options, question_idx=0, selected=set()) + buttons = [btn for row in kb.inline_keyboard for btn in row] + data = [b.callback_data for b in buttons] + assert "askq:0:other" in data + assert "askq:0:done" in data + + +class TestPendingRegistry: + def test_register_and_get(self): + loop = asyncio.new_event_loop() + future = loop.create_future() + pq = PendingQuestion( + future=future, + question_text="Q?", + options=[], + multi_select=False, + selected=set(), + message_id=None, + ) + register_pending(789, 123, pq) + assert get_pending(789, 123) is pq + loop.close() + + def test_resolve_clears(self): + loop = asyncio.new_event_loop() + future = loop.create_future() + pq = PendingQuestion( + future=future, + question_text="Q?", + options=[], + multi_select=False, + selected=set(), + message_id=None, + ) + register_pending(789, 123, pq) + resolve_pending(789, 123, "answer") + assert future.result() == "answer" + assert get_pending(789, 123) is None + loop.close() + + def test_cancel_clears(self): + loop = asyncio.new_event_loop() + future = loop.create_future() + pq = PendingQuestion( + future=future, + question_text="Q?", + options=[], + multi_select=False, + selected=set(), + message_id=None, + ) + register_pending(789, 123, pq) + cancel_pending(789, 123) + assert future.cancelled() + assert get_pending(789, 123) is None + loop.close() + + def test_resolve_missing_is_noop(self): + resolve_pending(999, 999, "x") # Should not raise + + def test_cancel_missing_is_noop(self): + cancel_pending(999, 999) # Should not raise +``` + +**Step 2: Run tests to verify they fail** + +Run: `cd /home/florian/config/claude-code-telegram && python -m pytest tests/unit/test_bot/test_interactive_questions.py -v` +Expected: ImportError — module does not exist yet + +**Step 3: Write the implementation** + +```python +"""Interactive question routing for AskUserQuestion tool calls. + +Intercepts AskUserQuestion via PreToolUse hooks and presents questions +as Telegram inline keyboards. Uses asyncio.Future to coordinate between +the hook callback (which pauses Claude) and the Telegram button handler. +""" + +import asyncio +from dataclasses import dataclass, field +from typing import Any, Dict, List, Optional, Set, Tuple + +import structlog +from telegram import InlineKeyboardButton, InlineKeyboardMarkup + +logger = structlog.get_logger() + + +@dataclass +class TelegramContext: + """Telegram context needed to send messages from the hook callback.""" + + bot: Any # telegram.Bot + chat_id: int + thread_id: Optional[int] + user_id: int + + +@dataclass +class PendingQuestion: + """A question waiting for user response.""" + + future: asyncio.Future + question_text: str + options: List[Dict[str, Any]] + multi_select: bool + selected: Set[int] + message_id: Optional[int] + + +# Module-level registry: (user_id, chat_id) → PendingQuestion +_pending: Dict[Tuple[int, int], PendingQuestion] = {} + + +def register_pending(user_id: int, chat_id: int, pq: PendingQuestion) -> None: + """Register a pending question for a user+chat.""" + _pending[(user_id, chat_id)] = pq + + +def get_pending(user_id: int, chat_id: int) -> Optional[PendingQuestion]: + """Get the pending question for a user+chat, if any.""" + return _pending.get((user_id, chat_id)) + + +def resolve_pending(user_id: int, chat_id: int, answer: Any) -> None: + """Resolve a pending question with the user's answer.""" + pq = _pending.pop((user_id, chat_id), None) + if pq and not pq.future.done(): + pq.future.set_result(answer) + + +def cancel_pending(user_id: int, chat_id: int) -> None: + """Cancel a pending question (e.g. on session error).""" + pq = _pending.pop((user_id, chat_id), None) + if pq and not pq.future.done(): + pq.future.cancel() + + +def format_question_text(question: str, options: List[Dict[str, Any]]) -> str: + """Format a question with its options as readable text.""" + lines = [f"**{question}**", ""] + for opt in options: + label = opt.get("label", "") + desc = opt.get("description", "") + if desc: + lines.append(f"• {label} — {desc}") + else: + lines.append(f"• {label}") + return "\n".join(lines) + + +def build_single_select_keyboard( + options: List[Dict[str, Any]], question_idx: int +) -> InlineKeyboardMarkup: + """Build inline keyboard for a single-select question.""" + buttons = [] + for i, opt in enumerate(options): + buttons.append( + InlineKeyboardButton( + text=opt.get("label", f"Option {i}"), + callback_data=f"askq:{question_idx}:{i}", + ) + ) + # Arrange in rows of 2 + rows = [buttons[i : i + 2] for i in range(0, len(buttons), 2)] + # Other button on its own row + rows.append( + [InlineKeyboardButton(text="Other...", callback_data=f"askq:{question_idx}:other")] + ) + return InlineKeyboardMarkup(rows) + + +def build_multi_select_keyboard( + options: List[Dict[str, Any]], question_idx: int, selected: Set[int] +) -> InlineKeyboardMarkup: + """Build inline keyboard for a multi-select question.""" + buttons = [] + for i, opt in enumerate(options): + label = opt.get("label", f"Option {i}") + prefix = "☑" if i in selected else "☐" + buttons.append( + InlineKeyboardButton( + text=f"{prefix} {label}", + callback_data=f"askq:{question_idx}:t{i}", + ) + ) + rows = [buttons[i : i + 2] for i in range(0, len(buttons), 2)] + rows.append( + [InlineKeyboardButton(text="Other...", callback_data=f"askq:{question_idx}:other")] + ) + rows.append( + [InlineKeyboardButton(text="Done ✓", callback_data=f"askq:{question_idx}:done")] + ) + return InlineKeyboardMarkup(rows) +``` + +**Step 4: Run tests to verify they pass** + +Run: `cd /home/florian/config/claude-code-telegram && python -m pytest tests/unit/test_bot/test_interactive_questions.py -v` +Expected: All 18 tests PASS + +**Step 5: Commit** + +```bash +git add src/bot/features/interactive_questions.py tests/unit/test_bot/test_interactive_questions.py +git commit -m "feat: add interactive questions module with pending registry and keyboard builders" +``` + +--- + +### Task 2: Create the PreToolUse hook callback factory + +**Files:** +- Modify: `src/bot/features/interactive_questions.py` +- Test: `tests/unit/test_bot/test_interactive_questions.py` + +**Step 1: Write failing tests** + +Add to `tests/unit/test_bot/test_interactive_questions.py`: + +```python +from src.bot.features.interactive_questions import make_ask_user_hook + + +class TestMakeAskUserHook: + @pytest.mark.asyncio + async def test_non_ask_user_tool_passes_through(self): + """Hook returns empty dict for non-AskUserQuestion tools.""" + ctx = TelegramContext(bot=MagicMock(), chat_id=1, thread_id=None, user_id=1) + hook = make_ask_user_hook(ctx) + result = await hook( + {"hook_event_name": "PreToolUse", "tool_name": "Bash", "tool_input": {}, "tool_use_id": "x", "session_id": "s", "transcript_path": "", "cwd": ""}, + "x", + {"signal": None}, + ) + assert result == {} + + @pytest.mark.asyncio + async def test_sends_keyboard_for_ask_user(self): + """Hook sends inline keyboard to Telegram for AskUserQuestion.""" + bot = AsyncMock() + sent_msg = MagicMock() + sent_msg.message_id = 42 + bot.send_message.return_value = sent_msg + + ctx = TelegramContext(bot=bot, chat_id=100, thread_id=5, user_id=200) + hook = make_ask_user_hook(ctx) + + tool_input = { + "questions": [ + { + "question": "Pick one?", + "header": "Choice", + "options": [ + {"label": "A", "description": "opt A"}, + {"label": "B", "description": "opt B"}, + ], + "multiSelect": False, + } + ] + } + + # Run hook in background, resolve the pending future after send + async def resolve_later(): + await asyncio.sleep(0.05) + pq = get_pending(200, 100) + assert pq is not None + assert pq.message_id == 42 + resolve_pending(200, 100, "A") + + asyncio.get_event_loop().create_task(resolve_later()) + + result = await hook( + {"hook_event_name": "PreToolUse", "tool_name": "AskUserQuestion", "tool_input": tool_input, "tool_use_id": "t1", "session_id": "s", "transcript_path": "", "cwd": ""}, + "t1", + {"signal": None}, + ) + + # Verify keyboard was sent + bot.send_message.assert_called_once() + call_kwargs = bot.send_message.call_args.kwargs + assert call_kwargs["chat_id"] == 100 + assert call_kwargs["message_thread_id"] == 5 + assert "reply_markup" in call_kwargs + + # Verify updatedInput has the answer + specific = result.get("hookSpecificOutput", {}) + assert specific["hookEventName"] == "PreToolUse" + updated = specific["updatedInput"] + assert updated["answers"]["Pick one?"] == "A" + + @pytest.mark.asyncio + async def test_multi_question_sequential(self): + """Hook processes multiple questions sequentially.""" + bot = AsyncMock() + sent_msg = MagicMock() + sent_msg.message_id = 10 + bot.send_message.return_value = sent_msg + bot.edit_message_text.return_value = sent_msg + + ctx = TelegramContext(bot=bot, chat_id=100, thread_id=None, user_id=200) + hook = make_ask_user_hook(ctx) + + tool_input = { + "questions": [ + { + "question": "First?", + "header": "Q1", + "options": [{"label": "X", "description": ""}], + "multiSelect": False, + }, + { + "question": "Second?", + "header": "Q2", + "options": [{"label": "Y", "description": ""}], + "multiSelect": False, + }, + ] + } + + async def resolve_both(): + await asyncio.sleep(0.05) + resolve_pending(200, 100, "X") + await asyncio.sleep(0.05) + resolve_pending(200, 100, "Y") + + asyncio.get_event_loop().create_task(resolve_both()) + + result = await hook( + {"hook_event_name": "PreToolUse", "tool_name": "AskUserQuestion", "tool_input": tool_input, "tool_use_id": "t1", "session_id": "s", "transcript_path": "", "cwd": ""}, + "t1", + {"signal": None}, + ) + + answers = result["hookSpecificOutput"]["updatedInput"]["answers"] + assert answers["First?"] == "X" + assert answers["Second?"] == "Y" +``` + +**Step 2: Run to verify failure** + +Run: `cd /home/florian/config/claude-code-telegram && python -m pytest tests/unit/test_bot/test_interactive_questions.py::TestMakeAskUserHook -v` +Expected: ImportError for `make_ask_user_hook` + +**Step 3: Implement make_ask_user_hook** + +Add to `src/bot/features/interactive_questions.py`: + +```python +def make_ask_user_hook(tg_ctx: TelegramContext): + """Create a PreToolUse hook callback that intercepts AskUserQuestion. + + The returned async function is registered as a PreToolUse hook in + ClaudeAgentOptions.hooks. When Claude calls AskUserQuestion, the hook: + 1. Sends the question as Telegram inline keyboard buttons + 2. Awaits the user's tap via asyncio.Future + 3. Returns updatedInput with pre-filled answers + """ + + async def hook( + input_data: Dict[str, Any], + tool_use_id: Optional[str], + context: Dict[str, Any], + ) -> Dict[str, Any]: + tool_name = input_data.get("tool_name", "") + if tool_name != "AskUserQuestion": + return {} + + tool_input = input_data.get("tool_input", {}) + questions = tool_input.get("questions", []) + if not questions: + return {} + + answers: Dict[str, str] = {} + + for q_idx, q in enumerate(questions): + question_text = q.get("question", "") + options = q.get("options", []) + multi_select = q.get("multiSelect", False) + + # Build keyboard and message text + text = format_question_text(question_text, options) + if multi_select: + keyboard = build_multi_select_keyboard(options, q_idx, set()) + else: + keyboard = build_single_select_keyboard(options, q_idx) + + # Create Future and register + loop = asyncio.get_running_loop() + future: asyncio.Future = loop.create_future() + pq = PendingQuestion( + future=future, + question_text=question_text, + options=options, + multi_select=multi_select, + selected=set(), + message_id=None, + ) + register_pending(tg_ctx.user_id, tg_ctx.chat_id, pq) + + # Send question to Telegram + try: + msg = await tg_ctx.bot.send_message( + chat_id=tg_ctx.chat_id, + text=text, + reply_markup=keyboard, + message_thread_id=tg_ctx.thread_id, + ) + pq.message_id = msg.message_id + except Exception as e: + logger.error("Failed to send question to Telegram", error=str(e)) + cancel_pending(tg_ctx.user_id, tg_ctx.chat_id) + return {} + + # Wait for user's answer + try: + answer = await future + except asyncio.CancelledError: + logger.info("Question cancelled", question=question_text) + return {} + + answers[question_text] = answer + + logger.info( + "User answered question", + question=question_text, + answer=answer, + ) + + # Return updatedInput with pre-filled answers + updated_input = {**tool_input, "answers": answers} + return { + "hookSpecificOutput": { + "hookEventName": "PreToolUse", + "updatedInput": updated_input, + } + } + + return hook +``` + +**Step 4: Run tests** + +Run: `cd /home/florian/config/claude-code-telegram && python -m pytest tests/unit/test_bot/test_interactive_questions.py -v` +Expected: All tests PASS + +**Step 5: Commit** + +```bash +git add src/bot/features/interactive_questions.py tests/unit/test_bot/test_interactive_questions.py +git commit -m "feat: add PreToolUse hook factory for AskUserQuestion interception" +``` + +--- + +### Task 3: Create the Telegram callback handler for askq: buttons + +**Files:** +- Modify: `src/bot/features/interactive_questions.py` +- Test: `tests/unit/test_bot/test_interactive_questions.py` + +**Step 1: Write failing tests** + +Add to the test file: + +```python +from src.bot.features.interactive_questions import askq_callback, askq_other_text + + +class TestAskqCallback: + @pytest.mark.asyncio + async def test_single_select_resolves(self): + """Tapping a button resolves the pending question.""" + loop = asyncio.get_event_loop() + future = loop.create_future() + pq = PendingQuestion( + future=future, + question_text="Pick?", + options=[{"label": "A", "description": ""}, {"label": "B", "description": ""}], + multi_select=False, + selected=set(), + message_id=42, + ) + register_pending(200, 100, pq) + + query = AsyncMock() + query.data = "askq:0:1" + query.from_user.id = 200 + query.message.chat_id = 100 + query.message.message_id = 42 + query.message.message_thread_id = None + + update = MagicMock() + update.callback_query = query + + context = MagicMock() + + await askq_callback(update, context) + + assert future.result() == "B" + query.answer.assert_called_once() + query.edit_message_text.assert_called_once() + + @pytest.mark.asyncio + async def test_multi_select_toggle(self): + """Tapping a multi-select toggle updates keyboard.""" + loop = asyncio.get_event_loop() + future = loop.create_future() + pq = PendingQuestion( + future=future, + question_text="Pick many?", + options=[{"label": "A", "description": ""}, {"label": "B", "description": ""}], + multi_select=True, + selected=set(), + message_id=42, + ) + register_pending(200, 100, pq) + + query = AsyncMock() + query.data = "askq:0:t0" + query.from_user.id = 200 + query.message.chat_id = 100 + query.message.message_id = 42 + query.message.message_thread_id = None + + update = MagicMock() + update.callback_query = query + + context = MagicMock() + + await askq_callback(update, context) + + assert 0 in pq.selected + assert not future.done() # Not resolved yet, waiting for Done + query.edit_message_reply_markup.assert_called_once() + + @pytest.mark.asyncio + async def test_multi_select_done(self): + """Done button resolves multi-select with selected labels.""" + loop = asyncio.get_event_loop() + future = loop.create_future() + pq = PendingQuestion( + future=future, + question_text="Pick many?", + options=[{"label": "A", "description": ""}, {"label": "B", "description": ""}], + multi_select=True, + selected={0, 1}, + message_id=42, + ) + register_pending(200, 100, pq) + + query = AsyncMock() + query.data = "askq:0:done" + query.from_user.id = 200 + query.message.chat_id = 100 + query.message.message_id = 42 + query.message.message_thread_id = None + + update = MagicMock() + update.callback_query = query + + context = MagicMock() + + await askq_callback(update, context) + + assert future.result() == "A, B" + + @pytest.mark.asyncio + async def test_other_sets_awaiting_text(self): + """Other button sets pq.awaiting_other and edits message.""" + loop = asyncio.get_event_loop() + future = loop.create_future() + pq = PendingQuestion( + future=future, + question_text="Pick?", + options=[{"label": "A", "description": ""}], + multi_select=False, + selected=set(), + message_id=42, + ) + register_pending(200, 100, pq) + + query = AsyncMock() + query.data = "askq:0:other" + query.from_user.id = 200 + query.message.chat_id = 100 + query.message.message_id = 42 + query.message.message_thread_id = None + + update = MagicMock() + update.callback_query = query + + context = MagicMock() + + await askq_callback(update, context) + + assert pq.awaiting_other is True + query.edit_message_text.assert_called_once() + + @pytest.mark.asyncio + async def test_expired_question(self): + """Tapping a button with no pending question shows expired message.""" + query = AsyncMock() + query.data = "askq:0:0" + query.from_user.id = 200 + query.message.chat_id = 100 + query.message.message_id = 42 + query.message.message_thread_id = None + + update = MagicMock() + update.callback_query = query + + context = MagicMock() + + await askq_callback(update, context) + + query.answer.assert_called_once_with(text="Question expired.", show_alert=True) + + +class TestAskqOtherText: + @pytest.mark.asyncio + async def test_captures_text_and_resolves(self): + """Free-text reply resolves the pending question.""" + loop = asyncio.get_event_loop() + future = loop.create_future() + pq = PendingQuestion( + future=future, + question_text="Pick?", + options=[], + multi_select=False, + selected=set(), + message_id=42, + ) + pq.awaiting_other = True + register_pending(200, 100, pq) + + message = MagicMock() + message.text = "My custom answer" + message.from_user.id = 200 + message.chat_id = 100 + + update = MagicMock() + update.message = message + + context = MagicMock() + + result = await askq_other_text(update, context) + + assert future.result() == "My custom answer" + + @pytest.mark.asyncio + async def test_ignores_when_not_awaiting(self): + """Text messages pass through when no Other is pending.""" + message = MagicMock() + message.text = "regular message" + message.from_user.id = 200 + message.chat_id = 100 + + update = MagicMock() + update.message = message + + context = MagicMock() + + result = await askq_other_text(update, context) + + # Should return None to let other handlers process it + assert result is None +``` + +**Step 2: Run to verify failure** + +Run: `cd /home/florian/config/claude-code-telegram && python -m pytest tests/unit/test_bot/test_interactive_questions.py::TestAskqCallback -v` +Expected: ImportError for `askq_callback` + +**Step 3: Implement callback handlers** + +Add to `src/bot/features/interactive_questions.py`. First, add `awaiting_other` field to `PendingQuestion`: + +Update the `PendingQuestion` dataclass: +```python +@dataclass +class PendingQuestion: + """A question waiting for user response.""" + + future: asyncio.Future + question_text: str + options: List[Dict[str, Any]] + multi_select: bool + selected: Set[int] + message_id: Optional[int] + awaiting_other: bool = False +``` + +Then add the handlers: + +```python +async def askq_callback(update: Any, context: Any) -> None: + """Handle inline keyboard button taps for askq: callbacks.""" + query = update.callback_query + user_id = query.from_user.id + chat_id = query.message.chat_id + data = query.data # e.g. "askq:0:1", "askq:0:t1", "askq:0:done", "askq:0:other" + + pq = get_pending(user_id, chat_id) + if not pq: + await query.answer(text="Question expired.", show_alert=True) + return + + # Parse callback data: askq:: + parts = data.split(":") + if len(parts) < 3: + await query.answer(text="Invalid.", show_alert=True) + return + + action = parts[2] + + if action == "other": + # Switch to free-text input mode + pq.awaiting_other = True + await query.answer() + await query.edit_message_text( + text=f"**{pq.question_text}**\n\nType your answer:" + ) + return + + if action == "done": + # Multi-select done — resolve with selected labels + selected_labels = [ + pq.options[i].get("label", f"Option {i}") + for i in sorted(pq.selected) + if i < len(pq.options) + ] + answer = ", ".join(selected_labels) if selected_labels else "" + await query.answer() + await query.edit_message_text( + text=f"**{pq.question_text}**\n\n✓ {answer}" + ) + resolve_pending(user_id, chat_id, answer) + return + + if action.startswith("t"): + # Multi-select toggle + try: + opt_idx = int(action[1:]) + except ValueError: + await query.answer(text="Invalid.", show_alert=True) + return + + if opt_idx in pq.selected: + pq.selected.discard(opt_idx) + else: + pq.selected.add(opt_idx) + + # Rebuild keyboard with updated state + q_idx = int(parts[1]) + keyboard = build_multi_select_keyboard(pq.options, q_idx, pq.selected) + await query.answer() + await query.edit_message_reply_markup(reply_markup=keyboard) + return + + # Single select — action is the option index + try: + opt_idx = int(action) + except ValueError: + await query.answer(text="Invalid.", show_alert=True) + return + + if opt_idx < len(pq.options): + label = pq.options[opt_idx].get("label", f"Option {opt_idx}") + else: + label = f"Option {opt_idx}" + + await query.answer() + await query.edit_message_text( + text=f"**{pq.question_text}**\n\n✓ {label}" + ) + resolve_pending(user_id, chat_id, label) + + +async def askq_other_text(update: Any, context: Any) -> Optional[bool]: + """Handle free-text replies for 'Other...' answers. + + This is registered as a MessageHandler with a low group number so it + runs before the main agentic_text handler. Returns None to pass through + if not handling an 'Other' response. + """ + message = update.message + if not message or not message.text: + return None + + user_id = message.from_user.id + chat_id = message.chat_id + + pq = get_pending(user_id, chat_id) + if not pq or not pq.awaiting_other: + return None # Not our message, let other handlers process it + + # Capture the text and resolve + answer = message.text.strip() + pq.awaiting_other = False + resolve_pending(user_id, chat_id, answer) + return True # Consumed the message +``` + +**Step 4: Run tests** + +Run: `cd /home/florian/config/claude-code-telegram && python -m pytest tests/unit/test_bot/test_interactive_questions.py -v` +Expected: All tests PASS + +**Step 5: Commit** + +```bash +git add src/bot/features/interactive_questions.py tests/unit/test_bot/test_interactive_questions.py +git commit -m "feat: add Telegram callback and text handlers for interactive questions" +``` + +--- + +### Task 4: Wire the hook into sdk_integration.py + +**Files:** +- Modify: `src/claude/sdk_integration.py` (lines 243-300) +- Modify: `src/claude/facade.py` (lines 40-176) +- Test: `tests/unit/test_claude/test_sdk_integration.py` + +**Step 1: Write failing tests** + +Add to `tests/unit/test_claude/test_sdk_integration.py`: + +```python +from src.bot.features.interactive_questions import TelegramContext + + +class TestExecuteCommandHooks: + @pytest.mark.asyncio + async def test_hooks_set_when_telegram_context_provided(self): + """Verify PreToolUse hook is registered when telegram_context is passed.""" + from unittest.mock import AsyncMock, MagicMock, patch + from src.claude.sdk_integration import ClaudeSDKManager + from src.config.settings import Settings + + settings = MagicMock(spec=Settings) + settings.claude_max_turns = 10 + settings.claude_max_cost_per_request = 1.0 + settings.claude_allowed_tools = [] + settings.claude_disallowed_tools = [] + settings.claude_cli_path = None + settings.sandbox_enabled = False + settings.sandbox_excluded_commands = [] + settings.enable_mcp = False + settings.mcp_config_path = None + settings.claude_timeout_seconds = 60 + + manager = ClaudeSDKManager(settings) + + tg_ctx = TelegramContext( + bot=AsyncMock(), chat_id=100, thread_id=5, user_id=200 + ) + + # We can't fully run execute_command without a real CLI, + # but we can verify the options are built correctly by + # patching ClaudeSDKClient + captured_options = {} + with patch("src.claude.sdk_integration.ClaudeSDKClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client._query = AsyncMock() + mock_client._query.receive_messages = AsyncMock(return_value=AsyncMock()) + mock_client_cls.return_value = mock_client + + # Make it raise early so we can inspect options + mock_client.connect.side_effect = Exception("test stop") + + try: + await manager.execute_command( + prompt="test", + working_directory=Path("/tmp"), + telegram_context=tg_ctx, + ) + except Exception: + pass + + # Check that hooks were set on the options + call_args = mock_client_cls.call_args + options = call_args[0][0] if call_args[0] else call_args[1].get("options") + assert options.hooks is not None + assert "PreToolUse" in options.hooks + matchers = options.hooks["PreToolUse"] + assert len(matchers) == 1 + assert matchers[0].matcher == "AskUserQuestion" + + @pytest.mark.asyncio + async def test_no_hooks_without_telegram_context(self): + """Verify no hooks set when telegram_context is None.""" + from unittest.mock import AsyncMock, MagicMock, patch + from src.claude.sdk_integration import ClaudeSDKManager + from src.config.settings import Settings + + settings = MagicMock(spec=Settings) + settings.claude_max_turns = 10 + settings.claude_max_cost_per_request = 1.0 + settings.claude_allowed_tools = [] + settings.claude_disallowed_tools = [] + settings.claude_cli_path = None + settings.sandbox_enabled = False + settings.sandbox_excluded_commands = [] + settings.enable_mcp = False + settings.mcp_config_path = None + settings.claude_timeout_seconds = 60 + + manager = ClaudeSDKManager(settings) + + captured_options = {} + with patch("src.claude.sdk_integration.ClaudeSDKClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client.connect.side_effect = Exception("test stop") + mock_client_cls.return_value = mock_client + + try: + await manager.execute_command( + prompt="test", + working_directory=Path("/tmp"), + ) + except Exception: + pass + + call_args = mock_client_cls.call_args + options = call_args[0][0] if call_args[0] else call_args[1].get("options") + assert not options.hooks # Empty or None +``` + +**Step 2: Run to verify failure** + +Run: `cd /home/florian/config/claude-code-telegram && python -m pytest tests/unit/test_claude/test_sdk_integration.py::TestExecuteCommandHooks -v` +Expected: TypeError — `execute_command()` doesn't accept `telegram_context` + +**Step 3: Modify sdk_integration.py** + +Add import at top of file (after line 30): + +```python +from src.bot.features.interactive_questions import ( + TelegramContext, + cancel_pending, + make_ask_user_hook, +) +``` + +Also import `HookMatcher` from the SDK (add to the existing import block): + +```python +from claude_agent_sdk import ( + ...existing imports... + HookMatcher, +) +``` + +Modify `execute_command` signature (line 243): + +```python +async def execute_command( + self, + prompt: str, + working_directory: Path, + session_id: Optional[str] = None, + continue_session: bool = False, + stream_callback: Optional[Callable[[StreamUpdate], None]] = None, + call_id: Optional[int] = None, + telegram_context: Optional["TelegramContext"] = None, +) -> ClaudeResponse: +``` + +After the `can_use_tool` block (after line 332), add: + +```python + # Register PreToolUse hook for AskUserQuestion if we have + # Telegram context to send questions to + if telegram_context: + ask_hook = make_ask_user_hook(telegram_context) + options.hooks = { + "PreToolUse": [ + HookMatcher( + matcher="AskUserQuestion", + hooks=[ask_hook], + ) + ] + } + logger.info("AskUserQuestion hook registered for Telegram") +``` + +In the `finally` block (around line 395-398), add cleanup: + +```python + finally: + if call_id is not None: + self._active_pids.pop(call_id, None) + # Cancel any pending questions on session end + if telegram_context: + cancel_pending( + telegram_context.user_id, + telegram_context.chat_id, + ) + await client.disconnect() +``` + +**Step 4: Run tests** + +Run: `cd /home/florian/config/claude-code-telegram && python -m pytest tests/unit/test_claude/test_sdk_integration.py::TestExecuteCommandHooks -v` +Expected: PASS + +**Step 5: Commit** + +```bash +git add src/claude/sdk_integration.py tests/unit/test_claude/test_sdk_integration.py +git commit -m "feat: wire PreToolUse hook for AskUserQuestion into SDK session options" +``` + +--- + +### Task 5: Thread telegram_context through facade.py + +**Files:** +- Modify: `src/claude/facade.py` (lines 40-176) + +**Step 1: Modify run_command signature** + +```python +async def run_command( + self, + prompt: str, + working_directory: Path, + user_id: int, + session_id: Optional[str] = None, + on_stream: Optional[Callable[[StreamUpdate], None]] = None, + force_new: bool = False, + call_id: Optional[int] = None, + telegram_context: Optional[Any] = None, +) -> ClaudeResponse: +``` + +**Step 2: Modify _execute signature and passthrough** + +```python +async def _execute( + self, + prompt: str, + working_directory: Path, + session_id: Optional[str] = None, + continue_session: bool = False, + stream_callback: Optional[Callable] = None, + call_id: Optional[int] = None, + telegram_context: Optional[Any] = None, +) -> ClaudeResponse: + """Execute command via SDK.""" + return await self.sdk_manager.execute_command( + prompt=prompt, + working_directory=working_directory, + session_id=session_id, + continue_session=continue_session, + stream_callback=stream_callback, + call_id=call_id, + telegram_context=telegram_context, + ) +``` + +**Step 3: Pass telegram_context through all _execute calls in run_command** + +In `run_command()`, update both `_execute` calls (lines 91-98 and 116-123): + +```python +response = await self._execute( + prompt=prompt, + working_directory=working_directory, + session_id=claude_session_id, + continue_session=should_continue, + stream_callback=on_stream, + call_id=call_id, + telegram_context=telegram_context, +) +``` + +And the retry call: + +```python +response = await self._execute( + prompt=prompt, + working_directory=working_directory, + session_id=None, + continue_session=False, + stream_callback=on_stream, + call_id=call_id, + telegram_context=telegram_context, +) +``` + +**Step 4: Run existing facade tests to verify no regressions** + +Run: `cd /home/florian/config/claude-code-telegram && python -m pytest tests/unit/test_claude/test_facade.py -v` +Expected: PASS (existing tests don't pass telegram_context, defaults to None) + +**Step 5: Commit** + +```bash +git add src/claude/facade.py +git commit -m "feat: thread telegram_context through facade run_command and _execute" +``` + +--- + +### Task 6: Wire orchestrator to pass telegram_context + +**Files:** +- Modify: `src/bot/orchestrator.py` (lines 846-920 and 307-365) + +**Step 1: Modify agentic_text to pass telegram_context** + +In `agentic_text()`, after extracting `chat = update.message.chat` (line 867), build the context: + +```python +from src.bot.features.interactive_questions import TelegramContext + +# Build Telegram context for interactive questions +tg_ctx = TelegramContext( + bot=context.bot, + chat_id=update.message.chat_id, + thread_id=getattr(update.message, "message_thread_id", None), + user_id=user_id, +) +``` + +Then pass it to `run_command()` (around line 909): + +```python +task = asyncio.create_task( + claude_integration.run_command( + prompt=message_text, + working_directory=current_dir, + user_id=user_id, + session_id=session_id, + on_stream=on_stream, + force_new=force_new, + call_id=call_id, + telegram_context=tg_ctx, + ) +) +``` + +**Step 2: Register askq callback handler** + +In `_register_agentic_handlers()` (around line 351-365 where callback handlers are), add: + +```python +from src.bot.features.interactive_questions import askq_callback, askq_other_text + +# Interactive question handlers +app.add_handler( + CallbackQueryHandler(askq_callback, pattern=r"^askq:"), + group=0, # Before menu callbacks +) + +# "Other..." free-text capture — must run before agentic_text (group 10) +from telegram.ext import MessageHandler, filters +app.add_handler( + MessageHandler( + filters.TEXT & ~filters.COMMAND, + askq_other_text, + ), + group=5, # Between auth/rate-limit and agentic_text (group 10) +) +``` + +**Step 3: Run full test suite to verify no regressions** + +Run: `cd /home/florian/config/claude-code-telegram && python -m pytest tests/ -v --timeout=30` +Expected: All existing tests PASS + +**Step 4: Commit** + +```bash +git add src/bot/orchestrator.py +git commit -m "feat: wire Telegram context and askq handlers into orchestrator" +``` + +--- + +### Task 7: Wire menu.py to pass telegram_context + +**Files:** +- Modify: `src/bot/handlers/menu.py` (lines 434-446) + +**Step 1: Build and pass telegram_context in INJECT_SKILL** + +In the INJECT_SKILL section, before `run_command()` (around line 434): + +```python +from src.bot.features.interactive_questions import TelegramContext + +tg_ctx = TelegramContext( + bot=context.bot, + chat_id=chat_id, + thread_id=thread_id, + user_id=query.from_user.id, +) +``` + +Note: `thread_id` is already extracted at line 472 — move it earlier (before the `run_command` call). + +Then pass to `run_command()`: + +```python +response = await claude_integration.run_command( + prompt=prompt, + working_directory=current_dir, + user_id=query.from_user.id, + session_id=session_id, + force_new=force_new, + telegram_context=tg_ctx, +) +``` + +**Step 2: Run menu tests** + +Run: `cd /home/florian/config/claude-code-telegram && python -m pytest tests/unit/test_bot/test_menu.py -v` +Expected: PASS + +**Step 3: Commit** + +```bash +git add src/bot/handlers/menu.py +git commit -m "feat: pass telegram_context from menu skill execution to Claude sessions" +``` + +--- + +### Task 8: Integration smoke test — deploy and test + +**Step 1: Run full test suite** + +```bash +cd /home/florian/config/claude-code-telegram && python -m pytest tests/ -v --timeout=30 +``` + +Expected: All tests PASS + +**Step 2: Lint** + +```bash +cd /home/florian/config/claude-code-telegram && black src/bot/features/interactive_questions.py src/claude/sdk_integration.py src/claude/facade.py src/bot/orchestrator.py src/bot/handlers/menu.py +``` + +**Step 3: Copy to installed location** + +```bash +cp src/bot/features/interactive_questions.py ~/.local/share/uv/tools/claude-code-telegram/lib/python3.12/site-packages/src/bot/features/interactive_questions.py +cp src/claude/sdk_integration.py ~/.local/share/uv/tools/claude-code-telegram/lib/python3.12/site-packages/src/claude/sdk_integration.py +cp src/claude/facade.py ~/.local/share/uv/tools/claude-code-telegram/lib/python3.12/site-packages/src/claude/facade.py +cp src/bot/orchestrator.py ~/.local/share/uv/tools/claude-code-telegram/lib/python3.12/site-packages/src/bot/orchestrator.py +cp src/bot/handlers/menu.py ~/.local/share/uv/tools/claude-code-telegram/lib/python3.12/site-packages/src/bot/handlers/menu.py +``` + +**Step 4: Restart bot** + +```bash +systemctl --user restart claude-telegram-bot +sleep 2 +journalctl --user -u claude-telegram-bot.service --since "30 seconds ago" --no-pager | tail -20 +``` + +Expected: Bot starts without errors, "AskUserQuestion hook registered" appears in logs on first interaction. + +**Step 5: Test in Telegram** + +1. Open /menu → pick a skill that uses AskUserQuestion (e.g. brainstorming from superpowers) +2. When Claude asks a question, verify inline buttons appear in Telegram +3. Tap a button, verify Claude receives the answer and continues +4. Test "Other..." flow: tap Other, type custom text, verify it's captured +5. Test multi-select if available + +**Step 6: Commit and push** + +```bash +git add -A +git commit -m "chore: lint and integration test pass" +git push origin feat/stop-command +``` From 252258392dbdb967cf19e10841ecc886ffd92bc6 Mon Sep 17 00:00:00 2001 From: F1orian <27896106+F1orian@users.noreply.github.com> Date: Mon, 2 Mar 2026 23:23:50 +0100 Subject: [PATCH 13/16] feat: add interactive questions module with pending registry and keyboard builders Co-Authored-By: Claude Opus 4.6 --- src/bot/features/interactive_questions.py | 117 +++++++ .../test_bot/test_interactive_questions.py | 324 ++++++++++++++++++ 2 files changed, 441 insertions(+) create mode 100644 src/bot/features/interactive_questions.py create mode 100644 tests/unit/test_bot/test_interactive_questions.py diff --git a/src/bot/features/interactive_questions.py b/src/bot/features/interactive_questions.py new file mode 100644 index 00000000..1d74a6ae --- /dev/null +++ b/src/bot/features/interactive_questions.py @@ -0,0 +1,117 @@ +"""Interactive question routing for AskUserQuestion tool calls.""" + +import asyncio +from dataclasses import dataclass, field +from typing import Any, Dict, List, Optional, Set, Tuple + +import structlog +from telegram import InlineKeyboardButton, InlineKeyboardMarkup + +logger = structlog.get_logger() + + +@dataclass +class TelegramContext: + """Telegram context needed to send messages from the hook callback.""" + + bot: Any # telegram.Bot + chat_id: int + thread_id: Optional[int] + user_id: int + + +@dataclass +class PendingQuestion: + """A question waiting for user response.""" + + future: asyncio.Future + question_text: str + options: List[Dict[str, Any]] + multi_select: bool + selected: Set[int] = field(default_factory=set) + message_id: Optional[int] = None + awaiting_other: bool = False + + +# Module-level registry: (user_id, chat_id) -> PendingQuestion +_pending: Dict[Tuple[int, int], PendingQuestion] = {} + + +def register_pending(user_id: int, chat_id: int, pq: PendingQuestion) -> None: + """Register a pending question for a user+chat.""" + _pending[(user_id, chat_id)] = pq + + +def get_pending(user_id: int, chat_id: int) -> Optional[PendingQuestion]: + """Get the pending question for a user+chat, if any.""" + return _pending.get((user_id, chat_id)) + + +def resolve_pending(user_id: int, chat_id: int, answer: Any) -> None: + """Resolve a pending question with the user's answer.""" + pq = _pending.pop((user_id, chat_id), None) + if pq and not pq.future.done(): + pq.future.set_result(answer) + + +def cancel_pending(user_id: int, chat_id: int) -> None: + """Cancel a pending question (e.g. on session error).""" + pq = _pending.pop((user_id, chat_id), None) + if pq and not pq.future.done(): + pq.future.cancel() + + +def format_question_text(question: str, options: List[Dict[str, Any]]) -> str: + """Format a question with its options as readable text.""" + lines = [f"**{question}**", ""] + for opt in options: + label = opt.get("label", "") + desc = opt.get("description", "") + if desc: + lines.append(f"• {label} — {desc}") + else: + lines.append(f"• {label}") + return "\n".join(lines) + + +def build_single_select_keyboard( + options: List[Dict[str, Any]], question_idx: int +) -> InlineKeyboardMarkup: + """Build inline keyboard for a single-select question.""" + buttons = [] + for i, opt in enumerate(options): + buttons.append( + InlineKeyboardButton( + text=opt.get("label", f"Option {i}"), + callback_data=f"askq:{question_idx}:{i}", + ) + ) + rows = [buttons[i : i + 2] for i in range(0, len(buttons), 2)] + rows.append( + [InlineKeyboardButton(text="Other...", callback_data=f"askq:{question_idx}:other")] + ) + return InlineKeyboardMarkup(rows) + + +def build_multi_select_keyboard( + options: List[Dict[str, Any]], question_idx: int, selected: Set[int] +) -> InlineKeyboardMarkup: + """Build inline keyboard for a multi-select question.""" + buttons = [] + for i, opt in enumerate(options): + label = opt.get("label", f"Option {i}") + prefix = "☑" if i in selected else "☐" + buttons.append( + InlineKeyboardButton( + text=f"{prefix} {label}", + callback_data=f"askq:{question_idx}:t{i}", + ) + ) + rows = [buttons[i : i + 2] for i in range(0, len(buttons), 2)] + rows.append( + [InlineKeyboardButton(text="Other...", callback_data=f"askq:{question_idx}:other")] + ) + rows.append( + [InlineKeyboardButton(text="Done ✓", callback_data=f"askq:{question_idx}:done")] + ) + return InlineKeyboardMarkup(rows) diff --git a/tests/unit/test_bot/test_interactive_questions.py b/tests/unit/test_bot/test_interactive_questions.py new file mode 100644 index 00000000..88d3b0eb --- /dev/null +++ b/tests/unit/test_bot/test_interactive_questions.py @@ -0,0 +1,324 @@ +"""Tests for the interactive questions module.""" + +from __future__ import annotations + +import asyncio +from unittest.mock import MagicMock + +import pytest + +from src.bot.features.interactive_questions import ( + PendingQuestion, + TelegramContext, + build_multi_select_keyboard, + build_single_select_keyboard, + cancel_pending, + format_question_text, + get_pending, + register_pending, + resolve_pending, + _pending, +) + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture(autouse=True) +def clear_registry(): + """Ensure the pending registry is clean for each test.""" + _pending.clear() + yield + _pending.clear() + + +@pytest.fixture +def sample_options(): + """Sample option dicts used across tests.""" + return [ + {"label": "Yes", "description": "Accept the change"}, + {"label": "No", "description": "Reject the change"}, + {"label": "Skip"}, + ] + + +@pytest.fixture +def sample_options_no_desc(): + """Options with no descriptions.""" + return [ + {"label": "Alpha"}, + {"label": "Beta"}, + ] + + +@pytest.fixture +def event_loop(): + """Provide a fresh event loop for Future creation.""" + loop = asyncio.new_event_loop() + yield loop + loop.close() + + +def _make_pending(loop: asyncio.AbstractEventLoop, **kwargs) -> PendingQuestion: + """Helper to create a PendingQuestion with a Future on the given loop.""" + defaults = { + "future": loop.create_future(), + "question_text": "Pick one", + "options": [{"label": "A"}, {"label": "B"}], + "multi_select": False, + } + defaults.update(kwargs) + return PendingQuestion(**defaults) + + +# --------------------------------------------------------------------------- +# TelegramContext +# --------------------------------------------------------------------------- + + +class TestTelegramContext: + def test_creation_with_thread(self): + ctx = TelegramContext(bot=MagicMock(), chat_id=100, thread_id=42, user_id=7) + assert ctx.chat_id == 100 + assert ctx.thread_id == 42 + assert ctx.user_id == 7 + + def test_creation_without_thread(self): + ctx = TelegramContext(bot=MagicMock(), chat_id=100, thread_id=None, user_id=7) + assert ctx.thread_id is None + + +# --------------------------------------------------------------------------- +# PendingQuestion +# --------------------------------------------------------------------------- + + +class TestPendingQuestion: + def test_defaults(self, event_loop): + pq = _make_pending(event_loop) + assert pq.selected == set() + assert pq.message_id is None + assert pq.awaiting_other is False + + def test_custom_fields(self, event_loop): + pq = _make_pending( + event_loop, + multi_select=True, + selected={0, 2}, + message_id=999, + awaiting_other=True, + ) + assert pq.multi_select is True + assert pq.selected == {0, 2} + assert pq.message_id == 999 + assert pq.awaiting_other is True + + +# --------------------------------------------------------------------------- +# format_question_text +# --------------------------------------------------------------------------- + + +class TestFormatQuestionText: + def test_with_descriptions(self, sample_options): + text = format_question_text("Choose wisely", sample_options) + assert text.startswith("**Choose wisely**") + assert "• Yes — Accept the change" in text + assert "• No — Reject the change" in text + # "Skip" has no description, so no dash + assert "• Skip" in text + assert "Skip —" not in text + + def test_without_descriptions(self, sample_options_no_desc): + text = format_question_text("Pick", sample_options_no_desc) + assert "• Alpha" in text + assert "• Beta" in text + # No dash at all + assert "—" not in text + + def test_empty_options(self): + text = format_question_text("Nothing?", []) + assert text == "**Nothing?**\n" + + +# --------------------------------------------------------------------------- +# build_single_select_keyboard +# --------------------------------------------------------------------------- + + +class TestBuildSingleSelectKeyboard: + def test_buttons_match_options(self, sample_options): + kb = build_single_select_keyboard(sample_options, question_idx=0) + # Flatten all buttons + all_buttons = [btn for row in kb.inline_keyboard for btn in row] + labels = [btn.text for btn in all_buttons] + assert "Yes" in labels + assert "No" in labels + assert "Skip" in labels + assert "Other..." in labels + + def test_callback_data_format(self, sample_options): + kb = build_single_select_keyboard(sample_options, question_idx=5) + all_buttons = [btn for row in kb.inline_keyboard for btn in row] + # Option buttons have askq:: + option_buttons = [b for b in all_buttons if b.text not in ("Other...",)] + for i, btn in enumerate(option_buttons): + assert btn.callback_data == f"askq:5:{i}" + + def test_other_button_present(self, sample_options): + kb = build_single_select_keyboard(sample_options, question_idx=0) + last_row = kb.inline_keyboard[-1] + assert len(last_row) == 1 + assert last_row[0].text == "Other..." + assert last_row[0].callback_data == "askq:0:other" + + def test_two_per_row_layout(self): + """Options are laid out 2 per row, plus the Other row.""" + options = [{"label": f"Opt{i}"} for i in range(5)] + kb = build_single_select_keyboard(options, question_idx=0) + # 5 options -> 3 rows of options (2, 2, 1), + 1 Other row = 4 rows + assert len(kb.inline_keyboard) == 4 + assert len(kb.inline_keyboard[0]) == 2 + assert len(kb.inline_keyboard[1]) == 2 + assert len(kb.inline_keyboard[2]) == 1 + assert len(kb.inline_keyboard[3]) == 1 # Other + + def test_fallback_label(self): + """Options missing 'label' get a fallback.""" + options = [{}] + kb = build_single_select_keyboard(options, question_idx=0) + option_btn = kb.inline_keyboard[0][0] + assert option_btn.text == "Option 0" + + +# --------------------------------------------------------------------------- +# build_multi_select_keyboard +# --------------------------------------------------------------------------- + + +class TestBuildMultiSelectKeyboard: + def test_unchecked_by_default(self, sample_options): + kb = build_multi_select_keyboard(sample_options, question_idx=0, selected=set()) + option_buttons = [ + btn + for row in kb.inline_keyboard + for btn in row + if btn.text not in ("Other...", "Done ✓") + ] + for btn in option_buttons: + assert btn.text.startswith("☐") + + def test_checked_state(self, sample_options): + kb = build_multi_select_keyboard(sample_options, question_idx=0, selected={0, 2}) + option_buttons = [ + btn + for row in kb.inline_keyboard + for btn in row + if btn.text not in ("Other...", "Done ✓") + ] + assert option_buttons[0].text.startswith("☑") # index 0 selected + assert option_buttons[1].text.startswith("☐") # index 1 not selected + assert option_buttons[2].text.startswith("☑") # index 2 selected + + def test_toggle_callback_data(self, sample_options): + kb = build_multi_select_keyboard(sample_options, question_idx=3, selected=set()) + option_buttons = [ + btn + for row in kb.inline_keyboard + for btn in row + if btn.text not in ("Other...", "Done ✓") + ] + for i, btn in enumerate(option_buttons): + assert btn.callback_data == f"askq:3:t{i}" + + def test_done_button(self, sample_options): + kb = build_multi_select_keyboard(sample_options, question_idx=0, selected=set()) + last_row = kb.inline_keyboard[-1] + assert len(last_row) == 1 + assert last_row[0].text == "Done ✓" + assert last_row[0].callback_data == "askq:0:done" + + def test_other_button(self, sample_options): + kb = build_multi_select_keyboard(sample_options, question_idx=0, selected=set()) + # Other is second-to-last row + other_row = kb.inline_keyboard[-2] + assert len(other_row) == 1 + assert other_row[0].text == "Other..." + assert other_row[0].callback_data == "askq:0:other" + + def test_fallback_label(self): + """Options missing 'label' get a fallback with checkbox prefix.""" + options = [{}] + kb = build_multi_select_keyboard(options, question_idx=0, selected=set()) + option_btn = kb.inline_keyboard[0][0] + assert option_btn.text == "☐ Option 0" + + +# --------------------------------------------------------------------------- +# Pending registry +# --------------------------------------------------------------------------- + + +class TestPendingRegistry: + def test_register_and_get(self, event_loop): + pq = _make_pending(event_loop) + register_pending(user_id=1, chat_id=2, pq=pq) + assert get_pending(1, 2) is pq + + def test_get_missing_returns_none(self): + assert get_pending(999, 999) is None + + def test_resolve_clears_and_sets_result(self, event_loop): + pq = _make_pending(event_loop) + register_pending(user_id=1, chat_id=2, pq=pq) + + resolve_pending(1, 2, "user_answer") + + assert get_pending(1, 2) is None + assert pq.future.done() + assert pq.future.result() == "user_answer" + + def test_cancel_clears_and_cancels_future(self, event_loop): + pq = _make_pending(event_loop) + register_pending(user_id=1, chat_id=2, pq=pq) + + cancel_pending(1, 2) + + assert get_pending(1, 2) is None + assert pq.future.cancelled() + + def test_resolve_missing_is_noop(self): + # Should not raise + resolve_pending(999, 999, "anything") + + def test_cancel_missing_is_noop(self): + # Should not raise + cancel_pending(999, 999) + + def test_resolve_already_done_is_noop(self, event_loop): + pq = _make_pending(event_loop) + pq.future.set_result("first") + register_pending(user_id=1, chat_id=2, pq=pq) + + # Should not raise even though future is already done + resolve_pending(1, 2, "second") + assert pq.future.result() == "first" + + def test_cancel_already_done_is_noop(self, event_loop): + pq = _make_pending(event_loop) + pq.future.set_result("done") + register_pending(user_id=1, chat_id=2, pq=pq) + + # Should not raise even though future is already done + cancel_pending(1, 2) + assert pq.future.result() == "done" + assert not pq.future.cancelled() + + def test_register_overwrites_existing(self, event_loop): + pq1 = _make_pending(event_loop, question_text="first") + pq2 = _make_pending(event_loop, question_text="second") + register_pending(user_id=1, chat_id=2, pq=pq1) + register_pending(user_id=1, chat_id=2, pq=pq2) + assert get_pending(1, 2) is pq2 From ab867dd3c11ec1c22e044c58f905ca8145a943c1 Mon Sep 17 00:00:00 2001 From: F1orian <27896106+F1orian@users.noreply.github.com> Date: Mon, 2 Mar 2026 23:28:24 +0100 Subject: [PATCH 14/16] feat: add PreToolUse hook factory and Telegram callback handlers for interactive questions Co-Authored-By: Claude Opus 4.6 --- src/bot/features/interactive_questions.py | 167 +++++++- .../test_bot/test_interactive_questions.py | 366 +++++++++++++++++- 2 files changed, 530 insertions(+), 3 deletions(-) diff --git a/src/bot/features/interactive_questions.py b/src/bot/features/interactive_questions.py index 1d74a6ae..f4e8a9f7 100644 --- a/src/bot/features/interactive_questions.py +++ b/src/bot/features/interactive_questions.py @@ -2,10 +2,11 @@ import asyncio from dataclasses import dataclass, field -from typing import Any, Dict, List, Optional, Set, Tuple +from typing import Any, Callable, Dict, List, Optional, Set, Tuple import structlog -from telegram import InlineKeyboardButton, InlineKeyboardMarkup +from telegram import InlineKeyboardButton, InlineKeyboardMarkup, Update +from telegram.ext import ContextTypes logger = structlog.get_logger() @@ -115,3 +116,165 @@ def build_multi_select_keyboard( [InlineKeyboardButton(text="Done ✓", callback_data=f"askq:{question_idx}:done")] ) return InlineKeyboardMarkup(rows) + + +def make_ask_user_hook( + tg_ctx: TelegramContext, +) -> Callable[..., Any]: + """Return an async PreToolUse hook callback that intercepts AskUserQuestion. + + The hook sends inline keyboards to the Telegram chat and awaits + the user's selection before returning the answers back to Claude. + """ + + async def hook( + input_data: Dict[str, Any], + tool_use_id: Optional[str], + context: Dict[str, Any], + ) -> Dict[str, Any]: + tool_name = input_data.get("tool_name", "") + if tool_name != "AskUserQuestion": + return {} + + tool_input: Dict[str, Any] = input_data.get("tool_input", {}) + questions: List[Dict[str, Any]] = tool_input.get("questions", []) + answers: Dict[str, str] = {} + + for q_idx, question in enumerate(questions): + q_text = question.get("question", "") + options: List[Dict[str, Any]] = question.get("options", []) + multi_select: bool = question.get("multiSelect", False) + + if multi_select: + keyboard = build_multi_select_keyboard(options, q_idx, set()) + else: + keyboard = build_single_select_keyboard(options, q_idx) + + loop = asyncio.get_running_loop() + future: asyncio.Future[str] = loop.create_future() + pq = PendingQuestion( + future=future, + question_text=q_text, + options=options, + multi_select=multi_select, + ) + register_pending(tg_ctx.user_id, tg_ctx.chat_id, pq) + + text = format_question_text(q_text, options) + try: + kwargs: Dict[str, Any] = { + "chat_id": tg_ctx.chat_id, + "text": text, + "reply_markup": keyboard, + "parse_mode": "Markdown", + } + if tg_ctx.thread_id is not None: + kwargs["message_thread_id"] = tg_ctx.thread_id + msg = await tg_ctx.bot.send_message(**kwargs) + pq.message_id = msg.message_id + except Exception: + logger.exception("Failed to send question to Telegram") + cancel_pending(tg_ctx.user_id, tg_ctx.chat_id) + return {} + + try: + answer = await future + except asyncio.CancelledError: + return {} + + answers[q_text] = answer + + return { + "hookSpecificOutput": { + "hookEventName": "PreToolUse", + "updatedInput": {**tool_input, "answers": answers}, + } + } + + return hook + + +async def askq_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) -> None: + """Handle inline keyboard callbacks for interactive questions (askq:* pattern).""" + query = update.callback_query + if query is None: + return + + data = query.data or "" + parts = data.split(":") + if len(parts) != 3 or parts[0] != "askq": + return + + _prefix, q_idx_str, action = parts + q_idx = int(q_idx_str) # noqa: F841 – kept for clarity / future use + + user_id = query.from_user.id if query.from_user else 0 + chat_id = query.message.chat.id if query.message else 0 + pq = get_pending(user_id, chat_id) + + if pq is None: + await query.answer("Question expired.", show_alert=True) + return + + # Single select: action is a digit + if action.isdigit(): + idx = int(action) + label = pq.options[idx].get("label", f"Option {idx}") + resolve_pending(user_id, chat_id, label) + await query.answer() + await query.edit_message_text(f"✓ {label}") + return + + # Multi-select toggle: action starts with "t" + if action.startswith("t") and action[1:].isdigit(): + idx = int(action[1:]) + if idx in pq.selected: + pq.selected.discard(idx) + else: + pq.selected.add(idx) + keyboard = build_multi_select_keyboard(pq.options, q_idx, pq.selected) + await query.answer() + await query.edit_message_reply_markup(reply_markup=keyboard) + return + + # Multi-select done + if action == "done": + labels = [ + pq.options[i].get("label", f"Option {i}") + for i in sorted(pq.selected) + ] + answer = ", ".join(labels) + resolve_pending(user_id, chat_id, answer) + await query.answer() + await query.edit_message_text(f"✓ {answer}") + return + + # Other: free-text input requested + if action == "other": + pq.awaiting_other = True + await query.answer() + await query.edit_message_text("Type your answer:") + return + + +async def askq_other_text( + update: Update, context: ContextTypes.DEFAULT_TYPE +) -> Optional[bool]: + """Handle free-text replies for the 'Other...' option. + + Returns ``True`` if the message was consumed (pending question resolved), + or ``None`` to let other handlers process the message. + """ + if update.message is None: + return None + + user_id = update.message.from_user.id if update.message.from_user else 0 + chat_id = update.message.chat.id + + pq = get_pending(user_id, chat_id) + if pq is None or not pq.awaiting_other: + return None + + answer = update.message.text or "" + resolve_pending(user_id, chat_id, answer) + return True diff --git a/tests/unit/test_bot/test_interactive_questions.py b/tests/unit/test_bot/test_interactive_questions.py index 88d3b0eb..b214a396 100644 --- a/tests/unit/test_bot/test_interactive_questions.py +++ b/tests/unit/test_bot/test_interactive_questions.py @@ -3,18 +3,21 @@ from __future__ import annotations import asyncio -from unittest.mock import MagicMock +from unittest.mock import AsyncMock, MagicMock import pytest from src.bot.features.interactive_questions import ( PendingQuestion, TelegramContext, + askq_callback, + askq_other_text, build_multi_select_keyboard, build_single_select_keyboard, cancel_pending, format_question_text, get_pending, + make_ask_user_hook, register_pending, resolve_pending, _pending, @@ -322,3 +325,364 @@ def test_register_overwrites_existing(self, event_loop): register_pending(user_id=1, chat_id=2, pq=pq1) register_pending(user_id=1, chat_id=2, pq=pq2) assert get_pending(1, 2) is pq2 + + +# --------------------------------------------------------------------------- +# Helpers for callback / hook tests +# --------------------------------------------------------------------------- + + +def _make_tg_ctx(user_id: int = 7, chat_id: int = 100, thread_id: int = None): + """Build a TelegramContext with an AsyncMock bot.""" + bot = AsyncMock() + sent_msg = MagicMock() + sent_msg.message_id = 42 + bot.send_message.return_value = sent_msg + return TelegramContext(bot=bot, chat_id=chat_id, thread_id=thread_id, user_id=user_id) + + +def _make_update_with_callback(data: str, user_id: int = 7, chat_id: int = 100): + """Build a minimal Update with a callback_query.""" + update = MagicMock(spec=["callback_query", "message"]) + update.message = None + + query = AsyncMock() + query.data = data + query.from_user = MagicMock() + query.from_user.id = user_id + query.message = MagicMock() + query.message.chat = MagicMock() + query.message.chat.id = chat_id + update.callback_query = query + return update + + +def _make_update_with_text(text: str, user_id: int = 7, chat_id: int = 100): + """Build a minimal Update with a text message.""" + update = MagicMock(spec=["callback_query", "message"]) + update.callback_query = None + + message = MagicMock() + message.text = text + message.from_user = MagicMock() + message.from_user.id = user_id + message.chat = MagicMock() + message.chat.id = chat_id + update.message = message + return update + + +# --------------------------------------------------------------------------- +# make_ask_user_hook +# --------------------------------------------------------------------------- + + +class TestMakeAskUserHook: + @pytest.mark.asyncio + async def test_non_ask_user_question_returns_empty(self): + tg_ctx = _make_tg_ctx() + hook = make_ask_user_hook(tg_ctx) + result = await hook( + {"tool_name": "Bash", "tool_input": {}}, + tool_use_id="t1", + context={}, + ) + assert result == {} + tg_ctx.bot.send_message.assert_not_called() + + @pytest.mark.asyncio + async def test_sends_keyboard_and_returns_updated_input(self): + tg_ctx = _make_tg_ctx() + hook = make_ask_user_hook(tg_ctx) + + tool_input = { + "questions": [ + { + "question": "Continue?", + "options": [{"label": "Yes"}, {"label": "No"}], + "multiSelect": False, + } + ] + } + + async def resolve_after_delay(): + """Wait briefly then resolve the pending question.""" + await asyncio.sleep(0.05) + pq = get_pending(tg_ctx.user_id, tg_ctx.chat_id) + assert pq is not None + resolve_pending(tg_ctx.user_id, tg_ctx.chat_id, "Yes") + + task = asyncio.create_task(resolve_after_delay()) + result = await hook( + {"tool_name": "AskUserQuestion", "tool_input": tool_input}, + tool_use_id="t2", + context={}, + ) + await task + + tg_ctx.bot.send_message.assert_called_once() + call_kwargs = tg_ctx.bot.send_message.call_args.kwargs + assert call_kwargs["chat_id"] == tg_ctx.chat_id + assert "reply_markup" in call_kwargs + + assert result == { + "hookSpecificOutput": { + "hookEventName": "PreToolUse", + "updatedInput": {**tool_input, "answers": {"Continue?": "Yes"}}, + } + } + + @pytest.mark.asyncio + async def test_multiple_questions_processed_sequentially(self): + tg_ctx = _make_tg_ctx() + hook = make_ask_user_hook(tg_ctx) + + tool_input = { + "questions": [ + { + "question": "First?", + "options": [{"label": "A"}], + "multiSelect": False, + }, + { + "question": "Second?", + "options": [{"label": "B"}], + "multiSelect": False, + }, + ] + } + + resolve_order = [] + + async def resolve_questions(): + for expected_q, answer in [("First?", "A"), ("Second?", "B")]: + # Wait for the pending question to appear + for _ in range(50): + pq = get_pending(tg_ctx.user_id, tg_ctx.chat_id) + if pq is not None and pq.question_text == expected_q: + break + await asyncio.sleep(0.02) + resolve_order.append(expected_q) + resolve_pending(tg_ctx.user_id, tg_ctx.chat_id, answer) + + task = asyncio.create_task(resolve_questions()) + result = await hook( + {"tool_name": "AskUserQuestion", "tool_input": tool_input}, + tool_use_id="t3", + context={}, + ) + await task + + assert resolve_order == ["First?", "Second?"] + answers = result["hookSpecificOutput"]["updatedInput"]["answers"] + assert answers == {"First?": "A", "Second?": "B"} + assert tg_ctx.bot.send_message.call_count == 2 + + @pytest.mark.asyncio + async def test_send_failure_cancels_and_returns_empty(self): + tg_ctx = _make_tg_ctx() + tg_ctx.bot.send_message.side_effect = Exception("network error") + hook = make_ask_user_hook(tg_ctx) + + tool_input = { + "questions": [ + { + "question": "Fail?", + "options": [{"label": "X"}], + "multiSelect": False, + } + ] + } + + result = await hook( + {"tool_name": "AskUserQuestion", "tool_input": tool_input}, + tool_use_id="t4", + context={}, + ) + assert result == {} + assert get_pending(tg_ctx.user_id, tg_ctx.chat_id) is None + + @pytest.mark.asyncio + async def test_thread_id_passed_when_set(self): + tg_ctx = _make_tg_ctx(thread_id=55) + hook = make_ask_user_hook(tg_ctx) + + tool_input = { + "questions": [ + { + "question": "Thread?", + "options": [{"label": "Ok"}], + "multiSelect": False, + } + ] + } + + async def resolve_after_delay(): + await asyncio.sleep(0.05) + resolve_pending(tg_ctx.user_id, tg_ctx.chat_id, "Ok") + + task = asyncio.create_task(resolve_after_delay()) + await hook( + {"tool_name": "AskUserQuestion", "tool_input": tool_input}, + tool_use_id="t5", + context={}, + ) + await task + + call_kwargs = tg_ctx.bot.send_message.call_args.kwargs + assert call_kwargs["message_thread_id"] == 55 + + +# --------------------------------------------------------------------------- +# askq_callback +# --------------------------------------------------------------------------- + + +class TestAskqCallback: + @pytest.mark.asyncio + async def test_single_select_resolves_with_label(self): + loop = asyncio.get_running_loop() + future = loop.create_future() + pq = PendingQuestion( + future=future, + question_text="Pick one", + options=[{"label": "Alpha"}, {"label": "Beta"}], + multi_select=False, + ) + register_pending(user_id=7, chat_id=100, pq=pq) + + update = _make_update_with_callback("askq:0:1", user_id=7, chat_id=100) + await askq_callback(update, MagicMock()) + + assert future.done() + assert future.result() == "Beta" + update.callback_query.answer.assert_awaited_once() + update.callback_query.edit_message_text.assert_awaited_once_with("✓ Beta") + + @pytest.mark.asyncio + async def test_multi_select_toggle_updates_selected(self): + loop = asyncio.get_running_loop() + future = loop.create_future() + pq = PendingQuestion( + future=future, + question_text="Pick many", + options=[{"label": "A"}, {"label": "B"}, {"label": "C"}], + multi_select=True, + ) + register_pending(user_id=7, chat_id=100, pq=pq) + + # Toggle index 1 + update = _make_update_with_callback("askq:0:t1", user_id=7, chat_id=100) + await askq_callback(update, MagicMock()) + + assert 1 in pq.selected + assert not future.done() + update.callback_query.edit_message_reply_markup.assert_awaited_once() + + # Toggle index 1 again to deselect + update2 = _make_update_with_callback("askq:0:t1", user_id=7, chat_id=100) + await askq_callback(update2, MagicMock()) + + assert 1 not in pq.selected + + @pytest.mark.asyncio + async def test_multi_select_done_resolves_with_joined_labels(self): + loop = asyncio.get_running_loop() + future = loop.create_future() + pq = PendingQuestion( + future=future, + question_text="Pick many", + options=[{"label": "X"}, {"label": "Y"}, {"label": "Z"}], + multi_select=True, + selected={0, 2}, + ) + register_pending(user_id=7, chat_id=100, pq=pq) + + update = _make_update_with_callback("askq:0:done", user_id=7, chat_id=100) + await askq_callback(update, MagicMock()) + + assert future.done() + assert future.result() == "X, Z" + update.callback_query.edit_message_text.assert_awaited_once_with("✓ X, Z") + + @pytest.mark.asyncio + async def test_other_sets_awaiting_other(self): + loop = asyncio.get_running_loop() + future = loop.create_future() + pq = PendingQuestion( + future=future, + question_text="Pick one", + options=[{"label": "A"}], + multi_select=False, + ) + register_pending(user_id=7, chat_id=100, pq=pq) + + update = _make_update_with_callback("askq:0:other", user_id=7, chat_id=100) + await askq_callback(update, MagicMock()) + + assert pq.awaiting_other is True + assert not future.done() + update.callback_query.edit_message_text.assert_awaited_once_with( + "Type your answer:" + ) + + @pytest.mark.asyncio + async def test_expired_question_shows_alert(self): + update = _make_update_with_callback("askq:0:0", user_id=7, chat_id=100) + await askq_callback(update, MagicMock()) + + update.callback_query.answer.assert_awaited_once_with( + "Question expired.", show_alert=True + ) + + +# --------------------------------------------------------------------------- +# askq_other_text +# --------------------------------------------------------------------------- + + +class TestAskqOtherText: + @pytest.mark.asyncio + async def test_captures_text_when_awaiting_other(self): + loop = asyncio.get_running_loop() + future = loop.create_future() + pq = PendingQuestion( + future=future, + question_text="Pick", + options=[{"label": "A"}], + multi_select=False, + awaiting_other=True, + ) + register_pending(user_id=7, chat_id=100, pq=pq) + + update = _make_update_with_text("custom answer", user_id=7, chat_id=100) + result = await askq_other_text(update, MagicMock()) + + assert result is True + assert future.done() + assert future.result() == "custom answer" + + @pytest.mark.asyncio + async def test_returns_none_when_no_pending(self): + update = _make_update_with_text("hello", user_id=7, chat_id=100) + result = await askq_other_text(update, MagicMock()) + assert result is None + + @pytest.mark.asyncio + async def test_returns_none_when_not_awaiting_other(self): + loop = asyncio.get_running_loop() + future = loop.create_future() + pq = PendingQuestion( + future=future, + question_text="Pick", + options=[{"label": "A"}], + multi_select=False, + awaiting_other=False, + ) + register_pending(user_id=7, chat_id=100, pq=pq) + + update = _make_update_with_text("hello", user_id=7, chat_id=100) + result = await askq_other_text(update, MagicMock()) + + assert result is None + assert not future.done() From eadc9f2f233415b6f6617357837ebb888a2a2b70 Mon Sep 17 00:00:00 2001 From: F1orian <27896106+F1orian@users.noreply.github.com> Date: Mon, 2 Mar 2026 23:39:22 +0100 Subject: [PATCH 15/16] feat: wire AskUserQuestion PreToolUse hook through SDK, facade, orchestrator, and menu Thread telegram_context from Telegram handlers through facade.py and into sdk_integration.py so the PreToolUse hook for AskUserQuestion can send interactive inline keyboards to the user's chat. - sdk_integration.py: accept telegram_context param, register HookMatcher for AskUserQuestion, cancel pending questions in finally block - facade.py: thread telegram_context through run_command and _execute - orchestrator.py: build TelegramContext in agentic_text, register askq callback and other-text handlers in _register_agentic_handlers - menu.py: build TelegramContext in INJECT_SKILL section, move thread_id extraction before run_command - interactive_questions.py: use ApplicationHandlerStop instead of return True to prevent double-handling of consumed messages - Lazy imports in sdk_integration.py to avoid circular import with bot.features Co-Authored-By: Claude Opus 4.6 --- src/bot/features/interactive_questions.py | 17 +++++----- src/bot/handlers/menu.py | 17 ++++++++-- src/bot/orchestrator.py | 31 +++++++++++++++++++ src/claude/facade.py | 5 +++ src/claude/sdk_integration.py | 31 ++++++++++++++++++- .../test_bot/test_interactive_questions.py | 21 ++++++++----- tests/unit/test_orchestrator.py | 13 +++++--- 7 files changed, 111 insertions(+), 24 deletions(-) diff --git a/src/bot/features/interactive_questions.py b/src/bot/features/interactive_questions.py index f4e8a9f7..7dd2b964 100644 --- a/src/bot/features/interactive_questions.py +++ b/src/bot/features/interactive_questions.py @@ -6,7 +6,7 @@ import structlog from telegram import InlineKeyboardButton, InlineKeyboardMarkup, Update -from telegram.ext import ContextTypes +from telegram.ext import ApplicationHandlerStop, ContextTypes logger = structlog.get_logger() @@ -259,22 +259,25 @@ async def askq_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) -> N async def askq_other_text( update: Update, context: ContextTypes.DEFAULT_TYPE -) -> Optional[bool]: +) -> None: """Handle free-text replies for the 'Other...' option. - Returns ``True`` if the message was consumed (pending question resolved), - or ``None`` to let other handlers process the message. + Raises ``ApplicationHandlerStop`` if the message was consumed (pending + question resolved), which prevents further handler groups (e.g. the + agentic_text handler at group 10) from processing this message. + If no pending "Other" question exists, returns normally so the message + falls through to the next handler group. """ if update.message is None: - return None + return user_id = update.message.from_user.id if update.message.from_user else 0 chat_id = update.message.chat.id pq = get_pending(user_id, chat_id) if pq is None or not pq.awaiting_other: - return None + return answer = update.message.text or "" resolve_pending(user_id, chat_id, answer) - return True + raise ApplicationHandlerStop() diff --git a/src/bot/handlers/menu.py b/src/bot/handlers/menu.py index 4cb94605..0de6223d 100644 --- a/src/bot/handlers/menu.py +++ b/src/bot/handlers/menu.py @@ -437,12 +437,26 @@ async def menu_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) -> N # session has access to all enabled skills. prompt = item.action_value # e.g. "/commit" + # Preserve topic/thread context for supergroup forums + thread_id = getattr(query.message, "message_thread_id", None) + + # Build Telegram context for interactive questions + from ..features.interactive_questions import TelegramContext + + tg_ctx = TelegramContext( + bot=context.bot, + chat_id=chat_id, + thread_id=thread_id, + user_id=query.from_user.id, + ) + response = await claude_integration.run_command( prompt=prompt, working_directory=current_dir, user_id=query.from_user.id, session_id=session_id, force_new=force_new, + telegram_context=tg_ctx, ) # Clear force_new flag on success @@ -468,9 +482,6 @@ async def menu_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) -> N except Exception: pass - # Preserve topic/thread context for supergroup forums - thread_id = getattr(query.message, "message_thread_id", None) - # Send formatted response sent_any = False for msg in formatted_messages: diff --git a/src/bot/orchestrator.py b/src/bot/orchestrator.py index 4c0dec93..4f9c32ba 100644 --- a/src/bot/orchestrator.py +++ b/src/bot/orchestrator.py @@ -364,6 +364,25 @@ def _register_agentic_handlers(self, app: Application) -> None: ) ) + # Interactive question handlers (AskUserQuestion) + from .features.interactive_questions import askq_callback, askq_other_text + + app.add_handler( + CallbackQueryHandler(askq_callback, pattern=r"^askq:"), + group=0, + ) + + # "Other..." free-text capture — runs before agentic_text (group 10). + # Raises ApplicationHandlerStop when it consumes the message, so the + # agentic_text handler in group 10 does not also process it. + app.add_handler( + MessageHandler( + filters.TEXT & ~filters.COMMAND, + askq_other_text, + ), + group=5, + ) + logger.info("Agentic handlers registered") def _register_classic_handlers(self, app: Application) -> None: @@ -905,6 +924,17 @@ async def agentic_text( success = True try: call_id = next(self._call_counter) + + # Build Telegram context for interactive questions (AskUserQuestion) + from .features.interactive_questions import TelegramContext + + tg_ctx = TelegramContext( + bot=context.bot, + chat_id=update.message.chat_id, + thread_id=getattr(update.message, "message_thread_id", None), + user_id=user_id, + ) + task = asyncio.create_task( claude_integration.run_command( prompt=message_text, @@ -914,6 +944,7 @@ async def agentic_text( on_stream=on_stream, force_new=force_new, call_id=call_id, + telegram_context=tg_ctx, ) ) thread_key = self._thread_key(context) diff --git a/src/claude/facade.py b/src/claude/facade.py index 65ebbdaa..bfd06979 100644 --- a/src/claude/facade.py +++ b/src/claude/facade.py @@ -46,6 +46,7 @@ async def run_command( on_stream: Optional[Callable[[StreamUpdate], None]] = None, force_new: bool = False, call_id: Optional[int] = None, + telegram_context: Optional[Any] = None, ) -> ClaudeResponse: """Run Claude Code command with full integration.""" logger.info( @@ -95,6 +96,7 @@ async def run_command( continue_session=should_continue, stream_callback=on_stream, call_id=call_id, + telegram_context=telegram_context, ) except Exception as resume_error: # If resume failed (e.g., session expired/missing on Claude's side), @@ -120,6 +122,7 @@ async def run_command( continue_session=False, stream_callback=on_stream, call_id=call_id, + telegram_context=telegram_context, ) else: raise @@ -164,6 +167,7 @@ async def _execute( continue_session: bool = False, stream_callback: Optional[Callable] = None, call_id: Optional[int] = None, + telegram_context: Optional[Any] = None, ) -> ClaudeResponse: """Execute command via SDK.""" return await self.sdk_manager.execute_command( @@ -173,6 +177,7 @@ async def _execute( continue_session=continue_session, stream_callback=stream_callback, call_id=call_id, + telegram_context=telegram_context, ) async def _find_resumable_session( diff --git a/src/claude/sdk_integration.py b/src/claude/sdk_integration.py index d8eb10e2..ff41a248 100644 --- a/src/claude/sdk_integration.py +++ b/src/claude/sdk_integration.py @@ -6,7 +6,7 @@ import signal from dataclasses import dataclass, field from pathlib import Path -from typing import Any, Callable, Dict, List, Optional +from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional import structlog from claude_agent_sdk import ( @@ -17,6 +17,7 @@ CLIConnectionError, CLIJSONDecodeError, CLINotFoundError, + HookMatcher, Message, PermissionResultAllow, PermissionResultDeny, @@ -40,6 +41,9 @@ ) from .monitor import _is_claude_internal_path, check_bash_directory_boundary +if TYPE_CHECKING: + from ..bot.features.interactive_questions import TelegramContext + logger = structlog.get_logger() @@ -248,6 +252,7 @@ async def execute_command( continue_session: bool = False, stream_callback: Optional[Callable[[StreamUpdate], None]] = None, call_id: Optional[int] = None, + telegram_context: Optional["TelegramContext"] = None, ) -> ClaudeResponse: """Execute Claude Code command via SDK.""" start_time = asyncio.get_event_loop().time() @@ -331,6 +336,22 @@ def _stderr_callback(line: str) -> None: approved_directory=self.config.approved_directory, ) + # Register PreToolUse hook for AskUserQuestion if we have + # Telegram context to send questions to + if telegram_context: + from ..bot.features.interactive_questions import make_ask_user_hook + + ask_hook = make_ask_user_hook(telegram_context) + options.hooks = { + "PreToolUse": [ + HookMatcher( + matcher="AskUserQuestion", + hooks=[ask_hook], + ) + ] + } + logger.info("AskUserQuestion hook registered for Telegram") + # Resume previous session if we have a session_id if session_id and continue_session: options.resume = session_id @@ -395,6 +416,14 @@ async def _run_client() -> None: finally: if call_id is not None: self._active_pids.pop(call_id, None) + # Cancel any pending questions on session end + if telegram_context: + from ..bot.features.interactive_questions import cancel_pending + + cancel_pending( + telegram_context.user_id, + telegram_context.chat_id, + ) await client.disconnect() # Execute with timeout diff --git a/tests/unit/test_bot/test_interactive_questions.py b/tests/unit/test_bot/test_interactive_questions.py index b214a396..56c156d6 100644 --- a/tests/unit/test_bot/test_interactive_questions.py +++ b/tests/unit/test_bot/test_interactive_questions.py @@ -644,6 +644,8 @@ async def test_expired_question_shows_alert(self): class TestAskqOtherText: @pytest.mark.asyncio async def test_captures_text_when_awaiting_other(self): + from telegram.ext import ApplicationHandlerStop + loop = asyncio.get_running_loop() future = loop.create_future() pq = PendingQuestion( @@ -656,20 +658,23 @@ async def test_captures_text_when_awaiting_other(self): register_pending(user_id=7, chat_id=100, pq=pq) update = _make_update_with_text("custom answer", user_id=7, chat_id=100) - result = await askq_other_text(update, MagicMock()) - assert result is True + with pytest.raises(ApplicationHandlerStop): + await askq_other_text(update, MagicMock()) + assert future.done() assert future.result() == "custom answer" @pytest.mark.asyncio - async def test_returns_none_when_no_pending(self): + async def test_no_op_when_no_pending(self): + """When no pending question exists, the handler returns without raising.""" update = _make_update_with_text("hello", user_id=7, chat_id=100) - result = await askq_other_text(update, MagicMock()) - assert result is None + # Should return normally (not raise ApplicationHandlerStop) + await askq_other_text(update, MagicMock()) @pytest.mark.asyncio - async def test_returns_none_when_not_awaiting_other(self): + async def test_no_op_when_not_awaiting_other(self): + """When pending question exists but awaiting_other is False, returns normally.""" loop = asyncio.get_running_loop() future = loop.create_future() pq = PendingQuestion( @@ -682,7 +687,7 @@ async def test_returns_none_when_not_awaiting_other(self): register_pending(user_id=7, chat_id=100, pq=pq) update = _make_update_with_text("hello", user_id=7, chat_id=100) - result = await askq_other_text(update, MagicMock()) + # Should return normally (not raise ApplicationHandlerStop) + await askq_other_text(update, MagicMock()) - assert result is None assert not future.done() diff --git a/tests/unit/test_orchestrator.py b/tests/unit/test_orchestrator.py index a414506a..1595af0b 100644 --- a/tests/unit/test_orchestrator.py +++ b/tests/unit/test_orchestrator.py @@ -150,10 +150,10 @@ def test_agentic_registers_text_document_photo_handlers(agentic_settings, deps): if isinstance(call[0][0], CallbackQueryHandler) ] - # 3 message handlers (text, document, photo) - assert len(msg_handlers) == 3 - # 2 callback handlers (cd: and menu:) - assert len(cb_handlers) == 2 + # 4 message handlers (askq_other_text at group 5, text/document/photo at group 10) + assert len(msg_handlers) == 4 + # 3 callback handlers (cd:, menu:, askq:) + assert len(cb_handlers) == 3 async def test_agentic_bot_commands(agentic_settings, deps): @@ -360,13 +360,16 @@ async def test_agentic_callback_scoped_to_cd_pattern(agentic_settings, deps): if isinstance(call[0][0], CallbackQueryHandler) ] - assert len(cb_handlers) == 2 + assert len(cb_handlers) == 3 # First handler: cd: pattern assert cb_handlers[0].pattern is not None assert cb_handlers[0].pattern.match("cd:my_project") # Second handler: menu: pattern assert cb_handlers[1].pattern is not None assert cb_handlers[1].pattern.match("menu:back") + # Third handler: askq: pattern (interactive questions) + assert cb_handlers[2].pattern is not None + assert cb_handlers[2].pattern.match("askq:0:1") async def test_agentic_document_rejects_large_files(agentic_settings, deps): From d407291e6e4fe3a6f505c3bc8127735364d5e0bc Mon Sep 17 00:00:00 2001 From: F1orian <27896106+F1orian@users.noreply.github.com> Date: Mon, 2 Mar 2026 23:46:04 +0100 Subject: [PATCH 16/16] style: black/isort formatting and fix setting_sources test Update test_setting_sources_includes_project to match the setting_sources=["project", "user"] change. Apply black and isort formatting to all interactive questions files. Co-Authored-By: Claude Opus 4.6 --- src/bot/features/interactive_questions.py | 19 ++++++++++++------- src/claude/sdk_integration.py | 3 +-- .../test_bot/test_interactive_questions.py | 11 +++++++---- .../unit/test_claude/test_sdk_integration.py | 6 +++--- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/bot/features/interactive_questions.py b/src/bot/features/interactive_questions.py index 7dd2b964..9ef71498 100644 --- a/src/bot/features/interactive_questions.py +++ b/src/bot/features/interactive_questions.py @@ -89,7 +89,11 @@ def build_single_select_keyboard( ) rows = [buttons[i : i + 2] for i in range(0, len(buttons), 2)] rows.append( - [InlineKeyboardButton(text="Other...", callback_data=f"askq:{question_idx}:other")] + [ + InlineKeyboardButton( + text="Other...", callback_data=f"askq:{question_idx}:other" + ) + ] ) return InlineKeyboardMarkup(rows) @@ -110,7 +114,11 @@ def build_multi_select_keyboard( ) rows = [buttons[i : i + 2] for i in range(0, len(buttons), 2)] rows.append( - [InlineKeyboardButton(text="Other...", callback_data=f"askq:{question_idx}:other")] + [ + InlineKeyboardButton( + text="Other...", callback_data=f"askq:{question_idx}:other" + ) + ] ) rows.append( [InlineKeyboardButton(text="Done ✓", callback_data=f"askq:{question_idx}:done")] @@ -240,8 +248,7 @@ async def askq_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) -> N # Multi-select done if action == "done": labels = [ - pq.options[i].get("label", f"Option {i}") - for i in sorted(pq.selected) + pq.options[i].get("label", f"Option {i}") for i in sorted(pq.selected) ] answer = ", ".join(labels) resolve_pending(user_id, chat_id, answer) @@ -257,9 +264,7 @@ async def askq_callback(update: Update, context: ContextTypes.DEFAULT_TYPE) -> N return -async def askq_other_text( - update: Update, context: ContextTypes.DEFAULT_TYPE -) -> None: +async def askq_other_text(update: Update, context: ContextTypes.DEFAULT_TYPE) -> None: """Handle free-text replies for the 'Other...' option. Raises ``ApplicationHandlerStop`` if the message was consumed (pending diff --git a/src/claude/sdk_integration.py b/src/claude/sdk_integration.py index ff41a248..79821ab5 100644 --- a/src/claude/sdk_integration.py +++ b/src/claude/sdk_integration.py @@ -310,8 +310,7 @@ def _stderr_callback(line: str) -> None: plugin_paths = get_enabled_plugin_paths() if plugin_paths: options.plugins = [ - SdkPluginConfig(type="local", path=p) - for p in plugin_paths + SdkPluginConfig(type="local", path=p) for p in plugin_paths ] logger.info( "Plugins configured for session", diff --git a/tests/unit/test_bot/test_interactive_questions.py b/tests/unit/test_bot/test_interactive_questions.py index 56c156d6..068ff6b7 100644 --- a/tests/unit/test_bot/test_interactive_questions.py +++ b/tests/unit/test_bot/test_interactive_questions.py @@ -10,6 +10,7 @@ from src.bot.features.interactive_questions import ( PendingQuestion, TelegramContext, + _pending, askq_callback, askq_other_text, build_multi_select_keyboard, @@ -20,10 +21,8 @@ make_ask_user_hook, register_pending, resolve_pending, - _pending, ) - # --------------------------------------------------------------------------- # Fixtures # --------------------------------------------------------------------------- @@ -214,7 +213,9 @@ def test_unchecked_by_default(self, sample_options): assert btn.text.startswith("☐") def test_checked_state(self, sample_options): - kb = build_multi_select_keyboard(sample_options, question_idx=0, selected={0, 2}) + kb = build_multi_select_keyboard( + sample_options, question_idx=0, selected={0, 2} + ) option_buttons = [ btn for row in kb.inline_keyboard @@ -338,7 +339,9 @@ def _make_tg_ctx(user_id: int = 7, chat_id: int = 100, thread_id: int = None): sent_msg = MagicMock() sent_msg.message_id = 42 bot.send_message.return_value = sent_msg - return TelegramContext(bot=bot, chat_id=chat_id, thread_id=thread_id, user_id=user_id) + return TelegramContext( + bot=bot, chat_id=chat_id, thread_id=thread_id, user_id=user_id + ) def _make_update_with_callback(data: str, user_id: int = 7, chat_id: int = 100): diff --git a/tests/unit/test_claude/test_sdk_integration.py b/tests/unit/test_claude/test_sdk_integration.py index e6780344..fb930f38 100644 --- a/tests/unit/test_claude/test_sdk_integration.py +++ b/tests/unit/test_claude/test_sdk_integration.py @@ -934,8 +934,8 @@ async def test_system_prompt_unchanged_without_claude_md( assert "Use relative paths." in opts.system_prompt assert "# Project Rules" not in opts.system_prompt - async def test_setting_sources_includes_project(self, sdk_manager, tmp_path): - """setting_sources=['project'] is passed to ClaudeAgentOptions.""" + async def test_setting_sources_includes_project_and_user(self, sdk_manager, tmp_path): + """setting_sources=['project', 'user'] is passed to ClaudeAgentOptions.""" captured: list = [] mock_factory = _mock_client_factory( _make_assistant_message("ok"), @@ -949,4 +949,4 @@ async def test_setting_sources_includes_project(self, sdk_manager, tmp_path): await sdk_manager.execute_command(prompt="test", working_directory=tmp_path) opts = captured[0] - assert opts.setting_sources == ["project"] + assert opts.setting_sources == ["project", "user"]