fixes

tests/test_api_handlers.py

@@ -1,5 +1,9 @@
-from app.api.server import chat, critic_feedback, health, list_events, resolve_permission, resolve_secret
-from app.core.permission_resolution import PermissionResolutionRequest, SecretResolutionRequest
+import asyncio
+import time
+
+import app.api.server as server
+from app.api.server import chat, critic_feedback, health, list_events, resolve_permission, resolve_review, resolve_secret
+from app.core.permission_resolution import PermissionResolutionRequest, ReviewResolutionRequest, SecretResolutionRequest
 from app.api.server import CriticFeedbackRequest
 from app.core.contracts import UserTask
 
@@ -16,8 +20,52 @@ def test_events_handler_returns_event_list() -> None:
 
 def test_chat_handler_returns_runtime_events() -> None:
     body = chat(UserTask(input="hello from handler test"))
-    assert body["status"] == "completed"
-    assert body["events"][0]["type"] == "task_received"
+    assert body["status"] in {"accepted", "completed"}
+    if body["status"] == "completed":
+        assert body["events"][0]["type"] == "task_received"
+
+
+def test_chat_handler_submits_task_without_waiting_for_completion(monkeypatch) -> None:
+    class SlowRuntime:
+        def submit_task(self, task):
+            return {"task_id": task.task_id, "status": "accepted"}
+
+        def handle_task(self, task):
+            time.sleep(0.25)
+            return {"task_id": task.task_id, "status": "completed", "events": []}
+
+    monkeypatch.setattr("app.api.server.runtime", SlowRuntime())
+    started = time.monotonic()
+    body = chat(UserTask(input="long task"))
+
+    assert time.monotonic() - started < 0.1
+    assert body["status"] == "accepted"
+
+
+def test_lifespan_loads_models_without_threadpool_executor(monkeypatch) -> None:
+    class FakeRuntime:
+        _memory_interface = None
+
+        def __init__(self) -> None:
+            self.loaded = False
+
+        def load_models_at_startup(self) -> None:
+            self.loaded = True
+
+    class FailingLoop:
+        def run_in_executor(self, *args, **kwargs):
+            raise AssertionError("lifespan must not load llama models via run_in_executor")
+
+    fake_runtime = FakeRuntime()
+    monkeypatch.setattr(server, "runtime", fake_runtime)
+    monkeypatch.setattr(server.asyncio, "get_event_loop", lambda: FailingLoop())
+
+    async def run_lifespan() -> None:
+        async with server.lifespan(None):
+            pass
+
+    asyncio.run(run_lifespan())
+    assert fake_runtime.loaded is True
 
 
 def test_resolve_permission_handler_allows_completion() -> None:
@@ -34,6 +82,29 @@ def test_resolve_secret_handler_requires_pending_request() -> None:
     assert body["status"] == "failed"
 
 
+def test_resolve_review_handler_submits_review_resolution(monkeypatch) -> None:
+    class ReviewRuntime:
+        def submit_review_resolution(self, task_id, decision, correction=None):
+            return {
+                "task_id": task_id,
+                "status": "accepted",
+                "decision": decision,
+                "correction": correction,
+            }
+
+    monkeypatch.setattr("app.api.server.runtime", ReviewRuntime())
+    body = resolve_review(
+        ReviewResolutionRequest(
+            task_id="task-1",
+            decision="wrong_action",
+            correction="replan",
+        )
+    )
+
+    assert body["status"] == "accepted"
+    assert body["decision"] == "wrong_action"
+
+
 def test_structured_feedback_can_be_accepted_without_memory_write() -> None:
     initial = chat(UserTask(input="feedback target"))
     body = critic_feedback(

tests/test_command_analyzer.py

@@ -0,0 +1,46 @@
+from app.core.command_analyzer import CommandAnalyzer
+from app.core.permission_service import PermissionService
+
+
+def _permission_service() -> PermissionService:
+    return PermissionService(
+        config={
+            "settings": {},
+            "command_categories": {
+                "no_always": {
+                    "allow_once": True,
+                    "allow_always": False,
+                    "commands": ["apt", "apt-get", "dpkg", "systemctl"],
+                }
+            },
+            "path_settings": {},
+        }
+    )
+
+
+def test_detects_unelevated_root_required_segment_after_sudo_chain() -> None:
+    analyzer = CommandAnalyzer(_permission_service())
+
+    diagnosis = analyzer.analyze(
+        command="sudo apt update && apt upgrade -y",
+        task_id="task-1",
+        session_id="session-1",
+    )
+
+    assert diagnosis["type"] == "privilege_scope_error"
+    assert diagnosis["root_required_segments"] == ["apt update", "apt upgrade -y"]
+    assert diagnosis["elevated_segments"] == ["apt update"]
+    assert diagnosis["unelevated_root_segments"] == ["apt upgrade -y"]
+
+
+def test_accepts_each_root_required_segment_when_each_is_elevated() -> None:
+    analyzer = CommandAnalyzer(_permission_service())
+
+    diagnosis = analyzer.analyze(
+        command="sudo apt update && sudo apt upgrade -y",
+        task_id="task-1",
+        session_id="session-1",
+    )
+
+    assert diagnosis["type"] == "ok"
+    assert diagnosis["unelevated_root_segments"] == []

tests/test_runtime_loop.py

@@ -14,12 +14,25 @@ def test_runtime_loop_emits_basic_events() -> None:
 
 
 def test_runtime_loop_routes_natural_language_shell_request_to_permission_flow() -> None:
+    import os, shutil
+    # Clear permission cache to ensure clean state
+    cache_file = os.path.join(os.path.dirname(__file__), '..', 'data', 'runtime', 'allowed_commands.json')
+    if os.path.exists(cache_file):
+        os.remove(cache_file)
+    
     controller = RuntimeController()
     result = controller.handle_task(UserTask(input="запусти sudo apt update"))
     event_types = [event["type"] for event in result["events"]]
+    # sudo commands require both permission and password
+    # First step: permission request
     assert result["status"] == "awaiting_permission"
     assert result["directive"]["type"] == "tool"
     assert result["directive"]["payload"]["tool"] == "shell_exec"
     assert "permission_requested" in event_types
     assert "task_awaiting_permission" in event_types
     assert result["result"]["error"] == "Permission required before execution."
+
+    # After granting permission, should request sudo password
+    resumed = controller.resolve_permission(task_id=result["task_id"], decision="allow_once")
+    assert resumed["status"] == "awaiting_input"
+    assert resumed["result"]["secret_request"]["kind"] == "sudo_password"

tests/test_tools_flow.py

@@ -2,7 +2,11 @@ import json
 from pathlib import Path
 
 from app.core.contracts import ExecutionDirective, UserTask
+from app.core.contracts import PermissionDecision
+from app.core.contracts import ToolResult
+from app.events.event_types import TOOL_OUTPUT_CHUNK
 from app.runtime.runtime_controller import RuntimeController
+from app.tools.sandbox import ToolSandbox
 
 
 def _write_config_tree(base_dir: Path) -> None:
@@ -27,9 +31,38 @@ def _write_config_tree(base_dir: Path) -> None:
             "critic_prompt": "",
         },
         "permissions.json": {
-            "dangerous_commands": {"rm": "ask_always", "sudo": "ask_always"},
-            "sensitive_paths": ["/etc", "/usr", "/var"],
-            "default_approval_behavior": "ask_always",
+            "settings": {
+                "allow_caching": True,
+                "cache_file": str(base_dir / "data/runtime/allowed_commands.json"),
+                "normalize_commands": True,
+                "split_chained": True
+            },
+            "command_categories": {
+                "hard_stop": {
+                    "commands": ["rm -rf /", "rm -rf /*", "dd if=/dev/zero of=/dev/sd*"]
+                },
+                "no_always": {
+                    "allow_once": True,
+                    "allow_always": False,
+                    "commands": [
+                        "rm -rf *", "rm -rf .*", "shutdown", "reboot", "halt",
+                        "apt", "apt-get", "dpkg", "yum", "dnf", "pacman",
+                        "systemctl stop", "systemctl start", "systemctl restart",
+                        "service stop", "service start", "killall", "pkill -9"
+                    ]
+                },
+                "normal": {
+                    "allow_once": True,
+                    "allow_always": True,
+                    "commands": ["shell_exec", "file_write"]
+                }
+            },
+            "path_settings": {
+                "allow_read_outside": True,
+                "allow_write_paths": [str(base_dir), "/tmp"],
+                "require_confirmation_for_write": True,
+                "require_confirmation_for_shell": True
+            }
         },
         "runtime.json": {
             "step_timeout_ms": 5000,
@@ -92,6 +125,8 @@ def test_shell_exec_requires_permission_for_dangerous_command(tmp_path: Path) ->
             },
         )
     )
+    # rm -rf /tmp/nonexistent is not hard_stop (only exact "rm -rf /" is)
+    # but it matches "rm -rf *" in no_always category
     assert result["status"] == "awaiting_permission"
     assert "permission_request" in result["result"]
 
@@ -108,8 +143,87 @@ def test_shell_exec_allows_safe_command(tmp_path: Path) -> None:
             },
         )
     )
+    # Even safe commands require permission in the new permission model
+    assert result["status"] == "awaiting_permission"
+    assert "permission_request" in result["result"]
+    # Grant permission and verify execution
+    resumed = controller.resolve_permission(task_id=result["task_id"], decision="allow_once")
+    assert resumed["status"] == "completed"
+    assert str(tmp_path) in resumed["result"]["output"]
+
+
+def test_shell_exec_publishes_output_chunks_before_completion(tmp_path: Path) -> None:
+    _write_config_tree(tmp_path)
+    controller = RuntimeController(base_dir=tmp_path)
+    perm_override = PermissionDecision(
+        action_type="shell_command",
+        pattern="printf",
+        decision="allow_always",
+    )
+
+    task = UserTask(
+        input="stream shell output",
+        context={
+            "requested_tool": "shell_exec",
+            "tool_args": {"command": "printf 'first\\n'; sleep 0.1; printf 'second\\n'"},
+        },
+    )
+    result = controller.execution_engine.execute(
+        task,
+        ExecutionDirective(
+            type="tool",
+            payload={
+                "tool": "shell_exec",
+                "args": {"command": "printf 'first\\n'; sleep 0.1; printf 'second\\n'"},
+            },
+        ),
+        permission_override=perm_override,
+    )
+
+    events = controller.event_bus.list_for_task(task.task_id)
+    chunk_events = [event for event in events if event.type == TOOL_OUTPUT_CHUNK]
+    completed_index = next(index for index, event in enumerate(events) if event.type == "tool_completed")
+    first_chunk_index = next(index for index, event in enumerate(events) if event.type == TOOL_OUTPUT_CHUNK)
     assert result["status"] == "completed"
-    assert str(tmp_path) in result["result"]["output"]
+    assert [event.payload["chunk"] for event in chunk_events] == ["first\n", "second\n"]
+    assert first_chunk_index < completed_index
+
+
+def test_streaming_shell_uses_idle_timeout_not_step_timeout(tmp_path: Path) -> None:
+    sandbox = ToolSandbox(
+        allowed_root=tmp_path,
+        timeout_ms=100,
+        command_timeout_ms=2000,
+        idle_timeout_ms=500,
+    )
+    chunks: list[str] = []
+
+    result = sandbox.run_shell(
+        command="printf 'first\\n'; sleep 0.2; printf 'second\\n'",
+        output_callback=lambda _stream, chunk: chunks.append(chunk),
+    )
+
+    assert result.returncode == 0
+    assert result.stdout == "first\nsecond\n"
+    assert chunks == ["first\n", "second\n"]
+
+
+def test_streaming_shell_timeout_kills_child_process_group(tmp_path: Path) -> None:
+    marker = tmp_path / "child-survived"
+    sandbox = ToolSandbox(
+        allowed_root=tmp_path,
+        timeout_ms=100,
+        command_timeout_ms=100,
+        idle_timeout_ms=1000,
+    )
+
+    result = sandbox.run_shell(
+        command=f"sh -c 'sleep 1; touch {marker}'",
+        output_callback=lambda _stream, _chunk: None,
+    )
+
+    assert result.returncode == -9
+    assert not marker.exists()
 
 
 class _RecoveryCritic:
@@ -122,6 +236,13 @@ def test_failed_shell_step_can_recover_and_continue(tmp_path: Path) -> None:
     controller = RuntimeController(base_dir=tmp_path)
     controller.execution_engine.set_critic(_RecoveryCritic())
     controller.execution_engine._recovery_limit = 1
+    # Bypass permission check for this test — we're testing recovery, not permissions
+    from app.core.contracts import PermissionDecision
+    perm_override = PermissionDecision(
+        action_type="shell_command",
+        pattern="grep",
+        decision="allow_always",
+    )
     result = controller.execution_engine.execute(
         UserTask(
             input="run grep with no matches and recover",
@@ -139,12 +260,177 @@ def test_failed_shell_step_can_recover_and_continue(tmp_path: Path) -> None:
                 ]
             },
         ),
+        permission_override=perm_override,
     )
     assert result["status"] == "completed"
     failed_result = result["result"]["step_results"][0]["result"]["result"]
     assert failed_result["metadata"]["exit_code"] == 1
 
 
+def test_privilege_scope_failure_awaits_user_review_before_replan(tmp_path: Path) -> None:
+    _write_config_tree(tmp_path)
+    controller = RuntimeController(base_dir=tmp_path)
+    task = UserTask(
+        input="обнови систему",
+        context={
+            "requested_tool": "shell_exec",
+            "tool_args": {"command": "sudo apt update && apt upgrade -y"},
+        },
+    )
+    class FailingShellTool:
+        def execute(self, task: UserTask, args: dict[str, object]) -> ToolResult:
+            return ToolResult(
+                tool="shell_exec",
+                ok=False,
+                output="Error: Unable to acquire the dpkg frontend lock (/var/lib/dpkg/lock-frontend), are you root?",
+                error="Command failed with exit code 100",
+                metadata={"exit_code": 100},
+            )
+
+    controller.tool_registry._tools["shell_exec"] = FailingShellTool()
+
+    initial = controller.handle_task(task)
+    assert initial["status"] == "awaiting_permission"
+    controller.resolve_permission(task_id=task.task_id, decision="allow_once")
+    result = controller.resolve_secret(task_id=task.task_id, secret="secret")
+
+    assert result["status"] == "awaiting_review"
+    assert result["result"]["review"]["diagnosis"]["type"] == "privilege_scope_error"
+    assert result["result"]["review"]["critic_assessment"]["classification"] == "model_planning_error"
+
+
+def test_plan_pauses_on_privilege_scope_review_instead_of_completing(tmp_path: Path) -> None:
+    _write_config_tree(tmp_path)
+    controller = RuntimeController(base_dir=tmp_path)
+
+    class FailingShellTool:
+        def execute(self, task: UserTask, args: dict[str, object]) -> ToolResult:
+            return ToolResult(
+                tool="shell_exec",
+                ok=False,
+                output="Error: Unable to acquire the dpkg frontend lock (/var/lib/dpkg/lock-frontend), are you root?",
+                error="Command failed with exit code 100",
+                metadata={"exit_code": 100},
+            )
+
+    controller.tool_registry._tools["shell_exec"] = FailingShellTool()
+    result = controller.execution_engine.execute(
+        UserTask(input="обнови систему"),
+        ExecutionDirective(
+            type="plan",
+            payload={
+                "steps": [
+                    {
+                        "id": "1",
+                        "tool": "shell_exec",
+                        "args": {"command": "sudo apt update && apt upgrade -y"},
+                        "depends_on": [],
+                    }
+                ]
+            },
+        ),
+        permission_override=PermissionDecision(
+            action_type="shell_command",
+            pattern="apt",
+            decision="allow_once",
+        ),
+        secret_override="secret",
+    )
+
+    assert result["status"] == "awaiting_review"
+    assert result["result"]["review"]["diagnosis"]["type"] == "privilege_scope_error"
+
+
+def test_sudo_auth_failure_requests_secret_retry_not_review(tmp_path: Path) -> None:
+    _write_config_tree(tmp_path)
+    controller = RuntimeController(base_dir=tmp_path)
+
+    class BadPasswordShellTool:
+        def execute(self, task: UserTask, args: dict[str, object]) -> ToolResult:
+            return ToolResult(
+                tool="shell_exec",
+                ok=False,
+                output="Sorry, try again.\nsudo: no password was provided\nsudo: 1 incorrect password attempt\n",
+                error="Command failed with exit code 1",
+                metadata={"exit_code": 1, "sudo_auth_failed": True},
+            )
+
+    controller.tool_registry._tools["shell_exec"] = BadPasswordShellTool()
+    result = controller.execution_engine.execute(
+        UserTask(input="обнови систему"),
+        ExecutionDirective(
+            type="plan",
+            payload={
+                "steps": [
+                    {
+                        "id": "1",
+                        "tool": "shell_exec",
+                        "args": {"command": "sudo apt update && apt upgrade -y"},
+                        "depends_on": [],
+                    }
+                ]
+            },
+        ),
+        permission_override=PermissionDecision(
+            action_type="shell_command",
+            pattern="apt",
+            decision="allow_once",
+        ),
+        secret_override="wrong",
+    )
+
+    assert result["status"] == "awaiting_input"
+    assert result["result"]["secret_request"]["kind"] == "sudo_password"
+    assert result["result"]["secret_request"]["prompt"] == "Sudo password incorrect. Try again"
+    assert result["result"]["attempt_failed"] is True
+
+
+def test_runtime_keeps_secret_state_after_bad_sudo_password(tmp_path: Path) -> None:
+    _write_config_tree(tmp_path)
+    controller = RuntimeController(base_dir=tmp_path)
+
+    class RetryPasswordShellTool:
+        calls = 0
+
+        def execute(self, task: UserTask, args: dict[str, object]) -> ToolResult:
+            self.calls += 1
+            if self.calls == 1:
+                return ToolResult(
+                    tool="shell_exec",
+                    ok=False,
+                    output="Sorry, try again.\nsudo: no password was provided\nsudo: 1 incorrect password attempt\n",
+                    error="Command failed with exit code 1",
+                    metadata={"exit_code": 1, "sudo_auth_failed": True},
+                )
+            return ToolResult(
+                tool="shell_exec",
+                ok=True,
+                output="root\n",
+                metadata={"exit_code": 0},
+            )
+
+    controller.tool_registry._tools["shell_exec"] = RetryPasswordShellTool()
+    task = UserTask(
+        input="кто root",
+        context={
+            "requested_tool": "shell_exec",
+            "tool_args": {"command": "sudo whoami"},
+        },
+    )
+    initial = controller.handle_task(task)
+    assert initial["status"] == "awaiting_permission"
+    allowed = controller.resolve_permission(task_id=task.task_id, decision="allow_once")
+    assert allowed["status"] == "awaiting_input"
+
+    retry = controller.resolve_secret(task_id=task.task_id, secret="wrong")
+    assert retry["status"] == "awaiting_input"
+    assert retry["result"]["attempt_failed"] is True
+
+    final = controller.resolve_secret(task_id=task.task_id, secret="correct")
+    assert final["status"] == "completed"
+    assert final["result"]["output"] == "root\n"
+
+
 def test_permission_resolution_can_resume_task(tmp_path: Path) -> None:
     _write_config_tree(tmp_path)
     controller = RuntimeController(base_dir=tmp_path)
@@ -169,12 +455,35 @@ def test_sudo_permission_resolution_requests_secret_input(tmp_path: Path) -> Non
     assert resumed["result"]["secret_request"]["kind"] == "sudo_password"
 
 
+def test_implicit_sudo_command_requests_password(tmp_path: Path) -> None:
+    """Commands like 'apt list --upgradable' that require sudo but don't start with 'sudo'
+    should also trigger password request after permission is granted."""
+    _write_config_tree(tmp_path)
+    controller = RuntimeController(base_dir=tmp_path)
+    # apt list --upgradable requires root but doesn't start with 'sudo'
+    initial = controller.handle_task(
+        UserTask(
+            input="проверь обновления",
+            context={
+                "requested_tool": "shell_exec",
+                "tool_args": {"command": "apt list --upgradable"},
+            },
+        )
+    )
+    assert initial["status"] == "awaiting_permission"
+    # Grant permission — should request sudo password since apt requires root
+    resumed = controller.resolve_permission(task_id=initial["task_id"], decision="allow_once")
+    assert resumed["status"] == "awaiting_input"
+    assert resumed["result"]["secret_request"]["kind"] == "sudo_password"
+
+
 def test_secret_resolution_continues_after_pending_secret_saved(tmp_path: Path) -> None:
     _write_config_tree(tmp_path)
     controller = RuntimeController(base_dir=tmp_path)
     initial = controller.handle_task(UserTask(input="запусти sudo apt update"))
+    assert initial["status"] == "awaiting_permission"
     resumed = controller.resolve_permission(task_id=initial["task_id"], decision="allow_once")
     assert resumed["status"] == "awaiting_input"
     final = controller.resolve_secret(task_id=initial["task_id"], secret="wrongpass")
-    assert final["status"] in {"completed", "failed"}
+    assert final["status"] in {"completed", "failed", "awaiting_input"}
     assert "error" in final["result"] or "output" in final["result"]