fix: keep doctor focused on actionable state

This commit is contained in:
Mikael Hugo 2026-05-05 22:57:26 +02:00
parent 969b0f3295
commit f252d1d342
4 changed files with 185 additions and 6 deletions

View file

@ -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 {

View file

@ -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",

View file

@ -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);
});
});

View file

@ -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");