fix(test): address import/export review — BOM, idempotency triad, If-Match *, --out tests, error split

Author: Andy00L

src/commands/test.test.ts

@@ -3407,6 +3407,185 @@ describe('runExport / runImport', () => {
       ),
     ).rejects.toMatchObject({ code: 'VALIDATION_ERROR', exitCode: 5 });
   });
+
+  it('import accepts a BOM-prefixed definition file (PowerShell utf8 writes one)', async () => {
+    const { credentialsPath } = makeCreds();
+    const dir = mkdtempSync(join(tmpdir(), 'cli-import-bom-'));
+    const file = join(dir, 'def.testsprite.json');
+    writeFileSync(
+      file,
+      `${JSON.stringify({
+        schemaVersion: 1,
+        projectId: 'project_alice',
+        type: 'backend',
+        name: 'BOM test',
+      })}`,
+      'utf8',
+    );
+    const fetchImpl = (async () =>
+      new Response(JSON.stringify({ testId: 'test_bom' }), {
+        status: 200,
+        headers: { 'content-type': 'application/json' },
+      })) as typeof fetch;
+    const result = await runImport(
+      { profile: 'default', output: 'json', debug: false, file },
+      { credentialsPath, fetchImpl, stdout: () => undefined, stderr: () => undefined },
+    );
+    expect(result).toEqual({ testId: 'test_bom', action: 'created' });
+  });
+
+  it('import splits missing-file from malformed-JSON errors', async () => {
+    const dir = mkdtempSync(join(tmpdir(), 'cli-import-err-'));
+    await expect(
+      runImport(
+        { profile: 'default', output: 'json', debug: false, file: join(dir, 'nope.json') },
+        { stdout: () => undefined },
+      ),
+    ).rejects.toMatchObject({
+      code: 'VALIDATION_ERROR',
+      nextAction: expect.stringContaining('file not found'),
+    });
+    const bad = join(dir, 'bad.json');
+    writeFileSync(bad, '{not json', 'utf8');
+    await expect(
+      runImport(
+        { profile: 'default', output: 'json', debug: false, file: bad },
+        { stdout: () => undefined },
+      ),
+    ).rejects.toMatchObject({
+      code: 'VALIDATION_ERROR',
+      nextAction: expect.stringContaining('is not valid JSON'),
+    });
+  });
+
+  it('import with codeVersion null sends If-Match: * and warns about the unconditional overwrite', async () => {
+    const { credentialsPath } = makeCreds();
+    const dir = mkdtempSync(join(tmpdir(), 'cli-import-null-'));
+    const file = join(dir, 'def.testsprite.json');
+    writeFileSync(
+      file,
+      JSON.stringify({
+        schemaVersion: 1,
+        testId: 'test_be',
+        projectId: 'project_alice',
+        type: 'backend',
+        name: 'Legacy row',
+        code: { language: 'python', framework: 'pytest', body: 'print(3)\n', codeVersion: null },
+      }),
+      'utf8',
+    );
+    const seen: Array<{ url: string; ifMatch: string | null }> = [];
+    const fetchImpl = (async (input: Parameters<typeof fetch>[0], init: RequestInit = {}) => {
+      const headers = new Headers(init.headers);
+      seen.push({ url: String(input), ifMatch: headers.get('if-match') });
+      return new Response(JSON.stringify({ testId: 'test_be' }), {
+        status: 200,
+        headers: { 'content-type': 'application/json' },
+      });
+    }) as typeof fetch;
+    const errs: string[] = [];
+    await runImport(
+      { profile: 'default', output: 'json', debug: false, file },
+      { credentialsPath, fetchImpl, stdout: () => undefined, stderr: line => errs.push(line) },
+    );
+    expect(seen[1]!.url).toContain('/code');
+    expect(seen[1]!.ifMatch).toBe('*');
+    expect(errs.join('\n')).toContain('If-Match: *');
+  });
+
+  it('import mints one echoed idempotency key and derives :meta/:code from a supplied one', async () => {
+    const { credentialsPath } = makeCreds();
+    const dir = mkdtempSync(join(tmpdir(), 'cli-import-idem-'));
+    const file = join(dir, 'def.testsprite.json');
+    writeFileSync(
+      file,
+      JSON.stringify({
+        schemaVersion: 1,
+        testId: 'test_be',
+        projectId: 'project_alice',
+        type: 'backend',
+        name: 'Idem test',
+        code: { language: 'python', framework: 'pytest', body: 'print(4)\n', codeVersion: 'v3' },
+      }),
+      'utf8',
+    );
+    const keys: Array<string | null> = [];
+    const fetchImpl = (async (input: Parameters<typeof fetch>[0], init: RequestInit = {}) => {
+      keys.push(new Headers(init.headers).get('idempotency-key'));
+      return new Response(JSON.stringify({ testId: 'test_be' }), {
+        status: 200,
+        headers: { 'content-type': 'application/json' },
+      });
+    }) as typeof fetch;
+    const errs: string[] = [];
+    await runImport(
+      { profile: 'default', output: 'json', debug: false, file, idempotencyKey: 'ci-key-1' },
+      { credentialsPath, fetchImpl, stdout: () => undefined, stderr: line => errs.push(line) },
+    );
+    expect(keys).toEqual(['ci-key-1:meta', 'ci-key-1:code']);
+    expect(errs.join('\n')).not.toContain('idempotency-key:');
+
+    keys.length = 0;
+    await runImport(
+      { profile: 'default', output: 'json', debug: false, file },
+      { credentialsPath, fetchImpl, stdout: () => undefined, stderr: line => errs.push(line) },
+    );
+    expect(keys[0]).toMatch(/^cli-import-.+:meta$/);
+    expect(keys[1]).toMatch(/^cli-import-.+:code$/);
+    expect(errs.join('\n')).toContain('idempotency-key: cli-import-');
+  });
+
+  it('export --out writes the file, refuses to overwrite without --force, overwrites with it', async () => {
+    const { credentialsPath } = makeCreds();
+    const fetchImpl = makeFetch(url => ({ body: url.includes('/code') ? CODE_ROW : TEST_ROW }));
+    const dir = mkdtempSync(join(tmpdir(), 'cli-export-out-'));
+    const outFile = join(dir, 'def.testsprite.json');
+    const errs: string[] = [];
+    await runExport(
+      {
+        profile: 'default',
+        output: 'json',
+        debug: false,
+        testId: 'test_be',
+        out: outFile,
+        force: false,
+      },
+      { credentialsPath, fetchImpl, stdout: () => undefined, stderr: line => errs.push(line) },
+    );
+    const written = JSON.parse(readFileSync(outFile, 'utf8')) as { testId?: string };
+    expect(written).toMatchObject({ schemaVersion: 1, testId: 'test_be' });
+    expect(errs.join('\n')).toContain('Definition written to');
+
+    await expect(
+      runExport(
+        {
+          profile: 'default',
+          output: 'json',
+          debug: false,
+          testId: 'test_be',
+          out: outFile,
+          force: false,
+        },
+        { credentialsPath, fetchImpl, stdout: () => undefined, stderr: () => undefined },
+      ),
+    ).rejects.toMatchObject({
+      code: 'VALIDATION_ERROR',
+      nextAction: expect.stringContaining('already exists'),
+    });
+
+    await runExport(
+      {
+        profile: 'default',
+        output: 'json',
+        debug: false,
+        testId: 'test_be',
+        out: outFile,
+        force: true,
+      },
+      { credentialsPath, fetchImpl, stdout: () => undefined, stderr: () => undefined },
+    );
+    expect(JSON.parse(readFileSync(outFile, 'utf8'))).toMatchObject({ testId: 'test_be' });
+  });
 });
 
 describe('runLint', () => {

src/commands/test.ts

@@ -4179,6 +4179,8 @@ export async function runExport(
 
 export interface ImportOptions extends CommonOptions {
   file: string;
+  /** --idempotency-key: caller-supplied; auto-minted UUID when absent. */
+  idempotencyKey?: string;
 }
 
 /**
@@ -4194,18 +4196,24 @@ export async function runImport(
 ): Promise<{ testId: string; action: 'created' | 'updated' }> {
   const out = makeOutput(opts.output, deps);
   const stderrFn = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`));
+  assertIdempotencyKey(opts.idempotencyKey);
 
   const absolute = resolveAbsolute(opts.file);
+  // Read and parse are split so a typo'd path and malformed JSON stop being
+  // the same message. stripBom mirrors every other JSON file-read in this
+  // file: PowerShell 5.1's `Set-Content -Encoding utf8` writes a BOM, and
+  // bare JSON.parse rejects it.
+  let raw: string;
+  try {
+    raw = stripBom(readFileSync(absolute, 'utf8'));
+  } catch {
+    throw localValidationError('file', `file not found: ${absolute}`, undefined, 'field');
+  }
   let parsed: unknown;
   try {
-    parsed = JSON.parse(readFileSync(absolute, 'utf8'));
+    parsed = JSON.parse(raw);
   } catch {
-    throw localValidationError(
-      'file',
-      `is not readable valid JSON: ${absolute}`,
-      undefined,
-      'field',
-    );
+    throw localValidationError('file', `is not valid JSON: ${absolute}`, undefined, 'field');
   }
   const def = parsed as Partial<CliTestDefinition>;
   if (def.schemaVersion !== 1) {
@@ -4239,6 +4247,17 @@ export async function runImport(
     return sample;
   }
 
+  // One base key covers the whole import; per-mutation keys derive from it
+  // deterministically (`:meta`, `:code`, `:create`) so a caller-supplied key
+  // replayed after an ambiguous transport failure reuses the SAME wire keys
+  // instead of minting fresh ones and duplicating the mutation (same contract
+  // as `test create` / `test update` / `code put`). Echoed on stderr below
+  // for the auto-minted case, mirroring the `test create` site.
+  const idempotencyKey = opts.idempotencyKey ?? `cli-import-${randomUUID()}`;
+  if (opts.idempotencyKey === undefined && (opts.output === 'json' || opts.verbose || opts.debug)) {
+    stderrFn(`idempotency-key: ${idempotencyKey}`);
+  }
+
   const client = makeClient(opts, deps);
   if (typeof def.testId === 'string' && def.testId.length > 0) {
     const testId = def.testId;
@@ -4247,19 +4266,28 @@ export async function runImport(
         name: def.name.trim(),
         ...(def.description !== undefined ? { description: def.description } : {}),
       },
-      headers: { 'idempotency-key': `cli-import-meta-${randomUUID()}` },
+      headers: { 'idempotency-key': `${idempotencyKey}:meta` },
     });
     if (def.code !== undefined && typeof def.code.body === 'string') {
+      const recordedVersion = def.code.codeVersion;
+      if (recordedVersion === null || recordedVersion === undefined) {
+        // Legacy export with no stamped codeVersion: `If-Match: *` applies the
+        // backend's force path (audit-logged) instead of silently omitting the
+        // precondition — same handling as `code put`'s auto-fetch on a legacy
+        // row, and the notice makes the unconditional overwrite visible.
+        stderrFn(
+          `note: definition has codeVersion: null (legacy export) — sending If-Match: * (unconditional overwrite of the server copy)`,
+        );
+      }
       await client.put(`/tests/${encodeURIComponent(testId)}/code`, {
         body: { code: def.code.body },
         headers: {
-          'idempotency-key': `cli-import-code-${randomUUID()}`,
+          'idempotency-key': `${idempotencyKey}:code`,
           // Replay the recorded provenance so a server copy that moved on
           // fails with the existing 412 PRECONDITION_FAILED contract instead
           // of being silently clobbered.
-          ...(def.code.codeVersion !== null && def.code.codeVersion !== undefined
-            ? { 'if-match': def.code.codeVersion }
-            : {}),
+          'if-match':
+            recordedVersion !== null && recordedVersion !== undefined ? recordedVersion : '*',
         },
       });
     }
@@ -4276,7 +4304,7 @@ export async function runImport(
       ...(def.description !== undefined ? { description: def.description } : {}),
       ...(def.code !== undefined ? { code: def.code.body } : {}),
     },
-    headers: { 'idempotency-key': `cli-import-create-${randomUUID()}` },
+    headers: { 'idempotency-key': `${idempotencyKey}:create` },
   });
   const result = { testId: created.testId, action: 'created' as const };
   out.print(result, () => `created: ${created.testId}`);
@@ -9111,9 +9139,16 @@ export function createTestCommand(deps: TestDeps = {}): Command {
     .description(
       'Create or update a test from a definition file produced by `test export`: a testId in the file updates (code PUT replays the recorded codeVersion as If-Match), no testId creates.',
     )
+    .option(
+      '--idempotency-key <key>',
+      'opaque key for safe retries (1–256 chars). Printed to stderr at --debug if auto-generated.',
+    )
     .addHelpText('after', GLOBAL_OPTS_HINT)
-    .action(async (file: string, _cmdOpts: unknown, command: Command) => {
-      await runImport({ ...resolveCommonOptions(command), file }, deps);
+    .action(async (file: string, cmdOpts: { idempotencyKey?: string }, command: Command) => {
+      await runImport(
+        { ...resolveCommonOptions(command), file, idempotencyKey: cmdOpts.idempotencyKey },
+        deps,
+      );
     });
 
   test

test/__snapshots__/help.snapshot.test.ts.snap

@@ -246,7 +246,7 @@ Commands:
                                         versionable JSON file. Frontend plans
                                         are write-only on the API, so FE
                                         exports carry planUnavailable: true.
-  import <file>                         Create or update a test from a
+  import [options] <file>               Create or update a test from a
                                         definition file produced by \`test
                                         export\`: a testId in the file updates
                                         (code PUT replays the recorded