From 040440026ca2c1fbd5e22b40b323438488a066a9 Mon Sep 17 00:00:00 2001 From: bcbench Date: Thu, 28 May 2026 08:55:23 +0200 Subject: [PATCH 1/2] Enable AL-LSP for Claude Code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors the Copilot AL-LSP integration for Claude Code. Per-task isolation via Claude's --plugin-dir flag — no marketplace registration, no ENABLE_LSP_TOOL global state, no cross-run plugin leakage. - agent/shared/lsp.py: split build_lsp_config into build_copilot_lsp_config (writes .github/lsp.json, auto-discovered by Copilot CLI) and build_claude_lsp_plugin (writes a per-task plugin folder under /.claude/plugins/al-lsp/). Both share _resolve_symbol_paths / _build_lsp_args via a tiny _prepare_lsp helper. - agent/claude/agent.py: call build_claude_lsp_plugin, pass --plugin-dir to claude, record al_lsp_enabled in ExperimentConfiguration. - commands/run.py + commands/evaluate.py: add --al-lsp to the claude commands. - .github/workflows/claude-evaluation.yml: add al-lsp boolean input; Install AL Tool now triggers on al-mcp OR al-lsp; altool bumped to the version that supports launchlspserver; requeue payload propagates the flag. - tests/test_lsp_config.py: split into TestCopilotLspConfig and TestClaudeLspPlugin (22 tests total); the Claude class verifies plugin folder layout, Claude's distinct schema (no lspServers wrapper, extensionToLanguage instead of fileExtensions), and the priority order (compiler folder > artifact cache). - EXPERIMENT.md: surface al-lsp alongside al-mcp in the workflow inputs list. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/claude-evaluation.yml | 14 +- EXPERIMENT.md | 2 +- src/bcbench/agent/claude/agent.py | 15 ++- src/bcbench/agent/copilot/agent.py | 4 +- src/bcbench/agent/shared/__init__.py | 4 +- src/bcbench/agent/shared/lsp.py | 87 ++++++++++--- src/bcbench/commands/evaluate.py | 2 + src/bcbench/commands/run.py | 12 +- tests/test_lsp_config.py | 165 ++++++++++++++++++------ 9 files changed, 237 insertions(+), 68 deletions(-) diff --git a/.github/workflows/claude-evaluation.yml b/.github/workflows/claude-evaluation.yml index e79d2d52c..f95e20050 100644 --- a/.github/workflows/claude-evaluation.yml +++ b/.github/workflows/claude-evaluation.yml @@ -33,6 +33,11 @@ on: required: false default: false type: boolean + al-lsp: + description: "Enable AL LSP server" + required: false + default: false + type: boolean repeat: description: "Number of times to run sequentially (ignored for test runs)" required: false @@ -100,9 +105,9 @@ jobs: node-version: 24 - name: Install AL Tool - if: ${{ inputs.al-mcp }} + if: ${{ inputs.al-mcp || inputs.al-lsp }} run: | - dotnet tool install -g Microsoft.Dynamics.BusinessCentral.Development.Tools --version 17.0.33.55542 + dotnet tool install -g Microsoft.Dynamics.BusinessCentral.Development.Tools --version 18.0.36.64936-beta echo "$HOME\.dotnet\tools" >> $env:GITHUB_PATH - name: Install Claude Code @@ -120,7 +125,8 @@ jobs: --category "${{ inputs.category }}" ` --repo-path "${{ steps.setup-env.outputs.repo_path }}" ` --output-dir "${{ env.EVALUATION_RESULTS_DIR }}" ` - ${{ inputs.al-mcp && '--al-mcp' || '' }} + ${{ inputs.al-mcp && '--al-mcp' || '' }} ` + ${{ inputs.al-lsp && '--al-lsp' || '' }} - name: Upload evaluation results uses: actions/upload-artifact@v6 @@ -155,4 +161,4 @@ jobs: workflow-file: claude-evaluation.yml repeat: ${{ inputs.repeat }} workflow-inputs: | - {"model": "${{ inputs.model }}", "category": "${{ inputs.category }}", "test-run": "${{ inputs.test-run }}", "al-mcp": "${{ inputs.al-mcp }}"} + {"model": "${{ inputs.model }}", "category": "${{ inputs.category }}", "test-run": "${{ inputs.test-run }}", "al-mcp": "${{ inputs.al-mcp }}", "al-lsp": "${{ inputs.al-lsp }}"} diff --git a/EXPERIMENT.md b/EXPERIMENT.md index e564f4a49..c2b856e7a 100644 --- a/EXPERIMENT.md +++ b/EXPERIMENT.md @@ -77,7 +77,7 @@ Trigger the evaluation workflow from the **Actions** tab: - **Workflow:** `Evaluation with GitHub Copilot` or `Evaluation with Claude Code` - **`test-run`:** `true` (default — runs 4 entries, ~10 min) -- **`model`**, **`category`**, **`al-mcp`**: as needed +- **`model`**, **`category`**, **`al-mcp`**, **`al-lsp`**: as needed This catches configuration mistakes cheaply. Do not skip it. diff --git a/src/bcbench/agent/claude/agent.py b/src/bcbench/agent/claude/agent.py index b80017c5c..c5cd5ba6a 100644 --- a/src/bcbench/agent/claude/agent.py +++ b/src/bcbench/agent/claude/agent.py @@ -6,7 +6,7 @@ import yaml from bcbench.agent.claude.metrics import parse_metrics -from bcbench.agent.shared import build_mcp_config, build_prompt, parse_tool_usage_from_hooks +from bcbench.agent.shared import build_claude_lsp_plugin, build_mcp_config, build_prompt, parse_tool_usage_from_hooks from bcbench.config import get_config from bcbench.dataset import BaseDatasetEntry from bcbench.exceptions import AgentError, AgentTimeoutError @@ -19,7 +19,14 @@ def run_claude_code( - entry: BaseDatasetEntry, model: str, category: EvaluationCategory, repo_path: Path, output_dir: Path, al_mcp: bool = False, container_name: str = "bcbench" + entry: BaseDatasetEntry, + model: str, + category: EvaluationCategory, + repo_path: Path, + output_dir: Path, + al_mcp: bool = False, + al_lsp: bool = False, + container_name: str = "bcbench", ) -> tuple[AgentMetrics | None, ExperimentConfiguration]: """Run Claude Code on a single dataset entry. @@ -33,12 +40,14 @@ def run_claude_code( prompt: str = build_prompt(entry, repo_path, claude_config, category, al_mcp=al_mcp) mcp_config_json, mcp_server_names = build_mcp_config(claude_config, entry, repo_path, al_mcp=al_mcp, container_name=container_name) + lsp_plugin_dir: Path | None = build_claude_lsp_plugin(entry, category, repo_path, al_lsp=al_lsp, container_name=container_name) instructions_enabled: bool = setup_instructions_from_config(claude_config, entry, repo_path, agent_type=AgentType.CLAUDE) skills_enabled: bool = setup_agent_skills(claude_config, entry, repo_path, agent_type=AgentType.CLAUDE) custom_agent: str | None = setup_custom_agent(claude_config, entry, repo_path, agent_type=AgentType.CLAUDE) tool_log_path: Path = setup_hooks(repo_path, AgentType.CLAUDE, output_dir) config = ExperimentConfiguration( mcp_servers=mcp_server_names, + al_lsp_enabled=lsp_plugin_dir is not None, custom_instructions=instructions_enabled, skills_enabled=skills_enabled, custom_agent=custom_agent, @@ -65,6 +74,8 @@ def run_claude_code( ] if mcp_config_json: cmd_args.append(f"--mcp-config={mcp_config_json}") + if lsp_plugin_dir is not None: + cmd_args.append(f"--plugin-dir={lsp_plugin_dir}") if custom_agent: cmd_args.append(f"--agent={custom_agent}") cmd_args.extend( diff --git a/src/bcbench/agent/copilot/agent.py b/src/bcbench/agent/copilot/agent.py index 4ecb97141..633d194aa 100644 --- a/src/bcbench/agent/copilot/agent.py +++ b/src/bcbench/agent/copilot/agent.py @@ -8,7 +8,7 @@ import yaml from bcbench.agent.copilot.metrics import parse_metrics -from bcbench.agent.shared import build_lsp_config, build_mcp_config, build_prompt, parse_tool_usage_from_hooks +from bcbench.agent.shared import build_copilot_lsp_config, build_mcp_config, build_prompt, parse_tool_usage_from_hooks from bcbench.config import get_config from bcbench.dataset import BaseDatasetEntry from bcbench.exceptions import AgentError, AgentTimeoutError @@ -42,7 +42,7 @@ def run_copilot_agent( prompt: str = build_prompt(entry, repo_path, copilot_config, category, al_mcp=al_mcp) mcp_config_json, mcp_server_names = build_mcp_config(copilot_config, entry, repo_path, al_mcp=al_mcp, container_name=container_name) - al_lsp_enabled: bool = build_lsp_config(entry, category, repo_path, al_lsp=al_lsp, container_name=container_name) + al_lsp_enabled: bool = build_copilot_lsp_config(entry, category, repo_path, al_lsp=al_lsp, container_name=container_name) instructions_enabled: bool = setup_instructions_from_config(copilot_config, entry, repo_path, agent_type=AgentType.COPILOT) skills_enabled: bool = setup_agent_skills(copilot_config, entry, repo_path, agent_type=AgentType.COPILOT) custom_agent: str | None = setup_custom_agent(copilot_config, entry, repo_path, agent_type=AgentType.COPILOT) diff --git a/src/bcbench/agent/shared/__init__.py b/src/bcbench/agent/shared/__init__.py index e394e7545..ce4c1565e 100644 --- a/src/bcbench/agent/shared/__init__.py +++ b/src/bcbench/agent/shared/__init__.py @@ -1,8 +1,8 @@ """Shared code for CLI-based agents (Claude, Copilot).""" from bcbench.agent.shared.hooks_parser import parse_tool_usage_from_hooks -from bcbench.agent.shared.lsp import build_lsp_config +from bcbench.agent.shared.lsp import build_claude_lsp_plugin, build_copilot_lsp_config from bcbench.agent.shared.mcp import build_mcp_config from bcbench.agent.shared.prompt import build_prompt -__all__ = ["build_lsp_config", "build_mcp_config", "build_prompt", "parse_tool_usage_from_hooks"] +__all__ = ["build_claude_lsp_plugin", "build_copilot_lsp_config", "build_mcp_config", "build_prompt", "parse_tool_usage_from_hooks"] diff --git a/src/bcbench/agent/shared/lsp.py b/src/bcbench/agent/shared/lsp.py index 25b9f7f85..4c7af905c 100644 --- a/src/bcbench/agent/shared/lsp.py +++ b/src/bcbench/agent/shared/lsp.py @@ -14,7 +14,8 @@ logger = get_logger(__name__) -_AL_LSP_RELATIVE_PATH = Path(".github") / "lsp.json" +_COPILOT_LSP_RELATIVE_PATH = Path(".github") / "lsp.json" +_CLAUDE_PLUGIN_RELATIVE_PATH = Path(".claude") / "plugins" / "al-lsp" def _resolve_symbol_paths(entry: BaseDatasetEntry, category: EvaluationCategory, container_name: str) -> tuple[list[str], list[str]]: @@ -48,13 +49,21 @@ def _build_lsp_args(project_paths: list[str], package_cache_paths: list[str], as return args -def build_lsp_config(entry: BaseDatasetEntry, category: EvaluationCategory, repo_path: Path, al_lsp: bool, container_name: str = "") -> bool: - """Write Copilot's project-level LSP config to /.github/lsp.json. +def _prepare_lsp(entry: BaseDatasetEntry, category: EvaluationCategory, repo_path: Path, container_name: str) -> list[str]: + """Common preparation shared by both agent variants: set runtime, resolve symbols, build args.""" + project_paths = [str(repo_path / p) for p in entry.project_paths] + set_runtime_version(project_paths) + package_cache_paths, assembly_probing_paths = _resolve_symbol_paths(entry, category, container_name) + return _build_lsp_args(project_paths, package_cache_paths, assembly_probing_paths) + - When ``al_lsp=False``, removes any stale config left over from a previous run and returns False. - When True, writes the `lspServers.altool` entry pointing at `altool launchlspserver` and returns True. +def build_copilot_lsp_config(entry: BaseDatasetEntry, category: EvaluationCategory, repo_path: Path, al_lsp: bool, container_name: str = "") -> bool: + """Write Copilot CLI's project-level LSP config to /.github/lsp.json. + + Copilot CLI auto-discovers ``.github/lsp.json`` on session start — no CLI flag needed. + Removes any stale config when ``al_lsp=False`` so toggling the flag off actually disables the server. """ - lsp_config_path = repo_path / _AL_LSP_RELATIVE_PATH + lsp_config_path = repo_path / _COPILOT_LSP_RELATIVE_PATH if not al_lsp: if lsp_config_path.is_file(): @@ -62,16 +71,7 @@ def build_lsp_config(entry: BaseDatasetEntry, category: EvaluationCategory, repo logger.info(f"Removed stale LSP config: {lsp_config_path}") return False - project_paths = [str(repo_path / p) for p in entry.project_paths] - set_runtime_version(project_paths) - - package_cache_paths, assembly_probing_paths = _resolve_symbol_paths(entry, category, container_name) - - args = _build_lsp_args( - project_paths=project_paths, - package_cache_paths=package_cache_paths, - assembly_probing_paths=assembly_probing_paths, - ) + args = _prepare_lsp(entry, category, repo_path, container_name) # Copilot CLI resolves `command` via PATH (absolute paths are silently rejected with # "Server is configured but not available"). `al` is the published altool @@ -89,7 +89,60 @@ def build_lsp_config(entry: BaseDatasetEntry, category: EvaluationCategory, repo lsp_config_path.parent.mkdir(parents=True, exist_ok=True) lsp_config_path.write_text(json.dumps(lsp_config, indent=2), encoding="utf-8") - logger.info(f"Wrote AL LSP config: {lsp_config_path}") + logger.info(f"Wrote Copilot LSP config: {lsp_config_path}") logger.debug(f"LSP configuration: {json.dumps(lsp_config, indent=2)}") return True + + +def build_claude_lsp_plugin(entry: BaseDatasetEntry, category: EvaluationCategory, repo_path: Path, al_lsp: bool, container_name: str = "") -> Path | None: + """Build a per-task Claude Code plugin folder containing the AL LSP server. + + Claude Code surfaces LSP servers through its plugin system. ``claude --plugin-dir `` + loads a plugin for a single session with no marketplace registration, no + ``ENABLE_LSP_TOOL`` global state, and no cross-run plugin leakage — the same + per-task isolation the Copilot side gets from ``.github/lsp.json``. + + Layout written under ``/.claude/plugins/al-lsp/``: + .claude-plugin/plugin.json — minimal manifest (only ``name`` is required) + .lsp.json — LSP server config in Claude's schema + + Returns the plugin folder path so the caller can pass it as ``--plugin-dir``, + or None when disabled. + """ + plugin_dir = repo_path / _CLAUDE_PLUGIN_RELATIVE_PATH + + if not al_lsp: + if plugin_dir.exists(): + for p in sorted(plugin_dir.rglob("*"), reverse=True): + p.rmdir() if p.is_dir() else p.unlink() + plugin_dir.rmdir() + logger.info(f"Removed stale Claude LSP plugin: {plugin_dir}") + return None + + args = _prepare_lsp(entry, category, repo_path, container_name) + + # Minimal plugin manifest. Name is the only required field; we don't ship skills/agents/hooks. + plugin_manifest = { + "name": "al-lsp", + "version": "1.0.0", + "description": "AL Language Server for Business Central agentic development", + } + # Claude's LSP schema differs from Copilot's: no `lspServers` wrapper, and uses + # `extensionToLanguage` instead of `fileExtensions`. + lsp_config = { + "altool": { + "command": "al", + "args": args, + "extensionToLanguage": {".al": "al"}, + } + } + + (plugin_dir / ".claude-plugin").mkdir(parents=True, exist_ok=True) + (plugin_dir / ".claude-plugin" / "plugin.json").write_text(json.dumps(plugin_manifest, indent=2), encoding="utf-8") + (plugin_dir / ".lsp.json").write_text(json.dumps(lsp_config, indent=2), encoding="utf-8") + + logger.info(f"Wrote Claude LSP plugin: {plugin_dir}") + logger.debug(f"LSP configuration: {json.dumps(lsp_config, indent=2)}") + + return plugin_dir diff --git a/src/bcbench/commands/evaluate.py b/src/bcbench/commands/evaluate.py index 9eefb9342..2bf912277 100644 --- a/src/bcbench/commands/evaluate.py +++ b/src/bcbench/commands/evaluate.py @@ -106,6 +106,7 @@ def evaluate_claude_code( output_dir: OutputDir = _config.paths.evaluation_results_path, run_id: RunId = "claude_code_test_run", al_mcp: Annotated[bool, typer.Option("--al-mcp", help="Enable AL MCP server")] = False, + al_lsp: Annotated[bool, typer.Option("--al-lsp", help="Enable AL LSP server")] = False, ) -> None: """ Evaluate Claude Code on single dataset entry. @@ -139,6 +140,7 @@ def evaluate_claude_code( model=ctx.model, output_dir=ctx.result_dir, al_mcp=al_mcp if ctx.container else False, + al_lsp=al_lsp, container_name=ctx.get_container().name if ctx.container else "", ), ) diff --git a/src/bcbench/commands/run.py b/src/bcbench/commands/run.py index 5919a8355..f7fa34ebf 100644 --- a/src/bcbench/commands/run.py +++ b/src/bcbench/commands/run.py @@ -65,6 +65,7 @@ def run_claude( repo_path: RepoPath = _config.paths.testbed_path, output_dir: OutputDir = _config.paths.evaluation_results_path, al_mcp: Annotated[bool, typer.Option("--al-mcp", help="Enable AL MCP server")] = False, + al_lsp: Annotated[bool, typer.Option("--al-lsp", help="Enable AL LSP server")] = False, ) -> None: """ Run Claude Code on a single entry to generate a patch (without building/testing). @@ -77,4 +78,13 @@ def run_claude( entry = category.entry_class.load(category.dataset_path, entry_id=entry_id)[0] category.pipeline.setup_workspace(entry, repo_path) - run_claude_code(entry=entry, repo_path=repo_path, model=model, category=category, output_dir=output_dir, al_mcp=al_mcp, container_name=container_name) + run_claude_code( + entry=entry, + repo_path=repo_path, + model=model, + category=category, + output_dir=output_dir, + al_mcp=al_mcp if container_name else False, + al_lsp=al_lsp, + container_name=container_name or "", + ) diff --git a/tests/test_lsp_config.py b/tests/test_lsp_config.py index c4c9b9d86..ffca778e2 100644 --- a/tests/test_lsp_config.py +++ b/tests/test_lsp_config.py @@ -4,7 +4,7 @@ import pytest -from bcbench.agent.shared.lsp import build_lsp_config +from bcbench.agent.shared.lsp import build_claude_lsp_plugin, build_copilot_lsp_config from bcbench.exceptions import AgentError from bcbench.types import EvaluationCategory from tests.conftest import create_dataset_entry @@ -20,14 +20,6 @@ def repo_path(tmp_path) -> Path: return tmp_path / "repo" -def _read_lsp(repo_path: Path) -> dict: - return json.loads((repo_path / ".github" / "lsp.json").read_text(encoding="utf-8")) - - -def _build(entry, repo_path, **kwargs): - return build_lsp_config(entry, EvaluationCategory.BUG_FIX, repo_path, **kwargs) - - @pytest.fixture def artifact_paths(): with patch( @@ -43,10 +35,17 @@ def no_artifacts(): yield m -class TestBuildLspConfig: +class TestCopilotLspConfig: + @staticmethod + def _read(repo_path: Path) -> dict: + return json.loads((repo_path / ".github" / "lsp.json").read_text(encoding="utf-8")) + + @staticmethod + def _build(entry, repo_path, **kwargs): + return build_copilot_lsp_config(entry, EvaluationCategory.BUG_FIX, repo_path, **kwargs) + def test_returns_false_when_disabled(self, entry, repo_path): - result = _build(entry, repo_path, al_lsp=False) - assert result is False + assert self._build(entry, repo_path, al_lsp=False) is False assert not (repo_path / ".github" / "lsp.json").exists() def test_removes_stale_config_when_disabled(self, entry, repo_path): @@ -54,60 +53,56 @@ def test_removes_stale_config_when_disabled(self, entry, repo_path): lsp_path.parent.mkdir(parents=True) lsp_path.write_text("{}") - _build(entry, repo_path, al_lsp=False) + self._build(entry, repo_path, al_lsp=False) assert not lsp_path.exists() @pytest.mark.usefixtures("artifact_paths") def test_returns_true_when_enabled(self, entry, repo_path): - assert _build(entry, repo_path, al_lsp=True) is True + assert self._build(entry, repo_path, al_lsp=True) is True @pytest.mark.usefixtures("artifact_paths") def test_writes_project_lsp_config(self, entry, repo_path): - _build(entry, repo_path, al_lsp=True) + self._build(entry, repo_path, al_lsp=True) - config = _read_lsp(repo_path) + config = self._read(repo_path) assert "lspServers" in config assert "altool" in config["lspServers"] @pytest.mark.usefixtures("artifact_paths") def test_command_is_unqualified_al(self, entry, repo_path): - _build(entry, repo_path, al_lsp=True) + self._build(entry, repo_path, al_lsp=True) - server = _read_lsp(repo_path)["lspServers"]["altool"] # Copilot CLI silently rejects absolute command paths — must resolve via PATH. - assert server["command"] == "al" + assert self._read(repo_path)["lspServers"]["altool"]["command"] == "al" @pytest.mark.usefixtures("artifact_paths") def test_al_file_extension_registered(self, entry, repo_path): - _build(entry, repo_path, al_lsp=True) + self._build(entry, repo_path, al_lsp=True) - server = _read_lsp(repo_path)["lspServers"]["altool"] - assert server["fileExtensions"] == {".al": "al"} + assert self._read(repo_path)["lspServers"]["altool"]["fileExtensions"] == {".al": "al"} @pytest.mark.usefixtures("artifact_paths") def test_project_paths_inserted_after_launchlspserver(self, entry, repo_path): - _build(entry, repo_path, al_lsp=True) + self._build(entry, repo_path, al_lsp=True) - args = _read_lsp(repo_path)["lspServers"]["altool"]["args"] + args = self._read(repo_path)["lspServers"]["altool"]["args"] launch_idx = args.index("launchlspserver") assert args[launch_idx + 1] == str(repo_path / "src/App") assert args[launch_idx + 2] == str(repo_path / "src/TestApp") @pytest.mark.usefixtures("artifact_paths") def test_artifact_cache_paths_used_for_package_cache(self, entry, repo_path): - _build(entry, repo_path, al_lsp=True) + self._build(entry, repo_path, al_lsp=True) - args = _read_lsp(repo_path)["lspServers"]["altool"]["args"] + args = self._read(repo_path)["lspServers"]["altool"]["args"] cache_idx = args.index("--packagecachepath") probing_idx = args.index("--assemblyprobingpaths") - # All entries between --packagecachepath and --assemblyprobingpaths are package paths. assert args[cache_idx + 1 : probing_idx] == ["C:/cache/w1/Extensions", "C:/cache/platform/Applications"] @pytest.mark.usefixtures("artifact_paths") def test_does_not_require_container_when_artifacts_present(self, entry, repo_path): - # No container_name supplied — should still succeed via artifact cache. - assert _build(entry, repo_path, al_lsp=True, container_name="") is True + assert self._build(entry, repo_path, al_lsp=True, container_name="") is True @pytest.mark.usefixtures("no_artifacts") def test_uses_container_compiler_folder_when_present(self, entry, repo_path, tmp_path): @@ -117,32 +112,124 @@ def test_uses_container_compiler_folder_when_present(self, entry, repo_path, tmp "bcbench.agent.shared.lsp.compiler_symbol_folder_for_container", return_value=(compiler_root, compiler_root / "symbols"), ): - result = _build(entry, repo_path, al_lsp=True, container_name="test-container") + assert self._build(entry, repo_path, al_lsp=True, container_name="test-container") is True - assert result is True - args = _read_lsp(repo_path)["lspServers"]["altool"]["args"] + args = self._read(repo_path)["lspServers"]["altool"]["args"] cache_idx = args.index("--packagecachepath") assert args[cache_idx + 1] == str(compiler_root / "symbols") @pytest.mark.usefixtures("artifact_paths") def test_container_compiler_folder_wins_over_artifact_cache(self, entry, repo_path, tmp_path): - # When BOTH sources exist, the container compiler folder must win — same - # arg shape as AL-MCP, easier to debug a "which symbols set is this?" question. compiler_root = tmp_path / "compiler" / "test-container" (compiler_root / "symbols").mkdir(parents=True) with patch( "bcbench.agent.shared.lsp.compiler_symbol_folder_for_container", return_value=(compiler_root, compiler_root / "symbols"), ): - _build(entry, repo_path, al_lsp=True, container_name="test-container") + self._build(entry, repo_path, al_lsp=True, container_name="test-container") - args = _read_lsp(repo_path)["lspServers"]["altool"]["args"] + args = self._read(repo_path)["lspServers"]["altool"]["args"] cache_idx = args.index("--packagecachepath") - # Probing paths may be absent when neither dlls/ nor system .NET is found — slice to end-of-list in that case. end_idx = args.index("--assemblyprobingpaths") if "--assemblyprobingpaths" in args else len(args) - assert args[cache_idx + 1 : end_idx] == [str(compiler_root / "symbols")] # single path, not the 2-path artifact-cache layout + assert args[cache_idx + 1 : end_idx] == [str(compiler_root / "symbols")] + + @pytest.mark.usefixtures("no_artifacts") + def test_raises_with_download_hint_when_neither_source_available(self, entry, repo_path): + with pytest.raises(AgentError, match=r"Download-BCSymbols\.ps1"): + self._build(entry, repo_path, al_lsp=True, container_name="") + + +class TestClaudeLspPlugin: + @staticmethod + def _plugin_dir(repo_path: Path) -> Path: + return repo_path / ".claude" / "plugins" / "al-lsp" + + @staticmethod + def _read_lsp(repo_path: Path) -> dict: + return json.loads((TestClaudeLspPlugin._plugin_dir(repo_path) / ".lsp.json").read_text(encoding="utf-8")) + + @staticmethod + def _read_manifest(repo_path: Path) -> dict: + return json.loads((TestClaudeLspPlugin._plugin_dir(repo_path) / ".claude-plugin" / "plugin.json").read_text(encoding="utf-8")) + + @staticmethod + def _build(entry, repo_path, **kwargs): + return build_claude_lsp_plugin(entry, EvaluationCategory.BUG_FIX, repo_path, **kwargs) + + def test_returns_none_when_disabled(self, entry, repo_path): + assert self._build(entry, repo_path, al_lsp=False) is None + assert not self._plugin_dir(repo_path).exists() + + def test_removes_stale_plugin_when_disabled(self, entry, repo_path): + plugin_dir = self._plugin_dir(repo_path) + (plugin_dir / ".claude-plugin").mkdir(parents=True) + (plugin_dir / ".claude-plugin" / "plugin.json").write_text("{}") + (plugin_dir / ".lsp.json").write_text("{}") + + self._build(entry, repo_path, al_lsp=False) + + assert not plugin_dir.exists() + + @pytest.mark.usefixtures("artifact_paths") + def test_returns_plugin_dir_when_enabled(self, entry, repo_path): + plugin_dir = self._build(entry, repo_path, al_lsp=True) + assert plugin_dir == self._plugin_dir(repo_path) + + @pytest.mark.usefixtures("artifact_paths") + def test_writes_minimal_manifest(self, entry, repo_path): + self._build(entry, repo_path, al_lsp=True) + + manifest = self._read_manifest(repo_path) + assert manifest["name"] == "al-lsp" # the only required field per Claude plugin docs + + @pytest.mark.usefixtures("artifact_paths") + def test_writes_lsp_config_at_plugin_root(self, entry, repo_path): + self._build(entry, repo_path, al_lsp=True) + + config = self._read_lsp(repo_path) + # Claude's schema: top-level server name (no `lspServers` wrapper, unlike Copilot). + assert "altool" in config + assert "lspServers" not in config + + @pytest.mark.usefixtures("artifact_paths") + def test_extension_to_language_uses_claude_schema(self, entry, repo_path): + self._build(entry, repo_path, al_lsp=True) + + # Claude uses `extensionToLanguage`; Copilot uses `fileExtensions`. + server = self._read_lsp(repo_path)["altool"] + assert server["extensionToLanguage"] == {".al": "al"} + assert "fileExtensions" not in server + + @pytest.mark.usefixtures("artifact_paths") + def test_command_is_unqualified_al(self, entry, repo_path): + self._build(entry, repo_path, al_lsp=True) + + assert self._read_lsp(repo_path)["altool"]["command"] == "al" + + @pytest.mark.usefixtures("artifact_paths") + def test_project_paths_inserted_after_launchlspserver(self, entry, repo_path): + self._build(entry, repo_path, al_lsp=True) + + args = self._read_lsp(repo_path)["altool"]["args"] + launch_idx = args.index("launchlspserver") + assert args[launch_idx + 1] == str(repo_path / "src/App") + assert args[launch_idx + 2] == str(repo_path / "src/TestApp") + + @pytest.mark.usefixtures("no_artifacts") + def test_uses_container_compiler_folder_when_present(self, entry, repo_path, tmp_path): + compiler_root = tmp_path / "compiler" / "test-container" + (compiler_root / "symbols").mkdir(parents=True) + with patch( + "bcbench.agent.shared.lsp.compiler_symbol_folder_for_container", + return_value=(compiler_root, compiler_root / "symbols"), + ): + assert self._build(entry, repo_path, al_lsp=True, container_name="test-container") is not None + + args = self._read_lsp(repo_path)["altool"]["args"] + cache_idx = args.index("--packagecachepath") + assert args[cache_idx + 1] == str(compiler_root / "symbols") @pytest.mark.usefixtures("no_artifacts") def test_raises_with_download_hint_when_neither_source_available(self, entry, repo_path): with pytest.raises(AgentError, match=r"Download-BCSymbols\.ps1"): - _build(entry, repo_path, al_lsp=True, container_name="") + self._build(entry, repo_path, al_lsp=True, container_name="") From 09a867cc96262f95b74bc95bb15309d3c60c710d Mon Sep 17 00:00:00 2001 From: bcbench Date: Thu, 28 May 2026 13:00:47 +0200 Subject: [PATCH 2/2] Unify Copilot + Claude on a single AL-LSP plugin folder Reviewer feedback: instead of two writers (.github/lsp.json for Copilot, .claude/plugins/al-lsp/ for Claude), use `--plugin-dir ` for both agents and ship a single plugin folder shape under /.bcbench/al-lsp-plugin/. Both CLIs look for the manifest at .claude-plugin/plugin.json, both accept --plugin-dir for ad-hoc per-session loading, so the only delta is the .lsp.json schema (Copilot wraps in 'lspServers' and uses 'fileExtensions'; Claude has no wrapper and uses 'extensionToLanguage'). That branch lives in a single ten-line _lsp_config_for() switch. - agent/shared/lsp.py: collapse build_copilot_lsp_config + build_claude_lsp_plugin into one build_al_lsp_plugin(entry, category, repo_path, agent_type, ...). 113 -> 95 LoC despite the new parameter. - agent/copilot/agent.py: drop the .github/lsp.json auto-discovery path; now passes --plugin-dir like Claude. - agent/claude/agent.py: same call shape, same plugin folder. - tests/test_lsp_config.py: parametrize across both AgentType values so every shared behavior is verified for both agents in one class (10 shared tests x 2 agents + 2 schema-specific tests = 22). 227 -> ~150 LoC, same coverage. Live-verified: copilot --plugin-dir now logs 'Loaded 1 LSP servers from 1 plugins' and reads our LSP config exactly the same way Claude would. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/bcbench/agent/claude/agent.py | 4 +- src/bcbench/agent/copilot/agent.py | 8 +- src/bcbench/agent/shared/__init__.py | 4 +- src/bcbench/agent/shared/lsp.py | 113 +++++-------- tests/test_lsp_config.py | 227 ++++++++++----------------- 5 files changed, 134 insertions(+), 222 deletions(-) diff --git a/src/bcbench/agent/claude/agent.py b/src/bcbench/agent/claude/agent.py index c5cd5ba6a..ae2e38214 100644 --- a/src/bcbench/agent/claude/agent.py +++ b/src/bcbench/agent/claude/agent.py @@ -6,7 +6,7 @@ import yaml from bcbench.agent.claude.metrics import parse_metrics -from bcbench.agent.shared import build_claude_lsp_plugin, build_mcp_config, build_prompt, parse_tool_usage_from_hooks +from bcbench.agent.shared import build_al_lsp_plugin, build_mcp_config, build_prompt, parse_tool_usage_from_hooks from bcbench.config import get_config from bcbench.dataset import BaseDatasetEntry from bcbench.exceptions import AgentError, AgentTimeoutError @@ -40,7 +40,7 @@ def run_claude_code( prompt: str = build_prompt(entry, repo_path, claude_config, category, al_mcp=al_mcp) mcp_config_json, mcp_server_names = build_mcp_config(claude_config, entry, repo_path, al_mcp=al_mcp, container_name=container_name) - lsp_plugin_dir: Path | None = build_claude_lsp_plugin(entry, category, repo_path, al_lsp=al_lsp, container_name=container_name) + lsp_plugin_dir: Path | None = build_al_lsp_plugin(entry, category, repo_path, AgentType.CLAUDE, al_lsp=al_lsp, container_name=container_name) instructions_enabled: bool = setup_instructions_from_config(claude_config, entry, repo_path, agent_type=AgentType.CLAUDE) skills_enabled: bool = setup_agent_skills(claude_config, entry, repo_path, agent_type=AgentType.CLAUDE) custom_agent: str | None = setup_custom_agent(claude_config, entry, repo_path, agent_type=AgentType.CLAUDE) diff --git a/src/bcbench/agent/copilot/agent.py b/src/bcbench/agent/copilot/agent.py index 633d194aa..a859a39ab 100644 --- a/src/bcbench/agent/copilot/agent.py +++ b/src/bcbench/agent/copilot/agent.py @@ -8,7 +8,7 @@ import yaml from bcbench.agent.copilot.metrics import parse_metrics -from bcbench.agent.shared import build_copilot_lsp_config, build_mcp_config, build_prompt, parse_tool_usage_from_hooks +from bcbench.agent.shared import build_al_lsp_plugin, build_mcp_config, build_prompt, parse_tool_usage_from_hooks from bcbench.config import get_config from bcbench.dataset import BaseDatasetEntry from bcbench.exceptions import AgentError, AgentTimeoutError @@ -42,14 +42,14 @@ def run_copilot_agent( prompt: str = build_prompt(entry, repo_path, copilot_config, category, al_mcp=al_mcp) mcp_config_json, mcp_server_names = build_mcp_config(copilot_config, entry, repo_path, al_mcp=al_mcp, container_name=container_name) - al_lsp_enabled: bool = build_copilot_lsp_config(entry, category, repo_path, al_lsp=al_lsp, container_name=container_name) + lsp_plugin_dir: Path | None = build_al_lsp_plugin(entry, category, repo_path, AgentType.COPILOT, al_lsp=al_lsp, container_name=container_name) instructions_enabled: bool = setup_instructions_from_config(copilot_config, entry, repo_path, agent_type=AgentType.COPILOT) skills_enabled: bool = setup_agent_skills(copilot_config, entry, repo_path, agent_type=AgentType.COPILOT) custom_agent: str | None = setup_custom_agent(copilot_config, entry, repo_path, agent_type=AgentType.COPILOT) tool_log_path: Path = setup_hooks(repo_path, AgentType.COPILOT, output_dir) config = ExperimentConfiguration( mcp_servers=mcp_server_names, - al_lsp_enabled=al_lsp_enabled, + al_lsp_enabled=lsp_plugin_dir is not None, custom_instructions=instructions_enabled, skills_enabled=skills_enabled, custom_agent=custom_agent, @@ -76,6 +76,8 @@ def run_copilot_agent( cmd_args.append("--no-custom-instructions") if mcp_config_json: cmd_args.append(f"--additional-mcp-config={mcp_config_json}") + if lsp_plugin_dir is not None: + cmd_args.append(f"--plugin-dir={lsp_plugin_dir}") if custom_agent: cmd_args.append(f"--agent={custom_agent}") diff --git a/src/bcbench/agent/shared/__init__.py b/src/bcbench/agent/shared/__init__.py index ce4c1565e..7820106ed 100644 --- a/src/bcbench/agent/shared/__init__.py +++ b/src/bcbench/agent/shared/__init__.py @@ -1,8 +1,8 @@ """Shared code for CLI-based agents (Claude, Copilot).""" from bcbench.agent.shared.hooks_parser import parse_tool_usage_from_hooks -from bcbench.agent.shared.lsp import build_claude_lsp_plugin, build_copilot_lsp_config +from bcbench.agent.shared.lsp import build_al_lsp_plugin from bcbench.agent.shared.mcp import build_mcp_config from bcbench.agent.shared.prompt import build_prompt -__all__ = ["build_claude_lsp_plugin", "build_copilot_lsp_config", "build_mcp_config", "build_prompt", "parse_tool_usage_from_hooks"] +__all__ = ["build_al_lsp_plugin", "build_mcp_config", "build_prompt", "parse_tool_usage_from_hooks"] diff --git a/src/bcbench/agent/shared/lsp.py b/src/bcbench/agent/shared/lsp.py index 4c7af905c..ebb068d09 100644 --- a/src/bcbench/agent/shared/lsp.py +++ b/src/bcbench/agent/shared/lsp.py @@ -1,4 +1,5 @@ import json +import shutil from pathlib import Path from bcbench.agent.shared.altool_paths import ( @@ -10,12 +11,16 @@ from bcbench.dataset import BaseDatasetEntry from bcbench.exceptions import AgentError from bcbench.logger import get_logger -from bcbench.types import EvaluationCategory +from bcbench.types import AgentType, EvaluationCategory logger = get_logger(__name__) -_COPILOT_LSP_RELATIVE_PATH = Path(".github") / "lsp.json" -_CLAUDE_PLUGIN_RELATIVE_PATH = Path(".claude") / "plugins" / "al-lsp" +# Per-task plugin folder location. Both Copilot CLI and Claude Code accept +# `--plugin-dir ` for ad-hoc plugin loading and both look for the +# manifest under `.claude-plugin/plugin.json`, so a single neutral path works +# for either agent. Lives under `.bcbench/` so it's visibly BC-Bench-owned +# and won't collide with either agent's auto-discovery paths. +_AL_LSP_PLUGIN_RELATIVE_PATH = Path(".bcbench") / "al-lsp-plugin" def _resolve_symbol_paths(entry: BaseDatasetEntry, category: EvaluationCategory, container_name: str) -> tuple[list[str], list[str]]: @@ -49,100 +54,68 @@ def _build_lsp_args(project_paths: list[str], package_cache_paths: list[str], as return args -def _prepare_lsp(entry: BaseDatasetEntry, category: EvaluationCategory, repo_path: Path, container_name: str) -> list[str]: - """Common preparation shared by both agent variants: set runtime, resolve symbols, build args.""" - project_paths = [str(repo_path / p) for p in entry.project_paths] - set_runtime_version(project_paths) - package_cache_paths, assembly_probing_paths = _resolve_symbol_paths(entry, category, container_name) - return _build_lsp_args(project_paths, package_cache_paths, assembly_probing_paths) +def _lsp_config_for(agent_type: AgentType, args: list[str]) -> dict: + """Build the agent-specific `.lsp.json` content. + Both agents launch the same `al launchlspserver` process — only the surrounding + LSP-routing schema differs: -def build_copilot_lsp_config(entry: BaseDatasetEntry, category: EvaluationCategory, repo_path: Path, al_lsp: bool, container_name: str = "") -> bool: - """Write Copilot CLI's project-level LSP config to /.github/lsp.json. + - Copilot CLI expects `{ "lspServers": { name: { ..., "fileExtensions": {".ext": "lang"} } } }` + - Claude Code expects `{ name: { ..., "extensionToLanguage": {".ext": "lang"} } }` (no wrapper, different extension key) - Copilot CLI auto-discovers ``.github/lsp.json`` on session start — no CLI flag needed. - Removes any stale config when ``al_lsp=False`` so toggling the flag off actually disables the server. + `command: "al"` is unqualified by design: Copilot CLI silently rejects absolute paths in LSP + `command` ("Server is configured but not available"), so the published `altool` wrapper + (`al`) must resolve via PATH on both sides. """ - lsp_config_path = repo_path / _COPILOT_LSP_RELATIVE_PATH + server = {"command": "al", "args": args} + match agent_type: + case AgentType.COPILOT: + return {"lspServers": {"altool": {**server, "fileExtensions": {".al": "al"}}}} + case AgentType.CLAUDE: + return {"altool": {**server, "extensionToLanguage": {".al": "al"}}} - if not al_lsp: - if lsp_config_path.is_file(): - lsp_config_path.unlink() - logger.info(f"Removed stale LSP config: {lsp_config_path}") - return False - - args = _prepare_lsp(entry, category, repo_path, container_name) - - # Copilot CLI resolves `command` via PATH (absolute paths are silently rejected with - # "Server is configured but not available"). `al` is the published altool - # wrapper installed via the .NET tool — it must be on PATH. - lsp_config = { - "lspServers": { - "altool": { - "command": "al", - "args": args, - "fileExtensions": {".al": "al"}, - } - } - } - - lsp_config_path.parent.mkdir(parents=True, exist_ok=True) - lsp_config_path.write_text(json.dumps(lsp_config, indent=2), encoding="utf-8") - logger.info(f"Wrote Copilot LSP config: {lsp_config_path}") - logger.debug(f"LSP configuration: {json.dumps(lsp_config, indent=2)}") +def build_al_lsp_plugin(entry: BaseDatasetEntry, category: EvaluationCategory, repo_path: Path, agent_type: AgentType, al_lsp: bool, container_name: str = "") -> Path | None: + """Build a per-task plugin folder containing the AL LSP server, return its path or None. - return True + Both Copilot CLI and Claude Code load this via ``--plugin-dir `` for a single session + — no marketplace registration, no global state, no cross-run plugin leakage. The plugin + folder layout is identical between agents; only the LSP-routing schema in ``.lsp.json`` + differs (see :func:`_lsp_config_for`). + Layout written under ``/.bcbench/al-lsp-plugin/``:: -def build_claude_lsp_plugin(entry: BaseDatasetEntry, category: EvaluationCategory, repo_path: Path, al_lsp: bool, container_name: str = "") -> Path | None: - """Build a per-task Claude Code plugin folder containing the AL LSP server. + .claude-plugin/plugin.json — minimal manifest (only ``name`` is required; + both CLIs check this path) + .lsp.json — LSP server config in the agent's schema - Claude Code surfaces LSP servers through its plugin system. ``claude --plugin-dir `` - loads a plugin for a single session with no marketplace registration, no - ``ENABLE_LSP_TOOL`` global state, and no cross-run plugin leakage — the same - per-task isolation the Copilot side gets from ``.github/lsp.json``. - - Layout written under ``/.claude/plugins/al-lsp/``: - .claude-plugin/plugin.json — minimal manifest (only ``name`` is required) - .lsp.json — LSP server config in Claude's schema - - Returns the plugin folder path so the caller can pass it as ``--plugin-dir``, - or None when disabled. + Returns the plugin folder path (to be passed as ``--plugin-dir``), or None when disabled. """ - plugin_dir = repo_path / _CLAUDE_PLUGIN_RELATIVE_PATH + plugin_dir = repo_path / _AL_LSP_PLUGIN_RELATIVE_PATH if not al_lsp: if plugin_dir.exists(): - for p in sorted(plugin_dir.rglob("*"), reverse=True): - p.rmdir() if p.is_dir() else p.unlink() - plugin_dir.rmdir() - logger.info(f"Removed stale Claude LSP plugin: {plugin_dir}") + shutil.rmtree(plugin_dir) + logger.info(f"Removed stale AL LSP plugin: {plugin_dir}") return None - args = _prepare_lsp(entry, category, repo_path, container_name) + project_paths = [str(repo_path / p) for p in entry.project_paths] + set_runtime_version(project_paths) + package_cache_paths, assembly_probing_paths = _resolve_symbol_paths(entry, category, container_name) + args = _build_lsp_args(project_paths, package_cache_paths, assembly_probing_paths) - # Minimal plugin manifest. Name is the only required field; we don't ship skills/agents/hooks. plugin_manifest = { "name": "al-lsp", "version": "1.0.0", "description": "AL Language Server for Business Central agentic development", } - # Claude's LSP schema differs from Copilot's: no `lspServers` wrapper, and uses - # `extensionToLanguage` instead of `fileExtensions`. - lsp_config = { - "altool": { - "command": "al", - "args": args, - "extensionToLanguage": {".al": "al"}, - } - } + lsp_config = _lsp_config_for(agent_type, args) (plugin_dir / ".claude-plugin").mkdir(parents=True, exist_ok=True) (plugin_dir / ".claude-plugin" / "plugin.json").write_text(json.dumps(plugin_manifest, indent=2), encoding="utf-8") (plugin_dir / ".lsp.json").write_text(json.dumps(lsp_config, indent=2), encoding="utf-8") - logger.info(f"Wrote Claude LSP plugin: {plugin_dir}") + logger.info(f"Wrote AL LSP plugin for {agent_type.value}: {plugin_dir}") logger.debug(f"LSP configuration: {json.dumps(lsp_config, indent=2)}") return plugin_dir diff --git a/tests/test_lsp_config.py b/tests/test_lsp_config.py index ffca778e2..e06d72551 100644 --- a/tests/test_lsp_config.py +++ b/tests/test_lsp_config.py @@ -4,11 +4,13 @@ import pytest -from bcbench.agent.shared.lsp import build_claude_lsp_plugin, build_copilot_lsp_config +from bcbench.agent.shared.lsp import build_al_lsp_plugin from bcbench.exceptions import AgentError -from bcbench.types import EvaluationCategory +from bcbench.types import AgentType, EvaluationCategory from tests.conftest import create_dataset_entry +_PLUGIN_REL = Path(".bcbench") / "al-lsp-plugin" + @pytest.fixture def entry(): @@ -35,201 +37,136 @@ def no_artifacts(): yield m -class TestCopilotLspConfig: - @staticmethod - def _read(repo_path: Path) -> dict: - return json.loads((repo_path / ".github" / "lsp.json").read_text(encoding="utf-8")) +def _read_lsp(repo_path: Path) -> dict: + return json.loads((repo_path / _PLUGIN_REL / ".lsp.json").read_text(encoding="utf-8")) - @staticmethod - def _build(entry, repo_path, **kwargs): - return build_copilot_lsp_config(entry, EvaluationCategory.BUG_FIX, repo_path, **kwargs) - def test_returns_false_when_disabled(self, entry, repo_path): - assert self._build(entry, repo_path, al_lsp=False) is False - assert not (repo_path / ".github" / "lsp.json").exists() +def _read_manifest(repo_path: Path) -> dict: + return json.loads((repo_path / _PLUGIN_REL / ".claude-plugin" / "plugin.json").read_text(encoding="utf-8")) - def test_removes_stale_config_when_disabled(self, entry, repo_path): - lsp_path = repo_path / ".github" / "lsp.json" - lsp_path.parent.mkdir(parents=True) - lsp_path.write_text("{}") - self._build(entry, repo_path, al_lsp=False) +def _build(entry, repo_path, agent_type: AgentType, **kwargs): + return build_al_lsp_plugin(entry, EvaluationCategory.BUG_FIX, repo_path, agent_type, **kwargs) - assert not lsp_path.exists() - @pytest.mark.usefixtures("artifact_paths") - def test_returns_true_when_enabled(self, entry, repo_path): - assert self._build(entry, repo_path, al_lsp=True) is True +@pytest.fixture(params=[AgentType.COPILOT, AgentType.CLAUDE], ids=lambda a: a.value) +def agent_type(request) -> AgentType: + """Parametrize across both agents — every shared behavior gets tested twice.""" + return request.param - @pytest.mark.usefixtures("artifact_paths") - def test_writes_project_lsp_config(self, entry, repo_path): - self._build(entry, repo_path, al_lsp=True) - config = self._read(repo_path) - assert "lspServers" in config - assert "altool" in config["lspServers"] +class TestSharedBehavior: + """Behavior that must hold for both Copilot and Claude variants.""" - @pytest.mark.usefixtures("artifact_paths") - def test_command_is_unqualified_al(self, entry, repo_path): - self._build(entry, repo_path, al_lsp=True) + def test_returns_none_when_disabled(self, entry, repo_path, agent_type): + assert _build(entry, repo_path, agent_type, al_lsp=False) is None + assert not (repo_path / _PLUGIN_REL).exists() - # Copilot CLI silently rejects absolute command paths — must resolve via PATH. - assert self._read(repo_path)["lspServers"]["altool"]["command"] == "al" + def test_removes_stale_plugin_when_disabled(self, entry, repo_path, agent_type): + plugin_dir = repo_path / _PLUGIN_REL + (plugin_dir / ".claude-plugin").mkdir(parents=True) + (plugin_dir / ".claude-plugin" / "plugin.json").write_text("{}") + (plugin_dir / ".lsp.json").write_text("{}") + + _build(entry, repo_path, agent_type, al_lsp=False) + + assert not plugin_dir.exists() @pytest.mark.usefixtures("artifact_paths") - def test_al_file_extension_registered(self, entry, repo_path): - self._build(entry, repo_path, al_lsp=True) + def test_returns_plugin_dir_when_enabled(self, entry, repo_path, agent_type): + assert _build(entry, repo_path, agent_type, al_lsp=True) == repo_path / _PLUGIN_REL - assert self._read(repo_path)["lspServers"]["altool"]["fileExtensions"] == {".al": "al"} + @pytest.mark.usefixtures("artifact_paths") + def test_writes_minimal_manifest(self, entry, repo_path, agent_type): + _build(entry, repo_path, agent_type, al_lsp=True) + assert _read_manifest(repo_path)["name"] == "al-lsp" # only required field @pytest.mark.usefixtures("artifact_paths") - def test_project_paths_inserted_after_launchlspserver(self, entry, repo_path): - self._build(entry, repo_path, al_lsp=True) + def test_command_is_unqualified_al(self, entry, repo_path, agent_type): + # Copilot CLI silently rejects absolute command paths in LSP `command`; must resolve via PATH. + _build(entry, repo_path, agent_type, al_lsp=True) + config = _read_lsp(repo_path) + # Navigate to the server entry regardless of schema wrapper: + server = config["lspServers"]["altool"] if "lspServers" in config else config["altool"] + assert server["command"] == "al" - args = self._read(repo_path)["lspServers"]["altool"]["args"] + @pytest.mark.usefixtures("artifact_paths") + def test_project_paths_inserted_after_launchlspserver(self, entry, repo_path, agent_type): + _build(entry, repo_path, agent_type, al_lsp=True) + config = _read_lsp(repo_path) + server = config["lspServers"]["altool"] if "lspServers" in config else config["altool"] + args = server["args"] launch_idx = args.index("launchlspserver") assert args[launch_idx + 1] == str(repo_path / "src/App") assert args[launch_idx + 2] == str(repo_path / "src/TestApp") @pytest.mark.usefixtures("artifact_paths") - def test_artifact_cache_paths_used_for_package_cache(self, entry, repo_path): - self._build(entry, repo_path, al_lsp=True) - - args = self._read(repo_path)["lspServers"]["altool"]["args"] + def test_artifact_cache_paths_used_for_package_cache(self, entry, repo_path, agent_type): + _build(entry, repo_path, agent_type, al_lsp=True) + config = _read_lsp(repo_path) + server = config["lspServers"]["altool"] if "lspServers" in config else config["altool"] + args = server["args"] cache_idx = args.index("--packagecachepath") probing_idx = args.index("--assemblyprobingpaths") assert args[cache_idx + 1 : probing_idx] == ["C:/cache/w1/Extensions", "C:/cache/platform/Applications"] - @pytest.mark.usefixtures("artifact_paths") - def test_does_not_require_container_when_artifacts_present(self, entry, repo_path): - assert self._build(entry, repo_path, al_lsp=True, container_name="") is True - @pytest.mark.usefixtures("no_artifacts") - def test_uses_container_compiler_folder_when_present(self, entry, repo_path, tmp_path): + def test_uses_container_compiler_folder_when_present(self, entry, repo_path, agent_type, tmp_path): compiler_root = tmp_path / "compiler" / "test-container" (compiler_root / "symbols").mkdir(parents=True) with patch( "bcbench.agent.shared.lsp.compiler_symbol_folder_for_container", return_value=(compiler_root, compiler_root / "symbols"), ): - assert self._build(entry, repo_path, al_lsp=True, container_name="test-container") is True + _build(entry, repo_path, agent_type, al_lsp=True, container_name="test-container") - args = self._read(repo_path)["lspServers"]["altool"]["args"] - cache_idx = args.index("--packagecachepath") - assert args[cache_idx + 1] == str(compiler_root / "symbols") + config = _read_lsp(repo_path) + server = config["lspServers"]["altool"] if "lspServers" in config else config["altool"] + cache_idx = server["args"].index("--packagecachepath") + assert server["args"][cache_idx + 1] == str(compiler_root / "symbols") @pytest.mark.usefixtures("artifact_paths") - def test_container_compiler_folder_wins_over_artifact_cache(self, entry, repo_path, tmp_path): + def test_container_compiler_folder_wins_over_artifact_cache(self, entry, repo_path, agent_type, tmp_path): + # When BOTH sources exist, the container compiler folder must win — same arg + # shape as AL-MCP, easier to debug a "which symbols set is this?" question. compiler_root = tmp_path / "compiler" / "test-container" (compiler_root / "symbols").mkdir(parents=True) with patch( "bcbench.agent.shared.lsp.compiler_symbol_folder_for_container", return_value=(compiler_root, compiler_root / "symbols"), ): - self._build(entry, repo_path, al_lsp=True, container_name="test-container") + _build(entry, repo_path, agent_type, al_lsp=True, container_name="test-container") - args = self._read(repo_path)["lspServers"]["altool"]["args"] + config = _read_lsp(repo_path) + server = config["lspServers"]["altool"] if "lspServers" in config else config["altool"] + args = server["args"] cache_idx = args.index("--packagecachepath") end_idx = args.index("--assemblyprobingpaths") if "--assemblyprobingpaths" in args else len(args) assert args[cache_idx + 1 : end_idx] == [str(compiler_root / "symbols")] @pytest.mark.usefixtures("no_artifacts") - def test_raises_with_download_hint_when_neither_source_available(self, entry, repo_path): + def test_raises_with_download_hint_when_neither_source_available(self, entry, repo_path, agent_type): with pytest.raises(AgentError, match=r"Download-BCSymbols\.ps1"): - self._build(entry, repo_path, al_lsp=True, container_name="") - - -class TestClaudeLspPlugin: - @staticmethod - def _plugin_dir(repo_path: Path) -> Path: - return repo_path / ".claude" / "plugins" / "al-lsp" - - @staticmethod - def _read_lsp(repo_path: Path) -> dict: - return json.loads((TestClaudeLspPlugin._plugin_dir(repo_path) / ".lsp.json").read_text(encoding="utf-8")) - - @staticmethod - def _read_manifest(repo_path: Path) -> dict: - return json.loads((TestClaudeLspPlugin._plugin_dir(repo_path) / ".claude-plugin" / "plugin.json").read_text(encoding="utf-8")) - - @staticmethod - def _build(entry, repo_path, **kwargs): - return build_claude_lsp_plugin(entry, EvaluationCategory.BUG_FIX, repo_path, **kwargs) + _build(entry, repo_path, agent_type, al_lsp=True, container_name="") - def test_returns_none_when_disabled(self, entry, repo_path): - assert self._build(entry, repo_path, al_lsp=False) is None - assert not self._plugin_dir(repo_path).exists() - - def test_removes_stale_plugin_when_disabled(self, entry, repo_path): - plugin_dir = self._plugin_dir(repo_path) - (plugin_dir / ".claude-plugin").mkdir(parents=True) - (plugin_dir / ".claude-plugin" / "plugin.json").write_text("{}") - (plugin_dir / ".lsp.json").write_text("{}") - - self._build(entry, repo_path, al_lsp=False) - - assert not plugin_dir.exists() - @pytest.mark.usefixtures("artifact_paths") - def test_returns_plugin_dir_when_enabled(self, entry, repo_path): - plugin_dir = self._build(entry, repo_path, al_lsp=True) - assert plugin_dir == self._plugin_dir(repo_path) +class TestAgentSpecificSchema: + """Each agent's `.lsp.json` schema differs slightly — verify the right keys land for each.""" @pytest.mark.usefixtures("artifact_paths") - def test_writes_minimal_manifest(self, entry, repo_path): - self._build(entry, repo_path, al_lsp=True) - - manifest = self._read_manifest(repo_path) - assert manifest["name"] == "al-lsp" # the only required field per Claude plugin docs + def test_copilot_uses_lspservers_wrapper_with_file_extensions(self, entry, repo_path): + _build(entry, repo_path, AgentType.COPILOT, al_lsp=True) + config = _read_lsp(repo_path) + # Copilot: `lspServers` wrapper + `fileExtensions`. + assert "lspServers" in config + assert config["lspServers"]["altool"]["fileExtensions"] == {".al": "al"} + assert "extensionToLanguage" not in config["lspServers"]["altool"] @pytest.mark.usefixtures("artifact_paths") - def test_writes_lsp_config_at_plugin_root(self, entry, repo_path): - self._build(entry, repo_path, al_lsp=True) - - config = self._read_lsp(repo_path) - # Claude's schema: top-level server name (no `lspServers` wrapper, unlike Copilot). - assert "altool" in config + def test_claude_uses_flat_schema_with_extension_to_language(self, entry, repo_path): + _build(entry, repo_path, AgentType.CLAUDE, al_lsp=True) + config = _read_lsp(repo_path) + # Claude: top-level server name (no wrapper) + `extensionToLanguage`. assert "lspServers" not in config - - @pytest.mark.usefixtures("artifact_paths") - def test_extension_to_language_uses_claude_schema(self, entry, repo_path): - self._build(entry, repo_path, al_lsp=True) - - # Claude uses `extensionToLanguage`; Copilot uses `fileExtensions`. - server = self._read_lsp(repo_path)["altool"] - assert server["extensionToLanguage"] == {".al": "al"} - assert "fileExtensions" not in server - - @pytest.mark.usefixtures("artifact_paths") - def test_command_is_unqualified_al(self, entry, repo_path): - self._build(entry, repo_path, al_lsp=True) - - assert self._read_lsp(repo_path)["altool"]["command"] == "al" - - @pytest.mark.usefixtures("artifact_paths") - def test_project_paths_inserted_after_launchlspserver(self, entry, repo_path): - self._build(entry, repo_path, al_lsp=True) - - args = self._read_lsp(repo_path)["altool"]["args"] - launch_idx = args.index("launchlspserver") - assert args[launch_idx + 1] == str(repo_path / "src/App") - assert args[launch_idx + 2] == str(repo_path / "src/TestApp") - - @pytest.mark.usefixtures("no_artifacts") - def test_uses_container_compiler_folder_when_present(self, entry, repo_path, tmp_path): - compiler_root = tmp_path / "compiler" / "test-container" - (compiler_root / "symbols").mkdir(parents=True) - with patch( - "bcbench.agent.shared.lsp.compiler_symbol_folder_for_container", - return_value=(compiler_root, compiler_root / "symbols"), - ): - assert self._build(entry, repo_path, al_lsp=True, container_name="test-container") is not None - - args = self._read_lsp(repo_path)["altool"]["args"] - cache_idx = args.index("--packagecachepath") - assert args[cache_idx + 1] == str(compiler_root / "symbols") - - @pytest.mark.usefixtures("no_artifacts") - def test_raises_with_download_hint_when_neither_source_available(self, entry, repo_path): - with pytest.raises(AgentError, match=r"Download-BCSymbols\.ps1"): - self._build(entry, repo_path, al_lsp=True, container_name="") + assert config["altool"]["extensionToLanguage"] == {".al": "al"} + assert "fileExtensions" not in config["altool"]