fix(generate): scope a contract endpoint's package to its declared surface (#991)

`codefly generate contracts` refuses a service whose owned proto tree carries
both its served API and a client facade for another service's API:

    proto declares services in multiple packages
    [saas.accounts.v1 saas.policylog.v1]; a contract endpoint must be
    single-package

The contract is in fact single-package. The facade is there because the service
CALLS that API and needs generated stubs, so the proto has to sit in the owned
tree for codegen to reach it — `--path saas`, with vendored imports excluded as
import-only. But the service serves none of it, and serviceContractPackage
derived the package purely by walking the proto tree, so a package the endpoint
does not serve made a well-formed endpoint look multi-package and the export
failed.

The answer was already on disk and unread. cli#947 gave a module the
CLI-owned contracts.codefly.yaml, whose `surfaces` name a per-endpoint
inventory of the services each endpoint actually serves, and
writeBuiltProtobufContract already receives that endpoint's surface in order to
fill the catalog's `services[]`. It just never reached the package derivation.
It does now: when a surface is declared, candidate packages are scoped to the
ones its services' fully-qualified names carry.

Deliberately unchanged in two directions, because a fix to a refusal must not
become a way past it:

- A module that declares NO surface narrows nothing. The single-package rule
  stands exactly as before, which is what keeps the default honest for a
  service that has no reason to carry a foreign package.
- A surface that names no package the descriptor set declares a service in
  narrows nothing either. That is a configuration error, and the existing
  unscoped diagnostics describe it far better than a silently empty contract
  would.

The copied proto sources are untouched: the descriptor set spans the whole owned
tree and the sources copy with it, as it already does for message-only packages.
Narrowing what gets copied is a separate question from which package the
endpoint's contract reports.

Found from module-saas-starter#1067, where accounts consumes a policy-log
witness (`policylogv1.WitnessServiceClient`) and `generate contracts --check`
refused the tree. Verified against that exact tree with a locally built binary:
before, the multi-package refusal; after, it proceeds and reports ordinary
drift — `run codefly generate contracts to update` — which is what --check
should say for a tree whose contracts have not been regenerated yet.

Tests: three cases in TestServiceContractPackage — a facade in a foreign package
scoped away by its surface (and still refused without one, so the rule is scoped
by a declaration rather than removed), a surface naming an absent package
falling back to the unscoped diagnostics, and a surface whose service name
carries no package qualifier narrowing nothing. Mutation-verified: emptying the
scoping set reds the first. `go test ./cmd/generate/` passes, `go build ./...`
and `go vet` clean. docs/commands.md records the behaviour in the
contracts.codefly.yaml section.

Co-authored-by: Codefly Automation <312536060+antoinetoussaint-byte@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: antoinetoussaint-byte

cmd/generate/contracts.go

@@ -10,6 +10,7 @@ import (
 	"path"
 	"path/filepath"
 	"sort"
+	"strings"
 	"time"
 
 	"github.com/codefly-dev/cli/cmd/common"
@@ -382,7 +383,7 @@ func writeBuiltProtobufContract(ctx context.Context, module *resources.Module, s
 		return nil, fmt.Errorf("cannot parse generated descriptor set: %w", err)
 	}
 
-	pkg, err := serviceContractPackage(&set, protoDir)
+	pkg, err := serviceContractPackage(&set, protoDir, surface)
 	if err != nil {
 		return nil, fmt.Errorf("service %s endpoint %s: %w", service.Name, endpoint.Name, err)
 	}
@@ -422,7 +423,18 @@ func writeBuiltProtobufContract(ctx context.Context, module *resources.Module, s
 // all of its RPC services in one package; message-only packages contributed
 // alongside it (composed settings, shared types) carry no service to expose in
 // a client contract and are ignored.
-func serviceContractPackage(set *descriptorpb.FileDescriptorSet, protoDir string) (string, error) {
+//
+// `surface` is the endpoint's declared served surface, when the module declares
+// one through contracts.codefly.yaml. It names which services this endpoint
+// serves, so it also settles which proto package the contract covers, and the
+// candidates are scoped to it. That matters for a service that must generate
+// client stubs from ANOTHER service's API: those protos declare services in a
+// foreign package and have to live in the owned tree for codegen to reach them,
+// yet the endpoint does not serve them, so without this they make a
+// single-package endpoint look multi-package and the export fails on a
+// contract that is in fact well formed. A module that declares no surface is
+// unchanged: nothing narrows the candidates and the single-package rule stands.
+func serviceContractPackage(set *descriptorpb.FileDescriptorSet, protoDir string, surface []composition.APIContractService) (string, error) {
 	own := map[string]struct{}{}
 	walkErr := filepath.WalkDir(protoDir, func(p string, entry os.DirEntry, err error) error {
 		if err != nil {
@@ -462,6 +474,23 @@ func serviceContractPackage(set *descriptorpb.FileDescriptorSet, protoDir string
 	}
 	sort.Strings(packages)
 
+	// Scope to the declared surface. Dropping to zero means the surface names
+	// no package the descriptor set declares a service in, which is a
+	// configuration error the unscoped diagnostics below describe better than a
+	// silent empty contract, so the narrowing is only applied when it keeps at
+	// least one candidate.
+	if served := surfacePackages(surface); len(served) > 0 {
+		scoped := make([]string, 0, len(packages))
+		for _, pkg := range packages {
+			if _, ok := served[pkg]; ok {
+				scoped = append(scoped, pkg)
+			}
+		}
+		if len(scoped) > 0 {
+			packages = scoped
+		}
+	}
+
 	switch len(packages) {
 	case 0:
 		// buf always includes the service's own files in the descriptor set,
@@ -481,6 +510,21 @@ func serviceContractPackage(set *descriptorpb.FileDescriptorSet, protoDir string
 	}
 }
 
+// surfacePackages is the set of proto packages a declared surface names, taken
+// from each service's fully-qualified name. A name carrying no package
+// qualifier contributes nothing rather than an empty-string package.
+func surfacePackages(surface []composition.APIContractService) map[string]struct{} {
+	packages := make(map[string]struct{}, len(surface))
+	for _, service := range surface {
+		dot := strings.LastIndex(service.FullName, ".")
+		if dot <= 0 {
+			continue
+		}
+		packages[service.FullName[:dot]] = struct{}{}
+	}
+	return packages
+}
+
 func writeRestContract(ctx context.Context, module *resources.Module, endpoint *basev0.Endpoint, rest *basev0.RestAPI, physicalDir, canonicalDir string) (*composition.APIContractEndpoint, error) {
 	if len(rest.Openapi) == 0 {
 		return nil, fmt.Errorf("endpoint %s has no OpenAPI document", endpoint.Name)

cmd/generate/contracts_test.go

@@ -543,7 +543,14 @@ func TestServiceContractPackage(t *testing.T) {
 	// protoDir holds the service's own files; imports buf pulls in live
 	// elsewhere and must be ignored even when they declare services.
 	protoDir := t.TempDir()
-	for _, rel := range []string{"saas/acct/v1/api.proto", "saas/shared/v1/types.proto"} {
+	// witness.proto stands for a client facade the service generates stubs from
+	// but does not serve. A subtest only sees it when its own descriptor set
+	// names it, so its presence here changes nothing for the others.
+	for _, rel := range []string{
+		"saas/acct/v1/api.proto",
+		"saas/shared/v1/types.proto",
+		"saas/policylog/v1/witness.proto",
+	} {
 		full := filepath.Join(protoDir, filepath.FromSlash(rel))
 		if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil {
 			t.Fatal(err)
@@ -568,7 +575,7 @@ func TestServiceContractPackage(t *testing.T) {
 			// an import: has a service but is not one of the service's own files.
 			file("google/protobuf/descriptor.proto", "google.protobuf", true),
 		}}
-		pkg, err := serviceContractPackage(set, protoDir)
+		pkg, err := serviceContractPackage(set, protoDir, nil)
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
 		}
@@ -582,7 +589,7 @@ func TestServiceContractPackage(t *testing.T) {
 			file("saas/acct/v1/api.proto", "saas.acct.v1", false),
 			file("saas/shared/v1/types.proto", "saas.shared.v1", false),
 		}}
-		if _, err := serviceContractPackage(set, protoDir); err == nil {
+		if _, err := serviceContractPackage(set, protoDir, nil); err == nil {
 			t.Fatal("expected an error when no own file declares a service")
 		}
 	})
@@ -592,11 +599,67 @@ func TestServiceContractPackage(t *testing.T) {
 			file("saas/acct/v1/api.proto", "saas.acct.v1", true),
 			file("saas/shared/v1/types.proto", "saas.shared.v1", true),
 		}}
-		if _, err := serviceContractPackage(set, protoDir); err == nil {
+		if _, err := serviceContractPackage(set, protoDir, nil); err == nil {
 			t.Fatal("expected an error when services span multiple packages")
 		}
 	})
 
+	t.Run("a declared surface scopes the package, so a client facade in a foreign package is not a second contract package", func(t *testing.T) {
+		// The shape that sent this to a CLI issue: a service whose own tree
+		// carries BOTH its served API and a client facade for another
+		// service's API. The facade has to sit in the owned tree for codegen to
+		// reach it, and it declares a service in its own package, so the
+		// unscoped rule reports a multi-package endpoint for a contract that is
+		// in fact single-package. The declared surface already says which
+		// services the endpoint serves.
+		set := &descriptorpb.FileDescriptorSet{File: []*descriptorpb.FileDescriptorProto{
+			file("saas/acct/v1/api.proto", "saas.acct.v1", true),
+			file("saas/policylog/v1/witness.proto", "saas.policylog.v1", true),
+		}}
+		surface := []composition.APIContractService{
+			{Name: "AcctService", FullName: "saas.acct.v1.AcctService"},
+		}
+		pkg, err := serviceContractPackage(set, protoDir, surface)
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if pkg != "saas.acct.v1" {
+			t.Fatalf("pkg = %q, want saas.acct.v1", pkg)
+		}
+		// Without the surface the same set is still refused, so the rule is
+		// scoped by a declaration rather than removed.
+		if _, err = serviceContractPackage(set, protoDir, nil); err == nil {
+			t.Fatal("expected the unscoped call to still refuse two service packages")
+		}
+	})
+
+	t.Run("a surface naming no declared package falls back to the unscoped diagnostics", func(t *testing.T) {
+		set := &descriptorpb.FileDescriptorSet{File: []*descriptorpb.FileDescriptorProto{
+			file("saas/acct/v1/api.proto", "saas.acct.v1", true),
+			file("saas/shared/v1/types.proto", "saas.shared.v1", true),
+		}}
+		surface := []composition.APIContractService{
+			{Name: "Absent", FullName: "saas.absent.v1.Absent"},
+		}
+		if _, err := serviceContractPackage(set, protoDir, surface); err == nil {
+			t.Fatal("expected the multi-package refusal rather than a silent empty contract")
+		}
+	})
+
+	t.Run("a surface whose service name carries no package qualifier narrows nothing", func(t *testing.T) {
+		set := &descriptorpb.FileDescriptorSet{File: []*descriptorpb.FileDescriptorProto{
+			file("saas/acct/v1/api.proto", "saas.acct.v1", true),
+		}}
+		surface := []composition.APIContractService{{Name: "Acct", FullName: "Acct"}}
+		pkg, err := serviceContractPackage(set, protoDir, surface)
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if pkg != "saas.acct.v1" {
+			t.Fatalf("pkg = %q, want saas.acct.v1", pkg)
+		}
+	})
+
 	t.Run("descriptor names not matching own paths reports a layout remap, not 'no services'", func(t *testing.T) {
 		// buf stripped a "saas/" prefix (a module path or roots entry), so no
 		// descriptor file name equals its path under protoDir even though the
@@ -605,7 +668,7 @@ func TestServiceContractPackage(t *testing.T) {
 			file("acct/v1/api.proto", "saas.acct.v1", true),
 			file("shared/v1/types.proto", "saas.shared.v1", false),
 		}}
-		_, err := serviceContractPackage(set, protoDir)
+		_, err := serviceContractPackage(set, protoDir, nil)
 		if err == nil {
 			t.Fatal("expected an error when no descriptor file matches an own path")
 		}

docs/commands.md

@@ -2772,6 +2772,18 @@ supply the inventory, since copying a full descriptor to each cannot prove which
 listener serves a method. A single endpoint keeps the full-descriptor default;
 restricted single listeners must also supply their inventory.
 
+A declared inventory also settles which proto **package** each endpoint's
+contract covers, which matters for a service that generates client stubs from
+another service's API. Those protos declare services in a foreign package and
+must sit in the owned tree for codegen to reach them, yet the endpoint does not
+serve them. Without an inventory such a tree is refused — `proto declares
+services in multiple packages …; a contract endpoint must be single-package` —
+for a contract that is in fact single-package. With one, candidate packages are
+scoped to those the inventory names. A service that declares no inventory is
+unaffected, and so is one whose inventory names no package the descriptor set
+declares a service in: the single-package rule and its diagnostics stand
+unchanged, rather than yielding an empty contract.
+
 This input is a stopgap for the released Core/agent endpoint report. In Core
 v0.18.0, `GrpcAPI.rpcs` is loaded from the service's shared proto, without
 listener filtering, while Connect is reported as `HttpAPI` with no RPC list.