Skip to content

fix(deploy): surface original error types in preflight steps. #1525

Merged
Hweinstock merged 17 commits into
aws:mainfrom
Hweinstock:fix/deploy-unknown-error
Jun 18, 2026
Merged

fix(deploy): surface original error types in preflight steps. #1525
Hweinstock merged 17 commits into
aws:mainfrom
Hweinstock:fix/deploy-unknown-error

Conversation

@Hweinstock

Copy link
Copy Markdown
Contributor

Problem

The deploy command has a very high error rate, with the majority coming from an unknown source. This makes it difficult to know if customers are blocked on a legitimate bug, or are simply using the command incorrectly.

Solution

  • Refactor some of the preflight steps to surface the original error type instead of rewrapping in Error.
  • Rename some error types to better communicate their purpose.
  • Add more error names to the common error name mapping for stronger classification.

Notes

  • will follow-up with backend schema change before releasing this.

Testing

  • ran integ and unit tests.

@github-actions github-actions Bot added the size/l PR size: L label Jun 11, 2026
@github-actions github-actions Bot added the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 11, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jun 11, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review did not actually analyze this PR (model took 0 turns — the skill likely failed during setup). See the run for details; a later push or re-run is needed for a real review.

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 11, 2026
@github-actions

github-actions Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.20.0.tgz

How to install

gh release download pr-1525-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.20.0.tgz

@agentcore-cli-automation agentcore-cli-automation 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.

Thanks for tackling this — strongly typed errors and richer telemetry classification are great improvements. I have a few concerns, primarily around inconsistencies that I think will undermine the PR's stated goal of reducing the unknown error rate.

The biggest one: in setupApiKeyCredentialProvider you correctly wrap AWS-credential failures in AwsCredentialsError, but in setupSingleOAuth2Provider and setupPaymentCredentialProviders the equivalent paths just mutate error.message on the original SDK error. The error's name (and therefore its telemetry classification) is unchanged, so the OAuth/payment paths will still fall through to UnknownError in classifyError — defeating the whole point of the change for those paths. Details inline.

A couple of smaller items also inline.

Comment thread src/cli/operations/deploy/pre-deploy-identity.ts
Comment thread src/cli/operations/deploy/pre-deploy-identity.ts
Comment thread src/cli/commands/deploy/actions.ts
Comment thread src/cli/telemetry/error.ts
Comment thread src/cli/operations/identity/oauth2-credential-provider.ts
@github-actions github-actions Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 11, 2026
@Hweinstock

Copy link
Copy Markdown
Contributor Author

Addressed the main concern by rewrapping the error instead of mutating. most of these are symptoms of the existing implementation.

Ignored comment about adding unit tests for a map.

@github-actions github-actions Bot added size/l PR size: L and removed size/l PR size: L labels Jun 11, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jun 11, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: the review did not actually analyze this PR (model took 0 turns — the skill likely failed during setup). See the run for details; a later push or re-run is needed for a real review.

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 11, 2026
@github-actions github-actions Bot removed the size/l PR size: L label Jun 11, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot added claude-security-reviewing Claude Code /security-review in progress and removed claude-security-reviewing Claude Code /security-review in progress labels Jun 17, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 17, 2026

@agentcore-cli-automation agentcore-cli-automation 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.

Two issues that need a quick fix before merge. Most of the substantive feedback was already raised by the previous bot review and addressed; flagging these two new ones.

Comment thread src/cli/operations/deploy/pre-deploy-identity.ts Outdated
Comment thread src/cli/commands/deploy/actions.ts Outdated
@github-actions github-actions Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 17, 2026
@Hweinstock Hweinstock force-pushed the fix/deploy-unknown-error branch from 081e2dc to 1e6fdda Compare June 18, 2026 15:54
@github-actions github-actions Bot added size/l PR size: L and removed size/l PR size: L labels Jun 18, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Jun 18, 2026
@github-actions github-actions Bot added size/l PR size: L and removed size/l PR size: L labels Jun 18, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot added claude-security-reviewing Claude Code /security-review in progress and removed claude-security-reviewing Claude Code /security-review in progress labels Jun 18, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 18, 2026
@Hweinstock Hweinstock marked this pull request as ready for review June 18, 2026 16:10
@Hweinstock Hweinstock requested a review from a team June 18, 2026 16:10

@notgitika notgitika left a comment

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.

looks good to me!

@Hweinstock Hweinstock merged commit 1f5d794 into aws:main Jun 18, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/l PR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants