fix: stop config fallback and duplicate instruction backups

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:

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;

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);
 }

src/core/instructions.js

@@ -366,21 +366,21 @@ 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);
     if (!info || await ownedProfileAt(destination, vaultPath)) continue;
     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;

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');

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(),
+  });
+});

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', () => {