From 75b3e3ec62c7f7900f21ad92ca926feb35e95898 Mon Sep 17 00:00:00 2001 From: Sergey Bykov Date: Wed, 30 Sep 2026 13:11:22 +0300 Subject: [PATCH 1/2] Link the merge request in Slack notifications - Add Project.MergeRequestLink: GitLab MR or GitHub PR URL and label from the review's external ID, empty unless it is a positive number - Show the MR link after the project tag so readers reach the MR in one click; the title still opens the review - Fix mrkdwn escaping: "&", "<" and ">" in the project, title, author or branches broke the review link - Cover link building, message text and notifySlack wiring with tests - Update the message format in docs/llm/Slack.md --- docs/llm/Slack.md | 11 +++++--- pkg/rest/rest.go | 3 +++ pkg/rest/rest_test.go | 22 ++++++++++++++++ pkg/reviewer/model.go | 20 ++++++++++++++ pkg/reviewer/model_test.go | 27 +++++++++++++++++++ pkg/slack/slack.go | 26 +++++++++++++----- pkg/slack/slack_test.go | 54 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 154 insertions(+), 9 deletions(-) create mode 100644 pkg/slack/slack_test.go diff --git a/docs/llm/Slack.md b/docs/llm/Slack.md index 545e97d..4390e8f 100644 --- a/docs/llm/Slack.md +++ b/docs/llm/Slack.md @@ -49,18 +49,23 @@ Webhook URL привязывается к каналу через VT-админ ```json { - "text": ":red_circle: ** by author (`source` → `target`) — 2 critical, 1 high, 3 medium, 0 low" + "text": ":red_circle: [project] ** by author (`source` → `target`) — 2 critical, 1 high, 3 medium, 0 low" } ``` +Ссылка на MR (``) выводится, только если `externalId` ревью — положительный номер MR, +а у проекта задан `vcsURL` (`reviewer.Project.MergeRequestLink`). Для GitLab — +`/-/merge_requests/` с меткой `MR !`, для GitHub — `/pull/` с меткой `PR #`. +Иначе ссылки нет, остальной текст не меняется. Символы `& < >` в проекте, заголовке, авторе и ветках экранируются. + Примеры: ``` -:red_circle: ** by john (`feature/pay` → `main`) — 2 critical, 1 high, 3 medium, 0 low +:red_circle: [group/app] ** by john (`feature/pay` → `main`) — 2 critical, 1 high, 3 medium, 0 low ``` ``` -:large_green_circle: ** by anna (`fix/typo` → `main`) — 0 critical, 0 high, 1 medium, 2 low +:large_green_circle: [group/app] ** by anna (`fix/typo` → `main`) — 0 critical, 0 high, 1 medium, 2 low ``` Маппинг `trafficLight` → emoji: diff --git a/pkg/rest/rest.go b/pkg/rest/rest.go index 896f656..14e2eae 100644 --- a/pkg/rest/rest.go +++ b/pkg/rest/rest.go @@ -103,6 +103,7 @@ func (h *Handler) notifySlack(project *reviewer.Project, rv *reviewer.Review) { stats.Low += rf.IssueStats.Low } + mrURL, mrLabel := project.MergeRequestLink(rv.ExternalID) notif := slack.ReviewNotification{ WebhookURL: project.SlackChannel.WebhookURL, ProjectTitle: project.Title, @@ -114,6 +115,8 @@ func (h *Handler) notifySlack(project *reviewer.Project, rv *reviewer.Review) { TrafficLight: rv.TrafficLight, IssueStats: stats, ReviewURL: fmt.Sprintf("%s/reviews/%d/", h.baseURL, rv.ID), + MRURL: mrURL, + MRLabel: mrLabel, } go h.notifier.Send(context.Background(), notif) diff --git a/pkg/rest/rest_test.go b/pkg/rest/rest_test.go index acd8539..49c8924 100644 --- a/pkg/rest/rest_test.go +++ b/pkg/rest/rest_test.go @@ -2,6 +2,7 @@ package rest import ( "context" + "encoding/json" "net/http" "net/http/httptest" "strconv" @@ -247,3 +248,24 @@ func TestNotifySlackSkipsMemberReviews(t *testing.T) { require.Eventually(t, func() bool { return posts.Load() == 1 }, 5*time.Second, 10*time.Millisecond) assert.Never(t, func() bool { return posts.Load() > 1 }, 200*time.Millisecond, 10*time.Millisecond, "a member review is not announced") } + +func TestNotifySlackLinksMergeRequest(t *testing.T) { + texts := make(chan string, 1) + webhook := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + var msg struct{ Text string } + _ = json.NewDecoder(r.Body).Decode(&msg) + texts <- msg.Text + })) + t.Cleanup(webhook.Close) + h := &Handler{notifier: slack.NewNotifier(embedlog.NewDevLogger()), baseURL: "http://localhost"} + project := &reviewer.Project{Project: db.Project{Title: "p", VcsURL: "https://git.example.com/group/app", SlackChannel: &db.SlackChannel{WebhookURL: webhook.URL}}} + + h.notifySlack(project, &reviewer.Review{Review: db.Review{ID: 1, ExternalID: "42", ReviewRole: reviewer.ReviewRoleSingle}}) + + select { + case text := <-texts: + assert.Contains(t, text, "") + case <-time.After(5 * time.Second): + t.Fatal("no slack notification sent") + } +} diff --git a/pkg/reviewer/model.go b/pkg/reviewer/model.go index 7e18187..a13b6a4 100644 --- a/pkg/reviewer/model.go +++ b/pkg/reviewer/model.go @@ -2,7 +2,10 @@ package reviewer import ( "errors" + "fmt" "slices" + "strconv" + "strings" "time" "reviewsrv/pkg/db" @@ -129,6 +132,23 @@ func (p *Project) HasSlackWebhook() bool { return p.SlackChannel != nil && p.SlackChannel.WebhookURL != "" } +// MergeRequestLink returns the VCS URL and label ("MR !42" on GitLab, "PR #42" on +// GitHub) of the merge request with the given external ID. Both are empty unless +// externalID is a positive MR number and the project has a VCS URL. +func (p *Project) MergeRequestLink(externalID string) (url, label string) { + iid, err := strconv.Atoi(externalID) + if err != nil || iid <= 0 || p.VcsURL == "" { + return "", "" + } + + base := strings.TrimRight(p.VcsURL, "/") + if strings.Contains(base, "github.com") { + return fmt.Sprintf("%s/pull/%d", base, iid), fmt.Sprintf("PR #%d", iid) + } + + return fmt.Sprintf("%s/-/merge_requests/%d", base, iid), fmt.Sprintf("MR !%d", iid) +} + type TaskTracker struct { db.TaskTracker } diff --git a/pkg/reviewer/model_test.go b/pkg/reviewer/model_test.go index dedf875..b4e16ae 100644 --- a/pkg/reviewer/model_test.go +++ b/pkg/reviewer/model_test.go @@ -267,6 +267,33 @@ func TestHasSlackWebhook(t *testing.T) { } } +func TestMergeRequestLink(t *testing.T) { + tests := []struct { + name string + vcsURL string + externalID string + wantURL string + wantLabel string + }{ + {"gitlab", "https://git.example.com/group/app", "42", "https://git.example.com/group/app/-/merge_requests/42", "MR !42"}, + {"github", "https://github.com/org/app", "7", "https://github.com/org/app/pull/7", "PR #7"}, + {"trailing slash", "https://git.example.com/group/app/", "42", "https://git.example.com/group/app/-/merge_requests/42", "MR !42"}, + {"empty id", "https://git.example.com/group/app", "", "", ""}, + {"zero id", "https://git.example.com/group/app", "0", "", ""}, + {"not a number", "https://git.example.com/group/app", "%EXTERNAL_ID%", "", ""}, + {"no vcs url", "", "42", "", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := &Project{db.Project{VcsURL: tt.vcsURL}} + url, label := p.MergeRequestLink(tt.externalID) + assert.Equal(t, tt.wantURL, url) + assert.Equal(t, tt.wantLabel, label) + }) + } +} + func TestReviewSearch_ToDB(t *testing.T) { t.Run("nil receiver", func(t *testing.T) { var s *ReviewSearch diff --git a/pkg/slack/slack.go b/pkg/slack/slack.go index 944a24f..af0ed06 100644 --- a/pkg/slack/slack.go +++ b/pkg/slack/slack.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "net/http" + "strings" "time" "github.com/vmkteam/embedlog" @@ -46,6 +47,9 @@ type ReviewNotification struct { TrafficLight string IssueStats IssueStats ReviewURL string + // MRURL and MRLabel are empty when the review has no merge request. + MRURL string + MRLabel string } func trafficLightEmoji(tl string) string { @@ -61,15 +65,25 @@ func trafficLightEmoji(tl string) string { } } +// escaper escapes the control characters of Slack mrkdwn, so a "<" or ">" in a +// title does not break the surrounding link. +var escaper = strings.NewReplacer("&", "&", "<", "<", ">", ">") + func (n ReviewNotification) text() string { - return fmt.Sprintf("%s [%s] *<%s|%s>* by %s (`%s` → `%s`) — %d critical, %d high, %d medium, %d low", + var mr string + if n.MRURL != "" { + mr = fmt.Sprintf("<%s|%s> ", n.MRURL, escaper.Replace(n.MRLabel)) + } + + return fmt.Sprintf("%s [%s] %s*<%s|%s>* by %s (`%s` → `%s`) — %d critical, %d high, %d medium, %d low", trafficLightEmoji(n.TrafficLight), - n.ProjectTitle, + escaper.Replace(n.ProjectTitle), + mr, n.ReviewURL, - n.Title, - n.Author, - n.SourceBranch, - n.TargetBranch, + escaper.Replace(n.Title), + escaper.Replace(n.Author), + escaper.Replace(n.SourceBranch), + escaper.Replace(n.TargetBranch), n.IssueStats.Critical, n.IssueStats.High, n.IssueStats.Medium, diff --git a/pkg/slack/slack_test.go b/pkg/slack/slack_test.go new file mode 100644 index 0000000..dd27342 --- /dev/null +++ b/pkg/slack/slack_test.go @@ -0,0 +1,54 @@ +package slack + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestReviewNotificationText(t *testing.T) { + base := ReviewNotification{ + ProjectTitle: "group/app", + Title: "ABC-1 Add upload", + Author: "john", + SourceBranch: "ABC-1", + TargetBranch: "main", + TrafficLight: "yellow", + IssueStats: IssueStats{Medium: 7, Low: 11}, + ReviewURL: "https://reviewer.example.com/reviews/5/", + } + + tests := []struct { + name string + edit func(*ReviewNotification) + want string + }{ + { + name: "without merge request", + edit: func(*ReviewNotification) {}, + want: ":large_yellow_circle: [group/app] ** by john (`ABC-1` → `main`) — 0 critical, 0 high, 7 medium, 11 low", + }, + { + name: "with merge request", + edit: func(n *ReviewNotification) { + n.MRURL, n.MRLabel = "https://git.example.com/group/app/-/merge_requests/42", "MR !42" + }, + want: ":large_yellow_circle: [group/app] ** by john (`ABC-1` → `main`) — 0 critical, 0 high, 7 medium, 11 low", + }, + { + name: "mrkdwn control characters are escaped", + edit: func(n *ReviewNotification) { + n.Title, n.Author, n.SourceBranch = "Use & friends", "a>b", "x* by a>b (`x<y` → `main`) — 0 critical, 0 high, 7 medium, 11 low", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + n := base + tt.edit(&n) + assert.Equal(t, tt.want, n.text()) + }) + } +} From b059e1ded9db58f1a739b567042d555093add210 Mon Sep 17 00:00:00 2001 From: Sergey Bykov Date: Wed, 30 Sep 2026 13:19:15 +0300 Subject: [PATCH 2/2] Publish the Docker image from the release workflow - Fix Docker publishing: a release created by GoReleaser with GITHUB_TOKEN never triggered docker.yml, so images needed a manual dispatch - Call docker.yml from go.yml after the release job, so images are built only for tags that passed tests and GoReleaser - Tag release images as latest too; manual rebuilds keep latest as is --- .github/workflows/docker.yml | 17 ++++++++++++----- .github/workflows/go.yml | 10 ++++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 0e26a61..d02b58a 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -1,8 +1,14 @@ name: Publish Docker image +# Called by go.yml after GoReleaser: a release it publishes with GITHUB_TOKEN +# does not trigger workflows, so `on: release` never fired. Manual dispatch +# rebuilds a tag without moving `latest`. on: - release: - types: [published] + workflow_call: + inputs: + tag: + type: string + required: true workflow_dispatch: inputs: tag: @@ -20,7 +26,7 @@ jobs: - name: Check out the repo uses: actions/checkout@v5 with: - ref: ${{ inputs.tag || github.ref }} + ref: ${{ inputs.tag }} - name: Log in to Docker Hub uses: docker/login-action@v3 @@ -43,7 +49,8 @@ jobs: vmkteam/reviewer ghcr.io/${{ github.repository }} tags: | - type=raw,value=${{ inputs.tag }},enable=${{ github.event_name == 'workflow_dispatch' }} + type=raw,value=${{ inputs.tag }} + type=raw,value=latest,enable=${{ github.event_name != 'workflow_dispatch' }} - name: Build and push Docker images uses: docker/build-push-action@v6 @@ -53,4 +60,4 @@ jobs: tags: ${{ steps.meta.outputs.tags }} labels: ${{ steps.meta.outputs.labels }} build-args: | - VERSION=${{ inputs.tag || github.ref_name }} + VERSION=${{ inputs.tag }} diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index c5da2c9..c2c27a7 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -58,3 +58,13 @@ jobs: args: release --clean env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + docker: + needs: release + permissions: + contents: read + packages: write + uses: ./.github/workflows/docker.yml + with: + tag: ${{ github.ref_name }} + secrets: inherit