fix(remote): materialize test suite artifacts against a remote daemon

packages/contracts/src/replay.ts

@@ -141,6 +141,13 @@ export type ReplaySuiteResult = {
   durationMs: number;
   failures: ReplaySuiteTestFailed[];
   tests: ReplaySuiteTestResult[];
+  /**
+   * The suite's own artifacts root (the parent of every test's `artifactsDir`), as resolved on
+   * the host that ran the suite. Absent when the suite produced no attempt (e.g. every source
+   * was filtered out). #2246: a remote daemon rewrites this to the caller-local path once the
+   * directory has been transferred back, so it always names a path the caller can open.
+   */
+  artifactsDir?: string;
   snapshotDiagnostics?: SnapshotDiagnosticsSummary;
 };
 

packages/kernel/src/contracts.ts

@@ -114,7 +114,8 @@ export type DaemonArtifactKnownType =
   | 'screen-recording'
   | 'screen-recording-chunk'
   | 'screen-recording-telemetry'
-  | 'trace-log';
+  | 'trace-log'
+  | 'test-artifacts';
 
 export type DaemonArtifactType = DaemonArtifactKnownType | (string & {});
 

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

@@ -4,11 +4,27 @@ import fs from 'node:fs';
 import os from 'node:os';
 import path from 'node:path';
 import {
+  DEFAULT_TEST_ARTIFACTS_ROOT,
   materializeReplayTestAttemptArtifacts,
   prepareReplayTestAttemptArtifacts,
+  resolveReplayTestArtifactsDir,
 } from '../session-test-artifacts.ts';
 import type { ReplayTestAttemptOutcome } from '../session-test-types.ts';
 
+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'));
+});
+
+test('resolveReplayTestArtifactsDir resolves an explicit relative artifactsDir against cwd', () => {
+  const dir = resolveReplayTestArtifactsDir({
+    artifactsDir: 'remote-device-artifacts/ad-test',
+    cwd: '/repo',
+    suiteInvocationId: 'abc123',
+  });
+  assert.equal(dir, path.resolve('/repo', 'remote-device-artifacts/ad-test', 'abc123'));
+});
+
 // Building outcomes from a DaemonResponse is the adapter's job and is pinned on that side; a
 // package test states the neutral outcome directly (#1478 P3b).
 const passedOutcome = (

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

@@ -3,7 +3,15 @@ import path from 'node:path';
 import { trimEdgeDashes } from '@agent-device/kernel/collections';
 import type { ReplayTestAttemptOutcome } from '@agent-device/replay-test';
 
-const DEFAULT_TEST_ARTIFACTS_ROOT = '.agent-device/test-artifacts';
+/**
+ * `test`'s default artifacts root when `--artifacts-dir` is not given. `src/remote/daemon-artifacts.ts`
+ * keeps its own copy of this literal (to redirect a remote request without resolving anything
+ * against the daemon's `cwd`, #2246): pulling it from `@agent-device/contracts` instead grew this
+ * package's eager import closure by 3 modules for one string, past its pinned budget
+ * (`scripts/__tests__/eager-closure-budgets.test.ts`) — not worth it for a value that changes
+ * only if this line does.
+ */
+export const DEFAULT_TEST_ARTIFACTS_ROOT = '.agent-device/test-artifacts';
 
 export function resolveReplayTestArtifactsDir(params: {
   artifactsDir?: string;

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

@@ -124,7 +124,12 @@ export async function runReplayTestSuite(
       );
     }
 
-    const data = summarizeReplayTestResults(plan.total, results, Date.now() - suiteStartedAt);
+    const data = summarizeReplayTestResults(
+      plan.total,
+      results,
+      Date.now() - suiteStartedAt,
+      plan.suiteArtifactsDir,
+    );
     return { status: 'completed', data };
   } catch (error) {
     const appErr = asAppError(error);
@@ -467,6 +472,7 @@ function summarizeReplayTestResults(
   total: number,
   results: ReplaySuiteTestResult[],
   durationMs: number,
+  artifactsDir: string,
 ): ReplaySuiteResult {
   const passed = results.filter((result) => result.status === 'passed').length;
   const failedResults = results.filter(
@@ -488,6 +494,7 @@ function summarizeReplayTestResults(
     durationMs,
     failures: failedResults,
     tests: results,
+    artifactsDir,
     ...(snapshotDiagnostics ? { snapshotDiagnostics } : {}),
   };
 }

src/daemon/replay/internal/__tests__/session-test-suite-command-remote-artifacts.test.ts

@@ -0,0 +1,172 @@
+/**
+ * #2246: once a remote daemon writes suite artifacts under a temp directory it owns (the client
+ * redirects `--artifacts-dir` there — see `daemon-artifacts-test-command.test.ts`), the response
+ * this handler builds must point the caller back at the REAL local root the client will download
+ * that directory into (`req.meta.clientArtifactPaths.artifactsDir`), and must register the
+ * directory as one downloadable artifact so the client's existing artifact transport can pull it.
+ * A local daemon never sets that hint, so the response must come back byte-for-byte unchanged.
+ *
+ * `daemonRoot` here is deliberately two levels (`<tempRoot>/<suiteInvocationId>`), matching what
+ * `resolveReplayTestArtifactsDir` actually produces: the redirected temp ROOT the client sent
+ * joined with the daemon-generated invocation id. The caller-local mirror of that is
+ * `clientRoot/<suiteInvocationId>` — NOT bare `clientRoot`, which is only the root the client
+ * resolved from `--artifacts-dir` *before* the suite ran and could not yet know the invocation id.
+ */
+import assert from 'node:assert/strict';
+import fs from 'node:fs';
+import path from 'node:path';
+import { test } from 'vitest';
+import type { ReplaySuiteResult } from '@agent-device/contracts/replay';
+import type { DaemonRequest } from '../../../types.ts';
+import { attachRemoteReplayTestArtifacts } from '../test-command.ts';
+import { mkdtempForTestSync } from '../../../../__tests__/test-utils/tmp-dir.ts';
+
+const SUITE_INVOCATION_ID = 'cd5f9c01feec8d70';
+
+function suiteResult(daemonRoot: string): ReplaySuiteResult {
+  return {
+    total: 2,
+    executed: 2,
+    passed: 1,
+    failed: 1,
+    skipped: 1,
+    notRun: 0,
+    durationMs: 10,
+    tests: [
+      {
+        file: path.join(daemonRoot, '..', '01-open.ad'),
+        session: 'default:test:1',
+        status: 'passed',
+        durationMs: 5,
+        attempts: 1,
+        artifactsDir: path.join(daemonRoot, 'qa-flows__open.ad', 'attempt-1'),
+        replayed: 1,
+        healed: 0,
+      },
+      {
+        file: path.join(daemonRoot, '..', '02-checkout.ad'),
+        session: 'default:test:2',
+        status: 'failed',
+        durationMs: 8,
+        attempts: 1,
+        artifactsDir: path.join(daemonRoot, 'qa-flows__checkout.ad', 'attempt-1'),
+        error: { code: 'COMMAND_FAILED', message: 'boom' },
+      },
+      {
+        file: 'skipped.ad',
+        status: 'skipped',
+        durationMs: 0,
+        reason: 'skipped-by-filter',
+        message: 'platform mismatch',
+      },
+    ],
+    // The failed test above is intentionally the SAME object reference here, mirroring
+    // `summarizeReplayTestResults` (`failures: results.filter(...)`, `tests: results`) — a fix
+    // that only rewrites `tests` and forgets `failures` would leave this one unrewritten.
+    get failures() {
+      return this.tests.filter(
+        (t): t is Extract<ReplaySuiteResult['tests'][number], { status: 'failed' }> =>
+          t.status === 'failed',
+      );
+    },
+    artifactsDir: daemonRoot,
+  };
+}
+
+function req(clientArtifactsRoot: string | undefined): DaemonRequest {
+  return {
+    token: 't',
+    session: 'default',
+    command: 'test',
+    positionals: [],
+    meta: clientArtifactsRoot ? { clientArtifactPaths: { artifactsDir: clientArtifactsRoot } } : {},
+  };
+}
+
+test('rewrites the suite, every test, and every failure to the caller-local root, and registers the directory for download', () => {
+  const tempRoot = mkdtempForTestSync('agent-device-remote-test-artifacts-daemon-');
+  const daemonRoot = path.join(tempRoot, SUITE_INVOCATION_ID);
+  fs.mkdirSync(path.join(daemonRoot, 'qa-flows__open.ad', 'attempt-1'), { recursive: true });
+  const clientRoot = '/Users/ci/work/installer-app/remote-device-artifacts/ad-test';
+  const clientSuiteRoot = path.join(clientRoot, SUITE_INVOCATION_ID);
+
+  const data = attachRemoteReplayTestArtifacts(suiteResult(daemonRoot), req(clientRoot));
+
+  assert.equal(data.artifactsDir, clientSuiteRoot);
+  const tests = data.tests as ReplaySuiteResult['tests'];
+  assert.equal(
+    (tests[0] as { artifactsDir?: string }).artifactsDir,
+    path.join(clientSuiteRoot, 'qa-flows__open.ad', 'attempt-1'),
+  );
+  assert.equal(
+    (tests[1] as { artifactsDir?: string }).artifactsDir,
+    path.join(clientSuiteRoot, 'qa-flows__checkout.ad', 'attempt-1'),
+  );
+  // A skipped test never had an artifactsDir; it must not gain one.
+  const skippedTest = tests[2];
+  assert.ok(skippedTest);
+  assert.equal('artifactsDir' in skippedTest, false);
+
+  const failures = data.failures as ReplaySuiteResult['failures'];
+  assert.equal(failures.length, 1);
+  assert.equal(
+    (failures[0] as { artifactsDir?: string }).artifactsDir,
+    path.join(clientSuiteRoot, 'qa-flows__checkout.ad', 'attempt-1'),
+  );
+
+  assert.deepEqual(data.artifacts, [
+    {
+      field: 'artifactsDir',
+      artifactType: 'test-artifacts',
+      path: daemonRoot,
+      // The download destination is the ROOT the client redirected to, not `clientSuiteRoot`:
+      // extracting the archive reproduces the invocation-id segment on its own.
+      localPath: clientRoot,
+      fileName: SUITE_INVOCATION_ID,
+    },
+  ]);
+});
+
+test('a local daemon (no clientArtifactPaths hint) returns the suite result unchanged', () => {
+  const tempRoot = mkdtempForTestSync('agent-device-remote-test-artifacts-local-');
+  const daemonRoot = path.join(tempRoot, SUITE_INVOCATION_ID);
+  fs.mkdirSync(path.join(daemonRoot, 'qa-flows__open.ad', 'attempt-1'), { recursive: true });
+  const result = suiteResult(daemonRoot);
+
+  const data = attachRemoteReplayTestArtifacts(result, req(undefined));
+
+  assert.equal(data, result);
+});
+
+test('a suite directory that was never created (all sources skipped) still reports the caller-local root, but registers no download', () => {
+  const tempRoot = mkdtempForTestSync('agent-device-remote-test-artifacts-empty-');
+  const daemonRoot = path.join(tempRoot, SUITE_INVOCATION_ID);
+  const clientRoot = '/Users/ci/work/installer-app/remote-device-artifacts/ad-test';
+  const result: ReplaySuiteResult = {
+    total: 1,
+    executed: 0,
+    passed: 0,
+    failed: 0,
+    skipped: 1,
+    notRun: 0,
+    durationMs: 1,
+    failures: [],
+    artifactsDir: daemonRoot,
+    tests: [
+      {
+        file: 'skipped.ad',
+        status: 'skipped',
+        durationMs: 0,
+        reason: 'skipped-by-filter',
+        message: 'x',
+      },
+    ],
+  };
+
+  const data = attachRemoteReplayTestArtifacts(result, req(clientRoot));
+
+  // Nothing to download, but the reported root is still the caller-local one the suite WOULD
+  // have used — not the unreachable daemon-local temp path.
+  assert.equal(data.artifactsDir, path.join(clientRoot, SUITE_INVOCATION_ID));
+  assert.equal(data.artifacts, undefined);
+});

src/daemon/replay/internal/test-command.ts

@@ -1,10 +1,12 @@
 /** Runs the replay-test scheduler and its nested replay attempts. */
 
+import fs from 'node:fs';
+import path from 'node:path';
 import type { CommandFlags } from '@agent-device/contracts/command';
-import type { ReplayScriptSourceBundle } from '@agent-device/contracts/replay';
+import type { ReplaySuiteResult, ReplayScriptSourceBundle } from '@agent-device/contracts/replay';
 import { REPLAY_SCRIPT_SOURCE_REQUIRED_MESSAGE } from '../../replay-script-source.ts';
 import type { ReplayScriptMetadata } from '@agent-device/ad-script';
-import type { DaemonRequest, DaemonResponse } from '../../types.ts';
+import type { DaemonRequest, DaemonResponse, DaemonResponseData } from '../../types.ts';
 import { expandSessionPath } from '../../session-paths.ts';
 import type { ReplayTestCommand } from './command-types.ts';
 import {
@@ -279,10 +281,89 @@ export async function runReplayTestCommand(command: ReplayTestCommand): Promise<
     cleanupSession,
   });
   return outcome.status === 'completed'
-    ? { ok: true, data: outcome.data }
+    ? { ok: true, data: attachRemoteReplayTestArtifacts(outcome.data, req) }
     : errorResponse(outcome.error.code, outcome.error.message);
 }
 
+/**
+ * #2246: against a remote daemon, the client redirects `--artifacts-dir` to a temp directory on
+ * THIS host before the suite runs (`prepareRemoteRequestArtifacts` in
+ * `src/remote/daemon-artifacts.ts`) — the caller's real `cwd` names no path here, so the
+ * scheduler above must never resolve suite artifacts against it. `req.meta.clientArtifactPaths`
+ * is how that client tells the daemon the real caller-local ROOT (e.g. `--artifacts-dir`, or its
+ * default) it will pull the directory back into once the request returns — that root is resolved
+ * client-side *before* the suite runs, so it never contains the suite's own invocation id.
+ *
+ * `data.artifactsDir` (the daemon-local suite directory) is always `<redirected root>/<suite
+ * invocation id>`: `createDirectoryArchive` (`src/daemon/artifact-tracking.ts`) tars it as
+ * `-C dirname(daemonRoot) -- basename(daemonRoot)`, so extracting that archive under the
+ * caller-local root reproduces the SAME invocation-id segment there. The caller-facing root is
+ * therefore `path.join(clientRoot, basename(daemonRoot))`, not `clientRoot` itself — every path
+ * this rewrites must land under that, matching where the client will actually put the files.
+ *
+ * Rewriting every artifact path in the response to that root, and registering the directory
+ * itself as one downloadable artifact (the same generic `data.artifacts` -> `trackArtifact`
+ * mechanism `screenshot`/`record` already use), are host concerns: the scheduler only ever
+ * reports paths on its own filesystem.
+ *
+ * A local daemon never sets `clientArtifactPaths` (the client only redirects for a remote one),
+ * so this is a no-op there and the response is returned unchanged.
+ */
+export function attachRemoteReplayTestArtifacts(
+  data: ReplaySuiteResult,
+  req: DaemonRequest,
+): DaemonResponseData {
+  const clientRoot = req.meta?.clientArtifactPaths?.artifactsDir;
+  const daemonRoot = data.artifactsDir;
+  if (!clientRoot || !daemonRoot) return data;
+
+  // The directory the client will actually extract the download into, on ITS filesystem —
+  // `clientRoot` plus the same invocation-id segment `daemonRoot` ends in (see doc comment).
+  const clientSuiteRoot = path.join(clientRoot, path.basename(daemonRoot));
+  const remapArtifactsDir = (candidate: string): string =>
+    candidate === daemonRoot || candidate.startsWith(daemonRoot + path.sep)
+      ? clientSuiteRoot + candidate.slice(daemonRoot.length)
+      : candidate;
+  // `tests` and `failures` both come from the scheduler's own `results` array
+  // (`summarizeReplayTestResults`) and share object references for every failed entry — each
+  // array is rewritten independently so both, not just whichever a caller happens to read, carry
+  // the caller-local path.
+  const rewritten: DaemonResponseData = {
+    ...data,
+    artifactsDir: clientSuiteRoot,
+    tests: data.tests.map((test) =>
+      'artifactsDir' in test && test.artifactsDir
+        ? { ...test, artifactsDir: remapArtifactsDir(test.artifactsDir) }
+        : test,
+    ),
+    failures: data.failures.map((failure) =>
+      failure.artifactsDir
+        ? { ...failure, artifactsDir: remapArtifactsDir(failure.artifactsDir) }
+        : failure,
+    ),
+  };
+
+  // No attempt ever ran (e.g. every source was filtered out), so the directory the client would
+  // try to download was never created — the rewritten paths above are still correct (they name
+  // where the suite WOULD have put its output), just nothing to register a download for.
+  if (!fs.existsSync(daemonRoot)) return rewritten;
+
+  return {
+    ...rewritten,
+    artifacts: [
+      {
+        field: 'artifactsDir',
+        artifactType: 'test-artifacts',
+        path: daemonRoot,
+        // The download destination is the ROOT, not `clientSuiteRoot`: the archive's own
+        // top-level entry already supplies the invocation-id segment on extraction.
+        localPath: clientRoot,
+        fileName: path.basename(daemonRoot),
+      },
+    ],
+  };
+}
+
 /**
  * Translates a daemon `test` request into the scheduler's neutral request (#1478 P3b).
  *