-
Notifications
You must be signed in to change notification settings - Fork 354
Surface pre-activation denial reason in job summary #24792
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5f092e4
c6878bd
1c92d23
516fff6
83b808e
aebc5d6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| // @ts-check | ||
| /// <reference types="@actions/github-script" /> | ||
|
|
||
| const path = require("path"); | ||
| const { renderTemplateFromFile } = require("./messages_core.cjs"); | ||
|
|
||
| /** | ||
| * Writes a pre-activation skip denial summary to the GitHub Actions job summary. | ||
| * Uses the pre_activation_skip.md template from the prompts directory when available, | ||
| * falling back to a hardcoded format when the template cannot be loaded (e.g. in tests). | ||
| * | ||
| * @param {string} reason - The denial reason message | ||
| * @param {string} remediation - Remediation hint for the operator | ||
| */ | ||
| async function writeDenialSummary(reason, remediation) { | ||
| let content; | ||
|
|
||
| const runnerTemp = process.env.RUNNER_TEMP; | ||
| if (runnerTemp) { | ||
| const templatePath = path.join(runnerTemp, "gh-aw", "prompts", "pre_activation_skip.md"); | ||
| try { | ||
| content = renderTemplateFromFile(templatePath, { reason, remediation }); | ||
| } catch (err) { | ||
| // Log unexpected errors but still fall through to the hardcoded fallback | ||
| if (err && typeof err === "object" && "code" in err && err.code !== "ENOENT") { | ||
| core.warning(`pre_activation_summary: could not read template ${templatePath}: ${err instanceof Error ? err.message : String(err)}`); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (!content) { | ||
| content = `## ⏭️ Workflow Activation Skipped\n\n> ${reason}\n\n**Remediation:** ${remediation}\n\n---\n_See the \`pre_activation\` job log for full details._`; | ||
| } | ||
|
|
||
| await core.summary.addRaw(content).write(); | ||
| } | ||
|
|
||
| module.exports = { writeDenialSummary }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| // @ts-check | ||
| import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; | ||
| import fs from "fs"; | ||
| import os from "os"; | ||
| import path from "path"; | ||
|
|
||
| describe("pre_activation_summary.cjs", () => { | ||
| let mockCore; | ||
| let originalRunnerTemp; | ||
|
|
||
| beforeEach(() => { | ||
| mockCore = { | ||
| info: vi.fn(), | ||
| warning: vi.fn(), | ||
| summary: { | ||
| addRaw: vi.fn().mockReturnThis(), | ||
| write: vi.fn().mockResolvedValue(undefined), | ||
| }, | ||
| }; | ||
| global.core = mockCore; | ||
| originalRunnerTemp = process.env.RUNNER_TEMP; | ||
| vi.resetModules(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| if (originalRunnerTemp !== undefined) { | ||
| process.env.RUNNER_TEMP = originalRunnerTemp; | ||
| } else { | ||
| delete process.env.RUNNER_TEMP; | ||
| } | ||
| delete global.core; | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| describe("writeDenialSummary", () => { | ||
| it("uses the markdown template when template file exists", async () => { | ||
| const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "gh-aw-test-")); | ||
| const promptsDir = path.join(tmpDir, "gh-aw", "prompts"); | ||
| fs.mkdirSync(promptsDir, { recursive: true }); | ||
| fs.writeFileSync(path.join(promptsDir, "pre_activation_skip.md"), "## Skipped\n\n> {reason}\n\n**Fix:** {remediation}\n", "utf8"); | ||
|
|
||
| process.env.RUNNER_TEMP = tmpDir; | ||
|
|
||
| try { | ||
| const { writeDenialSummary } = await import("./pre_activation_summary.cjs"); | ||
| await writeDenialSummary("Denied: insufficient perms", "Update frontmatter roles"); | ||
|
|
||
| expect(mockCore.summary.addRaw).toHaveBeenCalledWith("## Skipped\n\n> Denied: insufficient perms\n\n**Fix:** Update frontmatter roles\n"); | ||
| expect(mockCore.summary.write).toHaveBeenCalled(); | ||
| } finally { | ||
| fs.rmSync(tmpDir, { recursive: true, force: true }); | ||
| } | ||
| }); | ||
|
|
||
| it("falls back to hardcoded format when RUNNER_TEMP is not set", async () => { | ||
| delete process.env.RUNNER_TEMP; | ||
|
|
||
| const { writeDenialSummary } = await import("./pre_activation_summary.cjs"); | ||
| await writeDenialSummary("Bot not authorized", "Add bot to on.bots:"); | ||
|
|
||
| const rawCall = mockCore.summary.addRaw.mock.calls[0][0]; | ||
| expect(rawCall).toContain("Bot not authorized"); | ||
| expect(rawCall).toContain("Add bot to on.bots:"); | ||
| expect(mockCore.summary.write).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("falls back to hardcoded format when template file does not exist", async () => { | ||
| const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "gh-aw-test-")); | ||
| process.env.RUNNER_TEMP = tmpDir; | ||
| // No template file created | ||
|
|
||
| try { | ||
| const { writeDenialSummary } = await import("./pre_activation_summary.cjs"); | ||
| await writeDenialSummary("Stop time exceeded", "Update on.stop-after:"); | ||
|
|
||
| const rawCall = mockCore.summary.addRaw.mock.calls[0][0]; | ||
| expect(rawCall).toContain("Stop time exceeded"); | ||
| expect(rawCall).toContain("Update on.stop-after:"); | ||
| expect(mockCore.summary.write).toHaveBeenCalled(); | ||
| } finally { | ||
| fs.rmSync(tmpDir, { recursive: true, force: true }); | ||
| } | ||
| }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| ## ⏭️ Workflow Activation Skipped | ||
|
|
||
| > {reason} | ||
|
|
||
| **Remediation:** {remediation} | ||
|
|
||
| --- | ||
|
|
||
| _See the `pre_activation` job log for full details._ |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -439,6 +439,8 @@ func (c *Compiler) buildPreActivationJob(data *WorkflowData, needsPermissionChec | |||||||||||||||||||||||
| return job, nil | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // generateReportSkipStep generates the "Report skip reason" step for the pre-activation job. | ||||||||||||||||||||||||
| // The step runs with if: always() and writes skip reasons to the GitHub Actions job summary | ||||||||||||||||||||||||
| // extractPreActivationCustomFields extracts custom steps and outputs from jobs.pre-activation field in frontmatter. | ||||||||||||||||||||||||
| // It validates that only steps and outputs fields are present, and errors on any other fields. | ||||||||||||||||||||||||
| // If both jobs.pre-activation and jobs.pre_activation are defined, imports from both. | ||||||||||||||||||||||||
|
Comment on lines
+442
to
446
|
||||||||||||||||||||||||
| // generateReportSkipStep generates the "Report skip reason" step for the pre-activation job. | |
| // The step runs with if: always() and writes skip reasons to the GitHub Actions job summary | |
| // extractPreActivationCustomFields extracts custom steps and outputs from jobs.pre-activation field in frontmatter. | |
| // It validates that only steps and outputs fields are present, and errors on any other fields. | |
| // If both jobs.pre-activation and jobs.pre_activation are defined, imports from both. | |
| // extractPreActivationCustomFields extracts custom steps and outputs from the | |
| // jobs.pre-activation field in frontmatter. | |
| // It validates that only steps and outputs fields are present, and errors on | |
| // any other fields. | |
| // If both jobs.pre-activation and jobs.pre_activation are defined, it imports | |
| // from both. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The catch block intends to log “unexpected errors”, but it only warns when the thrown value has a
codeproperty and that code is notENOENT. IfrenderTemplateFromFilethrows an Error withoutcode, the error is silently swallowed and you fall back without any warning. Consider warning for all errors exceptENOENT(including whenerr.codeis missing) so template rendering problems don’t get hidden.