diff --git a/backend/internal/adapters/tracker/github/auth.go b/backend/internal/adapters/tracker/github/auth.go index 9aa810df..76ddd8fc 100644 --- a/backend/internal/adapters/tracker/github/auth.go +++ b/backend/internal/adapters/tracker/github/auth.go @@ -3,7 +3,9 @@ package github import ( "context" "errors" + "fmt" "os" + "os/exec" "strings" ) @@ -30,16 +32,27 @@ func (s StaticTokenSource) Token(context.Context) (string, error) { return t, nil } -// EnvTokenSource reads the first non-empty value from the listed env vars, -// falling back to 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. +// EnvTokenSource resolves a token from the user's environment with zero +// configuration on a stock developer machine. Lookup order: +// +// 1. Each name in EnvVars (project-configured first, e.g. AO_GITHUB_TOKEN). +// 2. The well-known GITHUB_TOKEN env var. +// 3. The `gh` CLI's auth state, via `gh auth token`. If the user has +// already run `gh auth login`, this just works — no env var required. +// +// If step 3 errors (gh not installed, not authenticated, exec failure) the +// error is swallowed and we fall through to ErrNoToken so the caller sees +// the same "configure a token" signal regardless of why no token was found. +// +// GH is the function invoked in step 3. Production code leaves it nil and +// the default `gh auth token` exec is used. Tests inject a fake to avoid +// shelling out to a real gh binary. type EnvTokenSource struct { EnvVars []string + GH func(ctx context.Context) (string, error) } -func (s EnvTokenSource) Token(context.Context) (string, error) { +func (s EnvTokenSource) Token(ctx context.Context) (string, error) { for _, name := range s.EnvVars { if v := strings.TrimSpace(os.Getenv(name)); v != "" { return v, nil @@ -48,5 +61,45 @@ func (s EnvTokenSource) Token(context.Context) (string, error) { if v := strings.TrimSpace(os.Getenv("GITHUB_TOKEN")); v != "" { return v, nil } - return "", ErrNoToken + gh := s.GH + if gh == nil { + gh = ghAuthToken + } + v, err := gh(ctx) + if err == nil { + if v = strings.TrimSpace(v); v != "" { + return v, nil + } + // gh succeeded with empty stdout — same outcome as no token, but + // no underlying error to carry forward. Fall through to plain ErrNoToken. + return "", ErrNoToken + } + // gh failed. "Binary missing" is the uninteresting case — the user + // simply doesn't have gh installed, and we'd be noise to surface it. + // Anything else (not logged in, network blip, hung subprocess) carries + // operator-useful explanation we should preserve so a daemon log shows + // WHY zero-config auth fell through, not just THAT it did. + if errors.Is(err, exec.ErrNotFound) { + return "", ErrNoToken + } + return "", fmt.Errorf("%w (gh fallback: %v)", ErrNoToken, err) +} + +// ghAuthToken shells out to the `gh` CLI and asks it for the user's +// currently logged-in token. On non-zero exit, gh writes its explanation +// ("not logged in to any GitHub hosts. Try `gh auth login`") to stderr; +// we capture it and fold it into the returned error so callers can surface +// the cause. +func ghAuthToken(ctx context.Context) (string, error) { + cmd := exec.CommandContext(ctx, "gh", "auth", "token") + var stderr strings.Builder + cmd.Stderr = &stderr + out, err := cmd.Output() + if err != nil { + if msg := strings.TrimSpace(stderr.String()); msg != "" { + return "", fmt.Errorf("%w: %s", err, msg) + } + return "", err + } + return strings.TrimSpace(string(out)), nil } diff --git a/backend/internal/adapters/tracker/github/doc.go b/backend/internal/adapters/tracker/github/doc.go index f37c4c90..1e060c53 100644 --- a/backend/internal/adapters/tracker/github/doc.go +++ b/backend/internal/adapters/tracker/github/doc.go @@ -13,6 +13,15 @@ // Writing back to the tracker (Comment, Transition) is deferred to issue // #40. The observer/polling loop is deferred to issue #35. // +// # Zero-config auth +// +// EnvTokenSource looks for a token in this order: configured env vars, +// GITHUB_TOKEN, then `gh auth token`. If the user has already run +// `gh auth login`, the adapter just works — no env var setup required. +// gh-CLI errors (binary missing, not authenticated) are intentionally +// swallowed and reported as ErrNoToken so the caller sees one uniform +// "configure a token" signal. +// // # Reverse state mapping // // GitHub Issues only have two native states (open, closed) plus a diff --git a/backend/internal/adapters/tracker/github/tracker.go b/backend/internal/adapters/tracker/github/tracker.go index bf6ffcbf..cebdaafe 100644 --- a/backend/internal/adapters/tracker/github/tracker.go +++ b/backend/internal/adapters/tracker/github/tracker.go @@ -37,6 +37,12 @@ const ( // (matching the legacy gh CLI default) when the caller passes 0. defaultListLimit = 30 maxListLimit = 100 + + // bootTokenProbeTimeout bounds the construction-time token probe so a + // hung `gh auth token` fallback (broken shell, unhealthy PATH) cannot + // stall daemon startup. gh is sub-100ms on a healthy box; 5s gives + // ~50x headroom without making daemon-start feel slow. + bootTokenProbeTimeout = 5 * time.Second ) // Sentinel errors. Adapter-level callers should match on these via @@ -104,13 +110,17 @@ type Tracker struct { } // New returns a Tracker. It fails fast when no token can be obtained so -// daemons crash at startup rather than at first issue lookup. +// daemons crash at startup rather than at first issue lookup. The token +// probe is bounded by a short deadline so a hung `gh auth token` fallback +// (broken shell, unhealthy PATH) cannot stall daemon startup. func New(opts Options) (*Tracker, error) { src := opts.Token if src == nil { return nil, ErrNoToken } - if _, err := src.Token(context.Background()); err != nil { + bootCtx, cancel := context.WithTimeout(context.Background(), bootTokenProbeTimeout) + defer cancel() + if _, err := src.Token(bootCtx); err != nil { return nil, err } t := &Tracker{ diff --git a/backend/internal/adapters/tracker/github/tracker_test.go b/backend/internal/adapters/tracker/github/tracker_test.go index a61a6899..1a39d73a 100644 --- a/backend/internal/adapters/tracker/github/tracker_test.go +++ b/backend/internal/adapters/tracker/github/tracker_test.go @@ -7,6 +7,8 @@ import ( "io" "net/http" "net/http/httptest" + "os" + "os/exec" "reflect" "strconv" "strings" @@ -101,6 +103,158 @@ func TestNewRejectsMissingToken(t *testing.T) { } } +// TestNewBoundsTokenProbe pins the construction-time deadline so a future +// refactor can't silently drop it. The TokenSource stub blocks until ctx +// is cancelled; New() must return within the configured timeout + slack. +func TestNewBoundsTokenProbe(t *testing.T) { + blocking := EnvTokenSource{ + GH: func(ctx context.Context) (string, error) { + <-ctx.Done() + return "", ctx.Err() + }, + } + t.Setenv("GITHUB_TOKEN", "") // force chain to reach the gh stub + started := time.Now() + _, err := New(Options{Token: blocking}) + elapsed := time.Since(started) + + if !errors.Is(err, ErrNoToken) { + t.Fatalf("err = %v, want ErrNoToken", err) + } + // Generous CI margin: deadline is 5s, accept 4-8s. + if elapsed < 4*time.Second { + t.Fatalf("New() returned in %v — deadline not enforced", elapsed) + } + if elapsed > 8*time.Second { + t.Fatalf("New() took %v — deadline ineffective", elapsed) + } +} + +// TestEnvTokenSource covers the auth lookup chain that gives us zero-config +// behavior on a stock dev machine: configured env vars beat GITHUB_TOKEN +// beats `gh auth token`. All three paths must work, AND a failing gh +// fallback must surface as ErrNoToken (errors.Is) — the exec error is +// wrapped under it for debuggability but never propagated as its own type. +func TestEnvTokenSource(t *testing.T) { + // t.Setenv handles its own restore, but several subtests call + // os.Unsetenv directly (Go has no t.Unsetenv). Save-and-restore at + // the parent level so an explicit unset in one subtest doesn't leak + // into the next or into other tests in the package. + saved := map[string]string{} + for _, k := range []string{"GITHUB_TOKEN", "AO_GITHUB_TOKEN"} { + saved[k] = os.Getenv(k) + _ = os.Unsetenv(k) + } + t.Cleanup(func() { + for k, v := range saved { + if v == "" { + _ = os.Unsetenv(k) + } else { + _ = os.Setenv(k, v) + } + } + }) + + t.Run("configured env var wins over everything", func(t *testing.T) { + t.Setenv("AO_GITHUB_TOKEN", "from-configured") + t.Setenv("GITHUB_TOKEN", "from-github-token") + ghCalled := false + src := EnvTokenSource{ + EnvVars: []string{"AO_GITHUB_TOKEN"}, + GH: func(context.Context) (string, error) { ghCalled = true; return "from-gh", nil }, + } + got, err := src.Token(context.Background()) + if err != nil || got != "from-configured" { + t.Fatalf("got=%q err=%v", got, err) + } + if ghCalled { + t.Fatal("GH fallback called despite env var being set") + } + }) + + t.Run("GITHUB_TOKEN used when configured env empty", func(t *testing.T) { + _ = os.Unsetenv("AO_GITHUB_TOKEN") + t.Setenv("GITHUB_TOKEN", "from-github-token") + ghCalled := false + src := EnvTokenSource{ + EnvVars: []string{"AO_GITHUB_TOKEN"}, + GH: func(context.Context) (string, error) { ghCalled = true; return "from-gh", nil }, + } + got, err := src.Token(context.Background()) + if err != nil || got != "from-github-token" { + t.Fatalf("got=%q err=%v", got, err) + } + if ghCalled { + t.Fatal("GH fallback called despite GITHUB_TOKEN being set") + } + }) + + t.Run("gh fallback used when env empty", func(t *testing.T) { + _ = os.Unsetenv("AO_GITHUB_TOKEN") + _ = os.Unsetenv("GITHUB_TOKEN") + src := EnvTokenSource{ + EnvVars: []string{"AO_GITHUB_TOKEN"}, + GH: func(context.Context) (string, error) { return " from-gh \n", nil }, // trimmed + } + got, err := src.Token(context.Background()) + if err != nil || got != "from-gh" { + t.Fatalf("got=%q err=%v", got, err) + } + }) + + t.Run("gh exec.ErrNotFound surfaces as bare ErrNoToken", func(t *testing.T) { + // Binary missing is the uninteresting case — operator hasn't + // installed gh. No useful explanation to surface; plain ErrNoToken. + _ = os.Unsetenv("AO_GITHUB_TOKEN") + _ = os.Unsetenv("GITHUB_TOKEN") + src := EnvTokenSource{ + EnvVars: []string{"AO_GITHUB_TOKEN"}, + GH: func(context.Context) (string, error) { return "", &exec.Error{Name: "gh", Err: exec.ErrNotFound} }, + } + _, err := src.Token(context.Background()) + if !errors.Is(err, ErrNoToken) { + t.Fatalf("err = %v, want ErrNoToken", err) + } + // Bare ErrNoToken: no wrapping suffix. + if got := err.Error(); strings.Contains(got, "gh fallback") { + t.Fatalf("err = %q, expected no gh-fallback annotation for missing-binary case", got) + } + }) + + t.Run("gh runtime error wrapped under ErrNoToken with cause", func(t *testing.T) { + // gh exists but failed (not logged in, etc). errors.Is contract still + // holds, AND the operator-useful explanation is in the message. + _ = os.Unsetenv("AO_GITHUB_TOKEN") + _ = os.Unsetenv("GITHUB_TOKEN") + src := EnvTokenSource{ + EnvVars: []string{"AO_GITHUB_TOKEN"}, + GH: func(context.Context) (string, error) { + return "", errors.New("not logged in to any GitHub hosts. Try `gh auth login`") + }, + } + _, err := src.Token(context.Background()) + if !errors.Is(err, ErrNoToken) { + t.Fatalf("err = %v, want errors.Is(err, ErrNoToken)", err) + } + if got := err.Error(); !strings.Contains(got, "not logged in") { + t.Fatalf("err = %q, expected gh's explanation in the wrapped message", got) + } + }) + + t.Run("gh fallback empty stdout falls through", func(t *testing.T) { + _ = os.Unsetenv("AO_GITHUB_TOKEN") + _ = os.Unsetenv("GITHUB_TOKEN") + src := EnvTokenSource{ + EnvVars: []string{"AO_GITHUB_TOKEN"}, + GH: func(context.Context) (string, error) { return " \n\t ", nil }, + } + _, err := src.Token(context.Background()) + if !errors.Is(err, ErrNoToken) { + t.Fatalf("err = %v, want ErrNoToken", err) + } + }) +} + func TestParseID(t *testing.T) { cases := []struct { name string