## Executive Summary This proposal outlines Phase 1 implementation for automatically resolving stale todos when merge requests are closed or merged. The current system resolves todos only for the user who performs the action, leaving other assignees/reviewers with accumulating stale todos that clutter their todo dashboard. **Key Goals:** 1. Auto-resolve todos for ALL affected users (not just the actor) when an MR closes/merges 2. Target specific "stale-prone" action types that become irrelevant after MR completion 3. Implement behind feature flag for safe rollout 4. Maintain performance with async processing --- ## Problem Statement ### Current Behavior When a merge request is merged or closed, the system calls: ```ruby # app/services/todo_service.rb:219-225 def resolve_todos_for_target(target, current_user) attributes = attributes_for_target(target) resolve_todos(pending_todos([current_user], attributes), current_user) GraphqlTriggers.issuable_todo_updated(target) end ``` **Issue:** This only resolves todos for `current_user` (the person who merged/closed the MR), but leaves pending todos for: - Other assignees - Reviewers who were requested but didn't complete review - Approvers whose approval is no longer needed - Anyone marked as APPROVAL_REQUIRED ### Impact From issue comments and description: - Todos accumulate over time, filling dashboards with "outdated litter" - Users who primarily use GitLab for code review (not task management) find the Todo feature unusable - Mentioned as important to commercial customers - Estimated 6.3M stale todos on GitLab.com (from issue description) --- ## Proposed Solution See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/229010+. ## Success Criteria ### Quantitative - [ ] Zero new stale todos created for target action types - [ ] Worker job completion rate > 99.9% - [ ] P95 job duration < 500ms per MR - [ ] Zero production incidents related to feature - [ ] Database query time < 50ms per update ### Qualitative - [ ] No user complaints about incorrect todo resolution - [ ] Positive feedback from dogfooding team - [ ] Code review approval from Code Review team - [ ] Security review approval (if required) --- ## Risks & Mitigations ### Risk 1: Users Want to Keep Certain Todos **Scenario:** A user wants to review an MR even after it's merged **Mitigation:** - Phase 1 only targets clearly stale action types (ASSIGNED, REVIEW_REQUESTED, APPROVAL_REQUIRED) - Does NOT auto-resolve MENTIONED or DIRECTLY_ADDRESSED todos - Users can still manually mark todos as "pending" if they want to track something - Future: Add user preference to opt-out of auto-resolution ### Risk 2: Performance Impact on Large MRs **Scenario:** MR with 50+ pending todos causes slow updates **Mitigation:** - Async processing prevents blocking merge operation - Batch update uses single SQL statement regardless of todo count - Monitored rollout catches issues early - Can add rate limiting or batch size limits if needed ### Risk 3: EE Action Types Unavailable in CE **Scenario:** `APPROVAL_REQUIRED` and `ADDED_APPROVER` don't exist in CE **Mitigation:** ```ruby stale_action_types = [ Todo::ASSIGNED, Todo::REVIEW_REQUESTED ] # Only include EE action types if they exist if defined?(Todo::APPROVAL_REQUIRED) stale_action_types << Todo::APPROVAL_REQUIRED stale_action_types << Todo::ADDED_APPROVER end ``` ### Risk 4: Race Condition with Reopened MRs **Scenario:** MR is closed, todos resolved, then MR is reopened **Mitigation:** - Todos stay resolved (correct behavior - they were already actioned) - New assignment/review request creates fresh todos - Users can manually re-create todos if needed - Document expected behavior in feature documentation --- ## Future Enhancements (Out of Scope for Phase 1) ### User Preferences Allow users to customize auto-resolution behavior: ```ruby # app/models/user_preference.rb attribute :auto_resolve_mr_todos, :boolean, default: true attribute :auto_resolve_issue_todos, :boolean, default: false ``` ### Phase 2: Historical Data Cleanup Background migration to clean up existing 6.3M stale todos (separate proposal) ### Extended Action Types Consider adding: - `UNMERGEABLE` - when MR becomes mergeable or is closed - `BUILD_FAILED` - when build succeeds or MR is closed ### Issue Todo Resolution Apply similar logic when issues are closed --- ## Questions for Discussion 1. **Should we resolve todos for the MR author?** - Author already gets their todos resolved when they close/merge - But if another user closes/merges, should author's todos also resolve? - Recommendation: Yes, for consistency 2. **Should this apply to draft MRs?** - Draft MRs can still be closed/merged - Recommendation: Yes, same behavior 3. **Should we add a GraphQL mutation to manually trigger this?** - Useful for testing and edge cases - Recommendation: Not for Phase 1, can add later if needed 4. **Should we send notifications about auto-resolved todos?** - Could be noisy for users with many todos - Recommendation: No notifications, silent resolution 5. **What about MRs that are closed then reopened?** - Todos stay resolved (they were addressed) - New assignment/review creates fresh todos - Recommendation: Document this behavior, no special handling --- ## References - **Issue:** https://gitlab.com/gitlab-org/gitlab/-/issues/20637 - **Related Issue:** https://gitlab.com/gitlab-org/gitlab-ce/issues/30521 - **Todo Model:** [app/models/todo.rb](app/models/todo.rb) - **TodoService:** [app/services/todo_service.rb](app/services/todo_service.rb) - **MR Close Service:** [app/services/merge_requests/close_service.rb](app/services/merge_requests/close_service.rb) - **MR Post-Merge Service:** [app/services/merge_requests/post_merge_service.rb](app/services/merge_requests/post_merge_service.rb)