Skip to content

Model routing: an omitted allowed-models is narrowed to an imported workflow's list instead of meaning any model #68877

Closed

Description

@rabeckett

Problem

Since #68495 (for #68494), omitting engine.model-routing.allowed-models means the router has no shortlist: it chooses from every model that models.allowed and models.blocked allow. For Claude and Codex, that's every model in the engine's family.

That breaks when routing settings also come from imported workflows. mergeImportedModelRoutingConfigs in pkg/workflow/compiler_orchestrator_engine.go combines the main workflow's allowed-models with each imported workflow's by appending the imported entries. It doesn't treat an omitted list differently from an empty one. So when the main workflow omits allowed-models and an import lists models, the result is only the import's list, and the main workflow's "any model" becomes "only these models". The compiler gives no warning.

The same happens in the other direction. If an import omits allowed-models and the main workflow lists models, the import's "any model" is dropped and only the main workflow's list is used. With two imports, one of which omits the list, only the other's list is used.

Listing one model in the main workflow therefore gives the router more choice (the union of both lists) than listing none.

Reproduction

.github/workflows/shared/routing.md:

---
engine:
  id: copilot
  model-routing:
    goal: cost-speed
    mode: robust
    allowed-models: [claude-sonnet-5]
---

.github/workflows/routed.md:

---
on: workflow_dispatch
permissions:
  contents: read
  copilot-requests: write
imports:
  - shared/routing.md
engine:
  id: copilot
  model-routing:
    goal: cost
    mode: economy
---

Summarize the README.

With gh-aw v0.91.14, gh aw compile routed succeeds with no warnings, and the AWF config in routed.lock.yml contains:

"routing":{..."candidateModels":["github-copilot/claude-sonnet-5"]}

Without the import, the same workflow compiles with no candidateModels, so the router can choose any allowed model.

Main workflow allowed-models Imported allowed-models Result today Expected
Omitted [claude-sonnet-5] [claude-sonnet-5] No shortlist
[gpt-6-sol] Omitted [gpt-6-sol] No shortlist
No routing One import omitted, another [claude-sonnet-5] [claude-sonnet-5] No shortlist
No routing [claude-sonnet-5] [claude-sonnet-5] Unchanged
[gpt-6-sol] [claude-sonnet-5, gpt-6-sol] [gpt-6-sol, claude-sonnet-5] Unchanged

Cause

mergeImportedModelRoutingConfigs was added in #66234, when allowed-models was required, so every routing configuration had a list and appending was correct. #68495 made the list optional, with an omitted list (AllowedModels == nil) meaning no shortlist, but didn't update the merge. The existing test, TestImportedWorkflowModelRoutingCandidatesAreMerged, only covers two lists.

Proposed fix

Keep merging lists as a union, and treat an omitted list as "any model", so it wins over any list:

  • In mergeImportedModelRoutingConfigs, when an imported routing configuration is merged into one that already exists, check whether either side's AllowedModels is nil. If so, set the merged AllowedModels to nil and move on to the next import. Otherwise keep the current deduplicated append.
  • Leave the branch that copies the first import's routing configuration, when the main workflow has none, unchanged. It already keeps an omitted list as nil.
  • Don't change goal and mode precedence. The main workflow's values still win.

Downstream code already handles a nil list: resolveModelRoutingAllowedModels returns no candidates, AWF config generation omits routing.candidateModels, and Claude and Codex fall back to their family.

This fix was prototyped on main at 03896845b7 as a small change to mergeImportedModelRoutingConfigs, plus the test below. With the patched compiler, the reproduction above compiles with no candidateModels. A main workflow that lists [gpt-5.6-sol] still gets ["github-copilot/gpt-5.6-sol","github-copilot/claude-sonnet-5"], and a main workflow without routing still gets the import's list.

Tests

Add a table-driven test to pkg/workflow/model_routing_subagents_test.go, next to TestImportedWorkflowModelRoutingCandidatesAreMerged. Each case should:

  1. Write the main workflow and its imported files to a temporary directory.
  2. Parse the main workflow with ParseWorkflowFile.
  3. Check EngineConfig.ModelRouting.AllowedModels.
  4. Build the AWF config with BuildAWFConfigJSON, and check that apiProxy.routing.candidateModels is absent when the expected list is nil and present otherwise.

Cover the five rows of the table above. In the prototype, the three "No shortlist" cases fail before the fix and all five pass after it. A full go test ./pkg/workflow run showed the same set of failures with and without the fix, all of them environment-specific failures on Windows.

Documentation

  • pkg/parser/schemas/main_workflow_schema.json: the allowed-models description says "Candidate lists from imported workflows using the same engine are merged." Add that if any of them omits allowed-models, the router has no shortlist. Then regenerate docs/src/content/docs/reference/frontmatter-full.md with make generate-schema-docs.
  • docs/src/content/docs/reference/model-routing.md: update the sentence "Candidate lists from imported workflows using the same engine are merged." in the same way. If Docs: rewrite the Model Routing reference page for workflow authors #68857 has already replaced the page, edit its "Shared routing settings" section instead. Remove "If it enables routing without allowed-models, the router chooses from only the imported entries", and state the new rule.
  • Add a patch changeset.

Out of scope

  • When the main workflow and an import set different goal or mode values, the import's values are dropped without a warning.
  • A main workflow that uses the short form engine: copilot can't import routing settings ("multiple engine fields found").

These will be filed separately.

Acceptance criteria

  • The five cases above produce the expected results, covered by the new test.
  • TestImportedWorkflowModelRoutingCandidatesAreMerged and the other model-routing tests still pass.
  • The schema description, generated frontmatter-full.md, and Model Routing page describe the rule.
  • make agent-finish passes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions