The gitea-workflow documentation named a gitea_edit_issue tool that no
namespace had ever registered. There was no sanctioned MCP path to change an
issue title or body at all: the only edit tool, gitea_edit_pr, PATCHes the
pull-request endpoint. An authorized body correction on issue #780 therefore
had to be recorded as a discussion comment instead.
Add edit_issue.py as the authoritative rule and gitea_edit_issue as the tool
built on it:
- Only the fields the caller names are sent, so labels, state, assignee, and
milestone cannot be overwritten from a stale read.
- A pull-request number is refused. Gitea serves pull requests from the same
/issues/{n} collection, so without that check the issue path would quietly
become a second, ungated PR edit path. gitea_edit_pr stays PR-only.
- Structurally invalid requests raise before any credential or network work;
a request that would change nothing is reported as an explicit no-op with a
next action rather than a silent success.
- Read-after-write proves the applied title/body and proves that state,
labels, assignees, and milestone did not move. Transport failures on the
pre-read, the PATCH, and the read-back are each reported, redacted, with
the correct performed/verified state.
Gates match every other issue mutation: profile permission via the shared
capability map (resolver task edit_issue, gitea.issue.comment), preflight
purity, branches worktree validation, anti-stomp inventory membership, and
audited mutation.
Fix the drift that hid this. docs/mcp-tool-inventory.md is now the canonical
registered-tool list, and mcp_tool_inventory.py compares it to the live
registry in both directions, plus checks that every tool named under skills/
is registered. The new guard immediately found a second instance of the same
defect: gitea_record_pre_review_command had lost its @mcp.tool() decorator
while the canonical review workflow still instructed reviewers to call it, so
that registration is restored.
Validation:
PASSED: venv/bin/python -m pytest tests/test_issue_781_edit_issue_tool.py -s -q
— 50 passed
FAILED: venv/bin/python -m pytest -s -q — 4095 passed, 11 failed, 6 skipped.
The identical 11 tests fail on clean master 8e149e6 with no changes applied
(254 passed, 11 failed across those five files), so they are pre-existing
and proven by a baseline run rather than asserted.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
196 lines
6.9 KiB
Python
196 lines
6.9 KiB
Python
"""Documented-vs-registered MCP tool inventory guard (#781).
|
|
|
|
The workflow documentation named a ``gitea_edit_issue`` tool that no namespace
|
|
had ever registered. Nothing compared the two lists, so an actor could plan a
|
|
mutation against a tool that did not exist and only discover it at execution
|
|
time — after the work was already scoped around it.
|
|
|
|
This module is that comparison, in two directions:
|
|
|
|
- :func:`assess_inventory_drift` compares the canonical inventory documented in
|
|
``docs/mcp-tool-inventory.md`` against the tools actually registered on the
|
|
MCP server. Either list drifting fails the guard, so a new tool must be
|
|
documented and a removed tool must be undocumented in the same change.
|
|
- :func:`assess_doc_references` catches the original defect directly: any tool
|
|
name a workflow/skill document tells an actor to call must be registered.
|
|
|
|
Module and script names legitimately appear in the same prose (``gitea_auth``,
|
|
``offline_mcp_runner``), so :data:`NON_TOOL_IDENTIFIERS` names the known
|
|
non-tool identifiers explicitly rather than loosening the pattern.
|
|
|
|
This module performs no I/O — callers own reading the files.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import re
|
|
from typing import Any, Iterable, Mapping
|
|
|
|
#: Canonical documented inventory, relative to the repository root.
|
|
INVENTORY_DOC_PATH = "docs/mcp-tool-inventory.md"
|
|
|
|
#: Delimiters around the generated inventory list in the doc.
|
|
INVENTORY_BEGIN_MARKER = "<!-- BEGIN REGISTERED TOOL INVENTORY -->"
|
|
INVENTORY_END_MARKER = "<!-- END REGISTERED TOOL INVENTORY -->"
|
|
|
|
#: Backticked identifiers that look like tool names but are modules/scripts.
|
|
#: Every entry is a real file in this repository, not an MCP tool.
|
|
NON_TOOL_IDENTIFIERS: frozenset[str] = frozenset(
|
|
{
|
|
"gitea_auth",
|
|
"gitea_config",
|
|
"gitea_mcp_server",
|
|
"mcp_server",
|
|
"offline_mcp_helper",
|
|
"offline_mcp_runner",
|
|
}
|
|
)
|
|
|
|
#: Prefixes that mark an identifier as a candidate MCP tool name.
|
|
TOOL_NAME_PREFIXES: tuple[str, ...] = ("gitea_", "mcp_")
|
|
|
|
_INVENTORY_ENTRY = re.compile(r"^-\s+`([A-Za-z_][A-Za-z0-9_]*)`")
|
|
_BACKTICKED = re.compile(r"`([A-Za-z_][A-Za-z0-9_]*)`")
|
|
|
|
|
|
def parse_documented_inventory(text: str) -> list[str]:
|
|
"""Return the tool names listed between the inventory markers.
|
|
|
|
Raises ``ValueError`` when the markers are missing or out of order, so a
|
|
mangled document fails the guard instead of silently documenting nothing.
|
|
"""
|
|
start = text.find(INVENTORY_BEGIN_MARKER)
|
|
end = text.find(INVENTORY_END_MARKER)
|
|
if start == -1 or end == -1 or end < start:
|
|
raise ValueError(
|
|
f"{INVENTORY_DOC_PATH} must contain "
|
|
f"'{INVENTORY_BEGIN_MARKER}' followed by "
|
|
f"'{INVENTORY_END_MARKER}' (fail closed)."
|
|
)
|
|
block = text[start + len(INVENTORY_BEGIN_MARKER) : end]
|
|
names: list[str] = []
|
|
for line in block.splitlines():
|
|
match = _INVENTORY_ENTRY.match(line.strip())
|
|
if match:
|
|
names.append(match.group(1))
|
|
return names
|
|
|
|
|
|
def looks_like_tool_name(identifier: str) -> bool:
|
|
"""Return whether a backticked identifier is a candidate tool name."""
|
|
if identifier in NON_TOOL_IDENTIFIERS:
|
|
return False
|
|
return identifier.startswith(TOOL_NAME_PREFIXES)
|
|
|
|
|
|
def extract_tool_references(text: str) -> set[str]:
|
|
"""Return candidate tool names a document tells an actor to call."""
|
|
return {
|
|
name
|
|
for name in _BACKTICKED.findall(text)
|
|
if looks_like_tool_name(name)
|
|
}
|
|
|
|
|
|
def assess_inventory_drift(
|
|
documented: Iterable[str],
|
|
registered: Iterable[str],
|
|
) -> dict[str, Any]:
|
|
"""Compare the documented inventory against the registered tool set."""
|
|
documented_list = list(documented)
|
|
documented_set = set(documented_list)
|
|
registered_set = set(registered)
|
|
|
|
duplicates = sorted(
|
|
{name for name in documented_list if documented_list.count(name) > 1}
|
|
)
|
|
documented_not_registered = sorted(documented_set - registered_set)
|
|
registered_not_documented = sorted(registered_set - documented_set)
|
|
unsorted = documented_list != sorted(documented_list)
|
|
|
|
reasons: list[str] = []
|
|
if documented_not_registered:
|
|
reasons.append(
|
|
"documented but not registered: "
|
|
+ ", ".join(documented_not_registered)
|
|
)
|
|
if registered_not_documented:
|
|
reasons.append(
|
|
"registered but not documented: "
|
|
+ ", ".join(registered_not_documented)
|
|
)
|
|
if duplicates:
|
|
reasons.append("listed more than once: " + ", ".join(duplicates))
|
|
if unsorted:
|
|
reasons.append("inventory entries are not in sorted order")
|
|
|
|
in_sync = not reasons
|
|
return {
|
|
"in_sync": in_sync,
|
|
"documented_count": len(documented_set),
|
|
"registered_count": len(registered_set),
|
|
"documented_not_registered": documented_not_registered,
|
|
"registered_not_documented": registered_not_documented,
|
|
"duplicates": duplicates,
|
|
"sorted": not unsorted,
|
|
"reasons": reasons,
|
|
"safe_next_action": (
|
|
""
|
|
if in_sync
|
|
else (
|
|
f"Update {INVENTORY_DOC_PATH} so the block between the "
|
|
"inventory markers lists exactly the registered tools, sorted, "
|
|
"one '- `tool_name`' entry per line."
|
|
)
|
|
),
|
|
}
|
|
|
|
|
|
def assess_doc_references(
|
|
references: Mapping[str, Iterable[str]],
|
|
registered: Iterable[str],
|
|
) -> dict[str, Any]:
|
|
"""Verify every tool a document names is actually registered.
|
|
|
|
*references* maps a document path to the candidate tool names it mentions.
|
|
"""
|
|
registered_set = set(registered)
|
|
unregistered: list[dict[str, Any]] = []
|
|
checked = 0
|
|
for path, names in sorted(references.items()):
|
|
for name in sorted(set(names)):
|
|
checked += 1
|
|
if name not in registered_set:
|
|
unregistered.append({"document": path, "tool": name})
|
|
|
|
clean = not unregistered
|
|
reasons = [
|
|
f"{entry['document']} documents '{entry['tool']}', "
|
|
"which no namespace registers"
|
|
for entry in unregistered
|
|
]
|
|
return {
|
|
"clean": clean,
|
|
"checked_count": checked,
|
|
"unregistered": unregistered,
|
|
"reasons": reasons,
|
|
"safe_next_action": (
|
|
""
|
|
if clean
|
|
else (
|
|
"Either register the named tool with @mcp.tool() or correct the "
|
|
"document. Documentation must never name a tool an actor cannot "
|
|
"reach. If the identifier is a module or script rather than a "
|
|
"tool, add it to mcp_tool_inventory.NON_TOOL_IDENTIFIERS."
|
|
)
|
|
),
|
|
}
|
|
|
|
|
|
def render_inventory_block(registered: Iterable[str]) -> str:
|
|
"""Render the marker-delimited inventory block for the documentation."""
|
|
lines = [INVENTORY_BEGIN_MARKER, ""]
|
|
lines.extend(f"- `{name}`" for name in sorted(set(registered)))
|
|
lines.extend(["", INVENTORY_END_MARKER])
|
|
return "\n".join(lines)
|