Merge pull request #36 from aoagents/feat/aa-26
feat(tracker): Tracker port (Get + List + Preflight) + GitHub adapter
This commit is contained in:
Harshit Singh Bhandari
GitHub
commit
ff441aaece
|
|
@ -0,0 +1,52 @@
|
|||
package github
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"os"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// TokenSource yields a GitHub bearer token on demand. It is intentionally
|
||||
// tiny so tests can inject a static token and production can layer env-var or
|
||||
// gh-CLI fallbacks behind the same surface. 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("github 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 GITHUB_TOKEN. The order matters: a project-configured
|
||||
// token (e.g. AO_GITHUB_TOKEN) should be preferred over the global default,
|
||||
// matching the pattern PR #28 uses on the SCM side so both adapters honor
|
||||
// the same precedence.
|
||||
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("GITHUB_TOKEN")); v != "" {
|
||||
return v, nil
|
||||
}
|
||||
return "", ErrNoToken
|
||||
}
|
||||
|
|
@ -0,0 +1,42 @@
|
|||
// Package github implements the ports.Tracker outbound port for GitHub
|
||||
// Issues. 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 repo (one page, no
|
||||
// auto-pagination in v1; PRs are filtered out client-side because
|
||||
// GitHub's /issues endpoint conflates them).
|
||||
// - Preflight performs a single GET /user against GitHub 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.
|
||||
//
|
||||
// # Reverse state mapping
|
||||
//
|
||||
// GitHub Issues only have two native states (open, closed) plus a
|
||||
// state_reason on closed issues (completed, not_planned, reopened). Get
|
||||
// projects them onto the normalized state vocabulary as follows:
|
||||
//
|
||||
// - closed + state_reason=not_planned -> cancelled
|
||||
// - closed + (completed | empty | other) -> done
|
||||
// - open + "in-review" label -> review (wins when
|
||||
// both status labels are present; the workflow is progress -> review)
|
||||
// - open + "in-progress" label -> in_progress
|
||||
// - otherwise -> open
|
||||
//
|
||||
// The "in-progress" and "in-review" labels are recognized because humans
|
||||
// (and other tooling) commonly apply them. The adapter does NOT write them
|
||||
// in v1 — see issue #40 for the write-side work.
|
||||
//
|
||||
// # Out of scope
|
||||
//
|
||||
// - No Comment, no Transition (issue #40).
|
||||
// - No List pagination beyond a single page (callers requesting more than
|
||||
// 100 results need to wait for the observer/polling work in issue #35).
|
||||
// - No webhook receiver, no polling goroutine, no fact projection into
|
||||
// LCM (issue #35).
|
||||
// - No richer per-provider metadata on Issue (milestones, project boards,
|
||||
// reactions); the port only carries fields all v1 providers can fill.
|
||||
package github
|
||||
|
|
@ -0,0 +1,514 @@
|
|||
package github
|
||||
|
||||
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://api.github.com"
|
||||
defaultUserAgent = "ao-agent-orchestrator/tracker-github"
|
||||
|
||||
// Status labels used by humans (and other tooling) on GitHub Issues.
|
||||
// Get's reverse mapping recognizes them so an externally-labeled issue
|
||||
// reports as in_progress / review. The adapter does NOT write these
|
||||
// labels in v1 — see issue #40 for the write-side work.
|
||||
labelInProgress = "in-progress"
|
||||
labelInReview = "in-review"
|
||||
|
||||
stateClosedGH = "closed"
|
||||
reasonNotPlan = "not_planned"
|
||||
|
||||
// List pagination — GitHub's per_page maxes at 100. We default to 30
|
||||
// (matching the legacy gh CLI default) when the caller passes 0.
|
||||
defaultListLimit = 30
|
||||
maxListLimit = 100
|
||||
)
|
||||
|
||||
// Sentinel errors. Adapter-level callers should match on these via
|
||||
// errors.Is; the orchestrator's lifecycle code is intentionally insulated
|
||||
// from raw HTTP status codes.
|
||||
var (
|
||||
ErrNotFound = errors.New("github tracker: issue not found")
|
||||
ErrRateLimited = errors.New("github tracker: rate limited")
|
||||
ErrAuthFailed = errors.New("github tracker: authentication failed")
|
||||
ErrWrongProvider = errors.New("github tracker: id is not a github tracker id")
|
||||
ErrBadID = errors.New("github tracker: malformed native id")
|
||||
)
|
||||
|
||||
// RateLimitError is returned when GitHub 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 "github 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 GitHub REST API.
|
||||
//
|
||||
// Construction performs a fail-fast token presence check (no network call).
|
||||
// The first Preflight call validates the token against GitHub 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 GitHub.
|
||||
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. If this stops compiling, the
|
||||
// port shape changed and the adapter needs to follow.
|
||||
var _ ports.Tracker = (*Tracker)(nil)
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Get
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
// ghIssue is the subset of fields we read off the REST issue payload.
|
||||
// PullRequest is present (non-nil) iff GitHub considers this row a PR —
|
||||
// the /repos/{o}/{r}/issues endpoint conflates the two. List uses it to
|
||||
// filter PRs out client-side so the SM never sees a PR number as an issue.
|
||||
type ghIssue struct {
|
||||
Number int `json:"number"`
|
||||
Title string `json:"title"`
|
||||
Body string `json:"body"`
|
||||
State string `json:"state"`
|
||||
StateReason string `json:"state_reason"`
|
||||
HTMLURL string `json:"html_url"`
|
||||
Labels []ghLabel `json:"labels"`
|
||||
Assignees []ghUser `json:"assignees"`
|
||||
PullRequest *json.RawMessage `json:"pull_request,omitempty"`
|
||||
}
|
||||
|
||||
type ghLabel struct {
|
||||
Name string `json:"name"`
|
||||
}
|
||||
|
||||
type ghUser struct {
|
||||
Login string `json:"login"`
|
||||
}
|
||||
|
||||
func (t *Tracker) Get(ctx context.Context, id domain.TrackerID) (domain.Issue, error) {
|
||||
owner, repo, number, err := t.parseID(id)
|
||||
if err != nil {
|
||||
return domain.Issue{}, err
|
||||
}
|
||||
path := fmt.Sprintf("/repos/%s/%s/issues/%d", owner, repo, number)
|
||||
|
||||
resp, err := t.do(ctx, http.MethodGet, path, nil)
|
||||
if err != nil {
|
||||
return domain.Issue{}, err
|
||||
}
|
||||
var raw ghIssue
|
||||
if err := json.Unmarshal(resp, &raw); err != nil {
|
||||
return domain.Issue{}, fmt.Errorf("github tracker: decode issue: %w", err)
|
||||
}
|
||||
return issueFromGH(owner, repo, raw), nil
|
||||
}
|
||||
|
||||
// issueFromGH projects a raw GitHub issue payload into the normalized
|
||||
// domain.Issue. owner and repo are passed in because the TrackerID.Native
|
||||
// shape is "owner/repo#N" and we want the returned ID to round-trip
|
||||
// through the same adapter even if the original caller used a zero
|
||||
// Provider.
|
||||
func issueFromGH(owner, repo string, raw ghIssue) domain.Issue {
|
||||
labels := make([]string, 0, len(raw.Labels))
|
||||
for _, l := range raw.Labels {
|
||||
labels = append(labels, l.Name)
|
||||
}
|
||||
assignees := make([]string, 0, len(raw.Assignees))
|
||||
for _, a := range raw.Assignees {
|
||||
assignees = append(assignees, a.Login)
|
||||
}
|
||||
out := domain.Issue{
|
||||
ID: domain.TrackerID{
|
||||
Provider: domain.TrackerProviderGitHub,
|
||||
Native: fmt.Sprintf("%s/%s#%d", owner, repo, raw.Number),
|
||||
},
|
||||
Title: raw.Title,
|
||||
Body: raw.Body,
|
||||
State: mapStateFromGitHub(raw.State, raw.StateReason, labels),
|
||||
URL: raw.HTMLURL,
|
||||
Labels: labels,
|
||||
Assignees: assignees,
|
||||
}
|
||||
if len(out.Labels) == 0 {
|
||||
out.Labels = nil
|
||||
}
|
||||
if len(out.Assignees) == 0 {
|
||||
out.Assignees = nil
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// mapStateFromGitHub projects GitHub's open/closed + state_reason + labels
|
||||
// surface onto the normalized state. "in-review" wins over "in-progress"
|
||||
// when both labels are present (the workflow is progress -> review -> done).
|
||||
func mapStateFromGitHub(state, reason string, labels []string) domain.NormalizedIssueState {
|
||||
switch strings.ToLower(state) {
|
||||
case stateClosedGH:
|
||||
if strings.EqualFold(reason, reasonNotPlan) {
|
||||
return domain.IssueCancelled
|
||||
}
|
||||
return domain.IssueDone
|
||||
}
|
||||
var hasProgress, hasReview bool
|
||||
for _, l := range labels {
|
||||
switch 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 repo, filtered by state/labels/assignee. PRs
|
||||
// that GitHub's /issues endpoint conflates into the response are filtered
|
||||
// out client-side. 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.TrackerProviderGitHub {
|
||||
return nil, fmt.Errorf("%w: provider=%q", ErrWrongProvider, repo.Provider)
|
||||
}
|
||||
owner, repoName, err := parseGitHubRepo(repo.Native)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
q := url.Values{}
|
||||
switch filter.State {
|
||||
case domain.ListOpen:
|
||||
q.Set("state", "open")
|
||||
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 != "" {
|
||||
q.Set("assignee", 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("/repos/%s/%s/issues?%s", owner, repoName, q.Encode())
|
||||
resp, err := t.do(ctx, http.MethodGet, path, nil)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
var raw []ghIssue
|
||||
if err := json.Unmarshal(resp, &raw); err != nil {
|
||||
return nil, fmt.Errorf("github tracker: decode list: %w", err)
|
||||
}
|
||||
out := make([]domain.Issue, 0, len(raw))
|
||||
for _, r := range raw {
|
||||
if r.PullRequest != nil {
|
||||
continue
|
||||
}
|
||||
out = append(out, issueFromGH(owner, repoName, r))
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Preflight
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
// Preflight verifies the configured token is currently accepted by GitHub
|
||||
// (one GET /user). It does NOT prove the token has the repo 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 GitHub's identity endpoint", not
|
||||
// "token can do everything the SM will ask of it." Per-repo authorization
|
||||
// is detected lazily at the first Get/List against that repo.
|
||||
//
|
||||
// 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("github 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("github tracker: build request: %w", err)
|
||||
}
|
||||
if body != nil {
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
}
|
||||
req.Header.Set("Accept", "application/vnd.github+json")
|
||||
req.Header.Set("X-GitHub-Api-Version", "2022-11-28")
|
||||
req.Header.Set("User-Agent", t.userAgent)
|
||||
tok, err := t.tokens.Token(ctx)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
req.Header.Set("Authorization", "Bearer "+tok)
|
||||
|
||||
resp, err := t.http.Do(req)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("github 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 := githubMessage(body)
|
||||
switch resp.StatusCode {
|
||||
case http.StatusNotFound:
|
||||
return fmt.Errorf("%w: %s", ErrNotFound, msg)
|
||||
case http.StatusTooManyRequests:
|
||||
return rateLimited(resp, msg)
|
||||
case http.StatusUnauthorized:
|
||||
// 401 is unambiguously an auth failure. GitHub never uses 401 for
|
||||
// rate limiting; that's always 403 or 429.
|
||||
return fmt.Errorf("%w: %s", ErrAuthFailed, msg)
|
||||
case http.StatusForbidden:
|
||||
// GitHub returns 403 for primary rate-limit exhaustion, for
|
||||
// secondary/abuse limits, and for genuine auth/permission failures.
|
||||
// Disambiguate by signal: primary limit sets X-RateLimit-Remaining=0;
|
||||
// secondary/abuse sets Retry-After (often without the Remaining
|
||||
// header); either case mentions "rate limit" / "abuse" in the body.
|
||||
// Everything else is an auth/permission failure (token missing the
|
||||
// right scope, repo not visible to this token, etc).
|
||||
if isRateLimited(resp, msg) {
|
||||
return rateLimited(resp, msg)
|
||||
}
|
||||
return fmt.Errorf("%w: %s", ErrAuthFailed, msg)
|
||||
}
|
||||
return fmt.Errorf("github tracker: %d %s", resp.StatusCode, msg)
|
||||
}
|
||||
|
||||
func isRateLimited(resp *http.Response, msg string) bool {
|
||||
if rem := resp.Header.Get("X-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, "abuse detection")
|
||||
}
|
||||
|
||||
func rateLimited(resp *http.Response, msg string) error {
|
||||
e := &RateLimitError{Message: msg}
|
||||
if reset := resp.Header.Get("X-RateLimit-Reset"); reset != "" {
|
||||
if sec, err := strconv.ParseInt(reset, 10, 64); err == nil && sec > 0 {
|
||||
e.ResetAt = time.Unix(sec, 0)
|
||||
}
|
||||
}
|
||||
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 githubMessage(body []byte) string {
|
||||
var p struct {
|
||||
Message string `json:"message"`
|
||||
}
|
||||
if json.Unmarshal(body, &p) == nil && p.Message != "" {
|
||||
return p.Message
|
||||
}
|
||||
return strings.TrimSpace(string(body))
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// ID parsing
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
func (t *Tracker) parseID(id domain.TrackerID) (owner, repo string, number int, err error) {
|
||||
// Strict: the Session Manager picks an adapter by Provider, so reaching
|
||||
// this adapter with a non-github Provider is a routing bug, not user
|
||||
// input. Empty Provider is treated the same way — it would round-trip
|
||||
// to an Issue whose ID can't be re-routed.
|
||||
if id.Provider != domain.TrackerProviderGitHub {
|
||||
return "", "", 0, fmt.Errorf("%w: provider=%q", ErrWrongProvider, id.Provider)
|
||||
}
|
||||
return parseGitHubID(id.Native)
|
||||
}
|
||||
|
||||
// parseGitHubID accepts "owner/repo#NUM" and returns the three components.
|
||||
// Forms like "owner/repo/issues/NUM" or bare numbers are intentionally
|
||||
// rejected so the rest of the system has one canonical id shape.
|
||||
func parseGitHubID(native string) (owner, repo string, number int, err error) {
|
||||
hash := strings.IndexByte(native, '#')
|
||||
if hash < 0 {
|
||||
return "", "", 0, fmt.Errorf("%w: missing #issue", ErrBadID)
|
||||
}
|
||||
repoPart := native[:hash]
|
||||
numPart := native[hash+1:]
|
||||
slash := strings.IndexByte(repoPart, '/')
|
||||
if slash < 0 {
|
||||
return "", "", 0, fmt.Errorf("%w: missing owner/repo separator", ErrBadID)
|
||||
}
|
||||
owner = repoPart[:slash]
|
||||
repo = repoPart[slash+1:]
|
||||
if owner == "" || repo == "" {
|
||||
return "", "", 0, fmt.Errorf("%w: empty owner or repo", ErrBadID)
|
||||
}
|
||||
n, parseErr := strconv.Atoi(numPart)
|
||||
if parseErr != nil || n <= 0 {
|
||||
return "", "", 0, fmt.Errorf("%w: bad issue number %q", ErrBadID, numPart)
|
||||
}
|
||||
return owner, repo, n, nil
|
||||
}
|
||||
|
||||
// parseGitHubRepo accepts "owner/repo" and rejects empty segments,
|
||||
// embedded slashes, "#", and whitespace. Leading dots are kept legal —
|
||||
// "owner/.github" is a real GitHub convention for repo-level config repos.
|
||||
func parseGitHubRepo(native string) (owner, repo string, err error) {
|
||||
if native == "" {
|
||||
return "", "", fmt.Errorf("%w: empty repo", ErrBadID)
|
||||
}
|
||||
slash := strings.IndexByte(native, '/')
|
||||
if slash < 0 {
|
||||
return "", "", fmt.Errorf("%w: missing owner/repo separator", ErrBadID)
|
||||
}
|
||||
owner = native[:slash]
|
||||
repo = native[slash+1:]
|
||||
if owner == "" || repo == "" {
|
||||
return "", "", fmt.Errorf("%w: empty owner or repo segment", ErrBadID)
|
||||
}
|
||||
if strings.ContainsAny(owner, "/# \t\n\r") {
|
||||
return "", "", fmt.Errorf("%w: invalid owner segment %q", ErrBadID, owner)
|
||||
}
|
||||
if strings.ContainsAny(repo, "/# \t\n\r") {
|
||||
return "", "", fmt.Errorf("%w: invalid repo segment %q", ErrBadID, repo)
|
||||
}
|
||||
return owner, repo, nil
|
||||
}
|
||||
|
|
@ -0,0 +1,558 @@
|
|||
package github
|
||||
|
||||
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 GitHub API surface the adapter touched.
|
||||
type recordedReq struct {
|
||||
Method string
|
||||
Path string
|
||||
Body string
|
||||
}
|
||||
|
||||
// fakeGH is a programmable httptest.Server that matches requests by
|
||||
// "METHOD path" and records every call. Unmatched requests fail the test —
|
||||
// that is the point of TDD here, so an accidental extra call is loud.
|
||||
type fakeGH struct {
|
||||
t *testing.T
|
||||
server *httptest.Server
|
||||
mu sync.Mutex
|
||||
requests []recordedReq
|
||||
handlers map[string]http.HandlerFunc
|
||||
}
|
||||
|
||||
func newFakeGH(t *testing.T) *fakeGH {
|
||||
t.Helper()
|
||||
f := &fakeGH{t: t, handlers: map[string]http.HandlerFunc{}}
|
||||
f.server = httptest.NewServer(http.HandlerFunc(f.serve))
|
||||
t.Cleanup(f.server.Close)
|
||||
return f
|
||||
}
|
||||
|
||||
func (f *fakeGH) on(method, path string, h http.HandlerFunc) {
|
||||
f.mu.Lock()
|
||||
defer f.mu.Unlock()
|
||||
f.handlers[method+" "+path] = h
|
||||
}
|
||||
|
||||
func (f *fakeGH) 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, 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 *fakeGH) calls() []recordedReq {
|
||||
f.mu.Lock()
|
||||
defer f.mu.Unlock()
|
||||
out := make([]recordedReq, len(f.requests))
|
||||
copy(out, f.requests)
|
||||
return out
|
||||
}
|
||||
|
||||
// newTrackerForTest constructs an adapter pointed at the fake server with a
|
||||
// static dev token. Production code uses EnvTokenSource; tests skip that to
|
||||
// keep the surface tiny.
|
||||
func newTrackerForTest(t *testing.T, f *fakeGH) *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
|
||||
wantOwner string
|
||||
wantRepo string
|
||||
wantNum int
|
||||
wantErr bool
|
||||
}{
|
||||
{"happy", "octocat/hello-world#42", "octocat", "hello-world", 42, false},
|
||||
{"missing hash", "octocat/hello-world", "", "", 0, true},
|
||||
{"missing slash", "octocat#42", "", "", 0, true},
|
||||
{"empty owner", "/repo#1", "", "", 0, true},
|
||||
{"empty repo", "owner/#1", "", "", 0, true},
|
||||
{"non-numeric", "o/r#abc", "", "", 0, true},
|
||||
{"zero", "o/r#0", "", "", 0, true},
|
||||
{"negative", "o/r#-1", "", "", 0, true},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
owner, repo, num, err := parseGitHubID(tc.native)
|
||||
if tc.wantErr {
|
||||
if err == nil {
|
||||
t.Fatalf("expected error, got %s/%s#%d", owner, repo, num)
|
||||
}
|
||||
return
|
||||
}
|
||||
if err != nil {
|
||||
t.Fatalf("parse: %v", err)
|
||||
}
|
||||
if owner != tc.wantOwner || repo != tc.wantRepo || num != tc.wantNum {
|
||||
t.Fatalf("got %s/%s#%d, want %s/%s#%d", owner, repo, num, tc.wantOwner, tc.wantRepo, tc.wantNum)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestGet_HappyPath(t *testing.T) {
|
||||
f := newFakeGH(t)
|
||||
f.on("GET", "/repos/octocat/hello-world/issues/42", func(w http.ResponseWriter, r *http.Request) {
|
||||
if got := r.Header.Get("Authorization"); got != "Bearer tkn-test" {
|
||||
t.Errorf("Authorization = %q, want Bearer tkn-test", got)
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_, _ = w.Write([]byte(`{
|
||||
"number": 42,
|
||||
"title": "Found a bug",
|
||||
"body": "It does not work",
|
||||
"state": "open",
|
||||
"html_url": "https://github.com/octocat/hello-world/issues/42",
|
||||
"labels": [{"name":"bug"},{"name":"in-progress"}],
|
||||
"assignees": [{"login":"alice"},{"login":"bob"}]
|
||||
}`))
|
||||
})
|
||||
tr := newTrackerForTest(t, f)
|
||||
|
||||
issue, err := tr.Get(ctx(), domain.TrackerID{Provider: domain.TrackerProviderGitHub, Native: "octocat/hello-world#42"})
|
||||
if err != nil {
|
||||
t.Fatalf("Get: %v", err)
|
||||
}
|
||||
want := domain.Issue{
|
||||
ID: domain.TrackerID{Provider: domain.TrackerProviderGitHub, Native: "octocat/hello-world#42"},
|
||||
Title: "Found a bug",
|
||||
Body: "It does not work",
|
||||
State: domain.IssueInProgress, // the "in-progress" label wins over plain "open"
|
||||
URL: "https://github.com/octocat/hello-world/issues/42",
|
||||
Labels: []string{"bug", "in-progress"},
|
||||
Assignees: []string{"alice", "bob"},
|
||||
}
|
||||
if !reflect.DeepEqual(issue, want) {
|
||||
t.Fatalf("issue = %#v\nwant %#v", issue, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGet_StateMappingFromGitHubFields(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
ghState string
|
||||
ghReason string
|
||||
labels []string
|
||||
wantState domain.NormalizedIssueState
|
||||
}{
|
||||
{"plain open", "open", "", nil, domain.IssueOpen},
|
||||
{"open with in-progress label", "open", "", []string{"in-progress"}, domain.IssueInProgress},
|
||||
{"open with in-review label", "open", "", []string{"in-review"}, domain.IssueInReview},
|
||||
{"review wins over progress when both present", "open", "", []string{"in-progress", "in-review"}, domain.IssueInReview},
|
||||
{"closed completed", "closed", "completed", nil, domain.IssueDone},
|
||||
{"closed not_planned", "closed", "not_planned", nil, domain.IssueCancelled},
|
||||
{"closed unknown reason maps to done", "closed", "", nil, domain.IssueDone},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
f := newFakeGH(t)
|
||||
payload := map[string]any{
|
||||
"number": 1,
|
||||
"title": "t",
|
||||
"body": "",
|
||||
"state": tc.ghState,
|
||||
"html_url": "https://github.com/o/r/issues/1",
|
||||
}
|
||||
if tc.ghReason != "" {
|
||||
payload["state_reason"] = tc.ghReason
|
||||
}
|
||||
if tc.labels != nil {
|
||||
ls := make([]map[string]string, len(tc.labels))
|
||||
for i, n := range tc.labels {
|
||||
ls[i] = map[string]string{"name": n}
|
||||
}
|
||||
payload["labels"] = ls
|
||||
}
|
||||
b, _ := json.Marshal(payload)
|
||||
f.on("GET", "/repos/o/r/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.TrackerProviderGitHub, Native: "o/r#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 := newFakeGH(t)
|
||||
f.on("GET", "/repos/o/r/issues/1", func(w http.ResponseWriter, r *http.Request) {
|
||||
http.Error(w, `{"message":"Not Found"}`, http.StatusNotFound)
|
||||
})
|
||||
tr := newTrackerForTest(t, f)
|
||||
_, err := tr.Get(ctx(), domain.TrackerID{Provider: domain.TrackerProviderGitHub, Native: "o/r#1"})
|
||||
if !errors.Is(err, ErrNotFound) {
|
||||
t.Fatalf("err = %v, want ErrNotFound", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGet_RateLimited(t *testing.T) {
|
||||
f := newFakeGH(t)
|
||||
reset := time.Now().Add(2 * time.Minute).Unix()
|
||||
f.on("GET", "/repos/o/r/issues/1", func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("X-RateLimit-Remaining", "0")
|
||||
w.Header().Set("X-RateLimit-Reset", strconv.FormatInt(reset, 10))
|
||||
http.Error(w, `{"message":"API rate limit exceeded"}`, http.StatusForbidden)
|
||||
})
|
||||
tr := newTrackerForTest(t, f)
|
||||
_, err := tr.Get(ctx(), domain.TrackerID{Provider: domain.TrackerProviderGitHub, Native: "o/r#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_SecondaryRateLimit covers the GitHub "abuse detection"
|
||||
// response — it lacks X-RateLimit-Remaining but sets Retry-After, and the
|
||||
// body mentions the limit. The classifier must still surface this as
|
||||
// ErrRateLimited rather than mis-categorizing it as auth failure.
|
||||
func TestGet_SecondaryRateLimit(t *testing.T) {
|
||||
f := newFakeGH(t)
|
||||
f.on("GET", "/repos/o/r/issues/1", func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Retry-After", "60")
|
||||
http.Error(w, `{"message":"You have exceeded a secondary rate limit"}`, http.StatusForbidden)
|
||||
})
|
||||
tr := newTrackerForTest(t, f)
|
||||
_, err := tr.Get(ctx(), domain.TrackerID{Provider: domain.TrackerProviderGitHub, Native: "o/r#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 := newFakeGH(t)
|
||||
tr := newTrackerForTest(t, f)
|
||||
_, err := tr.Get(ctx(), domain.TrackerID{Provider: domain.TrackerProviderGitLab, Native: "g/p#1"})
|
||||
if !errors.Is(err, ErrWrongProvider) {
|
||||
t.Fatalf("err = %v, want ErrWrongProvider", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGet_RejectsEmptyProvider(t *testing.T) {
|
||||
f := newFakeGH(t)
|
||||
tr := newTrackerForTest(t, f)
|
||||
_, err := tr.Get(ctx(), domain.TrackerID{Native: "o/r#1"})
|
||||
if !errors.Is(err, ErrWrongProvider) {
|
||||
t.Fatalf("err = %v, want ErrWrongProvider", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestGet_CanonicalizesProviderOnOutput pins the contract that returned
|
||||
// Issues always carry domain.TrackerProviderGitHub, so callers can re-route
|
||||
// without inspecting which adapter they originally talked to.
|
||||
func TestGet_CanonicalizesProviderOnOutput(t *testing.T) {
|
||||
f := newFakeGH(t)
|
||||
f.on("GET", "/repos/o/r/issues/1", func(w http.ResponseWriter, r *http.Request) {
|
||||
_, _ = w.Write([]byte(`{"number":1,"title":"t","body":"","state":"open","html_url":"https://github.com/o/r/issues/1"}`))
|
||||
})
|
||||
tr := newTrackerForTest(t, f)
|
||||
issue, err := tr.Get(ctx(), domain.TrackerID{Provider: domain.TrackerProviderGitHub, Native: "o/r#1"})
|
||||
if err != nil {
|
||||
t.Fatalf("Get: %v", err)
|
||||
}
|
||||
if issue.ID.Provider != domain.TrackerProviderGitHub {
|
||||
t.Fatalf("issue.ID.Provider = %q, want %q", issue.ID.Provider, domain.TrackerProviderGitHub)
|
||||
}
|
||||
if issue.ID.Native != "o/r#1" {
|
||||
t.Fatalf("issue.ID.Native = %q, want o/r#1", issue.ID.Native)
|
||||
}
|
||||
}
|
||||
|
||||
// TestGet_AuthFailed locks in that a 401 (and 403 without rate-limit
|
||||
// signals) maps to the typed ErrAuthFailed, so callers — especially
|
||||
// Preflight — can distinguish bad-token from other failures.
|
||||
func TestGet_AuthFailed(t *testing.T) {
|
||||
f := newFakeGH(t)
|
||||
f.on("GET", "/repos/o/r/issues/1", func(w http.ResponseWriter, r *http.Request) {
|
||||
http.Error(w, `{"message":"Bad credentials"}`, http.StatusUnauthorized)
|
||||
})
|
||||
tr := newTrackerForTest(t, f)
|
||||
_, err := tr.Get(ctx(), domain.TrackerID{Provider: domain.TrackerProviderGitHub, Native: "o/r#1"})
|
||||
if !errors.Is(err, ErrAuthFailed) {
|
||||
t.Fatalf("err = %v, want ErrAuthFailed", err)
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Preflight
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
func TestPreflight_HappyPath(t *testing.T) {
|
||||
f := newFakeGH(t)
|
||||
f.on("GET", "/user", func(w http.ResponseWriter, r *http.Request) {
|
||||
if got := r.Header.Get("Authorization"); got != "Bearer tkn-test" {
|
||||
t.Errorf("Authorization = %q", got)
|
||||
}
|
||||
_, _ = w.Write([]byte(`{"login":"octocat","id":1}`))
|
||||
})
|
||||
tr := newTrackerForTest(t, f)
|
||||
if err := tr.Preflight(ctx()); err != nil {
|
||||
t.Fatalf("Preflight: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPreflight_InvalidToken(t *testing.T) {
|
||||
f := newFakeGH(t)
|
||||
f.on("GET", "/user", func(w http.ResponseWriter, r *http.Request) {
|
||||
http.Error(w, `{"message":"Bad credentials"}`, 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 a successful check is cached so the
|
||||
// daemon doesn't burn a GET /user on every component start that wants to
|
||||
// confirm tracker auth.
|
||||
func TestPreflight_CachesSuccess(t *testing.T) {
|
||||
f := newFakeGH(t)
|
||||
f.on("GET", "/user", func(w http.ResponseWriter, r *http.Request) {
|
||||
_, _ = w.Write([]byte(`{"login":"octocat","id":1}`))
|
||||
})
|
||||
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 the recovery property: failures
|
||||
// must NOT be cached, otherwise a transient network glitch at startup would
|
||||
// permanently brick the tracker for the lifetime of the daemon.
|
||||
func TestPreflight_RetriesAfterFailure(t *testing.T) {
|
||||
f := newFakeGH(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(`{"login":"octocat","id":1}`))
|
||||
})
|
||||
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 := newFakeGH(t)
|
||||
f.on("GET", "/repos/o/r/issues", func(w http.ResponseWriter, r *http.Request) {
|
||||
q := r.URL.Query()
|
||||
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(`[
|
||||
{"number":1,"title":"first","body":"b1","state":"open","html_url":"https://github.com/o/r/issues/1","labels":[{"name":"bug"}],"assignees":[]},
|
||||
{"number":2,"title":"second","body":"b2","state":"closed","state_reason":"completed","html_url":"https://github.com/o/r/issues/2","labels":[],"assignees":[{"login":"alice"}]}
|
||||
]`))
|
||||
})
|
||||
tr := newTrackerForTest(t, f)
|
||||
issues, err := tr.List(ctx(), domain.TrackerRepo{Provider: domain.TrackerProviderGitHub, Native: "o/r"}, 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 != "o/r#1" || issues[0].State != domain.IssueOpen || issues[0].Title != "first" {
|
||||
t.Fatalf("issues[0] = %#v", issues[0])
|
||||
}
|
||||
if issues[1].ID.Native != "o/r#2" || issues[1].State != domain.IssueDone || len(issues[1].Assignees) != 1 || issues[1].Assignees[0] != "alice" {
|
||||
t.Fatalf("issues[1] = %#v", issues[1])
|
||||
}
|
||||
}
|
||||
|
||||
func TestList_FiltersOutPullRequests(t *testing.T) {
|
||||
f := newFakeGH(t)
|
||||
f.on("GET", "/repos/o/r/issues", func(w http.ResponseWriter, r *http.Request) {
|
||||
// GitHub's issues endpoint returns PRs too. We must filter them out
|
||||
// so the LCM never tries to spawn an agent against a PR number.
|
||||
_, _ = w.Write([]byte(`[
|
||||
{"number":10,"title":"real issue","state":"open","html_url":"https://github.com/o/r/issues/10"},
|
||||
{"number":11,"title":"a PR","state":"open","html_url":"https://github.com/o/r/pull/11","pull_request":{"url":"https://api.github.com/repos/o/r/pulls/11"}},
|
||||
{"number":12,"title":"another issue","state":"open","html_url":"https://github.com/o/r/issues/12"}
|
||||
]`))
|
||||
})
|
||||
tr := newTrackerForTest(t, f)
|
||||
issues, err := tr.List(ctx(), domain.TrackerRepo{Provider: domain.TrackerProviderGitHub, Native: "o/r"}, domain.ListFilter{})
|
||||
if err != nil {
|
||||
t.Fatalf("List: %v", err)
|
||||
}
|
||||
if len(issues) != 2 {
|
||||
t.Fatalf("len = %d, want 2 (PR must be filtered out)", len(issues))
|
||||
}
|
||||
if issues[0].ID.Native != "o/r#10" || issues[1].ID.Native != "o/r#12" {
|
||||
t.Fatalf("kept wrong items: %#v", issues)
|
||||
}
|
||||
}
|
||||
|
||||
func TestList_QueryEncoding(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
filter domain.ListFilter
|
||||
wantQ map[string]string
|
||||
}{
|
||||
{
|
||||
name: "open + labels + assignee + limit",
|
||||
filter: domain.ListFilter{State: domain.ListOpen, Labels: []string{"bug", "help wanted"}, Assignee: "alice", Limit: 50},
|
||||
wantQ: map[string]string{"state": "open", "labels": "bug,help wanted", "assignee": "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 := newFakeGH(t)
|
||||
f.on("GET", "/repos/o/r/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.TrackerProviderGitHub, Native: "o/r"}, tc.filter); err != nil {
|
||||
t.Fatalf("List: %v", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestList_RejectsWrongProvider(t *testing.T) {
|
||||
f := newFakeGH(t)
|
||||
tr := newTrackerForTest(t, f)
|
||||
_, err := tr.List(ctx(), domain.TrackerRepo{Provider: domain.TrackerProviderGitLab, Native: "g/p"}, 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 /
|
||||
"/repo", // empty owner
|
||||
"owner/", // empty repo
|
||||
"a/b/c", // extra slash
|
||||
" owner/repo", // leading whitespace in owner
|
||||
"owner/repo ", // trailing whitespace in repo
|
||||
"own er/repo", // embedded space in owner
|
||||
"owner/re#po", // embedded # in repo
|
||||
"\towner/repo", // tab in owner
|
||||
"owner/repo\n", // newline in repo
|
||||
}
|
||||
// Sanity: a benign leading-dot repo (".github" convention) must pass.
|
||||
if _, _, err := parseGitHubRepo("octocat/.github"); err != nil {
|
||||
t.Fatalf("leading-dot repo rejected unexpectedly: %v", err)
|
||||
}
|
||||
for _, native := range cases {
|
||||
t.Run(native, func(t *testing.T) {
|
||||
f := newFakeGH(t)
|
||||
tr := newTrackerForTest(t, f)
|
||||
_, err := tr.List(ctx(), domain.TrackerRepo{Provider: domain.TrackerProviderGitHub, Native: native}, domain.ListFilter{})
|
||||
if !errors.Is(err, ErrBadID) {
|
||||
t.Fatalf("native=%q: err = %v, want ErrBadID", native, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,88 @@
|
|||
package domain
|
||||
|
||||
// TrackerProvider identifies an issue-tracker provider implementation.
|
||||
// Provider differences (label-driven vs state-machine vs close-reason) are
|
||||
// absorbed inside each adapter; the rest of the system only sees
|
||||
// NormalizedIssueState.
|
||||
type TrackerProvider string
|
||||
|
||||
const (
|
||||
TrackerProviderGitHub TrackerProvider = "github"
|
||||
TrackerProviderGitLab TrackerProvider = "gitlab"
|
||||
TrackerProviderLinear TrackerProvider = "linear"
|
||||
)
|
||||
|
||||
// TrackerID identifies a single issue across providers. Native is the
|
||||
// provider's own canonical form ("owner/repo#123" for GitHub,
|
||||
// "group/project#456" for GitLab, "ABC-789" for Linear) and is parsed by the
|
||||
// adapter. Provider is the discriminator the Session Manager uses to pick an
|
||||
// adapter.
|
||||
type TrackerID struct {
|
||||
Provider TrackerProvider `json:"provider"`
|
||||
Native string `json:"native"`
|
||||
}
|
||||
|
||||
// NormalizedIssueState is the cross-provider issue-state vocabulary every
|
||||
// adapter must implement. The closed list is intentional — adding a value
|
||||
// here is a port-level decision because every adapter must map it.
|
||||
type NormalizedIssueState string
|
||||
|
||||
const (
|
||||
IssueOpen NormalizedIssueState = "open"
|
||||
IssueInProgress NormalizedIssueState = "in_progress"
|
||||
IssueInReview NormalizedIssueState = "review"
|
||||
IssueDone NormalizedIssueState = "done"
|
||||
IssueCancelled NormalizedIssueState = "cancelled"
|
||||
)
|
||||
|
||||
// Issue is the minimum projection every tracker can produce. Fields are
|
||||
// added only when all v1 providers (GitHub, GitLab, Linear) can populate
|
||||
// them faithfully; richer metadata stays inside provider-specific code paths.
|
||||
type Issue struct {
|
||||
ID TrackerID `json:"id"`
|
||||
Title string `json:"title"`
|
||||
Body string `json:"body"`
|
||||
State NormalizedIssueState `json:"state"`
|
||||
URL string `json:"url"`
|
||||
Labels []string `json:"labels,omitempty"`
|
||||
Assignees []string `json:"assignees,omitempty"`
|
||||
}
|
||||
|
||||
// TrackerRepo identifies a repository (or its provider-equivalent) for
|
||||
// cross-issue queries like Tracker.List. Native is the provider's canonical
|
||||
// owner/project form: "owner/repo" for GitHub, "group/project" for GitLab.
|
||||
// Linear has no native repo concept; adapters may use a team or workspace
|
||||
// identifier in Native when this port reaches Linear.
|
||||
type TrackerRepo struct {
|
||||
Provider TrackerProvider `json:"provider"`
|
||||
Native string `json:"native"`
|
||||
}
|
||||
|
||||
// ListStateFilter narrows Tracker.List results by the provider's coarse
|
||||
// state (open vs closed). It is intentionally NOT the 5-value normalized
|
||||
// enum — finer filtering (e.g. "only in-review issues") goes through the
|
||||
// Labels field of ListFilter.
|
||||
type ListStateFilter string
|
||||
|
||||
const (
|
||||
// ListAll is the zero value and returns issues in any state.
|
||||
ListAll ListStateFilter = ""
|
||||
ListOpen ListStateFilter = "open"
|
||||
ListClosed ListStateFilter = "closed"
|
||||
)
|
||||
|
||||
// ListFilter is the query the Session Manager passes to Tracker.List.
|
||||
// Empty / zero values mean "no filter on this dimension".
|
||||
//
|
||||
// Limit is the requested page size. The adapter applies its own default
|
||||
// when zero and SILENTLY CAPS at the provider's per-page maximum — a
|
||||
// caller asking for more than the cap gets exactly cap items back with no
|
||||
// error and no indication of truncation. v1 has no auto-pagination;
|
||||
// callers needing more results need to wait for the observer/polling work
|
||||
// in issue #35.
|
||||
type ListFilter struct {
|
||||
State ListStateFilter `json:"state,omitempty"`
|
||||
Labels []string `json:"labels,omitempty"`
|
||||
Assignee string `json:"assignee,omitempty"`
|
||||
Limit int `json:"limit,omitempty"`
|
||||
}
|
||||
|
|
@ -0,0 +1,31 @@
|
|||
package ports
|
||||
|
||||
import (
|
||||
"context"
|
||||
|
||||
"github.com/aoagents/agent-orchestrator/backend/internal/domain"
|
||||
)
|
||||
|
||||
// Tracker is the outbound port for issue trackers (GitHub Issues, GitLab
|
||||
// Issues, Linear). v1 is read-only:
|
||||
//
|
||||
// - Get returns a normalized snapshot of one issue, used by spawn-bootstrap
|
||||
// to hydrate the agent prompt.
|
||||
// - List returns a filtered slice of issues in a repo, used when the SM
|
||||
// needs to enumerate work (e.g. backlog view, status sweeps).
|
||||
// - Preflight verifies the configured credential is actually valid against
|
||||
// the provider so daemons fail fast at startup, not at first request.
|
||||
//
|
||||
// Mirroring agent lifecycle back onto the tracker (Comment, Transition) is
|
||||
// deferred to issue #40. The observer / polling loop is deferred to #35.
|
||||
//
|
||||
// All v1 providers share this interface. Provider differences (label vs
|
||||
// state machine vs close reason) are absorbed inside each adapter via
|
||||
// domain.NormalizedIssueState. Fields on domain.Issue exist only when every
|
||||
// provider can populate them; richer per-provider metadata belongs behind a
|
||||
// separate port.
|
||||
type Tracker interface {
|
||||
Get(ctx context.Context, id domain.TrackerID) (domain.Issue, error)
|
||||
List(ctx context.Context, repo domain.TrackerRepo, filter domain.ListFilter) ([]domain.Issue, error)
|
||||
Preflight(ctx context.Context) error
|
||||
}
|
||||
Loading…
Reference in New Issue