Skip to content

GitHub App comment author is derived from App name instead of slug #290

Description

@vrivellino

Summary

When argo-diff is configured as a GitHub App, getCommentUser() derives the bot login from the
App's display name instead of its slug. If the App name is not already slug-shaped, the
computed login never matches the real comment author, and argo-diff stops recognizing its own
previous comments.

Where

internal/github/comment.go:150

commentLogin = *app.Name + "[bot]"

Why it's wrong

GitHub builds a bot's login from the App slug — the URL-friendly form of the name (lowercased,
spaces replaced with hyphens) — not from the display name.

App display name Real comment author Value computed today
argo-diff argo-diff[bot] argo-diff[bot]
Argo Diff argo-diff[bot] Argo Diff[bot]
ArgoDiff Prod argodiff-prod[bot] ArgoDiff Prod[bot]

go-github already exposes the correct field — App.Slug alongside App.Name
(github/apps.go:22).

Impact

getExistingComments() filters candidate comments with:

if isGithubAction || *c.User.Login == commentLogin {

(internal/github/comment.go:279)

When commentLogin is wrong, that comparison never matches. The consequences:

  • argo-diff never finds its previous comment on a PR.
  • Every event creates a new full-size diff comment instead of editing the existing one.
  • The [Outdated argo-diff content] cleanup path never runs, so stale comments accumulate.

The symptom is duplicate comments on every push to a PR.

This is latent rather than universal because README.md recommends naming the App argo-diff,
which is already slug-shaped. Any other name silently breaks comment reuse. Nothing in the setup
docs warns that the name matters.

Suggested fix

Use the slug, and keep the name only as a fallback:

if app == nil || (app.Slug == nil && app.Name == nil) {
	log.Error().Msg("Empty app returned - not sure how I got here")
	return fmt.Errorf("empty app info")
}
appLogin := app.GetSlug()
if appLogin == "" {
	log.Warn().Msg("Github App slug is empty - falling back to app name")
	appLogin = app.GetName()
}
mux.Lock()
commentLogin = appLogin + "[bot]"
mux.Unlock()

Note the nil check at internal/github/comment.go:145 also needs updating, since it currently
guards only app.Name.

Test coverage gap

There is no test for the GitHub App identity path. internal/github/github_testdata/ has
payload-user.json for the PAT path but no /app fixture, and no test exercises
commentClientIsApp = true. A fixture where slug and name differ would pin this behavior down.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions