mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-11 03:13:51 +00:00
Make session IDs explicit tool parameters instead of auto-injecting them
Previously, Tool.apply_ex derived a session ID from the MCP session object (or "global" with no context) and injected it into any apply() method that declared a session_id parameter. This is being changed because: * the new MCP SDK v2 no longer provides session identifiers * handling it internally is more robust anyway, since clients did not consistently use sessions Session handling - Remove implicit session ID injection from Tool.apply_ex, along with the supporting _is_session_aware property and SESSION_ID_PARAM_NAME skip logic - SerenaAgent.create_system_prompt() now creates the session itself and reports its id, instead of receiving session_id as an argument Tool signatures - InitialInstructionsTool.apply() and ActivateProjectTool.apply() now declare session_id explicitly and rely on the LLM to pass it, rather than having it injected - Rename SerenaReplTool.apply()'s session parameter to session_id for consistency with the other tools Resolves #2061
This commit is contained in:
1 parent
2a6deef838
commit
7a16b3db5a
8 files changed
+37
-50
No files matched your search
@@ -34,6 +34,10 @@ Status of the `main` branch. Changes prior to the next official version change w
|
||||
the project's root path, so a `<project root>/**` entry matches only paths below the root and therefore
|
||||
trusts no project at all; the template now shows the bare root form alongside the parent-directory
|
||||
glob (#2001)
|
||||
- Session IDs are now created and tracked internally by Serena instead of being derived from the
|
||||
MCP session, since the MCP SDK v2 no longer provides session identifiers and client session usage
|
||||
was inconsistent anyway. Tools that need a session id (e.g. `activate_project`, the REPL tool) now
|
||||
take it as an explicit parameter, obtained from `initial_instructions`
|
||||
|
||||
* CLI:
|
||||
- Fix: `project health-check` reported `Health check passed - All tools working correctly` and
|
||||
|
||||
+8
-6
@@ -1157,15 +1157,18 @@ class SerenaAgent:
|
||||
"""
|
||||
return self._session_registry.get_session(session_id)
|
||||
|
||||
def create_system_prompt(self, session_id: str = "global") -> str:
|
||||
def create_system_prompt(self) -> str:
|
||||
"""
|
||||
Returns the 'Serena Instructions Manual', i.e. Serena's system prompt.
|
||||
The prompt also establishes a new Serena session (see `SerenaSession`), stating its id for use with tools
|
||||
which require it (e.g. the REPL tool).
|
||||
which require it (e.g. the REPL tool and project activation tool).
|
||||
|
||||
:param session_id: the client session ID for the case where this is run from a tool; "global" for the connection time case
|
||||
:return: the prompt
|
||||
"""
|
||||
# establish a Serena session
|
||||
serena_session = self.create_session()
|
||||
session_id = serena_session.session_id
|
||||
|
||||
global_memories = self._create_global_memory_manager().list_global_memories()
|
||||
global_memories_str = dict_string(global_memories.to_dict()) if len(global_memories) > 0 else ""
|
||||
|
||||
@@ -1194,10 +1197,9 @@ class SerenaAgent:
|
||||
elif self._project_activation_error:
|
||||
system_prompt += f"\n\nNo project is active ({self._project_activation_error})."
|
||||
|
||||
# establish a Serena session and state its id, which the LLM must pass to tools which require it
|
||||
serena_session = self.create_session()
|
||||
# inform about the session id
|
||||
system_prompt += "\n\n" + self._format_prompt_tag(
|
||||
f"Your Serena session id is `{serena_session.session_id}`. Pass it as the `session` parameter to tools which require it.",
|
||||
f"Your Serena session id is `{session_id}`. Pass it as the `session_id` parameter to tools which require it.",
|
||||
tag="session",
|
||||
)
|
||||
|
||||
|
||||
@@ -48,11 +48,15 @@ class SessionRegistry:
|
||||
self._idle_ttl_seconds = idle_ttl_seconds
|
||||
self._sessions: OrderedDict[str, SerenaSession] = OrderedDict()
|
||||
|
||||
@staticmethod
|
||||
def _next_session_id() -> str:
|
||||
return secrets.token_hex(4)
|
||||
|
||||
def create_session(self) -> SerenaSession:
|
||||
"""
|
||||
:return: a new session with a random id
|
||||
"""
|
||||
return self.get_session(secrets.token_hex(4))
|
||||
return self.get_session(self._next_session_id())
|
||||
|
||||
def get_session(self, session_id: str) -> SerenaSession:
|
||||
"""
|
||||
|
||||
@@ -41,13 +41,12 @@ class ActivateProjectTool(Tool, ToolMarkerDoesNotRequireActiveProject):
|
||||
Activates a project based on the project name or path.
|
||||
"""
|
||||
|
||||
# noinspection PyIncorrectDocstring
|
||||
# (session_id is injected via apply_ex)
|
||||
def apply(self, project: str, session_id: str) -> str:
|
||||
"""
|
||||
Activates the project with the given name or path.
|
||||
|
||||
:param project: the name of a registered project to activate or a path to a project directory
|
||||
:param session_id: your Serena session id, as provided in Serena's instructions (call `initial_instructions` if you do not have one)
|
||||
"""
|
||||
is_new_activation = self.agent.activate_project_from_path_or_name(project)
|
||||
mark_used(is_new_activation)
|
||||
|
||||
@@ -20,7 +20,7 @@ class SerenaReplTool(Tool, ToolMarkerOptional, ToolMarkerBeta):
|
||||
docs += "\n\nAvailable facades are provided at project activation"
|
||||
return docs
|
||||
|
||||
def apply(self, session: str, code: str) -> str:
|
||||
def apply(self, session_id: str, code: str) -> str:
|
||||
"""
|
||||
Executes the given Python code, which has access to Serena's functionality through the object `s`.
|
||||
The functionality is organised in facades, which are attributes of `s` (e.g. `s.myfacade`).
|
||||
@@ -47,8 +47,8 @@ class SerenaReplTool(Tool, ToolMarkerOptional, ToolMarkerBeta):
|
||||
`s.vars()` lists the persisted items, `s.clear()` removes them. Do not store facades (`s.<facade>`) in
|
||||
variables; access them via `s` at call time. Do not keep large results longer than needed.
|
||||
|
||||
:param session: your Serena session id, as provided in Serena's instructions (call `initial_instructions` if you do not have one)
|
||||
:param session_id: your Serena session id, as provided in Serena's instructions (call `initial_instructions` if you do not have one)
|
||||
:param code: the Python code to execute
|
||||
:return: the representation of the returned value, or the error if execution failed
|
||||
"""
|
||||
return self.agent.get_repl().execute(code, self.agent.get_session(session))
|
||||
return self.agent.get_repl().execute(code, self.agent.get_session(session_id))
|
||||
@@ -5,7 +5,6 @@ import json
|
||||
from abc import ABC
|
||||
from collections.abc import Callable, Iterable
|
||||
from dataclasses import dataclass
|
||||
from functools import cached_property
|
||||
from typing import TYPE_CHECKING, Any, Protocol, TypeVar, cast
|
||||
|
||||
from mcp import Implementation
|
||||
@@ -156,32 +155,12 @@ class Tool(Component):
|
||||
# (which is use by the LLM, so a good description is important)
|
||||
# and to validate the tool call arguments.
|
||||
|
||||
SESSION_ID_PARAM_NAME = "session_id"
|
||||
"""
|
||||
parameter name to use in apply method for the client session ID.
|
||||
This parameter will be ignored by the MCP interface but will be populated with the session ID of the current client session
|
||||
when the tool is called, allowing tools to be session-aware if needed.
|
||||
"""
|
||||
|
||||
_last_tool_call_client_str: str | None = None
|
||||
"""We can only get the client info from within a tool call. Each tool call will update this variable."""
|
||||
|
||||
def __init__(self, agent: "SerenaAgent"):
|
||||
super().__init__(agent)
|
||||
|
||||
@cached_property
|
||||
def _is_session_aware(self) -> bool:
|
||||
"""
|
||||
:return: whether the tool is session-aware, i.e. whether the apply method expects a session_id (str) parameter.
|
||||
"""
|
||||
# check apply method for session_id arg
|
||||
apply_fn = self.get_apply_fn()
|
||||
sig = inspect.signature(apply_fn)
|
||||
for param in sig.parameters.values():
|
||||
if param.name == self.SESSION_ID_PARAM_NAME:
|
||||
return True
|
||||
return False
|
||||
|
||||
@staticmethod
|
||||
def _sanitize_input_param(raw_param: str) -> str:
|
||||
# some clients replace < and > with their escaped html versions, we need to counteract this
|
||||
@@ -270,9 +249,9 @@ class Tool(Component):
|
||||
if apply_fn is None:
|
||||
raise AttributeError(f"apply method not defined in {cls}. Did you forget to implement it?")
|
||||
|
||||
return func_metadata(apply_fn, skip_names=["self", "cls", cls.SESSION_ID_PARAM_NAME], structured_output=structured_output)
|
||||
return func_metadata(apply_fn, skip_names=["self", "cls"], structured_output=structured_output)
|
||||
|
||||
def _log_tool_application(self, frame: Any, session_id: str) -> None:
|
||||
def _log_tool_application(self, frame: Any) -> None:
|
||||
params = {}
|
||||
ignored_params = {"self", "log_call", "catch_exceptions", "args", "apply_fn"}
|
||||
for param, value in frame.f_locals.items():
|
||||
@@ -282,7 +261,7 @@ class Tool(Component):
|
||||
params.update(value)
|
||||
else:
|
||||
params[param] = value
|
||||
log.info(f"{self.get_name_from_cls()}: {dict_string(params)}; session_id: {session_id}")
|
||||
log.info(f"{self.get_name_from_cls()}: {dict_string(params)}")
|
||||
|
||||
def _resolve_max_answer_chars(self, max_answer_chars: int) -> int:
|
||||
"""
|
||||
@@ -337,10 +316,8 @@ class Tool(Component):
|
||||
:param catch_exceptions: whether to catch exceptions and return their messages as strings, instead of raising a ToolCallError
|
||||
"""
|
||||
# obtain session ID and client info
|
||||
session_id = "global"
|
||||
if mcp_ctx is not None:
|
||||
try:
|
||||
session_id = "%x" % id(mcp_ctx.session)
|
||||
client_params = mcp_ctx.session.client_params
|
||||
if client_params is not None:
|
||||
client_info = cast(Implementation, client_params.clientInfo)
|
||||
@@ -361,7 +338,7 @@ class Tool(Component):
|
||||
)
|
||||
|
||||
if log_call:
|
||||
self._log_tool_application(inspect.currentframe(), session_id)
|
||||
self._log_tool_application(inspect.currentframe())
|
||||
|
||||
# check whether the tool requires an active project and language server
|
||||
if not isinstance(self, ToolMarkerDoesNotRequireActiveProject):
|
||||
@@ -373,8 +350,6 @@ class Tool(Component):
|
||||
|
||||
# construct apply kwargs, adding session_id if the tool is session-aware
|
||||
apply_kwargs = dict(kwargs)
|
||||
if self._is_session_aware:
|
||||
apply_kwargs["session_id"] = session_id
|
||||
|
||||
# apply the actual tool
|
||||
try:
|
||||
|
||||
@@ -31,15 +31,13 @@ class InitialInstructionsTool(Tool, ToolMarkerDoesNotRequireActiveProject):
|
||||
for clients that do not read the initial instructions when the MCP server is connected.
|
||||
"""
|
||||
|
||||
# noinspection PyIncorrectDocstring
|
||||
# (session_id is injected via apply_ex)
|
||||
def apply(self, session_id: str) -> str:
|
||||
def apply(self) -> str:
|
||||
"""
|
||||
Provides the 'Serena Instructions Manual', which contains essential information on how to use the Serena toolbox.
|
||||
IMPORTANT: If you have not yet read the manual, call this tool immediately after you are given your task by the user,
|
||||
as it will critically inform you!
|
||||
"""
|
||||
return self.agent.create_system_prompt(session_id=session_id)
|
||||
return self.agent.create_system_prompt()
|
||||
|
||||
|
||||
class SerenaInfoTool(Tool, ToolMarkerOptional, ToolMarkerDoesNotRequireActiveProject):
|
||||
|
||||
@@ -17,6 +17,7 @@ from serena.config.context_mode import SerenaAgentContext
|
||||
from serena.config.serena_config import AgentInterface, ProjectConfig, RegisteredProject, SerenaConfig
|
||||
from serena.lsp.lsp_diagnostics import DiagnosticsContext
|
||||
from serena.project import Project
|
||||
from serena.session import SessionRegistry
|
||||
from serena.tools import (
|
||||
SUCCESS_RESULT,
|
||||
ActivateProjectTool,
|
||||
@@ -1400,13 +1401,17 @@ class TestSerenaAgent:
|
||||
|
||||
|
||||
class TestPromptProvision:
|
||||
class MockContext:
|
||||
def __init__(self, session_id: str):
|
||||
self.session = session_id
|
||||
|
||||
@classmethod
|
||||
def _call_tool(cls, agent: SerenaAgent, tool_class: type[Tool], session_id: str = "global", **kwargs) -> str:
|
||||
result = agent.get_tool(tool_class).apply_ex(mcp_ctx=cls.MockContext(session_id), catch_exceptions=False, **kwargs)
|
||||
old_method = SessionRegistry._next_session_id
|
||||
if tool_class == InitialInstructionsTool:
|
||||
SessionRegistry._next_session_id = lambda x: session_id # type: ignore
|
||||
else:
|
||||
kwargs["session_id"] = session_id
|
||||
try:
|
||||
result = agent.get_tool(tool_class).apply_ex(catch_exceptions=False, **kwargs)
|
||||
finally:
|
||||
SessionRegistry._next_session_id = old_method
|
||||
return result
|
||||
|
||||
@staticmethod
|
||||
|
||||
Reference in new issue
Block a user