feat: add the experimentation-orchestration skill to the agent space in setup

cmd/aws_devops_agent/confirm_test.go

@@ -43,6 +43,12 @@ func (a countingAgent) CreateAgentSpace(context.Context, *devopsagent.CreateAgen
 	return &devopsagent.CreateAgentSpaceOutput{AgentSpace: &agenttypes.AgentSpace{AgentSpaceId: aws.String("space-new")}}, nil
 }
 
+func (a countingAgent) CreateAsset(context.Context, *devopsagent.CreateAssetInput, ...func(*devopsagent.Options)) (*devopsagent.CreateAssetOutput, error) {
+	a.writes.add("CreateAsset")
+
+	return &devopsagent.CreateAssetOutput{Asset: &agenttypes.Asset{AssetId: aws.String("ki-1")}}, nil
+}
+
 func (a countingAgent) DeregisterService(context.Context, *devopsagent.DeregisterServiceInput, ...func(*devopsagent.Options)) (*devopsagent.DeregisterServiceOutput, error) {
 	a.writes.add("DeregisterService")
 
@@ -83,6 +89,12 @@ func (a countingAgent) RegisterService(context.Context, *devopsagent.RegisterSer
 	return &devopsagent.RegisterServiceOutput{ServiceId: aws.String("mcp-1")}, nil
 }
 
+func (a countingAgent) UpdateAsset(context.Context, *devopsagent.UpdateAssetInput, ...func(*devopsagent.Options)) (*devopsagent.UpdateAssetOutput, error) {
+	a.writes.add("UpdateAsset")
+
+	return &devopsagent.UpdateAssetOutput{}, nil
+}
+
 type countingIAM struct{ writes *writes }
 
 func (c countingIAM) AttachRolePolicy(context.Context, *iam.AttachRolePolicyInput, ...func(*iam.Options)) (*iam.AttachRolePolicyOutput, error) {

cmd/aws_devops_agent/plan_print_test.go

@@ -65,6 +65,17 @@ func TestSetupPlanNamesRolesAndOperatorApp(t *testing.T) {
 	assert.Contains(t, out.String(), awsdevops.AgentSpaceRoleName)
 	assert.Contains(t, out.String(), awsdevops.OperatorAppRoleName)
 	assert.Contains(t, out.String(), "operator app")
+	assert.Contains(t, out.String(), "Skill:         Create skill "+awsdevops.ExperimentSkillName)
+}
+
+// A canceled plan does not create the skill.
+func TestSetupCanceledCreatesNoSkill(t *testing.T) {
+	cmd, _ := setupTestCmd(t, "n\n")
+	clients, w := countingClients()
+
+	require.Error(t, setupWith(cmd, clients, newSpaceOptions(), true, true))
+
+	assert.NotContains(t, w.calls, "CreateAsset")
 }
 
 func noTokenOptions() awsdevops.SetupOptions {
@@ -478,6 +489,43 @@ func TestSetupSaysWhenTheSecondPlanReusesAServer(t *testing.T) {
 	assert.Contains(t, stderrOf(cmd), "--access-token and --replace-mcp-token")
 }
 
+type failingSkillAgent struct{ countingAgent }
+
+func (failingSkillAgent) CreateAsset(context.Context, *devopsagent.CreateAssetInput, ...func(*devopsagent.Options)) (*devopsagent.CreateAssetOutput, error) {
+	return nil, &agenttypes.AccessDeniedException{Message: aws.String("not authorized to perform: aidevops:CreateAsset")}
+}
+
+// A failed skill step warns at the end, and setup still prints the app URL.
+func TestSetupWarnsWhenSkillFails(t *testing.T) {
+	cmd, errOut := setupTestCmd(t, "")
+	cmd.SetIn(noReadStdin{t})
+	clients, w := countingClients()
+	clients.Agent = failingSkillAgent{clients.Agent.(countingAgent)}
+
+	require.NoError(t, setupWith(cmd, clients, newSpaceOptions(), true, false))
+
+	assert.Contains(t, w.calls, "RegisterService")
+	assert.Contains(t, stdoutOf(cmd), "Warning: unable to create skill "+awsdevops.ExperimentSkillName)
+	assert.Contains(t, stdoutOf(cmd), "aidevops:CreateAsset")
+	assert.Contains(t, stdoutOf(cmd), "Open the DevOps Agent at:")
+	// The warning shows one time.
+	assert.Equal(t, 1, strings.Count(errOut.String()+stdoutOf(cmd), "unable to create skill"))
+}
+
+// stdout holds only the JSON, so the warning goes to stderr.
+func TestSetupJSONWarnsOnStderrWhenSkillFails(t *testing.T) {
+	cmd, errOut := setupTestCmd(t, "")
+	cmd.SetIn(noReadStdin{t})
+	clients, _ := countingClients()
+	clients.Agent = failingSkillAgent{clients.Agent.(countingAgent)}
+
+	require.NoError(t, setupWith(cmd, clients, newSpaceOptions(), false, false))
+
+	assert.True(t, json.Valid([]byte(stdoutOf(cmd))), "stdout is not JSON: %s", stdoutOf(cmd))
+	assert.Contains(t, stdoutOf(cmd), `"skillAction": "failed"`)
+	assert.Contains(t, errOut.String(), "Warning: unable to create skill "+awsdevops.ExperimentSkillName)
+}
+
 // Without a name from AWS, the warning gives only the ID, which identifies
 // the server, and does not guess a name.
 func TestSetupNamesANamelessServerByItsID(t *testing.T) {

cmd/aws_devops_agent/select_test.go

@@ -32,6 +32,22 @@ func (s stubAgent) AssociateService(context.Context, *devopsagent.AssociateServi
 	return nil, nil
 }
 
+func (s stubAgent) CreateAsset(context.Context, *devopsagent.CreateAssetInput, ...func(*devopsagent.Options)) (*devopsagent.CreateAssetOutput, error) {
+	return &devopsagent.CreateAssetOutput{Asset: &agenttypes.Asset{AssetId: aws.String("ki-1")}}, nil
+}
+
+func (s stubAgent) GetAssetFile(context.Context, *devopsagent.GetAssetFileInput, ...func(*devopsagent.Options)) (*devopsagent.GetAssetFileOutput, error) {
+	return nil, &agenttypes.ResourceNotFoundException{}
+}
+
+func (s stubAgent) ListAssets(context.Context, *devopsagent.ListAssetsInput, ...func(*devopsagent.Options)) (*devopsagent.ListAssetsOutput, error) {
+	return &devopsagent.ListAssetsOutput{}, nil
+}
+
+func (s stubAgent) UpdateAsset(context.Context, *devopsagent.UpdateAssetInput, ...func(*devopsagent.Options)) (*devopsagent.UpdateAssetOutput, error) {
+	return &devopsagent.UpdateAssetOutput{}, nil
+}
+
 func (s stubAgent) CreateAgentSpace(context.Context, *devopsagent.CreateAgentSpaceInput, ...func(*devopsagent.Options)) (*devopsagent.CreateAgentSpaceOutput, error) {
 	return nil, nil
 }
@@ -97,6 +113,10 @@ func (s stubAgent) ListAssociations(context.Context, *devopsagent.ListAssociatio
 	return &devopsagent.ListAssociationsOutput{Associations: s.associations}, nil
 }
 
+func (s stubAgent) UpdateAssociation(context.Context, *devopsagent.UpdateAssociationInput, ...func(*devopsagent.Options)) (*devopsagent.UpdateAssociationOutput, error) {
+	return &devopsagent.UpdateAssociationOutput{}, nil
+}
+
 func (s stubAgent) ListServices(context.Context, *devopsagent.ListServicesInput, ...func(*devopsagent.Options)) (*devopsagent.ListServicesOutput, error) {
 	return &devopsagent.ListServicesOutput{Services: s.services}, nil
 }

cmd/aws_devops_agent/setup.go

@@ -28,24 +28,27 @@ const (
 	replaceMCPTokenFlag = "replace-mcp-token"
 	mcpServiceIDFlag    = "mcp-service-id"
 	mcpEndpointFlag     = "mcp-endpoint"
+	replaceSkillFlag    = "replace-skill"
+	addMCPToolsFlag     = "add-mcp-tools"
 	yesFlag             = "yes"
 )
 
 func NewSetupCmd(analyticsTrackerFn analytics.TrackerFn) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "setup",
 		Short: "Provision the AWS DevOps Agent for LaunchDarkly",
-		Long: `Create the IAM roles, agent space, AWS account association, operator app and
-LaunchDarkly MCP server connection the AWS DevOps Agent needs.
+		Long: `Create the IAM roles, agent space, AWS account association, operator app,
+LaunchDarkly MCP server connection and experimentation skill the AWS DevOps
+Agent needs.
 
 When the account already has agent spaces, setup asks which one to connect
 LaunchDarkly to. On a space that setup did not make, setup only registers the
-MCP server. On a space that setup made, or with --full-setup, setup runs the
-full setup. A full setup turns on elevated actions on the space. Pass
---agent-space-id to pick a space without the list, or --new-agent-space to
-provision another space. A new space needs a name: pass --agent-space-name,
-or type one when setup asks. The run ends with the operator app URL, where
-the agent is used.
+MCP server and adds the skill. On a space that setup made, or with
+--full-setup, setup runs the full setup. A full setup turns on elevated
+actions on the space. Pass --agent-space-id to pick a space without the list,
+or --new-agent-space to provision another space. A new space needs a name:
+pass --agent-space-name, or type one when setup asks. The run ends with the
+operator app URL, where the agent is used.
 
 Each agent space gets its own LaunchDarkly MCP server registration, so each
 space can use a different token. The MCP server is connected with
@@ -55,7 +58,19 @@ session token from 'ldcli login' stops working once it expires. If the agent
 space already has an MCP server, setup reuses it and does not change its token.
 If it has none, setup asks for a service token in a terminal and checks it
 before it changes AWS. Without a terminal, or with --yes, it stops and asks
-for --access-token.`,
+for --access-token.
+
+A space that reuses a server keeps its tool allowlist. If the allowlist misses
+tools that this ldcli version allows, setup warns. To add them, pass
+--add-mcp-tools. AWS does not send back the classification of each tool, so
+the rewrite gives every tool the ldcli classification.
+
+The experiment-orchestration skill tells the agent how to run an experiment
+from a goal to a report. Every agent type in the space can load it. If the
+skill has the text of an older ldcli version, setup updates it. If someone
+edited it, setup does not change it. To overwrite the edits, pass
+--replace-skill. The skill is optional: if its step fails, setup warns and
+does the other steps.`,
 		Args:   cobra.NoArgs,
 		PreRun: trackRun(analyticsTrackerFn),
 		RunE:   runSetup,
@@ -76,6 +91,9 @@ for --access-token.`,
 	cmd.Flags().Bool(replaceMCPTokenFlag, false, "Give the agent space a new LaunchDarkly MCP server with the token passed with --access-token. Other agent spaces keep their server")
 	cmd.Flags().String(mcpServiceIDFlag, "", "Registered MCP server to connect, instead of the server of the agent space")
 	cmd.Flags().String(mcpEndpointFlag, "", "MCP endpoint to register. Setup reuses only a server at this endpoint. Defaults to "+awsdevops.MCPServerEndpoint)
+	cmd.Flags().Bool(replaceSkillFlag, false, "Overwrite the "+awsdevops.ExperimentSkillName+" skill if someone edited it")
+	cmd.Flags().Bool(addMCPToolsFlag, false, "Add the ldcli tools that the MCP server allowlist of an existing agent space misses. "+
+		"Every tool gets the ldcli classification")
 
 	cmd.Flags().Bool(yesFlag, false, "Ask no questions and accept every step")
 
@@ -174,6 +192,14 @@ func setupWith(
 	}
 
 	if !plaintext {
+		// stdout holds only the JSON, so a CI log shows the warning on stderr.
+		if result.MCPToolsError != "" {
+			_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Warning: %s\n", result.MCPToolsError)
+		}
+		if result.SkillError != "" {
+			_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Warning: %s\n", result.SkillError)
+		}
+
 		return printJSON(cmd, result)
 	}
 
@@ -184,6 +210,13 @@ func setupWith(
 			awsdevops.AccessTokenURL(opts.LDBaseURI))
 	}
 
+	if result.MCPToolsError != "" {
+		_, _ = fmt.Fprintf(cmd.OutOrStdout(), "\nWarning: %s\n", result.MCPToolsError)
+	}
+	if result.SkillError != "" {
+		_, _ = fmt.Fprintf(cmd.OutOrStdout(), "\nWarning: %s\n", result.SkillError)
+	}
+
 	// Only a connect run leaves the app off, on a space that setup did not
 	// make. The full setup would change that space, so point to the console.
 	if !result.OperatorAppEnabled {
@@ -507,7 +540,8 @@ func confirmSetup(cmd *cobra.Command, plan awsdevops.SetupPlan, ask bool) error
 	_, _ = fmt.Fprintf(out, "  Agent space:   %s\n", text.AgentSpace)
 	_, _ = fmt.Fprintf(out, "  Association:   %s\n", text.Association)
 	_, _ = fmt.Fprintf(out, "  Operator app:  %s\n", text.OperatorApp)
-	_, _ = fmt.Fprintf(out, "  MCP server:    %s\n\n", text.MCPServer)
+	_, _ = fmt.Fprintf(out, "  MCP server:    %s\n", text.MCPServer)
+	_, _ = fmt.Fprintf(out, "  Skill:         %s\n\n", text.Skill)
 	if !ask {
 		return nil
 	}
@@ -695,6 +729,8 @@ func setupOptions(cmd *cobra.Command) (awsdevops.SetupOptions, error) {
 		ReplaceMCPToken: mustBool(cmd, replaceMCPTokenFlag),
 		MCPServiceID:    mustString(cmd, mcpServiceIDFlag),
 		MCPEndpoint:     mustString(cmd, mcpEndpointFlag),
+		ReplaceSkill:    mustBool(cmd, replaceSkillFlag),
+		AddMCPTools:     mustBool(cmd, addMCPToolsFlag),
 	}
 
 	idcSet := opts.IdcInstanceARN != ""

internal/awsdevops/allowlist_test.go

@@ -0,0 +1,162 @@
+package awsdevops_test
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/aws/aws-sdk-go-v2/aws"
+	agenttypes "github.com/aws/aws-sdk-go-v2/service/devopsagent/types"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/launchdarkly/ldcli/internal/awsdevops"
+)
+
+// mcpAssociation is the association of server mcp-1 with space-2 that allows
+// these tools.
+func mcpAssociation(tools ...string) agenttypes.Association {
+	return agenttypes.Association{
+		AgentSpaceId:  aws.String("space-2"),
+		AssociationId: aws.String("assoc-mcp-1"),
+		ServiceId:     aws.String("mcp-1"),
+		Configuration: &agenttypes.ServiceConfigurationMemberMcpserver{
+			Value: agenttypes.MCPServerConfiguration{Tools: tools},
+		},
+		Capabilities: map[string]agenttypes.CapabilityConfiguration{"kept": {}},
+	}
+}
+
+func reuseServerOptions() awsdevops.SetupOptions {
+	opts := reuseOptions()
+	opts.LDAccessToken = ""
+	opts.AddMCPTools = true
+
+	return opts
+}
+
+// An older ldcli allowed fewer tools. A re-run adds the new ones, so the skill
+// can use them.
+func TestSetupAddsMissingToolsToAnExistingAllowlist(t *testing.T) {
+	agent := &fakeAgent{
+		services:     []agenttypes.RegisteredService{launchDarklyMCPService("mcp-1")},
+		associations: []agenttypes.Association{mcpAssociation("list-projects", "get-flag", "toggle-flag")},
+	}
+	clients := newTestClients(agent, newFakeIAM())
+
+	plan, err := awsdevops.PlanSetup(context.Background(), clients, reuseServerOptions(), true)
+	require.NoError(t, err)
+	require.Equal(t, awsdevops.MCPReuse, plan.MCP.Action)
+	assert.Contains(t, plan.Text().MCPServer, "and add get-project, get-environment, list-flags,")
+	assert.Contains(t, plan.Text().MCPServer, "update-flag-settings")
+
+	result, err := awsdevops.RunPlan(context.Background(), clients, reuseServerOptions(), plan)
+	require.NoError(t, err)
+
+	require.Len(t, agent.updateAssociations, 1)
+	in := agent.updateAssociations[0]
+	assert.Equal(t, "assoc-mcp-1", aws.ToString(in.AssociationId))
+	config, ok := in.Configuration.(*agenttypes.ServiceConfigurationMemberMcpserver)
+	require.True(t, ok)
+	assert.Contains(t, config.Value.Tools, "update-flag-settings")
+	assert.Contains(t, config.Value.ToolDetails, agenttypes.MCPToolDetail{
+		Name:               aws.String("update-flag-settings"),
+		ToolClassification: agenttypes.ToolClassificationMutative,
+	})
+	// AWS replaces the whole association, so setup sends the capabilities back.
+	assert.Contains(t, in.Capabilities, "kept")
+	assert.Contains(t, result.MCPToolsAdded, "create-metric")
+	assert.NotContains(t, result.MCPToolsAdded, "get-flag")
+	assert.Empty(t, result.MCPToolsError)
+}
+
+func TestSetupKeepsACompleteAllowlist(t *testing.T) {
+	agent := &fakeAgent{
+		services:     []agenttypes.RegisteredService{launchDarklyMCPService("mcp-1")},
+		associations: []agenttypes.Association{mcpAssociation(awsdevops.MCPTools()...)},
+	}
+
+	result, err := awsdevops.Setup(context.Background(), newTestClients(agent, newFakeIAM()), reuseServerOptions())
+	require.NoError(t, err)
+
+	assert.Empty(t, agent.updateAssociations)
+	assert.Empty(t, result.MCPToolsAdded)
+}
+
+// AWS does not send the classification of each tool back. Rewriting an
+// allowlist with a tool that ldcli does not know would make that tool
+// read-only, so setup warns instead.
+func TestSetupLeavesAnAllowlistWithForeignTools(t *testing.T) {
+	agent := &fakeAgent{
+		services:     []agenttypes.RegisteredService{launchDarklyMCPService("mcp-1")},
+		associations: []agenttypes.Association{mcpAssociation("get-flag", "delete-everything")},
+	}
+	clients := newTestClients(agent, newFakeIAM())
+
+	plan, err := awsdevops.PlanSetup(context.Background(), clients, reuseServerOptions(), true)
+	require.NoError(t, err)
+	assert.Contains(t, plan.Text().MCPServer, "which setup cannot add")
+
+	result, err := awsdevops.RunPlan(context.Background(), clients, reuseServerOptions(), plan)
+	require.NoError(t, err)
+
+	assert.Empty(t, agent.updateAssociations)
+	assert.Contains(t, result.MCPToolsError, "Add the missing tools in the AWS console")
+}
+
+// Setup cannot tell an older ldcli allowlist from one that someone trimmed,
+// so without --add-mcp-tools it only warns.
+func TestSetupWarnsAboutMissingToolsWithoutTheFlag(t *testing.T) {
+	agent := &fakeAgent{
+		services:     []agenttypes.RegisteredService{launchDarklyMCPService("mcp-1")},
+		associations: []agenttypes.Association{mcpAssociation("get-flag")},
+	}
+	clients := newTestClients(agent, newFakeIAM())
+	opts := reuseServerOptions()
+	opts.AddMCPTools = false
+
+	plan, err := awsdevops.PlanSetup(context.Background(), clients, opts, true)
+	require.NoError(t, err)
+	assert.Contains(t, plan.Text().MCPServer, "Pass --add-mcp-tools to add them")
+
+	result, err := awsdevops.RunPlan(context.Background(), clients, opts, plan)
+	require.NoError(t, err)
+
+	assert.Empty(t, agent.updateAssociations)
+	assert.Contains(t, result.MCPToolsError, "--add-mcp-tools")
+}
+
+// The run makes only the change that the plan showed.
+func TestSetupSkipsAnAllowlistThatChangedAfterThePlan(t *testing.T) {
+	agent := &fakeAgent{
+		services:     []agenttypes.RegisteredService{launchDarklyMCPService("mcp-1")},
+		associations: []agenttypes.Association{mcpAssociation("get-flag", "delete-everything")},
+	}
+	clients := newTestClients(agent, newFakeIAM())
+
+	plan, err := awsdevops.PlanSetup(context.Background(), clients, reuseServerOptions(), true)
+	require.NoError(t, err)
+	agent.associations = []agenttypes.Association{mcpAssociation("get-flag")}
+
+	result, err := awsdevops.RunPlan(context.Background(), clients, reuseServerOptions(), plan)
+	require.NoError(t, err)
+
+	assert.Empty(t, agent.updateAssociations)
+	assert.Contains(t, result.MCPToolsError, "changed after the plan")
+}
+
+// The server works with the tools that it has, so a failed update does not
+// fail setup.
+func TestSetupWarnsWhenTheAllowlistUpdateFails(t *testing.T) {
+	agent := &fakeAgent{
+		services:             []agenttypes.RegisteredService{launchDarklyMCPService("mcp-1")},
+		associations:         []agenttypes.Association{mcpAssociation("get-flag")},
+		updateAssociationErr: errors.New("AccessDeniedException: not allowed"),
+	}
+
+	result, err := awsdevops.Setup(context.Background(), newTestClients(agent, newFakeIAM()), reuseServerOptions())
+	require.NoError(t, err)
+
+	assert.Contains(t, result.MCPToolsError, "unable to add")
+	assert.Empty(t, result.MCPToolsAdded)
+}