fix(health): secondary account failures incorrectly appear healthy (#129088)

* fix(health): surface unhealthy secondary channel accounts

* test(health): prune obsolete assertion safety baseline entry
This commit is contained in:
Peter Steinberger
2026-08-25 01:25:29 -07:00
committed by GitHub
parent d1134a45f2
commit 140e0ccda5
5 changed files with 158 additions and 66 deletions
-1
View File
@@ -2621,7 +2621,6 @@ src/commands/export-trajectory.ts 1
src/commands/gateway-health-auth-diagnostic.ts 2
src/commands/gateway-presence.ts 1
src/commands/gateway-status/helpers.ts 11
src/commands/health.ts 1
src/commands/message-format.ts 21
src/commands/message.ts 4
src/commands/models/fallbacks-shared.ts 2
+86 -26
View File
@@ -1,5 +1,5 @@
import { describe, expect, it } from "vitest";
import type { HealthSummary } from "../gateway/health/types.js";
import type { ChannelAccountHealthSummary, HealthSummary } from "../gateway/health/types.js";
import { formatGatewayClosedDiagnostic, formatHealthChannelLines } from "./health-format.js";
describe("formatGatewayClosedDiagnostic", () => {
@@ -40,6 +40,38 @@ const createHealthSummary = (
...params,
});
function createMultiAccountHealthSummary(
secondary: Partial<ChannelAccountHealthSummary>,
): HealthSummary {
const primary = {
accountId: "main",
enabled: true,
configured: true,
linked: true,
healthState: "healthy",
probe: { ok: true, elapsedMs: 12 },
};
return createHealthSummary({
channels: {
matrix: {
...primary,
accounts: {
main: primary,
alerts: {
accountId: "alerts",
enabled: true,
configured: true,
linked: true,
...secondary,
},
},
},
},
channelOrder: ["matrix"],
channelLabels: { matrix: "Matrix" },
});
}
describe("formatHealthChannelLines", () => {
it("formats per-account probe timings", () => {
const summary = createHealthSummary({
@@ -164,32 +196,60 @@ describe("formatHealthChannelLines", () => {
expect(formatHealthChannelLines(summary)).toStrictEqual([`Test: ${expected}`]);
});
it.each([
["blocked", { healthState: "blocked" }],
["disconnected", { healthState: "disconnected" }],
["ingress-unavailable", { healthState: "ingress-unavailable" }],
["stale-socket", { healthState: "stale-socket" }],
["auth stabilizing", { healthState: "healthy", statusState: "unstable" }],
])(
"surfaces secondary account state %s in default and verbose health output",
(expected, state) => {
const summary = createMultiAccountHealthSummary(state);
for (const accountMode of ["default", "all"] as const) {
expect(formatHealthChannelLines(summary, { accountMode })).toStrictEqual([
`Matrix: ${expected}`,
]);
}
},
);
it("preserves explicitly scoped account health outside verbose output", () => {
const summary = createMultiAccountHealthSummary({ healthState: "blocked" });
const accountIdsByChannel = { matrix: ["main"] };
expect(
formatHealthChannelLines(summary, { accountMode: "default", accountIdsByChannel }),
).toStrictEqual(["Matrix: ok (12ms)"]);
expect(
formatHealthChannelLines(summary, { accountMode: "all", accountIdsByChannel }),
).toStrictEqual(["Matrix: blocked"]);
});
it.each([
["disabled", { enabled: false }],
["unconfigured", { configured: false }],
["unlinked", { linked: false }],
["disabled by status", { statusState: "disabled" }],
["unconfigured by status", { statusState: "unconfigured" }],
])("does not promote stale failures from an intentionally %s account", (_reason, inactive) => {
const summary = createMultiAccountHealthSummary({
healthState: "blocked",
probe: { ok: false, error: "stale old failure" },
...inactive,
});
expect(formatHealthChannelLines(summary)).toStrictEqual(["Matrix: ok (12ms)"]);
expect(formatHealthChannelLines(summary, { accountMode: "all" })).toStrictEqual([
"Matrix: ok (main:main:12ms)",
]);
});
it("surfaces a failed sibling probe over the selected account's passive healthy state", () => {
const summary = createHealthSummary({
channels: {
matrix: {
accountId: "main",
configured: true,
healthState: "healthy",
probe: { ok: true, elapsedMs: 12 },
accounts: {
main: {
accountId: "main",
configured: true,
healthState: "healthy",
probe: { ok: true, elapsedMs: 12 },
},
alerts: {
accountId: "alerts",
configured: true,
healthState: "healthy",
probe: { ok: false, error: "sync rejected" },
},
},
},
},
channelOrder: ["matrix"],
channelLabels: { matrix: "Matrix" },
const summary = createMultiAccountHealthSummary({
healthState: "healthy",
probe: { ok: false, error: "sync rejected" },
});
expect(formatHealthChannelLines(summary, { accountMode: "all" })).toStrictEqual([
+27 -15
View File
@@ -1,4 +1,3 @@
import { expectDefined } from "@openclaw/normalization-core";
/** Formatting helpers for `openclaw health` failures and channel summaries. */
import { asNullableRecord } from "@openclaw/normalization-core/record-coerce";
import { sanitizeTerminalText } from "../../packages/terminal-core/src/safe-text.js";
@@ -172,18 +171,31 @@ export const formatHealthChannelLines = (
accountMode === "all"
? Object.values(accountSummaries)
: (filteredSummaries ?? (channelSummary.accounts ? Object.values(accountSummaries) : []));
const baseSummary =
filteredSummaries && filteredSummaries.length > 0 ? filteredSummaries[0] : channelSummary;
const selectedSummary = expectDefined(baseSummary, "channel health summary");
const botUsernames = listSummaries
? listSummaries
.map((account) => {
const probeRecord = asNullableRecord(account.probe);
const bot = probeRecord ? asNullableRecord(probeRecord.bot) : null;
return bot && typeof bot.username === "string" ? bot.username : null;
})
.filter((value): value is string => Boolean(value))
: [];
const activeSummaries = listSummaries.filter(
(account) =>
account.enabled !== false &&
account.configured !== false &&
account.linked !== false &&
account.statusState !== "disabled" &&
account.statusState !== "unconfigured",
);
const selectedSummary =
activeSummaries.find(
(account) =>
(account.healthState && account.healthState !== "healthy") ||
(account.statusState &&
account.statusState !== "linked" &&
account.statusState !== "configured"),
) ??
filteredSummaries?.[0] ??
channelSummary;
const botUsernames = activeSummaries
.map((account) => {
const probeRecord = asNullableRecord(account.probe);
const bot = probeRecord ? asNullableRecord(probeRecord.bot) : null;
return bot && typeof bot.username === "string" ? bot.username : null;
})
.filter((value): value is string => Boolean(value));
const statusState =
typeof selectedSummary.statusState === "string" ? selectedSummary.statusState : null;
const healthState =
@@ -217,11 +229,11 @@ export const formatHealthChannelLines = (
const accountTimings =
accountMode === "all"
? listSummaries
? activeSummaries
.map((account) => formatAccountProbeTiming(account))
.filter((value): value is string => Boolean(value))
: [];
const failedSummary = listSummaries.find((summaryLocal) => isProbeFailure(summaryLocal));
const failedSummary = activeSummaries.find((summaryLocal) => isProbeFailure(summaryLocal));
if (failedSummary) {
const failureLine = formatProbeLine(failedSummary.probe, { botUsernames });
if (failureLine) {
+45 -1
View File
@@ -68,6 +68,9 @@ const createHealthSummary = (params: {
};
const callGatewayMock = vi.fn();
const listReadOnlyChannelPluginsForConfigMock = vi.fn(
(_config: unknown, _options?: unknown): unknown[] => [],
);
const isGatewayCredentialsRequiredErrorMock = vi.fn((_value: unknown) => false);
const isGatewaySecretRefUnavailableErrorMock = vi.fn((_value: unknown) => false);
const TEST_GATEWAY_URL = "ws://127.0.0.1:18789";
@@ -113,7 +116,8 @@ vi.mock("../cli/daemon-cli/probe.js", () => ({
}));
vi.mock("../channels/plugins/read-only.js", () => ({
listReadOnlyChannelPluginsForConfig: () => [],
listReadOnlyChannelPluginsForConfig: (config: unknown, options?: unknown) =>
listReadOnlyChannelPluginsForConfigMock(config, options),
}));
function requireFirstRuntimeLog(): string {
@@ -223,6 +227,46 @@ describe("healthCommand", () => {
expect(output).toContain("Gateway probe duration: 5ms");
});
it("surfaces unhealthy secondary accounts without an explicit account binding", async () => {
const primary = {
accountId: "main",
enabled: true,
configured: true,
linked: true,
healthState: "healthy",
probe: { ok: true, elapsedMs: 12 },
};
const snapshot = createHealthSummary({
channels: {
matrix: {
...primary,
accounts: {
main: primary,
alerts: {
accountId: "alerts",
enabled: true,
configured: true,
linked: true,
healthState: "blocked",
},
},
},
},
channelOrder: ["matrix"],
channelLabels: { matrix: "Matrix" },
});
callGatewayMock.mockResolvedValueOnce(snapshot);
listReadOnlyChannelPluginsForConfigMock.mockReturnValueOnce([
{ id: "matrix", config: { listAccountIds: () => ["main", "alerts"] } },
]);
await healthCommand({ json: false, timeoutMs: 1000, config: {} }, runtime as never);
const output = stripAnsi(runtime.log.mock.calls.map((call) => String(call[0])).join("\n"));
expect(output).toContain("Matrix: blocked");
expect(output).not.toContain("Matrix: ok");
});
it("shows every agent when an explicit fleet has no default owner", async () => {
const sessions = (agentId: string) => ({
path: `/tmp/${agentId}/sessions.json`,
-23
View File
@@ -394,24 +394,6 @@ export async function healthCommand(
runtime.log(` ${channelId}: ${probes.join(", ") || "(none)"}`);
}
}
const channelAccountFallbacks = Object.fromEntries(
displayPlugins.map((plugin) => {
const accountIds = plugin.config.listAccountIds(cfg);
const defaultAccountId = resolveChannelDefaultAccountId({
plugin,
cfg,
accountIds,
});
const preferred = resolvePreferredAccountId({
accountIds,
defaultAccountId,
boundAccounts: defaultAgentId
? (channelBindings.get(plugin.id)?.get(defaultAgentId) ?? [])
: [],
});
return [plugin.id, [preferred] as string[]] as const;
}),
);
const accountIdsByChannel = (() => {
const entries = displayAgents.length > 0 ? displayAgents : resolvedAgents;
const byChannel: Record<string, string[]> = {};
@@ -429,11 +411,6 @@ export async function healthCommand(
byChannel[channelId] = accountIds;
}
}
for (const [channelId, fallbackIds] of Object.entries(channelAccountFallbacks)) {
if (!byChannel[channelId] || byChannel[channelId].length === 0) {
byChannel[channelId] = fallbackIds;
}
}
return byChannel;
})();
const channelLines =