diff --git a/CHANGES b/CHANGES index 9b8c2de3..70af95a6 100644 --- a/CHANGES +++ b/CHANGES @@ -18,6 +18,19 @@ $ uv add gp-sphinx --prerelease allow +### Fixes + +#### `sphinx-autodoc-fastmcp`: Tool cross-references survive reserved-label slugs + +A tool whose slug matches a Sphinx-reserved standard label — `search`, +`genindex`, or `modindex` — had its `{tool}` / `{toolref}` / +`{toolicon*}` cross-reference silently link to that built-in page (the +site search page, for `search`) instead of the tool's own card, and the +build stayed clean. Tool roles now resolve the canonical +`fastmcp-tool-` anchor first, so a reserved slug no longer hijacks +the link. A tool that appears only under `:no-index:` — and so has no +canonical cross-reference home — now warns instead of mis-linking. (#60) + ## gp-sphinx 0.0.1a31 (2026-06-15) ### What's new diff --git a/packages/sphinx-autodoc-fastmcp/src/sphinx_autodoc_fastmcp/_transforms.py b/packages/sphinx-autodoc-fastmcp/src/sphinx_autodoc_fastmcp/_transforms.py index c76aa9ab..072bb275 100644 --- a/packages/sphinx-autodoc-fastmcp/src/sphinx_autodoc_fastmcp/_transforms.py +++ b/packages/sphinx-autodoc-fastmcp/src/sphinx_autodoc_fastmcp/_transforms.py @@ -153,13 +153,38 @@ def resolve_tool_refs( target = node.get("reftarget", "") show_badge = node.get("show_badge", True) icon_pos = node.get("icon_pos", "") - label_info = domain.labels.get(target) + tool_name = target.replace("-", "_") + # Resolve the canonical section id first so a tool whose slug + # collides with a Sphinx-reserved label (``search``, ``genindex``, + # ``modindex``) still links to its own card. Those built-ins occupy + # ``StandardDomain`` labels before any document is read, so the tool's + # bare-slug alias can never claim them — the heading-collision path + # (#48) only resolves same-document slug clashes. Fall back to the + # bare slug for the back-compat ``{ref}`` aliases the directive + # registers. + canonical = f"fastmcp-tool-{target}" + label_info = domain.labels.get(canonical) or domain.labels.get(target) if label_info is None: - node.replace_self(nodes.literal("", target.replace("-", "_"))) + node.replace_self(nodes.literal("", tool_name)) continue todocname, labelid, _title = label_info - tool_name = target.replace("-", "_") + if tool_name in tool_data and labelid != canonical: + # A known tool resolved to a foreign label (e.g. the reserved + # ``search`` page): its only directive(s) carry ``:no-index:``, so + # it has no canonical cross-reference home and the link silently + # points elsewhere. Surface it instead of shipping a wrong link. + logger.warning( + "sphinx_autodoc_fastmcp: tool %r cross-reference resolved to " + "%s#%s, not its canonical section %r; ensure exactly one " + "`{fastmcp-tool} %s` directive omits ``:no-index:`` so the " + "tool has a cross-reference home", + tool_name, + todocname, + labelid, + canonical, + tool_name, + ) newnode = nodes.reference("", "", internal=True) try: diff --git a/tests/ext/fastmcp/test_fastmcp_integration.py b/tests/ext/fastmcp/test_fastmcp_integration.py index 0c52b396..37646288 100644 --- a/tests/ext/fastmcp/test_fastmcp_integration.py +++ b/tests/ext/fastmcp/test_fastmcp_integration.py @@ -312,3 +312,141 @@ def test_heading_collision_anchor_counts( """The heading owns the bare anchor; tool links target the canonical id (#48).""" html = read_output(fastmcp_heading_collision_result, "index.html") assert html.count(needle) == expected_count + + +_RESERVED_MODULE_SOURCE = textwrap.dedent( + """\ + from __future__ import annotations + + import types + + + def search(terms: str, limit: int = 20) -> str: + \"\"\"Search prompt records. + + Parameters + ---------- + terms : str + Terms to match. + limit : int + Maximum number of results. + \"\"\" + + return "[]" + + + search.__fastmcp__ = types.SimpleNamespace( + name="search", + title="Search", + tags={"readonly"}, + annotations=None, + ) + """ +) + +_RESERVED_CONF_PY = textwrap.dedent( + """\ + from __future__ import annotations + + import sys + + sys.path.insert(0, r"__SCENARIO_SRCDIR__") + + extensions = [ + "sphinx_autodoc_fastmcp", + ] + + fastmcp_tool_modules = ["reserved_tools"] + fastmcp_area_map = {"reserved_tools": "api"} + fastmcp_collector_mode = "introspect" + """ +) + +# ``search`` is a Sphinx built-in std label (the JS "Search Page"), seeded +# into StandardDomain before any document is read, so a tool's bare-slug +# alias can never claim it (#48 only covers same-document heading +# collisions). The tool role must resolve via the canonical +# ``fastmcp-tool-search`` section id, not silently link to the site search. +_RESERVED_INDEX_RST = textwrap.dedent( + """\ + Reserved slug tools + =================== + + Use :toolref:`search` for an inline link. + + .. fastmcp-tool:: reserved_tools.search + """ +) + + +@pytest.fixture(scope="module") +def fastmcp_reserved_slug_result( + tmp_path_factory: pytest.TempPathFactory, +) -> SharedSphinxResult: + """Build a page with a tool whose slug collides with a reserved label.""" + cache_root = tmp_path_factory.mktemp("fastmcp-reserved-slug") + scenario = SphinxScenario( + files=( + ScenarioFile("reserved_tools.py", _RESERVED_MODULE_SOURCE), + ScenarioFile( + "conf.py", + _RESERVED_CONF_PY.replace("__SCENARIO_SRCDIR__", SCENARIO_SRCDIR_TOKEN), + substitute_srcdir=True, + ), + ScenarioFile("index.rst", _RESERVED_INDEX_RST), + ), + ) + return build_shared_sphinx_result( + cache_root, + scenario, + purge_modules=("reserved_tools",), + ) + + +class ReservedSlugFixture(t.NamedTuple): + """Expected occurrence count for an href under a reserved-slug collision.""" + + test_id: str + needle: str + expected_count: int + + +_RESERVED_SLUG_FIXTURES: list[ReservedSlugFixture] = [ + # The tool role targets its own canonical card anchor, not Sphinx's + # reserved ``search`` built-in (the site-search page). + ReservedSlugFixture( + test_id="toolref-targets-canonical-anchor", + needle='class="reference internal" href="#fastmcp-tool-search"> None: + """A tool whose slug collides with a reserved label links to its card.""" + html = read_output(fastmcp_reserved_slug_result, "index.html") + assert html.count(needle) == expected_count diff --git a/tests/ext/fastmcp/test_transforms.py b/tests/ext/fastmcp/test_transforms.py index 427bc794..4d37a1ef 100644 --- a/tests/ext/fastmcp/test_transforms.py +++ b/tests/ext/fastmcp/test_transforms.py @@ -2,12 +2,20 @@ from __future__ import annotations +import logging +import types import typing as t +import pytest from docutils import nodes +from sphinx.application import Sphinx from sphinx_autodoc_fastmcp._css import _CSS -from sphinx_autodoc_fastmcp._transforms import collect_tool_section_content +from sphinx_autodoc_fastmcp._roles import _tool_ref_placeholder +from sphinx_autodoc_fastmcp._transforms import ( + collect_tool_section_content, + resolve_tool_refs, +) from sphinx_ux_autodoc_layout import build_api_component @@ -27,3 +35,88 @@ def test_collect_tool_section_content_appends_siblings_to_api_content() -> None: assert trailing.parent is content assert list(content.children) == [trailing] + + +def _resolve_single_tool_ref( + *, + reftarget: str, + labels: dict[str, tuple[str, str, str]], + tools: dict[str, object], + fromdocname: str = "index", +) -> nodes.Element: + """Run ``resolve_tool_refs`` over one placeholder against a stubbed app. + + Returns the container so the caller can inspect the replacement node. The + Sphinx ``app``/``builder``/``env`` are stubbed because the warning branch + fires only for a tool whose every directive carries ``:no-index:`` — a + state that is awkward to stage inside a full build but trivial to express + as a label table missing the canonical id. + """ + container = nodes.section() + container += _tool_ref_placeholder("", reftarget=reftarget, show_badge=False) + + std = types.SimpleNamespace(labels=labels, anonlabels={}) + builder = types.SimpleNamespace( + get_relative_uri=lambda _frm, todoc: f"{todoc}.html", + ) + env = types.SimpleNamespace( + domains=types.SimpleNamespace(standard_domain=std), + fastmcp_tools=tools, + ) + app = types.SimpleNamespace(env=env, builder=builder) + + resolve_tool_refs( + t.cast(Sphinx, app), + t.cast(nodes.document, container), + fromdocname, + ) + return container + + +def test_resolve_tool_refs_warns_when_known_tool_has_no_canonical_home( + caplog: pytest.LogCaptureFixture, +) -> None: + """A known tool resolving to a foreign label (reserved ``search``) warns.""" + with caplog.at_level(logging.WARNING, logger="sphinx_autodoc_fastmcp._transforms"): + container = _resolve_single_tool_ref( + reftarget="search", + labels={"search": ("search", "", "Search Page")}, + tools={"search": object()}, + ) + + records = [ + r for r in caplog.records if r.name == "sphinx_autodoc_fastmcp._transforms" + ] + assert len(records) == 1 + message = records[0].getMessage() + assert "'search'" in message + assert "canonical section 'fastmcp-tool-search'" in message + # The link still renders (to the foreign label) rather than vanishing. + assert isinstance(container[0], nodes.reference) + + +def test_resolve_tool_refs_silent_when_canonical_label_present( + caplog: pytest.LogCaptureFixture, +) -> None: + """No warning when the canonical ``fastmcp-tool-`` label exists.""" + with caplog.at_level(logging.WARNING, logger="sphinx_autodoc_fastmcp._transforms"): + container = _resolve_single_tool_ref( + reftarget="search", + labels={ + "search": ("search", "", "Search Page"), + "fastmcp-tool-search": ( + "mcp/tools", + "fastmcp-tool-search", + "search", + ), + }, + tools={"search": object()}, + ) + + warnings = [ + r for r in caplog.records if r.name == "sphinx_autodoc_fastmcp._transforms" + ] + assert warnings == [] + reference = container[0] + assert isinstance(reference, nodes.reference) + assert reference["refuri"] == "mcp/tools.html#fastmcp-tool-search"