From e9ef4a849ba19c10d219dde9cfa764d0a13cc21d Mon Sep 17 00:00:00 2001 From: Akshar Patel Date: Wed, 9 Sep 2026 23:33:27 -0400 Subject: [PATCH] fix: stop ignored local files from churning vault history --- README.md | 2 ++ src/core/check.js | 7 +++-- src/core/device.js | 10 +++++-- src/core/git.js | 16 ++++++++++- src/core/registry.js | 13 +++++---- test/git.test.js | 66 ++++++++++++++++++++++++++++++++++++++++++-- 6 files changed, 100 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 634c797..ca76617 100644 --- a/README.md +++ b/README.md @@ -386,6 +386,8 @@ skillsync installed --device workstation The background service syncs when it starts and then checks every 120 seconds. It uses a macOS LaunchAgent or a Linux systemd user service. +Registry hashes exclude untracked files ignored by Git, so local generated files do not cause repeated sync commits across devices. Tracked files still count even when an ignore rule matches them. Copy-mode overwrite protection continues to check every deployed file, including ignored files. The first sync after upgrading may correct an existing hash once; upgrade every device to prevent older versions from restoring it. + Verify it on macOS: ```bash diff --git a/src/core/check.js b/src/core/check.js index 305594f..baa568f 100644 --- a/src/core/check.js +++ b/src/core/check.js @@ -2,8 +2,8 @@ import { createReadStream } from 'node:fs'; import { lstat, readFile, readdir } from 'node:fs/promises'; import path from 'node:path'; -import { assertSafePathSegment, hashDirectory } from './fs.js'; -import { git, isGitRepo } from './git.js'; +import { assertSafePathSegment } from './fs.js'; +import { git, hashSyncedDirectory, ignoredGitPaths, isGitRepo } from './git.js'; const GIT_ENTRY = '.git'; const LOCAL_STATE_ENTRY = '.skillsync-local'; @@ -238,6 +238,7 @@ export async function checkVault(vaultPath, { verifyRegistry = true } = {}) { } else { const actualNames = skillNames.sort(); const registeredNames = Object.keys(registry.skills).sort(); + const ignoredPaths = actualNames.length && !unsafeTree ? await ignoredGitPaths(root) : []; for (const name of actualNames) { const entry = registry.skills[name]; if (!entry || entry.path !== path.posix.join('skills', name)) { @@ -245,7 +246,7 @@ export async function checkVault(vaultPath, { verifyRegistry = true } = {}) { continue; } if (!unsafeTree) { - const hash = await hashDirectory(path.join(skillRoot, name)); + const hash = await hashSyncedDirectory(root, path.posix.join('skills', name), ignoredPaths); if (entry.hash !== hash) errors.push(`Registry hash is stale: ${name}`); } } diff --git a/src/core/device.js b/src/core/device.js index bc2767e..75625cb 100644 --- a/src/core/device.js +++ b/src/core/device.js @@ -1014,7 +1014,10 @@ export async function planLinks({ && !approvedReplacements.has(await canonicalTargetRoot(destination))) { throw new Error(`Refusing to overwrite unmanaged target path: ${destination}`); } - const sourceHash = registry.skills[skillName].hash; + // Copy protection covers every deployed file, including Git-ignored local files. + const sourceHash = group.mode === 'copy' || info.ownedCopy + ? await hashDirectory(source) + : registry.skills[skillName].hash; if (group.mode === 'copy') { if (info.ownedCopy) { const state = await copyState({ @@ -1177,14 +1180,15 @@ export async function managedProjections({ vaultPath, deviceId = defaultDeviceId if (!info.ownedCopy) { status = 'unmanaged'; } else { + const sourceHash = await hashDirectory(source); const state = await copyState({ destination, marker: info.copyMarker, - sourceHash: registryEntry.hash, + sourceHash, }); status = state.drifted ? 'drifted' - : state.localHash === registryEntry.hash + : state.localHash === sourceHash ? 'ok' : 'outdated'; } diff --git a/src/core/git.js b/src/core/git.js index a9eee5f..d9f51af 100644 --- a/src/core/git.js +++ b/src/core/git.js @@ -1,5 +1,5 @@ import { spawn } from 'node:child_process'; -import { assertSafePathSegment, exists } from './fs.js'; +import { assertSafePathSegment, exists, hashDirectory } from './fs.js'; import path from 'node:path'; export function run(command, args, options = {}) { @@ -76,6 +76,20 @@ export async function isGitRepo(repoPath) { return exists(path.join(repoPath, '.git')); } +export async function ignoredGitPaths(repoPath) { + if (!await isGitRepo(repoPath)) return []; + const { stdout } = await git(['ls-files', '--others', '--ignored', '--exclude-standard', '-z'], repoPath); + return stdout.split('\0').filter(Boolean); +} + +export async function hashSyncedDirectory(repoPath, relativePath, ignoredPaths) { + const directory = path.resolve(repoPath, relativePath); + const ignored = ignoredPaths ?? await ignoredGitPaths(repoPath); + return hashDirectory(directory, { + exclude: ignored.map((file) => path.relative(directory, path.resolve(repoPath, file))), + }); +} + export async function gitPrivatePath(repoPath, ...segments) { if (!await isGitRepo(repoPath)) return null; const relativePath = path.posix.join( diff --git a/src/core/registry.js b/src/core/registry.js index 40b5d7f..a4dbb34 100644 --- a/src/core/registry.js +++ b/src/core/registry.js @@ -2,6 +2,7 @@ import { mkdir, readdir, rm } from 'node:fs/promises'; import path from 'node:path'; import { checkSkillFolder } from './check.js'; +import { hashSyncedDirectory, ignoredGitPaths } from './git.js'; import { assertSafePathSegment, copyDir, @@ -127,8 +128,9 @@ export async function addSkillToVault({ vaultPath, sourcePath, name, overwrite = } if (comparison.status === 'identical') { const registry = await loadRegistry(vaultPath); - if (!registry.skills[comparison.name] || registry.skills[comparison.name].hash !== comparison.vaultHash) { - registry.skills[comparison.name] = await registryEntryForSkill(vaultPath, comparison.name); + const entry = await registryEntryForSkill(vaultPath, comparison.name); + if (registry.skills[comparison.name]?.hash !== entry.hash) { + registry.skills[comparison.name] = entry; await saveRegistry(vaultPath, registry); } return { name: comparison.name, path: comparison.path, status: 'identical' }; @@ -152,13 +154,13 @@ export async function ensureSkillInRegistry(vaultPath, skillName) { return registry.skills[skillName]; } -export async function registryEntryForSkill(vaultPath, skillName) { +export async function registryEntryForSkill(vaultPath, skillName, ignoredPaths) { assertSafePathSegment(skillName, 'Skill name'); const skillPath = path.join(vaultPath, 'skills', skillName); await validateSkillFolder(skillPath); return { path: path.posix.join('skills', skillName), - hash: await hashDirectory(skillPath), + hash: await hashSyncedDirectory(vaultPath, path.posix.join('skills', skillName), ignoredPaths), updated_at: new Date().toISOString(), }; } @@ -167,11 +169,12 @@ export async function buildRegistry(vaultPath) { const skillsDir = path.join(vaultPath, 'skills'); const entries = await readdir(skillsDir, { withFileTypes: true }); const registry = emptyRegistry(); + const ignoredPaths = await ignoredGitPaths(vaultPath); for (const entry of entries) { if (!entry.isDirectory()) continue; const skillName = assertSafePathSegment(entry.name, 'Skill name'); if (!await exists(path.join(skillsDir, skillName, 'SKILL.md'))) continue; - registry.skills[skillName] = await registryEntryForSkill(vaultPath, skillName); + registry.skills[skillName] = await registryEntryForSkill(vaultPath, skillName, ignoredPaths); } return registry; } diff --git a/test/git.test.js b/test/git.test.js index 113c539..97728ce 100644 --- a/test/git.test.js +++ b/test/git.test.js @@ -1,6 +1,6 @@ import test from 'node:test'; import assert from 'node:assert/strict'; -import { lstat, mkdir, mkdtemp, readFile, writeFile } from 'node:fs/promises'; +import { lstat, mkdir, mkdtemp, readFile, unlink, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import path from 'node:path'; @@ -10,6 +10,7 @@ import { autoImportNewLocalSkills, initializeLocalPathState, loadLocalDevice, + managedProjections, migrateLegacyLocalPathState, scanTargets, setGlobalInstructionsProfile, @@ -21,8 +22,9 @@ import { } from '../src/core/instructions.js'; import { exists } from '../src/core/fs.js'; import { git, gitPrivatePath, pushWithPullRebaseRetry, run } from '../src/core/git.js'; -import { ensureVault, loadRegistry, rebuildRegistry } from '../src/core/registry.js'; +import { addSkillToVault, ensureVault, loadRegistry, rebuildRegistry } from '../src/core/registry.js'; import { syncVault } from '../src/core/sync.js'; +import { checkVault } from '../src/core/check.js'; async function tempDir() { return mkdtemp(path.join(tmpdir(), 'skillsync-git-test-')); @@ -72,6 +74,66 @@ async function writeDesiredDevice(vaultPath, deviceId, installed = {}) { }, null, 2)}\n`); } +test('sync keeps registry hashes stable across clones with different ignored files', async () => { + const root = await tempDir(); + const vault = path.join(root, 'vault'); + const clone = path.join(root, 'clone'); + const skill = path.join(vault, 'skills', 'captions'); + await initializeVaultRepo(vault); + await mkdir(path.join(skill, 'fixtures'), { recursive: true }); + await writeFile(path.join(skill, 'SKILL.md'), '# Captions\n'); + await writeFile(path.join(skill, '.gitignore'), 'transcript.json\n'); + await writeFile(path.join(skill, 'fixtures', 'transcript.json'), '{"local":true}\n'); + await syncVault({ vaultPath: vault, pull: false, pushChanges: false }); + await git(['add', '-A'], vault); + await git(['commit', '-m', 'initial skill'], vault); + await git(['clone', vault, clone]); + + const before = await readFile(path.join(vault, 'registry.json'), 'utf8'); + await syncVault({ vaultPath: clone, pull: false, pushChanges: false }); + assert.equal(await readFile(path.join(clone, 'registry.json'), 'utf8'), before); + await writeFile(path.join(skill, 'fixtures', 'transcript.json'), '{"local":"changed"}\n'); + await syncVault({ vaultPath: vault, pull: false, pushChanges: false }); + assert.equal(await readFile(path.join(vault, 'registry.json'), 'utf8'), before); + await checkVault(vault); + await addSkillToVault({ vaultPath: vault, sourcePath: skill }); + assert.equal(await readFile(path.join(vault, 'registry.json'), 'utf8'), before); + assert.equal((await git(['status', '--porcelain'], vault)).stdout, ''); + + // Git still syncs explicitly tracked files even when an ignore rule matches. + await git(['add', '-f', 'skills/captions/fixtures/transcript.json'], vault); + await syncVault({ vaultPath: vault, pull: false, pushChanges: false }); + const trackedHash = (await loadRegistry(vault)).skills.captions.hash; + assert.notEqual(trackedHash, JSON.parse(before).skills.captions.hash); + await writeFile(path.join(skill, 'fixtures', 'transcript.json'), '{"tracked":"changed"}\n'); + await syncVault({ vaultPath: vault, pull: false, pushChanges: false }); + assert.notEqual((await loadRegistry(vault)).skills.captions.hash, trackedHash); + await unlink(path.join(skill, 'fixtures', 'transcript.json')); + await syncVault({ vaultPath: vault, pull: false, pushChanges: false }); + assert.equal((await loadRegistry(vault)).skills.captions.hash, JSON.parse(before).skills.captions.hash); +}); + +test('copy projections retain drift protection when the vault contains ignored files', async () => { + const root = await tempDir(); + const vault = path.join(root, 'vault'); + const target = path.join(root, 'target'); + const skill = path.join(vault, 'skills', 'captions'); + const deviceId = 'test'; + await initializeVaultRepo(vault); + await mkdir(skill, { recursive: true }); + await writeFile(path.join(skill, 'SKILL.md'), '# Captions\n'); + await writeFile(path.join(skill, '.gitignore'), 'transcript.json\n'); + await writeFile(path.join(skill, 'transcript.json'), '{}\n'); + await rebuildRegistry(vault); + await addTarget({ vaultPath: vault, deviceId, name: 'codex', targetPath: target, mode: 'copy' }); + await writeDesiredDevice(vault, deviceId, { captions: ['codex'] }); + await applyLinks({ vaultPath: vault, deviceId }); + assert.deepEqual((await applyLinks({ vaultPath: vault, deviceId })).operations, []); + assert.equal((await managedProjections({ vaultPath: vault, deviceId }))[0].status, 'ok'); + await writeFile(path.join(target, 'captions', 'transcript.json'), '{"edited":true}\n'); + await assert.rejects(applyLinks({ vaultPath: vault, deviceId }), /Managed copy has local changes/); +}); + test('run terminates commands that exceed their timeout', async () => { await assert.rejects( run(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { timeoutMs: 50 }),