-
Notifications
You must be signed in to change notification settings - Fork 6
fix: capability reporting and scraper auth #174
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ac71ca2 to
4659956
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes capability reporting and scraper authentication by removing the skip login verification feature and implementing real capability detection. The changes simplify the Twitter authentication flow and ensure more accurate capability reporting.
- Removed complex skip login verification logic from Twitter scraper authentication
- Simplified Twitter authentication to only use cookie-based authentication without fallback login
- Replaced JobServer-based capability reporting with real capability detection that probes actual APIs and actors
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/jobs/twitter/scraper.go | Replaced complex login verification logic with simple bearer token setting |
| internal/jobs/twitter/cookies.go | Removed logout call before loading cookies |
| internal/jobs/twitter/common.go | Removed skip verification configuration |
| internal/jobs/twitter/auth.go | Simplified authentication to only support cookie loading, removed login fallback |
| internal/jobs/twitter.go | Removed skip login verification from auth config |
| internal/jobs/stats/stats.go | Changed capability reporting to use real detection instead of JobServer |
| internal/config/config.go | Fixed configuration parsing bugs and improved log level handling |
| internal/capabilities/detector_test.go | Updated test expectations for real capability detection |
| internal/capabilities/detector.go | Changed to always perform real capability detection |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
mcamou
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving with a couple of coding comments.
🎯 Primary Focus: Capability Detection Fix
The main change in this branch addresses a critical issue where production workers were reporting capabilities they didn't actually have access to.
Key Problem Solved:
jobServer.GetWorkerCapabilities()which only checked configuration, not real API accesscapabilities.DetectCapabilities()which actually probes Apify actors to verify real access📊 Change Statistics:
🔧 Major Changes by Category:
1. Capability Detection Overhaul (Most Critical)
internal/capabilities/detector.go: Removed JobServer bypass, now always performs real API probinginternal/jobs/stats/stats.go: Switched from worker-based to real detectioninternal/capabilities/detector_test.go: Updated tests to reflect new behavior2. Twitter Authentication Improvements
internal/jobs/twitter/: Multiple files cleaned up and simplifiedinternal/jobs/twitter.go: Auth flow improvementsinternal/config/config.go: Configuration enhancements3. Cookie Management
internal/jobs/twitter/cookies.go: Cookie handling improvementsinternal/jobs/twitter_test.go: Related test updates🚀 Impact:
Production Behavior Change:
Expected Production Logs:
Reliability Improvement:
🎯 Bottom Line:
This branch fixes a critical reliability issue where miners could claim capabilities they didn't actually have access to. The fix ensures accurate capability reporting by forcing real API probing in production, matching the behavior you see in your tests.