fix(studio-cp): resolve short service name to full ECS name before ListTasks - #28
Merged
Merged
Conversation
…stTasks `observe_deployment` accepted either the display short name (`orca`) or the full ECS name (`oab-prod-orca`) when matching a service, then passed the caller's string verbatim to `instance_status`. ECS `ListTasks` filters by `service_name`, which only accepts the full name, so a short name 404s as `ServiceNotFoundException`. The roster / `get_agent_states` path collects short names, so it failed for every agent once credentials resolved to the right account (previously masked by an AccessDenied earlier in the chain). - studio-cp: centralise the short->full mapping (`canonical_service_name` + `find_service`) and query tasks by the resolved full name. - oabctl: harden `instance_status` to fail loud when handed a non-`oab-` name instead of surfacing an opaque ECS `ServiceNotFoundException`. - test: `service_selector_resolves_to_full_ecs_name` guards the short-name path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…site Address review (Jelly): - #3: add `ServiceStatus::service_name` carrying the raw ECS name verbatim, and query tasks by it instead of `format!`-rebuilding `oab-{ns}-{name}`. The rebuild is wrong for any service that doesn't fit the `oab-<ns>-<name>` shape (the parser falls back to `namespace = "?"`), so this is a correctness fix, not just cleanup. - #4: the resolver (`resolve_service`) now returns the matched service whose `service_name` is passed to `instance_status` with zero further transformation, so the test asserts the exact value the call site queries with — not just an isolated helper. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
Reviewed at
Non-blocking follow-up (already agreed): audit the other ECS / CloudWatch-logs boundaries (e.g. the events filter) to confirm they also use the full service name — separate issue. Build/clippy/test intentionally left to CI (the workspace statically links the full aws-sdk; local release builds OOM), so merge should wait for a green run. |
This was referenced Aug 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
OAB Studio's
deploy_list/get_agent_states(roster) fail with:even though the ECS services (
oab-prod-orca,oab-prod-mira) exist and theresolved identity has ECS read permissions. It surfaces only after fleet
credentials resolve to the right account — before that an
AccessDeniedearlierin the chain masked it.
Root cause
studio-cp::observe_deploymentmatches the caller'sserviceselector againsteither the full ECS name (
oab-{ns}-{name}) or the display short name(
{name}):…but then passed that same (possibly short) string straight to
instance_status, which callsecs.list_tasks().service_name(service). ECSListTaskskeys on the full service name only, so a short name 404s asServiceNotFoundException.get_agent_states(t_states) collects services byshort name (
s.name), so it fails for every agent.Fix
canonical_service_name()find_service()— and query tasks by the resolved full name, never thecaller's raw string. Single read choke point (
observe_deployment) coversboth
t_states(short names) andt_get(arbitrary caller input).instance_statusto fail loud on a non-oab-name insteadof letting it reach ECS and return an opaque
ServiceNotFoundException.service_selector_resolves_to_full_ecs_nameguards the short-namepath (pure, no live ECS).
Not fixed here (follow-up)
Other short-name->ECS/logs boundaries aren't audited yet (e.g. the events filter
goes through CloudWatch string filtering). Suggest a separate audit: every
ECS/logs boundary must use the full service name.
Rollout
No config change — existing
fleets.toml+ credentials unaffected. Diagnosisverified end-to-end against the live
oabcluster (ap-east-2,504190915686): identity/region resolve correctly andListServicessucceeds;this PR fixes the subsequent
ListTasksstep. Build validation left to CI(workspace statically links the full aws-sdk; local release builds OOM).
🤖 Generated with Claude Code