diff --git a/README.md b/README.md index ebc5b9f4..c762d013 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,7 @@ Commands: doctor Diagnose WorkOS integration issues skills Manage WorkOS skills for coding agents (install, uninstall, list) -Skills are automatically installed to detected coding agents when you run `workos install`. Use `workos skills list` to check status. +Skills auto-install to detected coding agents on `workos install` and `workos auth login`. Use `workos skills list` to check status, `workos doctor` to detect stale skills, or `workos doctor --fix` to refresh them in place (constrained to `workos/` and `workos-widgets/`). Resource Management: organization (org) Manage organizations diff --git a/package.json b/package.json index b60dbde3..582f2ee2 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,7 @@ "@clack/prompts": "1.0.1", "@napi-rs/keyring": "^1.2.0", "@workos-inc/node": "^8.7.0", - "@workos/skills": "0.2.4", + "@workos/skills": "0.4.0", "chalk": "^5.6.2", "diff": "^8.0.3", "fast-glob": "^3.3.3", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9f6d9516..38de34b7 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -30,8 +30,8 @@ importers: specifier: ^8.7.0 version: 8.7.0 '@workos/skills': - specifier: 0.2.4 - version: 0.2.4 + specifier: 0.4.0 + version: 0.4.0 chalk: specifier: ^5.6.2 version: 5.6.2 @@ -1071,8 +1071,8 @@ packages: resolution: {integrity: sha512-43HfXSR2Ez7M4ixpebuYVZzZf3gauh5jvv9lYnePg/x0XZMN2hjpEV3FD1LQX1vfMbqQ5gON3DN+/gH2rITm3A==} engines: {node: '>=20.15.0'} - '@workos/skills@0.2.4': - resolution: {integrity: sha512-Ku/8aD6zRwA7M4jREp/l1pUHOGCmDMBudlJAUfpkfZqFdDatxWJy+CQmYiIlMKt3IP4/GURe1rA72HpAcEkcaw==} + '@workos/skills@0.4.0': + resolution: {integrity: sha512-pH562ybfmXid9GcKsmer8bH/mNezhqIvXnwNN0T3UnR+m/ZHfBmiygI27GAxIf8Kl9ppNbOrTSGAVgzw1bG3Gg==} ansi-escapes@7.3.0: resolution: {integrity: sha512-BvU8nYgGQBxcmMuEeUEmNTvrMVjJNSH7RgW24vXexN4Ven6qCvy4TntnvlnwnMLTVlcRQQdbRY8NKnaIoeWDNg==} @@ -2515,7 +2515,7 @@ snapshots: iron-webcrypto: 2.0.0 jose: 6.1.3 - '@workos/skills@0.2.4': + '@workos/skills@0.4.0': dependencies: yaml: 2.8.3 diff --git a/src/bin.ts b/src/bin.ts index 169811be..bdbf96bd 100644 --- a/src/bin.ts +++ b/src/bin.ts @@ -343,6 +343,11 @@ yargs(rawArgs) default: false, description: 'Copy report to clipboard', }, + fix: { + type: 'boolean', + default: false, + description: 'Auto-update stale WorkOS skills (writes to /skills/workos/ and workos-widgets/ only)', + }, }), async (argv) => { const { handleDoctor } = await import('./commands/doctor.js'); diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 425aea85..419deee8 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -9,6 +9,7 @@ interface DoctorArgs { installDir?: string; json?: boolean; copy?: boolean; + fix?: boolean; } export async function handleDoctor(argv: ArgumentsCamelCase): Promise { @@ -19,6 +20,7 @@ export async function handleDoctor(argv: ArgumentsCamelCase): Promis skipAi: argv.skipAi ?? false, json: argv.json ?? false, copy: argv.copy ?? false, + fix: argv.fix ?? false, }; try { diff --git a/src/commands/install-skill.spec.ts b/src/commands/install-skill.spec.ts index 9b9d11e2..98539ba9 100644 --- a/src/commands/install-skill.spec.ts +++ b/src/commands/install-skill.spec.ts @@ -1,5 +1,6 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import { existsSync, mkdirSync, writeFileSync, rmSync, readFileSync } from 'fs'; +import { existsSync, mkdirSync, writeFileSync, rmSync, readFileSync, readdirSync, chmodSync } from 'fs'; +import { utimes } from 'fs/promises'; import { join } from 'path'; import { mkdtempSync } from 'fs'; import { tmpdir } from 'os'; @@ -9,6 +10,7 @@ import { detectAgents, installSkill, autoInstallSkills, + refreshWorkOSSkills, type AgentConfig, } from './install-skill.js'; @@ -22,6 +24,19 @@ vi.mock('@workos/skills', async (importOriginal) => { return { ...actual, getSkillsDir: vi.fn(actual.getSkillsDir) }; }); +// Wrap fs/promises so individual fns (rename, cp) can be temporarily overridden +// via mockImplementationOnce. ESM exports are not directly spy-able, so we +// explicitly recreate the namespace with vi.fn passthroughs for the spies we +// need; everything else stays as the real impl. +vi.mock('fs/promises', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + rename: vi.fn(actual.rename), + cp: vi.fn(actual.cp), + }; +}); + describe('install-skill', () => { let testDir: string; let skillsDir: string; @@ -164,6 +179,10 @@ describe('install-skill', () => { mkdirSync(join(skillsDir, 'test-skill')); writeFileSync(join(skillsDir, 'test-skill', 'SKILL.md'), '---\nname: test-skill\n---\n# Test Skill'); + // Sub-tree the install must copy alongside SKILL.md. + mkdirSync(join(skillsDir, 'test-skill', 'references')); + writeFileSync(join(skillsDir, 'test-skill', 'references', 'topic.md'), '# Topic'); + targetAgent = { name: 'test-agent', displayName: 'Test Agent', @@ -185,6 +204,16 @@ describe('install-skill', () => { expect(content).toContain('# Test Skill'); }); + it('copies the entire skill directory tree, including references/', async () => { + const result = await installSkill(skillsDir, 'test-skill', targetAgent); + + expect(result.success).toBe(true); + + const referenceFile = join(homeDir, '.test-agent/skills/test-skill/references/topic.md'); + expect(existsSync(referenceFile)).toBe(true); + expect(readFileSync(referenceFile, 'utf-8')).toContain('# Topic'); + }); + it('creates nested directories as needed', async () => { const result = await installSkill(skillsDir, 'test-skill', targetAgent); @@ -211,6 +240,115 @@ describe('install-skill', () => { const content = readFileSync(join(homeDir, '.test-agent/skills/test-skill/SKILL.md'), 'utf-8'); expect(content).toContain('# Updated Skill'); }); + + it('prunes stale files in the target that are not in the source (replace, not overlay)', async () => { + // First install: target now matches source. + await installSkill(skillsDir, 'test-skill', targetAgent); + + // Plant a stale file the agent had from a prior skill version. + const staleFile = join(homeDir, '.test-agent/skills/test-skill/references/workos-stale.md'); + writeFileSync(staleFile, '# Stale Topic'); + expect(existsSync(staleFile)).toBe(true); + + // Re-install — the new tree should fully replace the old one. + const result = await installSkill(skillsDir, 'test-skill', targetAgent); + + expect(result.success).toBe(true); + expect(existsSync(staleFile)).toBe(false); + // Source files still present. + expect(existsSync(join(homeDir, '.test-agent/skills/test-skill/references/topic.md'))).toBe(true); + }); + + it('rolls back to the original target when the temp→target rename fails mid-install', async () => { + // Seed an existing target so the backup-rename branch has something to back up. + await installSkill(skillsDir, 'test-skill', targetAgent); + const targetFile = join(homeDir, '.test-agent/skills/test-skill/SKILL.md'); + const originalContent = readFileSync(targetFile, 'utf-8'); + + const fsPromises = await import('fs/promises'); + const realRename = (await vi.importActual('fs/promises')).rename; + + // Call sequence inside installSkill: 1=target→backup, 2=temp→target (force-throw), + // 3=backup→target (rollback). After the Once chain consumes calls 1 and 2, + // the default vi.fn(actual.rename) impl handles call 3 with the real fs op. + vi.mocked(fsPromises.rename) + .mockImplementationOnce(realRename) + .mockImplementationOnce(async () => { + throw new Error('simulated rename failure'); + }); + + const result = await installSkill(skillsDir, 'test-skill', targetAgent); + + expect(result.success).toBe(false); + expect(result.error).toContain('simulated rename failure'); + // Original target restored from backup. + expect(existsSync(targetFile)).toBe(true); + expect(readFileSync(targetFile, 'utf-8')).toBe(originalContent); + }); + + it('cleans up the temp dir when the copy itself fails', async () => { + const fsPromises = await import('fs/promises'); + vi.mocked(fsPromises.cp).mockRejectedValueOnce(new Error('copy boom')); + + const result = await installSkill(skillsDir, 'test-skill', targetAgent); + + expect(result.success).toBe(false); + expect(result.error).toContain('copy boom'); + + // No leftover .workos.tmp-test-skill-* in the parent. + const parent = join(homeDir, '.test-agent/skills'); + if (existsSync(parent)) { + const leftovers = readdirSync(parent).filter((e) => e.startsWith('.workos.tmp-')); + expect(leftovers).toEqual([]); + } + }); + + it('removes orphaned .workos.tmp-* / .workos.bak-* siblings older than 1h', async () => { + const parent = join(homeDir, '.test-agent/skills'); + mkdirSync(parent, { recursive: true }); + + // mtime cutoff applies independently to BOTH prefixes, both directions. + const oldTime = new Date(Date.now() - 2 * 60 * 60 * 1000); // 2h ago + + const oldTmp = join(parent, '.workos.tmp-test-skill-deadbeef'); + mkdirSync(oldTmp); + await utimes(oldTmp, oldTime, oldTime); + + const oldBak = join(parent, '.workos.bak-test-skill-feedface'); + mkdirSync(oldBak); + await utimes(oldBak, oldTime, oldTime); + + // Fresh siblings simulate a concurrent run — must NOT be deleted. + const freshTmp = join(parent, '.workos.tmp-test-skill-deadc0de'); + mkdirSync(freshTmp); + + const freshBak = join(parent, '.workos.bak-test-skill-cafef00d'); + mkdirSync(freshBak); + + const result = await installSkill(skillsDir, 'test-skill', targetAgent); + expect(result.success).toBe(true); + + expect(existsSync(oldTmp)).toBe(false); + expect(existsSync(oldBak)).toBe(false); + expect(existsSync(freshTmp)).toBe(true); + expect(existsSync(freshBak)).toBe(true); + }); + + it('does not remove unrelated files in the parent', async () => { + const parent = join(homeDir, '.test-agent/skills'); + mkdirSync(parent, { recursive: true }); + + // A peer skill from a different installer — must be preserved. + const peerSkill = join(parent, 'unrelated-skill'); + mkdirSync(peerSkill); + writeFileSync(join(peerSkill, 'SKILL.md'), '# Other'); + const oldTime = new Date(Date.now() - 2 * 60 * 60 * 1000); + await utimes(peerSkill, oldTime, oldTime); + + const result = await installSkill(skillsDir, 'test-skill', targetAgent); + expect(result.success).toBe(true); + expect(existsSync(join(peerSkill, 'SKILL.md'))).toBe(true); + }); }); describe('autoInstallSkills', () => { @@ -225,7 +363,7 @@ describe('install-skill', () => { vi.restoreAllMocks(); }); - it('installs all skills to all detected agents', async () => { + it('installs all skills to all detected agents and returns a summary', async () => { // Set up skills mkdirSync(join(skillsDir, 'skill-a')); writeFileSync(join(skillsDir, 'skill-a', 'SKILL.md'), '# Skill A'); @@ -236,51 +374,88 @@ describe('install-skill', () => { mkdirSync(join(homeDir, '.claude')); mkdirSync(join(homeDir, '.codex')); - await autoInstallSkills(); + const result = await autoInstallSkills(); expect(existsSync(join(homeDir, '.claude/skills/skill-a/SKILL.md'))).toBe(true); expect(existsSync(join(homeDir, '.claude/skills/skill-b/SKILL.md'))).toBe(true); expect(existsSync(join(homeDir, '.codex/skills/skill-a/SKILL.md'))).toBe(true); expect(existsSync(join(homeDir, '.codex/skills/skill-b/SKILL.md'))).toBe(true); + + expect(result).not.toBeNull(); + expect(result!.skills.sort()).toEqual(['skill-a', 'skill-b']); + expect(result!.agents.sort()).toEqual(['Claude Code', 'Codex']); }); - it('no-ops silently when no agents are detected', async () => { + it('writes a version marker per agent when the bundled version is resolvable', async () => { + // Plant a deterministic package layout so getBundledSkillsVersion finds + // a real version. The function walks up 3 dirnames from skillsDir to + // locate the package.json, so we mirror that layout here: + // /package.json ← version source + // /plugins/workos/skills ← skillsDir + const { SKILL_VERSION_MARKER_FILENAME } = await import('./install-skill.js'); + const { getSkillsDir } = await import('@workos/skills'); + + const packageRoot = join(testDir, 'pkg'); + mkdirSync(packageRoot); + writeFileSync(join(packageRoot, 'package.json'), JSON.stringify({ name: '@workos/skills', version: '9.9.9' })); + const deepSkillsDir = join(packageRoot, 'plugins/workos/skills'); + mkdirSync(deepSkillsDir, { recursive: true }); + mkdirSync(join(deepSkillsDir, 'skill-a')); + writeFileSync(join(deepSkillsDir, 'skill-a', 'SKILL.md'), '# Skill A'); + vi.mocked(getSkillsDir).mockReturnValue(deepSkillsDir); + + mkdirSync(join(homeDir, '.claude')); + + const result = await autoInstallSkills(); + + expect(result).not.toBeNull(); + expect(result!.version).toBe('9.9.9'); + + const marker = join(homeDir, '.claude/skills', SKILL_VERSION_MARKER_FILENAME); + expect(existsSync(marker)).toBe(true); + expect(readFileSync(marker, 'utf8')).toBe('9.9.9'); + }); + + it('returns null when no agents are detected', async () => { mkdirSync(join(skillsDir, 'skill-a')); writeFileSync(join(skillsDir, 'skill-a', 'SKILL.md'), '# Skill A'); // No agent directories created — none detected - await expect(autoInstallSkills()).resolves.toBeUndefined(); + await expect(autoInstallSkills()).resolves.toBeNull(); }); - it('no-ops silently when no skills are discovered', async () => { + it('returns null when no skills are discovered', async () => { mkdirSync(join(homeDir, '.claude')); // No skills in skillsDir - await expect(autoInstallSkills()).resolves.toBeUndefined(); + await expect(autoInstallSkills()).resolves.toBeNull(); }); - it('swallows errors from discoverSkills', async () => { + it('swallows errors from discoverSkills and returns null', async () => { // Point to a nonexistent skills directory const { getSkillsDir } = await import('@workos/skills'); vi.mocked(getSkillsDir).mockReturnValue('/nonexistent/path'); - await expect(autoInstallSkills()).resolves.toBeUndefined(); + await expect(autoInstallSkills()).resolves.toBeNull(); }); - it('resolves silently when installSkill returns failure', async () => { + it('returns null when every installSkill call fails', async () => { // installSkill returns { success: false } on copy errors (doesn't throw). - // Verify autoInstallSkills completes without throwing even when installs fail. - // Simulate by creating a skill dir with SKILL.md for discovery, then making - // the target agent dir read-only so copyFile fails. + // When nothing succeeded, callers should get null so they don't advertise + // a bogus "installed" message. mkdirSync(join(skillsDir, 'test-skill')); writeFileSync(join(skillsDir, 'test-skill', 'SKILL.md'), '# Test'); - mkdirSync(join(homeDir, '.claude')); - // Create a file where the skills directory should be, so mkdir fails - mkdirSync(join(homeDir, '.claude/skills')); - writeFileSync(join(homeDir, '.claude/skills/test-skill'), 'not a directory'); + mkdirSync(join(homeDir, '.claude/skills'), { recursive: true }); + // Make the skills parent read-only so mkdtemp inside it fails with EACCES. + chmodSync(join(homeDir, '.claude/skills'), 0o555); - await expect(autoInstallSkills()).resolves.toBeUndefined(); + try { + await expect(autoInstallSkills()).resolves.toBeNull(); + } finally { + // Restore writable so afterEach cleanup can rm. + chmodSync(join(homeDir, '.claude/skills'), 0o755); + } }); it('does not produce any console output', async () => { @@ -300,4 +475,100 @@ describe('install-skill', () => { errorSpy.mockRestore(); }); }); + + describe('refreshWorkOSSkills', () => { + beforeEach(async () => { + const { homedir } = await import('os'); + const { getSkillsDir } = await import('@workos/skills'); + vi.mocked(homedir).mockReturnValue(homeDir); + vi.mocked(getSkillsDir).mockReturnValue(skillsDir); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('reports per-agent before/after marker state keyed by agent.name', async () => { + const { SKILL_VERSION_MARKER_FILENAME } = await import('./install-skill.js'); + + mkdirSync(join(skillsDir, 'skill-a')); + writeFileSync(join(skillsDir, 'skill-a', 'SKILL.md'), '# Skill A'); + + // Plant a pre-existing marker for one agent so we have a non-null "before". + mkdirSync(join(homeDir, '.claude/skills'), { recursive: true }); + writeFileSync(join(homeDir, '.claude/skills', SKILL_VERSION_MARKER_FILENAME), '0.0.1', 'utf8'); + + // Codex has no prior marker. + mkdirSync(join(homeDir, '.codex')); + + const result = await refreshWorkOSSkills(); + + expect(result).not.toBeNull(); + expect(result!.perAgentBefore).toMatchObject({ + 'claude-code': '0.0.1', + codex: null, + }); + // After: in this fixture skillsDir isn't an npm package layout, so + // getBundledSkillsVersion returns null and no marker is rewritten. + // Therefore perAgentAfter must equal perAgentBefore for every agent. + expect(result!.perAgentAfter).toMatchObject({ + 'claude-code': '0.0.1', + codex: null, + }); + }); + + it('skips marker writing when writeMarker is false', async () => { + const { SKILL_VERSION_MARKER_FILENAME } = await import('./install-skill.js'); + + mkdirSync(join(skillsDir, 'skill-a')); + writeFileSync(join(skillsDir, 'skill-a', 'SKILL.md'), '# Skill A'); + mkdirSync(join(homeDir, '.claude')); + + const result = await refreshWorkOSSkills({ writeMarker: false }); + + expect(result).not.toBeNull(); + const marker = join(homeDir, '.claude/skills', SKILL_VERSION_MARKER_FILENAME); + expect(existsSync(marker)).toBe(false); + }); + + it('filters skills by the skills option', async () => { + mkdirSync(join(skillsDir, 'skill-a')); + writeFileSync(join(skillsDir, 'skill-a', 'SKILL.md'), '# A'); + mkdirSync(join(skillsDir, 'skill-b')); + writeFileSync(join(skillsDir, 'skill-b', 'SKILL.md'), '# B'); + mkdirSync(join(homeDir, '.claude')); + + const result = await refreshWorkOSSkills({ skills: ['skill-a'] }); + + expect(result).not.toBeNull(); + expect(result!.skills).toEqual(['skill-a']); + expect(existsSync(join(homeDir, '.claude/skills/skill-a/SKILL.md'))).toBe(true); + expect(existsSync(join(homeDir, '.claude/skills/skill-b'))).toBe(false); + }); + + it('uses the agents option instead of detecting from $HOME', async () => { + mkdirSync(join(skillsDir, 'skill-a')); + writeFileSync(join(skillsDir, 'skill-a', 'SKILL.md'), '# A'); + + // No agent dirs in homeDir — detectAgents would return []. + // Pass an explicit agent so the install proceeds anyway. + const explicitAgent: AgentConfig = { + name: 'manual', + displayName: 'Manual Agent', + globalSkillsDir: join(homeDir, 'manual/skills'), + detect: () => true, + }; + + const result = await refreshWorkOSSkills({ agents: [explicitAgent] }); + + expect(result).not.toBeNull(); + expect(result!.agents.map((a) => a.name)).toEqual(['manual']); + expect(existsSync(join(homeDir, 'manual/skills/skill-a/SKILL.md'))).toBe(true); + }); + + it('returns null when no agents and no skills', async () => { + // Empty skillsDir, no detected agents. + await expect(refreshWorkOSSkills()).resolves.toBeNull(); + }); + }); }); diff --git a/src/commands/install-skill.ts b/src/commands/install-skill.ts index 749f71f8..6cecc35b 100644 --- a/src/commands/install-skill.ts +++ b/src/commands/install-skill.ts @@ -1,10 +1,45 @@ import { homedir } from 'os'; -import { join } from 'path'; +import { dirname, join } from 'path'; import { existsSync } from 'fs'; -import { mkdir, copyFile, readdir } from 'fs/promises'; +import { mkdir, mkdtemp, cp, rename, rm, readdir, readFile, stat, access, writeFile } from 'fs/promises'; import chalk from 'chalk'; import { getSkillsDir as getSkillsPackageDir } from '@workos/skills'; +export const SKILL_VERSION_MARKER_FILENAME = '.workos-skill-version'; + +// Stale-orphan cutoff for `.workos.tmp-*` / `.workos.bak-*` siblings left behind +// by a crashed prior run. Anything younger may belong to a concurrent install +// and must NOT be removed. +const ORPHAN_STALE_MS = 60 * 60 * 1000; + +/** Async equivalent of `existsSync` — `access` rejects with ENOENT when missing. */ +async function pathExists(p: string): Promise { + try { + await access(p); + return true; + } catch { + return false; + } +} + +/** + * Read the bundled @workos/skills version by walking up from the skills + * directory to the package.json. The package's `exports` map doesn't expose + * package.json, so we resolve it by filesystem convention. + * Returns null if the version can't be determined — callers treat that as + * "no marker written" rather than failing the install. + */ +export async function getBundledSkillsVersion(skillsDir: string = getSkillsPackageDir()): Promise { + try { + // skillsDir = /plugins/workos/skills + const packageRoot = dirname(dirname(dirname(skillsDir))); + const pkgJson = JSON.parse(await readFile(join(packageRoot, 'package.json'), 'utf8')); + return typeof pkgJson.version === 'string' ? pkgJson.version : null; + } catch { + return null; + } +} + export interface AgentConfig { name: string; displayName: string; @@ -53,7 +88,9 @@ export function getSkillsDir(): string { export async function discoverSkills(skillsDir: string): Promise { const entries = await readdir(skillsDir, { withFileTypes: true }); - return entries.filter((e) => e.isDirectory() && existsSync(join(skillsDir, e.name, 'SKILL.md'))).map((e) => e.name); + const dirs = entries.filter((e) => e.isDirectory()); + const checks = await Promise.all(dirs.map((e) => pathExists(join(skillsDir, e.name, 'SKILL.md')))); + return dirs.filter((_, i) => checks[i]).map((e) => e.name); } export function detectAgents(agents: Record, filter?: string[]): AgentConfig[] { @@ -69,20 +106,63 @@ export function detectAgents(agents: Record, filter?: strin return detected; } +/** + * Recursively install a skill directory (SKILL.md + references/ + any other + * files) with prune-replace semantics. Uses a sibling temp dir + backup-rename + * pattern so the operation is effectively atomic per skill: the target either + * matches the source exactly, or (on rollback) is restored to its prior state. + * + * Returns `{ success, error }` rather than throwing — callers (autoInstallSkills, + * runInstallSkill) accumulate failures across the (skill × agent) matrix. + */ export async function installSkill( skillsDir: string, skillName: string, agent: AgentConfig, ): Promise<{ success: boolean; error?: string }> { - const sourceFile = join(skillsDir, skillName, 'SKILL.md'); + const sourceDir = join(skillsDir, skillName); const targetDir = join(agent.globalSkillsDir, skillName); - const targetFile = join(targetDir, 'SKILL.md'); + const parent = dirname(targetDir); + // Setup (mkdir parent, mkdtemp) is inside the try so EACCES / ENOTDIR / etc. + // surface as `{ success: false }` rather than rejecting — runInstallSkill and + // refreshWorkOSSkills accumulate failures across the (skill × agent) matrix + // and would otherwise abort the whole batch on a single bad agent dir. + let tempDir: string | undefined; try { - await mkdir(targetDir, { recursive: true }); - await copyFile(sourceFile, targetFile); + await mkdir(parent, { recursive: true }); + // Best-effort cleanup of OLD orphans only — never current-run paths. + await cleanupStaleOrphans(parent, skillName).catch(() => {}); + + // mkdtemp gives us atomic creation + a random suffix that prevents + // collisions between concurrent installers. + tempDir = await mkdtemp(join(parent, `.workos.tmp-${skillName}-`)); + const backupDir = tempDir.replace('.workos.tmp-', '.workos.bak-'); + + await cp(sourceDir, tempDir, { recursive: true, errorOnExist: false }); + + const targetExisted = await pathExists(targetDir); + if (targetExisted) { + await rename(targetDir, backupDir); + } + try { + await rename(tempDir, targetDir); + } catch (renameErr) { + if (targetExisted) { + await rename(backupDir, targetDir).catch(() => {}); + } + throw renameErr; + } + // Backup cleanup is best-effort: target is already in place, so failure + // here leaves a stale backup that the next run's cleanup handles after 1h. + if (targetExisted) { + await rm(backupDir, { recursive: true, force: true }).catch(() => {}); + } return { success: true }; } catch (error) { + if (tempDir) { + await rm(tempDir, { recursive: true, force: true }).catch(() => {}); + } return { success: false, error: error instanceof Error ? error.message : 'Unknown error', @@ -90,6 +170,26 @@ export async function installSkill( } } +/** + * Remove `.workos.tmp-{skillName}-*` and `.workos.bak-{skillName}-*` siblings + * older than ORPHAN_STALE_MS. Fresh siblings (from a concurrent install) are + * preserved — destroying them would race the other run's final rename. + */ +async function cleanupStaleOrphans(parent: string, skillName: string): Promise { + if (!(await pathExists(parent))) return; + const entries = await readdir(parent).catch(() => []); + const cutoff = Date.now() - ORPHAN_STALE_MS; + for (const entry of entries) { + const isOrphan = entry.startsWith(`.workos.tmp-${skillName}-`) || entry.startsWith(`.workos.bak-${skillName}-`); + if (!isOrphan) continue; + const path = join(parent, entry); + const st = await stat(path).catch(() => null); + if (st && st.mtimeMs < cutoff) { + await rm(path, { recursive: true, force: true }).catch(() => {}); + } + } +} + export async function runInstallSkill(options: InstallSkillOptions): Promise { const home = homedir(); const agents = createAgents(home); @@ -120,7 +220,7 @@ export async function runInstallSkill(options: InstallSkillOptions): Promise = []; @@ -128,11 +228,7 @@ export async function runInstallSkill(options: InstallSkillOptions): Promise 0) { console.log(chalk.green(`✓ Installed ${successful.length} skill(s):\n`)); for (const r of successful) { - console.log(` ${chalk.cyan(r.skill)} → ${chalk.dim(r.agent)}`); + console.log(` ${chalk.cyan(r.skill)} → ${chalk.dim(r.agent.displayName)}`); + } + } + + // Write per-agent version markers for any agent that had at least one + // successful install, so `workos doctor` doesn't immediately flag the + // freshly-installed skills as stale or missing. Same primitive as + // refreshWorkOSSkills — single source of truth for marker semantics. + const version = await getBundledSkillsVersion(skillsDir); + if (version) { + const succeededAgents = new Set(); + for (const r of successful) succeededAgents.add(r.agent); + for (const agent of succeededAgents) { + await writeAgentSkillMarker(agent, version); } } if (failed.length > 0) { console.log(chalk.red(`\n✗ Failed to install ${failed.length}:\n`)); for (const r of failed) { - console.log(` ${r.skill} → ${r.agent}: ${chalk.dim(r.error)}`); + console.log(` ${r.skill} → ${r.agent.displayName}: ${chalk.dim(r.error)}`); } process.exit(1); } @@ -157,26 +266,139 @@ export async function runInstallSkill(options: InstallSkillOptions): Promise; + /** Marker version per agent.name AFTER refresh. */ + perAgentAfter: Record; +} + +async function readSkillVersionMarker(agent: AgentConfig): Promise { + const path = join(agent.globalSkillsDir, SKILL_VERSION_MARKER_FILENAME); + try { + return (await readFile(path, 'utf8')).trim() || null; + } catch { + return null; + } +} + /** - * Silently install all bundled skills to all detected coding agents. - * Errors are swallowed — this must never disrupt the calling flow. + * Best-effort marker write — any failure is swallowed (filesystem permission + * errors shouldn't fail the install; doctor treats missing markers as "unknown"). + * Single source of truth for the .workos-skill-version write semantics. */ -export async function autoInstallSkills(): Promise { +async function writeAgentSkillMarker(agent: AgentConfig, version: string): Promise { try { - const home = homedir(); - const agents = createAgents(home); - const skillsDir = getSkillsDir(); - const skills = await discoverSkills(skillsDir); - const targetAgents = detectAgents(agents); + await writeFile(join(agent.globalSkillsDir, SKILL_VERSION_MARKER_FILENAME), version, 'utf8'); + } catch { + // Marker is best-effort; doctor treats missing marker as "unknown". + } +} + +/** + * Reusable primitive: discover bundled skills, install each one to each agent, + * write per-agent version markers, and report before/after marker state. + * + * Both `autoInstallSkills` (best-effort hook called from install/login) and + * `doctor --fix` (Phase 3) call this — there is no duplicate copy logic. + * + * Returns null when nothing applied (no agents detected, no skills found, or + * every install attempt failed). + */ +export async function refreshWorkOSSkills(opts: RefreshOptions = {}): Promise { + const home = homedir(); + const skillsDir = getSkillsDir(); + const detected = opts.agents ?? detectAgents(createAgents(home)); + const allSkills = await discoverSkills(skillsDir).catch(() => []); + const skills = opts.skills ? allSkills.filter((s) => opts.skills!.includes(s)) : allSkills; + const writeMarker = opts.writeMarker ?? true; + + if (skills.length === 0 || detected.length === 0) return null; + + const version = await getBundledSkillsVersion(skillsDir); + const perAgentBefore: Record = {}; + const perAgentAfter: Record = {}; + const succeededAgents: AgentConfig[] = []; + // Union of skills that succeeded for at least one agent. Returning the full + // attempted list would inflate "Installed N skills" copy when some skills + // failed to copy; only count what actually landed somewhere. + const installedSkills = new Set(); - if (skills.length === 0 || targetAgents.length === 0) return; + for (const agent of detected) { + perAgentBefore[agent.name] = await readSkillVersionMarker(agent); + let agentSucceeded = false; for (const skill of skills) { - for (const agent of targetAgents) { - await installSkill(skillsDir, skill, agent); + const result = await installSkill(skillsDir, skill, agent); + if (result.success) { + agentSucceeded = true; + installedSkills.add(skill); } } + + if (agentSucceeded) { + succeededAgents.push(agent); + if (writeMarker && version) { + await writeAgentSkillMarker(agent, version); + } + } + + perAgentAfter[agent.name] = await readSkillVersionMarker(agent); + } + + if (succeededAgents.length === 0) return null; + + return { + agents: succeededAgents, + skills: skills.filter((s) => installedSkills.has(s)), + version, + perAgentBefore, + perAgentAfter, + }; +} + +/** + * Install all bundled skills to all detected coding agents. + * Returns a summary when anything was installed, or null when nothing applied. + * Performs minimal IO: writes a version marker file alongside installed + * skills so `workos doctor` can detect staleness later. Errors are swallowed + * so skill install never disrupts the calling flow. + * + * Thin back-compat wrapper around `refreshWorkOSSkills` — the install/auth-login + * call sites use this; doctor `--fix` (Phase 3) calls `refreshWorkOSSkills` + * directly to surface the per-agent before/after marker state. + */ +export async function autoInstallSkills(): Promise { + try { + const result = await refreshWorkOSSkills(); + if (!result) return null; + return { + skills: result.skills, + agents: result.agents.map((a) => a.displayName), + version: result.version, + }; } catch { - // Intentionally swallowed — skill install is best-effort + return null; } } diff --git a/src/commands/install.spec.ts b/src/commands/install.spec.ts index b28e09aa..2e3031ff 100644 --- a/src/commands/install.spec.ts +++ b/src/commands/install.spec.ts @@ -25,6 +25,8 @@ vi.mock('../utils/debug.js', () => ({ const { runInstaller } = await import('../run.js'); const { autoInstallSkills } = await import('./install-skill.js'); +const clack = (await import('../utils/clack.js')).default; +const { isJsonMode } = await import('../utils/output.js'); vi.spyOn(process, 'exit').mockImplementation((() => { throw new Error('process.exit called'); @@ -39,7 +41,7 @@ describe('handleInstall', () => { it('calls autoInstallSkills after successful install', async () => { vi.mocked(runInstaller).mockResolvedValue(undefined as any); - vi.mocked(autoInstallSkills).mockResolvedValue(undefined); + vi.mocked(autoInstallSkills).mockResolvedValue(null); await expect(handleInstall({ _: ['install'], $0: 'workos' } as any)).rejects.toThrow('process.exit called'); @@ -52,6 +54,44 @@ describe('handleInstall', () => { expect(autoInstallOrder).toBeGreaterThan(runInstallerOrder); }); + it('prints an info line when skills were installed in a TTY session', async () => { + vi.mocked(runInstaller).mockResolvedValue(undefined as any); + vi.mocked(autoInstallSkills).mockResolvedValue({ + skills: ['workos', 'workos-widgets'], + agents: ['Claude Code'], + version: '0.4.0', + }); + vi.mocked(isJsonMode).mockReturnValue(false); + + await expect(handleInstall({ _: ['install'], $0: 'workos' } as any)).rejects.toThrow('process.exit called'); + + expect(clack.log.info).toHaveBeenCalledWith(expect.stringContaining('Installed 2 WorkOS skills for Claude Code')); + }); + + it('does not print the info line when autoInstallSkills returns null', async () => { + vi.mocked(runInstaller).mockResolvedValue(undefined as any); + vi.mocked(autoInstallSkills).mockResolvedValue(null); + vi.mocked(isJsonMode).mockReturnValue(false); + + await expect(handleInstall({ _: ['install'], $0: 'workos' } as any)).rejects.toThrow('process.exit called'); + + expect(clack.log.info).not.toHaveBeenCalled(); + }); + + it('suppresses the info line in JSON mode', async () => { + vi.mocked(runInstaller).mockResolvedValue(undefined as any); + vi.mocked(autoInstallSkills).mockResolvedValue({ + skills: ['workos'], + agents: ['Claude Code'], + version: '0.4.0', + }); + vi.mocked(isJsonMode).mockReturnValue(true); + + await expect(handleInstall({ _: ['install'], $0: 'workos' } as any)).rejects.toThrow('process.exit called'); + + expect(clack.log.info).not.toHaveBeenCalled(); + }); + it('does not call autoInstallSkills when runInstaller throws', async () => { vi.mocked(runInstaller).mockRejectedValue(new Error('install failed')); diff --git a/src/commands/install.ts b/src/commands/install.ts index c023c995..d2b10dd8 100644 --- a/src/commands/install.ts +++ b/src/commands/install.ts @@ -29,7 +29,13 @@ export async function handleInstall(argv: ArgumentsCamelCase): Pr try { await runInstaller(options); - await autoInstallSkills(); + const skillResult = await autoInstallSkills(); + if (skillResult && !isJsonMode()) { + const skillWord = skillResult.skills.length === 1 ? 'skill' : 'skills'; + clack.log.info( + `Installed ${skillResult.skills.length} WorkOS ${skillWord} for ${skillResult.agents.join(', ')}. Your coding agent now has up-to-date WorkOS guidance.`, + ); + } process.exit(0); } catch (err) { const { getLogFilePath } = await import('../utils/debug.js'); diff --git a/src/commands/login.spec.ts b/src/commands/login.spec.ts index bb15df3e..eaf1c705 100644 --- a/src/commands/login.spec.ts +++ b/src/commands/login.spec.ts @@ -33,6 +33,16 @@ vi.mock('../lib/staging-api.js', () => ({ fetchStagingCredentials: (...args: unknown[]) => mockFetchStagingCredentials(...args), })); +// Mock skill install + JSON mode — installSkillsAfterLogin tests drive both. +vi.mock('./install-skill.js', () => ({ + autoInstallSkills: vi.fn(), +})); + +vi.mock('../utils/output.js', () => ({ + isJsonMode: vi.fn(() => false), + exitWithError: vi.fn(), +})); + let testDir: string; vi.mock('node:os', async (importOriginal) => { @@ -48,7 +58,10 @@ vi.mock('node:os', async (importOriginal) => { }); const { getConfig, setInsecureConfigStorage, clearConfig } = await import('../lib/config-store.js'); -const { provisionStagingEnvironment } = await import('./login.js'); +const { provisionStagingEnvironment, installSkillsAfterLogin } = await import('./login.js'); +const { autoInstallSkills } = await import('./install-skill.js'); +const { isJsonMode } = await import('../utils/output.js'); +const clackMod = await import('../utils/clack.js'); describe('login', () => { beforeEach(() => { @@ -189,4 +202,85 @@ describe('login', () => { expect(result).toBe(false); }); }); + + describe('installSkillsAfterLogin', () => { + it('invokes autoInstallSkills', async () => { + vi.mocked(autoInstallSkills).mockResolvedValueOnce(null); + + await installSkillsAfterLogin(); + + expect(autoInstallSkills).toHaveBeenCalledOnce(); + }); + + it('returns without throwing when autoInstallSkills rejects', async () => { + vi.mocked(autoInstallSkills).mockRejectedValueOnce(new Error('install boom')); + + // The whole point of the helper: login must keep its success even when + // skill install fails. Asserting no rejection IS the test. + await expect(installSkillsAfterLogin()).resolves.toBeUndefined(); + }); + + it('logs a one-line success message in human mode', async () => { + vi.mocked(autoInstallSkills).mockResolvedValueOnce({ + skills: ['workos', 'workos-widgets'], + agents: ['Claude Code', 'Codex'], + version: '0.4.0', + }); + + const infoSpy = vi.mocked(clackMod.default.log.info); + infoSpy.mockClear(); + + await installSkillsAfterLogin(); + + expect(infoSpy).toHaveBeenCalledOnce(); + const message = infoSpy.mock.calls[0]?.[0] as string; + expect(message).toContain('2 WorkOS skills'); + expect(message).toContain('Claude Code'); + expect(message).toContain('Codex'); + }); + + it('uses singular "skill" when exactly one skill installed', async () => { + vi.mocked(autoInstallSkills).mockResolvedValueOnce({ + skills: ['workos'], + agents: ['Claude Code'], + version: '0.4.0', + }); + + const infoSpy = vi.mocked(clackMod.default.log.info); + infoSpy.mockClear(); + + await installSkillsAfterLogin(); + + const message = infoSpy.mock.calls[0]?.[0] as string; + expect(message).toContain('1 WorkOS skill '); + expect(message).not.toContain('1 WorkOS skills'); + }); + + it('skips logging in JSON mode', async () => { + vi.mocked(isJsonMode).mockReturnValueOnce(true); + vi.mocked(autoInstallSkills).mockResolvedValueOnce({ + skills: ['workos'], + agents: ['Claude Code'], + version: '0.4.0', + }); + + const infoSpy = vi.mocked(clackMod.default.log.info); + infoSpy.mockClear(); + + await installSkillsAfterLogin(); + + expect(infoSpy).not.toHaveBeenCalled(); + }); + + it('skips logging when autoInstallSkills returns null', async () => { + vi.mocked(autoInstallSkills).mockResolvedValueOnce(null); + + const infoSpy = vi.mocked(clackMod.default.log.info); + infoSpy.mockClear(); + + await installSkillsAfterLogin(); + + expect(infoSpy).not.toHaveBeenCalled(); + }); + }); }); diff --git a/src/commands/login.ts b/src/commands/login.ts index ca033d61..87edbd69 100644 --- a/src/commands/login.ts +++ b/src/commands/login.ts @@ -9,6 +9,8 @@ import { fetchStagingCredentials } from '../lib/staging-api.js'; import { getConfig, saveConfig } from '../lib/config-store.js'; import type { CliConfig } from '../lib/config-store.js'; import { formatWorkOSCommand } from '../utils/command-invocation.js'; +import { autoInstallSkills } from './install-skill.js'; +import { isJsonMode } from '../utils/output.js'; /** * Parse JWT payload @@ -70,6 +72,40 @@ function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } +/** + * Best-effort skill install after a successful auth-login. + * + * Mirrors the install.ts hook copy, but wraps `autoInstallSkills` in its own + * try/catch AND a 30s timeout so a skill install hang (e.g. blocked filesystem + * call) never blocks login completion. Login already succeeded by the time + * this runs — the user having a working session is the contract that must hold. + * + * Extracted from runLogin so it can be unit-tested without standing up the + * device-auth polling loop. + */ +export const SKILL_INSTALL_TIMEOUT_MS = 30 * 1000; + +export async function installSkillsAfterLogin(): Promise { + let timeoutHandle: ReturnType | undefined; + try { + const timeout = new Promise((resolve) => { + timeoutHandle = setTimeout(() => resolve(null), SKILL_INSTALL_TIMEOUT_MS); + // Don't keep the event loop alive on this timer — process should exit + // immediately if everything else has resolved. + timeoutHandle.unref?.(); + }); + const result = await Promise.race([autoInstallSkills(), timeout]); + if (result && !isJsonMode()) { + const skillWord = result.skills.length === 1 ? 'skill' : 'skills'; + clack.log.info(`Installed ${result.skills.length} WorkOS ${skillWord} for ${result.agents.join(', ')}.`); + } + } catch { + // Skill install must never fail login. + } finally { + if (timeoutHandle) clearTimeout(timeoutHandle); + } +} + /** * Auto-provision a staging environment after login. * @@ -225,6 +261,10 @@ export async function runLogin(): Promise { } else { clack.log.info(chalk.dim('Run `workos env add` to configure an environment manually')); } + + // Best-effort skill install. Wrapped helper guarantees login never + // fails on skill errors. + await installSkillsAfterLogin(); return; } diff --git a/src/doctor/checks/skills-fix.spec.ts b/src/doctor/checks/skills-fix.spec.ts new file mode 100644 index 00000000..edfb503d --- /dev/null +++ b/src/doctor/checks/skills-fix.spec.ts @@ -0,0 +1,197 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'fs'; +import { tmpdir } from 'os'; +import { join } from 'path'; +import type { SkillsInfo } from '../types.js'; + +// Mock the two collaborators maybeRefreshSkills depends on. +vi.mock('../../commands/install-skill.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + refreshWorkOSSkills: vi.fn(), + }; +}); + +vi.mock('./skills.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + checkSkills: vi.fn(), + }; +}); + +const { refreshWorkOSSkills } = await import('../../commands/install-skill.js'); +const { checkSkills } = await import('./skills.js'); +const { maybeRefreshSkills, FIXABLE_SKILLS } = await import('../index.js'); + +const STALE: SkillsInfo = { + bundledVersion: '0.4.0', + agents: [ + { agent: 'Claude Code', installedVersion: '0.2.4', stale: true }, + { agent: 'Codex', installedVersion: null, stale: false }, + ], +}; + +const CURRENT: SkillsInfo = { + bundledVersion: '0.4.0', + agents: [ + { agent: 'Claude Code', installedVersion: '0.4.0', stale: false }, + { agent: 'Codex', installedVersion: '0.4.0', stale: false }, + ], +}; + +describe('maybeRefreshSkills', () => { + beforeEach(() => { + vi.mocked(refreshWorkOSSkills).mockReset(); + vi.mocked(checkSkills).mockReset(); + }); + + it('does NOT call refresh when fix=false even with stale skills', async () => { + const result = await maybeRefreshSkills({ fix: false }, STALE); + + expect(refreshWorkOSSkills).not.toHaveBeenCalled(); + expect(checkSkills).not.toHaveBeenCalled(); + expect(result.skillsRefresh).toBeUndefined(); + expect(result.skills).toBe(STALE); + }); + + it('does NOT call refresh when skills info is undefined', async () => { + const result = await maybeRefreshSkills({ fix: true }, undefined); + + expect(refreshWorkOSSkills).not.toHaveBeenCalled(); + expect(result.skillsRefresh).toBeUndefined(); + expect(result.skills).toBeUndefined(); + }); + + it('does NOT call refresh when no agent is stale or missing-marker', async () => { + const result = await maybeRefreshSkills({ fix: true }, CURRENT); + + expect(refreshWorkOSSkills).not.toHaveBeenCalled(); + expect(result.skillsRefresh).toBeUndefined(); + expect(result.skills).toBe(CURRENT); + }); + + it('passes ONLY the FIXABLE_SKILLS allowlist into refreshWorkOSSkills', async () => { + vi.mocked(refreshWorkOSSkills).mockResolvedValueOnce(null); + + await maybeRefreshSkills({ fix: true }, STALE); + + expect(refreshWorkOSSkills).toHaveBeenCalledOnce(); + expect(refreshWorkOSSkills).toHaveBeenCalledWith({ + skills: [...FIXABLE_SKILLS], + }); + expect(FIXABLE_SKILLS).toEqual(['workos', 'workos-widgets']); + }); + + it('returns skillsRefresh and post-refresh skills when refresh succeeds', async () => { + vi.mocked(refreshWorkOSSkills).mockResolvedValueOnce({ + // refreshWorkOSSkills's return shape — only the fields maybeRefreshSkills reads matter here. + agents: [], + skills: ['workos', 'workos-widgets'], + version: '0.4.0', + perAgentBefore: { 'claude-code': '0.2.4', codex: null }, + perAgentAfter: { 'claude-code': '0.4.0', codex: '0.4.0' }, + }); + vi.mocked(checkSkills).mockResolvedValueOnce({ + bundledVersion: '0.4.0', + agents: [ + { agent: 'Claude Code', installedVersion: '0.4.0', stale: false }, + { agent: 'Codex', installedVersion: '0.4.0', stale: false }, + ], + }); + + const result = await maybeRefreshSkills({ fix: true }, STALE); + + expect(result.skillsRefresh).toEqual({ + before: { 'claude-code': '0.2.4', codex: null }, + after: { 'claude-code': '0.4.0', codex: '0.4.0' }, + skillsInstalled: ['workos', 'workos-widgets'], + }); + // Re-read happens, so post-refresh state replaces the original. + expect(result.skills?.agents.every((a) => !a.stale)).toBe(true); + expect(checkSkills).toHaveBeenCalledOnce(); + }); + + it('returns the original skills (and no skillsRefresh) when refresh produces null', async () => { + // refreshWorkOSSkills returns null when no agents detected or all installs failed. + vi.mocked(refreshWorkOSSkills).mockResolvedValueOnce(null); + + const result = await maybeRefreshSkills({ fix: true }, STALE); + + expect(result.skillsRefresh).toBeUndefined(); + // Original skills preserved — we don't re-read on null refresh. + expect(result.skills).toBe(STALE); + expect(checkSkills).not.toHaveBeenCalled(); + }); +}); + +describe('--fix sibling protection (integration via real refreshWorkOSSkills)', () => { + let testDir: string; + let homeDir: string; + + beforeEach(async () => { + // The `vi.mock` calls above replace these for the unit-test suite; + // restore them here so this suite uses the real implementations. + vi.doUnmock('../../commands/install-skill.js'); + vi.doUnmock('./skills.js'); + vi.resetModules(); + + testDir = mkdtempSync(join(tmpdir(), 'doctor-fix-sibling-')); + homeDir = join(testDir, 'home'); + mkdirSync(homeDir); + }); + + afterEach(() => { + rmSync(testDir, { recursive: true, force: true }); + vi.resetAllMocks(); + vi.resetModules(); + }); + + it('only writes to skills on the FIXABLE_SKILLS allowlist; planted siblings are untouched', async () => { + // Re-import after unmocking so we get the real impls. The real + // refreshWorkOSSkills resolves its skill source via @workos/skills's + // getSkillsDir(), so we cannot mock the source side here — the meaningful + // assertion is target-side: pre-existing sibling skill dirs at the agent's + // target path must NOT be touched, regardless of what's in the bundled + // source. The opts.skills allowlist is what scopes refresh. + const { refreshWorkOSSkills: realRefresh, createAgents } = await import('../../commands/install-skill.js'); + const { FIXABLE_SKILLS: realFixable } = await import('../index.js'); + + const claudeSkillsDir = join(homeDir, '.claude/skills'); + mkdirSync(claudeSkillsDir, { recursive: true }); + + // Third-party skill — must survive the refresh. + mkdirSync(join(claudeSkillsDir, 'some-third-party-skill')); + writeFileSync(join(claudeSkillsDir, 'some-third-party-skill', 'SKILL.md'), '# Third Party (do not touch)'); + + // Hypothetical future bundled skill at the target but NOT on allowlist — + // also must survive (allowlist defends both source-side and target-side). + mkdirSync(join(claudeSkillsDir, 'workos-future-skill')); + writeFileSync(join(claudeSkillsDir, 'workos-future-skill', 'SKILL.md'), '# Future (do not touch)'); + + const explicitAgent = createAgents(homeDir)['claude-code']; + const result = await realRefresh({ + agents: [explicitAgent], + skills: [...realFixable], + writeMarker: false, + }); + + expect(result).not.toBeNull(); + // The result.skills set is the intersection of allowlist and what's actually + // bundled. workos and workos-widgets are both bundled today. + expect(result!.skills.sort()).toEqual(['workos', 'workos-widgets']); + + // Allowlist names WERE installed. + expect(existsSync(join(claudeSkillsDir, 'workos/SKILL.md'))).toBe(true); + expect(existsSync(join(claudeSkillsDir, 'workos-widgets/SKILL.md'))).toBe(true); + + // Siblings (whether unrelated or hypothetical-future) are byte-identical. + expect(readFileSync(join(claudeSkillsDir, 'some-third-party-skill/SKILL.md'), 'utf-8')).toBe( + '# Third Party (do not touch)', + ); + expect(readFileSync(join(claudeSkillsDir, 'workos-future-skill/SKILL.md'), 'utf-8')).toBe( + '# Future (do not touch)', + ); + }); +}); diff --git a/src/doctor/checks/skills.spec.ts b/src/doctor/checks/skills.spec.ts new file mode 100644 index 00000000..9aacc1b3 --- /dev/null +++ b/src/doctor/checks/skills.spec.ts @@ -0,0 +1,127 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'fs'; +import { join } from 'path'; +import { tmpdir } from 'os'; +import { SKILL_VERSION_MARKER_FILENAME } from '../../commands/install-skill.js'; + +// Mock getBundledSkillsVersion so we don't depend on the real bundled package. +vi.mock('../../commands/install-skill.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + getBundledSkillsVersion: vi.fn(), + }; +}); + +const { getBundledSkillsVersion } = await import('../../commands/install-skill.js'); +const { checkSkills } = await import('./skills.js'); + +describe('checkSkills', () => { + let testHome: string; + + beforeEach(() => { + testHome = mkdtempSync(join(tmpdir(), 'skills-check-')); + vi.mocked(getBundledSkillsVersion).mockResolvedValue('0.3.0'); + }); + + afterEach(() => { + rmSync(testHome, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + it('returns null when no agent skills directories exist', async () => { + expect(await checkSkills(testHome)).toBeNull(); + }); + + it('returns null when an agent skills dir exists but has no WorkOS skills (no marker, no workos/, no workos-widgets/)', async () => { + // The agent has its skills/ dir for unrelated user-installed skills. We + // must NOT report it as having WorkOS skills — `doctor --fix` would + // otherwise write workos/ + workos-widgets/ onto an agent that never + // opted in. Marker OR workos/ OR workos-widgets/ subdir is the signal. + mkdirSync(join(testHome, '.claude/skills/some-other-skill'), { recursive: true }); + writeFileSync(join(testHome, '.claude/skills/some-other-skill/SKILL.md'), '# Other'); + + expect(await checkSkills(testHome)).toBeNull(); + }); + + it('reports an agent with workos/ subdir but no marker as installedVersion=null and not stale', async () => { + // Pre-Phase-2 install (only SKILL.md was copied) — a real possible state. + mkdirSync(join(testHome, '.claude/skills/workos'), { recursive: true }); + + const result = await checkSkills(testHome); + + expect(result).not.toBeNull(); + expect(result!.bundledVersion).toBe('0.3.0'); + expect(result!.agents).toEqual([{ agent: 'Claude Code', installedVersion: null, stale: false }]); + }); + + it('reports an agent that only has workos-widgets/ (older explicit install)', async () => { + // workos-widgets is also on the FIXABLE_SKILLS allowlist; an agent that + // installed only it via an older CLI version should still be visible. + mkdirSync(join(testHome, '.claude/skills/workos-widgets'), { recursive: true }); + + const result = await checkSkills(testHome); + + expect(result).not.toBeNull(); + expect(result!.agents).toEqual([{ agent: 'Claude Code', installedVersion: null, stale: false }]); + }); + + it('flags an agent as stale when the marker trails the bundled version', async () => { + const skillsDir = join(testHome, '.claude/skills'); + mkdirSync(skillsDir, { recursive: true }); + writeFileSync(join(skillsDir, SKILL_VERSION_MARKER_FILENAME), '0.2.4'); + + const result = await checkSkills(testHome); + + expect(result!.agents).toEqual([{ agent: 'Claude Code', installedVersion: '0.2.4', stale: true }]); + }); + + it('does not flag stale when marker matches bundled', async () => { + const skillsDir = join(testHome, '.claude/skills'); + mkdirSync(skillsDir, { recursive: true }); + writeFileSync(join(skillsDir, SKILL_VERSION_MARKER_FILENAME), '0.3.0'); + + const result = await checkSkills(testHome); + + expect(result!.agents[0].stale).toBe(false); + }); + + it('never flags stale when bundledVersion is null (unknown)', async () => { + vi.mocked(getBundledSkillsVersion).mockResolvedValue(null); + const skillsDir = join(testHome, '.claude/skills'); + mkdirSync(skillsDir, { recursive: true }); + writeFileSync(join(skillsDir, SKILL_VERSION_MARKER_FILENAME), '0.2.4'); + + const result = await checkSkills(testHome); + + expect(result!.bundledVersion).toBeNull(); + expect(result!.agents[0].stale).toBe(false); + }); + + it('does not flag stale when installed > bundled (downgrade scenario)', async () => { + // String inequality would incorrectly fire here. Semver ordering must not. + const skillsDir = join(testHome, '.claude/skills'); + mkdirSync(skillsDir, { recursive: true }); + writeFileSync(join(skillsDir, SKILL_VERSION_MARKER_FILENAME), '0.5.0'); + + const result = await checkSkills(testHome); + + expect(result!.agents[0].installedVersion).toBe('0.5.0'); + expect(result!.agents[0].stale).toBe(false); + }); + + it('reports each detected agent separately', async () => { + mkdirSync(join(testHome, '.claude/skills'), { recursive: true }); + writeFileSync(join(testHome, '.claude/skills', SKILL_VERSION_MARKER_FILENAME), '0.2.4'); + mkdirSync(join(testHome, '.codex/skills'), { recursive: true }); + writeFileSync(join(testHome, '.codex/skills', SKILL_VERSION_MARKER_FILENAME), '0.3.0'); + + const result = await checkSkills(testHome); + + expect(result!.agents).toHaveLength(2); + const claude = result!.agents.find((a) => a.agent === 'Claude Code')!; + const codex = result!.agents.find((a) => a.agent === 'Codex')!; + expect(claude.stale).toBe(true); + expect(codex.stale).toBe(false); + }); +}); diff --git a/src/doctor/checks/skills.ts b/src/doctor/checks/skills.ts new file mode 100644 index 00000000..eb539889 --- /dev/null +++ b/src/doctor/checks/skills.ts @@ -0,0 +1,89 @@ +import { homedir } from 'node:os'; +import { access, readFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import semver from 'semver'; +import { createAgents, getBundledSkillsVersion, SKILL_VERSION_MARKER_FILENAME } from '../../commands/install-skill.js'; +import type { SkillsInfo, SkillAgentStatus } from '../types.js'; + +async function pathExists(p: string): Promise { + try { + await access(p); + return true; + } catch { + return false; + } +} + +/** + * Stale = installed version is strictly older than the bundled version. + * String inequality would also fire when installed > bundled (user installed + * via a newer CLI then downgraded), and the SKILLS_OUTDATED remediation would + * silently downgrade their agent's skills. Use semver ordering so we only + * recommend an update when the bundled set is actually ahead. + * + * Falls back to string inequality when either version can't be parsed as + * semver — better to flag a possibly-stale skill than to ignore drift entirely. + */ +function isStale(installed: string, bundled: string): boolean { + const installedValid = semver.valid(installed); + const bundledValid = semver.valid(bundled); + if (installedValid && bundledValid) { + return semver.lt(installedValid, bundledValid); + } + return installed !== bundled; +} + +/** + * Check the freshness of auto-installed WorkOS skills across detected coding + * agents. Compares each agent's version marker (written by autoInstallSkills) + * against the bundled @workos/skills version the CLI ships with. Returns null + * when no agents have a WorkOS skill installed at all — no noise for users who + * never installed through the CLI. + */ +export async function checkSkills(home: string = homedir()): Promise { + const bundledVersion = await getBundledSkillsVersion(); + const agents = createAgents(home); + + const statuses: SkillAgentStatus[] = []; + + for (const [, agent] of Object.entries(agents)) { + // Only report on agents that actually have a WorkOS skill installed. + // An agent's `skills/` dir existing (e.g. for unrelated user-installed + // skills) doesn't mean WE installed — and `doctor --fix` would otherwise + // happily write `workos/` and `workos-widgets/` onto an agent that never + // opted in. The marker OR a workos/ / workos-widgets/ subdir is the signal + // (either is enough — older explicit installs of just `workos-widgets` + // shouldn't be invisible to doctor). + const markerPath = join(agent.globalSkillsDir, SKILL_VERSION_MARKER_FILENAME); + const workosSkillDir = join(agent.globalSkillsDir, 'workos'); + const widgetsSkillDir = join(agent.globalSkillsDir, 'workos-widgets'); + const [hasMarker, hasWorkos, hasWidgets] = await Promise.all([ + pathExists(markerPath), + pathExists(workosSkillDir), + pathExists(widgetsSkillDir), + ]); + if (!hasMarker && !hasWorkos && !hasWidgets) continue; + + let installedVersion: string | null = null; + if (hasMarker) { + try { + installedVersion = (await readFile(markerPath, 'utf8')).trim() || null; + } catch { + installedVersion = null; + } + } + + statuses.push({ + agent: agent.displayName, + installedVersion, + stale: Boolean(bundledVersion && installedVersion && isStale(installedVersion, bundledVersion)), + }); + } + + if (statuses.length === 0) return null; + + return { + bundledVersion, + agents: statuses, + }; +} diff --git a/src/doctor/index.ts b/src/doctor/index.ts index 1b652d13..7c3cacb5 100644 --- a/src/doctor/index.ts +++ b/src/doctor/index.ts @@ -7,15 +7,63 @@ import { checkConnectivity } from './checks/connectivity.js'; import { checkDashboardSettings, compareRedirectUris } from './checks/dashboard.js'; import { checkAuthPatterns } from './checks/auth-patterns.js'; import { checkAiAnalysis } from './checks/ai-analysis.js'; +import { checkSkills } from './checks/skills.js'; +import { refreshWorkOSSkills } from '../commands/install-skill.js'; import { detectIssues } from './issues.js'; import { formatReport } from './output.js'; import { formatReportAsJson } from './json-output.js'; import { copyToClipboard } from './clipboard.js'; import Chalk from 'chalk'; -import type { DoctorOptions, DoctorReport } from './types.js'; +import type { DoctorOptions, DoctorReport, SkillsRefreshResult } from './types.js'; const DOCTOR_VERSION = '1.0.0'; +/** + * Skills `--fix` is allowed to refresh. Hardcoded — NOT derived from + * discoverSkills — so future bundled skills require an explicit opt-in here + * before doctor will write to their target directory. This is the contract's + * promise that `--fix` only ever touches `workos/` and `workos-widgets/`. + */ +export const FIXABLE_SKILLS = ['workos', 'workos-widgets'] as const; + +/** + * Refresh stale WorkOS skills if `--fix` is set and at least one agent is + * stale or has no marker. Always re-reads `checkSkills()` after a successful + * refresh so detectIssues sees the post-refresh state and we don't ship a + * doctor report that simultaneously claims "fixed" and "still stale". + * + * Extracted from runDoctor for unit testability — runDoctor itself depends on + * eight upstream checks that are expensive to mock. + */ +export async function maybeRefreshSkills( + options: Pick, + skills: DoctorReport['skills'], +): Promise<{ + skillsRefresh?: SkillsRefreshResult; + skills: DoctorReport['skills']; +}> { + if (!options.fix || !skills) return { skills }; + + const stalePresent = skills.agents.some((a) => a.stale || a.installedVersion === null); + if (!stalePresent) return { skills }; + + const refresh = await refreshWorkOSSkills({ + // Explicit allowlist — NOT discoverSkills — so the contract's + // workos/+workos-widgets-only constraint can't drift. + skills: [...FIXABLE_SKILLS], + }); + if (!refresh) return { skills }; + + return { + skillsRefresh: { + before: refresh.perAgentBefore, + after: refresh.perAgentAfter, + skillsInstalled: refresh.skills, + }, + skills: (await checkSkills()) ?? undefined, + }; +} + export async function runDoctor(options: DoctorOptions): Promise { // Environment check first - loads project's .env/.env.local files // Must run before connectivity so the resolved base URL is available @@ -30,6 +78,16 @@ export async function runDoctor(options: DoctorOptions): Promise { checkLanguage(options.installDir), ]); + let skills = (await checkSkills()) ?? undefined; + + // `--fix`: refresh stale WorkOS skills BEFORE earlyIssues + AI analysis so + // every downstream consumer (issue detection, AI prompt context) sees the + // post-refresh skill state and doesn't reference a SKILLS_OUTDATED warning + // that was just resolved. + const refreshOutcome = await maybeRefreshSkills(options, skills); + const skillsRefresh = refreshOutcome.skillsRefresh; + skills = refreshOutcome.skills; + // Dashboard settings + auth patterns + AI analysis (parallel, all need sdk/framework results) // AI analysis also receives early issues as context to avoid duplication const earlyIssues = detectIssues({ @@ -42,6 +100,7 @@ export async function runDoctor(options: DoctorOptions): Promise { framework, environment, connectivity, + skills, }); const [dashboardResult, authPatterns, aiAnalysis] = await Promise.all([ @@ -86,9 +145,11 @@ export async function runDoctor(options: DoctorOptions): Promise { redirectUris, authPatterns, aiAnalysis, + skills, + skillsRefresh, }; - // Detect issues based on collected data + // Detect issues based on (post-refresh) data. const issues = detectIssues(partialReport); // Calculate summary diff --git a/src/doctor/issues.ts b/src/doctor/issues.ts index d1bf183a..d653251e 100644 --- a/src/doctor/issues.ts +++ b/src/doctor/issues.ts @@ -148,6 +148,22 @@ export function detectIssues(report: Omit): } } + // Skill freshness — warn when agent-installed skills trail the bundled version. + // Surfaced so users know their coding agent is using older WorkOS guidance. + if (report.skills?.bundledVersion) { + const stale = report.skills.agents.filter((a) => a.stale); + if (stale.length > 0) { + const agentList = stale.map((a) => `${a.agent} (${a.installedVersion ?? 'unknown'})`).join(', '); + issues.push({ + code: 'SKILLS_OUTDATED', + severity: 'warning', + message: `WorkOS skills outdated for ${agentList} — bundled: ${report.skills.bundledVersion}`, + remediation: 'Run: workos skills install', + details: { bundledVersion: report.skills.bundledVersion, stale: stale.map((a) => a.agent) }, + }); + } + } + return issues; } diff --git a/src/doctor/output.ts b/src/doctor/output.ts index 8ccacbb9..ff9ddac9 100644 --- a/src/doctor/output.ts +++ b/src/doctor/output.ts @@ -1,4 +1,6 @@ import Chalk from 'chalk'; +import { homedir } from 'os'; +import { createAgents } from '../commands/install-skill.js'; import type { DoctorReport, Issue } from './types.js'; import { renderSummaryBox, type SummaryBoxItem } from '../utils/summary-box.js'; import type { LockExpression } from '../utils/lock-art.js'; @@ -150,6 +152,22 @@ export function formatReport(report: DoctorReport, options?: FormatOptions): voi } } + // Skills refresh (--fix path) + if (report.skillsRefresh) { + const agents = createAgents(homedir()); + const displayName = (slug: string): string => agents[slug]?.displayName ?? slug; + const formatVersion = (v: string | null): string => v ?? '(none)'; + + console.log(''); + console.log('Skills'); + const slugs = Object.keys(report.skillsRefresh.before); + for (const slug of slugs) { + const before = formatVersion(report.skillsRefresh.before[slug] ?? null); + const after = formatVersion(report.skillsRefresh.after[slug] ?? null); + console.log(` ${Chalk.green('✓')} Updated WorkOS skills for ${displayName(slug)}: ${before} → ${after}`); + } + } + // Verbose mode additions if (options?.verbose) { console.log(''); diff --git a/src/doctor/types.ts b/src/doctor/types.ts index 63e81897..d87c0efd 100644 --- a/src/doctor/types.ts +++ b/src/doctor/types.ts @@ -108,6 +108,31 @@ export interface AuthPatternInfo { findings: AuthPatternFinding[]; } +export interface SkillAgentStatus { + agent: string; + installedVersion: string | null; + stale: boolean; +} + +export interface SkillsInfo { + bundledVersion: string | null; + agents: SkillAgentStatus[]; +} + +/** + * Result of `workos doctor --fix` refreshing WorkOS skills. Captured per agent + * so the human-mode renderer can show a before/after line and the JSON consumer + * can reason about which agents changed. + */ +export interface SkillsRefreshResult { + /** Marker version per agent.name BEFORE refresh (null = no marker). */ + before: Record; + /** Marker version per agent.name AFTER refresh. */ + after: Record; + /** Skills the refresh was scoped to (the FIXABLE_SKILLS allowlist). */ + skillsInstalled: string[]; +} + export interface DoctorReport { version: string; timestamp: string; @@ -127,6 +152,9 @@ export interface DoctorReport { credentialValidation?: CredentialValidation; authPatterns?: AuthPatternInfo; aiAnalysis?: AiAnalysis; + skills?: SkillsInfo; + /** Present only when `--fix` actually performed a refresh. */ + skillsRefresh?: SkillsRefreshResult; issues: Issue[]; summary: { errors: number; @@ -159,4 +187,6 @@ export interface DoctorOptions { skipAi?: boolean; json?: boolean; copy?: boolean; + /** When true, refresh stale WorkOS skills (constrained to workos/ + workos-widgets/). */ + fix?: boolean; } diff --git a/src/utils/help-json.ts b/src/utils/help-json.ts index 660d03e4..c8ef2796 100644 --- a/src/utils/help-json.ts +++ b/src/utils/help-json.ts @@ -225,6 +225,14 @@ const commands: CommandSchema[] = [ default: false, hidden: false, }, + { + name: 'fix', + type: 'boolean', + description: 'Auto-update stale WorkOS skills (writes to /skills/workos/ and workos-widgets/ only)', + required: false, + default: false, + hidden: false, + }, ], }, {