Add AWF token steering frontmatter opt-out - #50122
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ 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 (67 additions detected). |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel analysis complete for PR #50122: Score 90/100 (Excellent). 3 tests analyzed: 100 0esign-focused, 0 0mplementation. All behavioral contracts verified (token steering override, parser extraction, nil-state handling). No violations detected. Build tags present, no mock libraries used, descriptive assertions. Inflation noted but justified. Ready for merge. |
There was a problem hiding this comment.
Pull request overview
Adds a per-workflow opt-out for AWF API proxy token steering while preserving default behavior.
Changes:
- Adds
sandbox.agent.token-steering. - Emits explicit AWF token-steering configuration with version gating.
- Updates tests, schema, autocomplete, and reference documentation.
Show a summary per file
| File | Description |
|---|---|
specs/awf-config-sources-spec.md |
Maps the new frontmatter source. |
pkg/workflow/sandbox.go |
Defines the sandbox option. |
pkg/workflow/frontmatter_extraction_security.go |
Extracts the boolean setting. |
pkg/workflow/frontmatter_extraction_security_test.go |
Tests extraction and omission. |
pkg/workflow/awf_config.go |
Emits the AWF proxy setting. |
pkg/workflow/awf_config_test.go |
Tests explicit disabling. |
pkg/parser/schemas/main_workflow_schema.json |
Adds schema validation. |
docs/src/content/docs/reference/sandbox.md |
Documents the opt-out. |
docs/src/content/docs/reference/frontmatter-full.md |
Adds the full-reference entry. |
docs/public/editor/autocomplete-data.json |
Adds editor autocomplete. |
.github/aw/syntax-agentic.md |
Adds syntax guidance. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 11/11 changed files
- Comments generated: 1
- Review effort level: Balanced
| if config.WorkflowData != nil && config.WorkflowData.SandboxConfig != nil && config.WorkflowData.SandboxConfig.Agent != nil && config.WorkflowData.SandboxConfig.Agent.TokenSteering != nil { | ||
| enableTokenSteering = *config.WorkflowData.SandboxConfig.Agent.TokenSteering | ||
| } |
PR Triage
Rationale: Adds a supported per-workflow opt-out ( Automated PR triage.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: Add AWF token steering frontmatter opt-out
The feature is well-structured — schema, extraction, config emission, tests, and docs all updated together. Two issues found:
Blocking
- Wrong example value in
frontmatter-full.md(line 2116):token-steering: trueshould betoken-steering: false. The only reason to set this field explicitly is to disable the default behavior. See inline comment.
Non-blocking
2. Silent ignore when AWF version predates token-steering support: If a user explicitly sets token-steering: false on an old pinned AWF version, the opt-out is silently dropped with no log warning. See inline comment on awf_config.go:579.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 35.7 AIC · ⌖ 10.5 AIC · ⊞ 5.4K
| # Enable or disable API proxy token steering. Set to false to preserve the | ||
| # explicitly configured provider and model. | ||
| # (optional) | ||
| token-steering: true |
There was a problem hiding this comment.
The example value should be false, not true. The entire purpose of this field is to opt out of token steering — true is already the default and does not need to be set explicitly. Showing true here will mislead readers.
# Should be:
token-steering: false@copilot please address this.
| awfConfigLog.Printf("Skipping apiProxy.enableTokenSteering: max-ai-credits is negative (disabled)") | ||
| awfConfigLog.Print("Disabling apiProxy.enableTokenSteering") | ||
| } else if !awfSupportsTokenSteering(firewallConfig) { | ||
| awfConfigLog.Printf("Skipping apiProxy.enableTokenSteering: AWF version %q requires at least %s", getAWFImageTag(firewallConfig), constants.AWFTokenSteeringMinVersion) |
There was a problem hiding this comment.
When awfSupportsTokenSteering returns false but the user explicitly set sandbox.agent.token-steering: false, the explicit opt-out is silently dropped — the log only fires for the version-gating path when enableTokenSteering is true. A user who pins an old AWF version and sets token-steering: false will get no indication that their setting was ignored.
Consider emitting a warning when the user set the field explicitly but the version does not support it:
} else if !awfSupportsTokenSteering(firewallConfig) {
if config.WorkflowData != nil && ... tokenSteering != nil {
awfConfigLog.Printf("Warning: sandbox.agent.token-steering is set but AWF version %q requires at least %s; setting ignored",
getAWFImageTag(firewallConfig), constants.AWFTokenSteeringMinVersion)
} else {
awfConfigLog.Printf("Skipping apiProxy.enableTokenSteering: ...")
}
}@copilot please address this.
There was a problem hiding this comment.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 52.2 AIC · ⌖ 8.31 AIC · ⊞ 7.1K
Comment /matt to run again
| maxAICredits = 0 | ||
| } | ||
| var tokenSteeringEnabled *bool | ||
| if awfSupportsTokenSteering(firewallConfig) && (enableTokenSteering || (config.WorkflowData != nil && config.WorkflowData.SandboxConfig != nil && config.WorkflowData.SandboxConfig.Agent != nil && config.WorkflowData.SandboxConfig.Agent.TokenSteering != nil)) { |
There was a problem hiding this comment.
[/codebase-design] The nil-guard chain on line 564 is a verbatim copy of the one on line 556, making the emit logic hard to follow. A shared local variable would eliminate the duplication and clarify intent.
💡 Suggested refactor
After line 557, capture the explicit-override flag once:
agent := func() *AgentSandboxConfig {
if config.WorkflowData != nil && config.WorkflowData.SandboxConfig != nil {
return config.WorkflowData.SandboxConfig.Agent
}
return nil
}()
explicitlySet := agent != nil && agent.TokenSteering != nilThen lines 556–564 simplify to:
if explicitlySet {
enableTokenSteering = *agent.TokenSteering
}
var tokenSteeringEnabled *bool
if awfSupportsTokenSteering(firewallConfig) && (enableTokenSteering || explicitlySet) {
tokenSteeringEnabled = &enableTokenSteering
}@copilot please address this.
| EnableTokenSteering: tokenSteeringEnabled, | ||
| } | ||
|
|
||
| if !enableTokenSteering { |
There was a problem hiding this comment.
[/diagnosing-bugs] The log message on line 577 says "Disabling" but that condition also fires when the sandbox override explicitly sets token-steering: false — the old message was more informative (it stated the reason: negative max-ai-credits). With the new opt-out path, the same branch covers two distinct causes without distinguishing them.
💡 Suggestion
Differentiate the two disablement paths in the log:
if !enableTokenSteering {
if explicitlySet {
awfConfigLog.Print("Disabling apiProxy.enableTokenSteering: sandbox.agent.token-steering=false")
} else {
awfConfigLog.Print("Disabling apiProxy.enableTokenSteering: max-ai-credits is negative")
}
}This keeps operational diagnostics actionable when users report unexpected behaviour.
@copilot please address this.
| } | ||
|
|
||
| jsonStr, err := BuildAWFConfigJSON(config) | ||
| require.NoError(t, err) |
There was a problem hiding this comment.
[/tdd] The new test verifies the happy path (sandbox disables steering), but misses the interaction case: when sandbox.agent.token-steering: false is set alongside a positive max-ai-credits, the feature should still be disabled. Without this test, a future refactor of the priority logic could silently regress the override behaviour.
💡 Suggested additional test case
t.Run("token steering sandbox override wins over positive max-ai-credits", func(t *testing.T) {
disabled := false
config := AWFCommandConfig{
EngineName: "copilot",
AllowedDomains: "github.com",
WorkflowData: &WorkflowData{
EngineConfig: &EngineConfig{ID: "copilot", MaxAICredits: 500},
NetworkPermissions: &NetworkPermissions{
Firewall: &FirewallConfig{Enabled: true},
},
SandboxConfig: &SandboxConfig{
Agent: &AgentSandboxConfig{TokenSteering: &disabled},
},
},
}
jsonStr, err := BuildAWFConfigJSON(config)
require.NoError(t, err)
assert.Contains(t, jsonStr, `"enableTokenSteering":false`)
})@copilot please address this.
| "type": "boolean", | ||
| "description": "Enable or disable API proxy token steering. Set to false to preserve the explicitly configured provider and model.", | ||
| "examples": [false] | ||
| }, |
There was a problem hiding this comment.
[/grill-with-docs] token-steering uses "type": "boolean" while its sibling model-fallback uses "$ref": "#/$defs/templatable_boolean" which also accepts GitHub Actions expressions. If a user tries token-steering: "${{ inputs.disable-steering }}" it will fail schema validation, which is an inconsistency they'll hit when parameterising workflows.
Consider using the same templatable_boolean ref — or if that's intentional (expressions not supported at runtime), add a comment in the schema noting why.
@copilot please address this.
| # Enable or disable API proxy token steering. Set to false to preserve the | ||
| # explicitly configured provider and model. | ||
| # (optional) | ||
| token-steering: true |
There was a problem hiding this comment.
[/grill-with-docs] The reference doc shows the default as token-steering: true, but the feature is a pure opt-out — the only user-facing value is false. Showing true as the canonical example may confuse authors who are looking for the opt-out pattern.
Suggest showing the opt-out value in the example, matching the usage example in sandbox.md and syntax-agentic.md:
token-steering: false@copilot please address this.
There was a problem hiding this comment.
🔎 Code quality review by PR Code Quality Reviewer · auto · 54.2 AIC · ⌖ 4.56 AIC · ⊞ 7.9K
Comment /review to run again
| // value (-1) omits that budget from the AWF config and disables token steering. | ||
| // When maxAICredits is 0 (runtime default), token steering stays enabled here. | ||
| enableTokenSteering := maxAICredits >= 0 | ||
| if config.WorkflowData != nil && config.WorkflowData.SandboxConfig != nil && config.WorkflowData.SandboxConfig.Agent != nil && config.WorkflowData.SandboxConfig.Agent.TokenSteering != nil { |
There was a problem hiding this comment.
Explicit token-steering:true silently re-enables steering even when max-ai-credits is negative (intentionally disabled), while maxAiCredits is still omitted from the config — an untested, inconsistent combination.
💡 Details
Before this PR, enableTokenSteering := maxAICredits >= 0 was the sole source of truth, and maxAICredits < 0 reliably meant "no budget, no steering". Now an explicit sandbox.agent.token-steering: true override runs after that computation but before maxAICredits is reset to 0 for omission, so:
enableTokenSteering := maxAICredits >= 0
if ...SandboxConfig.Agent.TokenSteering != nil {
enableTokenSteering = *config.WorkflowData.SandboxConfig.Agent.TokenSteering
}
if maxAICredits < 0 {
maxAICredits = 0 // omitted from config
}If a workflow sets max-ai-credits: -1 (explicitly disabling the budget) AND sandbox.agent.token-steering: true, the emitted config has enableTokenSteering:true with no maxAiCredits field. It is unclear whether AWF's token steering can function meaningfully without a budget to steer against, and no test exercises this combination (only max-ai-credits-negative alone and token-steering:false alone are covered).
Suggested fix: either ignore/reject the override when maxAICredits < 0 with a clear log message, or add a test that documents the intended behavior for this combination.
| maxAICredits = 0 | ||
| } | ||
| var tokenSteeringEnabled *bool | ||
| if awfSupportsTokenSteering(firewallConfig) && (enableTokenSteering || (config.WorkflowData != nil && config.WorkflowData.SandboxConfig != nil && config.WorkflowData.SandboxConfig.Agent != nil && config.WorkflowData.SandboxConfig.Agent.TokenSteering != nil)) { |
There was a problem hiding this comment.
The 4-level nil-check chain for SandboxConfig.Agent.TokenSteering is duplicated three times in ~15 lines — extract a helper to avoid drift.
💡 Details
if config.WorkflowData != nil && config.WorkflowData.SandboxConfig != nil && config.WorkflowData.SandboxConfig.Agent != nil && config.WorkflowData.SandboxConfig.Agent.TokenSteering != nil {This exact expression (or a near-duplicate) appears at line 556 and twice more within the if at line 564. Any future refactor of SandboxConfig/AgentSandboxConfig risks updating some occurrences but not others, silently breaking the override logic. Extract a small helper, e.g.:
func tokenSteeringOverride(wd *WorkflowData) *bool {
if wd == nil || wd.SandboxConfig == nil || wd.SandboxConfig.Agent == nil {
return nil
}
return wd.SandboxConfig.Agent.TokenSteering
}and reuse tokenSteeringOverride(config.WorkflowData) != nil / the dereferenced value at each call site. This also matches the existing pattern of small extractXxx helpers already used elsewhere in this file (e.g. extractModelFallback).
|
@copilot run pr-finisher skill |
|
🎉 Great work on this fix, What stands out:
This looks ready for review and merge. The implementation is clean, focused, and addresses exactly what the issue identified.
|
|
🎉 This pull request is included in a new release. Release: |
Token steering could override an explicitly configured provider/model with an alias-selected, unconfigured provider. Workflow authors had no supported per-workflow control to disable it.
Sandbox configuration
sandbox.agent.token-steeringboolean frontmatter.AWF configuration
apiProxy.enableTokenSteering: falsewhen explicitly disabled.max-ai-creditsbehavior.Discoverability