feat(copilot/byok): expose extraHeaders, extraBodyFields, sessionId in frontmatter#48096
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…er for copilot engine
Wire sandbox.agent.targets.copilot.{extraHeaders,extraBodyFields,sessionId} frontmatter
fields through to the AWF config JSON at apiProxy.targets.copilot. These map to
AWF_BYOK_EXTRA_HEADERS, AWF_BYOK_EXTRA_BODY_FIELDS, and AWF_PROVIDER_SESSION_ID in the
sidecar, enabling workflow authors to attach custom headers and body fields to Copilot
BYOK upstream requests without hand-editing generated AWF JSON.
- Add ExtraHeaders, ExtraBodyFields, SessionId to AgentAPIProxyTargetConfig (sandbox.go)
- Add ExtraHeaders, ExtraBodyFields, SessionId to AWFAPITargetConfig (awf_config.go)
- Add extractCopilotTargetConfig helper (engine_api_targets.go)
- Wire BYOK fields in BuildAWFConfigJSON for copilot target (awf_config.go)
- Add 6 unit tests + 1 schema compliance test (awf_config_test.go)
- Update awf-config-sources-spec.md drift table with 3 new rows
- Update file org test to forbid extractCopilotTargetConfig from awf_helpers.go
- Update README.md exported type docs
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds Copilot BYOK supplemental request configuration, but the frontmatter schema currently rejects the new fields.
Changes:
- Adds Copilot header, body-field, and session-ID configuration.
- Wires fields into generated AWF JSON with unit coverage.
- Updates generated references and configuration mapping documentation.
Show a summary per file
| File | Description |
|---|---|
specs/awf-config-sources-spec.md |
Documents new mappings. |
pkg/workflow/sandbox.go |
Adds frontmatter model fields. |
pkg/workflow/README.md |
Updates generated type reference. |
pkg/workflow/engine_api_targets.go |
Extracts Copilot target configuration. |
pkg/workflow/engine_api_targets_file_organization_test.go |
Updates helper organization checks. |
pkg/workflow/awf_config.go |
Emits fields into AWF JSON. |
pkg/workflow/awf_config_test.go |
Tests JSON output and schema compliance. |
.github/skills/agentic-workflows/SKILL.md |
Adds release-workflow reference. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Medium
| // extraHeaders: | ||
| // x-openrouter-title: my-workflow | ||
| // http-referer: https://github.com/org/repo | ||
| ExtraHeaders map[string]string `yaml:"extraHeaders,omitempty"` |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Test Quality Sentinel completed test quality analysis. |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design and /tdd — requesting changes on one blocking correctness gap (schema) and two important quality issues.
📋 Key Themes & Findings
Blocking
- Schema gap (correctness):
pkg/parser/schemas/main_workflow_schema.jsontargets.propertiesonly permitsopenai/anthropicwithadditionalProperties: false. Thecopilotkey is absent, sosandbox.agent.targets.copilotwill fail schema validation and silently drop all the new frontmatter fields. The feature is effectively unusable until the schema is updated.
Important
- Naming:
SessionIdshould beSessionIDper Go convention (the existing codebase usesSessionIDincopilot_events_jsonl.goandpi_logs.go). JSON/YAML tags stay assessionId. - Test coverage gap: No test verifying the copilot BYOK fields don't bleed into non-copilot target entries.
Positive Highlights
- ✅ Clean separation:
extractCopilotTargetConfigmirrors the existingextractAPITargetAuthHeaderpattern perfectly - ✅ Good decision not to auto-derive
sessionIdfromGITHUB_RUN_ID— explicitly documented rationale for Azure OpenAI HTTP 400 risk - ✅ Strong test suite: 6 unit tests + schema compliance case cover coexistence with host, entry creation without host, and absence when unconfigured
- ✅ Spec drift table updated correctly
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 45.9 AIC · ⌖ 5.09 AIC · ⊞ 6.7K
Comment /matt to run again
Comments that could not be inline-anchored
pkg/workflow/sandbox.go:130
[/codebase-design] Go convention for ID abbreviations is all-caps: SessionID not SessionId. The existing codebase uses SessionID in pkg/cli/copilot_events_jsonl.go and pkg/workflow/pi_logs.go. The JSON/YAML tag sessionId is fine to keep — only the exported field name should change.
<details>
<summary>💡 Suggested fix</summary>
SessionID string `yaml:"sessionId,omitempty"`This affects AWFAPITargetConfig.SessionId in awf_config.go as well — both should be renamed…
pkg/workflow/awf_config_test.go:267
[/tdd] Missing negative test: when the engine is openai or anthropic (not copilot), extraHeaders/extraBodyFields/sessionId set under sandbox.agent.targets.copilot should still be passed through (since extractCopilotTargetConfig doesn't gate on engine name). But more importantly — there's no test verifying the fields are NOT inadvertently written to a non-copilot target entry. This edge case should be covered to lock down the intended scope of these fields.
<details>
<summar…
Test Quality Sentinel ReportTest Quality Score: 87/100 — Excellent Analyzed 7 test(s): 7 design, 0 implementation, 0 violations. All 7 new tests verify user-visible JSON serialization contracts for the new BYOK copilot frontmatter fields (extraHeaders, extraBodyFields, sessionId). Build tags are present, no mock library usage, descriptive assertion messages throughout. Verdict: passed. 0% implementation tests (threshold: 30%).
|
…sessionId frontmatter exposure ADR generated by Design Decision Gate for PR #48096.
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (281 new lines in 📄 Draft ADR committed:
📋 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
|
There was a problem hiding this comment.
REQUEST_CHANGES — The feature is functionally broken due to a missing JSON schema update. Three additional issues warrant fixes before merge.
### Blocking issues
[Critical] JSON schema never updated — feature is unreachable from frontmatter
The frontmatter schema (pkg/parser/schemas/main_workflow_schema.json) enforces additionalProperties: false on both the targets object and the sandbox_agent_api_target definition. copilot is not an allowed key in targets, and the new fields (extraHeaders, extraBodyFields, sessionId) are not in the sandbox_agent_api_target definition. Any workflow that uses sandbox.agent.targets.copilot.* will fail schema validation at gh aw compile time. The Go structs and AWF wiring are correct, but they are gated behind a schema that was never updated. This makes the entire feature a no-op for real users.
Required changes:
- Add
copilottotargets.propertiesin the schema - Add a
sandbox_copilot_api_targetdefinition (or extendsandbox_agent_api_target) with the new fields and their types - Add a passing schema validation test for the
copilottarget
### Medium issues
[Medium] ExtraHeaders/ExtraBodyFields assigned by reference — defensive maps.Clone() needed to prevent caller-side mutations from corrupting the AWF config.
[Medium] Load-bearing ordering in BuildAWFConfigJSON — the GetCopilotAPITarget block must run before the BYOK merge block or Host is silently lost; this should be made explicit in code, not just in ordering.
[Medium] copilot.authHeader from frontmatter is silently dropped — extractAPITargetAuthHeader only loops over openai/anthropic; now that users can set targets.copilot.authHeader, it must be wired up too.
🔎 Code quality review by PR Code Quality Reviewer · sonnet46 · 63.4 AIC · ⌖ 5.3 AIC · ⊞ 5.7K
Comment /review to run again
Comments that could not be inline-anchored
pkg/workflow/awf_config.go:576
ExtraHeaders and ExtraBodyFields are assigned by reference — if the source AgentAPIProxyTargetConfig is reused or mutated after BuildAWFConfigJSON returns, the AWF config will silently reflect those mutations.
<details>
<summary>💡 Suggested fix</summary>
Use maps.Clone to copy the maps defensively:
if len(copilotFrontmatter.ExtraHeaders) > 0 {
existing.ExtraHeaders = maps.Clone(copilotFrontmatter.ExtraHeaders)
...
}
if len(copilotFrontmatter.ExtraBodyFields) > 0 {
…
</details>
<details><summary>pkg/workflow/awf_config.go:562</summary>
**Load-bearing ordering between `GetCopilotAPITarget` and the BYOK merge block**: line 562 writes `targets["copilot"] = &AWFAPITargetConfig{Host: copilotTarget}` (a fresh struct with only `Host` set). The BYOK block below then retrieves that entry and merges fields onto it. If anyone reorders these two blocks — or if `GetCopilotAPITarget` is called after the BYOK block — the BYOK block will create a new entry and `Host` will be silently lost.
<details>
<summary>💡 Suggested fix</summary>
Make…
</details>
<details><summary>pkg/workflow/awf_config.go:547</summary>
**`copilot.authHeader` from frontmatter is silently dropped**: `extractAPITargetAuthHeader` (the loop that wires `authHeader` into target entries) only iterates over `["openai", "anthropic"]`. Now that `AgentAPIProxyTargetConfig` has a `copilot` target with an `authHeader` field, a workflow author who sets `sandbox.agent.targets.copilot.authHeader` will get no error and no effect.
<details>
<summary>💡 Context</summary>
Look at the existing loop at `pkg/workflow/awf_config.go` around line 547…
</details>
There was a problem hiding this comment.
Review Summary
Two blocking issues found:
1. JSON schema not updated (noted by prior review at sandbox.go:118)
pkg/parser/schemas/main_workflow_schema.json still restricts sandbox.agent.targets to only openai and anthropic (line 3520 additionalProperties: false), and the sandbox_agent_api_target $def only declares authHeader. The three new fields (extraHeaders, extraBodyFields, sessionId) and the copilot key are absent. Any workflow using the new frontmatter will fail schema validation.
2. authHeader for copilot silently dropped (new comment on awf_config.go:585)
The authHeader wiring loop at line 547 only iterates ["openai", "anthropic"]. AgentAPIProxyTargetConfig.AuthHeader is now structurally present on the copilot target struct, but is never read into the AWF config — silent data loss for anyone who sets it.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 56 AIC · ⌖ 5.32 AIC · ⊞ 5K
| existing.SessionId = copilotFrontmatter.SessionId | ||
| awfConfigLog.Printf("API proxy: copilot sessionId configured") | ||
| } | ||
| } |
There was a problem hiding this comment.
The new extractCopilotTargetConfig block handles extraHeaders, extraBodyFields, and sessionId for the copilot target, but it silently ignores authHeader even though AgentAPIProxyTargetConfig.AuthHeader (added in the struct at sandbox.go) is a valid field on the copilot target.
The authHeader loop at line 547 iterates only ["openai", "anthropic"], so sandbox.agent.targets.copilot.authHeader is structurally accepted by the YAML parser but never wired into the AWF config. Either:
- Extend the loop to include
"copilot", or - Handle
AuthHeaderinside this block alongside the new fields.
Otherwise a user who sets sandbox.agent.targets.copilot.authHeader will see it silently dropped without any log or error.
@copilot please address this.
There was a problem hiding this comment.
Fixed in commits fix(schema): add copilot target... and fix(schema): clarify sessionId description....
- Added
copilotas a permitted key undersandbox.agent.targetsin the schema, pointing to a newsandbox_copilot_api_target$def - The new def includes
authHeader,extraHeaders(object of strings),extraBodyFields(object of strings), andsessionId(string), all withadditionalProperties: falsefor strictness - Added 6 end-to-end frontmatter schema validation tests covering: each field individually accepted, all three together, non-string header value rejected, and unknown field rejected
There was a problem hiding this comment.
Fixed in d04b095. Added AuthHeader handling to the copilot frontmatter block in BuildAWFConfigJSON (alongside ExtraHeaders, ExtraBodyFields, and SessionId), and added a dedicated test copilot authHeader from frontmatter sandbox.agent.targets.copilot is included to verify it's wired correctly and doesn't emit an empty host field.
|
@copilot please run the Unresolved review feedback:
|
…aders, extraBodyFields, sessionId fields The strict frontmatter schema was missing the copilot entry under sandbox.agent.targets, causing the documented BYOK fields to be rejected before reaching Go struct parsing. Changes: - Add `copilot` key to the targets object, pointing to a new `sandbox_copilot_api_target` $def - New def includes authHeader, extraHeaders (object of strings), extraBodyFields (object of strings), and sessionId (string) - additionalProperties: false enforces schema strictness for each Add 6 schema validation tests covering: extraHeaders accepted, extraBodyFields accepted, sessionId accepted, all three together, non-string header value rejected, unknown field rejected. Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
…upstream supports it Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
|
@copilot please run the Unresolved review feedback:
|
…pilation test Two issues addressed: 1. extractAgentSandboxConfig in frontmatter_extraction_security.go never extracted the `targets` key from the frontmatter map, so WorkflowData.SandboxConfig.Agent.Targets was always nil even when sandbox.agent.targets was present in frontmatter. Add full extraction for authHeader, extraHeaders, extraBodyFields, sessionId. 2. Add TestCopilotBYOKExtraFieldsInCompiledWorkflow: an end-to-end compilation test that writes a workflow with sandbox.agent.targets.copilot fields in frontmatter, compiles it, and verifies extraHeaders/extraBodyFields/sessionId appear in the generated AWF config JSON in the lock file. Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Addressed in two commits:
|
|
@copilot please run the Unresolved review feedback:
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
🤖 PR Triage — §30192186368
Why: Adds Next: Resolve CHANGES_REQUESTED review comments, verify CI, then expedite human review.
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in the latest commit. |
The AWF proxy sidecar already supports injecting custom headers (
AWF_BYOK_EXTRA_HEADERS), body fields (AWF_BYOK_EXTRA_BODY_FIELDS), and a session ID (AWF_PROVIDER_SESSION_ID) on Copilot BYOK upstream requests, but gh-aw provided no frontmatter surface to configure them — forcing authors to hand-edit generated AWF JSON that gets overwritten on everygh aw compile.Changes
sandbox.go— AddExtraHeaders,ExtraBodyFields,SessionIdtoAgentAPIProxyTargetConfig(frontmatter struct atsandbox.agent.targets.copilot)awf_config.go— Add the same three fields toAWFAPITargetConfig; wire them inBuildAWFConfigJSON— mutates an existing copilot target entry when a host is already set, or creates a header/field-only entry when no host override is configuredengine_api_targets.go— AddextractCopilotTargetConfighelper (mirrors existingextractAPITargetAuthHeaderpattern)awf_config_test.go— 6 unit tests + 1 schema compliance test covering each field individually, coexistence with host, entry creation without host, and absence when unconfiguredspecs/awf-config-sources-spec.md— 3 new rows in the drift table for the new frontmatter ↔ sidecar env var mappingsFrontmatter surface
sessionIdmust be set explicitly — it is never auto-derived fromGITHUB_RUN_ID, because strict OpenAI-compatible upstreams (e.g. Azure OpenAI) reject the unknownsession_idbody field with HTTP 400.run: https://github.com/github/gh-aw/actions/runs/30187302401