[alexandreferris]

---

PR Submission Checklist for internal contributors

• The PR Title

  • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
  • contains a reference JIRA issue number like SQPIT-764
  • answers the question: If merged, this PR will: ... ³

• The PR Description

  • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

---

What's new in this PR?

Issues

GitHub Actions workflow and job completion events were not rendered as Wire notifications

Causes

The webhook payload model and Mustache templates did not support workflow_run or workflow_job events

Solutions

• Added support for completed workflow runs and jobs
• Added centralized Mustache templates with conclusion-specific status icons
• Added webhook fixtures, rendering tests, and event-handling coverage
• Updated setup documentation and help text

Testing

Test Coverage (Optional)

• I have added automated test to this contribution

How to Test

• When Github App is running
• Add webhook to a conversation
• Select Actions/Workflow options

[claude]

Review: feat: Add Github Actions workflow in webhook #WPB-28780

Nice, focused addition — it slots into the existing event.action → Mustache template mechanism without touching Routing/TemplateHandler, reuses the emoji computed-property pattern already established by Review, and comes with both unit and HTTP-level tests plus docs. A few things I would like to see addressed before merge, mainly around payload nullability and the divergence between the two emoji maps.

---

🐛 Potential bugs

1. WorkflowRun.name / head_branch (and likely actor) are nullable in GitHub's payload → the delivery will 400 — src/main/kotlin/com/wire/github/response/model/WorkflowRun.kt:8-16

In GitHub's workflow_run schema, name and head_branch are string | null, and actor is nullable in the workflow-run schema too. The model declares them non-nullable with no default:

val name: String,
@SerialName("head_branch")
val headBranch: String,
val actor: User,

KtxSerializer sets coerceInputValues = true, but coercion only applies to non-nullable properties that have a default value — these do not. So an explicit null (or a missing key) throws MissingFieldException / SerializationException, which Routing.kt:113-117 turns into a 400. The notification is silently lost and GitHub records a failed delivery that will never succeed on redelivery.

This is also internally inconsistent: WorkflowJob correctly declares workflowName and headBranch as String? = null, but WorkflowRun does not for the same concepts. Suggested:

val name: String? = null,
@SerialName("head_branch")
val headBranch: String? = null,
val actor: User? = null,

…with the template guarding them via sections, the way workflow_job.completed.template already does for workflowName / headBranch. Worth the same audit on event and run_number while you are in there — defaults cost nothing and make the endpoint resilient to payload drift.

2. The two emoji maps have diverged — WorkflowJob.kt:17-24 vs WorkflowRun.kt:18-28

WorkflowRun maps timed_out, action_required and stale; WorkflowJob does not. Job conclusions can legitimately be timed_out / action_required, so a timed-out job renders as ℹ️ while a timed-out run renders as ⏱️ — which contradicts the README added in this PR ("including … timed-out results"). Since the mapping is conclusion-based and identical in intent, I would extract it once:

// com/wire/github/response/model/Conclusion.kt
internal fun conclusionEmoji(conclusion: String?): String = when (conclusion) {
    "success" -> "✅"
    "failure" -> "❌"
    "cancelled" -> "🚫"
    "timed_out" -> "⏱️"
    "action_required" -> "⚠️"
    "skipped" -> "⏭️"
    "stale" -> "💤"
    else -> "ℹ️"
}

and have both models delegate to it. That also removes the duplicated when block.

3. conclusion is rendered raw and unguarded — both templates

{{workflowRun.conclusion}} is typed String?. If it ever arrives null on a completed action, Mustache renders an empty string and the message becomes ℹ️ **Workflow :** [Build and test #42](…). A section guard, or a status computed property alongside emoji, fixes it cheaply.

Related polish: the raw value leaks GitHub's snake_case into user-facing text — **Workflow timed_out:**, **Job action_required:**. A small conclusion → display-text mapping ("timed out", "failed", "succeeded") would read much better in the conversation.

4. User-controlled text inside markdown link labels

[{{{workflowJob.name}}}]({{workflowJob.htmlUrl}})

Job and workflow names are author-controlled and can contain ] or ), which breaks the link (the triple stash means it is not escaped either). The existing templates deliberately avoid this — pull_request.opened.template uses a fixed label ([#{{number}}](url)) and puts the title outside the link, push.template uses [link](compare). Following that convention here would be both safer and more consistent:

{{emoji}} **Job {{conclusion}}:** {{{workflowJob.name}}} — [view run]({{workflowJob.htmlUrl}})

Not a security hole (Wire renders markdown, not HTML) — purely a rendering-correctness and consistency point.

---

📣 Notification volume

workflow_job.completed fires for every job, so a matrix build with 10 jobs plus the run summary is 11 messages per push — and the new help text in EventsHandler.kt:100-102 actively tells users to enable both. It is opt-in via GitHub's webhook event picker, so this is not a blocker, but I would consider either:

• notifying only on non-success job conclusions (the common pattern for CI chat integrations — you mostly want to hear about failures), or
• having the help text steer users to Workflow runs by default and mention Workflow jobs as the noisier per-job option.

Right now both the README and the help text present them as equally advisable.

---

🧪 Test coverage

Good coverage overall — fixture-based golden-file rendering plus an HTTP round trip is exactly the right shape. Gaps:

• does not render a workflow run/job before it completes does not test what the name says. The assertion passes purely because no workflow_run.in_progress.template exists; the .replace("\"conclusion\": \"success\"", "\"conclusion\": null") is a no-op for the outcome. Either rename it (e.g. does not render a workflow run for an unsupported action), or add a separate case that keeps action: completed with conclusion: null — which is the case that currently produces the malformed **Workflow :** output in point 3.
• No coverage for the optional-field branches. The {{#workflowName}} / {{#headBranch}} sections in workflow_job.completed.template are only exercised in the present case; a fixture with both absent would pin the fallback rendering and would also cover the nullability fix from point 1.
• eventFixture is duplicated in ApplicationTest.kt:334-335 and TemplateHandlerTest.kt:179-181. Worth a shared test helper, especially since the pre-existing src/test/fixtures/events/*.json files (push, issues, pull_request…) now have a loader and could back golden-file tests for those templates too.
• Minor inconsistency: the new ApplicationTest case uses application { configureRouting() } while every other test in the class uses module(). Intentional (to skip metrics), or drift?
• The new HTTP test asserts exactly = 2 sends but not the content. That is fine given TemplateHandlerTest covers rendering — just noting it so nobody mistakes it for end-to-end content coverage.

Also: File("src/test/fixtures/...") depends on the Gradle test working directory. It works today, but a resources-based loader (or at least a shared base-path constant) would be less brittle.

---

🔒 Security

Nothing concerning. The new events go through the same SignatureValidator gate before deserialization, no new endpoints or secrets are involved, and the added EventsHandler text introduces no new String.format arguments. The only user-controlled data reaching the message body is workflow / job / branch names, covered in point 4.

⚡ Performance

No concerns. DefaultMustacheFactory caches compiled templates, the models are small, and the only new cost is the extra message volume discussed above.

---

Summary

Blocking-ish: (1) the nullable workflow_run fields and (2) the divergent emoji maps. (3) and (4) are quick template fixes. The notification-volume and test-naming points are judgement calls — interested in your take. Nice work otherwise; the fixture-based golden files are a good pattern to have established here.

_{Note: I could not execute the Gradle test suite in this environment, so the findings above come from static reading of the diff and the surrounding code.}

[spoonman01]

@alexandreferris send a screenshot here in the PR about how it looks!