A mcpsrv/backend.go => mcpsrv/backend.go +211 -0
@@ 0,0 1,211 @@
+package mcpsrv
+
+import (
+ "context"
+ "errors"
+ "fmt"
+ "strings"
+
+ "github.com/go-git/go-git/v5/plumbing"
+
+ "sourcecraft.dev/bigbes/sr-ht-spec/core"
+ "sourcecraft.dev/bigbes/sr-ht-spec/doc"
+ "sourcecraft.dev/bigbes/sr-ht-spec/gitx"
+ "sourcecraft.dev/bigbes/sr-ht-spec/search"
+ "sourcecraft.dev/bigbes/sr-ht-spec/service"
+)
+
+// Reader is the part of the orchestration layer the read tools call.
+// *service.Service satisfies it.
+//
+// It is deliberately four methods. Every one of them is the same function the
+// REST handlers, the GraphQL resolvers and the web UI call, which is what the
+// design means by "the MCP tools call the same resolver layer, not a parallel
+// implementation" — a fifth method here that service/ does not have would be
+// the beginning of a second implementation.
+//
+// ReadDocument is absent on purpose. Addressing a document by id needs the
+// space's whole document set anyway (see resolvePage), and reading the body
+// from that set rather than issuing a second, path-only read keeps one code
+// path instead of two that must agree about what an id names.
+type Reader interface {
+ ListSpaces(ctx context.Context) ([]*service.Space, error)
+ OpenSpace(ctx context.Context, ref core.SpaceRef) (*service.Space, error)
+ ResolveRev(ctx context.Context, sp *service.Space, rev string) (string, error)
+ ListDocuments(ctx context.Context, sp *service.Space, rev string) ([]service.Document, error)
+}
+
+// Searcher is the query side of the one global index. *search.Index satisfies
+// it.
+type Searcher interface {
+ Search(ctx context.Context, q search.Query) (search.Results, error)
+}
+
+// Compile-time assertions that the production types satisfy the interfaces
+// this package is written against. They are here rather than in a test so that
+// a signature change in service/ or search/ breaks the build of the package
+// that depends on them, not of a test somebody may not run.
+var (
+ _ Reader = (*service.Service)(nil)
+ _ Searcher = (*search.Index)(nil)
+)
+
+const (
+ // minRevLen and maxRevLen bound a git object name. They are the design's
+ // X-Agent-Base grammar — 7-64 lowercase hex — reused here so that "a
+ // revision an agent may name" has one spelling across every surface.
+ minRevLen = 7
+ maxRevLen = 64
+)
+
+// parseSpace resolves the tool's space argument, which is written the way the
+// design writes a space everywhere else: "~owner/name".
+func parseSpace(s string) (core.SpaceRef, error) {
+ trimmed := strings.TrimSpace(s)
+ if trimmed == "" {
+ return core.SpaceRef{}, errors.New("space must not be empty; call spec_list with no arguments to list spaces")
+ }
+ ref, err := core.ParseSpaceRef(trimmed)
+ if err != nil {
+ return core.SpaceRef{}, fmt.Errorf("space %q: %w", trimmed, err)
+ }
+ return ref, nil
+}
+
+// parseRev turns the tool's optional rev argument into a revision string for
+// service/. An empty argument becomes service.ApprovedRev, which is the read
+// contract's default: the approved head.
+//
+// Anything else must be a git object name. Ref names are refused even though
+// service/ would happily resolve them, and that refusal is the whole point:
+// "proposals/42" is a legal revision one layer down, so accepting it here
+// would make unreviewed proposal content reachable by a plausible-looking
+// argument. Reading it requires naming a commit sha, which no read tool ever
+// hands back, so it cannot be reached by accident.
+func parseRev(s string) (string, error) {
+ rev := strings.TrimSpace(s)
+ if rev == "" {
+ return service.ApprovedRev, nil
+ }
+ if len(rev) < minRevLen || len(rev) > maxRevLen {
+ return "", fmt.Errorf("rev %q must be a git object name of %d-%d hex characters; "+
+ "omit rev to read the approved head", rev, minRevLen, maxRevLen)
+ }
+ for i := 0; i < len(rev); i++ {
+ c := rev[i]
+ if (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') {
+ continue
+ }
+ return "", fmt.Errorf("rev %q must be a git object name (lowercase hex), not a branch or ref name; "+
+ "omit rev to read the approved head", rev)
+ }
+ return rev, nil
+}
+
+// archiveAt reads a space at a revision and builds the addressable document
+// set, returning it together with the bodies keyed by path and the resolved
+// commit.
+//
+// The revision is resolved first and everything below is read at the resolved
+// sha, never at the caller's string. That is what makes an unpinned read
+// pinnable: the rev reported back names the exact bytes returned, so a merge
+// landing between the resolve and the read cannot make one answer describe two
+// revisions.
+//
+// The whole space is read for one document. At the confirmed volume — tens of
+// documents a day — that is cheap, and the alternative is worse: doc.Archive is
+// where the design's addressing rule (id when valid and unique, else path)
+// actually lives, and it is built from a revision's whole document set. A
+// path-only fast path would be a second addressing implementation.
+func archiveAt(ctx context.Context, b Backend, sp *service.Space, rev string) (*doc.Archive, map[string][]byte, string, error) {
+ resolved, err := b.Docs.ResolveRev(ctx, sp, rev)
+ if err != nil {
+ return nil, nil, "", err
+ }
+ docs, err := b.Docs.ListDocuments(ctx, sp, resolved)
+ if err != nil {
+ return nil, nil, "", err
+ }
+ converted, err := toGitDocuments(docs)
+ if err != nil {
+ return nil, nil, "", err
+ }
+ arc := doc.FromDocuments(sp.Ref, resolved, converted)
+ bodies := make(map[string][]byte, len(docs))
+ for _, d := range docs {
+ bodies[d.Path] = d.Data
+ }
+ return arc, bodies, resolved, nil
+}
+
+// toGitDocuments converts service/'s hex-object-name documents back into the
+// shape doc.FromDocuments takes.
+//
+// This conversion should not exist, and it is the one place this package
+// reaches below service/. service.Document carries Blob and Rev as hex strings
+// precisely so that api/, mcpsrv/, graph/ and web/ need not import gitx — but
+// doc.FromDocuments, which owns the addressing rule those surfaces have to
+// apply, takes []gitx.Document. Until service/ exposes the archive itself,
+// something above it has to bridge the two, and doing it here in four lines is
+// better than restating the addressing rule in Go. A malformed sha is an
+// error, not a zero hash: plumbing.NewHash silently yields the zero value for
+// anything it cannot parse, and a document whose render-cache key is zero is a
+// cache collision waiting to happen.
+func toGitDocuments(docs []service.Document) ([]gitx.Document, error) {
+ out := make([]gitx.Document, 0, len(docs))
+ for _, d := range docs {
+ if !plumbing.IsHash(d.Blob) {
+ return nil, fmt.Errorf("document %q carries a malformed blob id %q", d.Path, d.Blob)
+ }
+ out = append(out, gitx.Document{Path: d.Path, Blob: plumbing.NewHash(d.Blob), Data: d.Data})
+ }
+ return out, nil
+}
+
+// resolvePage applies the design's addressing rule to one caller-supplied
+// token, using the archive's own lookups rather than reimplementing them:
+//
+// - a document with a well-formed id that no other document in its space
+// claims is addressed by that id;
+// - a document whose id is absent, malformed or duplicated is addressed by
+// its path — with or without the ".md" extension, since the read plane's
+// URL grammar carries no extension and an agent copying a path out of a
+// search hit has one.
+//
+// A token naming an id that two documents claim resolves to neither, and says
+// so. Silently picking one would point an agent at a document its author did
+// not mean, and the ambiguity would be invisible.
+func resolvePage(arc *doc.Archive, token string) (*doc.Page, error) {
+ name := strings.TrimSpace(token)
+ if name == "" {
+ return nil, errors.New("document must not be empty")
+ }
+ if p, ok := arc.Page(name); ok {
+ return p, nil
+ }
+ if p, ok := arc.ByPath(name); ok {
+ return p, nil
+ }
+ if p, ok := arc.ByPath(name + ".md"); ok {
+ return p, nil
+ }
+ if dup := duplicateIDPaths(arc, name); len(dup) > 1 {
+ return nil, fmt.Errorf("document id %q is claimed by %d documents (%s) and resolves to none of them; "+
+ "address one of them by path instead", name, len(dup), strings.Join(dup, ", "))
+ }
+ return nil, fmt.Errorf("no document %q in %s at %s", name, arc.Space, arc.Rev)
+}
+
+// duplicateIDPaths lists the paths of every document claiming id. doc/ excludes
+// a duplicated id from the archive's lookup, which is the correct resolution
+// but leaves nothing to report; this walk exists only to turn "not found" into
+// a message that names the collision.
+func duplicateIDPaths(arc *doc.Archive, id string) []string {
+ var paths []string
+ for _, p := range arc.All() {
+ if p.DocID == id {
+ paths = append(paths, p.Path)
+ }
+ }
+ return paths
+}
A mcpsrv/hostguard_test.go => mcpsrv/hostguard_test.go +83 -0
@@ 0,0 1,83 @@
+package mcpsrv_test
+
+import (
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "testing"
+
+ "sourcecraft.dev/bigbes/sr-ht-spec/mcpsrv"
+)
+
+// The SDK's own DNS-rebinding guard is disabled (it cannot tell nginx from an
+// attacker), so this replacement is the only thing protecting /mcp. These cases
+// are the reason the bypass is defensible: a hostile Host is still refused, and
+// the legitimate proxied Host that the SDK guard would have rejected is let
+// through.
+func TestHostGuard(t *testing.T) {
+ const origin = "https://spec.srht.bigb.es"
+
+ for _, tc := range []struct {
+ name string
+ host string
+ want int
+ }{
+ {"proxied real hostname", "spec.srht.bigb.es", http.StatusOK},
+ {"proxied with port", "spec.srht.bigb.es:443", http.StatusOK},
+ {"case-insensitive", "SPEC.SRHT.BIGB.ES", http.StatusOK},
+ {"loopback for dev", "127.0.0.1:5091", http.StatusOK},
+ {"localhost for dev", "localhost:5091", http.StatusOK},
+ {"rebinding attacker domain", "evil.example.com", http.StatusForbidden},
+ {"attacker subdomain of us", "spec.srht.bigb.es.evil.com", http.StatusForbidden},
+ {"another srht service", "git.srht.bigb.es", http.StatusForbidden},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ r, s := newFixture()
+ h, err := mcpsrv.Handler(mcpsrv.Backend{Docs: r, Index: s}, "test", origin)
+ if err != nil {
+ t.Fatalf("Handler: %v", err)
+ }
+
+ req := httptest.NewRequest(http.MethodPost, "/mcp",
+ strings.NewReader(`{"jsonrpc":"2.0","id":1,"method":"initialize"}`))
+ req.Host = tc.host
+ req.Header.Set("Content-Type", "application/json")
+ req.Header.Set("Accept", "application/json, text/event-stream")
+
+ w := httptest.NewRecorder()
+ h.ServeHTTP(w, req)
+
+ if tc.want == http.StatusForbidden && w.Code != http.StatusForbidden {
+ t.Fatalf("Host %q got %d, want 403 — the guard did not block it", tc.host, w.Code)
+ }
+ if tc.want == http.StatusOK && w.Code == http.StatusForbidden {
+ t.Fatalf("Host %q got 403 — the guard blocked a legitimate request", tc.host)
+ }
+ })
+ }
+}
+
+// An unusable origin must leave a loud trail rather than silently unguarding the
+// endpoint. It still serves (refusing to start over a config typo is worse), but
+// Handler logs a warning; this pins that it does not instead start refusing
+// every request, which would look like the guard "working".
+func TestHostGuardWithoutOriginStillServes(t *testing.T) {
+ r, s := newFixture()
+ h, err := mcpsrv.Handler(mcpsrv.Backend{Docs: r, Index: s}, "test", "")
+ if err != nil {
+ t.Fatalf("Handler: %v", err)
+ }
+
+ req := httptest.NewRequest(http.MethodPost, "/mcp",
+ strings.NewReader(`{"jsonrpc":"2.0","id":1,"method":"initialize"}`))
+ req.Host = "anything.example.com"
+ req.Header.Set("Content-Type", "application/json")
+ req.Header.Set("Accept", "application/json, text/event-stream")
+
+ w := httptest.NewRecorder()
+ h.ServeHTTP(w, req)
+
+ if w.Code == http.StatusForbidden {
+ t.Fatalf("unconfigured origin should not 403 every request; got %d", w.Code)
+ }
+}
A mcpsrv/http_test.go => mcpsrv/http_test.go +85 -0
@@ 0,0 1,85 @@
+package mcpsrv_test
+
+import (
+ "context"
+ "io"
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "testing"
+
+ "github.com/go-chi/chi/v5"
+ "github.com/modelcontextprotocol/go-sdk/mcp"
+ "github.com/stretchr/testify/require"
+
+ "sourcecraft.dev/bigbes/sr-ht-spec/mcpsrv"
+)
+
+// The transport the design settled on: streamable HTTP on the same chi router
+// at /mcp, so that one listener and one nginx block cover the whole service.
+// This is the exact mounting call cmd/specsrht makes.
+func TestHandlerMountsOnChiAtMCP(t *testing.T) {
+ r, s := newFixture()
+
+ h, err := mcpsrv.Handler(mcpsrv.Backend{Docs: r, Index: s}, "test", "https://spec.srht.bigb.es")
+ require.NoError(t, err)
+
+ router := chi.NewRouter()
+ router.Handle("/mcp", h)
+
+ srv := httptest.NewServer(router)
+ t.Cleanup(srv.Close)
+
+ client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "test"}, nil)
+ session, err := client.Connect(context.Background(),
+ &mcp.StreamableClientTransport{Endpoint: srv.URL + "/mcp"}, nil)
+ require.NoError(t, err)
+ t.Cleanup(func() { _ = session.Close() })
+
+ require.Equal(t, mcpsrv.ServerName, session.InitializeResult().ServerInfo.Name)
+
+ res, err := session.CallTool(context.Background(), &mcp.CallToolParams{
+ Name: "spec_read",
+ Arguments: map[string]any{"space": "~bigbes/rfcs", "document": "SPEC-0007"},
+ })
+ require.NoError(t, err)
+ var out readResult
+ decode(t, res, &out)
+ require.Contains(t, out.Markdown, "the approved body")
+ require.Equal(t, approvedRev, out.Rev)
+}
+
+// The deployment shape, pinned by a test because it is invisible otherwise:
+// the daemon listens on loopback and nginx forwards with the public Host
+// (`proxy_set_header Host $host`). The SDK's DNS-rebinding guard rejects
+// exactly that combination with a 403, so Handler disables it. Without this
+// test the whole MCP endpoint would 403 in production and pass every local
+// check, since a local client sends a loopback Host.
+func TestHandlerAcceptsProxiedHostHeader(t *testing.T) {
+ r, s := newFixture()
+
+ h, err := mcpsrv.Handler(mcpsrv.Backend{Docs: r, Index: s}, "test", "https://spec.srht.bigb.es")
+ require.NoError(t, err)
+
+ router := chi.NewRouter()
+ router.Handle("/mcp", h)
+ srv := httptest.NewServer(router)
+ t.Cleanup(srv.Close)
+
+ body := `{"jsonrpc":"2.0","id":1,"method":"initialize","params":{` +
+ `"protocolVersion":"2025-06-18","capabilities":{},` +
+ `"clientInfo":{"name":"test-client","version":"test"}}}`
+
+ req, err := http.NewRequest(http.MethodPost, srv.URL+"/mcp", io.Reader(strings.NewReader(body)))
+ require.NoError(t, err)
+ req.Host = "spec.srht.bigb.es"
+ req.Header.Set("Content-Type", "application/json")
+ req.Header.Set("Accept", "application/json, text/event-stream")
+
+ resp, err := srv.Client().Do(req)
+ require.NoError(t, err)
+ t.Cleanup(func() { _ = resp.Body.Close() })
+
+ require.Equal(t, http.StatusOK, resp.StatusCode,
+ "a loopback listener behind a proxy that sets the public Host must not be refused")
+}
A mcpsrv/list.go => mcpsrv/list.go +111 -0
@@ 0,0 1,111 @@
+package mcpsrv
+
+import (
+ "context"
+ "errors"
+ "strings"
+)
+
+type listInput struct {
+ Space string `json:"space,omitempty" jsonschema:"the space to list documents from, written \"~owner/name\". Omit it to list the spaces on this instance instead."`
+ Rev string `json:"rev,omitempty" jsonschema:"pin the listing to one immutable revision, given as a git object name (lowercase hex). Omit to list the space's approved head. Requires space."`
+}
+
+type spaceEntry struct {
+ // Space is the name every other tool takes: "~owner/name".
+ Space string `json:"space"`
+ Owner string `json:"owner"`
+ Name string `json:"name"`
+}
+
+type documentEntry struct {
+ // ID is how spec_read addresses this document.
+ ID string `json:"id"`
+ DocID string `json:"doc_id,omitempty"`
+ Path string `json:"path"`
+ Blob string `json:"blob,omitempty"`
+ Title string `json:"title,omitempty"`
+ // Section is the top-level directory the document lives under, and is what
+ // spec_search's sections filter takes.
+ Section string `json:"section,omitempty"`
+ // Status is the authored lifecycle marker, not approval state.
+ Status string `json:"status,omitempty"`
+ Summary string `json:"summary,omitempty"`
+ Tags []string `json:"tags,omitempty"`
+}
+
+// listOutput carries whichever of the two listings was asked for. One tool
+// rather than two because the answer to "what can I read" is one question with
+// a drill-down, and an agent that has just been handed a space name should not
+// have to find a second tool to use it.
+type listOutput struct {
+ // Spaces is set when no space was named.
+ Spaces []spaceEntry `json:"spaces,omitempty"`
+ // Space, Rev and Documents are set when one was.
+ Space string `json:"space,omitempty"`
+ // Rev is the revision listed, resolved to an immutable commit. Pass it to
+ // spec_read to read any of these documents at exactly this revision.
+ Rev string `json:"rev,omitempty"`
+ Documents []documentEntry `json:"documents,omitempty"`
+}
+
+func listHandler(ctx context.Context, b Backend, in listInput) (listOutput, error) {
+ if strings.TrimSpace(in.Space) == "" {
+ // A rev with no space is a caller that meant to name one. Ignoring it
+ // would answer a different question than was asked and look like it
+ // had worked.
+ if strings.TrimSpace(in.Rev) != "" {
+ return listOutput{}, errors.New("rev names a revision of a space; pass space as well, or omit rev to list spaces")
+ }
+ return listSpaces(ctx, b)
+ }
+
+ ref, err := parseSpace(in.Space)
+ if err != nil {
+ return listOutput{}, err
+ }
+ rev, err := parseRev(in.Rev)
+ if err != nil {
+ return listOutput{}, err
+ }
+ sp, err := b.Docs.OpenSpace(ctx, ref)
+ if err != nil {
+ return listOutput{}, err
+ }
+ arc, _, resolved, err := archiveAt(ctx, b, sp, rev)
+ if err != nil {
+ return listOutput{}, err
+ }
+
+ out := listOutput{Space: ref.String(), Rev: resolved, Documents: make([]documentEntry, 0, len(arc.All()))}
+ for _, p := range arc.All() {
+ out.Documents = append(out.Documents, documentEntry{
+ ID: p.ID,
+ DocID: p.DocID,
+ Path: p.Path,
+ Blob: p.Blob,
+ Title: p.Title,
+ Section: p.Section,
+ Status: string(p.Status),
+ Summary: p.Summary,
+ Tags: p.Tags,
+ })
+ }
+ return out, nil
+}
+
+func listSpaces(ctx context.Context, b Backend) (listOutput, error) {
+ spaces, err := b.Docs.ListSpaces(ctx)
+ if err != nil {
+ return listOutput{}, err
+ }
+ out := listOutput{Spaces: make([]spaceEntry, 0, len(spaces))}
+ for _, sp := range spaces {
+ out.Spaces = append(out.Spaces, spaceEntry{
+ Space: sp.Ref.String(),
+ Owner: sp.Ref.Owner,
+ Name: sp.Ref.Name,
+ })
+ }
+ return out, nil
+}
A mcpsrv/mcpsrv.go => mcpsrv/mcpsrv.go +254 -0
@@ 0,0 1,254 @@
+// Package mcpsrv is spec.sr.ht's Model Context Protocol surface: the tools an
+// agent calls to find and read approved documents.
+//
+// It is warren's mcpsrv/ absorbed, and it keeps warren's two structural
+// choices — a narrow backend interface the tools are written against, and the
+// SDK's generic AddTool deriving every schema from a Go struct — while
+// discarding everything that assumed a local vault: the vault-wide id space,
+// the parent/child tools, and the hybrid keyword+semantic mode argument (vector
+// search is Phase 5 here).
+//
+// # A surface, not a wrapper
+//
+// The design's rule is that MCP is a first-class surface and that its tools
+// call the same resolver layer as REST, GraphQL and the web UI rather than a
+// parallel implementation. That is what this package is: every lookup goes
+// through service/, every addressing decision through doc.Archive, and every
+// query through search.Index. Nothing here re-derives which document an id
+// names or which spaces a project covers, because a second implementation of
+// those rules is how two surfaces start answering the same question
+// differently — silently, and months later.
+//
+// # The read contract is the reason this exists
+//
+// Bots need "the approved text of SPEC-0007", not "whatever a branch points at
+// while another agent rewrites it". So:
+//
+// - Omitting rev reads the space's approved head. That is the default,
+// and it is service.ApprovedRev — the same default every other surface has.
+// - Passing rev pins the read to one immutable revision, and rev must be an
+// object name: 7-64 lowercase hex, the same grammar the design pins for
+// X-Agent-Base. Ref names are refused outright, which is what makes
+// "read a proposal branch" impossible to reach by accident or by
+// mistyping — an agent can only reach unapproved content by naming a
+// commit sha it had to obtain deliberately, since neither spec_search nor
+// spec_list ever reports one.
+//
+// Serving drafts by default would poison every downstream agent context with
+// unreviewed text, which is the exact failure the service exists to prevent.
+// The write tools (spec_propose, spec_comment) are Phase 3 and are deliberately
+// absent; nothing here writes.
+//
+// # Layering
+//
+// Reader and Searcher are declared here rather than imported as concrete types
+// so the tools can be tested without a repository, a Postgres instance or a
+// bleve index. *service.Service satisfies Reader and *search.Index satisfies
+// Searcher, structurally, with no adapter.
+package mcpsrv
+
+import (
+ "context"
+ "errors"
+ "log/slog"
+ "net"
+ "net/http"
+ "net/url"
+ "strings"
+
+ "github.com/modelcontextprotocol/go-sdk/mcp"
+
+ "sourcecraft.dev/bigbes/sr-ht-spec/search"
+)
+
+// ServerName is the implementation name reported in the MCP handshake. It is
+// the service's config-section name, so a client listing several SourceHut MCP
+// endpoints sees which one it is talking to.
+const ServerName = "spec.sr.ht"
+
+// maxSearchLimit caps how many hits one call may ask for.
+const maxSearchLimit = 100
+
+// Backend is everything the tools read through. Both halves are required: a
+// nil one is a wiring mistake, and New reports it at startup rather than
+// letting the first tool call panic inside a request.
+type Backend struct {
+ // Docs is the orchestration layer — in production *service.Service.
+ Docs Reader
+ // Index is the one global bleve index — in production *search.Index.
+ Index Searcher
+}
+
+// New builds the MCP server with the Phase 2 read tools registered. version is
+// reported as the implementation version in the handshake.
+//
+// The returned server is not connected to a transport; Handler wires it to
+// streamable HTTP, and a caller that wants stdio can call Run itself.
+func New(b Backend, version string) (*mcp.Server, error) {
+ if b.Docs == nil {
+ return nil, errors.New("mcpsrv: backend has no document reader")
+ }
+ if b.Index == nil {
+ return nil, errors.New("mcpsrv: backend has no search index")
+ }
+
+ srv := mcp.NewServer(&mcp.Implementation{Name: ServerName, Version: version}, nil)
+ readOnly := &mcp.ToolAnnotations{ReadOnlyHint: true, IdempotentHint: true}
+
+ mcp.AddTool(srv, &mcp.Tool{
+ Name: "spec_search",
+ Annotations: readOnly,
+ Description: "Search every approved document on this instance, ranked. Each hit carries " +
+ "the space, the document id, its path, the revision it was indexed at, its title " +
+ "and a plain-text snippet — enough to fetch it with spec_read without a second " +
+ "lookup.\n\n" +
+ "Pass `spaces` to restrict the search to a set of spaces: that set is what a " +
+ "project is on this service — a saved filter over one global index, not a " +
+ "container. Omitting it searches everything, which is the meta-project.\n\n" +
+ "Results come from the approved revision of each space. Proposal branches are " +
+ "not indexed and never appear here.",
+ }, func(ctx context.Context, _ *mcp.CallToolRequest, in searchInput) (*mcp.CallToolResult, searchOutput, error) {
+ out, err := searchHandler(ctx, b, in)
+ return nil, out, err
+ })
+
+ mcp.AddTool(srv, &mcp.Tool{
+ Name: "spec_read",
+ Annotations: readOnly,
+ Description: "Read one document's markdown, frontmatter included, exactly as it is stored.\n\n" +
+ "By default this returns the space's APPROVED text — the reviewed, canonical " +
+ "revision — and reports the revision it resolved to in `rev`. Pass that value " +
+ "back as the `rev` argument later to re-read the identical bytes; a revision, " +
+ "once named, is immutable.\n\n" +
+ "Address the document by its frontmatter id (\"SPEC-0007\") when it has a " +
+ "well-formed one that no other document in the space claims, and otherwise by " +
+ "its path, with or without the \".md\" extension. A document whose id is " +
+ "duplicated within its space resolves to neither document and is reported as " +
+ "ambiguous rather than guessed at.",
+ }, func(ctx context.Context, _ *mcp.CallToolRequest, in readInput) (*mcp.CallToolResult, readOutput, error) {
+ out, err := readHandler(ctx, b, in)
+ return nil, out, err
+ })
+
+ mcp.AddTool(srv, &mcp.Tool{
+ Name: "spec_list",
+ Annotations: readOnly,
+ Description: "List spaces, or list the documents in one space.\n\n" +
+ "Omit `space` to get every space on the instance — those names are what " +
+ "spec_search's `spaces` filter takes. Pass `space` to get that space's " +
+ "documents at its approved head, each with the id spec_read addresses it by, " +
+ "its path, title, section and authored status. Pass `rev` as well to list a " +
+ "pinned revision instead.",
+ }, func(ctx context.Context, _ *mcp.CallToolRequest, in listInput) (*mcp.CallToolResult, listOutput, error) {
+ out, err := listHandler(ctx, b, in)
+ return nil, out, err
+ })
+
+ return srv, nil
+}
+
+// Handler mounts the server on streamable HTTP. cmd/specsrht hangs it off the
+// chi router at /mcp, which is what keeps MCP to one listener and one nginx
+// block rather than a second port.
+//
+// The SDK's DNS-rebinding guard is disabled deliberately, and the reason is
+// worth stating because disabling a security default usually is not.
+//
+// The guard rejects any request arriving on a loopback address that carries a
+// non-loopback Host header. That is precisely our deployment: the daemon binds
+// 127.0.0.1:5091 and nginx forwards with `proxy_set_header Host $host`, so
+// every genuine request would 403 — and it would 403 only in production,
+// because a local client sends a loopback Host and passes.
+//
+// It is not that the guard has nothing to catch. A browser running on the
+// daemon's own host could reach 127.0.0.1:5091 directly with an attacker's
+// Host header, which is the attack the guard is for. The problem is that the
+// guard cannot tell that request from nginx's: both arrive from loopback
+// carrying a Host that is not loopback, and the SDK exposes no allowlist to
+// separate them.
+//
+// So the guard is disabled and REPLACED, in the same constructor, by allowHosts
+// below — a stricter check than the one removed. The SDK asks only "is Host
+// loopback?"; we require Host to equal this instance's configured origin. A
+// rebinding attack carries the attacker's name in Host and fails that; nginx
+// forwards our real hostname and passes. Disabling the SDK guard without this
+// replacement would be a genuine regression, not a formality.
+func Handler(b Backend, version, origin string) (http.Handler, error) {
+ srv, err := New(b, version)
+ if err != nil {
+ return nil, err
+ }
+ h := mcp.NewStreamableHTTPHandler(
+ func(*http.Request) *mcp.Server { return srv },
+ &mcp.StreamableHTTPOptions{DisableLocalhostProtection: true},
+ )
+ return allowHosts(h, origin), nil
+}
+
+// allowHosts is this endpoint's DNS-rebinding protection, in the form the
+// deployment actually needs: Host must be the service's own origin hostname, or
+// a loopback name for local development.
+//
+// An empty or unparseable origin leaves the endpoint unguarded, so it says so
+// loudly. A misconfigured origin must not quietly become the difference between
+// protected and open — that is the class of failure nobody discovers.
+func allowHosts(next http.Handler, origin string) http.Handler {
+ want := originHost(origin)
+ if want == "" {
+ slog.Warn("mcpsrv: no usable origin configured; Host validation on /mcp is DISABLED")
+ return next
+ }
+ return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ if !hostAllowed(r.Host, want) {
+ http.Error(w, "Forbidden: unexpected Host header", http.StatusForbidden)
+ return
+ }
+ next.ServeHTTP(w, r)
+ })
+}
+
+// hostAllowed compares a request's Host against the expected hostname, ignoring
+// any port and IPv6 brackets. Loopback names stay allowed so `make run-dev` and
+// a local MCP client keep working.
+func hostAllowed(reqHost, want string) bool {
+ h := reqHost
+ if stripped, _, err := net.SplitHostPort(h); err == nil {
+ h = stripped
+ }
+ h = strings.TrimSuffix(strings.TrimPrefix(h, "["), "]")
+ switch {
+ case strings.EqualFold(h, want):
+ return true
+ case h == "localhost", h == "127.0.0.1", h == "::1":
+ return true
+ default:
+ return false
+ }
+}
+
+// originHost extracts the hostname from a configured origin URL.
+func originHost(origin string) string {
+ if origin == "" {
+ return ""
+ }
+ u, err := url.Parse(origin)
+ if err != nil {
+ return ""
+ }
+ return u.Hostname()
+}
+
+// clampLimit applies the hit-count policy: unset defers to search's own
+// default rather than restating it, and anything larger than maxSearchLimit is
+// clamped. A tool result is a context window, so an agent asking for a thousand
+// hits is asking for something it cannot use.
+func clampLimit(n int) int {
+ switch {
+ case n <= 0:
+ return search.DefaultLimit
+ case n > maxSearchLimit:
+ return maxSearchLimit
+ default:
+ return n
+ }
+}
A mcpsrv/mcpsrv_test.go => mcpsrv/mcpsrv_test.go +627 -0
@@ 0,0 1,627 @@
+package mcpsrv_test
+
+import (
+ "context"
+ "encoding/json"
+ "errors"
+ "fmt"
+ "sort"
+ "testing"
+
+ "github.com/modelcontextprotocol/go-sdk/mcp"
+ "github.com/stretchr/testify/require"
+
+ "sourcecraft.dev/bigbes/sr-ht-spec/core"
+ "sourcecraft.dev/bigbes/sr-ht-spec/mcpsrv"
+ "sourcecraft.dev/bigbes/sr-ht-spec/search"
+ "sourcecraft.dev/bigbes/sr-ht-spec/service"
+)
+
+// --- fixtures ---------------------------------------------------------------
+
+// The two revisions every read test is written against: an approved head whose
+// SPEC-0007 says "approved", and an older pinned revision whose SPEC-0007 says
+// something else. Nothing distinguishes them but the revision, which is the
+// point — one storage tier, one code path, a different ref.
+const (
+ approvedRev = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+ olderRev = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
+ // proposalRev is a real commit sha that happens to be a proposal branch
+ // tip. It is readable only because the caller named it.
+ proposalRev = "cccccccccccccccccccccccccccccccccccccccc"
+)
+
+var fxSpace = core.SpaceRef{Owner: "bigbes", Name: "rfcs"}
+
+func blob(n int) string { return fmt.Sprintf("%040x", n) }
+
+func md(id, title, status, body string) []byte {
+ return []byte(fmt.Sprintf("---\nid: %s\ntitle: %s\nstatus: %s\n---\n\n%s\n", id, title, status, body))
+}
+
+// fakeReader stands in for *service.Service: a document set per revision, and
+// nothing else. It holds no repository, which is the property that makes these
+// tests run without git.
+type fakeReader struct {
+ spaces []*service.Space
+ revs map[string][]service.Document
+ approved string
+
+ openErr error
+ listErr error
+ lastRevs []string
+}
+
+func (f *fakeReader) ListSpaces(context.Context) ([]*service.Space, error) {
+ return f.spaces, nil
+}
+
+func (f *fakeReader) OpenSpace(_ context.Context, ref core.SpaceRef) (*service.Space, error) {
+ if f.openErr != nil {
+ return nil, f.openErr
+ }
+ for _, sp := range f.spaces {
+ if sp.Ref == ref {
+ return sp, nil
+ }
+ }
+ return nil, fmt.Errorf("%w: space %s", service.ErrNotFound, ref)
+}
+
+func (f *fakeReader) ResolveRev(_ context.Context, _ *service.Space, rev string) (string, error) {
+ if rev == service.ApprovedRev {
+ return f.approved, nil
+ }
+ if _, ok := f.revs[rev]; !ok {
+ return "", fmt.Errorf("%w: revision %q", service.ErrNotFound, rev)
+ }
+ return rev, nil
+}
+
+func (f *fakeReader) ListDocuments(_ context.Context, _ *service.Space, rev string) ([]service.Document, error) {
+ if f.listErr != nil {
+ return nil, f.listErr
+ }
+ f.lastRevs = append(f.lastRevs, rev)
+ docs, ok := f.revs[rev]
+ if !ok {
+ return nil, fmt.Errorf("%w: revision %q", service.ErrNotFound, rev)
+ }
+ return docs, nil
+}
+
+type fakeSearcher struct {
+ last search.Query
+ calls int
+ results search.Results
+ err error
+}
+
+func (f *fakeSearcher) Search(_ context.Context, q search.Query) (search.Results, error) {
+ f.last = q
+ f.calls++
+ return f.results, f.err
+}
+
+// newFixture builds the standard two-revision, two-space backend.
+func newFixture() (*fakeReader, *fakeSearcher) {
+ doc := func(path string, data []byte, rev string, n int) service.Document {
+ return service.Document{Path: path, Blob: blob(n), Rev: rev, Data: data}
+ }
+ r := &fakeReader{
+ spaces: []*service.Space{
+ {Ref: fxSpace, ID: 1},
+ {Ref: core.SpaceRef{Owner: "bigbes", Name: "notes"}, ID: 2},
+ },
+ approved: approvedRev,
+ revs: map[string][]service.Document{
+ approvedRev: {
+ doc("specs/0007-storage.md", md("SPEC-0007", "Storage model", "review", "the approved body"), approvedRev, 1),
+ doc("notes/untitled.md", []byte("# Loose note\n\nno frontmatter here\n"), approvedRev, 2),
+ },
+ olderRev: {
+ doc("specs/0007-storage.md", md("SPEC-0007", "Storage model", "draft", "the older body"), olderRev, 3),
+ },
+ proposalRev: {
+ doc("specs/0007-storage.md", md("SPEC-0007", "Storage model", "draft", "unreviewed proposal body"), proposalRev, 4),
+ },
+ },
+ }
+ return r, &fakeSearcher{}
+}
+
+// --- plumbing ---------------------------------------------------------------
+
+func connect(t *testing.T, r mcpsrv.Reader, s mcpsrv.Searcher) *mcp.ClientSession {
+ t.Helper()
+ ctx := context.Background()
+ serverTransport, clientTransport := mcp.NewInMemoryTransports()
+
+ srv, err := mcpsrv.New(mcpsrv.Backend{Docs: r, Index: s}, "test")
+ require.NoError(t, err)
+ serverConn, err := srv.Connect(ctx, serverTransport, nil)
+ require.NoError(t, err)
+ t.Cleanup(func() { _ = serverConn.Close() })
+
+ client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "test"}, nil)
+ session, err := client.Connect(ctx, clientTransport, nil)
+ require.NoError(t, err)
+ t.Cleanup(func() { _ = session.Close() })
+ return session
+}
+
+func call(t *testing.T, s *mcp.ClientSession, name string, args map[string]any) *mcp.CallToolResult {
+ t.Helper()
+ res, err := s.CallTool(context.Background(), &mcp.CallToolParams{Name: name, Arguments: args})
+ require.NoError(t, err, "protocol-level failure calling %s", name)
+ return res
+}
+
+func decode(t *testing.T, res *mcp.CallToolResult, out any) {
+ t.Helper()
+ require.False(t, res.IsError, "unexpected tool error: %s", errorText(res))
+ require.NotNil(t, res.StructuredContent, "no structured output")
+ raw, err := json.Marshal(res.StructuredContent)
+ require.NoError(t, err)
+ require.NoError(t, json.Unmarshal(raw, out))
+}
+
+func errorText(res *mcp.CallToolResult) string {
+ var s string
+ for _, c := range res.Content {
+ if tc, ok := c.(*mcp.TextContent); ok {
+ s += tc.Text
+ }
+ }
+ return s
+}
+
+type readResult struct {
+ Space string `json:"space"`
+ ID string `json:"id"`
+ DocID string `json:"doc_id"`
+ Path string `json:"path"`
+ Rev string `json:"rev"`
+ Blob string `json:"blob"`
+ Pinned bool `json:"pinned"`
+ Title string `json:"title"`
+ Section string `json:"section"`
+ Status string `json:"status"`
+ Tags []string `json:"tags"`
+ Markdown string `json:"markdown"`
+}
+
+// --- the read contract ------------------------------------------------------
+
+// The default read is the approved head. This is the property the whole service
+// exists for: an agent that names no revision must never be handed unreviewed
+// text.
+func TestReadDefaultsToApprovedHead(t *testing.T) {
+ r, s := newFixture()
+ session := connect(t, r, s)
+
+ res := call(t, session, "spec_read", map[string]any{
+ "space": "~bigbes/rfcs", "document": "SPEC-0007",
+ })
+ var out readResult
+ decode(t, res, &out)
+
+ require.Contains(t, out.Markdown, "the approved body")
+ require.NotContains(t, out.Markdown, "older body")
+ require.Equal(t, approvedRev, out.Rev, "the approved head is reported so the caller can pin it")
+ require.False(t, out.Pinned)
+ require.Equal(t, "SPEC-0007", out.ID)
+ require.Equal(t, "SPEC-0007", out.DocID)
+ require.Equal(t, "specs/0007-storage.md", out.Path)
+ require.Equal(t, blob(1), out.Blob)
+ require.Equal(t, "review", out.Status, "status is authored metadata, not approval state")
+
+ // Everything below the tool read the resolved sha, never the empty string:
+ // a merge landing mid-read cannot make one answer describe two revisions.
+ require.Equal(t, []string{approvedRev}, r.lastRevs)
+}
+
+// A pinned rev returns that revision, not the head.
+func TestReadPinnedRevReturnsThatRevision(t *testing.T) {
+ r, s := newFixture()
+ session := connect(t, r, s)
+
+ res := call(t, session, "spec_read", map[string]any{
+ "space": "~bigbes/rfcs", "document": "SPEC-0007", "rev": olderRev,
+ })
+ var out readResult
+ decode(t, res, &out)
+
+ require.Contains(t, out.Markdown, "the older body")
+ require.Equal(t, olderRev, out.Rev)
+ require.True(t, out.Pinned)
+ require.Equal(t, "draft", out.Status)
+ require.Equal(t, []string{olderRev}, r.lastRevs)
+}
+
+// Proposal content is reachable only by naming its commit — never by naming a
+// branch. This is the guard that keeps unreviewed text out of an agent's
+// context by accident.
+func TestReadRefusesRefNames(t *testing.T) {
+ r, s := newFixture()
+ session := connect(t, r, s)
+
+ for _, rev := range []string{"proposals/42", "main", "HEAD", "cafe"} {
+ res := call(t, session, "spec_read", map[string]any{
+ "space": "~bigbes/rfcs", "document": "SPEC-0007", "rev": rev,
+ })
+ require.True(t, res.IsError, "rev %q was accepted", rev)
+ require.Contains(t, errorText(res), "object name")
+ require.Contains(t, errorText(res), "approved head")
+ }
+ require.Empty(t, r.lastRevs, "a rejected rev never reaches the service layer")
+
+ // Naming the commit itself is deliberate, and works.
+ res := call(t, session, "spec_read", map[string]any{
+ "space": "~bigbes/rfcs", "document": "SPEC-0007", "rev": proposalRev,
+ })
+ var out readResult
+ decode(t, res, &out)
+ require.Contains(t, out.Markdown, "unreviewed proposal body")
+ require.True(t, out.Pinned)
+}
+
+// The design's addressing rule, both halves: id when it is well-formed and
+// unique, path when there is no usable id.
+func TestReadAddressing(t *testing.T) {
+ r, s := newFixture()
+ session := connect(t, r, s)
+
+ cases := []struct{ name, document, wantID, wantPath string }{
+ {"by id", "SPEC-0007", "SPEC-0007", "specs/0007-storage.md"},
+ {"by path", "specs/0007-storage.md", "SPEC-0007", "specs/0007-storage.md"},
+ {"by extensionless path", "specs/0007-storage", "SPEC-0007", "specs/0007-storage.md"},
+ {"id-less document by path", "notes/untitled", "notes/untitled", "notes/untitled.md"},
+ {"id-less document by path with extension", "notes/untitled.md", "notes/untitled", "notes/untitled.md"},
+ }
+ for _, c := range cases {
+ t.Run(c.name, func(t *testing.T) {
+ res := call(t, session, "spec_read", map[string]any{
+ "space": "~bigbes/rfcs", "document": c.document,
+ })
+ var out readResult
+ decode(t, res, &out)
+ require.Equal(t, c.wantID, out.ID)
+ require.Equal(t, c.wantPath, out.Path)
+ })
+ }
+
+ // A document with no frontmatter id reports none rather than inventing one
+ // from its path.
+ res := call(t, session, "spec_read", map[string]any{
+ "space": "~bigbes/rfcs", "document": "notes/untitled",
+ })
+ var out readResult
+ decode(t, res, &out)
+ require.Empty(t, out.DocID)
+}
+
+// A duplicated id resolves to neither document, and says which two claim it.
+func TestReadDuplicateIDIsAmbiguous(t *testing.T) {
+ r, s := newFixture()
+ r.revs[approvedRev] = []service.Document{
+ {Path: "specs/a.md", Blob: blob(5), Rev: approvedRev, Data: md("SPEC-0007", "A", "draft", "a")},
+ {Path: "specs/b.md", Blob: blob(6), Rev: approvedRev, Data: md("SPEC-0007", "B", "draft", "b")},
+ }
+ session := connect(t, r, s)
+
+ res := call(t, session, "spec_read", map[string]any{
+ "space": "~bigbes/rfcs", "document": "SPEC-0007",
+ })
+ require.True(t, res.IsError)
+ require.Contains(t, errorText(res), "claimed by 2 documents")
+ require.Contains(t, errorText(res), "specs/a.md")
+ require.Contains(t, errorText(res), "specs/b.md")
+
+ // Both are still readable by path: a duplicated id is excluded from id
+ // resolution, not from the archive.
+ var out readResult
+ decode(t, call(t, session, "spec_read", map[string]any{
+ "space": "~bigbes/rfcs", "document": "specs/a.md",
+ }), &out)
+ require.Equal(t, "A", out.Title)
+}
+
+// A missing document, space or revision is a tool error with a usable message,
+// never a panic and never an empty document.
+func TestReadMissing(t *testing.T) {
+ r, s := newFixture()
+ session := connect(t, r, s)
+
+ res := call(t, session, "spec_read", map[string]any{
+ "space": "~bigbes/rfcs", "document": "SPEC-9999",
+ })
+ require.True(t, res.IsError)
+ require.Contains(t, errorText(res), `no document "SPEC-9999"`)
+ require.Contains(t, errorText(res), "~bigbes/rfcs")
+
+ res = call(t, session, "spec_read", map[string]any{
+ "space": "~bigbes/nope", "document": "SPEC-0007",
+ })
+ require.True(t, res.IsError)
+ require.Contains(t, errorText(res), "not found")
+
+ res = call(t, session, "spec_read", map[string]any{
+ "space": "~bigbes/rfcs", "document": "SPEC-0007", "rev": blob(99),
+ })
+ require.True(t, res.IsError)
+ require.Contains(t, errorText(res), "not found")
+
+ res = call(t, session, "spec_read", map[string]any{"space": "", "document": "SPEC-0007"})
+ require.True(t, res.IsError)
+ require.Contains(t, errorText(res), "space must not be empty")
+
+ res = call(t, session, "spec_read", map[string]any{"space": "~bigbes/rfcs", "document": " "})
+ require.True(t, res.IsError)
+ require.Contains(t, errorText(res), "document must not be empty")
+}
+
+// A failure below service/ surfaces as a tool error rather than taking the
+// session down.
+func TestReadBackendFailure(t *testing.T) {
+ r, s := newFixture()
+ r.listErr = errors.New("git object store is on fire")
+ session := connect(t, r, s)
+
+ res := call(t, session, "spec_read", map[string]any{
+ "space": "~bigbes/rfcs", "document": "SPEC-0007",
+ })
+ require.True(t, res.IsError)
+ require.Contains(t, errorText(res), "on fire")
+}
+
+// --- search -----------------------------------------------------------------
+
+type searchResult struct {
+ Hits []struct {
+ Space string `json:"space"`
+ ID string `json:"id"`
+ Path string `json:"path"`
+ Rev string `json:"rev"`
+ Anchor string `json:"anchor"`
+ Title string `json:"title"`
+ Section string `json:"section"`
+ Score float64 `json:"score"`
+ Snippet string `json:"snippet"`
+ } `json:"hits"`
+ Total uint64 `json:"total"`
+}
+
+// The spaces argument is the project filter, and it reaches the index as one.
+func TestSearchSpaceFilter(t *testing.T) {
+ r, s := newFixture()
+ session := connect(t, r, s)
+
+ call(t, session, "spec_search", map[string]any{
+ "query": "storage",
+ "spaces": []string{"~bigbes/rfcs", "bigbes/notes"},
+ })
+ require.Equal(t, []core.SpaceRef{
+ {Owner: "bigbes", Name: "rfcs"},
+ {Owner: "bigbes", Name: "notes"},
+ }, s.last.Spaces)
+
+ // Omitting it is the meta-project: a filter that excludes nothing.
+ call(t, session, "spec_search", map[string]any{"query": "storage"})
+ require.Nil(t, s.last.Spaces)
+
+ // Sections pass through the same way.
+ call(t, session, "spec_search", map[string]any{"query": "storage", "sections": []string{"specs"}})
+ require.Equal(t, []string{"specs"}, s.last.Sections)
+}
+
+// An unparseable space is refused rather than dropped: a dropped filter term
+// silently widens the search past what was asked for.
+func TestSearchRejectsBadSpace(t *testing.T) {
+ r, s := newFixture()
+ session := connect(t, r, s)
+
+ res := call(t, session, "spec_search", map[string]any{
+ "query": "storage", "spaces": []string{"~bigbes/rfcs", "not a space ref"},
+ })
+ require.True(t, res.IsError)
+ require.Contains(t, errorText(res), "not a space ref")
+ require.Zero(t, s.calls, "nothing was searched")
+
+ res = call(t, session, "spec_search", map[string]any{"query": " "})
+ require.True(t, res.IsError)
+ require.Contains(t, errorText(res), "query must not be empty")
+ require.Zero(t, s.calls)
+}
+
+func TestSearchLimit(t *testing.T) {
+ r, s := newFixture()
+ session := connect(t, r, s)
+
+ call(t, session, "spec_search", map[string]any{"query": "q"})
+ require.Equal(t, search.DefaultLimit, s.last.Limit)
+
+ call(t, session, "spec_search", map[string]any{"query": "q", "limit": 5, "offset": 10})
+ require.Equal(t, 5, s.last.Limit)
+ require.Equal(t, 10, s.last.Offset)
+
+ call(t, session, "spec_search", map[string]any{"query": "q", "limit": 100000})
+ require.Equal(t, 100, s.last.Limit)
+}
+
+// Hits carry everything spec_read needs, the snippet is plain text, and a log
+// entry's id is one spec_read accepts.
+func TestSearchHitShape(t *testing.T) {
+ r, s := newFixture()
+ s.results = search.Results{
+ Total: 2,
+ Hits: []search.Hit{
+ {
+ Space: fxSpace, ID: "SPEC-0007", Rev: approvedRev,
+ Path: "specs/0007-storage.md", Title: "Storage model", Section: "specs",
+ Score: 1.5, Snippet: "the <mark>approved</mark> body & nothing else",
+ },
+ {
+ Space: fxSpace, ID: "notes/dev-log#2026-05-31-1", Rev: approvedRev,
+ Path: "notes/dev-log.md", Anchor: "2026-05-31-shipped", Section: "log",
+ Title: "2026-05-31 shipped", Score: 0.9,
+ },
+ },
+ }
+ session := connect(t, r, s)
+
+ var out searchResult
+ decode(t, call(t, session, "spec_search", map[string]any{"query": "approved"}), &out)
+
+ require.Equal(t, uint64(2), out.Total)
+ require.Len(t, out.Hits, 2)
+
+ h := out.Hits[0]
+ require.Equal(t, "~bigbes/rfcs", h.Space)
+ require.Equal(t, "SPEC-0007", h.ID)
+ require.Equal(t, "specs/0007-storage.md", h.Path)
+ require.Equal(t, approvedRev, h.Rev)
+ require.Equal(t, "the approved body & nothing else", h.Snippet,
+ "the snippet is plain text: no <mark>, no HTML entities")
+
+ // The indexed id of a log entry carries the entry suffix; the id reported
+ // is the document one, and the entry's position is the anchor.
+ require.Equal(t, "notes/dev-log", out.Hits[1].ID)
+ require.Equal(t, "2026-05-31-shipped", out.Hits[1].Anchor)
+
+ // And that id round-trips through spec_read.
+ r.revs[approvedRev] = append(r.revs[approvedRev], service.Document{
+ Path: "notes/dev-log.md", Blob: blob(7), Rev: approvedRev,
+ Data: []byte("# Dev log\n\n## 2026-05-31 shipped\n\ndone\n"),
+ })
+ var doc readResult
+ decode(t, call(t, session, "spec_read", map[string]any{
+ "space": out.Hits[1].Space, "document": out.Hits[1].ID,
+ }), &doc)
+ require.Equal(t, "notes/dev-log.md", doc.Path)
+}
+
+func TestSearchBackendFailure(t *testing.T) {
+ r, s := newFixture()
+ s.err = errors.New("index is closed")
+ session := connect(t, r, s)
+
+ res := call(t, session, "spec_search", map[string]any{"query": "q"})
+ require.True(t, res.IsError)
+ require.Contains(t, errorText(res), "index is closed")
+}
+
+// --- list -------------------------------------------------------------------
+
+type listResult struct {
+ Spaces []struct {
+ Space string `json:"space"`
+ Owner string `json:"owner"`
+ Name string `json:"name"`
+ } `json:"spaces"`
+ Space string `json:"space"`
+ Rev string `json:"rev"`
+ Documents []struct {
+ ID string `json:"id"`
+ DocID string `json:"doc_id"`
+ Path string `json:"path"`
+ Blob string `json:"blob"`
+ Title string `json:"title"`
+ Section string `json:"section"`
+ Status string `json:"status"`
+ } `json:"documents"`
+}
+
+func TestListSpaces(t *testing.T) {
+ r, s := newFixture()
+ session := connect(t, r, s)
+
+ var out listResult
+ decode(t, call(t, session, "spec_list", map[string]any{}), &out)
+
+ require.Len(t, out.Spaces, 2)
+ require.Equal(t, "~bigbes/rfcs", out.Spaces[0].Space)
+ require.Equal(t, "bigbes", out.Spaces[0].Owner)
+ require.Equal(t, "rfcs", out.Spaces[0].Name)
+ require.Empty(t, out.Documents)
+}
+
+func TestListDocuments(t *testing.T) {
+ r, s := newFixture()
+ session := connect(t, r, s)
+
+ var out listResult
+ decode(t, call(t, session, "spec_list", map[string]any{"space": "~bigbes/rfcs"}), &out)
+
+ require.Equal(t, "~bigbes/rfcs", out.Space)
+ require.Equal(t, approvedRev, out.Rev, "listing defaults to the approved head, like every other read")
+ require.Len(t, out.Documents, 2)
+ require.Empty(t, out.Spaces)
+
+ byID := map[string]string{}
+ for _, d := range out.Documents {
+ byID[d.ID] = d.Path
+ }
+ ids := make([]string, 0, len(byID))
+ for id := range byID {
+ ids = append(ids, id)
+ }
+ sort.Strings(ids)
+ require.Equal(t, []string{"SPEC-0007", "notes/untitled"}, ids)
+ require.Equal(t, "specs/0007-storage.md", byID["SPEC-0007"])
+
+ // A pinned revision lists that revision.
+ decode(t, call(t, session, "spec_list", map[string]any{
+ "space": "~bigbes/rfcs", "rev": olderRev,
+ }), &out)
+ require.Equal(t, olderRev, out.Rev)
+ require.Len(t, out.Documents, 1)
+ require.Equal(t, "draft", out.Documents[0].Status)
+}
+
+// A rev with no space is a caller that meant to name one. Answering the other
+// question would look like it had worked.
+func TestListRevWithoutSpace(t *testing.T) {
+ r, s := newFixture()
+ session := connect(t, r, s)
+
+ res := call(t, session, "spec_list", map[string]any{"rev": approvedRev})
+ require.True(t, res.IsError)
+ require.Contains(t, errorText(res), "pass space as well")
+
+ res = call(t, session, "spec_list", map[string]any{"space": "~bigbes/rfcs", "rev": "proposals/42"})
+ require.True(t, res.IsError)
+ require.Contains(t, errorText(res), "object name")
+}
+
+// --- wiring -----------------------------------------------------------------
+
+// Only read tools exist. The write tools are Phase 3, and a half-wired one is
+// worse than none: an agent that sees spec_propose will call it.
+func TestOnlyReadToolsAreRegistered(t *testing.T) {
+ r, s := newFixture()
+ session := connect(t, r, s)
+
+ var names []string
+ for tool, err := range session.Tools(context.Background(), nil) {
+ require.NoError(t, err)
+ names = append(names, tool.Name)
+ require.True(t, tool.Annotations.ReadOnlyHint, "%s is not annotated read-only", tool.Name)
+ require.NotEmpty(t, tool.Description)
+ }
+ sort.Strings(names)
+ require.Equal(t, []string{"spec_list", "spec_read", "spec_search"}, names)
+}
+
+func TestNewRefusesAnIncompleteBackend(t *testing.T) {
+ r, s := newFixture()
+
+ _, err := mcpsrv.New(mcpsrv.Backend{Index: s}, "test")
+ require.ErrorContains(t, err, "no document reader")
+
+ _, err = mcpsrv.New(mcpsrv.Backend{Docs: r}, "test")
+ require.ErrorContains(t, err, "no search index")
+
+ _, err = mcpsrv.Handler(mcpsrv.Backend{}, "test", "https://spec.srht.bigb.es")
+ require.Error(t, err)
+}
A mcpsrv/read.go => mcpsrv/read.go +97 -0
@@ 0,0 1,97 @@
+package mcpsrv
+
+import (
+ "context"
+ "fmt"
+
+ "sourcecraft.dev/bigbes/sr-ht-spec/service"
+)
+
+type readInput struct {
+ Space string `json:"space" jsonschema:"the space to read from, written \"~owner/name\" as spec_list and spec_search report it"`
+ Document string `json:"document" jsonschema:"which document: its frontmatter id (\"SPEC-0007\"), or its path with or without the \".md\" extension (\"specs/0007-storage\")"`
+ Rev string `json:"rev,omitempty" jsonschema:"pin the read to one immutable revision, given as a git object name (lowercase hex, as returned in the rev field of any result). Omit this to read the space's approved head, which is what an agent almost always wants: the reviewed text. Branch names are not accepted."`
+}
+
+type readOutput struct {
+ Space string `json:"space"`
+ // ID is how this document is addressed: its frontmatter id when that is
+ // well-formed and unique in the space, otherwise its path without the
+ // extension.
+ ID string `json:"id"`
+ // DocID is the frontmatter id when the document has a well-formed one, and
+ // empty otherwise. It differs from ID exactly when the document has no
+ // usable id, which is the case an agent proposing an edit needs to see.
+ DocID string `json:"doc_id,omitempty"`
+ Path string `json:"path"`
+ // Rev is the commit this content was read at. It is immutable: pass it back
+ // as the rev argument to re-read these exact bytes. It is also the value
+ // the Phase 3 write plane takes as If-Match when it is an approved head.
+ Rev string `json:"rev"`
+ // Blob is the sha of this document's content, and changes only when the
+ // content does.
+ Blob string `json:"blob"`
+ // Pinned reports how Rev was chosen. False means the caller named no
+ // revision and this is the space's approved head as of this call — the
+ // reviewed, canonical text. True means the caller pinned a revision, and
+ // whether that revision is on the approved branch is the caller's business:
+ // this service does not claim it either way.
+ Pinned bool `json:"pinned"`
+ Title string `json:"title,omitempty"`
+ Section string `json:"section,omitempty"`
+ // Status is the document's authored lifecycle marker (draft, review,
+ // superseded). It is not approval state: a document is approved by being
+ // reachable from the approved ref, never by its frontmatter.
+ Status string `json:"status,omitempty"`
+ Summary string `json:"summary,omitempty"`
+ Tags []string `json:"tags,omitempty"`
+ // Markdown is the whole document, frontmatter included, exactly as stored.
+ // It is the text to edit and send back when proposing a change.
+ Markdown string `json:"markdown"`
+}
+
+func readHandler(ctx context.Context, b Backend, in readInput) (readOutput, error) {
+ ref, err := parseSpace(in.Space)
+ if err != nil {
+ return readOutput{}, err
+ }
+ rev, err := parseRev(in.Rev)
+ if err != nil {
+ return readOutput{}, err
+ }
+ sp, err := b.Docs.OpenSpace(ctx, ref)
+ if err != nil {
+ return readOutput{}, err
+ }
+ arc, bodies, resolved, err := archiveAt(ctx, b, sp, rev)
+ if err != nil {
+ return readOutput{}, err
+ }
+ page, err := resolvePage(arc, in.Document)
+ if err != nil {
+ return readOutput{}, err
+ }
+ body, ok := bodies[page.Path]
+ if !ok {
+ // The archive is built from these very bodies, so a page without one
+ // is a broken invariant rather than a missing document. Returning an
+ // empty markdown field would be indistinguishable from an empty
+ // document.
+ return readOutput{}, fmt.Errorf("document %s at %s in %s has no body", page.Path, resolved, ref)
+ }
+ return readOutput{
+ Space: ref.String(),
+ ID: page.ID,
+ DocID: page.DocID,
+ Path: page.Path,
+ Rev: resolved,
+ Blob: page.Blob,
+ Pinned: rev != service.ApprovedRev,
+ Title: page.Title,
+ Section: page.Section,
+ Status: string(page.Status),
+ Summary: page.Summary,
+ Tags: page.Tags,
+ Markdown: string(body),
+ }, nil
+}
A mcpsrv/search.go => mcpsrv/search.go +169 -0
@@ 0,0 1,169 @@
+package mcpsrv
+
+import (
+ "context"
+ "errors"
+ "html"
+ "strings"
+
+ "sourcecraft.dev/bigbes/sr-ht-spec/core"
+ "sourcecraft.dev/bigbes/sr-ht-spec/search"
+)
+
+type searchInput struct {
+ Query string `json:"query" jsonschema:"what to search for; matched over document titles and body text in both English and Russian"`
+ // Spaces is the project filter. The design's "a project is a saved filter
+ // over one global index, not a container" is this argument and nothing
+ // else, which is why it is a space list rather than a project name: a
+ // project's membership is resolved by whoever holds the project row, and
+ // what search takes is the set it resolves to.
+ Spaces []string `json:"spaces,omitempty" jsonschema:"restrict the search to these spaces, each written \"~owner/name\" as spec_list reports them. This is the project filter: a project on this service is a named set of spaces. Omit to search every space."`
+ Sections []string `json:"sections,omitempty" jsonschema:"restrict the search to these top-level sections (\"specs\", \"notes\", \"reports\"). Omit to search every section except the dated activity log; name \"log\" to search that."`
+ Limit int `json:"limit,omitempty" jsonschema:"maximum number of hits to return, clamped to 1..100"`
+ Offset int `json:"offset,omitempty" jsonschema:"how many of the top-ranked hits to skip, for paging through a large result set"`
+}
+
+// searchHit is one result as an agent sees it.
+//
+// It is search.Hit reshaped for a tool caller rather than the type itself:
+// Snippet becomes plain text, and ID is split from Anchor so that every id
+// reported here is one spec_read accepts.
+type searchHit struct {
+ Space string `json:"space"`
+ // ID addresses the document in spec_read.
+ ID string `json:"id"`
+ // Path is the document's path in the space's git tree.
+ Path string `json:"path,omitempty"`
+ // Rev is the revision the document was indexed at — the approved head of
+ // its space at index time. Pass it to spec_read to pin the read to exactly
+ // what was searched.
+ Rev string `json:"rev,omitempty"`
+ // Anchor is the heading fragment a hit inside a dated activity log lands
+ // on. Empty for an ordinary document.
+ Anchor string `json:"anchor,omitempty"`
+ Title string `json:"title,omitempty"`
+ Section string `json:"section,omitempty"`
+ Score float64 `json:"score"`
+ // Snippet is the matching fragment as plain text.
+ Snippet string `json:"snippet,omitempty"`
+}
+
+type searchOutput struct {
+ Hits []searchHit `json:"hits"`
+ // Total is how many documents matched, not how many are in Hits.
+ Total uint64 `json:"total"`
+}
+
+func searchHandler(ctx context.Context, b Backend, in searchInput) (searchOutput, error) {
+ text := strings.TrimSpace(in.Query)
+ if text == "" {
+ return searchOutput{}, errors.New("query must not be empty")
+ }
+ spaces, err := parseSpaceFilter(in.Spaces)
+ if err != nil {
+ return searchOutput{}, err
+ }
+ sections, err := trimAll("section", in.Sections)
+ if err != nil {
+ return searchOutput{}, err
+ }
+
+ res, err := b.Index.Search(ctx, search.Query{
+ Text: text,
+ Spaces: spaces,
+ Sections: sections,
+ Limit: clampLimit(in.Limit),
+ Offset: in.Offset,
+ })
+ if err != nil {
+ return searchOutput{}, err
+ }
+
+ out := searchOutput{Hits: make([]searchHit, 0, len(res.Hits)), Total: res.Total}
+ for _, h := range res.Hits {
+ out.Hits = append(out.Hits, searchHit{
+ Space: h.Space.String(),
+ ID: documentID(h.ID),
+ Path: h.Path,
+ Rev: h.Rev,
+ Anchor: h.Anchor,
+ Title: h.Title,
+ Section: h.Section,
+ Score: h.Score,
+ Snippet: plainSnippet(h.Snippet),
+ })
+ }
+ return out, nil
+}
+
+// parseSpaceFilter validates the project filter. An unparseable space is an
+// error rather than a dropped filter term: dropping one would silently widen
+// the search past the set the caller asked for, and a wider answer than
+// requested is indistinguishable from a correct one.
+func parseSpaceFilter(in []string) ([]core.SpaceRef, error) {
+ if len(in) == 0 {
+ return nil, nil
+ }
+ out := make([]core.SpaceRef, 0, len(in))
+ for _, s := range in {
+ ref, err := parseSpace(s)
+ if err != nil {
+ return nil, err
+ }
+ out = append(out, ref)
+ }
+ return out, nil
+}
+
+// trimAll trims each element and refuses an empty one. search/ rejects an empty
+// filter term outright; catching it here names the argument that carried it.
+func trimAll(what string, in []string) ([]string, error) {
+ if len(in) == 0 {
+ return nil, nil
+ }
+ out := make([]string, 0, len(in))
+ for _, s := range in {
+ t := strings.TrimSpace(s)
+ if t == "" {
+ return nil, errors.New(what + " must not be empty")
+ }
+ out = append(out, t)
+ }
+ return out, nil
+}
+
+// documentID strips the entry suffix an activity-log hit carries.
+//
+// search/ indexes each dated entry of a log as its own document under
+// "<page id>#<date>-<n>", so that a hit lands on the entry rather than on the
+// whole log. That id is not a document id: spec_read would not resolve it. The
+// entry's position is already reported separately as Anchor, so the split loses
+// nothing and makes every id in a search result one an agent can hand straight
+// to spec_read.
+func documentID(id string) string {
+ if i := strings.IndexByte(id, '#'); i >= 0 {
+ return id[:i]
+ }
+ return id
+}
+
+// plainSnippet converts bleve's highlighted fragment to plain text.
+//
+// search.Hit.Snippet is HTML: the matched terms are wrapped in <mark> and
+// everything around them is HTML-escaped, because the web UI renders it. A tool
+// result is not rendered, so leaving it would show an agent literal "&" and
+// "<mark>" and invite it to copy them into prose. Both are undone exactly
+// rather than by a general tag stripper: the only markup bleve's formatter
+// emits is that one tag pair, so removing it and unescaping restores the
+// document's own text byte for byte.
+//
+// This is coupled to search/'s choice of highlighter. If that ever stops being
+// bleve's default HTML formatter, this must change with it.
+func plainSnippet(s string) string {
+ if s == "" {
+ return ""
+ }
+ s = strings.ReplaceAll(s, "<mark>", "")
+ s = strings.ReplaceAll(s, "</mark>", "")
+ return html.UnescapeString(s)
+}
M service/read.go => service/read.go +31 -0
@@ 150,6 150,9 @@ func (s *Service) resolveRev(ctx context.Context, sp *Space, rev string) (plumbi
if sp == nil || sp.Repo == nil {
return plumbing.ZeroHash, "", errors.New("service: space has no open repository")
}
+ if err := ValidateReadRev(rev); err != nil {
+ return plumbing.ZeroHash, "", err
+ }
resolved := rev
if resolved == ApprovedRev {
resolved = sp.Repo.ApprovedBranch()
@@ 161,6 164,34 @@ func (s *Service) resolveRev(ctx context.Context, sp *Space, rev string) (plumbi
return commit, resolved, nil
}
+// ValidateReadRev enforces the read contract: the read plane serves the approved
+// head by default, and otherwise only an immutable object name.
+//
+// gitx.ResolveRev happily resolves ref names, so without this guard a caller
+// could pass rev="proposals/42" and have the READ plane hand back unreviewed
+// proposal content — the single failure this service exists to prevent, since
+// that text would then flow into agent context as though it were approved.
+// Reading a proposal branch is a deliberate act belonging to the review path,
+// not something any read surface can be talked into.
+//
+// Object names are required to be full: an abbreviation that is unique today
+// can become ambiguous later, so a pinned revision would silently stop meaning
+// one thing.
+func ValidateReadRev(rev string) error {
+ if rev == ApprovedRev {
+ return nil
+ }
+ if len(rev) != 40 {
+ return fmt.Errorf("%w: revision %q must be a full 40-character object name", ErrBadReadRev, rev)
+ }
+ for _, c := range rev {
+ if (c < '0' || c > '9') && (c < 'a' || c > 'f') {
+ return fmt.Errorf("%w: revision %q must be a full 40-character object name", ErrBadReadRev, rev)
+ }
+ }
+ return nil
+}
+
// readErr maps a gitx failure onto this package's sentinels so callers above
// service/ can branch on it without importing gitx. ErrNotFound and ErrBadRev
// both become ErrNotFound at this boundary — a crafted revision must not be
A service/readrev_test.go => service/readrev_test.go +46 -0
@@ 0,0 1,46 @@
+package service
+
+import (
+ "errors"
+ "strings"
+ "testing"
+)
+
+// The read plane must never be talkable into serving proposal content. gitx
+// resolves ref names happily, so this guard is the only thing standing between
+// a crafted rev and unreviewed text entering an agent's context as approved.
+func TestValidateReadRevRefusesRefNames(t *testing.T) {
+ sha := strings.Repeat("a1b2c3d4", 5) // 40 hex chars
+
+ for _, tc := range []struct {
+ name string
+ rev string
+ ok bool
+ }{
+ {"approved head sentinel", ApprovedRev, true},
+ {"full object name", sha, true},
+ {"proposal branch", "proposals/42", false},
+ {"approved branch by name", "main", false},
+ {"HEAD", "HEAD", false},
+ {"tag", "refs/tags/v1", false},
+ {"abbreviated sha", sha[:12], false},
+ {"uppercase hex", strings.ToUpper(sha), false},
+ {"sha with trailing path", sha + "/x", false},
+ {"almost hex", strings.Repeat("g", 40), false},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ err := ValidateReadRev(tc.rev)
+ if tc.ok && err != nil {
+ t.Fatalf("ValidateReadRev(%q) = %v, want nil", tc.rev, err)
+ }
+ if !tc.ok {
+ if err == nil {
+ t.Fatalf("ValidateReadRev(%q) = nil, want rejection", tc.rev)
+ }
+ if !errors.Is(err, ErrBadReadRev) {
+ t.Fatalf("ValidateReadRev(%q) error = %v, want ErrBadReadRev", tc.rev, err)
+ }
+ }
+ })
+ }
+}
M service/service.go => service/service.go +2 -0
@@ 32,6 32,8 @@ var (
// ErrNotFound marks a missing space, revision, document or row.
ErrNotFound = errors.New("service: not found")
+ // ErrBadReadRev rejects a revision that is not an immutable object name.
+ ErrBadReadRev = errors.New("service: revision must be an object name")
// ErrSpaceExists marks a create that would clobber an existing space,
// either its repository on disk or its row.