fix: only warn about stale skills when installed

.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.

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<typeof import('./SyncSkills.js')>()
-  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')

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<string, string>()
         const entries = collectSkillCommands(commands, [], groups, options.rootCommand)
         if (Skill.hash(entries) !== stored) {

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)!

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,22 +317,55 @@ 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 {
     return undefined
   }
 }
 
+/** Reads the names of previously synced skills that are still installed on disk. */
+function readInstalledSkills(
+  name: string,
+  options: { cwd?: string | undefined } = {},
+): Set<string> {
+  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

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<typeof import('./SyncSkills.js')>()
-  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'])