fix: use cached user list in /check instead of per-member GetUserInfo calls

[Copilot]

handleListMissing was calling api.GetUserInfo(uid) once per team member on every /check invocation — N serial Slack API calls adding latency and rate-limit exposure proportional to team size.

Changes

• handleListMissing: Replace the per-member api.GetUserInfo loop with a single getCachedUsers(api) call (5-minute TTL cache) before the loop, then build an id→slack.User map for O(1) lookups per member.
• Error handling: Log getCachedUsers failures rather than silently discarding the error; gracefully degrades by displaying raw Slack IDs when the cache call fails.

// Before: N API calls inside loop
for _, uid := range memberIDs {
    user, err := api.GetUserInfo(uid)  // one round-trip per member
    ...
}

// After: 1 cached call + map lookup
cachedUsers, err := getCachedUsers(api)  // served from cache after first call
userByID := buildUserMap(cachedUsers)
for _, uid := range memberIDs {
    u, found := userByID[uid]  // O(1)
    ...
}

---

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

[Copilot]

Pull request overview

Optimizes the Slack /check (missing reports) command by avoiding per-member users.info calls and instead leveraging the existing cached Slack user list to reduce latency and rate-limit exposure.

Changes:

• Reworked handleListMissing to fetch Slack users once via getCachedUsers(api) and do O(1) ID→user lookups via a map.
• Added logging for getCachedUsers failures and continued with a degraded display mode (raw Slack IDs).

Comments suppressed due to low confidence (3)

internal/integrations/slack/slack.go:890

• This change affects observable behavior (which Slack endpoints are called, and the fallback behavior when users.list fails) but there’s no automated coverage for /check/handleListMissing. Since the repo already has functional Slack tests with mock HTTP endpoints, please add a test that asserts /check relies on users.list (and does not issue per-member users.info calls) to prevent regressions.

	// Build an ID→User map from the cached users list to avoid N individual
	// GetUserInfo calls (one per team member) inside the loop below.
	cachedUsers, err := getCachedUsers(api)
	if err != nil {
		log.Printf("list-missing: getCachedUsers error: %v", err)
	}

internal/integrations/slack/slack.go:922

• getCachedUsers failure currently falls through to an empty userByID map, which means all members will lose name enrichment for this /check invocation (single-point-of-failure). If preserving names is important, consider falling back to api.GetUserInfo(uid) only for IDs not found in userByID (or when getCachedUsers errors) so a transient users.list issue doesn’t degrade the entire output.

	cachedUsers, err := getCachedUsers(api)
	if err != nil {
		log.Printf("list-missing: getCachedUsers error: %v", err)
	}
	userByID := make(map[string]slack.User, len(cachedUsers))
	for _, u := range cachedUsers {
		userByID[u.ID] = u
	}

	type missingMember struct {
		display string
		userID  string
	}
	var missing []missingMember
	var missingIDs []string
	for _, uid := range memberIDs {
		u, found := userByID[uid]
		nameCandidates := []string{uid}
		if found {
			if u.Profile.DisplayName != "" {
				nameCandidates = append(nameCandidates, u.Profile.DisplayName)
			}
			if u.RealName != "" {
				nameCandidates = append(nameCandidates, u.RealName)
			}
		}
		if memberReportedThisWeek(uid, nameCandidates, reportedAuthorIDs, reportedAuthors) {
			continue
		}
		if !found {
			missing = append(missing, missingMember{display: uid, userID: uid})
			missingIDs = append(missingIDs, uid)
			continue
		}

		display := u.Profile.DisplayName

internal/integrations/slack/slack.go:894

• Building userByID as map[string]slack.User copies every slack.User from the cached users.list response into a second data structure. In larger workspaces this can significantly increase per-/check allocations and memory footprint. Consider mapping IDs to indices (or to a small struct with just the needed name fields) to avoid copying the full slack.User objects.

	cachedUsers, err := getCachedUsers(api)
	if err != nil {
		log.Printf("list-missing: getCachedUsers error: %v", err)
	}
	userByID := make(map[string]slack.User, len(cachedUsers))
	for _, u := range cachedUsers {
		userByID[u.ID] = u
	}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.