Skip to content

improve: threat detection engine error template with progressive disclosure - #49497

Merged
pelikhan merged 11 commits into
mainfrom
copilot/improve-error-template
Aug 1, 2026
Merged

improve: threat detection engine error template with progressive disclosure#49497
pelikhan merged 11 commits into
mainfrom
copilot/improve-error-template

Conversation

Copilot AI commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

The threat detection engine error warning used a lowercase, non-bold title ("threat detection engine error") and a generic <summary>Details</summary> disclosure label, making it hard to scan and understand at a glance.

Changes

  • Inline warning title (generate_footer.cjs, messages_run_status.cjs): threat detection engine error**Threat Detection Engine Failure**; description tightened; <summary>Details</summary><summary>What happened</summary>
  • Detection runs issue comment (detection_runs_comment.md): adds an always-visible summary sentence up front; moves the raw run details table into a collapsible <details>Run details</details> block
  • Tests (generate_footer.test.cjs, messages.test.cjs): assertions updated to match new title

Before:

> [!WARNING]
> threat detection engine error
> The threat detection engine encountered an error and could not complete analysis...
> <details><summary>Details</summary>...

After:

> [!WARNING]
> **Threat Detection Engine Failure** — The analysis engine could not complete. This is a tooling failure, not a security finding.
> <details><summary>What happened</summary>...

Caution

agentic threat detected
Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.

Details

Potential security threats were detected in the agent output.

Review the workflow run logs for details.

run: https://github.com/github/gh-aw/actions/runs/30692749654

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 19.5 AIC · ⌖ 5.58 AIC · ⊞ 8.1K ·
Comment /souschef to run again

Copilot AI and others added 2 commits August 1, 2026 06:38
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…losure

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Comment thread actions/setup/js/generate_footer.cjs Outdated
const reasonText = getDetectionReasonText(detectionReason);
if (isToolingFailureReason(detectionReason)) {
return `> [!WARNING]\n> threat detection engine error\n> The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.\n> ${getThreatDetectedMarker(detectionReason)}\n>\n> <details>\n> <summary>Details</summary>\n>\n> ${reasonText}\n>\n> Review the [workflow run logs](${runUrl}) for details.\n> </details>`;
return `> [!WARNING]\n> **Threat Detection Engine Failure** — The analysis engine could not complete. This is a tooling failure, not a security finding.\n> ${getThreatDetectedMarker(detectionReason)}\n>\n> <details>\n> <summary>What happened</summary>\n>\n> ${reasonText}\n>\n> Review the [workflow run logs](${runUrl}) for details.\n> </details>`;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot move to template file in actions/setup/md and use render template file helpers to render.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. Created prompt_file_helpers.cjs with getPromptPath (with ../md fallback), renderTemplate, and renderTemplateFromFile. Updated generate_footer.cjs to import from this new module instead of using the inline local helpers. messages_core.cjs now also imports and re-exports from prompt_file_helpers.cjs for backward compatibility. Commit: a738c3c

Comment thread actions/setup/js/generate_footer.cjs Outdated
return `> [!WARNING]\n> threat detection engine error\n> The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.\n> ${getThreatDetectedMarker(detectionReason)}\n>\n> <details>\n> <summary>Details</summary>\n>\n> ${reasonText}\n>\n> Review the [workflow run logs](${runUrl}) for details.\n> </details>`;
return `> [!WARNING]\n> **Threat Detection Engine Failure** — The analysis engine could not complete. This is a tooling failure, not a security finding.\n> ${getThreatDetectedMarker(detectionReason)}\n>\n> <details>\n> <summary>What happened</summary>\n>\n> ${reasonText}\n>\n> Review the [workflow run logs](${runUrl}) for details.\n> </details>`;
}
return `> [!CAUTION]\n> agentic threat detected\n> Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.\n> ${getThreatDetectedMarker(detectionReason)}\n>\n> <details>\n> <summary>Details</summary>\n>\n> ${reasonText}\n>\n> Review the [workflow run logs](${runUrl}) for details.\n> </details>`;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot move to template file in actions/setup/md and use render template file helpers to render.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in the same commit. The threat_detection_caution.md template is rendered via renderTemplateFromFile(getPromptPath(...), context) from the new prompt_file_helpers.cjs module. Commit: a738c3c

@pelikhan
pelikhan marked this pull request as ready for review August 1, 2026 06:59
Copilot AI review requested due to automatic review settings August 1, 2026 06:59
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

PR Code Quality Reviewer completed the code quality review.

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR does not have the implementation label and has 0 new lines of code in business logic directories (threshold: 100).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Improves threat-detection failure messages with clearer titles and progressive disclosure.

Changes:

  • Clarifies tooling-failure warnings.
  • Collapses run details behind descriptive summaries.
  • Updates assertions and refreshes one unrelated workflow lock file.
Show a summary per file
File Description
actions/setup/md/detection_runs_comment.md Adds summary text and collapsible details.
actions/setup/js/messages.test.cjs Updates warning-title assertion.
actions/setup/js/messages_run_status.cjs Revises configured warning output.
actions/setup/js/generate_footer.test.cjs Updates footer assertions.
actions/setup/js/generate_footer.cjs Revises footer warning output.
.github/workflows/daily-byok-ollama-test.lock.yml Refreshes unrelated runtime configuration.

Review details

Tip

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

  • Files reviewed: 6/6 changed files
  • Comments generated: 4
  • Review effort level: Balanced

@@ -1,7 +1,14 @@
### {workflow_name}

The threat detection engine could not complete analysis for this run. This is a tooling failure, not a security finding.
Comment thread actions/setup/js/messages.test.cjs Outdated

expect(result).toContain("> [!WARNING]");
expect(result).toContain("threat detection engine error");
expect(result).toContain("Threat Detection Engine Failure");

expect(result).toContain("> [!WARNING]");
expect(result).toContain("threat detection engine error");
expect(result).toContain("Threat Detection Engine Failure");
(umask 177 && touch /tmp/gh-aw/threat-detection/detection.log)
GH_AW_MAX_AI_CREDITS="${{ vars.GH_AW_DEFAULT_DETECTION_MAX_AI_CREDITS || '400' }}"
printf '%s\n' "{\"\$schema\":\"https://github.com/github/gh-aw-firewall/releases/download/v0.27.43/awf-config.schema.json\",\"network\":{\"allowDomains\":[\"api.business.githubcopilot.com\",\"api.enterprise.githubcopilot.com\",\"api.github.com\",\"api.githubcopilot.com\",\"api.individual.githubcopilot.com\",\"github.com\",\"host.docker.internal\",\"raw.githubusercontent.com\",\"registry.npmjs.org\",\"telemetry.enterprise.githubcopilot.com\"]},\"apiProxy\":{\"enabled\":true,\"enableTokenSteering\":true,\"maxRuns\":500,\"maxAiCredits\":${GH_AW_MAX_AI_CREDITS},\"maxCacheMisses\":5,\"targets\":{\"copilot\":{\"host\":\"host.docker.internal:11434\"}}},\"container\":{\"imageTag\":\"0.27.43,squid=sha256:26be5e0b8c8f4c41c8a59126b29bb5d80b07253597472ded2a16bdd75abcbf9d,agent=sha256:04e2d1987a565000a8f114b89d806ae7a3864dd4f944be65275b28c93d8690e6,api-proxy=sha256:d85f57975af5ea23af4996e41ed73fbc8f5b4a47402472bfe82e508f352cb0c1,cli-proxy=sha256:65c45ea2967984d0024f3df61bc71335658a77ede96c8d9665da7a5f33a795ab\"},\"logging\":{\"proxyLogsDir\":\"/tmp/gh-aw/sandbox/firewall/logs\",\"auditDir\":\"/tmp/gh-aw/sandbox/firewall/audit\"}}" > "${RUNNER_TEMP}/gh-aw/awf-config.json"
printf '%s\n' "{\"\$schema\":\"https://github.com/github/gh-aw-firewall/releases/download/v0.27.43/awf-config.schema.json\",\"network\":{\"allowDomains\":[\"api.business.githubcopilot.com\",\"api.enterprise.githubcopilot.com\",\"api.github.com\",\"api.githubcopilot.com\",\"api.individual.githubcopilot.com\",\"github.com\",\"host.docker.internal\",\"raw.githubusercontent.com\",\"registry.npmjs.org\",\"telemetry.enterprise.githubcopilot.com\"]},\"apiProxy\":{\"enabled\":true,\"enableTokenSteering\":true,\"maxRuns\":500,\"maxAiCredits\":${GH_AW_MAX_AI_CREDITS},\"maxCacheMisses\":5,\"defaultAiCreditsPricing\":{\"input\":0.000001,\"output\":0.000001},\"targets\":{\"copilot\":{\"host\":\"host.docker.internal:11434\"}},\"models\":{\"agent\":[\"sonnet-6x\",\"gpt-5.4\",\"gpt-5.5\",\"gpt-5.6\",\"gpt-5.3\",\"gemini-pro\",\"any\"],\"antigravity\":[\"copilot/antigravity*\",\"google/antigravity*\",\"gemini/antigravity*\"],\"any\":[\"copilot/*\",\"anthropic/*\",\"openai/*\",\"google/*\",\"gemini/*\"],\"auto\":[\"copilot/auto\",\"large\"],\"claude\":[\"agent\"],\"codex\":[\"agent\"],\"coding\":[\"copilot/gpt-5*codex*\",\"openai/gpt-5*codex*\",\"gpt-5-codex\",\"kimi\"],\"computer-use\":[\"copilot/*computer-use*\",\"google/*computer-use*\",\"gemini/*computer-use*\",\"openai/*computer-use*\"],\"copilot\":[\"agent\"],\"deep-research\":[\"copilot/deep-research*\",\"copilot/o3-deep-research*\",\"copilot/o4-mini-deep-research*\",\"google/deep-research*\",\"gemini/deep-research*\",\"openai/o3-deep-research*\",\"openai/o4-mini-deep-research*\"],\"fable\":[\"copilot/*fable*\",\"anthropic/*fable*\"],\"gemini\":[\"agent\"],\"gemini-3-flash\":[\"copilot/gemini-3*flash*\",\"google/gemini-3*flash*\",\"gemini/gemini-3*flash*\"],\"gemini-3-pro\":[\"copilot/gemini-3*pro*\",\"google/gemini-3*pro*\",\"google/nano-banana*\",\"gemini/gemini-3*pro*\"],\"gemini-3.1-flash\":[\"copilot/gemini-3.1*flash*\",\"google/gemini-3.1*flash*\",\"gemini/gemini-3.1*flash*\"],\"gemini-3.1-pro\":[\"copilot/gemini-3.1*pro*\",\"google/gemini-3.1*pro*\",\"gemini/gemini-3.1*pro*\"],\"gemini-3.5-flash\":[\"copilot/gemini-3.5*flash*\",\"google/gemini-3.5*flash*\",\"gemini/gemini-3.5*flash*\"],\"gemini-3.6-flash\":[\"copilot/gemini-3.6*flash*\",\"google/gemini-3.6*flash*\",\"gemini/gemini-3.6*flash*\"],\"gemini-flash\":[\"copilot/gemini-*flash*\",\"google/gemini-*flash*\",\"gemini/gemini-*flash*\"],\"gemini-flash-lite\":[\"copilot/gemini-*flash*lite*\",\"google/gemini-*flash*lite*\",\"gemini/gemini-*flash*lite*\"],\"gemini-omni\":[\"copilot/gemini-omni*\",\"google/gemini-omni*\",\"gemini/gemini-omni*\"],\"gemini-pro\":[\"copilot/gemini-*pro*\",\"google/gemini-*pro*\",\"gemini/gemini-*pro*\"],\"gemma\":[\"copilot/gemma*\",\"google/gemma*\",\"gemini/gemma*\"],\"gpt-5\":[\"copilot/gpt-5*\",\"openai/gpt-5*\"],\"gpt-5-codex\":[\"copilot/gpt-5*codex*\",\"openai/gpt-5*codex*\"],\"gpt-5-mini\":[\"copilot/gpt-5*mini*\",\"openai/gpt-5*mini*\"],\"gpt-5-nano\":[\"copilot/gpt-5*nano*\",\"openai/gpt-5*nano*\"],\"gpt-5-pro\":[\"copilot/gpt-5*pro*\",\"openai/gpt-5*pro*\"],\"gpt-5.1\":[\"copilot/gpt-5.1*\",\"openai/gpt-5.1*\"],\"gpt-5.2\":[\"copilot/gpt-5.2*\",\"openai/gpt-5.2*\"],\"gpt-5.3\":[\"copilot/gpt-5.3*\",\"openai/gpt-5.3*\"],\"gpt-5.4\":[\"copilot/gpt-5.4*\",\"openai/gpt-5.4*\"],\"gpt-5.5\":[\"copilot/gpt-5.5*\",\"openai/gpt-5.5*\"],\"gpt-5.6\":[\"copilot/gpt-5.6*\",\"openai/gpt-5.6*\"],\"grok\":[\"copilot/*grok*\",\"openai/*grok*\"],\"haiku\":[\"copilot/*haiku*\",\"anthropic/*haiku*\"],\"image-generation\":[\"copilot/gpt-image*\",\"openai/gpt-image*\",\"openai/chatgpt-image*\",\"copilot/gemini-*image*\",\"google/gemini-*image*\",\"gemini/gemini-*image*\",\"google/imagen*\"],\"kimi\":[\"copilot/kimi*\",\"openai/kimi*\"],\"kiwi\":[\"copilot/kiwi*\",\"openai/kiwi*\"],\"large\":[\"sonnet\",\"gpt-5-pro\",\"gpt-5\",\"gemini-pro\"],\"lyria\":[\"google/lyria*\",\"gemini/lyria*\",\"copilot/lyria*\"],\"mai-code\":[\"copilot/MAI-Code*\",\"copilot/mai-code*\",\"openai/MAI-Code*\"],\"mai-code-1-flash-picker\":[\"copilot/MAI-Code-1-Flash-picker*\",\"copilot/mai-code-1-flash-picker*\",\"openai/MAI-Code-1-Flash-picker*\"],\"mini\":[\"haiku\",\"gpt-5-mini\",\"gpt-5-nano\",\"gemini-flash-lite\"],\"nano-banana\":[\"copilot/nano-banana*\",\"google/nano-banana*\",\"gemini/nano-banana*\"],\"opus\":[\"copilot/*opus*\",\"anthropic/*opus*\"],\"opusplan\":[\"opus?effort=high\"],\"raptor-mini\":[\"copilot/raptor*\",\"openai/raptor*\"],\"reasoning\":[\"copilot/o1*\",\"copilot/o3*\",\"copilot/o4*\",\"openai/o1*\",\"openai/o3*\",\"openai/o4*\"],\"robotics\":[\"copilot/*robotics*\",\"google/*robotics*\",\"gemini/*robotics*\"],\"small\":[\"mini\"],\"small-agent\":[\"haiku\",\"gpt-5-mini\",\"gemini-flash\"],\"sonnet\":[\"copilot/*sonnet*\",\"anthropic/*sonnet*\"],\"sonnet-6x\":[\"copilot/*sonnet-4.5*\",\"copilot/*sonnet-4.6*\",\"copilot/*sonnet-5*\",\"copilot/*sonnet-4-5-*\",\"anthropic/*sonnet-4-5-*\",\"copilot/*sonnet-4-6*\",\"anthropic/*sonnet-4-6*\",\"anthropic/*sonnet-5*\"],\"summarization\":[\"haiku\",\"gpt-5-mini\",\"gemini-flash-lite\",\"mini\"],\"veo\":[\"google/veo*\",\"gemini/veo*\"],\"vision\":[\"copilot/gemini-*image*\",\"google/gemini-*image*\",\"gemini/gemini-*image*\",\"copilot/gemini-*flash*\",\"google/gemini-*flash*\",\"gemini/gemini-*flash*\"]}},\"container\":{\"imageTag\":\"0.27.43,squid=sha256:26be5e0b8c8f4c41c8a59126b29bb5d80b07253597472ded2a16bdd75abcbf9d,agent=sha256:04e2d1987a565000a8f114b89d806ae7a3864dd4f944be65275b28c93d8690e6,api-proxy=sha256:d85f57975af5ea23af4996e41ed73fbc8f5b4a47402472bfe82e508f352cb0c1,cli-proxy=sha256:65c45ea2967984d0024f3df61bc71335658a77ede96c8d9665da7a5f33a795ab\"},\"logging\":{\"proxyLogsDir\":\"/tmp/gh-aw/sandbox/firewall/logs\",\"auditDir\":\"/tmp/gh-aw/sandbox/firewall/audit\"}}" > "${RUNNER_TEMP}/gh-aw/awf-config.json"
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Test Quality Sentinel completed test quality analysis.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Skills-Based Review 🧠

Applied /grill-with-docs — clean improvement, two minor consistency issues.

📋 Key Themes & Highlights

Key Themes

  • Naming: "Threat Detection Engine Failure" is bold, capitalised, and semantically clear — good.
  • Progressive disclosure: adding an always-visible summary in detection_runs_comment.md and collapsing the raw table is a solid UX win.
  • Consistency gap: the [!CAUTION] branch in both generate_footer.cjs and messages_run_status.cjs still uses <summary>Details</summary> while the [!WARNING] branch now says What happened. Left behind in this pass.
  • Generic summary: the lead sentence in detection_runs_comment.md does not surface {reason} or {conclusion}, so users still have to expand the block for any actionable context.

Positive Highlights

  • ✅ Tests updated to match new copy — no drift between implementation and assertions.
  • ✅ Consistent change applied across both generate_footer.cjs and messages_run_status.cjs.
  • "This is a tooling failure, not a security finding." is an excellent user-calming clarification.

@copilot please address the review comments above.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 27.8 AIC · ⌖ 9.76 AIC · ⊞ 7K
Comment /matt to run again

@@ -1,7 +1,14 @@
### {workflow_name}

The threat detection engine could not complete analysis for this run. This is a tooling failure, not a security finding.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/grill-with-docs] The always-visible summary is generic and does not mention the specific failure reason ({reason}), reducing its diagnostic value.

💡 Suggestion

Consider incorporating {reason} or {conclusion} into the lead sentence so readers get a quick signal without expanding the details block:

The threat detection engine could not complete analysis (`{conclusion}`). This is a tooling failure, not a security finding.

This keeps progressive disclosure intact while giving an at-a-glance clue.

@copilot please address this.

Comment thread actions/setup/js/generate_footer.cjs Outdated
return `> [!WARNING]\n> threat detection engine error\n> The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.\n> ${getThreatDetectedMarker(detectionReason)}\n>\n> <details>\n> <summary>Details</summary>\n>\n> ${reasonText}\n>\n> Review the [workflow run logs](${runUrl}) for details.\n> </details>`;
return `> [!WARNING]\n> **Threat Detection Engine Failure** — The analysis engine could not complete. This is a tooling failure, not a security finding.\n> ${getThreatDetectedMarker(detectionReason)}\n>\n> <details>\n> <summary>What happened</summary>\n>\n> ${reasonText}\n>\n> Review the [workflow run logs](${runUrl}) for details.\n> </details>`;
}
return `> [!CAUTION]\n> agentic threat detected\n> Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.\n> ${getThreatDetectedMarker(detectionReason)}\n>\n> <details>\n> <summary>Details</summary>\n>\n> ${reasonText}\n>\n> Review the [workflow run logs](${runUrl}) for details.\n> </details>`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/grill-with-docs] The [!CAUTION] branch (actual threat detections) still uses <summary>Details</summary>, inconsistent with the What happened label introduced in the [!WARNING] branch.

💡 Suggestion

Apply the same label update to the CAUTION path for consistency:

return `> [!CAUTION]\n> agentic threat detected\n> ...\n>\n> <details>\n> <summary>What happened</summary>\n>\n> ...`;

The same applies to the parallel template in messages_run_status.cjs.

@copilot please address this.

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 100/100 — Excellent

Analyzed 2 test(s): 2 design, 0 implementation, 0 violation(s).

📊 Metrics (2 tests)
Metric Value
Analyzed 2 (Go: 0, JS: 2)
✅ Design 2 (100%)
⚠️ Implementation 0 (0%)
Edge/error coverage 2 (100%)
Duplicate clusters 0
Inflation No (test +3 lines, prod +2 lines, ratio 1.5:1)
🚨 Violations 0
Test File Classification Issues
getExpiredEntityCautionAlert (with detection reason) generate_footer.test.cjs:486 design_test, high_value None
getDetectionCautionAlert messages.test.cjs:1411 design_test, high_value None

Summary

These changes update 3 string-literal assertions across 2 existing test functions to match the renamed error title. The tests verify user-visible output text and include both positive (.toContain) and negative (.not.toContain) assertions, providing solid behavioral coverage. No new test functions were added — this is a sync-only change following a production string rename.

Verdict

Passed. 0% implementation tests (threshold: 30%). Tests faithfully track the renamed error template title.

🧪 Test quality analysis by Test Quality Sentinel · sonnet46 · 36.7 AIC · ⌖ 9.48 AIC · ⊞ 8.4K ·
Comment /review to run again

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Test Quality Sentinel: 100/100. 0% implementation tests (threshold: 30%). Tests faithfully track the renamed error template title.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Blocking: this PR does not address the maintainer's own unresolved review comment to move templates to actions/setup/md files.

Themes
  • The wording improvements themselves are fine (bolded title, better summary label), but the underlying architectural request from @pelikhan on lines 136/138 of generate_footer.cjs — extract to a template file and use render helpers — was ignored, even though this same PR proves the pattern works (detection_runs_comment.md).
  • As a direct consequence, the near-identical warning string is now duplicated across generate_footer.cjs and messages_run_status.cjs, and has already drifted in wording from the .md template touched in the same PR. This is the exact maintenance risk the reviewer was trying to prevent.
  • The daily-byok-ollama-test.lock.yml diff appears to be an unrelated regenerated artifact (models/pricing config) and is out of scope for a wording-only PR; not blocking but worth a note/split.

🔎 Code quality review by PR Code Quality Reviewer · auto · 57.9 AIC · ⌖ 10.9 AIC · ⊞ 7.8K
Comment /review to run again

Comment thread actions/setup/js/generate_footer.cjs Outdated
const reasonText = getDetectionReasonText(detectionReason);
if (isToolingFailureReason(detectionReason)) {
return `> [!WARNING]\n> threat detection engine error\n> The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.\n> ${getThreatDetectedMarker(detectionReason)}\n>\n> <details>\n> <summary>Details</summary>\n>\n> ${reasonText}\n>\n> Review the [workflow run logs](${runUrl}) for details.\n> </details>`;
return `> [!WARNING]\n> **Threat Detection Engine Failure** — The analysis engine could not complete. This is a tooling failure, not a security finding.\n> ${getThreatDetectedMarker(detectionReason)}\n>\n> <details>\n> <summary>What happened</summary>\n>\n> ${reasonText}\n>\n> Review the [workflow run logs](${runUrl}) for details.\n> </details>`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@pelikhan's explicit review request on this exact line — move to a template file in actions/setup/md and render via existing helpers — was not addressed.

💡 Why this matters

The PR already demonstrates the correct pattern: detection_runs_comment.md was updated to use a proper template file with a <details> block. Yet this near-identical warning string was left as an inline JS template literal here, duplicated again almost verbatim in messages_run_status.cjs (see companion comment). This is exactly the maintenance burden the reviewer flagged: any future wording tweak must now be applied in at least two JS files plus the .md file, and they have already drifted (compare "The analysis engine could not complete" here vs. "The threat detection engine could not complete analysis for this run" in detection_runs_comment.md).

Suggested fix: extract this and the CAUTION variant into actions/setup/md/*.md template files and call the same render-template-file helper used elsewhere, exactly as requested.

const isEngineError = isToolingFailureReason(ctx.reason);
if (isEngineError) {
const defaultTemplate = `> [!WARNING]\n> threat detection engine error\n> The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.\n> ${getThreatDetectedMarkerTemplate()}\n>\n> <details>\n> <summary>Details</summary>\n>\n> {reason_text}\n>\n> Review the [workflow run logs]({run_url}) for details.\n> </details>`;
const defaultTemplate = `> [!WARNING]\n> **Threat Detection Engine Failure** — The analysis engine could not complete. This is a tooling failure, not a security finding.\n> ${getThreatDetectedMarkerTemplate()}\n>\n> <details>\n> <summary>What happened</summary>\n>\n> {reason_text}\n>\n> Review the [workflow run logs]({run_url}) for details.\n> </details>`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This warning string duplicates generate_footer.cjs line 136 verbatim, and its wording has already drifted from the .md template used elsewhere in this same PR.

💡 Drift risk detail

Compare the three near-identical strings introduced/touched by this PR:

  • Here and generate_footer.cjs: "The analysis engine could not complete."
  • detection_runs_comment.md: "The threat detection engine could not complete analysis for this run."

They already say different things despite describing the exact same failure condition, in the same PR. This confirms the maintainer's concern (unresolved comment on generate_footer.cjs lines 136/138): keeping the template inline instead of a shared .md file makes future edits error-prone — a wording change here is easy to forget to apply in generate_footer.cjs (or vice versa), and the .md copy is already out of sync as a preview of that risk.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review: Threat Detection Engine Error Template Improvements

The changes are clean, consistent, and well-tested. The progressive disclosure pattern improves scannability.

Observations:

  • Title change to bold **Threat Detection Engine Failure** is a meaningful UX improvement.
  • Description tightening is clearer than the original.
  • <summary>What happened</summary> is more informative than generic Details.
  • detection_runs_comment.md now provides an always-visible context sentence before the collapsible block — good pattern.
  • Test assertions are updated consistently across both test files.

Note: The .lock.yml change appears to be an unrelated firewall config update (model routing, pricing defaults) bundled into this PR — consider separating it.

Existing review comments already flag the right architectural follow-up: inline template strings in generate_footer.cjs and messages_run_status.cjs should be moved to template files under actions/setup/md/ using render-template helpers, consistent with the detection_runs_comment.md approach. No further blocking issues found.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 15 AIC · ⌖ 7.19 AIC · ⊞ 5.3K

Copilot AI requested a review from pelikhan August 1, 2026 08:15
@gh-aw-bot

Copy link
Copy Markdown
Collaborator

Caution

agentic threat detected
Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.

Details

Potential security threats were detected in the agent output.

Review the workflow run logs for details.

@copilot quick triage for this PR:

Generated by 👨🍳 PR Sous Chef
Comment /souschef to run again

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 19.5 AIC · ⌖ 5.58 AIC · ⊞ 8.1K ·
Comment /souschef to run again

…est test setup

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

[!CAUTION]
agentic threat detected
Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation....

Fixed the failing JS Tests (shard 3/4). The create_pull_request - threat detection caution test suite's beforeEach was missing copies of threat_detection_caution.md and threat_detection_engine_error.md in its prompts directory setup — tests were failing with ENOENT when trying to render those templates. Added the two missing copyPromptTemplate calls. Commit: 1e793d4

Copilot AI requested a review from gh-aw-bot August 1, 2026 09:14
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

PR Triage

Category: feature · Risk: low · Priority: medium · Score: 53/100 (impact 22, urgency 15, quality 16)

Recommended action: batch_review

UX polish for threat-detection error templates (bold titles, clearer disclosure labels). Active review thread (8 comments, 16 review comments). Groups well with other safe-outputs/threat-detection UX PRs for batch review.

Generated by 🔧 PR Triage Agent · auto · 49.2 AIC · ⌖ 8.64 AIC · ⊞ 8K ·

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

PR Triage

  • Category: feature
  • Risk: low
  • Priority: low
  • Score: 35/100 (Impact15/Urgency8/Quality12)
  • Recommended action: batch_review

Cosmetic error-template UX improvement, low risk, no urgency.

Generated by 🔧 PR Triage Agent · auto · 73.4 AIC · ⌖ 10.1 AIC · ⊞ 8K ·

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot quick triage for this PR:

  • Review feedback still worth closing out before merge:
    1. pelikhan — confirm the existing helpers are the only template-render path now and that no duplicate helper layer remains: improve: threat detection engine error template with progressive disclosure #49497
    2. github-actions[bot] — ensure the progressive-disclosure/template follow-up is fully dispositioned and summarize what remains maintainer-facing.
  • Please run the pr-finisher skill, resolve/disposition any satisfied review feedback, and summarize whether this PR is blocked on maintainer review only.

Generated by 👨🍳 PR Sous Chef
Comment /souschef to run again

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 21 AIC · ⌖ 5.28 AIC · ⊞ 5.7K ·
Comment /souschef to run again

…ons for progressive disclosure

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

@copilot quick triage for this PR:

  • Review feedback still worth closing out before merge:
    1. pelikhan — confirm the existing helpers are the only template-render path now and that no duplicate he...

All open items addressed in the latest commit:

  1. No duplicate helpersprompt_file_helpers.cjs was removed in cb7edf4. generate_footer.cjs and messages_run_status.cjs both import getPromptPath and renderTemplateFromFile directly from messages_core.cjs, which is the single source of truth for template helpers.

  2. Progressive-disclosure follow-up fully dispositioned:

    • detection_runs_comment.md: static "tooling failure" text replaced with neutral "Threat detection produced a **{conclusion}** result for this run." — correct for both actual threat warnings and engine failures (latest commit).
    • generate_footer.test.cjs and messages.test.cjs: assertions now check **Threat Detection Engine Failure** (bold title) and What happened (disclosure label), not just plain text.
  3. CI — the shard 3/4 ENOENT failure was fixed in 7a731b9 by adding the missing copyPromptTemplate calls to the create_pull_request - threat detection caution beforeEach.

This PR is now blocked on maintainer review only — no outstanding automation failures or unaddressed feedback.

@pelikhan

pelikhan commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

…ure test setup

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

@copilot fix https://github.com/github/gh-aw/actions/runs/30713984159/job/91407875470

Fixed. The detection caution placement in main() describe block in handle_agent_failure.test.cjs was missing threat_detection_caution.md and threat_detection_engine_error.md in its beforeEach template setup — the same ENOENT pattern as the previous shard 3/4 fix. Added both files via fs.writeFileSync matching the existing inline-content style. All 335 tests in that file now pass.

@pelikhan

pelikhan commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

@copilot resolve the merge conflicts on this branch.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts on this branch.

Done. Merged main into this branch and resolved the three conflicts:

  • messages_run_status.cjs: kept the template-file approach from this PR and adopted the distinct <!-- gh-aw-threat-engine-error --> marker introduced in be1047f — engine errors now use getThreatEngineErrorMarkerTemplate() while real threats use getThreatDetectedMarkerTemplate()
  • generate_footer.test.cjs and messages.test.cjs: combined both sides — assertions now check **Threat Detection Engine Failure**, What happened, <!-- gh-aw-threat-engine-error -->, and not.toContain("<!-- gh-aw-threat-detected -->")

All 268 workflows recompiled and pre-PR validation passed.

@pelikhan
pelikhan merged commit c81f086 into main Aug 1, 2026
8 checks passed
@pelikhan
pelikhan deleted the copilot/improve-error-template branch August 1, 2026 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants