Skip to content

Remove non-strict override from AOAI API key smoke workflow and restore Copilot BYOK env passthrough#41401

Closed
Copilot wants to merge 6 commits into
mainfrom
copilot/remove-strict-false-and-fix-env-support
Closed

Remove non-strict override from AOAI API key smoke workflow and restore Copilot BYOK env passthrough#41401
Copilot wants to merge 6 commits into
mainfrom
copilot/remove-strict-false-and-fix-env-support

Conversation

Copilot AI commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

This PR updates the AOAI API key smoke workflow to run under strict mode and fixes BYOK runtime env handling so Copilot provider variables are available inside AWF. Without this, BYOK provider routing/auth could be dropped by --exclude-env.

  • Strict-mode workflow source update

    • Removed strict: false from smoke-copilot-aoai-apikey.md so the workflow compiles and executes with strict validation defaults.
  • Copilot BYOK env exclusion fix

    • Updated Copilot AWF execution path to stop excluding BYOK provider env vars in BYOK mode:
      • COPILOT_PROVIDER_BASE_URL
      • COPILOT_PROVIDER_API_KEY
      • COPILOT_PROVIDER_BEARER_TOKEN
    • COPILOT_GITHUB_TOKEN exclusion behavior in BYOK mode remains unchanged.
  • Generated workflow alignment

    • Recompiled lock workflows so generated AWF commands reflect the corrected --exclude-env behavior (notably AOAI apikey and AOAI entra smoke lockfiles).
  • Targeted regression coverage

    • Expanded Copilot BYOK execution test assertions to ensure provider env vars are not emitted as --exclude-env flags in AWF BYOK mode.
if isBYOKMode {
    byokProviderEnvVars := map[string]struct{}{
        constants.CopilotProviderBaseURL:     {},
        constants.CopilotProviderAPIKey:      {},
        constants.CopilotProviderBearerToken: {},
    }
    filtered := make([]string, 0, len(excludeEnvVarNames))
    for _, envVarName := range excludeEnvVarNames {
        if _, drop := byokProviderEnvVars[envVarName]; !drop {
            filtered = append(filtered, envVarName)
        }
    }
    excludeEnvVarNames = filtered
}


✨ PR Review Safe Output Test - Run 28171956058

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

💥 [THE END] — Illustrated by Smoke Claude · 81.4 AIC · ⌖ 39 AIC · ⊞ 8.7K ·

Copilot AI and others added 3 commits June 25, 2026 06:50
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title Remove strict:false from AOAI apikey smoke workflow and fix BYOK env passthrough Remove non-strict override from AOAI API key smoke workflow and restore Copilot BYOK env passthrough Jun 25, 2026
Copilot AI requested a review from pelikhan June 25, 2026 07:02
@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot remove strict false from the other smoke BYOK workflows

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

Copilot AI commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

@copilot remove strict false from the other smoke BYOK workflows

Done in 42cc437 — removed strict: false from the remaining smoke BYOK workflow (smoke-copilot-aoai-entra.md) and recompiled its lockfile.

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Great work on this one! 🎉 The BYOK env passthrough fix is well-scoped and the description clearly explains the root cause — --exclude-env was silently dropping COPILOT_PROVIDER_BASE_URL, COPILOT_PROVIDER_API_KEY, and COPILOT_PROVIDER_BEARER_TOKEN in BYOK mode, which would break provider routing and auth.

All the bases are covered here:

  • The strict-mode removal from smoke-copilot-aoai-apikey.md is clean
  • Lock files are regenerated to match
  • The filter logic in copilot_engine_execution.go is precise and only touches BYOK-mode behaviour
  • Regression coverage in copilot_engine_test.go ensures the env vars won't creep back into --exclude-env flags

This PR looks ready for review. 🚀

Generated by ✅ Contribution Check · 181.9 AIC · ⌖ 11.3 AIC · ⊞ 6K ·

@github-actions github-actions Bot mentioned this pull request Jun 25, 2026
@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot confirm with awf specification that these environment variables are handled by awf

Copilot AI commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

@copilot confirm with awf specification that these environment variables are handled by awf

Confirmed against AWF spec:

  • environment.excludeEnv[] -> --exclude-env (AWF config spec §5)
  • with --env-all, all non-excluded env vars are forwarded (spec §8.4)
  • COPILOT_PROVIDER_BASE_URL is explicitly mapped (apiProxy.modelRouter.baseUrl) and COPILOT_PROVIDER_API_KEY is an explicit source credential (spec §5/§9.1).

So keeping those out of --exclude-env is correct for BYOK passthrough. COPILOT_PROVIDER_BEARER_TOKEN is not currently called out explicitly in the AWF spec, but with --env-all AWF still forwards it when it is not excluded.

@pelikhan pelikhan marked this pull request as ready for review June 25, 2026 12:22
Copilot AI review requested due to automatic review settings June 25, 2026 12:22
@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Test Quality Sentinel completed test quality analysis.

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

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

No ADR enforcement needed: PR does not have the 'implementation' label and has ≤100 new lines of code in business logic directories (26 additions detected, threshold is 100).

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actions Bot commented Jun 25, 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

This PR updates the Copilot engine’s AWF execution path so BYOK provider environment variables (e.g., COPILOT_PROVIDER_BASE_URL, COPILOT_PROVIDER_API_KEY, COPILOT_PROVIDER_BEARER_TOKEN) are not accidentally passed to AWF as --exclude-env flags in BYOK mode, ensuring the Copilot runtime inside AWF can still route/authenticate to the configured provider. It also removes strict: false overrides from the AOAI smoke workflows and updates the generated lockfiles accordingly.

Changes:

  • Adjust Copilot AWF command generation to filter out BYOK provider env vars from ExcludeEnvVarNames when BYOK mode is active.
  • Remove strict: false from AOAI smoke workflow markdown sources so they compile/run with strict defaults.
  • Update lockfiles and add/expand assertions in Copilot BYOK tests for the corrected --exclude-env behavior.
Show a summary per file
File Description
pkg/workflow/copilot_engine_execution.go Filters AWF --exclude-env list in BYOK mode to keep provider env vars available to the Copilot runtime.
pkg/workflow/copilot_engine_test.go Adds assertions ensuring BYOK provider env vars are not emitted as --exclude-env flags in BYOK + AWF mode.
.github/workflows/smoke-copilot-aoai-entra.md Removes strict: false override to use strict defaults.
.github/workflows/smoke-copilot-aoai-entra.lock.yml Regenerated compiled workflow reflecting strict mode and updated --exclude-env flags.
.github/workflows/smoke-copilot-aoai-apikey.md Removes strict: false override to use strict defaults.
.github/workflows/smoke-copilot-aoai-apikey.lock.yml Regenerated compiled workflow reflecting strict mode and updated --exclude-env flags.

Copilot's findings

Tip

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

  • Files reviewed: 6/6 changed files
  • Comments generated: 1

Comment thread pkg/workflow/copilot_engine_test.go
@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🎬 THE ENDSmoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

✅ All tools validated successfully! Agent Container Smoke Test confirms agent container is ready.

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟

Caution

agentic threat detected
Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.

Details

The threat detection engine failed to produce results.

Review the workflow run logs for details.

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

⚠️ Smoke Antigravity failed. Antigravity encountered unexpected challenges...

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🚀 Smoke Pi MISSION COMPLETE! Pi delivered. 🥧

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🚀 Smoke Gemini MISSION COMPLETE! Gemini has spoken. ✨

test

@github-actions

Copy link
Copy Markdown
Contributor

📰 BREAKING: Smoke Copilot - AOAI (apikey) is now investigating this pull request. Sources say the story is developing...

@github-actions

Copy link
Copy Markdown
Contributor

📰 BREAKING: Smoke Copilot - AOAI (Entra) is now investigating this pull request. Sources say the story is developing...

@github-actions

Copy link
Copy Markdown
Contributor

📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing...

@github-actions

Copy link
Copy Markdown
Contributor

Agent Container Tool Check

Tool Status Version
bash 5.2.21
sh available
git 2.54.0
jq 1.7
yq v4.53.3
curl 8.5.0
gh 2.95.0
node v22.23.0
python3 3.11.15 (PyPy 7.3.23)
go 1.24.13
java openjdk 21.0.11
dotnet 10.0.301

Result: 12/12 tools available ✅

Overall Status: PASS

🔧 Tool validation by Agent Container Smoke Test · 18.4 AIC · ⌖ 14.2 AIC · ⊞ 5.4K ·

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

Copilot AI commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

@copilot add workflow integration tests that use these env vars

Added in b49106e. I added workflow integration coverage that compiles both Copilot and Pi BYOK workflows using COPILOT_PROVIDER_BASE_URL and COPILOT_PROVIDER_API_KEY, and verifies those vars stay in step env without being emitted as --exclude-env flags.

@copilot ensure that these environment variables pi agentic engine also benefits from these changes since it uses the copilot endpoint

Fixed in b49106e. The Pi engine now keeps COPILOT_PROVIDER_BASE_URL and COPILOT_PROVIDER_API_KEY available in AWF BYOK mode for the Copilot backend, instead of excluding them from the sandbox env.

@github-actions

Copy link
Copy Markdown
Contributor

Smoke test FAIL
✅ GitHub CLI
❌ Web fetch
✅ File write
✅ Bash verify
❌ Build

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • proxy.golang.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "proxy.golang.org"

See Network Configuration for more information.

🥧 Smoke Pi — Powered by Pi · 4.2 AIC · ⌖ 10.8 AIC · ⊞ 1.9K ·

@github-actions

Copy link
Copy Markdown
Contributor

Caution

agentic threat detected
Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.

Details

The threat detection engine failed to produce results.

Review the workflow run logs for details.

Comment Memory

Build lights flicker now
Cached winds carry the quiet
Smoke test sleeps in green

Note

This comment is managed by comment memory.

It stores persistent context for this thread in the code block at the top of this comment.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

🔮 The oracle has spoken through Smoke Codex · 5.85 AIC · ⊞ 33.2K ·

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test Results

  • Test 1 (GitHub MCP Testing): ✅
  • Test 2 (Web Fetch Testing): ✅
  • Test 3 (File Writing Testing): ✅
  • Test 4 (Bash Tool Testing): ✅
  • Test 5 (Build gh-aw): ❌

Overall status: FAIL

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • localhost

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "localhost"

See Network Configuration for more information.

Smoke Gemini — Powered by Gemini ·

@github-actions

Copy link
Copy Markdown
Contributor

💥 Smoke Test: Claude — Run 28171956058

Overall: PARTIAL

Core #1-12: 1✅ 2✅ 3✅ 4✅ 5❌(Tavily unavailable) 6✅ 7✅ 8✅ 9✅ 10✅ 11✅ 12✅
PR review #13-19: 13✅ 14✅ 15✅ 16✅ 17✅ 18✅ 19⚠️(skipped)

Run

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

💥 [THE END] — Illustrated by Smoke Claude · 81.4 AIC · ⌖ 39 AIC · ⊞ 8.7K ·

@github-actions github-actions Bot 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.

💥 Automated smoke test review - all systems nominal!

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "accounts.google.com"
    - "android.clients.google.com"
    - "clients2.google.com"
    - "contentautofill.googleapis.com"
    - "safebrowsingohttpgateway.googleapis.com"
    - "www.google.com"

See Network Configuration for more information.

💥 [THE END] — Illustrated by Smoke Claude · 81.4 AIC · ⌖ 39 AIC · ⊞ 8.7K

"github.com/github/gh-aw/pkg/constants"
"github.com/github/gh-aw/pkg/testutil"
)

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.

Nice table-driven test covering BYOK env passthrough. Consider asserting on the exact env var names too. (smoke test)


func TestBYOKProviderEnvPassthroughIntegration(t *testing.T) {
t.Parallel()

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.

The struct fields are clear; a short comment on expectModelFlag semantics would help future readers. (smoke test)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants