feat(core): apply FQN naming to published tools

[tuannx]

Target

Apply one provider-neutral fully qualified name (FQN) strategy to published tools across:

• LLM tools
• supervisor tools
• generated MCP goal tools

Design

• Keep the naming policy in embabel-agent-api core.
• Decouple FQN and naming strategy from LlmInteraction.
• Resolve owner context at each tool publication boundary.
• Keep Agent.name, Action.name, and Goal.name unchanged.

Out of scope

• AgentPlatform aggregation collisions
• duplicate action/goal detection
• duplicate Agent.name handling during deployment
• validation and logging behavior

Related issue: #1990
Parent issue: #1834

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[igordayen]

@tuannx - thanks for reporting.
Per the process, could you please create an issue for this PR?
Also, could you please help interpret the write-up on the issue, preferably with simpler prose? A bit harder to understand the root cause hidden by the write-up potentially generated by AI.

Also - appears something got broken due to migration, or its existing issue; could you please try to dig into the history of the problem?
Thank you for contributing!

[tuannx]

Thanks for the review. Issue opened: #1834

On the history question — this is not from the migration. It goes back to
a977cfd5b,
which changed how the platform de-duplicates:

- get() = agents().flatMap { it.actions }.distinct()
+ get() = agents().flatMap { it.actions }.distinctBy { it.name }

- get() = agents().flatMap { it.goals }.toSet()
+ get() = agents().flatMap { it.goals }.distinctBy { it.name }.toSet()

Before: de-duplicate by value. Two identical goals collapse into one, which is
right.
After: de-duplicate by name. Two different goals sharing a name also collapse,
and one meaning is lost.

That is the only commit that ever touched those lines. conditions was already name-based before it.

The fix restores that distinction rather than removing the de-duplication:
elements equal by value are still collapsed, elements that merely share a name
are rejected at deploy time.

Sorry about the dense write-up. Issue and PR description are rewritten in
plainer terms.

One decision I would rather you made than inherited from a diff: deploy()
currently throws. I picked that because deploy() is an explicit call, so
silently not deploying seemed worse. The lighter option is to log an ERROR and
skip the conflicting agent, matching the direction in #1786 — existing apps keep
booting, but still lose the capability, just loudly. Happy to switch.

[igordayen]

Thank you, @tuannx - will follow up. Best regards!

[igordayen]

One decision I would rather you made than inherited from a diff: deploy()
currently throws. I picked that because deploy() is an explicit call, so
silently not deploying seemed worse. The lighter option is to log an ERROR and
skip the conflicting agent, matching the direction in #1786 — existing apps keep
booting, but still lose the capability, just loudly. Happy to switch.

@alexheifetz - could you please advise - conflicting goals:

• flag ERROR, continue booting
• throw an exception and stop booting?

Should probably align with logic:

val achievableGoalValidationResult = AchievableGoalValidator().validate(agenticInfo.agentName(), targetType, instance, requireInterfaceDeserializationAnnotations)
        if(!achievableGoalValidationResult.isValid) {
            val errorMsg = achievableGoalValidationResult.errors.map { it.message }.joinToString { it }
            logger.error(errorMsg)
            return null
        }

from PR #1801
@deleSerna - FYI

[igordayen]

Before: de-duplicate by value. Two identical goals collapse into one, which is
right.

@tuannx - could you please elaborate on "goal value"? Thank you.

[igordayen]

@tuannx @deleSerna @alexheifetz - since we are not sure whether it is an exception or a flagging error, it is a better option - may I suggest considering something like ErroneousAgentExitPolicy with default behavior ERROR.
and have documented the property
embabel.agent.platform.exit-on-error
Would it work 4all?
Thanks

[deleSerna]

from PR #1801

AchievesGoal without Action should definitely stop the Agent as it's spec violation and it can be easily fix while developing the Agent itself.
But the issue mentioned here seems a bit more tricky as goals/conditions can randomly be selected/dropped and that seems bad to me . We should throw an error if the developer can fix that duplicated goal/condition by renaming them but could the developer always do that?

Could these conflicting goals/action/conditions belongs to agents from third party libraries? If yes then I do not think there is a straight forward solution to this. But my knowledge here is limited. I always write Agent for a stand alone spring boot application. But, if Agent could also be in 3rd party library then just throwing an exception or just flagging an error also won't help as it's not actionable for the consumer of those conflicting libraries.

[igordayen]

Could these conflicting goals/action/conditions belongs to agents from third party libraries? If yes then I do not think there is a straight forward solution to this. B

==> That's the reason for suggesting having an error policy configurable. thanks

[tuannx]

@igordayen @deleSerna There's a third option, and the annotation path already does it. Annotated goals
are named after their agent:

name = "${stateClass.simpleName}.${method.name}"     // "WeatherAgent.myGoal"

and the tool naming strategy expects that shape:

/** "com.myco.MyAgent.myGoal" becomes "MyAgent_myGoal". */

So two annotated agents can both have a same goal and never collide. Only the
DSL takes the name literally — and AgentBuilder already holds the agent name:

    Goal(
(-)      name = name,
(+)       name = "${this@AgentBuilder.name}.$name",

That makes the collision impossible instead of reporting it better, and nobody
has to rename anything.

If you agree with the direction I'll rework this PR — the deploy-time check goes
away, and the test asserts both goals survive instead of asserting a rejection.

[deleSerna]

That's the reason for suggesting having an error policy configurable.

@igordayen But that would not also fix the real issue when the conflicting actions/goal are coming from multiple agents right?

name = "${stateClass.simpleName}.${method.name}"

This could also still result in duplicate name as it's still simpleName not Name .
Even if we use 'Name, 'Name+ method.name still cause duplicate names unless we use signature. Therefore, we should make sure that Name+ method.name, still not esult in duplicate names within the agent itself.

@igordayen @alexheifetz IMO, we should go in the direction suggested by @tuannx but should use 'Name+ method.name` every where. But that looks like a much bigger change. Therefore, IMO, need a bit more thought before implementing it.

[igordayen]

Three points raised:

1. Error policy doesn't fully address the problem — Even with a configurable
   error policy, it wouldn't solve the
   collision when conflicting actions/goals come from multiple agents rather than
   within a single agent.
2. simpleName still risks collisions — The proposed naming scheme
   ${stateClass.simpleName}.${method.name} uses simpleName, which can still
   duplicate. Even upgrading to Name (fully qualified), Name + method. name can
   still collide unless you include the full method signature.
3. Recommendation: use Name + method.name everywhere, but with caution —
   @deleSerna agrees with @tuannx tuannx's direction and suggests using Name + method. name
   universally, but flags it as a bigger change that needs more thought before
   implementation.

@tuannx - concrete naming examples, please, to substantiate the idea and impact assessment.
Thanks

[igordayen]

Consider renaming "fix(core): reject deployments that would silently drop goals or actions- #1833
" to "fix(core): reject deployments that would silently drop goals or actions DUE TO DUPLICATES" #1833

[tuannx]

Before we choose between throwing, an error policy, or qualification, could we do a
small test-only PR first?

@igordayen asked for concrete naming examples and an impact assessment. We can generate
those instead of writing them by hand: a test that records every name Embabel publishes
externally into a checked-in file.

mcp-tool   StarNewsFinder_findNewsStories
a2a-skill  embabel_goal_com.embabel.examples.StarNewsFinder.findNewsStories

Any naming change then shows up as a diff, so we can see what breaks on the wire before
deciding anything.

We have changed these names unnoticed before: #599 (Claude Desktop rejected a tool name)
and #306 (the $embabel_agent_api suffix). Neither had a test.

It would also show that two agents in different packages with the same state class name
still produce the same MCP tool name, because the naming strategy keeps only the last
two segments.

Happy to open it. This PR would then rebase on top.

[igordayen]

thanks @tuannx

Was actually inquiring about all patterns on validation logic in the agent validation package - what behavior do they expose by default? Is it consistent?

[deleSerna]

Was actually inquiring about all patterns on validation logic in the agent validation package
Please compile full documentation on known agent validators behavior for consistency

DefaultAgentStructureValidator currently report errors ( agent booting won’t stop) for the following cases:

• no actions, conditions, or goals defined
• Missing goals
• Duplicate action names
• Actions has preconditions with multiple parameters ( not sure why this is an issue)

AgentMetadataReader stop the agent for following cases:

• Missing EmbabelComponent or Agent annotation
• Both @agentic and @agent annotations present
• No description provided on the Agent
• No actions, conditions, or goals defined
  • Duplicate as it already there on DefaultAgentStructureValidator
• SuperVisor planner has more than one @AchievesGoal
• If embabel.agent.platform.planner.restricted-goals is true then all goals should return same type.
• @AchievesGoal cannot be applied to void-returning @action method

AgentMetadataReader reports errors for following case

• No goal defined

GoapPathToCompletionValidator reports error for cases where it can not to the goal

I have not checked in other places

@igordayen do you mean this report?

[igordayen]

@deleSerna - thanks for the analysis. So, Duplicate action names ==> already in place, but the algorithm requires refinements.

What is the flow?

metadata reader ==> validator ==> deployer.

Maybe propagate errors up to the deployer, and at the deployer level apply a proper exit policy?

Looking for an architecturally sound flow.

thanks

[igordayen]

@tuannx - Is this PR valid then? As the analysis attached to the issues clearly states that deployments should not be rejected.

[tuannx]

@tuannx - Is this PR valid then? As the analysis attached to the issues clearly states that deployments should not be rejected.

Hi @igordayen: Updated PR to log error only, please review whenever you're available. Thank you!

[tuannx]

Looking at this with the full picture: LlmOperations.generate() already receives agentProcess: AgentProcess and action: Action? — those two parameters already contain everything needed to construct the hierarchy. The naming context doesn't need to travel on LlmInteraction at all.

Option 1 — Compute at resolveTools() call site inside the SPI

Add an overload (or extra params) to resolveTools() that accepts strategy + hierarchy directly. The SPI implementations derive both from the parameters they already have:

  // resolveTools gains two optional params:
  fun resolveTools(
      toolConsumer: ToolConsumer,
      resolver: ToolGroupResolver,
      strategy: ToolNamingStrategy = LEGACY_NAME_ONLY,
      hierarchyName: String? = null,
  ): List<Tool>

  // Inside LlmOperations.generate() — data already present:
  val strategy  = agentProcess.processContext.platformServices.toolNamingStrategy()
  val hierarchy = "${agentProcess.agent.name}.${action?.name}"
  val tools = resolveTools(interaction, resolver, strategy, hierarchy)

Streaming / thinking variants: each variant is also invoked via generate() with the same agentProcess/action, so they recompute hierarchy fresh — no copy propagation needed and nothing to forget.

Parallel tool loop: if DefaultToolLoop calls resolveTools(), it already holds the AgentProcess reference (or can get it from context), so it passes the same pair.

Supervisor: SupervisorAction.execute(processContext) has processContext, so it reads agentProcess.agent.name + "." + action.name — identical construction, no special case.

LlmInteraction loses hierarchyName and toolNamingStrategy entirely.

Option 2 — Thin ToolNamingContext wrapper at the resolveTools() call site

Keep resolveTools(toolConsumer) signature unchanged. Wrap LlmInteraction transiently inside the SPI, using the same already-available params:

  // One small class, lives in the SPI layer:
  internal class ToolNamingContext(
      base: LlmInteraction,
      override val toolNamingStrategy: ToolNamingStrategy,
      private val hierarchy: String,
  ) : ToolConsumer by base {
      override fun fullHierarchyName() = hierarchy
  }

  // Inside LlmOperations.generate():
  val strategy  = agentProcess.processContext.platformServices.toolNamingStrategy()
  val hierarchy = "${agentProcess.agent.name}.${action?.name}"
  val tools = ToolConsumer.resolveTools(
      ToolNamingContext(interaction, strategy, hierarchy),
      resolver,
  )

Streaming / thinking: same generate() signature → same wrapper construction → no copy propagation.

Parallel tool loop: creates its own ToolNamingContext at resolveTools() time — the AgentProcess is in scope there.

Supervisor: SupervisorAction wraps its interaction the same way, reads processContext.platformServices.toolNamingStrategy() which it already has.

LlmInteraction loses both fields. The LlmInteraction companion factory stops copying them.

Comparison

┌──────────────────────────┬───────────────────────────────┬──────────────────────────────────────────┐
│                          │           Option 1            │                 Option 2                 │
├──────────────────────────┼───────────────────────────────┼──────────────────────────────────────────┤
│ resolveTools() signature │ Changes (two new params)      │ Unchanged                                │
├──────────────────────────┼───────────────────────────────┼──────────────────────────────────────────┤
│ New types                │ None                          │ One ToolNamingContext class              │
├──────────────────────────┼───────────────────────────────┼──────────────────────────────────────────┤
│ LlmInteraction           │ Fully clean                   │ Fully clean                              │
├──────────────────────────┼───────────────────────────────┼──────────────────────────────────────────┤
│ Streaming / thinking     │ Eliminated                    │ Eliminated                               │
│ copy                     │                               │                                          │
├──────────────────────────┼───────────────────────────────┼──────────────────────────────────────────┤
│ Invasiveness             │ Touches resolveTools()        │ Touches only the SPI generate() call     │
│                          │ callers                       │ sites                                    │
└──────────────────────────┴───────────────────────────────┴──────────────────────────────────────────┘

Option 2 is lower footprint — the wrapper is the minimal change and doesn't ripple into resolveTools() callers. Option 1 is more explicit if you want the naming params to be visible in the function signature rather than encapsulated in a wrapper type.

@igordayen Thanks, I agree that LlmInteraction should not carry hierarchyName or toolNamingStrategy, and that Option 2 has the smaller footprint.

The current streaming path does not go through LlmOperations.generate(), but this can be handled within the tool-resolution implementation.

Before proceeding, could we clarify only these scope decisions?

Question
Should FULL_HIERARCHY apply to direct LLM tools, supervisor action tools, and generated MCP goal tools?
Should Agent.name, Action.name, and Goal.name remain unchanged as source/planning identities, with the strategy applying only to published tool names?
How should the hierarchy be formed for action == null, already-qualified action names, and synthetic supervisor actions?
Is duplicate Agent.name handling in DefaultAgentPlatform.deploy() part of this scope, or a separate registry concern?

[igordayen]

Question
Should FULL_HIERARCHY apply to direct LLM tools, supervisor action tools, and generated MCP goal tools?
Should Agent.name, Action.name, and Goal.name remain unchanged as source/planning identities, with the strategy applying only to published tool names?
How should the hierarchy be formed for action == null, already-qualified action names, and synthetic supervisor actions?
Is duplicate Agent.name handling in DefaultAgentPlatform.deploy() part of this scope, or a separate registry concern

@tuannx im very confused with inquiries. It appears scope is not defined clearly. I see earlier you stated: " I will make that boundary explicit: collision reporting applies to all capability collections, while hierarchy naming is only for ToolConsumer and generated tool names".
Untill scope gets confirmed, PR should be postponed / rescheduled .
Thanks

[tuannx]

Question
Should FULL_HIERARCHY apply to direct LLM tools, supervisor action tools, and generated MCP goal tools?
Should Agent.name, Action.name, and Goal.name remain unchanged as source/planning identities, with the strategy applying only to published tool names?
How should the hierarchy be formed for action == null, already-qualified action names, and synthetic supervisor actions?
Is duplicate Agent.name handling in DefaultAgentPlatform.deploy() part of this scope, or a separate registry concern

@tuannx im very confused with inquiries. It appears scope is not defined clearly. I see earlier you stated: " I will make that boundary explicit: collision reporting applies to all capability collections, while hierarchy naming is only for ToolConsumer and generated tool names". Untill scope gets confirmed, PR should be postponed / rescheduled . Thanks

Thanks @igordayen. And very sorry for bring all the confusion. I see the source of confusion: the PR currently discusses related but separate concerns:

1. Collision reporting covers name-keyed capability collections such as domain types, actions, goals, conditions, and resolved tools.

2. FULL_HIERARCHY applies only to published tool names for LLM, supervisor, and generated MCP tools.

3. Agent.name, Action.name, and Goal.name remain source/planning identities and are not renamed by the tool strategy.

4. Validation/exit-policy behavior discussed in issue Two agents using the same goal name: one goal is silently dropped #1834 and PR Configure exit policy of AgentValidationManager using a property #1801 is separate from this PR.

This PR should only apply FULL_HIERARCHY for tool names: LLM, supervisor, and generated MCP tools.
Please let me know if we can scope this PR as subset of #1834

[igordayen]

@tuannx - thanks. Please rename PR accordingly. Issue: either rename it too or create subissues for actions. not sure about the name Hierarchy - is not an approach semantics: "fully qualified name" (FQN).
Please decouple FQN from LLMInteraction.

I assume action/goal detection on dups separate PR?

Separately clear please air on inquiry I posted on both PR and the issue:

"
The table in the "covers" column states:

"agent-api [GoapPathToCompletionValidator:196] within one agent goal↔action pairing" ==> what is semantics.

In rows 1-3 in the table, what is the execution flow? To assess the claim on ValidationErrorCode vs. Logging.
Naturally, I would expect:

fun A ==> Validation Error ==> fun B ==> logging ==> fun C ==> should not be duplicate noisy logging.

Therefore trying to get more clarification thanks
"
thanks

[tuannx]

@igordayen I answered below:

@tuannx - thanks. Please rename PR accordingly. Issue: either rename it too or create subissues for actions. not sure about the name Hierarchy - is not an approach semantics: "fully qualified name" (FQN). Please decouple FQN from LLMInteraction.

-> Update this PR and issue #1990

I assume action/goal detection on dups separate PR?

-> Working on it.

Separately clear please air on inquiry I posted on both PR and the issue:

" The table in the "covers" column states:

"agent-api [GoapPathToCompletionValidator:196] within one agent goal↔action pairing" ==> what is semantics.

->
2 | agent-api | GoapPathToCompletionValidator:196 | within one agent | goal↔action pairing | logger.error + continue, result stays valid | no
“Goal↔action pairing” means that the validator iterates over each goal within a single AgentScope and looks for an action with the same name:

 agentScope.actions.find { it.name == goal.name }

If no matching action is found, the validator logs an error and continues without adding a ValidationError, marking the goal as failed, or changing allGoalsAchievable. The validation result can therefore remain valid, and the agent is not blocked. This is a false-positive validation path.

In rows 1-3 in the table, what is the execution flow? To assess the claim on ValidationErrorCode vs. Logging. Naturally, I would expect:

fun A ==> Validation Error ==> fun B ==> logging ==> fun C ==> should not be duplicate noisy logging.

Therefore trying to get more clarification thanks " thanks

->
No. The expected flow is not fully met:

• Returned ValidationErrors are logged by DefaultAgentValidationManager and then logged again by AgentMetadataReader, causing duplicate noise.
• A missing goal-action pairing logs directly without creating a ValidationError, bypassing the expected flow entirely.

[igordayen]

@tuannx - added few commets. will continue reviewing, thank you

[igordayen]

@tuannx please elaborate on reason for scope change

[igordayen]

cryptic AI generated code, requires KDOC/comments

[igordayen]

@tuannx

few comments from claude:

over-engineering concerns:

1. Hex sanitization — converting . to 2e produces names like AgentA_2e_run-search. Cryptic to humans and
   potentially confusing to LLMs. Just replacing non-alphanumeric with _ gives AgentA_run-search, which is
   readable and still unique.
2. SHA-256 length bounding — if a name exceeds 64 chars the code silently truncates and appends a hash
   fragment. The right answer for too-long names is to fail fast at registration, not silently mangle the
   published name. An LLM calling a hash-suffixed tool name is not a good experience.

and still suggesting to explore whether agentName can be found in blackboard.

@azanux - could you please provide feedback at your convinience

Thank you

[igordayen]

@tuannx:
The root cause of the complexity of the agent name propagation is the ToolNameOwner interface on CurriedActionTool.

What's happening

CurriedActionTool was given agentName: String in its constructor solely to implement ToolNameOwner, so the
naming context can use agentName.actionName as the per-tool owner instead of just agentName. That
requires propagation of agentName through:

SupervisorAction → CurriedActionTool.forActions(agentName) → CurriedActionTool(agentName)

However, it's avoidable

ToolNamingContext.forLlmCall() already derives ownerName from agentProcess.agent.name — which is available
at every LLM call boundary without any new parameter:

val agentName = agentProcess.agent.name
val ownerName = action?.shortName()?.let { "$agentName.$it" } ?: agentName

ToolNameOwner on CurriedActionTool only overrides that context-level ownerName to use the tool's own
action name instead of the calling action. That's a refinement (action-level granularity), may not be required
for uniqueness — agentName-toolName is already unique across agents.

The simpler design

Drop ToolNameOwner from CurriedActionTool:

• Remove agentName: String constructor param
• Remove agentName: String from forActions() factory
• CurriedActionTool reverts to 3-param construction, no signature change on the calling chain

Trade AgentA_2e_run-search → AgentA-search. Still globally unique and arguably more readable for the
LLM.

The ToolNamingStrategy / ToolNamingContext abstractions are untouched — only the ToolNameOwner
escape-hatch on CurriedActionTool is removed.

Thanks

@azanux, @deleSerna - FYI

[tuannx]

@tuannx: The root cause of the complexity of the agent name propagation is the ToolNameOwner interface on CurriedActionTool.

What's happening

CurriedActionTool was given agentName: String in its constructor solely to implement ToolNameOwner, so the naming context can use agentName.actionName as the per-tool owner instead of just agentName. That requires propagation of agentName through:

SupervisorAction → CurriedActionTool.forActions(agentName) → CurriedActionTool(agentName)

However, it's avoidable

ToolNamingContext.forLlmCall() already derives ownerName from agentProcess.agent.name — which is available at every LLM call boundary without any new parameter:

val agentName = agentProcess.agent.name val ownerName = action?.shortName()?.let { "$agentName.$it" } ?: agentName

ToolNameOwner on CurriedActionTool only overrides that context-level ownerName to use the tool's own action name instead of the calling action. That's a refinement (action-level granularity), may not be required for uniqueness — agentName-toolName is already unique across agents.

The simpler design

Drop ToolNameOwner from CurriedActionTool:

• Remove agentName: String constructor param
• Remove agentName: String from forActions() factory
• CurriedActionTool reverts to 3-param construction, no signature change on the calling chain

Trade AgentA_2e_run-search → AgentA-search. Still globally unique and arguably more readable for the LLM.

The ToolNamingStrategy / ToolNamingContext abstractions are untouched — only the ToolNameOwner escape-hatch on CurriedActionTool is removed.

Thanks

@azanux, @deleSerna - FYI

@igordayen Good catch on the owner! I pushed a squashed commit updating it to just the agent name everywhere.

What changed:

• Dropped the action part from the owner (now AgentA-search instead of AgentA_2e_run-search).
• Fixed ToolNamingStrategy.kt: updated KDocs, removed unreachable branches, and rewrote unit tests with a new null/empty check.
• Goal tools now use short type names instead of full package names to avoid the >64 char limit. It will only use the full name if two types share the same short name.

[igordayen]

@tuannx - please resolve inquiries as "resolved"

[igordayen]

no NPE danger?

[igordayen]

@tuannx

From Codex:

1. PerGoalToolFactory still publishes explicit export.name values , so two different agents can expose the same
   public tool name and collide. In publishedName, the discriminator == null && exportName != null branch returns
   exportName unchanged, and the multi-input branch qualifies exportName with the input discriminator rather than the
   owning agent ([PerGoalToolFactory.kt:177-185]). The PR’s own disabled test shows the exact failure: HippoWizard and
   IbisWizard both publish shared.export twice under FULLY_QUALIFIED ([ToolNamingIntegrationTest.kt:472-485]). That
   directly misses the stated requirement that AgentA.tool1 and AgentB.tool1 be different published tools.

2. The shared LLM publication path still silently drops one tool when the same owner contributes two tools with the same
   simple name, instead of making them uniquely addressable. ToolConsumer.resolveTools qualifies first and then distinctBy
   { it.definition.name }, so any collision after naming is resolved by discarding one tool with no warning
   ([ToolConsumer.kt:136-147]). The PR adds an integration test documenting this current behavior: one of two forecast
   tools under DuplicateToolNameAgent is dropped ([ToolNamingIntegrationTest.kt:390-405]). If the goal is unique published
   tool names rather than “different only when owners differ,” this is still a behavioral hole.

Open question

• Is duplicate export.name intended to remain a documented opt-out from uniqueness, or does the requirement apply to all
  published tools, including explicitly named goal exports? The current PR and its tests assume the former, but your
  problem statement reads like the latter.

@azanux - could you please provide feedback, thanks

[tuannx]

@tuannx

From Codex:

1. PerGoalToolFactory still publishes explicit export.name values , so two different agents can expose the same
   public tool name and collide. In publishedName, the discriminator == null && exportName != null branch returns
   exportName unchanged, and the multi-input branch qualifies exportName with the input discriminator rather than the
   owning agent ([PerGoalToolFactory.kt:177-185]). The PR’s own disabled test shows the exact failure: HippoWizard and
   IbisWizard both publish shared.export twice under FULLY_QUALIFIED ([ToolNamingIntegrationTest.kt:472-485]). That
   directly misses the stated requirement that AgentA.tool1 and AgentB.tool1 be different published tools.
2. The shared LLM publication path still silently drops one tool when the same owner contributes two tools with the same
   simple name, instead of making them uniquely addressable. ToolConsumer.resolveTools qualifies first and then distinctBy
   { it.definition.name }, so any collision after naming is resolved by discarding one tool with no warning
   ([ToolConsumer.kt:136-147]). The PR adds an integration test documenting this current behavior: one of two forecast
   tools under DuplicateToolNameAgent is dropped ([ToolNamingIntegrationTest.kt:390-405]). If the goal is unique published
   tool names rather than “different only when owners differ,” this is still a behavioral hole.

Open question

• Is duplicate export.name intended to remain a documented opt-out from uniqueness, or does the requirement apply to all
  published tools, including explicitly named goal exports? The current PR and its tests assume the former, but your
  problem statement reads like the latter.

@azanux - could you please provide feedback, thanks

@igordayen Both Codex findings are the two disabled tests in this PR. That is what those tests are for — they record what qualification by owner cannot fix, so it is visible instead of hidden. This is the point I have been trying to get across for a while.

• export.name published as-is — by design. It is a public name the user chose for outside clients; qualifying it breaks what they asked for.
• distinctBy dropping a tool — not new. It was already there before this PR. I only moved the rename before it, which renaming requires.

Also: keeping the action in the owner would not have fixed either one. All tools in one interaction go to one call for one action, so the owner is the same either way. Dropping it lost nothing.

Your question: keep export.name as an opt-out, but stop being silent. Two agents picking the same export name should be reported at deploy time, using the nameCollisions code we already have. Same for distinctBy — warn instead of dropping quietly.

[azanux]

Hi @tuannx, thanks for the work. The naming rule is fine.

My concern is how much code it needs.

Why the code is heavy: the rename happens at the end, right before the LLM call. After that point, the rest of the code no longer knows the tool. So we add helpers to find the old name again.

1. sanitize() encodes too much. It hex-encodes every char outside [a-zA-Z0-9] - even _ and -, which providers accept. So my.tool becomes my_2e_tool, and my_tool becomes my_5f_tool. A simple replace(Regex("[^a-zA-Z0-9_-]"), "_") is enough. We still need bound(): even simple names can pass 64 chars.

2. ToolNamingContext.kt:95 - runCatching { ... }.getOrDefault(LEGACY) is there only for the mocks. Fix the tests instead. This code hides real errors.

3. The rename runs in 4 places: AbstractLlmOperations, ToolResolutionHelper, ToolLoop, Streaming. That is why name() needs the is QualifiedTool check, and why DefaultToolDecorator has a names() helper. One rename point, earlier, removes both.

4. SupervisorAgentFactory:330 - signature.removePrefix(...) fixes the string after building it. Build the signature with the published name instead. Then CurriedActionTool.action can stop being internal.

5. PerGoalToolFactory - does 3 things at once (owner qualification, input-type discrimination, dedup changes). Also, firstOrNull { it.goals.any { g === goal } } scans every agent by reference - the platform should give the owner directly.

Suggestion: split into 4 PRs or comcar je ne vois aps l'intreret mits

To be clear: these are suggestions only, nothing mandatory. The code was just not easy to follow as a reader - that is the main reason for this comment.

(1) strategy + config + one rename point, (2) MCP goal tools, (3) input-type discrimination, (4) collision reporting for #1834.

On @igordayen's question: I agree on export.name. It is a public name the user chose - do not rewrite it. But if two agents pick the same name, report it at deploy time. Do not stay silent.

[azanux]

Only the owner needs escaping

sanitize is applied to both the owner (Agent) and the tool name. The join point is the first -, so escaping the owner alone is enough to keep the split unambiguous. Escaping the tool name buys no extra uniqueness.

It does cost length. Every _ or . grows from 1 to 4 characters, and real MCP tool names use underscores heavily (brave_web_search, read_file).

Example:

ToolNamingStrategy.FULLY_QUALIFIED
    .nameFor("com.acme.research.DeepResearchAgent", "brave_web_search")

// actual:   com_2e_acme_2e_research_2e_DeepResearchAgent-brave__38a4886d2533
// expected: com_2e_acme_2e_research_2e_DeepResearchAgent-brave_web_search

The encoded name is 67 characters, over MAX_NAME_LENGTH = 64, so bound() truncates it and appends a hash. The model can no longer tell which tool it is calling. Dropping the tool-side escaping brings the same name to 61 characters and it stays readable.

Suggestion: escape the owner as it is now, and on the tool side escape only characters a provider would reject (anything outside [a-zA-Z0-9_-]). That keeps _ and - at one character while every published name still matches ^[a-zA-Z0-9_-]{1,64}$.

[tuannx]

Agreed that escaping common characters makes names longer and less readable. However, escaping only the owner protects the owner/tool separator, not the encoding within the tool name.
For example, preserving _ while keeping the current encoding for . would produce:

read.file     → AgentA-read_2e_file
read_2e_file  → AgentA-read_2e_file

The current underscore escaping keeps these distinct. I suggest retaining it for now, or replacing it with a shared encoding rule that improves readability while preserving this distinction across all publication paths.

[azanux]

Agreed with @tuannx on the principle: export.name is a public name the user chose for outside clients, so it should be published as-is, and a deploy-time warning on duplicates is the right answer rather than qualifying it.

One place does not follow that principle. With a single starting input type the name goes out untouched. With two or more it is passed through sanitize , so the pinned name is rewritten:

1 input type  -> my.export
2 input types -> my_2e_export-UserInput

The -UserInput suffix is expected - one goal becomes two tools and they have to differ. Turning my.export into my_2e_export is not. Adding an input type silently renames a tool an outside client may already be calling, which is the exact thing publishing it as-is was meant to protect.

If export names are trusted, they should be trusted on both branches: my.export and my.export-UserInput.

Related, same value: line 179 skips bound() as well, so an export name over 64 characters is never truncated

[tuannx]

You’re right: preserving an explicit name for one input but encoding it for multiple inputs is inconsistent.

Current:  my.export → my_2e_export-UserInput
Proposed: my.export → my.export-UserInput

Preserving the explicit base consistently makes sense. However, adding the suffix still changes the public name, so this does not preserve existing calls to my.export when another input type is added. That behavior needs to be explicit in the contract.
If an explicit name, including its suffix, exceeds the publication target’s limits, I would prefer a clear validation error over silently shortening it.

[azanux]

@igordayen Feedback on the open question:

Duplicate export.name should stay an opt-out from qualification — but not from detection.

export.name is a name the user chose as a public contract for outside clients. Qualifying or rewriting it would silently break a name those clients may already be calling, so uniqueness of an explicit export name has to remain the user's responsibility. What the platform owes them is to say when they got it wrong: report duplicate published names at deploy time via the existing nameCollisions support, instead of the second tool silently masking the first.

One scope note for the implementation: under FULLY_QUALIFIED, goalsToPublish() iterates agents() directly and bypasses the distinctBy { it.name } that AgentPlatform.goals applies - so the duplicate check should run on published names in goalTools(), not rely on the legacy goal-level dedup.

[tuannx]

Agreed: explicit names should opt out of qualification, not collision detection. The check should cover the complete final tool list, including generated names and platform tools:

Explicit "search" ↔ another explicit "search"
Explicit "AgentA-search" ↔ generated "AgentA-search"
Explicit "_confirm" ↔ platform tool "_confirm"

Checking only goalTools() would miss the platform-tool case. This is a publication-boundary check, not necessarily a deploy-time check, and I couldn’t find the earlier nameCollisions support on the current head.
Since validation is outside the current PR scope, I suggest agreeing on a narrow publication check or a follow-up. A warning alone would expose the collision but leave ambiguous registration or routing unresolved.

[tuannx]

Hi @tuannx, thanks for the work. The naming rule is fine.

My concern is how much code it needs.

Why the code is heavy: the rename happens at the end, right before the LLM call. After that point, the rest of the code no longer knows the tool. So we add helpers to find the old name again.

1. sanitize() encodes too much. It hex-encodes every char outside [a-zA-Z0-9] - even _ and -, which providers accept. So my.tool becomes my_2e_tool, and my_tool becomes my_5f_tool. A simple replace(Regex("[^a-zA-Z0-9_-]"), "_") is enough. We still need bound(): even simple names can pass 64 chars.

2. ToolNamingContext.kt:95 - runCatching { ... }.getOrDefault(LEGACY) is there only for the mocks. Fix the tests instead. This code hides real errors.

3. The rename runs in 4 places: AbstractLlmOperations, ToolResolutionHelper, ToolLoop, Streaming. That is why name() needs the is QualifiedTool check, and why DefaultToolDecorator has a names() helper. One rename point, earlier, removes both.

4. SupervisorAgentFactory:330 - signature.removePrefix(...) fixes the string after building it. Build the signature with the published name instead. Then CurriedActionTool.action can stop being internal.

5. PerGoalToolFactory - does 3 things at once (owner qualification, input-type discrimination, dedup changes). Also, firstOrNull { it.goals.any { g === goal } } scans every agent by reference - the platform should give the owner directly.

Suggestion: split into 4 PRs or comcar je ne vois aps l'intreret mits

To be clear: these are suggestions only, nothing mandatory. The code was just not easy to follow as a reader - that is the main reason for this comment.

(1) strategy + config + one rename point, (2) MCP goal tools, (3) input-type discrimination, (4) collision reporting for #1834.

On @igordayen's question: I agree on export.name. It is a public name the user chose - do not rewrite it. But if two agents pick the same name, report it at deploy time. Do not stay silent.

@azanux Thanks for these points.
The main design goal is one consistent, deterministic naming contract across LLM, supervisor, and MCP publication: the same owner, tool name, and strategy should produce the same published name.
I agree we should improve readability and fix the inconsistent handling of explicit export names. Any change should also preserve name disambiguation. Duplicate detection is a separate concern and should operate on the complete final list of published names.
cc @igordayen