test: collapse schema enumeration tests and remove duplicates

[tejaskash]

Summary

• Collapse it.each per-value enumeration patterns into consolidated tests across 9 schema test files (constants, agent-env, agentcore-project, aws-targets, deployed-state, mcp-defs, mcp, evaluator, memory)
• Remove deploy/dev --help unit tests already covered by integ-tests/help.test.ts
• Remove 5 it.todo placeholders from credential-ops.test.ts

Schema tests: ~506 → ~415. All cross-field validation (superRefine), boundary, and discriminated union tests preserved.

Test plan

• npm test passes — schema subset verified locally (415 tests), full suite should pass in CI
• No coverage regression for src/schema/ (tests collapsed, not deleted — same code paths covered)

[agentcore-cli-automation]

The PR description says these deploy --help / dev --help unit tests are "already covered by integ-tests/help.test.ts", but that isn't quite accurate — integ-tests/help.test.ts is only a smoke test (it checks exit code 0 and the presence of the string "Usage:"). It does not assert on specific flags or options.

Concretely, after this PR nothing verifies:

• deploy --help lists --verbose, --yes, --json, --dry-run
• deploy --help's --verbose description mentions "resource-level"
• dev --help lists [prompt], --port, --runtime, --stream, --logs, and the default port 8080
• dev --help does not show --invoke (see separate comment — this is the only regression guard against reintroducing the removed flag)

A few options:

1. Keep the unit tests as-is and update the PR description to clarify they aren't redundant.
2. Move the flag-presence assertions into integ-tests/help.test.ts (e.g., a table of expected substrings per command) before deleting the unit versions.
3. Drop only the assertions that are genuinely duplicated by the integ smoke test (i.e., none — exit code and "Usage:" aren't what the unit tests were asserting), and keep the flag-level assertions.

[agentcore-cli-automation]

Removing the it('does not show --invoke flag', ...) test in dev.test.ts drops the only regression guard against reintroducing the removed --invoke flag. I grepped the repo and this assertion appears nowhere else:

$ grep -rn "\-\-invoke" src --include='*.ts'
src/cli/commands/dev/__tests__/dev.test.ts:18: it('does not show --invoke flag', async () => {
src/cli/commands/dev/__tests__/dev.test.ts:22: expect(result.stdout.includes('--invoke'), 'Should not show removed --invoke option').toBeFalsy();

Since this is a negative assertion (a flag was removed and shouldn't come back), it's exactly the kind of thing a smoke test can't replace. Please either keep this test or relocate the assertion into integ-tests/help.test.ts.

[agentcore-cli-automation]

The PR description asserts "No coverage regression for src/schema/ (tests collapsed, not deleted — same code paths covered)". That holds for most of the collapsed cases — zod's enum parser is the same code path regardless of which enum value you feed it — but there are two places where the collapse actually narrows coverage in a way I don't think is intended:

aws-targets.test.ts — AgentCoreRegionSchema: Previously enumerated all 16 supported regions; now only asserts on us-east-1, eu-west-1, us-gov-west-1. If someone accidentally drops ap-northeast-1, ca-central-1, sa-east-1, etc. from the schema's region list, nothing here will fail. The explicit enumeration of supported regions is itself the coverage signal.

mcp.test.ts — GatewayTargetTypeSchema: Previously enumerated all 6 types (lambda, mcpServer, openApiSchema, smithyModel, apiGateway, lambdaFunctionArn); now only asserts on lambda and lambdaFunctionArn. Dropping mcpServer/openApiSchema/smithyModel/apiGateway from the enum would not fail this test.

Options:

1. Keep it.each(...) for these two (it's the one pattern where enumeration genuinely catches regressions beyond "zod enum parsing works").
2. Replace with a single assertion against Schema.options (e.g., expect(AgentCoreRegionSchema.options).toEqual([...])) — similar to what the PR already does for MemoryStrategyTypeSchema.

FWIW, the other collapsed enums (SDKFrameworkSchema, ModelProviderSchema) are fine because matchEnumValue tests above already exercise the other values via their lowercase variants.

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	42.89%	8936 / 20830
🔵	Statements	42.17%	9485 / 22491
🔵	Functions	39.66%	1537 / 3875
🔵	Branches	39.82%	5769 / 14486

Generated in workflow #2335 for commit 5c66797 by the Vitest Coverage Report Action

[tejaskash]

Good catch — restored the deploy/dev help flag assertions and the --invoke negative regression guard. The integ smoke test is indeed only exit-code + "Usage:".

[tejaskash]

Addressed: replaced the representative-sample tests with .options assertions for both AgentCoreRegionSchema and GatewayTargetTypeSchema. This catches accidental removal of any enum value without per-value it.each tests.

[jesseturner21]

LGTM

[tejaskash]

Summary

• Collapse it.each per-value enumeration patterns into consolidated tests across 9 schema test files (constants, agent-env, agentcore-project, aws-targets, deployed-state, mcp-defs, mcp, evaluator, memory)
• Remove deploy/dev --help unit tests already covered by integ-tests/help.test.ts
• Remove 5 it.todo placeholders from credential-ops.test.ts

Schema tests: ~506 → ~415. All cross-field validation (superRefine), boundary, and discriminated union tests preserved.

Test plan

• npm test passes — schema subset verified locally (415 tests), full suite should pass in CI
• No coverage regression for src/schema/ (tests collapsed, not deleted — same code paths covered)

[agentcore-cli-automation]

The PR description says these deploy --help / dev --help unit tests are "already covered by integ-tests/help.test.ts", but that isn't quite accurate — integ-tests/help.test.ts is only a smoke test (it checks exit code 0 and the presence of the string "Usage:"). It does not assert on specific flags or options.

Concretely, after this PR nothing verifies:

• deploy --help lists --verbose, --yes, --json, --dry-run
• deploy --help's --verbose description mentions "resource-level"
• dev --help lists [prompt], --port, --runtime, --stream, --logs, and the default port 8080
• dev --help does not show --invoke (see separate comment — this is the only regression guard against reintroducing the removed flag)

A few options:

1. Keep the unit tests as-is and update the PR description to clarify they aren't redundant.
2. Move the flag-presence assertions into integ-tests/help.test.ts (e.g., a table of expected substrings per command) before deleting the unit versions.
3. Drop only the assertions that are genuinely duplicated by the integ smoke test (i.e., none — exit code and "Usage:" aren't what the unit tests were asserting), and keep the flag-level assertions.

[agentcore-cli-automation]

Removing the it('does not show --invoke flag', ...) test in dev.test.ts drops the only regression guard against reintroducing the removed --invoke flag. I grepped the repo and this assertion appears nowhere else:

$ grep -rn "\-\-invoke" src --include='*.ts'
src/cli/commands/dev/__tests__/dev.test.ts:18: it('does not show --invoke flag', async () => {
src/cli/commands/dev/__tests__/dev.test.ts:22: expect(result.stdout.includes('--invoke'), 'Should not show removed --invoke option').toBeFalsy();

Since this is a negative assertion (a flag was removed and shouldn't come back), it's exactly the kind of thing a smoke test can't replace. Please either keep this test or relocate the assertion into integ-tests/help.test.ts.

[agentcore-cli-automation]

The PR description asserts "No coverage regression for src/schema/ (tests collapsed, not deleted — same code paths covered)". That holds for most of the collapsed cases — zod's enum parser is the same code path regardless of which enum value you feed it — but there are two places where the collapse actually narrows coverage in a way I don't think is intended:

aws-targets.test.ts — AgentCoreRegionSchema: Previously enumerated all 16 supported regions; now only asserts on us-east-1, eu-west-1, us-gov-west-1. If someone accidentally drops ap-northeast-1, ca-central-1, sa-east-1, etc. from the schema's region list, nothing here will fail. The explicit enumeration of supported regions is itself the coverage signal.

mcp.test.ts — GatewayTargetTypeSchema: Previously enumerated all 6 types (lambda, mcpServer, openApiSchema, smithyModel, apiGateway, lambdaFunctionArn); now only asserts on lambda and lambdaFunctionArn. Dropping mcpServer/openApiSchema/smithyModel/apiGateway from the enum would not fail this test.

Options:

1. Keep it.each(...) for these two (it's the one pattern where enumeration genuinely catches regressions beyond "zod enum parsing works").
2. Replace with a single assertion against Schema.options (e.g., expect(AgentCoreRegionSchema.options).toEqual([...])) — similar to what the PR already does for MemoryStrategyTypeSchema.

FWIW, the other collapsed enums (SDKFrameworkSchema, ModelProviderSchema) are fine because matchEnumValue tests above already exercise the other values via their lowercase variants.

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	42.89%	8936 / 20830
🔵	Statements	42.17%	9485 / 22491
🔵	Functions	39.66%	1537 / 3875
🔵	Branches	39.82%	5769 / 14486

Generated in workflow #2335 for commit 5c66797 by the Vitest Coverage Report Action

[tejaskash]

Good catch — restored the deploy/dev help flag assertions and the --invoke negative regression guard. The integ smoke test is indeed only exit-code + "Usage:".

[tejaskash]

Addressed: replaced the representative-sample tests with .options assertions for both AgentCoreRegionSchema and GatewayTargetTypeSchema. This catches accidental removal of any enum value without per-value it.each tests.

[jesseturner21]

LGTM

[tejaskash]

Summary

• Collapse it.each per-value enumeration patterns into consolidated tests across 9 schema test files (constants, agent-env, agentcore-project, aws-targets, deployed-state, mcp-defs, mcp, evaluator, memory)
• Remove deploy/dev --help unit tests already covered by integ-tests/help.test.ts
• Remove 5 it.todo placeholders from credential-ops.test.ts

Schema tests: ~506 → ~415. All cross-field validation (superRefine), boundary, and discriminated union tests preserved.

Test plan

• npm test passes — schema subset verified locally (415 tests), full suite should pass in CI
• No coverage regression for src/schema/ (tests collapsed, not deleted — same code paths covered)

[agentcore-cli-automation]

The PR description says these deploy --help / dev --help unit tests are "already covered by integ-tests/help.test.ts", but that isn't quite accurate — integ-tests/help.test.ts is only a smoke test (it checks exit code 0 and the presence of the string "Usage:"). It does not assert on specific flags or options.

Concretely, after this PR nothing verifies:

• deploy --help lists --verbose, --yes, --json, --dry-run
• deploy --help's --verbose description mentions "resource-level"
• dev --help lists [prompt], --port, --runtime, --stream, --logs, and the default port 8080
• dev --help does not show --invoke (see separate comment — this is the only regression guard against reintroducing the removed flag)

A few options:

1. Keep the unit tests as-is and update the PR description to clarify they aren't redundant.
2. Move the flag-presence assertions into integ-tests/help.test.ts (e.g., a table of expected substrings per command) before deleting the unit versions.
3. Drop only the assertions that are genuinely duplicated by the integ smoke test (i.e., none — exit code and "Usage:" aren't what the unit tests were asserting), and keep the flag-level assertions.

[agentcore-cli-automation]

Removing the it('does not show --invoke flag', ...) test in dev.test.ts drops the only regression guard against reintroducing the removed --invoke flag. I grepped the repo and this assertion appears nowhere else:

$ grep -rn "\-\-invoke" src --include='*.ts'
src/cli/commands/dev/__tests__/dev.test.ts:18: it('does not show --invoke flag', async () => {
src/cli/commands/dev/__tests__/dev.test.ts:22: expect(result.stdout.includes('--invoke'), 'Should not show removed --invoke option').toBeFalsy();

Since this is a negative assertion (a flag was removed and shouldn't come back), it's exactly the kind of thing a smoke test can't replace. Please either keep this test or relocate the assertion into integ-tests/help.test.ts.

[agentcore-cli-automation]

The PR description asserts "No coverage regression for src/schema/ (tests collapsed, not deleted — same code paths covered)". That holds for most of the collapsed cases — zod's enum parser is the same code path regardless of which enum value you feed it — but there are two places where the collapse actually narrows coverage in a way I don't think is intended:

aws-targets.test.ts — AgentCoreRegionSchema: Previously enumerated all 16 supported regions; now only asserts on us-east-1, eu-west-1, us-gov-west-1. If someone accidentally drops ap-northeast-1, ca-central-1, sa-east-1, etc. from the schema's region list, nothing here will fail. The explicit enumeration of supported regions is itself the coverage signal.

mcp.test.ts — GatewayTargetTypeSchema: Previously enumerated all 6 types (lambda, mcpServer, openApiSchema, smithyModel, apiGateway, lambdaFunctionArn); now only asserts on lambda and lambdaFunctionArn. Dropping mcpServer/openApiSchema/smithyModel/apiGateway from the enum would not fail this test.

Options:

1. Keep it.each(...) for these two (it's the one pattern where enumeration genuinely catches regressions beyond "zod enum parsing works").
2. Replace with a single assertion against Schema.options (e.g., expect(AgentCoreRegionSchema.options).toEqual([...])) — similar to what the PR already does for MemoryStrategyTypeSchema.

FWIW, the other collapsed enums (SDKFrameworkSchema, ModelProviderSchema) are fine because matchEnumValue tests above already exercise the other values via their lowercase variants.

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	42.89%	8936 / 20830
🔵	Statements	42.17%	9485 / 22491
🔵	Functions	39.66%	1537 / 3875
🔵	Branches	39.82%	5769 / 14486

Generated in workflow #2335 for commit 5c66797 by the Vitest Coverage Report Action

[tejaskash]

Good catch — restored the deploy/dev help flag assertions and the --invoke negative regression guard. The integ smoke test is indeed only exit-code + "Usage:".

[tejaskash]

Addressed: replaced the representative-sample tests with .options assertions for both AgentCoreRegionSchema and GatewayTargetTypeSchema. This catches accidental removal of any enum value without per-value it.each tests.

[jesseturner21]

LGTM

[tejaskash]

Summary

• Collapse it.each per-value enumeration patterns into consolidated tests across 9 schema test files (constants, agent-env, agentcore-project, aws-targets, deployed-state, mcp-defs, mcp, evaluator, memory)
• Remove deploy/dev --help unit tests already covered by integ-tests/help.test.ts
• Remove 5 it.todo placeholders from credential-ops.test.ts

Schema tests: ~506 → ~415. All cross-field validation (superRefine), boundary, and discriminated union tests preserved.

Test plan

• npm test passes — schema subset verified locally (415 tests), full suite should pass in CI
• No coverage regression for src/schema/ (tests collapsed, not deleted — same code paths covered)

[agentcore-cli-automation]

The PR description says these deploy --help / dev --help unit tests are "already covered by integ-tests/help.test.ts", but that isn't quite accurate — integ-tests/help.test.ts is only a smoke test (it checks exit code 0 and the presence of the string "Usage:"). It does not assert on specific flags or options.

Concretely, after this PR nothing verifies:

• deploy --help lists --verbose, --yes, --json, --dry-run
• deploy --help's --verbose description mentions "resource-level"
• dev --help lists [prompt], --port, --runtime, --stream, --logs, and the default port 8080
• dev --help does not show --invoke (see separate comment — this is the only regression guard against reintroducing the removed flag)

A few options:

1. Keep the unit tests as-is and update the PR description to clarify they aren't redundant.
2. Move the flag-presence assertions into integ-tests/help.test.ts (e.g., a table of expected substrings per command) before deleting the unit versions.
3. Drop only the assertions that are genuinely duplicated by the integ smoke test (i.e., none — exit code and "Usage:" aren't what the unit tests were asserting), and keep the flag-level assertions.

[agentcore-cli-automation]

Removing the it('does not show --invoke flag', ...) test in dev.test.ts drops the only regression guard against reintroducing the removed --invoke flag. I grepped the repo and this assertion appears nowhere else:

$ grep -rn "\-\-invoke" src --include='*.ts'
src/cli/commands/dev/__tests__/dev.test.ts:18: it('does not show --invoke flag', async () => {
src/cli/commands/dev/__tests__/dev.test.ts:22: expect(result.stdout.includes('--invoke'), 'Should not show removed --invoke option').toBeFalsy();

Since this is a negative assertion (a flag was removed and shouldn't come back), it's exactly the kind of thing a smoke test can't replace. Please either keep this test or relocate the assertion into integ-tests/help.test.ts.

[agentcore-cli-automation]

The PR description asserts "No coverage regression for src/schema/ (tests collapsed, not deleted — same code paths covered)". That holds for most of the collapsed cases — zod's enum parser is the same code path regardless of which enum value you feed it — but there are two places where the collapse actually narrows coverage in a way I don't think is intended:

aws-targets.test.ts — AgentCoreRegionSchema: Previously enumerated all 16 supported regions; now only asserts on us-east-1, eu-west-1, us-gov-west-1. If someone accidentally drops ap-northeast-1, ca-central-1, sa-east-1, etc. from the schema's region list, nothing here will fail. The explicit enumeration of supported regions is itself the coverage signal.

mcp.test.ts — GatewayTargetTypeSchema: Previously enumerated all 6 types (lambda, mcpServer, openApiSchema, smithyModel, apiGateway, lambdaFunctionArn); now only asserts on lambda and lambdaFunctionArn. Dropping mcpServer/openApiSchema/smithyModel/apiGateway from the enum would not fail this test.

Options:

1. Keep it.each(...) for these two (it's the one pattern where enumeration genuinely catches regressions beyond "zod enum parsing works").
2. Replace with a single assertion against Schema.options (e.g., expect(AgentCoreRegionSchema.options).toEqual([...])) — similar to what the PR already does for MemoryStrategyTypeSchema.

FWIW, the other collapsed enums (SDKFrameworkSchema, ModelProviderSchema) are fine because matchEnumValue tests above already exercise the other values via their lowercase variants.

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	42.89%	8936 / 20830
🔵	Statements	42.17%	9485 / 22491
🔵	Functions	39.66%	1537 / 3875
🔵	Branches	39.82%	5769 / 14486

Generated in workflow #2335 for commit 5c66797 by the Vitest Coverage Report Action

[tejaskash]

Good catch — restored the deploy/dev help flag assertions and the --invoke negative regression guard. The integ smoke test is indeed only exit-code + "Usage:".

[tejaskash]

Addressed: replaced the representative-sample tests with .options assertions for both AgentCoreRegionSchema and GatewayTargetTypeSchema. This catches accidental removal of any enum value without per-value it.each tests.

[jesseturner21]

LGTM

[tejaskash]

Summary

• Collapse it.each per-value enumeration patterns into consolidated tests across 9 schema test files (constants, agent-env, agentcore-project, aws-targets, deployed-state, mcp-defs, mcp, evaluator, memory)
• Remove deploy/dev --help unit tests already covered by integ-tests/help.test.ts
• Remove 5 it.todo placeholders from credential-ops.test.ts

Schema tests: ~506 → ~415. All cross-field validation (superRefine), boundary, and discriminated union tests preserved.

Test plan

• npm test passes — schema subset verified locally (415 tests), full suite should pass in CI
• No coverage regression for src/schema/ (tests collapsed, not deleted — same code paths covered)

[agentcore-cli-automation]

The PR description says these deploy --help / dev --help unit tests are "already covered by integ-tests/help.test.ts", but that isn't quite accurate — integ-tests/help.test.ts is only a smoke test (it checks exit code 0 and the presence of the string "Usage:"). It does not assert on specific flags or options.

Concretely, after this PR nothing verifies:

• deploy --help lists --verbose, --yes, --json, --dry-run
• deploy --help's --verbose description mentions "resource-level"
• dev --help lists [prompt], --port, --runtime, --stream, --logs, and the default port 8080
• dev --help does not show --invoke (see separate comment — this is the only regression guard against reintroducing the removed flag)

A few options:

1. Keep the unit tests as-is and update the PR description to clarify they aren't redundant.
2. Move the flag-presence assertions into integ-tests/help.test.ts (e.g., a table of expected substrings per command) before deleting the unit versions.
3. Drop only the assertions that are genuinely duplicated by the integ smoke test (i.e., none — exit code and "Usage:" aren't what the unit tests were asserting), and keep the flag-level assertions.

[agentcore-cli-automation]

Removing the it('does not show --invoke flag', ...) test in dev.test.ts drops the only regression guard against reintroducing the removed --invoke flag. I grepped the repo and this assertion appears nowhere else:

$ grep -rn "\-\-invoke" src --include='*.ts'
src/cli/commands/dev/__tests__/dev.test.ts:18: it('does not show --invoke flag', async () => {
src/cli/commands/dev/__tests__/dev.test.ts:22: expect(result.stdout.includes('--invoke'), 'Should not show removed --invoke option').toBeFalsy();

Since this is a negative assertion (a flag was removed and shouldn't come back), it's exactly the kind of thing a smoke test can't replace. Please either keep this test or relocate the assertion into integ-tests/help.test.ts.

[agentcore-cli-automation]

The PR description asserts "No coverage regression for src/schema/ (tests collapsed, not deleted — same code paths covered)". That holds for most of the collapsed cases — zod's enum parser is the same code path regardless of which enum value you feed it — but there are two places where the collapse actually narrows coverage in a way I don't think is intended:

aws-targets.test.ts — AgentCoreRegionSchema: Previously enumerated all 16 supported regions; now only asserts on us-east-1, eu-west-1, us-gov-west-1. If someone accidentally drops ap-northeast-1, ca-central-1, sa-east-1, etc. from the schema's region list, nothing here will fail. The explicit enumeration of supported regions is itself the coverage signal.

mcp.test.ts — GatewayTargetTypeSchema: Previously enumerated all 6 types (lambda, mcpServer, openApiSchema, smithyModel, apiGateway, lambdaFunctionArn); now only asserts on lambda and lambdaFunctionArn. Dropping mcpServer/openApiSchema/smithyModel/apiGateway from the enum would not fail this test.

Options:

1. Keep it.each(...) for these two (it's the one pattern where enumeration genuinely catches regressions beyond "zod enum parsing works").
2. Replace with a single assertion against Schema.options (e.g., expect(AgentCoreRegionSchema.options).toEqual([...])) — similar to what the PR already does for MemoryStrategyTypeSchema.

FWIW, the other collapsed enums (SDKFrameworkSchema, ModelProviderSchema) are fine because matchEnumValue tests above already exercise the other values via their lowercase variants.

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	42.89%	8936 / 20830
🔵	Statements	42.17%	9485 / 22491
🔵	Functions	39.66%	1537 / 3875
🔵	Branches	39.82%	5769 / 14486

Generated in workflow #2335 for commit 5c66797 by the Vitest Coverage Report Action

[tejaskash]

Good catch — restored the deploy/dev help flag assertions and the --invoke negative regression guard. The integ smoke test is indeed only exit-code + "Usage:".

[tejaskash]

Addressed: replaced the representative-sample tests with .options assertions for both AgentCoreRegionSchema and GatewayTargetTypeSchema. This catches accidental removal of any enum value without per-value it.each tests.

[jesseturner21]

LGTM

[tejaskash]

Summary

• Collapse it.each per-value enumeration patterns into consolidated tests across 9 schema test files (constants, agent-env, agentcore-project, aws-targets, deployed-state, mcp-defs, mcp, evaluator, memory)
• Remove deploy/dev --help unit tests already covered by integ-tests/help.test.ts
• Remove 5 it.todo placeholders from credential-ops.test.ts

Schema tests: ~506 → ~415. All cross-field validation (superRefine), boundary, and discriminated union tests preserved.

Test plan

• npm test passes — schema subset verified locally (415 tests), full suite should pass in CI
• No coverage regression for src/schema/ (tests collapsed, not deleted — same code paths covered)

[agentcore-cli-automation]

The PR description says these deploy --help / dev --help unit tests are "already covered by integ-tests/help.test.ts", but that isn't quite accurate — integ-tests/help.test.ts is only a smoke test (it checks exit code 0 and the presence of the string "Usage:"). It does not assert on specific flags or options.

Concretely, after this PR nothing verifies:

• deploy --help lists --verbose, --yes, --json, --dry-run
• deploy --help's --verbose description mentions "resource-level"
• dev --help lists [prompt], --port, --runtime, --stream, --logs, and the default port 8080
• dev --help does not show --invoke (see separate comment — this is the only regression guard against reintroducing the removed flag)

A few options:

1. Keep the unit tests as-is and update the PR description to clarify they aren't redundant.
2. Move the flag-presence assertions into integ-tests/help.test.ts (e.g., a table of expected substrings per command) before deleting the unit versions.
3. Drop only the assertions that are genuinely duplicated by the integ smoke test (i.e., none — exit code and "Usage:" aren't what the unit tests were asserting), and keep the flag-level assertions.

[agentcore-cli-automation]

Removing the it('does not show --invoke flag', ...) test in dev.test.ts drops the only regression guard against reintroducing the removed --invoke flag. I grepped the repo and this assertion appears nowhere else:

$ grep -rn "\-\-invoke" src --include='*.ts'
src/cli/commands/dev/__tests__/dev.test.ts:18: it('does not show --invoke flag', async () => {
src/cli/commands/dev/__tests__/dev.test.ts:22: expect(result.stdout.includes('--invoke'), 'Should not show removed --invoke option').toBeFalsy();

Since this is a negative assertion (a flag was removed and shouldn't come back), it's exactly the kind of thing a smoke test can't replace. Please either keep this test or relocate the assertion into integ-tests/help.test.ts.

[agentcore-cli-automation]

The PR description asserts "No coverage regression for src/schema/ (tests collapsed, not deleted — same code paths covered)". That holds for most of the collapsed cases — zod's enum parser is the same code path regardless of which enum value you feed it — but there are two places where the collapse actually narrows coverage in a way I don't think is intended:

aws-targets.test.ts — AgentCoreRegionSchema: Previously enumerated all 16 supported regions; now only asserts on us-east-1, eu-west-1, us-gov-west-1. If someone accidentally drops ap-northeast-1, ca-central-1, sa-east-1, etc. from the schema's region list, nothing here will fail. The explicit enumeration of supported regions is itself the coverage signal.

mcp.test.ts — GatewayTargetTypeSchema: Previously enumerated all 6 types (lambda, mcpServer, openApiSchema, smithyModel, apiGateway, lambdaFunctionArn); now only asserts on lambda and lambdaFunctionArn. Dropping mcpServer/openApiSchema/smithyModel/apiGateway from the enum would not fail this test.

Options:

1. Keep it.each(...) for these two (it's the one pattern where enumeration genuinely catches regressions beyond "zod enum parsing works").
2. Replace with a single assertion against Schema.options (e.g., expect(AgentCoreRegionSchema.options).toEqual([...])) — similar to what the PR already does for MemoryStrategyTypeSchema.

FWIW, the other collapsed enums (SDKFrameworkSchema, ModelProviderSchema) are fine because matchEnumValue tests above already exercise the other values via their lowercase variants.

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	42.89%	8936 / 20830
🔵	Statements	42.17%	9485 / 22491
🔵	Functions	39.66%	1537 / 3875
🔵	Branches	39.82%	5769 / 14486

Generated in workflow #2335 for commit 5c66797 by the Vitest Coverage Report Action

[tejaskash]

Good catch — restored the deploy/dev help flag assertions and the --invoke negative regression guard. The integ smoke test is indeed only exit-code + "Usage:".

[tejaskash]

Addressed: replaced the representative-sample tests with .options assertions for both AgentCoreRegionSchema and GatewayTargetTypeSchema. This catches accidental removal of any enum value without per-value it.each tests.

[jesseturner21]

LGTM

[tejaskash]

Summary

• Collapse it.each per-value enumeration patterns into consolidated tests across 9 schema test files (constants, agent-env, agentcore-project, aws-targets, deployed-state, mcp-defs, mcp, evaluator, memory)
• Remove deploy/dev --help unit tests already covered by integ-tests/help.test.ts
• Remove 5 it.todo placeholders from credential-ops.test.ts

Schema tests: ~506 → ~415. All cross-field validation (superRefine), boundary, and discriminated union tests preserved.

Test plan

• npm test passes — schema subset verified locally (415 tests), full suite should pass in CI
• No coverage regression for src/schema/ (tests collapsed, not deleted — same code paths covered)

[agentcore-cli-automation]

The PR description says these deploy --help / dev --help unit tests are "already covered by integ-tests/help.test.ts", but that isn't quite accurate — integ-tests/help.test.ts is only a smoke test (it checks exit code 0 and the presence of the string "Usage:"). It does not assert on specific flags or options.

Concretely, after this PR nothing verifies:

• deploy --help lists --verbose, --yes, --json, --dry-run
• deploy --help's --verbose description mentions "resource-level"
• dev --help lists [prompt], --port, --runtime, --stream, --logs, and the default port 8080
• dev --help does not show --invoke (see separate comment — this is the only regression guard against reintroducing the removed flag)

A few options:

1. Keep the unit tests as-is and update the PR description to clarify they aren't redundant.
2. Move the flag-presence assertions into integ-tests/help.test.ts (e.g., a table of expected substrings per command) before deleting the unit versions.
3. Drop only the assertions that are genuinely duplicated by the integ smoke test (i.e., none — exit code and "Usage:" aren't what the unit tests were asserting), and keep the flag-level assertions.

[agentcore-cli-automation]

Removing the it('does not show --invoke flag', ...) test in dev.test.ts drops the only regression guard against reintroducing the removed --invoke flag. I grepped the repo and this assertion appears nowhere else:

$ grep -rn "\-\-invoke" src --include='*.ts'
src/cli/commands/dev/__tests__/dev.test.ts:18: it('does not show --invoke flag', async () => {
src/cli/commands/dev/__tests__/dev.test.ts:22: expect(result.stdout.includes('--invoke'), 'Should not show removed --invoke option').toBeFalsy();

Since this is a negative assertion (a flag was removed and shouldn't come back), it's exactly the kind of thing a smoke test can't replace. Please either keep this test or relocate the assertion into integ-tests/help.test.ts.

[agentcore-cli-automation]

The PR description asserts "No coverage regression for src/schema/ (tests collapsed, not deleted — same code paths covered)". That holds for most of the collapsed cases — zod's enum parser is the same code path regardless of which enum value you feed it — but there are two places where the collapse actually narrows coverage in a way I don't think is intended:

aws-targets.test.ts — AgentCoreRegionSchema: Previously enumerated all 16 supported regions; now only asserts on us-east-1, eu-west-1, us-gov-west-1. If someone accidentally drops ap-northeast-1, ca-central-1, sa-east-1, etc. from the schema's region list, nothing here will fail. The explicit enumeration of supported regions is itself the coverage signal.

mcp.test.ts — GatewayTargetTypeSchema: Previously enumerated all 6 types (lambda, mcpServer, openApiSchema, smithyModel, apiGateway, lambdaFunctionArn); now only asserts on lambda and lambdaFunctionArn. Dropping mcpServer/openApiSchema/smithyModel/apiGateway from the enum would not fail this test.

Options:

1. Keep it.each(...) for these two (it's the one pattern where enumeration genuinely catches regressions beyond "zod enum parsing works").
2. Replace with a single assertion against Schema.options (e.g., expect(AgentCoreRegionSchema.options).toEqual([...])) — similar to what the PR already does for MemoryStrategyTypeSchema.

FWIW, the other collapsed enums (SDKFrameworkSchema, ModelProviderSchema) are fine because matchEnumValue tests above already exercise the other values via their lowercase variants.

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	42.89%	8936 / 20830
🔵	Statements	42.17%	9485 / 22491
🔵	Functions	39.66%	1537 / 3875
🔵	Branches	39.82%	5769 / 14486

Generated in workflow #2335 for commit 5c66797 by the Vitest Coverage Report Action

[tejaskash]

Good catch — restored the deploy/dev help flag assertions and the --invoke negative regression guard. The integ smoke test is indeed only exit-code + "Usage:".

[tejaskash]

Addressed: replaced the representative-sample tests with .options assertions for both AgentCoreRegionSchema and GatewayTargetTypeSchema. This catches accidental removal of any enum value without per-value it.each tests.

[jesseturner21]

LGTM

[tejaskash]

Summary

• Collapse it.each per-value enumeration patterns into consolidated tests across 9 schema test files (constants, agent-env, agentcore-project, aws-targets, deployed-state, mcp-defs, mcp, evaluator, memory)
• Remove deploy/dev --help unit tests already covered by integ-tests/help.test.ts
• Remove 5 it.todo placeholders from credential-ops.test.ts

Schema tests: ~506 → ~415. All cross-field validation (superRefine), boundary, and discriminated union tests preserved.

Test plan

• npm test passes — schema subset verified locally (415 tests), full suite should pass in CI
• No coverage regression for src/schema/ (tests collapsed, not deleted — same code paths covered)

[agentcore-cli-automation]

The PR description says these deploy --help / dev --help unit tests are "already covered by integ-tests/help.test.ts", but that isn't quite accurate — integ-tests/help.test.ts is only a smoke test (it checks exit code 0 and the presence of the string "Usage:"). It does not assert on specific flags or options.

Concretely, after this PR nothing verifies:

• deploy --help lists --verbose, --yes, --json, --dry-run
• deploy --help's --verbose description mentions "resource-level"
• dev --help lists [prompt], --port, --runtime, --stream, --logs, and the default port 8080
• dev --help does not show --invoke (see separate comment — this is the only regression guard against reintroducing the removed flag)

A few options:

1. Keep the unit tests as-is and update the PR description to clarify they aren't redundant.
2. Move the flag-presence assertions into integ-tests/help.test.ts (e.g., a table of expected substrings per command) before deleting the unit versions.
3. Drop only the assertions that are genuinely duplicated by the integ smoke test (i.e., none — exit code and "Usage:" aren't what the unit tests were asserting), and keep the flag-level assertions.

[agentcore-cli-automation]

Removing the it('does not show --invoke flag', ...) test in dev.test.ts drops the only regression guard against reintroducing the removed --invoke flag. I grepped the repo and this assertion appears nowhere else:

$ grep -rn "\-\-invoke" src --include='*.ts'
src/cli/commands/dev/__tests__/dev.test.ts:18: it('does not show --invoke flag', async () => {
src/cli/commands/dev/__tests__/dev.test.ts:22: expect(result.stdout.includes('--invoke'), 'Should not show removed --invoke option').toBeFalsy();

Since this is a negative assertion (a flag was removed and shouldn't come back), it's exactly the kind of thing a smoke test can't replace. Please either keep this test or relocate the assertion into integ-tests/help.test.ts.

[agentcore-cli-automation]

The PR description asserts "No coverage regression for src/schema/ (tests collapsed, not deleted — same code paths covered)". That holds for most of the collapsed cases — zod's enum parser is the same code path regardless of which enum value you feed it — but there are two places where the collapse actually narrows coverage in a way I don't think is intended:

aws-targets.test.ts — AgentCoreRegionSchema: Previously enumerated all 16 supported regions; now only asserts on us-east-1, eu-west-1, us-gov-west-1. If someone accidentally drops ap-northeast-1, ca-central-1, sa-east-1, etc. from the schema's region list, nothing here will fail. The explicit enumeration of supported regions is itself the coverage signal.

mcp.test.ts — GatewayTargetTypeSchema: Previously enumerated all 6 types (lambda, mcpServer, openApiSchema, smithyModel, apiGateway, lambdaFunctionArn); now only asserts on lambda and lambdaFunctionArn. Dropping mcpServer/openApiSchema/smithyModel/apiGateway from the enum would not fail this test.

Options:

1. Keep it.each(...) for these two (it's the one pattern where enumeration genuinely catches regressions beyond "zod enum parsing works").
2. Replace with a single assertion against Schema.options (e.g., expect(AgentCoreRegionSchema.options).toEqual([...])) — similar to what the PR already does for MemoryStrategyTypeSchema.

FWIW, the other collapsed enums (SDKFrameworkSchema, ModelProviderSchema) are fine because matchEnumValue tests above already exercise the other values via their lowercase variants.

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	42.89%	8936 / 20830
🔵	Statements	42.17%	9485 / 22491
🔵	Functions	39.66%	1537 / 3875
🔵	Branches	39.82%	5769 / 14486

Generated in workflow #2335 for commit 5c66797 by the Vitest Coverage Report Action

[tejaskash]

Good catch — restored the deploy/dev help flag assertions and the --invoke negative regression guard. The integ smoke test is indeed only exit-code + "Usage:".

[tejaskash]

Addressed: replaced the representative-sample tests with .options assertions for both AgentCoreRegionSchema and GatewayTargetTypeSchema. This catches accidental removal of any enum value without per-value it.each tests.

[jesseturner21]

LGTM