Conversation
…agentic-threat-detected label - Prepend the threat detection caution alert to the very top of the PR body in `create_pull_request.cjs` so it is immediately visible to reviewers - Add `agentic-threat-detected` label to the PR when threat detection produces a warning - Add `skipDetectionCaution` option to `generateFooterWithMessages` in `messages_footer.cjs` to avoid duplicating the caution alert in the footer when it has already been placed at the top of the body - Import `getDetectionCautionAlert` in `create_pull_request.cjs` - All 7045 JS tests pass, TypeScript checks clean Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d90da164-04bf-4b03-8ae9-c721d2b3e1b7 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d90da164-04bf-4b03-8ae9-c721d2b3e1b7 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot ensure extra new lines when caution inserted |
…lert Agent-Logs-Url: https://github.com/github/gh-aw/sessions/adab9e29-e34d-4824-93fb-79d65c01e8a1 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates PR body rendering so threat-detection CAUTION alerts are immediately visible at the top of the PR description, and tags PRs with a dedicated label when a threat warning is present.
Changes:
- Add an
optionsparameter togenerateFooterWithMessagesto allow omitting the detection caution from the footer. - Prepend the threat-detection CAUTION block to the top of newly created PR bodies when detection concludes “warning”.
- Add
agentic-threat-detectedto the PR label list when a threat warning is present.
Show a summary per file
| File | Description |
|---|---|
actions/setup/js/messages_footer.cjs |
Adds skipDetectionCaution option to prevent duplicating the detection CAUTION in the footer. |
actions/setup/js/create_pull_request.cjs |
Prepends detection CAUTION to the top of PR body and appends an agentic-threat-detected label on warning. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (2)
actions/setup/js/messages_footer.cjs:422
skipDetectionCautionadds new behavior (omitting the detection caution from the footer), but there doesn’t appear to be any test coverage exercising the newoptionsparameter. Please add a unit test (e.g., inactions/setup/js/messages.test.cjs) that setsGH_AW_DETECTION_CONCLUSION=warningand asserts the caution is present by default and absent when{ skipDetectionCaution: true }is passed.
const { skipDetectionCaution = false } = options || {};
// Collect guard notices to show BEFORE the attribution footer
let guardNotices = "";
actions/setup/js/create_pull_request.cjs:930
bodyLines.unshift(detectionCaution, "", "")results in two blank lines between the caution block and the rest of the body (because the caution string is followed by three consecutive\nwhen joined). If the intent is a single blank separator line (as described), consider unshifting only one empty string (or update the comment/PR description to reflect the extra spacing).
// unshift(caution, "", "") places the caution alert at index 0 and two blank
// separator lines so the main body content follows after a full empty line.
bodyLines.unshift(detectionCaution, "", "");
}
- Files reviewed: 2/2 changed files
- Comments generated: 3
| * @param {string|null|undefined} [historyUrl] - GitHub search URL for items created by this workflow | ||
| * @returns {string} Complete footer text | ||
| */ |
There was a problem hiding this comment.
There are now two JSDoc blocks describing generateFooterWithMessages back-to-back (one ending at @returns here, then another starting below). The first block is effectively orphaned/duplicative after inserting the typedef and can confuse tooling/ts-check—please remove it or merge into a single JSDoc immediately preceding the function.
This issue also appears on line 418 of the same file.
| // Prepend threat detection caution alert at the very top of the PR body so it is | ||
| // immediately visible to reviewers. The caution is omitted from the footer to | ||
| // avoid duplication (skipDetectionCaution is passed to generateFooterWithMessages). | ||
| const detectionCaution = getDetectionCautionAlert(workflowName, runUrl); | ||
| if (detectionCaution) { |
There was a problem hiding this comment.
The new behavior of prepending the threat-detection caution to the top of the PR body isn’t covered by tests in create_pull_request.test.cjs (no scenarios set GH_AW_DETECTION_CONCLUSION=warning and assert body ordering). Please add a test that verifies the caution appears at the very beginning of the generated PR body when detection is warning, and that it is absent when detection is not warning.
This issue also appears on line 927 of the same file.
| // Add agentic-threat-detected label when threat detection produced a warning | ||
| if (detectionCaution && !labels.includes("agentic-threat-detected")) { | ||
| labels.push("agentic-threat-detected"); | ||
| } |
There was a problem hiding this comment.
Adding the agentic-threat-detected label based on threat-detection warnings is new behavior, but there’s no test coverage in create_pull_request.test.cjs asserting the label set includes it when GH_AW_DETECTION_CONCLUSION=warning (and does not include it otherwise). Please add/extend a test to validate the labels passed to issues.addLabels in both cases.
|
@copilot Review all tests |
…essages_footer Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f36dc55f-6578-4446-b624-09990dcd9eca Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 2000285. Added 5 new tests in a
Also fixed the duplicate JSDoc block in |
Summary
Changes the rendering order of the pull request body so that the threat detection warning/caution alert appears at the top of the body (immediately visible to reviewers) rather than just before the footer. Also adds an
agentic-threat-detectedlabel to the PR when threat detection produces a warning.Changes
actions/setup/js/messages_footer.cjs@param {GenerateFooterOptions} [options]and@returns {string}) immediately precedinggenerateFooterWithMessagesGenerateFooterOptionstypedef with askipDetectionCautionpropertyoptionsparameter togenerateFooterWithMessagesskipDetectionCaution: trueis passed, the caution alert is omitted from the footer (avoids duplication when the caller has already placed it at the top of the body)actions/setup/js/create_pull_request.cjsgetDetectionCautionAlertfrommessages_footer.cjsworkflowName/runUrlare resolved, gets the detection caution alertbodyLines(before any agent-generated content) with two blank separator lines for proper visual spacingagentic-threat-detectedlabel to the PR's label list when a threat warning is present{ skipDetectionCaution: true }togenerateFooterWithMessagesto prevent the alert from being duplicated in the footeractions/setup/js/create_pull_request.test.cjscreate_pull_request - threat detection cautiondescribe block with 5 tests:GH_AW_DETECTION_CONCLUSION=warningagentic-threat-detectedlabel is included inaddLabelscall when warningagentic-threat-detectedlabel is absent when conclusion is not warningBefore / After
Before (threat detected):
After (threat detected):
Labels on PR now include:
agentic-threat-detectedTesting
make fmtpasses