Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions roadmap-planner/backend/cmd/server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,9 +210,13 @@ func initTeamAnalytics(ctx context.Context, router *gin.Engine, cfg *config.Conf
service.SetPillarMap(pillarMap)
service.SetStatusClassifier(contributions.NewStatusClassifier(cfg.TeamAnalytics.Statuses))
aggregator := contributions.NewAggregator(store)
api.AddContributionsRoutes(router, store, service, aggregator)
allowlist := contributions.BuildAllowlist(cfg.TeamAnalytics)
aggregator.SetAllowlist(allowlist)
api.AddContributionsRoutes(router, store, service, aggregator, allowlist)
logger.Info("Contributions API routes added",
zap.Int("pillars_configured", len(pillarMap.Order())))
zap.Int("pillars_configured", len(pillarMap.Order())),
zap.Bool("allowlist_enabled", allowlist.Enabled()),
zap.Int("allowlist_size", allowlist.Size()))

// Optional Jira sync goroutine.
//
Expand Down Expand Up @@ -259,7 +263,7 @@ func initTeamAnalytics(ctx context.Context, router *gin.Engine, cfg *config.Conf

// Optional GitLab sync goroutine.
if cfg.GitLab.Enabled {
startGitLabSync(ctx, cfg, store, aggregator)
startGitLabSync(ctx, cfg, store, aggregator, allowlist)
}

go func() {
Expand Down Expand Up @@ -344,7 +348,7 @@ func buildGitHubClient(cfg *config.GitHub) (*ghclient.Client, string, error) {
//
// Same fail-soft contract as startGitHubSync — bad config logs and
// continues without a GitLab sync rather than refusing to start.
func startGitLabSync(ctx context.Context, cfg *config.Config, store storage.Store, aggregator *contributions.Aggregator) {
func startGitLabSync(ctx context.Context, cfg *config.Config, store storage.Store, aggregator *contributions.Aggregator, allowlist contributions.Allowlist) {
specs := parseGroupSpecs(cfg.GitLab.Groups)
if len(specs) == 0 {
logger.Warn("gitlab.enabled but gitlab.groups is empty; skipping sync")
Expand Down Expand Up @@ -372,6 +376,15 @@ func startGitLabSync(ctx context.Context, cfg *config.Config, store storage.Stor
syncer := glclient.NewSyncer(client, store, specs, glclient.DefaultLinker(projectKey), backfill)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug (bug/missing-feature): The GitLab syncer receives AllowedMemberIDs from the allowlist, but the GitHub syncer path in startGitHubSync has no equivalent allowlist filter. GitHub PRs authored by non-allowlisted members will still be inserted into pull_requests and rolled up — inconsistent dashboard counts when both sources are active.

Suggested change
syncer := glclient.NewSyncer(client, store, specs, glclient.DefaultLinker(projectKey), backfill)
// TODO(W2): wire GitHub syncer AuthorAllowlist to filter GitHub PRs the same way
// Pass B filters GitLab MRs, so GitHub counts are consistent across sources.

syncer.HydrateDiff = cfg.GitLab.HydrateDiff
syncer.IncludeArchived = cfg.GitLab.IncludeArchived
syncer.MemberInstanceSweep = cfg.GitLab.MemberInstanceSweep
if allowlist.Enabled() {
ids := allowlist.IDs()
allowed := make(map[string]struct{}, len(ids))
for _, id := range ids {
allowed[id] = struct{}{}
}
syncer.AllowedMemberIDs = allowed
}

interval, err := time.ParseDuration(cfg.GitLab.SyncInterval)
if err != nil {
Expand All @@ -381,7 +394,9 @@ func startGitLabSync(ctx context.Context, cfg *config.Config, store storage.Stor
logger.Info("GitLab sync started",
zap.Duration("interval", interval),
zap.Int("specs", len(specs)),
zap.Int("backfill_days", backfill))
zap.Int("backfill_days", backfill),
zap.Bool("pass_b_sweep", syncer.MemberInstanceSweep && len(syncer.AllowedMemberIDs) > 0),
zap.Int("allowlisted_members", len(syncer.AllowedMemberIDs)))
}

func parseGroupSpecs(specs []string) []glclient.GroupSpec {
Expand Down
44 changes: 43 additions & 1 deletion roadmap-planner/backend/config.example.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -123,4 +123,46 @@ github:
installation_id: 0 # numeric, from the org's "installed apps" page
# Provide ONE of:
private_key_path: "" # path to the PEM file (mount via Secret)
private_key_pem: "" # raw PEM contents (use env GITHUB_APP_PRIVATE_KEY_PEM in K8s)
private_key_pem: "" # raw PEM contents (use env GITHUB_APP_PRIVATE_KEY_PEM in K8s)

# GitLab ingestion — mirrors `github` for self-hosted GitLab installs.
# Requires storage.enabled = true.
gitlab:
enabled: false
base_url: "" # e.g. "https://gitlab-ce.example.com"
token: "" # personal access token, read_api scope (or GITLAB_TOKEN env)
sync_interval: "30m"
project_key: "" # for the default Linker; defaults to jira.project
backfill_days: 0 # 0 = inherit storage.backfill_days
groups:
- "devops/**" # "**" = whole subtree; "group/*" = direct children only
hydrate_diff: false # fetch additions/deletions/changed_files per merged MR
include_archived: false # include archived projects in the sweep
# W1 Pass B (2026-05-19). When true, every cycle also sweeps
# `/merge_requests?scope=all&author_username=<u>` for every member in
# the derived allowlist (`team_analytics.{github,gitlab}_username_prefills`
# minus `team_analytics.member_denylist`). Captures MRs your team
# filed in projects outside `groups`. No-op when no prefills are
# configured — adopt gradually.
member_instance_sweep: false

# Team analytics — pillar attribution + identity prefills + W1 filters.
# All keys here are optional and additive; the dashboard works without
# them (it just shows the un-curated raw data).
team_analytics:
# Identity prefills — one-shot github_login / gitlab_username
# assignment to Jira members on the first sync. Manual UI edits always
# win on subsequent syncs.
github_login_prefills:
# daniel: danielfbm
# jtcheng: chengjingtao
gitlab_username_prefills:
# daniel: daniel
# jtcheng: chengjingtao
# W1 allowlist denylist. The effective allowlist is the union of the
# two prefill maps above minus the entries here. Use this to suppress
# ex-team-members or contractors who shouldn't count.
member_denylist: []
# - gxjiao
# - lmhe
# pillars: { … } # see docs/team-analytics/PROPOSAL.md
44 changes: 43 additions & 1 deletion roadmap-planner/backend/internal/api/handlers/contributions.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,33 @@ import (
// contributions.Service, and returns JSON. All logic lives one layer
// down. This makes it easy to keep the API stable while we evolve the
// rollup pipeline (B2/B3).
//
// allowlist is the W1 "who counts" filter; when disabled (Enabled()
// returns false) every endpoint reverts to the pre-W1 behaviour and
// shows whoever is in the underlying tables.
type ContributionsHandler struct {
store storage.Store
service *contributions.Service
aggregator *contributions.Aggregator
allowlist contributions.Allowlist
}

func NewContributionsHandler(store storage.Store, service *contributions.Service, aggregator *contributions.Aggregator) *ContributionsHandler {
return &ContributionsHandler{store: store, service: service, aggregator: aggregator}
}

// SetAllowlist installs the W1 allowlist filter. Safe to call at
// startup; not safe to swap at runtime (no synchronisation on reads).
func (h *ContributionsHandler) SetAllowlist(al contributions.Allowlist) {
h.allowlist = al
}

// ListMembers — GET /api/contributions/members?include_inactive=1
//
// Inactive Jira users (deactivated accounts) are hidden by default — the
// team-overview UI only shows people who can still receive work.
// team-overview UI only shows people who can still receive work. When
// the W1 allowlist is configured, members outside it are filtered out
// regardless of their Active flag.
func (h *ContributionsHandler) ListMembers(c *gin.Context) {
members, err := h.store.ListMembers(c.Request.Context())
if err != nil {
Expand All @@ -49,6 +62,7 @@ func (h *ContributionsHandler) ListMembers(c *gin.Context) {
if !truthy(c.Query("include_inactive")) {
members = filterActive(members)
}
members = filterAllowlist(members, h.allowlist)
c.JSON(http.StatusOK, gin.H{"members": members})
}

Expand Down Expand Up @@ -96,6 +110,19 @@ func (h *ContributionsHandler) TeamOverview(c *gin.Context) {
out = filtered
}
}
// W1 allowlist: when enabled, drop summaries for members outside
// the configured set. The aggregator already filters them out of
// the rollup table, but a manual rebuild that uses a different
// allowlist may have left stragglers — defence in depth.
if h.allowlist.Enabled() {
filtered := out[:0]
for _, ms := range out {
if h.allowlist.Contains(ms.MemberID) {
filtered = append(filtered, ms)
}
}
out = filtered
}
c.JSON(http.StatusOK, gin.H{"members": out, "from": q.From, "to": q.To})
}

Expand Down Expand Up @@ -403,3 +430,18 @@ func filterActive(in []storage.Member) []storage.Member {
}
return out
}

// filterAllowlist returns only members whose ID is in the allowlist.
// No-op when the allowlist is disabled.
func filterAllowlist(in []storage.Member, al contributions.Allowlist) []storage.Member {
if !al.Enabled() {
return in
}
out := in[:0]
for _, m := range in {
if al.Contains(m.ID) {
out = append(out, m)
}
}
return out
}
3 changes: 2 additions & 1 deletion roadmap-planner/backend/internal/api/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,12 @@ func NewRouter(cfg *config.Config) *gin.Engine {
// GET /api/contributions/network — review-network density panel
// GET /api/contributions/pillars — pillar throughput stack
// GET /api/contributions/status — last-sync timestamps
func AddContributionsRoutes(router *gin.Engine, store storage.Store, service *contributions.Service, aggregator *contributions.Aggregator) {
func AddContributionsRoutes(router *gin.Engine, store storage.Store, service *contributions.Service, aggregator *contributions.Aggregator, allowlist contributions.Allowlist) {
if store == nil || service == nil {
return
}
h := handlers.NewContributionsHandler(store, service, aggregator)
h.SetAllowlist(allowlist)
api := router.Group("/api")
g := api.Group("/contributions")
g.Use(middleware.AuthMiddleware())
Expand Down
30 changes: 30 additions & 0 deletions roadmap-planner/backend/internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,17 @@ type TeamAnalytics struct {
// daniel: daniel
// jtcheng: chengjingtao
GitLabUsernamePrefills map[string]string `mapstructure:"gitlab_username_prefills" yaml:"gitlab_username_prefills"`
// MemberDenylist is the operator-curated list of Jira member ids
// (slugified email — the same key shape used in the prefill maps)
// that should be excluded from the team-analytics rollups and from
// every contributions API response, regardless of whether they have
// an entry in the prefill maps.
//
// This is the W1 escape hatch for people who appear in Jira (so the
// auto-discovery picks them up) but should not count toward the
// dashboard — bots that aren't on the bot allowlist, people who
// left the team, contractors loaned out to another pillar, etc.
MemberDenylist []string `mapstructure:"member_denylist" yaml:"member_denylist"`
// Statuses is the W4 (2026-05-19) configurable status → lane map
// used by the sprint card. Status names match Jira's `status.name`
// case-folded; an unknown status falls back to `in_progress` and
Expand Down Expand Up @@ -207,6 +218,12 @@ type GitLab struct {
BackfillDays int `mapstructure:"backfill_days"`
HydrateDiff bool `mapstructure:"hydrate_diff"`
IncludeArchived bool `mapstructure:"include_archived"`
// MemberInstanceSweep turns on the W1 Pass B: after the group-scoped
// fetch finishes, sweep `/merge_requests?scope=all&author_username=<u>`
// for every allowlisted member whose GitLab username we know. Off
// by default so existing installs upgrade with no behaviour change;
// flip on once `team_analytics.gitlab_username_prefills` is curated.
MemberInstanceSweep bool `mapstructure:"member_instance_sweep"`
}

// Logger represents logger configuration settings
Expand Down Expand Up @@ -418,6 +435,7 @@ func Load() (*Config, error) {
viper.SetDefault("gitlab.backfill_days", 0)
viper.SetDefault("gitlab.hydrate_diff", true)
viper.SetDefault("gitlab.include_archived", false)
viper.SetDefault("gitlab.member_instance_sweep", false)

// GitHub defaults
viper.SetDefault("github.enabled", false)
Expand Down Expand Up @@ -490,6 +508,18 @@ func Load() (*Config, error) {
TeamAnalytics TeamAnalytics `yaml:"team_analytics"`
}
if uerr := yaml.Unmarshal(raw, &preserve); uerr == nil && len(preserve.TeamAnalytics.Pillars) > 0 {
// Preserve the YAML-side pillar names (case-sensitive),
// but merge back the viper-loaded denylist + prefills
// so they aren't lost when the YAML file omits them.
if len(preserve.TeamAnalytics.MemberDenylist) == 0 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggestion (refactor/robustness): Using len(...) == 0 for nil-slice safety. If struct fields can ever be nil (not just empty slices), consider if field == nil || len(field) == 0 for belt-and-suspenders robustness.

preserve.TeamAnalytics.MemberDenylist = config.TeamAnalytics.MemberDenylist
}
if len(preserve.TeamAnalytics.GitHubLoginPrefills) == 0 {
preserve.TeamAnalytics.GitHubLoginPrefills = config.TeamAnalytics.GitHubLoginPrefills
}
if len(preserve.TeamAnalytics.GitLabUsernamePrefills) == 0 {
preserve.TeamAnalytics.GitLabUsernamePrefills = config.TeamAnalytics.GitLabUsernamePrefills
}
config.TeamAnalytics = preserve.TeamAnalytics
}
}
Expand Down
Loading
Loading