morluto/rea
1
0
Fork 0
mirror of https://github.com/morluto/rea.git synced 2026-10-12 02:19:24 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(cli): return nonzero status for operation failures (#121)

Browse Source
This commit is contained in:
morluto
2026-07-14 06:33:34 +08:00
committed by GitHub
parent 73852202d4
commit 00c187e32b
9 changed files with 207 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files
README.md
+21
View File
@@ -420,6 +420,27 @@ rea mcp
REA accepts a Mac `.app` folder directly. If an agent cannot find an app by name, tell it where the app is installed.
### CLI exit status
| Status | Meaning |
| --------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| `0` | The requested operation completed. Truthful unknowns, warnings, and bounded or truncated evidence remain successful results. |
| `1` | Arguments, policy, permission, provider analysis, integrity checking, cancellation, timeout, setup, diagnostics, update, uninstall, output encoding, or output writing prevented completion. Structured output identifies the failure category when REA could encode it. |
| `128 + N` | The process ended from signal `N`, where the shell or runtime preserves the conventional signal-derived status. |
`setup` returns `1` for `planned`, `needs_confirmation`, or `needs_human`
because configuration is not ready; rerun it after approval or remediation.
`doctor` returns `1` when any check is unhealthy. Output format, full envelopes,
filters, and token controls never change the operation status.
When REA feeds a shell pipeline, enable `pipefail` so a downstream formatter
cannot hide its failure:
```bash
set -o pipefail
rea inventory-artifact ./app.asar --json | jq . > inventory.json
```
## Current Hopper provider
REA starts Hopper when needed; Hopper does not need to be running first. Hopper's launcher internally activates the application, so opening a target may bring Hopper to the foreground. REA asks macOS to start Hopper hidden and in the background when possible, but cannot guarantee that it will remain behind the current application.
scripts/verify-package.mjs
+56 -14
View File
@@ -114,7 +114,12 @@ try {
const cli = join(prefix, "bin", "rea");
const help = await run(cli, ["--help"], environment);
const llms = await run(cli, ["--llms"], environment);
const doctor = json(await run(cli, ["doctor", "--json"], environment));
const doctorExecution = await runWithStatus(
cli,
["doctor", "--json"],
environment,
);
const doctor = json(doctorExecution.stdout);
const supportedSetupHost =
doctor.checks?.find(({ name }) => name === "host")?.ok === true;
const expectedDoctorHealth = doctor.checks?.every(({ ok }) => ok) === true;
@@ -136,6 +141,7 @@ try {
!llms.includes("providers") ||
!llms.includes("inventory-artifact") ||
doctor.healthy !== expectedDoctorHealth ||
doctorExecution.status !== (expectedDoctorHealth ? 0 : 1) ||
doctor.checks?.find(({ name }) => name === "hopper")?.ok !== true
)
throw new Error(
@@ -328,10 +334,16 @@ try {
await writeFile(skillPath, "stale managed skill\n");
await writeFile(siblingSkillPath, "unrelated skill\n");
}
const planned = json(await run(cli, ["setup", "--json"], environment));
const plannedExecution = await runWithStatus(
cli,
["setup", "--json"],
environment,
);
const planned = json(plannedExecution.stdout);
if (supportedSetupHost) {
if (
planned.status !== "needs_confirmation" ||
plannedExecution.status !== 1 ||
planned.appliedActions.length !== 0 ||
!planned.plannedActions.some(({ kind }) => kind === "configure_client") ||
!planned.plannedActions.some(({ kind }) => kind === "install_skill") ||
@@ -341,19 +353,25 @@ try {
)
throw new Error("packaged setup plan mutated files before approval");
}
const first = json(await run(cli, ["setup", "--yes", "--json"], environment));
const second = json(
await run(
cli,
["setup", "--yes", "--install-hopper", "--json"],
environment,
),
const firstExecution = await runWithStatus(
cli,
["setup", "--yes", "--json"],
environment,
);
const first = json(firstExecution.stdout);
const secondExecution = await runWithStatus(
cli,
["setup", "--yes", "--install-hopper", "--json"],
environment,
);
const second = json(secondExecution.stdout);
if (supportedSetupHost) {
const status = process.platform === "linux" ? "needs_human" : "ready";
if (
first.status !== status ||
second.status !== status ||
firstExecution.status !== (status === "ready" ? 0 : 1) ||
secondExecution.status !== (status === "ready" ? 0 : 1) ||
second.appliedActions.length !== 0
)
throw new Error("packaged setup was not ready and idempotent");
@@ -387,19 +405,29 @@ try {
throw new Error("packaged stale-skill upgrade was not isolated");
await writeFile(cursorConfig, "malformed");
const failed = json(
await run(cli, ["setup", "--yes", "--json"], environment),
const failedExecution = await runWithStatus(
cli,
["setup", "--yes", "--json"],
environment,
);
const failed = json(failedExecution.stdout);
if (
failed.status !== "needs_human" ||
failedExecution.status !== 1 ||
(await readFile(cursorConfig, "utf8")) !== "malformed"
)
throw new Error("packaged setup failure recovery did not preserve input");
await writeFile(cursorConfig, "{}\n");
const recovered = json(
await run(cli, ["setup", "--yes", "--json"], environment),
const recoveredExecution = await runWithStatus(
cli,
["setup", "--yes", "--json"],
environment,
);
if (recovered.status !== status)
const recovered = json(recoveredExecution.stdout);
if (
recovered.status !== status ||
recoveredExecution.status !== (status === "ready" ? 0 : 1)
)
throw new Error("packaged setup did not recover");
} else if (
first.status !== "needs_human" ||
@@ -503,6 +531,20 @@ try {
async function run(command, args, env) {
return (await exec(command, args, { env })).stdout;
}
async function runWithStatus(command, args, env) {
try {
return { stdout: await run(command, args, env), status: 0 };
} catch (cause) {
if (
typeof cause === "object" &&
cause !== null &&
cause.code === 1 &&
typeof cause.stdout === "string"
)
return { stdout: cause.stdout, status: 1 };
throw cause;
}
}
function json(text) {
return JSON.parse(text);
}
src/application/ProcessCli.ts
-13
View File
@@ -25,19 +25,6 @@ export interface ProcessCliErrorOutput {
readonly message: string;
}
/** Distinguish a safe process-command failure from successful Evidence. */
export const isProcessCliErrorOutput = (
value: unknown,
): value is ProcessCliErrorOutput =>
typeof value === "object" &&
value !== null &&
"error" in value &&
value.error === "Process command failed" &&
"category" in value &&
typeof value.category === "string" &&
"message" in value &&
typeof value.message === "string";
/** Capture one JSON scenario through the same policy and evidence contract as MCP. */
export const captureProcessScenarioFile = async (path: string) => {
try {
src/cliLogging.ts
+21 -3
View File
@@ -1,5 +1,21 @@
import type { Logger } from "./logger.js";
const isRecord = (value: unknown): value is Readonly<Record<string, unknown>> =>
typeof value === "object" && value !== null && !Array.isArray(value);
/** Identify caller-visible CLI outcomes that mean the requested operation failed. */
export const isCliOperationFailure = (value: unknown): boolean => {
if (!isRecord(value)) return false;
if (typeof value.error === "string") return true;
if (value.healthy === false) return true;
return (
value.status === "failed" ||
value.status === "needs_confirmation" ||
value.status === "needs_human" ||
value.status === "planned"
);
};
/** Log one CLI command with duration and a stable success or failure status. */
export const logCliCommand = async <Value>(
logger: Logger,
@@ -9,14 +25,16 @@ export const logCliCommand = async <Value>(
const startedAt = performance.now();
try {
const value = await execute();
logger.info(
const failed = isCliOperationFailure(value);
logger[failed ? "error" : "info"](
{
command,
durationMs: Math.round((performance.now() - startedAt) * 100) / 100,
status: "ok",
status: failed ? "error" : "ok",
},
"CLI command completed",
failed ? "CLI command failed" : "CLI command completed",
);
if (failed) process.exitCode = 1;
return value;
} catch (cause: unknown) {
logger.error(
src/cliProcessCommands.ts
+3 -21
View File
@@ -1,26 +1,12 @@
import { Cli, Errors, z } from "incur";
import { Cli, z } from "incur";
import {
captureProcessScenarioFile,
compareProcessEvidenceFiles,
isProcessCliErrorOutput,
} from "./application/ProcessCli.js";
import { logCliCommand } from "./cliLogging.js";
import type { Logger } from "./logger.js";
const requireProcessCommandSuccess = async <Value>(
execute: () => Promise<Value>,
): Promise<Value> => {
const value = await execute();
if (isProcessCliErrorOutput(value))
throw new Errors.IncurError({
code: "PROCESS_COMMAND_FAILED",
message: value.message,
retryable: false,
});
return value;
};
/** Register Process Capture v4 one-shot commands through shared application services. */
export const registerProcessCommands = (
cli: ReturnType<typeof Cli.create>,
@@ -31,9 +17,7 @@ export const registerProcessCommands = (
args: z.object({ scenario: z.string().describe("Scenario JSON path") }),
run: ({ args }) =>
logCliCommand(logger, "capture-process", () =>
requireProcessCommandSuccess(() =>
captureProcessScenarioFile(args.scenario),
),
captureProcessScenarioFile(args.scenario),
),
});
cli.command("compare-process-captures", {
@@ -44,9 +28,7 @@ export const registerProcessCommands = (
}),
run: ({ args }) =>
logCliCommand(logger, "compare-process-captures", () =>
requireProcessCommandSuccess(() =>
compareProcessEvidenceFiles(args.left, args.right),
),
compareProcessEvidenceFiles(args.left, args.right),
),
});
};
src/logger.ts
+2 -2
View File
@@ -12,7 +12,7 @@ const logLevelSchema = z
export const parseLogLevel = (value: unknown): LogLevel =>
logLevelSchema.parse(value);
/** Create a JSON logger whose destination cannot corrupt MCP stdout frames. */
/** Create a JSON logger on stderr so protocol and CLI result stdout stay valid. */
export const createLogger = (mode: "mcp" | "cli", level: LogLevel): Logger =>
pino(
{
@@ -32,7 +32,7 @@ export const createLogger = (mode: "mcp" | "cli", level: LogLevel): Logger =>
censor: "[Redacted]",
},
},
pino.destination({ dest: mode === "mcp" ? 2 : 1, sync: false }),
pino.destination({ dest: 2, sync: false }),
);
/** Logger used by embedders that have not opted into process output. */
tests/cliLogging.test.ts
+50
View File
@@ -0,0 +1,50 @@
import { afterEach, describe, expect, it } from "vitest";
import { isCliOperationFailure, logCliCommand } from "../src/cliLogging.js";
import { silentLogger } from "../src/logger.js";
const originalExitCode = process.exitCode;
afterEach(() => {
process.exitCode = originalExitCode;
});
describe("CLI operation status", () => {
it.each([
["typed error", { error: "Analysis failed", category: "timeout" }],
["unhealthy diagnostic", { healthy: false, checks: [] }],
["failed status", { status: "failed" }],
["confirmation requirement", { status: "needs_confirmation" }],
["human remediation", { status: "needs_human" }],
["unapplied plan", { status: "planned" }],
])("classifies %s as failure", (_label, value) => {
expect(isCliOperationFailure(value)).toBe(true);
});
it.each([
["ready setup", { status: "ready" }],
["complete uninstall", { status: "complete" }],
["current version", { status: "current" }],
["completed upgrade", { status: "upgraded" }],
["healthy diagnostics", { healthy: true, checks: [] }],
["bounded evidence", { evidence: [{ truncated: true }] }],
])("keeps %s successful", (_label, value) => {
expect(isCliOperationFailure(value)).toBe(false);
});
it("sets a nonzero process status without replacing structured output", async () => {
const output = {
error: "Analysis failed",
category: "integrity_mismatch",
message: "Artifact integrity check failed.",
details: { logical_path: "main.js" },
};
await expect(
logCliCommand(silentLogger, "inventory-artifact", () =>
Promise.resolve(output),
),
).resolves.toBe(output);
expect(process.exitCode).toBe(1);
});
});
tests/cliOutput.test.ts
+53 -1
View File
@@ -102,12 +102,15 @@ describe("CLI output boundary", () => {
await writeFile(join(`${archive}.unpacked`, "main.js"), "changed();\n");
for (const flags of [["--json"], ["--full-output", "--json"]]) {
const { stdout } = await execFileAsync(process.execPath, [
const execution = execFileAsync(process.execPath, [
"scripts/rea.mjs",
...flags,
"inventory-artifact",
archive,
]);
const failure = await execution.catch((cause: unknown) => cause);
expect(failure).toMatchObject({ code: 1 });
const { stdout } = failure as { readonly stdout: string };
const output = JSON.stringify(JSON.parse(stdout) as unknown);
expect(output).toContain('"logical_path":"main.js"');
expect(output).toMatch(/"declared_sha256":"[a-f0-9]{64}"/u);
@@ -115,4 +118,53 @@ describe("CLI output boundary", () => {
expect(output).toContain('"unpacked":true');
}
}, 20_000);
it("keeps operation failure status independent of output controls", async () => {
const variants = [
["--format", "toon"],
["--format", "json"],
["--format", "yaml"],
["--format", "md"],
["--format", "jsonl"],
["--full-output", "--json"],
["--filter-output", "category", "--json"],
["--token-limit", "5", "--json"],
["--token-count", "--json"],
];
for (const flags of variants) {
await expect(
execFileAsync(process.execPath, [
"scripts/rea.mjs",
...flags,
"investigate-versions",
"/tmp/left",
"/tmp/right",
"/tmp/workspace.json",
]),
).rejects.toMatchObject({ code: 1, stdout: expect.any(String) });
}
}, 20_000);
it("keeps failure logs out of structured stdout", async () => {
const execution = execFileAsync(
process.execPath,
[
"scripts/rea.mjs",
"--json",
"investigate-versions",
"/tmp/left",
"/tmp/right",
"/tmp/workspace.json",
],
{ env: { ...process.env, REA_LOG_LEVEL: "error" } },
);
const failure = await execution.catch((cause: unknown) => cause);
expect(failure).toMatchObject({
code: 1,
stderr: expect.stringContaining('"status":"error"'),
});
const { stdout } = failure as { readonly stdout: string };
expect(JSON.parse(stdout)).toMatchObject({ error: "ApprovalRequired" });
}, 20_000);
});
tests/processCli.test.ts
+1 -1
View File
@@ -40,7 +40,7 @@ describe("process CLI errors", () => {
),
).rejects.toMatchObject({
code: 1,
stdout: expect.stringContaining("PROCESS_COMMAND_FAILED"),
stdout: expect.stringContaining("category: invalid_input"),
});
});