Move the AST-based ReadOnlyHint scan introduced in #2486 out of
pkg/github's test file and into a new exported package, pkg/toolvalidation,
so downstream consumers (notably github/github-mcp-server-remote, which
uses this repo as a library) can apply the same guardrail to their own
tool registrations with a one-line test:
violations, err := toolvalidation.ScanReadOnlyHint(pkgDir)
Changes:
- New pkg/toolvalidation/readonlyhint.go with ScanReadOnlyHint,
FormatReadOnlyHintViolations, and the ReadOnlyHintViolation type.
- Dedicated unit tests for the scanner using in-memory fixtures
(compliant, missing-hint, missing-annotations, non-literal,
aliased import, positional fields, file without mcp import).
- pkg/github/tools_static_validation_test.go shrunk to a thin wrapper
that calls ScanReadOnlyHint against its own package directory; the
existing behavior for pkg/github is preserved.
No production-code, schema, or toolsnap changes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Resolve each file's local alias for github.com/modelcontextprotocol/go-sdk/mcp
via file.Imports rather than hard-coding the "mcp" qualifier, so the check
also covers files that import the SDK under a non-default alias.
- Detect positional (unkeyed) composite literals and report a dedicated
diagnostic instead of producing misleading "missing field" violations.
- Drop the brittle 'expected to discover at least one mcp.Tool literal'
assertion: if registrations move behind constructors/factories the AST
walker legitimately finds nothing.
- Use strconv.Unquote to decode tool-name string literals (handles escapes
in interpreted strings); fall back to the raw lexeme on parse error.
Adds a source-level (AST) validation test that walks every non-test Go file in pkg/github and fails if any mcp.Tool composite literal omits Annotations.ReadOnlyHint.
The existing TestAllToolsHaveRequiredMetadata can only assert that Annotations is non-nil at runtime: Go cannot distinguish an unset bool field from one explicitly set to false. The new test closes that gap so future read-intent tools cannot silently default to ReadOnlyHint=false, which has caused downstream agents to prompt for human approval on safe read operations.
All 97 current mcp.Tool registrations pass. Fault-injected by removing ReadOnlyHint from issue_read and confirmed the test reports the exact file, line, tool name, and reason.
Refs github/github-mcp-server#2483
Sam Morrow