Move max AI credits expression to step env and validate integer in shell run block - #58116
Conversation
… in shell run block Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. No ADR enforcement needed: PR does not have the implementation label and has <=100 new lines of code in business logic directories (38 added).
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch PR file list
|
|
No review comments posted because the pre-fetched PR diff was empty. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"See Network Configuration for more information.
|
|
✅ PR Code Quality Reviewer completed the code quality review. Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
[!TIP] tools:
github:
mode: gh-proxySee GitHub Tools for more information on To allow these domains, add them to the network:
allowed:
- defaults
- "api.github.com"
- "github.com"See Network Configuration for more information.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"
- "proxy.golang.org"See Network Configuration for more information.
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment. Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
[!TIP] tools:
github:
mode: gh-proxySee GitHub Tools for more information on To allow these domains, add them to the network:
allowed:
- defaults
- "api.github.com"
- "github.com"See Network Configuration for more information.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
No blocking correctness or maintainability regressions stood out in the max-ai-credits env migration.
### Review notes
I checked the changed runtime path in pkg/workflow/awf_command_builder.go and the engine env wiring across the touched engines. The new flow consistently moves the GitHub expression into step env, preserves the detection/evals/main default split, and validates the shell value before it is injected into the AWF config JSON. I did not find a changed-line issue that clearly justifies blocking this PR.
Warning
Firewall blocked 2 domains
The following domains were blocked by the firewall during workflow execution:
api.github.comgithub.com
[!TIP]
api.github.com is blocked because GitHub API access uses the built-in GitHub tools by default. Instead of adding api.github.com to network.allowed, use tools.github.mode: gh-proxy for direct pre-authenticated GitHub CLI access without requiring network access to api.github.com:
tools:
github:
mode: gh-proxySee GitHub Tools for more information on gh-proxy mode.
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "api.github.com"
- "github.com"See Network Configuration for more information.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 48 AIC · ⌖ 7.34 AIC · ⊞ 23.5K
Comment /review to run again
There was a problem hiding this comment.
🟡 Changes recommended
Budget validation and external-detector environment propagation have unresolved moderate issues.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Moves AI-credit expressions from executable shell bodies into step environments, adds runtime validation, and regenerates affected workflow artifacts.
Changes:
- Propagates AI-credit defaults through engine environments.
- Adds shell validation and fallback budgets.
- Refreshes tests, golden fixtures, and compiled workflows.
File summaries
| File | Description |
|---|---|
pkg/workflow/universal_llm_consumer_engine.go |
Adds budget environment propagation. |
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/with-imports.golden |
Refreshes compiled fixture. |
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/smoke-copilot.golden |
Refreshes compiled fixture. |
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/playwright-cli-mode.golden |
Refreshes compiled fixture. |
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/basic-copilot.golden |
Refreshes compiled fixture. |
pkg/workflow/testdata/TestWasmGolden_AllEngines/pi.golden |
Refreshes Pi output. |
pkg/workflow/testdata/TestWasmGolden_AllEngines/gemini.golden |
Refreshes Gemini output. |
pkg/workflow/testdata/TestWasmGolden_AllEngines/copilot.golden |
Refreshes Copilot output. |
pkg/workflow/testdata/TestWasmGolden_AllEngines/codex.golden |
Refreshes Codex output. |
pkg/workflow/testdata/TestWasmGolden_AllEngines/claude.golden |
Refreshes Claude output. |
pkg/workflow/pi_engine.go |
Adds Pi budget environment propagation. |
pkg/workflow/gemini_engine.go |
Adds budget propagation; external detection can lose the value due to first-step extraction (moderate, 1 vote). |
pkg/workflow/codex_engine.go |
Adds Codex budget environment propagation. |
pkg/workflow/claude_engine.go |
Adds Claude budget environment propagation. |
pkg/workflow/behavior_defined_engine.go |
Adds budget propagation; setup steps can prevent external detectors from receiving it (moderate, 1 vote). |
pkg/workflow/awf_config_test.go |
Tests generated budget-validation logic. |
pkg/workflow/awf_command_builder.go |
Adds shell validation; budget semantics remain incorrect (moderate, 2 votes), and an obsolete option remains (nit, 1 vote). |
.github/workflows/workflow-health-manager.lock.yml |
Regenerates budget handling. |
.github/workflows/smoke-copilot-aoai-entra.lock.yml |
Regenerates budget handling. |
.github/workflows/smoke-copilot-aoai-apikey.lock.yml |
Regenerates budget handling. |
.github/workflows/semantic-function-refactor.lock.yml |
Moves the budget expression into the environment. |
.github/workflows/schema-consistency-checker.lock.yml |
Regenerates budget handling. |
.github/workflows/safe-output-health.lock.yml |
Moves the budget expression into the environment. |
.github/workflows/pr-triage-agent.lock.yml |
Regenerates budget handling. |
.github/workflows/portfolio-analyst.lock.yml |
Moves the budget expression into the environment. |
.github/workflows/notion-issue-summary.lock.yml |
Moves the budget expression into the environment. |
.github/workflows/hippo-embed.lock.yml |
Moves the budget expression into the environment. |
.github/workflows/firewall.lock.yml |
Adds budget validation. |
.github/workflows/example-permissions-warning.lock.yml |
Moves the budget expression into the environment. |
.github/workflows/detection-analysis-report.lock.yml |
Moves the detection budget into the environment. |
.github/workflows/daily-team-evolution-insights.lock.yml |
Adds detection-budget validation. |
.github/workflows/daily-safe-output-optimizer.lock.yml |
Moves the detection budget into the environment. |
.github/workflows/daily-regulatory.lock.yml |
Moves the detection budget into the environment. |
.github/workflows/daily-observability-report.lock.yml |
Moves the detection budget into the environment. |
.github/workflows/daily-max-ai-credits-test.lock.yml |
Updates budget test workflow output. |
.github/workflows/daily-grader-audit.lock.yml |
Moves the detection budget into the environment. |
.github/workflows/daily-compiler-quality.lock.yml |
Adds detection-budget validation. |
.github/workflows/daily-cli-tools-tester.lock.yml |
Moves the detection budget into the environment. |
.github/workflows/daily-arxiv-researcher.lock.yml |
Adds detection-budget validation. |
.github/workflows/copilot-centralization-optimizer.lock.yml |
Moves the detection budget into the environment. |
.github/workflows/copilot-centralization-drilldown.lock.yml |
Moves the detection budget into the environment. |
.github/workflows/codex-github-remote-mcp-test.lock.yml |
Moves the agent budget into the environment. |
.github/workflows/code-simplifier.lock.yml |
Adds detection-budget validation. |
.github/workflows/ci-coach.lock.yml |
Adds detection-budget validation. |
.github/workflows/audit-workflows.lock.yml |
Moves the detection budget into the environment. |
.github/workflows/archivx-agentic-workflows-analyzer.lock.yml |
Moves the detection budget into the environment. |
.github/workflows/agent-performance-analyzer.lock.yml |
Adds detection-budget validation. |
.github/workflows/agent-job-health.lock.yml |
Moves the detection budget into the environment. |
Review details
- Files reviewed: 61/312 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| @@ -346,6 +346,7 @@ touch %s | |||
|
|
|||
| // Add safe outputs env | |||
| applySafeOutputEnvToMap(env, workflowData) | |||
| applyDefaultMaxAICreditsEnvToMap(env, workflowData) | |||
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design — one actionable finding on a now-dead config flag; core CodeQL fix itself is sound and well-tested.
📋 Key Themes & Highlights
Key Themes
- Dead config field:
ResolveMaxAICreditsFromEnvinAWFCommandConfigis settrueonly by the copilot engine but is never read bybuildMaxAICreditsExport/buildAWFConfigFileSetup. Behavior is unconditional for all engines now, so the flag misleads future readers into thinking other engines keep the old inline-expression path.
Positive Highlights
- ✅ Correctly moves the
${{ vars.* }}expression out of the shellrun:body and into stepenv:, addressing the CodeQLactions/code-injection/mediumfinding at its root rather than just suppressing it. - ✅ Adds a defensive
^[0-9]+$regex check before using the env value in the generated AWF config JSON, preventing malformed/injected values from reaching the firewall budget. - ✅ Good test coverage added in
awf_config_test.goandawf_env_test.gocovering agent/detection/evals default fallback paths.
Warning
Firewall blocked 2 domains
The following domains were blocked by the firewall during workflow execution:
github.comproxy.golang.org
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "github.com"
- "proxy.golang.org"See Network Configuration for more information.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 98.3 AIC · ⌖ 13.7 AIC · ⊞ 10.3K
Comment /matt to run again
|
🎉 This pull request is included in a new release. Release: |
Compiled workflows previously interpolated
${{ vars.GH_AW_DEFAULT_DETECTION_MAX_AI_CREDITS || '400' }}(and agent equivalent) directly into executable shellrun:bodies, triggering CodeQLactions/code-injection/mediumalerts. This PR shifts expression evaluation into stepenv:blocks and adds shell-level integer validation before configuring the AWF firewall budget.Changes
buildMaxAICreditsExportinpkg/workflow/awf_command_builder.goto stop embedding${{ vars.* }}expressions in the shell script. The generated bash script now checks thatGH_AW_MAX_AI_CREDITSstrictly matches^[0-9]+$, falling back to default budgets (400 for detection/evals, 1000 for agents) if invalid or empty.applyDefaultMaxAICreditsEnvToMapto Claude, Codex, Gemini, Pi, BehaviorDefined, and UniversalLLMConsumer engine execution step generators soGH_AW_MAX_AI_CREDITSis provided via YAMLenv:.pkg/workflow/awf_config_test.go, refreshed wasm golden fixtures, and recompiled workflow lock files across the repository.Example Generated Shell Change