rsp2k
daec2d9e72
Tests / test (==2.12.2) (push) Canceled after 0s
Tests / test (==2.14.3) (push) Canceled after 0s
Tests / test (>=3.3,<4) (push) Canceled after 0s
Tests / test (>=4,<5) (push) Canceled after 0s
Tests / test-postgres (==2.12.2) (push) Canceled after 0s
Tests / test-postgres (>=4,<5) (push) Canceled after 0s
Tests / canary (push) Canceled after 0s
The sink ships with a JSONB variant and is documented for asyncpg, but the suite only ever exercised SQLite. The embeddings work will need pgvector, so this adds a local Postgres stack and a way to run the database tests on it, here and in CI. - docker-compose.yml: ffb-postgres (pgvector/pgvector:pg16) on a private network, published on 127.0.0.1 only, with an init script that adds the vector extension to the test database and template1 - Makefile: pg-up/pg-down/pg-reset/pg-logs/pg-psql, test, test-pg, test-all, lint; .env is generated from .env.example with a random password, and FFB_TEST_DATABASE_URL from the environment wins over it - tests: a db_url fixture parametrized over sqlite and postgres. The postgres leg skips unless FFB_TEST_DATABASE_URL is set, and creates a uniquely named database per test (with vector) that is dropped after. The DatabaseSink factories in the events, retention, correlation, soft-error and instrumentation tests now use it, so their 38 database tests run on both. The missing-events-table test accepts either backend's error. New tests check isolation and that vector works. - asyncpg and pgvector added to the dev and test extras - CI: a test-postgres job on FastMCP 2.12.2 and 4.x with a pgvector service container, failing if the Postgres cases skip - CONTRIBUTING: how to run the suite against Postgres
417 lines
14 KiB
Python
417 lines
14 KiB
Python
"""Tests for linking feedback to the tool calls that preceded it."""
|
|
|
|
import asyncio
|
|
import contextlib
|
|
import json
|
|
import sys
|
|
from datetime import UTC, datetime, timedelta
|
|
from pathlib import Path
|
|
|
|
import fastmcp
|
|
import pytest
|
|
from fastmcp import Client, Context, FastMCP
|
|
from fastmcp.client.transports import PythonStdioTransport
|
|
from fastmcp.exceptions import ToolError
|
|
|
|
from fastmcp_feedback import add_feedback_tools
|
|
from fastmcp_feedback.instrumentation import (
|
|
DatabaseSink,
|
|
MemorySink,
|
|
instrument,
|
|
)
|
|
from fastmcp_feedback.instrumentation import middleware as mw_module
|
|
from fastmcp_feedback.instrumentation.correlation import (
|
|
RULE_SESSION,
|
|
RULE_USER_WINDOW,
|
|
Candidate,
|
|
select_calls,
|
|
)
|
|
|
|
NOW = datetime(2026, 9, 27, 12, 0, tzinfo=UTC)
|
|
|
|
# FastMCP 4's Client speaks MCP 2026-07-28, which has no sessions, so each
|
|
# in-memory call gets a new ctx.session_id and there is no header or stdio
|
|
# signal to fall back on. Calls from one test client link by user instead.
|
|
FASTMCP_4 = int(fastmcp.__version__.split(".")[0]) >= 4
|
|
SAME_CLIENT_RULE = RULE_USER_WINDOW if FASTMCP_4 else RULE_SESSION
|
|
|
|
|
|
@pytest.fixture
|
|
async def db(db_url):
|
|
"""Make DatabaseSinks whose engines are always disposed, even on failure.
|
|
|
|
An undisposed aiosqlite engine keeps a worker thread alive and the test
|
|
process never exits.
|
|
"""
|
|
made = []
|
|
|
|
def factory(name="calls", cls=DatabaseSink):
|
|
sink = cls(db_url(name), create_tables=True)
|
|
made.append(sink)
|
|
return sink
|
|
|
|
yield factory
|
|
for sink in made:
|
|
await sink.engine.dispose()
|
|
|
|
|
|
def cand(id, minutes_ago, session="s1", user="u1", tool="t", ok=True):
|
|
return Candidate(id, tool, NOW - timedelta(minutes=minutes_ago), ok, session, user)
|
|
|
|
|
|
class TestSelectCalls:
|
|
def test_session_first_then_user_window_chronological(self):
|
|
links = select_calls(
|
|
[cand("a", 1), cand("b", 3, session="s2"), cand("c", 2)],
|
|
session_id="s1",
|
|
user_sub="u1",
|
|
since=NOW - timedelta(minutes=15),
|
|
limit=10,
|
|
)
|
|
assert [(link.call_id, link.rule, link.position) for link in links] == [
|
|
("b", RULE_USER_WINDOW, 0),
|
|
("c", RULE_SESSION, 1),
|
|
("a", RULE_SESSION, 2),
|
|
]
|
|
|
|
def test_session_matches_take_priority_over_newer_user_calls(self):
|
|
links = select_calls(
|
|
[cand("old", 10), cand("new", 1, session="s2")],
|
|
session_id="s1",
|
|
user_sub="u1",
|
|
since=NOW - timedelta(minutes=15),
|
|
limit=1,
|
|
)
|
|
assert [(link.call_id, link.rule) for link in links] == [("old", RULE_SESSION)]
|
|
|
|
def test_user_window_excludes_old_calls_and_other_users(self):
|
|
links = select_calls(
|
|
[
|
|
cand("recent", 5, session="s2"),
|
|
cand("stale", 30, session="s2"),
|
|
cand("other", 1, session="s3", user="u2"),
|
|
],
|
|
session_id="s9",
|
|
user_sub="u1",
|
|
since=NOW - timedelta(minutes=15),
|
|
limit=10,
|
|
)
|
|
assert [link.call_id for link in links] == ["recent"]
|
|
|
|
def test_limit_keeps_the_most_recent(self):
|
|
links = select_calls(
|
|
[cand(str(i), i) for i in range(10)],
|
|
session_id="s1",
|
|
user_sub=None,
|
|
since=NOW,
|
|
limit=3,
|
|
)
|
|
assert [link.call_id for link in links] == ["2", "1", "0"]
|
|
|
|
def test_duplicates_counted_once_and_naive_times_treated_as_utc(self):
|
|
naive = Candidate(
|
|
"a",
|
|
"t",
|
|
(NOW - timedelta(minutes=1)).replace(tzinfo=None),
|
|
True,
|
|
"s1",
|
|
"u1",
|
|
)
|
|
links = select_calls(
|
|
[naive, cand("a", 1)], session_id="s1", user_sub=None, since=NOW, limit=5
|
|
)
|
|
assert len(links) == 1 and links[0].started_at.tzinfo is not None
|
|
|
|
def test_no_session_or_user_means_no_links(self):
|
|
assert (
|
|
select_calls(
|
|
[cand("a", 1)], session_id=None, user_sub=None, since=NOW, limit=5
|
|
)
|
|
== []
|
|
)
|
|
|
|
|
|
def build(sinks=None, user="u1", **kwargs):
|
|
app = FastMCP("correlated")
|
|
|
|
@app.tool
|
|
def echo(text: str) -> str:
|
|
return text
|
|
|
|
@app.tool
|
|
def render(scene: str) -> str:
|
|
raise RuntimeError(f"render of {scene} hung")
|
|
|
|
captured = {}
|
|
|
|
@app.tool
|
|
async def report(ctx: Context) -> list:
|
|
links = await mw.capture_recent_calls(ctx)
|
|
captured["links"] = links
|
|
return [link.call_id for link in links]
|
|
|
|
identity = {"user": user}
|
|
mw = instrument(
|
|
app,
|
|
sinks if sinks is not None else [MemorySink()],
|
|
identity_resolver=lambda c: {"user_sub": identity["user"]},
|
|
**kwargs,
|
|
)
|
|
return app, mw, captured, identity
|
|
|
|
|
|
class TestCaptureRecentCalls:
|
|
async def test_same_session_calls_including_failures(self):
|
|
app, mw, captured, _ = build()
|
|
async with Client(app) as client:
|
|
await client.call_tool("echo", {"text": "a"})
|
|
with contextlib.suppress(ToolError):
|
|
await client.call_tool("render", {"scene": "castle"})
|
|
await client.call_tool("report", {})
|
|
|
|
links = captured["links"]
|
|
assert [(link.tool, link.ok, link.rule) for link in links] == [
|
|
("echo", True, SAME_CLIENT_RULE),
|
|
("render", False, SAME_CLIENT_RULE),
|
|
]
|
|
|
|
async def test_new_session_per_call_falls_back_to_user_window(self):
|
|
"""PAT/CI clients open a new MCP session for every call."""
|
|
app, mw, captured, _ = build()
|
|
for text in ("one", "two"):
|
|
async with Client(app) as client:
|
|
await client.call_tool("echo", {"text": text})
|
|
async with Client(app) as client:
|
|
await client.call_tool("report", {})
|
|
|
|
assert [link.rule for link in captured["links"]] == [RULE_USER_WINDOW] * 2
|
|
|
|
async def test_other_users_calls_are_not_linked(self):
|
|
app, mw, captured, identity = build()
|
|
identity["user"] = "someone-else"
|
|
async with Client(app) as client:
|
|
await client.call_tool("echo", {"text": "not mine"})
|
|
identity["user"] = "u1"
|
|
async with Client(app) as client:
|
|
await client.call_tool("report", {})
|
|
assert captured["links"] == []
|
|
|
|
async def test_calls_still_queued_are_found(self):
|
|
release = asyncio.Event()
|
|
|
|
class Stalled:
|
|
async def write(self, records):
|
|
await release.wait()
|
|
|
|
app, mw, captured, _ = build(sinks=[Stalled()])
|
|
async with Client(app) as client:
|
|
await client.call_tool("echo", {"text": "queued"})
|
|
await client.call_tool("report", {})
|
|
release.set()
|
|
assert [link.tool for link in captured["links"]] == ["echo"]
|
|
|
|
async def test_database_fallback_after_restart(self, db):
|
|
app1, mw1, _, _ = build(sinks=[db("restart")])
|
|
async with Client(app1) as client:
|
|
await client.call_tool("echo", {"text": "before restart"})
|
|
await mw1.aclose()
|
|
|
|
# A new process: empty in-memory buffer, same database.
|
|
app2, mw2, captured, _ = build(sinks=[db("restart")])
|
|
async with Client(app2) as client:
|
|
await client.call_tool("report", {})
|
|
await mw2.aclose()
|
|
|
|
[link] = captured["links"]
|
|
assert link.tool == "echo" and link.rule == RULE_USER_WINDOW
|
|
|
|
@pytest.mark.skipif(
|
|
FASTMCP_4, reason="FastMCP 4 Client is sessionless (MCP 2026-07-28)"
|
|
)
|
|
async def test_failing_resolver_still_links_by_session(self):
|
|
app, mw, captured, _ = build()
|
|
mw.identity_resolver = lambda c: 1 / 0
|
|
async with Client(app) as client:
|
|
await client.call_tool("echo", {"text": "a"})
|
|
await client.call_tool("report", {})
|
|
assert [link.rule for link in captured["links"]] == [RULE_SESSION]
|
|
|
|
async def test_slow_database_is_skipped(self, db):
|
|
class SlowDb(DatabaseSink):
|
|
async def recent_calls(self, **kwargs):
|
|
await asyncio.sleep(5)
|
|
return []
|
|
|
|
sink = db("slow", SlowDb)
|
|
app, mw, captured, _ = build(sinks=[sink], correlation_timeout=0.05)
|
|
async with Client(app) as client:
|
|
await client.call_tool("echo", {"text": "a"})
|
|
await client.call_tool("report", {})
|
|
await mw.aclose()
|
|
# The lookup timed out, so capture returned [] rather than hanging.
|
|
assert captured["links"] == []
|
|
|
|
async def test_outside_a_tool_call_returns_empty(self):
|
|
_, mw, _, _ = build()
|
|
mw.identity_resolver = None
|
|
assert await mw.capture_recent_calls() == []
|
|
|
|
|
|
class TestLinksAndContext:
|
|
async def test_link_and_read_back_with_external_ids(self, db):
|
|
sink = db("links")
|
|
app, mw, captured, _ = build(sinks=[sink])
|
|
async with Client(app) as client:
|
|
await client.call_tool("echo", {"text": "a"})
|
|
with contextlib.suppress(ToolError):
|
|
await client.call_tool("render", {"scene": "castle"})
|
|
await client.call_tool("report", {})
|
|
|
|
assert await mw.link_feedback("bug-7Q2X", captured["links"])
|
|
await mw.flush()
|
|
context = await mw.feedback_context("bug-7Q2X")
|
|
await mw.aclose()
|
|
|
|
assert [(c["position"], c["tool"], c["ok"], c["rule"]) for c in context] == [
|
|
(0, "echo", True, SAME_CLIENT_RULE),
|
|
(1, "render", False, SAME_CLIENT_RULE),
|
|
]
|
|
assert context[1]["error_type"] == "RuntimeError"
|
|
|
|
async def test_relinking_replaces_links(self, db):
|
|
sink = db("relink")
|
|
app, mw, captured, _ = build(sinks=[sink])
|
|
async with Client(app) as client:
|
|
await client.call_tool("echo", {"text": "a"})
|
|
await client.call_tool("report", {})
|
|
links = captured["links"]
|
|
await mw.link_feedback("fb-1", links)
|
|
await mw.link_feedback("fb-1", links)
|
|
assert len(await mw.feedback_context("fb-1")) == 1
|
|
await mw.aclose()
|
|
|
|
async def test_context_fills_unflushed_calls_from_memory(self, db):
|
|
release = asyncio.Event()
|
|
|
|
class StalledDb(DatabaseSink):
|
|
async def write(self, records):
|
|
await release.wait()
|
|
|
|
sink = db("unflushed", StalledDb)
|
|
app, mw, captured, _ = build(sinks=[sink])
|
|
async with Client(app) as client:
|
|
await client.call_tool("echo", {"text": "a"})
|
|
await client.call_tool("report", {})
|
|
await mw.link_feedback("fb-2", captured["links"])
|
|
|
|
[row] = await mw.feedback_context("fb-2")
|
|
release.set()
|
|
await mw.aclose()
|
|
assert row["tool"] == "echo" and row["ok"] is True
|
|
|
|
async def test_link_without_database_sink_is_false(self):
|
|
_, mw, _, _ = build()
|
|
assert await mw.link_feedback("x", []) is False
|
|
assert await mw.feedback_context("x") == []
|
|
|
|
|
|
class TestSubmitFeedbackAutoLinks:
|
|
async def test_submission_links_preceding_calls(self, db):
|
|
app = FastMCP("app")
|
|
|
|
@app.tool
|
|
def render(scene: str) -> str:
|
|
raise RuntimeError("hung")
|
|
|
|
sink = db("submit")
|
|
mw = instrument(app, [sink], identity_resolver=lambda c: {"user_sub": "u1"})
|
|
add_feedback_tools(app, database_url="sqlite:///:memory:", instrumentation=mw)
|
|
|
|
async with Client(app) as client:
|
|
with contextlib.suppress(ToolError):
|
|
await client.call_tool("render", {"scene": "castle"})
|
|
result = await client.call_tool(
|
|
"submit_feedback",
|
|
{
|
|
"request": {
|
|
"type": "bug",
|
|
"title": "render hangs",
|
|
"description": "the render tool hung",
|
|
"submitter": "model",
|
|
}
|
|
},
|
|
)
|
|
|
|
assert result.data["linked_calls"] == 1
|
|
await mw.flush()
|
|
[row] = await mw.feedback_context(result.data["feedback_id"])
|
|
await mw.aclose()
|
|
assert (row["tool"], row["ok"], row["error_type"]) == (
|
|
"render",
|
|
False,
|
|
"RuntimeError",
|
|
)
|
|
|
|
async def test_without_instrumentation_response_unchanged(self):
|
|
app = FastMCP("plain")
|
|
add_feedback_tools(app, database_url="sqlite:///:memory:")
|
|
async with Client(app) as client:
|
|
result = await client.call_tool(
|
|
"submit_feedback",
|
|
{
|
|
"request": {
|
|
"type": "bug",
|
|
"title": "t",
|
|
"description": "no instrumentation",
|
|
"submitter": "m",
|
|
}
|
|
},
|
|
)
|
|
assert "linked_calls" not in result.data
|
|
|
|
|
|
class FakeContext:
|
|
def __init__(self, transport=None, session_id="ctx-session"):
|
|
self.transport = transport
|
|
self.session_id = session_id
|
|
|
|
|
|
class TestSessionKey:
|
|
def test_http_session_header_wins(self, monkeypatch):
|
|
import fastmcp.server.dependencies as deps
|
|
|
|
monkeypatch.setattr(
|
|
deps,
|
|
"get_http_headers",
|
|
lambda include_all=False: {"mcp-session-id": "hdr-1"},
|
|
)
|
|
assert (
|
|
mw_module.session_key(FakeContext(transport="streamable-http")) == "hdr-1"
|
|
)
|
|
|
|
def test_stdio_uses_one_key_per_process(self):
|
|
a = mw_module.session_key(FakeContext(transport="stdio", session_id="x"))
|
|
b = mw_module.session_key(FakeContext(transport="stdio", session_id="y"))
|
|
assert a == b and a.startswith("stdio:")
|
|
|
|
def test_falls_back_to_ctx_session_id(self):
|
|
assert mw_module.session_key(FakeContext()) == "ctx-session"
|
|
|
|
def test_no_context(self):
|
|
assert mw_module.session_key(None) is None
|
|
|
|
|
|
class TestStdioEndToEnd:
|
|
async def test_same_stdio_client_links_by_session(self):
|
|
"""On a real stdio transport, one client's calls link by session on every FastMCP version."""
|
|
server = (
|
|
Path(__file__).parent.parent / "fixtures" / "stdio_correlation_server.py"
|
|
)
|
|
transport = PythonStdioTransport(str(server), python_cmd=sys.executable)
|
|
async with Client(transport) as client:
|
|
await client.call_tool("echo", {"text": "a"})
|
|
await client.call_tool("echo", {"text": "b"})
|
|
result = await client.call_tool("report", {})
|
|
pairs = json.loads(result.content[0].text)
|
|
assert pairs == [["echo", RULE_SESSION], ["echo", RULE_SESSION]]
|