From dc31fbcc9df17111f89a60ee501785e9a9c7f4f1 Mon Sep 17 00:00:00 2001 From: Altynbek Orumbayev Date: Wed, 6 May 2026 09:23:51 +0200 Subject: [PATCH 1/8] refactor(core): P1+P2-A/B dead code, stream readers, process termination - Delete dead private wrappers in _acp.py (_acp_startup_timeout_seconds, _friendly_startup_error_message, _acp_process_exit_hint) - Delete legacy dict API (BackendSpec.to_legacy_config, guidance_hints, get_backend) - Remove get_backend from __init__.py exports - Update callers in cli/tools.py and _environment_checks.py to use typed API - Fix _ByteCountingStreamReader: remove asyncio.StreamReader inheritance, remove __getattr__ catch-all, use explicit forward methods - Fix JsonRpcObjectStreamReader: remove asyncio.StreamReader inheritance - Consolidate duplicate process termination into _subprocess.terminate_process() - Update tests to remove legacy API coverage --- src/kagan/cli/chat/acp.py | 3 +- src/kagan/cli/tools.py | 5 +- src/kagan/core/__init__.py | 2 - src/kagan/core/_acp.py | 38 ++------------- src/kagan/core/_acp_spawn.py | 32 ++----------- src/kagan/core/_acp_streams.py | 11 ++--- src/kagan/core/_agent.py | 55 +++------------------- src/kagan/core/_environment_checks.py | 8 +++- src/kagan/core/_subprocess.py | 29 +++++++++++- tests/core/unit/test_backend_spec.py | 52 -------------------- tests/unit/test_acp_timeout_config.py | 26 ++++------ tests/unit/test_agent_registry.py | 12 ----- tests/unit/test_agent_spawn_acp.py | 1 - tests/unit/test_pi_coding_agent_backend.py | 7 --- 14 files changed, 67 insertions(+), 214 deletions(-) diff --git a/src/kagan/cli/chat/acp.py b/src/kagan/cli/chat/acp.py index 7f7dd70f..a2fbac4e 100644 --- a/src/kagan/cli/chat/acp.py +++ b/src/kagan/cli/chat/acp.py @@ -62,7 +62,6 @@ def get_lock(self, backend: str) -> asyncio.Lock: _acp_handshake_timeout_seconds = acp_handshake_timeout_seconds -_acp_process_exit_hint = acp_process_exit_hint _friendly_acp_error_message = friendly_acp_error_message @@ -252,7 +251,7 @@ async def _acp_process_exit_message(agent_backend: str, process: Any, *, during: if details: compact = " ".join(line.strip() for line in details.splitlines() if line.strip()) message = f"{message} {compact[:500]}" - hint = _acp_process_exit_hint(agent_backend=agent_backend, details=details) + hint = acp_process_exit_hint(agent_backend=agent_backend, details=details) if hint: message = f"{message} {hint}" return message diff --git a/src/kagan/cli/tools.py b/src/kagan/cli/tools.py index 07cb956d..66dba347 100644 --- a/src/kagan/cli/tools.py +++ b/src/kagan/cli/tools.py @@ -186,10 +186,9 @@ def _resolve_backend(agent_backend: str | None, tool: str | None) -> str: def _get_backend_executable(backend_name: str) -> str: - from kagan.core import get_backend + from kagan.core import get_backend_spec - backend = get_backend(backend_name) - executable = backend.get("executable") + executable = get_backend_spec(backend_name).executable if not isinstance(executable, str) or not executable: raise click.ClickException(f"Invalid backend configuration for: {backend_name}") return executable diff --git a/src/kagan/core/__init__.py b/src/kagan/core/__init__.py index d2a0059c..8e6462a6 100644 --- a/src/kagan/core/__init__.py +++ b/src/kagan/core/__init__.py @@ -32,7 +32,6 @@ BackendSpec, build_agent_environment, build_mcp_manifest, - get_backend, get_backend_spec, list_available_backends, list_backend_specs, @@ -345,7 +344,6 @@ "format_duration", "format_percentage", "friendly_acp_error_message", - "get_backend", "get_backend_spec", "get_conflicts", "get_latest_session", diff --git a/src/kagan/core/_acp.py b/src/kagan/core/_acp.py index 2c02c010..b64691ca 100644 --- a/src/kagan/core/_acp.py +++ b/src/kagan/core/_acp.py @@ -51,6 +51,7 @@ OPENCODE_BACKEND, get_backend_spec, ) +from kagan.core._subprocess import terminate_process from kagan.core.errors import AgentError _ACP_STARTUP_TIMEOUT_ENV_KEY = "KAGAN_ACP_STARTUP_TIMEOUT_SECONDS" @@ -118,10 +119,6 @@ def acp_startup_timeout_seconds(agent_backend: str) -> float: return _default_acp_timeout_seconds(agent_backend) -def _acp_startup_timeout_seconds(agent_backend: str) -> float: - return acp_startup_timeout_seconds(agent_backend) - - def _infer_backend_name_from_process(process: asyncio.subprocess.Process) -> str: args = [str(part).lower() for part in (getattr(process, "args", []) or [])] joined = " ".join(args) @@ -192,14 +189,6 @@ def friendly_acp_error_message(*, error: object, agent_backend: str, during: str return f"{prefix} {raw}" -def _friendly_startup_error_message(*, error: object, agent_backend: str, during: str) -> str: - return friendly_acp_error_message( - error=error, - agent_backend=agent_backend, - during=during, - ) - - class KaganACPClient(acp.Client): """ACP client implementation forwarding session updates to a callback.""" @@ -437,10 +426,6 @@ def acp_process_exit_hint(*, agent_backend: str, details: str) -> str | None: return None -def _acp_process_exit_hint(*, agent_backend: str, details: str) -> str | None: - return acp_process_exit_hint(agent_backend=agent_backend, details=details) - - async def _acp_process_exit_message( process: asyncio.subprocess.Process, *, during: str, agent_backend: str ) -> str | None: @@ -462,19 +447,6 @@ async def _acp_process_exit_message( return message -async def _terminate_acp_process(process: asyncio.subprocess.Process) -> None: - if process.returncode is not None: - return - with contextlib.suppress(ProcessLookupError): - process.terminate() - try: - await asyncio.wait_for(process.wait(), timeout=5.0) - except TimeoutError: - with contextlib.suppress(ProcessLookupError): - process.kill() - await process.wait() - - async def run_acp_session( process: asyncio.subprocess.Process, client: KaganACPClient, @@ -492,7 +464,7 @@ async def run_acp_session( stdout = JsonRpcObjectStreamReader(process.stdout, backend_name=resolved_backend) conn = acp.connect_to_agent(client, process.stdin, stdout) - timeout_s = _acp_startup_timeout_seconds(resolved_backend) + timeout_s = acp_startup_timeout_seconds(resolved_backend) try: try: await asyncio.wait_for( @@ -534,13 +506,13 @@ async def run_acp_session( ) raise RuntimeError(timeout_message) from exc await conn.prompt(session_id=session.session_id, prompt=[acp.text_block(prompt)]) - await _terminate_acp_process(process) + await terminate_process(process) logger.info( "ACP one-shot prompt completed for pid={} rc={}", process.pid, process.returncode ) except (RequestError, OSError, RuntimeError, ValueError, AttributeError) as exc: logger.exception("ACP session failed for pid={} cwd={}", process.pid, worktree_path) - await _terminate_acp_process(process) + await terminate_process(process) raise RuntimeError( friendly_acp_error_message( error=exc, @@ -553,4 +525,4 @@ async def run_acp_session( with contextlib.suppress(RequestError, OSError, RuntimeError): await conn.close() finally: - await _terminate_acp_process(process) + await terminate_process(process) diff --git a/src/kagan/core/_acp_spawn.py b/src/kagan/core/_acp_spawn.py index d9f6433f..2deed3a6 100644 --- a/src/kagan/core/_acp_spawn.py +++ b/src/kagan/core/_acp_spawn.py @@ -2,46 +2,22 @@ from __future__ import annotations -import asyncio -import contextlib from contextlib import asynccontextmanager -from typing import TYPE_CHECKING, Any, cast +from typing import TYPE_CHECKING, Any from acp.client.connection import ClientSideConnection from acp.transports import spawn_stdio_transport from kagan.core._acp_streams import JsonRpcObjectStreamReader +from kagan.core._subprocess import terminate_process if TYPE_CHECKING: - from collections.abc import AsyncIterator, Awaitable, Callable, Mapping + from collections.abc import AsyncIterator, Callable, Mapping from pathlib import Path import acp -async def _terminate_stdio_process(process: Any) -> None: - if getattr(process, "returncode", None) is not None: - return - terminate = getattr(process, "terminate", None) - if callable(terminate): - with contextlib.suppress(ProcessLookupError): - terminate() - wait = getattr(process, "wait", None) - if not callable(wait): - return - try: - wait_result = cast("Awaitable[Any]", wait()) - await asyncio.wait_for(wait_result, timeout=5.0) - except TimeoutError: - kill = getattr(process, "kill", None) - if callable(kill): - with contextlib.suppress(ProcessLookupError): - kill() - wait_result = cast("Awaitable[Any]", wait()) - with contextlib.suppress(TimeoutError): - await asyncio.wait_for(wait_result, timeout=5.0) - - @asynccontextmanager async def spawn_filtered_agent_process( to_client: Callable[[acp.Agent], acp.Client] | acp.Client, @@ -69,4 +45,4 @@ async def spawn_filtered_agent_process( try: await conn.close() finally: - await _terminate_stdio_process(process) + await terminate_process(process) diff --git a/src/kagan/core/_acp_streams.py b/src/kagan/core/_acp_streams.py index 1801a480..9f109869 100644 --- a/src/kagan/core/_acp_streams.py +++ b/src/kagan/core/_acp_streams.py @@ -2,14 +2,16 @@ from __future__ import annotations -import asyncio import json -from typing import Any +from typing import TYPE_CHECKING, Any from loguru import logger +if TYPE_CHECKING: + import asyncio -class JsonRpcObjectStreamReader(asyncio.StreamReader): + +class JsonRpcObjectStreamReader: """Drop non-JSON-RPC stdout lines before the ACP SDK parses them. ACP transports are line-delimited JSON-RPC. Some Windows agent launchers can @@ -48,9 +50,6 @@ async def readline(self) -> bytes: def at_eof(self) -> bool: return self._reader.at_eof() - def __getattr__(self, name: str) -> Any: - return getattr(self._reader, name) - def _record_drop(self, line: bytes, *, reason: str) -> None: self._dropped += 1 if self._dropped > 1: diff --git a/src/kagan/core/_agent.py b/src/kagan/core/_agent.py index a040ac12..4a5a6913 100644 --- a/src/kagan/core/_agent.py +++ b/src/kagan/core/_agent.py @@ -179,19 +179,6 @@ class BackendSpec: install: Mapping[OS, BackendCommand] | None = None auth: Mapping[OS, BackendCommand] | None = None - def to_legacy_config(self) -> dict[str, Any]: - """Project the typed spec into a dict mapping (legacy compat).""" - return { - "capabilities": tuple(sorted(cap.value for cap in self.capabilities)), - "executable": self.executable, - "prompt_flag": self.prompt_flag, - "workdir_flag": self.workdir_flag, - "env_vars": dict(self.env_vars), - "supports_acp": self.has_capability(BackendCapability.ACP_STREAMING), - "acp_command": list(self.acp_command), - "acp_args": list(self.acp_args), - } - def has_capability(self, capability: BackendCapability) -> bool: """Return whether the backend declares *capability*.""" return capability in self.capabilities @@ -229,21 +216,6 @@ def resolve_command( return exact return mapping.get("*") - def guidance_hints(self) -> tuple[str, ...]: - """Return explicit setup hint strings for this backend (legacy shim). - - Derives human-readable hints from the structured ``install`` / ``auth`` - mappings so that existing callers continue to work during the transition. - Each hint is formatted as ``"{description} {command}"`` so callers that - previously searched hint text for the command string continue to work. - """ - hints: list[str] = [] - for action in ("install", "auth"): - # type: ignore[arg-type] — looping over known-valid Literal strings. - cmd = self.resolve_command(action) # type: ignore[arg-type] - if cmd is not None: - hints.append(f"{cmd.description} {cmd.command}") - return tuple(hints) CLAUDE_CODE_BACKEND: Final = "claude-code" @@ -681,11 +653,6 @@ def list_backend_specs() -> dict[str, BackendSpec]: return dict(_BACKEND_SPECS) -def get_backend(name: str) -> dict[str, Any]: - """Return the legacy config dict for *name*, raising AgentError if unknown.""" - return get_backend_spec(name).to_legacy_config() - - def list_backends() -> list[str]: """Return all registered backend names.""" return list(_BACKEND_SPECS) @@ -899,17 +866,13 @@ async def spawn_agent( _MAX_CUMULATIVE_BYTES: Final[int] = 500 * 1024 * 1024 # 500 MB total per session -class _ByteCountingStreamReader(asyncio.StreamReader): - """StreamReader subclass that enforces a cumulative byte cap. +class _ByteCountingStreamReader: + """Composition wrapper that enforces a cumulative byte cap on reads. - The ACP JSON-RPC read loop (inside the ``acp`` library) calls ``readline()`` - or ``read()`` on the underlying reader. This subclass wraps another reader, - counts every byte returned, and terminates the associated process when the - cumulative limit is exceeded, preventing unbounded memory growth. - - Inherits from ``asyncio.StreamReader`` so ``isinstance()`` checks pass when - the ACP SDK validates stream types in ``ClientSideConnection.__init__``. - Delegates all reads to the wrapped reader rather than using inherited state. + The ACP JSON-RPC read loop calls ``readline()`` or ``read()`` on the + underlying reader. This wrapper counts every byte returned and terminates + the associated process when the cumulative limit is exceeded, preventing + unbounded memory growth. """ def __init__( @@ -918,9 +881,6 @@ def __init__( process: asyncio.subprocess.Process, cumulative_limit: int = _MAX_CUMULATIVE_BYTES, ) -> None: - # Note: We skip calling super().__init__() because we delegate all - # operations to self._reader. The inheritance is only to pass - # isinstance() checks in the ACP SDK. self._reader = reader self._process = process self._cumulative_bytes = 0 @@ -964,9 +924,6 @@ async def readexactly(self, n: int) -> bytes: def at_eof(self) -> bool: return self._reader.at_eof() - def __getattr__(self, name: str) -> Any: - return getattr(self._reader, name) - async def spawn_agent_via_acp( backend_name: str, diff --git a/src/kagan/core/_environment_checks.py b/src/kagan/core/_environment_checks.py index 381edad8..999bceea 100644 --- a/src/kagan/core/_environment_checks.py +++ b/src/kagan/core/_environment_checks.py @@ -296,7 +296,13 @@ def resolve_backend_guidance(backend_name: str) -> str | None: from kagan.core.errors import KaganError try: - guidance = get_backend_spec(backend_name).guidance_hints() + spec = get_backend_spec(backend_name) + hints: list[str] = [] + for action in ("install", "auth"): + cmd = spec.resolve_command(action) + if cmd is not None: + hints.append(f"{cmd.description} {cmd.command}") + guidance = tuple(hints) except (ImportError, KaganError): return None return " ".join(guidance) if guidance else None diff --git a/src/kagan/core/_subprocess.py b/src/kagan/core/_subprocess.py index b14f3433..12c5df80 100644 --- a/src/kagan/core/_subprocess.py +++ b/src/kagan/core/_subprocess.py @@ -11,11 +11,36 @@ so every spawn site works correctly on Windows while being a no-op on POSIX. """ +import asyncio +import contextlib import shutil import sys from pathlib import Path - -__all__ = ["resolve_spawn_command"] +from typing import Any + +__all__ = ["resolve_spawn_command", "terminate_process"] + + +async def terminate_process(proc: Any, timeout: float = 5.0) -> None: + """Terminate *proc* gracefully, then kill after *timeout* seconds.""" + if getattr(proc, "returncode", None) is not None: + return + terminate = getattr(proc, "terminate", None) + if callable(terminate): + with contextlib.suppress(ProcessLookupError): + terminate() + wait = getattr(proc, "wait", None) + if not callable(wait): + return + try: + await asyncio.wait_for(wait(), timeout=timeout) + except TimeoutError: + kill = getattr(proc, "kill", None) + if callable(kill): + with contextlib.suppress(ProcessLookupError): + kill() + with contextlib.suppress(TimeoutError): + await asyncio.wait_for(wait(), timeout=timeout) def resolve_spawn_command(executable: str, *args: str) -> list[str]: diff --git a/tests/core/unit/test_backend_spec.py b/tests/core/unit/test_backend_spec.py index eab864ca..dea19663 100644 --- a/tests/core/unit/test_backend_spec.py +++ b/tests/core/unit/test_backend_spec.py @@ -124,56 +124,4 @@ def test_resolve_command_missing_platform_no_wildcard_returns_none() -> None: assert result is None -# --------------------------------------------------------------------------- -# BackendSpec.guidance_hints — legacy shim -# --------------------------------------------------------------------------- - - -def test_guidance_hints_derives_from_install_and_auth() -> None: - spec = _make_spec( - install={"*": BackendCommand(description="Install hint text", command="cmd-install")}, - auth={"*": BackendCommand(description="Auth hint text", command="cmd-auth")}, - ) - hints = spec.guidance_hints() - assert len(hints) == 2 - # Each hint contains both the description and the command. - assert any("Install hint text" in h and "cmd-install" in h for h in hints) - assert any("Auth hint text" in h and "cmd-auth" in h for h in hints) - - -def test_guidance_hints_empty_when_no_mappings() -> None: - spec = _make_spec(install=None, auth=None) - assert spec.guidance_hints() == () - -def test_guidance_hints_partial_mappings() -> None: - spec = _make_spec( - install={"*": BackendCommand(description="Just install", command="cmd")}, - auth=None, - ) - hints = spec.guidance_hints() - assert len(hints) == 1 - assert "Just install" in hints[0] - assert "cmd" in hints[0] - - -# --------------------------------------------------------------------------- -# to_legacy_config round-trip — BackendCommand does not appear in legacy config -# --------------------------------------------------------------------------- - - -def test_to_legacy_config_excludes_install_auth() -> None: - """The legacy config dict must not contain 'install' or 'auth' keys.""" - spec = BackendSpec( - name="test", - executable="test-exe", - capabilities=frozenset({BackendCapability.ACP_STREAMING}), - install={"*": BackendCommand(description="d", command="c")}, - auth={"*": BackendCommand(description="d2", command="c2")}, - ) - config = spec.to_legacy_config() - assert "install" not in config - assert "auth" not in config - assert "install_hint" not in config - assert "auth_hint" not in config - assert config["supports_acp"] is True diff --git a/tests/unit/test_acp_timeout_config.py b/tests/unit/test_acp_timeout_config.py index 4356aaac..212f552b 100644 --- a/tests/unit/test_acp_timeout_config.py +++ b/tests/unit/test_acp_timeout_config.py @@ -1,10 +1,8 @@ import pytest from kagan.cli.chat.acp import _acp_handshake_timeout_seconds -from kagan.cli.chat.acp import _acp_process_exit_hint as _chat_exit_hint from kagan.core import CLAUDE_CODE_BACKEND, CODEX_BACKEND -from kagan.core._acp import _acp_process_exit_hint as _core_exit_hint -from kagan.core._acp import _acp_startup_timeout_seconds, friendly_acp_error_message +from kagan.core._acp import acp_process_exit_hint, acp_startup_timeout_seconds, friendly_acp_error_message pytestmark = [pytest.mark.unit] @@ -14,9 +12,9 @@ def test_acp_timeout_defaults_are_backend_aware() -> None: assert _acp_handshake_timeout_seconds("codex") == 45.0 assert _acp_handshake_timeout_seconds("gemini-cli") == 20.0 - assert _acp_startup_timeout_seconds("claude-code") == 12.0 - assert _acp_startup_timeout_seconds("codex") == 45.0 - assert _acp_startup_timeout_seconds("gemini-cli") == 20.0 + assert acp_startup_timeout_seconds("claude-code") == 12.0 + assert acp_startup_timeout_seconds("codex") == 45.0 + assert acp_startup_timeout_seconds("gemini-cli") == 20.0 def test_chat_timeout_reads_both_env_keys(monkeypatch: pytest.MonkeyPatch) -> None: @@ -31,26 +29,22 @@ def test_chat_timeout_reads_both_env_keys(monkeypatch: pytest.MonkeyPatch) -> No def test_core_timeout_reads_both_env_keys(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.delenv("KAGAN_ACP_STARTUP_TIMEOUT_SECONDS", raising=False) monkeypatch.setenv("KAGAN_ACP_HANDSHAKE_TIMEOUT_SECONDS", "31") - assert _acp_startup_timeout_seconds("gemini-cli") == 31.0 + assert acp_startup_timeout_seconds("gemini-cli") == 31.0 monkeypatch.setenv("KAGAN_ACP_STARTUP_TIMEOUT_SECONDS", "29") - assert _acp_startup_timeout_seconds("gemini-cli") == 29.0 + assert acp_startup_timeout_seconds("gemini-cli") == 29.0 def test_acp_exit_hints_include_codex_eacces_recovery() -> None: details = "spawnSync ... codex-acp EACCES permission denied" - chat_hint = _chat_exit_hint(agent_backend=CODEX_BACKEND, details=details) - core_hint = _core_exit_hint(agent_backend=CODEX_BACKEND, details=details) - assert chat_hint is not None and "npm npx cache permission issue" in chat_hint - assert core_hint is not None and "npm npx cache permission issue" in core_hint + hint = acp_process_exit_hint(agent_backend=CODEX_BACKEND, details=details) + assert hint is not None and "npm npx cache permission issue" in hint def test_acp_exit_hints_cover_generic_permission_denied() -> None: details = "permission denied while executing backend" - chat_hint = _chat_exit_hint(agent_backend="gemini-cli", details=details) - core_hint = _core_exit_hint(agent_backend="gemini-cli", details=details) - assert isinstance(chat_hint, str) and "permission" in chat_hint.lower() - assert isinstance(core_hint, str) and "permission" in core_hint.lower() + hint = acp_process_exit_hint(agent_backend="gemini-cli", details=details) + assert isinstance(hint, str) and "permission" in hint.lower() @pytest.mark.parametrize( diff --git a/tests/unit/test_agent_registry.py b/tests/unit/test_agent_registry.py index af85fb11..b4d4ca95 100644 --- a/tests/unit/test_agent_registry.py +++ b/tests/unit/test_agent_registry.py @@ -11,7 +11,6 @@ REFERENCE_BACKENDS, AgentError, BackendCapability, - get_backend, get_backend_spec, list_backend_specs, ) @@ -28,17 +27,6 @@ def test_each_backend_has_required_keys_and_nonempty_executable() -> None: assert spec.capabilities, f"{name}: capabilities must not be empty" -def test_get_backend_raises_for_unknown_name() -> None: - with pytest.raises(AgentError, match="unknown agent backend"): - get_backend("nonexistent-backend") - - -def test_get_backend_accepts_legacy_aliases() -> None: - assert get_backend("kimi")["executable"] == get_backend_spec("kimi-cli").executable - assert get_backend("gemini")["executable"] == get_backend_spec("gemini-cli").executable - assert get_backend("claude")["executable"] == get_backend_spec("claude-code").executable - - def test_backend_specs_expose_reference_backends() -> None: assert REFERENCE_BACKENDS == ("claude-code", "codex") diff --git a/tests/unit/test_agent_spawn_acp.py b/tests/unit/test_agent_spawn_acp.py index 80984831..e8de9f9f 100644 --- a/tests/unit/test_agent_spawn_acp.py +++ b/tests/unit/test_agent_spawn_acp.py @@ -144,7 +144,6 @@ async def test_spawn_agent_acp_uses_typed_capability_over_legacy_supports_flag( } monkeypatch.setattr("kagan.core._agent.get_backend_spec", lambda _name: spec) - monkeypatch.setattr("kagan.core._agent.get_backend", lambda _name: legacy_entry) # resolve_spawn_command uses shutil.which from _subprocess, not _agent. monkeypatch.setattr("kagan.core._subprocess.shutil.which", lambda exe: exe) diff --git a/tests/unit/test_pi_coding_agent_backend.py b/tests/unit/test_pi_coding_agent_backend.py index ef50764b..9496fbe7 100644 --- a/tests/unit/test_pi_coding_agent_backend.py +++ b/tests/unit/test_pi_coding_agent_backend.py @@ -85,13 +85,6 @@ def test_pi_coding_agent_install_hint_present() -> None: assert "pi-coding-agent" in cmd.command or "mariozechner" in cmd.command -def test_pi_coding_agent_legacy_config_does_not_claim_acp() -> None: - """to_legacy_config() must reflect supports_acp=False.""" - spec = get_backend_spec(PI_CODING_AGENT_BACKEND) - cfg = spec.to_legacy_config() - assert cfg["supports_acp"] is False - - # --------------------------------------------------------------------------- # Environment gate # --------------------------------------------------------------------------- From fc484b6771ec524fb2aa283e1375f9940119f293 Mon Sep 17 00:00:00 2001 From: Altynbek Orumbayev Date: Wed, 6 May 2026 09:40:21 +0200 Subject: [PATCH 2/8] refactor(core): P2-C/D split Sessions.run and async-ify DB helpers - Extract _run_detached() and _run_attached() from run() - Compute shared setup (db_path_str, backend_spec) once before branching - Convert _update_session_pid, _mark_session_running, _complete_session, _fail_session from sync to async using _db_async directly - Update all callers to use await self._method() instead of await asyncio.to_thread(self._method, ...) - Keep _set_status as externally-injected sync callable --- src/kagan/core/_sessions.py | 226 +++++++++++++++----------- src/kagan/tui/screens/kanban.py | 92 ++++++----- src/kagan/tui/widgets/chat.py | 147 +++++++++-------- tests/tui/test_post_audit_cleanups.py | 8 +- 4 files changed, 264 insertions(+), 209 deletions(-) diff --git a/src/kagan/core/_sessions.py b/src/kagan/core/_sessions.py index 836d840f..aed158a8 100644 --- a/src/kagan/core/_sessions.py +++ b/src/kagan/core/_sessions.py @@ -13,6 +13,7 @@ from kagan.core._agent import ( BackendCapability, + BackendSpec, get_backend_spec, resolve_default_agent_backend, spawn_agent, @@ -30,7 +31,6 @@ from kagan.core._db import default_db_path from kagan.core._db_helpers import ( _db_async, - _db_sync, _sa_col, _setting_enabled, _utc_now, @@ -316,7 +316,7 @@ async def active_session_summaries(self, task_ids: list[str]) -> dict[str, Sessi # -- Sync DB helper delegates ------------------------------------------- - def _update_session_pid(self, session_id: str, pid: int) -> None: + async def _update_session_pid(self, session_id: str, pid: int) -> None: def op(s): obj = s.get(Session, session_id) if obj: @@ -324,18 +324,18 @@ def op(s): obj.status = SessionStatus.RUNNING s.add(obj) - _db_sync(self._engine, op, commit=True) + await _db_async(self._engine, op, commit=True) - def _mark_session_running(self, session_id: str) -> None: + async def _mark_session_running(self, session_id: str) -> None: def op(s): obj = s.get(Session, session_id) if obj and obj.status == SessionStatus.PENDING: obj.status = SessionStatus.RUNNING s.add(obj) - _db_sync(self._engine, op, commit=True) + await _db_async(self._engine, op, commit=True) - def _complete_session(self, session_id: str) -> None: + async def _complete_session(self, session_id: str) -> None: def op(s): obj = s.get(Session, session_id) if obj and obj.status in {SessionStatus.PENDING, SessionStatus.RUNNING}: @@ -361,9 +361,9 @@ def op(s): s.add(obj) - _db_sync(self._engine, op, commit=True) + await _db_async(self._engine, op, commit=True) - def _fail_session(self, session_id: str) -> None: + async def _fail_session(self, session_id: str) -> None: def op(s): obj = s.get(Session, session_id) if obj and obj.status in {SessionStatus.PENDING, SessionStatus.RUNNING}: @@ -371,7 +371,7 @@ def op(s): obj.ended_at = _utc_now() s.add(obj) - _db_sync(self._engine, op, commit=True) + await _db_async(self._engine, op, commit=True) # -- Orchestration methods (kept on class) ------------------------------ @@ -450,7 +450,7 @@ async def run( launcher: str | None = None, ide: str | None = None, persona: str | None = None, - ): + ) -> Session: task, ws, session_obj = await self._prepare_session( task_id, agent_backend=agent_backend, @@ -458,93 +458,122 @@ async def run( launcher=launcher, ) + db_path_str = str(self._db_path or default_db_path()) + backend_spec = get_backend_spec(agent_backend) + if launcher is None: - settings_dict = await _db_async( - self._engine, - lambda s: {row.key: row.value for row in s.exec(select(Setting)).all()}, + return await self._run_detached( + task, ws, session_obj, task_id, agent_backend, persona, db_path_str, backend_spec ) + return await self._run_attached( + task, ws, session_obj, task_id, agent_backend, launcher, ide, db_path_str, backend_spec + ) - project_path = Path(ws.worktree_path) - if task.status is TaskStatus.REVIEW: - prompt = resolve_review_prompt(task_id, settings_dict, project_path) - else: - learnings = await self._fetch_project_learnings(task.project_id) - task_criteria_texts = await _db_async( - self._engine, - lambda s: [ - c.text - for c in s.exec( - select(AcceptanceCriterion).where( - AcceptanceCriterion.task_id == task_id - ) - ).all() - ], - ) - prompt = resolve_task_prompt( - task, - settings_dict, - project_path, - learnings=learnings, - criteria_texts=task_criteria_texts, - ) + async def _run_detached( + self, + task: Task, + ws: Worktree, + session_obj: Session, + task_id: str, + agent_backend: str, + persona: str | None, + db_path_str: str, + backend_spec: BackendSpec, + ) -> Session: + settings_dict = await _db_async( + self._engine, + lambda s: {row.key: row.value for row in s.exec(select(Setting)).all()}, + ) - persona_prompt: str | None = None - if persona: - persona_prompt = get_persona_prompt(persona, settings_dict) - if persona_prompt and persona_prompt.strip(): - prompt = f"{build_persona_section(persona_prompt)}\n\n{prompt}" - db_path_str = str(self._db_path or default_db_path()) - backend_spec = get_backend_spec(agent_backend) - - if backend_spec.has_capability(BackendCapability.ACP_STREAMING): - # Register the completion handler as an AgentEnd subscriber so - # that wait_idle() can synchronise callers waiting for the - # session to finish processing (settlement rule). - self._events.register_agent_end_subscriber(session_obj.id) - pid, reader_task = await spawn_agent_via_acp( - agent_backend, - Path(ws.worktree_path), - prompt, - session_id=session_obj.id, - task_id=task_id, - db_path=db_path_str, - project_id=task.project_id, - on_session_update=self._make_acp_callback(task_id, session_obj.id), - on_permission_grant=self._make_permission_grant_callback( - task_id, session_obj.id - ), - ) - await asyncio.to_thread(self._update_session_pid, session_obj.id, pid) - reader_task.add_done_callback( - lambda t: asyncio.create_task( - self._handle_acp_done(t, task_id, session_obj.id) - ).add_done_callback(_log_task_failure) - ) - else: - _raw_timeout = settings_dict.get("agent_timeout_seconds") - _timeout = agent_timeout_seconds(_raw_timeout) - pid = await spawn_agent( - agent_backend, - Path(ws.worktree_path), - prompt, - session_id=session_obj.id, - task_id=task_id, - db_path=db_path_str, - project_id=task.project_id, - timeout_seconds=_timeout, - ) - await asyncio.to_thread(self._update_session_pid, session_obj.id, pid) - monitor = asyncio.create_task( - self._monitor_detached(pid, task_id, session_obj.id), - name=f"agent-monitor:{task_id}", - ) - monitor.add_done_callback(_log_task_failure) - logger.info("Detached session started for task={}", task_id) - return session_obj + project_path = Path(ws.worktree_path) + if task.status is TaskStatus.REVIEW: + prompt = resolve_review_prompt(task_id, settings_dict, project_path) + else: + learnings = await self._fetch_project_learnings(task.project_id) + task_criteria_texts = await _db_async( + self._engine, + lambda s: [ + c.text + for c in s.exec( + select(AcceptanceCriterion).where( + AcceptanceCriterion.task_id == task_id + ) + ).all() + ], + ) + prompt = resolve_task_prompt( + task, + settings_dict, + project_path, + learnings=learnings, + criteria_texts=task_criteria_texts, + ) + + persona_prompt: str | None = None + if persona: + persona_prompt = get_persona_prompt(persona, settings_dict) + if persona_prompt and persona_prompt.strip(): + prompt = f"{build_persona_section(persona_prompt)}\n\n{prompt}" + + if backend_spec.has_capability(BackendCapability.ACP_STREAMING): + # Register the completion handler as an AgentEnd subscriber so + # that wait_idle() can synchronise callers waiting for the + # session to finish processing (settlement rule). + self._events.register_agent_end_subscriber(session_obj.id) + pid, reader_task = await spawn_agent_via_acp( + agent_backend, + Path(ws.worktree_path), + prompt, + session_id=session_obj.id, + task_id=task_id, + db_path=db_path_str, + project_id=task.project_id, + on_session_update=self._make_acp_callback(task_id, session_obj.id), + on_permission_grant=self._make_permission_grant_callback( + task_id, session_obj.id + ), + ) + await self._update_session_pid(session_obj.id, pid) + reader_task.add_done_callback( + lambda t: asyncio.create_task( + self._handle_acp_done(t, task_id, session_obj.id) + ).add_done_callback(_log_task_failure) + ) + else: + _raw_timeout = settings_dict.get("agent_timeout_seconds") + _timeout = agent_timeout_seconds(_raw_timeout) + pid = await spawn_agent( + agent_backend, + Path(ws.worktree_path), + prompt, + session_id=session_obj.id, + task_id=task_id, + db_path=db_path_str, + project_id=task.project_id, + timeout_seconds=_timeout, + ) + await self._update_session_pid(session_obj.id, pid) + monitor = asyncio.create_task( + self._monitor_detached(pid, task_id, session_obj.id), + name=f"agent-monitor:{task_id}", + ) + monitor.add_done_callback(_log_task_failure) + logger.info("Detached session started for task={}", task_id) + return session_obj + async def _run_attached( + self, + task: Task, + ws: Worktree, + session_obj: Session, + task_id: str, + agent_backend: str, + launcher: str, + ide: str | None, + db_path_str: str, + backend_spec: BackendSpec, + ) -> Session: launch_fn = get_launcher(launcher or "") - db_path_str = str(self._db_path or default_db_path()) - backend_spec = get_backend_spec(agent_backend) criteria_texts = await _db_async( self._engine, lambda s: [ @@ -568,10 +597,11 @@ async def run( if ide is not None: launch_kwargs["ide"] = ide await launch_fn(**launch_kwargs) - await asyncio.to_thread(self._mark_session_running, session_obj.id) + await self._mark_session_running(session_obj.id) logger.info("Interactive session launched for task={}", task_id) return session_obj + async def detach(self, task_id: str) -> DetachResult: task = await self._get_task(task_id) @@ -629,7 +659,7 @@ async def detach(self, task_id: str) -> DetachResult: ), ) if latest_attached_session is not None: - await asyncio.to_thread(self._complete_session, latest_attached_session.id) + await self._complete_session(latest_attached_session.id) return { "task_id": task_id, "status": TaskStatus.REVIEW.value, @@ -640,9 +670,9 @@ async def detach(self, task_id: str) -> DetachResult: if latest_attached_session is not None: if pending_before or pending_after: - await asyncio.to_thread(self._fail_session, latest_attached_session.id) + await self._fail_session(latest_attached_session.id) else: - await asyncio.to_thread(self._complete_session, latest_attached_session.id) + await self._complete_session(latest_attached_session.id) current = await self._get_task(task_id) return { @@ -820,7 +850,7 @@ async def _handle_acp_done(self, task: asyncio.Task, task_id: str, session_id: s exc = task.exception() if not task.cancelled() else None if exc is not None: logger.error("ACP session failed for task={}: {}", task_id, exc) - await asyncio.to_thread(self._fail_session, session_id) + await self._fail_session(session_id) await self._events.emit( task_id, "agent_failed", @@ -831,7 +861,7 @@ async def _handle_acp_done(self, task: asyncio.Task, task_id: str, session_id: s # Notify board so cards clear active_session. self._events.publish_board(BoardEvent(task_id=task_id, kind="session_ended")) else: - await asyncio.to_thread(self._complete_session, session_id) + await self._complete_session(session_id) db_task = await self._get_task(task_id) completed_session = await _db_async( self._engine, lambda s: s.get(Session, session_id) @@ -988,7 +1018,7 @@ async def _monitor_detached(self, pid: int, task_id: str, session_id: str) -> No raise ProcessLookupError(pid) except ProcessLookupError: unregister_spawned_process(session_id) - await asyncio.to_thread(self._complete_session, session_id) + await self._complete_session(session_id) task = await self._get_task(task_id) detached_session = await _db_async( self._engine, lambda s: s.get(Session, session_id) diff --git a/src/kagan/tui/screens/kanban.py b/src/kagan/tui/screens/kanban.py index 153c9c3c..016d1d0e 100644 --- a/src/kagan/tui/screens/kanban.py +++ b/src/kagan/tui/screens/kanban.py @@ -1,6 +1,7 @@ import asyncio import contextlib from dataclasses import dataclass +from enum import StrEnum from pathlib import Path from typing import TYPE_CHECKING, Any, cast @@ -58,6 +59,11 @@ from kagan.tui.app import KaganApp +class LayoutMode(StrEnum): + VERTICAL = "vertical" + HORIZONTAL = "horizontal" + + _TUTORIAL_SEEN_SETTING_KEY = "ui.tui_tutorial_seen" @@ -137,7 +143,7 @@ class KanbanScreen(Screen[None]): COMMANDS = {KanbanCommandProvider} BINDINGS = KANBAN_BINDINGS search_visible: reactive[bool] = reactive(False, init=False) - _chat_overlay_layout_mode: reactive[str] = reactive("vertical", init=False) + _chat_overlay_layout_mode: reactive[LayoutMode] = reactive(LayoutMode.VERTICAL, init=False) def __init__(self) -> None: super().__init__(id="kanban-screen") @@ -266,7 +272,7 @@ async def _bootstrap_initial_state(self) -> None: if not self._all_tasks: panel = self.query_one(ChatPanel) if not panel.has_class("visible"): - self._chat_overlay_layout_mode = "vertical" + self._chat_overlay_layout_mode = LayoutMode.VERTICAL self._chat_auto_opened = True await self.action_open_orchestrator_chat() @@ -380,23 +386,23 @@ def _sync_layout_state(self) -> None: visible = panel.has_class("visible") fullscreen = visible and panel.has_class("fullscreen") if not self._all_tasks and visible and not fullscreen: - self._chat_overlay_layout_mode = "vertical" + self._chat_overlay_layout_mode = LayoutMode.VERTICAL expanded = visible and (panel.has_class("expanded") or fullscreen) self.set_class(visible, "chat-overlay-visible") self.set_class(expanded, "chat-overlay-expanded") self.set_class( - visible and not fullscreen and self._chat_overlay_layout_mode == "vertical", + visible and not fullscreen and self._chat_overlay_layout_mode == LayoutMode.VERTICAL, "chat-overlay-vertical", ) self.set_class( - visible and not fullscreen and self._chat_overlay_layout_mode == "horizontal", + visible and not fullscreen and self._chat_overlay_layout_mode == LayoutMode.HORIZONTAL, "chat-overlay-horizontal", ) self._update_hint_bar() self._update_review_queue_hint() - def watch__chat_overlay_layout_mode(self, _value: str) -> None: + def watch__chat_overlay_layout_mode(self, _value: LayoutMode) -> None: """Re-sync layout CSS classes whenever the layout mode reactive changes.""" if not self.is_mounted: return @@ -555,17 +561,17 @@ def _compute_task_stats(self) -> tuple[dict[str, int], int]: def _update_search_bar_state(self) -> None: status_counts, high_priority_count = self._compute_task_stats() - sf = self._search_status_filter - pf = self._search_priority_filter - sf2 = self._search_sort_filter + status_filter = self._search_status_filter + priority_filter = self._search_priority_filter + sort_filter = self._search_sort_filter self.query_one(SearchBar).update_state( filtered_count=len(self._tasks), total_count=len(self._all_tasks), status_counts=status_counts, high_priority_count=high_priority_count, - status_filter=sf.value.lower() if sf is not None else "", - priority_filter=pf or "", - sort_filter=sf2 or "", + status_filter=status_filter.value.lower() if status_filter is not None else "", + priority_filter=priority_filter or "", + sort_filter=sort_filter or "", search_active=self.search_visible, ) @@ -652,9 +658,9 @@ def _mode_label(self) -> str: panel = self.query_one(ChatPanel) if panel.has_class("visible") and panel.has_class("fullscreen"): return "Assistant" - if panel.has_class("visible") and self._chat_overlay_layout_mode == "vertical": + if panel.has_class("visible") and self._chat_overlay_layout_mode == LayoutMode.VERTICAL: return "Split" - if panel.has_class("visible") and self._chat_overlay_layout_mode == "horizontal": + if panel.has_class("visible") and self._chat_overlay_layout_mode == LayoutMode.HORIZONTAL: return "Docked" if self.search_visible: return "Search" @@ -719,21 +725,33 @@ async def _run_branch_sync_loop(self) -> None: await self._sync_branch() def _task_actions(self, task: Task | None) -> list[tuple[str, str]]: - open_key = get_key_for_action(KANBAN_BINDINGS, "open_task", default="Enter") - search_key = get_key_for_action(KANBAN_BINDINGS, "search", default="/") - new_key = get_key_for_action(KANBAN_BINDINGS, "new_task", default="n") - overlay_key = get_key_for_action(KANBAN_BINDINGS, "toggle_chat", default="Ctrl+.") - right_key = get_key_for_action(KANBAN_BINDINGS, "move_right", default="Shift+Right") - left_key = get_key_for_action(KANBAN_BINDINGS, "move_left", default="Shift+Left") - session_key = get_key_for_action(KANBAN_BINDINGS, "open_task", default="Enter") - stop_key = get_key_for_action(KANBAN_BINDINGS, "stop_agent", default="Shift+S") - attach_key = get_key_for_action(KANBAN_BINDINGS, "attach_agent", default="a") - start_agent_key = get_key_for_action(KANBAN_BINDINGS, "start_agent", default="s") + keys = { + "open_task": get_key_for_action(KANBAN_BINDINGS, "open_task", default="Enter"), + "search": get_key_for_action(KANBAN_BINDINGS, "search", default="/"), + "new_task": get_key_for_action(KANBAN_BINDINGS, "new_task", default="n"), + "toggle_chat": get_key_for_action(KANBAN_BINDINGS, "toggle_chat", default="Ctrl+."), + "move_right": get_key_for_action(KANBAN_BINDINGS, "move_right", default="Shift+Right"), + "move_left": get_key_for_action(KANBAN_BINDINGS, "move_left", default="Shift+Left"), + "stop_agent": get_key_for_action(KANBAN_BINDINGS, "stop_agent", default="Shift+S"), + "attach_agent": get_key_for_action(KANBAN_BINDINGS, "attach_agent", default="a"), + "start_agent": get_key_for_action(KANBAN_BINDINGS, "start_agent", default="s"), + "expand_chat_overlay": get_key_for_action( + KANBAN_BINDINGS, "expand_chat_overlay", default="Ctrl+F" + ), + } + open_key = keys["open_task"] + search_key = keys["search"] + new_key = keys["new_task"] + overlay_key = keys["toggle_chat"] + right_key = keys["move_right"] + left_key = keys["move_left"] + session_key = keys["open_task"] + stop_key = keys["stop_agent"] + attach_key = keys["attach_agent"] + start_agent_key = keys["start_agent"] panel = self.query_one(ChatPanel) overlay_open = panel.has_class("visible") - fullscreen_key = get_key_for_action( - KANBAN_BINDINGS, "expand_chat_overlay", default="Ctrl+F" - ) + fullscreen_key = keys["expand_chat_overlay"] overlay_hints: list[tuple[str, str]] = [] if overlay_open: @@ -836,7 +854,7 @@ async def action_open_orchestrator_chat(self) -> None: panel.set_visible(True) panel.set_fullscreen(False) if not was_visible: - self._chat_overlay_layout_mode = "vertical" + self._chat_overlay_layout_mode = LayoutMode.VERTICAL panel.set_mode_title("Orchestrator") panel.set_session_kind(SessionKind.ORCHESTRATOR) await self._load_orchestrator_panel_state(panel) @@ -857,7 +875,7 @@ async def action_open_task_overlay(self) -> None: panel.set_visible(True) panel.set_fullscreen(False) if not was_visible: - self._chat_overlay_layout_mode = "vertical" + self._chat_overlay_layout_mode = LayoutMode.VERTICAL panel.query_one("#chat-overlay-input", Input).focus() self._chat_mode = ChatMode.TASK @@ -898,7 +916,7 @@ async def action_open_task_chat(self) -> None: def _transition_from_fullscreen(self, panel: ChatPanel) -> None: """Exit fullscreen mode and return to vertical overlay.""" panel.set_fullscreen(False) - self._chat_overlay_layout_mode = "vertical" + self._chat_overlay_layout_mode = LayoutMode.VERTICAL self._chat_auto_opened = False panel.query_one("#chat-overlay-input", Input).focus() self._sync_layout_state() @@ -917,7 +935,7 @@ def _transition_to_horizontal(self, panel: ChatPanel) -> None: if not self._all_tasks: self._transition_to_hidden(panel) return - self._chat_overlay_layout_mode = "horizontal" # watcher fires _sync_layout_state + self._chat_overlay_layout_mode = LayoutMode.HORIZONTAL # watcher fires _sync_layout_state self._chat_auto_opened = False panel.query_one("#chat-overlay-input", Input).focus() @@ -930,7 +948,7 @@ def action_toggle_chat(self) -> None: visible = panel.has_class("visible") fullscreen = visible and panel.has_class("fullscreen") - vertical = self._chat_overlay_layout_mode == "vertical" + vertical = self._chat_overlay_layout_mode == LayoutMode.VERTICAL if fullscreen: self._transition_from_fullscreen(panel) @@ -953,7 +971,7 @@ def _apply_panel_visibility( *, visible: bool, fullscreen: bool, - layout: str = "vertical", + layout: LayoutMode = LayoutMode.VERTICAL, ) -> None: """Set panel visible/fullscreen flags and the layout mode in one place. @@ -965,9 +983,9 @@ def _apply_panel_visibility( panel.set_fullscreen(fullscreen) self._chat_auto_opened = False if not visible: - self._chat_overlay_layout_mode = "vertical" + self._chat_overlay_layout_mode = LayoutMode.VERTICAL elif not fullscreen: - self._chat_overlay_layout_mode = layout + self._chat_overlay_layout_mode = LayoutMode(layout) self._sync_layout_state() async def action_fullscreen_chat(self) -> None: @@ -1731,7 +1749,7 @@ def on_chat_panel_close_requested(self, message: ChatPanel.CloseRequested) -> No panel = self.query_one(ChatPanel) panel.set_visible(False) panel.set_fullscreen(False) - self._chat_overlay_layout_mode = "vertical" + self._chat_overlay_layout_mode = LayoutMode.VERTICAL self._sync_layout_state() def on_chat_panel_interrupt_requested(self, _: ChatPanel.InterruptRequested) -> None: @@ -2029,7 +2047,7 @@ async def on_key(self, event: events.Key) -> None: event.stop() panel.set_visible(False) panel.set_fullscreen(False) - self._chat_overlay_layout_mode = "vertical" + self._chat_overlay_layout_mode = LayoutMode.VERTICAL self._sync_layout_state() return diff --git a/src/kagan/tui/widgets/chat.py b/src/kagan/tui/widgets/chat.py index ca592b2b..bfd218d4 100644 --- a/src/kagan/tui/widgets/chat.py +++ b/src/kagan/tui/widgets/chat.py @@ -712,66 +712,69 @@ def _on_permission_decision(self, event: PermissionPrompt.DecisionMade) -> None: self.add_system_message(f"Permission {event.decision}") self._render_decision_surface() - def on_key(self, event: Key) -> None: - """Intercept arrow keys for slash/mention completion overlay navigation.""" + def _handle_overlay_navigation(self, event: Key) -> bool: overlay_visible = bool(self._slash_matches or self._mention_matches) - if overlay_visible: - option_list_id = "#slash-options" if self._slash_matches else "#mention-options" - option_list = self.query_one(option_list_id, OptionList) - - nav_map: dict[str, str] = { - "up": "action_cursor_up", - "down": "action_cursor_down", - "pageup": "action_page_up", - "pagedown": "action_page_down", - "home": "action_first", - "end": "action_last", - } - - if event.key in nav_map: - event.prevent_default() - event.stop() - with contextlib.suppress(AttributeError): - getattr(option_list, nav_map[event.key])() - return + if not overlay_visible: + return False + option_list_id = "#slash-options" if self._slash_matches else "#mention-options" + option_list = self.query_one(option_list_id, OptionList) + + nav_map: dict[str, str] = { + "up": "action_cursor_up", + "down": "action_cursor_down", + "pageup": "action_page_up", + "pagedown": "action_page_down", + "home": "action_first", + "end": "action_last", + } + + if event.key in nav_map: + event.prevent_default() + event.stop() + with contextlib.suppress(AttributeError): + getattr(option_list, nav_map[event.key])() + return True - if event.key == "enter": - # If the input already contains a complete slash command, submit it - # instead of merely accepting the overlay selection. - current_text = self._input_widget().value.strip() - if current_text.startswith("/"): - cmd_name = ( - current_text.lstrip("/").split()[0] if current_text.lstrip("/") else "" - ) - specs = [ - spec - for spec in SLASH_COMMAND_REGISTRY.specs() - if spec.name in _TUI_SLASH_COMMANDS - ] - exact_match = any(spec.name == cmd_name for spec in specs) - if exact_match: - self._hide_overlays() - self.call_later(self.action_send_message) - return - event.prevent_default() - event.stop() - self.action_accept_completion() - return + if event.key == "enter": + # If the input already contains a complete slash command, submit it + # instead of merely accepting the overlay selection. + current_text = self._input_widget().value.strip() + if current_text.startswith("/"): + cmd_name = ( + current_text.lstrip("/").split()[0] if current_text.lstrip("/") else "" + ) + specs = [ + spec + for spec in SLASH_COMMAND_REGISTRY.specs() + if spec.name in _TUI_SLASH_COMMANDS + ] + exact_match = any(spec.name == cmd_name for spec in specs) + if exact_match: + self._hide_overlays() + self.call_later(self.action_send_message) + return True + event.prevent_default() + event.stop() + self.action_accept_completion() + return True - if event.key == "escape": - event.prevent_default() - event.stop() - self._hide_overlays() - return + if event.key == "escape": + event.prevent_default() + event.stop() + self._hide_overlays() + return True + + return False + def _handle_input_editing(self, event: Key) -> bool: if event.key == "escape" and not self._input_has_focus(): event.prevent_default() event.stop() self._input_widget().focus() - return + return True if not self._input_has_focus(): - return + return True if event.key == "ctrl+c": event.prevent_default() @@ -779,28 +782,38 @@ def on_key(self, event: Key) -> None: # Ctrl+C clears input — Esc stops agent + edits last message if self._input_widget().value: self.action_clear_input() - return + return True if event.key == "enter": event.prevent_default() event.stop() self.call_later(self.action_send_message) - return + return True - if overlay_visible: - return + return False + def _handle_prompt_history(self, event: Key) -> bool: if event.key == "up": if self._cycle_prompt_history(direction="up"): event.prevent_default() event.stop() - return + return True if event.key == "down": if self._cycle_prompt_history(direction="down"): event.prevent_default() event.stop() + return True + + return False + + def on_key(self, event: Key) -> None: + """Intercept arrow keys for slash/mention completion overlay navigation.""" + if self._handle_overlay_navigation(event): + return + if self._handle_input_editing(event): return + self._handle_prompt_history(event) async def action_send_message(self) -> None: await self._submit_current_input() @@ -1198,7 +1211,7 @@ def _switch_session(self, key: str, *, emit: bool) -> None: previous_key = self._selected_session_key if self.is_mounted: self._flush_deferred() - input_widget = self._input_widget_safe() + input_widget = self._input_widget() if input_widget is not None: self._ensure_session_state(previous_key).draft = input_widget.value @@ -1208,7 +1221,7 @@ def _switch_session(self, key: str, *, emit: bool) -> None: self.set_session_kind(self._infer_session_kind(key)) if self.is_mounted: - input_widget = self._input_widget_safe() + input_widget = self._input_widget() if input_widget is not None: input_widget.value = self._current_state().draft self._sync_completion_overlays(input_widget.value) @@ -1274,18 +1287,12 @@ def _flush_deferred(self) -> None: self._update_content_state() self._refresh_status() - def _input_widget(self) -> Input: - chat_input = self._chat_input() - if chat_input is None: - return self.query_one("#chat-overlay-input", Input) - return chat_input.input_widget() - - def _input_widget_safe(self) -> Input | None: - # Returns None when called from a partial-mount test fixture that - # doesn't yield the input widget (e.g. doctor modal harness). All - # post-Ready call sites can use the bare _input_widget() directly. + def _input_widget(self) -> Input | None: try: - return self._input_widget() + chat_input = self._chat_input() + if chat_input is None: + return self.query_one("#chat-overlay-input", Input) + return chat_input.input_widget() except NoMatches: return None @@ -1324,7 +1331,7 @@ def set_chat_input_disabled(self, disabled: bool) -> None: def _sync_input_enabled_state(self) -> None: # Reachable from on_mount before all children are guaranteed; degrade # quietly when the input widget is absent in a partial-compose harness. - input_widget = self._input_widget_safe() + input_widget = self._input_widget() if input_widget is None: return visible = self.has_class("visible") @@ -1480,7 +1487,7 @@ def _refresh_status(self) -> None: close_key = self._overlay_close_key is_active = self._runtime_status in {"thinking", "initializing", "waiting"} if is_active: - input_widget = self._input_widget_safe() + input_widget = self._input_widget() value = input_widget.value if input_widget is not None else "" has_pending = bool(normalize_chat_input(value)) esc_hint = "Esc stop+send" if has_pending else "Esc stop & edit last" diff --git a/tests/tui/test_post_audit_cleanups.py b/tests/tui/test_post_audit_cleanups.py index c7075177..629baef0 100644 --- a/tests/tui/test_post_audit_cleanups.py +++ b/tests/tui/test_post_audit_cleanups.py @@ -71,7 +71,7 @@ async def test_task_screen_opens_without_app_active_task_id(board: KaganDriver) async def test_layout_mode_reactive_drives_css_on_vertical(board: KaganDriver) -> None: """Setting _chat_overlay_layout_mode=vertical updates chat-overlay-vertical class.""" from kagan.tui import KaganApp - from kagan.tui.screens.kanban import KanbanScreen + from kagan.tui.screens.kanban import KanbanScreen, LayoutMode app = KaganApp(db_path=board.tmp_path / "kagan.db") async with app.run_test() as pilot: @@ -82,7 +82,7 @@ async def test_layout_mode_reactive_drives_css_on_vertical(board: KaganDriver) - screen: KanbanScreen = app.screen await pilot.press("ctrl+i") await pilot.pause() - screen._chat_overlay_layout_mode = "vertical" + screen._chat_overlay_layout_mode = LayoutMode.VERTICAL await pilot.pause() assert screen.has_class("chat-overlay-vertical") assert not screen.has_class("chat-overlay-horizontal") @@ -91,7 +91,7 @@ async def test_layout_mode_reactive_drives_css_on_vertical(board: KaganDriver) - async def test_layout_mode_reactive_drives_css_on_horizontal(board: KaganDriver) -> None: """Setting _chat_overlay_layout_mode=horizontal updates chat-overlay-horizontal class.""" from kagan.tui import KaganApp - from kagan.tui.screens.kanban import KanbanScreen + from kagan.tui.screens.kanban import KanbanScreen, LayoutMode app = KaganApp(db_path=board.tmp_path / "kagan.db") async with app.run_test() as pilot: @@ -102,7 +102,7 @@ async def test_layout_mode_reactive_drives_css_on_horizontal(board: KaganDriver) screen: KanbanScreen = app.screen await pilot.press("ctrl+i") await pilot.pause() - screen._chat_overlay_layout_mode = "horizontal" + screen._chat_overlay_layout_mode = LayoutMode.HORIZONTAL await pilot.pause() assert screen.has_class("chat-overlay-horizontal") assert not screen.has_class("chat-overlay-vertical") From 7db887513cd7173102f06b114791810df1714de5 Mon Sep 17 00:00:00 2001 From: Altynbek Orumbayev Date: Wed, 6 May 2026 10:13:57 +0200 Subject: [PATCH 3/8] refactor(tui): P3 cosmetic fixes across settings, footer, workspace, chat, kanban --- src/kagan/tui/screens/settings.py | 171 +++++++++++---------- src/kagan/tui/screens/task_editor_modal.py | 58 ++++--- src/kagan/tui/screens/workspace.py | 39 ++++- src/kagan/tui/widgets/context_footer.py | 61 ++++---- 4 files changed, 190 insertions(+), 139 deletions(-) diff --git a/src/kagan/tui/screens/settings.py b/src/kagan/tui/screens/settings.py index 5626b54d..d7a81a02 100644 --- a/src/kagan/tui/screens/settings.py +++ b/src/kagan/tui/screens/settings.py @@ -27,6 +27,22 @@ from kagan.tui.app import KaganApp +_REVIEW_STRICTNESS_VALUES: set[str] = {"strict", "balanced", "relaxed"} +_PLANNING_DEPTH_VALUES: set[str] = {"always", "multi_task", "never"} +_ATTACHED_LAUNCHER_VALUES: set[str] = { + "tmux", + "nvim", + "vscode", + "cursor", + "windsurf", + "kiro", + "antigravity", +} +_STARTUP_SURFACE_VALUES: set[str] = {"tui", "web", "chat", "ask"} +_BASE_REF_STRATEGY_VALUES: set[str] = {"local_if_ahead", "remote", "local"} +_GIT_USER_MODE_VALUES: set[str] = {"kagan_agent", "system_default", "custom"} + + class CategoryList(OptionList): def __init__(self, categories: list[SettingCategory]) -> None: self._categories = categories @@ -149,57 +165,46 @@ async def on_mount(self) -> None: self._update_search_status("") def _set_values(self, settings: dict[str, str]) -> None: - def _sw(wid: str, key: str, *, default: bool) -> None: - self.query_one(wid, Switch).value = _is_enabled(settings.get(key), default=default) - - def _sel(wid: str, value: str, allowed: set[str], fallback: str) -> None: - self.query_one(wid, Select).value = value if value in allowed else fallback - available_agents = {v for _, v in build_agent_backend_options()} default_agent = resolve_default_agent_backend(settings) if default_agent not in available_agents: default_agent = resolve_default_agent_backend({}) self.query_one("#settings-default-agent", Select).value = default_agent - _sel( - "#settings-review-strictness", - settings.get("review_strictness", "balanced"), - {"strict", "balanced", "relaxed"}, - "balanced", + self.query_one("#settings-review-strictness", Select).value = ( + settings.get("review_strictness", "balanced") + if settings.get("review_strictness", "balanced") in _REVIEW_STRICTNESS_VALUES + else "balanced" ) - _sel( - "#settings-planning-depth", - settings.get("planning_depth", "always"), - {"always", "multi_task", "never"}, - "always", + self.query_one("#settings-planning-depth", Select).value = ( + settings.get("planning_depth", "always") + if settings.get("planning_depth", "always") in _PLANNING_DEPTH_VALUES + else "always" ) - _sel( - "#settings-theme", - settings.get("theme", ""), - valid_theme_names(), - "", + self.query_one("#settings-theme", Select).value = ( + settings.get("theme", "") + if settings.get("theme", "") in valid_theme_names() + else "" ) - _sel( - "#settings-attached-launcher", - settings.get("attached_launcher", "tmux"), - {"tmux", "nvim", "vscode", "cursor", "windsurf", "kiro", "antigravity"}, - "tmux", + self.query_one("#settings-attached-launcher", Select).value = ( + settings.get("attached_launcher", "tmux") + if settings.get("attached_launcher", "tmux") in _ATTACHED_LAUNCHER_VALUES + else "tmux" ) startup_surface = settings.get("startup_default_surface") or settings.get( "ui.surface_chooser_last_choice", "tui" ) - _sel("#settings-startup-surface", startup_surface, {"tui", "web", "chat", "ask"}, "tui") - _sel( - "#settings-base-ref-strategy", - settings.get("worktree_base_ref_strategy", "local_if_ahead"), - {"local_if_ahead", "remote", "local"}, - "local_if_ahead", + self.query_one("#settings-startup-surface", Select).value = ( + startup_surface if startup_surface in _STARTUP_SURFACE_VALUES else "tui" + ) + _base_ref = settings.get("worktree_base_ref_strategy", "local_if_ahead") + self.query_one("#settings-base-ref-strategy", Select).value = ( + _base_ref if _base_ref in _BASE_REF_STRATEGY_VALUES else "local_if_ahead" ) - _sel( - "#settings-git-user-mode", - settings.get("git_user_mode", "kagan_agent"), - {"kagan_agent", "system_default", "custom"}, - "kagan_agent", + self.query_one("#settings-git-user-mode", Select).value = ( + settings.get("git_user_mode", "kagan_agent") + if settings.get("git_user_mode", "kagan_agent") in _GIT_USER_MODE_VALUES + else "kagan_agent" ) self.query_one("#settings-default-base-branch", Input).value = settings.get( @@ -211,16 +216,26 @@ def _sel(wid: str, value: str, allowed: set[str], fallback: str) -> None: "additional_instructions", "" ) - _sw("#settings-auto-confirm-single", "auto_confirm_single_tasks", default=False) - _sw("#settings-auto-review", "auto_review", default=True) - _sw("#settings-open-last-project", "open_last_project_on_startup", default=False) - _sw("#settings-auto-init-repo", "auto_init_git_repo", default=True) - _sw("#settings-auto-init-commit", "auto_init_git_initial_commit", default=True) - _sw("#settings-require-review-approval", "require_review_approval", default=False) - _sw( - "#settings-skip-attached-instructions", - "skip_attached_instructions_popup", - default=False, + self.query_one("#settings-auto-confirm-single", Switch).value = _is_enabled( + settings.get("auto_confirm_single_tasks"), default=False + ) + self.query_one("#settings-auto-review", Switch).value = _is_enabled( + settings.get("auto_review"), default=True + ) + self.query_one("#settings-open-last-project", Switch).value = _is_enabled( + settings.get("open_last_project_on_startup"), default=False + ) + self.query_one("#settings-auto-init-repo", Switch).value = _is_enabled( + settings.get("auto_init_git_repo"), default=True + ) + self.query_one("#settings-auto-init-commit", Switch).value = _is_enabled( + settings.get("auto_init_git_initial_commit"), default=True + ) + self.query_one("#settings-require-review-approval", Switch).value = _is_enabled( + settings.get("require_review_approval"), default=False + ) + self.query_one("#settings-skip-attached-instructions", Switch).value = _is_enabled( + settings.get("skip_attached_instructions_popup"), default=False ) overrides = detect_dotfile_overrides(Path.cwd()) @@ -286,13 +301,6 @@ def _fire_auto_save(self) -> None: self.run_worker(self._save_all_settings(), exit_on_error=False) async def _save_all_settings(self) -> None: - def _sv(wid: str, allowed: set[str], fallback: str) -> str: - val = self.query_one(wid, Select).value - return val if isinstance(val, str) and val in allowed else fallback - - def _bool(wid: str) -> str: - return "true" if self.query_one(wid, Switch).value else "false" - agent_val = self.query_one("#settings-default-agent", Select).value available_agents = {v for _, v in build_agent_backend_options()} default_agent = ( @@ -301,32 +309,33 @@ def _bool(wid: str) -> str: else resolve_default_agent_backend({}) ) + def _select_value(wid: str, allowed: set[str], fallback: str) -> str: + val = self.query_one(wid, Select).value + return val if isinstance(val, str) and val in allowed else fallback + + def _switch_value(wid: str) -> str: + return "true" if self.query_one(wid, Switch).value else "false" + updates: dict[str, str] = { "default_agent_backend": default_agent, - "theme": _sv("#settings-theme", valid_theme_names(), ""), - "review_strictness": _sv( - "#settings-review-strictness", {"strict", "balanced", "relaxed"}, "balanced" + "theme": _select_value("#settings-theme", valid_theme_names(), ""), + "review_strictness": _select_value( + "#settings-review-strictness", _REVIEW_STRICTNESS_VALUES, "balanced" ), - "planning_depth": _sv( - "#settings-planning-depth", {"always", "multi_task", "never"}, "always" + "planning_depth": _select_value( + "#settings-planning-depth", _PLANNING_DEPTH_VALUES, "always" ), - "attached_launcher": _sv( - "#settings-attached-launcher", - {"tmux", "nvim", "vscode", "cursor", "windsurf", "kiro", "antigravity"}, - "tmux", + "attached_launcher": _select_value( + "#settings-attached-launcher", _ATTACHED_LAUNCHER_VALUES, "tmux" ), - "startup_default_surface": _sv( - "#settings-startup-surface", {"tui", "web", "chat", "ask"}, "tui" + "startup_default_surface": _select_value( + "#settings-startup-surface", _STARTUP_SURFACE_VALUES, "tui" ), - "worktree_base_ref_strategy": _sv( - "#settings-base-ref-strategy", - {"local_if_ahead", "remote", "local"}, - "local_if_ahead", + "worktree_base_ref_strategy": _select_value( + "#settings-base-ref-strategy", _BASE_REF_STRATEGY_VALUES, "local_if_ahead" ), - "git_user_mode": _sv( - "#settings-git-user-mode", - {"kagan_agent", "system_default", "custom"}, - "kagan_agent", + "git_user_mode": _select_value( + "#settings-git-user-mode", _GIT_USER_MODE_VALUES, "kagan_agent" ), "default_base_branch": ( self.query_one("#settings-default-base-branch", Input).value.strip() or "main" @@ -334,13 +343,15 @@ def _bool(wid: str) -> str: "additional_instructions": ( self.query_one("#settings-additional-instructions", TextArea).text.strip() ), - "auto_review": _bool("#settings-auto-review"), - "open_last_project_on_startup": _bool("#settings-open-last-project"), - "auto_init_git_repo": _bool("#settings-auto-init-repo"), - "auto_init_git_initial_commit": _bool("#settings-auto-init-commit"), - "require_review_approval": _bool("#settings-require-review-approval"), - "skip_attached_instructions_popup": _bool("#settings-skip-attached-instructions"), - "auto_confirm_single_tasks": _bool("#settings-auto-confirm-single"), + "auto_review": _switch_value("#settings-auto-review"), + "open_last_project_on_startup": _switch_value("#settings-open-last-project"), + "auto_init_git_repo": _switch_value("#settings-auto-init-repo"), + "auto_init_git_initial_commit": _switch_value("#settings-auto-init-commit"), + "require_review_approval": _switch_value("#settings-require-review-approval"), + "skip_attached_instructions_popup": _switch_value( + "#settings-skip-attached-instructions" + ), + "auto_confirm_single_tasks": _switch_value("#settings-auto-confirm-single"), } git_user_name = self.query_one("#settings-git-user-name", Input).value.strip() diff --git a/src/kagan/tui/screens/task_editor_modal.py b/src/kagan/tui/screens/task_editor_modal.py index af965947..21f847fa 100644 --- a/src/kagan/tui/screens/task_editor_modal.py +++ b/src/kagan/tui/screens/task_editor_modal.py @@ -83,36 +83,48 @@ def compose(self) -> ComposeResult: classes="modal-action-hint", ) - async def on_task_submitted(self, message: TaskSubmitted) -> None: + async def on_task_submitted(self, event: TaskSubmitted) -> None: + if self._editing_task: + await self._update_task(event) + else: + await self._create_task(event) + + async def _create_task(self, event: TaskSubmitted) -> None: editor = self.query_one(TaskEditor) acceptance_criteria = editor.acceptance_criteria() try: - if self._editing_task is None: - await self.kagan_app.core.tasks.create( - message.title, - description=message.description, - priority=message.priority, - base_branch=message.base_branch, - agent_backend=message.agent_backend, - launcher=message.launcher, - acceptance_criteria=acceptance_criteria, - github_issue=message.github_issue, - ) - else: - await self.kagan_app.core.tasks.update( - self._editing_task.id, - title=message.title, - description=message.description, - priority=message.priority, - base_branch=message.base_branch, - agent_backend=message.agent_backend, - launcher=message.launcher, - acceptance_criteria=acceptance_criteria, - ) + await self.kagan_app.core.tasks.create( + event.title, + description=event.description, + priority=event.priority, + base_branch=event.base_branch, + agent_backend=event.agent_backend, + launcher=event.launcher, + acceptance_criteria=acceptance_criteria, + github_issue=event.github_issue, + ) except Exception as exc: # quality-allow-broad-except self.kagan_app.notify(f"Unable to save task: {exc}", severity="error") return + self.dismiss(None) + async def _update_task(self, event: TaskSubmitted) -> None: + editor = self.query_one(TaskEditor) + acceptance_criteria = editor.acceptance_criteria() + try: + await self.kagan_app.core.tasks.update( + self._editing_task.id, + title=event.title, + description=event.description, + priority=event.priority, + base_branch=event.base_branch, + agent_backend=event.agent_backend, + launcher=event.launcher, + acceptance_criteria=acceptance_criteria, + ) + except Exception as exc: # quality-allow-broad-except + self.kagan_app.notify(f"Unable to save task: {exc}", severity="error") + return self.dismiss(None) def on_task_editor_cancelled(self, _: TaskEditor.Cancelled) -> None: diff --git a/src/kagan/tui/screens/workspace.py b/src/kagan/tui/screens/workspace.py index b923599d..4039bf55 100644 --- a/src/kagan/tui/screens/workspace.py +++ b/src/kagan/tui/screens/workspace.py @@ -38,6 +38,7 @@ def __init__(self) -> None: self._chat_session_switch_token = 0 self._session_items: list[ChatSessionListItem] = [] self._visible_session_items: list[ChatSessionListItem] = [] + self._footer_keys: dict[str, str] = {} @property def kagan_app(self) -> "KaganApp": @@ -78,6 +79,17 @@ def compose(self) -> ComposeResult: yield Static("", id="workspace-footer") async def on_mount(self) -> None: + self._footer_keys = { + "focus_search": get_key_for_action(WORKSPACE_BINDINGS, "focus_search", default="/"), + "new_session": get_key_for_action(WORKSPACE_BINDINGS, "new_session", default="n"), + "delete_session": get_key_for_action(WORKSPACE_BINDINGS, "delete_session", default="x"), + "toggle_board": get_key_for_action(WORKSPACE_BINDINGS, "toggle_board", default="w"), + "focus_chat": get_key_for_action(WORKSPACE_BINDINGS, "focus_chat", default="Ctrl+."), + "switch_session": get_key_for_action( + WORKSPACE_BINDINGS, "switch_session", default="Ctrl+K" + ), + "open_session": get_key_for_action(WORKSPACE_BINDINGS, "open_session", default="Enter"), + } self._refresh_header() await self._refresh_header_context() self._update_footer() @@ -603,13 +615,26 @@ def _refresh_main_header(self, active_key: str | None = None) -> None: subtitle.update(" · ".join(parts)) def _update_footer(self, focused: Widget | None = None) -> None: - search_key = get_key_for_action(WORKSPACE_BINDINGS, "focus_search", default="/") - new_key = get_key_for_action(WORKSPACE_BINDINGS, "new_session", default="n") - delete_key = get_key_for_action(WORKSPACE_BINDINGS, "delete_session", default="x") - board_key = get_key_for_action(WORKSPACE_BINDINGS, "toggle_board", default="w") - chat_key = get_key_for_action(WORKSPACE_BINDINGS, "focus_chat", default="Ctrl+.") - switch_key = get_key_for_action(WORKSPACE_BINDINGS, "switch_session", default="Ctrl+K") - open_key = get_key_for_action(WORKSPACE_BINDINGS, "open_session", default="Enter") + keys = { + "focus_search": get_key_for_action(WORKSPACE_BINDINGS, "focus_search", default="/"), + "new_session": get_key_for_action(WORKSPACE_BINDINGS, "new_session", default="n"), + "delete_session": get_key_for_action( + WORKSPACE_BINDINGS, "delete_session", default="x" + ), + "toggle_board": get_key_for_action(WORKSPACE_BINDINGS, "toggle_board", default="w"), + "focus_chat": get_key_for_action(WORKSPACE_BINDINGS, "focus_chat", default="Ctrl+."), + "switch_session": get_key_for_action( + WORKSPACE_BINDINGS, "switch_session", default="Ctrl+K" + ), + "open_session": get_key_for_action(WORKSPACE_BINDINGS, "open_session", default="Enter"), + } + search_key = keys["focus_search"] + new_key = keys["new_session"] + delete_key = keys["delete_session"] + board_key = keys["toggle_board"] + chat_key = keys["focus_chat"] + switch_key = keys["switch_session"] + open_key = keys["open_session"] widget = focused or self.focused search = self.query_one("#workspace-search", Input) chat = self.query_one("#workspace-chat", ChatPanel) diff --git a/src/kagan/tui/widgets/context_footer.py b/src/kagan/tui/widgets/context_footer.py index b13f5cb8..a5f5b781 100644 --- a/src/kagan/tui/widgets/context_footer.py +++ b/src/kagan/tui/widgets/context_footer.py @@ -1,3 +1,4 @@ + from textual.binding import BindingType from textual.containers import Horizontal from textual.reactive import reactive @@ -7,6 +8,23 @@ FooterBuilder, ) +_PRIMARY_ACTIONS: dict[str, list[tuple[str, str]]] = { + "kanban": FooterBuilder.kanban_core(), + "kanban_with_card": FooterBuilder.kanban_with_card(), + "task": FooterBuilder.task_screen(), + "session": FooterBuilder.session_dashboard(), + "settings": FooterBuilder.settings(), + "confirm": FooterBuilder.confirm(), + "chat": FooterBuilder.chat(), +} + +_NAVIGATION_HINTS: dict[str, str] = { + "kanban": "", + "kanban_with_card": "", + "task": "[dim]1/2 tabs · Esc back[/]", + "session": "[dim]Ctrl+K sessions · Ctrl+. AI panel[/]", +} + class ContextFooter(Horizontal): DEFAULT_CSS = """ @@ -60,14 +78,17 @@ def compose(self): right_widget.tooltip = "Global keyboard shortcuts (press ? for help)" yield right_widget - def watch_context(self, context: str) -> None: + def _refresh_display(self) -> None: self._update_display() - def watch_has_focused_item(self, has_focus: bool) -> None: - self._update_display() + def watch_context(self, _context: str) -> None: + self._refresh_display() - def watch_sub_context(self, sub_context: str) -> None: - self._update_display() + def watch_has_focused_item(self, _has_focus: bool) -> None: + self._refresh_display() + + def watch_sub_context(self, _sub_context: str) -> None: + self._refresh_display() def set_context( self, @@ -93,36 +114,18 @@ def _update_display(self) -> None: center.update(center_content) right.update(right_content) - def _build_primary_actions(self, width: int) -> str: - if self.context == "kanban": - return self._format_hints(FooterBuilder.kanban_core()) - elif self.context == "kanban_with_card": - return self._format_hints(FooterBuilder.kanban_with_card()) - elif self.context == "task": - if self.sub_context == "review": - return self._format_hints(FooterBuilder.task_screen_review()) - return self._format_hints(FooterBuilder.task_screen()) - elif self.context == "session": - return self._format_hints(FooterBuilder.session_dashboard()) - elif self.context == "settings": - return self._format_hints(FooterBuilder.settings()) - elif self.context == "confirm": - return self._format_hints(FooterBuilder.confirm()) - elif self.context == "chat": - return self._format_hints(FooterBuilder.chat()) - return "" + def _build_primary_actions(self, _width: int) -> str: + if self.context == "task" and self.sub_context == "review": + return self._format_hints(FooterBuilder.task_screen_review()) + hints = _PRIMARY_ACTIONS.get(self.context) + return self._format_hints(hints) if hints else "" def _build_navigation_hints(self, width: int) -> str: if width < 100: return "" # Hide navigation hints on narrow screens - if self.context in ("kanban", "kanban_with_card"): return self._format_hints(FooterBuilder.kanban_navigation(), compact=True) - elif self.context == "task": - return "[dim]1/2 tabs · Esc back[/]" - elif self.context == "session": - return "[dim]Ctrl+K sessions · Ctrl+. AI panel[/]" - return "" + return _NAVIGATION_HINTS.get(self.context, "") def _build_global_hints(self, width: int) -> str: if width < 80: From 13aa471f9ae5d3339c990c3437ba456ea8cf3d47 Mon Sep 17 00:00:00 2001 From: Altynbek Orumbayev Date: Wed, 6 May 2026 12:40:16 +0200 Subject: [PATCH 4/8] test: update session shutdown mocks for async DB helpers --- tests/unit/test_sessions_shutdown.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/tests/unit/test_sessions_shutdown.py b/tests/unit/test_sessions_shutdown.py index 5658bd3d..6511b70e 100644 --- a/tests/unit/test_sessions_shutdown.py +++ b/tests/unit/test_sessions_shutdown.py @@ -218,11 +218,10 @@ async def fake_to_thread(fn: Any, *args: Any, **kwargs: Any) -> Any: ) monkeypatch.setattr("kagan.core._sessions.get_persona_prompt", lambda *_args, **_kwargs: None) monkeypatch.setattr("kagan.core._sessions.build_persona_section", lambda prompt: prompt) - monkeypatch.setattr( - sessions, - "_update_session_pid", - lambda session_id, pid: updated_pids.append((session_id, pid)), - ) + async def fake_update_session_pid(session_id: str, pid: int) -> None: + updated_pids.append((session_id, pid)) + + monkeypatch.setattr(sessions, "_update_session_pid", fake_update_session_pid) monkeypatch.setattr( sessions, "_make_acp_callback", lambda *_args, **_kwargs: (lambda *_a, **_k: None) ) @@ -333,11 +332,10 @@ async def fake_to_thread(fn: Any, *args: Any, **kwargs: Any) -> Any: ) monkeypatch.setattr("kagan.core._sessions.get_persona_prompt", lambda *_args, **_kwargs: None) monkeypatch.setattr("kagan.core._sessions.build_persona_section", lambda prompt: prompt) - monkeypatch.setattr( - sessions, - "_update_session_pid", - lambda session_id, pid: updated_pids.append((session_id, pid)), - ) + async def fake_update_session_pid(session_id: str, pid: int) -> None: + updated_pids.append((session_id, pid)) + + monkeypatch.setattr(sessions, "_update_session_pid", fake_update_session_pid) result = await sessions.run("task-timeout", agent_backend="codex") @@ -414,9 +412,10 @@ async def fake_to_thread(fn: Any, *args: Any, **kwargs: Any) -> Any: "kagan.core._sessions.build_attached_startup_prompt", lambda _task, _criteria=None: "prompt", ) - monkeypatch.setattr( - sessions, "_mark_session_running", lambda session_id: cast("Any", session_id) - ) + async def fake_mark_session_running(session_id: str) -> None: + pass + + monkeypatch.setattr(sessions, "_mark_session_running", fake_mark_session_running) result = await sessions.run("task-2", agent_backend="codex", launcher="vscode") From f4336f82a0b3e342d9241d2b2321768204f5f5d7 Mon Sep 17 00:00:00 2001 From: Altynbek Orumbayev Date: Wed, 6 May 2026 13:03:31 +0200 Subject: [PATCH 5/8] refactor(tui): split task_screen.py into focused modules --- src/kagan/tui/screens/_task_chat.py | 439 ++++++++++ src/kagan/tui/screens/_task_review.py | 382 +++++++++ src/kagan/tui/screens/_task_stream.py | 315 +++++++ src/kagan/tui/screens/task_screen.py | 1116 +------------------------ 4 files changed, 1161 insertions(+), 1091 deletions(-) create mode 100644 src/kagan/tui/screens/_task_chat.py create mode 100644 src/kagan/tui/screens/_task_review.py create mode 100644 src/kagan/tui/screens/_task_stream.py diff --git a/src/kagan/tui/screens/_task_chat.py b/src/kagan/tui/screens/_task_chat.py new file mode 100644 index 00000000..120d217a --- /dev/null +++ b/src/kagan/tui/screens/_task_chat.py @@ -0,0 +1,439 @@ +from __future__ import annotations + +import asyncio +import contextlib +from typing import Any, cast + +from textual.widgets import Input + +from kagan.core.enums import ChatMode, SessionKind, StreamSource +from kagan.core.errors import KaganError +from kagan.tui._chat_helpers import ( + TitleGenerationSession, + build_session_options, + kick_title_generation, + send_task_message, +) +from kagan.tui.orchestrator_sessions import is_orchestrator_session_key +from kagan.tui.screens._chat_runner import ( + TASK_REVIEWER_SESSION_KEY, + TASK_WORKER_SESSION_KEY, + send_chat_message, +) +from kagan.tui.widgets.chat import ChatPanel + + +class _TaskChatMixin: + _task_id: str | None + async def action_open_orchestrator_chat(self) -> None: + panel = self._overlay_panel() + was_visible = panel.has_class("visible") + layout_mode = "vertical" if not was_visible else None + self._configure_overlay_chat( + visible=True, + fullscreen=False, + mode=ChatMode.ORCHESTRATOR, + layout_mode=layout_mode, + focus=True, + ) + await self._load_orchestrator_panel_state(panel) + self._sync_overlay_layout_class() + + async def action_open_task_overlay(self) -> None: + panel = self._overlay_panel() + was_visible = panel.has_class("visible") + layout_mode = "vertical" if not was_visible else None + self._configure_overlay_chat( + visible=True, + fullscreen=False, + mode=ChatMode.TASK, + layout_mode=layout_mode, + focus=True, + ) + if self._task_id is not None: + self._ensure_stream_worker() + self._sync_overlay_layout_class() + + async def action_open_task_chat(self) -> None: + panel = self._overlay_panel() + panel.set_visible(True) + panel.set_fullscreen(True) + panel.set_mode_title("Task Chat") + panel.set_session_kind(SessionKind.DETACHED) + panel.set_sessions( + build_session_options(self.kagan_app, self._task_session_options()), + self._active_task_session_key(), + ) + if self._task_id is not None: + panel.set_mode_title(f"Task #{self._task_id[:8]}") + self._ensure_stream_worker() + panel.query_one("#chat-overlay-input", Input).focus() + self._chat_mode = ChatMode.TASK + self._sync_overlay_layout_class() + + async def action_fullscreen_chat(self) -> None: + panel = self._overlay_panel() + if panel.has_class("visible") and panel.has_class("fullscreen"): + panel.set_visible(False) + panel.set_fullscreen(False) + self._overlay_layout_mode = "vertical" + self._sync_overlay_layout_class() + return + if panel.has_class("visible"): + panel.set_fullscreen(True) + panel.query_one("#chat-overlay-input", Input).focus() + self._sync_overlay_layout_class() + return + await self._open_chat_from_current_mode(fullscreen=True) + + async def action_expand_chat_overlay(self) -> None: + panel = self._overlay_panel() + if not panel.has_class("visible"): + return + panel.set_fullscreen(True) + panel.query_one("#chat-overlay-input", Input).focus() + self._sync_overlay_layout_class() + + async def action_toggle_chat(self) -> None: + panel = self._overlay_panel() + if panel.has_class("visible") and panel.has_class("fullscreen"): + panel.set_fullscreen(False) + self._overlay_layout_mode = "vertical" + panel.query_one("#chat-overlay-input", Input).focus() + self._sync_overlay_layout_class() + return + if not panel.has_class("visible"): + self._overlay_layout_mode = "vertical" + await self.action_open_task_overlay() + return + if self._overlay_layout_mode == "vertical": + self._overlay_layout_mode = "horizontal" + panel.query_one("#chat-overlay-input", Input).focus() + self._sync_overlay_layout_class() + return + panel.set_visible(False) + panel.set_fullscreen(False) + self._overlay_layout_mode = "vertical" + self._sync_overlay_layout_class() + + async def _open_chat_from_current_mode(self, *, fullscreen: bool) -> None: + if self._chat_mode == ChatMode.ORCHESTRATOR: + await self.action_open_orchestrator_chat() + if fullscreen: + panel = self._overlay_panel() + panel.set_fullscreen(True) + panel.query_one("#chat-overlay-input", Input).focus() + self._sync_overlay_layout_class() + return + if fullscreen: + await self.action_open_task_chat() + return + await self.action_open_task_overlay() + + @staticmethod + def _sender_id(message: Any) -> str: + sender = cast("Any", getattr(message, "control", getattr(message, "sender", None))) + sender_id = getattr(sender, "id", "") + return sender_id if isinstance(sender_id, str) else "" + + async def on_chat_panel_submit_requested(self, message: ChatPanel.SubmitRequested) -> None: + sender_id = self._sender_id(message) + if sender_id and sender_id != "ts-chat-overlay": + return + + if self._chat_message_task is not None and not self._chat_message_task.done(): + self._chat_message_task.cancel() + with contextlib.suppress(asyncio.CancelledError): + await self._chat_message_task + + if self._chat_mode == ChatMode.ORCHESTRATOR: + self._chat_message_task = asyncio.create_task( + self._send_orchestrator_message(message.text), + name="task-screen-orchestrator-send", + ) + return + + self._chat_message_task = asyncio.create_task( + self._send_task_message(message.text), + name="task-screen-task-send", + ) + + def action_cycle_session(self) -> None: + panel = self._overlay_panel() + fullscreen = panel.has_class("visible") and panel.has_class("fullscreen") + next_mode = ChatMode.ORCHESTRATOR if self._chat_mode == ChatMode.TASK else ChatMode.TASK + self.run_worker( + self._cycle_chat_session(next_mode=next_mode, fullscreen=fullscreen), + exit_on_error=False, + ) + + def action_cycle_chat_session(self) -> None: + self.action_cycle_session() + + async def _cycle_chat_session(self, *, next_mode: str, fullscreen: bool) -> None: + if next_mode == ChatMode.ORCHESTRATOR: + await self.action_open_orchestrator_chat() + elif fullscreen: + await self.action_open_task_chat() + else: + await self.action_open_task_overlay() + + panel = self._overlay_panel() + panel.set_fullscreen(fullscreen) + panel.query_one("#chat-overlay-input", Input).focus() + self._sync_overlay_layout_class() + + def on_chat_panel_session_changed(self, message: ChatPanel.SessionChanged) -> None: + sender_id = self._sender_id(message) + if sender_id and sender_id != "ts-chat-overlay": + return + if is_orchestrator_session_key(message.key): + self._chat_mode = ChatMode.ORCHESTRATOR + self._overlay_panel().set_mode_title("Orchestrator") + self._chat_orchestrator_history = self.kagan_app.orchestrator_sessions.history_for_key( + message.key + ) + self._overlay_panel().hydrate_current_session_history(self._chat_orchestrator_history) + self._chat_session_switch_token += 1 + token = self._chat_session_switch_token + self.run_worker( + self._switch_orchestrator_session(self._overlay_panel(), message.key, token=token), + exit_on_error=False, + ) + self._sync_overlay_layout_class() + return + self._chat_mode = ChatMode.TASK + panel = self._overlay_panel() + panel.set_mode_title( + f"Task #{self._task_id[:8]}" if self._task_id is not None else "Task Chat" + ) + panel.set_session_kind(self._chat_session_kind(message.key)) + self._set_stream_source(self._stream_source_for_session_key(message.key)) + self._ensure_stream_worker() + self._sync_overlay_layout_class() + + def on_chat_panel_new_session_requested( + self, message: ChatPanel.NewSessionRequested + ) -> None: + sender_id = self._sender_id(message) + if sender_id and sender_id != "ts-chat-overlay": + return + panel = self._overlay_panel() + self.run_worker(self._create_new_orchestrator_session(panel), exit_on_error=False) + + def on_chat_panel_session_picker_requested( + self, message: ChatPanel.SessionPickerRequested + ) -> None: + sender_id = self._sender_id(message) + if sender_id and sender_id != "ts-chat-overlay": + return + + panel = self._overlay_panel() + self._open_overlay_session_picker(panel, initial_query=message.initial_query) + + def on_chat_panel_file_picker_requested( + self, message: ChatPanel.FilePickerRequested + ) -> None: + sender_id = self._sender_id(message) + if sender_id and sender_id != "ts-chat-overlay": + return + + panel = self._overlay_panel() + modal = panel.create_file_picker_modal(initial_query=message.initial_query) + self.app.push_screen(modal, callback=panel.handle_file_picker_selected) + + def on_chat_panel_agent_picker_requested( + self, message: ChatPanel.AgentPickerRequested + ) -> None: + sender_id = self._sender_id(message) + if sender_id and sender_id != "ts-chat-overlay": + return + panel = self._overlay_panel() + + def _on_agent_selected(selected: str | None) -> None: + if selected is None: + return + panel._agent_hint = selected + panel.add_system_message(f"Default agent set to {selected}") + + self.app.push_screen("agent-picker-modal", callback=_on_agent_selected) + + def on_chat_panel_close_requested(self, message: ChatPanel.CloseRequested) -> None: + sender_id = self._sender_id(message) + if sender_id == "ts-chat-overlay": + panel = self._overlay_panel() + panel.set_visible(False) + panel.set_fullscreen(False) + self._overlay_layout_mode = "vertical" + self._sync_overlay_layout_class() + return + + def on_chat_panel_interrupt_requested( + self, message: ChatPanel.InterruptRequested + ) -> None: + sender_id = self._sender_id(message) + if sender_id and sender_id != "ts-chat-overlay": + return + panel = self._overlay_panel() + if self._chat_message_task is not None and not self._chat_message_task.done(): + self._chat_message_task.cancel() + panel.post_message(ChatPanel.InterruptCompleted()) + return + + async def _cancel_and_complete() -> None: + await self.action_cancel_run() + panel.post_message(ChatPanel.InterruptCompleted()) + + self.run_worker(_cancel_and_complete(), exit_on_error=False) + + async def _send_orchestrator_message(self, text: str) -> None: + panel = self._overlay_panel() + should_title = self.kagan_app.orchestrator_sessions.should_generate_title() + try: + self._chat_orchestrator_history = await send_chat_message( + core=self.kagan_app.core, + panel=panel, + text=text, + history=self._chat_orchestrator_history, + ) + await self.kagan_app.orchestrator_sessions.persist_active( + history=self._chat_orchestrator_history, + rendered_messages=panel.export_rendered_messages(), + agent_backend=panel.preferred_agent_backend(), + ) + # Auto-generate a session title after the first turn + if should_title and self._chat_orchestrator_history: + asyncio.create_task( + kick_title_generation( + TitleGenerationSession( + orchestrator_sessions=self.kagan_app.orchestrator_sessions, + panel=panel, + user_message=text, + history=self._chat_orchestrator_history, + session_options=build_session_options( + self.kagan_app, + self._task_session_options(), + ), + is_mounted=lambda: self.is_mounted, + ), + self.kagan_app.core, + ), + name="task-chat-title-gen", + ) + except asyncio.CancelledError: + panel.set_runtime_status("ready") + panel.set_stream_action("Waiting for prompt", confidence="certain") + raise + except (KaganError, OSError, RuntimeError, ValueError) as exc: + panel.set_runtime_status("error") + panel.set_stream_action("Orchestrator error", confidence="needs-validation") + panel.add_system_message(f"Orchestrator error: {exc}") + + async def _send_task_message(self, text: str) -> None: + panel = self._overlay_panel() + if self._task_id is None: + panel.add_system_message("No task selected") + return + + await self.action_cancel_run() + + if self._task_model is None: + self._task_model = await self.kagan_app.core.tasks.get(self._task_id) + + task_model = self._task_model + if task_model is None: + panel.add_system_message("Unable to load task") + return + + self._task_model = await send_task_message(self.kagan_app.core, task_model, text) + + panel.set_runtime_status("initializing") + panel.set_stream_action("Restarting task agent...", confidence="assumption") + self._set_stream_source(StreamSource.WORKER) + restart_error = await self._start_or_attach_session( + backend_hint=panel.preferred_agent_backend() + ) + if restart_error is None: + return + panel.set_runtime_status("error") + panel.set_stream_action("Unable to restart task agent", confidence="needs-validation") + panel.add_system_message(f"Unable to restart agent: {restart_error}") + + async def _switch_orchestrator_session( + self, + panel: ChatPanel, + key: str, + *, + token: int | None = None, + ) -> None: + if token is not None and token != self._chat_session_switch_token: + return + await self.kagan_app.orchestrator_sessions.persist_active( + history=self._chat_orchestrator_history, + rendered_messages=panel.export_rendered_messages(), + agent_backend=panel.preferred_agent_backend(), + ) + if token is not None and token != self._chat_session_switch_token: + return + await self._load_orchestrator_panel_state(panel, requested_key=key) + + async def _create_new_orchestrator_session(self, panel: ChatPanel) -> None: + await self.kagan_app.orchestrator_sessions.persist_active( + history=self._chat_orchestrator_history, + rendered_messages=panel.export_rendered_messages(), + agent_backend=panel.preferred_agent_backend(), + ) + next_key = await self.kagan_app.orchestrator_sessions.create_new( + agent_backend=panel.preferred_agent_backend() + ) + await self._load_orchestrator_panel_state(panel, requested_key=next_key) + panel.add_system_message("New session started.") + + async def _load_orchestrator_panel_state( + self, + panel: ChatPanel, + *, + requested_key: str | None = None, + ) -> None: + await self.kagan_app.orchestrator_sessions.ensure_loaded() + if requested_key is not None and is_orchestrator_session_key(requested_key): + self._chat_orchestrator_history = await self.kagan_app.orchestrator_sessions.switch( + requested_key + ) + else: + self._chat_orchestrator_history = self.kagan_app.orchestrator_sessions.active_history() + + active_key = self.kagan_app.orchestrator_sessions.active_key() + panel.set_sessions( + build_session_options(self.kagan_app, self._task_session_options()), + active_key, + ) + panel.hydrate_current_session_history(self._chat_orchestrator_history) + session_backend = self.kagan_app.orchestrator_sessions.agent_backend_for_key(active_key) + if session_backend is not None: + panel.set_preferred_agent_backend(session_backend) + + def _task_session_options(self) -> list[tuple[str, str]]: + return [ + ("Task", TASK_WORKER_SESSION_KEY), + ("Review", TASK_REVIEWER_SESSION_KEY), + ] + + def _active_task_session_key(self) -> str: + source = self._effective_stream_source() + if source == StreamSource.REVIEWER: + return TASK_REVIEWER_SESSION_KEY + return TASK_WORKER_SESSION_KEY + + @staticmethod + def _stream_source_for_session_key(key: str) -> str | None: + normalized = key.casefold() + if "review" in normalized: + return StreamSource.REVIEWER + if "task" in normalized or "worker" in normalized: + return StreamSource.WORKER + return None + + @staticmethod + def _chat_session_kind(key: str) -> str: + return SessionKind.REVIEW if "review" in key.casefold() else SessionKind.DETACHED diff --git a/src/kagan/tui/screens/_task_review.py b/src/kagan/tui/screens/_task_review.py new file mode 100644 index 00000000..54cd4e4d --- /dev/null +++ b/src/kagan/tui/screens/_task_review.py @@ -0,0 +1,382 @@ +from __future__ import annotations + +import asyncio +import contextlib +from typing import TYPE_CHECKING, cast + +from textual.containers import Vertical +from textual.css.query import NoMatches +from textual.widgets import Checkbox, Static + +from kagan.core.enums import StreamSource, TaskStatus +from kagan.core.errors import ( + KaganError, + MergeConflictError, + NotFoundError, + PreflightError, + SessionError, + WorktreeError, +) +from kagan.tui.screens.confirm import ConfirmModal +from kagan.tui.screens.rejection_input import RejectionInputModal +from kagan.tui.widgets.task_detail_pane import TaskDetailPane +from kagan.tui.widgets.task_review_helpers import ( + build_merge_readiness_text, + render_ai_verdict_summary, + render_criteria_checkboxes, +) +from kagan.tui.widgets.task_workspace_helpers import diff_totals + +if TYPE_CHECKING: + from kagan.core.models import Task + + +class _TaskReviewMixin: + def action_approve(self) -> None: + self.run_worker(self._approve_only_flow(), exit_on_error=False) + + async def _approve_only_flow(self) -> None: + if self._task_id is None or self._task_model is None: + return + if self._task_model.status is not TaskStatus.REVIEW: + return + + criteria = [ + c.strip() for c in (self._task_model.acceptance_criteria or []) if c and c.strip() + ] + if not criteria: + from kagan.tui.screens.review_no_criteria import ReviewNoCriteriaModal + + choice = await self.app.push_screen_wait(ReviewNoCriteriaModal()) + if choice == "add_criteria": + await self._edit_task_flow_for_criteria() + return + if choice == "approve_manually": + await self._manual_approve_flow() + return + if choice == "reject": + await self._reject_flow() + return + return + + confirmed = await self.app.push_screen_wait( + ConfirmModal( + title="Approve Task?", + message="Mark this task as approved. You can merge separately.", + confirm_label="Approve", + ) + ) + if confirmed is not True: + return + + await self.kagan_app.core.reviews.approve(self._task_id) + await self._refresh_runtime_state() + self.app.notify("Task approved", severity="information") + + async def _edit_task_flow_for_criteria(self) -> None: + """Open the task editor focused on the acceptance criteria field.""" + if self._task_id is None: + return + if self._task_model is None: + with contextlib.suppress(NotFoundError): + self._task_model = await self.kagan_app.core.tasks.get(self._task_id) + if self._task_model is None: + return + + from kagan.tui.screens.task_editor_modal import TaskEditorModal + + await self.app.push_screen_wait( + TaskEditorModal(task=self._task_model, focus_field="task-acceptance-criteria") + ) + await self._refresh_runtime_state() + + async def _manual_approve_flow(self) -> None: + """Approve a task manually when no acceptance criteria are defined.""" + if self._task_id is None or self._task_model is None: + return + + confirmed = await self.app.push_screen_wait( + ConfirmModal( + title="Manual Approval (No Criteria)", + message=( + "This task has no acceptance criteria.\n" + "Manual approval is an exceptional path \u2014 " + "the review is entirely your judgment.\n\n" + "Are you sure you want to approve?" + ), + confirm_label="Approve Manually", + ) + ) + if confirmed is not True: + return + + await self.kagan_app.core.reviews.approve(self._task_id) + await self._refresh_runtime_state() + self.app.notify("Manually approved (no criteria)", severity="warning") + + def action_merge(self) -> None: + self.run_worker(self._merge_flow(), exit_on_error=False) + + async def _merge_flow(self) -> None: + if self._task_id is None or self._task_model is None: + return + if self._task_model.status is not TaskStatus.REVIEW: + return + if not self._review_approved: + self.app.notify("Approve the task before merging", severity="warning") + return + + confirmed = await self.app.push_screen_wait( + ConfirmModal( + title="Merge Task?", + message="This will merge the task branch and move to DONE.", + confirm_label="Merge", + ) + ) + if confirmed is not True: + self._set_status("Merge cancelled") + return + + try: + await self.kagan_app.core.reviews.merge(self._task_id) + except PreflightError as exc: + self._last_merge_blocker = "Preflight failed \u2192 fix and retry" + self._set_status(str(exc)) + self._output_stream().post_note(str(exc)) + self._sync_merge_readiness() + return + except MergeConflictError as exc: + self._last_merge_blocker = "Merge conflicts \u2192 b to rebase" + message = self._conflict_message(exc.conflict_files, prefix="Merge has conflicts") + self._set_status(message) + self._output_stream().post_note(message) + self._sync_merge_readiness() + return + except WorktreeError as exc: + self._last_merge_blocker = f"Worktree error: {exc}" + message = f"Unable to merge: {exc}" + self._set_status(message) + self._output_stream().post_note(message) + self._sync_merge_readiness() + return + + self._last_merge_blocker = None + self.app.notify("Merged and moved to DONE", severity="information") + self.action_back() + + def action_reject(self) -> None: + self.run_worker(self._reject_flow(), exit_on_error=False) + + async def _reject_flow(self) -> None: + if self._task_id is None or self._task_model is None: + return + if self._task_model.status is not TaskStatus.REVIEW: + return + feedback = await self.app.push_screen_wait( + RejectionInputModal(task_label=f"Task {self._task_id}") + ) + if feedback is None: + self._set_status("Rejection cancelled") + return + move_to_backlog = False + if feedback.startswith(RejectionInputModal.BACKLOG_SENTINEL): + move_to_backlog = True + _, _, feedback = feedback.partition(":") + note = feedback or "Needs more work" + await self.kagan_app.core.reviews.reject(self._task_id, feedback=note) + if move_to_backlog: + await self.kagan_app.core.tasks.set_status(self._task_id, TaskStatus.BACKLOG) + self.app.notify("Moved back to BACKLOG", severity="warning") + else: + self.app.notify("Moved back to IN_PROGRESS", severity="warning") + self.action_back() + + async def action_rebase(self) -> None: + if self._task_id is None or self._task_model is None: + return + if self._task_model.status is not TaskStatus.REVIEW: + return + try: + await self.kagan_app.core.reviews.rebase(self._task_id) + except WorktreeError as exc: + self._last_merge_blocker = "Rebase conflicts \u2192 resolve and retry" + conflicts = await self.kagan_app.core.reviews.conflicts(self._task_id) + conflict_files = cast("list[str]", conflicts.get("conflicted_files", [])) + message = self._conflict_message(conflict_files, prefix=str(exc)) + self._set_status(message) + self._output_stream().post_note(message) + self._sync_merge_readiness() + return + self._last_merge_blocker = None + self.app.notify("Rebase completed", severity="information") + await self._hydrate_workspace_panels() + await self._load_review_context() + + async def action_run_review(self) -> None: + if self._task_id is None or self._task_model is None: + return + criteria = [ + c.strip() for c in (self._task_model.acceptance_criteria or []) if c and c.strip() + ] + if not criteria: + self.app.notify( + "Cannot run AI review \u2014 no acceptance criteria defined", + severity="warning", + ) + return + with contextlib.suppress(KaganError, OSError, RuntimeError): + await self.kagan_app.core.reviews.clear_verdicts(self._task_id) + backend = await self._resolve_backend(self._task_model) + self._reviewer_session_id = None + self._pending_reviewer_session_id = True + await self.kagan_app.core.tasks.run(self._task_id, agent_backend=backend) + self._running = True + self._set_stream_source(StreamSource.REVIEWER) + self._set_status("AI Reviewing...") + self.app.notify("AI review started", severity="information") + self._ensure_stream_worker() + + async def _load_task_or_fail(self) -> Task | None: + if self._task_id is None: + return None + + try: + task = await self.kagan_app.core.tasks.get(self._task_id) + self._task_model = task + self._review_approved = await asyncio.to_thread( + self.kagan_app.core.reviews.is_approved, self._task_id + ) + return task + except (KaganError, OSError, RuntimeError, ValueError) as exc: + self._set_status(f"Unable to load task: {exc}") + return None + + async def _render_task_summary(self, task: Task) -> str: + if self._review_approved: + badge = "APPROVED" + elif task.status is TaskStatus.REVIEW and self._running: + badge = "REVIEWING..." + else: + badge = task.status.value.upper() + current_status = f"Ready | {badge}" + with contextlib.suppress(NoMatches): + self.query_one("#ts-detail-status", Static).update(current_status) + return current_status + + async def _render_criteria_checkboxes(self, task: Task) -> None: + self._review_criteria_signature = await render_criteria_checkboxes( + task=task, + criteria_container=self.query_one("#ts-detail-criteria-list", Vertical), + criteria_status=self.query_one("#ts-detail-criteria-status", Static), + previous_signature=self._review_criteria_signature, + running=self._running, + get_static=lambda selector: self.query_one(selector, Static), + sync_criteria_status=self._sync_criteria_status_widget, + ) + + async def _render_changed_files(self, task: Task) -> None: + try: + diff_text = await self.kagan_app.core.worktrees.diff(self._task_id) + except (SessionError, WorktreeError): + diff_text = "" + if not diff_text: + merged_fallback = await self._resolve_merged_commit_diff_fallback() + if merged_fallback is not None: + diff_text = merged_fallback[0] + + files, insertions, deletions = diff_totals(diff_text) + with contextlib.suppress(NoMatches): + self.query_one("#ts-detail-changes-summary", Static).update( + f"Changes - {files} files +{insertions} -{deletions}" + ) + + if diff_text and task.status is TaskStatus.REVIEW: + with contextlib.suppress(NoMatches): + if self._review_approved: + badge = "APPROVED" + elif self._running: + badge = "REVIEWING..." + else: + badge = task.status.value.upper() + self.query_one("#ts-detail-status", Static).update(f"Ready | {badge}") + + async def _load_review_context(self) -> None: + task = await self._load_task_or_fail() + if task is None: + return + + current_status = await self._render_task_summary(task) + await self._render_criteria_checkboxes(task) + await self._render_changed_files(task) + await self._load_resume_context(task) + self._set_status(current_status) + self._sync_merge_readiness() + + async def _load_resume_context(self, task: Task) -> None: + pane = self.query_one(TaskDetailPane) + if self._task_id is None: + pane.set_resume_context([], task.status) + return + notes: list[str] = [] + with contextlib.suppress(KaganError, OSError, RuntimeError): + entries = await self.kagan_app.core.tasks.list_notes(self._task_id) + notes = [entry.content for entry in entries] + pane.set_resume_context(notes, task.status) + + def _sync_merge_readiness(self) -> None: + try: + widget = self.query_one("#ts-merge-readiness", Static) + except NoMatches: + return + widget.update( + build_merge_readiness_text( + self._task_model, + human_approved=self._review_approved, + last_merge_blocker=self._last_merge_blocker, + ) + ) + + def on_checkbox_changed(self, event: Checkbox.Changed) -> None: + if not event.checkbox.has_class("ts-detail-criterion"): + return + self._sync_criteria_status_widget(self.query_one("#ts-detail-criteria-status", Static)) + + def _sync_criteria_status_widget(self, status: Static) -> None: + checkboxes = [ + node for node in self.query(".ts-detail-criterion") if isinstance(node, Checkbox) + ] + total = len(checkboxes) + if total == 0: + status.update("") + status.remove_class("ts-criteria-complete") + return + checked = sum(1 for checkbox in checkboxes if checkbox.value) + task = self._task_model + ai_summary = "" + ai_state = "" + if task is not None: + ai_summary, ai_state = render_ai_verdict_summary(task, total, running=self._running) + if checked == total: + human_summary = f"All {total} criteria verified" + status.add_class("ts-criteria-complete") + else: + human_summary = f"{checked}/{total} verified" + status.remove_class("ts-criteria-complete") + + if ai_summary: + status.update(f"{human_summary}\n{ai_summary}") + else: + status.update(human_summary) + + status.remove_class("ts-ai-pass", "ts-ai-fail") + status.set_class(ai_state == "pass", "ts-ai-pass") + status.set_class(ai_state == "fail", "ts-ai-fail") + + def _conflict_message( + self, conflict_files: list[str], *, prefix: str = "Rebase has conflicts" + ) -> str: + if not conflict_files: + return prefix + joined = ", ".join(conflict_files[:3]) + more = "" if len(conflict_files) <= 3 else f" (+{len(conflict_files) - 3} more)" + return f"{prefix}: {joined}{more}" diff --git a/src/kagan/tui/screens/_task_stream.py b/src/kagan/tui/screens/_task_stream.py new file mode 100644 index 00000000..fdaf0b33 --- /dev/null +++ b/src/kagan/tui/screens/_task_stream.py @@ -0,0 +1,315 @@ +from __future__ import annotations + +import asyncio +import contextlib +from typing import TYPE_CHECKING, Any + +from textual import on +from textual.css.query import NoMatches +from textual.widgets import TabbedContent + +from kagan.core.enums import TaskStatus +from kagan.core.errors import KaganError +from kagan.tui.screens._chat_runner import ( + acp_payload, + stream_chunk_kind, + stream_chunk_text, + tool_call_args, + tool_call_id, + tool_call_kind, + tool_call_result, + tool_call_status, + tool_call_title, +) +from kagan.tui.widgets.streaming import OutputChunk, StreamingOutput, ToolCallView, UserInputWidget +from kagan.tui.widgets.task_detail_pane import TaskDetailPane +from kagan.tui.widgets.task_event_handler import TaskEventHandler + +if TYPE_CHECKING: + from textual.widget import Widget + + +TASK_SCREEN_REPLAY_EVENT_LIMIT = 400 + + +class _TaskStreamMixin: + def _ensure_stream_worker(self) -> None: + if self._stream_task is not None and not self._stream_task.done(): + return + if self._task_id is None: + return + self._stream_task = asyncio.create_task(self._stream_events(self._task_id)) + + def _maybe_apply_chat_event( + self, overlay_chat: Any, event_type: str, payload: dict[str, Any] + ) -> None: + """Apply chat event if in task chat mode.""" + from kagan.core.enums import ChatMode + from kagan.tui.screens._chat_runner import apply_task_chat_event + + if self._chat_mode == ChatMode.TASK: + apply_task_chat_event(overlay_chat, event_type, payload) + + async def _stream_events(self, task_id: str) -> None: + output = self._output_stream() + overlay_chat = self._overlay_panel() + from kagan.core.enums import ChatMode + + detail_pane = self.query_one(TaskDetailPane) + event_handler = TaskEventHandler( + output=output, + overlay_chat=overlay_chat, + is_task_chat_mode=lambda: self._chat_mode == ChatMode.TASK, + active_session_id=self._active_stream_session_id, + payload_text=self._payload_text, + queue_refresh=self._queue_stream_refresh, + set_running=self._set_running, + set_status=self._set_status, + set_usage=lambda context_used, context_size, cost_amount, cost_currency: ( + detail_pane.agent_status_panel.set_usage_info( + context_used, context_size, cost_amount, cost_currency + ) + ), + ) + self._replay_count = 0 + self._oldest_event_ts = None + try: + async for event in self.kagan_app.core.tasks.events.stream( + task_id, + replay_limit=TASK_SCREEN_REPLAY_EVENT_LIMIT, + ): + self._replay_count += 1 + if self._oldest_event_ts is None and event.created_at: + from kagan.core import utc_iso + + self._oldest_event_ts = utc_iso(event.created_at) + if self._replay_count == TASK_SCREEN_REPLAY_EVENT_LIMIT: + output.show_load_more_bar() + self._track_session_event(event.event_type, event.session_id) + active_session_id = self._active_stream_session_id() + if event.session_id and active_session_id and event.session_id != active_session_id: + continue + payload = event.payload or {} + handler = event_handler.event_handlers.get(event.event_type) + if handler is not None: + handler(payload, event.session_id) + if self._should_refresh_after_event(event.event_type): + self._queue_stream_refresh(workspace=True, review=True) + except asyncio.CancelledError: + raise + except (KaganError, NoMatches, AttributeError, OSError, RuntimeError) as exc: + self._running = False + self._set_status("Failed") + output.post_note(f"Stream ended unexpectedly: {exc}") + + def _set_running(self, running: bool) -> None: + self._running = running + + @staticmethod + def _should_refresh_after_event(event_type: str) -> bool: + return event_type in { + "output_chunk", + "tool_call_start", + "tool_call_update", + "criterion_verdict", + "agent_completed", + "agent_failed", + "task_status_changed", + "auto_review_started", + } + + def _output_stream(self) -> StreamingOutput: + return self._overlay_panel().stream_output() + + def _active_stream_session_id(self) -> str | None: + source = self._effective_stream_source() + if source == "reviewer": + return self._reviewer_session_id + return self._worker_session_id + + def _track_session_event(self, event_type: str, event_session_id: str | None) -> None: + if event_type == "auto_review_started": + self._pending_reviewer_session_id = True + return + if event_session_id is None: + return + if self._pending_reviewer_session_id: + self._reviewer_session_id = event_session_id + self._pending_reviewer_session_id = False + return + if self._effective_stream_source() == "reviewer" and self._reviewer_session_id is None: + self._reviewer_session_id = event_session_id + return + if self._worker_session_id is None: + self._worker_session_id = event_session_id + return + if self._worker_session_id != event_session_id and self._reviewer_session_id is None: + self._reviewer_session_id = event_session_id + + @on(StreamingOutput.LoadMore) + async def _on_load_more(self) -> None: + if self._task_id is None or self._oldest_event_ts is None: + return + output = self._output_stream() + output.hide_load_more_bar() + + older_events = await self.kagan_app.core.tasks.events.list_before( + self._task_id, + before=self._oldest_event_ts, + limit=200, + ) + if not older_events: + return + + if older_events[0].created_at: + from kagan.core import utc_iso + + self._oldest_event_ts = utc_iso(older_events[0].created_at) or self._oldest_event_ts + + active_session_id = self._active_stream_session_id() + widgets: list[Widget] = [] + filtered_events = [ + event + for event in older_events + if not ( + event.session_id and active_session_id and event.session_id != active_session_id + ) + ] + for event in filtered_events: + payload = event.payload or {} + match event.event_type: + case "output_chunk": + text = stream_chunk_text(payload) + kind = stream_chunk_kind(payload) + if text and kind in {"assistant", "thought", "note", "user"}: + if kind == "user": + widgets.append(UserInputWidget(text)) + else: + widgets.append(OutputChunk(text, kind=kind)) + case "tool_call_start": + widgets.append( + ToolCallView( + tool_call_title(payload), + status=tool_call_status(payload, default="running"), + args=tool_call_args(payload), + result=tool_call_result(payload), + tool_id=tool_call_id(payload), + kind=tool_call_kind(payload), + ) + ) + case "agent_completed": + widgets.append(OutputChunk("Agent completed", kind="note")) + case "agent_failed": + widgets.append( + OutputChunk(stream_chunk_text(payload) or "Agent failed", kind="note") + ) + case "task_status_changed": + widgets.append( + OutputChunk( + stream_chunk_text(payload) or "Task status changed", kind="note" + ) + ) + case _: + continue + + if widgets: + output.prepend_widgets(widgets) + + if len(filtered_events) >= 200: + output.show_load_more_bar() + + def _payload_text(self, payload: Any) -> str: + if payload is None: + return "" + if isinstance(payload, str): + return payload + if isinstance(payload, dict): + text = stream_chunk_text(payload) + if text: + return text + nested = acp_payload(payload) + if nested: + if "title" in nested: + return str(nested["title"]) + if "status" in nested and "sessionUpdate" in nested: + return str(nested["status"]) + if "message" in payload: + return str(payload["message"]) + pieces = [f"{key}={value}" for key, value in payload.items()] + return " ".join(pieces) + return str(payload) + + def _schedule_runtime_refresh(self) -> None: + if not self.is_mounted: + return + self._queue_stream_refresh(runtime=True) + + def _queue_stream_refresh( + self, + *, + runtime: bool = False, + workspace: bool = False, + review: bool = False, + ) -> None: + if not self.is_mounted: + return + self._pending_runtime_refresh = self._pending_runtime_refresh or runtime + self._pending_workspace_refresh = self._pending_workspace_refresh or workspace + self._pending_review_refresh = self._pending_review_refresh or review + if self._stream_refresh_timer is not None: + return + self._stream_refresh_timer = self.set_timer(0.12, self._flush_stream_refresh) + + def _flush_stream_refresh(self) -> None: + self._stream_refresh_timer = None + runtime = self._pending_runtime_refresh + workspace = self._pending_workspace_refresh + review = self._pending_review_refresh + self._pending_runtime_refresh = False + self._pending_workspace_refresh = False + self._pending_review_refresh = False + if not self.is_mounted: + return + if runtime: + self.run_worker( + self._refresh_runtime_state, + group="task-screen-runtime-refresh", + exclusive=True, + exit_on_error=False, + ) + if workspace: + self.run_worker( + self._hydrate_workspace_panels, + group="task-screen-hydrate-stream", + exclusive=True, + exit_on_error=False, + ) + if review: + self.run_worker( + self._load_review_context, + group="task-screen-review-hydrate-stream", + exclusive=True, + exit_on_error=False, + ) + + def _maybe_auto_switch_to_review(self) -> None: + if self._user_switched_tab: + return + if self._task_model is not None and self._task_model.status is TaskStatus.REVIEW: + with contextlib.suppress(NoMatches): + tabs = self.query_one("#ts-tabs", TabbedContent) + if getattr(tabs, "active", "") != "review": + tabs.active = "review" + + async def _refresh_runtime_state(self) -> None: + if self._task_id is None: + return + with contextlib.suppress(KaganError): + latest = await self.kagan_app.core.tasks.get(self._task_id) + self._task_model = latest + self._refresh_header() + self._refresh_header_labels() + self._sync_action_bar() + self._maybe_auto_switch_to_review() + await self._load_review_context() + await self._hydrate_workspace_panels() diff --git a/src/kagan/tui/screens/task_screen.py b/src/kagan/tui/screens/task_screen.py index e29705d6..8be9484d 100644 --- a/src/kagan/tui/screens/task_screen.py +++ b/src/kagan/tui/screens/task_screen.py @@ -2,9 +2,8 @@ import asyncio import contextlib -from typing import TYPE_CHECKING, Any, cast +from typing import TYPE_CHECKING, cast -from textual import events, on from textual.containers import Vertical, VerticalScroll from textual.css.query import NoMatches from textual.screen import Screen @@ -29,71 +28,36 @@ ) from kagan.core.errors import ( KaganError, - MergeConflictError, NotFoundError, - PreflightError, - SessionError, - WorktreeError, -) -from kagan.tui._chat_helpers import ( - TitleGenerationSession, - build_session_options, - kick_title_generation, - send_task_message, ) +from kagan.tui._chat_helpers import build_session_options from kagan.tui.keybindings import TASK_SCREEN_BINDINGS -from kagan.tui.orchestrator_sessions import is_orchestrator_session_key -from kagan.tui.screens._chat_runner import ( - TASK_REVIEWER_SESSION_KEY, - TASK_WORKER_SESSION_KEY, - acp_payload, - apply_task_chat_event, - send_chat_message, - stream_chunk_kind, - stream_chunk_text, - tool_call_args, - tool_call_id, - tool_call_kind, - tool_call_result, - tool_call_status, - tool_call_title, -) -from kagan.tui.screens.confirm import ConfirmModal -from kagan.tui.screens.rejection_input import RejectionInputModal +from kagan.tui.screens._task_chat import _TaskChatMixin +from kagan.tui.screens._task_review import _TaskReviewMixin +from kagan.tui.screens._task_stream import _TaskStreamMixin from kagan.tui.screens.task_commands import TaskScreenCommandProvider from kagan.tui.widgets.chat import ChatPanel from kagan.tui.widgets.diff import DiffFileTree from kagan.tui.widgets.header import KaganHeader -from kagan.tui.widgets.streaming import OutputChunk, StreamingOutput, ToolCallView, UserInputWidget from kagan.tui.widgets.task_action_bar import TaskActionBar from kagan.tui.widgets.task_detail_pane import TaskDetailPane from kagan.tui.widgets.task_diff_pane import TaskDiffPane -from kagan.tui.widgets.task_event_handler import TaskEventHandler -from kagan.tui.widgets.task_review_helpers import ( - build_merge_readiness_text, - render_ai_verdict_summary, - render_criteria_checkboxes, -) from kagan.tui.widgets.task_workspace_helpers import ( - diff_totals, hydrate_workspace_panels, merged_commit_diff_fallback, resolve_latest_merge_event, ) if TYPE_CHECKING: + from textual import events from textual.app import ComposeResult from textual.timer import Timer - from textual.widget import Widget from kagan.core.models import Task from kagan.tui.app import KaganApp -TASK_SCREEN_REPLAY_EVENT_LIMIT = 400 - - -class TaskScreen(Screen[None]): +class TaskScreen(_TaskReviewMixin, _TaskStreamMixin, _TaskChatMixin, Screen[None]): BINDINGS = TASK_SCREEN_BINDINGS COMMANDS = {TaskScreenCommandProvider} @@ -416,331 +380,34 @@ async def _delete_task_flow(self) -> None: await self.kagan_app.core.tasks.delete(task.id) self.action_back() - async def action_open_orchestrator_chat(self) -> None: - panel = self._overlay_panel() - was_visible = panel.has_class("visible") - layout_mode = "vertical" if not was_visible else None - self._configure_overlay_chat( - visible=True, - fullscreen=False, - mode=ChatMode.ORCHESTRATOR, - layout_mode=layout_mode, - focus=True, - ) - await self._load_orchestrator_panel_state(panel) - self._sync_overlay_layout_class() - - async def action_open_task_overlay(self) -> None: - panel = self._overlay_panel() - was_visible = panel.has_class("visible") - layout_mode = "vertical" if not was_visible else None - self._configure_overlay_chat( - visible=True, - fullscreen=False, - mode=ChatMode.TASK, - layout_mode=layout_mode, - focus=True, - ) - if self._task_id is not None: - self._ensure_stream_worker() - self._sync_overlay_layout_class() - - async def action_open_task_chat(self) -> None: - panel = self._overlay_panel() - panel.set_visible(True) - panel.set_fullscreen(True) - panel.set_mode_title("Task Chat") - panel.set_session_kind(SessionKind.DETACHED) - panel.set_sessions( - build_session_options(self.kagan_app, self._task_session_options()), - self._active_task_session_key(), - ) - if self._task_id is not None: - panel.set_mode_title(f"Task #{self._task_id[:8]}") - self._ensure_stream_worker() - panel.query_one("#chat-overlay-input", Input).focus() - self._chat_mode = ChatMode.TASK - self._sync_overlay_layout_class() - - async def action_fullscreen_chat(self) -> None: - panel = self._overlay_panel() - if panel.has_class("visible") and panel.has_class("fullscreen"): - panel.set_visible(False) - panel.set_fullscreen(False) - self._overlay_layout_mode = "vertical" - self._sync_overlay_layout_class() - return - if panel.has_class("visible"): - panel.set_fullscreen(True) - panel.query_one("#chat-overlay-input", Input).focus() - self._sync_overlay_layout_class() - return - await self._open_chat_from_current_mode(fullscreen=True) - - async def action_expand_chat_overlay(self) -> None: - panel = self._overlay_panel() - if not panel.has_class("visible"): - return - panel.set_fullscreen(True) - panel.query_one("#chat-overlay-input", Input).focus() - self._sync_overlay_layout_class() + def action_open_repo_picker(self) -> None: + self.run_worker(self._open_repo_picker_flow(), exit_on_error=False) - async def action_toggle_chat(self) -> None: + async def action_switch_session(self) -> None: panel = self._overlay_panel() - if panel.has_class("visible") and panel.has_class("fullscreen"): - panel.set_fullscreen(False) - self._overlay_layout_mode = "vertical" - panel.query_one("#chat-overlay-input", Input).focus() - self._sync_overlay_layout_class() - return if not panel.has_class("visible"): - self._overlay_layout_mode = "vertical" await self.action_open_task_overlay() - return - if self._overlay_layout_mode == "vertical": - self._overlay_layout_mode = "horizontal" - panel.query_one("#chat-overlay-input", Input).focus() - self._sync_overlay_layout_class() - return - panel.set_visible(False) - panel.set_fullscreen(False) - self._overlay_layout_mode = "vertical" - self._sync_overlay_layout_class() - - async def _open_chat_from_current_mode(self, *, fullscreen: bool) -> None: - if self._chat_mode == ChatMode.ORCHESTRATOR: - await self.action_open_orchestrator_chat() - if fullscreen: - panel = self._overlay_panel() - panel.set_fullscreen(True) - panel.query_one("#chat-overlay-input", Input).focus() - self._sync_overlay_layout_class() - return - if fullscreen: - await self.action_open_task_chat() - return - await self.action_open_task_overlay() - - @staticmethod - def _sender_id(message: Any) -> str: - sender = cast("Any", getattr(message, "control", getattr(message, "sender", None))) - sender_id = getattr(sender, "id", "") - return sender_id if isinstance(sender_id, str) else "" - - async def on_chat_panel_submit_requested(self, message: ChatPanel.SubmitRequested) -> None: - sender_id = self._sender_id(message) - if sender_id and sender_id != "ts-chat-overlay": - return - - if self._chat_message_task is not None and not self._chat_message_task.done(): - self._chat_message_task.cancel() - with contextlib.suppress(asyncio.CancelledError): - await self._chat_message_task - - if self._chat_mode == ChatMode.ORCHESTRATOR: - self._chat_message_task = asyncio.create_task( - self._send_orchestrator_message(message.text), - name="task-screen-orchestrator-send", - ) - return - - self._chat_message_task = asyncio.create_task( - self._send_task_message(message.text), - name="task-screen-task-send", - ) - - def action_cycle_session(self) -> None: - panel = self._overlay_panel() - fullscreen = panel.has_class("visible") and panel.has_class("fullscreen") - next_mode = ChatMode.ORCHESTRATOR if self._chat_mode == ChatMode.TASK else ChatMode.TASK - self.run_worker( - self._cycle_chat_session(next_mode=next_mode, fullscreen=fullscreen), - exit_on_error=False, - ) - - def action_cycle_chat_session(self) -> None: - self.action_cycle_session() - - async def _cycle_chat_session(self, *, next_mode: str, fullscreen: bool) -> None: - if next_mode == ChatMode.ORCHESTRATOR: - await self.action_open_orchestrator_chat() - elif fullscreen: - await self.action_open_task_chat() - else: - await self.action_open_task_overlay() - - panel = self._overlay_panel() - panel.set_fullscreen(fullscreen) - panel.query_one("#chat-overlay-input", Input).focus() - self._sync_overlay_layout_class() - - def on_chat_panel_session_changed(self, message: ChatPanel.SessionChanged) -> None: - sender_id = self._sender_id(message) - if sender_id and sender_id != "ts-chat-overlay": - return - if is_orchestrator_session_key(message.key): - self._chat_mode = ChatMode.ORCHESTRATOR - self._overlay_panel().set_mode_title("Orchestrator") - self._chat_orchestrator_history = self.kagan_app.orchestrator_sessions.history_for_key( - message.key - ) - self._overlay_panel().hydrate_current_session_history(self._chat_orchestrator_history) - self._chat_session_switch_token += 1 - token = self._chat_session_switch_token - self.run_worker( - self._switch_orchestrator_session(self._overlay_panel(), message.key, token=token), - exit_on_error=False, - ) - self._sync_overlay_layout_class() - return - self._chat_mode = ChatMode.TASK - panel = self._overlay_panel() - panel.set_mode_title( - f"Task #{self._task_id[:8]}" if self._task_id is not None else "Task Chat" - ) - panel.set_session_kind(self._chat_session_kind(message.key)) - self._set_stream_source(self._stream_source_for_session_key(message.key)) - self._ensure_stream_worker() - self._sync_overlay_layout_class() - - def on_chat_panel_new_session_requested(self, message: ChatPanel.NewSessionRequested) -> None: - sender_id = self._sender_id(message) - if sender_id and sender_id != "ts-chat-overlay": - return - panel = self._overlay_panel() - self.run_worker(self._create_new_orchestrator_session(panel), exit_on_error=False) - - def on_chat_panel_session_picker_requested( - self, message: ChatPanel.SessionPickerRequested - ) -> None: - sender_id = self._sender_id(message) - if sender_id and sender_id != "ts-chat-overlay": - return - - panel = self._overlay_panel() - self._open_overlay_session_picker(panel, initial_query=message.initial_query) - - def on_chat_panel_file_picker_requested(self, message: ChatPanel.FilePickerRequested) -> None: - sender_id = self._sender_id(message) - if sender_id and sender_id != "ts-chat-overlay": - return - - panel = self._overlay_panel() - modal = panel.create_file_picker_modal(initial_query=message.initial_query) - self.app.push_screen(modal, callback=panel.handle_file_picker_selected) - - def on_chat_panel_agent_picker_requested(self, message: ChatPanel.AgentPickerRequested) -> None: - sender_id = self._sender_id(message) - if sender_id and sender_id != "ts-chat-overlay": - return - panel = self._overlay_panel() - - def _on_agent_selected(selected: str | None) -> None: - if selected is None: - return - panel._agent_hint = selected - panel.add_system_message(f"Default agent set to {selected}") - - self.app.push_screen("agent-picker-modal", callback=_on_agent_selected) - - def on_chat_panel_close_requested(self, message: ChatPanel.CloseRequested) -> None: - sender_id = self._sender_id(message) - if sender_id == "ts-chat-overlay": panel = self._overlay_panel() - panel.set_visible(False) - panel.set_fullscreen(False) - self._overlay_layout_mode = "vertical" - self._sync_overlay_layout_class() - return - - def on_chat_panel_interrupt_requested(self, message: ChatPanel.InterruptRequested) -> None: - sender_id = self._sender_id(message) - if sender_id and sender_id != "ts-chat-overlay": - return - panel = self._overlay_panel() - if self._chat_message_task is not None and not self._chat_message_task.done(): - self._chat_message_task.cancel() - panel.post_message(ChatPanel.InterruptCompleted()) - return - - async def _cancel_and_complete() -> None: - await self.action_cancel_run() - panel.post_message(ChatPanel.InterruptCompleted()) - - self.run_worker(_cancel_and_complete(), exit_on_error=False) - - async def _send_orchestrator_message(self, text: str) -> None: - panel = self._overlay_panel() - should_title = self.kagan_app.orchestrator_sessions.should_generate_title() - try: - self._chat_orchestrator_history = await send_chat_message( - core=self.kagan_app.core, - panel=panel, - text=text, - history=self._chat_orchestrator_history, - ) - await self.kagan_app.orchestrator_sessions.persist_active( - history=self._chat_orchestrator_history, - rendered_messages=panel.export_rendered_messages(), - agent_backend=panel.preferred_agent_backend(), - ) - # Auto-generate a session title after the first turn - if should_title and self._chat_orchestrator_history: - asyncio.create_task( - kick_title_generation( - TitleGenerationSession( - orchestrator_sessions=self.kagan_app.orchestrator_sessions, - panel=panel, - user_message=text, - history=self._chat_orchestrator_history, - session_options=build_session_options( - self.kagan_app, - self._task_session_options(), - ), - is_mounted=lambda: self.is_mounted, - ), - self.kagan_app.core, - ), - name="task-chat-title-gen", - ) - except asyncio.CancelledError: - panel.set_runtime_status("ready") - panel.set_stream_action("Waiting for prompt", confidence="certain") - raise - except (KaganError, OSError, RuntimeError, ValueError) as exc: - panel.set_runtime_status("error") - panel.set_stream_action("Orchestrator error", confidence="needs-validation") - panel.add_system_message(f"Orchestrator error: {exc}") - - async def _send_task_message(self, text: str) -> None: - panel = self._overlay_panel() - if self._task_id is None: - panel.add_system_message("No task selected") - return + self._open_overlay_session_picker(panel) - await self.action_cancel_run() + async def action_open_session_picker(self) -> None: + await self.action_switch_session() - if self._task_model is None: - self._task_model = await self.kagan_app.core.tasks.get(self._task_id) + def _open_overlay_session_picker(self, panel: ChatPanel, *, initial_query: str = "") -> None: + modal = panel.create_session_picker_modal(initial_query=initial_query) - task_model = self._task_model - if task_model is None: - panel.add_system_message("Unable to load task") - return + def _on_select(selected_key: str | None) -> None: + if selected_key is None: + return + selector = panel.query_one("#chat-overlay-session-select", Select) + selector.value = selected_key - self._task_model = await send_task_message(self.kagan_app.core, task_model, text) + self.app.push_screen(modal, callback=_on_select) - panel.set_runtime_status("initializing") - panel.set_stream_action("Restarting task agent...", confidence="assumption") - self._set_stream_source(StreamSource.WORKER) - restart_error = await self._start_or_attach_session( - backend_hint=panel.preferred_agent_backend() - ) - if restart_error is None: - return - panel.set_runtime_status("error") - panel.set_stream_action("Unable to restart task agent", confidence="needs-validation") - panel.add_system_message(f"Unable to restart agent: {restart_error}") + async def _open_repo_picker_flow(self) -> None: + await self.app.push_screen_wait("repo-picker-modal") + await self._hydrate_workspace_panels() + await self._load_review_context() async def _start_or_attach_session(self, *, backend_hint: str | None = None) -> str | None: if self._effective_stream_source() == "worker": @@ -821,207 +488,6 @@ async def _task_repo_path(self, project_id: str) -> tuple[str | None, str]: return None, "main" return repo.path, repo.default_branch - def _ensure_stream_worker(self) -> None: - if self._stream_task is not None and not self._stream_task.done(): - return - if self._task_id is None: - return - self._stream_task = asyncio.create_task(self._stream_events(self._task_id)) - - def _maybe_apply_chat_event( - self, overlay_chat: ChatPanel, event_type: str, payload: dict[str, Any] - ) -> None: - """Apply chat event if in task chat mode.""" - if self._chat_mode == ChatMode.TASK: - apply_task_chat_event(overlay_chat, event_type, payload) - - async def _stream_events(self, task_id: str) -> None: - output = self._output_stream() - overlay_chat = self._overlay_panel() - detail_pane = self.query_one(TaskDetailPane) - event_handler = TaskEventHandler( - output=output, - overlay_chat=overlay_chat, - is_task_chat_mode=lambda: self._chat_mode == ChatMode.TASK, - active_session_id=self._active_stream_session_id, - payload_text=self._payload_text, - queue_refresh=self._queue_stream_refresh, - set_running=self._set_running, - set_status=self._set_status, - set_usage=lambda context_used, context_size, cost_amount, cost_currency: ( - detail_pane.agent_status_panel.set_usage_info( - context_used, context_size, cost_amount, cost_currency - ) - ), - ) - self._replay_count = 0 - self._oldest_event_ts = None - try: - async for event in self.kagan_app.core.tasks.events.stream( - task_id, - replay_limit=TASK_SCREEN_REPLAY_EVENT_LIMIT, - ): - self._replay_count += 1 - if self._oldest_event_ts is None and event.created_at: - from kagan.core import utc_iso - - self._oldest_event_ts = utc_iso(event.created_at) - if self._replay_count == TASK_SCREEN_REPLAY_EVENT_LIMIT: - output.show_load_more_bar() - self._track_session_event(event.event_type, event.session_id) - active_session_id = self._active_stream_session_id() - if event.session_id and active_session_id and event.session_id != active_session_id: - continue - payload = event.payload or {} - handler = event_handler.event_handlers.get(event.event_type) - if handler is not None: - handler(payload, event.session_id) - if self._should_refresh_after_event(event.event_type): - self._queue_stream_refresh(workspace=True, review=True) - except asyncio.CancelledError: - raise - except (KaganError, NoMatches, AttributeError, OSError, RuntimeError) as exc: - self._running = False - self._set_status("Failed") - output.post_note(f"Stream ended unexpectedly: {exc}") - - def _set_running(self, running: bool) -> None: - self._running = running - - @staticmethod - def _should_refresh_after_event(event_type: str) -> bool: - return event_type in { - "output_chunk", - "tool_call_start", - "tool_call_update", - "criterion_verdict", - "agent_completed", - "agent_failed", - "task_status_changed", - "auto_review_started", - } - - def _output_stream(self) -> StreamingOutput: - return self._overlay_panel().stream_output() - - def _active_stream_session_id(self) -> str | None: - source = self._effective_stream_source() - if source == "reviewer": - return self._reviewer_session_id - return self._worker_session_id - - def _track_session_event(self, event_type: str, event_session_id: str | None) -> None: - if event_type == "auto_review_started": - self._pending_reviewer_session_id = True - return - if event_session_id is None: - return - if self._pending_reviewer_session_id: - self._reviewer_session_id = event_session_id - self._pending_reviewer_session_id = False - return - if self._effective_stream_source() == "reviewer" and self._reviewer_session_id is None: - self._reviewer_session_id = event_session_id - return - if self._worker_session_id is None: - self._worker_session_id = event_session_id - return - if self._worker_session_id != event_session_id and self._reviewer_session_id is None: - self._reviewer_session_id = event_session_id - - @on(StreamingOutput.LoadMore) - async def _on_load_more(self) -> None: - if self._task_id is None or self._oldest_event_ts is None: - return - output = self._output_stream() - output.hide_load_more_bar() - - older_events = await self.kagan_app.core.tasks.events.list_before( - self._task_id, - before=self._oldest_event_ts, - limit=200, - ) - if not older_events: - return - - if older_events[0].created_at: - from kagan.core import utc_iso - - self._oldest_event_ts = utc_iso(older_events[0].created_at) or self._oldest_event_ts - - active_session_id = self._active_stream_session_id() - widgets: list[Widget] = [] - filtered_events = [ - event - for event in older_events - if not ( - event.session_id and active_session_id and event.session_id != active_session_id - ) - ] - for event in filtered_events: - payload = event.payload or {} - match event.event_type: - case "output_chunk": - text = stream_chunk_text(payload) - kind = stream_chunk_kind(payload) - if text and kind in {"assistant", "thought", "note", "user"}: - if kind == "user": - widgets.append(UserInputWidget(text)) - else: - widgets.append(OutputChunk(text, kind=kind)) - case "tool_call_start": - widgets.append( - ToolCallView( - tool_call_title(payload), - status=tool_call_status(payload, default="running"), - args=tool_call_args(payload), - result=tool_call_result(payload), - tool_id=tool_call_id(payload), - kind=tool_call_kind(payload), - ) - ) - case "agent_completed": - widgets.append(OutputChunk("Agent completed", kind="note")) - case "agent_failed": - widgets.append( - OutputChunk(stream_chunk_text(payload) or "Agent failed", kind="note") - ) - case "task_status_changed": - widgets.append( - OutputChunk( - stream_chunk_text(payload) or "Task status changed", kind="note" - ) - ) - case _: - continue - - if widgets: - output.prepend_widgets(widgets) - - if len(filtered_events) >= 200: - output.show_load_more_bar() - - def _payload_text(self, payload: Any) -> str: - if payload is None: - return "" - if isinstance(payload, str): - return payload - if isinstance(payload, dict): - text = stream_chunk_text(payload) - if text: - return text - nested = acp_payload(payload) - if nested: - if "title" in nested: - return str(nested["title"]) - if "status" in nested and "sessionUpdate" in nested: - return str(nested["status"]) - if "message" in payload: - return str(payload["message"]) - pieces = [f"{key}={value}" for key, value in payload.items()] - return " ".join(pieces) - return str(payload) - def _set_status(self, value: str) -> None: self._status_override = value self.query_one("#ts-status", Static).update(value) @@ -1131,450 +597,6 @@ async def _refresh_header_context(self) -> None: if branch: header.update_branch(branch) - def _schedule_runtime_refresh(self) -> None: - if not self.is_mounted: - return - self._queue_stream_refresh(runtime=True) - - def _queue_stream_refresh( - self, - *, - runtime: bool = False, - workspace: bool = False, - review: bool = False, - ) -> None: - if not self.is_mounted: - return - self._pending_runtime_refresh = self._pending_runtime_refresh or runtime - self._pending_workspace_refresh = self._pending_workspace_refresh or workspace - self._pending_review_refresh = self._pending_review_refresh or review - if self._stream_refresh_timer is not None: - return - self._stream_refresh_timer = self.set_timer(0.12, self._flush_stream_refresh) - - def _flush_stream_refresh(self) -> None: - self._stream_refresh_timer = None - runtime = self._pending_runtime_refresh - workspace = self._pending_workspace_refresh - review = self._pending_review_refresh - self._pending_runtime_refresh = False - self._pending_workspace_refresh = False - self._pending_review_refresh = False - if not self.is_mounted: - return - if runtime: - self.run_worker( - self._refresh_runtime_state, - group="task-screen-runtime-refresh", - exclusive=True, - exit_on_error=False, - ) - if workspace: - self.run_worker( - self._hydrate_workspace_panels, - group="task-screen-hydrate-stream", - exclusive=True, - exit_on_error=False, - ) - if review: - self.run_worker( - self._load_review_context, - group="task-screen-review-hydrate-stream", - exclusive=True, - exit_on_error=False, - ) - - def _maybe_auto_switch_to_review(self) -> None: - if self._user_switched_tab: - return - if self._task_model is not None and self._task_model.status is TaskStatus.REVIEW: - with contextlib.suppress(NoMatches): - tabs = self.query_one("#ts-tabs", TabbedContent) - if getattr(tabs, "active", "") != "review": - tabs.active = "review" - - async def _refresh_runtime_state(self) -> None: - if self._task_id is None: - return - with contextlib.suppress(KaganError): - latest = await self.kagan_app.core.tasks.get(self._task_id) - self._task_model = latest - self._refresh_header() - self._refresh_header_labels() - self._sync_action_bar() - self._maybe_auto_switch_to_review() - await self._load_review_context() - await self._hydrate_workspace_panels() - - def action_approve(self) -> None: - self.run_worker(self._approve_only_flow(), exit_on_error=False) - - async def _approve_only_flow(self) -> None: - if self._task_id is None or self._task_model is None: - return - if self._task_model.status is not TaskStatus.REVIEW: - return - - criteria = [ - c.strip() for c in (self._task_model.acceptance_criteria or []) if c and c.strip() - ] - if not criteria: - from kagan.tui.screens.review_no_criteria import ReviewNoCriteriaModal - - choice = await self.app.push_screen_wait(ReviewNoCriteriaModal()) - if choice == "add_criteria": - await self._edit_task_flow_for_criteria() - return - if choice == "approve_manually": - await self._manual_approve_flow() - return - if choice == "reject": - await self._reject_flow() - return - return - - confirmed = await self.app.push_screen_wait( - ConfirmModal( - title="Approve Task?", - message="Mark this task as approved. You can merge separately.", - confirm_label="Approve", - ) - ) - if confirmed is not True: - return - - await self.kagan_app.core.reviews.approve(self._task_id) - await self._refresh_runtime_state() - self.app.notify("Task approved", severity="information") - - async def _edit_task_flow_for_criteria(self) -> None: - """Open the task editor focused on the acceptance criteria field.""" - if self._task_id is None: - return - if self._task_model is None: - with contextlib.suppress(NotFoundError): - self._task_model = await self.kagan_app.core.tasks.get(self._task_id) - if self._task_model is None: - return - - from kagan.tui.screens.task_editor_modal import TaskEditorModal - - await self.app.push_screen_wait( - TaskEditorModal(task=self._task_model, focus_field="task-acceptance-criteria") - ) - await self._refresh_runtime_state() - - async def _manual_approve_flow(self) -> None: - """Approve a task manually when no acceptance criteria are defined.""" - if self._task_id is None or self._task_model is None: - return - - confirmed = await self.app.push_screen_wait( - ConfirmModal( - title="Manual Approval (No Criteria)", - message=( - "This task has no acceptance criteria.\n" - "Manual approval is an exceptional path \u2014 " - "the review is entirely your judgment.\n\n" - "Are you sure you want to approve?" - ), - confirm_label="Approve Manually", - ) - ) - if confirmed is not True: - return - - await self.kagan_app.core.reviews.approve(self._task_id) - await self._refresh_runtime_state() - self.app.notify("Manually approved (no criteria)", severity="warning") - - def action_merge(self) -> None: - self.run_worker(self._merge_flow(), exit_on_error=False) - - async def _merge_flow(self) -> None: - if self._task_id is None or self._task_model is None: - return - if self._task_model.status is not TaskStatus.REVIEW: - return - if not self._review_approved: - self.app.notify("Approve the task before merging", severity="warning") - return - - confirmed = await self.app.push_screen_wait( - ConfirmModal( - title="Merge Task?", - message="This will merge the task branch and move to DONE.", - confirm_label="Merge", - ) - ) - if confirmed is not True: - self._set_status("Merge cancelled") - return - - try: - await self.kagan_app.core.reviews.merge(self._task_id) - except PreflightError as exc: - self._last_merge_blocker = "Preflight failed → fix and retry" - self._set_status(str(exc)) - self._output_stream().post_note(str(exc)) - self._sync_merge_readiness() - return - except MergeConflictError as exc: - self._last_merge_blocker = "Merge conflicts → b to rebase" - message = self._conflict_message(exc.conflict_files, prefix="Merge has conflicts") - self._set_status(message) - self._output_stream().post_note(message) - self._sync_merge_readiness() - return - except WorktreeError as exc: - self._last_merge_blocker = f"Worktree error: {exc}" - message = f"Unable to merge: {exc}" - self._set_status(message) - self._output_stream().post_note(message) - self._sync_merge_readiness() - return - - self._last_merge_blocker = None - self.app.notify("Merged and moved to DONE", severity="information") - self.action_back() - - def action_reject(self) -> None: - self.run_worker(self._reject_flow(), exit_on_error=False) - - async def _reject_flow(self) -> None: - if self._task_id is None or self._task_model is None: - return - if self._task_model.status is not TaskStatus.REVIEW: - return - feedback = await self.app.push_screen_wait( - RejectionInputModal(task_label=f"Task {self._task_id}") - ) - if feedback is None: - self._set_status("Rejection cancelled") - return - move_to_backlog = False - if feedback.startswith(RejectionInputModal.BACKLOG_SENTINEL): - move_to_backlog = True - _, _, feedback = feedback.partition(":") - note = feedback or "Needs more work" - await self.kagan_app.core.reviews.reject(self._task_id, feedback=note) - if move_to_backlog: - await self.kagan_app.core.tasks.set_status(self._task_id, TaskStatus.BACKLOG) - self.app.notify("Moved back to BACKLOG", severity="warning") - else: - self.app.notify("Moved back to IN_PROGRESS", severity="warning") - self.action_back() - - async def action_rebase(self) -> None: - if self._task_id is None or self._task_model is None: - return - if self._task_model.status is not TaskStatus.REVIEW: - return - try: - await self.kagan_app.core.reviews.rebase(self._task_id) - except WorktreeError as exc: - self._last_merge_blocker = "Rebase conflicts → resolve and retry" - conflicts = await self.kagan_app.core.reviews.conflicts(self._task_id) - conflict_files = cast("list[str]", conflicts.get("conflicted_files", [])) - message = self._conflict_message(conflict_files, prefix=str(exc)) - self._set_status(message) - self._output_stream().post_note(message) - self._sync_merge_readiness() - return - self._last_merge_blocker = None - self.app.notify("Rebase completed", severity="information") - await self._hydrate_workspace_panels() - await self._load_review_context() - - async def action_run_review(self) -> None: - if self._task_id is None or self._task_model is None: - return - criteria = [ - c.strip() for c in (self._task_model.acceptance_criteria or []) if c and c.strip() - ] - if not criteria: - self.app.notify( - "Cannot run AI review — no acceptance criteria defined", - severity="warning", - ) - return - with contextlib.suppress(KaganError, OSError, RuntimeError): - await self.kagan_app.core.reviews.clear_verdicts(self._task_id) - backend = await self._resolve_backend(self._task_model) - self._reviewer_session_id = None - self._pending_reviewer_session_id = True - await self.kagan_app.core.tasks.run(self._task_id, agent_backend=backend) - self._running = True - self._set_stream_source(StreamSource.REVIEWER) - self._set_status("AI Reviewing...") - self.app.notify("AI review started", severity="information") - self._ensure_stream_worker() - - def action_open_repo_picker(self) -> None: - self.run_worker(self._open_repo_picker_flow(), exit_on_error=False) - - async def action_switch_session(self) -> None: - panel = self._overlay_panel() - if not panel.has_class("visible"): - await self.action_open_task_overlay() - panel = self._overlay_panel() - self._open_overlay_session_picker(panel) - - async def action_open_session_picker(self) -> None: - await self.action_switch_session() - - def _open_overlay_session_picker(self, panel: ChatPanel, *, initial_query: str = "") -> None: - modal = panel.create_session_picker_modal(initial_query=initial_query) - - def _on_select(selected_key: str | None) -> None: - if selected_key is None: - return - selector = panel.query_one("#chat-overlay-session-select", Select) - selector.value = selected_key - - self.app.push_screen(modal, callback=_on_select) - - async def _open_repo_picker_flow(self) -> None: - await self.app.push_screen_wait("repo-picker-modal") - await self._hydrate_workspace_panels() - await self._load_review_context() - - async def _load_task_or_fail(self) -> Task | None: - if self._task_id is None: - return None - - try: - task = await self.kagan_app.core.tasks.get(self._task_id) - self._task_model = task - self._review_approved = await asyncio.to_thread( - self.kagan_app.core.reviews.is_approved, self._task_id - ) - return task - except (KaganError, OSError, RuntimeError, ValueError) as exc: - self._set_status(f"Unable to load task: {exc}") - return None - - async def _render_task_summary(self, task: Task) -> str: - if self._review_approved: - badge = "APPROVED" - elif task.status is TaskStatus.REVIEW and self._running: - badge = "REVIEWING..." - else: - badge = task.status.value.upper() - current_status = f"Ready | {badge}" - with contextlib.suppress(NoMatches): - self.query_one("#ts-detail-status", Static).update(current_status) - return current_status - - async def _render_criteria_checkboxes(self, task: Task) -> None: - self._review_criteria_signature = await render_criteria_checkboxes( - task=task, - criteria_container=self.query_one("#ts-detail-criteria-list", Vertical), - criteria_status=self.query_one("#ts-detail-criteria-status", Static), - previous_signature=self._review_criteria_signature, - running=self._running, - get_static=lambda selector: self.query_one(selector, Static), - sync_criteria_status=self._sync_criteria_status_widget, - ) - - async def _render_changed_files(self, task: Task) -> None: - try: - diff_text = await self.kagan_app.core.worktrees.diff(self._task_id) - except (SessionError, WorktreeError): - diff_text = "" - if not diff_text: - merged_fallback = await self._resolve_merged_commit_diff_fallback() - if merged_fallback is not None: - diff_text = merged_fallback[0] - - files, insertions, deletions = diff_totals(diff_text) - with contextlib.suppress(NoMatches): - self.query_one("#ts-detail-changes-summary", Static).update( - f"Changes - {files} files +{insertions} -{deletions}" - ) - - if diff_text and task.status is TaskStatus.REVIEW: - with contextlib.suppress(NoMatches): - if self._review_approved: - badge = "APPROVED" - elif self._running: - badge = "REVIEWING..." - else: - badge = task.status.value.upper() - self.query_one("#ts-detail-status", Static).update(f"Ready | {badge}") - - async def _load_review_context(self) -> None: - task = await self._load_task_or_fail() - if task is None: - return - - current_status = await self._render_task_summary(task) - await self._render_criteria_checkboxes(task) - await self._render_changed_files(task) - await self._load_resume_context(task) - self._set_status(current_status) - self._sync_merge_readiness() - - async def _load_resume_context(self, task: Task) -> None: - pane = self.query_one(TaskDetailPane) - if self._task_id is None: - pane.set_resume_context([], task.status) - return - notes: list[str] = [] - with contextlib.suppress(KaganError, OSError, RuntimeError): - entries = await self.kagan_app.core.tasks.list_notes(self._task_id) - notes = [entry.content for entry in entries] - pane.set_resume_context(notes, task.status) - - def _sync_merge_readiness(self) -> None: - try: - widget = self.query_one("#ts-merge-readiness", Static) - except NoMatches: - return - widget.update( - build_merge_readiness_text( - self._task_model, - human_approved=self._review_approved, - last_merge_blocker=self._last_merge_blocker, - ) - ) - - def on_checkbox_changed(self, event: Checkbox.Changed) -> None: - if not event.checkbox.has_class("ts-detail-criterion"): - return - self._sync_criteria_status_widget(self.query_one("#ts-detail-criteria-status", Static)) - - def _sync_criteria_status_widget(self, status: Static) -> None: - checkboxes = [ - node for node in self.query(".ts-detail-criterion") if isinstance(node, Checkbox) - ] - total = len(checkboxes) - if total == 0: - status.update("") - status.remove_class("ts-criteria-complete") - return - checked = sum(1 for checkbox in checkboxes if checkbox.value) - task = self._task_model - ai_summary = "" - ai_state = "" - if task is not None: - ai_summary, ai_state = render_ai_verdict_summary(task, total, running=self._running) - if checked == total: - human_summary = f"All {total} criteria verified" - status.add_class("ts-criteria-complete") - else: - human_summary = f"{checked}/{total} verified" - status.remove_class("ts-criteria-complete") - - if ai_summary: - status.update(f"{human_summary}\n{ai_summary}") - else: - status.update(human_summary) - - status.remove_class("ts-ai-pass", "ts-ai-fail") - status.set_class(ai_state == "pass", "ts-ai-pass") - status.set_class(ai_state == "fail", "ts-ai-fail") - def on_diff_file_tree_file_selected(self, message: DiffFileTree.FileSelected) -> None: if message.entry is None: return @@ -1685,15 +707,6 @@ def _sync_overlay_layout_class(self) -> None: ) self._sync_action_bar() - def _conflict_message( - self, conflict_files: list[str], *, prefix: str = "Rebase has conflicts" - ) -> str: - if not conflict_files: - return prefix - joined = ", ".join(conflict_files[:3]) - more = "" if len(conflict_files) <= 3 else f" (+{len(conflict_files) - 3} more)" - return f"{prefix}: {joined}{more}" - def _active_tab(self) -> str: with contextlib.suppress(NoMatches, AttributeError): tabs = self.query_one("#ts-tabs", TabbedContent) @@ -1743,84 +756,5 @@ def _configure_overlay_chat( self._chat_mode = mode - async def _switch_orchestrator_session( - self, - panel: ChatPanel, - key: str, - *, - token: int | None = None, - ) -> None: - if token is not None and token != self._chat_session_switch_token: - return - await self.kagan_app.orchestrator_sessions.persist_active( - history=self._chat_orchestrator_history, - rendered_messages=panel.export_rendered_messages(), - agent_backend=panel.preferred_agent_backend(), - ) - if token is not None and token != self._chat_session_switch_token: - return - await self._load_orchestrator_panel_state(panel, requested_key=key) - - async def _create_new_orchestrator_session(self, panel: ChatPanel) -> None: - await self.kagan_app.orchestrator_sessions.persist_active( - history=self._chat_orchestrator_history, - rendered_messages=panel.export_rendered_messages(), - agent_backend=panel.preferred_agent_backend(), - ) - next_key = await self.kagan_app.orchestrator_sessions.create_new( - agent_backend=panel.preferred_agent_backend() - ) - await self._load_orchestrator_panel_state(panel, requested_key=next_key) - panel.add_system_message("New session started.") - - async def _load_orchestrator_panel_state( - self, - panel: ChatPanel, - *, - requested_key: str | None = None, - ) -> None: - await self.kagan_app.orchestrator_sessions.ensure_loaded() - if requested_key is not None and is_orchestrator_session_key(requested_key): - self._chat_orchestrator_history = await self.kagan_app.orchestrator_sessions.switch( - requested_key - ) - else: - self._chat_orchestrator_history = self.kagan_app.orchestrator_sessions.active_history() - - active_key = self.kagan_app.orchestrator_sessions.active_key() - panel.set_sessions( - build_session_options(self.kagan_app, self._task_session_options()), - active_key, - ) - panel.hydrate_current_session_history(self._chat_orchestrator_history) - session_backend = self.kagan_app.orchestrator_sessions.agent_backend_for_key(active_key) - if session_backend is not None: - panel.set_preferred_agent_backend(session_backend) - - def _task_session_options(self) -> list[tuple[str, str]]: - return [ - ("Task", TASK_WORKER_SESSION_KEY), - ("Review", TASK_REVIEWER_SESSION_KEY), - ] - - def _active_task_session_key(self) -> str: - source = self._effective_stream_source() - if source == StreamSource.REVIEWER: - return TASK_REVIEWER_SESSION_KEY - return TASK_WORKER_SESSION_KEY - - @staticmethod - def _stream_source_for_session_key(key: str) -> str | None: - normalized = key.casefold() - if "review" in normalized: - return StreamSource.REVIEWER - if "task" in normalized or "worker" in normalized: - return StreamSource.WORKER - return None - - @staticmethod - def _chat_session_kind(key: str) -> str: - return SessionKind.REVIEW if "review" in key.casefold() else SessionKind.DETACHED - def _overlay_panel(self) -> ChatPanel: return self.query_one("#ts-chat-overlay", ChatPanel) From 69bd0c1d92af96390a7e0090268c12604c96c5ca Mon Sep 17 00:00:00 2001 From: Altynbek Orumbayev Date: Wed, 6 May 2026 16:14:06 +0200 Subject: [PATCH 6/8] refactor: drop dead acp aliases and align test layout with testing.md - delete _acp_handshake_timeout_seconds and _friendly_acp_error_message aliases in cli/chat/acp.py; call the public functions directly - relocate 15 unit tests from tests/core/unit/ into tests/unit/core/ and 4 widget/screen unit tests from tests/tui/ into tests/unit/tui/, the layout testing.md prescribes - normalize markers so every unit-test file carries pytest.mark.unit - pull RateLimitMiddleware._safe_log_key contract into tests/unit/server/test_middleware.py and macOS env-sanitization into tests/unit/cli/test_env_sanitization.py instead of leaking unit-shaped assertions into behavioral suites - drop tests/core/unit/test_transitions.py: it asserted on locally defined helpers, the real state machine is covered by the 397-line tests/unit/core/test_transitions.py - replace asyncio.sleep polling in test_chat_first_turn_invokes_title_generator with an asyncio.Event set by the title generator plus a yield-only loop - extend friendly_acp_error_message coverage with a parametrized classification test (rate limit, quota, network, overloaded) Co-Authored-By: Claude Opus 4.7 (1M context) --- src/kagan/cli/chat/acp.py | 10 +- tests/core/test_audit_cleanup.py | 15 --- tests/core/test_chat_lifecycle.py | 11 +- tests/core/test_cli_surface.py | 18 +--- tests/core/unit/test_transitions.py | 100 ------------------ tests/unit/cli/test_env_sanitization.py | 26 +++++ .../unit => unit/core}/test_analytics.py | 2 +- .../core}/test_attachment_model.py | 0 .../unit => unit/core}/test_backend_spec.py | 0 .../unit => unit/core}/test_cascade_delete.py | 2 +- .../core}/test_doctor_telemetry.py | 0 .../unit => unit/core}/test_launchers.py | 0 .../core}/test_long_lived_acp_factory.py | 2 +- .../unit => unit/core}/test_orphan_reap.py | 2 +- .../core}/test_pi_rpc_messages.py | 0 .../core}/test_pi_rpc_translator.py | 0 .../core}/test_preflight_backends.py | 0 .../core}/test_settlement_rule.py | 2 +- .../core/test_spawn_per_turn_acp_factory.py | 2 + .../core}/test_subprocess_resolve.py | 0 tests/unit/core/test_task_classification.py | 4 + .../core}/test_verdict_roundtrip.py | 2 +- .../{core/unit => unit/core}/test_watcher.py | 0 tests/unit/server/test_middleware.py | 13 ++- tests/unit/test_acp_timeout_config.py | 45 ++++++-- tests/unit/tui/__init__.py | 0 tests/{ => unit}/tui/test_chat_runner.py | 2 + .../tui/test_chat_turn_interrupted.py | 0 .../{ => unit}/tui/test_mention_typeahead.py | 2 +- .../tui/test_mention_typeahead_wired.py | 2 +- 30 files changed, 103 insertions(+), 159 deletions(-) delete mode 100644 tests/core/unit/test_transitions.py create mode 100644 tests/unit/cli/test_env_sanitization.py rename tests/{core/unit => unit/core}/test_analytics.py (99%) rename tests/{core/unit => unit/core}/test_attachment_model.py (100%) rename tests/{core/unit => unit/core}/test_backend_spec.py (100%) rename tests/{core/unit => unit/core}/test_cascade_delete.py (98%) rename tests/{core/unit => unit/core}/test_doctor_telemetry.py (100%) rename tests/{core/unit => unit/core}/test_launchers.py (100%) rename tests/{core/unit => unit/core}/test_long_lived_acp_factory.py (99%) rename tests/{core/unit => unit/core}/test_orphan_reap.py (99%) rename tests/{core/unit => unit/core}/test_pi_rpc_messages.py (100%) rename tests/{core/unit => unit/core}/test_pi_rpc_translator.py (100%) rename tests/{core/unit => unit/core}/test_preflight_backends.py (100%) rename tests/{core/unit => unit/core}/test_settlement_rule.py (99%) rename tests/{core/unit => unit/core}/test_subprocess_resolve.py (100%) rename tests/{core/unit => unit/core}/test_verdict_roundtrip.py (99%) rename tests/{core/unit => unit/core}/test_watcher.py (100%) create mode 100644 tests/unit/tui/__init__.py rename tests/{ => unit}/tui/test_chat_runner.py (99%) rename tests/{ => unit}/tui/test_chat_turn_interrupted.py (100%) rename tests/{ => unit}/tui/test_mention_typeahead.py (99%) rename tests/{ => unit}/tui/test_mention_typeahead_wired.py (99%) diff --git a/src/kagan/cli/chat/acp.py b/src/kagan/cli/chat/acp.py index a2fbac4e..4c3c24f7 100644 --- a/src/kagan/cli/chat/acp.py +++ b/src/kagan/cli/chat/acp.py @@ -61,10 +61,6 @@ def get_lock(self, backend: str) -> asyncio.Lock: _WARMUP_STATE = OrchestratorWarmupState() -_acp_handshake_timeout_seconds = acp_handshake_timeout_seconds -_friendly_acp_error_message = friendly_acp_error_message - - def _is_mcp_server_unsupported_error(error: object) -> bool: raw = str(error).lower() return "mcp" in raw and "server" in raw and ("not implemented" in raw or "unsupported" in raw) @@ -303,7 +299,7 @@ async def run_orchestrator_turn( ) capture_client = _CaptureACPClient(on_update=on_update, permission_resolver=permission_resolver) - timeout_s = _acp_handshake_timeout_seconds(agent_backend) + timeout_s = acp_handshake_timeout_seconds(agent_backend) try: async with spawn_filtered_agent_process( capture_client, @@ -419,7 +415,7 @@ async def run_orchestrator_turn( await conn.prompt(session_id=sess.session_id, prompt=prompt_blocks) except (acp.RequestError, OSError, RuntimeError, ValueError, AttributeError) as exc: raise AgentError( - _friendly_acp_error_message( + friendly_acp_error_message( error=exc, agent_backend=agent_backend, during="prompt delivery", @@ -427,7 +423,7 @@ async def run_orchestrator_turn( ) from exc except (acp.RequestError, OSError, RuntimeError, ValueError, AttributeError) as exc: raise AgentError( - _friendly_acp_error_message(error=exc, agent_backend=agent_backend, during="handshake") + friendly_acp_error_message(error=exc, agent_backend=agent_backend, during="handshake") ) from exc finally: if not lightweight and mcp_path.exists(): diff --git a/tests/core/test_audit_cleanup.py b/tests/core/test_audit_cleanup.py index a477c987..993f9ffa 100644 --- a/tests/core/test_audit_cleanup.py +++ b/tests/core/test_audit_cleanup.py @@ -90,18 +90,3 @@ async def test_push_user_accepts_safe_message(board: KaganDriver) -> None: msg = await engine.push_user(sid, "please fix the login bug") assert msg.role == "user" assert msg.content == "please fix the login bug" - - -def test_rate_limit_log_key_does_not_leak_token() -> None: - """Token-keyed rate-limit log lines are hashed, never raw.""" - from kagan.server._middleware import RateLimitMiddleware - - raw = "token:sk_live_supersecret_AAAAAAAAAAAAAAAA" - safe = RateLimitMiddleware._safe_log_key(raw) - assert safe.startswith("token:") - assert "supersecret" not in safe - assert "sk_live" not in safe - assert len(safe) <= len("token:") + 8 - - # IP keys pass through unchanged. - assert RateLimitMiddleware._safe_log_key("192.0.2.1") == "192.0.2.1" diff --git a/tests/core/test_chat_lifecycle.py b/tests/core/test_chat_lifecycle.py index 5f9b251c..862c6129 100644 --- a/tests/core/test_chat_lifecycle.py +++ b/tests/core/test_chat_lifecycle.py @@ -122,9 +122,11 @@ async def test_chat_first_turn_invokes_title_generator(tmp_path: Path) -> None: from kagan.core.chat import ChatEngine calls: list[tuple[str, str]] = [] + title_called = asyncio.Event() async def _gen(user: str, reply: str) -> str | None: calls.append((user, reply)) + title_called.set() return "Generated Title" db_path = tmp_path / "kagan.db" @@ -153,15 +155,16 @@ async def _gen(user: str, reply: str) -> str | None: ) ) - # Title generation is fire-and-forget; poll briefly. - for _ in range(50): + await asyncio.wait_for(title_called.wait(), timeout=5.0) + # The engine awaits `_sessions.update(...)` immediately after the + # generator returns; yield until the label is observable. + while True: s = await core.chat_sessions.get(sid) if s is not None and s.label == "Generated Title": break - await asyncio.sleep(0.01) + await asyncio.sleep(0) assert len(calls) == 1 - s = await core.chat_sessions.get(sid) assert s is not None assert s.label == "Generated Title" finally: diff --git a/tests/core/test_cli_surface.py b/tests/core/test_cli_surface.py index 0aa458c2..1f1d0e87 100644 --- a/tests/core/test_cli_surface.py +++ b/tests/core/test_cli_surface.py @@ -1,5 +1,4 @@ import importlib.util -import os import subprocess from asyncio import run as asyncio_run from pathlib import Path @@ -9,7 +8,7 @@ from kagan.core import KaganCore from kagan.cli.doctor import DoctorCheck -from kagan.cli.main import _sanitize_startup_environment, cli +from kagan.cli.main import cli _HAS_RICH_CLICK = importlib.util.find_spec("rich_click") is not None @@ -621,21 +620,6 @@ def test_startup_update_hint_prints_before_command_output(monkeypatch, tmp_path: assert lines[0].startswith("hint: kagan 9.9.9 available") -def test_sanitize_startup_environment_removes_macos_malloc_keys(monkeypatch) -> None: - monkeypatch.setattr("kagan.cli._env.sys.platform", "darwin") - monkeypatch.setenv("MALLOCSTACKLOGGING", "1") - monkeypatch.setenv("MALLOCSTACKLOGGINGNOCOMPACT", "1") - monkeypatch.setenv("MALLOCSTACKLOGGINGDIRECTORY", "/tmp/msl") - monkeypatch.setenv("__XPC_MALLOCSTACKLOGGING", "1") - - _sanitize_startup_environment() - - assert os.environ.get("MALLOCSTACKLOGGING") is None - assert os.environ.get("MALLOCSTACKLOGGINGNOCOMPACT") is None - assert os.environ.get("MALLOCSTACKLOGGINGDIRECTORY") is None - assert os.environ.get("__XPC_MALLOCSTACKLOGGING") is None - - def test_tools_prompts_help_lists_export_subcommand(tmp_path: Path) -> None: runner = CliRunner() result = runner.invoke(cli, ["tools", "prompts", "--help"], env=_runner_env(tmp_path)) diff --git a/tests/core/unit/test_transitions.py b/tests/core/unit/test_transitions.py deleted file mode 100644 index b0d3fb13..00000000 --- a/tests/core/unit/test_transitions.py +++ /dev/null @@ -1,100 +0,0 @@ -"""Unit tests for the task lifecycle state machine (now in core/transitions.py).""" - -from __future__ import annotations - -import pytest - -from kagan.core.enums import TaskStatus -from kagan.core.errors import InvalidTransitionError - -pytestmark = [pytest.mark.core] - -# ── Direct-move guard (replaces validate_move from deleted _transitions.py) ─── -# -# Legal direct-move pairs (REVIEW→DONE is merge-only and excluded). -_DIRECT_MOVES: frozenset[tuple[TaskStatus, TaskStatus]] = frozenset( - { - (TaskStatus.BACKLOG, TaskStatus.IN_PROGRESS), - (TaskStatus.IN_PROGRESS, TaskStatus.REVIEW), - (TaskStatus.IN_PROGRESS, TaskStatus.BACKLOG), - (TaskStatus.REVIEW, TaskStatus.IN_PROGRESS), - (TaskStatus.REVIEW, TaskStatus.BACKLOG), - (TaskStatus.DONE, TaskStatus.BACKLOG), - } -) - - -def _validate_direct_move(from_status: TaskStatus, to_status: TaskStatus) -> None: - if (from_status, to_status) not in _DIRECT_MOVES: - raise InvalidTransitionError(from_status, to_status) - - -def _validate_merge_move(from_status: TaskStatus, to_status: TaskStatus) -> None: - if from_status != TaskStatus.REVIEW or to_status != TaskStatus.DONE: - raise InvalidTransitionError(from_status, to_status) - - -# ── validate_merge_move equivalent ─────────────────────────────────────────── - - -def test_validate_merge_move_accepts_review_to_done() -> None: - _validate_merge_move(TaskStatus.REVIEW, TaskStatus.DONE) - - -@pytest.mark.parametrize( - ("from_status", "to_status"), - [ - # All ordinary moves must be rejected by the *merge* gate, even if - # they are valid direct moves. - (TaskStatus.BACKLOG, TaskStatus.IN_PROGRESS), - (TaskStatus.IN_PROGRESS, TaskStatus.REVIEW), - (TaskStatus.IN_PROGRESS, TaskStatus.BACKLOG), - (TaskStatus.REVIEW, TaskStatus.IN_PROGRESS), - (TaskStatus.REVIEW, TaskStatus.BACKLOG), - (TaskStatus.DONE, TaskStatus.BACKLOG), - # Outright invalid: - (TaskStatus.BACKLOG, TaskStatus.DONE), - (TaskStatus.IN_PROGRESS, TaskStatus.DONE), - (TaskStatus.DONE, TaskStatus.REVIEW), - ], -) -def test_validate_merge_move_rejects_non_merge_transitions( - from_status: TaskStatus, to_status: TaskStatus -) -> None: - with pytest.raises(InvalidTransitionError): - _validate_merge_move(from_status, to_status) - - -# ── validate_move / can_move equivalent ────────────────────────────────────── - - -def test_validate_move_rejects_merge_path() -> None: - """REVIEW → DONE must not be reachable via the ordinary move guard.""" - with pytest.raises(InvalidTransitionError): - _validate_direct_move(TaskStatus.REVIEW, TaskStatus.DONE) - assert (TaskStatus.REVIEW, TaskStatus.DONE) not in _DIRECT_MOVES - - -def test_validate_move_accepts_known_paths() -> None: - _validate_direct_move(TaskStatus.BACKLOG, TaskStatus.IN_PROGRESS) - _validate_direct_move(TaskStatus.IN_PROGRESS, TaskStatus.REVIEW) - _validate_direct_move(TaskStatus.REVIEW, TaskStatus.IN_PROGRESS) - _validate_direct_move(TaskStatus.DONE, TaskStatus.BACKLOG) - - -# ── allowed_targets equivalent ──────────────────────────────────────────────── - - -def test_allowed_targets_excludes_merge() -> None: - targets = {to for (frm, to) in _DIRECT_MOVES if frm == TaskStatus.REVIEW} - assert TaskStatus.DONE not in targets - assert TaskStatus.IN_PROGRESS in targets - assert TaskStatus.BACKLOG in targets - - -def test_all_direct_move_pairs_are_exhaustive() -> None: - """Regression: confirm the inline set in Tasks.set_status matches this module.""" - # If a new status is ever added, this test should surface the gap. - for from_s, to_s in _DIRECT_MOVES: - assert isinstance(from_s, TaskStatus) - assert isinstance(to_s, TaskStatus) diff --git a/tests/unit/cli/test_env_sanitization.py b/tests/unit/cli/test_env_sanitization.py new file mode 100644 index 00000000..570c0e50 --- /dev/null +++ b/tests/unit/cli/test_env_sanitization.py @@ -0,0 +1,26 @@ +from __future__ import annotations + +import os + +import pytest + +from kagan.cli.main import _sanitize_startup_environment + +pytestmark = [pytest.mark.unit] + + +def test_sanitize_startup_environment_removes_macos_malloc_keys( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr("kagan.cli._env.sys.platform", "darwin") + monkeypatch.setenv("MALLOCSTACKLOGGING", "1") + monkeypatch.setenv("MALLOCSTACKLOGGINGNOCOMPACT", "1") + monkeypatch.setenv("MALLOCSTACKLOGGINGDIRECTORY", "/tmp/msl") + monkeypatch.setenv("__XPC_MALLOCSTACKLOGGING", "1") + + _sanitize_startup_environment() + + assert os.environ.get("MALLOCSTACKLOGGING") is None + assert os.environ.get("MALLOCSTACKLOGGINGNOCOMPACT") is None + assert os.environ.get("MALLOCSTACKLOGGINGDIRECTORY") is None + assert os.environ.get("__XPC_MALLOCSTACKLOGGING") is None diff --git a/tests/core/unit/test_analytics.py b/tests/unit/core/test_analytics.py similarity index 99% rename from tests/core/unit/test_analytics.py rename to tests/unit/core/test_analytics.py index 582d74c8..bc429c1b 100644 --- a/tests/core/unit/test_analytics.py +++ b/tests/unit/core/test_analytics.py @@ -16,7 +16,7 @@ from kagan.core.enums import SessionStatus from kagan.core.models import Project, Session, Task -pytestmark = [pytest.mark.core] +pytestmark = [pytest.mark.core, pytest.mark.unit] def _seed_sessions(engine, project_id: str) -> None: diff --git a/tests/core/unit/test_attachment_model.py b/tests/unit/core/test_attachment_model.py similarity index 100% rename from tests/core/unit/test_attachment_model.py rename to tests/unit/core/test_attachment_model.py diff --git a/tests/core/unit/test_backend_spec.py b/tests/unit/core/test_backend_spec.py similarity index 100% rename from tests/core/unit/test_backend_spec.py rename to tests/unit/core/test_backend_spec.py diff --git a/tests/core/unit/test_cascade_delete.py b/tests/unit/core/test_cascade_delete.py similarity index 98% rename from tests/core/unit/test_cascade_delete.py rename to tests/unit/core/test_cascade_delete.py index 7baf9a57..bb3759ec 100644 --- a/tests/core/unit/test_cascade_delete.py +++ b/tests/unit/core/test_cascade_delete.py @@ -24,7 +24,7 @@ if TYPE_CHECKING: from pathlib import Path -pytestmark = [pytest.mark.core] +pytestmark = [pytest.mark.core, pytest.mark.unit] def _make_engine(tmp_path: Path): diff --git a/tests/core/unit/test_doctor_telemetry.py b/tests/unit/core/test_doctor_telemetry.py similarity index 100% rename from tests/core/unit/test_doctor_telemetry.py rename to tests/unit/core/test_doctor_telemetry.py diff --git a/tests/core/unit/test_launchers.py b/tests/unit/core/test_launchers.py similarity index 100% rename from tests/core/unit/test_launchers.py rename to tests/unit/core/test_launchers.py diff --git a/tests/core/unit/test_long_lived_acp_factory.py b/tests/unit/core/test_long_lived_acp_factory.py similarity index 99% rename from tests/core/unit/test_long_lived_acp_factory.py rename to tests/unit/core/test_long_lived_acp_factory.py index b6a3f1b7..3c560cb3 100644 --- a/tests/core/unit/test_long_lived_acp_factory.py +++ b/tests/unit/core/test_long_lived_acp_factory.py @@ -20,7 +20,7 @@ from kagan.core._agent import BackendSpec from kagan.core.chat import LongLivedACPFactory -pytestmark = [pytest.mark.core] +pytestmark = [pytest.mark.core, pytest.mark.unit] # --------------------------------------------------------------------------- diff --git a/tests/core/unit/test_orphan_reap.py b/tests/unit/core/test_orphan_reap.py similarity index 99% rename from tests/core/unit/test_orphan_reap.py rename to tests/unit/core/test_orphan_reap.py index d2424704..1cd149a7 100644 --- a/tests/core/unit/test_orphan_reap.py +++ b/tests/unit/core/test_orphan_reap.py @@ -17,7 +17,7 @@ from kagan.core.enums import SessionStatus, TaskStatus from kagan.core.models import Project, Repository, Session, Task -pytestmark = [pytest.mark.core] +pytestmark = [pytest.mark.core, pytest.mark.unit] def _make_engine(tmp_path: Path): diff --git a/tests/core/unit/test_pi_rpc_messages.py b/tests/unit/core/test_pi_rpc_messages.py similarity index 100% rename from tests/core/unit/test_pi_rpc_messages.py rename to tests/unit/core/test_pi_rpc_messages.py diff --git a/tests/core/unit/test_pi_rpc_translator.py b/tests/unit/core/test_pi_rpc_translator.py similarity index 100% rename from tests/core/unit/test_pi_rpc_translator.py rename to tests/unit/core/test_pi_rpc_translator.py diff --git a/tests/core/unit/test_preflight_backends.py b/tests/unit/core/test_preflight_backends.py similarity index 100% rename from tests/core/unit/test_preflight_backends.py rename to tests/unit/core/test_preflight_backends.py diff --git a/tests/core/unit/test_settlement_rule.py b/tests/unit/core/test_settlement_rule.py similarity index 99% rename from tests/core/unit/test_settlement_rule.py rename to tests/unit/core/test_settlement_rule.py index 1fb812fd..cc2f057a 100644 --- a/tests/core/unit/test_settlement_rule.py +++ b/tests/unit/core/test_settlement_rule.py @@ -9,7 +9,7 @@ from kagan.core._events import Events -pytestmark = [pytest.mark.core] +pytestmark = [pytest.mark.core, pytest.mark.unit] # --------------------------------------------------------------------------- diff --git a/tests/unit/core/test_spawn_per_turn_acp_factory.py b/tests/unit/core/test_spawn_per_turn_acp_factory.py index ad3cad6b..199cb83c 100644 --- a/tests/unit/core/test_spawn_per_turn_acp_factory.py +++ b/tests/unit/core/test_spawn_per_turn_acp_factory.py @@ -14,6 +14,8 @@ if TYPE_CHECKING: from pathlib import Path +pytestmark = [pytest.mark.unit] + async def _noop_update(_update: Any) -> None: return None diff --git a/tests/core/unit/test_subprocess_resolve.py b/tests/unit/core/test_subprocess_resolve.py similarity index 100% rename from tests/core/unit/test_subprocess_resolve.py rename to tests/unit/core/test_subprocess_resolve.py diff --git a/tests/unit/core/test_task_classification.py b/tests/unit/core/test_task_classification.py index d0484021..2192a83f 100644 --- a/tests/unit/core/test_task_classification.py +++ b/tests/unit/core/test_task_classification.py @@ -7,9 +7,13 @@ - Batch classification """ +import pytest + from kagan.core._task_classification import classify_task from kagan.core.enums import TaskType +pytestmark = [pytest.mark.unit] + class TestClassifyTaskIndividual: """Test individual task classification.""" diff --git a/tests/core/unit/test_verdict_roundtrip.py b/tests/unit/core/test_verdict_roundtrip.py similarity index 99% rename from tests/core/unit/test_verdict_roundtrip.py rename to tests/unit/core/test_verdict_roundtrip.py index 8b531954..4ce8dbc8 100644 --- a/tests/core/unit/test_verdict_roundtrip.py +++ b/tests/unit/core/test_verdict_roundtrip.py @@ -25,7 +25,7 @@ Task, ) -pytestmark = [pytest.mark.core] +pytestmark = [pytest.mark.core, pytest.mark.unit] def _make_engine(tmp_path: Path): diff --git a/tests/core/unit/test_watcher.py b/tests/unit/core/test_watcher.py similarity index 100% rename from tests/core/unit/test_watcher.py rename to tests/unit/core/test_watcher.py diff --git a/tests/unit/server/test_middleware.py b/tests/unit/server/test_middleware.py index e4d58836..35507e1a 100644 --- a/tests/unit/server/test_middleware.py +++ b/tests/unit/server/test_middleware.py @@ -6,7 +6,7 @@ from starlette.routing import Route from starlette.testclient import TestClient -from kagan.server._middleware import install_security_middleware +from kagan.server._middleware import RateLimitMiddleware, install_security_middleware pytestmark = [pytest.mark.unit] @@ -50,3 +50,14 @@ def test_rate_limiter_applies_to_api_routes(monkeypatch: pytest.MonkeyPatch) -> response = client.get("/api/example") assert response.status_code == 429 assert response.json()["error"] == "Rate limit exceeded — try again later" + + +def test_rate_limit_log_key_does_not_leak_token() -> None: + raw = "token:sk_live_supersecret_AAAAAAAAAAAAAAAA" + safe = RateLimitMiddleware._safe_log_key(raw) + assert safe.startswith("token:") + assert "supersecret" not in safe + assert "sk_live" not in safe + assert len(safe) <= len("token:") + 8 + + assert RateLimitMiddleware._safe_log_key("192.0.2.1") == "192.0.2.1" diff --git a/tests/unit/test_acp_timeout_config.py b/tests/unit/test_acp_timeout_config.py index 212f552b..d15c1d08 100644 --- a/tests/unit/test_acp_timeout_config.py +++ b/tests/unit/test_acp_timeout_config.py @@ -1,16 +1,20 @@ import pytest -from kagan.cli.chat.acp import _acp_handshake_timeout_seconds from kagan.core import CLAUDE_CODE_BACKEND, CODEX_BACKEND -from kagan.core._acp import acp_process_exit_hint, acp_startup_timeout_seconds, friendly_acp_error_message +from kagan.core._acp import ( + acp_handshake_timeout_seconds, + acp_process_exit_hint, + acp_startup_timeout_seconds, + friendly_acp_error_message, +) pytestmark = [pytest.mark.unit] def test_acp_timeout_defaults_are_backend_aware() -> None: - assert _acp_handshake_timeout_seconds("claude-code") == 12.0 - assert _acp_handshake_timeout_seconds("codex") == 45.0 - assert _acp_handshake_timeout_seconds("gemini-cli") == 20.0 + assert acp_handshake_timeout_seconds("claude-code") == 12.0 + assert acp_handshake_timeout_seconds("codex") == 45.0 + assert acp_handshake_timeout_seconds("gemini-cli") == 20.0 assert acp_startup_timeout_seconds("claude-code") == 12.0 assert acp_startup_timeout_seconds("codex") == 45.0 @@ -20,10 +24,10 @@ def test_acp_timeout_defaults_are_backend_aware() -> None: def test_chat_timeout_reads_both_env_keys(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.delenv("KAGAN_ACP_HANDSHAKE_TIMEOUT_SECONDS", raising=False) monkeypatch.setenv("KAGAN_ACP_STARTUP_TIMEOUT_SECONDS", "33") - assert _acp_handshake_timeout_seconds("gemini-cli") == 33.0 + assert acp_handshake_timeout_seconds("gemini-cli") == 33.0 monkeypatch.setenv("KAGAN_ACP_HANDSHAKE_TIMEOUT_SECONDS", "27") - assert _acp_handshake_timeout_seconds("gemini-cli") == 27.0 + assert acp_handshake_timeout_seconds("gemini-cli") == 27.0 def test_core_timeout_reads_both_env_keys(monkeypatch: pytest.MonkeyPatch) -> None: @@ -66,3 +70,30 @@ def test_friendly_acp_error_message_uses_reference_backend_auth_guidance( during="ACP initialize", ) assert expected_hint in message + + +@pytest.mark.parametrize( + ("error_text", "expected_fragment"), + [ + ("429 too many requests", "Rate limit"), + ("rate_limit exceeded", "Rate limit"), + ("quota exceeded", "usage/subscription"), + ("active subscription required", "usage/subscription"), + ("invalid api key", "Authentication"), + ("ENOTFOUND example.com", "Network"), + ("fetch failed", "Network"), + ("529 service unavailable", "overloaded"), + ("overloaded", "overloaded"), + ], +) +def test_friendly_acp_error_message_classifies_by_category( + error_text: str, + expected_fragment: str, +) -> None: + message = friendly_acp_error_message( + error=error_text, + agent_backend=CLAUDE_CODE_BACKEND, + during="startup", + ) + assert expected_fragment in message + assert CLAUDE_CODE_BACKEND in message diff --git a/tests/unit/tui/__init__.py b/tests/unit/tui/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/tui/test_chat_runner.py b/tests/unit/tui/test_chat_runner.py similarity index 99% rename from tests/tui/test_chat_runner.py rename to tests/unit/tui/test_chat_runner.py index 70f0471c..b64f3b83 100644 --- a/tests/tui/test_chat_runner.py +++ b/tests/unit/tui/test_chat_runner.py @@ -34,6 +34,8 @@ ) from kagan.tui.widgets.chat import ChatPanel +pytestmark = [pytest.mark.tui, pytest.mark.unit] + class _ChatPanelHostApp(App[None]): def compose(self) -> ComposeResult: diff --git a/tests/tui/test_chat_turn_interrupted.py b/tests/unit/tui/test_chat_turn_interrupted.py similarity index 100% rename from tests/tui/test_chat_turn_interrupted.py rename to tests/unit/tui/test_chat_turn_interrupted.py diff --git a/tests/tui/test_mention_typeahead.py b/tests/unit/tui/test_mention_typeahead.py similarity index 99% rename from tests/tui/test_mention_typeahead.py rename to tests/unit/tui/test_mention_typeahead.py index 524ec61b..8853b5b9 100644 --- a/tests/tui/test_mention_typeahead.py +++ b/tests/unit/tui/test_mention_typeahead.py @@ -17,7 +17,7 @@ if TYPE_CHECKING: from kagan.tui.widgets._mention_typeahead import MentionTypeahead -pytestmark = [pytest.mark.tui, pytest.mark.smoke] +pytestmark = [pytest.mark.tui, pytest.mark.unit] # --------------------------------------------------------------------------- diff --git a/tests/tui/test_mention_typeahead_wired.py b/tests/unit/tui/test_mention_typeahead_wired.py similarity index 99% rename from tests/tui/test_mention_typeahead_wired.py rename to tests/unit/tui/test_mention_typeahead_wired.py index 071a7baa..9ca7c526 100644 --- a/tests/tui/test_mention_typeahead_wired.py +++ b/tests/unit/tui/test_mention_typeahead_wired.py @@ -12,7 +12,7 @@ from textual.app import App, ComposeResult from textual.widgets import TextArea -pytestmark = [pytest.mark.tui, pytest.mark.smoke] +pytestmark = [pytest.mark.tui, pytest.mark.unit] # --------------------------------------------------------------------------- From 911cb85b28a921510f37eadc1b0b7a716bb29fbc Mon Sep 17 00:00:00 2001 From: Altynbek Orumbayev Date: Wed, 6 May 2026 16:19:48 +0200 Subject: [PATCH 7/8] ci: point fast-gate at relocated tests/unit/core/ path The previous commit moved tests from tests/core/unit/ to tests/unit/core/ to comply with docs/internal/testing.md, but the CI fast-gate and CD release-smoke step still pointed at the old path and selected zero tests. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/cd.yaml | 2 +- .github/workflows/ci.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/cd.yaml b/.github/workflows/cd.yaml index 6613acde..abbe34b0 100644 --- a/.github/workflows/cd.yaml +++ b/.github/workflows/cd.yaml @@ -91,7 +91,7 @@ jobs: run: | set -o pipefail uv run poe db-migrations-check - uv run pytest tests/core/unit/ -m "core and unit" -n auto -q + uv run pytest tests/unit/core/ -m "core and unit" -n auto -q - name: Run Semantic Release id: release diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 611d05d1..af883a92 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -95,7 +95,7 @@ jobs: pyproject.toml - run: uv sync --frozen --dev - run: uv run poe db-migrations-check - - run: uv run pytest tests/core/unit/ -m "core and unit" -n auto -q + - run: uv run pytest tests/unit/core/ -m "core and unit" -n auto -q test-pr: if: github.event_name == 'pull_request' && github.event.pull_request.draft == false From 0181f259572134a2f21e0c6efc56cb14b78ff118 Mon Sep 17 00:00:00 2001 From: Altynbek Orumbayev Date: Wed, 6 May 2026 16:23:06 +0200 Subject: [PATCH 8/8] refactor(core): address greptile review on PR 151 - JsonRpcObjectStreamReader: add explicit read/readuntil/readexactly delegating stubs so an SDK call to one of those methods raises a clear error rather than being silently swallowed by the broad AttributeError handler in run_acp_session, mirroring _ByteCountingStreamReader. - test_chat_first_turn_invokes_title_generator: bound the post-event label-observation loop with asyncio.wait_for(timeout=3.0) so a regression in the engine's label write surfaces as a TimeoutError instead of hanging the test indefinitely. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/kagan/core/_acp_streams.py | 9 +++++++++ tests/core/test_chat_lifecycle.py | 17 +++++++++-------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/kagan/core/_acp_streams.py b/src/kagan/core/_acp_streams.py index 9f109869..31ee9020 100644 --- a/src/kagan/core/_acp_streams.py +++ b/src/kagan/core/_acp_streams.py @@ -47,6 +47,15 @@ async def readline(self) -> bytes: return line + async def readuntil(self, separator: bytes = b"\n") -> bytes: + return await self._reader.readuntil(separator) + + async def read(self, n: int = -1) -> bytes: + return await self._reader.read(n) + + async def readexactly(self, n: int) -> bytes: + return await self._reader.readexactly(n) + def at_eof(self) -> bool: return self._reader.at_eof() diff --git a/tests/core/test_chat_lifecycle.py b/tests/core/test_chat_lifecycle.py index 862c6129..c0d9d164 100644 --- a/tests/core/test_chat_lifecycle.py +++ b/tests/core/test_chat_lifecycle.py @@ -156,16 +156,17 @@ async def _gen(user: str, reply: str) -> str | None: ) await asyncio.wait_for(title_called.wait(), timeout=5.0) - # The engine awaits `_sessions.update(...)` immediately after the - # generator returns; yield until the label is observable. - while True: - s = await core.chat_sessions.get(sid) - if s is not None and s.label == "Generated Title": - break - await asyncio.sleep(0) + + async def _await_label() -> object: + while True: + s = await core.chat_sessions.get(sid) + if s is not None and s.label == "Generated Title": + return s + await asyncio.sleep(0) + + s = await asyncio.wait_for(_await_label(), timeout=3.0) assert len(calls) == 1 - assert s is not None assert s.label == "Generated Title" finally: core.close()