Use workflow frontmatter emoji for activation status comments - #53494
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #53494 does not have the implementation label and has only 3 new lines of code in business logic directories (threshold: 100).
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Ponytail Reviewer completed successfully! Lean already. Ship.
|
|
❌ Test Quality Sentinel was cancelled during test quality analysis. Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details.
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict
I don't see a blocking correctness or maintainability problem in this diff.
Review notes
- The emoji plumb-through is consistent from workflow frontmatter to activation comment rendering.
- Existing behavior still falls back to
🚀when no workflow emoji is configured. - Tests cover the new happy path in both the message formatter and the status-comment builder.
- I did notice the test gap for whitespace-only
emojivalues, but the implementation already trims and falls back safely, so that's not enough to block this PR.
🔎 Code quality review by PR Code Quality Reviewer · gpt54 · 11.6 AIC · ⌖ 6.82 AIC · ⊞ 4.5K
Comment /review to run again
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /tdd and /codebase-design — approving with two minor observations.
📋 Key Themes & Highlights
Key Themes
- Correct layering: the emoji flows naturally from Go compiler → env var → JS runtime → message template, consistent with how
GH_AW_WORKFLOW_NAMEandGH_AW_TRACKER_IDare threaded - Good defensive normalization: whitespace-only emoji strings fall back to 🚀 at the JS layer; non-empty guard at the Go layer avoids emitting an unnecessary env var
- Test coverage: both new happy-path tests and the custom-template-with-
{emoji}test are solid
Positive Highlights
- ✅ Conditional env emission in Go (
if ctx.data.FrontmatterEmoji != "") mirrors the existingTrackerIDpattern exactly — easy to follow - ✅ Spread
{ ...ctx, emoji: normalizedEmoji }avoids mutating the caller's object - ✅
teardowncorrectly deletesGH_AW_WORKFLOW_EMOJIto prevent test bleed
Minor Observations (non-blocking)
-
Whitespace-only emoji not tested at the env boundary —
messages_run_status.test.cjsvalidates the trim-and-fallback logic insidegetRunStartedMessage, butadd_workflow_run_comment.test.cjshas no test forGH_AW_WORKFLOW_EMOJI = " "(should fall back to 🚀). Low risk since the normalization lives entirely ingetRunStartedMessage, but an explicit integration test would close the gap. -
Custom template without
{emoji}placeholder — there is no test asserting that a customrunStartedtemplate that omits{emoji}still renders correctly when an emoji is present in context. The substitution mechanism silently drops unknown placeholders, so this is safe today, but a test would document the intentional behaviour.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 28.7 AIC · ⌖ 12.8 AIC · ⊞ 7.8K
Comment /matt to run again
There was a problem hiding this comment.
The implementation is clean and well-structured. The three-layer change (Go compiler → env var → JS message builder) is correct. Backward compatibility is preserved via the 🚀 default fallback. The %q formatting in Go safely quotes the emoji value in generated YAML. Tests cover the new env var path and custom template support. No blocking issues found.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 28.5 AIC · ⌖ 8.75 AIC · ⊞ 5.7K
There was a problem hiding this comment.
Pull request overview
Updates activation status comments to use each workflow’s configured frontmatter emoji, retaining 🚀 as the fallback.
Changes:
- Exports workflow emoji during activation.
- Renders default and custom started messages with the emoji.
- Adds JavaScript coverage and regenerates command metadata.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/compiler_activation_steps.go |
Exports the frontmatter emoji. |
actions/setup/js/messages_run_status.cjs |
Adds emoji-aware started-message rendering. |
actions/setup/js/messages_run_status.test.cjs |
Tests rendering and custom templates. |
actions/setup/js/add_workflow_run_comment.cjs |
Passes the environment emoji into rendering. |
actions/setup/js/add_workflow_run_comment.test.cjs |
Tests emoji use in comment bodies. |
.github/workflows/agentic_commands.yml |
Regenerates command help metadata. |
Review details
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Review effort level: Balanced
| const normalizedEmoji = typeof ctx?.emoji === "string" && ctx.emoji.trim() ? ctx.emoji.trim() : "🚀"; | ||
| return renderConfiguredMessage("runStarted", "{emoji} [{workflow_name}]({run_url}) has started processing this {event_type}", { ...ctx, emoji: normalizedEmoji }); |
| if ctx.data.FrontmatterEmoji != "" { | ||
| ctx.steps = append(ctx.steps, fmt.Sprintf(" GH_AW_WORKFLOW_EMOJI: %q\n", ctx.data.FrontmatterEmoji)) |
|
@copilot run pr-finisher skill |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Completed the PR-finisher pass in |
|
/smoke-copilot |
|
📰 BREAKING: Smoke Copilot is now investigating this issue comment. Sources say the story is developing... |
|
@copilot make it work for centralized slash commands and label commands as well |
|
@copilot Please review the latest feedback and finish this PR. Remaining maintainer-facing work:
|
|
Smoke Test Summary (author: app/copilot-swe-agent, assignees: pelikhan, Copilot)
PR reviewed: "Use workflow frontmatter emoji for activation status comments" Overall status: FAIL (1 test failed: Serena) Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "www.google.com"
- "www.gstatic.com"See Network Configuration for more information.
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment. Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "www.google.com"
- "www.gstatic.com"See Network Configuration for more information.
|
There was a problem hiding this comment.
Smoke test review: all good, ugh.
Warning
Firewall blocked 6 domains
The following domains were blocked by the firewall during workflow execution:
accounts.google.comandroid.clients.google.comclients2.google.comcontentautofill.googleapis.comwww.google.comwww.gstatic.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "www.google.com"
- "www.gstatic.com"See Network Configuration for more information.
📰 BREAKING: Report filed by Smoke Copilot · auto · 36.5 AIC · ⌖ 2.3 AIC · ⊞ 9.1K
Comment /smoke-copilot to run again
Add label smoke to run again
Comments that could not be inline-anchored
.github/workflows/ace-editor.lock.yml:1
Smoke test: inline review comment (caveman test say hi).
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Implemented in |
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Updated the branch with current |
PR TriageCategory: feature · Risk: medium · Score: 53/100 Breakdown: impact 30/50 · urgency 15/30 · quality 8/20 Recommended action: Uses workflow frontmatter emoji for activation status comments; touches compiler code (
|
Status comments for agentic workflows were always emitted with a rocket in the “run started” message, even when the workflow markdown defined a different emoji (e.g., 🤖). This change makes status comments honor the workflow’s configured emoji.
What changed
emojiinto activation comment generation by exportingGH_AW_WORKFLOW_EMOJIin activation step env.{emoji}with a default fallback to🚀when no workflow emoji is configured.add_workflow_run_commentso created/reused status comments render the correct icon.Behavioral impact
emoji: 🤖now post started-status comments prefixed with 🤖 instead of 🚀.Representative snippet
run: https://github.com/github/gh-aw/actions/runs/32080816855> Generated by 👨🍳 PR Sous Chef · gpt54 · 10.1 AIC · ⌖ 7.87 AIC · ⊞ 8.8K · ◷