Fix node-module-error plugin matching identifiers in multi-line comments (#1554)

* Fix node-module-error plugin matching identifiers in multi-line comments

The findIdentifierUsage function only stripped single-line comments and
same-line block comments, but didn't track multi-line block comment state.
Lines inside JSDoc/block comments (e.g. ` * ... Writable stream`) passed
through unstripped, causing the plugin to point at comments instead of
actual code usage.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Strip string literals before scanning for comment delimiters

Move string stripping before comment detection so that comment delimiters
inside string literals (e.g. `const s = "/*"`) don't incorrectly trigger
block comment mode. Adds regression test.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Peter Wielander <mittgfu@gmail.com>

Author: 3 people

.changeset/fix-multiline-comment-skip.md

@@ -0,0 +1,5 @@
+---
+'@workflow/builders': patch
+---
+
+Fix node-module-error plugin pointing at multi-line comments instead of code usage

packages/builders/src/node-module-esbuild-plugin.test.ts

@@ -378,6 +378,62 @@ describe('workflow-node-module-error plugin', () => {
     }
   });
 
+  it('should not point to JSDoc comments when finding usage', async () => {
+    const testCode = `
+      import { Writable } from "stream";
+      /**
+       * Convert a Web WritableStream<string> into a Node.js Writable stream
+       */
+      export function workflow() {
+        return new Writable();
+      }
+    `;
+
+    const { failure } = await buildWorkflowWithViolation(testCode);
+
+    expect(failure.errors).toHaveLength(1);
+    const violation = failure.errors[0];
+    expect(violation.text).toContain('"stream" which is a Node.js module');
+    // Should point to the actual code usage, not the JSDoc comment
+    expect(violation.location?.lineText).toContain('new Writable()');
+    expect(violation.location?.lineText).not.toContain('*');
+  });
+
+  it('should not point to single-line block comments when finding usage', async () => {
+    const testCode = `
+      import { Writable } from "stream";
+      /* Writable is used below */
+      export function workflow() {
+        return new Writable();
+      }
+    `;
+
+    const { failure } = await buildWorkflowWithViolation(testCode);
+
+    expect(failure.errors).toHaveLength(1);
+    const violation = failure.errors[0];
+    // Should point to the actual code usage, not the comment
+    expect(violation.location?.lineText).toContain('new Writable()');
+  });
+
+  it('should not treat comment delimiters inside strings as comments', async () => {
+    const testCode = `
+      import { Writable } from "stream";
+      const pattern = "/* Writable */";
+      export function workflow() {
+        return new Writable();
+      }
+    `;
+
+    const { failure } = await buildWorkflowWithViolation(testCode);
+
+    expect(failure.errors).toHaveLength(1);
+    const violation = failure.errors[0];
+    expect(violation.text).toContain('"stream" which is a Node.js module');
+    // Should point to actual code, not the string containing "Writable"
+    expect(violation.location?.lineText).toContain('new Writable()');
+  });
+
   it('should error on Bun module imports', async () => {
     const testCode = `
       import { serve } from "bun";

packages/builders/src/node-module-esbuild-plugin.ts

@@ -159,28 +159,59 @@ function findIdentifierUsage(
   identifier: string
 ) {
   const usageRegex = new RegExp(`\\b${escapeRegExp(identifier)}\\b`);
+  let inBlockComment = false;
 
   for (let i = startIndex; i < lines.length; i += 1) {
-    const line = lines[i];
-
-    // Skip comments (both // and /* */ style)
-    const withoutComments = line
-      .replace(/\/\*[\s\S]*?\*\//g, '')
-      .replace(/\/\/.*$/, '');
+    // Strip string literals first so that comment delimiters inside strings
+    // (e.g. `const s = "/*";`) don't confuse the comment scanner.
+    const stringsStripped = lines[i]
+      .replace(/'(?:[^'\\]|\\.)*'/g, (s) => ' '.repeat(s.length))
+      .replace(/"(?:[^"\\]|\\.)*"/g, (s) => ' '.repeat(s.length))
+      .replace(/`(?:[^`\\]|\\.)*`/g, (s) => ' '.repeat(s.length));
+
+    let line = stringsStripped;
+
+    // Handle multi-line block comments (including JSDoc)
+    if (inBlockComment) {
+      const endIdx = line.indexOf('*/');
+      if (endIdx === -1) {
+        continue;
+      }
+      line = ' '.repeat(endIdx + 2) + line.slice(endIdx + 2);
+      inBlockComment = false;
+    }
 
-    // Remove (replace with spaces) string literals to avoid matching inside paths
-    // Use escaped quote handling to properly match strings with escaped quotes
-    const withoutStrings = withoutComments
-      .replace(/'(?:[^'\\]|\\.)*'/g, (segment) => ' '.repeat(segment.length))
-      .replace(/"(?:[^"\\]|\\.)*"/g, (segment) => ' '.repeat(segment.length))
-      .replace(/`(?:[^`\\]|\\.)*`/g, (segment) => ' '.repeat(segment.length));
+    // Scan for comments, replacing them with spaces
+    let processed = '';
+    let j = 0;
+    while (j < line.length) {
+      if (line[j] === '/' && line[j + 1] === '/') {
+        processed += ' '.repeat(line.length - j);
+        break;
+      }
+      if (line[j] === '/' && line[j + 1] === '*') {
+        const endIdx = line.indexOf('*/', j + 2);
+        if (endIdx !== -1) {
+          const len = endIdx + 2 - j;
+          processed += ' '.repeat(len);
+          j = endIdx + 2;
+        } else {
+          processed += ' '.repeat(line.length - j);
+          inBlockComment = true;
+          break;
+        }
+      } else {
+        processed += line[j];
+        j += 1;
+      }
+    }
 
-    const match = withoutStrings.match(usageRegex);
+    const match = processed.match(usageRegex);
     if (match && match.index !== undefined) {
       return {
         line: i,
         column: match.index,
-        lineText: line,
+        lineText: lines[i],
       };
     }
   }