Skip to content

Commit 6f43c50

Browse files
steipetesteipete
authored
fix(cli): preserve failure exit semantics (#112210)
1 parent b68b972 commit 6f43c50

13 files changed

Lines changed: 544 additions & 81 deletions

‎docs/cli/agent.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ openclaw agent --agent ops --message "Run locally" --local
5858
- `--session-key` selects an explicit session key. Agent-prefixed keys must use `agent:<agent-id>:<session-key>`, and `--agent` must match the key's agent id when both are given. Bare non-sentinel keys scope to `--agent` when supplied, or to the configured default agent otherwise; for example `--agent ops --session-key incident-42` routes to `agent:ops:incident-42`. The literal keys `global` and `unknown` stay unscoped only when no `--agent` is supplied.
5959
- `--json` reserves stdout for the JSON response; Gateway, plugin, and `--local` diagnostics go to stderr so scripts can parse stdout directly.
6060
- After transient handshake retries are exhausted, a Gateway timeout or closed connection fails the command; the CLI never silently reruns the turn embedded. Transport loss is ambiguous — the Gateway may have accepted and may still finish the turn — so the stderr hint says to check `openclaw gateway status` and the session transcript before retrying or rerunning with `--local`, to avoid executing the turn twice.
61-
- `SIGTERM`/`SIGINT` interrupt a waiting Gateway-backed request; if the Gateway already accepted the run, the CLI also sends `chat.abort` for that run id before exiting. `--local` runs receive the same signal but do not send `chat.abort`. If the internal run-dedup key already has an active run for this session, the response reports `status: "in_flight"` and the non-JSON CLI prints a stderr diagnostic instead of an empty reply. For external cron/systemd wrappers, keep a hard-kill backstop such as `timeout -k 60 600 openclaw agent ...` so the supervisor can reap the process if shutdown cannot drain.
61+
- `SIGTERM`/`SIGINT` interrupt a waiting Gateway-backed request; if the Gateway already accepted the run, the CLI also sends `chat.abort` for that run id before exiting. `--local` runs receive the same signal but do not send `chat.abort`. A launcher child that terminates from the first forwarded `SIGINT` or `SIGTERM` exits with status 130 or 143, respectively. If the internal run-dedup key already has an active run for this session, the response reports `status: "in_flight"` and the non-JSON CLI prints a stderr diagnostic instead of an empty reply. For external cron/systemd wrappers, keep a hard-kill backstop such as `timeout -k 60 600 openclaw agent ...` so the supervisor can reap the process if shutdown cannot drain.
6262
- When this command triggers `models.json` regeneration, SecretRef-managed provider credentials are persisted as non-secret markers (for example env var names, `secretref-env:ENV_VAR_NAME`, or `secretref-managed`), never resolved secret plaintext. Marker writes come from the active source config snapshot, not from resolved runtime secret values.
6363

6464
## JSON delivery status

‎openclaw.mjs‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,8 @@ const runRespawnedChild = (command, args, env) => {
140140
let signalExitTimer = null;
141141
let signalForceKillTimer = null;
142142
let signalHardExitTimer = null;
143+
let firstForwardedSignal = null;
144+
let hardKillBackstopStarted = false;
143145
const detach = () => {
144146
for (const [signal, listener] of listeners) {
145147
process.off(signal, listener);
@@ -172,6 +174,7 @@ const runRespawnedChild = (command, args, env) => {
172174
// Best-effort shutdown fallback.
173175
}
174176
signalForceKillTimer = setTimeout(() => {
177+
hardKillBackstopStarted = true;
175178
forceKillChild();
176179
signalHardExitTimer = setTimeout(() => {
177180
process.exit(1);
@@ -180,7 +183,8 @@ const runRespawnedChild = (command, args, env) => {
180183
}, respawnSignalForceKillGraceMs);
181184
signalForceKillTimer.unref?.();
182185
};
183-
const scheduleParentExit = () => {
186+
const scheduleParentExit = (signal) => {
187+
firstForwardedSignal ??= signal;
184188
if (signalExitTimer) {
185189
return;
186190
}
@@ -196,7 +200,7 @@ const runRespawnedChild = (command, args, env) => {
196200
} catch {
197201
// Best-effort signal forwarding.
198202
}
199-
scheduleParentExit();
203+
scheduleParentExit(signal);
200204
};
201205
try {
202206
process.on(signal, listener);
@@ -208,7 +212,15 @@ const runRespawnedChild = (command, args, env) => {
208212
child.once("exit", (code, signal) => {
209213
detach();
210214
if (signal) {
211-
process.exit(1);
215+
const forwardedSignalExitCode =
216+
!hardKillBackstopStarted && signal === firstForwardedSignal
217+
? signal === "SIGINT"
218+
? 130
219+
: signal === "SIGTERM"
220+
? 143
221+
: undefined
222+
: undefined;
223+
process.exit(forwardedSignalExitCode ?? 1);
212224
}
213225
process.exit(code ?? 1);
214226
});

‎src/cli/gateway-cli/run.option-collisions.test.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// Gateway run option collision tests cover gateway run flag registration boundaries.
2+
import { createServer } from "node:http";
23
import { Command } from "commander";
34
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
45
import { CONFIG_AUDIT_STORE_LABEL } from "../../config/io.audit.js";
@@ -1595,6 +1596,46 @@ describe("gateway run option collisions", () => {
15951596
expect(writeDiagnosticStabilityBundleForFailureSync).not.toHaveBeenCalled();
15961597
});
15971598

1599+
it.each([
1600+
"gateway already running (pid 4242); lock timeout after 5000ms",
1601+
"another gateway instance is already listening on ws://127.0.0.1",
1602+
])("exits 1 for unmanaged healthy-port lock conflicts: %s", async (message) => {
1603+
const healthyGateway = createServer((_req, res) => {
1604+
res.writeHead(200, { "content-type": "application/json" });
1605+
res.end(JSON.stringify({ ok: true, status: "live" }));
1606+
});
1607+
await new Promise<void>((resolve) => {
1608+
healthyGateway.listen(0, "127.0.0.1", resolve);
1609+
});
1610+
const address = healthyGateway.address();
1611+
if (!address || typeof address === "string") {
1612+
throw new Error("expected TCP server address");
1613+
}
1614+
const port = address.port;
1615+
configState.snapshot = {
1616+
config: { gateway: { port } },
1617+
exists: false,
1618+
sourceConfig: {},
1619+
valid: true,
1620+
};
1621+
const err = Object.assign(new Error(`${message}:${port}`), {
1622+
name: "GatewayLockError",
1623+
});
1624+
startGatewayServer.mockRejectedValueOnce(err);
1625+
1626+
try {
1627+
await withEnvAsync(withoutSupervisorEnv, async () => {
1628+
await expect(runGatewayCli(["gateway", "run", "--allow-unconfigured"])).rejects.toThrow(
1629+
"__exit__:1",
1630+
);
1631+
});
1632+
} finally {
1633+
await new Promise<void>((resolve, reject) => {
1634+
healthyGateway.close((closeError) => (closeError ? reject(closeError) : resolve()));
1635+
});
1636+
}
1637+
});
1638+
15981639
it("blocks startup when the observed snapshot loses gateway.mode", async () => {
15991640
configState.cfg = {
16001641
gateway: {

‎src/cli/gateway-cli/run.supervised-lock.test.ts‎

Lines changed: 57 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,14 @@ import { describe, expect, it, vi } from "vitest";
44
import { GatewayLockError } from "../../infra/gateway-lock.js";
55
import { testing } from "./run.test-support.js";
66

7+
const loadGatewayTlsRuntimeMock = vi.hoisted(() =>
8+
vi.fn(async () => ({ enabled: false, required: true })),
9+
);
10+
11+
vi.mock("../../infra/tls/gateway.js", () => ({
12+
loadGatewayTlsRuntime: loadGatewayTlsRuntimeMock,
13+
}));
14+
715
function createLogger() {
816
return {
917
info: vi.fn(),
@@ -61,26 +69,28 @@ describe("supervised gateway lock recovery", () => {
6169
});
6270
const probeHealth = vi.fn(async () => true);
6371

64-
await expect(
65-
testing.runGatewayLoopWithSupervisedLockRecovery({
72+
let failure: unknown;
73+
try {
74+
await testing.runGatewayLoopWithSupervisedLockRecovery({
6675
startLoop,
6776
supervisor: "systemd",
6877
port: 18789,
6978
healthHost: "127.0.0.1",
7079
log: createLogger(),
7180
probeHealth,
72-
}),
73-
).rejects.toThrow("exiting with code 78 to prevent a systemd Restart=always loop");
81+
});
82+
} catch (err) {
83+
failure = err;
84+
}
7485

86+
expect(failure).toMatchObject({
87+
message: expect.stringContaining(
88+
"exiting with code 78 to prevent a systemd Restart=always loop",
89+
),
90+
});
7591
expect(startLoop).toHaveBeenCalledTimes(1);
7692
expect(probeHealth).toHaveBeenCalledWith({ host: "127.0.0.1", port: 18789 });
77-
expect(
78-
testing.resolveGatewayLockErrorExitCode(
79-
new GatewayLockError("gateway already running under systemd; existing gateway is healthy"),
80-
"systemd",
81-
true,
82-
),
83-
).toBe(78);
93+
expect(testing.resolveGatewayLockErrorExitCode(failure)).toBe(78);
8494
});
8595

8696
it("bounds supervised retries when the existing gateway stays unhealthy", async () => {
@@ -92,8 +102,9 @@ describe("supervised gateway lock recovery", () => {
92102
now += ms;
93103
});
94104

95-
await expect(
96-
testing.runGatewayLoopWithSupervisedLockRecovery({
105+
let failure: unknown;
106+
try {
107+
await testing.runGatewayLoopWithSupervisedLockRecovery({
97108
startLoop,
98109
supervisor: "systemd",
99110
port: 18789,
@@ -104,11 +115,16 @@ describe("supervised gateway lock recovery", () => {
104115
sleep,
105116
retryMs: 5,
106117
timeoutMs: 12,
107-
}),
108-
).rejects.toThrow(
109-
"gateway already running under systemd; existing gateway did not become healthy after 12ms",
110-
);
118+
});
119+
} catch (err) {
120+
failure = err;
121+
}
111122

123+
expect(failure).toMatchObject({
124+
message:
125+
"gateway already running under systemd; existing gateway did not become healthy after 12ms",
126+
});
127+
expect(testing.resolveGatewayLockErrorExitCode(failure)).toBe(1);
112128
expect(startLoop).toHaveBeenCalledTimes(4);
113129
expect(sleep).toHaveBeenNthCalledWith(1, 5);
114130
expect(sleep).toHaveBeenNthCalledWith(2, 5);
@@ -149,11 +165,31 @@ describe("supervised gateway lock recovery", () => {
149165
expect(sleep).toHaveBeenNthCalledWith(3, 2);
150166
});
151167

152-
it("requires a confirmed healthy gateway for unmanaged duplicate starts", () => {
153-
const err = new GatewayLockError("another gateway instance is already listening");
168+
it.each(["gateway already running", "another gateway instance is already listening"])(
169+
"uses exit 1 for unmanaged lock errors: %s",
170+
(message) => {
171+
expect(testing.resolveGatewayLockErrorExitCode(new GatewayLockError(message))).toBe(1);
172+
},
173+
);
174+
175+
it("retries non-mutating TLS fingerprint loads until certificate material is ready", async () => {
176+
loadGatewayTlsRuntimeMock.mockClear();
177+
const probeHealth = testing.createConfiguredGatewayHealthProbe({
178+
gateway: { tls: { enabled: true, autoGenerate: true } },
179+
});
180+
181+
await expect(probeHealth({ host: "127.0.0.1", port: 18789 })).resolves.toBe(false);
182+
await expect(probeHealth({ host: "127.0.0.1", port: 18789 })).resolves.toBe(false);
154183

155-
expect(testing.resolveGatewayLockErrorExitCode(err, null, false)).toBe(1);
156-
expect(testing.resolveGatewayLockErrorExitCode(err, null, true)).toBe(0);
184+
expect(loadGatewayTlsRuntimeMock).toHaveBeenCalledTimes(2);
185+
expect(loadGatewayTlsRuntimeMock).toHaveBeenNthCalledWith(1, {
186+
enabled: true,
187+
autoGenerate: false,
188+
});
189+
expect(loadGatewayTlsRuntimeMock).toHaveBeenNthCalledWith(2, {
190+
enabled: true,
191+
autoGenerate: false,
192+
});
157193
});
158194

159195
it("recognizes only the OpenClaw health response", () => {

‎src/cli/gateway-cli/run.test-support.ts‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { OpenClawConfig } from "../../config/types.openclaw.js";
12
import type { RespawnSupervisor } from "../../infra/supervisor-markers.js";
23
import "./run.js";
34

@@ -7,6 +8,9 @@ type GatewayRunTestLogger = {
78
};
89

910
type GatewayRunTestApi = {
11+
createConfiguredGatewayHealthProbe(
12+
cfg: OpenClawConfig,
13+
): (params: { host: string; port: number }) => Promise<boolean>;
1014
isGatewayHealthzResponse(statusCode: number | undefined, body: string): boolean;
1115
normalizeGatewayHealthProbeHost(host: string): string;
1216
probeGatewayHealthz(params: {
@@ -15,11 +19,7 @@ type GatewayRunTestApi = {
1519
timeoutMs?: number;
1620
tlsFingerprint?: string;
1721
}): Promise<boolean>;
18-
resolveGatewayLockErrorExitCode(
19-
err: unknown,
20-
supervisor: RespawnSupervisor | null,
21-
healthyGatewayConfirmed: boolean,
22-
): number;
22+
resolveGatewayLockErrorExitCode(err: unknown): number;
2323
resolveGatewayStartupFailureExitCode(err: unknown): number;
2424
runGatewayLoopWithSupervisedLockRecovery(params: {
2525
startLoop: () => Promise<void>;
@@ -42,6 +42,9 @@ function getTestApi(): GatewayRunTestApi {
4242
}
4343

4444
export const testing: GatewayRunTestApi = {
45+
createConfiguredGatewayHealthProbe(cfg) {
46+
return getTestApi().createConfiguredGatewayHealthProbe(cfg);
47+
},
4548
isGatewayHealthzResponse(statusCode, body) {
4649
return getTestApi().isGatewayHealthzResponse(statusCode, body);
4750
},
@@ -51,8 +54,8 @@ export const testing: GatewayRunTestApi = {
5154
async probeGatewayHealthz(params) {
5255
return await getTestApi().probeGatewayHealthz(params);
5356
},
54-
resolveGatewayLockErrorExitCode(err, supervisor, healthyGatewayConfirmed) {
55-
return getTestApi().resolveGatewayLockErrorExitCode(err, supervisor, healthyGatewayConfirmed);
57+
resolveGatewayLockErrorExitCode(err) {
58+
return getTestApi().resolveGatewayLockErrorExitCode(err);
5659
},
5760
resolveGatewayStartupFailureExitCode(err) {
5861
return getTestApi().resolveGatewayStartupFailureExitCode(err);

0 commit comments

Comments
 (0)