From 2c11aa6cffb9b7ab5f96f0869429c80f11732eaa Mon Sep 17 00:00:00 2001 From: Ronald Roy Date: Mon, 2 Feb 2026 20:51:27 -0800 Subject: [PATCH] fix: Always overwrite server-owned commands and skills during vault setup Previously, vault setup only checked for file existence and skipped copying if the file/directory was already present. This meant updates to bundled commands and skills wouldn't propagate to existing vaults. Now commands and skills are always copied from the server's templates, treating them as server-owned rather than user-customizable. The setup reports "Installed N" for new files and "Updated N" for overwrites. Co-Authored-By: Claude Opus 4.5 --- backend/src/__tests__/vault-setup.test.ts | 41 +++++++------- backend/src/vault-setup.ts | 66 ++++++++++++++--------- 2 files changed, 62 insertions(+), 45 deletions(-) diff --git a/backend/src/__tests__/vault-setup.test.ts b/backend/src/__tests__/vault-setup.test.ts index 2357f862..1c4564db 100644 --- a/backend/src/__tests__/vault-setup.test.ts +++ b/backend/src/__tests__/vault-setup.test.ts @@ -186,35 +186,36 @@ describe("installCommands", () => { expect(files).toContain("weekly-synthesis.md"); }); - test("skips existing files", async () => { + test("updates existing files on second install", async () => { // First install const result1 = await installCommands(vaultPath); expect(result1.success).toBe(true); expect(result1.installed.length).toBeGreaterThan(0); - // Second install should skip all + // Second install should update all (server-owned files) const result2 = await installCommands(vaultPath); expect(result2.success).toBe(true); - expect(result2.installed.length).toBe(0); - expect(result2.message).toContain("already existed"); + expect(result2.installed.length).toBe(0); // None are "new" + expect(result2.message).toContain("Updated"); }); - test("does not overwrite existing files with different content", async () => { + test("overwrites existing files with server version", async () => { // Create commands directory with a custom file const commandsDir = join(vaultPath, COMMANDS_DEST_PATH); await mkdir(commandsDir, { recursive: true }); - const customContent = "# Custom daily-debrief content\n\nDo not overwrite me."; + const customContent = "# Custom daily-debrief content\n\nThis will be overwritten."; await writeFile(join(commandsDir, "daily-debrief.md"), customContent); // Run install await installCommands(vaultPath); - // Verify custom file was preserved + // Verify custom file was replaced with server version const content = await readFile(join(commandsDir, "daily-debrief.md"), "utf-8"); - expect(content).toBe(customContent); + expect(content).not.toBe(customContent); + expect(content).toContain("Quick focused conversation"); // Should have the real content }); - test("handles mixed existing and new files", async () => { + test("reports mixed installed and updated files", async () => { // Create commands directory with one existing file const commandsDir = join(vaultPath, COMMANDS_DEST_PATH); await mkdir(commandsDir, { recursive: true }); @@ -223,10 +224,11 @@ describe("installCommands", () => { const result = await installCommands(vaultPath); expect(result.success).toBe(true); - // Should install all except daily-debrief.md + // daily-debrief.md was updated, others were installed expect(result.installed).not.toContain("daily-debrief.md"); expect(result.installed.length).toBeGreaterThan(0); - expect(result.message).toContain("already existed"); + expect(result.message).toContain("Installed"); + expect(result.message).toContain("Updated"); }); test("returns list of installed commands", async () => { @@ -318,32 +320,33 @@ describe("installSkills", () => { expect(stats.mode & 0o100).toBeTruthy(); }); - test("skips existing skill directories", async () => { + test("updates existing skills on second install", async () => { // First install const result1 = await installSkills(vaultPath); expect(result1.success).toBe(true); expect(result1.installed.length).toBeGreaterThan(0); - // Second install should skip all + // Second install should update all (server-owned) const result2 = await installSkills(vaultPath); expect(result2.success).toBe(true); - expect(result2.installed.length).toBe(0); - expect(result2.message).toContain("already existed"); + expect(result2.installed.length).toBe(0); // None are "new" + expect(result2.message).toContain("Updated"); }); - test("does not overwrite existing skill", async () => { + test("overwrites existing skill with server version", async () => { // Create skills directory with a custom file in the skill const skillDir = join(vaultPath, SKILLS_DEST_PATH, "vault-task-management"); await mkdir(skillDir, { recursive: true }); - const customContent = "# Custom SKILL.md content\n\nDo not overwrite me."; + const customContent = "# Custom SKILL.md content\n\nThis will be overwritten."; await writeFile(join(skillDir, "SKILL.md"), customContent); // Run install await installSkills(vaultPath); - // Verify custom file was preserved + // Verify custom file was replaced with server version const content = await readFile(join(skillDir, "SKILL.md"), "utf-8"); - expect(content).toBe(customContent); + expect(content).not.toBe(customContent); + expect(content).toContain("vault"); // Should have the real content }); test("returns list of installed skills", async () => { diff --git a/backend/src/vault-setup.ts b/backend/src/vault-setup.ts index 54d41431..fe755197 100644 --- a/backend/src/vault-setup.ts +++ b/backend/src/vault-setup.ts @@ -8,7 +8,7 @@ * 4. Write setup completion marker */ -import { copyFile, mkdir, readdir, readFile, writeFile, stat, chmod } from "node:fs/promises"; +import { copyFile, mkdir, readdir, readFile, writeFile, stat, chmod, rm } from "node:fs/promises"; import { join, dirname } from "node:path"; import { fileURLToPath } from "node:url"; import { createLogger } from "./logger"; @@ -135,7 +135,7 @@ function getCommandTemplatesDir(): string { /** * Installs command templates to the vault's .claude/commands/ directory. - * Skips files that already exist (does not overwrite). + * Always overwrites existing files (server-owned, not user-customizable). * * @param vaultPath - Absolute path to the vault root * @returns Setup step result with list of installed commands @@ -146,7 +146,7 @@ export async function installCommands( log.info(`Installing commands to ${vaultPath}`); const installed: string[] = []; - const skipped: string[] = []; + const updated: string[] = []; const errors: string[] = []; const templatesDir = getCommandTemplatesDir(); @@ -194,23 +194,24 @@ export async function installCommands( }; } - // Copy each template + // Copy each template (always overwrite - server-owned files) for (const template of templates) { const srcPath = join(templatesDir, template); const destPath = join(destDir, template); - // Check if file already exists - if (await fileExists(destPath)) { - log.debug(`Skipping existing: ${template}`); - skipped.push(template); - continue; - } + // Track if this is an update vs new install + const existed = await fileExists(destPath); // Copy template try { await copyFile(srcPath, destPath); - log.info(`Installed: ${template}`); - installed.push(template); + if (existed) { + log.debug(`Updated: ${template}`); + updated.push(template); + } else { + log.info(`Installed: ${template}`); + installed.push(template); + } } catch (error) { const message = error instanceof Error ? error.message : String(error); log.warn(`Failed to install ${template}: ${message}`); @@ -223,8 +224,8 @@ export async function installCommands( if (installed.length > 0) { parts.push(`Installed ${installed.length} command(s)`); } - if (skipped.length > 0) { - parts.push(`${skipped.length} already existed`); + if (updated.length > 0) { + parts.push(`Updated ${updated.length} command(s)`); } const resultMessage = parts.join(", ") || "No commands to install"; @@ -299,7 +300,7 @@ async function copyDirectoryRecursive( /** * Installs skill directories to the vault's .claude/skills/ directory. * Each skill is a directory containing SKILL.md and supporting files. - * Skips skills that already exist (does not overwrite). + * Always overwrites existing skills (server-owned, not user-customizable). * * @param vaultPath - Absolute path to the vault root * @returns Setup step result with list of installed skills @@ -310,7 +311,7 @@ export async function installSkills( log.info(`Installing skills to ${vaultPath}`); const installed: string[] = []; - const skipped: string[] = []; + const updated: string[] = []; const errors: string[] = []; const templatesDir = getSkillTemplatesDir(); @@ -358,23 +359,36 @@ export async function installSkills( }; } - // Copy each skill directory + // Copy each skill directory (always overwrite - server-owned) for (const skillName of skillDirs) { const srcPath = join(templatesDir, skillName); const destPath = join(destDir, skillName); - // Check if skill already exists - if (await directoryExists(destPath)) { - log.debug(`Skipping existing skill: ${skillName}`); - skipped.push(skillName); - continue; + // Track if this is an update vs new install + const existed = await directoryExists(destPath); + + // Remove existing skill directory to avoid orphan files + if (existed) { + try { + await rm(destPath, { recursive: true }); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + log.warn(`Failed to remove existing skill ${skillName}: ${message}`); + errors.push(`${skillName}: failed to remove existing: ${message}`); + continue; + } } // Copy skill directory recursively try { await copyDirectoryRecursive(srcPath, destPath, vaultPath); - log.info(`Installed skill: ${skillName}`); - installed.push(skillName); + if (existed) { + log.debug(`Updated skill: ${skillName}`); + updated.push(skillName); + } else { + log.info(`Installed skill: ${skillName}`); + installed.push(skillName); + } } catch (error) { const message = error instanceof Error ? error.message : String(error); log.warn(`Failed to install skill ${skillName}: ${message}`); @@ -387,8 +401,8 @@ export async function installSkills( if (installed.length > 0) { parts.push(`Installed ${installed.length} skill(s)`); } - if (skipped.length > 0) { - parts.push(`${skipped.length} already existed`); + if (updated.length > 0) { + parts.push(`Updated ${updated.length} skill(s)`); } const resultMessage = parts.join(", ") || "No skills to install";