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
20 changes: 16 additions & 4 deletions actions/setup/js/log_parser_bootstrap.cjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// @ts-check
/// <reference types="@actions/github-script" />

const { generatePlainTextSummary, generateCopilotCliStyleSummary, formatSafeOutputsPreview } = require("./log_parser_shared.cjs");
const { generatePlainTextSummary, generateCopilotCliStyleSummary, wrapAgentLogInSection, formatSafeOutputsPreview } = require("./log_parser_shared.cjs");
const { getErrorMessage } = require("./error_helpers.cjs");

/**
Expand Down Expand Up @@ -157,8 +157,14 @@ async function runLogParser(options) {
parserName,
});

// Wrap the agent log in a details/summary section (open by default)
const wrappedAgentLog = wrapAgentLogInSection(copilotCliStyleMarkdown, {
parserName,
open: true,
});

// Add safe outputs preview to step summary
let fullMarkdown = copilotCliStyleMarkdown;
let fullMarkdown = wrappedAgentLog;
Comment on lines +160 to +167
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

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

This wrapping is applied for the main parsed-markdown paths, but the earlier Copilot conversation.md fast-path still writes result.markdown directly to the step summary (unwrapped). That means Copilot --share output won’t get the collapsible section even though other paths do. Consider wrapping that path too (or update the PR description/behavior to be consistent).

Copilot uses AI. Check for mistakes.
if (safeOutputsContent) {
const safeOutputsMarkdown = formatSafeOutputsPreview(safeOutputsContent, { isPlainText: false });
if (safeOutputsMarkdown) {
Expand All @@ -179,8 +185,14 @@ async function runLogParser(options) {
}
}

// Write original markdown to step summary if available
let fullMarkdown = markdown;
// Wrap the original markdown in a details/summary section (open by default)
const wrappedAgentLog = wrapAgentLogInSection(markdown, {
parserName,
open: true,
});

// Write wrapped markdown to step summary if available
let fullMarkdown = wrappedAgentLog;
if (safeOutputsContent) {
const safeOutputsMarkdown = formatSafeOutputsPreview(safeOutputsContent, { isPlainText: false });
if (safeOutputsMarkdown) {
Expand Down
4 changes: 2 additions & 2 deletions actions/setup/js/log_parser_bootstrap.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe("log_parser_bootstrap.cjs", () => {
(runLogParser({ parseLog: mockParseLog, parserName: "TestParser" }),
expect(mockParseLog).toHaveBeenCalledWith("Test log content"),
expect(mockCore.info).toHaveBeenCalledWith("TestParser log parsed successfully"),
expect(mockCore.summary.addRaw).toHaveBeenCalledWith("## Parsed Log\n\nSuccess!"),
expect(mockCore.summary.addRaw).toHaveBeenCalledWith("<details open>\n<summary>🤖 TestParser CLI Session</summary>\n\n## Parsed Log\n\nSuccess!\n</details>"),
expect(mockCore.summary.write).toHaveBeenCalled(),
fs.unlinkSync(logFile),
fs.rmdirSync(tmpDir));
Expand All @@ -57,7 +57,7 @@ describe("log_parser_bootstrap.cjs", () => {
const mockParseLog = vi.fn().mockReturnValue({ markdown: "## Result\n", mcpFailures: [], maxTurnsHit: !1 });
(runLogParser({ parseLog: mockParseLog, parserName: "TestParser" }),
expect(mockCore.info).toHaveBeenCalledWith("TestParser log parsed successfully"),
expect(mockCore.summary.addRaw).toHaveBeenCalledWith("## Result\n"),
expect(mockCore.summary.addRaw).toHaveBeenCalledWith("<details open>\n<summary>🤖 TestParser CLI Session</summary>\n\n## Result\n\n</details>"),
expect(mockCore.setFailed).not.toHaveBeenCalled(),
fs.unlinkSync(logFile),
fs.rmdirSync(tmpDir));
Expand Down
23 changes: 23 additions & 0 deletions actions/setup/js/log_parser_shared.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -1414,6 +1414,28 @@ function generateCopilotCliStyleSummary(logEntries, options = {}) {
return lines.join("\n");
}

/**
* Wraps agent log markdown in a details/summary section
* @param {string} markdown - The agent log markdown content
* @param {Object} options - Configuration options
* @param {string} [options.parserName="Agent"] - Name of the parser (e.g., "Copilot", "Claude")
* @param {boolean} [options.open=true] - Whether the section should be open by default
* @returns {string} Wrapped markdown in details/summary tags
*/
function wrapAgentLogInSection(markdown, options = {}) {
const { parserName = "Agent", open = true } = options;

if (!markdown || markdown.trim().length === 0) {
return "";
}

const openAttr = open ? " open" : "";
const emoji = "🤖";
const title = `${emoji} ${parserName} CLI Session`;

return `<details${openAttr}>\n<summary>${title}</summary>\n\n${markdown}\n</details>`;
Comment on lines +1432 to +1436
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

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

parserName is interpolated directly into the <summary> HTML without escaping. If it ever contains characters like <, &, or ", it can break the rendered markup (and potentially allow HTML injection in step summaries/issue bodies). Please HTML-escape parserName (and/or the computed title) before embedding it in the <summary> tag.

Copilot uses AI. Check for mistakes.
}

/**
* Formats safe outputs preview for display in logs
* @param {string} safeOutputsContent - The raw JSONL content from safe outputs file
Expand Down Expand Up @@ -1591,6 +1613,7 @@ module.exports = {
formatToolCallAsDetails,
generatePlainTextSummary,
generateCopilotCliStyleSummary,
wrapAgentLogInSection,
formatSafeOutputsPreview,
wrapLogParser,
createEngineLogParser,
Expand Down
60 changes: 60 additions & 0 deletions actions/setup/js/log_parser_shared.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -2087,6 +2087,66 @@ describe("log_parser_shared.cjs", () => {
});
});

describe("wrapAgentLogInSection", () => {
it("should wrap markdown in details/summary with default open attribute", async () => {
const { wrapAgentLogInSection } = await import("./log_parser_shared.cjs");

const markdown = "```\nConversation:\n\nAgent: Hello\n```";
const result = wrapAgentLogInSection(markdown, { parserName: "Copilot" });

expect(result).toContain("<details open>");
expect(result).toContain("<summary>🤖 Copilot CLI Session</summary>");
expect(result).toContain(markdown);
expect(result).toContain("</details>");
});

it("should support custom parser names", async () => {
const { wrapAgentLogInSection } = await import("./log_parser_shared.cjs");

const markdown = "Test content";
const result = wrapAgentLogInSection(markdown, { parserName: "Claude" });

expect(result).toContain("🤖 Claude CLI Session");
});

it("should allow closed state when open is false", async () => {
const { wrapAgentLogInSection } = await import("./log_parser_shared.cjs");

const markdown = "Test content";
const result = wrapAgentLogInSection(markdown, { parserName: "Copilot", open: false });

expect(result).toContain("<details>");
expect(result).not.toContain("<details open>");
});

it("should default to Agent parser name when not provided", async () => {
const { wrapAgentLogInSection } = await import("./log_parser_shared.cjs");

const markdown = "Test content";
const result = wrapAgentLogInSection(markdown);

expect(result).toContain("🤖 Agent CLI Session");
});

it("should return empty string for empty or undefined markdown", async () => {
const { wrapAgentLogInSection } = await import("./log_parser_shared.cjs");

expect(wrapAgentLogInSection("")).toBe("");
expect(wrapAgentLogInSection(" ")).toBe("");
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

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

Test name says it covers “undefined markdown”, but it never calls wrapAgentLogInSection(undefined). Add an explicit undefined case or adjust the test description to match what’s being tested.

Suggested change
expect(wrapAgentLogInSection(" ")).toBe("");
expect(wrapAgentLogInSection(" ")).toBe("");
expect(wrapAgentLogInSection(undefined)).toBe("");

Copilot uses AI. Check for mistakes.
});

it("should properly escape markdown content", async () => {
const { wrapAgentLogInSection } = await import("./log_parser_shared.cjs");

const markdown = "Content with <tags> and `code`";
const result = wrapAgentLogInSection(markdown, { parserName: "Copilot" });

expect(result).toContain(markdown);
expect(result).toContain("<details open>");
expect(result).toContain("</details>");
});
Comment on lines +2138 to +2147
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

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

This test claims the wrapper “properly escape[s] markdown content”, but wrapAgentLogInSection currently injects markdown verbatim and the assertions only check that the raw string is present. Either update the test name/expectations, or implement escaping/sanitization if the intent is to render <tags> literally rather than as HTML.

Copilot uses AI. Check for mistakes.
});

describe("wrapLogParser", () => {
it("should call parser function and return result on success", async () => {
const { wrapLogParser } = await import("./log_parser_shared.cjs");
Expand Down
Loading