feat(mcpkit)!: deferred tool registration + Risk/Group primitives (MC-43)

examples/http/main.go

@@ -30,7 +30,7 @@ func (helloCap) Attach(r *mcpkit.Registrar) error {
 	mcpkit.AddTool(r, &mcpx.Tool{
 		Name:        "hello",
 		Description: "greets the caller by name",
-	}, func(_ context.Context, _ *mcpx.CallToolRequest, in helloIn) (*mcpx.CallToolResult, helloOut, error) {
+	}, mcpkit.ReadOnly, func(_ context.Context, _ *mcpx.CallToolRequest, in helloIn) (*mcpx.CallToolResult, helloOut, error) {
 		name := in.Name
 		if name == "" {
 			name = "world"

examples/minimal/main.go

@@ -26,7 +26,7 @@ func (helloCap) Attach(r *mcpkit.Registrar) error {
 	mcpkit.AddTool(r, &mcpx.Tool{
 		Name:        "hello",
 		Description: "greets the caller by name",
-	}, func(_ context.Context, _ *mcpx.CallToolRequest, in helloIn) (*mcpx.CallToolResult, helloOut, error) {
+	}, mcpkit.ReadOnly, func(_ context.Context, _ *mcpx.CallToolRequest, in helloIn) (*mcpx.CallToolResult, helloOut, error) {
 		name := in.Name
 		if name == "" {
 			name = "world"

examples/server/main.go

@@ -33,7 +33,7 @@ func (pingCap) Attach(r *mcpkit.Registrar) error {
 	mcpkit.AddTool(r, &mcpx.Tool{
 		Name:        "ping",
 		Description: "replies with pong",
-	}, func(_ context.Context, _ *mcpx.CallToolRequest, _ pingIn) (*mcpx.CallToolResult, pingOut, error) {
+	}, mcpkit.ReadOnly, func(_ context.Context, _ *mcpx.CallToolRequest, _ pingIn) (*mcpx.CallToolResult, pingOut, error) {
 		return nil, pingOut{Message: "pong"}, nil
 	})
 	return nil

mcpkit.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"sync"
 
 	"github.com/dangernoodle-io/mcpkit/host"
 	"github.com/dangernoodle-io/mcpkit/mcpx"
@@ -22,26 +23,86 @@ type Info struct {
 	Instructions string
 }
 
+// Risk classifies a tool's blast radius, from a client's perspective, for
+// gating and annotation purposes. Every AddTool call requires one
+// (fail-closed): a caller can't omit risk classification and accidentally
+// ship a write/destructive tool unannotated.
+type Risk int
+
+const (
+	// ReadOnly tools only observe state; they never mutate anything a
+	// client cares about.
+	ReadOnly Risk = iota
+	// Write tools mutate state, but the mutation is reversible/benign
+	// enough not to warrant a destructive warning.
+	Write
+	// Destructive tools perform an irreversible or high-blast-radius
+	// mutation (data loss, external side effects that can't be undone).
+	Destructive
+)
+
+// ToolOption configures optional per-tool metadata at AddTool time.
+type ToolOption interface {
+	applyTool(*toolMeta)
+}
+
+// toolMeta is the mutable state ToolOption values apply to.
+type toolMeta struct {
+	group string
+}
+
+// groupOption is the ToolOption Group returns.
+type groupOption string
+
+func (g groupOption) applyTool(m *toolMeta) {
+	m.group = string(g)
+}
+
+// Group tags a tool with an arbitrary consumer-defined group name, recorded
+// in the registry's byGroup bookkeeping post-registration. mcpkit imposes no
+// meaning on the group string; a consumer's own gating (MC-44/MC-45) is what
+// interprets it.
+func Group(name string) ToolOption {
+	return groupOption(name)
+}
+
 // Registrar is what a Capability's Attach method uses to register itself
 // against the underlying server and inspect the target host. Capabilities
 // register tools through the package-level AddTool, not against mcpx
 // directly, so mcpkit owns a single tool-registration chokepoint.
 type Registrar struct {
 	server *mcpx.Server
 	host   host.Adapter
+	reg    *registry
 }
 
 // Host returns the host.Adapter the app is composed for.
 func (r *Registrar) Host() host.Adapter {
 	return r.host
 }
 
-// AddTool registers a typed tool handler through r. This is the only
-// tool-registration chokepoint capabilities should use; MC-8 wraps every
-// handler in a panic-recover here so a panicking tool surfaces as an
-// IsError result instead of crashing the server process. Annotations/risk
-// remain future-additive at this same chokepoint.
-func AddTool[In, Out any](r *Registrar, t *mcpx.Tool, h mcpx.Handler[In, Out]) {
+// AddTool captures a typed tool handler through r for deferred registration
+// against the underlying server. This is the only tool-registration
+// chokepoint capabilities should use; MC-8 wraps every handler in a
+// panic-recover here so a panicking tool surfaces as an IsError result
+// instead of crashing the server process. Registration itself is deferred
+// until the App's finalize runs (MC-43) so a later gate (MC-44) can filter
+// which pending tools actually register before anything is exposed to a
+// client.
+//
+// risk is required (fail-closed): a caller cannot omit a tool's risk
+// classification. When t.Annotations is nil, AddTool derives it from risk
+// via mcpx.RiskAnnotations; an explicitly-set Annotations is left untouched.
+func AddTool[In, Out any](r *Registrar, t *mcpx.Tool, risk Risk, h mcpx.Handler[In, Out], opts ...ToolOption) {
+	if t.Annotations == nil {
+		t.Annotations = mcpx.RiskAnnotations(risk == ReadOnly, risk == Destructive)
+	}
+
+	meta := toolMeta{}
+	for _, opt := range opts {
+		opt.applyTool(&meta)
+	}
+
 	wrapped := func(ctx context.Context, req *mcpx.CallToolRequest, in In) (res *mcpx.CallToolResult, out Out, err error) {
 		defer func() {
 			if p := recover(); p != nil {
@@ -50,7 +111,67 @@ func AddTool[In, Out any](r *Registrar, t *mcpx.Tool, h mcpx.Handler[In, Out]) {
 		}()
 		return h(ctx, req, in)
 	}
-	mcpx.AddTool(r.server, t, wrapped)
+
+	r.reg.add(pendingTool{
+		name:  t.Name,
+		group: meta.group,
+		risk:  risk,
+		register: func(s *mcpx.Server) {
+			mcpx.AddTool(s, t, wrapped)
+		},
+	})
+}
+
+// pendingTool is one captured-but-not-yet-registered AddTool call. register
+// closes over the tool's In/Out type parameters (erased here) and the
+// panic-recover-wrapped handler; calling it performs the actual
+// mcpx.AddTool registration against a live server.
+type pendingTool struct {
+	name, group string
+	risk        Risk
+	register    func(*mcpx.Server)
+}
+
+// registry accumulates pending tool registrations shared between a
+// Registrar (during composition, via AddTool) and its App (at finalize
+// time). Deferring registration out of AddTool is what lets a later gate
+// (MC-44) filter which pending tools actually register before finalize ever
+// touches the live server.
+type registry struct {
+	mu      sync.Mutex
+	pending []pendingTool
+	byGroup map[string][]string
+	started bool
+}
+
+// add appends t to the pending set. Safe for concurrent Attach calls.
+func (reg *registry) add(t pendingTool) {
+	reg.mu.Lock()
+	defer reg.mu.Unlock()
+	reg.pending = append(reg.pending, t)
+}
+
+// finalize registers every pending tool against srv exactly once. It is
+// idempotent: a second call (e.g. Run then Connect, or Run called twice) is
+// a guarded no-op rather than double-registering or panicking.
+func (reg *registry) finalize(srv *mcpx.Server) {
+	reg.mu.Lock()
+	defer reg.mu.Unlock()
+
+	if reg.started {
+		return
+	}
+	reg.started = true
+
+	if reg.byGroup == nil {
+		reg.byGroup = make(map[string][]string)
+	}
+
+	for _, t := range reg.pending {
+		// MC-44 gate predicate slots in here (a per-tool allow check that may `continue`).
+		t.register(srv)
+		reg.byGroup[t.group] = append(reg.byGroup[t.group], t.name)
+	}
 }
 
 // Capability is a self-contained unit of server functionality, attached to
@@ -63,43 +184,61 @@ type Capability interface {
 type App struct {
 	server *mcpx.Server
 	host   host.Adapter
+	reg    *registry
 }
 
 // New composes an App from a host.Adapter and zero or more Capabilities,
-// attaching each capability in order.
+// attaching each capability in order. Tool registration is deferred: no
+// tool is registered against the underlying server until Run, Connect, or
+// HTTPHandler calls finalize.
 func New(info Info, h host.Adapter, caps ...Capability) (*App, error) {
 	if h == nil {
 		return nil, fmt.Errorf("mcpkit: host adapter must not be nil")
 	}
 
 	srv := mcpx.NewServer(mcpx.Implementation{Name: info.Name, Version: info.Version}, info.Instructions)
-	r := &Registrar{server: srv, host: h}
+	reg := &registry{}
+	r := &Registrar{server: srv, host: h, reg: reg}
 
 	for _, c := range caps {
 		if err := c.Attach(r); err != nil {
 			return nil, fmt.Errorf("mcpkit: attach capability: %w", err)
 		}
 	}
 
-	return &App{server: srv, host: h}, nil
+	return &App{server: srv, host: h, reg: reg}, nil
+}
+
+// finalize registers every pending tool against a's server exactly once,
+// regardless of how many of Run/Connect/HTTPHandler trigger it or in what
+// order.
+func (a *App) finalize() {
+	a.reg.finalize(a.server)
 }
 
 // Run serves the app over its host's transport until the client disconnects
 // or ctx is cancelled.
 func (a *App) Run(ctx context.Context) error {
+	a.finalize()
 	return a.server.Run(ctx, a.host.Transport())
 }
 
 // Connect connects the app over t without blocking, for use by testkit and
 // other in-process harnesses.
 func (a *App) Connect(ctx context.Context, t mcpx.Transport) (*mcpx.Session, error) {
+	a.finalize()
 	return a.server.Connect(ctx, t)
 }
 
 // HTTPHandler exposes the composed server over streamable-HTTP for the
 // consumer to mount. mcpkit is path-agnostic: the returned handler is bare
 // and MCP-over-HTTP is entirely opt-in — the consumer decides whether and
-// where to mount it.
+// where to mount it. finalize runs here too (not just Run/Connect) because
+// an HTTP-only consumer (see cli.ServerCmd's --http path and
+// examples/http) never calls Run or Connect at all — without this,
+// deferred registration would silently ship zero tools over HTTP,
+// regressing MC-43's behavior-preservation goal.
 func (a *App) HTTPHandler(opts ...mcpx.HTTPOption) http.Handler {
+	a.finalize()
 	return a.server.HTTPHandler(opts...)
 }

mcpkit_test.go

@@ -28,7 +28,7 @@ func (helloCap) Attach(r *mcpkit.Registrar) error {
 	mcpkit.AddTool(r, &mcpx.Tool{
 		Name:        "hello",
 		Description: "greets the caller by name",
-	}, func(_ context.Context, _ *mcpx.CallToolRequest, in helloIn) (*mcpx.CallToolResult, helloOut, error) {
+	}, mcpkit.ReadOnly, func(_ context.Context, _ *mcpx.CallToolRequest, in helloIn) (*mcpx.CallToolResult, helloOut, error) {
 		name := in.Name
 		if name == "" {
 			name = "world"
@@ -76,7 +76,7 @@ func (panicCap) Attach(r *mcpkit.Registrar) error {
 	mcpkit.AddTool(r, &mcpx.Tool{
 		Name:        "panics",
 		Description: "always panics",
-	}, func(_ context.Context, _ *mcpx.CallToolRequest, _ panicIn) (*mcpx.CallToolResult, panicOut, error) {
+	}, mcpkit.ReadOnly, func(_ context.Context, _ *mcpx.CallToolRequest, _ panicIn) (*mcpx.CallToolResult, panicOut, error) {
 		panic("kaboom")
 	})
 	return nil
@@ -146,7 +146,7 @@ func (annotatedCap) Attach(r *mcpkit.Registrar) error {
 			ReadOnlyHint:    true,
 			DestructiveHint: mcpx.BoolPtr(true),
 		},
-	}, func(_ context.Context, _ *mcpx.CallToolRequest, _ annotatedIn) (*mcpx.CallToolResult, annotatedOut, error) {
+	}, mcpkit.ReadOnly, func(_ context.Context, _ *mcpx.CallToolRequest, _ annotatedIn) (*mcpx.CallToolResult, annotatedOut, error) {
 		return nil, annotatedOut{OK: true}, nil
 	})
 	return nil
@@ -194,3 +194,89 @@ func TestAppHTTPHandler(t *testing.T) {
 	require.Equal(t, http.StatusBadRequest, rec.Code)
 	require.Contains(t, rec.Body.String(), "text/event-stream")
 }
+
+type riskIn struct{}
+
+type riskOut struct{}
+
+// riskCap registers one tool per Risk value, each left with a nil
+// Annotations so AddTool must derive it from risk via mcpx.RiskAnnotations.
+type riskCap struct{}
+
+func (riskCap) Attach(r *mcpkit.Registrar) error {
+	handler := func(_ context.Context, _ *mcpx.CallToolRequest, _ riskIn) (*mcpx.CallToolResult, riskOut, error) {
+		return nil, riskOut{}, nil
+	}
+	mcpkit.AddTool(r, &mcpx.Tool{Name: "risk-readonly", Description: "d"}, mcpkit.ReadOnly, handler)
+	mcpkit.AddTool(r, &mcpx.Tool{Name: "risk-write", Description: "d"}, mcpkit.Write, handler)
+	mcpkit.AddTool(r, &mcpx.Tool{Name: "risk-destructive", Description: "d"}, mcpkit.Destructive, handler)
+	return nil
+}
+
+// TestAddToolRiskAutoAnnotation proves AddTool derives ToolAnnotations from
+// each Risk value via mcpx.RiskAnnotations when the caller left
+// t.Annotations nil: ReadOnlyHint tracks Risk == ReadOnly, and
+// DestructiveHint tracks Risk == Destructive (Write gets neither hint set).
+func TestAddToolRiskAutoAnnotation(t *testing.T) {
+	app, err := mcpkit.New(mcpkit.Info{Name: "risk-e2e", Version: "0.0.1"}, generic.New(), riskCap{})
+	require.NoError(t, err)
+
+	h := testkit.New(t, app)
+
+	res, err := h.ListTools(context.Background())
+	require.NoError(t, err)
+
+	byName := make(map[string]*mcpx.ToolAnnotations, len(res.Tools))
+	for _, tool := range res.Tools {
+		byName[tool.Name] = tool.Annotations
+	}
+
+	ro := byName["risk-readonly"]
+	require.NotNil(t, ro)
+	require.True(t, ro.ReadOnlyHint)
+	require.NotNil(t, ro.DestructiveHint)
+	require.False(t, *ro.DestructiveHint)
+
+	wr := byName["risk-write"]
+	require.NotNil(t, wr)
+	require.False(t, wr.ReadOnlyHint)
+	require.NotNil(t, wr.DestructiveHint)
+	require.False(t, *wr.DestructiveHint)
+
+	de := byName["risk-destructive"]
+	require.NotNil(t, de)
+	require.False(t, de.ReadOnlyHint)
+	require.NotNil(t, de.DestructiveHint)
+	require.True(t, *de.DestructiveHint)
+}
+
+// TestDeferredRegistrationAdvertisesAfterConnect proves the deferred
+// registration MC-43 introduces is behavior-preserving from a client's
+// perspective: a tool registered via AddTool before the first Connect is
+// fully advertised (tools/list) and callable once a session exists.
+func TestDeferredRegistrationAdvertisesAfterConnect(t *testing.T) {
+	app, err := mcpkit.New(mcpkit.Info{Name: "deferred-e2e", Version: "0.0.1"}, generic.New(), helloCap{})
+	require.NoError(t, err)
+
+	h := testkit.New(t, app)
+
+	testkit.AssertToolSet(t, h, "hello")
+}
+
+// TestAppConnectIdempotentAcrossSessions proves a second Connect against the
+// same App (finalize's idempotency guard) neither panics nor loses tools: a
+// second in-memory session still sees the same fully-registered tool set,
+// proving finalize's first run is what registered it and the second run
+// was a no-op rather than a duplicate-registration panic.
+func TestAppConnectIdempotentAcrossSessions(t *testing.T) {
+	app, err := mcpkit.New(mcpkit.Info{Name: "reconnect-e2e", Version: "0.0.1"}, generic.New(), helloCap{})
+	require.NoError(t, err)
+
+	h1 := testkit.New(t, app)
+	testkit.AssertToolSet(t, h1, "hello")
+
+	require.NotPanics(t, func() {
+		h2 := testkit.New(t, app)
+		testkit.AssertToolSet(t, h2, "hello")
+	})
+}