From 92d2d424df1793c121d0940bc0db6847a900b8d6 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Tue, 27 Oct 2026 18:59:49 -0400 Subject: [PATCH 1/2] fix(run): a composition run's roots leave out what the environment withdraws MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `codefly run workspace` took every service of every composed module as a root and consulted only --exclude-dependency and the run profile, never the environment's exclude-services. The dependency graph is built without a withdrawn service (architecture.ExcludeServices), so a root naming one was a sort target the graph did not hold, and the run refused at ordering: cannot initialize service saas/marketing: runService: flow.InitManagers: cannot order services: cannot topologically sort to : service runtime/admission-witness is not in the graph Seen on obin-ai/platform-obin, whose `local` now withdraws the authority module and the runtime's admission witness exactly as its hosted environments do. CompositionRoots now reads env.ComposedServices — the one list every loop over a module's services for one environment reads — skips a module the environment withdraws whole, and does not seed a withdrawn service-entry. A run profile's and the flag's exclusions still apply on top. Co-Authored-By: Claude Fable 5.1 --- cmd/run/workspace.go | 12 +++++- cmd/run/workspace_roots_test.go | 76 +++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 cmd/run/workspace_roots_test.go diff --git a/cmd/run/workspace.go b/cmd/run/workspace.go index ef7d86b0..bb826ccb 100644 --- a/cmd/run/workspace.go +++ b/cmd/run/workspace.go @@ -65,6 +65,11 @@ var WorkspaceCmd = &cobra.Command{ // The host module is the one exception: its ingress services are the roots, // and the rest of it is reached through their dependencies, as a person would // reach it. +// +// A service the environment withdraws (exclude-services) is no root: the +// dependency graph is built without it, so a root naming it would be a sort +// target the graph does not hold. A module the environment withdraws whole +// contributes nothing, its service-entry included — it is not composed there. func CompositionRoots(ctx context.Context, workspace *resources.Workspace) ([]string, error) { env, err := orchestration.SelectEnvironment(workspace, environmentName) if err != nil { @@ -114,10 +119,13 @@ func CompositionRoots(ctx context.Context, workspace *resources.Workspace) ([]st if err != nil { return nil, fmt.Errorf("%s: %w", ref.Name, err) } - if mod.ServiceEntry != "" { + if env.WithdrawsModule(mod) { + continue + } + if mod.ServiceEntry != "" && !env.Excludes(mod.Name, mod.ServiceEntry) { add(resources.ServiceUnique(mod.Name, mod.ServiceEntry)) } - for _, service := range mod.ServiceReferences { + for _, service := range env.ComposedServices(mod) { add(resources.ServiceUnique(mod.Name, service.Name)) } } diff --git a/cmd/run/workspace_roots_test.go b/cmd/run/workspace_roots_test.go new file mode 100644 index 00000000..1f3f2a24 --- /dev/null +++ b/cmd/run/workspace_roots_test.go @@ -0,0 +1,76 @@ +package run + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/codefly-dev/core/resources" + "github.com/stretchr/testify/require" +) + +// excludingWorkspace lays down two modules and an environment that withdraws +// what exclusions says: shop (api, vault, cache) and auth (hop, local). +func excludingWorkspace(t *testing.T, exclusions string) *resources.Workspace { + t.Helper() + root := t.TempDir() + service := func(name, extra string) string { + return "kind: service\nname: " + name + "\nversion: 0.0.0\nagent:\n kind: runtime::service\n name: go-grpc\n version: 0.0.1\n publisher: codefly.ai\n" + extra + } + files := map[string]string{ + resources.WorkspaceConfigurationName: `name: acme +layout: modules +modules: + - name: shop + - name: auth +environments: + - name: local +` + exclusions, + "modules/shop/module.codefly.yaml": "kind: module\nname: shop\nservice-entry: api\nservices:\n - name: api\n - name: vault\n - name: cache\n", + "modules/shop/services/api/service.codefly.yaml": service("api", "service-dependencies:\n - name: vault\n"), + "modules/shop/services/vault/service.codefly.yaml": service("vault", ""), + "modules/shop/services/cache/service.codefly.yaml": service("cache", ""), + "modules/auth/module.codefly.yaml": "kind: module\nname: auth\nservices:\n - name: hop\n - name: local\n", + "modules/auth/services/hop/service.codefly.yaml": service("hop", ""), + "modules/auth/services/local/service.codefly.yaml": service("local", ""), + } + for rel, content := range files { + full := filepath.Join(root, rel) + require.NoError(t, os.MkdirAll(filepath.Dir(full), 0o755)) + require.NoError(t, os.WriteFile(full, []byte(content), 0o644)) + } + workspace, err := resources.LoadWorkspaceFromDir(context.Background(), root) + require.NoError(t, err) + return workspace +} + +func useEnvironmentName(t *testing.T, value string) { + t.Helper() + previous := environmentName + t.Cleanup(func() { environmentName = previous }) + environmentName = value +} + +// A service the environment withdraws is no root, and a module withdrawn whole +// contributes none: the graph is built without them, so a root naming one would +// be a sort target the graph does not hold ("cannot topologically sort to"). +func TestCompositionRootsLeaveOutWhatTheEnvironmentWithdraws(t *testing.T) { + useEnvironmentName(t, "local") + workspace := excludingWorkspace(t, ` exclude-services: + - service: shop/vault + withdrawn-from: [shop/api] + - service: auth/hop + - service: auth/local +`) + roots, err := CompositionRoots(context.Background(), workspace) + require.NoError(t, err) + require.Equal(t, []string{"shop/api", "shop/cache"}, roots) + + // Without exclusions every service of every module is a root. + useEnvironmentName(t, "local") + all := excludingWorkspace(t, "") + roots, err = CompositionRoots(context.Background(), all) + require.NoError(t, err) + require.Equal(t, []string{"shop/api", "shop/vault", "shop/cache", "auth/hop", "auth/local"}, roots) +} From 447bb76916e705653a8703dca9d174e319876df1 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Tue, 27 Oct 2026 19:26:22 -0400 Subject: [PATCH 2/2] docs(run): `run workspace` roots honour the environment's exclude-services; regression orders every root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The standing rule: a behaviour change updates docs/ in the same PR. `docs/commands.md` now says, under `codefly run workspace`, that the selected environment's `exclude-services` removes services from the composition roots — a module withdrawn whole contributes none, its service-entry included — and that CLI and run-profile exclusions still apply on top. The regression builds an isolated two-module workspace, selects `local` (which withdraws worker/witness) and `staging` (which does not), applies CLI and profile exclusions beside them, calls the real CompositionRoots, builds the real Core dependency graph with the same selected-environment filter the flow uses, and orders to every selected root — the exact shape that failed on main: `cannot topologically sort to : service worker/witness is not in the graph`. Red on the pre-fix CompositionRoots, green on this branch (independent review, codex-cli1204-exclusion-review). Patch authored by the Codex independent reviewer; adopted unchanged after reading it, with `go test ./cmd/run/` and golangci-lint clean. Co-Authored-By: Claude Fable 5.1 --- .../workspace_environment_exclusions_test.go | 98 +++++++++++++++++++ docs/commands.md | 5 +- 2 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 cmd/run/workspace_environment_exclusions_test.go diff --git a/cmd/run/workspace_environment_exclusions_test.go b/cmd/run/workspace_environment_exclusions_test.go new file mode 100644 index 00000000..3e31e7db --- /dev/null +++ b/cmd/run/workspace_environment_exclusions_test.go @@ -0,0 +1,98 @@ +package run + +import ( + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/codefly-dev/cli/pkg/environments" + "github.com/codefly-dev/cli/pkg/orchestration" + "github.com/codefly-dev/core/architecture" + "github.com/codefly-dev/core/resources" + "github.com/stretchr/testify/require" +) + +// A composition root must survive the same selected-environment exclusions +// that orchestration applies when it builds the dependency graph. +func TestCompositionRootsRespectSelectedEnvironmentExclusions(t *testing.T) { + oldEnv, oldProfile, oldExcluded := environmentName, profile, excludeDependencies + t.Cleanup(func() { environmentName, profile, excludeDependencies = oldEnv, oldProfile, oldExcluded }) + profile = "small" + excludeDependencies = []string{"host/admin", "worker/cli-only"} + root := t.TempDir() + workspaceYAML := `name: roots +layout: modules +modules: + - name: host + - name: worker +run-profiles: + small: + exclude-dependencies: [worker/profile-only] +environments: + - name: local + host: + coordinate: example/local/cell + component: host + domain: example + audience: host + trust_domain: cluster.example + delivery: host/web/rest + ingress: + - name: web + service: web + endpoint: rest + hosts: [web.example.test] + - name: admin + service: admin + endpoint: rest + hosts: [admin.example.test] + exclude-services: + - service: worker/witness + - name: staging +` + for module, names := range map[string][]string{ + "host": {"web", "admin"}, + "worker": {"api", "witness", "cli-only", "profile-only"}, + } { + dir := filepath.Join(root, "modules", module) + require.NoError(t, os.MkdirAll(dir, 0o755)) + doc := "kind: module\nname: " + module + "\nservices:\n" + for _, name := range names { + doc += " - name: " + name + "\n" + serviceDir := filepath.Join(dir, "services", name) + require.NoError(t, os.MkdirAll(serviceDir, 0o755)) + service := fmt.Sprintf("kind: service\nname: %s\nmodule: %s\nversion: 0.0.0\nagent:\n kind: runtime::service\n name: go\n version: 0.0.1\n publisher: codefly.ai\nendpoints:\n - name: rest\n api: rest\n visibility: public\n exposure: none\n", name, module) + require.NoError(t, os.WriteFile(filepath.Join(serviceDir, "service.codefly.yaml"), []byte(service), 0o600)) + } + require.NoError(t, os.WriteFile(filepath.Join(dir, "module.codefly.yaml"), []byte(doc), 0o600)) + } + require.NoError(t, os.WriteFile(filepath.Join(root, "workspace.codefly.yaml"), []byte(workspaceYAML), 0o600)) + workspace, err := resources.LoadWorkspaceFromDir(t.Context(), root) + require.NoError(t, err) + for _, tc := range []struct { + env string + want []string + }{ + {"local", []string{"host/web", "worker/api"}}, + {"staging", []string{"host/web", "worker/api", "worker/witness"}}, + } { + t.Run(tc.env, func(t *testing.T) { + environmentName = tc.env + roots, err := CompositionRoots(t.Context(), workspace) + require.NoError(t, err) + env, err := orchestration.SelectEnvironment(workspace, tc.env) + require.NoError(t, err) + require.NoError(t, environments.ValidateEnvironmentExclusions(t.Context(), workspace, env)) + graph, err := architecture.NewServiceDependencies(t.Context(), workspace, architecture.ExcludeServices(env.ExcludedServiceUniques()...)) + require.NoError(t, err) + for _, root := range roots { + _, err = graph.OrderTo(t.Context(), root) + if err != nil { + t.Errorf("selected composition root %s cannot be ordered: %v", root, err) + } + } + require.Equal(t, tc.want, roots) + }) + } +} diff --git a/docs/commands.md b/docs/commands.md index 42dd9b0c..bb4e84d3 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -118,7 +118,10 @@ service` that do not name one root service — `--naming-scope`, `--temporary-ports`, `--output-env`, `--runtime-context`, `--set` among them, never `--stand-alone` or `--exclude-root`; `--exclude-dependency` and `--exclude-workspace-configuration` leave a service or a configuration group -out. The deterministic port hash folds every endpoint into a few thousand +out. The selected environment's `exclude-services` also removes services from +the composition roots; a module withdrawn whole contributes no root, including +its service-entry. CLI and run-profile exclusions still apply on top. +The deterministic port hash folds every endpoint into a few thousand buckets, so a `--naming-scope` can hash two endpoints to one port and abort the run; `--temporary-ports` sidesteps the hash, under the ordering its allocation needs — a workspace configuration reference to a service that has