From 758a6ba7f6a0b776bc8cee90a71baa7acb3de2bb Mon Sep 17 00:00:00 2001 From: tmm Date: Sat, 18 Apr 2026 17:21:15 -0400 Subject: [PATCH 1/2] fix: only warn about stale skills when installed --- src/Cli.test.ts | 19 +++++++++++++- src/Cli.ts | 2 +- src/SyncSkills.test.ts | 26 +++++++++++++++++++ src/SyncSkills.ts | 57 +++++++++++++++++++++++++++++++++++++----- src/e2e.test.ts | 17 ++++++++++++- 5 files changed, 112 insertions(+), 9 deletions(-) diff --git a/src/Cli.test.ts b/src/Cli.test.ts index 8409a45..3f2a72c 100644 --- a/src/Cli.test.ts +++ b/src/Cli.test.ts @@ -12,10 +12,15 @@ afterAll(() => { }) let __mockSkillsHash: string | undefined +let __mockSkillsInstalled = true vi.mock('./SyncSkills.js', async (importOriginal) => { const actual = await importOriginal() - return { ...actual, readHash: () => __mockSkillsHash } + return { + ...actual, + hasInstalledSkills: () => __mockSkillsInstalled, + readHash: () => __mockSkillsHash, + } }) async function serve( @@ -2652,11 +2657,13 @@ describe('skills staleness', () => { beforeEach(() => { stderrSpy = vi.spyOn(process.stderr, 'write').mockImplementation(() => true) __mockSkillsHash = undefined + __mockSkillsInstalled = true }) afterEach(() => { stderrSpy.mockRestore() __mockSkillsHash = undefined + __mockSkillsInstalled = true }) test('includes skills CTA when stale', async () => { @@ -2728,6 +2735,16 @@ describe('skills staleness', () => { expect(output).not.toContain('Skills are out of date') }) + test('does not warn when skills are not installed', async () => { + __mockSkillsHash = '0000000000000000' + __mockSkillsInstalled = false + const cli = Cli.create('test') + cli.command('ping', { description: 'Health check', run: () => ({ pong: true }) }) + + const { output } = await serve(cli, ['ping']) + expect(output).not.toContain('Skills are out of date') + }) + test('does not warn for skills add', async () => { __mockSkillsHash = '0000000000000000' const cli = Cli.create('test') diff --git a/src/Cli.ts b/src/Cli.ts index 47bbf20..01a7e86 100644 --- a/src/Cli.ts +++ b/src/Cli.ts @@ -562,7 +562,7 @@ async function serveImpl( const isMcpAdd = builtinIdx(filtered, name, 'mcp') !== -1 if (!isSkillsAdd && !isMcpAdd) { const stored = SyncSkills.readHash(name) - if (stored) { + if (stored && SyncSkills.hasInstalledSkills(name, { cwd: options.sync?.cwd })) { const groups = new Map() const entries = collectSkillCommands(commands, [], groups, options.rootCommand) if (Skill.hash(entries) !== stored) { diff --git a/src/SyncSkills.test.ts b/src/SyncSkills.test.ts index ac93859..02c0eef 100644 --- a/src/SyncSkills.test.ts +++ b/src/SyncSkills.test.ts @@ -205,6 +205,32 @@ test('list shows installed status after sync', async () => { rmSync(tmp, { recursive: true, force: true }) }) +test('list shows not installed when synced skills are removed', async () => { + const tmp = join(tmpdir(), `clac-list-missing-test-${Date.now()}`) + mkdirSync(tmp, { recursive: true }) + process.env.XDG_DATA_HOME = tmp + + const cli = Cli.create('test') + cli.command('ping', { description: 'Ping', run: () => ({}) }) + + const commands = Cli.toCommands.get(cli)! + const installDir = join(tmp, 'install') + mkdirSync(join(installDir, '.agents', 'skills'), { recursive: true }) + + const sync = await SyncSkills.sync('test', commands, { + global: false, + cwd: installDir, + }) + + rmSync(sync.paths[0]!, { recursive: true, force: true }) + + const result = await SyncSkills.list('test', commands) + expect(result).toHaveLength(1) + expect(result[0]!.installed).toBe(false) + + rmSync(tmp, { recursive: true, force: true }) +}) + test('list returns empty for CLI with no commands', async () => { const cli = Cli.create('empty') const commands = Cli.toCommands.get(cli)! diff --git a/src/SyncSkills.ts b/src/SyncSkills.ts index 96f3b1b..0c16e1f 100644 --- a/src/SyncSkills.ts +++ b/src/SyncSkills.ts @@ -71,7 +71,12 @@ export async function sync( // Write skills hash + names for staleness detection const hashEntries = collectEntries(commands, [], undefined, options.rootCommand) - writeMeta(name, Skill.hash(hashEntries), [...currentNames]) + writeMeta( + name, + Skill.hash(hashEntries), + [...currentNames], + [...paths, ...agents.map((agent) => agent.path)], + ) return { skills: skills.sort((a, b) => a.name.localeCompare(b.name)), paths, agents } } finally { @@ -140,8 +145,7 @@ export async function list( const files = Skill.split(name, entries, depth, groups) const skills: list.Skill[] = [] - const meta = readMeta(name) - const installed = new Set(meta?.skills) + const installed = readInstalledSkills(name, { cwd }) for (const file of files) { const nameMatch = file.content.match(/^name:\s*(.+)$/m) @@ -180,6 +184,14 @@ export async function list( return skills.sort((a, b) => a.name.localeCompare(b.name)) } +/** Returns whether any previously synced skills are still installed on disk. */ +export function hasInstalledSkills( + name: string, + options: { cwd?: string | undefined } = {}, +): boolean { + return readInstalledSkills(name, options).size > 0 +} + export declare namespace list { /** Options for listing skills. */ type Options = { @@ -305,15 +317,20 @@ function hashPath(name: string): string { } /** @internal Writes the skills metadata for staleness detection and cleanup. */ -function writeMeta(name: string, hash: string, skills: string[]) { +function writeMeta(name: string, hash: string, skills: string[], paths: string[]) { const file = hashPath(name) const dir = path.dirname(file) if (!fsSync.existsSync(dir)) fsSync.mkdirSync(dir, { recursive: true }) - fsSync.writeFileSync(file, JSON.stringify({ hash, skills, at: new Date().toISOString() }) + '\n') + fsSync.writeFileSync( + file, + JSON.stringify({ hash, skills, paths, at: new Date().toISOString() }) + '\n', + ) } /** @internal Reads the stored metadata for a CLI. */ -function readMeta(name: string): { hash: string; skills?: string[] } | undefined { +function readMeta( + name: string, +): { hash: string; paths?: string[] | undefined; skills?: string[] | undefined } | undefined { try { return JSON.parse(fsSync.readFileSync(hashPath(name), 'utf-8')) } catch { @@ -321,6 +338,34 @@ function readMeta(name: string): { hash: string; skills?: string[] } | undefined } } +/** Reads the names of previously synced skills that are still installed on disk. */ +function readInstalledSkills( + name: string, + options: { cwd?: string | undefined } = {}, +): Set { + const meta = readMeta(name) + if (!meta?.skills?.length) return new Set() + + if (meta.paths?.length) { + const installed = meta.paths + .filter((skillPath) => isInstalledSkillPath(skillPath)) + .map((skillPath) => path.basename(skillPath)) + return new Set(installed) + } + + const cwd = options.cwd ?? process.cwd() + const bases = [path.join(os.homedir(), '.agents', 'skills'), path.join(cwd, '.agents', 'skills')] + const installed = meta.skills.filter((skill) => + bases.some((base) => isInstalledSkillPath(path.join(base, skill))), + ) + return new Set(installed) +} + +/** Returns whether a skill directory currently contains a skill file. */ +function isInstalledSkillPath(skillPath: string): boolean { + return fsSync.existsSync(path.join(skillPath, 'SKILL.md')) +} + /** Reads the stored skills hash for a CLI. Returns `undefined` if no hash exists. */ export function readHash(name: string): string | undefined { return readMeta(name)?.hash diff --git a/src/e2e.test.ts b/src/e2e.test.ts index 750cd5c..f44516a 100644 --- a/src/e2e.test.ts +++ b/src/e2e.test.ts @@ -3,6 +3,7 @@ import { Cli, Errors, Skill, Typegen, z } from 'incur' import { app as honoApp } from '../test/fixtures/hono-api.js' let __mockSkillsHash: string | undefined +let __mockSkillsInstalled = true const originalIsTTY = process.stdout.isTTY beforeAll(() => { @@ -14,7 +15,11 @@ afterAll(() => { vi.mock('./SyncSkills.js', async (importOriginal) => { const actual = await importOriginal() - return { ...actual, readHash: () => __mockSkillsHash } + return { + ...actual, + hasInstalledSkills: () => __mockSkillsInstalled, + readHash: () => __mockSkillsHash, + } }) describe('routing', () => { @@ -1983,11 +1988,13 @@ describe('skills staleness', () => { beforeEach(() => { stderrSpy = vi.spyOn(process.stderr, 'write').mockImplementation(() => true) __mockSkillsHash = undefined + __mockSkillsInstalled = true }) afterEach(() => { stderrSpy.mockRestore() __mockSkillsHash = undefined + __mockSkillsInstalled = true }) test('includes skills CTA when stale', async () => { @@ -2016,6 +2023,14 @@ describe('skills staleness', () => { expect(output).not.toContain('Skills are out of date') }) + test('no warning when skills are not installed', async () => { + __mockSkillsHash = '0000000000000000' + __mockSkillsInstalled = false + const { output } = await serve(createApp(), ['ping']) + expect(output).toContain('pong: true') + expect(output).not.toContain('Skills are out of date') + }) + test('no warning for --llms', async () => { __mockSkillsHash = '0000000000000000' const { output } = await serve(createApp(), ['--llms']) From c63a8fe44a53a8a733158ded194e4d1526ee56b1 Mon Sep 17 00:00:00 2001 From: tmm Date: Sat, 18 Apr 2026 17:28:16 -0400 Subject: [PATCH 2/2] docs: add changeset for skills install warning fix --- .changeset/tmm-skills-installed-staleness.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/tmm-skills-installed-staleness.md diff --git a/.changeset/tmm-skills-installed-staleness.md b/.changeset/tmm-skills-installed-staleness.md new file mode 100644 index 0000000..8b4f735 --- /dev/null +++ b/.changeset/tmm-skills-installed-staleness.md @@ -0,0 +1,5 @@ +--- +"incur": patch +--- + +Fixed stale skills warnings to only appear when synced skill files were still installed on disk, and updated `skills list` to reflect actual install state instead of stale metadata.