Skip to content

Model routing: an omitted allowed-models means any model when merging imports - #68883

Merged
SivaKesava1 merged 5 commits into
mainfrom
copilot/fix-allowed-models-routing
Oct 15, 2026
Merged

SivaKesava1 merged 5 commits into
mainfrom
copilot/fix-allowed-models-routing

Conversation

Copilot AI commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

When imported workflows also set routing for the same engine, the compiler combines their allowed-models lists. An omitted list means "no shortlist" (any model allowed by models.allowed/models.blocked), but the merge treated it as empty. So one workflow's list silently narrowed another's "any model". For example, a main workflow without allowed-models that imports allowed-models: [claude-sonnet-5] was compiled to candidateModels: ["github-copilot/claude-sonnet-5"].

Changes

  • Merge rule (mergeImportedModelRoutingConfigs in pkg/workflow/compiler_orchestrator_engine.go): when an import's routing settings are merged into existing ones and either side's AllowedModels is nil, the result is nil. It stays nil for later imports. When every side has a list, the deduplicated union is unchanged. Two things are untouched:

    • When the main workflow has no routing settings, the first import's settings are still copied as is.
    • The main workflow's goal and mode still win.
    if engineConfig.ModelRouting.AllowedModels == nil || importedConfig.ModelRouting.AllowedModels == nil {
        engineConfig.ModelRouting.AllowedModels = nil
        continue
    }
  • Tests (TestImportedWorkflowModelRoutingOmittedAllowedModelsMeansAnyModel): a table-driven test, run through ParseWorkflowFile and BuildAWFConfigJSON. For each case it checks AllowedModels and whether apiProxy.routing.candidateModels is present.

    Main allowed-models Imports Result
    Omitted [claude-sonnet-5] No shortlist
    [gpt-6-sol] Omitted No shortlist
    No routing Omitted, then [claude-sonnet-5] No shortlist
    No routing [claude-sonnet-5], then omitted No shortlist
    No routing [claude-sonnet-5] [claude-sonnet-5]
    [gpt-6-sol] [claude-sonnet-5, gpt-6-sol] [gpt-6-sol, claude-sonnet-5]

    The test workflows don't pin sandbox.agent.version. With a pinned version, compiling routing that has no list fails with "missing model-routing candidates".

  • Docs:

    • model-routing.md, "Shared routing settings": the old "chooses from only the imported entries" sentence is replaced with the new rule.
    • main_workflow_schema.json: one sentence added to the allowed-models description; frontmatter-full.md is regenerated.
    • A patch changeset is added.

Out of scope

These are tracked separately:

  • An import's goal/mode are dropped without a warning when they differ from the main workflow's.
  • A main workflow using the short form engine: copilot can't import routing settings.

…imports

Co-authored-by: SivaKesava1 <11771739+SivaKesava1@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix model routing to handle omitted allowed-models correctly Model routing: an omitted allowed-models means any model when merging imports Oct 15, 2026
Copilot AI requested a review from SivaKesava1 October 15, 2026 00:02
@SivaKesava1
SivaKesava1 marked this pull request as ready for review October 15, 2026 00:07
Copilot AI max review requested due to automatic review settings October 15, 2026 00:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The localized fix has focused regression coverage, and the remaining documentation clarification is nonblocking.

1 open finding
What changed in this PR

Fixes gh-aw’s routing import merge so an omitted allowed-models list remains unrestricted instead of inheriting another workflow’s shortlist.

Changes:

  • Preserve unrestricted routing while retaining explicit-list unions.
  • Add regression cases for omissions, inheritance, and import order.
  • Update routing documentation and add a patch changeset.
File Description
pkg/​workflow/​model_routing_subagents_test.go Tests merged lists and emitted routing candidates.
pkg/​workflow/​compiler_orchestrator_engine.go Preserves omitted lists during routing merges.
pkg/​parser/​schemas/​main_workflow_schema.json Documents routing merge semantics.
docs/​src/​content/​docs/​reference/​model-routing.md Updates shared-routing guidance.
docs/​src/​content/​docs/​reference/​frontmatter-full.md Refreshes generated field descriptions.
.changeset/​model-routing-omitted-allowed-models-import-merge.md Records the patch release fix.

🧠 Review effort: Max

Comment thread pkg/parser/schemas/main_workflow_schema.json Outdated
@github-actions

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer is reviewing this pull request using Matt Pocock's engineering skills...

@github-actions

Copy link
Copy Markdown
Contributor

🔎 PR Code Quality Reviewer is reviewing code quality for this pull request...

@github-actions

Copy link
Copy Markdown
Contributor

✂️ Ponytail Reviewer has started processing this pull request

@github-actions

Copy link
Copy Markdown
Contributor

🔬 Test Quality Sentinel is analyzing test quality on this pull request...

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Design Decision Gate 🏗️ is checking for design decision records on this pull request...

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot address the following outstanding work in one pass:

  1. Update this branch with the latest main using make merge-main, resolving any conflicts and preserving the intended changes.
  2. Review (pkg/parser/schemas/main_workflow_schema.json:15228): Limit the no-shortlist rule to workflows that enable routing for the same engine. A main workflow without model-routing still inherits an import's explicit list, as the new main without routing, import lists models test confirms. Imports without routing are skipped. The current wording incorrectly implies that either case clears the shortlist. - Model routing: an omitted allowed-models means any model when merging imports #68883 (comment)

Push the necessary fixes, reply to each listed review thread and resolve it when addressed. Ignore feedback already answered or resolved. Use the pr-finisher skill and stop when only human review or CI remains; do not trigger CI.

Sous-chef head: 6474740
Sous-chef work: e9399e4e9c153a02ba27e13312095223099c9aefa205a0ff00a31b6ce68de88a
Sous-chef state: 9bb9b1b0574b02141e3ee4e6102175d6a152640297a19633109aafbad59f8a86

Generated by 👨‍🍳 PR Sous Chef · pi · haiku45 · 4.5 AIC · ⌖ 10.8 AIC · ⊞ 1.1K · ◷
Comment /souschef to run again

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

@copilot address the following outstanding work in one pass:

  1. Update this branch with the latest main using make merge-main, resolving any conflicts and preserving the intended changes....

Pushed 4a294ab with the wording clarification and regenerated schema docs. make merge-main confirmed this branch already includes latest main and completed its build/recompile/format steps. The focused regression test, final progress gate, and secret scan passed. Replied to the linked review thread; marking it resolved requires a maintainer because no authenticated thread-resolution tool is available. No CI was triggered; checks on the new HEAD remain unverified. Automated review was unavailable; a separate read-only review found no significant issues.

@SivaKesava1
SivaKesava1 merged commit 2b42b5a into main Oct 15, 2026
36 checks passed
@SivaKesava1
SivaKesava1 deleted the copilot/fix-allowed-models-routing branch October 15, 2026 03:54
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.92.0

github-actions Bot added a commit that referenced this pull request Oct 15, 2026
- Document the new multi-repository checkout allowlist (`multi`,
  `allowed-repos`, `allow-private-repos`, `max-repos`) added in #68593/#68816,
  extracted into a dedicated syntax-checkout.md to keep syntax-agentic.md
  under its line budget and add it to the syntax.md index.
- Fix engine.model-routing docs: allowed-models is optional (not required),
  and document the omitted-value and import-merge semantics from #68495/#68883.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
pelikhan added a commit that referenced this pull request Oct 15, 2026
…current schema (#68999)

* Sync checkout and model-routing instruction docs with current schema

- Document the new multi-repository checkout allowlist (`multi`,
  `allowed-repos`, `allow-private-repos`, `max-repos`) added in #68593/#68816,
  extracted into a dedicated syntax-checkout.md to keep syntax-agentic.md
  under its line budget and add it to the syntax.md index.
- Fix engine.model-routing docs: allowed-models is optional (not required),
  and document the omitted-value and import-merge semantics from #68495/#68883.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Refresh dispatcher instruction file index

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

* Sync dispatcher instruction file list

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

4 participants