diff --git a/src/application/DoctorScope.ts b/src/application/DoctorScope.ts index 49df606bab..15255ce300 100644 --- a/src/application/DoctorScope.ts +++ b/src/application/DoctorScope.ts @@ -1,5 +1,5 @@ import { uniqueSorted } from "../domain/canonicalOrdering.js"; -import { blocksProcessStart } from "../logger.js"; +import { blocksProcessStart } from "../config/processStart.js"; import type { ClientRegistrationStatus, UnhealthyClientRegistrationStatus, diff --git a/src/cli.ts b/src/cli.ts index 224a4ef8fd..2e4f46acba 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -22,7 +22,8 @@ import type { FridaInstrumentationPort } from "./application/frida/FridaInstrume import { Cli } from "incur"; import { normalizeCliArguments } from "./cliOutput.js"; -import { createLogger, parseLogLevel } from "./logger.js"; +import { createLogger } from "./logger.js"; +import { parseLogLevel } from "./config/processStart.js"; import { projectAnalysisError } from "./domain/analysisErrorProjection.js"; import { PRODUCT_IDENTITY } from "./identity.js"; import { npxRegistrationCommand } from "./application/ClientRegistrationIdentity.js"; @@ -67,7 +68,7 @@ export { closeCliJavaScriptResources } from "./cli/javascriptApplicationAnalysis /** * Describe CLI settings that are invalid before any command runs, as MCP does. - * Keep this aligned with PROCESS_START_CONFIGURATION_SETTINGS. + * CLI, MCP and Doctor share the process-start configuration contract. */ export const cliConfigurationError = ( selectedEnvironment: Readonly>, diff --git a/src/cliLogging.test.ts b/src/cliLogging.test.ts deleted file mode 100644 index d6daa6fa1a..0000000000 --- a/src/cliLogging.test.ts +++ /dev/null @@ -1,156 +0,0 @@ -import { describe, expect, it } from "vitest"; - -import { ArtifactOperationError } from "./domain/artifactOperationError.js"; -import { ConfigurationError } from "./domain/configurationErrors.js"; -import { projectAnalysisError } from "./domain/analysisErrorProjection.js"; - -import { isCliOperationFailure, logCliCommand } from "./cliLogging.js"; -import { silentLogger } from "./logger.js"; -import { runSetup } from "./application/Setup.js"; -import { isSetupFailure, type SetupResult } from "./application/SetupTypes.js"; -import { - FakeSetupHost, - options as setupOptions, -} from "./application/Setup.fixture.js"; -import { err } from "./domain/result.js"; -import { runUpdate, type UpdateHost } from "./application/Update.js"; -import { isUpdateFailure } from "./application/UpdateFailure.js"; -import { runUninstall, type UninstallHost } from "./application/Uninstall.js"; -import { isUninstallFailure } from "./application/UninstallFailure.js"; -import { CLI_COMMANDS } from "./cliCommandNames.js"; - -it("does not treat ordinary error-looking data or nested Evidence as command failure", () => { - expect( - isCliOperationFailure({ - code: "configuration_invalid", - message: "source data", - }), - ).toBe(false); - expect( - isCliOperationFailure({ - result: projectAnalysisError(new ConfigurationError("source setting")), - }), - ).toBe(false); -}); - -describe("CLI command outcome classification", () => { - it("returns nonzero for the UpdateResult failure variant", async () => { - const installation = { - manager: "npm", - prefix: "/fixture", - packageRoot: "/fixture/lib/node_modules/rea-agents", - } as const; - const updateHost: UpdateHost = { - installation: () => Promise.resolve(installation), - latestVersion: () => - Promise.resolve( - err({ kind: "unavailable", detail: "registry unavailable" }), - ), - installVersion: () => Promise.resolve(err("not called")), - installedVersion: () => Promise.resolve(err("not called")), - planMaintenance: () => - Promise.resolve({ status: "current", plannedActions: [] }), - }; - const result = await runUpdate("3.2.1", updateHost); - expect(result.status).toBe("failed"); - const previousExitCode = process.exitCode; - const output = await logCliCommand( - silentLogger, - CLI_COMMANDS.update, - async () => result, - isUpdateFailure, - ); - expect(output).toBe(result); - expect(process.exitCode).toBe(1); - process.exitCode = previousExitCode; - }); - - it.each(["needs_confirmation", "needs_human"] as const)( - "returns nonzero for a real setup result with status %s", - async (expectedStatus) => { - const host = new FakeSetupHost(); - const setup: SetupResult = - expectedStatus === "needs_human" - ? await runSetup( - { ...setupOptions(false), structured: false }, - host, - () => Promise.resolve(true), - ) - : await runSetup( - { ...setupOptions(false), clientIds: ["cursor"] }, - Object.assign(host, { - clients: [{ name: "cursor", configPath: "/cursor.json" }], - }), - ); - expect(setup.status).toBe(expectedStatus); - const previousExitCode = process.exitCode; - const output = await logCliCommand( - silentLogger, - CLI_COMMANDS.setup, - async () => setup, - isSetupFailure, - ); - expect(output).toBe(setup); - expect(process.exitCode).toBe(1); - process.exitCode = previousExitCode; - }, - ); - - it("returns nonzero when the real uninstall outcome contains a failed action", async () => { - const host: UninstallHost = { - clients: () => Promise.resolve([]), - inspectClient: () => Promise.resolve(undefined), - removeClient: () => - Promise.resolve({ name: "client", status: "failed", detail: "write" }), - removeSkill: () => - Promise.resolve({ name: "skill", status: "failed", detail: "write" }), - purgeData: () => Promise.resolve([]), - }; - const result = await runUninstall(false, host); - expect(result.status).toBe("failed"); - const previousExitCode = process.exitCode; - await logCliCommand( - silentLogger, - CLI_COMMANDS.uninstall, - async () => result, - isUninstallFailure, - ); - expect(process.exitCode).toBe(1); - process.exitCode = previousExitCode; - }); -}); - -describe("CLI thrown analysis failures", () => { - it("returns the shared projection with actionable failure coordinates", async () => { - const previousExitCode = process.exitCode; - try { - const projected = await logCliCommand( - silentLogger, - "fixture", - async () => { - throw new ArtifactOperationError( - "analyze_javascript_application", - "integrity", - { - logicalPath: "src/main.js", - declaredSha256: "a".repeat(64), - calculatedSha256: "b".repeat(64), - unpacked: true, - }, - ); - }, - ); - expect(projected).toMatchObject({ - code: "artifact_integrity_mismatch", - category: "integrity_mismatch", - details: { - logical_path: "src/main.js", - declared_sha256: "a".repeat(64), - calculated_sha256: "b".repeat(64), - }, - }); - } finally { - process.exitCode = previousExitCode; - } - }); -}); diff --git a/src/config/environment.ts b/src/config/environment.ts index 43d8f635fc..ee09ba4105 100644 --- a/src/config/environment.ts +++ b/src/config/environment.ts @@ -30,6 +30,7 @@ import { mcpToolsListBudgetMsSchema, } from "./mcpStartupBudgets.js"; import { parseLoaderArgs, parseStringArray } from "./parsers.js"; +import { processStartConfigurationSchema } from "./processStart.js"; const environmentSchema = z.object({ REA_ANALYSIS_PROVIDER: analysisProviderSelectorSchema.default("auto"), @@ -93,9 +94,7 @@ const environmentSchema = z.object({ HOPPER_TARGET_PATH: z.string().min(1).optional(), HOPPER_TARGET_KIND: z.enum(["executable", "database"]).default("executable"), HOPPER_LOADER_ARGS_JSON: z.string().optional(), - REA_LOG_LEVEL: z - .enum(["trace", "debug", "info", "warn", "error", "fatal", "silent"]) - .default("info"), + REA_LOG_LEVEL: processStartConfigurationSchema.shape.REA_LOG_LEVEL, REA_REFERENCE_SECRET_PATTERNS_JSON: z.string().default("[]"), }); diff --git a/src/config/processStart.ts b/src/config/processStart.ts new file mode 100644 index 0000000000..78a9bf24c9 --- /dev/null +++ b/src/config/processStart.ts @@ -0,0 +1,38 @@ +import { z } from "zod"; + +import { ConfigurationError } from "../domain/configurationErrors.js"; +import { err, ok, type Result } from "../domain/result.js"; + +/** Settings rejected before CLI commands and MCP startup, required in every Doctor scope. */ +export const processStartConfigurationSchema = z.object({ + REA_LOG_LEVEL: z + .enum(["trace", "debug", "info", "warn", "error", "fatal", "silent"]) + .default("info"), +}); + +/** Supported structured log levels at the process boundary. */ +export type LogLevel = z.infer< + typeof processStartConfigurationSchema +>["REA_LOG_LEVEL"]; + +/** True when `setting` prevents the REA process from starting. */ +export const blocksProcessStart = (setting: string): boolean => + Object.hasOwn(processStartConfigurationSchema.shape, setting); + +/** Parse the optional REA_LOG_LEVEL environment variable. */ +export const parseLogLevel = ( + value: unknown, +): Result => { + const parsed = + processStartConfigurationSchema.shape.REA_LOG_LEVEL.safeParse(value); + if (parsed.success) return ok(parsed.data); + return err( + new ConfigurationError("Invalid REA environment configuration", { + cause: parsed.error, + settings: parsed.error.issues.map(({ message }) => ({ + setting: "REA_LOG_LEVEL", + constraint: message, + })), + }), + ); +}; diff --git a/src/config/types.ts b/src/config/types.ts index 0e3ff20192..a977fb1b3d 100644 --- a/src/config/types.ts +++ b/src/config/types.ts @@ -1,4 +1,4 @@ -import type { LogLevel } from "../logger.js"; +import type { LogLevel } from "./processStart.js"; import type { ReferenceSourcePolicy } from "../domain/referenceSourcePolicy.js"; import type { AnalysisProviderSelector } from "../contracts/providerSelection.js"; diff --git a/src/logger.ts b/src/logger.ts index 6e5280cd8e..497a013328 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -1,44 +1,5 @@ import pino, { type Logger } from "pino"; -import { z } from "zod"; - -import { ConfigurationError } from "./domain/configurationErrors.js"; -import { err, ok, type Result } from "./domain/result.js"; - -/** - * Settings the executable rejects before a command starts. MCP startup rejects - * the same values. Doctor requires them in every readiness scope. - */ -export const PROCESS_START_CONFIGURATION_SETTINGS = ["REA_LOG_LEVEL"] as const; - -/** True when `setting` prevents the REA process from starting. */ -export const blocksProcessStart = (setting: string): boolean => - PROCESS_START_CONFIGURATION_SETTINGS.some( - (candidate) => candidate === setting, - ); - -/** Supported structured log levels at the process boundary. */ -export type LogLevel = z.infer; - -const logLevelSchema = z - .enum(["trace", "debug", "info", "warn", "error", "fatal", "silent"]) - .default("info"); - -/** Parse the optional REA_LOG_LEVEL environment variable. */ -export const parseLogLevel = ( - value: unknown, -): Result => { - const parsed = logLevelSchema.safeParse(value); - if (parsed.success) return ok(parsed.data); - return err( - new ConfigurationError("Invalid REA environment configuration", { - cause: parsed.error, - settings: parsed.error.issues.map(({ message }) => ({ - setting: "REA_LOG_LEVEL", - constraint: message, - })), - }), - ); -}; +import type { LogLevel } from "./config/processStart.js"; /** Create a JSON logger on stderr so protocol and CLI result stdout stay valid. */ export const createLogger = (mode: "mcp" | "cli", level: LogLevel): Logger => diff --git a/tests/process-global/cliLogging.test.ts b/tests/process-global/cliLogging.test.ts index 84363d3fd7..a016b47648 100644 --- a/tests/process-global/cliLogging.test.ts +++ b/tests/process-global/cliLogging.test.ts @@ -1,52 +1,192 @@ -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { logCliCommand } from "../../src/cliLogging.js"; -import { silentLogger } from "../../src/logger.js"; import { ArtifactOperationError } from "../../src/domain/artifactOperationError.js"; +import { ConfigurationError } from "../../src/domain/configurationErrors.js"; import { projectAnalysisError } from "../../src/domain/analysisErrorProjection.js"; + +import { isCliOperationFailure, logCliCommand } from "../../src/cliLogging.js"; +import { silentLogger } from "../../src/logger.js"; import { runSetup } from "../../src/application/Setup.js"; -import { isSetupFailure } from "../../src/application/SetupTypes.js"; +import { + isSetupFailure, + type SetupResult, +} from "../../src/application/SetupTypes.js"; import { FakeSetupHost, options as setupOptions, } from "../../src/application/Setup.fixture.js"; +import { err } from "../../src/domain/result.js"; +import { runUpdate, type UpdateHost } from "../../src/application/Update.js"; +import { isUpdateFailure } from "../../src/application/UpdateFailure.js"; +import { + runUninstall, + type UninstallHost, +} from "../../src/application/Uninstall.js"; +import { isUninstallFailure } from "../../src/application/UninstallFailure.js"; +import { CLI_COMMANDS } from "../../src/cliCommandNames.js"; const originalExitCode = process.exitCode; +beforeEach(() => { + process.exitCode = undefined; +}); + afterEach(() => { process.exitCode = originalExitCode; }); -describe("CLI operation status", () => { - it("treats a requested setup dry run and cancellation as successful outcomes", async () => { - for (const status of ["planned", "cancelled"]) { - process.exitCode = undefined; - await logCliCommand(silentLogger, "setup", () => - Promise.resolve({ status }), - ); - expect(process.exitCode).toBeUndefined(); - } - }); +it("does not treat ordinary error-looking data or nested Evidence as command failure", () => { + expect( + isCliOperationFailure({ + code: "configuration_invalid", + message: "source data", + }), + ).toBe(false); + expect( + isCliOperationFailure({ + result: projectAnalysisError(new ConfigurationError("source setting")), + }), + ).toBe(false); +}); - it("keeps unapproved setup applications unsuccessful", async () => { - const host = new FakeSetupHost(); - host.availableClients = [{ name: "cursor", configPath: "/cursor.json" }]; - const result = await runSetup( - { ...setupOptions(false), clientIds: ["cursor"] }, - host, - ); - expect(result.status).toBe("needs_confirmation"); - expect(isSetupFailure(result)).toBe(true); - process.exitCode = undefined; - await logCliCommand( +describe("CLI command outcome classification", () => { + it("returns nonzero for the UpdateResult failure variant", async () => { + const installation = { + manager: "npm", + prefix: "/fixture", + packageRoot: "/fixture/lib/node_modules/rea-agents", + } as const; + const updateHost: UpdateHost = { + installation: () => Promise.resolve(installation), + latestVersion: () => + Promise.resolve( + err({ kind: "unavailable", detail: "registry unavailable" }), + ), + installVersion: () => Promise.resolve(err("not called")), + installedVersion: () => Promise.resolve(err("not called")), + planMaintenance: () => + Promise.resolve({ status: "current", plannedActions: [] }), + }; + const result = await runUpdate("3.2.1", updateHost); + expect(result.status).toBe("failed"); + const output = await logCliCommand( silentLogger, - "setup", - () => Promise.resolve(result), - isSetupFailure, + CLI_COMMANDS.update, + async () => result, + isUpdateFailure, ); + expect(output).toBe(result); expect(process.exitCode).toBe(1); }); + it.each(["needs_confirmation", "needs_human"] as const)( + "returns nonzero for a real setup result with status %s", + async (expectedStatus) => { + const host = new FakeSetupHost(); + const setup: SetupResult = + expectedStatus === "needs_human" + ? await runSetup( + { ...setupOptions(false), structured: false }, + host, + () => Promise.resolve(true), + ) + : await runSetup( + { ...setupOptions(false), clientIds: ["cursor"] }, + Object.assign(host, { + clients: [{ name: "cursor", configPath: "/cursor.json" }], + }), + ); + expect(setup.status).toBe(expectedStatus); + const output = await logCliCommand( + silentLogger, + CLI_COMMANDS.setup, + async () => setup, + isSetupFailure, + ); + expect(output).toBe(setup); + expect(process.exitCode).toBe(1); + }, + ); + + it("returns nonzero when the real uninstall outcome contains a failed action", async () => { + const host: UninstallHost = { + clients: () => Promise.resolve([]), + inspectClient: () => Promise.resolve(undefined), + removeClient: () => + Promise.resolve({ name: "client", status: "failed", detail: "write" }), + removeSkill: () => + Promise.resolve({ name: "skill", status: "failed", detail: "write" }), + purgeData: () => Promise.resolve([]), + }; + const result = await runUninstall(false, host); + expect(result.status).toBe("failed"); + await logCliCommand( + silentLogger, + CLI_COMMANDS.uninstall, + async () => result, + isUninstallFailure, + ); + expect(process.exitCode).toBe(1); + }); +}); + +describe("CLI thrown analysis failures", () => { + it("returns the shared projection with actionable failure coordinates", async () => { + const projected = await logCliCommand(silentLogger, "fixture", async () => { + throw new ArtifactOperationError( + "analyze_javascript_application", + "integrity", + { + logicalPath: "src/main.js", + declaredSha256: "a".repeat(64), + calculatedSha256: "b".repeat(64), + unpacked: true, + }, + ); + }); + expect(projected).toMatchObject({ + code: "artifact_integrity_mismatch", + category: "integrity_mismatch", + details: { + logical_path: "src/main.js", + declared_sha256: "a".repeat(64), + calculated_sha256: "b".repeat(64), + }, + }); + }); +}); + +describe("CLI successful setup outcomes", () => { + it.each(["planned", "cancelled"] as const)( + "keeps a real %s setup result successful", + async (status) => { + const host = new FakeSetupHost(); + const result = await runSetup( + { + ...setupOptions(false), + dryRun: status === "planned", + structured: status === "planned", + }, + host, + () => Promise.resolve(false), + ); + expect(result.status).toBe(status); + const output = await logCliCommand( + silentLogger, + CLI_COMMANDS.setup, + async () => result, + isSetupFailure, + ); + expect(output).toBe(result); + expect(process.exitCode).toBeUndefined(); + expect(host.hopperInstalls).toBe(0); + expect(host.configurations).toBe(0); + expect(host.skillInstalls).toBe(0); + }, + ); +}); + +describe("CLI structured failure output", () => { it("sets a nonzero process status without replacing structured output", async () => { const output = { error: "Analysis failed",