refactor(core): deprecate testId fallback and harden reportFileName (#2264)

* refactor(core): deprecate testId fallback and harden reportFileName

* fix(cli): align CDP tests with reportFileName migration

* fix(core): ci

---------

Co-authored-by: ottomao <ottomao@gmail.com>

Author: yuyutaotao

packages/cli/src/create-yaml-player.ts

@@ -43,30 +43,31 @@ export const launchServer = async (
 };
 
 /**
- * Resolves the testId with proper priority handling.
- * Priority: CLI testId > YAML testId > fileName
+ * Resolves reportFileName with proper priority handling.
+ * Priority: YAML reportFileName > CLI testId (legacy) > YAML testId (legacy) > fileName
  */
-function resolveTestId(
+function resolveReportFileName(
+  yamlReportFileName: string | undefined,
   cliTestId: string | undefined,
   yamlTestId: string | undefined,
   fileName: string,
 ): string {
-  return cliTestId ?? yamlTestId ?? fileName;
+  return yamlReportFileName ?? cliTestId ?? yamlTestId ?? fileName;
 }
 
 /**
- * Builds agent options by merging YAML agent config with processed cache and testId.
+ * Builds agent options by merging YAML agent config with processed cache and report name.
  * Handles the spread of agent options and ensures proper cache configuration.
  */
 function buildAgentOptions(
   yamlAgent: MidsceneYamlScriptAgentOpt | undefined,
-  preferenceTestId: string,
+  reportFileName: string,
   fileName: string,
 ): Partial<AgentOpt> {
   return {
     ...(yamlAgent || {}),
     cache: processCacheConfig(yamlAgent?.cache, fileName),
-    testId: preferenceTestId,
+    reportFileName,
   };
 }
 
@@ -92,8 +93,8 @@ export async function createYamlPlayer(
   const preference = {
     headed: options?.headed,
     keepWindow: options?.keepWindow,
-    // Priority: CLI testId > YAML testId > fileName
-    testId: resolveTestId(
+    reportFileName: resolveReportFileName(
+      clonedYamlScript.agent?.reportFileName,
       options?.testId,
       clonedYamlScript.agent?.testId,
       fileName,
@@ -194,7 +195,7 @@ export async function createYamlPlayer(
               ...preference,
               ...buildAgentOptions(
                 clonedYamlScript.agent,
-                preference.testId,
+                preference.reportFileName,
                 fileName,
               ),
             },
@@ -226,7 +227,7 @@ export async function createYamlPlayer(
               ...preference,
               ...buildAgentOptions(
                 clonedYamlScript.agent,
-                preference.testId,
+                preference.reportFileName,
                 fileName,
               ),
             },
@@ -262,7 +263,7 @@ export async function createYamlPlayer(
           closeConflictServer: true,
           ...buildAgentOptions(
             clonedYamlScript.agent,
-            preference.testId,
+            preference.reportFileName,
             fileName,
           ),
         });
@@ -295,7 +296,7 @@ export async function createYamlPlayer(
           ...androidTarget, // Pass all Android config options
           ...buildAgentOptions(
             clonedYamlScript.agent,
-            preference.testId,
+            preference.reportFileName,
             fileName,
           ),
         });
@@ -320,7 +321,7 @@ export async function createYamlPlayer(
           ...iosTarget, // Pass all iOS config options
           ...buildAgentOptions(
             clonedYamlScript.agent,
-            preference.testId,
+            preference.reportFileName,
             fileName,
           ),
         });
@@ -345,7 +346,7 @@ export async function createYamlPlayer(
           ...computerTarget,
           ...buildAgentOptions(
             clonedYamlScript.agent,
-            preference.testId,
+            preference.reportFileName,
             fileName,
           ),
         });
@@ -405,7 +406,7 @@ export async function createYamlPlayer(
           device,
           buildAgentOptions(
             clonedYamlScript.agent,
-            preference.testId,
+            preference.reportFileName,
             fileName,
           ),
         );

packages/cli/tests/unit-test/create-yaml-player.test.ts

@@ -717,11 +717,11 @@ describe('create-yaml-player', () => {
         await setupFnCallback();
       }
 
-      // Verify that when agent config is undefined, testId is still set from fileName
+      // Verify that when agent config is undefined, reportFileName is set from fileName
       // and aiActionContext is not present (undefined fields are not spread)
       const callArgs = getMockCallArg(vi.mocked(agentFromAdbDevice), 0, 1);
       expect(callArgs).toMatchObject({
-        testId: 'script',
+        reportFileName: 'script',
         deviceId: 'test-device',
       });
       expect(callArgs).not.toHaveProperty('aiActionContext');
@@ -754,11 +754,11 @@ describe('create-yaml-player', () => {
         await setupFnCallback();
       }
 
-      // Verify that when agent config is undefined, testId is still set from fileName
+      // Verify that when agent config is undefined, reportFileName is set from fileName
       // and aiActionContext is not present (undefined fields are not spread)
       const callArgs = getMockCallArg(vi.mocked(agentFromWebDriverAgent), 0, 0);
       expect(callArgs).toMatchObject({
-        testId: 'script',
+        reportFileName: 'script',
       });
       expect(callArgs).not.toHaveProperty('aiActionContext');
     });
@@ -772,7 +772,6 @@ describe('create-yaml-player', () => {
           url: 'test.html',
         },
         agent: {
-          testId: 'custom-test-id',
           groupName: 'Custom Group',
           groupDescription: 'Custom description',
           generateReport: true,
@@ -820,7 +819,6 @@ describe('create-yaml-player', () => {
           reportFileName: 'custom-report',
           replanningCycleLimit: 25,
           aiActionContext: 'Test context',
-          testId: 'custom-test-id', // YAML testId is used
         }),
         undefined, // browser
         undefined, // page
@@ -951,7 +949,7 @@ describe('create-yaml-player', () => {
       );
     });
 
-    test('should prioritize CLI preference testId over YAML testId', async () => {
+    test('should prioritize CLI legacy testId over YAML legacy testId for reportFileName', async () => {
       const mockScript: MidsceneYamlScript = {
         web: {
           serve: './test',
@@ -987,18 +985,18 @@ describe('create-yaml-player', () => {
         await setupFnCallback();
       }
 
-      // CLI testId should take priority
+      // CLI legacy testId should take priority for reportFileName
       expect(puppeteerAgentForTarget).toHaveBeenCalledWith(
         expect.any(Object),
         expect.objectContaining({
-          testId: 'cli-test-id',
+          reportFileName: 'cli-test-id',
         }),
         undefined, // browser
         undefined, // page
       );
     });
 
-    test('should use YAML testId when no preference testId exists', async () => {
+    test('should use YAML legacy testId as reportFileName when no explicit reportFileName exists', async () => {
       const mockScript: MidsceneYamlScript = {
         web: {
           serve: './test',
@@ -1032,11 +1030,11 @@ describe('create-yaml-player', () => {
         await setupFnCallback();
       }
 
-      // When no explicit CLI testId is provided, YAML testId takes precedence over fileName
+      // When no explicit reportFileName/CLI value is provided, YAML legacy testId takes precedence over fileName
       expect(puppeteerAgentForTarget).toHaveBeenCalledWith(
         expect.any(Object),
         expect.objectContaining({
-          testId: 'yaml-test-id',
+          reportFileName: 'yaml-test-id',
         }),
         undefined, // browser
         undefined, // page
@@ -1182,7 +1180,7 @@ describe('create-yaml-player', () => {
       expect(puppeteerAgentForTarget).toHaveBeenCalledWith(
         expect.any(Object),
         expect.objectContaining({
-          testId: 'cdp-test',
+          reportFileName: 'cdp-test',
           groupName: 'CDP Tests',
         }),
         expect.any(Object),

packages/core/src/agent/agent.ts

@@ -351,6 +351,8 @@ export class Agent<
     this.dump = this.resetDump();
     this.reportFileName =
       opts?.reportFileName ||
+      // Keep deprecated testId behavior for generated report names until it is
+      // fully removed from the public API.
       getReportFileName(opts?.testId || this.interface.interfaceType || 'web');
 
     this.reportGenerator = ReportGenerator.create(this.reportFileName!, {
@@ -1488,7 +1490,7 @@ export class Agent<
     // Use the unified utils function to process cache configuration
     const cacheConfig = processCacheConfig(
       opts.cache,
-      opts.cacheId || opts.testId || 'default',
+      opts.cacheId || 'default',
     );
 
     if (!cacheConfig) {

packages/core/src/report-generator.ts

@@ -95,6 +95,7 @@ export class ReportGenerator implements IReportGenerator {
 
     // In browser environment, file system is not available
     if (ifInBrowser) return nullReportGenerator;
+    validateReportFileName(reportFileName);
 
     if (opts.outputFormat === 'html-and-external-assets') {
       const outputDir = join(getMidsceneRunSubDir('report'), reportFileName);
@@ -293,3 +294,21 @@ export class ReportGenerator implements IReportGenerator {
     );
   }
 }
+
+function validateReportFileName(reportFileName: string): void {
+  if (!reportFileName?.trim()) {
+    throw new Error('reportFileName must be a non-empty string');
+  }
+
+  if (/[\\/]/.test(reportFileName)) {
+    throw new Error(
+      'reportFileName must not contain path separators (`/` or `\\\\`)',
+    );
+  }
+
+  if (/[:*?"<>|]/.test(reportFileName)) {
+    throw new Error(
+      'reportFileName contains illegal filename characters: : * ? " < > |',
+    );
+  }
+}

packages/core/src/types.ts

@@ -1073,6 +1073,7 @@ export type Cache =
   | CacheConfig; // Object configuration (requires explicit id)
 
 export interface AgentOpt {
+  // @deprecated Use `reportFileName` and `cache.id` instead.
   testId?: string;
   // @deprecated
   cacheId?: string; // Keep backward compatibility, but marked as deprecated

packages/core/src/yaml.ts

@@ -76,14 +76,14 @@ export interface MidsceneYamlTask {
  * fields like functions and complex objects. All fields are optional.
  *
  * @remarks
- * - testId priority: CLI parameter > YAML agent.testId > filename
+ * - testId is deprecated; prefer reportFileName and cache.id
  * - These settings apply to all platforms (Web, Android, iOS, Generic Interface)
  * - modelConfig is configured through environment variables, not in YAML
  *
  * @example
  * ```yaml
  * agent:
- *   testId: "checkout-test"
+ *   reportFileName: "checkout-report"
  *   groupName: "E2E Test Suite"
  *   generateReport: true
  *   replanningCycleLimit: 30
@@ -94,7 +94,7 @@ export interface MidsceneYamlTask {
  */
 export type MidsceneYamlScriptAgentOpt = Pick<
   AgentOpt,
-  | 'testId'
+  | 'testId' // deprecated, kept for backward compatibility
   | 'groupName'
   | 'groupDescription'
   | 'generateReport'

packages/core/tests/unit-test/report-generator.test.ts

@@ -619,6 +619,24 @@ describe('ReportGenerator — append-only model', () => {
       expect(reportPath).toContain('test-dir');
       expect(reportPath).toContain('index.html');
     });
+
+    it('should throw for reportFileName with path separators', () => {
+      expect(() => ReportGenerator.create('../bad-name', {})).toThrow(
+        'reportFileName must not contain path separators',
+      );
+      expect(() => ReportGenerator.create('bad/name', {})).toThrow(
+        'reportFileName must not contain path separators',
+      );
+    });
+
+    it('should throw for reportFileName with illegal filename characters', () => {
+      expect(() => ReportGenerator.create('bad:name', {})).toThrow(
+        'reportFileName contains illegal filename characters',
+      );
+      expect(() => ReportGenerator.create('bad*name', {})).toThrow(
+        'reportFileName contains illegal filename characters',
+      );
+    });
   });
 
   describe('lazy loading — memory release behavior', () => {

packages/web-integration/src/playwright/ai-fixture.ts

@@ -93,7 +93,7 @@ export const PlaywrightAiFixture = (options?: {
       const cacheConfig = processTestCacheConfig(testInfo);
 
       pageAgentMap[idForPage] = new PlaywrightAgent(page, {
-        testId: `playwright-${testId}-${idForPage}`,
+        reportFileName: `playwright-${testId}-${idForPage}`,
         forceSameTabNavigation,
         cache: cacheConfig,
         groupName: title,

packages/web-integration/src/puppeteer/agent-launcher.ts

@@ -344,7 +344,6 @@ export async function puppeteerAgentForTarget(
   } & Partial<
     Pick<
       AgentOpt,
-      | 'testId'
       | 'groupName'
       | 'groupDescription'
       | 'generateReport'