Skip to content

feat: read agent space ownership and operator app state - #927

Merged
cchu-ld merged 6 commits into
cchu/aws-devops-3-statusfrom
cchu/aws-devops-4a-space-state
Oct 21, 2026
Merged

cchu-ld merged 6 commits into
cchu/aws-devops-3-statusfrom
cchu/aws-devops-4a-space-state

Conversation

@cchu-ld

@cchu-ld cchu-ld commented Oct 20, 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

None.

Describe the solution you've provided

  • Problem: Setup had no way to tell its own agent spaces from other spaces.
    • Solution: AgentSpaces marks a space as ManagedByLDCLI only if it has the tag ldcli:managed=aws-devops-agent, the tag that setup already puts on its roles. A user can change the description, so setup does not read it. A later PR tags each space that setup creates.
  • Problem: An agent space does not report its ARN, and the tag API needs one.
    • Solution: AgentSpaces reads the tags of each space, so the list costs one ListTagsForResource call per space. It builds the ARN as arn:aws:aidevops:<region>:<account>:agentspace/<id>, the format in the role trust policy. If AgentSpaces cannot read the tags of a space, the summary of that space gets TagErr, and the other spaces stay in the list. A caller must not use that space as if its owner is known, because setup uses the owner to decide if it can change the roles and the operator app. If every lookup fails, AgentSpaces returns the error. AgentSpace, for one space, also puts a tag error on the summary. The error names aidevops:ListTagsForResource only for AccessDeniedException. The status command does not call AgentSpaces, so a failed tag lookup does not affect it.
  • Problem: A caller with a space ID had to list every space to learn whether setup made it.
    • Solution: AgentSpace reads one space with the same ownership check as AgentSpaces.
  • Problem: status printed an operator app URL even when the app was off, and that URL does not work.
    • Solution: appState reads the app with GetOperatorApp. GetStatus uses it for each space. If the app is off, the status has operatorAppEnabled: false and no URL, and the plaintext output says off. If the app is on, status shows the URL that AWS sends, like setup. It builds the URL only if AWS sends none.
  • Problem: status is a read command. A policy without aidevops:GetOperatorApp must not hide the roles, services, and connections.
    • Solution: If GetOperatorApp fails, status reports the app as unknown and shows the rest of the space. The JSON gets operatorAppError with the AWS error code, for example AccessDeniedException. The plaintext output says Operator app: unknown (AccessDeniedException).
  • Problem: AWS sends ResourceNotFoundException for an app that is off and for a space that does not exist.
    • Solution: appState treats the error as an app that is off. Its doc comment says that the space must exist. Each caller in the later PRs skips a space that setup will create.

The setup flow does not use AgentSpaces or AgentSpace yet. Later PRs use them. The plan PR, #928, uses AppAction, its constants AppEnable, AppKeep, and AppChange, and AppStep.Action. This PR does not set or read them.

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 agent space discovery with ownership detection via AgentSpaces / AgentSpace, using ListTagsForResource on a constructed agentspace ARN to set ManagedByLDCLI from the ldcli:managed tag. Tag read failures surface as per-space TagErr (or fail the whole list only if every lookup fails), with permission-specific messaging for AccessDeniedException.

Operator app reporting now calls GetOperatorApp through shared appState instead of always printing a synthetic URL. Status JSON gains operatorAppEnabled and operatorAppError; the CLI shows the AWS URL when enabled, off when disabled, or unknown (<code>) when the app cannot be read—without blocking roles, services, or associations.

Also extends the DevOps Agent client surface (GetOperatorApp, ListTagsForResource) and introduces AppStep / AppAction types for upcoming setup/plan work (not wired into setup flow in this PR).

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

@cchu-ld
cchu-ld force-pushed the cchu/aws-devops-4a-space-state branch from aca5dde to 81cc52a 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-4a-space-state branch from 81cc52a to 23bfc72 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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 23bfc72. Configure here.

@cchu-ld
cchu-ld marked this pull request as ready for review October 20, 2026 19:32
@cchu-ld
cchu-ld requested a review from a team as a code owner October 20, 2026 19:32
@cchu-ld
cchu-ld requested review from a team and btesfamariam and removed request for a team October 20, 2026 19:32
@Andrewjeska

Copy link
Copy Markdown
Contributor

Two notes — both about error paths in the new lookup / ownership helpers.

1. One ListTagsForResource failure hides every space from status

Any single ListTagsForResource error aborts the whole listing, so one transient 5xx or one weird space hides every other space from status. For a read command that's harsh. Two options: (a) continue the loop and mark the failing space with an "unknown" tri-state so status can show it as ?, or (b) if the all-or-nothing is deliberate because setup must never mis-detect ownership, say so in the AgentSpaces doc comment — right now a reader can't tell which this is.

2. appState conflates "operator app disabled" with "agent space doesn't exist"

ResourceNotFoundException from GetOperatorApp can mean either "the app is disabled" or "the agent space doesn't exist." Treating both as AppStep{} is fine when the caller just confirmed the space exists (today's GetStatus path), but the plan flow in #928 calls this before the space is created. Could you either (a) narrow the detection — e.g. check the exception message for the app-specific code — or (b) add a doc comment pinning down the precondition "space must exist" so the next PR knows to pre-check?

via LD Research 🤖

@cchu-ld
cchu-ld force-pushed the cchu/aws-devops-4a-space-state branch from 23bfc72 to 0885ccc Compare October 21, 2026 17:34
@cchu-ld
cchu-ld force-pushed the cchu/aws-devops-4a-space-state branch from 0885ccc to 15d4879 Compare October 21, 2026 17:47
@cchu-ld
cchu-ld force-pushed the cchu/aws-devops-4a-space-state branch from 15d4879 to 2a32490 Compare October 21, 2026 17:52
@cchu-ld
cchu-ld force-pushed the cchu/aws-devops-4a-space-state branch 2 times, most recently from b85e861 to 57bbce9 Compare October 21, 2026 18:18
@cchu-ld
cchu-ld force-pushed the cchu/aws-devops-4a-space-state branch from 57bbce9 to 446b9ef Compare October 21, 2026 18:36
@cchu-ld

cchu-ld commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

@Andrewjeska, thank you. Commit ea3c6d63 addresses both points.

  1. ListTagsForResource: One failed lookup no longer hides the other spaces. Commit 01fabfa2 keeps each space whose tags setup can read, and marks the others with TagErr. Commit 36c0096f on feat: ask which agent space to use and confirm the setup plan #922 leaves those spaces out of the list and adds a note that names them and says to pass --agent-space-id <id>. Setup does not guess the owner of a hidden space: the name checks still count it, and a run that cannot ask stops rather than pick around it. If every lookup fails, setup stops as before. The SDK already retries a throttle or a server error, and the error names aidevops:ListTagsForResource only for AccessDeniedException.

status does not call AgentSpaces, so a failed tag lookup does not affect it.

  1. appState: The doc comment now says that the space must exist, because AWS sends ResourceNotFoundException for an app that is off and for a space that does not exist. I did not match on the message text, because AWS does not document it. Each caller in the later PRs already skips a space that setup will create: planApp and checkApp return early for SpaceCreate, and the connect plan reads only a space that it found.

This reply is from Claude.

return summary, nil
}

// hasManagedTag reports the ldcli:managed tag on an agent space.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

wise

AgentSpaces lists the agent spaces in the region. A space counts as
made by setup (ManagedByLDCLI) only if it has the tag
ldcli:managed=aws-devops-agent. A user can change the description, so
setup does not read it. Each space costs one ListTagsForResource call.
An agent space does not report its ARN, so setup builds it. If setup
cannot read a tag, it stops with an error that names
aidevops:ListTagsForResource.
AgentSpace reads one space with the same check.

appState reads the operator app with GetOperatorApp. GetStatus uses
it. If the app is off, the status has operatorAppEnabled false and no
URL, and the plaintext output says "off".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cchu-ld and others added 5 commits October 21, 2026 14:32
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
status failed if GetOperatorApp failed for a reason other than an app
that is off, for example without aidevops:GetOperatorApp. The whole
report was then lost. status now reports the app as unknown with the AWS
error code, and shows the rest of the space.

status also uses the operator app URL that AWS sends, like setup. It
builds the URL only if AWS sends none.

Doc comments now say that appState needs a space that exists, and that
AgentSpaces fails on purpose if it cannot read the tag of a space.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The tag error said that setup needs aidevops:ListTagsForResource for
every failure. For another error, such as a server error that the SDK
retried, the message now gives only the AWS error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AgentSpaces failed if one tag lookup failed, so one space could hide every
other space. Now the summary of that space has TagErr, and the other
spaces stay in the list. If every lookup fails, AgentSpaces still returns
the error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AgentSpace failed if it could not read the tags of the space. It now
puts the error on the summary as TagErr, like AgentSpaces, so the caller
decides what to do with a space whose owner is unknown.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cchu-ld
cchu-ld force-pushed the cchu/aws-devops-4a-space-state branch from 42c2b8e to ff4e2fd Compare October 21, 2026 21:32
@cchu-ld
cchu-ld merged commit 3b4933e into main Oct 21, 2026
8 checks passed
@cchu-ld
cchu-ld deleted the cchu/aws-devops-4a-space-state 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