fix(status): keep diagnostics config reads read-only (#103252)

* fix: keep status config reads non-observing

Co-authored-by: Sascha Kuhlmann <coolmanns@users.noreply.github.com>

* fix: keep status preflight non-observing

* fix: keep status-all diagnosis non-observing

* chore: leave release changelog to maintainers

---------

Co-authored-by: Sascha Kuhlmann <coolmanns@users.noreply.github.com>
This commit is contained in:
Peter Steinberger
2026-07-10 05:16:44 +01:00
committed by GitHub
parent a2d5f32cef
commit 6fd9c1df69
10 changed files with 157 additions and 12 deletions
+8
View File
@@ -186,6 +186,12 @@ describe("ensureConfigReady", () => {
}
});
it("keeps status config guard reads non-observing", async () => {
await runEnsureConfigReady(["status"]);
expect(readConfigFileSnapshotMock).toHaveBeenCalledWith({ observe: false });
});
it("runs doctor flow when lightweight startup detection finds legacy state", async () => {
const root = useTempOpenClawHome();
writeLegacyTaskSidecarMarker(root);
@@ -196,6 +202,7 @@ describe("ensureConfigReady", () => {
migrateState: true,
migrateLegacyConfig: false,
invalidConfigNote: false,
observe: false,
});
});
@@ -209,6 +216,7 @@ describe("ensureConfigReady", () => {
migrateState: true,
migrateLegacyConfig: false,
invalidConfigNote: false,
observe: false,
});
});
+12 -4
View File
@@ -168,7 +168,10 @@ function shouldRequireStartupMigrationCheckpoint(commandPath: string[]): boolean
);
}
async function getConfigSnapshot() {
async function getConfigSnapshot(options?: { observe: false }) {
if (options?.observe === false) {
return readConfigFileSnapshot(options);
}
// Tests often mutate config fixtures; caching can make those flaky.
if (process.env.VITEST === "true") {
return readConfigFileSnapshot();
@@ -193,6 +196,8 @@ export async function ensureConfigReady(params: {
beforeStateMigrations?: (snapshot?: ConfigFileSnapshot) => Promise<boolean>;
}): Promise<void> {
const commandPath = params.commandPath ?? [];
const commandName = commandPath[0];
const subcommandName = commandPath[1];
let preflightSnapshot: Awaited<ReturnType<typeof readConfigFileSnapshot>> | null = null;
const shouldConsiderStateMigration = shouldMigrateStateFromPath(commandPath);
const requiresLegacyStateInput = shouldRunStateMigrationOnlyWithLegacyInputs(commandPath);
@@ -203,6 +208,7 @@ export async function ensureConfigReady(params: {
migrateState: true,
migrateLegacyConfig: false,
invalidConfigNote: false,
...(commandName === "status" ? { observe: false } : {}),
...(shouldRequireStartupMigrationCheckpoint(commandPath)
? { requireStartupMigrationCheckpoint: true }
: {}),
@@ -230,7 +236,11 @@ export async function ensureConfigReady(params: {
preflightSnapshot = await runStateMigrationPreflight();
}
let snapshot = preflightSnapshot ?? (await getConfigSnapshot());
// Status performs a second non-observing read for its materialized/source pair;
// keep the startup guard from recording config health before the command begins.
const configSnapshotOptions =
commandName === "status" ? ({ observe: false } as const) : undefined;
let snapshot = preflightSnapshot ?? (await getConfigSnapshot(configSnapshotOptions));
if (
!preflightSnapshot &&
!didRunDoctorConfigFlow &&
@@ -242,8 +252,6 @@ export async function ensureConfigReady(params: {
preflightSnapshot = await runStateMigrationPreflight();
snapshot = preflightSnapshot;
}
const commandName = commandPath[0];
const subcommandName = commandPath[1];
const isBareGatewayForegroundRun =
commandName === "gateway" && (subcommandName === undefined || subcommandName.trim() === "");
const isReadOnlyTaskStateCommand =
+40 -1
View File
@@ -1,14 +1,53 @@
// Doctor config preflight tests cover last-known-good snapshots and config snapshot promotion.
import fs from "node:fs/promises";
import { describe, expect, it } from "vitest";
import { afterEach, describe, expect, it } from "vitest";
import { promoteConfigSnapshotToLastKnownGood, readConfigFileSnapshot } from "../config/config.js";
import { withTempHome, writeOpenClawConfig } from "../config/test-helpers.js";
import { executeSqliteQueryTakeFirstSync, getNodeSqliteKysely } from "../infra/kysely-sync.js";
import type { DB as OpenClawStateKyselyDatabase } from "../state/openclaw-state-db.generated.js";
import {
closeOpenClawStateDatabaseForTest,
openOpenClawStateDatabase,
} from "../state/openclaw-state-db.js";
import {
runDoctorConfigPreflight,
shouldSkipPluginValidationForDoctorConfigPreflight,
} from "./doctor-config-preflight.js";
type ConfigHealthDatabase = Pick<OpenClawStateKyselyDatabase, "config_health_entries">;
function readConfigHealthRow(env: NodeJS.ProcessEnv, configPath: string) {
const { db } = openOpenClawStateDatabase({ env });
const healthDb = getNodeSqliteKysely<ConfigHealthDatabase>(db);
return executeSqliteQueryTakeFirstSync(
db,
healthDb
.selectFrom("config_health_entries")
.select("config_path")
.where("config_path", "=", configPath),
);
}
describe("runDoctorConfigPreflight", () => {
afterEach(() => {
closeOpenClawStateDatabaseForTest();
});
it("supports non-observing config reads", async () => {
await withTempHome(async (home) => {
const configPath = await writeOpenClawConfig(home, { gateway: { mode: "local" } });
await runDoctorConfigPreflight({
migrateState: false,
migrateLegacyConfig: false,
invalidConfigNote: false,
observe: false,
});
expect(readConfigHealthRow({ ...process.env, HOME: home }, configPath)).toBeUndefined();
});
});
it("skips plugin schema validation while doctor is running inside update", () => {
expect(
shouldSkipPluginValidationForDoctorConfigPreflight({
+2
View File
@@ -178,6 +178,7 @@ export async function runDoctorConfigPreflight(
repairPrefixedConfig?: boolean;
recoverCorruptTargetStore?: boolean;
invalidConfigNote?: string | false;
observe?: boolean;
/** Return false or reject on config drift; the preflight always unwinds owned resources. */
beforeStateMigrations?: (snapshot?: ConfigFileSnapshot) => Promise<boolean>;
requireStartupMigrationCheckpoint?: boolean;
@@ -254,6 +255,7 @@ export async function runDoctorConfigPreflight(
}
const readOptions = {
...(options.observe === false ? { observe: false } : {}),
skipPluginValidation: shouldSkipPluginValidationForDoctorConfigPreflight(),
};
let snapshot = addDoctorLegacyIssues(await readConfigFileSnapshot(readOptions));
@@ -0,0 +1,71 @@
// Status-all report data tests cover local read-only diagnosis probes.
import { beforeEach, describe, expect, it, vi } from "vitest";
const mocks = vi.hoisted(() => ({
readConfigFileSnapshot: vi.fn(async () => ({ path: "/tmp/openclaw.json" })),
}));
vi.mock("../../agents/exec-defaults.js", () => ({ canExecRequestNode: () => false }));
vi.mock("../../config/config.js", () => ({
readConfigFileSnapshot: mocks.readConfigFileSnapshot,
resolveGatewayPort: () => 18789,
}));
vi.mock("../../daemon/diagnostics.js", () => ({
readLastGatewayErrorLine: async () => null,
}));
vi.mock("../../infra/ports.js", () => ({ inspectPortUsage: async () => null }));
vi.mock("../../infra/restart-sentinel.js", () => ({ readRestartSentinel: async () => null }));
vi.mock("../../plugins/status.js", () => ({ buildPluginCompatibilityNotices: () => [] }));
vi.mock("../../skills/discovery/status.js", () => ({ buildWorkspaceSkillStatus: () => null }));
vi.mock("../../skills/runtime/remote.js", () => ({ getRemoteSkillEligibility: () => ({}) }));
vi.mock("../status-overview-rows.ts", () => ({ buildStatusAllOverviewRows: () => [] }));
vi.mock("../status-overview-surface.ts", () => ({
buildStatusOverviewSurfaceFromOverview: () => ({}),
}));
vi.mock("../status-runtime-shared.ts", () => ({
resolveStatusGatewayDiagnosticsSafe: async () => null,
resolveStatusGatewayHealthSafe: async () => undefined,
}));
vi.mock("../status-update-restart.ts", () => ({
formatUpdateRestartStatusValue: () => null,
}));
vi.mock("../status.gateway-connection.ts", () => ({
resolveStatusAllConnectionDetails: () => [],
}));
import { buildStatusAllReportData } from "./report-data.js";
describe("buildStatusAllReportData", () => {
beforeEach(() => {
mocks.readConfigFileSnapshot.mockClear();
});
it("keeps local config diagnosis non-observing", async () => {
await buildStatusAllReportData({
overview: {
cfg: {},
gatewaySnapshot: {
gatewayReachable: false,
gatewayProbe: null,
gatewayCallOverrides: undefined,
gatewayConnection: {},
remoteUrlMissing: false,
},
secretDiagnostics: [],
tailscaleMode: "off",
tailscaleDns: null,
agentStatus: { agents: [], defaultId: null },
channels: { rows: [], details: [] },
channelIssues: [],
osSummary: { label: "test" },
} as never,
daemon: {} as never,
nodeService: {} as never,
nodeOnlyGateway: {} as never,
progress: { setLabel: vi.fn(), tick: vi.fn() },
});
expect(mocks.readConfigFileSnapshot).toHaveBeenCalledOnce();
expect(mocks.readConfigFileSnapshot).toHaveBeenCalledWith({ observe: false });
});
});
+1 -1
View File
@@ -80,7 +80,7 @@ async function resolveStatusAllLocalDiagnosis(params: {
};
}> {
const { overview } = params;
const snap = await readConfigFileSnapshot().catch(() => null);
const snap = await readConfigFileSnapshot({ observe: false }).catch(() => null);
const configPath = resolveStatusAllConfigPath(snap?.path);
const health = params.nodeOnlyGateway
@@ -134,6 +134,10 @@ describe("collectStatusScanOverview", () => {
useGatewayCallOverridesForChannelsStatus: true,
});
expect(mocks.readBestEffortConfigSnapshot).toHaveBeenCalledWith({
observe: false,
skipPluginValidation: undefined,
});
expect(mocks.callGateway).toHaveBeenCalledOnce();
const gatewayRequest = firstGatewayRequest();
expect(gatewayRequest?.method).toBe("channels.status");
+1
View File
@@ -194,6 +194,7 @@ export async function collectStatusScanOverview(params: {
allowMissingConfigFastPath: params.allowMissingConfigFastPath,
readConfigSnapshot: async () =>
(await loadConfigModule()).readBestEffortConfigSnapshot({
observe: false,
skipPluginValidation: params.skipConfigPluginValidation,
}),
resolveConfig: async (loadedConfig) =>
+13 -3
View File
@@ -243,9 +243,9 @@ describe("readBestEffortConfig", () => {
});
});
it("returns source and materialized config from one snapshot", async () => {
it("controls observation while returning source and materialized config", async () => {
await withTempHome(async (home) => {
await writeOpenClawConfig(home, {
const configPath = await writeOpenClawConfig(home, {
auth: {
profiles: {
"anthropic:api": { provider: "anthropic", mode: "api_key" },
@@ -257,12 +257,22 @@ describe("readBestEffortConfig", () => {
},
},
});
const configRaw = await fs.readFile(configPath, "utf-8");
const snapshot = await readBestEffortConfigSnapshot();
const snapshot = await readBestEffortConfigSnapshot({ observe: false });
expect(snapshot.sourceConfig.agents?.defaults?.contextPruning?.mode).toBeUndefined();
expect(snapshot.config.agents?.defaults?.contextPruning?.mode).toBe("cache-ttl");
expect(snapshot.config.agents?.defaults?.compaction?.mode).toBe("safeguard");
await expect(fs.readFile(configPath, "utf-8")).resolves.toBe(configRaw);
expect(readConfigHealthRow({ ...process.env, HOME: home }, configPath)).toBeUndefined();
await readBestEffortConfigSnapshot();
expect(readConfigHealthRow({ ...process.env, HOME: home }, configPath)).toMatchObject({
config_path: configPath,
last_known_good_json: expect.any(String),
});
});
});
});
+5 -3
View File
@@ -2826,11 +2826,13 @@ export async function readBestEffortConfig(options?: {
}
export async function readBestEffortConfigSnapshot(options?: {
observe?: boolean;
skipPluginValidation?: boolean;
}): Promise<BestEffortConfigSnapshot> {
return await createConfigIO(
options?.skipPluginValidation ? { pluginValidation: "skip" } : {},
).readBestEffortConfigSnapshot();
return await createConfigIO({
...(options?.observe === false ? { observe: false } : {}),
...(options?.skipPluginValidation ? { pluginValidation: "skip" } : {}),
}).readBestEffortConfigSnapshot();
}
export async function readSourceConfigBestEffort(): Promise<OpenClawConfig> {