diff --git a/package.json b/package.json index 8661cd3..41d3c94 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "dist", "!dist/voice", "!dist/composio", + "spec", "README.md", "LICENSE" ], @@ -43,7 +44,7 @@ "build": "tsc", "dev": "tsc --watch", "start": "node dist/index.js", - "test": "node --test test/*.test.ts --experimental-strip-types" + "test": "node --experimental-strip-types --experimental-loader=./test/ts-resolve-hook.mjs --no-warnings --test test/*.test.ts" }, "engines": { "node": ">=20" diff --git a/spec/schemas/workflow.schema.json b/spec/schemas/workflow.schema.json new file mode 100644 index 0000000..1ab34ea --- /dev/null +++ b/spec/schemas/workflow.schema.json @@ -0,0 +1,69 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "$id": "https://gitagent.dev/spec/workflow.schema.json", + "title": "Gitagent SkillFlow Workflow", + "description": "A SkillFlow workflow: a named sequence of steps, where each step invokes a skill with a prompt. Runtime semantics are defined by src/workflows.ts.", + "type": "object", + "additionalProperties": false, + "required": ["name", "description", "steps"], + "properties": { + "name": { + "type": "string", + "description": "Kebab-case identifier for the workflow. Used as the file name.", + "pattern": "^[a-z0-9]+(-[a-z0-9]+)*$" + }, + "description": { + "type": "string", + "description": "One-line description of what the workflow does.", + "minLength": 1 + }, + "steps": { + "type": "array", + "description": "Ordered list of steps. Steps execute top-to-bottom.", + "minItems": 1, + "items": { "$ref": "#/definitions/step" } + } + }, + "definitions": { + "step": { + "type": "object", + "additionalProperties": false, + "required": ["skill", "prompt"], + "properties": { + "id": { + "type": "string", + "description": "Optional snake_case identifier for the step. Used when other steps reference this one via depends_on.", + "pattern": "^[a-z0-9]+(_[a-z0-9]+)*$" + }, + "skill": { + "type": "string", + "description": "Name of an installed skill (kebab-case) the step will invoke. Must match an entry in the agent's skills/ directory, or 'approval' for a human-review step.", + "pattern": "^[a-z0-9]+(-[a-z0-9]+)*$" + }, + "prompt": { + "type": "string", + "description": "The natural-language instruction passed to the skill for this step.", + "minLength": 1 + }, + "channel": { + "type": "string", + "description": "Optional channel/destination for the step output (e.g. a Slack channel name).", + "minLength": 1 + }, + "depends_on": { + "type": "array", + "description": "Optional list of step ids that must complete before this step runs.", + "items": { + "type": "string", + "pattern": "^[a-z0-9]+(_[a-z0-9]+)*$" + }, + "uniqueItems": true + }, + "requires_approval": { + "type": "boolean", + "description": "If true, the workflow pauses for human approval before this step runs." + } + } + } + } +} diff --git a/src/commands/workflow.ts b/src/commands/workflow.ts new file mode 100644 index 0000000..1d7ff84 --- /dev/null +++ b/src/commands/workflow.ts @@ -0,0 +1,243 @@ +import { existsSync } from "fs"; +import { mkdir, readFile, writeFile } from "fs/promises"; +import { join, resolve, sep } from "path"; +import { discoverSkills } from "../skills.js"; +import { validateSkillReferences, validateWorkflow } from "../utils/schemas.js"; +import { generateWorkflow, type LlmClient } from "../utils/workflow-generator.js"; + +interface GenerateFlags { + dir: string; + prompt?: string; + refine?: string; + model?: string; + apiKey?: string; + dryRun: boolean; + force?: boolean; + allowMissingSkills?: boolean; +} + +const RED = (s: string) => `\x1b[31m${s}\x1b[0m`; +const GREEN = (s: string) => `\x1b[32m${s}\x1b[0m`; +const DIM = (s: string) => `\x1b[2m${s}\x1b[0m`; +const BOLD = (s: string) => `\x1b[1m${s}\x1b[0m`; + +const MAX_RETRIES = 2; + +function printHelp(): void { + console.log(`${BOLD("gitagent workflow")} — generate SkillFlow workflows from natural language + +Usage: + gitagent workflow generate [options] + +Options: + -d, --dir Agent directory (default: current directory) + -p, --prompt Natural-language description of the workflow (required) + --refine Refine an existing workflow YAML by applying --prompt as an instruction + (must be a path inside the agent directory) + -m, --model LLM model in provider:model form (default: openai:gpt-4o) + --api-key API key for the provider (falls back to OPENAI_API_KEY or _API_KEY) + --dry-run Print the generated YAML to stdout instead of writing a file + -f, --force Overwrite workflows/.yaml if it already exists + (without this flag an existing file is never replaced) + --allow-missing-skills + Accept steps that reference skills which are not installed + (by default generation fails rather than writing a workflow + that cannot run) + -h, --help Show this help message + +Examples: + gitagent workflow generate -p "every morning summarize unread emails and post to Slack" + gitagent workflow generate -p "add a human approval step before the Slack post" --refine workflows/morning-digest.yaml +`); +} + +// Reads the value that follows a flag, erroring out when the flag is the last +// token. Without this, argv[++i] yields undefined and the failure surfaces far +// from the parse site (e.g. resolve(undefined) throwing a bare TypeError). +function valueFor(argv: string[], i: number, flag: string): string { + const v = argv[i + 1]; + if (v === undefined) { + console.error(RED(`${flag} requires a value`)); + process.exit(2); + } + return v; +} + +function parseFlags(argv: string[]): GenerateFlags { + const flags: GenerateFlags = { dir: process.cwd(), dryRun: false }; + for (let i = 0; i < argv.length; i++) { + const a = argv[i]; + switch (a) { + case "-d": + case "--dir": + flags.dir = valueFor(argv, i++, a); + break; + case "-p": + case "--prompt": + flags.prompt = valueFor(argv, i++, a); + break; + case "--refine": + flags.refine = valueFor(argv, i++, a); + break; + case "-m": + case "--model": + flags.model = valueFor(argv, i++, a); + break; + case "--api-key": + flags.apiKey = valueFor(argv, i++, a); + break; + case "--dry-run": + flags.dryRun = true; + break; + case "-f": + case "--force": + flags.force = true; + break; + case "--allow-missing-skills": + flags.allowMissingSkills = true; + break; + case "-h": + case "--help": + printHelp(); + process.exit(0); + break; + default: + if (!a.startsWith("-") && flags.prompt === undefined) { + flags.prompt = a; + } else { + console.error(RED(`Unknown option: ${a}`)); + process.exit(2); + } + } + } + return flags; +} + +function slugify(name: string): string { + const cleaned = name + .toLowerCase() + .trim() + .replace(/[^a-z0-9-]+/g, "-") + .replace(/^-+|-+$/g, "") + .replace(/-+/g, "-"); + return cleaned || "workflow"; +} + +export interface RunGenerateOptions { + flags: GenerateFlags; + llm?: LlmClient; +} + +export async function runGenerate(opts: RunGenerateOptions): Promise<{ filePath?: string; yaml: string; }> { + const { flags } = opts; + if (!flags.prompt || !flags.prompt.trim()) { + throw new Error("--prompt is required"); + } + + const agentDir = resolve(flags.dir); + const skills = await discoverSkills(agentDir); + const skillNames = skills.map((s) => s.name); + + let previousWorkflow: string | undefined; + if (flags.refine) { + // resolve() ignores the base when the second argument is absolute, so an + // absolute path or a ../ traversal would otherwise read (and ship to the + // LLM) any file on disk. Keep --refine inside the agent directory. + const refinePath = resolve(agentDir, flags.refine); + if (refinePath !== agentDir && !refinePath.startsWith(agentDir + sep)) { + throw new Error(`--refine path must be inside the agent directory: ${refinePath}`); + } + previousWorkflow = await readFile(refinePath, "utf-8"); + } + + let promptForLlm = flags.prompt.trim(); + let lastErrors: string[] = []; + let yaml = ""; + + for (let attempt = 0; attempt <= MAX_RETRIES; attempt++) { + console.error(DIM(attempt === 0 ? "Generating workflow..." : `Retry ${attempt}/${MAX_RETRIES} — fixing validation errors...`)); + yaml = await generateWorkflow({ + prompt: promptForLlm, + skills, + previousWorkflow, + model: flags.model, + apiKey: flags.apiKey, + llm: opts.llm, + }); + const result = validateWorkflow(yaml); + // A schema-valid workflow can still name skills that aren't installed, + // which would only surface as a run-time failure. Fold that into the same + // retry loop so the model gets a chance to pick real skills. + const skillErrors = result.valid && !flags.allowMissingSkills + ? validateSkillReferences(result.data!, skillNames) + : []; + if (result.valid && skillErrors.length === 0) { + lastErrors = []; + break; + } + lastErrors = [...result.errors, ...skillErrors]; + if (attempt < MAX_RETRIES) { + promptForLlm = + `${flags.prompt.trim()}\n\nThe previous attempt failed schema validation or referenced skills that are not installed. Fix these errors and return the full YAML again:\n` + + lastErrors.map((e) => `- ${e}`).join("\n"); + } + } + + if (lastErrors.length > 0) { + console.error(RED("\nWorkflow validation failed after retries:")); + for (const e of lastErrors) console.error(RED(` - ${e}`)); + if (lastErrors.some((e) => e.includes("is not an installed skill"))) { + console.error(DIM(`\nInstalled skills: ${skillNames.join(", ") || "(none)"}`)); + console.error( + DIM( + "Create the missing skill(s) under skills/ first, or re-run with --allow-missing-skills to write the workflow anyway.", + ), + ); + } + console.error(DIM("\nLast generated YAML:\n")); + console.error(yaml); + throw new Error("Validation failed after retries"); + } + + if (flags.dryRun) { + process.stdout.write(yaml.endsWith("\n") ? yaml : yaml + "\n"); + return { yaml }; + } + + // Parse the validated YAML to get the workflow name for the file path. + const validated = validateWorkflow(yaml).data!; + const slug = slugify(validated.name); + const workflowsDir = join(agentDir, "workflows"); + const filePath = join(workflowsDir, `${slug}.yaml`); + if (!flags.force && existsSync(filePath)) { + throw new Error( + `${filePath} already exists. Re-run with --force to overwrite it, or use --dry-run to preview the generated workflow.`, + ); + } + await mkdir(workflowsDir, { recursive: true }); + await writeFile(filePath, yaml.endsWith("\n") ? yaml : yaml + "\n", "utf-8"); + console.error(GREEN(`\nWrote workflow to ${filePath}`)); + return { filePath, yaml }; +} + +export async function handleWorkflowCommand(argv: string[]): Promise { + // argv is the raw process.argv tail starting at the 'workflow' token. + // argv[0] === "workflow"; argv[1] is the sub-command. + const sub = argv[1]; + if (!sub || sub === "-h" || sub === "--help") { + printHelp(); + return; + } + if (sub !== "generate") { + console.error(RED(`Unknown subcommand: ${sub}`)); + printHelp(); + process.exit(2); + } + const flags = parseFlags(argv.slice(2)); + try { + await runGenerate({ flags }); + } catch (err: any) { + console.error(RED(`\nError: ${err?.message ?? String(err)}`)); + process.exit(1); + } +} diff --git a/src/index.ts b/src/index.ts index ba4eeb2..5130991 100644 --- a/src/index.ts +++ b/src/index.ts @@ -26,6 +26,7 @@ import type { LocalSession } from "./session.js"; // Imported dynamically below so the slim core has no static dependency on it — // users without voice get a clean install + a clear error if they try --voice. import { handlePluginCommand } from "./plugin-cli.js"; +import { handleWorkflowCommand } from "./commands/workflow.js"; import { context as otelContext } from "@opentelemetry/api"; import { initTelemetry, @@ -305,6 +306,12 @@ async function ensureRepo(dir: string, model?: string): Promise { } async function main(): Promise { + // Handle workflow subcommand: gitagent workflow + if (process.argv[2] === "workflow") { + await handleWorkflowCommand(process.argv.slice(2)); + return; + } + // Handle plugin subcommand: gitagent plugin if (process.argv[2] === "plugin") { const allArgs = process.argv.slice(3); diff --git a/src/utils/schemas.ts b/src/utils/schemas.ts new file mode 100644 index 0000000..1eaeebf --- /dev/null +++ b/src/utils/schemas.ts @@ -0,0 +1,294 @@ +import { readFileSync, existsSync } from "fs"; +import { dirname, join, resolve } from "path"; +import { fileURLToPath } from "url"; +import yaml from "js-yaml"; + +export interface WorkflowStep { + id?: string; + skill: string; + prompt: string; + channel?: string; + depends_on?: string[]; + requires_approval?: boolean; +} + +export interface WorkflowDef { + name: string; + description: string; + steps: WorkflowStep[]; +} + +export interface ValidationResult { + valid: boolean; + errors: string[]; + data?: WorkflowDef; +} + +let cachedSchema: any = null; +let cachedSchemaText: string | null = null; + +function resolveSchemaPath(): string { + const here = dirname(fileURLToPath(import.meta.url)); + // Try candidates relative to this module's location. + // 1. Running from src/utils/ in tests: ../../spec/schemas/workflow.schema.json + // 2. Running from dist/utils/ after build: ../../spec/schemas/workflow.schema.json + // 3. Running from dist/utils/ when spec/ is not packed: walk upward. + const candidates = [ + resolve(here, "..", "..", "spec", "schemas", "workflow.schema.json"), + resolve(here, "..", "..", "..", "spec", "schemas", "workflow.schema.json"), + ]; + for (const p of candidates) { + if (existsSync(p)) return p; + } + // Fallback: walk up to 6 levels looking for the schema. + let cur = here; + for (let i = 0; i < 6; i++) { + const guess = join(cur, "spec", "schemas", "workflow.schema.json"); + if (existsSync(guess)) return guess; + const parent = dirname(cur); + if (parent === cur) break; + cur = parent; + } + throw new Error(`Could not locate spec/schemas/workflow.schema.json relative to ${here}`); +} + +export function loadWorkflowSchema(): any { + if (cachedSchema) return cachedSchema; + const path = resolveSchemaPath(); + cachedSchemaText = readFileSync(path, "utf-8"); + cachedSchema = JSON.parse(cachedSchemaText); + return cachedSchema; +} + +export function getWorkflowSchemaText(): string { + if (cachedSchemaText) return cachedSchemaText; + loadWorkflowSchema(); + return cachedSchemaText!; +} + +function typeOf(v: any): string { + if (v === null) return "null"; + if (Array.isArray(v)) return "array"; + return typeof v; +} + +function matchesType(v: any, expected: string | string[]): boolean { + const types = Array.isArray(expected) ? expected : [expected]; + const actual = typeOf(v); + return types.includes(actual) || (types.includes("integer") && actual === "number" && Number.isInteger(v)); +} + +interface Issue { + path: string; + message: string; +} + +function validateAgainst(data: any, schema: any, path: string, root: any, issues: Issue[]): void { + // Resolve $ref + if (schema && typeof schema === "object" && schema.$ref) { + const ref = schema.$ref as string; + if (!ref.startsWith("#/")) { + issues.push({ path, message: `unsupported $ref "${ref}" (only local refs are supported)` }); + return; + } + const segments = ref.slice(2).split("/"); + let resolved: any = root; + for (const seg of segments) { + resolved = resolved?.[seg]; + } + if (!resolved) { + issues.push({ path, message: `cannot resolve $ref "${ref}"` }); + return; + } + validateAgainst(data, resolved, path, root, issues); + return; + } + + if (!schema || typeof schema !== "object") return; + + if (schema.type && !matchesType(data, schema.type)) { + issues.push({ + path, + message: `expected type ${Array.isArray(schema.type) ? schema.type.join("|") : schema.type}, got ${typeOf(data)}`, + }); + return; + } + + if (typeOf(data) === "object") { + const required: string[] = Array.isArray(schema.required) ? schema.required : []; + for (const key of required) { + if (!(key in data)) { + issues.push({ path: path || "(root)", message: `missing required property "${key}"` }); + } + } + + const props = schema.properties ?? {}; + const additionalAllowed = schema.additionalProperties !== false; + for (const key of Object.keys(data)) { + const childPath = path ? `${path}.${key}` : key; + if (props[key]) { + validateAgainst(data[key], props[key], childPath, root, issues); + } else if (!additionalAllowed) { + issues.push({ path: path || "(root)", message: `unknown property "${key}"` }); + } + } + } else if (typeOf(data) === "array") { + if (schema.minItems != null && data.length < schema.minItems) { + issues.push({ path: path || "(root)", message: `array must have at least ${schema.minItems} item(s), got ${data.length}` }); + } + if (schema.items) { + for (let i = 0; i < data.length; i++) { + validateAgainst(data[i], schema.items, `${path}[${i}]`, root, issues); + } + } + if (schema.uniqueItems === true) { + const seen = new Set(); + for (let i = 0; i < data.length; i++) { + const key = JSON.stringify(data[i]); + if (seen.has(key)) { + issues.push({ path: `${path}[${i}]`, message: `duplicate item` }); + } + seen.add(key); + } + } + } else if (typeOf(data) === "string") { + if (schema.minLength != null && data.length < schema.minLength) { + issues.push({ path: path || "(root)", message: `string must be at least ${schema.minLength} character(s)` }); + } + if (schema.pattern && !new RegExp(schema.pattern).test(data)) { + issues.push({ path: path || "(root)", message: `value "${data}" does not match pattern ${schema.pattern}` }); + } + } +} + +/** + * Depth-first search over the depends_on graph. Returns the offending path + * (e.g. ["a", "b", "a"]) for the first cycle found, or null when the graph is + * acyclic. Edges pointing at undeclared ids are ignored — those are already + * reported by the unknown-id check. + */ +function findDependencyCycle(steps: any[]): string[] | null { + const edges = new Map(); + for (const step of steps) { + if (!step || typeof step.id !== "string") continue; + const deps = Array.isArray(step.depends_on) ? step.depends_on.filter((d: any) => typeof d === "string") : []; + // Later duplicates of an id merge rather than overwrite, so no edge is lost. + edges.set(step.id, [...(edges.get(step.id) ?? []), ...deps]); + } + + const done = new Set(); + const stack: string[] = []; + const onStack = new Set(); + + function visit(id: string): string[] | null { + if (done.has(id)) return null; + if (onStack.has(id)) return [...stack.slice(stack.indexOf(id)), id]; + onStack.add(id); + stack.push(id); + for (const dep of edges.get(id) ?? []) { + if (!edges.has(dep)) continue; + const found = visit(dep); + if (found) return found; + } + stack.pop(); + onStack.delete(id); + done.add(id); + return null; + } + + for (const id of edges.keys()) { + const found = visit(id); + if (found) return found; + } + return null; +} + +/** + * Pseudo-skill for human-approval steps. It never exists under `skills/`, so it + * has to be exempt from the installed-skill check below. + */ +export const APPROVAL_SKILL = "approval"; + +/** + * Cross-check every step's `skill` against the skills actually installed in the + * agent directory. The JSON Schema can only check shape, so without this a + * plausible-looking workflow referencing skills that don't exist is written to + * disk and fails at run time instead of at generation time. + * + * Returns validator-shaped error strings so callers can fold them into the same + * retry loop as schema errors. An empty `installedSkills` list disables the + * check — that is the same escape hatch the generator's system prompt takes, + * where the model is told to invent sensible skill names. + */ +export function validateSkillReferences(workflow: WorkflowDef, installedSkills: string[]): string[] { + if (installedSkills.length === 0) return []; + const known = new Set([...installedSkills, APPROVAL_SKILL]); + const errors: string[] = []; + const steps = Array.isArray(workflow?.steps) ? workflow.steps : []; + steps.forEach((step: any, i: number) => { + const skill = typeof step?.skill === "string" ? step.skill.trim() : ""; + if (skill && !known.has(skill)) { + errors.push(`steps[${i}].skill: "${skill}" is not an installed skill`); + } + }); + return errors; +} + +export function validateWorkflow(yamlText: string): ValidationResult { + let parsed: unknown; + try { + parsed = yaml.load(yamlText); + } catch (err: any) { + return { valid: false, errors: [`YAML parse error: ${err?.message ?? String(err)}`] }; + } + + if (parsed === null || parsed === undefined) { + return { valid: false, errors: ["workflow is empty"] }; + } + + if (typeof parsed !== "object" || Array.isArray(parsed)) { + return { valid: false, errors: [`workflow must be an object, got ${typeOf(parsed)}`] }; + } + + const schema = loadWorkflowSchema(); + const issues: Issue[] = []; + validateAgainst(parsed, schema, "", schema, issues); + + // Cross-field check: depends_on ids must reference a step declared earlier. + // The set grows as steps are visited rather than being pre-built, so a forward + // reference is rejected here instead of passing validation and failing later in + // loadFlowDefinition — after the file is already on disk and out of the retry + // loop. Steps execute in declaration order, so these are the same semantics. + const data = parsed as any; + if (Array.isArray(data.steps)) { + const available = new Set(); + data.steps.forEach((step: any, i: number) => { + if (step && Array.isArray(step.depends_on)) { + for (const dep of step.depends_on) { + if (typeof dep === "string" && !available.has(dep)) { + issues.push({ + path: `steps[${i}].depends_on`, + message: `references unknown step id "${dep}" (must be declared in a preceding step)`, + }); + } + } + } + if (step && typeof step.id === "string") available.add(step.id); + }); + + // Cross-field check: the depends_on graph must be acyclic. A self-reference + // or an A -> B -> A cycle would deadlock (or loop) at execution time. + const cycle = findDependencyCycle(data.steps); + if (cycle) { + issues.push({ path: "steps", message: `depends_on cycle detected: ${cycle.join(" -> ")}` }); + } + } + + if (issues.length === 0) { + return { valid: true, errors: [], data: parsed as WorkflowDef }; + } + return { + valid: false, + errors: issues.map((i) => (i.path ? `${i.path}: ${i.message}` : i.message)), + }; +} diff --git a/src/utils/workflow-generator.ts b/src/utils/workflow-generator.ts new file mode 100644 index 0000000..61ff0a7 --- /dev/null +++ b/src/utils/workflow-generator.ts @@ -0,0 +1,204 @@ +import type { SkillMetadata } from "../skills.js"; +import { getWorkflowSchemaText } from "./schemas.js"; + +export type LlmRole = "system" | "user" | "assistant"; + +export interface LlmMessage { + role: LlmRole; + content: string; +} + +export interface LlmCallOptions { + model: string; + temperature?: number; + apiKey?: string; +} + +export type LlmClient = (messages: LlmMessage[], opts: LlmCallOptions) => Promise; + +export interface GenerateWorkflowOptions { + prompt: string; + skills: SkillMetadata[]; + previousWorkflow?: string; + model?: string; + apiKey?: string; + llm?: LlmClient; +} + +const DEFAULT_MODEL = "openai:gpt-4o"; + +const SYSTEM_RULES = `Rules (MUST follow): +- Output ONLY valid YAML. No markdown fences, no prose, no commentary before or after. +- The output MUST validate against the schema above. +- "name" must be kebab-case (lowercase letters, digits, and single hyphens). +- Every step "skill" must reference an installed skill from the list below, unless the step is human approval — in that case use skill: "approval" and set requires_approval: true. +- Never invent a skill name. Steps naming a skill that is not installed are rejected, so express the request using the installed skills even if that means fewer steps. +- When multiple steps need ordering beyond top-to-bottom, give them snake_case "id" values and use "depends_on" to express the dependency. +- Keep "prompt" fields concrete and self-contained — a downstream agent will read them verbatim. +- Do not invent fields not in the schema.`; + +const FEW_SHOT_USER_1 = "Every morning, summarize my unread emails and post the summary to Slack."; + +const FEW_SHOT_ASSISTANT_1 = `name: morning-email-digest +description: Summarize unread emails and post the digest to Slack each morning. +steps: + - skill: gmail + prompt: Fetch all unread emails from the last 24 hours and return subject, sender, and a one-sentence summary for each. + - skill: summarize + prompt: Compose a single-paragraph digest of the unread emails, grouped by sender priority. + - skill: slack + prompt: Post the digest to the configured channel. + channel: "#daily-digest" +`; + + +const FEW_SHOT_USER_2 = "Pull yesterday's sales data, get sign-off, then send the report to the team."; + +const FEW_SHOT_ASSISTANT_2 = `name: daily-sales-report +description: Pull sales data, require human approval, then distribute the report. +steps: + - id: pull_data + skill: analytics + prompt: Pull yesterday's sales totals broken down by region and product line. + - id: approve + skill: approval + prompt: Review the pulled sales data for accuracy and approve distribution. + requires_approval: true + depends_on: [pull_data] + - id: send_report + skill: email + prompt: Send the approved sales report to the sales-leadership distribution list. + depends_on: [approve] +`; + +function formatSkillsForPrompt(skills: SkillMetadata[]): string { + if (skills.length === 0) { + return "(no installed skills detected — use generic skill names that match the user's intent, e.g. gmail, slack, summarize)"; + } + return skills.map((s) => `- ${s.name}: ${s.description}`).join("\n"); +} + +export function buildSystemPrompt(skills: SkillMetadata[]): string { + const schemaText = getWorkflowSchemaText(); + const skillList = formatSkillsForPrompt(skills); + return `You are a workflow builder for GitClaw SkillFlow. + +Your job is to translate the user's natural-language description into a YAML workflow that conforms exactly to this JSON Schema: + + +${schemaText} + + +Installed skills the workflow may invoke: + +${skillList} + + +${SYSTEM_RULES}`; +} + +export function buildMessages(opts: GenerateWorkflowOptions): LlmMessage[] { + const messages: LlmMessage[] = [ + { role: "system", content: buildSystemPrompt(opts.skills) }, + { role: "user", content: FEW_SHOT_USER_1 }, + { role: "assistant", content: FEW_SHOT_ASSISTANT_1 }, + { role: "user", content: FEW_SHOT_USER_2 }, + { role: "assistant", content: FEW_SHOT_ASSISTANT_2 }, + ]; + + if (opts.previousWorkflow && opts.previousWorkflow.trim()) { + messages.push({ + role: "user", + content: `Here is the current workflow:\n\n${opts.previousWorkflow.trim()}\n\nApply this refinement: ${opts.prompt}\n\nReturn the complete updated workflow as YAML — not a diff.`, + }); + } else { + messages.push({ role: "user", content: opts.prompt }); + } + + return messages; +} + +// Pull the first fenced block out of the response regardless of any surrounding +// commentary ("Here is your workflow: ```yaml ... ``` Let me know if..."). An +// anchored match would leave the prose in place and burn a retry on a YAML +// parse error. Falls back to the raw trimmed text when there is no fence. +const FENCE_INNER_RE = /```(?:ya?ml)?[ \t]*\r?\n([\s\S]*?)\r?\n?```/i; + +export function stripCodeFences(raw: string): string { + const m = raw.match(FENCE_INNER_RE); + return m ? m[1].trim() : raw.trim(); +} + +async function defaultLlmClient(messages: LlmMessage[], opts: LlmCallOptions): Promise { + const [providerRaw, ...modelParts] = opts.model.split(":"); + const provider = providerRaw?.trim(); + const modelId = modelParts.join(":").trim(); + if (!provider || !modelId) { + throw new Error(`Invalid model spec "${opts.model}". Expected "provider:model-id" (e.g. "openai:gpt-4o").`); + } + + const apiKey = + opts.apiKey || + process.env[`${provider.toUpperCase()}_API_KEY`] || + process.env.OPENAI_API_KEY; + if (!apiKey) { + throw new Error( + `No API key found. Pass --api-key or set ${provider.toUpperCase()}_API_KEY (or OPENAI_API_KEY) in your environment.`, + ); + } + + const [piAi, piAgentCore] = await Promise.all([ + import("@mariozechner/pi-ai") as Promise, + import("@mariozechner/pi-agent-core") as Promise, + ]); + const { getModel } = piAi; + const { Agent } = piAgentCore; + + if (!process.env[`${provider.toUpperCase()}_API_KEY`]) { + process.env[`${provider.toUpperCase()}_API_KEY`] = apiKey; + } + + const model = getModel(provider as any, modelId as any); + const systemMessage = messages.find((m) => m.role === "system")?.content ?? ""; + const conversation = messages.filter((m) => m.role !== "system"); + + const agent = new Agent({ + initialState: { + systemPrompt: systemMessage, + model, + tools: [], + temperature: opts.temperature ?? 0, + maxTokens: 4096, + }, + }); + + let collected = ""; + agent.subscribe((event: any) => { + if (event.type === "message_end" && event.message?.role === "assistant") { + for (const block of event.message.content) { + if (block.type === "text") collected += block.text; + } + } + }); + + // Replay prior assistant/user turns as a single composed prompt so we don't + // have to drive Agent through multiple chat turns. The few-shot pairs are + // preserved as part of the prompt text so the model still sees them. + const composed = conversation + .map((m) => `[${m.role.toUpperCase()}]\n${m.content}`) + .join("\n\n"); + + await agent.prompt(composed); + return collected; +} + +export async function generateWorkflow(opts: GenerateWorkflowOptions): Promise { + if (!opts.prompt || !opts.prompt.trim()) { + throw new Error("generateWorkflow: prompt is required"); + } + const llm = opts.llm ?? defaultLlmClient; + const model = opts.model ?? DEFAULT_MODEL; + const messages = buildMessages(opts); + const raw = await llm(messages, { model, apiKey: opts.apiKey, temperature: 0 }); + return stripCodeFences(raw); +} diff --git a/src/workflows.ts b/src/workflows.ts index 03fa300..72cb182 100644 --- a/src/workflows.ts +++ b/src/workflows.ts @@ -4,9 +4,15 @@ import { mkdirSync } from "fs"; import yaml from "js-yaml"; export interface SkillFlowStep { + /** Optional snake_case id other steps reference via depends_on. */ + id?: string; skill: string; prompt: string; channel?: string; + /** Ids of steps that must complete first. Kept in sync with workflow.schema.json. */ + depends_on?: string[]; + /** Pause for human approval before running this step. */ + requires_approval?: boolean; } export interface SkillFlowDefinition { @@ -24,6 +30,24 @@ export interface WorkflowMetadata { steps?: SkillFlowStep[]; } +const KEBAB_RE = /^[a-z0-9]+(-[a-z0-9]+)*$/; + +/** + * Normalize raw YAML steps into SkillFlowStep. Every field declared in + * spec/schemas/workflow.schema.json is carried through — dropping `id` or + * `depends_on` here would silently ignore ordering the author declared. + */ +function normalizeSteps(rawSteps: any[]): SkillFlowStep[] { + return rawSteps.map((s: any) => ({ + ...(s?.id ? { id: String(s.id) } : {}), + skill: String(s?.skill || ""), + prompt: String(s?.prompt || ""), + ...(s?.channel ? { channel: String(s.channel) } : {}), + ...(Array.isArray(s?.depends_on) ? { depends_on: s.depends_on.map((d: any) => String(d)) } : {}), + ...(s?.requires_approval != null ? { requires_approval: Boolean(s.requires_approval) } : {}), + })); +} + function parseFrontmatter(content: string): { frontmatter: Record; body: string } { const match = content.match(/^---\r?\n([\s\S]*?)\r?\n---\r?\n?([\s\S]*)$/); if (!match) { @@ -64,11 +88,7 @@ export async function discoverWorkflows(agentDir: string): Promise ({ - skill: String(s.skill || ""), - prompt: String(s.prompt || ""), - ...(s.channel ? { channel: String(s.channel) } : {}), - })), + steps: normalizeSteps(data.steps as any[]), } : { type: "basic" as const }), }); } @@ -98,22 +118,82 @@ export async function discoverWorkflows(agentDir: string): Promise a.name.localeCompare(b.name)); } -const KEBAB_RE = /^[a-z0-9]+(-[a-z0-9]+)*$/; - -export async function loadFlowDefinition(filePath: string): Promise { +let warnedLegacyFlowPath = false; + +/** + * Load a flow by name from `/workflows/.yaml`. + * + * The path is built here rather than accepted from the caller so the function + * owns its own safety: `flowName` must be kebab-case, which rules out traversal + * segments and absolute paths no matter how the name reached this code. + */ +export async function loadFlowDefinition(agentDir: string, flowName: string): Promise; +/** + * Legacy single-argument form, kept so callers written against the previous + * signature (e.g. `@open-gitagent/voice`) keep working. It reads the path as + * given and therefore provides no containment — migrate to + * `loadFlowDefinition(agentDir, flowName)`. + * + * @deprecated Pass `(agentDir, flowName)` instead. + */ +export async function loadFlowDefinition(filePath: string): Promise; +export async function loadFlowDefinition(agentDirOrPath: string, flowName?: string): Promise { + // Which form was used is decided by arity, not by inspecting the string: a + // missing second argument would otherwise stringify to "undefined", which + // passes KEBAB_RE and fails later as a confusing ENOENT. + let filePath: string; + if (flowName === undefined) { + if (!warnedLegacyFlowPath) { + warnedLegacyFlowPath = true; + console.warn( + "[gitagent] loadFlowDefinition(filePath) is deprecated — call loadFlowDefinition(agentDir, flowName) so the path is validated and built internally.", + ); + } + filePath = agentDirOrPath; + } else { + if (!KEBAB_RE.test(flowName)) { + throw new Error(`Invalid flow name "${flowName}": must be kebab-case (e.g. my-flow-name)`); + } + filePath = join(agentDirOrPath, "workflows", `${flowName}.yaml`); + } const raw = await readFile(filePath, "utf-8"); - const data = yaml.load(raw) as Record; - if (!data?.name || !data?.steps || !Array.isArray(data.steps)) { + const data = yaml.load(raw); + + // yaml.load returns null for an empty file and a string/number for a + // single scalar document — both need a clearer error than "missing name". + if (data === null || typeof data !== "object" || Array.isArray(data)) { + throw new Error("Invalid flow definition: file must be a YAML mapping"); + } + const doc = data as Record; + if (!doc.name || !Array.isArray(doc.steps)) { throw new Error("Invalid flow definition: missing name or steps"); } + + const steps = normalizeSteps(doc.steps); + const badStep = steps.findIndex((s) => !s.skill.trim()); + if (badStep !== -1) { + throw new Error(`Invalid flow definition: step[${badStep}] has an empty skill`); + } + + // Steps execute in declaration order, so every depends_on must name a + // preceding step. This also rejects self-references and cycles, and keeps + // declared dependencies from being silently ignored at run time. + const available = new Set(); + steps.forEach((s, i) => { + for (const dep of s.depends_on ?? []) { + if (!available.has(dep)) { + throw new Error( + `Invalid flow definition: step[${i}] depends_on "${dep}", which is not the id of a preceding step`, + ); + } + } + if (s.id) available.add(s.id); + }); + return { - name: data.name, - description: data.description || "", - steps: data.steps.map((s: any) => ({ - skill: String(s.skill || ""), - prompt: String(s.prompt || ""), - ...(s.channel ? { channel: String(s.channel) } : {}), - })), + name: String(doc.name), + description: String(doc.description || ""), + steps, }; } @@ -130,7 +210,14 @@ export async function saveFlowDefinition(agentDir: string, flow: SkillFlowDefini const content = yaml.dump({ name: flow.name, description: flow.description || "", - steps: flow.steps.map((s) => ({ skill: s.skill, prompt: s.prompt, ...(s.channel ? { channel: s.channel } : {}) })), + steps: flow.steps.map((s) => ({ + ...(s.id ? { id: s.id } : {}), + skill: s.skill, + prompt: s.prompt, + ...(s.channel ? { channel: s.channel } : {}), + ...(s.depends_on?.length ? { depends_on: s.depends_on } : {}), + ...(s.requires_approval != null ? { requires_approval: s.requires_approval } : {}), + })), }, { lineWidth: 120 }); await writeFile(filePath, content, "utf-8"); return filePath; diff --git a/test/ts-resolve-hook.mjs b/test/ts-resolve-hook.mjs new file mode 100644 index 0000000..3fa4398 --- /dev/null +++ b/test/ts-resolve-hook.mjs @@ -0,0 +1,27 @@ +// ESM resolve hook: if a `.js` import inside src/ has no matching `.js` +// on disk, retry with the `.ts` extension. Lets node --experimental-strip-types +// run TypeScript sources whose internal imports follow the Node16 `.js` style. + +import { existsSync } from "node:fs"; +import { fileURLToPath } from "node:url"; + +export async function resolve(specifier, context, nextResolve) { + if (specifier.startsWith(".") && specifier.endsWith(".js")) { + try { + return await nextResolve(specifier, context); + } catch (err) { + if (err?.code !== "ERR_MODULE_NOT_FOUND") throw err; + const tsSpecifier = specifier.slice(0, -3) + ".ts"; + try { + const candidate = await nextResolve(tsSpecifier, context); + if (candidate?.url?.startsWith("file://") && existsSync(fileURLToPath(candidate.url))) { + return candidate; + } + } catch { + // fall through and re-throw the original .js miss + } + throw err; + } + } + return nextResolve(specifier, context); +} diff --git a/test/workflow-generator.test.ts b/test/workflow-generator.test.ts new file mode 100644 index 0000000..47d5b6a --- /dev/null +++ b/test/workflow-generator.test.ts @@ -0,0 +1,402 @@ +// Unit tests for src/utils/workflow-generator.ts and the retry loop in +// src/commands/workflow.ts. The LLM is fully mocked — no network calls. + +import test from "node:test"; +import assert from "node:assert/strict"; +import { mkdtemp, readFile, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { + buildMessages, + buildSystemPrompt, + stripCodeFences, + generateWorkflow, + type LlmClient, + type LlmMessage, +} from "../src/utils/workflow-generator.ts"; +import { runGenerate } from "../src/commands/workflow.ts"; +import type { SkillMetadata } from "../src/skills.ts"; + +const SKILLS: SkillMetadata[] = [ + { name: "gmail", description: "Read and send email", directory: "/x/skills/gmail", filePath: "/x/skills/gmail/SKILL.md" }, + { name: "slack", description: "Post to Slack", directory: "/x/skills/slack", filePath: "/x/skills/slack/SKILL.md" }, + { name: "summarize", description: "Summarize text", directory: "/x/skills/summarize", filePath: "/x/skills/summarize/SKILL.md" }, +]; + +const VALID_YAML = `name: morning-digest +description: Summarize unread emails and post to Slack each morning. +steps: + - skill: gmail + prompt: Fetch unread emails. + - skill: summarize + prompt: Compose a digest. + - skill: slack + prompt: Post the digest. + channel: "#daily-digest" +`; + +const INVALID_YAML_MISSING_NAME = `description: no name here +steps: + - skill: gmail + prompt: hi +`; + +// ── buildSystemPrompt / buildMessages ────────────────────────────────── + +test("buildSystemPrompt embeds the schema text and the skill list", () => { + const sys = buildSystemPrompt(SKILLS); + assert.ok(sys.includes(""), "system prompt missing tag"); + assert.ok(sys.includes('"$id": "https://gitagent.dev/spec/workflow.schema.json"'), "system prompt missing schema $id"); + assert.ok(sys.includes("- gmail: Read and send email"), "system prompt missing gmail skill"); + assert.ok(sys.includes("- slack: Post to Slack"), "system prompt missing slack skill"); + assert.ok(sys.includes("Output ONLY valid YAML"), "system prompt missing rule about raw YAML"); +}); + +test("buildSystemPrompt handles empty skill list with a fallback hint", () => { + const sys = buildSystemPrompt([]); + assert.ok(sys.includes("no installed skills detected"), "system prompt missing empty-skills fallback"); +}); + +test("buildMessages includes two few-shot pairs and the user prompt", () => { + const messages = buildMessages({ prompt: "Do the thing", skills: SKILLS }); + assert.equal(messages[0].role, "system"); + assert.equal(messages[1].role, "user"); + assert.equal(messages[2].role, "assistant"); + assert.equal(messages[3].role, "user"); + assert.equal(messages[4].role, "assistant"); + assert.equal(messages[5].role, "user"); + assert.equal(messages[5].content, "Do the thing"); +}); + +test("buildMessages wraps refine-mode prompts with the previous YAML and instruction", () => { + const messages = buildMessages({ + prompt: "Add an approval step before the Slack post.", + skills: SKILLS, + previousWorkflow: VALID_YAML, + }); + const last = messages[messages.length - 1]; + assert.equal(last.role, "user"); + assert.ok(last.content.includes("Here is the current workflow")); + assert.ok(last.content.includes("Add an approval step before the Slack post.")); + assert.ok(last.content.includes("morning-digest")); + assert.ok(last.content.includes("Return the complete updated workflow as YAML — not a diff.")); +}); + +// ── stripCodeFences ──────────────────────────────────────────────────── + +test("stripCodeFences removes generic fenced code", () => { + const input = "```\nname: foo\n```\n"; + assert.equal(stripCodeFences(input), "name: foo"); +}); + +test("stripCodeFences removes yaml-tagged fences", () => { + const input = "```yaml\nname: foo\n```"; + assert.equal(stripCodeFences(input), "name: foo"); +}); + +test("stripCodeFences leaves unfenced YAML alone", () => { + const input = "name: foo\n"; + assert.equal(stripCodeFences(input), "name: foo"); +}); + +test("stripCodeFences extracts the block when the LLM wraps it in prose", () => { + const input = [ + "Here is your workflow:", + "", + "```yaml", + VALID_YAML.trimEnd(), + "```", + "", + "Let me know if you want an approval step added!", + ].join("\n"); + const out = stripCodeFences(input); + assert.equal(out.startsWith("name: morning-digest"), true, out); + assert.equal(out.includes("Here is your workflow"), false, out); + assert.equal(out.includes("Let me know"), false, out); + assert.equal(out.includes("```"), false, out); +}); + +// ── generateWorkflow with an injected LLM ────────────────────────────── + +test("generateWorkflow returns the LLM output after stripping fences", async () => { + const captured: { messages: LlmMessage[] } = { messages: [] }; + const llm: LlmClient = async (messages) => { + captured.messages = messages; + return "```yaml\n" + VALID_YAML + "```"; + }; + const out = await generateWorkflow({ + prompt: "summarize emails and post to slack", + skills: SKILLS, + llm, + }); + assert.equal(out.trim().startsWith("name: morning-digest"), true); + assert.equal(captured.messages.length, 6); + assert.equal(captured.messages[0].role, "system"); +}); + +test("generateWorkflow throws if prompt is empty", async () => { + await assert.rejects( + () => generateWorkflow({ prompt: " ", skills: SKILLS, llm: async () => VALID_YAML }), + /prompt is required/, + ); +}); + +// ── Retry loop in runGenerate ────────────────────────────────────────── + +test("runGenerate retries when validation fails, then writes the file when the second attempt is valid", async () => { + const dir = await mkdtemp(join(tmpdir(), "gitagent-test-")); + try { + let calls = 0; + const llm: LlmClient = async (messages) => { + calls++; + const userMsg = messages[messages.length - 1].content; + if (calls === 1) return INVALID_YAML_MISSING_NAME; + // Second attempt: ensure the retry prompt included the validation error. + assert.ok(userMsg.includes("schema validation"), `retry user message did not mention validation: ${userMsg}`); + return VALID_YAML; + }; + const result = await runGenerate({ + flags: { + dir, + prompt: "summarize unread emails and post to Slack", + dryRun: false, + }, + llm, + }); + assert.equal(calls, 2); + assert.ok(result.filePath, "expected a written file path"); + assert.equal(result.filePath!.endsWith(join("workflows", "morning-digest.yaml")), true, result.filePath); + const written = await readFile(result.filePath!, "utf-8"); + assert.ok(written.includes("name: morning-digest")); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +test("runGenerate honours --dry-run by returning YAML without writing", async () => { + const dir = await mkdtemp(join(tmpdir(), "gitagent-test-")); + try { + const llm: LlmClient = async () => VALID_YAML; + const result = await runGenerate({ + flags: { dir, prompt: "x", dryRun: true }, + llm, + }); + assert.equal(result.filePath, undefined); + assert.ok(result.yaml.includes("name: morning-digest")); + // workflows/ must not have been created. + await assert.rejects(() => readFile(join(dir, "workflows", "morning-digest.yaml"), "utf-8")); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +test("runGenerate gives up after MAX_RETRIES and throws", async () => { + const dir = await mkdtemp(join(tmpdir(), "gitagent-test-")); + try { + let calls = 0; + const llm: LlmClient = async () => { + calls++; + return INVALID_YAML_MISSING_NAME; + }; + await assert.rejects( + () => runGenerate({ flags: { dir, prompt: "x", dryRun: true }, llm }), + /Validation failed after retries/, + ); + assert.equal(calls, 3); // 1 initial + 2 retries + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +test("runGenerate refuses to overwrite an existing workflow unless --force is passed", async () => { + const dir = await mkdtemp(join(tmpdir(), "gitagent-test-")); + try { + const llm: LlmClient = async () => VALID_YAML; + const first = await runGenerate({ flags: { dir, prompt: "x", dryRun: false }, llm }); + assert.ok(first.filePath); + + await assert.rejects( + () => runGenerate({ flags: { dir, prompt: "x", dryRun: false }, llm }), + /already exists/, + ); + + const forced = await runGenerate({ flags: { dir, prompt: "x", dryRun: false, force: true }, llm }); + assert.equal(forced.filePath, first.filePath); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +test("runGenerate rejects a --refine path outside the agent directory", async () => { + const dir = await mkdtemp(join(tmpdir(), "gitagent-test-")); + try { + const llm: LlmClient = async () => VALID_YAML; + await assert.rejects( + () => runGenerate({ flags: { dir, prompt: "x", refine: "/etc/hostname", dryRun: true }, llm }), + /must be inside the agent directory/, + ); + await assert.rejects( + () => runGenerate({ flags: { dir, prompt: "x", refine: "../../etc/hostname", dryRun: true }, llm }), + /must be inside the agent directory/, + ); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +// ── Installed-skill cross-check ──────────────────────────────────────── + +const UNKNOWN_SKILL_YAML = `name: morning-weather-summary +description: Check the weather each morning and send a text summary. +steps: + - id: fetch_weather + skill: weather + prompt: Fetch today's forecast. + - id: send_summary + skill: sms + prompt: Text a one-sentence summary. + depends_on: [fetch_weather] +`; + +// Materializes real skills//SKILL.md files so discoverSkills() finds them. +async function withInstalledSkills(names: string[], fn: (dir: string) => Promise): Promise { + const { mkdir, writeFile } = await import("node:fs/promises"); + const dir = await mkdtemp(join(tmpdir(), "gitagent-skills-")); + try { + for (const name of names) { + await mkdir(join(dir, "skills", name), { recursive: true }); + await writeFile( + join(dir, "skills", name, "SKILL.md"), + `---\nname: ${name}\ndescription: Test skill ${name}\n---\n\nDo ${name} things.\n`, + "utf-8", + ); + } + await fn(dir); + } finally { + await rm(dir, { recursive: true, force: true }); + } +} + +test("runGenerate retries when a step names a skill that is not installed", async () => { + await withInstalledSkills(["gmail", "slack", "summarize"], async (dir) => { + let calls = 0; + let retryPrompt = ""; + const llm: LlmClient = async (messages) => { + calls++; + if (calls === 1) return UNKNOWN_SKILL_YAML; + retryPrompt = messages[messages.length - 1].content; + return VALID_YAML; + }; + const result = await runGenerate({ flags: { dir, prompt: "text me the weather", dryRun: true }, llm }); + assert.equal(calls, 2); + assert.ok(retryPrompt.includes('"weather" is not an installed skill'), retryPrompt); + assert.ok(retryPrompt.includes('"sms" is not an installed skill'), retryPrompt); + assert.ok(result.yaml.includes("name: morning-digest")); + }); +}); + +test("runGenerate fails rather than writing a workflow that references missing skills", async () => { + await withInstalledSkills(["gmail", "slack", "summarize"], async (dir) => { + const llm: LlmClient = async () => UNKNOWN_SKILL_YAML; + await assert.rejects( + () => runGenerate({ flags: { dir, prompt: "text me the weather", dryRun: false }, llm }), + /Validation failed after retries/, + ); + await assert.rejects(() => readFile(join(dir, "workflows", "morning-weather-summary.yaml"), "utf-8")); + }); +}); + +test("runGenerate --allow-missing-skills writes the workflow anyway", async () => { + await withInstalledSkills(["gmail", "slack", "summarize"], async (dir) => { + let calls = 0; + const llm: LlmClient = async () => { + calls++; + return UNKNOWN_SKILL_YAML; + }; + const result = await runGenerate({ + flags: { dir, prompt: "text me the weather", dryRun: false, allowMissingSkills: true }, + llm, + }); + assert.equal(calls, 1, "should not retry when the check is disabled"); + assert.ok(result.filePath); + const written = await readFile(result.filePath!, "utf-8"); + assert.ok(written.includes("skill: weather")); + }); +}); + +test("runGenerate skips the skill check when no skills are installed", async () => { + // An agent dir with no skills/ folder is the documented escape hatch: the + // system prompt tells the model to invent sensible names, so rejecting them + // here would make generation impossible on a fresh project. + const dir = await mkdtemp(join(tmpdir(), "gitagent-test-")); + try { + let calls = 0; + const llm: LlmClient = async () => { + calls++; + return UNKNOWN_SKILL_YAML; + }; + const result = await runGenerate({ flags: { dir, prompt: "text me the weather", dryRun: true }, llm }); + assert.equal(calls, 1); + assert.ok(result.yaml.includes("skill: weather")); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +test("approval is accepted as a pseudo-skill even though it is not installed", async () => { + await withInstalledSkills(["analytics", "email"], async (dir) => { + const approvalYaml = `name: daily-sales-report +description: Pull sales data, require approval, then send. +steps: + - id: pull_data + skill: analytics + prompt: Pull yesterday's totals. + - id: approve + skill: approval + prompt: Review the data before it goes out. + requires_approval: true + depends_on: [pull_data] + - skill: email + prompt: Send the approved report. + depends_on: [approve] +`; + let calls = 0; + const llm: LlmClient = async () => { + calls++; + return approvalYaml; + }; + const result = await runGenerate({ flags: { dir, prompt: "sales report with sign-off", dryRun: true }, llm }); + assert.equal(calls, 1, "approval step should not trigger a retry"); + assert.ok(result.yaml.includes("skill: approval")); + }); +}); + +test("runGenerate refine mode reads previous YAML and passes it to the LLM", async () => { + const dir = await mkdtemp(join(tmpdir(), "gitagent-test-")); + try { + const { writeFile, mkdir } = await import("node:fs/promises"); + await mkdir(join(dir, "workflows"), { recursive: true }); + const refinePath = join(dir, "workflows", "starter.yaml"); + await writeFile(refinePath, VALID_YAML, "utf-8"); + + let observed = ""; + const llm: LlmClient = async (messages) => { + observed = messages[messages.length - 1].content; + return VALID_YAML; + }; + await runGenerate({ + flags: { + dir, + prompt: "add an approval step before slack", + refine: "workflows/starter.yaml", + dryRun: true, + }, + llm, + }); + assert.ok(observed.includes("Here is the current workflow")); + assert.ok(observed.includes("add an approval step before slack")); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); diff --git a/test/workflow-validator.test.ts b/test/workflow-validator.test.ts new file mode 100644 index 0000000..c917607 --- /dev/null +++ b/test/workflow-validator.test.ts @@ -0,0 +1,279 @@ +// Unit tests for src/utils/schemas.ts — the SkillFlow workflow validator. + +import test from "node:test"; +import assert from "node:assert/strict"; + +import { validateWorkflow, loadWorkflowSchema, validateSkillReferences } from "../src/utils/schemas.ts"; + +const VALID_YAML = `name: morning-digest +description: Summarize unread emails and post to Slack each morning. +steps: + - skill: gmail + prompt: Fetch unread emails from the last 24h. + - skill: summarize + prompt: Compose a digest grouped by sender priority. + - skill: slack + prompt: Post the digest to the team channel. + channel: "#daily-digest" +`; + +test("loadWorkflowSchema returns the parsed schema with required top-level keys", () => { + const schema = loadWorkflowSchema(); + assert.equal(typeof schema, "object"); + assert.deepEqual(schema.required, ["name", "description", "steps"]); + assert.equal(schema.definitions.step.required.includes("skill"), true); + assert.equal(schema.definitions.step.required.includes("prompt"), true); +}); + +test("validateWorkflow accepts a well-formed workflow", () => { + const r = validateWorkflow(VALID_YAML); + assert.equal(r.valid, true); + assert.deepEqual(r.errors, []); + assert.equal(r.data?.name, "morning-digest"); + assert.equal(r.data?.steps.length, 3); +}); + +test("validateWorkflow rejects missing name", () => { + const yaml = `description: foo +steps: + - skill: gmail + prompt: do thing +`; + const r = validateWorkflow(yaml); + assert.equal(r.valid, false); + assert.ok(r.errors.some((e) => e.includes('missing required property "name"')), `errors: ${JSON.stringify(r.errors)}`); +}); + +test("validateWorkflow rejects non-kebab-case name", () => { + const yaml = `name: MyWorkflow +description: foo +steps: + - skill: gmail + prompt: do thing +`; + const r = validateWorkflow(yaml); + assert.equal(r.valid, false); + assert.ok(r.errors.some((e) => e.includes("pattern")), `errors: ${JSON.stringify(r.errors)}`); +}); + +test("validateWorkflow rejects empty steps array", () => { + const yaml = `name: empty-flow +description: nothing +steps: [] +`; + const r = validateWorkflow(yaml); + assert.equal(r.valid, false); + assert.ok(r.errors.some((e) => e.includes("at least 1")), `errors: ${JSON.stringify(r.errors)}`); +}); + +test("validateWorkflow rejects a step missing required prompt", () => { + const yaml = `name: bad-step +description: missing prompt +steps: + - skill: gmail +`; + const r = validateWorkflow(yaml); + assert.equal(r.valid, false); + assert.ok( + r.errors.some((e) => e.includes('missing required property "prompt"')), + `errors: ${JSON.stringify(r.errors)}`, + ); +}); + +test("validateWorkflow rejects unknown step property", () => { + const yaml = `name: extra-prop +description: bad +steps: + - skill: gmail + prompt: do thing + nonsense: true +`; + const r = validateWorkflow(yaml); + assert.equal(r.valid, false); + assert.ok( + r.errors.some((e) => e.includes('unknown property "nonsense"')), + `errors: ${JSON.stringify(r.errors)}`, + ); +}); + +test("validateWorkflow flags depends_on referencing a missing id", () => { + const yaml = `name: bad-deps +description: dangling dep +steps: + - id: a + skill: gmail + prompt: fetch + - skill: slack + prompt: post + depends_on: [does_not_exist] +`; + const r = validateWorkflow(yaml); + assert.equal(r.valid, false); + assert.ok( + r.errors.some((e) => e.includes('references unknown step id "does_not_exist"')), + `errors: ${JSON.stringify(r.errors)}`, + ); +}); + +test("validateWorkflow flags a self-referencing depends_on", () => { + const yaml = `name: self-cycle +description: step depends on itself +steps: + - id: a + skill: gmail + prompt: fetch + depends_on: [a] +`; + const r = validateWorkflow(yaml); + assert.equal(r.valid, false); + assert.ok(r.errors.some((e) => e.includes("cycle")), `errors: ${JSON.stringify(r.errors)}`); +}); + +test("validateWorkflow flags an A -> B -> A depends_on cycle", () => { + const yaml = `name: mutual-cycle +description: two steps depend on each other +steps: + - id: a + skill: gmail + prompt: fetch + depends_on: [b] + - id: b + skill: slack + prompt: post + depends_on: [a] +`; + const r = validateWorkflow(yaml); + assert.equal(r.valid, false); + assert.ok( + r.errors.some((e) => e.includes("depends_on cycle detected")), + `errors: ${JSON.stringify(r.errors)}`, + ); +}); + +test("validateWorkflow rejects a forward depends_on reference", () => { + // Steps run in declaration order, so depending on a later step can never be + // satisfied. Catching it here keeps validate in step with loadFlowDefinition + // and gives the retry loop a chance to fix it before anything is written. + const yaml = `name: forward-ref +description: first step depends on the second +steps: + - skill: gmail + prompt: fetch + depends_on: [post] + - id: post + skill: slack + prompt: post +`; + const r = validateWorkflow(yaml); + assert.equal(r.valid, false); + assert.ok( + r.errors.some((e) => e.includes('references unknown step id "post"') && e.includes("preceding step")), + `errors: ${JSON.stringify(r.errors)}`, + ); +}); + +test("validateWorkflow accepts a diamond-shaped depends_on graph", () => { + const yaml = `name: diamond-flow +description: fan out then fan in +steps: + - id: root + skill: gmail + prompt: fetch + - id: left + skill: summarize + prompt: summarize inbox + depends_on: [root] + - id: right + skill: summarize + prompt: summarize archive + depends_on: [root] + - id: post + skill: slack + prompt: post both summaries + depends_on: [left, right] +`; + const r = validateWorkflow(yaml); + assert.equal(r.valid, true, `errors: ${JSON.stringify(r.errors)}`); +}); + +test("validateWorkflow accepts approval step with requires_approval", () => { + const yaml = `name: approval-flow +description: needs sign-off +steps: + - id: pull + skill: analytics + prompt: Pull data. + - id: approve + skill: approval + prompt: Approve distribution. + requires_approval: true + depends_on: [pull] + - skill: email + prompt: Send report. + depends_on: [approve] +`; + const r = validateWorkflow(yaml); + assert.equal(r.valid, true, `errors: ${JSON.stringify(r.errors)}`); +}); + +test("validateWorkflow surfaces YAML parse errors", () => { + const yaml = `name: bad +description: : : +steps: + - skill: [unterminated +`; + const r = validateWorkflow(yaml); + assert.equal(r.valid, false); + assert.ok(r.errors[0].startsWith("YAML parse error"), `errors: ${JSON.stringify(r.errors)}`); +}); + +test("validateWorkflow rejects empty document", () => { + const r = validateWorkflow(""); + assert.equal(r.valid, false); + assert.ok(r.errors[0].includes("empty")); +}); + +// ── validateSkillReferences ──────────────────────────────────────────── + +const UNKNOWN_SKILLS_YAML = `name: morning-weather-summary +description: Check the weather each morning and send a text summary. +steps: + - id: fetch_weather + skill: weather + prompt: Fetch today's forecast. + - id: send_summary + skill: sms + prompt: Text a one-sentence summary. + depends_on: [fetch_weather] +`; + +test("validateSkillReferences reports every step naming an uninstalled skill", () => { + const data = validateWorkflow(UNKNOWN_SKILLS_YAML).data!; + const errors = validateSkillReferences(data, ["gmail", "slack", "summarize"]); + assert.deepEqual(errors, [ + 'steps[0].skill: "weather" is not an installed skill', + 'steps[1].skill: "sms" is not an installed skill', + ]); +}); + +test("validateSkillReferences passes when every skill is installed", () => { + const data = validateWorkflow(VALID_YAML).data!; + assert.deepEqual(validateSkillReferences(data, ["gmail", "slack", "summarize"]), []); +}); + +test("validateSkillReferences exempts the approval pseudo-skill", () => { + const yaml = `name: with-approval +description: Approval step uses a pseudo-skill. +steps: + - skill: approval + prompt: Sign off before sending. + requires_approval: true +`; + const data = validateWorkflow(yaml).data!; + assert.deepEqual(validateSkillReferences(data, ["gmail"]), []); +}); + +test("validateSkillReferences is a no-op when no skills are installed", () => { + const data = validateWorkflow(UNKNOWN_SKILLS_YAML).data!; + assert.deepEqual(validateSkillReferences(data, []), []); +}); diff --git a/test/workflows.test.ts b/test/workflows.test.ts new file mode 100644 index 0000000..2e45b1e --- /dev/null +++ b/test/workflows.test.ts @@ -0,0 +1,140 @@ +// Unit tests for loadFlowDefinition in src/workflows.ts — path containment, +// structural guards, and dependency handling. + +import test from "node:test"; +import assert from "node:assert/strict"; +import { mkdtemp, mkdir, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { loadFlowDefinition } from "../src/workflows.ts"; + +async function withAgentDir(files: Record, fn: (dir: string) => Promise): Promise { + const dir = await mkdtemp(join(tmpdir(), "gitagent-flows-")); + try { + await mkdir(join(dir, "workflows"), { recursive: true }); + for (const [name, content] of Object.entries(files)) { + await writeFile(join(dir, "workflows", name), content, "utf-8"); + } + await fn(dir); + } finally { + await rm(dir, { recursive: true, force: true }); + } +} + +const VALID_FLOW = `name: morning-digest +description: Summarize unread emails and post to Slack. +steps: + - id: fetch + skill: gmail + prompt: Fetch unread emails. + - skill: slack + prompt: Post the digest. + channel: "#daily-digest" + depends_on: [fetch] + requires_approval: true +`; + +test("loadFlowDefinition loads a flow by name and preserves id/depends_on/requires_approval", async () => { + await withAgentDir({ "morning-digest.yaml": VALID_FLOW }, async (dir) => { + const flow = await loadFlowDefinition(dir, "morning-digest"); + assert.equal(flow.name, "morning-digest"); + assert.equal(flow.steps.length, 2); + assert.equal(flow.steps[0].id, "fetch"); + assert.deepEqual(flow.steps[1].depends_on, ["fetch"]); + assert.equal(flow.steps[1].requires_approval, true); + assert.equal(flow.steps[1].channel, "#daily-digest"); + }); +}); + +test("loadFlowDefinition rejects non-kebab-case names, including traversal attempts", async () => { + await withAgentDir({ "morning-digest.yaml": VALID_FLOW }, async (dir) => { + for (const bad of ["../../etc/passwd", "/etc/passwd", "..", "Not_Kebab", "with space"]) { + await assert.rejects(() => loadFlowDefinition(dir, bad), /must be kebab-case/, `accepted "${bad}"`); + } + }); +}); + +test("loadFlowDefinition reports a clear error for an empty file", async () => { + await withAgentDir({ "empty.yaml": "" }, async (dir) => { + await assert.rejects(() => loadFlowDefinition(dir, "empty"), /must be a YAML mapping/); + }); +}); + +test("loadFlowDefinition reports a clear error for a scalar document", async () => { + await withAgentDir({ "scalar.yaml": "just a string\n" }, async (dir) => { + await assert.rejects(() => loadFlowDefinition(dir, "scalar"), /must be a YAML mapping/); + }); +}); + +test("loadFlowDefinition coerces a non-string description", async () => { + const flow = `name: numeric-desc +description: 42 +steps: + - skill: gmail + prompt: Fetch. +`; + await withAgentDir({ "numeric-desc.yaml": flow }, async (dir) => { + const loaded = await loadFlowDefinition(dir, "numeric-desc"); + assert.equal(typeof loaded.description, "string"); + assert.equal(loaded.description, "42"); + }); +}); + +test("loadFlowDefinition rejects a step with an empty skill", async () => { + const flow = `name: empty-skill +description: missing skill +steps: + - skill: gmail + prompt: Fetch. + - prompt: Post it somewhere. +`; + await withAgentDir({ "empty-skill.yaml": flow }, async (dir) => { + await assert.rejects(() => loadFlowDefinition(dir, "empty-skill"), /step\[1\] has an empty skill/); + }); +}); + +test("loadFlowDefinition still accepts the legacy single full-path argument", async () => { + await withAgentDir({ "morning-digest.yaml": VALID_FLOW }, async (dir) => { + const legacyPath = join(dir, "workflows", "morning-digest.yaml"); + const flow = await loadFlowDefinition(legacyPath); + assert.equal(flow.name, "morning-digest"); + assert.equal(flow.steps.length, 2); + }); +}); + +test("loadFlowDefinition does not read workflows/undefined.yaml when the name is omitted", async () => { + await withAgentDir({ "morning-digest.yaml": VALID_FLOW }, async (dir) => { + // The legacy form treats the lone argument as a file path, so an agent dir + // passed alone must fail as a missing file rather than silently looking for + // a flow literally named "undefined". + await assert.rejects(() => loadFlowDefinition(dir), (err: any) => { + assert.ok(!String(err.message).includes("undefined"), err.message); + return true; + }); + }); +}); + +test("loadFlowDefinition rejects depends_on that does not name a preceding step", async () => { + const forward = `name: forward-dep +description: depends on a later step +steps: + - skill: gmail + prompt: Fetch. + depends_on: [post] + - id: post + skill: slack + prompt: Post. +`; + const dangling = `name: dangling-dep +description: depends on nothing that exists +steps: + - skill: gmail + prompt: Fetch. + depends_on: [nope] +`; + await withAgentDir({ "forward-dep.yaml": forward, "dangling-dep.yaml": dangling }, async (dir) => { + await assert.rejects(() => loadFlowDefinition(dir, "forward-dep"), /not the id of a preceding step/); + await assert.rejects(() => loadFlowDefinition(dir, "dangling-dep"), /not the id of a preceding step/); + }); +});