From cd591a6752852a8da818ecbfd3c98e25c21ea4a5 Mon Sep 17 00:00:00 2001 From: KT Date: Wed, 29 Jul 2026 15:13:48 +0800 Subject: [PATCH 1/5] test: clear the audit debt in the test suite A read-only audit of all 298 test files (issue #223) found the suite in good shape -- exactly one genuinely fake test -- with the real debt sitting in file names and a scattering of weak assertions. This clears that, and fixes the bugs found while clearing it. Removed two tests that could never pass anything: - test_chat_e2e.py was a module-level skip over three NotImplementedError stubs. The wire-up it waited for has shipped (tui_commands instantiates SubscriptionEmitter) and the proposal doc it cited is gone. - test_tui_exit_e2e.py asserted that building the agent loop leaves lancedb's thread live. It does not: the loop is constructed but never started, and the memory plugin talks to everos over HTTP, so lancedb is not even imported. The assertion could not hold in any configuration. Renamed 19 files off phase, ticket and bug codes (AGENTS.md section 5.1), reversed one CLI name, and moved two integration files onto a legal kind. Reference updates ride along: .pre-commit-config.yaml names one of these files in a detect-private-key exclude, and three docs pages point at them. Tightened assertions that passed for the wrong reason: eight over-broad pytest.raises(Exception) narrowed to the type each site actually raises, an except clause that swallowed the AssertionError its fakes use as a tripwire, a hasattr probe replaced by binding a broker and checking it stuck, and a recall assertion that read as a pass on an empty result set. Cleaned ten dead locals, three dead code blocks, six importorskip guards on hard dependencies, and two hardcoded /tmp paths. Two gaps the deletions exposed are now covered: - tests/test_cli_exit.py pins the hard-exit guard, which had no tests at all. - The TUI chat e2e gains a multi-turn case. It reads the persisted session rather than the screen, because a prompt is echoed into the transcript the moment it is typed -- screen-scraping for a planted word passes even when history is broken. Fixed three timing bugs in the TUI e2e tests along the way. They keyed off the banner, which paints about seven seconds before the app accepts input, and submitted the moment text was typed. Dropped prompts made the pipeline look dead in one test and made a negative assertion pass vacuously in another. The status-bar patterns both tests wait on now live in one module instead of a hand-mirrored copy per file. Full unit suite: 4654 passed, 25 deselected. The e2e tier passes locally with tui-use installed. No production code changes. The same audit covered the two features that landed while this was in review, and a few gaps there were small enough to close here rather than defer: - session.py fills update_available / update_command for the status bar, but removing that wiring left the suite green; the init-bundle tests now pin both the populated and the up-to-date case. - update_notice._upgrade_command_works was stubbed to True by a fixture everywhere, so its real branches had no coverage. - One episodeSummary case was an empty test body, reported as passing while the +added -removed rendering it named went untested. - clipToWidth and hasMeaningfulReasoning had no coverage at all: replacing the first with an identity function and making the second return true unconditionally both left the suite green. - test_tools_registry.py is renamed to test_tool_registry_execute.py, matching the singular module name its sibling test already uses. Each of those was checked by mutating the code under test and confirming the new assertion fails. Co-authored-by: Claude (claude-opus-5) --- .pre-commit-config.yaml | 2 +- docs/everos-memory-e2e-test-plan.md | 2 +- docs/memory-plugin-architecture.md | 6 +- docs/sandbox/usage.md | 24 ++-- tests/conftest.py | 11 +- tests/integration/test_chat_e2e.py | 37 ------ tests/integration/test_everos_backend_e2e.py | 6 +- ...tegration.py => test_sandbox_cli_smoke.py} | 0 ...integration.py => test_sandbox_real_vm.py} | 0 tests/integration/test_tui_exit_e2e.py | 90 -------------- ...py => test_agent_loop_backend_dispatch.py} | 6 +- tests/test_appworld_precheck.py | 4 +- tests/test_auth_allowlist.py | 4 +- ...h.py => test_backend_feedback_dispatch.py} | 2 +- tests/test_cli_deep_research_commands.py | 6 +- tests/test_cli_exit.py | 74 ++++++++++++ ...ugin_stack.py => test_cli_plugin_stack.py} | 4 - ...ox_cli.py => test_cli_sandbox_commands.py} | 0 tests/test_cli_update_notice.py | 27 +++++ ..._cfg1.py => test_config_raven_sections.py} | 7 +- ...gine.py => test_context_engine_factory.py} | 0 tests/test_decision_consumer.py | 16 ++- tests/test_deep_research_tool.py | 9 +- ..._em2_backend.py => test_everos_backend.py} | 2 - ...m3_http.py => test_everos_http_adapter.py} | 4 +- ...ton.py => test_everos_plugin_discovery.py} | 2 +- tests/test_memory_store_lt_additions.py | 2 +- ...1_skeleton.py => test_package_skeleton.py} | 6 +- tests/test_plugin_command.py | 3 - tests/test_plugin_tools.py | 3 +- tests/test_routine_learner_decay.py | 14 --- tests/test_routine_store.py | 2 +- ...int_bug2.py => test_runtime_checkpoint.py} | 6 +- ...eep.py => test_runtime_checkpoint_deep.py} | 8 +- tests/test_sandbox_unit.py | 7 +- tests/test_sentinel_nudge_and_pending.py | 3 +- ...a.py => test_skill_forge_loader_parity.py} | 9 +- ....py => test_skill_router_everos_source.py} | 4 +- ...ter_sr2.py => test_skill_router_fusion.py} | 2 +- ...1.py => test_skill_router_local_source.py} | 2 +- tests/test_task_discoverer.py | 18 --- ...istry.py => test_tool_registry_execute.py} | 0 tests/test_tui_rpc_session_init_bundle.py | 30 +++++ tests/test_tui_rpc_slash_routing.py | 27 +++-- tests/test_tui_rpc_turn_cancel.py | 3 +- tests/test_tui_rpc_turn_send.py | 5 +- tests/test_tui_rpc_turn_subscribe.py | 3 +- tests/tui/autotest/statusbar.py | 20 ++++ .../autotest/tests/test_e2e_raven_tui_chat.py | 113 +++++++++++++++--- .../test_e2e_streaming_no_log_overlay.py | 19 ++- ui-tui/src/__tests__/episodeSummary.test.ts | 8 +- ui-tui/src/__tests__/reasoning.test.ts | 17 ++- ui-tui/src/__tests__/text.test.ts | 22 ++++ 53 files changed, 415 insertions(+), 286 deletions(-) delete mode 100644 tests/integration/test_chat_e2e.py rename tests/integration/{test_sandbox_cli_integration.py => test_sandbox_cli_smoke.py} (100%) rename tests/integration/{test_sandbox_integration.py => test_sandbox_real_vm.py} (100%) delete mode 100644 tests/integration/test_tui_exit_e2e.py rename tests/{test_ag1_backend_dispatch.py => test_agent_loop_backend_dispatch.py} (96%) rename tests/{test_fb1_feedback_dispatch.py => test_backend_feedback_dispatch.py} (99%) create mode 100644 tests/test_cli_exit.py rename tests/{test_cl1_plugin_stack.py => test_cli_plugin_stack.py} (99%) rename tests/{test_sandbox_cli.py => test_cli_sandbox_commands.py} (100%) rename tests/{test_config_cfg1.py => test_config_raven_sections.py} (98%) rename tests/{test_phase_a_default_engine.py => test_context_engine_factory.py} (100%) rename tests/{test_em2_backend.py => test_everos_backend.py} (99%) rename tests/{test_em3_http.py => test_everos_http_adapter.py} (99%) rename tests/{test_em1_skeleton.py => test_everos_plugin_discovery.py} (99%) rename tests/{test_tier1_skeleton.py => test_package_skeleton.py} (97%) rename tests/{test_runtime_checkpoint_bug2.py => test_runtime_checkpoint.py} (98%) rename tests/{test_runtime_checkpoint_bug2_deep.py => test_runtime_checkpoint_deep.py} (99%) rename tests/{test_skill_forge_phase_a.py => test_skill_forge_loader_parity.py} (98%) rename tests/{test_skill_router_sr4.py => test_skill_router_everos_source.py} (97%) rename tests/{test_skill_router_sr2.py => test_skill_router_fusion.py} (98%) rename tests/{test_skill_router_sr1.py => test_skill_router_local_source.py} (99%) rename tests/{test_tools_registry.py => test_tool_registry_execute.py} (100%) create mode 100644 tests/tui/autotest/statusbar.py 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..6927ef71 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): @@ -1135,8 +1136,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 +1160,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__/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__/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__/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') + }) +}) From d8f3c8600e41f67bb5c67efbc77f4ab8d376bf8f Mon Sep 17 00:00:00 2001 From: KT Date: Wed, 29 Jul 2026 17:07:07 +0800 Subject: [PATCH 2/5] test(ui-tui): revive the permanently-skipped slash routing test slashParity.test.ts shelled out to python for raven_cli.commands.COMMAND_REGISTRY and skipped all three cases when the import failed. That package no longer exists -- there is no python-side command registry at all now; unknown slashes go to the CLI through slash.exec -- so the file had been dead, reported as three skips on every run. The parity it wanted needs no python. 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. The test now asserts that predicate directly. Dropped NATIVE_MUTATING_COMMANDS and the three-way classifyRoute with it: all six names in that set are also in the local registry, so the native branch could not change any outcome and the assertions keyed on it were restating the set. Local vitest: 936 passed, no skips (was 933 passed, 3 skipped). Co-authored-by: Claude (claude-opus-5) --- ui-tui/src/__tests__/slashParity.test.ts | 92 ++++-------------------- 1 file changed, 14 insertions(+), 78 deletions(-) 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')) }) }) From 9c03caaafb564c7f36b8c503f471b7bd290d653b Mon Sep 17 00:00:00 2001 From: KT Date: Wed, 29 Jul 2026 18:12:15 +0800 Subject: [PATCH 3/5] docs(tui): drop upstream review attributions from ui-tui comments Comments across ui-tui cited another project's review threads and issue numbers -- 36 mentions of #19835 as "Copilot round-N review", plus #18994, #19194 and #14045. None of those resolve to anything in this repo, and AGENTS.md section 1.1 rules out comments that reference information only visible elsewhere. The technical rationale each one carried is kept; only the attribution clause is gone, and four describe() titles lose a trailing "(#18994)". Three comments went further and documented a python counterpart that does not exist here: - platform.ts claimed the ``ctrl`` / ``alt`` spellings are normalized identically by raven_cli/voice.py, and that the CLI warns at startup for ``super``. There is no such module; raven/tui_rpc/methods/_stubs.py answers voice.toggle with "voice not supported in Raven v0.1". Reduced to what the file itself does. - useConfigSync.ts and its test credited an ``interrupt`` framework default to raven_cli/config.py and tui_gateway/server.py::_load_busy_input_mode. busy_input_mode appears nowhere in python, and neither module exists; the TUI default is the only one there is. Comments and test titles only -- no behaviour change. Local vitest 936 passed, tsc and prettier clean. Co-authored-by: Claude (claude-opus-5) --- .../createGatewayEventHandler.test.ts | 2 +- .../src/__tests__/createSlashHandler.test.ts | 19 +++-- ui-tui/src/__tests__/platform.test.ts | 45 +++++------- ui-tui/src/__tests__/useConfigSync.test.ts | 31 ++++---- ui-tui/src/app/createGatewayEventHandler.ts | 2 +- ui-tui/src/app/slash/commands/session.ts | 22 +++--- ui-tui/src/app/useConfigSync.ts | 28 ++++---- ui-tui/src/app/useMainApp.ts | 2 +- ui-tui/src/components/messageLine.tsx | 2 +- ui-tui/src/components/textInput.tsx | 6 +- ui-tui/src/lib/platform.ts | 72 ++++++++----------- 11 files changed, 103 insertions(+), 128 deletions(-) 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__/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__/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..aa2e25d0 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 } } From bdea3eae9113314aaa954ff5ac5e9ac748b5de4f Mon Sep 17 00:00:00 2001 From: KT Date: Wed, 29 Jul 2026 20:15:20 +0800 Subject: [PATCH 4/5] docs(tui): drop a stray slash left by a comment rewrap Rejoining the parseVoiceRecordKey doc comment in the previous commit pulled the block's closing marker into the text, leaving "disables the shortcut. / */". Comment text only, so tsc, prettier and vitest never saw it. Replaying the rewrap with the marker handled correctly reproduces the rest of that commit byte for byte, so this line was the only site affected. Co-authored-by: Claude (claude-opus-5) --- ui-tui/src/lib/platform.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui-tui/src/lib/platform.ts b/ui-tui/src/lib/platform.ts index aa2e25d0..ea27816f 100644 --- a/ui-tui/src/lib/platform.ts +++ b/ui-tui/src/lib/platform.ts @@ -222,7 +222,7 @@ const _matchesNamedKey = (named: VoiceRecordKeyNamed, key: RuntimeKeyEvent, ch: * ``voice.record_key: true`` would otherwise crash ``.trim()`` on a * non-string scalar. Non-string / empty / unrecognised values fall * back to the documented Ctrl+B default so a typo never silently - * disables the shortcut. / */ + * disables the shortcut. */ export const parseVoiceRecordKey = (raw: unknown): ParsedVoiceRecordKey => { if (typeof raw !== 'string') { return DEFAULT_VOICE_RECORD_KEY From 093a7201ae3d45883ac4b324966dc8c86cb1e910 Mon Sep 17 00:00:00 2001 From: KT Date: Wed, 29 Jul 2026 20:26:38 +0800 Subject: [PATCH 5/5] test(agent): stop the mcp sandbox-guard test from spawning a real process test_stdio_no_executor_does_not_raise set command="true" and let connect_mcp_servers actually spawn it, so the assertion depended on how a real child process raced the MCP handshake. It failed on CI in the full-suite run (CancelledError out of the session's cancel scope) while passing 30 out of 30 standalone runs locally. It also asserted almost nothing: only that whatever exception came back was not SandboxInitError. Following the idiom its sibling test_stdio_sandboxed_with_spawning_does_not_raise already uses, the transport now raises instead of running, and the test asserts the stdio branch was reached with the configured command. Both halves fail under mutation -- dropping "executor is not None" from the guard, and making the stdio branch unreachable. Co-authored-by: Claude (claude-opus-5) --- tests/test_sandbox_unit.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/tests/test_sandbox_unit.py b/tests/test_sandbox_unit.py index 6927ef71..cefb56ea 100644 --- a/tests/test_sandbox_unit.py +++ b/tests/test_sandbox_unit.py @@ -1024,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."""