From d77a514684d852233306af0dd4ddd445deee68d3 Mon Sep 17 00:00:00 2001 From: garrytan-agents Date: Sun, 10 May 2026 04:09:37 +0000 Subject: [PATCH] fix: merge resolver entries from all files (RESOLVER.md + AGENTS.md) OpenClaw deployments typically have AGENTS.md at the workspace root as the real skill dispatcher (200+ entries), while gbrain skillpacks install a thin skills/RESOLVER.md (~40 entries). The previous first-match-wins policy meant check-resolvable only saw the thin RESOLVER.md, reporting 187 skills as 'unreachable' when they were fully routed in AGENTS.md. Now: check-resolvable collects entries from ALL resolver files across both the skills directory and its parent. Entries are deduped by skillPath (first occurrence wins). The combined content is also passed to the routing-eval (Check 5) so routing fixtures see the full trigger index. New function findAllResolverFiles() in resolver-filenames.ts returns all matching files instead of just the first. findResolverFile() is unchanged (backward-compatible for callers that need a single path). Before: 37/224 reachable (our deployment) After: 200/224 reachable (remaining 24 are genuine gaps) Tests: 8 new (findAllResolverFiles + checkResolvable merge behavior) --- src/core/check-resolvable.ts | 40 ++++++++-- src/core/resolver-filenames.ts | 16 ++++ test/resolver-merge.test.ts | 142 +++++++++++++++++++++++++++++++++ 3 files changed, 190 insertions(+), 8 deletions(-) create mode 100644 test/resolver-merge.test.ts diff --git a/src/core/check-resolvable.ts b/src/core/check-resolvable.ts index 1757d9e79..bb1249d1c 100644 --- a/src/core/check-resolvable.ts +++ b/src/core/check-resolvable.ts @@ -12,7 +12,7 @@ import { readFileSync, existsSync, readdirSync } from 'fs'; import { join, relative } from 'path'; -import { findResolverFile, RESOLVER_FILENAMES_LABEL } from './resolver-filenames.ts'; +import { findResolverFile, findAllResolverFiles, RESOLVER_FILENAMES_LABEL } from './resolver-filenames.ts'; import { loadOrDeriveManifest } from './skill-manifest.ts'; import { indexResolverTriggers, @@ -251,10 +251,20 @@ export function checkResolvable(skillsDir: string): ResolvableReport { // Load inputs // Accept RESOLVER.md or AGENTS.md (W1). Also check one level up: the // reference OpenClaw deployment layout places AGENTS.md at the - // workspace root, with skills/ below. We try skills dir first - // (gbrain-native), then its parent (OpenClaw-native). - const resolverPath = - findResolverFile(skillsDir) ?? findResolverFile(join(skillsDir, '..')); + // workspace root, with skills/ below. + // + // Merge strategy (D-CX-14): collect entries from ALL resolver files + // across both directories (skills dir + parent). This handles the + // common OpenClaw layout where a skillpack installs a thin + // skills/RESOLVER.md while the real dispatcher lives in ../AGENTS.md. + // Entries are deduped by skillPath (first occurrence wins). + const allResolverPaths = [ + ...findAllResolverFiles(skillsDir), + ...findAllResolverFiles(join(skillsDir, '..')), + ]; + + // Primary resolver: first found (for error messages and --fix targets) + const resolverPath = allResolverPaths[0] ?? null; if (!resolverPath) { const suggested = join(skillsDir, 'RESOLVER.md'); const missingIssue: ResolvableIssue = { @@ -274,8 +284,22 @@ export function checkResolvable(skillsDir: string): ResolvableReport { }; } - const resolverContent = readFileSync(resolverPath, 'utf-8'); - const entries = parseResolverEntries(resolverContent); + // Merge entries from all resolver files, dedup by skillPath. + // Also build a combined resolverContent for routing-eval (Check 5). + const seenSkillPaths = new Set(); + const entries: ResolverEntry[] = []; + const resolverContentParts: string[] = []; + for (const rPath of allResolverPaths) { + const content = readFileSync(rPath, 'utf-8'); + resolverContentParts.push(content); + for (const entry of parseResolverEntries(content)) { + if (!seenSkillPaths.has(entry.skillPath)) { + seenSkillPaths.add(entry.skillPath); + entries.push(entry); + } + } + } + const resolverContent = resolverContentParts.join('\n\n'); const { skills: manifest } = loadOrDeriveManifest(skillsDir); // Build lookup sets @@ -306,7 +330,7 @@ export function checkResolvable(skillsDir: string): ResolvableReport { type: 'unreachable', severity: 'error', skill: skill.name, - message: `Skill '${skill.name}' is in manifest but has no trigger row in RESOLVER.md`, + message: `Skill '${skill.name}' is in manifest but has no trigger row in ${RESOLVER_FILENAMES_LABEL}`, action: `Add a trigger row for 'skills/${skill.path}' in RESOLVER.md under ${section}`, fix: { type: 'add_trigger', diff --git a/src/core/resolver-filenames.ts b/src/core/resolver-filenames.ts index ea921bba4..e0b855a0d 100644 --- a/src/core/resolver-filenames.ts +++ b/src/core/resolver-filenames.ts @@ -32,6 +32,22 @@ export function findResolverFile(dir: string): string | null { return null; } +/** + * Return ALL existing resolver files in `dir`. + * OpenClaw deployments often have both skills/RESOLVER.md (from skillpack) + * and ../AGENTS.md (workspace root, the real dispatcher). Returning all + * files lets callers merge entries instead of silently ignoring the richer + * file. + */ +export function findAllResolverFiles(dir: string): string[] { + const results: string[] = []; + for (const name of RESOLVER_FILENAMES) { + const candidate = join(dir, name); + if (existsSync(candidate)) results.push(candidate); + } + return results; +} + /** True iff `dir` contains at least one recognized resolver file. */ export function hasResolverFile(dir: string): boolean { return findResolverFile(dir) !== null; diff --git a/test/resolver-merge.test.ts b/test/resolver-merge.test.ts new file mode 100644 index 000000000..77ddc5152 --- /dev/null +++ b/test/resolver-merge.test.ts @@ -0,0 +1,142 @@ +/** + * Verify: check-resolvable merges entries from all resolver files. + * + * The common OpenClaw layout has: + * workspace/AGENTS.md — the real dispatcher (200+ entries) + * workspace/skills/RESOLVER.md — thin skillpack-installed subset (~40 entries) + * + * Before this fix, RESOLVER.md won by first-match policy, so 160+ skills + * showed as "unreachable" even though AGENTS.md had routing for them. + * + * After: entries from both files are merged (deduped by skillPath). + */ +import { describe, it, expect, beforeAll, afterAll } from 'bun:test'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'fs'; +import { join } from 'path'; +import { tmpdir } from 'os'; +import { checkResolvable, parseResolverEntries } from '../src/core/check-resolvable.ts'; +import { findAllResolverFiles } from '../src/core/resolver-filenames.ts'; + +// --------------------------------------------------------------------------- +// findAllResolverFiles +// --------------------------------------------------------------------------- + +describe('findAllResolverFiles', () => { + let dir: string; + + beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), 'resolver-merge-')); + }); + afterAll(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + it('returns empty when no resolver files exist', () => { + expect(findAllResolverFiles(dir)).toEqual([]); + }); + + it('returns RESOLVER.md when only it exists', () => { + writeFileSync(join(dir, 'RESOLVER.md'), '# test'); + const files = findAllResolverFiles(dir); + expect(files).toHaveLength(1); + expect(files[0]).toEndWith('RESOLVER.md'); + rmSync(join(dir, 'RESOLVER.md')); + }); + + it('returns AGENTS.md when only it exists', () => { + writeFileSync(join(dir, 'AGENTS.md'), '# test'); + const files = findAllResolverFiles(dir); + expect(files).toHaveLength(1); + expect(files[0]).toEndWith('AGENTS.md'); + rmSync(join(dir, 'AGENTS.md')); + }); + + it('returns both when both exist (RESOLVER.md first)', () => { + writeFileSync(join(dir, 'RESOLVER.md'), '# resolver'); + writeFileSync(join(dir, 'AGENTS.md'), '# agents'); + const files = findAllResolverFiles(dir); + expect(files).toHaveLength(2); + expect(files[0]).toEndWith('RESOLVER.md'); + expect(files[1]).toEndWith('AGENTS.md'); + rmSync(join(dir, 'RESOLVER.md')); + rmSync(join(dir, 'AGENTS.md')); + }); +}); + +// --------------------------------------------------------------------------- +// Merge behavior in checkResolvable +// --------------------------------------------------------------------------- + +describe('checkResolvable merges resolver files', () => { + let workspace: string; + let skillsDir: string; + + beforeAll(() => { + workspace = mkdtempSync(join(tmpdir(), 'resolver-merge-e2e-')); + skillsDir = join(workspace, 'skills'); + mkdirSync(skillsDir, { recursive: true }); + + // Create 3 skills on disk + for (const name of ['alpha', 'beta', 'gamma']) { + const skillDir = join(skillsDir, name); + mkdirSync(skillDir, { recursive: true }); + writeFileSync(join(skillDir, 'SKILL.md'), `---\nname: ${name}\ntriggers:\n - "${name} trigger"\n---\n# ${name}\n`); + } + }); + + afterAll(() => { + rmSync(workspace, { recursive: true, force: true }); + }); + + it('with only skills/RESOLVER.md covering 1 of 3 → 2 unreachable', () => { + writeFileSync(join(skillsDir, 'RESOLVER.md'), `# Resolver\n\n| Trigger | Skill |\n|---|---|\n| do alpha | \`skills/alpha/SKILL.md\` |\n`); + const report = checkResolvable(skillsDir); + expect(report.summary.reachable).toBe(1); + expect(report.summary.unreachable).toBe(2); + rmSync(join(skillsDir, 'RESOLVER.md')); + }); + + it('with skills/RESOLVER.md (1 skill) + ../AGENTS.md (2 more) → all 3 reachable', () => { + // Thin RESOLVER.md in skills dir (e.g. from skillpack) + writeFileSync(join(skillsDir, 'RESOLVER.md'), + `# Resolver\n\n| Trigger | Skill |\n|---|---|\n| do alpha | \`skills/alpha/SKILL.md\` |\n`); + + // Rich AGENTS.md at workspace root (OpenClaw convention) + writeFileSync(join(workspace, 'AGENTS.md'), + `# AGENTS.md\n\n## Skills\n\n| Trigger | Skill |\n|---|---|\n| do beta | \`skills/beta/SKILL.md\` |\n| do gamma | \`skills/gamma/SKILL.md\` |\n`); + + const report = checkResolvable(skillsDir); + expect(report.summary.reachable).toBe(3); + expect(report.summary.unreachable).toBe(0); + expect(report.ok).toBe(true); + + rmSync(join(skillsDir, 'RESOLVER.md')); + rmSync(join(workspace, 'AGENTS.md')); + }); + + it('deduplicates overlapping entries (first occurrence wins)', () => { + // Both files reference alpha + writeFileSync(join(skillsDir, 'RESOLVER.md'), + `# Resolver\n\n| Trigger | Skill |\n|---|---|\n| do alpha | \`skills/alpha/SKILL.md\` |\n`); + writeFileSync(join(workspace, 'AGENTS.md'), + `# AGENTS\n\n| Trigger | Skill |\n|---|---|\n| alpha thing | \`skills/alpha/SKILL.md\` |\n| do beta | \`skills/beta/SKILL.md\` |\n| do gamma | \`skills/gamma/SKILL.md\` |\n`); + + const report = checkResolvable(skillsDir); + expect(report.summary.reachable).toBe(3); + expect(report.summary.unreachable).toBe(0); + + rmSync(join(skillsDir, 'RESOLVER.md')); + rmSync(join(workspace, 'AGENTS.md')); + }); + + it('AGENTS.md at workspace root works alone (no RESOLVER.md)', () => { + writeFileSync(join(workspace, 'AGENTS.md'), + `# AGENTS\n\n| Trigger | Skill |\n|---|---|\n| a | \`skills/alpha/SKILL.md\` |\n| b | \`skills/beta/SKILL.md\` |\n| c | \`skills/gamma/SKILL.md\` |\n`); + + const report = checkResolvable(skillsDir); + expect(report.summary.reachable).toBe(3); + expect(report.summary.unreachable).toBe(0); + + rmSync(join(workspace, 'AGENTS.md')); + }); +});