fix(agent): fix enable user input bug

[aws-rafams]

Fixes #626

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the project license.

[aws-rafams]

I have understood what is the problem here, when enableUserInput: true, this action group is visible on the console under Agent builder -> Additional Settings -> User Input. This part gets created by this piece of code here:

generative-ai-cdk-constructs/src/cdk-lib/bedrock/agent.ts

Lines 492 to 498 in 6b5bbeb

	if (props.enableUserInput) {
	this.addActionGroup(new AgentActionGroup(this, 'userInputEnabledActionGroup', {
	actionGroupName: 'UserInputAction',
	parentActionGroupSignature: 'AMAZON.UserInput',
	actionGroupState: 'ENABLED',
	}));
	}

the problem is that when the update happens, this content of this if statement is not executed and thus the ActionGroup is not synthesized, thus disappearing from the template and triggering DELETE operation, however, this delete operation is not allowed, because it should either be set to DISABLED (an update to the ActionGroup resource), or skipResourceInUseCheckOnDelete must be set to true. I believe the easiest solution to this bug is to set:

+      skipResourceInUseCheckOnDelete: true,

during the ActionGroup Creation as in PR #642. Otherwise the other solution would be to always create the action group and according to whether it must be enabled, set the actionGroupState accordingly:

this.addActionGroup(new AgentActionGroup(this, 'userInputEnabledActionGroup', {
    actionGroupName: 'UserInputAction',
    parentActionGroupSignature: 'AMAZON.UserInput',
    actionGroupState: props.enableUserInput ? 'ENABLED' :  'DISABLED',
}));

[scottschreckengaust]

How might the second option operate?

actionGroupState: props.enableUserInput ? 'ENABLED' :  'DISABLED',

[aws-rafams]

props.enableUserInput ? 'ENABLED' :  'DISABLED',

I have tested both solutions, and they are both functional. However, there is a subtle difference in their behaviour. With the second approach, the action group is always created (as demonstrated in the screenshot below), and the only variation lies in whether it is enabled or disabled.

In any case this action group will not be shown in the console as an action group but rather as a configuration parameter on the agent.

[scottschreckengaust]

props.enableUserInput ? 'ENABLED' :  'DISABLED',

I have tested both solutions, and they are both functional. However, there is a subtle difference in their behaviour. With the second approach, the action group is always created (as demonstrated in the screenshot below), and the only variation lies in whether it is enabled or disabled.

Thank you very much. Upon review, would you mind updating for the second option?

[dineshSajwan]

@aws-rafams . Thank you so much for suggesting the fix. It looks good to me however option 2 by Scott also make sense. once you create the userinput on the console you can enable or disable it. With option 1, Can we please verify that CDK destroy is removing all the resources with SkipResourceInUseCheckOnDelete set to true.

[aws-rafams]

I have switched to option two
https://github.com/aws-rafams/generative-ai-cdk-constructs/blob/de7078bb4cf8eabfdb1686497ddac68d54244e02/src/cdk-lib/bedrock/agent.ts#L492-L496

[scottschreckengaust]

Please review test results (specifically the agent.test.ts). Thank you.

% npx projen test --testPathPattern='.*agent\.test.*'
...
  Agent with guardrails through addGuardrail
    ✓ Knowledge Base is created (105 ms)
    ✓ Data Source is created (1 ms)
    ✓ Agent is created (1 ms)
    ✓ Agent is created with one knowledge base (1 ms)
    ✕ Agent action group and ApiSchema from S3 (2 ms)
    ✓ Guardrail is associated (17 ms)
    ✓ Agent Alias is created (1 ms)
    ✓ No unsuppressed Errors (58 ms)
  Agent with guardrails through constructor
    ✓ Knowledge Base is created (48 ms)
    ✓ Data Source is created (1 ms)
    ✓ Agent is created (1 ms)
    ✓ Agent is created with one knowledge base
    ✕ Agent action group and ApiSchema from S3 (1 ms)
    ✓ Guardrail is associated (3 ms)
    ✓ Agent Alias is created (1 ms)
    ✓ No unsuppressed Errors (46 ms)
  Agent without guardrails
    ✓ Knowledge Base is created (47 ms)
    ✓ Data Source is created (1 ms)
    ✓ Agent is created (1 ms)
    ✓ Agent is created with one knowledge base
    ✕ Agent action group and ApiSchema from S3 (1 ms)
    ✓ Guardrail should not be associated (1 ms)
    ✓ Agent Alias is created (1 ms)
    ✓ No unsuppressed Errors (44 ms)
...

[aws-rafams]

tests have been updated

[dineshSajwan]

LGTM

[aws-rafams]

is anything missing for this PR to be merged?

[krokoko]

I will just test it today in my account and if everything is good will approve, thanks @aws-rafams !

[krokoko]

Tested by deploying an agent and following the steps described in the bug ticket, works as expected. Thanks @aws-rafams for your contribution!