Skip to content

feat: plan the AWS setup steps - #928

Merged
cchu-ld merged 5 commits into
cchu/aws-devops-4a-space-statefrom
cchu/aws-devops-4b-setup-plan
Oct 21, 2026
Merged

cchu-ld merged 5 commits into
cchu/aws-devops-4a-space-statefrom
cchu/aws-devops-4b-setup-plan

Conversation

@cchu-ld

@cchu-ld cchu-ld commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary
This PR adds PlanSetup, which decides every setup step before setup writes to AWS. It reads AWS and returns a plan for the IAM roles, the agent space, the account association, the operator app, and the MCP server. It writes nothing. Nothing calls it yet, and main's Setup still runs. The next PR runs the plan.

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

Related issues

None.

Describe the solution you've provided

  • Problem: Setup wrote to AWS as it went. It created the IAM roles before it looked at the agent space. You could not see what it would change before it changed it.
    • Solution: PlanSetup reads AWS and returns a SetupPlan. The plan has one step each for the IAM roles, the agent space, the account association, the operator app, and the MCP server. PlanSetup writes nothing.
  • Problem: A plan with a step and a separate text line for each part can disagree. For example, a connect plan said No change for the operator app, but its step kept the current sign-in.
    • Solution: SetupPlan holds only the steps. Text describes them, so the printed plan always comes from the steps. In a connect plan, the app step keeps the current sign-in, because the run reports it, and Text says No change. MCPSkip is the step for a run in which the user gave no token.
  • Problem: Setup called EnableOperatorApp on every run with the default iam sign-in. On a reused space, this changed a sign-in that someone had set up.
    • Solution: If the operator app of a reused space is on, the plan keeps its sign-in, unless AuthFlowSet is true. The plan reads the app with appState.
  • Problem: FindAgentSpace reused the first space with the name. With two spaces of the same name, the choice was not predictable.
    • Solution: 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. A connect plan needs an existing space, so it stops if no space matches. A connect plan never creates a space, so --new-agent-space does not make a used name an error there. An empty name matches no space.
  • Problem: --mcp-service-id accepted any registered MCP server, so setup could connect a server that is not a LaunchDarkly MCP server.
    • Solution: The server must serve the LaunchDarkly endpoint, or the endpoint that you pass with --mcp-endpoint. Setup finds a LaunchDarkly MCP server by its endpoint everywhere else, because the name is the choice of the customer. For a server at another endpoint, the plan stops and names --mcp-endpoint for a LaunchDarkly MCP server that you host.

Nothing calls PlanSetup yet. main's Setup still runs. The next PR runs the plan. The plan is stricter than Setup: it stops on two spaces with one name, and on --new-agent-space with a used name, where Setup takes the first space or creates a second one. The next PR replaces Setup with the plan, so the printed plan matches what runs. The plan line for a reused MCP server shows the name of that server in AWS. The plan line for a sign-in change names the current sign-in, for example Change the operator app sign-in from IAM to IdP. The tests in plan_test.go check each plan decision.

Describe alternatives you've considered

None.

Additional context

Testing: go test ./... and golangci-lint pass on this branch.

🤖 Generated with Claude Code


Note

Overview
Adds PlanSetup, a read-only dry-run that inspects AWS and returns a SetupPlan (agent space, IAM roles, account association, operator app, MCP server) before any writes. Human-readable output comes from SetupPlan.Text(), which is derived from the structured steps so connect vs full-provision plans stay consistent.

Planning behavior is stricter and more conservative than today’s Setup: duplicate agent space names require --agent-space-id; connect mode never creates spaces; operator app sign-in is kept on reused spaces unless AuthFlowSet (explicit --auth-flow); --mcp-service-id must point at a LaunchDarkly MCP server at the expected (or --mcp-endpoint) URL, with replace flows naming affected spaces via spacesWith. findMCPService / checkMCPServiceID and lookup helpers support that validation.

Adds plan_test.go and extends test fakes (spaceLinks on ListAssociations). Setup is unchanged and nothing in production calls PlanSetup yet—intended for a follow-up PR to execute the plan.

Reviewed by Cursor Bugbot for commit 7121d9c. Bugbot is set up for automated code reviews on this repo. Configure here.

@cchu-ld
cchu-ld force-pushed the cchu/aws-devops-4b-setup-plan branch from 07a0ea4 to 3489632 Compare October 20, 2026 19:13
@cchu-ld
cchu-ld added this pull request to stack #929 October 20, 2026 19:13
@cchu-ld
cchu-ld force-pushed the cchu/aws-devops-4b-setup-plan branch from 3489632 to 57a42ab Compare October 20, 2026 19:22
@cchu-ld

cchu-ld commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 57a42ab. Configure here.

Comment thread internal/awsdevops/setup.go Resolved
Comment thread internal/awsdevops/setup.go Resolved
@cchu-ld
cchu-ld force-pushed the cchu/aws-devops-4b-setup-plan branch from 57a42ab to e4b9c97 Compare October 20, 2026 19:47
@cchu-ld
cchu-ld marked this pull request as ready for review October 20, 2026 19:50
@cchu-ld
cchu-ld requested a review from a team as a code owner October 20, 2026 19:50
@cchu-ld
cchu-ld requested review from wchieng and removed request for a team October 20, 2026 19:50
@cchu-ld cchu-ld mentioned this pull request Oct 20, 2026
1 of 3 tasks
@cchu-ld
cchu-ld requested a review from a team October 20, 2026 20:26
@Andrewjeska

Copy link
Copy Markdown
Contributor

Three notes on the plan representation and --mcp-service-id validation.

1. SetupPlan carries parallel structured + string schemas

SetupPlan has twin fields for each step — structured (Space, App, Link, MCP, RoleSteps) and pre-formatted (AgentSpace, OperatorApp, Association, MCPServer, Roles). If the renderer in #921 reads strings and tests read structs, they can diverge without failing anything. Consider making the plan purely structured and giving the step types a String()/describe() the renderer calls, so there's one source of truth.

2. Connect mode makes plan.App and plan.OperatorApp disagree

Connect mode sets plan.App.Action = AppKeep but keeps the real Flow/URL from appState, while plan.OperatorApp is overwritten to "No change". If a future consumer trusts plan.App.Flow, it gets different information than the string. Either zero the whole AppStep or compute the string from it.

3. checkMCPServiceID doesn't verify the service is a LaunchDarkly MCP

checkMCPServiceID only verifies the ID names some registered MCP server, not that it's a LaunchDarkly one. If the user passes an unrelated service's ID, the plan accepts it and setup later associates the wrong MCP server. Match on name/endpoint (or at minimum warn when it diverges).

via LD Research 🤖

@cchu-ld
cchu-ld force-pushed the cchu/aws-devops-4b-setup-plan branch from e4b9c97 to da7bd70 Compare October 21, 2026 17:34
@cchu-ld
cchu-ld force-pushed the cchu/aws-devops-4b-setup-plan branch from da7bd70 to e83aee9 Compare October 21, 2026 17:47
@cchu-ld
cchu-ld force-pushed the cchu/aws-devops-4b-setup-plan branch from e83aee9 to 4b37991 Compare October 21, 2026 17:52
@cchu-ld
cchu-ld force-pushed the cchu/aws-devops-4b-setup-plan branch from 4b37991 to 0217e1a Compare October 21, 2026 18:10
@cchu-ld
cchu-ld force-pushed the cchu/aws-devops-4b-setup-plan branch from 0217e1a to 0b7a5d8 Compare October 21, 2026 18:18
@cchu-ld
cchu-ld force-pushed the cchu/aws-devops-4b-setup-plan branch 3 times, most recently from d245990 to 9a7ec1f Compare October 21, 2026 19:11
@cchu-ld
cchu-ld force-pushed the cchu/aws-devops-4b-setup-plan branch from 9a7ec1f to dd626b0 Compare October 21, 2026 19:19
@cchu-ld

cchu-ld commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

@Andrewjeska, thanks! All three points are fixed.

  1. Parallel schemas: SetupPlan now holds only the structured steps (Space, Link, RoleSteps, App, AuthFlow, MCP). The text fields are gone. plan.Text() builds every printed line from the steps, so the printed plan and the steps cannot disagree. The renderer in feat: ask which agent space to use and confirm the setup plan #922 and the tests read the lines through Text(). feat: ask which agent space to use and confirm the setup plan #922 also wrote its own "Skip MCP server" line after the user pressed Enter at the token prompt. It now sets a new MCPSkip step, and RunPlan in feat: run the setup plan #921 skips the MCP server for that step. Commits: ee9853aa (feat: plan the AWS setup steps #928), 680bbba0 (feat: run the setup plan #921), ff6c1363 (feat: ask which agent space to use and confirm the setup plan #922).

  2. Connect mode: The fix for point 1 also fixes this. In a connect plan, App.Action is AppKeep, and App.Flow stays the current sign-in, because ConnectPlan reports it in operatorAppEnabled. Text() derives "No change" or "No change. The operator app is off" from that step, so the line and the step agree. TestPlanTextOfAConnectPlan covers both cases.

  3. --mcp-service-id: The server must now serve the LaunchDarkly endpoint, or the endpoint passed with --mcp-endpoint. Setup already finds a LaunchDarkly MCP server by its endpoint everywhere else, because the customer chooses the name. For a server at another endpoint, the plan stops. This is a behavior change. Before, --mcp-service-id alone could point at a self-hosted server on another endpoint. That case now needs --mcp-endpoint too. The README says so. Commit ee9853aa.

This reply is from Claude.

@cchu-ld
cchu-ld force-pushed the cchu/aws-devops-4b-setup-plan branch from 0371dfe to 53256f2 Compare October 21, 2026 19:59
Comment thread internal/awsdevops/setup.go Resolved
cchu-ld and others added 5 commits October 21, 2026 14:32
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Co-authored-by: David Brackbill <77071769+davidbrackbill@users.noreply.github.com>
@cchu-ld
cchu-ld force-pushed the cchu/aws-devops-4b-setup-plan branch from 7121d9c to ab6b870 Compare October 21, 2026 21:32
@cchu-ld
cchu-ld merged commit 80173e2 into main Oct 21, 2026
8 checks passed
@cchu-ld
cchu-ld deleted the cchu/aws-devops-4b-setup-plan branch October 21, 2026 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants