From f252d1d342c9ee24364a2a0b2e88c985d1fcd9dd Mon Sep 17 00:00:00 2001 From: Mikael Hugo Date: Tue, 5 May 2026 22:57:26 +0200 Subject: [PATCH] fix: keep doctor focused on actionable state --- .../extensions/sf/doctor-providers.js | 47 +++++++++- src/resources/extensions/sf/doctor.js | 28 +++++- .../sf/tests/doctor-providers.test.mjs | 86 +++++++++++++++++++ .../sf/tests/doctor-sf-form-lint.test.mjs | 30 +++++++ 4 files changed, 185 insertions(+), 6 deletions(-) create mode 100644 src/resources/extensions/sf/tests/doctor-providers.test.mjs diff --git a/src/resources/extensions/sf/doctor-providers.js b/src/resources/extensions/sf/doctor-providers.js index 8a2eb54a0..f5919d274 100644 --- a/src/resources/extensions/sf/doctor-providers.js +++ b/src/resources/extensions/sf/doctor-providers.js @@ -36,6 +36,15 @@ function modelToProviderId(model) { "google-vertex": "google-vertex", anthropic: "anthropic", openai: "openai", + zai: "zai", + minimax: "minimax", + "kimi-coding": "kimi-coding", + xiaomi: "xiaomi", + ollama: "ollama", + "ollama-cloud": "ollama-cloud", + opencode: "opencode", + "opencode-go": "opencode-go", + "alibaba-coding-plan": "alibaba-coding-plan", }; if (prefixMap[prefix]) return prefixMap[prefix]; } @@ -161,6 +170,21 @@ const CLI_AUTH_PROVIDERS = new Set([ function checkLlmProviders() { const required = collectConfiguredModelProviders(); const results = []; + const lookups = new Map(); + for (const providerId of required) { + if (CLI_AUTH_PROVIDERS.has(providerId)) { + lookups.set(providerId, { + found: true, + source: "cli", + backedOff: false, + }); + continue; + } + lookups.set(providerId, resolveKey(providerId)); + } + const hasUsableConfiguredProvider = [...lookups.values()].some( + (lookup) => lookup.found && !lookup.backedOff, + ); for (const providerId of required) { // CLI-authenticated providers don't need API keys — skip key check if (CLI_AUTH_PROVIDERS.has(providerId)) { @@ -180,7 +204,7 @@ function checkLlmProviders() { providerId === "anthropic-vertex" ? "Anthropic Vertex" : (info?.label ?? providerId); - const lookup = resolveKey(providerId); + const lookup = lookups.get(providerId) ?? resolveKey(providerId); if (!lookup.found) { // Check if a cross-provider can serve this provider's models const routes = PROVIDER_ROUTES[providerId]; @@ -200,6 +224,17 @@ function checkLlmProviders() { }); continue; } + if (hasUsableConfiguredProvider) { + results.push({ + name: providerId, + label, + category: "llm", + status: "unconfigured", + message: `${label} — not configured (another configured model route is available)`, + required: false, + }); + continue; + } const envVar = providerId === "anthropic-vertex" ? "ANTHROPIC_VERTEX_PROJECT_ID" @@ -259,16 +294,20 @@ function checkRemoteQuestionsProvider() { const label = info?.label ?? channel; const lookup = resolveKey(providerId); if (!lookup.found) { + const autoResolvable = + rq.auto_resolve_on_timeout === true && rq.auto_resolve_strategy; return { name: providerId, label, category: "remote", - status: "warning", - message: `${label} — channel configured but token not found`, + status: autoResolvable ? "unconfigured" : "warning", + message: autoResolvable + ? `${label} — token not found (headless questions auto-resolve on timeout)` + : `${label} — channel configured but token not found`, detail: info?.envVar ? `Set ${info.envVar} or run /sf keys` : `Run /sf keys to configure`, - required: true, + required: !autoResolvable, }; } return { diff --git a/src/resources/extensions/sf/doctor.js b/src/resources/extensions/sf/doctor.js index 7a11bee20..53f47d2bb 100644 --- a/src/resources/extensions/sf/doctor.js +++ b/src/resources/extensions/sf/doctor.js @@ -1061,7 +1061,19 @@ function matchesScope(unitId, scope) { if (!scope) return true; return unitId === scope || unitId.startsWith(`${scope}/`); } -function auditRequirements(content) { +function shouldRunDeepMilestoneDoctorChecks(milestoneId, state, scope) { + if (scope) { + return ( + matchesScope(milestoneId, scope) || scope.startsWith(`${milestoneId}/`) + ); + } + return milestoneId === state.activeMilestone?.id; +} +function isLegacySlugDuplicateSliceDir(slicesDir, entry) { + const match = entry.match(/^(S\d+)-.+$/); + return Boolean(match && existsSync(join(slicesDir, match[1]))); +} +function auditRequirements(content, options = {}) { if (!content) return []; const issues = []; const blocks = content.split(/^###\s+/m).slice(1); @@ -1088,6 +1100,7 @@ function auditRequirements(content) { status === "active" && (!owner || owner === "none" || owner === "none yet") ) { + if (!options.includeOwnerWarnings) continue; // #4414: Downgrade to warning. A newly-created requirement has // primary_owner='' by default until the planning agent wires it to // a slice via sf_requirement_update. Flagging as error during normal @@ -1361,9 +1374,13 @@ export async function runSFDoctor(basePath, options) { } const requirementsPath = resolveSfRootFile(basePath, "REQUIREMENTS"); const requirementsContent = await loadFile(requirementsPath); - issues.push(...auditRequirements(requirementsContent)); const t0state = Date.now(); const state = await deriveState(basePath); + issues.push( + ...auditRequirements(requirementsContent, { + includeOwnerWarnings: Boolean(options?.scope), + }), + ); // Provider / auth health checks — only relevant when there is active work to dispatch. // Skipped for idle projects (no active milestone) to avoid noise in environments // where CI/test runners have no API key configured. @@ -1402,6 +1419,12 @@ export async function runSFDoctor(basePath, options) { const milestoneId = milestone.id; const milestonePath = resolveMilestonePath(basePath, milestoneId); if (!milestonePath) continue; + const runDeepChecks = shouldRunDeepMilestoneDoctorChecks( + milestoneId, + state, + options?.scope, + ); + if (!runDeepChecks) continue; // Validate milestone title for delimiter characters that break state documents. const milestoneTitleIssue = validateTitle(milestone.title); if (milestoneTitleIssue) { @@ -1496,6 +1519,7 @@ export async function runSFDoctor(basePath, options) { continue; } if (!knownSliceIds.has(entry)) { + if (isLegacySlugDuplicateSliceDir(slicesDir, entry)) continue; issues.push({ severity: "warning", code: "orphaned_slice_directory", diff --git a/src/resources/extensions/sf/tests/doctor-providers.test.mjs b/src/resources/extensions/sf/tests/doctor-providers.test.mjs new file mode 100644 index 000000000..b55f413dc --- /dev/null +++ b/src/resources/extensions/sf/tests/doctor-providers.test.mjs @@ -0,0 +1,86 @@ +import assert from "node:assert/strict"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, test } from "vitest"; +import { runProviderChecks } from "../doctor-providers.js"; + +const originalCwd = process.cwd(); +const originalEnv = { ...process.env }; +const tmpDirs = []; + +afterEach(() => { + process.chdir(originalCwd); + process.env = { ...originalEnv }; + while (tmpDirs.length > 0) { + rmSync(tmpDirs.pop(), { recursive: true, force: true }); + } +}); + +function makePreferencesProject(globalPreferences) { + const root = mkdtempSync(join(tmpdir(), "sf-doctor-providers-")); + tmpDirs.push(root); + const home = join(root, "home"); + const project = join(root, "project"); + mkdirSync(home, { recursive: true }); + mkdirSync(join(project, ".sf"), { recursive: true }); + writeFileSync(join(home, "preferences.md"), globalPreferences, "utf-8"); + writeFileSync( + join(project, ".sf", "PREFERENCES.md"), + "---\nversion: 1\nmodels: {}\n---\n", + "utf-8", + ); + process.env.SF_HOME = home; + process.chdir(project); + return project; +} + +describe("doctor provider checks", () => { + test("runProviderChecks_when_any_configured_llm_route_is_usable_does_not_require_every_preferred_provider", () => { + makePreferencesProject( + [ + "---", + "version: 1", + "models:", + " execution: kimi-coding/kimi-k2.6", + " reassess: mistral/devstral-latest", + "---", + "", + ].join("\n"), + ); + process.env.KIMI_API_KEY = "test-kimi-key"; + delete process.env.MISTRAL_API_KEY; + + const results = runProviderChecks(); + const mistral = results.find((result) => result.name === "mistral"); + + assert.equal(mistral?.status, "unconfigured"); + assert.equal(mistral?.required, false); + }); + + test("runProviderChecks_when_remote_questions_auto_resolve_does_not_require_channel_token", () => { + makePreferencesProject( + [ + "---", + "version: 1", + "models:", + " execution: kimi-coding/kimi-k2.6", + "remote_questions:", + " channel: telegram", + " channel_id: 562797207", + " auto_resolve_on_timeout: true", + " auto_resolve_strategy: recommended-option", + "---", + "", + ].join("\n"), + ); + process.env.KIMI_API_KEY = "test-kimi-key"; + delete process.env.TELEGRAM_BOT_TOKEN; + + const results = runProviderChecks(); + const telegram = results.find((result) => result.name === "telegram_bot"); + + assert.equal(telegram?.status, "unconfigured"); + assert.equal(telegram?.required, false); + }); +}); diff --git a/src/resources/extensions/sf/tests/doctor-sf-form-lint.test.mjs b/src/resources/extensions/sf/tests/doctor-sf-form-lint.test.mjs index e889d4572..725541057 100644 --- a/src/resources/extensions/sf/tests/doctor-sf-form-lint.test.mjs +++ b/src/resources/extensions/sf/tests/doctor-sf-form-lint.test.mjs @@ -29,6 +29,36 @@ function makeProject() { } describe("doctor .sf form lint", () => { + test("runSFDoctor_scoped_milestone_reports_requested_historical_delimiter_noise", async () => { + const project = makeProject(); + mkdirSync(join(project, ".sf", "milestones", "M001"), { + recursive: true, + }); + writeFileSync( + join(project, ".sf", "milestones", "M001", "M001-ROADMAP.md"), + [ + "# M001: Done milestone", + "", + "## Slice Overview", + "| ID | Slice | Risk | Depends | Done | After this |", + "|----|-------|------|---------|------|------------|", + "| S01 | Old — noisy | low | — | ✅ | done |", + "", + ].join("\n"), + "utf-8", + ); + + const report = await runSFDoctor(project, { scope: "M001" }); + + assert.equal( + report.issues.some( + (issue) => + issue.code === "delimiter_in_title" && issue.unitId === "M001/S01", + ), + true, + ); + }); + test("runSFDoctor_reports_invalid_json_yaml_jsonl_and_markdown_frontmatter", async () => { const project = makeProject(); writeFileSync(join(project, ".sf", "bad.json"), "{", "utf-8");