fix collaborator API access

Author: Thomas Stromberg

pkg/prx/collaborators_cache.go

@@ -43,6 +43,41 @@ func newCollaboratorsCache(cacheDir string) *collaboratorsCache {
 	return cc
 }
 
+// get retrieves a cached collaborators list if it exists and is not expired.
+func (cc *collaboratorsCache) get(owner, repo string) (map[string]string, bool) {
+	key := fmt.Sprintf("%s/%s", owner, repo)
+
+	cc.mu.RLock()
+	defer cc.mu.RUnlock()
+
+	entry, exists := cc.memory[key]
+	if !exists {
+		return nil, false
+	}
+
+	// Check if cache entry is expired
+	if time.Since(entry.CachedAt) > collaboratorsCacheDuration {
+		return nil, false
+	}
+
+	return entry.Collaborators, true
+}
+
+// set stores a collaborators list in the cache.
+func (cc *collaboratorsCache) set(owner, repo string, collaborators map[string]string) error {
+	key := fmt.Sprintf("%s/%s", owner, repo)
+
+	cc.mu.Lock()
+	cc.memory[key] = collaboratorsEntry{
+		Collaborators: collaborators,
+		CachedAt:      time.Now(),
+	}
+	cc.mu.Unlock()
+
+	// Save to disk after releasing the lock
+	return cc.saveToDisk()
+}
+
 // loadFromDisk loads the cache from disk.
 func (cc *collaboratorsCache) loadFromDisk() error {
 	// Skip if no disk path is set (in-memory only mode)
@@ -75,3 +110,36 @@ func (cc *collaboratorsCache) loadFromDisk() error {
 
 	return nil
 }
+
+// saveToDisk saves the current cache to disk.
+func (cc *collaboratorsCache) saveToDisk() error {
+	// Skip if no disk path is set (in-memory only mode)
+	if cc.diskPath == "" {
+		return nil
+	}
+
+	// Create a copy to avoid holding the lock during I/O
+	cc.mu.RLock()
+	cacheCopy := make(map[string]collaboratorsEntry, len(cc.memory))
+	for k, v := range cc.memory {
+		cacheCopy[k] = v
+	}
+	cc.mu.RUnlock()
+
+	data, err := json.Marshal(cacheCopy)
+	if err != nil {
+		return fmt.Errorf("marshaling collaborators cache: %w", err)
+	}
+
+	// Write to temp file first, then rename for atomicity
+	tempFile := cc.diskPath + ".tmp"
+	if err := os.WriteFile(tempFile, data, 0o600); err != nil {
+		return fmt.Errorf("writing collaborators cache: %w", err)
+	}
+
+	if err := os.Rename(tempFile, cc.diskPath); err != nil {
+		return fmt.Errorf("renaming collaborators cache: %w", err)
+	}
+
+	return nil
+}

pkg/prx/graphql_complete.go

@@ -1471,7 +1471,7 @@ func (*Client) parseGraphQLTimelineEvent(_ /* ctx */ context.Context, item map[s
 }
 
 // writeAccessFromAssociation calculates write access from association.
-func (*Client) writeAccessFromAssociation(_ /* ctx */ context.Context, _ /* owner */, _ /* repo */, user, association string) int {
+func (c *Client) writeAccessFromAssociation(ctx context.Context, owner, repo, user, association string) int {
 	if user == "" {
 		return WriteAccessNA
 	}
@@ -1480,17 +1480,71 @@ func (*Client) writeAccessFromAssociation(_ /* ctx */ context.Context, _ /* owne
 	case "OWNER", "COLLABORATOR":
 		return WriteAccessDefinitely
 	case "MEMBER":
-		// For MEMBER, we'd need an additional API call to check permissions
-		// This is the one case where GraphQL doesn't give us everything
-		// For now, return likely
-		return WriteAccessLikely
+		// For MEMBER, check collaborators cache to determine actual permission level
+		// Members can have various permissions (admin, write, read) so we need to check
+		return c.checkCollaboratorPermission(ctx, owner, repo, user)
 	case "CONTRIBUTOR", "NONE", "FIRST_TIME_CONTRIBUTOR", "FIRST_TIMER":
 		return WriteAccessUnlikely
 	default:
 		return WriteAccessNA
 	}
 }
 
+// checkCollaboratorPermission checks if a user has write access by looking them up in the collaborators list.
+// Uses cache to avoid repeated API calls (4 hour TTL).
+func (c *Client) checkCollaboratorPermission(ctx context.Context, owner, repo, user string) int {
+	// Check cache first
+	if collabs, ok := c.collaboratorsCache.get(owner, repo); ok {
+		switch collabs[user] {
+		case "admin", "maintain", "write":
+			return WriteAccessDefinitely
+		case "read", "triage", "none":
+			return WriteAccessNo
+		default:
+			// User not in collaborators list
+			return WriteAccessUnlikely
+		}
+	}
+
+	// Cache miss - fetch collaborators from API
+	gc, ok := c.github.(*githubClient)
+	if !ok {
+		// Not a real GitHub client (probably test mock) - return likely as fallback
+		return WriteAccessLikely
+	}
+
+	collabs, err := gc.collaborators(ctx, owner, repo)
+	if err != nil {
+		// API call failed (could be 403 if no permission to list collaborators)
+		// Return likely as fallback
+		c.logger.WarnContext(ctx, "failed to fetch collaborators for write access check",
+			"owner", owner,
+			"repo", repo,
+			"user", user,
+			"error", err)
+		return WriteAccessLikely
+	}
+
+	// Store in cache
+	if err := c.collaboratorsCache.set(owner, repo, collabs); err != nil {
+		// Cache write failed, just log it and continue
+		c.logger.WarnContext(ctx, "failed to cache collaborators",
+			"owner", owner,
+			"repo", repo,
+			"error", err)
+	}
+
+	switch collabs[user] {
+	case "admin", "maintain", "write":
+		return WriteAccessDefinitely
+	case "read", "triage", "none":
+		return WriteAccessNo
+	default:
+		// User not in collaborators list
+		return WriteAccessUnlikely
+	}
+}
+
 // extractRequiredChecksFromGraphQL gets required checks from GraphQL response.
 func (*Client) extractRequiredChecksFromGraphQL(data *graphQLPullRequestComplete) []string {
 	checkMap := make(map[string]bool)