Skip to content

Propagate resolved AI credits into failure footer context#38412

Merged
pelikhan merged 12 commits into
mainfrom
copilot/investigate-agent-failure-issue-again
Jun 10, 2026
Merged

Propagate resolved AI credits into failure footer context#38412
pelikhan merged 12 commits into
mainfrom
copilot/investigate-agent-failure-issue-again

Conversation

Copilot AI commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Conclusion-job failure issues could miss AIC in the footer when GH_AW_AIC was unset, even though AIC had already been resolved from audit/stdio fallbacks. This made budget-related failures (like #38405) lose key cost context in the issue body.

  • Root cause

    • handle_agent_failure.cjs resolved AI credits via resolveAICreditsFailureState(), but footer rendering primarily read AIC from environment-derived values.
    • In conclusion-job paths where GH_AW_AIC was not pre-populated, footer generation could skip resolved AIC, and the comment-footer path did not yet consume the propagated value.
  • Behavior change

    • Resolved aiCredits is now passed explicitly in footer context from handle_agent_failure.cjs (issue and comment paths), instead of mutating process.env.
    • Both getFooterAgentFailureIssueMessage and getFooterAgentFailureCommentMessage now honor context-provided aiCredits with existing environment fallback behavior.
    • Explicit context AIC values are normalized consistently so 0 suppresses env fallback instead of silently reverting to GH_AW_AIC, while invalid explicit values render no AIC suffix.
  • Observability

    • Added core.info diagnostics when generating failure issue/comment footers to log the aiCredits value present in footer context, making AIC propagation visible in run logs.
  • Regression coverage

    • Kept focused coverage in handle_agent_failure.test.cjs for: GH_AW_AIC unset + audit log contains ai_credits ⇒ failure issue footer includes · <AIC> AIC.
    • Added companion coverage for the failure comment path and explicit test preconditions around GH_AW_AIC being unset.
    • Added footer-level tests for explicit context AIC handling in comment footers and zero-AIC suppression of env fallback.
const { aiCredits } = resolveAICreditsFailureState();

const ctx = {
  workflowName,
  runUrl,
  workflowSource,
  workflowSourceUrl: workflowSourceURL,
  historyUrl: historyUrl || undefined,
  aiCredits,
};

Copilot AI and others added 3 commits June 10, 2026 16:56
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>
Copilot AI changed the title Investigate missing AIC in conclusion failure issue footer Propagate resolved AI credits into failure-issue footer context Jun 10, 2026
Copilot AI requested a review from pelikhan June 10, 2026 17:06
@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot NEVER mutate process.env. Fix this differently.

if (aiCredits && !process.env.GH_AW_AIC) {
process.env.GH_AW_AIC = aiCredits; # DO NOT DO THIS
}

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

@copilot NEVER mutate process.env. Fix this differently.

if (aiCredits && !process.env.GH_AW_AIC) {
...

Updated in a0029c3: removed the process.env mutation and now pass resolved aiCredits through the footer context so issue/comment footers include AIC without mutating environment state.

Copilot AI changed the title Propagate resolved AI credits into failure-issue footer context Propagate resolved AI credits into failure footer context Jun 10, 2026
@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot add core.info logging

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

@copilot add core.info logging

Added core.info diagnostics in commit 5049bd3 to log the aiCredits value when generating both failure comment and failure issue footers.

@pelikhan pelikhan marked this pull request as ready for review June 10, 2026 18:14
Copilot AI review requested due to automatic review settings June 10, 2026 18:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Propagates resolved AI Credits (AIC) into agent-failure issue footer rendering so budget-related failures include cost context even when GH_AW_AIC is unset, and adds lightweight diagnostics for footer generation.

Changes:

  • Pass aiCredits explicitly from handle_agent_failure.cjs into failure footer context (issue + comment paths) and log the value during footer generation.
  • Update getFooterAgentFailureIssueMessage to prefer context-provided aiCredits (with env fallback) and format it consistently.
  • Add regression coverage for “AIC resolved from audit log while GH_AW_AIC is unset”.
Show a summary per file
File Description
actions/setup/js/messages_footer.cjs Honors ctx.aiCredits (parsed/validated) when rendering agent-failure issue footers, while keeping env fallback behavior.
actions/setup/js/handle_agent_failure.cjs Threads aiCredits into footer context and emits core.info diagnostics for issue/comment footer generation.
actions/setup/js/handle_agent_failure.test.cjs Adds a regression test ensuring audit-log-resolved AIC appears in the failure issue footer when env is unset.
.github/workflows/daily-token-consumption-report.lock.yml Adjusts GH_AW_MODELS_JSON_PATH and uploads /tmp/gh-aw/models.json (but currently misses creating that file).

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 4/4 changed files
  • Comments generated: 2

Comment on lines 974 to 978
printf '%s\n' "{\"\$schema\":\"https://github.com/github/gh-aw-firewall/releases/download/v0.27.0/awf-config.schema.json\",\"network\":{\"allowDomains\":[\"*.githubusercontent.com\",\"*.grafana.net\",\"*.sentry.io\",\"anthropic.com\",\"api.anthropic.com\",\"api.github.com\",\"api.snapcraft.io\",\"archive.ubuntu.com\",\"azure.archive.ubuntu.com\",\"cdn.playwright.dev\",\"codeload.github.com\",\"crl.geotrust.com\",\"crl.globalsign.com\",\"crl.identrust.com\",\"crl.sectigo.com\",\"crl.thawte.com\",\"crl.usertrust.com\",\"crl.verisign.com\",\"crl3.digicert.com\",\"crl4.digicert.com\",\"crls.ssl.com\",\"files.pythonhosted.org\",\"ghcr.io\",\"github-cloud.githubusercontent.com\",\"github-cloud.s3.amazonaws.com\",\"github.com\",\"host.docker.internal\",\"json-schema.org\",\"json.schemastore.org\",\"keyserver.ubuntu.com\",\"lfs.github.com\",\"objects.githubusercontent.com\",\"ocsp.digicert.com\",\"ocsp.geotrust.com\",\"ocsp.globalsign.com\",\"ocsp.identrust.com\",\"ocsp.sectigo.com\",\"ocsp.ssl.com\",\"ocsp.thawte.com\",\"ocsp.usertrust.com\",\"ocsp.verisign.com\",\"packagecloud.io\",\"packages.cloud.google.com\",\"packages.microsoft.com\",\"playwright.download.prss.microsoft.com\",\"ppa.launchpad.net\",\"pypi.org\",\"raw.githubusercontent.com\",\"registry.npmjs.org\",\"s.symcb.com\",\"s.symcd.com\",\"security.ubuntu.com\",\"sentry.io\",\"statsig.anthropic.com\",\"ts-crl.ws.symantec.com\",\"ts-ocsp.ws.symantec.com\",\"www.googleapis.com\"]},\"apiProxy\":{\"enabled\":true,\"enableTokenSteering\":true,\"maxRuns\":500,\"maxAiCredits\":${GH_AW_MAX_AI_CREDITS},\"models\":{\"agent\":[\"sonnet-6x\",\"gpt-5.4\",\"gpt-5.3\",\"gemini-pro\",\"any\"],\"antigravity\":[\"copilot/antigravity*\",\"google/antigravity*\",\"gemini/antigravity*\"],\"any\":[\"copilot/*\",\"anthropic/*\",\"openai/*\",\"google/*\",\"gemini/*\"],\"claude\":[\"agent\"],\"codex\":[\"agent\"],\"coding\":[\"copilot/gpt-5*codex*\",\"openai/gpt-5*codex*\",\"gpt-5-codex\"],\"computer-use\":[\"copilot/*computer-use*\",\"google/*computer-use*\",\"gemini/*computer-use*\",\"openai/*computer-use*\"],\"copilot\":[\"agent\"],\"deep-research\":[\"copilot/deep-research*\",\"copilot/o3-deep-research*\",\"copilot/o4-mini-deep-research*\",\"google/deep-research*\",\"gemini/deep-research*\",\"openai/o3-deep-research*\",\"openai/o4-mini-deep-research*\"],\"gemini\":[\"agent\"],\"gemini-3-flash\":[\"copilot/gemini-3*flash*\",\"google/gemini-3*flash*\",\"gemini/gemini-3*flash*\"],\"gemini-3-pro\":[\"copilot/gemini-3*pro*\",\"google/gemini-3*pro*\",\"google/nano-banana*\",\"gemini/gemini-3*pro*\"],\"gemini-3.1-flash\":[\"copilot/gemini-3.1*flash*\",\"google/gemini-3.1*flash*\",\"gemini/gemini-3.1*flash*\"],\"gemini-3.1-pro\":[\"copilot/gemini-3.1*pro*\",\"google/gemini-3.1*pro*\",\"gemini/gemini-3.1*pro*\"],\"gemini-3.5-flash\":[\"copilot/gemini-3.5*flash*\",\"google/gemini-3.5*flash*\",\"gemini/gemini-3.5*flash*\"],\"gemini-flash\":[\"copilot/gemini-*flash*\",\"google/gemini-*flash*\",\"gemini/gemini-*flash*\"],\"gemini-flash-lite\":[\"copilot/gemini-*flash*lite*\",\"google/gemini-*flash*lite*\",\"gemini/gemini-*flash*lite*\"],\"gemini-pro\":[\"copilot/gemini-*pro*\",\"google/gemini-*pro*\",\"gemini/gemini-*pro*\"],\"gemma\":[\"copilot/gemma*\",\"google/gemma*\",\"gemini/gemma*\"],\"gpt-5\":[\"copilot/gpt-5*\",\"openai/gpt-5*\"],\"gpt-5-codex\":[\"copilot/gpt-5*codex*\",\"openai/gpt-5*codex*\"],\"gpt-5-mini\":[\"copilot/gpt-5*mini*\",\"openai/gpt-5*mini*\"],\"gpt-5-nano\":[\"copilot/gpt-5*nano*\",\"openai/gpt-5*nano*\"],\"gpt-5-pro\":[\"copilot/gpt-5*pro*\",\"openai/gpt-5*pro*\"],\"gpt-5.2\":[\"copilot/gpt-5.2*\",\"openai/gpt-5.2*\"],\"gpt-5.3\":[\"copilot/gpt-5.3*\",\"openai/gpt-5.3*\"],\"gpt-5.4\":[\"copilot/gpt-5.4*\",\"openai/gpt-5.4*\"],\"gpt-5.5\":[\"copilot/gpt-5.5*\",\"openai/gpt-5.5*\"],\"haiku\":[\"copilot/*haiku*\",\"anthropic/*haiku*\"],\"large\":[\"sonnet\",\"gpt-5-pro\",\"gpt-5\",\"gemini-pro\"],\"mai-code\":[\"copilot/MAI-Code*\",\"copilot/mai-code*\",\"openai/MAI-Code*\"],\"mini\":[\"haiku\",\"gpt-5-mini\",\"gpt-5-nano\",\"gemini-flash-lite\"],\"nano-banana\":[\"copilot/nano-banana*\",\"google/nano-banana*\",\"gemini/nano-banana*\"],\"opus\":[\"copilot/*opus*\",\"anthropic/*opus*\"],\"opusplan\":[\"opus?effort=high\"],\"reasoning\":[\"copilot/o1*\",\"copilot/o3*\",\"copilot/o4*\",\"openai/o1*\",\"openai/o3*\",\"openai/o4*\"],\"robotics\":[\"copilot/*robotics*\",\"google/*robotics*\",\"gemini/*robotics*\"],\"small\":[\"mini\"],\"small-agent\":[\"haiku\",\"gpt-5-mini\",\"gemini-flash\"],\"sonnet\":[\"copilot/*sonnet*\",\"anthropic/*sonnet*\"],\"sonnet-6x\":[\"copilot/*sonnet-4.5*\",\"copilot/*sonnet-4.6*\",\"copilot/*sonnet-4-5-*\",\"anthropic/*sonnet-4-5-*\",\"copilot/*sonnet-4-6*\",\"anthropic/*sonnet-4-6*\"],\"summarization\":[\"haiku\",\"gpt-5-mini\",\"gemini-flash-lite\",\"mini\"],\"vision\":[\"copilot/gemini-*image*\",\"gemini/gemini-*image*\",\"copilot/gemini-*flash*\",\"gemini/gemini-*flash*\"]}},\"container\":{\"imageTag\":\"0.27.0\"}}" > "${RUNNER_TEMP}/gh-aw/awf-config.json"
GH_AW_MODEL_MULTIPLIERS_PATH="/tmp/gh-aw/model_multipliers.json" node "${RUNNER_TEMP}/gh-aw/actions/merge_awf_model_multipliers.cjs"
cp "${RUNNER_TEMP}/gh-aw/awf-config.json" /tmp/gh-aw/awf-config.json
export GH_AW_MODELS_JSON_PATH="${RUNNER_TEMP}/gh-aw/actions/models.json"
export GH_AW_MODELS_JSON_PATH="/tmp/gh-aw/models.json"
GH_AW_DOCKER_HOST_PATH_PREFIX_ARGS=""
Comment on lines 1551 to 1555
printf '%s\n' "{\"\$schema\":\"https://github.com/github/gh-aw-firewall/releases/download/v0.27.0/awf-config.schema.json\",\"network\":{\"allowDomains\":[\"*.githubusercontent.com\",\"anthropic.com\",\"api.anthropic.com\",\"api.github.com\",\"api.snapcraft.io\",\"archive.ubuntu.com\",\"azure.archive.ubuntu.com\",\"cdn.playwright.dev\",\"codeload.github.com\",\"crl.geotrust.com\",\"crl.globalsign.com\",\"crl.identrust.com\",\"crl.sectigo.com\",\"crl.thawte.com\",\"crl.usertrust.com\",\"crl.verisign.com\",\"crl3.digicert.com\",\"crl4.digicert.com\",\"crls.ssl.com\",\"files.pythonhosted.org\",\"ghcr.io\",\"github-cloud.githubusercontent.com\",\"github-cloud.s3.amazonaws.com\",\"github.com\",\"host.docker.internal\",\"json-schema.org\",\"json.schemastore.org\",\"keyserver.ubuntu.com\",\"lfs.github.com\",\"objects.githubusercontent.com\",\"ocsp.digicert.com\",\"ocsp.geotrust.com\",\"ocsp.globalsign.com\",\"ocsp.identrust.com\",\"ocsp.sectigo.com\",\"ocsp.ssl.com\",\"ocsp.thawte.com\",\"ocsp.usertrust.com\",\"ocsp.verisign.com\",\"packagecloud.io\",\"packages.cloud.google.com\",\"packages.microsoft.com\",\"playwright.download.prss.microsoft.com\",\"ppa.launchpad.net\",\"pypi.org\",\"raw.githubusercontent.com\",\"registry.npmjs.org\",\"s.symcb.com\",\"s.symcd.com\",\"security.ubuntu.com\",\"sentry.io\",\"statsig.anthropic.com\",\"ts-crl.ws.symantec.com\",\"ts-ocsp.ws.symantec.com\"]},\"apiProxy\":{\"enabled\":true,\"enableTokenSteering\":true,\"maxRuns\":500,\"maxAiCredits\":${GH_AW_MAX_AI_CREDITS}},\"container\":{\"imageTag\":\"0.27.0\"}}" > "${RUNNER_TEMP}/gh-aw/awf-config.json"
GH_AW_MODEL_MULTIPLIERS_PATH="/tmp/gh-aw/model_multipliers.json" node "${RUNNER_TEMP}/gh-aw/actions/merge_awf_model_multipliers.cjs"
cp "${RUNNER_TEMP}/gh-aw/awf-config.json" /tmp/gh-aw/awf-config.json
export GH_AW_MODELS_JSON_PATH="${RUNNER_TEMP}/gh-aw/actions/models.json"
export GH_AW_MODELS_JSON_PATH="/tmp/gh-aw/models.json"
GH_AW_DOCKER_HOST_PATH_PREFIX_ARGS=""
@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel completed test quality analysis.

Caution

agentic threat detected
Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.

Details

Potential security threats were detected in the agent output.

Review the workflow run logs for details.

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

PR Code Quality Reviewer completed the code quality review.

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #38412 does not have the 'implementation' label and has 0 new lines of code in business logic directories (≤100 threshold).

@github-actions github-actions Bot mentioned this pull request Jun 10, 2026
@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot run pr-finisher skill

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REQUEST_CHANGES — two correctness issues and an incomplete fix block merge.

🔎 Findings summary

Blocking

  1. aiCredits dead code in comment footer (handle_agent_failure.cjs:2843) — aiCredits is added to the comment ctx and logged as if it were used, but getFooterAgentFailureCommentMessage never processes it. The AIC fix is silently absent from comment footers.

  2. parsePositiveAIC rejects zero, falls back to env (messages_footer.cjs:342) — a resolved ctx.aiCredits = 0 triggers a silent fallback to the env var. The getFooterMessage equivalent uses ctx.aiCredits ?? envAIC which handles zero correctly.

  3. hasExplicitContextAIC logic diverges from getFooterMessage (messages_footer.cjs:344) — two parallel code paths behave differently for ctx.aiCredits = 0 and non-numeric strings. They should share a single guard pattern.

Non-blocking

  1. Dead is:pr branch reintroduced in new test (handle_agent_failure.test.cjs:246) — both arms return the same object; this contradicts the cleanup made to the existing test in the same PR.

🔎 Code quality review by PR Code Quality Reviewer · ⌖ 13.9 AIC

workflowSource,
workflowSourceUrl: workflowSourceURL,
historyUrl: historyUrl || undefined,
aiCredits,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aiCredits is passed to getFooterAgentFailureCommentMessage but the function ignores it — the comment footer will never show AIC, making this field dead code and the core.info log misleading.

💡 Details

getFooterAgentFailureCommentMessage in messages_footer.cjs (lines 395–422) never calls getAICFromEnv(), never computes aiCreditsSuffix, and the default template contains no {ai_credits_suffix} placeholder. Passing aiCredits into ctx has zero effect on the rendered comment footer.

The diagnostic log on line 2845 says "Generating failure comment footer with aiCredits context: X" — which implies AIC was incorporated, but it was not. This misleads anyone debugging why AIC is absent from a comment.

Fix one of:

  • If AIC should appear in comment footers (consistent with the fix for issue footers): update getFooterAgentFailureCommentMessage to process ctx.aiCredits and include {ai_credits_suffix} in its default template, mirroring the new getFooterAgentFailureIssueMessage logic.
  • If AIC is intentionally omitted from comment footers: remove aiCredits from the comment ctx and drop the misleading core.info log.

Comment thread actions/setup/js/messages_footer.cjs Outdated
threatDetectionAiCreditsSuffix,
} = getAICFromEnv();
const { ambientContext, ambientContextFormatted, ambientContextSuffix } = getAmbientContextFromEnv();
const explicitContextAIC = parsePositiveAIC(ctx.aiCredits != null ? String(ctx.aiCredits) : undefined);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parsePositiveAIC rejects ctx.aiCredits = 0, silently falling back to the env var — zero is a valid resolved AIC and should suppress the env value, not defer to it.

💡 Details
const explicitContextAIC = parsePositiveAIC(ctx.aiCredits != null ? String(ctx.aiCredits) : undefined);
const aiCredits = explicitContextAIC ?? envAIC;

parsePositiveAIC requires parsed > 0 (strictly positive). If ctx.aiCredits = 0 — an agent that ran but consumed zero AI credits — parsePositiveAIC returns undefined, and the ?? falls back to envAIC. The explicitly-resolved audit-log value is silently discarded.

Compare with getFooterMessage (line 158), which simply does:

const aiCredits = ctx.aiCredits ?? envAIC;

This correctly uses 0 when explicitly passed and only falls back to env for null/undefined.

Suggested fix — mirror the getFooterMessage pattern:

const aiCredits = ctx.aiCredits != null ? ctx.aiCredits : envAIC;
const hasExplicitContextAIC = ctx.aiCredits !== undefined && ctx.aiCredits !== null;

Comment thread actions/setup/js/messages_footer.cjs Outdated
const { ambientContext, ambientContextFormatted, ambientContextSuffix } = getAmbientContextFromEnv();
const explicitContextAIC = parsePositiveAIC(ctx.aiCredits != null ? String(ctx.aiCredits) : undefined);
const aiCredits = explicitContextAIC ?? envAIC;
const hasExplicitContextAIC = explicitContextAIC !== undefined;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hasExplicitContextAIC diverges from the same flag in getFooterMessage — the two implementations behave differently for the same inputs, creating a maintenance hazard.

💡 Details

Here (line 344):

const hasExplicitContextAIC = explicitContextAIC !== undefined;
// where explicitContextAIC = parsePositiveAIC(String(ctx.aiCredits))

In getFooterMessage (line 167):

const hasExplicitContextAIC = ctx.aiCredits !== undefined && ctx.aiCredits !== null;

Behavioral differences:

ctx.aiCredits This function getFooterMessage
0 false (falls back to env) true (suppresses env)
"invalid" false (falls back to env) true (aiCredits = "invalid", formatting returns undefined)
undefined false false
2.5 true true

The two code paths should share a single helper or use the same guard expression to avoid silent divergence.

global.github = {
rest: {
search: {
issuesAndPullRequests: vi.fn(async ({ q }) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead branch reintroduced: both arms of this if return identical objects, making the condition unreachable dead code — the same pattern that was cleaned up in the existing test earlier in this PR.

💡 Details
issuesAndPullRequests: vi.fn(async ({ q }) => {
  if (q.includes("is:pr")) {
    return { data: { total_count: 0, items: [] } }; // same as below
  }
  return { data: { total_count: 0, items: [] } };   // same
}),

This is exactly what the existing test mock was simplified to (removing the q.includes("is:pr") branch) earlier in this PR. Apply the same simplification here:

issuesAndPullRequests: vi.fn(async () => ({
  data: { total_count: 0, items: [] },
})),

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skills-Based Review 🧠

Applied /diagnose and /tdd — no blocking issues, but three meaningful gaps worth addressing.

📋 Key Themes & Highlights

Key Themes

  • Comment footer not fully wired (/diagnose): getFooterAgentFailureCommentMessage shares the updated AgentFailureContext type (with aiCredits) but never calls getAICFromEnv() or computes aiCreditsSuffix. Custom agentFailureComment templates that reference {ai_credits_suffix} would silently receive undefined. The issue footer fix (lines 330–346) should be mirrored in the comment footer.

  • Regression test precondition is implicit (/tdd): The new test names its scenario "GH_AW_AIC is unset" but never asserts that fact at the start of the test body. It passes today because the prior test cleans up in finally, but any future ordering change or missed cleanup would cause it to validate the wrong path.

  • Comment path has no AIC test coverage (/tdd): handle_agent_failure.cjs now passes aiCredits to both footer call sites, but only the issue-creation path is asserted in tests. The comment-creation path is dark.

Positive Highlights

  • ✅ Root cause correctly identified and addressed: explicit context propagation beats env-mutation for correctness in conclusion-job paths
  • ✅ The two-tier precedence (explicitContextAIC ?? envAIC) in getFooterAgentFailureIssueMessage is clean and backward-compatible
  • core.info diagnostics at both call sites are a good observability addition — makes AIC propagation visible in run logs
  • ✅ Regression test added for the exact failure scenario from #38405

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 305 AIC · ⌖ 13.6 AIC

const aiCredits = explicitContextAIC ?? envAIC;
const hasExplicitContextAIC = explicitContextAIC !== undefined;
const aiCreditsFormatted = hasExplicitContextAIC ? (aiCredits ? formatAIC(aiCredits) : undefined) : envAICFormatted;
const aiCreditsSuffix = hasExplicitContextAIC ? (aiCreditsFormatted ? ` · ${aiCreditsFormatted} AIC` : "") : envAICSuffix;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/diagnose] getFooterAgentFailureCommentMessage (line 395) shares the AgentFailureContext type but does not compute aiCreditsSuffix — so custom agentFailureComment templates using {ai_credits_suffix} would receive undefined, silently producing a malformed footer.

💡 Suggested fix for getFooterAgentFailureCommentMessage

The issue path now correctly resolves aiCreditsSuffix via explicitContextAIC ?? envAIC (lines 342–346 here). The comment path should mirror this:

// In getFooterAgentFailureCommentMessage:
const { aiCredits: envAIC, aiCreditsFormatted: envAICFormatted, aiCreditsSuffix: envAICSuffix } = getAICFromEnv();
const explicitContextAIC = parsePositiveAIC(ctx.aiCredits != null ? String(ctx.aiCredits) : undefined);
const aiCredits = explicitContextAIC ?? envAIC;
const hasExplicitContextAIC = explicitContextAIC !== undefined;
const aiCreditsFormatted = hasExplicitContextAIC ? (aiCredits ? formatAIC(aiCredits) : undefined) : envAICFormatted;
const aiCreditsSuffix = hasExplicitContextAIC ? (aiCreditsFormatted ? ` · ${aiCreditsFormatted} AIC` : "") : envAICSuffix;
const templateContext = toSnakeCase({ ...ctx, historyLink, agenticWorkflowUrl, aiCredits, aiCreditsFormatted, aiCreditsSuffix });

This ensures the raw ctx.aiCredits string passed from handle_agent_failure.cjs is properly parsed and that {ai_credits_suffix} is available to custom templates.

}
});

it("includes AIC in failure issue footer when resolved from audit log and GH_AW_AIC is unset", async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] The test title says "GH_AW_AIC is unset" but the test body never asserts that precondition. If a prior test sets GH_AW_AIC and its cleanup runs after this one (e.g. a flaky finally), the new test could pass because env AIC is set — not because the audit-log fallback path works.

💡 Add an explicit guard

Add a precondition check at the top of the test:

assert.equal(
  process.env.GH_AW_AIC,
  undefined,
  "GH_AW_AIC must be unset so the audit-log fallback is exercised"
);

This turns an implicit reliance on test-ordering into a loud, self-diagnosing failure if the assumption ever breaks.


try {
await main();
expect(capturedIssueBody).toContain("> Generated from [Test Workflow](https://github.com/owner/repo/actions/runs/123456) · 2.5 AIC");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] Only the issue-creation path is tested. The PR description says aiCredits is now propagated in both the comment and issue paths, but there is no corresponding assertion that the failure comment footer (via getFooterAgentFailureCommentMessage) also carries through the AIC value. Without that coverage, a future regression in the comment path would go undetected.

💡 Suggested additional test

Add a companion test that captures the comment body (from issues.createComment or equivalent) and asserts it contains the expected AIC string, mirroring this test but targeting the comment code path in handle_agent_failure.cjs.

@github-actions

Copy link
Copy Markdown
Contributor

Caution

agentic threat detected
Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.

Details

Potential security threats were detected in the agent output.

Review the workflow run logs for details.

test post

🧪 Test quality analysis by Test Quality Sentinel · 450.3 AIC · ⌖ 25.2 AIC ·

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Test Quality Sentinel: 90/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). The new regression test for AIC-from-audit-log propagation is a well-structured behavioral contract test.

pelikhan and others added 7 commits June 10, 2026 11:46
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…e-issue-again' into copilot/investigate-agent-failure-issue-again

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>
…e-issue-again' into copilot/investigate-agent-failure-issue-again

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

@copilot run pr-finisher skill

Ran the PR-finisher pass. Latest work is on 345f99a, which syncs the branch after merging the latest main; local make fmt, make lint, make test, and make recompile passed on the updated branch.

@pelikhan pelikhan merged commit 3faf908 into main Jun 10, 2026
20 checks passed
@pelikhan pelikhan deleted the copilot/investigate-agent-failure-issue-again branch June 10, 2026 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants