Address review comments: URL mismatch tests, multi-meta warning, 60s timeout

deepu-mungamuri94 · cursoragent · deepu-mungamuri94 · commit da39c4c99c4c · 2026-02-18T12:04:06.000+05:30
- Add comprehensive tests for devServerUrl vs actualDevServerUrl mismatch
  combinations (explicit URL match/mismatch, manifest, skipDevServer cases)
- Warn when multiple .webapplication-meta.xml files found in directory;
  use first match for backward compatibility
- Increase dev server startup timeout from 30 to 60 seconds to align with
  VS Code extension and support slower dev server startups

Co-authored-by: Cursor &lt;cursoragent@cursor.com&gt;
diff --git a/src/commands/webapp/dev.ts b/src/commands/webapp/dev.ts
@@ -286,6 +286,7 @@ export default class WebappDev extends SfCommand<WebAppDevResult> {
           this.devServerManager = new DevServerManager({
             command: devCommand,
             cwd: webappDir,
+            startupTimeout: 60_000, // 60 seconds - aligned with VS Code extension
           });
 
           // Setup dev server event handlers
@@ -314,13 +315,13 @@ export default class WebappDev extends SfCommand<WebAppDevResult> {
           const actualDevServerUrl = await new Promise<string>((resolve, reject) => {
             const timeout = setTimeout(() => {
               reject(
-                new SfError('❌ Dev server did not start within 30 seconds.', 'DevServerTimeoutError', [
+                new SfError('❌ Dev server did not start within 60 seconds.', 'DevServerTimeoutError', [
                   'The dev server may be taking longer than expected to start',
                   'Check if the dev server command is correct in webapplication.json',
                   'Try running the dev server command manually to see if it starts',
                 ])
               );
-            }, 30_000);
+            }, 60_000);
 
             this.devServerManager?.on('ready', (url: string) => {
               clearTimeout(timeout);
diff --git a/src/config/webappDiscovery.ts b/src/config/webappDiscovery.ts
@@ -16,9 +16,11 @@
 
 import { access, readdir, readFile } from 'node:fs/promises';
 import { basename, dirname, join, relative } from 'node:path';
-import { SfError, SfProject } from '@salesforce/core';
+import { Logger, SfError, SfProject } from '@salesforce/core';
 import type { WebAppManifest } from './manifest.js';
 
+const logger = Logger.childFromRoot('WebappDiscovery');
+
 /**
  * Default command to run when no webapplication.json manifest is found
  */
@@ -76,18 +78,32 @@ function isWebapplicationsFolder(folderName: string): boolean {
 
 /**
  * Check if a directory contains a {name}.webapplication-meta.xml file
- * Returns the webapp name extracted from the filename, or null if not found
+ * Returns the webapp name extracted from the filename, or null if not found.
+ * Logs a warning if multiple metadata files are found (uses first match).
  */
 async function findWebappMetaXml(dirPath: string): Promise<string | null> {
   try {
     const entries = await readdir(dirPath);
+    const matches: string[] = [];
+
     for (const entry of entries) {
       const match = WEBAPP_META_XML_PATTERN.exec(entry);
       if (match) {
-        return match[1]; // Return the webapp name from the filename
+        matches.push(match[1]);
       }
     }
-    return null;
+
+    if (matches.length === 0) {
+      return null;
+    }
+
+    if (matches.length > 1) {
+      logger.warn(
+        `Multiple .webapplication-meta.xml files found in ${dirPath}: ${matches.join(', ')}. Using "${matches[0]}".`
+      );
+    }
+
+    return matches[0];
   } catch {
     return null;
   }
diff --git a/test/commands/webapp/dev.test.ts b/test/commands/webapp/dev.test.ts
@@ -258,4 +258,156 @@ describe('webapp:dev command integration', () => {
       expect(manifest.dev?.command).to.not.be.empty;
     });
   });
+
+  /**
+   * Tests for devServerUrl vs actualDevServerUrl mismatch handling.
+   * Mirrors the logic in dev.ts (lines 335-342) to enumerate all combinations.
+   *
+   * Combinations:
+   * - explicitUrlProvided: --url flag was passed
+   * - skipDevServer: --url was reachable, so we never start dev server
+   * - actualDevServerUrl: URL from DevServerManager 'ready' event (only when we start dev server)
+   */
+  describe('Dev Server URL Mismatch', () => {
+    /**
+     * Helper that mirrors the mismatch check logic from dev.ts:
+     * if (explicitUrlProvided && flags.url && flags.url !== actualDevServerUrl) { this.warn(...) }
+     */
+    function shouldWarnUrlMismatch(
+      explicitUrlProvided: boolean,
+      flagsUrl: string | undefined,
+      actualDevServerUrl: string
+    ): boolean {
+      return !!(explicitUrlProvided && flagsUrl && flagsUrl !== actualDevServerUrl);
+    }
+
+    /**
+     * Helper that mirrors the final devServerUrl assignment when dev server is started:
+     * devServerUrl = actualDevServerUrl
+     */
+    function getFinalDevServerUrlWhenStarted(actualDevServerUrl: string): string {
+      return actualDevServerUrl;
+    }
+
+    describe('when --url is provided and dev server is started (explicitUrlProvided=true)', () => {
+      it('should NOT warn when flags.url matches actualDevServerUrl', () => {
+        const flagsUrl = 'http://localhost:5173';
+        const actualDevServerUrl = 'http://localhost:5173';
+
+        expect(shouldWarnUrlMismatch(true, flagsUrl, actualDevServerUrl)).to.be.false;
+        expect(getFinalDevServerUrlWhenStarted(actualDevServerUrl)).to.equal(flagsUrl);
+      });
+
+      it('should warn when flags.url differs from actualDevServerUrl (different port)', () => {
+        const flagsUrl = 'http://localhost:3000';
+        const actualDevServerUrl = 'http://localhost:5173';
+
+        expect(shouldWarnUrlMismatch(true, flagsUrl, actualDevServerUrl)).to.be.true;
+        expect(getFinalDevServerUrlWhenStarted(actualDevServerUrl)).to.equal(actualDevServerUrl);
+      });
+
+      it('should warn when flags.url differs from actualDevServerUrl (localhost vs 127.0.0.1)', () => {
+        const flagsUrl = 'http://localhost:5173';
+        const actualDevServerUrl = 'http://127.0.0.1:5173';
+
+        expect(shouldWarnUrlMismatch(true, flagsUrl, actualDevServerUrl)).to.be.true;
+        expect(getFinalDevServerUrlWhenStarted(actualDevServerUrl)).to.equal(actualDevServerUrl);
+      });
+
+      it('should warn when flags.url differs from actualDevServerUrl (trailing slash)', () => {
+        const flagsUrl = 'http://localhost:5173';
+        const actualDevServerUrl = 'http://localhost:5173/';
+
+        expect(shouldWarnUrlMismatch(true, flagsUrl, actualDevServerUrl)).to.be.true;
+        expect(getFinalDevServerUrlWhenStarted(actualDevServerUrl)).to.equal(actualDevServerUrl);
+      });
+
+      it('should always use actualDevServerUrl as final devServerUrl when dev server is started', () => {
+        const actualDevServerUrl = 'http://localhost:8080';
+        expect(getFinalDevServerUrlWhenStarted(actualDevServerUrl)).to.equal(actualDevServerUrl);
+      });
+    });
+
+    describe('when --url is NOT provided (explicitUrlProvided=false)', () => {
+      it('should NOT warn - no mismatch check when no explicit URL', () => {
+        const actualDevServerUrl = 'http://localhost:5173';
+
+        expect(shouldWarnUrlMismatch(false, undefined, actualDevServerUrl)).to.be.false;
+        expect(getFinalDevServerUrlWhenStarted(actualDevServerUrl)).to.equal(actualDevServerUrl);
+      });
+
+      it('should use actualDevServerUrl from dev server (manifest has dev.command)', () => {
+        const actualDevServerUrl = 'http://localhost:3000';
+        expect(getFinalDevServerUrlWhenStarted(actualDevServerUrl)).to.equal(actualDevServerUrl);
+      });
+    });
+
+    describe('when --url is reachable (skipDevServer=true)', () => {
+      it('should use flags.url as devServerUrl - no actualDevServerUrl, no mismatch possible', () => {
+        const flagsUrl = 'http://localhost:5173';
+        // When skipDevServer, we never get actualDevServerUrl - devServerUrl stays as flags.url
+        expect(flagsUrl).to.equal('http://localhost:5173');
+      });
+    });
+
+    describe('when manifest has dev.url and no --url (skipDevServer=false, no dev server start)', () => {
+      it('should use manifest.dev.url as devServerUrl - no actualDevServerUrl', () => {
+        const manifestUrl = 'http://localhost:5173';
+        // When manifest.dev.url && !explicitUrlProvided, we use manifest url, don't start dev server
+        expect(manifestUrl).to.equal('http://localhost:5173');
+      });
+    });
+
+    describe('combination matrix: explicitUrlProvided × match/mismatch', () => {
+      const combinations: Array<{
+        explicitUrlProvided: boolean;
+        flagsUrl: string | undefined;
+        actualDevServerUrl: string;
+        expectMismatch: boolean;
+        description: string;
+      }> = [
+        {
+          explicitUrlProvided: true,
+          flagsUrl: 'http://localhost:5173',
+          actualDevServerUrl: 'http://localhost:5173',
+          expectMismatch: false,
+          description: 'explicit URL matches actual',
+        },
+        {
+          explicitUrlProvided: true,
+          flagsUrl: 'http://localhost:3000',
+          actualDevServerUrl: 'http://localhost:5173',
+          expectMismatch: true,
+          description: 'explicit URL different port',
+        },
+        {
+          explicitUrlProvided: true,
+          flagsUrl: 'http://127.0.0.1:5173',
+          actualDevServerUrl: 'http://localhost:5173',
+          expectMismatch: true,
+          description: 'explicit URL different host',
+        },
+        {
+          explicitUrlProvided: true,
+          flagsUrl: 'https://localhost:5173',
+          actualDevServerUrl: 'http://localhost:5173',
+          expectMismatch: true,
+          description: 'explicit URL different protocol',
+        },
+        {
+          explicitUrlProvided: false,
+          flagsUrl: undefined,
+          actualDevServerUrl: 'http://localhost:5173',
+          expectMismatch: false,
+          description: 'no explicit URL - no mismatch check',
+        },
+      ];
+
+      combinations.forEach(({ explicitUrlProvided, flagsUrl, actualDevServerUrl, expectMismatch, description }) => {
+        it(`should ${expectMismatch ? 'warn' : 'not warn'} when ${description}`, () => {
+          expect(shouldWarnUrlMismatch(explicitUrlProvided, flagsUrl, actualDevServerUrl)).to.equal(expectMismatch);
+        });
+      });
+    });
+  });
 });
diff --git a/test/config/webappDiscovery.test.ts b/test/config/webappDiscovery.test.ts
@@ -285,5 +285,21 @@ describe('webappDiscovery', () => {
       expect(result.webapp?.name).to.equal('standalone-app');
       expect(result.allWebapps).to.have.length(1);
     });
+
+    it('should warn and use first match when directory has multiple .webapplication-meta.xml files', async () => {
+      setupSfdxProject();
+
+      // Create webapp directory with multiple metadata files (misconfiguration)
+      const multiMetaPath = join(sfdxWebappsPath, 'multi-meta-app');
+      mkdirSync(multiMetaPath, { recursive: true });
+      writeFileSync(join(multiMetaPath, 'alpha.webapplication-meta.xml'), '<WebApplication/>');
+      writeFileSync(join(multiMetaPath, 'beta.webapplication-meta.xml'), '<WebApplication/>');
+
+      const result = await discoverWebapp(undefined, testDir);
+
+      // Discovery should succeed - uses first match (order depends on readdir)
+      expect(result.allWebapps).to.have.length(1);
+      expect(['alpha', 'beta']).to.include(result.allWebapps[0].name);
+    });
   });
 });
