Sync AWF v0.25.43 spec updates and wire apiProxy.modelMultipliers usage#31398
Conversation
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>
apiProxy.modelMultipliers usage
There was a problem hiding this comment.
Pull request overview
This PR syncs gh-aw’s embedded AWF config schema to the latest gh-aw-firewall surface area and wires existing workflow engine.token-weights.multipliers inputs into generated AWF config via apiProxy.modelMultipliers.
Changes:
- Updated the embedded AWF JSON schema snapshot and drift-tracking documentation to reflect newly available config paths.
- Extended AWF config generation to emit
apiProxy.modelMultiplierswhen token-weight multipliers are present. - Added tests covering emission and omission of
modelMultipliers.
Show a summary per file
| File | Description |
|---|---|
specs/awf-config-sources-spec.md |
Tracks newly introduced schema paths in the “known drift” list. |
pkg/workflow/schemas/awf-config.schema.json |
Syncs embedded AWF schema and adds new config surface area (auth, maxRuns, dockerHostPathPrefix, etc.). |
pkg/workflow/awf_config.go |
Emits apiProxy.modelMultipliers derived from engine.token-weights.multipliers. |
pkg/workflow/awf_config_test.go |
Adds tests for modelMultipliers emission/omission behavior. |
.changeset/patch-awf-spec-sync-v0-25-43.md |
Adds a patch changeset entry describing the schema sync + new emission behavior. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/5 changed files
- Comments generated: 3
| { | ||
| "$schema": "https://json-schema.org/draft/2020-12/schema", | ||
| "$id": "https://github.com/github/gh-aw-firewall/releases/download/v0.25.38/awf-config.schema.json", | ||
| "$id": "https://raw.githubusercontent.com/github/gh-aw-firewall/main/docs/awf-config.schema.json", | ||
| "title": "AWF Configuration", |
| apiProxy := &AWFAPIProxyConfig{ | ||
| Enabled: true, | ||
| MaxEffectiveTokens: maxEffectiveTokens, | ||
| } | ||
|
|
||
| if modelMultipliers := extractModelMultipliers(config.WorkflowData); len(modelMultipliers) > 0 { | ||
| apiProxy.ModelMultipliers = modelMultipliers | ||
| awfConfigLog.Printf("API proxy: %d model multipliers configured", len(apiProxy.ModelMultipliers)) | ||
| } |
| if len(workflowData.EngineConfig.TokenWeights.Multipliers) == 0 { | ||
| return nil | ||
| } | ||
| return workflowData.EngineConfig.TokenWeights.Multipliers |
🧪 Test Quality Sentinel ReportTest Quality Score: 75/100
Test Classification Details
ObservationsTest inflation (minor)The test file gained 50 lines against the production file's 18 lines (ratio ≈ 2.8:1, threshold 2:1). This is largely due to the verbose Error-path coverage (suggestion)Neither new sub-test exercises an error return from Build tag ✅
Assertion messages ✅All assertions include descriptive message strings. No mock libraries ✅Real component interactions — no Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does — observable outputs, state changes, error handling — and catch behavioral regressions if deleted. Implementation Tests (Low Value) verify how the system works internally — prone to breaking on legitimate refactoring even when behavior is correct. References: §25647329883
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Test Quality Sentinel: 75/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%). Both new sub-tests verify observable JSON output (behavioral contracts). Minor suggestion: consider adding nil-branch table rows to cover the three nil guards in extractModelMultipliers.
There was a problem hiding this comment.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · ● 3.9M
| @@ -324,3 +332,13 @@ | |||
| } | |||
There was a problem hiding this comment.
[/tdd] extractModelMultipliers returns the original map directly — if BuildAWFConfigJSON ever mutates apiProxy.ModelMultipliers post-assignment (e.g. filtering zero values), it would silently corrupt the caller's WorkflowData. Consider returning a shallow copy:
result := make(map[string]float64, len(workflowData.EngineConfig.TokenWeights.Multipliers))
for k, v := range workflowData.EngineConfig.TokenWeights.Multipliers {
result[k] = v
}
return resultThis is also the right place to enforce the schema's exclusiveMinimum: 0 constraint — filter or reject zero/negative entries before they propagate to JSON generation.
| assert.NotContains(t, jsonStr, `"modelMultipliers"`, "apiProxy should omit modelMultipliers when empty") | ||
| }) | ||
|
|
||
| t.Run("anthropic API target is included in apiProxy targets", func(t *testing.T) { |
There was a problem hiding this comment.
[/tdd] The two new test cases exercise the empty-multipliers path but not the nil-EngineConfig or nil-TokenWeights guard branches in extractModelMultipliers. Since those nil paths flow through the public BuildAWFConfigJSON function, they can be tested without reaching into the private function:
t.Run("apiProxy modelMultipliers omitted when EngineConfig is nil", func(t *testing.T) {
config := AWFCommandConfig{
EngineName: "copilot",
AllowedDomains: "github.com",
WorkflowData: &WorkflowData{
// EngineConfig intentionally nil
NetworkPermissions: &NetworkPermissions{
Firewall: &FirewallConfig{Enabled: true},
},
},
}
jsonStr, err := BuildAWFConfigJSON(config)
require.NoError(t, err)
assert.NotContains(t, jsonStr, `"modelMultipliers"`, "apiProxy should omit modelMultipliers when EngineConfig is nil")
})This ensures the guard clauses are exercised through the same public interface the rest of the tests use.
|
Commit pushed:
|
🏗️ Design Decision Gate — ADR RequiredThis PR introduces significant changes to core business logic (190 new lines in AI has analyzed the PR diff and generated a draft ADR to help you get started: 📄 Draft ADR: The draft has been committed to this PR's branch ( 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. What the draft capturedThe draft identifies two coupled decisions in this PR:
The second decision is the more architecturally significant one — it gives 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:
This repository's ADRs also include a Part 2 — Normative Specification in RFC 2119 language to make the decision machine-checkable. The draft for this PR follows that two-part structure. All ADRs are stored in References: §25647329877
|
|
@copilot also wore a top level max-runs field to the maxRuns in config |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
apiProxy.modelMultipliers usageapiProxy.modelMultipliers/apiProxy.maxRuns usage
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
apiProxy.modelMultipliers/apiProxy.maxRuns usageapiProxy.modelMultipliers usage
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
|
|
🌑 The shadows whisper... Smoke Codex failed to deliver outputs. The oracle requires further meditation... |
|
💫 TO BE CONTINUED... Smoke Claude failed to deliver outputs! Our hero faces unexpected challenges... |
|
🚀 Smoke Pi MISSION COMPLETE! Pi delivered. 🥧 |
Agent Container Tool Check
Result: 12/12 tools available ✅ Overall Status: PASS
|
|
Smoke Test: Codex - 25649467850 Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
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.
|
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
💥 [THE END] — Illustrated by Smoke Claude · ● 5.4M
| MaxEffectiveTokens int64 `json:"maxEffectiveTokens,omitempty"` | ||
|
|
||
| // ModelMultipliers configures per-model ET accounting multipliers in AWF. | ||
| ModelMultipliers map[string]float64 `json:"modelMultipliers,omitempty"` |
There was a problem hiding this comment.
📝 Smoke test review comment: consider documenting the expected value range for ModelMultipliers (e.g., positive floats; semantics when a model is missing). — Run 25649467832
| } | ||
|
|
||
| if modelMultipliers := extractModelMultipliers(config.WorkflowData); len(modelMultipliers) > 0 { | ||
| apiProxy.ModelMultipliers = modelMultipliers |
There was a problem hiding this comment.
📝 Smoke test review comment: log line could include a sampled key for easier debugging when multipliers are unexpectedly empty. — Run 25649467832
|
CAVEMAN WAS HERE! UGH! Me smoke test agent. Me visit. Me test all thing. All work good! 🔥 Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
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.
|
|
PR #31398: Sync AWF v0.25.43 spec\n\n1 GitHub MCP ✅ | 2 MCP Scripts ✅ | 3 Serena ✅ | 4 Playwright ✅ | 5 Web Fetch ✅ | 6 File Write ✅ | 7 Bash ✅ | 8 Discussion ✅ | 9 Build ✅ | 10 Artifact ✅ | 11 New Discussion ✅ | 12 Dispatch ✅ | 13 PR Review ✅ | 14 Comment Memory Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
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.
|
There was a problem hiding this comment.
UGH! Caveman look at code. Code good. Remove maxRuns clean. Wire modelMultipliers smart. Me say LGTM!
Warning
Firewall blocked 6 domains
The following domains were blocked by the firewall during workflow execution:
accounts.google.comandroid.clients.google.comclients2.google.comcontentautofill.googleapis.comsafebrowsingohttpgateway.googleapis.comwww.google.com
To allow these domains, add them to the
network.allowedlist 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.
📰 BREAKING: Report filed by Smoke Copilot · ● 12.7M
| // MaxEffectiveTokens is the explicit ET budget enforced by the API proxy. | ||
| MaxEffectiveTokens int64 `json:"maxEffectiveTokens,omitempty"` | ||
|
|
||
| // ModelMultipliers configures per-model ET accounting multipliers in AWF. |
There was a problem hiding this comment.
Caveman see ModelMultipliers field. Good field. Me like JSON tag with omitempty. Very clean!
| } | ||
|
|
||
| if modelMultipliers := extractModelMultipliers(config.WorkflowData); len(modelMultipliers) > 0 { | ||
| apiProxy.ModelMultipliers = modelMultipliers |
There was a problem hiding this comment.
Caveman approve! modelMultipliers wiring good. Short. Clean. No big mess. UGH!
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
✨ Enhancement
This PR aligns
gh-awwith the latestgh-aw-firewallrelease updates referenced in #31379 by syncing the embedded AWF config schema and updating feature usage in config generation. It also updates internal drift documentation so new AWF config surface is explicitly tracked.What does this improve?
gh-awnow emits a newly supported AWF config feature from existing workflow inputs:apiProxy.modelMultipliersfromengine.token-weights.multipliersIt also carries the current schema contract from firewall
v0.25.43.Why is this valuable?
It reduces schema/implementation drift and ensures current AWF capabilities are expressed through
gh-awwithout requiring new frontmatter fields.Implementation approach:
Schema alignment
pkg/workflow/schemas/awf-config.schema.jsonto the latest firewall spec snapshot, including new fields such as:apiProxy.maxRunsapiProxy.auth.*container.dockerHostPathPrefixFeature usage update
apiProxy.modelMultiplierswhenengine.token-weights.multipliersis configuredDrift-spec refresh
specs/awf-config-sources-spec.md“known drift” coverage list to include the newly introduced config paths.Coverage updates
modelMultiplierswhen configured{ "apiProxy": { "enabled": true, "maxEffectiveTokens": 10000000, "modelMultipliers": { "gpt-5": 1.2, "gpt-5-mini": 0.8 } } }✨ PR Review Safe Output Test - Run 25649467832