From 26bbdac873708f4137c7cb073b8371042bdc9ea3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 13:04:03 +0000 Subject: [PATCH 1/2] feat(cli): `os g skill` scaffolds an AI skill as NAME.skill.ts (#11025) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second half of the ADR-0063 Option A ruling whose first half retired `os g agent` (#10359). That retirement told authors to write src/skills/NAME.skill.ts by hand because no scaffolder existed; this adds one, and the refusal message, the CLI README and the CLI docs now name the command instead of apologising for its absence. The filename is the point. DEFAULT_METADATA_TYPE_REGISTRY declares skill's file convention as *.skill.ts / *.skill.yml while this harness has always written NAME.ts, and skill is allowRuntimeCreate: true — a type the platform expects to discover. A scaffold matching no pattern would type-check, validate and publish with nothing reporting that it was skipped: the silent-strip shape the agent retirement closed, re-entering through the scaffolder that replaced it. So skill overrides the harness filename through a new per-generator hook, and the barrel re-export is derived from the file 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 is a repo-wide decision and is deliberately not taken here. Template decisions, each with its reasoning written into the generated file: tools: [] (ADR-0064 removed the global fall-through, so an empty list grants nothing, while a placeholder name resolves to nothing and is reported as ai-skill-tool-unresolved); surface: 'ask' written out rather than left to the schema default, because the affinity is enforced at load and a silent default is invisible to the next author; and defineSkill rather than a bare typed literal, so the object is parsed at module load. Not copied from SkillSchema or defineSkill's @example blocks — both pass triggerPhrases, a retired-key tombstone that rejects on parse (#11026). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r --- .changeset/os-g-skill-scaffolder.md | 42 +++ content/docs/deployment/cli.mdx | 34 ++- packages/cli/README.md | 11 +- packages/cli/src/commands/generate.ts | 125 +++++++- .../test/generate-agent-retired.e2e.test.ts | 7 +- packages/cli/test/generate-skill.e2e.test.ts | 280 ++++++++++++++++++ 6 files changed, 477 insertions(+), 22 deletions(-) create mode 100644 .changeset/os-g-skill-scaffolder.md create mode 100644 packages/cli/test/generate-skill.e2e.test.ts 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/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';"); + }); +}); From 6a8fbbc43176de9c474fef964c140057b5d69855 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 14:05:04 +0000 Subject: [PATCH 2/2] docs(changeset): the agent-retirement note stops saying no skill scaffolder exists (#11025) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `.changeset/retire-agent-generator.md` is unconsumed, so it compiles into the same release notes as this card's own changeset. It quoted the `os g agent` refusal verbatim, and this card changed that output; it also stated "there is no command to rewrite the call into", "`os g skill` does not exist yet", and a six-entry generator roster. All three ship as customer-facing text in the release that adds the seventh. Bounded in-place repair, same defect class as the card (the sentences that go stale), mechanical, no other claim on the file, and inside the changeset gate family this diff already runs. The quoted block is now the output the command actually prints, measured. The ADR-0087 disposition is unchanged — a CLI command name is still not authorable metadata and there is still no FROM -> TO rewrite to prescribe, since a skill is a different artifact and not a renamed agent. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r --- .changeset/retire-agent-generator.md | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) 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`. - +