From ee8c3fb8a4b70dc2686e28c3e2c73b42583b7504 Mon Sep 17 00:00:00 2001 From: harshitsinghbhandari <24b4506@iitb.ac.in> Date: Sat, 30 May 2026 23:52:18 +0530 Subject: [PATCH 1/2] feat(tracker): GitLab adapter (read-only Get/List/Preflight) Implements ports.Tracker for GitLab Issues over REST v4. Mirrors the GitHub adapter pattern (TokenSource, sentinel errors, typed RateLimitError, atomic+mutex preflight cache, strict provider check) with GitLab-specific deviations: - PRIVATE-TOKEN header instead of Authorization: Bearer - state spelling "opened" (not "open") in query + payload - rate-limit headers without X- prefix; 429 status - URL-encoded full project path (subgroups of arbitrary depth) - assignee_username query param - label-based closed-reason convention (cancelled/wontfix) since GitLab REST v4 has no native state_reason/close_reason field v1 scope: Get, List (single page, no auto-pagination), Preflight. Comment / Transition deferred to #40; observer/polling to #35. Co-Authored-By: Claude Opus 4.7 --- .../internal/adapters/tracker/gitlab/auth.go | 49 ++ .../internal/adapters/tracker/gitlab/doc.go | 74 +++ .../adapters/tracker/gitlab/tracker.go | 537 +++++++++++++++ .../adapters/tracker/gitlab/tracker_test.go | 623 ++++++++++++++++++ 4 files changed, 1283 insertions(+) create mode 100644 backend/internal/adapters/tracker/gitlab/auth.go create mode 100644 backend/internal/adapters/tracker/gitlab/doc.go create mode 100644 backend/internal/adapters/tracker/gitlab/tracker.go create mode 100644 backend/internal/adapters/tracker/gitlab/tracker_test.go diff --git a/backend/internal/adapters/tracker/gitlab/auth.go b/backend/internal/adapters/tracker/gitlab/auth.go new file mode 100644 index 00000000..c668692d --- /dev/null +++ b/backend/internal/adapters/tracker/gitlab/auth.go @@ -0,0 +1,49 @@ +package gitlab + +import ( + "context" + "errors" + "os" + "strings" +) + +// TokenSource yields a GitLab personal-access (or project-access) token on +// demand. Mirrors the GitHub adapter's surface so daemon wiring is uniform +// across providers: the Tracker calls Token once at construction (fail-fast) +// and again per request (so rotated tokens are picked up without restart). +type TokenSource interface { + Token(ctx context.Context) (string, error) +} + +// ErrNoToken is returned when no token source could yield a non-empty token. +var ErrNoToken = errors.New("gitlab tracker: no token configured") + +// StaticTokenSource is a literal token, typically used in tests. +type StaticTokenSource string + +func (s StaticTokenSource) Token(context.Context) (string, error) { + t := strings.TrimSpace(string(s)) + if t == "" { + return "", ErrNoToken + } + return t, nil +} + +// EnvTokenSource reads the first non-empty value from the listed env vars, +// falling back to GITLAB_TOKEN. The order matters: a project-configured token +// (e.g. AO_GITLAB_TOKEN) should be preferred over the global default. +type EnvTokenSource struct { + EnvVars []string +} + +func (s EnvTokenSource) Token(context.Context) (string, error) { + for _, name := range s.EnvVars { + if v := strings.TrimSpace(os.Getenv(name)); v != "" { + return v, nil + } + } + if v := strings.TrimSpace(os.Getenv("GITLAB_TOKEN")); v != "" { + return v, nil + } + return "", ErrNoToken +} diff --git a/backend/internal/adapters/tracker/gitlab/doc.go b/backend/internal/adapters/tracker/gitlab/doc.go new file mode 100644 index 00000000..2154e5a9 --- /dev/null +++ b/backend/internal/adapters/tracker/gitlab/doc.go @@ -0,0 +1,74 @@ +// Package gitlab implements the ports.Tracker outbound port for GitLab +// Issues over the REST v4 API. v1 is read-only: +// +// - Get returns a normalized snapshot of one issue (spawn-bootstrap reads +// it to hydrate the agent prompt). +// - List returns a filtered slice of issues in a project (one page, no +// auto-pagination in v1). +// - Preflight performs a single GET /user against GitLab to verify the +// token is accepted; success is cached for the lifetime of the Tracker, +// failures are not. +// +// Writing back to the tracker (Comment, Transition) is deferred to issue +// #40. The observer / polling loop is deferred to issue #35. +// +// # Authentication +// +// The adapter uses the PRIVATE-TOKEN header (not Authorization: Bearer) +// because that is GitLab's recommended path for personal-access and +// project-access tokens. The TokenSource interface lets callers inject a +// static token in tests or read GITLAB_TOKEN (plus arbitrary higher-priority +// env vars) in production. +// +// # ID and repo shape +// +// TrackerID.Native is "group/project#iid". Subgroup paths +// ("group/sub/project#iid", arbitrary depth) are accepted; the FULL project +// path is URL-encoded with url.PathEscape when forming the endpoint URL — +// without that, GitLab routes /projects/group/sub/project/... as a missing +// project rather than the nested one. TrackerRepo.Native is the same shape +// minus the "#iid". +// +// The IID is the project-internal sequential id (shown in the web UI as +// "#42"), not the global database id. That matches GitLab's recommendation +// to use IIDs in URLs. +// +// # Reverse state mapping +// +// GitLab Issues have two coarse states ("opened", "closed") and — unlike +// GitHub — do not expose a structured close_reason / state_reason on the +// REST v4 issue payload. The adapter projects them onto the normalized +// vocabulary as follows: +// +// - closed + label "cancelled" OR "wontfix" -> cancelled +// - closed (no cancelled/wontfix label) -> done +// - opened + label "in-review" -> review (wins when +// both status labels are present; the workflow is progress -> review) +// - opened + label "in-progress" -> in_progress +// - otherwise -> open +// +// The "in-progress" / "in-review" convention is borrowed verbatim from the +// GitHub adapter so a downstream consumer sees the same shape regardless of +// which tracker an issue lives in. The "cancelled" / "wontfix" labels are +// recognized because GitLab has no native equivalent of GitHub's +// state_reason=not_planned — humans (and other tooling) use one of these +// labels to mark issues abandoned rather than resolved. The adapter does +// NOT write any of these labels in v1 (issue #40). +// +// # Rate limiting +// +// GitLab uses RateLimit-Remaining / RateLimit-Reset (no X- prefix, unlike +// GitHub) and 429 (not 403) for rate-limit responses. On 429 the adapter +// returns a typed *RateLimitError matching errors.Is(err, ErrRateLimited); +// callers that want to back off intelligently can extract ResetAt / +// RetryAfter via errors.As. +// +// # Out of scope +// +// - No Comment, no Transition (issue #40). +// - No List pagination beyond a single page; callers needing more results +// need to wait for the observer/polling work in issue #35. +// - No webhook receiver, no polling goroutine, no fact projection into LCM. +// - No richer per-provider metadata on Issue (milestones, epics, +// iterations); the port only carries fields all v1 providers can fill. +package gitlab diff --git a/backend/internal/adapters/tracker/gitlab/tracker.go b/backend/internal/adapters/tracker/gitlab/tracker.go new file mode 100644 index 00000000..4815a444 --- /dev/null +++ b/backend/internal/adapters/tracker/gitlab/tracker.go @@ -0,0 +1,537 @@ +package gitlab + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "strconv" + "strings" + "sync" + "sync/atomic" + "time" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + "github.com/aoagents/agent-orchestrator/backend/internal/ports" +) + +const ( + defaultBaseURL = "https://gitlab.com/api/v4" + defaultUserAgent = "ao-agent-orchestrator/tracker-gitlab" + + // Status labels recognized on open issues. Adopted verbatim from the + // GitHub adapter so the cross-provider contract is one shape: humans + // (and other tooling) put "in-progress" / "in-review" on issues, and + // the adapter projects them onto the normalized state. v1 does NOT + // write these labels — see issue #40 for the write-side work. + labelInProgress = "in-progress" + labelInReview = "in-review" + + // Status labels recognized on closed issues. GitLab has no equivalent + // of GitHub's state_reason=not_planned, so we use a label convention: + // "cancelled" or "wontfix" on a closed issue means cancelled rather + // than completed. + labelCancelled = "cancelled" + labelWontfix = "wontfix" + + stateClosedGL = "closed" + + // List pagination — GitLab's per_page maxes at 100. Default 30 to + // match the GitHub adapter (GitLab's API default is 20, but uniform + // defaults across providers make the SM's behavior predictable). + defaultListLimit = 30 + maxListLimit = 100 +) + +// Sentinel errors. Callers should match via errors.Is; the orchestrator's +// lifecycle code is intentionally insulated from raw HTTP status codes. +var ( + ErrNotFound = errors.New("gitlab tracker: issue not found") + ErrRateLimited = errors.New("gitlab tracker: rate limited") + ErrAuthFailed = errors.New("gitlab tracker: authentication failed") + ErrWrongProvider = errors.New("gitlab tracker: id is not a gitlab tracker id") + ErrBadID = errors.New("gitlab tracker: malformed native id") +) + +// RateLimitError is returned when GitLab reports the request was rate-limited. +// Callers that want to back off intelligently can extract ResetAt / RetryAfter +// via errors.As; callers that only need the category can use +// errors.Is(err, ErrRateLimited). +type RateLimitError struct { + ResetAt time.Time + RetryAfter time.Duration + Message string +} + +func (e *RateLimitError) Error() string { + if e == nil { + return ErrRateLimited.Error() + } + if e.Message != "" { + return "gitlab tracker: rate limited: " + e.Message + } + return ErrRateLimited.Error() +} + +func (e *RateLimitError) Is(target error) bool { return target == ErrRateLimited } + +// Options configures a Tracker. All fields except Token are optional — +// production code typically sets Token alone; tests inject HTTPClient and +// BaseURL to point at an httptest fake. +type Options struct { + Token TokenSource + HTTPClient *http.Client + BaseURL string + UserAgent string +} + +// Tracker implements ports.Tracker against the GitLab REST v4 API. +// +// Construction performs a fail-fast token presence check (no network call). +// The first Preflight call validates the token against GitLab itself; a +// successful preflight is cached for the lifetime of the Tracker so repeat +// calls are free, while failures are intentionally NOT cached so a transient +// startup glitch doesn't permanently brick the adapter. +type Tracker struct { + http *http.Client + tokens TokenSource + baseURL string + userAgent string + + // preflightOK is the fast-path: once a Preflight succeeds, every + // subsequent call short-circuits via atomic.Load without touching the + // mutex. preflightMu serializes the one-time network call so concurrent + // first-callers don't all fire GET /user against GitLab. + preflightOK atomic.Bool + preflightMu sync.Mutex +} + +// New returns a Tracker. It fails fast when no token can be obtained so +// daemons crash at startup rather than at first issue lookup. +func New(opts Options) (*Tracker, error) { + src := opts.Token + if src == nil { + return nil, ErrNoToken + } + if _, err := src.Token(context.Background()); err != nil { + return nil, err + } + t := &Tracker{ + http: opts.HTTPClient, + tokens: src, + baseURL: opts.BaseURL, + userAgent: opts.UserAgent, + } + if t.http == nil { + t.http = &http.Client{Timeout: 30 * time.Second} + } + if t.baseURL == "" { + t.baseURL = defaultBaseURL + } + if t.userAgent == "" { + t.userAgent = defaultUserAgent + } + return t, nil +} + +// Statically assert Tracker satisfies the port. +var _ ports.Tracker = (*Tracker)(nil) + +// --------------------------------------------------------------------------- +// Get +// --------------------------------------------------------------------------- + +// glIssue is the subset of fields we read off the REST issue payload. +// Note: state is "opened" / "closed" (GitLab spelling, not GitHub's "open"). +// Labels are returned as plain strings by default (no with_labels_details +// param), so a []string is sufficient. +type glIssue struct { + IID int `json:"iid"` + Title string `json:"title"` + Description string `json:"description"` + State string `json:"state"` + WebURL string `json:"web_url"` + Labels []string `json:"labels"` + Assignees []glUser `json:"assignees"` +} + +type glUser struct { + Username string `json:"username"` +} + +func (t *Tracker) Get(ctx context.Context, id domain.TrackerID) (domain.Issue, error) { + project, iid, err := t.parseID(id) + if err != nil { + return domain.Issue{}, err + } + path := fmt.Sprintf("/projects/%s/issues/%d", url.PathEscape(project), iid) + + resp, err := t.do(ctx, http.MethodGet, path, nil) + if err != nil { + return domain.Issue{}, err + } + var raw glIssue + if err := json.Unmarshal(resp, &raw); err != nil { + return domain.Issue{}, fmt.Errorf("gitlab tracker: decode issue: %w", err) + } + return issueFromGL(project, raw), nil +} + +// issueFromGL projects a raw GitLab issue payload into the normalized +// domain.Issue. project is passed in because the TrackerID.Native shape is +// "group/project#iid" and we want the returned ID to round-trip through the +// same adapter even if the original caller used a zero Provider. +func issueFromGL(project string, raw glIssue) domain.Issue { + labels := append([]string(nil), raw.Labels...) + assignees := make([]string, 0, len(raw.Assignees)) + for _, a := range raw.Assignees { + assignees = append(assignees, a.Username) + } + out := domain.Issue{ + ID: domain.TrackerID{ + Provider: domain.TrackerProviderGitLab, + Native: fmt.Sprintf("%s#%d", project, raw.IID), + }, + Title: raw.Title, + Body: raw.Description, + State: mapStateFromGitLab(raw.State, labels), + URL: raw.WebURL, + Labels: labels, + Assignees: assignees, + } + if len(out.Labels) == 0 { + out.Labels = nil + } + if len(out.Assignees) == 0 { + out.Assignees = nil + } + return out +} + +// mapStateFromGitLab projects GitLab's opened/closed + labels onto the +// normalized state. On closed issues a "cancelled" or "wontfix" label maps +// to cancelled; on open issues "in-review" wins over "in-progress" when +// both are present (the workflow is progress -> review -> done). +// +// Label matching is case-insensitive — GitLab label names are +// case-insensitive in the UI, so a user typing "In-Progress" should be +// recognized just as "in-progress" is. +func mapStateFromGitLab(state string, labels []string) domain.NormalizedIssueState { + switch strings.ToLower(state) { + case stateClosedGL: + for _, l := range labels { + switch strings.ToLower(l) { + case labelCancelled, labelWontfix: + return domain.IssueCancelled + } + } + return domain.IssueDone + } + var hasProgress, hasReview bool + for _, l := range labels { + switch strings.ToLower(l) { + case labelInProgress: + hasProgress = true + case labelInReview: + hasReview = true + } + } + switch { + case hasReview: + return domain.IssueInReview + case hasProgress: + return domain.IssueInProgress + default: + return domain.IssueOpen + } +} + +// --------------------------------------------------------------------------- +// List +// --------------------------------------------------------------------------- + +// List returns issues for a project, filtered by state/labels/assignee_username. +// Pagination is intentionally NOT implemented in v1 — callers get one page +// bounded by ListFilter.Limit (default 30, max 100). +func (t *Tracker) List(ctx context.Context, repo domain.TrackerRepo, filter domain.ListFilter) ([]domain.Issue, error) { + if repo.Provider != domain.TrackerProviderGitLab { + return nil, fmt.Errorf("%w: provider=%q", ErrWrongProvider, repo.Provider) + } + project, err := parseGitLabRepo(repo.Native) + if err != nil { + return nil, err + } + + q := url.Values{} + switch filter.State { + case domain.ListOpen: + // GitLab uses "opened" — NOT "open" like GitHub. + q.Set("state", "opened") + case domain.ListClosed: + q.Set("state", "closed") + default: + q.Set("state", "all") + } + if len(filter.Labels) > 0 { + q.Set("labels", strings.Join(filter.Labels, ",")) + } + if filter.Assignee != "" { + // GitLab spells this assignee_username (the bare "assignee" param + // expects a numeric user id, which we don't carry on the port). + q.Set("assignee_username", filter.Assignee) + } + limit := filter.Limit + if limit <= 0 { + limit = defaultListLimit + } + if limit > maxListLimit { + limit = maxListLimit + } + q.Set("per_page", strconv.Itoa(limit)) + + path := fmt.Sprintf("/projects/%s/issues?%s", url.PathEscape(project), q.Encode()) + resp, err := t.do(ctx, http.MethodGet, path, nil) + if err != nil { + return nil, err + } + var raw []glIssue + if err := json.Unmarshal(resp, &raw); err != nil { + return nil, fmt.Errorf("gitlab tracker: decode list: %w", err) + } + out := make([]domain.Issue, 0, len(raw)) + for _, r := range raw { + out = append(out, issueFromGL(project, r)) + } + return out, nil +} + +// --------------------------------------------------------------------------- +// Preflight +// --------------------------------------------------------------------------- + +// Preflight verifies the configured token is currently accepted by GitLab +// (one GET /user). It does NOT prove the token has the scope or visibility +// needed for any specific Get/List call — those may still fail with +// ErrAuthFailed even after a successful Preflight. The guarantee is "token +// exists and is valid against GitLab's identity endpoint", not "token can +// do everything the SM will ask of it." Per-project authorization is +// detected lazily at the first Get/List against that project. +// +// Successful checks are cached for the lifetime of the Tracker via a +// double-checked atomic+mutex pattern: the hot path is one atomic.Load with +// no contention; concurrent first-callers serialize on the mutex so only +// one GET /user is in flight. Failures are intentionally NOT cached so a +// transient startup glitch is recoverable on a subsequent call. +func (t *Tracker) Preflight(ctx context.Context) error { + if t.preflightOK.Load() { + return nil + } + t.preflightMu.Lock() + defer t.preflightMu.Unlock() + // Re-check after acquiring the lock — another goroutine may have raced + // us through the network call and stored success while we were waiting. + if t.preflightOK.Load() { + return nil + } + if _, err := t.do(ctx, http.MethodGet, "/user", nil); err != nil { + return err + } + t.preflightOK.Store(true) + return nil +} + +// --------------------------------------------------------------------------- +// HTTP plumbing +// --------------------------------------------------------------------------- + +func (t *Tracker) do(ctx context.Context, method, path string, body any) ([]byte, error) { + var rdr io.Reader + if body != nil { + b, err := json.Marshal(body) + if err != nil { + return nil, fmt.Errorf("gitlab tracker: encode body: %w", err) + } + rdr = bytes.NewReader(b) + } + req, err := http.NewRequestWithContext(ctx, method, t.baseURL+path, rdr) + if err != nil { + return nil, fmt.Errorf("gitlab tracker: build request: %w", err) + } + if body != nil { + req.Header.Set("Content-Type", "application/json") + } + req.Header.Set("Accept", "application/json") + req.Header.Set("User-Agent", t.userAgent) + tok, err := t.tokens.Token(ctx) + if err != nil { + return nil, err + } + // PRIVATE-TOKEN is GitLab's recommended header for personal-access / + // project-access tokens. We intentionally do NOT also set + // Authorization: Bearer — sending both would be redundant and the + // recommended path is enough. + req.Header.Set("PRIVATE-TOKEN", tok) + + resp, err := t.http.Do(req) + if err != nil { + return nil, fmt.Errorf("gitlab tracker: %s %s: %w", method, path, err) + } + defer resp.Body.Close() + respBody, _ := io.ReadAll(resp.Body) + if resp.StatusCode >= 200 && resp.StatusCode < 300 { + return respBody, nil + } + return respBody, classifyError(resp, respBody) +} + +func classifyError(resp *http.Response, body []byte) error { + msg := gitlabMessage(body) + switch resp.StatusCode { + case http.StatusNotFound: + return fmt.Errorf("%w: %s", ErrNotFound, msg) + case http.StatusTooManyRequests: + // GitLab's canonical rate-limit status. Headers may include + // RateLimit-Remaining / RateLimit-Reset / Retry-After (no X- prefix). + return rateLimited(resp, msg) + case http.StatusUnauthorized: + // 401 is unambiguously an auth failure on GitLab. + return fmt.Errorf("%w: %s", ErrAuthFailed, msg) + case http.StatusForbidden: + // On GitLab, 403 without rate-limit headers is permission denied + // (token lacks the right scope, project not visible). 403 with + // rate-limit signals is rare but handle it the same way as 429 + // to be safe. + if isRateLimited(resp, msg) { + return rateLimited(resp, msg) + } + return fmt.Errorf("%w: %s", ErrAuthFailed, msg) + } + return fmt.Errorf("gitlab tracker: %d %s", resp.StatusCode, msg) +} + +func isRateLimited(resp *http.Response, msg string) bool { + if rem := resp.Header.Get("RateLimit-Remaining"); rem != "" { + if n, err := strconv.Atoi(rem); err == nil && n == 0 { + return true + } + } + if resp.Header.Get("Retry-After") != "" { + return true + } + low := strings.ToLower(msg) + return strings.Contains(low, "rate limit") || strings.Contains(low, "retry later") || strings.Contains(low, "too many requests") +} + +func rateLimited(resp *http.Response, msg string) error { + e := &RateLimitError{Message: msg} + if reset := resp.Header.Get("RateLimit-Reset"); reset != "" { + if sec, err := strconv.ParseInt(reset, 10, 64); err == nil && sec > 0 { + e.ResetAt = time.Unix(sec, 0) + } + } + // Retry-After: v1 parses integer-seconds only. RFC 7231 also allows an + // HTTP-date form; GitLab's current implementation uses seconds, so the + // date form is out of scope for v1 and a future caller hitting it just + // won't populate RetryAfter — they can still back off via ResetAt or + // generic retry policy. + if ra := resp.Header.Get("Retry-After"); ra != "" { + if sec, err := strconv.Atoi(ra); err == nil && sec >= 0 { + e.RetryAfter = time.Duration(sec) * time.Second + } + } + return e +} + +func gitlabMessage(body []byte) string { + // GitLab error bodies vary: sometimes {"message":"..."}, sometimes + // {"error":"..."}, sometimes a nested {"message":{"base":["..."]}}. + // Try the two flat-string forms first; on failure fall back to the + // raw body trimmed. + var flat struct { + Message string `json:"message"` + Error string `json:"error"` + } + if json.Unmarshal(body, &flat) == nil { + if flat.Message != "" { + return flat.Message + } + if flat.Error != "" { + return flat.Error + } + } + return strings.TrimSpace(string(body)) +} + +// --------------------------------------------------------------------------- +// ID parsing +// --------------------------------------------------------------------------- + +func (t *Tracker) parseID(id domain.TrackerID) (project string, iid int, err error) { + // Strict: the Session Manager picks an adapter by Provider, so reaching + // this adapter with a non-gitlab Provider is a routing bug, not user + // input. Empty Provider is treated the same way. + if id.Provider != domain.TrackerProviderGitLab { + return "", 0, fmt.Errorf("%w: provider=%q", ErrWrongProvider, id.Provider) + } + return parseGitLabID(id.Native) +} + +// parseGitLabID accepts "group/project#iid" — including subgroup paths +// "group/sub/.../project#iid" of arbitrary depth — and returns the project +// path and IID. Bare numbers and forms without the # separator are +// intentionally rejected so the rest of the system has one canonical id +// shape. +func parseGitLabID(native string) (project string, iid int, err error) { + hash := strings.IndexByte(native, '#') + if hash < 0 { + return "", 0, fmt.Errorf("%w: missing #iid", ErrBadID) + } + project = native[:hash] + numPart := native[hash+1:] + if err := validateProjectPath(project); err != nil { + return "", 0, err + } + n, parseErr := strconv.Atoi(numPart) + if parseErr != nil || n <= 0 { + return "", 0, fmt.Errorf("%w: bad iid %q", ErrBadID, numPart) + } + return project, n, nil +} + +// parseGitLabRepo accepts "group/project" or "group/.../project" and +// rejects empty segments, embedded "#", and whitespace. +func parseGitLabRepo(native string) (string, error) { + if native == "" { + return "", fmt.Errorf("%w: empty repo", ErrBadID) + } + if err := validateProjectPath(native); err != nil { + return "", err + } + return native, nil +} + +// validateProjectPath enforces the shared rules for both Get and List +// inputs: at least two non-empty segments, no embedded whitespace or "#". +// Subgroup nesting of arbitrary depth is allowed. +func validateProjectPath(p string) error { + if p == "" { + return fmt.Errorf("%w: empty project path", ErrBadID) + } + if strings.ContainsAny(p, "# \t\n\r") { + return fmt.Errorf("%w: invalid characters in project path %q", ErrBadID, p) + } + segs := strings.Split(p, "/") + if len(segs) < 2 { + return fmt.Errorf("%w: project path needs at least group/project", ErrBadID) + } + for _, s := range segs { + if s == "" { + return fmt.Errorf("%w: empty segment in project path %q", ErrBadID, p) + } + } + return nil +} diff --git a/backend/internal/adapters/tracker/gitlab/tracker_test.go b/backend/internal/adapters/tracker/gitlab/tracker_test.go new file mode 100644 index 00000000..116d94d5 --- /dev/null +++ b/backend/internal/adapters/tracker/gitlab/tracker_test.go @@ -0,0 +1,623 @@ +package gitlab + +import ( + "context" + "encoding/json" + "errors" + "io" + "net/http" + "net/http/httptest" + "reflect" + "strconv" + "strings" + "sync" + "testing" + "time" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" +) + +// recordedReq captures one inbound HTTP request so tests can assert against +// the exact GitLab API surface the adapter touched. EscapedPath preserves the +// URL-encoded form so subgroup tests can prove the path was escaped properly +// (r.URL.Path is decoded by Go's http server). +type recordedReq struct { + Method string + Path string + EscapedPath string + Body string +} + +// fakeGL is a programmable httptest.Server matching by "METHOD path" (decoded +// path). Unmatched requests fail the test — that's the point of TDD, so an +// accidental extra call is loud. +type fakeGL struct { + t *testing.T + server *httptest.Server + mu sync.Mutex + requests []recordedReq + handlers map[string]http.HandlerFunc +} + +func newFakeGL(t *testing.T) *fakeGL { + t.Helper() + f := &fakeGL{t: t, handlers: map[string]http.HandlerFunc{}} + f.server = httptest.NewServer(http.HandlerFunc(f.serve)) + t.Cleanup(f.server.Close) + return f +} + +func (f *fakeGL) on(method, path string, h http.HandlerFunc) { + f.mu.Lock() + defer f.mu.Unlock() + f.handlers[method+" "+path] = h +} + +func (f *fakeGL) serve(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + key := r.Method + " " + r.URL.Path + f.mu.Lock() + f.requests = append(f.requests, recordedReq{ + Method: r.Method, + Path: r.URL.Path, + EscapedPath: r.URL.EscapedPath(), + Body: string(body), + }) + h, ok := f.handlers[key] + f.mu.Unlock() + if !ok { + f.t.Errorf("unexpected request: %s", key) + http.Error(w, "no handler", http.StatusNotImplemented) + return + } + r.Body = io.NopCloser(strings.NewReader(string(body))) + h(w, r) +} + +func (f *fakeGL) calls() []recordedReq { + f.mu.Lock() + defer f.mu.Unlock() + out := make([]recordedReq, len(f.requests)) + copy(out, f.requests) + return out +} + +func newTrackerForTest(t *testing.T, f *fakeGL) *Tracker { + t.Helper() + tr, err := New(Options{ + BaseURL: f.server.URL, + Token: StaticTokenSource("tkn-test"), + HTTPClient: f.server.Client(), + }) + if err != nil { + t.Fatalf("New: %v", err) + } + return tr +} + +func ctx() context.Context { return context.Background() } + +func TestNewRejectsMissingToken(t *testing.T) { + if _, err := New(Options{Token: StaticTokenSource("")}); !errors.Is(err, ErrNoToken) { + t.Fatalf("New with empty token = %v, want ErrNoToken", err) + } + if _, err := New(Options{}); !errors.Is(err, ErrNoToken) { + t.Fatalf("New with no source = %v, want ErrNoToken", err) + } +} + +func TestParseID(t *testing.T) { + cases := []struct { + name string + native string + wantProject string + wantIID int + wantErr bool + }{ + {"happy", "group/project#42", "group/project", 42, false}, + {"subgroup one level", "group/sub/project#7", "group/sub/project", 7, false}, + {"subgroup deeper", "group/sub1/sub2/project#1", "group/sub1/sub2/project", 1, false}, + {"missing hash", "group/project", "", 0, true}, + {"missing slash", "project#42", "", 0, true}, + {"empty leading segment", "/project#1", "", 0, true}, + {"empty trailing segment", "group/#1", "", 0, true}, + {"empty middle segment", "group//project#1", "", 0, true}, + {"non-numeric", "g/p#abc", "", 0, true}, + {"zero iid", "g/p#0", "", 0, true}, + {"negative iid", "g/p#-1", "", 0, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + project, iid, err := parseGitLabID(tc.native) + if tc.wantErr { + if err == nil { + t.Fatalf("expected error, got %s#%d", project, iid) + } + return + } + if err != nil { + t.Fatalf("parse: %v", err) + } + if project != tc.wantProject || iid != tc.wantIID { + t.Fatalf("got %s#%d, want %s#%d", project, iid, tc.wantProject, tc.wantIID) + } + }) + } +} + +func TestParseRepo(t *testing.T) { + cases := []struct { + name string + native string + want string + wantErr bool + }{ + {"top-level", "group/project", "group/project", false}, + {"subgroup", "group/sub/project", "group/sub/project", false}, + {"deep subgroup", "g/a/b/c/project", "g/a/b/c/project", false}, + {"empty", "", "", true}, + {"single segment", "project", "", true}, + {"leading slash", "/project", "", true}, + {"trailing slash", "group/", "", true}, + {"empty middle", "group//project", "", true}, + {"embedded hash", "group/pro#ject", "", true}, + {"embedded space", "group/pro ject", "", true}, + {"embedded newline", "group/project\n", "", true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := parseGitLabRepo(tc.native) + if tc.wantErr { + if err == nil { + t.Fatalf("expected error, got %q", got) + } + return + } + if err != nil { + t.Fatalf("parse: %v", err) + } + if got != tc.want { + t.Fatalf("got %q, want %q", got, tc.want) + } + }) + } +} + +func TestGet_HappyPath(t *testing.T) { + f := newFakeGL(t) + f.on("GET", "/projects/group/project/issues/42", func(w http.ResponseWriter, r *http.Request) { + if got := r.Header.Get("PRIVATE-TOKEN"); got != "tkn-test" { + t.Errorf("PRIVATE-TOKEN = %q, want tkn-test", got) + } + if got := r.Header.Get("Authorization"); got != "" { + t.Errorf("Authorization = %q, want empty (PRIVATE-TOKEN auth)", got) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "iid": 42, + "title": "Found a bug", + "description": "It does not work", + "state": "opened", + "web_url": "https://gitlab.com/group/project/-/issues/42", + "labels": ["bug","in-progress"], + "assignees": [{"username":"alice"},{"username":"bob"}] + }`)) + }) + tr := newTrackerForTest(t, f) + + issue, err := tr.Get(ctx(), domain.TrackerID{Provider: domain.TrackerProviderGitLab, Native: "group/project#42"}) + if err != nil { + t.Fatalf("Get: %v", err) + } + want := domain.Issue{ + ID: domain.TrackerID{Provider: domain.TrackerProviderGitLab, Native: "group/project#42"}, + Title: "Found a bug", + Body: "It does not work", + State: domain.IssueInProgress, + URL: "https://gitlab.com/group/project/-/issues/42", + Labels: []string{"bug", "in-progress"}, + Assignees: []string{"alice", "bob"}, + } + if !reflect.DeepEqual(issue, want) { + t.Fatalf("issue = %#v\nwant %#v", issue, want) + } +} + +// TestGet_URLEncodesSubgroupPath proves the FULL project path is URL-encoded +// when forming the endpoint — otherwise GitLab returns 404 for any project +// inside a subgroup. +func TestGet_URLEncodesSubgroupPath(t *testing.T) { + f := newFakeGL(t) + f.on("GET", "/projects/group/sub/project/issues/5", func(w http.ResponseWriter, r *http.Request) { + // The wire path must contain encoded slashes, otherwise GitLab will + // route the request as /projects/group/sub/project/issues/5 instead + // of /projects//issues/5. + if !strings.Contains(r.URL.EscapedPath(), "%2F") { + t.Errorf("escaped path = %q, want slashes encoded as %%2F", r.URL.EscapedPath()) + } + _, _ = w.Write([]byte(`{"iid":5,"title":"t","description":"","state":"opened","web_url":"https://gitlab.com/group/sub/project/-/issues/5"}`)) + }) + tr := newTrackerForTest(t, f) + issue, err := tr.Get(ctx(), domain.TrackerID{Provider: domain.TrackerProviderGitLab, Native: "group/sub/project#5"}) + if err != nil { + t.Fatalf("Get: %v", err) + } + if issue.ID.Native != "group/sub/project#5" { + t.Fatalf("native = %q, want group/sub/project#5", issue.ID.Native) + } +} + +func TestGet_StateMapping(t *testing.T) { + cases := []struct { + name string + state string + labels []string + wantState domain.NormalizedIssueState + }{ + {"plain opened", "opened", nil, domain.IssueOpen}, + {"opened with in-progress", "opened", []string{"in-progress"}, domain.IssueInProgress}, + {"opened with in-review", "opened", []string{"in-review"}, domain.IssueInReview}, + {"review wins over progress", "opened", []string{"in-progress", "in-review"}, domain.IssueInReview}, + {"closed plain", "closed", nil, domain.IssueDone}, + {"closed cancelled label", "closed", []string{"cancelled"}, domain.IssueCancelled}, + {"closed wontfix label", "closed", []string{"wontfix"}, domain.IssueCancelled}, + {"closed mixed labels still cancelled", "closed", []string{"bug", "wontfix"}, domain.IssueCancelled}, + // Label matching is case-insensitive — GitLab label names are + // case-insensitive in the UI, so a user typing "In-Progress" or + // "Cancelled" should be recognized. + {"opened mixed-case in-progress", "opened", []string{"In-Progress"}, domain.IssueInProgress}, + {"opened upper-case in-review", "opened", []string{"IN-REVIEW"}, domain.IssueInReview}, + {"closed mixed-case cancelled", "closed", []string{"Cancelled"}, domain.IssueCancelled}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + f := newFakeGL(t) + payload := map[string]any{ + "iid": 1, + "title": "t", + "description": "", + "state": tc.state, + "web_url": "https://gitlab.com/g/p/-/issues/1", + } + if tc.labels != nil { + payload["labels"] = tc.labels + } + b, _ := json.Marshal(payload) + f.on("GET", "/projects/g/p/issues/1", func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write(b) + }) + tr := newTrackerForTest(t, f) + issue, err := tr.Get(ctx(), domain.TrackerID{Provider: domain.TrackerProviderGitLab, Native: "g/p#1"}) + if err != nil { + t.Fatalf("Get: %v", err) + } + if issue.State != tc.wantState { + t.Fatalf("state = %q, want %q", issue.State, tc.wantState) + } + }) + } +} + +func TestGet_NotFound(t *testing.T) { + f := newFakeGL(t) + f.on("GET", "/projects/g/p/issues/1", func(w http.ResponseWriter, r *http.Request) { + http.Error(w, `{"message":"404 Not found"}`, http.StatusNotFound) + }) + tr := newTrackerForTest(t, f) + _, err := tr.Get(ctx(), domain.TrackerID{Provider: domain.TrackerProviderGitLab, Native: "g/p#1"}) + if !errors.Is(err, ErrNotFound) { + t.Fatalf("err = %v, want ErrNotFound", err) + } +} + +// TestGet_RateLimited covers GitLab's 429 path. Headers are RateLimit-Remaining +// / RateLimit-Reset (no X- prefix, unlike GitHub). +func TestGet_RateLimited(t *testing.T) { + f := newFakeGL(t) + reset := time.Now().Add(2 * time.Minute).Unix() + f.on("GET", "/projects/g/p/issues/1", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("RateLimit-Remaining", "0") + w.Header().Set("RateLimit-Reset", strconv.FormatInt(reset, 10)) + http.Error(w, `{"message":"Retry later"}`, http.StatusTooManyRequests) + }) + tr := newTrackerForTest(t, f) + _, err := tr.Get(ctx(), domain.TrackerID{Provider: domain.TrackerProviderGitLab, Native: "g/p#1"}) + if !errors.Is(err, ErrRateLimited) { + t.Fatalf("err = %v, want ErrRateLimited", err) + } + var rle *RateLimitError + if !errors.As(err, &rle) { + t.Fatalf("err = %v, want *RateLimitError", err) + } + if got := rle.ResetAt.Unix(); got != reset { + t.Fatalf("ResetAt = %d, want %d", got, reset) + } +} + +// TestGet_RateLimitedRetryAfter covers the path where GitLab returns 429 with +// only the Retry-After header (e.g. some application-level limits). +func TestGet_RateLimitedRetryAfter(t *testing.T) { + f := newFakeGL(t) + f.on("GET", "/projects/g/p/issues/1", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Retry-After", "60") + http.Error(w, `{"message":"This endpoint has been requested too many times."}`, http.StatusTooManyRequests) + }) + tr := newTrackerForTest(t, f) + _, err := tr.Get(ctx(), domain.TrackerID{Provider: domain.TrackerProviderGitLab, Native: "g/p#1"}) + if !errors.Is(err, ErrRateLimited) { + t.Fatalf("err = %v, want ErrRateLimited", err) + } + var rle *RateLimitError + if !errors.As(err, &rle) { + t.Fatalf("err = %v, want *RateLimitError", err) + } + if rle.RetryAfter != 60*time.Second { + t.Fatalf("RetryAfter = %v, want 60s", rle.RetryAfter) + } +} + +func TestGet_RejectsWrongProvider(t *testing.T) { + f := newFakeGL(t) + tr := newTrackerForTest(t, f) + _, err := tr.Get(ctx(), domain.TrackerID{Provider: domain.TrackerProviderGitHub, Native: "o/r#1"}) + if !errors.Is(err, ErrWrongProvider) { + t.Fatalf("err = %v, want ErrWrongProvider", err) + } +} + +func TestGet_RejectsEmptyProvider(t *testing.T) { + f := newFakeGL(t) + tr := newTrackerForTest(t, f) + _, err := tr.Get(ctx(), domain.TrackerID{Native: "g/p#1"}) + if !errors.Is(err, ErrWrongProvider) { + t.Fatalf("err = %v, want ErrWrongProvider", err) + } +} + +// TestGet_CanonicalizesProviderOnOutput pins that returned Issues always carry +// domain.TrackerProviderGitLab so callers can re-route without inspecting +// which adapter they originally talked to. +func TestGet_CanonicalizesProviderOnOutput(t *testing.T) { + f := newFakeGL(t) + f.on("GET", "/projects/g/p/issues/1", func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"iid":1,"title":"t","description":"","state":"opened","web_url":"https://gitlab.com/g/p/-/issues/1"}`)) + }) + tr := newTrackerForTest(t, f) + issue, err := tr.Get(ctx(), domain.TrackerID{Provider: domain.TrackerProviderGitLab, Native: "g/p#1"}) + if err != nil { + t.Fatalf("Get: %v", err) + } + if issue.ID.Provider != domain.TrackerProviderGitLab { + t.Fatalf("issue.ID.Provider = %q, want %q", issue.ID.Provider, domain.TrackerProviderGitLab) + } + if issue.ID.Native != "g/p#1" { + t.Fatalf("issue.ID.Native = %q, want g/p#1", issue.ID.Native) + } +} + +// TestGet_AuthFailed locks in that a 401 maps to ErrAuthFailed. +func TestGet_AuthFailed(t *testing.T) { + f := newFakeGL(t) + f.on("GET", "/projects/g/p/issues/1", func(w http.ResponseWriter, r *http.Request) { + http.Error(w, `{"message":"401 Unauthorized"}`, http.StatusUnauthorized) + }) + tr := newTrackerForTest(t, f) + _, err := tr.Get(ctx(), domain.TrackerID{Provider: domain.TrackerProviderGitLab, Native: "g/p#1"}) + if !errors.Is(err, ErrAuthFailed) { + t.Fatalf("err = %v, want ErrAuthFailed", err) + } +} + +// TestGet_ForbiddenIsAuth pins that 403 without rate-limit signals maps to +// ErrAuthFailed (not ErrRateLimited). +func TestGet_ForbiddenIsAuth(t *testing.T) { + f := newFakeGL(t) + f.on("GET", "/projects/g/p/issues/1", func(w http.ResponseWriter, r *http.Request) { + http.Error(w, `{"message":"403 Forbidden"}`, http.StatusForbidden) + }) + tr := newTrackerForTest(t, f) + _, err := tr.Get(ctx(), domain.TrackerID{Provider: domain.TrackerProviderGitLab, Native: "g/p#1"}) + if !errors.Is(err, ErrAuthFailed) { + t.Fatalf("err = %v, want ErrAuthFailed", err) + } +} + +// --------------------------------------------------------------------------- +// Preflight +// --------------------------------------------------------------------------- + +func TestPreflight_HappyPath(t *testing.T) { + f := newFakeGL(t) + f.on("GET", "/user", func(w http.ResponseWriter, r *http.Request) { + if got := r.Header.Get("PRIVATE-TOKEN"); got != "tkn-test" { + t.Errorf("PRIVATE-TOKEN = %q", got) + } + _, _ = w.Write([]byte(`{"id":1,"username":"alice"}`)) + }) + tr := newTrackerForTest(t, f) + if err := tr.Preflight(ctx()); err != nil { + t.Fatalf("Preflight: %v", err) + } +} + +func TestPreflight_InvalidToken(t *testing.T) { + f := newFakeGL(t) + f.on("GET", "/user", func(w http.ResponseWriter, r *http.Request) { + http.Error(w, `{"message":"401 Unauthorized"}`, http.StatusUnauthorized) + }) + tr := newTrackerForTest(t, f) + err := tr.Preflight(ctx()) + if !errors.Is(err, ErrAuthFailed) { + t.Fatalf("err = %v, want ErrAuthFailed", err) + } +} + +// TestPreflight_CachesSuccess pins that successful checks are cached. +func TestPreflight_CachesSuccess(t *testing.T) { + f := newFakeGL(t) + f.on("GET", "/user", func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"id":1,"username":"alice"}`)) + }) + tr := newTrackerForTest(t, f) + for i := 0; i < 5; i++ { + if err := tr.Preflight(ctx()); err != nil { + t.Fatalf("Preflight #%d: %v", i, err) + } + } + if got := len(f.calls()); got != 1 { + t.Fatalf("HTTP calls = %d, want 1 (success should be cached)", got) + } +} + +// TestPreflight_RetriesAfterFailure pins that failures are NOT cached. +func TestPreflight_RetriesAfterFailure(t *testing.T) { + f := newFakeGL(t) + var calls int + f.on("GET", "/user", func(w http.ResponseWriter, r *http.Request) { + calls++ + if calls == 1 { + http.Error(w, `{"message":"server exploded"}`, http.StatusInternalServerError) + return + } + _, _ = w.Write([]byte(`{"id":1,"username":"alice"}`)) + }) + tr := newTrackerForTest(t, f) + if err := tr.Preflight(ctx()); err == nil { + t.Fatalf("first Preflight expected to fail") + } + if err := tr.Preflight(ctx()); err != nil { + t.Fatalf("second Preflight: %v", err) + } + if got := len(f.calls()); got != 2 { + t.Fatalf("HTTP calls = %d, want 2 (first fail not cached)", got) + } +} + +// --------------------------------------------------------------------------- +// List +// --------------------------------------------------------------------------- + +func TestList_HappyPathAndDefaults(t *testing.T) { + f := newFakeGL(t) + f.on("GET", "/projects/g/p/issues", func(w http.ResponseWriter, r *http.Request) { + q := r.URL.Query() + // GitLab state names are different from GitHub: "all" / "opened" / "closed". + if got := q.Get("state"); got != "all" { + t.Errorf("state = %q, want all (default)", got) + } + if got := q.Get("per_page"); got != "30" { + t.Errorf("per_page = %q, want 30 (default)", got) + } + _, _ = w.Write([]byte(`[ + {"iid":1,"title":"first","description":"b1","state":"opened","web_url":"https://gitlab.com/g/p/-/issues/1","labels":["bug"],"assignees":[]}, + {"iid":2,"title":"second","description":"b2","state":"closed","web_url":"https://gitlab.com/g/p/-/issues/2","labels":[],"assignees":[{"username":"alice"}]} + ]`)) + }) + tr := newTrackerForTest(t, f) + issues, err := tr.List(ctx(), domain.TrackerRepo{Provider: domain.TrackerProviderGitLab, Native: "g/p"}, domain.ListFilter{}) + if err != nil { + t.Fatalf("List: %v", err) + } + if len(issues) != 2 { + t.Fatalf("len = %d, want 2", len(issues)) + } + if issues[0].ID.Native != "g/p#1" || issues[0].State != domain.IssueOpen || issues[0].Title != "first" { + t.Fatalf("issues[0] = %#v", issues[0]) + } + if issues[1].ID.Native != "g/p#2" || issues[1].State != domain.IssueDone || len(issues[1].Assignees) != 1 || issues[1].Assignees[0] != "alice" { + t.Fatalf("issues[1] = %#v", issues[1]) + } +} + +// TestList_URLEncodesSubgroupPath proves subgroup repos work for List too. +func TestList_URLEncodesSubgroupPath(t *testing.T) { + f := newFakeGL(t) + f.on("GET", "/projects/group/sub/project/issues", func(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.URL.EscapedPath(), "%2F") { + t.Errorf("escaped path = %q, want slashes encoded", r.URL.EscapedPath()) + } + _, _ = w.Write([]byte(`[]`)) + }) + tr := newTrackerForTest(t, f) + if _, err := tr.List(ctx(), domain.TrackerRepo{Provider: domain.TrackerProviderGitLab, Native: "group/sub/project"}, domain.ListFilter{}); err != nil { + t.Fatalf("List: %v", err) + } +} + +func TestList_QueryEncoding(t *testing.T) { + cases := []struct { + name string + filter domain.ListFilter + wantQ map[string]string + }{ + { + name: "opened + labels + assignee_username + limit", + filter: domain.ListFilter{State: domain.ListOpen, Labels: []string{"bug", "help wanted"}, Assignee: "alice", Limit: 50}, + wantQ: map[string]string{"state": "opened", "labels": "bug,help wanted", "assignee_username": "alice", "per_page": "50"}, + }, + { + name: "closed only", + filter: domain.ListFilter{State: domain.ListClosed}, + wantQ: map[string]string{"state": "closed", "per_page": "30"}, + }, + { + name: "limit capped at 100", + filter: domain.ListFilter{Limit: 9999}, + wantQ: map[string]string{"state": "all", "per_page": "100"}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + f := newFakeGL(t) + f.on("GET", "/projects/g/p/issues", func(w http.ResponseWriter, r *http.Request) { + got := r.URL.Query() + for k, want := range tc.wantQ { + if g := got.Get(k); g != want { + t.Errorf("query[%q] = %q, want %q", k, g, want) + } + } + _, _ = w.Write([]byte(`[]`)) + }) + tr := newTrackerForTest(t, f) + if _, err := tr.List(ctx(), domain.TrackerRepo{Provider: domain.TrackerProviderGitLab, Native: "g/p"}, tc.filter); err != nil { + t.Fatalf("List: %v", err) + } + }) + } +} + +func TestList_RejectsWrongProvider(t *testing.T) { + f := newFakeGL(t) + tr := newTrackerForTest(t, f) + _, err := tr.List(ctx(), domain.TrackerRepo{Provider: domain.TrackerProviderGitHub, Native: "o/r"}, domain.ListFilter{}) + if !errors.Is(err, ErrWrongProvider) { + t.Fatalf("err = %v, want ErrWrongProvider", err) + } + if calls := f.calls(); len(calls) != 0 { + t.Fatalf("unexpected HTTP calls: %#v", calls) + } +} + +func TestList_RejectsBadRepo(t *testing.T) { + cases := []string{ + "", // empty + "noseparator", // missing / + "/project", // empty leading + "group/", // empty trailing + "group//proj", // empty middle + "group/pro ject", // embedded space + "group/pro#ject", // embedded # + "\tgroup/project", // leading tab + "group/project\n", // trailing newline + } + for _, native := range cases { + t.Run(native, func(t *testing.T) { + f := newFakeGL(t) + tr := newTrackerForTest(t, f) + _, err := tr.List(ctx(), domain.TrackerRepo{Provider: domain.TrackerProviderGitLab, Native: native}, domain.ListFilter{}) + if !errors.Is(err, ErrBadID) { + t.Fatalf("native=%q: err = %v, want ErrBadID", native, err) + } + }) + } +} From 1b78ea4b0d383c327e43aef0e6af83cfac2f40df Mon Sep 17 00:00:00 2001 From: harshitsinghbhandari <24b4506@iitb.ac.in> Date: Sun, 31 May 2026 00:07:41 +0530 Subject: [PATCH 2/2] feat(tracker): glab auth token fallback in EnvTokenSource MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zero-config auth per AO-26 directive: a user who has run `glab auth login` gets working credentials with no extra setup. Lookup order is now 1. configured EnvVars (first non-empty wins) 2. GITLAB_TOKEN 3. `glab auth token` 4. else ErrNoToken The glab fallback is on by default — no opt-in flag. Failure (binary missing, not logged in, empty stdout) falls through silently to ErrNoToken so the caller's failure mode stays uniform regardless of whether glab is installed. Stderr is discarded for the same reason: an unauthenticated glab shouldn't print noise during a silent fallthrough. Tests inject a GLAB hook on EnvTokenSource to assert lookup order without touching $PATH. The hook signature matches the parallel `gh auth token` work coming to the GitHub adapter so both auth.go shapes stay parallel. Co-Authored-By: Claude Opus 4.7 --- .../internal/adapters/tracker/gitlab/auth.go | 56 +++++- .../adapters/tracker/gitlab/auth_test.go | 166 ++++++++++++++++++ 2 files changed, 218 insertions(+), 4 deletions(-) create mode 100644 backend/internal/adapters/tracker/gitlab/auth_test.go diff --git a/backend/internal/adapters/tracker/gitlab/auth.go b/backend/internal/adapters/tracker/gitlab/auth.go index c668692d..c941c61a 100644 --- a/backend/internal/adapters/tracker/gitlab/auth.go +++ b/backend/internal/adapters/tracker/gitlab/auth.go @@ -3,7 +3,9 @@ package gitlab import ( "context" "errors" + "io" "os" + "os/exec" "strings" ) @@ -29,14 +31,31 @@ func (s StaticTokenSource) Token(context.Context) (string, error) { return t, nil } -// EnvTokenSource reads the first non-empty value from the listed env vars, -// falling back to GITLAB_TOKEN. The order matters: a project-configured token -// (e.g. AO_GITLAB_TOKEN) should be preferred over the global default. +// EnvTokenSource resolves a token from, in order: +// +// 1. The configured EnvVars (first non-empty wins). +// 2. GITLAB_TOKEN. +// 3. `glab auth token` (the GitLab CLI), if installed. +// +// The glab fallback is on by default — there is no opt-in flag. If glab is +// not installed, fails, or returns nothing, the source falls through silently +// to ErrNoToken; the exec error is NOT propagated, so the caller's failure +// mode is uniform ("no token configured") regardless of whether the user has +// glab installed. This matches the zero-friction directive AO-26: a user +// who has already done `glab auth login` gets working credentials with no +// extra setup, while CI environments using explicit env vars are unaffected. +// +// GLAB is the injection seam for tests. Production callers leave it nil and +// the source uses the real `glab auth token` exec; tests replace it with a +// fake to assert lookup order and contract behavior without touching $PATH. +// The signature mirrors the GitHub adapter's parallel `gh auth token` hook +// so both adapters' auth.go shapes stay parallel. type EnvTokenSource struct { EnvVars []string + GLAB func(ctx context.Context) (string, error) } -func (s EnvTokenSource) Token(context.Context) (string, error) { +func (s EnvTokenSource) Token(ctx context.Context) (string, error) { for _, name := range s.EnvVars { if v := strings.TrimSpace(os.Getenv(name)); v != "" { return v, nil @@ -45,5 +64,34 @@ func (s EnvTokenSource) Token(context.Context) (string, error) { if v := strings.TrimSpace(os.Getenv("GITLAB_TOKEN")); v != "" { return v, nil } + fn := s.GLAB + if fn == nil { + fn = defaultGlabToken + } + // Silent fallthrough: any error or empty output from glab maps to + // ErrNoToken. We intentionally do NOT surface the exec error — a missing + // CLI binary is the common case and shouldn't look like a configuration + // problem to the caller. + if tok, err := fn(ctx); err == nil { + if trimmed := strings.TrimSpace(tok); trimmed != "" { + return trimmed, nil + } + } return "", ErrNoToken } + +// defaultGlabToken shells out to `glab auth token`. The ctx is threaded +// through so a caller wiring a startup deadline gets honored. Any error +// (binary missing, not logged in, glab unhappy for any reason) is returned +// for the caller to discard — see EnvTokenSource.Token's silent-fallthrough +// rule. Stderr is discarded so an unauthenticated glab doesn't print noise +// during a fallthrough that the caller has chosen to handle silently. +func defaultGlabToken(ctx context.Context) (string, error) { + cmd := exec.CommandContext(ctx, "glab", "auth", "token") + cmd.Stderr = io.Discard + out, err := cmd.Output() + if err != nil { + return "", err + } + return string(out), nil +} diff --git a/backend/internal/adapters/tracker/gitlab/auth_test.go b/backend/internal/adapters/tracker/gitlab/auth_test.go new file mode 100644 index 00000000..b4642ed4 --- /dev/null +++ b/backend/internal/adapters/tracker/gitlab/auth_test.go @@ -0,0 +1,166 @@ +package gitlab + +import ( + "context" + "errors" + "testing" +) + +// TestEnvTokenSource_EnvVarWins pins that a configured env var, when present, +// short-circuits both GITLAB_TOKEN and the glab fallback. The glab fake is +// installed to prove it is NOT consulted on the happy path. +func TestEnvTokenSource_EnvVarWins(t *testing.T) { + t.Setenv("AO_GITLAB_TOKEN", "from-env") + t.Setenv("GITLAB_TOKEN", "from-default-env") + src := EnvTokenSource{ + EnvVars: []string{"AO_GITLAB_TOKEN"}, + GLAB: func(context.Context) (string, error) { + t.Fatalf("glab fallback called even though env var was set") + return "", nil + }, + } + got, err := src.Token(context.Background()) + if err != nil { + t.Fatalf("Token: %v", err) + } + if got != "from-env" { + t.Fatalf("token = %q, want from-env", got) + } +} + +// TestEnvTokenSource_DefaultEnvWins pins that GITLAB_TOKEN is used when the +// configured EnvVars list is empty or yields nothing, before falling through +// to glab. +func TestEnvTokenSource_DefaultEnvWins(t *testing.T) { + t.Setenv("GITLAB_TOKEN", "from-default-env") + src := EnvTokenSource{ + GLAB: func(context.Context) (string, error) { + t.Fatalf("glab fallback called even though GITLAB_TOKEN was set") + return "", nil + }, + } + got, err := src.Token(context.Background()) + if err != nil { + t.Fatalf("Token: %v", err) + } + if got != "from-default-env" { + t.Fatalf("token = %q, want from-default-env", got) + } +} + +// TestEnvTokenSource_GlabFallback pins that when no env var yields a token, +// the glab shell-out is used. +func TestEnvTokenSource_GlabFallback(t *testing.T) { + t.Setenv("GITLAB_TOKEN", "") + var called bool + src := EnvTokenSource{ + GLAB: func(context.Context) (string, error) { + called = true + return "glpat-from-glab", nil + }, + } + got, err := src.Token(context.Background()) + if err != nil { + t.Fatalf("Token: %v", err) + } + if !called { + t.Fatalf("glab fallback was not called") + } + if got != "glpat-from-glab" { + t.Fatalf("token = %q, want glpat-from-glab", got) + } +} + +// TestEnvTokenSource_GlabFallbackTrimsWhitespace pins that the glab stdout is +// trimmed (the real CLI ends with a newline). +func TestEnvTokenSource_GlabFallbackTrimsWhitespace(t *testing.T) { + t.Setenv("GITLAB_TOKEN", "") + src := EnvTokenSource{ + GLAB: func(context.Context) (string, error) { + return " glpat-padded\n", nil + }, + } + got, err := src.Token(context.Background()) + if err != nil { + t.Fatalf("Token: %v", err) + } + if got != "glpat-padded" { + t.Fatalf("token = %q, want glpat-padded", got) + } +} + +// TestEnvTokenSource_GlabFailureFallsThrough pins the silent-fallthrough +// contract: if glab errors (not installed, not logged in, etc.) the source +// returns ErrNoToken instead of propagating the exec error. This keeps the +// caller's failure mode uniform — "no token configured" — regardless of +// whether the user has glab installed. +func TestEnvTokenSource_GlabFailureFallsThrough(t *testing.T) { + t.Setenv("GITLAB_TOKEN", "") + src := EnvTokenSource{ + GLAB: func(context.Context) (string, error) { + return "", errors.New("exec: glab: executable file not found in $PATH") + }, + } + _, err := src.Token(context.Background()) + if !errors.Is(err, ErrNoToken) { + t.Fatalf("err = %v, want ErrNoToken (glab error should fall through silently)", err) + } +} + +// TestEnvTokenSource_GlabEmptyFallsThrough pins that an empty (whitespace-only) +// glab output is treated as "no token" rather than as a valid token of zero +// length — otherwise StaticTokenSource-style downstream checks would yield +// "no token configured" inconsistently. +func TestEnvTokenSource_GlabEmptyFallsThrough(t *testing.T) { + t.Setenv("GITLAB_TOKEN", "") + src := EnvTokenSource{ + GLAB: func(context.Context) (string, error) { + return " \n", nil + }, + } + _, err := src.Token(context.Background()) + if !errors.Is(err, ErrNoToken) { + t.Fatalf("err = %v, want ErrNoToken (empty glab output should fall through)", err) + } +} + +// TestEnvTokenSource_NilGlabUsesRealExec is a smoke test: with the GLAB field +// nil and no env vars, the source attempts a real exec. We don't depend on +// glab being installed on the test machine — we just assert that the +// fallthrough produces ErrNoToken (either because glab isn't installed OR +// because it has no token to give). Either way, the public contract holds: +// "no token configured" surfaces as ErrNoToken, never as a raw exec error. +func TestEnvTokenSource_NilGlabUsesRealExec(t *testing.T) { + t.Setenv("GITLAB_TOKEN", "") + src := EnvTokenSource{} // GLAB nil → defaults to real exec. + // Either glab is installed and returns a real token (rare on CI), or it + // fails / is missing. In both failure modes we must see ErrNoToken; in + // the rare success case we get a non-empty token. Both are valid. + tok, err := src.Token(context.Background()) + if err != nil && !errors.Is(err, ErrNoToken) { + t.Fatalf("err = %v, want ErrNoToken or a real token", err) + } + if err == nil && tok == "" { + t.Fatalf("got empty token with nil error") + } +} + +// TestEnvTokenSource_ContextThreadedToGlab pins that the ctx is passed through +// to the GLAB hook. The github adapter's parallel work uses the same shape, +// so any future caller wiring a deadline at startup gets honored. +func TestEnvTokenSource_ContextThreadedToGlab(t *testing.T) { + t.Setenv("GITLAB_TOKEN", "") + type ctxKey struct{} + parent := context.WithValue(context.Background(), ctxKey{}, "sentinel") + src := EnvTokenSource{ + GLAB: func(ctx context.Context) (string, error) { + if v, _ := ctx.Value(ctxKey{}).(string); v != "sentinel" { + t.Fatalf("ctx not threaded through to GLAB hook: got %v", v) + } + return "tok", nil + }, + } + if _, err := src.Token(parent); err != nil { + t.Fatalf("Token: %v", err) + } +}