diff --git a/packages/software-factory/.claude/CLAUDE.md b/packages/software-factory/.claude/CLAUDE.md index 2645d5f3ffb..7cb249bf7c7 100644 --- a/packages/software-factory/.claude/CLAUDE.md +++ b/packages/software-factory/.claude/CLAUDE.md @@ -13,6 +13,10 @@ pnpm factory:go --brief-url --target-realm - `--debug` — verbose logs. - `--agent openrouter` — use the opencode-OpenRouter passthrough agent (routes via the realm-server's `/_openrouter/chat/completions` proxy). +- `--skills-dir ` — replace the on-demand skill library with this + directory (bundled plugin skills are not read; workflow skills still + front-load from the factory package). Fails fast if it yields no + skills. Repeatable. ## Running tests @@ -25,11 +29,18 @@ pnpm factory:go --brief-url --target-realm Two parallel skill paths exist, one per factory run mode: - **SDK orchestrator** (`pnpm factory:go`): the loader at - `src/factory-skill-loader.ts` reads from - **`.agents/skills-orchestrator/`** first. Those skills describe the - factory-MCP-tool surface (`signal_done`, `get_card_schema`, - `run_lint`, …) that `ToolUseFactoryAgent` actually provides at - runtime. + `src/factory-skill-loader.ts` searches exactly two directories — + **`.agents/skills-orchestrator/`** (the factory workflow skills, + which describe the factory-MCP-tool surface: `signal_done`, + `get_card_schema`, `run_lint`, …) and + **`packages/boxel-cli/plugin/skills/`** (the domain-skill library: + skills bundled from `cardstack/boxel-skills` plus CLI-native skills + like `boxel-api` / `boxel-command`). Only the workflow skill is + front-loaded into the system prompt; every other skill appears as a + one-line index entry and is fetched on demand via the `read_skill` + factory tool. A small exclusion list in the loader keeps + realm-lifecycle skills (`realm-sync`, `file-ops`, …) out of the + index — the orchestrator owns workspace→realm sync. - **Interactive Claude Code** (paste the prompt from `docs/runbook.md`): Claude Code reads **`.agents/skills/`** via the `.claude/skills` symlink. Those @@ -37,20 +48,10 @@ Two parallel skill paths exist, one per factory run mode: status lifecycle. The interactive flow has no orchestrator process; the agent drives the loop directly. -Fallback dirs for both modes (skills that aren't software-factory -specific): - -1. `packages/boxel-cli/plugin/skills/` — boxel-cli Claude Code - plugin skills (`boxel-api`, `boxel-command`); same directory - the plugin distributes to end users. -2. monorepo root `.agents/skills/` — general domain skills - (`boxel-development`, `boxel-file-structure`, - `ember-best-practices`). - -The two software-factory skill sets diverged during CS-11149. They -stay separated until the SDK orchestrator is retired; at that -point the orchestrator code and `.agents/skills-orchestrator/` get deleted -together. +The two software-factory skill sets diverged during CS-11149 and stay +separated permanently — each run mode has its own tool surface, so +neither skill set can serve the other. Keep edits to workflow guidance +in sync across both when the change isn't surface-specific. ## Architectural principle diff --git a/packages/software-factory/prompts/system.md b/packages/software-factory/prompts/system.md index f4d4af9df4e..690f2fbcd28 100644 --- a/packages/software-factory/prompts/system.md +++ b/packages/software-factory/prompts/system.md @@ -24,6 +24,10 @@ Factory tools (call by name): - **`get_card_schema({ module, name })`** — fetch the live JSON Schema of a card definition. Required before writing a tracker (Project / Issue / KnowledgeArticle) or Spec card. +- **`read_skill({ skill, reference? })`** — load a skill from the skill + library (see the Skill library section below). Returns the skill text + plus its reference filenames; pass `reference` to fetch one of those + documents. - **`run_lint`** / **`run_parse`** / **`run_evaluate`** / **`run_instantiate`** / **`run_tests`** — mid-turn validators. Optional: the orchestrator runs these automatically after `signal_done`. @@ -43,14 +47,15 @@ Inspect existing state before making changes; do not guess. # Rules -- **Stay in your target realm.** The loaded skills + the issue - description contain everything you need to implement the card. Do +- **Stay in your target realm.** The loaded skills, the skill library + (via `read_skill`), and the issue description contain everything you + need to implement the card. Do NOT run `boxel file ls` / `boxel search` / `boxel read-transpiled` against any realm other than the target realm shown below — not the base realm, not the software-factory realm, not experiments{{#if enableBoxelUiDiscovery}}{{else}}, not catalog{{/if}}. Cross-realm exploration burns tokens and time without helping. If a pattern isn't - covered by your skills, write the card using your own knowledge and let - validation tell you what to fix.{{#if enableBoxelUiDiscovery}} + covered by the skill library, write the card using your own knowledge + and let validation tell you what to fix.{{#if enableBoxelUiDiscovery}} - **Exception — catalog component specs (mandatory).** The catalog realm (`@cardstack/catalog/`) publishes one Spec card per boxel-ui component, indexed and searchable. Before writing **any** UI in a @@ -110,6 +115,34 @@ tool introspects the real `CardDef` at runtime, so the shape stays correct as the tracker schema evolves. Schemas are cached per-process, so repeat calls are cheap. +{{#if skillIndex}} + +# Skill library (load on demand) + +Beyond the skills included in full below, a library of skills is +available on demand. Each entry is one skill; load it with +`read_skill({ skill: '' })` **before** doing work its description +covers — do not guess at conventions a skill documents. The result +lists the skill's reference documents; fetch one with +`read_skill({ skill: '', reference: '' })`. Load skills +selectively — only what the current issue actually needs. + +Entries marked **(suggested)** were matched to this issue's text; treat +them as a starting point, not a limit. + +These skills document the `boxel` CLI for several environments, not +just this one. In this factory session, read-only commands they mention +(`boxel search`, `boxel read-transpiled`) are fine via Bash, but +**never** run realm-mutating commands they mention — `boxel realm +push` / `sync` / `create`, `boxel file write` / `touch` / `delete`. You +write workspace files with `Write` / `Edit`; the orchestrator owns +syncing them to the realm. + +{{#each skillIndex}} +- `{{name}}`{{#if suggested}} **(suggested)**{{/if}} — {{description}} +{{/each}} +{{/if}} + {{#each skills}} # Skill: {{name}} diff --git a/packages/software-factory/scripts/smoke-tests/factory-skill-smoke.ts b/packages/software-factory/scripts/smoke-tests/factory-skill-smoke.ts index 424f05e19da..34b237c0521 100644 --- a/packages/software-factory/scripts/smoke-tests/factory-skill-smoke.ts +++ b/packages/software-factory/scripts/smoke-tests/factory-skill-smoke.ts @@ -33,7 +33,7 @@ const SAMPLE_ISSUES: { label: string; issue: IssueData }[] = [ }, }, { - label: '.gts component work (triggers ember-best-practices)', + label: '.gts component work (triggers UI skill suggestions)', issue: { id: 'Issues/gts-component', title: 'Build a dashboard component', @@ -117,11 +117,12 @@ async function main(): Promise { log.info(` Issue: ${issue.title}`); // 1. Resolve - let skillNames = resolver.resolve(issue, project); - log.info(` Resolved skills: [${skillNames.join(', ')}]`); + let { load: skillNames, suggested } = resolver.resolve(issue, project); + log.info(` Front-loaded skills: [${skillNames.join(', ')}]`); + log.info(` Suggested (on demand): [${suggested.join(', ')}]`); - // 2. Load (with issue context for reference filtering) - let skills = await loader.loadAll(skillNames, issue); + // 2. Load the front-loaded set + let skills = await loader.loadAll(skillNames); log.info(` Loaded: ${skills.length}/${skillNames.length} skills`); for (let skill of skills) { @@ -150,33 +151,22 @@ async function main(): Promise { } } - // Summary: list all discoverable skills - log.info('--- All discoverable skills ---'); - let allSkillNames = [ - 'boxel-development', - 'boxel-file-structure', - 'boxel-api', - 'boxel-command', - 'ember-best-practices', - 'software-factory-operations', - ]; - - let allSkills = await loader.loadAll(allSkillNames); + // Summary: the on-demand skill index (what read_skill exposes) + log.info('--- On-demand skill index ---'); + let index = await loader.buildIndex(); let grandTotal = 0; - for (let skill of allSkills) { + for (let entry of index) { + let skill = await loader.load(entry.name); let tokens = estimateTokens(skill); grandTotal += tokens; let refCount = skill.references?.length ?? 0; let refNote = refCount > 0 ? ` (${refCount} refs)` : ''; - log.info(` ${skill.name}: ~${tokens} tokens${refNote}`); + log.info(` ${entry.name}: ~${tokens} tokens${refNote}`); } - let missing = allSkillNames.filter( - (n) => !allSkills.find((s) => s.name === n), + log.info( + ` Total: ~${grandTotal} tokens across ${index.length} indexed skills ` + + '(loaded on demand, never all at once)', ); - if (missing.length > 0) { - log.info(` Not found: [${missing.join(', ')}]`); - } - log.info(` Total: ~${grandTotal} tokens across ${allSkills.length} skills`); log.info('\nSmoke test passed.'); } diff --git a/packages/software-factory/src/factory-agent/claude-code.ts b/packages/software-factory/src/factory-agent/claude-code.ts index 9a9aa80b5b6..0bc4b3dd2eb 100644 --- a/packages/software-factory/src/factory-agent/claude-code.ts +++ b/packages/software-factory/src/factory-agent/claude-code.ts @@ -295,6 +295,7 @@ export class ClaudeCodeFactoryAgent implements LoopAgent { darkfactoryModuleUrl: requireDarkfactoryModuleUrl(context), enableBoxelUiDiscovery: context.enableBoxelUiDiscovery === true, skills, + skillIndex: context.skillIndex ?? [], }); // Two tool surfaces are visible to the model on the Claude backend: diff --git a/packages/software-factory/src/factory-agent/opencode.ts b/packages/software-factory/src/factory-agent/opencode.ts index 48c0d4c1474..23f10ad9c86 100644 --- a/packages/software-factory/src/factory-agent/opencode.ts +++ b/packages/software-factory/src/factory-agent/opencode.ts @@ -473,6 +473,7 @@ export class OpencodeFactoryAgent implements LoopAgent { darkfactoryModuleUrl: requireDarkfactoryModuleUrl(context), enableBoxelUiDiscovery: context.enableBoxelUiDiscovery === true, skills, + skillIndex: context.skillIndex ?? [], }); } diff --git a/packages/software-factory/src/factory-agent/types.ts b/packages/software-factory/src/factory-agent/types.ts index 7d60327fc50..fe99a127433 100644 --- a/packages/software-factory/src/factory-agent/types.ts +++ b/packages/software-factory/src/factory-agent/types.ts @@ -114,6 +114,22 @@ export interface ResolvedSkill { references?: string[]; } +/** + * One row of the on-demand skill index rendered into the system prompt. + * The agent reads the description to decide relevance, then loads the full + * skill via the `read_skill` factory tool. + */ +export interface SkillIndexEntry { + name: string; + description: string; + /** + * Set when the resolver flagged this skill as likely relevant to the + * current issue. A hint for the agent, not a gate — any indexed skill is + * loadable. + */ + suggested?: boolean; +} + export interface ToolArg { name: string; type: string; @@ -216,7 +232,14 @@ export interface AgentContext { project: ProjectData; issue: IssueData; knowledge: KnowledgeArticleData[]; + /** Front-loaded skills, rendered in full into the system prompt. */ skills: ResolvedSkill[]; + /** + * On-demand skill index: every skill the agent may load via `read_skill`, + * one line each in the system prompt. Entries the resolver considers + * relevant to the current issue carry `suggested: true`. + */ + skillIndex?: SkillIndexEntry[]; /** @deprecated Tools are now provided separately as FactoryTool[] to agent.run(). */ tools?: ToolManifest[]; /** @deprecated Use validationResults/validationContext instead. */ diff --git a/packages/software-factory/src/factory-context-builder.ts b/packages/software-factory/src/factory-context-builder.ts index 5979201be50..46cfdf6524a 100644 --- a/packages/software-factory/src/factory-context-builder.ts +++ b/packages/software-factory/src/factory-context-builder.ts @@ -3,6 +3,7 @@ import type { IssueData, KnowledgeArticleData, ProjectData, + SkillIndexEntry, TestResult, ValidationResults, } from './factory-agent/index.ts'; @@ -71,14 +72,42 @@ export class ContextBuilder { this.enableBoxelUiDiscovery = config.enableBoxelUiDiscovery === true; } + /** + * Resolve, load, and index skills for one issue. + * + * Front-loaded skills (`resolution.load`) are read in full and budgeted; + * everything else in the library becomes an on-demand index entry, with + * the resolver's `suggested` picks marked. Skills that made it into the + * front-loaded set are dropped from the index — their full text is + * already in the prompt. + */ + private async resolveSkills( + issue: IssueData, + project: ProjectData, + ): Promise<{ skills: ResolvedSkill[]; skillIndex: SkillIndexEntry[] }> { + let resolution = this.skillResolver.resolve(issue, project); + + let skills = await this.skillLoader.loadAll(resolution.load); + skills = enforceSkillBudget(skills, this.maxSkillTokens); + + let loadedNames = new Set(skills.map((s) => s.name)); + let suggestedNames = new Set(resolution.suggested); + let skillIndex = (await this.skillLoader.buildIndex()) + .filter((entry) => !loadedNames.has(entry.name)) + .map((entry) => + suggestedNames.has(entry.name) ? { ...entry, suggested: true } : entry, + ); + + return { skills, skillIndex }; + } + /** * Assemble a complete AgentContext for one iteration of the execution loop. * * Steps: - * 1. Resolve skill names from issue + project context - * 2. Load all resolved skills from disk - * 3. Apply skill budget if configured - * 4. Return AgentContext (tools are provided separately as FactoryTool[]) + * 1. Resolve front-loaded skill names + suggestions from issue/project + * 2. Load front-loaded skills from disk, build the on-demand skill index + * 3. Return AgentContext (tools are provided separately as FactoryTool[]) */ async build(params: { project: ProjectData; @@ -92,24 +121,14 @@ export class ContextBuilder { let { project, issue, knowledge, targetRealm, darkfactoryModuleUrl } = params; - // Step 1: Resolve which skills are needed for this issue - let skillNames = this.skillResolver.resolve(issue, project); - - // Step 2: Load skill content from disk - let skills: ResolvedSkill[] = await this.skillLoader.loadAll( - skillNames, - issue, - ); - - // Step 3: Enforce token budget if configured - skills = enforceSkillBudget(skills, this.maxSkillTokens); + let { skills, skillIndex } = await this.resolveSkills(issue, project); - // Step 4: Assemble the context let context: AgentContext = { project, issue, knowledge, skills, + skillIndex, targetRealm, enableBoxelUiDiscovery: this.enableBoxelUiDiscovery, ...(darkfactoryModuleUrl ? { darkfactoryModuleUrl } : {}), @@ -170,22 +189,16 @@ export class ContextBuilder { } } - // Step 2: Resolve and load skills - let skillNames = this.skillResolver.resolve(issue, project); - let skills: ResolvedSkill[] = await this.skillLoader.loadAll( - skillNames, - issue, - ); - - // Step 3: Enforce token budget if configured - skills = enforceSkillBudget(skills, this.maxSkillTokens); + // Step 2: Resolve and load front-loaded skills + on-demand index + let { skills, skillIndex } = await this.resolveSkills(issue, project); - // Step 4: Assemble the context + // Step 3: Assemble the context let context: AgentContext = { project, issue, knowledge, skills, + skillIndex, targetRealm, enableBoxelUiDiscovery: this.enableBoxelUiDiscovery, ...(darkfactoryModuleUrl ? { darkfactoryModuleUrl } : {}), diff --git a/packages/software-factory/src/factory-entrypoint.ts b/packages/software-factory/src/factory-entrypoint.ts index e93736d877b..51ffda7d1ea 100644 --- a/packages/software-factory/src/factory-entrypoint.ts +++ b/packages/software-factory/src/factory-entrypoint.ts @@ -77,6 +77,14 @@ export interface FactoryEntrypointOptions { * Set via `--enable-boxel-ui-discovery` on the CLI. */ enableBoxelUiDiscovery?: boolean; + /** + * Override skill-library directories. When set, the on-demand skill + * library (index + read_skill) comes exclusively from these directories; + * front-loaded workflow skills still come from the bundled sources. An + * empty library fails the run at startup. Set via one or more + * `--skills-dir ` flags on the CLI. + */ + skillsDirs?: string[]; } export interface FactoryEntrypointAction { @@ -206,6 +214,13 @@ export function getFactoryEntrypointUsage(): string { ' When unset (and OPENROUTER_API_KEY env is also unset), the', ' backend falls back to the realm server passthrough at', ' `/_openrouter/chat/completions` — burns boxel tokens.', + ' --skills-dir Use this directory as the ONLY source of on-demand', + ' skills (the index + read_skill tool) — the bundled', + ' boxel-cli plugin skills are not read at all. The', + ' front-loaded factory workflow skills still come from', + ' the factory package. Fails at startup if the', + ' directory yields no skills. Repeatable; earlier', + ' flags win on name collisions.', ' --debug Log LLM prompts and responses to stderr', ' --enable-boxel-ui-discovery Make the agent search the catalog for @cardstack/boxel-ui', ' component Spec cards before writing UI in a .gts template.', @@ -263,6 +278,10 @@ export function parseFactoryEntrypointArgs( 'enable-boxel-ui-discovery': { type: 'boolean', }, + 'skills-dir': { + type: 'string', + multiple: true, + }, }, }); } catch (error) { @@ -306,6 +325,23 @@ export function parseFactoryEntrypointArgs( } } + let skillsDirs: string[] | undefined; + let rawSkillsDirs = parsed.values['skills-dir']; + if (Array.isArray(rawSkillsDirs)) { + let cleaned = rawSkillsDirs + .filter((d): d is string => typeof d === 'string') + .map((d) => d.trim()) + .filter((d) => d !== ''); + if (cleaned.length !== rawSkillsDirs.length) { + throw new FactoryEntrypointUsageError( + '--skills-dir requires a non-empty path', + ); + } + if (cleaned.length > 0) { + skillsDirs = cleaned; + } + } + return { briefUrl: normalizeUrl(briefUrl, '--brief-url'), targetRealm: normalizeUrl(targetRealm, '--target-realm'), @@ -317,6 +353,7 @@ export function parseFactoryEntrypointArgs( retryBlocked: parsed.values['no-retry-blocked'] === true ? false : true, enableBoxelUiDiscovery: parsed.values['enable-boxel-ui-discovery'] === true ? true : undefined, + skillsDirs, }; } @@ -462,6 +499,7 @@ export async function runFactoryEntrypoint( debug: options.debug, retryBlocked: options.retryBlocked, enableBoxelUiDiscovery: options.enableBoxelUiDiscovery, + skillLibraryDirs: options.skillsDirs, // Wire the board and the seed issue's project the moment the bootstrap // issue finishes, rather than after the whole loop returns — so a run // whose later issues stall or get interrupted still ends up with the diff --git a/packages/software-factory/src/factory-issue-loop-wiring.ts b/packages/software-factory/src/factory-issue-loop-wiring.ts index a2722d7aa96..e504310a7e1 100644 --- a/packages/software-factory/src/factory-issue-loop-wiring.ts +++ b/packages/software-factory/src/factory-issue-loop-wiring.ts @@ -103,6 +103,15 @@ export interface IssueLoopWiringConfig { * awareness of boxel-ui components. See CS-10527. */ enableBoxelUiDiscovery?: boolean; + /** + * Override skill-library directories (`--skills-dir`). When set, the + * on-demand skill library (index + read_skill) comes EXCLUSIVELY from + * these directories — nothing bundled is indexed or readable, and no + * exclusion filtering applies. Front-loaded workflow skills still come + * from the bundled sources. A library that yields zero skills fails the + * run at startup. + */ + skillLibraryDirs?: string[]; /** * Invoked once, right after the bootstrap issue completes. The entrypoint * uses this to link the realm index's `board` relationship as soon as the @@ -143,11 +152,30 @@ export async function runFactoryIssueLoop( workspaceDir, realmUrl: targetRealm, }); + // One SkillLoader for both the context builder (front-loaded skills + + // on-demand index) and the read_skill tool, so they share the same + // library view and per-run cache. + let skillLibraryDirs = (config.skillLibraryDirs ?? []).map((dir) => + resolve(dir), + ); + let skillLoader = new SkillLoader(undefined, undefined, { + libraryDirs: skillLibraryDirs, + }); + if (skillLibraryDirs.length > 0) { + // Fail fast on a misconfigured library (empty dir, wrong path, skills + // without frontmatter descriptions) instead of surfacing it as a + // skill-less agent mid-run. buildIndex throws in that case. + let index = await skillLoader.buildIndex(); + log.info( + `Skill library overridden by --skills-dir (${index.length} skill(s)): ` + + skillLibraryDirs.join(', '), + ); + } let contextBuilder = new ContextBuilder({ skillResolver: new DefaultSkillResolver({ enableBoxelUiDiscovery: config.enableBoxelUiDiscovery === true, }), - skillLoader: new SkillLoader(), + skillLoader, issueLoader, enableBoxelUiDiscovery: config.enableBoxelUiDiscovery === true, }); @@ -214,6 +242,7 @@ export async function runFactoryIssueLoop( hostAppUrl, syncWorkspace, validationCache, + skillReader: skillLoader, }; let tools: FactoryTool[] = buildFactoryTools( diff --git a/packages/software-factory/src/factory-prompt-loader.ts b/packages/software-factory/src/factory-prompt-loader.ts index cc3d0b7ea35..45cfa6bf93d 100644 --- a/packages/software-factory/src/factory-prompt-loader.ts +++ b/packages/software-factory/src/factory-prompt-loader.ts @@ -415,6 +415,7 @@ export function assembleSystemPrompt( targetRealm: context.targetRealm, darkfactoryModuleUrl: requireDarkfactoryModuleUrl(context), skills, + skillIndex: context.skillIndex ?? [], }); } diff --git a/packages/software-factory/src/factory-skill-loader.ts b/packages/software-factory/src/factory-skill-loader.ts index 73b996c19c7..52787a7e91c 100644 --- a/packages/software-factory/src/factory-skill-loader.ts +++ b/packages/software-factory/src/factory-skill-loader.ts @@ -5,6 +5,7 @@ import type { IssueData, ProjectData, ResolvedSkill, + SkillIndexEntry, } from './factory-agent/index.ts'; import { logger } from './logger.ts'; @@ -16,61 +17,68 @@ const log = logger('factory-skill-loader'); const PACKAGE_ROOT = resolve(import.meta.dirname, '..'); const MONOREPO_ROOT = resolve(PACKAGE_ROOT, '../..'); + /** - * The SDK orchestrator and the new interactive Claude Code path each get - * their own copies of `software-factory-bootstrap` / `software-factory-operations`. - * The orchestrator loads from `.agents/skills-orchestrator/`; its skills still describe - * the factory-MCP-tool surface (`signal_done`, `get_card_schema`, `run_lint`, …) - * that the orchestrator's `ToolUseFactoryAgent` actually provides. Interactive - * Claude Code reads from `.agents/skills/` via the `.claude/skills` symlink; - * those skills describe the `boxel` CLI surface and the agent-owned status - * lifecycle. The two diverged during CS-11149 and need to stay separated - * until the orchestrator is retired. + * The factory's own workflow skills (`software-factory-bootstrap`, + * `software-factory-operations`). These describe the factory-MCP-tool + * surface (`signal_done`, `get_card_schema`, `run_lint`, …) that the + * orchestrator's agent actually provides, so they are front-loaded in + * full — the agent must follow the delivery workflow, not discover it. + * + * (The sibling `.agents/skills/` directory holds the interactive Claude + * Code variants of these skills — a different tool surface — and is + * deliberately NOT on this loader's search path.) */ const DEFAULT_SKILLS_DIR = join(PACKAGE_ROOT, '.agents', 'skills-orchestrator'); /** - * Additional skill search directories, checked in order when a skill is not - * found in the primary directory. + * The domain-skill library: `packages/boxel-cli/plugin/skills/`. * - * - `packages/boxel-cli/plugin/skills/` hosts the boxel-cli Claude Code - * plugin skills (`boxel-api`, `boxel-command`, etc.) — boxel-cli owns the - * entire Boxel API surface, so its skills describe the platform. Same - * directory the plugin distributes to end users. - * - The monorepo root `.agents/skills/` hosts shared domain skills - * (`boxel-development`, `boxel-file-structure`, `ember-best-practices`). + * This is the factory's single source of truth for everything that isn't + * the factory workflow itself. It contains two populations, both consumed + * on demand via the skill index + `read_skill` tool: + * + * - skills bundled from the `cardstack/boxel-skills` repo at a pinned tag + * by boxel-cli's `build:skills` script (`boxel`, `boxel-patterns`, + * `boxel-design`, …); + * - CLI-native skills authored in place (`boxel-api`, `boxel-command`, + * `boxel-file-structure`, …) documenting the `boxel` CLI surface. */ const DEFAULT_FALLBACK_DIRS = [ join(MONOREPO_ROOT, 'packages', 'boxel-cli', 'plugin', 'skills'), - join(MONOREPO_ROOT, '.agents', 'skills'), - // Package-local interactive skills (`packages/software-factory/.agents/skills`) - // are the primary skill set for the runbook (interactive Claude Code) loop and - // also where the optional `boxel-ui-component-discovery` skill lives. Listing - // them here lets the orchestrator's resolver pick them up too when the - // matching flag (`--enable-boxel-ui-discovery`) is on. - join(PACKAGE_ROOT, '.agents', 'skills'), ]; +/** + * Skills excluded from the factory's on-demand index. Their entire subject + * is workspace/realm lifecycle the orchestrator owns — an agent following + * them (e.g. `boxel realm push`, `boxel file write`) would mutate the realm + * behind the orchestrator's workspace→realm sync and corrupt the loop's + * view of what changed. + */ +const FACTORY_EXCLUDED_SKILLS: ReadonlySet = new Set([ + 'file-ops', + 'realm-sync', + 'realm-history', + 'profile', + 'boxel-environment', +]); + /** Approximate characters per token for budget estimation. */ const CHARS_PER_TOKEN = 4; /** - * Priority order for skills. Lower index = higher priority. + * Priority order for front-loaded skills. Lower index = higher priority. * Skills not in this list get appended at the end (lowest priority). + * Only the factory workflow skills are front-loaded, so this list is + * short; knowledge-tag opt-ins land behind them. */ const SKILL_PRIORITY: readonly string[] = [ 'software-factory-bootstrap', - 'boxel-development', - 'boxel-file-structure', - 'boxel-api', - 'boxel-command', - 'boxel-ui-component-discovery', - 'ember-best-practices', 'software-factory-operations', ]; // --------------------------------------------------------------------------- -// Keyword matchers for skill resolution +// Keyword matchers for skill suggestion // --------------------------------------------------------------------------- /** Keywords in issue content that indicate .gts component work. */ @@ -84,39 +92,6 @@ const GTS_KEYWORDS = [ 'FieldDef', ]; -/** - * Reference files in `boxel-development/references/` and the keywords that - * trigger their inclusion. When an issue doesn't match any keyword, only the - * "always load" references from SKILL.md are included. - */ -const REFERENCE_KEYWORD_MAP: Record = { - 'dev-core-patterns.md': ['pattern', 'card', 'structure', 'safe'], - 'dev-template-patterns.md': ['template', 'component', 'render'], - 'dev-delegated-rendering.md': ['delegat', 'render', 'template'], - 'dev-styling-design.md': ['style', 'css', 'design', 'layout', 'visual'], - 'dev-theme-design-system.md': ['theme', 'design system', 'token', 'style'], - 'dev-fitted-formats.md': ['fitted', 'format', 'grid', 'dashboard'], - 'dev-query-systems.md': ['query', 'search', 'filter', 'find'], - 'dev-data-management.md': ['data', 'manage', 'relationship', 'link'], - 'dev-file-def.md': ['file', 'asset', 'FileDef', 'upload'], - 'dev-enumerations.md': ['enum', 'select', 'option', 'choice'], - 'dev-defensive-programming.md': ['defensive', 'guard', 'error', 'safe'], - 'dev-external-libraries.md': ['library', 'external', 'third-party', 'npm'], - 'dev-command-development.md': ['command', 'action', 'invoke'], - 'dev-spec-usage.md': ['spec', 'catalog', 'specification'], - 'dev-qunit-testing.md': ['test', 'qunit', 'test.gts', 'verify'], - 'dev-replicate-ai.md': ['replicate', 'ai', 'model', 'ml'], -}; - -/** References that are always loaded for boxel-development (per SKILL.md). */ -const ALWAYS_LOAD_REFERENCES: readonly string[] = [ - 'dev-core-concept.md', - 'dev-technical-rules.md', - 'dev-quick-reference.md', - 'dev-qunit-testing.md', - 'dev-spec-usage.md', -]; - // --------------------------------------------------------------------------- // Internal types for tracking reference metadata // --------------------------------------------------------------------------- @@ -136,16 +111,28 @@ interface RawSkillData { // SkillResolver // --------------------------------------------------------------------------- +/** + * The resolver's split decision for one issue: which skills go into the + * system prompt in full, and which merely get a "suggested" marker in the + * on-demand skill index. The agent can load ANY indexed skill via + * `read_skill` regardless of the suggestions — they are a hint, not a gate. + */ +export interface SkillResolution { + /** Skill names front-loaded in full into the system prompt. */ + load: string[]; + /** Skill names highlighted as suggested in the on-demand skill index. */ + suggested: string[]; +} + export interface SkillResolver { - resolve(issue: IssueData, project: ProjectData): string[]; + resolve(issue: IssueData, project: ProjectData): SkillResolution; } export interface DefaultSkillResolverOptions { /** * Feature flag — when true, `boxel-ui-component-discovery` is added to the - * always-loaded skill set so the agent searches the catalog for boxel-ui - * Specs before writing UI in `.gts` files. When false (default), the skill - * is omitted and the agent has no awareness of it. Pair with the + * front-loaded skill set so the agent searches the catalog for boxel-ui + * Specs before writing UI in `.gts` files. Pair with the * `enableBoxelUiDiscovery` flag passed to the system-prompt template so * the catalog-search exception in `prompts/system.md` is also enabled. * See CS-10527. @@ -161,69 +148,69 @@ export class DefaultSkillResolver implements SkillResolver { } /** - * Determine which skills to load based on issue and project context. + * Decide which skills to front-load and which to suggest for this issue. * - * Resolution rules: - * 1. boxel-development + boxel-file-structure — always loaded - * 2. ember-best-practices — when issue involves .gts component code - * 3. software-factory-operations — for factory delivery workflow issues - * 4. boxel-api + boxel-command — always loaded so the agent has the realm - * search query syntax and host-command failure modes inline. - * 5. boxel-ui-component-discovery — always loaded when the - * `enableBoxelUiDiscovery` flag was passed at construction time. - * 6. KnowledgeArticle tags can specify additional skills. + * Front-loaded (full text in the system prompt): + * 1. The factory workflow skill — `software-factory-bootstrap` for + * bootstrap issues, `software-factory-operations` for everything else. + * 2. `boxel-ui-component-discovery` under its feature flag. + * 3. Any skills opted in via KnowledgeArticle `skill:` tags — a deliberate + * per-project/per-issue authoring decision. + * + * Suggested (marked in the on-demand index, loaded only when the agent + * calls `read_skill`): + * - The core domain skills every implementation issue plausibly needs + * (`boxel`, `boxel-file-structure`, `boxel-api`, `boxel-command`). + * - UI-flavored skills when the issue text indicates .gts component work. */ - resolve(issue: IssueData, project: ProjectData): string[] { + resolve(issue: IssueData, project: ProjectData): SkillResolution { let issueText = extractIssueText(issue); let issueType = (issue as Record).issueType; - // Bootstrap issues get the bootstrap skill instead of implementation skills + let load: string[]; + let suggested: string[]; + if (issueType === 'bootstrap') { - return ['software-factory-bootstrap', 'boxel-file-structure']; + load = ['software-factory-bootstrap']; + suggested = ['boxel-file-structure']; + } else { + load = ['software-factory-operations']; + suggested = [ + 'boxel', + 'boxel-file-structure', + 'boxel-api', + 'boxel-command', + ]; + + if (matchesAnyKeyword(issueText, GTS_KEYWORDS)) { + suggested.push('boxel-ui-guidelines', 'boxel-design', 'boxel-patterns'); + } } - let skills: string[] = [ - 'boxel-development', - 'boxel-file-structure', - 'boxel-api', - 'boxel-command', - ]; - if (this.enableBoxelUiDiscovery) { - // Always loaded under the feature flag. The directive must apply even - // when the issue text doesn't contain `component` / `.gts` / `template` - // literally — briefs that just describe a form, view, or "isolated - // render" would otherwise miss the discovery skill and the agent would - // hand-roll UI. See CS-10527 for the test run where that happened. - skills.push('boxel-ui-component-discovery'); + // Always front-loaded under the feature flag. The directive must apply + // even when the issue text doesn't contain `component` / `.gts` / + // `template` literally — briefs that just describe a form, view, or + // "isolated render" would otherwise miss the discovery skill and the + // agent would hand-roll UI. See CS-10527. + load.push('boxel-ui-component-discovery'); } - if (matchesAnyKeyword(issueText, GTS_KEYWORDS)) { - skills.push('ember-best-practices'); - } - - // Every non-bootstrap issue is a factory delivery issue (write/edit - // cards, run the validators, record progress), so always load the - // operations skill. It used to be gated on keywords in the issue text, - // which silently dropped the edit-in-place / guard-the-baseline guidance - // for sparse, human-authored issues (e.g. a one-line "Modernize the - // look" adjustment added via the board UI). - skills.push('software-factory-operations'); - - // Check for additional skills from knowledge articles on the project - // and from related knowledge on the issue itself. + // KnowledgeArticle skill tags are the deliberate opt-in for a + // must-have skill, so they stay front-loaded rather than suggested. let additionalSkills = extractKnowledgeSkillTags(project, issue); for (let skillName of additionalSkills) { - if (!skills.includes(skillName)) { - skills.push(skillName); + if (!load.includes(skillName)) { + load.push(skillName); } } log.info( - `Resolved skills for issue "${issue.id}" (issueType=${issueType ?? '(none)'}): ${skills.join(', ')}`, + `Resolved skills for issue "${issue.id}" (issueType=${issueType ?? '(none)'}): ` + + `load=[${load.join(', ')}], suggested=[${suggested.join(', ')}]`, ); - return skills; + return { load, suggested }; } } @@ -232,52 +219,94 @@ export class DefaultSkillResolver implements SkillResolver { // --------------------------------------------------------------------------- export interface SkillLoaderInterface { - load(skillName: string, issue?: IssueData): Promise; - loadAll(skillNames: string[], issue?: IssueData): Promise; + load(skillName: string): Promise; + loadAll(skillNames: string[]): Promise; + buildIndex(): Promise; +} + +/** + * The read surface the `read_skill` factory tool needs. Kept separate from + * `SkillLoaderInterface` so the tool builder can depend on exactly the + * on-demand operations. + */ +export interface SkillReaderInterface { + buildIndex(): Promise; + readSkill(skillName: string): Promise; + readReference(skillName: string, fileName: string): Promise; +} + +export interface ReadSkillResult { + name: string; + content: string; + /** Filenames of this skill's reference documents, fetchable individually. */ + referenceFiles: string[]; } -export class SkillLoader implements SkillLoaderInterface { - private skillsDirs: string[]; +export class SkillLoader implements SkillLoaderInterface, SkillReaderInterface { + /** Bundled sources — always serve `load()`/`loadAll()` (front-loaded skills). */ + private defaultDirs: string[]; + /** + * When set (`--skills-dir`), the on-demand library — `buildIndex()`, + * `readSkill()`, `readReference()` — comes EXCLUSIVELY from these + * directories, with no exclusion filtering (the operator curates them). + * Front-loaded skills still resolve from the bundled sources. + */ + private libraryDirs: string[] | undefined; private rawCache: Map = new Map(); + private indexCache: SkillIndexEntry[] | undefined; /** * @param skillsDir Primary directory to search for skills. * @param fallbackDirs Additional directories checked (in order) when a * skill is not found in the primary directory. Defaults - * to the monorepo root `.agents/skills/`. + * to boxel-cli's `plugin/skills/`. + * @param options.libraryDirs Optional operator-supplied skill directories + * (`--skills-dir`). When present, they fully replace + * the on-demand skill library — nothing outside them + * is indexed or readable via `read_skill`, and the + * exclusion list does not apply. */ constructor( skillsDir: string = DEFAULT_SKILLS_DIR, fallbackDirs: string[] = DEFAULT_FALLBACK_DIRS, + options: { libraryDirs?: string[] } = {}, ) { - this.skillsDirs = [skillsDir, ...fallbackDirs]; + this.defaultDirs = [skillsDir, ...fallbackDirs]; + this.libraryDirs = + options.libraryDirs && options.libraryDirs.length > 0 + ? options.libraryDirs + : undefined; } /** - * Load a single skill by name. + * Load a single skill by name from the bundled sources (front-load path; + * unaffected by `--skills-dir`). * Searches the primary skills directory first, then each fallback directory. - * When an issue is provided, `boxel-development` references are filtered to - * only include issue-relevant files (always applied, not just with a budget). * Results are cached for the duration of the factory run. */ - async load(skillName: string, issue?: IssueData): Promise { - let raw = await this.loadRaw(skillName); - return toResolvedSkill(raw, issue); + async load(skillName: string): Promise { + let raw = await this.loadRaw(skillName, this.defaultDirs, 'default'); + let refContents = + raw.references && raw.references.length > 0 + ? raw.references.map((r) => r.content) + : undefined; + return { + name: raw.name, + content: raw.content, + ...(refContents ? { references: refContents } : {}), + }; } /** * Load all skills matching the resolved names. * Missing skills log a warning but do not fail the batch. */ - async loadAll( - skillNames: string[], - issue?: IssueData, - ): Promise { + async loadAll(skillNames: string[]): Promise { let results: ResolvedSkill[] = []; for (let name of skillNames) { try { - let skill = await this.load(name, issue); + let skill = await this.load(name); results.push(skill); } catch (error) { console.warn( @@ -291,24 +320,143 @@ export class SkillLoader implements SkillLoaderInterface { return results; } - /** Clear the cache so skills are re-read from disk on next load. */ + /** + * Build the on-demand skill index: one `{ name, description }` entry per + * skill directory across the library, keyed by directory name (first + * directory wins on name collisions). Descriptions come from the + * `description:` field of each SKILL.md's frontmatter — skills without one + * are skipped with a warning, since an undescribed entry gives the agent + * nothing to decide relevance by. Cached for the run duration. + * + * Library scope: the bundled sources minus FACTORY_EXCLUDED_SKILLS — or, + * when `libraryDirs` was configured, exactly those directories with no + * exclusions. An override library that yields zero skills is a + * configuration error and throws rather than silently degrading the + * agent to no on-demand skills. + */ + async buildIndex(): Promise { + if (this.indexCache) { + return this.indexCache; + } + + let scanDirs = this.libraryDirs ?? this.defaultDirs; + let applyExclusions = this.libraryDirs === undefined; + let entries = new Map(); + + for (let baseDir of scanDirs) { + let dirNames: string[]; + try { + dirNames = await readdir(baseDir); + } catch { + continue; + } + + for (let name of dirNames.sort()) { + if ( + entries.has(name) || + (applyExclusions && FACTORY_EXCLUDED_SKILLS.has(name)) + ) { + continue; + } + let skillMdPath = join(baseDir, name, 'SKILL.md'); + let content: string; + try { + content = await readFile(skillMdPath, 'utf8'); + } catch { + // Not a skill directory (plain file, or missing SKILL.md) — skip. + continue; + } + let description = parseFrontmatterDescription(content); + if (!description) { + log.warn( + `Skill "${name}" has no frontmatter description — omitting from the skill index`, + ); + continue; + } + entries.set(name, { name, description }); + } + } + + if (this.libraryDirs && entries.size === 0) { + throw new SkillLoadError( + `--skills-dir provided but no skills found in: ${this.libraryDirs.join(', ')}. ` + + 'Each skill must be a directory containing a SKILL.md with a frontmatter description.', + ); + } + + this.indexCache = [...entries.values()].sort((a, b) => + a.name.localeCompare(b.name), + ); + return this.indexCache; + } + + /** + * Read one skill's SKILL.md for on-demand consumption, along with the + * filenames of its reference documents (fetchable via `readReference`). + * Served from the library scope (see `buildIndex`). + */ + async readSkill(skillName: string): Promise { + let raw = await this.loadRawFromLibrary(skillName); + return { + name: raw.name, + content: raw.content, + referenceFiles: (raw.references ?? []).map((r) => r.fileName), + }; + } + + /** + * Read a single named reference document of a skill, from the library + * scope. `fileName` must be one of the names returned by + * `readSkill().referenceFiles` — arbitrary paths are rejected. + */ + async readReference(skillName: string, fileName: string): Promise { + let raw = await this.loadRawFromLibrary(skillName); + let ref = (raw.references ?? []).find((r) => r.fileName === fileName); + if (!ref) { + let available = (raw.references ?? []).map((r) => r.fileName); + throw new SkillLoadError( + `Skill "${skillName}" has no reference "${fileName}". ` + + (available.length > 0 + ? `Available references: ${available.join(', ')}` + : 'This skill has no reference files.'), + ); + } + return ref.content; + } + + /** Clear the caches so skills are re-read from disk on next load. */ clearCache(): void { this.rawCache.clear(); + this.indexCache = undefined; + } + + /** Load raw skill data from the on-demand library scope. */ + private async loadRawFromLibrary(skillName: string): Promise { + return this.libraryDirs + ? this.loadRaw(skillName, this.libraryDirs, 'library') + : this.loadRaw(skillName, this.defaultDirs, 'default'); } /** * Load raw skill data (with reference filenames preserved) from disk. - * Cached for the factory run duration. + * Cached for the factory run duration. `cacheScope` keeps the bundled + * and override-library caches apart — the same skill name can resolve + * to different directories in each. */ - private async loadRaw(skillName: string): Promise { - let cached = this.rawCache.get(skillName); + private async loadRaw( + skillName: string, + searchDirs: string[], + cacheScope: 'default' | 'library', + ): Promise { + let cacheKey = `${cacheScope}:${skillName}`; + let cached = this.rawCache.get(cacheKey); if (cached) { return cached; } - let skillDir = await this.findSkillDir(skillName); + let skillDir = await this.findSkillDir(skillName, searchDirs); if (!skillDir) { - let searched = this.skillsDirs.map((d) => join(d, skillName)).join(', '); + let searched = searchDirs.map((d) => join(d, skillName)).join(', '); throw new SkillLoadError( `Skill "${skillName}" not found. Searched: ${searched}`, ); @@ -327,8 +475,8 @@ export class SkillLoader implements SkillLoaderInterface { let references: NamedReference[] | undefined; - // Check for rules/ directory first (e.g., ember-best-practices). - // Load compiled AGENTS.md instead of individual rule files. + // Check for rules/ directory first. Load compiled AGENTS.md instead of + // individual rule files. let rulesDir = join(skillDir, 'rules'); if (await directoryExists(rulesDir)) { let agentsMdPath = join(skillDir, 'AGENTS.md'); @@ -340,8 +488,8 @@ export class SkillLoader implements SkillLoaderInterface { } } - // Check for references/ directory (e.g., boxel-development). - // Load all reference files with their filenames preserved. + // Check for references/ directory. Load all reference files with their + // filenames preserved. if (!references) { let refsDir = join(skillDir, 'references'); if (await directoryExists(refsDir)) { @@ -362,16 +510,19 @@ export class SkillLoader implements SkillLoaderInterface { ...(references && references.length > 0 ? { references } : {}), }; - this.rawCache.set(skillName, raw); + this.rawCache.set(cacheKey, raw); return raw; } /** - * Search all configured skill directories for a skill by name. + * Search the given skill directories for a skill by name. * Returns the full path to the skill directory, or undefined if not found. */ - private async findSkillDir(skillName: string): Promise { - for (let baseDir of this.skillsDirs) { + private async findSkillDir( + skillName: string, + searchDirs: string[], + ): Promise { + for (let baseDir of searchDirs) { let candidate = join(baseDir, skillName); let skillMd = join(candidate, 'SKILL.md'); try { @@ -385,6 +536,41 @@ export class SkillLoader implements SkillLoaderInterface { } } +// --------------------------------------------------------------------------- +// Frontmatter parsing +// --------------------------------------------------------------------------- + +/** + * Extract the `description:` value from a SKILL.md's YAML frontmatter. + * Handles the single-line scalar form every skill in the search path uses + * (optionally quoted); returns undefined when there is no frontmatter block + * or no description field. + */ +export function parseFrontmatterDescription( + content: string, +): string | undefined { + if (!content.startsWith('---')) { + return undefined; + } + let end = content.indexOf('\n---', 3); + if (end === -1) { + return undefined; + } + let block = content.slice(3, end); + let match = block.match(/^description:[ \t]*(.+)$/m); + if (!match) { + return undefined; + } + let value = match[1].trim(); + if ( + (value.startsWith('"') && value.endsWith('"')) || + (value.startsWith("'") && value.endsWith("'")) + ) { + value = value.slice(1, -1); + } + return value === '' ? undefined : value; +} + // --------------------------------------------------------------------------- // Context budget enforcement // --------------------------------------------------------------------------- @@ -396,9 +582,9 @@ export class SkillLoader implements SkillLoaderInterface { * skipped, and later (lower-priority) skills may still be included if they * fit within the remaining budget. * - * Reference filtering for `boxel-development` is handled at load time by the - * SkillLoader when an issue is provided — it is always applied regardless of - * whether a budget is set. + * Only front-loaded skills pass through here (the workflow skill plus + * knowledge-tag opt-ins) — on-demand skills are never budgeted, they are + * fetched as tool results. */ export function enforceSkillBudget( skills: ResolvedSkill[], @@ -455,66 +641,6 @@ export function estimateTokens(skill: ResolvedSkill): number { return Math.ceil(total / CHARS_PER_TOKEN); } -// --------------------------------------------------------------------------- -// Internal: convert raw data to public ResolvedSkill -// --------------------------------------------------------------------------- - -/** - * Convert RawSkillData (with named references) to the public ResolvedSkill - * interface. For `boxel-development`, filters references by issue relevance - * using actual filenames — this happens on every load, not just when a - * budget is enforced. - */ -function toResolvedSkill(raw: RawSkillData, issue?: IssueData): ResolvedSkill { - let refs = raw.references; - - if (refs && raw.name === 'boxel-development' && issue) { - refs = filterBoxelDevelopmentRefs(refs, issue); - } - - let refContents = - refs && refs.length > 0 ? refs.map((r) => r.content) : undefined; - - return { - name: raw.name, - content: raw.content, - ...(refContents ? { references: refContents } : {}), - }; -} - -/** - * Filter boxel-development references to include only the "always load" - * references plus those whose keywords match the issue text. - * Uses actual filenames from disk — no index-based reconstruction. - */ -function filterBoxelDevelopmentRefs( - refs: NamedReference[], - issue: IssueData, -): NamedReference[] { - let issueText = extractIssueText(issue); - - return refs.filter((ref) => { - // Always-load refs are always included - if ( - ALWAYS_LOAD_REFERENCES.includes( - ref.fileName as (typeof ALWAYS_LOAD_REFERENCES)[number], - ) - ) { - return true; - } - - // Check if issue text matches any keywords for this reference - let keywords = REFERENCE_KEYWORD_MAP[ref.fileName]; - if (keywords && matchesAnyKeyword(issueText, keywords)) { - return true; - } - - // References not in the keyword map (e.g., dev-file-editing.md) are only - // loaded when no issue context is available (handled by the caller). - return false; - }); -} - // --------------------------------------------------------------------------- // Helper functions // --------------------------------------------------------------------------- diff --git a/packages/software-factory/src/factory-tool-builder.ts b/packages/software-factory/src/factory-tool-builder.ts index 1ee5585f532..a33d955e76b 100644 --- a/packages/software-factory/src/factory-tool-builder.ts +++ b/packages/software-factory/src/factory-tool-builder.ts @@ -16,6 +16,7 @@ import { type RunEvaluateInMemoryOptions, type RunEvaluateResult, } from './eval-execution.ts'; +import type { SkillReaderInterface } from './factory-skill-loader.ts'; import type { ToolExecutor } from './factory-tool-executor.ts'; import type { ToolRegistry } from './factory-tool-registry.ts'; import { @@ -95,6 +96,12 @@ export interface ToolBuilderConfig { * once by the pipeline). */ validationCache?: ValidationRunCache; + /** + * On-demand skill reader backing the `read_skill` tool. Shared with the + * ContextBuilder's SkillLoader so both see the same skill library (and + * cache). When omitted, the `read_skill` tool is not registered. + */ + skillReader?: SkillReaderInterface; /** Injected for testing — defaults to runLintInMemory. */ runLintInMemory?: (options: RunLintInMemoryOptions) => Promise; /** Injected for testing — defaults to runTestsInMemory. */ @@ -172,6 +179,10 @@ export function buildFactoryTools( buildRequestClarificationTool(), ]; + if (config.skillReader) { + tools.push(buildReadSkillTool(config.skillReader)); + } + // Wrap registered realm-api manifests (currently just `realm-create`). // Tracker-schema cards (Project / IssueTracker / Issue / KnowledgeArticle / // Spec / issue comments) used to have dedicated wrapper tools here that @@ -311,6 +322,79 @@ function buildGetCardSchemaTool(config: ToolBuilderConfig): FactoryTool { }; } +function buildReadSkillTool(skillReader: SkillReaderInterface): FactoryTool { + return { + name: 'read_skill', + description: + 'Load a skill from the factory skill library on demand. The system ' + + 'prompt lists every available skill with a one-line description — ' + + 'call this with the skill name BEFORE doing work its description ' + + 'covers, instead of guessing at conventions. Without "reference", ' + + "returns the skill's SKILL.md content plus the filenames of its " + + 'reference documents; call again with "reference" set to one of ' + + 'those filenames to fetch that document. Results are served from ' + + 'the local skill library — cheap to call, no realm access.', + parameters: { + type: 'object', + properties: { + skill: { + type: 'string', + description: + 'Skill name exactly as it appears in the skill index (e.g. ' + + '`boxel`, `boxel-patterns`).', + }, + reference: { + type: 'string', + description: + 'Optional reference-document filename from a prior read_skill ' + + 'result (e.g. `lint-workflow.md`). Omit to read the skill ' + + 'itself.', + }, + }, + required: ['skill'], + }, + execute: async (args) => { + let skill = requireStringArg(args, 'skill', 'read_skill'); + let rawReference = args.reference; + let reference = + typeof rawReference === 'string' && rawReference.trim() !== '' + ? rawReference.trim() + : undefined; + // Gate on the index rather than the filesystem: skills excluded from + // the index (workspace/realm lifecycle the orchestrator owns) must + // not be loadable either, and an unknown name gets the list of what + // IS loadable so the model can self-correct in one round-trip. + let index = await skillReader.buildIndex(); + if (!index.some((entry) => entry.name === skill)) { + return { + ok: false, + error: + `Unknown skill "${skill}". Available skills: ` + + `${index.map((entry) => entry.name).join(', ')}.`, + }; + } + try { + if (reference) { + let content = await skillReader.readReference(skill, reference); + return { ok: true, skill, reference, content }; + } + let result = await skillReader.readSkill(skill); + return { + ok: true, + skill: result.name, + content: result.content, + referenceFiles: result.referenceFiles, + }; + } catch (error) { + return { + ok: false, + error: error instanceof Error ? error.message : String(error), + }; + } + }, + }; +} + function buildRunTestsTool(config: ToolBuilderConfig): FactoryTool { let execute = config.runTestsInMemory ?? runTestsInMemory; return { diff --git a/packages/software-factory/tests/factory-context-builder.test.ts b/packages/software-factory/tests/factory-context-builder.test.ts index ea1810f233b..b373aff4f35 100644 --- a/packages/software-factory/tests/factory-context-builder.test.ts +++ b/packages/software-factory/tests/factory-context-builder.test.ts @@ -5,6 +5,7 @@ import type { KnowledgeArticleData, ProjectData, ResolvedSkill, + SkillIndexEntry, TestResult, ValidationResults, IssueData, @@ -18,6 +19,7 @@ import { import type { SkillLoaderInterface, + SkillResolution, SkillResolver, } from '../src/factory-skill-loader.ts'; @@ -26,32 +28,40 @@ import type { // --------------------------------------------------------------------------- class StubSkillResolver implements SkillResolver { - /** Pre-configured skill names returned by resolve(). */ - skillNames: string[]; + /** Pre-configured resolution returned by resolve(). */ + resolution: SkillResolution; /** Records all (issue, project) pairs passed to resolve(). */ calls: { issue: IssueData; project: ProjectData }[] = []; - constructor(skillNames: string[] = ['boxel-development']) { - this.skillNames = skillNames; + constructor( + load: string[] = ['boxel-development'], + suggested: string[] = [], + ) { + this.resolution = { load, suggested }; } - resolve(issue: IssueData, project: ProjectData): string[] { + resolve(issue: IssueData, project: ProjectData): SkillResolution { this.calls.push({ issue, project }); - return this.skillNames; + return this.resolution; } } class StubSkillLoader implements SkillLoaderInterface { /** Map from skill name to the ResolvedSkill that load() returns. */ private skillMap: Map; - /** Records all loadAll() calls: [skillNames, issue]. */ - loadAllCalls: { skillNames: string[]; issue?: IssueData }[] = []; + /** Index entries returned by buildIndex(). */ + indexEntries: SkillIndexEntry[]; + /** Records all loadAll() calls. */ + loadAllCalls: { skillNames: string[] }[] = []; - constructor(skills: ResolvedSkill[] = []) { + constructor(skills: ResolvedSkill[] = [], indexEntries?: SkillIndexEntry[]) { this.skillMap = new Map(skills.map((s) => [s.name, s])); + this.indexEntries = + indexEntries ?? + skills.map((s) => ({ name: s.name, description: `About ${s.name}` })); } - async load(skillName: string, _issue?: IssueData): Promise { + async load(skillName: string): Promise { let skill = this.skillMap.get(skillName); if (!skill) { throw new Error(`StubSkillLoader: unknown skill "${skillName}"`); @@ -59,11 +69,8 @@ class StubSkillLoader implements SkillLoaderInterface { return skill; } - async loadAll( - skillNames: string[], - issue?: IssueData, - ): Promise { - this.loadAllCalls.push({ skillNames, issue }); + async loadAll(skillNames: string[]): Promise { + this.loadAllCalls.push({ skillNames }); let results: ResolvedSkill[] = []; for (let name of skillNames) { let skill = this.skillMap.get(name); @@ -73,6 +80,10 @@ class StubSkillLoader implements SkillLoaderInterface { } return results; } + + async buildIndex(): Promise { + return this.indexEntries; + } } class StubIssueRelationshipLoader implements IssueRelationshipLoader { @@ -205,43 +216,89 @@ module('factory-context-builder > skill resolution', function () { assert.deepEqual( loader.loadAllCalls[0].skillNames, ['boxel-development', 'ember-best-practices'], - 'loadAll() received the resolved skill names', + 'loadAll() received the front-load skill names', ); }); - test('passes issue to loadAll for reference filtering', async function (assert) { - let resolver = new StubSkillResolver(['boxel-development']); - let loader = new StubSkillLoader([makeSkill('boxel-development')]); + test('includes resolved skills in the context', async function (assert) { + let resolver = new StubSkillResolver([ + 'boxel-development', + 'ember-best-practices', + ]); + let loader = new StubSkillLoader([ + makeSkill('boxel-development', 'BD content'), + makeSkill('ember-best-practices', 'EBP content'), + ]); let { config } = makeConfig({ skillResolver: resolver, skillLoader: loader, }); let builder = new ContextBuilder(config); - let issue = makeIssue(); - await builder.build({ + let ctx = await builder.build({ project: makeProject(), - issue, + issue: makeIssue(), knowledge: [], targetRealm: 'https://example.test/target/', }); - assert.strictEqual( - loader.loadAllCalls[0].issue, - issue, - 'loadAll() received the issue for reference filtering', + assert.strictEqual(ctx.skills.length, 2, 'two skills in context'); + assert.strictEqual(ctx.skills[0].name, 'boxel-development'); + assert.strictEqual(ctx.skills[1].name, 'ember-best-practices'); + }); +}); + +// --------------------------------------------------------------------------- +// Tests: On-demand skill index +// --------------------------------------------------------------------------- + +module('factory-context-builder > skill index', function () { + test('context carries the skill index minus front-loaded skills', async function (assert) { + let resolver = new StubSkillResolver(['software-factory-operations']); + let loader = new StubSkillLoader( + [makeSkill('software-factory-operations')], + [ + { name: 'boxel', description: 'Core Boxel coding skill' }, + { name: 'boxel-patterns', description: 'Copy-paste patterns' }, + { + name: 'software-factory-operations', + description: 'Factory workflow', + }, + ], + ); + let { config } = makeConfig({ + skillResolver: resolver, + skillLoader: loader, + }); + let builder = new ContextBuilder(config); + + let ctx = await builder.build({ + project: makeProject(), + issue: makeIssue(), + knowledge: [], + targetRealm: 'https://example.test/target/', + }); + + assert.deepEqual( + ctx.skillIndex?.map((entry) => entry.name), + ['boxel', 'boxel-patterns'], + 'front-loaded skill is dropped from the index', ); }); - test('includes resolved skills in the context', async function (assert) { - let resolver = new StubSkillResolver([ - 'boxel-development', - 'ember-best-practices', - ]); - let loader = new StubSkillLoader([ - makeSkill('boxel-development', 'BD content'), - makeSkill('ember-best-practices', 'EBP content'), - ]); + test('resolver suggestions are marked in the index', async function (assert) { + let resolver = new StubSkillResolver( + ['software-factory-operations'], + ['boxel', 'boxel-design'], + ); + let loader = new StubSkillLoader( + [makeSkill('software-factory-operations')], + [ + { name: 'boxel', description: 'Core Boxel coding skill' }, + { name: 'boxel-design', description: 'Visual decisions' }, + { name: 'boxel-patterns', description: 'Copy-paste patterns' }, + ], + ); let { config } = makeConfig({ skillResolver: resolver, skillLoader: loader, @@ -255,9 +312,51 @@ module('factory-context-builder > skill resolution', function () { targetRealm: 'https://example.test/target/', }); - assert.strictEqual(ctx.skills.length, 2, 'two skills in context'); - assert.strictEqual(ctx.skills[0].name, 'boxel-development'); - assert.strictEqual(ctx.skills[1].name, 'ember-best-practices'); + let byName = new Map(ctx.skillIndex!.map((entry) => [entry.name, entry])); + assert.true(byName.get('boxel')?.suggested, 'boxel marked suggested'); + assert.true( + byName.get('boxel-design')?.suggested, + 'boxel-design marked suggested', + ); + assert.strictEqual( + byName.get('boxel-patterns')?.suggested, + undefined, + 'unsuggested entry carries no marker', + ); + }); + + test('buildForIssue also carries the skill index', async function (assert) { + let resolver = new StubSkillResolver( + ['software-factory-operations'], + ['boxel'], + ); + let loader = new StubSkillLoader( + [makeSkill('software-factory-operations')], + [{ name: 'boxel', description: 'Core Boxel coding skill' }], + ); + let issueLoader = new StubIssueRelationshipLoader(); + let builder = new ContextBuilder({ + skillResolver: resolver, + skillLoader: loader, + issueLoader, + }); + + let ctx = await builder.buildForIssue({ + issue: makeIssue(), + targetRealm: 'https://example.test/target/', + }); + + assert.deepEqual( + ctx.skillIndex, + [ + { + name: 'boxel', + description: 'Core Boxel coding skill', + suggested: true, + }, + ], + 'index present with suggestion marker', + ); }); }); diff --git a/packages/software-factory/tests/factory-entrypoint.test.ts b/packages/software-factory/tests/factory-entrypoint.test.ts index 77506511695..89e9281ff72 100644 --- a/packages/software-factory/tests/factory-entrypoint.test.ts +++ b/packages/software-factory/tests/factory-entrypoint.test.ts @@ -76,9 +76,43 @@ module('factory-entrypoint', function (hooks) { debug: undefined, retryBlocked: true, enableBoxelUiDiscovery: undefined, + skillsDirs: undefined, }); }); + test('parseFactoryEntrypointArgs collects repeatable --skills-dir flags', function (assert) { + let options = parseFactoryEntrypointArgs([ + '--brief-url', + briefUrl, + '--target-realm', + targetRealm, + '--skills-dir', + '/tmp/skill-overrides', + '--skills-dir', + './more-skills', + ]); + + assert.deepEqual(options.skillsDirs, [ + '/tmp/skill-overrides', + './more-skills', + ]); + }); + + test('parseFactoryEntrypointArgs rejects an empty --skills-dir', function (assert) { + assert.throws( + () => + parseFactoryEntrypointArgs([ + '--brief-url', + briefUrl, + '--target-realm', + targetRealm, + '--skills-dir', + '', + ]), + /--skills-dir requires a non-empty path/, + ); + }); + test('parseFactoryEntrypointArgs accepts --agent claude', function (assert) { let options = parseFactoryEntrypointArgs([ '--brief-url', diff --git a/packages/software-factory/tests/factory-prompt-loader.test.ts b/packages/software-factory/tests/factory-prompt-loader.test.ts index 97b12db5972..9204df0e47f 100644 --- a/packages/software-factory/tests/factory-prompt-loader.test.ts +++ b/packages/software-factory/tests/factory-prompt-loader.test.ts @@ -326,6 +326,47 @@ module('factory-prompt-loader > assembleSystemPrompt', function () { assert.notOk(result.includes('# Skill:'), 'no skill section when empty'); }); + test('renders the on-demand skill index with suggested markers', function (assert) { + let loader = new FilePromptLoader(); + let ctx = makeMinimalContext({ + skills: [], + skillIndex: [ + { + name: 'boxel', + description: 'Core Boxel coding skill.', + suggested: true, + }, + { name: 'boxel-patterns', description: 'Copy-paste patterns.' }, + ], + }); + let result = assembleSystemPrompt({ context: ctx, loader }); + + assert.ok( + result.includes('# Skill library (load on demand)'), + 'skill library section present', + ); + assert.ok( + result.includes('`boxel` **(suggested)** — Core Boxel coding skill.'), + 'suggested entry carries the marker', + ); + assert.ok( + result.includes('`boxel-patterns` — Copy-paste patterns.'), + 'unsuggested entry has no marker', + ); + assert.ok(result.includes('read_skill'), 'explains how to load skills'); + }); + + test('omits the skill library section when the index is empty', function (assert) { + let loader = new FilePromptLoader(); + let ctx = makeMinimalContext({ skills: [], skillIndex: [] }); + let result = assembleSystemPrompt({ context: ctx, loader }); + + assert.notOk( + result.includes('# Skill library'), + 'no skill library section without index entries', + ); + }); + test('snapshot: system prompt with sample skills', function (assert) { let loader = new FilePromptLoader(); let ctx = makeMinimalContext({ diff --git a/packages/software-factory/tests/factory-skill-loader.test.ts b/packages/software-factory/tests/factory-skill-loader.test.ts index 03c9a9c137a..8bef463b90e 100644 --- a/packages/software-factory/tests/factory-skill-loader.test.ts +++ b/packages/software-factory/tests/factory-skill-loader.test.ts @@ -17,6 +17,7 @@ import { enforceSkillBudget, estimateTokens, extractIssueText, + parseFrontmatterDescription, } from '../src/factory-skill-loader.ts'; // --------------------------------------------------------------------------- @@ -92,86 +93,89 @@ function makeProject(overrides?: Partial): ProjectData { // --------------------------------------------------------------------------- module('factory-skill-loader > DefaultSkillResolver', function () { - test('always includes boxel-development and boxel-file-structure', function (assert) { + test('front-loads only the operations skill for implementation issues', function (assert) { let resolver = new DefaultSkillResolver(); let issue = makeIssue({ description: 'Generic task with no keywords' }); let project = makeProject(); - let skills = resolver.resolve(issue, project); + let { load, suggested } = resolver.resolve(issue, project); - assert.true( - skills.includes('boxel-development'), - 'includes boxel-development', + assert.deepEqual( + load, + ['software-factory-operations'], + 'only the workflow skill is front-loaded', ); - assert.true( - skills.includes('boxel-file-structure'), - 'includes boxel-file-structure', + assert.deepEqual( + suggested, + ['boxel', 'boxel-file-structure', 'boxel-api', 'boxel-command'], + 'core domain skills are suggested, not loaded', ); - assert.true(skills.includes('boxel-api'), 'includes boxel-api'); - assert.true(skills.includes('boxel-command'), 'includes boxel-command'); }); - test('includes ember-best-practices when issue mentions .gts', function (assert) { + test('bootstrap issues front-load the bootstrap skill', function (assert) { let resolver = new DefaultSkillResolver(); - let issue = makeIssue({ - description: 'Create a new card definition in .gts format', - }); + let issue = makeIssue({ issueType: 'bootstrap' }); let project = makeProject(); - let skills = resolver.resolve(issue, project); + let { load, suggested } = resolver.resolve(issue, project); - assert.true( - skills.includes('ember-best-practices'), - 'includes ember-best-practices for .gts work', - ); + assert.deepEqual(load, ['software-factory-bootstrap']); + assert.deepEqual(suggested, ['boxel-file-structure']); }); - test('includes ember-best-practices when issue mentions component', function (assert) { + test('suggests UI skills when issue mentions .gts', function (assert) { let resolver = new DefaultSkillResolver(); let issue = makeIssue({ - description: 'Build a new component for the dashboard', + description: 'Create a new card definition in .gts format', }); let project = makeProject(); - let skills = resolver.resolve(issue, project); + let { suggested } = resolver.resolve(issue, project); - assert.true( - skills.includes('ember-best-practices'), - 'includes ember-best-practices for component work', - ); + assert.true(suggested.includes('boxel-ui-guidelines')); + assert.true(suggested.includes('boxel-design')); + assert.true(suggested.includes('boxel-patterns')); }); - test('includes ember-best-practices when issue mentions CardDef', function (assert) { + test('suggests UI skills when issue mentions component or CardDef', function (assert) { let resolver = new DefaultSkillResolver(); - let issue = makeIssue({ - title: 'Define a new CardDef for employees', - }); let project = makeProject(); - let skills = resolver.resolve(issue, project); + let componentIssue = makeIssue({ + description: 'Build a new component for the dashboard', + }); + assert.true( + resolver + .resolve(componentIssue, project) + .suggested.includes('boxel-ui-guidelines'), + 'component keyword triggers UI suggestions', + ); + let cardDefIssue = makeIssue({ + title: 'Define a new CardDef for employees', + }); assert.true( - skills.includes('ember-best-practices'), - 'includes ember-best-practices for CardDef work', + resolver + .resolve(cardDefIssue, project) + .suggested.includes('boxel-ui-guidelines'), + 'CardDef keyword triggers UI suggestions', ); }); - test('includes software-factory-operations for delivery workflow issues', function (assert) { + test('no UI suggestions without .gts keywords', function (assert) { let resolver = new DefaultSkillResolver(); let issue = makeIssue({ description: 'Improve the factory delivery pipeline', }); let project = makeProject(); - let skills = resolver.resolve(issue, project); + let { suggested } = resolver.resolve(issue, project); - assert.true( - skills.includes('software-factory-operations'), - 'includes software-factory-operations for factory workflow', - ); + assert.false(suggested.includes('boxel-ui-guidelines')); + assert.false(suggested.includes('boxel-design')); }); - test('includes software-factory-operations even for sparse issues with no workflow keywords', function (assert) { + test('front-loads operations even for sparse issues with no workflow keywords', function (assert) { // Regression: a one-line human-authored adjustment added via the board // UI ("Modernize the look") used to miss the operations skill because the // loader gated it on workflow keywords in the issue text. Every @@ -183,15 +187,15 @@ module('factory-skill-loader > DefaultSkillResolver', function () { }); let project = makeProject(); - let skills = resolver.resolve(issue, project); + let { load } = resolver.resolve(issue, project); assert.true( - skills.includes('software-factory-operations'), - 'sparse adjustment issue still loads the operations skill', + load.includes('software-factory-operations'), + 'sparse adjustment issue still front-loads the operations skill', ); }); - test('extra skills can be opted in via knowledge article tags', function (assert) { + test('knowledge article tags opt skills into the front-loaded set', function (assert) { let resolver = new DefaultSkillResolver(); let issue = makeIssue(); let project = makeProject({ @@ -203,38 +207,19 @@ module('factory-skill-loader > DefaultSkillResolver', function () { ], }); - let skills = resolver.resolve(issue, project); + let { load } = resolver.resolve(issue, project); - // Knowledge-tag opt-in is the deliberate way to include any non-default - // skill (e.g., a project-specific domain skill that lives in a knowledge - // article). The resolver returns the name; the loader resolves it on disk. - assert.true( - skills.includes('custom-extension'), - 'custom-extension included from knowledge tag', - ); + // Knowledge-tag opt-in is the deliberate way to force-load any + // non-default skill, so tagged skills are front-loaded, not merely + // suggested. The resolver returns the name; the loader resolves it + // on disk. assert.true( - skills.includes('another-domain-skill'), - 'another-domain-skill included from knowledge tag', + load.includes('custom-extension'), + 'custom-extension front-loaded from knowledge tag', ); - }); - - test('extracts additional skills from knowledge article tags', function (assert) { - let resolver = new DefaultSkillResolver(); - let issue = makeIssue(); - let project = makeProject({ - knowledge: [ - { - id: 'Knowledge Articles/custom', - tags: ['skill:custom-skill', 'not-a-skill'], - }, - ], - }); - - let skills = resolver.resolve(issue, project); - assert.true( - skills.includes('custom-skill'), - 'includes skill from knowledge article tag', + load.includes('another-domain-skill'), + 'another-domain-skill front-loaded from knowledge tag', ); }); @@ -250,10 +235,10 @@ module('factory-skill-loader > DefaultSkillResolver', function () { ], }); - let skills = resolver.resolve(issue, project); + let { load } = resolver.resolve(issue, project); - assert.true(skills.includes('extra-skill-a'), 'includes extra-skill-a'); - assert.true(skills.includes('extra-skill-b'), 'includes extra-skill-b'); + assert.true(load.includes('extra-skill-a'), 'includes extra-skill-a'); + assert.true(load.includes('extra-skill-b'), 'includes extra-skill-b'); }); test('reads skills from knowledgeBase field (Project schema)', function (assert) { @@ -268,10 +253,10 @@ module('factory-skill-loader > DefaultSkillResolver', function () { ], }); - let skills = resolver.resolve(issue, project); + let { load } = resolver.resolve(issue, project); assert.true( - skills.includes('schema-skill'), + load.includes('schema-skill'), 'includes skill from knowledgeBase field', ); }); @@ -288,31 +273,45 @@ module('factory-skill-loader > DefaultSkillResolver', function () { }); let project = makeProject(); - let skills = resolver.resolve(issue, project); + let { load } = resolver.resolve(issue, project); assert.true( - skills.includes('issue-skill'), + load.includes('issue-skill'), 'includes skill from issue relatedKnowledge', ); }); - test('does not duplicate skills', function (assert) { + test('does not duplicate front-loaded skills', function (assert) { let resolver = new DefaultSkillResolver(); - let issue = makeIssue({ - description: 'Create a .gts component with template patterns', - }); + let issue = makeIssue(); let project = makeProject({ knowledge: [ { id: 'Knowledge Articles/dup', - skills: ['boxel-development'], + skills: ['software-factory-operations'], }, ], }); - let skills = resolver.resolve(issue, project); - let devCount = skills.filter((s) => s === 'boxel-development').length; - assert.strictEqual(devCount, 1, 'boxel-development appears only once'); + let { load } = resolver.resolve(issue, project); + let opsCount = load.filter( + (s) => s === 'software-factory-operations', + ).length; + assert.strictEqual( + opsCount, + 1, + 'software-factory-operations appears only once', + ); + }); + + test('front-loads boxel-ui-component-discovery under the feature flag', function (assert) { + let resolver = new DefaultSkillResolver({ enableBoxelUiDiscovery: true }); + let issue = makeIssue(); + let project = makeProject(); + + let { load } = resolver.resolve(issue, project); + + assert.true(load.includes('boxel-ui-component-discovery')); }); test('reads issue text from title, description, tags, and labels', function (assert) { @@ -337,11 +336,10 @@ module('factory-skill-loader > DefaultSkillResolver', function () { let issue = makeIssue(); let project = makeProject(); // no knowledge field - let skills = resolver.resolve(issue, project); + let { load, suggested } = resolver.resolve(issue, project); - // Should still resolve the base skills without error - assert.true(skills.includes('boxel-development')); - assert.true(skills.includes('boxel-file-structure')); + assert.deepEqual(load, ['software-factory-operations']); + assert.true(suggested.includes('boxel')); }); }); @@ -664,103 +662,272 @@ module('factory-skill-loader > SkillLoader', function (hooks) { rmSync(fallbackDir, { recursive: true, force: true }); } }); +}); - test('filters boxel-development references by issue when loaded with issue', async function (assert) { - // Set up a boxel-development skill with references matching the real structure - writeSkill(tempDir, 'boxel-development', '# Boxel Development', { - references: { - 'dev-core-concept.md': 'Core concept content', - 'dev-technical-rules.md': 'Technical rules content', - 'dev-quick-reference.md': 'Quick reference content', - 'dev-styling-design.md': 'Styling design content', - 'dev-file-editing.md': 'File editing content', - 'dev-query-systems.md': 'Query systems content', - }, - }); +// --------------------------------------------------------------------------- +// Skill index + on-demand reads +// --------------------------------------------------------------------------- + +module('factory-skill-loader > skill index', function (hooks) { + hooks.beforeEach(function () { + tempDir = createTempSkillsDir(); + }); + + hooks.afterEach(function () { + if (tempDir && existsSync(tempDir)) { + rmSync(tempDir, { recursive: true, force: true }); + } + }); + + function frontmatter(description: string): string { + return `---\ndescription: ${description}\n---\n\n# Body\n`; + } + + test('buildIndex lists name + frontmatter description per skill', async function (assert) { + writeSkill(tempDir, 'skill-b', frontmatter('Skill B does things')); + writeSkill(tempDir, 'skill-a', frontmatter('Skill A does other things')); let loader = new SkillLoader(tempDir, []); + let index = await loader.buildIndex(); - // Load WITHOUT issue — should get all references - let allRefs = await loader.load('boxel-development'); - assert.strictEqual( - allRefs.references!.length, - 6, - 'all 6 references loaded without issue', - ); + assert.deepEqual(index, [ + { name: 'skill-a', description: 'Skill A does other things' }, + { name: 'skill-b', description: 'Skill B does things' }, + ]); + }); - // Load WITH a styling issue — should get always-load + styling only - loader.clearCache(); - let stylingIssue = makeIssue({ - description: 'Fix the CSS styling on the card', - }); - let filtered = await loader.load('boxel-development', stylingIssue); + test('buildIndex skips skills without a frontmatter description', async function (assert) { + writeSkill(tempDir, 'described', frontmatter('Has a description')); + writeSkill(tempDir, 'undescribed', '# No frontmatter at all'); - assert.true( - filtered.references!.length < 6, - 'fewer references with issue filtering', - ); - // Always-load refs should be present - assert.true( - filtered.references!.some((r) => r.includes('Core concept')), - 'always-load ref included', + let loader = new SkillLoader(tempDir, []); + let index = await loader.buildIndex(); + + assert.deepEqual( + index.map((entry) => entry.name), + ['described'], + 'undescribed skill omitted', ); - // Styling ref should be present (keyword match) - assert.true( - filtered.references!.some((r) => r.includes('Styling design')), - 'keyword-matched ref included', + }); + + test('buildIndex excludes factory-inappropriate skills', async function (assert) { + writeSkill(tempDir, 'boxel', frontmatter('Core skill')); + writeSkill(tempDir, 'realm-sync', frontmatter('Push/pull realms')); + writeSkill(tempDir, 'file-ops', frontmatter('Write realm files')); + + let loader = new SkillLoader(tempDir, []); + let index = await loader.buildIndex(); + + assert.deepEqual( + index.map((entry) => entry.name), + ['boxel'], + 'realm-sync and file-ops are excluded from the index', ); - // File editing ref should NOT be present (no keyword match, not always-load) - assert.false( - filtered.references!.some((r) => r.includes('File editing')), - 'non-matching ref excluded', + }); + + test('buildIndex merges dirs with primary winning name collisions', async function (assert) { + let fallbackDir = createTempSkillsDir(); + writeSkill(tempDir, 'shared', frontmatter('Primary description')); + writeSkill(fallbackDir, 'shared', frontmatter('Fallback description')); + writeSkill(fallbackDir, 'only-fallback', frontmatter('Fallback-only')); + + try { + let loader = new SkillLoader(tempDir, [fallbackDir]); + let index = await loader.buildIndex(); + + assert.deepEqual(index, [ + { name: 'only-fallback', description: 'Fallback-only' }, + { name: 'shared', description: 'Primary description' }, + ]); + } finally { + rmSync(fallbackDir, { recursive: true, force: true }); + } + }); + + test('buildIndex ignores plain files in a skills dir', async function (assert) { + writeSkill(tempDir, 'real-skill', frontmatter('A real skill')); + writeFileSync(join(tempDir, 'glossary.md'), '# Not a skill dir', 'utf8'); + + let loader = new SkillLoader(tempDir, []); + let index = await loader.buildIndex(); + + assert.deepEqual( + index.map((entry) => entry.name), + ['real-skill'], ); }); - test('reference filtering works without budget (no-budget path)', async function (assert) { - // Verifies the P1 fix: callers that omit maxSkillTokens still get - // issue-relevant references, not all 19. The filtering happens at load - // time, so enforceSkillBudget(skills, undefined) returns already-filtered - // skills — no budget required. - writeSkill(tempDir, 'boxel-development', '# Boxel Development', { + test('readSkill returns content plus reference filenames', async function (assert) { + writeSkill(tempDir, 'with-refs', frontmatter('Skill with refs'), { references: { - 'dev-core-concept.md': 'Core concept content', - 'dev-technical-rules.md': 'Technical rules content', - 'dev-quick-reference.md': 'Quick reference content', - 'dev-styling-design.md': 'Styling design content', - 'dev-file-editing.md': 'File editing content', - 'dev-query-systems.md': 'Query systems content', + 'guide-a.md': 'Guide A content', + 'guide-b.md': 'Guide B content', }, }); let loader = new SkillLoader(tempDir, []); - let issue = makeIssue({ - description: 'Fix the CSS styling on the card', + let result = await loader.readSkill('with-refs'); + + assert.strictEqual(result.name, 'with-refs'); + assert.true(result.content.includes('# Body')); + assert.deepEqual(result.referenceFiles, ['guide-a.md', 'guide-b.md']); + }); + + test('readReference returns a single named reference', async function (assert) { + writeSkill(tempDir, 'with-refs', frontmatter('Skill with refs'), { + references: { + 'guide-a.md': 'Guide A content', + }, }); - // Load with issue — filtering happens at load time - let skills = await loader.loadAll(['boxel-development'], issue); - assert.strictEqual(skills.length, 1); + let loader = new SkillLoader(tempDir, []); + let content = await loader.readReference('with-refs', 'guide-a.md'); - // Pass through enforceSkillBudget with NO budget (undefined) - let result = enforceSkillBudget(skills, undefined); + assert.strictEqual(content, 'Guide A content'); + }); - // Should still have the filtered references from load time - assert.strictEqual(result.length, 1); - assert.true( - result[0].references!.length < 6, - 'references were filtered at load time even without budget', + test('library dirs fully replace the on-demand library', async function (assert) { + let libraryDir = createTempSkillsDir(); + writeSkill(tempDir, 'bundled-only', frontmatter('Bundled skill')); + writeSkill(tempDir, 'shared', frontmatter('Bundled description')); + writeSkill(libraryDir, 'shared', frontmatter('Library description')); + writeSkill(libraryDir, 'library-only', frontmatter('Only in the library')); + + try { + let loader = new SkillLoader(tempDir, [], { libraryDirs: [libraryDir] }); + + let index = await loader.buildIndex(); + assert.deepEqual( + index, + [ + { name: 'library-only', description: 'Only in the library' }, + { name: 'shared', description: 'Library description' }, + ], + 'bundled skills are not indexed at all', + ); + + let read = await loader.readSkill('shared'); + assert.true( + read.content.includes('Library description'), + 'readSkill serves the library version', + ); + + try { + await loader.readSkill('bundled-only'); + assert.ok(false, 'should have thrown'); + } catch (err) { + assert.true( + err instanceof SkillLoadError, + 'bundled skills are unreadable when the library is overridden', + ); + } + + // Front-loaded skills are unaffected by the override. + let frontLoaded = await loader.load('bundled-only'); + assert.true( + frontLoaded.content.includes('Bundled skill'), + 'load() still resolves from the bundled sources', + ); + } finally { + rmSync(libraryDir, { recursive: true, force: true }); + } + }); + + test('no exclusion filtering in an override library', async function (assert) { + let libraryDir = createTempSkillsDir(); + writeSkill(libraryDir, 'realm-sync', frontmatter('Curated sync skill')); + + try { + let loader = new SkillLoader(tempDir, [], { libraryDirs: [libraryDir] }); + let index = await loader.buildIndex(); + + assert.deepEqual( + index.map((entry) => entry.name), + ['realm-sync'], + 'the operator-curated library is not filtered', + ); + } finally { + rmSync(libraryDir, { recursive: true, force: true }); + } + }); + + test('an override library with no skills fails fast', async function (assert) { + let emptyDir = createTempSkillsDir(); + + try { + let loader = new SkillLoader(tempDir, [], { libraryDirs: [emptyDir] }); + try { + await loader.buildIndex(); + assert.ok(false, 'should have thrown'); + } catch (err) { + assert.true(err instanceof SkillLoadError); + assert.true( + (err as Error).message.includes('--skills-dir'), + 'error names the flag', + ); + } + } finally { + rmSync(emptyDir, { recursive: true, force: true }); + } + }); + + test('readReference rejects unknown filenames and lists available ones', async function (assert) { + writeSkill(tempDir, 'with-refs', frontmatter('Skill with refs'), { + references: { + 'guide-a.md': 'Guide A content', + }, + }); + + let loader = new SkillLoader(tempDir, []); + try { + await loader.readReference('with-refs', '../../../etc/passwd'); + assert.ok(false, 'should have thrown'); + } catch (err) { + assert.true(err instanceof SkillLoadError); + assert.true( + (err as Error).message.includes('guide-a.md'), + 'error lists the available reference filenames', + ); + } + }); +}); + +// --------------------------------------------------------------------------- +// parseFrontmatterDescription +// --------------------------------------------------------------------------- + +module('factory-skill-loader > parseFrontmatterDescription', function () { + test('extracts a plain description', function (assert) { + assert.strictEqual( + parseFrontmatterDescription('---\ndescription: Does things\n---\n# Hi'), + 'Does things', ); - assert.true( - result[0].references!.some((r) => r.includes('Core concept')), - 'always-load ref present in no-budget path', + }); + + test('strips surrounding quotes', function (assert) { + assert.strictEqual( + parseFrontmatterDescription('---\ndescription: "Quoted value"\n---\n'), + 'Quoted value', ); - assert.true( - result[0].references!.some((r) => r.includes('Styling design')), - 'keyword-matched ref present in no-budget path', + }); + + test('handles frontmatter with other fields', function (assert) { + assert.strictEqual( + parseFrontmatterDescription( + '---\nname: my-skill\ndescription: The description\nboxel:\n kind: skill\n---\n', + ), + 'The description', ); - assert.false( - result[0].references!.some((r) => r.includes('File editing')), - 'non-matching ref excluded in no-budget path', + }); + + test('returns undefined without frontmatter or description', function (assert) { + assert.strictEqual( + parseFrontmatterDescription('# Just a heading'), + undefined, + ); + assert.strictEqual( + parseFrontmatterDescription('---\nname: no-description\n---\n'), + undefined, ); }); }); @@ -835,8 +1002,8 @@ module('factory-skill-loader > enforceSkillBudget', function () { try { let skills: ResolvedSkill[] = [ - { name: 'boxel-development', content: 'a'.repeat(2000) }, // 500 tokens - { name: 'boxel-file-structure', content: 'b'.repeat(2000) }, // 500 tokens + { name: 'software-factory-bootstrap', content: 'a'.repeat(2000) }, // 500 tokens + { name: 'software-factory-operations', content: 'b'.repeat(2000) }, // 500 tokens { name: 'low-priority-test-skill', content: 'c'.repeat(2000) }, // 500 tokens ]; @@ -846,12 +1013,12 @@ module('factory-skill-loader > enforceSkillBudget', function () { assert.strictEqual(result.length, 2, 'only two skills fit'); assert.strictEqual( result[0].name, - 'boxel-development', + 'software-factory-bootstrap', 'highest priority kept', ); assert.strictEqual( result[1].name, - 'boxel-file-structure', + 'software-factory-operations', 'second priority kept', ); assert.true( @@ -871,8 +1038,8 @@ module('factory-skill-loader > enforceSkillBudget', function () { let skills: ResolvedSkill[] = [ // Present in reverse priority order { name: 'low-priority-test-skill', content: 'c'.repeat(2000) }, // not in SKILL_PRIORITY → lowest - { name: 'boxel-file-structure', content: 'b'.repeat(2000) }, // priority 2 - { name: 'boxel-development', content: 'a'.repeat(2000) }, // priority 1 + { name: 'software-factory-operations', content: 'b'.repeat(2000) }, // priority 2 + { name: 'software-factory-bootstrap', content: 'a'.repeat(2000) }, // priority 1 ]; // Budget enough for only 2 skills @@ -881,12 +1048,12 @@ module('factory-skill-loader > enforceSkillBudget', function () { assert.strictEqual(result.length, 2); assert.strictEqual( result[0].name, - 'boxel-development', + 'software-factory-bootstrap', 'highest priority first', ); assert.strictEqual( result[1].name, - 'boxel-file-structure', + 'software-factory-operations', 'second priority second', ); } finally { @@ -901,7 +1068,7 @@ module('factory-skill-loader > enforceSkillBudget', function () { try { let skills: ResolvedSkill[] = [ { name: 'unknown-skill', content: 'x'.repeat(2000) }, // not in priority list - { name: 'boxel-development', content: 'a'.repeat(2000) }, + { name: 'software-factory-operations', content: 'a'.repeat(2000) }, ]; let result = enforceSkillBudget(skills, 600); @@ -909,7 +1076,7 @@ module('factory-skill-loader > enforceSkillBudget', function () { assert.strictEqual(result.length, 1); assert.strictEqual( result[0].name, - 'boxel-development', + 'software-factory-operations', 'known skill kept over unknown', ); } finally { @@ -923,7 +1090,7 @@ module('factory-skill-loader > enforceSkillBudget', function () { // --------------------------------------------------------------------------- module('factory-skill-loader > re-resolution on new issue', function () { - test('resolver produces different skills for different issues', function (assert) { + test('resolver produces different suggestions for different issues', function (assert) { let resolver = new DefaultSkillResolver(); let project = makeProject(); @@ -934,27 +1101,24 @@ module('factory-skill-loader > re-resolution on new issue', function () { description: 'Improve the factory delivery pipeline', }); - let skills1 = resolver.resolve(issue1, project); - let skills2 = resolver.resolve(issue2, project); + let resolution1 = resolver.resolve(issue1, project); + let resolution2 = resolver.resolve(issue2, project); assert.true( - skills1.includes('ember-best-practices'), - 'issue1 gets ember-best-practices', + resolution1.suggested.includes('boxel-ui-guidelines'), + 'issue1 gets UI suggestions', ); - - // software-factory-operations now loads for every non-bootstrap issue, - // so it's no longer the differentiator — ember-best-practices is. assert.true( - skills1.includes('software-factory-operations'), - 'issue1 gets software-factory-operations (always loaded)', + resolution1.load.includes('software-factory-operations'), + 'issue1 front-loads software-factory-operations', ); assert.true( - skills2.includes('software-factory-operations'), - 'issue2 gets software-factory-operations', + resolution2.load.includes('software-factory-operations'), + 'issue2 front-loads software-factory-operations', ); assert.false( - skills2.includes('ember-best-practices'), - 'issue2 does not get ember-best-practices', + resolution2.suggested.includes('boxel-ui-guidelines'), + 'issue2 does not get UI suggestions', ); }); diff --git a/packages/software-factory/tests/factory-tool-builder.test.ts b/packages/software-factory/tests/factory-tool-builder.test.ts index ceab5eed6a4..0c6fbda2443 100644 --- a/packages/software-factory/tests/factory-tool-builder.test.ts +++ b/packages/software-factory/tests/factory-tool-builder.test.ts @@ -185,6 +185,119 @@ module('factory-tool-builder > tool building', function () { }); }); +// --------------------------------------------------------------------------- +// read_skill +// --------------------------------------------------------------------------- + +module('factory-tool-builder > read_skill', function () { + function makeSkillReader() { + return { + async buildIndex() { + return [ + { name: 'boxel', description: 'Core Boxel coding skill' }, + { name: 'boxel-patterns', description: 'Copy-paste patterns' }, + ]; + }, + async readSkill(skillName: string) { + if (skillName !== 'boxel') { + throw new Error(`unknown skill "${skillName}"`); + } + return { + name: 'boxel', + content: '# Boxel skill body', + referenceFiles: ['lint-workflow.md'], + }; + }, + async readReference(skillName: string, fileName: string) { + if (skillName === 'boxel' && fileName === 'lint-workflow.md') { + return 'Lint workflow content'; + } + throw new Error(`no reference "${fileName}"`); + }, + }; + } + + test('not registered when config has no skillReader', function (assert) { + let registry = new ToolRegistry(); + let { executor } = createMockToolExecutor(new Map()); + let tools = buildFactoryTools(makeConfig(), executor, registry); + + assert.false(tools.some((t) => t.name === 'read_skill')); + }); + + test('reads a skill and lists its reference files', async function (assert) { + let registry = new ToolRegistry(); + let { executor } = createMockToolExecutor(new Map()); + let config = makeConfig({ skillReader: makeSkillReader() }); + let tools = buildFactoryTools(config, executor, registry); + let tool = findTool(tools, 'read_skill'); + + let result = (await tool.execute({ skill: 'boxel' })) as { + ok: boolean; + skill: string; + content: string; + referenceFiles: string[]; + }; + + assert.true(result.ok); + assert.strictEqual(result.skill, 'boxel'); + assert.strictEqual(result.content, '# Boxel skill body'); + assert.deepEqual(result.referenceFiles, ['lint-workflow.md']); + }); + + test('reads a named reference document', async function (assert) { + let registry = new ToolRegistry(); + let { executor } = createMockToolExecutor(new Map()); + let config = makeConfig({ skillReader: makeSkillReader() }); + let tools = buildFactoryTools(config, executor, registry); + let tool = findTool(tools, 'read_skill'); + + let result = (await tool.execute({ + skill: 'boxel', + reference: 'lint-workflow.md', + })) as { ok: boolean; content: string }; + + assert.true(result.ok); + assert.strictEqual(result.content, 'Lint workflow content'); + }); + + test('unknown skill returns error listing available skills', async function (assert) { + let registry = new ToolRegistry(); + let { executor } = createMockToolExecutor(new Map()); + let config = makeConfig({ skillReader: makeSkillReader() }); + let tools = buildFactoryTools(config, executor, registry); + let tool = findTool(tools, 'read_skill'); + + let result = (await tool.execute({ skill: 'realm-sync' })) as { + ok: boolean; + error: string; + }; + + assert.false(result.ok, 'index-gated: excluded/unknown skills rejected'); + assert.true(result.error.includes('realm-sync')); + assert.true( + result.error.includes('boxel, boxel-patterns'), + 'error lists the loadable skills', + ); + }); + + test('unknown reference surfaces the loader error', async function (assert) { + let registry = new ToolRegistry(); + let { executor } = createMockToolExecutor(new Map()); + let config = makeConfig({ skillReader: makeSkillReader() }); + let tools = buildFactoryTools(config, executor, registry); + let tool = findTool(tools, 'read_skill'); + + let result = (await tool.execute({ + skill: 'boxel', + reference: 'nope.md', + })) as { ok: boolean; error: string }; + + assert.false(result.ok); + assert.true(result.error.includes('nope.md')); + }); +}); + // --------------------------------------------------------------------------- // Signal tools // ---------------------------------------------------------------------------