[sangkyoonnam]

Linked issue: #1150

Purpose of change

User-visible outcome

ModelRouter.of(...).build() and new AgentPlan(...) now reject a router declaration with a duplicate or empty candidate, no candidates, or a default model that is not a candidate, with an IllegalArgumentException that names the offending value. Previously such a declaration passed both and failed only when the router was resolved on the TaskManager, once per routed request, before the durable routing call.

Intent

These are static declaration errors, like a rule key that names no candidate or a judge model that is not registered. #1042 moved those two checks to plan construction; this change gives the candidate list and the default model the same treatment.

Runtime flow

Unchanged for a valid declaration. For an invalid one the exception surfaces at build() (fluent path) or at new AgentPlan(...) (descriptor path) instead of at ModelRoutingResolver#resolve -> ctx.getResource on the TaskManager. The constructor keeps the check for a descriptor that reaches it without passing through either.

Key decisions

• One static check, RoutingCandidateValidator#validate(subject, candidates, defaultModel), called by the constructor, the builder and AgentPlan, so the three paths share the check and the message without exception wrapping. It is a separate util class in api, like ToolParameterInjectionValidator (also called from AgentPlan), rather than a method on ModelRouter, and it is marked @Internal like SkillUrlUtils.
• The validator takes List<?> and Object, because a deserialized descriptor may carry non-String elements; those are rejected with the candidate-name message rather than a ClassCastException.
• ModelRouter.DEFAULT_MODEL_KEY replaces the literal "default_model" in the constructor, the builder and AgentPlan, as CANDIDATES_KEY already does.

Behavioral Semantics

Interaction decisions

build() runs the candidate check before the existing rule-key check, so a declaration with several problems reports the first one in this order: candidates, default model, rule keys. AgentPlan#validateRoutingStrategies runs it after the candidates shape guard, for every strategy type.

Behavioral contracts

1. A null or empty candidate list raises IllegalArgumentException ("requires at least one candidate").
2. A candidate that is not a non-empty String raises IllegalArgumentException naming the value.
3. A repeated candidate raises IllegalArgumentException naming the candidate.
4. A default model that is not a candidate raises IllegalArgumentException naming the model and the candidates.
5. Contracts 1 to 4 hold at build(), at plan construction, and in the constructor. The constructor's messages for 3 and 4 are unchanged; plan construction prefixes them with Model router '<name>' like the other plan-time router diagnostics. A default_model that is not a String is rejected the same way in the constructor (second commit).

Failure behavior

An invalid declaration fails the registration call or plan construction and never reaches the TaskManager. Previously, under the default FAIL policy the job failed on the first routed record, and under IGNORE every routed record was dropped while the job stayed RUNNING.

Tests

Contracts to tests

Contract	Tests
1	RoutingTest.builderRequiresAtLeastOneCandidate, AgentPlanLlmJudgeValidationTest.emptyCandidateListFailsAtPlanConstruction
2	RoutingTest.builderRejectsBlankCandidateName, RoutingTest.routerConstructionRejectsBlankCandidateName
3	RoutingTest.builderRejectsDuplicateCandidate, AgentPlanLlmJudgeValidationTest.duplicateCandidateFailsAtPlanConstruction
4	RoutingTest.builderRejectsDefaultModelThatIsNotACandidate, AgentPlanLlmJudgeValidationTest.defaultModelNamingNonCandidateFailsAtPlanConstruction, RoutingTest.routerConstructionRejectsNonStringDefaultModel
5	the pairs above cover the builder and plan paths; routerConstructionRejectsBlankCandidateName and the pre-existing defaultModelMustBeCandidate cover the constructor

Coverage and what was not verified

mvn -pl api,plan test: api 460, plan 408, 0 failures. mvn -pl runtime test: 980, 0 failures. spotless:check and ./tools/check-license.sh pass. Without the api change four of the five new RoutingTest cases fail (routerConstructionRejectsBlankCandidateName already passed, because RoutingCandidate rejected a blank name); without the constructor change in the second commit routerConstructionRejectsNonStringDefaultModel fails with ClassCastException; without the AgentPlan change the three new plan cases fail; nothing else changes.

Not verified: the per-record failure on the TaskManager described above is read from the call path and from the #1042 discussion, not observed on a running job. The Python lane does not apply: routers cannot be registered from Python yet.

Implementation invariants and supporting evidence

• A missing or empty candidates list is no longer deferred to the constructor at plan construction, so the candidates == null early return in validateRuleKeys became dead and is removed.
• AgentPlanRoutingBackstopTest's minimal router fixture gains one candidate, as it gained a strategy when strategy validation was added.
• The docs table for the builder states the rules for of(...) and build().

API

Compatibility impact

ModelRouter.DEFAULT_MODEL_KEY is added. RoutingCandidateValidator is added as @Internal; it is a contract shared with the plan module, not a stable public API. No existing signature changes. An empty-string candidate name, previously accepted by build(), is now rejected. A plan that previously constructed with an empty or missing candidates list, or a non-candidate default_model, now fails at plan construction; such a plan could never route a request.

Documentation

• doc-needed
• doc-not-needed
• doc-included

Was this patch authored or co-authored using generative AI tooling?

• Yes
• No

Generated-by: Claude Code 2.1.280 (Claude Fable 5.1)

[sangkyoonnam]

Two corrections to the description and one small commit. The old failure happened when the router was resolved on the TaskManager, before the durable routing call, not inside it. And only four of the five new RoutingTest cases fail without the api change: the blank-name constructor case already passed, because RoutingCandidate rejected blank names. While checking that I found defaultModelMustBeCandidate called build() inside the assertion, so it no longer reached the constructor, and a non-String default_model from a deserialized descriptor hit the String cast before the validator and threw ClassCastException. a76df74 reads it as Object so the validator rejects it, exercises the constructor directly in that test, adds a case for the non-String value, and rewords the comments that still placed the old failure inside the durable call.

[wenjin272]

Thanks for taking this on. LGTM