Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions actions/setup/js/comment_memory.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs");
const { getTrackerID } = require("./get_tracker_id.cjs");
const { generateHistoryUrl } = require("./generate_history_link.cjs");
const { enforceCommentLimits } = require("./comment_limit_helpers.cjs");
const { COMMENT_MEMORY_TAG, COMMENT_MEMORY_MAX_SCAN_PAGES } = require("./comment_memory_helpers.cjs");
const { COMMENT_MEMORY_TAG, COMMENT_MEMORY_MAX_SCAN_PAGES, COMMENT_MEMORY_CODE_FENCE } = require("./comment_memory_helpers.cjs");
// 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:";
Expand Down Expand Up @@ -45,7 +45,7 @@ function buildManagedMemoryBody(rawBody, memoryID, options) {
const openingTag = `<${COMMENT_MEMORY_TAG} id="${memoryID}">`;
const closingTag = `</${COMMENT_MEMORY_TAG}>`;
core.info(`comment_memory: building managed body for memory_id='${memoryID}'`);
let body = `${MANAGED_COMMENT_HEADER}\n\n${openingTag}\n${sanitizeContent(rawBody)}\n${closingTag}`;
let body = `${MANAGED_COMMENT_HEADER}\n\n${openingTag}\n${COMMENT_MEMORY_CODE_FENCE}\n${sanitizeContent(rawBody)}\n${COMMENT_MEMORY_CODE_FENCE}\n${closingTag}`;

Comment on lines 45 to 49
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.

Wrapping memory content with a fixed six-backtick fence will break Markdown rendering if the memory itself contains a line equal to the same six-backtick sequence (it will prematurely close the outer fence and corrupt the rest of the comment). Consider selecting a fence length that is longer than any backtick-run present in the sanitized memory content (minimum 6), and update the stripping/parser logic to accept that variable fence length so round-tripping remains reliable.

Copilot uses AI. Check for mistakes.
const tracker = getTrackerID("markdown");
if (tracker) {
Expand Down
2 changes: 2 additions & 0 deletions actions/setup/js/comment_memory.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ describe("comment_memory", () => {
});
expect(body).toContain("### Comment Memory");
expect(body).toContain('<gh-aw-comment-memory id="default">');
expect(body).toContain("``````");
expect(body).toContain("Hello world");
expect(body).toContain("</gh-aw-comment-memory>");
});
Expand All @@ -63,6 +64,7 @@ describe("comment_memory", () => {
});
expect(body).toContain("### Comment Memory");
expect(body).toContain('<gh-aw-comment-memory id="session">');
expect(body).toContain("``````");
expect(body).toContain("Persist me");
expect(body).toContain("> [!NOTE]");
expect(body).toContain("> <summary>What this comment does</summary>");
Expand Down
21 changes: 20 additions & 1 deletion actions/setup/js/comment_memory_helpers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,23 @@ const COMMENT_MEMORY_MAX_SCAN_PAGES = 50;
const COMMENT_MEMORY_MAX_SCAN_EMPTY_PAGES = 5;
const COMMENT_MEMORY_PROMPT_START_MARKER = "<!-- gh-aw-comment-memory-prompt:start -->";
const COMMENT_MEMORY_PROMPT_END_MARKER = "<!-- gh-aw-comment-memory-prompt:end -->";
const COMMENT_MEMORY_CODE_FENCE = "``````";
const ESCAPED_COMMENT_MEMORY_CODE_FENCE = COMMENT_MEMORY_CODE_FENCE.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");

function stripCommentMemoryCodeFence(content) {
const trimmed = typeof content === "string" ? content.trim() : "";
if (trimmed.length === 0) {
return "";
}
if (!trimmed.startsWith(COMMENT_MEMORY_CODE_FENCE)) {
return trimmed;
}
const match = trimmed.match(new RegExp(`^${ESCAPED_COMMENT_MEMORY_CODE_FENCE}[^\\n]*\\n([\\s\\S]*)\\n${ESCAPED_COMMENT_MEMORY_CODE_FENCE}$`));
if (!match) {
Comment on lines +17 to +26
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.

stripCommentMemoryCodeFence() constructs a new RegExp on every call. This helper runs in a loop while scanning many comments/pages, so repeatedly compiling the same regex is unnecessary overhead. Consider precompiling the fence-stripping regex once at module scope and reusing it inside the function.

Copilot uses AI. Check for mistakes.
return trimmed;
}
return match[1].trim();
}

function isSafeMemoryId(memoryId) {
if (typeof memoryId !== "string" || memoryId.length === 0 || memoryId.length > MAX_MEMORY_ID_LENGTH) {
Expand Down Expand Up @@ -57,7 +74,7 @@ function extractCommentMemoryEntries(commentBody, warn = () => {}) {
if (isSafeMemoryId(memoryId)) {
entries.push({
memoryId,
content: (commentBody.slice(contentStart, closeStart) || "").trim(),
content: stripCommentMemoryCodeFence(commentBody.slice(contentStart, closeStart)),
});
} else {
warn(`skipping unsafe memory_id '${memoryId}'`);
Expand Down Expand Up @@ -99,7 +116,9 @@ module.exports = {
COMMENT_MEMORY_MAX_SCAN_EMPTY_PAGES,
COMMENT_MEMORY_PROMPT_START_MARKER,
COMMENT_MEMORY_PROMPT_END_MARKER,
COMMENT_MEMORY_CODE_FENCE,
isSafeMemoryId,
stripCommentMemoryCodeFence,
extractCommentMemoryEntries,
listCommentMemoryFiles,
resolveCommentMemoryConfig,
Expand Down
37 changes: 36 additions & 1 deletion actions/setup/js/comment_memory_helpers.test.cjs
Original file line number Diff line number Diff line change
@@ -1,12 +1,47 @@
import { describe, it, expect, vi } from "vitest";
import { extractCommentMemoryEntries, isSafeMemoryId } from "./comment_memory_helpers.cjs";
import { extractCommentMemoryEntries, isSafeMemoryId, stripCommentMemoryCodeFence } from "./comment_memory_helpers.cjs";

describe("comment_memory_helpers", () => {
it("extracts managed memory entries", () => {
const entries = extractCommentMemoryEntries('<gh-aw-comment-memory id="default">\n``````\nhello\n``````\n</gh-aw-comment-memory>');
expect(entries).toEqual([{ memoryId: "default", content: "hello" }]);
});

it("supports legacy memory entries without code fence markers", () => {
const entries = extractCommentMemoryEntries('<gh-aw-comment-memory id="default">\nhello\n</gh-aw-comment-memory>');
expect(entries).toEqual([{ memoryId: "default", content: "hello" }]);
});

it("keeps fenced text unchanged when trailing content exists after closing fence", () => {
const content = "``````\nhello\n``````\ntrailing";
expect(stripCommentMemoryCodeFence(content)).toBe(content);
});

it("keeps fenced text unchanged when closing fence is missing", () => {
const content = "``````\nhello";
expect(stripCommentMemoryCodeFence(content)).toBe(content);
});

it("keeps malformed fenced text unchanged", () => {
const content = "``````hello\n``````";
expect(stripCommentMemoryCodeFence(content)).toBe(content);
});

it("strips valid fenced text with extra newlines before content", () => {
const content = "``````\n\nhello\n``````";
expect(stripCommentMemoryCodeFence(content)).toBe("hello");
});

it("strips valid fenced text when content contains six-backtick lines", () => {
const content = "``````\nline 1\n``````\nline 2\n``````";
expect(stripCommentMemoryCodeFence(content)).toBe("line 1\n``````\nline 2");
});

it("keeps fenced text unchanged when closing fence has no leading newline", () => {
const content = "``````\nhello``````";
expect(stripCommentMemoryCodeFence(content)).toBe(content);
});

it("rejects unsafe memory IDs", () => {
const warning = vi.fn();
const entries = extractCommentMemoryEntries('<gh-aw-comment-memory id="../bad">\nhello\n</gh-aw-comment-memory>', warning);
Expand Down
10 changes: 5 additions & 5 deletions actions/setup/js/setup_comment_memory_files.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe("setup_comment_memory_files", () => {

it("extracts memory entries from managed comment body", async () => {
const module = await import("./setup_comment_memory_files.cjs");
const entries = module.extractCommentMemoryEntries('<gh-aw-comment-memory id="default">\nhello\n</gh-aw-comment-memory>');
const entries = module.extractCommentMemoryEntries('<gh-aw-comment-memory id="default">\n``````\nhello\n``````\n</gh-aw-comment-memory>');
expect(entries).toEqual([{ memoryId: "default", content: "hello" }]);
});

Expand All @@ -47,7 +47,7 @@ describe("setup_comment_memory_files", () => {
listComments: vi.fn().mockResolvedValue({
data: [
{
body: '<gh-aw-comment-memory id="default">\nSaved memory\n</gh-aw-comment-memory>\nfooter',
body: '<gh-aw-comment-memory id="default">\n``````\nSaved memory\n``````\n</gh-aw-comment-memory>\nfooter',
},
],
}),
Expand Down Expand Up @@ -76,7 +76,7 @@ describe("setup_comment_memory_files", () => {
});
}
if (page === 6) {
return Promise.resolve({ data: [{ body: '<gh-aw-comment-memory id="default">\nLate memory\n</gh-aw-comment-memory>' }] });
return Promise.resolve({ data: [{ body: '<gh-aw-comment-memory id="default">\n``````\nLate memory\n``````\n</gh-aw-comment-memory>' }] });
}
return Promise.resolve({ data: [] });
});
Expand Down Expand Up @@ -156,7 +156,7 @@ describe("setup_comment_memory_files", () => {
})
);
const listComments = vi.fn().mockResolvedValue({
data: [{ body: '<gh-aw-comment-memory id="default">\nCross repo memory\n</gh-aw-comment-memory>' }],
data: [{ body: '<gh-aw-comment-memory id="default">\n``````\nCross repo memory\n``````\n</gh-aw-comment-memory>' }],
});
global.github = {
rest: {
Expand Down Expand Up @@ -192,7 +192,7 @@ describe("setup_comment_memory_files", () => {
})
);
const listComments = vi.fn().mockResolvedValue({
data: [{ body: '<gh-aw-comment-memory id="default">\nSame repo memory\n</gh-aw-comment-memory>' }],
data: [{ body: '<gh-aw-comment-memory id="default">\n``````\nSame repo memory\n``````\n</gh-aw-comment-memory>' }],
});
global.github = {
rest: {
Expand Down
10 changes: 5 additions & 5 deletions pkg/cli/spec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1117,11 +1117,11 @@ func TestSpec_PublicAPI_ValidateWorkflowIntent(t *testing.T) {
// Spec: "Sets a field in frontmatter YAML"
func TestSpec_PublicAPI_UpdateFieldInFrontmatter(t *testing.T) {
tests := []struct {
name string
content string
fieldName string
fieldValue string
wantErr bool
name string
content string
fieldName string
fieldValue string
wantErr bool
checkContains string
}{
{
Expand Down
Loading