From cc290050b439bcc712636d580fe8eca2f703fe37 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sun, 31 May 2026 14:48:15 +0100 Subject: [PATCH] fix(doctor): diagnose malformed provider catalogs Move malformed static provider catalog diagnostics into `openclaw doctor` instead of adding fallback behavior to runtime projection. Doctor now validates full provider registrations for malformed static catalog hooks, result containers, provider keys, model arrays, model iteration, model ids/names, invalid catalog order, and proxy/access errors. Runtime unified text provider catalog projection remains strict on the typed provider catalog contract. Verification: - `node scripts/run-vitest.mjs src/flows/doctor-core-checks.runtime.test.ts src/flows/doctor-core-checks.test.ts src/flows/doctor-health-contributions.test.ts src/flows/doctor-health-conversion-plan.test.ts src/plugin-sdk/provider-entry.test.ts` - `node_modules/.bin/oxfmt --check src/plugins/provider-catalog-unified-text.ts src/flows/doctor-core-checks.ts src/flows/doctor-core-checks.test.ts src/flows/doctor-core-checks.runtime.ts src/flows/doctor-core-checks.runtime.test.ts src/flows/doctor-health-contributions.ts src/flows/doctor-health-contributions.test.ts src/flows/doctor-health-conversion-plan.ts src/flows/doctor-health-conversion-plan.test.ts` - `node scripts/run-oxlint.mjs src/flows/doctor-core-checks.runtime.ts src/flows/doctor-core-checks.runtime.test.ts src/plugins/provider-catalog-unified-text.ts` - `pnpm tsgo:test` - `git diff --check origin/main...HEAD` - `.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main --prompt-file /tmp/provider-catalog-doctor-review-context.txt` - GitHub PR checks green on head `876fdda5a352b0f15bfbe2abe9be43ebada7c596` Co-authored-by: Vincent Koc --- src/flows/doctor-core-checks.runtime.test.ts | 561 +++++++++++++++++- src/flows/doctor-core-checks.runtime.ts | 467 +++++++++++++++ src/flows/doctor-core-checks.test.ts | 40 ++ src/flows/doctor-core-checks.ts | 24 + src/flows/doctor-health-contributions.test.ts | 43 ++ src/flows/doctor-health-contributions.ts | 35 +- src/flows/doctor-health-conversion-plan.ts | 6 + src/plugins/provider-catalog-unified-text.ts | 2 + 8 files changed, 1165 insertions(+), 13 deletions(-) diff --git a/src/flows/doctor-core-checks.runtime.test.ts b/src/flows/doctor-core-checks.runtime.test.ts index c106b8280cd4..9814cf0f16b6 100644 --- a/src/flows/doctor-core-checks.runtime.test.ts +++ b/src/flows/doctor-core-checks.runtime.test.ts @@ -8,6 +8,7 @@ const mocks = vi.hoisted(() => ({ disposeBundleRuntime: vi.fn(), loadModelCatalog: vi.fn(async (): Promise>> => []), normalizeProviderToolSchemasWithPlugin: vi.fn(), + resolvePluginProviders: vi.fn((): Array> => []), resolveDefaultModelForAgent: vi.fn(() => ({ provider: "openai", model: "gpt-5.5" })), })); @@ -38,7 +39,16 @@ vi.mock("../plugins/provider-runtime.js", () => ({ normalizeProviderToolSchemasWithPlugin: mocks.normalizeProviderToolSchemasWithPlugin, })); -const { collectRuntimeToolSchemaFindings } = await import("./doctor-core-checks.runtime.js"); +vi.mock("../plugins/provider-discovery.js", async (importOriginal) => ({ + ...(await importOriginal()), +})); + +vi.mock("../plugins/providers.runtime.js", () => ({ + resolvePluginProviders: mocks.resolvePluginProviders, +})); + +const { collectProviderCatalogProjectionFindings, collectRuntimeToolSchemaFindings } = + await import("./doctor-core-checks.runtime.js"); function tool(name: string, parameters: unknown): AnyAgentTool { return { @@ -59,20 +69,21 @@ function bundleMcpTool(name: string, parameters: unknown): AnyAgentTool { describe("doctor runtime tool schema checks", () => { beforeEach(() => { mocks.createOpenClawCodingTools.mockReset().mockReturnValue([]); - mocks.createBundleMcpToolRuntime.mockReset().mockResolvedValue({ + mocks.createBundleMcpToolRuntime.mockReset().mockReturnValue({ tools: [], dispose: mocks.disposeBundleRuntime, }); - mocks.disposeBundleRuntime.mockReset().mockResolvedValue(undefined); + mocks.disposeBundleRuntime.mockReset().mockReturnValue(undefined); mocks.loadModelCatalog.mockClear(); mocks.normalizeProviderToolSchemasWithPlugin .mockReset() .mockImplementation(({ context }) => context.tools); + mocks.resolvePluginProviders.mockReset().mockReturnValue([]); mocks.resolveDefaultModelForAgent.mockClear(); }); it("reports active bundle MCP tool schemas that would be quarantined before a model turn", async () => { - mocks.createBundleMcpToolRuntime.mockResolvedValueOnce({ + mocks.createBundleMcpToolRuntime.mockReturnValueOnce({ tools: [ bundleMcpTool("dofbot__healthy", { type: "object", properties: {} }), bundleMcpTool("dofbot__dofbot_move_angles", { @@ -166,7 +177,7 @@ describe("doctor runtime tool schema checks", () => { }); it("reports bundle MCP runtime diagnostics when tool listing fails schema validation", async () => { - mocks.createBundleMcpToolRuntime.mockResolvedValueOnce({ + mocks.createBundleMcpToolRuntime.mockReturnValueOnce({ tools: [], diagnostics: [ { @@ -201,7 +212,7 @@ describe("doctor runtime tool schema checks", () => { }); it("reports bundle MCP runtime diagnostics for exact MCP tool allowlists", async () => { - mocks.createBundleMcpToolRuntime.mockResolvedValueOnce({ + mocks.createBundleMcpToolRuntime.mockReturnValueOnce({ tools: [], diagnostics: [ { @@ -232,7 +243,7 @@ describe("doctor runtime tool schema checks", () => { }); it("reports exact MCP allowlists when the safe server name contains the separator", async () => { - mocks.createBundleMcpToolRuntime.mockResolvedValueOnce({ + mocks.createBundleMcpToolRuntime.mockReturnValueOnce({ tools: [], diagnostics: [ { @@ -263,7 +274,7 @@ describe("doctor runtime tool schema checks", () => { }); it("reports bundle MCP runtime diagnostics for glob MCP tool allowlists", async () => { - mocks.createBundleMcpToolRuntime.mockResolvedValueOnce({ + mocks.createBundleMcpToolRuntime.mockReturnValueOnce({ tools: [], diagnostics: [ { @@ -416,7 +427,7 @@ describe("doctor runtime tool schema checks", () => { }); it("does not report bundle MCP schemas filtered out by the final runtime tool policy", async () => { - mocks.createBundleMcpToolRuntime.mockResolvedValueOnce({ + mocks.createBundleMcpToolRuntime.mockReturnValueOnce({ tools: [ bundleMcpTool("dofbot__dofbot_move_angles", { type: "array", @@ -439,7 +450,7 @@ describe("doctor runtime tool schema checks", () => { }); it("does not report bundle MCP diagnostics filtered out by the final runtime tool policy", async () => { - mocks.createBundleMcpToolRuntime.mockResolvedValueOnce({ + mocks.createBundleMcpToolRuntime.mockReturnValueOnce({ tools: [], diagnostics: [ { @@ -465,7 +476,7 @@ describe("doctor runtime tool schema checks", () => { }); it("does not report bundle MCP diagnostics filtered out by server-level deny policy", async () => { - mocks.createBundleMcpToolRuntime.mockResolvedValueOnce({ + mocks.createBundleMcpToolRuntime.mockReturnValueOnce({ tools: [], diagnostics: [ { @@ -490,3 +501,531 @@ describe("doctor runtime tool schema checks", () => { ).resolves.toEqual([]); }); }); + +describe("doctor provider catalog projection checks", () => { + beforeEach(() => { + mocks.resolvePluginProviders.mockReset().mockReturnValue([]); + }); + + it("reports provider catalog rows that fail unified text projection", async () => { + const providers = Object.defineProperty( + { + healthy: { + api: "openai-completions" as const, + baseUrl: "https://healthy.test/v1", + models: [{ id: "healthy-model", name: "Healthy Model", maxTokens: 1 }], + }, + }, + "broken", + { + enumerable: true, + get() { + throw new Error("provider catalog entry read failed"); + }, + }, + ); + mocks.resolvePluginProviders.mockReturnValueOnce([ + { + id: "mockplugin", + pluginId: "mockplugin", + label: "Mock", + auth: [], + staticCatalog: { + order: "simple", + run: async () => ({ providers }), + }, + }, + ]); + + await expect(collectProviderCatalogProjectionFindings({})).resolves.toContainEqual({ + checkId: "core/doctor/provider-catalog-projection", + severity: "error", + message: "Provider catalog broken entry cannot be read during doctor validation.", + path: "plugins.entries.mockplugin", + target: "broken", + requirement: "provider catalog entry read failed", + fixHint: + "Fix the plugin provider catalog hook or disable the plugin, then rerun doctor before relying on model discovery.", + }); + }); + + it("loads full provider registrations for static catalog validation", async () => { + await collectProviderCatalogProjectionFindings({}); + + expect(mocks.resolvePluginProviders).toHaveBeenCalledWith( + expect.not.objectContaining({ + discoveryEntriesOnly: true, + }), + ); + }); + + it("reports provider catalog model rows with invalid ids", async () => { + mocks.resolvePluginProviders.mockReturnValueOnce([ + { + id: "mockplugin", + pluginId: "mockplugin", + label: "Mock", + auth: [], + staticCatalog: { + order: "simple", + run: async () => ({ + providers: { + mockplugin: { + api: "openai-completions" as const, + baseUrl: "https://mockplugin.test/v1", + models: [{ name: "Missing ID" }], + }, + }, + }), + }, + }, + ]); + + const findings = await collectProviderCatalogProjectionFindings({}); + expect(findings).toContainEqual( + expect.objectContaining({ + checkId: "core/doctor/provider-catalog-projection", + severity: "error", + path: "plugins.entries.mockplugin", + target: "mockplugin", + message: "Provider catalog mockplugin model row 0 has an invalid model id.", + requirement: "model id must be a non-empty trimmed string", + }), + ); + }); + + it("reports whitespace-only provider catalog model ids", async () => { + mocks.resolvePluginProviders.mockReturnValueOnce([ + { + id: "mockplugin", + pluginId: "mockplugin", + label: "Mock", + auth: [], + staticCatalog: { + order: "simple", + run: async () => ({ + providers: { + mockplugin: { + api: "openai-completions" as const, + baseUrl: "https://mockplugin.test/v1", + models: [{ id: " " }], + }, + }, + }), + }, + }, + ]); + + const findings = await collectProviderCatalogProjectionFindings({}); + expect(findings).toContainEqual( + expect.objectContaining({ + checkId: "core/doctor/provider-catalog-projection", + severity: "error", + path: "plugins.entries.mockplugin", + target: "mockplugin", + message: "Provider catalog mockplugin model row 0 has an invalid model id.", + requirement: "model id must be a non-empty trimmed string", + }), + ); + }); + + it("reports provider catalog model rows with invalid names", async () => { + mocks.resolvePluginProviders.mockReturnValueOnce([ + { + id: "mockplugin", + pluginId: "mockplugin", + label: "Mock", + auth: [], + staticCatalog: { + order: "simple", + run: async () => ({ + provider: { + api: "openai-completions" as const, + baseUrl: "https://mockplugin.test/v1", + models: [{ id: "mock-model", name: { label: "Mock" } }], + }, + }), + }, + }, + ]); + + const findings = await collectProviderCatalogProjectionFindings({}); + expect(findings).toContainEqual( + expect.objectContaining({ + checkId: "core/doctor/provider-catalog-projection", + severity: "error", + path: "plugins.entries.mockplugin", + target: "mockplugin", + message: "Provider catalog mockplugin model row 0 has an invalid model name.", + requirement: "model name must be a string when present", + }), + ); + }); + + it("reports provider catalog model lists with invalid shapes", async () => { + mocks.resolvePluginProviders.mockReturnValueOnce([ + { + id: "mockplugin", + pluginId: "mockplugin", + label: "Mock", + auth: [], + staticCatalog: { + order: "simple", + run: async () => ({ + provider: { + api: "openai-completions" as const, + baseUrl: "https://mockplugin.test/v1", + models: {}, + }, + }), + }, + }, + ]); + + const findings = await collectProviderCatalogProjectionFindings({}); + expect(findings).toContainEqual( + expect.objectContaining({ + checkId: "core/doctor/provider-catalog-projection", + severity: "error", + path: "plugins.entries.mockplugin", + target: "mockplugin", + message: "Provider catalog mockplugin models value is invalid during doctor validation.", + requirement: "models must be an array", + }), + ); + }); + + it("reports provider catalog model lists with invalid iterators", async () => { + const models = [{ id: "mock-model" }]; + Object.defineProperty(models, Symbol.iterator, { + value: () => { + throw new Error("model iterator failed"); + }, + }); + mocks.resolvePluginProviders.mockReturnValueOnce([ + { + id: "mockplugin", + pluginId: "mockplugin", + label: "Mock", + auth: [], + staticCatalog: { + order: "simple", + run: async () => ({ + provider: { + api: "openai-completions" as const, + baseUrl: "https://mockplugin.test/v1", + models, + }, + }), + }, + }, + ]); + + const findings = await collectProviderCatalogProjectionFindings({}); + expect(findings).toContainEqual( + expect.objectContaining({ + checkId: "core/doctor/provider-catalog-projection", + severity: "error", + path: "plugins.entries.mockplugin", + target: "mockplugin", + message: + "Provider catalog mockplugin model rows cannot be enumerated during doctor validation.", + requirement: "model iterator failed", + }), + ); + }); + + it("reports provider catalog results without provider containers", async () => { + mocks.resolvePluginProviders.mockReturnValueOnce([ + { + id: "mockplugin", + pluginId: "mockplugin", + label: "Mock", + auth: [], + staticCatalog: { + order: "simple", + run: async () => ({ providers: undefined }), + }, + }, + ]); + + await expect(collectProviderCatalogProjectionFindings({})).resolves.toContainEqual( + expect.objectContaining({ + checkId: "core/doctor/provider-catalog-projection", + severity: "error", + path: "plugins.entries.mockplugin", + target: "mockplugin", + message: "Provider catalog mockplugin result is invalid during doctor validation.", + requirement: "result must include provider or providers object", + }), + ); + }); + + it("reports invalid multi-provider catalog keys", async () => { + mocks.resolvePluginProviders.mockReturnValueOnce([ + { + id: "mockplugin", + pluginId: "mockplugin", + label: "Mock", + auth: [], + staticCatalog: { + order: "simple", + run: async () => ({ + providers: { + " ": { + api: "openai-completions" as const, + baseUrl: "https://mockplugin.test/v1", + models: [{ id: "mock-model" }], + }, + }, + }), + }, + }, + ]); + + await expect(collectProviderCatalogProjectionFindings({})).resolves.toContainEqual( + expect.objectContaining({ + checkId: "core/doctor/provider-catalog-projection", + severity: "error", + path: "plugins.entries.mockplugin", + target: "mockplugin", + message: "Provider catalog mockplugin provider key is invalid during doctor validation.", + requirement: "provider key must be a non-empty trimmed string", + }), + ); + }); + + it("reports falsy non-empty provider catalog results", async () => { + mocks.resolvePluginProviders.mockReturnValueOnce([ + { + id: "mockplugin", + pluginId: "mockplugin", + label: "Mock", + auth: [], + staticCatalog: { + order: "simple", + run: async () => false as never, + }, + }, + ]); + + await expect(collectProviderCatalogProjectionFindings({})).resolves.toContainEqual( + expect.objectContaining({ + checkId: "core/doctor/provider-catalog-projection", + severity: "error", + path: "plugins.entries.mockplugin", + target: "mockplugin", + message: "Provider catalog mockplugin result is invalid during doctor validation.", + requirement: "result must be an object", + }), + ); + }); + + it("reports invalid provider catalog orders without aborting doctor", async () => { + mocks.resolvePluginProviders.mockReturnValueOnce([ + { + id: "mockplugin", + pluginId: "mockplugin", + label: "Mock", + auth: [], + staticCatalog: { + order: "middle" as never, + run: async () => ({ + providers: { + mockplugin: { + api: "openai-completions" as const, + baseUrl: "https://mockplugin.test/v1", + models: [{ id: " " }], + }, + }, + }), + }, + }, + ]); + + const findings = await collectProviderCatalogProjectionFindings({}); + expect(findings).toContainEqual( + expect.objectContaining({ + checkId: "core/doctor/provider-catalog-projection", + severity: "error", + path: "plugins.entries.mockplugin", + target: "mockplugin", + message: "Provider catalog mockplugin order is invalid during doctor validation.", + requirement: "order must be simple, profile, paired, or late", + }), + ); + expect(findings).toContainEqual( + expect.objectContaining({ + checkId: "core/doctor/provider-catalog-projection", + severity: "error", + path: "plugins.entries.mockplugin", + target: "mockplugin", + message: "Provider catalog mockplugin model row 0 has an invalid model id.", + requirement: "model id must be a non-empty trimmed string", + }), + ); + }); + + it("validates static catalog rows when live catalog order access fails", async () => { + mocks.resolvePluginProviders.mockReturnValueOnce([ + { + id: "mockplugin", + pluginId: "mockplugin", + label: "Mock", + auth: [], + get catalog() { + throw new Error("live catalog order failed"); + }, + staticCatalog: { + order: "simple", + run: async () => ({ + providers: { + mockplugin: { + api: "openai-completions" as const, + baseUrl: "https://mockplugin.test/v1", + models: [{ id: " " }], + }, + }, + }), + }, + }, + ]); + + await expect(collectProviderCatalogProjectionFindings({})).resolves.toContainEqual( + expect.objectContaining({ + checkId: "core/doctor/provider-catalog-projection", + severity: "error", + path: "plugins.entries.mockplugin", + target: "mockplugin", + message: "Provider catalog mockplugin model row 0 has an invalid model id.", + requirement: "model id must be a non-empty trimmed string", + }), + ); + }); + + it("reports static catalog hook access failures without aborting doctor", async () => { + mocks.resolvePluginProviders.mockReturnValueOnce([ + { + id: "mockplugin", + pluginId: "mockplugin", + label: "Mock", + auth: [], + staticCatalog: { + order: "simple", + get run() { + throw new Error("run getter failed"); + }, + }, + }, + ]); + + await expect(collectProviderCatalogProjectionFindings({})).resolves.toContainEqual( + expect.objectContaining({ + checkId: "core/doctor/provider-catalog-projection", + severity: "error", + path: "plugins.entries.mockplugin", + target: "mockplugin", + message: + "Provider catalog mockplugin static catalog hook cannot be read during doctor validation.", + requirement: "run getter failed", + }), + ); + }); + + it("reports static catalog hooks with non-function run values", async () => { + mocks.resolvePluginProviders.mockReturnValueOnce([ + { + id: "mockplugin", + pluginId: "mockplugin", + label: "Mock", + auth: [], + staticCatalog: { + order: "simple", + run: "not-callable", + }, + }, + ]); + + await expect(collectProviderCatalogProjectionFindings({})).resolves.toContainEqual( + expect.objectContaining({ + checkId: "core/doctor/provider-catalog-projection", + severity: "error", + path: "plugins.entries.mockplugin", + target: "mockplugin", + message: + "Provider catalog mockplugin static catalog hook is invalid during doctor validation.", + requirement: "static catalog run must be a function", + }), + ); + }); + + it("reports revoked provider catalog result proxies without crashing doctor", async () => { + const { proxy, revoke } = Proxy.revocable( + { + providers: {}, + }, + {}, + ); + revoke(); + mocks.resolvePluginProviders.mockReturnValueOnce([ + { + id: "mockplugin", + pluginId: "mockplugin", + label: "Mock", + auth: [], + staticCatalog: { + order: "simple", + // Awaiting a promise resolved with a proxy reads "then", so revoked + // catalog results fail at the hook boundary before result key checks. + run: async () => proxy, + }, + }, + ]); + + await expect(collectProviderCatalogProjectionFindings({})).resolves.toContainEqual( + expect.objectContaining({ + checkId: "core/doctor/provider-catalog-projection", + severity: "error", + path: "plugins.entries.mockplugin", + target: "mockplugin", + message: "Provider catalog mockplugin failed during doctor validation.", + requirement: "Cannot perform 'get' on a proxy that has been revoked", + }), + ); + }); + + it("reports present but invalid single-provider catalog branches", async () => { + mocks.resolvePluginProviders.mockReturnValueOnce([ + { + id: "mockplugin", + pluginId: "mockplugin", + label: "Mock", + auth: [], + staticCatalog: { + order: "simple", + run: async () => ({ + provider: undefined, + providers: { + mockplugin: { + api: "openai-completions" as const, + baseUrl: "https://mockplugin.test/v1", + models: [{ id: "mock-model" }], + }, + }, + }), + }, + }, + ]); + + await expect(collectProviderCatalogProjectionFindings({})).resolves.toContainEqual( + expect.objectContaining({ + checkId: "core/doctor/provider-catalog-projection", + severity: "error", + path: "plugins.entries.mockplugin", + target: "mockplugin", + message: "Provider catalog mockplugin provider value is invalid during doctor validation.", + requirement: "provider must be an object", + }), + ); + }); +}); diff --git a/src/flows/doctor-core-checks.runtime.ts b/src/flows/doctor-core-checks.runtime.ts index d95af1a7ea91..06d7dfa00685 100644 --- a/src/flows/doctor-core-checks.runtime.ts +++ b/src/flows/doctor-core-checks.runtime.ts @@ -6,6 +6,7 @@ import { import { listAgentEntries, listAgentIds, + resolveDefaultAgentDir, resolveAgentWorkspaceDir, resolveDefaultAgentId, } from "../agents/agent-scope.js"; @@ -33,11 +34,14 @@ import type { OpenClawConfig } from "../config/types.openclaw.js"; import { formatErrorMessage } from "../infra/errors.js"; import type { ProviderRuntimeModel } from "../plugins/provider-runtime-model.types.js"; import { getPluginToolMeta, setPluginToolMeta } from "../plugins/tools.js"; +import type { ProviderCatalogOrder, ProviderPlugin } from "../plugins/types.js"; import { normalizeAgentId } from "../routing/session-key.js"; import { buildWorkspaceSkillStatus, type SkillStatusEntry } from "../skills/discovery/status.js"; import type { HealthFinding } from "./health-checks.js"; type BundleMcpToolRuntime = Awaited>; +const PROVIDER_CATALOG_ORDERS = ["simple", "profile", "paired", "late"] as const; +const PROVIDER_CATALOG_ORDER_SET = new Set(PROVIDER_CATALOG_ORDERS); export function detectUnavailableSkills(cfg: OpenClawConfig): SkillStatusEntry[] { const agentId = resolveDefaultAgentId(cfg); @@ -49,6 +53,469 @@ export function detectUnavailableSkills(cfg: OpenClawConfig): SkillStatusEntry[] return collectUnavailableAgentSkills(report); } +function providerCatalogPath(pluginId: string | undefined): string | undefined { + return pluginId ? `plugins.entries.${pluginId}` : undefined; +} + +function providerCatalogProjectionFinding(params: { + providerId: string; + pluginId?: string; + message: string; + error: unknown; +}): HealthFinding { + const path = providerCatalogPath(params.pluginId); + return { + checkId: "core/doctor/provider-catalog-projection", + severity: "error", + message: params.message, + ...(path ? { path } : {}), + target: params.providerId, + requirement: formatErrorMessage(params.error), + fixHint: + "Fix the plugin provider catalog hook or disable the plugin, then rerun doctor before relying on model discovery.", + }; +} + +function isReadableRecord(value: unknown): value is Record { + return value !== null && typeof value === "object"; +} + +function isTrimmedNonEmptyString(value: unknown): value is string { + return typeof value === "string" && value.trim() === value && value.length > 0; +} + +function hasProviderCatalogKey(params: { + value: Record; + key: string; + providerId: string; + pluginId?: string; +}): { ok: true; present: boolean } | { ok: false; finding: HealthFinding } { + try { + return { ok: true, present: params.key in params.value }; + } catch (error) { + return { + ok: false, + finding: providerCatalogProjectionFinding({ + providerId: params.providerId, + pluginId: params.pluginId, + message: `Provider catalog ${params.providerId} result keys cannot be checked during doctor validation.`, + error, + }), + }; + } +} + +function readProviderCatalogValue(params: { + value: unknown; + key: string; + providerId: string; + pluginId?: string; +}): { ok: true; value: unknown } | { ok: false; finding: HealthFinding } { + if (!isReadableRecord(params.value)) { + return { ok: true, value: undefined }; + } + try { + return { ok: true, value: params.value[params.key] }; + } catch (error) { + return { + ok: false, + finding: providerCatalogProjectionFinding({ + providerId: params.providerId, + pluginId: params.pluginId, + message: `Provider catalog ${params.providerId} entry cannot be read during doctor validation.`, + error, + }), + }; + } +} + +function collectProviderCatalogModelFindings(params: { + providerId: string; + pluginId?: string; + models: unknown; +}): HealthFinding[] { + const findings: HealthFinding[] = []; + let models: unknown[]; + try { + if (!Array.isArray(params.models)) { + return [ + providerCatalogProjectionFinding({ + providerId: params.providerId, + pluginId: params.pluginId, + message: `Provider catalog ${params.providerId} models value is invalid during doctor validation.`, + error: new Error("models must be an array"), + }), + ]; + } + models = params.models; + } catch (error) { + return [ + providerCatalogProjectionFinding({ + providerId: params.providerId, + pluginId: params.pluginId, + message: `Provider catalog ${params.providerId} models value cannot be checked during doctor validation.`, + error, + }), + ]; + } + let modelEntries: Array<[number, unknown]>; + try { + modelEntries = []; + let index = 0; + for (const model of models) { + modelEntries.push([index, model]); + index += 1; + } + } catch (error) { + return [ + providerCatalogProjectionFinding({ + providerId: params.providerId, + pluginId: params.pluginId, + message: `Provider catalog ${params.providerId} model rows cannot be enumerated during doctor validation.`, + error, + }), + ]; + } + for (const [index, model] of modelEntries) { + const modelId = readProviderCatalogValue({ + value: model, + key: "id", + providerId: params.providerId, + pluginId: params.pluginId, + }); + if (!modelId.ok) { + findings.push(modelId.finding); + continue; + } + if (!isTrimmedNonEmptyString(modelId.value)) { + findings.push( + providerCatalogProjectionFinding({ + providerId: params.providerId, + pluginId: params.pluginId, + message: `Provider catalog ${params.providerId} model row ${index} has an invalid model id.`, + error: new Error("model id must be a non-empty trimmed string"), + }), + ); + } + const modelName = readProviderCatalogValue({ + value: model, + key: "name", + providerId: params.providerId, + pluginId: params.pluginId, + }); + if (!modelName.ok) { + findings.push(modelName.finding); + continue; + } + if (modelName.value !== undefined && typeof modelName.value !== "string") { + findings.push( + providerCatalogProjectionFinding({ + providerId: params.providerId, + pluginId: params.pluginId, + message: `Provider catalog ${params.providerId} model row ${index} has an invalid model name.`, + error: new Error("model name must be a string when present"), + }), + ); + } + } + return findings; +} + +function collectProviderCatalogResultFindings(params: { + providerId: string; + pluginId?: string; + result: unknown; +}): HealthFinding[] { + if (params.result == null) { + return []; + } + if (!isReadableRecord(params.result)) { + return [ + providerCatalogProjectionFinding({ + providerId: params.providerId, + pluginId: params.pluginId, + message: `Provider catalog ${params.providerId} result is invalid during doctor validation.`, + error: new Error("result must be an object"), + }), + ]; + } + const hasProvider = hasProviderCatalogKey({ + value: params.result, + key: "provider", + providerId: params.providerId, + pluginId: params.pluginId, + }); + if (!hasProvider.ok) { + return [hasProvider.finding]; + } + const provider = readProviderCatalogValue({ + value: params.result, + key: "provider", + providerId: params.providerId, + pluginId: params.pluginId, + }); + if (!provider.ok) { + return [provider.finding]; + } + if (hasProvider.present && !isReadableRecord(provider.value)) { + return [ + providerCatalogProjectionFinding({ + providerId: params.providerId, + pluginId: params.pluginId, + message: `Provider catalog ${params.providerId} provider value is invalid during doctor validation.`, + error: new Error("provider must be an object"), + }), + ]; + } + if (isReadableRecord(provider.value)) { + const models = readProviderCatalogValue({ + value: provider.value, + key: "models", + providerId: params.providerId, + pluginId: params.pluginId, + }); + return models.ok + ? collectProviderCatalogModelFindings({ ...params, models: models.value }) + : [models.finding]; + } + + const providers = readProviderCatalogValue({ + value: params.result, + key: "providers", + providerId: params.providerId, + pluginId: params.pluginId, + }); + if (!providers.ok) { + return [providers.finding]; + } + if (!isReadableRecord(providers.value)) { + return [ + providerCatalogProjectionFinding({ + providerId: params.providerId, + pluginId: params.pluginId, + message: `Provider catalog ${params.providerId} result is invalid during doctor validation.`, + error: new Error("result must include provider or providers object"), + }), + ]; + } + let providerIds: string[]; + try { + providerIds = Object.keys(providers.value); + } catch (error) { + return [ + providerCatalogProjectionFinding({ + providerId: params.providerId, + pluginId: params.pluginId, + message: `Provider catalog ${params.providerId} provider entries cannot be enumerated during doctor validation.`, + error, + }), + ]; + } + const findings: HealthFinding[] = []; + for (const providerId of providerIds) { + if (!isTrimmedNonEmptyString(providerId)) { + findings.push( + providerCatalogProjectionFinding({ + providerId: params.providerId, + pluginId: params.pluginId, + message: `Provider catalog ${params.providerId} provider key is invalid during doctor validation.`, + error: new Error("provider key must be a non-empty trimmed string"), + }), + ); + continue; + } + const providerConfig = readProviderCatalogValue({ + value: providers.value, + key: providerId, + providerId, + pluginId: params.pluginId, + }); + if (!providerConfig.ok) { + findings.push(providerConfig.finding); + continue; + } + if (!isReadableRecord(providerConfig.value)) { + findings.push( + providerCatalogProjectionFinding({ + providerId, + pluginId: params.pluginId, + message: `Provider catalog ${providerId} provider entry is invalid during doctor validation.`, + error: new Error("provider entry must be an object"), + }), + ); + continue; + } + const models = readProviderCatalogValue({ + value: providerConfig.value, + key: "models", + providerId, + pluginId: params.pluginId, + }); + findings.push( + ...(models.ok + ? collectProviderCatalogModelFindings({ + providerId, + pluginId: params.pluginId, + models: models.value, + }) + : [models.finding]), + ); + } + return findings; +} + +function readProviderCatalogOrder( + provider: ProviderPlugin, +): { ok: true; order: ProviderCatalogOrder } | { ok: false; finding: HealthFinding } { + let order: unknown; + try { + order = provider.staticCatalog?.order ?? "late"; + } catch (error) { + return { + ok: false, + finding: providerCatalogProjectionFinding({ + providerId: provider.id, + pluginId: provider.pluginId, + message: `Provider catalog ${provider.id} order cannot be read during doctor validation.`, + error, + }), + }; + } + if (PROVIDER_CATALOG_ORDER_SET.has(order as ProviderCatalogOrder)) { + return { ok: true, order: order as ProviderCatalogOrder }; + } + return { + ok: false, + finding: providerCatalogProjectionFinding({ + providerId: provider.id, + pluginId: provider.pluginId, + message: `Provider catalog ${provider.id} order is invalid during doctor validation.`, + error: new Error("order must be simple, profile, paired, or late"), + }), + }; +} + +function groupProviderCatalogsForDoctor(providers: readonly ProviderPlugin[]): { + findings: HealthFinding[]; + byOrder: Record; +} { + const findings: HealthFinding[] = []; + const byOrder: Record = { + simple: [], + profile: [], + paired: [], + late: [], + }; + for (const provider of providers) { + const order = readProviderCatalogOrder(provider); + if (!order.ok) { + findings.push(order.finding); + byOrder.late.push(provider); + continue; + } + byOrder[order.order].push(provider); + } + for (const order of PROVIDER_CATALOG_ORDERS) { + byOrder[order].sort((a, b) => a.label.localeCompare(b.label)); + } + return { findings, byOrder }; +} + +export async function collectProviderCatalogProjectionFindings( + cfg: OpenClawConfig, +): Promise { + const { runProviderStaticCatalog } = await import("../plugins/provider-discovery.js"); + const { resolvePluginProviders } = await import("../plugins/providers.runtime.js"); + const env = process.env; + const agentDir = resolveDefaultAgentDir(cfg); + const workspaceDir = resolveAgentWorkspaceDir(cfg, resolveDefaultAgentId(cfg)); + let providers: Awaited>; + try { + providers = resolvePluginProviders({ + config: cfg, + workspaceDir, + env, + includeUntrustedWorkspacePlugins: false, + }); + } catch (error) { + return [ + { + checkId: "core/doctor/provider-catalog-projection", + severity: "error", + message: "Provider catalog hooks could not be loaded for doctor validation.", + requirement: formatErrorMessage(error), + fixHint: "Fix plugin provider discovery loading, then rerun doctor.", + }, + ]; + } + + const findings: HealthFinding[] = []; + const grouped = groupProviderCatalogsForDoctor(providers); + findings.push(...grouped.findings); + for (const order of PROVIDER_CATALOG_ORDERS) { + for (const provider of grouped.byOrder[order]) { + let staticCatalog: unknown; + let staticCatalogRun: unknown; + try { + staticCatalog = provider.staticCatalog; + staticCatalogRun = isReadableRecord(staticCatalog) ? staticCatalog.run : undefined; + } catch (error) { + findings.push( + providerCatalogProjectionFinding({ + providerId: provider.id, + pluginId: provider.pluginId, + message: `Provider catalog ${provider.id} static catalog hook cannot be read during doctor validation.`, + error, + }), + ); + continue; + } + if (staticCatalog === undefined) { + continue; + } + if (typeof staticCatalogRun !== "function") { + findings.push( + providerCatalogProjectionFinding({ + providerId: provider.id, + pluginId: provider.pluginId, + message: `Provider catalog ${provider.id} static catalog hook is invalid during doctor validation.`, + error: new Error("static catalog run must be a function"), + }), + ); + continue; + } + let result: Awaited>; + try { + result = await runProviderStaticCatalog({ + provider, + config: cfg, + agentDir, + workspaceDir, + env, + }); + } catch (error) { + findings.push( + providerCatalogProjectionFinding({ + providerId: provider.id, + pluginId: provider.pluginId, + message: `Provider catalog ${provider.id} failed during doctor validation.`, + error, + }), + ); + continue; + } + findings.push( + ...collectProviderCatalogResultFindings({ + providerId: provider.id, + pluginId: provider.pluginId, + result, + }), + ); + } + } + return findings; +} + function buildDoctorRuntimeModel(params: { entry?: ModelCatalogEntry; provider: string; diff --git a/src/flows/doctor-core-checks.test.ts b/src/flows/doctor-core-checks.test.ts index 003327bfcdf5..3e0cb039382b 100644 --- a/src/flows/doctor-core-checks.test.ts +++ b/src/flows/doctor-core-checks.test.ts @@ -80,6 +80,9 @@ function createDeps(overrides: Partial = {}): CoreHealthChe async collectRuntimeToolSchemaFindings() { return []; }, + async collectProviderCatalogProjectionFindings() { + return []; + }, ...overrides, }; } @@ -664,4 +667,41 @@ describe("registerCoreHealthChecks", () => { }), ); }); + + it("reports active provider catalog projection findings", async () => { + const check = getCheck( + createCoreHealthChecks( + createDeps({ + async collectProviderCatalogProjectionFindings(): Promise { + return [ + { + checkId: "core/doctor/provider-catalog-projection", + severity: "error", + message: + "Provider catalog mockplugin cannot be projected into the unified text model catalog.", + path: "plugins.entries.mockplugin", + target: "mockplugin", + requirement: "mockplugin provider catalog entry read failed", + }, + ]; + }, + }), + ), + "core/doctor/provider-catalog-projection", + ); + + await expect( + check.detect({ + mode: "doctor", + runtime, + cfg: {}, + }), + ).resolves.toContainEqual( + expect.objectContaining({ + checkId: "core/doctor/provider-catalog-projection", + severity: "error", + target: "mockplugin", + }), + ); + }); }); diff --git a/src/flows/doctor-core-checks.ts b/src/flows/doctor-core-checks.ts index a78ff5bfaa60..8aa4f73d9150 100644 --- a/src/flows/doctor-core-checks.ts +++ b/src/flows/doctor-core-checks.ts @@ -42,6 +42,9 @@ export type CoreHealthCheckDeps = { readonly collectRuntimeToolSchemaFindings: ( ctx: HealthCheckContext, ) => Promise; + readonly collectProviderCatalogProjectionFindings: ( + ctx: HealthCheckContext, + ) => Promise; }; async function detectUnavailableSkillsWithRuntime( @@ -79,11 +82,19 @@ async function collectRuntimeToolSchemaFindingsWithRuntime( return runtime.collectRuntimeToolSchemaFindings(ctx.cfg); } +async function collectProviderCatalogProjectionFindingsWithRuntime( + ctx: HealthCheckContext, +): Promise { + const runtime = await loadDoctorCoreChecksRuntimeModule(); + return runtime.collectProviderCatalogProjectionFindings(ctx.cfg); +} + const defaultCoreHealthCheckDeps: CoreHealthCheckDeps = { detectUnavailableSkills: detectUnavailableSkillsWithRuntime, collectSecurityWarnings: collectSecurityWarningsWithRuntime, collectWorkspaceSuggestionNotes: collectWorkspaceSuggestionNotesWithRuntime, collectRuntimeToolSchemaFindings: collectRuntimeToolSchemaFindingsWithRuntime, + collectProviderCatalogProjectionFindings: collectProviderCatalogProjectionFindingsWithRuntime, }; export function configValidationIssuesToHealthFindings( @@ -421,6 +432,18 @@ function createRuntimeToolSchemaCheck(deps: CoreHealthCheckDeps): HealthCheck { }; } +function createProviderCatalogProjectionCheck(deps: CoreHealthCheckDeps): HealthCheck { + return { + id: "core/doctor/provider-catalog-projection", + kind: "core", + description: "Provider catalog hooks project into unified text model catalog rows.", + source: "doctor", + async detect(ctx) { + return deps.collectProviderCatalogProjectionFindings(ctx); + }, + }; +} + function normalizeDoctorNoteLine(line: string): string { return line.replace(/^- /, "").trim(); } @@ -896,6 +919,7 @@ function createConvertedWorkflowChecks(deps: CoreHealthCheckDeps): readonly Heal openAIOAuthTlsCheck, hooksModelCheck, bootstrapSizeCheck, + createProviderCatalogProjectionCheck(deps), createRuntimeToolSchemaCheck(deps), createWorkspaceSuggestionsCheck(deps), ]; diff --git a/src/flows/doctor-health-contributions.test.ts b/src/flows/doctor-health-contributions.test.ts index 07eceb61866f..154b62f16629 100644 --- a/src/flows/doctor-health-contributions.test.ts +++ b/src/flows/doctor-health-contributions.test.ts @@ -384,6 +384,49 @@ describe("doctor health contributions", () => { ); }); + it("reports provider catalog projection blockers during normal doctor runs", async () => { + const contribution = requireDoctorContribution("doctor:provider-catalog-projection"); + mocks.getHealthCheck.mockReturnValue({ + id: "core/doctor/provider-catalog-projection", + detect: vi.fn(async () => [ + { + checkId: "core/doctor/provider-catalog-projection", + severity: "error", + message: + "Provider catalog mockplugin cannot be projected into the unified text model catalog.", + path: "plugins.entries.mockplugin", + target: "mockplugin", + requirement: "provider catalog entry read failed", + fixHint: + "Fix the plugin provider catalog hook or disable the plugin, then rerun doctor before relying on model discovery.", + }, + ]), + }); + const ctx = { + cfg: {}, + configResult: { cfg: {} }, + sourceConfigValid: true, + prompter: buildDoctorPrompter(false), + runtime: { log: vi.fn(), error: vi.fn(), exit: vi.fn() }, + options: {}, + cfgForPersistence: {}, + configPath: "/tmp/fake-openclaw.json", + env: {}, + } as Parameters<(typeof contribution)["run"]>[0]; + + await contribution.run(ctx); + + expect(ctx.healthOk).toBe(false); + expect(mocks.note).toHaveBeenCalledWith( + expect.stringContaining("Provider catalog mockplugin cannot be projected"), + "Doctor warnings", + ); + expect(mocks.note).toHaveBeenCalledWith( + expect.stringContaining("issue: provider catalog entry read failed"), + "Doctor warnings", + ); + }); + it("skips doctor config writes under legacy update parents", () => { expect( shouldSkipLegacyUpdateDoctorConfigWrite({ diff --git a/src/flows/doctor-health-contributions.ts b/src/flows/doctor-health-contributions.ts index 896ee64b7775..c187997703ac 100644 --- a/src/flows/doctor-health-contributions.ts +++ b/src/flows/doctor-health-contributions.ts @@ -935,7 +935,7 @@ async function runFinalConfigValidationHealth(ctx: DoctorHealthFlowContext): Pro } } -function formatRuntimeToolSchemaFindings(findings: readonly HealthFinding[]): string { +function formatHealthFindings(findings: readonly HealthFinding[]): string { return findings .map((finding) => { const lines = [`- ${finding.message}`]; @@ -953,6 +953,31 @@ function formatRuntimeToolSchemaFindings(findings: readonly HealthFinding[]): st .join("\n"); } +async function runProviderCatalogProjectionHealth(ctx: DoctorHealthFlowContext): Promise { + const { registerCoreHealthChecks } = await loadDoctorCoreChecksModule(); + const { getHealthCheck } = await loadHealthCheckRegistryModule(); + const { resolveAgentWorkspaceDir, resolveDefaultAgentId } = await loadAgentScopeModule(); + const { note } = await loadNoteModule(); + + registerCoreHealthChecks(); + const check = getHealthCheck("core/doctor/provider-catalog-projection"); + if (!check) { + return; + } + const findings = await check.detect({ + mode: "doctor", + runtime: ctx.runtime, + cfg: ctx.cfg, + cwd: resolveAgentWorkspaceDir(ctx.cfg, resolveDefaultAgentId(ctx.cfg)), + configPath: ctx.configPath, + }); + if (findings.length === 0) { + return; + } + ctx.healthOk = false; + note(formatHealthFindings(findings), "Doctor warnings"); +} + async function runRuntimeToolSchemasHealth(ctx: DoctorHealthFlowContext): Promise { const { registerCoreHealthChecks } = await loadDoctorCoreChecksModule(); const { getHealthCheck } = await loadHealthCheckRegistryModule(); @@ -975,7 +1000,7 @@ async function runRuntimeToolSchemasHealth(ctx: DoctorHealthFlowContext): Promis return; } ctx.healthOk = false; - note(formatRuntimeToolSchemaFindings(findings), "Doctor warnings"); + note(formatHealthFindings(findings), "Doctor warnings"); } export function resolveDoctorHealthContributions(): DoctorHealthContribution[] { @@ -1116,6 +1141,12 @@ export function resolveDoctorHealthContributions(): DoctorHealthContribution[] { label: "Tool result cap", run: runToolResultCapHealth, }), + createDoctorHealthContribution({ + id: "doctor:provider-catalog-projection", + label: "Provider catalog projection", + healthCheckIds: ["core/doctor/provider-catalog-projection"], + run: runProviderCatalogProjectionHealth, + }), createDoctorHealthContribution({ id: "doctor:runtime-tool-schemas", label: "Runtime tool schemas", diff --git a/src/flows/doctor-health-conversion-plan.ts b/src/flows/doctor-health-conversion-plan.ts index c10437113f79..ffa4fc0a6214 100644 --- a/src/flows/doctor-health-conversion-plan.ts +++ b/src/flows/doctor-health-conversion-plan.ts @@ -179,6 +179,12 @@ export const doctorHealthConversionRules = [ target: ["core/doctor/tool-result-cap"], rule: "Detect explicit live tool-result cap overrides that are stale or ineffective; preserve deep-mode effective cap output as finding metadata.", }, + { + contributionId: "doctor:provider-catalog-projection", + conversion: "detect-only", + target: ["core/doctor/provider-catalog-projection"], + rule: "Validate provider catalog hooks against unified text catalog projection and report malformed plugin catalog rows during doctor.", + }, { contributionId: "doctor:runtime-tool-schemas", conversion: "detect-only", diff --git a/src/plugins/provider-catalog-unified-text.ts b/src/plugins/provider-catalog-unified-text.ts index c90fba4b4929..2e5142b2d8b8 100644 --- a/src/plugins/provider-catalog-unified-text.ts +++ b/src/plugins/provider-catalog-unified-text.ts @@ -14,6 +14,8 @@ export function projectProviderCatalogResultToUnifiedTextRows(params: { ? { [params.providerId]: params.result.provider } : params.result.providers; const rows: UnifiedModelCatalogEntry[] = []; + // Doctor owns malformed plugin catalog diagnostics; runtime projection stays on + // the typed provider catalog contract instead of carrying fallback semantics. for (const [providerId, providerConfig] of Object.entries(providers)) { for (const model of providerConfig.models ?? []) { rows.push({