fix(doctor): parse flow-style skill triggers (#3887) (#3888)

Wave-assembled from PR #3888 by @RerankerGuo. Conflict resolution: KEY_FILES.md
check-resolvable/skill-frontmatter/skill-trigger-index entries taken as the
PR's current-state prose; the PR's stale pre-fold 'doctor.ts extension' entry
dropped (master already folded it into the main doctor entry).

Co-Authored-By: RerankerGuo <121015044+RerankerGuo@users.noreply.github.com>

docs/architecture/KEY_FILES.md

@@ -149,9 +149,9 @@ 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 (13 `PER_TASK_KEYS`, now including provider-neutral `models.contextual_synopsis` with legacy-key/env attribution), 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/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-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`, including block and wrapped flow-sequence arrays. 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).
+- `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`). `extractTriggers()` delegates to the shared SKILL.md parser, so MECE gap detection and the trigger index agree on block lists, single-line flow sequences, wrapped flow sequences, and CRLF input. `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` (resolver shapes plus trigger array syntax regressions) + `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).
 - `src/core/repo-root.ts` — Shared `findRepoRoot(startDir?)`: walks up from `startDir` (default `process.cwd()`) looking for `skills/RESOLVER.md`. Zero-dependency, imported by `doctor.ts` and `check-resolvable.ts`; parameterized `startDir` makes tests hermetic. Read-path / write-path split: `autoDetectSkillsDir` (shared, read+write-safe) has tier-0 `$GBRAIN_SKILLS_DIR` operator override ahead of the 4-tier chain. `autoDetectSkillsDirReadOnly` wraps it with a tier-5 install-path fallback that walks up from `fileURLToPath(import.meta.url)` and gates on `isGbrainRepoRoot` so unrelated repos can't false-positive. Read-path callers (`doctor`, `check-resolvable`, `routing-eval`) use the read-only variant; write-path callers (`skillpack install`, `skillify scaffold`, `post-install-advisory`) stay on the shared function so install-from-`~` can't retarget the bundled gbrain `skills/` instead of the user's workspace. `SkillsDirSource` variants `'env_explicit'`, `'install_path'`; `AUTO_DETECT_HINT_READ_ONLY` documents the extra tier. The `--fix` safety gate in `doctor.ts` + `check-resolvable.ts` refuses auto-repair when `detected.source === 'install_path'`.
 - `src/core/skills-integrity.ts` — Tamper-evidence manifest for the bundled `skills/` tree (#159); NOT a signature system. Pure functions over `node:crypto` sha256: `computeSkillsManifest(dir)` (recursive, sorted '/'-relative paths, excludes the manifest itself, skips symlinks), `renderSkillsManifest(dir)` (2-space JSON + trailing newline, deterministic), `verifySkillsManifest(dir, manifest)` → `{modified, missing, extra}`. Committed manifest lives at `skills/skills.lock.json` (`SKILLS_MANIFEST_FILENAME`); regenerate via `bun run scripts/generate-skills-manifest.ts`. Consumers: the warn-only `skills_manifest_integrity` doctor check in `src/commands/doctor.ts` (ok/skip when no manifest is present — user workspaces and compiled-binary installs are not drift) and the CI freshness guard `scripts/check-skills-manifest-fresh.sh` (`bun run check:skills-manifest`, in `bun run verify`). Pinned by `test/skills-integrity.test.ts`.
 - `src/commands/check-resolvable.ts` — Standalone CLI wrapper over `checkResolvable()`. Exports `parseFlags`, `resolveSkillsDir`, `DEFERRED`, `runCheckResolvable`. Exit rule: **1 on any issue (warnings OR errors)**, stricter than doctor's `ok` flag. Stable JSON envelope `{ok, skillsDir, report, autoFix, deferred, error, message}` — same shape on success and error. `--fix` runs `autoFixDryViolations` BEFORE `checkResolvable` (same ordering as doctor). `scripts/skillify-check.ts` subprocess-calls `gbrain check-resolvable --json` (cached per process) and fails loud on binary-missing. AGENTS.md workspaces resolve natively (see `src/core/resolver-filenames.ts`). `DEFERRED[]` is empty. Resolver lookup is the multi-file merge in `src/core/check-resolvable.ts` — entries collected from every `RESOLVER.md`/`AGENTS.md` across the skills dir AND its parent, deduped by `skillPath` (first occurrence wins). Uses `autoDetectSkillsDirReadOnly` so `cd ~ && gbrain check-resolvable` finds bundled skills via the install-path fallback; `--fix` carries the same install-path safety gate (refuses to write when `detected.source === 'install_path'`).
@@ -169,7 +169,7 @@ per-release `**vX.Y.Z:**` narration — CI enforces this
 - `src/core/skill-manifest.ts` — parser for `skill-manifest.json` records. Used by skillpack installer to detect drift between the shipped bundle and the user's local edits, so updates merge instead of overwriting.
 - `src/commands/routing-eval.ts` + `src/core/routing-eval.ts` — `gbrain routing-eval` catches user phrasings that route to the wrong skill. Reads `skills/<name>/routing-eval.jsonl` fixtures (`{intent, expected_skill, ambiguous_with?}`). Structural layer runs in `check-resolvable` by default (zero API cost). `--llm` is a placeholder for a future LLM tie-break layer; today it emits a stderr notice and runs structural only. Uses `autoDetectSkillsDirReadOnly` and the same multi-file resolver merge as `check-resolvable`, so on OpenClaw layouts (`skills/RESOLVER.md` + `../AGENTS.md`) all three commands see the same trigger index. RESOLVER.md rows carry the full frontmatter `triggers:` arrays so the structural matcher sees realistic phrasings; ambiguous-fixture annotations cover deliberate skill chains like `enrich → article-enrichment`.
 - `src/core/filing-audit.ts` + `skills/_brain-filing-rules.json` — Check 6 of `check-resolvable`. Parses `writes_pages:` / `writes_to:` frontmatter on skills and audits their filing claims against the filing-rules JSON (error severity). Internal `parseFrontmatter` is a thin wrapper over the shared `src/core/skill-frontmatter.ts` parser so both filing-audit and skill-brain-first read the same shape (`tools?`, `triggers?`, `brain_first?: 'exempt'`, typed `brain_first_typo`) from one source of truth.
-- `src/core/skill-frontmatter.ts` — shared content-based SKILL.md frontmatter parser. Recognizes the `brain_first: 'exempt'` declarative opt-out and surfaces near-miss declarations (`brain-first`, `BrainFirst`, quoted values, unknown values) as a typed `brain_first_typo` field so doctor can emit a paste-ready hint rather than fail silently. Single canonical form: snake_case `brain_first: exempt`, lowercase, unquoted.
+- `src/core/skill-frontmatter.ts` — shared content-based SKILL.md frontmatter parser. Array fields (`writes_to`, `tools`, `triggers`) use `js-yaml` with the failsafe schema, accepting block lists and single-line or wrapped flow sequences while preserving the tolerant legacy regex fallback for partially malformed YAML; CRLF is normalized before fence parsing. Recognizes the `brain_first: 'exempt'` declarative opt-out and surfaces near-miss declarations (`brain-first`, `BrainFirst`, quoted values, unknown values) as a typed `brain_first_typo` field so doctor can emit a paste-ready hint rather than fail silently. Single canonical form: snake_case `brain_first: exempt`, lowercase, unquoted.
 - `src/core/skill-brain-first.ts` — pure analyzer. `analyzeSkillBrainFirst(skillPath, content): SkillBrainFirstResult` walks the compliance ladder for every SKILL.md: (1) absent external-lookup pattern → `no_external`; (2) `brain_first: exempt` frontmatter → `exempt_frontmatter`; (3) canonical `> **Convention:** see [conventions/brain-first.md](...)` callout → `compliant_callout`; (4) explicit `## Phase 1: Brain` heading → `compliant_phase`; (5) first `gbrain search`/`query`/`get_page` reference precedes first external pattern in the BODY (frontmatter stripped) → `compliant_position`; (6) else `missing_brain_first` warn. External pattern set: word-boundary regex over `web_search`, `web_fetch`, `exa`, `perplexity`, `happenstance`, `crustdata`, `captain_api`, `firecrawl`. Position scan is BODY-ONLY so a `tools: [web_search]` frontmatter declaration doesn't false-flag the skill. The 40-name `FORMERLY_HARDCODED_EXEMPT` list is preserved so doctor can emit a "this used to be auto-exempt, declare `brain_first: exempt` if still appropriate" hint. Consumed by 3 surfaces: doctor check, skillify-check item 12, dry-fix MISSING_RULE_PATTERNS.
 - `src/core/skill-fix-gates.ts` — shared safety primitives extracted from `dry-fix.ts`. `getWorkingTreeStatus(file)` 3-state (`'clean' | 'dirty' | 'not_a_repo'`); `isInsideCodeFence(content, offset)`; `findAfterH1Paragraph(content)` (canonical insertion offset for the auto-inserted Convention callout). Both REPLACE expanders (DRY violations) and the INSERT expander (MISSING_RULE_PATTERNS) consume from here so the install-path refusal and dirty-tree gates apply uniformly. `src/core/dry-fix.ts` re-exports for back-compat with existing call sites.
 - `src/core/audit-skill-brain-first.ts` — snapshot+diff JSONL audit at `~/.gbrain/audit/skill-brain-first-YYYY-Www.jsonl` (ISO-week rotated, honors `GBRAIN_AUDIT_DIR` via shared `resolveAuditDir()`). `recordBrainFirstRun(results)` reads the previous snapshot at `~/.gbrain/audit/skill-brain-first-snapshot.json`, diffs against current results, writes transition events (`detected | resolved | fixed`) one line per change, then atomically overwrites the snapshot via `.tmp + rename`. **Transition-only writes** — a stable brain produces 0 audit lines per doctor run. `readRecentBrainFirstEvents(days)` is the readback path for the future `skill_brain_first_trend` doctor check. Snapshot file is last-writer-wins under concurrent doctor runs; subsequent runs reconcile.