fix: correct AB test execution role IAM policy and promote stability - #1126

Merged
jariy17 merged 1 commit into
previewfrom
fix/ab-test-role-evaluator-permission-preview
May 6, 2026
Merged

fix: correct AB test execution role IAM policy and promote stability#1126
jariy17 merged 1 commit into
previewfrom
fix/ab-test-role-evaluator-permission-preview

Conversation

@jariy17

Copy link
Copy Markdown
Contributor

Cherry-pick of #1120 onto preview.

Summary

  • IAM policy — Aligns the auto-created AB test execution role with the official docs, fixing a 400 "Access denied when validating evaluator" error on deploy
  • Promote stabilitypromote ab-test waits for executionStatus === 'RUNNING' before stopping, avoiding 409 errors when promote runs immediately after resume
  • DescribeLogGroups — Split into its own statement with Resource: "*" (AWS requirement)
  • E2E coverage — Added AB test reaches RUNNING status after deploy to ab-test-target-based.test.ts
  • Status test — Fixed incorrect http-gateway resourceType assertion
  • Local e2e script — Added scripts/run-e2e-local.sh

See #1120 for full details.

…1120)
* fix: add GetEvaluator permission to AB test execution role
The AB test API validates evaluator ARNs server-side using the execution
role. The auto-created role policy was missing bedrock-agentcore:GetEvaluator,
causing a 400 "Access denied when validating evaluator" error on every deploy
that included a target-based AB test with a custom evaluator.
* fix: correct status assertion in target-based AB test e2e
HTTP gateways are not surfaced as top-level resources in agentcore status —
they are only used internally to build AB test invocation URLs. The test was
asserting on resourceType 'http-gateway' which never appears; fix it to
assert on the 'ab-test' resource instead.
* fix: add invocationUrl to status resource type cast in e2e test
The resource cast was missing invocationUrl, causing a TypeScript error
when asserting the AB test's gateway invocation URL was present.
* fix: improve promote error message and add local e2e run script
- Include stdout in promote failure message so the JSON error is visible
- Add scripts/run-e2e-local.sh to replicate the GitHub Actions e2e workflow locally
* fix: retry promote stop on 409 UPDATING status
When promote is called immediately after resume, the AB test may still be
in UPDATING state and reject the STOPPED transition with a 409. Retry up
to 6 times with a 10s delay to wait for the transition to complete.
* fix: wait for AB test to leave UPDATING state before promoting
Poll getABTest until executionStatus is no longer UPDATING before
attempting to stop. The service rejects updates with 409 while a
state transition is in progress (e.g. after resume).
* fix: wait for RUNNING before stopping AB test in promote
Poll getABTest until executionStatus === 'RUNNING' before issuing the
STOPPED transition. The service 409s if the test is still UPDATING
(e.g. transitioning from a prior resume).
* fix: resolve evaluator ARNs transitively from AB test online eval configs
Previously GetEvaluator was scoped to all project evaluators. Now it's
scoped to only the evaluators referenced by the specific online eval
configs this AB test uses, handling all three cases:
- Custom evaluators: looked up from deployedResources.evaluators
- Builtin.* evaluators: ARN constructed from the builtin ID
- External ARN references: used as-is
* fix: align AB test execution role policy with official docs
Replace the hand-rolled per-resource policy with the canonical policy
from https://docs.aws.amazon.com/bedrock-agentcore/latest/devguide/ab-testing-prereqs.html:
- Trust policy: add SourceAccount + SourceArn conditions
- Permissions: single AgentCoreResources statement scoped to
arn:aws:bedrock-agentcore:*:${accountId}:* with ResourceAccount condition
- Add missing actions: GetGatewayTarget, ListGatewayTargets,
ListConfigurationBundleVersions
- CloudWatch logs scoped to account via PrincipalAccount pattern
- Remove per-evaluator/per-resource ARN tracking (no longer needed)
* test: update IAM role unit tests to match new policy structure
* fix: throw error if AB test never reaches RUNNING before promote
* test: add unit tests for promote polling logic and extract to promote-utils
Extract waitForRunningThenStop into promote-utils.ts so it can be unit
tested without pulling in React/ink. Add 4 tests covering: immediate
RUNNING, polling until RUNNING, timeout throws, and error message content.
* fix: split DescribeLogGroups into wildcard resource statement
* test: verify AB test reaches RUNNING status after deploy in both e2e suites
* fix: remove console.log statements that leaked ARNs and fix bundle ARN/version format in config-bundle e2e test
* test: remove second deploy and RUNNING check from config-bundle e2e (follow-up with real bundles)
---------
Co-authored-by: Local E2E <ci@local>
@jariy17
jariy17 requested a review from a teamMay 5, 2026 23:04
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label May 5, 2026
@agentcore-cli-automation

Copy link
Copy Markdown

Reviewed the cherry-pick. All the serious issues I'd raise have already been flagged in the existing discussion:

  • The Builtin/external evaluator ARN concern was raised on fix: correct AB test execution role IAM policy and promote stability #1120 and addressed by moving to the docs-aligned policy template.
  • The gateway scope loosening was raised and justified by the author against the AWS docs.
  • The TUI promote race condition (same updateABTest({ executionStatus: 'STOPPED' }) call at ABTestDetailScreen.tsx:341 without a waitForRunningThenStop) was already raised by @notgitika on this PR and is still awaiting a response/fix.

The cherry-pick onto preview itself looks clean — the diff matches what landed on main via #1120, and the new waitForRunningThenStop utility + tests are correct. LGTM aside from the already-flagged TUI question.

@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 5, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines43.6%9964 / 22852
🔵Statements42.89%10589 / 24686
🔵Functions40.63%1684 / 4144
🔵Branches40.28%6430 / 15962
Generated in workflow #2463 for commit f8e8cdf by the Vitest Coverage Report Action

@notgitikanotgitika 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.

LGTM

@jariy17
jariy17 merged commit 62419a6 into previewMay 6, 2026
17 checks passed
@jariy17
jariy17 deleted the fix/ab-test-role-evaluator-permission-preview branch May 6, 2026 13:13
Sign up for freeto 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

@jariy17@agentcore-cli-automation@notgitika
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix: correct AB test execution role IAM policy and promote stability - #1126

Merged
jariy17 merged 1 commit into
previewfrom
fix/ab-test-role-evaluator-permission-preview
May 6, 2026
Merged

fix: correct AB test execution role IAM policy and promote stability#1126
jariy17 merged 1 commit into
previewfrom
fix/ab-test-role-evaluator-permission-preview

Conversation

@jariy17

Copy link
Copy Markdown
Contributor

Cherry-pick of #1120 onto preview.

Summary

  • IAM policy — Aligns the auto-created AB test execution role with the official docs, fixing a 400 "Access denied when validating evaluator" error on deploy
  • Promote stabilitypromote ab-test waits for executionStatus === 'RUNNING' before stopping, avoiding 409 errors when promote runs immediately after resume
  • DescribeLogGroups — Split into its own statement with Resource: "*" (AWS requirement)
  • E2E coverage — Added AB test reaches RUNNING status after deploy to ab-test-target-based.test.ts
  • Status test — Fixed incorrect http-gateway resourceType assertion
  • Local e2e script — Added scripts/run-e2e-local.sh

See #1120 for full details.

…1120)
* fix: add GetEvaluator permission to AB test execution role
The AB test API validates evaluator ARNs server-side using the execution
role. The auto-created role policy was missing bedrock-agentcore:GetEvaluator,
causing a 400 "Access denied when validating evaluator" error on every deploy
that included a target-based AB test with a custom evaluator.
* fix: correct status assertion in target-based AB test e2e
HTTP gateways are not surfaced as top-level resources in agentcore status —
they are only used internally to build AB test invocation URLs. The test was
asserting on resourceType 'http-gateway' which never appears; fix it to
assert on the 'ab-test' resource instead.
* fix: add invocationUrl to status resource type cast in e2e test
The resource cast was missing invocationUrl, causing a TypeScript error
when asserting the AB test's gateway invocation URL was present.
* fix: improve promote error message and add local e2e run script
- Include stdout in promote failure message so the JSON error is visible
- Add scripts/run-e2e-local.sh to replicate the GitHub Actions e2e workflow locally
* fix: retry promote stop on 409 UPDATING status
When promote is called immediately after resume, the AB test may still be
in UPDATING state and reject the STOPPED transition with a 409. Retry up
to 6 times with a 10s delay to wait for the transition to complete.
* fix: wait for AB test to leave UPDATING state before promoting
Poll getABTest until executionStatus is no longer UPDATING before
attempting to stop. The service rejects updates with 409 while a
state transition is in progress (e.g. after resume).
* fix: wait for RUNNING before stopping AB test in promote
Poll getABTest until executionStatus === 'RUNNING' before issuing the
STOPPED transition. The service 409s if the test is still UPDATING
(e.g. transitioning from a prior resume).
* fix: resolve evaluator ARNs transitively from AB test online eval configs
Previously GetEvaluator was scoped to all project evaluators. Now it's
scoped to only the evaluators referenced by the specific online eval
configs this AB test uses, handling all three cases:
- Custom evaluators: looked up from deployedResources.evaluators
- Builtin.* evaluators: ARN constructed from the builtin ID
- External ARN references: used as-is
* fix: align AB test execution role policy with official docs
Replace the hand-rolled per-resource policy with the canonical policy
from https://docs.aws.amazon.com/bedrock-agentcore/latest/devguide/ab-testing-prereqs.html:
- Trust policy: add SourceAccount + SourceArn conditions
- Permissions: single AgentCoreResources statement scoped to
arn:aws:bedrock-agentcore:*:${accountId}:* with ResourceAccount condition
- Add missing actions: GetGatewayTarget, ListGatewayTargets,
ListConfigurationBundleVersions
- CloudWatch logs scoped to account via PrincipalAccount pattern
- Remove per-evaluator/per-resource ARN tracking (no longer needed)
* test: update IAM role unit tests to match new policy structure
* fix: throw error if AB test never reaches RUNNING before promote
* test: add unit tests for promote polling logic and extract to promote-utils
Extract waitForRunningThenStop into promote-utils.ts so it can be unit
tested without pulling in React/ink. Add 4 tests covering: immediate
RUNNING, polling until RUNNING, timeout throws, and error message content.
* fix: split DescribeLogGroups into wildcard resource statement
* test: verify AB test reaches RUNNING status after deploy in both e2e suites
* fix: remove console.log statements that leaked ARNs and fix bundle ARN/version format in config-bundle e2e test
* test: remove second deploy and RUNNING check from config-bundle e2e (follow-up with real bundles)
---------
Co-authored-by: Local E2E <ci@local>
@jariy17
jariy17 requested a review from a teamMay 5, 2026 23:04
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label May 5, 2026
@agentcore-cli-automation

Copy link
Copy Markdown

Reviewed the cherry-pick. All the serious issues I'd raise have already been flagged in the existing discussion:

  • The Builtin/external evaluator ARN concern was raised on fix: correct AB test execution role IAM policy and promote stability #1120 and addressed by moving to the docs-aligned policy template.
  • The gateway scope loosening was raised and justified by the author against the AWS docs.
  • The TUI promote race condition (same updateABTest({ executionStatus: 'STOPPED' }) call at ABTestDetailScreen.tsx:341 without a waitForRunningThenStop) was already raised by @notgitika on this PR and is still awaiting a response/fix.

The cherry-pick onto preview itself looks clean — the diff matches what landed on main via #1120, and the new waitForRunningThenStop utility + tests are correct. LGTM aside from the already-flagged TUI question.

@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 5, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines43.6%9964 / 22852
🔵Statements42.89%10589 / 24686
🔵Functions40.63%1684 / 4144
🔵Branches40.28%6430 / 15962
Generated in workflow #2463 for commit f8e8cdf by the Vitest Coverage Report Action

@notgitikanotgitika 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.

LGTM

@jariy17
jariy17 merged commit 62419a6 into previewMay 6, 2026
17 checks passed
@jariy17
jariy17 deleted the fix/ab-test-role-evaluator-permission-preview branch May 6, 2026 13:13
Sign up for freeto 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

@jariy17@agentcore-cli-automation@notgitika
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: correct AB test execution role IAM policy and promote stability - #1126

Merged
jariy17 merged 1 commit into
previewfrom
fix/ab-test-role-evaluator-permission-preview
May 6, 2026
Merged

fix: correct AB test execution role IAM policy and promote stability#1126
jariy17 merged 1 commit into
previewfrom
fix/ab-test-role-evaluator-permission-preview

Conversation

@jariy17

Copy link
Copy Markdown
Contributor

Cherry-pick of #1120 onto preview.

Summary

  • IAM policy — Aligns the auto-created AB test execution role with the official docs, fixing a 400 "Access denied when validating evaluator" error on deploy
  • Promote stabilitypromote ab-test waits for executionStatus === 'RUNNING' before stopping, avoiding 409 errors when promote runs immediately after resume
  • DescribeLogGroups — Split into its own statement with Resource: "*" (AWS requirement)
  • E2E coverage — Added AB test reaches RUNNING status after deploy to ab-test-target-based.test.ts
  • Status test — Fixed incorrect http-gateway resourceType assertion
  • Local e2e script — Added scripts/run-e2e-local.sh

See #1120 for full details.

…1120)
* fix: add GetEvaluator permission to AB test execution role
The AB test API validates evaluator ARNs server-side using the execution
role. The auto-created role policy was missing bedrock-agentcore:GetEvaluator,
causing a 400 "Access denied when validating evaluator" error on every deploy
that included a target-based AB test with a custom evaluator.
* fix: correct status assertion in target-based AB test e2e
HTTP gateways are not surfaced as top-level resources in agentcore status —
they are only used internally to build AB test invocation URLs. The test was
asserting on resourceType 'http-gateway' which never appears; fix it to
assert on the 'ab-test' resource instead.
* fix: add invocationUrl to status resource type cast in e2e test
The resource cast was missing invocationUrl, causing a TypeScript error
when asserting the AB test's gateway invocation URL was present.
* fix: improve promote error message and add local e2e run script
- Include stdout in promote failure message so the JSON error is visible
- Add scripts/run-e2e-local.sh to replicate the GitHub Actions e2e workflow locally
* fix: retry promote stop on 409 UPDATING status
When promote is called immediately after resume, the AB test may still be
in UPDATING state and reject the STOPPED transition with a 409. Retry up
to 6 times with a 10s delay to wait for the transition to complete.
* fix: wait for AB test to leave UPDATING state before promoting
Poll getABTest until executionStatus is no longer UPDATING before
attempting to stop. The service rejects updates with 409 while a
state transition is in progress (e.g. after resume).
* fix: wait for RUNNING before stopping AB test in promote
Poll getABTest until executionStatus === 'RUNNING' before issuing the
STOPPED transition. The service 409s if the test is still UPDATING
(e.g. transitioning from a prior resume).
* fix: resolve evaluator ARNs transitively from AB test online eval configs
Previously GetEvaluator was scoped to all project evaluators. Now it's
scoped to only the evaluators referenced by the specific online eval
configs this AB test uses, handling all three cases:
- Custom evaluators: looked up from deployedResources.evaluators
- Builtin.* evaluators: ARN constructed from the builtin ID
- External ARN references: used as-is
* fix: align AB test execution role policy with official docs
Replace the hand-rolled per-resource policy with the canonical policy
from https://docs.aws.amazon.com/bedrock-agentcore/latest/devguide/ab-testing-prereqs.html:
- Trust policy: add SourceAccount + SourceArn conditions
- Permissions: single AgentCoreResources statement scoped to
arn:aws:bedrock-agentcore:*:${accountId}:* with ResourceAccount condition
- Add missing actions: GetGatewayTarget, ListGatewayTargets,
ListConfigurationBundleVersions
- CloudWatch logs scoped to account via PrincipalAccount pattern
- Remove per-evaluator/per-resource ARN tracking (no longer needed)
* test: update IAM role unit tests to match new policy structure
* fix: throw error if AB test never reaches RUNNING before promote
* test: add unit tests for promote polling logic and extract to promote-utils
Extract waitForRunningThenStop into promote-utils.ts so it can be unit
tested without pulling in React/ink. Add 4 tests covering: immediate
RUNNING, polling until RUNNING, timeout throws, and error message content.
* fix: split DescribeLogGroups into wildcard resource statement
* test: verify AB test reaches RUNNING status after deploy in both e2e suites
* fix: remove console.log statements that leaked ARNs and fix bundle ARN/version format in config-bundle e2e test
* test: remove second deploy and RUNNING check from config-bundle e2e (follow-up with real bundles)
---------
Co-authored-by: Local E2E <ci@local>
@jariy17
jariy17 requested a review from a teamMay 5, 2026 23:04
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label May 5, 2026
@agentcore-cli-automation

Copy link
Copy Markdown

Reviewed the cherry-pick. All the serious issues I'd raise have already been flagged in the existing discussion:

  • The Builtin/external evaluator ARN concern was raised on fix: correct AB test execution role IAM policy and promote stability #1120 and addressed by moving to the docs-aligned policy template.
  • The gateway scope loosening was raised and justified by the author against the AWS docs.
  • The TUI promote race condition (same updateABTest({ executionStatus: 'STOPPED' }) call at ABTestDetailScreen.tsx:341 without a waitForRunningThenStop) was already raised by @notgitika on this PR and is still awaiting a response/fix.

The cherry-pick onto preview itself looks clean — the diff matches what landed on main via #1120, and the new waitForRunningThenStop utility + tests are correct. LGTM aside from the already-flagged TUI question.

@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 5, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines43.6%9964 / 22852
🔵Statements42.89%10589 / 24686
🔵Functions40.63%1684 / 4144
🔵Branches40.28%6430 / 15962
Generated in workflow #2463 for commit f8e8cdf by the Vitest Coverage Report Action

@notgitikanotgitika 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.

LGTM

@jariy17
jariy17 merged commit 62419a6 into previewMay 6, 2026
17 checks passed
@jariy17
jariy17 deleted the fix/ab-test-role-evaluator-permission-preview branch May 6, 2026 13:13
Sign up for freeto 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

@jariy17@agentcore-cli-automation@notgitika
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: correct AB test execution role IAM policy and promote stability - #1126

Merged
jariy17 merged 1 commit into
previewfrom
fix/ab-test-role-evaluator-permission-preview
May 6, 2026
Merged

fix: correct AB test execution role IAM policy and promote stability#1126
jariy17 merged 1 commit into
previewfrom
fix/ab-test-role-evaluator-permission-preview

Conversation

@jariy17

Copy link
Copy Markdown
Contributor

Cherry-pick of #1120 onto preview.

Summary

  • IAM policy — Aligns the auto-created AB test execution role with the official docs, fixing a 400 "Access denied when validating evaluator" error on deploy
  • Promote stabilitypromote ab-test waits for executionStatus === 'RUNNING' before stopping, avoiding 409 errors when promote runs immediately after resume
  • DescribeLogGroups — Split into its own statement with Resource: "*" (AWS requirement)
  • E2E coverage — Added AB test reaches RUNNING status after deploy to ab-test-target-based.test.ts
  • Status test — Fixed incorrect http-gateway resourceType assertion
  • Local e2e script — Added scripts/run-e2e-local.sh

See #1120 for full details.

…1120)
* fix: add GetEvaluator permission to AB test execution role
The AB test API validates evaluator ARNs server-side using the execution
role. The auto-created role policy was missing bedrock-agentcore:GetEvaluator,
causing a 400 "Access denied when validating evaluator" error on every deploy
that included a target-based AB test with a custom evaluator.
* fix: correct status assertion in target-based AB test e2e
HTTP gateways are not surfaced as top-level resources in agentcore status —
they are only used internally to build AB test invocation URLs. The test was
asserting on resourceType 'http-gateway' which never appears; fix it to
assert on the 'ab-test' resource instead.
* fix: add invocationUrl to status resource type cast in e2e test
The resource cast was missing invocationUrl, causing a TypeScript error
when asserting the AB test's gateway invocation URL was present.
* fix: improve promote error message and add local e2e run script
- Include stdout in promote failure message so the JSON error is visible
- Add scripts/run-e2e-local.sh to replicate the GitHub Actions e2e workflow locally
* fix: retry promote stop on 409 UPDATING status
When promote is called immediately after resume, the AB test may still be
in UPDATING state and reject the STOPPED transition with a 409. Retry up
to 6 times with a 10s delay to wait for the transition to complete.
* fix: wait for AB test to leave UPDATING state before promoting
Poll getABTest until executionStatus is no longer UPDATING before
attempting to stop. The service rejects updates with 409 while a
state transition is in progress (e.g. after resume).
* fix: wait for RUNNING before stopping AB test in promote
Poll getABTest until executionStatus === 'RUNNING' before issuing the
STOPPED transition. The service 409s if the test is still UPDATING
(e.g. transitioning from a prior resume).
* fix: resolve evaluator ARNs transitively from AB test online eval configs
Previously GetEvaluator was scoped to all project evaluators. Now it's
scoped to only the evaluators referenced by the specific online eval
configs this AB test uses, handling all three cases:
- Custom evaluators: looked up from deployedResources.evaluators
- Builtin.* evaluators: ARN constructed from the builtin ID
- External ARN references: used as-is
* fix: align AB test execution role policy with official docs
Replace the hand-rolled per-resource policy with the canonical policy
from https://docs.aws.amazon.com/bedrock-agentcore/latest/devguide/ab-testing-prereqs.html:
- Trust policy: add SourceAccount + SourceArn conditions
- Permissions: single AgentCoreResources statement scoped to
arn:aws:bedrock-agentcore:*:${accountId}:* with ResourceAccount condition
- Add missing actions: GetGatewayTarget, ListGatewayTargets,
ListConfigurationBundleVersions
- CloudWatch logs scoped to account via PrincipalAccount pattern
- Remove per-evaluator/per-resource ARN tracking (no longer needed)
* test: update IAM role unit tests to match new policy structure
* fix: throw error if AB test never reaches RUNNING before promote
* test: add unit tests for promote polling logic and extract to promote-utils
Extract waitForRunningThenStop into promote-utils.ts so it can be unit
tested without pulling in React/ink. Add 4 tests covering: immediate
RUNNING, polling until RUNNING, timeout throws, and error message content.
* fix: split DescribeLogGroups into wildcard resource statement
* test: verify AB test reaches RUNNING status after deploy in both e2e suites
* fix: remove console.log statements that leaked ARNs and fix bundle ARN/version format in config-bundle e2e test
* test: remove second deploy and RUNNING check from config-bundle e2e (follow-up with real bundles)
---------
Co-authored-by: Local E2E <ci@local>
@jariy17
jariy17 requested a review from a teamMay 5, 2026 23:04
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label May 5, 2026
@agentcore-cli-automation

Copy link
Copy Markdown

Reviewed the cherry-pick. All the serious issues I'd raise have already been flagged in the existing discussion:

  • The Builtin/external evaluator ARN concern was raised on fix: correct AB test execution role IAM policy and promote stability #1120 and addressed by moving to the docs-aligned policy template.
  • The gateway scope loosening was raised and justified by the author against the AWS docs.
  • The TUI promote race condition (same updateABTest({ executionStatus: 'STOPPED' }) call at ABTestDetailScreen.tsx:341 without a waitForRunningThenStop) was already raised by @notgitika on this PR and is still awaiting a response/fix.

The cherry-pick onto preview itself looks clean — the diff matches what landed on main via #1120, and the new waitForRunningThenStop utility + tests are correct. LGTM aside from the already-flagged TUI question.

@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 5, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines43.6%9964 / 22852
🔵Statements42.89%10589 / 24686
🔵Functions40.63%1684 / 4144
🔵Branches40.28%6430 / 15962
Generated in workflow #2463 for commit f8e8cdf by the Vitest Coverage Report Action

@notgitikanotgitika 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.

LGTM

@jariy17
jariy17 merged commit 62419a6 into previewMay 6, 2026
17 checks passed
@jariy17
jariy17 deleted the fix/ab-test-role-evaluator-permission-preview branch May 6, 2026 13:13
Sign up for freeto 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

@jariy17@agentcore-cli-automation@notgitika
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix: correct AB test execution role IAM policy and promote stability - #1126

Merged
jariy17 merged 1 commit into
previewfrom
fix/ab-test-role-evaluator-permission-preview
May 6, 2026
Merged

fix: correct AB test execution role IAM policy and promote stability#1126
jariy17 merged 1 commit into
previewfrom
fix/ab-test-role-evaluator-permission-preview

Conversation

@jariy17

Copy link
Copy Markdown
Contributor

Cherry-pick of #1120 onto preview.

Summary

  • IAM policy — Aligns the auto-created AB test execution role with the official docs, fixing a 400 "Access denied when validating evaluator" error on deploy
  • Promote stabilitypromote ab-test waits for executionStatus === 'RUNNING' before stopping, avoiding 409 errors when promote runs immediately after resume
  • DescribeLogGroups — Split into its own statement with Resource: "*" (AWS requirement)
  • E2E coverage — Added AB test reaches RUNNING status after deploy to ab-test-target-based.test.ts
  • Status test — Fixed incorrect http-gateway resourceType assertion
  • Local e2e script — Added scripts/run-e2e-local.sh

See #1120 for full details.

…1120)
* fix: add GetEvaluator permission to AB test execution role
The AB test API validates evaluator ARNs server-side using the execution
role. The auto-created role policy was missing bedrock-agentcore:GetEvaluator,
causing a 400 "Access denied when validating evaluator" error on every deploy
that included a target-based AB test with a custom evaluator.
* fix: correct status assertion in target-based AB test e2e
HTTP gateways are not surfaced as top-level resources in agentcore status —
they are only used internally to build AB test invocation URLs. The test was
asserting on resourceType 'http-gateway' which never appears; fix it to
assert on the 'ab-test' resource instead.
* fix: add invocationUrl to status resource type cast in e2e test
The resource cast was missing invocationUrl, causing a TypeScript error
when asserting the AB test's gateway invocation URL was present.
* fix: improve promote error message and add local e2e run script
- Include stdout in promote failure message so the JSON error is visible
- Add scripts/run-e2e-local.sh to replicate the GitHub Actions e2e workflow locally
* fix: retry promote stop on 409 UPDATING status
When promote is called immediately after resume, the AB test may still be
in UPDATING state and reject the STOPPED transition with a 409. Retry up
to 6 times with a 10s delay to wait for the transition to complete.
* fix: wait for AB test to leave UPDATING state before promoting
Poll getABTest until executionStatus is no longer UPDATING before
attempting to stop. The service rejects updates with 409 while a
state transition is in progress (e.g. after resume).
* fix: wait for RUNNING before stopping AB test in promote
Poll getABTest until executionStatus === 'RUNNING' before issuing the
STOPPED transition. The service 409s if the test is still UPDATING
(e.g. transitioning from a prior resume).
* fix: resolve evaluator ARNs transitively from AB test online eval configs
Previously GetEvaluator was scoped to all project evaluators. Now it's
scoped to only the evaluators referenced by the specific online eval
configs this AB test uses, handling all three cases:
- Custom evaluators: looked up from deployedResources.evaluators
- Builtin.* evaluators: ARN constructed from the builtin ID
- External ARN references: used as-is
* fix: align AB test execution role policy with official docs
Replace the hand-rolled per-resource policy with the canonical policy
from https://docs.aws.amazon.com/bedrock-agentcore/latest/devguide/ab-testing-prereqs.html:
- Trust policy: add SourceAccount + SourceArn conditions
- Permissions: single AgentCoreResources statement scoped to
arn:aws:bedrock-agentcore:*:${accountId}:* with ResourceAccount condition
- Add missing actions: GetGatewayTarget, ListGatewayTargets,
ListConfigurationBundleVersions
- CloudWatch logs scoped to account via PrincipalAccount pattern
- Remove per-evaluator/per-resource ARN tracking (no longer needed)
* test: update IAM role unit tests to match new policy structure
* fix: throw error if AB test never reaches RUNNING before promote
* test: add unit tests for promote polling logic and extract to promote-utils
Extract waitForRunningThenStop into promote-utils.ts so it can be unit
tested without pulling in React/ink. Add 4 tests covering: immediate
RUNNING, polling until RUNNING, timeout throws, and error message content.
* fix: split DescribeLogGroups into wildcard resource statement
* test: verify AB test reaches RUNNING status after deploy in both e2e suites
* fix: remove console.log statements that leaked ARNs and fix bundle ARN/version format in config-bundle e2e test
* test: remove second deploy and RUNNING check from config-bundle e2e (follow-up with real bundles)
---------
Co-authored-by: Local E2E <ci@local>
@jariy17
jariy17 requested a review from a teamMay 5, 2026 23:04
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label May 5, 2026
@agentcore-cli-automation

Copy link
Copy Markdown

Reviewed the cherry-pick. All the serious issues I'd raise have already been flagged in the existing discussion:

  • The Builtin/external evaluator ARN concern was raised on fix: correct AB test execution role IAM policy and promote stability #1120 and addressed by moving to the docs-aligned policy template.
  • The gateway scope loosening was raised and justified by the author against the AWS docs.
  • The TUI promote race condition (same updateABTest({ executionStatus: 'STOPPED' }) call at ABTestDetailScreen.tsx:341 without a waitForRunningThenStop) was already raised by @notgitika on this PR and is still awaiting a response/fix.

The cherry-pick onto preview itself looks clean — the diff matches what landed on main via #1120, and the new waitForRunningThenStop utility + tests are correct. LGTM aside from the already-flagged TUI question.

@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 5, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines43.6%9964 / 22852
🔵Statements42.89%10589 / 24686
🔵Functions40.63%1684 / 4144
🔵Branches40.28%6430 / 15962
Generated in workflow #2463 for commit f8e8cdf by the Vitest Coverage Report Action

@notgitikanotgitika 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.

LGTM

@jariy17
jariy17 merged commit 62419a6 into previewMay 6, 2026
17 checks passed
@jariy17
jariy17 deleted the fix/ab-test-role-evaluator-permission-preview branch May 6, 2026 13:13
Sign up for freeto 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

@jariy17@agentcore-cli-automation@notgitika
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: correct AB test execution role IAM policy and promote stability - #1126

Merged
jariy17 merged 1 commit into
previewfrom
fix/ab-test-role-evaluator-permission-preview
May 6, 2026
Merged

fix: correct AB test execution role IAM policy and promote stability#1126
jariy17 merged 1 commit into
previewfrom
fix/ab-test-role-evaluator-permission-preview

Conversation

@jariy17

Copy link
Copy Markdown
Contributor

Cherry-pick of #1120 onto preview.

Summary

  • IAM policy — Aligns the auto-created AB test execution role with the official docs, fixing a 400 "Access denied when validating evaluator" error on deploy
  • Promote stabilitypromote ab-test waits for executionStatus === 'RUNNING' before stopping, avoiding 409 errors when promote runs immediately after resume
  • DescribeLogGroups — Split into its own statement with Resource: "*" (AWS requirement)
  • E2E coverage — Added AB test reaches RUNNING status after deploy to ab-test-target-based.test.ts
  • Status test — Fixed incorrect http-gateway resourceType assertion
  • Local e2e script — Added scripts/run-e2e-local.sh

See #1120 for full details.

…1120)
* fix: add GetEvaluator permission to AB test execution role
The AB test API validates evaluator ARNs server-side using the execution
role. The auto-created role policy was missing bedrock-agentcore:GetEvaluator,
causing a 400 "Access denied when validating evaluator" error on every deploy
that included a target-based AB test with a custom evaluator.
* fix: correct status assertion in target-based AB test e2e
HTTP gateways are not surfaced as top-level resources in agentcore status —
they are only used internally to build AB test invocation URLs. The test was
asserting on resourceType 'http-gateway' which never appears; fix it to
assert on the 'ab-test' resource instead.
* fix: add invocationUrl to status resource type cast in e2e test
The resource cast was missing invocationUrl, causing a TypeScript error
when asserting the AB test's gateway invocation URL was present.
* fix: improve promote error message and add local e2e run script
- Include stdout in promote failure message so the JSON error is visible
- Add scripts/run-e2e-local.sh to replicate the GitHub Actions e2e workflow locally
* fix: retry promote stop on 409 UPDATING status
When promote is called immediately after resume, the AB test may still be
in UPDATING state and reject the STOPPED transition with a 409. Retry up
to 6 times with a 10s delay to wait for the transition to complete.
* fix: wait for AB test to leave UPDATING state before promoting
Poll getABTest until executionStatus is no longer UPDATING before
attempting to stop. The service rejects updates with 409 while a
state transition is in progress (e.g. after resume).
* fix: wait for RUNNING before stopping AB test in promote
Poll getABTest until executionStatus === 'RUNNING' before issuing the
STOPPED transition. The service 409s if the test is still UPDATING
(e.g. transitioning from a prior resume).
* fix: resolve evaluator ARNs transitively from AB test online eval configs
Previously GetEvaluator was scoped to all project evaluators. Now it's
scoped to only the evaluators referenced by the specific online eval
configs this AB test uses, handling all three cases:
- Custom evaluators: looked up from deployedResources.evaluators
- Builtin.* evaluators: ARN constructed from the builtin ID
- External ARN references: used as-is
* fix: align AB test execution role policy with official docs
Replace the hand-rolled per-resource policy with the canonical policy
from https://docs.aws.amazon.com/bedrock-agentcore/latest/devguide/ab-testing-prereqs.html:
- Trust policy: add SourceAccount + SourceArn conditions
- Permissions: single AgentCoreResources statement scoped to
arn:aws:bedrock-agentcore:*:${accountId}:* with ResourceAccount condition
- Add missing actions: GetGatewayTarget, ListGatewayTargets,
ListConfigurationBundleVersions
- CloudWatch logs scoped to account via PrincipalAccount pattern
- Remove per-evaluator/per-resource ARN tracking (no longer needed)
* test: update IAM role unit tests to match new policy structure
* fix: throw error if AB test never reaches RUNNING before promote
* test: add unit tests for promote polling logic and extract to promote-utils
Extract waitForRunningThenStop into promote-utils.ts so it can be unit
tested without pulling in React/ink. Add 4 tests covering: immediate
RUNNING, polling until RUNNING, timeout throws, and error message content.
* fix: split DescribeLogGroups into wildcard resource statement
* test: verify AB test reaches RUNNING status after deploy in both e2e suites
* fix: remove console.log statements that leaked ARNs and fix bundle ARN/version format in config-bundle e2e test
* test: remove second deploy and RUNNING check from config-bundle e2e (follow-up with real bundles)
---------
Co-authored-by: Local E2E <ci@local>
@jariy17
jariy17 requested a review from a teamMay 5, 2026 23:04
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label May 5, 2026
@agentcore-cli-automation

Copy link
Copy Markdown

Reviewed the cherry-pick. All the serious issues I'd raise have already been flagged in the existing discussion:

  • The Builtin/external evaluator ARN concern was raised on fix: correct AB test execution role IAM policy and promote stability #1120 and addressed by moving to the docs-aligned policy template.
  • The gateway scope loosening was raised and justified by the author against the AWS docs.
  • The TUI promote race condition (same updateABTest({ executionStatus: 'STOPPED' }) call at ABTestDetailScreen.tsx:341 without a waitForRunningThenStop) was already raised by @notgitika on this PR and is still awaiting a response/fix.

The cherry-pick onto preview itself looks clean — the diff matches what landed on main via #1120, and the new waitForRunningThenStop utility + tests are correct. LGTM aside from the already-flagged TUI question.

@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 5, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines43.6%9964 / 22852
🔵Statements42.89%10589 / 24686
🔵Functions40.63%1684 / 4144
🔵Branches40.28%6430 / 15962
Generated in workflow #2463 for commit f8e8cdf by the Vitest Coverage Report Action

@notgitikanotgitika 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.

LGTM

@jariy17
jariy17 merged commit 62419a6 into previewMay 6, 2026
17 checks passed
@jariy17
jariy17 deleted the fix/ab-test-role-evaluator-permission-preview branch May 6, 2026 13:13
Sign up for freeto 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

@jariy17@agentcore-cli-automation@notgitika
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: correct AB test execution role IAM policy and promote stability - #1126

Merged
jariy17 merged 1 commit into
previewfrom
fix/ab-test-role-evaluator-permission-preview
May 6, 2026
Merged

fix: correct AB test execution role IAM policy and promote stability#1126
jariy17 merged 1 commit into
previewfrom
fix/ab-test-role-evaluator-permission-preview

Conversation

@jariy17

Copy link
Copy Markdown
Contributor

Cherry-pick of #1120 onto preview.

Summary

  • IAM policy — Aligns the auto-created AB test execution role with the official docs, fixing a 400 "Access denied when validating evaluator" error on deploy
  • Promote stabilitypromote ab-test waits for executionStatus === 'RUNNING' before stopping, avoiding 409 errors when promote runs immediately after resume
  • DescribeLogGroups — Split into its own statement with Resource: "*" (AWS requirement)
  • E2E coverage — Added AB test reaches RUNNING status after deploy to ab-test-target-based.test.ts
  • Status test — Fixed incorrect http-gateway resourceType assertion
  • Local e2e script — Added scripts/run-e2e-local.sh

See #1120 for full details.

…1120)
* fix: add GetEvaluator permission to AB test execution role
The AB test API validates evaluator ARNs server-side using the execution
role. The auto-created role policy was missing bedrock-agentcore:GetEvaluator,
causing a 400 "Access denied when validating evaluator" error on every deploy
that included a target-based AB test with a custom evaluator.
* fix: correct status assertion in target-based AB test e2e
HTTP gateways are not surfaced as top-level resources in agentcore status —
they are only used internally to build AB test invocation URLs. The test was
asserting on resourceType 'http-gateway' which never appears; fix it to
assert on the 'ab-test' resource instead.
* fix: add invocationUrl to status resource type cast in e2e test
The resource cast was missing invocationUrl, causing a TypeScript error
when asserting the AB test's gateway invocation URL was present.
* fix: improve promote error message and add local e2e run script
- Include stdout in promote failure message so the JSON error is visible
- Add scripts/run-e2e-local.sh to replicate the GitHub Actions e2e workflow locally
* fix: retry promote stop on 409 UPDATING status
When promote is called immediately after resume, the AB test may still be
in UPDATING state and reject the STOPPED transition with a 409. Retry up
to 6 times with a 10s delay to wait for the transition to complete.
* fix: wait for AB test to leave UPDATING state before promoting
Poll getABTest until executionStatus is no longer UPDATING before
attempting to stop. The service rejects updates with 409 while a
state transition is in progress (e.g. after resume).
* fix: wait for RUNNING before stopping AB test in promote
Poll getABTest until executionStatus === 'RUNNING' before issuing the
STOPPED transition. The service 409s if the test is still UPDATING
(e.g. transitioning from a prior resume).
* fix: resolve evaluator ARNs transitively from AB test online eval configs
Previously GetEvaluator was scoped to all project evaluators. Now it's
scoped to only the evaluators referenced by the specific online eval
configs this AB test uses, handling all three cases:
- Custom evaluators: looked up from deployedResources.evaluators
- Builtin.* evaluators: ARN constructed from the builtin ID
- External ARN references: used as-is
* fix: align AB test execution role policy with official docs
Replace the hand-rolled per-resource policy with the canonical policy
from https://docs.aws.amazon.com/bedrock-agentcore/latest/devguide/ab-testing-prereqs.html:
- Trust policy: add SourceAccount + SourceArn conditions
- Permissions: single AgentCoreResources statement scoped to
arn:aws:bedrock-agentcore:*:${accountId}:* with ResourceAccount condition
- Add missing actions: GetGatewayTarget, ListGatewayTargets,
ListConfigurationBundleVersions
- CloudWatch logs scoped to account via PrincipalAccount pattern
- Remove per-evaluator/per-resource ARN tracking (no longer needed)
* test: update IAM role unit tests to match new policy structure
* fix: throw error if AB test never reaches RUNNING before promote
* test: add unit tests for promote polling logic and extract to promote-utils
Extract waitForRunningThenStop into promote-utils.ts so it can be unit
tested without pulling in React/ink. Add 4 tests covering: immediate
RUNNING, polling until RUNNING, timeout throws, and error message content.
* fix: split DescribeLogGroups into wildcard resource statement
* test: verify AB test reaches RUNNING status after deploy in both e2e suites
* fix: remove console.log statements that leaked ARNs and fix bundle ARN/version format in config-bundle e2e test
* test: remove second deploy and RUNNING check from config-bundle e2e (follow-up with real bundles)
---------
Co-authored-by: Local E2E <ci@local>
@jariy17
jariy17 requested a review from a teamMay 5, 2026 23:04
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label May 5, 2026
@agentcore-cli-automation

Copy link
Copy Markdown

Reviewed the cherry-pick. All the serious issues I'd raise have already been flagged in the existing discussion:

  • The Builtin/external evaluator ARN concern was raised on fix: correct AB test execution role IAM policy and promote stability #1120 and addressed by moving to the docs-aligned policy template.
  • The gateway scope loosening was raised and justified by the author against the AWS docs.
  • The TUI promote race condition (same updateABTest({ executionStatus: 'STOPPED' }) call at ABTestDetailScreen.tsx:341 without a waitForRunningThenStop) was already raised by @notgitika on this PR and is still awaiting a response/fix.

The cherry-pick onto preview itself looks clean — the diff matches what landed on main via #1120, and the new waitForRunningThenStop utility + tests are correct. LGTM aside from the already-flagged TUI question.

@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 5, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines43.6%9964 / 22852
🔵Statements42.89%10589 / 24686
🔵Functions40.63%1684 / 4144
🔵Branches40.28%6430 / 15962
Generated in workflow #2463 for commit f8e8cdf by the Vitest Coverage Report Action

@notgitikanotgitika 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.

LGTM

@jariy17
jariy17 merged commit 62419a6 into previewMay 6, 2026
17 checks passed
@jariy17
jariy17 deleted the fix/ab-test-role-evaluator-permission-preview branch May 6, 2026 13:13
Sign up for freeto 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

@jariy17@agentcore-cli-automation@notgitika
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix: correct AB test execution role IAM policy and promote stability - #1126

Merged
jariy17 merged 1 commit into
previewfrom
fix/ab-test-role-evaluator-permission-preview
May 6, 2026
Merged

fix: correct AB test execution role IAM policy and promote stability#1126
jariy17 merged 1 commit into
previewfrom
fix/ab-test-role-evaluator-permission-preview

Conversation

@jariy17

Copy link
Copy Markdown
Contributor

Cherry-pick of #1120 onto preview.

Summary

  • IAM policy — Aligns the auto-created AB test execution role with the official docs, fixing a 400 "Access denied when validating evaluator" error on deploy
  • Promote stabilitypromote ab-test waits for executionStatus === 'RUNNING' before stopping, avoiding 409 errors when promote runs immediately after resume
  • DescribeLogGroups — Split into its own statement with Resource: "*" (AWS requirement)
  • E2E coverage — Added AB test reaches RUNNING status after deploy to ab-test-target-based.test.ts
  • Status test — Fixed incorrect http-gateway resourceType assertion
  • Local e2e script — Added scripts/run-e2e-local.sh

See #1120 for full details.

…1120)
* fix: add GetEvaluator permission to AB test execution role
The AB test API validates evaluator ARNs server-side using the execution
role. The auto-created role policy was missing bedrock-agentcore:GetEvaluator,
causing a 400 "Access denied when validating evaluator" error on every deploy
that included a target-based AB test with a custom evaluator.
* fix: correct status assertion in target-based AB test e2e
HTTP gateways are not surfaced as top-level resources in agentcore status —
they are only used internally to build AB test invocation URLs. The test was
asserting on resourceType 'http-gateway' which never appears; fix it to
assert on the 'ab-test' resource instead.
* fix: add invocationUrl to status resource type cast in e2e test
The resource cast was missing invocationUrl, causing a TypeScript error
when asserting the AB test's gateway invocation URL was present.
* fix: improve promote error message and add local e2e run script
- Include stdout in promote failure message so the JSON error is visible
- Add scripts/run-e2e-local.sh to replicate the GitHub Actions e2e workflow locally
* fix: retry promote stop on 409 UPDATING status
When promote is called immediately after resume, the AB test may still be
in UPDATING state and reject the STOPPED transition with a 409. Retry up
to 6 times with a 10s delay to wait for the transition to complete.
* fix: wait for AB test to leave UPDATING state before promoting
Poll getABTest until executionStatus is no longer UPDATING before
attempting to stop. The service rejects updates with 409 while a
state transition is in progress (e.g. after resume).
* fix: wait for RUNNING before stopping AB test in promote
Poll getABTest until executionStatus === 'RUNNING' before issuing the
STOPPED transition. The service 409s if the test is still UPDATING
(e.g. transitioning from a prior resume).
* fix: resolve evaluator ARNs transitively from AB test online eval configs
Previously GetEvaluator was scoped to all project evaluators. Now it's
scoped to only the evaluators referenced by the specific online eval
configs this AB test uses, handling all three cases:
- Custom evaluators: looked up from deployedResources.evaluators
- Builtin.* evaluators: ARN constructed from the builtin ID
- External ARN references: used as-is
* fix: align AB test execution role policy with official docs
Replace the hand-rolled per-resource policy with the canonical policy
from https://docs.aws.amazon.com/bedrock-agentcore/latest/devguide/ab-testing-prereqs.html:
- Trust policy: add SourceAccount + SourceArn conditions
- Permissions: single AgentCoreResources statement scoped to
arn:aws:bedrock-agentcore:*:${accountId}:* with ResourceAccount condition
- Add missing actions: GetGatewayTarget, ListGatewayTargets,
ListConfigurationBundleVersions
- CloudWatch logs scoped to account via PrincipalAccount pattern
- Remove per-evaluator/per-resource ARN tracking (no longer needed)
* test: update IAM role unit tests to match new policy structure
* fix: throw error if AB test never reaches RUNNING before promote
* test: add unit tests for promote polling logic and extract to promote-utils
Extract waitForRunningThenStop into promote-utils.ts so it can be unit
tested without pulling in React/ink. Add 4 tests covering: immediate
RUNNING, polling until RUNNING, timeout throws, and error message content.
* fix: split DescribeLogGroups into wildcard resource statement
* test: verify AB test reaches RUNNING status after deploy in both e2e suites
* fix: remove console.log statements that leaked ARNs and fix bundle ARN/version format in config-bundle e2e test
* test: remove second deploy and RUNNING check from config-bundle e2e (follow-up with real bundles)
---------
Co-authored-by: Local E2E <ci@local>
@jariy17
jariy17 requested a review from a teamMay 5, 2026 23:04
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label May 5, 2026
@agentcore-cli-automation

Copy link
Copy Markdown

Reviewed the cherry-pick. All the serious issues I'd raise have already been flagged in the existing discussion:

  • The Builtin/external evaluator ARN concern was raised on fix: correct AB test execution role IAM policy and promote stability #1120 and addressed by moving to the docs-aligned policy template.
  • The gateway scope loosening was raised and justified by the author against the AWS docs.
  • The TUI promote race condition (same updateABTest({ executionStatus: 'STOPPED' }) call at ABTestDetailScreen.tsx:341 without a waitForRunningThenStop) was already raised by @notgitika on this PR and is still awaiting a response/fix.

The cherry-pick onto preview itself looks clean — the diff matches what landed on main via #1120, and the new waitForRunningThenStop utility + tests are correct. LGTM aside from the already-flagged TUI question.

@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 5, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines43.6%9964 / 22852
🔵Statements42.89%10589 / 24686
🔵Functions40.63%1684 / 4144
🔵Branches40.28%6430 / 15962
Generated in workflow #2463 for commit f8e8cdf by the Vitest Coverage Report Action

@notgitikanotgitika 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.

LGTM

@jariy17
jariy17 merged commit 62419a6 into previewMay 6, 2026
17 checks passed
@jariy17
jariy17 deleted the fix/ab-test-role-evaluator-permission-preview branch May 6, 2026 13:13
Sign up for freeto 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

@jariy17@agentcore-cli-automation@notgitika