Add comment_memory safe output support with editable file workflow integration#27479
Add comment_memory safe output support with editable file workflow integration#27479
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/7a86331a-2d62-45b3-b52d-8bd4b8ec8b01 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/7a86331a-2d62-45b3-b52d-8bd4b8ec8b01 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
| if (!/^[a-zA-Z0-9_-]+$/.test(memoryID)) { | ||
| throw new Error("memory_id must contain only alphanumeric characters, hyphens, and underscores"); | ||
| } | ||
| let body = `<comment-memory id="${memoryID}">\n${sanitizeContent(rawBody)}\n</comment-memory>`; |
| } | ||
|
|
||
| async function findManagedComment(github, owner, repo, itemNumber, memoryID) { | ||
| const marker = `<comment-memory id="${memoryID}">`; |
There was a problem hiding this comment.
@copilot use gh-aw-comment-memory (refactor id in constant)
There was a problem hiding this comment.
Implemented in 59628d0. Refactored marker usage to shared constants (COMMENT_MEMORY_TAG) and reused it for both body generation and lookup matching.
| const marker = `<comment-memory id="${memoryID}">`; | ||
| let page = 1; | ||
| const perPage = 100; | ||
| while (true) { |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/27e277b0-1803-4347-afdc-28261f19684b Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
This reverts commit c54ab39. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/27e277b0-1803-4347-afdc-28261f19684b Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Implemented in 59628d0. Added extensive 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/8f6cfa44-79b4-46b5-96ab-081eb8dcc7b7 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
This reverts commit 766afef. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8f6cfa44-79b4-46b5-96ab-081eb8dcc7b7 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8f6cfa44-79b4-46b5-96ab-081eb8dcc7b7 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8f6cfa44-79b4-46b5-96ab-081eb8dcc7b7 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Implemented in a2c0451 (core feature commits: 11a9c42, cda8351). Added a pre-agent JS setup step that materializes 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:
|
| const COMMENT_MEMORY_TAG = "gh-aw-comment-memory"; | ||
| const COMMENT_MEMORY_MAX_SCAN_PAGES = 50; | ||
|
|
||
| function logInfo(message) { |
| } | ||
| } | ||
|
|
||
| function logWarning(message) { |
There was a problem hiding this comment.
Smoke test agent confirming this thread — the bounded pagination approach with COMMENT_MEMORY_MAX_SCAN_PAGES is a solid safeguard! ✅
📰 BREAKING: Report filed by Smoke Copilot · ● 998.2K
| const path = require("path"); | ||
| const { getErrorMessage } = require("./error_helpers.cjs"); | ||
|
|
||
| const COMMENT_MEMORY_TAG = "gh-aw-comment-memory"; |
There was a problem hiding this comment.
@copilot refactor parsing and constant in helper file
| } | ||
| }, | ||
| { | ||
| "name": "comment_memory", |
There was a problem hiding this comment.
@copilot this tool is not needed, the agent should just edit the comment memory files directly.
There was a problem hiding this comment.
Implemented in 18bdc3a. Removed comment_memory from safe-output tool definitions/prompts; the agent now edits /tmp/gh-aw/comment-memory/*.md directly and the handler manager auto-syncs those files into managed comment memory. UI screenshot: N/A (no UI changes).
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b01eb4b0-28aa-4b71-81ea-83ef827f360e Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
This reverts commit 91d752b. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
|
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation... |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
🚀 Smoke OpenCode MISSION COMPLETE! OpenCode delivered. 🔥 |
|
|
Agent Container Tool Check
Result: 12/12 tools available ✅ Overall Status: PASS
|
|
Smoke Test 24735013467 | PR #27479 |
Overall: ✅ PASS (14/14)
|
There was a problem hiding this comment.
Reviewed the new comment_memory.cjs module. The implementation is well-structured with proper validation and bounded pagination. Minor suggestions: use debug level for rejected memory IDs, and add inline comments explaining the provenance marker requirement.
📰 BREAKING: Report filed by Smoke Copilot · ● 998.2K
| // Require provenance marker to avoid accidentally updating user-authored comments | ||
| // that happen to contain a matching comment-memory tag. | ||
| const MANAGED_COMMENT_PROVENANCE_MARKER = "<!-- gh-aw-agentic-workflow:"; | ||
|
|
There was a problem hiding this comment.
The MANAGED_COMMENT_PROVENANCE_MARKER guard is a good defensive check — it ensures only workflow-managed comments are updated. Consider adding a comment explaining why this marker is required for future readers.
| core.info(`comment_memory: rejected invalid memory_id '${normalized}'`); | ||
| return null; | ||
| } | ||
| return normalized; |
There was a problem hiding this comment.
The sanitizeMemoryID function correctly validates memory_id format, but consider logging the rejected value at debug level rather than info to avoid noise in non-debug runs.
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
Smoke Test Results — Run §24735013438Core Tests #1–12: ✅✅✅✅✅✅✅✅✅❌✅✅ Overall: PARTIAL (17✅ · 1❌ · 2
|
There was a problem hiding this comment.
💥 Automated smoke test review — Claude engine nominal! All systems tested and functional. Run 24735013438.
💥 [THE END] — Illustrated by Smoke Claude · ● 251.7K
| <safe-output-tools> | ||
| Tools: add_comment(max:10), create_issue(max:5), create_discussion(max:2), missing_tool, missing_data, noop | ||
| GH_AW_PROMPT_e8efe57977b9f20b_EOF | ||
| cat "${RUNNER_TEMP}/gh-aw/prompts/safe_outputs_comment_memory.md" |
There was a problem hiding this comment.
🤖 Smoke test review comment — The comment memory prompt injection is correctly placed here, sandwiched between the EOF heredoc boundaries. This ensures the agent receives comment-memory context as part of the safe-output tools prompt. ✅
| <safe-output-tools> | ||
| Tools: create_discussion, missing_tool, missing_data, noop | ||
| GH_AW_PROMPT_2068395ca0f8adef_EOF | ||
| cat "${RUNNER_TEMP}/gh-aw/prompts/safe_outputs_comment_memory.md" |
There was a problem hiding this comment.
🤖 Smoke test review comment — Good pattern: the comment-memory prompt file is catted after the safe-output tools list but before the closing EOF, so the agent prompt gets the full context. This is consistent across all generated lock files. ✅
|
…ut-type-comment-memory # Conflicts: # .github/workflows/agentic-observability-kit.lock.yml # .github/workflows/daily-hippo-learn.lock.yml # .github/workflows/portfolio-analyst.lock.yml Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/0ebb1620-0947-424a-abed-2a43a5aea483 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Implemented in 4f31106 and 5662ef4. Merged 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:
|
|
Hey Checklist summary:
Verdict: 🟢 Aligned —
|
Add new glossary entries for terms introduced in recent changes: - GitHub Access Mode (tools.github.mode / gh-proxy) from PR #27707 - Comment Memory (tools.comment-memory) from PR #27479 - Supersede Older Reviews (supersede-older-reviews) from PR #27662 - Update AWF entry to mention sandbox.agent.version from PR #27626 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
comment_memorysafe output type that persists memory in a managed issue/PR comment/tmp/gh-aw/comment-memory/(extracting user content from managed XML blocks)comment_memoryfrom the agent-exposed safe-output tool list and prompt tools list; agent now edits memory files directlycomment_memorytool calldocs/src/content/docs/reference/safe-outputs-specification.md) to add thecomment_memorytype and formally document the end-to-end data flow from GitHub comment → local files → agent → artifacts/threat-detection context → managed comment upsertmemory_idlength validation with runtime validation ...comment-memoryexplicitly in smoke workflows so smoke runs cover the feature directly:.github/workflows/smoke-codex.md.github/workflows/smoke-copilot.mdsmoke-codex.lock.yml,smoke-copilot.lock.yml)smoke-codexandsmoke-copilotappend an original 3-line haiku to/tmp/gh-aw/comment-memory/*.md(without removing existing content), explicitly exercising file-based comment-memory editing behavior during smoke runsmainand recompile workflow lock filescomment_memorycomments with an H3 heading (### Comment Memory) so memory comments are clearly labeled, with JS tests updated accordingly✨ PR Review Safe Output Test - Run 24735013438
> 💥 THE END] — Illustrated by [Smoke Claude · ● 251.7K · ◷