From 99a36470b931bf3ce201744a9422e552cd9fb3d3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 14 Jun 2026 18:17:38 +0000 Subject: [PATCH 01/14] harden(runner): shell-free, consistent command execution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The agent runs LLM-generated commands via ToolRunner. Execution was inconsistent across paths: POSIX sync used shlex.split (safe), but the Windows sync path used shell=True (cmd.exe) and execute_async() used create_subprocess_shell — both shell-injection surfaces. validate_command only checked the first token against the allowlist and never rejected shell operators or command substitution. - execute(): use shell=False on all platforms. On Windows the raw string goes to CreateProcess (no cmd.exe), so metacharacters are not interpreted. - execute_async(): replace create_subprocess_shell with create_subprocess_exec and bring it to parity with execute() (plugin interception, risky-command confirmation, output truncation, FileNotFoundError handling, logging, on_output) so it can never become a future footgun. - validate_command(): reject command substitution ($(...), backticks) and standalone shell-operator tokens (; | || & && > >> < <<) via a token-based check that does not false-positive on URLs (& inside a quoted arg). Handle unbalanced quotes gracefully instead of crashing. - Factor truncation into _truncate_output() (removes a duplicated long line). Adds 12 tests covering operator/substitution rejection, URL-with-ampersand acceptance, unbalanced-quote handling, shell-free proof (chained command's second half never runs; $HOME not interpolated), and async parity. Full suite: 755 passed, 40 skipped. --- hackbot/core/runner.py | 130 ++++++++++++++++++++++++++++++++++++++--- tests/test_runner.py | 107 +++++++++++++++++++++++++++++++++ 2 files changed, 228 insertions(+), 9 deletions(-) diff --git a/hackbot/core/runner.py b/hackbot/core/runner.py index bc35314..5a520bf 100644 --- a/hackbot/core/runner.py +++ b/hackbot/core/runner.py @@ -108,6 +108,11 @@ def output(self) -> str: "bash -i", ] +# Standalone shell-operator tokens that indicate command chaining or redirection. +# Detected as discrete tokens (via shlex), so characters inside a single argument +# (e.g. an '&' in a URL query string) are never flagged. +SHELL_OPERATOR_TOKENS = frozenset({";", "|", "||", "&", "&&", ">", ">>", "<", "<<"}) + class ToolRunner: """ @@ -313,6 +318,36 @@ def _extract_validated_tool(self, parts: List[str]) -> str: return candidate + @staticmethod + def _contains_shell_operators(command: str) -> Optional[str]: + """Return the offending marker if *command* contains shell command + substitution or a standalone shell-operator token, else None. + + Operates on the NORMALIZED command (wrapping backticks and a leading + ``$ `` prompt have already been stripped by ``_normalize_command``), so + only *embedded* substitution survives to be rejected here. Detection is + token-based to avoid false positives on legitimate arguments such as URL + query strings (``http://x/?a=1&b=2`` stays inside a single shlex token). + + Returns ``""`` when the command cannot be tokenized + (e.g. unbalanced quotes) so the caller can reject it gracefully. + """ + # Command substitution: any remaining backtick or "$(" is rejected. + if "`" in command: + return "`" + if "$(" in command: + return "$(" + + try: + tokens = shlex.split(command) + except ValueError: + return "" + + for tok in tokens: + if tok in SHELL_OPERATOR_TOKENS: + return tok + return None + def validate_command(self, command: str) -> tuple[bool, str]: """ Validate a command for safety. @@ -330,8 +365,21 @@ def validate_command(self, command: str) -> tuple[bool, str]: if blocked in cmd_lower: return False, f"Blocked command detected: {blocked}" + # Reject shell command-substitution and standalone operator tokens. + # Unconditional (not safe_mode-gated): execution is shell-free, so this + # is a clear, early hard-reject of unsupported syntax rather than a + # confirmable RISKY warning. + offender = self._contains_shell_operators(command) + if offender == "": + return False, "Invalid command: unbalanced quotes" + if offender is not None: + return False, f"Shell metacharacter not allowed: '{offender}'" + # Extract tool name (skip 'sudo' prefix for validation) - parts = shlex.split(command) if platform.system() != "Windows" else command.split() + try: + parts = shlex.split(command) if platform.system() != "Windows" else command.split() + except ValueError: + return False, "Invalid command: unbalanced quotes" if not parts: return False, "Empty command" @@ -472,11 +520,13 @@ def execute(self, command: str, tool_name: str = "", explanation: str = "") -> T try: is_windows = platform.system() == "Windows" proc = subprocess.Popen( + # Windows: pass the raw string to CreateProcess (no cmd.exe, so + # shell metacharacters are NOT interpreted). POSIX: split to argv. command if is_windows else shlex.split(command), stdout=subprocess.PIPE, stderr=subprocess.PIPE, stdin=subprocess.PIPE if stdin_data else None, - shell=is_windows, + shell=False, text=True, env=self._get_env(), ) @@ -491,10 +541,7 @@ def execute(self, command: str, tool_name: str = "", explanation: str = "") -> T duration = time.time() - start # Truncate if needed - truncated = False - if len(stdout) > self.MAX_OUTPUT_SIZE: - stdout = stdout[: self.MAX_OUTPUT_SIZE] + f"\n\n[OUTPUT TRUNCATED at {self.MAX_OUTPUT_SIZE} bytes]" - truncated = True + stdout, truncated = self._truncate_output(stdout) result = ToolResult( tool=self._infer_tool_name(command, tool_name), @@ -540,12 +587,17 @@ def execute(self, command: str, tool_name: str = "", explanation: str = "") -> T return result async def execute_async(self, command: str, tool_name: str = "") -> ToolResult: - """Execute a command asynchronously.""" + """Execute a command asynchronously (shell-free; parity with execute()).""" command = self._normalize_command(command) + # Plugin execution — intercept hackbot-plugin commands + if command.strip().startswith("hackbot-plugin "): + return self._execute_plugin(command, tool_name) + # Apply sudo prefix if enabled command = self._apply_sudo(command) + # Validate is_safe, reason = self.validate_command(command) if not is_safe: return ToolResult( @@ -558,11 +610,46 @@ async def execute_async(self, command: str, tool_name: str = "") -> ToolResult: success=False, ) + # Check for risky commands + if "RISKY" in reason and not self.auto_confirm: + if self.on_confirm: + confirmed = self.on_confirm(command, reason) + if not confirmed: + return ToolResult( + tool=self._infer_tool_name(command, tool_name), + command=command, + stdout="", + stderr="User declined execution", + return_code=-2, + duration=0, + success=False, + ) + start = time.time() stdin_data = self._feed_sudo_password() if "sudo -S" in command else None + + # Tokenize without a shell so /bin/sh is never invoked (parity with execute()). try: - proc = await asyncio.create_subprocess_shell( - command, + args = shlex.split(command) + except ValueError as e: + result = ToolResult( + tool=self._infer_tool_name(command, tool_name), + command=command, + stdout="", + stderr=f"Execution error: {e}", + return_code=-4, + duration=time.time() - start, + success=False, + ) + self.history.append(result) + self._log_execution(result) + if self.on_output: + self.on_output(result.output) + return result + + try: + proc = await asyncio.create_subprocess_exec( + *args, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, stdin=asyncio.subprocess.PIPE if stdin_data else None, @@ -582,6 +669,9 @@ async def execute_async(self, command: str, tool_name: str = "") -> ToolResult: duration = time.time() - start + # Truncate if needed + stdout, truncated = self._truncate_output(stdout) + result = ToolResult( tool=self._infer_tool_name(command, tool_name), command=command, @@ -590,6 +680,18 @@ async def execute_async(self, command: str, tool_name: str = "") -> ToolResult: return_code=proc.returncode or 0, duration=duration, success=(proc.returncode or 0) == 0, + truncated=truncated, + ) + except FileNotFoundError: + duration = time.time() - start + result = ToolResult( + tool=self._infer_tool_name(command, tool_name), + command=command, + stdout="", + stderr=f"Tool not found: {self._infer_tool_name(command, tool_name)}", + return_code=-3, + duration=duration, + success=False, ) except Exception as e: duration = time.time() - start @@ -604,8 +706,18 @@ async def execute_async(self, command: str, tool_name: str = "") -> ToolResult: ) self.history.append(result) + self._log_execution(result) + if self.on_output: + self.on_output(result.output) return result + def _truncate_output(self, stdout: str) -> tuple[str, bool]: + """Cap *stdout* at MAX_OUTPUT_SIZE. Returns (text, truncated).""" + if len(stdout) <= self.MAX_OUTPUT_SIZE: + return stdout, False + notice = f"\n\n[OUTPUT TRUNCATED at {self.MAX_OUTPUT_SIZE} bytes]" + return stdout[: self.MAX_OUTPUT_SIZE] + notice, True + def _get_env(self) -> Dict[str, str]: """Get sanitized environment for subprocess execution.""" env = os.environ.copy() diff --git a/tests/test_runner.py b/tests/test_runner.py index 90e7710..0642e22 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -1,5 +1,6 @@ """Tests for HackBot Tool Runner.""" +import asyncio import platform import pytest @@ -271,3 +272,109 @@ def test_validate_sudo_only_no_tool(): r = ToolRunner(allowed_tools=["nmap"], timeout=10) is_safe, reason = r.validate_command("sudo -n") assert not is_safe + + +# ── Shell-free execution & metacharacter rejection ── + + +def test_windows_exec_is_shell_free(runner, monkeypatch): + """execute() must never spawn a shell (guards against shell=is_windows).""" + captured = {} + + class FakePopen: + def __init__(self, *args, **kwargs): + captured["shell"] = kwargs.get("shell") + self.returncode = 0 + + def communicate(self, input=None, timeout=None): + return ("ok", "") + + monkeypatch.setattr("hackbot.core.runner.subprocess.Popen", FakePopen) + runner.execute("echo hi", tool_name="echo") + assert captured["shell"] is False + + +def test_validate_rejects_semicolon_operator(runner): + """Command chaining with ';' is rejected as a shell metacharacter.""" + is_safe, reason = runner.validate_command("echo a ; echo b") + assert not is_safe + assert "metacharacter" in reason.lower() + assert "';'" in reason + + +def test_validate_rejects_pipe_operator(runner): + """A pipe operator token is rejected.""" + is_safe, reason = runner.validate_command("nmap 127.0.0.1 | grep open") + assert not is_safe + assert "|" in reason + + +def test_validate_rejects_and_operator(runner): + """Logical-and chaining is rejected.""" + is_safe, reason = runner.validate_command("echo a && echo b") + assert not is_safe + assert "&&" in reason + + +def test_validate_rejects_redirect_operator(runner): + """Output redirection is rejected.""" + is_safe, reason = runner.validate_command("nmap 127.0.0.1 > /tmp/out") + assert not is_safe + assert ">" in reason + + +def test_validate_rejects_command_substitution_dollar_paren(runner): + """$(...) command substitution is rejected.""" + is_safe, reason = runner.validate_command("nmap $(whoami)") + assert not is_safe + assert "$(" in reason + + +def test_validate_rejects_embedded_backticks(runner): + """Embedded backtick substitution is rejected (distinct from wrapping backticks).""" + is_safe, reason = runner.validate_command("nmap `whoami`") + assert not is_safe + assert "`" in reason + + +def test_validate_allows_url_with_ampersand(runner): + """A URL with '&'/'?'/'=' in a quoted arg is NOT flagged (token-based check).""" + is_safe, reason = runner.validate_command("curl 'http://example.com/?a=1&b=2'") + assert is_safe + assert reason == "OK" + is_safe2, _ = runner.validate_command('curl "http://example.com/?x=1&y=2"') + assert is_safe2 + + +def test_validate_rejects_unbalanced_quotes(runner): + """Unbalanced quotes are rejected gracefully (no crash).""" + is_safe, reason = runner.validate_command('nmap "unclosed') + assert not is_safe + assert "unbalanced" in reason.lower() + + +def test_chained_command_does_not_execute_second_half(runner, tmp_path): + """The executor is shell-free: a chained command never runs its second half.""" + if platform.system() == "Windows": + pytest.skip("POSIX-only shell-free proof") + sentinel = tmp_path / "pwned" + result = runner.execute(f"echo hi ; touch {sentinel}", tool_name="echo") + # Blocked at validation, and the second command never ran. + assert result.return_code == -1 + assert "metacharacter" in result.stderr.lower() + assert not sentinel.exists() + + +def test_exec_does_not_interpolate_shell_variables(runner): + """Content that passes validation is still not shell-interpolated.""" + if platform.system() == "Windows": + pytest.skip("POSIX-only shell-free proof") + result = runner.execute("echo $HOME", tool_name="echo") + assert "$HOME" in result.stdout # literal, not expanded + + +def test_execute_async_parity_blocked(runner): + """execute_async runs validation and is shell-free (rejects chaining).""" + result = asyncio.run(runner.execute_async("echo a ; echo b")) + assert result.return_code == -1 + assert "metacharacter" in result.stderr.lower() From 009b8aa43b4ff4740b6c6822ece475d32ed59c08 Mon Sep 17 00:00:00 2001 From: JMAN730 Date: Mon, 22 Jun 2026 00:29:26 -0400 Subject: [PATCH 02/14] Add DeepSeek key management --- .../2026-06-21-deepseek-multi-key-failover.md | 1100 +++++++++++++++++ ...6-21-deepseek-multi-key-failover-design.md | 212 ++++ hackbot/cli.py | 55 +- hackbot/config.py | 61 + tests/test_cli.py | 39 + tests/test_config.py | 131 ++ 6 files changed, 1585 insertions(+), 13 deletions(-) create mode 100644 docs/superpowers/plans/2026-06-21-deepseek-multi-key-failover.md create mode 100644 docs/superpowers/specs/2026-06-21-deepseek-multi-key-failover-design.md create mode 100644 tests/test_cli.py diff --git a/docs/superpowers/plans/2026-06-21-deepseek-multi-key-failover.md b/docs/superpowers/plans/2026-06-21-deepseek-multi-key-failover.md new file mode 100644 index 0000000..4b36023 --- /dev/null +++ b/docs/superpowers/plans/2026-06-21-deepseek-multi-key-failover.md @@ -0,0 +1,1100 @@ +# DeepSeek Multi-Key Failover Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Let users store a pool of DeepSeek API keys so HackBot automatically rotates to the next working key and retries the request when the active key is dead or out of credit. + +**Architecture:** A key pool (`AIConfig.api_keys`) persists in `config.yaml` with the active key mirrored into the existing `api_key` field for backward compatibility. `AIEngine` classifies failures and, only for the `deepseek` provider, rotates to the next untried key and retries — transparently for blocking calls, and (for streaming) only when no token has been emitted yet. REPL commands manage the pool. + +**Tech Stack:** Python 3.9+, OpenAI SDK (provider-agnostic client), Click + Rich CLI, pytest with `unittest.mock`. + +**Spec:** `docs/superpowers/specs/2026-06-21-deepseek-multi-key-failover-design.md` + +--- + +## File Structure + +- `hackbot/config.py` — add `api_keys` field, `reconcile_keys()`, pool helpers (`add_key`/`remove_key`/`set_active_key`), `mask_key()`. Persist via `save_config`. +- `hackbot/core/engine.py` — `KeyPoolExhaustedError`, `_key_failure_reason()` classifier, rotation state + `_maybe_rotate()`, failover loops in `_blocking_chat`/`_stream_chat`, `on_notice` hook. +- `hackbot/cli.py` — `_rebuild_engine()` helper, `_set_key()` subcommand routing, `_key_add`/`_key_remove`/`_list_keys`, `/keys` dispatch entry. +- `hackbot/ui/__init__.py` — `/help` text additions. +- `tests/test_config.py`, `tests/test_engine.py`, `tests/test_keypool_cli.py` — coverage. + +--- + +## Task 1: Config — `api_keys` field, persistence & reconciliation + +**Files:** +- Modify: `hackbot/config.py` +- Test: `tests/test_config.py` + +- [ ] **Step 1: Write the failing tests** + +Add to `tests/test_config.py` (extend the import from `hackbot.config` to include `reconcile_keys`, `save_config`): + +```python +from hackbot.config import reconcile_keys, save_config + + +def test_reconcile_seeds_pool_from_single_key(): + ai = {"api_key": "sk-a", "api_keys": []} + reconcile_keys(ai) + assert ai["api_keys"] == ["sk-a"] + assert ai["api_key"] == "sk-a" + + +def test_reconcile_active_is_pool_first(): + ai = {"api_key": "", "api_keys": ["sk-a", "sk-b"]} + reconcile_keys(ai) + assert ai["api_key"] == "sk-a" + + +def test_reconcile_env_key_prepended_and_deduped(): + ai = {"api_key": "sk-env", "api_keys": ["sk-a", "sk-env", "sk-b"]} + reconcile_keys(ai) + assert ai["api_keys"] == ["sk-env", "sk-a", "sk-b"] + assert ai["api_key"] == "sk-env" + + +def test_reconcile_empty_pool(): + ai = {"api_key": "", "api_keys": []} + reconcile_keys(ai) + assert ai["api_keys"] == [] + assert ai["api_key"] == "" + + +def test_api_keys_round_trip(tmp_path, monkeypatch): + import hackbot.config as cfgmod + monkeypatch.setattr(cfgmod, "CONFIG_FILE", tmp_path / "config.yaml") + for var in ("DEEPSEEK_API_KEY", "OPENAI_API_KEY", "HACKBOT_API_KEY", "ANTHROPIC_API_KEY"): + monkeypatch.delenv(var, raising=False) + + cfg = cfgmod.HackBotConfig() + cfg.ai.provider = "deepseek" + cfg.ai.api_keys = ["sk-a", "sk-b"] + cfg.ai.api_key = "sk-a" + cfgmod.save_config(cfg) + + loaded = cfgmod.load_config() + assert loaded.ai.api_keys == ["sk-a", "sk-b"] + assert loaded.ai.api_key == "sk-a" +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `pytest tests/test_config.py -k "reconcile or round_trip" -v` +Expected: FAIL — `ImportError: cannot import name 'reconcile_keys'` (and `AIConfig` has no `api_keys`). + +- [ ] **Step 3: Implement** + +In `hackbot/config.py`: + +3a. Add `api_keys` to the default config — inside `DEFAULT_CONFIG["ai"]`, after `"base_url": "",`: + +```python + "api_keys": [], +``` + +3b. Add the field to `AIConfig` (after `api_key: str = ""`): + +```python + api_keys: List[str] = field(default_factory=list) +``` + +3c. Persist it — in `save_config`, inside the `"ai": { ... }` dict, after `"api_key": cfg.ai.api_key,`: + +```python + "api_keys": cfg.ai.api_keys, +``` + +3d. Add the reconciliation function (place it just above `load_config`): + +```python +def reconcile_keys(ai: Dict[str, Any]) -> None: + """Normalize the API key pool in an ``ai`` config dict, in place. + + Dedupes, drops empties, and keeps the active key first so the invariant + ``api_key == api_keys[0]`` always holds. Seeds the pool from a legacy + single ``api_key`` and lets an env-provided ``api_key`` take priority by + placing it first. + """ + pool = [k for k in dict.fromkeys([ai.get("api_key", "")] + list(ai.get("api_keys") or [])) if k] + ai["api_keys"] = pool + ai["api_key"] = pool[0] if pool else "" +``` + +3e. Call it in `load_config`, immediately after the `allowed_tools` migration block and before `cfg = HackBotConfig(`: + +```python + reconcile_keys(merged.setdefault("ai", {})) +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `pytest tests/test_config.py -v` +Expected: PASS (all existing config tests plus the 5 new ones). + +- [ ] **Step 5: Commit** + +```bash +git add hackbot/config.py tests/test_config.py +git commit -m "$(printf 'feat(config): add DeepSeek api_keys pool with reconciliation\n\nCo-Authored-By: Claude Opus 4.8 ')" +``` + +--- + +## Task 2: Config — pool helpers & key masking + +**Files:** +- Modify: `hackbot/config.py` +- Test: `tests/test_config.py` + +- [ ] **Step 1: Write the failing tests** + +Add to `tests/test_config.py` (extend the `hackbot.config` import to include `add_key`, `remove_key`, `set_active_key`, `mask_key`): + +```python +from hackbot.config import add_key, remove_key, set_active_key, mask_key + + +def test_add_key_appends_and_dedupes(): + cfg = AIConfig(provider="deepseek", api_key="sk-a", api_keys=["sk-a"]) + add_key(cfg, "sk-b") + assert cfg.api_keys == ["sk-a", "sk-b"] + add_key(cfg, "sk-b") # duplicate is a no-op + assert cfg.api_keys == ["sk-a", "sk-b"] + assert cfg.api_key == "sk-a" # active unchanged + + +def test_add_key_seeds_active_when_empty(): + cfg = AIConfig(provider="deepseek", api_key="", api_keys=[]) + add_key(cfg, "sk-a") + assert cfg.api_keys == ["sk-a"] + assert cfg.api_key == "sk-a" + + +def test_remove_active_key_promotes_next(): + cfg = AIConfig(provider="deepseek", api_key="sk-a", api_keys=["sk-a", "sk-b"]) + removed = remove_key(cfg, 0) + assert removed == "sk-a" + assert cfg.api_keys == ["sk-b"] + assert cfg.api_key == "sk-b" + + +def test_remove_nonactive_key_keeps_active(): + cfg = AIConfig(provider="deepseek", api_key="sk-a", api_keys=["sk-a", "sk-b"]) + removed = remove_key(cfg, 1) + assert removed == "sk-b" + assert cfg.api_keys == ["sk-a"] + assert cfg.api_key == "sk-a" + + +def test_remove_key_bad_index_raises(): + cfg = AIConfig(api_keys=["sk-a"]) + with pytest.raises(IndexError): + remove_key(cfg, 5) + + +def test_set_active_key_moves_to_front(): + cfg = AIConfig(provider="deepseek", api_key="sk-a", api_keys=["sk-a", "sk-b"]) + set_active_key(cfg, "sk-b") + assert cfg.api_keys == ["sk-b", "sk-a"] + assert cfg.api_key == "sk-b" + + +def test_set_active_key_new_key_inserts_front(): + cfg = AIConfig(provider="deepseek", api_key="sk-a", api_keys=["sk-a"]) + set_active_key(cfg, "sk-new") + assert cfg.api_keys == ["sk-new", "sk-a"] + assert cfg.api_key == "sk-new" + + +def test_mask_key(): + assert mask_key("sk-1234567890abcd") == "sk-12…abcd" + assert mask_key("short") == "sho…" + assert mask_key("") == "(empty)" +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `pytest tests/test_config.py -k "add_key or remove or set_active or mask" -v` +Expected: FAIL — `ImportError: cannot import name 'add_key'`. + +- [ ] **Step 3: Implement** + +Add to `hackbot/config.py` (just below `reconcile_keys`): + +```python +def add_key(cfg: "AIConfig", key: str) -> None: + """Append *key* to the pool if not already present; keep ``api_key`` valid.""" + key = key.strip() + if not key or key in cfg.api_keys: + return + cfg.api_keys.append(key) + if not cfg.api_key: + cfg.api_key = cfg.api_keys[0] + + +def remove_key(cfg: "AIConfig", index: int) -> str: + """Remove and return the pool entry at *index*. Raises IndexError if invalid. + + If the removed key was the active one, the new active key becomes the new + ``api_keys[0]`` (or empty when the pool is exhausted). + """ + removed = cfg.api_keys.pop(index) # raises IndexError on bad index + cfg.api_key = cfg.api_keys[0] if cfg.api_keys else "" + return removed + + +def set_active_key(cfg: "AIConfig", key: str) -> None: + """Make *key* the active key by moving/inserting it at the front of the pool.""" + key = key.strip() + if not key: + return + if key in cfg.api_keys: + cfg.api_keys.remove(key) + cfg.api_keys.insert(0, key) + cfg.api_key = key + + +def mask_key(key: str) -> str: + """Return a display-safe masked form of an API key.""" + if not key: + return "(empty)" + if len(key) <= 8: + return key[:3] + "…" + return f"{key[:5]}…{key[-4:]}" +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `pytest tests/test_config.py -v` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add hackbot/config.py tests/test_config.py +git commit -m "$(printf 'feat(config): add pool helpers and mask_key\n\nCo-Authored-By: Claude Opus 4.8 ')" +``` + +--- + +## Task 3: Engine — failure classifier & `KeyPoolExhaustedError` + +**Files:** +- Modify: `hackbot/core/engine.py` +- Test: `tests/test_engine.py` + +- [ ] **Step 1: Write the failing tests** + +Add to `tests/test_engine.py` (extend the `hackbot.core.engine` import to include `KeyPoolExhaustedError`): + +```python +from hackbot.core.engine import KeyPoolExhaustedError + + +class TestKeyFailureReason: + """AIEngine._key_failure_reason classifies which failures rotate a key.""" + + @pytest.mark.parametrize("message,expected", [ + ("Error code: 401 - Unauthorized", "dead"), + ("Invalid API key provided", "dead"), + ("Error code: 403 Forbidden", "disabled"), + ("Error code: 402 - Insufficient Balance", "out of credit"), + ("Error code: 429 - You exceeded your current quota, insufficient_quota", "out of credit"), + ("Error code: 429 - Rate limit exceeded", None), + ("Error code: 404 - Model not found", None), + ("Error code: 400 - Bad request", None), + ("Connection refused", None), + ("Request timed out", None), + ]) + def test_classifier(self, message, expected): + assert AIEngine._key_failure_reason(Exception(message)) == expected + + def test_classifier_uses_status_code(self): + exc = Exception("payment required") + exc.status_code = 402 + assert AIEngine._key_failure_reason(exc) == "out of credit" +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `pytest tests/test_engine.py -k "KeyFailureReason" -v` +Expected: FAIL — `ImportError: cannot import name 'KeyPoolExhaustedError'`. + +- [ ] **Step 3: Implement** + +In `hackbot/core/engine.py`: + +3a. Change the config import line `from hackbot.config import AIConfig` to: + +```python +from hackbot.config import AIConfig, mask_key +``` + +3b. Add the exception class immediately after the imports (above `# ── Provider Registry`): + +```python +class KeyPoolExhaustedError(RuntimeError): + """Raised when every key in the DeepSeek pool is dead or out of credit.""" +``` + +3c. Add the classifier as a `@staticmethod` on `AIEngine` (place it just above `validate_api_key`): + +```python + @staticmethod + def _key_failure_reason(exc: Exception) -> Optional[str]: + """Classify an API error as a key-rotation reason, or None. + + Returns 'out of credit' | 'dead' | 'disabled' when the *current key* is + unusable, or None for transient / non-key errors (plain rate-limits, + 404 model-not-found, 400, timeouts, connection failures) that rotating + through the pool would not fix. + """ + msg = str(exc).lower() + code = getattr(exc, "status_code", None) + if (code == 402 or "insufficient balance" in msg or "insufficient_quota" in msg + or "exceeded your current quota" in msg): + return "out of credit" + if (code == 401 or "401" in msg or "unauthorized" in msg + or "invalid api key" in msg or "invalid_api_key" in msg): + return "dead" + if code == 403 or "403" in msg or "forbidden" in msg: + return "disabled" + return None +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `pytest tests/test_engine.py -k "KeyFailureReason" -v` +Expected: PASS (12 parametrized cases + status-code case). + +- [ ] **Step 5: Commit** + +```bash +git add hackbot/core/engine.py tests/test_engine.py +git commit -m "$(printf 'feat(engine): add key-failure classifier and KeyPoolExhaustedError\n\nCo-Authored-By: Claude Opus 4.8 ')" +``` + +--- + +## Task 4: Engine — blocking-call failover + +**Files:** +- Modify: `hackbot/core/engine.py` +- Test: `tests/test_engine.py` + +- [ ] **Step 1: Write the failing tests** + +Add to `tests/test_engine.py`: + +```python +class TestBlockingFailover: + @patch("hackbot.core.engine.OpenAI") + def test_rotates_on_out_of_credit(self, mock_openai_cls): + mock_client = MagicMock() + mock_openai_cls.return_value = mock_client + ok = MagicMock() + ok.choices = [MagicMock()] + ok.choices[0].message.content = "hi from key2" + mock_client.chat.completions.create.side_effect = [ + Exception("Error code: 402 - Insufficient Balance"), + ok, + ] + cfg = AIConfig(provider="deepseek", model="deepseek-chat", + api_key="sk-1", api_keys=["sk-1", "sk-2"]) + engine = AIEngine(cfg) + notices = [] + engine.on_notice = notices.append + + result = engine.chat(create_conversation("chat"), stream=False) + + assert result == "hi from key2" + assert cfg.api_key == "sk-2" + assert mock_client.chat.completions.create.call_count == 2 + assert notices and "switched to key #2" in notices[0] + + @patch("hackbot.core.engine.OpenAI") + def test_raises_when_all_keys_exhausted(self, mock_openai_cls): + mock_client = MagicMock() + mock_openai_cls.return_value = mock_client + mock_client.chat.completions.create.side_effect = [ + Exception("Error code: 402 - Insufficient Balance"), + Exception("Error code: 401 - Unauthorized"), + ] + cfg = AIConfig(provider="deepseek", model="deepseek-chat", + api_key="sk-1", api_keys=["sk-1", "sk-2"]) + engine = AIEngine(cfg) + + with pytest.raises(KeyPoolExhaustedError) as info: + engine.chat(create_conversation("chat"), stream=False) + assert "exhausted" in str(info.value).lower() + + @patch("hackbot.core.engine.OpenAI") + def test_no_failover_for_non_deepseek(self, mock_openai_cls): + mock_client = MagicMock() + mock_openai_cls.return_value = mock_client + mock_client.chat.completions.create.side_effect = Exception( + "Error code: 402 - Insufficient Balance" + ) + cfg = AIConfig(provider="openai", model="gpt-4o", + api_key="sk-1", api_keys=["sk-1", "sk-2"]) + engine = AIEngine(cfg) + + with pytest.raises(Exception) as info: + engine.chat(create_conversation("chat"), stream=False) + assert "402" in str(info.value) + assert mock_client.chat.completions.create.call_count == 1 +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `pytest tests/test_engine.py -k "BlockingFailover" -v` +Expected: FAIL — the second `create` is never retried (`call_count == 1`), so `test_rotates_on_out_of_credit` errors out of `_blocking_chat`. + +- [ ] **Step 3: Implement** + +3a. In `AIEngine.__init__`, after `self._setup_client()`, add rotation state: + +```python + self.on_notice: Optional[Callable[[str], None]] = None + self._bad: Dict[str, str] = {} # session-only: key -> failure reason +``` + +3b. Add these members to `AIEngine` (place above `chat`): + +```python + @property + def _failover_enabled(self) -> bool: + return self.config.provider == "deepseek" and len([k for k in self.config.api_keys if k]) > 1 + + @property + def bad_keys(self) -> Dict[str, str]: + """Session-only map of pool keys that have failed → reason.""" + return dict(self._bad) + + def _key_index(self, key: str) -> int: + try: + return self.config.api_keys.index(key) + 1 + except ValueError: + return 0 + + def _next_good_key(self) -> Optional[str]: + for k in self.config.api_keys: + if k and k not in self._bad: + return k + return None + + def _activate(self, key: str) -> None: + self.config.api_key = key + self._setup_client() + + def _exhausted_message(self) -> str: + lines = [f" • {mask_key(k)}: {self._bad.get(k, 'unknown')}" + for k in self.config.api_keys if k] + return ("All DeepSeek keys are exhausted or invalid:\n" + + "\n".join(lines) + + "\n\nAdd a working key with: /key add ") + + def _maybe_rotate(self, exc: Exception) -> bool: + """Rotate to the next good key after a key-related failure. + + Returns True if rotation happened (caller should retry), False if the + error is transient / not key-related or failover is disabled. Raises + KeyPoolExhaustedError when no usable key remains. + """ + if not self._failover_enabled: + return False + reason = self._key_failure_reason(exc) + if reason is None: + return False + bad_key = self.config.api_key + self._bad[bad_key] = reason + nxt = self._next_good_key() + if nxt is None: + raise KeyPoolExhaustedError(self._exhausted_message()) from exc + prev_n = self._key_index(bad_key) + self._activate(nxt) + if self.on_notice: + self.on_notice(f"DeepSeek key #{prev_n} {reason} — switched to key #{self._key_index(nxt)}") + return True +``` + +3c. Replace the body of `_blocking_chat` with a failover loop: + +```python + def _blocking_chat(self, messages: List[Dict[str, str]]) -> str: + while True: + try: + response = self.client.chat.completions.create( + model=self.config.model, + messages=messages, + temperature=self.config.temperature, + max_tokens=self.config.max_tokens, + ) + return response.choices[0].message.content or "" + except Exception as exc: + if not self._maybe_rotate(exc): + raise +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `pytest tests/test_engine.py -k "BlockingFailover" -v` +Expected: PASS (3 tests). + +- [ ] **Step 5: Commit** + +```bash +git add hackbot/core/engine.py tests/test_engine.py +git commit -m "$(printf 'feat(engine): DeepSeek key failover for blocking calls\n\nCo-Authored-By: Claude Opus 4.8 ')" +``` + +--- + +## Task 5: Engine — streaming failover + +**Files:** +- Modify: `hackbot/core/engine.py` +- Test: `tests/test_engine.py` + +- [ ] **Step 1: Write the failing tests** + +Add to `tests/test_engine.py`: + +```python +def _chunk(text): + c = MagicMock() + c.choices = [MagicMock()] + c.choices[0].delta.content = text + return c + + +class TestStreamingFailover: + @patch("hackbot.core.engine.OpenAI") + def test_retries_before_first_token(self, mock_openai_cls): + mock_client = MagicMock() + mock_openai_cls.return_value = mock_client + mock_client.chat.completions.create.side_effect = [ + Exception("Error code: 402 - Insufficient Balance"), + iter([_chunk("hi"), _chunk(" there")]), + ] + cfg = AIConfig(provider="deepseek", model="deepseek-chat", + api_key="sk-1", api_keys=["sk-1", "sk-2"]) + engine = AIEngine(cfg) + tokens = [] + + result = engine.chat(create_conversation("chat"), stream=True, on_token=tokens.append) + + assert result == "hi there" + assert tokens == ["hi", " there"] + assert cfg.api_key == "sk-2" + + @patch("hackbot.core.engine.OpenAI") + def test_no_retry_after_first_token(self, mock_openai_cls): + mock_client = MagicMock() + mock_openai_cls.return_value = mock_client + + def gen(): + yield _chunk("partial") + raise Exception("Error code: 402 - Insufficient Balance") + + mock_client.chat.completions.create.return_value = gen() + cfg = AIConfig(provider="deepseek", model="deepseek-chat", + api_key="sk-1", api_keys=["sk-1", "sk-2"]) + engine = AIEngine(cfg) + tokens = [] + + with pytest.raises(Exception) as info: + engine.chat(create_conversation("chat"), stream=True, on_token=tokens.append) + + assert "402" in str(info.value) + assert tokens == ["partial"] + assert mock_client.chat.completions.create.call_count == 1 +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `pytest tests/test_engine.py -k "StreamingFailover" -v` +Expected: FAIL — `test_retries_before_first_token` raises instead of rotating. + +- [ ] **Step 3: Implement** + +Replace the body of `_stream_chat` with a failover loop that only retries before the first token is emitted: + +```python + def _stream_chat( + self, + messages: List[Dict[str, str]], + on_token: Callable[[str], None], + ) -> str: + while True: + emitted = False + parts: List[str] = [] + try: + stream = self.client.chat.completions.create( + model=self.config.model, + messages=messages, + temperature=self.config.temperature, + max_tokens=self.config.max_tokens, + stream=True, + ) + for chunk in stream: + if chunk.choices and chunk.choices[0].delta.content: + token = chunk.choices[0].delta.content + emitted = True + parts.append(token) + on_token(token) + return "".join(parts) + except Exception as exc: + # Already streamed output — retrying would duplicate it. + if emitted: + raise + if not self._maybe_rotate(exc): + raise + # rotated to a fresh key — retry the request from the top +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `pytest tests/test_engine.py -v` +Expected: PASS (all engine tests, including the 13 existing `validate_api_key` tests). + +- [ ] **Step 5: Commit** + +```bash +git add hackbot/core/engine.py tests/test_engine.py +git commit -m "$(printf 'feat(engine): DeepSeek key failover for streaming calls\n\nCo-Authored-By: Claude Opus 4.8 ')" +``` + +--- + +## Task 6: CLI — `_rebuild_engine` helper & notice wiring + +**Files:** +- Modify: `hackbot/cli.py` + +This refactor introduces one helper that rebuilds the engine *and* wires the rotation notice to `print_warning`, then routes the two existing rebuild sites through it. (`_set_key` is rewritten in Task 7 and will use the same helper.) + +- [ ] **Step 1: Add the helper** + +In `hackbot/cli.py`, add this method to `HackBotApp` (place it just above `_show_config`): + +```python + def _rebuild_engine(self) -> None: + """Rebuild the AI engine from current config and re-wire it to all modes.""" + self.engine = AIEngine(self.config.ai) + self.engine.on_notice = lambda msg: print_warning(msg) + self.chat.engine = self.engine + self.plan.engine = self.engine + if self.agent: + self.agent.engine = self.engine + self.agent.summarizer.engine = self.engine +``` + +- [ ] **Step 2: Wire the notice on the initial engine** + +In `HackBotApp.__init__`, immediately after `self._start_time = time.time()`, add: + +```python + self.engine.on_notice = lambda msg: print_warning(msg) +``` + +- [ ] **Step 3: Route `_set_model` through the helper** + +In `_set_model`, replace these lines: + +```python + self.config.ai.model = model + self.engine = AIEngine(self.config.ai) + # Update all modes with new engine + self.chat.engine = self.engine + self.plan.engine = self.engine + if self.agent: + self.agent.engine = self.engine + self.agent.summarizer.engine = self.engine + save_config(self.config) +``` + +with: + +```python + self.config.ai.model = model + self._rebuild_engine() + save_config(self.config) +``` + +- [ ] **Step 4: Route `_set_provider` through the helper** + +In `_set_provider`, replace these lines: + +```python + self.engine = AIEngine(self.config.ai) + self.chat.engine = self.engine + self.plan.engine = self.engine + if self.agent: + self.agent.engine = self.engine + self.agent.summarizer.engine = self.engine + save_config(self.config) +``` + +with: + +```python + self._rebuild_engine() + save_config(self.config) +``` + +- [ ] **Step 5: Verify nothing regressed** + +Run: `python -c "from hackbot.cli import HackBotApp; from hackbot.config import HackBotConfig; a = HackBotApp(HackBotConfig()); print('on_notice wired:', callable(a.engine.on_notice))"` +Expected: `on_notice wired: True` + +Run: `pytest tests/ -q` +Expected: PASS (no regressions). + +- [ ] **Step 6: Commit** + +```bash +git add hackbot/cli.py +git commit -m "$(printf 'refactor(cli): add _rebuild_engine helper and wire rotation notices\n\nCo-Authored-By: Claude Opus 4.8 ')" +``` + +--- + +## Task 7: CLI — `/key` subcommands, `/keys`, and help text + +**Files:** +- Modify: `hackbot/cli.py`, `hackbot/ui/__init__.py` +- Test: `tests/test_keypool_cli.py` (create) + +- [ ] **Step 1: Write the failing tests** + +Create `tests/test_keypool_cli.py`: + +```python +"""Tests for the DeepSeek key-pool REPL commands.""" + +from unittest.mock import MagicMock, patch + +import pytest + +from hackbot.cli import HackBotApp +from hackbot.config import HackBotConfig + + +def _app(provider="deepseek", keys=None, active=""): + cfg = HackBotConfig() + cfg.ai.provider = provider + cfg.ai.model = "deepseek-chat" if provider == "deepseek" else "gpt-4o" + cfg.ai.api_keys = list(keys or []) + cfg.ai.api_key = active or (cfg.ai.api_keys[0] if cfg.ai.api_keys else "") + return HackBotApp(cfg) + + +@patch("hackbot.cli.save_config") +@patch("hackbot.core.engine.OpenAI") +def test_key_add_appends(mock_openai, mock_save): + mock_client = MagicMock() + mock_openai.return_value = mock_client + mock_client.chat.completions.create.return_value = MagicMock(model="deepseek-chat") + app = _app(keys=["sk-1"], active="sk-1") + + app._set_key("add sk-2") + + assert app.config.ai.api_keys == ["sk-1", "sk-2"] + assert mock_save.called + + +@patch("hackbot.cli.save_config") +@patch("hackbot.core.engine.OpenAI") +def test_key_add_guarded_on_non_deepseek(mock_openai, mock_save): + mock_openai.return_value = MagicMock() + app = _app(provider="openai", keys=["sk-1"], active="sk-1") + + app._set_key("add sk-2") + + assert app.config.ai.api_keys == ["sk-1"] # unchanged + assert not mock_save.called + + +@patch("hackbot.cli.save_config") +@patch("hackbot.core.engine.OpenAI") +def test_key_remove(mock_openai, mock_save): + mock_openai.return_value = MagicMock() + app = _app(keys=["sk-1", "sk-2"], active="sk-1") + + app._set_key("remove 1") + + assert app.config.ai.api_keys == ["sk-2"] + assert app.config.ai.api_key == "sk-2" + + +@patch("hackbot.cli.save_config") +@patch("hackbot.core.engine.OpenAI") +def test_bare_key_sets_active_front(mock_openai, mock_save): + mock_client = MagicMock() + mock_openai.return_value = mock_client + mock_client.chat.completions.create.return_value = MagicMock(model="deepseek-chat") + app = _app(keys=["sk-1", "sk-2"], active="sk-1") + + app._set_key("sk-2") + + assert app.config.ai.api_keys == ["sk-2", "sk-1"] + assert app.config.ai.api_key == "sk-2" + + +@patch("hackbot.cli.save_config") +@patch("hackbot.core.engine.OpenAI") +def test_list_keys_runs(mock_openai, mock_save): + mock_openai.return_value = MagicMock() + app = _app(keys=["sk-1", "sk-2"], active="sk-1") + + assert app._list_keys() is True # does not raise +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `pytest tests/test_keypool_cli.py -v` +Expected: FAIL — `_set_key("add sk-2")` currently treats `"add sk-2"` as a literal key; `_list_keys` does not exist. + +- [ ] **Step 3: Extend the config import in `cli.py`** + +Change the existing `from hackbot.config import (...)` block to add the four new names: + +```python +from hackbot.config import ( + CONFIG_DIR, + HackBotConfig, + AIConfig, + add_key, + detect_platform, + detect_tools, + load_config, + mask_key, + remove_key, + save_config, + set_active_key, +) +``` + +- [ ] **Step 4: Rewrite `_set_key` with subcommand routing** + +Replace the entire existing `_set_key` method with: + +```python + def _set_key(self, args: str) -> bool: + parts = args.strip().split(maxsplit=1) + sub = parts[0].lower() if parts else "" + rest = parts[1].strip() if len(parts) > 1 else "" + + if sub == "add": + return self._key_add(rest) + if sub == "remove": + return self._key_remove(rest) + if sub == "list": + return self._list_keys() + + key = args.strip() + if not key: + print_error( + "Usage: /key Set the active API key\n" + " /key add Add a DeepSeek key to the pool\n" + " /key remove Remove DeepSeek key #n\n" + " /keys List the DeepSeek key pool" + ) + return True + + if self.config.ai.provider == "deepseek": + set_active_key(self.config.ai, key) + else: + self.config.ai.api_key = key + self._rebuild_engine() + + print_info("Validating API key...") + result = self.engine.validate_api_key() + if result["valid"]: + save_config(self.config) + print_success(result["message"]) + else: + print_error(result["message"]) + print_warning("API key saved but may not work. Use /key to set a valid key.") + save_config(self.config) + return True + + def _key_add(self, key: str) -> bool: + if self.config.ai.provider != "deepseek": + print_error("Multi-key pooling is DeepSeek-only — switch with /provider deepseek.") + return True + key = key.strip() + if not key: + print_error("Usage: /key add ") + return True + if key in self.config.ai.api_keys: + print_info(f"Key {mask_key(key)} is already in the pool.") + return True + + print_info("Validating API key...") + probe = AIEngine(AIConfig(provider="deepseek", model=self.config.ai.model, api_key=key)) + result = probe.validate_api_key() + if result["valid"]: + print_success(result["message"]) + else: + print_warning(f"{result['message']} — adding anyway.") + + add_key(self.config.ai, key) + self._rebuild_engine() + save_config(self.config) + print_success(f"Added DeepSeek key {mask_key(key)} (pool size: {len(self.config.ai.api_keys)}).") + return True + + def _key_remove(self, arg: str) -> bool: + if self.config.ai.provider != "deepseek": + print_error("Multi-key pooling is DeepSeek-only — switch with /provider deepseek.") + return True + arg = arg.strip() + if not arg.isdigit(): + print_error("Usage: /key remove (see numbers with /keys)") + return True + try: + removed = remove_key(self.config.ai, int(arg) - 1) + except IndexError: + print_error(f"No key #{arg} in the pool (see /keys).") + return True + self._rebuild_engine() + save_config(self.config) + print_success(f"Removed DeepSeek key {mask_key(removed)}.") + if not self.config.ai.api_keys: + print_warning("Key pool is now empty. Add one with /key add .") + return True + + def _list_keys(self) -> bool: + if self.config.ai.provider != "deepseek": + print_info( + "Multi-key pooling is DeepSeek-only. " + f"Active key: {mask_key(self.config.ai.api_key)}" + ) + return True + keys = self.config.ai.api_keys + if not keys: + print_info("No DeepSeek keys configured. Add one with /key add .") + return True + bad = self.engine.bad_keys + console.print("\n[bold]DeepSeek key pool:[/]") + for i, k in enumerate(keys, 1): + active = "[green]●[/]" if k == self.config.ai.api_key else " " + status = f" [red]✗ {bad[k]}[/]" if k in bad else "" + console.print(f" {active} #{i} {mask_key(k)}{status}") + print_info(f"{len(keys)} key(s). ● = active.") + return True +``` + +- [ ] **Step 5: Register `/keys` in the dispatch table** + +In `_handle_command`, in the `commands = { ... }` dict, add this entry directly after the `"/key": ...` line: + +```python + "/keys": lambda: self._list_keys(), +``` + +- [ ] **Step 6: Add the commands to `/help`** + +In `hackbot/ui/__init__.py`, inside `show_help`, replace this line: + +```python + /key Set API key +``` + +with: + +```python + /key Set API key + /keys List DeepSeek key pool (active / bad) + /key add Add a DeepSeek key to the pool + /key remove Remove DeepSeek key #n from the pool +``` + +- [ ] **Step 7: Run tests to verify they pass** + +Run: `pytest tests/test_keypool_cli.py -v` +Expected: PASS (5 tests). + +- [ ] **Step 8: Commit** + +```bash +git add hackbot/cli.py hackbot/ui/__init__.py tests/test_keypool_cli.py +git commit -m "$(printf 'feat(cli): DeepSeek key pool commands (/key add, /keys, /key remove)\n\nCo-Authored-By: Claude Opus 4.8 ')" +``` + +--- + +## Task 8: Full verification + +**Files:** none (verification only) + +- [ ] **Step 1: Full test suite** + +Run: `pytest tests/ -q` +Expected: PASS — all tests green, including the new config/engine/cli pool tests. + +- [ ] **Step 2: Lint & format** + +Run: `ruff check hackbot/ tests/` +Expected: no errors. If import-order (I) issues appear, run `ruff check --fix hackbot/ tests/` and re-run. + +Run: `black hackbot/ tests/` +Expected: files already formatted (or reformatted cleanly). + +- [ ] **Step 3: Type check (non-blocking, as in CI)** + +Run: `mypy hackbot/ --ignore-missing-imports` +Expected: no new errors introduced by these changes. + +- [ ] **Step 4: Manual REPL smoke test** + +With two real DeepSeek keys (one valid, one anything), run `hackbot` and enter: + +```text +/provider deepseek +/key +/key add +/keys +/key remove 2 +/keys +``` + +Expected: +- `/keys` lists the pool masked, with `●` on the active key. +- `/key remove 2` removes the second key; the follow-up `/keys` shows one key. +- On another provider (e.g. `/provider openai` then `/key add x`), the command prints the DeepSeek-only guard message and does not modify anything. + +- [ ] **Step 5: Final commit (only if lint/format changed files)** + +```bash +git add -A +git commit -m "$(printf 'chore: lint/format pass for DeepSeek key failover\n\nCo-Authored-By: Claude Opus 4.8 ')" +``` + +--- + +## Notes / deviations from spec + +- **`validate_api_key()` is left intact** rather than rewritten to call the new + classifier. Its categorization serves a different purpose (user-facing + validation messages: it reports rate-limits as *valid*, and distinguishes + 404/timeout). `_key_failure_reason` is a focused sibling that reads the same + signals (status code + message markers) for rotation decisions, so the two do + not drift. The 13 existing validation tests stay green. +- **No standalone `_with_failover(do_call)` wrapper.** Blocking and streaming + retries have different shapes (streaming must not retry once a token is + emitted), so the shared rotation logic lives in `_maybe_rotate()`, which both + `_blocking_chat` and `_stream_chat` call. This preserves the spec's + single-source-of-truth intent. +- **CLI tests added** (`tests/test_keypool_cli.py`) beyond the spec's + engine+config testing scope — cheap, no-network coverage of the new commands. +- **No `refresh_pool()` method** (the spec named one). Every CLI pool edit calls + `_rebuild_engine()`, which constructs a fresh `AIEngine` with empty bad-key + state, so a separate refresh method would be dead code. The spec's intent — + the engine reflects pool edits made by a command — is preserved. +``` diff --git a/docs/superpowers/specs/2026-06-21-deepseek-multi-key-failover-design.md b/docs/superpowers/specs/2026-06-21-deepseek-multi-key-failover-design.md new file mode 100644 index 0000000..3c6d6c1 --- /dev/null +++ b/docs/superpowers/specs/2026-06-21-deepseek-multi-key-failover-design.md @@ -0,0 +1,212 @@ +# DeepSeek Multi-Key Failover — Design + +- **Date:** 2026-06-21 +- **Status:** Approved (pending spec review) +- **Scope:** DeepSeek provider only + +## Summary + +Let users store a **pool of DeepSeek API keys** so that when the active key +goes **bad** (revoked/invalid) or **runs out** (insufficient balance / quota), +HackBot automatically rotates to the next working key and **retries the same +request transparently**, with a brief notice. Keys are managed from the REPL +(`/key add`, `/keys`, `/key remove`). All other providers behave exactly as +they do today. + +## Motivation + +A single DeepSeek key is a single point of failure: when it is revoked or its +balance hits zero, every request fails until the user notices and manually +swaps the key. A small key pool with automatic failover keeps an assessment or +chat session running without interruption. + +## Goals + +- Store multiple DeepSeek keys, ordered by priority. +- On a rotation-worthy failure, switch to the next untried key and retry the + same request with no user action. +- Manage the pool interactively: add, list, remove. +- Full backward compatibility with existing single-key configs. + +## Non-goals (out of scope) + +- Cross-provider failover (DeepSeek → Groq, etc.). +- Multi-key support for any provider other than DeepSeek. +- Rotating on transient errors (plain rate-limits, network/timeout, model-not-found). +- A GUI management surface for the pool (failover still works in the GUI because + it is engine-level; only the *management UI* is deferred — see Future Work). +- Encrypted key storage (keys remain plaintext in `config.yaml`, unchanged from + today's `api_key`). +- Changing the `hackbot setup` command (it stays single-key and seeds the pool). + +## Requirements (decisions captured during brainstorming) + +1. **Failover scope:** more keys for the **same provider** (DeepSeek), not other providers. +2. **Trigger behavior:** **automatic** rotation **with retry** of the failed request. +3. **What counts as failure worth rotating:** key is **dead** (`401`/`403`) or + **out of credit** (`402` Insufficient Balance, or `429` with + `insufficient_quota`). Plain `429` rate-limits, `404`, `400`, and + network/timeout errors do **not** rotate. +4. **Storage:** a single DeepSeek key pool (DeepSeek-only feature; no per-provider map). +5. **Management:** REPL commands `/key add `, `/keys`, `/key remove `. +6. **Bad-key memory:** session-only. Every run starts fresh with the full pool + (quotas refill, balances get topped up). + +## Design + +### 1. Data model & config (`config.py`) + +- Add **`api_keys: list[str]`** to `AIConfig` (default `[]`): the ordered pool, + highest priority first. Update `DEFAULT_CONFIG["ai"]` and `save_config()` to + round-trip it. +- **Invariant:** `api_key` always mirrors the **active** key (`pool[0]` at + startup). Every existing reader of `config.ai.api_key` keeps working + unchanged — this is what preserves backward compatibility. +- **Reconciliation** at the end of `load_config()`, after env-var overrides so + env-provided keys still win: + + ```python + keys = [k for k in dict.fromkeys([api_key] + api_keys) if k] # dedupe, active-first + merged["ai"]["api_keys"] = keys + merged["ai"]["api_key"] = keys[0] if keys else "" + ``` + + Effects: an old single-key config auto-seeds the pool `[that key]`; a pool + config sets the active key to `pool[0]`; an env-provided key is prepended as + the active key. No migration script required. +- **Pure pool helpers** (testable without the CLI), operating on `AIConfig` and + maintaining the dedupe + active-first invariant: + - `add_key(cfg, key) -> None` + - `remove_key(cfg, index) -> str` (returns removed key; raises on bad index) + - `set_active_key(cfg, key) -> None` (moves/inserts key to front, updates `api_key`) + +### 2. Engine failover (`core/engine.py`) + +- **Gating:** failover engages only when `config.provider == "deepseek"`. Any + other provider performs a single attempt and raises on error — today's + behavior, unchanged. +- **Shared error classifier:** extract the categorization currently inside + `validate_api_key()` into `_key_failure_reason(exc) -> str | None`, used by + both validation and failover so they cannot drift: + + | Condition | Result | + |---|---| + | `status_code == 401` | `"dead"` → rotate | + | `status_code == 403` | `"disabled"` → rotate | + | `status_code == 402` (Insufficient Balance) | `"out of credit"` → rotate | + | `429` with `insufficient_quota` / `insufficient balance` in message | `"out of credit"` → rotate | + | plain `429` rate-limit, `404`, `400`, timeout/connection | `None` → do not rotate, re-raise | + + Classification reads `getattr(exc, "status_code", None)` and falls back to + scanning `str(exc)` for the quota markers. +- **Engine runtime state:** `self._bad: dict[str, str]` (key → reason) tracks + session-bad keys; the active key is `config.api_key`. A `refresh_pool()` method + re-reads the config after the pool is mutated by a command. +- **`_with_failover(do_call, *, on_notice)`** wraps the completion call: + 1. Try `do_call(self.client)`. + 2. On exception, compute `reason = _key_failure_reason(exc)`. If `reason is + None` or failover is not enabled → re-raise. + 3. Mark the active key bad (`self._bad`), pick the next key not in `self._bad`, + set `config.api_key` to it, rebuild via `_setup_client()`. + 4. If no good key remains → raise `KeyPoolExhaustedError` with a masked + per-key summary (`sk-…a1b2: out of credit`) and an "add one with `/key add`" + hint. + 5. Otherwise fire `on_notice(...)` and retry. +- **`_blocking_chat`** runs inside `_with_failover` directly (the call is atomic). +- **Streaming boundary (`_stream_chat`):** auto-retry only if the failure happens + **before the first token** is emitted (exactly when auth/quota errors fire). If + any token already streamed, re-raise rather than duplicating output: + + ```python + emitted = False + try: + stream = self.client.chat.completions.create(..., stream=True) + for chunk in stream: + tok = ... + if tok: + emitted = True + on_token(tok) + except Exception as e: + if emitted or _key_failure_reason(e) is None or not failover_enabled: + raise + # rotate to next key (or raise KeyPoolExhaustedError) and retry from the top + ``` + +- **Notice plumbing (UI-agnostic):** `AIEngine` gains an + `on_notice: Callable[[str], None] | None` attribute (no signature change to + modes). The CLI sets it once to `print_warning` via a `_wire_engine()` helper, + re-applied wherever the engine is rebuilt (`/key`, `/model`, `/provider`). The + GUI may set its own sink. Message form: + `DeepSeek key #1 out of credit — switched to key #2`. +- **`KeyPoolExhaustedError`** is a `RuntimeError` subclass so existing CLI/agent + error handling surfaces it normally, while tests can assert on its type. + +### 3. REPL key-management commands (`cli.py`) + +CLI handlers are thin wrappers over the `config.py` pool helpers; after any +mutation they call `save_config()` and re-wire the engine. + +- **`/key `** — unchanged entry point. Sets the active key; for DeepSeek the + key is moved/inserted to the front of the pool. A real key is `sk-…`, so it + never collides with the subcommand words below. Routing lives in `_set_key`: + if the first token is `add` / `remove` / `list`, dispatch to the pool handler; + otherwise treat the whole argument as a key (today's behavior). +- **`/key add `** — validate via `validate_api_key`, then append to the pool + (dedup). Warn-but-add on failed validation (consistent with today's `_set_key`, + which saves even when validation fails). +- **`/key remove `** — remove pool entry `#n` (1-indexed). If it was active, + the new active becomes `pool[0]` and the client rebuilds. Out-of-range or + non-numeric → usage error. +- **`/keys`** — new entry in the command dispatch table; lists the pool masked + (`sk-…a1b2`), marking the active key (`●`) and any session-bad keys + (`✗ `), plus the count. `/key list` aliases to it. +- **Scope guard:** pool commands are DeepSeek-only. On another provider, + `/key add` prints `Multi-key pooling is DeepSeek-only — switch with /provider + deepseek`; `/keys` notes the same and shows the single active key. `/key + ` works on every provider. + +### 4. Error handling & edge cases + +- **All keys exhausted mid-session** → `KeyPoolExhaustedError` propagates through + `_handle_message` / the agent loop, printing the masked summary + `/key add` hint. +- **Removing the last key** → empty pool → falls back to today's + "API key not configured" messaging. +- **Duplicate add** → no-op with a note. +- **Adding an invalid key** → warn but still add (mirrors current behavior so a + transient validation failure does not block the user). +- **Non-DeepSeek provider** → pool is ignored by the engine; single-attempt + behavior, byte-for-byte unchanged. +- The rotation notice prints before the response streams (failover happens before + the first token), so it never interleaves with streamed output. + +## Testing + +- **`tests/test_engine.py`** + - `_key_failure_reason` table: `401`/`403`/`402`/`429+insufficient_quota` → + rotate reasons; plain `429`, `404`, connection → `None`. + - Failover rotates: mocked client raises `402` on key 1, succeeds on key 2 → + asserts retry happened, notice fired, key 2's content returned. + - All-exhausted → `KeyPoolExhaustedError` with masked summary. + - **Non-DeepSeek provider:** `402` raises immediately, no rotation. + - Streaming: failure before first token rotates and retries; failure after a + token re-raises (no duplicate output). + - All via a mocked OpenAI client — no network calls. +- **`tests/test_config.py`** + - Reconciliation: single key → pool seeded; pool → `api_key == pool[0]`; + dedupe + active-first; env key prepended. + - `api_keys` save/load round-trip. + - `add_key` / `remove_key` / `set_active_key` helpers (including invariant and + bad-index handling). + +## Backward compatibility + +- Existing `config.yaml` files with a single `ai.api_key` load unchanged and are + silently upgraded to a one-entry pool on first load. +- `config.ai.api_key` keeps its meaning (the active key) for every existing reader. +- `/key `, `hackbot setup`, and all provider env vars keep working as before. + +## Future work (explicitly deferred) + +- GUI management UI for the pool (`/api/config` could accept `api_keys`). +- Optional comma-separated bulk entry (`DEEPSEEK_API_KEY=k1,k2,k3`, `setup`). +- Generalizing the pool to other providers, if ever wanted. diff --git a/hackbot/cli.py b/hackbot/cli.py index c0c720f..0ef574e 100644 --- a/hackbot/cli.py +++ b/hackbot/cli.py @@ -74,6 +74,12 @@ load_dotenv() +PROVIDER_ALIASES = { + "deep": "deepseek", + "deepseek": "deepseek", +} + + def _should_block_root_gui_launch() -> bool: """Return True if GUI launch should be blocked due to root/sudo context. @@ -134,6 +140,14 @@ def _on_agent_step(self, step) -> None: elif step.action == "report": print_success(step.description) + def _rebuild_ai_engine(self) -> None: + self.engine = AIEngine(self.config.ai) + self.chat.engine = self.engine + self.plan.engine = self.engine + if self.agent: + self.agent.engine = self.engine + self.agent.summarizer.engine = self.engine + def _on_confirm(self, command: str, reason: str) -> bool: """Handle confirmation for risky commands.""" return confirm_action(command, reason) @@ -492,15 +506,35 @@ def _check_update(self, args: str) -> bool: def _set_key(self, key: str) -> bool: if not key: - print_error("Usage: /key ") + print_error("Usage: /key or /key deepseek ") + return True + + key_parts = key.split(maxsplit=1) + requested_provider = PROVIDER_ALIASES.get(key_parts[0].lower()) + if requested_provider: + if len(key_parts) == 1: + print_error(f"Usage: /key {key_parts[0]} ") + print_info(f"Or run: /provider {requested_provider} then /key ") + return True + + preset = PROVIDERS[requested_provider] + self.config.ai.provider = requested_provider + if preset["models"]: + self.config.ai.model = preset["models"][0]["id"] + self.config.ai.base_url = "" + key = key_parts[1].strip() + if not key: + print_error(f"Usage: /key {key_parts[0]} ") + return True + print_info(f"Provider: {preset['name']}") + + if key.lower() in PROVIDER_ALIASES: + print_error(f"Usage: /key {key} ") + print_info(f"Or run: /provider {PROVIDER_ALIASES[key.lower()]} then /key ") return True + self.config.ai.api_key = key - self.engine = AIEngine(self.config.ai) - self.chat.engine = self.engine - self.plan.engine = self.engine - if self.agent: - self.agent.engine = self.engine - self.agent.summarizer.engine = self.engine + self._rebuild_ai_engine() # Validate the key before saving print_info("Validating API key...") @@ -730,12 +764,7 @@ def _set_provider(self, provider: str) -> bool: self.config.ai.model = preset["models"][0]["id"] # Clear base_url so engine uses preset self.config.ai.base_url = "" - self.engine = AIEngine(self.config.ai) - self.chat.engine = self.engine - self.plan.engine = self.engine - if self.agent: - self.agent.engine = self.engine - self.agent.summarizer.engine = self.engine + self._rebuild_ai_engine() save_config(self.config) print_success( f"Provider: {preset['name']}\n" diff --git a/hackbot/config.py b/hackbot/config.py index 7024a48..53d3870 100644 --- a/hackbot/config.py +++ b/hackbot/config.py @@ -41,6 +41,7 @@ def ensure_dirs() -> None: "provider": "openai", "model": "gpt-4o", "api_key": "", + "api_keys": [], "base_url": "", "temperature": 0.2, "max_tokens": 4096, @@ -165,6 +166,7 @@ class AIConfig: provider: str = "openai" model: str = "gpt-4o" api_key: str = "" + api_keys: List[str] = field(default_factory=list) base_url: str = "" temperature: float = 0.2 max_tokens: int = 4096 @@ -222,6 +224,62 @@ class HackBotConfig: rag: RAGConfig = field(default_factory=RAGConfig) +def reconcile_keys(ai: Dict[str, Any]) -> None: + """Normalize the API key pool in an ``ai`` config dict, in place. + + Dedupes, drops empties, and keeps the active key first so the invariant + ``api_key == api_keys[0]`` always holds. Seeds the pool from a legacy + single ``api_key`` and lets an env-provided ``api_key`` take priority by + placing it first. + """ + pool = [k for k in dict.fromkeys([ai.get("api_key", "")] + list(ai.get("api_keys") or [])) if k] + ai["api_keys"] = pool + ai["api_key"] = pool[0] if pool else "" + + +def add_key(cfg: "AIConfig", key: str) -> None: + """Append *key* to the pool if not already present; keep ``api_key`` valid.""" + key = key.strip() + if not key or key in cfg.api_keys: + return + cfg.api_keys.append(key) + if not cfg.api_key: + cfg.api_key = cfg.api_keys[0] + + +def remove_key(cfg: "AIConfig", index: int) -> str: + """Remove and return the pool entry at *index*. Raises IndexError if invalid. + + If the removed key was the active one, the new active key becomes the new + ``api_keys[0]`` (or empty when the pool is exhausted). + """ + if index < 0 or index >= len(cfg.api_keys): + raise IndexError("API key index out of range") + removed = cfg.api_keys.pop(index) + cfg.api_key = cfg.api_keys[0] if cfg.api_keys else "" + return removed + + +def set_active_key(cfg: "AIConfig", key: str) -> None: + """Make *key* the active key by moving/inserting it at the front of the pool.""" + key = key.strip() + if not key: + return + if key in cfg.api_keys: + cfg.api_keys.remove(key) + cfg.api_keys.insert(0, key) + cfg.api_key = key + + +def mask_key(key: str) -> str: + """Return a display-safe masked form of an API key.""" + if not key: + return "(empty)" + if len(key) <= 8: + return key[:3] + "…" + return f"{key[:5]}…{key[-4:]}" + + def load_config() -> HackBotConfig: """Load configuration from disk, env vars, and defaults.""" ensure_dirs() @@ -305,6 +363,8 @@ def load_config() -> HackBotConfig: DEFAULT_CONFIG["agent"]["allowed_tools"], ) + reconcile_keys(merged.setdefault("ai", {})) + cfg = HackBotConfig( ai=AIConfig(**merged.get("ai", {})), agent=AgentConfig(**merged.get("agent", {})), @@ -324,6 +384,7 @@ def save_config(cfg: HackBotConfig) -> None: "provider": cfg.ai.provider, "model": cfg.ai.model, "api_key": cfg.ai.api_key, + "api_keys": cfg.ai.api_keys, "base_url": cfg.ai.base_url, "temperature": cfg.ai.temperature, "max_tokens": cfg.ai.max_tokens, diff --git a/tests/test_cli.py b/tests/test_cli.py new file mode 100644 index 0000000..42aed64 --- /dev/null +++ b/tests/test_cli.py @@ -0,0 +1,39 @@ +"""Tests for HackBot CLI command handling.""" + +from hackbot.cli import HackBotApp +from hackbot.config import HackBotConfig + + +def test_key_deepseek_prefix_switches_provider_before_validation(monkeypatch): + saved = {} + + def fake_validate(self): + saved["provider_at_validation"] = self.config.provider + saved["model_at_validation"] = self.config.model + saved["api_key_at_validation"] = self.config.api_key + return {"valid": True, "message": "ok"} + + monkeypatch.setattr("hackbot.cli.save_config", lambda cfg: saved.setdefault("saved", cfg)) + monkeypatch.setattr("hackbot.core.engine.AIEngine.validate_api_key", fake_validate) + + app = HackBotApp(HackBotConfig()) + + assert app._set_key("deepseek sk-deepseek-test") is True + assert app.config.ai.provider == "deepseek" + assert app.config.ai.model == "deepseek-chat" + assert app.config.ai.api_key == "sk-deepseek-test" + assert saved["provider_at_validation"] == "deepseek" + assert saved["model_at_validation"] == "deepseek-chat" + assert saved["api_key_at_validation"] == "sk-deepseek-test" + + +def test_key_deep_alias_requires_key_and_does_not_save(monkeypatch): + saved = {"called": False} + monkeypatch.setattr("hackbot.cli.save_config", lambda cfg: saved.update(called=True)) + + app = HackBotApp(HackBotConfig()) + + assert app._set_key("deep") is True + assert app.config.ai.provider == "openai" + assert app.config.ai.api_key == "" + assert saved["called"] is False diff --git a/tests/test_config.py b/tests/test_config.py index 300121c..4e81e35 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -17,6 +17,12 @@ _deep_merge, _merge_allowed_tools, load_config, + reconcile_keys, + save_config, + add_key, + remove_key, + set_active_key, + mask_key, ) @@ -153,3 +159,128 @@ def test_load_config_migrates_allowed_tools_from_old_file(tmp_path, monkeypatch) assert "msfconsole" in cfg.agent.allowed_tools assert "wifite" in cfg.agent.allowed_tools assert "aircrack-ng" in cfg.agent.allowed_tools + + +def test_reconcile_seeds_pool_from_single_key(): + ai = {"api_key": "sk-a", "api_keys": []} + reconcile_keys(ai) + assert ai["api_keys"] == ["sk-a"] + assert ai["api_key"] == "sk-a" + + +def test_reconcile_active_is_pool_first(): + ai = {"api_key": "", "api_keys": ["sk-a", "sk-b"]} + reconcile_keys(ai) + assert ai["api_key"] == "sk-a" + assert ai["api_keys"][0] == ai["api_key"] + + +def test_reconcile_env_key_prepended_and_deduped(): + ai = {"api_key": "sk-env", "api_keys": ["sk-a", "sk-env", "sk-b"]} + reconcile_keys(ai) + assert ai["api_keys"] == ["sk-env", "sk-a", "sk-b"] + assert ai["api_key"] == "sk-env" + + +def test_reconcile_empty_pool(): + ai = {"api_key": "", "api_keys": []} + reconcile_keys(ai) + assert ai["api_keys"] == [] + assert ai["api_key"] == "" + + +def test_api_keys_round_trip(tmp_path, monkeypatch): + import hackbot.config as cfgmod + monkeypatch.setattr(cfgmod, "CONFIG_FILE", tmp_path / "config.yaml") + for var in ( + "DEEPSEEK_API_KEY", "OPENAI_API_KEY", "HACKBOT_API_KEY", "ANTHROPIC_API_KEY", + "GEMINI_API_KEY", "GOOGLE_API_KEY", "GROQ_API_KEY", + "MISTRAL_API_KEY", "TOGETHER_API_KEY", "OPENROUTER_API_KEY", + ): + monkeypatch.delenv(var, raising=False) + + cfg = cfgmod.HackBotConfig() + cfg.ai.provider = "deepseek" + cfg.ai.api_keys = ["sk-a", "sk-b"] + cfg.ai.api_key = "sk-a" + cfgmod.save_config(cfg) + + loaded = cfgmod.load_config() + assert loaded.ai.api_keys == ["sk-a", "sk-b"] + assert loaded.ai.api_key == "sk-a" + + +def test_add_key_appends_and_dedupes(): + cfg = AIConfig(provider="deepseek", api_key="sk-a", api_keys=["sk-a"]) + add_key(cfg, "sk-b") + assert cfg.api_keys == ["sk-a", "sk-b"] + add_key(cfg, "sk-b") # duplicate is a no-op + assert cfg.api_keys == ["sk-a", "sk-b"] + assert cfg.api_key == "sk-a" # active unchanged + + +def test_add_key_seeds_active_when_empty(): + cfg = AIConfig(provider="deepseek", api_key="", api_keys=[]) + add_key(cfg, "sk-a") + assert cfg.api_keys == ["sk-a"] + assert cfg.api_key == "sk-a" + + +def test_remove_active_key_promotes_next(): + cfg = AIConfig(provider="deepseek", api_key="sk-a", api_keys=["sk-a", "sk-b"]) + removed = remove_key(cfg, 0) + assert removed == "sk-a" + assert cfg.api_keys == ["sk-b"] + assert cfg.api_key == "sk-b" + + +def test_remove_nonactive_key_keeps_active(): + cfg = AIConfig(provider="deepseek", api_key="sk-a", api_keys=["sk-a", "sk-b"]) + removed = remove_key(cfg, 1) + assert removed == "sk-b" + assert cfg.api_keys == ["sk-a"] + assert cfg.api_key == "sk-a" + + +def test_remove_key_bad_index_raises(): + cfg = AIConfig(api_keys=["sk-a"]) + with pytest.raises(IndexError): + remove_key(cfg, 5) + + +def test_remove_key_negative_index_raises(): + # /key remove 0 maps to index -1; that must error, not pop the last key. + cfg = AIConfig(api_keys=["sk-a", "sk-b"]) + with pytest.raises(IndexError): + remove_key(cfg, -1) + assert cfg.api_keys == ["sk-a", "sk-b"] # unchanged + + +def test_remove_last_key_empties_pool(): + cfg = AIConfig(api_key="sk-a", api_keys=["sk-a"]) + remove_key(cfg, 0) + assert cfg.api_keys == [] + assert cfg.api_key == "" + + +def test_set_active_key_moves_to_front(): + cfg = AIConfig(provider="deepseek", api_key="sk-a", api_keys=["sk-a", "sk-b"]) + set_active_key(cfg, "sk-b") + assert cfg.api_keys == ["sk-b", "sk-a"] + assert cfg.api_key == "sk-b" + + +def test_set_active_key_new_key_inserts_front(): + cfg = AIConfig(provider="deepseek", api_key="sk-a", api_keys=["sk-a"]) + set_active_key(cfg, "sk-new") + assert cfg.api_keys == ["sk-new", "sk-a"] + assert cfg.api_key == "sk-new" + + +def test_mask_key(): + assert mask_key("sk-1234567890abcd") == "sk-12…abcd" + assert mask_key("short") == "sho…" + assert mask_key("") == "(empty)" + # boundary: len 8 uses the short branch, len 9 uses the long branch + assert mask_key("12345678") == "123…" + assert mask_key("123456789") == "12345…6789" From a7e6165c5a07d6d1e73e1e07431af46a72e4b8bf Mon Sep 17 00:00:00 2001 From: JMAN730 Date: Mon, 22 Jun 2026 01:21:30 -0400 Subject: [PATCH 03/14] feat: improve gui agent auto-run and sidebar collapse --- README.md | 3 +- hackbot/gui/templates/index.html | 305 ++++++++++++++++++++++++++----- manual/03-gui-reference.md | 17 +- tests/test_gui_template.py | 43 +++++ 4 files changed, 311 insertions(+), 57 deletions(-) create mode 100644 tests/test_gui_template.py diff --git a/README.md b/README.md index 03703ba..b51ed07 100644 --- a/README.md +++ b/README.md @@ -122,9 +122,10 @@ sudo apt install -y python3-gi python3-gi-cairo gir1.2-gtk-3.0 gir1.2-webkit2-4. - **Dark cybersecurity theme** with a polished, modern interface - **Real-time streaming** — AI responses stream in via Server-Sent Events (SSE) - **All modes accessible** — Chat, Agent, Plan, CVE Lookup, OSINT, Topology, Compliance panels +- **Collapsible sidebar** — Minimize navigation to an icon rail with the preference remembered locally - **Session management** — Browse, restore, and delete saved sessions from the sidebar - **Provider switching** — Change AI provider and model on the fly from the settings panel -- **Agent control panel** — Start assessments, step through actions, view findings live +- **Agent control panel** — Start assessments that auto-run steps, stop them on demand, and view findings live - **CVE search panel** — Search by keyword, CVE ID, or browse exploits with severity filters - **OSINT panel** — Run subdomain enumeration, DNS lookups, WHOIS, and tech fingerprinting - **Topology visualization** — Paste nmap/masscan output and see an interactive D3.js network graph diff --git a/hackbot/gui/templates/index.html b/hackbot/gui/templates/index.html index 846970b..308519f 100644 --- a/hackbot/gui/templates/index.html +++ b/hackbot/gui/templates/index.html @@ -34,13 +34,23 @@ /* ── Layout ───────────────────────────────────────────────────────────────── */ .app-frame{display:flex;flex-direction:column;height:100vh;width:100vw} .app{display:flex;flex:1;overflow:hidden} -.sidebar{width:260px;min-width:260px;background:var(--bg2);border-right:1px solid var(--border);display:flex;flex-direction:column;overflow:hidden} +.sidebar{width:260px;min-width:260px;background:var(--bg2);border-right:1px solid var(--border);display:flex;flex-direction:column;overflow:hidden;transition:width .18s ease,min-width .18s ease} .main{flex:1;display:flex;flex-direction:column;overflow:hidden} +.app.sidebar-collapsed .sidebar{width:60px;min-width:60px} +.app.sidebar-collapsed .logo{padding:12px 8px;justify-content:center;flex-direction:column;gap:8px} +.app.sidebar-collapsed .logo-brand h1,.app.sidebar-collapsed .logo-brand span,.app.sidebar-collapsed .nav-section-title,.app.sidebar-collapsed .nav-btn span:not(.icon),.app.sidebar-collapsed .sidebar-footer{display:none} +.app.sidebar-collapsed .logo-brand img{margin-right:0!important} +.app.sidebar-collapsed .nav{padding:8px} +.app.sidebar-collapsed .nav-btn{justify-content:center;padding:12px} +.app.sidebar-collapsed .sidebar-toggle{width:32px;height:28px;justify-content:center} /* ── Sidebar ──────────────────────────────────────────────────────────────── */ -.logo{padding:20px;border-bottom:1px solid var(--border);text-align:center} -.logo h1{font-size:1.3rem;color:var(--green);letter-spacing:1px} -.logo span{font-size:.75rem;color:var(--text-dim)} +.logo{padding:16px 12px 16px 20px;border-bottom:1px solid var(--border);display:flex;align-items:center;justify-content:space-between;gap:10px} +.logo-brand{display:flex;align-items:center;gap:8px;min-width:0} +.logo h1{font-size:1.3rem;color:var(--green);letter-spacing:1px;white-space:nowrap} +.logo span{font-size:.75rem;color:var(--text-dim);white-space:nowrap} +.sidebar-toggle{width:30px;height:30px;border:1px solid var(--border);border-radius:6px;background:var(--bg3);color:var(--text);cursor:pointer;display:inline-flex;align-items:center;justify-content:center;font-size:1rem;line-height:1;transition:all .15s;flex-shrink:0} +.sidebar-toggle:hover{border-color:var(--border2);background:var(--bg4);color:var(--text-bright)} .nav{flex:1;padding:12px;overflow-y:auto} .nav-section{margin-bottom:16px} @@ -181,7 +191,10 @@ /* ── Responsive ───────────────────────────────────────────────────────────── */ @media(max-width:768px){ .sidebar{width:60px;min-width:60px} - .sidebar .logo h1,.sidebar .logo span,.nav-section-title,.nav-btn span:not(.icon),.sidebar-footer{display:none} + .logo{padding:12px 8px;justify-content:center;flex-direction:column;gap:8px} + .sidebar .logo-brand h1,.sidebar .logo-brand span,.nav-section-title,.nav-btn span:not(.icon),.sidebar-footer{display:none} + .sidebar .logo-brand img{margin-right:0!important} + .nav{padding:8px} .nav-btn{justify-content:center;padding:12px} .titlebar-title span.full{display:none} } @@ -213,17 +226,22 @@ -
+
-
+