Skip to content

feat: no security issues should show up as green progress bar#109

Merged
lirantal merged 1 commit intomainfrom
fix/no-security-issues-should-be-green
Oct 1, 2025
Merged

feat: no security issues should show up as green progress bar#109
lirantal merged 1 commit intomainfrom
fix/no-security-issues-should-be-green

Conversation

@lirantal
Copy link
Copy Markdown
Owner

@lirantal lirantal commented Oct 1, 2025

User description

Description

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Checklist:

  • I have updated the documentation (if required).
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I added a picture of a cute animal cause it's fun

PR Type

Enhancement


Description

  • Modified progress bar styling for security issues

  • Added emptyIsGood parameter to progress bar function

  • Zero security/version issues now display green styling

  • Added comprehensive test coverage for zero-issue scenarios


Diagram Walkthrough

flowchart LR
  A["Security Issues Count"] --> B{"Count = 0?"}
  B -->|Yes| C["Green Progress Bar"]
  B -->|No| D["Red Progress Bar"]
  E["Version Issues Count"] --> F{"Count = 0?"}
  F -->|Yes| G["Green Progress Bar"]
  F -->|No| H["Red Progress Bar"]
Loading

File Walkthrough

Relevant files
Enhancement
summary.ts
Enhanced progress bar styling logic                                           

src/components/summary.ts

  • Added emptyIsGood parameter to createProgressBar function
  • Modified security and version bars to use green styling when count is
    zero
  • Updated function calls to pass emptyIsGood=true for security/version
    bars
+10/-5   
Tests
summary-component.test.ts
Added test for zero-issue scenarios                                           

tests/summary-component.test.ts

  • Added new test case for zero security and version issues
  • Verifies green progress bar behavior when counts are zero
  • Includes detailed comments explaining the expected behavior
+27/-0   

@qodo-code-review
Copy link
Copy Markdown

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.94%. Comparing base (bc64f95) to head (b241dc7).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #109      +/-   ##
==========================================
+ Coverage   80.90%   80.94%   +0.03%     
==========================================
  Files          21       21              
  Lines        2587     2592       +5     
  Branches      295      297       +2     
==========================================
+ Hits         2093     2098       +5     
  Misses        488      488              
  Partials        6        6              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Oct 1, 2025

Open in StackBlitz

npm i https://pkg.pr.new/lirantal/ls-mcp@109

commit: b241dc7

@qodo-code-review
Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Simplify logic by determining color externally

Refactor the code to determine the progress bar color within the
SummaryComponent rather than inside createProgressBar. This simplifies
createProgressBar into a purely presentational component by removing the complex
emptyIsGood logic.

Examples:

src/components/summary.ts [20-62]
  const securityBar = createProgressBar(highRiskCredentials, totalServers, 20, 'red', true)
  const versionBar = createProgressBar(implicitLatestVersions, totalServers, 20, 'red', true)

  // Format transport breakdown
  const transportText = `stdio: ${transportBreakdown.stdio} | SSE: ${transportBreakdown.sse} | HTTP: ${transportBreakdown.http}`

  // Build the summary output
  const lines = [
    'SUMMARY',
    `      SERVERS     ${runningBar} ${runningServers} / ${totalServers} Running`,

 ... (clipped 33 lines)

Solution Walkthrough:

Before:

function SummaryComponent(stats) {
  // ...
  const securityBar = createProgressBar(stats.highRiskCredentials, ..., 'red', true);
  // ...
}

function createProgressBar(count, total, width, color, emptyIsGood = false) {
  // ...
  if (color === 'red') {
    // ...
    if (count === 0 && emptyIsGood) {
      empty = styleText(['green'], ...); // Special case logic
    } else {
      empty = styleText(['red'], ...);
    }
  }
  // ...
}

After:

function SummaryComponent(stats) {
  // ...
  const securityColor = stats.highRiskCredentials === 0 ? 'green' : 'red';
  const securityBar = createProgressBar(stats.highRiskCredentials, ..., securityColor);
  // ...
}

function createProgressBar(count, total, width, color) {
  // ...
  if (color === 'red') {
    filled = styleText(['redBright'], ...);
    empty = styleText(['red'], ...);
  } else { // green
    filled = styleText(['greenBright'], ...);
    empty = styleText(['green'], ...);
  }
  // ...
}
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a design issue where business logic is mixed into a presentational function, and proposes a cleaner alternative that improves separation of concerns and code clarity.

Medium
General
Refactor progress bar color logic

Refactor the createProgressBar function to handle the count === 0 && emptyIsGood
case with an early return to simplify the color selection logic.

src/components/summary.ts [48-62]

-let filled: string
-let empty: string
-
-if (color === 'green') {
-  filled = styleText(['greenBright'], '█'.repeat(filledWidth))
-  empty = styleText(['green'], '░'.repeat(emptyWidth))
-} else { // red
-  filled = styleText(['redBright'], '█'.repeat(filledWidth))
-  // If count is 0 and emptyIsGood is true, use green for empty sections (0 issues is good)
-  if (count === 0 && emptyIsGood) {
-    empty = styleText(['green'], '░'.repeat(emptyWidth))
-  } else {
-    empty = styleText(['red'], '░'.repeat(emptyWidth))
-  }
+if (count === 0 && emptyIsGood) {
+  // When there are no issues, the bar should be fully green.
+  return styleText(['green'], '░'.repeat(width))
 }
 
+const filled = styleText(
+  color === 'green' ? ['greenBright'] : ['redBright'],
+  '█'.repeat(filledWidth)
+)
+const empty = styleText(
+  color === 'green' ? ['green'] : ['red'],
+  '░'.repeat(emptyWidth)
+)
+

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies a way to refactor the code for better readability and maintainability by handling a special case with an early return, which simplifies the main logic path.

Low
  • More

@lirantal lirantal merged commit 0c94ebd into main Oct 1, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants