From 365e1a62f94039d98b1ad39db1f05636b753b96a Mon Sep 17 00:00:00 2001 From: Naseem Alnaji Date: Thu, 27 Aug 2026 18:10:19 +0000 Subject: [PATCH 1/4] =?UTF-8?q?feat:=20sturdier=20session=20handles=20?= =?UTF-8?q?=E2=80=94=20issue=20IDs=20first,=20structured=20mcp=5Fsession?= =?UTF-8?q?=20status?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Session IDs issued at the tail of long tool results can be lost to client-side truncation, so issuance now moves to the top of the response: the mint-back text block leads as the first content element, and the mcp_session mirror is inserted as the first key of structuredContent. The agent-facing copy is rewritten leaner and self-describing, with every response state pre-announced in the schema: - The structured mirror key is renamed _mcp_instructions -> mcp_session, and its prose instructions member is replaced by a machine-readable status enum (issued | active | unrecognized), present in prompted mode only; hook mode and customer-owned session parameters mirror agent_id alone. - New pre-announced text-block headers ([session_id issued ...] / [session_id unrecognized ...]) with a bare "session_id: " line. - One agent_id description now serves both modes; hook mode gains its own mcp_session field description. - The session_id and agent_id parameter descriptions explain the handles in their own terms and name every place an ID can come back. - session_id and agent_id are now required, with an explicit start value and a validation pattern on session_id — every call states its session explicitly and sessions never split by accident. Servers still accept calls that omit the parameters (treated as the start of a new task), so requiredness is enforced only by schema-aware clients. Breaking wire changes, stated for reviewers: the structuredContent mirror key is renamed, the instructions member is removed, and the text block moved from last to first content element. Version 2.1.0. --- pyproject.toml | 2 +- src/agentcat/modules/callpath.py | 27 +-- src/agentcat/modules/constants.py | 44 ++--- src/agentcat/modules/handles.py | 82 ++++----- src/agentcat/modules/injection.py | 100 ++++++----- src/agentcat/types.py | 4 +- src/agentcat/utils.py | 5 +- tests/community/test_community_v3_handles.py | 45 ++--- .../test_community_v3_nested_calls.py | 10 +- tests/community/test_community_v3_openapi.py | 5 +- .../community_v3/test_agent_handle_http.py | 33 ++-- .../community_v3/test_event_capture_http.py | 7 +- tests/e2e/community_v3/test_stateless_http.py | 2 +- .../community_v4/test_agent_handle_http.py | 35 ++-- tests/e2e/community_v4/test_session_http.py | 19 +- tests/e2e/community_v4/test_stateless_http.py | 2 +- tests/e2e/official/test_agent_handle_http.py | 27 ++- tests/e2e/official/test_session_http.py | 26 ++- tests/e2e/official/test_stateless_http.py | 2 +- .../official_modern/test_agent_handle_http.py | 33 ++-- .../official_modern/test_mcpserver_http.py | 28 +-- .../e2e/official_modern/test_session_http.py | 26 ++- .../official_modern/test_stateless_http.py | 2 +- tests/test_callpath.py | 37 ++-- tests/test_community_v4_handles.py | 25 ++- tests/test_community_v4_nested_calls.py | 8 +- tests/test_concurrency_handles.py | 14 +- tests/test_constants_copy.py | 152 ++++++++-------- tests/test_customer_owned_parameters.py | 10 +- tests/test_dynamic_tracking.py | 9 +- tests/test_handles.py | 101 ++++++++--- tests/test_injection.py | 170 ++++++++++++------ tests/test_inner_tap.py | 22 +-- tests/test_lowlevel_v1_handles.py | 53 +++--- tests/test_lowlevel_v2_handles.py | 51 +++--- tests/test_rebuild_on_demand.py | 10 +- tests/test_removed_events.py | 4 +- tests/test_report_missing.py | 33 ++-- tests/test_response_shape.py | 6 +- tests/test_session_id_validation.py | 99 ++++++---- tests/test_tool_context.py | 23 +-- 41 files changed, 832 insertions(+), 561 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 4f5f40d..024038e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "agentcat" -version = "2.0.2" +version = "2.1.0" description = "Analytics tool for MCP (Model Context Protocol) servers, Claude Connectors, and ChatGPT Plugins - tracks tool usage patterns and provides insights" authors = [ { name = "AgentCat, Inc.", email = "support@agentcat.com" }, diff --git a/src/agentcat/modules/callpath.py b/src/agentcat/modules/callpath.py index 93ce149..6d24eb8 100644 --- a/src/agentcat/modules/callpath.py +++ b/src/agentcat/modules/callpath.py @@ -111,7 +111,8 @@ async def resolve_call( A tool absent from `declared_session_params` is ours — including one this instance never listed. That is the common case on stateless HTTP and it degrades safely: a customer's foreign value in that window is classified - `invalid` rather than `foreign`, and both publish sessionless. + `invalid` rather than `foreign`, and both publish sessionless (a value + spelling `start` instead mints, exactly as on a parameter we injected). """ resolution = await resolve_handles( raw_arguments, @@ -157,9 +158,10 @@ async def get_stripped_arguments( rebuild falls back to the shape+config-aware strip (see `injected_parameter_names`): a name is removed only when the enabled options would have injected it AND, for `session_id`, the value matches - our minted shape — a customer-declared parameter rides through to their - handler. The fallback also clears the output-injection registry, so the - structured mirror stops gating on knowledge we no longer have (§3.4b). + our minted shape or the `start` sentinel — a customer-declared parameter + rides through to their handler. The fallback also clears the + output-injection registry, so the structured mirror stops gating on + knowledge we no longer have (§3.4b). """ registry = data.injected_params_registry if registry is None and rebuild is not None: @@ -224,19 +226,20 @@ def decorate_content( res: HandleResolution, make_text_block: Callable[[str], Any], ) -> list[Any] | None: - """The trailing mint-back block, or None to leave the result untouched. + """The leading mint-back block, or None to leave the result untouched. - Error state is deliberately not an input: the retry after a failure has to - carry the same session, so `isError` results decorate on identical terms - (§3.4a). Whether there is anything to say at all stays the single ruling of - `build_mint_back_text` — a session minted on this call, or a supplied one - this server never issued; never in hook mode, and never for a parameter - AgentCat did not inject. + Prepended, not appended: an ID at the tail of a long result is what + clients truncate away. Error state is deliberately not an input: the retry + after a failure has to carry the same session, so `isError` results + decorate on identical terms (§3.4a). Whether there is anything to say at + all stays the single ruling of `build_mint_back_text` — a session minted + on this call, or a supplied one this server never issued; never in hook + mode, and never for a parameter AgentCat did not inject. """ text = build_mint_back_text(res) if text is None or not isinstance(content, list): return None - return [*content, make_text_block(text)] + return [make_text_block(text), *content] def structured_mirror( diff --git a/src/agentcat/modules/constants.py b/src/agentcat/modules/constants.py index 2b433a8..f89dc70 100644 --- a/src/agentcat/modules/constants.py +++ b/src/agentcat/modules/constants.py @@ -26,7 +26,7 @@ CONTEXT_PARAM = "context" GET_MORE_TOOLS_NAME = "get_more_tools" AGENT_ID_PREFIX = "agt" # reserved; server-side agent minting was removed -MCP_INSTRUCTIONS_KEY = "_mcp_instructions" +MCP_SESSION_KEY = "mcp_session" META_CLIENT_INFO_KEY = "io.modelcontextprotocol/clientInfo" META_PROTOCOL_VERSION_KEY = "io.modelcontextprotocol/protocolVersion" AGENTCAT_TAG_SESSION_SOURCE = "agentcat_session_id_source" @@ -44,27 +44,29 @@ # Source of truth: agentcat-typescript-sdk/src/modules/constants.ts (and the # copy spec it cites). Do not reword here without changing the TS SDK first; # tests/test_constants_copy.py enforces byte parity. -SESSION_ID_PARAM_DESCRIPTION = "REQUIRED on every call after your first. This MCP server associates all of your tool calls for a given task using session_id; a call that arrives without it cannot be associated with your earlier work and is treated as the start of an unrelated task, disconnected from the original goal. Omit it on your first call only — the server will issue one in the _mcp_instructions field of the result (or a trailing [MCP INSTRUCTIONS] text block) — then echo that exact value on every later call. Never invent a value, and do not issue parallel tool calls until the server has issued your session_id. One session_id covers the whole goal from start to finish: when you spawn subagents you MUST give them this same session_id, or their work is severed from yours. Without session_id, this server does not function as intended." -AGENT_ID_PARAM_DESCRIPTION = "REQUIRED on every call, including your first. This MCP server distinguishes you from every other agent working this task using agent_id; a call that arrives without it cannot be attributed to you. Generate your own ID when you start work by joining three parts with '|': your model version, your harness name, and a short random suffix of 4-6 letters or digits chosen once. Example: opus-4.80-1m|claude-code|k3n9x. Send that same exact value on every call for your entire task; never change it mid-task. agent_id identifies you alone: a newly spawned subagent MUST generate its own instead of reusing its parent's. Without agent_id, this server does not function as intended." -AGENT_ID_PARAM_DESCRIPTION_HOOK_MODE = "REQUIRED on every call, including your first. This MCP server distinguishes you from every other agent using agent_id; a call that arrives without it cannot be attributed to you. Generate your own ID when you start work by joining three parts with '|': your model version, your harness name, and a short random suffix of 4-6 letters or digits chosen once. Example: opus-4.80-1m|claude-code|k3n9x. Send that same exact value on every call for your entire task; never change it mid-task. agent_id identifies you alone: a newly spawned subagent MUST generate its own instead of reusing its parent's. Without agent_id, this server does not function as intended." -MINT_BACK_HEADER_SESSION = "[MCP INSTRUCTIONS]: session_id issued." -MINT_BACK_HEADER_INVALID = "[MCP INSTRUCTIONS]: session_id not recognized." -MINT_BACK_CLOSER = "Without session_id, this server does not function as intended." -MINT_BACK_INVALID_LINE = " The session_id you sent was not issued by this server. Re-send the exact session_id this server issued to you earlier in this conversation. Never invent a value. If this server has not issued you a session_id yet, omit the parameter and one will be issued." -MCP_INSTRUCTIONS_FIELD_DESCRIPTION = "Your handles for this task, confirmed by this MCP server on every response, and the instructions for echoing them on later calls. Read and follow." -MCP_INSTRUCTIONS_SESSION_ID_DESCRIPTION = ( - "Echo this exact value as the session_id argument on every subsequent tool call." +SESSION_ID_PARAM_DESCRIPTION = "Session continuity handle, one of two values: the ses_ ID issued for the task underway, or start. This server cannot link your calls between requests on its own, so session continuity travels in this parameter instead. If you were handed a session_id for this task — for example by the agent that spawned you — send that exact value from your first call. Otherwise send start on your first call; the server will issue an opaque correlation ID in the mcp_session field of the result, or in a text block at the start of the result beginning [session_id issued. Then send that exact ses_ value on every later call and hand it to any subagents working the same task. start always begins a new, unrelated task — never send it mid-task. If you send a value this server does not recognize, the result reports it: mcp_session.status of unrecognized, or a text block beginning [session_id unrecognized; re-send the ID issued for this task, or start if none was issued yet. Never invent a ses_ value." +# Wire literals of the session_id value contract: the pattern the injected +# parameter's schema declares, and the sentinel value that begins a new task. +# The pattern is is_valid_session_id's shape with the start alternative added; +# resolution accepts the sentinel case-insensitively and still treats an +# absent value as a start (stale schemas and scripted callers never error). +SESSION_ID_PARAM_PATTERN = "^(start|ses_[0-9A-Za-z]{27})$" +SESSION_START_SENTINEL = "start" +AGENT_ID_PARAM_DESCRIPTION = "Agent identity handle, required on every call including your first. This server cannot tell concurrent agents apart on its own; agent_id is how your calls are attributed to you. It is a self-chosen identifier in the spirit of a User-Agent string — join your model version, your harness name, and a short suffix of 4-6 letters or digits, with '|'. Example: opus-4.80-1m|claude-code|k3n9x. Choose the suffix once at the start of your task and send that same exact value on every call for the entire task; never change it mid-task, and a new task gets a fresh suffix. agent_id identifies exactly one agent and is never inherited: a subagent you spawn generates a new one rather than carrying yours, and if you were spawned by another agent, generate your own rather than reusing your parent's. A call without agent_id cannot be attributed to you." +MINT_BACK_HEADER_ISSUED = ( + "[session_id issued — see this tool's session_id parameter description]" ) -MCP_INSTRUCTIONS_AGENT_ID_DESCRIPTION = "Your agent_id as this server received it. Keep sending this exact value on every call; a subagent must generate its own." +MINT_BACK_ISSUED_BODY = "This is the first-call issuance described in this tool's session_id parameter description." +MINT_BACK_HEADER_UNRECOGNIZED = ( + "[session_id unrecognized — see this tool's session_id parameter description]" +) +MINT_BACK_UNRECOGNIZED_BODY = "The value sent was not issued by this server. Re-send the session_id issued earlier for this task; if none was issued yet, send start and one will be issued." +MCP_SESSION_FIELD_DESCRIPTION = "Session continuity and agent attribution state for this task, returned on completed responses that carry structured output. This server cannot link your calls between requests on its own, so session continuity travels here instead." +MCP_SESSION_FIELD_DESCRIPTION_HOOK_MODE = "Agent attribution state for this task, returned on completed responses that carry structured output." +MCP_SESSION_SESSION_ID_DESCRIPTION = "Opaque correlation ID for this task, issued by this server. Use this as the session_id argument of every later call, and hand it to any subagents working the same task. Absent when status is unrecognized; no replacement is issued in that response — recovery is described under status." +MCP_SESSION_AGENT_ID_DESCRIPTION = "Present only when you sent agent_id on this call. Your agent_id, echoed as received. Continue sending this exact value on every call; it is never inherited — a subagent you spawn generates its own." +MCP_SESSION_STATUS_DESCRIPTION = "issued: first call of a task; the session_id above was just created. active: the session_id you sent was accepted; keep sending it. unrecognized: the value sent was not issued by this server — re-send the one issued earlier for this task; if none was issued yet, send start to be issued a new one." def mint_back_session_line(session_id: str) -> str: - return f" session_id={session_id} — required on every subsequent tool call" - - -def mint_back_confirmed(names: list[str]) -> str: - tail = "these exact values" if len(names) > 1 else "this exact value" - return ( - f"[MCP INSTRUCTIONS]: {' and '.join(names)} confirmed. " - f"Keep sending {tail} on every call." - ) + return f"session_id: {session_id}" diff --git a/src/agentcat/modules/handles.py b/src/agentcat/modules/handles.py index e630bc8..5733f8f 100644 --- a/src/agentcat/modules/handles.py +++ b/src/agentcat/modules/handles.py @@ -18,14 +18,14 @@ AGENTCAT_TAG_NESTED, AGENTCAT_TAG_PROTOCOL_VERSION, AGENTCAT_TAG_SESSION_SOURCE, - MCP_INSTRUCTIONS_KEY, - MINT_BACK_CLOSER, - MINT_BACK_HEADER_INVALID, - MINT_BACK_HEADER_SESSION, - MINT_BACK_INVALID_LINE, + MCP_SESSION_KEY, + MINT_BACK_HEADER_ISSUED, + MINT_BACK_HEADER_UNRECOGNIZED, + MINT_BACK_ISSUED_BODY, + MINT_BACK_UNRECOGNIZED_BODY, SESSION_ID_PARAM, SESSION_ID_PREFIX, - mint_back_confirmed, + SESSION_START_SENTINEL, mint_back_session_line, ) from agentcat.modules.hooks import run_hook @@ -184,7 +184,13 @@ async def resolve_handles( ) supplied = extract_handle(arguments, SESSION_ID_PARAM) - if supplied: + # The `start` sentinel is checked BEFORE shape validation and resolves + # exactly like an absent value: it is the explicit spelling of "begin a + # new task" the required parameter's copy asks for. Case-insensitive + # (extract_handle already trimmed), and only ever read here — a foreign + # customer-owned value took the branch above, so `start` in a parameter + # the customer declared is never interpreted as a sentinel. + if supplied and supplied.lower() != SESSION_START_SENTINEL: if is_valid_session_id(supplied): return HandleResolution( supplied, @@ -210,39 +216,24 @@ async def resolve_handles( ) -def _echoes_session_id(res: HandleResolution) -> bool: - """Whether the agent has an AgentCat `session_id` value to echo back. - - Three ways to have none: hook mode and the no-parameter cases collapsed - into `prompts_session_id`, plus `invalid` — where the parameter is ours - but there is no value to confirm. That branch corrects the agent rather - than issuing a replacement, so naming a `session_id` would be a lie. - """ - return ( - res.prompts_session_id and not res.hook_mode and res.session_source != "invalid" - ) - - def build_mint_back_text(res: HandleResolution) -> str | None: if res.hook_mode or not res.prompts_session_id: return None if res.session_source == "minted": return "\n".join( [ - MINT_BACK_HEADER_SESSION, + MINT_BACK_HEADER_ISSUED, mint_back_session_line(res.session_id), - MINT_BACK_CLOSER, + MINT_BACK_ISSUED_BODY, ] ) if res.session_source == "invalid": # No replacement is handed out. An agent that sent something was # usually already issued a good ID, and giving it a second one splits a # session that was never split. The closing sentence of - # MINT_BACK_INVALID_LINE is the way out for the agent that was never - # issued one: omit the parameter and take the `minted` branch. - return "\n".join( - [MINT_BACK_HEADER_INVALID, MINT_BACK_INVALID_LINE, MINT_BACK_CLOSER] - ) + # MINT_BACK_UNRECOGNIZED_BODY is the way out for the agent that was + # never issued one: send `start` and take the `minted` branch. + return "\n".join([MINT_BACK_HEADER_UNRECOGNIZED, MINT_BACK_UNRECOGNIZED_BODY]) return None @@ -251,7 +242,9 @@ def build_structured_mint_back(res: HandleResolution) -> dict[str, Any] | None: Unlike `build_mint_back_text` (mint announcements only), this is present on EVERY response, so an agent can re-read its own handles mid-session. - Handles the agent cannot echo are never named. + Handles the agent cannot echo are never named: `status` and `session_id` + appear only in prompted mode — hook mode and a customer-owned session + parameter mirror `agent_id` alone. Suppression is per-HANDLE, not per-response: a `session_id` collision skips only `session_id`. `agent_id` is a separate injection and still landed in @@ -259,33 +252,34 @@ def build_structured_mint_back(res: HandleResolution) -> dict[str, Any] | None: mirror would withhold a handle AgentCat issued purely because a neighbouring one belongs to the customer. """ - echoes = _echoes_session_id(res) - names: list[str] = [] - if echoes: - names.append(SESSION_ID_PARAM) - if res.agent_id: - names.append(AGENT_ID_PARAM) - text = build_mint_back_text(res) - # `not names` alone would drop the `invalid` correction whenever no - # agent_id is in play — the one branch that has something to say and - # nothing to echo. - if not names and not text: - return None + prompted = res.prompts_session_id and not res.hook_mode mint: dict[str, Any] = {} - if echoes: + if prompted and res.session_source in ("minted", "supplied"): + # `invalid` names no session_id: no replacement is issued, and the + # recovery path is described under `status`. mint[SESSION_ID_PARAM] = res.session_id if res.agent_id: mint[AGENT_ID_PARAM] = res.agent_id - mint["instructions"] = text or mint_back_confirmed(names) + if prompted: + if res.session_source == "minted": + mint["status"] = "issued" + elif res.session_source == "supplied": + mint["status"] = "active" + elif res.session_source == "invalid": + mint["status"] = "unrecognized" + if not mint: + return None return mint def mirror_into_structured_content( sc: Any, mint: dict[str, Any] ) -> dict[str, Any] | None: - if not isinstance(sc, dict) or MCP_INSTRUCTIONS_KEY in sc: + if not isinstance(sc, dict) or MCP_SESSION_KEY in sc: return None - return {**sc, MCP_INSTRUCTIONS_KEY: mint} + # Mirror first: an ID at the tail of a long payload is what clients + # truncate away. + return {MCP_SESSION_KEY: mint, **sc} def _clamp_tag_value(value: str) -> str: diff --git a/src/agentcat/modules/injection.py b/src/agentcat/modules/injection.py index 5e62095..62e44ae 100644 --- a/src/agentcat/modules/injection.py +++ b/src/agentcat/modules/injection.py @@ -9,7 +9,7 @@ Two passes over the tools, in order: -1. Handle pass — session_id/agent_id input params plus the _mcp_instructions +1. Handle pass — session_id/agent_id input params plus the mcp_session outputSchema extension. Runs only when at least one handle is injectable (prompted-mode tracing -> session_id, agent tracking -> agent_id); when neither is, the pass is skipped wholesale and schemas keep their original @@ -20,10 +20,11 @@ events. Resulting property order: customer params, session_id, agent_id, context. -session_id is never required — omission is the minting signal. agent_id and -context are both appended to required; that is client-side compliance -only, since callWrap tolerates either being absent, and it is the only -enforcement an injected parameter has. +Every injected param is appended to required, and session_id also declares +the start|ses_ value pattern; both are client-side compliance only, since +callWrap tolerates an absent or malformed value (treating it as the start +of a new task), and schema-aware clients are the only enforcement an +injected parameter has. """ from collections.abc import Iterable @@ -33,15 +34,18 @@ from agentcat.modules.constants import ( AGENT_ID_PARAM, AGENT_ID_PARAM_DESCRIPTION, - AGENT_ID_PARAM_DESCRIPTION_HOOK_MODE, CONTEXT_PARAM, GET_MORE_TOOLS_NAME, - MCP_INSTRUCTIONS_AGENT_ID_DESCRIPTION, - MCP_INSTRUCTIONS_FIELD_DESCRIPTION, - MCP_INSTRUCTIONS_KEY, - MCP_INSTRUCTIONS_SESSION_ID_DESCRIPTION, + MCP_SESSION_AGENT_ID_DESCRIPTION, + MCP_SESSION_FIELD_DESCRIPTION, + MCP_SESSION_FIELD_DESCRIPTION_HOOK_MODE, + MCP_SESSION_KEY, + MCP_SESSION_SESSION_ID_DESCRIPTION, + MCP_SESSION_STATUS_DESCRIPTION, SESSION_ID_PARAM, SESSION_ID_PARAM_DESCRIPTION, + SESSION_ID_PARAM_PATTERN, + SESSION_START_SENTINEL, ) from agentcat.modules.logging import write_to_log from agentcat.types import AgentCatOptions @@ -69,7 +73,7 @@ class InjectionResult: injected_params has an entry for EVERY tool seen (possibly empty), so the strip fallback applies only to tools never seen in any listing. - output_injected lists tools whose outputSchema gained _mcp_instructions. + output_injected lists tools whose outputSchema gained mcp_session. declared_session_params lists tools whose OWN schema declared `session_id`; see `build_injected_schemas` for why that is a separate signal rather than "absent from injected_params". @@ -80,30 +84,40 @@ class InjectionResult: declared_session_params: set[str] = field(default_factory=set) -def mcp_instructions_schema_property( +def mcp_session_schema_property( include_session_id: bool, include_agent_id: bool ) -> dict[str, Any]: - """Build a fresh _mcp_instructions outputSchema fragment. + """Build a fresh mcp_session outputSchema fragment. - Sub-properties mirror the modes — no session_id in hook mode, no agent_id - when tracking is off — so the copy never references a parameter the - agent cannot see. `instructions` is unconditional. + Sub-properties mirror the modes — no session_id or status in hook mode, + no agent_id when tracking is off — so the copy never references a + parameter the agent cannot see. Hook mode is `include_session_id` False + with `include_agent_id` on, and carries its own field description. """ sub_properties: dict[str, Any] = {} if include_session_id: sub_properties[SESSION_ID_PARAM] = { "type": "string", - "description": MCP_INSTRUCTIONS_SESSION_ID_DESCRIPTION, + "description": MCP_SESSION_SESSION_ID_DESCRIPTION, } if include_agent_id: sub_properties[AGENT_ID_PARAM] = { "type": "string", - "description": MCP_INSTRUCTIONS_AGENT_ID_DESCRIPTION, + "description": MCP_SESSION_AGENT_ID_DESCRIPTION, + } + if include_session_id: + sub_properties["status"] = { + "type": "string", + "enum": ["issued", "active", "unrecognized"], + "description": MCP_SESSION_STATUS_DESCRIPTION, } - sub_properties["instructions"] = {"type": "string"} return { "type": "object", - "description": MCP_INSTRUCTIONS_FIELD_DESCRIPTION, + "description": ( + MCP_SESSION_FIELD_DESCRIPTION + if include_session_id + else MCP_SESSION_FIELD_DESCRIPTION_HOOK_MODE + ), "properties": sub_properties, } @@ -115,6 +129,7 @@ def _inject_param( description: str, required: bool, entry: set[str], + pattern: str | None = None, ) -> None: """Add one string param, honoring collisions and the required array.""" properties = schema.setdefault("properties", {}) @@ -124,7 +139,10 @@ def _inject_param( f"Skipping {name} injection." ) return - properties[name] = {"type": "string", "description": description} + prop: dict[str, Any] = {"type": "string", "description": description} + if pattern is not None: + prop["pattern"] = pattern + properties[name] = prop if required: existing = schema.get("required") if isinstance(existing, list): @@ -205,18 +223,14 @@ def _add_handle_parameters( schema, SESSION_ID_PARAM, SESSION_ID_PARAM_DESCRIPTION, - False, + True, entry, + pattern=SESSION_ID_PARAM_PATTERN, ) if inject_agent_id: - # Hook mode has no session_id param anywhere; never reference one the - # agent cannot see. - description = ( - AGENT_ID_PARAM_DESCRIPTION - if inject_session_id - else AGENT_ID_PARAM_DESCRIPTION_HOOK_MODE + _inject_param( + tool.name, schema, AGENT_ID_PARAM, AGENT_ID_PARAM_DESCRIPTION, True, entry ) - _inject_param(tool.name, schema, AGENT_ID_PARAM, description, True, entry) _extend_output_schema(tool, inject_session_id, inject_agent_id, result) @@ -227,7 +241,7 @@ def _extend_output_schema( inject_agent_id: bool, result: InjectionResult, ) -> None: - """Declare the optional _mcp_instructions property on a plain-object + """Declare the optional mcp_session property on a plain-object outputSchema so schema-validating clients accept the mirrored field. Never added to required. Composed schemas have no single properties bag @@ -240,18 +254,18 @@ def _extend_output_schema( if any(key in schema for key in _COMPOSED_KEYS): write_to_log( f'WARN: Tool "{tool.name}" has complex outputSchema ' - f"(oneOf/allOf/anyOf). Skipping {MCP_INSTRUCTIONS_KEY} injection; " + f"(oneOf/allOf/anyOf). Skipping {MCP_SESSION_KEY} injection; " "mint-back stays content-only for this tool." ) return properties = schema.setdefault("properties", {}) - if MCP_INSTRUCTIONS_KEY in properties: + if MCP_SESSION_KEY in properties: write_to_log( f'WARN: Tool "{tool.name}" already declares ' - f"'{MCP_INSTRUCTIONS_KEY}' in outputSchema. Skipping injection." + f"'{MCP_SESSION_KEY}' in outputSchema. Skipping injection." ) return - properties[MCP_INSTRUCTIONS_KEY] = mcp_instructions_schema_property( + properties[MCP_SESSION_KEY] = mcp_session_schema_property( inject_session_id, inject_agent_id ) result.output_injected.add(tool.name) @@ -270,8 +284,9 @@ def _add_context_parameter(tool: ToolSpec, description: str, entry: set[str]) -> means agents quietly stop supplying it and `user_intent` coverage decays with nothing to show for it. - `session_id` is the deliberate exception (never required): omitting it is how - an agent signals "mint me one". + `session_id` is required the same way, with `start` as its explicit + first-call value — an agent asks to be minted one by sending the sentinel, + and an absent value still mints so a stale schema never errors. """ schema = tool.input_schema properties = schema.get("properties") @@ -359,7 +374,8 @@ def injected_parameter_names( - `session_id` counts as ours iff prompted-mode tracing would have injected it (`enable_tracing` on, no `resolve_session_id` hook) AND the - value the agent sent is absent or matches our minted `ses_` KSUID shape. + value the agent sent is absent, the `start` sentinel our copy tells + agents to send first, or our minted `ses_` KSUID shape. A non-minted value is presumed the customer's own parameter: it is not stripped, and — because this same set feeds `resolve_handles` — it is not branded invalid or corrected on the wire either. @@ -388,10 +404,14 @@ def injected_parameter_names( names: set[str] = set() if opts.enable_tracing and opts.resolve_session_id is None: value = args.get(SESSION_ID_PARAM) - looks_minted = value is None or ( - isinstance(value, str) and is_valid_session_id(value.strip()) + looks_ours = value is None or ( + isinstance(value, str) + and ( + value.strip().lower() == SESSION_START_SENTINEL + or is_valid_session_id(value.strip()) + ) ) - if looks_minted: + if looks_ours: names.add(SESSION_ID_PARAM) if opts.enable_tracing and opts.enable_agent_tracking: names.add(AGENT_ID_PARAM) diff --git a/src/agentcat/types.py b/src/agentcat/types.py index 7b7582d..5290dda 100644 --- a/src/agentcat/types.py +++ b/src/agentcat/types.py @@ -262,7 +262,7 @@ class AgentCatOptions: event_properties: EventPropertiesFunction | None = None # Default False. Set True to inject a required agent_id parameter into every # tool. Agents self-generate the value (model|harness|nonce, e.g. - # "opus-4.80-1m|claude-code|k3n9x"); it is echoed back in _mcp_instructions + # "opus-4.80-1m|claude-code|k3n9x"); it is echoed back in mcp_session # and stamped on events as tags. Omission never rejects a call server-side — # the event is simply published without agent identity. The intended # enforcement is client-side: a strict schema-validating MCP client will @@ -294,7 +294,7 @@ class AgentCatData: # Per-tool set of parameter names AgentCat injected into the schema # (context/session_id/agent_id), so they can be stripped before the tool runs. injected_params_registry: dict[str, set[str]] | None = None - # Tools whose results receive an _mcp_instructions injection. + # Tools whose results receive an mcp_session injection. output_injection_registry: set[str] | None = None # Tools whose OWN input schema declares `session_id` — the customer's # parameter, never ours to read at call time. Membership only ever grows; diff --git a/src/agentcat/utils.py b/src/agentcat/utils.py index e176cb6..37d175f 100644 --- a/src/agentcat/utils.py +++ b/src/agentcat/utils.py @@ -59,7 +59,7 @@ def generate_prefixed_ksuid( prefix: str, use_milliseconds: bool = False, dt: Optional[datetime] = None ) -> str: """ - Generate a prefixed KSUID (e.g., "ses_iewjf9023rjdf"). + Generate a prefixed KSUID (e.g., "ses_2cOHEO0LYGADMzRvWTXXVbbgxgm"). Args: prefix: The prefix to add (e.g., "ses", "usr", "evt") @@ -96,7 +96,8 @@ def parse_prefixed_ksuid( Parse a prefixed KSUID string back into its prefix and Ksuid object. Args: - prefixed_ksuid: The prefixed KSUID string (e.g., "ses_iewjf9023rjdf") + prefixed_ksuid: The prefixed KSUID string + (e.g., "ses_2cOHEO0LYGADMzRvWTXXVbbgxgm") use_milliseconds: If True, parses as KsuidMs Returns: diff --git a/tests/community/test_community_v3_handles.py b/tests/community/test_community_v3_handles.py index e7fe69f..1139868 100644 --- a/tests/community/test_community_v3_handles.py +++ b/tests/community/test_community_v3_handles.py @@ -16,7 +16,7 @@ AGENTCAT_TAG_AGENT_ID, AGENTCAT_TAG_AGENT_SOURCE, AGENTCAT_TAG_SESSION_SOURCE, - MCP_INSTRUCTIONS_KEY, + MCP_SESSION_KEY, META_CLIENT_INFO_KEY, ) from agentcat.modules.handles import derive_session_id @@ -36,7 +36,7 @@ reason="Community FastMCP not available", ) -MINT_BACK_HEADER = "[MCP INSTRUCTIONS]: session_id issued." +MINT_BACK_HEADER = "[session_id issued — see this tool's session_id parameter description]" # noqa: E501 @pytest.fixture(autouse=True) @@ -75,10 +75,11 @@ async def test_prompted_mode_end_to_end(capture): listed = await client.list_tools() add = _named(listed, "add_todo") assert list(add.inputSchema["properties"])[-2:] == ["session_id", "context"] - assert "session_id" not in add.inputSchema.get("required", []) - # session_id is the one injected param that is never required — omitting - # it is the minting signal. `context` is required, which is the only - # thing that makes agents supply intent at all. + # Both injected params are required on the wire — schema-aware clients + # are the only enforcement, and session_id names `start` as its + # explicit first-call value. + assert "session_id" in add.inputSchema["required"] + assert "pattern" in add.inputSchema["properties"]["session_id"] assert "context" in add.inputSchema["required"] assert any(t.name == "get_more_tools" for t in listed) @@ -86,12 +87,13 @@ async def test_prompted_mode_end_to_end(capture): "add_todo", { "text": "hi", + "session_id": "start", "context": "Adding a todo item for the user's task list to track work", }, ) text = _text(r1) assert MINT_BACK_HEADER in text - minted = text.split("session_id=")[1].split(" ")[0] + minted = text.split("session_id: ")[1].split("\n")[0] assert minted.startswith("ses_") r2 = await client.call_tool("add_todo", {"text": "again", "session_id": minted}) @@ -109,14 +111,14 @@ async def test_prompted_mode_end_to_end(capture): # ...and the customer's result, undecorated. assert call_events[0].response is not None assert "Added todo" in json.dumps(call_events[0].response) - assert "[MCP INSTRUCTIONS]" not in json.dumps(call_events[0].response) + assert "[session_id" not in json.dumps(call_events[0].response) assert call_events[0].tags[AGENTCAT_TAG_SESSION_SOURCE] == "minted" assert call_events[1].tags[AGENTCAT_TAG_SESSION_SOURCE] == "supplied" assert call_events[0].user_intent.startswith("Adding a todo item") async def test_structured_mint_back_mirrors_into_structured_content(capture): - """A tool with an output schema gets `_mcp_instructions` mirrored in, and + """A tool with an output schema gets `mcp_session` mirrored in, and its schema declares the field so schema-validating clients still accept it.""" server = create_community_todo_server() @@ -124,12 +126,12 @@ async def test_structured_mint_back_mirrors_into_structured_content(capture): async with create_community_test_client(server) as client: listed = await client.list_tools() - assert MCP_INSTRUCTIONS_KEY in _named(listed, "add_todo").outputSchema[ + assert MCP_SESSION_KEY in _named(listed, "add_todo").outputSchema[ "properties" ] result = await client.call_tool("add_todo", {"text": "structured"}) - mint = result.structured_content[MCP_INSTRUCTIONS_KEY] + mint = result.structured_content[MCP_SESSION_KEY] assert mint["session_id"].startswith("ses_") assert mint["session_id"] == _call_events(capture)[0].session_id # The customer's own structured payload survives untouched. @@ -138,7 +140,7 @@ async def test_structured_mint_back_mirrors_into_structured_content(capture): async def test_handler_sees_stripped_args_and_customer_result_untouched(capture): """The injected params never reach the tool body, and what the tool returned - is exactly what the agent gets back (minus AgentCat's trailing block).""" + is exactly what the agent gets back (minus AgentCat's leading block).""" seen: dict = {} server = _new_server() @@ -160,7 +162,7 @@ def probe(text: str) -> str: assert result.is_error is False, _text(result) assert seen == {"text": "payload"} assert result.content[0].text == "probe:payload" - # session_id was supplied, so nothing is minted back and nothing is appended. + # session_id was supplied, so nothing is minted back and nothing is added. assert len(result.content) == 1 assert _call_events(capture)[0].session_id == sid("supplied") @@ -173,8 +175,9 @@ async def test_get_more_tools_keeps_its_own_context_and_publishes(capture): listed = await client.list_tools() gmt = _named(listed, "get_more_tools") # Its bespoke `context` is a real parameter: still required, still - # described by the tool's own copy — and handles ride alongside. - assert gmt.inputSchema["required"] == ["context"] + # described by the tool's own copy — and handles ride alongside, + # required like everywhere else. + assert gmt.inputSchema["required"] == ["context", "session_id"] assert gmt.inputSchema["properties"]["context"]["description"].startswith( "A description of your goal" ) @@ -505,7 +508,7 @@ async def test_agent_tracking_injection(capture): "context", ] assert "agent_id" in add.inputSchema["required"] - assert "session_id" not in add.inputSchema["required"] + assert "session_id" in add.inputSchema["required"] result = await client.call_tool( "add_todo", {"text": "with agent", "agent_id": "opus|claude-code|k3n9x"} @@ -533,9 +536,9 @@ async def test_hook_mode(capture): r1 = await client.call_tool("add_todo", {"text": "hook one"}) r2 = await client.call_tool("add_todo", {"text": "hook two"}) - assert "[MCP INSTRUCTIONS]" not in _text(r1) - assert "[MCP INSTRUCTIONS]" not in _text(r2) - assert MCP_INSTRUCTIONS_KEY not in (r1.structured_content or {}) + assert "[session_id" not in _text(r1) + assert "[session_id" not in _text(r2) + assert MCP_SESSION_KEY not in (r1.structured_content or {}) call_events = _call_events(capture) expected = derive_session_id("cust-1", "proj_test") @@ -578,7 +581,7 @@ async def test_tracing_disabled_strips_but_publishes_nothing(capture): "add_todo", {"text": "quiet", "context": "no tracing"} ) assert result.is_error is False, _text(result) - assert "[MCP INSTRUCTIONS]" not in _text(result) + assert "[session_id" not in _text(result) assert capture == [] @@ -1077,7 +1080,7 @@ def search(query: str, context: str) -> str: listed = await client.list_tools() schema = _named(listed, "search").inputSchema assert "description" not in schema["properties"]["context"] - assert schema["required"] == ["query", "context"] + assert schema["required"] == ["query", "context", "session_id"] await client.call_tool("search", {"query": "q", "context": "theirs"}) diff --git a/tests/community/test_community_v3_nested_calls.py b/tests/community/test_community_v3_nested_calls.py index 46d1709..61c420c 100644 --- a/tests/community/test_community_v3_nested_calls.py +++ b/tests/community/test_community_v3_nested_calls.py @@ -20,7 +20,7 @@ from agentcat.modules.constants import ( AGENTCAT_TAG_NESTED, AGENTCAT_TAG_SESSION_SOURCE, - MCP_INSTRUCTIONS_KEY, + MCP_SESSION_KEY, ) from ..test_utils.community_catalog_server import ( @@ -53,7 +53,7 @@ reason="fastmcp CatalogTransform not available", ) -MINT_BACK_HEADER = "[MCP INSTRUCTIONS]: session_id issued." +MINT_BACK_HEADER = "[session_id issued — see this tool's session_id parameter description]" # noqa: E501 CONTEXT = "Driving the hidden catalog through the meta tool to exercise nesting" @@ -76,7 +76,7 @@ def _call_events(capture) -> list: def _minted_from(result) -> str: - minted = _text(result).split("session_id=")[1].split(" ")[0] + minted = _text(result).split("session_id: ")[1].split("\n")[0] assert minted.startswith("ses_") return minted @@ -165,7 +165,7 @@ async def test_a_nested_call_joins_the_session_and_is_never_decorated(capture): # The inner result, as agent-authored code would consume it, is the # customer's data and nothing else: no mint-back block, no mirror key. assert observed["inner_text"] == "echo:hello" - assert "[MCP INSTRUCTIONS]" not in observed["inner_text"] + assert "[session_id" not in observed["inner_text"] assert observed["inner_structured"] == {"result": "echo:hello"} @@ -183,7 +183,7 @@ async def test_the_outer_call_is_still_fully_decorated(): result = await client.call_tool("run", {"program": "hello", "context": CONTEXT}) assert MINT_BACK_HEADER in _text(result) - mint = result.structured_content[MCP_INSTRUCTIONS_KEY] + mint = result.structured_content[MCP_SESSION_KEY] assert mint["session_id"] == _minted_from(result) assert result.structured_content["result"] == "ran:hello" diff --git a/tests/community/test_community_v3_openapi.py b/tests/community/test_community_v3_openapi.py index ac1ae57..147a9cf 100644 --- a/tests/community/test_community_v3_openapi.py +++ b/tests/community/test_community_v3_openapi.py @@ -174,7 +174,10 @@ async def test_get_more_tools_alongside_openapi(capture): # get_more_tools carries its own context arg by design; other tools get # one injected. assert "context" in by_name["get_severity"].inputSchema.get("properties", {}) - assert by_name["get_more_tools"].inputSchema["required"] == ["context"] + assert by_name["get_more_tools"].inputSchema["required"] == [ + "context", + "session_id", + ] result = await client.call_tool( "get_more_tools", {"context": "need more tools"} ) diff --git a/tests/e2e/community_v3/test_agent_handle_http.py b/tests/e2e/community_v3/test_agent_handle_http.py index 7e7dab7..101192d 100644 --- a/tests/e2e/community_v3/test_agent_handle_http.py +++ b/tests/e2e/community_v3/test_agent_handle_http.py @@ -28,13 +28,15 @@ AGENTCAT_TAG_AGENT_ID, AGENTCAT_TAG_AGENT_SOURCE, AGENTCAT_TAG_SESSION_SOURCE, - MCP_INSTRUCTIONS_KEY, + MCP_SESSION_KEY, SESSION_ID_PARAM, ) pytestmark = pytest.mark.e2e -MINT_BACK_HEADER = "[MCP INSTRUCTIONS]: session_id issued." +MINT_BACK_HEADER = ( + "[session_id issued — see this tool's session_id parameter description]" # noqa: E501 +) AGENT = "opus-4.80-1m|claude-code|k3n9x" @@ -58,8 +60,8 @@ async def test_the_agent_handle_survives_the_wire(v3_http_server, capture_queue) """Listing with agent tracking on: the schema the agent is handed. Property order is the contract (`modules/injection.py` §"Resulting property - order"), and `agent_id` is required where `session_id` is not — omission is - the minting signal for one and nothing for the other. + order"), and both handles are required — `session_id` names `start` as its + explicit first-call value, and a call that omits either is still served. """ from fastmcp import Client from fastmcp.client.transports import StreamableHttpTransport @@ -75,8 +77,9 @@ async def test_the_agent_handle_survives_the_wire(v3_http_server, capture_queue) "context", ] assert AGENT_ID_PARAM in add.inputSchema["required"] - assert SESSION_ID_PARAM not in add.inputSchema["required"] - assert MCP_INSTRUCTIONS_KEY in add.outputSchema["properties"] + assert SESSION_ID_PARAM in add.inputSchema["required"] + assert "pattern" in add.inputSchema["properties"][SESSION_ID_PARAM] + assert MCP_SESSION_KEY in add.outputSchema["properties"] @pytest.mark.asyncio @@ -95,8 +98,8 @@ async def test_a_supplied_agent_handle_tags_the_event(v3_http_server, capture_qu # handle fails here rather than passing silently. text = _text(result) assert MINT_BACK_HEADER in text - minted = text.split("session_id=")[1].split(" ")[0] - mirror = result.structured_content[MCP_INSTRUCTIONS_KEY] + minted = text.split("session_id: ")[1].split("\n")[0] + mirror = result.structured_content[MCP_SESSION_KEY] assert mirror[SESSION_ID_PARAM] == minted assert mirror[AGENT_ID_PARAM] == AGENT @@ -122,16 +125,22 @@ async def test_both_handles_echo_across_calls(v3_http_server, capture_queue): url, _ = v3_http_server async with Client(StreamableHttpTransport(url)) as client: first = await client.call_tool( - "add_todo", {"text": "one", AGENT_ID_PARAM: AGENT, "context": "start"} + "add_todo", + { + "text": "one", + SESSION_ID_PARAM: "start", + AGENT_ID_PARAM: AGENT, + "context": "start", + }, ) - minted = _text(first).split("session_id=")[1].split(" ")[0] + minted = _text(first).split("session_id: ")[1].split("\n")[0] second = await client.call_tool( "add_todo", {"text": "two", SESSION_ID_PARAM: minted, AGENT_ID_PARAM: AGENT}, ) assert MINT_BACK_HEADER not in _text(second) - assert second.structured_content[MCP_INSTRUCTIONS_KEY][AGENT_ID_PARAM] == AGENT + assert second.structured_content[MCP_SESSION_KEY][AGENT_ID_PARAM] == AGENT time.sleep(0.5) events = _call_events(capture_queue)[-2:] @@ -167,7 +176,7 @@ async def test_omitting_the_required_agent_handle_degrades_to_absence( async with Client(StreamableHttpTransport(url)) as client: result = await client.call_tool("add_todo", {"text": "no agent"}) assert MINT_BACK_HEADER in _text(result) - assert AGENT_ID_PARAM not in result.structured_content[MCP_INSTRUCTIONS_KEY] + assert AGENT_ID_PARAM not in result.structured_content[MCP_SESSION_KEY] time.sleep(0.5) event = _call_events(capture_queue)[-1] diff --git a/tests/e2e/community_v3/test_event_capture_http.py b/tests/e2e/community_v3/test_event_capture_http.py index a86c714..7f3c8f7 100644 --- a/tests/e2e/community_v3/test_event_capture_http.py +++ b/tests/e2e/community_v3/test_event_capture_http.py @@ -60,8 +60,11 @@ async def test_task_handle_is_minted_then_echoed(v3_http_server, capture_queue): async with Client(StreamableHttpTransport(url)) as client: first = await client.call_tool("add_todo", {"text": "one"}) text = "".join(c.text for c in first.content if hasattr(c, "text")) - assert "[MCP INSTRUCTIONS]: session_id issued." in text - minted = text.split("session_id=")[1].split(" ")[0] + assert ( + "[session_id issued — see this tool's session_id parameter description]" + in text + ) # noqa: E501 + minted = text.split("session_id: ")[1].split("\n")[0] await client.call_tool("add_todo", {"text": "two", "session_id": minted}) diff --git a/tests/e2e/community_v3/test_stateless_http.py b/tests/e2e/community_v3/test_stateless_http.py index 685652b..3b697de 100644 --- a/tests/e2e/community_v3/test_stateless_http.py +++ b/tests/e2e/community_v3/test_stateless_http.py @@ -38,7 +38,7 @@ async def test_every_call_carries_a_task_handle(v3_http_server, capture_queue): async with Client(StreamableHttpTransport(url)) as client: first = await client.call_tool("add_todo", {"text": "s", "context": "x"}) text = "".join(c.text for c in first.content if hasattr(c, "text")) - minted = text.split("session_id=")[1].split(" ")[0] + minted = text.split("session_id: ")[1].split("\n")[0] await client.call_tool("add_todo", {"text": "s2", "session_id": minted}) diff --git a/tests/e2e/community_v4/test_agent_handle_http.py b/tests/e2e/community_v4/test_agent_handle_http.py index 898b3a6..7e268fa 100644 --- a/tests/e2e/community_v4/test_agent_handle_http.py +++ b/tests/e2e/community_v4/test_agent_handle_http.py @@ -28,13 +28,15 @@ AGENTCAT_TAG_AGENT_ID, AGENTCAT_TAG_AGENT_SOURCE, AGENTCAT_TAG_SESSION_SOURCE, - MCP_INSTRUCTIONS_KEY, + MCP_SESSION_KEY, SESSION_ID_PARAM, ) pytestmark = pytest.mark.e2e -MINT_BACK_HEADER = "[MCP INSTRUCTIONS]: session_id issued." +MINT_BACK_HEADER = ( + "[session_id issued — see this tool's session_id parameter description]" # noqa: E501 +) AGENT = "opus-4.80-1m|claude-code|k3n9x" @@ -59,9 +61,9 @@ async def test_the_agent_handle_survives_the_wire(v4_http_server, capture_queue) FastMCP validates its own outbound results, so a schema that is malformed once `agent_id` joins `session_id` and `context` fails server-side here rather than reaching the agent. Property order is the contract - (`modules/injection.py` §"Resulting property order"), and `agent_id` is - required where `session_id` is not — omission is the minting signal for one - and nothing for the other. + (`modules/injection.py` §"Resulting property order"), and both handles are + required — `session_id` names `start` as its explicit first-call value, + and a call that omits either is still served. """ from fastmcp import Client from fastmcp.client.transports import StreamableHttpTransport @@ -78,8 +80,9 @@ async def test_the_agent_handle_survives_the_wire(v4_http_server, capture_queue) "context", ] assert AGENT_ID_PARAM in add.input_schema["required"] - assert SESSION_ID_PARAM not in add.input_schema["required"] - assert MCP_INSTRUCTIONS_KEY in add.output_schema["properties"] + assert SESSION_ID_PARAM in add.input_schema["required"] + assert "pattern" in add.input_schema["properties"][SESSION_ID_PARAM] + assert MCP_SESSION_KEY in add.output_schema["properties"] async def test_a_supplied_agent_handle_tags_the_event(v4_http_server, capture_queue): @@ -97,8 +100,8 @@ async def test_a_supplied_agent_handle_tags_the_event(v4_http_server, capture_qu # handle fails here rather than passing silently. text = _text(result) assert MINT_BACK_HEADER in text - minted = text.split("session_id=")[1].split(" ")[0] - mirror = result.structured_content[MCP_INSTRUCTIONS_KEY] + minted = text.split("session_id: ")[1].split("\n")[0] + mirror = result.structured_content[MCP_SESSION_KEY] assert mirror[SESSION_ID_PARAM] == minted assert mirror[AGENT_ID_PARAM] == AGENT @@ -123,16 +126,22 @@ async def test_both_handles_echo_across_calls(v4_http_server, capture_queue): url, _ = v4_http_server async with Client(StreamableHttpTransport(url)) as client: first = await client.call_tool( - "add_todo", {"text": "one", AGENT_ID_PARAM: AGENT, "context": "start"} + "add_todo", + { + "text": "one", + SESSION_ID_PARAM: "start", + AGENT_ID_PARAM: AGENT, + "context": "start", + }, ) - minted = _text(first).split("session_id=")[1].split(" ")[0] + minted = _text(first).split("session_id: ")[1].split("\n")[0] second = await client.call_tool( "add_todo", {"text": "two", SESSION_ID_PARAM: minted, AGENT_ID_PARAM: AGENT}, ) assert MINT_BACK_HEADER not in _text(second) - assert second.structured_content[MCP_INSTRUCTIONS_KEY][AGENT_ID_PARAM] == AGENT + assert second.structured_content[MCP_SESSION_KEY][AGENT_ID_PARAM] == AGENT time.sleep(0.5) events = _call_events(capture_queue)[-2:] @@ -167,7 +176,7 @@ async def test_omitting_the_required_agent_handle_degrades_to_absence( async with Client(StreamableHttpTransport(url)) as client: result = await client.call_tool("add_todo", {"text": "no agent"}) assert MINT_BACK_HEADER in _text(result) - assert AGENT_ID_PARAM not in result.structured_content[MCP_INSTRUCTIONS_KEY] + assert AGENT_ID_PARAM not in result.structured_content[MCP_SESSION_KEY] time.sleep(0.5) event = _call_events(capture_queue)[-1] diff --git a/tests/e2e/community_v4/test_session_http.py b/tests/e2e/community_v4/test_session_http.py index 9107e08..fce6e74 100644 --- a/tests/e2e/community_v4/test_session_http.py +++ b/tests/e2e/community_v4/test_session_http.py @@ -22,13 +22,15 @@ from agentcat.modules.constants import ( AGENTCAT_TAG_SESSION_SOURCE, - MCP_INSTRUCTIONS_KEY, + MCP_SESSION_KEY, SESSION_ID_PARAM, ) pytestmark = pytest.mark.e2e -MINT_BACK_HEADER = "[MCP INSTRUCTIONS]: session_id issued." +MINT_BACK_HEADER = ( + "[session_id issued — see this tool's session_id parameter description]" # noqa: E501 +) def _call_events(capture_queue): @@ -52,7 +54,7 @@ async def test_handshake_and_list_publish_nothing(v4_http_server, capture_queue) time.sleep(0.5) add = next(t for t in listed if t.name == "add_todo") assert list(add.input_schema["properties"])[-2:] == [SESSION_ID_PARAM, "context"] - assert MCP_INSTRUCTIONS_KEY in add.output_schema["properties"] + assert MCP_SESSION_KEY in add.output_schema["properties"] assert capture_queue == [], [e.event_type for e in capture_queue] @@ -65,13 +67,18 @@ async def test_task_handle_is_minted_then_echoed(v4_http_server, capture_queue): url, _ = v4_http_server async with Client(StreamableHttpTransport(url)) as client: first = await client.call_tool( - "add_todo", {"text": "one", "context": "first call of the task"} + "add_todo", + { + "text": "one", + SESSION_ID_PARAM: "start", + "context": "first call of the task", + }, ) text = _text(first) assert MINT_BACK_HEADER in text - minted = text.split("session_id=")[1].split(" ")[0] + minted = text.split("session_id: ")[1].split("\n")[0] assert minted.startswith("ses_") - assert first.structured_content[MCP_INSTRUCTIONS_KEY]["session_id"] == minted + assert first.structured_content[MCP_SESSION_KEY]["session_id"] == minted second = await client.call_tool( "add_todo", {"text": "two", SESSION_ID_PARAM: minted} diff --git a/tests/e2e/community_v4/test_stateless_http.py b/tests/e2e/community_v4/test_stateless_http.py index a81af17..5092920 100644 --- a/tests/e2e/community_v4/test_stateless_http.py +++ b/tests/e2e/community_v4/test_stateless_http.py @@ -48,7 +48,7 @@ async def test_a_task_handle_survives_with_no_server_side_session( first = await client.call_tool( "add_todo", {"text": "s", "context": "stateless first call"} ) - minted = _text(first).split("session_id=")[1].split(" ")[0] + minted = _text(first).split("session_id: ")[1].split("\n")[0] await client.call_tool("add_todo", {"text": "s2", "session_id": minted}) time.sleep(0.5) diff --git a/tests/e2e/official/test_agent_handle_http.py b/tests/e2e/official/test_agent_handle_http.py index 78b2ee1..625cefd 100644 --- a/tests/e2e/official/test_agent_handle_http.py +++ b/tests/e2e/official/test_agent_handle_http.py @@ -30,7 +30,7 @@ AGENTCAT_TAG_AGENT_ID, AGENTCAT_TAG_AGENT_SOURCE, AGENTCAT_TAG_SESSION_SOURCE, - MCP_INSTRUCTIONS_KEY, + MCP_SESSION_KEY, SESSION_ID_PARAM, ) from tests.test_utils import NEEDS_STRUCTURED_OUTPUT @@ -38,7 +38,9 @@ pytestmark = pytest.mark.e2e -MINT_BACK_HEADER = "[MCP INSTRUCTIONS]: session_id issued." +MINT_BACK_HEADER = ( + "[session_id issued — see this tool's session_id parameter description]" # noqa: E501 +) AGENT = "opus-4.80-1m|claude-code|k3n9x" @@ -62,8 +64,8 @@ async def test_the_agent_handle_survives_the_wire(official_http_server, capture_ """Listing with agent tracking on: the schema the agent is handed. Property order is the contract (`modules/injection.py` §"Resulting property - order"), and `agent_id` is required where `session_id` is not — omission is - the minting signal for one and nothing for the other. + order"), and both handles are required — `session_id` names `start` as its + explicit first-call value, and a call that omits either is still served. """ url, _ = official_http_server async with streamablehttp_client(url) as (read, write, _): @@ -78,7 +80,8 @@ async def test_the_agent_handle_survives_the_wire(official_http_server, capture_ "context", ] assert AGENT_ID_PARAM in add.inputSchema["required"] - assert SESSION_ID_PARAM not in add.inputSchema["required"] + assert SESSION_ID_PARAM in add.inputSchema["required"] + assert "pattern" in add.inputSchema["properties"][SESSION_ID_PARAM] @pytest.mark.asyncio @@ -124,9 +127,15 @@ async def test_both_handles_echo_across_calls(official_http_server, capture_queu async with ClientSession(read, write) as client: await client.initialize() first = await client.call_tool( - "add_todo", {"text": "one", AGENT_ID_PARAM: AGENT, "context": "start"} + "add_todo", + { + "text": "one", + SESSION_ID_PARAM: "start", + AGENT_ID_PARAM: AGENT, + "context": "start", + }, ) - minted = _text(first).split("session_id=")[1].split(" ")[0] + minted = _text(first).split("session_id: ")[1].split("\n")[0] second = await client.call_tool( "add_todo", @@ -222,12 +231,12 @@ async def test_both_handles_are_mirrored_into_structured_content( await client.initialize() listed = await client.list_tools() add = next(t for t in listed.tools if t.name == "add_todo") - assert MCP_INSTRUCTIONS_KEY in add.outputSchema["properties"] + assert MCP_SESSION_KEY in add.outputSchema["properties"] result = await client.call_tool( "add_todo", {"text": "mirrored", AGENT_ID_PARAM: AGENT} ) - mirror = result.structuredContent[MCP_INSTRUCTIONS_KEY] + mirror = result.structuredContent[MCP_SESSION_KEY] assert mirror[SESSION_ID_PARAM].startswith("ses_") assert mirror[AGENT_ID_PARAM] == AGENT # The customer's own structured payload survives untouched. diff --git a/tests/e2e/official/test_session_http.py b/tests/e2e/official/test_session_http.py index 78fdb6b..b967fc0 100644 --- a/tests/e2e/official/test_session_http.py +++ b/tests/e2e/official/test_session_http.py @@ -184,18 +184,23 @@ async def test_minted_session_id_is_echoed_across_http_calls( async with ClientSession(read, write) as client: await client.initialize() first = await client.call_tool( - "add_todo", {"text": "one", "context": "first call of the task"} + "add_todo", + { + "text": "one", + "session_id": "start", + "context": "first call of the task", + }, ) text = _text(first) - assert "[MCP INSTRUCTIONS]: session_id issued." in text - minted = text.split("session_id=")[1].split(" ")[0] + assert "[session_id issued — see this tool's session_id parameter description]" in text # noqa: E501 + minted = text.split("session_id: ")[1].split("\n")[0] assert minted.startswith("ses_") second = await client.call_tool( "add_todo", {"text": "two", "session_id": minted} ) # Already supplied: nothing is minted back a second time. - assert "[MCP INSTRUCTIONS]: session_id issued." not in _text(second) + assert "[session_id issued — see this tool's session_id parameter description]" not in _text(second) # noqa: E501 time.sleep(0.5) events = _call_events(capture_queue) @@ -211,8 +216,8 @@ async def test_minted_session_id_is_echoed_across_http_calls( async def test_separate_connections_get_separate_tasks( official_http_server, capture_queue ): - """Nothing is stored server-side, so two agents that never echo a handle - get two different tasks.""" + """Nothing is stored server-side, so two agents that each send `start` + get two different tasks — start always begins a new, unrelated one.""" url, _ = official_http_server async def call_once(text: str) -> str: @@ -220,9 +225,14 @@ async def call_once(text: str) -> str: async with ClientSession(read, write) as client: await client.initialize() result = await client.call_tool( - "add_todo", {"text": text, "context": "independent task"} + "add_todo", + { + "text": text, + "session_id": "start", + "context": "independent task", + }, ) - return _text(result).split("session_id=")[1].split(" ")[0] + return _text(result).split("session_id: ")[1].split("\n")[0] first = await call_once("a") second = await call_once("b") diff --git a/tests/e2e/official/test_stateless_http.py b/tests/e2e/official/test_stateless_http.py index 3765dcb..e992e0e 100644 --- a/tests/e2e/official/test_stateless_http.py +++ b/tests/e2e/official/test_stateless_http.py @@ -41,7 +41,7 @@ async def test_every_call_carries_a_task_handle(official_http_server, capture_qu "add_todo", {"text": "s", "context": "stateless"} ) text = "".join(c.text for c in result.content if hasattr(c, "text")) - minted = text.split("session_id=")[1].split(" ")[0] + minted = text.split("session_id: ")[1].split("\n")[0] await client.call_tool("add_todo", {"text": "s2", "session_id": minted}) diff --git a/tests/e2e/official_modern/test_agent_handle_http.py b/tests/e2e/official_modern/test_agent_handle_http.py index e38853b..8d1c200 100644 --- a/tests/e2e/official_modern/test_agent_handle_http.py +++ b/tests/e2e/official_modern/test_agent_handle_http.py @@ -30,13 +30,15 @@ AGENTCAT_TAG_AGENT_ID, AGENTCAT_TAG_AGENT_SOURCE, AGENTCAT_TAG_SESSION_SOURCE, - MCP_INSTRUCTIONS_KEY, + MCP_SESSION_KEY, SESSION_ID_PARAM, ) pytestmark = pytest.mark.e2e -MINT_BACK_HEADER = "[MCP INSTRUCTIONS]: session_id issued." +MINT_BACK_HEADER = ( + "[session_id issued — see this tool's session_id parameter description]" # noqa: E501 +) AGENT = "opus-4.80-1m|claude-code|k3n9x" @@ -59,8 +61,8 @@ async def test_the_agent_handle_survives_the_wire(modern_http_server, capture_qu """Listing with agent tracking on: the schema the agent is handed. Property order is the contract (`modules/injection.py` §"Resulting property - order"), and `agent_id` is required where `session_id` is not — omission is - the minting signal for one and nothing for the other. + order"), and both handles are required — `session_id` names `start` as its + explicit first-call value, and a call that omits either is still served. """ url, _ = modern_http_server async with Client(url) as client: @@ -74,8 +76,9 @@ async def test_the_agent_handle_survives_the_wire(modern_http_server, capture_qu "context", ] assert AGENT_ID_PARAM in add.input_schema["required"] - assert SESSION_ID_PARAM not in add.input_schema["required"] - assert MCP_INSTRUCTIONS_KEY in add.output_schema["properties"] + assert SESSION_ID_PARAM in add.input_schema["required"] + assert "pattern" in add.input_schema["properties"][SESSION_ID_PARAM] + assert MCP_SESSION_KEY in add.output_schema["properties"] async def test_a_supplied_agent_handle_tags_the_event( @@ -93,9 +96,9 @@ async def test_a_supplied_agent_handle_tags_the_event( assert result.is_error is False, _text(result) text = _text(result) assert MINT_BACK_HEADER in text - minted = text.split("session_id=")[1].split(" ")[0] + minted = text.split("session_id: ")[1].split("\n")[0] # Both handles are mirrored, so an agent can re-read either mid-session. - mirror = result.structured_content[MCP_INSTRUCTIONS_KEY] + mirror = result.structured_content[MCP_SESSION_KEY] assert mirror[SESSION_ID_PARAM] == minted assert mirror[AGENT_ID_PARAM] == AGENT @@ -118,9 +121,15 @@ async def test_both_handles_echo_across_calls(modern_http_server, capture_queue) url, _ = modern_http_server async with Client(url) as client: first = await client.call_tool( - "add_todo", {"text": "one", AGENT_ID_PARAM: AGENT, "context": "start"} + "add_todo", + { + "text": "one", + SESSION_ID_PARAM: "start", + AGENT_ID_PARAM: AGENT, + "context": "start", + }, ) - minted = _text(first).split("session_id=")[1].split(" ")[0] + minted = _text(first).split("session_id: ")[1].split("\n")[0] second = await client.call_tool( "add_todo", @@ -128,7 +137,7 @@ async def test_both_handles_echo_across_calls(modern_http_server, capture_queue) ) assert second.is_error is False, _text(second) assert MINT_BACK_HEADER not in _text(second) - assert second.structured_content[MCP_INSTRUCTIONS_KEY][AGENT_ID_PARAM] == AGENT + assert second.structured_content[MCP_SESSION_KEY][AGENT_ID_PARAM] == AGENT time.sleep(0.5) events = _call_events(capture_queue)[-2:] @@ -165,7 +174,7 @@ async def test_omitting_the_required_agent_handle_degrades_to_absence( assert result.is_error is False, _text(result) assert MINT_BACK_HEADER in _text(result) # Nothing to confirm, so the mirror names only the session. - assert AGENT_ID_PARAM not in result.structured_content[MCP_INSTRUCTIONS_KEY] + assert AGENT_ID_PARAM not in result.structured_content[MCP_SESSION_KEY] time.sleep(0.5) event = _call_events(capture_queue)[-1] diff --git a/tests/e2e/official_modern/test_mcpserver_http.py b/tests/e2e/official_modern/test_mcpserver_http.py index c4278d9..5744f68 100644 --- a/tests/e2e/official_modern/test_mcpserver_http.py +++ b/tests/e2e/official_modern/test_mcpserver_http.py @@ -20,7 +20,7 @@ from agentcat.modules.constants import ( AGENTCAT_TAG_SESSION_SOURCE, - MCP_INSTRUCTIONS_KEY, + MCP_SESSION_KEY, ) from tests.test_utils.delivery import delivered_arguments_for from tests.test_utils.modern_server import create_mcpserver_todo_server @@ -31,7 +31,9 @@ SERVER_FACTORY = create_mcpserver_todo_server -MINT_BACK_HEADER = "[MCP INSTRUCTIONS]: session_id issued." +MINT_BACK_HEADER = ( + "[session_id issued — see this tool's session_id parameter description]" # noqa: E501 +) def _call_events(capture_queue): @@ -51,16 +53,17 @@ async def test_mcpserver_injects_strips_and_publishes( listed = await client.list_tools() add = next(t for t in listed.tools if t.name == "add_todo") assert list(add.input_schema["properties"])[-2:] == ["session_id", "context"] - assert MCP_INSTRUCTIONS_KEY in add.output_schema["properties"] + assert MCP_SESSION_KEY in add.output_schema["properties"] result = await client.call_tool( - "add_todo", {"text": "over http", "context": "why"} + "add_todo", + {"text": "over http", "session_id": "start", "context": "why"}, ) assert result.is_error is False, _text(result) text = _text(result) assert MINT_BACK_HEADER in text - minted = text.split("session_id=")[1].split(" ")[0] - assert result.structured_content[MCP_INSTRUCTIONS_KEY]["session_id"] == minted + minted = text.split("session_id: ")[1].split("\n")[0] + assert result.structured_content[MCP_SESSION_KEY]["session_id"] == minted assert result.structured_content["result"].startswith("Added todo") # The tool layer never saw `context` — the only observation on this shape @@ -73,12 +76,17 @@ async def test_mcpserver_injects_strips_and_publishes( assert [e.resource_name for e in events] == ["add_todo"] assert events[0].session_id == minted assert events[0].tags[AGENTCAT_TAG_SESSION_SOURCE] == "minted" - # The event carries the RAW arguments and the UNDECORATED response. - assert events[0].parameters["arguments"] == {"text": "over http", "context": "why"} - assert MCP_INSTRUCTIONS_KEY not in (events[0].response or {}).get( + # The event carries the RAW arguments — the sentinel as the agent sent + # it — and the UNDECORATED response. + assert events[0].parameters["arguments"] == { + "text": "over http", + "session_id": "start", + "context": "why", + } + assert MCP_SESSION_KEY not in (events[0].response or {}).get( "structuredContent", {} ) - assert MCP_INSTRUCTIONS_KEY not in (events[0].response or {}).get( + assert MCP_SESSION_KEY not in (events[0].response or {}).get( "structured_content", {} ) diff --git a/tests/e2e/official_modern/test_session_http.py b/tests/e2e/official_modern/test_session_http.py index c088817..a62178e 100644 --- a/tests/e2e/official_modern/test_session_http.py +++ b/tests/e2e/official_modern/test_session_http.py @@ -22,7 +22,7 @@ pytestmark = pytest.mark.e2e -MINT_BACK_HEADER = "[MCP INSTRUCTIONS]: session_id issued." +MINT_BACK_HEADER = "[session_id issued — see this tool's session_id parameter description]" # noqa: E501 def _call_events(capture_queue): @@ -42,11 +42,16 @@ async def test_minted_session_id_is_echoed_across_http_calls( url, _ = modern_http_server async with Client(url) as client: first = await client.call_tool( - "add_todo", {"text": "one", "context": "first call of the task"} + "add_todo", + { + "text": "one", + "session_id": "start", + "context": "first call of the task", + }, ) text = _text(first) assert MINT_BACK_HEADER in text - minted = text.split("session_id=")[1].split(" ")[0] + minted = text.split("session_id: ")[1].split("\n")[0] assert minted.startswith("ses_") second = await client.call_tool( @@ -70,16 +75,21 @@ async def test_minted_session_id_is_echoed_across_http_calls( async def test_separate_connections_get_separate_tasks( modern_http_server, capture_queue ): - """Nothing is stored server-side, so two agents that never echo a handle - get two different tasks.""" + """Nothing is stored server-side, so two agents that each send `start` + get two different tasks — start always begins a new, unrelated one.""" url, _ = modern_http_server async def call_once(text: str) -> str: async with Client(url) as client: result = await client.call_tool( - "add_todo", {"text": text, "context": "independent task"} + "add_todo", + { + "text": text, + "session_id": "start", + "context": "independent task", + }, ) - return _text(result).split("session_id=")[1].split(" ")[0] + return _text(result).split("session_id: ")[1].split("\n")[0] first = await call_once("a") second = await call_once("b") @@ -255,4 +265,4 @@ async def test_injected_schema_survives_the_wire(modern_http_server, capture_que add = next(t for t in listed.tools if t.name == "add_todo") assert list(add.input_schema["properties"])[-2:] == ["session_id", "context"] - assert "_mcp_instructions" in add.output_schema["properties"] + assert "mcp_session" in add.output_schema["properties"] diff --git a/tests/e2e/official_modern/test_stateless_http.py b/tests/e2e/official_modern/test_stateless_http.py index fa0ccb9..baa4ee6 100644 --- a/tests/e2e/official_modern/test_stateless_http.py +++ b/tests/e2e/official_modern/test_stateless_http.py @@ -40,7 +40,7 @@ async def test_a_task_handle_survives_with_no_server_side_session( first = await client.call_tool( "add_todo", {"text": "s", "context": "stateless first call"} ) - minted = _text(first).split("session_id=")[1].split(" ")[0] + minted = _text(first).split("session_id: ")[1].split("\n")[0] await client.call_tool("add_todo", {"text": "s2", "session_id": minted}) time.sleep(0.5) diff --git a/tests/test_callpath.py b/tests/test_callpath.py index b23ee61..1b3e2b8 100644 --- a/tests/test_callpath.py +++ b/tests/test_callpath.py @@ -95,30 +95,34 @@ def test_decorate_content_only_when_minted_prompted(): minted = HandleResolution(sid("T"), "minted") out = decorate_content([{"type": "text", "text": "x"}], minted, text_block) assert out is not None - assert "[MCP INSTRUCTIONS]: session_id issued." in out[-1]["text"] + assert out[0]["text"].startswith("[session_id issued") assert decorate_content([{"t": 1}], HandleResolution(sid("T"), "supplied"), dict) is None # noqa: E501 assert decorate_content([{"t": 1}], HandleResolution(sid("T"), "minted", hook_mode=True), dict) is None # noqa: E501 assert decorate_content("not-a-list", minted, dict) is None -# §3.4a: the appended text carries the id the agent must echo, built by the one -# source of truth (handles.build_mint_back_text) rather than re-derived here. -def test_decorate_content_appends_the_minted_id(): - out = decorate_content([], HandleResolution(sid("ABC"), "minted"), text_block) - assert out is not None and len(out) == 1 - assert "session_id=ses_ABC" in out[0]["text"] - assert out[0]["text"].endswith( - "Without session_id, this server does not function as intended." +# §3.4a: the prepended text carries the id the agent must echo, built by the +# one source of truth (handles.build_mint_back_text) rather than re-derived +# here. It is the FIRST content element, where client-side truncation of long +# results cannot reach it. +def test_decorate_content_prepends_the_minted_id(): + out = decorate_content( + [{"type": "text", "text": "payload"}], + HandleResolution(sid("ABC"), "minted"), + text_block, ) + assert out is not None and len(out) == 2 + assert "session_id: ses_ABC" in out[0]["text"] + assert out[1] == {"type": "text", "text": "payload"} -# §3.4a: "Append it to error results too" — decorate_content never inspects -# error state, so an isError result decorates on exactly the same terms. +# §3.4a: error results decorate too — decorate_content never inspects error +# state, so an isError result decorates on exactly the same terms. def test_decorate_content_ignores_error_state(): error_content = [{"type": "text", "text": "boom: tool failed"}] out = decorate_content(error_content, HandleResolution(sid("T"), "minted"), text_block) # noqa: E501 assert out is not None and len(out) == 2 - assert "[MCP INSTRUCTIONS]" in out[-1]["text"] + assert out[0]["text"].startswith("[session_id issued") # Never mutate the customer's result: the returned list is new and the @@ -143,7 +147,9 @@ def test_structured_mirror_gate_matrix(): mirrored = structured_mirror({"ok": True}, res, "search", {"search"}) assert mirrored is not None assert mirrored["ok"] is True - assert mirrored["_mcp_instructions"]["session_id"] == sid("T") + assert mirrored["mcp_session"]["session_id"] == sid("T") + # Mirror inserted as the FIRST key of structuredContent. + assert list(mirrored) == ["mcp_session", "ok"] # Registry exists but this tool is not in it: mirroring would fail the # customer's own schema validation. assert structured_mirror({"ok": True}, res, "other", {"search"}) is None @@ -160,7 +166,7 @@ def test_structured_mirror_delegates_shape_rules(): assert structured_mirror(["a"], res, "search", None) is None assert structured_mirror("str", res, "search", None) is None # Customer data under the key wins. - assert structured_mirror({"_mcp_instructions": "mine"}, res, "search", None) is None + assert structured_mirror({"mcp_session": "mine"}, res, "search", None) is None def test_structured_mirror_hook_mode_without_agent_has_nothing_to_mirror(): @@ -172,8 +178,7 @@ def test_structured_mirror_hook_mode_without_agent_has_nothing_to_mirror(): ) mirrored = structured_mirror({"ok": True}, with_agent, "search", None) assert mirrored is not None - assert "session_id" not in mirrored["_mcp_instructions"] - assert mirrored["_mcp_instructions"]["agent_id"] == "agt|x|1" + assert mirrored["mcp_session"] == {"agent_id": "agt|x|1"} # ── get_stripped_arguments ─────────────────────────────────────────────────── diff --git a/tests/test_community_v4_handles.py b/tests/test_community_v4_handles.py index 5d2e165..a10d283 100644 --- a/tests/test_community_v4_handles.py +++ b/tests/test_community_v4_handles.py @@ -36,7 +36,7 @@ AGENTCAT_TAG_AGENT_ID, AGENTCAT_TAG_MRTR, AGENTCAT_TAG_SESSION_SOURCE, - MCP_INSTRUCTIONS_KEY, + MCP_SESSION_KEY, SESSION_ID_PARAM, ) from agentcat.modules.detection import ServerFlavor, detect_server @@ -60,7 +60,7 @@ reason="Community FastMCP not available", ) -MINT_BACK_HEADER = "[MCP INSTRUCTIONS]: session_id issued." +MINT_BACK_HEADER = "[session_id issued — see this tool's session_id parameter description]" # noqa: E501 @pytest.fixture(autouse=True) @@ -311,18 +311,25 @@ async def test_prompted_mode_end_to_end(capture): add = _named(listed, "add_todo") tail = list(add.input_schema["properties"])[-2:] assert tail == [SESSION_ID_PARAM, "context"] - assert SESSION_ID_PARAM not in add.input_schema.get("required", []) - assert MCP_INSTRUCTIONS_KEY in add.output_schema["properties"] + # Required, with the start|ses_ value contract as its pattern. + assert SESSION_ID_PARAM in add.input_schema["required"] + assert "pattern" in add.input_schema["properties"][SESSION_ID_PARAM] + assert MCP_SESSION_KEY in add.output_schema["properties"] assert any(t.name == "get_more_tools" for t in listed) r1 = await client.call_tool( - "add_todo", {"text": "hi", "context": "tracking the user's work"} + "add_todo", + { + "text": "hi", + SESSION_ID_PARAM: "start", + "context": "tracking the user's work", + }, ) text = _text(r1) assert MINT_BACK_HEADER in text - minted = text.split("session_id=")[1].split(" ")[0] + minted = text.split("session_id: ")[1].split("\n")[0] assert minted.startswith("ses_") - assert r1.structured_content[MCP_INSTRUCTIONS_KEY]["session_id"] == minted + assert r1.structured_content[MCP_SESSION_KEY]["session_id"] == minted r2 = await client.call_tool( "add_todo", {"text": "again", SESSION_ID_PARAM: minted} @@ -501,7 +508,7 @@ def guarded(text: str) -> str | mt.InputRequiredResult: assert events[1].tags[AGENTCAT_TAG_MRTR] == "continuation" # The completing round is the one that mints, and its handle is the one the # agent was handed. - minted = _text(result).split("session_id=")[1].split(" ")[0] + minted = _text(result).split("session_id: ")[1].split("\n")[0] assert events[1].session_id == minted @@ -649,7 +656,7 @@ async def on_list_tools(self, context, call_next): # The cache below us never saw an injected argument... assert below["arguments"] == [{"text": "same"}] # ...and its hit did not swallow the second call. - minted = [_text(r).split("session_id=")[1].split(" ")[0] for r in (r1, r2)] + minted = [_text(r).split("session_id: ")[1].split("\n")[0] for r in (r1, r2)] assert minted[0] != minted[1] assert [e.session_id for e in _call_events(capture)] == minted diff --git a/tests/test_community_v4_nested_calls.py b/tests/test_community_v4_nested_calls.py index dca22a7..1c8d3d7 100644 --- a/tests/test_community_v4_nested_calls.py +++ b/tests/test_community_v4_nested_calls.py @@ -14,7 +14,7 @@ from agentcat.modules.constants import ( AGENTCAT_TAG_NESTED, AGENTCAT_TAG_SESSION_SOURCE, - MCP_INSTRUCTIONS_KEY, + MCP_SESSION_KEY, ) from .test_utils.community_catalog_server import ( @@ -34,7 +34,7 @@ reason="Community FastMCP with CatalogTransform not available", ) -MINT_BACK_HEADER = "[MCP INSTRUCTIONS]: session_id issued." +MINT_BACK_HEADER = "[session_id issued — see this tool's session_id parameter description]" # noqa: E501 CONTEXT = "Driving the hidden catalog through the meta tool to exercise nesting" @@ -67,7 +67,7 @@ async def test_the_echoed_session_id_still_strips_after_a_nested_call(capture): assert {"session_id", "context"} <= set(run_tool.input_schema["properties"]) r1 = await client.call_tool("run", {"program": "first", "context": CONTEXT}) - minted = _text(r1).split("session_id=")[1].split(" ")[0] + minted = _text(r1).split("session_id: ")[1].split("\n")[0] assert minted.startswith("ses_") # A raise here is the regression: the nested catalog fetch inside call @@ -105,5 +105,5 @@ async def test_a_nested_call_joins_the_session_and_is_never_decorated(capture): ) # ...while the outer wire result keeps both mint-back forms. assert MINT_BACK_HEADER in _text(result) - mint = result.structured_content[MCP_INSTRUCTIONS_KEY] + mint = result.structured_content[MCP_SESSION_KEY] assert mint["session_id"] == outer.session_id diff --git a/tests/test_concurrency_handles.py b/tests/test_concurrency_handles.py index d325a02..1f170c0 100644 --- a/tests/test_concurrency_handles.py +++ b/tests/test_concurrency_handles.py @@ -26,8 +26,8 @@ from agentcat import AgentCatOptions, track from agentcat.modules.constants import ( AGENTCAT_TAG_SESSION_SOURCE, - MCP_INSTRUCTIONS_KEY, - MINT_BACK_HEADER_SESSION, + MCP_SESSION_KEY, + MINT_BACK_HEADER_ISSUED, SESSION_ID_PARAM, ) @@ -107,7 +107,7 @@ async def test_simultaneous_calls_never_cross_attribute_a_handle(flavor, capture listed = await flavor.list_tools(client) echo = next(tool for tool in listed if tool.name == "echo") assert SESSION_ID_PARAM in echo.input_schema["properties"] - assert MCP_INSTRUCTIONS_KEY in (echo.output_schema or {})["properties"] + assert MCP_SESSION_KEY in (echo.output_schema or {})["properties"] results = await asyncio.wait_for( asyncio.gather( @@ -132,14 +132,14 @@ async def test_simultaneous_calls_never_cross_attribute_a_handle(flavor, capture for index, result in enumerate(results): mine = _session_id(index) assert result.text.startswith(f"echo:t{index:02d}") - assert result.structured[MCP_INSTRUCTIONS_KEY][SESSION_ID_PARAM] == mine + assert result.structured[MCP_SESSION_KEY][SESSION_ID_PARAM] == mine # Nobody else's handle is anywhere in this response. dumped = json.dumps(result.structured) + result.text others = [_session_id(other) for other in range(TOTAL) if other != index] assert not [handle for handle in others if handle in dumped] # Every handle was supplied, so nothing was minted and no mint-back # text block exists to name one. - assert MINT_BACK_HEADER_SESSION not in result.text + assert MINT_BACK_HEADER_ISSUED not in result.text # ── what AgentCat published ────────────────────────────────────────────── events = _call_events(capture) @@ -185,10 +185,10 @@ async def test_simultaneous_calls_each_mint_their_own_handle(flavor, capture): } assert len(set(published.values())) == TOTAL, "two calls minted one handle" for index, result in enumerate(results): - minted = result.structured[MCP_INSTRUCTIONS_KEY][SESSION_ID_PARAM] + minted = result.structured[MCP_SESSION_KEY][SESSION_ID_PARAM] assert minted.startswith("ses_") # The handle in the mint-back text and the one in the mirror are the # same object of trust the agent echoes back, and the event has to be # keyed on it. - assert f"session_id={minted} " in result.text + assert f"session_id: {minted}\n" in result.text assert published[f"t{index:02d}"] == minted diff --git a/tests/test_constants_copy.py b/tests/test_constants_copy.py index ebc1dca..26f6e1e 100644 --- a/tests/test_constants_copy.py +++ b/tests/test_constants_copy.py @@ -8,6 +8,7 @@ """ import ast +import re from pathlib import Path from agentcat.modules import constants as c @@ -16,16 +17,19 @@ SRC_ROOT = Path(__file__).resolve().parents[1] / "src" / "agentcat" # ── Expected bytes, copied verbatim from TS constants.ts ──────────────────── -TS_SESSION_ID_PARAM_DESCRIPTION = "REQUIRED on every call after your first. This MCP server associates all of your tool calls for a given task using session_id; a call that arrives without it cannot be associated with your earlier work and is treated as the start of an unrelated task, disconnected from the original goal. Omit it on your first call only — the server will issue one in the _mcp_instructions field of the result (or a trailing [MCP INSTRUCTIONS] text block) — then echo that exact value on every later call. Never invent a value, and do not issue parallel tool calls until the server has issued your session_id. One session_id covers the whole goal from start to finish: when you spawn subagents you MUST give them this same session_id, or their work is severed from yours. Without session_id, this server does not function as intended." # noqa: E501 -TS_AGENT_ID_PARAM_DESCRIPTION = "REQUIRED on every call, including your first. This MCP server distinguishes you from every other agent working this task using agent_id; a call that arrives without it cannot be attributed to you. Generate your own ID when you start work by joining three parts with '|': your model version, your harness name, and a short random suffix of 4-6 letters or digits chosen once. Example: opus-4.80-1m|claude-code|k3n9x. Send that same exact value on every call for your entire task; never change it mid-task. agent_id identifies you alone: a newly spawned subagent MUST generate its own instead of reusing its parent's. Without agent_id, this server does not function as intended." # noqa: E501 -TS_AGENT_ID_PARAM_DESCRIPTION_HOOK_MODE = "REQUIRED on every call, including your first. This MCP server distinguishes you from every other agent using agent_id; a call that arrives without it cannot be attributed to you. Generate your own ID when you start work by joining three parts with '|': your model version, your harness name, and a short random suffix of 4-6 letters or digits chosen once. Example: opus-4.80-1m|claude-code|k3n9x. Send that same exact value on every call for your entire task; never change it mid-task. agent_id identifies you alone: a newly spawned subagent MUST generate its own instead of reusing its parent's. Without agent_id, this server does not function as intended." # noqa: E501 -TS_MINT_BACK_HEADER_SESSION = "[MCP INSTRUCTIONS]: session_id issued." -TS_MINT_BACK_HEADER_INVALID = "[MCP INSTRUCTIONS]: session_id not recognized." -TS_MINT_BACK_CLOSER = "Without session_id, this server does not function as intended." -TS_MINT_BACK_INVALID_LINE = " The session_id you sent was not issued by this server. Re-send the exact session_id this server issued to you earlier in this conversation. Never invent a value. If this server has not issued you a session_id yet, omit the parameter and one will be issued." # noqa: E501 -TS_MCP_INSTRUCTIONS_FIELD_DESCRIPTION = "Your handles for this task, confirmed by this MCP server on every response, and the instructions for echoing them on later calls. Read and follow." # noqa: E501 -TS_MCP_INSTRUCTIONS_SESSION_ID_DESCRIPTION = "Echo this exact value as the session_id argument on every subsequent tool call." # noqa: E501 -TS_MCP_INSTRUCTIONS_AGENT_ID_DESCRIPTION = "Your agent_id as this server received it. Keep sending this exact value on every call; a subagent must generate its own." # noqa: E501 +TS_SESSION_ID_PARAM_DESCRIPTION = "Session continuity handle, one of two values: the ses_ ID issued for the task underway, or start. This server cannot link your calls between requests on its own, so session continuity travels in this parameter instead. If you were handed a session_id for this task — for example by the agent that spawned you — send that exact value from your first call. Otherwise send start on your first call; the server will issue an opaque correlation ID in the mcp_session field of the result, or in a text block at the start of the result beginning [session_id issued. Then send that exact ses_ value on every later call and hand it to any subagents working the same task. start always begins a new, unrelated task — never send it mid-task. If you send a value this server does not recognize, the result reports it: mcp_session.status of unrecognized, or a text block beginning [session_id unrecognized; re-send the ID issued for this task, or start if none was issued yet. Never invent a ses_ value." # noqa: E501 +TS_SESSION_ID_PARAM_PATTERN = "^(start|ses_[0-9A-Za-z]{27})$" +TS_SESSION_START_SENTINEL = "start" +TS_AGENT_ID_PARAM_DESCRIPTION = "Agent identity handle, required on every call including your first. This server cannot tell concurrent agents apart on its own; agent_id is how your calls are attributed to you. It is a self-chosen identifier in the spirit of a User-Agent string — join your model version, your harness name, and a short suffix of 4-6 letters or digits, with '|'. Example: opus-4.80-1m|claude-code|k3n9x. Choose the suffix once at the start of your task and send that same exact value on every call for the entire task; never change it mid-task, and a new task gets a fresh suffix. agent_id identifies exactly one agent and is never inherited: a subagent you spawn generates a new one rather than carrying yours, and if you were spawned by another agent, generate your own rather than reusing your parent's. A call without agent_id cannot be attributed to you." # noqa: E501 +TS_MINT_BACK_HEADER_ISSUED = "[session_id issued — see this tool's session_id parameter description]" # noqa: E501 +TS_MINT_BACK_ISSUED_BODY = "This is the first-call issuance described in this tool's session_id parameter description." # noqa: E501 +TS_MINT_BACK_HEADER_UNRECOGNIZED = "[session_id unrecognized — see this tool's session_id parameter description]" # noqa: E501 +TS_MINT_BACK_UNRECOGNIZED_BODY = "The value sent was not issued by this server. Re-send the session_id issued earlier for this task; if none was issued yet, send start and one will be issued." # noqa: E501 +TS_MCP_SESSION_FIELD_DESCRIPTION = "Session continuity and agent attribution state for this task, returned on completed responses that carry structured output. This server cannot link your calls between requests on its own, so session continuity travels here instead." # noqa: E501 +TS_MCP_SESSION_FIELD_DESCRIPTION_HOOK_MODE = "Agent attribution state for this task, returned on completed responses that carry structured output." # noqa: E501 +TS_MCP_SESSION_SESSION_ID_DESCRIPTION = "Opaque correlation ID for this task, issued by this server. Use this as the session_id argument of every later call, and hand it to any subagents working the same task. Absent when status is unrecognized; no replacement is issued in that response — recovery is described under status." # noqa: E501 +TS_MCP_SESSION_AGENT_ID_DESCRIPTION = "Present only when you sent agent_id on this call. Your agent_id, echoed as received. Continue sending this exact value on every call; it is never inherited — a subagent you spawn generates its own." # noqa: E501 +TS_MCP_SESSION_STATUS_DESCRIPTION = "issued: first call of a task; the session_id above was just created. active: the session_id you sent was accepted; keep sending it. unrecognized: the value sent was not issued by this server — re-send the one issued earlier for this task; if none was issued yet, send start to be issued a new one." # noqa: E501 TS_DEFAULT_CONTEXT_PARAMETER_DESCRIPTION = 'Explain why you are calling this tool and how it fits into the user\'s overall goal. This parameter is used for analytics and user intent tracking. YOU MUST provide 15-25 words (count carefully). NEVER use first person (\'I\', \'we\', \'you\') - maintain third-person perspective. NEVER include sensitive information such as credentials, passwords, or personal data. Example (20 words): "Searching across the organization\'s repositories to find all open issues related to performance complaints and latency issues for team prioritization."' # noqa: E501 # ── Expected bytes, copied verbatim from TS tools.ts ──────────────────────── @@ -39,7 +43,7 @@ def test_param_names_and_keys(): assert c.AGENT_ID_PARAM == "agent_id" assert c.CONTEXT_PARAM == "context" assert c.GET_MORE_TOOLS_NAME == "get_more_tools" - assert c.MCP_INSTRUCTIONS_KEY == "_mcp_instructions" + assert c.MCP_SESSION_KEY == "mcp_session" assert c.META_CLIENT_INFO_KEY == "io.modelcontextprotocol/clientInfo" assert c.META_PROTOCOL_VERSION_KEY == "io.modelcontextprotocol/protocolVersion" assert c.AGENTCAT_TAG_SESSION_SOURCE == "agentcat_session_id_source" @@ -52,86 +56,92 @@ def test_param_names_and_keys(): def test_mint_back_assembly(): - assert c.MINT_BACK_HEADER_SESSION == TS_MINT_BACK_HEADER_SESSION - assert c.MINT_BACK_HEADER_SESSION == "[MCP INSTRUCTIONS]: session_id issued." - assert c.MINT_BACK_CLOSER == TS_MINT_BACK_CLOSER - assert ( - c.MINT_BACK_CLOSER - == "Without session_id, this server does not function as intended." - ) - assert ( - c.mint_back_session_line("ses_X") - == " session_id=ses_X — required on every subsequent tool call" - ) - assert c.mint_back_confirmed(["session_id"]) == ( - "[MCP INSTRUCTIONS]: session_id confirmed. " - "Keep sending this exact value on every call." - ) - assert c.mint_back_confirmed(["session_id", "agent_id"]) == ( - "[MCP INSTRUCTIONS]: session_id and agent_id confirmed. " - "Keep sending these exact values on every call." - ) - assert c.mint_back_confirmed(["agent_id"]) == ( - "[MCP INSTRUCTIONS]: agent_id confirmed. " - "Keep sending this exact value on every call." - ) + assert c.MINT_BACK_HEADER_ISSUED == TS_MINT_BACK_HEADER_ISSUED + assert c.MINT_BACK_ISSUED_BODY == TS_MINT_BACK_ISSUED_BODY + assert c.mint_back_session_line("ses_X") == "session_id: ses_X" + + +def test_headers_match_the_param_description_promise(): + """The param description promises a text block "beginning [session_id + issued" / "beginning [session_id unrecognized"; the headers must keep + those prefixes or the promise breaks.""" + assert c.MINT_BACK_HEADER_ISSUED.startswith("[session_id issued") + assert c.MINT_BACK_HEADER_UNRECOGNIZED.startswith("[session_id unrecognized") + assert "[session_id issued" in c.SESSION_ID_PARAM_DESCRIPTION + assert "[session_id unrecognized" in c.SESSION_ID_PARAM_DESCRIPTION -def test_invalid_correction_copy_matches_ts(): - """The `invalid` branch corrects the agent without issuing a replacement. +def test_unrecognized_correction_copy_matches_ts(): + """The `unrecognized` branch corrects the agent without issuing a + replacement. - The closing sentence is load-bearing: an agent that hallucinated a - session_id on its FIRST call was never issued one, so "re-send what you - were given" names a value that does not exist. Omitting the parameter puts - it back on the `minted` path. + The closing sentence is load-bearing: an agent that invented a session_id + on its FIRST call was never issued one, so "re-send what you were given" + names a value that does not exist. Sending `start` puts it back on the + `minted` path. """ - assert c.MINT_BACK_HEADER_INVALID == TS_MINT_BACK_HEADER_INVALID - assert c.MINT_BACK_INVALID_LINE == TS_MINT_BACK_INVALID_LINE - assert c.MINT_BACK_INVALID_LINE.endswith( - "If this server has not issued you a session_id yet, omit the parameter " - "and one will be issued." + assert c.MINT_BACK_HEADER_UNRECOGNIZED == TS_MINT_BACK_HEADER_UNRECOGNIZED + assert c.MINT_BACK_UNRECOGNIZED_BODY == TS_MINT_BACK_UNRECOGNIZED_BODY + assert c.MINT_BACK_UNRECOGNIZED_BODY.endswith( + "send start and one will be issued." ) # No value is handed out anywhere in the correction. - assert "ses_" not in c.MINT_BACK_HEADER_INVALID + c.MINT_BACK_INVALID_LINE + correction = c.MINT_BACK_HEADER_UNRECOGNIZED + c.MINT_BACK_UNRECOGNIZED_BODY + assert "ses_" not in correction -def test_session_id_param_description_matches_ts(): +def test_param_descriptions_match_ts(): assert c.SESSION_ID_PARAM_DESCRIPTION == TS_SESSION_ID_PARAM_DESCRIPTION assert c.SESSION_ID_PARAM_DESCRIPTION.startswith( - "REQUIRED on every call after your first." + "Session continuity handle, one of two values:" ) - assert c.SESSION_ID_PARAM_DESCRIPTION.endswith( - "Without session_id, this server does not function as intended." + assert c.SESSION_ID_PARAM_DESCRIPTION.endswith("Never invent a ses_ value.") + assert c.AGENT_ID_PARAM_DESCRIPTION == TS_AGENT_ID_PARAM_DESCRIPTION + assert c.AGENT_ID_PARAM_DESCRIPTION.startswith("Agent identity handle,") + assert c.AGENT_ID_PARAM_DESCRIPTION.endswith( + "A call without agent_id cannot be attributed to you." ) - assert "session_id and agent_id" not in c.SESSION_ID_PARAM_DESCRIPTION -def test_agent_id_param_descriptions_match_ts(): - assert c.AGENT_ID_PARAM_DESCRIPTION == TS_AGENT_ID_PARAM_DESCRIPTION - assert ( - c.AGENT_ID_PARAM_DESCRIPTION_HOOK_MODE - == TS_AGENT_ID_PARAM_DESCRIPTION_HOOK_MODE - ) - assert "working this task" in c.AGENT_ID_PARAM_DESCRIPTION - assert "working this task" not in c.AGENT_ID_PARAM_DESCRIPTION_HOOK_MODE - assert ( - c.AGENT_ID_PARAM_DESCRIPTION.replace(" working this task", "") - == c.AGENT_ID_PARAM_DESCRIPTION_HOOK_MODE - ) +def test_session_value_contract_constants_match_ts(): + """The v4 value contract: session_id is `start` or an ID this SDK issued. + + The schema pattern must equal the issued-ID shape with the `start` + alternative added — never looser — and the sentinel must be the exact + spelling the parameter description and the mint-back bodies tell agents + to send. The pattern itself is strict lowercase `start`; the lenient + case-insensitive reading lives in resolution, not in the schema. + """ + assert c.SESSION_ID_PARAM_PATTERN == TS_SESSION_ID_PARAM_PATTERN + assert c.SESSION_START_SENTINEL == TS_SESSION_START_SENTINEL + accepted = re.compile(c.SESSION_ID_PARAM_PATTERN) + assert accepted.fullmatch(c.SESSION_START_SENTINEL) + assert accepted.fullmatch("ses_2cOHEO0LYGADMzRvWTXXVbbgxgm") + for rejected in ("Start", " start", "ses_" + "a" * 26, "ses_" + "a" * 28, ""): + assert not accepted.fullmatch(rejected), rejected + # The copy that tells agents what to send names both alternatives. + assert ", or start." in c.SESSION_ID_PARAM_DESCRIPTION + assert "send start on your first call" in c.SESSION_ID_PARAM_DESCRIPTION + assert "send start and one will be issued." in c.MINT_BACK_UNRECOGNIZED_BODY + assert "send start to be issued a new one." in c.MCP_SESSION_STATUS_DESCRIPTION -def test_mcp_instructions_descriptions_match_ts(): - assert ( - c.MCP_INSTRUCTIONS_FIELD_DESCRIPTION == TS_MCP_INSTRUCTIONS_FIELD_DESCRIPTION - ) + + +def test_mcp_session_descriptions_match_ts(): + assert c.MCP_SESSION_FIELD_DESCRIPTION == TS_MCP_SESSION_FIELD_DESCRIPTION assert ( - c.MCP_INSTRUCTIONS_SESSION_ID_DESCRIPTION - == TS_MCP_INSTRUCTIONS_SESSION_ID_DESCRIPTION + c.MCP_SESSION_FIELD_DESCRIPTION_HOOK_MODE + == TS_MCP_SESSION_FIELD_DESCRIPTION_HOOK_MODE ) assert ( - c.MCP_INSTRUCTIONS_AGENT_ID_DESCRIPTION - == TS_MCP_INSTRUCTIONS_AGENT_ID_DESCRIPTION + c.MCP_SESSION_SESSION_ID_DESCRIPTION == TS_MCP_SESSION_SESSION_ID_DESCRIPTION ) + assert c.MCP_SESSION_AGENT_ID_DESCRIPTION == TS_MCP_SESSION_AGENT_ID_DESCRIPTION + assert c.MCP_SESSION_STATUS_DESCRIPTION == TS_MCP_SESSION_STATUS_DESCRIPTION + # Every response state is pre-announced in the schema copy. + for state in ("issued", "active", "unrecognized"): + assert state in c.MCP_SESSION_STATUS_DESCRIPTION def test_existing_context_description_unchanged(): diff --git a/tests/test_customer_owned_parameters.py b/tests/test_customer_owned_parameters.py index 20705d6..9a836ea 100644 --- a/tests/test_customer_owned_parameters.py +++ b/tests/test_customer_owned_parameters.py @@ -35,7 +35,7 @@ from agentcat import AgentCatOptions, track from agentcat.modules.constants import ( AGENTCAT_TAG_SESSION_SOURCE, - MCP_INSTRUCTIONS_KEY, + MCP_SESSION_KEY, SESSION_ID_PARAM, SESSION_ID_PARAM_DESCRIPTION, ) @@ -96,10 +96,10 @@ async def test_a_customers_own_session_id_is_never_the_handle(flavor, capture): # customer's tool does — nor confirms their value back to them. # (Their own tool body still echoes TASK-1234 in its result — that is their # data. What must not appear is an AgentCat-authored block naming it, which - # is what the absence of MCP_INSTRUCTIONS_KEY asserts.) + # is what the absence of MCP_SESSION_KEY asserts.) for result in (first, second): - assert "[MCP INSTRUCTIONS]" not in result.text - assert MCP_INSTRUCTIONS_KEY not in (result.structured or {}) + assert "[session_id" not in result.text + assert MCP_SESSION_KEY not in (result.structured or {}) @pytest.mark.parametrize("flavor", flavors(), ids=lambda f: f.id) @@ -148,7 +148,7 @@ async def test_agent_id_is_still_confirmed_on_a_colliding_tool(flavor, capture): {"session_id": "TASK-1234", "note": "n", "agent_id": "opus|cc|k3n9x"}, ) - mint = (result.structured or {}).get(MCP_INSTRUCTIONS_KEY) + mint = (result.structured or {}).get(MCP_SESSION_KEY) assert mint is not None, "agent_id was withheld because session_id collided" assert mint["agent_id"] == "opus|cc|k3n9x" assert SESSION_ID_PARAM not in mint diff --git a/tests/test_dynamic_tracking.py b/tests/test_dynamic_tracking.py index 392c37b..1cf1e5f 100644 --- a/tests/test_dynamic_tracking.py +++ b/tests/test_dynamic_tracking.py @@ -202,8 +202,13 @@ async def call_tool(name: str, arguments: dict) -> list[Any]: "agent_id", "context", ] - # session_id is the one injected param that is never required. - assert tool.inputSchema["required"] == ["agent_id", "context"] + # Every injected param is required — session_id included, with + # `start` as its explicit first-call value. + assert tool.inputSchema["required"] == [ + "session_id", + "agent_id", + "context", + ] # The handler above rejects anything but `value`, so this call # only succeeds if both injected parameters were stripped. diff --git a/tests/test_handles.py b/tests/test_handles.py index 75738d5..57b8936 100644 --- a/tests/test_handles.py +++ b/tests/test_handles.py @@ -7,6 +7,10 @@ never the literal. """ +from agentcat.modules.constants import ( + MINT_BACK_HEADER_ISSUED, + MINT_BACK_ISSUED_BODY, +) from agentcat.modules.handles import ( HandleResolution, build_handle_tags, @@ -80,6 +84,53 @@ async def test_prompted_supplied_vs_minted(): assert r2.session_source == "minted" and r2.session_id.startswith("ses_") +async def test_the_start_sentinel_resolves_exactly_like_omission(): + """`start` is the explicit spelling of "begin a new task". + + It resolves on the minted path with the minted announcement — session + source, status, and text block all identical to an omitted session_id — + and it is read off the trimmed value case-insensitively, checked BEFORE + shape validation, so it can never fall through to `invalid`. + """ + o = AgentCatOptions() + for value in ("start", "Start", "START", " start ", "\tSTART \n"): + r = await resolve_handles({"session_id": value}, o, "proj", None, None) + assert r.session_source == "minted", value + assert r.session_id.startswith("ses_") and r.prompts_session_id is True + text = build_mint_back_text(r) + assert text is not None and text.startswith(MINT_BACK_HEADER_ISSUED) + assert build_structured_mint_back(r) == { + "session_id": r.session_id, + "status": "issued", + } + + +async def test_each_start_begins_a_new_unrelated_task(): + # The copy warns "start always begins a new, unrelated task" — so two + # sentinel calls must never share a session. + o = AgentCatOptions() + a = await resolve_handles({"session_id": "start"}, o, "proj", None, None) + b = await resolve_handles({"session_id": "start"}, o, "proj", None, None) + assert a.session_id != b.session_id + + +async def test_start_in_a_foreign_param_is_never_a_sentinel(): + # The sentinel is only ever read from OUR parameter. A customer tool whose + # own schema declares session_id keeps every value — `start` included — + # and stays sessionless, exactly like any other foreign value. + r = await resolve_handles( + {"session_id": "start"}, + AgentCatOptions(), + "proj", + None, + None, + frozenset(), + session_param_is_ours=False, + ) + assert (r.session_source, r.session_id) == ("foreign", "") + assert build_mint_back_text(r) is None + + # TS handles.test.ts:248-271 — the agent is never minted server-side, so an # omitted agent_id stays unresolved in both the minted and the supplied # (subagent-continuation) flows. @@ -276,7 +327,9 @@ async def test_the_gate_is_per_handle(): # customer's own value back to the agent. mint = build_structured_mint_back(r) assert mint is not None and "session_id" not in mint - assert mint["agent_id"] == "agt|x|1" + # `status` is prompted-mode-only vocabulary; a foreign session param + # mirrors the agent handle alone. + assert mint == {"agent_id": "agt|x|1"} assert "SESSION-1234" not in str(mint) @@ -333,9 +386,7 @@ async def test_agent_extraction_gated_on_option(): def test_mint_back_text_rules(): minted = HandleResolution(session_id=sid("T"), session_source="minted") assert build_mint_back_text(minted) == ( - "[MCP INSTRUCTIONS]: session_id issued.\n" - f" session_id={sid('T')} — required on every subsequent tool call\n" - "Without session_id, this server does not function as intended." + f"{MINT_BACK_HEADER_ISSUED}\nsession_id: {sid('T')}\n{MINT_BACK_ISSUED_BODY}" ) assert build_mint_back_text(HandleResolution(sid("T"), "supplied")) is None assert build_mint_back_text(HandleResolution(sid("T"), "minted", hook_mode=True)) is None # noqa: E501 @@ -355,55 +406,53 @@ def test_mint_back_text_ignores_a_supplied_agent(): def test_structured_mint_back_omission_rules(): both = HandleResolution(sid("T"), "supplied", agent_id="A", agent_source="supplied") m = build_structured_mint_back(both) - assert m == { - "session_id": sid("T"), - "agent_id": "A", - "instructions": "[MCP INSTRUCTIONS]: session_id and agent_id confirmed. Keep sending these exact values on every call.", # noqa: E501 - } + assert m == {"session_id": sid("T"), "agent_id": "A", "status": "active"} hook_agent = HandleResolution( sid("T"), "hook", agent_id="A", agent_source="supplied", hook_mode=True ) + # Hook mode carries the agent handle alone — no session_id, no status. m = build_structured_mint_back(hook_agent) - assert "session_id" not in m and m["agent_id"] == "A" and "agent_id confirmed" in m["instructions"] # noqa: E501 + assert m == {"agent_id": "A"} assert build_structured_mint_back(HandleResolution(sid("T"), "hook", hook_mode=True)) is None # noqa: E501 minted = HandleResolution(sid("T"), "minted") - assert build_structured_mint_back(minted)["instructions"].startswith("[MCP INSTRUCTIONS]: session_id issued.") # noqa: E501 + assert build_structured_mint_back(minted) == { + "session_id": sid("T"), + "status": "issued", + } # TS handles.test.ts:428-441 — a minted task with a supplied agent echoes both -# ids but keeps the issued (not confirmed) copy, and never claims to have -# issued an agent_id. +# ids; `status` reports the session only, and never claims an agent_id was +# issued (the agent's value is echoed as received). def test_structured_mint_back_minted_task_with_supplied_agent(): m = build_structured_mint_back( HandleResolution(sid("T"), "minted", agent_id=A, agent_source="supplied") ) - assert m["session_id"] == sid("T") and m["agent_id"] == A - assert "session_id issued" in m["instructions"] - assert "agent_id issued" not in m["instructions"] + assert m == {"session_id": sid("T"), "agent_id": A, "status": "issued"} -# TS handles.test.ts:458-469 — agent tracking off: task only, singular copy. -def test_structured_mint_back_task_only_uses_singular_confirmed_copy(): +# TS handles.test.ts:458-469 — agent tracking off: task only. +def test_structured_mint_back_task_only(): m = build_structured_mint_back(HandleResolution(sid("T"), "supplied")) - assert m == { - "session_id": sid("T"), - "instructions": "[MCP INSTRUCTIONS]: session_id confirmed. Keep sending this exact value on every call.", # noqa: E501 - } + assert m == {"session_id": sid("T"), "status": "active"} def test_mirror_rules(): - mint = {"session_id": sid("T"), "instructions": "i"} + mint = {"session_id": sid("T"), "status": "issued"} assert mirror_into_structured_content(None, mint) is None assert mirror_into_structured_content([1], mint) is None - assert mirror_into_structured_content({"_mcp_instructions": "customer"}, mint) is None # noqa: E501 + assert mirror_into_structured_content({"mcp_session": "customer"}, mint) is None out = mirror_into_structured_content({"a": 1}, mint) - assert out == {"a": 1, "_mcp_instructions": mint} + assert out == {"a": 1, "mcp_session": mint} + # Mirror FIRST: an ID at the tail of a long payload is what clients + # truncate away. + assert list(out) == ["mcp_session", "a"] # TS handles.test.ts:498-516 — customer objects are never mutated, and any # non-plain-object structured content is left alone. def test_mirror_never_mutates_and_skips_non_mappings(): - mint = {"session_id": sid("T"), "instructions": "i"} + mint = {"session_id": sid("T"), "status": "issued"} sc = {"a": 1} out = mirror_into_structured_content(sc, mint) assert sc == {"a": 1} and out is not sc diff --git a/tests/test_injection.py b/tests/test_injection.py index 52d681a..b4de9e0 100644 --- a/tests/test_injection.py +++ b/tests/test_injection.py @@ -16,7 +16,7 @@ ToolSpec, build_injected_schemas, injected_parameter_names, - mcp_instructions_schema_property, + mcp_session_schema_property, strip_injected_arguments, ) from agentcat.types import AgentCatOptions @@ -43,12 +43,16 @@ def test_param_order_and_descriptions(): props = s.input_schema["properties"] assert props["session_id"]["description"] == c.SESSION_ID_PARAM_DESCRIPTION assert props["agent_id"]["description"] == c.AGENT_ID_PARAM_DESCRIPTION - assert "session_id" not in s.input_schema.get("required", []) - assert "agent_id" in s.input_schema["required"] + # session_id declares the start|ses_ value contract; agent_id is + # free-form by design and carries no pattern in any mode. + assert props["session_id"]["pattern"] == c.SESSION_ID_PARAM_PATTERN + assert "pattern" not in props["agent_id"] + assert "pattern" not in props["context"] + assert s.input_schema["required"] == ["session_id", "agent_id", "context"] assert r.injected_params["t"] == {"session_id", "agent_id", "context"} -def test_hook_mode_omits_session_id_and_switches_agent_copy(): +def test_hook_mode_omits_session_id_and_keeps_the_single_agent_copy(): s = spec() build_injected_schemas( [s], @@ -58,7 +62,11 @@ def test_hook_mode_omits_session_id_and_switches_agent_copy(): ) props = s.input_schema["properties"] assert "session_id" not in props - assert props["agent_id"]["description"] == c.AGENT_ID_PARAM_DESCRIPTION_HOOK_MODE + # One agent_id description in both modes. + assert props["agent_id"]["description"] == c.AGENT_ID_PARAM_DESCRIPTION + # Requiredness rides injection: no session_id is injected in hook mode, + # so none is required; agent_id is injected, so it is. + assert s.input_schema["required"] == ["agent_id", "context"] def test_tracing_disabled_skips_handles_but_not_context(): @@ -85,6 +93,10 @@ def test_collision_skips_that_param_only(): s = spec(props={"session_id": {"type": "string", "description": "customer"}}) r = build_injected_schemas([s], AgentCatOptions(enable_agent_tracking=True)) assert s.input_schema["properties"]["session_id"]["description"] == "customer" + # The customer's parameter is untouched in every respect: no redescribe, + # no pattern, and no requiredness it never asked for. + assert "pattern" not in s.input_schema["properties"]["session_id"] + assert s.input_schema["required"] == ["agent_id", "context"] assert r.injected_params["t"] == {"agent_id", "context"} @@ -92,19 +104,20 @@ def test_get_more_tools_gets_handles_but_not_context(): s = spec(name=c.GET_MORE_TOOLS_NAME, props={"context": {"type": "string"}}) r = build_injected_schemas([s], AgentCatOptions()) assert r.injected_params[c.GET_MORE_TOOLS_NAME] == {"session_id"} + assert s.input_schema["required"] == ["session_id"] def test_output_schema_extension_and_registry(): s = spec(out={"type": "object", "properties": {"answer": {"type": "string"}}}) r = build_injected_schemas([s], AgentCatOptions()) - prop = s.output_schema["properties"][c.MCP_INSTRUCTIONS_KEY] - assert prop["description"] == c.MCP_INSTRUCTIONS_FIELD_DESCRIPTION + prop = s.output_schema["properties"][c.MCP_SESSION_KEY] + assert prop["description"] == c.MCP_SESSION_FIELD_DESCRIPTION task_prop = prop["properties"]["session_id"] - assert task_prop["description"] == c.MCP_INSTRUCTIONS_SESSION_ID_DESCRIPTION + assert task_prop["description"] == c.MCP_SESSION_SESSION_ID_DESCRIPTION # Default options are prompted mode with agent tracking off, so the copy # never mentions an agent_id the agent was not asked for. - assert list(prop["properties"]) == ["session_id", "instructions"] - assert c.MCP_INSTRUCTIONS_KEY not in s.output_schema.get("required", []) + assert list(prop["properties"]) == ["session_id", "status"] + assert c.MCP_SESSION_KEY not in s.output_schema.get("required", []) assert r.output_injected == {"t"} s2 = spec(out={"oneOf": []}) r2 = build_injected_schemas([s2], AgentCatOptions()) @@ -153,41 +166,62 @@ def test_strip_registry_driven_and_heuristic(): # ── Ported from the TS SDK: required-array semantics ───────────────────────── -# src/tests/handle-injection.test.ts — "creates the required array when the -# schema has none", "does not duplicate agent_id in an existing required -# array", "leaves required untouched when the customer declares agent_id". +# src/tests/handle-injection.test.ts — the required array is created when the +# schema has none, injected names are appended without duplication, and a +# customer-declared handle is never required by us. Requiredness rides +# injection exactly: every injected param joins required, and only those. -def test_required_list_preserved_when_appending_agent_id(): +def test_required_list_preserved_when_appending_handles(): s = spec(props={"text": {"type": "string"}}, extra={"required": ["text"]}) build_injected_schemas([s], AgentCatOptions(enable_agent_tracking=True)) - assert s.input_schema["required"] == ["text", "agent_id", "context"] + # Customer entries stay first and in order; ours append after them. + assert s.input_schema["required"] == [ + "text", + "session_id", + "agent_id", + "context", + ] def test_required_is_created_for_the_params_that_need_it(): with_agent = spec() build_injected_schemas([with_agent], AgentCatOptions(enable_agent_tracking=True)) - assert with_agent.input_schema["required"] == ["agent_id", "context"] - - # session_id is the one injected param that is never required — omitting it - # is how an agent asks to be minted one — but context is, so a schema that - # declared no required array does grow one. + assert with_agent.input_schema["required"] == [ + "session_id", + "agent_id", + "context", + ] + + # Every injected param is required — session_id included, with `start` as + # its explicit first-call value — so a schema that declared no required + # array grows one. without_agent = spec() build_injected_schemas([without_agent], AgentCatOptions()) - assert without_agent.input_schema["required"] == ["context"] + assert without_agent.input_schema["required"] == ["session_id", "context"] - # ...and with the context pass off there is nothing to require at all. + # ...the handle pass requires its own even with the context pass off... + handles_only = spec() + build_injected_schemas( + [handles_only], AgentCatOptions(enable_tool_call_context=False) + ) + assert handles_only.input_schema["required"] == ["session_id"] + + # ...and with nothing injected there is nothing to require at all. neither = spec() - build_injected_schemas([neither], AgentCatOptions(enable_tool_call_context=False)) + build_injected_schemas( + [neither], + AgentCatOptions(enable_tracing=False, enable_tool_call_context=False), + ) assert "required" not in neither.input_schema -def test_agent_id_not_duplicated_in_existing_required_array(): +def test_injected_names_not_duplicated_in_existing_required_array(): # A schema can list a name in required without declaring the property; - # injection must not push a second copy. - s = spec(props={}, extra={"required": ["agent_id"]}) + # injection must not push a second copy of either handle. + s = spec(props={}, extra={"required": ["session_id", "agent_id"]}) build_injected_schemas([s], AgentCatOptions(enable_agent_tracking=True)) - assert s.input_schema["required"] == ["agent_id", "context"] + assert s.input_schema["required"] == ["session_id", "agent_id", "context"] def test_customer_declared_agent_id_leaves_required_untouched(): @@ -197,8 +231,8 @@ def test_customer_declared_agent_id_leaves_required_untouched(): ) r = build_injected_schemas([s], AgentCatOptions(enable_agent_tracking=True)) # agent_id is theirs, so it is neither redescribed nor required by us. - # context is still ours, and still required. - assert s.input_schema["required"] == ["context"] + # session_id and context are still ours, and still required. + assert s.input_schema["required"] == ["session_id", "context"] assert s.input_schema["properties"]["agent_id"]["description"] == "mine" assert "agent_id" not in r.injected_params["t"] @@ -226,7 +260,7 @@ def test_additional_properties_true_and_dict_forms_untouched(): # ── Ported: deep-copy isolation ────────────────────────────────────────────── # The adapter hands the pipeline deep copies and the pipeline mutates them in # place, so nested customer structures must come out byte-identical and the -# minted _mcp_instructions fragment must never be shared between tools. +# minted mcp_session fragment must never be shared between tools. def test_nested_customer_schema_is_not_mutated(): @@ -253,38 +287,45 @@ def test_pipeline_mutates_in_place_rather_than_replacing_schemas(): assert s.input_schema is schema_ref -def test_mcp_instructions_fragment_is_not_shared_between_tools(): +def test_mcp_session_fragment_is_not_shared_between_tools(): a = spec(name="a", out={"type": "object", "properties": {}}) b = spec(name="b", out={"type": "object", "properties": {}}) build_injected_schemas([a, b], AgentCatOptions()) - frag_a = a.output_schema["properties"][c.MCP_INSTRUCTIONS_KEY] - frag_b = b.output_schema["properties"][c.MCP_INSTRUCTIONS_KEY] + frag_a = a.output_schema["properties"][c.MCP_SESSION_KEY] + frag_b = b.output_schema["properties"][c.MCP_SESSION_KEY] assert frag_a == frag_b frag_a["properties"]["session_id"]["description"] = "mutated" assert frag_b["properties"]["session_id"]["description"] != "mutated" -def test_mcp_instructions_schema_property_shape(): - frag = mcp_instructions_schema_property(True, True) +def test_mcp_session_schema_property_shape(): + frag = mcp_session_schema_property(True, True) assert frag["type"] == "object" - assert frag["description"] == c.MCP_INSTRUCTIONS_FIELD_DESCRIPTION - assert list(frag["properties"]) == ["session_id", "agent_id", "instructions"] + assert frag["description"] == c.MCP_SESSION_FIELD_DESCRIPTION + assert list(frag["properties"]) == ["session_id", "agent_id", "status"] task_prop = frag["properties"]["session_id"] - assert task_prop["description"] == c.MCP_INSTRUCTIONS_SESSION_ID_DESCRIPTION + assert task_prop["description"] == c.MCP_SESSION_SESSION_ID_DESCRIPTION agent_prop = frag["properties"]["agent_id"] - assert agent_prop["description"] == c.MCP_INSTRUCTIONS_AGENT_ID_DESCRIPTION - assert frag["properties"]["instructions"] == {"type": "string"} - assert mcp_instructions_schema_property(True, True) is not frag + assert agent_prop["description"] == c.MCP_SESSION_AGENT_ID_DESCRIPTION + assert frag["properties"]["status"] == { + "type": "string", + "enum": ["issued", "active", "unrecognized"], + "description": c.MCP_SESSION_STATUS_DESCRIPTION, + } + assert mcp_session_schema_property(True, True) is not frag -def test_mcp_instructions_schema_property_tracks_the_flags(): +def test_mcp_session_schema_property_tracks_the_flags(): # The copy never references a parameter the agent cannot see, so each - # sub-property is gated by the handle that produced it. `instructions` is - # unconditional. - prompted_only = mcp_instructions_schema_property(True, False) - assert list(prompted_only["properties"]) == ["session_id", "instructions"] - hook_only = mcp_instructions_schema_property(False, True) - assert list(hook_only["properties"]) == ["agent_id", "instructions"] + # sub-property is gated by the handle that produced it — session_id and + # status are prompted-mode-only, and hook mode carries its own field + # description. + prompted_only = mcp_session_schema_property(True, False) + assert list(prompted_only["properties"]) == ["session_id", "status"] + assert prompted_only["description"] == c.MCP_SESSION_FIELD_DESCRIPTION + hook_only = mcp_session_schema_property(False, True) + assert list(hook_only["properties"]) == ["agent_id"] + assert hook_only["description"] == c.MCP_SESSION_FIELD_DESCRIPTION_HOOK_MODE # ── Ported: registry completeness & injection order ────────────────────────── @@ -359,16 +400,16 @@ def test_output_schema_contract_otherwise_untouched(): assert s.output_schema["properties"]["count"] == {"type": "number"} -def test_customer_declared_mcp_instructions_output_never_clobbered(): +def test_customer_declared_mcp_session_output_never_clobbered(): out = { "type": "object", "properties": { - c.MCP_INSTRUCTIONS_KEY: {"type": "string", "description": "customer-owned"} + c.MCP_SESSION_KEY: {"type": "string", "description": "customer-owned"} }, } s = spec(out=out) r = build_injected_schemas([s], AgentCatOptions()) - prop = s.output_schema["properties"][c.MCP_INSTRUCTIONS_KEY] + prop = s.output_schema["properties"][c.MCP_SESSION_KEY] assert prop == {"type": "string", "description": "customer-owned"} assert r.output_injected == set() @@ -385,7 +426,7 @@ def test_output_schema_without_properties_bag_gets_one_created(): # the pipeline mints `properties` rather than skipping the tool. s = spec(out={"type": "object"}) r = build_injected_schemas([s], AgentCatOptions()) - assert list(s.output_schema["properties"]) == [c.MCP_INSTRUCTIONS_KEY] + assert list(s.output_schema["properties"]) == [c.MCP_SESSION_KEY] assert r.output_injected == {"t"} @@ -397,7 +438,7 @@ def test_composed_input_schema_skips_output_injection_too(): output_schema=out, ) r = build_injected_schemas([s], AgentCatOptions()) - assert c.MCP_INSTRUCTIONS_KEY not in s.output_schema["properties"] + assert c.MCP_SESSION_KEY not in s.output_schema["properties"] assert r.output_injected == set() assert r.injected_params["mixed"] == set() @@ -452,16 +493,17 @@ def test_hook_mode_with_agent_tracking_extends_output_without_session_id(): enable_agent_tracking=True, resolve_session_id=lambda q, e: "x" ), ) - frag = s.output_schema["properties"][c.MCP_INSTRUCTIONS_KEY] - assert list(frag["properties"]) == ["agent_id", "instructions"] + frag = s.output_schema["properties"][c.MCP_SESSION_KEY] + assert list(frag["properties"]) == ["agent_id"] + assert frag["description"] == c.MCP_SESSION_FIELD_DESCRIPTION_HOOK_MODE assert r.output_injected == {"t"} def test_prompted_mode_with_agent_tracking_extends_output_with_both(): s = spec(out=_out()) build_injected_schemas([s], AgentCatOptions(enable_agent_tracking=True)) - frag = s.output_schema["properties"][c.MCP_INSTRUCTIONS_KEY] - assert list(frag["properties"]) == ["session_id", "agent_id", "instructions"] + frag = s.output_schema["properties"][c.MCP_SESSION_KEY] + assert list(frag["properties"]) == ["session_id", "agent_id", "status"] # ── Input-schema normalization ─────────────────────────────────────────────── @@ -474,7 +516,7 @@ def test_empty_input_schema_is_normalized_before_injection(): build_injected_schemas([s], AgentCatOptions()) assert s.input_schema["type"] == "object" assert list(s.input_schema["properties"]) == ["session_id", "context"] - assert s.input_schema["required"] == ["context"] + assert s.input_schema["required"] == ["session_id", "context"] def test_input_schema_without_properties_bag_gets_one_created(): @@ -523,6 +565,14 @@ def test_injected_names_is_the_single_source_the_strip_reads(): assert injected_parameter_names( "t", None, {"session_id": sid("mine")} ) == frozenset({"session_id", "context"}) + # So does the `start` sentinel our copy tells agents to send on a first + # call — read the way resolution reads it: case-insensitive, trimmed. + assert injected_parameter_names( + "t", None, {"session_id": "start"} + ) == frozenset({"session_id", "context"}) + assert injected_parameter_names( + "t", None, {"session_id": " START "} + ) == frozenset({"session_id", "context"}) # A resolve_session_id hook never injects session_id, so it never strips it. assert injected_parameter_names( "t", None, options=AgentCatOptions(resolve_session_id=lambda req, extra: "x") @@ -569,6 +619,10 @@ def test_strip_heuristic_leaves_unrelated_arguments_alone(): "text": "x", "id": 7, } + # The `start` sentinel is stripped like a minted-shape value: it is our + # parameter's first-call spelling, never a customer argument. + started = {"text": "x", "session_id": "Start", "context": "c"} + assert strip_injected_arguments("any", started, None) == {"text": "x"} def test_injection_result_equality_is_value_based(): diff --git a/tests/test_inner_tap.py b/tests/test_inner_tap.py index dd67cbf..1a70ce9 100644 --- a/tests/test_inner_tap.py +++ b/tests/test_inner_tap.py @@ -546,9 +546,9 @@ async def call(server): assert tracked.isError == untracked.isError is True # The SDK's own error block, byte for byte. (A tracked result also - # carries AgentCat's task mint-back, which is v2 behavior the tap - # neither adds to nor removes from.) - assert tracked.content[0].model_dump() == untracked.content[0].model_dump() + # carries AgentCat's task mint-back in front of it, which is v2 + # behavior the tap neither adds to nor removes from.) + assert tracked.content[1].model_dump() == untracked.content[0].model_dump() # Positive control. Everything above is an equality between two runs, # so it passes just as well when track() is a no-op — verified by # reducing track() to `return server`, which leaves the assertions @@ -719,9 +719,9 @@ async def test_a_schema_validation_failure_keeps_the_surfaced_message( result = await client.call_tool("boom", {"marker": 123}) assert result.isError is True - # content[0] is the SDK's own error block; anything after it is - # AgentCat's task mint-back, which every v2 result carries. - surfaced = result.content[0].text + # content[0] is AgentCat's task mint-back, which every v2 result + # carries in front; the SDK's own error block follows it. + surfaced = result.content[1].text assert surfaced.startswith("Input validation error:") error = _one(events, "boom").error assert error == {"message": surfaced, "type": None, "platform": "python"} @@ -904,7 +904,7 @@ async def call(server): # holds trivially when track() does nothing. assert _call_events(events, "boom") assert len(tracked.content) > len(untracked.content) - assert tracked.content[0].model_dump() == untracked.content[0].model_dump() + assert tracked.content[1].model_dump() == untracked.content[0].model_dump() @pytest.mark.asyncio async def test_parallel_failures_each_get_their_own_slot(self, events): @@ -1170,9 +1170,9 @@ async def test_a_proxied_upstream_error_keeps_the_surfaced_message(self, events) ) assert result.is_error is True - # content[0] is the backend's own error block; anything after it is - # AgentCat's task mint-back, which every v2 result carries. - upstream = result.content[0].text + # content[0] is AgentCat's task mint-back, which every v2 result + # carries in front; the backend's own error block follows it. + upstream = result.content[1].text assert "kaboom one" in upstream error = _one(events, "boom").error assert error["message"] == upstream @@ -1202,7 +1202,7 @@ async def call(server): tracked = await call(tracked_server) assert tracked.is_error == untracked.is_error is True - assert tracked.content[0].model_dump() == untracked.content[0].model_dump() + assert tracked.content[1].model_dump() == untracked.content[0].model_dump() # Positive control: see the v1 sibling. An equality between two runs # holds trivially when track() does nothing. assert _call_events(events, "boom") diff --git a/tests/test_lowlevel_v1_handles.py b/tests/test_lowlevel_v1_handles.py index 7e84ddf..fe45910 100644 --- a/tests/test_lowlevel_v1_handles.py +++ b/tests/test_lowlevel_v1_handles.py @@ -25,7 +25,8 @@ AGENTCAT_TAG_AGENT_ID, AGENTCAT_TAG_AGENT_SOURCE, AGENTCAT_TAG_SESSION_SOURCE, - MCP_INSTRUCTIONS_KEY, + MCP_SESSION_KEY, + SESSION_ID_PARAM_PATTERN, ) from agentcat.modules.handles import derive_session_id @@ -34,7 +35,9 @@ from .test_utils.delivery import delivered_arguments_for, record_delivered_arguments from .test_utils.todo_server import create_todo_server -MINT_BACK_HEADER = "[MCP INSTRUCTIONS]: session_id issued." +MINT_BACK_HEADER = ( + "[session_id issued — see this tool's session_id parameter description]" # noqa: E501 +) @pytest.fixture(autouse=True) @@ -75,10 +78,14 @@ async def test_prompted_mode_end_to_end(capture): listed = await client.list_tools() add = next(t for t in listed.tools if t.name == "add_todo") assert list(add.inputSchema["properties"])[-2:] == ["session_id", "context"] - assert "session_id" not in add.inputSchema.get("required", []) - # session_id is the one injected param that is never required — omitting - # it is the minting signal. `context` is required, which is the only - # thing that makes agents supply intent at all. + # Both injected params are required on the wire — schema-aware clients + # are the only enforcement, and session_id names `start` as its + # explicit first-call value, with the value contract as its pattern. + assert "session_id" in add.inputSchema["required"] + assert ( + add.inputSchema["properties"]["session_id"]["pattern"] + == SESSION_ID_PARAM_PATTERN + ) assert "context" in add.inputSchema["required"] assert any(t.name == "get_more_tools" for t in listed.tools) @@ -86,12 +93,13 @@ async def test_prompted_mode_end_to_end(capture): "add_todo", { "text": "hi", + "session_id": "start", "context": "Adding a todo item for the user's task list to track work", }, ) text = _text(r1) assert MINT_BACK_HEADER in text - minted = text.split("session_id=")[1].split(" ")[0] + minted = text.split("session_id: ")[1].split("\n")[0] assert minted.startswith("ses_") r2 = await client.call_tool("add_todo", {"text": "again", "session_id": minted}) @@ -109,7 +117,7 @@ async def test_prompted_mode_end_to_end(capture): # ...and the customer's result, undecorated. assert call_events[0].response is not None assert "Added todo" in json.dumps(call_events[0].response) - assert "[MCP INSTRUCTIONS]" not in json.dumps(call_events[0].response) + assert "[session_id" not in json.dumps(call_events[0].response) assert call_events[0].tags[AGENTCAT_TAG_SESSION_SOURCE] == "minted" assert call_events[1].tags[AGENTCAT_TAG_SESSION_SOURCE] == "supplied" assert call_events[0].user_intent.startswith("Adding a todo item") @@ -117,7 +125,7 @@ async def test_prompted_mode_end_to_end(capture): @NEEDS_STRUCTURED_OUTPUT async def test_structured_mint_back_mirrors_into_structured_content(capture): - """A tool with an outputSchema gets `_mcp_instructions` mirrored in, and its + """A tool with an outputSchema gets `mcp_session` mirrored in, and its schema declares the field so schema-validating clients still accept it.""" server = create_todo_server() track(server, "proj_test", AgentCatOptions()) @@ -125,10 +133,10 @@ async def test_structured_mint_back_mirrors_into_structured_content(capture): async with create_test_client(server) as client: listed = await client.list_tools() add = next(t for t in listed.tools if t.name == "add_todo") - assert MCP_INSTRUCTIONS_KEY in add.outputSchema["properties"] + assert MCP_SESSION_KEY in add.outputSchema["properties"] result = await client.call_tool("add_todo", {"text": "structured"}) - mint = result.structuredContent[MCP_INSTRUCTIONS_KEY] + mint = result.structuredContent[MCP_SESSION_KEY] assert mint["session_id"].startswith("ses_") assert mint["session_id"] == _call_events(capture)[0].session_id # The customer's own structured payload survives untouched. @@ -137,7 +145,7 @@ async def test_structured_mint_back_mirrors_into_structured_content(capture): async def test_handler_sees_stripped_args_and_customer_result_untouched(capture): """The injected params never reach the tool body, and what the tool returned - is exactly what the agent gets back (minus AgentCat's trailing block). + is exactly what the agent gets back (minus AgentCat's leading block). `seen` is filled at the tool manager, not inside `probe`. A typed body can only ever report the parameters it declared, and this manager drops an @@ -166,7 +174,7 @@ def probe(text: str) -> str: assert result.isError is False, _text(result) assert seen == [("probe", {"text": "payload"})] assert result.content[0].text == "probe:payload" - # session_id was supplied, so nothing is minted back and nothing is appended. + # session_id was supplied, so nothing is minted back and nothing is added. assert len(result.content) == 1 assert _call_events(capture)[0].session_id == sid("supplied") @@ -179,8 +187,9 @@ async def test_get_more_tools_keeps_its_own_context_and_publishes(capture): listed = await client.list_tools() gmt = next(t for t in listed.tools if t.name == "get_more_tools") # Its bespoke `context` is a real parameter: still required, still - # described by the tool's own copy — and handles ride alongside. - assert gmt.inputSchema["required"] == ["context"] + # described by the tool's own copy — and handles ride alongside, + # required like everywhere else. + assert gmt.inputSchema["required"] == ["context", "session_id"] assert "session_id" in gmt.inputSchema["properties"] assert read_only_hint(gmt) is True @@ -208,7 +217,9 @@ async def test_agent_tracking_injection(capture): "context", ] assert "agent_id" in add.inputSchema["required"] - assert "session_id" not in add.inputSchema["required"] + assert "session_id" in add.inputSchema["required"] + # agent_id is free-form by design: no pattern in any mode. + assert "pattern" not in add.inputSchema["properties"]["agent_id"] result = await client.call_tool( "add_todo", {"text": "with agent", "agent_id": "opus|claude-code|k3n9x"} @@ -236,15 +247,13 @@ async def test_hook_mode(capture): r1 = await client.call_tool("add_todo", {"text": "hook one"}) r2 = await client.call_tool("add_todo", {"text": "hook two"}) - assert "[MCP INSTRUCTIONS]" not in _text(r1) - assert "[MCP INSTRUCTIONS]" not in _text(r2) + assert "[session_id" not in _text(r1) + assert "[session_id" not in _text(r2) # `getattr`, not attribute access: `structuredContent` is a real field # from mcp 1.10 and an unset extra before it, and pydantic raises # AttributeError for an extra that was never assigned. The claim here # is absence either way. - assert MCP_INSTRUCTIONS_KEY not in ( - getattr(r1, "structuredContent", None) or {} - ) + assert MCP_SESSION_KEY not in (getattr(r1, "structuredContent", None) or {}) call_events = _call_events(capture) expected = derive_session_id("cust-1", "proj_test") @@ -268,7 +277,7 @@ async def test_tracing_disabled_strips_but_publishes_nothing(capture): "add_todo", {"text": "quiet", "context": "no tracing"} ) assert result.isError is False, _text(result) - assert "[MCP INSTRUCTIONS]" not in _text(result) + assert "[session_id" not in _text(result) # Read at the tool manager: `add_todo` is a typed body, which cannot show # an argument that arrived and was dropped on the way in. diff --git a/tests/test_lowlevel_v2_handles.py b/tests/test_lowlevel_v2_handles.py index 938b9f2..cd2a67d 100644 --- a/tests/test_lowlevel_v2_handles.py +++ b/tests/test_lowlevel_v2_handles.py @@ -31,7 +31,7 @@ AGENTCAT_TAG_MRTR, AGENTCAT_TAG_PROTOCOL_VERSION, AGENTCAT_TAG_SESSION_SOURCE, - MCP_INSTRUCTIONS_KEY, + MCP_SESSION_KEY, ) from agentcat.modules.handles import derive_session_id @@ -44,7 +44,7 @@ create_modern_client, ) -MINT_BACK_HEADER = "[MCP INSTRUCTIONS]: session_id issued." +MINT_BACK_HEADER = "[session_id issued — see this tool's session_id parameter description]" # noqa: E501 MODERN = "2026-07-28" @@ -93,10 +93,11 @@ async def test_prompted_mode_end_to_end(capture): listed = await client.list_tools() add = _tool(listed, "add_todo") assert list(add.input_schema["properties"])[-2:] == ["session_id", "context"] - # session_id is the one injected param that is never required — omitting - # it is the minting signal. `context` is required, which is the only - # thing that makes agents supply intent at all. - assert "session_id" not in add.input_schema.get("required", []) + # Both injected params are required on the wire — schema-aware clients + # are the only enforcement, and session_id names `start` as its + # explicit first-call value. + assert "session_id" in add.input_schema["required"] + assert "pattern" in add.input_schema["properties"]["session_id"] assert "context" in add.input_schema["required"] assert any(t.name == "get_more_tools" for t in listed.tools) @@ -104,12 +105,13 @@ async def test_prompted_mode_end_to_end(capture): "add_todo", { "text": "hi", + "session_id": "start", "context": "Adding a todo item for the user's task list to track work", }, ) text = _text(r1) assert MINT_BACK_HEADER in text - minted = text.split("session_id=")[1].split(" ")[0] + minted = text.split("session_id: ")[1].split("\n")[0] assert minted.startswith("ses_") r2 = await client.call_tool("add_todo", {"text": "again", "session_id": minted}) @@ -127,7 +129,7 @@ async def test_prompted_mode_end_to_end(capture): # ...and the customer's result, undecorated. assert call_events[0].response is not None assert "Added todo" in json.dumps(call_events[0].response) - assert "[MCP INSTRUCTIONS]" not in json.dumps(call_events[0].response) + assert "[session_id" not in json.dumps(call_events[0].response) assert call_events[0].tags[AGENTCAT_TAG_SESSION_SOURCE] == "minted" assert call_events[1].tags[AGENTCAT_TAG_SESSION_SOURCE] == "supplied" assert call_events[0].user_intent.startswith("Adding a todo item") @@ -164,7 +166,7 @@ async def test_client_identity_ladder(capture): async def test_structured_mint_back_mirrors_into_structured_content(capture): - """A tool with an output schema gets `_mcp_instructions` mirrored in, and + """A tool with an output schema gets `mcp_session` mirrored in, and its schema declares the field so schema-validating clients still accept it.""" server = create_lowlevel_todo_server() @@ -172,12 +174,12 @@ async def test_structured_mint_back_mirrors_into_structured_content(capture): async with create_modern_client(server, mode=MODERN) as client: listed = await client.list_tools() - assert MCP_INSTRUCTIONS_KEY in _tool(listed, "add_todo").output_schema[ + assert MCP_SESSION_KEY in _tool(listed, "add_todo").output_schema[ "properties" ] result = await client.call_tool("add_todo", {"text": "structured"}) - mint = result.structured_content[MCP_INSTRUCTIONS_KEY] + mint = result.structured_content[MCP_SESSION_KEY] assert mint["session_id"].startswith("ses_") assert mint["session_id"] == _call_events(capture)[0].session_id # The customer's own structured payload survives untouched. @@ -186,7 +188,7 @@ async def test_structured_mint_back_mirrors_into_structured_content(capture): async def test_handler_sees_stripped_args_and_customer_result_untouched(capture): """The injected params never reach the tool body, and what the tool returned - is exactly what the agent gets back (minus AgentCat's trailing block).""" + is exactly what the agent gets back (minus AgentCat's leading block).""" server = create_lowlevel_todo_server() track(server, "proj_test") @@ -201,7 +203,7 @@ async def test_handler_sees_stripped_args_and_customer_result_untouched(capture) # surfaces here rather than passing silently. assert result.is_error is False, _text(result) assert result.content[0].text == 'Added todo: "payload" with ID 1' - # session_id was supplied, so nothing is minted back and nothing is appended. + # session_id was supplied, so nothing is minted back and nothing is added. assert len(result.content) == 1 assert _call_events(capture)[0].session_id == sid("supplied") @@ -241,7 +243,7 @@ async def test_agent_tracking_injection(capture): "context", ] assert "agent_id" in add.input_schema["required"] - assert "session_id" not in add.input_schema["required"] + assert "session_id" in add.input_schema["required"] result = await client.call_tool( "add_todo", {"text": "with agent", "agent_id": "opus|claude-code|k3n9x"} @@ -269,9 +271,9 @@ async def test_hook_mode(capture): r1 = await client.call_tool("add_todo", {"text": "hook one"}) r2 = await client.call_tool("add_todo", {"text": "hook two"}) - assert "[MCP INSTRUCTIONS]" not in _text(r1) - assert "[MCP INSTRUCTIONS]" not in _text(r2) - assert MCP_INSTRUCTIONS_KEY not in (r1.structured_content or {}) + assert "[session_id" not in _text(r1) + assert "[session_id" not in _text(r2) + assert MCP_SESSION_KEY not in (r1.structured_content or {}) call_events = _call_events(capture) expected = derive_session_id("cust-1", "proj_test") @@ -299,7 +301,7 @@ async def test_tracing_disabled_strips_but_publishes_nothing(capture): "add_todo", {"text": "quiet", "context": "no tracing"} ) assert result.is_error is False, _text(result) - assert "[MCP INSTRUCTIONS]" not in _text(result) + assert "[session_id" not in _text(result) # get_more_tools is advertised and still answers with tracing off # (changelog 6.6) — it is a tool, not telemetry. @@ -440,7 +442,7 @@ async def test_intermediate_mrtr_round_is_tagged_but_never_decorated(capture): assert events[0].session_id.startswith("ses_") # Only the completing round carries the mint-back. assert MINT_BACK_HEADER in _text(second) - assert MCP_INSTRUCTIONS_KEY in second.structured_content + assert MCP_SESSION_KEY in second.structured_content async def test_input_responses_alone_still_tags_a_continuation(capture): @@ -662,7 +664,7 @@ async def test_a_minted_first_round_does_not_yet_correlate(capture): assert events[0].session_id != events[1].session_id # What the agent is handed is the COMPLETING round's handle, so every call # after the conversation joins that task. - minted = _text(result).split("session_id=")[1].split(" ")[0] + minted = _text(result).split("session_id: ")[1].split("\n")[0] assert events[1].session_id == minted @@ -711,7 +713,7 @@ async def on_call_tool(ctx, params): ) assert result is returned[0] - assert MCP_INSTRUCTIONS_KEY not in result.structured_content + assert MCP_SESSION_KEY not in result.structured_content assert [block.text for block in result.content] == ["so far so good"] assert _call_events(capture)[0].tags[AGENTCAT_TAG_MRTR] == "input_required" @@ -801,8 +803,9 @@ async def test_get_more_tools_keeps_its_own_context_and_publishes(capture): listed = await client.list_tools() gmt = _tool(listed, "get_more_tools") # Its bespoke `context` is a real parameter: still required, still - # described by the tool's own copy — and handles ride alongside. - assert gmt.input_schema["required"] == ["context"] + # described by the tool's own copy — and handles ride alongside, + # required like everywhere else. + assert gmt.input_schema["required"] == ["context", "session_id"] assert "session_id" in gmt.input_schema["properties"] assert gmt.annotations.read_only_hint is True @@ -1135,7 +1138,7 @@ async def test_mcpserver_end_to_end(capture): assert result.is_error is False, _text(result) text = _text(result) assert MINT_BACK_HEADER in text - minted = text.split("session_id=")[1].split(" ")[0] + minted = text.split("session_id: ")[1].split("\n")[0] # `add_todo(text: str)` is typed and this manager drops an undeclared # argument silently, so the delivered dict is the only place a failed diff --git a/tests/test_rebuild_on_demand.py b/tests/test_rebuild_on_demand.py index 17fbd31..830b2ed 100644 --- a/tests/test_rebuild_on_demand.py +++ b/tests/test_rebuild_on_demand.py @@ -25,7 +25,7 @@ AGENTCAT_TAG_SESSION_SOURCE, CONTEXT_PARAM, GET_MORE_TOOLS_NAME, - MCP_INSTRUCTIONS_KEY, + MCP_SESSION_KEY, SESSION_ID_PARAM, ) @@ -99,7 +99,7 @@ async def test_a_call_before_any_listing_rebuilds_the_registries( # The mirror is gated on the rebuilt OUTPUT registry, and `echo` is in it. assert data.output_injection_registry is not None assert "echo" in data.output_injection_registry - assert result.structured[MCP_INSTRUCTIONS_KEY][SESSION_ID_PARAM] == SUPPLIED + assert result.structured[MCP_SESSION_KEY][SESSION_ID_PARAM] == SUPPLIED event = capture[0] assert event.session_id == SUPPLIED @@ -252,9 +252,9 @@ async def test_a_failed_rebuild_spares_a_customers_own_session_id( assert built.seen == [ ("complete_task", {"session_id": "TICKET-9", "note": "done"}) ] - # No "session_id not recognized" correction steering agents away from the + # No "session_id unrecognized" correction steering agents away from the # customer's own parameter. - assert "not recognized" not in (result.text or "") + assert "unrecognized" not in (result.text or "") # Sessionless, tagged for the dashboard — the honest degraded record. event = capture[0] assert event.session_id is None @@ -302,7 +302,7 @@ async def test_a_failed_rebuild_still_protects_get_more_tools_own_context( ] assert "customer answered: I need a tool to send email" in result.text # ...and the handle still rides back with no registry to gate it. - assert result.structured[MCP_INSTRUCTIONS_KEY][SESSION_ID_PARAM] == SUPPLIED + assert result.structured[MCP_SESSION_KEY][SESSION_ID_PARAM] == SUPPLIED assert capture[0].session_id == SUPPLIED diff --git a/tests/test_removed_events.py b/tests/test_removed_events.py index 304fdf7..7cf10b1 100644 --- a/tests/test_removed_events.py +++ b/tests/test_removed_events.py @@ -46,7 +46,9 @@ async def test_a_full_lifecycle_publishes_only_tool_calls(flavor, capture): assert {tool.name for tool in listed} == {"echo", GET_MORE_TOOLS_NAME} first = await flavor.call(client, "echo", {"text": "one"}) - minted = first.text.split("session_id=")[1].split(" ")[0] + # The mint line is "session_id: ", on its own line at the START + # of the result text (the block is the first content element). + minted = first.text.split("session_id: ")[1].split("\n")[0] await flavor.call(client, "echo", {"text": "two", SESSION_ID_PARAM: minted}) # AgentCat answers this one itself, and it is still just a tool call. await flavor.call( diff --git a/tests/test_report_missing.py b/tests/test_report_missing.py index a4ffde7..a57191e 100644 --- a/tests/test_report_missing.py +++ b/tests/test_report_missing.py @@ -72,10 +72,10 @@ async def test_report_missing_tool_call_success(self): # Verify successful response. get_more_tools publishes events like # any other tool, so it mints a task and carries the mint-back - # block after its own answer. + # block in front of its own answer. assert result.content[0].type == "text" - assert "Unfortunately" in result.content[0].text - assert "[MCP INSTRUCTIONS]: session_id issued." in result.content[-1].text + assert result.content[0].text.startswith("[session_id issued") + assert "Unfortunately" in result.content[1].text @pytest.mark.asyncio async def test_report_missing_with_valid_params(self): @@ -100,8 +100,9 @@ async def test_report_missing_with_valid_params(self): for params in test_cases: result = await client.call_tool("get_more_tools", params) - assert result.content[0].text - assert "Unfortunately" in result.content[0].text + # content[0] is the minted-session block; the answer follows. + assert result.content[1].text + assert "Unfortunately" in result.content[1].text @pytest.mark.asyncio async def test_report_missing_with_missing_params(self): @@ -116,12 +117,12 @@ async def test_report_missing_with_missing_params(self): # call over its own analytics, so a lax client still gets an answer. result = await client.call_tool("get_more_tools", {}) assert result.isError is False - assert "Unfortunately" in result.content[0].text + assert "Unfortunately" in result.content[1].text # Test with valid context result = await client.call_tool("get_more_tools", {"context": "test_tool"}) - assert result.content[0].text - assert "Unfortunately" in result.content[0].text + assert result.content[1].text + assert "Unfortunately" in result.content[1].text @pytest.mark.asyncio async def test_report_missing_with_extra_params(self): @@ -139,8 +140,8 @@ async def test_report_missing_with_extra_params(self): ) # Should still work normally - assert result.content[0].text - assert "Unfortunately" in result.content[0].text + assert result.content[1].text + assert "Unfortunately" in result.content[1].text @pytest.mark.asyncio async def test_report_missing_with_other_tools(self): @@ -152,17 +153,17 @@ async def test_report_missing_with_other_tools(self): async with create_test_client(server) as client: # First use a regular tool add_result = await client.call_tool("add_todo", {"text": "Test todo item"}) - assert "Added todo" in add_result.content[0].text + assert "Added todo" in add_result.content[1].text # Then use report_missing report_result = await client.call_tool( "get_more_tools", {"context": "Delete a todo item"} ) - assert "Unfortunately" in report_result.content[0].text + assert "Unfortunately" in report_result.content[1].text # Verify the original tool still works list_result = await client.call_tool("list_todos") - assert "Test todo item" in list_result.content[0].text + assert "Test todo item" in list_result.content[1].text @pytest.mark.asyncio async def test_multiple_report_missing_calls(self): @@ -187,8 +188,8 @@ async def test_multiple_report_missing_calls(self): }, ) # Each call should work identically - assert result.content[0].text - assert "Unfortunately" in result.content[0].text + assert result.content[1].text + assert "Unfortunately" in result.content[1].text @pytest.mark.asyncio async def test_report_missing_with_context_enabled(self): @@ -247,7 +248,7 @@ async def test_report_missing_with_null_values(self): # fail the call either. result = await client.call_tool("get_more_tools", {"context": None}) assert result.isError is False - assert "Unfortunately" in result.content[0].text + assert "Unfortunately" in result.content[1].text @pytest.mark.asyncio async def test_report_missing_publishes_event(self): diff --git a/tests/test_response_shape.py b/tests/test_response_shape.py index 60aa586..39a2cdd 100644 --- a/tests/test_response_shape.py +++ b/tests/test_response_shape.py @@ -23,7 +23,7 @@ import pytest from agentcat import AgentCatOptions, track -from agentcat.modules.constants import MCP_INSTRUCTIONS_KEY, SESSION_ID_PARAM +from agentcat.modules.constants import MCP_SESSION_KEY, SESSION_ID_PARAM from .test_utils import FASTMCP_TOOLRESULT_HAS_IS_ERROR, sid from .test_utils.flavors import flavors @@ -86,5 +86,5 @@ async def test_the_event_response_keeps_its_eras_own_field_names(flavor, capture # ...and it is the customer's own result, undecorated: the mint-back is # wire-only and must never reach the analytics payload. assert response[structured_key] == {"result": "echo:hi"} - assert MCP_INSTRUCTIONS_KEY not in json.dumps(response) - assert "[MCP INSTRUCTIONS]" not in json.dumps(response) + assert MCP_SESSION_KEY not in json.dumps(response) + assert "[session_id" not in json.dumps(response) diff --git a/tests/test_session_id_validation.py b/tests/test_session_id_validation.py index 35d605a..344feeb 100644 --- a/tests/test_session_id_validation.py +++ b/tests/test_session_id_validation.py @@ -26,8 +26,8 @@ from agentcat import AgentCatOptions, track from agentcat.modules.constants import ( AGENTCAT_TAG_SESSION_SOURCE, - MINT_BACK_HEADER_INVALID, - MINT_BACK_HEADER_SESSION, + MINT_BACK_HEADER_ISSUED, + MINT_BACK_HEADER_UNRECOGNIZED, SESSION_ID_PARAM, ) from agentcat.modules.handles import ( @@ -89,9 +89,13 @@ def test_rejects_anything_this_sdk_did_not_issue(label, value): # # | args.session_id | ours? | shape | Event.session_id | source | # | absent | yes | — | new_session_id() | minted | +# | "start" (ci) | yes | — | new_session_id() | minted | # | present | yes | valid | verbatim | supplied | # | present | yes | invalid | "" (sessionless) | invalid | # | present/absent | no | — | "" (sessionless) | foreign | +# +# The `start` sentinel row is checked before shape validation, on the trimmed +# value, case-insensitively — and only where the parameter is OURS. async def _resolve(arguments, *, ours=True, **options): @@ -112,6 +116,18 @@ async def test_absent_and_ours_mints(): assert is_valid_session_id(r.session_id) +@pytest.mark.parametrize("value", ["start", "Start", "START", " start ", " StArT\t"]) +async def test_the_start_sentinel_and_ours_mints(value): + """`start` resolves exactly like an omitted session_id: minted, issued. + + Case variants and padded whitespace all count — resolution is lenient even + though the schema pattern only names the lowercase spelling. + """ + r = await _resolve({SESSION_ID_PARAM: value}) + assert r.session_source == "minted" + assert is_valid_session_id(r.session_id) + + async def test_valid_and_ours_is_taken_verbatim(): r = await _resolve({SESSION_ID_PARAM: sid("parent")}) assert (r.session_source, r.session_id) == ("supplied", sid("parent")) @@ -136,7 +152,16 @@ async def test_the_rejected_value_is_never_stored_anywhere(): assert secret not in str(build_structured_mint_back(r)) -@pytest.mark.parametrize("arguments", [{}, {SESSION_ID_PARAM: "customer-value"}]) +@pytest.mark.parametrize( + "arguments", + [ + {}, + {SESSION_ID_PARAM: "customer-value"}, + # `start` included: a customer-owned value is never read as the + # sentinel, so it cannot trigger a mint on a foreign parameter. + {SESSION_ID_PARAM: "start"}, + ], +) async def test_a_foreign_param_is_sessionless_whatever_the_agent_sent(arguments): r = await _resolve(arguments, ours=False) assert (r.session_source, r.session_id) == ("foreign", "") @@ -164,7 +189,8 @@ async def test_a_missing_registry_still_validates(): Nothing is in `declared_session_params` yet, so the tool counts as ours and the value is validated rather than adopted. A customer's foreign value in that window degrades to `invalid` instead of `foreign` — both - sessionless, only the tag differs. + sessionless, only the tag differs (a value spelling `start` instead + mints, exactly as on a parameter we injected). """ r = await _resolve({SESSION_ID_PARAM: "TICKET-77"}) assert (r.session_source, r.session_id) == ("invalid", "") @@ -176,28 +202,26 @@ async def test_a_missing_registry_still_validates(): async def test_invalid_corrects_the_agent_without_issuing_a_replacement(): r = await _resolve({SESSION_ID_PARAM: "nope"}) text = build_mint_back_text(r) - assert text.startswith(MINT_BACK_HEADER_INVALID) - assert "Re-send the exact session_id" in text - assert "omit the parameter and one will be issued" in text + assert text.startswith(MINT_BACK_HEADER_UNRECOGNIZED) + assert "Re-send the session_id issued earlier" in text + assert "send start and one will be issued" in text # Nothing that looks like an issued ID appears — this branch corrects, it # does not mint. Handing out a second ID would split a session that was # never split. assert "ses_" not in text - assert MINT_BACK_HEADER_SESSION not in text + assert MINT_BACK_HEADER_ISSUED not in text -async def test_invalid_mirror_carries_instructions_but_no_session_id(): - """The regression `not names: return None` would cause. +async def test_invalid_mirror_reports_unrecognized_without_a_session_id(): + """The one branch with something to say and nothing to echo. - With no agent_id in play there is nothing echoable, so the old early - return dropped the correction entirely — the one branch that has something - to say and nothing to confirm. + With no agent_id in play there is nothing echoable, but the response must + still report the rejection — `status` carries it, and no session_id is + named (no replacement is issued). """ r = await _resolve({SESSION_ID_PARAM: "nope"}) mint = build_structured_mint_back(r) - assert mint is not None - assert SESSION_ID_PARAM not in mint - assert "not recognized" in mint["instructions"] + assert mint == {"status": "unrecognized"} async def test_foreign_says_nothing_about_session_id_at_all(): @@ -209,9 +233,8 @@ async def test_foreign_says_nothing_about_session_id_at_all(): async def test_foreign_never_confirms_a_value_agentcat_did_not_issue(): """The confirmation loop, pinned. - The bug was `mint_back_confirmed` telling the agent its own - customer-semantics value was "confirmed. Keep sending this exact value on - every call." + The bug was the mirror echoing the agent's own customer-semantics value + back as an AgentCat handle to keep sending. """ r = await _resolve( {SESSION_ID_PARAM: "customer-value", "agent_id": "opus|cc|k3n9x"}, @@ -220,14 +243,9 @@ async def test_foreign_never_confirms_a_value_agentcat_did_not_issue(): ) mint = build_structured_mint_back(r) # agent_id is a separate injection and still landed, so it is still ours - # to confirm — suppression is per handle, not per response. - assert mint == { - "agent_id": "opus|cc|k3n9x", - "instructions": ( - "[MCP INSTRUCTIONS]: agent_id confirmed. " - "Keep sending this exact value on every call." - ), - } + # to mirror — suppression is per handle, not per response. No status + # either: that vocabulary belongs to the session parameter AgentCat owns. + assert mint == {"agent_id": "opus|cc|k3n9x"} assert "customer-value" not in str(mint) @@ -366,7 +384,7 @@ async def test_an_invalid_id_publishes_sessionless_and_corrects_the_agent( client, "echo", {"text": "hi", SESSION_ID_PARAM: "not-a-real-id"} ) - assert "session_id not recognized" in result.text + assert "[session_id unrecognized" in result.text assert "not-a-real-id" not in result.text (event,) = capture @@ -378,16 +396,23 @@ async def test_an_invalid_id_publishes_sessionless_and_corrects_the_agent( assert event.parameters["arguments"][SESSION_ID_PARAM] == "not-a-real-id" +@pytest.mark.parametrize( + "recovery_args", + [ + {"text": "b", SESSION_ID_PARAM: "start"}, + {"text": "b"}, + ], + ids=["sends-start", "omits-the-parameter"], +) @pytest.mark.parametrize("flavor", flavors(), ids=lambda f: f.id) -async def test_the_correction_lets_an_agent_recover_by_omitting_the_parameter( - flavor, capture -): +async def test_the_correction_lets_an_agent_recover(flavor, capture, recovery_args): """The deadlock the closing sentence of the copy exists to prevent. An agent that hallucinates a session_id on its FIRST call was never issued - one, so "re-send what you were given" names nothing. Omitting the - parameter has to put it back on the minting path — otherwise the - conversation can never acquire a session at all. + one, so "re-send what you were given" names nothing. Sending `start` — the + recovery the copy names — has to put it back on the minting path, and an + omitted parameter still mints too: a stale schema or scripted caller must + never be locked out of acquiring a session. """ built = flavor.build("invalid-recovery") track(built.server, "proj_test", AgentCatOptions()) @@ -395,10 +420,10 @@ async def test_the_correction_lets_an_agent_recover_by_omitting_the_parameter( async with flavor.client(built.server) as client: await flavor.list_tools(client) await flavor.call(client, "echo", {"text": "a", SESSION_ID_PARAM: "guessed"}) - recovered = await flavor.call(client, "echo", {"text": "b"}) + recovered = await flavor.call(client, "echo", recovery_args) - assert MINT_BACK_HEADER_SESSION in recovered.text - issued = recovered.structured["_mcp_instructions"][SESSION_ID_PARAM] + assert MINT_BACK_HEADER_ISSUED in recovered.text + issued = recovered.structured["mcp_session"][SESSION_ID_PARAM] assert is_valid_session_id(issued) rejected, minted = capture diff --git a/tests/test_tool_context.py b/tests/test_tool_context.py index c5e6bdf..110df1d 100644 --- a/tests/test_tool_context.py +++ b/tests/test_tool_context.py @@ -7,8 +7,8 @@ `context` stays REQUIRED, as it was in 1.x. Nothing server-side rejects a call that omits it — a schema-validating client refusing to send one is the whole enforcement mechanism, and without it agents quietly stop supplying intent. -`session_id` is the one injected parameter that is never required: omitting it is -how an agent asks to be minted one. +`session_id` is required the same way, with `start` as its explicit first-call +value; an absent value still mints, so a stale schema never errors. """ import time @@ -53,8 +53,9 @@ async def test_context_parameter_injection_enabled(self): # Required, as in 1.x: a strict client refusing to send a call # without it is the only thing that makes agents supply intent. assert "context" in tool.inputSchema["required"] - # ...and session_id is not, because omitting it is the mint signal. - assert "session_id" not in tool.inputSchema.get("required", []) + # ...and so is session_id, whose copy names start as the value + # that asks to be minted one. + assert "session_id" in tool.inputSchema["required"] @pytest.mark.asyncio async def test_context_parameter_not_injected_when_disabled(self): @@ -94,7 +95,7 @@ def simple_tool(): simple = _named(await _tools(mcp), "simple_tool") assert simple.inputSchema is not None assert "context" in simple.inputSchema["properties"] - assert simple.inputSchema["required"] == ["context"] + assert simple.inputSchema["required"] == ["session_id", "context"] @pytest.mark.asyncio async def test_schema_with_empty_properties(self): @@ -118,7 +119,7 @@ async def test_schema_with_existing_required_fields(self): track(server, "test_project", AgentCatOptions(enable_tool_call_context=True)) add_todo = _named(await _tools(server), "add_todo") - assert add_todo.inputSchema["required"] == ["text", "context"] + assert add_todo.inputSchema["required"] == ["text", "session_id", "context"] @pytest.mark.asyncio async def test_schema_with_no_required_fields(self): @@ -133,10 +134,10 @@ def optional_params_tool(param1: str = "default"): track(mcp, "test_project", AgentCatOptions(enable_tool_call_context=True)) tool = _named(await _tools(mcp), "optional_params_tool") - assert tool.inputSchema["required"] == ["context"] + assert tool.inputSchema["required"] == ["session_id", "context"] - # With the context pass off there is nothing to require at all, so the - # tool's own (absent) required array is left absent. + # With the context pass off, the handle pass still requires the + # session_id it injected — requiredness rides injection exactly. untouched = FastMCP("test-server-2") @untouched.tool() @@ -148,7 +149,7 @@ def other_tool(param1: str = "default"): untouched, "test_project", AgentCatOptions(enable_tool_call_context=False) ) listed = _named(await _tools(untouched), "other_tool") - assert listed.inputSchema.get("required", []) == [] + assert listed.inputSchema.get("required", []) == ["session_id"] @pytest.mark.asyncio async def test_server_with_no_tools(self): @@ -173,7 +174,7 @@ async def test_get_more_tools_exclusion_with_context(self): get_more_tools = _named(tools, "get_more_tools") context_schema = get_more_tools.inputSchema["properties"]["context"] assert context_schema["description"] != DEFAULT_CONTEXT_DESCRIPTION - assert get_more_tools.inputSchema["required"] == ["context"] + assert get_more_tools.inputSchema["required"] == ["context", "session_id"] for tool in tools: if tool.name == "get_more_tools": From 0e390e6ecf5c7cddd1c967fb952397034e7cb4fd Mon Sep 17 00:00:00 2001 From: Naseem Alnaji Date: Sat, 29 Aug 2026 21:40:37 +0000 Subject: [PATCH 2/4] test: legacy suites read the tool output after the leading mint-back, expect session_id required handles.py now prepends the session-id mint-back block, and injection.py requires session_id. test_inner_tap.py followed; these three files are _LEGACY_ONLY (conftest) so the mcp 2.x dev env never ran them. --- tests/community/test_community_v3_openapi.py | 6 +++--- tests/test_dynamic_tracking.py | 14 ++++++++------ tests/test_tool_context.py | 20 +++++++++++--------- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/tests/community/test_community_v3_openapi.py b/tests/community/test_community_v3_openapi.py index 147a9cf..490d8a2 100644 --- a/tests/community/test_community_v3_openapi.py +++ b/tests/community/test_community_v3_openapi.py @@ -68,9 +68,9 @@ async def test_list_tools_injects_handles_and_context(capture): assert SESSION_ID_PARAM in props, f"session_id not injected into {name}" assert "context" in props, f"context not injected into {name}" assert props["context"]["description"] == CONTEXT_DESC - # session_id is never required — omitting it is the minting signal — - # but context is, as it was in 1.x. - assert SESSION_ID_PARAM not in schema.get("required", []) + # Every injected param is required — session_id included, with + # `start` as its explicit first-call value — and context as in 1.x. + assert SESSION_ID_PARAM in schema["required"] assert "context" in schema["required"] diff --git a/tests/test_dynamic_tracking.py b/tests/test_dynamic_tracking.py index 1cf1e5f..58e90e3 100644 --- a/tests/test_dynamic_tracking.py +++ b/tests/test_dynamic_tracking.py @@ -66,7 +66,9 @@ def early_tool(x: int) -> str: assert "early_tool" in [t.name for t in listed.tools] result = await client.call_tool("early_tool", {"x": 42}) - assert result.content[0].text == "42" + # content[0] is AgentCat's task mint-back, which every v2 result + # carries in front; the tool's own output follows it. + assert result.content[1].text == "42" assert [e.resource_name for e in capture] == ["early_tool"] assert get_server_tracking_data(fastmcp_server) is not None @@ -92,7 +94,7 @@ def late_tool(x: int) -> str: result = await client.call_tool( "late_tool", {"x": 123, "context": "late registration"} ) - assert result.content[0].text == "123" + assert result.content[1].text == "123" assert [e.resource_name for e in capture] == ["late_tool"] assert capture[0].user_intent == "late registration" @@ -121,7 +123,7 @@ def rebuilt_tool(x: int) -> str: ) assert result.isError is False - assert result.content[0].text == "Result: 42" + assert result.content[1].text == "Result: 42" assert seen == [("rebuilt_tool", {"x": 42})] # The EVENT still carries the raw pre-strip arguments, by design. assert capture[0].parameters["arguments"]["context"] == "no listing yet" @@ -217,7 +219,7 @@ async def call_tool(name: str, arguments: dict) -> list[Any]: {"value": "test123", "agent_id": "a|b|c", "context": "why"}, ) assert result.isError is False, result.content - assert result.content[0].text == "Low-level result: test123" + assert result.content[1].text == "Low-level result: test123" assert capture[0].tags["agentcat_agent_id"] == "a|b|c" assert get_server_tracking_data(lowlevel_server) is not None @@ -244,7 +246,7 @@ def server2_tool(x: int) -> str: "server1_tool" ] result = await client.call_tool("server1_tool", {"x": 10}) - assert result.content[0].text == "Server1: 10" + assert result.content[1].text == "Server1: 10" assert (await client.call_tool("server2_tool", {"x": 1})).isError is True async with create_test_client(server2) as client: @@ -253,7 +255,7 @@ def server2_tool(x: int) -> str: "server2_tool", ] result = await client.call_tool("server2_tool", {"x": 20}) - assert result.content[0].text == "Server2: 20" + assert result.content[1].text == "Server2: 20" assert get_server_tracking_data(server1).project_id == "project1" assert get_server_tracking_data(server2).project_id == "project2" diff --git a/tests/test_tool_context.py b/tests/test_tool_context.py index 110df1d..d880925 100644 --- a/tests/test_tool_context.py +++ b/tests/test_tool_context.py @@ -255,7 +255,9 @@ def tool_with_context(context: str, data: str): result = await client.call_tool( "tool_with_context", {"context": "mine", "data": "d"} ) - assert "Original context: mine" in result.content[0].text + # content[0] is AgentCat's task mint-back, which every v2 result + # carries in front; the tool's own output follows it. + assert "Original context: mine" in result.content[1].text @pytest.mark.asyncio async def test_schema_with_allof_anyof_oneof(self): @@ -322,7 +324,7 @@ async def test_tool_call_with_valid_context(self): }, ) - assert "Added todo" in result.content[0].text + assert "Added todo" in result.content[1].text @pytest.mark.asyncio async def test_tool_call_without_context_still_succeeds(self): @@ -333,7 +335,7 @@ async def test_tool_call_without_context_still_succeeds(self): async with create_test_client(server) as client: result = await client.call_tool("add_todo", {"text": "Test todo item"}) - assert "Added todo" in result.content[0].text + assert "Added todo" in result.content[1].text @pytest.mark.asyncio @pytest.mark.parametrize( @@ -362,7 +364,7 @@ async def test_tool_call_with_edge_case_context(self, context): ) assert result.isError is False - assert "Added todo" in result.content[0].text + assert "Added todo" in result.content[1].text assert delivered_arguments_for(server, "add_todo") == [{"text": "Test todo"}] @pytest.mark.asyncio @@ -382,13 +384,13 @@ async def test_original_functionality_preserved(self): list_result = await client.call_tool( "list_todos", {"context": "Listing all todos to verify they were added"} ) - assert "First todo" in list_result.content[0].text - assert "Second todo" in list_result.content[0].text + assert "First todo" in list_result.content[1].text + assert "Second todo" in list_result.content[1].text complete_result = await client.call_tool( "complete_todo", {"id": 1, "context": "Completing the first todo"} ) - assert "Completed todo" in complete_result.content[0].text + assert "Completed todo" in complete_result.content[1].text @pytest.mark.asyncio async def test_context_not_passed_to_original_handler(self): @@ -408,7 +410,7 @@ async def test_context_not_passed_to_original_handler(self): ) assert result.isError is False - assert "Added todo" in result.content[0].text + assert "Added todo" in result.content[1].text assert delivered_arguments_for(server, "add_todo") == [{"text": "test data"}] @pytest.mark.asyncio @@ -580,7 +582,7 @@ async def test_custom_context_with_tool_call(self): "context": "Adding todo to test custom context description feature", }, ) - assert "Added todo" in result.content[0].text + assert "Added todo" in result.content[1].text class TestGetMoreToolsContextSchema: From d7361d162a4db33b5bed239dcfd214f33c9d2b87 Mon Sep 17 00:00:00 2001 From: Naseem Alnaji Date: Sat, 29 Aug 2026 21:52:47 +0000 Subject: [PATCH 3/4] fix(tap): restore a crash's own text on the event for mcp >= 2.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mcp 2.1 wraps a tool crash in UnexpectedToolError("Error executing tool ") and keeps the crash's text off the wire; every earlier generation put ": " after the prefix. The cause still carries it, so InnerTap.error() appends it to the event's message — matched by class name, skipped when already present (nested crashes) — and error.message reads the same on every generation. The wire is untouched. Tests: MCPSERVER_CRASH_WRAPPER / MCPSERVER_CRASH_TEXT_ON_WIRE probes in test_utils; TestOfficialV2 asserts the wrapper name via the probe and gates only the wire check; two new tests pin the parity and no-duplication. --- src/agentcat/modules/adapters/_inner_tap.py | 23 +++++++- tests/test_inner_tap.py | 61 +++++++++++++++++++-- tests/test_utils/__init__.py | 19 +++++++ 3 files changed, 97 insertions(+), 6 deletions(-) diff --git a/src/agentcat/modules/adapters/_inner_tap.py b/src/agentcat/modules/adapters/_inner_tap.py index b6ee951..d1a78f4 100644 --- a/src/agentcat/modules/adapters/_inner_tap.py +++ b/src/agentcat/modules/adapters/_inner_tap.py @@ -98,7 +98,7 @@ from types import TracebackType from typing import Any -from agentcat.modules.exceptions import capture_exception +from agentcat.modules.exceptions import _safe_str, capture_exception from agentcat.modules.logging import write_to_log from agentcat.types import ErrorData @@ -202,12 +202,31 @@ def error(self, flattened: Any) -> ErrorData: an upstream error a proxy passed through, or a tool that returned ``is_error`` without anything having been raised at all, both of which have no local exception and never will. + + One generation's wrapper is patched up: mcp 2.1 keeps a crash's own + text off the wire by raising ``UnexpectedToolError("Error executing + tool ")`` from it, where every earlier generation put + ``: `` after the prefix. The cause still carries the text, so + it goes back on the event's message — the wire is not ours to touch — + and the message reads the same on every generation. Matched by class + name (the class exists only where the policy does) and skipped when + the text is already there (a nested crash's wrapper embeds the inner + wrapper's, and a future upstream may embed it again). """ # `is not None`, not truthiness: an exception class that defines # `__len__` or `__bool__` can be falsy, and losing its traceback to # that would be a very quiet bug. exc = self._cell.exc - return capture_exception(exc if exc is not None else flattened) + error = capture_exception(exc if exc is not None else flattened) + try: + cause = getattr(exc, "__cause__", None) + if type(exc).__name__ == "UnexpectedToolError" and cause is not None: + text = _safe_str(cause) + if text not in error["message"]: + error["message"] = f"{error['message']}: {text}" + except Exception: # a hostile __cause__ costs the suffix, never the event + pass + return error def inner_tap() -> InnerTap: diff --git a/tests/test_inner_tap.py b/tests/test_inner_tap.py index 1a70ce9..29437aa 100644 --- a/tests/test_inner_tap.py +++ b/tests/test_inner_tap.py @@ -39,6 +39,8 @@ from .test_utils import ( LEGACY_ONLY, + MCPSERVER_CRASH_TEXT_ON_WIRE, + MCPSERVER_CRASH_WRAPPER, MODERN_ONLY, NEEDS_CONCURRENT_DISPATCH, NEEDS_LOWLEVEL_ERROR_SEAM, @@ -783,7 +785,7 @@ async def test_mcpserver_publishes_the_tool_error_and_its_cause(self, events): assert result.is_error is True error = _one(events, "boom").error - assert error["type"] == "ToolError" + assert error["type"] == MCPSERVER_CRASH_WRAPPER assert "kaboom one" in error["message"] assert error["stack"] assert error["frames"] @@ -794,6 +796,53 @@ async def test_mcpserver_publishes_the_tool_error_and_its_cause(self, events): assert tool_frames and tool_frames[0]["in_app"] is True assert "raise Boom" in tool_frames[0]["context_line"] + @pytest.mark.asyncio + async def test_a_crash_publishes_the_same_message_on_every_generation( + self, events + ): + """mcp 2.1 keeps a crash's text off the wire; the event still has it.""" + from .test_utils.modern_server import create_modern_client + + server = _mcpserver_with_a_raising_tool() + track(server, "test_project", AgentCatOptions()) + + async with create_modern_client(server) as client: + await client.call_tool("boom", {"marker": "one"}) + + error = _one(events, "boom").error + assert error["message"] == "Error executing tool boom: kaboom one" + assert error["chained_errors"][0]["message"] == "kaboom one" + + @pytest.mark.asyncio + async def test_a_nested_crash_names_the_inner_tool_once(self, events): + """The restored text never repeats what the wrapper already carries.""" + from mcp.server.mcpserver import MCPServer + + from .test_utils.modern_server import create_modern_client + + server = MCPServer("nesting-server") + + @server.tool() + async def inner(marker: str) -> str: + """Always fails.""" + raise Boom(f"INNER {marker}") + + @server.tool() + async def outer(marker: str) -> str: + """Lets inner's failure through.""" + await server.call_tool("inner", {"marker": marker}) + return "unreachable" + + track(server, "test_project", AgentCatOptions()) + + async with create_modern_client(server) as client: + await client.call_tool("outer", {"marker": "one"}) + + message = _one(events, "outer").error["message"] + prefix = "Error executing tool outer: Error executing tool inner" + assert message.startswith(prefix) + assert message.count("Error executing tool inner") == 1 + @pytest.mark.asyncio async def test_tracking_the_lowlevel_object_directly_still_taps(self, events): """`track(mcpserver._lowlevel_server)` hands over no facade at all.""" @@ -805,7 +854,7 @@ async def test_tracking_the_lowlevel_object_directly_still_taps(self, events): async with create_modern_client(server) as client: await client.call_tool("boom", {"marker": "one"}) - assert _one(events, "boom").error["type"] == "ToolError" + assert _one(events, "boom").error["type"] == MCPSERVER_CRASH_WRAPPER @pytest.mark.asyncio async def test_a_repeated_track_does_not_stack_a_second_tap(self, events): @@ -822,7 +871,7 @@ async def test_a_repeated_track_does_not_stack_a_second_tap(self, events): await client.call_tool("boom", {"marker": "one"}) error = _one(events, "boom").error - assert error["type"] == "ToolError" + assert error["type"] == MCPSERVER_CRASH_WRAPPER assert len([f for f in error["frames"] if f["function"] == "tap"]) == 1 @pytest.mark.asyncio @@ -972,7 +1021,11 @@ async def outer(marker: str) -> str: result = await client.call_tool("outer", {"marker": "one"}) wire = "".join(c.text for c in result.content if hasattr(c, "text")) - assert "OUTER one" in wire and "INNER one" not in wire + # The crash's own text is on the wire only where upstream puts it + # (see MCPSERVER_CRASH_TEXT_ON_WIRE); the event carries it everywhere. + assert "tool outer" in wire and "INNER one" not in wire + if MCPSERVER_CRASH_TEXT_ON_WIRE: + assert "OUTER one" in wire error = _one(events, "outer").error assert "OUTER one" in error["message"] assert "INNER one" not in error["message"] diff --git a/tests/test_utils/__init__.py b/tests/test_utils/__init__.py index e1e1162..86d1bc0 100644 --- a/tests/test_utils/__init__.py +++ b/tests/test_utils/__init__.py @@ -91,6 +91,25 @@ ) +# mcp 2.1 split the MCPServer tool-crash wrapper out of `ToolError` into an +# `UnexpectedToolError` subclass and, in the same change, stopped copying the +# crash's own text into the wrapper: the model sees only `Error executing +# tool `, and the original rides `__cause__`. The tap restores that +# text on the published EVENT (`InnerTap.error`), so only the type name and +# the wire differ by generation. Probed on the symbol rather than the version +# so a backport is honoured; mcp 1.x has no `mcpserver` package at all and +# takes the same fallback, which `MODERN_ONLY` then skips. +try: + from mcp.server.mcpserver.exceptions import ( + UnexpectedToolError as _UnexpectedToolError, + ) +except ImportError: + MCPSERVER_CRASH_WRAPPER = "ToolError" + MCPSERVER_CRASH_TEXT_ON_WIRE = True +else: + MCPSERVER_CRASH_WRAPPER = _UnexpectedToolError.__name__ + MCPSERVER_CRASH_TEXT_ON_WIRE = False + # Whether the installed community FastMCP models an error result at all. # `ToolResult.is_error` arrived in fastmcp 3.4 (PR #4217); below it a plain # `ToolResult` dumps three fields and carries no error key in either spelling. From cce91c78832669f97ef6c1137395e58cc0cc1070 Mon Sep 17 00:00:00 2001 From: Naseem Alnaji Date: Sat, 29 Aug 2026 21:58:36 +0000 Subject: [PATCH 4/4] test: proxied community error reads the backend's block from the end fastmcp < 3.4 has no ToolResult.is_error, so an error result gets no mint-back in front; >= 3.4 does. The backend's own error block is last on every version, so index it from the end instead of assuming the mint-back. --- tests/test_inner_tap.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/test_inner_tap.py b/tests/test_inner_tap.py index 29437aa..39d7f08 100644 --- a/tests/test_inner_tap.py +++ b/tests/test_inner_tap.py @@ -1223,9 +1223,10 @@ async def test_a_proxied_upstream_error_keeps_the_surfaced_message(self, events) ) assert result.is_error is True - # content[0] is AgentCat's task mint-back, which every v2 result - # carries in front; the backend's own error block follows it. - upstream = result.content[1].text + # The backend's own error block is the last one: fastmcp >= 3.4 carries + # AgentCat's task mint-back in front of it, while earlier versions (no + # `ToolResult.is_error`) put no mint-back on an error result at all. + upstream = result.content[-1].text assert "kaboom one" in upstream error = _one(events, "boom").error assert error["message"] == upstream