diff --git a/apps/backend/internal/github/auth_resolver.go b/apps/backend/internal/github/auth_resolver.go index 69083724ad..3a49b62ab5 100644 --- a/apps/backend/internal/github/auth_resolver.go +++ b/apps/backend/internal/github/auth_resolver.go @@ -89,6 +89,7 @@ type CredentialResolver struct { legacyGit legacyTransportCredentialFactory ghToken ghAccountTokenResolver now func() time.Time + rateTracker *RateTracker mu sync.Mutex cache map[credentialCacheKey]credentialCacheEntry @@ -125,6 +126,10 @@ func (r *CredentialResolver) SetLegacyTransportFactory(factory legacyTransportCr } } +func (r *CredentialResolver) SetRateTracker(tracker *RateTracker) { + r.rateTracker = tracker +} + func (r *CredentialResolver) SetInstallationProvider(provider installationCredentialProvider) { r.app = provider } @@ -431,6 +436,11 @@ func (r *CredentialResolver) resolveLegacy( (purpose == CredentialPurposeGitTransport && strings.TrimSpace(credential) == "") { return nil, ErrGitHubNotConfigured } + tracker := r.rateTracker + if tracker == nil { + tracker = NewRateTracker(nil, nil) + } + wireRateTracker(client, tracker) login, _ := client.GetAuthenticatedUser(ctx) return &ResolvedCredential{ Client: client, @@ -441,7 +451,7 @@ func (r *CredentialResolver) resolveLegacy( Source: ConnectionSourceLegacyShared, Login: login, }, - RateTracker: NewRateTracker(nil, nil), + RateTracker: tracker, }, nil } diff --git a/apps/backend/internal/github/auth_resolver_test.go b/apps/backend/internal/github/auth_resolver_test.go index 0d2431c768..c2f4f34e67 100644 --- a/apps/backend/internal/github/auth_resolver_test.go +++ b/apps/backend/internal/github/auth_resolver_test.go @@ -267,6 +267,42 @@ func TestCredentialResolverUsesLegacyOnlyForMigratedSource(t *testing.T) { } } +func TestLegacyResolverUsesServiceRateTracker(t *testing.T) { + store := newTestStore(t) + seedConnectionWorkspaces(t, store, "workspace-legacy-rate-limit") + connection := &WorkspaceConnection{ + WorkspaceID: "workspace-legacy-rate-limit", + Source: ConnectionSourceLegacyShared, + GitHubHost: defaultGitHubHost, + Status: ConnectionStatusActive, + CredentialGeneration: 1, + } + if err := store.UpsertWorkspaceConnection(context.Background(), connection); err != nil { + t.Fatalf("seed workspace connection: %v", err) + } + + service := NewService(nil, AuthMethodNone, nil, store, nil, testLogger(t)) + legacy := NewPATClient("legacy-token") + legacy.username = "legacy-user" + service.resolver.SetLegacyFactory(func(context.Context) (Client, string, error) { + return legacy, AuthMethodPAT, nil + }) + + resolved, err := service.resolver.Resolve(context.Background(), ResolveCredentialRequest{ + WorkspaceID: connection.WorkspaceID, + Purpose: CredentialPurposeAutomation, + }) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if resolved.RateTracker != service.rateTracker { + t.Fatalf("resolved tracker = %p, want service tracker %p", resolved.RateTracker, service.rateTracker) + } + if legacy.rateTracker != service.rateTracker { + t.Fatalf("legacy client tracker = %p, want service tracker %p", legacy.rateTracker, service.rateTracker) + } +} + func TestCredentialResolverNamedCLIUsesSelectedLogin(t *testing.T) { connections := &fakeConnectionReader{workspaces: map[string]*WorkspaceConnection{ "work": {WorkspaceID: "work", Source: ConnectionSourceGHCLI, GitHubHost: "github.com", Login: "work-user", Status: ConnectionStatusActive, CredentialGeneration: 2}, diff --git a/apps/backend/internal/github/client.go b/apps/backend/internal/github/client.go index 1cd7e73ff7..f7c0724cef 100644 --- a/apps/backend/internal/github/client.go +++ b/apps/backend/internal/github/client.go @@ -5,6 +5,14 @@ import ( "time" ) +// RateLimitFetcher exposes the optional initial rate-limit probe supported by +// authenticated GitHub clients. It is deliberately separate from Client so +// in-memory and specialized clients do not need to implement a settings-only +// operation. +type RateLimitFetcher interface { + FetchRateLimit(ctx context.Context) error +} + // Client defines the interface for interacting with the GitHub API. type Client interface { // IsAuthenticated checks if the client is authenticated with GitHub. diff --git a/apps/backend/internal/github/controller.go b/apps/backend/internal/github/controller.go index 5cdec399f0..15cd83398b 100644 --- a/apps/backend/internal/github/controller.go +++ b/apps/backend/internal/github/controller.go @@ -133,9 +133,19 @@ func (c *Controller) RegisterHTTPRoutes(router *gin.Engine) { func (c *Controller) httpGetStatus(ctx *gin.Context) { workspaceID := ctx.Query("workspace_id") - status, err := c.service.GetWorkspaceAuthStatus( - ctx.Request.Context(), workspaceID, currentGitHubUserID(ctx), + var ( + status *WorkspaceAuthStatus + err error ) + if ctx.Query("refresh_rate_limit") == "true" { + status, err = c.service.RefreshWorkspaceRateLimit( + ctx.Request.Context(), workspaceID, currentGitHubUserID(ctx), + ) + } else { + status, err = c.service.GetWorkspaceAuthStatus( + ctx.Request.Context(), workspaceID, currentGitHubUserID(ctx), + ) + } if err != nil { writeGitHubAuthError(ctx, err) return diff --git a/apps/backend/internal/github/gh_client.go b/apps/backend/internal/github/gh_client.go index d9f2329b09..de49579b87 100644 --- a/apps/backend/internal/github/gh_client.go +++ b/apps/backend/internal/github/gh_client.go @@ -1121,22 +1121,6 @@ func firstArg(args []string) string { return args[0] } -// ghRateLimitResponse mirrors the GET /rate_limit JSON shape so we can seed -// the tracker on startup and after CLI failures without parsing prose. -type ghRateLimitResponse struct { - Resources struct { - Core ghRateLimitBucket `json:"core"` - GraphQL ghRateLimitBucket `json:"graphql"` - Search ghRateLimitBucket `json:"search"` - } `json:"resources"` -} - -type ghRateLimitBucket struct { - Limit int `json:"limit"` - Remaining int `json:"remaining"` - Reset int64 `json:"reset"` -} - // FetchRateLimit calls `gh api rate_limit` and seeds the tracker with the // returned snapshots. Best-effort: a failure (e.g. CLI absent, network) is // logged and ignored. @@ -1148,26 +1132,11 @@ func (c *GHClient) FetchRateLimit(ctx context.Context) error { if err != nil { return fmt.Errorf("fetch rate limit: %w", err) } - var raw ghRateLimitResponse - if err := json.Unmarshal([]byte(out), &raw); err != nil { + raw, err := decodeRateLimitResponse([]byte(out)) + if err != nil { return fmt.Errorf("parse rate_limit response: %w", err) } - now := time.Now().UTC() - record := func(resource Resource, b ghRateLimitBucket) { - if b.Limit == 0 && b.Remaining == 0 && b.Reset == 0 { - return - } - c.rateTracker.Record(RateSnapshot{ - Resource: resource, - Limit: b.Limit, - Remaining: b.Remaining, - ResetAt: time.Unix(b.Reset, 0).UTC(), - UpdatedAt: now, - }) - } - record(ResourceCore, raw.Resources.Core) - record(ResourceGraphQL, raw.Resources.GraphQL) - record(ResourceSearch, raw.Resources.Search) + recordRateLimitResources(c.rateTracker, raw.Resources) return nil } diff --git a/apps/backend/internal/github/pat_client_testserver_test.go b/apps/backend/internal/github/pat_client_testserver_test.go index 3f4e8c74e0..b4f521ca6f 100644 --- a/apps/backend/internal/github/pat_client_testserver_test.go +++ b/apps/backend/internal/github/pat_client_testserver_test.go @@ -1,6 +1,7 @@ package github import ( + "context" "io" "net/http" "net/http/httptest" @@ -10,6 +11,39 @@ import ( "testing" ) +func TestTokenClientFetchRateLimitSeedsAllBuckets(t *testing.T) { + client, requests := newRecordingPATServer(t, map[string]string{ + "/rate_limit": `{"resources":{ + "core":{"limit":5000,"remaining":4321,"reset":2000000000}, + "graphql":{"limit":5000,"remaining":4900,"reset":2000000000}, + "search":{"limit":30,"remaining":25,"reset":2000000000} + }}`, + }) + tracker := NewRateTracker(nil, nil) + client.WithRateTracker(tracker) + + if err := client.FetchRateLimit(context.Background()); err != nil { + t.Fatalf("FetchRateLimit: %v", err) + } + if len(*requests) != 1 || (*requests)[0].Method != http.MethodGet || (*requests)[0].Path != "/rate_limit" { + t.Fatalf("requests = %+v, want one GET /rate_limit", *requests) + } + + for resource, wantRemaining := range map[Resource]int{ + ResourceCore: 4321, + ResourceGraphQL: 4900, + ResourceSearch: 25, + } { + snapshot, ok := tracker.Snapshot(resource) + if !ok { + t.Fatalf("missing %s rate-limit snapshot", resource) + } + if snapshot.Remaining != wantRemaining { + t.Errorf("%s remaining = %d, want %d", resource, snapshot.Remaining, wantRemaining) + } + } +} + // patRequest records what a PATClient actually put on the wire so a test can // assert method, path, query, headers and body rather than only the decoded // result. diff --git a/apps/backend/internal/github/provider.go b/apps/backend/internal/github/provider.go index b90c69739c..656c2416cc 100644 --- a/apps/backend/internal/github/provider.go +++ b/apps/backend/internal/github/provider.go @@ -19,16 +19,24 @@ const rateLimitSeedTimeout = 5 * time.Second // implementation we ended up with, then seeds initial snapshots. The seed is // best-effort — a missing CLI or network blip should not block startup. func attachRateTracker(client Client, tracker *RateTracker, log *logger.Logger) { + wireRateTracker(client, tracker) + c, ok := client.(*GHClient) + if !ok { + return + } + seedCtx, cancel := context.WithTimeout(context.Background(), rateLimitSeedTimeout) + defer cancel() + if err := c.FetchRateLimit(seedCtx); err != nil { + log.Debug("seed gh rate limit failed", zap.Error(err)) + } +} + +func wireRateTracker(client Client, tracker *RateTracker) { switch c := client.(type) { case *PATClient: c.WithRateTracker(tracker) case *GHClient: c.WithRateTracker(tracker) - seedCtx, cancel := context.WithTimeout(context.Background(), rateLimitSeedTimeout) - defer cancel() - if err := c.FetchRateLimit(seedCtx); err != nil { - log.Debug("seed gh rate limit failed", zap.Error(err)) - } } } diff --git a/apps/backend/internal/github/rate_limit_fetch.go b/apps/backend/internal/github/rate_limit_fetch.go new file mode 100644 index 0000000000..81511f98ba --- /dev/null +++ b/apps/backend/internal/github/rate_limit_fetch.go @@ -0,0 +1,71 @@ +package github + +import ( + "context" + "encoding/json" + "fmt" + "time" +) + +// rateLimitResponse mirrors the GitHub GET /rate_limit JSON shape. The +// endpoint includes additional resource buckets, but these are the buckets +// Kandev tracks and displays. +type rateLimitResponse struct { + Resources rateLimitResources `json:"resources"` +} + +type rateLimitResources struct { + Core rateLimitBucket `json:"core"` + GraphQL rateLimitBucket `json:"graphql"` + Search rateLimitBucket `json:"search"` +} + +type rateLimitBucket struct { + Limit int `json:"limit"` + Remaining int `json:"remaining"` + Reset int64 `json:"reset"` +} + +func recordRateLimitResources(tracker *RateTracker, resources rateLimitResources) { + if tracker == nil { + return + } + now := time.Now().UTC() + record := func(resource Resource, bucket rateLimitBucket) { + if bucket.Limit == 0 && bucket.Remaining == 0 && bucket.Reset == 0 { + return + } + tracker.Record(RateSnapshot{ + Resource: resource, + Limit: bucket.Limit, + Remaining: bucket.Remaining, + ResetAt: time.Unix(bucket.Reset, 0).UTC(), + UpdatedAt: now, + }) + } + record(ResourceCore, resources.Core) + record(ResourceGraphQL, resources.GraphQL) + record(ResourceSearch, resources.Search) +} + +// FetchRateLimit calls GET /rate_limit and seeds the client's tracker with +// the current REST, GraphQL, and search buckets. +func (c *TokenClient) FetchRateLimit(ctx context.Context) error { + if c.rateTracker == nil { + return nil + } + var raw rateLimitResponse + if err := c.get(ctx, "/rate_limit", &raw); err != nil { + return fmt.Errorf("fetch rate limit: %w", err) + } + recordRateLimitResources(c.rateTracker, raw.Resources) + return nil +} + +func decodeRateLimitResponse(data []byte) (rateLimitResponse, error) { + var raw rateLimitResponse + if err := json.Unmarshal(data, &raw); err != nil { + return rateLimitResponse{}, err + } + return raw, nil +} diff --git a/apps/backend/internal/github/ratelimit.go b/apps/backend/internal/github/ratelimit.go index bf4790ed49..88f4f27119 100644 --- a/apps/backend/internal/github/ratelimit.go +++ b/apps/backend/internal/github/ratelimit.go @@ -70,6 +70,8 @@ type RateTracker struct { snapshots map[Resource]RateSnapshot exhausted map[Resource]bool lastEmitted map[Resource]time.Time + refreshMu sync.Mutex + refreshDone chan struct{} bus bus.EventBus log *logger.Logger } @@ -144,6 +146,30 @@ func (r *RateTracker) All() map[Resource]RateSnapshot { return r.allLocked() } +func (r *RateTracker) beginRateLimitRefresh(force bool) (chan struct{}, bool) { + r.refreshMu.Lock() + defer r.refreshMu.Unlock() + if r.refreshDone != nil { + return r.refreshDone, false + } + if !force && hasCompleteRateLimitSnapshot(r) { + return nil, false + } + done := make(chan struct{}) + r.refreshDone = done + return done, true +} + +func (r *RateTracker) finishRateLimitRefresh(done chan struct{}) { + r.refreshMu.Lock() + defer r.refreshMu.Unlock() + if r.refreshDone != done { + return + } + r.refreshDone = nil + close(done) +} + func (r *RateTracker) allLocked() map[Resource]RateSnapshot { out := make(map[Resource]RateSnapshot, len(r.snapshots)) for k, v := range r.snapshots { diff --git a/apps/backend/internal/github/service.go b/apps/backend/internal/github/service.go index 603dda29c5..98cfbe7f69 100644 --- a/apps/backend/internal/github/service.go +++ b/apps/backend/internal/github/service.go @@ -208,6 +208,7 @@ func NewService(client Client, authMethod string, secrets SecretProvider, store } if store != nil { service.resolver = NewCredentialResolver(store, secrets) + service.resolver.SetRateTracker(service.rateTracker) service.resolver.SetLegacyFactory(func(ctx context.Context) (Client, string, error) { return NewClient(ctx, secrets, log) }) diff --git a/apps/backend/internal/github/service_app_auth.go b/apps/backend/internal/github/service_app_auth.go index 5f2149998e..49e090a918 100644 --- a/apps/backend/internal/github/service_app_auth.go +++ b/apps/backend/internal/github/service_app_auth.go @@ -5,8 +5,13 @@ import ( "errors" "fmt" "strings" + "time" + + "go.uber.org/zap" ) +const workspaceRateLimitSeedTimeout = 5 * time.Second + type githubAppRuntime struct { registrationID string source DeploymentAppSource @@ -444,6 +449,23 @@ type WorkspaceAuthStatus struct { func (s *Service) GetWorkspaceAuthStatus( ctx context.Context, workspaceID, userID string, +) (*WorkspaceAuthStatus, error) { + return s.getWorkspaceAuthStatus(ctx, workspaceID, userID, false) +} + +// RefreshWorkspaceRateLimit returns the workspace status after fetching a +// fresh GitHub rate-limit snapshot for the resolved automation credential. +func (s *Service) RefreshWorkspaceRateLimit( + ctx context.Context, + workspaceID, userID string, +) (*WorkspaceAuthStatus, error) { + return s.getWorkspaceAuthStatus(ctx, workspaceID, userID, true) +} + +func (s *Service) getWorkspaceAuthStatus( + ctx context.Context, + workspaceID, userID string, + refreshRateLimit bool, ) (*WorkspaceAuthStatus, error) { if strings.TrimSpace(workspaceID) == "" { return nil, ErrGitHubWorkspaceRequired @@ -492,6 +514,7 @@ func (s *Service) GetWorkspaceAuthStatus( status.Automation.Actor = principalPointer(automation.Principal) status.Automation.Capabilities = automation.Capabilities status.Automation.MissingCapabilities = missingGitHubCapabilities(automation.Capabilities) + s.seedWorkspaceRateLimit(ctx, automation, refreshRateLimit) status.Automation.RateLimit = rateLimitInfoForTracker(automation.RateTracker) status.RateLimit = status.Automation.RateLimit @@ -499,6 +522,55 @@ func (s *Service) GetWorkspaceAuthStatus( return status, nil } +func (s *Service) seedWorkspaceRateLimit( + ctx context.Context, + resolved *ResolvedCredential, + force bool, +) { + if resolved == nil || resolved.RateTracker == nil { + return + } + fetcher, ok := resolved.Client.(RateLimitFetcher) + if !ok { + return + } + tracker := resolved.RateTracker + for { + done, shouldFetch := tracker.beginRateLimitRefresh(force) + if shouldFetch { + defer tracker.finishRateLimitRefresh(done) + break + } + if done == nil { + return + } + select { + case <-done: + if hasCompleteRateLimitSnapshot(tracker) { + return + } + case <-ctx.Done(): + return + } + } + seedCtx, cancel := context.WithTimeout(ctx, workspaceRateLimitSeedTimeout) + defer cancel() + if err := fetcher.FetchRateLimit(seedCtx); err != nil && s.logger != nil { + s.logger.Debug("seed workspace GitHub rate limit failed", zap.Error(err)) + } +} + +func hasCompleteRateLimitSnapshot(tracker *RateTracker) bool { + if tracker == nil { + return false + } + all := tracker.All() + _, hasCore := all[ResourceCore] + _, hasGraphQL := all[ResourceGraphQL] + _, hasSearch := all[ResourceSearch] + return hasCore && hasGraphQL && hasSearch +} + func (s *Service) workspacePersonalConnection( ctx context.Context, workspaceID, userID string, diff --git a/apps/backend/internal/github/service_app_auth_test.go b/apps/backend/internal/github/service_app_auth_test.go index 9ae0e2a0a7..a2ce4c271a 100644 --- a/apps/backend/internal/github/service_app_auth_test.go +++ b/apps/backend/internal/github/service_app_auth_test.go @@ -3,9 +3,256 @@ package github import ( "context" "errors" + "sync" "testing" + "time" ) +type rateLimitSeedClient struct { + *MockClient + tracker *RateTracker + calls int +} + +func (c *rateLimitSeedClient) FetchRateLimit(context.Context) error { + c.calls++ + now := time.Now().UTC() + for _, snapshot := range []RateSnapshot{ + {Resource: ResourceCore, Remaining: 4321, Limit: 5000}, + {Resource: ResourceGraphQL, Remaining: 4322, Limit: 5000}, + {Resource: ResourceSearch, Remaining: 29, Limit: 30}, + } { + snapshot.ResetAt = now.Add(time.Hour) + snapshot.UpdatedAt = now + c.tracker.Record(snapshot) + } + return nil +} + +type rateLimitAutomationProvider struct { + client Client + tracker *RateTracker +} + +type concurrentRateLimitSeedClient struct { + *MockClient + tracker *RateTracker + mu sync.Mutex + calls int + firstStarted chan struct{} + secondStarted chan struct{} + release chan struct{} +} + +func (c *concurrentRateLimitSeedClient) FetchRateLimit(ctx context.Context) error { + c.mu.Lock() + c.calls++ + call := c.calls + c.mu.Unlock() + switch call { + case 1: + close(c.firstStarted) + case 2: + close(c.secondStarted) + } + select { + case <-c.release: + case <-ctx.Done(): + return ctx.Err() + } + now := time.Now().UTC() + for _, snapshot := range []RateSnapshot{ + {Resource: ResourceCore, Remaining: 4000 - call, Limit: 5000}, + {Resource: ResourceGraphQL, Remaining: 4001 - call, Limit: 5000}, + {Resource: ResourceSearch, Remaining: 28 - call, Limit: 30}, + } { + snapshot.ResetAt = now.Add(time.Hour) + snapshot.UpdatedAt = now + c.tracker.Record(snapshot) + } + return nil +} + +func (c *concurrentRateLimitSeedClient) callCount() int { + c.mu.Lock() + defer c.mu.Unlock() + return c.calls +} + +func (p rateLimitAutomationProvider) ResolveAutomation( + context.Context, + *WorkspaceConnection, + ResolveCredentialRequest, +) (*ResolvedCredential, error) { + return &ResolvedCredential{ + Client: p.client, + Capabilities: allTokenCapabilities(), + Principal: AuthPrincipal{ + Kind: AuthPrincipalHuman, + Source: ConnectionSourceGHCLI, + Login: "octocat", + }, + RateTracker: p.tracker, + }, nil +} + +func TestWorkspaceAuthStatusSeedsRateLimitOncePerCachedCredential(t *testing.T) { + store := newTestStore(t) + seedConnectionWorkspaces(t, store, "workspace-rate-limit") + connection := &WorkspaceConnection{ + WorkspaceID: "workspace-rate-limit", + Source: ConnectionSourceGHCLI, + GitHubHost: defaultGitHubHost, + Login: "octocat", + Status: ConnectionStatusActive, + CredentialGeneration: 1, + } + if err := store.UpsertWorkspaceConnection(context.Background(), connection); err != nil { + t.Fatalf("seed workspace connection: %v", err) + } + + tracker := NewRateTracker(nil, nil) + client := &rateLimitSeedClient{MockClient: NewMockClient(), tracker: tracker} + service := NewService(client, AuthMethodPAT, nil, store, nil, testLogger(t)) + service.resolver = NewCredentialResolver(testConnectionReader{ + workspaces: map[string]*WorkspaceConnection{connection.WorkspaceID: connection}, + }, nil) + service.resolver.SetAutomationProvider(rateLimitAutomationProvider{client: client, tracker: tracker}) + + for i := 0; i < 2; i++ { + status, err := service.GetWorkspaceAuthStatus(context.Background(), connection.WorkspaceID, DefaultUserID) + if err != nil { + t.Fatalf("GetWorkspaceAuthStatus call %d: %v", i+1, err) + } + if status.RateLimit == nil || status.RateLimit.Core == nil { + t.Fatalf("call %d rate limit = %+v, want seeded core snapshot", i+1, status.RateLimit) + } + if status.RateLimit.Core.Remaining != 4321 { + t.Errorf("call %d remaining = %d, want 4321", i+1, status.RateLimit.Core.Remaining) + } + } + if client.calls != 1 { + t.Fatalf("FetchRateLimit calls = %d, want 1 for cached credential", client.calls) + } + + status, err := service.RefreshWorkspaceRateLimit( + context.Background(), connection.WorkspaceID, DefaultUserID, + ) + if err != nil { + t.Fatalf("RefreshWorkspaceRateLimit: %v", err) + } + if status.RateLimit == nil || status.RateLimit.Core == nil { + t.Fatalf("forced refresh rate limit = %+v, want seeded core snapshot", status.RateLimit) + } + if client.calls != 2 { + t.Fatalf("FetchRateLimit calls after forced refresh = %d, want 2", client.calls) + } +} + +func TestWorkspaceAuthStatusCompletesPartialRateLimitSeed(t *testing.T) { + store := newTestStore(t) + seedConnectionWorkspaces(t, store, "workspace-partial-rate-limit") + connection := &WorkspaceConnection{ + WorkspaceID: "workspace-partial-rate-limit", + Source: ConnectionSourceGHCLI, + GitHubHost: defaultGitHubHost, + Login: "octocat", + Status: ConnectionStatusActive, + CredentialGeneration: 1, + } + if err := store.UpsertWorkspaceConnection(context.Background(), connection); err != nil { + t.Fatalf("seed workspace connection: %v", err) + } + + tracker := NewRateTracker(nil, nil) + tracker.Record(RateSnapshot{ + Resource: ResourceCore, Remaining: 4999, Limit: 5000, + ResetAt: time.Now().Add(time.Hour).UTC(), UpdatedAt: time.Now().UTC(), + }) + client := &rateLimitSeedClient{MockClient: NewMockClient(), tracker: tracker} + service := NewService(client, AuthMethodPAT, nil, store, nil, testLogger(t)) + service.resolver = NewCredentialResolver(testConnectionReader{ + workspaces: map[string]*WorkspaceConnection{connection.WorkspaceID: connection}, + }, nil) + service.resolver.SetAutomationProvider(rateLimitAutomationProvider{client: client, tracker: tracker}) + + status, err := service.GetWorkspaceAuthStatus( + context.Background(), connection.WorkspaceID, DefaultUserID, + ) + if err != nil { + t.Fatalf("GetWorkspaceAuthStatus: %v", err) + } + if client.calls != 1 { + t.Fatalf("FetchRateLimit calls = %d, want 1 for a partial tracker", client.calls) + } + if status.RateLimit == nil || status.RateLimit.GraphQL == nil || status.RateLimit.Search == nil { + t.Fatalf("rate limit = %+v, want all seeded buckets", status.RateLimit) + } +} + +func TestWorkspaceRateLimitSeedSerializesConcurrentRefreshes(t *testing.T) { + store := newTestStore(t) + seedConnectionWorkspaces(t, store, "workspace-concurrent-rate-limit") + connection := &WorkspaceConnection{ + WorkspaceID: "workspace-concurrent-rate-limit", + Source: ConnectionSourceGHCLI, + GitHubHost: defaultGitHubHost, + Login: "octocat", + Status: ConnectionStatusActive, + CredentialGeneration: 1, + } + if err := store.UpsertWorkspaceConnection(context.Background(), connection); err != nil { + t.Fatalf("seed workspace connection: %v", err) + } + + tracker := NewRateTracker(nil, nil) + client := &concurrentRateLimitSeedClient{ + MockClient: NewMockClient(), + tracker: tracker, + firstStarted: make(chan struct{}), + secondStarted: make(chan struct{}), + release: make(chan struct{}), + } + service := NewService(client, AuthMethodPAT, nil, store, nil, testLogger(t)) + service.resolver = NewCredentialResolver(testConnectionReader{ + workspaces: map[string]*WorkspaceConnection{connection.WorkspaceID: connection}, + }, nil) + service.resolver.SetAutomationProvider(rateLimitAutomationProvider{client: client, tracker: tracker}) + + regularDone := make(chan error, 1) + go func() { + _, err := service.GetWorkspaceAuthStatus(context.Background(), connection.WorkspaceID, DefaultUserID) + regularDone <- err + }() + <-client.firstStarted + + forcedDone := make(chan error, 1) + go func() { + _, err := service.RefreshWorkspaceRateLimit(context.Background(), connection.WorkspaceID, DefaultUserID) + forcedDone <- err + }() + + secondStarted := false + select { + case <-client.secondStarted: + secondStarted = true + case <-time.After(100 * time.Millisecond): + } + close(client.release) + if err := <-regularDone; err != nil { + t.Fatalf("regular status: %v", err) + } + if err := <-forcedDone; err != nil { + t.Fatalf("forced status: %v", err) + } + if secondStarted { + t.Fatal("concurrent forced refresh started a duplicate rate-limit fetch") + } + if got := client.callCount(); got != 1 { + t.Fatalf("FetchRateLimit calls = %d, want 1", got) + } +} + func TestAppAuthRegistryHotAddsAndInvalidatesRegistrationsIndependently(t *testing.T) { store := newTestStore(t) service := NewService(nil, AuthMethodNone, nil, store, nil, testLogger(t)) diff --git a/apps/web/components/github/github-access-help.tsx b/apps/web/components/github/github-access-help.tsx index 6bd2fc8f9d..fdc51984bd 100644 --- a/apps/web/components/github/github-access-help.tsx +++ b/apps/web/components/github/github-access-help.tsx @@ -19,14 +19,20 @@ export function GitHubAccessHelp({ title, description, content, + onOpen, }: { label: string; title: string; description: string; content?: ReactNode; + onOpen?: () => void; }) { const usesTouchDrawer = useTouchDrawer(); const [open, setOpen] = useState(false); + const handleDrawerOpenChange = (nextOpen: boolean) => { + setOpen(nextOpen); + if (nextOpen) onOpen?.(); + }; const button = (