Skip to content

Fix comment-memory disclosure template lookup in safe_outputs #27989

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.

Sign up for GitHub

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

Merged
pelikhan merged 5 commits into from
Apr 23, 2026
+35 −24
Merged

Fix comment-memory disclosure template lookup in safe_outputs #27989

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cb266a7
Plan: fix comment-memory disclosure template path resolution
Copilot Apr 23, 2026
15155e3
Fix comment_memory disclosure note path in safe_outputs runtime
Copilot Apr 23, 2026
f61b850
Refactor comment_memory template loading to match standard helper pat…
Copilot Apr 23, 2026
a0e16d2
Use fileURLToPath in comment_memory test for cross-platform path hand…
Copilot Apr 23, 2026
e6c0800
Render comment_memory disclosure template on demand via helper
Copilot Apr 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions .github/workflows/agentics-maintenance.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,26 +389,26 @@ jobs:
const fs = require('node:fs');
const reportPath = './.cache/gh-aw/activity-report-logs/report.md';
if (!fs.existsSync(reportPath)) {
core.warning(`Activity report markdown not found at ${reportPath}; skipping issue creation.`);
core.warning('Activity report markdown not found at ' + reportPath + '; skipping issue creation.');
return;
}
let reportBody = '';
try {
reportBody = fs.readFileSync(reportPath, 'utf8').trim();
} catch (error) {
core.warning(`Failed to read activity report markdown at ${reportPath}: ${error.message}`);
core.warning('Failed to read activity report markdown at ' + reportPath + ': ' + error.message);
return;
}
if (!reportBody) {
core.warning(`Activity report markdown is empty at ${reportPath}; skipping issue creation.`);
core.warning('Activity report markdown is empty at ' + reportPath + '; skipping issue creation.');
return;
}
const repoSlug = `${context.repo.owner}/${context.repo.repo}`;
const repoSlug = context.repo.owner + '/' + context.repo.repo;
const body = [
'### Agentic workflow activity report',
'',
`Repository: ${repoSlug}`,
`Generated at: ${new Date().toISOString()}`,
'Repository: ' + repoSlug,
'Generated at: ' + new Date().toISOString(),
'',
reportBody,
].join('\n');
Expand All @@ -419,7 +419,7 @@ jobs:
body,
labels: ['agentic-workflows'],
});
core.info(`Created issue #${createdIssue.data.number}: ${createdIssue.data.html_url}`);
core.info('Created issue #' + createdIssue.data.number + ': ' + createdIssue.data.html_url);

close_agentic_workflows_issues:
if: ${{ (github.event_name == 'workflow_dispatch' || github.event_name == 'workflow_call') && inputs.operation == 'close_agentic_workflows_issues' && (!(github.event.repository.fork)) }}
Expand Down
26 changes: 10 additions & 16 deletions actions/setup/js/comment_memory.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
/// <reference types="@actions/github-script" />
require("./shim.cjs");

const path = require("path");
const { sanitizeContent } = require("./sanitize_content.cjs");
const { getErrorMessage } = require("./error_helpers.cjs");
const { SAFE_OUTPUT_E001 } = require("./error_codes.cjs");
Expand All @@ -20,7 +19,14 @@ const { COMMENT_MEMORY_TAG, COMMENT_MEMORY_MAX_SCAN_PAGES } = require("./comment
// that happen to contain a matching comment-memory tag.
const MANAGED_COMMENT_PROVENANCE_MARKER = "<!-- gh-aw-agentic-workflow:";
const MANAGED_COMMENT_HEADER = "### Comment Memory";
const MANAGED_COMMENT_DISCLOSURE_NOTE_PATH = path.join(__dirname, "../md/comment_memory_disclosure_note.md");

function renderManagedCommentDisclosureNote() {
const promptsDir = process.env.GH_AW_PROMPTS_DIR || `${process.env.RUNNER_TEMP}/gh-aw/prompts`;
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

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

renderManagedCommentDisclosureNote() falls back to ${process.env.RUNNER_TEMP}/gh-aw/prompts when GH_AW_PROMPTS_DIR is unset, but if RUNNER_TEMP is missing this produces a path starting with undefined/... and the ensuing readFileSync failure can be confusing to diagnose. Consider adding an explicit guard that throws/logs a clear error when neither GH_AW_PROMPTS_DIR nor RUNNER_TEMP is available.

Suggested change
const promptsDir = process.env.GH_AW_PROMPTS_DIR || `${process.env.RUNNER_TEMP}/gh-aw/prompts`;
const configuredPromptsDir = process.env.GH_AW_PROMPTS_DIR;
const runnerTemp = process.env.RUNNER_TEMP;
if (!configuredPromptsDir && !runnerTemp) {
throw new Error("comment_memory: unable to resolve prompts directory; set GH_AW_PROMPTS_DIR or ensure RUNNER_TEMP is available");
}
const promptsDir = configuredPromptsDir || `${runnerTemp}/gh-aw/prompts`;

Copilot uses AI. Check for mistakes.
const templatePath = `${promptsDir}/comment_memory_disclosure_note.md`;
return renderTemplateFromFile(templatePath, {
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot Apr 23, 2026

Choose a reason for hiding this comment

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

The new renderManagedCommentDisclosureNote helper nicely encapsulates the prompts-dir resolution logic. Consider caching the result if this function is called multiple times per run to avoid repeated file reads.

comment_memory_tag: COMMENT_MEMORY_TAG,
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot Apr 23, 2026

Choose a reason for hiding this comment

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

🔍 Smoke Test Review: The renderManagedCommentDisclosureNote helper correctly uses GH_AW_PROMPTS_DIR env var with a fallback to RUNNER_TEMP/gh-aw/prompts. This aligns with the standard template path resolution pattern used by other handlers — good consistency!

});
}

function sanitizeMemoryID(memoryID) {
const normalized = String(memoryID || "default").trim();
Expand All @@ -32,7 +38,7 @@ function sanitizeMemoryID(memoryID) {
}

function buildManagedMemoryBody(rawBody, memoryID, options) {
const { includeFooter, runUrl, workflowName, workflowSource, workflowSourceURL, historyUrl, triggeringIssueNumber, triggeringPRNumber, disclosureNote } = options;
const { includeFooter, runUrl, workflowName, workflowSource, workflowSourceURL, historyUrl, triggeringIssueNumber, triggeringPRNumber } = options;
if (!/^[a-zA-Z0-9_-]+$/.test(memoryID)) {
throw new Error(`${SAFE_OUTPUT_E001}: memory_id must contain only alphanumeric characters, hyphens, and underscores`);
}
Expand All @@ -48,11 +54,7 @@ function buildManagedMemoryBody(rawBody, memoryID, options) {

if (includeFooter) {
core.info(`comment_memory: footer enabled for memory_id='${memoryID}'`);
const resolvedDisclosureNote =
disclosureNote ??
renderTemplateFromFile(MANAGED_COMMENT_DISCLOSURE_NOTE_PATH, {
comment_memory_tag: COMMENT_MEMORY_TAG,
});
const resolvedDisclosureNote = renderManagedCommentDisclosureNote();
body += "\n\n" + resolvedDisclosureNote;
Comment on lines 55 to 58
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

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

buildManagedMemoryBody() now calls renderManagedCommentDisclosureNote() each time includeFooter is true, which re-reads the disclosure note template from disk for every processed item. Consider caching the rendered disclosure note (e.g., module-level lazy cache) or resolving it once per main() invocation to avoid repeated synchronous file I/O when maxCount > 1 and to keep this hot path cheaper.

Copilot uses AI. Check for mistakes.
body += "\n\n" + generateFooterWithMessages(workflowName, runUrl, workflowSource, workflowSourceURL, triggeringIssueNumber, triggeringPRNumber, undefined, historyUrl).trimEnd();
} else {
Expand Down Expand Up @@ -118,7 +120,6 @@ async function main(config = {}) {
core.info(`comment_memory: initialized with max=${maxCount}, defaultMemoryID='${defaultMemoryID}', target='${target}', footer=${includeFooter}, staged=${staged}`);

let processedCount = 0;
let cachedDisclosureNote = null;

return async message => {
if (!message || message.type !== "comment_memory") {
Expand Down Expand Up @@ -175,12 +176,6 @@ async function main(config = {}) {
serverUrl: context.serverUrl,
}) || undefined;

if (cachedDisclosureNote === null) {
cachedDisclosureNote = renderTemplateFromFile(MANAGED_COMMENT_DISCLOSURE_NOTE_PATH, {
comment_memory_tag: COMMENT_MEMORY_TAG,
});
}

const managedBody = buildManagedMemoryBody(message.body || "", memoryID, {
includeFooter,
runUrl,
Expand All @@ -190,7 +185,6 @@ async function main(config = {}) {
historyUrl,
triggeringIssueNumber,
triggeringPRNumber,
disclosureNote: cachedDisclosureNote,
});
try {
enforceCommentLimits(managedBody);
Expand Down
19 changes: 18 additions & 1 deletion actions/setup/js/comment_memory.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,24 @@
// @ts-check
import { describe, it, expect } from "vitest";
import { describe, it, expect, beforeAll, afterAll } from "vitest";
import path from "path";
import { fileURLToPath } from "url";

describe("comment_memory", () => {
let originalPromptsDir;

beforeAll(() => {
originalPromptsDir = process.env.GH_AW_PROMPTS_DIR;
process.env.GH_AW_PROMPTS_DIR = path.join(path.dirname(fileURLToPath(import.meta.url)), "../md");
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot Apr 23, 2026

Choose a reason for hiding this comment

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

🔍 Smoke Test Review: The beforeAll / afterAll hooks properly set GH_AW_PROMPTS_DIR to point to the local md/ directory during tests and restore the original value afterward. This is a clean approach for cross-platform path handling — the use of fileURLToPath(import.meta.url) avoids brittle __dirname usage in ESM contexts.

});

afterAll(() => {
if (originalPromptsDir === undefined) {
delete process.env.GH_AW_PROMPTS_DIR;
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot Apr 23, 2026

Choose a reason for hiding this comment

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

Good addition of beforeAll/afterAll hooks to properly isolate the GH_AW_PROMPTS_DIR env var during tests. This pattern ensures test state doesn't leak between test runs.

return;
}
process.env.GH_AW_PROMPTS_DIR = originalPromptsDir;
});

it("sanitizes valid memory IDs", async () => {
const module = await import("./comment_memory.cjs");
expect(module.sanitizeMemoryID("Session_1")).toBe("Session_1");
Expand Down
Loading