Skip to content

Commit f9b08aa

Browse files
feat(scm): GitHub provider adapter — Observe(prURL) → PRObservation (#69)
* feat(scm): GitHub provider adapter — Observe(prURL) → PRObservation A fresh GitHub SCM provider adapter under backend/internal/adapters/scm/github/ exposing one method: (*Provider).Observe(ctx, prURL) (ports.PRObservation, error) It performs a REST GET on /repos/{o}/{r}/pulls/{n} for the authoritative draft/merged/closed/head-SHA, one GraphQL query for the reviewDecision + mergeStateStatus + statusCheckRollup + unresolved review threads, and (only for failure-class CheckRuns) a REST GET on /actions/jobs/{job_id}/logs to splice the last 20 lines of the failed job into the observation. The package is the observation primitive; the polling loop, cadence selection, daemon wiring, persistence and webhook receiver are all intentionally out of scope (separate PRs / lanes). Closes #27 — this supersedes PR #28's attempt, which targeted types (domain.SCMProvider / SCMSnapshot / ports.SCMObserveRequest) that the PR #62 simplification refactor has since removed. The GraphQL queries and mergeability composition logic are credited to @whoisasx from PR #28's provider.go; the package was re-implemented against the current ports.PRObservation seam (post-#62) rather than rebased. Bot-author detection uses ONLY GitHub's typed signal (__typename "Bot" / User.Type "Bot"). The strings.Contains(login, "bot") fallback from PR #28 was intentionally dropped — aa-18's review flagged it as a false-positive magnet for logins like "robothon" / "lambot123". 46 table-driven tests against httptest.NewServer cover happy path, draft, merged, closed (not merged), CI passing/failing/pending, StatusContext legacy, log-tail extraction (and the best-effort log-fetch failure case), mergeability mergeable/conflicting/blocked (including ci-failing → blocked even when GitHub still says CLEAN — the load-bearing aa-18 contract)/unstable/unknown, review approved/changes-requested/required/none, bot-author filtering (including the robothon false-positive guard), unresolved-only threads, all-bots → empty Comments, ETag-304 cache hit, primary + secondary rate-limit (with errors.As → *RateLimitError), 401 → ErrAuthFailed, malformed JSON → Fetched:false, network error → Fetched:false, Authorization Bearer header injection, StaticTokenSource blank/whitespace rejection, GHTokenSource memoize + invalidate. Verification: - go build ./... clean - go vet ./... clean - gofmt -l backend/internal/adapters/scm/ clean - golangci-lint run ./... (v2.12, repo .golangci.yml) 0 issues - go test -race ./internal/adapters/scm/github/... 46/46 PASS References: - aa-18 review of PR #28: ~/.ao/agent-reports/aa-18.md - aa-26 tracker adapter (sibling Go-adapter pattern): #36 / agent-reports/aa-26.md Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(scm): address greptile review on #69 Four fixes from the greptile review of PR #69: 1. CI rollup pagination (P1) — when GraphQL reports pageInfo.hasNextPage=true for the statusCheckRollup contexts, a visible "all passing" set could be hiding a failing context on the next page. ciSummaryFromGraphQL now degrades Passing / Pending / Unknown to CIUnknown in that case; a known CIFailing on the visible page is still safe and is NOT degraded. Also bumped the per-page limit from 50 to 100 (GraphQL's documented max for the contexts connection). Two new tests pin both branches. 2. Empty GraphQL inline fragment (P2) — dropped `... on User { }` from the reviewThreads author selection. The empty selection set was technically invalid GraphQL and a future API tightening could reject the query. __typename already tells us whether the actor is a Bot, so the fragment carried no information. 3. rest.MergeStateStatus dead-code (P2) — the field decoded from the non-existent REST `merge_state_status` was always empty, making the firstNonEmpty fallback dead code. Removed the field and switched the tiebreaker to rest.MergeableState (the actual REST field, upper- cased so the same switch covers both GraphQL and REST shapes). 4. Wrong Accept header on /actions/jobs/{id}/logs (P2) — GitHub's REST API validates the Accept header before issuing the 302 to the log blob; sending text/plain risks a 406. Switched to the canonical application/vnd.github+json; the redirected blob serves text/plain regardless. Verification: - go build ./... clean - go vet ./... clean - golangci-lint run ./... 0 issues - go test -race ./internal/adapters/scm/github/... 48 / 48 PASS Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent c8f6050 commit f9b08aa

5 files changed

Lines changed: 2516 additions & 0 deletions

File tree

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
package github
2+
3+
import (
4+
"context"
5+
"errors"
6+
"os"
7+
"os/exec"
8+
"strings"
9+
"sync"
10+
"time"
11+
)
12+
13+
// TokenSource yields a GitHub bearer token on demand. Production wires this
14+
// to EnvTokenSource or GHTokenSource; tests inject StaticTokenSource.
15+
type TokenSource interface {
16+
Token(ctx context.Context) (string, error)
17+
}
18+
19+
// tokenInvalidator is the optional capability of dropping a cached token so
20+
// the next call re-fetches it. The Client invokes this whenever GitHub
21+
// responds with an auth-class failure: the next request will pick up a
22+
// rotated token without restarting the daemon.
23+
type tokenInvalidator interface {
24+
InvalidateToken()
25+
}
26+
27+
// ErrNoToken is returned when no token source could yield a non-empty token.
28+
var ErrNoToken = errors.New("github scm: no token configured")
29+
30+
// StaticTokenSource is a literal token, typically used in tests.
31+
type StaticTokenSource string
32+
33+
// Token returns the literal token, or ErrNoToken if it is blank.
34+
func (s StaticTokenSource) Token(context.Context) (string, error) {
35+
t := strings.TrimSpace(string(s))
36+
if t == "" {
37+
return "", ErrNoToken
38+
}
39+
return t, nil
40+
}
41+
42+
// EnvTokenSource reads the first non-empty value from the listed env vars,
43+
// falling back to GITHUB_TOKEN. Order matters: a project-scoped variable
44+
// (AO_GITHUB_TOKEN) should win over the global default.
45+
type EnvTokenSource struct {
46+
EnvVars []string
47+
}
48+
49+
// Token returns the first non-empty env-var value found, or ErrNoToken.
50+
func (s EnvTokenSource) Token(context.Context) (string, error) {
51+
for _, name := range s.EnvVars {
52+
if v := strings.TrimSpace(os.Getenv(name)); v != "" {
53+
return v, nil
54+
}
55+
}
56+
if v := strings.TrimSpace(os.Getenv("GITHUB_TOKEN")); v != "" {
57+
return v, nil
58+
}
59+
return "", ErrNoToken
60+
}
61+
62+
const defaultGHTokenCacheTTL = 5 * time.Minute
63+
64+
// GHTokenSource shells out to `gh auth token` when env vars are not
65+
// configured. It memoizes the result for TokenTTL so we don't fork-exec on
66+
// every request, but the Client invalidates the cache on auth failures so a
67+
// rotated token is picked up on the next call. Tests inject GH so the gh
68+
// binary is never required.
69+
type GHTokenSource struct {
70+
// GH is the shell-out hook. Production leaves this nil and falls back
71+
// to `exec.CommandContext("gh", "auth", "token")`; tests inject a
72+
// fake to avoid touching the real binary.
73+
GH func(ctx context.Context) (string, error)
74+
// TokenTTL is how long a successful read is memoized. Zero means use
75+
// defaultGHTokenCacheTTL.
76+
TokenTTL time.Duration
77+
// Clock allows tests to drive expiration. Zero means time.Now.
78+
Clock func() time.Time
79+
80+
mu sync.Mutex
81+
token string
82+
expiresAt time.Time
83+
}
84+
85+
// Token returns the cached token if still fresh, otherwise re-runs gh.
86+
func (s *GHTokenSource) Token(ctx context.Context) (string, error) {
87+
s.mu.Lock()
88+
defer s.mu.Unlock()
89+
now := s.now()
90+
if s.token != "" && now.Before(s.expiresAt) {
91+
return s.token, nil
92+
}
93+
run := s.GH
94+
if run == nil {
95+
run = ghAuthToken
96+
}
97+
out, err := run(ctx)
98+
if err != nil {
99+
return "", err
100+
}
101+
token := strings.TrimSpace(out)
102+
if token == "" {
103+
return "", ErrNoToken
104+
}
105+
s.token = token
106+
s.expiresAt = now.Add(s.ttl())
107+
return token, nil
108+
}
109+
110+
// InvalidateToken drops the memoized token so the next Token call shells
111+
// out again. The Client calls this on 401/403-auth responses.
112+
func (s *GHTokenSource) InvalidateToken() {
113+
s.mu.Lock()
114+
defer s.mu.Unlock()
115+
s.token = ""
116+
s.expiresAt = time.Time{}
117+
}
118+
119+
func (s *GHTokenSource) now() time.Time {
120+
if s.Clock != nil {
121+
return s.Clock()
122+
}
123+
return time.Now()
124+
}
125+
126+
func (s *GHTokenSource) ttl() time.Duration {
127+
if s.TokenTTL > 0 {
128+
return s.TokenTTL
129+
}
130+
return defaultGHTokenCacheTTL
131+
}
132+
133+
func ghAuthToken(ctx context.Context) (string, error) {
134+
out, err := exec.CommandContext(ctx, "gh", "auth", "token").Output()
135+
if err != nil {
136+
return "", err
137+
}
138+
return string(out), nil
139+
}

0 commit comments

Comments
 (0)