diff --git a/CHANGELOG.md b/CHANGELOG.md index dd4a0f734a..ad981b052e 100644 --- a/CHANGELOG.md +++ b/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, diff --git a/src/cli/replay-test/__tests__/reporting.test.ts b/src/cli/replay-test/__tests__/reporting.test.ts new file mode 100644 index 0000000000..11cacc3cec --- /dev/null +++ b/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/); + } +}); diff --git a/src/cli/replay-test/reporters/__tests__/registry.test.ts b/src/cli/replay-test/reporters/__tests__/registry.test.ts index 25bec8c311..a1598a68f5 100644 --- a/src/cli/replay-test/reporters/__tests__/registry.test.ts +++ b/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'), + ); + } + }, +); diff --git a/src/cli/replay-test/reporters/registry.ts b/src/cli/replay-test/reporters/registry.ts index e712af1c4d..9f708fa53d 100644 --- a/src/cli/replay-test/reporters/registry.ts +++ b/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; } diff --git a/src/cli/replay-test/reporters/types.ts b/src/cli/replay-test/reporters/types.ts index 970c60ccd7..53a9c32013 100644 --- a/src/cli/replay-test/reporters/types.ts +++ b/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; + /** Return an integer from 0 to 255, or undefined; a reporter can only raise the suite exit code. */ getExitCode?(suite: ReplaySuiteResult): number | undefined; }; diff --git a/src/cli/replay-test/reporting.ts b/src/cli/replay-test/reporting.ts index ebed906f82..34d53df189 100644 --- a/src/cli/replay-test/reporting.ts +++ b/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: { diff --git a/src/commands/replay/index.ts b/src/commands/replay/index.ts index d5c7e6e15b..764f7ed5e2 100644 --- a/src/commands/replay/index.ts +++ b/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:) 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:) 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)), diff --git a/website/docs/docs/replay-e2e.md b/website/docs/docs/replay-e2e.md index ab9802198f..e56d5936d6 100644 --- a/website/docs/docs/replay-e2e.md +++ b/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