Skip to content

Conversation

@marleystipich2
Copy link
Collaborator

@marleystipich2 marleystipich2 commented Nov 24, 2025

…utionsNeedReboot

Summary by Sourcery

Filter and process only valid remediation issues when resolving whether resolutions need a reboot.

Bug Fixes:

  • Avoid processing null, undefined, or malformed remediations when determining reboot requirements for issues.

Enhancements:

  • Refactor reboot resolution logic to pre-filter valid remediations and streamline issue processing.

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Nov 24, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Refactors resolveResolutionsNeedReboot to ignore invalid remediation objects, pre-filter issues before processing, and streamline the reboot-resolution mapping logic while preserving behavior of encoding reboot status into the resolution field.

Sequence diagram for resolveResolutionsNeedReboot processing valid issues

sequenceDiagram
    participant Caller
    participant resolveResolutionsNeedReboot
    participant Lodash
    participant Identifiers
    participant Issues
    participant ResolutionResolver
    participant PromiseAll

    Caller->>resolveResolutionsNeedReboot: call with remediations
    activate resolveResolutionsNeedReboot

    resolveResolutionsNeedReboot->>resolveResolutionsNeedReboot: filter invalid remediations
    resolveResolutionsNeedReboot->>Lodash: flatMap issues from validRemediations
    Lodash-->>resolveResolutionsNeedReboot: allIssues array

    resolveResolutionsNeedReboot->>PromiseAll: map allIssues to async tasks
    activate PromiseAll

    loop for each issue in allIssues
        PromiseAll->>Identifiers: parse(issue.issue_id)
        Identifiers-->>PromiseAll: id

        PromiseAll->>Issues: getHandler(id)
        Issues-->>PromiseAll: handler

        PromiseAll->>ResolutionResolver: handler.getResolutionResolver()
        ResolutionResolver-->>PromiseAll: resolver

        PromiseAll->>ResolutionResolver: isRebootNeeded(id, issue.resolution)
        ResolutionResolver-->>PromiseAll: needsReboot (boolean or null)

        alt needsReboot is not null
            PromiseAll->>PromiseAll: issue.resolution = { needsReboot }
        else needsReboot is null
            PromiseAll->>PromiseAll: issue.resolution = false
        end
    end

    PromiseAll-->>resolveResolutionsNeedReboot: all async tasks resolved
    deactivate PromiseAll

    resolveResolutionsNeedReboot-->>Caller: resolved Promise
    deactivate resolveResolutionsNeedReboot
Loading

File-Level Changes

Change Details Files
Harden and refactor resolveResolutionsNeedReboot to filter invalid remediations and streamline issue processing.
  • Introduce a validRemediations array that filters out null/undefined remediations and those without an issues array.
  • Build an allIssues collection by flat-mapping validRemediations.issues and filtering out issues missing issue_id or resolution.
  • Replace the chained lodash/Promise workflow with P.all over allIssues.map to resolve reboot status for each issue asynchronously.
  • Simplify the reboot resolution assignment so issue.resolution is set to { needsReboot } when non-null, otherwise to false, maintaining the unconventional use of the resolution field.
src/remediations/controller.read.js

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes - here's some feedback:

  • By filtering out invalid remediations instead of logging them, the function now silently ignores bad inputs; if this change is intentional, consider at least a single aggregated log or metric to preserve observability of malformed remediations.
  • Since issue.resolution is being overloaded from its original string meaning into either a boolean or an object, consider extracting this into a small helper (e.g., setRebootResolution(issue, needsReboot)) to centralize and clarify this unconventional usage and make future refactors easier.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- By filtering out invalid remediations instead of logging them, the function now silently ignores bad inputs; if this change is intentional, consider at least a single aggregated log or metric to preserve observability of malformed remediations.
- Since `issue.resolution` is being overloaded from its original string meaning into either a boolean or an object, consider extracting this into a small helper (e.g., `setRebootResolution(issue, needsReboot)`) to centralize and clarify this unconventional usage and make future refactors easier.

## Individual Comments

### Comment 1
<location> `src/remediations/controller.read.js:49-53` </location>
<code_context>
-            log.warn({ index: i, remediation: r }, "resolveResolutionsNeedReboot: remediation is missing 'issues' array");
-        }
-    });
+    // Filter out invalid remediations (null/undefined or missing issues array)
+    const validRemediations = remediations.filter(r => r && Array.isArray(r.issues));
+    
+    // Collect all issues from all remediations
</code_context>

<issue_to_address>
**suggestion:** Consider retaining some observability when remediations are filtered out as invalid.

Previously, invalid remediations produced warnings that helped detect upstream data issues. Now they’re silently dropped. If this behavior is intentional, consider logging when `remediations.length !== validRemediations.length` (or at least the number filtered out) so unexpected input shapes remain visible without cluttering the main logic.

```suggestion
function resolveResolutionsNeedReboot (...remediations) {
    // Filter out invalid remediations (null/undefined or missing issues array)
    const validRemediations = remediations.filter(r => r && Array.isArray(r.issues));

    // Retain observability: log when some remediations are filtered out as invalid
    if (remediations.length !== validRemediations.length) {
        const filteredOut = remediations.length - validRemediations.length;

        log.warn(
            {
                totalRemediations: remediations.length,
                validRemediations: validRemediations.length,
                filteredOut
            },
            'resolveResolutionsNeedReboot: filtered out invalid remediations (null/undefined or missing issues array)'
        );
    }

    // Collect all issues from all remediations
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant