From 7e1262e154915e73035c617cb2c04bfb947f5615 Mon Sep 17 00:00:00 2001 From: Dongni Yang Date: Thu, 23 Jul 2026 12:06:27 +0800 Subject: [PATCH 1/2] fix(cli): translate the shields exit sentinel into a clean exit code Refs #7382 Co-Authored-By: Claude Opus 4.8 Co-Authored-By: Claude Fable 5 Signed-off-by: Dongni Yang --- .../sandbox/oclif-command-adapters.test.ts | 31 +++++++++++ src/lib/cli/nemoclaw-oclif-command.test.ts | 55 +++++++++++++++++++ src/lib/cli/nemoclaw-oclif-command.ts | 14 +++++ src/lib/shields/deferred-exit.ts | 33 +++++++++++ src/lib/shields/index.ts | 17 ++---- 5 files changed, 139 insertions(+), 11 deletions(-) create mode 100644 src/lib/shields/deferred-exit.ts diff --git a/src/commands/sandbox/oclif-command-adapters.test.ts b/src/commands/sandbox/oclif-command-adapters.test.ts index 71218f4d0d1..dd6a3609117 100644 --- a/src/commands/sandbox/oclif-command-adapters.test.ts +++ b/src/commands/sandbox/oclif-command-adapters.test.ts @@ -313,6 +313,37 @@ describe("sandbox oclif command adapters", () => { expect(mocks.shieldsStatus).toHaveBeenCalledWith("alpha"); }); + it("translates shields exit sentinels into exit codes without a traceback (#7382)", async () => { + const error = vi.spyOn(console, "error").mockImplementation(() => undefined); + const previousExitCode = process.exitCode; + process.exitCode = undefined; + try { + mocks.shieldsUp.mockImplementationOnce(() => { + throw Object.assign(new Error("Config not locked: OpenClaw config guard lock failed"), { + name: "DeferredShieldsExit", + exitCode: 1, + }); + }); + mocks.shieldsDown.mockImplementationOnce(() => { + throw Object.assign(new Error("Config remains unlocked — manual intervention required"), { + name: "DeferredShieldsExit", + exitCode: 1, + }); + }); + + await expect(ShieldsUpCommand.run(["alpha"], rootDir)).resolves.toBeUndefined(); + expect(process.exitCode).toBe(1); + + process.exitCode = undefined; + await expect(ShieldsDownCommand.run(["alpha"], rootDir)).resolves.toBeUndefined(); + expect(process.exitCode).toBe(1); + expect(error).not.toHaveBeenCalled(); + } finally { + process.exitCode = previousExitCode; + error.mockRestore(); + } + }); + it("sets a nonzero JSON exit when doctor reports inference.local failure (#6192)", async () => { const previousExitCode = process.exitCode; process.exitCode = undefined; diff --git a/src/lib/cli/nemoclaw-oclif-command.test.ts b/src/lib/cli/nemoclaw-oclif-command.test.ts index cdf76a685c3..4a7d6dee498 100644 --- a/src/lib/cli/nemoclaw-oclif-command.test.ts +++ b/src/lib/cli/nemoclaw-oclif-command.test.ts @@ -34,6 +34,42 @@ class ParsingTestCommand extends NemoClawCommand { } } +class ShieldsSentinelCommand extends NemoClawCommand { + static id = "shields-sentinel-test"; + static flags = {}; + + public async run(): Promise { + await this.parse(ShieldsSentinelCommand); + throw Object.assign(new Error("Config remains unlocked — already printed"), { + name: "DeferredShieldsExit", + exitCode: 1, + }); + } +} + +class DriftSentinelCommand extends NemoClawCommand { + static id = "drift-sentinel-test"; + static flags = {}; + + public async run(): Promise { + await this.parse(DriftSentinelCommand); + throw Object.assign(new Error("Locked shields state has filesystem drift"), { + name: "DeferredShieldsExit", + exitCode: 2, + }); + } +} + +class PlainFailureCommand extends NemoClawCommand { + static id = "plain-failure-test"; + static flags = {}; + + public async run(): Promise { + await this.parse(PlainFailureCommand); + throw Object.assign(new Error("real failure"), { exitCode: 7 }); + } +} + function makeCommand(): TestCommand { return Object.create(TestCommand.prototype) as TestCommand; } @@ -100,4 +136,23 @@ describe("NemoClawCommand", () => { await ParsingTestCommand.run(["--debug"], process.cwd()); expect(log.level).toBe("debug"); }); + + it("translates a shields exit sentinel into an exit code without reprinting (#7382)", async () => { + const error = vi.spyOn(console, "error").mockImplementation(() => undefined); + + await expect(ShieldsSentinelCommand.run([], process.cwd())).resolves.toBeUndefined(); + + expect(process.exitCode).toBe(1); + expect(error).not.toHaveBeenCalled(); + }); + + it("keeps the sentinel's non-default exit code", async () => { + await expect(DriftSentinelCommand.run([], process.cwd())).resolves.toBeUndefined(); + + expect(process.exitCode).toBe(2); + }); + + it("passes non-sentinel failures to the default oclif handler", async () => { + await expect(PlainFailureCommand.run([], process.cwd())).rejects.toThrow("real failure"); + }); }); diff --git a/src/lib/cli/nemoclaw-oclif-command.ts b/src/lib/cli/nemoclaw-oclif-command.ts index cfee9056011..8c4bb4827af 100644 --- a/src/lib/cli/nemoclaw-oclif-command.ts +++ b/src/lib/cli/nemoclaw-oclif-command.ts @@ -3,6 +3,7 @@ import { Command, Flags, type Interfaces } from "@oclif/core"; import { redactForLog } from "../security/redact"; +import { isDeferredShieldsExit } from "../shields/deferred-exit"; import { log } from "./logger"; export type CommandExitResult = { @@ -69,6 +70,19 @@ export abstract class NemoClawCommand extends Command { return parsed; } + protected override async catch(error: unknown): Promise { + // Shields transitions defer process.exit through a sentinel so an exit + // cannot strand the transition lock (see failShieldsCommand). By the time + // oclif routes the rejection here every lock has been released, and the + // failure lines were already printed at the throw site, so only the exit + // code remains to record. Everything else keeps oclif's default handling. + if (isDeferredShieldsExit(error)) { + this.setExitCode(error.exitCode); + return; + } + return super.catch(error as Error & { exitCode?: number }); + } + protected logJson(json: unknown): void { console.log(JSON.stringify(redactForLog(json), null, 2)); } diff --git a/src/lib/shields/deferred-exit.ts b/src/lib/shields/deferred-exit.ts new file mode 100644 index 00000000000..6706ee8fe9a --- /dev/null +++ b/src/lib/shields/deferred-exit.ts @@ -0,0 +1,33 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** + * Sentinel thrown in place of process.exit while a shields transition lock is + * active: process.exit skips finally blocks and would strand the canonical + * lock. Public command wrappers translate the sentinel into an exit code once + * every lock has been released (NemoClawCommand.catch). + * + * Kept in a leaf module so the CLI base command can guard against the + * sentinel without loading the shields coordinator, and so tests that mock + * the shields module keep the real guard. + */ +export class DeferredShieldsExit extends Error { + readonly exitCode: number; + + constructor(message: string, exitCode: number) { + super(message); + this.name = "DeferredShieldsExit"; + this.exitCode = exitCode; + } +} + +/** + * Name-keyed guard: instanceof breaks when dist and src copies of the class + * are both loaded, so match on the sentinel's shape instead (same pattern as + * snapshotCommandError). + */ +export function isDeferredShieldsExit(error: unknown): error is DeferredShieldsExit { + if (!error || typeof error !== "object") return false; + const candidate = error as { name?: unknown; exitCode?: unknown }; + return candidate.name === "DeferredShieldsExit" && typeof candidate.exitCode === "number"; +} diff --git a/src/lib/shields/index.ts b/src/lib/shields/index.ts index 58cbfb5768f..5b97588abc5 100644 --- a/src/lib/shields/index.ts +++ b/src/lib/shields/index.ts @@ -764,20 +764,15 @@ function ensureConfigHashSensitiveFile(target: T): return { ...target, sensitiveFiles: [...sensitiveFiles, hashPath] } as T; } -class DeferredShieldsExit extends Error { - readonly exitCode: number; - - constructor(message: string, exitCode: number) { - super(message); - this.name = "DeferredShieldsExit"; - this.exitCode = exitCode; - } -} +const { + DeferredShieldsExit, +}: typeof import("./deferred-exit") = require("./deferred-exit"); function failShieldsCommand(message: string, _shouldThrow?: boolean): never { // Never terminate while a transition-lock callback is active: process.exit - // skips finally blocks and would strand the canonical lock. Public command - // wrappers translate this sentinel only after the lock has been released. + // skips finally blocks and would strand the canonical lock. NemoClawCommand + // translates this sentinel into an exit code after the lock has been + // released (isDeferredShieldsExit in ./deferred-exit). throw new DeferredShieldsExit(message, 1); } From 9187ee2dfe1c1b96b5b78421dde7e210cebab914 Mon Sep 17 00:00:00 2001 From: Dongni Yang Date: Thu, 23 Jul 2026 12:17:53 +0800 Subject: [PATCH 2/2] style(shields): collapse deferred-exit destructure to one line per biome Refs #7382 Co-Authored-By: Claude Fable 5 Signed-off-by: Dongni Yang --- src/lib/shields/index.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/lib/shields/index.ts b/src/lib/shields/index.ts index 5b97588abc5..fcbe3085e2d 100644 --- a/src/lib/shields/index.ts +++ b/src/lib/shields/index.ts @@ -764,9 +764,7 @@ function ensureConfigHashSensitiveFile(target: T): return { ...target, sensitiveFiles: [...sensitiveFiles, hashPath] } as T; } -const { - DeferredShieldsExit, -}: typeof import("./deferred-exit") = require("./deferred-exit"); +const { DeferredShieldsExit }: typeof import("./deferred-exit") = require("./deferred-exit"); function failShieldsCommand(message: string, _shouldThrow?: boolean): never { // Never terminate while a transition-lock callback is active: process.exit