Fix line explosion on Windows PowerShell

packages/safe-chain/src/shell-integration/helpers.js

@@ -18,15 +18,15 @@ export const knownAikidoTools = [
  * Example: "npm, npx, yarn, pnpm, and pnpx commands"
  */
 export function getPackageManagerList() {
-  const tools = knownAikidoTools.map(t => t.tool);
+  const tools = knownAikidoTools.map((t) => t.tool);
   if (tools.length <= 1) {
-    return `${tools[0] || ''} commands`;
+    return `${tools[0] || ""} commands`;
   }
   if (tools.length === 2) {
     return `${tools[0]} and ${tools[1]} commands`;
   }
   const lastTool = tools.pop();
-  return `${tools.join(', ')}, and ${lastTool} commands`;
+  return `${tools.join(", ")}, and ${lastTool} commands`;
 }
 
 export function doesExecutableExistOnSystem(executableName) {
@@ -47,7 +47,7 @@ export function removeLinesMatchingPattern(filePath, pattern, eol) {
   eol = eol || os.EOL;
 
   const fileContent = fs.readFileSync(filePath, "utf-8");
-  const lines = fileContent.split(/[\r\n\u2028\u2029]/);
+  const lines = fileContent.split(/\r?\n|\r|\u2028|\u2029/);
   const updatedLines = lines.filter((line) => !shouldRemoveLine(line, pattern));
   fs.writeFileSync(filePath, updatedLines.join(eol), "utf-8");
 }

packages/safe-chain/src/shell-integration/helpers.spec.js

@@ -16,8 +16,8 @@ describe("removeLinesMatchingPatternTests", () => {
       namedExports: {
         EOL: "\r\n", // Simulate Windows line endings
         tmpdir: tmpdir,
-        platform: () => "linux"
+        platform: () => "linux",
-      }
+      },
     });
   });
 
@@ -31,54 +31,59 @@ describe("removeLinesMatchingPatternTests", () => {
     mock.reset();
   });
 
-
   it("should handle mixed line endings without wiping entire file", async () => {
     // Import helpers after setting up the mock
     const { removeLinesMatchingPattern } = await import("./helpers.js");
-    
+
     // Create a file with Unix line endings but os.EOL expects Windows
     const fileContent = [
       "# keep this line",
-      "alias npm='remove-this'", 
+      "alias npm='remove-this'",
       "# keep this line too",
       "alias yarn='remove-this-too'",
-      "# final line to keep"
+      "# final line to keep",
     ].join("\n"); // File has Unix line endings
-    
+
     fs.writeFileSync(testFile, fileContent, "utf-8");
-    
+
     // Try to remove lines containing 'alias'
     const pattern = /alias.*=/;
     removeLinesMatchingPattern(testFile, pattern);
-    
+
     const result = fs.readFileSync(testFile, "utf-8");
-    
+
     // This test will fail because the function splits on '\r\n' but file uses '\n'
     // So it treats the entire content as one line and if any part matches, removes everything
-    assert.ok(result.includes("keep this line"), "Should preserve non-matching lines");
+    assert.ok(
-    assert.ok(result.includes("final line to keep"), "Should preserve final line");
+      result.includes("keep this line"),
+      "Should preserve non-matching lines"
+    );
+    assert.ok(
+      result.includes("final line to keep"),
+      "Should preserve final line"
+    );
   });
 
   it("should handle mixed line endings with short matching content", async () => {
     // Import helpers after setting up the mock
     const { removeLinesMatchingPattern } = await import("./helpers.js");
-    
+
-    // Create a file with Unix line endings, but make the entire content short 
+    // Create a file with Unix line endings, but make the entire content short
     // to bypass the maxLineLength protection
     const fileContent = [
       "# keep1",
       "alias x=y", // Short alias line that should be removed
-      "# keep2"
+      "# keep2",
     ].join("\n"); // File has Unix line endings, total length < 100 chars
-    
+
     fs.writeFileSync(testFile, fileContent, "utf-8");
-    
+
     // Try to remove lines containing 'alias'
     const pattern = /alias/;
     removeLinesMatchingPattern(testFile, pattern);
-    
+
     const result = fs.readFileSync(testFile, "utf-8");
-    
+
     // This should now be protected by the newline detection
     assert.ok(result.includes("keep1"), "Should preserve first line");
     assert.ok(result.includes("keep2"), "Should preserve third line");
@@ -87,27 +92,93 @@ describe("removeLinesMatchingPatternTests", () => {
   it("should handle Unicode line separators that bypass newline detection", async () => {
     // Import helpers after setting up the mock
     const { removeLinesMatchingPattern } = await import("./helpers.js");
-    
+
     // Use Unicode line separator (U+2028) and paragraph separator (U+2029)
     // These are considered line breaks but aren't \n or \r
-    const fileContent = [
+    const fileContent = ["keep this", "alias test=value", "keep that"].join(
-      "keep this",
+      "\u2028"
-      "alias test=value",
+    ); // Unicode line separator
-      "keep that"
+
-    ].join("\u2028"); // Unicode line separator
-    
     fs.writeFileSync(testFile, fileContent, "utf-8");
-    
+
     // Try to remove lines containing 'alias'
     const pattern = /alias/;
     removeLinesMatchingPattern(testFile, pattern);
-    
+
     const result = fs.readFileSync(testFile, "utf-8");
-    
+
     // This could still wipe everything if split() treats it as one line
     // but the content doesn't contain \n or \r so passes the newline check
     assert.ok(result.includes("keep this"), "Should preserve first part");
     assert.ok(result.includes("keep that"), "Should preserve last part");
   });
 
+  it("should handle Windows CRLF line endings without creating empty lines", async () => {
+    // Import helpers after setting up the mock
+    const { removeLinesMatchingPattern } = await import("./helpers.js");
+
+    // Create a file with Windows CRLF line endings
+    const fileContent = [
+      "# comment 1",
+      "alias npm='aikido-npm'",
+      "# comment 2",
+      "export PATH=$PATH:/usr/local/bin",
+      "",
+      "# comment 3",
+    ].join("\r\n"); // Windows line endings
+
+    fs.writeFileSync(testFile, fileContent, "utf-8");
+
+    // Try to remove lines containing 'alias'
+    const pattern = /alias/;
+    removeLinesMatchingPattern(testFile, pattern, "\r\n");
+
+    const result = fs.readFileSync(testFile, "utf-8");
+
+    // Should preserve non-matching lines without adding empty lines
+    assert.ok(result.includes("# comment 1"), "Should preserve first comment");
+    assert.ok(result.includes("# comment 2"), "Should preserve second comment");
+    assert.ok(result.includes("# comment 3"), "Should preserve third comment");
+    assert.ok(result.includes("export PATH"), "Should preserve export line");
+    assert.ok(!result.includes("alias npm"), "Should remove alias line");
+
+    // The key test: when we split on \r\n, we should get exactly 4 lines
+    // Bug: if split(/[\r\n]/) was used, it creates empty lines between each real line
+    // because \r\n becomes two separators, resulting in: ["# comment 1", "", "# comment 2", "", "export...", "", "# comment 3", ""]
+    const resultLines = result.split("\r\n");
+    assert.strictEqual(resultLines.length, 5, "Should have exactly 5 lines");
+  });
+
+  it("should not remove empty lines on unix line endings", async () => {
+    // Import helpers after setting up the mock
+    const { removeLinesMatchingPattern } = await import("./helpers.js");
+
+    // Create a file with Unix line endings and empty lines
+    const fileContent = [
+      "# comment 1",
+      "alias npm='aikido-npm'",
+      "# comment 2",
+      "export PATH=$PATH:/usr/local/bin",
+      "",
+      "# comment 3",
+    ].join("\n"); // Unix line endings
+
+    fs.writeFileSync(testFile, fileContent, "utf-8");
+
+    // Try to remove lines containing 'alias'
+    const pattern = /alias/;
+    removeLinesMatchingPattern(testFile, pattern, "\n");
+
+    const result = fs.readFileSync(testFile, "utf-8");
+
+    // Should preserve non-matching lines including empty lines
+    assert.ok(result.includes("# comment 1"), "Should preserve first comment");
+    assert.ok(result.includes("# comment 2"), "Should preserve second comment");
+    assert.ok(result.includes("# comment 3"), "Should preserve third comment");
+    assert.ok(result.includes("export PATH"), "Should preserve export line");
+    assert.ok(!result.includes("alias npm"), "Should remove alias line");
+
+    const resultLines = result.split("\n");
+    assert.strictEqual(resultLines.length, 5, "Should have exactly 5 lines");
+  });
 });