fix(test): preserve colliding diagnostic artifacts

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Fixed: Replay test artifacts with colliding filenames retain distinct copies without overwriting
+  other diagnostics, replay sources, timing traces, or attempt manifests.
 - Fixed: Custom test reporters reject invalid exit codes, including values such as `256` that
   could wrap to success and hide a failing suite. `getExitCode` accepts integers from `0` to `255`
   or `undefined`; JSON output reports an invalid code as one `INVALID_ARGS` error.

packages/replay-test/src/internal/__tests__/session-test-artifacts.test.ts

@@ -11,6 +11,11 @@ import {
 } from '../session-test-artifacts.ts';
 import type { ReplayTestAttemptOutcome } from '../session-test-types.ts';
 
+// Package tests compile within their own rootDir and cannot import the root test utility.
+function mkdtempForTestSync(prefix: string): string {
+  return fs.mkdtempSync(path.join(os.tmpdir(), prefix));
+}
+
 test('resolveReplayTestArtifactsDir falls back to the default root when artifactsDir is omitted', () => {
   const dir = resolveReplayTestArtifactsDir({ cwd: '/repo', suiteInvocationId: 'abc123' });
   assert.equal(dir, path.resolve('/repo', DEFAULT_TEST_ARTIFACTS_ROOT, 'abc123'));
@@ -119,3 +124,197 @@ test('materializeReplayTestAttemptArtifacts writes failure manifest and copies l
   assert.match(resultText, /timeoutMode: cooperative/);
   assert.match(resultText, /copiedArtifacts: capture\.png, daemon\.log/);
 });
+
+test.each([
+  {
+    names: ['device.log', 'device.log', 'device-2.log'],
+    copied: ['device.log', 'device-2.log', 'device-2-2.log'],
+  },
+  {
+    names: ['device.log', 'device-2.log', 'device.log'],
+    copied: ['device.log', 'device-2.log', 'device-3.log'],
+  },
+  {
+    names: ['log', 'log', 'log-2'],
+    copied: ['log', 'log-2', 'log-2-2'],
+  },
+])('materialization preserves colliding artifacts: $names', ({ names, copied }) => {
+  const root = mkdtempForTestSync('agent-device-artifact-collisions-');
+  const replayPath = path.join(root, 'flow.ad');
+  const attemptDir = path.join(root, 'attempt-1');
+  fs.writeFileSync(replayPath, 'context platform=android\nopen demo.app\n');
+  const artifactPaths = names.map((name, index) => {
+    const dir = path.join(root, String(index));
+    fs.mkdirSync(dir);
+    const artifactPath = path.join(dir, name);
+    fs.writeFileSync(artifactPath, `artifact ${index}`);
+    return artifactPath;
+  });
+
+  prepareReplayTestAttemptArtifacts(replayPath, attemptDir);
+  materializeReplayTestAttemptArtifacts({
+    outcome: passedOutcome({ artifactPaths }),
+    filePath: replayPath,
+    sessionName: 'artifact-collisions',
+    attempts: 1,
+    maxAttempts: 1,
+    attemptArtifactsDir: attemptDir,
+  });
+
+  for (const [index, name] of copied.entries()) {
+    assert.equal(fs.readFileSync(path.join(attemptDir, name), 'utf8'), `artifact ${index}`);
+  }
+  const manifest = fs.readFileSync(path.join(attemptDir, 'result.txt'), 'utf8');
+  assert.ok(manifest.includes(`copiedArtifacts: ${copied.join(', ')}\n`));
+});
+
+test('materialization preserves replay sources and diagnostics named after attempt manifests', () => {
+  const root = mkdtempForTestSync('agent-device-artifact-reserved-');
+  const replayPath = path.join(root, 'flow.yml');
+  const attemptDir = path.join(root, 'attempt-1');
+  const source = 'appId: demo.app\n---\n- assertVisible: Welcome\n';
+  fs.writeFileSync(replayPath, source);
+  const names = ['replay.ad', 'flow.yml', 'result.txt', 'failure.txt'];
+  const artifactPaths = names.map((name) => {
+    const dir = path.join(root, 'diagnostics', name);
+    fs.mkdirSync(dir, { recursive: true });
+    const artifactPath = path.join(dir, name);
+    fs.writeFileSync(artifactPath, `diagnostic ${name}`);
+    return artifactPath;
+  });
+
+  prepareReplayTestAttemptArtifacts(replayPath, attemptDir);
+  materializeReplayTestAttemptArtifacts({
+    outcome: {
+      status: 'failed',
+      error: { code: 'COMMAND_FAILED', message: 'original failure' },
+      artifactPaths,
+      infrastructure: false,
+    },
+    filePath: replayPath,
+    sessionName: 'artifact-reserved',
+    attempts: 1,
+    maxAttempts: 1,
+    attemptArtifactsDir: attemptDir,
+  });
+
+  assert.equal(fs.readFileSync(path.join(attemptDir, 'replay.ad'), 'utf8'), source);
+  assert.equal(fs.readFileSync(path.join(attemptDir, 'flow.yml'), 'utf8'), source);
+  for (const name of names) {
+    const extension = path.extname(name);
+    const copiedName = `${path.basename(name, extension)}-2${extension}`;
+    assert.equal(fs.readFileSync(path.join(attemptDir, copiedName), 'utf8'), `diagnostic ${name}`);
+  }
+  const manifest = fs.readFileSync(path.join(attemptDir, 'result.txt'), 'utf8');
+  assert.match(manifest, /status: failed\ncode: COMMAND_FAILED\nmessage: original failure/);
+  assert.equal(fs.readFileSync(path.join(attemptDir, 'failure.txt'), 'utf8'), manifest);
+  assert.match(
+    manifest,
+    /copiedArtifacts: replay-2\.ad, flow-2\.yml, result-2\.txt, failure-2\.txt/,
+  );
+});
+
+test.each([false, true])(
+  'materialization preserves artifacts already in the attempt directory (local first: %s)',
+  (localFirst) => {
+    const root = mkdtempForTestSync('agent-device-artifact-local-');
+    const replayPath = path.join(root, 'flow.ad');
+    const attemptDir = path.join(root, 'attempt-1');
+    const externalTrace = path.join(root, 'replay-timing.ndjson');
+    const localTrace = path.join(attemptDir, 'replay-timing.ndjson');
+    fs.writeFileSync(replayPath, 'context platform=android\nopen demo.app\n');
+    fs.writeFileSync(externalTrace, 'external trace');
+    prepareReplayTestAttemptArtifacts(replayPath, attemptDir);
+    fs.writeFileSync(localTrace, 'attempt trace');
+    const artifactPaths = localFirst ? [localTrace, externalTrace] : [externalTrace, localTrace];
+
+    materializeReplayTestAttemptArtifacts({
+      outcome: passedOutcome({ artifactPaths: [...artifactPaths, path.join(root, 'missing.log')] }),
+      filePath: replayPath,
+      sessionName: 'artifact-local',
+      attempts: 1,
+      maxAttempts: 1,
+      attemptArtifactsDir: attemptDir,
+    });
+
+    assert.equal(fs.readFileSync(localTrace, 'utf8'), 'attempt trace');
+    assert.equal(
+      fs.readFileSync(path.join(attemptDir, 'replay-timing-2.ndjson'), 'utf8'),
+      'external trace',
+    );
+    assert.deepEqual(fs.readdirSync(attemptDir).sort(), [
+      'replay-timing-2.ndjson',
+      'replay-timing.ndjson',
+      'replay.ad',
+      'result.txt',
+    ]);
+    const manifest = fs.readFileSync(path.join(attemptDir, 'result.txt'), 'utf8');
+    assert.equal(manifest.includes('missing.log'), false);
+  },
+);
+
+test('materialization copies a log listed in both the outcome and error only once', () => {
+  const root = mkdtempForTestSync('agent-device-artifact-log-');
+  const replayPath = path.join(root, 'flow.ad');
+  const logPath = path.join(root, 'daemon.log');
+  const attemptDir = path.join(root, 'attempt-1');
+  fs.writeFileSync(replayPath, 'context platform=android\nopen demo.app\n');
+  fs.writeFileSync(logPath, 'diagnostic log');
+  prepareReplayTestAttemptArtifacts(replayPath, attemptDir);
+
+  materializeReplayTestAttemptArtifacts({
+    outcome: {
+      status: 'failed',
+      error: { code: 'COMMAND_FAILED', message: 'failed', logPath },
+      artifactPaths: [logPath, logPath],
+      infrastructure: false,
+    },
+    filePath: replayPath,
+    sessionName: 'artifact-log',
+    attempts: 1,
+    maxAttempts: 1,
+    attemptArtifactsDir: attemptDir,
+  });
+
+  assert.equal(fs.readFileSync(path.join(attemptDir, 'daemon.log'), 'utf8'), 'diagnostic log');
+  assert.equal(fs.existsSync(path.join(attemptDir, 'daemon-2.log')), false);
+  assert.match(
+    fs.readFileSync(path.join(attemptDir, 'result.txt'), 'utf8'),
+    /copiedArtifacts: daemon\.log\n/,
+  );
+});
+
+test.each(['result.txt', 'failure.txt', 'RESULT.TXT'])(
+  'materialization preserves an in-place diagnostic named %s before writing manifests',
+  (name) => {
+    const root = mkdtempForTestSync('agent-device-artifact-in-place-manifest-');
+    const replayPath = path.join(root, 'flow.ad');
+    const attemptDir = path.join(root, 'attempt-1');
+    fs.writeFileSync(replayPath, 'context platform=android\nopen demo.app\n');
+    prepareReplayTestAttemptArtifacts(replayPath, attemptDir);
+    const diagnosticPath = path.join(attemptDir, name);
+    fs.writeFileSync(diagnosticPath, 'diagnostic contents');
+
+    materializeReplayTestAttemptArtifacts({
+      outcome: {
+        status: 'failed',
+        error: { code: 'COMMAND_FAILED', message: 'original failure' },
+        artifactPaths: [diagnosticPath],
+        infrastructure: false,
+      },
+      filePath: replayPath,
+      sessionName: 'artifact-in-place-manifest',
+      attempts: 1,
+      maxAttempts: 1,
+      attemptArtifactsDir: attemptDir,
+    });
+
+    const extension = path.extname(name);
+    const copiedName = `${path.basename(name, extension)}-2${extension}`;
+    assert.equal(fs.readFileSync(path.join(attemptDir, copiedName), 'utf8'), 'diagnostic contents');
+    const manifest = fs.readFileSync(path.join(attemptDir, 'result.txt'), 'utf8');
+    assert.ok(manifest.includes(`copiedArtifacts: ${copiedName}\n`));
+    assert.match(manifest, /status: failed/);
+    assert.equal(fs.readFileSync(path.join(attemptDir, 'failure.txt'), 'utf8'), manifest);
+  },
+);

packages/replay-test/src/internal/session-test-artifacts.ts

@@ -59,11 +59,16 @@ export function materializeReplayTestAttemptArtifacts(params: {
 }): void {
   const { outcome, filePath, sessionName, attempts, maxAttempts, attemptArtifactsDir } = params;
   const passed = outcome.status === 'passed';
-  const sourcePaths = [...new Set(outcome.artifactPaths)];
+  const sourcePaths = new Set(outcome.artifactPaths);
   if (outcome.status === 'failed' && typeof outcome.error.logPath === 'string') {
-    sourcePaths.push(outcome.error.logPath);
+    sourcePaths.add(outcome.error.logPath);
   }
-  const copiedArtifacts = copyReplayTestArtifacts(sourcePaths, attemptArtifactsDir);
+  const resultPath = path.join(attemptArtifactsDir, 'result.txt');
+  const failurePath = path.join(attemptArtifactsDir, 'failure.txt');
+  const copiedArtifacts = copyReplayTestArtifacts([...sourcePaths], attemptArtifactsDir, [
+    resultPath,
+    failurePath,
+  ]);
 
   const lines = [
     `file: ${filePath}`,
@@ -90,24 +95,33 @@ export function materializeReplayTestAttemptArtifacts(params: {
     );
   }
 
-  const resultPath = path.join(attemptArtifactsDir, 'result.txt');
   const output = `${lines.join('\n')}\n`;
   fs.writeFileSync(resultPath, output);
   if (!passed) {
-    fs.writeFileSync(path.join(attemptArtifactsDir, 'failure.txt'), output);
+    fs.writeFileSync(failurePath, output);
   }
 }
 
-function copyReplayTestArtifacts(paths: string[], attemptArtifactsDir: string): string[] {
+function copyReplayTestArtifacts(
+  paths: string[],
+  attemptArtifactsDir: string,
+  manifestPaths: string[],
+): string[] {
   const copiedPaths: string[] = [];
-  const usedNames = new Map<string, number>();
+  const reservedNames = new Set(manifestPaths.map((entry) => path.basename(entry).toLowerCase()));
   for (const sourcePath of paths) {
     if (!isExistingFile(sourcePath)) continue;
-    const fileName = buildUniqueArtifactFileName(path.basename(sourcePath), usedNames);
-    const destinationPath = path.join(attemptArtifactsDir, fileName);
-    if (path.resolve(sourcePath) !== path.resolve(destinationPath)) {
-      fs.copyFileSync(sourcePath, destinationPath);
+    const sourceName = path.basename(sourcePath);
+    if (
+      path.resolve(sourcePath) === path.resolve(attemptArtifactsDir, sourceName) &&
+      !reservedNames.has(sourceName.toLowerCase())
+    ) {
+      copiedPaths.push(sourcePath);
+      continue;
     }
+    const fileName = buildUniqueArtifactFileName(sourceName, attemptArtifactsDir, reservedNames);
+    const destinationPath = path.join(attemptArtifactsDir, fileName);
+    fs.copyFileSync(sourcePath, destinationPath);
     copiedPaths.push(destinationPath);
   }
   return copiedPaths;
@@ -129,13 +143,22 @@ function copyReplaySourceFile(filePath: string, attemptArtifactsDir: string): vo
   }
 }
 
-function buildUniqueArtifactFileName(fileName: string, usedNames: Map<string, number>): string {
+function buildUniqueArtifactFileName(
+  fileName: string,
+  attemptArtifactsDir: string,
+  reservedNames: ReadonlySet<string>,
+): string {
   const extension = path.extname(fileName);
   const stem = extension ? fileName.slice(0, -extension.length) : fileName;
-  const current = usedNames.get(fileName) ?? 0;
-  usedNames.set(fileName, current + 1);
-  if (current === 0) return fileName;
-  return `${stem}-${current + 1}${extension}`;
+  let candidate = fileName;
+  let suffix = 2;
+  while (
+    reservedNames.has(candidate.toLowerCase()) ||
+    fs.existsSync(path.join(attemptArtifactsDir, candidate))
+  ) {
+    candidate = `${stem}-${suffix++}${extension}`;
+  }
+  return candidate;
 }
 
 function isExistingFile(filePath: string): boolean {

src/commands/replay/index.ts

@@ -246,7 +246,7 @@ export const testCommandFacet = defineCommandFacet({
   text: {
     summary: 'Run replay test suites',
     cliDetail:
-      "Relative globs are expanded on the caller from its working directory, whose name is treated literally. Quote glob inputs to defer expansion to test. JUnit reports (--reporter junit:<path>) replace characters forbidden by XML 1.0 with U+FFFD and preserve legal Unicode and whitespace. JSON and other reporters retain the original suite values. Custom reporter getExitCode hooks must return an integer from 0 to 255 or undefined; the highest valid code wins and cannot lower a failing suite's exit code.",
+      "Relative globs are expanded on the caller from its working directory, whose name is treated literally. Quote glob inputs to defer expansion to test. Copied diagnostic artifacts receive numbered filenames when needed to preserve other artifacts, replay sources, timing traces, and attempt manifests. JUnit reports (--reporter junit:<path>) replace characters forbidden by XML 1.0 with U+FFFD and preserve legal Unicode and whitespace. JSON and other reporters retain the original suite values. Custom reporter getExitCode hooks must return an integer from 0 to 255 or undefined; the highest valid code wins and cannot lower a failing suite's exit code.",
   },
   metadata: testCommandMetadata,
   run: (client, input) => client.replay.test(withCommandRuntimeHints(input)),

website/docs/docs/replay-e2e.md

@@ -119,6 +119,7 @@ agent-device test ./workflows --reporter default --reporter junit:./tmp/junit.xm
 - `--platform` is a filter for suite discovery; files without platform metadata are skipped when a filter is present.
 - `context timeout=...` and `context retries=...` can be declared per script; CLI flags override metadata. Retries are capped at `3`, and duplicate keys in the context header fail fast instead of silently overriding each other.
 - By default, suite artifacts are written under `.agent-device/test-artifacts/<run-id>/...`. Each attempt writes `replay.ad`, `result.txt`, and `replay-timing.ndjson`. Failed attempts also keep copied logs and artifact files when the replay produced them.
+- Copied diagnostic artifacts receive numbered filenames when their names collide with another artifact, a replay source, timing trace, or attempt manifest. `result.txt` lists the retained names in `copiedArtifacts`.
 - `replay-timing.ndjson` records attempt, cleanup, and per-step start/stop events with durations. Upload it from CI even for passing runs when comparing local and CI performance.
 - Timeouts are cooperative: the runner marks the attempt failed at the timeout boundary, then gives the underlying replay a short grace period to stop before session cleanup.
 - The default text reporter streams live progress on stderr while a suite runs, then prints the final summary, failed tests, and passed-on-retry flaky tests. Use `--verbose` to include step traces in completed-test progress output.