Merge pull request #213 from laststance/feat/coverage-100

test: drive coverage to a pragmatic 100% bar with a thresholds gate

Author: ryota-murakami

src/main/services/agentScanner.test.ts

@@ -324,4 +324,48 @@ describe('scanAgents', () => {
     const claude = agents.find((a) => a.id === 'claude-code')!
     expect(claude.localSkillCount).toBe(0)
   })
+
+  it('falls back to zero skill counts when an existing agent dir cannot be read', async () => {
+    // Arrange
+    // Agent dir passes the existence probe (access resolves)...
+    accessMock.mockResolvedValue(undefined)
+    // ...but enumerating its contents fails (e.g. EACCES / transient I/O error),
+    // so both the symlink tally and the local-folder tally must degrade to 0
+    // instead of throwing and crashing the whole scan.
+    readdirMock.mockImplementation(async (path: string) => {
+      if (path === '/mock/agents/claude/skills') {
+        throw new Error('EACCES: permission denied')
+      }
+      return []
+    })
+
+    // Act
+    const { scanAgents } = await import('./agentScanner')
+    const agents = await scanAgents()
+
+    // Assert
+    const claude = agents.find((a) => a.id === 'claude-code')!
+    expect(claude.exists).toBe(true)
+    expect(claude.skillCount).toBe(0)
+    expect(claude.localSkillCount).toBe(0)
+  })
+
+  it('omits filesystem identity for an agent whose directory stats cannot be read', async () => {
+    // Arrange
+    // Agent dir exists and is empty, but lstat fails after the existence probe
+    // (e.g. the dir is unstattable), so filesystemIdentity must be left
+    // undefined rather than propagating the lstat rejection.
+    accessMock.mockResolvedValue(undefined)
+    readdirMock.mockResolvedValue([])
+    lstatMock.mockRejectedValue(new Error('EACCES: permission denied'))
+
+    // Act
+    const { scanAgents } = await import('./agentScanner')
+    const agents = await scanAgents()
+
+    // Assert
+    const claude = agents.find((a) => a.id === 'claude-code')!
+    expect(claude.exists).toBe(true)
+    expect(claude.filesystemIdentity).toBeUndefined()
+  })
 })

src/main/services/cliCommandService.test.ts

@@ -1,3 +1,4 @@
+import * as nodeFs from 'node:fs'
 import { mkdtempSync, realpathSync } from 'node:fs'
 import {
   lstat,
@@ -31,6 +32,8 @@ const servicePromise = (async () => import('./cliCommandService'))()
 
 describe('cliCommandService', () => {
   afterEach(async () => {
+    // Drop any per-test fs.promises spies so later tests hit the real filesystem.
+    vi.restoreAllMocks()
     await rm(join(sharedHome, '.local'), { recursive: true, force: true })
   })
 
@@ -173,4 +176,116 @@ exec open -b "io.laststance.skills-desktop"
     })
     expect((await lstat(commandPath)).isSymbolicLink()).toBe(true)
   })
+
+  it('blocks management when a directory squats on the command path', async () => {
+    // Arrange
+    const { getCliCommandStatus } = await servicePromise
+    await mkdir(commandPath, { recursive: true })
+
+    // Act
+    const status = await getCliCommandStatus()
+
+    // Assert
+    expect(status).toEqual({
+      status: 'blocked',
+      commandName: 'skills-desktop',
+      commandPath,
+      message: `${commandPath} is already occupied by another filesystem entry.`,
+    })
+  })
+
+  it('blocks management when the command path cannot be inspected', async () => {
+    // Arrange
+    const { getCliCommandStatus } = await servicePromise
+    // Make ~/.local/bin a regular file so lstat-ing a child path throws ENOTDIR.
+    await mkdir(join(sharedHome, '.local'), { recursive: true })
+    await writeFile(
+      join(sharedHome, '.local', 'bin'),
+      'not a directory',
+      'utf-8',
+    )
+
+    // Act
+    const status = await getCliCommandStatus()
+
+    // Assert
+    expect(status.status).toBe('blocked')
+    expect(status.commandName).toBe('skills-desktop')
+    expect(status.commandPath).toBe(commandPath)
+    expect(status.message).toContain(`Could not inspect ${commandPath}:`)
+  })
+
+  it('reports success without rewriting when the command is already installed', async () => {
+    // Arrange
+    const { installCliCommand } = await servicePromise
+    await installCliCommand()
+
+    // Act
+    const result = await installCliCommand()
+
+    // Assert
+    expect(result).toEqual({
+      ok: true,
+      status: {
+        status: 'installed',
+        commandName: 'skills-desktop',
+        commandPath,
+        message: 'Command is installed.',
+      },
+      message: 'Command is already installed.',
+    })
+  })
+
+  it('surfaces a failure message when writing the shim throws', async () => {
+    // Arrange
+    const { installCliCommand } = await servicePromise
+    vi.spyOn(nodeFs.promises, 'writeFile').mockRejectedValueOnce(
+      new Error('disk full'),
+    )
+
+    // Act
+    const result = await installCliCommand()
+
+    // Assert
+    expect(result.ok).toBe(false)
+    expect(result.status.status).toBe('not-installed')
+    expect(result.message).toBe('Could not install command: disk full')
+  })
+
+  it('reports success when the command is already absent', async () => {
+    // Arrange
+    const { removeCliCommand } = await servicePromise
+
+    // Act
+    const result = await removeCliCommand()
+
+    // Assert
+    expect(result).toEqual({
+      ok: true,
+      status: {
+        status: 'not-installed',
+        commandName: 'skills-desktop',
+        commandPath,
+        message: 'Command is not installed.',
+      },
+      message: 'Command is already removed.',
+    })
+  })
+
+  it('surfaces a failure message when deleting the managed shim throws', async () => {
+    // Arrange
+    const { installCliCommand, removeCliCommand } = await servicePromise
+    await installCliCommand()
+    vi.spyOn(nodeFs.promises, 'unlink').mockRejectedValueOnce(
+      new Error('disk full'),
+    )
+
+    // Act
+    const result = await removeCliCommand()
+
+    // Assert
+    expect(result.ok).toBe(false)
+    expect(result.status.status).toBe('installed')
+    expect(result.message).toBe('Could not remove command: disk full')
+  })
 })

src/main/services/fileReader.test.ts

@@ -303,6 +303,53 @@ describe('listSkillFiles', () => {
     // Assert
     expect(result).toEqual([])
   })
+
+  it('falls back to an empty listing when traversal throws on a malformed entry', async () => {
+    // Arrange
+    // readdir resolves, but the entry's own type-check throws mid-walk —
+    // this escapes the readdir try/catch and must be swallowed by the
+    // top-level listSkillFiles guard rather than crashing the caller.
+    const explodingEntry = {
+      name: 'corrupt',
+      isFile: () => false,
+      isDirectory: () => false,
+      isSymbolicLink: () => {
+        throw new Error('EIO: corrupt dirent')
+      },
+    }
+    ;(mockFs.readdir as ReturnType<typeof vi.fn>).mockResolvedValue([
+      explodingEntry,
+    ])
+    mockStat()
+
+    // Act
+    const result = await listSkillFiles('/skills/my-skill')
+
+    // Assert
+    expect(result).toEqual([])
+  })
+
+  it('drops a file from the listing when its stat call fails', async () => {
+    // Arrange
+    mockTree({
+      '/skills/my-skill': [makeDirent('SKILL.md'), makeDirent('vanished.md')],
+    })
+    // SKILL.md stats fine; vanished.md disappears between readdir and stat.
+    ;(mockFs.stat as ReturnType<typeof vi.fn>).mockImplementation(
+      async (path: string) => {
+        if (path === '/skills/my-skill/vanished.md') {
+          throw new Error('ENOENT: stat after readdir')
+        }
+        return { size: 100 }
+      },
+    )
+
+    // Act
+    const names = (await listSkillFiles('/skills/my-skill')).map((f) => f.name)
+
+    // Assert
+    expect(names).toEqual(['SKILL.md'])
+  })
 })
 
 describe('readSkillFile', () => {

src/main/services/filesystemIdentity.test.ts

@@ -1,8 +1,11 @@
+import type { Stats } from 'node:fs'
+
 import { describe, expect, it } from 'vitest'
 
 import type { FilesystemEntryIdentity } from '@/shared/types'
 
 import {
+  filesystemIdentityFromStats,
   isReviewedEntryUnchangedIdentity,
   isSameFilesystemEntryIdentity,
 } from './filesystemIdentity'
@@ -186,4 +189,69 @@ describe('filesystem identity guards for destructive deletes', () => {
       expect(isSame).toBe(false)
     })
   })
+
+  describe('filesystemIdentityFromStats (kind discriminant from fs.Stats)', () => {
+    // A fully-shaped Stats fixture with every predicate defaulting to false, so
+    // each arm test only flips the single predicate the ternary branches on.
+    const baseStats = {
+      isFile: () => false,
+      isDirectory: () => false,
+      isBlockDevice: () => false,
+      isCharacterDevice: () => false,
+      isSymbolicLink: () => false,
+      isFIFO: () => false,
+      isSocket: () => false,
+      dev: 16777233,
+      ino: 99,
+      mode: 0,
+      nlink: 0,
+      uid: 0,
+      gid: 0,
+      rdev: 0,
+      size: 96,
+      blksize: 0,
+      blocks: 0,
+      atimeMs: 0,
+      mtimeMs: 2_000,
+      ctimeMs: 1_000,
+      birthtimeMs: 0,
+      atime: new Date(0),
+      mtime: new Date(0),
+      ctime: new Date(0),
+      birthtime: new Date(0),
+    } satisfies Stats
+
+    it('labels an entry as a symlink so destructive UI treats it as a link, not its target', () => {
+      // Arrange: lstat saw a symbolic link (directory/file predicates irrelevant).
+      const symlinkStats = { ...baseStats, isSymbolicLink: () => true }
+
+      // Act
+      const identity = filesystemIdentityFromStats(symlinkStats)
+
+      // Assert
+      expect(identity.kind).toBe('symlink')
+    })
+
+    it('labels a non-symlink directory as a directory so a folder delete is gated as a folder', () => {
+      // Arrange: a real directory — symlink predicate false, directory predicate true.
+      const directoryStats = { ...baseStats, isDirectory: () => true }
+
+      // Act
+      const identity = filesystemIdentityFromStats(directoryStats)
+
+      // Assert
+      expect(identity.kind).toBe('directory')
+    })
+
+    it('labels a plain file as a file when it is neither a symlink nor a directory', () => {
+      // Arrange: a regular file — only isFile is true.
+      const fileStats = { ...baseStats, isFile: () => true }
+
+      // Act
+      const identity = filesystemIdentityFromStats(fileStats)
+
+      // Assert
+      expect(identity.kind).toBe('file')
+    })
+  })
 })

src/main/services/mainWindowState.test.ts

@@ -0,0 +1,73 @@
+import type { BrowserWindow } from 'electron'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+import { getMainWindow, setMainWindow } from './mainWindowState'
+
+/**
+ * Build the minimal BrowserWindow surface `getMainWindow` queries (only
+ * `isDestroyed`), so node-lane tests can drive the live/destroyed branches
+ * without instantiating a real Electron window.
+ * @param isDestroyed - Whether the stub reports itself as destroyed.
+ * @returns A BrowserWindow-typed stub exposing a controllable `isDestroyed`.
+ * @example
+ * setMainWindow(makeWindowStub(false)) // window stays handed out
+ */
+function makeWindowStub(isDestroyed: boolean): BrowserWindow {
+  // Only `isDestroyed` is read by the module; cast is necessary because a real
+  // BrowserWindow cannot be constructed in the node lane.
+  return {
+    isDestroyed: vi.fn(() => isDestroyed),
+  } as unknown as BrowserWindow
+}
+
+describe('main window reference store', () => {
+  beforeEach(() => {
+    // Reset the module-scoped reference so each spec starts from "no window".
+    setMainWindow(null)
+  })
+
+  it('reports no window before any main window has been created', () => {
+    // Arrange — fresh state from beforeEach: no window has been stored
+
+    // Act
+    const currentWindow = getMainWindow()
+
+    // Assert
+    expect(currentWindow).toBeNull()
+  })
+
+  it('hands back the live main window while it is open', () => {
+    // Arrange
+    const liveWindow = makeWindowStub(false)
+    setMainWindow(liveWindow)
+
+    // Act
+    const currentWindow = getMainWindow()
+
+    // Assert
+    expect(currentWindow).toBe(liveWindow)
+  })
+
+  it('stops handing out the window once Electron has destroyed it', () => {
+    // Arrange
+    const destroyedWindow = makeWindowStub(true)
+    setMainWindow(destroyedWindow)
+
+    // Act
+    const currentWindow = getMainWindow()
+
+    // Assert
+    expect(currentWindow).toBeNull()
+  })
+
+  it('clears the stored window when passed null on the close event', () => {
+    // Arrange
+    setMainWindow(makeWindowStub(false))
+
+    // Act
+    setMainWindow(null)
+
+    // Assert
+    expect(getMainWindow()).toBeNull()
+  })
+})