fix validation responsibility boundaries

[arnabnandy7]

Summary

Fixes #1991 by separating validation, logging, and deployment-policy responsibilities.

• Return GOAL_ACTION_NOT_FOUND when a goal has no matching action.
• Keep validation error logging within DefaultAgentValidationManager.
• Remove duplicate validation error logging from AgentMetadataReader.
• Preserve permissive deployment by default.
• Ensure skip-agent-deployment-on-error=true rejects agents with missing goal actions.
• Clarify the validation-manager configuration documentation.
• Add regression coverage for structured errors, strict rejection, and single logging.

Testing

./mvnw -pl embabel-agent-api -Dtest=AchievableGoalValidatorTest test

Validation flow

1. AgentMetadataReader creates the AgentScope.
2. Validators return structured validation errors.
3. DefaultAgentValidationManager aggregates and logs the errors.
4. AgentMetadataReader applies skip-agent-deployment-on-error.

DefaultAgentStructureValidator retains its lifecycle check at DEBUG level, while DefaultAgentValidationManager is the final validation-error reporter.

Scenario verification

The validation flow was exercised using real annotated agent instances and the in-memory AgentPlatform.

• Valid agents deploy under permissive and strict policies.
• Invalid annotated agents remain deployable under the default permissive policy.
• Invalid annotated agents are rejected when skip-agent-deployment-on-error=true.
• MISSING_ACTION_ANNOTATION is logged exactly once.
• An assembled goal without a corresponding action returns and logs GOAL_ACTION_NOT_FOUND exactly once.
• The DefaultAgentStructureValidator lifecycle check remains operational.

Tests run: 11, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

Command:

mvn -pl embabel-agent-api "-Dtest=AgentValidationDeploymentScenarioTest,AchievableGoalValidatorTest,DefaultAgentStructureValidatorTest,DefaultAgentValidationManagerTest" test

[igordayen]

@arnabnandy7 - few comments to consider, thank you

[igordayen]

@arnabnandy7 - please also cross-check for double-logging

[arnabnandy7]

@arnabnandy7 - please also cross-check for double-logging

@igordayen I cross-checked the validation paths and found one additional validator-owned logging path in DefaultAgentStructureValidator.afterPropertiesSet(). It independently reported empty agent structures, which could later be reported again through the validation manager.

I’ve removed that lifecycle logging and its now-unused application-context dependency. Structural validators now return errors through validate(), and DefaultAgentValidationManager remains the single validation-error logging boundary.

[igordayen]

@arnabnandy7 - looks good. few inquiries:

• removal of lifecycle afterProperties , required testing well beypond unit tests on real agents. Understood , intent to eliminate double logging, but would not seggregation by log levels mitigate the same?
• document in PR invocation flow: chain of collection errors components, with final component as reporter
• new error code requires simulation of behavior on real agents.
  Thank you

[deleSerna]

AS @igordayen mentioned please test with a sample agent application all the scenarios to make sure it is working as expected.

[arnabnandy7]

• removal of lifecycle afterProperties , required testing well beypond unit tests on real agents. Understood , intent to eliminate double logging, but would not seggregation by log levels mitigate the same?

Without proof that every deployment goes through AgentMetadataReader, removing the lifecycle check could introduce a behavioral change. I’ve restored afterPropertiesSet() and changed its diagnostic to DEBUG.

This preserves the existing lifecycle coverage while ensuring validation errors are reported at ERROR only by DefaultAgentValidationManager.

• document in PR invocation flow: chain of collection errors components, with final component as reporter

Added invocation flow for clarity:

AgentMetadataReader
  → creates the AgentScope
  → invokes DefaultAgentValidationManager
  → validators return structured ValidationResult errors
  → DefaultAgentValidationManager aggregates and logs the errors
  → AgentMetadataReader applies skip-agent-deployment-on-error

• new error code requires simulation of behavior on real agents.

The focused tests cover the validator and deployment-policy paths, but the real sample-agent scenarios should also be exercised before merge. I’ve included the relevant verification command and will report the results separately rather than claiming coverage from unit tests alone.

[arnabnandy7]

AS @igordayen mentioned please test with a sample agent application all the scenarios to make sure it is working as expected.

@deleSerna the scenarios to verify are permissive deployment, strict rejection, a valid goal/action pair, and a goal without a corresponding action. I haven’t run the sample-application verification yet. I’ll share the exact results once it has been executed.

[deleSerna]

Looks good to me.

[arnabnandy7]

As requested, I tested the validation scenarios using real annotated agent instances and the repository’s in-memory AgentPlatform.

Verified:

• valid agent deployment with both permissive and strict policies;
• permissive deployment of an invalid annotated agent;
• rejection of that agent when skip-agent-deployment-on-error=true;
• single reporting of MISSING_ACTION_ANNOTATION;
• structured return and single reporting of GOAL_ACTION_NOT_FOUND;
• preservation of the structure validator lifecycle check.

Result: 11 tests run, 0 failures, 0 errors, 0 skipped. Build completed successfully.

cc @igordayen @deleSerna

[igordayen]

Result: 11 tests run, 0 failures, 0 errors, 0 skipped. Build completed successfully.

@arnabnandy7 - excellent!

[igordayen]

@arnabnandy7 - citing from Discord, from @tuannx

"
quick heads-up on PR #1996 before it lands. The logging separation and deployment flow look great, but while verifying against issue #1991, I found a regression in GoapPathToCompletionValidator:

It currently assumes every goal must match an action name 1:1. That breaks condition/output-based goals like in our DSL (splitGarden, evenMoreEvilWizard) and goal getters. For instance, splitGarden executes successfully to completion at runtime, but the validator rejects it with GOAL_ACTION_NOT_FOUND. evenMoreEvilWizard in DefaultAgentValidationManagerTest was also flipped to assertFalse to match this.

Could we hold off merging for a bit to fix or let me know if you need any repro details from my side.

"

Please provide details on reproducing this. thanks.

looping @deleSerna

[arnabnandy7]

@igordayen @tuannx fix applied, you may validate on latest changes.

[tuannx]

@igordayen @tuannx fix applied, you may validate on latest changes.

Thanks @arnabnandy7 for the quick and clean fix! Validated locally against both DSL agents (splitGarden, evenMoreEvilWizard) and the unmapped goal scenario — all test cases pass and the validation boundaries align nicely.

LGTM from my side!

[arnabnandy7]

@igordayen @tuannx fix applied, you may validate on latest changes.

Thanks @arnabnandy7 for the quick and clean fix! Validated locally against both DSL agents (splitGarden, evenMoreEvilWizard) and the unmapped goal scenario — all test cases pass and the validation boundaries align nicely.

LGTM from my side!

@tuannx thanks for validating the change.