From 846ed1eba38c1235a6f701b9f3f471ae717954c8 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Tue, 27 Oct 2026 20:01:53 -0400 Subject: [PATCH] fix(resources): a run profile is validated for shape at load; its references at resolve MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Workspace.postLoad validated every run profile that carries an exclusion by loading every service, which a composition of pinned modules cannot do: those modules are materialized by the CLI after this load, so declaring a profile made the workspace unloadable for every command, doctor included ("module resolves to a pinned artifact … not loadable as a local checkout"). ResolveRunProfile already runs the same inventory check at run time, once the composition is loadable; the eager copy was the one that could not work. Load now checks shape only: a non-empty profile name, every excluded dependency a non-empty, well-formed service reference, every excluded workspace configuration non-empty. An unknown service or configuration is reported by ResolveRunProfile, where the inventory exists. The at-load tests for unknown references move to resolve time; a new test loads a pinned composition with a profile, and fails before this change. Co-Authored-By: Claude Fable 5.1 --- resources/run_profile.go | 39 ++++++++++++--------- resources/run_profile_test.go | 66 +++++++++++++++++++++++++++++++---- 2 files changed, 83 insertions(+), 22 deletions(-) diff --git a/resources/run_profile.go b/resources/run_profile.go index d829f691a..9c7401953 100644 --- a/resources/run_profile.go +++ b/resources/run_profile.go @@ -52,30 +52,37 @@ func (workspace *Workspace) ResolveRunProfile(ctx context.Context, name string, return inventory.resolve(combined) } -func (workspace *Workspace) validateRunProfiles(ctx context.Context) error { +// validateRunProfiles checks each declared profile's SHAPE at load: a non-empty +// name, and every exclusion a non-empty, well-formed reference. It deliberately +// does not check that the references exist. That needs the service inventory, +// and a composition of pinned modules has none until the CLI has materialized +// them — which happens after this load, so an inventory check here refused every +// such workspace at load, `doctor` included. ResolveRunProfile checks the +// references at run time, once the composition is loadable, and is where an +// unknown service or workspace configuration is reported. +func (workspace *Workspace) validateRunProfiles(_ context.Context) error { names := make([]string, 0, len(workspace.RunProfiles)) - needsInventory := false - for name, profile := range workspace.RunProfiles { + for name := range workspace.RunProfiles { if strings.TrimSpace(name) == "" { return fmt.Errorf("run profile name cannot be empty") } names = append(names, name) - needsInventory = needsInventory || - len(profile.ExcludeDependencies) > 0 || - len(profile.ExcludeWorkspaceConfigurations) > 0 } - if !needsInventory { - return nil - } - slices.Sort(names) - inventory, err := workspace.loadRunProfileInventory(ctx) - if err != nil { - return err - } for _, name := range names { - if _, err := inventory.resolve(workspace.RunProfiles[name]); err != nil { - return fmt.Errorf("run profile %q: %w", name, err) + profile := workspace.RunProfiles[name] + for _, dependency := range profile.ExcludeDependencies { + if strings.TrimSpace(dependency) == "" { + return fmt.Errorf("run profile %q: an excluded dependency cannot be empty", name) + } + if _, err := ParseServiceWithOptionalModule(strings.TrimSpace(dependency)); err != nil { + return fmt.Errorf("run profile %q: excluded dependency %q: %w", name, dependency, err) + } + } + for _, configuration := range profile.ExcludeWorkspaceConfigurations { + if strings.TrimSpace(configuration) == "" { + return fmt.Errorf("run profile %q: an excluded workspace configuration cannot be empty", name) + } } } return nil diff --git a/resources/run_profile_test.go b/resources/run_profile_test.go index 493e86ae3..aff3a3c46 100644 --- a/resources/run_profile_test.go +++ b/resources/run_profile_test.go @@ -78,16 +78,16 @@ func TestWorkspaceRunProfileValidation(t *testing.T) { want: "run profile name cannot be empty", }, { - name: "unknown service", + name: "malformed excluded dependency", old: "users/accounts", - replacement: "users/missing", - want: "unknown service", + replacement: "users/accounts/extra", + want: "excluded dependency", }, { - name: "unknown workspace configuration", + name: "empty excluded workspace configuration", old: "internal-auth", - replacement: "missing-auth", - want: "unknown workspace configuration", + replacement: "\"\"", + want: "excluded workspace configuration cannot be empty", }, } @@ -165,3 +165,57 @@ func assertRunComposition( require.NoError(t, err) require.ElementsMatch(t, want, order) } + +// TestRunProfileReferencesAreCheckedAtResolveNotLoad: a profile naming a service +// or workspace configuration the composition does not have still LOADS — the +// inventory that would tell is not available at load for a composition of +// pinned modules — and is refused where the inventory exists, at ResolveRunProfile. +func TestRunProfileReferencesAreCheckedAtResolveNotLoad(t *testing.T) { + fixture := "testdata/workspaces/run-profiles" + content, err := os.ReadFile(filepath.Join(fixture, resources.WorkspaceConfigurationName)) + require.NoError(t, err) + + tests := []struct { + name string + old string + replacement string + want string + }{ + {name: "unknown service", old: "users/accounts", replacement: "users/missing", want: "unknown service"}, + {name: "unknown workspace configuration", old: "internal-auth", replacement: "missing-auth", want: "unknown workspace configuration"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + require.NoError(t, os.CopyFS(root, os.DirFS(fixture))) + invalid := strings.Replace(string(content), tt.old, tt.replacement, 1) + require.NoError(t, os.WriteFile(filepath.Join(root, resources.WorkspaceConfigurationName), []byte(invalid), 0o600)) + + workspace, err := resources.LoadWorkspaceFromDir(ctx, root) + require.NoError(t, err, "a reference the inventory cannot confirm is not a load failure") + + _, err = workspace.ResolveRunProfile(ctx, "local", resources.RunProfile{}) + require.ErrorContains(t, err, tt.want) + }) + } +} + +// TestRunProfileDoesNotMakeAPinnedCompositionUnloadable: a workspace that +// composes a pinned module cannot load that module's services until the CLI has +// materialized it. Declaring a run profile with exclusions must not turn that +// into "cannot load workspace" — before this, every command refused at load, +// doctor included. +func TestRunProfileDoesNotMakeAPinnedCompositionUnloadable(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + writeWorkspace(t, dir, "name: solution\nlayout: modules\nmodules:\n - name: wiki\n source: acme/core-solutions\n version: \"0a18964f\"\nrun-profiles:\n local:\n exclude-dependencies: [documents/docx]\n exclude-workspace-configurations: [document-formats]\n") + + workspace, err := resources.LoadWorkspaceFromDir(ctx, dir) + require.NoError(t, err) + require.Contains(t, workspace.RunProfiles, "local") + + resolution, err := workspace.ResolveModule(ctx, workspace.Modules[0]) + require.NoError(t, err) + require.Equal(t, resources.ResolutionPinned, resolution.Kind) +}