From 03f2a8114f5c9eef6ec19879e6e16f6b36c31f0f Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 12 Sep 2026 16:37:16 -0700 Subject: [PATCH] fix: honor standalone doctor config and validate provider names --- CHANGELOG.md | 10 +++----- docs/providers.md | 6 +++++ src/app.ts | 45 ++++++++------------------------- src/doctor.test.ts | 60 ++++++++++++++++++++++++++++++++++++++++++++ src/provider.test.ts | 7 ++++++ src/provider.ts | 2 +- 6 files changed, 89 insertions(+), 41 deletions(-) create mode 100644 src/doctor.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index ba56747..9a723e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,14 +2,12 @@ ## 0.8.1 - Unreleased -- Prevented oversized provider, validation, and PR-publishing timeout overrides from overflowing into one-millisecond deadlines. - -- Preserved observed source edits in failed patch attempts when a provider writes files before exiting with an error. - - Fixed nested-project repairs to ignore their own state and sibling changes, fingerprint project-relative source paths, and record both sides of renames. - +- Preserved observed source edits in failed patch attempts when a provider writes files before exiting with an error. +- Prevented oversized provider, validation, and PR-publishing timeout overrides from overflowing into one-millisecond deadlines. +- Fixed `doctor` to honor standalone provider configuration before project initialization. +- Rejected inherited object-property names as unsupported providers instead of failing during harness invocation. - Updated Zod and development tooling, aligned Node typings with the Node 22 floor, and added Node 22/24 runtime CI with pinned GitHub Actions. - - Updated workflow and architecture docs to match current providers, explicit PR creation, validation order, and stale-lock recovery. ## 0.8.0 - 2026-09-07 diff --git a/docs/providers.md b/docs/providers.md index f64d1bc..0ae0f31 100644 --- a/docs/providers.md +++ b/docs/providers.md @@ -14,6 +14,12 @@ providers are out of scope; see the project [vision](../VISION.md). clawpatch doctor ``` +`doctor` loads configuration before checking whether project state exists, so +`--config`, `CLAWPATCH_CONFIG`, and discovered project config also work before +`init`. Provider, model, and reasoning flags override environment and config +settings in the same order as review. Unknown provider names are rejected before +invoking a harness. + Provider names today: - `codex`: shells out to `codex exec` (default) diff --git a/src/app.ts b/src/app.ts index 4cc8bd3..1a9dfb0 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1,5 +1,5 @@ import { appendFile } from "node:fs/promises"; -import { loadConfig, parseReasoningEffort, resolveStateDir } from "./config.js"; +import { loadConfig, resolveStateDir } from "./config.js"; import { applyProviderFlags, providerOptions, stringFlag } from "./command-support.js"; import { loadProjectState, type AppContext } from "./app-context.js"; import { detectProject } from "./detect.js"; @@ -236,40 +236,17 @@ export async function doctorCommand( context: AppContext, flags: Record = {}, ): Promise { - let loaded: Awaited> | null; - try { - loaded = await loadProjectState(context); - } catch (error) { - if (error instanceof ClawpatchError && error.code === "not-initialized") { - loaded = null; - } else { - throw error; - } - } - const root = loaded?.root ?? context.root; - const providerName = - stringFlag(flags, "provider") ?? - process.env["CLAWPATCH_PROVIDER"] ?? - loaded?.config.provider.name ?? - "codex"; - const model = - stringFlag(flags, "model") ?? - process.env["CLAWPATCH_MODEL"] ?? - loaded?.config.provider.model ?? - null; - const reasoningEffort = - parseReasoningEffort(stringFlag(flags, "reasoningEffort")) ?? - parseReasoningEffort(process.env["CLAWPATCH_REASONING_EFFORT"]) ?? - loaded?.config.provider.reasoningEffort ?? - null; - const provider = providerByName(providerName); - const providerVersion = await provider.check(root); + const config = applyProviderFlags(await loadConfig(context.root, context.options), flags); + const paths = statePaths(resolveStateDir(context.root, config)); + const project = await readProject(paths); + const provider = providerByName(config.provider.name); + const providerVersion = await provider.check(context.root); return { - root, - state: loaded === null ? "missing" : "ok", - provider: providerName, - model, - reasoningEffort, + root: context.root, + state: project === null ? "missing" : "ok", + provider: config.provider.name, + model: config.provider.model, + reasoningEffort: config.provider.reasoningEffort, providerVersion, secrets: "redacted", }; diff --git a/src/doctor.test.ts b/src/doctor.test.ts new file mode 100644 index 0000000..d7376c7 --- /dev/null +++ b/src/doctor.test.ts @@ -0,0 +1,60 @@ +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { doctorCommand, makeContext } from "./app.js"; +import { defaultConfig } from "./config.js"; +import { pathExists } from "./fs.js"; +import { providerByName } from "./provider.js"; +import { fixtureRoot, testOptions, writeFixture } from "./test-helpers.js"; + +beforeEach(() => { + for (const name of [ + "CLAWPATCH_CONFIG", + "CLAWPATCH_PROVIDER", + "CLAWPATCH_MODEL", + "CLAWPATCH_REASONING_EFFORT", + ]) { + vi.stubEnv(name, undefined); + } + vi.spyOn(providerByName("codex"), "check").mockResolvedValue("unexpected default provider"); +}); +afterEach(() => vi.unstubAllEnvs()); + +describe("doctor configuration before initialization", () => { + it.each(["option", "environment"])("loads standalone config selected by %s", async (source) => { + const root = await fixtureRoot("clawpatch-doctor-config-"); + const config = defaultConfig(); + config.provider = { + ...config.provider, + name: "mock", + model: "fixture-model", + reasoningEffort: "high", + }; + await writeFixture(root, "trusted.json", JSON.stringify(config)); + const options = testOptions(root); + if (source === "option") options.config = join(root, "trusted.json"); + else vi.stubEnv("CLAWPATCH_CONFIG", join(root, "trusted.json")); + const context = await makeContext(options); + expect(await doctorCommand(context)).toMatchObject({ + state: "missing", + provider: "mock", + model: "fixture-model", + reasoningEffort: "high", + providerVersion: "mock", + }); + expect(await pathExists(join(root, ".clawpatch"))).toBe(false); + }); + + it("keeps flag overrides ahead of environment and config", async () => { + const root = await fixtureRoot("clawpatch-doctor-precedence-"); + const config = defaultConfig(); + config.provider.name = "mock-fail"; + await writeFixture(root, "clawpatch.config.json", JSON.stringify(config)); + vi.stubEnv("CLAWPATCH_PROVIDER", "codex"); + vi.stubEnv("CLAWPATCH_MODEL", "environment-model"); + const context = await makeContext(testOptions(root)); + expect(await doctorCommand(context, { provider: "mock", model: "flag-model" })).toMatchObject({ + provider: "mock", + model: "flag-model", + }); + }); +}); diff --git a/src/provider.test.ts b/src/provider.test.ts index 1258dca..25eedda 100644 --- a/src/provider.test.ts +++ b/src/provider.test.ts @@ -1738,6 +1738,13 @@ describe("extractOpencodeJson", () => { }); describe("providerByName", () => { + it.each(["constructor", "toString", "__proto__", "hasOwnProperty"])( + "rejects inherited object key %s as an unsupported provider", + (name) => { + expect(() => providerByName(name)).toThrow(`unsupported provider: ${name}`); + }, + ); + it("returns provider instances for optional CLI-backed providers", () => { expect(providerByName("acpx").name).toBe("acpx"); expect(providerByName("claude").name).toBe("claude"); diff --git a/src/provider.ts b/src/provider.ts index 0206b15..4f43652 100644 --- a/src/provider.ts +++ b/src/provider.ts @@ -26,7 +26,7 @@ const providers: Readonly> = { }; export function providerByName(name: string): Provider { - const provider = providers[name]; + const provider = Object.hasOwn(providers, name) ? providers[name] : undefined; if (provider !== undefined) { return provider; }