diff --git a/src/lib/config-handler.ts b/src/lib/config-handler.ts index 357abb0..e8c22f4 100644 --- a/src/lib/config-handler.ts +++ b/src/lib/config-handler.ts @@ -21,7 +21,6 @@ import { type OpencodeClientLike, } from './model-availability.js' import { - formatSkillDescription, type LoadedSkill, loadSkill, wrapSkillTemplate, @@ -57,9 +56,7 @@ function isSystematicCommandConfig( ): boolean { const description = command?.description return ( - typeof description === 'string' && - (description.startsWith('(Systematic) ') || - description.startsWith('(Systematic - Skill) ')) + typeof description === 'string' && description.startsWith('(Systematic) ') ) } @@ -515,7 +512,7 @@ function collectSkillsAsCommands( * OpenCode's skill registry or permissions here to compensate. */ function loadDiscoveredSkillAsCommand(skill: DiscoveredSkill): CommandConfig { - const description = formatSkillDescription(skill.description, skill.name) + const description = skill.description || `${skill.name} skill` if (skill.frontmatter.disableModelInvocation === true) { // Body was read once at discovery time (DiscoveredSkill.body); no re-read. @@ -728,15 +725,13 @@ export function createConfigHandler(deps: ConfigHandlerDeps) { ...bundledSkills, ...discoveredSkillCommands, } - // Drop any existing entry Systematic previously emitted (identified by - // the `(Systematic) `/`(Systematic - Skill) ` description marker), - // regardless of key. If it's still valid this pass, `emittedCommands` - // re-adds it below (mergeSystematicEntries drops-then-adds); if the - // source (bundled command/skill, or a discovered on-disk skill) no - // longer exists, it stays dropped. This also fixes orphaned - // discovered-skill commands: those use a bare key (not a - // `systematic:`/`ce:`-prefixed key), so removing a skill from disk left - // its stale command behind under the old, narrower key-based predicate. + // Bundled commands/skills carry the `(Systematic) ` marker and are + // dropped-then-re-emitted every load (refresh semantics). Discovered-skill + // commands are intentionally UNMARKED — they're the user's own skills, + // not Systematic's — so they're add-if-absent and never refreshed or + // cleaned up across reloads. That's fine: OpenCode rebuilds config + // in-memory on every launch, so a deleted skill just isn't re-emitted; + // nothing stale persists to disk. config.command = mergeSystematicEntries( existingCommands as Record, emittedCommands as Record, diff --git a/src/lib/skill-loader.ts b/src/lib/skill-loader.ts index 591cddd..07bef55 100644 --- a/src/lib/skill-loader.ts +++ b/src/lib/skill-loader.ts @@ -4,7 +4,7 @@ import { parseFrontmatter } from './frontmatter.js' import type { SkillDeprecated, SkillInfo } from './skills.js' const SKILL_PREFIX = 'systematic:' -const SKILL_DESCRIPTION_PREFIX = '(Systematic - Skill) ' +const SKILL_DESCRIPTION_PREFIX = '(Systematic) ' export interface LoadedSkill { name: string diff --git a/tests/unit/config-handler.test.ts b/tests/unit/config-handler.test.ts index 4fe69a4..581501a 100644 --- a/tests/unit/config-handler.test.ts +++ b/tests/unit/config-handler.test.ts @@ -255,7 +255,7 @@ Command template for ${name}.`, expect(config.command).toBeDefined() expect(config.command?.['systematic:test-skill']).toBeDefined() expect(config.command?.['systematic:test-skill']?.description).toBe( - '(Systematic - Skill) A test skill', + '(Systematic) A test skill', ) expect(config.command?.['systematic:test-skill']?.template).toContain( '', @@ -295,7 +295,7 @@ Skill content for ce:plan.`, expect(config.command?.['ce:plan']).toBeDefined() expect(config.command?.['systematic:ce:plan']).toBeUndefined() expect(config.command?.['ce:plan']?.description).toBe( - '(Systematic - Skill) Plan workflow skill', + '(Systematic) Plan workflow skill', ) }) @@ -420,7 +420,7 @@ Skill content for ce:plan.`, template: 'native command template', }) expect(config.command?.['systematic:test-skill']?.description).toBe( - '(Systematic - Skill) A test skill', + '(Systematic) A test skill', ) }) @@ -483,7 +483,7 @@ Skill content for ce:plan.`, template: 'native command template', }, 'ce:plan': { - description: '(Systematic - Skill) Old plan command', + description: '(Systematic) Old plan command', template: 'old plan template', }, 'systematic:legacy-command': { @@ -491,7 +491,7 @@ Skill content for ce:plan.`, template: 'old command template', }, 'systematic:legacy-skill': { - description: '(Systematic - Skill) Old skill command', + description: '(Systematic) Old skill command', template: 'old skill template', }, 'systematic:test-command': { @@ -499,7 +499,7 @@ Skill content for ce:plan.`, template: 'old command template', }, 'systematic:test-skill': { - description: '(Systematic - Skill) Old skill command', + description: '(Systematic) Old skill command', template: 'old skill template', }, }, @@ -518,17 +518,14 @@ Skill content for ce:plan.`, 'Agent prompt for systematic-implementer.', ) // Unlike agents (which still use a key-based `bundledAgentKeys` guard), - // the command drop predicate was simplified to drop ANY existing entry - // carrying the `(Systematic) `/`(Systematic - Skill) ` marker, - // regardless of key — this is required to clean up orphaned - // discovered-skill commands (which use bare, unprefixed keys with no - // way to distinguish "still owned" from "stale" other than the - // marker). A side effect: a bare, non-prefixed key that merely *looks* - // like Systematic output but isn't re-emitted (like this synthetic - // 'native-command' fixture) is now dropped rather than preserved. + // the command drop predicate drops ANY existing entry carrying the + // `(Systematic) ` marker, regardless of key. A side effect: a bare, + // non-prefixed key that merely *looks* like Systematic output but isn't + // re-emitted (like this synthetic 'native-command' fixture) is now + // dropped rather than preserved. expect(config.command?.['native-command']).toBeUndefined() expect(config.command?.['ce:plan']?.description).toBe( - '(Systematic - Skill) A plan skill', + '(Systematic) A plan skill', ) expect(config.command?.['ce:plan']?.template).toContain( 'Skill content for ce:plan', @@ -542,7 +539,7 @@ Skill content for ce:plan.`, 'Command template for test-command', ) expect(config.command?.['systematic:test-skill']?.description).toBe( - '(Systematic - Skill) A test skill', + '(Systematic) A test skill', ) }) @@ -1750,9 +1747,7 @@ Discovered body for ${name}.`, const command = config.command?.foo expect(command).toBeDefined() - expect(command?.description).toBe( - '(Systematic - Skill) A discovered skill', - ) + expect(command?.description).toBe('A discovered skill') expect(command?.template).toContain('skill tool') expect(command?.template).toContain('\n$ARGUMENTS') }) @@ -1775,9 +1770,7 @@ Discovered body for ${name}.`, const command = config.command?.bar expect(command).toBeDefined() - expect(command?.description).toBe( - '(Systematic - Skill) A command-only skill', - ) + expect(command?.description).toBe('A command-only skill') expect(command?.template).toContain('') expect(command?.template).toContain('Discovered body for bar') expect(command?.template).not.toContain('skill tool') @@ -1851,7 +1844,7 @@ Discovered body for ${name}.`, expect(Object.keys(config.command ?? {})).toEqual(['foo']) }) - test('orphaned discovered-skill command is removed once its skill is deleted from disk', async () => { + test('discovered-skill command persists after its skill is deleted from disk (unmarked, not refreshed)', async () => { const discoveredDir = path.join(projectDir, '.opencode/skills') writeDiscoveredSkill(discoveredDir, 'foo') @@ -1871,8 +1864,11 @@ Discovered body for ${name}.`, force: true, }) + // Discovered-skill commands carry no `(Systematic) ` marker, so the + // drop-then-re-emit refresh never touches them: this is intentional, + // since OpenCode rebuilds config in-memory on every launch anyway. await handler(config) - expect(config.command?.foo).toBeUndefined() + expect(config.command?.foo).toBeDefined() }) test('user-defined command of the same name still survives after the skill is removed (R4)', async () => { @@ -1907,6 +1903,25 @@ Discovered body for ${name}.`, expect(config.command?.foo?.template).toBe('User template') }) + test('discovered skill command description has no Systematic prefix (honest attribution)', async () => { + writeDiscoveredSkill(path.join(projectDir, '.opencode/skills'), 'baz', { + description: 'My own custom skill', + }) + + const handler = createConfigHandler({ + directory: projectDir, + bundledSkillsDir: path.join(bundledDir, 'skills'), + bundledAgentsDir: path.join(bundledDir, 'agents'), + bundledCommandsDir: path.join(bundledDir, 'commands'), + }) + + const config: Config = {} + await handler(config) + + expect(config.command?.baz?.description).toBe('My own custom skill') + expect(config.command?.baz?.description).not.toContain('(Systematic') + }) + test('discovered skill with user-invocable: false does not emit a command', async () => { const skillDir = path.join(projectDir, '.opencode/skills', 'hidden') fs.mkdirSync(skillDir, { recursive: true }) diff --git a/tests/unit/skill-loader.test.ts b/tests/unit/skill-loader.test.ts index 7d2e39c..4182465 100644 --- a/tests/unit/skill-loader.test.ts +++ b/tests/unit/skill-loader.test.ts @@ -34,21 +34,21 @@ describe('skill-loader', () => { }) describe('formatSkillDescription', () => { - test('adds (Systematic - Skill) prefix to description', () => { + test('adds (Systematic) prefix to description', () => { expect(formatSkillDescription('A test skill', 'test')).toBe( - '(Systematic - Skill) A test skill', + '(Systematic) A test skill', ) }) test('does not double-prefix already prefixed description', () => { - expect( - formatSkillDescription('(Systematic - Skill) A test skill', 'test'), - ).toBe('(Systematic - Skill) A test skill') + expect(formatSkillDescription('(Systematic) A test skill', 'test')).toBe( + '(Systematic) A test skill', + ) }) test('uses fallback name when description is empty', () => { expect(formatSkillDescription('', 'my-skill')).toBe( - '(Systematic - Skill) my-skill skill', + '(Systematic) my-skill skill', ) }) }) @@ -165,7 +165,7 @@ description: A test skill expect(loaded.name).toBe('test-skill') expect(loaded.prefixedName).toBe('systematic:test-skill') - expect(loaded.description).toBe('(Systematic - Skill) A test skill') + expect(loaded.description).toBe('(Systematic) A test skill') expect(loaded.wrappedTemplate).toContain('') expect(loaded.wrappedTemplate).toContain('# Test Content') })