diff --git a/cmd/server/main.go b/cmd/server/main.go index 8f5b3a8..cf22201 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -693,8 +693,9 @@ func runBotCoordinators( lastHealthCheck: time.Now(), } - // Start initial coordinators - cm.startCoordinators(ctx) + // Discover GitHub installations and start coordinators immediately at startup + slog.Info("discovering GitHub installations at startup") + cm.handleRefreshInstallations(ctx) // Refresh installations every 5 minutes installationTicker := time.NewTicker(5 * time.Minute) diff --git a/pkg/home/ui.go b/pkg/home/ui.go index 453f2b3..14b498c 100644 --- a/pkg/home/ui.go +++ b/pkg/home/ui.go @@ -13,7 +13,7 @@ import ( // BuildBlocks creates Slack Block Kit UI for the home dashboard. // Design matches dashboard at https://ready-to-review.dev - modern minimal with indigo accents. -func BuildBlocks(dashboard *Dashboard) []slack.Block { +func BuildBlocks(dashboard *Dashboard, userTZ string) []slack.Block { var blocks []slack.Block // Header @@ -30,51 +30,53 @@ func BuildBlocks(dashboard *Dashboard) []slack.Block { slack.NewTextBlockObject("plain_text", "🔄 Refresh Dashboard", true, false), ).WithStyle("primary"), ), - slack.NewDividerBlock(), ) - // PR sections - blocks = append(blocks, BuildPRSections(dashboard.IncomingPRs, dashboard.OutgoingPRs)...) - - // Organizations section - blocks = append(blocks, slack.NewDividerBlock()) - - var orgLines []string - for _, org := range dashboard.WorkspaceOrgs { - // URL-escape org name to prevent injection - esc := url.PathEscape(org) - orgLine := fmt.Sprintf("• <%s|%s> [<%s|config>, <%s|dashboard>]", - fmt.Sprintf("https://github.com/%s", esc), - org, - fmt.Sprintf("https://github.com/%s/.github/blob/main/.codeGROOVE/slack.yaml", esc), - fmt.Sprintf("https://%s.ready-to-review.dev", esc), - ) - orgLines = append(orgLines, orgLine) - } - + // Updated timestamp - right after refresh button + now := formatTimestamp(time.Now(), userTZ) blocks = append(blocks, - slack.NewSectionBlock( + slack.NewContextBlock("", slack.NewTextBlockObject("mrkdwn", - "*Organizations*\n"+strings.Join(orgLines, "\n"), + fmt.Sprintf("Updated %s", now), false, false, ), - nil, - nil, ), + slack.NewDividerBlock(), ) - // Updated timestamp - now := time.Now().Format("Jan 2, 3:04pm MST") - blocks = append(blocks, - slack.NewContextBlock("", - slack.NewTextBlockObject("mrkdwn", - fmt.Sprintf("Updated: %s", now), - false, - false, + // PR sections + blocks = append(blocks, BuildPRSections(dashboard.IncomingPRs, dashboard.OutgoingPRs)...) + + // Organizations section - only show if there are orgs configured + if len(dashboard.WorkspaceOrgs) > 0 { + blocks = append(blocks, slack.NewDividerBlock()) + + var orgLines []string + for _, org := range dashboard.WorkspaceOrgs { + // URL-escape org name to prevent injection + esc := url.PathEscape(org) + orgLine := fmt.Sprintf("• <%s|%s> [<%s|config>, <%s|dashboard>]", + fmt.Sprintf("https://github.com/%s", esc), + org, + fmt.Sprintf("https://github.com/%s/.github/blob/main/.codeGROOVE/slack.yaml", esc), + fmt.Sprintf("https://%s.ready-to-review.dev", esc), + ) + orgLines = append(orgLines, orgLine) + } + + blocks = append(blocks, + slack.NewSectionBlock( + slack.NewTextBlockObject("mrkdwn", + "*Organizations*\n"+strings.Join(orgLines, "\n"), + false, + false, + ), + nil, + nil, ), - ), - ) + ) + } return blocks } @@ -223,12 +225,13 @@ func BuildPRSections(incoming, outgoing []PR) []slack.Block { } // BuildBlocksWithDebug creates Slack Block Kit UI with debug information about user mapping. -func BuildBlocksWithDebug(dashboard *Dashboard, mapping *usermapping.ReverseMapping) []slack.Block { +func BuildBlocksWithDebug(dashboard *Dashboard, userTZ string, mapping *usermapping.ReverseMapping) []slack.Block { // Build standard blocks first - blocks := BuildBlocks(dashboard) + blocks := BuildBlocks(dashboard, userTZ) - // Add debug section if mapping info is available - if mapping != nil { + // Add debug section based on mapping status + switch { + case mapping != nil: blocks = append(blocks, slack.NewDividerBlock(), slack.NewSectionBlock( @@ -260,8 +263,27 @@ func BuildBlocksWithDebug(dashboard *Dashboard, mapping *usermapping.ReverseMapp ), ) } - } else { - // No mapping found - show error message + case len(dashboard.WorkspaceOrgs) == 0: + // No organizations configured for this workspace (likely startup/race condition) + blocks = append(blocks, + slack.NewDividerBlock(), + slack.NewSectionBlock( + slack.NewTextBlockObject("mrkdwn", + "⏳ *Setting up workspace...*\n"+ + "No organizations configured yet. This usually resolves automatically.\n\n"+ + "If this persists:\n"+ + "1. Ensure your GitHub App is installed for your organization\n"+ + "2. Check that `.codeGROOVE/slack.yaml` exists in your org's `.github` repo\n"+ + "3. Verify the `slack:` field matches this workspace's domain", + false, + false, + ), + nil, + nil, + ), + ) + default: + // User mapping failed blocks = append(blocks, slack.NewDividerBlock(), slack.NewSectionBlock( @@ -279,3 +301,20 @@ func BuildBlocksWithDebug(dashboard *Dashboard, mapping *usermapping.ReverseMapp return blocks } + +// formatTimestamp formats a timestamp in the user's timezone without the colon after "Updated". +// Example: "Nov 6, 12:48am America/Los_Angeles". +func formatTimestamp(t time.Time, tzName string) string { + // Load the timezone + loc, err := time.LoadLocation(tzName) + if err != nil { + // Fallback to UTC if timezone is invalid + loc = time.UTC + } + + // Convert to user's timezone + t = t.In(loc) + + // Format as "Jan 2, 3:04pm MST" + return t.Format("Jan 2, 3:04pm MST") +} diff --git a/pkg/home/ui_test.go b/pkg/home/ui_test.go index c445a95..6ea1594 100644 --- a/pkg/home/ui_test.go +++ b/pkg/home/ui_test.go @@ -5,6 +5,7 @@ import ( "testing" "time" + "github.com/codeGROOVE-dev/slacker/pkg/usermapping" "github.com/slack-go/slack" ) @@ -82,13 +83,13 @@ func TestBuildBlocks(t *testing.T) { t.Error("expected dashboard link in Organizations section") } - // Should have Updated timestamp + // Should have Updated timestamp (no colon after "Updated") foundTimestamp := false for _, block := range blocks { if cb, ok := block.(*slack.ContextBlock); ok { for _, elem := range cb.ContextElements.Elements { if txt, ok := elem.(*slack.TextBlockObject); ok { - if strings.Contains(txt.Text, "Updated:") { + if strings.Contains(txt.Text, "Updated ") { foundTimestamp = true } } @@ -234,7 +235,7 @@ func TestBuildBlocks(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - blocks := BuildBlocks(tt.dashboard) + blocks := BuildBlocks(tt.dashboard, "America/Los_Angeles") tt.validate(t, blocks) }) } @@ -248,7 +249,7 @@ func TestBuildBlocks_RefreshButton(t *testing.T) { OutgoingPRs: []PR{}, } - blocks := BuildBlocks(dashboard) + blocks := BuildBlocks(dashboard, "America/Los_Angeles") // Should have action block with refresh button foundRefresh := false @@ -285,7 +286,7 @@ func TestBuildBlocks_DividersBetweenSections(t *testing.T) { }, } - blocks := BuildBlocks(dashboard) + blocks := BuildBlocks(dashboard, "America/Los_Angeles") // Count dividers dividerCount := 0 @@ -408,3 +409,109 @@ func TestBuildPRSections_SortOrder(t *testing.T) { t.Error("outgoing blocked PRs should use :large_green_circle:") } } + +// TestBuildBlocksWithDebug_NoOrgsConfigured verifies improved handling for startup race conditions. +func TestBuildBlocksWithDebug_NoOrgsConfigured(t *testing.T) { + tests := []struct { + name string + dashboard *Dashboard + mapping *usermapping.ReverseMapping + expectText string + notExpect string + description string + }{ + { + name: "no orgs configured - startup race condition", + dashboard: &Dashboard{ + WorkspaceOrgs: []string{}, // Empty - no orgs found yet + IncomingPRs: []PR{}, + OutgoingPRs: []PR{}, + }, + mapping: nil, + expectText: "Setting up workspace", + notExpect: "Could not map Slack user to GitHub", + description: "should show startup message, not user mapping error", + }, + { + name: "has orgs but user mapping failed", + dashboard: &Dashboard{ + WorkspaceOrgs: []string{"test-org", "another-org"}, + IncomingPRs: []PR{}, + OutgoingPRs: []PR{}, + }, + mapping: nil, + expectText: "Could not map Slack user to GitHub", + notExpect: "Setting up workspace", + description: "should show user mapping error, not startup message", + }, + { + name: "successful mapping with good confidence", + dashboard: &Dashboard{ + WorkspaceOrgs: []string{"test-org"}, + IncomingPRs: []PR{}, + OutgoingPRs: []PR{}, + }, + mapping: &usermapping.ReverseMapping{ + GitHubUsername: "testuser", + SlackEmail: "test@example.com", + MatchMethod: "email_match", + Confidence: 95, + }, + expectText: "Debug Info", + notExpect: "Low confidence mapping", + description: "should show debug info without low confidence warning", + }, + { + name: "successful mapping with low confidence", + dashboard: &Dashboard{ + WorkspaceOrgs: []string{"test-org"}, + IncomingPRs: []PR{}, + OutgoingPRs: []PR{}, + }, + mapping: &usermapping.ReverseMapping{ + GitHubUsername: "testuser", + SlackEmail: "test@example.com", + MatchMethod: "name_similarity", + Confidence: 65, + }, + expectText: "Low confidence mapping", + notExpect: "", + description: "should show low confidence warning with suggestion", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + blocks := BuildBlocksWithDebug(tt.dashboard, "America/Los_Angeles", tt.mapping) + + // Convert all blocks to text for searching + var allText string + for _, block := range blocks { + if sb, ok := block.(*slack.SectionBlock); ok { + if sb.Text != nil { + allText += sb.Text.Text + "\n" + } + } + if cb, ok := block.(*slack.ContextBlock); ok { + for _, elem := range cb.ContextElements.Elements { + if txt, ok := elem.(*slack.TextBlockObject); ok { + allText += txt.Text + "\n" + } + } + } + } + + // Check expected text is present + if tt.expectText != "" && !strings.Contains(allText, tt.expectText) { + t.Errorf("%s: expected to find %q in output, but didn't.\nGot: %s", + tt.description, tt.expectText, allText) + } + + // Check unwanted text is not present + if tt.notExpect != "" && strings.Contains(allText, tt.notExpect) { + t.Errorf("%s: should not contain %q, but found it.\nGot: %s", + tt.description, tt.notExpect, allText) + } + }) + } +} diff --git a/pkg/slack/home_handler.go b/pkg/slack/home_handler.go index a656ffd..2c2b9a1 100644 --- a/pkg/slack/home_handler.go +++ b/pkg/slack/home_handler.go @@ -85,7 +85,7 @@ func (h *HomeHandler) tryHandleAppHomeOpened(ctx context.Context, teamID, slackU workspaceOrgs := h.workspaceOrgs(teamID) if len(workspaceOrgs) == 0 { slog.Warn("no workspace orgs found", "team_id", teamID) - return h.publishPlaceholderHome(ctx, slackClient, slackUserID, nil) + return h.publishPlaceholderHome(ctx, slackClient, slackUserID, []string{}, nil) } // Get config for first org to extract domain and user overrides @@ -105,7 +105,7 @@ func (h *HomeHandler) tryHandleAppHomeOpened(ctx context.Context, teamID, slackU slog.Warn("failed to map Slack user to GitHub", "slack_user_id", slackUserID, "error", err) - return h.publishPlaceholderHome(ctx, slackClient, slackUserID, nil) + return h.publishPlaceholderHome(ctx, slackClient, slackUserID, workspaceOrgs, nil) } githubUsername := mapping.GitHubUsername @@ -133,14 +133,23 @@ func (h *HomeHandler) tryHandleAppHomeOpened(ctx context.Context, teamID, slackU slog.Error("failed to fetch dashboard", "github_user", githubUsername, "error", err) - return h.publishPlaceholderHome(ctx, slackClient, slackUserID, mapping) + return h.publishPlaceholderHome(ctx, slackClient, slackUserID, workspaceOrgs, mapping) } // Add workspace orgs to dashboard for UI display dashboard.WorkspaceOrgs = workspaceOrgs + // Get user's timezone for timestamp display + userTZ, err := slackClient.UserTimezone(ctx, slackUserID) + if err != nil { + slog.Warn("failed to get user timezone, defaulting to UTC", + "slack_user_id", slackUserID, + "error", err) + userTZ = "UTC" + } + // Build Block Kit UI with debug info - blocks := home.BuildBlocksWithDebug(dashboard, mapping) + blocks := home.BuildBlocksWithDebug(dashboard, userTZ, mapping) // Publish to Slack if err := slackClient.PublishHomeView(ctx, slackUserID, blocks); err != nil { @@ -180,14 +189,29 @@ func (h *HomeHandler) workspaceOrgs(teamID string) []string { } // publishPlaceholderHome publishes a simple placeholder home view. -func (*HomeHandler) publishPlaceholderHome(ctx context.Context, slackClient *Client, slackUserID string, mapping *usermapping.ReverseMapping) error { +func (*HomeHandler) publishPlaceholderHome( + ctx context.Context, + slackClient *Client, + slackUserID string, + workspaceOrgs []string, + mapping *usermapping.ReverseMapping, +) error { slog.Debug("publishing placeholder home", "user_id", slackUserID) + // Get user's timezone for timestamp display + userTZ, err := slackClient.UserTimezone(ctx, slackUserID) + if err != nil { + slog.Debug("failed to get user timezone for placeholder, defaulting to UTC", + "slack_user_id", slackUserID, + "error", err) + userTZ = "UTC" + } + blocks := home.BuildBlocksWithDebug(&home.Dashboard{ IncomingPRs: nil, OutgoingPRs: nil, - WorkspaceOrgs: []string{"your-org"}, - }, mapping) + WorkspaceOrgs: workspaceOrgs, + }, userTZ, mapping) return slackClient.PublishHomeView(ctx, slackUserID, blocks) }