Skip to content

Commit d139d39

Browse files
giodl73-repogiodl73-repo
committed
refactor: simplify doctor repair checks
1 parent 679a46d commit d139d39

5 files changed

Lines changed: 577 additions & 58 deletions

File tree

‎src/flows/doctor-lint-flow.test.ts‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { describe, expect, it } from "vitest";
22
import { exitCodeFromFindings, runDoctorLintChecks } from "./doctor-lint-flow.js";
3+
import { normalizeHealthCheck } from "./health-check-adapter.js";
4+
import type { RunnableHealthCheck } from "./health-check-runner-types.js";
35
import type { HealthCheck, HealthCheckContext } from "./health-checks.js";
46

57
const ctx: HealthCheckContext = {
@@ -17,7 +19,7 @@ function check(id: string, detect: HealthCheck["detect"]): HealthCheck {
1719
id,
1820
kind: "core",
1921
description: id,
20-
detect,
22+
detect: detect ?? (async () => []),
2123
};
2224
}
2325

@@ -36,6 +38,34 @@ describe("runDoctorLintChecks", () => {
3638
expect(result.findings.map((finding) => finding.checkId)).toEqual(["a"]);
3739
});
3840

41+
it("supports single-run checks in lint mode", async () => {
42+
const runnable: RunnableHealthCheck = {
43+
id: "run-check",
44+
kind: "core",
45+
description: "run check",
46+
async run(runCtx) {
47+
expect(runCtx).toMatchObject({
48+
mode: "lint",
49+
repair: false,
50+
});
51+
return {
52+
findings: [
53+
{
54+
checkId: "run-check",
55+
severity: "warning",
56+
message: "warn",
57+
},
58+
],
59+
};
60+
},
61+
};
62+
const check = normalizeHealthCheck(runnable);
63+
64+
const result = await runDoctorLintChecks(ctx, { checks: [check] });
65+
66+
expect(result.findings.map((finding) => finding.checkId)).toEqual(["run-check"]);
67+
});
68+
3969
it("turns thrown checks into error findings", async () => {
4070
const result = await runDoctorLintChecks(ctx, {
4171
checks: [

‎src/flows/doctor-repair-flow.test.ts‎

Lines changed: 212 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { describe, expect, it } from "vitest";
22
import type { OpenClawConfig } from "../config/types.openclaw.js";
33
import { runDoctorHealthRepairs } from "./doctor-repair-flow.js";
4+
import { defineSplitHealthCheck, normalizeHealthCheck } from "./health-check-adapter.js";
5+
import type { RunnableHealthCheck } from "./health-check-runner-types.js";
46
import type { HealthCheck, HealthRepairContext } from "./health-checks.js";
57

68
function ctx(cfg: OpenClawConfig): HealthRepairContext {
@@ -16,10 +18,56 @@ function ctx(cfg: OpenClawConfig): HealthRepairContext {
1618
}
1719

1820
describe("runDoctorHealthRepairs", () => {
21+
it("repairs single-run checks and validates through lint mode", async () => {
22+
const runModes: string[] = [];
23+
const scopes: unknown[] = [];
24+
const runnable: RunnableHealthCheck = {
25+
id: "test/run-repairable",
26+
kind: "core",
27+
description: "run repairable",
28+
async run(ctx, scope) {
29+
runModes.push(ctx.mode);
30+
if (scope !== undefined) {
31+
scopes.push(scope);
32+
}
33+
const findings =
34+
ctx.cfg.gateway?.mode === "local"
35+
? []
36+
: [
37+
{
38+
checkId: "test/run-repairable",
39+
severity: "warning" as const,
40+
message: "gateway mode missing",
41+
path: "gateway.mode",
42+
},
43+
];
44+
if (!ctx.repair || findings.length === 0) {
45+
return { findings };
46+
}
47+
return {
48+
findings,
49+
config: { ...ctx.cfg, gateway: { ...ctx.cfg.gateway, mode: "local" } },
50+
changes: ["Set gateway.mode to local."],
51+
};
52+
},
53+
};
54+
const checks: HealthCheck[] = [normalizeHealthCheck(runnable)];
55+
56+
const result = await runDoctorHealthRepairs(ctx({}), { checks });
57+
58+
expect(result.config.gateway?.mode).toBe("local");
59+
expect(result.changes).toEqual(["Set gateway.mode to local."]);
60+
expect(result.checksRepaired).toBe(1);
61+
expect(result.checksValidated).toBe(1);
62+
expect(result.remainingFindings).toEqual([]);
63+
expect(runModes).toEqual(["fix", "lint"]);
64+
expect(scopes).toMatchObject([{ paths: ["gateway.mode"] }]);
65+
});
66+
1967
it("repairs modern checks and threads updated config", async () => {
2068
const scopes: unknown[] = [];
2169
const checks: HealthCheck[] = [
22-
{
70+
defineSplitHealthCheck({
2371
id: "test/repairable",
2472
kind: "core",
2573
description: "repairable",
@@ -44,7 +92,7 @@ describe("runDoctorHealthRepairs", () => {
4492
changes: ["Set gateway.mode to local."],
4593
};
4694
},
47-
},
95+
}),
4896
];
4997

5098
const result = await runDoctorHealthRepairs(ctx({}), { checks });
@@ -57,9 +105,35 @@ describe("runDoctorHealthRepairs", () => {
57105
expect(scopes).toMatchObject([{ paths: ["gateway.mode"] }]);
58106
});
59107

108+
it("keeps repairable out of split repair result types", () => {
109+
const check = defineSplitHealthCheck({
110+
id: "test/repair-result-status-boundary",
111+
kind: "core",
112+
description: "repair result status boundary",
113+
async detect() {
114+
return [
115+
{
116+
checkId: "test/repair-result-status-boundary",
117+
severity: "warning",
118+
message: "needs repair",
119+
},
120+
];
121+
},
122+
// @ts-expect-error repairable is a run-result preview status, not a split repair result.
123+
async repair() {
124+
return {
125+
status: "repairable",
126+
changes: [],
127+
};
128+
},
129+
});
130+
131+
expect(check.id).toBe("test/repair-result-status-boundary");
132+
});
133+
60134
it("leaves non-repairable checks for legacy doctor behavior", async () => {
61135
const checks: HealthCheck[] = [
62-
{
136+
defineSplitHealthCheck({
63137
id: "test/legacy-only",
64138
kind: "core",
65139
description: "legacy only",
@@ -72,7 +146,7 @@ describe("runDoctorHealthRepairs", () => {
72146
},
73147
];
74148
},
75-
},
149+
}),
76150
];
77151

78152
const result = await runDoctorHealthRepairs(ctx({}), { checks });
@@ -85,9 +159,44 @@ describe("runDoctorHealthRepairs", () => {
85159
expect(result.checksValidated).toBe(0);
86160
});
87161

88-
it("reports repair validation findings that remain after repair", async () => {
162+
it("keeps split check findings when repair throws", async () => {
89163
const checks: HealthCheck[] = [
164+
defineSplitHealthCheck({
165+
id: "test/repair-throws",
166+
kind: "core",
167+
description: "repair throws",
168+
async detect() {
169+
return [
170+
{
171+
checkId: "test/repair-throws",
172+
severity: "warning",
173+
message: "needs repair",
174+
path: "gateway.mode",
175+
},
176+
];
177+
},
178+
async repair() {
179+
throw new Error("repair exploded");
180+
},
181+
}),
182+
];
183+
184+
const result = await runDoctorHealthRepairs(ctx({}), { checks });
185+
186+
expect(result.findings).toMatchObject([
90187
{
188+
checkId: "test/repair-throws",
189+
path: "gateway.mode",
190+
},
191+
]);
192+
expect(result.warnings).toEqual(["test/repair-throws repair failed: repair exploded"]);
193+
expect(result.checksRepaired).toBe(0);
194+
expect(result.checksValidated).toBe(0);
195+
});
196+
197+
it("reports repair validation findings that remain after repair", async () => {
198+
const checks: HealthCheck[] = [
199+
defineSplitHealthCheck({
91200
id: "test/not-fixed",
92201
kind: "core",
93202
description: "not fixed",
@@ -106,7 +215,7 @@ describe("runDoctorHealthRepairs", () => {
106215
changes: ["Tried repair."],
107216
};
108217
},
109-
},
218+
}),
110219
];
111220

112221
const result = await runDoctorHealthRepairs(ctx({}), { checks });
@@ -122,10 +231,49 @@ describe("runDoctorHealthRepairs", () => {
122231
expect(result.warnings).toEqual(["test/not-fixed repair left 1 finding(s)"]);
123232
});
124233

234+
it("validates successful repairs by default", async () => {
235+
let detectCalls = 0;
236+
const checks: HealthCheck[] = [
237+
defineSplitHealthCheck({
238+
id: "test/no-default-validation",
239+
kind: "core",
240+
description: "no default validation",
241+
async detect() {
242+
detectCalls++;
243+
return [
244+
{
245+
checkId: "test/no-default-validation",
246+
severity: "warning",
247+
message: "needs repair",
248+
},
249+
];
250+
},
251+
async repair() {
252+
return {
253+
changes: ["Ran repair."],
254+
};
255+
},
256+
}),
257+
];
258+
259+
const result = await runDoctorHealthRepairs(ctx({}), { checks });
260+
261+
expect(detectCalls).toBe(2);
262+
expect(result.checksRepaired).toBe(1);
263+
expect(result.checksValidated).toBe(1);
264+
expect(result.remainingFindings).toEqual([
265+
{
266+
checkId: "test/no-default-validation",
267+
severity: "warning",
268+
message: "needs repair",
269+
},
270+
]);
271+
});
272+
125273
it("does not validate skipped or failed repair results", async () => {
126274
let validationCalls = 0;
127275
const checks: HealthCheck[] = [
128-
{
276+
defineSplitHealthCheck({
129277
id: "test/skipped",
130278
kind: "core",
131279
description: "skipped",
@@ -146,7 +294,7 @@ describe("runDoctorHealthRepairs", () => {
146294
changes: [],
147295
};
148296
},
149-
},
297+
}),
150298
];
151299

152300
const result = await runDoctorHealthRepairs(ctx({}), { checks });
@@ -162,7 +310,7 @@ describe("runDoctorHealthRepairs", () => {
162310
const repairContexts: HealthRepairContext[] = [];
163311
let detectCalls = 0;
164312
const checks: HealthCheck[] = [
165-
{
313+
defineSplitHealthCheck({
166314
id: "test/dry-run",
167315
kind: "core",
168316
description: "dry run",
@@ -202,7 +350,7 @@ describe("runDoctorHealthRepairs", () => {
202350
],
203351
};
204352
},
205-
},
353+
}),
206354
];
207355

208356
const result = await runDoctorHealthRepairs(ctx({}), {
@@ -220,4 +368,58 @@ describe("runDoctorHealthRepairs", () => {
220368
expect(detectCalls).toBe(1);
221369
expect(repairContexts[0]).toMatchObject({ dryRun: true, diff: true });
222370
});
371+
372+
it("passes diff false and true through the repair API", async () => {
373+
const repairContexts: HealthRepairContext[] = [];
374+
const checks: HealthCheck[] = [
375+
defineSplitHealthCheck({
376+
id: "test/diff-preview",
377+
kind: "core",
378+
description: "diff preview",
379+
async detect() {
380+
return [
381+
{
382+
checkId: "test/diff-preview",
383+
severity: "warning",
384+
message: "config needs repair",
385+
path: "gateway.mode",
386+
},
387+
];
388+
},
389+
async repair(ctx) {
390+
repairContexts.push(ctx);
391+
return {
392+
changes: ["Would set gateway.mode to local."],
393+
diffs:
394+
ctx.diff === true
395+
? [
396+
{
397+
kind: "config",
398+
path: "gateway.mode",
399+
before: undefined,
400+
after: "local",
401+
},
402+
]
403+
: [],
404+
};
405+
},
406+
}),
407+
];
408+
409+
const withoutDiff = await runDoctorHealthRepairs(ctx({}), {
410+
checks,
411+
dryRun: true,
412+
diff: false,
413+
});
414+
const withDiff = await runDoctorHealthRepairs(ctx({}), {
415+
checks,
416+
dryRun: true,
417+
diff: true,
418+
});
419+
420+
expect(repairContexts[0]).toMatchObject({ dryRun: true, diff: false });
421+
expect(withoutDiff.diffs).toEqual([]);
422+
expect(repairContexts[1]).toMatchObject({ dryRun: true, diff: true });
423+
expect(withDiff.diffs).toMatchObject([{ kind: "config", path: "gateway.mode" }]);
424+
});
223425
});

0 commit comments

Comments
 (0)