Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions internal/cli/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,10 @@ func runPerRepoInstall(ctx context.Context, c perRepoInstallConfig) error {
// VendorBinary, UpstreamRef, UpstreamTag. Extra fields are included to stay aligned

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] code-organization

The configLayout computation is duplicated between the dry-run path and real-install path. This follows a pre-existing pattern in the file where both paths construct separate InstallConfig structs.

// 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,
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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) {
Expand Down
69 changes: 69 additions & 0 deletions internal/cli/admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
90 changes: 85 additions & 5 deletions internal/repos/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
155 changes: 155 additions & 0 deletions internal/repos/install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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{
Expand Down
Loading