mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-30 03:12:32 +00:00
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)
This commit is contained in:
garrytan-agents
@@ -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<string>();
|
||||
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',
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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'));
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user