From 30c93bc95e44a128c233f044349a66c092f6180e Mon Sep 17 00:00:00 2001 From: CleverThis Date: Thu, 4 Jun 2026 12:11:12 -0400 Subject: [PATCH] fix(lsp,tui): repair five unit_test scenarios in CI features/lsp_actor_service_wiring.feature scenarios "handles multiple LSP servers" and "tool specs have correct schema": add an explicit `| CAPABILITIES |` header to the capability tables. Behave treats the first table row as the heading, so `| DIAGNOSTICS |` (the only row) was being silently dropped, leaving the registered server with zero capabilities and the adapter generating no tool specs. features/steps/lsp_actor_service_steps.py: strip surrounding double quotes from each entry in the comma-separated `fields` placeholder of the `requires "..."` Then step so a feature line like `requires "file_path", "line", "column"` resolves to the three unquoted field names rather than `file_path"`, `"line"`, `"column"`. features/steps/tui_persona_cycle_steps.py: include the surrounding double quotes in the step pattern for "the registry last persona should be set to ..." so the feature literal `"p2"` matches as `p2` (without the quotes the placeholder captured `"p2"` and the equality check against the registry value failed). src/cleveragents/tui/persona/registry.py: reject absolute paths from `resolve_export_path` and `resolve_import_path` with the messages the "Persona export/import rejects absolute path targets" scenarios in features/repl_input_modes.feature expect. The previous behaviour silently accepted absolute paths, defeating the working-directory sandboxing intent. ISSUES CLOSED: #5663 --- features/lsp_actor_service_wiring.feature | 11 +++++++---- features/steps/lsp_actor_service_steps.py | 6 +++++- features/steps/tui_persona_cycle_steps.py | 4 ++-- src/cleveragents/tui/persona/registry.py | 20 ++++++++++---------- 4 files changed, 24 insertions(+), 17 deletions(-) diff --git a/features/lsp_actor_service_wiring.feature b/features/lsp_actor_service_wiring.feature index 2d446b23b..b5aa762b4 100644 --- a/features/lsp_actor_service_wiring.feature +++ b/features/lsp_actor_service_wiring.feature @@ -21,9 +21,11 @@ Feature: LSP Actor Service — wire LspRuntime and LspToolAdapter into actor exe Scenario: LspActorService handles multiple LSP servers Given an LSP server "local/pyright" is registered with capabilities: - | DIAGNOSTICS | + | CAPABILITIES | + | DIAGNOSTICS | And an LSP server "local/eslint" is registered with capabilities: - | DIAGNOSTICS | + | CAPABILITIES | + | DIAGNOSTICS | And an actor with LSP bindings: | lsp_server_name | | local/pyright | @@ -77,8 +79,9 @@ Feature: LSP Actor Service — wire LspRuntime and LspToolAdapter into actor exe Scenario: LspActorService tool specs have correct schema Given an LSP server "local/pyright" is registered with capabilities: - | DIAGNOSTICS | - | HOVER | + | CAPABILITIES | + | DIAGNOSTICS | + | HOVER | And an actor with LSP bindings: | lsp_server_name | | local/pyright | diff --git a/features/steps/lsp_actor_service_steps.py b/features/steps/lsp_actor_service_steps.py index af0a8159c..5a5932bfd 100644 --- a/features/steps/lsp_actor_service_steps.py +++ b/features/steps/lsp_actor_service_steps.py @@ -290,7 +290,11 @@ def step_input_schema_requires_fields( context: Any, tool_name: str, fields: str ) -> None: """Verify that the input schema requires specific fields.""" - field_list = [f.strip() for f in fields.split(",")] + # The fields placeholder captures everything between the first and last + # quote on the line, so a list like '"file_path", "line", "column"' comes + # through as the literal string 'file_path", "line", "column'. Strip + # surrounding whitespace and quotes from each comma-separated entry. + field_list = [f.strip().strip('"') for f in fields.split(",")] spec = next((s for s in context.tool_specs if s["name"] == tool_name), None) assert spec is not None, f"Tool spec {tool_name} not found" diff --git a/features/steps/tui_persona_cycle_steps.py b/features/steps/tui_persona_cycle_steps.py index 044c727a2..1d4ef6146 100644 --- a/features/steps/tui_persona_cycle_steps.py +++ b/features/steps/tui_persona_cycle_steps.py @@ -30,7 +30,7 @@ def step_cycle_persona(context: Context, session_id: str) -> None: context.tui_state.cycle_persona(session_id) -@then("the registry last persona should be set to {persona_name}") +@then('the registry last persona should be set to "{persona_name}"') def step_registry_last_persona(context: Context, persona_name: str) -> None: last = context.tui_registry.get_last_persona() - assert last == persona_name + assert last == persona_name, f"expected {persona_name!r}, got {last!r}" diff --git a/src/cleveragents/tui/persona/registry.py b/src/cleveragents/tui/persona/registry.py index 288cd61a5..ab500439e 100644 --- a/src/cleveragents/tui/persona/registry.py +++ b/src/cleveragents/tui/persona/registry.py @@ -79,24 +79,24 @@ class PersonaRegistry: return result def resolve_export_path(self, output_path: Path) -> Path: - """Resolve export path, accepting both absolute and relative paths.""" - resolved = output_path.resolve() - # Allow absolute paths directly + """Resolve a relative export path inside the current working directory.""" if output_path.is_absolute(): - return resolved - # For relative paths, ensure they stay within working directory + raise ValueError( + "Export path must be relative to current working directory" + ) + resolved = output_path.resolve() base = Path.cwd().resolve() if not resolved.is_relative_to(base): raise ValueError("Export path must stay within working directory") return resolved def resolve_import_path(self, input_path: Path) -> Path: - """Resolve import path, accepting both absolute and relative paths.""" - resolved = input_path.resolve() - # Allow absolute paths directly + """Resolve a relative import path inside the current working directory.""" if input_path.is_absolute(): - return resolved - # For relative paths, ensure they stay within working directory + raise ValueError( + "Import path must be relative to current working directory" + ) + resolved = input_path.resolve() base = Path.cwd().resolve() if not resolved.is_relative_to(base): raise ValueError("Import path must stay within working directory")