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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,16 @@ frozen.
replay as before. A workstream that searched misses the prompt cache once, on its first request
after the update. One that searched again for tools it had already found resends each of those
searches, with the definitions it loaded, until it compacts.
- **A refused tool call says the tool is not available, not "Unknown tool" (#1287).** Tools can
leave a session mid-conversation: an MCP server drops one or goes away, or another user sends on
a shared workstream, whose tools follow the sender. When the model called one, it was told
"Unknown tool" and to use a listed name exactly, and the operator saw "Model called unknown
tool", as if the model had made the name up. Both now say the tool is not available now, and the
model is told it may have been removed or be misspelled, or, on a shared workstream, belong to
another user. The tools that reply lists are the ones the request offers, where it used to name
every built-in tool, including coordinator tools in a regular session, tools a persona hides and
tools an operator revoked. With tool search on, it lists the tools offered outright and says more
may be found by searching, and a task agent sees its own tools.
- **A storage error while opening a workstream no longer replaces its saved settings.** Building the
session saves its defaults when no settings are saved, and a read that failed counted as none
saved, so one failed read could overwrite the saved model, sampling, instructions and skill. That
Expand Down
10 changes: 7 additions & 3 deletions tests/test_coordinator_governance.py
Original file line number Diff line number Diff line change
Expand Up @@ -581,23 +581,27 @@ def test_prepare_tool_blocks_revoked_tool():
def test_prepare_tool_allows_non_revoked_tool():
"""The revocation gate must not fire on a tool name that isn't in
the revoked set. We pick a name that's also not in the preparers
dict so we can assert the 'unknown tool' result shape without
dict so we can assert the 'not available' result shape without
exercising a real preparer."""
from turnstone.core.session import ChatSession

session = ChatSession.__new__(ChatSession)
session._revoked_tools = frozenset({"spawn_workstream"})
session._mcp_client = None
session._shared_workstream = False
session.ui = MagicMock()

tc = {
"id": "call-2",
"function": {"name": "this_tool_is_not_registered", "arguments": "{}"},
}
item = session._prepare_tool(tc)
# Unknown tool path — not the revocation error path.
# A bare session has no model lane to derive its tool offer from, so the
# refusal is handed one.
item = session._prepare_tool_for_principal(tc, "", offered=frozenset())
# Not-available path — not the revocation error path.
err = str(item.get("error") or "")
assert "revoked" not in err.lower()
assert "is not available now" in err


# ---------------------------------------------------------------------------
Expand Down
31 changes: 14 additions & 17 deletions tests/test_mcp_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -723,9 +723,11 @@ def test_unknown_tool_without_mcp(self, tmp_db):
}
prepared = session._prepare_tool(tc)
assert "error" in prepared
assert "Unknown tool" in prepared["error"]
# Error lists available tools so the model can self-correct
assert "'nonexistent' is not available now" in prepared["error"]
# Error lists the offered tools so the model can self-correct...
assert "bash" in prepared["error"]
# ...and only those: an interactive session offers no coordinator tools.
assert "spawn_workstream" not in prepared["error"]
# Surfaces warning to user
session.ui.on_error.assert_called_once()
assert "nonexistent" in session.ui.on_error.call_args[0][0]
Expand Down Expand Up @@ -890,34 +892,29 @@ def test_session_close_removes_listener_with_same_user_id(self, tmp_db):
assert registered_cb is removed_cb

def test_session_unknown_tool_lists_user_scoped_catalog(self, tmp_db):
"""The "Unknown tool" error message lists tools the session can
actually invoke — drawn from the merged user-scoped catalog,
not the manager's private static-only ``_tool_map``."""
"""The refusal of a tool dispatch cannot run lists the MCP tools the
session offers — drawn from the user's merged catalog, static and
pool entries alike."""
mock_mcp = MagicMock()
# Pretend the user's merged view contains a static + pool entry.
mock_mcp.get_tools.return_value = [
# A static entry everyone sees, plus a pool entry only in user-7's view.
mock_mcp.get_tools.side_effect = lambda user_id=None: [
_fake_openai_tool("mcp__static__list"),
_fake_openai_tool("mcp__pool-srv__do"),
*([_fake_openai_tool("mcp__pool-srv__do")] if user_id == "user-7" else []),
]
mock_mcp.is_mcp_tool.return_value = False
session = self._make_session(mcp_client=mock_mcp, user_id="user-7")
# Reset the call counter so we observe only the _prepare_tool call.
mock_mcp.get_tools.reset_mock()
# Tool search off, so the MCP tools are offered outright, not deferred.
session = self._make_session(mcp_client=mock_mcp, user_id="user-7", tool_search="off")

tc = {
"id": "call_unknown",
"function": {"name": "no_such_tool", "arguments": "{}"},
}
prepared = session._prepare_tool(tc)
assert "error" in prepared
# The error mentions both static and pool tools — proves we're
# consulting the merged catalog rather than ``_tool_map``.
# Both entries are listed, and the pool one exists only in user-7's
# view: the list is this user's merged catalog.
assert "mcp__static__list" in prepared["error"]
assert "mcp__pool-srv__do" in prepared["error"]
# And the catalog request was scoped to this session's user.
assert any(
call.kwargs.get("user_id") == "user-7" for call in mock_mcp.get_tools.call_args_list
)


# ---------------------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion tests/test_task_agent_compaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ def test_estimator_observe_without_served_figure_keeps_ratio_and_own_estimate()
assert estimator.estimate(messages) == own_estimate


def _prepared_tool(tool_call: dict[str, Any], _principal: str):
def _prepared_tool(tool_call: dict[str, Any], _principal: str, **_kwargs: Any):
call_id = tool_call["id"]
return {
"call_id": call_id,
Expand Down
209 changes: 209 additions & 0 deletions tests/test_unavailable_tool_refusal.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,209 @@
"""The refusal of a tool call that dispatch cannot run (#1287).

A session's catalog can change mid-conversation: an MCP server drops a tool or goes away, or
another user sends on a shared workstream and the catalog follows the sender. The name the model
calls may be one it saw earlier, so the refusal says the tool is not available now, and lists only
the tools the request offers, leaving deferred tools to tool search.
"""

from __future__ import annotations

from typing import Any
from unittest.mock import MagicMock, patch

import pytest

from tests._session_helpers import make_result, make_session
from turnstone.core.personas import PersonaSnapshot
from turnstone.core.providers._protocol import ModelCapabilities
from turnstone.core.trajectory import Role, Turn

GONE = "mcp__gone__lookup"


def _tool(name: str) -> dict[str, Any]:
return {
"type": "function",
"function": {
"name": name,
"description": f"The {name} tool.",
"parameters": {"type": "object", "properties": {}},
},
}


def _call(name: str = GONE) -> dict[str, Any]:
return {"id": "call-gone", "type": "function", "function": {"name": name, "arguments": "{}"}}


def _mcp(*names: str) -> MagicMock:
mcp = MagicMock()
mcp.get_tools.return_value = [_tool(name) for name in names]
# Dispatch finds no MCP tool by the called name: it left the catalog.
mcp.is_mcp_tool.return_value = False
mcp.resource_count_for_user.return_value = 0
mcp.prompt_count_for_user.return_value = 0
return mcp


def _listed(error: str) -> list[str]:
return error.split("Other tools offered now: ", 1)[1].split(".", 1)[0].split(", ")


def test_refusal_says_the_tool_is_gone_and_lists_the_offer(tmp_db) -> None:
ui = MagicMock()
session = make_session(ui=ui, mcp_client=_mcp("mcp__docs__read"), tool_search="off")

item = session._prepare_tool(_call())

error = item["error"]
# A single-user workstream: another user cannot be the cause, so it is not named.
assert error.startswith(
f"Tool '{GONE}' is not available now. It may have been removed since it was offered, "
"or its name may be misspelled."
)
assert error.endswith("To call one, use its name exactly as listed.")
listed = _listed(error)
assert "bash" in listed
assert "mcp__docs__read" in listed
# A coordinator tool has a preparer, but an interactive session does not offer it.
assert "spawn_workstream" not in listed
assert "tool search" not in error
assert item["header"].endswith(f"{GONE}: not available")
ui.on_error.assert_called_once_with(f"Model called a tool that is not available now: '{GONE}'")


def test_a_shared_workstream_names_another_user_as_a_cause(tmp_db) -> None:
session = make_session(mcp_client=_mcp("mcp__docs__read"), tool_search="off")
# Latched once a non-owner speaks; the catalog then follows the sender.
session._shared_workstream = True

error = session._prepare_tool(_call())["error"]

assert error.startswith(
f"Tool '{GONE}' is not available now. It may have been removed since it was offered, "
"or be available only to another user of this workstream, or its name may be misspelled."
)


def test_revoked_tools_are_not_offered_as_alternatives(tmp_db) -> None:
session = make_session(mcp_client=_mcp("mcp__docs__read"), tool_search="off")
# Revocation leaves the tool on the wire; dispatch refuses it.
session.revoke_tools({"bash"})

listed = _listed(session._prepare_tool(_call())["error"])

assert "bash" not in listed
assert "read_file" in listed


def test_an_offer_of_nothing_else_says_so(tmp_db) -> None:
session = make_session()

error = session._prepare_tool_for_principal(_call(), "user-a", offered=frozenset({GONE}))[
"error"
]

# An offered name is no misspelling, and with nothing else listed there is nothing to call.
assert error == (
f"Tool '{GONE}' is not available now. It may have been removed since it was offered. "
"No other tools are offered now."
)


def test_a_persona_refusal_lists_only_the_tools_its_set_shows(tmp_db) -> None:
persona = PersonaSnapshot(
name="guard", prompt="", tools=frozenset({"bash", "read_file"}), mcp=True, memory=True
)
session = make_session(mcp_client=_mcp("mcp__docs__read"), persona_snapshot=persona)

error = session._prepare_tool(_call())["error"]

assert "Other tools offered now: bash, read_file." in error
assert "tool search" not in error


@pytest.mark.parametrize("native", [True, False], ids=["native", "client-side"])
def test_deferred_tools_are_left_to_tool_search(tmp_db, native: bool) -> None:
session = make_session(mcp_client=_mcp("mcp__docs__read", "mcp__docs__write"), tool_search="on")
assert session._tool_search is not None
session._tool_search.expand_visible(["mcp__docs__read"])
caps = ModelCapabilities(supports_tool_search=native)

with patch.object(session, "_get_capabilities", return_value=caps):
error = session._prepare_tool(_call())["error"]

listed = _listed(error)
# A discovered tool is offered outright; a deferred one is not listed.
assert "mcp__docs__read" in listed
assert "mcp__docs__write" not in listed
# Client-side search is itself an offered tool; native search is the provider's.
assert ("tool_search" in listed) is not native
assert "More tools may be found with tool search." in error


def test_a_task_agent_is_shown_its_own_tools(tmp_db) -> None:
session = make_session()
contexts: list[list[Turn]] = []
responses = [
make_result("", tool_calls=[_call()], finish_reason="tool_calls"),
make_result("done"),
]

def plant(_lane: Any, turns: list[Turn], **_kwargs: Any) -> Any:
contexts.append(list(turns))
return responses.pop(0)

with (
patch.object(session, "_context_window_for_lane", return_value=1_000_000),
patch("turnstone.core.session.model_turn", side_effect=plant),
):
session._run_agent(
[Turn.system("task identity"), Turn.user("look it up")],
label="task",
tools=[_tool("read_file"), _tool(GONE)],
auto_tools={"read_file"},
parent_call_id="task-parent",
principal_id="user-a",
)

assert responses == []
[result] = [turn for turn in contexts[1] if turn.role == Role.TOOL]
assert f"Tool '{GONE}' is not available now." in result.text
# The agent was offered that exact name, so it is no misspelling.
assert "misspelled" not in result.text
# The agent's list, not the session's: the session offers bash, the agent does not.
assert "Other tools offered now: read_file." in result.text


def test_an_agents_offer_does_not_outlive_its_call(tmp_db) -> None:
session = make_session()

agent_item = session._prepare_tool_for_principal(
_call(), "user-a", offered=frozenset({"read_file"})
)
session_item = session._prepare_tool(_call())

assert "Other tools offered now: read_file." in agent_item["error"]
assert "bash" in session_item["error"]


def test_deferred_names_survive_the_catalog_dropping_tool_search_mid_read(tmp_db) -> None:
# The refusal and every request read the deferred names. An MCP catalog refresh on another
# thread can drop the tool search manager at any point; here it lands right after the
# manager's truth test, the window a free-threaded build (or a caller passing no caps) opens.
session = make_session()

class DroppedMidRead:
def __bool__(self) -> bool:
session._tool_search = None
return True

def get_deferred_tools(self) -> list[dict[str, Any]]:
return [_tool("mcp__docs__write")]

session._tool_search = DroppedMidRead() # type: ignore[assignment]

names = session._get_deferred_names(ModelCapabilities(supports_tool_search=True))

assert names == frozenset({"mcp__docs__write"})
Loading
Loading