fix(test): reject reporter exit codes that can wrap to success (#2497)

Co-authored-by: PLASMA-FR <173463847+PLASMA-FR@users.noreply.github.com>

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+- 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.
 - Fixed: Android `record start` no longer refuses to begin after a reused emulator reassigned the
   previous recorder's pid. A completed recording's native marker is retired only once its recorder is
   proven gone, but only an absent pid counted as proof — a pid that now names an unrelated process,

src/cli/replay-test/__tests__/reporting.test.ts

@@ -0,0 +1,53 @@
+import assert from 'node:assert/strict';
+import fs from 'node:fs/promises';
+import path from 'node:path';
+import { test } from 'vitest';
+import { runCliCapture } from '../../../__tests__/cli-capture.ts';
+import { mkdtempForTest } from '../../../__tests__/test-utils/tmp-dir.ts';
+
+test.each([false, true])('CLI refuses a wrapping reporter exit code (json=%s)', async (json) => {
+  const root = await mkdtempForTest('agent-device-reporter-exit-');
+  const flow = path.join(root, 'flow.ad');
+  const reporter = path.join(root, 'reporter.mjs');
+  await fs.writeFile(flow, 'open Demo\n');
+  await fs.writeFile(reporter, "export default { name: 'wrapping', getExitCode: () => 256 };\n");
+  const failed = {
+    file: flow,
+    session: 'test:reporter',
+    status: 'failed',
+    durationMs: 1,
+    attempts: 1,
+    error: { message: 'fixture assertion failed' },
+  };
+  const result = await runCliCapture(
+    ['test', flow, '--reporter', reporter, ...(json ? ['--json'] : [])],
+    async () => ({
+      ok: true,
+      data: {
+        total: 1,
+        executed: 1,
+        passed: 0,
+        failed: 1,
+        skipped: 0,
+        notRun: 0,
+        durationMs: 1,
+        failures: [failed],
+        tests: [failed],
+      },
+    }),
+  );
+
+  assert.equal(result.calls.length, 1);
+  assert.equal(result.calls[0]?.command, 'test');
+  assert.equal(result.code, 1);
+  if (json) {
+    const output = JSON.parse(result.stdout);
+    assert.equal(output.success, false);
+    assert.equal(output.error.code, 'INVALID_ARGS');
+    assert.match(output.error.message, /wrapping.*getExitCode.*0 to 255/);
+  } else {
+    assert.equal(result.stdout, '');
+    assert.match(result.stderr, /INVALID_ARGS/);
+    assert.match(result.stderr, /wrapping.*getExitCode.*0 to 255/);
+  }
+});

src/cli/replay-test/reporters/__tests__/registry.test.ts

@@ -1,5 +1,6 @@
 import assert from 'node:assert/strict';
 import { test } from 'vitest';
+import { AppError } from '@agent-device/kernel/errors';
 import type { ReplaySuiteResult } from '@agent-device/contracts/replay';
 import {
   getReplayTestReporterExitCode,
@@ -252,3 +253,30 @@ test('reporter exit codes can raise but never lower the suite exit code', () =>
     1,
   );
 });
+
+test.each([0, 1, 3, 255, undefined])('preserves valid reporter exit code %s', (code) => {
+  const reporters: ReplayTestReporter[] = [{ name: 'valid', getExitCode: () => code }];
+  assert.equal(getReplayTestReporterExitCode(reporters, suite()), code ?? 0);
+  assert.equal(getReplayTestReporterExitCode(reporters, suite(1)), Math.max(1, code ?? 0));
+});
+
+test.each([-1, 0.5, 256, 512, Number.NaN, Infinity, -Infinity, '3', null])(
+  'rejects invalid reporter exit code %s instead of coercing or wrapping it',
+  (code) => {
+    const reporters: ReplayTestReporter[] = [
+      { name: 'valid', getExitCode: () => 3 },
+      { name: 'invalid', getExitCode: () => code as number },
+    ];
+    for (const value of [suite(), suite(1)]) {
+      assert.throws(
+        () => getReplayTestReporterExitCode(reporters, value),
+        (error: unknown) =>
+          error instanceof AppError &&
+          error.code === 'INVALID_ARGS' &&
+          error.message.includes('invalid') &&
+          error.message.includes('getExitCode') &&
+          error.message.includes('0 to 255'),
+      );
+    }
+  },
+);

src/cli/replay-test/reporters/registry.ts

@@ -1,5 +1,6 @@
 import type { ReplaySuiteResult } from '@agent-device/contracts/replay';
 import type { RequestProgressEvent } from '@agent-device/contracts/progress';
+import { AppError } from '@agent-device/kernel/errors';
 import { createCustomReplayTestReporter } from './custom.ts';
 import { createDefaultReplayTestReporter } from './default.ts';
 import { getReplayTestExitCode } from './format.ts';
@@ -99,7 +100,14 @@ export function getReplayTestReporterExitCode(
   let exitCode = getReplayTestExitCode(suite);
   for (const reporter of reporters) {
     const reporterExitCode = reporter.getExitCode?.(suite);
-    if (reporterExitCode !== undefined) exitCode = Math.max(exitCode, reporterExitCode);
+    if (reporterExitCode === undefined) continue;
+    if (!Number.isInteger(reporterExitCode) || reporterExitCode < 0 || reporterExitCode > 255) {
+      throw new AppError(
+        'INVALID_ARGS',
+        `Test reporter ${reporter.name} getExitCode must return an integer from 0 to 255 or undefined.`,
+      );
+    }
+    exitCode = Math.max(exitCode, reporterExitCode);
   }
   return exitCode;
 }

src/cli/replay-test/reporters/types.ts

@@ -74,6 +74,7 @@ export type ReplayTestReporter = {
   onTestStep?(test: ReplayTestStep, context: ReplayTestReporterContext): void;
   onTestResult?(test: ReplayTestResult, context: ReplayTestReporterContext): void;
   onSuiteEnd?(suite: ReplaySuiteResult, context: ReplayTestReporterContext): void | Promise<void>;
+  /** Return an integer from 0 to 255, or undefined; a reporter can only raise the suite exit code. */
   getExitCode?(suite: ReplaySuiteResult): number | undefined;
 };
 

src/cli/replay-test/reporting.ts

@@ -38,11 +38,12 @@ export async function renderReplayTestResponse(options: {
     options.reporterRuntime ??
     (await createReplayTestReporterRuntime({ debug, verbose, reporter, reportJunit, json }));
   await runReplayTestReporters(runtime.reporters, suite, runtime.context);
+  const exitCode = getReplayTestReporterExitCode(runtime.reporters, suite);
   if (json) {
     const { printJson } = await import('../../commands/output/json.ts');
     printJson({ success: true, data: suite });
   }
-  return getReplayTestReporterExitCode(runtime.reporters, suite);
+  return exitCode;
 }
 
 export async function createReplayTestReporterRuntime(options: {

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.',
+      "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.",
   },
   metadata: testCommandMetadata,
   run: (client, input) => client.replay.test(withCommandRuntimeHints(input)),

website/docs/docs/replay-e2e.md

@@ -210,7 +210,7 @@ export default createReporter;
 
 The CLI loads reporter modules with Node dynamic `import()`. Use `.mjs` or `.js` files at runtime; for TypeScript, compile the reporter to JavaScript before passing it to `--reporter`. Loading `.ts` files directly depends on Node's type-stripping behavior and is not part of the supported reporter contract.
 
-Live reporter hooks are semantic: `onSuiteStart`, `onTestStart`, `onTestStep`, and `onTestResult` run while the daemon request is active; generic command progress frames are not exposed to test reporters. These live hooks are synchronous — they run from the progress stream as events arrive and are not awaited, so keep their work synchronous and defer anything async to `onSuiteEnd`, which the CLI awaits before exiting. `onSuiteEnd` receives the final suite result. `getExitCode` can only raise the suite exit code, never lower it: the highest reporter-provided code wins and failed tests still exit with `1` when no reporter raises it further, so a reporter cannot mask a failing suite.
+Live reporter hooks are semantic: `onSuiteStart`, `onTestStart`, `onTestStep`, and `onTestResult` run while the daemon request is active; generic command progress frames are not exposed to test reporters. These live hooks are synchronous — they run from the progress stream as events arrive and are not awaited, so keep their work synchronous and defer anything async to `onSuiteEnd`, which the CLI awaits before exiting. `onSuiteEnd` receives the final suite result. `getExitCode` can only raise the suite exit code, never lower it: the highest reporter-provided code wins and failed tests still exit with `1` when no reporter raises it further, so a reporter cannot mask a failing suite. Return an integer from `0` to `255`, or `undefined` to leave the exit code unchanged. Other values fail with `INVALID_ARGS`; in particular, codes such as `256` are rejected before they can wrap to a successful process exit.
 
 ## Parametrise `.ad` scripts