diff --git a/clink/agents/__init__.py b/clink/agents/__init__.py index 753f92655..5561d509c 100644 --- a/clink/agents/__init__.py +++ b/clink/agents/__init__.py @@ -7,12 +7,14 @@ from .base import AgentOutput, BaseCLIAgent, CLIAgentError from .claude import ClaudeAgent from .codex import CodexAgent +from .copilot import CopilotAgent from .gemini import GeminiAgent _AGENTS: dict[str, type[BaseCLIAgent]] = { "gemini": GeminiAgent, "codex": CodexAgent, "claude": ClaudeAgent, + "copilot": CopilotAgent, } diff --git a/clink/agents/copilot.py b/clink/agents/copilot.py new file mode 100644 index 000000000..9e1a867b1 --- /dev/null +++ b/clink/agents/copilot.py @@ -0,0 +1,147 @@ +"""GitHub Copilot-specific CLI agent hooks.""" + +from __future__ import annotations + +import asyncio +import shutil +import time +from collections.abc import Sequence + +from clink.constants import DEFAULT_STREAM_LIMIT +from clink.models import ResolvedCLIClient, ResolvedCLIRole +from clink.parsers.base import ParserError + +from .base import AgentOutput, BaseCLIAgent, CLIAgentError + + +class CopilotAgent(BaseCLIAgent): + """Copilot CLI agent that passes the prompt via -p argument.""" + + def __init__(self, client: ResolvedCLIClient): + super().__init__(client) + + async def run( + self, + *, + role: ResolvedCLIRole, + prompt: str, + system_prompt: str | None = None, + files: Sequence[str], + images: Sequence[str], + ) -> AgentOutput: + _ = (files, images) + command = self._build_command(role=role, system_prompt=system_prompt) + # Copilot uses -p for non-interactive mode instead of stdin. + command.extend(["-p", prompt]) + env = self._build_environment() + + executable_name = command[0] + resolved_executable = shutil.which(executable_name) + if resolved_executable is None: + raise CLIAgentError( + f"Executable '{executable_name}' not found in PATH for CLI '{self.client.name}'. " + f"Ensure the command is installed and accessible." + ) + command[0] = resolved_executable + + sanitized_command = ["[REDACTED]" if i > 0 and command[i - 1] == "-p" else arg for i, arg in enumerate(command)] + cwd = str(self.client.working_dir) if self.client.working_dir else None + limit = DEFAULT_STREAM_LIMIT + start_time = time.monotonic() + + self._logger.debug("Executing CLI command: %s", " ".join(sanitized_command)) + + try: + process = await asyncio.create_subprocess_exec( + *command, + stdin=asyncio.subprocess.DEVNULL, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + cwd=cwd, + limit=limit, + env=env, + ) + except FileNotFoundError as exc: + raise CLIAgentError(f"Executable not found for CLI '{self.client.name}': {exc}") from exc + + try: + stdout_bytes, stderr_bytes = await asyncio.wait_for( + process.communicate(), + timeout=self.client.timeout_seconds, + ) + except asyncio.TimeoutError as exc: + process.kill() + await process.communicate() + raise CLIAgentError( + f"CLI '{self.client.name}' timed out after {self.client.timeout_seconds} seconds", + returncode=None, + ) from exc + + duration = time.monotonic() - start_time + return_code = process.returncode + stdout_text = stdout_bytes.decode("utf-8", errors="replace") + stderr_text = stderr_bytes.decode("utf-8", errors="replace") + + if return_code != 0: + recovered = self._recover_from_error( + returncode=return_code, + stdout=stdout_text, + stderr=stderr_text, + sanitized_command=sanitized_command, + duration_seconds=duration, + output_file_content=None, + ) + if recovered is not None: + return recovered + raise CLIAgentError( + f"CLI '{self.client.name}' exited with status {return_code}", + returncode=return_code, + stdout=stdout_text, + stderr=stderr_text, + ) + + try: + parsed = self._parser.parse(stdout_text, stderr_text) + except ParserError as exc: + raise CLIAgentError( + f"Failed to parse output from CLI '{self.client.name}': {exc}", + returncode=return_code, + stdout=stdout_text, + stderr=stderr_text, + ) from exc + + return AgentOutput( + parsed=parsed, + sanitized_command=sanitized_command, + returncode=return_code, + stdout=stdout_text, + stderr=stderr_text, + duration_seconds=duration, + parser_name=self._parser.name, + ) + + def _recover_from_error( + self, + *, + returncode: int, + stdout: str, + stderr: str, + sanitized_command: list[str], + duration_seconds: float, + output_file_content: str | None, + ) -> AgentOutput | None: + try: + parsed = self._parser.parse(stdout, stderr) + except ParserError: + return None + + return AgentOutput( + parsed=parsed, + sanitized_command=sanitized_command, + returncode=returncode, + stdout=stdout, + stderr=stderr, + duration_seconds=duration_seconds, + parser_name=self._parser.name, + output_file_content=output_file_content, + ) diff --git a/clink/constants.py b/clink/constants.py index 54b8caf72..64f03a6fb 100644 --- a/clink/constants.py +++ b/clink/constants.py @@ -45,4 +45,10 @@ class CLIInternalDefaults: default_role_prompt="systemprompts/clink/default.txt", runner="claude", ), + "copilot": CLIInternalDefaults( + parser="copilot_jsonl", + additional_args=["--output-format", "json"], + default_role_prompt="systemprompts/clink/default.txt", + runner="copilot", + ), } diff --git a/clink/parsers/__init__.py b/clink/parsers/__init__.py index 864224258..04fa2565b 100644 --- a/clink/parsers/__init__.py +++ b/clink/parsers/__init__.py @@ -5,12 +5,14 @@ from .base import BaseParser, ParsedCLIResponse, ParserError from .claude import ClaudeJSONParser from .codex import CodexJSONLParser +from .copilot import CopilotJSONLParser from .gemini import GeminiJSONParser _PARSER_CLASSES: dict[str, type[BaseParser]] = { CodexJSONLParser.name: CodexJSONLParser, GeminiJSONParser.name: GeminiJSONParser, ClaudeJSONParser.name: ClaudeJSONParser, + CopilotJSONLParser.name: CopilotJSONLParser, } diff --git a/clink/parsers/copilot.py b/clink/parsers/copilot.py new file mode 100644 index 000000000..695c1dd4b --- /dev/null +++ b/clink/parsers/copilot.py @@ -0,0 +1,60 @@ +"""Parser for GitHub Copilot CLI JSONL output.""" + +from __future__ import annotations + +import json +from typing import Any + +from .base import BaseParser, ParsedCLIResponse, ParserError + + +class CopilotJSONLParser(BaseParser): + """Parse JSONL stdout emitted by `copilot -p ... --output-format json`.""" + + name = "copilot_jsonl" + + def parse(self, stdout: str, stderr: str) -> ParsedCLIResponse: + lines = [line.strip() for line in (stdout or "").splitlines() if line.strip()] + events: list[dict[str, Any]] = [] + assistant_messages: list[str] = [] + result_event: dict[str, Any] | None = None + + for line in lines: + if not line.startswith("{"): + continue + try: + event = json.loads(line) + except json.JSONDecodeError: + continue + + events.append(event) + event_type = event.get("type") + + if event_type == "assistant.message": + data = event.get("data") or {} + content = data.get("content") + if isinstance(content, str) and content.strip(): + assistant_messages.append(content.strip()) + + elif event_type == "result": + result_event = event + + if not assistant_messages: + raise ParserError("Copilot CLI JSONL output did not include an assistant.message event") + + content = "\n\n".join(assistant_messages).strip() + metadata: dict[str, Any] = {"events": events} + + if result_event: + metadata["result"] = result_event + session_id = result_event.get("sessionId") + if isinstance(session_id, str): + metadata["session_id"] = session_id + usage = result_event.get("usage") + if isinstance(usage, dict): + metadata["usage"] = usage + + if stderr and stderr.strip(): + metadata["stderr"] = stderr.strip() + + return ParsedCLIResponse(content=content, metadata=metadata) diff --git a/conf/cli_clients/copilot.json b/conf/cli_clients/copilot.json new file mode 100644 index 000000000..cfca6de5d --- /dev/null +++ b/conf/cli_clients/copilot.json @@ -0,0 +1,20 @@ +{ + "name": "copilot", + "command": "copilot", + "additional_args": [], + "env": {}, + "roles": { + "default": { + "prompt_path": "systemprompts/clink/default.txt", + "role_args": [] + }, + "planner": { + "prompt_path": "systemprompts/clink/default_planner.txt", + "role_args": [] + }, + "codereviewer": { + "prompt_path": "systemprompts/clink/default_codereviewer.txt", + "role_args": [] + } + } +} diff --git a/simulator_tests/test_chat_simple_validation.py b/simulator_tests/test_chat_simple_validation.py index a452d71e9..c6709584d 100644 --- a/simulator_tests/test_chat_simple_validation.py +++ b/simulator_tests/test_chat_simple_validation.py @@ -13,7 +13,6 @@ - Conversation context preservation across turns """ - from .conversation_base_test import ConversationBaseTest diff --git a/simulator_tests/test_conversation_chain_validation.py b/simulator_tests/test_conversation_chain_validation.py index 2d70b862b..5ca53338d 100644 --- a/simulator_tests/test_conversation_chain_validation.py +++ b/simulator_tests/test_conversation_chain_validation.py @@ -21,7 +21,6 @@ - Properly traverse parent relationships for history reconstruction """ - from .conversation_base_test import ConversationBaseTest diff --git a/simulator_tests/test_cross_tool_comprehensive.py b/simulator_tests/test_cross_tool_comprehensive.py index 8389953ec..6cdd33901 100644 --- a/simulator_tests/test_cross_tool_comprehensive.py +++ b/simulator_tests/test_cross_tool_comprehensive.py @@ -12,7 +12,6 @@ 5. Proper tool chaining with context """ - from .conversation_base_test import ConversationBaseTest diff --git a/simulator_tests/test_ollama_custom_url.py b/simulator_tests/test_ollama_custom_url.py index f23b6ee8d..f40c1e106 100644 --- a/simulator_tests/test_ollama_custom_url.py +++ b/simulator_tests/test_ollama_custom_url.py @@ -9,7 +9,6 @@ - Model alias resolution for local models """ - from .base_test import BaseSimulatorTest diff --git a/simulator_tests/test_openrouter_fallback.py b/simulator_tests/test_openrouter_fallback.py index 91fc058ab..74023437f 100644 --- a/simulator_tests/test_openrouter_fallback.py +++ b/simulator_tests/test_openrouter_fallback.py @@ -8,7 +8,6 @@ - Auto mode correctly selects OpenRouter models """ - from .base_test import BaseSimulatorTest diff --git a/simulator_tests/test_openrouter_models.py b/simulator_tests/test_openrouter_models.py index bd69806a5..5fb3348bb 100644 --- a/simulator_tests/test_openrouter_models.py +++ b/simulator_tests/test_openrouter_models.py @@ -9,7 +9,6 @@ - Error handling when models are not available """ - from .base_test import BaseSimulatorTest diff --git a/simulator_tests/test_xai_models.py b/simulator_tests/test_xai_models.py index 41c57e3a4..e8d32740a 100644 --- a/simulator_tests/test_xai_models.py +++ b/simulator_tests/test_xai_models.py @@ -9,7 +9,6 @@ - API integration and response validation """ - from .base_test import BaseSimulatorTest diff --git a/tests/test_clink_copilot_agent.py b/tests/test_clink_copilot_agent.py new file mode 100644 index 000000000..c0be6a820 --- /dev/null +++ b/tests/test_clink_copilot_agent.py @@ -0,0 +1,149 @@ +import asyncio +import shutil +from pathlib import Path + +import pytest + +from clink.agents.base import CLIAgentError +from clink.agents.copilot import CopilotAgent +from clink.models import ResolvedCLIClient, ResolvedCLIRole + + +class DummyProcess: + def __init__(self, *, stdout: bytes = b"", stderr: bytes = b"", returncode: int = 0): + self._stdout = stdout + self._stderr = stderr + self.returncode = returncode + + async def communicate(self): + return self._stdout, self._stderr + + +@pytest.fixture() +def copilot_agent(): + prompt_path = Path("systemprompts/clink/default.txt").resolve() + role = ResolvedCLIRole(name="default", prompt_path=prompt_path, role_args=[]) + client = ResolvedCLIClient( + name="copilot", + executable=["copilot"], + internal_args=[], + config_args=["--allow-all-tools", "--output-format", "json"], + env={}, + timeout_seconds=30, + parser="copilot_jsonl", + roles={"default": role}, + output_to_file=None, + working_dir=None, + ) + return CopilotAgent(client), role + + +async def _run_agent_with_process(monkeypatch, agent, role, process): + async def fake_create_subprocess_exec(*_args, **_kwargs): + return process + + def fake_which(executable_name): + return f"/usr/bin/{executable_name}" + + monkeypatch.setattr(asyncio, "create_subprocess_exec", fake_create_subprocess_exec) + monkeypatch.setattr(shutil, "which", fake_which) + return await agent.run(role=role, prompt="do something", files=[], images=[]) + + +@pytest.mark.asyncio +async def test_copilot_agent_success(monkeypatch, copilot_agent): + agent, role = copilot_agent + stdout = ( + b'{"type":"assistant.message","data":{"content":"Hello from Copilot"}}\n' + b'{"type":"result","sessionId":"sess-1","usage":{"input_tokens":10,"output_tokens":5}}\n' + ) + process = DummyProcess(stdout=stdout, returncode=0) + result = await _run_agent_with_process(monkeypatch, agent, role, process) + + assert result.returncode == 0 + assert "Hello from Copilot" in result.parsed.content + assert result.parsed.metadata["usage"]["output_tokens"] == 5 + # Prompt must be redacted in sanitized_command (metadata/logs) + assert "do something" not in result.sanitized_command + p_idx = result.sanitized_command.index("-p") + assert result.sanitized_command[p_idx + 1] == "[REDACTED]" + + +@pytest.mark.asyncio +async def test_copilot_agent_passes_prompt_via_p_flag(monkeypatch, copilot_agent): + agent, role = copilot_agent + stdout = b'{"type":"assistant.message","data":{"content":"OK"}}\n' + process = DummyProcess(stdout=stdout, returncode=0) + + captured_args = {} + + async def fake_create_subprocess_exec(*args, **kwargs): + captured_args["command"] = list(args) + return process + + def fake_which(executable_name): + return f"/usr/bin/{executable_name}" + + monkeypatch.setattr(asyncio, "create_subprocess_exec", fake_create_subprocess_exec) + monkeypatch.setattr(shutil, "which", fake_which) + await agent.run(role=role, prompt="test prompt", files=[], images=[]) + + cmd = captured_args["command"] + assert "-p" in cmd + idx = cmd.index("-p") + assert cmd[idx + 1] == "test prompt" + + +@pytest.mark.asyncio +async def test_copilot_agent_uses_devnull_stdin(monkeypatch, copilot_agent): + agent, role = copilot_agent + stdout = b'{"type":"assistant.message","data":{"content":"OK"}}\n' + process = DummyProcess(stdout=stdout, returncode=0) + + captured_kwargs = {} + + async def fake_create_subprocess_exec(*args, **kwargs): + captured_kwargs.update(kwargs) + return process + + def fake_which(executable_name): + return f"/usr/bin/{executable_name}" + + monkeypatch.setattr(asyncio, "create_subprocess_exec", fake_create_subprocess_exec) + monkeypatch.setattr(shutil, "which", fake_which) + await agent.run(role=role, prompt="test", files=[], images=[]) + + assert captured_kwargs["stdin"] == asyncio.subprocess.DEVNULL + + +@pytest.mark.asyncio +async def test_copilot_agent_recovers_from_error(monkeypatch, copilot_agent): + agent, role = copilot_agent + stdout = b'{"type":"assistant.message","data":{"content":"Partial output"}}\n' + process = DummyProcess(stdout=stdout, returncode=1) + result = await _run_agent_with_process(monkeypatch, agent, role, process) + + assert result.returncode == 1 + assert "Partial output" in result.parsed.content + + +@pytest.mark.asyncio +async def test_copilot_agent_propagates_unparseable_error(monkeypatch, copilot_agent): + agent, role = copilot_agent + process = DummyProcess(stdout=b"not json at all", returncode=1) + + with pytest.raises(CLIAgentError): + await _run_agent_with_process(monkeypatch, agent, role, process) + + +@pytest.mark.asyncio +async def test_copilot_agent_executable_not_found(monkeypatch, copilot_agent): + agent, role = copilot_agent + + def fake_which(_name): + return None + + monkeypatch.setattr(shutil, "which", fake_which) + + with pytest.raises(CLIAgentError, match="not found in PATH"): + await agent.run(role=role, prompt="test", files=[], images=[]) diff --git a/tests/test_clink_copilot_parser.py b/tests/test_clink_copilot_parser.py new file mode 100644 index 000000000..bb301836d --- /dev/null +++ b/tests/test_clink_copilot_parser.py @@ -0,0 +1,61 @@ +"""Tests for the Copilot CLI JSONL parser.""" + +import pytest + +from clink.parsers.base import ParserError +from clink.parsers.copilot import CopilotJSONLParser + + +def test_copilot_parser_success(): + parser = CopilotJSONLParser() + stdout = ( + '{"type":"assistant.message","data":{"content":"Hello from Copilot"}}\n' + '{"type":"result","sessionId":"sess-123","usage":{"input_tokens":10,"output_tokens":5}}\n' + ) + parsed = parser.parse(stdout=stdout, stderr="") + assert parsed.content == "Hello from Copilot" + assert parsed.metadata["session_id"] == "sess-123" + assert parsed.metadata["usage"]["output_tokens"] == 5 + + +def test_copilot_parser_multiple_assistant_messages(): + parser = CopilotJSONLParser() + stdout = ( + '{"type":"assistant.message","data":{"content":"First part"}}\n' + '{"type":"assistant.message","data":{"content":"Second part"}}\n' + '{"type":"result","sessionId":"sess-456"}\n' + ) + parsed = parser.parse(stdout=stdout, stderr="") + assert "First part" in parsed.content + assert "Second part" in parsed.content + + +def test_copilot_parser_requires_assistant_message(): + parser = CopilotJSONLParser() + stdout = '{"type":"result","sessionId":"sess-789"}\n' + with pytest.raises(ParserError): + parser.parse(stdout=stdout, stderr="") + + +def test_copilot_parser_skips_non_json_lines(): + parser = CopilotJSONLParser() + stdout = "some debug output\n" '{"type":"assistant.message","data":{"content":"Answer"}}\n' '{"type":"result"}\n' + parsed = parser.parse(stdout=stdout, stderr="") + assert parsed.content == "Answer" + + +def test_copilot_parser_captures_stderr(): + parser = CopilotJSONLParser() + stdout = '{"type":"assistant.message","data":{"content":"OK"}}\n' + parsed = parser.parse(stdout=stdout, stderr="warning: something") + assert parsed.metadata["stderr"] == "warning: something" + + +def test_copilot_parser_empty_content_skipped(): + parser = CopilotJSONLParser() + stdout = ( + '{"type":"assistant.message","data":{"content":" "}}\n' + '{"type":"assistant.message","data":{"content":"Real answer"}}\n' + ) + parsed = parser.parse(stdout=stdout, stderr="") + assert parsed.content == "Real answer" diff --git a/tests/test_directory_expansion_tracking.py b/tests/test_directory_expansion_tracking.py index f4e56a019..79ac5adf9 100644 --- a/tests/test_directory_expansion_tracking.py +++ b/tests/test_directory_expansion_tracking.py @@ -37,8 +37,7 @@ def temp_directory_with_files(self, project_path): files = [] for i in range(5): swift_file = temp_path / f"File{i}.swift" - swift_file.write_text( - f""" + swift_file.write_text(f""" import Foundation class TestClass{i} {{ @@ -46,18 +45,15 @@ class TestClass{i} {{ return "test{i}" }} }} -""" - ) +""") files.append(str(swift_file)) # Create a Python file as well python_file = temp_path / "helper.py" - python_file.write_text( - """ + python_file.write_text(""" def helper_function(): return "helper" -""" - ) +""") files.append(str(python_file)) try: diff --git a/tests/test_docker_implementation.py b/tests/test_docker_implementation.py index d93ca9ff4..ad99976e3 100644 --- a/tests/test_docker_implementation.py +++ b/tests/test_docker_implementation.py @@ -310,13 +310,11 @@ def temp_project_dir(): # Create base files (temp_path / "server.py").write_text("# Mock server.py") - (temp_path / "Dockerfile").write_text( - """ + (temp_path / "Dockerfile").write_text(""" FROM python:3.11-slim COPY server.py /app/ CMD ["python", "/app/server.py"] -""" - ) +""") yield temp_path diff --git a/tests/test_prompt_regression.py b/tests/test_prompt_regression.py index bf40164c7..a2bdf45c7 100644 --- a/tests/test_prompt_regression.py +++ b/tests/test_prompt_regression.py @@ -86,16 +86,14 @@ async def test_chat_with_files(self): # Create a temporary Python file for testing with tempfile.NamedTemporaryFile(mode="w", suffix=".py", delete=False) as f: - f.write( - """ + f.write(""" def hello_world(): \"\"\"A simple hello world function.\"\"\" return "Hello, World!" if __name__ == "__main__": print(hello_world()) -""" - ) +""") temp_file = f.name try: @@ -155,8 +153,7 @@ async def test_codereview_normal_review(self): # Create a temporary Python file for testing with tempfile.NamedTemporaryFile(mode="w", suffix=".py", delete=False) as f: - f.write( - """ + f.write(""" def process_user_input(user_input): # Potentially unsafe code for demonstration query = f"SELECT * FROM users WHERE name = '{user_input}'" @@ -166,8 +163,7 @@ def main(): user_name = input("Enter name: ") result = process_user_input(user_name) print(result) -""" - ) +""") temp_file = f.name try: @@ -241,8 +237,7 @@ async def test_analyze_normal_question(self): # Create a temporary Python file demonstrating MVC pattern with tempfile.NamedTemporaryFile(mode="w", suffix=".py", delete=False) as f: - f.write( - """ + f.write(""" # Model class User: def __init__(self, name, email): @@ -262,8 +257,7 @@ def __init__(self, model, view): def get_user_display(self): return self.view.display_user(self.model) -""" - ) +""") temp_file = f.name try: