diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index bb368ba8..f0805c8b 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -22,7 +22,7 @@ repos: args: [--maxkb=1024] - id: check-merge-conflict - id: detect-private-key - exclude: ^tests/test_runtime_checkpoint_bug2_deep\.py$ + exclude: ^tests/test_runtime_checkpoint_deep\.py$ - repo: local hooks: diff --git a/docs/everos-memory-e2e-test-plan.md b/docs/everos-memory-e2e-test-plan.md index 87cdc9ce..ac4c9a3c 100644 --- a/docs/everos-memory-e2e-test-plan.md +++ b/docs/everos-memory-e2e-test-plan.md @@ -42,7 +42,7 @@ expectations. | Layer | What | Where | LLM? | |---|---|---|---| -| L1 | Backend translation (owner routing, message conversion, result flatten, degradation) | `tests/test_em2_backend.py`, `tests/test_em3_http.py` | no (fakes) — default CI | +| L1 | Backend translation (owner routing, message conversion, result flatten, degradation) | `tests/test_everos_backend.py`, `tests/test_everos_http_adapter.py` | no (fakes) — default CI | | L2 | everos extraction quality — direct service + `is_final` flush | `tests/integration/test_everos_extraction_real_llm.py` | yes (`real_llm`) | | L3 | backend ↔ everos e2e — embedded mode `store`/`recall` | `tests/integration/test_everos_backend_e2e.py` | yes (`real_llm`) | diff --git a/docs/memory-plugin-architecture.md b/docs/memory-plugin-architecture.md index 28d64eaa..3f122605 100644 --- a/docs/memory-plugin-architecture.md +++ b/docs/memory-plugin-architecture.md @@ -323,8 +323,8 @@ place (raven's `pyproject.toml`). The upgrade surface is one line. written in adapter comments. 3. **Test (all three layers)**: ```bash - uv run pytest tests/test_em1_skeleton.py tests/test_em2_backend.py \ - tests/test_em3_http.py tests/test_memory_backend_protocol.py \ + uv run pytest tests/test_everos_plugin_discovery.py tests/test_everos_backend.py \ + tests/test_everos_http_adapter.py tests/test_memory_backend_protocol.py \ tests/test_memory_backend_contract.py -q # unit (mock adapter) uv run pytest tests/integration/test_everos_backend_e2e.py -m real_llm # real python scripts/everos_memory_roundtrip.py # native shell smoke @@ -342,7 +342,7 @@ place (raven's `pyproject.toml`). The upgrade surface is one line. | Layer | Result | |---|---| -| Unit (em1/em2/em3, protocol, contract, plugin discovery/command/tools, cl1, context, config, ag1/fb1, agent-loop pipeline) | 240 passed | +| Unit (everos plugin discovery / backend / http adapter, protocol, contract, plugin command/tools, cli plugin stack, context, config, agent-loop backend dispatch + feedback, agent-loop pipeline) | 240 passed | | `raven plugins` | everos-memory · Source=`bundled` · Status=`activated` | | `real_llm` e2e (`test_everos_backend_e2e.py`) | 2 passed, 1 xfailed (best-effort skill-cluster check) — store→extract→recall + dual-track isolation | | roundtrip script (new import path) | OK; `users/user-raven/user.md` generated | diff --git a/docs/sandbox/usage.md b/docs/sandbox/usage.md index dbbfe4cd..fb80d883 100644 --- a/docs/sandbox/usage.md +++ b/docs/sandbox/usage.md @@ -632,7 +632,7 @@ On Linux without `/dev/kvm` the entire file is **automatically skipped** — no **First run — pre-pull OCI images:** -A session-scoped fixture in `test_sandbox_integration.py` pre-pulls all required images +A session-scoped fixture in `test_sandbox_real_vm.py` pre-pulls all required images (`ubuntu:22.04` and `node:20-slim`) before the first test. On a fast connection this takes ~30–60 s on first run and is instant on subsequent runs (images are cached by boxlite). @@ -646,18 +646,18 @@ SKIPPED OCI image pull failed for 'ubuntu:22.04' — likely a network issue, no **Run all integration tests:** ```bash -uv run python -m pytest tests/test_sandbox_integration.py -v +uv run python -m pytest tests/integration/test_sandbox_real_vm.py -v ``` Expected output: ``` -tests/test_sandbox_integration.py::TestBoxliteExecutorIntegration::test_exec_echo PASSED -tests/test_sandbox_integration.py::TestBoxliteExecutorIntegration::test_exec_timeout PASSED -tests/test_sandbox_integration.py::TestBoxliteExecutorIntegration::test_exec_cwd PASSED -tests/test_sandbox_integration.py::TestBoxliteExecutorIntegration::test_volume_mount_file_visible_in_vm PASSED -tests/test_sandbox_integration.py::TestBoxliteExecutorIntegration::test_lifecycle_context_manager PASSED -tests/test_sandbox_integration.py::TestBoxliteStdioMCPRoundtrip::test_npx_mcp_server_everything PASSED +tests/integration/test_sandbox_real_vm.py::TestBoxliteExecutorIntegration::test_exec_echo PASSED +tests/integration/test_sandbox_real_vm.py::TestBoxliteExecutorIntegration::test_exec_timeout PASSED +tests/integration/test_sandbox_real_vm.py::TestBoxliteExecutorIntegration::test_exec_cwd PASSED +tests/integration/test_sandbox_real_vm.py::TestBoxliteExecutorIntegration::test_volume_mount_file_visible_in_vm PASSED +tests/integration/test_sandbox_real_vm.py::TestBoxliteExecutorIntegration::test_lifecycle_context_manager PASSED +tests/integration/test_sandbox_real_vm.py::TestBoxliteStdioMCPRoundtrip::test_npx_mcp_server_everything PASSED 6 passed in ~55s ``` @@ -670,13 +670,13 @@ on each run (~15 s), then starts the MCP server and validates the full `initiali **Run unit and integration tests together:** ```bash -uv run python -m pytest tests/test_sandbox_unit.py tests/test_sandbox_integration.py -v +uv run python -m pytest tests/test_sandbox_unit.py tests/integration/test_sandbox_real_vm.py -v ``` **Run the full project test suite** (all test files, excluding integration): ```bash -uv run python -m pytest tests/ --ignore=tests/test_sandbox_integration.py -q +uv run python -m pytest tests/ --ignore=tests/integration/test_sandbox_real_vm.py -q ``` --- @@ -688,7 +688,7 @@ uv run python -m pytest tests/ --ignore=tests/test_sandbox_integration.py -q uv run python -m pytest "tests/test_sandbox_unit.py::TestBoxliteTranslateCwd::test_subdir_translates_correctly" -v # A single integration test -uv run python -m pytest "tests/test_sandbox_integration.py::TestBoxliteStdioMCPRoundtrip::test_npx_mcp_server_everything" -v -s +uv run python -m pytest "tests/integration/test_sandbox_real_vm.py::TestBoxliteStdioMCPRoundtrip::test_npx_mcp_server_everything" -v -s ``` --- @@ -768,5 +768,5 @@ python -c "from raven.sandbox import build_executor, SandboxConfig; print('sandb To verify end-to-end (requires KVM / Apple Silicon): ```bash -uv run python -m pytest tests/test_sandbox_integration.py -v +uv run python -m pytest tests/integration/test_sandbox_real_vm.py -v ``` diff --git a/tests/conftest.py b/tests/conftest.py index 0354414c..e51398db 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -12,11 +12,12 @@ def pytest_unconfigure(config: pytest.Config) -> None: """On CI, hard-exit past interpreter finalization once the run is over. - Native runtimes pulled in by the suite (lancedb's Rust/tokio thread, asyncio - subprocess transports finalized during GC) segfault Py_FinalizeEx on Linux, - turning a fully green run into exit 139. raven.cli._exit guards the CLI the - same way; the pytest process needs its own guard because it finalizes with - those runtimes live. + A fully green run still exited 139 on Linux: the suite finalizes with + native state live (asyncio subprocess transports collected during GC), + and Py_FinalizeEx segfaults on it, masking the recorded status. The CLI + routes its exit through the same helper, but on a different trigger -- + see raven.cli._exit for the lancedb-specific gate it uses, which is not + what fires here. Local runs keep normal semantics so nothing masks an exit-time error, and the recorded status is preserved either way -- a failing run still exits diff --git a/tests/integration/test_chat_e2e.py b/tests/integration/test_chat_e2e.py deleted file mode 100644 index 2bd7e93e..00000000 --- a/tests/integration/test_chat_e2e.py +++ /dev/null @@ -1,37 +0,0 @@ -"""Placeholder for chat E2E integration tests — DEFERRED to production wire-up. - -The chat infrastructure (turn.* / SubscriptionEmitter / chat_stream) is all -testable, but the `raven tui` production startup path is not yet wired to -instantiate AgentLoop + SubscriptionEmitter. That wire (and the JS side -``chatStream.attach()`` flip) lives in the follow-up production wire-up. - -When that ships, replace ``@pytest.mark.skip`` below with the 3 E2E -scenarios (short chat / long chat / Ctrl+C cancel). -""" - -from __future__ import annotations - -import pytest - -pytestmark = pytest.mark.skip( - reason=( - "production wire deferred to tui-chat-prodwire L2 v0.1.1 — " - "see docs/openspec/changes/tui-chat/proposal.md §4.2 amendment " - "2026-05-19 'B 路径' decision" - ), -) - - -def test_chat_e2e_short_placeholder() -> None: - """Placeholder — see module docstring.""" - raise NotImplementedError("activated by tui-chat-prodwire L2") - - -def test_chat_e2e_long_placeholder() -> None: - """Placeholder — see module docstring.""" - raise NotImplementedError("activated by tui-chat-prodwire L2") - - -def test_chat_e2e_ctrl_c_placeholder() -> None: - """Placeholder — see module docstring.""" - raise NotImplementedError("activated by tui-chat-prodwire L2") diff --git a/tests/integration/test_everos_backend_e2e.py b/tests/integration/test_everos_backend_e2e.py index 6d99588b..d18dc34f 100644 --- a/tests/integration/test_everos_backend_e2e.py +++ b/tests/integration/test_everos_backend_e2e.py @@ -83,7 +83,11 @@ async def test_user_track_recall_through_backend( user_id=ids.user_id, top_k=5, ) - assert isinstance(hits, list) + # Recall returning nothing is a best-effort miss per this module's + # strategy, but it must not read as a pass: the per-hit assertions below + # are vacuous on an empty list. + if not hits: + pytest.xfail("recall returned no hits; the per-hit assertions would be vacuous") for h in hits: assert isinstance(h, Memory) assert h.metadata.get("owner_type") == "user" diff --git a/tests/integration/test_sandbox_cli_integration.py b/tests/integration/test_sandbox_cli_smoke.py similarity index 100% rename from tests/integration/test_sandbox_cli_integration.py rename to tests/integration/test_sandbox_cli_smoke.py diff --git a/tests/integration/test_sandbox_integration.py b/tests/integration/test_sandbox_real_vm.py similarity index 100% rename from tests/integration/test_sandbox_integration.py rename to tests/integration/test_sandbox_real_vm.py diff --git a/tests/integration/test_tui_exit_e2e.py b/tests/integration/test_tui_exit_e2e.py deleted file mode 100644 index 6cb76a04..00000000 --- a/tests/integration/test_tui_exit_e2e.py +++ /dev/null @@ -1,90 +0,0 @@ -"""`raven` commands that build the agent loop must not segfault on exit. - -The agent loop opens a lancedb-backed store; lancedb starts a process-global -Rust/tokio background thread (``LanceDBBackgroundEventLoop``) with no public -shutdown hook. A normal CPython interpreter finalization races that live native -runtime and segfaults (exit 139), masking the command's real exit code and -failing ``expect_exit(0)`` for the whole TUI e2e suite. - -Fix: the CLI exit chokepoint ``raven.cli.commands.run`` hard-exits past -finalization (flush stdio + loguru, then ``os._exit``) when -``raven.cli._exit.lancedb_finalization_hazard`` reports the thread live. These -tests build the real agent loop in a subprocess so the native thread is -genuinely live. -""" - -from __future__ import annotations - -import subprocess -import sys -import textwrap - -import pytest - -# Exit code the child uses to signal "agent loop could not be built in this -# environment" (e.g. no provider configured) so the test skips instead of -# reporting a false regression. -_BUILD_FAILED = 42 - -_BUILD_LOOP = """ -import sys -try: - from raven.cli.tui_commands import _build_tui_agent_loop - loop = _build_tui_agent_loop() -except BaseException as e: - print(f"BUILD_FAILED: {type(e).__name__}: {e}", file=sys.stderr) - sys.exit(42) -""" - - -def _run(child_src: str) -> subprocess.CompletedProcess: - return subprocess.run( - [sys.executable, "-c", textwrap.dedent(child_src)], - capture_output=True, - text=True, - timeout=120, - ) - - -# Exit code the child uses to signal "the hazard gate failed to detect the -# live lancedb thread" — distinct from a clean 0 or the build-failed sentinel. -_HAZARD_ABSENT = 43 - - -def test_hazard_gate_fires_and_hard_exit_is_clean(): - """After building the loop the hazard gate reports the native thread live - (so the CLI chokepoint knows to guard), and exiting via the hard-exit - helper returns cleanly (exit 0) instead of a SIGSEGV.""" - src = ( - _BUILD_LOOP - + "from raven.cli._exit import flush_and_hard_exit, lancedb_finalization_hazard\n" - + "if not lancedb_finalization_hazard():\n" - + " sys.exit(43)\n" - + "flush_and_hard_exit(0)\n" - ) - result = _run(src) - if result.returncode == _BUILD_FAILED: - pytest.skip(f"agent loop unbuildable here: {result.stderr.strip()[-200:]}") - assert result.returncode != _HAZARD_ABSENT, ( - "hazard gate did not detect the live lancedb thread after building the loop" - ) - assert result.returncode == 0, ( - f"hard-exit path did not exit 0 (rc={result.returncode}); stderr tail:\n{result.stderr[-1500:]}" - ) - - -def test_normal_finalization_still_reproduces_the_crash(): - """Guard that the hard-exit is load-bearing: with the native memory stack - live, a normal interpreter finalization crashes. Skips when the current - environment cannot reproduce the crash (so the suite stays green on hosts - without the offending native runtime).""" - src = _BUILD_LOOP + "sys.exit(0)\n" - result = _run(src) - if result.returncode == _BUILD_FAILED: - pytest.skip(f"agent loop unbuildable here: {result.stderr.strip()[-200:]}") - if result.returncode == 0: - pytest.skip("native finalization crash not reproducible in this environment") - # subprocess.run reports a signal-killed child as a negative return code - # (-11 for SIGSEGV, -6 for a Rust abort, etc.); the native runtime tearing - # down mid-finalization is a fatal signal, never a clean nonzero exit. - assert result.returncode < 0, f"expected a fatal-signal crash on finalization, got rc={result.returncode}" diff --git a/tests/test_ag1_backend_dispatch.py b/tests/test_agent_loop_backend_dispatch.py similarity index 96% rename from tests/test_ag1_backend_dispatch.py rename to tests/test_agent_loop_backend_dispatch.py index 53d77378..1cc6e248 100644 --- a/tests/test_ag1_backend_dispatch.py +++ b/tests/test_agent_loop_backend_dispatch.py @@ -1,4 +1,4 @@ -"""AG-1 — AgentLoop ``backend`` wiring + ``_dispatch_backend_store``. +"""AgentLoop ``backend`` wiring + ``_dispatch_backend_store``. The two after-turn callsites (system-message path + REPL path) now call :meth:`AgentLoop._dispatch_backend_store` as the third peer step in the @@ -150,7 +150,7 @@ async def test_backend_exception_swallowed( # --------------------------------------------------------------------------- -# Legacy compatibility — pre-AG-1 callsites still pass +# Legacy compatibility -- callsites predating the backend keyword still pass # --------------------------------------------------------------------------- @@ -159,7 +159,7 @@ def test_construction_without_backend_unchanged( self, tmp_path: Path, ) -> None: - """Pre-AG-1 construction (no ``backend=`` keyword) still works + """Construction without the ``backend=`` keyword still works end-to-end. After Phase B-3 the ``self.memory`` facade is gone; we now assert against the direct subsystem fields AgentLoop holds (``memory_consolidator`` + ``context.skills``).""" diff --git a/tests/test_appworld_precheck.py b/tests/test_appworld_precheck.py index 105c4bcf..cb133958 100644 --- a/tests/test_appworld_precheck.py +++ b/tests/test_appworld_precheck.py @@ -12,13 +12,11 @@ from pathlib import Path from types import SimpleNamespace -import pytest - REPO_ROOT = Path(__file__).resolve().parents[1] if str(REPO_ROOT) not in sys.path: sys.path.insert(0, str(REPO_ROOT)) -httpx = pytest.importorskip("httpx") +import httpx from benchmarks.appworld.evolve.precheck import ( # noqa: E402 _endpoint_problem, diff --git a/tests/test_auth_allowlist.py b/tests/test_auth_allowlist.py index 249c6a7d..2ca571e7 100644 --- a/tests/test_auth_allowlist.py +++ b/tests/test_auth_allowlist.py @@ -10,7 +10,7 @@ from __future__ import annotations import logging -from dataclasses import dataclass +from dataclasses import FrozenInstanceError, dataclass from typing import Any import pytest @@ -215,5 +215,5 @@ def test_locked_field_membership(self): def test_frozen_dataclass(self): s = ManagedSettings() - with pytest.raises(Exception): # FrozenInstanceError subclass + with pytest.raises(FrozenInstanceError): s.description = "mutated" # type: ignore[misc] diff --git a/tests/test_fb1_feedback_dispatch.py b/tests/test_backend_feedback_dispatch.py similarity index 99% rename from tests/test_fb1_feedback_dispatch.py rename to tests/test_backend_feedback_dispatch.py index b948a787..a2090e8f 100644 --- a/tests/test_fb1_feedback_dispatch.py +++ b/tests/test_backend_feedback_dispatch.py @@ -1,4 +1,4 @@ -"""FB-1 — qualified_id feedback dispatcher. +"""qualified_id feedback dispatcher. Exercises: 1. ``_filter_qualified_ids`` helper — prefix matching, native id diff --git a/tests/test_cli_deep_research_commands.py b/tests/test_cli_deep_research_commands.py index ece518d4..0c6fd44d 100644 --- a/tests/test_cli_deep_research_commands.py +++ b/tests/test_cli_deep_research_commands.py @@ -179,7 +179,7 @@ def test_configure_validation_fail_then_save(tmp_path: Path, monkeypatch): def test_configure_validation_fail_then_cancel(tmp_path: Path, monkeypatch): fail = {"ok": False, "status": "http_401", "model_ids": None, "error": "bad"} - p = _setup_interactive(monkeypatch, tmp_path, ["configure", "cancel"], validate=lambda *a, **k: fail) + _setup_interactive(monkeypatch, tmp_path, ["configure", "cancel"], validate=lambda *a, **k: fail) assert configure_deep_research(non_interactive=False, warnings=[]) is False # cancelled, nothing written @@ -231,9 +231,7 @@ def _flaky(*a, **k): calls["n"] += 1 return {"ok": calls["n"] > 1, "status": "http_401" if calls["n"] == 1 else "ok", "model_ids": [], "error": None} - p = _setup_interactive( - monkeypatch, tmp_path, ["configure", "retry", "mirothinker-1-7-deepresearch"], validate=_flaky - ) + _setup_interactive(monkeypatch, tmp_path, ["configure", "retry", "mirothinker-1-7-deepresearch"], validate=_flaky) assert configure_deep_research(non_interactive=False, warnings=[]) is True assert calls["n"] == 2 # first validate failed, retry validated ok diff --git a/tests/test_cli_exit.py b/tests/test_cli_exit.py new file mode 100644 index 00000000..e3a879df --- /dev/null +++ b/tests/test_cli_exit.py @@ -0,0 +1,74 @@ +"""Tests for the hard-exit guard in ``raven.cli._exit``. + +The guard exists because native runtimes loaded by the agent loop segfault +during interpreter finalization; ``raven.cli.commands.run`` and the pytest +session both route their exit through it. These pin the guard's own behaviour +in-process, so the coverage does not depend on provoking a real native crash. + +Scope note, so these tests are not mistaken for proof the guard is load-bearing: +``flush_and_hard_exit`` is live (the pytest session calls it on CI), but the +``lancedb_finalization_hazard`` gate in ``commands.py`` is currently dormant -- +the memory plugin talks to everos over HTTP and opens no local lancedb +connection, so the probe returns False in every configuration today. +""" + +from __future__ import annotations + +import subprocess +import sys +import textwrap +import threading + +import pytest + +from raven.cli._exit import lancedb_finalization_hazard + +_LANCEDB_THREAD = "LanceDBBackgroundEventLoop" + + +def test_hazard_false_without_the_lancedb_thread() -> None: + if any(t.name == _LANCEDB_THREAD for t in threading.enumerate()): + pytest.skip("a lancedb connection is already open in this process") + assert lancedb_finalization_hazard() is False + + +def test_hazard_true_while_a_thread_of_that_name_is_alive() -> None: + """The probe keys on the thread name, not on lancedb being imported.""" + stop = threading.Event() + worker = threading.Thread(target=stop.wait, name=_LANCEDB_THREAD, daemon=True) + worker.start() + try: + assert lancedb_finalization_hazard() is True + finally: + stop.set() + worker.join(timeout=5) + assert lancedb_finalization_hazard() is False + + +def _hard_exit_child(code: int, *, prints: str = "") -> subprocess.CompletedProcess: + src = f""" + import sys + from raven.cli._exit import flush_and_hard_exit + + sys.stdout.write({prints!r}) + flush_and_hard_exit({code}) + sys.stdout.write("UNREACHABLE") + """ + return subprocess.run( + [sys.executable, "-c", textwrap.dedent(src)], + capture_output=True, + text=True, + timeout=60, + ) + + +def test_hard_exit_propagates_the_status_code() -> None: + assert _hard_exit_child(0).returncode == 0 + assert _hard_exit_child(3).returncode == 3 + + +def test_hard_exit_flushes_buffered_stdout_and_stops_the_interpreter() -> None: + """Buffered output survives the bypass, and nothing after the call runs.""" + result = _hard_exit_child(0, prints="buffered-before-exit") + assert result.stdout == "buffered-before-exit" + assert "UNREACHABLE" not in result.stdout diff --git a/tests/test_cl1_plugin_stack.py b/tests/test_cli_plugin_stack.py similarity index 99% rename from tests/test_cl1_plugin_stack.py rename to tests/test_cli_plugin_stack.py index 8241bd36..835f881f 100644 --- a/tests/test_cl1_plugin_stack.py +++ b/tests/test_cli_plugin_stack.py @@ -9,10 +9,6 @@ from pathlib import Path -import pytest - -pytest.importorskip("raven.plugin.memory.everos") - from raven.cli._plugin_stack import ( build_plugin_registry, maybe_build_memory_backend, diff --git a/tests/test_sandbox_cli.py b/tests/test_cli_sandbox_commands.py similarity index 100% rename from tests/test_sandbox_cli.py rename to tests/test_cli_sandbox_commands.py diff --git a/tests/test_cli_update_notice.py b/tests/test_cli_update_notice.py index 7bc7929d..4c9ce3d8 100644 --- a/tests/test_cli_update_notice.py +++ b/tests/test_cli_update_notice.py @@ -194,3 +194,30 @@ def _fetch_latest_release(self): # noqa: D102 - test double def _version_key(self, value): # noqa: D102 - test double raise RuntimeError(value) + + +# --------------------------------------------------------------------------- +# _upgrade_command_works: the real implementation, not the fixture's stub +# --------------------------------------------------------------------------- + + +def test_upgrade_command_works_follows_the_install_kind(monkeypatch) -> None: + """It delegates to the uv-tool probe, so editable installs are not nudged.""" + import raven.cli.upgrade_commands as upgrade + + monkeypatch.setattr(upgrade, "_is_uv_tool_install", lambda: True) + assert un._upgrade_command_works() is True + + monkeypatch.setattr(upgrade, "_is_uv_tool_install", lambda: False) + assert un._upgrade_command_works() is False + + +def test_upgrade_command_works_treats_a_probe_failure_as_no(monkeypatch) -> None: + """A malformed uv receipt raises; unknown must read as "cannot upgrade".""" + import raven.cli.upgrade_commands as upgrade + + def _boom() -> bool: + raise RuntimeError("malformed uv receipt") + + monkeypatch.setattr(upgrade, "_is_uv_tool_install", _boom) + assert un._upgrade_command_works() is False diff --git a/tests/test_config_cfg1.py b/tests/test_config_raven_sections.py similarity index 98% rename from tests/test_config_cfg1.py rename to tests/test_config_raven_sections.py index aa963a95..9d0dfa14 100644 --- a/tests/test_config_cfg1.py +++ b/tests/test_config_raven_sections.py @@ -1,4 +1,4 @@ -"""CFG-1 — RavenConfig: plugins / memory / skill_router sections + migration.""" +"""RavenConfig: plugins / memory / skill_router sections + migration.""" from __future__ import annotations @@ -8,6 +8,7 @@ from pathlib import Path import pytest +from pydantic import ValidationError from raven.config.loader import EXTENSION_KEYS from raven.config.raven import ( @@ -279,7 +280,7 @@ def test_no_legacy_field_no_warning(self, tmp_path: Path) -> None: class TestStrictness: def test_unknown_field_in_plugins_rejected(self) -> None: - with pytest.raises(Exception): + with pytest.raises(ValidationError): # ``extra='forbid'`` — typo catches at startup PluginsConfig.model_validate( { @@ -290,5 +291,5 @@ def test_unknown_field_in_plugins_rejected(self) -> None: ) def test_unknown_field_in_memory_rejected(self) -> None: - with pytest.raises(Exception): + with pytest.raises(ValidationError): MemoryConfig.model_validate({"backend": "x", "typo": 1}) diff --git a/tests/test_phase_a_default_engine.py b/tests/test_context_engine_factory.py similarity index 100% rename from tests/test_phase_a_default_engine.py rename to tests/test_context_engine_factory.py diff --git a/tests/test_decision_consumer.py b/tests/test_decision_consumer.py index 6db0c90b..a7eb3bf6 100644 --- a/tests/test_decision_consumer.py +++ b/tests/test_decision_consumer.py @@ -298,7 +298,7 @@ async def test_error_status_renders_user_facing_apology(pending_store): @pytest.mark.asyncio -async def test_decision_consumer_short_circuits_agent_loop(pending_store): +async def test_decision_consumer_short_circuits_agent_loop(pending_store, tmp_path): """Smoke test the decision_consumer hook on AgentLoop. We mock the consumer to check only that it's wired and short-circuits the process_message path without running the full LLM pipeline.""" @@ -317,7 +317,7 @@ async def _consumer_hook(req): channel=req.source.channel, chat_id=req.source.chat_id, content="✓ consumed by decision_consumer" ) - workspace = Path("/tmp") / f"ec-test-{_NOW_MS}" + workspace = tmp_path / "ws" workspace.mkdir(parents=True, exist_ok=True) loop = AgentLoop( @@ -334,7 +334,7 @@ async def _consumer_hook(req): @pytest.mark.asyncio -async def test_decision_consumer_falls_through_on_none(pending_store): +async def test_decision_consumer_falls_through_on_none(pending_store, tmp_path): """If consumer returns None, AgentLoop continues with normal flow. We can't easily test the full normal flow here without a full LLM setup — settle for verifying the hook is called and the result is @@ -347,7 +347,7 @@ async def _no_consume_hook(msg): calls["n"] += 1 return None - workspace = Path("/tmp") / f"ec-test-noconsume-{_NOW_MS}" + workspace = tmp_path / "ws" workspace.mkdir(parents=True, exist_ok=True) class _FakeProvider: @@ -376,8 +376,12 @@ class _R: msg = _msg("hello") try: await loop._process_message(msg) + except AssertionError: + # Never swallow an assertion: the fakes use them to signal "this must + # not be reached", so hiding one would turn a real failure into a pass. + raise except Exception: - # The fake provider may throw mid-flow; that's fine — we only - # care that the consumer was called once before the failure. + # Anything else from the full turn path is tolerated: this test only + # pins that the consumer hook fired before the turn continued. pass assert calls["n"] == 1 diff --git a/tests/test_deep_research_tool.py b/tests/test_deep_research_tool.py index a41c789f..989028b7 100644 --- a/tests/test_deep_research_tool.py +++ b/tests/test_deep_research_tool.py @@ -552,8 +552,13 @@ def _boom(**_kw): def test_both_deep_research_variants_accept_broker(): # The CLI double-bind loop over ("ask_user", "deep_research") wires whichever # variant is registered; both ask the user deep-vs-regular, so both take a broker. - assert hasattr(DeepResearchOfferTool, "set_broker") - assert hasattr(DeepResearchTool, "set_broker") + # Bind a real object rather than probing for the attribute: the wiring loop + # calls set_broker, so accepting the call is the contract, not merely existing. + broker = object() + for cls in (DeepResearchOfferTool, DeepResearchTool): + tool = cls.__new__(cls) + tool.set_broker(broker) + assert tool._broker is broker, f"{cls.__name__}.set_broker did not bind the broker" def test_deep_research_mode_two_states(monkeypatch): diff --git a/tests/test_em2_backend.py b/tests/test_everos_backend.py similarity index 99% rename from tests/test_em2_backend.py rename to tests/test_everos_backend.py index ef3269d7..8339ea60 100644 --- a/tests/test_em2_backend.py +++ b/tests/test_everos_backend.py @@ -16,8 +16,6 @@ import pytest -pytest.importorskip("raven.plugin.memory.everos") - from raven.memory_engine import MemoryBackend from raven.plugin import PluginContext, ServiceLocator from raven.plugin.memory.everos.backend import ( diff --git a/tests/test_em3_http.py b/tests/test_everos_http_adapter.py similarity index 99% rename from tests/test_em3_http.py rename to tests/test_everos_http_adapter.py index e17e6196..60c7a218 100644 --- a/tests/test_em3_http.py +++ b/tests/test_everos_http_adapter.py @@ -1,4 +1,4 @@ -"""EM-3 — EverosBackend HTTP mode (remote EverOS).""" +"""EverosBackend HTTP mode (remote EverOS).""" from __future__ import annotations @@ -9,8 +9,6 @@ import httpx import pytest -pytest.importorskip("raven.plugin.memory.everos") - from raven.plugin import PluginContext, ServiceLocator from raven.plugin.memory.everos.backend import ( EverosBackend, diff --git a/tests/test_em1_skeleton.py b/tests/test_everos_plugin_discovery.py similarity index 99% rename from tests/test_em1_skeleton.py rename to tests/test_everos_plugin_discovery.py index 754da64b..4965cf6f 100644 --- a/tests/test_em1_skeleton.py +++ b/tests/test_everos_plugin_discovery.py @@ -1,4 +1,4 @@ -"""EM-1 — everos plugin skeleton + end-to-end plugin discovery. +"""everos plugin skeleton + end-to-end plugin discovery. The EverOS backend ships **bundled** inside raven at ``raven/plugin/memory/everos/`` (not as an external entry-point diff --git a/tests/test_memory_store_lt_additions.py b/tests/test_memory_store_lt_additions.py index 69055da6..99136795 100644 --- a/tests/test_memory_store_lt_additions.py +++ b/tests/test_memory_store_lt_additions.py @@ -182,7 +182,7 @@ def writer(label: str, delay_inside_lock_s: float) -> None: with store.locked(): # Inside the lock, record observation order observed_orders.append(label) - current = store.read_long_term() + store.read_long_term() # Simulate slow LLM-driven section build time.sleep(delay_inside_lock_s) # Each writer claims its own H2 section diff --git a/tests/test_tier1_skeleton.py b/tests/test_package_skeleton.py similarity index 97% rename from tests/test_tier1_skeleton.py rename to tests/test_package_skeleton.py index 0d3eac7e..55163f8d 100644 --- a/tests/test_tier1_skeleton.py +++ b/tests/test_package_skeleton.py @@ -1,4 +1,4 @@ -"""Tier 1 smoke tests — verify the skeleton imports, +"""Package skeleton smoke tests - verify the skeleton imports, and the surviving interface ABCs behave correctly. These tests should pass on a fresh checkout with only Python stdlib and @@ -154,7 +154,7 @@ def test_config_safe_defaults(): assert cfg.skill_forge.auto_detect is False assert cfg.skill_forge.auto_evolve is False assert cfg.token_wise.smart_routing.enabled is False - # Baseline memory/skill feature layer defaults ON (EverOS R8 + CFG-1): a + # Baseline memory/skill feature layer defaults ON: a # fresh install runs the everos memory backend, the SkillForgeRouter, and # empty-response recovery. Pinned so a future silent flip gets caught. assert cfg.memory.backend == "everos" @@ -192,5 +192,5 @@ def test_tick_interval_seconds_rejects_sub_minute_values(): if __name__ == "__main__": - # Allow `python tests/test_tier1_skeleton.py` as a quick smoke run. + # Allow `python tests/test_package_skeleton.py` as a quick smoke run. pytest.main([__file__, "-v"]) diff --git a/tests/test_plugin_command.py b/tests/test_plugin_command.py index 6d0016c2..eec6cb41 100644 --- a/tests/test_plugin_command.py +++ b/tests/test_plugin_command.py @@ -14,11 +14,8 @@ from pathlib import Path from typing import Any -import pytest from typer.testing import CliRunner -pytest.importorskip("raven.plugin.memory.everos") - def _make_runner_args(tmp_path: Path, config: dict[str, Any]) -> list[str]: """Write a config file + return the typer args to point at it.""" diff --git a/tests/test_plugin_tools.py b/tests/test_plugin_tools.py index 49c36989..47cb186c 100644 --- a/tests/test_plugin_tools.py +++ b/tests/test_plugin_tools.py @@ -294,8 +294,7 @@ def test_no_media_returns_text(self) -> None: # EverOS understand_media tool (needs raven.plugin.memory.everos importable) # --------------------------------------------------------------------------- -pytest.importorskip("raven.plugin.memory.everos") -from raven.plugin.memory.everos.tools import UnderstandMediaTool, make_understand_media_tool # noqa: E402 +from raven.plugin.memory.everos.tools import UnderstandMediaTool, make_understand_media_tool class TestUnderstandMediaTool: diff --git a/tests/test_routine_learner_decay.py b/tests/test_routine_learner_decay.py index 88cdbbef..9bc16bea 100644 --- a/tests/test_routine_learner_decay.py +++ b/tests/test_routine_learner_decay.py @@ -58,20 +58,6 @@ def test_learn_with_decay_recent_routine_outweighs_stale_one(): now_fn=lambda: _NOW, ) - fresh_dates = [ - _NOW - timedelta(days=0, hours=0), - _NOW - timedelta(days=7, hours=0), - _NOW - timedelta(days=14, hours=0), - ] - # Stale routine — same Tuesday-9am bin but 30+ days old - stale_dates = [ - _NOW - timedelta(days=35), - _NOW - timedelta(days=42), - _NOW - timedelta(days=49), - ] - # Use different days-of-week so they're separate bins - # fresh: Friday 9am (weekday 4), stale: Tuesday 9am (weekday 1) - fresh_tuesdays = [d.replace(hour=9, minute=0) for d in [_NOW - timedelta(days=0)]] # Build entries for two clearly-distinct bins: fresh_entries = [] stale_entries = [] diff --git a/tests/test_routine_store.py b/tests/test_routine_store.py index b52657da..a71a3528 100644 --- a/tests/test_routine_store.py +++ b/tests/test_routine_store.py @@ -74,7 +74,7 @@ def test_merge_preserves_user_confirmed_status(tmp_path: Path): assert store.upgrade("dow1-h09-meeting", confirmed_at_ms=_NOW_MS + 60_000) is True # Second merge with refreshed stats — status should stay active refreshed = _routine(occurrence_count=8, weight=8.0, keywords=("meeting",)) - merged = store.merge([refreshed], now_ms=_NOW_MS + 120_000) + store.merge([refreshed], now_ms=_NOW_MS + 120_000) persisted = store.get("dow1-h09-meeting") assert persisted is not None diff --git a/tests/test_runtime_checkpoint_bug2.py b/tests/test_runtime_checkpoint.py similarity index 98% rename from tests/test_runtime_checkpoint_bug2.py rename to tests/test_runtime_checkpoint.py index 50ceee0f..f9e3efc1 100644 --- a/tests/test_runtime_checkpoint_bug2.py +++ b/tests/test_runtime_checkpoint.py @@ -1,4 +1,4 @@ -"""Bug2 — per-turn shadow-git checkpoint + max-iter interrupted handling. +"""Per-turn shadow-git checkpoint + max-iter interrupted handling. Covers the runtime-discipline safety net gated by ``config.runtime.checkpoint.policy`` × ``AgentLoop(interactive=...)``: @@ -173,13 +173,13 @@ async def test_max_iter_baseline_preserved_when_disabled(workspace): # Note: tests that spied on ``_trigger_local_extraction`` were removed when the # embedded extraction path was retired by feature/integrate-everos (Phase B-1). -# The Bug2 axiom "interrupted turn != completed turn" now lives in two places +# The axiom "interrupted turn != completed turn" now lives in two places # preserved by this merge: # 1. Shadow-git snapshot is taken regardless (see test_max_iter_snapshot...) # 2. ``outcome.status`` distinguishes interrupted vs completed for any caller # that wants to gate downstream actions on it (the new after-turn # pipeline at the caller level can choose to honor this — out of scope -# for Bug2 itself). +# for the checkpoint itself). # --- I5/I6: completed and error terminal states ------------------------------ diff --git a/tests/test_runtime_checkpoint_bug2_deep.py b/tests/test_runtime_checkpoint_deep.py similarity index 99% rename from tests/test_runtime_checkpoint_bug2_deep.py rename to tests/test_runtime_checkpoint_deep.py index c4d8cbe9..fbd59b61 100644 --- a/tests/test_runtime_checkpoint_bug2_deep.py +++ b/tests/test_runtime_checkpoint_deep.py @@ -1,6 +1,6 @@ -"""Bug2 deep tests — pathological inputs, edge state, and concurrency. +"""Deep tests - pathological inputs, edge state, and concurrency. -The base file ``test_runtime_checkpoint_bug2.py`` validates the happy path and +The base file ``test_runtime_checkpoint.py`` validates the happy path and core regressions; this file is the fail-safe hardening tier: - D1: filesystem pathology (unicode names, symlinks, deep nesting, file<->dir @@ -341,7 +341,7 @@ def _agent(workspace: Path, *, policy: str, interactive: bool) -> AgentLoop: def test_d6_policy_never_disables_checkpoint(workspace): """``policy="never"`` is the kill switch — no shadow git regardless of - interactive. Loop is byte-identical to the pre-Bug2 baseline.""" + interactive. Loop is byte-identical to the pre-checkpoint baseline.""" a_inter = _agent(workspace, policy="never", interactive=True) a_one_shot = _agent(workspace, policy="never", interactive=False) assert a_inter._checkpoint is None @@ -519,7 +519,7 @@ async def test_d8_one_shot_mode_creates_no_shadow_dir(workspace): # D9 — Containment safety: shadow_dir must stay strictly under the workspace # ============================================================================= # -# Without this check the per-workspace recovery isolation Bug2 depends on +# Without this check the per-workspace recovery isolation this suite depends on # silently breaks: a misconfigured ``shadow_dir`` can put the shadow git in # a sibling/global path and let a second AgentLoop on a different workspace # share the repo, cross-contaminating ``edited_files`` in the recovery diff --git a/tests/test_sandbox_unit.py b/tests/test_sandbox_unit.py index d94f9c64..cefb56ea 100644 --- a/tests/test_sandbox_unit.py +++ b/tests/test_sandbox_unit.py @@ -10,6 +10,7 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest +from pydantic import ValidationError from raven.sandbox import ( DirectExecutor, @@ -132,7 +133,7 @@ def test_extra_volumes_valid(self): assert c.extra_volumes == [["/data", "/data", "ro"]] def test_extra_config_key_rejected(self): - with pytest.raises(Exception): + with pytest.raises(ValidationError): SandboxConfig(unknown_key="x") # extra="forbid" def test_aliases_accept_both_camel_and_snake(self): @@ -1023,25 +1024,33 @@ async def test_stdio_sandboxed_no_spawning_raises(self): with pytest.raises(SandboxInitError, match="stdio transport"): await connect_mcp_servers({"svc": cfg}, ToolRegistry(), AsyncExitStack(), executor=executor) - async def test_stdio_no_executor_does_not_raise(self): + async def test_stdio_no_executor_does_not_raise(self, monkeypatch): """executor=None falls through to the normal stdio path (no guard triggered).""" from contextlib import AsyncExitStack + import mcp.client.stdio + from raven.agent.tools.mcp import connect_mcp_servers from raven.agent.tools.registry import ToolRegistry + reached = [] + + def fake_stdio_client(params): + reached.append(params.command) + raise RuntimeError("stdio_client reached — expected in test") + + monkeypatch.setattr(mcp.client.stdio, "stdio_client", fake_stdio_client) + cfg = MagicMock() cfg.type = "stdio" - cfg.command = "true" + cfg.command = "mcp-server" cfg.args = [] cfg.env = None cfg.tool_timeout = 30 - # connect will fail at stdio_client level (not installed / not available) but - # that error is caught per-server and logged — it must NOT be a SandboxInitError. - try: - await connect_mcp_servers({"svc": cfg}, ToolRegistry(), AsyncExitStack(), executor=None) - except SandboxInitError: - pytest.fail("SandboxInitError should not be raised when executor=None") + # Guard should NOT raise; the transport error is caught per-server and logged. + await connect_mcp_servers({"svc": cfg}, ToolRegistry(), AsyncExitStack(), executor=None) + + assert reached == ["mcp-server"] async def test_stdio_sandboxed_with_spawning_does_not_raise(self): """Sandboxed executor that supports spawning does not trigger the guard.""" @@ -1135,8 +1144,6 @@ async def start(self) -> None: async def stop(self) -> None: stopped.append(True) - original_build = None - async def fake_build(cfg, workspace): return TrackingExecutor() @@ -1161,8 +1168,6 @@ def _patched_build(cfg, workspace, owned_ids=None): subagent_mod.build_executor = _patched_build try: # Patch the inner method so the agent loop completes quickly - original_inner = manager._run_subagent_inner - async def _fast_inner(task_id, task, label, origin, executor): await manager._announce_result(task_id, label, task, "done", origin, "ok") diff --git a/tests/test_sentinel_nudge_and_pending.py b/tests/test_sentinel_nudge_and_pending.py index 6839d510..d2a7be84 100644 --- a/tests/test_sentinel_nudge_and_pending.py +++ b/tests/test_sentinel_nudge_and_pending.py @@ -152,7 +152,6 @@ def test_put_returns_empty_when_consumed_decision_present(tmp_path: Path): superseding it isn't a concern (user already picked or cancelled). Only un-consumed awaiting_confirm decisions matter.""" store = PendingDecisionStore(tmp_path / "pending.json") - consumed = _make_decision(decision_id="dec_consumed", consumed=True, created_at_ms=_NOW_MS - 1000) # Bypass the lifecycle methods — direct hand-poke for setup store.put(_make_decision(decision_id="dec_temp")) store.mark_consumed("dec_temp", picked_option_id="opt_1", consumed_at_ms=_NOW_MS - 500) @@ -191,7 +190,7 @@ async def test_discoverer_calls_policy_check_and_record_fired(memory_store, tmp_ # NudgePolicy should now know about the fire (next check on same # session for a "nudge" with same content would be denied via dedup) - second_check = policy.check( + policy.check( "nudge", session_key="feishu:ou_xxx", # Use the same menu preview that TaskDiscoverer used internally diff --git a/tests/test_skill_forge_phase_a.py b/tests/test_skill_forge_loader_parity.py similarity index 98% rename from tests/test_skill_forge_phase_a.py rename to tests/test_skill_forge_loader_parity.py index 71037e70..694dbb9e 100644 --- a/tests/test_skill_forge_phase_a.py +++ b/tests/test_skill_forge_loader_parity.py @@ -1,8 +1,7 @@ -"""Phase A acceptance tests for SkillForge. +"""SkillForge parity with the legacy SkillsLoader. -Phase A is the structural migration: ``agent/skills.py`` is dismantled -into ``skill_forge/{store,service,types}.py`` while preserving the exact -external behavior of the legacy ``SkillsLoader``. These tests pin that +``agent/skills.py`` was dismantled into ``skill_forge/{store,service,types}.py`` +while preserving the loader's exact external behavior. These tests pin that behavior down so future refactors can't drift. No LLM calls — everything runs offline against a synthetic workspace @@ -526,7 +525,7 @@ def test_skill_names_narrows_xml_directory(self, tmp_workspace, tmp_builtin, mon assert "always_flag_top" not in xml_block def test_empty_skill_names_falls_back_to_full_directory(self, tmp_workspace, tmp_builtin): - """Phase A stub selector returns [] — must fall back, not strip all.""" + """A stub selector returning [] must fall back, not strip all.""" from raven.agent.context import ContextBuilder from raven.memory_engine.skill_forge import LocalSkillCatalog diff --git a/tests/test_skill_router_sr4.py b/tests/test_skill_router_everos_source.py similarity index 97% rename from tests/test_skill_router_sr4.py rename to tests/test_skill_router_everos_source.py index e1355af6..3c3f5718 100644 --- a/tests/test_skill_router_sr4.py +++ b/tests/test_skill_router_everos_source.py @@ -1,4 +1,4 @@ -"""SR-4 — EverosSkillSource wraps MemoryBackend.recall + emits RouterHit.""" +"""EverosSkillSource wraps MemoryBackend.recall + emits RouterHit.""" from __future__ import annotations @@ -108,7 +108,7 @@ async def test_agent_id_does_not_change_across_calls( assert all(c["user_id"] is None for c in backend.recall_calls) async def test_history_not_forwarded(self, source, backend) -> None: - """For SR-4 we deliberately don't pass history through; the + """This source deliberately does not pass history through; the MemoryBackend Protocol has no field for it. Test pins the decision so future changes are conscious.""" history = [{"role": "user", "content": "earlier"}] diff --git a/tests/test_skill_router_sr2.py b/tests/test_skill_router_fusion.py similarity index 98% rename from tests/test_skill_router_sr2.py rename to tests/test_skill_router_fusion.py index f56747a5..f91b652c 100644 --- a/tests/test_skill_router_sr2.py +++ b/tests/test_skill_router_fusion.py @@ -1,4 +1,4 @@ -"""SR-2 — SkillForgeRouter weighted RRF fusion + concurrent fan-out + failure isolation.""" +"""SkillForgeRouter weighted RRF fusion + concurrent fan-out + failure isolation.""" from __future__ import annotations diff --git a/tests/test_skill_router_sr1.py b/tests/test_skill_router_local_source.py similarity index 99% rename from tests/test_skill_router_sr1.py rename to tests/test_skill_router_local_source.py index 65e0abfc..7ce6ed93 100644 --- a/tests/test_skill_router_sr1.py +++ b/tests/test_skill_router_local_source.py @@ -1,4 +1,4 @@ -"""SR-1 — SkillSource Protocol shape, LocalSkillSource emission, +"""SkillSource Protocol shape, LocalSkillSource emission, LocalSkillCatalog rendering. The Local source/catalog own the :class:`LocalPool` / diff --git a/tests/test_task_discoverer.py b/tests/test_task_discoverer.py index 7a0f21d7..969635ae 100644 --- a/tests/test_task_discoverer.py +++ b/tests/test_task_discoverer.py @@ -341,24 +341,6 @@ async def test_discoverer_drops_malformed_options_keeps_valid(memory_store, pend @pytest.mark.asyncio async def test_discoverer_rejects_routine_confirm_without_routine_id(memory_store, pending_store): dispatcher, _posted = _wire_dispatcher(lambda: _NOW) - response = _StubResponse( - [ - _option_dict( - title="bad routine", - type="routine_confirm", - exec_kind="routine_confirm", - exec_payload={}, # missing routine_id - ), - _option_dict( - title="good routine", - type="routine_confirm", - exec_kind="routine_confirm", - exec_payload={"routine_id": "dow1-h09-meeting", "make_cron": True}, - ), - ] - ) - provider = _StubProvider(response) - # pad with 2 ad_hoc options so we get the minimum 3 response = _StubResponse( [ diff --git a/tests/test_tools_registry.py b/tests/test_tool_registry_execute.py similarity index 100% rename from tests/test_tools_registry.py rename to tests/test_tool_registry_execute.py diff --git a/tests/test_tui_rpc_session_init_bundle.py b/tests/test_tui_rpc_session_init_bundle.py index 46e94882..5ee774e8 100644 --- a/tests/test_tui_rpc_session_init_bundle.py +++ b/tests/test_tui_rpc_session_init_bundle.py @@ -314,3 +314,33 @@ def test_boot_context_max_uses_live_window_for_openrouter(config, monkeypatch) - info = _default_session_info(loop, config) assert info["usage"]["context_max"] == 163840 + + +# --------------------------------------------------------------------------- +# Upgrade-nudge fields (the producer side; the TUI already reads them) +# --------------------------------------------------------------------------- + + +def test_default_session_info_carries_the_upgrade_nudge(fake_agent_loop, config, monkeypatch) -> None: + """A pending release surfaces as ``update_available`` / ``update_command``. + + The TUI status bar reads both fields, so leaving them unpopulated is the + exact defect the nudge feature fixed -- and nothing else in the suite fails + if this wiring is removed. + """ + monkeypatch.setattr(session_module, "update_notice", lambda _v: (True, "raven upgrade")) + + info = _default_session_info(fake_agent_loop, config) + + assert info["update_available"] is True + assert info["update_command"] == "raven upgrade" + + +def test_default_session_info_omits_the_nudge_when_up_to_date(fake_agent_loop, config, monkeypatch) -> None: + """No pending release means the keys stay absent, not present-and-false.""" + monkeypatch.setattr(session_module, "update_notice", lambda _v: None) + + info = _default_session_info(fake_agent_loop, config) + + assert "update_available" not in info + assert "update_command" not in info diff --git a/tests/test_tui_rpc_slash_routing.py b/tests/test_tui_rpc_slash_routing.py index 30e4f1aa..ca07fc1f 100644 --- a/tests/test_tui_rpc_slash_routing.py +++ b/tests/test_tui_rpc_slash_routing.py @@ -53,6 +53,18 @@ def echo(text: str) -> None: def boom() -> None: raise click.UsageError("bad arg") + # A real two-token subcommand, so the space-splitting test has something to + # split: hermes sends `/channels status` as command="channels status". + channels = typer.Typer(no_args_is_help=False) + + @channels.command("status") + def channels_status() -> None: + import raven.cli.commands as ec_commands + + ec_commands.console.print("channels: ok") + + fake.add_typer(channels, name="channels") + return fake @@ -87,18 +99,9 @@ async def test_slash_exec_routes_to_cli_dispatch_status(fake_app_patch): async def test_slash_exec_routes_channels_status_with_space(fake_app_patch): """``/channels status`` arrives as command="channels status" — needs split.""" - - # Add a channels-status fake command to verify dispatch routing. - @fake_app_patch.command(name="channels-status") - def _channels_status() -> None: # pragma: no cover (registered for routing) - import raven.cli.commands as ec_commands - - ec_commands.console.print("channels: ok") - - # The real path uses the EC CLI's `channels status` (already whitelisted). - # We use the patched echo as a stand-in to keep the test isolated. - result = await slash_exec({"command": "echo hello-tui", "session_id": "sid-abc"}) - assert "hello-tui" in result["output"] + result = await slash_exec({"command": "channels status", "session_id": "sid-abc"}) + assert "channels: ok" in result["output"], f"two-token subcommand did not run; got {result!r}" + assert result.get("warning") in (None, "") async def test_slash_exec_shlex_quoted_args(fake_app_patch): diff --git a/tests/test_tui_rpc_turn_cancel.py b/tests/test_tui_rpc_turn_cancel.py index 33abf706..4dda3d35 100644 --- a/tests/test_tui_rpc_turn_cancel.py +++ b/tests/test_tui_rpc_turn_cancel.py @@ -16,6 +16,7 @@ from unittest.mock import AsyncMock import pytest +from pydantic import ValidationError from raven.tui_rpc.dispatcher import Dispatcher from raven.tui_rpc.methods.turn import ( @@ -153,7 +154,7 @@ async def test_turn_cancel_keeps_subscription_open_for_next_turn( async def test_turn_cancel_rejects_missing_session_key(emitter: SubscriptionEmitter) -> None: - with pytest.raises(Exception): # noqa: BLE001 + with pytest.raises(ValidationError): await turn_cancel({}, emitter=emitter) diff --git a/tests/test_tui_rpc_turn_send.py b/tests/test_tui_rpc_turn_send.py index 58a68a7a..ad4a4fda 100644 --- a/tests/test_tui_rpc_turn_send.py +++ b/tests/test_tui_rpc_turn_send.py @@ -14,6 +14,7 @@ from unittest.mock import patch import pytest +from pydantic import ValidationError from raven.tui_rpc.dispatcher import Dispatcher from raven.tui_rpc.errors import ModelNotAvailableError, RpcError, TurnInProgressError @@ -191,12 +192,12 @@ class _BuildErr(RpcError): async def test_turn_send_rejects_missing_session_key() -> None: - with pytest.raises(Exception): # noqa: BLE001 + with pytest.raises(ValidationError): await turn_send({"content": "missing session_key"}, scheduler=FakeScheduler()) async def test_turn_send_rejects_missing_content() -> None: - with pytest.raises(Exception): # noqa: BLE001 + with pytest.raises(ValidationError): await turn_send({"session_key": "tui:default"}, scheduler=FakeScheduler()) diff --git a/tests/test_tui_rpc_turn_subscribe.py b/tests/test_tui_rpc_turn_subscribe.py index cfb48727..74e155df 100644 --- a/tests/test_tui_rpc_turn_subscribe.py +++ b/tests/test_tui_rpc_turn_subscribe.py @@ -11,6 +11,7 @@ from unittest.mock import AsyncMock import pytest +from pydantic import ValidationError from raven.tui_rpc.dispatcher import Dispatcher from raven.tui_rpc.methods.turn import ( @@ -64,7 +65,7 @@ async def test_turn_subscribe_rejects_missing_session_key( emitter: SubscriptionEmitter, ) -> None: """Missing required ``session_key`` → validation error.""" - with pytest.raises(Exception): # noqa: BLE001 + with pytest.raises(ValidationError): await turn_subscribe({}, emitter=emitter) diff --git a/tests/tui/autotest/statusbar.py b/tests/tui/autotest/statusbar.py new file mode 100644 index 00000000..552bb870 --- /dev/null +++ b/tests/tui/autotest/statusbar.py @@ -0,0 +1,20 @@ +"""Status-bar patterns the e2e tests read turn state from. + +Mirrored from ``ui-tui/src/content/verbs.ts``; kept in one place so a verb-pool +edit there has a single place to land rather than one copy per test file. +""" + +from __future__ import annotations + +import re + +# Idle turn state (word-bounded so "readiness" won't match). +READY_RE = re.compile(r"\bready\b", re.IGNORECASE) + +# Working-state verbs shown while a turn is in flight, mirroring VERBS. +WORKING_RE = re.compile( + r"\b(pondering|contemplating|musing|cogitating|ruminating|deliberating|" + r"mulling|reflecting|processing|reasoning|analyzing|computing|" + r"synthesizing|formulating|brainstorming)…", + re.IGNORECASE, +) diff --git a/tests/tui/autotest/tests/test_e2e_raven_tui_chat.py b/tests/tui/autotest/tests/test_e2e_raven_tui_chat.py index 576bb761..d0f25824 100644 --- a/tests/tui/autotest/tests/test_e2e_raven_tui_chat.py +++ b/tests/tui/autotest/tests/test_e2e_raven_tui_chat.py @@ -33,22 +33,13 @@ import pytest from tests.tui.autotest.runner import BackendError +from tests.tui.autotest.statusbar import READY_RE as _READY_RE +from tests.tui.autotest.statusbar import WORKING_RE as _WORKING_RE # Content-neutral prompt: we never assert WHAT the model says, only that the # pipeline ran the turn. _PROMPT = "Reply with a short friendly sentence." -# Working-state verbs the status bar shows while a turn is in flight, mirrored -# from ui-tui/src/content/verbs.ts (VERBS). Keep in sync with that pool. -_WORKING_RE = re.compile( - r"\b(pondering|contemplating|musing|cogitating|ruminating|deliberating|" - r"mulling|reflecting|processing|reasoning|analyzing|computing|" - r"synthesizing|formulating|brainstorming)…", - re.IGNORECASE, -) -# Idle turn state in the status bar (word-bounded so "readiness" won't match). -_READY_RE = re.compile(r"\bready\b", re.IGNORECASE) - @pytest.mark.e2e def test_tui_chat_round_trip(harness): @@ -56,15 +47,20 @@ def test_tui_chat_round_trip(harness): # runs as a regular live E2E ACCEPTANCE for the chat streaming path # through the TUI. harness.spawn("uv run raven tui") - # Note: tui-use snapshot returns ALT-SCREEN rendered text only — the 🦞 - # emoji visible in tui-use wait --text (which searches full stream incl. - # scrollback) is NOT in snapshot once Ink switches to alt-screen. - # Use "Raven" brand text (alt-screen-visible) for readiness instead. - assert harness.wait(r"Raven", timeout=25.0), ( - f"TUI Raven readiness banner not seen in 25s; screen=\n{harness.screen()}" + # Wait for the status bar to report ready, not merely for the banner: the + # banner paints seconds before the app accepts input, so keying off it drops + # the prompt and the turn never starts. + assert harness.wait(_READY_RE, timeout=45.0), ( + f"TUI status bar never reported ready in 45s; screen=\n{harness.screen()}" ) harness.type(_PROMPT) + # Confirm the composer actually received the text before submitting: typing + # and pressing enter back to back can outrun the render, and the dropped + # prompt then looks like a dead pipeline. + assert harness.wait(re.escape(_PROMPT), timeout=10.0), ( + f"composer never showed the typed prompt; screen=\n{harness.screen()}" + ) harness.press("enter") # Liveness, content-agnostic: the pipeline accepts the prompt (status bar @@ -105,3 +101,86 @@ def test_tui_chat_round_trip(harness): except BackendError: pass # already exiting after the first Ctrl+C assert harness.expect_exit(0, timeout=10.0), f"TUI did not exit 0 after Ctrl+C; final screen=\n{harness.screen()}" + + +def _await_ready(harness) -> None: + """Block until the status bar reports an idle turn state.""" + assert harness.wait(_READY_RE, timeout=45.0), ( + f"TUI status bar never reported ready in 45s; screen=\n{harness.screen()}" + ) + + +def _run_turn(harness, prompt: str) -> None: + """Submit one prompt and return once the turn has started and settled.""" + harness.type(prompt) + assert harness.wait(re.escape(prompt), timeout=10.0), ( + f"composer never showed {prompt!r}; screen=\n{harness.screen()}" + ) + harness.press("enter") + + deadline = time.monotonic() + 20.0 + while time.monotonic() < deadline: + screen = harness.screen() + if re.search(r"error:\s*model_not_available", screen): + pytest.skip("default model returned model_not_available; configure an accessible model and re-run.") + if _WORKING_RE.search(screen): + break + time.sleep(0.2) + else: + pytest.fail(f"turn never started for {prompt!r}; screen=\n{harness.screen()}") + + assert harness.wait(_READY_RE, timeout=60.0), f"turn never completed for {prompt!r}; screen=\n{harness.screen()}" + + +@pytest.mark.e2e +def test_tui_chat_multi_turn_accumulates_the_session(harness): + """Three turns land in one session file, in order, each with a reply. + + The round-trip above proves one turn runs. This proves turns accumulate + rather than each starting fresh. The evidence is the persisted session, not + the screen: a prompt is echoed into the transcript as soon as it is typed, + so screen-scraping for a planted word passes even when history is broken. + """ + from raven.cli._helpers import load_runtime_config + from raven.session.manager import SessionManager + + sessions = SessionManager(load_runtime_config(None, None).workspace_path) + before = sessions.find_most_recent_chat_id("tui") + + harness.spawn("uv run raven tui") + _await_ready(harness) + + prompts = [ + "Reply with just the word alpha.", + "Reply with just the word bravo.", + "Reply with just the word charlie.", + ] + for prompt in prompts: + _run_turn(harness, prompt) + + harness.press("ctrl+c") + time.sleep(0.5) + try: + harness.press("ctrl+c") + except BackendError: + pass + assert harness.expect_exit(0, timeout=10.0), f"TUI did not exit 0 after Ctrl+C; final screen=\n{harness.screen()}" + + # The run created a fresh tui session; it is now the most recent one. + chat_id = sessions.find_most_recent_chat_id("tui") + assert chat_id and chat_id != before, ( + f"the run did not create a new tui session (before={before!r}, after={chat_id!r})" + ) + session = sessions.peek(f"tui:{chat_id}") + assert session is not None, f"no persisted session for tui:{chat_id}" + + user_turns = [m.get("content", "") for m in session.get_history() if m.get("role") == "user"] + for prompt in prompts: + assert prompt in user_turns, f"turn missing from the persisted session: {prompt!r}; got {user_turns!r}" + assert user_turns.index(prompts[0]) < user_turns.index(prompts[1]) < user_turns.index(prompts[2]), ( + f"turns did not accumulate in order: {user_turns!r}" + ) + replies = [m for m in session.get_history() if m.get("role") == "assistant"] + assert len(replies) >= len(prompts), ( + f"expected one assistant reply per turn, got {len(replies)} for {len(prompts)} prompts" + ) diff --git a/tests/tui/autotest/tests/test_e2e_streaming_no_log_overlay.py b/tests/tui/autotest/tests/test_e2e_streaming_no_log_overlay.py index 4ced4782..8d3af98c 100644 --- a/tests/tui/autotest/tests/test_e2e_streaming_no_log_overlay.py +++ b/tests/tui/autotest/tests/test_e2e_streaming_no_log_overlay.py @@ -27,6 +27,8 @@ import pytest from tests.tui.autotest.runner import BackendError +from tests.tui.autotest.statusbar import READY_RE as _READY_RE +from tests.tui.autotest.statusbar import WORKING_RE as _WORKING_RE _LEAK_RE = re.compile( r"LiteLLM:(DEBUG|INFO|WARNING|ERROR)" @@ -39,9 +41,16 @@ @pytest.mark.e2e def test_tui_chat_streaming_no_log_overlay(harness): + prompt = "Reply in exactly 30 words about anything." harness.spawn("uv run raven tui") - assert harness.wait(r"Raven", timeout=25.0), f"TUI did not reach banner; screen=\n{harness.screen()}" - harness.type("Reply in exactly 30 words about anything.") + # Key off the status bar, not the banner, and confirm the composer took the + # text before submitting. The assertion below is negative (no leak), so a + # dropped prompt would mean no streaming, no leak, and a vacuous pass. + assert harness.wait(_READY_RE, timeout=45.0), f"TUI status bar never reported ready; screen=\n{harness.screen()}" + harness.type(prompt) + assert harness.wait(re.escape(prompt), timeout=10.0), ( + f"composer never showed the typed prompt; screen=\n{harness.screen()}" + ) harness.press("enter") # If the configured default model isn't accessible (e.g. claude-sonnet-4-6 @@ -54,6 +63,12 @@ def test_tui_chat_streaming_no_log_overlay(harness): "accessible model configured as default (e.g. openrouter/qwen)." ) + # Positive precondition before a negative assertion: the turn must actually + # be streaming, or "no leak" proves nothing. + assert harness.wait(_WORKING_RE, timeout=20.0), ( + f"turn never started, so there was no streaming to leak from; screen=\n{harness.screen()}" + ) + # Race the leak pattern against the streaming response. If a log line # surfaces on the alt-screen at any moment in the next 30 s, fail with # the captured frame so the fix author can see the exact leaked text. diff --git a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts index d5836d91..560d5263 100644 --- a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts +++ b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts @@ -816,7 +816,7 @@ describe('createGatewayEventHandler', () => { } finally { // Drain pending fake timers BEFORE restoring real timers so a mid- // test assertion failure can't leak the interrupt-cooldown setTimeout - // across test files (the original Copilot concern). + // across test files. vi.runAllTimers() vi.useRealTimers() } diff --git a/ui-tui/src/__tests__/createSlashHandler.test.ts b/ui-tui/src/__tests__/createSlashHandler.test.ts index b26cf3d3..d9143938 100644 --- a/ui-tui/src/__tests__/createSlashHandler.test.ts +++ b/ui-tui/src/__tests__/createSlashHandler.test.ts @@ -227,11 +227,10 @@ describe('createSlashHandler', () => { expect(ctx.gateway.gw.request).not.toHaveBeenCalled() }) - // Regressions from Copilot review on #19835: /voice output + frontend - // binding state must both track the gateway's fresh ``record_key`` on - // every response, or a config edit shows the new shortcut in text - // while push-to-talk still fires the old one until the next mtime - // poll (~5s). + // /voice output and frontend binding state must both track the + // gateway's fresh ``record_key`` on every response, or a config edit + // shows the new shortcut in text while push-to-talk still fires the + // old one until the next mtime poll (~5s). it('/voice status renders the gateway record_key and pushes it into frontend state', async () => { const rpc = vi.fn(() => Promise.resolve({ enabled: true, record_key: 'ctrl+space', tts: false })) const ctx = buildCtx({ gateway: { ...buildGateway(), rpc } }) @@ -267,11 +266,11 @@ describe('createSlashHandler', () => { }) }) - // Round-2 Copilot review on #19835: a response missing ``record_key`` - // (e.g. the old tts branch, or any future branch that forgets to - // include it) MUST NOT clobber the user's cached binding back to - // Ctrl+B. The label still renders the default for display; the - // frontend state keeps whatever was last authoritatively set. + // A response missing ``record_key`` (e.g. the old tts branch, or any + // future branch that forgets to include it) MUST NOT clobber the + // user's cached binding back to Ctrl+B. The label still renders the + // default for display; the frontend state keeps whatever was last + // authoritatively set. it('/voice tts without record_key does not clobber cached frontend binding', async () => { const rpc = vi.fn(() => Promise.resolve({ enabled: true, tts: true })) const ctx = buildCtx({ gateway: { ...buildGateway(), rpc } }) diff --git a/ui-tui/src/__tests__/episodeSummary.test.ts b/ui-tui/src/__tests__/episodeSummary.test.ts index 9c3ab32e..1dfb477f 100644 --- a/ui-tui/src/__tests__/episodeSummary.test.ts +++ b/ui-tui/src/__tests__/episodeSummary.test.ts @@ -52,7 +52,13 @@ describe('toolsSummary', () => { expect(toolsSummary([tool('web_search', 'hermes agent')])).toBe('searched "hermes agent"') }) - it('shows +added -removed for an edited file', () => {}) + it('shows +added -removed for an edited file', () => { + const edited = { ...tool('edit_file', 'notes.md'), added: 12, removed: 3 } + expect(toolsSummary([edited])).toBe('edited notes.md (+12 -3)') + // A missing side reads as zero, and no stats at all means no suffix. + expect(toolsSummary([{ ...tool('edit_file', 'notes.md'), added: 5 }])).toBe('edited notes.md (+5 -0)') + expect(toolsSummary([tool('edit_file', 'notes.md')])).toBe('edited notes.md') + }) it('splits a tool row into verb + detail (full path, not just basename)', () => { expect(toolParts(tool('read_file', 'src/approve.go'))).toEqual({ verb: 'read', detail: 'src/approve.go' }) diff --git a/ui-tui/src/__tests__/platform.test.ts b/ui-tui/src/__tests__/platform.test.ts index 592d543c..fa2b3f22 100644 --- a/ui-tui/src/__tests__/platform.test.ts +++ b/ui-tui/src/__tests__/platform.test.ts @@ -77,9 +77,8 @@ describe('isVoiceToggleKey', () => { expect(isVoiceToggleKey({ ctrl: false, meta: false, super: true }, 'b')).toBe(true) // ``key.meta`` is NOT accepted as Cmd — raven-ink uses meta for - // Alt too, so accepting it leaked Alt+B into the default binding - // (Copilot round-6 review on #19835). Legacy-terminal mac users - // get strict Ctrl+B. + // Alt too, so accepting it leaked Alt+B into the default + // binding. Legacy-terminal mac users get strict Ctrl+B. expect(isVoiceToggleKey({ ctrl: false, meta: true, super: false }, 'b')).toBe(false) }) @@ -98,7 +97,7 @@ describe('isVoiceToggleKey', () => { }) }) -describe('parseVoiceRecordKey (#18994)', () => { +describe('parseVoiceRecordKey', () => { it('falls back to Ctrl+B for empty input', async () => { const { DEFAULT_VOICE_RECORD_KEY, parseVoiceRecordKey } = await importPlatform('linux') @@ -127,9 +126,9 @@ describe('parseVoiceRecordKey (#18994)', () => { // ``meta`` / ``cmd`` / ``command`` are ambiguous on the wire: // raven-ink sets ``key.meta`` for plain Alt on every platform AND // for Cmd on legacy macOS terminals. Accepting any of them would - // produce a display/binding mismatch (Copilot round-6 review on - // #19835). Users on modern kitty-style terminals spell the - // platform action modifier ``super`` / ``win``. + // produce a display/binding mismatch. Users on modern kitty-style + // terminals spell the platform action modifier ``super`` / + // ``win``. expect(parseVoiceRecordKey('meta+b')).toEqual(DEFAULT_VOICE_RECORD_KEY) expect(parseVoiceRecordKey('cmd+b')).toEqual(DEFAULT_VOICE_RECORD_KEY) expect(parseVoiceRecordKey('command+b')).toEqual(DEFAULT_VOICE_RECORD_KEY) @@ -166,7 +165,6 @@ describe('parseVoiceRecordKey (#18994)', () => { expect(parseVoiceRecordKey('ctrl+f5')).toEqual(DEFAULT_VOICE_RECORD_KEY) }) - // Round-3 Copilot review regressions on #19835. it('does not throw on non-string YAML scalars — falls back instead', async () => { const { DEFAULT_VOICE_RECORD_KEY, parseVoiceRecordKey } = await importPlatform('linux') @@ -190,7 +188,6 @@ describe('parseVoiceRecordKey (#18994)', () => { expect(parseVoiceRecordKey('alt+ctrl+space')).toEqual(DEFAULT_VOICE_RECORD_KEY) }) - // Round-4 Copilot review regressions on #19835. it('rejects bare-char configs without an explicit modifier', async () => { const { DEFAULT_VOICE_RECORD_KEY, parseVoiceRecordKey } = await importPlatform('linux') @@ -217,7 +214,7 @@ describe('parseVoiceRecordKey (#18994)', () => { expect(parseVoiceRecordKey('alt+c').mod).toBe('alt') // ``ctrl+x`` is intentionally allowed — only intercepted during // queue-edit (``queueEditIdx !== null``), so the voice binding - // works for most of the session (Copilot round-8 review). + // works for most of the session. expect(parseVoiceRecordKey('ctrl+x').mod).toBe('ctrl') expect(parseVoiceRecordKey('ctrl+x').ch).toBe('x') }) @@ -242,7 +239,7 @@ describe('parseVoiceRecordKey (#18994)', () => { // Kitty/CSI-u users on non-mac report Cmd/Super as ``key.super``, // but the TUI's global shortcuts (copy/exit/clear/paste) key off // Ctrl there, so ``super+`` doesn't collide. Reject would - // silently coerce valid configs to Ctrl+B (Copilot round-8 review). + // silently coerce valid configs to Ctrl+B. expect(parseVoiceRecordKey('super+c').mod).toBe('super') expect(parseVoiceRecordKey('super+d').mod).toBe('super') expect(parseVoiceRecordKey('super+l').mod).toBe('super') @@ -256,7 +253,7 @@ describe('parseVoiceRecordKey (#18994)', () => { // ``isActionMod`` on darwin accepts ``key.meta`` as the action // modifier. So ``alt+c`` / ``alt+d`` / ``alt+l`` get claimed by // isCopyShortcut / isAction('d') / isAction('l') before voice - // runs (Copilot round-12 on #19835). + // runs. expect(parseVoiceRecordKey('alt+c')).toEqual(DEFAULT_VOICE_RECORD_KEY) expect(parseVoiceRecordKey('alt+d')).toEqual(DEFAULT_VOICE_RECORD_KEY) expect(parseVoiceRecordKey('alt+l')).toEqual(DEFAULT_VOICE_RECORD_KEY) @@ -275,7 +272,6 @@ describe('parseVoiceRecordKey (#18994)', () => { expect(parseVoiceRecordKey('alt+l').mod).toBe('alt') }) - // Round-5 Copilot review regressions on #19835. it('super+ does NOT fire on key.meta-only events (Alt+X false-fire guard)', async () => { const { isVoiceToggleKey, parseVoiceRecordKey } = await importPlatform('darwin') @@ -292,7 +288,6 @@ describe('parseVoiceRecordKey (#18994)', () => { expect(isVoiceToggleKey({ ctrl: false, escape: true, meta: true, super: false }, '', superEscape)).toBe(false) }) - // Round-6 Copilot review regressions on #19835. it('default ctrl+b does NOT fire on Alt+B via isActionMod meta leak', async () => { const { DEFAULT_VOICE_RECORD_KEY, isVoiceToggleKey } = await importPlatform('darwin') @@ -366,7 +361,7 @@ describe('parseVoiceRecordKey (#18994)', () => { }) }) -describe('formatVoiceRecordKey (#18994)', () => { +describe('formatVoiceRecordKey', () => { it('renders as the user expects in /voice status', async () => { const { formatVoiceRecordKey, parseVoiceRecordKey } = await importPlatform('linux') @@ -389,7 +384,7 @@ describe('formatVoiceRecordKey (#18994)', () => { }) }) -describe('isVoiceToggleKey honours configured record key (#18994)', () => { +describe('isVoiceToggleKey honours configured record key', () => { it('binds the configured letter, not hardcoded b', async () => { const { isVoiceToggleKey, parseVoiceRecordKey } = await importPlatform('linux') const ctrlO = parseVoiceRecordKey('ctrl+o') @@ -445,12 +440,12 @@ describe('isVoiceToggleKey honours configured record key (#18994)', () => { expect(isVoiceToggleKey({ ctrl: true, meta: false, super: false }, 'o')).toBe(false) }) - // Regressions from Copilot review on #19835: the previous implementation - // accepted ``isActionMod(key)`` in the ``ctrl`` branch for every - // configured key, so bare Esc (which raven-ink reports with - // ``key.meta`` on some macOS terminals) fired ``ctrl+escape``, and - // Alt+Space / Alt+Tab fired ``ctrl+space`` / ``ctrl+tab``. The fallback - // is now gated to the documented default (``ctrl+b``) only. + // The previous implementation accepted ``isActionMod(key)`` in the + // ``ctrl`` branch for every configured key, so bare Esc (which + // raven-ink reports with ``key.meta`` on some macOS terminals) fired + // ``ctrl+escape``, and Alt+Space / Alt+Tab fired ``ctrl+space`` / + // ``ctrl+tab``. The fallback is now gated to the documented default + // (``ctrl+b``) only. it('ctrl+escape does NOT fire on bare Esc via key.meta on macOS', async () => { const { isVoiceToggleKey, parseVoiceRecordKey } = await importPlatform('darwin') const ctrlEscape = parseVoiceRecordKey('ctrl+escape') @@ -480,7 +475,7 @@ describe('isVoiceToggleKey honours configured record key (#18994)', () => { expect(isVoiceToggleKey({ ctrl: false, meta: false, super: true }, 'b', DEFAULT_VOICE_RECORD_KEY)).toBe(true) // Cmd+B via legacy ``key.meta`` NO LONGER works — ``key.meta`` is // raven-ink's Alt signal, so accepting it leaked Alt+B into the - // default binding (Copilot round-6 review on #19835). + // default binding. expect(isVoiceToggleKey({ ctrl: false, meta: true, super: false }, 'b', DEFAULT_VOICE_RECORD_KEY)).toBe(false) }) @@ -503,14 +498,12 @@ describe('isVoiceToggleKey honours configured record key (#18994)', () => { // Kitty-style: key.super fires the binding. expect(isVoiceToggleKey({ ctrl: false, meta: false, super: true }, 'b', superB)).toBe(true) // ``key.meta`` is NOT accepted — raven-ink uses meta for Alt too, - // so accepting it here would make super+b silently fire on Alt+B - // (Copilot round-5 review on #19835). + // so accepting it here would make super+b silently fire on Alt+B. expect(isVoiceToggleKey({ ctrl: false, meta: true, super: false }, 'b', superB)).toBe(false) // Ctrl held at the same time → reject (different chord). expect(isVoiceToggleKey({ ctrl: true, meta: false, super: true }, 'b', superB)).toBe(false) }) - // Round-2 Copilot review regressions on #19835. it('super+b renders "Super+B" on Linux (not "Cmd+B")', async () => { const { formatVoiceRecordKey, parseVoiceRecordKey } = await importPlatform('linux') diff --git a/ui-tui/src/__tests__/reasoning.test.ts b/ui-tui/src/__tests__/reasoning.test.ts index 2618d883..cb874ac9 100644 --- a/ui-tui/src/__tests__/reasoning.test.ts +++ b/ui-tui/src/__tests__/reasoning.test.ts @@ -5,7 +5,7 @@ import { describe, expect, it } from 'vitest' -import { hasReasoningTag, splitReasoning } from '../lib/reasoning.js' +import { hasMeaningfulReasoning, hasReasoningTag, splitReasoning } from '../lib/reasoning.js' import { cleanThinkingText } from '../lib/text.js' describe('splitReasoning', () => { @@ -64,3 +64,18 @@ describe('cleanThinkingText', () => { ).toBe('**Resolving comments on GitHub**\nActual step\nnext step') }) }) + +describe('hasMeaningfulReasoning', () => { + it('rejects placeholder bursts that carry no words', () => { + // Some models emit reasoning_content that is only dots during a mechanical + // tool loop; showing that as thought is noise. + expect(hasMeaningfulReasoning('.\n.\n.')).toBe(false) + expect(hasMeaningfulReasoning('')).toBe(false) + expect(hasMeaningfulReasoning(' ')).toBe(false) + }) + + it('accepts words in any script', () => { + expect(hasMeaningfulReasoning('weighing the options')).toBe(true) + expect(hasMeaningfulReasoning('step 2')).toBe(true) + }) +}) diff --git a/ui-tui/src/__tests__/slashParity.test.ts b/ui-tui/src/__tests__/slashParity.test.ts index 4deea007..58b5f2a2 100644 --- a/ui-tui/src/__tests__/slashParity.test.ts +++ b/ui-tui/src/__tests__/slashParity.test.ts @@ -3,22 +3,14 @@ // Modifications Copyright (c) 2026 EverMind. // See NOTICES.md and LICENSES/MIT-hermes-agent.txt. -import { execFileSync } from 'node:child_process' -import { dirname, resolve } from 'node:path' -import { fileURLToPath } from 'node:url' import { describe, expect, it } from 'vitest' -import { SLASH_COMMANDS } from '../app/slash/registry.js' - -type CommandRoute = 'fallback' | 'local' | 'native' - -interface CommandRegistryLoad { - error?: string - names: string[] -} - -const NATIVE_MUTATING_COMMANDS = new Set(['browser', 'busy', 'fast', 'reload-mcp', 'rollback', 'stop']) +import { findSlashCommand } from '../app/slash/registry.js' +// A command that changes session state has to run in this process. +// createSlashHandler.ts:45-51 routes a slash locally when findSlashCommand +// resolves it and otherwise hands it to slash.exec, which runs the CLI in a +// subprocess -- where a mutation cannot reach the live session at all. const MUTATING_COMMANDS = [ 'background', 'branch', @@ -45,73 +37,17 @@ const MUTATING_COMMANDS = [ 'yolo' ] as const -const loadCommandRegistryNames = (): CommandRegistryLoad => { - const here = dirname(fileURLToPath(import.meta.url)) - - try { - const names = JSON.parse( - execFileSync( - process.env.PYTHON ?? 'python3', - [ - '-c', - 'import json; from raven_cli.commands import COMMAND_REGISTRY; print(json.dumps([c.name for c in COMMAND_REGISTRY]))' - ], - { cwd: resolve(here, '../../..'), encoding: 'utf8' } - ) - ) as string[] - - return { names: [...new Set(names)] } - } catch (error) { - return { - error: error instanceof Error ? error.message : String(error), - names: [] - } - } -} - -const commandRegistry = loadCommandRegistryNames() -const registryIt = commandRegistry.error ? it.skip : it -const skipReason = commandRegistry.error ? commandRegistry.error.split('\n')[0] : '' - -const LOCAL_COMMAND_NAMES = new Set( - SLASH_COMMANDS.flatMap(command => [command.name, ...(command.aliases ?? [])].map(name => name.toLowerCase())) -) - -const classifyRoute = (name: string): CommandRoute => { - const normalized = name.toLowerCase() - - if (NATIVE_MUTATING_COMMANDS.has(normalized)) { - return 'native' - } - - if (LOCAL_COMMAND_NAMES.has(normalized)) { - return 'local' - } - - return 'fallback' -} - -describe('slash parity matrix', () => { - if (commandRegistry.error) { - it.skip(`Python command registry unavailable: ${skipReason}`, () => {}) - } - - registryIt('classifies each command registry command as local/native/fallback', () => { - const routes = Object.fromEntries(commandRegistry.names.map(name => [name, classifyRoute(name)])) - - expect(routes['model']).toBe('local') - expect(routes['browser']).toBe('native') - expect(routes['reload-mcp']).toBe('native') - expect(routes['rollback']).toBe('native') - expect(routes['stop']).toBe('native') +describe('slash routing', () => { + it('resolves every mutating command locally instead of the CLI slash worker', () => { + expect(MUTATING_COMMANDS.filter(name => !findSlashCommand(name))).toEqual([]) }) - registryIt('keeps every mutating command off slash-worker fallback', () => { - const routes = Object.fromEntries(commandRegistry.names.map(name => [name, classifyRoute(name)])) + it('leaves an unknown command unresolved so it reaches the CLI', () => { + expect(findSlashCommand('channels-status')).toBeUndefined() + }) - for (const name of MUTATING_COMMANDS) { - expect(routes[name], `missing command in registry: ${name}`).toBeDefined() - expect(routes[name], `mutating command must not fallback: ${name}`).not.toBe('fallback') - } + it('resolves aliases and is case-insensitive', () => { + expect(findSlashCommand('MODEL')).toBe(findSlashCommand('model')) + expect(findSlashCommand('bg')).toBe(findSlashCommand('background')) }) }) diff --git a/ui-tui/src/__tests__/text.test.ts b/ui-tui/src/__tests__/text.test.ts index 00b1f322..c758b342 100644 --- a/ui-tui/src/__tests__/text.test.ts +++ b/ui-tui/src/__tests__/text.test.ts @@ -6,6 +6,7 @@ import { describe, expect, it } from 'vitest' import { + clipToWidth, boundedHistoryRenderText, boundedLiveRenderText, buildToolTrailLine, @@ -169,3 +170,24 @@ describe('estimateRows', () => { expect(estimateRows(snake, w)).toBe(estimateRows(plain, w)) }) }) + +describe('clipToWidth', () => { + it('leaves text that already fits, collapsing whitespace', () => { + expect(clipToWidth('hello world', 20)).toBe('hello world') + expect(clipToWidth(' a\n\tb ', 20)).toBe('a b') + }) + + it('clips to the cell budget with an ellipsis', () => { + expect(clipToWidth('hello world', 8)).toBe('hello w\u2026') + }) + + it('counts display cells, not code points, so wide glyphs do not overflow', () => { + // Each CJK glyph is two cells: a budget of 6 fits two glyphs plus the + // ellipsis, not six glyphs. This is the reason the helper exists. + expect(clipToWidth('\u4e2d\u6587\u5b57\u7b26\u6d4b\u8bd5', 6)).toBe('\u4e2d\u6587\u2026') + }) + + it('treats a non-positive budget as no clipping', () => { + expect(clipToWidth('abc', 0)).toBe('abc') + }) +}) diff --git a/ui-tui/src/__tests__/useConfigSync.test.ts b/ui-tui/src/__tests__/useConfigSync.test.ts index acffa839..62c84076 100644 --- a/ui-tui/src/__tests__/useConfigSync.test.ts +++ b/ui-tui/src/__tests__/useConfigSync.test.ts @@ -222,11 +222,9 @@ describe('normalizeBusyInputMode', () => { }) it('defaults to queue for missing/unknown values (TUI-only override)', () => { - // CLI / messaging adapters keep `interrupt` as the framework default - // (see raven_cli/config.py + tui_gateway/server.py::_load_busy_input_mode); - // the TUI ships `queue` because typing a follow-up while the agent - // streams is the common authoring pattern and an unintended interrupt - // loses work. + // The TUI ships `queue` because typing a follow-up while the agent + // streams is the common authoring pattern and an unintended + // interrupt loses work. expect(normalizeBusyInputMode(undefined)).toBe('queue') expect(normalizeBusyInputMode(null)).toBe('queue') expect(normalizeBusyInputMode('')).toBe('queue') @@ -309,10 +307,10 @@ describe('applyDisplay → tui_status_indicator', () => { }) }) -// Regressions from Copilot review on #19835: the config-hydration path -// for voice.record_key was untested, so a future regression in the -// hydration or mtime-reapply wiring would slip past the suite. -describe('applyDisplay → voice.record_key (#18994)', () => { +// The config-hydration path for voice.record_key was untested, so a +// regression in the hydration or mtime-reapply wiring would slip past +// the suite. +describe('applyDisplay → voice.record_key', () => { beforeEach(() => { resetUiState() }) @@ -351,8 +349,7 @@ describe('applyDisplay → voice.record_key (#18994)', () => { // quietRpc() collapses request failures to null. Resetting the // cached shortcut on every null would clobber a custom binding - // after one transient error until the next successful poll - // (Copilot round-8 review on #19835). + // after one transient error until the next successful poll. applyDisplay(null, setBell, setVoiceRecordKey) expect(setVoiceRecordKey).not.toHaveBeenCalled() @@ -362,11 +359,11 @@ describe('applyDisplay → voice.record_key (#18994)', () => { }) }) -// Round-12 Copilot review regression on #19835: the live mtime-reload -// path was previously untested, so a regression in the polling/RPC -// wiring to applyDisplay would only be visible at runtime. The fetch -// + apply body is now shared as ``hydrateFullConfig()``, exercised -// directly from both the initial hydration and the poll-tick body. +// The live mtime-reload path was previously untested, so a regression +// in the polling/RPC wiring to applyDisplay would only be visible at +// runtime. The fetch + apply body is now shared as +// ``hydrateFullConfig()``, exercised directly from both the initial +// hydration and the poll-tick body. describe('hydrateFullConfig', () => { beforeEach(() => { resetUiState() @@ -416,7 +413,7 @@ describe('hydrateFullConfig', () => { const result = await hydrateFullConfig(gw, setBell, setVoiceRecordKey) // quietRpc() swallows the error and returns null; applyDisplay - // sees cfg=null and skips the voice setter (Copilot round-8). + // sees cfg=null and skips the voice setter. expect(result).toBeNull() expect(setVoiceRecordKey).not.toHaveBeenCalled() // bell setter still fires — applyDisplay's null-cfg path applies diff --git a/ui-tui/src/app/createGatewayEventHandler.ts b/ui-tui/src/app/createGatewayEventHandler.ts index 6f7e0e2e..0020db34 100644 --- a/ui-tui/src/app/createGatewayEventHandler.ts +++ b/ui-tui/src/app/createGatewayEventHandler.ts @@ -172,7 +172,7 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev: // Terminal statuses are never overwritten by late-arriving live events — // otherwise a stale `subagent.start` / `spawn_requested` can clobber a - // `failed` or `interrupted` terminal state (Copilot review #14045). + // `failed` or `interrupted` terminal state. const isTerminalStatus = (s: SubagentProgress['status']) => s === 'completed' || s === 'failed' || s === 'interrupted' const keepTerminalElseRunning = (s: SubagentProgress['status']) => (isTerminalStatus(s) ? s : 'running') diff --git a/ui-tui/src/app/slash/commands/session.ts b/ui-tui/src/app/slash/commands/session.ts index 4daefc4d..6f5f956e 100644 --- a/ui-tui/src/app/slash/commands/session.ts +++ b/ui-tui/src/app/slash/commands/session.ts @@ -355,19 +355,19 @@ export const sessionCommands: SlashCommand[] = [ // Render the configured record key (config.yaml ``voice.record_key``) // instead of hardcoded "Ctrl+B" — the gateway response carries the // current value so /voice status and /voice on stay in sync with - // both the CLI and the TUI's actual binding (#18994). + // both the CLI and the TUI's actual binding. // - // Copilot review on #19835 caught that rendering from the fresh - // backend response WITHOUT updating the frontend ``voice.recordKey`` - // state would skew display and binding between config-edit and - // the next ``mtime`` poll (~5s). Parse once, push into state so - // ``useInputHandlers()`` picks up the new binding immediately. + // Rendering from the fresh backend response WITHOUT updating the + // frontend ``voice.recordKey`` state would skew display and binding + // between config-edit and the next ``mtime`` poll (~5s). Parse once, + // push into state so ``useInputHandlers()`` picks up the new binding + // immediately. // - // Round-2 follow-up: only push state when the response actually - // carries ``record_key`` — otherwise an older gateway (or a future - // branch that forgets to include it) would clobber a custom user - // binding back to the default on every /voice invocation. The - // label still falls back to the documented default for display. + // Only push state when the response actually carries + // ``record_key`` — otherwise an older gateway (or a future branch + // that forgets to include it) would clobber a custom user binding + // back to the default on every /voice invocation. The label still + // falls back to the documented default for display. const parsed = r.record_key ? parseVoiceRecordKey(r.record_key) : undefined if (parsed) { diff --git a/ui-tui/src/app/useConfigSync.ts b/ui-tui/src/app/useConfigSync.ts index 07c7d7e1..8ae966b6 100644 --- a/ui-tui/src/app/useConfigSync.ts +++ b/ui-tui/src/app/useConfigSync.ts @@ -39,13 +39,11 @@ export const normalizeStatusBar = (raw: unknown): StatusBarMode => const BUSY_MODES = new Set(['interrupt', 'queue', 'steer']) -// TUI defaults to `queue` even though the framework default -// (`raven_cli/config.py`) is `interrupt`. Rationale: in a full-screen -// TUI you're typically authoring the next prompt while the agent is -// still streaming, and an unintended interrupt loses work. Set -// `display.busy_input_mode: interrupt` (or `steer`) explicitly to -// opt out per-config; CLI / messaging adapters keep their `interrupt` -// default unchanged. +// `queue` rather than `interrupt`: in a full-screen TUI you're +// typically authoring the next prompt while the agent is still +// streaming, and an unintended interrupt loses work. Set +// `display.busy_input_mode: interrupt` (or `steer`) explicitly to opt +// out per-config. const TUI_BUSY_DEFAULT: BusyInputMode = 'queue' export const normalizeBusyInputMode = (raw: unknown): BusyInputMode => { @@ -106,10 +104,10 @@ const _voiceRecordKeyFromConfig = (cfg: ConfigFullResponse | null): ParsedVoiceR /** Fetch ``config.get full`` and fan the result through ``applyDisplay``. * * Extracted so the mtime-reload path can be exercised by the test - * suite without a React runtime (Copilot round-12 review on #19835). - * Both the initial hydration and the mtime poller use this shared - * helper, so a regression in the fetch/apply plumbing now fails the - * useConfigSync tests instead of only being visible at runtime. */ + * suite without a React runtime. Both the initial hydration and the + * mtime poller use this shared helper, so a regression in the + * fetch/apply plumbing now fails the useConfigSync tests instead of + * only being visible at runtime. */ export async function hydrateFullConfig( gw: GatewayClient, setBell: (v: boolean) => void, @@ -133,10 +131,10 @@ export const applyDisplay = ( // Only push the voice record key when the RPC actually returned a // config payload. ``quietRpc()`` collapses failures to ``null``; if we // reset the cached shortcut on every null we would clobber a custom - // binding after one transient RPC error until the next config edit - // (Copilot round-8 review on #19835). The mtime-poll loop advances - // ``mtimeRef`` before this call, so staying silent on null preserves - // the last-good state and lets the next successful poll refresh it. + // binding after one transient RPC error until the next config edit. + // The mtime-poll loop advances ``mtimeRef`` before this call, so + // staying silent on null preserves the last-good state and lets the + // next successful poll refresh it. if (setVoiceRecordKey && cfg) { setVoiceRecordKey(_voiceRecordKeyFromConfig(cfg)) } diff --git a/ui-tui/src/app/useMainApp.ts b/ui-tui/src/app/useMainApp.ts index b5a56806..1f68019e 100644 --- a/ui-tui/src/app/useMainApp.ts +++ b/ui-tui/src/app/useMainApp.ts @@ -397,7 +397,7 @@ export function useMainApp(gw: GatewayClient, rpcClient?: ChatStreamRpcClient) { // alive (stdin listener keeps the event loop open), so the process.on('exit') // handler in entry.tsx — which sends the final resetTerminalModes() — never // fires. This leaves kitty keyboard protocol, mouse modes, etc. enabled - // in the parent shell. See issue #19194. + // in the parent shell. process.exit(0) }, [exit, gw]) diff --git a/ui-tui/src/components/messageLine.tsx b/ui-tui/src/components/messageLine.tsx index ea2d3397..6853a558 100644 --- a/ui-tui/src/components/messageLine.tsx +++ b/ui-tui/src/components/messageLine.tsx @@ -49,7 +49,7 @@ export const MessageLine = memo(function MessageLine({ // calls + Activity; an assistant message with thinking/tools metadata // feeds Thinking + Tool calls. Gating on every section would let // `thinking` (expanded by default) keep an empty wrapper alive when only - // `tools` is hidden — exactly the empty-Box bug Copilot caught. + // `tools` is hidden, leaving a stray empty Box on screen. const thinkingMode = sectionMode('thinking', detailsMode, sections, detailsModeCommandOverride) const toolsMode = sectionMode('tools', detailsMode, sections, detailsModeCommandOverride) const activityMode = sectionMode('activity', detailsMode, sections, detailsModeCommandOverride) diff --git a/ui-tui/src/components/textInput.tsx b/ui-tui/src/components/textInput.tsx index b1007d31..83d27eac 100644 --- a/ui-tui/src/components/textInput.tsx +++ b/ui-tui/src/components/textInput.tsx @@ -739,9 +739,9 @@ export function TextInput({ // Configured voice shortcut wins over composer-level defaults like // paste/copy so users who bind voice to ctrl+v / alt+v / cmd+v - // actually get voice toggled instead of a paste (Copilot round-7 - // follow-up on #19835). The pass-through predicate is a no-op for - // ordinary typing and plain paste when voice is unbound to 'v'. + // actually get voice toggled instead of a paste. The pass-through + // predicate is a no-op for ordinary typing and plain paste when + // voice is unbound to 'v'. if (shouldPassThroughToGlobalHandler(inp, k, voiceRecordKey)) { return } diff --git a/ui-tui/src/lib/platform.ts b/ui-tui/src/lib/platform.ts index e44a4499..ea27816f 100644 --- a/ui-tui/src/lib/platform.ts +++ b/ui-tui/src/lib/platform.ts @@ -60,7 +60,7 @@ export const isCopyShortcut = ( * ``config.yaml`` (default ``ctrl+b``). * * Documented in tips.py, the Python CLI prompt_toolkit handler, and the - * config.yaml default. The TUI honours the same config knob (#18994); + * config.yaml default. The TUI honours the same config knob; * when ``voice.record_key`` is e.g. ``ctrl+o`` the TUI binds Ctrl+O. * * Only the documented default (``ctrl+b``) additionally accepts the @@ -99,20 +99,15 @@ export const DEFAULT_VOICE_RECORD_KEY: ParsedVoiceRecordKey = { * modifier would produce a display/binding mismatch — a config like * ``cmd+b`` would render as ``Cmd+B`` but silently fire on Alt+B, or * never fire at all on legacy terminals even though the UI advertises - * it (Copilot round-6 review on #19835). Users on modern kitty-style - * terminals (iTerm2 CSI-u, Ghostty, Kitty, WezTerm, Alacritty) spell - * the platform action modifier ``super`` / ``win``, which match the - * unambiguous ``key.super`` bit. macOS users on Terminal.app stick - * with the documented ``ctrl+b``. + * it. Users on modern kitty-style terminals (iTerm2 CSI-u, Ghostty, + * Kitty, WezTerm, Alacritty) spell the platform action modifier + * ``super`` / ``win``, which match the unambiguous ``key.super`` bit. + * macOS users on Terminal.app stick with the documented ``ctrl+b``. * - * Cross-runtime parity: the ``ctrl`` / ``control`` / ``alt`` / ``option`` / - * ``opt`` spellings are normalized identically in the classic CLI - * (``raven_cli/voice.py::normalize_voice_record_key_for_prompt_toolkit``) - * so one ``voice.record_key`` value binds the same shortcut in both - * runtimes (Copilot round-9 review on #19835). The ``super`` / - * ``win`` / ``windows`` spellings are TUI-only — prompt_toolkit has no - * super modifier, so the CLI falls back to the documented default and - * logs a warning at startup (Copilot round-11 review on #19835). */ + * ``ctrl`` / ``control`` and ``alt`` / ``option`` / ``opt`` are accepted + * spellings of the same two modifiers, so a config written either way + * binds the same shortcut. ``super`` / ``win`` / ``windows`` only ever + * arrive as ``key.super`` on a kitty-style terminal. */ const _MOD_ALIASES: Record = { alt: 'alt', control: 'ctrl', @@ -148,13 +143,12 @@ const _NAMED_KEY_ALIASES: Record = { * voice check runs, so a binding like ``ctrl+c`` (interrupt), * ``ctrl+d`` (quit), or ``ctrl+l`` (clear screen) would be advertised * in /voice status but never fire push-to-talk. Reject at parse time - * so the user gets the documented Ctrl+B instead of a dead shortcut - * (Copilot round-4 review on #19835). + * so the user gets the documented Ctrl+B instead of a dead shortcut. * * ``ctrl+x`` is intentionally NOT here — it's only claimed during * queue-edit (``queueEditIdx !== null``), so the voice binding works * for most of the session and matches CLI parity for ``ctrl+`` - * bindings (Copilot round-8 review on #19835). */ + * bindings. */ const _RESERVED_CTRL_CHARS = new Set(['c', 'd', 'l']) /** On macOS the action-modifier intercepts these editor chords via @@ -166,7 +160,7 @@ const _RESERVED_CTRL_CHARS = new Set(['c', 'd', 'l']) * On Linux/Windows those globals key off Ctrl instead of Super, so * super+ bindings don't collide. Gate the rejection to darwin * at parse time so kitty/CSI-u ``super+`` configs still work for - * non-mac users (Copilot round-8 review on #19835). */ + * non-mac users. */ const _RESERVED_SUPER_CHARS = new Set(['c', 'd', 'l', 'v']) /** On macOS ``isActionMod`` accepts ``key.meta`` as the action @@ -174,8 +168,7 @@ const _RESERVED_SUPER_CHARS = new Set(['c', 'd', 'l', 'v']) * terminals. So on darwin a configured ``alt+c`` / ``alt+d`` / ``alt+l`` * gets swallowed by ``isCopyShortcut`` / ``isAction`` before the voice * check runs. Block at parse time so /voice status doesn't advertise - * a shortcut that actually copies / quits / clears (Copilot round-12 - * review on #19835). */ + * a shortcut that actually copies / quits / clears. */ const _RESERVED_ALT_CHARS_MAC = new Set(['c', 'd', 'l']) interface RuntimeKeyEvent { @@ -227,10 +220,9 @@ const _matchesNamedKey = (named: VoiceRecordKeyNamed, key: RuntimeKeyEvent, ch: * Accepts ``unknown`` because the source is raw YAML via * ``config.get full`` — a hand-edited ``voice.record_key: 1`` or * ``voice.record_key: true`` would otherwise crash ``.trim()`` on a - * non-string scalar (Copilot round-3 review on #19835). Non-string / - * empty / unrecognised values fall back to the documented Ctrl+B - * default so a typo never silently disables the shortcut. - */ + * non-string scalar. Non-string / empty / unrecognised values fall + * back to the documented Ctrl+B default so a typo never silently + * disables the shortcut. */ export const parseVoiceRecordKey = (raw: unknown): ParsedVoiceRecordKey => { if (typeof raw !== 'string') { return DEFAULT_VOICE_RECORD_KEY @@ -257,9 +249,9 @@ export const parseVoiceRecordKey = (raw: unknown): ParsedVoiceRecordKey => { // Reject multi-modifier chords (``ctrl+alt+r``, ``cmd+ctrl+b``) rather // than silently dropping the extra modifier — the previous // single-token validator made a typo bind a different shortcut than - // the user configured (Copilot round-3 review on #19835). The classic - // CLI only supports single-modifier bindings via prompt_toolkit's - // ``c-x`` / ``a-x`` rewrite in ``cli.py``, so this matches CLI parity. + // the user configured. The classic CLI only supports single-modifier + // bindings via prompt_toolkit's ``c-x`` / ``a-x`` rewrite in + // ``cli.py``, so this matches CLI parity. if (modCandidates.length > 1) { return DEFAULT_VOICE_RECORD_KEY } @@ -267,8 +259,7 @@ export const parseVoiceRecordKey = (raw: unknown): ParsedVoiceRecordKey => { // Require an explicit modifier. A bare ``o`` / ``space`` / ``escape`` // has no sensible mapping: the CLI's prompt_toolkit binds the raw // key (no rewrite) so bare-char configs would silently diverge - // between the two runtimes (Copilot round-4 review on #19835). - // Fall back to the documented default. + // between the two runtimes. Fall back to the documented default. if (modCandidates.length === 0) { return DEFAULT_VOICE_RECORD_KEY } @@ -305,7 +296,7 @@ export const parseVoiceRecordKey = (raw: unknown): ParsedVoiceRecordKey => { // accepts as the mac action modifier. So ``alt+c`` / ``alt+d`` / ``alt+l`` // collide with copy / exit / clear in ``useInputHandlers()`` before the // voice check. Reject at parse time on darwin only — non-mac ``alt+`` - // bindings are still usable (Copilot round-12 review on #19835). + // bindings are still usable. if (isMac && mod === 'alt' && last.length === 1 && _RESERVED_ALT_CHARS_MAC.has(last)) { return DEFAULT_VOICE_RECORD_KEY } @@ -329,8 +320,7 @@ export const parseVoiceRecordKey = (raw: unknown): ParsedVoiceRecordKey => { * * Platform-aware for the ``super`` modifier: renders ``Cmd`` on macOS and * ``Super`` elsewhere. Previously rendered ``Cmd`` universally, which told - * Linux/Windows users the wrong modifier to press (Copilot review, round - * 2 on #19835). */ + * Linux/Windows users the wrong modifier to press. */ export const formatVoiceRecordKey = (parsed: ParsedVoiceRecordKey): string => { const modLabel = parsed.mod === 'super' ? (isMac ? 'Cmd' : 'Super') : parsed.mod[0].toUpperCase() + parsed.mod.slice(1) @@ -346,7 +336,7 @@ export const formatVoiceRecordKey = (parsed: ParsedVoiceRecordKey): string => { * * Compare on the parsed spec rather than ``raw`` so semantically-equal * aliases (``control+b``, ``ctrl + b``) still get the macOS Cmd+B - * muscle-memory fallback (Copilot review, round 2 on #19835). */ + * muscle-memory fallback. */ const _isDefaultVoiceKey = (parsed: ParsedVoiceRecordKey): boolean => parsed.mod === DEFAULT_VOICE_RECORD_KEY.mod && parsed.ch === DEFAULT_VOICE_RECORD_KEY.ch && @@ -371,8 +361,7 @@ export const isVoiceToggleKey = ( // The parser rejects multi-modifier configs (``ctrl+shift+b`` etc.), // so at match time Shift must always be clear — otherwise // ``ctrl+tab`` would also fire on Ctrl+Shift+Tab and ``alt+enter`` - // on Alt+Shift+Enter, triggering a different chord than configured - // (Copilot round-5 review on #19835). + // on Alt+Shift+Enter, triggering a different chord than configured. if (key.shift === true) { return false } @@ -387,14 +376,13 @@ export const isVoiceToggleKey = ( // // Bare Escape on raven-ink can arrive as ``key.meta=true`` on some // terminals, so a configured ``alt+escape`` must not match that shape; - // require an explicit alt bit for escape chords (Copilot round-7 - // follow-up on #19835). + // require an explicit alt bit for escape chords. return (key.alt === true || (key.meta && key.escape !== true)) && !key.ctrl && key.super !== true case 'ctrl': // Require the Ctrl bit AND a clear Alt/Super so a chord like // Ctrl+Alt+ / Ctrl+Cmd+ doesn't spuriously match - // ``ctrl+`` (Copilot round-6 review on #19835). + // ``ctrl+``. // // The documented default (``ctrl+b``) additionally accepts the // explicit ``key.super`` bit on macOS for Cmd+B muscle memory — @@ -410,10 +398,10 @@ export const isVoiceToggleKey = ( case 'super': // Require the explicit ``key.super`` bit (kitty-style protocol) // AND clear Ctrl/Alt/Meta so Ctrl+Cmd+X or Alt+Cmd+X don't - // spuriously fire the super binding (Copilot round-6 review on - // #19835). Legacy-terminal users whose Cmd arrives as - // ``key.meta`` need a kitty-protocol terminal — see the - // _MOD_ALIASES doc-comment for the rationale. + // spuriously fire the super binding. Legacy-terminal users + // whose Cmd arrives as ``key.meta`` need a kitty-protocol + // terminal — see the _MOD_ALIASES doc-comment for the + // rationale. return key.super === true && !key.ctrl && !key.alt && !key.meta } }