diff --git a/.changeset/os-g-skill-scaffolder.md b/.changeset/os-g-skill-scaffolder.md new file mode 100644 index 0000000000..1f59704fba --- /dev/null +++ b/.changeset/os-g-skill-scaffolder.md @@ -0,0 +1,42 @@ +--- +"@objectstack/cli": minor +--- + +feat(cli): `os g skill NAME` scaffolds an AI skill, and writes it as `NAME.skill.ts` so the loader can find it (#11025) + +Completes the second half of the ADR-0063 Option A ruling whose first half +retired `os g agent` (#10359). That retirement left authors told to write +`src/skills/NAME.skill.ts` by hand because no scaffolder existed; this adds it, +and `os g agent`'s refusal, the CLI README and the CLI docs now name the +command instead of apologising for its absence. + +The filename is the point, not a detail. `DEFAULT_METADATA_TYPE_REGISTRY` +declares `skill`'s file convention as `*.skill.ts` / `*.skill.yml`, while this +harness has always written `NAME.ts`. `skill` is `allowRuntimeCreate: true` — +a type the platform expects to discover — so a scaffold matching no pattern +would type-check, validate and publish with nothing anywhere reporting that it +had been skipped: the silent-strip shape the `agent` retirement closed, +re-entering through the scaffolder that replaced it. `skill` therefore +overrides the harness filename through a new per-generator hook, and the +barrel re-export is derived from the file that was actually written rather +than rebuilt from the metadata name. + +**The other six generators are unchanged** and still write `NAME.ts` with a +`'./NAME'` barrel line, pinned by a control assertion in the new test. +Converging the whole scaffolder on the registry's `NAME.TYPE.ts` convention — +the shape the example apps already author in — moves every generator's output +plus the docs and examples that show it, and is deliberately left as its own +decision. + +Three authoring choices the template makes, each written into the generated +file so the next author inherits the reasoning and not just the value: +`tools: []`, because under ADR-0064 an agent's tool set is the union of its +skills' tools with no global fall-through, so an empty list grants nothing +while a placeholder name would resolve to nothing and be reported by +`os validate` as `ai-skill-tool-unresolved`; `surface: 'ask'` written out +rather than left to the schema default, because the affinity it declares is +enforced at load and a default taken in silence is invisible to whoever edits +the file next; and `defineSkill` rather than a bare typed literal, so the +object is parsed at module load. The template is **not** copied from +`SkillSchema`'s or `defineSkill`'s `@example` blocks — both pass +`triggerPhrases`, a retired-key tombstone that rejects on parse (#11026). diff --git a/.changeset/retire-agent-generator.md b/.changeset/retire-agent-generator.md index 1000ac17d8..b5345a8b3c 100644 --- a/.changeset/retire-agent-generator.md +++ b/.changeset/retire-agent-generator.md @@ -41,21 +41,25 @@ What you see: Author a SKILL instead. Skills (plus tools / MCP) are the third-party extension primitive ADR-0063 names — the live surface this one was not. - There is no `os g skill` scaffolder yet. Write the file by hand: + Scaffold one — the file lands where the loader looks for it: - src/skills/.skill.ts - import { defineSkill } from '@objectstack/spec/ai'; + os g skill -> src/skills/.skill.ts + + It writes a `defineSkill` template with `surface` and `tools` filled in + and explained, ready to edit. Docs: https://objectstack.ai/docs/ai/agents ``` -**There is no command to rewrite the call into.** A skill is a different -artifact with a different schema, not a renamed agent, and `os g skill` does not -exist yet — the skill scaffolder is tracked separately, and this message says so -rather than promising it. Delete the call; author the skill file by hand. +**The call is not mechanically rewritable.** A skill is a different artifact +with a different schema, not a renamed agent, so delete the `os g agent` call +rather than renaming it — then run `os g skill` and fill the template in. (This +message originally said no scaffolder existed; `os g skill` shipped in the same +release, so the text above is what the command prints today.) -The generator roster is now `object`, `view`, `action`, `flow`, `dashboard`, -`app`. The docs that advertised the retired one — the `os g agent support` +`agent` leaves the generator roster, which is `object`, `view`, `action`, +`flow`, `dashboard`, `app` — plus `skill`, added in this same release. The docs +that advertised the retired one — the `os g agent support` example, the `agent` / `src/agents/` row of the Available types table, and `os g agent sales-assistant` in the Typical Workflow block — are gone from `content/docs/deployment/cli.mdx`, which carries the retirement note instead; @@ -63,4 +67,4 @@ example, the `agent` / `src/agents/` row of the Available types table, and map, which listed `src/agents` as the directory an app author writes AI metadata into, now names `src/skills`. - + diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index e45f98c8df..33828a7719 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -1080,6 +1080,7 @@ os g action approve # Generate an action os g flow customer # Generate an automation flow os g dashboard sales # Generate a dashboard os g app crm # Generate an app definition +os g skill lead-qual # Generate an AI skill os g object task -d lib/ # Override target directory os g object task --dry-run # Preview without writing @@ -1087,14 +1088,27 @@ os g object task --dry-run # Preview without writing **Available types:** -| Type | Default Directory | Description | -|------|------------------|-------------| -| `object` | `src/objects/` | Business data object with fields | -| `view` | `src/views/` | List or form view definition | -| `action` | `src/actions/` | Button or batch action | -| `flow` | `src/flows/` | Automation flow | -| `dashboard` | `src/dashboards/` | Analytics dashboard | -| `app` | `src/apps/` | Application navigation | +| Type | Default Directory | Written as | Description | +|------|------------------|------------|-------------| +| `object` | `src/objects/` | `NAME.ts` | Business data object with fields | +| `view` | `src/views/` | `NAME.ts` | List or form view definition | +| `action` | `src/actions/` | `NAME.ts` | Button or batch action | +| `flow` | `src/flows/` | `NAME.ts` | Automation flow | +| `dashboard` | `src/dashboards/` | `NAME.ts` | Analytics dashboard | +| `app` | `src/apps/` | `NAME.ts` | Application navigation | +| `skill` | `src/skills/` | `NAME.skill.ts` | AI skill — the ADR-0063 extension primitive | + + +`skill` is the one type whose scaffold is written as `NAME.skill.ts` rather +than `NAME.ts`. The metadata type registry declares that type's file +convention as `*.skill.ts` / `*.skill.yml`, and `skill` is discoverable +metadata — a file matching neither pattern still type-checks, still validates +and still publishes, with nothing anywhere reporting that it was skipped. + +The other six generators keep `NAME.ts`. Aligning the whole scaffolder with +the registry's `NAME.TYPE.ts` convention — the shape the example apps already +author in — would change every generator's output and is a separate decision. + There is no `agent` type. Running `os g agent ` fails with a message @@ -1106,8 +1120,8 @@ Agents are **platform-internal**: the kernel ships exactly two (`ask` and scaffolded `src/agents/*.ts` therefore passed `os validate`, published without complaint, and never appeared — silently. Skills (plus tools / MCP) are the third-party extension primitive, authored as `src/skills/.skill.ts` with -`defineSkill`; see [AI Agents](/docs/ai/agents). There is no `os g skill` -scaffolder yet, so write that file by hand for now. +`defineSkill`; see [AI Agents](/docs/ai/agents). Scaffold one with +`os g skill `, which writes exactly that path. **Options:** diff --git a/packages/cli/README.md b/packages/cli/README.md index 8b88656e12..0e6cce5a8f 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -59,13 +59,20 @@ os compile | `os generate ` | Generate metadata files (alias: `os g`) | | `os create [name]` | Create a new package/plugin/example from template | -Available generate types: `object`, `view`, `action`, `flow`, `dashboard`, `app` +Available generate types: `object`, `view`, `action`, `flow`, `dashboard`, `app`, `skill` `agent` is **retired** (ADR-0063 §2): agents are platform-internal, so a scaffolded `src/agents/*.ts` validated, published and was then filtered out of the runtime catalog without a word. `os g agent` now says so and points at skills — the third-party extension primitive — which are authored as `src/skills/.skill.ts` -with `defineSkill`. There is no `os g skill` scaffolder yet. +with `defineSkill`, and which `os g skill ` scaffolds for you. + +`skill` is the one type written as `NAME.skill.ts` rather than the harness's usual +`NAME.ts`: the metadata type registry declares that type's file convention as +`*.skill.ts` / `*.skill.yml`, and a discoverable-metadata file matching no pattern +type-checks, validates and publishes with nothing reporting that it was skipped. +Aligning the other six generators with the registry's `NAME.TYPE.ts` convention is +a separate decision and is deliberately not made here. ### Cloud — publish & install diff --git a/packages/cli/src/commands/generate.ts b/packages/cli/src/commands/generate.ts index 573e891744..100ba6a2f4 100644 --- a/packages/cli/src/commands/generate.ts +++ b/packages/cli/src/commands/generate.ts @@ -12,6 +12,21 @@ const GENERATORS: Record string; + /** + * Per-generator override for the written file's name. Optional, and today + * exactly one generator sets it — see `skill` below for why that one is + * different and why the divergence was not resolved the other way. + * + * The default is `NAME.ts`, which is what this harness has always written. + * `DEFAULT_METADATA_TYPE_REGISTRY` (`packages/spec`) meanwhile gives every + * metadata type `filePatterns` of the form `NAME.TYPE.ts`, and the example + * apps author that way (`account.object.ts`, `lead.view.ts`). Converging + * the harness on the registry's convention for all seven types is the + * repo-wide change that would settle the mismatch properly — it moves every + * generator's output plus the docs and examples that show it, so it is its + * own decision and deliberately does NOT ride in here (#11025). + */ + fileName?: (name: string) => string; }> = { object: { description: 'Business data object', @@ -161,6 +176,91 @@ const ${toCamelCase(name)}App: UI.App = { }; export default ${toCamelCase(name)}App; +`, + }, + + skill: { + description: 'AI skill (ADR-0063 extension primitive)', + defaultDir: 'src/skills', + /** + * The ONE generator that overrides the harness's `NAME.ts` convention, and + * the reason is not cosmetic. + * + * `DEFAULT_METADATA_TYPE_REGISTRY` declares this type's file convention + * as `*.skill.ts` / `*.skill.yml`, and that declaration is load-bearing: + * `MetadataPlugin._loadFromFileSystem` globs each registered type by its + * own `filePatterns`, `MetadataManager.getTypeInfo` publishes them to + * Studio and every other type-descriptor consumer, and it is the shape + * the example apps and the manifest's own glob keys are written in + * throughout (`account.object.ts`, `lead.view.ts`, and so on for every + * type). + * + * A scaffold written as `lead_qualification.ts` matches NEITHER pattern. + * `skill` is `allowRuntimeCreate: true` — a type the platform expects to + * discover rather than one wired in by hand — and a file no pattern + * matches still type-checks, still passes `os validate` and still + * publishes, with nothing anywhere saying it was skipped. That is the + * silent-strip shape ADR-0063's retirement of `os g agent` closed + * (#10359), re-entering through the scaffolder that replaced it + * (#11025), which is why a template alone would have been worse than no + * generator at all. + * + * Scoped to `skill` rather than fixed for all seven types on purpose: the + * other six write into the same mismatch, but nothing there is + * filesystem-discovered today, and moving the whole harness to + * `NAME.TYPE.ts` moves every existing generator's output plus the docs + * and examples that show it. That is a repo-wide decision of its own — + * see the `fileName` docblock above. + */ + fileName: (name: string) => `${toSnakeCase(name)}.skill.ts`, + generate: (name: string) => `import { defineSkill } from '@objectstack/spec/ai'; + +/** + * ${toTitleCase(name)} Skill + * + * Skills are the third-party AI extension primitive (ADR-0063 §2) — agents are + * platform-internal, so a skill, plus the declarative actions your app already + * ships, is how you give the assistant a new capability. + * + * Authored through \`defineSkill\` rather than as a bare typed literal so the + * object is parsed the moment this module loads: an unknown or retired key is + * a startup error naming the key, not a field that goes missing later. + */ +const ${toCamelCase(name)}Skill = defineSkill({ + name: '${toSnakeCase(name)}', + label: '${toTitleCase(name)}', + description: 'One line on what this skill is for — the model routes on it.', + + // ADR-0063 §3 — the kernel agent surface this skill binds to, enforced at + // load time: 'ask' (the data console), 'build' (the authoring surface), or + // 'both' for a genuinely shared read-only capability. A skill only binds to + // an agent whose surface it matches. 'ask' is also the schema default, so + // writing it changes nothing at runtime; it is here because a default taken + // in silence is a decision the next author cannot see they are inheriting. + surface: 'ask', + + // Injected into the active agent's system prompt, and projected onto the MCP + // \`prompts\` primitive by @objectstack/mcp — the half of a skill that runs in + // every distribution. This is the text the model actually reads. + instructions: 'Explain when this skill applies and how to use its tools.', + + // Empty on purpose, and a complete skill as it stands: it contributes its + // instructions and no tools. Under ADR-0064 an agent's tool set is the union + // of its surface-compatible skills' tools with NO global fall-through, so an + // empty list grants nothing rather than everything. + // + // Fill it with names that resolve — a platform-registered tool, or + // \`action_NAME\` materialised from one of your own declarative actions that + // opts in with \`ai: { exposed: true, description: '…' }\` (ADR-0011/0109). + // A made-up placeholder would be worse than nothing: \`os validate\` reports + // it (\`ai-skill-tool-unresolved\`), and at runtime the reference is dropped + // while the instructions keep promising the capability. + // + // tools: ['action_${toSnakeCase(name)}', 'query_records'], + tools: [], +}); + +export default ${toCamelCase(name)}Skill; `, }, }; @@ -207,10 +307,12 @@ const RETIRED_GENERATORS: Record -> src/skills/.skill.ts', '', - ' src/skills/.skill.ts', - " import { defineSkill } from '@objectstack/spec/ai';", + "It writes a `defineSkill` template with `surface` and `tools` filled in", + 'and explained, ready to edit.', '', 'Docs: https://objectstack.ai/docs/ai/agents', ], @@ -364,7 +466,16 @@ async function runMetadataGeneration(type: string, name: string, flags: { dir?: } const dir = flags.dir || generator.defaultDir; - const fileName = `${toSnakeCase(name)}.ts`; + // `NAME.ts` unless the generator declares otherwise — see the `fileName` + // docblock on GENERATORS, and `skill`, the one type that must not use it. + const fileName = generator.fileName ? generator.fileName(name) : `${toSnakeCase(name)}.ts`; + // The barrel re-export has to name the file that was actually written, so + // it is derived from `fileName` rather than rebuilt from `name`. For the + // six generators that take the default this is byte-identical to the old + // `./${toSnakeCase(name)}`; for `skill` it keeps the `.skill` infix the + // loader's file pattern requires instead of pointing at a module that + // does not exist. + const moduleSpecifier = `./${fileName.replace(/\.ts$/, '')}`; const filePath = path.join(process.cwd(), dir, fileName); console.log(` ${chalk.dim('Type:')} ${chalk.cyan(type)} — ${generator.description}`); @@ -407,15 +518,15 @@ async function runMetadataGeneration(type: string, name: string, flags: { dir?: const indexPath = path.join(process.cwd(), dir, 'index.ts'); if (fs.existsSync(indexPath)) { const indexContent = fs.readFileSync(indexPath, 'utf-8'); - const exportLine = `export { default as ${toCamelCase(name)} } from './${toSnakeCase(name)}';`; - + const exportLine = `export { default as ${toCamelCase(name)} } from '${moduleSpecifier}';`; + if (!indexContent.includes(toCamelCase(name))) { fs.appendFileSync(indexPath, exportLine + '\n'); printSuccess(`Updated ${dir}/index.ts with export`); } } else { // Create barrel index - const exportLine = `export { default as ${toCamelCase(name)} } from './${toSnakeCase(name)}';\n`; + const exportLine = `export { default as ${toCamelCase(name)} } from '${moduleSpecifier}';\n`; fs.writeFileSync(indexPath, exportLine); printSuccess(`Created ${dir}/index.ts`); } diff --git a/packages/cli/test/generate-agent-retired.e2e.test.ts b/packages/cli/test/generate-agent-retired.e2e.test.ts index 0186ad7f37..7f8de57dac 100644 --- a/packages/cli/test/generate-agent-retired.e2e.test.ts +++ b/packages/cli/test/generate-agent-retired.e2e.test.ts @@ -25,9 +25,10 @@ * `printError` writes to stdout. Spawned through `bin/run-dev.js` + tsx so the * suite does not depend on `packages/cli/dist` having been built. * - * ⛔ Not asserted, because it is out of scope and stays that way: that - * `os g skill` exists. It does not — the skill scaffolder is the split-out - * follow-up half, and the message says so rather than promising it. + * The refusal's pointer at skills is asserted here only as TEXT. That the + * command it now names actually exists and writes a loadable file is pinned + * next door, in `generate-skill.e2e.test.ts` (#11025) — when this message was + * first written there was no `os g skill` to point at, and it said so. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; diff --git a/packages/cli/test/generate-skill.e2e.test.ts b/packages/cli/test/generate-skill.e2e.test.ts new file mode 100644 index 0000000000..7326f51826 --- /dev/null +++ b/packages/cli/test/generate-skill.e2e.test.ts @@ -0,0 +1,280 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * PIN (#11025) — `os g skill` writes a file the loader actually FINDS, and + * that file parses. + * + * ## Why "it writes a file" is the wrong assertion + * + * A test asserting only that the command created something passes just as + * happily when the filename matches no `filePatterns` entry — which IS the + * defect. `DEFAULT_METADATA_TYPE_REGISTRY` declares `skill`'s file + * convention as `*.skill.ts` / `*.skill.yml`; + * `MetadataPlugin._loadFromFileSystem` globs each type by its own + * `filePatterns`, `MetadataManager.getTypeInfo` publishes them to every + * type-descriptor consumer, and `skill` is `allowRuntimeCreate: true` — a + * type the platform expects to discover. The harness's own convention is + * `NAME.ts`, so a naive skill scaffold lands as `lead_qualification.ts`, + * matches neither pattern, and then type-checks, validates and publishes with + * nothing anywhere saying it was skipped. That is the silent-strip shape + * ADR-0063's retirement of `os g agent` closed (#10359), re-entering through + * the scaffolder that replaced it. + * + * So the two pins below are: + * + * 1. **the written path is matched by the `skill` entry's REAL patterns**, + * read out of `DEFAULT_METADATA_TYPE_REGISTRY` at test time rather than + * restated here — a copy of the patterns would go stale in exactly the + * direction that hides the bug; and the file under test is DISCOVERED by + * listing the output directory, not assumed, so a generator that wrote the + * wrong name fails the pattern assertion instead of the existence one. + * 2. **the generated file parses**, proven by importing it in a child + * process: the template calls `defineSkill(…)` at module scope, so the + * import IS `SkillSchema.parse`. This is the pin that fails if anyone + * ever copies the template from `SkillSchema`'s or `defineSkill`'s + * `@example` blocks — both pass `triggerPhrases`, a `retiredKey()` + * tombstone that rejects on parse (#11026). + * + * `matchesGlob` comes from `node:path` on purpose. Hand-rolling a glob + * matcher here would re-introduce the restatement the first pin exists to + * avoid, one layer down: the whole point is that nothing in this file decides + * what `**` + `*.skill.ts` mean. + * + * ## The fence this file also holds + * + * The filename override is scoped to `skill` alone; converging all seven + * generators on `NAME.TYPE.ts` is a repo-wide decision that does not ride in + * on a skill template (#11025). `os g object` is therefore exercised here as + * a CONTROL: it must still write `customer.ts`, with a barrel line that still + * says `'./customer'`. + * + * Assertions run against a REAL CHILD PROCESS and read stdout, for the two + * reasons `generate-agent-retired.e2e.test.ts` documents: `process.exitCode` + * inside a vitest worker is not an exit status, and these commands print + * through `utils/format.ts`, which writes to stdout. Spawned through + * `bin/run-dev.js` + tsx, so the suite does not depend on `packages/cli/dist`. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFile } from 'node:child_process'; +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + readdirSync, + rmSync, + symlinkSync, + unlinkSync, + writeFileSync, +} from 'node:fs'; +import { createRequire } from 'node:module'; +import { tmpdir } from 'node:os'; +import { dirname, join, matchesGlob, relative, resolve, sep } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { DEFAULT_METADATA_TYPE_REGISTRY } from '@objectstack/spec/kernel'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const CLI = resolve(HERE, '../bin/run-dev.js'); +const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); + +/** + * Resolved through node_modules rather than by walking up from this file: + * `packages/cli` already depends on `@objectstack/spec`, so the dependency is + * one turbo already knows about, and a package specifier is not a + * cross-package source read. Same reasoning as `migrate-meta.e2e.test.ts`. + */ +const SPEC_PACKAGE_ROOT = dirname(createRequire(import.meta.url).resolve('@objectstack/spec/package.json')); + +/** oclif + tsx cold start, with every command module loaded; ~2-10 s when healthy. */ +const RUN_TIMEOUT_MS = 180_000; + +/** The contract under test, read from the registry — never restated. */ +const SKILL_ENTRY = DEFAULT_METADATA_TYPE_REGISTRY.find(entry => entry.type === 'skill'); + +interface Run { + code: number; + stdout: string; + stderr: string; +} + +function runTsx(args: string[], cwd: string): Promise { + return new Promise((resolvePromise) => { + execFile( + TSX, + args, + { cwd, maxBuffer: 8 * 1024 * 1024, env: { ...process.env, NO_COLOR: '1' } }, + (err, stdout, stderr) => { + resolvePromise({ + // `err.code` is the real exit status; null/undefined means the child + // was signalled — a different failure, never reported as 0. + code: err + ? typeof (err as { code?: unknown }).code === 'number' + ? (err as unknown as { code: number }).code + : 1 + : 0, + stdout: String(stdout), + stderr: String(stderr), + }); + }, + ); + }); +} + +/** `a/b/c.ts` with forward slashes, the spelling a glob pattern is written in. */ +function toPosixRelative(root: string, target: string): string { + return relative(root, target).split(sep).join('/'); +} + +let dir: string; +let specLink: string; + +let generated: Run; +let control: Run; +let parseProbe: Run; + +/** The file the generator actually wrote — listed, not assumed. */ +let writtenName: string; +let writtenSource: string; +let barrelSource: string; + +beforeAll(async () => { + dir = mkdtempSync(join(tmpdir(), 'os-g-skill-')); + + // The generated file imports `@objectstack/spec/ai`, and the parse probe has + // to run it for real. Link the package into the scratch project so ordinary + // node resolution finds it from `dir/src/skills/…`. + mkdirSync(join(dir, 'node_modules', '@objectstack'), { recursive: true }); + specLink = join(dir, 'node_modules', '@objectstack', 'spec'); + symlinkSync(SPEC_PACKAGE_ROOT, specLink, 'dir'); + + // Sequential on purpose: cold tsx starts, each loading every command module, + // in a container several agents share. + generated = await runTsx([CLI, 'g', 'skill', 'lead-qualification'], dir); + control = await runTsx([CLI, 'g', 'object', 'customer'], dir); + + const skillDir = join(dir, 'src', 'skills'); + const produced = existsSync(skillDir) + ? readdirSync(skillDir).filter(f => f !== 'index.ts') + : []; + writtenName = produced[0] ?? ''; + writtenSource = writtenName ? readFileSync(join(skillDir, writtenName), 'utf-8') : ''; + barrelSource = existsSync(join(skillDir, 'index.ts')) + ? readFileSync(join(skillDir, 'index.ts'), 'utf-8') + : ''; + + // Importing the module RUNS `defineSkill(…)`, i.e. `SkillSchema.parse`. A + // non-zero exit here means the scaffold this command ships does not parse. + if (writtenName) { + const probe = join(dir, 'parse-probe.ts'); + writeFileSync( + probe, + `import skill from './src/skills/${writtenName}';\n` + + 'process.stdout.write(JSON.stringify(skill));\n', + ); + parseProbe = await runTsx([probe], dir); + } +}, RUN_TIMEOUT_MS); + +afterAll(() => { + // Unlinked BEFORE the recursive remove, and named explicitly: this symlink + // points at the real `packages/spec` in the checkout, and the one thing that + // must never be ambiguous in a cleanup is whether it can follow it. + try { unlinkSync(specLink); } catch { /* already gone */ } + try { rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } +}); + +describe('[#11025] `os g skill` exists and succeeds', () => { + it('is a real generator, not an unknown type', () => { + expect(generated.code).toBe(0); + expect(generated.stdout).not.toContain('Unknown type:'); + expect(generated.stdout).not.toContain('was retired'); + }); + + it('writes exactly one skill file into `src/skills/`', () => { + expect(writtenName).not.toBe(''); + }); +}); + +describe('[#11025] the written path is one the loader can find', () => { + it('the registry still describes `skill` as filesystem-discovered', () => { + // If either of these ever stops holding, the pin below is measuring + // something other than what it claims — so it says so out loud rather + // than passing quietly. + expect(SKILL_ENTRY).toBeDefined(); + expect(SKILL_ENTRY!.allowRuntimeCreate).toBe(true); + expect(SKILL_ENTRY!.filePatterns.length).toBeGreaterThan(0); + }); + + it('matches a `filePatterns` entry read out of DEFAULT_METADATA_TYPE_REGISTRY', () => { + const relPath = toPosixRelative(dir, join(dir, 'src', 'skills', writtenName)); + const matched = SKILL_ENTRY!.filePatterns.filter(pattern => matchesGlob(relPath, pattern)); + + // The failure message is the point of this assertion: it names the path + // that was written and the patterns it had to satisfy. + expect( + matched.length, + `generated "${relPath}" matches none of ${JSON.stringify(SKILL_ENTRY!.filePatterns)} — ` + + 'it would validate, publish and never load', + ).toBeGreaterThan(0); + }); + + it('is announced to the author by the path it actually took', () => { + expect(generated.stdout).toContain(`src/skills/${writtenName}`); + }); + + it('exports itself from a barrel that names the real module', () => { + // A barrel rebuilt from the metadata name rather than the filename would + // read `'./lead_qualification'` and resolve to nothing. + expect(barrelSource).toContain(`from './${writtenName.replace(/\.ts$/, '')}'`); + }); +}); + +describe('[#11025] the generated skill parses', () => { + it('imports cleanly — `defineSkill` runs at module scope, so this IS the parse', () => { + // The child's whole stderr is deliberately NOT the assertion message. It + // carries a full ZodError with absolute paths, and vitest's stack + // formatter crashes trying to source-map its way through one: measured + // while reverse-verifying this pin, the run still failed but reported an + // unhandled "Test Run Error" with blank counts instead of naming the + // assertion that caught the defect. One line says what happened, and a + // red that names itself is worth more here than a full dump. + const headline = parseProbe.stderr.split('\n').find(line => line.includes('Error')) ?? ''; + expect(parseProbe.code, headline.slice(0, 200)).toBe(0); + }); + + it('produces a skill whose machine name and surface survive the parse', () => { + const parsed = JSON.parse(parseProbe.stdout) as Record; + expect(parsed.name).toBe('lead_qualification'); + expect(parsed.surface).toBe('ask'); + expect(parsed.tools).toEqual([]); + }); + + it('carries no `triggerPhrases` — the retired key both spec `@example` blocks still pass', () => { + // Textual, deliberately: the parse above already refuses the tombstone, so + // this assertion exists to name the specific copy-source hazard (#11026) + // for whoever edits the template next. + expect(writtenSource).not.toContain('triggerPhrases'); + }); + + it('writes `surface` and `tools` out rather than leaning on the schema defaults', () => { + // `surface` defaults to 'ask' and would parse identically if omitted, so + // the parsed value above cannot tell whether the template emits the key. + // This can. + expect(writtenSource).toMatch(/^\s*surface: 'ask',$/m); + expect(writtenSource).toMatch(/^\s*tools: \[\],$/m); + }); +}); + +describe('[#11025] the harness convention is unchanged for the other generators', () => { + it('`os g object` still writes `NAME.ts`, with no type infix', () => { + expect(control.code).toBe(0); + expect(existsSync(join(dir, 'src', 'objects', 'customer.ts'))).toBe(true); + expect(existsSync(join(dir, 'src', 'objects', 'customer.object.ts'))).toBe(false); + }); + + it('and its barrel line is byte-identical to what it was before', () => { + const objectBarrel = readFileSync(join(dir, 'src', 'objects', 'index.ts'), 'utf-8'); + expect(objectBarrel).toContain("export { default as customer } from './customer';"); + }); +});