diff --git a/gateway/platforms/discord.py b/gateway/platforms/discord.py index ec14dd2d6..c50f25381 100644 --- a/gateway/platforms/discord.py +++ b/gateway/platforms/discord.py @@ -1740,9 +1740,12 @@ class DiscordAdapter(BasePlatformAdapter): if not channel: channel = await self._client.fetch_channel(int(chat_id)) + # Discord embed description limit is 4096; show full command up to that + max_desc = 4088 + cmd_display = command if len(command) <= max_desc else command[: max_desc - 3] + "..." embed = discord.Embed( title="Command Approval Required", - description=f"```\n{command[:500]}\n```", + description=f"```\n{cmd_display}\n```", color=discord.Color.orange(), ) embed.set_footer(text=f"Approval ID: {approval_id}") diff --git a/tests/conftest.py b/tests/conftest.py index af73fb5cb..313a3cecf 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -107,7 +107,11 @@ def _ensure_current_event_loop(request): @pytest.fixture(autouse=True) def _enforce_test_timeout(): - """Kill any individual test that takes longer than 30 seconds.""" + """Kill any individual test that takes longer than 30 seconds. + SIGALRM is Unix-only; skip on Windows.""" + if sys.platform == "win32": + yield + return old = signal.signal(signal.SIGALRM, _timeout_handler) signal.alarm(30) yield diff --git a/tests/tools/test_approval.py b/tests/tools/test_approval.py index f6328989f..5afe31975 100644 --- a/tests/tools/test_approval.py +++ b/tests/tools/test_approval.py @@ -385,75 +385,47 @@ class TestPatternKeyUniqueness: assert is_approved("legacy-find", key_delete) is True -class TestViewFullCommand: - """Tests for the 'view full command' option in prompt_dangerous_approval.""" +class TestFullCommandAlwaysShown: + """The full command is always shown in the approval prompt (no truncation). - def test_view_then_once_fallback(self): - """Pressing 'v' shows the full command, then 'o' approves once.""" + Previously there was a [v]iew full option for long commands. Now the full + command is always displayed. These tests verify the basic approval flow + still works with long commands. (#1553) + """ + + def test_once_with_long_command(self): + """Pressing 'o' approves once even for very long commands.""" long_cmd = "rm -rf " + "a" * 200 - inputs = iter(["v", "o"]) - with mock_patch("builtins.input", side_effect=inputs): - result = prompt_dangerous_approval(long_cmd, "recursive delete") - assert result == "once" - - def test_view_then_deny_fallback(self): - """Pressing 'v' shows the full command, then 'd' denies.""" - long_cmd = "rm -rf " + "b" * 200 - inputs = iter(["v", "d"]) - with mock_patch("builtins.input", side_effect=inputs): - result = prompt_dangerous_approval(long_cmd, "recursive delete") - assert result == "deny" - - def test_view_then_session_fallback(self): - """Pressing 'v' shows the full command, then 's' approves for session.""" - long_cmd = "rm -rf " + "c" * 200 - inputs = iter(["v", "s"]) - with mock_patch("builtins.input", side_effect=inputs): - result = prompt_dangerous_approval(long_cmd, "recursive delete") - assert result == "session" - - def test_view_then_always_fallback(self): - """Pressing 'v' shows the full command, then 'a' approves always.""" - long_cmd = "rm -rf " + "d" * 200 - inputs = iter(["v", "a"]) - with mock_patch("builtins.input", side_effect=inputs): - result = prompt_dangerous_approval(long_cmd, "recursive delete") - assert result == "always" - - def test_view_then_session_when_permanent_hidden(self): - """The view-full flow still works when allow_permanent=False.""" - long_cmd = "rm -rf " + "d" * 200 - inputs = iter(["v", "s"]) - with mock_patch("builtins.input", side_effect=inputs): - result = prompt_dangerous_approval( - long_cmd, - "recursive delete", - allow_permanent=False, - ) - assert result == "session" - - def test_view_not_shown_for_short_command(self): - """Short commands don't offer the view option; 'v' falls through to deny.""" - short_cmd = "rm -rf /tmp" - with mock_patch("builtins.input", return_value="v"): - result = prompt_dangerous_approval(short_cmd, "recursive delete") - # 'v' is not a valid choice for short commands, should deny - assert result == "deny" - - def test_once_without_view(self): - """Directly pressing 'o' without viewing still works.""" - long_cmd = "rm -rf " + "e" * 200 with mock_patch("builtins.input", return_value="o"): result = prompt_dangerous_approval(long_cmd, "recursive delete") assert result == "once" - def test_view_ignored_after_already_shown(self): - """After viewing once, 'v' on a now-untruncated display falls through to deny.""" - long_cmd = "rm -rf " + "f" * 200 - inputs = iter(["v", "v"]) # second 'v' should not match since is_truncated is False - with mock_patch("builtins.input", side_effect=inputs): + def test_session_with_long_command(self): + """Pressing 's' approves for session with long commands.""" + long_cmd = "rm -rf " + "c" * 200 + with mock_patch("builtins.input", return_value="s"): result = prompt_dangerous_approval(long_cmd, "recursive delete") - # After first 'v', is_truncated becomes False, so second 'v' -> deny + assert result == "session" + + def test_always_with_long_command(self): + """Pressing 'a' approves always with long commands.""" + long_cmd = "rm -rf " + "d" * 200 + with mock_patch("builtins.input", return_value="a"): + result = prompt_dangerous_approval(long_cmd, "recursive delete") + assert result == "always" + + def test_deny_with_long_command(self): + """Pressing 'd' denies with long commands.""" + long_cmd = "rm -rf " + "b" * 200 + with mock_patch("builtins.input", return_value="d"): + result = prompt_dangerous_approval(long_cmd, "recursive delete") + assert result == "deny" + + def test_invalid_input_denies(self): + """Invalid input (like 'v' which no longer exists) falls through to deny.""" + short_cmd = "rm -rf /tmp" + with mock_patch("builtins.input", return_value="v"): + result = prompt_dangerous_approval(short_cmd, "recursive delete") assert result == "deny" diff --git a/tools/approval.py b/tools/approval.py index 9f1b541ff..60c6e5fe5 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -220,17 +220,15 @@ def prompt_dangerous_approval(command: str, description: str, os.environ["HERMES_SPINNER_PAUSE"] = "1" try: - is_truncated = len(command) > 80 while True: print() print(f" ⚠️ DANGEROUS COMMAND: {description}") - print(f" {command[:80]}{'...' if is_truncated else ''}") + print(f" {command}") print() - view_hint = " | [v]iew full" if is_truncated else "" if allow_permanent: - print(f" [o]nce | [s]ession | [a]lways | [d]eny{view_hint}") + print(" [o]nce | [s]ession | [a]lways | [d]eny") else: - print(f" [o]nce | [s]ession | [d]eny{view_hint}") + print(" [o]nce | [s]ession | [d]eny") print() sys.stdout.flush() @@ -252,12 +250,6 @@ def prompt_dangerous_approval(command: str, description: str, return "deny" choice = result["choice"] - if choice in ('v', 'view') and is_truncated: - print() - print(" Full command:") - print(f" {command}") - is_truncated = False - continue if choice in ('o', 'once'): print(" ✓ Allowed once") return "once" @@ -394,7 +386,10 @@ def check_dangerous_command(command: str, env_type: str, "status": "approval_required", "command": command, "description": description, - "message": f"⚠️ This command is potentially dangerous ({description}). Asking the user for approval...", + "message": ( + f"⚠️ This command is potentially dangerous ({description}). " + f"Asking the user for approval.\n\n**Command:**\n```\n{command}\n```" + ), } choice = prompt_dangerous_approval(command, description, @@ -542,7 +537,9 @@ def check_all_command_guards(command: str, env_type: str, "status": "approval_required", "command": command, "description": combined_desc, - "message": f"⚠️ {combined_desc}. Asking the user for approval...", + "message": ( + f"⚠️ {combined_desc}. Asking the user for approval.\n\n**Command:**\n```\n{command}\n```" + ), } # CLI interactive: single combined prompt