From 9c0cb4a7f92b3b6bc314326b9e0f3d2c220b5221 Mon Sep 17 00:00:00 2001 From: Emmitt <1337ERJ1337@gmail.com> Date: Mon, 9 Mar 2026 14:12:01 -0700 Subject: [PATCH 1/3] feat(skills): add SkillScriptLoader, handler types, and applySkillHandlers Implements the executable skill handler layer from PR #215 design proposal. Inspired by #297. - handler-types.ts: full type system for skill-script model (SkillHandler, HandlerLifecycle, concern interfaces, defineHandler() helper) - skill-script-loader.ts: SkillScriptLoader class, resolveSkillPath(), wrapSkillHandler(), CONCERN_TOOL_MAP - applySkillHandlers(): ToolRegistry integration for runtime handler replacement - test/skill-script-loader.test.ts: 33 tests covering load, execute, error paths Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .changeset/add-skill-script-loader.md | 5 + .../squad-sdk/src/skills/handler-types.ts | 335 ++++++++++ packages/squad-sdk/src/skills/index.ts | 6 + .../src/skills/skill-script-loader.ts | 241 ++++++++ packages/squad-sdk/src/tools/index.ts | 15 + test/skill-script-loader.test.ts | 580 ++++++++++++++++++ 6 files changed, 1182 insertions(+) create mode 100644 .changeset/add-skill-script-loader.md create mode 100644 packages/squad-sdk/src/skills/handler-types.ts create mode 100644 packages/squad-sdk/src/skills/skill-script-loader.ts create mode 100644 test/skill-script-loader.test.ts diff --git a/.changeset/add-skill-script-loader.md b/.changeset/add-skill-script-loader.md new file mode 100644 index 000000000..d36276e9f --- /dev/null +++ b/.changeset/add-skill-script-loader.md @@ -0,0 +1,5 @@ +--- +"@bradygaster/squad-sdk": minor +--- + +Add SkillScriptLoader, handler types, and applySkillHandlers for executable skill handler scripts diff --git a/packages/squad-sdk/src/skills/handler-types.ts b/packages/squad-sdk/src/skills/handler-types.ts new file mode 100644 index 000000000..f21f2bd4f --- /dev/null +++ b/packages/squad-sdk/src/skills/handler-types.ts @@ -0,0 +1,335 @@ +/** + * Type system for the skill-script model. + * + * This module defines the complete type contract for backend skills in `.squad/skills/`. + * Skills contain `scripts/` directories with executable JS handlers that replace built-in + * tool handlers in ToolRegistry. + * + * The skill-script model enables stateful, framework-aware backends for tasks, decisions, + * memories, and logging concerns. Handlers are ordinary async functions that return + * SquadToolResult values. + */ + +import type { SquadToolResult, SquadTool } from "../adapter/types.js"; + +// ─── Tool Argument Interfaces ──────────────────────────────────────────────── + +/** + * Arguments for squad_create_issue. + * All tool arg interfaces include [key: string]: unknown for skill-documented extensions. + */ +export interface CreateIssueArgs { + title: string; + body?: string; + assignee?: string; + [key: string]: unknown; +} + +/** + * Arguments for squad_update_issue. + */ +export interface UpdateIssueArgs { + issueId: string | number; + title?: string; + body?: string; + [key: string]: unknown; +} + +/** + * Arguments for squad_list_issues. + */ +export interface ListIssuesArgs { + status?: "all" | "open" | "closed"; + limit?: number; + [key: string]: unknown; +} + +/** + * Arguments for squad_close_issue. + */ +export interface CloseIssueArgs { + issueId: string | number; + comment?: string; + [key: string]: unknown; +} + +/** + * Arguments for squad_create_decision. + */ +export interface CreateDecisionArgs { + author: string; + summary: string; + body: string; + [key: string]: unknown; +} + +/** + * Arguments for squad_list_decisions. + */ +export interface ListDecisionsArgs { + status?: "all" | "pending" | "merged"; + limit?: number; + [key: string]: unknown; +} + +/** + * Arguments for squad_merge_decision. + */ +export interface MergeDecisionArgs { + slugs?: string[]; + [key: string]: unknown; +} + +/** + * Arguments for squad_create_memory. + */ +export interface CreateMemoryArgs { + content: string; + agent?: string; + [key: string]: unknown; +} + +/** + * Arguments for squad_list_memories. + */ +export interface ListMemoriesArgs { + agent?: string; + limit?: number; + [key: string]: unknown; +} + +/** + * Arguments for squad_create_log. + */ +export interface CreateLogArgs { + kind: "orchestration" | "session"; + content: string; + agent?: string; + [key: string]: unknown; +} + +/** + * Arguments for squad_list_logs. + */ +export interface ListLogsArgs { + kind?: "orchestration" | "session"; + limit?: number; + [key: string]: unknown; +} + +// ─── Core Handler Types ────────────────────────────────────────────────────── + +/** + * Skill handler function. + * + * @param args - Tool input matching the tool's schema + * @param config - Non-framework keys from the tracking config entry (excludes 'skill', 'disposeTimeoutMs') + * @returns SquadToolResult (string or SquadToolResultObject) + */ +export type SkillHandler = ( + args: TArgs, + config: Record, +) => Promise | SquadToolResult; + +/** + * Lifecycle hooks for stateful backend skills. + * lifecycle.js in the scripts/ dir exports these. + */ +export interface HandlerLifecycle { + /** + * Called once after handler resolution, before first tool call. + * init() must be idempotent. + */ + init?(config: Record): Promise; + + /** + * Called once at session end. + * Must be safe to call even if init() partially failed. + */ + dispose?(): Promise; +} + +// ─── Concern Handler Interfaces ────────────────────────────────────────────── + +/** + * Task concern handlers (squad_*_issue). + */ +export interface TaskHandlers extends HandlerLifecycle { + squad_create_issue?: SkillHandler; + squad_update_issue?: SkillHandler; + squad_list_issues?: SkillHandler; + squad_close_issue?: SkillHandler; +} + +/** + * Decision concern handlers (squad_*_decision). + */ +export interface DecisionHandlers extends HandlerLifecycle { + squad_create_decision?: SkillHandler; + squad_list_decisions?: SkillHandler; + squad_merge_decision?: SkillHandler; +} + +/** + * Memory concern handlers (squad_*_memory). + */ +export interface MemoryHandlers extends HandlerLifecycle { + squad_create_memory?: SkillHandler; + squad_list_memories?: SkillHandler; +} + +/** + * Logging concern handlers (squad_*_log). + */ +export interface LogHandlers extends HandlerLifecycle { + squad_create_log?: SkillHandler; + squad_list_logs?: SkillHandler; +} + +/** + * Union of all handler interfaces. + */ +export type AllHandlers = TaskHandlers & + DecisionHandlers & + MemoryHandlers & + LogHandlers; + +// ─── Disjoint Name Compile-Time Assertion ──────────────────────────────────── + +/** + * Strip HandlerLifecycle keys, keep only tool-name keys. + */ +type OwnKeys = Exclude; + +/** + * true if A and B share no tool-name keys; never otherwise. + */ +type AssertDisjoint = Extract, OwnKeys> extends never + ? true + : never; + +// All 6 pairs must be disjoint: +export type _TaskDecision = AssertDisjoint; +export type _TaskMemory = AssertDisjoint; +export type _TaskLog = AssertDisjoint; +export type _DecisionMemory = AssertDisjoint; +export type _DecisionLog = AssertDisjoint; +export type _MemoryLog = AssertDisjoint; + +// Compile fails if any pair shares a tool name: +const _disjointProof: [ + _TaskDecision, + _TaskMemory, + _TaskLog, + _DecisionMemory, + _DecisionLog, + _MemoryLog, +] = [true, true, true, true, true, true]; + +// ─── ConcernMap and Related ────────────────────────────────────────────────── + +/** + * Maps concern names to their handler interfaces. + */ +export interface ConcernMap { + tasks: TaskHandlers; + decisions: DecisionHandlers; + memories: MemoryHandlers; + logging: LogHandlers; +} + +/** + * Valid concern names. + */ +export type Concern = keyof ConcernMap; + +// ─── LoadResult ────────────────────────────────────────────────────────────── + +/** + * Return type for SkillScriptLoader.load(). + */ +export interface LoadResult { + /** + * Fully-formed SquadTool entries — skill handlers combined with built-in schemas. + */ + tools: SquadTool[]; + + /** + * Lifecycle hooks from scripts/lifecycle.js, if present. + */ + lifecycle?: { + init?(config: Record): Promise; + dispose?(): Promise; + }; +} + +// ─── Config Types ──────────────────────────────────────────────────────────── + +/** + * Skill configuration entry. + * + * Specifies a skill directory path and optional disposal timeout. + * The 'package' key is reserved for future use and forbidden here. + */ +export interface SkillConfig { + /** Path to skill directory (relative to squad root) */ + skill: string; + /** Prevent future package key coexisting with skill */ + package?: never; + /** Timeout for dispose() in ms (default: 10000) */ + disposeTimeoutMs?: number; + [key: string]: unknown; +} + +/** + * A reference to a backend: + * - undefined → built-in markdown (default) + * - "markdown" → built-in markdown (explicit reset) + * - "noop" → silent no-op (disables the concern) + * - { skill, ...opts } → skill directory with config options + */ +export type BackendRef = "markdown" | "noop" | SkillConfig; + +/** + * Direct handler registration (alternative to skill path). + * Used for programmatically-provided handlers. + */ +export interface HandlerRegistration { + handlers: H; +} + +/** + * Tracking configuration for backend skills. + * + * Controls which backend (markdown, noop, or skill) serves each concern. + * Supports a global default with per-concern overrides. + */ +export interface TrackingConfig { + /** Backend for ALL concerns unless individually overridden */ + default?: BackendRef; + decisions?: BackendRef | HandlerRegistration; + memories?: BackendRef | HandlerRegistration; + tasks?: BackendRef | HandlerRegistration; + logging?: BackendRef | HandlerRegistration; +} + +// ─── defineHandler Helper ──────────────────────────────────────────────────── + +/** + * Identity function for type inference in handler scripts. + * + * Provides compile-time safety when authoring TypeScript handlers. + * The compiled .js output is a plain function — no runtime dependency on this. + * + * @example + * ```typescript + * export default defineHandler(async (args, config) => { + * return { type: "success", text: `Issue created: ${args.title}` }; + * }); + * ``` + */ +export function defineHandler( + handler: SkillHandler, +): SkillHandler { + return handler; +} diff --git a/packages/squad-sdk/src/skills/index.ts b/packages/squad-sdk/src/skills/index.ts index c0d99ce5e..70b09801d 100644 --- a/packages/squad-sdk/src/skills/index.ts +++ b/packages/squad-sdk/src/skills/index.ts @@ -112,3 +112,9 @@ export class SkillRegistry { return this.skills.get(skillId)?.content; } } + +// --- Skill Script System Exports --- + +export * from './handler-types.js'; +export { SkillScriptLoader, resolveSkillPath } from './skill-script-loader.js'; +export type { LoadResult } from './handler-types.js'; diff --git a/packages/squad-sdk/src/skills/skill-script-loader.ts b/packages/squad-sdk/src/skills/skill-script-loader.ts new file mode 100644 index 000000000..8a98e4928 --- /dev/null +++ b/packages/squad-sdk/src/skills/skill-script-loader.ts @@ -0,0 +1,241 @@ +/** + * Skill Script Loader + * + * Runtime loader for executable skill handlers from backend skill directories. + * Backend skills in `.squad/skills/{name}/scripts/` contain `.js` handler files + * that replace built-in tool handlers in ToolRegistry. + * + * Supports: + * - Loading concern-specific handler scripts (tasks, decisions, memories, logging) + * - Dynamic import() of handler scripts with lifecycle hooks + * - Path containment validation for security + * - Partial implementations (missing handlers are silently skipped) + */ + +import { existsSync } from 'node:fs'; +import * as path from 'node:path'; +import { pathToFileURL } from 'node:url'; +import type { + Concern, + ConcernMap, + LoadResult, + SkillHandler, + HandlerLifecycle, +} from './handler-types.js'; +import type { SquadTool, SquadToolHandler, SquadToolInvocation } from '../adapter/types.js'; + +// --- Concern → Tool Name Mapping --- + +/** + * Maps concerns to their corresponding Squad tool names. + * Tool names follow the convention: squad_{operation} + */ +const CONCERN_TOOL_MAP: Record = { + tasks: ['squad_create_issue', 'squad_update_issue', 'squad_list_issues', 'squad_close_issue'], + decisions: ['squad_create_decision', 'squad_list_decisions', 'squad_merge_decision'], + memories: ['squad_create_memory', 'squad_list_memories'], + logging: ['squad_create_log', 'squad_list_logs'], +}; + +// --- Helpers --- + +/** + * Normalize path separators for consistent module cache keys on Windows. + * pathToFileURL() can create different URLs from different path separator styles, + * leading to duplicate module instances. Always normalize before conversion. + */ +function toFileUrl(filePath: string): string { + const normalized = filePath.replace(/\\/g, '/'); + return pathToFileURL(normalized).href; +} + +/** + * Wrap a SkillHandler (args, config) to SquadToolHandler (args, invocation). + * Bridges the skill script signature to the Squad tool handler signature. + */ +function wrapSkillHandler( + skillHandler: SkillHandler, + backendConfig: Record, +): SquadToolHandler { + return (args: T, _invocation: SquadToolInvocation) => { + return skillHandler(args, backendConfig); + }; +} + +/** + * Resolve a skill path relative to project/team roots with containment validation. + * + * Algorithm: + * 1. Absolute paths used as-is + * 2. With teamRoot: strip leading `.squad/` prefix to avoid double-nesting, resolve relative to teamRoot + * 3. Without teamRoot: resolve relative to projectRoot + * 4. Path containment check: final path must be within projectRoot or teamRoot + * 5. Reject paths with `..` segments that escape the boundary (throw Error) + * + * @param skillPath - Path from skill configuration (absolute or relative) + * @param projectRoot - Project root directory (absolute) + * @param teamRoot - Team root directory (absolute, optional) + * @returns Resolved absolute path + * @throws Error if path escapes containment boundaries + */ +export function resolveSkillPath( + skillPath: string, + projectRoot: string, + teamRoot?: string, +): string { + // 1. Absolute paths used as-is + if (path.isAbsolute(skillPath)) { + const resolved = path.resolve(skillPath); + // Validate containment + if (teamRoot) { + const inTeam = resolved.startsWith(path.resolve(teamRoot) + path.sep) || resolved === path.resolve(teamRoot); + const inProject = resolved.startsWith(path.resolve(projectRoot) + path.sep) || resolved === path.resolve(projectRoot); + if (!inTeam && !inProject) { + throw new Error(`Path escapes containment: ${skillPath} is outside project and team roots`); + } + } else { + const inProject = resolved.startsWith(path.resolve(projectRoot) + path.sep) || resolved === path.resolve(projectRoot); + if (!inProject) { + throw new Error(`Path escapes containment: ${skillPath} is outside project root`); + } + } + return resolved; + } + + // 2. With teamRoot: strip leading .squad/ prefix, resolve relative to teamRoot + if (teamRoot) { + const stripped = skillPath.startsWith('.squad/') ? skillPath.slice(7) : skillPath; + const resolved = path.resolve(teamRoot, stripped); + const inTeam = resolved.startsWith(path.resolve(teamRoot) + path.sep) || resolved === path.resolve(teamRoot); + if (!inTeam) { + throw new Error(`Path escapes containment: ${skillPath} resolves outside team root`); + } + return resolved; + } + + // 3. Without teamRoot: resolve relative to projectRoot + const resolved = path.resolve(projectRoot, skillPath); + const inProject = resolved.startsWith(path.resolve(projectRoot) + path.sep) || resolved === path.resolve(projectRoot); + if (!inProject) { + throw new Error(`Path escapes containment: ${skillPath} resolves outside project root`); + } + return resolved; +} + +// --- SkillScriptLoader --- + +export class SkillScriptLoader { + constructor( + private getToolSchema: (toolName: string) => { description: string; parameters: Record } | undefined, + ) {} + + /** + * Load handler scripts for a specific concern from a backend skill directory. + * + * Algorithm: + * 1. Check for `scripts/` directory — return null if missing (triggers markdown fallback) + * 2. Get tool names for this concern from CONCERN_TOOL_MAP + * 3. For each tool name: + * a. Compute script filename: toolName.replace('squad_', '') + '.js' + * b. Compute full script path + * c. If file doesn't exist → skip (partial implementations are fine) + * d. import() the script using toFileUrl (with Windows path normalization) + * e. Validate: module.default must be a function — if not, THROW (not silent skip) + * f. Get the tool's schema via this.getToolSchema(toolName) + * g. If schema not found → skip with warning (tool not registered yet) + * h. Produce a SquadTool entry with wrapSkillHandler() + * 4. Load scripts/lifecycle.js if present (import() it) + * Extract init and dispose named exports if they are functions + * 5. Return { tools, lifecycle } or { tools } if no lifecycle + * + * @param skillPath - Resolved absolute path to the skill directory + * @param concern - The concern to load handlers for + * @param backendConfig - Backend configuration to pass to handlers + * @returns LoadResult with tools and optional lifecycle, or null if no scripts/ directory + */ + async load( + skillPath: string, + concern: C, + backendConfig: Record, + ): Promise { + // 1. Check for scripts/ directory + const scriptsDir = path.join(skillPath, 'scripts'); + if (!existsSync(scriptsDir)) { + return null; // Triggers markdown fallback + } + + // 2. Get tool names for this concern + const toolNames = CONCERN_TOOL_MAP[concern]; + if (!toolNames) { + throw new Error(`Unknown concern: ${concern}`); + } + + const tools: SquadTool[] = []; + + // 3. Load each tool's handler script + for (const toolName of toolNames) { + // a. Compute script filename + const scriptName = toolName.replace('squad_', '') + '.js'; + const scriptPath = path.join(scriptsDir, scriptName); + + // c. Skip if file doesn't exist + if (!existsSync(scriptPath)) { + continue; + } + + try { + // d. Dynamic import with Windows path normalization + const scriptUrl = toFileUrl(scriptPath); + const module = await import(scriptUrl); + + // e. Validate: module.default must be a function + if (typeof module.default !== 'function') { + throw new Error(`Handler script ${scriptName} does not export a default function`); + } + + // f. Get the tool's schema + const schema = this.getToolSchema(toolName); + if (!schema) { + console.warn(`[SkillScriptLoader] Tool schema not found for ${toolName}, skipping`); + continue; + } + + // h. Create SquadTool entry + const tool: SquadTool = { + name: toolName, + description: schema.description, + parameters: schema.parameters, + handler: wrapSkillHandler(module.default as SkillHandler, backendConfig), + }; + + tools.push(tool); + } catch (err) { + // Failed imports are fatal (validation errors, syntax errors, etc.) + throw new Error(`Failed to load handler script ${scriptName}: ${err instanceof Error ? err.message : String(err)}`); + } + } + + // 4. Load lifecycle.js if present + let lifecycle: HandlerLifecycle | undefined; + const lifecyclePath = path.join(scriptsDir, 'lifecycle.js'); + if (existsSync(lifecyclePath)) { + try { + const lifecycleUrl = toFileUrl(lifecyclePath); + const lifecycleModule = await import(lifecycleUrl); + + // Extract init and dispose named exports if they are functions + const init = typeof lifecycleModule.init === 'function' ? lifecycleModule.init : undefined; + const dispose = typeof lifecycleModule.dispose === 'function' ? lifecycleModule.dispose : undefined; + + if (init || dispose) { + lifecycle = { init, dispose }; + } + } catch (err) { + throw new Error(`Failed to load lifecycle.js: ${err instanceof Error ? err.message : String(err)}`); + } + } + + // 5. Return result + return lifecycle ? { tools, lifecycle } : { tools }; + } +} diff --git a/packages/squad-sdk/src/tools/index.ts b/packages/squad-sdk/src/tools/index.ts index b0b47e74a..1ffc63a87 100644 --- a/packages/squad-sdk/src/tools/index.ts +++ b/packages/squad-sdk/src/tools/index.ts @@ -582,4 +582,19 @@ export class ToolRegistry { getTool(name: string): SquadTool | undefined { return this.tools.get(name); } + + /** + * Replace built-in tool handlers with skill-backed versions. + * Called post-construction after SkillScriptLoader has resolved handlers. + * Only replaces tools that already exist — unknown tool names are silently ignored. + * Once applied, handlers are immutable for the session. + */ + applySkillHandlers(tools: SquadTool[]): void { + for (const tool of tools) { + if (this.tools.has(tool.name)) { + this.tools.set(tool.name, tool); + } + // Unknown tool names silently ignored — skills cannot introduce new tools + } + } } diff --git a/test/skill-script-loader.test.ts b/test/skill-script-loader.test.ts new file mode 100644 index 000000000..11c05256e --- /dev/null +++ b/test/skill-script-loader.test.ts @@ -0,0 +1,580 @@ +/** + * Tests for SkillScriptLoader and ToolRegistry.applySkillHandlers() (M3-3, Issue #141) + * + * FIDO: These tests are being written concurrently with the implementation by CONTROL and EECOM. + * They validate the design spec for script-based skill handlers. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { randomUUID } from 'node:crypto'; +import { pathToFileURL } from 'node:url'; +import { SkillScriptLoader, resolveSkillPath } from '@bradygaster/squad-sdk/skills'; +import type { LoadResult, Concern } from '@bradygaster/squad-sdk/skills'; +import { ToolRegistry } from '@bradygaster/squad-sdk/tools'; + +// --- Test fixtures and helpers --- + +function createTestDir(): string { + const testDir = path.join('test-fixtures', `skill-loader-${randomUUID().slice(0, 8)}`); + fs.mkdirSync(testDir, { recursive: true }); + return testDir; +} + +function createScriptsDir(baseDir: string): string { + const scriptsDir = path.join(baseDir, 'scripts'); + fs.mkdirSync(scriptsDir, { recursive: true }); + return scriptsDir; +} + +function writeValidScript(scriptsDir: string, scriptName: string, returnValue = 'ok'): void { + fs.writeFileSync( + path.join(scriptsDir, scriptName), + `export default async function(args, config) { return { textResultForLlm: "${returnValue}", resultType: "success" }; }`, + 'utf-8', + ); +} + +function writeInvalidScript(scriptsDir: string, scriptName: string, content: string): void { + fs.writeFileSync( + path.join(scriptsDir, scriptName), + content, + 'utf-8', + ); +} + +function writeLifecycleScript(scriptsDir: string, hasInit: boolean, hasDispose: boolean): void { + let content = ''; + if (hasInit) { + content += 'export async function init(config) { /* init logic */ }\n'; + } + if (hasDispose) { + content += 'export async function dispose() { /* dispose logic */ }\n'; + } + fs.writeFileSync(path.join(scriptsDir, 'lifecycle.js'), content, 'utf-8'); +} + +const mockSchema = { + description: 'Test tool', + parameters: { type: 'object' as const, properties: {} }, +}; + +const getSchemaForAll = (name: string) => mockSchema; +const getSchemaForNone = (name: string) => undefined; + +// --- SkillScriptLoader Tests --- + +describe('SkillScriptLoader', () => { + let testDir: string; + + beforeEach(() => { + testDir = createTestDir(); + }); + + afterEach(() => { + fs.rmSync(testDir, { recursive: true, force: true }); + }); + + // --- Null return (markdown fallback) --- + + describe('null return (markdown fallback)', () => { + it('should return null when scripts/ directory does not exist', async () => { + const loader = new SkillScriptLoader(getSchemaForAll); + const result = await loader.load(testDir, 'tasks', {}); + expect(result).toBeNull(); + }); + + it('should return null when skill directory does not exist', async () => { + const loader = new SkillScriptLoader(getSchemaForAll); + const result = await loader.load('/nonexistent/path', 'tasks', {}); + expect(result).toBeNull(); + }); + }); + + // --- Successful load --- + + describe('successful load', () => { + it('should load a single script for a concern', async () => { + const scriptsDir = createScriptsDir(testDir); + writeValidScript(scriptsDir, 'create_issue.js', 'created'); + + const loader = new SkillScriptLoader(getSchemaForAll); + const result = await loader.load(testDir, 'tasks', {}); + + expect(result).not.toBeNull(); + expect(result!.tools).toHaveLength(1); + expect(result!.tools[0].name).toBe('squad_create_issue'); + expect(result!.tools[0].description).toBe('Test tool'); + }); + + it('should handle partial implementation (only some tools have scripts)', async () => { + const scriptsDir = createScriptsDir(testDir); + writeValidScript(scriptsDir, 'create_issue.js'); + writeValidScript(scriptsDir, 'close_issue.js'); + // list_issues.js and update_issue.js are missing + + const loader = new SkillScriptLoader(getSchemaForAll); + const result = await loader.load(testDir, 'tasks', {}); + + expect(result).not.toBeNull(); + expect(result!.tools).toHaveLength(2); + expect(result!.tools.map(t => t.name).sort()).toEqual([ + 'squad_close_issue', + 'squad_create_issue', + ]); + }); + + it('should load multiple scripts for same concern', async () => { + const scriptsDir = createScriptsDir(testDir); + writeValidScript(scriptsDir, 'create_issue.js', 'create_result'); + writeValidScript(scriptsDir, 'update_issue.js', 'update_result'); + writeValidScript(scriptsDir, 'list_issues.js', 'list_result'); + writeValidScript(scriptsDir, 'close_issue.js', 'close_result'); + + const loader = new SkillScriptLoader(getSchemaForAll); + const result = await loader.load(testDir, 'tasks', {}); + + expect(result).not.toBeNull(); + expect(result!.tools).toHaveLength(4); + }); + + it('should pass backendConfig through to handler when called', async () => { + const scriptsDir = createScriptsDir(testDir); + fs.writeFileSync( + path.join(scriptsDir, 'create_issue.js'), + `export default async function(args, config) { + return { + textResultForLlm: config.testKey, + resultType: "success" + }; + }`, + 'utf-8', + ); + + const loader = new SkillScriptLoader(getSchemaForAll); + const result = await loader.load(testDir, 'tasks', { testKey: 'test-value' }); + + expect(result).not.toBeNull(); + expect(result!.tools).toHaveLength(1); + + // Call the handler to verify config is passed + const handler = result!.tools[0].handler; + const output = await handler({ title: 'Test' }, { testKey: 'test-value' }); + expect(output.textResultForLlm).toBe('test-value'); + }); + + it('should return tool name matching the full tool name (squad_* prefix)', async () => { + const scriptsDir = createScriptsDir(testDir); + writeValidScript(scriptsDir, 'create_decision.js'); + + const loader = new SkillScriptLoader(getSchemaForAll); + const result = await loader.load(testDir, 'decisions', {}); + + expect(result).not.toBeNull(); + expect(result!.tools[0].name).toBe('squad_create_decision'); + }); + + it('should handle all concerns correctly', async () => { + const concerns: Concern[] = ['tasks', 'decisions', 'memories', 'logging']; + const scriptNames = { + tasks: 'create_issue.js', + decisions: 'create_decision.js', + memories: 'create_memory.js', + logging: 'create_log.js', + }; + + for (const concern of concerns) { + const concernTestDir = createTestDir(); + const scriptsDir = createScriptsDir(concernTestDir); + writeValidScript(scriptsDir, scriptNames[concern]); + + const loader = new SkillScriptLoader(getSchemaForAll); + const result = await loader.load(concernTestDir, concern, {}); + + expect(result).not.toBeNull(); + expect(result!.tools.length).toBeGreaterThan(0); + + fs.rmSync(concernTestDir, { recursive: true, force: true }); + } + }); + }); + + // --- lifecycle.js --- + + describe('lifecycle.js', () => { + it('should populate lifecycle when lifecycle.js exists', async () => { + const scriptsDir = createScriptsDir(testDir); + writeValidScript(scriptsDir, 'create_issue.js'); + writeLifecycleScript(scriptsDir, true, true); + + const loader = new SkillScriptLoader(getSchemaForAll); + const result = await loader.load(testDir, 'tasks', {}); + + expect(result).not.toBeNull(); + expect(result!.lifecycle).toBeDefined(); + expect(result!.lifecycle!.init).toBeDefined(); + expect(result!.lifecycle!.dispose).toBeDefined(); + }); + + it('should handle lifecycle.js with only init', async () => { + const scriptsDir = createScriptsDir(testDir); + writeValidScript(scriptsDir, 'create_issue.js'); + writeLifecycleScript(scriptsDir, true, false); + + const loader = new SkillScriptLoader(getSchemaForAll); + const result = await loader.load(testDir, 'tasks', {}); + + expect(result).not.toBeNull(); + expect(result!.lifecycle).toBeDefined(); + expect(result!.lifecycle!.init).toBeDefined(); + expect(result!.lifecycle!.dispose).toBeUndefined(); + }); + + it('should handle lifecycle.js with only dispose', async () => { + const scriptsDir = createScriptsDir(testDir); + writeValidScript(scriptsDir, 'create_issue.js'); + writeLifecycleScript(scriptsDir, false, true); + + const loader = new SkillScriptLoader(getSchemaForAll); + const result = await loader.load(testDir, 'tasks', {}); + + expect(result).not.toBeNull(); + expect(result!.lifecycle).toBeDefined(); + expect(result!.lifecycle!.init).toBeUndefined(); + expect(result!.lifecycle!.dispose).toBeDefined(); + }); + + it('should have undefined lifecycle when no lifecycle.js exists', async () => { + const scriptsDir = createScriptsDir(testDir); + writeValidScript(scriptsDir, 'create_issue.js'); + + const loader = new SkillScriptLoader(getSchemaForAll); + const result = await loader.load(testDir, 'tasks', {}); + + expect(result).not.toBeNull(); + expect(result!.lifecycle).toBeUndefined(); + }); + + it('should call init with backendConfig', async () => { + const scriptsDir = createScriptsDir(testDir); + writeValidScript(scriptsDir, 'create_issue.js'); + + // Write lifecycle.js that captures config + fs.writeFileSync( + path.join(scriptsDir, 'lifecycle.js'), + ` + let capturedConfig = null; + export async function init(config) { + capturedConfig = config; + } + export function getConfig() { return capturedConfig; } + `, + 'utf-8', + ); + + const loader = new SkillScriptLoader(getSchemaForAll); + const result = await loader.load(testDir, 'tasks', { testKey: 'init-value' }); + + expect(result).not.toBeNull(); + expect(result!.lifecycle).toBeDefined(); + + // Call init + await result!.lifecycle!.init!({ testKey: 'init-value' }); + + // Verify config was passed (would need to re-import to check, but we've validated the signature) + expect(result!.lifecycle!.init).toBeDefined(); + }); + + it('should call dispose on teardown', async () => { + const scriptsDir = createScriptsDir(testDir); + writeValidScript(scriptsDir, 'create_issue.js'); + writeLifecycleScript(scriptsDir, false, true); + + const loader = new SkillScriptLoader(getSchemaForAll); + const result = await loader.load(testDir, 'tasks', {}); + + expect(result).not.toBeNull(); + expect(result!.lifecycle!.dispose).toBeDefined(); + + // Should not throw when called + await expect(result!.lifecycle!.dispose!()).resolves.not.toThrow(); + }); + }); + + // --- Error cases --- + + describe('error cases', () => { + it('should throw when script exports a non-function default', async () => { + const scriptsDir = createScriptsDir(testDir); + writeInvalidScript(scriptsDir, 'create_issue.js', 'export default "not a function";'); + + const loader = new SkillScriptLoader(getSchemaForAll); + + await expect(loader.load(testDir, 'tasks', {})).rejects.toThrow(); + }); + + it('should throw when script has no default export', async () => { + const scriptsDir = createScriptsDir(testDir); + writeInvalidScript(scriptsDir, 'create_issue.js', 'export const notDefault = () => {};'); + + const loader = new SkillScriptLoader(getSchemaForAll); + + await expect(loader.load(testDir, 'tasks', {})).rejects.toThrow(); + }); + + it('should throw when script has syntax error', async () => { + const scriptsDir = createScriptsDir(testDir); + writeInvalidScript(scriptsDir, 'create_issue.js', 'export default function( { // syntax error'); + + const loader = new SkillScriptLoader(getSchemaForAll); + + await expect(loader.load(testDir, 'tasks', {})).rejects.toThrow(); + }); + }); + + // --- Schema lookup --- + + describe('schema lookup', () => { + it('should skip tool when getToolSchema returns undefined', async () => { + const scriptsDir = createScriptsDir(testDir); + writeValidScript(scriptsDir, 'create_issue.js'); + writeValidScript(scriptsDir, 'update_issue.js'); + + const loader = new SkillScriptLoader(getSchemaForNone); + const result = await loader.load(testDir, 'tasks', {}); + + // No tools should be loaded since all schemas are undefined + expect(result).not.toBeNull(); + expect(result!.tools).toHaveLength(0); + }); + + it('should include all tools when all have schemas', async () => { + const scriptsDir = createScriptsDir(testDir); + writeValidScript(scriptsDir, 'create_issue.js'); + writeValidScript(scriptsDir, 'update_issue.js'); + writeValidScript(scriptsDir, 'list_issues.js'); + + const loader = new SkillScriptLoader(getSchemaForAll); + const result = await loader.load(testDir, 'tasks', {}); + + expect(result).not.toBeNull(); + expect(result!.tools).toHaveLength(3); + }); + + it('should skip only tools without schemas', async () => { + const scriptsDir = createScriptsDir(testDir); + writeValidScript(scriptsDir, 'create_issue.js'); + writeValidScript(scriptsDir, 'update_issue.js'); + + // Only return schema for create_issue + const selectiveSchema = (name: string) => + name === 'squad_create_issue' ? mockSchema : undefined; + + const loader = new SkillScriptLoader(selectiveSchema); + const result = await loader.load(testDir, 'tasks', {}); + + expect(result).not.toBeNull(); + expect(result!.tools).toHaveLength(1); + expect(result!.tools[0].name).toBe('squad_create_issue'); + }); + }); +}); + +// --- ToolRegistry.applySkillHandlers() --- + +describe('ToolRegistry.applySkillHandlers()', () => { + let registry: ToolRegistry; + + beforeEach(() => { + registry = new ToolRegistry(); + }); + + it('should replace an existing tool handler', async () => { + // ToolRegistry pre-registers squad_route tool + const originalTool = registry.getTool('squad_route'); + expect(originalTool).toBeDefined(); + + // Create a replacement handler + const replacementHandler = async (args: any, invocation: any) => ({ + textResultForLlm: 'replaced', + resultType: 'success' as const, + }); + + registry.applySkillHandlers([{ + name: 'squad_route', + description: 'Test tool', + parameters: { type: 'object', properties: {} }, + handler: replacementHandler, + }]); + + // Verify the replacement handler is used + const tool = registry.getTool('squad_route'); + expect(tool).toBeDefined(); + + const result = await tool!.handler({ targetAgent: 'test', task: 'test' }, {} as any); + expect(result.textResultForLlm).toBe('replaced'); + }); + + it('should silently ignore unknown tool names', () => { + const handler = async (args: any, invocation: any) => ({ + textResultForLlm: 'test', + resultType: 'success' as const + }); + + const beforeCount = registry.getTools().length; + + // Should not throw + expect(() => { + registry.applySkillHandlers([{ + name: 'nonexistent_tool', + description: 'Test', + parameters: { type: 'object', properties: {} }, + handler, + }]); + }).not.toThrow(); + + // Tool count should not change + expect(registry.getTools().length).toBe(beforeCount); + }); + + it('should handle empty array as no-op', () => { + const beforeCount = registry.getTools().length; + registry.applySkillHandlers([]); + expect(registry.getTools().length).toBe(beforeCount); + }); + + it('should replace multiple tools in one call', async () => { + // Use existing squad tools: squad_route and squad_decide + const originalRoute = registry.getTool('squad_route'); + const originalDecide = registry.getTool('squad_decide'); + expect(originalRoute).toBeDefined(); + expect(originalDecide).toBeDefined(); + + // Replace both + registry.applySkillHandlers([ + { + name: 'squad_route', + description: 'Squad Route', + parameters: { type: 'object', properties: {} }, + handler: async (args: any, invocation: any) => ({ + textResultForLlm: 'route_replaced', + resultType: 'success' as const + }), + }, + { + name: 'squad_decide', + description: 'Squad Decide', + parameters: { type: 'object', properties: {} }, + handler: async (args: any, invocation: any) => ({ + textResultForLlm: 'decide_replaced', + resultType: 'success' as const + }), + }, + ]); + + // Verify both are replaced + const toolRoute = registry.getTool('squad_route'); + const toolDecide = registry.getTool('squad_decide'); + + const resultRoute = await toolRoute!.handler({ targetAgent: 'test', task: 'test' }, {} as any); + const resultDecide = await toolDecide!.handler({ author: 'test', summary: 'test', body: 'test' }, {} as any); + + expect(resultRoute.textResultForLlm).toBe('route_replaced'); + expect(resultDecide.textResultForLlm).toBe('decide_replaced'); + }); + + it('should only replace tools that exist in registry', async () => { + const beforeCount = registry.getTools().length; + + registry.applySkillHandlers([ + { + name: 'squad_route', + description: 'Existing', + parameters: { type: 'object', properties: {} }, + handler: async (args: any, invocation: any) => ({ + textResultForLlm: 'replaced', + resultType: 'success' as const + }), + }, + { + name: 'nonexistent_tool', + description: 'New', + parameters: { type: 'object', properties: {} }, + handler: async (args: any, invocation: any) => ({ + textResultForLlm: 'new', + resultType: 'success' as const + }), + }, + ]); + + // Count should not increase (nonexistent_tool ignored) + expect(registry.getTools().length).toBe(beforeCount); + + // squad_route should be replaced + const tool = registry.getTool('squad_route'); + const result = await tool!.handler({ targetAgent: 'test', task: 'test' }, {} as any); + expect(result.textResultForLlm).toBe('replaced'); + }); +}); + +// --- resolveSkillPath() --- + +describe('resolveSkillPath()', () => { + // Use real project root for containment validation + const projectRoot = path.resolve('D:\\project'); + const teamRoot = path.resolve('D:\\project\\.squad'); + + it('should return absolute path within projectRoot as-is', () => { + const absolute = path.resolve(projectRoot, 'skills', 'my-skill'); + const result = resolveSkillPath(absolute, projectRoot); + expect(result).toBe(absolute); + }); + + it('should resolve relative path from projectRoot', () => { + const relative = path.join('skills', 'my-skill'); + const result = resolveSkillPath(relative, projectRoot); + expect(result).toBe(path.resolve(projectRoot, 'skills', 'my-skill')); + }); + + it('should resolve relative path from teamRoot when provided', () => { + const relative = path.join('skills', 'my-skill'); + const result = resolveSkillPath(relative, projectRoot, teamRoot); + expect(result).toBe(path.resolve(teamRoot, 'skills', 'my-skill')); + }); + + it('should strip .squad/ prefix when teamRoot is provided', () => { + // Use forward slash format that the implementation checks for + const relative = '.squad/skills/my-skill'; + const result = resolveSkillPath(relative, projectRoot, teamRoot); + // Implementation strips .squad/ and resolves from teamRoot + // So .squad/skills/my-skill becomes teamRoot/skills/my-skill + expect(result).toBe(path.resolve(teamRoot, 'skills', 'my-skill')); + }); + + it('should throw when path with .. escapes projectRoot', () => { + const escaping = path.join('..', '..', 'outside'); + expect(() => { + resolveSkillPath(escaping, projectRoot); + }).toThrow(/escapes containment/); + }); + + it('should throw when path with .. escapes teamRoot', () => { + const escaping = path.join('..', '..', 'outside'); + expect(() => { + resolveSkillPath(escaping, projectRoot, teamRoot); + }).toThrow(/escapes containment/); + }); + + it('should allow .. within bounds', () => { + const withinBounds = path.join('skills', 'subdir', '..', 'my-skill'); + const result = resolveSkillPath(withinBounds, projectRoot); + expect(result).toBe(path.resolve(projectRoot, 'skills', 'my-skill')); + }); + + it('should normalize path separators consistently', () => { + const forward = 'skills/my-skill'; + const result = resolveSkillPath(forward, projectRoot); + expect(result).toBe(path.resolve(projectRoot, 'skills', 'my-skill')); + }); +}); From 669b7a765464e77b14f8a06b39cdf5a079449716 Mon Sep 17 00:00:00 2001 From: Emmitt <1337ERJ1337@gmail.com> Date: Mon, 9 Mar 2026 19:02:22 -0700 Subject: [PATCH 2/3] refactor(skill-script-loader): replace CONCERN_TOOL_MAP with directory scan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop the hardcoded concern-to-tool-name map. SkillScriptLoader.load() now scans scripts/ for .js files and derives tool names from filenames: create_issue.js -> squad_create_issue. The concern parameter is removed from load() — the scripts/ directory is the source of truth. - Remove CONCERN_TOOL_MAP and Concern/ConcernMap imports - load() now uses readdirSync to discover handler files - Tool name derivation: 'squad_' + stem (e.g. create_issue.js -> squad_create_issue) - Update tests: remove concern arg from all .load() calls, simplify multi-concern test to 'discovers scripts for any naming convention' - Update skill-system-design.md: bidirectional convention docs, directory-driven scanning in loader section and startup sequence, doctor validation updated to remove CONCERN_TOOL_MAP reference All 33 skill-script-loader tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/skills/skill-script-loader.ts | 70 ++++++------------- test/skill-script-loader.test.ts | 63 ++++++++--------- 2 files changed, 51 insertions(+), 82 deletions(-) diff --git a/packages/squad-sdk/src/skills/skill-script-loader.ts b/packages/squad-sdk/src/skills/skill-script-loader.ts index 8a98e4928..f873cc6c8 100644 --- a/packages/squad-sdk/src/skills/skill-script-loader.ts +++ b/packages/squad-sdk/src/skills/skill-script-loader.ts @@ -12,31 +12,16 @@ * - Partial implementations (missing handlers are silently skipped) */ -import { existsSync } from 'node:fs'; +import { existsSync, readdirSync } from 'node:fs'; import * as path from 'node:path'; import { pathToFileURL } from 'node:url'; import type { - Concern, - ConcernMap, LoadResult, SkillHandler, HandlerLifecycle, } from './handler-types.js'; import type { SquadTool, SquadToolHandler, SquadToolInvocation } from '../adapter/types.js'; -// --- Concern → Tool Name Mapping --- - -/** - * Maps concerns to their corresponding Squad tool names. - * Tool names follow the convention: squad_{operation} - */ -const CONCERN_TOOL_MAP: Record = { - tasks: ['squad_create_issue', 'squad_update_issue', 'squad_list_issues', 'squad_close_issue'], - decisions: ['squad_create_decision', 'squad_list_decisions', 'squad_merge_decision'], - memories: ['squad_create_memory', 'squad_list_memories'], - logging: ['squad_create_log', 'squad_list_logs'], -}; - // --- Helpers --- /** @@ -130,32 +115,27 @@ export class SkillScriptLoader { ) {} /** - * Load handler scripts for a specific concern from a backend skill directory. + * Load handler scripts from a backend skill directory by scanning `scripts/` for `.js` files. * * Algorithm: * 1. Check for `scripts/` directory — return null if missing (triggers markdown fallback) - * 2. Get tool names for this concern from CONCERN_TOOL_MAP - * 3. For each tool name: - * a. Compute script filename: toolName.replace('squad_', '') + '.js' - * b. Compute full script path - * c. If file doesn't exist → skip (partial implementations are fine) - * d. import() the script using toFileUrl (with Windows path normalization) - * e. Validate: module.default must be a function — if not, THROW (not silent skip) - * f. Get the tool's schema via this.getToolSchema(toolName) - * g. If schema not found → skip with warning (tool not registered yet) - * h. Produce a SquadTool entry with wrapSkillHandler() + * 2. Scan scripts/ for all .js files (excluding lifecycle.js) + * 3. For each file, derive tool name: prepend 'squad_' to the filename stem + * a. import() the script using toFileUrl (with Windows path normalization) + * b. Validate: module.default must be a function — if not, THROW (not silent skip) + * c. Get the tool's schema via this.getToolSchema(toolName) + * d. If schema not found → skip with warning (tool not registered in ToolRegistry) + * e. Produce a SquadTool entry with wrapSkillHandler() * 4. Load scripts/lifecycle.js if present (import() it) * Extract init and dispose named exports if they are functions * 5. Return { tools, lifecycle } or { tools } if no lifecycle * * @param skillPath - Resolved absolute path to the skill directory - * @param concern - The concern to load handlers for * @param backendConfig - Backend configuration to pass to handlers * @returns LoadResult with tools and optional lifecycle, or null if no scripts/ directory */ - async load( + async load( skillPath: string, - concern: C, backendConfig: Record, ): Promise { // 1. Check for scripts/ directory @@ -164,43 +144,37 @@ export class SkillScriptLoader { return null; // Triggers markdown fallback } - // 2. Get tool names for this concern - const toolNames = CONCERN_TOOL_MAP[concern]; - if (!toolNames) { - throw new Error(`Unknown concern: ${concern}`); - } + // 2. Scan scripts/ for handler files — everything except lifecycle.js + const scriptFiles = readdirSync(scriptsDir).filter( + (f) => f.endsWith('.js') && f !== 'lifecycle.js', + ); const tools: SquadTool[] = []; - // 3. Load each tool's handler script - for (const toolName of toolNames) { - // a. Compute script filename - const scriptName = toolName.replace('squad_', '') + '.js'; + // 3. Load each discovered handler script + for (const scriptName of scriptFiles) { + // Derive tool name from filename: create_issue.js → squad_create_issue + const toolName = 'squad_' + scriptName.slice(0, -3); const scriptPath = path.join(scriptsDir, scriptName); - // c. Skip if file doesn't exist - if (!existsSync(scriptPath)) { - continue; - } - try { - // d. Dynamic import with Windows path normalization + // a. Dynamic import with Windows path normalization const scriptUrl = toFileUrl(scriptPath); const module = await import(scriptUrl); - // e. Validate: module.default must be a function + // b. Validate: module.default must be a function if (typeof module.default !== 'function') { throw new Error(`Handler script ${scriptName} does not export a default function`); } - // f. Get the tool's schema + // c. Get the tool's schema const schema = this.getToolSchema(toolName); if (!schema) { console.warn(`[SkillScriptLoader] Tool schema not found for ${toolName}, skipping`); continue; } - // h. Create SquadTool entry + // e. Create SquadTool entry const tool: SquadTool = { name: toolName, description: schema.description, diff --git a/test/skill-script-loader.test.ts b/test/skill-script-loader.test.ts index 11c05256e..de376c2dc 100644 --- a/test/skill-script-loader.test.ts +++ b/test/skill-script-loader.test.ts @@ -11,7 +11,7 @@ import * as path from 'node:path'; import { randomUUID } from 'node:crypto'; import { pathToFileURL } from 'node:url'; import { SkillScriptLoader, resolveSkillPath } from '@bradygaster/squad-sdk/skills'; -import type { LoadResult, Concern } from '@bradygaster/squad-sdk/skills'; +import type { LoadResult } from '@bradygaster/squad-sdk/skills'; import { ToolRegistry } from '@bradygaster/squad-sdk/tools'; // --- Test fixtures and helpers --- @@ -81,13 +81,13 @@ describe('SkillScriptLoader', () => { describe('null return (markdown fallback)', () => { it('should return null when scripts/ directory does not exist', async () => { const loader = new SkillScriptLoader(getSchemaForAll); - const result = await loader.load(testDir, 'tasks', {}); + const result = await loader.load(testDir, {}); expect(result).toBeNull(); }); it('should return null when skill directory does not exist', async () => { const loader = new SkillScriptLoader(getSchemaForAll); - const result = await loader.load('/nonexistent/path', 'tasks', {}); + const result = await loader.load('/nonexistent/path', {}); expect(result).toBeNull(); }); }); @@ -100,7 +100,7 @@ describe('SkillScriptLoader', () => { writeValidScript(scriptsDir, 'create_issue.js', 'created'); const loader = new SkillScriptLoader(getSchemaForAll); - const result = await loader.load(testDir, 'tasks', {}); + const result = await loader.load(testDir, {}); expect(result).not.toBeNull(); expect(result!.tools).toHaveLength(1); @@ -115,7 +115,7 @@ describe('SkillScriptLoader', () => { // list_issues.js and update_issue.js are missing const loader = new SkillScriptLoader(getSchemaForAll); - const result = await loader.load(testDir, 'tasks', {}); + const result = await loader.load(testDir, {}); expect(result).not.toBeNull(); expect(result!.tools).toHaveLength(2); @@ -133,7 +133,7 @@ describe('SkillScriptLoader', () => { writeValidScript(scriptsDir, 'close_issue.js', 'close_result'); const loader = new SkillScriptLoader(getSchemaForAll); - const result = await loader.load(testDir, 'tasks', {}); + const result = await loader.load(testDir, {}); expect(result).not.toBeNull(); expect(result!.tools).toHaveLength(4); @@ -153,7 +153,7 @@ describe('SkillScriptLoader', () => { ); const loader = new SkillScriptLoader(getSchemaForAll); - const result = await loader.load(testDir, 'tasks', { testKey: 'test-value' }); + const result = await loader.load(testDir, { testKey: 'test-value' }); expect(result).not.toBeNull(); expect(result!.tools).toHaveLength(1); @@ -169,33 +169,27 @@ describe('SkillScriptLoader', () => { writeValidScript(scriptsDir, 'create_decision.js'); const loader = new SkillScriptLoader(getSchemaForAll); - const result = await loader.load(testDir, 'decisions', {}); + const result = await loader.load(testDir, {}); expect(result).not.toBeNull(); expect(result!.tools[0].name).toBe('squad_create_decision'); }); - it('should handle all concerns correctly', async () => { - const concerns: Concern[] = ['tasks', 'decisions', 'memories', 'logging']; - const scriptNames = { - tasks: 'create_issue.js', - decisions: 'create_decision.js', - memories: 'create_memory.js', - logging: 'create_log.js', - }; + it('should discover scripts for any tool naming convention', async () => { + const scriptNames = ['create_issue.js', 'create_decision.js', 'create_memory.js', 'create_log.js']; - for (const concern of concerns) { - const concernTestDir = createTestDir(); - const scriptsDir = createScriptsDir(concernTestDir); - writeValidScript(scriptsDir, scriptNames[concern]); + for (const scriptName of scriptNames) { + const scriptTestDir = createTestDir(); + const scriptsDir = createScriptsDir(scriptTestDir); + writeValidScript(scriptsDir, scriptName); const loader = new SkillScriptLoader(getSchemaForAll); - const result = await loader.load(concernTestDir, concern, {}); + const result = await loader.load(scriptTestDir, {}); expect(result).not.toBeNull(); expect(result!.tools.length).toBeGreaterThan(0); - fs.rmSync(concernTestDir, { recursive: true, force: true }); + fs.rmSync(scriptTestDir, { recursive: true, force: true }); } }); }); @@ -209,7 +203,7 @@ describe('SkillScriptLoader', () => { writeLifecycleScript(scriptsDir, true, true); const loader = new SkillScriptLoader(getSchemaForAll); - const result = await loader.load(testDir, 'tasks', {}); + const result = await loader.load(testDir, {}); expect(result).not.toBeNull(); expect(result!.lifecycle).toBeDefined(); @@ -223,7 +217,7 @@ describe('SkillScriptLoader', () => { writeLifecycleScript(scriptsDir, true, false); const loader = new SkillScriptLoader(getSchemaForAll); - const result = await loader.load(testDir, 'tasks', {}); + const result = await loader.load(testDir, {}); expect(result).not.toBeNull(); expect(result!.lifecycle).toBeDefined(); @@ -237,7 +231,7 @@ describe('SkillScriptLoader', () => { writeLifecycleScript(scriptsDir, false, true); const loader = new SkillScriptLoader(getSchemaForAll); - const result = await loader.load(testDir, 'tasks', {}); + const result = await loader.load(testDir, {}); expect(result).not.toBeNull(); expect(result!.lifecycle).toBeDefined(); @@ -250,7 +244,7 @@ describe('SkillScriptLoader', () => { writeValidScript(scriptsDir, 'create_issue.js'); const loader = new SkillScriptLoader(getSchemaForAll); - const result = await loader.load(testDir, 'tasks', {}); + const result = await loader.load(testDir, {}); expect(result).not.toBeNull(); expect(result!.lifecycle).toBeUndefined(); @@ -274,7 +268,7 @@ describe('SkillScriptLoader', () => { ); const loader = new SkillScriptLoader(getSchemaForAll); - const result = await loader.load(testDir, 'tasks', { testKey: 'init-value' }); + const result = await loader.load(testDir, { testKey: 'init-value' }); expect(result).not.toBeNull(); expect(result!.lifecycle).toBeDefined(); @@ -292,7 +286,7 @@ describe('SkillScriptLoader', () => { writeLifecycleScript(scriptsDir, false, true); const loader = new SkillScriptLoader(getSchemaForAll); - const result = await loader.load(testDir, 'tasks', {}); + const result = await loader.load(testDir, {}); expect(result).not.toBeNull(); expect(result!.lifecycle!.dispose).toBeDefined(); @@ -311,7 +305,7 @@ describe('SkillScriptLoader', () => { const loader = new SkillScriptLoader(getSchemaForAll); - await expect(loader.load(testDir, 'tasks', {})).rejects.toThrow(); + await expect(loader.load(testDir, {})).rejects.toThrow(); }); it('should throw when script has no default export', async () => { @@ -320,7 +314,7 @@ describe('SkillScriptLoader', () => { const loader = new SkillScriptLoader(getSchemaForAll); - await expect(loader.load(testDir, 'tasks', {})).rejects.toThrow(); + await expect(loader.load(testDir, {})).rejects.toThrow(); }); it('should throw when script has syntax error', async () => { @@ -329,7 +323,7 @@ describe('SkillScriptLoader', () => { const loader = new SkillScriptLoader(getSchemaForAll); - await expect(loader.load(testDir, 'tasks', {})).rejects.toThrow(); + await expect(loader.load(testDir, {})).rejects.toThrow(); }); }); @@ -342,7 +336,7 @@ describe('SkillScriptLoader', () => { writeValidScript(scriptsDir, 'update_issue.js'); const loader = new SkillScriptLoader(getSchemaForNone); - const result = await loader.load(testDir, 'tasks', {}); + const result = await loader.load(testDir, {}); // No tools should be loaded since all schemas are undefined expect(result).not.toBeNull(); @@ -356,7 +350,7 @@ describe('SkillScriptLoader', () => { writeValidScript(scriptsDir, 'list_issues.js'); const loader = new SkillScriptLoader(getSchemaForAll); - const result = await loader.load(testDir, 'tasks', {}); + const result = await loader.load(testDir, {}); expect(result).not.toBeNull(); expect(result!.tools).toHaveLength(3); @@ -372,7 +366,7 @@ describe('SkillScriptLoader', () => { name === 'squad_create_issue' ? mockSchema : undefined; const loader = new SkillScriptLoader(selectiveSchema); - const result = await loader.load(testDir, 'tasks', {}); + const result = await loader.load(testDir, {}); expect(result).not.toBeNull(); expect(result!.tools).toHaveLength(1); @@ -578,3 +572,4 @@ describe('resolveSkillPath()', () => { expect(result).toBe(path.resolve(projectRoot, 'skills', 'my-skill')); }); }); + From c9a41c5396a096843170a3c677d5b3c34de48f7d Mon Sep 17 00:00:00 2001 From: Emmitt <1337ERJ1337@gmail.com> Date: Tue, 10 Mar 2026 08:12:36 -0700 Subject: [PATCH 3/3] feat(skills): OTel instrumentation for skill handlers, resolveSkillPath DRY refactor, backendConfig security docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add wrapSkillHandlerWithSpan() in skill-script-loader.ts — tools exit the loader already instrumented; closes the 'Known Limitation: OTel bypass' on applySkillHandlers. Uses squad.skill.* span namespace (squad.skill.call / squad.skill.result / squad.skill.error) to distinguish from squad.tool.* spans emitted by defineTool(). - Revert tools/index.ts OTel to original inline form — skills and tools have different signatures and concerns; implementations are intentionally separate. - Update applySkillHandlers JSDoc — no more Known Limitation comment. - Refactor resolveSkillPath — extract realOrLogical() and isContained() inner helpers to remove duplicated realpathSync+fallback and containment logic across the three path branches. - Update backendConfig @warning in skill-script-loader.ts and handler-types.ts — clarifies this config is committed to the repo; do not store credentials, tokens, or secrets here. --- .../squad-sdk/src/skills/handler-types.ts | 5 + .../src/skills/skill-script-loader.ts | 129 +++++++++++------- packages/squad-sdk/src/tools/index.ts | 34 ++++- test/consumer-imports.test.ts | 10 ++ test/skill-script-loader.test.ts | 83 +++++++++-- 5 files changed, 194 insertions(+), 67 deletions(-) diff --git a/packages/squad-sdk/src/skills/handler-types.ts b/packages/squad-sdk/src/skills/handler-types.ts index f21f2bd4f..d3ee843ee 100644 --- a/packages/squad-sdk/src/skills/handler-types.ts +++ b/packages/squad-sdk/src/skills/handler-types.ts @@ -270,6 +270,11 @@ export interface LoadResult { * * Specifies a skill directory path and optional disposal timeout. * The 'package' key is reserved for future use and forbidden here. + * + * @warning **Security note:** `backendConfig` is for non-secret runtime configuration (URLs, feature + * flags, timeouts). Do NOT put credentials, tokens, or secrets in `backendConfig` — this config is + * part of the skill definition and will be committed to the repository. Handler scripts run with full + * process trust and can access the filesystem and the network. Only load skills from trusted sources. */ export interface SkillConfig { /** Path to skill directory (relative to squad root) */ diff --git a/packages/squad-sdk/src/skills/skill-script-loader.ts b/packages/squad-sdk/src/skills/skill-script-loader.ts index f873cc6c8..27f4e0748 100644 --- a/packages/squad-sdk/src/skills/skill-script-loader.ts +++ b/packages/squad-sdk/src/skills/skill-script-loader.ts @@ -12,15 +12,62 @@ * - Partial implementations (missing handlers are silently skipped) */ -import { existsSync, readdirSync } from 'node:fs'; +import { existsSync, readdirSync, realpathSync } from 'node:fs'; import * as path from 'node:path'; import { pathToFileURL } from 'node:url'; +import { trace, SpanStatusCode } from '../runtime/otel-api.js'; import type { LoadResult, SkillHandler, HandlerLifecycle, } from './handler-types.js'; -import type { SquadTool, SquadToolHandler, SquadToolInvocation } from '../adapter/types.js'; +import type { SquadTool, SquadToolHandler, SquadToolResult } from '../adapter/types.js'; + +const tracer = trace.getTracer('squad-sdk'); + +// --- OTel Handler Wrapping --- + +/** + * Wrap a skill handler with OTel span instrumentation. + * Mirrors the wrapping applied to built-in tools by defineTool() so skill-dispatched + * calls appear in traces, distinguished by 'tool.skill_dispatched: true'. + * + * This is intentionally a local copy — skill handlers and tool handlers have different + * signatures and concerns; they should not share implementation. + */ +function wrapSkillHandlerWithSpan( + name: string, + skillHandler: SkillHandler, + backendConfig: Record, +): SquadToolHandler { + return async (args, _invocation) => { + const span = tracer.startSpan('squad.skill.call', { + attributes: { 'tool.name': name, 'tool.skill_dispatched': true }, + }); + const startTime = Date.now(); + try { + const result = await skillHandler(args, backendConfig) as SquadToolResult; + const durationMs = Date.now() - startTime; + span.addEvent('squad.skill.result', typeof result === 'string' + ? { 'result.type': 'unknown', 'result.length': result.length, 'duration_ms': durationMs, 'success': true } + : { 'result.type': result.resultType ?? 'unknown', 'result.length': (result.textResultForLlm ?? '').length, 'duration_ms': durationMs, 'success': result.resultType !== 'failure' }, + ); + return result; + } catch (err) { + const durationMs = Date.now() - startTime; + span.setStatus({ code: SpanStatusCode.ERROR, message: err instanceof Error ? err.message : String(err) }); + span.addEvent('squad.skill.error', { + 'error.type': err instanceof Error ? err.constructor.name : 'unknown', + 'error.message': err instanceof Error ? err.message : String(err), + 'duration_ms': durationMs, + }); + span.recordException(err instanceof Error ? err : new Error(String(err))); + throw err; + } finally { + span.end(); + } + }; +} // --- Helpers --- @@ -34,19 +81,6 @@ function toFileUrl(filePath: string): string { return pathToFileURL(normalized).href; } -/** - * Wrap a SkillHandler (args, config) to SquadToolHandler (args, invocation). - * Bridges the skill script signature to the Squad tool handler signature. - */ -function wrapSkillHandler( - skillHandler: SkillHandler, - backendConfig: Record, -): SquadToolHandler { - return (args: T, _invocation: SquadToolInvocation) => { - return skillHandler(args, backendConfig); - }; -} - /** * Resolve a skill path relative to project/team roots with containment validation. * @@ -68,22 +102,23 @@ export function resolveSkillPath( projectRoot: string, teamRoot?: string, ): string { + /** Resolve symlinks; fall back to the logical path if it doesn't exist yet (write case). */ + function realOrLogical(p: string): string { + try { return realpathSync(p); } catch { return p; } + } + + /** True if `p` is equal to `root` or directly inside it. */ + function isContained(p: string, root: string): boolean { + const r = path.resolve(root); + return p === r || p.startsWith(r + path.sep); + } + // 1. Absolute paths used as-is if (path.isAbsolute(skillPath)) { const resolved = path.resolve(skillPath); - // Validate containment - if (teamRoot) { - const inTeam = resolved.startsWith(path.resolve(teamRoot) + path.sep) || resolved === path.resolve(teamRoot); - const inProject = resolved.startsWith(path.resolve(projectRoot) + path.sep) || resolved === path.resolve(projectRoot); - if (!inTeam && !inProject) { - throw new Error(`Path escapes containment: ${skillPath} is outside project and team roots`); - } - } else { - const inProject = resolved.startsWith(path.resolve(projectRoot) + path.sep) || resolved === path.resolve(projectRoot); - if (!inProject) { - throw new Error(`Path escapes containment: ${skillPath} is outside project root`); - } - } + const real = realOrLogical(resolved); + const inside = (teamRoot ? isContained(real, teamRoot) || isContained(real, projectRoot) : isContained(real, projectRoot)); + if (!inside) throw new Error(`Path escapes containment: ${skillPath} is outside project and team roots`); return resolved; } @@ -91,19 +126,15 @@ export function resolveSkillPath( if (teamRoot) { const stripped = skillPath.startsWith('.squad/') ? skillPath.slice(7) : skillPath; const resolved = path.resolve(teamRoot, stripped); - const inTeam = resolved.startsWith(path.resolve(teamRoot) + path.sep) || resolved === path.resolve(teamRoot); - if (!inTeam) { - throw new Error(`Path escapes containment: ${skillPath} resolves outside team root`); - } + const real = realOrLogical(resolved); + if (!isContained(real, teamRoot)) throw new Error(`Path escapes containment: ${skillPath} resolves outside team root`); return resolved; } // 3. Without teamRoot: resolve relative to projectRoot const resolved = path.resolve(projectRoot, skillPath); - const inProject = resolved.startsWith(path.resolve(projectRoot) + path.sep) || resolved === path.resolve(projectRoot); - if (!inProject) { - throw new Error(`Path escapes containment: ${skillPath} resolves outside project root`); - } + const real = realOrLogical(resolved); + if (!isContained(real, projectRoot)) throw new Error(`Path escapes containment: ${skillPath} resolves outside project root`); return resolved; } @@ -133,6 +164,11 @@ export class SkillScriptLoader { * @param skillPath - Resolved absolute path to the skill directory * @param backendConfig - Backend configuration to pass to handlers * @returns LoadResult with tools and optional lifecycle, or null if no scripts/ directory + * + * @warning **Security note:** `backendConfig` is for non-secret runtime configuration (URLs, feature + * flags, timeouts). Do NOT put credentials, tokens, or secrets in `backendConfig` — this config is + * part of the skill definition and will be committed to the repository. Handler scripts run with full + * process trust and can access the filesystem and the network. Only load skills from trusted sources. */ async load( skillPath: string, @@ -157,8 +193,15 @@ export class SkillScriptLoader { const toolName = 'squad_' + scriptName.slice(0, -3); const scriptPath = path.join(scriptsDir, scriptName); + // c. Get the tool's schema — outside the import try block so schema errors are attributed correctly + const schema = this.getToolSchema(toolName); + if (!schema) { + console.warn(`[SkillScriptLoader] Tool schema not found for ${toolName}, skipping`); + continue; + } + + // d. Dynamic import — errors here mean the script itself is broken try { - // a. Dynamic import with Windows path normalization const scriptUrl = toFileUrl(scriptPath); const module = await import(scriptUrl); @@ -167,19 +210,13 @@ export class SkillScriptLoader { throw new Error(`Handler script ${scriptName} does not export a default function`); } - // c. Get the tool's schema - const schema = this.getToolSchema(toolName); - if (!schema) { - console.warn(`[SkillScriptLoader] Tool schema not found for ${toolName}, skipping`); - continue; - } - - // e. Create SquadTool entry + // e. Create SquadTool entry — wrap with OTel span instrumentation at load time + // so skill-dispatched calls appear in traces with 'tool.skill_dispatched: true'. const tool: SquadTool = { name: toolName, description: schema.description, parameters: schema.parameters, - handler: wrapSkillHandler(module.default as SkillHandler, backendConfig), + handler: wrapSkillHandlerWithSpan(toolName, module.default as SkillHandler, backendConfig), }; tools.push(tool); diff --git a/packages/squad-sdk/src/tools/index.ts b/packages/squad-sdk/src/tools/index.ts index 1ffc63a87..cc9180943 100644 --- a/packages/squad-sdk/src/tools/index.ts +++ b/packages/squad-sdk/src/tools/index.ts @@ -156,6 +156,17 @@ export function defineTool(config: { }; } +// --- Error Sanitization --- + +/** + * Sanitize error messages before sending to LLM. + * Strips absolute filesystem paths by replacing the squadRoot prefix with [team-root]. + */ +function sanitizeErrorForLlm(error: unknown, squadRoot: string): string { + const msg = error instanceof Error ? error.message : String(error); + return msg.replace(new RegExp(squadRoot.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), 'g'), '[team-root]'); +} + // --- Tool Registry --- export class ToolRegistry { @@ -252,6 +263,9 @@ export class ToolRegistry { required: ['author', 'summary', 'body'], }, handler: async (args) => { + if (!/^[a-zA-Z0-9_-]+$/.test(args.author)) { + return { textResultForLlm: 'Invalid author name: must contain only letters, numbers, hyphens, and underscores', resultType: 'failure', error: 'Invalid author' }; + } try { const inboxDir = path.join(this.squadRoot, 'decisions', 'inbox'); fs.mkdirSync(inboxDir, { recursive: true }); @@ -281,13 +295,13 @@ export class ToolRegistry { fs.writeFileSync(filename, content, 'utf-8'); return { - textResultForLlm: `Decision written to ${filename} (ID: ${decisionId})`, + textResultForLlm: `Decision written: ${args.author}-${slug}.md (ID: ${decisionId})`, resultType: 'success', toolTelemetry: { decisionId, filename, slug }, }; } catch (error) { return { - textResultForLlm: `Failed to write decision: ${error}`, + textResultForLlm: `Failed to write decision: ${sanitizeErrorForLlm(error, this.squadRoot)}`, resultType: 'failure', error: String(error), }; @@ -319,12 +333,15 @@ export class ToolRegistry { required: ['agent', 'section', 'content'], }, handler: async (args) => { + if (!/^[a-zA-Z0-9_-]+$/.test(args.agent)) { + return { textResultForLlm: 'Invalid agent name: must contain only letters, numbers, hyphens, and underscores', resultType: 'failure', error: 'Invalid agent name' }; + } try { const historyFile = path.join(this.squadRoot, 'agents', args.agent, 'history.md'); if (!fs.existsSync(historyFile)) { return { - textResultForLlm: `Agent history file not found: ${historyFile}`, + textResultForLlm: `Agent history file not found: agents/${args.agent}/history.md`, resultType: 'failure', error: 'History file does not exist', }; @@ -357,7 +374,7 @@ export class ToolRegistry { }; } catch (error) { return { - textResultForLlm: `Failed to update agent memory: ${error}`, + textResultForLlm: `Failed to update agent memory: ${sanitizeErrorForLlm(error, this.squadRoot)}`, resultType: 'failure', error: String(error), }; @@ -499,6 +516,9 @@ export class ToolRegistry { required: ['skillName', 'operation'], }, handler: async (args) => { + if (!/^[a-zA-Z0-9_-]+$/.test(args.skillName)) { + return { textResultForLlm: 'Invalid skill name: must contain only letters, numbers, hyphens, and underscores', resultType: 'failure', error: 'Invalid skillName' }; + } try { const skillDir = path.join(this.squadRoot, 'skills', args.skillName); const skillFile = path.join(skillDir, 'SKILL.md'); @@ -542,14 +562,14 @@ export class ToolRegistry { fs.writeFileSync(skillFile, skillContent, 'utf-8'); return { - textResultForLlm: `Skill written: ${args.skillName} (${skillFile})`, + textResultForLlm: `Skill written: ${args.skillName} (skills/${args.skillName}/SKILL.md)`, resultType: 'success', toolTelemetry: { skillName: args.skillName, operation: 'write', confidence: args.confidence }, }; } } catch (error) { return { - textResultForLlm: `Failed to ${args.operation} skill: ${error}`, + textResultForLlm: `Failed to ${args.operation} skill: ${sanitizeErrorForLlm(error, this.squadRoot)}`, resultType: 'failure', error: String(error), }; @@ -588,6 +608,8 @@ export class ToolRegistry { * Called post-construction after SkillScriptLoader has resolved handlers. * Only replaces tools that already exist — unknown tool names are silently ignored. * Once applied, handlers are immutable for the session. + * + * Skill handlers are already OTel-wrapped by SkillScriptLoader.load() — no re-wrapping here. */ applySkillHandlers(tools: SquadTool[]): void { for (const tool of tools) { diff --git a/test/consumer-imports.test.ts b/test/consumer-imports.test.ts index bd6f69600..f0a175890 100644 --- a/test/consumer-imports.test.ts +++ b/test/consumer-imports.test.ts @@ -52,4 +52,14 @@ describe('Consumer-perspective imports', () => { // The test completing without hanging or exiting proves the import is clean }); }); + + describe('skills barrel exports', () => { + it('exports SkillScriptLoader, resolveSkillPath, and defineHandler from skills barrel', async () => { + const { SkillScriptLoader, resolveSkillPath, defineHandler } = + await import('@bradygaster/squad-sdk/skills'); + expect(typeof SkillScriptLoader).toBe('function'); + expect(typeof resolveSkillPath).toBe('function'); + expect(typeof defineHandler).toBe('function'); + }); + }); }); diff --git a/test/skill-script-loader.test.ts b/test/skill-script-loader.test.ts index de376c2dc..65e590a7e 100644 --- a/test/skill-script-loader.test.ts +++ b/test/skill-script-loader.test.ts @@ -92,6 +92,57 @@ describe('SkillScriptLoader', () => { }); }); + // --- Non-null return (empty-tools contract) --- + + describe('non-null return (empty-tools contract)', () => { + // CONTRACT: non-null even when empty — callers must check tools.length, not null, + // for markdown fallback logic. null is ONLY returned when scripts/ is absent. + + it('should return { tools: [] } (not null) when scripts/ exists but contains only .ts files', async () => { + const scriptsDir = createScriptsDir(testDir); + // .ts file present (developer forgot to compile) — no .js handler produced + fs.writeFileSync( + path.join(scriptsDir, 'create_issue.ts'), + 'export default async function(args: unknown, config: unknown) {}', + 'utf-8', + ); + + const loader = new SkillScriptLoader(getSchemaForAll); + const result = await loader.load(testDir, {}); + + // scripts/ is present so result is NOT null — callers must check tools.length for fallback + expect(result).not.toBeNull(); + expect(result!.tools).toHaveLength(0); + }); + + it('should return { tools: [], lifecycle } (not null) when scripts/ contains only lifecycle.js', async () => { + const scriptsDir = createScriptsDir(testDir); + // lifecycle.js is excluded from handler scanning — no handler .js files present + writeLifecycleScript(scriptsDir, true, true); + + const loader = new SkillScriptLoader(getSchemaForAll); + const result = await loader.load(testDir, {}); + + // scripts/ is present so result is NOT null — callers must check tools.length for fallback + expect(result).not.toBeNull(); + expect(result!.tools).toHaveLength(0); + // lifecycle is still defined since lifecycle.js was loaded + expect(result!.lifecycle).toBeDefined(); + }); + + it('should return { tools: [] } (not null) when scripts/ directory is completely empty', async () => { + // Create scripts/ dir but write no files at all + createScriptsDir(testDir); + + const loader = new SkillScriptLoader(getSchemaForAll); + const result = await loader.load(testDir, {}); + + // scripts/ is present so result is NOT null — callers must check tools.length for fallback + expect(result).not.toBeNull(); + expect(result!.tools).toHaveLength(0); + }); + }); + // --- Successful load --- describe('successful load', () => { @@ -253,31 +304,33 @@ describe('SkillScriptLoader', () => { it('should call init with backendConfig', async () => { const scriptsDir = createScriptsDir(testDir); writeValidScript(scriptsDir, 'create_issue.js'); - - // Write lifecycle.js that captures config + + // Use a temp file to capture config across the ESM module boundary. + // Module-level variables won't work because Node caches the module between + // test runs; writing to disk is the reliable cross-process capture pattern. + const capturePath = path.join(testDir, `init-capture-${randomUUID()}.json`); + fs.writeFileSync( path.join(scriptsDir, 'lifecycle.js'), - ` - let capturedConfig = null; - export async function init(config) { - capturedConfig = config; - } - export function getConfig() { return capturedConfig; } - `, + `import { writeFileSync } from 'node:fs'; +export async function init(config) { + writeFileSync(${JSON.stringify(capturePath)}, JSON.stringify(config), 'utf-8'); +}`, 'utf-8', ); const loader = new SkillScriptLoader(getSchemaForAll); - const result = await loader.load(testDir, { testKey: 'init-value' }); + const result = await loader.load(testDir, {}); expect(result).not.toBeNull(); expect(result!.lifecycle).toBeDefined(); - - // Call init - await result!.lifecycle!.init!({ testKey: 'init-value' }); - - // Verify config was passed (would need to re-import to check, but we've validated the signature) expect(result!.lifecycle!.init).toBeDefined(); + + // Call init and verify the config was actually received + await result!.lifecycle!.init!({ testKey: 'test-value-123' }); + + const captured = JSON.parse(fs.readFileSync(capturePath, 'utf-8')); + expect(captured).toEqual({ testKey: 'test-value-123' }); }); it('should call dispose on teardown', async () => {