~bigbes/sr-ht-spec

42d4ee137ee6f4c70dd2ce4097c21606151f4a29 — Eugene Blikh 26 days ago f8a7a17
feat(mcpsrv): spec_propose write tool (Phase 3)

The agent-facing half of the write plane over MCP. spec_propose uploads
whole documents and returns {proposal, url, merged}, calling the same
service.Propose the REST PUT will — one implementation of If-Match,
provenance and auto-merge behind both surfaces, never two that drift.

- Writer is an optional Backend field: nil keeps the server read-only
  (the three read tools, unchanged), so a read-only deploy or a test
  needs no mutable backend. Set, it registers spec_propose with neither
  the read-only nor the idempotent hint — proposing twice opens two
  proposals.
- The acting agent is resolved from the bearer token on the tool call by
  the resolver middleware now installed on /mcp; the read tools never
  needed it. The ACL stays in service/: service.Propose refuses a
  non-agent, so an anonymous or owner caller is rejected there.
4 files changed, 236 insertions(+), 1 deletions(-)

M cmd/specsrht/main.go
M mcpsrv/mcpsrv.go
A mcpsrv/propose.go
A mcpsrv/propose_internal_test.go
M cmd/specsrht/main.go => cmd/specsrht/main.go +6 -1
@@ 471,11 471,16 @@ func newSurfaces(conf ini.File, cfg service.Config, svc *service.Service, versio
	// guard cannot tell a reverse proxy from an attacker (both reach a loopback
	// listener with a non-loopback Host), so it is replaced by a check against
	// this value. Traefik must pass the Host header through or every call 403s.
	mcp, err := mcpsrv.Handler(mcpsrv.Backend{Docs: svc, Index: index}, version, cfg.Origin)
	mcp, err := mcpsrv.Handler(mcpsrv.Backend{Docs: svc, Index: index, Write: svc}, version, cfg.Origin)
	if err != nil {
		index.Close()
		return nil, fmt.Errorf("assemble the MCP surface: %w", err)
	}
	// spec_propose resolves the acting agent from the bearer token on the tool
	// call, so /mcp needs the principal middleware the read tools never did.
	// It sets an anonymous principal when there is no token, which service.Propose
	// refuses — the ACL stays in service/, this only populates the identity.
	mcp = svc.Resolver().Middleware()(mcp)

	gql, err := graph.New(graph.Options{
		Reader:    svc,

M mcpsrv/mcpsrv.go => mcpsrv/mcpsrv.go +30 -0
@@ 77,6 77,12 @@ type Backend struct {
	Docs Reader
	// Index is the one global bleve index — in production *search.Index.
	Index Searcher
	// Write is the write side — in production *service.Service. It is optional:
	// when nil the server registers only the read tools, so a read-only
	// deployment or a test needs no mutable backend. When set, spec_propose is
	// registered and every write goes through the same service.Propose the REST
	// PUT calls.
	Write Writer
}

// New builds the MCP server with the Phase 2 read tools registered. version is


@@ 144,6 150,30 @@ func New(b Backend, version string) (*mcp.Server, error) {
		return nil, out, err
	})

	// The one write tool, registered only when a write backend is wired. It is
	// not read-only or idempotent — proposing twice opens two proposals — so it
	// carries neither hint, which is how a client tells a tool it can retry
	// freely from one it cannot.
	if b.Write != nil {
		mcp.AddTool(srv, &mcp.Tool{
			Name: "spec_propose",
			Description: "Propose a change to a space: upload whole documents and get back a proposal " +
				"and a URL to hand a human for review.\n\n" +
				"Pass `if_match` as the `rev` you read the approved head at (from spec_read) — it becomes " +
				"the proposal's base, and a base the approved branch has moved off is rejected so you " +
				"refetch and re-propose. Each document in `documents` is the WHOLE markdown, frontmatter " +
				"included; there are no patches.\n\n" +
				"Omit `proposal` to open a new one (give it a `title`); pass an existing proposal id to add " +
				"more documents to it, sending the same `if_match` you opened it with.\n\n" +
				"The response always carries the proposal `url`. Surface it: the human reviews there, and a " +
				"proposal whose link you never mention is invisible. When `merged` is true the space's " +
				"auto_merge policy landed the change immediately; otherwise it is open and waiting.",
		}, func(ctx context.Context, _ *mcp.CallToolRequest, in proposeInput) (*mcp.CallToolResult, proposeOutput, error) {
			out, err := proposeHandler(ctx, b.Write, in)
			return nil, out, err
		})
	}

	return srv, nil
}


A mcpsrv/propose.go => mcpsrv/propose.go +105 -0
@@ 0,0 1,105 @@
package mcpsrv

import (
	"context"
	"fmt"

	"sourcecraft.dev/bigbes/sr-ht-spec/authn"
	"sourcecraft.dev/bigbes/sr-ht-spec/service"
)

// Writer is the write side of the orchestration layer the propose tool calls —
// the same service.Propose that the REST PUT calls, so the two write surfaces
// share one implementation of If-Match, provenance and auto-merge rather than
// drifting apart. *service.Service satisfies it.
//
// It is a distinct interface from Reader, and optional on the Backend, because
// the read tools need no write path and a read-only deployment (or a test)
// should be able to register the three read tools without a service that can
// mutate anything.
type Writer interface {
	Propose(ctx context.Context, req service.ProposeRequest) (service.ProposeResult, error)
}

// proposeDoc is one whole-document upload. The write plane takes whole
// documents, not patches — that is how agents work and what makes the merge
// model plumbing — so an agent sends the full markdown it wants the document to
// have, frontmatter included.
type proposeDoc struct {
	Path    string `json:"path" jsonschema:"the document's path in the space, e.g. \"specs/0007-storage.md\""`
	Content string `json:"content" jsonschema:"the whole document, frontmatter included, exactly as it should be stored"`
}

type proposeInput struct {
	Space string `json:"space" jsonschema:"the space to propose against, written \"~owner/name\""`
	// IfMatch is the base: the approved-head sha the agent read the document at,
	// which is the rev field spec_read returns for an approved read. It pins the
	// proposal's base and is what staleness is measured against.
	IfMatch   string       `json:"if_match" jsonschema:"the approved-head revision you read at — the rev value from spec_read of the approved head. It becomes the proposal's base; a base the approved branch has moved off is rejected."`
	Proposal  int          `json:"proposal,omitempty" jsonschema:"add these documents to an existing open proposal with this id, rather than opening a new one. Omit to open a new proposal."`
	Title     string       `json:"title,omitempty" jsonschema:"a short title for a new proposal (required when opening one, ignored when adding)"`
	Rationale string       `json:"rationale,omitempty" jsonschema:"why the change is proposed, for the reviewer"`
	Message   string       `json:"message,omitempty" jsonschema:"the commit message for this write; defaults to the title when opening"`
	Documents []proposeDoc `json:"documents" jsonschema:"the whole documents to write, at least one"`
}

type proposeOutput struct {
	// Proposal is the proposal id, and Url the stable link to hand a human. An
	// agent that proposes without surfacing the url makes the work invisible.
	Proposal int    `json:"proposal"`
	URL      string `json:"url"`
	// Merged reports whether auto-merge policy landed this immediately. When
	// true the change is already on the approved head; when false it is open and
	// waiting for a human, and the url is where they review it.
	Merged bool `json:"merged"`
	// State is the proposal's lifecycle state after this write: "open" or, when
	// policy auto-merged, "merged".
	State string `json:"state"`
	// Branch is the proposal branch, "proposals/<id>". BaseRev is the base the
	// proposal is measured against — the value to keep sending as if_match when
	// adding to this proposal.
	Branch  string `json:"branch"`
	BaseRev string `json:"base_rev"`
}

func proposeHandler(ctx context.Context, w Writer, in proposeInput) (proposeOutput, error) {
	ref, err := parseSpace(in.Space)
	if err != nil {
		return proposeOutput{}, err
	}
	if len(in.Documents) == 0 {
		return proposeOutput{}, fmt.Errorf("documents must not be empty; a proposal writes at least one whole document")
	}
	writes := make([]service.DocumentWrite, 0, len(in.Documents))
	for _, d := range in.Documents {
		writes = append(writes, service.DocumentWrite{Path: d.Path, Content: []byte(d.Content)})
	}

	// The principal is resolved by the resolver middleware on /mcp from the
	// bearer token on this very request. service.Propose refuses a non-agent, so
	// an anonymous or owner caller is rejected there rather than here — the ACL
	// has one home, in service/.
	principal := authn.PrincipalFromContext(ctx)

	res, err := w.Propose(ctx, service.ProposeRequest{
		Space:      ref,
		Principal:  principal,
		ProposalID: in.Proposal,
		Title:      in.Title,
		Rationale:  in.Rationale,
		IfMatch:    in.IfMatch,
		Message:    in.Message,
		Writes:     writes,
	})
	if err != nil {
		return proposeOutput{}, err
	}
	return proposeOutput{
		Proposal: res.Proposal.ID,
		URL:      res.URL,
		Merged:   res.Merged,
		State:    string(res.Proposal.State),
		Branch:   res.Proposal.Branch,
		BaseRev:  res.Proposal.BaseRev,
	}, nil
}

A mcpsrv/propose_internal_test.go => mcpsrv/propose_internal_test.go +95 -0
@@ 0,0 1,95 @@
package mcpsrv

import (
	"context"
	"testing"

	"sourcecraft.dev/bigbes/sr-ht-spec/authn"
	"sourcecraft.dev/bigbes/sr-ht-spec/core"
	"sourcecraft.dev/bigbes/sr-ht-spec/service"
)

// fakeWriter captures the request the tool builds and returns a canned result,
// so the handler's mapping — arguments in, principal from context, result out —
// is checked without a service, a repository or a database.
type fakeWriter struct {
	got service.ProposeRequest
	res service.ProposeResult
	err error
}

func (f *fakeWriter) Propose(_ context.Context, req service.ProposeRequest) (service.ProposeResult, error) {
	f.got = req
	return f.res, f.err
}

// TestProposeHandlerForwardsPrincipalAndArgs proves the tool passes the acting
// agent (resolved onto the request context by the /mcp middleware) and every
// argument through to service.Propose, and maps the result — id, url, merged —
// back out.
func TestProposeHandlerForwardsPrincipalAndArgs(t *testing.T) {
	principal := authn.Principal{
		Kind:    authn.KindAgent,
		Owner:   "bigbes",
		Agent:   "claude-code/spec-writer",
		Session: "sess-1",
	}
	ctx := authn.WithPrincipal(context.Background(), principal)

	w := &fakeWriter{res: service.ProposeResult{
		Proposal: service.Proposal{
			ID:      7,
			Branch:  "proposals/7",
			BaseRev: "1f0c1d1a1e2b3c4d5e6f708192a3b4c5d6e7f809",
			State:   core.StateOpen,
		},
		URL:    "https://spec.srht.bigb.es/~bigbes/rfcs/p/7",
		Merged: false,
	}}

	out, err := proposeHandler(ctx, w, proposeInput{
		Space:     "~bigbes/rfcs",
		IfMatch:   "1f0c1d1a1e2b3c4d5e6f708192a3b4c5d6e7f809",
		Title:     "Add a note",
		Rationale: "because",
		Message:   "write it",
		Documents: []proposeDoc{{Path: "notes/a.md", Content: "---\nid: N-1\n---\n"}},
	})
	if err != nil {
		t.Fatalf("proposeHandler: %v", err)
	}

	// The request the service saw.
	if w.got.Principal != principal {
		t.Errorf("principal = %+v, want the one on the context %+v", w.got.Principal, principal)
	}
	if w.got.Space != (core.SpaceRef{Owner: "bigbes", Name: "rfcs"}) {
		t.Errorf("space = %v, want ~bigbes/rfcs", w.got.Space)
	}
	if w.got.IfMatch != "1f0c1d1a1e2b3c4d5e6f708192a3b4c5d6e7f809" || w.got.Title != "Add a note" {
		t.Errorf("request args not forwarded: %+v", w.got)
	}
	if len(w.got.Writes) != 1 || w.got.Writes[0].Path != "notes/a.md" {
		t.Errorf("writes = %+v, want the one document", w.got.Writes)
	}

	// The result mapped out.
	if out.Proposal != 7 || out.URL != "https://spec.srht.bigb.es/~bigbes/rfcs/p/7" || out.Merged {
		t.Errorf("output = %+v, want id 7, the url, merged=false", out)
	}
	if out.State != "open" || out.Branch != "proposals/7" {
		t.Errorf("output state/branch = %q/%q, want open/proposals/7", out.State, out.Branch)
	}
}

// TestProposeHandlerRejectsEmptyDocuments refuses a call with no documents
// before it reaches the service.
func TestProposeHandlerRejectsEmptyDocuments(t *testing.T) {
	w := &fakeWriter{}
	if _, err := proposeHandler(context.Background(), w, proposeInput{Space: "~bigbes/rfcs"}); err == nil {
		t.Fatal("proposeHandler with no documents = nil, want error")
	}
	if w.got.Writes != nil || w.got.Space != (core.SpaceRef{}) {
		t.Fatal("service was called despite empty documents")
	}
}