diff --git a/internal/cli/admin.go b/internal/cli/admin.go index 8047e867f..ef87a3415 100644 --- a/internal/cli/admin.go +++ b/internal/cli/admin.go @@ -862,6 +862,10 @@ func runPerRepoInstall(ctx context.Context, c perRepoInstallConfig) error { // VendorBinary, UpstreamRef, UpstreamTag. Extra fields are included to stay aligned // with the non-dry-run installCfg; Skip* flags are omitted because // they control Install() flow, not scaffold file generation. + dryRunConfigLayout := repos.ConfigLayoutLayered + if alreadyInstalled { + dryRunConfigLayout = repos.ConfigLayoutUpgrade + } dryRunFiles, dryRunErr := repos.BuildScaffoldFiles(repos.InstallConfig{ Owner: owner, Repo: repo, @@ -876,6 +880,7 @@ func runPerRepoInstall(ctx context.Context, c perRepoInstallConfig) error { WIFProvider: inferenceWIFProvider, VendorBinary: vendor, Direct: c.Direct, + ConfigLayout: dryRunConfigLayout, }) if dryRunErr != nil { return fmt.Errorf("generating scaffold files for dry run: %w", dryRunErr) @@ -1084,6 +1089,11 @@ func runPerRepoInstall(ctx context.Context, c perRepoInstallConfig) error { // Resolve review app client ID for provenance validation. reviewAppClientID := resolveReviewAppClientID(ctx, client, c.AppSet) + configLayout := repos.ConfigLayoutLayered + if alreadyInstalled { + configLayout = repos.ConfigLayoutUpgrade + } + installCfg := repos.InstallConfig{ Owner: owner, Repo: repo, @@ -1101,6 +1111,7 @@ func runPerRepoInstall(ctx context.Context, c perRepoInstallConfig) error { VendorBinary: vendor, Direct: c.Direct, SkipScaffoldAndConfig: vendor, + ConfigLayout: configLayout, } progressFn := func(_ string, phase, msg string) { diff --git a/internal/cli/admin_test.go b/internal/cli/admin_test.go index 6af1a00d5..571393695 100644 --- a/internal/cli/admin_test.go +++ b/internal/cli/admin_test.go @@ -2280,6 +2280,75 @@ func TestRunPerRepoInstall_AlreadyInstalledUpgrade(t *testing.T) { require.NoError(t, err) } +// TestRunPerRepoInstall_FreshInstall_LayeredConfig verifies that a fresh +// per-repo install generates both config.base.yaml (scaffold defaults) +// and config.yaml (minimal overlay) per the layered config model. +func TestRunPerRepoInstall_FreshInstall_LayeredConfig(t *testing.T) { + client := forge.NewFakeClient() + client.Repos = []forge.Repository{ + {Name: "widget", FullName: "acme/widget", DefaultBranch: "main"}, + } + + cfg := perRepoTestBase() + cfg.testClient = client + cfg.Direct = true + + err := runPerRepoInstall(context.Background(), cfg) + require.NoError(t, err) + + // Verify scaffold files include both config.base.yaml and config.yaml. + require.NotEmpty(t, client.CommittedFiles, "expected scaffold files to be committed") + paths := make(map[string]string) + for _, f := range client.CommittedFiles[0].Files { + paths[f.Path] = string(f.Content) + } + + assert.Contains(t, paths, ".fullsend/config.base.yaml", + "fresh install should generate config.base.yaml") + assert.Contains(t, paths, ".fullsend/config.yaml", + "fresh install should generate config.yaml overlay") + + // config.base.yaml should contain scaffold defaults (roles). + assert.Contains(t, paths[".fullsend/config.base.yaml"], "roles:", + "config.base.yaml should contain roles") + + // config.yaml should be a minimal overlay (no roles baked in). + assert.NotContains(t, paths[".fullsend/config.yaml"], "roles:", + "config.yaml overlay should not contain roles") +} + +// TestRunPerRepoInstall_ReOnboarding_PreservesUserConfig verifies that +// re-onboarding generates config.base.yaml with new defaults but does +// NOT overwrite config.yaml (the user-owned overlay). +func TestRunPerRepoInstall_ReOnboarding_PreservesUserConfig(t *testing.T) { + client := forge.NewFakeClient() + client.VariableValues = map[string]string{ + "acme/widget/FULLSEND_MINT_URL": "https://mint.example.com/v1/token", + } + client.Repos = []forge.Repository{ + {Name: "widget", FullName: "acme/widget", DefaultBranch: "main"}, + } + + cfg := perRepoTestBase() + cfg.testClient = client + cfg.Direct = true + + err := runPerRepoInstall(context.Background(), cfg) + require.NoError(t, err) + + // Verify scaffold files include config.base.yaml but NOT config.yaml. + require.NotEmpty(t, client.CommittedFiles, "expected scaffold files to be committed") + paths := make(map[string]bool) + for _, f := range client.CommittedFiles[0].Files { + paths[f.Path] = true + } + + assert.True(t, paths[".fullsend/config.base.yaml"], + "re-onboarding should generate config.base.yaml") + assert.False(t, paths[".fullsend/config.yaml"], + "re-onboarding should NOT overwrite config.yaml — it is user-owned") +} + func TestRunPerRepoInstall_DryRun(t *testing.T) { client := forge.NewFakeClient() diff --git a/internal/repos/install.go b/internal/repos/install.go index f94e6380f..2977ee3c0 100644 --- a/internal/repos/install.go +++ b/internal/repos/install.go @@ -24,6 +24,44 @@ var WIFProviderPattern = regexp.MustCompile( `^projects/\d+/locations/global/workloadIdentityPools/[a-z][a-z0-9-]{2,30}[a-z0-9]/providers/[a-z][a-z0-9-]{2,30}[a-z0-9]$`, ) +// ConfigLayout controls how BuildScaffoldFiles generates per-repo config +// files. The default (empty string) preserves backward-compatible behavior. +type ConfigLayout string + +const ( + // ConfigLayoutDefault generates .fullsend/config.yaml with full + // scaffold defaults baked in. This is the backward-compatible default + // used by callers that have not opted into the layered config model + // (e.g. converge/drift detection). + ConfigLayoutDefault ConfigLayout = "" + + // ConfigLayoutLayered generates .fullsend/config.base.yaml with + // scaffold defaults and .fullsend/config.yaml as a minimal user-owned + // overlay. Used for fresh installs via admin install. + ConfigLayoutLayered ConfigLayout = "layered" + + // ConfigLayoutUpgrade generates only .fullsend/config.base.yaml with + // scaffold defaults. config.yaml is user-owned and left untouched. + // Used for re-onboarding via admin install. + ConfigLayoutUpgrade ConfigLayout = "upgrade" +) + +// minimalOverlayYAML is the stub config.yaml overlay committed on fresh +// installs when ConfigLayoutLayered is active. Scaffold defaults live in +// config.base.yaml; this file is user-owned for customization. +const minimalOverlayYAML = `# fullsend per-repo configuration (overlay) +# https://github.com/fullsend-ai/fullsend +# +# This file is the per-repo overlay for fullsend configuration. +# Scaffold defaults are provided by config.base.yaml. +# Values set here override the base layer. Omitted fields inherit +# from config.base.yaml, then from compiled-in code defaults. +# +# See https://fullsend.sh/docs/guides/infrastructure/layered-config-reference + +version: "1" +` + // InstallConfig is a pure data struct holding all inputs needed for a // per-repo installation. CLI flags, environment variables, and interactive // prompts are resolved by the caller before constructing this struct. @@ -83,6 +121,10 @@ type InstallConfig struct { // pipeline YAML so that agent jobs are routed to specific runners. RunnerTags []string + // ConfigLayout controls how BuildScaffoldFiles generates per-repo + // config files. See the ConfigLayout* constants. + ConfigLayout ConfigLayout + // Direct controls scaffold delivery: true pushes directly to the default // branch; false creates a PR. Direct bool @@ -402,11 +444,49 @@ func BuildScaffoldFiles(cfg InstallConfig) ([]forge.TreeFile, error) { Mode: f.Mode, }) } - files = append(files, forge.TreeFile{ - Path: ".fullsend/config.yaml", - Content: cfgYAML, - Mode: "100644", - }) + + // When PerRepoConfig is set, the caller provided an explicit config + // (e.g. migration path). Always write it as config.yaml regardless + // of ConfigLayout. When PerRepoConfig is nil, ConfigLayout controls + // the file strategy. + if cfg.PerRepoConfig != nil { + files = append(files, forge.TreeFile{ + Path: ".fullsend/config.yaml", + Content: cfgYAML, + Mode: "100644", + }) + } else { + switch cfg.ConfigLayout { + case ConfigLayoutLayered: + // Fresh install with layered config: write scaffold defaults + // to config.base.yaml and a minimal overlay to config.yaml. + files = append(files, forge.TreeFile{ + Path: ".fullsend/config.base.yaml", + Content: cfgYAML, + Mode: "100644", + }) + files = append(files, forge.TreeFile{ + Path: ".fullsend/config.yaml", + Content: []byte(minimalOverlayYAML), + Mode: "100644", + }) + case ConfigLayoutUpgrade: + // Re-onboarding: regenerate config.base.yaml with new + // defaults. config.yaml is user-owned and left untouched. + files = append(files, forge.TreeFile{ + Path: ".fullsend/config.base.yaml", + Content: cfgYAML, + Mode: "100644", + }) + default: + // Legacy/default: single config.yaml with full defaults. + files = append(files, forge.TreeFile{ + Path: ".fullsend/config.yaml", + Content: cfgYAML, + Mode: "100644", + }) + } + } return files, nil } diff --git a/internal/repos/install_test.go b/internal/repos/install_test.go index cb05794d6..fce2c52c3 100644 --- a/internal/repos/install_test.go +++ b/internal/repos/install_test.go @@ -7,6 +7,7 @@ import ( "sync" "testing" + "github.com/fullsend-ai/fullsend/internal/config" "github.com/fullsend-ai/fullsend/internal/forge" "github.com/fullsend-ai/fullsend/internal/scaffold" ) @@ -898,6 +899,160 @@ func TestBuildScaffoldFiles_UnsupportedForge(t *testing.T) { } } +// TestBuildScaffoldFiles_LayeredFreshInstall verifies that a fresh install +// with ConfigLayoutLayered generates both config.base.yaml (scaffold +// defaults) and config.yaml (minimal overlay). +func TestBuildScaffoldFiles_LayeredFreshInstall(t *testing.T) { + cfg := baseCfg() + cfg.ConfigLayout = ConfigLayoutLayered + + files, err := BuildScaffoldFiles(cfg) + if err != nil { + t.Fatalf("BuildScaffoldFiles() returned error: %v", err) + } + + paths := make(map[string][]byte) + for _, f := range files { + paths[f.Path] = f.Content + } + + // config.base.yaml must exist with scaffold defaults. + baseContent, hasBase := paths[".fullsend/config.base.yaml"] + if !hasBase { + t.Fatal("expected .fullsend/config.base.yaml in scaffold files") + } + if !strings.Contains(string(baseContent), "roles:") { + t.Error("config.base.yaml should contain roles") + } + + // config.yaml must exist as a minimal overlay. + overlayContent, hasOverlay := paths[".fullsend/config.yaml"] + if !hasOverlay { + t.Fatal("expected .fullsend/config.yaml in scaffold files") + } + if !strings.Contains(string(overlayContent), "version:") { + t.Error("config.yaml overlay should contain version") + } + // The overlay must NOT contain scaffold defaults like roles. + if strings.Contains(string(overlayContent), "roles:") { + t.Error("config.yaml overlay should not contain roles (those belong in config.base.yaml)") + } +} + +// TestBuildScaffoldFiles_UpgradeSkipsConfigYAML verifies that re-onboarding +// with ConfigLayoutUpgrade generates config.base.yaml but does NOT generate +// config.yaml, leaving the user's existing overlay untouched. +func TestBuildScaffoldFiles_UpgradeSkipsConfigYAML(t *testing.T) { + cfg := baseCfg() + cfg.ConfigLayout = ConfigLayoutUpgrade + + files, err := BuildScaffoldFiles(cfg) + if err != nil { + t.Fatalf("BuildScaffoldFiles() returned error: %v", err) + } + + var hasBase, hasConfig bool + for _, f := range files { + switch f.Path { + case ".fullsend/config.base.yaml": + hasBase = true + if !strings.Contains(string(f.Content), "roles:") { + t.Error("config.base.yaml should contain roles") + } + case ".fullsend/config.yaml": + hasConfig = true + } + } + if !hasBase { + t.Fatal("expected .fullsend/config.base.yaml in scaffold files") + } + if hasConfig { + t.Error("config.yaml should NOT be generated on upgrade — it is user-owned") + } +} + +// TestBuildScaffoldFiles_UpgradeRuntime verifies that --runtime on +// re-onboarding writes the runtime to config.base.yaml. +func TestBuildScaffoldFiles_UpgradeRuntime(t *testing.T) { + cfg := baseCfg() + cfg.ConfigLayout = ConfigLayoutUpgrade + cfg.Runtime = "pi" + + files, err := BuildScaffoldFiles(cfg) + if err != nil { + t.Fatalf("BuildScaffoldFiles() returned error: %v", err) + } + + for _, f := range files { + if f.Path == ".fullsend/config.base.yaml" { + if !strings.Contains(string(f.Content), "runtime: pi") { + t.Errorf("config.base.yaml should contain runtime: pi:\n%s", f.Content) + } + return + } + } + t.Fatal("expected .fullsend/config.base.yaml in scaffold files") +} + +// TestBuildScaffoldFiles_DefaultLayoutBackwardCompat verifies that the +// default ConfigLayout (empty string) preserves backward-compatible +// behavior: a single config.yaml with full defaults, no config.base.yaml. +func TestBuildScaffoldFiles_DefaultLayoutBackwardCompat(t *testing.T) { + cfg := baseCfg() + // ConfigLayout is zero value (ConfigLayoutDefault). + + files, err := BuildScaffoldFiles(cfg) + if err != nil { + t.Fatalf("BuildScaffoldFiles() returned error: %v", err) + } + + var hasConfig, hasBase bool + for _, f := range files { + switch f.Path { + case ".fullsend/config.yaml": + hasConfig = true + case ".fullsend/config.base.yaml": + hasBase = true + } + } + if !hasConfig { + t.Error("expected .fullsend/config.yaml with default layout") + } + if hasBase { + t.Error("config.base.yaml should NOT be generated with default layout") + } +} + +// TestBuildScaffoldFiles_PerRepoConfigOverrideIgnoresLayout verifies that +// when PerRepoConfig is set (migration/override path), ConfigLayout is +// ignored and config.yaml is generated from the provided config. +func TestBuildScaffoldFiles_PerRepoConfigOverrideIgnoresLayout(t *testing.T) { + cfg := baseCfg() + cfg.ConfigLayout = ConfigLayoutUpgrade + cfg.PerRepoConfig = config.NewPerRepoConfig([]string{"triage"}, "acme/widgets") + + files, err := BuildScaffoldFiles(cfg) + if err != nil { + t.Fatalf("BuildScaffoldFiles() returned error: %v", err) + } + + var hasConfig, hasBase bool + for _, f := range files { + switch f.Path { + case ".fullsend/config.yaml": + hasConfig = true + case ".fullsend/config.base.yaml": + hasBase = true + } + } + if !hasConfig { + t.Error("expected .fullsend/config.yaml when PerRepoConfig is set") + } + if hasBase { + t.Error("config.base.yaml should NOT be generated when PerRepoConfig is set") + } +} + func TestInstall_FreshInstall_GitLab(t *testing.T) { fc := newFakeClientWithRepo() cfg := InstallConfig{