Skip to content

Commit 5b063c2

Browse files
steipetesteipete
committed
fix: keep legacy config repair in doctor
1 parent 3f2c3a6 commit 5b063c2

10 files changed

Lines changed: 177 additions & 123 deletions

‎src/cli/program/config-guard.test.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@ import { ensureConfigReady, __test__ } from "./config-guard.js";
33

44
const loadAndMaybeMigrateDoctorConfigMock = vi.hoisted(() => vi.fn());
55
const readConfigFileSnapshotMock = vi.hoisted(() => vi.fn());
6+
const setRuntimeConfigSnapshotMock = vi.hoisted(() => vi.fn());
67

78
vi.mock("../../commands/doctor-config-preflight.js", () => ({
89
runDoctorConfigPreflight: loadAndMaybeMigrateDoctorConfigMock,
910
}));
1011

1112
vi.mock("../../config/config.js", () => ({
1213
readConfigFileSnapshot: readConfigFileSnapshotMock,
14+
setRuntimeConfigSnapshot: setRuntimeConfigSnapshotMock,
1315
}));
1416

1517
function makeSnapshot() {
@@ -105,6 +107,23 @@ describe("ensureConfigReady", () => {
105107
}
106108
});
107109

110+
it("pins a valid preflight snapshot for command code reuse", async () => {
111+
const snapshot = {
112+
...makeSnapshot(),
113+
config: { runtime: true },
114+
runtimeConfig: { runtime: true, materialized: true },
115+
sourceConfig: { source: true },
116+
};
117+
readConfigFileSnapshotMock.mockResolvedValue(snapshot);
118+
119+
await runEnsureConfigReady(["status"]);
120+
121+
expect(setRuntimeConfigSnapshotMock).toHaveBeenCalledWith(
122+
snapshot.runtimeConfig,
123+
snapshot.sourceConfig,
124+
);
125+
});
126+
108127
it("exits for invalid config on non-allowlisted commands", async () => {
109128
setInvalidSnapshot();
110129
const runtime = await runEnsureConfigReady(["message"]);

‎src/cli/program/config-guard.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { readConfigFileSnapshot } from "../../config/config.js";
1+
import { readConfigFileSnapshot, setRuntimeConfigSnapshot } from "../../config/config.js";
22
import type { RuntimeEnv } from "../../runtime.js";
33
import { shouldMigrateStateFromPath } from "../argv.js";
44

@@ -93,6 +93,9 @@ export async function ensureConfigReady(params: {
9393
snapshot.legacyIssues.length > 0 ? formatConfigIssueLines(snapshot.legacyIssues, "-") : [];
9494

9595
const invalid = snapshot.exists && !snapshot.valid;
96+
if (!invalid) {
97+
setRuntimeConfigSnapshot(snapshot.runtimeConfig ?? snapshot.config, snapshot.sourceConfig);
98+
}
9699
if (!invalid) {
97100
return;
98101
}

‎src/commands/doctor-config-preflight.test.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { describe, expect, it } from "vitest";
2+
import { withTempHome, writeOpenClawConfig } from "../config/test-helpers.js";
3+
import { runDoctorConfigPreflight } from "./doctor-config-preflight.js";
4+
5+
describe("runDoctorConfigPreflight", () => {
6+
it("collects legacy config issues outside the normal config read path", async () => {
7+
await withTempHome(async (home) => {
8+
await writeOpenClawConfig(home, {
9+
memorySearch: {
10+
provider: "local",
11+
fallback: "none",
12+
},
13+
});
14+
15+
const preflight = await runDoctorConfigPreflight({
16+
migrateState: false,
17+
migrateLegacyConfig: false,
18+
invalidConfigNote: false,
19+
});
20+
21+
expect(preflight.snapshot.valid).toBe(false);
22+
expect(preflight.snapshot.legacyIssues.some((issue) => issue.path === "memorySearch")).toBe(
23+
true,
24+
);
25+
expect((preflight.baseConfig as { memorySearch?: unknown }).memorySearch).toMatchObject({
26+
provider: "local",
27+
fallback: "none",
28+
});
29+
});
30+
});
31+
});

‎src/commands/doctor-config-preflight.ts‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,13 @@ import fs from "node:fs/promises";
22
import path from "node:path";
33
import { readConfigFileSnapshot, recoverConfigFromJsonRootSuffix } from "../config/io.js";
44
import { formatConfigIssueLines } from "../config/issue-format.js";
5+
import { findLegacyConfigIssues } from "../config/legacy.js";
6+
import type { LegacyConfigIssue } from "../config/types.js";
57
import type { OpenClawConfig } from "../config/types.openclaw.js";
8+
import {
9+
collectRelevantDoctorPluginIds,
10+
listPluginDoctorLegacyConfigRules,
11+
} from "../plugins/doctor-contract-registry.js";
612
import { note } from "../terminal/note.js";
713
import { resolveHomeDir } from "../utils.js";
814
import { noteIncludeConfinementWarning } from "./doctor-config-analysis.js";
@@ -55,6 +61,33 @@ export type DoctorConfigPreflightResult = {
5561
baseConfig: OpenClawConfig;
5662
};
5763

64+
function collectDoctorLegacyIssues(
65+
snapshot: Awaited<ReturnType<typeof readConfigFileSnapshot>>,
66+
): LegacyConfigIssue[] {
67+
if (!snapshot.exists) {
68+
return [];
69+
}
70+
const resolvedRaw = snapshot.sourceConfig ?? snapshot.config ?? {};
71+
const sourceRaw = snapshot.parsed ?? resolvedRaw;
72+
return findLegacyConfigIssues(
73+
resolvedRaw,
74+
sourceRaw,
75+
listPluginDoctorLegacyConfigRules({
76+
pluginIds: collectRelevantDoctorPluginIds(resolvedRaw),
77+
}),
78+
);
79+
}
80+
81+
function addDoctorLegacyIssues(
82+
snapshot: Awaited<ReturnType<typeof readConfigFileSnapshot>>,
83+
): Awaited<ReturnType<typeof readConfigFileSnapshot>> {
84+
const legacyIssues = collectDoctorLegacyIssues(snapshot);
85+
if (legacyIssues.length === 0) {
86+
return snapshot;
87+
}
88+
return { ...snapshot, legacyIssues };
89+
}
90+
5891
export async function runDoctorConfigPreflight(
5992
options: {
6093
migrateState?: boolean;
@@ -81,15 +114,15 @@ export async function runDoctorConfigPreflight(
81114
}
82115
}
83116

84-
let snapshot = await readConfigFileSnapshot();
117+
let snapshot = addDoctorLegacyIssues(await readConfigFileSnapshot());
85118
if (
86119
options.repairPrefixedConfig === true &&
87120
snapshot.exists &&
88121
!snapshot.valid &&
89122
(await recoverConfigFromJsonRootSuffix(snapshot))
90123
) {
91124
note("Removed non-JSON prefix from openclaw.json; original saved as .clobbered.*.", "Config");
92-
snapshot = await readConfigFileSnapshot();
125+
snapshot = addDoctorLegacyIssues(await readConfigFileSnapshot());
93126
}
94127
const invalidConfigNote =
95128
options.invalidConfigNote ?? "Config invalid; doctor will run with best-effort config.";

‎src/commands/models/load-config.runtime.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
export { getModelsCommandSecretTargetIds } from "../../cli/command-secret-targets.js";
22
export {
33
getRuntimeConfig,
4-
readSourceConfigSnapshotForWrite,
4+
getRuntimeConfigSourceSnapshot,
55
setRuntimeConfigSnapshot,
66
type OpenClawConfig,
77
} from "../../config/config.js";

‎src/commands/models/load-config.test.ts‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
22

33
const mocks = vi.hoisted(() => ({
44
getRuntimeConfig: vi.fn(),
5-
readSourceConfigSnapshotForWrite: vi.fn(),
5+
getRuntimeConfigSourceSnapshot: vi.fn(),
66
setRuntimeConfigSnapshot: vi.fn(),
77
resolveCommandSecretRefsViaGateway: vi.fn(),
88
getModelsCommandSecretTargetIds: vi.fn(),
99
}));
1010

1111
vi.mock("../../config/config.js", () => ({
1212
getRuntimeConfig: mocks.getRuntimeConfig,
13-
readSourceConfigSnapshotForWrite: mocks.readSourceConfigSnapshotForWrite,
13+
getRuntimeConfigSourceSnapshot: mocks.getRuntimeConfigSourceSnapshot,
1414
setRuntimeConfigSnapshot: mocks.setRuntimeConfigSnapshot,
1515
}));
1616

@@ -35,10 +35,7 @@ describe("models load-config", () => {
3535

3636
function mockResolvedConfigFlow(params: { sourceConfig: unknown; diagnostics: string[] }) {
3737
mocks.getRuntimeConfig.mockReturnValue(runtimeConfig);
38-
mocks.readSourceConfigSnapshotForWrite.mockResolvedValue({
39-
snapshot: { valid: true, sourceConfig: params.sourceConfig, resolved: params.sourceConfig },
40-
writeOptions: {},
41-
});
38+
mocks.getRuntimeConfigSourceSnapshot.mockReturnValue(params.sourceConfig);
4239
mocks.getModelsCommandSecretTargetIds.mockReturnValue(targetIds);
4340
mocks.resolveCommandSecretRefsViaGateway.mockResolvedValue({
4441
resolvedConfig,
@@ -88,4 +85,19 @@ describe("models load-config", () => {
8885
await expect(loadModelsConfig({ commandName: "models list" })).resolves.toBe(resolvedConfig);
8986
expect(mocks.setRuntimeConfigSnapshot).toHaveBeenCalledWith(resolvedConfig, sourceConfig);
9087
});
88+
89+
it("does not reread config when no source snapshot is pinned", async () => {
90+
mocks.getRuntimeConfig.mockReturnValue(runtimeConfig);
91+
mocks.getRuntimeConfigSourceSnapshot.mockReturnValue(null);
92+
mocks.getModelsCommandSecretTargetIds.mockReturnValue(targetIds);
93+
mocks.resolveCommandSecretRefsViaGateway.mockResolvedValue({
94+
resolvedConfig,
95+
diagnostics: [],
96+
});
97+
98+
const result = await loadModelsConfigWithSource({ commandName: "models list" });
99+
100+
expect(result.sourceConfig).toBe(runtimeConfig);
101+
expect(mocks.setRuntimeConfigSnapshot).toHaveBeenCalledWith(resolvedConfig);
102+
});
91103
});

‎src/commands/models/load-config.ts‎

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { resolveCommandConfigWithSecrets } from "../../cli/command-config-resolu
22
import type { RuntimeEnv } from "../../runtime.js";
33
import {
44
getRuntimeConfig,
5-
readSourceConfigSnapshotForWrite,
5+
getRuntimeConfigSourceSnapshot,
66
setRuntimeConfigSnapshot,
77
type OpenClawConfig,
88
getModelsCommandSecretTargetIds,
@@ -14,31 +14,24 @@ export type LoadedModelsConfig = {
1414
diagnostics: string[];
1515
};
1616

17-
async function loadSourceConfigSnapshot(fallback: OpenClawConfig): Promise<OpenClawConfig> {
18-
try {
19-
const { snapshot } = await readSourceConfigSnapshotForWrite();
20-
if (snapshot.valid) {
21-
return snapshot.sourceConfig;
22-
}
23-
} catch {
24-
// Fall back to runtime-loaded config if source snapshot cannot be read.
25-
}
26-
return fallback;
27-
}
28-
2917
export async function loadModelsConfigWithSource(params: {
3018
commandName: string;
3119
runtime?: RuntimeEnv;
3220
}): Promise<LoadedModelsConfig> {
3321
const runtimeConfig = getRuntimeConfig();
34-
const sourceConfig = await loadSourceConfigSnapshot(runtimeConfig);
22+
const pinnedSourceConfig = getRuntimeConfigSourceSnapshot();
23+
const sourceConfig = pinnedSourceConfig ?? runtimeConfig;
3524
const { resolvedConfig, diagnostics } = await resolveCommandConfigWithSecrets({
3625
config: runtimeConfig,
3726
commandName: params.commandName,
3827
targetIds: getModelsCommandSecretTargetIds(),
3928
runtime: params.runtime,
4029
});
41-
setRuntimeConfigSnapshot(resolvedConfig, sourceConfig);
30+
if (pinnedSourceConfig) {
31+
setRuntimeConfigSnapshot(resolvedConfig, sourceConfig);
32+
} else {
33+
setRuntimeConfigSnapshot(resolvedConfig);
34+
}
4235
return {
4336
sourceConfig,
4437
resolvedConfig,

0 commit comments

Comments
 (0)