Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/31aedde0-6b77-46bd-81d2-65765f154fc1 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Add byok-copilot feature flag for Copilot CLI offline BYOK mode
Add Apr 16, 2026
byok-copilot feature flag for Copilot offline BYOK mode wiring
Collaborator
|
@copilot update the firewall version to v0.25.21 |
Contributor
🧪 Test Quality Sentinel ReportTest Quality Score: 72/100
Test Classification DetailsView All Test Classifications (5 tests)
Flagged Tests — Suggested Improvements
|
| Component | Score | Notes |
|---|---|---|
| Behavioral Coverage (40 pts) | 40/40 | All 5 tests verify observable output or state, not internals |
| Error/Edge Case Coverage (30 pts) | 12/30 | Only 2 of 5 tests cover a negative/edge case scenario |
| Low Duplication (20 pts) | 20/20 | No copy-paste patterns detected |
| Proportional Growth (10 pts) | 0/10 | Test additions significantly exceed production code additions (e.g. 35 test lines vs 4 production lines for installer); this is not necessarily bad engineering, but triggers the penalty |
Language Support
Tests analyzed:
- 🐹 Go (
*_test.go): 5 tests — all unit (//go:build !integration) - 🟨 JavaScript (
*.test.cjs,*.test.js): 0 tests
Verdict
✅ Check passed. 0% of new tests are implementation tests (threshold: 30%). All new tests verify behavioral contracts. The main opportunity for improvement is adding negative/default-path tests alongside the positive BYOK cases.
📖 Understanding Test Classifications
Design Tests (High Value) verify what the system does:
- Assert on observable outputs, return values, or state changes
- Cover error paths and boundary conditions
- Would catch a behavioral regression if deleted
- Remain valid even after internal refactoring
Implementation Tests (Low Value) verify how the system does it:
- Assert on internal function calls (mocking internals)
- Only test the happy path with typical inputs
- Break during legitimate refactoring even when behavior is correct
- Give false assurance: they pass even when the system is wrong
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.
References: §24490726922
🧪 Test quality analysis by Test Quality Sentinel · ● 374.6K · ◷
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces a first-class features.byok-copilot flag to wire Copilot offline BYOK mode automatically (dummy API key injection, implicit CLI proxy enablement, and forcing Copilot CLI install to latest), plus accompanying tests and docs.
Changes:
- Add
byok-copilotfeature flag and dummy BYOK API key constant. - Implicitly enable
cli-proxyfor Copilot workflows whenbyok-copilotis enabled; inject dummyCOPILOT_API_KEY; force installer version tolatest. - Add/extend tests covering the new implication, AWF args, env injection, and installer behavior; update glossary docs.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/features.go | Adds byok-copilot ⇒ implicit cli-proxy enablement logic for Copilot engine workflows. |
| pkg/workflow/features_test.go | Extends feature-flag tests to cover BYOK implication behavior. |
| pkg/workflow/copilot_engine_installation.go | Forces Copilot CLI install version to latest when BYOK is enabled. |
| pkg/workflow/copilot_engine_execution.go | Injects dummy COPILOT_API_KEY env var when BYOK is enabled. |
| pkg/workflow/copilot_installer_test.go | Adds test asserting latest is used for installs under BYOK. |
| pkg/workflow/copilot_engine_test.go | Adds test asserting dummy COPILOT_API_KEY injection under BYOK. |
| pkg/workflow/awf_helpers_test.go | Adds test asserting CLI-proxy-related AWF args are present under BYOK. |
| pkg/constants/feature_constants.go | Defines ByokCopilotFeatureFlag and documents its semantics. |
| pkg/constants/engine_constants.go | Defines CopilotBYOKDummyAPIKey constant. |
| docs/src/content/docs/reference/glossary.md | Documents byok-copilot in the feature flags glossary entry. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 10/10 changed files
- Comments generated: 2
Comment on lines
+317
to
+319
| if isFeatureEnabled(constants.ByokCopilotFeatureFlag, workflowData) { | ||
| env["COPILOT_API_KEY"] = constants.CopilotBYOKDummyAPIKey | ||
| } |
There was a problem hiding this comment.
In BYOK mode, COPILOT_API_KEY is set before custom env vars are merged in from workflowData.EngineConfig.Env / agentConfig.Env later in this function. If a workflow still has legacy/manual wiring that sets COPILOT_API_KEY in engine.env, that later maps.Copy will overwrite the dummy key and break the intended offline BYOK path. Consider applying the BYOK injection after all env merges (or explicitly overriding COPILOT_API_KEY when byok-copilot is enabled) so the feature flag reliably enforces the dummy value.
Comment on lines
195
to
+199
| if tt.frontmatter != nil { | ||
| workflowData = &WorkflowData{ | ||
| Features: tt.frontmatter, | ||
| EngineConfig: &EngineConfig{ | ||
| ID: tt.engineID, |
There was a problem hiding this comment.
GH_AW_FEATURES is only set for subtests when envValue != "". For cases expecting an empty env, the test will inherit whatever GH_AW_FEATURES is set to in the outer process environment, which can make these assertions flaky. It’s safer to always call t.Setenv("GH_AW_FEATURES", tt.envValue) (including empty string) for each subtest.
Generated by Design Decision Gate workflow. Captures the decision to introduce byok-copilot as a composing feature flag that bundles cli-proxy enablement, dummy API key injection, and latest-version install for Copilot offline BYOK mode. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Commit pushed:
|
Contributor
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (154 new lines in AI has analyzed the PR diff and generated a draft ADR to help you get started: 📄 Draft ADR: The draft captures three interrelated design decisions this PR makes implicitly:
What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
References: Note 🔒 Integrity filter blocked 1 itemThe following item were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | none
|
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3ea79c99-08ff-4adf-b710-b7eb8a71e584 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Contributor
Author
Updated in commit Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot
AI
changed the title
Add
Add Apr 16, 2026
byok-copilot feature flag for Copilot offline BYOK mode wiringbyok-copilot feature flag for Copilot offline BYOK mode wiring and bump default firewall to v0.25.21
- Move COPILOT_API_KEY injection after engine/agent env merges so legacy manual wiring in engine.env or agent.env cannot overwrite the dummy sentinel value that triggers AWF BYOK detection - Always set GH_AW_FEATURES env var in feature tests (including empty string) to prevent flakiness from inherited outer process environment Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Apr 16, 2026
Closed
github-actions bot
added a commit
that referenced
this pull request
Apr 16, 2026
…ment object form - Add byok-copilot feature flag subsection to frontmatter.md (from #26544: bundles cli-proxy, COPILOT_API_KEY injection, and latest Copilot CLI install behind a single flag) - Update all github-app examples to use client-id (preferred) instead of app-id (deprecated alias) across auth.mdx, checkout.md, faq.md, imports.md, safe-outputs.md, safe-outputs-pull-requests.md, tokens.md, triggers.md, and glossary.md (from #26551: run gh aw fix to auto-migrate) - Add object-form documentation for status-comment in triggers.md, with issues/pull-requests/discussions toggle fields (from #26535) - Update status-comment description in frontmatter.md to mention object form Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This introduces a first-class
features.byok-copilotswitch so Copilot offline BYOK mode no longer requires manual, error-prone frontmatter composition. Enabling it now composes the expected compiler/runtime setup for AWF API-proxy + CLI-proxy usage.Feature flag surface
byok-copilotto feature constants.dummy-byok-key-for-offline-mode.Compiler/runtime behavior
byok-copilotnow implicitly enablescli-proxyforengine: copilot.COPILOT_API_KEY: dummy-byok-key-for-offline-modelatestwhen BYOK is enabled (ignores pinned engine version in this mode).Firewall default update (review feedback)
DefaultFirewallVersionfromv0.25.20tov0.25.21.Coverage updates
byok-copilot⇒ implicitcli-proxy(copilot-only)COPILOT_API_KEYlatestunder BYOKDocs
featuresto includebyok-copilotsemantics.With this flag enabled, workflow authors no longer need to manually wire the dummy key + CLI proxy + latest installer behavior for BYOK mode.
> [!WARNING]
>
>