From 69743defce50e917c02668b09e3636a5d686418e Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Tue, 23 Jun 2026 17:54:11 +0000 Subject: [PATCH] feat: ungate AWS skills, managed memory, and read-only harness Version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The NY-Summit AgentCore Harness CFN type is now public in all regions, so the harness summit-preview features can be ungated. This removes the ENABLE_GATED_FEATURES gate at the in-scope call sites only — knowledge-base, gateway passthrough, and config-bundle branch stay gated (they were not part of this ungating). Ungated: - AWS Skills: --aws-skills source on `add skill` and the TUI skill-source picker (no longer "Coming soon"). - Managed memory: the managed/existing/disabled memory-mode union and the managed-tuning flags. The legacy "No persistent memory" enabled/disabled TUI screen is removed; the auto-created `${name}Memory` sibling is gone. "No memory" (--no-memory / --memory-mode disabled) now writes { mode: 'disabled' }, which maps to CFN Memory: { Disabled: {} } — a true opt-out instead of silently getting a service-auto-created memory. - Harness Version: `status` and the deploy drift note show the config version unconditionally. Fixes found while testing (in-scope): - Reject --no-memory combined with managed-only flags (--memory-strategies / --memory-event-expiry-days / --memory-encryption-key-arn) instead of silently dropping them. - TUI existing-memory retrieval tuning (messages count / topK / relevance) was silently dropped; the wizard setters now write into the memory union and the add/create flows share one toMemoryAddOptions() translation helper. - `deploy --json` no longer emits the managed-memory heads-up notice to stdout (it corrupted the JSON output); the notice is suppressed under --json and still recorded in the deploy log. Tests: ungated and corrected the integ memory-mode coverage (removed the legacy auto-memory assertions); added unit coverage for memory-mode resolution and the TUI translation helper; added harness-managed-memory and harness-aws-skills e2e suites (deploy -> invoke -> memory round-trip / skills-loaded -> teardown), verified against a real account. 5425 unit tests pass; typecheck + lint clean. Depends on aws/agentcore-l3-cdk-constructs#289. --- e2e-tests/harness-aws-skills.test.ts | 13 ++ e2e-tests/harness-e2e-helper.ts | 131 +++++++++++++++++- e2e-tests/harness-managed-memory.test.ts | 8 ++ integ-tests/add-remove-harness.test.ts | 88 ++++++++---- .../harness-privatelink-guard.test.ts | 64 ++++----- src/cli/commands/add/skill-action.ts | 16 +-- src/cli/commands/add/skill-command.ts | 3 +- src/cli/commands/add/validate.ts | 82 +++++------ src/cli/commands/deploy/actions.ts | 17 +-- src/cli/commands/deploy/command.tsx | 24 ++-- .../commands/status/__tests__/action.test.ts | 30 +--- src/cli/commands/status/action.ts | 11 +- .../__tests__/managed-memory-notice.test.ts | 25 +--- .../deploy/managed-memory-notice.ts | 16 ++- src/cli/primitives/HarnessPrimitive.ts | 105 +++----------- .../HarnessPrimitive.add.memory.test.ts | 91 ++++++++++++ src/cli/tui/screens/create/useCreateFlow.ts | 10 +- .../tui/screens/harness/AddHarnessFlow.tsx | 30 +--- .../tui/screens/harness/AddHarnessScreen.tsx | 60 ++------ .../harness/__tests__/memory-options.test.ts | 78 +++++++++++ src/cli/tui/screens/harness/memory-options.ts | 50 +++++++ src/cli/tui/screens/harness/types.ts | 27 +--- .../screens/harness/useAddHarnessWizard.ts | 58 +++----- 23 files changed, 609 insertions(+), 428 deletions(-) create mode 100644 e2e-tests/harness-aws-skills.test.ts create mode 100644 e2e-tests/harness-managed-memory.test.ts create mode 100644 src/cli/primitives/__tests__/HarnessPrimitive.add.memory.test.ts create mode 100644 src/cli/tui/screens/harness/__tests__/memory-options.test.ts create mode 100644 src/cli/tui/screens/harness/memory-options.ts diff --git a/e2e-tests/harness-aws-skills.test.ts b/e2e-tests/harness-aws-skills.test.ts new file mode 100644 index 000000000..fa0b6704a --- /dev/null +++ b/e2e-tests/harness-aws-skills.test.ts @@ -0,0 +1,13 @@ +import { createHarnessE2ESuite } from './harness-e2e-helper.js'; + +// AWS Skills deployed end-to-end: create → add skill --aws-skills → deploy → invoke → confirm the +// deployed agent actually loaded the skills → teardown. Uses disabled memory so the deploy is fast +// (no managed-memory provisioning) — this suite is about skills, not memory, which managed-memory +// e2e covers. The 'loads AWS skills' step asks the agent what skills it has and asserts it references +// the AWS ones, proving the spec → CFN → runtime path works, not just that the flag parses. +createHarnessE2ESuite({ + modelProvider: 'bedrock', + awsSkills: 'core-skills/*', + skipMemory: true, + labelSuffix: 'aws-skills', +}); diff --git a/e2e-tests/harness-e2e-helper.ts b/e2e-tests/harness-e2e-helper.ts index 65999ca43..8c05aa31b 100644 --- a/e2e-tests/harness-e2e-helper.ts +++ b/e2e-tests/harness-e2e-helper.ts @@ -15,8 +15,24 @@ interface HarnessE2EConfig { modelProvider: 'bedrock' | 'open_ai' | 'gemini'; /** Env var holding the API key ARN — its value is passed as --api-key-arn. */ apiKeyArnEnvVar?: string; + /** Skip memory entirely (`create --no-harness-memory` → disabled). Mutually exclusive with memoryRoundTrip. */ skipMemory?: boolean; skipInvoke?: boolean; + /** + * After creating the (managed-memory) harness, exercise a 2-turn memory round-trip: state a fact, + * then in the SAME session confirm it's recalled. Proves the execution role's memory data-plane + * grant actually works at runtime (the AccessDenied-on-ListEvents scenario) — not just that + * invoke returns 200. Only meaningful when the harness has managed (or omitted) memory. + */ + memoryRoundTrip?: boolean; + /** + * AWS Skills to attach to the harness via `add skill --aws-skills ` before deploy. Use '' + * for all skills, or a comma-separated path filter (e.g. 'core-skills/*'). When set, an extra + * step asks the deployed agent what skills it has and asserts the response references them. + */ + awsSkills?: string; + /** Suffix for the suite label, so multiple suites with the same provider don't collide in output. */ + labelSuffix?: string; } export function createHarnessE2ESuite(cfg: HarnessE2EConfig) { @@ -24,10 +40,11 @@ export function createHarnessE2ESuite(cfg: HarnessE2EConfig) { const canRun = baseCanRun && hasRequiredVar; const providerLabel = - cfg.modelProvider === 'open_ai' ? 'OpenAI' : cfg.modelProvider === 'gemini' ? 'Gemini' : 'Bedrock'; + (cfg.modelProvider === 'open_ai' ? 'OpenAI' : cfg.modelProvider === 'gemini' ? 'Gemini' : 'Bedrock') + + (cfg.labelSuffix ? `/${cfg.labelSuffix}` : ''); // note: this is created outside of beforeAll since beforeAll is skipped if all tests are skipped. - const logger = getLogger(`harness-${providerLabel.toLowerCase()}`); + const logger = getLogger(`harness-${providerLabel.toLowerCase().replace('/', '-')}`); if (!canRun) { logger.warn( `tests are skipped due to insufficient conditions. ` + @@ -74,6 +91,16 @@ export function createHarnessE2ESuite(cfg: HarnessE2EConfig) { const json = parseJsonOutput(result.stdout) as { projectPath: string }; projectPath = json.projectPath; + // Attach AWS Skills to the harness before deploy, if configured. '' means all skills; a + // non-empty value is a comma-separated path filter passed as the --aws-skills argument. + if (cfg.awsSkills !== undefined) { + const skillArgs = ['add', 'skill', '--harness', harnessName, '--aws-skills']; + if (cfg.awsSkills) skillArgs.push(cfg.awsSkills); + skillArgs.push('--json'); + const skillResult = await runAgentCoreCLI(skillArgs, projectPath); + expect(skillResult.exitCode, `add skill --aws-skills failed: ${skillResult.stderr}`).toBe(0); + } + await writeAwsTargets(projectPath); installCdkTarball(projectPath); }, 300000); @@ -91,6 +118,9 @@ export function createHarnessE2ESuite(cfg: HarnessE2EConfig) { async () => { expect(projectPath, 'Project should have been created').toBeTruthy(); + // Retry the deploy: managed-memory provisioning (3-5 min) can run long, and CFN/global-setup + // contention occasionally surfaces a transient non-zero on the first attempt. `deploy` is + // idempotent (re-applies the same stack), so a second attempt is safe and lands the stack. await retry( async () => { const result = await runAgentCoreCLI(['deploy', '--yes', '--json'], projectPath); @@ -100,11 +130,11 @@ export function createHarnessE2ESuite(cfg: HarnessE2EConfig) { const json = parseJsonOutput(result.stdout) as { success: boolean }; expect(json.success, 'Deploy should report success').toBe(true); }, - 1, + 2, 30000 ); }, - 600000 + 900000 ); it.skipIf(!canRun || !!cfg.skipInvoke)( @@ -131,6 +161,99 @@ export function createHarnessE2ESuite(cfg: HarnessE2EConfig) { 180000 ); + // Memory round-trip: prove the execution-role memory data-plane grant works at runtime. The + // harness must read/write its managed memory without AccessDenied (the bug #286/#287 fixed) — + // an invoke that returns 200 isn't enough; the SECOND turn must recall the fact from the FIRST. + it.skipIf(!canRun || !cfg.memoryRoundTrip)( + 'remembers a fact across turns (managed memory round-trip)', + async () => { + // 33+ char session id (service constraint), no random/Date in the literal source needed — + // harnessName already carries a per-run timestamp suffix, which is enough to isolate runs. + const sessionId = `e2e-mem-roundtrip-${harnessName}-padding`; + + const turn1 = await runAgentCoreCLI( + [ + 'invoke', + '--harness', + harnessName, + '--session-id', + sessionId, + '--prompt', + 'My favorite color is teal. Remember it.', + '--json', + ], + projectPath + ); + expect(turn1.exitCode, `Turn 1 failed: stderr=${turn1.stderr}, stdout=${turn1.stdout}`).toBe(0); + const t1 = parseJsonOutput(turn1.stdout) as { success: boolean; response?: string }; + expect(t1.success, 'Turn 1 invoke should succeed (no AccessDenied on memory)').toBe(true); + + // Recall in a fresh invoke on the same session — the harness must read its memory store. + const turn2 = await retry( + async () => { + const result = await runAgentCoreCLI( + [ + 'invoke', + '--harness', + harnessName, + '--session-id', + sessionId, + '--prompt', + 'What is my favorite color? Answer with just the color.', + '--json', + ], + projectPath + ); + expect(result.exitCode, `Turn 2 failed: stderr=${result.stderr}, stdout=${result.stdout}`).toBe(0); + const json = parseJsonOutput(result.stdout) as { success: boolean; response?: string }; + expect(json.success, 'Turn 2 invoke should succeed').toBe(true); + expect( + (json.response ?? '').toLowerCase(), + `Harness should recall "teal" from memory; got: ${json.response}` + ).toContain('teal'); + return json; + }, + 3, + 15000 + ); + expect(turn2.success).toBe(true); + }, + 240000 + ); + + // AWS Skills: prove the deployed agent actually loaded the configured skills (not just that the + // spec carried them). Ask the agent what skills/tools it has and assert it references AWS ones. + it.skipIf(!canRun || cfg.awsSkills === undefined)( + 'loads AWS skills on the deployed harness', + async () => { + await retry( + async () => { + const result = await runAgentCoreCLI( + [ + 'invoke', + '--harness', + harnessName, + '--prompt', + 'List the AWS skills or tools you have access to. Be brief.', + '--json', + ], + projectPath + ); + expect(result.exitCode, `Skills invoke failed: stderr=${result.stderr}, stdout=${result.stdout}`).toBe(0); + const json = parseJsonOutput(result.stdout) as { success: boolean; response?: string }; + expect(json.success, 'Skills invoke should succeed').toBe(true); + expect( + (json.response ?? '').toLowerCase(), + `Agent should reference its loaded AWS skills; got: ${json.response}` + ).toContain('aws'); + }, + 3, + 15000 + ); + }, + 240000 + ); + it.skipIf(!canRun)( 'status shows the deployed harness', async () => { diff --git a/e2e-tests/harness-managed-memory.test.ts b/e2e-tests/harness-managed-memory.test.ts new file mode 100644 index 000000000..53b940171 --- /dev/null +++ b/e2e-tests/harness-managed-memory.test.ts @@ -0,0 +1,8 @@ +import { createHarnessE2ESuite } from './harness-e2e-helper.js'; + +// Managed memory (the default) deployed end-to-end: create → deploy (provisions a dedicated +// AgentCore Memory) → invoke → memory round-trip → teardown. The round-trip step is the load-bearing +// assertion: it proves the harness execution role can read/write its managed memory at runtime +// without AccessDenied on bedrock-agentcore:ListEvents — the regression #286/#287 fixed and that the +// ungating must not reintroduce. +createHarnessE2ESuite({ modelProvider: 'bedrock', memoryRoundTrip: true, labelSuffix: 'managed-memory' }); diff --git a/integ-tests/add-remove-harness.test.ts b/integ-tests/add-remove-harness.test.ts index 7893c5e8a..1e3d900f6 100644 --- a/integ-tests/add-remove-harness.test.ts +++ b/integ-tests/add-remove-harness.test.ts @@ -51,10 +51,15 @@ describe('integration: harness add/remove lifecycle', () => { expect(await exists(promptPath), 'system-prompt.md should exist').toBe(true); }); - it('auto-creates memory resource', async () => { + it('defaults to managed memory and creates NO sibling memory resource', async () => { + // The harness owns its memory internally (managed mode). It must NOT auto-create a + // `${name}Memory` sibling in the project (the legacy pre-managed-memory behavior). + const spec = await readHarnessSpec(project.projectPath, harnessName); + expect(spec.memory?.mode, 'default harness memory should be managed').toBe('managed'); + const config = await readProjectConfig(project.projectPath); - const memories = config.memories ?? []; - expect(memories.length, 'Should have auto-created memory').toBeGreaterThan(0); + const sibling = (config.memories ?? []).find((m: { name: string }) => m.name === `${harnessName}Memory`); + expect(sibling, 'no `${name}Memory` sibling should be auto-created').toBeFalsy(); }); it('rejects duplicate harness name', async () => { @@ -72,12 +77,9 @@ describe('integration: harness add/remove lifecycle', () => { const config = await readProjectConfig(project.projectPath); const found = config.harnesses?.find((h: { name: string }) => h.name === harnessName); expect(found, `Harness "${harnessName}" should be removed`).toBeFalsy(); - - const associatedMemory = (config.memories ?? []).find((m: { name: string }) => m.name === `${harnessName}Memory`); - expect(associatedMemory, 'Associated memory should be removed with harness').toBeFalsy(); }); - it('re-adds harness after removal without duplicate memory error', async () => { + it('re-adds harness after removal', async () => { const result = await runCLI(['add', 'harness', '--name', harnessName, '--json'], project.projectPath); expect(result.exitCode, `stdout: ${result.stdout}, stderr: ${result.stderr}`).toBe(0); @@ -236,7 +238,6 @@ describe('integration: create project with harness', () => { describe('integration: harness config shape', () => { let project: TestProject; - const gatedEnv = { ENABLE_GATED_FEATURES: '1' }; beforeAll(async () => { project = await createTestProject({ noAgent: true }); @@ -420,9 +421,34 @@ describe('integration: harness config shape', () => { expect(git.path).toBe('pack'); expect(git.auth).toEqual({ credentialName: 'gitCred', username: 'git-user' }); }); + + it('adds AWS skills — all paths and a path filter', async () => { + await runCLI(['add', 'harness', '--name', 'AwsSkillHarness', '--no-memory', '--json'], project.projectPath); + + const all = await runCLI( + ['add', 'skill', '--harness', 'AwsSkillHarness', '--aws-skills', '--json'], + project.projectPath + ); + expect(all.exitCode, `stdout: ${all.stdout}, stderr: ${all.stderr}`).toBe(0); + + const filtered = await runCLI( + ['add', 'skill', '--harness', 'AwsSkillHarness', '--aws-skills', 'core-skills/*', '--json'], + project.projectPath + ); + expect(filtered.exitCode, `stdout: ${filtered.stdout}, stderr: ${filtered.stderr}`).toBe(0); + + const spec = await readHarnessSpec(project.projectPath, 'AwsSkillHarness'); + // "all" → awsSkills with no paths key; filtered → awsSkills.paths includes the glob. + expect(spec.skills.some((s: { awsSkills?: { paths?: string[] } }) => s.awsSkills && !s.awsSkills.paths)).toBe( + true + ); + expect( + spec.skills.some((s: { awsSkills?: { paths?: string[] } }) => s.awsSkills?.paths?.includes('core-skills/*')) + ).toBe(true); + }); }); - describe('memory modes (gated)', () => { + describe('memory modes', () => { it('adds a managed-memory harness with explicit strategies', async () => { const name = 'ManagedMemHarness'; const result = await runCLI( @@ -437,8 +463,7 @@ describe('integration: harness config shape', () => { 'SEMANTIC,EPISODIC', '--json', ], - project.projectPath, - { env: gatedEnv } + project.projectPath ); expect(result.exitCode, `stdout: ${result.stdout}, stderr: ${result.stderr}`).toBe(0); @@ -447,6 +472,27 @@ describe('integration: harness config shape', () => { expect(spec.memory.strategies).toEqual(['SEMANTIC', 'EPISODIC']); }); + it('defaults to managed memory when no memory flags are passed', async () => { + const name = 'DefaultMemHarness'; + const result = await runCLI(['add', 'harness', '--name', name, '--json'], project.projectPath); + + expect(result.exitCode, `stdout: ${result.stdout}, stderr: ${result.stderr}`).toBe(0); + const spec = await readHarnessSpec(project.projectPath, name); + expect(spec.memory.mode).toBe('managed'); + }); + + it('writes disabled memory for --memory-mode disabled (true opt-out, no sibling)', async () => { + const name = 'DisabledMemHarness'; + const result = await runCLI( + ['add', 'harness', '--name', name, '--memory-mode', 'disabled', '--json'], + project.projectPath + ); + + expect(result.exitCode, `stdout: ${result.stdout}, stderr: ${result.stderr}`).toBe(0); + const spec = await readHarnessSpec(project.projectPath, name); + expect(spec.memory.mode).toBe('disabled'); + }); + it('adds an existing-memory harness referencing a sibling by name with tuning', async () => { const name = 'ExistingMemHarness'; const result = await runCLI( @@ -465,8 +511,7 @@ describe('integration: harness config shape', () => { '5', '--json', ], - project.projectPath, - { env: gatedEnv } + project.projectPath ); expect(result.exitCode, `stdout: ${result.stdout}, stderr: ${result.stderr}`).toBe(0); @@ -481,26 +526,17 @@ describe('integration: harness config shape', () => { { label: 'retrievalConfig tuning when memory is referenced by ARN', args: ['--memory-mode', 'existing', '--memory-arn', MEMORY_ARN, '--memory-top-k', '5'], - env: gatedEnv, }, { label: '--memory-mode existing without a memory reference', args: ['--memory-mode', 'existing'], - env: gatedEnv, }, { - label: '--memory-mode when the gated feature is disabled', - // Force the gate OFF explicitly: cleanSpawnEnv inherits the host process.env, so a - // developer/CI shell with ENABLE_GATED_FEATURES=1 exported would otherwise flip this - // case (the CLI would accept --memory-mode and exit 0). An empty string is "off" - // because isGatedFeaturesEnabled() checks `=== '1'`. - args: ['--memory-mode', 'managed'], - env: { ENABLE_GATED_FEATURES: '' }, + label: '--no-memory combined with managed-only flags (--memory-strategies)', + args: ['--no-memory', '--memory-strategies', 'SEMANTIC'], }, - ])('rejects $label', async ({ args, env }) => { - const result = await runCLI(['add', 'harness', '--name', 'BadMem', ...args, '--json'], project.projectPath, { - env, - }); + ])('rejects $label', async ({ args }) => { + const result = await runCLI(['add', 'harness', '--name', 'BadMem', ...args, '--json'], project.projectPath); expect(result.exitCode).not.toBe(0); }); }); diff --git a/src/cli/commands/add/__tests__/harness-privatelink-guard.test.ts b/src/cli/commands/add/__tests__/harness-privatelink-guard.test.ts index 3795b3b09..d060e7651 100644 --- a/src/cli/commands/add/__tests__/harness-privatelink-guard.test.ts +++ b/src/cli/commands/add/__tests__/harness-privatelink-guard.test.ts @@ -1,6 +1,6 @@ import type { AddHarnessCliOptions } from '../types'; import { validateAddHarnessOptions } from '../validate'; -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { describe, expect, it } from 'vitest'; const base: AddHarnessCliOptions = { name: 'h1', @@ -73,6 +73,32 @@ describe('validateAddHarnessOptions — memory flag coupling', () => { if (!result.valid) expect(result.error).toContain('--no-memory'); }); + // --no-memory means disabled memory, so managed-only knobs (strategies/event-expiry/KMS) are + // contradictory and must be rejected, not silently dropped. Without an explicit --memory-mode the + // earlier `noMemory && memoryMode in {managed,existing}` guard doesn't fire, so this is the case + // that guards against silently discarding the flags. + it('rejects --no-memory combined with --memory-strategies (managed-only flag)', () => { + const result = validateAddHarnessOptions({ ...base, memory: false, memoryStrategies: 'SEMANTIC' }); + expect(result.valid).toBe(false); + if (!result.valid) expect(result.error).toContain('--no-memory'); + }); + + it('rejects --no-memory combined with --memory-event-expiry-days', () => { + const result = validateAddHarnessOptions({ ...base, memory: false, memoryEventExpiryDays: 30 }); + expect(result.valid).toBe(false); + if (!result.valid) expect(result.error).toContain('--no-memory'); + }); + + it('rejects --no-memory combined with --memory-encryption-key-arn', () => { + const result = validateAddHarnessOptions({ + ...base, + memory: false, + memoryEncryptionKeyArn: 'arn:aws:kms:us-west-2:123456789012:key/abc', + }); + expect(result.valid).toBe(false); + if (!result.valid) expect(result.error).toContain('--no-memory'); + }); + it('accepts --memory-name alone', () => { expect(validateAddHarnessOptions({ ...base, memoryName: 'mem' }).valid).toBe(true); }); @@ -103,42 +129,10 @@ describe('validateAddHarnessOptions — gateway oauth flag coupling', () => { }); }); -describe('validateAddHarnessOptions — memory modes (gated OFF)', () => { - const prev = process.env.ENABLE_GATED_FEATURES; - beforeEach(() => { - delete process.env.ENABLE_GATED_FEATURES; - }); - afterEach(() => { - if (prev === undefined) delete process.env.ENABLE_GATED_FEATURES; - else process.env.ENABLE_GATED_FEATURES = prev; - }); - - it('rejects --memory-mode as not-yet-available', () => { - const r = validateAddHarnessOptions({ ...base, memoryMode: 'managed' }); - expect(r.valid).toBe(false); - if (!r.valid) expect(r.error).toContain('not yet available'); - }); - - it('rejects managed-only flags as not-yet-available', () => { - const r = validateAddHarnessOptions({ ...base, memoryStrategies: 'SEMANTIC' }); - expect(r.valid).toBe(false); - if (!r.valid) expect(r.error).toContain('not yet available'); - }); - - it('still accepts the legacy --memory-name reference', () => { +describe('validateAddHarnessOptions — memory modes', () => { + it('accepts an existing --memory-name reference', () => { expect(validateAddHarnessOptions({ ...base, memoryName: 'mem' }).valid).toBe(true); }); -}); - -describe('validateAddHarnessOptions — memory modes (gated ON)', () => { - const prev = process.env.ENABLE_GATED_FEATURES; - beforeEach(() => { - process.env.ENABLE_GATED_FEATURES = '1'; - }); - afterEach(() => { - if (prev === undefined) delete process.env.ENABLE_GATED_FEATURES; - else process.env.ENABLE_GATED_FEATURES = prev; - }); it('rejects --memory-mode existing with neither arn nor name', () => { const r = validateAddHarnessOptions({ ...base, memoryMode: 'existing' }); diff --git a/src/cli/commands/add/skill-action.ts b/src/cli/commands/add/skill-action.ts index f87e64130..025d620a7 100644 --- a/src/cli/commands/add/skill-action.ts +++ b/src/cli/commands/add/skill-action.ts @@ -1,6 +1,5 @@ import { ConfigIO } from '../../../lib'; import type { HarnessSpec } from '../../../schema'; -import { isGatedFeaturesEnabled } from '@/cli/feature-flags'; import { getSkillKey, validateGitSkillCredential } from '@/cli/operations/harness/skill-utils'; import { ValidationError } from '@/lib/errors/types'; import type { Result } from '@/lib/result'; @@ -34,23 +33,12 @@ export async function handleAddSkill( }; } - if (options.awsSkills && !isGatedFeaturesEnabled()) { - return { - success: false, - error: new ValidationError('AWS skills are not yet available.'), - }; - } - - const sources = [options.path, options.s3, options.git, ...(isGatedFeaturesEnabled() ? [options.awsSkills] : [])]; + const sources = [options.path, options.s3, options.git, options.awsSkills]; const sourceCount = sources.filter(Boolean).length; if (sourceCount !== 1) { return { success: false, - error: new ValidationError( - isGatedFeaturesEnabled() - ? 'Exactly one of --path, --s3, --git, or --aws-skills is required' - : 'Exactly one of --path, --s3, or --git is required' - ), + error: new ValidationError('Exactly one of --path, --s3, --git, or --aws-skills is required'), }; } diff --git a/src/cli/commands/add/skill-command.ts b/src/cli/commands/add/skill-command.ts index 6f5e1fc00..354a6f4a4 100644 --- a/src/cli/commands/add/skill-command.ts +++ b/src/cli/commands/add/skill-command.ts @@ -1,6 +1,5 @@ import { findConfigRoot } from '../../../lib'; import { getErrorMessage } from '../../errors'; -import { isGatedFeaturesEnabled } from '../../feature-flags'; import { withCommandRunTelemetry } from '../../telemetry/cli-command-run.js'; import { SkillSourceType, standardize } from '../../telemetry/schemas/common-shapes.js'; import { handleAddSkill } from './skill-action'; @@ -23,7 +22,7 @@ export function registerAddSkill(addCmd: Command): void { .option('--git-path ', 'Subdirectory within the git repo (for --git)') .option('--credential ', 'Name of an API key credential in the project (for git auth)') .option('--username ', 'Username for git auth (for --git)') - .addOption(isGatedFeaturesEnabled() ? awsSkillsOption : awsSkillsOption.hideHelp()) + .addOption(awsSkillsOption) .option('--json', 'Output as JSON') .action(async cliOptions => { if (!findConfigRoot()) { diff --git a/src/cli/commands/add/validate.ts b/src/cli/commands/add/validate.ts index 59c0d324b..b0309f941 100644 --- a/src/cli/commands/add/validate.ts +++ b/src/cli/commands/add/validate.ts @@ -1207,63 +1207,55 @@ export function validateAddHarnessOptions(options: AddHarnessCliOptions): Valida options.memoryMessagesCount !== undefined || options.memoryTopK !== undefined || options.memoryRelevanceScore !== undefined; + // Managed-only knobs (strategies/event-expiry/KMS) describe a managed memory; they are meaningless + // with --no-memory (which produces disabled memory) and on non-managed modes. + const managedOnlyFlags = + options.memoryStrategies !== undefined || + options.memoryEventExpiryDays !== undefined || + options.memoryEncryptionKeyArn !== undefined; if (options.memoryArn && options.memoryName) { return { valid: false, error: '--memory-arn and --memory-name are mutually exclusive' }; } - if (noMemory && (options.memoryArn || options.memoryName || memoryTuningGiven)) { + if (noMemory && (options.memoryArn || options.memoryName || memoryTuningGiven || managedOnlyFlags)) { return { valid: false, - error: '--no-memory cannot be combined with --memory-arn, --memory-name, or memory tuning flags', + error: + '--no-memory cannot be combined with --memory-arn, --memory-name, memory tuning flags, or managed-memory flags (--memory-strategies, --memory-event-expiry-days, --memory-encryption-key-arn)', }; } - // Managed-memory mode validation — only when the gated feature is enabled. When gated off, any - // --memory-mode / managed-only flag is rejected up front as "not yet available". - const managedOnlyFlags = - options.memoryStrategies !== undefined || - options.memoryEventExpiryDays !== undefined || - options.memoryEncryptionKeyArn !== undefined; - if (!isGatedFeaturesEnabled()) { - if (options.memoryMode !== undefined || managedOnlyFlags) { - return { - valid: false, - error: - '--memory-mode and managed-memory flags (--memory-strategies, --memory-event-expiry-days, --memory-encryption-key-arn) are not yet available.', - }; - } - } else { - if (options.memoryMode && !VALID_MEMORY_MODES.includes(options.memoryMode as (typeof VALID_MEMORY_MODES)[number])) { - return { - valid: false, - error: `Invalid --memory-mode '${options.memoryMode}'. Use ${VALID_MEMORY_MODES.join(', ')}.`, - }; - } - if (noMemory && (options.memoryMode === 'managed' || options.memoryMode === 'existing')) { - return { valid: false, error: '--no-memory cannot be combined with --memory-mode managed/existing' }; - } - if (options.memoryMode === 'existing' && !options.memoryArn && !options.memoryName) { - return { valid: false, error: '--memory-mode existing requires --memory-arn or --memory-name' }; - } - if (managedOnlyFlags && options.memoryMode && options.memoryMode !== 'managed') { + // Managed-memory mode validation. + if (options.memoryMode && !VALID_MEMORY_MODES.includes(options.memoryMode as (typeof VALID_MEMORY_MODES)[number])) { + return { + valid: false, + error: `Invalid --memory-mode '${options.memoryMode}'. Use ${VALID_MEMORY_MODES.join(', ')}.`, + }; + } + if (noMemory && (options.memoryMode === 'managed' || options.memoryMode === 'existing')) { + return { valid: false, error: '--no-memory cannot be combined with --memory-mode managed/existing' }; + } + if (options.memoryMode === 'existing' && !options.memoryArn && !options.memoryName) { + return { valid: false, error: '--memory-mode existing requires --memory-arn or --memory-name' }; + } + if (managedOnlyFlags && options.memoryMode && options.memoryMode !== 'managed') { + return { + valid: false, + error: + '--memory-strategies, --memory-event-expiry-days, and --memory-encryption-key-arn are only valid with --memory-mode managed', + }; + } + if (options.memoryStrategies) { + const bad = options.memoryStrategies + .split(',') + .map(s => s.trim()) + .filter(Boolean) + .filter(s => !VALID_MANAGED_STRATEGIES.includes(s as (typeof VALID_MANAGED_STRATEGIES)[number])); + if (bad.length) { return { valid: false, - error: - '--memory-strategies, --memory-event-expiry-days, and --memory-encryption-key-arn are only valid with --memory-mode managed', + error: `Invalid managed memory strateg${bad.length > 1 ? 'ies' : 'y'}: ${bad.join(', ')}. Valid: ${VALID_MANAGED_STRATEGIES.join(', ')}`, }; } - if (options.memoryStrategies) { - const bad = options.memoryStrategies - .split(',') - .map(s => s.trim()) - .filter(Boolean) - .filter(s => !VALID_MANAGED_STRATEGIES.includes(s as (typeof VALID_MANAGED_STRATEGIES)[number])); - if (bad.length) { - return { - valid: false, - error: `Invalid managed memory strateg${bad.length > 1 ? 'ies' : 'y'}: ${bad.join(', ')}. Valid: ${VALID_MANAGED_STRATEGIES.join(', ')}`, - }; - } - } } if (options.authorizerType) { diff --git a/src/cli/commands/deploy/actions.ts b/src/cli/commands/deploy/actions.ts index b3e349f69..737f118e4 100644 --- a/src/cli/commands/deploy/actions.ts +++ b/src/cli/commands/deploy/actions.ts @@ -22,7 +22,6 @@ import { parseRuntimeEndpointOutputs, } from '../../cloudformation'; import { getErrorMessage } from '../../errors'; -import { isGatedFeaturesEnabled } from '../../feature-flags'; import { ExecLogger } from '../../logging'; import { MANAGED_MEMORY_DEPLOY_NOTICE, @@ -736,16 +735,14 @@ export async function handleDeploy(options: ValidatedDeployOptions): Promise { : undefined; // One-shot user-facing notices (e.g. the managed-memory heads-up before the slow CFN apply). - // Always shown, independent of --progress/--verbose. Clear any active spinner line first so the - // multi-line notice prints cleanly, then resume the spinner frame on the next onProgress tick. - const onNotice = (message: string) => { - if (spinner) { - clearInterval(spinner); - spinner = undefined; - process.stdout.write('\r\x1b[K'); - } - console.log(`\n${message}\n`); - }; + // Shown independent of --progress/--verbose, but NEVER under --json: stdout must stay pure + // machine-readable JSON there (the notice would corrupt it, breaking `deploy --json` parsing for + // any managed-memory harness). The notice is still captured in the deploy log via logger.log. + // Clear any active spinner line first so the multi-line notice prints cleanly. + const onNotice = options.json + ? undefined + : (message: string) => { + if (spinner) { + clearInterval(spinner); + spinner = undefined; + process.stdout.write('\r\x1b[K'); + } + console.log(`\n${message}\n`); + }; const result = await handleDeploy({ target: options.target!, diff --git a/src/cli/commands/status/__tests__/action.test.ts b/src/cli/commands/status/__tests__/action.test.ts index 4cb7ac66a..42af29bc0 100644 --- a/src/cli/commands/status/__tests__/action.test.ts +++ b/src/cli/commands/status/__tests__/action.test.ts @@ -25,11 +25,6 @@ vi.mock('../../../aws/bedrock-agent', () => ({ getLatestIngestionJob: (...args: unknown[]) => mockGetLatestIngestionJob(...args), })); -const mockIsGatedFeaturesEnabled = vi.fn(() => true); -vi.mock('../../../feature-flags', () => ({ - isGatedFeaturesEnabled: () => mockIsGatedFeaturesEnabled(), -})); - const loggedLines: string[] = []; vi.mock('../../../logging', () => { return { @@ -483,7 +478,7 @@ describe('computeResourceStatuses', () => { expect(harnessEntry!.identifier).toBe('arn:aws:bedrock:us-east-1:123456789:harness/h-456'); }); - it('renders the config version (v{N}) on a deployed harness when gated features are enabled', () => { + it('renders the config version (v{N}) on a deployed harness', () => { const project = { ...baseProject, harnesses: [{ name: 'my-harness', path: 'harnesses/my-harness' }], @@ -505,29 +500,6 @@ describe('computeResourceStatuses', () => { expect(harnessEntry!.detail).toBe('v3'); }); - it('does not render the config version when gated features are disabled', () => { - mockIsGatedFeaturesEnabled.mockReturnValueOnce(false); - const project = { - ...baseProject, - harnesses: [{ name: 'my-harness', path: 'harnesses/my-harness' }], - } as unknown as AgentCoreProjectSpec; - const resources: DeployedResourceState = { - harnesses: { - 'my-harness': { - harnessId: 'h-123', - harnessArn: 'arn:aws:bedrock:us-east-1:123456789:harness/h-123', - roleArn: 'arn:aws:iam::123456789:role/test', - status: 'READY', - harnessVersion: 3, - }, - }, - }; - - const result = computeResourceStatuses(project, resources); - const harnessEntry = result.find(r => r.resourceType === 'harness' && r.name === 'my-harness'); - expect(harnessEntry!.detail).toBeUndefined(); - }); - it('handles mixed deployed and local-only resources', () => { const project = { ...baseProject, diff --git a/src/cli/commands/status/action.ts b/src/cli/commands/status/action.ts index c62920c37..bc1eb2002 100644 --- a/src/cli/commands/status/action.ts +++ b/src/cli/commands/status/action.ts @@ -6,7 +6,6 @@ import { getEvaluator, getOnlineEvaluationConfig } from '../../aws/agentcore-con import { getPaymentManager } from '../../aws/agentcore-payments'; import { getKnowledgeBase, getLatestIngestionJob } from '../../aws/bedrock-agent'; import { getErrorMessage } from '../../errors'; -import { isGatedFeaturesEnabled } from '../../feature-flags'; import { ExecLogger } from '../../logging'; import type { ResourceDeploymentState } from './constants'; import { buildRuntimeInvocationUrl } from './constants'; @@ -318,13 +317,11 @@ export function computeResourceStatuses( getLocalDetail: () => undefined, }); - // Config version (gated): the harness Version is service-incremented and only known from deployed + // Config version: the harness Version is service-incremented and only known from deployed // state, so enrich each entry's detail post-pass rather than via getLocalDetail (local spec has none). - if (isGatedFeaturesEnabled()) { - for (const entry of harnesses) { - const version = resources?.harnesses?.[entry.name]?.harnessVersion; - if (version !== undefined) entry.detail = entry.detail ? `${entry.detail} v${version}` : `v${version}`; - } + for (const entry of harnesses) { + const version = resources?.harnesses?.[entry.name]?.harnessVersion; + if (version !== undefined) entry.detail = entry.detail ? `${entry.detail} v${version}` : `v${version}`; } const payments = diffResourceSet({ diff --git a/src/cli/operations/deploy/__tests__/managed-memory-notice.test.ts b/src/cli/operations/deploy/__tests__/managed-memory-notice.test.ts index 9d22e4cd4..56cd2f5e6 100644 --- a/src/cli/operations/deploy/__tests__/managed-memory-notice.test.ts +++ b/src/cli/operations/deploy/__tests__/managed-memory-notice.test.ts @@ -4,7 +4,7 @@ import { MANAGED_MEMORY_DEPLOY_NOTICE, hasManagedMemoryHarness, } from '../managed-memory-notice'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; /** * Builds a stub ConfigIO whose readHarnessSpec returns the given mode per harness name. @@ -23,27 +23,10 @@ function stubConfigIO(modes: Record): ConfigIO { } describe('hasManagedMemoryHarness', () => { - const originalGate = process.env.ENABLE_GATED_FEATURES; - - beforeEach(() => { - process.env.ENABLE_GATED_FEATURES = '1'; - }); - afterEach(() => { - if (originalGate === undefined) { - delete process.env.ENABLE_GATED_FEATURES; - } else { - process.env.ENABLE_GATED_FEATURES = originalGate; - } vi.clearAllMocks(); }); - it('returns false when the gate is off, even with a managed harness', async () => { - delete process.env.ENABLE_GATED_FEATURES; - const configIO = stubConfigIO({ h1: 'managed' }); - expect(await hasManagedMemoryHarness(configIO, [{ name: 'h1' }])).toBe(false); - }); - it('returns false when there are no harnesses', async () => { expect(await hasManagedMemoryHarness(stubConfigIO({}), [])).toBe(false); expect(await hasManagedMemoryHarness(stubConfigIO({}), undefined)).toBe(false); @@ -59,6 +42,12 @@ describe('hasManagedMemoryHarness', () => { expect(await hasManagedMemoryHarness(configIO, [{ name: 'h1' }, { name: 'h2' }])).toBe(false); }); + it('returns true for a harness whose memory config is OMITTED (service auto-provisions managed)', async () => { + // mode undefined → stub resolves { memory: undefined } (omitted), which auto-provisions on deploy. + const configIO = stubConfigIO({ h1: undefined }); + expect(await hasManagedMemoryHarness(configIO, [{ name: 'h1' }])).toBe(true); + }); + it('treats an unreadable harness spec as non-managed (does not throw)', async () => { const configIO = stubConfigIO({ h1: 'managed' }); expect(await hasManagedMemoryHarness(configIO, [{ name: 'missing' }, { name: 'h1' }])).toBe(true); diff --git a/src/cli/operations/deploy/managed-memory-notice.ts b/src/cli/operations/deploy/managed-memory-notice.ts index 51699a2c3..b92531f35 100644 --- a/src/cli/operations/deploy/managed-memory-notice.ts +++ b/src/cli/operations/deploy/managed-memory-notice.ts @@ -1,5 +1,4 @@ import type { ConfigIO } from '../../../lib'; -import { isGatedFeaturesEnabled } from '../../feature-flags'; /** * One-shot heads-up shown before the CFN apply when a harness uses managed memory. @@ -26,7 +25,13 @@ export const MANAGED_MEMORY_ADD_NOTICE = 'provisioning time. To skip it, recreate the harness with --memory-mode disabled.'; /** - * Returns true when the gate is on and at least one harness in the project uses managed memory. + * Returns true when at least one harness in the project will provision a NEW managed memory on + * deploy (the slow 3-5 min step the notice explains). That happens for an explicit `managed` mode + * AND for an omitted memory config — the service auto-provisions a managed memory whenever the + * harness is not explicitly `disabled` or pointed at an `existing` memory (mirrors the CDK + * derivation in AgentCoreHarnessEnvironment). `existing` references a pre-existing memory (no + * provisioning) and `disabled` opts out, so neither triggers the notice. + * * The memory mode lives in each harness's harness.json (not the agentcore.json pointer list), so * the per-harness specs are read to detect it. */ @@ -34,12 +39,11 @@ export async function hasManagedMemoryHarness( configIO: ConfigIO, harnesses: { name: string }[] | undefined ): Promise { - if (!isGatedFeaturesEnabled()) { - return false; - } for (const h of harnesses ?? []) { const harnessSpec = await configIO.readHarnessSpec(h.name).catch(() => undefined); - if (harnessSpec?.memory?.mode === 'managed') { + // Only a successfully-loaded spec counts: an explicit `managed` mode, or an omitted memory + // config (auto-provisions). An unreadable spec stays unknown and does not trigger the notice. + if (harnessSpec && (harnessSpec.memory?.mode === 'managed' || harnessSpec.memory === undefined)) { return true; } } diff --git a/src/cli/primitives/HarnessPrimitive.ts b/src/cli/primitives/HarnessPrimitive.ts index 8bbc2dceb..6da163b4f 100644 --- a/src/cli/primitives/HarnessPrimitive.ts +++ b/src/cli/primitives/HarnessPrimitive.ts @@ -8,16 +8,13 @@ import type { HarnessModelProvider, HarnessSpec, ManagedMemoryStrategy, - MemoryStrategy, - MemoryStrategyType, NetworkMode, PrivateEndpoint, RuntimeAuthorizerType, } from '../../schema'; -import { DEFAULT_EPISODIC_REFLECTION_NAMESPACES, DEFAULT_STRATEGY_NAMESPACES, HarnessSpecSchema } from '../../schema'; +import { HarnessSpecSchema } from '../../schema'; import { deleteHarness, isHarnessNotFoundError } from '../aws/agentcore-harness'; import { getErrorMessage } from '../errors'; -import { isGatedFeaturesEnabled } from '../feature-flags'; import { MANAGED_MEMORY_ADD_NOTICE } from '../operations/deploy'; import { findOrphanHarnesses } from '../operations/harness/orphan'; import type { OrphanHarness } from '../operations/harness/orphan'; @@ -26,7 +23,6 @@ import { withCommandRunTelemetry } from '../telemetry/cli-command-run.js'; import type { SubCommand } from '../telemetry/schemas/command-run.js'; import { getTemplatePath } from '../templates/templateRoot'; import { requireTTY } from '../tui/guards/tty'; -import { DEFAULT_MEMORY_EXPIRY_DAYS } from '../tui/screens/generate/defaults'; import { BasePrimitive } from './BasePrimitive'; import { buildAuthorizerConfigFromJwtConfig, createManagedOAuthCredential } from './auth-utils'; import type { JwtConfigOptions } from './auth-utils'; @@ -73,15 +69,6 @@ function strictFloat(label: string): (value: string) => number { }; } -/** - * Hide a gated option from `--help` when ENABLE_GATED_FEATURES is off. The option still PARSES - * (so explicit use is caught by validation with a clean "not yet available" message) but does not - * advertise itself. Mirrors the AWS Skills gating pattern (skill-command.ts). - */ -function gatedOption(option: T): T { - return isGatedFeaturesEnabled() ? option : option.hideHelp(); -} - /** Commander accumulator for repeatable `--env`/`--tag` KEY=VALUE flags. Last write wins per key. */ function collectKeyValue(value: string, previous: Record): Record { const eq = value.indexOf('='); @@ -125,13 +112,13 @@ export interface AddHarnessOptions { modelMaxTokens?: number; systemPrompt?: string; skipMemory?: boolean; - /** Memory mode (gated). managed = harness owns its memory; existing = BYO; disabled = none. */ + /** Memory mode. managed = harness owns its memory; existing = BYO; disabled = none. */ memoryMode?: 'managed' | 'existing' | 'disabled'; - /** Managed-memory strategies (gated). Subset of SEMANTIC/SUMMARIZATION/USER_PREFERENCE/EPISODIC. */ + /** Managed-memory strategies. Subset of SEMANTIC/SUMMARIZATION/USER_PREFERENCE/EPISODIC. */ memoryStrategies?: string[]; - /** Managed-memory event retention in days, 3-365 (gated). */ + /** Managed-memory event retention in days, 3-365. */ memoryEventExpiryDays?: number; - /** Managed-memory KMS CMK ARN, create-only (gated). */ + /** Managed-memory KMS CMK ARN, create-only. */ memoryEncryptionKeyArn?: string; /** Reference an existing memory by name or ARN instead of auto-creating one. */ memoryName?: string; @@ -226,17 +213,11 @@ export class HarnessPrimitive extends BasePrimitive ({ - type, - ...(DEFAULT_STRATEGY_NAMESPACES[type] && { namespaces: DEFAULT_STRATEGY_NAMESPACES[type] }), - ...(type === 'EPISODIC' && { reflectionNamespaces: DEFAULT_EPISODIC_REFLECTION_NAMESPACES }), - })); - - project.memories.push({ - name: autoCreateMemoryName, - eventExpiryDuration: DEFAULT_MEMORY_EXPIRY_DAYS, - strategies, - }); - } - project.harnesses = [ ...harnesses, { @@ -685,30 +651,20 @@ export class HarnessPrimitive extends BasePrimitive