diff --git a/scripts/rea.mjs b/scripts/rea.mjs index cb97056f8e..fa956d3293 100755 --- a/scripts/rea.mjs +++ b/scripts/rea.mjs @@ -129,7 +129,12 @@ if (!(await compiledRuntimeExists(runtimeFiles))) { try { // Doctor is the readiness report for this failure. Other commands still stop // here so they do not run with an unusable log level. - if (configurationError !== undefined && !isDoctorCommand(cliArgs)) { + if ( + configurationError !== undefined && + (isShellCompletion || + builtinArguments.splitIncurBuiltinArguments(cliArgs).command[0] !== + "doctor") + ) { process.stderr.write(`${configurationError}\n`); process.exitCode = 1; } else if (!outputArguments.ok) { @@ -236,27 +241,6 @@ if (!(await compiledRuntimeExists(runtimeFiles))) { throw dispatchFailure; } -/** True when the first positional command is `doctor`, after global output flags. */ -function isDoctorCommand(arguments_) { - const valuedFlags = new Set([ - "--format", - "--filter-output", - "--token-limit", - "--token-offset", - ]); - for (let index = 0; index < arguments_.length; index += 1) { - const argument = arguments_[index]; - if (argument === "--") return arguments_[index + 1] === "doctor"; - if (valuedFlags.has(argument)) { - index += 1; - continue; - } - if (argument.startsWith("-")) continue; - return argument === "doctor"; - } - return false; -} - /** * A reader that closes the pipe early, as `rea ... | head` does, has declined * the rest of the output. Stop writing without reporting a failure, and exit diff --git a/tests/boundary/cli/cliLogLevelConfiguration.test.ts b/tests/boundary/cli/cliLogLevelConfiguration.test.ts index fb6490091f..9aba657f40 100644 --- a/tests/boundary/cli/cliLogLevelConfiguration.test.ts +++ b/tests/boundary/cli/cliLogLevelConfiguration.test.ts @@ -1,6 +1,6 @@ import { expect } from "vitest"; -import { cliTest } from "../../support/cli/cliFixture.js"; +import { cliTest, runFileCli } from "../../support/cli/cliFixture.js"; const invalidLogLevelMessage = 'REA configuration is invalid: REA_LOG_LEVEL: Invalid option: expected one of "trace"|"debug"|"info"|"warn"|"error"|"fatal"|"silent".\n'; @@ -17,23 +17,28 @@ for (const level of ["loud", "INFO"]) expect(help.stdout).toBe(""); expect(help.stderr).toBe(invalidLogLevelMessage); - const doctor = await cli.run({ + const doctorInvocation = { arguments: ["--json", "doctor", "--provider", "hopper"], environment: { REA_LOG_LEVEL: level }, - }); - expect(doctor.exitCode, doctor.stderr).toBe(1); - expect(doctor.stderr).toBe(""); - expect(doctor.json).toMatchObject({ - healthy: false, - scope_checks: expect.arrayContaining([ - expect.objectContaining({ - name: "runtime-configuration:REA_LOG_LEVEL", - ok: false, - classification: "config_drift", - detail: expect.stringContaining("REA_LOG_LEVEL"), - }), - ]), - }); + }; + for (const doctor of [ + await cli.run(doctorInvocation), + await runFileCli(cli, doctorInvocation), + ]) { + expect(doctor.exitCode, doctor.stderr).toBe(1); + expect(doctor.stderr).toBe(""); + expect(doctor.json).toMatchObject({ + healthy: false, + scope_checks: expect.arrayContaining([ + expect.objectContaining({ + name: "runtime-configuration:REA_LOG_LEVEL", + ok: false, + classification: "config_drift", + detail: expect.stringContaining("REA_LOG_LEVEL"), + }), + ]), + }); + } const client = await cli.run({ arguments: ["--json", "doctor", "--client", "codex"], @@ -58,3 +63,46 @@ for (const level of ["loud", "INFO"]) }); }, ); + +cliTest( + "keeps fresh and shared CLI configuration precedence for doctor-looking data", + async ({ cli }) => { + for (const arguments_ of [ + ["--filter-output", "doctor", "providers"], + ["providers", "doctor"], + ["--format", "", "doctor"], + ["--filter-output", "", "doctor"], + ["--token-limit", "", "doctor"], + ["--token-offset", "", "doctor"], + ["--unknown", "doctor"], + ["--version", "doctor"], + ["--", "doctor"], + ]) { + const invocation = { + arguments: arguments_, + environment: { REA_LOG_LEVEL: "loud" }, + }; + const fresh = await cli.run(invocation); + const shared = await runFileCli(cli, invocation); + for (const result of [fresh, shared]) { + expect(result.exitCode, JSON.stringify(arguments_)).toBe(1); + expect(result.stdout, JSON.stringify(arguments_)).toBe(""); + expect(result.stderr, JSON.stringify(arguments_)).toBe( + invalidLogLevelMessage, + ); + } + } + + const completion = { + arguments: ["doctor"], + environment: { REA_LOG_LEVEL: "loud", COMPLETE: "bash" }, + }; + const fresh = await cli.run(completion); + const shared = await runFileCli(cli, completion); + for (const result of [fresh, shared]) { + expect(result.exitCode).toBe(1); + expect(result.stdout).toBe(""); + expect(result.stderr).toBe(invalidLogLevelMessage); + } + }, +); diff --git a/tests/boundary/cli/dispatcher.test.ts b/tests/boundary/cli/dispatcher.test.ts index 93e4dba95c..16b7205361 100644 --- a/tests/boundary/cli/dispatcher.test.ts +++ b/tests/boundary/cli/dispatcher.test.ts @@ -94,6 +94,96 @@ describe("version report dispatch", () => { ); }); +describe("doctor configuration dispatch", () => { + workspaceCliTest( + "exempts only the production root doctor tokens from invalid logging configuration", + async ({ processes, workspace }) => { + const imported = "CLI command runtime imported"; + await installCliRuntime( + workspace, + `process.stderr.write(${JSON.stringify(`${imported}\n`)}); export const closeCliJavaScriptResources = () => {}; export const createCli = () => ({ serve: (args, options) => options.stdout(JSON.stringify({ adapter: "cli", args })) });\n`, + ); + // Both token ownership and output/configuration admission use production code. + await workspace.write( + "dist/cli-runtime/cliOutput.js", + `export * from ${JSON.stringify(pathToFileURL(resolve("dist/cli-runtime/cliOutput.js")).href)};\n`, + ); + const invoke = (args: readonly string[], completion = "") => + processes.run( + process.execPath, + [workspace.path("scripts/rea.mjs"), ...args], + { + cwd: workspace.root, + env: { + ...process.env, + REA_LOG_LEVEL: "invalid-level", + COMPLETE: completion, + }, + }, + ); + + for (const [args, normalized] of [ + [["doctor"], ["doctor"]], + [ + ["--json", "doctor"], + ["--json", "doctor"], + ], + [ + ["--format=json", "doctor"], + ["--format", "json", "doctor"], + ], + [ + ["--filter-output", "healthy", "doctor"], + ["--filter-output", "healthy", "doctor"], + ], + [ + ["--token-limit", "10", "doctor"], + ["--token-limit", "10", "doctor"], + ], + [ + ["--token-offset", "1", "doctor"], + ["--token-offset", "1", "doctor"], + ], + ] as const) { + const result = await invoke(args); + expect( + result.exitCode, + `${JSON.stringify(args)}: ${result.stderr}`, + ).toBe(0); + expect(result.stderr).toBe(`${imported}\n`); + expect(JSON.parse(result.stdout)).toEqual({ + adapter: "cli", + args: normalized, + }); + } + + for (const args of [ + ["--filter-output", "doctor", "providers"], + ["providers", "doctor"], + ["--format", "", "doctor"], + ["--filter-output", "", "doctor"], + ["--token-limit", "", "doctor"], + ["--token-offset", "", "doctor"], + ["--unknown", "doctor"], + ["--version", "doctor"], + ["--", "doctor"], + ]) { + const result = await invoke(args); + expect(result.exitCode, JSON.stringify(args)).toBe(1); + expect(result.stdout, JSON.stringify(args)).toBe(""); + expect(result.stderr, JSON.stringify(args)).toContain("REA_LOG_LEVEL"); + expect(result.stderr, JSON.stringify(args)).not.toContain(imported); + } + + const completion = await invoke(["doctor"], "bash"); + expect(completion.exitCode).toBe(1); + expect(completion.stdout).toBe(""); + expect(completion.stderr).toContain("REA_LOG_LEVEL"); + expect(completion.stderr).not.toContain(imported); + }, + ); +}); + describe("executable dispatcher", () => { workspaceCliTest( "does not append recovery output after delegated stdout has begun", diff --git a/tests/support/cli/cliCommandSessionWorker.mjs b/tests/support/cli/cliCommandSessionWorker.mjs index 54d44b60d3..c98b6aa3ba 100644 --- a/tests/support/cli/cliCommandSessionWorker.mjs +++ b/tests/support/cli/cliCommandSessionWorker.mjs @@ -39,6 +39,7 @@ const prepareCliProcess = () => { const runtimeFiles = [ "dist/cli-runtime/cli.js", "dist/cli-runtime/cliOutput.js", + "dist/cli-runtime/cliBuiltinArguments.js", "dist/cli-runtime/cliResultOutput.js", ]; @@ -54,6 +55,8 @@ const { closeCliJavaScriptResources, createCli } = await import("../../../dist/cli-runtime/cli.js"); const { createCliResultOutput } = await import("../../../dist/cli-runtime/cliResultOutput.js"); +const { cliCompletionShell, splitIncurBuiltinArguments } = + await import("../../../dist/cli-runtime/cliBuiltinArguments.js"); const { cliConfigurationError, normalizeCliArguments, @@ -161,11 +164,18 @@ async function runOneShot(args) { process.exitCode = undefined; let explicitExit; try { - const cliArgs = normalizeCliArguments(args); - const outputOptions = parseCliOutputArguments(cliArgs); + const isShellCompletion = cliCompletionShell(process.env) !== undefined; + const cliArgs = isShellCompletion ? args : normalizeCliArguments(args); + const outputOptions = parseCliOutputArguments( + isShellCompletion ? [] : cliArgs, + ); const outputArguments = outputOptions.validation; const configurationError = cliConfigurationError(process.env); - if (configurationError !== undefined && !isDoctorCommand(cliArgs)) { + if ( + configurationError !== undefined && + (isShellCompletion || + splitIncurBuiltinArguments(cliArgs).command[0] !== "doctor") + ) { process.stderr.write(`${configurationError}\n`); process.exitCode = 1; } else if (!outputArguments.ok) { @@ -231,26 +241,6 @@ function captureWrite(chunks) { }; } -function isDoctorCommand(arguments_) { - const valuedFlags = new Set([ - "--format", - "--filter-output", - "--token-limit", - "--token-offset", - ]); - for (let index = 0; index < arguments_.length; index += 1) { - const argument = arguments_[index]; - if (argument === "--") return arguments_[index + 1] === "doctor"; - if (valuedFlags.has(argument)) { - index += 1; - continue; - } - if (argument.startsWith("-")) continue; - return argument === "doctor"; - } - return false; -} - async function compiledRuntimeExists(paths) { for (const path of paths) { try {