diff --git a/go.mod b/go.mod index d162a23..61a7a94 100644 --- a/go.mod +++ b/go.mod @@ -6,12 +6,13 @@ require ( github.com/codeGROOVE-dev/ds9 v0.6.0 github.com/codeGROOVE-dev/gh-mailto v0.0.0-20251024133418-149270eb16a9 github.com/codeGROOVE-dev/gsm v0.0.0-20251019065141-833fe2363d22 - github.com/codeGROOVE-dev/prx v0.0.0-20251028202628-9f237ee71356 + github.com/codeGROOVE-dev/prx v0.0.0-20251030022101-ff906928a1e4 github.com/codeGROOVE-dev/retry v1.3.0 github.com/codeGROOVE-dev/sprinkler v0.0.0-20251029020504-57f2ea3ae37a - github.com/codeGROOVE-dev/turnclient v0.0.0-20251028130307-1f85c9aa43c4 + github.com/codeGROOVE-dev/turnclient v0.0.0-20251030022425-bc3b14acf75e github.com/golang-jwt/jwt/v5 v5.3.0 github.com/google/go-github/v50 v50.2.0 + github.com/google/uuid v1.6.0 github.com/gorilla/mux v1.8.1 github.com/shurcooL/githubv4 v0.0.0-20240727222349-48295856cce7 github.com/slack-go/slack v0.17.3 @@ -26,7 +27,6 @@ require ( github.com/cloudflare/circl v1.6.1 // indirect github.com/google/go-cmp v0.7.0 // indirect github.com/google/go-querystring v1.1.0 // indirect - github.com/google/uuid v1.6.0 // indirect github.com/gorilla/websocket v1.5.3 // indirect github.com/kr/pretty v0.3.1 // indirect github.com/rogpeppe/go-internal v1.14.1 // indirect diff --git a/go.sum b/go.sum index 67aa2b6..68f660f 100644 --- a/go.sum +++ b/go.sum @@ -8,14 +8,14 @@ github.com/codeGROOVE-dev/gh-mailto v0.0.0-20251024133418-149270eb16a9 h1:eyWcEZ github.com/codeGROOVE-dev/gh-mailto v0.0.0-20251024133418-149270eb16a9/go.mod h1:4Hr2ySB8dcpeZqZq/7UbXdEJ/5RK9coYGHvW90ZfieE= github.com/codeGROOVE-dev/gsm v0.0.0-20251019065141-833fe2363d22 h1:gtN3rOc6YspO646BkcOxBhPjEqKUz+jl175jIqglfDg= github.com/codeGROOVE-dev/gsm v0.0.0-20251019065141-833fe2363d22/go.mod h1:KV+w19ubP32PxZPE1hOtlCpTaNpF0Bpb32w5djO8UTg= -github.com/codeGROOVE-dev/prx v0.0.0-20251028202628-9f237ee71356 h1:lHoHnylLAp7/7BMhdiTh9Z2+p4ATcQ7aFcgqxOFGzE4= -github.com/codeGROOVE-dev/prx v0.0.0-20251028202628-9f237ee71356/go.mod h1:FEy3gz9IYDXWnKWkoDSL+pWu6rujxbBSrF4w5A8QSK0= +github.com/codeGROOVE-dev/prx v0.0.0-20251030022101-ff906928a1e4 h1:DSuoUwP3oyR4cHrX0cUh9c7CtYjXNIcyCmqpIwHilIU= +github.com/codeGROOVE-dev/prx v0.0.0-20251030022101-ff906928a1e4/go.mod h1:FEy3gz9IYDXWnKWkoDSL+pWu6rujxbBSrF4w5A8QSK0= github.com/codeGROOVE-dev/retry v1.3.0 h1:/+ipAWRJLL6y1R1vprYo0FSjSBvH6fE5j9LKXjpD54g= github.com/codeGROOVE-dev/retry v1.3.0/go.mod h1:8OgefgV1XP7lzX2PdKlCXILsYKuz6b4ZpHa/20iLi8E= github.com/codeGROOVE-dev/sprinkler v0.0.0-20251029020504-57f2ea3ae37a h1:iLjcbNpCKXT8ZAlGGesh0x9g/ntgG5OCuvM2QJ9+27E= github.com/codeGROOVE-dev/sprinkler v0.0.0-20251029020504-57f2ea3ae37a/go.mod h1:/kd3ncsRNldD0MUpbtp5ojIzfCkyeXB7JdOrpuqG7Gg= -github.com/codeGROOVE-dev/turnclient v0.0.0-20251028130307-1f85c9aa43c4 h1:si9tMEo5SXpDuDXGkJ1zNnnpP8TbmakrkNujAbpKlqA= -github.com/codeGROOVE-dev/turnclient v0.0.0-20251028130307-1f85c9aa43c4/go.mod h1:bFWMd0JeaJY0kSIO5AcRQdJLXF3Fo3eKclE49vmIZes= +github.com/codeGROOVE-dev/turnclient v0.0.0-20251030022425-bc3b14acf75e h1:WXHdC8o5KmP5CwkQRiGVywYzsj93fjkRPq7clhfZPq0= +github.com/codeGROOVE-dev/turnclient v0.0.0-20251030022425-bc3b14acf75e/go.mod h1:dVS3MlJDgL6WkfurJAyS7I9Fe1yxxoxxarjVifY5bIo= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= diff --git a/pkg/notify/interfaces.go b/pkg/notify/interfaces.go index 5abccc0..628e505 100644 --- a/pkg/notify/interfaces.go +++ b/pkg/notify/interfaces.go @@ -25,6 +25,7 @@ type SlackClient interface { // DM operations SendDirectMessage(ctx context.Context, userID, text string) (dmChannelID, messageTS string, err error) + UpdateDMMessage(ctx context.Context, userID, prURL, newText string) error HasRecentDMAboutPR(ctx context.Context, userID, prURL string) (bool, error) SaveDMMessageInfo(ctx context.Context, userID, prURL, dmChannelID, messageTS, message string) error diff --git a/pkg/notify/notify.go b/pkg/notify/notify.go index 9af178b..0bcc200 100644 --- a/pkg/notify/notify.go +++ b/pkg/notify/notify.go @@ -4,12 +4,14 @@ package notify import ( "context" "encoding/json" + "errors" "fmt" "log/slog" "sort" "strings" "time" + "github.com/codeGROOVE-dev/slacker/pkg/slack" "github.com/codeGROOVE-dev/slacker/pkg/state" "github.com/codeGROOVE-dev/turnclient/pkg/turn" "github.com/google/uuid" @@ -881,6 +883,33 @@ func (m *Manager) NotifyUser(ctx context.Context, workspaceID, userID, channelID "action_required", action, "message", message) + // Try to update existing DM first (if one exists in our state store) + // UpdateDMMessage returns ErrNoDMToUpdate if no DM exists + updateErr := slackClient.UpdateDMMessage(ctx, userID, pr.HTMLURL, message) + if updateErr == nil { + // Successfully updated existing DM + slog.Info("successfully updated existing DM with new PR state", + "user", userID, + "pr", fmt.Sprintf("%s/%s#%d", pr.Owner, pr.Repo, pr.Number), + "pr_state", pr.State, + "action_required", action) + return nil + } + + // Check if it's because no DM exists (expected case for first notification) + if !errors.Is(updateErr, slack.ErrNoDMToUpdate) { + // Actual error occurred during update - log warning but continue to send new DM + slog.Warn("failed to update existing DM, will send new one", + "user", userID, + "pr", fmt.Sprintf("%s/%s#%d", pr.Owner, pr.Repo, pr.Number), + "error", updateErr) + } + + slog.Debug("no existing DM to update, will check for recent DMs and potentially send new one", + "user", userID, + "pr", fmt.Sprintf("%s/%s#%d", pr.Owner, pr.Repo, pr.Number), + "reason", "DM not in state store or too old") + // Check if we recently sent a DM about this PR (prevents duplicates during rolling deployments) hasRecent, err := slackClient.HasRecentDMAboutPR(ctx, userID, pr.HTMLURL) if err != nil { diff --git a/pkg/notify/notify_user_test.go b/pkg/notify/notify_user_test.go index 48438f2..59538b9 100644 --- a/pkg/notify/notify_user_test.go +++ b/pkg/notify/notify_user_test.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "github.com/codeGROOVE-dev/slacker/pkg/slack" "github.com/codeGROOVE-dev/slacker/pkg/state" slackapi "github.com/slack-go/slack" ) @@ -16,6 +17,7 @@ type mockSlackClient struct { isUserInChannelFunc func(ctx context.Context, channelID, userID string) bool userTimezoneFunc func(ctx context.Context, userID string) (string, error) sendDirectMessageFunc func(ctx context.Context, userID, text string) (dmChannelID, messageTS string, err error) + updateDMMessageFunc func(ctx context.Context, userID, prURL, newText string) error hasRecentDMAboutPRFunc func(ctx context.Context, userID, prURL string) (bool, error) saveDMMessageInfoFunc func(ctx context.Context, userID, prURL, dmChannelID, messageTS, message string) error apiFunc func() *slackapi.Client @@ -49,6 +51,14 @@ func (m *mockSlackClient) SendDirectMessage(ctx context.Context, userID, text st return "D123", "1234567890.123456", nil } +func (m *mockSlackClient) UpdateDMMessage(ctx context.Context, userID, prURL, newText string) error { + if m.updateDMMessageFunc != nil { + return m.updateDMMessageFunc(ctx, userID, prURL, newText) + } + // Default: return ErrNoDMToUpdate (no DM exists to update) + return slack.ErrNoDMToUpdate +} + func (m *mockSlackClient) HasRecentDMAboutPR(ctx context.Context, userID, prURL string) (bool, error) { if m.hasRecentDMAboutPRFunc != nil { return m.hasRecentDMAboutPRFunc(ctx, userID, prURL) diff --git a/pkg/slack/client_additional_test.go b/pkg/slack/client_additional_test.go index e03d47c..0633d41 100644 --- a/pkg/slack/client_additional_test.go +++ b/pkg/slack/client_additional_test.go @@ -18,9 +18,9 @@ func TestUpdateDMMessage(t *testing.T) { prURL := "https://github.com/test/repo/pull/123" err := client.UpdateDMMessage(ctx, "U001", prURL, "New text") - // Should not error when state store is nil (graceful degradation) - if err != nil { - t.Fatalf("unexpected error: %v", err) + // Should return ErrNoDMToUpdate when state store is nil + if !errors.Is(err, ErrNoDMToUpdate) { + t.Fatalf("expected ErrNoDMToUpdate, got: %v", err) } }) } diff --git a/pkg/slack/slack.go b/pkg/slack/slack.go index 99ecd29..d086c06 100644 --- a/pkg/slack/slack.go +++ b/pkg/slack/slack.go @@ -8,6 +8,7 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" + "errors" "fmt" "io" "log/slog" @@ -24,6 +25,12 @@ import ( "github.com/slack-go/slack/slackevents" ) +// Errors +var ( + // ErrNoDMToUpdate indicates no DM exists to update. + ErrNoDMToUpdate = errors.New("no DM found to update") +) + // Constants for input validation. const ( maxCommandInputLength = 200 @@ -460,9 +467,14 @@ func (c *Client) SendDirectMessage(ctx context.Context, userID, text string) (dm var msgTS string // Then send message with retry + // Disable unfurling for GitHub links in DMs. + options := []slack.MsgOption{ + slack.MsgOptionText(text, false), + slack.MsgOptionDisableLinkUnfurl(), + } err = retry.Do( func() error { - _, ts, err := c.api.PostMessageContext(ctx, channelID, slack.MsgOptionText(text, false)) + _, ts, err := c.api.PostMessageContext(ctx, channelID, options...) if err != nil { if isRateLimitError(err) { slog.Warn("rate limited sending DM, backing off", "user", userID) @@ -522,6 +534,7 @@ func (c *Client) SaveDMMessageInfo(ctx context.Context, userID, prURL, channelID } // UpdateDMMessage updates a previously sent DM message. +// Returns nil and logs debug if no DM exists, returns error if update fails. func (c *Client) UpdateDMMessage(ctx context.Context, userID, prURL, newText string) error { c.stateStoreMu.RLock() store := c.stateStore @@ -529,7 +542,7 @@ func (c *Client) UpdateDMMessage(ctx context.Context, userID, prURL, newText str if store == nil { slog.Debug("no state store configured, cannot update DM", "user", userID, "pr_url", prURL) - return nil + return ErrNoDMToUpdate } // Get stored DM message info @@ -539,7 +552,7 @@ func (c *Client) UpdateDMMessage(ctx context.Context, userID, prURL, newText str "user", userID, "pr_url", prURL, "reason", "never sent or too old") - return nil + return ErrNoDMToUpdate } // Update the message using Slack API