Fix comment-memory disclosure template lookup in safe_outputs#27989
Merged
Fix comment-memory disclosure template lookup in safe_outputs#27989
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3dc69fc0-7d6b-4c07-ba59-82f528722877 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3dc69fc0-7d6b-4c07-ba59-82f528722877 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
pelikhan
April 23, 2026 05:46
View session
pelikhan
reviewed
Apr 23, 2026
Comment on lines
+23
to
+36
| const MANAGED_COMMENT_DISCLOSURE_NOTE_PATH = path.join(__dirname, "../md/comment_memory_disclosure_note.md"); | ||
| const MANAGED_COMMENT_DISCLOSURE_NOTE_FILENAME = "comment_memory_disclosure_note.md"; | ||
|
|
||
| function resolveManagedCommentDisclosureNotePath() { | ||
| const promptDirFromEnv = process.env.GH_AW_PROMPTS_DIR || (process.env.RUNNER_TEMP ? `${process.env.RUNNER_TEMP}/gh-aw/prompts` : ""); | ||
| const candidates = [promptDirFromEnv ? path.join(promptDirFromEnv, MANAGED_COMMENT_DISCLOSURE_NOTE_FILENAME) : "", path.join(__dirname, "../md", MANAGED_COMMENT_DISCLOSURE_NOTE_FILENAME)].filter(Boolean); | ||
|
|
||
| for (const candidate of candidates) { | ||
| if (fs.existsSync(candidate)) { | ||
| return candidate; | ||
| } | ||
| } | ||
|
|
||
| return candidates[0]; |
Collaborator
There was a problem hiding this comment.
@copilot use render template from file helper like other templates
Contributor
Author
There was a problem hiding this comment.
Contributor
There was a problem hiding this comment.
Smoke test agent here — noted! The template-from-file approach is cleaner and consistent with the rest of the codebase. ✅
📰 BREAKING: Report filed by Smoke Copilot · ● 2M
…tern Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3341b7d6-7dd8-4e12-933b-8cfaa390b0d5 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…ling Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3341b7d6-7dd8-4e12-933b-8cfaa390b0d5 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a267f359-ae3c-43d1-8edd-19d6b6c78eee Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Done in e6c0800. I switched to an on-demand helper function that calls Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Contributor
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
Contributor
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
Contributor
|
|
Contributor
|
🚀 Smoke OpenCode MISSION COMPLETE! OpenCode delivered. 🔥 |
Contributor
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
Contributor
|
|
Contributor
Agent Container Tool Check
Result: 12/12 tools available ✅ Overall Status: PASS
|
Contributor
|
Smoke test (run 24820197109): Warning The following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"
- "chatgpt.com"See Network Configuration for more information.
|
Contributor
Comment MemoryMorning builds run true Quiet logs drift through cold cache paths Green checks wake the branchNote This comment is managed by comment memory. What this comment doesIt stores persistent context for this thread in the Warning The following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"
- "chatgpt.com"See Network Configuration for more information.
|
Contributor
|
Smoke Test Results — Run §24820197107 Core (#1–#12): ✅✅✅✅✅✅✅✅✅❌✅✅ Status: PARTIAL — test #10 (agentic-workflows MCP) failed (MCP error -32603), #16 (resolve thread)
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal! The comment_memory disclosure template refactor looks solid — aligning template loading with the standard renderTemplateFromFile helper pattern improves maintainability and fixes the runtime path resolution issue.
💥 [THE END] — Illustrated by Smoke Claude · ● 218.8K
| const promptsDir = process.env.GH_AW_PROMPTS_DIR || `${process.env.RUNNER_TEMP}/gh-aw/prompts`; | ||
| const templatePath = `${promptsDir}/comment_memory_disclosure_note.md`; | ||
| return renderTemplateFromFile(templatePath, { | ||
| comment_memory_tag: COMMENT_MEMORY_TAG, |
Contributor
There was a problem hiding this comment.
🔍 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!
|
|
||
| beforeAll(() => { | ||
| originalPromptsDir = process.env.GH_AW_PROMPTS_DIR; | ||
| process.env.GH_AW_PROMPTS_DIR = path.join(path.dirname(fileURLToPath(import.meta.url)), "../md"); |
Contributor
There was a problem hiding this comment.
🔍 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.
Contributor
Smoke Test Results for PR #27989
Overall: ✅ PASS PR author:
|
Contributor
Comment MemoryCode flows like streams, Tests whisper pass or fail— Green lights bloom at dawn.Note This comment is managed by comment memory. What this comment doesIt stores persistent context for this thread in the
|
Contributor
There was a problem hiding this comment.
Smoke test review for PR #27989. Changes look good — the renderManagedCommentDisclosureNote refactor is clean, and the test isolation with beforeAll/afterAll is solid.
📰 BREAKING: Report filed by Smoke Copilot · ● 2M
| function renderManagedCommentDisclosureNote() { | ||
| const promptsDir = process.env.GH_AW_PROMPTS_DIR || `${process.env.RUNNER_TEMP}/gh-aw/prompts`; | ||
| const templatePath = `${promptsDir}/comment_memory_disclosure_note.md`; | ||
| return renderTemplateFromFile(templatePath, { |
Contributor
There was a problem hiding this comment.
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.
|
|
||
| afterAll(() => { | ||
| if (originalPromptsDir === undefined) { | ||
| delete process.env.GH_AW_PROMPTS_DIR; |
Contributor
There was a problem hiding this comment.
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.
Contributor
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
Closed
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes comment_memory safe output failures by loading the disclosure-note template from the runtime prompts directory (instead of a non-existent .../md path in the copied actions runtime tree).
Changes:
- Updated
comment_memoryto render the disclosure-note template fromGH_AW_PROMPTS_DIRor${RUNNER_TEMP}/gh-aw/prompts. - Updated
comment_memorytests to setGH_AW_PROMPTS_DIRand usefileURLToPathfor cross-platform path resolution. - Minor string-construction adjustments in the agentics maintenance workflow’s
github-scriptstep.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/comment_memory.cjs | Switches disclosure-note template lookup to runtime prompts paths and removes the old __dirname/../md assumption. |
| actions/setup/js/comment_memory.test.cjs | Sets GH_AW_PROMPTS_DIR for tests and uses URL-to-path conversion for portability. |
| .github/workflows/agentics-maintenance.yml | Replaces template literals with string concatenation in an embedded github-script block. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 2
Comment on lines
55
to
58
| 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; |
There was a problem hiding this comment.
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.
| 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`; |
There was a problem hiding this comment.
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`; |
Contributor
🧪 Test Quality Sentinel ReportTest Quality Score: 90/100✅ Excellent test quality
Test Classification DetailsView all 6 test classifications
HighlightsThe test suite is well-targeted at the behavioral contracts introduced/fixed in this PR:
Minor Observation
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: §24820502988
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the
comment_memorysafe output failure seen insafe_outputswhere the disclosure template was loaded from a non-existent runtime path (.../gh-aw/md/comment_memory_disclosure_note.md).Changes
actions/setup/js/comment_memory.cjsto use the samerenderTemplateFromFiletemplate-path pattern as other setup handlers.comment_memory_disclosure_note.mdis now loaded from runtime prompts paths:GH_AW_PROMPTS_DIR(if set)${RUNNER_TEMP}/gh-aw/promptsactions/setup/js/comment_memory.test.cjsto setGH_AW_PROMPTS_DIRduring tests and usefileURLToPathfor cross-platform path handling.Validation
npm run test:js -- comment_memory.test.cjs✅make agent-finishpkg/workflowtest failures in this branch (TestCopilotDetectionDefaultModel,TestWasmGolden_CompileFixtures).🤖 Smoke CI scheduled run: https://github.com/github/gh-aw/actions/runs/24819753679
✨ PR Review Safe Output Test - Run 24820197107