Skip to content

feat: readable aws-devops-agent output, setup plan, and existing agent spaces - #875

Closed
ffantl-ld wants to merge 57 commits into
mainfrom
ffantl/aws-devops-existing-space
Closed

ffantl-ld wants to merge 57 commits into
mainfrom
ffantl/aws-devops-existing-space

Conversation

@ffantl-ld

@ffantl-ld ffantl-ld commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

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

Describe the solution you've provided

  • Setup asks which existing agent space to connect LaunchDarkly to (from Install LD MCP in existing aws devops agent space #853). --agent-space-id and --new-agent-space are mutually exclusive.
  • Setup decides by who made the space. It finds its own spaces by the description Managed by the LaunchDarkly CLI, and the list marks them made by ldcli. A space that setup made gets the full setup. Another space only gets the MCP server, unless you pass --auth-flow.
  • --agent-space-name has no default. With a name, Enter at the list picks the space with that name, or a new space with that name. Without a name, the list has no default, and a new space makes setup ask for a name. Setup does not create a space with a name that a space already uses.
  • An unattended run with --agent-space-name X uses X, or creates X if no space has that name. Without a name, it reuses the one space that setup made, or stops.
  • Setup prints a plan before the first AWS write: account, region, profile, IAM roles, agent space, operator app, and MCP server. In a terminal it asks Continue? [y/N]. --yes skips every question.
  • The plan, the space list, the prompts, and the progress go to stderr. Stdout has only the result, so a json run shows the plan and keeps valid JSON.
  • A connect run reads the operator app. If the app is off, setup does not print its URL, and the result has "operatorAppEnabled": false. This needs aidevops:GetOperatorApp.
  • Progress lines and status show names and plain states, not ARNs and raw IDs.
  • The credential error is short, with at most 5 profile names. Only SSO profiles get the aws sso login advice. Other profiles get the SDK cause, but not the cause from the EC2 metadata service.
  • Setup and status offer to run aws sso login when the SSO session of a profile expired.
  • 15 supported regions, up from 11 (us-east-2, ap-northeast-2, eu-west-3, eu-north-1 were missing).
  • The token check sends LD-API-Version: 20240415.

Behavior change from main: --agent-space-id on a space that setup did not make now only connects the MCP server. On main, it ran the full setup. No release has the command yet.

Describe alternatives you've considered

Commit 3654d6e1 (service token creation) is not included on purpose.

A tag on new agent spaces would be a stronger ownership marker than the description. Setup does not use one, because it must call ListTagsForResource once for each space to read tags.

Additional context

Testing: go test ./... and golangci-lint pass. I ran setup against a sandbox AWS account. These cases worked: a new space with a typed name, a new space next to existing spaces, and a second EnableOperatorApp call (IAM to IAM) on a space whose app is on.

Not tested against real AWS:

  • A change of sign-in method on an app that is on, for example IAM to IdC.
  • Role creation in an account without DevOpsAgentRole-AgentSpace and DevOpsAgentRole-WebappAdmin.

@Andrewjeska, can you review the space selection parts? They carry the #853 behavior, with the changes above.

🤖 Generated with Claude Code


Note

Overview
ldcli aws-devops-agent setup is reworked for existing AWS accounts: it can list agent spaces, let you pick one (or provision another via --agent-space-id / --new-agent-space), and choose full provisioning only for spaces managed by ldcli (or when --auth-flow is set); other spaces get MCP connect-only. A setup plan prints to stderr before any AWS write, with Continue? [y/N] in interactive mode; --yes and non-TTY/json runs skip prompts. Progress and prompts go to stderr so --output json keeps parseable stdout; missing tokens fail early in non-interactive runs unless an MCP server already exists. Results can report operatorAppEnabled: false when the operator app is off (uses GetOperatorApp).

status and setup share loadClients, which can offer aws sso login for expired SSO profiles. Credential errors are shorter and SSO-aware; four regions are added to the supported list. Token validation sends LD-API-Version: 20240415. Human status output uses plain labels instead of ARNs. README documents the new flows. Extensive tests cover planning, confirmation, space selection, and plan-drift (ErrPlanChanged).

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

Andrewjeska and others added 13 commits October 14, 2026 11:03
Setup now accepts us-east-2, ap-northeast-2, eu-west-3, and eu-north-1.
The access token check sends LD-API-Version 20240415.
The credential error no longer shows the AWS SDK cause.
It lists at most 5 profiles and points to aws configure list-profiles.
errors.Unwrap still returns the cause.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
If credential lookup fails for a profile with SSO configuration, setup
and status ask to run `aws sso login --profile <name>`. On yes, the
command runs it and builds the AWS clients one more time.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The status command now names each role with a state, not an ARN.
Each association names its service, not only the service ID.
If nothing is set up, the output names the setup command.
The JSON output does not change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Setup now shows a plan before the first write to AWS. The plan names
the AWS account, the region, the profile, the agent space action, and
the MCP server action. Setup then asks "Continue? [y/N]". Only "y" or
"yes" continues. Any other answer stops setup with an error.

The new --yes flag skips the question. Without a terminal, or with
--output json, setup does not ask.

The progress lines now name the thing first and put IDs in
parentheses. They do not show ARNs. The JSON result keeps the ARNs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Setup now prints the plan in every plaintext run, also with --yes and
without a terminal. It asks the question only in an interactive run.
JSON runs print no plan, so stdout stays valid JSON.

The plan now names each IAM role and what setup will do to it, and
what setup will do to the operator app.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The aws sso login offer now follows the same rule as the setup
question: JSON runs never ask. A service with no name or type now
prints as "Service (id)" in the status list. The token check uses the
API version constant from internal/resources.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With both flags, setup ran the full provisioning on the existing
space, which overwrote its operator app sign-in configuration. The
flags are now mutually exclusive.

The README now describes the setup plan and the aws sso login offer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ffantl-ld
ffantl-ld requested a review from Andrewjeska October 14, 2026 19:02
@devin-ai-integration

Copy link
Copy Markdown
Contributor

First review pass at 9d85a32. The build and go test pass. I ran the binary in a real terminal against a local mock of the DevOps Agent, STS, and IAM APIs (AWS_ENDPOINT_URL and fake keys). Nobody has run it against real AWS yet.

Main issues

  1. No plan and no prompt when stdout is not a terminal. ldcli sets --output to json when stdout is not a TTY (cmd/root.go:353). So in CI, or with ldcli aws-devops-agent setup | tee log, setup shows no plan, no picker, and no progress, and it changes AWS at once. The README says "Without a terminal, setup shows the plan". That is true only with --output plaintext. Fix the README, or base interactive on stdin/stderr and not on the output kind.
  2. Enter at the picker picks the first space that AWS returns (setup.go:285). In my run, that was another team's prod-ops space. I suggest a default of the space that matches --agent-space-name, and no default when no space matches:
    defaultChoice := 0
    for i, space := range spaces {
        if strings.EqualFold(space.Name, agentSpaceName) {
            defaultChoice = i + 1
        }
    }
    // prompt "[N]" only when defaultChoice > 0; when it is 0, an empty answer asks again
  3. The launchdarkly space gets different treatment in each mode. If you pick it in the picker, setup only connects the MCP server ("IAM role: No change", "Operator app: No change"). A non-interactive run finds the same space by name (setup.go:189) and does the full provisioning on it. These two paths must agree.

Smaller issues

  • If a launchdarkly space exists, "provision a new agent space" still plans "Create agent space launchdarkly". Then two spaces have the same name.
  • The credential error always says "Run aws sso login", and the cause is no longer shown. For a credential_process profile, this is the wrong advice. Also, the InvalidIdentityToken error still goes to stderr. Use HasSSOConfig to give the SSO advice only to SSO profiles, and show the cause for other profiles.
  • Each prompt makes a new bufio.NewReader on stdin (setup.go:241, :276, readLine). The shared reader at :119 skips *os.File, which is the real terminal case. It works now because a TTY gives one line for each read. Typed-ahead answers still worked in my test.
  • The plan and prompts go to stdout, and the progress goes to stderr.

Works as expected: the plan output, the [y/N] cancel path, the retry after an invalid picker answer, --yes, the mutually exclusive --agent-space-id/--new-agent-space, the new regions, the shortened profile list, the expired-SSO prompt, and the new status layout.

Written by Devin

ffantl-ld and others added 15 commits October 14, 2026 16:46
With --auth-flow or another sign-in flag, --agent-space-id now runs
the full setup on that space: roles, account association, and the
operator app sign-in. Without one, setup only connects the MCP server.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
--agent-space-id now runs the full setup only when --auth-flow is
passed. Before, --idc-instance-arn alone also triggered it and then
enabled the operator app with the default iam sign-in.

IdC flags now need --auth-flow=idc, and IdP flags need
--auth-flow=idp.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The setup test helper now gives stderr its own buffer, so a test can
tell which stream a line went to. setupWith now sets the progress
logger, so tests that call it see the progress lines.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The aws sso login prompt took stdin before setup shared it, so with
piped input it could consume the lines meant for the later prompts.
All prompts now read through one shared reader.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The token prompt made its own terminal check. It now uses the same
interactive value as the rest of setup, so one decision controls
every prompt.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Setup now builds one plan, shows it, and executes that plan. The plan
holds a step for each IAM role: create, update, or leave unchanged.
If a role changed after the plan, setup stops with ErrPlanChanged and
does not adopt or recreate it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Setup now compares both IAM roles with their planned steps before it
changes either one, so a changed operator app role no longer leaves a
half-updated agent space role. A role that appeared with the
ldcli:managed tag counts as already done, and setup continues.

The plan-changed errors are now short and name the role. The silent
token prompt test now runs with a terminal present, so it can fail.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When a role planned for creation already exists with the
ldcli:managed tag, setup now prints that it updated the role. If an
untagged role appears while setup creates it, the error now says the
role appeared.

Tests now cover CreateRole reporting an existing role, and the idp
sign-in with --agent-space-id.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The "Role X already exists, so ldcli updated it" line now prints once,
after the role update succeeds, and only for a planned create that
found the role. Tests now cover both roles, and check that a created
role, a role left unchanged, and a failed update print no such line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Tests now fail UpdateAssumeRolePolicy, AttachRolePolicy, and
PutRolePolicy for each role, and check that no "already exists" line
prints. A planned update also prints no such line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The write-failure tests now also run when CreateRole finds the role,
and check that the error comes from the call that failed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The plan now holds a typed agent space step: create, reuse, or
connect, with the space ID and its name from AWS. Setup checks the
step before any write, then uses the planned ID and does not search
by name again. If a space with the planned name appeared, or the
planned space is gone, setup stops with ErrPlanChanged.

The reuse line now shows the space name that AWS returned.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The create check now stops only for a same-name space that the plan
did not see, so --new-agent-space works when a space with that name
already exists. Setup checks the space again right before it creates
it, and RunPlan rejects a connect step.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The plan now reads the same-name spaces once, so a space that appears
between two lists cannot slip into the set the plan saw. FindAgentSpace
gets its doc comment back.

Tests now cover a space that appears during the role writes, and the
connect plan rejection.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A test now adds a same-name space right after the plan lists spaces,
and checks that setup stops instead of creating a duplicate.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ffantl-ld and others added 7 commits October 15, 2026 12:11
With --yes, setup no longer offers to run aws sso login. The help now
says that setup stops for a missing token only when no MCP server is
registered. A test covers --yes with an agent space that matches the
name.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Setup now checks every token that it will send to AWS, for a register
or a token replace, before it changes anything. A token that
LaunchDarkly rejects with 401 or 403 stops setup. That covers a first
setup, the connect path, and a pasted token. A token for a server that
setup reuses is not sent, so it is not checked.

Tests replace awsdevops.CheckToken so that they never call the real
LaunchDarkly API.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With --replace-mcp-token, the plan now names each agent space that
uses the MCP server, because the replace disconnects and reconnects
each one. If that set of spaces changes before the run, setup stops
before any write.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Setup now compares the spaces linked to the MCP server with the plan
again right before it disconnects them, so a space linked during the
role or space writes stops setup. A test checks that list order does
not matter. disassociateEverywhere gets its doc comment back.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When you choose an existing agent space from the list and pass
--auth-flow, setup now runs the full setup on that space, as it does
for --agent-space-id. Without --auth-flow, it only connects the MCP
server.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cchu-ld and others added 10 commits October 19, 2026 15:21
Setup wrote the plan and its prompts to stdout and the progress to
stderr. A json run showed no plan. If you redirected stdout, setup
hid its prompts and waited for input.

Now the plan, the space list, the prompts, and the progress go to
stderr. Stdout contains only the result. Every run shows the plan,
and a json run keeps valid JSON on stdout.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Before, a space that you picked from the list got only the MCP server.
An unattended run found a space by its name and ran the full setup on
it. Thus one space got two different results. Also, Enter at the list
picked the first space that AWS returned.

Now setup finds the spaces that it made by their description. A space
that setup made gets the full setup in every mode. A space that setup
did not make gets only the MCP server, unless you pass --auth-flow. An
unattended run no longer uses a space only because of its name.

The list marks the spaces that setup made. Enter picks a space only if
exactly one space that setup made has the --agent-space-name name.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A connect run built the operator app URL from the space ID and did not
check the app. If the app was off, setup printed a URL that did not
work.

Now the connect plan reads the app. If the app is on, setup prints the
URL that AWS returns. If the app is off, the plan says so, and the run
tells you to run setup again with --auth-flow.

The connect path now needs the aidevops:GetOperatorApp permission, as
the full setup already did.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The credential error told every named profile to run aws sso login,
and it never showed the cause. For a credential_process profile, that
advice was incorrect, and you could not see what failed.

Now an SSO profile gets the aws sso login advice. Another profile gets
the SDK cause. A profile with no credentials gets a message that says
so. The cause stays hidden if it comes from the EC2 metadata service,
because that error only says that no other source had credentials.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With --new-agent-space, or the new space choice in the list, setup
made a second space with the --agent-space-name name. After that,
setup could not pick its own space, so unattended runs stopped and
the list had no default.

Now setup stops before it changes AWS and asks for another name. A new
space never has a used name at plan time, so the plan no longer keeps
the list of same-name spaces.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Setup and ConnectExisting only had callers in tests, so they are now in
export_test.go. FindAgentSpace had no callers, so this commit deletes
it. resolveAgentSpace now reads opts.AuthFlowSet, so authFlagSet is
gone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
--agent-space-name had the default launchdarkly. That name is not
useful for a team space, because LaunchDarkly is only one MCP server
in it.

Now the flag has no default. With a name, Enter at the list picks the
space with that name, or a new space with that name when no space has
it. Without a name, the list has no default. A new space without a
name makes setup ask for one, and it asks again for an empty or used
name. An unattended run reuses the one space that setup made, and it
needs --agent-space-name to create a space.

Every prompt now reads through one shared stdin reader, so the list
does not take the answer for the name prompt.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
If GetOperatorApp returned no URL for an app that was on, RunPlan gave
an empty URL. Then setup told you that the app was off. Now RunPlan
builds the URL from the space ID, as ConnectPlan does.

This commit also adds a test for the name prompt with
--new-agent-space.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An unattended run with --agent-space-name stopped when no space had
the name, and when the named space was not made by setup. Thus no
single command worked on the first run and on later runs.

Now the run does what Enter does at the list. It uses the one space
with the name: full setup if setup made it, connect only if not. It
creates the space when no space has the name. It stops when two or
more spaces have the name. Without a name, it still reuses the one
space that setup made.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A json run that connected to a space with its operator app off only
left out operatorAppUrl. Now the result has operatorAppEnabled. A full
setup sets it to true, and a connect run sets it from the app state.
The plaintext message about an app that is off uses the same field.

This commit also deletes the "Pass --new-agent-space" hint. The CLI
always sets the space ID before it reuses a space, so the hint never
showed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cchu-ld

cchu-ld commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Taking over this PR -- these commits (235ba77..e5a2342) fix the issues from the earlier review.

  • 76ffd7f: The plan, the space list, and all prompts go to stderr. Stdout has only the result. A json run now shows the plan, and stdout stays valid JSON.
  • b4764a5: Setup decides by who made the space. It finds its own spaces by the description Managed by the LaunchDarkly CLI. A space that setup made gets the full setup. Another space gets only the MCP server, unless you pass --auth-flow.
  • 909818a: A connect run reads the operator app. If the app is off, setup does not print a URL that does not work. The connect path now needs aidevops:GetOperatorApp.
  • 88c8cf6: Only SSO profiles get the aws sso login advice. Other profiles get the SDK cause, but not the IMDS cause.
  • 84a1575: Setup does not create a second space with a name that a space already uses.
  • 03eab02: Setup and ConnectExisting moved to export_test.go. This commit deletes FindAgentSpace and authFlagSet.
  • d6f1267: --agent-space-name has no default. In a terminal, setup asks for the name of a new space. With a name, Enter picks the space with that name, or a new space with that name.
  • 560e245: A kept operator app always gets a URL.
  • 2b672e3: An unattended run with --agent-space-name X uses X, or creates X if no space has that name. Thus one command works on every CI run.
  • e5a2342: The result has operatorAppEnabled.

Behavior change from main: --agent-space-id on a space that setup did not make now only connects the MCP server. No release has the command yet.

Testing: go test ./... and golangci-lint pass. I ran setup against a sandbox account. A new space with a typed name worked. A second EnableOperatorApp call (IAM to IAM) worked.

Not tested against real AWS:

  • A change of sign-in method on an app that is on, for example IAM to IdC.
  • Role creation in an account without DevOpsAgentRole-AgentSpace and DevOpsAgentRole-WebappAdmin.

/this is a robot

@cchu-ld
cchu-ld marked this pull request as ready for review October 19, 2026 23:54
@cchu-ld
cchu-ld requested a review from a team as a code owner October 19, 2026 23:54
@cchu-ld
cchu-ld requested review from a team and wchieng and removed request for a team October 19, 2026 23:54
@cchu-ld

cchu-ld commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

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 1 potential issue.

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 e5a2342. Configure here.

Comment thread cmd/aws_devops_agent/prompt.go Resolved

@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 e5a2342. Configure here.

Comment thread cmd/aws_devops_agent/prompt.go Resolved
Comment thread cmd/aws_devops_agent/clients.go Resolved
Each prompt put a new buffer around stdin, so the first prompt took all
pasted lines. Now prompts read stdin one byte at a time.

Without --profile or AWS_PROFILE, the SDK uses the default profile. The
SSO login offer and the credential error now use it too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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