From dfac74f30a23aa42594ce5382c9422b354211d87 Mon Sep 17 00:00:00 2001 From: Christina Chu Date: Tue, 20 Oct 2026 12:13:11 -0700 Subject: [PATCH 1/5] feat: plan the AWS setup steps PlanSetup reads AWS and returns a SetupPlan. It writes nothing. The plan has one step each for the IAM roles, the agent space, the account association, the operator app, and the MCP server, and a line that describes each step. A role that does not exist gets created. A role that setup made gets updated. A role that setup did not make stays as it is. The plan creates a space, reuses it, or only connects to it. If two spaces have the name, the plan stops and asks for --agent-space-id. With --new-agent-space, the plan does not create a space with a name that a space in the region uses. If the operator app is off, the plan enables it. If it is on, the plan keeps its sign-in, unless AuthFlowSet is true. The plan reuses the MCP server, registers it, replaces its token, or asks for a token. Nothing calls PlanSetup yet. The next PR runs the plan. Co-Authored-By: Claude Opus 5.5 --- internal/awsdevops/awsdevops_test.go | 8 +- internal/awsdevops/lookup.go | 16 ++ internal/awsdevops/plan_test.go | 285 ++++++++++++++++++++ internal/awsdevops/setup.go | 372 ++++++++++++++++++++++++++- 4 files changed, 679 insertions(+), 2 deletions(-) create mode 100644 internal/awsdevops/plan_test.go diff --git a/internal/awsdevops/awsdevops_test.go b/internal/awsdevops/awsdevops_test.go index ff7193494..a87562ef6 100644 --- a/internal/awsdevops/awsdevops_test.go +++ b/internal/awsdevops/awsdevops_test.go @@ -32,6 +32,8 @@ type fakeAgent struct { tagsErr error // tagsErrFor fails the tag lookup of one resource ARN. tagsErrFor map[string]error + // spaceLinks holds associations by space ID. + spaceLinks map[string][]agenttypes.Association // operatorApps holds enabled apps by space. operatorApps map[string]*devopsagent.GetOperatorAppOutput // operatorAppErr fails every GetOperatorApp call. @@ -177,7 +179,11 @@ func (f *fakeAgent) ListServices(_ context.Context, in *devopsagent.ListServices } func (f *fakeAgent) ListAssociations(_ context.Context, in *devopsagent.ListAssociationsInput, _ ...func(*devopsagent.Options)) (*devopsagent.ListAssociationsOutput, error) { - items, next := page(f.associations, in.NextToken, f.paginate) + links := f.associations + if f.spaceLinks != nil { + links = f.spaceLinks[aws.ToString(in.AgentSpaceId)] + } + items, next := page(links, in.NextToken, f.paginate) return &devopsagent.ListAssociationsOutput{Associations: items, NextToken: next}, nil } diff --git a/internal/awsdevops/lookup.go b/internal/awsdevops/lookup.go index 645b12d44..5e854e1d5 100644 --- a/internal/awsdevops/lookup.go +++ b/internal/awsdevops/lookup.go @@ -209,6 +209,22 @@ func FindAgentSpace(ctx context.Context, clients Clients, name string) (string, return "", nil } +// spacesNamed lists IDs of spaces named name. +func spacesNamed(ctx context.Context, clients Clients, name string) ([]string, error) { + spaces, err := listAgentSpaces(ctx, clients) + if err != nil { + return nil, err + } + var ids []string + for _, space := range spaces { + if strings.EqualFold(aws.ToString(space.Name), name) { + ids = append(ids, aws.ToString(space.AgentSpaceId)) + } + } + + return ids, nil +} + // FindAssociation returns the ID of an existing association between an agent // space and a service, so setup can be re-run without AWS rejecting duplicates. func FindAssociation(ctx context.Context, clients Clients, agentSpaceID, serviceID string) (string, error) { diff --git a/internal/awsdevops/plan_test.go b/internal/awsdevops/plan_test.go new file mode 100644 index 000000000..05a5f6c33 --- /dev/null +++ b/internal/awsdevops/plan_test.go @@ -0,0 +1,285 @@ +package awsdevops_test + +import ( + "context" + "strings" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/devopsagent" + agenttypes "github.com/aws/aws-sdk-go-v2/service/devopsagent/types" + iamtypes "github.com/aws/aws-sdk-go-v2/service/iam/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/launchdarkly/ldcli/internal/awsdevops" +) + +var planRoles = []string{awsdevops.AgentSpaceRoleName, awsdevops.OperatorAppRoleName} + +func planWith(t *testing.T, iamClient *fakeIAM, provision bool) awsdevops.SetupPlan { + t.Helper() + plan, err := awsdevops.PlanSetup(context.Background(), newTestClients(&fakeAgent{}, iamClient), awsdevops.SetupOptions{ + AgentSpaceName: "launchdarkly", + AgentSpaceID: map[bool]string{true: "", false: "space-2"}[provision], + }, provision) + require.NoError(t, err) + + return plan +} + +// roleLine returns the plan line naming role. +func roleLine(t *testing.T, plan awsdevops.SetupPlan, role string) string { + t.Helper() + for _, line := range plan.Roles { + if strings.Contains(line, role+" ") || strings.HasSuffix(line, role) { + return strings.ToLower(line) + } + } + t.Fatalf("no plan line names role %s in %q", role, plan.Roles) + + return "" +} + +func TestPlanSetupCreatesMissingRoles(t *testing.T) { + plan := planWith(t, newFakeIAM(), true) + + for _, role := range planRoles { + assert.Contains(t, roleLine(t, plan, role), "create") + } +} + +func TestPlanSetupUpdatesManagedRoles(t *testing.T) { + iamClient := newFakeIAM() + for _, role := range planRoles { + iamClient.addManagedRole(role) + } + + plan := planWith(t, iamClient, true) + + for _, role := range planRoles { + line := roleLine(t, plan, role) + assert.Contains(t, line, "update") + assert.NotContains(t, line, "create") + } +} + +// A tag with the right key but wrong value is not ours. +func TestPlanSetupLeavesUnmanagedRoles(t *testing.T) { + iamClient := newFakeIAM() + for _, role := range planRoles { + iamClient.existingRoles[role] = true + iamClient.roleTags[role] = []iamtypes.Tag{{ + Key: aws.String(awsdevops.ManagedTagKey), + Value: aws.String("something-else"), + }} + } + + plan := planWith(t, iamClient, true) + + for _, role := range planRoles { + line := roleLine(t, plan, role) + assert.Contains(t, line, "unchanged") + assert.NotContains(t, line, "update") + assert.NotContains(t, line, "create") + } +} + +func TestPlanSetupEnablesTheOperatorApp(t *testing.T) { + plan := planWith(t, newFakeIAM(), true) + + assert.Contains(t, strings.ToLower(plan.OperatorApp), "enable") +} + +// Connecting never touches roles or the app. +func TestPlanConnectLeavesRolesAndAppAlone(t *testing.T) { + plan := planWith(t, newFakeIAM(), false) + + require.NotEmpty(t, plan.Roles) + for _, line := range plan.Roles { + lower := strings.ToLower(line) + assert.NotContains(t, lower, "create") + assert.NotContains(t, lower, "update") + } + require.NotEmpty(t, plan.OperatorApp) + assert.NotContains(t, strings.ToLower(plan.OperatorApp), "enable") +} + +func TestPlanSetupMakesNoIAMWrites(t *testing.T) { + iamClient := newFakeIAM() + iamClient.addManagedRole(awsdevops.AgentSpaceRoleName) + + _ = planWith(t, iamClient, true) + + assert.Empty(t, iamClient.createdRoles) + assert.Empty(t, iamClient.trustPolicies) + assert.Empty(t, iamClient.inlinePolicies) + assert.Empty(t, iamClient.attachedPolicy) +} + +func TestPlanNamesTheAccountAssociation(t *testing.T) { + for name, links := range map[string][]agenttypes.Association{"keep": awsLink(), "associate": nil} { + t.Run(name, func(t *testing.T) { + agent := &fakeAgent{ + agentSpaces: []agenttypes.AgentSpace{namedSpace("space-7", "launchdarkly")}, + associations: links, + } + + plan, err := awsdevops.PlanSetup(context.Background(), newTestClients(agent, newFakeIAM()), runPlanOptions(), true) + + require.NoError(t, err) + assert.Contains(t, strings.ToLower(plan.Association), name) + assert.Contains(t, plan.Association, testAccountID) + }) + } +} + +func TestPlanConnectChangesNothingElse(t *testing.T) { + agent := reuseWithApp(idcApp()) + opts := runPlanOptions() + opts.AgentSpaceID = "space-7" + + plan, err := awsdevops.PlanSetup(context.Background(), newTestClients(agent, newFakeIAM()), opts, false) + + require.NoError(t, err) + assert.Equal(t, "No change", plan.Association) + assert.Equal(t, "No change", plan.OperatorApp) + assert.Contains(t, agent.calls, "GetOperatorApp") + assert.NotContains(t, agent.calls, "EnableOperatorApp") +} + +// An app with no sign-in is off. +func TestPlanEnablesAppWithNoSignIn(t *testing.T) { + agent := reuseWithApp(&devopsagent.GetOperatorAppOutput{}) + + plan, err := awsdevops.PlanSetup(context.Background(), newTestClients(agent, newFakeIAM()), runPlanOptions(), true) + + require.NoError(t, err) + assert.Contains(t, strings.ToLower(plan.OperatorApp), "enable") +} + +// The same flow is not a change. +func TestPlanSameFlowIsNotAChange(t *testing.T) { + agent := reuseWithApp(&devopsagent.GetOperatorAppOutput{ + Iam: &agenttypes.IamAuthConfiguration{}, + OperatorAppUrl: aws.String("https://space-7.aidevops.global.app.aws"), + }) + opts := runPlanOptions() + opts.AuthFlowSet = true + + plan, err := awsdevops.PlanSetup(context.Background(), newTestClients(agent, newFakeIAM()), opts, true) + + require.NoError(t, err) + assert.NotContains(t, strings.ToLower(plan.OperatorApp), "change") + assert.Contains(t, plan.OperatorApp, "IAM") +} + +// Same IdC or IdP flow changes settings. +func TestPlanSameIdcFlowSaysUpdate(t *testing.T) { + apps := map[string]*devopsagent.GetOperatorAppOutput{ + "idc": idcApp(), + "idp": {Idp: &agenttypes.IdpAuthConfiguration{}, OperatorAppUrl: aws.String("https://space-7.aidevops.global.app.aws")}, + } + for flow, app := range apps { + t.Run(flow, func(t *testing.T) { + agent := reuseWithApp(app) + opts := runPlanOptions() + opts.AuthFlow, opts.AuthFlowSet = flow, true + + plan, err := awsdevops.PlanSetup(context.Background(), newTestClients(agent, newFakeIAM()), opts, true) + + require.NoError(t, err) + assert.NotContains(t, plan.OperatorApp, "again") + assert.Contains(t, strings.ToLower(plan.OperatorApp), "update") + }) + } +} + +func TestPlanAcceptsRegisteredMCPServiceID(t *testing.T) { + agent := &fakeAgent{services: []agenttypes.RegisteredService{launchDarklyMCPService("mcp-1")}} + opts := runPlanOptions() + opts.MCPServiceID = "mcp-1" + + plan, err := awsdevops.PlanSetup(context.Background(), newTestClients(agent, newFakeIAM()), opts, true) + + require.NoError(t, err) + assert.Equal(t, awsdevops.MCPReuse, plan.MCP.Action) +} + +// The replace plan names touched spaces. +func TestPlanReplaceNamesEachSpace(t *testing.T) { + opts := runPlanOptions() + opts.ReplaceMCPToken = true + + plan, err := awsdevops.PlanSetup(context.Background(), newTestClients(replaceAcrossSpaces(), newFakeIAM()), opts, true) + + require.NoError(t, err) + assert.Contains(t, plan.MCPServer, "space-1") + assert.Contains(t, plan.MCPServer, "space-3") + assert.NotContains(t, plan.MCPServer, "space-2") +} + +// Two spaces with one name leave no safe choice. +func TestPlanStopsOnTwoSpacesWithTheName(t *testing.T) { + agent := &fakeAgent{agentSpaces: []agenttypes.AgentSpace{ + {AgentSpaceId: aws.String("space-1"), Name: aws.String("launchdarkly")}, + {AgentSpaceId: aws.String("space-2"), Name: aws.String("LaunchDarkly")}, + }} + + _, err := awsdevops.PlanSetup(context.Background(), newTestClients(agent, newFakeIAM()), runPlanOptions(), true) + + require.Error(t, err) + assert.Contains(t, err.Error(), "--agent-space-id") + assert.Contains(t, err.Error(), "space-1, space-2") +} + +func awsLink() []agenttypes.Association { + return []agenttypes.Association{{AssociationId: aws.String("assoc-aws"), ServiceId: aws.String("aws")}} +} + +func runPlanOptions() awsdevops.SetupOptions { + return awsdevops.SetupOptions{AgentSpaceName: "launchdarkly", AuthFlow: "iam", LDAccessToken: "api-token"} +} + +func reuseWithApp(app *devopsagent.GetOperatorAppOutput) *fakeAgent { + agent := &fakeAgent{agentSpaces: []agenttypes.AgentSpace{namedSpace("space-7", "launchdarkly")}} + if app != nil { + agent.operatorApps = map[string]*devopsagent.GetOperatorAppOutput{"space-7": app} + } + + return agent +} + +func idcApp() *devopsagent.GetOperatorAppOutput { + return &devopsagent.GetOperatorAppOutput{ + Idc: &agenttypes.IdcAuthConfiguration{}, + OperatorAppUrl: aws.String("https://space-7.aidevops.global.app.aws"), + } +} + +func replaceAcrossSpaces() *fakeAgent { + return &fakeAgent{ + agentSpaces: []agenttypes.AgentSpace{ + namedSpace("space-1", "launchdarkly"), namedSpace("space-2", "other"), namedSpace("space-3", "third"), + }, + services: []agenttypes.RegisteredService{launchDarklyMCPService("mcp-old")}, + spaceLinks: map[string][]agenttypes.Association{"space-1": mcpLink("mcp-old"), "space-3": mcpLink("mcp-old")}, + } +} + +func mcpLink(serviceID string) []agenttypes.Association { + return []agenttypes.Association{{AssociationId: aws.String("assoc-" + serviceID), ServiceId: aws.String(serviceID)}} +} + +// A connect plan needs a space, and it never plans to create one. +func TestPlanConnectWithoutASpaceStops(t *testing.T) { + agent := &fakeAgent{agentSpaces: []agenttypes.AgentSpace{namedSpace("space-1", "other")}} + opts := runPlanOptions() + opts.AgentSpaceName = "missing" + + _, err := awsdevops.PlanSetup(context.Background(), newTestClients(agent, newFakeIAM()), opts, false) + + require.Error(t, err) + assert.Contains(t, err.Error(), "no agent space named missing") + assert.NotContains(t, agent.calls, "GetOperatorApp") +} diff --git a/internal/awsdevops/setup.go b/internal/awsdevops/setup.go index 8f379710c..b7c14437a 100644 --- a/internal/awsdevops/setup.go +++ b/internal/awsdevops/setup.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "slices" "strings" "time" @@ -57,7 +58,9 @@ type SetupOptions struct { AgentSpaceName string NewAgentSpace bool - AuthFlow string + AuthFlow string + // AuthFlowSet: the user passed --auth-flow. + AuthFlowSet bool IdcInstanceARN string IssuerURL string IdpClientID string @@ -92,6 +95,74 @@ type SetupResult struct { MCPAssociationID string `json:"mcpAssociationId,omitempty"` } +// RoleAction is what setup does to a role. +type RoleAction int + +const ( + RoleCreate RoleAction = iota + RoleUpdate + RoleLeave +) + +// RoleStep is one planned role change. +type RoleStep struct { + Name string + Action RoleAction +} + +func (s RoleStep) describe() string { + switch s.Action { + case RoleCreate: + return "Create role " + s.Name + case RoleUpdate: + return fmt.Sprintf("Update the trust policy of role %s", s.Name) + default: + return fmt.Sprintf("Leave role %s unchanged because it has no %s tag", s.Name, ManagedTagKey) + } +} + +// SpaceAction is the planned space action. +type SpaceAction int + +const ( + SpaceCreate SpaceAction = iota + SpaceReuse + SpaceConnect +) + +// SpaceStep is the planned agent space action. +type SpaceStep struct { + Action SpaceAction + ID string + Name string +} + +func (s SpaceStep) describe() string { + label := fmt.Sprintf("%s (%s)", s.Name, s.ID) + switch s.Action { + case SpaceCreate: + return "Create agent space " + s.Name + case SpaceConnect: + return "Connect to agent space " + label + default: + return "Reuse agent space " + label + } +} + +// LinkStep is the planned account association. +type LinkStep struct { + // Exists means the association was there. + Exists bool +} + +func (l LinkStep) describe(accountID string) string { + if l.Exists { + return fmt.Sprintf("Keep the AWS account %s association", accountID) + } + + return fmt.Sprintf("Associate AWS account %s with the agent space", accountID) +} + // AppAction is the planned operator app action. type AppAction int @@ -109,6 +180,35 @@ type AppStep struct { Flow string } +func (a AppStep) describe(flow string) string { + switch a.Action { + case AppKeep: + return "Keep the operator app and its sign-in" + case AppChange: + switch { + case a.Flow == flow && flow == "iam": + return "Set the operator app sign-in to IAM again" + case a.Flow == flow: + return fmt.Sprintf("Update the operator app %s sign-in settings", flowLabel(flow)) + } + + return "Change the operator app sign-in to " + flowLabel(flow) + default: + return fmt.Sprintf("Enable the operator app with %s sign-in", flowLabel(flow)) + } +} + +func flowLabel(flow string) string { + switch flow { + case "idc": + return "IdC" + case "idp": + return "IdP" + default: + return "IAM" + } +} + // appState reads the app. Flow "" means off. The space must exist. AWS sends // ResourceNotFoundException for an app that is off and for a space that does // not exist, so appState cannot tell the two apart. @@ -134,6 +234,254 @@ func appState(ctx context.Context, clients Clients, spaceID string) (AppStep, er return state, nil } +func planApp(ctx context.Context, clients Clients, opts SetupOptions, space SpaceStep) (AppStep, error) { + if space.Action == SpaceCreate { + return AppStep{Action: AppEnable}, nil + } + state, err := appState(ctx, clients, space.ID) + switch { + case err != nil: + return AppStep{}, err + case state.Flow == "": + return AppStep{Action: AppEnable}, nil + case opts.AuthFlowSet: + state.Action = AppChange + default: + state.Action = AppKeep + } + + return state, nil +} + +// MCPAction is the planned MCP server action. +type MCPAction int + +const ( + MCPRegister MCPAction = iota + MCPReuse + MCPReplace + MCPAsk +) + +// MCPStep is the planned MCP server step. +type MCPStep struct { + Action MCPAction + ServiceID string + // Spaces lists spaces linked to the server. + Spaces []string +} + +// SetupPlan describes what setup will change. +type SetupPlan struct { + MCP MCPStep + App AppStep + AccountID string + Region string + AgentSpace string + Space SpaceStep + Association string + // Link is set only in a full setup plan. A connect plan does not read or + // change the account association. + Link LinkStep + MCPServer string + Roles []string + RoleSteps []RoleStep + OperatorApp string +} + +// PlanSetup describes a run without changing AWS. +func PlanSetup(ctx context.Context, clients Clients, opts SetupOptions, provision bool) (SetupPlan, error) { + accountID, err := clients.AccountID(ctx) + if err != nil { + return SetupPlan{}, err + } + plan := SetupPlan{AccountID: accountID, Region: clients.Region} + + plan.Space, err = planAgentSpace(ctx, clients, opts, provision) + if err != nil { + return plan, err + } + plan.AgentSpace = plan.Space.describe() + + plan.MCP, err = planMCPStep(ctx, clients, opts) + if err != nil { + return plan, err + } + plan.MCPServer = plan.MCP.describe(opts.mcpEndpoint()) + + if !provision { + // Read the app, so the run does not print a URL that does not work. + plan.App, err = appState(ctx, clients, plan.Space.ID) + if err != nil { + return plan, err + } + plan.App.Action = AppKeep + plan.Roles = []string{"No change"} + plan.OperatorApp = "No change" + if plan.App.Flow == "" { + plan.OperatorApp = "No change. The operator app is off" + } + plan.Association = "No change" + + return plan, nil + } + if plan.Space.Action != SpaceCreate { + id, err := FindAssociation(ctx, clients, plan.Space.ID, awsServiceID) + if err != nil { + return plan, err + } + plan.Link.Exists = id != "" + } + plan.Association = plan.Link.describe(accountID) + for _, name := range []string{AgentSpaceRoleName, OperatorAppRoleName} { + step, err := planRole(ctx, clients.IAM, name) + if err != nil { + return plan, err + } + plan.RoleSteps = append(plan.RoleSteps, step) + plan.Roles = append(plan.Roles, step.describe()) + } + plan.App, err = planApp(ctx, clients, opts, plan.Space) + if err != nil { + return plan, err + } + plan.OperatorApp = plan.App.describe(opts.AuthFlow) + + return plan, nil +} + +// planRole says what setup will do to a role. +func planRole(ctx context.Context, client IAMAPI, name string) (RoleStep, error) { + _, err := client.GetRole(ctx, &iam.GetRoleInput{RoleName: aws.String(name)}) + var missing *iamtypes.NoSuchEntityException + switch { + case errors.As(err, &missing): + return RoleStep{Name: name, Action: RoleCreate}, nil + case err != nil: + return RoleStep{}, fmt.Errorf("unable to read the %s role: %w", name, err) + } + + managed, err := isManagedRole(ctx, client, name) + if err != nil { + return RoleStep{}, err + } + if !managed { + return RoleStep{Name: name, Action: RoleLeave}, nil + } + + return RoleStep{Name: name, Action: RoleUpdate}, nil +} + +func planAgentSpace(ctx context.Context, clients Clients, opts SetupOptions, provision bool) (SpaceStep, error) { + id := opts.AgentSpaceID + if id == "" { + seen, err := spacesNamed(ctx, clients, opts.AgentSpaceName) + switch { + case err != nil: + return SpaceStep{}, err + case len(seen) > 0 && opts.NewAgentSpace: + // Two spaces with one name make the next run unable to pick. + return SpaceStep{}, fmt.Errorf( + "an agent space named %s already exists (%s). Pass --agent-space-name with another name", + opts.AgentSpaceName, strings.Join(seen, ", "), + ) + case len(seen) > 1: + return SpaceStep{}, fmt.Errorf( + "%d agent spaces are named %s (%s). Pass --agent-space-id", + len(seen), opts.AgentSpaceName, strings.Join(seen, ", "), + ) + case len(seen) == 1: + id = seen[0] + } + } + if id == "" && !provision { + // A connect run never creates a space. + return SpaceStep{}, fmt.Errorf("no agent space named %s to connect to. Pass --agent-space-id", opts.AgentSpaceName) + } + if id == "" && opts.AgentSpaceName == "" { + return SpaceStep{}, errors.New("a new agent space needs a name. Pass --agent-space-name") + } + if id == "" { + return SpaceStep{Action: SpaceCreate, Name: opts.AgentSpaceName}, nil + } + + space, err := clients.Agent.GetAgentSpace(ctx, &devopsagent.GetAgentSpaceInput{AgentSpaceId: aws.String(id)}) + if err != nil { + return SpaceStep{}, fmt.Errorf("unable to read agent space %s: %w", id, err) + } + step := SpaceStep{Action: SpaceReuse, ID: id, Name: aws.ToString(space.AgentSpace.Name)} + if !provision { + step.Action = SpaceConnect + } + + return step, nil +} + +func planMCPStep(ctx context.Context, clients Clients, opts SetupOptions) (MCPStep, error) { + existing := opts.MCPServiceID + if existing != "" { + if err := checkMCPServiceID(ctx, clients, existing); err != nil { + return MCPStep{}, err + } + } + if existing == "" { + found, err := FindMCPServer(ctx, clients, opts.mcpEndpoint()) + if err != nil { + return MCPStep{}, err + } + existing = found + } + + switch { + case existing != "" && opts.ReplaceMCPToken && opts.LDAccessToken != "": + spaces, err := spacesWith(ctx, clients, existing) + if err != nil { + return MCPStep{}, err + } + + return MCPStep{Action: MCPReplace, ServiceID: existing, Spaces: spaces}, nil + case existing != "": + return MCPStep{Action: MCPReuse, ServiceID: existing}, nil + case opts.LDAccessToken != "": + return MCPStep{Action: MCPRegister}, nil + default: + return MCPStep{Action: MCPAsk}, nil + } +} + +// checkMCPServiceID needs a registered MCP server. +func checkMCPServiceID(ctx context.Context, clients Clients, id string) error { + services, err := listServices(ctx, clients) + if err != nil { + return err + } + for _, service := range services { + if aws.ToString(service.ServiceId) == id && service.ServiceType == agenttypes.ServiceMcpServer { + return nil + } + } + + return fmt.Errorf("ID %s names no registered MCP server", id) +} + +func (m MCPStep) describe(endpoint string) string { + switch m.Action { + case MCPReplace: + line := fmt.Sprintf("Replace the token of MCP server %s (%s)", MCPServerName, m.ServiceID) + if len(m.Spaces) > 0 { + line += ", reconnecting " + strings.Join(m.Spaces, ", ") + } + + return line + case MCPReuse: + return fmt.Sprintf("Reuse MCP server %s (%s)", MCPServerName, m.ServiceID) + case MCPRegister: + return fmt.Sprintf("Register MCP server %s at %s", MCPServerName, endpoint) + default: + return fmt.Sprintf("Ask for a token, then register MCP server %s", MCPServerName) + } +} + // Setup provisions the IAM roles, agent space, service associations and the // MCP server the AWS DevOps Agent needs to manage LaunchDarkly flags. func Setup(ctx context.Context, clients Clients, opts SetupOptions) (SetupResult, error) { @@ -447,6 +795,28 @@ func registerMCPServer( return nil } +// spacesWith lists spaces associated with serviceID. +func spacesWith(ctx context.Context, clients Clients, serviceID string) ([]string, error) { + spaces, err := listAgentSpaces(ctx, clients) + if err != nil { + return nil, err + } + var ids []string + for _, space := range spaces { + id := aws.ToString(space.AgentSpaceId) + link, err := FindAssociation(ctx, clients, id, serviceID) + if err != nil { + return nil, err + } + if link != "" { + ids = append(ids, id) + } + } + slices.Sort(ids) + + return ids, nil +} + // disassociateEverywhere removes a service from every agent space that uses // it, which AWS requires before the service can be deregistered. It returns // the agent spaces it was removed from, so the re-registered service can be From af700462852b24ad5c2b31a09a61a15ca911b5c9 Mon Sep 17 00:00:00 2001 From: Christina Chu Date: Wed, 21 Oct 2026 12:26:26 -0700 Subject: [PATCH 2/5] fix: describe the plan from its steps, and check the MCP server ID SetupPlan had a structured step and a text line for each part of the plan. A connect plan set the text to "No change" but kept other values in the steps, so the two could disagree. SetupPlan now holds only the steps, and Text describes them. MCPSkip is the step for a run without a token. --mcp-service-id must name an MCP server at the LaunchDarkly endpoint. For a server at another endpoint, setup stops and names --mcp-endpoint for a LaunchDarkly MCP server that the customer hosts. Co-Authored-By: Claude Opus 5.5 --- internal/awsdevops/plan_test.go | 103 +++++++++++++++++++++++++------ internal/awsdevops/setup.go | 104 +++++++++++++++++++++++--------- 2 files changed, 159 insertions(+), 48 deletions(-) diff --git a/internal/awsdevops/plan_test.go b/internal/awsdevops/plan_test.go index 05a5f6c33..8d74a4c3b 100644 --- a/internal/awsdevops/plan_test.go +++ b/internal/awsdevops/plan_test.go @@ -31,12 +31,12 @@ func planWith(t *testing.T, iamClient *fakeIAM, provision bool) awsdevops.SetupP // roleLine returns the plan line naming role. func roleLine(t *testing.T, plan awsdevops.SetupPlan, role string) string { t.Helper() - for _, line := range plan.Roles { + for _, line := range plan.Text().Roles { if strings.Contains(line, role+" ") || strings.HasSuffix(line, role) { return strings.ToLower(line) } } - t.Fatalf("no plan line names role %s in %q", role, plan.Roles) + t.Fatalf("no plan line names role %s in %q", role, plan.Text().Roles) return "" } @@ -88,21 +88,21 @@ func TestPlanSetupLeavesUnmanagedRoles(t *testing.T) { func TestPlanSetupEnablesTheOperatorApp(t *testing.T) { plan := planWith(t, newFakeIAM(), true) - assert.Contains(t, strings.ToLower(plan.OperatorApp), "enable") + assert.Contains(t, strings.ToLower(plan.Text().OperatorApp), "enable") } // Connecting never touches roles or the app. func TestPlanConnectLeavesRolesAndAppAlone(t *testing.T) { plan := planWith(t, newFakeIAM(), false) - require.NotEmpty(t, plan.Roles) - for _, line := range plan.Roles { + require.NotEmpty(t, plan.Text().Roles) + for _, line := range plan.Text().Roles { lower := strings.ToLower(line) assert.NotContains(t, lower, "create") assert.NotContains(t, lower, "update") } - require.NotEmpty(t, plan.OperatorApp) - assert.NotContains(t, strings.ToLower(plan.OperatorApp), "enable") + require.NotEmpty(t, plan.Text().OperatorApp) + assert.NotContains(t, strings.ToLower(plan.Text().OperatorApp), "enable") } func TestPlanSetupMakesNoIAMWrites(t *testing.T) { @@ -128,8 +128,8 @@ func TestPlanNamesTheAccountAssociation(t *testing.T) { plan, err := awsdevops.PlanSetup(context.Background(), newTestClients(agent, newFakeIAM()), runPlanOptions(), true) require.NoError(t, err) - assert.Contains(t, strings.ToLower(plan.Association), name) - assert.Contains(t, plan.Association, testAccountID) + assert.Contains(t, strings.ToLower(plan.Text().Association), name) + assert.Contains(t, plan.Text().Association, testAccountID) }) } } @@ -142,8 +142,8 @@ func TestPlanConnectChangesNothingElse(t *testing.T) { plan, err := awsdevops.PlanSetup(context.Background(), newTestClients(agent, newFakeIAM()), opts, false) require.NoError(t, err) - assert.Equal(t, "No change", plan.Association) - assert.Equal(t, "No change", plan.OperatorApp) + assert.Equal(t, "No change", plan.Text().Association) + assert.Equal(t, "No change", plan.Text().OperatorApp) assert.Contains(t, agent.calls, "GetOperatorApp") assert.NotContains(t, agent.calls, "EnableOperatorApp") } @@ -155,7 +155,7 @@ func TestPlanEnablesAppWithNoSignIn(t *testing.T) { plan, err := awsdevops.PlanSetup(context.Background(), newTestClients(agent, newFakeIAM()), runPlanOptions(), true) require.NoError(t, err) - assert.Contains(t, strings.ToLower(plan.OperatorApp), "enable") + assert.Contains(t, strings.ToLower(plan.Text().OperatorApp), "enable") } // The same flow is not a change. @@ -170,8 +170,8 @@ func TestPlanSameFlowIsNotAChange(t *testing.T) { plan, err := awsdevops.PlanSetup(context.Background(), newTestClients(agent, newFakeIAM()), opts, true) require.NoError(t, err) - assert.NotContains(t, strings.ToLower(plan.OperatorApp), "change") - assert.Contains(t, plan.OperatorApp, "IAM") + assert.NotContains(t, strings.ToLower(plan.Text().OperatorApp), "change") + assert.Contains(t, plan.Text().OperatorApp, "IAM") } // Same IdC or IdP flow changes settings. @@ -189,8 +189,8 @@ func TestPlanSameIdcFlowSaysUpdate(t *testing.T) { plan, err := awsdevops.PlanSetup(context.Background(), newTestClients(agent, newFakeIAM()), opts, true) require.NoError(t, err) - assert.NotContains(t, plan.OperatorApp, "again") - assert.Contains(t, strings.ToLower(plan.OperatorApp), "update") + assert.NotContains(t, plan.Text().OperatorApp, "again") + assert.Contains(t, strings.ToLower(plan.Text().OperatorApp), "update") }) } } @@ -206,6 +206,71 @@ func TestPlanAcceptsRegisteredMCPServiceID(t *testing.T) { assert.Equal(t, awsdevops.MCPReuse, plan.MCP.Action) } +// An ID of a server at another endpoint is not a LaunchDarkly MCP server. +func TestPlanStopsOnAnMCPServiceIDAtAnotherEndpoint(t *testing.T) { + agent := &fakeAgent{services: []agenttypes.RegisteredService{ + mcpService("mcp-9", "Datadog", "https://mcp.datadoghq.com/mcp"), + }} + opts := runPlanOptions() + opts.MCPServiceID = "mcp-9" + + _, err := awsdevops.PlanSetup(context.Background(), newTestClients(agent, newFakeIAM()), opts, true) + + require.Error(t, err) + assert.ErrorContains(t, err, "MCP server mcp-9 serves https://mcp.datadoghq.com/mcp") + assert.ErrorContains(t, err, "--mcp-endpoint https://mcp.datadoghq.com/mcp") +} + +// A LaunchDarkly MCP server that the customer hosts needs --mcp-endpoint. +func TestPlanAcceptsAnMCPServiceIDWithItsEndpoint(t *testing.T) { + agent := &fakeAgent{services: []agenttypes.RegisteredService{ + mcpService("mcp-9", "LaunchDarkly", "https://mcp.example.internal/mcp"), + }} + opts := runPlanOptions() + opts.MCPServiceID = "mcp-9" + opts.MCPEndpoint = "https://mcp.example.internal/mcp" + + plan, err := awsdevops.PlanSetup(context.Background(), newTestClients(agent, newFakeIAM()), opts, true) + + require.NoError(t, err) + assert.Equal(t, awsdevops.MCPReuse, plan.MCP.Action) + assert.Equal(t, "mcp-9", plan.MCP.ServiceID) +} + +// The text of a connect plan comes from its steps. +func TestPlanTextOfAConnectPlan(t *testing.T) { + for name, tc := range map[string]struct { + flow string + want string + }{ + "app on": {flow: "iam", want: "No change"}, + "app off": {want: "No change. The operator app is off"}, + } { + t.Run(name, func(t *testing.T) { + plan := awsdevops.SetupPlan{ + AccountID: testAccountID, + Space: awsdevops.SpaceStep{Action: awsdevops.SpaceConnect, ID: "space-7", Name: "platform"}, + App: awsdevops.AppStep{Action: awsdevops.AppKeep, Flow: tc.flow}, + MCP: awsdevops.MCPStep{Action: awsdevops.MCPRegister, Endpoint: awsdevops.MCPServerEndpoint}, + } + + text := plan.Text() + + assert.Equal(t, "Connect to agent space platform (space-7)", text.AgentSpace) + assert.Equal(t, "No change", text.Association) + assert.Equal(t, []string{"No change"}, text.Roles) + assert.Equal(t, tc.want, text.OperatorApp) + assert.Equal(t, "Register MCP server "+awsdevops.MCPServerName+" at "+awsdevops.MCPServerEndpoint, text.MCPServer) + }) + } +} + +func TestPlanTextOfASkippedMCPServer(t *testing.T) { + plan := awsdevops.SetupPlan{MCP: awsdevops.MCPStep{Action: awsdevops.MCPSkip}} + + assert.Equal(t, "Skip MCP server "+awsdevops.MCPServerName+", because no token was given", plan.Text().MCPServer) +} + // The replace plan names touched spaces. func TestPlanReplaceNamesEachSpace(t *testing.T) { opts := runPlanOptions() @@ -214,9 +279,9 @@ func TestPlanReplaceNamesEachSpace(t *testing.T) { plan, err := awsdevops.PlanSetup(context.Background(), newTestClients(replaceAcrossSpaces(), newFakeIAM()), opts, true) require.NoError(t, err) - assert.Contains(t, plan.MCPServer, "space-1") - assert.Contains(t, plan.MCPServer, "space-3") - assert.NotContains(t, plan.MCPServer, "space-2") + assert.Contains(t, plan.Text().MCPServer, "space-1") + assert.Contains(t, plan.Text().MCPServer, "space-3") + assert.NotContains(t, plan.Text().MCPServer, "space-2") } // Two spaces with one name leave no safe choice. diff --git a/internal/awsdevops/setup.go b/internal/awsdevops/setup.go index b7c14437a..2e15e0582 100644 --- a/internal/awsdevops/setup.go +++ b/internal/awsdevops/setup.go @@ -261,32 +261,67 @@ const ( MCPReuse MCPReplace MCPAsk + // MCPSkip means that the user gave no token, so the run skips the server. + MCPSkip ) // MCPStep is the planned MCP server step. type MCPStep struct { Action MCPAction ServiceID string + // Endpoint is the URL that the server serves. + Endpoint string // Spaces lists spaces linked to the server. Spaces []string } -// SetupPlan describes what setup will change. +// SetupPlan describes what setup will change. It holds only the steps. Text +// describes them, so the printed plan cannot disagree with the steps. type SetupPlan struct { - MCP MCPStep - App AppStep - AccountID string - Region string - AgentSpace string - Space SpaceStep - Association string + AccountID string + Region string + Space SpaceStep // Link is set only in a full setup plan. A connect plan does not read or // change the account association. - Link LinkStep - MCPServer string + Link LinkStep + RoleSteps []RoleStep + // App is the operator app at plan time. In a connect plan, its Action is + // AppKeep and its Flow is the current sign-in, which the run reports. + App AppStep + // AuthFlow is the sign-in that a full setup sets. + AuthFlow string + MCP MCPStep +} + +// PlanText is the plan as lines for a person to read. +type PlanText struct { + AgentSpace string + Association string Roles []string - RoleSteps []RoleStep OperatorApp string + MCPServer string +} + +// Text describes each step of the plan. +func (p SetupPlan) Text() PlanText { + text := PlanText{AgentSpace: p.Space.describe(), MCPServer: p.MCP.describe()} + if p.Space.Action == SpaceConnect { + text.Association = "No change" + text.Roles = []string{"No change"} + text.OperatorApp = "No change" + if p.App.Flow == "" { + text.OperatorApp = "No change. The operator app is off" + } + + return text + } + text.Association = p.Link.describe(p.AccountID) + for _, step := range p.RoleSteps { + text.Roles = append(text.Roles, step.describe()) + } + text.OperatorApp = p.App.describe(p.AuthFlow) + + return text } // PlanSetup describes a run without changing AWS. @@ -301,13 +336,11 @@ func PlanSetup(ctx context.Context, clients Clients, opts SetupOptions, provisio if err != nil { return plan, err } - plan.AgentSpace = plan.Space.describe() plan.MCP, err = planMCPStep(ctx, clients, opts) if err != nil { return plan, err } - plan.MCPServer = plan.MCP.describe(opts.mcpEndpoint()) if !provision { // Read the app, so the run does not print a URL that does not work. @@ -316,15 +349,10 @@ func PlanSetup(ctx context.Context, clients Clients, opts SetupOptions, provisio return plan, err } plan.App.Action = AppKeep - plan.Roles = []string{"No change"} - plan.OperatorApp = "No change" - if plan.App.Flow == "" { - plan.OperatorApp = "No change. The operator app is off" - } - plan.Association = "No change" return plan, nil } + plan.AuthFlow = opts.AuthFlow if plan.Space.Action != SpaceCreate { id, err := FindAssociation(ctx, clients, plan.Space.ID, awsServiceID) if err != nil { @@ -332,20 +360,17 @@ func PlanSetup(ctx context.Context, clients Clients, opts SetupOptions, provisio } plan.Link.Exists = id != "" } - plan.Association = plan.Link.describe(accountID) for _, name := range []string{AgentSpaceRoleName, OperatorAppRoleName} { step, err := planRole(ctx, clients.IAM, name) if err != nil { return plan, err } plan.RoleSteps = append(plan.RoleSteps, step) - plan.Roles = append(plan.Roles, step.describe()) } plan.App, err = planApp(ctx, clients, opts, plan.Space) if err != nil { return plan, err } - plan.OperatorApp = plan.App.describe(opts.AuthFlow) return plan, nil } @@ -418,9 +443,16 @@ func planAgentSpace(ctx context.Context, clients Clients, opts SetupOptions, pro } func planMCPStep(ctx context.Context, clients Clients, opts SetupOptions) (MCPStep, error) { + step, err := planMCPAction(ctx, clients, opts) + step.Endpoint = opts.mcpEndpoint() + + return step, err +} + +func planMCPAction(ctx context.Context, clients Clients, opts SetupOptions) (MCPStep, error) { existing := opts.MCPServiceID if existing != "" { - if err := checkMCPServiceID(ctx, clients, existing); err != nil { + if err := checkMCPServiceID(ctx, clients, existing, opts.mcpEndpoint()); err != nil { return MCPStep{}, err } } @@ -449,22 +481,34 @@ func planMCPStep(ctx context.Context, clients Clients, opts SetupOptions) (MCPSt } } -// checkMCPServiceID needs a registered MCP server. -func checkMCPServiceID(ctx context.Context, clients Clients, id string) error { +// checkMCPServiceID needs a registered MCP server at the LaunchDarkly +// endpoint. Setup finds a LaunchDarkly MCP server by its endpoint, because the +// name is the choice of the customer. A server at another endpoint is not one, +// so setup does not connect it to the agent space. +func checkMCPServiceID(ctx context.Context, clients Clients, id, endpoint string) error { services, err := listServices(ctx, clients) if err != nil { return err } for _, service := range services { - if aws.ToString(service.ServiceId) == id && service.ServiceType == agenttypes.ServiceMcpServer { - return nil + if aws.ToString(service.ServiceId) != id || service.ServiceType != agenttypes.ServiceMcpServer { + continue + } + if served := mcpEndpointOf(service); served != endpoint { + return fmt.Errorf( + "MCP server %s serves %s, not the LaunchDarkly MCP server at %s. "+ + "If it is a LaunchDarkly MCP server that you host, pass --mcp-endpoint %s", + id, served, endpoint, served, + ) } + + return nil } return fmt.Errorf("ID %s names no registered MCP server", id) } -func (m MCPStep) describe(endpoint string) string { +func (m MCPStep) describe() string { switch m.Action { case MCPReplace: line := fmt.Sprintf("Replace the token of MCP server %s (%s)", MCPServerName, m.ServiceID) @@ -476,7 +520,9 @@ func (m MCPStep) describe(endpoint string) string { case MCPReuse: return fmt.Sprintf("Reuse MCP server %s (%s)", MCPServerName, m.ServiceID) case MCPRegister: - return fmt.Sprintf("Register MCP server %s at %s", MCPServerName, endpoint) + return fmt.Sprintf("Register MCP server %s at %s", MCPServerName, m.Endpoint) + case MCPSkip: + return fmt.Sprintf("Skip MCP server %s, because no token was given", MCPServerName) default: return fmt.Sprintf("Ask for a token, then register MCP server %s", MCPServerName) } From d3f06cc33e4daa96ad4d32e9b1fd56d5aaab9dbd Mon Sep 17 00:00:00 2001 From: Christina Chu Date: Wed, 21 Oct 2026 12:33:59 -0700 Subject: [PATCH 3/5] fix: name the reused MCP server, and tidy the connect plan checks The plan line for a reused MCP server shows its name in AWS. Setup finds the server by its endpoint, so the name can differ from LaunchDarkly. A connect plan never creates a space, so --new-agent-space does not make a used name an error. Without a name, the connect error no longer has an empty gap where the name goes. An empty name matches no space. Co-Authored-By: Claude Opus 5.5 --- internal/awsdevops/lookup.go | 18 +++++++++-- internal/awsdevops/plan_test.go | 40 +++++++++++++++++++++++++ internal/awsdevops/setup.go | 53 +++++++++++++++++++++------------ 3 files changed, 89 insertions(+), 22 deletions(-) diff --git a/internal/awsdevops/lookup.go b/internal/awsdevops/lookup.go index 5e854e1d5..a09dabf79 100644 --- a/internal/awsdevops/lookup.go +++ b/internal/awsdevops/lookup.go @@ -180,17 +180,25 @@ func agentSpaceARN(region, accountID, spaceID string) string { // account has one. AWS keeps MCP servers at the account level, so a second // agent space reuses the existing registration. func FindMCPServer(ctx context.Context, clients Clients, endpoint string) (string, error) { + id, _, err := findMCPService(ctx, clients, endpoint) + + return id, err +} + +// findMCPService returns the ID and the name of the MCP server registered at +// endpoint, if the account has one. +func findMCPService(ctx context.Context, clients Clients, endpoint string) (id, name string, err error) { services, err := listServices(ctx, clients) if err != nil { - return "", err + return "", "", err } for _, service := range services { if mcpEndpointOf(service) == endpoint { - return aws.ToString(service.ServiceId), nil + return aws.ToString(service.ServiceId), serviceName(service), nil } } - return "", nil + return "", "", nil } // FindAgentSpace returns the ID of the agent space with the given name, or an @@ -211,6 +219,10 @@ func FindAgentSpace(ctx context.Context, clients Clients, name string) (string, // spacesNamed lists IDs of spaces named name. func spacesNamed(ctx context.Context, clients Clients, name string) ([]string, error) { + // An empty name names no space, even a space without a name. + if name == "" { + return nil, nil + } spaces, err := listAgentSpaces(ctx, clients) if err != nil { return nil, err diff --git a/internal/awsdevops/plan_test.go b/internal/awsdevops/plan_test.go index 8d74a4c3b..a43f4c7cf 100644 --- a/internal/awsdevops/plan_test.go +++ b/internal/awsdevops/plan_test.go @@ -271,6 +271,46 @@ func TestPlanTextOfASkippedMCPServer(t *testing.T) { assert.Equal(t, "Skip MCP server "+awsdevops.MCPServerName+", because no token was given", plan.Text().MCPServer) } +// The plan names the server by its name in AWS, which the customer can change. +func TestPlanNamesTheReusedMCPServerByItsName(t *testing.T) { + for name, id := range map[string]string{"found by endpoint": "", "given by ID": "mcp-1"} { + t.Run(name, func(t *testing.T) { + agent := &fakeAgent{services: []agenttypes.RegisteredService{ + mcpService("mcp-1", "LaunchDarkly (prod)", awsdevops.MCPServerEndpoint), + }} + opts := runPlanOptions() + opts.MCPServiceID = id + + plan, err := awsdevops.PlanSetup(context.Background(), newTestClients(agent, newFakeIAM()), opts, true) + + require.NoError(t, err) + assert.Equal(t, "Reuse MCP server LaunchDarkly (prod) (mcp-1)", plan.Text().MCPServer) + }) + } +} + +// A connect plan never creates a space, so a used name is not a clash. +func TestPlanConnectIgnoresNewAgentSpace(t *testing.T) { + agent := &fakeAgent{agentSpaces: []agenttypes.AgentSpace{namedSpace("space-7", "platform")}} + opts := runPlanOptions() + opts.AgentSpaceName = "platform" + opts.NewAgentSpace = true + + plan, err := awsdevops.PlanSetup(context.Background(), newTestClients(agent, newFakeIAM()), opts, false) + + require.NoError(t, err) + assert.Equal(t, awsdevops.SpaceStep{Action: awsdevops.SpaceConnect, ID: "space-7", Name: "platform"}, plan.Space) +} + +func TestPlanConnectWithoutANameAsksForTheID(t *testing.T) { + opts := runPlanOptions() + opts.AgentSpaceName = "" + + _, err := awsdevops.PlanSetup(context.Background(), newTestClients(&fakeAgent{}, newFakeIAM()), opts, false) + + assert.EqualError(t, err, "no agent space to connect to. Pass --agent-space-id") +} + // The replace plan names touched spaces. func TestPlanReplaceNamesEachSpace(t *testing.T) { opts := runPlanOptions() diff --git a/internal/awsdevops/setup.go b/internal/awsdevops/setup.go index 2e15e0582..95a96910a 100644 --- a/internal/awsdevops/setup.go +++ b/internal/awsdevops/setup.go @@ -271,6 +271,9 @@ type MCPStep struct { ServiceID string // Endpoint is the URL that the server serves. Endpoint string + // Name is the name of an existing server in AWS. The customer can change + // it, so it can differ from MCPServerName. + Name string // Spaces lists spaces linked to the server. Spaces []string } @@ -404,7 +407,7 @@ func planAgentSpace(ctx context.Context, clients Clients, opts SetupOptions, pro switch { case err != nil: return SpaceStep{}, err - case len(seen) > 0 && opts.NewAgentSpace: + case len(seen) > 0 && opts.NewAgentSpace && provision: // Two spaces with one name make the next run unable to pick. return SpaceStep{}, fmt.Errorf( "an agent space named %s already exists (%s). Pass --agent-space-name with another name", @@ -421,6 +424,10 @@ func planAgentSpace(ctx context.Context, clients Clients, opts SetupOptions, pro } if id == "" && !provision { // A connect run never creates a space. + if opts.AgentSpaceName == "" { + return SpaceStep{}, errors.New("no agent space to connect to. Pass --agent-space-id") + } + return SpaceStep{}, fmt.Errorf("no agent space named %s to connect to. Pass --agent-space-id", opts.AgentSpaceName) } if id == "" && opts.AgentSpaceName == "" { @@ -451,17 +458,15 @@ func planMCPStep(ctx context.Context, clients Clients, opts SetupOptions) (MCPSt func planMCPAction(ctx context.Context, clients Clients, opts SetupOptions) (MCPStep, error) { existing := opts.MCPServiceID + var name string + var err error if existing != "" { - if err := checkMCPServiceID(ctx, clients, existing, opts.mcpEndpoint()); err != nil { - return MCPStep{}, err - } + name, err = checkMCPServiceID(ctx, clients, existing, opts.mcpEndpoint()) + } else { + existing, name, err = findMCPService(ctx, clients, opts.mcpEndpoint()) } - if existing == "" { - found, err := FindMCPServer(ctx, clients, opts.mcpEndpoint()) - if err != nil { - return MCPStep{}, err - } - existing = found + if err != nil { + return MCPStep{}, err } switch { @@ -471,9 +476,9 @@ func planMCPAction(ctx context.Context, clients Clients, opts SetupOptions) (MCP return MCPStep{}, err } - return MCPStep{Action: MCPReplace, ServiceID: existing, Spaces: spaces}, nil + return MCPStep{Action: MCPReplace, ServiceID: existing, Name: name, Spaces: spaces}, nil case existing != "": - return MCPStep{Action: MCPReuse, ServiceID: existing}, nil + return MCPStep{Action: MCPReuse, ServiceID: existing, Name: name}, nil case opts.LDAccessToken != "": return MCPStep{Action: MCPRegister}, nil default: @@ -485,40 +490,50 @@ func planMCPAction(ctx context.Context, clients Clients, opts SetupOptions) (MCP // endpoint. Setup finds a LaunchDarkly MCP server by its endpoint, because the // name is the choice of the customer. A server at another endpoint is not one, // so setup does not connect it to the agent space. -func checkMCPServiceID(ctx context.Context, clients Clients, id, endpoint string) error { +// It returns the name of the server. +func checkMCPServiceID(ctx context.Context, clients Clients, id, endpoint string) (string, error) { services, err := listServices(ctx, clients) if err != nil { - return err + return "", err } for _, service := range services { if aws.ToString(service.ServiceId) != id || service.ServiceType != agenttypes.ServiceMcpServer { continue } if served := mcpEndpointOf(service); served != endpoint { - return fmt.Errorf( + return "", fmt.Errorf( "MCP server %s serves %s, not the LaunchDarkly MCP server at %s. "+ "If it is a LaunchDarkly MCP server that you host, pass --mcp-endpoint %s", id, served, endpoint, served, ) } - return nil + return serviceName(service), nil + } + + return "", fmt.Errorf("ID %s names no registered MCP server", id) +} + +// name is the name of the server in AWS, or the name that setup registers. +func (m MCPStep) name() string { + if m.Name != "" { + return m.Name } - return fmt.Errorf("ID %s names no registered MCP server", id) + return MCPServerName } func (m MCPStep) describe() string { switch m.Action { case MCPReplace: - line := fmt.Sprintf("Replace the token of MCP server %s (%s)", MCPServerName, m.ServiceID) + line := fmt.Sprintf("Replace the token of MCP server %s (%s)", m.name(), m.ServiceID) if len(m.Spaces) > 0 { line += ", reconnecting " + strings.Join(m.Spaces, ", ") } return line case MCPReuse: - return fmt.Sprintf("Reuse MCP server %s (%s)", MCPServerName, m.ServiceID) + return fmt.Sprintf("Reuse MCP server %s (%s)", m.name(), m.ServiceID) case MCPRegister: return fmt.Sprintf("Register MCP server %s at %s", MCPServerName, m.Endpoint) case MCPSkip: From ae261445490d04def1529292df575165ffbed7d0 Mon Sep 17 00:00:00 2001 From: Christina Chu Date: Wed, 21 Oct 2026 12:56:38 -0700 Subject: [PATCH 4/5] fix: name the current sign-in in a planned sign-in change The plan line for a sign-in change now says "from IAM to IdP", so a change to a sign-in that someone else set up is visible before setup asks to continue. Co-Authored-By: Claude Opus 5.5 --- internal/awsdevops/plan_test.go | 12 ++++++++++++ internal/awsdevops/setup.go | 4 +++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/internal/awsdevops/plan_test.go b/internal/awsdevops/plan_test.go index a43f4c7cf..bf1d1ea5a 100644 --- a/internal/awsdevops/plan_test.go +++ b/internal/awsdevops/plan_test.go @@ -195,6 +195,18 @@ func TestPlanSameIdcFlowSaysUpdate(t *testing.T) { } } +// A change names the current sign-in, so a reader sees what it replaces. +func TestPlanNamesTheCurrentSignInOfAChange(t *testing.T) { + agent := reuseWithApp(&devopsagent.GetOperatorAppOutput{Iam: &agenttypes.IamAuthConfiguration{}}) + opts := runPlanOptions() + opts.AuthFlow, opts.AuthFlowSet = "idp", true + + plan, err := awsdevops.PlanSetup(context.Background(), newTestClients(agent, newFakeIAM()), opts, true) + + require.NoError(t, err) + assert.Equal(t, "Change the operator app sign-in from IAM to IdP", plan.Text().OperatorApp) +} + func TestPlanAcceptsRegisteredMCPServiceID(t *testing.T) { agent := &fakeAgent{services: []agenttypes.RegisteredService{launchDarklyMCPService("mcp-1")}} opts := runPlanOptions() diff --git a/internal/awsdevops/setup.go b/internal/awsdevops/setup.go index 95a96910a..b138675c8 100644 --- a/internal/awsdevops/setup.go +++ b/internal/awsdevops/setup.go @@ -192,7 +192,9 @@ func (a AppStep) describe(flow string) string { return fmt.Sprintf("Update the operator app %s sign-in settings", flowLabel(flow)) } - return "Change the operator app sign-in to " + flowLabel(flow) + // Name the current sign-in, so a change to a sign-in that someone + // else set up is visible in the plan. + return fmt.Sprintf("Change the operator app sign-in from %s to %s", flowLabel(a.Flow), flowLabel(flow)) default: return fmt.Sprintf("Enable the operator app with %s sign-in", flowLabel(flow)) } From ab6b87065f07e0d0543de47ad02ed07eaeb5267f Mon Sep 17 00:00:00 2001 From: cchu-ld Date: Wed, 21 Oct 2026 14:27:23 -0700 Subject: [PATCH 5/5] Update internal/awsdevops/setup.go Co-authored-by: David Brackbill <77071769+davidbrackbill@users.noreply.github.com> --- internal/awsdevops/setup.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/awsdevops/setup.go b/internal/awsdevops/setup.go index b138675c8..2aef9db90 100644 --- a/internal/awsdevops/setup.go +++ b/internal/awsdevops/setup.go @@ -115,7 +115,7 @@ func (s RoleStep) describe() string { case RoleCreate: return "Create role " + s.Name case RoleUpdate: - return fmt.Sprintf("Update the trust policy of role %s", s.Name) + return fmt.Sprintf("Update the trust policy and permissions of role %s", s.Name) default: return fmt.Sprintf("Leave role %s unchanged because it has no %s tag", s.Name, ManagedTagKey) }