Add RequiredTaskHeader cop to enforce task annotation blocks in Rake files

[nagooen]

Jira Ticket

ES-49595

Why are we making this change? Why are we doing it this way?

Introducing a custom rubo-coprule, to help enforce the habit of labelling and spring cleaning rake tasks. Violations will trigger for new rake tasks that do not include the required headers.

A violation will also occur for rake tasks that have the header and the prescribed date has passed.

Valid Examples

Temporary Task (with cleanup date)

# Task: my_task
#   Type: one-off
#   Created: 2025-01-15
#   Ownership: @bettercaring/payments
#   Cleanup Card: ES-123
#   Cleanup Date: 2026-12-31
task :my_task do
end

Indefinite Task (without cleanup date)

# Task: my_task
#   Type: operational
#   Created: 2025-01-15
#   Ownership: @bettercaring/payments
task :my_task do
end

Operational Task with Cleanup Date

# Task: my_task
#   Type: operational
#   Created: 2025-01-15
#   Ownership: @bettercaring/payments
#   Cleanup Card: ES-456
#   Cleanup Date: 2026-12-31
task :my_task do
end

Namespaced Task

namespace :example do
  # Task: example:my_task
  #   Type: operational
  #   Created: 2025-01-15
  #   Ownership: @bettercaring/payments
  task :my_task do
  end
end

[Copilot]

Pull request overview

This PR introduces a custom RuboCop cop Mable/RequiredTaskHeader to enforce documentation standards for Rake tasks by requiring annotation blocks with metadata fields including Type, Created, Ownership, Cleanup Card, and Cleanup Date.

Key Changes:

• New custom cop with modular architecture (task extraction, annotation parsing, and validation)
• Comprehensive test coverage with 30+ test scenarios covering valid/invalid annotations
• Configuration added to default.yml with the cop enabled but non-blocking

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
lib/rubocop/cop/mable/required_task_header/required_task_header.rb	Main cop implementation with message constants and file exclusion logic
lib/rubocop/cop/mable/required_task_header/annotation_validator.rb	Validation logic for annotation fields including type-specific rules and date expiration checks
lib/rubocop/cop/mable/required_task_header/annotation_parser.rb	Parser for extracting annotation blocks and field values from comments
lib/rubocop/cop/mable/required_task_header/task_extractor.rb	AST traversal logic for finding tasks and building fully-qualified names with namespaces
lib/rubocop/cop/mable/required_task_header/validation_rules.rb	Configuration of validation rules for each required field
lib/rubocop/cop/mable_cops.rb	Registration of the new cop in the cops loader
spec/rubocop/cop/mable/required_task_header_spec.rb	Comprehensive test suite covering valid annotations, missing fields, invalid values, and edge cases
config/default.yml	Cop configuration with description and version information

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This error message is hardcoded inline rather than using a predefined constant from the Messages module. This is inconsistent with the pattern used elsewhere in the codebase where all other messages use constants like MSG_INVALID_CLEANUP_CARD, MSG_MISSING_CLEANUP_DATE, etc. Consider defining a new constant in the Messages module (e.g., MSG_ONE_OFF_OPERATIONAL_CLEANUP_DATE) for consistency and maintainability.

[Copilot]

These error messages are hardcoded inline rather than using predefined constants from the Messages module. This is inconsistent with the pattern used elsewhere in the codebase. Consider defining constants in the Messages module for these operational task validation messages to maintain consistency and make the messages easier to update and reuse.

[Copilot]

The cleanup date '2025-07-15' in this test has already passed (current date is December 17, 2025). This means the cop will likely trigger two offenses: one for the invalid cleanup card being 'Operational' for a one-off task, and another for the cleanup date having passed. However, the test only expects one offense. Consider using a future date (e.g., '2026-07-15') to avoid triggering the date expiration check, or update the test to expect both offenses.

[Copilot]

The example cleanup date '2025-07-15' is in the past (current date is December 17, 2025). Using a past date in documentation examples could be confusing since the cop validates that cleanup dates haven't passed. Consider updating to a future date (e.g., '2026-07-15') to provide a more accurate example that won't trigger the expiration validation.

[Copilot]

The example cleanup date '2025-07-15' is in the past (current date is December 17, 2025). Consider updating to a future date (e.g., '2026-07-15') to provide a more accurate example that won't trigger the expiration validation in the cop.

[Copilot]

The validation logic for operational tasks is incomplete. According to tests at lines 234-286 in the spec file, when an operational task provides one of cleanup_card or cleanup_date, it must provide both. However, this method only validates that provided values are valid - it doesn't check if one field is present without the other. Add logic to verify that if either cleanup_card or cleanup_date is provided (and not 'Operational'), then both must be provided.

[spheric]

AI Code Review

Complexity: MEDIUM | Verdict: CHANGES REQUESTED

Found 11 potential issues (0 critical, 3 high, 5 medium, 3 low)

Summary

This PR implements a well-structured RuboCop cop for enforcing task annotation metadata in Rake files. The code follows good separation of concerns by splitting functionality into focused modules (parser, validator, extractor, rules).

Key Issues Requiring Changes

1. Control Flow Logic (High): The validate_field method's boolean return values and the next if usage create confusing control flow that could lead to missed validations.

2. Duplicate Validation Logic (High): The "Operational" value handling for cleanup_date exists in both skip_validation? and the validator lambda, creating potential inconsistency.

3. File Loading Pattern (Medium): require_relative inside the class body is non-idiomatic and fragile.

Positive Aspects

• Clean module separation following Single Responsibility Principle
• Good use of frozen string literals
• Comprehensive message formatting with examples
• Proper namespace handling for nested rake tasks

---

🤖 Reviewed by AI Code Review Agent

[spheric]

🟠 High - Logic Bug

The validate_fields method iterates through VALIDATION_RULES but uses next if validate_field(...). The return value logic appears inverted - validate_field returns true when there's an error (missing field) and false otherwise. This means it skips to next field on error instead of continuing validation.

Suggested fix: Review the boolean logic. If check_missing_field returns true when field is missing and offense was added, the next if will skip remaining validation for that field - which may be intentional. Add comments to clarify intent, or restructure to make control flow clearer.

[spheric]

🟠 High - Logic Bug

The skip_validation? method returns true when field == :cleanup_date && value == 'Operational', but this logic is also duplicated in the validator lambda in validation_rules.rb. This creates confusing dual validation paths.

Suggested fix: Consolidate the 'Operational' value handling in one place. Either handle it entirely in the validator lambda or entirely in skip_validation?, not both.

[spheric]

🟠 High - Logic Bug

Off-by-one edge case: The loop (task_line - 1).downto(1) never processes line index 0 since it stops at 1. If a task is on line 1, the method returns nil prematurely at line 11.

Suggested fix: Add a guard for task_line == 1 case explicitly, or document this as expected behavior. Consider if tasks on line 2 with annotation on line 1 are handled correctly.

[spheric]

🟡 Medium - Code Quality

Using require_relative inside the class definition after Messages module is defined creates a tight coupling between file load order and class constant availability. This is fragile and non-idiomatic Ruby.

Suggested fix: Move all require_relative statements to the top of the file after frozen_string_literal. Define Messages in a separate file if needed, or restructure to avoid circular dependency issues.

[spheric]

🟡 Medium - Code Quality

The validate method receives cop as a parameter and passes it through a context hash. This creates tight coupling and makes the module difficult to test in isolation.

Suggested fix: Consider returning a collection of validation errors/offenses instead of directly calling cop.add_offense. This would make the validator testable in isolation and follow better separation of concerns.

[spheric]

🟡 Medium - Code Quality

The check cleanup_card.is_a?(String) && cleanup_card.strip.empty? is redundant since cleanup_card from the parsed annotation will always be a String (from match[1].strip in annotation_parser.rb). The strip is also redundant as values are already stripped during parsing.

Suggested fix: Simplify to cleanup_card.empty? since values are already stripped strings or nil.

[spheric]

🟡 Medium - Performance

processed_source.raw_source.lines is called once per task when collect_annotation_lines is invoked. For files with many tasks, this repeatedly allocates the same array of lines.

Suggested fix: Cache the lines array at the investigation level (in on_new_investigation) and pass it through, or use processed_source.lines if available.

[spheric]

🟡 Medium - Logic Bug

The check tasks.key?(full_task_name) prevents duplicate task names from being processed, but silently ignores duplicates. If a file has duplicate task definitions (which is likely a bug), users won't be warned.

Suggested fix: Consider adding a warning or processing all tasks with the same name, not just the first one found.

[spheric]

🔵 Low - Code Quality

The regex pattern ^#\\s+ requires one or more spaces after # for Type, Created, etc., but ^#\\s* allows zero or more for Task. This inconsistency could cause valid annotations to not be parsed if indentation varies.

Suggested fix: Consider using consistent patterns or documenting the required format clearly.

[spheric]

🔵 Low - Code Quality

File is missing newline at end of file.

Suggested fix: Add a newline at the end of the file to follow POSIX conventions and avoid diff noise.