fix: intercept non-cli errors and allow agent to continue
This commit is contained in:
@@ -136,6 +136,18 @@ class BaseCLIAgent:
|
|||||||
if output_file_content and not stdout_text.strip():
|
if output_file_content and not stdout_text.strip():
|
||||||
stdout_text = output_file_content
|
stdout_text = output_file_content
|
||||||
|
|
||||||
|
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=output_file_content,
|
||||||
|
)
|
||||||
|
if recovered is not None:
|
||||||
|
return recovered
|
||||||
|
|
||||||
if return_code != 0:
|
if return_code != 0:
|
||||||
raise CLIAgentError(
|
raise CLIAgentError(
|
||||||
f"CLI '{self.client.name}' exited with status {return_code}",
|
f"CLI '{self.client.name}' exited with status {return_code}",
|
||||||
@@ -177,3 +189,25 @@ class BaseCLIAgent:
|
|||||||
env = os.environ.copy()
|
env = os.environ.copy()
|
||||||
env.update(self.client.env)
|
env.update(self.client.env)
|
||||||
return env
|
return env
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# Error recovery hooks
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
|
||||||
|
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:
|
||||||
|
"""Hook for subclasses to convert CLI errors into successful outputs.
|
||||||
|
|
||||||
|
Return an AgentOutput to treat the failure as success, or None to signal
|
||||||
|
that normal error handling should proceed.
|
||||||
|
"""
|
||||||
|
|
||||||
|
return None
|
||||||
|
|||||||
@@ -2,13 +2,85 @@
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
from clink.models import ResolvedCLIClient
|
import json
|
||||||
|
from typing import Any
|
||||||
|
|
||||||
from .base import BaseCLIAgent
|
from clink.models import ResolvedCLIClient
|
||||||
|
from clink.parsers.base import ParsedCLIResponse
|
||||||
|
|
||||||
|
from .base import AgentOutput, BaseCLIAgent
|
||||||
|
|
||||||
|
|
||||||
class GeminiAgent(BaseCLIAgent):
|
class GeminiAgent(BaseCLIAgent):
|
||||||
"""Placeholder for Gemini-specific behaviour."""
|
"""Gemini-specific behaviour."""
|
||||||
|
|
||||||
def __init__(self, client: ResolvedCLIClient):
|
def __init__(self, client: ResolvedCLIClient):
|
||||||
super().__init__(client)
|
super().__init__(client)
|
||||||
|
|
||||||
|
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:
|
||||||
|
combined = "\n".join(part for part in (stderr, stdout) if part)
|
||||||
|
if not combined:
|
||||||
|
return None
|
||||||
|
|
||||||
|
brace_index = combined.find("{")
|
||||||
|
if brace_index == -1:
|
||||||
|
return None
|
||||||
|
|
||||||
|
json_candidate = combined[brace_index:]
|
||||||
|
try:
|
||||||
|
payload: dict[str, Any] = json.loads(json_candidate)
|
||||||
|
except json.JSONDecodeError:
|
||||||
|
return None
|
||||||
|
|
||||||
|
error_block = payload.get("error")
|
||||||
|
if not isinstance(error_block, dict):
|
||||||
|
return None
|
||||||
|
|
||||||
|
code = error_block.get("code")
|
||||||
|
err_type = error_block.get("type")
|
||||||
|
detail_message = error_block.get("message")
|
||||||
|
|
||||||
|
prologue = combined[:brace_index].strip()
|
||||||
|
lines: list[str] = []
|
||||||
|
if prologue and (not detail_message or prologue not in detail_message):
|
||||||
|
lines.append(prologue)
|
||||||
|
if detail_message:
|
||||||
|
lines.append(detail_message)
|
||||||
|
|
||||||
|
header = "Gemini CLI reported a tool failure"
|
||||||
|
if code:
|
||||||
|
header = f"{header} ({code})"
|
||||||
|
elif err_type:
|
||||||
|
header = f"{header} ({err_type})"
|
||||||
|
|
||||||
|
content_lines = [header.rstrip(".") + "."]
|
||||||
|
content_lines.extend(lines)
|
||||||
|
message = "\n".join(content_lines).strip()
|
||||||
|
|
||||||
|
metadata = {
|
||||||
|
"cli_error_recovered": True,
|
||||||
|
"cli_error_code": code,
|
||||||
|
"cli_error_type": err_type,
|
||||||
|
"cli_error_payload": payload,
|
||||||
|
}
|
||||||
|
|
||||||
|
parsed = ParsedCLIResponse(content=message or header, metadata=metadata)
|
||||||
|
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,
|
||||||
|
)
|
||||||
|
|||||||
76
tests/test_clink_gemini_agent.py
Normal file
76
tests/test_clink_gemini_agent.py
Normal file
@@ -0,0 +1,76 @@
|
|||||||
|
import asyncio
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from clink.agents.base import CLIAgentError
|
||||||
|
from clink.agents.gemini import GeminiAgent
|
||||||
|
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, _input):
|
||||||
|
return self._stdout, self._stderr
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture()
|
||||||
|
def gemini_agent():
|
||||||
|
prompt_path = Path("systemprompts/clink/gemini_default.txt").resolve()
|
||||||
|
role = ResolvedCLIRole(name="default", prompt_path=prompt_path, role_args=[])
|
||||||
|
client = ResolvedCLIClient(
|
||||||
|
name="gemini",
|
||||||
|
executable=["gemini"],
|
||||||
|
internal_args=[],
|
||||||
|
config_args=[],
|
||||||
|
env={},
|
||||||
|
timeout_seconds=30,
|
||||||
|
parser="gemini_json",
|
||||||
|
roles={"default": role},
|
||||||
|
output_to_file=None,
|
||||||
|
working_dir=None,
|
||||||
|
)
|
||||||
|
return GeminiAgent(client), role
|
||||||
|
|
||||||
|
|
||||||
|
async def _run_agent_with_process(monkeypatch, agent, role, process):
|
||||||
|
async def fake_create_subprocess_exec(*_args, **_kwargs):
|
||||||
|
return process
|
||||||
|
|
||||||
|
monkeypatch.setattr(asyncio, "create_subprocess_exec", fake_create_subprocess_exec)
|
||||||
|
return await agent.run(role=role, prompt="do something", files=[], images=[])
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_gemini_agent_recovers_tool_error(monkeypatch, gemini_agent):
|
||||||
|
agent, role = gemini_agent
|
||||||
|
error_json = """{
|
||||||
|
"error": {
|
||||||
|
"type": "FatalToolExecutionError",
|
||||||
|
"message": "Error executing tool replace: Failed to edit",
|
||||||
|
"code": "edit_expected_occurrence_mismatch"
|
||||||
|
}
|
||||||
|
}"""
|
||||||
|
stderr = ("Error: Failed to edit, expected 1 occurrence but found 2.\n" + error_json).encode()
|
||||||
|
process = DummyProcess(stderr=stderr, returncode=54)
|
||||||
|
|
||||||
|
result = await _run_agent_with_process(monkeypatch, agent, role, process)
|
||||||
|
|
||||||
|
assert result.returncode == 54
|
||||||
|
assert result.parsed.metadata["cli_error_recovered"] is True
|
||||||
|
assert result.parsed.metadata["cli_error_code"] == "edit_expected_occurrence_mismatch"
|
||||||
|
assert "Gemini CLI reported a tool failure" in result.parsed.content
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_gemini_agent_propagates_unrecoverable_error(monkeypatch, gemini_agent):
|
||||||
|
agent, role = gemini_agent
|
||||||
|
stderr = b"Plain failure without structured payload"
|
||||||
|
process = DummyProcess(stderr=stderr, returncode=54)
|
||||||
|
|
||||||
|
with pytest.raises(CLIAgentError):
|
||||||
|
await _run_agent_with_process(monkeypatch, agent, role, process)
|
||||||
@@ -258,11 +258,6 @@ class CLinkTool(SimpleTool):
|
|||||||
finally:
|
finally:
|
||||||
self._active_system_prompt = ""
|
self._active_system_prompt = ""
|
||||||
|
|
||||||
def _merge_metadata(self, base: dict[str, Any] | None, extra: dict[str, Any]) -> dict[str, Any]:
|
|
||||||
merged = dict(base or {})
|
|
||||||
merged.update(extra)
|
|
||||||
return merged
|
|
||||||
|
|
||||||
def _build_success_metadata(
|
def _build_success_metadata(
|
||||||
self,
|
self,
|
||||||
client: ResolvedCLIClient,
|
client: ResolvedCLIClient,
|
||||||
@@ -286,6 +281,11 @@ class CLinkTool(SimpleTool):
|
|||||||
metadata["raw_output_file"] = result.output_file_content
|
metadata["raw_output_file"] = result.output_file_content
|
||||||
return metadata
|
return metadata
|
||||||
|
|
||||||
|
def _merge_metadata(self, base: dict[str, Any] | None, extra: dict[str, Any]) -> dict[str, Any]:
|
||||||
|
merged = dict(base or {})
|
||||||
|
merged.update(extra)
|
||||||
|
return merged
|
||||||
|
|
||||||
def _build_error_metadata(self, client: ResolvedCLIClient, exc: CLIAgentError) -> dict[str, Any]:
|
def _build_error_metadata(self, client: ResolvedCLIClient, exc: CLIAgentError) -> dict[str, Any]:
|
||||||
"""Assemble metadata for failed CLI calls."""
|
"""Assemble metadata for failed CLI calls."""
|
||||||
metadata: dict[str, Any] = {
|
metadata: dict[str, Any] = {
|
||||||
|
|||||||
Reference in New Issue
Block a user