diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/agent/api/annotation/support/AgentMetadataReader.kt b/embabel-agent-api/src/main/kotlin/com/embabel/agent/api/annotation/support/AgentMetadataReader.kt index c70083b1e..4c07c8eed 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/agent/api/annotation/support/AgentMetadataReader.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/agent/api/annotation/support/AgentMetadataReader.kt @@ -27,15 +27,16 @@ import com.embabel.agent.core.Export import com.embabel.agent.core.support.NIRVANA import com.embabel.agent.core.support.Rerun import com.embabel.agent.core.support.safelyGetToolsFrom +import com.embabel.agent.spi.validation.AchievableGoalValidator import com.embabel.agent.spi.validation.AgentStructureAgentValidator import com.embabel.agent.spi.validation.DefaultAgentValidationManager import com.embabel.agent.spi.validation.GoapPathToCompletionValidator import com.embabel.agent.spi.validation.PathToCompletionAgentValidator +import com.embabel.agent.spi.validation.isActionMethod +import com.embabel.agent.spi.validation.isConditionMethod +import com.embabel.agent.spi.validation.isMethodFromSupertype import com.embabel.common.core.types.Semver import com.embabel.common.util.NameUtils -import com.embabel.common.util.loggerFor -import com.fasterxml.jackson.annotation.JsonTypeInfo -import tools.jackson.databind.annotation.JsonDeserialize import org.slf4j.LoggerFactory import org.springframework.beans.factory.annotation.Value import org.springframework.cglib.proxy.Enhancer @@ -110,23 +111,20 @@ internal data class AgenticInfo( class AgentMetadataReader( private val actionMethodManager: ActionMethodManager = DefaultActionMethodManager(), private val nameGenerator: MethodDefinedOperationNameGenerator = MethodDefinedOperationNameGenerator(), - agentStructureValidator: AgentStructureAgentValidator = AgentStructureAgentValidator.PERMIT_ALL, - pathToCompletionValidator: PathToCompletionAgentValidator = GoapPathToCompletionValidator(), + private val agentStructureValidator: AgentStructureAgentValidator = AgentStructureAgentValidator.PERMIT_ALL, + private val pathToCompletionValidator: PathToCompletionAgentValidator = GoapPathToCompletionValidator(), private val requireInterfaceDeserializationAnnotations: Boolean = false, @Value("\${embabel.agent.platform.planner.restricted-goals:false}") private val restrictedGoals: Boolean = false, + @Value("\${embabel.agent.api.validation.manager.skip-agent-deployment-on-error:false}") + private val skipAgentDeploymentOnError: Boolean = false, ) { private val supervisorAgentFactory = SupervisorAgentFactory() private val logger = LoggerFactory.getLogger(AgentMetadataReader::class.java) - private val agentValidationManager: AgentValidationManager = DefaultAgentValidationManager( - listOf( - agentStructureValidator, - pathToCompletionValidator - ) - ) + private lateinit var agentValidationManager: AgentValidationManager fun createAgentScopes(vararg instances: Any): List = instances.mapNotNull { createAgentMetadata(it) } @@ -152,13 +150,14 @@ class AgentMetadataReader( val targetType = agenticInfo.getTargetType() if (!agenticInfo.agentic()) { - logger.debug( + logger.warn( "No @{} or @{} annotation found on {}", EmbabelComponent::class.simpleName, Agent::class.simpleName, targetType.name, ) - return null + // Don't put this behind skipAgentDeploymentOnError as any bean can be reached here. + return null } if (agenticInfo.validationErrors().isNotEmpty()) { @@ -168,10 +167,21 @@ class AgentMetadataReader( Agent::class.simpleName, targetType.name, ) - return null + if (skipAgentDeploymentOnError) { + return null + } } rejectOperationContextConstructorInjection(targetType) + val plannerType = agenticInfo.agentAnnotation?.planner ?: PlannerType.GOAP + agentValidationManager = DefaultAgentValidationManager( + listOf( + agentStructureValidator, + pathToCompletionValidator, + AchievableGoalValidator(agenticInfo.agentName(), targetType, instance, requireInterfaceDeserializationAnnotations) + ) + ) + val getterGoals = findGoalGetters(targetType).map { getGoal(it, instance) } val actionMethods = findActionMethods(targetType) val conditionMethods = findConditionMethods(targetType) @@ -204,8 +214,6 @@ class AgentMetadataReader( ) } - val plannerType = agenticInfo.agentAnnotation?.planner ?: PlannerType.GOAP - val goals = buildSet { addAll(getterGoals) addAll(allGoals) @@ -222,27 +230,23 @@ class AgentMetadataReader( Condition::class.simpleName, targetType.name, ) - return null + if (skipAgentDeploymentOnError) { + return null + } } val agent = if (agenticInfo.agentAnnotation != null) { val goalActions = actionMethods.filter { it.isAnnotationPresent(AchievesGoal::class.java) } if (plannerType == PlannerType.SUPERVISOR) { - // Find the goal action (the action with @AchievesGoal) - if (goalActions.isEmpty()) { - logger.warn( - "SUPERVISOR planner requires at least one @AchievesGoal action on {}", - targetType.name, - ) - return null - } if (goalActions.size > 1) { logger.warn( "SUPERVISOR planner currently supports only one @AchievesGoal action, found {} on {}", goalActions.size, targetType.name, ) - return null + if (skipAgentDeploymentOnError) { + return null + } } val goalAction = allActions.find { action -> goalActions.any { method -> @@ -304,8 +308,9 @@ class AgentMetadataReader( val validationResult = agentValidationManager.validate(agent) if (!validationResult.isValid) { logger.warn("Agent validation failed:\n${validationResult.errors.joinToString("\n")}") - // TODO: Uncomment to strengthen validation and refactor the test if needed. Because some tests might fail. - // return null + if (skipAgentDeploymentOnError) { + return null + } } } @@ -340,7 +345,7 @@ class AgentMetadataReader( stateClass, ) allActions.add(action) - createGoalFromStateActionMethod(actionMethod, action, stateClass, agentInstance)?.let { + createGoalFromStateActionMethod(actionMethod, action, stateClass)?.let { allGoals.add(it) } // Recursively unroll if this action also returns a @State type @@ -431,7 +436,6 @@ class AgentMetadataReader( method: Method, action: CoreAction, stateClass: Class<*>, - agentInstance: Any, ): AgentCoreGoal? { val actionAnnotation = method.getAnnotation(Action::class.java) val goalAnnotation = method.getAnnotation(AchievesGoal::class.java) ?: return null @@ -511,68 +515,13 @@ class AgentMetadataReader( type, { method -> actionMethods.add(method) }, // Get annotated methods from this type and interfaces - { method -> isActionMethod(method, type) }) + { method -> isActionMethod(logger,method, type, requireInterfaceDeserializationAnnotations) }) if (actionMethods.isEmpty()) { logger.debug("No methods annotated with @{} found in {}", Action::class.simpleName, type) } return actionMethods } - private fun isActionMethod( - method: Method, - type: Class<*>, - ): Boolean { - return method.isAnnotationPresent(Action::class.java) && - (type.declaredMethods.contains(method) || isMethodFromSupertype(method, type)) && - (!method.returnType.isInterface || !requireInterfaceDeserializationAnnotations || hasRequiredJsonDeserializeAnnotationOnInterfaceReturnType( - method - )) - } - - private fun isConditionMethod( - method: Method, - type: Class<*>, - ): Boolean { - return method.isAnnotationPresent(Condition::class.java) && - (type.declaredMethods.contains(method) || isMethodFromSupertype(method, type)) - } - - private fun isMethodFromSupertype( - method: Method, - type: Class<*>, - ): Boolean { - // Check interfaces - if (type.interfaces.any { interfaceType -> - interfaceType.declaredMethods.any { interfaceMethod -> - methodSignaturesMatch(method, interfaceMethod) - } - }) { - return true - } - - // Check superclasses - var superclass = type.superclass - while (superclass != null && superclass != Any::class.java) { - if (superclass.declaredMethods.any { superMethod -> - methodSignaturesMatch(method, superMethod) - }) { - return true - } - superclass = superclass.superclass - } - - return false - } - - private fun methodSignaturesMatch( - method1: Method, - method2: Method, - ): Boolean { - return method1.name == method2.name && - method1.parameterTypes.contentEquals(method2.parameterTypes) && - method1.returnType == method2.returnType - } - private fun findGoalGetters(type: Class<*>): List { val goalGetters = mutableListOf() type.declaredMethods.forEach { method -> @@ -781,22 +730,3 @@ private fun rejectOperationContextConstructorInjection(agentClass: Class<*>) { ) } } - -/** - * Checks if a method returning an interface returns a type with a @JsonDeserialize annotation. - * @param method The Java method to check. - * @return true if the return type has a @JsonDeserialize annotation, false otherwise - */ -private fun hasRequiredJsonDeserializeAnnotationOnInterfaceReturnType(method: Method): Boolean { - val hasRequiredAnnotation = method.returnType.isAnnotationPresent(JsonDeserialize::class.java) || - method.returnType.isAnnotationPresent(JsonTypeInfo::class.java) - if (!hasRequiredAnnotation) { - loggerFor().warn( - "❓Interface {} used as return type of {}.{} must have @JsonDeserialize or @JsonTypeInfo annotation", - method.returnType.name, - method.declaringClass.name, - method.name, - ) - } - return hasRequiredAnnotation -} diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/validation/AchievableGoalValidator.kt b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/validation/AchievableGoalValidator.kt new file mode 100644 index 000000000..a41bcf3a3 --- /dev/null +++ b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/validation/AchievableGoalValidator.kt @@ -0,0 +1,57 @@ +package com.embabel.agent.spi.validation + +import com.embabel.agent.api.annotation.AchievesGoal +import com.embabel.agent.core.AgentScope +import com.embabel.common.core.validation.ValidationError +import com.embabel.common.core.validation.ValidationErrorCodes +import com.embabel.common.core.validation.ValidationLocation +import com.embabel.common.core.validation.ValidationResult +import com.embabel.common.core.validation.ValidationSeverity +import org.slf4j.LoggerFactory +import org.springframework.util.ReflectionUtils +import java.lang.reflect.Method + +/** + * Validator that checks methods annotated with AchievesGoal. + * + * Specific check includes: + * - Verifying that @Action annotation is present on it. + */ +open class AchievableGoalValidator ( private val agentName: String, + private val agentClass: Class<*>, + private val agentInstance: Any, + private val requireInterfaceDeserializationAnnotations: Boolean): AgentValidator +{ + private val logger = LoggerFactory.getLogger(AchievableGoalValidator::class.java) + + private fun isMethodAnnotatedWithAchievesGoal( + method: Method, + ): Boolean { + return method.isAnnotationPresent(AchievesGoal::class.java) + } + + override fun validate(agentScope: AgentScope): ValidationResult { + val errors = mutableListOf() + ReflectionUtils.doWithMethods( + agentClass, + { method -> + if(!isActionMethod(logger,method, agentClass, requireInterfaceDeserializationAnnotations)) { + errors.add( + ValidationError( + code = ValidationErrorCodes.MISSING_ACTION_ANNOTATION, + message = "@Action annotation is missing on the method '${agentInstance.javaClass.name}.${method.name}' annotated with @AchievesGoal.", + severity = ValidationSeverity.ERROR, + location = ValidationLocation( + type = "Agent", + name = agentInstance.javaClass.name, + agentName = agentName, + component = method.name + ) + ) + ) + } + }, + { method -> isMethodAnnotatedWithAchievesGoal(method) }) + return ValidationResult(errors.isEmpty(), errors) + } +} diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/validation/ValidatorUtils.kt b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/validation/ValidatorUtils.kt new file mode 100644 index 000000000..2379a29a7 --- /dev/null +++ b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/validation/ValidatorUtils.kt @@ -0,0 +1,108 @@ +package com.embabel.agent.spi.validation + +import com.embabel.agent.api.annotation.Action +import com.embabel.agent.api.annotation.Condition +import com.fasterxml.jackson.annotation.JsonTypeInfo +import com.fasterxml.jackson.databind.annotation.JsonDeserialize +import org.slf4j.Logger +import org.springframework.util.ClassUtils +import java.lang.reflect.Method + +/** + * Returns true, if the given method is + * - annotated with Action and + * - declared in the given agent class, or in it's super type. + * - TODO - + */ +fun isActionMethod( + logger: Logger, + method: Method, + agentClass: Class<*>, + requireInterfaceDeserializationAnnotations : Boolean, +): Boolean { + // annotated with Action ? + return method.isAnnotationPresent(Action::class.java) && + // declared in the given agent class, or in its super type? + (agentClass.declaredMethods.contains(method) || isMethodFromSupertype(method, agentClass)) && + // TODO please fill after discussion. + (!method.returnType.isInterface || !requireInterfaceDeserializationAnnotations || hasRequiredJsonDeserializeAnnotationOnInterfaceReturnType( + method, + logger + )) +} + +/** + * Returns true, if the given method is declared in its super type. + */ +fun isMethodFromSupertype( + method: Method, + type: Class<*>, +): Boolean { + // Check interfaces + if (type.interfaces.any { interfaceType -> + interfaceType.declaredMethods.any { interfaceMethod -> + methodSignaturesMatch(method, interfaceMethod) + } + }) { + return true + } + + // Check superclasses + var superclass = type.superclass + while (superclass != null && superclass != Any::class.java) { + if (superclass.declaredMethods.any { superMethod -> + methodSignaturesMatch(method, superMethod) + }) { + return true + } + superclass = superclass.superclass + } + + return false +} + +private fun methodSignaturesMatch( + method1: Method, + method2: Method, +): Boolean { + // Finds if method2 matches method1's name and parameter types. + val match = ClassUtils.getMethodIfAvailable( + method2.declaringClass, + method1.name, + *method1.parameterTypes + ) + return match == method2 && + method1.returnType == method2.returnType +} + +/** + * Checks if a method returning an interface returns a type with a @JsonDeserialize annotation. + * @param method The Java method to check. + * @return true if the return type has a @JsonDeserialize annotation, false otherwise + */ +private fun hasRequiredJsonDeserializeAnnotationOnInterfaceReturnType(method: Method, logger: Logger): Boolean { + val hasRequiredAnnotation = method.returnType.isAnnotationPresent(JsonDeserialize::class.java) || + method.returnType.isAnnotationPresent(JsonTypeInfo::class.java) + if (!hasRequiredAnnotation) { + logger.warn( + "❓Interface {} used as return type of {}.{} must have @JsonDeserialize or @JsonTypeInfo annotation", + method.returnType.name, + method.declaringClass.name, + method.name, + ) + } + return hasRequiredAnnotation +} + +/** + * Returns true, if the given method is + * - annotated with Condition and + * - declared in the given agent class, or in its super type. + */ +fun isConditionMethod( + method: Method, + agentClass: Class<*>, +): Boolean { + return method.isAnnotationPresent(Condition::class.java) && + (agentClass.declaredMethods.contains(method) || isMethodFromSupertype(method, agentClass)) +} diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/common/core/validation/ValidationErrorCodes.kt b/embabel-agent-api/src/main/kotlin/com/embabel/common/core/validation/ValidationErrorCodes.kt index 2459f9d6f..48c2a6249 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/common/core/validation/ValidationErrorCodes.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/common/core/validation/ValidationErrorCodes.kt @@ -37,5 +37,8 @@ class ValidationErrorCodes { /** Agent has goals but no actions, so no plan can ever reach them. */ const val NO_ACTIONS_TO_GOALS = "NO_ACTIONS_TO_GOALS" + + /** Action annotation is missing on the method. */ + const val MISSING_ACTION_ANNOTATION = "MISSING_ACTION_ANNOTATION" } } diff --git a/embabel-agent-api/src/test/kotlin/com/embabel/agent/api/annotation/support/AgentMetadataReaderMetadataTest.kt b/embabel-agent-api/src/test/kotlin/com/embabel/agent/api/annotation/support/AgentMetadataReaderMetadataTest.kt index e5908f3f4..bae6e3798 100644 --- a/embabel-agent-api/src/test/kotlin/com/embabel/agent/api/annotation/support/AgentMetadataReaderMetadataTest.kt +++ b/embabel-agent-api/src/test/kotlin/com/embabel/agent/api/annotation/support/AgentMetadataReaderMetadataTest.kt @@ -42,14 +42,14 @@ class AgentMetadataReaderMetadataTest { @Test fun `no annotation`() { - val reader = AgentMetadataReader() - val metadata = reader.createAgentMetadata(PersonWithReverseTool("John Doe")) + val reader = AgentMetadataReader(skipAgentDeploymentOnError = true) + val metadata = reader.createAgentMetadata(PersonWithReverseTool("John Doe"), ) assertNull(metadata) } @Test fun `no methods`() { - val reader = AgentMetadataReader() + val reader = AgentMetadataReader(skipAgentDeploymentOnError = true) assertNull(reader.createAgentMetadata(NoMethods())) } @@ -60,7 +60,7 @@ class AgentMetadataReaderMetadataTest { @Test fun `invalid action signature returning interface without serialization annotation with check`() { - val reader = AgentMetadataReader(requireInterfaceDeserializationAnnotations = true) + val reader = AgentMetadataReader(requireInterfaceDeserializationAnnotations = true, skipAgentDeploymentOnError = true) assertNull(reader.createAgentMetadata(InvalidActionNoDeserializationInInterfaceGoal())) } diff --git a/embabel-agent-api/src/test/kotlin/com/embabel/agent/api/annotation/support/testTypes.kt b/embabel-agent-api/src/test/kotlin/com/embabel/agent/api/annotation/support/testTypes.kt index 112f132cf..8b7f7e1d0 100644 --- a/embabel-agent-api/src/test/kotlin/com/embabel/agent/api/annotation/support/testTypes.kt +++ b/embabel-agent-api/src/test/kotlin/com/embabel/agent/api/annotation/support/testTypes.kt @@ -869,3 +869,23 @@ class AgentWithOperationContextConstructorInjection( @Action fun act(input: UserInput): PersonWithReverseTool = PersonWithReverseTool(input.content) } + +@Agent(description = "goal method is not annotated with Action") +class AgentWithAchievesGoalNoActionAnnotation { + @Action + fun makeFrogFromPerson(userInput: UserInput): Frog { + return Frog(userInput.content) + } + + @AchievesGoal(description = "goal") + fun goal(frog: Frog): PersonWithReverseTool = PersonWithReverseTool(frog.name) +} + +@Agent(description = "valid goal method") +class AgentWithValidAchievesGoalMethod { + @Action + @AchievesGoal(description = "goal") + fun goal(input: UserInput): String { + return "dummy" + } +} diff --git a/embabel-agent-api/src/test/kotlin/com/embabel/agent/validation/AchievableGoalValidatorTest.kt b/embabel-agent-api/src/test/kotlin/com/embabel/agent/validation/AchievableGoalValidatorTest.kt new file mode 100644 index 000000000..335fc73da --- /dev/null +++ b/embabel-agent-api/src/test/kotlin/com/embabel/agent/validation/AchievableGoalValidatorTest.kt @@ -0,0 +1,44 @@ +package com.embabel.agent.validation + +import com.embabel.agent.api.annotation.support.AgentMetadataReader +import com.embabel.agent.api.annotation.support.AgentWithAchievesGoalNoActionAnnotation +import com.embabel.agent.api.annotation.support.AgentWithValidAchievesGoalMethod +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertNotNull +import org.junit.jupiter.api.assertNull +import org.junit.jupiter.api.extension.ExtendWith +import org.springframework.boot.test.system.CapturedOutput +import org.springframework.boot.test.system.OutputCaptureExtension +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +@ExtendWith(OutputCaptureExtension::class) +class AchievableGoalValidatorTest { + val noActionErrorMessage = """@Action annotation is missing on the method 'com.embabel.agent.api.annotation.support.AgentWithAchievesGoalNoActionAnnotation.goal' annotated with @AchievesGoal.""" + + @Test + fun `no Action annotation on AchievesGoal method and skip-agent-on-error is false`(output: CapturedOutput) { + val reader = AgentMetadataReader() + val agentScope = reader.createAgentMetadata(AgentWithAchievesGoalNoActionAnnotation()) + assertNotNull(agentScope, "Validation error is unexpectedly not ignored.") + assertTrue(output.out.contains(noActionErrorMessage), "Error message about missing @Action is absent.") + } + + @Test + fun `no Action annotation on AchievesGoal method but skip-agent-on-error is true`(output: CapturedOutput) { + val reader = AgentMetadataReader(skipAgentDeploymentOnError = true) + val agentScope = reader.createAgentMetadata(AgentWithAchievesGoalNoActionAnnotation()) + assertNull(agentScope, "Validation error is unexpectedly ignored.") + assertTrue(output.out.contains(noActionErrorMessage), "Error message about missing @Action is absent.") + } + + @Test + fun `valid goal method`(output: CapturedOutput) { + val reader = AgentMetadataReader() + reader.createAgentMetadata(AgentWithValidAchievesGoalMethod()) + assertFalse( + output.out.contains(noActionErrorMessage), + "Error message about mission @Action is unexpectedly present." + ) + } +} diff --git a/embabel-agent-docs/src/main/asciidoc/reference/configuration/page.adoc b/embabel-agent-docs/src/main/asciidoc/reference/configuration/page.adoc index c59fe26c9..e5a5e221a 100644 --- a/embabel-agent-docs/src/main/asciidoc/reference/configuration/page.adoc +++ b/embabel-agent-docs/src/main/asciidoc/reference/configuration/page.adoc @@ -194,6 +194,11 @@ Controls how the GOAP planner processes annotated agents during startup. |`false` |When `false` (default), allows `@Agent` to declare multiple `@AchievesGoal` actions with different distinct return types, enabling the planner to gate actions by mutually exclusive `@Condition` annotations. When `true`, restricts to a single unique return type across all `@AchievesGoal` actions; any `@Agent` violating this is rejected at startup. +|`embabel.agent.api.validation.manager.skip-agent-deployment-on-error` +|Boolean +|`false` +|When `false` (default), validation manager only logs structural errors of `@Agent`s. When `true`, any invalid `@Agent` reported by validation manager is rejected at startup. + |=== .Example - opt-in restriction for strict single-goal enforcement