diff --git a/backend/internal/adapters/tracker/gitlab/auth.go b/backend/internal/adapters/tracker/gitlab/auth.go new file mode 100644 index 00000000..c941c61a --- /dev/null +++ b/backend/internal/adapters/tracker/gitlab/auth.go @@ -0,0 +1,97 @@ +package gitlab + +import ( + "context" + "errors" + "io" + "os" + "os/exec" + "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 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(ctx 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 + } + 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) + } +} 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) + } + }) + } +}