rename daily_effective_workflow_* → daily_ai_credits_*#38611
Conversation
…er, actions, and golden fixtures Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Renames the “daily effective workflow” guardrail output/env/context keys to the “daily AI credits” terminology across the workflow compiler (Go), runtime scripts (JS), templates (MD), tests, golden fixtures, and regenerated compiled .lock.yml workflows.
Changes:
- Updated compiler wiring to emit/read
daily_ai_credits_*activation outputs and guard conditions instead ofdaily_effective_workflow_*. - Updated setup/runtime scripts and failure templates to use the new env/context key names.
- Regenerated golden fixtures and compiled workflow lockfiles to reflect the renamed keys.
Show a summary per file
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 264/264 changed files
- Comments generated: 3
| if !strings.Contains(lockStr, "if: needs.activation.outputs.daily_ai_credits_exceeded != 'true'") { | ||
| t.Fatal("expected the agent job to be skipped when the daily workflow ET guardrail is exceeded") | ||
| } |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #38611 does not have the "implementation" label and has 52 new lines of code in business logic directories, which is at or below the 100-line threshold (requires_adr_by_default_volume=false). |
|
🧪 Test Quality Sentinel completed test quality analysis. |
|
🧠 Matt Pocock Skills Reviewer was skipped during the skills-based review. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
COMMENT — rename is mechanically correct end-to-end; two non-blocking test-quality findings.
Review summary
What was verified
- All 264 lock files, Go compiler source (
compiler_activation_job_builder.go,compiler_main_job.go,notify_comment.go,daily_aic_workflow.go), JS guardrail + failure handler, golden test fixtures, and template markdown are consistently renamed fromdaily_effective_workflow_*/GH_AW_DAILY_EFFECTIVE_WORKFLOW_*todaily_ai_credits_*/GH_AW_DAILY_AI_CREDITS_*. avenger.lock.ymlcollapses a multilineif: >to a single line — semantically equivalent, no functional change.- Compiler outputs, activation job conditions, conclusion job env wiring, and failure-match category strings are all updated consistently.
- No instances of the old names remain in source files.
Non-blocking findings (inline comments)
daily_aic_workflow_guardrail_test.golines 142–149: Two issues flagged inline — a vacuous negative assertion (line 142) and 10+ stale "ET" / "daily workflow ET" error messages (line 149 and throughout the file).
Pre-existing notes (not re-raised)
Inline comments from prior review at lines 297 and 454 of check_daily_aic_workflow_guardrail.cjs regarding the unwired daily_ai_credits_total_ai_credits output remain open and are not duplicated here.
🔎 Code quality review by PR Code Quality Reviewer · ⌖ 13.3 AIC
| } | ||
| if !strings.Contains(lockStr, "GH_AW_DAILY_EFFECTIVE_WORKFLOW_EXCEEDED: ${{ needs.activation.outputs.daily_effective_workflow_exceeded }}") { | ||
| if !strings.Contains(lockStr, "GH_AW_DAILY_AI_CREDITS_EXCEEDED: ${{ needs.activation.outputs.daily_ai_credits_exceeded }}") { | ||
| t.Fatal("expected the conclusion job to receive the daily workflow ET guardrail output") |
There was a problem hiding this comment.
Stale "ET" terminology in test assertion messages: this t.Fatal and at least 9 others in the file still say "daily workflow ET" / "daily ET" instead of "AI Credits", making test failures confusing to diagnose.
💡 All stale messages (beyond the one already flagged at line 147)
Every t.Fatal below will print old "effective tokens" / "ET" language when it fires, even though the wired outputs now use daily_ai_credits_* terminology. This will cause engineers to waste time chasing a renamed concept on a red test.
| Line | Current stale message | Suggested replacement |
|---|---|---|
| 125 | "expected activation job to include the daily workflow ET guardrail step" |
"...daily AI Credits guardrail step" |
| 140 | "expected activation job to expose the aggregated ET total output" |
"...aggregated AI Credits total output" |
| 143 | "expected activation job to avoid surfacing a separate daily workflow ET issue URL" |
"...daily AI Credits issue URL" |
| 149 | "expected the conclusion job to receive the daily workflow ET guardrail output" |
"...daily AI Credits guardrail output" |
| 158 | "expected activation permissions to avoid issues: write for the daily ET guardrail" |
"...daily AI Credits guardrail" |
| 197 | "expected activation job to emit the daily ET guardrail step even when threshold is unset" |
"...daily AI Credits guardrail step..." |
| 200 | "expected emitted daily ET guardrail step to be dynamically skipped when threshold is unset" |
"...daily AI Credits guardrail step..." |
| 203 | "expected workflows to continue wiring daily ET outputs when guardrail step is emitted" |
"...daily AI Credits outputs..." |
| 244 | "expected activation job to include the daily ET guardrail step when env var is configured" |
"...daily AI Credits guardrail step..." |
| 247 | "expected daily ET guardrail step to gate execution on GH_AW_MAX_DAILY_AI_CREDITS" |
"...daily AI Credits guardrail step..." |
| 250 | "expected setup step to conditionally install artifact client when daily ET guardrail is env-configured" |
"...daily AI Credits guardrail..." |
A single sed -i pass replacing daily (workflow )?ET guardrail → daily AI Credits guardrail and aggregated ET total → aggregated AI Credits total in this file would fix all of them.
| if !strings.Contains(lockStr, "daily_ai_credits_total_effective_tokens: ${{ steps.daily-effective-workflow-guardrail.outputs.daily_ai_credits_total_effective_tokens || '' }}") { | ||
| t.Fatal("expected activation job to expose the aggregated ET total output") | ||
| } | ||
| if strings.Contains(lockStr, "daily_effective_workflow_issue_url") { |
There was a problem hiding this comment.
Negative assertion is now vacuous: after the rename, daily_effective_workflow_issue_url no longer exists anywhere in the codebase, so this guard can never trip — the design invariant it was meant to enforce is silently unenforced.
💡 Why this matters and what to fix
The original intent was: "make sure the compiler does not emit a separate issue-URL output for the daily guardrail". After the rename, the old string daily_effective_workflow_issue_url is permanently gone, so the strings.Contains check always returns false and t.Fatal never fires — regardless of what the compiler actually emits.
If a future change accidentally introduces daily_ai_credits_issue_url as a compiler output, this test will not catch it.
Fix: update the search string to match the new naming:
// was: "daily_effective_workflow_issue_url"
if strings.Contains(lockStr, "daily_ai_credits_issue_url") {
t.Fatal("expected activation job to avoid surfacing a separate daily AI Credits issue URL")
}
🧪 Test Quality Sentinel Report✅ Test Quality Score: 90/100 — Excellent
📊 Metrics & Test Classification (3 tests analyzed)
Test Classification Details
Language SupportTests analyzed:
Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators. References: §27348883517
|
|
@copilot run pr-finisher skill |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done. I ran the PR-finisher pass and pushed follow-up fixes in |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot review issue title and reprhase "effective workflow" to AI Credits terminology. |
|
@copilot the ambient context measure is missing from the generated footers Investigate the logs in https://github.com/github/gh-aw/actions/runs/27355745225/job/80833046748 to determine why and fix. Or add more logging to debug. |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Investigated run/job logs and found the configured footer template only used Fixed in |
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Resolved. I merged |
|
@copilot review all comments and address unresolved review feedback.
|
|
@copilot merge main and recompile |
…effective-tokens Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in |
|
@copilot review all comments and address unresolved review feedback.
|
|
@copilot please refresh the branch and rerun the checks, then summarize any remaining blockers.
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Completes the rename from "daily effective tokens" to "daily AI credits" terminology across all wiring layers.
Compiler output/condition wiring
compiler_activation_job_builder.go: output keysdaily_effective_workflow_{exceeded,total_effective_tokens,threshold}→daily_ai_credits_*compiler_main_job.go: activation guard condition updatednotify_comment.go: env var namesGH_AW_DAILY_EFFECTIVE_WORKFLOW_*→GH_AW_DAILY_AI_CREDITS_*and output property accessesActions
check_daily_aic_workflow_guardrail.cjs: allcore.setOutput(...)keys renamedhandle_agent_failure.cjs: env var reads + context keydaily_effective_workflow_exceeded_context→daily_ai_credits_exceeded_context{daily_effective_workflow_exceeded_context}→{daily_ai_credits_exceeded_context}Tests & fixtures
daily_aic_workflow_guardrail_test.go: assertions updated to new key namescheck_daily_aic_workflow_guardrail.test.cjs: samedaily_aic_workflow.go: logger tagworkflow:daily_effective_workflow→workflow:daily_ai_creditsLock files
All 245
.lock.ymlfiles regenerated viamake recompile.✨ PR Review Safe Output Test - Run 27355692552
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.comSee Network Configuration for more information.
Changeset
daily_effective_workflow_*todaily_ai_credits_*.