Skip to content

Move detection caution callout to top of agent failure issue/comment bodies#27896

Merged
pelikhan merged 6 commits intomainfrom
copilot/update-agent-failure-comment-position
Apr 22, 2026
Merged

Move detection caution callout to top of agent failure issue/comment bodies#27896
pelikhan merged 6 commits intomainfrom
copilot/update-agent-failure-comment-position

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 22, 2026

  • Identify where agent failure issue body and detection caution callout are generated
  • Run baseline validation (make agent-finish) before code changes
  • Update issue/comment body generation so detection caution callout appears at the top as the first element
  • Update tests to verify caution callout placement behavior
  • Run focused tests (npm run test:js -- messages.test.cjs handle_agent_failure.test.cjs)
  • Revert unrelated workflow lock file permission diffs flagged by review
  • Run full validation (make agent-finish) and record existing unrelated lint failures in pkg/agentdrain/spec_test.go
  • Create/update pull request with the change

Copilot AI and others added 3 commits April 22, 2026 21:06
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Moves the “detection caution” callout out of the agent-failure footer generators and into handle_agent_failure.cjs so it appears at the top of newly created agent-failure issues and comments.

Changes:

  • Remove detection caution prepending from getFooterAgentFailureIssueMessage / getFooterAgentFailureCommentMessage.
  • Prepend the detection caution block to the top of agent-failure issue bodies and comment bodies in handle_agent_failure.cjs.
  • Update footer-focused tests to assert the agent-failure footers no longer contain the caution block.
Show a summary per file
File Description
actions/setup/js/messages_footer.cjs Stops injecting the detection caution into agent-failure footer message functions.
actions/setup/js/messages.test.cjs Updates expectations to ensure agent-failure footer functions do not include the caution callout.
actions/setup/js/handle_agent_failure.cjs Prepends the caution callout to the top of agent-failure issue/comment bodies before adding the footer.

Copilot's findings

Tip

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

Comments suppressed due to low confidence (1)

actions/setup/js/handle_agent_failure.cjs:1551

  • The new detection caution callout placement for newly created agent-failure issues isn’t exercised by tests in this PR. Consider adding coverage in handle_agent_failure.test.cjs to verify the generated issue body starts with the caution block when GH_AW_DETECTION_CONCLUSION=warning, and that the caution block is not duplicated elsewhere (e.g., in the footer).
        // Prepend detection caution alert (when present) so it appears first in the issue body
        const detectionCaution = getDetectionCautionAlert(workflowName, runUrl);

        // Combine issue body with footer
        const bodyLines = detectionCaution ? [detectionCaution, "", issueBodyContent, "", footerWithExpires] : [issueBodyContent, "", footerWithExpires];
        const issueBody = bodyLines.join("\n");
  • Files reviewed: 3/3 changed files
  • Comments generated: 1

Comment on lines +1377 to 1383
// Prepend detection caution alert (when present) so it appears first in the comment body
const detectionCaution = getDetectionCautionAlert(workflowName, runUrl);
const fullCommentBodyRaw = detectionCaution ? `${detectionCaution}\n\n${commentBody}\n\n${footer}` : `${commentBody}\n\n${footer}`;

// Combine comment body with footer
const fullCommentBody = sanitizeContent(commentBody + "\n\n" + footer, { maxLength: 65000 });
const fullCommentBody = sanitizeContent(fullCommentBodyRaw, { maxLength: 65000 });

Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

The new detection caution callout placement for agent-failure comments isn’t covered by tests in this PR. messages.test.cjs now asserts the footer does not contain the callout, but there’s no corresponding assertion that handle_agent_failure actually prepends the callout to the comment body (and that it appears before the rendered template text). Add a focused test in handle_agent_failure.test.cjs that sets GH_AW_DETECTION_CONCLUSION=warning and verifies the created comment body begins with the caution block and only includes it once.

This issue also appears on line 1546 of the same file.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 70/100

⚠️ Acceptable — with suggestions

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

Test Classification Details

View all 2 modified tests
Test File Classification Issues Detected
should not include caution alert in getFooterAgentFailureIssueMessage when detection is warning actions/setup/js/messages.test.cjs:~921 ✅ Design No error/edge cases; missing positive assertion for new top-of-body behavior
should not include caution alert in getFooterAgentFailureCommentMessage when detection is warning actions/setup/js/messages.test.cjs:~934 ✅ Design No error/edge cases; missing positive assertion for new top-of-body behavior

Flagged Tests — Suggestions for Improvement

⚠️ Both modified tests — missing positive coverage for new behavior

Classification: Design test (behavioral contract) ✅
Issue: The tests correctly verify that getFooterAgentFailureIssueMessage and getFooterAgentFailureCommentMessage no longer include the caution alert. However, the positive side of the behavioral change — that the caution alert now appears at the top of the issue/comment body (assembled in handle_agent_failure.cjs) — has no test coverage at all.

What design invariant is enforced? That the footer functions no longer include the caution block. ✅
Gap: The invariant "caution appears first in the body when detection is warning" is untested. If someone accidentally reverted the handle_agent_failure.cjs change, no test would catch it.

Suggested improvement: Add an integration-level test (or extend an existing one for handle_agent_failure.cjs) that:

  1. Sets GH_AW_DETECTION_CONCLUSION=warning
  2. Calls the body assembly logic
  3. Asserts that the caution block appears before the issue/comment body content (e.g., result.indexOf("[!CAUTION]") < result.indexOf(issueBodyContent))

i️ Edge case coverage score is 0% (zero error paths tested)

Both tests set GH_AW_DETECTION_CONCLUSION=warning (a defined conditional branch) but neither uses .toThrow(), .rejects(), or tests null/undefined detection values. The existing getDetectionCautionAlert tests (lines ~832+) already provide that coverage, so this is a minor gap rather than a critical one.


Language Support

Tests analyzed:

  • 🟨 JavaScript (*.test.cjs): 2 modified tests (vitest)
  • 🐹 Go (*_test.go): No changes

Verdict

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

The score of 70/100 reflects full behavioral-contract credit (40/40) and no duplication/inflation penalties (30/30), but zero edge-case coverage points (0/30) since neither modified test exercises an error path. The main suggestion is to add a positive assertion for the new "caution at the top" behavior, which is currently unverified.


📖 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

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

  • Assert on internal function calls (mocking internals)
  • Only test the happy path
  • Break during legitimate refactoring even when behavior is correct

References: §24806809476

🧪 Test quality analysis by Test Quality Sentinel · ● 550.7K ·

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

✅ Test Quality Sentinel: 70/100. Test quality is acceptable — 0% of modified tests are implementation tests (threshold: 30%). Both tests correctly verify the behavioral change (footer functions no longer include the caution alert). Suggestion: add a positive assertion verifying the caution now appears at the top of the body in handle_agent_failure.cjs.

@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot review all comments

Copilot AI and others added 3 commits April 22, 2026 23:08
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 22, 2026

@copilot review all comments

Addressed. I added focused coverage in handle_agent_failure.test.cjs to verify the detection caution callout is prepended to both existing-issue comments and new issue bodies, and that it appears exactly once (b0cdfd2). I also reverted unrelated workflow lock-file permission diffs from this PR (bea4c5d).

@pelikhan pelikhan merged commit dd13871 into main Apr 22, 2026
@pelikhan pelikhan deleted the copilot/update-agent-failure-comment-position branch April 22, 2026 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants