From 192c29bfbc3adfdc5c868dd6dde87fe6e20842f6 Mon Sep 17 00:00:00 2001 From: Matic Jurglic Date: Tue, 23 Jun 2026 09:53:25 +0200 Subject: [PATCH 1/4] Add migrate-skill host command to convert legacy Skill cards to SKILL.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a `migrate-skill` host command that scans a realm for legacy `Skill` cards (and subclasses) and writes each one out as a `skills//SKILL.md` file with `boxel.kind: skill` frontmatter — the markdown-first skill representation the platform now indexes via `MarkdownDef.frontmatter` / `SkillFrontmatterField`. For each skill the command emits the shared top-level `name`/`description` keys plus a `boxel:` namespace carrying `kind: skill` and the skill's `commands` (codeRef + requiresApproval) in the same shape `SkillFrontmatterField.commands` parses back out. Instructions become the markdown body. Targets that already exist are skipped unless `overwrite` is set, and slug collisions are de-duplicated within a run. CS-11549 Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/base/command.gts | 18 ++ packages/host/app/commands/index.ts | 6 + packages/host/app/commands/migrate-skill.ts | 168 +++++++++++++ .../commands/migrate-skill-test.gts | 229 ++++++++++++++++++ 4 files changed, 421 insertions(+) create mode 100644 packages/host/app/commands/migrate-skill.ts create mode 100644 packages/host/tests/integration/commands/migrate-skill-test.gts diff --git a/packages/base/command.gts b/packages/base/command.gts index ff19b8cb5b1..d45c48cd78b 100644 --- a/packages/base/command.gts +++ b/packages/base/command.gts @@ -159,6 +159,24 @@ export class WriteTextFileInput extends CardDef { @field useNonConflictingFilename = contains(BooleanField); } +export class MigrateSkillInput extends CardDef { + // The realm to migrate: legacy `Skill` cards are read from it and the + // resulting `skills//SKILL.md` files are written back into it. + @field realm = contains(StringField); + // Overwrite an existing `SKILL.md` at the target path. When false (default), + // a skill whose target already exists is left untouched and reported as + // skipped. + @field overwrite = contains(BooleanField); +} + +export class MigrateSkillResult extends CardDef { + // Absolute URLs of the `SKILL.md` files written by this run. + @field migratedFiles = containsMany(StringField); + // Ids of legacy `Skill` cards skipped because their target `SKILL.md` + // already exists and `overwrite` was not set. + @field skippedSkillIds = containsMany(StringField); +} + export class WriteBinaryFileInput extends CardDef { @field path = contains(StringField); @field realm = contains(StringField); diff --git a/packages/host/app/commands/index.ts b/packages/host/app/commands/index.ts index 07c71a0eafe..b762bbb61c8 100644 --- a/packages/host/app/commands/index.ts +++ b/packages/host/app/commands/index.ts @@ -45,6 +45,7 @@ import * as InvalidateRealmIdentifiersCommandModule from './invalidate-realm-ide import * as InviteUserToRoomCommandModule from './invite-user-to-room'; import * as LintAndFixCommandModule from './lint-and-fix'; import * as ListingBuildCommandModule from './listing-action-build'; +import * as MigrateSkillCommandModule from './migrate-skill'; import * as OneShotLlmRequestCommandModule from './one-shot-llm-request'; import * as OpenAiAssistantRoomCommandModule from './open-ai-assistant-room'; import * as OpenCreateListingModalCommandModule from './open-create-listing-modal'; @@ -209,6 +210,10 @@ export function shimHostCommands(virtualNetwork: VirtualNetwork) { '@cardstack/boxel-host/commands/listing-action-build', ListingBuildCommandModule, ); + virtualNetwork.shimModule( + '@cardstack/boxel-host/commands/migrate-skill', + MigrateSkillCommandModule, + ); virtualNetwork.shimModule( '@cardstack/boxel-host/commands/create-listing-pr-request', CreateListingPRRequestCommandModule, @@ -567,6 +572,7 @@ export const HostCommandClasses: (typeof HostBaseCommand)[] = [ UpdateRoomSkillsCommandModule.default, UseAiAssistantCommandModule.default, ValidateRealmCommandModule.default, + MigrateSkillCommandModule.default, WriteBinaryFileCommandModule.default, WriteTextFileCommandModule.default, ]; diff --git a/packages/host/app/commands/migrate-skill.ts b/packages/host/app/commands/migrate-skill.ts new file mode 100644 index 00000000000..39463d5cb33 --- /dev/null +++ b/packages/host/app/commands/migrate-skill.ts @@ -0,0 +1,168 @@ +import { service } from '@ember/service'; + +import { stringify as stringifyYaml } from 'yaml'; + +import { rri, skillCardRef } from '@cardstack/runtime-common'; + +import type * as BaseCommandModule from 'https://cardstack.com/base/command'; +import type { Skill } from 'https://cardstack.com/base/skill'; + +import HostBaseCommand from '../lib/host-base-command'; + +import type CardService from '../services/card-service'; +import type RealmService from '../services/realm'; +import type StoreService from '../services/store'; + +// A single command in the migrated frontmatter — the same shape +// `SkillFrontmatterField.commands` (a `containsMany(CommandField)`) parses back +// out of `boxel.commands`. +interface FrontmatterCommand { + codeRef: { module: string; name: string }; + requiresApproval?: boolean; +} + +// Convert a skill name into a directory-safe slug for `skills//SKILL.md`. +function slugify(text: string): string { + return text + .toLowerCase() + .trim() + .replace(/[^\w\s-]/g, '') + .replace(/[\s_]+/g, '-') + .replace(/-+/g, '-') + .replace(/^-+|-+$/g, ''); +} + +// Last path segment of a card id, minus its extension — a stable slug fallback +// when a skill has no usable name. +function basenameSlug(id: string): string { + let pathname: string; + try { + pathname = new URL(id).pathname; + } catch { + pathname = id; + } + let name = pathname.split('/').pop() ?? ''; + return slugify(name.replace(/\.[^/.]+$/, '')); +} + +export default class MigrateSkillCommand extends HostBaseCommand< + typeof BaseCommandModule.MigrateSkillInput, + typeof BaseCommandModule.MigrateSkillResult +> { + @service declare private cardService: CardService; + @service declare private realm: RealmService; + @service declare private store: StoreService; + + description = `Migrate a realm's legacy Skill cards into skills//SKILL.md \ +files with boxel.kind: skill frontmatter.`; + static actionVerb = 'Migrate'; + + async getInputType() { + let commandModule = await this.loadCommandModule(); + const { MigrateSkillInput } = commandModule; + return MigrateSkillInput; + } + + requireInputFields = ['realm']; + + protected async run( + input: BaseCommandModule.MigrateSkillInput, + ): Promise { + let realmUrl = this.realm.realmOf(rri(input.realm)); + if (!realmUrl) { + throw new Error(`Invalid or unknown realm provided: ${input.realm}`); + } + + // The `type` filter matches the legacy `Skill` card and its subclasses + // (e.g. `SkillPlus`, `SkillPlusMarkdown`), so every flavour of legacy skill + // in the realm is migrated. + let skills = await this.store.search( + { filter: { type: skillCardRef } }, + [realmUrl], + ); + + let migratedFiles: string[] = []; + let skippedSkillIds: string[] = []; + let usedSlugs = new Set(); + + for (let skill of skills) { + let slug = this.slugForSkill(skill, usedSlugs); + usedSlugs.add(slug); + + let url = new URL(`skills/${slug}/SKILL.md`, realmUrl); + + if (!input.overwrite && (await this.fileExists(url))) { + if (skill.id) { + skippedSkillIds.push(skill.id); + } + continue; + } + + let content = this.buildSkillMarkdown(skill); + await this.cardService.saveSource( + url, + content, + input.overwrite ? 'editor' : 'create-file', + ); + migratedFiles.push(url.href); + } + + let commandModule = await this.loadCommandModule(); + const { MigrateSkillResult } = commandModule; + return new MigrateSkillResult({ migratedFiles, skippedSkillIds }); + } + + private slugForSkill(skill: Skill, usedSlugs: Set): string { + let name = skill.cardTitle ?? ''; + let base = slugify(name) || basenameSlug(skill.id ?? '') || 'skill'; + let slug = base; + let suffix = 2; + while (usedSlugs.has(slug)) { + slug = `${base}-${suffix++}`; + } + return slug; + } + + private buildSkillMarkdown(skill: Skill): string { + let commands = (skill.commands ?? []).reduce( + (acc, command) => { + let module = command.codeRef?.module; + let name = command.codeRef?.name; + if (module && name) { + let entry: FrontmatterCommand = { codeRef: { module, name } }; + if (command.requiresApproval) { + entry.requiresApproval = true; + } + acc.push(entry); + } + return acc; + }, + [], + ); + + // Shared top-level keys (read byte-for-byte by Claude Code) first, then the + // Boxel-only `boxel:` namespace that carries `kind` and `commands`. + let frontmatter: Record = { + name: skill.cardTitle ?? '', + description: skill.cardDescription ?? '', + boxel: { + kind: 'skill', + ...(commands.length > 0 ? { commands } : {}), + }, + }; + + let body = (skill.instructions ?? '').trim(); + return `---\n${stringifyYaml(frontmatter)}---\n\n${body}\n`; + } + + private async fileExists(url: URL): Promise { + let { status } = await this.cardService.getSource(url); + if (status === 404) { + return false; + } + if (status === 200 || status === 406) { + return true; + } + throw new Error(`Error checking if file exists at ${url}: ${status}`); + } +} diff --git a/packages/host/tests/integration/commands/migrate-skill-test.gts b/packages/host/tests/integration/commands/migrate-skill-test.gts new file mode 100644 index 00000000000..00c2b6cb790 --- /dev/null +++ b/packages/host/tests/integration/commands/migrate-skill-test.gts @@ -0,0 +1,229 @@ +import { getOwner } from '@ember/owner'; +import type { RenderingTestContext } from '@ember/test-helpers'; + +import { getService } from '@universal-ember/test-support'; +import { module, test } from 'qunit'; + +import { parse as parseYaml } from 'yaml'; + +import { baseRealm } from '@cardstack/runtime-common'; +import type { Loader } from '@cardstack/runtime-common/loader'; + +import MigrateSkillCommand from '@cardstack/host/commands/migrate-skill'; +import RealmService from '@cardstack/host/services/realm'; + +import { + setupCardLogs, + setupIntegrationTestRealm, + setupLocalIndexing, + setupOnSave, + testRealmInfo, + testRealmURL, + setupRealmCacheTeardown, + withCachedRealmSetup, +} from '../../helpers'; +import { setupBaseRealm, CommandField, Skill } from '../../helpers/base-realm'; +import { setupMockMatrix } from '../../helpers/mock-matrix'; +import { setupRenderingTest } from '../../helpers/setup'; + +class StubRealmService extends RealmService { + get defaultReadableRealm() { + return { + path: testRealmURL, + info: testRealmInfo, + }; + } + + realmOf(input: URL | string) { + let str = input instanceof URL ? input.href : input; + if (str === testRealmURL) { + return testRealmURL as ReturnType; + } + return undefined; + } +} + +// Split a `--- … ---` frontmatter block off the front of a SKILL.md and parse +// the YAML, so assertions can read the structured frontmatter rather than match +// exact YAML formatting. +function readFrontmatter(content: string): { + data: Record; + body: string; +} { + let match = /^---\n([\s\S]*?)\n---\n?/.exec(content); + if (!match) { + return { data: {}, body: content }; + } + return { + data: (parseYaml(match[1]) as Record) ?? {}, + body: content.slice(match[0].length), + }; +} + +const COMMAND_MODULE = `${testRealmURL}test-command.gts`; + +module('Integration | commands | migrate-skill', function (hooks) { + setupRenderingTest(hooks); + setupLocalIndexing(hooks); + let mockMatrixUtils = setupMockMatrix(hooks); + setupBaseRealm(hooks); + setupOnSave(hooks); + + let loader: Loader; + + hooks.beforeEach(function (this: RenderingTestContext) { + getOwner(this)!.register('service:realm', StubRealmService); + loader = getService('loader-service').loader; + }); + + setupCardLogs( + hooks, + async () => await loader.import(`${baseRealm.url}card-api`), + ); + + setupRealmCacheTeardown(hooks); + + hooks.beforeEach(async function () { + await withCachedRealmSetup(async () => + setupIntegrationTestRealm({ + mockMatrixUtils, + realmURL: testRealmURL, + contents: { + 'test-command.gts': `import { Command } from '@cardstack/runtime-common'; + +export class DoThing extends Command { + static displayName = 'Test Command'; + async getInputType() { + return undefined; + } +}`, + 'Skill/data-management.json': new Skill({ + cardTitle: 'Data Management', + cardDescription: 'Manage data in a realm', + instructions: '# Data\n\nDo data things.', + commands: [ + new CommandField({ + codeRef: { module: COMMAND_MODULE, name: 'DoThing' }, + requiresApproval: true, + }), + ], + }), + 'Skill/no-commands.json': new Skill({ + cardTitle: 'No Commands', + cardDescription: 'A skill without commands', + instructions: 'Just instructions.', + }), + }, + }), + ); + }); + + test('migrates a Skill card with commands into a SKILL.md', async function (assert) { + let commandContext = getService('command-service').commandContext; + let cardService = getService('card-service'); + let command = new MigrateSkillCommand(commandContext); + + let result = await command.execute({ realm: testRealmURL }); + + let skillUrl = `${testRealmURL}skills/data-management/SKILL.md`; + assert.true( + result.migratedFiles.includes(skillUrl), + 'the data-management SKILL.md is reported as migrated', + ); + + let { data, body } = readFrontmatter( + (await cardService.getSource(new URL(skillUrl))).content, + ); + assert.strictEqual(data.name, 'Data Management', 'top-level name is set'); + assert.strictEqual( + data.description, + 'Manage data in a realm', + 'top-level description is set', + ); + assert.strictEqual(data.boxel.kind, 'skill', 'boxel.kind is skill'); + assert.deepEqual( + data.boxel.commands, + [ + { + codeRef: { module: COMMAND_MODULE, name: 'DoThing' }, + requiresApproval: true, + }, + ], + 'the command round-trips into boxel.commands', + ); + assert.strictEqual( + body.trim(), + '# Data\n\nDo data things.', + 'the instructions become the markdown body', + ); + }); + + test('omits boxel.commands when the skill has none', async function (assert) { + let commandContext = getService('command-service').commandContext; + let cardService = getService('card-service'); + let command = new MigrateSkillCommand(commandContext); + + await command.execute({ realm: testRealmURL }); + + let { data } = readFrontmatter( + ( + await cardService.getSource( + new URL(`${testRealmURL}skills/no-commands/SKILL.md`), + ) + ).content, + ); + assert.strictEqual(data.boxel.kind, 'skill', 'boxel.kind is skill'); + assert.notOk( + 'commands' in data.boxel, + 'no commands key when the skill has none', + ); + }); + + test('skips existing targets unless overwrite is set', async function (assert) { + let commandContext = getService('command-service').commandContext; + let cardService = getService('card-service'); + let command = new MigrateSkillCommand(commandContext); + + let first = await command.execute({ realm: testRealmURL }); + assert.strictEqual( + first.migratedFiles.length, + 2, + 'both skills migrate on the first run', + ); + + let second = await command.execute({ realm: testRealmURL }); + assert.strictEqual( + second.migratedFiles.length, + 0, + 'nothing is rewritten on the second run', + ); + assert.strictEqual( + second.skippedSkillIds.length, + 2, + 'both skills are reported as skipped', + ); + + // Overwrite re-migrates even though the targets already exist. + let third = await command.execute({ realm: testRealmURL, overwrite: true }); + assert.strictEqual( + third.migratedFiles.length, + 2, + 'both skills migrate again with overwrite', + ); + assert.strictEqual( + third.skippedSkillIds.length, + 0, + 'nothing is skipped with overwrite', + ); + + // The overwritten content is still well-formed. + let { data } = readFrontmatter( + ( + await cardService.getSource( + new URL(`${testRealmURL}skills/data-management/SKILL.md`), + ) + ).content, + ); + assert.strictEqual(data.boxel.kind, 'skill'); + }); +}); From 64fe7a77b0582ab653adb81a87a6584278edda0e Mon Sep 17 00:00:00 2001 From: Matic Jurglic Date: Tue, 23 Jun 2026 11:03:18 +0200 Subject: [PATCH 2/4] Register realm-service stub before realm helpers in migrate-skill test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `service:realm` stub was registered after `setupBaseRealm` / `setupMockMatrix`, whose beforeEach hooks look up `service:realm` first and instantiate the real singleton — so the later `register` never took effect and `realmOf` ran unstubbed. Resolution then depended on whether the realm happened to be known to the real service, which held only for the first test (cached setup) and failed after teardown. Move the registration ahead of those helpers, matching update-room-skills-test. CS-11549 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../integration/commands/migrate-skill-test.gts | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/host/tests/integration/commands/migrate-skill-test.gts b/packages/host/tests/integration/commands/migrate-skill-test.gts index 00c2b6cb790..74ecaa722dd 100644 --- a/packages/host/tests/integration/commands/migrate-skill-test.gts +++ b/packages/host/tests/integration/commands/migrate-skill-test.gts @@ -63,19 +63,24 @@ function readFrontmatter(content: string): { const COMMAND_MODULE = `${testRealmURL}test-command.gts`; module('Integration | commands | migrate-skill', function (hooks) { - setupRenderingTest(hooks); - setupLocalIndexing(hooks); - let mockMatrixUtils = setupMockMatrix(hooks); - setupBaseRealm(hooks); - setupOnSave(hooks); - let loader: Loader; + setupRenderingTest(hooks); + + // Register the realm-service stub before the realm/base-realm/matrix helpers + // run, so their `lookup('service:realm')` resolves the stub rather than + // instantiating the real singleton first (which would leave `realmOf` + // unstubbed and realm resolution test-order dependent). hooks.beforeEach(function (this: RenderingTestContext) { getOwner(this)!.register('service:realm', StubRealmService); loader = getService('loader-service').loader; }); + setupLocalIndexing(hooks); + let mockMatrixUtils = setupMockMatrix(hooks); + setupBaseRealm(hooks); + setupOnSave(hooks); + setupCardLogs( hooks, async () => await loader.import(`${baseRealm.url}card-api`), From c120d633bc87bd4b872e2fb184ef0214bc278237 Mon Sep 17 00:00:00 2001 From: Matic Jurglic Date: Tue, 23 Jun 2026 12:12:15 +0200 Subject: [PATCH 3/4] Harden migrate-skill: deterministic slugs, guard empty instructions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two robustness fixes from a review pass: - Sort skills by id before assigning slugs, so collision suffixes (-2/-3) are stable across runs and the skip-if-exists check stays idempotent rather than minting duplicate files when search order shifts. - Skip and report (new `emptySkillIds`) skills with no instructions instead of writing an empty SKILL.md. This guards markdown-backed subclasses (e.g. SkillPlusMarkdown), whose `instructions` is computed from a linked file that may not resolve in a search result — avoiding silent data loss. CS-11549 Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/base/command.gts | 4 +++ packages/host/app/commands/migrate-skill.ts | 29 ++++++++++++++++--- .../commands/migrate-skill-test.gts | 26 +++++++++++++++++ 3 files changed, 55 insertions(+), 4 deletions(-) diff --git a/packages/base/command.gts b/packages/base/command.gts index d45c48cd78b..3b26c0e7313 100644 --- a/packages/base/command.gts +++ b/packages/base/command.gts @@ -175,6 +175,10 @@ export class MigrateSkillResult extends CardDef { // Ids of legacy `Skill` cards skipped because their target `SKILL.md` // already exists and `overwrite` was not set. @field skippedSkillIds = containsMany(StringField); + // Ids of skills skipped because they had no instructions to write — e.g. a + // markdown-backed skill whose linked instructions did not resolve. Reported + // rather than written out as an empty `SKILL.md`. + @field emptySkillIds = containsMany(StringField); } export class WriteBinaryFileInput extends CardDef { diff --git a/packages/host/app/commands/migrate-skill.ts b/packages/host/app/commands/migrate-skill.ts index 39463d5cb33..ff577af0ee9 100644 --- a/packages/host/app/commands/migrate-skill.ts +++ b/packages/host/app/commands/migrate-skill.ts @@ -81,11 +81,29 @@ files with boxel.kind: skill frontmatter.`; [realmUrl], ); + // Sort by id so slug de-duplication is deterministic: re-running the command + // assigns the same `-2`/`-3` suffixes in the same order, which keeps the + // skip-if-exists check stable instead of producing fresh duplicates. + skills.sort((a, b) => (a.id ?? '').localeCompare(b.id ?? '')); + let migratedFiles: string[] = []; let skippedSkillIds: string[] = []; + let emptySkillIds: string[] = []; let usedSlugs = new Set(); for (let skill of skills) { + // Skip — and report — skills with nothing to transcribe rather than + // writing an empty `SKILL.md`. This guards the markdown-backed subclasses + // (e.g. `SkillPlusMarkdown`), whose `instructions` is computed from a + // linked file that may not have resolved in the search result. + let body = (skill.instructions ?? '').trim(); + if (!body) { + if (skill.id) { + emptySkillIds.push(skill.id); + } + continue; + } + let slug = this.slugForSkill(skill, usedSlugs); usedSlugs.add(slug); @@ -98,7 +116,7 @@ files with boxel.kind: skill frontmatter.`; continue; } - let content = this.buildSkillMarkdown(skill); + let content = this.buildSkillMarkdown(skill, body); await this.cardService.saveSource( url, content, @@ -109,7 +127,11 @@ files with boxel.kind: skill frontmatter.`; let commandModule = await this.loadCommandModule(); const { MigrateSkillResult } = commandModule; - return new MigrateSkillResult({ migratedFiles, skippedSkillIds }); + return new MigrateSkillResult({ + migratedFiles, + skippedSkillIds, + emptySkillIds, + }); } private slugForSkill(skill: Skill, usedSlugs: Set): string { @@ -123,7 +145,7 @@ files with boxel.kind: skill frontmatter.`; return slug; } - private buildSkillMarkdown(skill: Skill): string { + private buildSkillMarkdown(skill: Skill, body: string): string { let commands = (skill.commands ?? []).reduce( (acc, command) => { let module = command.codeRef?.module; @@ -151,7 +173,6 @@ files with boxel.kind: skill frontmatter.`; }, }; - let body = (skill.instructions ?? '').trim(); return `---\n${stringifyYaml(frontmatter)}---\n\n${body}\n`; } diff --git a/packages/host/tests/integration/commands/migrate-skill-test.gts b/packages/host/tests/integration/commands/migrate-skill-test.gts index 74ecaa722dd..6bb2d966daf 100644 --- a/packages/host/tests/integration/commands/migrate-skill-test.gts +++ b/packages/host/tests/integration/commands/migrate-skill-test.gts @@ -118,6 +118,11 @@ export class DoThing extends Command { cardDescription: 'A skill without commands', instructions: 'Just instructions.', }), + 'Skill/empty.json': new Skill({ + cardTitle: 'Empty Skill', + cardDescription: 'A skill with no instructions', + instructions: ' ', + }), }, }), ); @@ -184,6 +189,27 @@ export class DoThing extends Command { ); }); + test('reports skills with no instructions instead of writing an empty file', async function (assert) { + let commandContext = getService('command-service').commandContext; + let cardService = getService('card-service'); + let command = new MigrateSkillCommand(commandContext); + + let result = await command.execute({ realm: testRealmURL }); + + assert.true( + result.emptySkillIds.includes(`${testRealmURL}Skill/empty`), + 'the empty skill is reported in emptySkillIds', + ); + assert.notOk( + result.migratedFiles.some((f: string) => f.includes('/empty/')), + 'no SKILL.md is written for the empty skill', + ); + let { status } = await cardService.getSource( + new URL(`${testRealmURL}skills/empty/SKILL.md`), + ); + assert.strictEqual(status, 404, 'the empty skill target does not exist'); + }); + test('skips existing targets unless overwrite is set', async function (assert) { let commandContext = getService('command-service').commandContext; let cardService = getService('card-service'); From 8d797a051bbbb63fedb34fce1fdd8603e4c020a1 Mon Sep 17 00:00:00 2001 From: Matic Jurglic Date: Wed, 24 Jun 2026 12:02:13 +0200 Subject: [PATCH 4/4] Preserve explicit requiresApproval: false when migrating skill commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The frontmatter only carried requiresApproval when truthy, so a command with an explicit `requiresApproval: false` lost it. Downstream the host auto-executes a command only when `requiresApproval === false` (command-auto-execute.ts) and otherwise defaults a missing value to `true` (message-builder.ts) — so the dropped `false` silently flipped an auto-executing command back to approval-required. Emit `requiresApproval` explicitly for every command, defaulting a missing source value to `true` to match the downstream default. Test now covers both an approval-required and an auto-execute command and asserts the explicit `false` survives migration. CS-11549 Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/host/app/commands/migrate-skill.ts | 17 +++++++++++------ .../commands/migrate-skill-test.gts | 18 +++++++++++++++++- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/packages/host/app/commands/migrate-skill.ts b/packages/host/app/commands/migrate-skill.ts index ff577af0ee9..f450852cd73 100644 --- a/packages/host/app/commands/migrate-skill.ts +++ b/packages/host/app/commands/migrate-skill.ts @@ -18,7 +18,13 @@ import type StoreService from '../services/store'; // out of `boxel.commands`. interface FrontmatterCommand { codeRef: { module: string; name: string }; - requiresApproval?: boolean; + // Always emitted explicitly. The host auto-executes a command only when + // `requiresApproval === false` (`command-auto-execute.ts`) and otherwise + // treats a missing value as `true` (`message-builder.ts`), so dropping an + // explicit `false` would silently flip an auto-executing command back to + // approval-required. Preserve the source value, defaulting a missing one to + // `true` to match that downstream behavior. + requiresApproval: boolean; } // Convert a skill name into a directory-safe slug for `skills//SKILL.md`. @@ -151,11 +157,10 @@ files with boxel.kind: skill frontmatter.`; let module = command.codeRef?.module; let name = command.codeRef?.name; if (module && name) { - let entry: FrontmatterCommand = { codeRef: { module, name } }; - if (command.requiresApproval) { - entry.requiresApproval = true; - } - acc.push(entry); + acc.push({ + codeRef: { module, name }, + requiresApproval: command.requiresApproval ?? true, + }); } return acc; }, diff --git a/packages/host/tests/integration/commands/migrate-skill-test.gts b/packages/host/tests/integration/commands/migrate-skill-test.gts index 6bb2d966daf..918d6b3aa52 100644 --- a/packages/host/tests/integration/commands/migrate-skill-test.gts +++ b/packages/host/tests/integration/commands/migrate-skill-test.gts @@ -101,6 +101,13 @@ export class DoThing extends Command { async getInputType() { return undefined; } +} + +export class DoThingQuietly extends Command { + static displayName = 'Test Command (no approval)'; + async getInputType() { + return undefined; + } }`, 'Skill/data-management.json': new Skill({ cardTitle: 'Data Management', @@ -111,6 +118,11 @@ export class DoThing extends Command { codeRef: { module: COMMAND_MODULE, name: 'DoThing' }, requiresApproval: true, }), + // requiresApproval: false must survive migration — see assertion. + new CommandField({ + codeRef: { module: COMMAND_MODULE, name: 'DoThingQuietly' }, + requiresApproval: false, + }), ], }), 'Skill/no-commands.json': new Skill({ @@ -158,8 +170,12 @@ export class DoThing extends Command { codeRef: { module: COMMAND_MODULE, name: 'DoThing' }, requiresApproval: true, }, + { + codeRef: { module: COMMAND_MODULE, name: 'DoThingQuietly' }, + requiresApproval: false, + }, ], - 'the command round-trips into boxel.commands', + 'commands round-trip into boxel.commands, preserving an explicit requiresApproval: false', ); assert.strictEqual( body.trim(),