feat: an operation names the tool it is offered as

CHANGELOG.md

@@ -1,5 +1,21 @@
 # Changelog
 
+## Unreleased — an operation names the tool it is offered as
+
+- **`Operation.Tool`** is the model-facing name of a marked read. Empty keeps
+  the previous behaviour exactly: the name falls back to the operationId. It is
+  validated as snake_case within 64 characters and refused rather than
+  sanitised, because core's `toolName` keeps what it is handed, so a capital or
+  a hyphen would reach a model as typed.
+- The operationId does not move. Generated clients are named from it; a nicer
+  tool name is not worth renaming every caller's method.
+- **One name names one operation.** Two reads offering one tool name are
+  refused where a duplicate operationId already is: a tool name is the identity
+  a model addresses, and nothing downstream dedupes on the word.
+- **Only a read is named.** A name on a mutation, a stream, an excluded route
+  or an any-checked-caller route is refused, because none of them is ever
+  marked: the name would be silently dropped while reading as an exposure.
+
 ## Unreleased — every read a solution serves is a Runnable operation, and so a tool
 
 - **The artifact marks each read (`x-codefly-operation`).** An operation whose

README.md

@@ -662,7 +662,22 @@ core's `x-codefly-operation` marker — the proto3 JSON of
 (`runnable.RouteMarker`): recomputed on an uncertain outcome, core's default
 bounds, invoked under exactly the scopes the policy requires, for the audience
 this runtime's hop checks (`Manifest.ID`), presented as a read-only tool named
-by the operationId and described by the `Summary`. `codefly generate runnables`
+by `Operation.Tool` — or, left empty, by the operationId — and described by the
+`Summary`.
+
+`Operation.Tool` is the one part of the marker the owner says rather than core
+derives, because it is the one part a model reads: snake_case, at most 64
+characters, refused at validation rather than sanitised, since core's own
+`toolName` keeps whatever it is given. One name names one operation, as an
+operationId names one route — two reads claiming one name publish two tools a
+model cannot tell apart, and nothing downstream collapses them. And only a read
+is named, because only a read is marked: a name on a mutation, a stream, an
+excluded route or an any-checked-caller route is a declaration that does
+nothing and reads as the opposite, so it is refused. The fallback identifies the route and
+describes nothing — `Wiki_Get_V1_Wiki_Upload-configuration` says where a route
+is, not what calling it does. The operationId is deliberately not changed to
+read better: generated clients are named from it, so moving it would rename
+every caller's method to improve one tool's name. `codefly generate runnables`
 then derives a Runnable package from it, as it does from every method a module
 serves behind the hop; a `GET`'s query parameters are its input. A mutation is
 not marked: it is resolved by an effect receipt, read through a receipt route

operations.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"net/http"
 	"reflect"
+	"regexp"
 	"slices"
 	"strings"
 	"unicode"
@@ -74,6 +75,18 @@ type Operation struct {
 	// route and every relayed route carry it, being a protocol and another
 	// module's call rather than this backend's own operation.
 	Exclude bool
+	// Tool is the name this operation is offered to a model under, when it is
+	// marked as a Runnable operation at all. It is the one part of the marker
+	// the owner says rather than core derives, because it is the one part a
+	// model reads: snake_case, 1..64 characters, starting with a letter.
+	//
+	// Left empty the name falls back to the operationId, which is this
+	// document's own identity for the route and a fine key but a poor word to
+	// put in front of a model — `Wiki_Get_V1_Wiki_Upload-configuration` says
+	// where the route is, not what calling it does. The operationId is not
+	// changed to improve it: generated clients are named from it, so moving it
+	// would rename every caller's method to make one tool read better.
+	Tool string
 	// Exactly one of Handler and RequestHandler answers the route.
 	Handler        Handler
 	RequestHandler RequestHandler
@@ -342,6 +355,7 @@ func swaggerDocument(info InterfaceInfo, ops []Operation, audience string) (map[
 	}
 	paths := map[string]map[string]any{}
 	ids := map[string]string{}
+	tools := map[string]string{}
 	for _, op := range ops {
 		if err := op.validate(); err != nil {
 			return nil, err
@@ -362,6 +376,18 @@ func swaggerDocument(info InterfaceInfo, ops []Operation, audience string) (map[
 			return nil, fmt.Errorf("operations %s and %s %s share the operationId %q", other, strings.ToUpper(method), route, id)
 		}
 		ids[id] = strings.ToUpper(method) + " " + route
+		// A tool name is the identity a MODEL addresses, so it names exactly
+		// one operation, as an operationId names exactly one route. Two reads
+		// claiming one name publish two tools a model cannot tell apart, and
+		// nothing downstream collapses them: a derived package is keyed by the
+		// operationId, and Robin's catalogue dedupes on the operation, the
+		// contract digest and the policy — never on the word.
+		if op.Tool != "" {
+			if other, taken := tools[op.Tool]; taken {
+				return nil, fmt.Errorf("operations %s and %s %s both offer the tool %q", other, strings.ToUpper(method), route, op.Tool)
+			}
+			tools[op.Tool] = strings.ToUpper(method) + " " + route
+		}
 		endpoint := map[string]any{
 			"operationId": id,
 			"summary":     op.Summary,
@@ -376,7 +402,7 @@ func swaggerDocument(info InterfaceInfo, ops []Operation, audience string) (map[
 		if op.Policy != nil {
 			endpoint[PolicyExtension] = op.Policy
 		}
-		marker, err := operationMarker(op, method, route, id, audience)
+		marker, err := operationMarker(op, method, route, cmp.Or(op.Tool, id), audience)
 		if err != nil {
 			return nil, err
 		}
@@ -456,6 +482,17 @@ func operationResponses(op Operation, definitions map[string]any) map[string]any
 	}
 }
 
+// toolName is what a model-facing name may be: lowercase letters, digits and
+// underscores, starting with a letter. It is core's admitted set narrowed to
+// one spelling, so two owners' tools read the same way.
+var toolName = regexp.MustCompile(`^[a-z][a-z0-9]*(_[a-z0-9]+)*$`)
+
+// maxToolName is the longest name a model-facing tool carries, as core bounds
+// one (corerunnable.BoundedName over its own maxToolName).
+const maxToolName = 64
+
+const toolNameSpelling = "lowercase letters and digits in underscore-separated words, starting with a letter"
+
 // validate reports an operation that cannot be rendered because it is
 // undocumented or unserved.
 func (o Operation) validate() error {
@@ -476,6 +513,20 @@ func (o Operation) validate() error {
 		return fmt.Errorf("operation %s declares two handlers", o.Path)
 	case !slices.Contains(servedMethods, o.verb()):
 		return fmt.Errorf("operation %s declares the method %q: an operation is one of %s", o.Path, o.Method, strings.Join(servedMethods, ", "))
+	case o.Tool != "" && (len(o.Tool) > maxToolName || !toolName.MatchString(o.Tool)):
+		// Held here rather than sanitised: core's toolName keeps whatever it
+		// is given, so a name typed with a capital or a hyphen would reach a
+		// model as typed. An owner naming its own tool is told what a name is.
+		return fmt.Errorf("operation %s names the tool %q: a tool name is 1..%d characters of %s", o.Path, o.Tool, maxToolName, toolNameSpelling)
+	case o.Tool != "" && o.Exclude:
+		return fmt.Errorf("operation %s names the tool %q and excludes itself from every agent: one of the two", o.Path, o.Tool)
+	case o.Tool != "" && o.Streaming:
+		return fmt.Errorf("operation %s names the tool %q and streams: a stream is not one finite call, so it is never offered as a tool", o.Path, o.Tool)
+	case o.Tool != "" && !o.Policy.readOnly():
+		// A name on something that can never be marked is a declaration that
+		// does nothing, and it reads as the opposite: the owner believes they
+		// exposed a tool. Only a read is marked, so only a read is named.
+		return fmt.Errorf("operation %s names the tool %q, and only a read is offered as one: its policy requires more than %q, so the operation is resolved by its effect receipt and stays a route its own page calls", o.Path, o.Tool, corerunnable.ReadOnlyScopeAction)
 	}
 	if err := o.Policy.validate(); err != nil {
 		return fmt.Errorf("operation %s %s %w", o.verb(), o.Path, err)

runnable_marker_test.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0"
@@ -39,6 +40,7 @@ func releasesServer() *Server {
 	return New(Manifest{ID: "releases"}).Operations(
 		Operation{Path: "/v1/releases/recorded", Summary: "Read the board as records holds it, without asking GitHub",
 			Callers: "Any member who may read records", Behavior: "The newest recorded pin of every component",
+			Tool:     "read_recorded_board",
 			Response: recordedBoard{}, Policy: reads, Handler: answer},
 		Operation{Path: "/v1/releases/history", Summary: "Read the recorded tags of one repository and the recorded pins of one component",
 			Callers: "Any member who may read records", Behavior: "Newest first",
@@ -225,3 +227,143 @@ func TestAnExcludedReadIsNoOperation(t *testing.T) {
 		t.Fatalf("the same read, not excluded, was not marked (%v): %s", err, doc)
 	}
 }
+
+// TestAnOperationNamesTheToolItIsOfferedAs: the one part of the marker a model
+// actually reads is the one part the owner says. Left empty it falls back to
+// the operationId, which identifies the route and describes nothing, and the
+// operationId itself never moves — generated clients are named from it.
+func TestAnOperationNamesTheToolItIsOfferedAs(t *testing.T) {
+	doc := releasesArtifact(t)
+	var parsed struct {
+		Paths map[string]map[string]struct {
+			OperationID string `json:"operationId"`
+			Marker      *struct {
+				// The marker is proto3 JSON; decoded here as the plain
+				// document a reader sees, not through the proto type, whose
+				// enum does not round-trip encoding/json.
+				Tool struct {
+					Name   string `json:"name"`
+					Effect string `json:"effect"`
+				} `json:"tool"`
+			} `json:"x-codefly-operation"`
+		} `json:"paths"`
+	}
+	if err := json.Unmarshal(doc, &parsed); err != nil {
+		t.Fatal(err)
+	}
+	named := parsed.Paths["/v1/releases/recorded"]["get"]
+	if named.Marker == nil {
+		t.Fatal("the named read is marked as a Runnable operation")
+	}
+	if got := named.Marker.Tool.Name; got != "read_recorded_board" {
+		t.Errorf("the tool is offered as %q, not the name its owner gave it", got)
+	}
+	if named.OperationID != "Releases_Get_V1_Releases_Recorded" {
+		t.Errorf("naming the tool moved the operationId to %q, which renames every generated client", named.OperationID)
+	}
+
+	fallback := parsed.Paths["/v1/releases/history"]["get"]
+	if fallback.Marker == nil {
+		t.Fatal("the unnamed read is still marked")
+	}
+	if got := fallback.Marker.Tool.Name; got != fallback.OperationID {
+		t.Errorf("an operation that names no tool falls back to its operationId, got %q", got)
+	}
+}
+
+// TestAToolNameIsOneSpelling: core's own sanitiser keeps whatever it is given,
+// so a name typed with a capital or a hyphen would reach a model as typed. An
+// owner naming its own tool is told what a name is, at the one place that can
+// still refuse it.
+func TestAToolNameIsOneSpelling(t *testing.T) {
+	for _, name := range []string{"Read_Board", "read-board", "9read", "", "read board", "read_board_"} {
+		op := Operation{
+			Path: "/v1/releases/recorded", Summary: "Read the board", Callers: "Anyone", Behavior: "One object",
+			Tool: name, Response: recordedBoard{}, Policy: Requires("records.records", "read"),
+			Handler: func(context.Context, *Gateway) (any, error) { return nil, nil },
+		}
+		err := op.validate()
+		if name == "" {
+			if err != nil {
+				t.Errorf("an operation that names no tool is valid, got %v", err)
+			}
+			continue
+		}
+		if err == nil {
+			t.Errorf("the tool name %q was accepted; a model-facing name is one spelling", name)
+		}
+	}
+}
+
+// TestOneToolNameNamesOneOperation: a tool name is the identity a model
+// addresses, so it names exactly one operation, as an operationId names
+// exactly one route. Two reads claiming one name would publish two tools a
+// model cannot tell apart, and nothing downstream collapses them — a derived
+// package is keyed by the operationId, and Robin's catalogue dedupes on the
+// operation, the contract digest and the policy, never on the word.
+func TestOneToolNameNamesOneOperation(t *testing.T) {
+	answer := func(context.Context, *Gateway) (any, error) { return nil, nil }
+	reads := Requires("records.records", "read")
+	dir := t.TempDir()
+	if err := os.WriteFile(filepath.Join(dir, "service.codefly.yaml"), []byte("kind: service\nname: backend\nversion: 0.3.0\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	server := New(Manifest{ID: "releases"}).Operations(
+		Operation{Path: "/v1/releases/one", Summary: "One", Callers: "Any member", Behavior: "One object",
+			Tool: "read_board", Response: recordResult{}, Policy: reads, Handler: answer},
+		Operation{Path: "/v1/releases/two", Summary: "Two", Callers: "Any member", Behavior: "One object",
+			Tool: "read_board", Response: recordResult{}, Policy: reads, Handler: answer},
+	)
+	_, err := server.InterfaceArtifact(dir, InterfaceInfo{Title: "releases", OperationPrefix: "Releases", Tag: "releases"})
+	if err == nil {
+		t.Fatal("two operations offered one tool name and the artifact rendered")
+	}
+	if !strings.Contains(err.Error(), "both offer the tool") || !strings.Contains(err.Error(), "read_board") {
+		t.Errorf("the refusal must name the tool and both routes, got %v", err)
+	}
+}
+
+// TestANameOnWhatIsNeverATool: only a read is marked, so only a read is named.
+// A name on anything else is a declaration that does nothing and reads as the
+// opposite — the owner believes they exposed a tool.
+func TestANameOnWhatIsNeverATool(t *testing.T) {
+	answer := func(context.Context, *Gateway) (any, error) { return nil, nil }
+	base := func() Operation {
+		return Operation{Path: "/v1/releases/thing", Summary: "Thing", Callers: "Any member",
+			Behavior: "One object", Tool: "read_thing", Response: recordResult{},
+			Policy: Requires("records.records", "read"), Handler: answer}
+	}
+	for _, c := range []struct {
+		why   string
+		of    func(Operation) Operation
+		names string
+	}{
+		{"a mutation", func(o Operation) Operation {
+			o.Method, o.Policy = "post", Requires("records.records", "read", "write")
+			return o
+		}, "only a read"},
+		{"an excluded route", func(o Operation) Operation { o.Exclude = true; return o }, "excludes itself"},
+		{"a stream", func(o Operation) Operation { o.Streaming = true; return o }, "streams"},
+		{"any checked caller", func(o Operation) Operation { o.Policy = AnyCheckedCaller(); return o }, "only a read"},
+	} {
+		err := c.of(base()).validate()
+		if err == nil {
+			t.Errorf("%s named a tool and was accepted", c.why)
+			continue
+		}
+		if !strings.Contains(err.Error(), c.names) {
+			t.Errorf("%s: the refusal must say why it is never a tool, got %v", c.why, err)
+		}
+	}
+	// The same operation without a name is valid: this refuses the name, not
+	// the operation.
+	for _, of := range []func(Operation) Operation{
+		func(o Operation) Operation { o.Tool, o.Exclude = "", true; return o },
+		func(o Operation) Operation { o.Tool, o.Streaming = "", true; return o },
+		func(o Operation) Operation { o.Tool, o.Policy = "", AnyCheckedCaller(); return o },
+	} {
+		if err := of(base()).validate(); err != nil {
+			t.Errorf("an unnamed operation must stay valid, got %v", err)
+		}
+	}
+}