diff --git a/asv/benchmarks/tui_reference_fuzzy_bench.py b/asv/benchmarks/tui_reference_fuzzy_bench.py index e8497ac8d..9acc25f76 100644 --- a/asv/benchmarks/tui_reference_fuzzy_bench.py +++ b/asv/benchmarks/tui_reference_fuzzy_bench.py @@ -10,14 +10,17 @@ from cleveragents.tui.search.fuzzy import rank_candidates class TuiReferenceFuzzyBench: """Bench weighted fuzzy ranking on a medium candidate set.""" - params: ClassVar[list[int]] = [1_000, 10_000] - param_names: ClassVar[list[str]] = ["candidate_count"] + params: ClassVar[list[list[object]]] = [ + [1_000, 10_000], + ["module_42/file_42", "zzzzzz-no-match-query"], + ] + param_names: ClassVar[list[str]] = ["candidate_count", "query"] def setup(self, candidate_count: int) -> None: self.values = [ f"src/module_{idx}/file_{idx}.py" for idx in range(candidate_count) ] - def time_rank_candidates(self, candidate_count: int) -> None: + def time_rank_candidates(self, candidate_count: int, query: str) -> None: del candidate_count - rank_candidates("module_42/file_42", self.values, limit=20) + rank_candidates(query, self.values, limit=20) diff --git a/features/mocks/fake_repl_input.py b/features/mocks/fake_repl_input.py new file mode 100644 index 000000000..16e06f760 --- /dev/null +++ b/features/mocks/fake_repl_input.py @@ -0,0 +1,22 @@ +"""Shared fake input helper for REPL Behave scenarios.""" + +from __future__ import annotations + + +class FakeReplInput: + """Feed predefined lines and then terminate input with EOFError.""" + + def __init__(self, lines: list[str], *, interrupt_token: str | None = None) -> None: + self._lines = list(lines) + self._index = 0 + self._interrupt_token = interrupt_token + + def __call__(self, prompt: str = "") -> str: + del prompt + if self._index >= len(self._lines): + raise EOFError + line = self._lines[self._index] + self._index += 1 + if self._interrupt_token is not None and line == self._interrupt_token: + raise KeyboardInterrupt + return line diff --git a/features/repl_input_modes.feature b/features/repl_input_modes.feature index 2f75d8305..f40524afb 100644 --- a/features/repl_input_modes.feature +++ b/features/repl_input_modes.feature @@ -17,11 +17,20 @@ Feature: REPL input modes and persona controls Scenario: Shell mode runs host shell commands Given a temporary REPL config directory When I run the REPL with input lines - | line | - | !printf hello | + | line | + | !echo hello | Then the REPL mode output should contain "hello" + Scenario: Shell mode blocks dangerous commands without confirmation + Given a temporary REPL config directory + When I run the REPL with input lines + | line | + | !rm -rf / | + | n | + Then the REPL mode output should contain "Blocked dangerous shell command." + Scenario: @ references are expanded with canonical category prefixes + Given a deterministic REPL reference catalog fixture When I expand REPL references for "info @README.md" Then the expanded REPL line should contain "@file:README.md" @@ -33,6 +42,14 @@ Feature: REPL input modes and persona controls | /persona reviewer | Then the REPL mode output should contain "Active persona: reviewer" + Scenario: CLI persona model rejects ANSI/control characters in names + When I validate CLI persona name "bad\\x1bname" + Then CLI persona validation should fail with "path/control" + + Scenario: CLI persona model preserves color field + When I validate CLI persona payload with color "red" + Then CLI persona color should be "red" + Scenario: Persona export import and delete workflow Given a temporary REPL config directory And a temporary persona export path @@ -48,6 +65,38 @@ Feature: REPL input modes and persona controls And the REPL mode output should contain "Imported persona: dev" And the REPL mode output should contain "Active persona: dev" + Scenario: Persona create rejects path traversal names + Given a temporary REPL config directory + When I run the REPL with input lines + | line | + | /persona create ../../etc/cron.d/evil --actor local/mock-default | + Then the REPL mode output should contain "name must not contain path/control separators" + + Scenario: Persona export rejects absolute path targets + Given a temporary REPL config directory + And an absolute persona export path + When I run the REPL with input lines + | line | + | /persona create dev --actor local/mock-default | + | /persona export dev {export_path} | + Then the REPL mode output should contain "Export path must be relative to current working directory" + + Scenario: Persona export rejects parent directory escape + Given a temporary REPL config directory + When I run the REPL with input lines + | line | + | /persona create dev --actor local/mock-default | + | /persona export dev ../outside.yaml | + Then the REPL mode output should contain "Export path must stay within working directory" + + Scenario: Persona import rejects absolute path targets + Given a temporary REPL config directory + And an absolute persona import file path + When I run the REPL with input lines + | line | + | /persona import {absolute_import_path} | + Then the REPL mode output should contain "Import path must be relative to current working directory" + Scenario: Persona binding is independent per REPL session Given a temporary REPL config directory When I run the REPL with input lines @@ -62,3 +111,32 @@ Feature: REPL input modes and persona controls | /session list | Then the REPL mode output should contain "work (persona=dev)" And the REPL mode output should contain "default (persona=reviewer)" + + Scenario: Deleting persona resets all sessions using it + Given a temporary REPL config directory + When I run the REPL with input lines + | line | + | /persona create dev --actor local/mock-default | + | /session new work | + | /session switch work | + | /persona set dev | + | /session switch default | + | /persona set dev | + | /persona delete dev | + | /session list | + Then the REPL mode output should contain "work (persona=default)" + And the REPL mode output should contain "default (persona=default)" + + Scenario: Slash persona commands surface validation and usage errors + Given a temporary REPL config directory + And an invalid persona import file path + When I run the REPL with input lines + | line | + | /persona set missing | + | /persona create dev | + | /persona delete default | + | /persona import {invalid_import_path} | + Then the REPL mode output should contain "Persona not found: missing" + And the REPL mode output should contain "Usage: /persona create --actor " + And the REPL mode output should contain "Cannot delete default persona." + And the REPL mode output should contain "Invalid persona YAML." diff --git a/features/steps/m1_sourcecode_smoke_steps.py b/features/steps/m1_sourcecode_smoke_steps.py index 2b58f424c..0e5383486 100644 --- a/features/steps/m1_sourcecode_smoke_steps.py +++ b/features/steps/m1_sourcecode_smoke_steps.py @@ -10,6 +10,7 @@ import contextlib import json from datetime import datetime from pathlib import Path +from typing import Any from unittest.mock import MagicMock, patch import yaml @@ -35,24 +36,6 @@ _FIXTURES_DIR = Path(__file__).resolve().parents[1] / "fixtures" / "m1" _PLAN_ULID = "01M1SM0KE00000000000000001" -def _register_patcher_cleanup(context: Context, patcher: object) -> None: - """Register a patcher's stop() with Behave scenario cleanup.""" - stop = getattr(patcher, "stop", None) - if not callable(stop): - return - - def _safe_stop() -> None: - with contextlib.suppress(RuntimeError): - stop() - - if hasattr(context, "add_cleanup"): - context.add_cleanup(_safe_stop) - return - if not hasattr(context, "_cleanup_handlers"): - context._cleanup_handlers = [] - context._cleanup_handlers.append(_safe_stop) - - def _make_m1_plan( *, name: str = "local/m1-smoke-plan", @@ -105,6 +88,18 @@ def _make_m1_action( ) +def _register_patcher_cleanup(context: Context, patcher: Any) -> None: + """Start a patcher and ensure it is always stopped.""" + patcher.start() + context.add_cleanup(_safe_stop_patcher, patcher) + + +def _safe_stop_patcher(patcher: Any) -> None: + """Best-effort patcher stop for repeated/failed scenarios.""" + with contextlib.suppress(RuntimeError): + patcher.stop() + + # --------------------------------------------------------------------------- # Background # --------------------------------------------------------------------------- @@ -153,16 +148,14 @@ def step_m1_smoke_mock_service(context: Context) -> None: context.link_patcher = patch( "cleveragents.cli.commands.project._get_resource_link_repo", ) - context.plan_patcher.start() _register_patcher_cleanup(context, context.plan_patcher) - context.action_patcher.start() _register_patcher_cleanup(context, context.action_patcher) context.mock_project_repo = context.project_patcher.start() - _register_patcher_cleanup(context, context.project_patcher) + context.add_cleanup(_safe_stop_patcher, context.project_patcher) context.mock_resource_svc = context.resource_patcher.start() - _register_patcher_cleanup(context, context.resource_patcher) + context.add_cleanup(_safe_stop_patcher, context.resource_patcher) context.mock_link_repo = context.link_patcher.start() - _register_patcher_cleanup(context, context.link_patcher) + context.add_cleanup(_safe_stop_patcher, context.link_patcher) context.last_result = None context.captured_plan_id = None @@ -476,7 +469,6 @@ def step_m1_plan_in_strategize(context: Context) -> None: "cleveragents.cli.commands.plan._get_plan_executor", return_value=MagicMock(), ) - context._m1_executor_patcher.start() _register_patcher_cleanup(context, context._m1_executor_patcher) @@ -528,7 +520,6 @@ def step_m1_plan_with_changeset(context: Context) -> None: "cleveragents.cli.commands.plan._get_apply_service", return_value=mock_apply_svc, ) - context.apply_patcher.start() _register_patcher_cleanup(context, context.apply_patcher) diff --git a/features/steps/m4_correction_subplan_smoke_steps.py b/features/steps/m4_correction_subplan_smoke_steps.py index beb92baee..3a0bbe293 100644 --- a/features/steps/m4_correction_subplan_smoke_steps.py +++ b/features/steps/m4_correction_subplan_smoke_steps.py @@ -41,24 +41,6 @@ _PLAN_ULID = "01M4SM0KE00000000000000001" _DECISION_ULID = "01M4DEC00000000000000000001" -def _register_patcher_cleanup(context: Context, patcher: object) -> None: - """Register a patcher's stop() with Behave scenario cleanup.""" - stop = getattr(patcher, "stop", None) - if not callable(stop): - return - - def _safe_stop() -> None: - with contextlib.suppress(RuntimeError): - stop() - - if hasattr(context, "add_cleanup"): - context.add_cleanup(_safe_stop) - return - if not hasattr(context, "_cleanup_handlers"): - context._cleanup_handlers = [] - context._cleanup_handlers.append(_safe_stop) - - def _make_m4_plan( *, phase: PlanPhase = PlanPhase.EXECUTE, @@ -96,6 +78,18 @@ def _make_m4_plan( ) +def _register_patcher_cleanup(context: Context, patcher: object) -> None: + """Start a patcher and always stop it via Behave cleanup.""" + patcher.start() + context.add_cleanup(_safe_stop_patcher, patcher) + + +def _safe_stop_patcher(patcher: object) -> None: + """Stop patcher safely even if it was already stopped.""" + with contextlib.suppress(RuntimeError): + patcher.stop() + + # --------------------------------------------------------------------------- # Background # --------------------------------------------------------------------------- @@ -117,7 +111,6 @@ def step_m4_smoke_mock_service(context: Context) -> None: "cleveragents.cli.commands.plan._get_lifecycle_service", return_value=context.m4_mock_service, ) - context.m4_plan_patcher.start() _register_patcher_cleanup(context, context.m4_plan_patcher) context.m4_last_result = None @@ -249,7 +242,6 @@ def step_m4_plan_with_decision_tree(context: Context) -> None: "cleveragents.application.services.correction_service.CorrectionService", return_value=mock_correction_svc, ) - context.m4_correction_patcher.start() _register_patcher_cleanup(context, context.m4_correction_patcher) context.m4_mock_correction_service = mock_correction_svc @@ -263,7 +255,6 @@ def step_m4_plan_with_decision_tree(context: Context) -> None: "cleveragents.application.container.get_container", return_value=mock_container, ) - context.m4_container_patcher.start() _register_patcher_cleanup(context, context.m4_container_patcher) diff --git a/features/steps/plan_lifecycle_cli_steps.py b/features/steps/plan_lifecycle_cli_steps.py index 47faba3a5..faea92931 100644 --- a/features/steps/plan_lifecycle_cli_steps.py +++ b/features/steps/plan_lifecycle_cli_steps.py @@ -2,6 +2,7 @@ from __future__ import annotations +import importlib from types import SimpleNamespace from unittest.mock import MagicMock, patch @@ -113,11 +114,12 @@ def step_call_lifecycle_service_helper(context) -> None: settings = MagicMock() service = PlanLifecycleService(settings=settings) container = SimpleNamespace(plan_lifecycle_service=MagicMock(return_value=service)) + reloaded_plan_module = importlib.reload(plan_module) with patch( "cleveragents.application.container.get_container", return_value=container ): context.helper_settings = settings - context.helper_service = plan_module._get_lifecycle_service() + context.helper_service = reloaded_plan_module._get_lifecycle_service() @then("the lifecycle service helper should return a plan lifecycle service") diff --git a/features/steps/repl_input_modes_steps.py b/features/steps/repl_input_modes_steps.py index 6285f995a..efc415cf5 100644 --- a/features/steps/repl_input_modes_steps.py +++ b/features/steps/repl_input_modes_steps.py @@ -10,23 +10,11 @@ from unittest.mock import patch from behave import given, then, when from behave.runner import Context +from pydantic import ValidationError from cleveragents.cli.commands.repl import _expand_references, run_repl - - -class _FakeInput: - """Feed predefined lines and then terminate REPL with EOF.""" - - def __init__(self, lines: list[str]) -> None: - self._lines = list(lines) - self._idx = 0 - - def __call__(self, prompt: str = "") -> str: - if self._idx >= len(self._lines): - raise EOFError - line = self._lines[self._idx] - self._idx += 1 - return line +from cleveragents.cli.persona import PersonaConfig +from features.mocks.fake_repl_input import FakeReplInput def _run_repl_capture(lines: list[str], config_dir: Path) -> tuple[int, str]: @@ -36,7 +24,7 @@ def _run_repl_capture(lines: list[str], config_dir: Path) -> tuple[int, str]: output = StringIO() console = Console(file=output, no_color=True, width=180) - fake_input = _FakeInput(lines) + fake_input = FakeReplInput(lines) with ( patch("cleveragents.cli.commands.repl._console", console), patch("builtins.input", fake_input), @@ -53,8 +41,23 @@ def step_temp_repl_config(context: Context) -> None: context.add_cleanup(lambda: shutil.rmtree(str(config_dir), ignore_errors=True)) +@given("a deterministic REPL reference catalog fixture") +def step_repl_reference_fixture(context: Context) -> None: + fixture_dir = Path(tempfile.mkdtemp()) + (fixture_dir / "README.md").write_text("# fixture\n", encoding="utf-8") + context.repl_reference_fixture_dir = fixture_dir + context.add_cleanup(lambda: shutil.rmtree(str(fixture_dir), ignore_errors=True)) + + @given("a temporary persona export path") def step_temp_persona_export_path(context: Context) -> None: + export_path = Path("tmp-persona-export.yaml") + context.repl_mode_export_path = export_path + context.add_cleanup(lambda: export_path.unlink(missing_ok=True)) + + +@given("an absolute persona export path") +def step_absolute_persona_export_path(context: Context) -> None: export_path = Path(tempfile.mkdtemp()) / "persona-export.yaml" context.repl_mode_export_path = export_path context.add_cleanup( @@ -62,14 +65,40 @@ def step_temp_persona_export_path(context: Context) -> None: ) +@given("an invalid persona import file path") +def step_invalid_persona_import_file(context: Context) -> None: + invalid_path = Path("tmp-invalid-persona.yaml") + invalid_path.write_text("- bad\n- yaml\n", encoding="utf-8") + context.repl_mode_invalid_import_path = invalid_path + context.add_cleanup(lambda: invalid_path.unlink(missing_ok=True)) + + +@given("an absolute persona import file path") +def step_absolute_persona_import_path(context: Context) -> None: + temp_dir = Path(tempfile.mkdtemp()) + input_path = temp_dir / "persona-import.yaml" + input_path.write_text("name: x\nactor: local/mock-default\n", encoding="utf-8") + context.repl_mode_absolute_import_path = input_path + context.add_cleanup(lambda: shutil.rmtree(str(temp_dir), ignore_errors=True)) + + @when("I run the REPL with input lines") def step_run_repl_lines(context: Context) -> None: + replacements: dict[str, str] = {} export_path = getattr(context, "repl_mode_export_path", None) + if export_path is not None: + replacements["export_path"] = str(export_path) + invalid_import_path = getattr(context, "repl_mode_invalid_import_path", None) + if invalid_import_path is not None: + replacements["invalid_import_path"] = str(invalid_import_path) + absolute_import_path = getattr(context, "repl_mode_absolute_import_path", None) + if absolute_import_path is not None: + replacements["absolute_import_path"] = str(absolute_import_path) lines = [] for row in context.table: line = row["line"] - if export_path is not None: - line = line.format(export_path=str(export_path)) + if replacements: + line = line.format(**replacements) lines.append(line) code, output = _run_repl_capture(lines, context.repl_mode_config_dir) context.repl_mode_code = code @@ -85,7 +114,12 @@ def step_output_contains(context: Context, text: str) -> None: @when('I expand REPL references for "{line}"') def step_expand_references(context: Context, line: str) -> None: - context.expanded_repl_line = _expand_references(line) + fixture_dir = getattr(context, "repl_reference_fixture_dir", None) + if fixture_dir is None: + context.expanded_repl_line = _expand_references(line) + return + with patch("cleveragents.cli.commands.repl.Path.cwd", return_value=fixture_dir): + context.expanded_repl_line = _expand_references(line) @then('the expanded REPL line should contain "{text}"') @@ -93,3 +127,31 @@ def step_expanded_contains(context: Context, text: str) -> None: assert text in context.expanded_repl_line, ( f"Expected {text!r} in expanded line: {context.expanded_repl_line}" ) + + +@when('I validate CLI persona name "{name}"') +def step_validate_cli_persona_name(context: Context, name: str) -> None: + decoded = name.encode("utf-8").decode("unicode_escape") + context.repl_cli_persona_error = None + try: + PersonaConfig(name=decoded, actor="local/mock-default") + except ValidationError as exc: + context.repl_cli_persona_error = exc + + +@then('CLI persona validation should fail with "{message}"') +def step_cli_persona_validation_fails(context: Context, message: str) -> None: + assert context.repl_cli_persona_error is not None + assert message in str(context.repl_cli_persona_error) + + +@when('I validate CLI persona payload with color "{color}"') +def step_validate_cli_persona_payload_with_color(context: Context, color: str) -> None: + context.repl_cli_persona_payload = PersonaConfig.model_validate( + {"name": "colorful", "actor": "local/mock-default", "color": color} + ) + + +@then('CLI persona color should be "{color}"') +def step_cli_persona_color(context: Context, color: str) -> None: + assert context.repl_cli_persona_payload.color == color diff --git a/features/steps/repl_steps.py b/features/steps/repl_steps.py index af92f890a..fd57854d8 100644 --- a/features/steps/repl_steps.py +++ b/features/steps/repl_steps.py @@ -14,29 +14,13 @@ from cleveragents.cli.commands.repl import ( _get_prompt_context, run_repl, ) +from features.mocks.fake_repl_input import FakeReplInput # --------------------------------------------------------------------------- # Helpers # --------------------------------------------------------------------------- -class _FakeInput: - """Simulate user input for the REPL by feeding lines then raising EOFError.""" - - def __init__(self, lines: list[str]) -> None: - self._lines = list(lines) - self._index = 0 - - def __call__(self, prompt: str = "") -> str: - if self._index >= len(self._lines): - raise EOFError - line = self._lines[self._index] - self._index += 1 - if line == "__KEYBOARD_INTERRUPT__": - raise KeyboardInterrupt - return line - - def _run_repl_with_input( lines: list[str], *, @@ -55,7 +39,7 @@ def _run_repl_with_input( if history_path is not None: kw["history_path"] = history_path - fake = _FakeInput(lines) + fake = FakeReplInput(lines, interrupt_token="__KEYBOARD_INTERRUPT__") with ( patch("cleveragents.cli.commands.repl._console", console), patch("builtins.input", fake), diff --git a/features/steps/tui_input_modes_steps.py b/features/steps/tui_input_modes_steps.py index a1949509c..3d2c34d78 100644 --- a/features/steps/tui_input_modes_steps.py +++ b/features/steps/tui_input_modes_steps.py @@ -2,11 +2,19 @@ from __future__ import annotations -from behave import then, when +import os +import shutil +import tempfile +from pathlib import Path +from unittest.mock import patch + +from behave import given, then, when from behave.runner import Context +from cleveragents.tui.input import reference_parser from cleveragents.tui.input.modes import InputModeRouter -from cleveragents.tui.input.reference_parser import parse_references +from cleveragents.tui.input.reference_parser import parse_references, suggestions +from cleveragents.tui.input.shell_exec import looks_dangerous @when('I parse TUI references for "{line}"') @@ -40,3 +48,69 @@ def step_shell_stdout(context: Context, value: str) -> None: shell = context.tui_mode_result.shell_result assert shell is not None assert value in shell.stdout + + +@then('the TUI shell stderr should contain "{value}"') +def step_shell_stderr(context: Context, value: str) -> None: + shell = context.tui_mode_result.shell_result + assert shell is not None + assert value in shell.stderr + + +@when('I check shell danger detection for "{command}"') +def step_check_shell_danger(context: Context, command: str) -> None: + context.shell_danger_result = looks_dangerous(command) + + +@then('shell danger detection result should be "{value}"') +def step_shell_danger_result(context: Context, value: str) -> None: + expected = value.strip().lower() == "true" + assert context.shell_danger_result is expected + + +@given("a deterministic TUI reference fixture") +def step_deterministic_tui_reference_fixture(context: Context) -> None: + fixture_dir = Path(tempfile.mkdtemp()) + (fixture_dir / "README.md").write_text("# fixture\n", encoding="utf-8") + (fixture_dir / ".git").mkdir(exist_ok=True) + (fixture_dir / ".git" / "secret.txt").write_text("secret\n", encoding="utf-8") + context.tui_reference_fixture_dir = fixture_dir + context.add_cleanup(lambda: shutil.rmtree(str(fixture_dir), ignore_errors=True)) + + +@when("I parse and suggest TUI references using fixture") +def step_parse_and_suggest_with_fixture(context: Context) -> None: + context.tui_walk_count = 0 + context.tui_fixture_suggestions = [] + reference_parser._catalog_cache["cwd"] = None + reference_parser._catalog_cache["created_at"] = 0.0 + reference_parser._catalog_cache["catalog"] = None + real_walk = os.walk + + def counting_walk(*args: object, **kwargs: object): + context.tui_walk_count += 1 + return real_walk(*args, **kwargs) + + with ( + patch( + "cleveragents.tui.input.reference_parser.Path.cwd", + return_value=context.tui_reference_fixture_dir, + ), + patch( + "cleveragents.tui.input.reference_parser.os.walk", side_effect=counting_walk + ), + ): + parse_references("inspect @README.md") + context.tui_fixture_suggestions = suggestions("secret", category="resource") + + +@then('TUI walk count should be "{count}"') +def step_tui_walk_count(context: Context, count: str) -> None: + assert context.tui_walk_count == int(count) + + +@then('TUI suggestions should not include "{value}"') +def step_tui_suggestions_not_include(context: Context, value: str) -> None: + assert all( + value not in suggestion for suggestion in context.tui_fixture_suggestions + ) diff --git a/features/steps/tui_persona_system_steps.py b/features/steps/tui_persona_system_steps.py index 1253d50e4..10a59a1dc 100644 --- a/features/steps/tui_persona_system_steps.py +++ b/features/steps/tui_persona_system_steps.py @@ -4,13 +4,14 @@ from __future__ import annotations import shutil import tempfile +import threading from pathlib import Path from behave import given, then, when from behave.runner import Context from cleveragents.tui.persona.registry import PersonaRegistry -from cleveragents.tui.persona.schema import Persona +from cleveragents.tui.persona.schema import Persona, PersonaPreset from cleveragents.tui.persona.state import PersonaState @@ -50,6 +51,25 @@ def step_given_save_persona(context: Context, name: str, actor: str) -> None: step_save_persona(context, name, actor) +@given( + 'I save TUI persona "{name}" with actor "{actor}" and extra preset "{preset_name}"' +) +def step_save_persona_with_extra_preset( + context: Context, name: str, actor: str, preset_name: str +) -> None: + persona = Persona( + name=name, + actor=actor, + argument_presets=[ + PersonaPreset(name="default", display="default", overrides={}), + PersonaPreset( + name=preset_name, display=preset_name, overrides={"mode": "x"} + ), + ], + ) + context.tui_registry.save(persona) + + @given( 'I save a TUI persona named "{name}" with actor "{actor}" and cycle order {cycle:d}' ) @@ -103,3 +123,93 @@ def step_set_active_persona( def step_active_persona(context: Context, session_id: str, persona_name: str) -> None: persona = context.tui_state.active_persona(session_id) assert persona.name == persona_name + + +@when('I cycle persona preset for session "{session_id}"') +def step_cycle_preset(context: Context, session_id: str) -> None: + if not hasattr(context, "tui_state"): + context.tui_state = PersonaState(registry=context.tui_registry) + context.tui_state.cycle_preset(session_id) + + +@then('current preset for session "{session_id}" should be "{preset_name}"') +def step_current_preset(context: Context, session_id: str, preset_name: str) -> None: + assert context.tui_state.current_preset(session_id) == preset_name + + +@given('I create malformed TUI persona file "{name}"') +def step_create_malformed_persona_file(context: Context, name: str) -> None: + malformed = context.tui_registry.personas_dir / f"{name}.yaml" + context.tui_registry.ensure_dirs() + malformed.write_text("name: broken\nactor:\n- not-a-string\n", encoding="utf-8") + + +@when("I list TUI personas") +def step_list_tui_personas(context: Context) -> None: + context.tui_list_error = None + context.tui_persona_names = [] + try: + personas = context.tui_registry.list_personas() + context.tui_persona_names = [persona.name for persona in personas] + except Exception as exc: + context.tui_list_error = exc + + +@then('TUI persona list should include "{name}"') +def step_tui_persona_list_includes(context: Context, name: str) -> None: + assert context.tui_list_error is None + assert name in context.tui_persona_names + + +@when('I export TUI persona "{name}" to "{output_path}"') +def step_export_tui_persona(context: Context, name: str, output_path: str) -> None: + context.tui_export_error = None + try: + context.tui_registry.export_persona(name, Path(output_path)) + except Exception as exc: + context.tui_export_error = exc + + +@then('TUI export should fail with "{message}"') +def step_tui_export_fails(context: Context, message: str) -> None: + assert context.tui_export_error is not None + assert message in str(context.tui_export_error) + + +@when('I import TUI persona from "{input_path}"') +def step_import_tui_persona(context: Context, input_path: str) -> None: + context.tui_import_error = None + try: + context.tui_registry.import_persona(Path(input_path)) + except Exception as exc: + context.tui_import_error = exc + + +@then('TUI import should fail with "{message}"') +def step_tui_import_fails(context: Context, message: str) -> None: + assert context.tui_import_error is not None + assert message in str(context.tui_import_error) + + +@when("I concurrently set TUI last persona names") +def step_concurrent_set_last_persona_names(context: Context) -> None: + context.tui_concurrency_errors = [] + + def _worker(persona_name: str) -> None: + try: + context.tui_registry.set_last_persona(persona_name) + except Exception as exc: # pragma: no cover - assertion handled after join + context.tui_concurrency_errors.append(exc) + + threads: list[threading.Thread] = [] + for row in context.table: + thread = threading.Thread(target=_worker, args=(row["name"],)) + threads.append(thread) + thread.start() + for thread in threads: + thread.join() + + +@then("TUI concurrent update should complete without errors") +def step_tui_concurrent_update_no_errors(context: Context) -> None: + assert context.tui_concurrency_errors == [] diff --git a/features/tui_input_modes.feature b/features/tui_input_modes.feature index a12bb8d3e..0e39fb770 100644 --- a/features/tui_input_modes.feature +++ b/features/tui_input_modes.feature @@ -11,6 +11,23 @@ Feature: TUI input modes And the TUI command result should be "ok:/help" Scenario: Shell mode runs host command - When I route TUI input "!printf hello" + When I route TUI input "!echo hello" Then the TUI mode should be "shell" And the TUI shell stdout should contain "hello" + + Scenario: Shell mode blocks dangerous command by default + When I route TUI input "!rm -rf /" + Then the TUI mode should be "shell" + And the TUI shell stderr should contain "blocked dangerous shell command" + + Scenario: Dangerous shell pattern detection helper + When I check shell danger detection for "rm -rf /tmp" + Then shell danger detection result should be "true" + When I check shell danger detection for "echo hello" + Then shell danger detection result should be "false" + + Scenario: Reference catalog ignores .git and caches scans + Given a deterministic TUI reference fixture + When I parse and suggest TUI references using fixture + Then TUI walk count should be "1" + And TUI suggestions should not include ".git/secret.txt" diff --git a/features/tui_persona_system.feature b/features/tui_persona_system.feature index be421b5b9..95605e5ac 100644 --- a/features/tui_persona_system.feature +++ b/features/tui_persona_system.feature @@ -25,3 +25,39 @@ Feature: TUI persona system And I set active persona to "reviewer" for session "s2" Then active persona for session "s1" should be "dev" And active persona for session "s2" should be "reviewer" + + Scenario: Persona state cycles through presets per session + Given a temporary TUI persona registry + And I save TUI persona "cycler" with actor "local/mock-default" and extra preset "focused" + When I set active persona to "cycler" for session "s1" + And I cycle persona preset for session "s1" + Then current preset for session "s1" should be "focused" + When I cycle persona preset for session "s1" + Then current preset for session "s1" should be "default" + + Scenario: Malformed persona YAML does not break registry operations + Given a temporary TUI persona registry + And I save a TUI persona named "good" with actor "local/mock-default" + And I create malformed TUI persona file "broken" + When I list TUI personas + Then TUI persona list should include "good" + When I save a TUI persona named "stillworks" with actor "local/mock-default" + Then loading persona "stillworks" should succeed + + Scenario: TUI registry rejects unsafe export and import paths + Given a temporary TUI persona registry + And I save a TUI persona named "safe" with actor "local/mock-default" + When I export TUI persona "safe" to "../escape.yaml" + Then TUI export should fail with "stay within working directory" + When I import TUI persona from "../escape.yaml" + Then TUI import should fail with "stay within working directory" + + Scenario: TUI registry state writes tolerate concurrent updates + Given a temporary TUI persona registry + When I concurrently set TUI last persona names + | name | + | a | + | b | + | c | + | d | + Then TUI concurrent update should complete without errors diff --git a/robot/tui_smoke.robot b/robot/tui_smoke.robot index 0e229afd2..48f828648 100644 --- a/robot/tui_smoke.robot +++ b/robot/tui_smoke.robot @@ -8,3 +8,35 @@ TUI Headless Startup Check Returns JSON Should Be Equal As Integers ${result.rc} 0 Should Contain ${result.stdout} textual_available Should Contain ${result.stdout} default_persona + +TUI Headless Works When Shell Disabled + ${result}= Run Process ${PYTHON} -m cleveragents tui --headless shell=False stderr=STDOUT env:CLEVERAGENTS_DATABASE_URL=sqlite:///:memory: env:CLEVERAGENTS_DISABLE_SHELL_MODE=1 + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} textual_available + Should Contain ${result.stdout} default_persona + +TUI Input Mode Router And Prompt Widget Behavior + ${script}= Catenate SEPARATOR=\n + ... from cleveragents.tui.input.modes import InputModeRouter + ... from cleveragents.tui.widgets.prompt import PromptInput + ... prompt = PromptInput() + ... prompt.value = "hello" + ... payload = prompt.consume_text() + ... assert payload.text == "hello" + ... assert prompt.value == "" + ... router = InputModeRouter(command_handler=lambda raw: f"ok:{raw}") + ... command_result = router.process("/help") + ... assert command_result.mode.value == "command" + ... assert command_result.command_result == "ok:help" + ... normal_result = router.process("inspect @README.md") + ... assert normal_result.mode.value == "normal" + ... print("router-widget-ok") + ${result}= Run Process ${PYTHON} -c ${script} shell=False stderr=STDOUT + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} router-widget-ok + +TUI Headless Includes Router Help Payload + ${result}= Run Process ${PYTHON} -m cleveragents tui --headless shell=False stderr=STDOUT env:CLEVERAGENTS_DATABASE_URL=sqlite:///:memory: + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} "help" + Should Contain ${result.stdout} /persona diff --git a/src/cleveragents/application/container.py b/src/cleveragents/application/container.py index 3120c93fe..0a53d0966 100644 --- a/src/cleveragents/application/container.py +++ b/src/cleveragents/application/container.py @@ -96,6 +96,8 @@ from cleveragents.providers.registry import ProviderRegistry, get_provider_regis from cleveragents.reactive.route_bridge import RouteBridge from cleveragents.reactive.stream_router import ReactiveStreamRouter from cleveragents.shared.redaction import redact_value +from cleveragents.tui.persona.registry import PersonaRegistry +from cleveragents.tui.persona.state import PersonaState _logger = structlog.get_logger(__name__) @@ -539,6 +541,10 @@ class Container(containers.DeclarativeContainer): plan_service=plan_service, ) + # TUI persona wiring through DI container + persona_registry = providers.Factory(PersonaRegistry) + persona_state = providers.Factory(PersonaState, registry=persona_registry) + # Metrics Emitter - structured metric emission (Forgejo #579) metrics_emitter = providers.Singleton( MetricsEmitter.from_settings, diff --git a/src/cleveragents/application/services/resource_file_watcher.py b/src/cleveragents/application/services/resource_file_watcher.py index 2f9ffb4e2..645281ad4 100644 --- a/src/cleveragents/application/services/resource_file_watcher.py +++ b/src/cleveragents/application/services/resource_file_watcher.py @@ -29,6 +29,7 @@ from watchdog.events import ( FileSystemEventHandler, ) from watchdog.observers import Observer # type: ignore[import-untyped] +from watchdog.observers.polling import PollingObserver # type: ignore[import-untyped] from cleveragents.infrastructure.events.models import DomainEvent from cleveragents.infrastructure.events.protocol import EventBus @@ -100,6 +101,7 @@ class ResourceFileWatcher: # resolved path (str) -> pending debounce Timer self._pending_timers: dict[str, threading.Timer] = {} self._running = False + self._using_polling = False _logger.debug( "file_watcher.initialized", @@ -157,11 +159,28 @@ class ResourceFileWatcher: and parent_str not in self._dir_watches ): handler = _ResourceChangeHandler(self) - watch_handle = self._observer.schedule( - handler, - parent_str, - recursive=False, - ) + try: + watch_handle = self._observer.schedule( + handler, + parent_str, + recursive=False, + ) + except OSError as exc: + if not self._using_polling: + _logger.warning( + "file_watcher.watch_schedule_fallback", + error_type=type(exc).__name__, + error_message=str(exc), + fallback="polling_observer", + ) + self._switch_to_polling_observer_locked() + watch_handle = self._observer.schedule( # type: ignore[union-attr] + handler, + parent_str, + recursive=False, + ) + else: + raise self._dir_watches[parent_str] = watch_handle _logger.debug( @@ -229,23 +248,20 @@ class ResourceFileWatcher: if self._running: msg = "File watcher is already running" raise RuntimeError(msg) - observer = Observer() - observer.daemon = True - - handler = _ResourceChangeHandler(self) - seen_dirs: set[str] = set() - for path_str in self._watched_paths: - parent_str = str(Path(path_str).parent) - if parent_str not in seen_dirs: - watch_handle = observer.schedule( - handler, - parent_str, - recursive=False, - ) - self._dir_watches[parent_str] = watch_handle - seen_dirs.add(parent_str) - - observer.start() + observer = self._build_observer(use_polling=False) + try: + observer.start() + self._using_polling = False + except OSError as exc: + _logger.warning( + "file_watcher.inotify_unavailable", + error_type=type(exc).__name__, + error_message=str(exc), + fallback="polling_observer", + ) + observer = self._build_observer(use_polling=True) + observer.start() + self._using_polling = True self._observer = observer self._running = True @@ -255,6 +271,38 @@ class ResourceFileWatcher: watched_dirs=len(self._dir_watches), ) + def _build_observer(self, *, use_polling: bool) -> Observer: # type: ignore[valid-type] + observer: Observer = ( # type: ignore[valid-type] + PollingObserver() if use_polling else Observer() + ) + observer.daemon = True + self._dir_watches.clear() + handler = _ResourceChangeHandler(self) + seen_dirs: set[str] = set() + for path_str in self._watched_paths: + parent_str = str(Path(path_str).parent) + if parent_str in seen_dirs: + continue + watch_handle = observer.schedule( + handler, + parent_str, + recursive=False, + ) + self._dir_watches[parent_str] = watch_handle + seen_dirs.add(parent_str) + return observer + + def _switch_to_polling_observer_locked(self) -> None: + """Swap current observer to polling backend while lock is held.""" + old_observer = self._observer + if old_observer is not None: + old_observer.stop() + old_observer.join(timeout=5.0) + polling_observer = self._build_observer(use_polling=True) + polling_observer.start() + self._observer = polling_observer + self._using_polling = True + def stop(self) -> None: """Stop the watchdog observer and cancel pending timers.""" observer: Observer | None = None # type: ignore[valid-type] @@ -268,6 +316,7 @@ class ResourceFileWatcher: self._observer = None self._dir_watches.clear() self._running = False + self._using_polling = False # Join outside lock to avoid deadlock with timer callbacks. if observer is not None: diff --git a/src/cleveragents/cli/commands/repl.py b/src/cleveragents/cli/commands/repl.py index d7d09f3da..c2c321574 100644 --- a/src/cleveragents/cli/commands/repl.py +++ b/src/cleveragents/cli/commands/repl.py @@ -13,6 +13,7 @@ import readline import shlex import subprocess import sys +import time from collections.abc import Sequence from dataclasses import dataclass from pathlib import Path @@ -23,6 +24,7 @@ import yaml from rich.console import Console from cleveragents.cli.persona import PersonaConfig, PersonaRegistry +from cleveragents.tui.input.shell_exec import looks_dangerous # --------------------------------------------------------------------------- # Constants @@ -33,6 +35,10 @@ _DEFAULT_HISTORY_PATH = _DEFAULT_HISTORY_DIR / "history" _MULTILINE_CONTINUATION = "\\" _LAST_COMMAND_TOKEN = "!!" _MAX_REFERENCE_FILES = 300 +_SHELL_TIMEOUT_SECONDS = 30 +_IGNORED_DIRS = {".git", ".venv", "node_modules", "__pycache__", ".mypy_cache"} +_REFERENCE_CACHE_TTL_SECONDS = 5.0 +_reference_cache: dict[str, object] = {"cwd": None, "created_at": 0.0, "catalog": None} # The subset of first-position CLI commands exposed inside the REPL. # Mirrors the ``valid_cmds`` list in ``main.py`` minus the flags. @@ -194,15 +200,33 @@ def _print_repl_help() -> None: def _build_reference_catalog() -> dict[str, list[str]]: """Build lightweight catalog for @ reference expansion.""" cwd = Path.cwd() + now = time.time() + cached_cwd = _reference_cache.get("cwd") + cached_time = _reference_cache.get("created_at") + cached_catalog = _reference_cache.get("catalog") + if ( + isinstance(cached_cwd, Path) + and cached_cwd == cwd + and isinstance(cached_time, float) + and now - cached_time < _REFERENCE_CACHE_TTL_SECONDS + and isinstance(cached_catalog, dict) + ): + return cached_catalog + files: list[str] = [] - for path in cwd.rglob("*"): - if len(files) >= _MAX_REFERENCE_FILES: - break - if path.is_file(): + for root, dirs, filenames in os.walk(cwd, followlinks=False): + dirs[:] = [name for name in dirs if name not in _IGNORED_DIRS] + root_path = Path(root) + for filename in filenames: + if len(files) >= _MAX_REFERENCE_FILES: + break + path = root_path / filename try: files.append(str(path.relative_to(cwd).as_posix())) except ValueError: files.append(path.as_posix()) + if len(files) >= _MAX_REFERENCE_FILES: + break actors: set[str] = set() actor_examples = cwd / "examples" / "actors" @@ -228,7 +252,7 @@ def _build_reference_catalog() -> dict[str, list[str]]: project_env = os.environ.get("CLEVERAGENTS_PROJECT", "") plan_env = os.environ.get("CLEVERAGENTS_PLAN", "") - return { + catalog = { "file": sorted(files), "actor": sorted(actors), "tool": sorted(tools), @@ -236,6 +260,10 @@ def _build_reference_catalog() -> dict[str, list[str]]: "project": [project_env] if project_env else [], "plan": [plan_env] if plan_env else [], } + _reference_cache["cwd"] = cwd + _reference_cache["created_at"] = now + _reference_cache["catalog"] = catalog + return catalog def _best_fuzzy_match(query: str, options: list[str]) -> str | None: @@ -270,9 +298,11 @@ def _find_reference_candidates( return (prefix + contains + fuzzy)[:limit] -def _expand_references(line: str) -> str: +def _expand_references( + line: str, *, catalog: dict[str, list[str]] | None = None +) -> str: """Expand inline @ tokens to canonical references when possible.""" - catalog = _build_reference_catalog() + active_catalog = catalog or _build_reference_catalog() tokens = line.split() expanded_tokens: list[str] = [] @@ -289,12 +319,12 @@ def _expand_references(line: str) -> str: query = raw if ":" in raw: maybe_category, rest = raw.split(":", 1) - if maybe_category in catalog: + if maybe_category in active_catalog: category = maybe_category query = rest if category: - candidate = _best_fuzzy_match(query, catalog.get(category, [])) + candidate = _best_fuzzy_match(query, active_catalog.get(category, [])) if candidate: expanded_tokens.append(f"@{category}:{candidate}") continue @@ -303,7 +333,7 @@ def _expand_references(line: str) -> str: best_category = "" best_value = "" - for candidate_category, values in catalog.items(): + for candidate_category, values in active_catalog.items(): candidate = _best_fuzzy_match(query, values) if candidate: best_category = candidate_category @@ -317,9 +347,11 @@ def _expand_references(line: str) -> str: return " ".join(expanded_tokens) -def _reference_suggestions(line: str) -> list[str]: +def _reference_suggestions( + line: str, *, catalog: dict[str, list[str]] | None = None +) -> list[str]: """Return lightweight suggestion hints for unresolved @ tokens.""" - catalog = _build_reference_catalog() + active_catalog = catalog or _build_reference_catalog() for token in line.split(): if not token.startswith("@") or token.startswith("\\@"): continue @@ -328,14 +360,14 @@ def _reference_suggestions(line: str) -> list[str]: continue if ":" in raw: category, query = raw.split(":", 1) - if category in catalog: + if category in active_catalog: candidates = _find_reference_candidates( - query, catalog[category], limit=5 + query, active_catalog[category], limit=5 ) return [f"@{category}:{value}" for value in candidates] else: merged: list[str] = [] - for category, values in catalog.items(): + for category, values in active_catalog.items(): for value in _find_reference_candidates(raw, values, limit=2): merged.append(f"@{category}:{value}") return merged[:5] @@ -343,11 +375,37 @@ def _reference_suggestions(line: str) -> list[str]: def _run_shell_command(command_text: str) -> int: - """Execute a shell command and print output.""" + """Execute a shell command with minimal safeguards. + + Threat model: + - Shell mode is intended for trusted local development workflows. + - It is not a sandbox and must never be exposed as a remote execution surface. + - Dangerous patterns require explicit user confirmation before execution. + """ if not command_text.strip(): _console.print("[yellow]Shell mode requires a command after '!'.[/yellow]") return 2 - proc = subprocess.run(command_text, shell=True, text=True, capture_output=True) # nosec B602 + if os.environ.get("CLEVERAGENTS_DISABLE_SHELL_MODE", "").strip() in {"1", "true"}: + _console.print("[yellow]Shell mode is disabled by configuration.[/yellow]") + return 2 + if looks_dangerous(command_text): + answer = input("Dangerous command detected. Continue? [y/N]: ").strip().lower() + if answer not in {"y", "yes"}: + _console.print("[yellow]Blocked dangerous shell command.[/yellow]") + return 1 + try: + proc = subprocess.run( + command_text, + shell=True, + text=True, + capture_output=True, + timeout=_SHELL_TIMEOUT_SECONDS, + ) # nosec B602 + except subprocess.TimeoutExpired: + _console.print( + f"[red]Shell command timed out after {_SHELL_TIMEOUT_SECONDS}s.[/red]" + ) + return 124 if proc.stdout: _console.print(proc.stdout.rstrip("\n")) if proc.stderr: @@ -507,8 +565,12 @@ def _handle_slash_command( if not deleted: _console.print(f"[red]Persona not found:[/red] {target}") return 1, current_session - if sessions[current_session].active_persona == target: - sessions[current_session].active_persona = "default" + reset_any = False + for state in sessions.values(): + if state.active_persona == target: + state.active_persona = "default" + reset_any = True + if reset_any: registry.set_active_persona_name("default") _console.print(f"[green]Deleted persona:[/green] {target}") return 0, current_session @@ -521,26 +583,34 @@ def _handle_slash_command( if persona is None: _console.print(f"[red]Persona not found:[/red] {target}") return 1, current_session - output_path = ( - Path(tokens[3]) if len(tokens) > 3 else Path.cwd() / f"{target}.yaml" - ) + output_path = Path(tokens[3]) if len(tokens) > 3 else Path(f"{target}.yaml") + try: + safe_output_path = registry.resolve_export_path(output_path) + except ValueError as exc: + _console.print(f"[red]{exc}[/red]") + return 1, current_session payload = persona.model_dump(mode="json", exclude_none=True) - output_path.write_text( + safe_output_path.write_text( yaml.safe_dump(payload, sort_keys=False, default_flow_style=False), encoding="utf-8", ) - _console.print(f"[green]Exported persona:[/green] {output_path}") + _console.print(f"[green]Exported persona:[/green] {safe_output_path}") return 0, current_session if tokens[1] == "import": if len(tokens) < 3: _console.print("[red]Usage:[/red] /persona import ") return 2, current_session input_path = Path(tokens[2]) - if not input_path.exists(): - _console.print(f"[red]File not found:[/red] {input_path}") + try: + safe_input_path = registry.resolve_import_path(input_path) + except ValueError as exc: + _console.print(f"[red]{exc}[/red]") + return 1, current_session + if not safe_input_path.exists(): + _console.print(f"[red]File not found:[/red] {safe_input_path}") return 1, current_session - raw = yaml.safe_load(input_path.read_text(encoding="utf-8")) or {} + raw = yaml.safe_load(safe_input_path.read_text(encoding="utf-8")) or {} if not isinstance(raw, dict): _console.print("[red]Invalid persona YAML.[/red]") return 1, current_session @@ -714,11 +784,12 @@ def run_repl( continue # Normal mode @ references - expanded_line = _expand_references(line) + reference_catalog = _build_reference_catalog() if "@" in line else None + expanded_line = _expand_references(line, catalog=reference_catalog) if expanded_line != line: _console.print(f"[dim]{expanded_line}[/dim]") elif "@" in line: - hints = _reference_suggestions(line) + hints = _reference_suggestions(line, catalog=reference_catalog) if hints: _console.print("[dim]Reference suggestions:[/dim] " + ", ".join(hints)) line = expanded_line diff --git a/src/cleveragents/cli/commands/tui.py b/src/cleveragents/cli/commands/tui.py index d52f68c4b..0ad83620f 100644 --- a/src/cleveragents/cli/commands/tui.py +++ b/src/cleveragents/cli/commands/tui.py @@ -6,8 +6,6 @@ from typing import Annotated import typer -from cleveragents.tui.commands import run_tui - app = typer.Typer(help="Launch the Textual TUI.") @@ -22,4 +20,7 @@ def tui_callback( ] = False, ) -> None: """Launch the CleverAgents TUI.""" + # Import lazily so non-TUI commands avoid Textual startup cost. + from cleveragents.tui.commands import run_tui + raise typer.Exit(run_tui(headless=headless)) diff --git a/src/cleveragents/cli/persona.py b/src/cleveragents/cli/persona.py index aa1ca2c36..cdc69a336 100644 --- a/src/cleveragents/cli/persona.py +++ b/src/cleveragents/cli/persona.py @@ -1,177 +1,51 @@ -"""Persona schema and local YAML storage for interactive CLI/TUI usage. - -Personas are intentionally a presentation-layer concept. They are stored as -YAML files under the user config directory and never persisted in domain -models or database tables. -""" +"""CLI compatibility layer for canonical TUI persona modules.""" from __future__ import annotations -import os -from dataclasses import dataclass, field from pathlib import Path -from typing import Any -import yaml -from pydantic import BaseModel, ConfigDict, Field, field_validator +from cleveragents.tui.persona.registry import PersonaRegistry as _TuiPersonaRegistry +from cleveragents.tui.persona.schema import Persona as PersonaConfig -def _default_config_dir() -> Path: - from_env = os.environ.get("CLEVERAGENTS_CONFIG_DIR", "").strip() - if from_env: - return Path(from_env) - return Path.home() / ".config" / "cleveragents" +class PersonaRegistry(_TuiPersonaRegistry): + """Backwards-compatible API for CLI callers. - -class PersonaPreset(BaseModel): - """Named argument overlay for a persona.""" - - name: str = Field(..., min_length=1) - display: str = Field(..., min_length=1) - overrides: dict[str, Any] = Field(default_factory=dict) - - model_config = ConfigDict(str_strip_whitespace=True, validate_assignment=True) - - -class PersonaConfig(BaseModel): - """Persona definition loaded from YAML.""" - - name: str = Field(..., min_length=1) - description: str = Field(default="") - icon: str = Field(default="") - actor: str = Field(..., min_length=1) - base_arguments: dict[str, Any] = Field(default_factory=dict) - scoped_projects: list[str] = Field(default_factory=list) - scoped_plans: list[str] = Field(default_factory=list) - argument_presets: list[PersonaPreset] = Field(default_factory=list) - cycle_order: int = Field(default=0, ge=0) - greeting: str = Field(default="") - - @field_validator("actor") - @classmethod - def _validate_actor(cls, value: str) -> str: - if "/" not in value: - raise ValueError("actor must be namespaced (namespace/name)") - return value - - @field_validator("argument_presets") - @classmethod - def _validate_default_preset( - cls, value: list[PersonaPreset] - ) -> list[PersonaPreset]: - if not value: - return [PersonaPreset(name="default", display="default", overrides={})] - defaults = [preset for preset in value if preset.name == "default"] - if len(defaults) != 1: - raise ValueError("persona must contain exactly one 'default' preset") - if defaults[0].overrides: - raise ValueError("'default' preset must have empty overrides") - return value - - model_config = ConfigDict(str_strip_whitespace=True, validate_assignment=True) - - -@dataclass(slots=True) -class PersonaRegistry: - """Loads and persists persona YAML files from user config space.""" - - config_dir: Path = field(default_factory=_default_config_dir) - - @property - def personas_dir(self) -> Path: - return self.config_dir / "personas" + Canonical persona schema/registry live in `cleveragents.tui.persona.*`. + This class preserves old method names used by REPL and CLI command code. + """ @property def state_file(self) -> Path: - return self.config_dir / "tui-state.yaml" + return self.state_path - def ensure_dirs(self) -> None: - self.personas_dir.mkdir(parents=True, exist_ok=True) + @property + def personas_lock_file(self) -> Path: + return self.personas_lock_path - def persona_path(self, name: str) -> Path: - return self.personas_dir / f"{name}.yaml" + @property + def state_lock_file(self) -> Path: + return self.state_lock_path def list_personas(self) -> list[PersonaConfig]: - self.ensure_dirs() - personas: list[PersonaConfig] = [] - for path in sorted(self.personas_dir.glob("*.y*ml")): - raw = yaml.safe_load(path.read_text(encoding="utf-8")) or {} - if not isinstance(raw, dict): - continue - personas.append(PersonaConfig.model_validate(raw)) - return personas + return super().list_personas() def get_persona(self, name: str) -> PersonaConfig | None: - path = self.persona_path(name) - if not path.exists(): - return None - raw = yaml.safe_load(path.read_text(encoding="utf-8")) or {} - if not isinstance(raw, dict): - return None - return PersonaConfig.model_validate(raw) + return super().get(name) def save_persona(self, persona: PersonaConfig) -> Path: - self.ensure_dirs() - if persona.cycle_order > 0: - for existing in self.list_personas(): - if ( - existing.name != persona.name - and existing.cycle_order == persona.cycle_order - ): - raise ValueError( - "cycle_order must be unique across personas " - "with cycle_order > 0" - ) - path = self.persona_path(persona.name) - payload = persona.model_dump(mode="json", exclude_none=True) - path.write_text( - yaml.safe_dump(payload, sort_keys=False, default_flow_style=False), - encoding="utf-8", - ) - return path + return super().save(persona) def delete_persona(self, name: str) -> bool: - path = self.persona_path(name) - if not path.exists(): - return False - path.unlink() - return True + return super().delete(name) def get_active_persona_name(self) -> str | None: - if not self.state_file.exists(): - return None - raw = yaml.safe_load(self.state_file.read_text(encoding="utf-8")) or {} - if not isinstance(raw, dict): - return None - active = raw.get("last_persona") - return active if isinstance(active, str) and active else None + return super().get_last_persona() def set_active_persona_name(self, name: str) -> None: - self.ensure_dirs() - state: dict[str, Any] = {} - if self.state_file.exists(): - loaded = yaml.safe_load(self.state_file.read_text(encoding="utf-8")) or {} - if isinstance(loaded, dict): - state = dict(loaded) - state["last_persona"] = name - self.state_file.write_text( - yaml.safe_dump(state, sort_keys=False, default_flow_style=False), - encoding="utf-8", - ) + super().set_last_persona(name) def ensure_default_persona( self, actor: str = "local/mock-default" ) -> PersonaConfig: - self.ensure_dirs() - existing = self.get_persona("default") - if existing is not None: - return existing - default_persona = PersonaConfig( - name="default", - description="Default persona", - actor=actor, - argument_presets=[PersonaPreset(name="default", display="default")], - ) - self.save_persona(default_persona) - self.set_active_persona_name(default_persona.name) - return default_persona + return super().ensure_default(actor=actor) diff --git a/src/cleveragents/tui/__init__.py b/src/cleveragents/tui/__init__.py index bd33abd42..650585e82 100644 --- a/src/cleveragents/tui/__init__.py +++ b/src/cleveragents/tui/__init__.py @@ -3,7 +3,14 @@ from __future__ import annotations from cleveragents.tui.app import CleverAgentsTuiApp -from cleveragents.tui.commands import run_tui + + +def run_tui(*, headless: bool = False) -> int: + """Run TUI command entrypoint lazily to avoid import cycles.""" + from cleveragents.tui.commands import run_tui as _run_tui + + return _run_tui(headless=headless) + __all__ = [ "CleverAgentsTuiApp", diff --git a/src/cleveragents/tui/app.py b/src/cleveragents/tui/app.py index 918f2585e..214a058a1 100644 --- a/src/cleveragents/tui/app.py +++ b/src/cleveragents/tui/app.py @@ -3,8 +3,9 @@ from __future__ import annotations import importlib +import os from dataclasses import dataclass -from typing import Any, ClassVar, Protocol +from typing import TYPE_CHECKING, Any, ClassVar, Protocol from cleveragents.tui.input.modes import InputMode, InputModeRouter from cleveragents.tui.input.reference_parser import suggestions @@ -14,6 +15,11 @@ from cleveragents.tui.widgets.prompt import PromptInput from cleveragents.tui.widgets.reference_picker import ReferencePickerOverlay from cleveragents.tui.widgets.slash_command_overlay import SlashCommandOverlay +if TYPE_CHECKING: + InputSubmittedEvent = Any +else: + InputSubmittedEvent = Any + _TEXTUAL_AVAILABLE = False _TextualApp: type[Any] = object _Vertical: type[Any] = object @@ -135,7 +141,7 @@ if _TEXTUAL_AVAILABLE: scope_text=scope_text, ) - def on_input_submitted(self, event: object) -> None: + def on_input_submitted(self, event: InputSubmittedEvent) -> None: del event prompt = self.query_one("#prompt", PromptInput) payload = prompt.consume_text() @@ -146,7 +152,11 @@ if _TEXTUAL_AVAILABLE: mode_router = InputModeRouter( command_handler=lambda raw: self._command_router.handle( raw, session_id=self._session.session_id - ) + ), + shell_confirm=lambda _cmd: ( + os.environ.get("CLEVERAGENTS_ALLOW_DANGEROUS_SHELL", "").strip() + in {"1", "true"} + ), ) result = mode_router.process(text) conversation = self.query_one("#conversation", _Static) diff --git a/src/cleveragents/tui/commands.py b/src/cleveragents/tui/commands.py index cab175faa..1134e995e 100644 --- a/src/cleveragents/tui/commands.py +++ b/src/cleveragents/tui/commands.py @@ -51,9 +51,8 @@ class TuiCommandRouter: def run_tui(*, headless: bool = False) -> int: """Run the Textual TUI app or a headless startup check.""" container = get_container() - _ = container.tui_adapters() - registry = PersonaRegistry() - state = PersonaState(registry=registry) + registry = container.persona_registry() + state = container.persona_state(registry=registry) router = TuiCommandRouter(persona_registry=registry, persona_state=state) if headless: diff --git a/src/cleveragents/tui/input/modes.py b/src/cleveragents/tui/input/modes.py index 920fdf121..76231444c 100644 --- a/src/cleveragents/tui/input/modes.py +++ b/src/cleveragents/tui/input/modes.py @@ -35,8 +35,16 @@ class ModeResult: class InputModeRouter: """Dispatch user input to normal/command/shell handlers.""" - def __init__(self, command_handler: Callable[[str], str]) -> None: + def __init__( + self, + command_handler: Callable[[str], str], + *, + shell_confirm: Callable[[str], bool] | None = None, + shell_timeout_seconds: int = 30, + ) -> None: self._command_handler = command_handler + self._shell_confirm = shell_confirm + self._shell_timeout_seconds = shell_timeout_seconds @staticmethod def detect_mode(text: str) -> InputMode: @@ -61,7 +69,11 @@ class InputModeRouter: ) if mode == InputMode.SHELL: command = text.lstrip()[1:].strip() - shell_result = run_shell_command(command) + shell_result = run_shell_command( + command, + confirm_dangerous=self._shell_confirm, + timeout_seconds=self._shell_timeout_seconds, + ) return ModeResult( mode=mode, expanded_text=text, diff --git a/src/cleveragents/tui/input/reference_parser.py b/src/cleveragents/tui/input/reference_parser.py index af0075b02..779f32a73 100644 --- a/src/cleveragents/tui/input/reference_parser.py +++ b/src/cleveragents/tui/input/reference_parser.py @@ -2,12 +2,18 @@ from __future__ import annotations +import os +import time from dataclasses import dataclass from pathlib import Path from cleveragents.tui.search.fuzzy import rank_candidates _REFERENCE_TYPES = ("project", "plan", "resource", "actor", "tool", "skill") +_IGNORED_DIRS = {".git", ".venv", "node_modules", "__pycache__", ".mypy_cache"} +_CATALOG_LIMIT = 400 +_CATALOG_CACHE_TTL_SECONDS = 5.0 +_catalog_cache: dict[str, object] = {"cwd": None, "created_at": 0.0, "catalog": None} @dataclass(slots=True, frozen=True) @@ -30,17 +36,34 @@ class ReferenceParseResult: def _catalog() -> dict[str, list[str]]: cwd = Path.cwd() - files = [] - for path in cwd.rglob("*"): - if path.is_file(): - try: - files.append(path.relative_to(cwd).as_posix()) - except ValueError: - files.append(path.as_posix()) - if len(files) >= 400: - break + now = time.time() + cached_cwd = _catalog_cache.get("cwd") + cached_time = _catalog_cache.get("created_at") + cached_catalog = _catalog_cache.get("catalog") + if ( + isinstance(cached_cwd, Path) + and cached_cwd == cwd + and isinstance(cached_time, float) + and now - cached_time < _CATALOG_CACHE_TTL_SECONDS + and isinstance(cached_catalog, dict) + ): + return cached_catalog - return { + files = [] + for root, dirs, filenames in os.walk(cwd, followlinks=False): + dirs[:] = [name for name in dirs if name not in _IGNORED_DIRS] + root_path = Path(root) + for filename in filenames: + try: + files.append((root_path / filename).relative_to(cwd).as_posix()) + except ValueError: + files.append((root_path / filename).as_posix()) + if len(files) >= _CATALOG_LIMIT: + break + if len(files) >= _CATALOG_LIMIT: + break + + catalog = { "resource": sorted(files), "project": [value for value in [cwd.name] if value], "plan": [], @@ -63,6 +86,10 @@ def _catalog() -> dict[str, list[str]]: if (cwd / "examples" / "skills").is_dir() else [], } + _catalog_cache["cwd"] = cwd + _catalog_cache["created_at"] = now + _catalog_cache["catalog"] = catalog + return catalog def _resolve(category: str, query: str, items: dict[str, list[str]]) -> str | None: diff --git a/src/cleveragents/tui/input/shell_exec.py b/src/cleveragents/tui/input/shell_exec.py index 552b9b7db..015214695 100644 --- a/src/cleveragents/tui/input/shell_exec.py +++ b/src/cleveragents/tui/input/shell_exec.py @@ -2,7 +2,9 @@ from __future__ import annotations +import os import subprocess +from collections.abc import Callable from dataclasses import dataclass @@ -29,13 +31,56 @@ def looks_dangerous(command: str) -> bool: return any(pattern in lowered for pattern in patterns) -def run_shell_command(command: str) -> ShellResult: - """Execute shell command and capture outputs.""" +def run_shell_command( + command: str, + *, + confirm_dangerous: Callable[[str], bool] | None = None, + timeout_seconds: int = 30, +) -> ShellResult: + """Execute shell command with basic safeguards. + + Threat model: + - Shell mode is a convenience feature for local development, not a sandbox. + - We block obviously dangerous command patterns unless explicitly confirmed. + - We apply a timeout to avoid hanging the UI event loop indefinitely. + """ if not command.strip(): return ShellResult( command=command, exit_code=2, stdout="", stderr="empty command" ) - proc = subprocess.run(command, shell=True, text=True, capture_output=True) # nosec B602 + if os.environ.get("CLEVERAGENTS_DISABLE_SHELL_MODE", "").strip() in {"1", "true"}: + return ShellResult( + command=command, + exit_code=2, + stdout="", + stderr="shell mode is disabled", + ) + if looks_dangerous(command): + confirmed = False + if confirm_dangerous is not None: + confirmed = confirm_dangerous(command) + if not confirmed: + return ShellResult( + command=command, + exit_code=1, + stdout="", + stderr="blocked dangerous shell command", + ) + try: + proc = subprocess.run( + command, + shell=True, + text=True, + capture_output=True, + timeout=timeout_seconds, + ) # nosec B602 + except subprocess.TimeoutExpired: + return ShellResult( + command=command, + exit_code=124, + stdout="", + stderr=f"command timed out after {timeout_seconds}s", + ) return ShellResult( command=command, exit_code=proc.returncode, diff --git a/src/cleveragents/tui/persona/registry.py b/src/cleveragents/tui/persona/registry.py index 6e94fa1cb..da7784847 100644 --- a/src/cleveragents/tui/persona/registry.py +++ b/src/cleveragents/tui/persona/registry.py @@ -2,15 +2,21 @@ from __future__ import annotations +import fcntl +import logging import os +import tempfile from dataclasses import dataclass, field from pathlib import Path from typing import Any import yaml +from pydantic import ValidationError from cleveragents.tui.persona.schema import Persona +_logger = logging.getLogger(__name__) + def _default_config_dir() -> Path: override = os.environ.get("CLEVERAGENTS_CONFIG_DIR", "").strip() @@ -33,20 +39,82 @@ class PersonaRegistry: def state_path(self) -> Path: return self.config_dir / "tui-state.yaml" + @property + def personas_lock_path(self) -> Path: + return self.config_dir / ".personas.lock" + + @property + def state_lock_path(self) -> Path: + return self.config_dir / ".state.lock" + def ensure_dirs(self) -> None: self.personas_dir.mkdir(parents=True, exist_ok=True) + def _lock_file(self, lock_path: Path) -> Any: + lock_path.parent.mkdir(parents=True, exist_ok=True) + handle = lock_path.open("a+", encoding="utf-8") + fcntl.flock(handle.fileno(), fcntl.LOCK_EX) + return handle + + @staticmethod + def _atomic_write_yaml(path: Path, payload: dict[str, Any]) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + with tempfile.NamedTemporaryFile( + mode="w", + encoding="utf-8", + dir=str(path.parent), + prefix=f".{path.name}.", + suffix=".tmp", + delete=False, + ) as handle: + yaml.safe_dump(payload, handle, sort_keys=False, default_flow_style=False) + temp_name = handle.name + Path(temp_name).replace(path) + + def persona_path(self, name: str) -> Path: + Persona.validate_name_safe(name) + result = (self.personas_dir / f"{name}.yaml").resolve() + if not result.is_relative_to(self.personas_dir.resolve()): + raise ValueError(f"Invalid persona name: {name}") + return result + + def resolve_export_path(self, output_path: Path) -> Path: + if output_path.is_absolute(): + raise ValueError( + "Export path must be relative to current working directory" + ) + base = Path.cwd().resolve() + resolved = (base / output_path).resolve() + if not resolved.is_relative_to(base): + raise ValueError("Export path must stay within working directory") + return resolved + + def resolve_import_path(self, input_path: Path) -> Path: + if input_path.is_absolute(): + raise ValueError( + "Import path must be relative to current working directory" + ) + base = Path.cwd().resolve() + resolved = (base / input_path).resolve() + if not resolved.is_relative_to(base): + raise ValueError("Import path must stay within working directory") + return resolved + def list_personas(self) -> list[Persona]: self.ensure_dirs() personas: list[Persona] = [] for file in sorted(self.personas_dir.glob("*.y*ml")): - raw = yaml.safe_load(file.read_text(encoding="utf-8")) or {} - if isinstance(raw, dict): + try: + raw = yaml.safe_load(file.read_text(encoding="utf-8")) or {} + if not isinstance(raw, dict): + continue personas.append(Persona.model_validate(raw)) + except (yaml.YAMLError, ValidationError) as exc: + _logger.warning("Skipping invalid persona file %s: %s", file, exc) return personas def get(self, name: str) -> Persona | None: - file = self.personas_dir / f"{name}.yaml" + file = self.persona_path(name) if not file.exists(): return None raw = yaml.safe_load(file.read_text(encoding="utf-8")) or {} @@ -65,17 +133,19 @@ class PersonaRegistry: def save(self, persona: Persona) -> Path: self.ensure_dirs() - self._validate_cycle_order(persona) - file = self.personas_dir / f"{persona.name}.yaml" - payload = persona.model_dump(mode="json", exclude_none=True) - file.write_text( - yaml.safe_dump(payload, sort_keys=False, default_flow_style=False), - encoding="utf-8", - ) - return file + lock = self._lock_file(self.personas_lock_path) + try: + self._validate_cycle_order(persona) + file = self.persona_path(persona.name) + payload = persona.model_dump(mode="json", exclude_none=True) + self._atomic_write_yaml(file, payload) + return file + finally: + fcntl.flock(lock.fileno(), fcntl.LOCK_UN) + lock.close() def delete(self, name: str) -> bool: - file = self.personas_dir / f"{name}.yaml" + file = self.persona_path(name) if not file.exists(): return False file.unlink() @@ -86,14 +156,13 @@ class PersonaRegistry: if persona is None: raise ValueError(f"Persona not found: {name}") payload = persona.model_dump(mode="json", exclude_none=True) - output_path.write_text( - yaml.safe_dump(payload, sort_keys=False, default_flow_style=False), - encoding="utf-8", - ) - return output_path + safe_output = self.resolve_export_path(output_path) + self._atomic_write_yaml(safe_output, payload) + return safe_output def import_persona(self, input_path: Path) -> Persona: - raw = yaml.safe_load(input_path.read_text(encoding="utf-8")) or {} + safe_input = self.resolve_import_path(input_path) + raw = yaml.safe_load(safe_input.read_text(encoding="utf-8")) or {} if not isinstance(raw, dict): raise ValueError("Invalid persona file") persona = Persona.model_validate(raw) @@ -110,10 +179,7 @@ class PersonaRegistry: def save_state(self, state: dict[str, Any]) -> None: self.ensure_dirs() - self.state_path.write_text( - yaml.safe_dump(state, sort_keys=False, default_flow_style=False), - encoding="utf-8", - ) + self._atomic_write_yaml(self.state_path, state) def get_last_persona(self) -> str | None: state = self.load_state() @@ -121,9 +187,14 @@ class PersonaRegistry: return value if isinstance(value, str) and value else None def set_last_persona(self, name: str) -> None: - state = self.load_state() - state["last_persona"] = name - self.save_state(state) + lock = self._lock_file(self.state_lock_path) + try: + state = self.load_state() + state["last_persona"] = name + self.save_state(state) + finally: + fcntl.flock(lock.fileno(), fcntl.LOCK_UN) + lock.close() def ensure_default(self, actor: str = "local/mock-default") -> Persona: existing = self.get("default") diff --git a/src/cleveragents/tui/persona/schema.py b/src/cleveragents/tui/persona/schema.py index 1692dc49f..0f9553d0f 100644 --- a/src/cleveragents/tui/persona/schema.py +++ b/src/cleveragents/tui/persona/schema.py @@ -39,19 +39,18 @@ class Persona(BaseModel): raise ValueError("actor must be namespaced (namespace/name)") return value - @field_validator("argument_presets") + @field_validator("name") @classmethod - def validate_default_preset( - cls, presets: list[PersonaPreset] - ) -> list[PersonaPreset]: - if not presets: - return presets - defaults = [preset for preset in presets if preset.name == "default"] - if len(defaults) != 1: - raise ValueError("must contain exactly one default preset") - if defaults[0].overrides: - raise ValueError("default preset overrides must be empty") - return presets + def validate_name_safe(cls, value: str) -> str: + if ( + "/" in value + or "\\" in value + or ".." in value + or "\x00" in value + or any(ord(char) < 32 or ord(char) == 127 for char in value) + ): + raise ValueError("name must not contain path/control separators") + return value @model_validator(mode="after") def ensure_default_preset(self) -> Persona: