Two agents using the same goal name: one goal is silently dropped

[tuannx]

What happens

Two agents, each with a goal named done, but meaning different things:

platform.deploy(agentA)   // goal "done", description "what A means"
platform.deploy(agentB)   // goal "done", description "what B means"

platform.agents().size    // 2
platform.goals.size       // 1   <- one goal is gone

Both deploy. agents() returns both. Nothing is logged. One goal no longer
exists. Same for actions and conditions.

Why it matters

PerGoalToolFactory publishes one MCP tool per goal, so a dropped goal is a
tool missing from the MCP server, with no error.

Which one survives depends on agent name order, since agents() sorts by name.
Deploying a new unrelated agent can therefore change which goal an existing
tool points at.

Where

AgentPlatform.kt aggregates all three with distinctBy { it.name }:

override val actions: List<Action>
    get() = agents().filterNot { it.opaque }.flatMap { it.actions }.distinctBy { it.name }

override val goals: Set<Goal>
    get() = agents().flatMap { it.goals }.distinctBy { it.name }.toSet()

override val conditions: Set<Condition>
    get() = agents().filterNot { it.opaque }.flatMap { it.conditions }.distinctBy { it.name }.toSet()

Annotation agents mostly escape it, because AgentMetadataReader names goals
ClassName.methodName. DSL agents use whatever name the developer types.

How it turned up

Found while writing the MCP end-to-end tests in #1832. Two test fixtures
happened to share a goal name, and one goal never reached tools/list.

Fix

PR #1833.

[tuannx]

Duplicate-name handling: where it is checked, and what happens

All links pinned to main @ 4ef247a.
Line numbers are as of that commit.

Eight places react to a duplicate name. They disagree on what counts as a duplicate
and on what to do about it.

Rows 1–4 are one chain, in execution order: two validators produce errors, a manager
aggregates them, a reader consumes the result. Rows 5–8 are independent and silent.
The Module column omits the embabel- prefix.

#	Module	Where	Scope	Covers	On duplicate	Blocks?
1	agent-api	DefaultAgentStructureValidator:116	within one agent	action only	returns ValidationError(ERROR), logs nothing itself	no — row 4 decides
2	agent-api	GoapPathToCompletionValidator:196	within one agent	goal↔action pairing	logger.error + continue, result stays valid	no
3	agent-api	DefaultAgentValidationManager:47-56	aggregates rows 1–2	all	logs each error at ERROR, returns result	no
4	agent-api	AgentMetadataReader:304-308	consumes row 3	all	logs the same errors again at WARN, continues	no — return null commented out
5	agent-api	AgentPlatform:139/142/145	aggregation across agents	goal, action, condition	distinctBy { it.name } → drop, silently	no
6	agent-api	toolUtils.kt:43 safelyGetTools	across tool objects	tool name	distinctBy → drop, silently	no
7	agent-api	toolUtils.kt:72 safelyGetToolsFrom	within one tool object, after renaming	tool name	distinctBy → drop, silently	no
8	agent-api	DefaultToolLoop:430	LLM tool loop	tool name	skip already-present name, logger.debug	no

All eight sit in one module, embabel-agent-api, and still disagree with each other.
None of them stops anything.

Worth noting for rows 1→3→4: an error raised by a validator is logged twice, at ERROR
by the manager and again at WARN by the reader, and then ignored.

Three places produce a published name and check nothing. These do span modules:

Module	Where	Produces	Checks
agent-api	PerGoalToolFactory:142	MCP tool name	nothing
agent-mcpserver	McpPromptFactory:48	MCP prompt name	nothing
agent-a2a	FromGoalsAgentSkillFactory:33	A2A skill id	nothing

---

Four observations

1. Only actions have a duplicate error code

ValidationErrorCodes:30
defines exactly one duplicate code, DUPLICATE_ACTION_NAME. There is no
DUPLICATE_GOAL_NAME and no DUPLICATE_CONDITION_NAME, so row 1 does not see two
goals sharing a name inside a single agent.

2. The validation pipeline currently blocks nothing

AgentMetadataReader:304-308:

if (!validationResult.isValid) {
    logger.warn("Agent validation failed:\n…")
    // TODO: Uncomment to strengthen validation and refactor the test if needed.
    // return null
}

The return null is commented out, so a duplicate action name inside one agent yields
one log line and the agent still deploys.

Separately,
AgentMetadataReader:113
defaults to AgentStructureAgentValidator.PERMIT_ALL. In a Spring context
DefaultAgentStructureValidator is a @Component and gets injected, but constructing
the reader directly — tests, embedded use — runs with no structural validation at all.

3. Row 2 reports a false pass

The continue at
GoapPathToCompletionValidator:195-198
jumps before both allGoalsAchievable = false and failedGoals.add(...). A goal with
no action that achieves it therefore produces a VALID result — the exact condition
the validator exists to catch.

4. Duplicate tool names give a shorter list, not a broken one

safelyGetTools applies distinctBy { it.definition.name }, and
McpToolExport
goes through it. So MCP never publishes an invalid tool list — it silently publishes one
tool fewer.

The same disappearance therefore happens at three levels: a goal at AgentPlatform
(row 5), a tool at safelyGetTools (row 6), and a tool again inside the LLM loop
(row 8).

Reproduced with two @Agent classes both named Wizard in different packages:

goals published : com.embabel.probe.alpha.Wizard.done
                  com.embabel.probe.beta.Wizard.done      <- distinct
tools published : [app_Wizard_done, app_Wizard_done]      <- same name, twice

SanitizedGoalNameToolNamingStrategy:34
keeps the last two dot-separated segments, so two distinct goal names collapse into one
tool name. PerGoalToolFactory does not de-duplicate, and nothing anywhere checks the
resulting tool name.

One more case: safelyGetToolsFrom de-duplicates after applying
toolObject.namingStrategy.transform. A renaming strategy can map two distinct tools
onto one name, and the collision it creates is dropped four lines later.

---

Summary

Two responses to the same condition, chosen by layer rather than by rule:

• Report and let the caller decide — rows 1–4; the caller currently decides to
  continue, so the error is logged twice and blocks nothing
• Drop silently — rows 5, 6, 7, 8

Nothing in this table throws.
spi/validation
contains zero throw statements — it is report-only by design. The one place in the
agent-loading path that does throw,
AgentMetadataReader.validateStateClass:393,
sits outside that abstraction and is unrelated to duplicate names.

[igordayen]

@tuannx @deleSerna

@simeshev -FYI

Aggregation on metadata reader makes sense....

I validated that AgentDeployer and Agent Platform "deploy" do not seem intended for validation error checking

override val actions: List
get() = agents().filterNot { it.opaque }.flatMap { it.actions }.distinctBy { it.name }==> collect distinct actions by name...

    AgentPlatform:139/142/145	aggregation across agents	goal, action, condition	distinctBy { it.name } → drop, silently	no

6 agent-api toolUtils.kt:43 safelyGetTools ==> same logic as above

DefaultAgentValidationManager:47-56 aggregates rows 1–2 all logs each error at ERROR, returns result no
4 agent-api AgentMetadataReader:304-308 consumes row 3 all logs the same errors again at WARN, continues ==> double logging is not good, please confirm 100%, I would keep as error

GoapPathToCompletionValidator:195-198 ==> runs after metadata reader?

Drop silently — rows 5, 6, 7, 8==> after Validation?

Ideally: Component A ==> generate error code ==> Component B aggregator - Logs error as ERROR

So, it's evident by design that Validation is not intended to throw an error then.

Please confirm which artifact in the chain is absolutely last in error reporting.

Excellent analysis, thank you!

[deleSerna]

@tuannx Good analysis 👍 .

AgentMetadataReader:113
defaults to AgentStructureAgentValidator.PERMIT_ALL

I have missed that PERMIT_ALL is the default.

if (!validationResult.isValid) {
logger.warn("Agent validation failed:\n…")
// TODO: Uncomment to strengthen validation and refactor the test if needed.
// return null
}

Should not we remove this TODO as it looks like not intended to fail?

IIUC, silently dropping the duplicate goal is OK. What would be the next action for this issue?

[igordayen]

What would be the next action for this issue?

@tuannx - could you please address inquiries from my side? thank you

[tuannx]

@igordayen I answered base on current code flow:

double logging is not good, please confirm 100%, I would keep as error

Confirmed 100%.
DefaultAgentValidationManager:47-56
logs a header plus one line per error, all at ERROR. Then
AgentMetadataReader:306
logs the same errors again, joined, at WARN.

GoapPathToCompletionValidator:195-198 ==> runs after metadata reader?

Not really — it runs inside it. It is passed to the reader's constructor as
pathToCompletionValidator, placed in the manager's list at line 124, and invoked when the
reader calls agentValidationManager.validate(agent) at line 304. One
createAgentMetadata call covers reader → manager → both validators → back to reader.

One qualifier I should have included in the table: that call is guarded by
line 303:

if (plannerType == PlannerType.GOAP && agenticInfo.isAgent()) {

So validation does not run at all for SUPERVISOR or UTILITY planners, nor for
@EmbabelComponent scopes that are not agents.

Drop silently — rows 5, 6, 7, 8 ==> after Validation?

Yes for 5,6,8. Row 5 is recomputed on every property read, which is after deploy, rows 6
at tool extraction, row 8 during the LLM tool loop.
Row 7 is called from multiple places.

Ideally: Component A ==> generate error code ==> Component B aggregator - Logs error as ERROR

That is already the design, with one deviation. GoapPathToCompletionValidator collects
errors with errors.add(...) at lines 51, 105 and 227, but at
line 196
it logs directly and continues. That continue skips allGoalsAchievable = false, so a
goal with no achieving action still produces a valid result.

Please confirm which artifact in the chain is absolutely last in error reporting.

AbstractAgentProcess.executeTurn:343-345:

if (agent.goals.isEmpty() && processOptions.plannerType.needsGoals) {
    logger.info("🛑 Process {} has no goals: {}", this.id, agent.goals)
    error("Agent ${agent.name} has no goals: ...")   // kotlin.error → IllegalStateException
}

At runtime, per process, for GOAP / HYBRID / SUPERVISOR (needsGoals = true) but not
UTILITY.

Severity runs backwards along the chain: ERROR at the manager, WARN at the reader, nothing
through aggregation and tool export, then INFO immediately before a throw.

So, it's evident by design that Validation is not intended to throw an error then.

Correct for the package — spi/validation contains zero throw statements. Two things
sit outside it that are worth stating, because they change what the exit-policy question is
actually about.

The same failure gives three different outcomes, depending on how the agent is reached:

Caller	On null
AgentScanningPostProcessorDelegate	agent skipped, boot continues
AgentScopeBuilder:59 and :71	IllegalArgumentException
RunSubagent:35 — ... as Agent on a nullable	NullPointerException at runtime

[igordayen]

Thanks, @tuannx
Summary: Is this accurate?

1. Fatal exception (not directly linked to validation)
2. Skipping agent deployment (null)
3. Continue with agent deployment and flag ERROR

As the Metadata Reader seems the final "decision maker" - could you please confirm whether it is doing both:
validation errors aggregation AND returns null (commented out currently).

Regards.

[igordayen]

And it's time to consolidate issues and PRs, thanks