Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions copilotj/core/ui.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"CLI",
"UIEventState",
"UIEventPost",
"UIEventError",
"UIEventPostReasoningChunk",
"UIEventPostContentChunk",
"UIEventToolCall",
Expand Down
10 changes: 10 additions & 0 deletions copilotj/multiagent/Executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

from copilotj.core import ChatAgent, ModelClient, ModelSyntaxError, TextMessage, Tool
from copilotj.multiagent.leader_prompts import build_tool_prompt
from copilotj.plugin.api import PluginNotConnectedError

__all__ = ["Executor"]

Expand Down Expand Up @@ -140,6 +141,12 @@ async def run(self, task: str) -> str:
}
)

except PluginNotConnectedError:
# No ImageJ plugin connected: propagate immediately so the leader's
# tool-exec short-circuit surfaces the curated message instead of
# burning retries here.
raise

except Exception as e:
error_msg = f"❌ Error executing action: {str(e)}"
self.log_error(error_msg)
Expand All @@ -160,6 +167,9 @@ async def run(self, task: str) -> str:

return self._generate_final_summary(conversation_context)

except PluginNotConnectedError:
# Let it propagate to the leader's tool-exec short-circuit (do not stringify).
raise
except Exception as e:
self.log_error(f"Executor error: {e}")
return f"❌ {self.name} encountered an error: {str(e)}"
Expand Down
43 changes: 29 additions & 14 deletions copilotj/multiagent/leader_multiagent.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
)
from copilotj.multiagent.tools import system_info
from copilotj.plugin import ClientPluginAPI
from copilotj.plugin.api import PluginNotConnectedError, PluginRequestError
from copilotj.util import ReActChatCompletionClient

__all__ = ["LeaderDriven"]
Expand Down Expand Up @@ -194,6 +195,21 @@ async def _build_system_prompt(self) -> str:
default_image_path=str(py_tools.get_project_temp_dir()),
)

async def _safe_window_info(self) -> str:
"""Best-effort ImageJ window info for prompt context.

Window info is optional context; any plugin failure (no client connected,
timeout, plugin-side error) is tolerated as an empty string so dialog entry
and prompt optimization degrade gracefully instead of crashing the run.
Returns ``""`` when ``plugin_tools`` is absent (e.g. a stripped-down leader).
"""
if not hasattr(self, "plugin_tools"):
return ""
try:
return await self.plugin_tools.imagej_windowInfo()
except PluginRequestError:
return ""

async def begin_dialog(self, main_task: str, *, trace_ctx: Langfuse | None = None) -> ModelResponse:
"""Start a new dialog, seeding the conversation with system + initial user turn.

Expand All @@ -202,7 +218,7 @@ async def begin_dialog(self, main_task: str, *, trace_ctx: Langfuse | None = Non
here and then retained in ``self._dialog_messages`` so subsequent steps
only append new turns rather than resending the whole prompt.
"""
self.imagej_windowInfo_text = await self.plugin_tools.imagej_windowInfo()
self.imagej_windowInfo_text = await self._safe_window_info()
system_prompt = await self._build_system_prompt()

initial_user = build_initial_user_message(
Expand Down Expand Up @@ -235,7 +251,7 @@ async def continue_dialog(
if assistant_text:
self._dialog_messages.append(TextMessage(role="assistant", text=assistant_text))

self.imagej_windowInfo_text = await self.plugin_tools.imagej_windowInfo()
self.imagej_windowInfo_text = await self._safe_window_info()
user_text = build_observation_message(
tool_response=observation,
imagej_window_info=self.imagej_windowInfo_text,
Expand Down Expand Up @@ -303,7 +319,7 @@ async def delegate_task(self, agent: str, task: str) -> str:

agent_instance = self.agents[agent]
self.log_info(f"[CALL] Calling Agent: {agent} | Params/Task: {task}")
self.imagej_windowInfo_text = await self.plugin_tools.imagej_windowInfo()
self.imagej_windowInfo_text = await self._safe_window_info()

context = f"{task}{self.imagej_windowInfo_text}Previous Chat History: \n{self._prompt_chat_history_json()}"
return await agent_instance.run(context)
Expand All @@ -328,12 +344,7 @@ async def optimize_prompt(self, user_prompt: str) -> str:
"""
# Get ImageJ window info for context (only if available)
# This is read-only, no side effects
window_info = ""
try:
if hasattr(self, "plugin_tools"):
window_info = await self.plugin_tools.imagej_windowInfo()
except Exception:
window_info = ""
window_info = await self._safe_window_info()

# Build context section
context_parts = []
Expand Down Expand Up @@ -511,11 +522,7 @@ async def optimize_prompt(self, user_prompt: str) -> str:
"""

# Get ImageJ window info for context (only if available)
window_info = ""
try:
window_info = await self.leader_agent.plugin_tools.imagej_windowInfo()
except Exception:
window_info = ""
window_info = await self.leader_agent._safe_window_info()

# Build context section
context_parts = []
Expand Down Expand Up @@ -850,6 +857,14 @@ async def run(self, task: str, trace_ctx: Langfuse | None = None) -> None:
resp = await self.leader_agent._call_tool(tool_call)
tool_retry_counter = 0 # Reset retry counter on success

except PluginNotConnectedError as e:
# No ImageJ plugin connected: don't burn retries on a tool that can't
# succeed. Surface the curated message to the user and end the dialog.
await self.leader_agent.print_error(str(e))
dialog_context["status"] = "failed"
dialog_context["steps"].append({"name": tool_call.tool.name, "error": str(e)})
break

except Exception as e:
self.log_error(f"[ERROR] Error executing '{tool_call.tool.name}': {e}")
resp = f"""
Expand Down
74 changes: 68 additions & 6 deletions copilotj/plugin/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# SPDX-License-Identifier: Apache-2.0

import abc
import logging
import uuid
import warnings
from datetime import datetime
Expand Down Expand Up @@ -31,7 +32,47 @@
if TYPE_CHECKING:
from copilotj.server import Bridge

__all__ = ["PluginAPI", "HTTPPluginAPI", "BridgePluginAPI", "ClientPluginAPI"]
_log = logging.getLogger(__name__)

__all__ = [
"PluginAPI",
"HTTPPluginAPI",
"BridgePluginAPI",
"ClientPluginAPI",
"PluginRequestError",
"PluginNotConnectedError",
"CLIENT_NOT_FOUND_ERR",
]

# Bridge error string returned when no ImageJ plugin client is connected. Shared with
# ``copilotj.server.bridge`` so the producer and the consumer agree on the literal.
CLIENT_NOT_FOUND_ERR = "Client not found"


class PluginRequestError(RuntimeError):
"""Any failure of a plugin request (no client connected, timeout, plugin-side error).

Catch this base class where plugin failures should be tolerated (e.g. optional
ImageJ window-info lookups). Catch :class:`PluginNotConnectedError` where the
"plugin not connected" case specifically should short-circuit.
"""


class PluginNotConnectedError(PluginRequestError):
"""No ImageJ plugin client is connected to the bridge.

Carries a curated, user-facing message (``DEFAULT_MESSAGE``) so the agent/UI layer
can surface it directly without leaking internal detail.
"""

DEFAULT_MESSAGE = (
"ImageJ is not connected. Please start Fiji with the CopilotJ plugin "
"installed and wait for it to connect to the server, then resend your request."
)

def __init__(self, message: str | None = None) -> None:
super().__init__(message or self.DEFAULT_MESSAGE)


##########################################################

Expand Down Expand Up @@ -185,11 +226,29 @@ async def _request[R: Response](self, client_id: uuid.UUID, data: Request[R], *,
"/api/plugins/events", data=bytes(request, "utf-8"), headers={"Content-Type": "application/json"}
) as resp:
respStr = await resp.text()
# The bridge returns HTTP 500 + a BridgeResponse body (err=...) for plugin-side
# failures such as "Client not found". Parse the body even on >=400 so those map
# to the typed plugin errors instead of a generic Exception (the status check
# used to fire first, hiding the not-connected mapping).
result: BridgeResponse | None
try:
result = BridgeResponse.model_validate_json(respStr)
except Exception:
result = None
err = result.err if result is not None else None
if err == CLIENT_NOT_FOUND_ERR:
raise PluginNotConnectedError()
if resp.status >= 400:
raise Exception(f"Plugin API request failed with {resp.status}: {respStr}")
result = BridgeResponse.model_validate_json(respStr)
if result.err:
raise Exception(result.err)
# Body may be plugin/proxy-controlled (echoed paths, script text) — log it
# server-side only, keep the exception message free of raw response text so
# it can't flow into the LLM retry observation or the UI.
_log.warning("Plugin API request failed: status=%s body=%s", resp.status, respStr)
raise PluginRequestError(f"Plugin API request failed with HTTP {resp.status}")
if err:
_log.warning("Plugin request returned error: %s", err)
raise PluginRequestError("Plugin request failed")
if result is None:
raise PluginRequestError("Plugin API returned a malformed response")
return cast(R, data.response_type.model_validate(result.data))

@override
Expand All @@ -212,7 +271,10 @@ async def _request[R: Response](self, client_id: uuid.UUID, data: Request[R], *,
BridgeRequest(client_id=client_id, event=data.event, data=data, timeout=timeout)
)
if response.err:
raise RuntimeError(f"Plugin request failed: {response.err}")
if response.err == CLIENT_NOT_FOUND_ERR:
raise PluginNotConnectedError()
_log.warning("Plugin request failed: %s", response.err)
raise PluginRequestError("Plugin request failed")
adapter = TypeAdapter(data.response_type)
return cast(R, adapter.validate_python(response.data))

Expand Down
10 changes: 7 additions & 3 deletions copilotj/server/bridge.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import pydantic

from copilotj.core.config import SINGLE_CLIENT_ID, is_single_client
from copilotj.plugin.api import CLIENT_NOT_FOUND_ERR, PluginRequestError
from copilotj.util import Base64ImageTruncator, IndentedRawJson

__all__ = ["BridgeRequest", "BridgeResponse", "Bridge"]
Expand Down Expand Up @@ -141,7 +142,10 @@ async def send_event(self, event: str, data: Any, *, timeout: float = DEFAULT_TI
return await asyncio.wait_for(future, timeout=timeout)
except asyncio.TimeoutError:
_log.warning("Timeout waiting for response to event: %s (id: %s)", event, event_id)
raise Exception(f"Timeout waiting for response to event: {event} (id: {event_id})")
# Normalize into the plugin-error hierarchy so callers that tolerate
# PluginRequestError (e.g. optional window-info lookups) also tolerate
# timeouts, instead of getting a bare Exception.
raise PluginRequestError(f"Timeout waiting for response to event: {event} (id: {event_id})") from None

finally:
# Clean up the registered event
Expand Down Expand Up @@ -256,8 +260,8 @@ async def send_event(self, req: BridgeRequest) -> BridgeResponse:

client = self._clients.get(client_id)
if client is None:
_log.error(f"Client not found: {req.client_id}")
return BridgeResponse(err="Client not found")
_log.error(f"{CLIENT_NOT_FOUND_ERR}: {req.client_id}")
return BridgeResponse(err=CLIENT_NOT_FOUND_ERR)

# Forward the event
timeout = req.timeout or DEFAULT_TIMEOUT
Expand Down
16 changes: 14 additions & 2 deletions copilotj/server/threads.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
import pydantic
from langfuse import propagate_attributes

from copilotj.core import UI, UIEvent, UIEventPost, UIEventState
from copilotj.core import UI, UIEvent, UIEventError, UIEventPost, UIEventState
from copilotj.core.config import Config, resolve_vision_config
from copilotj.core.ui import UIEventContentMarkdown
from copilotj.multiagent.leader_multiagent import LeaderDriven
from copilotj.plugin.api import BridgePluginAPI, PluginAPI
from copilotj.plugin.api import BridgePluginAPI, PluginAPI, PluginNotConnectedError

if TYPE_CHECKING:
from copilotj.server.bridge import Bridge
Expand All @@ -31,6 +31,11 @@
# Timeout for thread locks in seconds (300ms)
THREAD_LOCK_TIMEOUT = 0.3

# Generic, leakage-safe message shown to the user when the agent crashes on an
# unexpected error. Exception text is kept in server logs only (it may contain
# provider payloads, file paths, or script content).
GENERIC_AGENT_ERROR_MSG = "The agent stopped unexpectedly. Please check the server logs or try again."

_log = logging.getLogger(__name__)

# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -338,9 +343,16 @@ async def _run_agent(self, prompt: str, done_event: asyncio.Event) -> None:
):
await self._agent.run(prompt, trace_ctx=self._trace_ctx)

except PluginNotConnectedError as e:
# Curated, actionable message — safe to show verbatim.
_log.warning("Plugin not connected for thread %s", self.thread_id)
await self.send(UIEventError(role="system", data=str(e)))
except Exception:
# Unknown crash: log full detail, show only a generic message so provider
# payloads / file paths / script text never leak to the frontend.
_log.exception("Agent run failed for thread %s", self.thread_id)
self._agent.log_error(f"Agent run failed for thread {self.thread_id}:\n{traceback.format_exc()}")
await self.send(UIEventError(role="system", data=GENERIC_AGENT_ERROR_MSG))
finally:
done_event.set() # Signal that the chat is done

Expand Down
65 changes: 65 additions & 0 deletions copilotj/test/multiagent/test_executor_plugin_not_connected.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# SPDX-FileCopyrightText: Copyright contributors to the CopilotJ project.
#
# SPDX-License-Identifier: Apache-2.0

"""Tests for Executor propagation of PluginNotConnectedError.

When a delegated agent's tool fails because no plugin is connected, the Executor
must re-raise PluginNotConnectedError out of ``run()`` (skipping its retry loop)
so the leader's tool-exec short-circuit can surface the curated message. A generic
tool error still goes through the existing retry path and returns a string.
"""

import asyncio
from types import SimpleNamespace

import pytest

from copilotj.multiagent.Executor import Executor
from copilotj.plugin.api import PluginNotConnectedError


def _make_executor(*, call_tool_exc: BaseException) -> Executor:
"""An Executor bypassing __init__, with _create/_call_tool stubbed."""
exe = Executor.__new__(Executor)
exe.name = "test_executor"
exe.tools = []
exe.system_prompt = ""
exe.max_iterations = 15
exe.tool_retry_counter = 0
exe.max_tool_retry = 3

fake_tool_call = SimpleNamespace(tool=SimpleNamespace(name="take_snapshot"), args=SimpleNamespace())
fake_response = SimpleNamespace(content=None, reasoning_content=None, tool_calls=[fake_tool_call])

async def _create(*_a: object, **_k: object) -> SimpleNamespace:
return fake_response

async def _call_tool(_tool_call: object) -> str:
raise call_tool_exc

exe._create = _create # type: ignore[method-assign]
exe._call_tool = _call_tool # type: ignore[method-assign]
exe._build_execution_context = lambda _ctx, _i: "" # type: ignore[method-assign]
exe._is_task_complete = lambda _text: False # type: ignore[method-assign]
exe._suggest_tool_based_on_context = lambda _thought, _task: "" # type: ignore[method-assign]
exe._generate_final_summary = lambda _ctx: "summary" # type: ignore[method-assign]
exe.log_info = lambda *_a, **_k: None # type: ignore[method-assign]
exe.log_error = lambda *_a, **_k: None # type: ignore[method-assign]
return exe


def test_plugin_not_connected_propagates_out_of_run():
exe = _make_executor(call_tool_exc=PluginNotConnectedError())

with pytest.raises(PluginNotConnectedError):
asyncio.run(exe.run("do something"))


def test_generic_tool_error_does_not_propagate():
# A non-plugin error stays on the retry path and returns a string (existing behaviour).
exe = _make_executor(call_tool_exc=ValueError("not a plugin error"))

result = asyncio.run(exe.run("do something"))

assert isinstance(result, str)
Loading