[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

The onboarding message was very specific to webhook usage, which is not the only purpose of Github App anymore

Solutions

Remove webhook help message from onboarding message

Before

After

[claude]

Code Review — chore: Remove WebHook generation from Onboarding message

Nice, focused change. The diff is two deleted lines in EventsHandler.onAppAddedToConversation, and the resulting message matches the "New Text" in the description (appendLine(WELCOME_TEXT) + appendLine() yields the intended blank line before the help hint). I found no functional bugs — notes below are mostly follow-ups.

What I like

This is a real security improvement, beyond the stated motivation. The old onboarding message printed the conversation's HMAC webhook secret (getOrCreateSecret is called inside webhookHelp) unprompted into the conversation the moment the app was added — so the secret landed in every member's message history, and in any conversation backup/export, with no user action. Now the secret is only disclosed when someone explicitly runs /github tokens or /github webhook help. Worth calling out in the PR description.

The lazy-secret side effect that disappears is safe. Removing the webhookHelp call also removes the implicit Redis write at onboarding, so a conversation can now exist with no stored secret. That path is already handled: SignatureValidator.isValid throws IOException on a missing secret and Routing.kt:97-108 turns it into a permanent 403 with a warning log, rather than a 500 GitHub would redeliver. And anyone actually wiring up a webhook must run the help/tokens command first, which creates the secret. So no regression here — but it was the one thing I'd have worried about, and it checks out.

Suggestions (non-blocking)

1. webhookHelp can now be private. GitHubCommandHandler.webhookHelp (GitHubCommandHandler.kt:44) was public only because EventsHandler called it. With that caller gone, the sole remaining non-test caller is response() itself, so it could be private to match its siblings tokens() and actionsHelp(). The only thing blocking it is the test at GitHubCommandHandlerTest.kt:109.

2. That test now covers a dead scenario. webhook setup generated for app onboarding is not counted as a help command exists to assert that the onboarding path doesn't inflate the onHelpCommand metric. There is no onboarding path anymore, so the name is misleading to the next reader and it pins webhookHelp open. I'd drop it (the /github webhook help test already asserts exactly = 1 on the metric) and tighten the visibility in the same commit. If you'd rather leave visibility alone, at least rename the test so it doesn't reference onboarding.

3. Test coverage for the message itself. EventsHandler has no unit tests — ApplicationTest.kt:89 only exercises the onAppAddedToConversation metric, not the handler. So nothing currently asserts the onboarding text, and nothing would have caught a regression that re-added secret material to it. Given the secret-disclosure angle, a small test asserting the welcome message contains the help hint and does not contain the stored secret would be cheap insurance. Testing EventsHandler directly is awkward today (it resolves GlobalContext and manager at construction), so this is probably a follow-up rather than a blocker for this PR.

Performance

No impact, other than one fewer Redis GET/SET round-trip per app-add — a marginal win.

Docs / process

README.md still documents /github webhook help as the way to get the URL and secret, which stays accurate, so no doc change is needed. Minor process note: the checklist item for a JIRA reference (e.g. SQPIT-###) in the title is unchecked — worth adding if your workflow requires it.

Verdict: LGTM. The suggestions above are cleanups, not blockers.

[bbaarriiss]

Hello @alexandreferris I will share my comments here as we talked 1-1.

At the moment, the first welcome message doesn't help but we need to write /github help to see the help even at the very beginning.

Since the help options are now more than the previous, I can understand why we don't have all content in the welcome message. I agree with it.
But when I write /github help I see the following:

When I went through the commands, I got confused, especially tokens command contains the tokens used in webhook and actions, so it caused some confusion to me. Because I can see these credentials where they are really needed (inside webhook and actions help commands)

---

👉🏼 Below I will write how it might look better from my point of view (So it is open for discussion. Cc: @spoonman01 @MarianKijewski )

1. Welcome message:

👋 Hi, I'm GitHub App. Thanks for adding me to the conversation.
You can use me to receive GitHub notifications in Wire.
I'm here to help make everyday work a little easier.

Available commands:
1. /github webhook help : Configure GitHub repository webhooks
2. /github actions help : Send custom messages from GitHub Actions
3. /github help : See the list of possible commands 

2. Content of /github webhook help:
   As is. It's good.

3. Content of /github actions help:
   Content needs improvement with some guidance added like we have in webhook help content. Also having the same look as it is in /github webhook help content will be nice for consistency.

4. Content of /github help:

Available commands:
1. /github webhook help : Configure GitHub repository webhooks
2. /github actions help : Send custom messages from GitHub Actions
3. /github help : See the list of possible commands 

[bbaarriiss]

I checked on staging after the changes and now looks more clear to me.
Thanks Alex.