feat: add Lambda interceptors for AgentCore gateways#1330
Open
aidandaly24 wants to merge 10 commits into
Open
Conversation
Contributor
Package TarballHow to installgh release download pr-1330-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.17.0.tgz |
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
a2daa82 to
dc2fe62
Compare
Contributor
|
Claude Security Review: the review run failed before completing. See the run for details. |
Contributor
|
Claude Security Review: the review run failed before completing. See the run for details. |
Contributor
|
Claude Security Review: the review run failed before completing. See the run for details. |
Adds end-to-end support for Lambda interceptors at gateway REQUEST and RESPONSE points: a new InterceptorPrimitive, three vended templates (pass-through, jwt-scope-authorizer, tools-list-filter) for both Python and Node.js, deploy-side preflight, and `logs interceptor` / `invoke interceptor` verbs. Primitive surface ----------------- - `agentcore add interceptor --name --gateway --interception-points --template --runtime [--timeout] [--no-pass-request-headers] [--additional-policies]` for managed mode - External mode via direct `agentcore.json` edit referencing a customer-provided unqualified Lambda ARN - `agentcore remove interceptor --name` reconciles deploy state on next `agentcore deploy` - `agentcore logs interceptor --name [--follow|--since|--until]` tails CloudWatch for managed interceptors; external interceptors short- circuit to a remediation message pointing at `aws logs tail` - `agentcore invoke interceptor --name [--payload]` invokes the underlying Lambda for managed interceptors with the same external- mode remediation message Schema ------ - `interceptors` array on `AgentCoreProjectSpec` with cross-field superRefine for cardinality (max 1 per point per gateway, max 2 per gateway) and gateway-name reference checks - Lambda ARN regex tightened to reject version/alias qualifiers - `passRequestHeaders`, `entrypoint`, `timeoutSeconds`, `runtime` are optional with consumption-time defaults so user-edited `agentcore.json` stays sparse across CLI round-trips Deploy ------ - `validateInterceptors()` checks managed-mode codeLocation existence and emits a masked cross-account WARN for external mode (with full `aws lambda add-permission` remediation snippet); also wired into `agentcore validate` so the pre-check works without invoking deploy - `validateGatewayTargetLambdas()` does a best-effort `lambda:GetFunction` per `lambda-function-arn` target and WARNs on any failure (NotFound, AccessDenied, throttle) so a typo'd ARN surfaces before CFN ROLLBACK - Account IDs go through `maskAccountId()` for all user-visible warning output (PII masking) Templates --------- - `pass-through`, `jwt-scope-authorizer`, `tools-list-filter` for Python (`pyproject.toml` + hatchling) and Node.js (`index.mjs` + `package.json`) - `PackageName` Handlebars var for NPM-safe lowercase package naming Telemetry --------- - `has_cross_account_warning` boolean on the InterceptorPrimitive command-run telemetry schema
ink (via cli-cursor) writes \x1b[?25h to its render stream on App unmount. The cli-cursor isTTY check was passing because ink's stdout wrapper inherits isTTY from the underlying stream object, even when stdout is piped or redirected. The trailing escape corrupted machine-parseable output for every CLI invocation that captured stdout — most visibly --json (broke JSON parsing) but also any pipe/redirect workflow. Found by the round-2 bug bash (E6) when an agent tried to `json.load()` the output of `agentcore add interceptor --json` and got "Extra data" — the bytes after the JSON were the cursor-show escape. Fix: at the CLI entry, wrap process.stdout and process.stderr writes to strip \x1b[?25[hl] (cursor show / hide) when the stream is NOT a TTY. Real TTYs are untouched so interactive terminal behavior — cursor hiding during spinners, restoring on exit — keeps working. Verified live with the bundled CLI: - agentcore --version: ends with newline only, no escape - agentcore validate: ends with newline only - agentcore add gateway --json: produces valid JSON, json.load() succeeds
The 'Limitations / out of scope (P0)' section captured deferred features for internal review context, not customer-facing guidance. Move it to the PR description so reviewers still see what was deliberately excluded.
Three fixes for CI failures on the open PR: - schemas/agentcore.schema.v1.json: revert to origin/main. The CI gate rejects any change to this file because it is regenerated and released automatically — my rebase had carried my staged version. - .prettierignore: add `src/assets/**/*.mjs`. The new Node interceptor templates ship as raw .mjs that's vended to users; we shouldn't reformat them via prettier on the way in. - npm-shrinkwrap.json: revert to origin/main. My local `npm install` during the rebase produced a different ordering than what's on upstream — keeping upstream's authoritative version avoids drift.
Four unit tests asserted against pre-FIX-BATCH-3 behavior. Bringing them in line with the shipped code: 1. logs/__tests__/interceptor.test.ts — error message no longer mentions the internal field name `interceptorFunctionName`. Assert against the new "has no Lambda function" copy that the user actually sees. 2. shared/__tests__/interceptor-mode-check.test.ts — error message no longer leaks `deployed-state.json`. Assert against the new "is not deployed" copy. 3. schema/.../interceptor.test.ts — schema fields are .optional() and defaults are applied at consumption time (not parse time), so a sparse parse leaves them undefined rather than filling them in. 4. operations/deploy/__tests__/preflight.test.ts — the new validateGatewayTargetLambdas() preflight imports getCredentialProvider from aws/account.js. Stub it in the existing vi.mock so existing fixtures (which have no lambda-function-arn targets) keep working.
The vended CDK project at src/assets/cdk/ runs against whichever @aws/agentcore-cdk version the user installs from npm. Until the CDK PR merges and publishes a new alpha, the registry version doesn't declare `interceptors` on the project spec type — so the e2e job fails with TS2353 when building the vended cdk/ project. Drop the explicit `interceptors: []` from the test object literal. Zod's default fills in `[]` at parse time when the new CDK ships, and the field's absence is harmless in the meantime. The synth assertion itself doesn't care about interceptors at all. Snapshot updated to match.
Mirror the CDK schema change: AgentCoreProjectSpecSchema.interceptors is now `.optional()` (matching the `datasets` convention) instead of `.default([])`, so an agentcore.json that doesn't use interceptors stays sparse across CLI round-trips. Consumer follow-through for the now-optional field: - InterceptorPrimitive: guard every project.interceptors access with `??= []` (mutating paths) or `?? []` (read-only paths). - AddScreen: add 'interceptor' to the AddResourceType union (the BASE_ADD_RESOURCES entry referenced a union member that the rebased upstream type definition didn't include). The schema-level superRefine cross-field checks (unknown gateway, max-2 cardinality, duplicate names) already guard with `?? []`. All 70 interceptor unit tests pass; tsc/lint/format clean.
Three test files unrelated to interceptors were swept into the feature commit by an editor autofix: - import-gateway-targets.test.ts (resolved during rebase by taking main's aws#1437 rewrite) - import-gateway-spec.test.ts (restored: main keeps the eslint-disable guarding the (gw as any) cast; dropping it would fail lint) - resolve-ui-dist-dir.test.ts (restored: main keeps the split node:fs imports) None relate to Lambda interceptors; reverting to match origin/main.
c0b9ce8 to
700bc32
Compare
The lockfile was regenerated during rebase to add @aws-sdk/client-lambda alongside main's new @aws-sdk/client-efs / client-s3files deps; npm's output formatting differs from prettier, failing the format CI check. No dependency changes — formatting only.
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
…tional-policies Completes the interceptor feature surface so it matches every other GA primitive — no more TUI dead-ends. Add wizard (src/cli/tui/screens/interceptor/): - Full interactive add flow: name → gateway → interception points → mode (managed | external) → [managed: template → runtime → advanced] → confirm. - Advanced settings page (multi-select, mirrors agent BYO computeByoSteps): Lambda timeout, additional IAM policies, pass request headers — each injects its sub-step only when selected; unselected use defaults. - computeInterceptorSteps() is a pure, unit-tested step machine; honors the dynamic-step closure gotcha (explicit setStep on mode/advanced change). - Interception points persist in canonical REQUEST,RESPONSE order regardless of toggle order, for diff-stable agentcore.json. - lambda-arn input validates against the schema pattern + 170-char cap. Remove wizard: - RemoveInterceptorScreen + RemoveFlow select-interceptor path mirror the evaluator flow: select → previewRemove (shows dir + JSON diff) → confirm. - Interceptor is now a first-class entry in both the add and remove pickers. status visibility: - agentcore status renders an Interceptors section (deployed / local-only / pending-removal) with mode + interception points, via the generic diffResourceSet over mcp.interceptors. --additional-policies: - Wired the flag end to end in the add command (schema + CDK already supported it); comma-parsed, rejected in external mode, written sparsely (empty/whitespace-only input omits the field). mask.ts: corrected the stale 'Used by' comment to list only real call sites. Verified via tui-harness drive-through (add + remove end to end, on-disk agentcore.json + scaffolding asserted) plus typecheck, lint, and 4622/4623 unit tests (1 pre-existing unrelated LogsScreen timing flake that passes in isolation).
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds end-to-end support for Lambda interceptors at gateway REQUEST and RESPONSE points: a new
InterceptorPrimitive, three vended templates (pass-through,jwt-scope-authorizer,tools-list-filter) for both Python and Node.js, deploy-side preflight, andlogs interceptor/invoke interceptorverbs.Primitive surface
Schema
Deploy
Templates
Bug fixes
Out of scope (P0)
The following were deliberately excluded from this PR. None are bugs — flagged here so reviewers don't ask "where is X":
Related Issue
Closes #
Documentation PR
Type of Change
Testing
Additionally:
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.