Skip to content

fix: restructure cache miss alert template format (#28609)#28611

Merged
pelikhan merged 4 commits intomainfrom
copilot/fix-cache-miss-alert-template-format
Apr 26, 2026
Merged

fix: restructure cache miss alert template format (#28609)#28611
pelikhan merged 4 commits intomainfrom
copilot/fix-cache-miss-alert-template-format

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 26, 2026

Summary

Fixes the cache miss alert template format as described in issue #28609.

Problem: The cache_memory_miss.md template rendered a > [!WARNING] callout block with a <details> section whose <summary> was "How to fix". The issue title ("Cache Configuration Problem") was buried inside the callout, not surfaced as the collapsible summary.

Fix: Restructure the template so:

  • The <details> block is nested inside the > [!WARNING] alert
  • The cache miss title (Cache Configuration Problem: ...) is the <summary> of the <details> block (no redundant emoji — the WARNING alert already provides the visual warning)
  • The how-to-fix content is inside the <details> body

Changes

  • actions/setup/md/cache_memory_miss.md: Replaced the flat > [!WARNING] + separate <details><summary>How to fix</summary> pattern with a > [!WARNING] alert wrapping a <details><summary>Cache Configuration Problem: ...</summary> block containing the fix content
  • actions/setup/js/handle_agent_failure.test.cjs: Updated mock template strings to match new format, updated assertions to check for > [!WARNING] and <summary>, extracted long strings into named const variables for readability

Before / After

Before:

> [!WARNING]
> **Cache Configuration Problem**: cache miss detected despite cache-memory being configured.

<details>
<summary>How to fix</summary>

...fix content...
</details>

After:

> [!WARNING]
> <details>
> <summary>Cache Configuration Problem: cache miss detected despite cache-memory being configured.</summary>
>
> ...fix content...
>
> </details>

Copilot AI and others added 3 commits April 26, 2026 15:43
- Move 'Cache Configuration Problem' title into <summary> tag
- Move fix content into <details> body
- Remove standalone > [!WARNING] block
- Update tests to reflect new format

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/aa9be2ff-7984-4afa-8fb5-c57244bb3af4

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@@ -1,8 +1,5 @@
> [!WARNING]
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 must still be wrapped in a WARNING alert

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 61be0d1 — the <details> block is now nested inside a > [!WARNING] blockquote alert.

Comment thread actions/setup/md/cache_memory_miss.md Outdated

<details>
<summary>How to fix</summary>
<summary>⚠️ Cache Configuration Problem: cache miss detected despite cache-memory being configured.</summary>
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 remove warning emoji, already in alert title

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.

Removed in 61be0d1 — summary is now Cache Configuration Problem: cache miss detected despite cache-memory being configured. without the emoji.

…rom summary

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/239b52a1-b761-4ca7-8be1-245a9ef3a507

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI requested a review from pelikhan April 26, 2026 15:57
@pelikhan pelikhan marked this pull request as ready for review April 26, 2026 16:10
Copilot AI review requested due to automatic review settings April 26, 2026 16:10
@pelikhan pelikhan merged commit d185f07 into main Apr 26, 2026
25 checks passed
@pelikhan pelikhan deleted the copilot/fix-cache-miss-alert-template-format branch April 26, 2026 16:10
@github-actions github-actions Bot mentioned this pull request Apr 26, 2026
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

Restructures the cache-miss warning markdown template so the cache configuration problem title is surfaced as the <details><summary> inside a GitHub [!WARNING] callout (per issue #28609).

Changes:

  • Reworked cache_memory_miss.md to nest a <details> block inside the [!WARNING] callout and move the problem title into the <summary>.
  • Updated handle_agent_failure unit tests to align mocked template content and assertions with the new markdown structure.
  • Updated several embedded action pins (including removal of a github/gh-aw-actions/setup pin entry).
Show a summary per file
File Description
pkg/workflow/data/action_pins.json Updates embedded action version/SHA pins (and removes the github/gh-aw-actions/setup pin).
actions/setup/md/cache_memory_miss.md Restructures warning template to make the cache configuration title the collapsible summary inside the warning callout.
actions/setup/js/handle_agent_failure.test.cjs Updates mocked template strings/assertions to match the new callout + details format.

Copilot's findings

Tip

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

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

"sha": "19c436149e80e5be4f0adbd9cdeb391acea5fa91"
"version": "v4.35.2",
"sha": "7fc6561ed893d15cec696e062df840b21db27eb0"
},
@github-actions
Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 93/100

Excellent test quality

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

Test Classification Details

View Test Classification Table
Test File Classification Issues Detected
"appends cache configuration warning when cacheMemoryEnabled is true and cache_memory_miss item present" actions/setup/js/handle_agent_failure.test.cjs:1137 ✅ Design None — verifies observable output contains correct warning format
"captures reason-only missing_data items (no data_type) and detects cache miss" actions/setup/js/handle_agent_failure.test.cjs:1164 ✅ Design None — verifies observable output includes > [!WARNING] block

Analysis Summary

Both modified tests update expected template content and add a new expect(result).toContain("> [!WARNING]") assertion to track the production change: the cache miss alert template was reformatted from a bare <details> block to a GitHub Markdown > [!WARNING] alert block wrapping the details.

Strengths:

  • Both tests verify the observable output of buildMissingDataContext() — a behavioral contract that a consumer (GitHub issue comment) would directly observe
  • The new > [!WARNING] assertion correctly anchors the new format requirement, ensuring regressions would be caught
  • fs I/O is mocked (acceptable — external side effect), but assertions are on the function's return value, not on mock call counts
  • Test additions (9 lines) vs. template file additions (11 lines) — no inflation

Minor note: The score is slightly below 100 because these are format-tracking updates rather than new scenario coverage; no new behavioral edge cases are introduced. The existing test suite already covers the broader buildMissingDataContext logic.


Language Support

Tests analyzed:

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

Verdict

Check passed. 0% of new/modified 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: §24961102679

🧪 Test quality analysis by Test Quality Sentinel · ● 738.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: 93/100. Test quality is excellent — 0% of new/modified tests are implementation tests (threshold: 30%). Both modified tests correctly track the new > [!WARNING] cache miss alert format and assert on observable function output.

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