mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-28 14:59:47 +00:00
fix(doctor): honor explicit subagent model config (#3408)
This commit is contained in:
@@ -2,9 +2,10 @@
|
||||
|
||||
## community fix-wave follow-ups (filed v0.42.60.0)
|
||||
|
||||
- [ ] **P2 — cherry-pick #2112's uncovered doctor.ts hunk.** Fix-wave A (#2820) superseded
|
||||
- [x] **P2 — cherry-pick #2112's uncovered doctor.ts hunk.** Fix-wave A (#2820) superseded
|
||||
most of #2112 but not its `checkSubagentCapability` fix (check explicit `models.subagent`
|
||||
before `models.tier.subagent`). Refile or cherry-pick; the rest of that PR is covered.
|
||||
before `models.tier.subagent`). Implemented: `checkSubagentCapability` now resolves
|
||||
`models.subagent` before tier/default fallbacks and has regression coverage.
|
||||
|
||||
## v0.42.59.0 follow-ups (five-fix rollup #2735–#2739)
|
||||
|
||||
|
||||
@@ -141,7 +141,7 @@ per-release `**vX.Y.Z:**` narration — CI enforces this
|
||||
- `src/commands/models.ts` — `gbrain models [--json]` read-only routing dashboard: prints tier defaults (`utility`/`reasoning`/`deep`/`subagent`), the resolved value for each (re-walking the resolution chain), every per-task override (11 `PER_TASK_KEYS`: `models.dream.synthesize`, `models.dream.patterns`, `models.drift`, `models.auto_think`, `models.think`, `models.subagent`, `facts.extraction_model`, `models.eval.longmemeval`, `models.expansion`, `models.chat`, `models.dream.synthesize_verdict`), the alias map, and a source-of-truth column (`default` / `config: <key>` / `env: <VAR>`). `gbrain models doctor [--skip=<provider>] [--json]` fires a 1-token `gateway.chat()` probe against each configured chat + expansion model and classifies failures into `{model_not_found, auth, rate_limit, network, unknown}`. Wired into `cli.ts` dispatch + `CLI_ONLY` set. A zero-token `embedding_config` probe runs FIRST, before any chat/expansion probes spend money: `probeEmbeddingConfig()` reads `getEmbeddingModel()` + `getEmbeddingDimensions()` and (for Voyage flexible-dim models) checks `isValidVoyageOutputDim(dims)` against `VOYAGE_VALID_OUTPUT_DIMS`. `ProbeStatus` variant `'config'` + optional `fix?: string` on `ProbeResult` surface a paste-ready `gbrain config set ...` line in human + JSON output; touchpoint label `'embedding_config'` joins `'chat'` and `'expansion'`.
|
||||
- `src/core/init-embed-check.ts` — embedding-key validation at `gbrain init`. `runInitEmbedCheck(opts)` runs a config-only `diagnoseEmbedding` (catches a missing key for ANY provider) plus a best-effort `liveTestEmbed` (1-token `gateway.embed(['probe'], {inputType:'query', abortSignal})`, 5s `AbortController` timeout, never throws — catches an invalid/expired key). Loud warning to stderr; init still exits 0 (`--no-embedding` is the deferred-setup escape; `--skip-embed-check` / `GBRAIN_INIT_SKIP_EMBED_CHECK=1` skip the check). Builds the effective env (`process.env` + file-plane `openai/anthropic/zeroentropy_api_key` from `loadConfigFileOnly()` + `opts.apiKey`) and configures the gateway via `buildGatewayConfig` before diagnose/probe, so the check sees the same keys AND provider base URLs runtime will (no false "missing key" for config.json-keyed users; the probe hits the right endpoint). Init-specific warning text names `--no-embedding` / `--skip-embed-check`, not the sync-flavored `--no-embed`. Wired into `initPGLite` + `initPostgres` in `src/commands/init.ts`, with the result added to the `--json` envelope as `embedding_check {ok, reason?, live_ok?}`. Pinned by `test/init-embed-check.test.ts` (hermetic via the gateway embed-transport seam + `withEnv`).
|
||||
- `src/core/ai/build-gateway-config.ts` — `buildGatewayConfig(c: GBrainConfig): AIGatewayConfig`, extracted from `src/cli.ts` (which re-exports it for back-compat). Lets core modules (`init-embed-check.ts`) reuse it without importing the CLI entrypoint. Single owner of folding file-plane API keys (openai/anthropic/zeroentropy) into the gateway env and threading local-server `*_BASE_URL` env vars into base_urls. `process.env` wins EXCEPT empty-string / undefined values are dropped before the merge, so an injected empty `ANTHROPIC_API_KEY=''` (Claude Code neuters subprocess LLM calls this way) can't clobber a valid config-plane key; `'0'` / `'false'` are preserved. Pinned by `test/ai/build-gateway-config.test.ts`.
|
||||
- `src/commands/doctor.ts` extension — `subagent_provider` check (layer 3 of 3). Warns when `models.tier.subagent` is explicitly set non-Anthropic (message names the bad value + paste-ready fix `gbrain config set models.tier.subagent anthropic:claude-sonnet-4-6`); also warns when `models.default` would sneak `subagent` into a non-Anthropic provider via tier inheritance. OK when subagent tier resolves to Anthropic. Tests in `test/doctor.test.ts`.
|
||||
- `src/commands/doctor.ts` extension — `subagent_provider` check (layer 3 of 3). Resolves subagent model config in runtime order (`models.subagent` > `models.default` > `models.tier.subagent` > built-in default) and warns when the selected model lacks native tool-loop capability (message names the bad value + paste-ready fix `gbrain config set models.tier.subagent anthropic:claude-sonnet-4-6`); also warns when `models.default` would sneak `subagent` into a non-Anthropic provider via tier inheritance. OK when subagent tier resolves to Anthropic. Tests in `test/doctor.test.ts`.
|
||||
- `src/core/skill-trigger-index.ts` — Shared loader that unions per-skill SKILL.md frontmatter `triggers:` with curated RESOLVER.md / AGENTS.md rows from `skillsDir` AND the parent dir (preserves the OpenClaw workspace-root layout). UNION semantics: explicit RESOLVER.md rows ADD to frontmatter triggers (don't replace). Dedup keyed on `(skillPath, trigger.trim().toLowerCase())`. Three consumers fold through this primitive — `checkResolvable`, `runRoutingEvalCli`, `mounts-cache.composeResolvers` — so fixing frontmatter reaches all of them. Exports `loadSkillTriggerIndex(skillsDir): SkillTriggerEntry[]`, `entriesToResolverContent(entries): string` (synthesizes a markdown-table resolver string for `runRoutingEval`'s string-content API), `findPrimaryResolverPath(skillsDir): string | null`, the `FRONTMATTER_SECTION` constant, and `_resetWarnedSkillsForTests`. Skip rules: non-directory entries, `_*`/`.*` prefixes, `conventions/`+`migrations/` subdirs, skills with no `SKILL.md` (deprecated `install/` graceful-skipped), no `triggers:` array, or malformed YAML (warn-once + skip). Reuses `parseSkillFrontmatter` from `src/core/skill-frontmatter.ts` (regex-based, not full YAML). Pinned by `test/skill-trigger-index.test.ts` (18 hermetic cases). CI gate `bun run check:resolver` (= `bun src/cli.ts check-resolvable --strict --skills-dir skills/`) wired into `bun run verify`.
|
||||
- `src/core/skill-catalog.ts` — host-repo skill catalog backing the MCP `list_skills` / `get_skill` ops. Lets a thin MCP client (Codex desktop, Claude Code, Claude Cowork, Perplexity) DISCOVER + FOLLOW the agent repo's fat-markdown skills over `gbrain serve` — a skill is prose, so "using" one = fetching its body then calling the gbrain MCP tools the server already exposes. Read-scope, NOT localOnly (defensible only via the full mitigation stack): (1) **publish gate** — `assertPublishEnabled(ctx, publishSkills)`; remote callers require `mcp.publish_skills === true`, default-OFF so an upgrade never silently grants existing read tokens host-skill read; local callers (`ctx.remote === false`) always pass. (2) **path confinement** — `assertSkillNameShape` rejects separators/`..`/null/space before any FS access; the client `name` is a manifest LOOKUP KEY (via `loadOrDeriveManifest`), never a raw path segment; `confineManifestPath` does realpath + relative-containment + `SKILL.md`-regular-file check on EVERY entry (defeats poisoned manifest.json `path`, symlink/`..` escape). (3) **frontmatter allowlist** — `GetSkillResult.frontmatter` projects a safe subset; private `writes_to` + `sources` dropped. (4) **prose-only + 256KB cap** (`MAX_SKILL_MD_BYTES`, env `GBRAIN_MAX_SKILL_MD_BYTES`), size-checked twice (statSync + UTF-8 byte length). (5) **no install_path serve for remote** — remote callers use `autoDetectSkillsDir` (no install-path tier) so a hosted gbrain with no agent repo returns `storage_error`; local callers use `autoDetectSkillsDirReadOnly`. (6) MCP rate-limiter caps call rate. Config reads honor BOTH planes: `readMcpPublishSkills` / `readMcpSkillsDir` prefer the DB plane (`engine.getConfig`) over the file plane (`ctx.config.mcp`). Tool-honesty: `crossReferenceTools(declared, ctx)` splits a skill's declared `tools:` into `usable_tools` vs `unavailable_tools`; `buildSkillCatalog`'s `instructions` envelope (`SKILL_CATALOG_INSTRUCTIONS`) carries the "these are prose, follow-then-call-tools" protocol. Skills are host-filesystem repo-global — `sourceScopeOpts(ctx)` / `ctx.brainId` deliberately do NOT apply. `buildSkillCatalog` is resilient (one malformed/escaping skill is skipped, never throws). Config keys in `src/core/config.ts`: `GBrainConfig.mcp?: { publish_skills?, skills_dir? }` + `KNOWN_CONFIG_KEYS` entries `mcp.publish_skills`/`mcp.publish_skills_prompted`/`mcp.skills_dir` + `mcp.` prefix in `KNOWN_CONFIG_KEY_PREFIXES`. `src/commands/init.ts` writes `config.mcp = { publish_skills: true, ... }` for new installs (existing config wins on re-init). `src/commands/upgrade.ts:runPostUpgrade` adds a one-time consent prompt (gated by `mcp.publish_skills_prompted`; existing installs stay OFF until owner opts in). Two ops register in `src/core/operations.ts` (`list_skills` with optional `section` filter + `cliHints:{name:'skills'}`; `get_skill` taking `name` + `cliHints:{name:'skill', positional:['name']}`) and dynamically import this module to avoid the import cycle (skill-catalog statically imports the `operations` array). Descriptions in `src/core/operations-descriptions.ts` (`LIST_SKILLS_DESCRIPTION`, `GET_SKILL_DESCRIPTION`, `SKILL_CATALOG_INSTRUCTIONS`, `SKILL_CLIENT_GUIDANCE`), pinned by `test/operations-descriptions.test.ts`. CLI: `gbrain skills` / `gbrain skill <name>`. Pinned by `test/skill-catalog.test.ts`, `test/skill-catalog-security.test.ts` (path-confinement / poisoned-manifest / symlink-escape), `test/skill-catalog-transports.test.ts` (publish-gate + remote-vs-local) over `test/fixtures/skill-catalog/`.
|
||||
- `src/core/check-resolvable.ts` — Resolver validation: reachability, MECE overlap, DRY checks, structured fix objects. `CROSS_CUTTING_PATTERNS.conventions` is an array (notability gate accepts `conventions/quality.md` and `_brain-filing-rules.md`). `extractDelegationTargets()` parses `> **Convention:**`, `> **Filing rule:**`, and inline backtick references. DRY suppression is proximity-based via `DRY_PROXIMITY_LINES = 40`. `parseResolverEntries` accepts BOTH the markdown table AND a compact list format (`- **skill-name**: trigger1 | trigger2 | trigger3` or `- skill-name: trigger1 | trigger2`); shapes can mix in one file, folded by the multi-resolver merge. Skill name MUST be kebab-lowercase (regex `[a-z][a-z0-9-]+`) so prose bullets like `- **Note**:`/`- **Convention**:`/`- **TODO**:` don't false-match as skill rows. `skillPath` is ALWAYS derived as `skills/<name>/SKILL.md`: an optional `→ \`skills/path\`` (or ASCII `->`) suffix is stripped from the trigger but NOT honored as the path — two consumers (`routing-eval.ts:skillSlugFromPath`, the manifest lookup) assume the convention; use the table format for non-conventional paths. Multi-trigger rows fan out to one entry per trigger sharing the same `skillPath`; `checkResolvable` dedupes so the reachability count counts each skill once. Pinned by `test/check-resolvable.test.ts` (11 cases: bold+plain forms, Unicode+ASCII suffix strip, ellipsis filter, empty pipe segments, mixed shapes, prose-bullet rejection) + `test/check-resolvable-openclaw-compact.test.ts` (8 cases over `test/fixtures/openclaw-compact-resolver/` and `test/fixtures/openclaw-mixed-merge/`). Tutorial: `docs/guides/scaling-skills.md` (three-tier scaling: ~300-skill agent to ~4K tokens/turn from ~25K).
|
||||
|
||||
+18
-6
@@ -879,8 +879,8 @@ export async function doctorReportRemote(engine: BrainEngine): Promise<DoctorRep
|
||||
checks.push(await checkEmbeddingEnvOverride(engine));
|
||||
|
||||
// v0.31.12 subagent runtime enforcement (Layer 3 of 3 — Codex F13).
|
||||
// The subagent loop is Anthropic-only. If models.tier.subagent or
|
||||
// models.default is explicitly set to a non-Anthropic provider, warn here
|
||||
// The subagent loop requires native tool-calling. If models.subagent,
|
||||
// models.tier.subagent, or models.default resolves to a limited provider, warn here
|
||||
// so the user sees it at the next `gbrain doctor` run instead of at the
|
||||
// next subagent job submission. (Layers 1+2 also enforce — this is the
|
||||
// surfacing layer.)
|
||||
@@ -3054,6 +3054,7 @@ async function checkEmbeddingEnvOverride(engine: BrainEngine): Promise<Check> {
|
||||
export async function checkSubagentCapability(engine: BrainEngine): Promise<Check> {
|
||||
try {
|
||||
const { classifyCapabilities } = await import('../core/ai/capabilities.ts');
|
||||
const modelsSubagent = await engine.getConfig('models.subagent');
|
||||
const tierSubagent = await engine.getConfig('models.tier.subagent');
|
||||
const modelsDefault = await engine.getConfig('models.default');
|
||||
|
||||
@@ -3094,12 +3095,23 @@ export async function checkSubagentCapability(engine: BrainEngine): Promise<Chec
|
||||
return null;
|
||||
};
|
||||
|
||||
if (tierSubagent) {
|
||||
const issue = explain(tierSubagent, 'models.tier.subagent');
|
||||
let resolvedSource: string | null = null;
|
||||
let resolvedModel: string | null = null;
|
||||
if (modelsSubagent) {
|
||||
resolvedSource = 'models.subagent';
|
||||
resolvedModel = modelsSubagent;
|
||||
const issue = explain(modelsSubagent, resolvedSource);
|
||||
if (issue) return issue;
|
||||
} else if (modelsDefault) {
|
||||
resolvedSource = 'models.default';
|
||||
resolvedModel = modelsDefault;
|
||||
const issue = explain(modelsDefault, 'models.default');
|
||||
if (issue) return issue;
|
||||
} else if (tierSubagent) {
|
||||
resolvedSource = 'models.tier.subagent';
|
||||
resolvedModel = tierSubagent;
|
||||
const issue = explain(tierSubagent, resolvedSource);
|
||||
if (issue) return issue;
|
||||
}
|
||||
// v0.37 (T10 / D7) + v0.38 (D7 capability rename): warn when the configured
|
||||
// chat_model is non-Anthropic AND ANTHROPIC_API_KEY isn't set. With
|
||||
@@ -3132,8 +3144,8 @@ export async function checkSubagentCapability(engine: BrainEngine): Promise<Chec
|
||||
return {
|
||||
name: 'subagent_capability',
|
||||
status: 'ok',
|
||||
message: tierSubagent
|
||||
? `Subagent tier resolves to "${tierSubagent}" with full tool-loop capability`
|
||||
message: resolvedModel && resolvedSource
|
||||
? `Subagent model resolves via ${resolvedSource} to "${resolvedModel}" with full tool-loop capability`
|
||||
: `Subagent tier resolves to default (claude-sonnet-4-6) — full tool-loop capability`,
|
||||
};
|
||||
} catch (e) {
|
||||
|
||||
@@ -52,6 +52,53 @@ describe('doctor command', () => {
|
||||
expect(check.issues![0].action).toContain('trigger');
|
||||
});
|
||||
|
||||
test('subagent_capability checks explicit models.subagent before tier/default fallbacks', async () => {
|
||||
const { checkSubagentCapability } = await import('../src/commands/doctor.ts');
|
||||
const config = new Map<string, string | null>([
|
||||
['models.subagent', 'openai:gpt-5.2'],
|
||||
['models.tier.subagent', 'anthropic:claude-sonnet-4-6'],
|
||||
['models.default', 'anthropic:claude-sonnet-4-6'],
|
||||
]);
|
||||
const check = await checkSubagentCapability({
|
||||
async getConfig(key: string): Promise<string | null> {
|
||||
return config.get(key) ?? null;
|
||||
},
|
||||
} as any);
|
||||
expect(check.status).toBe('warn');
|
||||
expect(check.message).toContain('models.subagent is "openai:gpt-5.2"');
|
||||
expect(check.message).toContain('prompt caching');
|
||||
});
|
||||
|
||||
test('subagent_capability reports explicit models.subagent on the ok path', async () => {
|
||||
const { checkSubagentCapability } = await import('../src/commands/doctor.ts');
|
||||
const config = new Map<string, string | null>([
|
||||
['models.subagent', 'anthropic:claude-opus-4-7'],
|
||||
['models.tier.subagent', 'anthropic:claude-haiku-4-5'],
|
||||
]);
|
||||
const check = await checkSubagentCapability({
|
||||
async getConfig(key: string): Promise<string | null> {
|
||||
return config.get(key) ?? null;
|
||||
},
|
||||
} as any);
|
||||
expect(check.status).toBe('ok');
|
||||
expect(check.message).toContain('Subagent model resolves via models.subagent to "anthropic:claude-opus-4-7"');
|
||||
});
|
||||
|
||||
test('subagent_capability checks models.default before tier fallback', async () => {
|
||||
const { checkSubagentCapability } = await import('../src/commands/doctor.ts');
|
||||
const config = new Map<string, string | null>([
|
||||
['models.tier.subagent', 'anthropic:claude-sonnet-4-6'],
|
||||
['models.default', 'openai:gpt-5.2'],
|
||||
]);
|
||||
const check = await checkSubagentCapability({
|
||||
async getConfig(key: string): Promise<string | null> {
|
||||
return config.get(key) ?? null;
|
||||
},
|
||||
} as any);
|
||||
expect(check.status).toBe('warn');
|
||||
expect(check.message).toContain('models.default is "openai:gpt-5.2"');
|
||||
});
|
||||
|
||||
test('reranker_health warns on repeated unknown rerank failures', async () => {
|
||||
const { checkRerankerHealth } = await import('../src/commands/doctor.ts');
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gbrain-rerank-doctor-'));
|
||||
|
||||
Reference in New Issue
Block a user