Merge branch 'main' into fix/cli-operation-failure-status

* fix(hopper): avoid blocking owned Linux shutdown

* fix(hopper): complete owned shutdown cleanup

README.md

@@ -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.

bridge/hopper_bridge.py

@@ -628,13 +628,13 @@ def _dispatch(method, params):
     global _selected_document
     if method == "health":
         return {"name": "REA Hopper bridge", "version": "1.0.0", "run_id": REA_RUN_ID}
-    if method == "shutdown":
+    if method in ("shutdown", "shutdown_document"):
         document = _session_document()
         if document is None:
             return {"shutdown": True, "analysis_stopped": True, "document_closed": True}
         if document.backgroundProcessActive():
             document.requestBackgroundProcessStop()
-        if REA_OWNS_PROCESS_LIFETIME:
+        if method == "shutdown" and REA_OWNS_PROCESS_LIFETIME:
             return {
                 "shutdown": True,
                 "analysis_stopped": not document.backgroundProcessActive(),
@@ -813,7 +813,9 @@ def _serve_connection(connection):
             if not isinstance(request["token"], str) or not hmac.compare_digest(request["token"], REA_TOKEN):
                 raise PermissionError("Invalid bridge capability")
             result = _dispatch(request["method"], request["params"])
-            should_stop = request["method"] == "shutdown"
+            should_stop = request["method"] == "shutdown_document" or (
+                request["method"] == "shutdown" and not result.get("cleanup_required", False)
+            )
             response = {"id": request_id, "result": _json_safe(result)}
         except Exception as error:
             response = {"id": request_id if isinstance(request_id, int) else 0, "error": {"code": -32000, "message": str(error)[:512], "type": _diagnostic_type(error)}}

scripts/verify-package.mjs

@@ -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

@@ -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

@@ -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

@@ -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/domain/errors.ts

@@ -3,8 +3,7 @@ import {
   hopperStartupFailure,
   type HopperStartupFailureCode,
 } from "./hopperStartupFailure.js";
-export { hopperStartupFailure } from "./hopperStartupFailure.js";
-export type { HopperStartupFailureCode } from "./hopperStartupFailure.js";
+export { hopperStartupFailure, type HopperStartupFailureCode };
 
 /** Stable tags exposed by safe analysis-error projections. */
 const ANALYSIS_ERROR_TAGS = [
@@ -359,22 +358,21 @@ export const projectAnalysisError = (
   assertKnownTag(error._tag);
   const code =
     error instanceof HopperProcessError ? error.failureCode : undefined;
-  const details =
-    error instanceof ArtifactOperationError ? error.artifactDetails : undefined;
+  const artifact =
+    error instanceof ArtifactOperationError && error.artifactDetails;
+  const details = artifact
+    ? {
+        logical_path: artifact.logicalPath,
+        declared_sha256: artifact.declaredSha256,
+        calculated_sha256: artifact.calculatedSha256,
+        unpacked: artifact.unpacked,
+      }
+    : undefined;
   return {
     category: errorCategory(error),
     message: userMessage(error),
     ...(code === undefined ? {} : { code }),
-    ...(details === undefined
-      ? {}
-      : {
-          details: {
-            logical_path: details.logicalPath,
-            declared_sha256: details.declaredSha256,
-            calculated_sha256: details.calculatedSha256,
-            unpacked: details.unpacked,
-          },
-        }),
+    ...(details === undefined ? {} : { details }),
   };
 };
 

src/hopper/HopperClient.ts

@@ -167,6 +167,7 @@ export class HopperClient {
     this.#closing = true;
     try {
       const socket = this.#socket;
+      let cleanupAttempted = false;
       if (socket !== undefined && !socket.destroyed) {
         const shutdown = await this.#request(
           "shutdown",
@@ -174,12 +175,23 @@ export class HopperClient {
           { timeoutMs: SHUTDOWN_TIMEOUT_MS },
         ).catch(() => err(new HopperProcessError(null)));
         if (
-          !shutdown.ok ||
-          !isShutdownAcknowledgement(
-            shutdown.value,
-            this.#launch?.shutdownByCleanup === true,
-          )
-        )
+          shutdown.ok &&
+          isCleanupRequired(shutdown.value) &&
+          this.#launch?.shutdownByCleanup === true &&
+          this.#launch.cleanup !== undefined
+        ) {
+          cleanupAttempted = true;
+          const cleanup = await this.#launch.cleanup();
+          if (!cleanup.cleaned) {
+            await this.#fallbackDocumentShutdown();
+            const retried = await this.#launch.cleanup();
+            if (!retried.cleaned)
+              this.#logger.warn(
+                { reason: retried.reason },
+                "Owned launcher cleanup failed closed",
+              );
+          }
+        } else if (!shutdown.ok || !isShutdownAcknowledgement(shutdown.value))
           this.#logger.warn(
             { status: shutdown.ok ? "invalid-acknowledgement" : "failed" },
             "Hopper document shutdown was not confirmed",
@@ -189,7 +201,7 @@ export class HopperClient {
       socket?.destroy();
       this.#socket = undefined;
       if (this.#launch?.ownsProcessLifetime === true) {
-        if (this.#launch.cleanup !== undefined) {
+        if (this.#launch.cleanup !== undefined && !cleanupAttempted) {
           const cleanup = await this.#launch.cleanup();
           if (!cleanup.cleaned)
             this.#logger.warn(
@@ -211,6 +223,19 @@ export class HopperClient {
     }
   }
 
+  async #fallbackDocumentShutdown(): Promise<void> {
+    const fallback = await this.#request(
+      "shutdown_document",
+      {},
+      { timeoutMs: SHUTDOWN_TIMEOUT_MS },
+    ).catch(() => err(new HopperProcessError(null)));
+    if (!fallback.ok || !isShutdownAcknowledgement(fallback.value))
+      this.#logger.warn(
+        { status: fallback.ok ? "invalid-acknowledgement" : "failed" },
+        "Hopper document shutdown fallback was not confirmed",
+      );
+  }
+
   async #connect(
     socketPath: string,
     signal?: AbortSignal,
@@ -418,18 +443,22 @@ const connectOnce = async (
     });
   });
 
-const isShutdownAcknowledgement = (
-  value: JsonValue,
-  ownsProcessLifetime: boolean,
-): boolean => {
+const isShutdownAcknowledgement = (value: JsonValue): boolean => {
   if (typeof value !== "object" || value === null || Array.isArray(value))
     return false;
   if (value.shutdown !== true) return false;
   if (value.analysis_stopped === true && value.document_closed === true)
     return true;
-  return ownsProcessLifetime && value.cleanup_required === true;
+  return false;
 };
 
+const isCleanupRequired = (value: JsonValue): boolean =>
+  typeof value === "object" &&
+  value !== null &&
+  !Array.isArray(value) &&
+  value.shutdown === true &&
+  value.cleanup_required === true;
+
 const parseServerInfo = (
   value: JsonValue,
   expectedRunId: string,

src/logger.ts

@@ -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/bridgeTruthfulness.test.ts

@@ -104,8 +104,13 @@ describe("Hopper bridge truthfulness", () => {
     expect(bridgeSource).toContain("document.getDatabaseFilePath()");
     expect(bridgeSource).toContain("document.backgroundProcessActive()");
     expect(bridgeSource).toContain("document.requestBackgroundProcessStop()");
-    expect(bridgeSource).toContain("if REA_OWNS_PROCESS_LIFETIME:");
+    expect(bridgeSource).toContain(
+      'if method == "shutdown" and REA_OWNS_PROCESS_LIFETIME:',
+    );
     expect(bridgeSource).toContain('"cleanup_required": True');
+    expect(bridgeSource).toContain(
+      'method in ("shutdown", "shutdown_document")',
+    );
     expect(bridgeSource).toContain("document.waitForBackgroundProcessToEnd()");
     expect(bridgeSource).toContain("document.closeDocument()");
     expect(bridgeSource).toContain(

tests/cliLogging.test.ts

@@ -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

@@ -90,7 +90,7 @@ describe("CLI output boundary", () => {
       stdout:
         'ok: false\nerror:\n  code: VALIDATION_ERROR\n  message: "REA could not read the command arguments. Run `rea --help`, correct the arguments, then try again."\n',
     });
-  });
+  }, 20_000);
 
   it("preserves artifact diagnostics in ordinary and full JSON output", async () => {
     const root = await mkdtemp(join(tmpdir(), "rea-cli-diagnostics-"));
@@ -102,17 +102,69 @@ 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);
       expect(output).toMatch(/"calculated_sha256":"[a-f0-9]{64}"/u);
       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

@@ -40,7 +40,7 @@ describe("process CLI errors", () => {
       ),
     ).rejects.toMatchObject({
       code: 1,
-      stdout: expect.stringContaining("PROCESS_COMMAND_FAILED"),
+      stdout: expect.stringContaining("category: invalid_input"),
     });
   });