From ca89cdf951b74dc458889e23fa85ae3f120198c9 Mon Sep 17 00:00:00 2001 From: Codex Date: Sun, 1 Mar 2026 12:59:13 -0800 Subject: [PATCH 1/7] Add interactive nudge status updates --- internal/app/app.go | 2 +- internal/integrations/llm/llm.go | 101 +++- internal/integrations/llm/llm_test.go | 41 ++ internal/integrations/slack/deps.go | 20 +- .../slack/nudge_interaction_test.go | 351 ++++++++++++++ internal/integrations/slack/slack.go | 198 +++++++- internal/nudge/deps.go | 7 + internal/nudge/nudge.go | 449 +++++++++++++++++- internal/nudge/nudge_render_test.go | 263 ++++++++++ internal/storage/sqlite/db.go | 10 + internal/storage/sqlite/db_test.go | 5 +- 11 files changed, 1421 insertions(+), 26 deletions(-) create mode 100644 internal/integrations/slack/nudge_interaction_test.go create mode 100644 internal/nudge/nudge_render_test.go diff --git a/internal/app/app.go b/internal/app/app.go index d0ca83b..60bda79 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -45,7 +45,7 @@ func Main() { slack.OptionAppLevelToken(cfg.SlackAppToken), ) - nudge.StartNudgeScheduler(cfg, api) + nudge.StartNudgeScheduler(cfg, db, api) fetch.StartAutoFetchScheduler(cfg, db, api) log.Println("Starting Engineering Report Bot...") diff --git a/internal/integrations/llm/llm.go b/internal/integrations/llm/llm.go index 14d4f01..939cca5 100644 --- a/internal/integrations/llm/llm.go +++ b/internal/integrations/llm/llm.go @@ -9,6 +9,8 @@ import ( "log" "net/http" "os" + "regexp" + "strconv" "strings" "sync" @@ -65,6 +67,8 @@ const defaultOpenAIModel = "gpt-4o-mini" const maxTemplateGuidanceChars = 8000 const defaultLLMMaxConcurrentBatches = 4 +var confidenceFieldRe = regexp.MustCompile(`("confidence"\s*:\s*)([^,}\]]+)`) + func llmBatchConcurrencyLimit(totalBatches int) int { if totalBatches <= 0 { return 1 @@ -295,7 +299,7 @@ Also: - choose normalized_status from: done, in testing, in progress, other - extract ticket IDs if present (e.g. [1247202] or bare ticket numbers) - if this item is the same underlying work as an existing item, set duplicate_of to that existing key (Kxx); otherwise empty string -- set confidence between 0 and 1. +- set confidence between 0 and 1, using digits only (example: 0.91). Never spell out numbers. %s%s Respond with JSON only (no markdown): @@ -362,7 +366,14 @@ func parseSectionClassifiedResponse(responseText string) (map[int64]LLMSectionDe var classified []sectionClassifiedItem if err := json.Unmarshal([]byte(responseText), &classified); err != nil { - return nil, fmt.Errorf("parsing LLM section response: %w (response: %s)", err, responseText) + repaired := repairSectionClassifiedResponse(responseText) + if repaired == responseText { + return nil, fmt.Errorf("parsing LLM section response: %w (response: %s)", err, responseText) + } + if repairErr := json.Unmarshal([]byte(repaired), &classified); repairErr != nil { + return nil, fmt.Errorf("parsing LLM section response: %w (response: %s)", err, responseText) + } + log.Printf("llm section response repaired before parse") } decisions := make(map[int64]LLMSectionDecision) @@ -424,6 +435,92 @@ func parseTicketIDsField(raw json.RawMessage) string { return "" } +func repairSectionClassifiedResponse(responseText string) string { + return confidenceFieldRe.ReplaceAllStringFunc(responseText, func(match string) string { + parts := confidenceFieldRe.FindStringSubmatch(match) + if len(parts) != 3 { + return match + } + return parts[1] + normalizeConfidenceLiteral(parts[2]) + }) +} + +func normalizeConfidenceLiteral(raw string) string { + text := strings.TrimSpace(strings.Trim(raw, `"`)) + if text == "" { + return "0" + } + if parsed, ok := parseLooseConfidence(text); ok { + return strconv.FormatFloat(parsed, 'f', -1, 64) + } + return "0" +} + +func parseLooseConfidence(raw string) (float64, bool) { + text := strings.TrimSpace(raw) + if text == "" { + return 0, false + } + if parsed, err := strconv.ParseFloat(text, 64); err == nil { + return clampConfidence(parsed), true + } + + compact := strings.ReplaceAll(strings.ToLower(text), " ", "") + if parsed, err := strconv.ParseFloat(compact, 64); err == nil { + return clampConfidence(parsed), true + } + + if strings.HasPrefix(compact, "0.") { + if digit, ok := digitWord(compact[2:]); ok { + return float64(digit) / 10, true + } + } + if compact == "zero" { + return 0, true + } + if compact == "one" { + return 1, true + } + return 0, false +} + +func digitWord(s string) (int, bool) { + switch s { + case "zero": + return 0, true + case "one": + return 1, true + case "two": + return 2, true + case "three": + return 3, true + case "four": + return 4, true + case "five": + return 5, true + case "six": + return 6, true + case "seven": + return 7, true + case "eight": + return 8, true + case "nine": + return 9, true + default: + return 0, false + } +} + +func clampConfidence(v float64) float64 { + if v < 0 { + return 0 + } + if v > 1 { + return 1 + } + return v +} + func loadGlossaryIfConfigured(cfg Config) (*LLMGlossary, error) { if strings.TrimSpace(cfg.LLMGlossaryPath) == "" { return nil, nil diff --git a/internal/integrations/llm/llm_test.go b/internal/integrations/llm/llm_test.go index 52df775..089fb76 100644 --- a/internal/integrations/llm/llm_test.go +++ b/internal/integrations/llm/llm_test.go @@ -129,6 +129,47 @@ func TestParseSectionClassifiedResponse_AcceptsArrayTicketIDs(t *testing.T) { } } +func TestParseSectionClassifiedResponse_RepairsMalformedConfidence(t *testing.T) { + response := `[ + {"id": 1, "section_id": "S0_0", "normalized_status": "done", "ticket_ids": "", "duplicate_of": "", "confidence": 0. Nine}, + {"id": 2, "section_id": "S0_1", "normalized_status": "in progress", "ticket_ids": "", "duplicate_of": "", "confidence": 0. seven}, + {"id": 3, "section_id": "S0_2", "normalized_status": "done", "ticket_ids": "", "duplicate_of": "", "confidence": nope} + ]` + + got, err := parseSectionClassifiedResponse(response) + if err != nil { + t.Fatalf("parseSectionClassifiedResponse should repair malformed confidence: %v", err) + } + if got[1].Confidence != 0.9 { + t.Fatalf("expected repaired confidence 0.9, got %v", got[1].Confidence) + } + if got[2].Confidence != 0.7 { + t.Fatalf("expected repaired confidence 0.7, got %v", got[2].Confidence) + } + if got[3].Confidence != 0 { + t.Fatalf("expected fallback confidence 0, got %v", got[3].Confidence) + } +} + +func TestNormalizeConfidenceLiteral(t *testing.T) { + tests := []struct { + raw string + want string + }{ + {raw: "0.91", want: "0.91"}, + {raw: `"0.82"`, want: "0.82"}, + {raw: "0. Nine", want: "0.9"}, + {raw: "0. seven", want: "0.7"}, + {raw: "garbage", want: "0"}, + } + + for _, tt := range tests { + if got := normalizeConfidenceLiteral(tt.raw); got != tt.want { + t.Fatalf("normalizeConfidenceLiteral(%q) = %q, want %q", tt.raw, got, tt.want) + } + } +} + func TestParseTicketIDsField_MixedArray(t *testing.T) { raw := json.RawMessage(`[ "123", 456, "", " 789 " ]`) got := parseTicketIDsField(raw) diff --git a/internal/integrations/slack/deps.go b/internal/integrations/slack/deps.go index ae9b051..2998c2a 100644 --- a/internal/integrations/slack/deps.go +++ b/internal/integrations/slack/deps.go @@ -24,6 +24,7 @@ type ClassificationStats = domain.ClassificationStats type sectionOption = llm.SectionOption type BuildResult = report.BuildResult type LLMSectionDecision = llm.LLMSectionDecision +type RenderedNudge = nudge.RenderedNudge type loadStatus int @@ -32,6 +33,13 @@ const ( templateFirstEver ) +const ( + actionNudgeDone = nudge.ActionDone + actionNudgeMore = nudge.ActionMore + actionNudgePagePrev = nudge.ActionPagePrev + actionNudgePageNext = nudge.ActionPageNext +) + func ReportWeekRange(cfg Config, now time.Time) (time.Time, time.Time) { return domain.ReportWeekRange(cfg, now) } @@ -136,6 +144,10 @@ func UpdateWorkItemTextAndStatus(db *sql.DB, id int64, description, status strin return sqlite.UpdateWorkItemTextAndStatus(db, id, description, status) } +func UpdateWorkItemStatus(db *sql.DB, id int64, status string) error { + return sqlite.UpdateWorkItemStatus(db, id, status) +} + func UpdateWorkItemCategory(db *sql.DB, id int64, category string) error { return sqlite.UpdateWorkItemCategory(db, id, category) } @@ -180,8 +192,12 @@ func extractGlossaryPhrase(description string) string { return llm.ExtractGlossaryPhrase(description) } -func sendNudges(api *slack.Client, cfg Config, memberIDs []string, reportChannelID string) { - nudge.SendNudges(api, cfg, memberIDs, reportChannelID) +func sendNudges(api *slack.Client, db *sql.DB, cfg Config, memberIDs []string, reportChannelID string) { + nudge.SendNudges(api, db, cfg, memberIDs, reportChannelID) +} + +func RenderNudgeForUser(api *slack.Client, db *sql.DB, cfg Config, userID, reportChannelID string, now time.Time, page int, updated bool) (RenderedNudge, error) { + return nudge.RenderNudgeForUser(api, db, cfg, userID, reportChannelID, now, page, updated) } func normalizeTextToken(s string) string { diff --git a/internal/integrations/slack/nudge_interaction_test.go b/internal/integrations/slack/nudge_interaction_test.go new file mode 100644 index 0000000..98a7fb1 --- /dev/null +++ b/internal/integrations/slack/nudge_interaction_test.go @@ -0,0 +1,351 @@ +package slackbot + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "testing" + "time" + + "github.com/slack-go/slack" +) + +type nudgeUpdateRecorder struct { + updateCalls int + lastUpdateText string + ephemeralCalls int +} + +type mockSlackRoundTripper func(*http.Request) (*http.Response, error) + +func (fn mockSlackRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + return fn(req) +} + +func newMockSlackAPIForNudgeInteractions(t *testing.T, rec *nudgeUpdateRecorder) *slack.Client { + t.Helper() + httpClient := &http.Client{ + Transport: mockSlackRoundTripper(func(req *http.Request) (*http.Response, error) { + form, err := mockSlackRequestForm(req) + if err != nil { + return nil, err + } + + payload := map[string]any{"ok": true} + switch strings.TrimPrefix(req.URL.Path, "/api/") { + case "users.info": + userID := form.Get("user") + display := userID + if userID == "U_MEMBER" { + display = "Pat" + } + payload = map[string]any{ + "ok": true, + "user": map[string]any{ + "id": userID, + "name": strings.ToLower(display), + "real_name": display + " Real", + "profile": map[string]any{ + "display_name": display, + }, + }, + } + case "chat.update": + rec.updateCalls++ + rec.lastUpdateText = form.Get("text") + payload = map[string]any{"ok": true, "channel": "D123", "ts": "123.45", "text": rec.lastUpdateText} + case "chat.postEphemeral": + rec.ephemeralCalls++ + payload = map[string]any{"ok": true} + } + + return mockSlackJSONResponse(req, payload) + }), + } + + return slack.New( + "xoxb-test", + slack.OptionAPIURL("https://slack.test/api/"), + slack.OptionHTTPClient(httpClient), + ) +} + +func mockSlackRequestForm(req *http.Request) (url.Values, error) { + values := req.URL.Query() + if req.Body == nil { + return values, nil + } + body, err := io.ReadAll(req.Body) + if err != nil { + return nil, err + } + if len(body) == 0 { + return values, nil + } + parsed, err := url.ParseQuery(string(body)) + if err != nil { + return nil, err + } + for key, vals := range parsed { + for _, val := range vals { + values.Add(key, val) + } + } + req.Body = io.NopCloser(bytes.NewReader(body)) + return values, nil +} + +func mockSlackJSONResponse(req *http.Request, payload any) (*http.Response, error) { + body, err := json.Marshal(payload) + if err != nil { + return nil, err + } + return &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: io.NopCloser(bytes.NewReader(body)), + Request: req, + }, nil +} + +func TestHandleBlockActions_NudgeDoneUpdatesStatusAndRefreshesMessage(t *testing.T) { + db := newTestDB(t) + now := time.Now().UTC().Truncate(time.Second) + if err := InsertWorkItem(db, WorkItem{ + Description: "Close stale deployment work", + Author: "Pat", + AuthorID: "U_MEMBER", + Source: "slack", + Status: "in progress", + ReportedAt: now, + }); err != nil { + t.Fatalf("insert work item: %v", err) + } + items, err := GetItemsByDateRange(db, now.Add(-time.Hour), now.Add(time.Hour)) + if err != nil || len(items) != 1 { + t.Fatalf("load items err=%v len=%d", err, len(items)) + } + + rec := &nudgeUpdateRecorder{} + api := newMockSlackAPIForNudgeInteractions(t, rec) + cfg := Config{Location: time.UTC} + cb := slack.InteractionCallback{ + User: slack.User{ID: "U_MEMBER"}, + Container: slack.Container{ChannelID: "D123", MessageTs: "123.45"}, + ActionCallback: slack.ActionCallbacks{ + BlockActions: []*slack.BlockAction{ + {ActionID: actionNudgeDone, Value: fmt.Sprintf("done|U_MEMBER|%d|0", items[0].ID)}, + }, + }, + } + + handleBlockActions(api, db, cfg, cb) + + updated, err := GetWorkItemByID(db, items[0].ID) + if err != nil { + t.Fatalf("GetWorkItemByID failed: %v", err) + } + if updated.Status != "done" { + t.Fatalf("expected status done, got %q", updated.Status) + } + if rec.updateCalls != 1 { + t.Fatalf("expected one chat.update call, got %d", rec.updateCalls) + } + if !strings.Contains(rec.lastUpdateText, "Updated. No items are currently marked in progress.") { + t.Fatalf("unexpected updated message text: %q", rec.lastUpdateText) + } +} + +func TestHandleBlockActions_NudgeMoreUpdatesStatus(t *testing.T) { + db := newTestDB(t) + now := time.Now().UTC().Truncate(time.Second) + if err := InsertWorkItem(db, WorkItem{ + Description: "Validate build pipeline", + Author: "Pat", + AuthorID: "U_MEMBER", + Source: "slack", + Status: "in progress", + ReportedAt: now, + }); err != nil { + t.Fatalf("insert work item: %v", err) + } + items, _ := GetItemsByDateRange(db, now.Add(-time.Hour), now.Add(time.Hour)) + + rec := &nudgeUpdateRecorder{} + api := newMockSlackAPIForNudgeInteractions(t, rec) + cfg := Config{Location: time.UTC} + cb := slack.InteractionCallback{ + User: slack.User{ID: "U_MEMBER"}, + Container: slack.Container{ChannelID: "D123", MessageTs: "123.45"}, + ActionCallback: slack.ActionCallbacks{ + BlockActions: []*slack.BlockAction{ + { + ActionID: actionNudgeMore, + SelectedOption: slack.OptionBlockObject{ + Value: fmt.Sprintf("status|U_MEMBER|%d|in testing|0", items[0].ID), + }, + }, + }, + }, + } + + handleBlockActions(api, db, cfg, cb) + + updated, err := GetWorkItemByID(db, items[0].ID) + if err != nil { + t.Fatalf("GetWorkItemByID failed: %v", err) + } + if updated.Status != "in testing" { + t.Fatalf("expected status in testing, got %q", updated.Status) + } + if rec.updateCalls != 1 { + t.Fatalf("expected one chat.update call, got %d", rec.updateCalls) + } +} + +func TestHandleBlockActions_NudgePageNextRefreshesWithoutMutation(t *testing.T) { + db := newTestDB(t) + now := time.Now().UTC().Truncate(time.Second) + for i := 0; i < 11; i++ { + if err := InsertWorkItem(db, WorkItem{ + Description: fmt.Sprintf("Paginated active item %02d", i+1), + Author: "Pat", + AuthorID: "U_MEMBER", + Source: "slack", + Status: "in progress", + ReportedAt: now.Add(time.Duration(i) * time.Minute), + }); err != nil { + t.Fatalf("insert work item %d: %v", i, err) + } + } + items, _ := GetItemsByDateRange(db, now.Add(-time.Hour), now.Add(2*time.Hour)) + + rec := &nudgeUpdateRecorder{} + api := newMockSlackAPIForNudgeInteractions(t, rec) + cfg := Config{Location: time.UTC} + cb := slack.InteractionCallback{ + User: slack.User{ID: "U_MEMBER"}, + Container: slack.Container{ChannelID: "D123", MessageTs: "123.45"}, + ActionCallback: slack.ActionCallbacks{ + BlockActions: []*slack.BlockAction{ + {ActionID: actionNudgePageNext, Value: "page|U_MEMBER|1"}, + }, + }, + } + + handleBlockActions(api, db, cfg, cb) + + if rec.updateCalls != 1 { + t.Fatalf("expected one chat.update call, got %d", rec.updateCalls) + } + if !strings.Contains(rec.lastUpdateText, "Showing 11-11 of 11 active items") { + t.Fatalf("expected second-page summary, got %q", rec.lastUpdateText) + } + unchanged, err := GetWorkItemByID(db, items[0].ID) + if err != nil { + t.Fatalf("GetWorkItemByID failed: %v", err) + } + if unchanged.Status != "in progress" { + t.Fatalf("expected navigation not to mutate status, got %q", unchanged.Status) + } +} + +func TestHandleBlockActions_NudgeDoneClampsPageAfterLastItemRemoved(t *testing.T) { + db := newTestDB(t) + now := time.Now().UTC().Truncate(time.Second) + for i := 0; i < 11; i++ { + if err := InsertWorkItem(db, WorkItem{ + Description: fmt.Sprintf("Paginated active item %02d", i+1), + Author: "Pat", + AuthorID: "U_MEMBER", + Source: "slack", + Status: "in progress", + ReportedAt: now.Add(time.Duration(i) * time.Minute), + }); err != nil { + t.Fatalf("insert work item %d: %v", i, err) + } + } + items, err := GetItemsByDateRange(db, now.Add(-time.Hour), now.Add(2*time.Hour)) + if err != nil { + t.Fatalf("GetItemsByDateRange failed: %v", err) + } + + rec := &nudgeUpdateRecorder{} + api := newMockSlackAPIForNudgeInteractions(t, rec) + cfg := Config{Location: time.UTC} + lastItem := items[len(items)-1] + cb := slack.InteractionCallback{ + User: slack.User{ID: "U_MEMBER"}, + Container: slack.Container{ChannelID: "D123", MessageTs: "123.45"}, + ActionCallback: slack.ActionCallbacks{ + BlockActions: []*slack.BlockAction{ + {ActionID: actionNudgeDone, Value: fmt.Sprintf("done|U_MEMBER|%d|1", lastItem.ID)}, + }, + }, + } + + handleBlockActions(api, db, cfg, cb) + + updated, err := GetWorkItemByID(db, lastItem.ID) + if err != nil { + t.Fatalf("GetWorkItemByID failed: %v", err) + } + if updated.Status != "done" { + t.Fatalf("expected status done, got %q", updated.Status) + } + if rec.updateCalls != 1 { + t.Fatalf("expected one chat.update call, got %d", rec.updateCalls) + } + if !strings.Contains(rec.lastUpdateText, "Showing 1-10 of 10 active items") { + t.Fatalf("expected page clamp to first page, got %q", rec.lastUpdateText) + } +} + +func TestHandleBlockActions_NudgeUnauthorizedUserCannotUpdate(t *testing.T) { + db := newTestDB(t) + now := time.Now().UTC().Truncate(time.Second) + if err := InsertWorkItem(db, WorkItem{ + Description: "Protected item", + Author: "Pat", + AuthorID: "U_MEMBER", + Source: "slack", + Status: "in progress", + ReportedAt: now, + }); err != nil { + t.Fatalf("insert work item: %v", err) + } + items, _ := GetItemsByDateRange(db, now.Add(-time.Hour), now.Add(time.Hour)) + + rec := &nudgeUpdateRecorder{} + api := newMockSlackAPIForNudgeInteractions(t, rec) + cfg := Config{Location: time.UTC} + cb := slack.InteractionCallback{ + User: slack.User{ID: "U_OTHER"}, + Container: slack.Container{ChannelID: "D123", MessageTs: "123.45"}, + ActionCallback: slack.ActionCallbacks{ + BlockActions: []*slack.BlockAction{ + {ActionID: actionNudgeDone, Value: fmt.Sprintf("done|U_MEMBER|%d|0", items[0].ID)}, + }, + }, + } + + handleBlockActions(api, db, cfg, cb) + + unchanged, err := GetWorkItemByID(db, items[0].ID) + if err != nil { + t.Fatalf("GetWorkItemByID failed: %v", err) + } + if unchanged.Status != "in progress" { + t.Fatalf("expected unauthorized action not to mutate status, got %q", unchanged.Status) + } + if rec.updateCalls != 0 { + t.Fatalf("expected no chat.update call, got %d", rec.updateCalls) + } + if rec.ephemeralCalls != 1 { + t.Fatalf("expected one ephemeral denial message, got %d", rec.ephemeralCalls) + } +} diff --git a/internal/integrations/slack/slack.go b/internal/integrations/slack/slack.go index 4781f67..13372b7 100644 --- a/internal/integrations/slack/slack.go +++ b/internal/integrations/slack/slack.go @@ -1083,6 +1083,15 @@ func handleBlockActions(api *slack.Client, db *sql.DB, cfg Config, cb slack.Inte case actionNudgeAll: openNudgeConfirmModal(api, cfg, cb.TriggerID, channelID, act.Value) return + case actionNudgeDone: + handleNudgeDoneAction(api, db, cfg, cb, act) + return + case actionNudgeMore: + handleNudgeMoreAction(api, db, cfg, cb, act) + return + case actionNudgePagePrev, actionNudgePageNext: + handleNudgePageAction(api, db, cfg, cb, act) + return case actionRetroApply: handleRetroApply(api, db, cfg, cb, act) return @@ -1136,7 +1145,7 @@ func handleBlockActions(api *slack.Client, db *sql.DB, cfg Config, cb slack.Inte func handleViewSubmission(api *slack.Client, db *sql.DB, cfg Config, cb slack.InteractionCallback) { if cb.View.CallbackID == modalNudgeConfirmCallback { - handleNudgeConfirm(api, cfg, cb) + handleNudgeConfirm(api, db, cfg, cb) return } @@ -1583,7 +1592,7 @@ func openNudgeConfirmModal(api *slack.Client, cfg Config, triggerID, channelID, } } -func handleNudgeConfirm(api *slack.Client, cfg Config, cb slack.InteractionCallback) { +func handleNudgeConfirm(api *slack.Client, db *sql.DB, cfg Config, cb slack.InteractionCallback) { userID := cb.User.ID if !cfg.IsManagerID(userID) { log.Printf("nudge confirm denied user=%s (not manager)", userID) @@ -1615,11 +1624,194 @@ func handleNudgeConfirm(api *slack.Client, cfg Config, cb slack.InteractionCallb return } - sendNudges(api, cfg, targetIDs, cfg.ReportChannelID) + sendNudges(api, db, cfg, targetIDs, cfg.ReportChannelID) postEphemeralTo(api, channelID, userID, fmt.Sprintf("Sent nudge to %d member(s).", len(targetIDs))) log.Printf("nudge sent from /check user=%s count=%d", userID, len(targetIDs)) } +func handleNudgeDoneAction(api *slack.Client, db *sql.DB, cfg Config, cb slack.InteractionCallback, act *slack.BlockAction) { + targetUserID, itemID, page, ok := parseNudgeDonePayload(act.Value) + if !ok { + log.Printf("nudge done invalid payload=%q", act.Value) + return + } + if !authorizeNudgeAction(api, cb, targetUserID) { + return + } + if !canActOnNudgeItem(api, db, cfg, itemID, targetUserID) { + log.Printf("nudge done denied item=%d target=%s", itemID, targetUserID) + return + } + if err := UpdateWorkItemStatus(db, itemID, "done"); err != nil { + log.Printf("nudge done update error item=%d: %v", itemID, err) + return + } + refreshNudgeMessage(api, db, cfg, cb, targetUserID, page, true) +} + +func handleNudgeMoreAction(api *slack.Client, db *sql.DB, cfg Config, cb slack.InteractionCallback, act *slack.BlockAction) { + val := strings.TrimSpace(act.Value) + if strings.TrimSpace(act.SelectedOption.Value) != "" { + val = strings.TrimSpace(act.SelectedOption.Value) + } + targetUserID, itemID, status, page, ok := parseNudgeStatusPayload(val) + if !ok { + log.Printf("nudge more invalid payload=%q", val) + return + } + if !authorizeNudgeAction(api, cb, targetUserID) { + return + } + if !canActOnNudgeItem(api, db, cfg, itemID, targetUserID) { + log.Printf("nudge status denied item=%d target=%s", itemID, targetUserID) + return + } + if err := UpdateWorkItemStatus(db, itemID, status); err != nil { + log.Printf("nudge status update error item=%d status=%q: %v", itemID, status, err) + return + } + refreshNudgeMessage(api, db, cfg, cb, targetUserID, page, true) +} + +func handleNudgePageAction(api *slack.Client, db *sql.DB, cfg Config, cb slack.InteractionCallback, act *slack.BlockAction) { + targetUserID, page, ok := parseNudgePagePayload(act.Value) + if !ok { + log.Printf("nudge page invalid payload=%q", act.Value) + return + } + if !authorizeNudgeAction(api, cb, targetUserID) { + return + } + refreshNudgeMessage(api, db, cfg, cb, targetUserID, page, false) +} + +func authorizeNudgeAction(api *slack.Client, cb slack.InteractionCallback, targetUserID string) bool { + if strings.TrimSpace(cb.User.ID) != strings.TrimSpace(targetUserID) { + channelID := nudgeActionChannelID(cb) + if channelID != "" { + postEphemeralTo(api, channelID, cb.User.ID, "This nudge action is only available to the message recipient.") + } + log.Printf("nudge action denied user=%s target=%s", cb.User.ID, targetUserID) + return false + } + return true +} + +func canActOnNudgeItem(api *slack.Client, db *sql.DB, cfg Config, itemID int64, targetUserID string) bool { + item, err := GetWorkItemByID(db, itemID) + if err != nil { + return false + } + monday, nextMonday := ReportWeekRange(cfg, time.Now().In(cfg.Location)) + if !itemInRange(item, monday, nextMonday) { + return false + } + if strings.TrimSpace(item.AuthorID) != "" { + return strings.EqualFold(strings.TrimSpace(item.AuthorID), strings.TrimSpace(targetUserID)) + } + + user, err := api.GetUserInfo(targetUserID) + if err != nil { + return false + } + candidates := []string{user.Profile.DisplayName, user.RealName, user.Name} + for _, candidate := range candidates { + candidate = strings.TrimSpace(candidate) + if candidate == "" { + continue + } + if strings.EqualFold(strings.TrimSpace(item.Author), candidate) || nameMatches(item.Author, candidate) || nameMatches(candidate, item.Author) { + return true + } + } + return false +} + +func refreshNudgeMessage(api *slack.Client, db *sql.DB, cfg Config, cb slack.InteractionCallback, targetUserID string, page int, updated bool) { + channelID := nudgeActionChannelID(cb) + messageTS := strings.TrimSpace(cb.Container.MessageTs) + if messageTS == "" { + messageTS = strings.TrimSpace(cb.MessageTs) + } + if channelID == "" || messageTS == "" { + log.Printf("nudge refresh missing channel/message ts channel=%q ts=%q", channelID, messageTS) + return + } + + rendered, err := RenderNudgeForUser(api, db, cfg, targetUserID, cfg.ReportChannelID, time.Now().In(cfg.Location), page, updated) + if err != nil { + log.Printf("nudge refresh render error user=%s: %v", targetUserID, err) + return + } + _, _, _, err = api.UpdateMessage( + channelID, + messageTS, + slack.MsgOptionText(rendered.Text, false), + slack.MsgOptionBlocks(rendered.Blocks...), + ) + if err != nil { + log.Printf("nudge refresh update error channel=%s ts=%s: %v", channelID, messageTS, err) + } +} + +func nudgeActionChannelID(cb slack.InteractionCallback) string { + channelID := strings.TrimSpace(cb.Channel.ID) + if channelID == "" { + channelID = strings.TrimSpace(cb.Container.ChannelID) + } + return channelID +} + +func parseNudgeDonePayload(raw string) (targetUserID string, itemID int64, page int, ok bool) { + parts := strings.Split(strings.TrimSpace(raw), "|") + if len(parts) != 4 || parts[0] != "done" { + return "", 0, 0, false + } + itemID, err := strconv.ParseInt(parts[2], 10, 64) + if err != nil { + return "", 0, 0, false + } + page, err = strconv.Atoi(parts[3]) + if err != nil { + return "", 0, 0, false + } + return strings.TrimSpace(parts[1]), itemID, page, true +} + +func parseNudgeStatusPayload(raw string) (targetUserID string, itemID int64, status string, page int, ok bool) { + parts := strings.Split(strings.TrimSpace(raw), "|") + if len(parts) != 5 || parts[0] != "status" { + return "", 0, "", 0, false + } + itemID, err := strconv.ParseInt(parts[2], 10, 64) + if err != nil { + return "", 0, "", 0, false + } + page, err = strconv.Atoi(parts[4]) + if err != nil { + return "", 0, "", 0, false + } + status = strings.TrimSpace(parts[3]) + switch status { + case "in testing", "in progress": + default: + return "", 0, "", 0, false + } + return strings.TrimSpace(parts[1]), itemID, status, page, true +} + +func parseNudgePagePayload(raw string) (targetUserID string, page int, ok bool) { + parts := strings.Split(strings.TrimSpace(raw), "|") + if len(parts) != 3 || parts[0] != "page" { + return "", 0, false + } + page, err := strconv.Atoi(parts[2]) + if err != nil { + return "", 0, false + } + return strings.TrimSpace(parts[1]), page, true +} + func handleReportStats(api *slack.Client, db *sql.DB, cfg Config, cmd slack.SlashCommand) { isManager, err := isManagerUser(api, cfg, cmd.UserID) if err != nil { diff --git a/internal/nudge/deps.go b/internal/nudge/deps.go index b78d3fe..74f655a 100644 --- a/internal/nudge/deps.go +++ b/internal/nudge/deps.go @@ -1,8 +1,10 @@ package nudge import ( + "database/sql" "reportbot/internal/config" "reportbot/internal/domain" + "reportbot/internal/storage/sqlite" "strings" "time" @@ -10,11 +12,16 @@ import ( ) type Config = config.Config +type WorkItem = domain.WorkItem func ReportWeekRange(cfg Config, now time.Time) (time.Time, time.Time) { return domain.ReportWeekRange(cfg, now) } +func GetItemsByDateRange(db *sql.DB, from, to time.Time) ([]WorkItem, error) { + return sqlite.GetItemsByDateRange(db, from, to) +} + func resolveUserIDs(api *slack.Client, identifiers []string) ([]string, []string, error) { var ids []string var names []string diff --git a/internal/nudge/nudge.go b/internal/nudge/nudge.go index 851801d..d01ae95 100644 --- a/internal/nudge/nudge.go +++ b/internal/nudge/nudge.go @@ -1,14 +1,40 @@ package nudge import ( + "database/sql" "fmt" "log" + "regexp" + "sort" "strings" "time" "github.com/slack-go/slack" ) +const ( + ActionDone = "nudge_done" + ActionMore = "nudge_more" + ActionPagePrev = "nudge_page_prev" + ActionPageNext = "nudge_page_next" + nudgePageSize = 10 +) + +var parenPattern = regexp.MustCompile(`\([^)]*\)|（[^）]*）`) + +type RenderedNudge struct { + Text string + Blocks []slack.Block +} + +type renderedNudgeState struct { + active []WorkItem + page int + pageStart int + pageEnd int + totalPages int +} + var dayMap = map[string]time.Weekday{ "sunday": time.Sunday, "monday": time.Monday, @@ -19,7 +45,7 @@ var dayMap = map[string]time.Weekday{ "saturday": time.Saturday, } -func StartNudgeScheduler(cfg Config, api *slack.Client) { +func StartNudgeScheduler(cfg Config, db *sql.DB, api *slack.Client) { if len(cfg.TeamMembers) == 0 { log.Println("No team_members configured, nudge disabled") return @@ -58,7 +84,7 @@ func StartNudgeScheduler(cfg Config, api *slack.Client) { log.Printf("Next nudge at %s (in %s)", next.Format("Mon Jan 2 15:04"), wait.Round(time.Minute)) time.Sleep(wait) - sendNudges(api, cfg, memberIDs, cfg.ReportChannelID) + sendNudges(api, db, cfg, memberIDs, cfg.ReportChannelID) } }() } @@ -75,39 +101,428 @@ func nextWeekday(now time.Time, day time.Weekday, hour, min int) time.Time { return time.Date(now.Year(), now.Month(), now.Day()+int(daysUntil), hour, min, 0, 0, now.Location()) } -func sendNudges(api *slack.Client, cfg Config, memberIDs []string, reportChannelID string) { - monday, nextMonday := ReportWeekRange(cfg, time.Now().In(cfg.Location)) +func sendNudges(api *slack.Client, db *sql.DB, cfg Config, memberIDs []string, reportChannelID string) { + for _, userID := range memberIDs { + channel, _, _, err := api.OpenConversation(&slack.OpenConversationParameters{ + Users: []string{userID}, + }) + if err != nil { + log.Printf("Error opening DM with %s: %v", userID, err) + continue + } + + rendered, err := RenderNudgeForUser(api, db, cfg, userID, reportChannelID, time.Now().In(cfg.Location), 0, false) + if err != nil { + log.Printf("Error rendering nudge for %s: %v", userID, err) + rendered = renderGenericNudge(cfg, reportChannelID, time.Now().In(cfg.Location)) + } + + _, _, err = api.PostMessage( + channel.ID, + slack.MsgOptionText(rendered.Text, false), + slack.MsgOptionBlocks(rendered.Blocks...), + ) + if err != nil { + log.Printf("Error sending nudge to %s: %v", userID, err) + } else { + log.Printf("Sent nudge to %s", userID) + } + } +} + +func SendNudges(api *slack.Client, db *sql.DB, cfg Config, memberIDs []string, reportChannelID string) { + sendNudges(api, db, cfg, memberIDs, reportChannelID) +} + +func RenderNudgeForUser(api *slack.Client, db *sql.DB, cfg Config, userID, reportChannelID string, now time.Time, page int, updated bool) (RenderedNudge, error) { + now = now.In(cfg.Location) + monday, nextMonday := ReportWeekRange(cfg, now) + reminder := buildGenericNudgeText(reportChannelID, monday, nextMonday) + + items, err := GetItemsByDateRange(db, monday, nextMonday) + if err != nil { + return RenderedNudge{}, err + } + + user, err := api.GetUserInfo(userID) + if err != nil { + log.Printf("nudge user lookup failed user=%s: %v", userID, err) + user = nil + } + + active, state := buildNudgeState(items, userID, user, page) + if len(active) == 0 { + if updated { + return renderUpdatedNoItems(reminder), nil + } + return renderGenericNudge(cfg, reportChannelID, now), nil + } + + return renderInteractiveNudge(reminder, userID, active, state, updated), nil +} + +func renderGenericNudge(cfg Config, reportChannelID string, now time.Time) RenderedNudge { + monday, nextMonday := ReportWeekRange(cfg, now.In(cfg.Location)) + text := buildGenericNudgeText(reportChannelID, monday, nextMonday) + blocks := []slack.Block{ + slack.NewSectionBlock( + slack.NewTextBlockObject(slack.MarkdownType, text, false, false), + nil, + nil, + ), + } + return RenderedNudge{Text: text, Blocks: blocks} +} + +func renderUpdatedNoItems(reminder string) RenderedNudge { + msg := "Updated. No items are currently marked `in progress`." + blocks := []slack.Block{ + slack.NewSectionBlock(slack.NewTextBlockObject(slack.MarkdownType, reminder, false, false), nil, nil), + slack.NewDividerBlock(), + slack.NewSectionBlock(slack.NewTextBlockObject(slack.MarkdownType, msg, false, false), nil, nil), + } + return RenderedNudge{ + Text: reminder + "\n\nUpdated. No items are currently marked in progress.", + Blocks: blocks, + } +} + +func renderInteractiveNudge(reminder, userID string, active []WorkItem, state renderedNudgeState, updated bool) RenderedNudge { + intro := renderIntroText(len(active), updated) + blocks := []slack.Block{ + slack.NewSectionBlock(slack.NewTextBlockObject(slack.MarkdownType, reminder, false, false), nil, nil), + slack.NewDividerBlock(), + slack.NewSectionBlock(slack.NewTextBlockObject(slack.MarkdownType, intro, false, false), nil, nil), + } + + pageItems := active[state.pageStart:state.pageEnd] + textLines := []string{stripMarkdownFormatting(reminder), stripMarkdownFormatting(intro)} + for _, item := range pageItems { + itemText := formatNudgeItem(item) + blocks = append(blocks, + slack.NewSectionBlock(slack.NewTextBlockObject(slack.MarkdownType, itemText, false, false), nil, nil), + slack.NewActionBlock( + fmt.Sprintf("nudge_actions_%d", item.ID), + buildDoneButton(userID, item.ID, state.page), + buildMoreMenu(userID, item.ID, state.page), + ), + ) + textLines = append(textLines, "- "+stripMarkdownFormatting(itemText)) + } + + summary := fmt.Sprintf("Showing %d-%d of %d active items", state.pageStart+1, state.pageEnd, len(active)) + blocks = append(blocks, slack.NewSectionBlock(slack.NewTextBlockObject(slack.MarkdownType, summary, false, false), nil, nil)) + textLines = append(textLines, summary) + + if len(active) > nudgePageSize { + var nav []slack.BlockElement + if state.page > 0 { + nav = append(nav, slack.NewButtonBlockElement( + ActionPagePrev, + fmt.Sprintf("page|%s|%d", userID, state.page-1), + slack.NewTextBlockObject(slack.PlainTextType, "Prev", false, false), + )) + } + if state.page+1 < state.totalPages { + nav = append(nav, slack.NewButtonBlockElement( + ActionPageNext, + fmt.Sprintf("page|%s|%d", userID, state.page+1), + slack.NewTextBlockObject(slack.PlainTextType, "Next", false, false), + )) + } + if len(nav) > 0 { + blocks = append(blocks, slack.NewActionBlock("nudge_nav", nav...)) + } + } + + return RenderedNudge{ + Text: strings.Join(textLines, "\n\n"), + Blocks: blocks, + } +} + +func buildDoneButton(userID string, itemID int64, page int) *slack.ButtonBlockElement { + return slack.NewButtonBlockElement( + ActionDone, + fmt.Sprintf("done|%s|%d|%d", userID, itemID, page), + slack.NewTextBlockObject(slack.PlainTextType, "Mark as Done", false, false), + ).WithStyle(slack.StylePrimary) +} + +func buildMoreMenu(userID string, itemID int64, page int) *slack.OverflowBlockElement { + return slack.NewOverflowBlockElement( + ActionMore, + slack.NewOptionBlockObject( + fmt.Sprintf("status|%s|%d|in testing|%d", userID, itemID, page), + slack.NewTextBlockObject(slack.PlainTextType, "Mark as In testing", false, false), + nil, + ), + slack.NewOptionBlockObject( + fmt.Sprintf("status|%s|%d|in progress|%d", userID, itemID, page), + slack.NewTextBlockObject(slack.PlainTextType, "Mark as In progress", false, false), + nil, + ), + ) +} + +func buildNudgeState(items []WorkItem, userID string, user *slack.User, requestedPage int) ([]WorkItem, renderedNudgeState) { + active := dedupeAndFilterActiveItems(matchUserItems(items, userID, user)) + totalPages := 0 + if len(active) > 0 { + totalPages = (len(active) + nudgePageSize - 1) / nudgePageSize + } + page := clampPage(requestedPage, totalPages) + start, end := pageBounds(len(active), page) + return active, renderedNudgeState{ + active: active, + page: page, + pageStart: start, + pageEnd: end, + totalPages: totalPages, + } +} + +func matchUserItems(items []WorkItem, userID string, user *slack.User) []WorkItem { + var matched []WorkItem + for _, item := range items { + if itemBelongsToUser(item, userID, user) { + matched = append(matched, item) + } + } + return matched +} + +func itemBelongsToUser(item WorkItem, userID string, user *slack.User) bool { + if strings.TrimSpace(item.AuthorID) != "" { + return strings.TrimSpace(item.AuthorID) == strings.TrimSpace(userID) + } + if user == nil { + return false + } + for _, candidate := range userNameCandidates(user) { + if strings.EqualFold(strings.TrimSpace(item.Author), candidate) || nameMatches(item.Author, candidate) || nameMatches(candidate, item.Author) { + return true + } + } + return false +} + +func userNameCandidates(user *slack.User) []string { + if user == nil { + return nil + } + var out []string + add := func(s string) { + s = strings.TrimSpace(s) + if s == "" { + return + } + for _, existing := range out { + if strings.EqualFold(existing, s) { + return + } + } + out = append(out, s) + } + add(user.Profile.DisplayName) + add(user.RealName) + add(user.Name) + return out +} + +func dedupeAndFilterActiveItems(items []WorkItem) []WorkItem { + latest := make(map[string]WorkItem, len(items)) + for _, item := range items { + key := nudgeDedupeKey(item) + if key == "|" { + continue + } + existing, ok := latest[key] + if !ok || item.ReportedAt.After(existing.ReportedAt) || (item.ReportedAt.Equal(existing.ReportedAt) && item.ID > existing.ID) { + latest[key] = item + } + } + + active := make([]WorkItem, 0, len(latest)) + for _, item := range latest { + if statusContainsInProgress(item.Status) { + active = append(active, item) + } + } + sort.SliceStable(active, func(i, j int) bool { + if !active[i].ReportedAt.Equal(active[j].ReportedAt) { + return active[i].ReportedAt.Before(active[j].ReportedAt) + } + return active[i].ID < active[j].ID + }) + return active +} + +func nudgeDedupeKey(item WorkItem) string { + tickets := canonicalTicketIDs(item.TicketIDs) + desc := normalizeDescriptionForDedupe(item.Description, tickets) + return tickets + "|" + desc +} + +func normalizeDescriptionForDedupe(description, tickets string) string { + description = strings.TrimSpace(stripLeadingTicketPrefixIfSame(description, tickets)) + return strings.ToLower(description) +} + +func statusContainsInProgress(status string) bool { + return strings.Contains(strings.ToLower(strings.TrimSpace(status)), "in progress") +} + +func pageBounds(total, page int) (int, int) { + if total <= 0 { + return 0, 0 + } + start := page * nudgePageSize + if start > total { + start = total + } + end := start + nudgePageSize + if end > total { + end = total + } + return start, end +} + +func clampPage(page, totalPages int) int { + if totalPages <= 0 { + return 0 + } + if page < 0 { + return 0 + } + if page >= totalPages { + return totalPages - 1 + } + return page +} + +func renderIntroText(count int, updated bool) string { + if updated { + if count == 1 { + return "Updated. This item is still marked `in progress`:" + } + return "Updated. These items are still marked `in progress`:" + } + if count == 1 { + return "This item is still marked `in progress`. Use *Mark as Done* or the *More* menu to update it if its status has changed:" + } + return "These items are still marked `in progress`. Use *Mark as Done* or the *More* menu to update any that changed:" +} + +func buildGenericNudgeText(reportChannelID string, monday, nextMonday time.Time) string { channelRef := "" if reportChannelID != "" { channelRef = fmt.Sprintf(" Please report in <#%s>.", reportChannelID) } - msg := fmt.Sprintf( + return fmt.Sprintf( "Hey! Friendly reminder to report your work items for this week (%s - %s) using `/report`.%s\n"+ "Example: `/report [ticket_id] Add pagination to user list API (done)`", monday.Format("Jan 2"), nextMonday.AddDate(0, 0, -1).Format("Jan 2"), channelRef, ) +} - for _, userID := range memberIDs { - channel, _, _, err := api.OpenConversation(&slack.OpenConversationParameters{ - Users: []string{userID}, - }) - if err != nil { - log.Printf("Error opening DM with %s: %v", userID, err) +func formatNudgeItem(item WorkItem) string { + description := strings.TrimSpace(item.Description) + tickets := canonicalTicketIDs(item.TicketIDs) + if tickets != "" { + description = stripLeadingTicketPrefixIfSame(description, tickets) + description = fmt.Sprintf("[%s] %s", tickets, description) + } + return fmt.Sprintf("%s _(current: %s)_", description, strings.TrimSpace(item.Status)) +} + +func canonicalTicketIDs(ticketIDs string) string { + if strings.TrimSpace(ticketIDs) == "" { + return "" + } + parts := strings.Split(ticketIDs, ",") + cleaned := make([]string, 0, len(parts)) + seen := make(map[string]bool, len(parts)) + for _, part := range parts { + part = strings.TrimSpace(strings.Trim(part, "[]")) + if part == "" { continue } + key := strings.ToLower(part) + if seen[key] { + continue + } + seen[key] = true + cleaned = append(cleaned, part) + } + return strings.Join(cleaned, ",") +} + +func stripLeadingTicketPrefixIfSame(description, tickets string) string { + description = strings.TrimSpace(description) + if description == "" || tickets == "" { + return description + } + + if strings.HasPrefix(description, "[") { + end := strings.Index(description, "]") + if end > 1 { + leading := canonicalTicketIDs(description[1:end]) + if strings.EqualFold(leading, tickets) { + return strings.TrimSpace(description[end+1:]) + } + } + } + return description +} - _, _, err = api.PostMessage(channel.ID, slack.MsgOptionText(msg, false)) - if err != nil { - log.Printf("Error sending nudge to %s: %v", userID, err) +func stripMarkdownFormatting(s string) string { + replacer := strings.NewReplacer("`", "", "*", "", "_", "") + return replacer.Replace(s) +} + +func normalizeNameTokens(s string) []string { + if s == "" { + return nil + } + s = parenPattern.ReplaceAllString(s, " ") + s = strings.ToLower(s) + var b strings.Builder + for _, r := range s { + if (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') { + b.WriteRune(r) } else { - log.Printf("Sent nudge to %s", userID) + b.WriteRune(' ') } } + parts := strings.Fields(b.String()) + if len(parts) == 0 { + return nil + } + return parts } -func SendNudges(api *slack.Client, cfg Config, memberIDs []string, reportChannelID string) { - sendNudges(api, cfg, memberIDs, reportChannelID) +func nameMatches(a, b string) bool { + at := normalizeNameTokens(a) + bt := normalizeNameTokens(b) + if len(at) == 0 || len(bt) == 0 { + return false + } + return allIn(at, bt) || allIn(bt, at) +} + +func allIn(needles, haystack []string) bool { + set := make(map[string]bool, len(haystack)) + for _, t := range haystack { + set[t] = true + } + for _, t := range needles { + if !set[t] { + return false + } + } + return true } func parseTime(s string) (int, int, error) { diff --git a/internal/nudge/nudge_render_test.go b/internal/nudge/nudge_render_test.go new file mode 100644 index 0000000..00c65bb --- /dev/null +++ b/internal/nudge/nudge_render_test.go @@ -0,0 +1,263 @@ +package nudge + +import ( + "bytes" + "database/sql" + "encoding/json" + "io" + "net/http" + "net/url" + "path/filepath" + sqlitedb "reportbot/internal/storage/sqlite" + "strings" + "testing" + "time" + + "github.com/slack-go/slack" +) + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (fn roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { + return fn(req) +} + +func newRenderTestDB(t *testing.T) *sql.DB { + t.Helper() + dbPath := filepath.Join(t.TempDir(), "nudge-test.db") + db, err := sqlitedb.InitDB(dbPath) + if err != nil { + t.Fatalf("init db: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + return db +} + +func newNudgeMockSlackAPI(t *testing.T, users map[string]map[string]string) *slack.Client { + t.Helper() + httpClient := &http.Client{ + Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + form, err := requestForm(req) + if err != nil { + return nil, err + } + + payload := map[string]any{"ok": true} + switch strings.TrimPrefix(req.URL.Path, "/api/") { + case "users.info": + userID := form.Get("user") + profile := users[userID] + if profile == nil { + payload = map[string]any{"ok": false, "error": "user_not_found"} + break + } + payload = map[string]any{ + "ok": true, + "user": map[string]any{ + "id": userID, + "name": profile["name"], + "real_name": profile["real_name"], + "profile": map[string]any{ + "display_name": profile["display_name"], + }, + }, + } + } + return jsonResponse(req, payload) + }), + } + return slack.New( + "xoxb-test", + slack.OptionAPIURL("https://slack.test/api/"), + slack.OptionHTTPClient(httpClient), + ) +} + +func requestForm(req *http.Request) (url.Values, error) { + values := req.URL.Query() + if req.Body == nil { + return values, nil + } + body, err := io.ReadAll(req.Body) + if err != nil { + return nil, err + } + if len(body) == 0 { + return values, nil + } + parsed, err := url.ParseQuery(string(body)) + if err != nil { + return nil, err + } + for key, vals := range parsed { + for _, val := range vals { + values.Add(key, val) + } + } + req.Body = io.NopCloser(bytes.NewReader(body)) + return values, nil +} + +func jsonResponse(req *http.Request, payload any) (*http.Response, error) { + body, err := json.Marshal(payload) + if err != nil { + return nil, err + } + return &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: io.NopCloser(bytes.NewReader(body)), + Request: req, + }, nil +} + +func TestRenderNudgeForUser_GenericOnlyWhenNoActiveItems(t *testing.T) { + db := newRenderTestDB(t) + api := newNudgeMockSlackAPI(t, map[string]map[string]string{ + "U_MEMBER": { + "name": "member", + "real_name": "Member Real", + "display_name": "Member Display", + }, + }) + + now := time.Date(2026, 3, 3, 9, 0, 0, 0, time.UTC) + if err := sqlitedb.InsertWorkItem(db, sqlitedb.WorkItem{ + Description: "Already shipped", + Author: "Member Display", + AuthorID: "U_MEMBER", + Source: "slack", + Status: "done", + ReportedAt: now, + }); err != nil { + t.Fatalf("insert work item: %v", err) + } + + cfg := Config{Location: time.UTC} + rendered, err := RenderNudgeForUser(api, db, cfg, "U_MEMBER", "C_REPORT", now, 0, false) + if err != nil { + t.Fatalf("RenderNudgeForUser returned error: %v", err) + } + + if !strings.Contains(rendered.Text, "Friendly reminder to report your work items") { + t.Fatalf("expected generic reminder text, got %q", rendered.Text) + } + if strings.Contains(rendered.Text, "still marked") { + t.Fatalf("did not expect active-item copy in generic nudge: %q", rendered.Text) + } + if len(rendered.Blocks) != 1 { + t.Fatalf("expected one generic block, got %d", len(rendered.Blocks)) + } +} + +func TestRenderNudgeForUser_PaginatesAndUsesAllSources(t *testing.T) { + db := newRenderTestDB(t) + api := newNudgeMockSlackAPI(t, map[string]map[string]string{ + "U_MEMBER": { + "name": "pat", + "real_name": "Pat User", + "display_name": "Pat", + }, + }) + now := time.Date(2026, 3, 3, 9, 0, 0, 0, time.UTC) + + insert := func(item sqlitedb.WorkItem) { + t.Helper() + if err := sqlitedb.InsertWorkItem(db, item); err != nil { + t.Fatalf("insert work item: %v", err) + } + } + + insert(sqlitedb.WorkItem{ + Description: "Slack task", + Author: "Pat", + AuthorID: "U_MEMBER", + Source: "slack", + Status: "in progress", + ReportedAt: now, + }) + insert(sqlitedb.WorkItem{ + Description: "GitLab task", + Author: "Pat User", + Source: "gitlab", + Status: "in progress", + ReportedAt: now.Add(1 * time.Minute), + }) + insert(sqlitedb.WorkItem{ + Description: "GitHub task", + Author: "Pat", + Source: "github", + Status: "resolved in session; root cause analysis in progress", + ReportedAt: now.Add(2 * time.Minute), + }) + // Older in-progress row should be hidden by newer done row with same dedupe key. + insert(sqlitedb.WorkItem{ + Description: "[7001] Duplicate task", + Author: "Pat", + Source: "gitlab", + Status: "in progress", + TicketIDs: "7001", + ReportedAt: now.Add(3 * time.Minute), + }) + insert(sqlitedb.WorkItem{ + Description: "[7001] Duplicate task", + Author: "Pat", + Source: "gitlab", + Status: "done", + TicketIDs: "7001", + ReportedAt: now.Add(4 * time.Minute), + }) + for i := 0; i < 9; i++ { + insert(sqlitedb.WorkItem{ + Description: "Paginated task " + string(rune('A'+i)), + Author: "Pat", + Source: "slack", + Status: "in progress", + ReportedAt: now.Add(time.Duration(10+i) * time.Minute), + }) + } + + cfg := Config{Location: time.UTC} + firstPage, err := RenderNudgeForUser(api, db, cfg, "U_MEMBER", "", now, 0, false) + if err != nil { + t.Fatalf("RenderNudgeForUser first page error: %v", err) + } + if !strings.Contains(firstPage.Text, "Showing 1-10 of 12 active items") { + t.Fatalf("expected first-page summary, got %q", firstPage.Text) + } + if !strings.Contains(firstPage.Text, "GitLab task") || !strings.Contains(firstPage.Text, "GitHub task") { + t.Fatalf("expected all-source active items in rendered text, got %q", firstPage.Text) + } + if strings.Contains(firstPage.Text, "Duplicate task") { + t.Fatalf("expected deduped latest done item to be excluded, got %q", firstPage.Text) + } + + secondPage, err := RenderNudgeForUser(api, db, cfg, "U_MEMBER", "", now, 99, false) + if err != nil { + t.Fatalf("RenderNudgeForUser second page error: %v", err) + } + if !strings.Contains(secondPage.Text, "Showing 11-12 of 12 active items") { + t.Fatalf("expected clamped second-page summary, got %q", secondPage.Text) + } +} + +func TestRenderNudgeForUser_UpdatedNoItems(t *testing.T) { + db := newRenderTestDB(t) + api := newNudgeMockSlackAPI(t, map[string]map[string]string{ + "U_MEMBER": { + "name": "member", + "real_name": "Member Real", + "display_name": "Member Display", + }, + }) + now := time.Date(2026, 3, 3, 9, 0, 0, 0, time.UTC) + cfg := Config{Location: time.UTC} + + rendered, err := RenderNudgeForUser(api, db, cfg, "U_MEMBER", "", now, 0, true) + if err != nil { + t.Fatalf("RenderNudgeForUser returned error: %v", err) + } + if !strings.Contains(rendered.Text, "Updated. No items are currently marked in progress.") { + t.Fatalf("expected updated no-items text, got %q", rendered.Text) + } +} diff --git a/internal/storage/sqlite/db.go b/internal/storage/sqlite/db.go index 777c8cd..928846a 100644 --- a/internal/storage/sqlite/db.go +++ b/internal/storage/sqlite/db.go @@ -209,6 +209,16 @@ func UpdateWorkItemTextAndStatus(db *sql.DB, id int64, description, status strin return err } +func UpdateWorkItemStatus(db *sql.DB, id int64, status string) error { + _, err := db.Exec( + `UPDATE work_items + SET status = ? + WHERE id = ?`, + status, id, + ) + return err +} + func DeleteWorkItemByID(db *sql.DB, id int64) error { _, err := db.Exec(`DELETE FROM work_items WHERE id = ?`, id) return err diff --git a/internal/storage/sqlite/db_test.go b/internal/storage/sqlite/db_test.go index 6c83150..b267f67 100644 --- a/internal/storage/sqlite/db_test.go +++ b/internal/storage/sqlite/db_test.go @@ -151,6 +151,9 @@ func TestWorkItemCRUDAndQueries(t *testing.T) { if err := UpdateWorkItemTextAndStatus(db, updateID, "Implement feature A v2", "in testing"); err != nil { t.Fatalf("UpdateWorkItemTextAndStatus failed: %v", err) } + if err := UpdateWorkItemStatus(db, updateID, "done"); err != nil { + t.Fatalf("UpdateWorkItemStatus failed: %v", err) + } if err := UpdateCategories(db, map[int64]string{updateID: "S0_0"}); err != nil { t.Fatalf("UpdateCategories failed: %v", err) } @@ -168,7 +171,7 @@ func TestWorkItemCRUDAndQueries(t *testing.T) { if updated.Description != "Implement feature A v2" { t.Fatalf("unexpected updated description: %q", updated.Description) } - if updated.Status != "in testing" { + if updated.Status != "done" { t.Fatalf("unexpected updated status: %q", updated.Status) } if updated.TicketIDs != "123456" { From 69bef6025f9b27e43d25b7a0b684426179eb7c85 Mon Sep 17 00:00:00 2001 From: Codex Date: Sun, 1 Mar 2026 13:35:05 -0800 Subject: [PATCH 2/7] Address PR review feedback --- internal/integrations/llm/llm.go | 2 +- .../slack/nudge_interaction_test.go | 24 ++++++++++++++++--- internal/integrations/slack/slack.go | 1 + internal/nudge/nudge.go | 2 -- 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/internal/integrations/llm/llm.go b/internal/integrations/llm/llm.go index 939cca5..d9a7472 100644 --- a/internal/integrations/llm/llm.go +++ b/internal/integrations/llm/llm.go @@ -371,7 +371,7 @@ func parseSectionClassifiedResponse(responseText string) (map[int64]LLMSectionDe return nil, fmt.Errorf("parsing LLM section response: %w (response: %s)", err, responseText) } if repairErr := json.Unmarshal([]byte(repaired), &classified); repairErr != nil { - return nil, fmt.Errorf("parsing LLM section response: %w (response: %s)", err, responseText) + return nil, fmt.Errorf("parsing LLM section response after repair: %w (original error: %v, response: %s)", repairErr, err, repaired) } log.Printf("llm section response repaired before parse") } diff --git a/internal/integrations/slack/nudge_interaction_test.go b/internal/integrations/slack/nudge_interaction_test.go index 98a7fb1..1787ed4 100644 --- a/internal/integrations/slack/nudge_interaction_test.go +++ b/internal/integrations/slack/nudge_interaction_test.go @@ -173,7 +173,13 @@ func TestHandleBlockActions_NudgeMoreUpdatesStatus(t *testing.T) { }); err != nil { t.Fatalf("insert work item: %v", err) } - items, _ := GetItemsByDateRange(db, now.Add(-time.Hour), now.Add(time.Hour)) + items, err := GetItemsByDateRange(db, now.Add(-time.Hour), now.Add(time.Hour)) + if err != nil { + t.Fatalf("GetItemsByDateRange failed: %v", err) + } + if len(items) == 0 { + t.Fatal("expected at least one work item in date range") + } rec := &nudgeUpdateRecorder{} api := newMockSlackAPIForNudgeInteractions(t, rec) @@ -222,7 +228,13 @@ func TestHandleBlockActions_NudgePageNextRefreshesWithoutMutation(t *testing.T) t.Fatalf("insert work item %d: %v", i, err) } } - items, _ := GetItemsByDateRange(db, now.Add(-time.Hour), now.Add(2*time.Hour)) + items, err := GetItemsByDateRange(db, now.Add(-time.Hour), now.Add(2*time.Hour)) + if err != nil { + t.Fatalf("GetItemsByDateRange failed: %v", err) + } + if len(items) == 0 { + t.Fatal("expected at least one work item in date range") + } rec := &nudgeUpdateRecorder{} api := newMockSlackAPIForNudgeInteractions(t, rec) @@ -318,7 +330,13 @@ func TestHandleBlockActions_NudgeUnauthorizedUserCannotUpdate(t *testing.T) { }); err != nil { t.Fatalf("insert work item: %v", err) } - items, _ := GetItemsByDateRange(db, now.Add(-time.Hour), now.Add(time.Hour)) + items, err := GetItemsByDateRange(db, now.Add(-time.Hour), now.Add(time.Hour)) + if err != nil { + t.Fatalf("GetItemsByDateRange failed: %v", err) + } + if len(items) == 0 { + t.Fatal("expected at least one work item in date range") + } rec := &nudgeUpdateRecorder{} api := newMockSlackAPIForNudgeInteractions(t, rec) diff --git a/internal/integrations/slack/slack.go b/internal/integrations/slack/slack.go index 26ff8d1..b06f4f6 100644 --- a/internal/integrations/slack/slack.go +++ b/internal/integrations/slack/slack.go @@ -1708,6 +1708,7 @@ func authorizeNudgeAction(api *slack.Client, cb slack.InteractionCallback, targe func canActOnNudgeItem(api *slack.Client, db *sql.DB, cfg Config, itemID int64, targetUserID string) bool { item, err := GetWorkItemByID(db, itemID) if err != nil { + log.Printf("canActOnNudgeItem: failed to get work item id=%d: %v", itemID, err) return false } monday, nextMonday := ReportWeekRange(cfg, time.Now().In(cfg.Location)) diff --git a/internal/nudge/nudge.go b/internal/nudge/nudge.go index d01ae95..188d0da 100644 --- a/internal/nudge/nudge.go +++ b/internal/nudge/nudge.go @@ -28,7 +28,6 @@ type RenderedNudge struct { } type renderedNudgeState struct { - active []WorkItem page int pageStart int pageEnd int @@ -274,7 +273,6 @@ func buildNudgeState(items []WorkItem, userID string, user *slack.User, requeste page := clampPage(requestedPage, totalPages) start, end := pageBounds(len(active), page) return active, renderedNudgeState{ - active: active, page: page, pageStart: start, pageEnd: end, From 7a9a478a1e3979c9cbddaf2540012b6d85c5623b Mon Sep 17 00:00:00 2001 From: Codex Date: Sun, 1 Mar 2026 15:56:17 -0800 Subject: [PATCH 3/7] Add test nudge slash command --- README.md | 9 ++ .../slack/nudge_interaction_test.go | 15 ++-- internal/integrations/slack/slack.go | 84 +++++++++++++++++++ .../integrations/slack/slack_logic_test.go | 67 +++++++++++++++ 4 files changed, 170 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index e5d098e..fc843cb 100644 --- a/README.md +++ b/README.md @@ -109,6 +109,7 @@ See [docs/agentic-features-overview.md](docs/agentic-features-overview.md) for a | `/gen` | Alias of `/generate-report` | | `/list` | List this week's work items | | `/check` | List missing members with nudge buttons | + | `/nudge` | Send a test nudge DM (self by default; managers can target one member) | | `/retrospect` | Analyze corrections and suggest improvements | | `/stats` | View classification accuracy dashboard | | `/help` | Show help and usage | @@ -376,6 +377,14 @@ Anyone can view this week's items: **On-demand**: `/check` lists team members who haven't reported this week, with a "Nudge" button next to each member and a "Nudge All" button at the bottom. Clicking opens a confirmation before sending the DM. +**Testing**: + +```text +/nudge # Send a test nudge to your own DM +/nudge Member Name # Manager only: send a test nudge to one member +/nudge U123ABC456 # Manager only: target a Slack user ID directly +``` + On Monday before `monday_cutoff_time` (default `12:00`) in configured `timezone`, report commands use the previous calendar week. Accepts any day name: `Monday`, `Tuesday`, ..., `Sunday`. diff --git a/internal/integrations/slack/nudge_interaction_test.go b/internal/integrations/slack/nudge_interaction_test.go index 1787ed4..5299631 100644 --- a/internal/integrations/slack/nudge_interaction_test.go +++ b/internal/integrations/slack/nudge_interaction_test.go @@ -20,6 +20,11 @@ type nudgeUpdateRecorder struct { ephemeralCalls int } +func nudgeTestReportedAt() time.Time { + monday, _ := ReportWeekRange(Config{Location: time.UTC, MondayCutoffTime: "12:00"}, time.Now().UTC()) + return monday.Add(48 * time.Hour).Truncate(time.Second) +} + type mockSlackRoundTripper func(*http.Request) (*http.Response, error) func (fn mockSlackRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { @@ -114,7 +119,7 @@ func mockSlackJSONResponse(req *http.Request, payload any) (*http.Response, erro func TestHandleBlockActions_NudgeDoneUpdatesStatusAndRefreshesMessage(t *testing.T) { db := newTestDB(t) - now := time.Now().UTC().Truncate(time.Second) + now := nudgeTestReportedAt() if err := InsertWorkItem(db, WorkItem{ Description: "Close stale deployment work", Author: "Pat", @@ -162,7 +167,7 @@ func TestHandleBlockActions_NudgeDoneUpdatesStatusAndRefreshesMessage(t *testing func TestHandleBlockActions_NudgeMoreUpdatesStatus(t *testing.T) { db := newTestDB(t) - now := time.Now().UTC().Truncate(time.Second) + now := nudgeTestReportedAt() if err := InsertWorkItem(db, WorkItem{ Description: "Validate build pipeline", Author: "Pat", @@ -215,7 +220,7 @@ func TestHandleBlockActions_NudgeMoreUpdatesStatus(t *testing.T) { func TestHandleBlockActions_NudgePageNextRefreshesWithoutMutation(t *testing.T) { db := newTestDB(t) - now := time.Now().UTC().Truncate(time.Second) + now := nudgeTestReportedAt() for i := 0; i < 11; i++ { if err := InsertWorkItem(db, WorkItem{ Description: fmt.Sprintf("Paginated active item %02d", i+1), @@ -268,7 +273,7 @@ func TestHandleBlockActions_NudgePageNextRefreshesWithoutMutation(t *testing.T) func TestHandleBlockActions_NudgeDoneClampsPageAfterLastItemRemoved(t *testing.T) { db := newTestDB(t) - now := time.Now().UTC().Truncate(time.Second) + now := nudgeTestReportedAt() for i := 0; i < 11; i++ { if err := InsertWorkItem(db, WorkItem{ Description: fmt.Sprintf("Paginated active item %02d", i+1), @@ -319,7 +324,7 @@ func TestHandleBlockActions_NudgeDoneClampsPageAfterLastItemRemoved(t *testing.T func TestHandleBlockActions_NudgeUnauthorizedUserCannotUpdate(t *testing.T) { db := newTestDB(t) - now := time.Now().UTC().Truncate(time.Second) + now := nudgeTestReportedAt() if err := InsertWorkItem(db, WorkItem{ Description: "Protected item", Author: "Pat", diff --git a/internal/integrations/slack/slack.go b/internal/integrations/slack/slack.go index b06f4f6..bc1a4cb 100644 --- a/internal/integrations/slack/slack.go +++ b/internal/integrations/slack/slack.go @@ -102,6 +102,8 @@ func handleSlashCommand(client *socketmode.Client, api *slack.Client, db *sql.DB handleListItems(api, db, cfg, cmd) case "/check": handleListMissing(api, db, cfg, cmd) + case "/nudge": + handleTestNudge(api, db, cfg, cmd) case "/retrospect": handleRetrospective(api, db, cfg, cmd) case "/stats": @@ -393,6 +395,43 @@ func handleFetchMRs(api *slack.Client, db *sql.DB, cfg Config, cmd slack.SlashCo log.Printf("fetch inserted=%d alreadyTracked=%d skippedNonTeam=%d", result.Inserted, result.AlreadyTracked, result.SkippedNonTeam) } +func handleTestNudge(api *slack.Client, db *sql.DB, cfg Config, cmd slack.SlashCommand) { + targetID := cmd.UserID + targetLabel := "yourself" + rawTarget := strings.TrimSpace(cmd.Text) + + if rawTarget != "" { + isManager, err := isManagerUser(api, cfg, cmd.UserID) + if err != nil { + postEphemeral(api, cmd, fmt.Sprintf("Error checking permissions: %v", err)) + log.Printf("nudge auth error user=%s: %v", cmd.UserID, err) + return + } + if !isManager { + postEphemeral(api, cmd, "Usage: /nudge\nManagers can also use `/nudge ` to test another member's nudge DM.") + return + } + + resolvedID, resolvedLabel, err := resolveNudgeTarget(api, cfg, rawTarget) + if err != nil { + postEphemeral(api, cmd, fmt.Sprintf("Unable to resolve nudge target %q: %v", rawTarget, err)) + return + } + targetID = resolvedID + targetLabel = resolvedLabel + } + + sendNudges(api, db, cfg, []string{targetID}, cfg.ReportChannelID) + if targetID == cmd.UserID { + postEphemeral(api, cmd, "Sent a test nudge to your DM.") + log.Printf("test nudge sent user=%s target=self", cmd.UserID) + return + } + + postEphemeral(api, cmd, fmt.Sprintf("Sent a test nudge to %s.", targetLabel)) + log.Printf("test nudge sent user=%s target=%s", cmd.UserID, targetID) +} + // deriveBossReportFromTeamReport attempts to derive a boss report from an existing team report file. // It returns: // - (filePath, bossReport, nil) if the team report exists and was successfully processed @@ -1946,6 +1985,7 @@ func handleHelp(api *slack.Client, cfg Config, cmd slack.SlashCommand) { ">(in progress)```", "", "`/list` — List this week's items.", + "`/nudge` — Send yourself a test nudge DM.", "`/help` — Show this help.", } @@ -1958,6 +1998,7 @@ func handleHelp(api *slack.Client, cfg Config, cmd slack.SlashCommand) { "`/generate-report [team|boss] [private]` — Generate weekly report (channel by default).", "`/gen` — Alias of `/generate-report`.", "`/check` — List missing members with inline nudge buttons.", + "`/nudge ` — Send a test nudge DM to one member.", "`/retrospect` — Analyze recent corrections and suggest improvements.", "`/stats` — Show classification accuracy dashboard.", ) @@ -1970,6 +2011,49 @@ func isManagerUser(_ *slack.Client, cfg Config, userID string) (bool, error) { return cfg.IsManagerID(userID), nil } +func resolveNudgeTarget(api *slack.Client, cfg Config, input string) (string, string, error) { + input = strings.TrimSpace(input) + if input == "" { + return "", "", fmt.Errorf("empty target") + } + + if isLikelySlackID(input) { + if user, err := api.GetUserInfo(input); err == nil { + label := strings.TrimSpace(user.Profile.DisplayName) + if label == "" { + label = strings.TrimSpace(user.RealName) + } + if label == "" { + label = fmt.Sprintf("<@%s>", input) + } + return input, label, nil + } + return input, fmt.Sprintf("<@%s>", input), nil + } + + if member, ok := resolveDelegatedAuthorName(input, cfg.TeamMembers); ok { + ids, unresolved, err := resolveUserIDs(api, []string{member}) + if err == nil && len(ids) == 1 && len(unresolved) == 0 { + return ids[0], member, nil + } + } + + ids, unresolved, err := resolveUserIDs(api, []string{input}) + if err != nil && len(ids) == 0 { + return "", "", err + } + if len(ids) == 1 { + return ids[0], input, nil + } + if len(ids) > 1 { + return "", "", fmt.Errorf("resolved to multiple Slack users") + } + if len(unresolved) > 0 { + return "", "", fmt.Errorf("no matching Slack user found") + } + return "", "", fmt.Errorf("no matching Slack user found") +} + func resolveDelegatedAuthorName(input string, teamMembers []string) (string, bool) { input = strings.TrimSpace(input) if input == "" || len(teamMembers) == 0 { diff --git a/internal/integrations/slack/slack_logic_test.go b/internal/integrations/slack/slack_logic_test.go index f3a23b0..47a8298 100644 --- a/internal/integrations/slack/slack_logic_test.go +++ b/internal/integrations/slack/slack_logic_test.go @@ -2,11 +2,14 @@ package slackbot import ( "fmt" + "net/http" "os" "path/filepath" "strings" "testing" "time" + + "github.com/slack-go/slack" ) func TestParseReportItemsSingleAndSharedStatus(t *testing.T) { @@ -106,6 +109,70 @@ func TestResolveDelegatedAuthorName(t *testing.T) { } } +func TestResolveNudgeTarget(t *testing.T) { + api := slack.New( + "xoxb-test", + slack.OptionAPIURL("https://slack.test/api/"), + slack.OptionHTTPClient(&http.Client{ + Transport: mockSlackRoundTripper(func(req *http.Request) (*http.Response, error) { + form, err := mockSlackRequestForm(req) + if err != nil { + return nil, err + } + switch strings.TrimPrefix(req.URL.Path, "/api/") { + case "users.info": + userID := form.Get("user") + return mockSlackJSONResponse(req, map[string]any{ + "ok": true, + "user": map[string]any{ + "id": userID, + "name": "pat", + "real_name": "Pat Example", + "profile": map[string]any{ + "display_name": "Pat Example", + }, + }, + }) + case "users.list": + return mockSlackJSONResponse(req, map[string]any{ + "ok": true, + "members": []map[string]any{ + { + "id": "U0PAT1234", + "name": "pat", + "real_name": "Pat Example", + "profile": map[string]any{ + "display_name": "Pat Example", + }, + }, + }, + }) + default: + return mockSlackJSONResponse(req, map[string]any{"ok": true}) + } + }), + }), + ) + + cfg := Config{TeamMembers: []string{"Pat Example"}} + + id, label, err := resolveNudgeTarget(api, cfg, "Pat") + if err != nil { + t.Fatalf("resolveNudgeTarget by name failed: %v", err) + } + if id != "U0PAT1234" || label != "Pat Example" { + t.Fatalf("unexpected name resolution: id=%q label=%q", id, label) + } + + id, label, err = resolveNudgeTarget(api, cfg, "U0PAT1234") + if err != nil { + t.Fatalf("resolveNudgeTarget by ID failed: %v", err) + } + if id != "U0PAT1234" || label != "Pat Example" { + t.Fatalf("unexpected ID resolution: id=%q label=%q", id, label) + } +} + func TestMapMRStatusAndReportedAt(t *testing.T) { base := time.Date(2026, 2, 20, 10, 0, 0, 0, time.UTC) From f1195e0ed1bee6d32586c4c04e0ab93923d10ea0 Mon Sep 17 00:00:00 2001 From: Weizhi Li Date: Sun, 1 Mar 2026 16:00:00 -0800 Subject: [PATCH 4/7] Update internal/integrations/slack/slack.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- internal/integrations/slack/slack.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/integrations/slack/slack.go b/internal/integrations/slack/slack.go index bc1a4cb..29e8a1b 100644 --- a/internal/integrations/slack/slack.go +++ b/internal/integrations/slack/slack.go @@ -1747,6 +1747,7 @@ func authorizeNudgeAction(api *slack.Client, cb slack.InteractionCallback, targe func canActOnNudgeItem(api *slack.Client, db *sql.DB, cfg Config, itemID int64, targetUserID string) bool { item, err := GetWorkItemByID(db, itemID) if err != nil { + log.Printf("canActOnNudgeItem: failed to get work item id=%d: %v", itemID, err) log.Printf("canActOnNudgeItem: failed to get work item id=%d: %v", itemID, err) return false } From ed983a3b8e8a67b4232aeeecc07c732e4bf9f7cf Mon Sep 17 00:00:00 2001 From: Codex Date: Sun, 1 Mar 2026 16:53:35 -0800 Subject: [PATCH 5/7] Tighten nudge item layout --- .../slack/nudge_interaction_test.go | 10 ++-- internal/integrations/slack/slack.go | 12 +++-- .../integrations/slack/slack_logic_test.go | 15 ++++++ internal/nudge/nudge.go | 51 ++++++++++++++----- internal/nudge/nudge_test.go | 23 +++++++++ 5 files changed, 90 insertions(+), 21 deletions(-) diff --git a/internal/integrations/slack/nudge_interaction_test.go b/internal/integrations/slack/nudge_interaction_test.go index 5299631..04fc1b7 100644 --- a/internal/integrations/slack/nudge_interaction_test.go +++ b/internal/integrations/slack/nudge_interaction_test.go @@ -137,7 +137,7 @@ func TestHandleBlockActions_NudgeDoneUpdatesStatusAndRefreshesMessage(t *testing rec := &nudgeUpdateRecorder{} api := newMockSlackAPIForNudgeInteractions(t, rec) - cfg := Config{Location: time.UTC} + cfg := Config{Location: time.UTC, MondayCutoffTime: "12:00"} cb := slack.InteractionCallback{ User: slack.User{ID: "U_MEMBER"}, Container: slack.Container{ChannelID: "D123", MessageTs: "123.45"}, @@ -188,7 +188,7 @@ func TestHandleBlockActions_NudgeMoreUpdatesStatus(t *testing.T) { rec := &nudgeUpdateRecorder{} api := newMockSlackAPIForNudgeInteractions(t, rec) - cfg := Config{Location: time.UTC} + cfg := Config{Location: time.UTC, MondayCutoffTime: "12:00"} cb := slack.InteractionCallback{ User: slack.User{ID: "U_MEMBER"}, Container: slack.Container{ChannelID: "D123", MessageTs: "123.45"}, @@ -243,7 +243,7 @@ func TestHandleBlockActions_NudgePageNextRefreshesWithoutMutation(t *testing.T) rec := &nudgeUpdateRecorder{} api := newMockSlackAPIForNudgeInteractions(t, rec) - cfg := Config{Location: time.UTC} + cfg := Config{Location: time.UTC, MondayCutoffTime: "12:00"} cb := slack.InteractionCallback{ User: slack.User{ID: "U_MEMBER"}, Container: slack.Container{ChannelID: "D123", MessageTs: "123.45"}, @@ -293,7 +293,7 @@ func TestHandleBlockActions_NudgeDoneClampsPageAfterLastItemRemoved(t *testing.T rec := &nudgeUpdateRecorder{} api := newMockSlackAPIForNudgeInteractions(t, rec) - cfg := Config{Location: time.UTC} + cfg := Config{Location: time.UTC, MondayCutoffTime: "12:00"} lastItem := items[len(items)-1] cb := slack.InteractionCallback{ User: slack.User{ID: "U_MEMBER"}, @@ -345,7 +345,7 @@ func TestHandleBlockActions_NudgeUnauthorizedUserCannotUpdate(t *testing.T) { rec := &nudgeUpdateRecorder{} api := newMockSlackAPIForNudgeInteractions(t, rec) - cfg := Config{Location: time.UTC} + cfg := Config{Location: time.UTC, MondayCutoffTime: "12:00"} cb := slack.InteractionCallback{ User: slack.User{ID: "U_OTHER"}, Container: slack.Container{ChannelID: "D123", MessageTs: "123.45"}, diff --git a/internal/integrations/slack/slack.go b/internal/integrations/slack/slack.go index 29e8a1b..bbd33e7 100644 --- a/internal/integrations/slack/slack.go +++ b/internal/integrations/slack/slack.go @@ -809,7 +809,8 @@ func renderListItems(api *slack.Client, db *sql.DB, cfg Config, channelID, userI isManager, _ := isManagerUser(api, cfg, userID) user, _ := api.GetUserInfo(userID) - for _, item := range items[start:end] { + for idx, item := range items[start:end] { + lineNumber := start + idx + 1 source := "" switch item.Source { case "gitlab": @@ -821,9 +822,7 @@ func renderListItems(api *slack.Client, db *sql.DB, cfg Config, channelID, userI if item.Category != "" { category = fmt.Sprintf(" _%s_", item.Category) } - description := formatItemDescriptionForList(item) - text := fmt.Sprintf("*%s*: %s (%s)%s%s", - item.Author, description, item.Status, source, category) + text := formatListItemText(lineNumber, item, source, category) if canManageItem(item, isManager, user) { editOpt := slack.NewOptionBlockObject( fmt.Sprintf("edit:%d", item.ID), @@ -1531,6 +1530,11 @@ func formatItemDescriptionForList(item WorkItem) string { return fmt.Sprintf("[%s] %s", tickets, description) } +func formatListItemText(lineNumber int, item WorkItem, source, category string) string { + return fmt.Sprintf("%d. *%s*: %s (%s)%s%s", + lineNumber, item.Author, formatItemDescriptionForList(item), item.Status, source, category) +} + func leadingTicketPrefix(description string) (string, bool) { description = strings.TrimSpace(description) if !strings.HasPrefix(description, "[") { diff --git a/internal/integrations/slack/slack_logic_test.go b/internal/integrations/slack/slack_logic_test.go index 47a8298..b05b8fb 100644 --- a/internal/integrations/slack/slack_logic_test.go +++ b/internal/integrations/slack/slack_logic_test.go @@ -250,6 +250,21 @@ func TestFormatItemDescriptionForList(t *testing.T) { } } +func TestFormatListItemText_AddsLineNumber(t *testing.T) { + item := WorkItem{ + Author: "Alex Rivera", + Description: "Tune query timeout for widget pipeline", + Status: "in progress", + TicketIDs: "7003001", + } + + got := formatListItemText(7, item, " [GitLab]", " _Support Cases_") + want := "7. *Alex Rivera*: [7003001] Tune query timeout for widget pipeline (in progress) [GitLab] _Support Cases_" + if got != want { + t.Fatalf("formatListItemText() = %q, want %q", got, want) + } +} + func TestMemberReportedThisWeek(t *testing.T) { reportedIDs := map[string]bool{"U123": true} reportedAuthors := []string{"Alex Rivera", "Jordan Patel"} diff --git a/internal/nudge/nudge.go b/internal/nudge/nudge.go index 188d0da..e87019f 100644 --- a/internal/nudge/nudge.go +++ b/internal/nudge/nudge.go @@ -13,11 +13,12 @@ import ( ) const ( - ActionDone = "nudge_done" - ActionMore = "nudge_more" - ActionPagePrev = "nudge_page_prev" - ActionPageNext = "nudge_page_next" - nudgePageSize = 10 + ActionDone = "nudge_done" + ActionMore = "nudge_more" + ActionPagePrev = "nudge_page_prev" + ActionPageNext = "nudge_page_next" + nudgePageSize = 10 + nudgeItemMaxRunes = 110 ) var parenPattern = regexp.MustCompile(`\([^)]*\)|（[^）]*）`) @@ -196,17 +197,21 @@ func renderInteractiveNudge(reminder, userID string, active []WorkItem, state re pageItems := active[state.pageStart:state.pageEnd] textLines := []string{stripMarkdownFormatting(reminder), stripMarkdownFormatting(intro)} - for _, item := range pageItems { - itemText := formatNudgeItem(item) + for idx, item := range pageItems { + lineNumber := state.pageStart + idx + 1 + itemText := formatNudgeItem(lineNumber, item) blocks = append(blocks, - slack.NewSectionBlock(slack.NewTextBlockObject(slack.MarkdownType, itemText, false, false), nil, nil), + slack.NewSectionBlock( + slack.NewTextBlockObject(slack.MarkdownType, itemText, false, false), + nil, + slack.NewAccessory(buildDoneButton(userID, item.ID, state.page)), + ), slack.NewActionBlock( fmt.Sprintf("nudge_actions_%d", item.ID), - buildDoneButton(userID, item.ID, state.page), buildMoreMenu(userID, item.ID, state.page), ), ) - textLines = append(textLines, "- "+stripMarkdownFormatting(itemText)) + textLines = append(textLines, stripMarkdownFormatting(itemText)) } summary := fmt.Sprintf("Showing %d-%d of %d active items", state.pageStart+1, state.pageEnd, len(active)) @@ -425,14 +430,18 @@ func buildGenericNudgeText(reportChannelID string, monday, nextMonday time.Time) ) } -func formatNudgeItem(item WorkItem) string { +func formatNudgeItem(lineNumber int, item WorkItem) string { description := strings.TrimSpace(item.Description) tickets := canonicalTicketIDs(item.TicketIDs) if tickets != "" { description = stripLeadingTicketPrefixIfSame(description, tickets) description = fmt.Sprintf("[%s] %s", tickets, description) } - return fmt.Sprintf("%s _(current: %s)_", description, strings.TrimSpace(item.Status)) + statusText := strings.TrimSpace(item.Status) + suffix := fmt.Sprintf(" _(current: %s)_", statusText) + prefix := fmt.Sprintf("%d. ", lineNumber) + description = truncateRunes(description, nudgeItemMaxRunes-runeLen(prefix)-runeLen(suffix)) + return fmt.Sprintf("%s%s%s", prefix, description, suffix) } func canonicalTicketIDs(ticketIDs string) string { @@ -480,6 +489,24 @@ func stripMarkdownFormatting(s string) string { return replacer.Replace(s) } +func truncateRunes(s string, max int) string { + if max <= 0 { + return "" + } + runes := []rune(s) + if len(runes) <= max { + return s + } + if max == 1 { + return "…" + } + return string(runes[:max-1]) + "…" +} + +func runeLen(s string) int { + return len([]rune(s)) +} + func normalizeNameTokens(s string) []string { if s == "" { return nil diff --git a/internal/nudge/nudge_test.go b/internal/nudge/nudge_test.go index e93beeb..064f8fe 100644 --- a/internal/nudge/nudge_test.go +++ b/internal/nudge/nudge_test.go @@ -1,6 +1,7 @@ package nudge import ( + "strings" "testing" "time" ) @@ -49,3 +50,25 @@ func TestNextWeekday(t *testing.T) { t.Fatalf("unexpected nextWeekday cross-day result: got %v want %v", next, want) } } + +func TestFormatNudgeItem_AddsLineNumberAndTruncates(t *testing.T) { + item := WorkItem{ + Description: "This is a very long work item description that should be truncated so the primary action can stay aligned with the text row in Slack.", + Status: "resolved in session; root cause analysis in progress", + TicketIDs: "7003001", + } + + got := formatNudgeItem(3, item) + if got[:3] != "3. " { + t.Fatalf("expected line number prefix, got %q", got) + } + if !strings.Contains(got, "[7003001]") { + t.Fatalf("expected ticket prefix, got %q", got) + } + if !strings.Contains(got, "_(current: resolved in session; root cause analysis in progress)_") { + t.Fatalf("expected status suffix, got %q", got) + } + if !strings.Contains(got, "…") { + t.Fatalf("expected truncated description, got %q", got) + } +} From 19f78547acefafca5be04c4b7ee780acf4dfcbc7 Mon Sep 17 00:00:00 2001 From: Codex Date: Sun, 1 Mar 2026 17:12:28 -0800 Subject: [PATCH 6/7] Stabilize Slack test user cache --- .../slack/functional_slack_github_test.go | 3 +++ .../slack/nudge_interaction_test.go | 1 + .../integrations/slack/slack_logic_test.go | 1 + .../integrations/slack/test_helpers_test.go | 18 ++++++++++++++++-- 4 files changed, 21 insertions(+), 2 deletions(-) diff --git a/internal/integrations/slack/functional_slack_github_test.go b/internal/integrations/slack/functional_slack_github_test.go index f02a463..da95dd2 100644 --- a/internal/integrations/slack/functional_slack_github_test.go +++ b/internal/integrations/slack/functional_slack_github_test.go @@ -139,6 +139,7 @@ func withMockGitHubAPI(t *testing.T) { func newMockSlackAPI(t *testing.T) (*slack.Client, *int) { t.Helper() + resetUserCacheForTest(t) postEphemeralCalls := 0 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -171,6 +172,7 @@ func newMockSlackAPI(t *testing.T) (*slack.Client, *int) { func newMockSlackAPIWithUsers(t *testing.T) *slack.Client { t.Helper() + resetUserCacheForTest(t) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { path := strings.TrimPrefix(r.URL.Path, "/api/") @@ -214,6 +216,7 @@ func newMockSlackAPIWithUsers(t *testing.T) *slack.Client { func newMockSlackAPIWithManagerNotify(t *testing.T) (*slack.Client, *int, *string) { t.Helper() + resetUserCacheForTest(t) managerMsgCalls := 0 lastManagerMsg := "" diff --git a/internal/integrations/slack/nudge_interaction_test.go b/internal/integrations/slack/nudge_interaction_test.go index 04fc1b7..41a7c6f 100644 --- a/internal/integrations/slack/nudge_interaction_test.go +++ b/internal/integrations/slack/nudge_interaction_test.go @@ -33,6 +33,7 @@ func (fn mockSlackRoundTripper) RoundTrip(req *http.Request) (*http.Response, er func newMockSlackAPIForNudgeInteractions(t *testing.T, rec *nudgeUpdateRecorder) *slack.Client { t.Helper() + resetUserCacheForTest(t) httpClient := &http.Client{ Transport: mockSlackRoundTripper(func(req *http.Request) (*http.Response, error) { form, err := mockSlackRequestForm(req) diff --git a/internal/integrations/slack/slack_logic_test.go b/internal/integrations/slack/slack_logic_test.go index b05b8fb..67dffba 100644 --- a/internal/integrations/slack/slack_logic_test.go +++ b/internal/integrations/slack/slack_logic_test.go @@ -110,6 +110,7 @@ func TestResolveDelegatedAuthorName(t *testing.T) { } func TestResolveNudgeTarget(t *testing.T) { + resetUserCacheForTest(t) api := slack.New( "xoxb-test", slack.OptionAPIURL("https://slack.test/api/"), diff --git a/internal/integrations/slack/test_helpers_test.go b/internal/integrations/slack/test_helpers_test.go index f3abac6..557e598 100644 --- a/internal/integrations/slack/test_helpers_test.go +++ b/internal/integrations/slack/test_helpers_test.go @@ -9,11 +9,25 @@ import ( func newTestDB(t *testing.T) *sql.DB { t.Helper() - dbPath := filepath.Join(t.TempDir(), "test.db") + dbPath := filepath.Join(t.TempDir(), "slack-test.db") db, err := sqlitedb.InitDB(dbPath) if err != nil { - t.Fatalf("init test db: %v", err) + t.Fatalf("InitDB failed: %v", err) } t.Cleanup(func() { _ = db.Close() }) return db } + +func resetUserCacheForTest(t *testing.T) { + t.Helper() + userCache.Lock() + userCache.users = nil + userCache.fetchedAt = userCache.fetchedAt.AddDate(-10, 0, 0) + userCache.Unlock() + t.Cleanup(func() { + userCache.Lock() + userCache.users = nil + userCache.fetchedAt = userCache.fetchedAt.AddDate(-10, 0, 0) + userCache.Unlock() + }) +} From aa5d1898988d25a6485a2ff795f802cdb2083d3d Mon Sep 17 00:00:00 2001 From: Codex Date: Sun, 1 Mar 2026 19:03:08 -0800 Subject: [PATCH 7/7] Refine nudge prompt copy --- internal/nudge/nudge.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/internal/nudge/nudge.go b/internal/nudge/nudge.go index e87019f..32a6b16 100644 --- a/internal/nudge/nudge.go +++ b/internal/nudge/nudge.go @@ -201,13 +201,10 @@ func renderInteractiveNudge(reminder, userID string, active []WorkItem, state re lineNumber := state.pageStart + idx + 1 itemText := formatNudgeItem(lineNumber, item) blocks = append(blocks, - slack.NewSectionBlock( - slack.NewTextBlockObject(slack.MarkdownType, itemText, false, false), - nil, - slack.NewAccessory(buildDoneButton(userID, item.ID, state.page)), - ), + slack.NewSectionBlock(slack.NewTextBlockObject(slack.MarkdownType, itemText, false, false), nil, nil), slack.NewActionBlock( fmt.Sprintf("nudge_actions_%d", item.ID), + buildDoneButton(userID, item.ID, state.page), buildMoreMenu(userID, item.ID, state.page), ), ) @@ -412,9 +409,9 @@ func renderIntroText(count int, updated bool) string { return "Updated. These items are still marked `in progress`:" } if count == 1 { - return "This item is still marked `in progress`. Use *Mark as Done* or the *More* menu to update it if its status has changed:" + return "This item is still marked `in progress`. Please update the status if it has changed." } - return "These items are still marked `in progress`. Use *Mark as Done* or the *More* menu to update any that changed:" + return "These items are still marked `in progress`. Please update the status if any have changed." } func buildGenericNudgeText(reportChannelID string, monday, nextMonday time.Time) string {