feat: poison hearbeats - #1676
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
There are correctness and contract issues (notably the default_scope recursion risk and timezone-inconsistent cutoff coercion) that should be addressed before merge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a “heartbeat poisoning” mechanism to hide a user’s heartbeats up to an admin-defined cutoff (without deleting rows), plus a superadmin-only Admin API to apply/remove the ban and expose ban state. This fits into the moderation/admin tooling layer while ensuring downstream derived totals (dashboard rollups, leaderboard cache) are invalidated on state changes.
Changes:
- Introduces
User#apply_poison!/#remove_poison!and user fields (poisoned_until,poisoned_at,poison_reason) to track ban state. - Adds
HeartbeatPoisoningto automatically exclude poisoned heartbeats by default, with a thread-local escape hatch (Heartbeat.including_poison). - Adds admin endpoints (
GET/POST/DELETE /api/admin/v1/ban/:hackatime_id) with tests and Swagger/Rswag documentation updates.
File summaries
| File | Description |
|---|---|
| test/models/concerns/heartbeat_poisoning_test.rb | Exercises hiding/unhiding behaviour, timezone cutoffs, and derived-data invalidation scheduling. |
| test/controllers/api/admin/v1/bans_controller_test.rb | Integration tests for the admin ban API, including auth rules and response payload fields. |
| swagger/admin/swagger.yaml | Documents the new ban endpoints and request/response schemas for the admin OpenAPI spec. |
| spec/requests/api/admin/v1/bans_spec.rb | Rswag specs that generate/validate the admin Swagger for the ban endpoints. |
| db/schema.rb | Reflects new poison-related columns and index on users.poisoned_until. |
| db/migrate/20260903120000_add_poison_to_users.rb | Adds poison tracking columns + partial index to users. |
| config/routes.rb | Adds admin routes for ban show/create/destroy. |
| app/models/user.rb | Implements poisoning state, cutoff coercion/validation, and derived-data invalidation. |
| app/models/heartbeat.rb | Mixes in HeartbeatPoisoning so poisoning affects heartbeat reads by default. |
| app/models/concerns/heartbeat_poisoning.rb | Implements the default exclusion logic + including_poison override. |
| app/controllers/api/admin/v1/bans_controller.rb | New superadmin-only controller to apply/remove poison and report ban state. |
| app/controllers/api/admin/application_controller.rb | Adds an include_poison around_action to optionally include poisoned heartbeats for admin requests. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 8
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Greptile SummaryAdds heartbeat poisoning as a superadmin moderation capability.
Confidence Score: 5/5The PR appears safe to merge, with the previously reported findings resolved and no new actionable regression identified. The latest changes correctly introduce a permitted parameter boundary without breaking the documented JSON body or tested plain-text date input. Every previous review thread is manually resolved, so no outstanding finding remains. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Superadmin ban request] --> B[Resolve user and cutoff]
B --> C[Store poison state]
C --> D[Default heartbeat scope]
D --> E[Hide pre-cutoff heartbeats]
C --> F[Refresh dashboard rollups]
C --> G[Rebuild current leaderboards]
C --> H[Refresh activity caches]
I[Heartbeat ingestion] --> J[Unfiltered deduplication lookup]
J --> K[(Stored heartbeat records)]
L[Superadmin unban request] --> M[Clear poison state]
M --> F
M --> G
M --> H
M --> D
Reviews (6): Last reviewed commit: "Merge branch 'hearbeat-poisoning' of htt..." | Re-trigger Greptile |
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
No description provided.