Skip to content

feat: add BYO filesystem e2e test and supporting infrastructure - #1461

Merged
padmak30 merged 2 commits into
mainfrom
feat/filesystem-e2e-test
Jun 5, 2026
Merged

feat: add BYO filesystem e2e test and supporting infrastructure#1461
padmak30 merged 2 commits into
mainfrom
feat/filesystem-e2e-test

Conversation

@padmak30

Copy link
Copy Markdown
Contributor
  • Add strands-bedrock-byo-filesystem.test.ts: e2e test for EFS and S3 Files mounts with unique file content assertion
  • Add e2e-tests/fixtures/filesystem/setup_byo_filesystem.py: one-time setup script to provision VPC, EFS, S3 Files access points in AWS
  • Extend E2EConfig with apiKeyEnvVar, requiredEnvVars, efsAccessPoints, s3AccessPoints, networkConfig, invokePrompt, invokeResponseCheck
  • Fix VPC warning in invoke action to suppress when --json is passed
  • Add filesystem env vars to e2e-tests.yml env blocks
  • Add filesystem resources fixture file

- Add strands-bedrock-byo-filesystem.test.ts: e2e test for EFS and S3
Files mounts with unique file content assertion
- Add e2e-tests/fixtures/filesystem/setup_byo_filesystem.py: one-time
setup script to provision VPC, EFS, S3 Files access points in AWS
- Extend E2EConfig with apiKeyEnvVar, requiredEnvVars, efsAccessPoints,
s3AccessPoints, networkConfig, invokePrompt, invokeResponseCheck
- Fix VPC warning in invoke action to suppress when --json is passed
- Add filesystem env vars to e2e-tests.yml env blocks
- Add filesystem resources fixture file
@padmak30
padmak30 requested a review from a teamJune 4, 2026 01:15
@github-actionsgithub-actionsBot added the size/l PR size: L label Jun 4, 2026
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 4, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.17.0.tgz

How to install

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

@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026

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

Nice work setting up filesystem e2e coverage end-to-end. A few issues to address before merging:

  1. The S3 Files IAM trust policy looks wrong — it allows elasticfilesystem.amazonaws.com to assume the role, which is the EFS service principal, not S3 Files. This will likely make the S3 Files mount unable to read the bucket.
  2. The new invokeResponseCheck is silently skipped when json.response is falsy, which weakens the assertion. The whole point of this check is to fail when the response is wrong (or missing).
  3. The test prompt only exercises the EFS mount; the S3 Files mount is configured but never read or written from at runtime, so a misconfigured S3 mount would still pass.

Inline comments below.

Comment threade2e-tests/fixtures/filesystem/setup_byo_filesystem.py
Comment threade2e-tests/e2e-helper.ts Outdated
Comment threade2e-tests/strands-bedrock-byo-filesystem.test.ts Outdated
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 4, 2026
@github-actions

github-actionsBot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines35.54%11183 / 31461
🔵Statements34.88%11890 / 34084
🔵Functions30.28%1880 / 6207
🔵Branches29.31%7134 / 24334
Generated in workflow #3488 for commit 946d9f6 by the Vitest Coverage Report Action

- Fix invokeResponseCheck to assert response is truthy before running
the check — previously silently skipped on empty/undefined response
- Extend filesystem test prompt to write and read unique tokens on both
EFS (/mnt/efs) and S3 Files (/mnt/s3) mounts, asserting both appear
in the response so a misconfigured S3 mount is caught
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels Jun 4, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026

@jariy17jariy17 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 but just one nit

modelProvider: string;
requiredEnvVar?: string;
/** Env var holding the API key — must be set for the suite to run, and its value is passed as --api-key. */
apiKeyEnvVar?: string;

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.

Why did we seperate out the apiKey into their own variable?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

requiredEnvVar was doing two different things: (1) gating the test suite, and (2) forwarding the env var's value as --api-key to the create command. These had to be separated because the filesystem test needs to gate on 4 env vars (E2E_EFS_ACCESS_POINT_ARN, E2E_S3_ACCESS_POINT_ARN, E2E_FILESYSTEM_SUBNET_ID, E2E_FILESYSTEM_SECURITY_GROUP_ID) that are not API keys — they're ARNs/IDs used in the --efs-access-point-arn etc. args. requiredEnvVars: string[] handles "gate only" for any number of vars; apiKeyEnvVar handles the "gate + pass as --api-key" case that existing tests need.

@padmak30
padmak30 merged commit dd255e3 into mainJun 5, 2026
33 of 35 checks passed
@padmak30
padmak30 deleted the feat/filesystem-e2e-test branch June 5, 2026 17:06
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/lPR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@padmak30@jariy17@agentcore-cli-automation
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
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;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
feat: add BYO filesystem e2e test and supporting infrastructure by padmak30 · Pull Request #1461 · aws/agentcore-cli · GitHub
Skip to content

feat: add BYO filesystem e2e test and supporting infrastructure - #1461

Merged
padmak30 merged 2 commits into
mainfrom
feat/filesystem-e2e-test
Jun 5, 2026
Merged

feat: add BYO filesystem e2e test and supporting infrastructure#1461
padmak30 merged 2 commits into
mainfrom
feat/filesystem-e2e-test

Conversation

@padmak30

Copy link
Copy Markdown
Contributor
  • Add strands-bedrock-byo-filesystem.test.ts: e2e test for EFS and S3 Files mounts with unique file content assertion
  • Add e2e-tests/fixtures/filesystem/setup_byo_filesystem.py: one-time setup script to provision VPC, EFS, S3 Files access points in AWS
  • Extend E2EConfig with apiKeyEnvVar, requiredEnvVars, efsAccessPoints, s3AccessPoints, networkConfig, invokePrompt, invokeResponseCheck
  • Fix VPC warning in invoke action to suppress when --json is passed
  • Add filesystem env vars to e2e-tests.yml env blocks
  • Add filesystem resources fixture file

- Add strands-bedrock-byo-filesystem.test.ts: e2e test for EFS and S3
Files mounts with unique file content assertion
- Add e2e-tests/fixtures/filesystem/setup_byo_filesystem.py: one-time
setup script to provision VPC, EFS, S3 Files access points in AWS
- Extend E2EConfig with apiKeyEnvVar, requiredEnvVars, efsAccessPoints,
s3AccessPoints, networkConfig, invokePrompt, invokeResponseCheck
- Fix VPC warning in invoke action to suppress when --json is passed
- Add filesystem env vars to e2e-tests.yml env blocks
- Add filesystem resources fixture file
@padmak30
padmak30 requested a review from a teamJune 4, 2026 01:15
@github-actionsgithub-actionsBot added the size/l PR size: L label Jun 4, 2026
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 4, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.17.0.tgz

How to install

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

@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026

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

Nice work setting up filesystem e2e coverage end-to-end. A few issues to address before merging:

  1. The S3 Files IAM trust policy looks wrong — it allows elasticfilesystem.amazonaws.com to assume the role, which is the EFS service principal, not S3 Files. This will likely make the S3 Files mount unable to read the bucket.
  2. The new invokeResponseCheck is silently skipped when json.response is falsy, which weakens the assertion. The whole point of this check is to fail when the response is wrong (or missing).
  3. The test prompt only exercises the EFS mount; the S3 Files mount is configured but never read or written from at runtime, so a misconfigured S3 mount would still pass.

Inline comments below.

Comment threade2e-tests/fixtures/filesystem/setup_byo_filesystem.py
Comment threade2e-tests/e2e-helper.ts Outdated
Comment threade2e-tests/strands-bedrock-byo-filesystem.test.ts Outdated
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 4, 2026
@github-actions

github-actionsBot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines35.54%11183 / 31461
🔵Statements34.88%11890 / 34084
🔵Functions30.28%1880 / 6207
🔵Branches29.31%7134 / 24334
Generated in workflow #3488 for commit 946d9f6 by the Vitest Coverage Report Action

- Fix invokeResponseCheck to assert response is truthy before running
the check — previously silently skipped on empty/undefined response
- Extend filesystem test prompt to write and read unique tokens on both
EFS (/mnt/efs) and S3 Files (/mnt/s3) mounts, asserting both appear
in the response so a misconfigured S3 mount is caught
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels Jun 4, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026

@jariy17jariy17 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 but just one nit

modelProvider: string;
requiredEnvVar?: string;
/** Env var holding the API key — must be set for the suite to run, and its value is passed as --api-key. */
apiKeyEnvVar?: string;

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.

Why did we seperate out the apiKey into their own variable?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

requiredEnvVar was doing two different things: (1) gating the test suite, and (2) forwarding the env var's value as --api-key to the create command. These had to be separated because the filesystem test needs to gate on 4 env vars (E2E_EFS_ACCESS_POINT_ARN, E2E_S3_ACCESS_POINT_ARN, E2E_FILESYSTEM_SUBNET_ID, E2E_FILESYSTEM_SECURITY_GROUP_ID) that are not API keys — they're ARNs/IDs used in the --efs-access-point-arn etc. args. requiredEnvVars: string[] handles "gate only" for any number of vars; apiKeyEnvVar handles the "gate + pass as --api-key" case that existing tests need.

@padmak30
padmak30 merged commit dd255e3 into mainJun 5, 2026
33 of 35 checks passed
@padmak30
padmak30 deleted the feat/filesystem-e2e-test branch June 5, 2026 17:06
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/lPR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

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

feat: add BYO filesystem e2e test and supporting infrastructure - #1461

Merged
padmak30 merged 2 commits into
mainfrom
feat/filesystem-e2e-test
Jun 5, 2026
Merged

feat: add BYO filesystem e2e test and supporting infrastructure#1461
padmak30 merged 2 commits into
mainfrom
feat/filesystem-e2e-test

Conversation

@padmak30

Copy link
Copy Markdown
Contributor
  • Add strands-bedrock-byo-filesystem.test.ts: e2e test for EFS and S3 Files mounts with unique file content assertion
  • Add e2e-tests/fixtures/filesystem/setup_byo_filesystem.py: one-time setup script to provision VPC, EFS, S3 Files access points in AWS
  • Extend E2EConfig with apiKeyEnvVar, requiredEnvVars, efsAccessPoints, s3AccessPoints, networkConfig, invokePrompt, invokeResponseCheck
  • Fix VPC warning in invoke action to suppress when --json is passed
  • Add filesystem env vars to e2e-tests.yml env blocks
  • Add filesystem resources fixture file

- Add strands-bedrock-byo-filesystem.test.ts: e2e test for EFS and S3
Files mounts with unique file content assertion
- Add e2e-tests/fixtures/filesystem/setup_byo_filesystem.py: one-time
setup script to provision VPC, EFS, S3 Files access points in AWS
- Extend E2EConfig with apiKeyEnvVar, requiredEnvVars, efsAccessPoints,
s3AccessPoints, networkConfig, invokePrompt, invokeResponseCheck
- Fix VPC warning in invoke action to suppress when --json is passed
- Add filesystem env vars to e2e-tests.yml env blocks
- Add filesystem resources fixture file
@padmak30
padmak30 requested a review from a teamJune 4, 2026 01:15
@github-actionsgithub-actionsBot added the size/l PR size: L label Jun 4, 2026
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 4, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.17.0.tgz

How to install

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

@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026

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

Nice work setting up filesystem e2e coverage end-to-end. A few issues to address before merging:

  1. The S3 Files IAM trust policy looks wrong — it allows elasticfilesystem.amazonaws.com to assume the role, which is the EFS service principal, not S3 Files. This will likely make the S3 Files mount unable to read the bucket.
  2. The new invokeResponseCheck is silently skipped when json.response is falsy, which weakens the assertion. The whole point of this check is to fail when the response is wrong (or missing).
  3. The test prompt only exercises the EFS mount; the S3 Files mount is configured but never read or written from at runtime, so a misconfigured S3 mount would still pass.

Inline comments below.

Comment threade2e-tests/fixtures/filesystem/setup_byo_filesystem.py
Comment threade2e-tests/e2e-helper.ts Outdated
Comment threade2e-tests/strands-bedrock-byo-filesystem.test.ts Outdated
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 4, 2026
@github-actions

github-actionsBot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines35.54%11183 / 31461
🔵Statements34.88%11890 / 34084
🔵Functions30.28%1880 / 6207
🔵Branches29.31%7134 / 24334
Generated in workflow #3488 for commit 946d9f6 by the Vitest Coverage Report Action

- Fix invokeResponseCheck to assert response is truthy before running
the check — previously silently skipped on empty/undefined response
- Extend filesystem test prompt to write and read unique tokens on both
EFS (/mnt/efs) and S3 Files (/mnt/s3) mounts, asserting both appear
in the response so a misconfigured S3 mount is caught
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels Jun 4, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026

@jariy17jariy17 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 but just one nit

modelProvider: string;
requiredEnvVar?: string;
/** Env var holding the API key — must be set for the suite to run, and its value is passed as --api-key. */
apiKeyEnvVar?: string;

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.

Why did we seperate out the apiKey into their own variable?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

requiredEnvVar was doing two different things: (1) gating the test suite, and (2) forwarding the env var's value as --api-key to the create command. These had to be separated because the filesystem test needs to gate on 4 env vars (E2E_EFS_ACCESS_POINT_ARN, E2E_S3_ACCESS_POINT_ARN, E2E_FILESYSTEM_SUBNET_ID, E2E_FILESYSTEM_SECURITY_GROUP_ID) that are not API keys — they're ARNs/IDs used in the --efs-access-point-arn etc. args. requiredEnvVars: string[] handles "gate only" for any number of vars; apiKeyEnvVar handles the "gate + pass as --api-key" case that existing tests need.

@padmak30
padmak30 merged commit dd255e3 into mainJun 5, 2026
33 of 35 checks passed
@padmak30
padmak30 deleted the feat/filesystem-e2e-test branch June 5, 2026 17:06
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/lPR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

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

feat: add BYO filesystem e2e test and supporting infrastructure - #1461

Merged
padmak30 merged 2 commits into
mainfrom
feat/filesystem-e2e-test
Jun 5, 2026
Merged

feat: add BYO filesystem e2e test and supporting infrastructure#1461
padmak30 merged 2 commits into
mainfrom
feat/filesystem-e2e-test

Conversation

@padmak30

Copy link
Copy Markdown
Contributor
  • Add strands-bedrock-byo-filesystem.test.ts: e2e test for EFS and S3 Files mounts with unique file content assertion
  • Add e2e-tests/fixtures/filesystem/setup_byo_filesystem.py: one-time setup script to provision VPC, EFS, S3 Files access points in AWS
  • Extend E2EConfig with apiKeyEnvVar, requiredEnvVars, efsAccessPoints, s3AccessPoints, networkConfig, invokePrompt, invokeResponseCheck
  • Fix VPC warning in invoke action to suppress when --json is passed
  • Add filesystem env vars to e2e-tests.yml env blocks
  • Add filesystem resources fixture file

- Add strands-bedrock-byo-filesystem.test.ts: e2e test for EFS and S3
Files mounts with unique file content assertion
- Add e2e-tests/fixtures/filesystem/setup_byo_filesystem.py: one-time
setup script to provision VPC, EFS, S3 Files access points in AWS
- Extend E2EConfig with apiKeyEnvVar, requiredEnvVars, efsAccessPoints,
s3AccessPoints, networkConfig, invokePrompt, invokeResponseCheck
- Fix VPC warning in invoke action to suppress when --json is passed
- Add filesystem env vars to e2e-tests.yml env blocks
- Add filesystem resources fixture file
@padmak30
padmak30 requested a review from a teamJune 4, 2026 01:15
@github-actionsgithub-actionsBot added the size/l PR size: L label Jun 4, 2026
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 4, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.17.0.tgz

How to install

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

@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026

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

Nice work setting up filesystem e2e coverage end-to-end. A few issues to address before merging:

  1. The S3 Files IAM trust policy looks wrong — it allows elasticfilesystem.amazonaws.com to assume the role, which is the EFS service principal, not S3 Files. This will likely make the S3 Files mount unable to read the bucket.
  2. The new invokeResponseCheck is silently skipped when json.response is falsy, which weakens the assertion. The whole point of this check is to fail when the response is wrong (or missing).
  3. The test prompt only exercises the EFS mount; the S3 Files mount is configured but never read or written from at runtime, so a misconfigured S3 mount would still pass.

Inline comments below.

Comment threade2e-tests/fixtures/filesystem/setup_byo_filesystem.py
Comment threade2e-tests/e2e-helper.ts Outdated
Comment threade2e-tests/strands-bedrock-byo-filesystem.test.ts Outdated
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 4, 2026
@github-actions

github-actionsBot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines35.54%11183 / 31461
🔵Statements34.88%11890 / 34084
🔵Functions30.28%1880 / 6207
🔵Branches29.31%7134 / 24334
Generated in workflow #3488 for commit 946d9f6 by the Vitest Coverage Report Action

- Fix invokeResponseCheck to assert response is truthy before running
the check — previously silently skipped on empty/undefined response
- Extend filesystem test prompt to write and read unique tokens on both
EFS (/mnt/efs) and S3 Files (/mnt/s3) mounts, asserting both appear
in the response so a misconfigured S3 mount is caught
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels Jun 4, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026

@jariy17jariy17 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 but just one nit

modelProvider: string;
requiredEnvVar?: string;
/** Env var holding the API key — must be set for the suite to run, and its value is passed as --api-key. */
apiKeyEnvVar?: string;

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.

Why did we seperate out the apiKey into their own variable?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

requiredEnvVar was doing two different things: (1) gating the test suite, and (2) forwarding the env var's value as --api-key to the create command. These had to be separated because the filesystem test needs to gate on 4 env vars (E2E_EFS_ACCESS_POINT_ARN, E2E_S3_ACCESS_POINT_ARN, E2E_FILESYSTEM_SUBNET_ID, E2E_FILESYSTEM_SECURITY_GROUP_ID) that are not API keys — they're ARNs/IDs used in the --efs-access-point-arn etc. args. requiredEnvVars: string[] handles "gate only" for any number of vars; apiKeyEnvVar handles the "gate + pass as --api-key" case that existing tests need.

@padmak30
padmak30 merged commit dd255e3 into mainJun 5, 2026
33 of 35 checks passed
@padmak30
padmak30 deleted the feat/filesystem-e2e-test branch June 5, 2026 17:06
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/lPR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

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

feat: add BYO filesystem e2e test and supporting infrastructure - #1461

Merged
padmak30 merged 2 commits into
mainfrom
feat/filesystem-e2e-test
Jun 5, 2026
Merged

feat: add BYO filesystem e2e test and supporting infrastructure#1461
padmak30 merged 2 commits into
mainfrom
feat/filesystem-e2e-test

Conversation

@padmak30

Copy link
Copy Markdown
Contributor
  • Add strands-bedrock-byo-filesystem.test.ts: e2e test for EFS and S3 Files mounts with unique file content assertion
  • Add e2e-tests/fixtures/filesystem/setup_byo_filesystem.py: one-time setup script to provision VPC, EFS, S3 Files access points in AWS
  • Extend E2EConfig with apiKeyEnvVar, requiredEnvVars, efsAccessPoints, s3AccessPoints, networkConfig, invokePrompt, invokeResponseCheck
  • Fix VPC warning in invoke action to suppress when --json is passed
  • Add filesystem env vars to e2e-tests.yml env blocks
  • Add filesystem resources fixture file

- Add strands-bedrock-byo-filesystem.test.ts: e2e test for EFS and S3
Files mounts with unique file content assertion
- Add e2e-tests/fixtures/filesystem/setup_byo_filesystem.py: one-time
setup script to provision VPC, EFS, S3 Files access points in AWS
- Extend E2EConfig with apiKeyEnvVar, requiredEnvVars, efsAccessPoints,
s3AccessPoints, networkConfig, invokePrompt, invokeResponseCheck
- Fix VPC warning in invoke action to suppress when --json is passed
- Add filesystem env vars to e2e-tests.yml env blocks
- Add filesystem resources fixture file
@padmak30
padmak30 requested a review from a teamJune 4, 2026 01:15
@github-actionsgithub-actionsBot added the size/l PR size: L label Jun 4, 2026
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 4, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.17.0.tgz

How to install

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

@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026

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

Nice work setting up filesystem e2e coverage end-to-end. A few issues to address before merging:

  1. The S3 Files IAM trust policy looks wrong — it allows elasticfilesystem.amazonaws.com to assume the role, which is the EFS service principal, not S3 Files. This will likely make the S3 Files mount unable to read the bucket.
  2. The new invokeResponseCheck is silently skipped when json.response is falsy, which weakens the assertion. The whole point of this check is to fail when the response is wrong (or missing).
  3. The test prompt only exercises the EFS mount; the S3 Files mount is configured but never read or written from at runtime, so a misconfigured S3 mount would still pass.

Inline comments below.

Comment threade2e-tests/fixtures/filesystem/setup_byo_filesystem.py
Comment threade2e-tests/e2e-helper.ts Outdated
Comment threade2e-tests/strands-bedrock-byo-filesystem.test.ts Outdated
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 4, 2026
@github-actions

github-actionsBot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines35.54%11183 / 31461
🔵Statements34.88%11890 / 34084
🔵Functions30.28%1880 / 6207
🔵Branches29.31%7134 / 24334
Generated in workflow #3488 for commit 946d9f6 by the Vitest Coverage Report Action

- Fix invokeResponseCheck to assert response is truthy before running
the check — previously silently skipped on empty/undefined response
- Extend filesystem test prompt to write and read unique tokens on both
EFS (/mnt/efs) and S3 Files (/mnt/s3) mounts, asserting both appear
in the response so a misconfigured S3 mount is caught
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels Jun 4, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026

@jariy17jariy17 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 but just one nit

modelProvider: string;
requiredEnvVar?: string;
/** Env var holding the API key — must be set for the suite to run, and its value is passed as --api-key. */
apiKeyEnvVar?: string;

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.

Why did we seperate out the apiKey into their own variable?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

requiredEnvVar was doing two different things: (1) gating the test suite, and (2) forwarding the env var's value as --api-key to the create command. These had to be separated because the filesystem test needs to gate on 4 env vars (E2E_EFS_ACCESS_POINT_ARN, E2E_S3_ACCESS_POINT_ARN, E2E_FILESYSTEM_SUBNET_ID, E2E_FILESYSTEM_SECURITY_GROUP_ID) that are not API keys — they're ARNs/IDs used in the --efs-access-point-arn etc. args. requiredEnvVars: string[] handles "gate only" for any number of vars; apiKeyEnvVar handles the "gate + pass as --api-key" case that existing tests need.

@padmak30
padmak30 merged commit dd255e3 into mainJun 5, 2026
33 of 35 checks passed
@padmak30
padmak30 deleted the feat/filesystem-e2e-test branch June 5, 2026 17:06
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/lPR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

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

feat: add BYO filesystem e2e test and supporting infrastructure - #1461

Merged
padmak30 merged 2 commits into
mainfrom
feat/filesystem-e2e-test
Jun 5, 2026
Merged

feat: add BYO filesystem e2e test and supporting infrastructure#1461
padmak30 merged 2 commits into
mainfrom
feat/filesystem-e2e-test

Conversation

@padmak30

Copy link
Copy Markdown
Contributor
  • Add strands-bedrock-byo-filesystem.test.ts: e2e test for EFS and S3 Files mounts with unique file content assertion
  • Add e2e-tests/fixtures/filesystem/setup_byo_filesystem.py: one-time setup script to provision VPC, EFS, S3 Files access points in AWS
  • Extend E2EConfig with apiKeyEnvVar, requiredEnvVars, efsAccessPoints, s3AccessPoints, networkConfig, invokePrompt, invokeResponseCheck
  • Fix VPC warning in invoke action to suppress when --json is passed
  • Add filesystem env vars to e2e-tests.yml env blocks
  • Add filesystem resources fixture file

- Add strands-bedrock-byo-filesystem.test.ts: e2e test for EFS and S3
Files mounts with unique file content assertion
- Add e2e-tests/fixtures/filesystem/setup_byo_filesystem.py: one-time
setup script to provision VPC, EFS, S3 Files access points in AWS
- Extend E2EConfig with apiKeyEnvVar, requiredEnvVars, efsAccessPoints,
s3AccessPoints, networkConfig, invokePrompt, invokeResponseCheck
- Fix VPC warning in invoke action to suppress when --json is passed
- Add filesystem env vars to e2e-tests.yml env blocks
- Add filesystem resources fixture file
@padmak30
padmak30 requested a review from a teamJune 4, 2026 01:15
@github-actionsgithub-actionsBot added the size/l PR size: L label Jun 4, 2026
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 4, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.17.0.tgz

How to install

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

@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026

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

Nice work setting up filesystem e2e coverage end-to-end. A few issues to address before merging:

  1. The S3 Files IAM trust policy looks wrong — it allows elasticfilesystem.amazonaws.com to assume the role, which is the EFS service principal, not S3 Files. This will likely make the S3 Files mount unable to read the bucket.
  2. The new invokeResponseCheck is silently skipped when json.response is falsy, which weakens the assertion. The whole point of this check is to fail when the response is wrong (or missing).
  3. The test prompt only exercises the EFS mount; the S3 Files mount is configured but never read or written from at runtime, so a misconfigured S3 mount would still pass.

Inline comments below.

Comment threade2e-tests/fixtures/filesystem/setup_byo_filesystem.py
Comment threade2e-tests/e2e-helper.ts Outdated
Comment threade2e-tests/strands-bedrock-byo-filesystem.test.ts Outdated
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 4, 2026
@github-actions

github-actionsBot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines35.54%11183 / 31461
🔵Statements34.88%11890 / 34084
🔵Functions30.28%1880 / 6207
🔵Branches29.31%7134 / 24334
Generated in workflow #3488 for commit 946d9f6 by the Vitest Coverage Report Action

- Fix invokeResponseCheck to assert response is truthy before running
the check — previously silently skipped on empty/undefined response
- Extend filesystem test prompt to write and read unique tokens on both
EFS (/mnt/efs) and S3 Files (/mnt/s3) mounts, asserting both appear
in the response so a misconfigured S3 mount is caught
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels Jun 4, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026

@jariy17jariy17 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 but just one nit

modelProvider: string;
requiredEnvVar?: string;
/** Env var holding the API key — must be set for the suite to run, and its value is passed as --api-key. */
apiKeyEnvVar?: string;

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.

Why did we seperate out the apiKey into their own variable?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

requiredEnvVar was doing two different things: (1) gating the test suite, and (2) forwarding the env var's value as --api-key to the create command. These had to be separated because the filesystem test needs to gate on 4 env vars (E2E_EFS_ACCESS_POINT_ARN, E2E_S3_ACCESS_POINT_ARN, E2E_FILESYSTEM_SUBNET_ID, E2E_FILESYSTEM_SECURITY_GROUP_ID) that are not API keys — they're ARNs/IDs used in the --efs-access-point-arn etc. args. requiredEnvVars: string[] handles "gate only" for any number of vars; apiKeyEnvVar handles the "gate + pass as --api-key" case that existing tests need.

@padmak30
padmak30 merged commit dd255e3 into mainJun 5, 2026
33 of 35 checks passed
@padmak30
padmak30 deleted the feat/filesystem-e2e-test branch June 5, 2026 17:06
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/lPR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@padmak30@jariy17@agentcore-cli-automation
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); feat: add BYO filesystem e2e test and supporting infrastructure by padmak30 · Pull Request #1461 · aws/agentcore-cli · GitHub
Skip to content

feat: add BYO filesystem e2e test and supporting infrastructure - #1461

Merged
padmak30 merged 2 commits into
mainfrom
feat/filesystem-e2e-test
Jun 5, 2026
Merged

feat: add BYO filesystem e2e test and supporting infrastructure#1461
padmak30 merged 2 commits into
mainfrom
feat/filesystem-e2e-test

Conversation

@padmak30

Copy link
Copy Markdown
Contributor
  • Add strands-bedrock-byo-filesystem.test.ts: e2e test for EFS and S3 Files mounts with unique file content assertion
  • Add e2e-tests/fixtures/filesystem/setup_byo_filesystem.py: one-time setup script to provision VPC, EFS, S3 Files access points in AWS
  • Extend E2EConfig with apiKeyEnvVar, requiredEnvVars, efsAccessPoints, s3AccessPoints, networkConfig, invokePrompt, invokeResponseCheck
  • Fix VPC warning in invoke action to suppress when --json is passed
  • Add filesystem env vars to e2e-tests.yml env blocks
  • Add filesystem resources fixture file

- Add strands-bedrock-byo-filesystem.test.ts: e2e test for EFS and S3
Files mounts with unique file content assertion
- Add e2e-tests/fixtures/filesystem/setup_byo_filesystem.py: one-time
setup script to provision VPC, EFS, S3 Files access points in AWS
- Extend E2EConfig with apiKeyEnvVar, requiredEnvVars, efsAccessPoints,
s3AccessPoints, networkConfig, invokePrompt, invokeResponseCheck
- Fix VPC warning in invoke action to suppress when --json is passed
- Add filesystem env vars to e2e-tests.yml env blocks
- Add filesystem resources fixture file
@padmak30
padmak30 requested a review from a teamJune 4, 2026 01:15
@github-actionsgithub-actionsBot added the size/l PR size: L label Jun 4, 2026
@github-actionsgithub-actionsBot added the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 4, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.17.0.tgz

How to install

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

@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026

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

Nice work setting up filesystem e2e coverage end-to-end. A few issues to address before merging:

  1. The S3 Files IAM trust policy looks wrong — it allows elasticfilesystem.amazonaws.com to assume the role, which is the EFS service principal, not S3 Files. This will likely make the S3 Files mount unable to read the bucket.
  2. The new invokeResponseCheck is silently skipped when json.response is falsy, which weakens the assertion. The whole point of this check is to fail when the response is wrong (or missing).
  3. The test prompt only exercises the EFS mount; the S3 Files mount is configured but never read or written from at runtime, so a misconfigured S3 mount would still pass.

Inline comments below.

Comment threade2e-tests/fixtures/filesystem/setup_byo_filesystem.py
Comment threade2e-tests/e2e-helper.ts Outdated
Comment threade2e-tests/strands-bedrock-byo-filesystem.test.ts Outdated
@github-actionsgithub-actionsBot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Jun 4, 2026
@github-actions

github-actionsBot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

StatusCategoryPercentageCovered / Total
🔵Lines35.54%11183 / 31461
🔵Statements34.88%11890 / 34084
🔵Functions30.28%1880 / 6207
🔵Branches29.31%7134 / 24334
Generated in workflow #3488 for commit 946d9f6 by the Vitest Coverage Report Action

- Fix invokeResponseCheck to assert response is truthy before running
the check — previously silently skipped on empty/undefined response
- Extend filesystem test prompt to write and read unique tokens on both
EFS (/mnt/efs) and S3 Files (/mnt/s3) mounts, asserting both appear
in the response so a misconfigured S3 mount is caught
@github-actionsgithub-actionsBot added size/l PR size: L and removed size/l PR size: L labels Jun 4, 2026
@agentcore-devx-automationagentcore-devx-automationBot added the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

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

@agentcore-devx-automationagentcore-devx-automationBot removed the claude-security-reviewing Claude Code /security-review in progress label Jun 4, 2026

@jariy17jariy17 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 but just one nit

modelProvider: string;
requiredEnvVar?: string;
/** Env var holding the API key — must be set for the suite to run, and its value is passed as --api-key. */
apiKeyEnvVar?: string;

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.

Why did we seperate out the apiKey into their own variable?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

requiredEnvVar was doing two different things: (1) gating the test suite, and (2) forwarding the env var's value as --api-key to the create command. These had to be separated because the filesystem test needs to gate on 4 env vars (E2E_EFS_ACCESS_POINT_ARN, E2E_S3_ACCESS_POINT_ARN, E2E_FILESYSTEM_SUBNET_ID, E2E_FILESYSTEM_SECURITY_GROUP_ID) that are not API keys — they're ARNs/IDs used in the --efs-access-point-arn etc. args. requiredEnvVars: string[] handles "gate only" for any number of vars; apiKeyEnvVar handles the "gate + pass as --api-key" case that existing tests need.

@padmak30
padmak30 merged commit dd255e3 into mainJun 5, 2026
33 of 35 checks passed
@padmak30
padmak30 deleted the feat/filesystem-e2e-test branch June 5, 2026 17:06
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/lPR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@padmak30@jariy17@agentcore-cli-automation