Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions internal/discord/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,5 +48,25 @@ func getCommands() []*discordgo.ApplicationCommand {
},
},
},
{
Name: "changelog",
Description: "View changes between two versions",
Options: []*discordgo.ApplicationCommandOption{
{
Type: discordgo.ApplicationCommandOptionString,
Name: "base",
Description: "The base version (e.g. v2.6.0)",
Required: true,
Autocomplete: true,
},
{
Type: discordgo.ApplicationCommandOptionString,
Name: "head",
Description: "The head version (e.g. v2.6.4)",
Required: true,
Autocomplete: true,
},
},
},
}
}
170 changes: 170 additions & 0 deletions internal/discord/handlers/changelog_handler.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
package handlers

import (
"fmt"
"log"
"strings"
"sync"
"time"

"github.com/bwmarrin/discordgo"
gogithub "github.com/google/go-github/v57/github"
)

var (
releaseCache []*gogithub.RepositoryRelease
releaseCacheMutex sync.RWMutex
lastCacheUpdate time.Time
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider adding a comment explaining the cache invalidation strategy. The double-checked locking pattern is correctly implemented, but it would be helpful to document why a 5-minute cache duration was chosen and what tradeoffs it represents (freshness vs. API rate limits).

Suggested change
lastCacheUpdate time.Time
lastCacheUpdate time.Time
// cacheDuration determines how long release data is cached before refreshing.
// A 5-minute duration balances data freshness with API rate limits: frequent updates
// could exceed GitHub's rate limits, while longer durations may serve stale data.

Copilot uses AI. Check for mistakes.
cacheDuration = 5 * time.Minute
)

func handleChangelog(s *discordgo.Session, i *discordgo.InteractionCreate) {
options := i.ApplicationCommandData().Options
optionMap := make(map[string]*discordgo.ApplicationCommandInteractionDataOption, len(options))
for _, opt := range options {
optionMap[opt.Name] = opt
}

var base, head string
if opt, ok := optionMap["base"]; ok {
base = opt.StringValue()
}
if opt, ok := optionMap["head"]; ok {
head = opt.StringValue()
}

if base == "" || head == "" {
s.InteractionRespond(i.Interaction, &discordgo.InteractionResponse{
Type: discordgo.InteractionResponseChannelMessageWithSource,
Data: &discordgo.InteractionResponseData{
Content: "Please provide both base and head versions.",
Flags: discordgo.MessageFlagsEphemeral,
},
})
return
}

// Defer response as API call might take time
s.InteractionRespond(i.Interaction, &discordgo.InteractionResponse{
Type: discordgo.InteractionResponseDeferredChannelMessageWithSource,
})

comparison, err := GithubClient.CompareCommits(GithubOwner, GithubRepo, base, head)
if err != nil {
log.Printf("Error comparing commits: %v", err)
s.InteractionResponseEdit(i.Interaction, &discordgo.WebhookEdit{
Content: &[]string{fmt.Sprintf("Failed to compare versions: %s...%s", base, head)}[0],
})
return
}

// Format the output
message := formatChangelogMessage(base, head, comparison)

s.InteractionResponseEdit(i.Interaction, &discordgo.WebhookEdit{
Content: &message,
})
}
Comment on lines 21 to 68
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test coverage for handleChangelog function. The handler orchestrates validation, deferred response, GitHub API calls, and error handling, but none of these behaviors are tested. Consider adding tests for:

  • Missing base/head version validation
  • Successful changelog retrieval and formatting
  • Error handling when GitHub API fails
  • Interaction response behavior

Copilot uses AI. Check for mistakes.

func formatChangelogMessage(base, head string, comparison *gogithub.CommitsComparison) string {
var sb strings.Builder
sb.WriteString(fmt.Sprintf("## Changes from %s to %s\n", base, head))
sb.WriteString(fmt.Sprintf("Total commits: %d\n\n", comparison.GetTotalCommits()))

// List commits (limit to last 10 to avoid hitting message length limits)
commits := comparison.Commits
if len(commits) > 10 {
sb.WriteString(fmt.Sprintf("*Showing last 10 of %d commits*\n\n", len(commits)))
commits = commits[len(commits)-10:]
}

for _, commit := range commits {
message := commit.GetCommit().GetMessage()
// Take only the first line of the commit message
if idx := strings.Index(message, "\n"); idx != -1 {
message = message[:idx]
}

author := commit.GetAuthor().GetLogin()
if author == "" {
author = commit.GetCommit().GetAuthor().GetName()
}

sb.WriteString(fmt.Sprintf("- [`%s`](<%s>) %s - *%s*\n",
commit.GetSHA()[:7],
commit.GetHTMLURL(),
message,
author,
))
}

sb.WriteString(fmt.Sprintf("\n[View Full Comparison](<%s>)", comparison.GetHTMLURL()))
return sb.String()
}

func handleChangelogAutocomplete(s *discordgo.Session, i *discordgo.InteractionCreate) {
// Update cache if needed
if err := updateReleaseCache(); err != nil {
log.Printf("Error updating release cache: %v", err)
}
Comment on lines +115 to +119
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The autocomplete handler handleChangelogAutocomplete lacks test coverage. Consider adding tests to verify: 1) filtering releases based on user input, 2) limiting results to 25 choices, and 3) handling empty or failed cache updates.

Copilot uses AI. Check for mistakes.

releaseCacheMutex.RLock()
defer releaseCacheMutex.RUnlock()

data := i.ApplicationCommandData()
var currentInput string
for _, opt := range data.Options {
if opt.Focused {
currentInput = strings.ToLower(opt.StringValue())
break
}
}

choices := make([]*discordgo.ApplicationCommandOptionChoice, 0, 25)
for _, release := range releaseCache {
tagName := release.GetTagName()
if currentInput == "" || strings.Contains(strings.ToLower(tagName), currentInput) {
choices = append(choices, &discordgo.ApplicationCommandOptionChoice{
Name: tagName,
Value: tagName,
})
}
if len(choices) >= 25 {
break
}
}

s.InteractionRespond(i.Interaction, &discordgo.InteractionResponse{
Type: discordgo.InteractionApplicationCommandAutocompleteResult,
Data: &discordgo.InteractionResponseData{
Choices: choices,
},
})
}
Comment on lines +115 to +153
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test coverage for handleChangelogAutocomplete function. This function handles cache updates, filtering, and Discord response construction but has no tests. Consider adding tests for:

  • Cache update behavior
  • Filtering releases by user input
  • Limiting to 25 choices
  • Empty cache handling
  • Error handling when cache update fails

Copilot uses AI. Check for mistakes.

func updateReleaseCache() error {
releaseCacheMutex.RLock()
if time.Since(lastCacheUpdate) < cacheDuration && len(releaseCache) > 0 {
releaseCacheMutex.RUnlock()
return nil
}
releaseCacheMutex.RUnlock()

releaseCacheMutex.Lock()
defer releaseCacheMutex.Unlock()

// Double check after acquiring write lock
if time.Since(lastCacheUpdate) < cacheDuration && len(releaseCache) > 0 {
return nil
}
Comment on lines +155 to +169
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cache update logic with double-checked locking lacks test coverage. Consider adding tests to verify: 1) concurrent access doesn't cause race conditions, 2) cache expiration works correctly, and 3) the double-check mechanism prevents redundant API calls.

Copilot uses AI. Check for mistakes.

// Fetch releases
releases, err := GithubClient.GetReleases(GithubOwner, GithubRepo, 100)
if err != nil {
return err
}

releaseCache = releases
lastCacheUpdate = time.Now()
return nil
}
Comment on lines +155 to +180
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test coverage for updateReleaseCache function. This function implements double-checked locking pattern for cache management, which is complex and error-prone. The concurrency behavior, cache expiration, and error handling should be tested to ensure correctness. Consider adding tests for:

  • Cache expiration after duration
  • Concurrent access patterns
  • Error handling when GitHub API fails
  • Cache initialization from empty state

Copilot uses AI. Check for mistakes.
121 changes: 121 additions & 0 deletions internal/discord/handlers/changelog_handler_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
package handlers

import (
"strings"
"testing"

gogithub "github.com/google/go-github/v57/github"
)

func TestFormatChangelogMessage(t *testing.T) {
strPtr := func(s string) *string { return &s }
intPtr := func(i int) *int { return &i }

tests := []struct {
name string
base string
head string
comparison *gogithub.CommitsComparison
want []string // Substrings that should be present
dontWant []string // Substrings that should NOT be present
}{
{
name: "basic comparison",
base: "v1.0.0",
head: "v1.1.0",
comparison: &gogithub.CommitsComparison{
TotalCommits: intPtr(2),
HTMLURL: strPtr("https://github.com/org/repo/compare/v1.0.0...v1.1.0"),
Commits: []*gogithub.RepositoryCommit{
{
SHA: strPtr("abcdef123456"),
HTMLURL: strPtr("https://github.com/org/repo/commit/abcdef1"),
Commit: &gogithub.Commit{
Message: strPtr("feat: cool feature"),
Author: &gogithub.CommitAuthor{
Name: strPtr("John Doe"),
},
},
Author: &gogithub.User{
Login: strPtr("johndoe"),
},
},
{
SHA: strPtr("123456abcdef"),
HTMLURL: strPtr("https://github.com/org/repo/commit/123456a"),
Commit: &gogithub.Commit{
Message: strPtr("fix: nasty bug\n\nSome details"),
Author: &gogithub.CommitAuthor{
Name: strPtr("Jane Smith"),
},
},
Author: &gogithub.User{
Login: strPtr("janesmith"),
},
},
},
},
want: []string{
"## Changes from v1.0.0 to v1.1.0",
"Total commits: 2",
"[`abcdef1`](<https://github.com/org/repo/commit/abcdef1>)",
"feat: cool feature",
"johndoe",
"[`123456a`](<https://github.com/org/repo/commit/123456a>)",
"fix: nasty bug",
"janesmith",
"[View Full Comparison](<https://github.com/org/repo/compare/v1.0.0...v1.1.0>)",
},
dontWant: []string{
"Some details",
"Showing last 10",
},
},
{
name: "many commits truncated",
base: "v1.0.0",
head: "v1.1.0",
comparison: &gogithub.CommitsComparison{
TotalCommits: intPtr(15),
HTMLURL: strPtr("https://github.com/compare"),
Commits: func() []*gogithub.RepositoryCommit {
commits := make([]*gogithub.RepositoryCommit, 15)
for i := 0; i < 15; i++ {
commits[i] = &gogithub.RepositoryCommit{
SHA: strPtr("longhashvalue"),
HTMLURL: strPtr("url"),
Commit: &gogithub.Commit{
Message: strPtr("msg"),
Author: &gogithub.CommitAuthor{Name: strPtr("author")},
},
Author: &gogithub.User{Login: strPtr("user")},
}
}
return commits
}(),
},
want: []string{
"Total commits: 15",
"*Showing last 10 of 15 commits*",
},
},
Comment on lines +74 to +101
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a test case with a SHA shorter than 7 characters to ensure the code handles this edge case gracefully (currently it would panic at line 95 of changelog_handler.go).

Copilot uses AI. Check for mistakes.
Comment on lines +74 to +101
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a test case that covers the scenario where commit.GetAuthor() returns nil or where commit.GetCommit().GetAuthor() returns nil. This would help catch the nil pointer dereference issue at line 91 and ensure the fallback to "Unknown" works correctly.

Copilot uses AI. Check for mistakes.
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := formatChangelogMessage(tt.base, tt.head, tt.comparison)

for _, w := range tt.want {
if !strings.Contains(got, w) {
t.Errorf("formatChangelogMessage() missing %q\nGot:\n%s", w, got)
}
}

for _, dw := range tt.dontWant {
if strings.Contains(got, dw) {
t.Errorf("formatChangelogMessage() unexpectedly contains %q", dw)
}
}
})
}
}
Comment on lines +10 to +121
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing edge case handling in tests for formatChangelogMessage. The test should verify behavior when:

  • comparison.Commits is nil (potential panic)
  • commit.SHA is nil or shorter than 7 characters (would panic on line 94)
  • commit.Author is nil (handled but not tested)
  • commit.Commit.Author is nil (potential panic when falling back)

Consider adding test cases for these nil/edge case scenarios to ensure robustness.

Copilot uses AI. Check for mistakes.
14 changes: 9 additions & 5 deletions internal/discord/handlers/interaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,11 @@ type ModalState struct {
var modalStates = make(map[string]*ModalState)

var commandHandlers = map[string]func(s *discordgo.Session, i *discordgo.InteractionCreate){
"tapsign": handleTapsign,
"feature": handleFeature,
"faq": handleFaq,
"bug": handleBug,
"tapsign": handleTapsign,
"feature": handleFeature,
"faq": handleFaq,
"bug": handleBug,
"changelog": handleChangelog,
}

// HandleInteraction routes interactions to appropriate handlers
Expand All @@ -60,7 +61,8 @@ func handleTapsign(s *discordgo.Session, i *discordgo.InteractionCreate) {
helpText := "**How to get help or make a suggestion:**\n" +
"`/bug`: To report a bug with the app.\n" +
"`/feature`: To request a new feature. \n" +
"`/faq`: Frequently Asked Questions.\n"
"`/faq`: Frequently Asked Questions.\n" +
"`/changelog`: View changes between two versions.\n"

s.InteractionRespond(i.Interaction, &discordgo.InteractionResponse{
Type: discordgo.InteractionResponseChannelMessageWithSource,
Expand All @@ -77,5 +79,7 @@ func handleAutocomplete(s *discordgo.Session, i *discordgo.InteractionCreate) {
switch data.Name {
case "faq":
handleFaqAutocomplete(s, i)
case "changelog":
handleChangelogAutocomplete(s, i)
}
}
19 changes: 19 additions & 0 deletions internal/github/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,25 @@ func NewClient(token string) *Client {
}
}

func (c *Client) GetReleases(owner, repo string, limit int) ([]*github.RepositoryRelease, error) {
opts := &github.ListOptions{
PerPage: limit,
}
releases, _, err := c.client.Repositories.ListReleases(c.ctx, owner, repo, opts)
if err != nil {
return nil, fmt.Errorf("failed to list releases: %w", err)
}
return releases, nil
}
Comment on lines +43 to +52
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent error handling compared to other methods in this file. The CreateIssue method (lines 77-82) checks the response object for status codes to provide more detailed error messages, but GetReleases doesn't follow this pattern. Consider updating to match:

releases, resp, err := c.client.Repositories.ListReleases(c.ctx, owner, repo, opts)
if err != nil {
    if resp != nil {
        return nil, fmt.Errorf("github API returned %d: failed to list releases: %w", resp.StatusCode, err)
    }
    return nil, fmt.Errorf("failed to list releases: %w", err)
}

Copilot uses AI. Check for mistakes.

func (c *Client) CompareCommits(owner, repo, base, head string) (*github.CommitsComparison, error) {
comparison, _, err := c.client.Repositories.CompareCommits(c.ctx, owner, repo, base, head, nil)
if err != nil {
return nil, fmt.Errorf("failed to compare commits: %w", err)
}
return comparison, nil
}
Comment on lines +54 to +60
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent error handling compared to other methods in this file. The CreateIssue method (lines 77-82) checks the response object for status codes to provide more detailed error messages, but CompareCommits doesn't follow this pattern. Consider updating to match:

comparison, resp, err := c.client.Repositories.CompareCommits(c.ctx, owner, repo, base, head, nil)
if err != nil {
    if resp != nil {
        return nil, fmt.Errorf("github API returned %d: failed to compare commits: %w", resp.StatusCode, err)
    }
    return nil, fmt.Errorf("failed to compare commits: %w", err)
}

Copilot uses AI. Check for mistakes.

func (c *Client) CreateIssue(owner, repo, title, body string, labels []string) (*IssueResponse, error) {
log.Printf("[GitHub API] Creating issue in %s/%s", owner, repo)
log.Printf("[GitHub API] Title: %s", title)
Expand Down