Skip to content

refactor(core): use common arn util for parsing. - #2254

Merged
Hweinstock merged 5 commits into
aws:refactorfrom
Hweinstock:plain/arn-parsing-helpers
Sep 9, 2026
Merged

refactor(core): use common arn util for parsing. #2254
Hweinstock merged 5 commits into
aws:refactorfrom
Hweinstock:plain/arn-parsing-helpers

Conversation

@Hweinstock

Copy link
Copy Markdown
Contributor

Problem

Each component is re-implementing arn parsing, rather than relying on the top level src/core/arn.ts.

Solution

  • Migrate consumers within core to this shared implementation.
  • Consumers out of core (there are a few in handlers) are untouched.

Verification

  • tests passing.

@github-actions github-actions Bot added the size/s PR size: S label Sep 9, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added agentcore-harness-reviewing AgentCore Harness review in progress claude-security-reviewing Claude Code /security-review in progress labels Sep 9, 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 Sep 9, 2026

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

AgentCore Harness Review

Verdict: Looks good

Pure refactor that consolidates duplicated ARN parsing onto src/core/arn.ts. I traced each call site against the code it replaces and the behavior is preserved for real inputs:

  • accountIdFromArn / accountIdFromRoleArn: parseArn(arn)?.account returns the same value as arn.split(":")[4] for well-formed ARNs, and still throws for malformed input.
  • roleNameFromArnresourceNameFromArn: identical last-segment-after-/ result for IAM role ARNs (paths included).
  • resourceIdFromArn in policy.tsxresourceNameFromArn: same "strip if ARN, else pass through" contract via parseArn returning undefined for non-ARNs.
  • providerFromModelArn: only invoked after startsWith("arn:"), so equivalent for foundation-model and inference-profile ARNs.
  • knowledgeBaseRegion: the ||?? swap is safe because regionFromArn already normalizes an empty region to undefined, and returns undefined for non-ARNs.
  • resolveSkills in export.ts: credentialName/credentialArn are validated as simple names or ARNs, so the "contains / → last segment" fallback isn't needed in practice.

arn.ts itself is well-covered by arn.test.ts. No new user-facing behavior, so no telemetry is needed. Nothing blocking — LGTM to merge.

@agentcore-devx-automation agentcore-devx-automation Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Sep 9, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.73684% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 97.04%. Comparing base (d9ab959) to head (9d7e3eb).
⚠️ Report is 1 commits behind head on refactor.

Files with missing lines Patch % Lines
src/core/abTestExecutionRole.tsx 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           refactor    #2254   +/-   ##
=========================================
  Coverage     97.04%   97.04%           
=========================================
  Files           566      566           
  Lines         39409    39407    -2     
=========================================
  Hits          38244    38244           
+ Misses         1165     1163    -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Hweinstock
Hweinstock marked this pull request as ready for review September 9, 2026 16:49
@Hweinstock
Hweinstock merged commit e9136c5 into aws:refactor Sep 9, 2026
18 of 22 checks passed
@Hweinstock
Hweinstock deleted the plain/arn-parsing-helpers branch September 9, 2026 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/s PR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants