Merge ecc673e into bac398f

Author: lubien

internal/command/launch/plan_builder.go

@@ -465,9 +465,13 @@ func determineBaseAppConfig(ctx context.Context) (*appconfig.Config, bool, error
 
 		// if --attach is specified, we should return the config as the base config
 		attach := flag.GetBool(ctx, "attach")
-		copyConfig := flag.GetBool(ctx, "copy-config") || attach
+		// An explicit --config flag means the caller deliberately chose the file
+		// (e.g. the deployer passing --config fly.api-server.toml). Treat it as
+		// copy-config so we never prompt and never fall back to source scanning.
+		explicitConfig := flag.IsSpecified(ctx, "config")
+		copyConfig := flag.GetBool(ctx, "copy-config") || attach || explicitConfig
 
-		if !flag.IsSpecified(ctx, "copy-config") && !attach && !flag.GetYes(ctx) {
+		if !flag.IsSpecified(ctx, "copy-config") && !attach && !explicitConfig && !flag.GetYes(ctx) {
 			var err error
 			copyConfig, err = prompt.Confirm(ctx, colorize.Yellow("Would you like to use this fly.toml configuration for this app?"))
 			fmt.Fprintln(io.Out)
@@ -671,6 +675,10 @@ func determineOrg(ctx context.Context, config *appconfig.Config) (*fly.Organizat
 	for _, o := range orgs {
 		bySlug[o.Slug] = o
 	}
+	byRawSlug := make(map[string]fly.Organization, len(orgs))
+	for _, o := range orgs {
+		byRawSlug[o.RawSlug] = o
+	}
 	byName := make(map[string]fly.Organization, len(orgs))
 	for _, o := range orgs {
 		byName[o.Name] = o
@@ -699,6 +707,13 @@ func determineOrg(ctx context.Context, config *appconfig.Config) (*fly.Organizat
 			return &org, "specified on the command line", nil
 		}
 
+		// The personal org's canonical Slug is always "personal", but callers
+		// (e.g. the deployer) may supply the real/raw slug (e.g. "lubien-339").
+		// Fall back to a RawSlug lookup before giving up.
+		if org, foundRawSlug := byRawSlug[orgRequested]; foundRawSlug {
+			return &org, "specified on the command line", nil
+		}
+
 		if !foundPersonal {
 			return nil, "", errors.New("no personal organization found")
 		}

internal/command/launch/plan_builder_test.go

@@ -0,0 +1,193 @@
+package launch
+
+import (
+	"context"
+	"testing"
+
+	"github.com/spf13/pflag"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	fly "github.com/superfly/fly-go"
+	"github.com/superfly/flyctl/internal/appconfig"
+	"github.com/superfly/flyctl/internal/flag/flagctx"
+	"github.com/superfly/flyctl/internal/flyutil"
+	"github.com/superfly/flyctl/internal/mock"
+	"github.com/superfly/flyctl/iostreams"
+)
+
+func newDetermineOrgCtx(t *testing.T, orgFlag string) context.Context {
+	t.Helper()
+
+	ctx := context.Background()
+
+	flagSet := pflag.NewFlagSet("test", pflag.ContinueOnError)
+	flagSet.String("org", "", "")
+	flagSet.Bool("attach", false, "")
+
+	if orgFlag != "" {
+		require.NoError(t, flagSet.Set("org", orgFlag))
+	}
+
+	return flagctx.NewContext(ctx, flagSet)
+}
+
+func TestDetermineOrg(t *testing.T) {
+	personalOrg := fly.Organization{
+		ID:      "org-id-personal",
+		Slug:    "personal",
+		RawSlug: "logan-griswold-339",
+		Name:    "Logan Griswold",
+		Type:    "PERSONAL",
+	}
+	teamOrg := fly.Organization{
+		ID:      "org-id-team",
+		Slug:    "my-team",
+		RawSlug: "my-team",
+		Name:    "My Team",
+		Type:    "SHARED",
+	}
+	orgs := []fly.Organization{personalOrg, teamOrg}
+
+	makeClient := func() *mock.Client {
+		return &mock.Client{
+			GetOrganizationsFunc: func(ctx context.Context, filters ...fly.OrganizationFilter) ([]fly.Organization, error) {
+				return orgs, nil
+			},
+		}
+	}
+
+	t.Run("no org flag defaults to personal", func(t *testing.T) {
+		ctx := newDetermineOrgCtx(t, "")
+		ctx = flyutil.NewContextWithClient(ctx, makeClient())
+
+		org, _, err := determineOrg(ctx, nil)
+		require.NoError(t, err)
+		assert.Equal(t, "personal", org.Slug)
+		assert.Equal(t, "logan-griswold-339", org.RawSlug)
+	})
+
+	t.Run("canonical personal slug", func(t *testing.T) {
+		ctx := newDetermineOrgCtx(t, "personal")
+		ctx = flyutil.NewContextWithClient(ctx, makeClient())
+
+		org, _, err := determineOrg(ctx, nil)
+		require.NoError(t, err)
+		assert.Equal(t, "personal", org.Slug)
+		assert.Equal(t, "logan-griswold-339", org.RawSlug)
+	})
+
+	t.Run("real raw slug of personal org", func(t *testing.T) {
+		ctx := newDetermineOrgCtx(t, "logan-griswold-339")
+		ctx = flyutil.NewContextWithClient(ctx, makeClient())
+
+		org, _, err := determineOrg(ctx, nil)
+		require.NoError(t, err)
+		assert.Equal(t, "personal", org.Slug)
+		assert.Equal(t, "logan-griswold-339", org.RawSlug)
+	})
+
+	t.Run("team org by slug", func(t *testing.T) {
+		ctx := newDetermineOrgCtx(t, "my-team")
+		ctx = flyutil.NewContextWithClient(ctx, makeClient())
+
+		org, _, err := determineOrg(ctx, nil)
+		require.NoError(t, err)
+		assert.Equal(t, "my-team", org.Slug)
+	})
+
+	t.Run("org by display name", func(t *testing.T) {
+		ctx := newDetermineOrgCtx(t, "My Team")
+		ctx = flyutil.NewContextWithClient(ctx, makeClient())
+
+		org, _, err := determineOrg(ctx, nil)
+		require.NoError(t, err)
+		assert.Equal(t, "my-team", org.Slug)
+	})
+
+	t.Run("unknown org returns error and falls back to personal", func(t *testing.T) {
+		ctx := newDetermineOrgCtx(t, "does-not-exist")
+		ctx = flyutil.NewContextWithClient(ctx, makeClient())
+
+		org, _, err := determineOrg(ctx, nil)
+		assert.Error(t, err)
+		require.NotNil(t, org)
+		assert.Equal(t, "personal", org.Slug)
+	})
+}
+
+// newDetermineBaseAppConfigCtx builds a context wired with the flags that
+// determineBaseAppConfig reads. Pass configPath="" to leave --config unset.
+func newDetermineBaseAppConfigCtx(t *testing.T, copyConfigFlag, explicitConfigPath bool) context.Context {
+	t.Helper()
+
+	ctx := context.Background()
+	ctx = iostreams.NewContext(ctx, iostreams.System())
+
+	flagSet := pflag.NewFlagSet("test", pflag.ContinueOnError)
+	flagSet.String("config", "", "")
+	flagSet.Bool("copy-config", false, "")
+	flagSet.Bool("attach", false, "")
+	flagSet.Bool("yes", false, "")
+
+	if copyConfigFlag {
+		require.NoError(t, flagSet.Set("copy-config", "true"))
+	}
+	if explicitConfigPath {
+		require.NoError(t, flagSet.Set("config", "fly.custom.toml"))
+	}
+
+	return flagctx.NewContext(ctx, flagSet)
+}
+
+func TestDetermineBaseAppConfig(t *testing.T) {
+	// existingCfg simulates what LoadAppConfigIfPresent puts in context when
+	// the customer has a custom fly.toml with a non-default dockerfile.
+	existingCfg := appconfig.NewConfig()
+	existingCfg.Build = &appconfig.Build{
+		Dockerfile: "docker.ui-server.dockerfile",
+	}
+
+	t.Run("no flags and no existing config returns blank config", func(t *testing.T) {
+		ctx := newDetermineBaseAppConfigCtx(t, false, false)
+		// No config in context — simulates no fly.toml present.
+
+		cfg, copied, err := determineBaseAppConfig(ctx)
+		require.NoError(t, err)
+		assert.False(t, copied)
+		assert.Nil(t, cfg.Build)
+	})
+
+	t.Run("--copy-config adopts existing config without prompting", func(t *testing.T) {
+		ctx := newDetermineBaseAppConfigCtx(t, true, false)
+		ctx = appconfig.WithConfig(ctx, existingCfg)
+
+		cfg, copied, err := determineBaseAppConfig(ctx)
+		require.NoError(t, err)
+		assert.True(t, copied)
+		assert.Equal(t, "docker.ui-server.dockerfile", cfg.Build.Dockerfile)
+	})
+
+	t.Run("explicit --config adopts existing config without prompting", func(t *testing.T) {
+		// This is the deployer scenario: --config fly.custom.toml is passed but
+		// --copy-config is not. The explicit path signals intent, so we must
+		// not fall through to source scanning with an empty config.
+		ctx := newDetermineBaseAppConfigCtx(t, false, true)
+		ctx = appconfig.WithConfig(ctx, existingCfg)
+
+		cfg, copied, err := determineBaseAppConfig(ctx)
+		require.NoError(t, err)
+		assert.True(t, copied)
+		assert.Equal(t, "docker.ui-server.dockerfile", cfg.Build.Dockerfile)
+	})
+
+	t.Run("no flags in non-interactive mode returns error", func(t *testing.T) {
+		ctx := newDetermineBaseAppConfigCtx(t, false, false)
+		ctx = appconfig.WithConfig(ctx, existingCfg)
+		// Non-interactive iostreams → prompt.Confirm returns ErrNonInteractive.
+		ios, _, _, _ := iostreams.Test()
+		ctx = iostreams.NewContext(ctx, ios)
+
+		_, _, err := determineBaseAppConfig(ctx)
+		assert.Error(t, err)
+	})
+}

internal/command/launch/plan_commands.go

@@ -38,6 +38,7 @@ func newPropose() *cobra.Command {
 	flag.Add(cmd,
 		flag.Region(),
 		flag.Org(),
+		flag.AppConfig(),
 		flag.String{
 			Name:        "from",
 			Description: "A github repo URL to use as a template for the new app",
@@ -183,6 +184,12 @@ func newGenerate() *cobra.Command {
 		flag.Region(),
 		flag.Org(),
 		flag.AppConfig(),
+		flag.Yes(),
+		flag.Bool{
+			Name:        "copy-config",
+			Description: "Use the configuration file if present without prompting",
+			Default:     false,
+		},
 		flag.Bool{
 			Name:        "no-deploy",
 			Description: "Don't deploy the app",
@@ -222,7 +229,18 @@ func runPropose(ctx context.Context) error {
 		ctx = logger.NewContext(ctx, logger.New(os.Stderr, logger.FromContext(ctx).Level(), iostreams.IsTerminalWriter(os.Stdout)))
 	}
 
-	err := RunPlan(ctx, "propose")
+	// LoadAppConfigIfPresent is registered on the parent "plan" command, but
+	// because that command has no Run func cobra never executes its RunE and the
+	// preparer is never called. Load the config here explicitly so that a custom
+	// path supplied via --config (e.g. fly.api-server.toml) is in context before
+	// buildManifest → determineBaseAppConfig reads it.
+	var err error
+	ctx, err = command.LoadAppConfigIfPresent(ctx)
+	if err != nil {
+		return err
+	}
+
+	err = RunPlan(ctx, "propose")
 	if err != nil {
 		return err
 	}
@@ -257,5 +275,16 @@ func runTigris(ctx context.Context) error {
 func runGenerate(ctx context.Context) error {
 	flag.SetString(ctx, "from-manifest", flag.FirstArg(ctx))
 
+	// LoadAppConfigIfPresent is registered on the parent "plan" command, but
+	// because that command has no Run func cobra never executes its RunE and the
+	// preparer is never called. Load the config here explicitly so that a custom
+	// path supplied via --config (e.g. fly.api-server.toml) is in context before
+	// buildManifest → determineBaseAppConfig reads it.
+	var err error
+	ctx, err = command.LoadAppConfigIfPresent(ctx)
+	if err != nil {
+		return err
+	}
+
 	return RunPlan(ctx, "generate")
 }