Skip to content

Fix imported engine.model silently dropped when max-ai-credits is set - #48482

Merged
pelikhan merged 3 commits into
mainfrom
copilot/fix-silent-drop-engine-model
Jul 28, 2026
Merged

Fix imported engine.model silently dropped when max-ai-credits is set#48482
pelikhan merged 3 commits into
mainfrom
copilot/fix-silent-drop-engine-model

Conversation

Copilot AI commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

When a workflow sets max-ai-credits at the top level and sources engine.model from an import, the model pin is silently discarded — the compiled lock falls back to ${{ vars.GH_AW_MODEL_AGENT_COPILOT || ... }} with zero compiler warnings.

Root cause

max-ai-credits at the top level causes ExtractEngineConfig to return a non-nil engineConfig (to carry the budget). In resolveEngineFromIncludesAndImports, model extraction from allEngines was gated on engineConfig == nil, so a budget-only engineConfig caused the import's engine.model to be skipped entirely. The applyEngineImportDefaults path that handles MergedEngineModel is not exercised here because an engine with an id goes into MergedEngines, not MergedEngineModel.

Fix

  • compiler_orchestrator_engine.go — add an else if branch in resolveEngineFromIncludesAndImports that extracts the model from allEngines[0] when model == "" and engineConfig != nil:
} else if model == "" && len(allEngines) > 0 {
    _, extractedModel, extractErr := c.extractEngineConfigFromJSON(allEngines[0])
    if extractErr == nil && extractedModel != "" {
        model = extractedModel
    }
}
  • compiler_orchestrator_engine_test.go — regression test TestSetupEngineAndImports_ImportedModelPreservedWithTopLevelMaxAICredits covering the exact failure scenario: max-ai-credits: 1500 in the main workflow + engine: { id: copilot, model: gpt-5.6-sol } in an import.

…is set

When a main workflow set max-ai-credits at the top level (creating a
non-nil engineConfig from budget fields alone) and an imported file
provided engine: { id: ..., model: ... }, the model was silently
discarded. The extraction branch in resolveEngineFromIncludesAndImports
was guarded by `engineConfig == nil`, so when engineConfig was non-nil
the imported model was never read.

Fix: add an else-if branch that extracts just the model from the first
imported engine config when model is still empty and engineConfig is
already non-nil.

Adds a regression test that covers the exact scenario from the issue
report: max-ai-credits: 1500 in the main workflow + engine.model in
an imported shared workflow.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix silent drop of imported engine.model when max-ai-credits is set Fix imported engine.model silently dropped when max-ai-credits is set Jul 28, 2026
Copilot AI requested a review from pelikhan July 28, 2026 02:34
@pelikhan
pelikhan marked this pull request as ready for review July 28, 2026 02:36
Copilot AI review requested due to automatic review settings July 28, 2026 02:36
@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Test Quality Sentinel completed test quality analysis.

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Design Decision Gate 🏗️ completed the design decision gate check.

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

PR Code Quality Reviewer completed the code quality review.

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.

Pull request overview

Fixes imported engine model pins being dropped when top-level AI credit limits create a partial engine configuration.

Changes:

  • Preserves imported engine.model values.
  • Adds regression coverage for max-ai-credits.
  • Refreshes the generated smoke workflow lock.
Show a summary per file
File Description
pkg/workflow/compiler_orchestrator_engine.go Extracts the imported model when needed.
pkg/workflow/compiler_orchestrator_engine_test.go Tests model and budget preservation.
.github/workflows/smoke-copilot-auto.lock.yml Refreshes generated model-cost metadata.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Medium

@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot resolve the merge conflicts on this branch.

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

Copilot AI commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts on this branch.

Resolved. I merged origin/main into this branch, fixed the conflict in .github/workflows/smoke-copilot-auto.lock.yml, and pushed commit d904707.

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.

Imported engine.model is silently dropped when max-ai-credits is set

3 participants