diff --git a/README.md b/README.md index b77249e..a03d6f3 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,8 @@ skillsync service install Repeat the install, existing-vault setup, and service steps on each device. +SkillSync stores local configuration in `~/.config/skillsync/config.json`. Read and parse errors, including broken symlinks in the config path, are reported instead of silently switching vaults. Repair the file or symlink before retrying; an absent config file still uses the default vault at `~/.skillsync/repo`. + ## Interactive UI Run SkillSync without a command: diff --git a/src/cli.js b/src/cli.js index 798df79..f2f68d4 100755 --- a/src/cli.js +++ b/src/cli.js @@ -215,8 +215,8 @@ function promptPageSize(itemCount, { min = 8, max = 28, reservedRows = 6 } = {}) return Math.max(1, Math.min(itemCount, max, availableRows)); } -async function configured({ initialize = true } = {}) { - const config = await loadConfig(); +async function configured({ initialize = true, config } = {}) { + config ??= await loadConfig(); if (!config.repoPath || !await exists(config.repoPath)) { throw new Error('SkillSync is not set up. Run: skillsync setup'); } @@ -1910,9 +1910,9 @@ async function daemon(rest) { async function runUi() { if (!process.stdin.isTTY) return listSkills(); - let config; + let config = await loadConfig(); try { - config = await configured(); + config = await configured({ config }); } catch { const shouldSetup = await confirm({ message: 'SkillSync is not set up. Run setup now?', default: true }); if (!shouldSetup) return; diff --git a/src/core/config.js b/src/core/config.js index 7f829ef..b12af76 100644 --- a/src/core/config.js +++ b/src/core/config.js @@ -1,8 +1,8 @@ -import { mkdir } from 'node:fs/promises'; +import { lstat, mkdir, realpath } from 'node:fs/promises'; import { homedir } from 'node:os'; import path from 'node:path'; -import { exists, expandHome, readJson, writeJson } from './fs.js'; +import { expandHome, readJson, writeJson } from './fs.js'; import { defaultDeviceId } from './device.js'; export function defaultConfigPath() { @@ -13,8 +13,33 @@ export function defaultRepoPath() { return path.join(homedir(), '.skillsync', 'repo'); } +async function configFileExists(configPath) { + const filePath = path.resolve(configPath); + let candidate = filePath; + while (true) { + const info = await lstat(candidate).catch((error) => { + if (error.code === 'ENOENT') return null; + throw error; + }); + if (info) { + if (candidate === filePath) return true; + // Missing descendants use defaults only when their existing ancestor resolves. + await realpath(candidate); + return false; + } + const parent = path.dirname(candidate); + if (parent === candidate) return false; + candidate = parent; + } +} + export async function loadConfig(configPath = defaultConfigPath()) { - const config = await readJson(configPath, null).catch(() => null); + let config = null; + try { + if (await configFileExists(configPath)) config = await readJson(configPath); + } catch (error) { + throw new Error(`Cannot read SkillSync config at ${configPath}: ${error.message}`, { cause: error }); + } return { version: 1, repo: config?.repo || null, @@ -29,7 +54,7 @@ export async function saveConfig(config, configPath = defaultConfigPath()) { } export async function isConfigured(configPath = defaultConfigPath()) { - if (!await exists(configPath)) return false; + if (!await configFileExists(configPath)) return false; const config = await loadConfig(configPath); return Boolean(config.repoPath); } diff --git a/src/core/instructions.js b/src/core/instructions.js index 60ff79d..acc032c 100644 --- a/src/core/instructions.js +++ b/src/core/instructions.js @@ -366,7 +366,7 @@ async function createOwnedInstructionsSymlink(source, destination) { } async function preserveUnmanagedDestinations(vaultPath, targetPaths) { - const unmanaged = []; + const unmanaged = new Map(); for (const targetPath of targetPaths) { const destination = resolvedDestination(targetPath); const info = await pathInfo(destination); @@ -374,13 +374,13 @@ async function preserveUnmanagedDestinations(vaultPath, targetPaths) { if (info.isDirectory()) { throw new Error(`Global instructions destination is a directory: ${destination}`); } - unmanaged.push({ destination }); + unmanaged.set(await canonicalDestination(destination), destination); } const backups = []; - for (const entry of unmanaged) { + for (const destination of unmanaged.values()) { backups.push({ - destination: entry.destination, - backup: await backupPath(entry.destination), + destination, + backup: await backupPath(destination), }); } return backups; @@ -429,12 +429,10 @@ export async function selectGlobalInstructionsProfile({ ? device.instructions.agents.paths : [DEFAULT_GLOBAL_INSTRUCTIONS_PATH], )]; - for (const targetPath of paths) { - if (resolvedDestination(targetPath) === path.resolve(selected.source)) { - throw new Error('Global instructions destination cannot be its vault profile file'); - } - } const destinations = new Set(await Promise.all(paths.map(canonicalDestination))); + if (destinations.has(await canonicalDestination(selected.source))) { + throw new Error('Global instructions destination cannot be its vault profile file'); + } const removed = []; for (const targetPath of device.instructions.agents?.paths || []) { if (destinations.has(await canonicalDestination(targetPath))) continue; diff --git a/test/cli.test.js b/test/cli.test.js index 644e028..766afa9 100644 --- a/test/cli.test.js +++ b/test/cli.test.js @@ -97,6 +97,33 @@ exit 1 }; } +for (const interactive of [false, true]) { + test(`${interactive ? 'interactive menu' : 'status'} stops on invalid config without touching the default vault`, async () => { + const home = await tempDir(); + const vault = path.join(home, '.skillsync', 'repo'); + await writeConfig(home, vault); + const configPath = path.join(home, '.config', 'skillsync', 'config.json'); + await writeFile(configPath, '{'); + const args = interactive + ? ['--import', 'data:text/javascript,process.stdin.isTTY=true', path.resolve('src/cli.js')] + : [path.resolve('src/cli.js'), 'status']; + + await assert.rejects( + () => execFileAsync(process.execPath, args, { env: cliEnv(home), timeout: 5000 }), + (error) => { + assert.equal(error.code, 1); + assert.ok(error.stderr.includes(configPath)); + assert.match(error.stderr, /Cannot read SkillSync config/); + assert.doesNotMatch(error.stdout, /Run setup now/); + return true; + }, + ); + assert.equal(await readFile(configPath, 'utf8'), '{'); + assert.equal(await exists(path.join(vault, 'registry.json')), false); + assert.equal(await exists(await gitPrivatePath(vault, 'skillsync')), false); + }); +} + test('setup detects the Grok Bot target when its agent-data directory exists', async () => { const home = await tempDir(); const vault = path.join(home, '.skillsync', 'repo'); diff --git a/test/config.test.js b/test/config.test.js new file mode 100644 index 0000000..6da8261 --- /dev/null +++ b/test/config.test.js @@ -0,0 +1,72 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdir, mkdtemp, symlink, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; + +import { defaultRepoPath, loadConfig } from '../src/core/config.js'; +import { defaultDeviceId } from '../src/core/device.js'; + +test('missing configuration keeps the default vault and device', async () => { + const root = await mkdtemp(path.join(tmpdir(), 'skillsync-config-test-')); + + assert.deepEqual(await loadConfig(path.join(root, 'missing', 'skillsync', 'config.json')), { + version: 1, + repo: null, + repoPath: defaultRepoPath(), + deviceId: defaultDeviceId(), + }); +}); + +test('configuration parse and read errors identify the file instead of using defaults', async () => { + const root = await mkdtemp(path.join(tmpdir(), 'skillsync-config-test-')); + const configPath = path.join(root, 'config.json'); + await writeFile(configPath, '{'); + + for (const filePath of [configPath, root]) { + await assert.rejects(() => loadConfig(filePath), (error) => { + assert.ok(error.message.includes(filePath)); + assert.match(error.message, /Cannot read SkillSync config/); + return true; + }); + } +}); + +test('configuration symlinks load their target and fail when it is missing', async () => { + const root = await mkdtemp(path.join(tmpdir(), 'skillsync-config-test-')); + const configPath = path.join(root, 'config.json'); + const target = path.join(root, 'dotfiles-config.json'); + await symlink(target, configPath); + + await assert.rejects(() => loadConfig(configPath), (error) => { + assert.ok(error.message.includes(configPath)); + assert.match(error.message, /Cannot read SkillSync config.*ENOENT/); + return true; + }); + + const config = { version: 1, repo: 'test/skills', repoPath: path.join(root, 'vault'), deviceId: 'laptop' }; + await writeFile(target, JSON.stringify(config)); + assert.deepEqual(await loadConfig(configPath), config); +}); + +test('missing configuration under a directory symlink requires a readable target', async () => { + const root = await mkdtemp(path.join(tmpdir(), 'skillsync-config-test-')); + const target = path.join(root, 'dotfiles'); + const alias = path.join(root, 'skillsync'); + const configPath = path.join(alias, 'nested', 'config.json'); + await symlink(target, alias, 'dir'); + + await assert.rejects(() => loadConfig(configPath), (error) => { + assert.ok(error.message.includes(configPath)); + assert.match(error.message, /Cannot read SkillSync config.*ENOENT/); + return true; + }); + + await mkdir(target); + assert.deepEqual(await loadConfig(configPath), { + version: 1, + repo: null, + repoPath: defaultRepoPath(), + deviceId: defaultDeviceId(), + }); +}); diff --git a/test/core.test.js b/test/core.test.js index c3832c7..40468e7 100644 --- a/test/core.test.js +++ b/test/core.test.js @@ -1197,6 +1197,36 @@ test('remote instruction assignment waits for the target to report profile suppo assert.equal((await loadDevice(vault, 'remote')).instructions.agents.profile, 'shared'); }); +test('instruction profile selection backs up an aliased local file only once', async () => { + const root = await tempDir(); + const vault = path.join(root, 'vault'); + const profile = globalInstructionsVaultPath(vault); + const directory = path.join(root, 'local'); + const alias = path.join(root, 'alias'); + const localFile = path.join(directory, 'AGENTS.md'); + const aliasedFile = path.join(alias, 'AGENTS.md'); + await mkdir(path.dirname(profile), { recursive: true }); + await mkdir(directory); + await symlink(directory, alias, 'dir'); + await writeFile(profile, '# Shared\n'); + await writeFile(localFile, '# Local\n'); + + const result = await selectGlobalInstructionsProfile({ + vaultPath: vault, + deviceId: 'macbook', + profile: 'shared', + targetPaths: [localFile, aliasedFile], + }); + + assert.equal(result.backups.length, 1); + assert.equal(await readFile(result.backups[0].backup, 'utf8'), '# Local\n'); + for (const destination of [localFile, aliasedFile]) { + assert.equal((await lstat(destination)).isSymbolicLink(), true); + assert.equal(await realpath(destination), await realpath(profile)); + assert.equal(await readFile(destination, 'utf8'), '# Shared\n'); + } +}); + test('instruction profile selection validates every destination before moving local files', async () => { const root = await tempDir(); const vault = path.join(root, 'vault'); @@ -1221,16 +1251,21 @@ test('instruction profile selection validates every destination before moving lo assert.equal(await readFile(localFile, 'utf8'), '# Local\n'); assert.equal(await readFile(profile, 'utf8'), '# Shared\n'); - await assert.rejects( - () => selectGlobalInstructionsProfile({ - vaultPath: vault, - deviceId: 'macbook', - profile: 'shared', - targetPaths: [profile], - }), - /cannot be its vault profile file/, - ); - assert.equal(await readFile(profile, 'utf8'), '# Shared\n'); + const profileAlias = path.join(root, 'profile-alias'); + await symlink(path.dirname(profile), profileAlias, 'dir'); + for (const destination of [profile, path.join(profileAlias, 'AGENTS.md')]) { + await assert.rejects( + () => selectGlobalInstructionsProfile({ + vaultPath: vault, + deviceId: 'macbook', + profile: 'shared', + targetPaths: [localFile, destination], + }), + /cannot be its vault profile file/, + ); + assert.equal(await readFile(profile, 'utf8'), '# Shared\n'); + assert.equal(await readFile(localFile, 'utf8'), '# Local\n'); + } }); test('skill matrix distinguishes assigned, detected, and absent skills', () => {