Fix markdown separation before injected security scanning caution in add_comment output

[Copilot]

Injected security-scanning cautions could render as malformed markdown when appended directly after user content in safeoutputs/add_comment. The generated caution/footer block now starts on a guaranteed new paragraph boundary.

• Root cause

  • add_comment.cjs appended generateFooterWithMessages(...).trimEnd() directly to processedBody with no separator.
  • When detection injected a > [!CAUTION] block, blockquote/admonition syntax could attach to preceding text.

• Change

  • Added explicit paragraph separation before generated footer/guard notices in add_comment.cjs:
    • processedBody += "\n\n" + generateFooterWithMessages(...).trimEnd();

• Regression coverage

  • Added/strengthened add_comment.test.cjs assertions to verify:
    • a blank line exists before > [!CAUTION],
    • caution content is present,
    • caution block appears before attribution footer.

// add_comment.cjs
if (includeFooter) {
  processedBody += "\n\n" + generateFooterWithMessages(...).trimEnd();
}

[Copilot]

Pull request overview

This PR fixes malformed Markdown rendering when a security-scanning caution block (> [!CAUTION]) is injected into the generated footer for add_comment output by ensuring the footer starts on a new paragraph boundary.

Changes:

• Add explicit paragraph separation before appending generateFooterWithMessages(...) in add_comment.cjs.
• Add a regression test to assert a blank line exists before the injected caution block and that the caution appears before the attribution footer.

Show a summary per file

File	Description
actions/setup/js/add_comment.cjs	Prepends a paragraph break before appending the generated footer to avoid admonition/blockquote Markdown attaching to user content.
actions/setup/js/add_comment.test.cjs	Adds assertions to verify correct spacing and ordering for the injected security-scanning caution block in comment bodies.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Copilot]

generateFooterWithMessages(...) already starts with "\n\n" when there are no guard notices (i.e., the common non-warning case). Prepending another "\n\n" here changes the rendered output to include two blank paragraphs ("\n\n\n\n") before the normal attribution footer, which is a behavior change unrelated to the caution/admonition fix. Consider building the footer string first and only inserting an extra separator when the footer/guard content does not already begin on a new-paragraph boundary (e.g., when detection caution is present and the footer starts with "> [!CAUTION]").

Suggested change

	processedBody += "\n\n" + generateFooterWithMessages(workflowName, runUrl, workflowSource, workflowSourceURL, triggeringIssueNumber, triggeringPRNumber, triggeringDiscussionNumber, historyUrl).trimEnd();
	const footerWithMessages = generateFooterWithMessages(
	workflowName,
	runUrl,
	workflowSource,
	workflowSourceURL,
	triggeringIssueNumber,
	triggeringPRNumber,
	triggeringDiscussionNumber,
	historyUrl,
	).trimEnd();
	processedBody += footerWithMessages.startsWith("\n\n") || footerWithMessages.startsWith("\r\n\r\n")
	? footerWithMessages
	: "\n\n" + footerWithMessages;

[github-actions]

🧪 Test Quality Sentinel Report

Test Quality Score: 80/100

✅ Excellent test quality

Metric	Value
New/modified tests analyzed	1
✅ Design tests (behavioral contracts)	1 (100%)
⚠️ Implementation tests (low value)	0 (0%)
Tests with error/edge cases	1 (100%)
Duplicate test clusters	0
Test inflation detected	⚠️ Yes — 38 test lines added vs 1 production line added (38:1 ratio)
🚨 Coding-guideline violations	None

---

Test Classification Details

View test details

Test	File	Classification	Issues Detected
should add a blank line before injected security scanning caution footer	actions/setup/js/add_comment.test.cjs:1885	✅ Design	Test inflation flag (38:1 ratio — expected for a one-line bug fix)

---

Analysis: New Test

✅ should add a blank line before injected security scanning caution footer (add_comment.test.cjs:1885)

Classification: Design test — Behavioral contract

What design invariant does this test enforce?
This test enforces the markdown rendering contract: when a security-scanning warning is present, the CAUTION block must be preceded by a blank line (\n\n) so that GitHub's Markdown renderer correctly formats it as a block-quote. The assertion expect(capturedBody).toContain("Comment body for warning case\n\n> [!CAUTION]") directly verifies the output format.

What would break if deleted?
The exact regression this PR fixes (missing blank line before the CAUTION footer) would be undetectable. A future change to processedBody += concatenation logic could silently re-introduce the bug.

Assessment: Exercises a non-default path (requires GH_AW_DETECTION_CONCLUSION=warning and GH_AW_DETECTION_REASON=threat_detected), asserts on 4 distinct properties of the observable output (including a regex for structural ordering), and directly covers the one-line production fix. Well-designed.

Minor note on test inflation: The test adds 38 lines against 1 production line added (38:1). This technically exceeds the 2:1 threshold, but is expected and justified — the one-line fix ("\n\n" +) requires substantial test scaffolding to execute the full handler pipeline. The inflation metric is deducted mechanically but does not represent a real quality concern here.

---

Language Support

Tests analyzed:

• 🟨 JavaScript (*.test.cjs): 1 test (vitest)
• 🐹 Go (*_test.go): 0 tests changed

---

Verdict

✅ Check passed. 0% of new tests are implementation tests (threshold: 30%). No coding-guideline violations detected.

---

📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

• Assert on observable outputs, return values, or state changes
• Cover error paths and boundary conditions
• Would catch a behavioral regression if deleted
• Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

• Assert on internal function calls (mocking internals)
• Only test the happy path with typical inputs
• Break during legitimate refactoring even when behavior is correct
• Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

References: §24673956587

🧪 Test quality analysis by Test Quality Sentinel · ● 931.5K · ◷

[github-actions]

✅ Test Quality Sentinel: 80/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). The single new test directly verifies the behavioral contract of the one-line markdown fix and exercises a non-default security-warning code path.