diff --git a/src/core/hooks/HookRegistry.ts b/src/core/hooks/HookRegistry.ts index 34afdf0b..e2eff7f7 100644 --- a/src/core/hooks/HookRegistry.ts +++ b/src/core/hooks/HookRegistry.ts @@ -26,9 +26,9 @@ function isExpectedHookError(error: unknown): boolean { * Windows PowerShell scripts) and expected filesystem errors gracefully. */ export class HookRegistry { - /** Checks if a hooks directory is a global hooks directory (contains Dirac/Hooks path). */ + /** Checks if a hooks directory is a global hooks directory (legacy Dirac/Hooks or new .dirac/Hooks). */ static isGlobalHooksDir(dir: string): boolean { - return /[/\\][Dd]irac[/\\][Hh]ooks/i.test(dir) + return /[/\\](?:\.dirac|[Dd]irac)[/\\][Hh]ooks/i.test(dir) } /** Finds all hook scripts for the given hook name across all hooks directories. */ diff --git a/src/core/hooks/__tests__/HookRegistry.test.ts b/src/core/hooks/__tests__/HookRegistry.test.ts new file mode 100644 index 00000000..c2fae335 --- /dev/null +++ b/src/core/hooks/__tests__/HookRegistry.test.ts @@ -0,0 +1,23 @@ +import { expect } from "chai" +import { describe, it } from "mocha" +import { HookRegistry } from "../HookRegistry" + +describe("HookRegistry.isGlobalHooksDir", () => { + it("recognizes legacy ~/Documents/Dirac/Hooks path", () => { + expect(HookRegistry.isGlobalHooksDir("/Users/user/Documents/Dirac/Hooks")).to.be.true + expect(HookRegistry.isGlobalHooksDir("/Users/user/Documents/dirac/hooks")).to.be.true + expect(HookRegistry.isGlobalHooksDir("C:\\Users\\user\\Documents\\Dirac\\Hooks")).to.be.true + }) + + // Review fix #2: relocated global hooks at ~/.dirac/Hooks must be classified as global. + it("recognizes new ~/.dirac/Hooks path", () => { + expect(HookRegistry.isGlobalHooksDir("/Users/user/.dirac/Hooks")).to.be.true + expect(HookRegistry.isGlobalHooksDir("/Users/user/.dirac/hooks")).to.be.true + expect(HookRegistry.isGlobalHooksDir("C:\\Users\\user\\.dirac\\Hooks")).to.be.true + }) + + it("rejects workspace hooks dirs", () => { + expect(HookRegistry.isGlobalHooksDir("/project/.diracrules/hooks")).to.be.false + expect(HookRegistry.isGlobalHooksDir("/project/hooks")).to.be.false + }) +}) diff --git a/src/core/storage/__tests__/directoryEnsurers.test.ts b/src/core/storage/__tests__/directoryEnsurers.test.ts new file mode 100644 index 00000000..2d31c640 --- /dev/null +++ b/src/core/storage/__tests__/directoryEnsurers.test.ts @@ -0,0 +1,190 @@ +/** + * Tests for ensureRules/Workflows/HooksDirectoryExists — FU-9. + * + * Verifies the relocation from the TCC-protected ~/Documents/Dirac/{Rules,Workflows,Hooks} + * to ~/.dirac/{Rules,Workflows,Hooks}, plus the best-effort migration that copies existing + * files without clobbering and swallows EPERM on the legacy dir. + */ +import { expect } from "chai" +import fs from "fs/promises" +import { afterEach, beforeEach, describe, it } from "mocha" +import os from "os" +import path from "path" +import * as sinon from "sinon" +import { Logger } from "@/shared/services/Logger" +import { ensureHooksDirectoryExists, ensureRulesDirectoryExists, ensureWorkflowsDirectoryExists } from "../directoryEnsurers" +import * as pathsModule from "../paths" + +const SUBDIRS = ["Rules", "Workflows", "Hooks"] as const +type Subdir = (typeof SUBDIRS)[number] + +const ENSURER: Record Promise> = { + Rules: ensureRulesDirectoryExists, + Workflows: ensureWorkflowsDirectoryExists, + Hooks: ensureHooksDirectoryExists, +} + +describe("directoryEnsurers — FU-9 (TCC-protected path relocation)", () => { + let sandbox: sinon.SinonSandbox + let fakeHome: string + let fakeDocuments: string + let realHome: string + + beforeEach(async () => { + sandbox = sinon.createSandbox() + realHome = os.homedir() + const tmpBase = path.join(os.tmpdir(), `dirac-ensurers-${Date.now()}-${Math.random().toString(36).slice(2)}`) + fakeHome = path.join(tmpBase, "home") + fakeDocuments = path.join(tmpBase, "Documents") + await fs.mkdir(fakeHome, { recursive: true }) + await fs.mkdir(fakeDocuments, { recursive: true }) + // Stub getDiracHomePath → ~/.dirac under fakeHome; getDocumentsPath → fakeDocuments + sandbox.stub(pathsModule, "getDiracHomePath").returns(path.join(fakeHome, ".dirac")) + sandbox.stub(pathsModule, "getDocumentsPath").resolves(fakeDocuments) + }) + + afterEach(async () => { + sandbox.restore() + // sanity: real homedir was never touched + if (realHome !== os.homedir()) throw new Error("homedir was mutated") + }) + + for (const subdir of SUBDIRS) { + describe(`ensure${subdir}DirectoryExists`, () => { + it(`returns ~/.dirac/${subdir} (not ~/Documents/Dirac/${subdir})`, async () => { + const result = await ENSURER[subdir]() + expect(result).to.equal(path.join(fakeHome, ".dirac", subdir)) + }) + + it(`creates ~/.dirac/${subdir} if it does not exist`, async () => { + const result = await ENSURER[subdir]() + const stat = await fs.stat(result) + expect(stat.isDirectory()).to.be.true + }) + + it("migrates files from legacy ~/Documents/Dirac/ to the new location", async () => { + const legacyDir = path.join(fakeDocuments, "Dirac", subdir) + await fs.mkdir(legacyDir, { recursive: true }) + await fs.writeFile(path.join(legacyDir, "rule1.md"), "content-1") + await fs.writeFile(path.join(legacyDir, "rule2.md"), "content-2") + + const result = await ENSURER[subdir]() + + const migrated = await fs.readdir(result) + expect(migrated.sort()).to.deep.equal(["rule1.md", "rule2.md"]) + expect(await fs.readFile(path.join(result, "rule1.md"), "utf8")).to.equal("content-1") + }) + + it("does not clobber existing files in the new location (idempotent)", async () => { + const legacyDir = path.join(fakeDocuments, "Dirac", subdir) + await fs.mkdir(legacyDir, { recursive: true }) + await fs.writeFile(path.join(legacyDir, "shared.md"), "legacy-content") + + const newDir = path.join(fakeHome, ".dirac", subdir) + await fs.mkdir(newDir, { recursive: true }) + await fs.writeFile(path.join(newDir, "shared.md"), "new-content") + + await ENSURER[subdir]() + + expect(await fs.readFile(path.join(newDir, "shared.md"), "utf8")).to.equal("new-content") + }) + + it("swallows EPERM on legacy dir (TCC-protected ~/Documents)", async () => { + const legacyDir = path.join(fakeDocuments, "Dirac", subdir) + await fs.mkdir(legacyDir, { recursive: true }) + await fs.writeFile(path.join(legacyDir, "rule.md"), "content") + // Stub readdir to reject EPERM only for the legacy dir (TCC denial) + const realReaddir = fs.readdir.bind(fs) + sandbox.stub(fs, "readdir").callsFake(((p: string) => { + if (p === legacyDir) return Promise.reject(Object.assign(new Error("EPERM"), { code: "EPERM" })) + return realReaddir(p) + }) as typeof fs.readdir) + + const result = await ENSURER[subdir]() + + // Still returns the new dir, just without migration + expect(result).to.equal(path.join(fakeHome, ".dirac", subdir)) + const migrated = await fs.readdir(result) + expect(migrated).to.deep.equal([]) + }) + + it("does nothing when legacy dir does not exist", async () => { + const result = await ENSURER[subdir]() + const migrated = await fs.readdir(result) + expect(migrated).to.deep.equal([]) + }) + + // Review fix #1: nested dirs must be migrated recursively (not skipped as EISDIR). + it("migrates nested subdirectories from legacy dir (recursive)", async () => { + const legacyDir = path.join(fakeDocuments, "Dirac", subdir) + await fs.mkdir(path.join(legacyDir, "nested"), { recursive: true }) + await fs.writeFile(path.join(legacyDir, "top.md"), "top") + await fs.writeFile(path.join(legacyDir, "nested", "deep.md"), "deep") + + const result = await ENSURER[subdir]() + + expect(await fs.readFile(path.join(result, "top.md"), "utf8")).to.equal("top") + expect(await fs.readFile(path.join(result, "nested", "deep.md"), "utf8")).to.equal("deep") + }) + + // One bad sibling must not block the rest (data safety — per-file isolation). + it("continues migrating siblings when one copyFile fails", async () => { + const legacyDir = path.join(fakeDocuments, "Dirac", subdir) + await fs.mkdir(legacyDir, { recursive: true }) + await fs.writeFile(path.join(legacyDir, "good.md"), "good") + await fs.writeFile(path.join(legacyDir, "bad.md"), "bad") + const realCopyFile = fs.copyFile.bind(fs) + sandbox.stub(fs, "copyFile").callsFake(((src: string, dest: string, mode?: number) => { + if (src.endsWith("bad.md")) return Promise.reject(Object.assign(new Error("EIO"), { code: "EIO" })) + return realCopyFile(src, dest, mode) + }) as typeof fs.copyFile) + + const result = await ENSURER[subdir]() + + expect(await fs.readFile(path.join(result, "good.md"), "utf8")).to.equal("good") + await fs.stat(path.join(result, "bad.md")).then( + () => expect.fail("bad.md should not have been migrated"), + () => undefined, + ) + }) + + // Review fix #3: EACCES is an expected legacy-path denial, same class as EPERM. + it("swallows EACCES on legacy dir", async () => { + const legacyDir = path.join(fakeDocuments, "Dirac", subdir) + await fs.mkdir(legacyDir, { recursive: true }) + await fs.writeFile(path.join(legacyDir, "rule.md"), "content") + const realReaddir = fs.readdir.bind(fs) + sandbox.stub(fs, "readdir").callsFake(((p: string) => { + if (p === legacyDir) return Promise.reject(Object.assign(new Error("EACCES"), { code: "EACCES" })) + return realReaddir(p) + }) as typeof fs.readdir) + + const result = await ENSURER[subdir]() + + expect(result).to.equal(path.join(fakeHome, ".dirac", subdir)) + expect(await fs.readdir(result)).to.deep.equal([]) + }) + + // Review fix #3: unexpected errors must be logged and rethrown, not swallowed. + it("rejects on unexpected readdir errors (not ENOENT/EPERM/EACCES)", async () => { + const legacyDir = path.join(fakeDocuments, "Dirac", subdir) + await fs.mkdir(legacyDir, { recursive: true }) + const warnStub = sandbox.stub(Logger, "warn") + const realReaddir = fs.readdir.bind(fs) + sandbox.stub(fs, "readdir").callsFake(((p: string) => { + if (p === legacyDir) return Promise.reject(Object.assign(new Error("ENOSYS"), { code: "ENOSYS" })) + return realReaddir(p) + }) as typeof fs.readdir) + + await ENSURER[subdir]().then( + () => expect.fail("unexpected migration error should reject"), + (error) => { + expect((error as NodeJS.ErrnoException).code).to.equal("ENOSYS") + }, + ) + expect(warnStub.called).to.be.true + expect(String(warnStub.firstCall.args[0])).to.match(/migration: failed/) + }) + }) + } +}) diff --git a/src/core/storage/directoryEnsurers.ts b/src/core/storage/directoryEnsurers.ts index cd12f1ef..84509553 100644 --- a/src/core/storage/directoryEnsurers.ts +++ b/src/core/storage/directoryEnsurers.ts @@ -1,50 +1,73 @@ import fs from "fs/promises" -import os from "os" import * as path from "path" -import { getDocumentsPath } from "./paths" +import { Logger } from "@/shared/services/Logger" import { getGlobalStorageDir } from "./globalStorageDir" +import { getDiracHomePath, getDocumentsPath } from "./paths" // Ensures the per-task directory exists and returns its path. export async function ensureTaskDirectoryExists(taskId: string): Promise { return getGlobalStorageDir("tasks", taskId) } -// Ensures the global Rules directory exists, falling back to ~/Documents/Dirac/Rules. -export async function ensureRulesDirectoryExists(): Promise { - const userDocumentsPath = await getDocumentsPath() - const diracRulesDir = path.join(userDocumentsPath, "Dirac", "Rules") - try { - await fs.mkdir(diracRulesDir, { recursive: true }) - } catch (_error) { - return path.join(os.homedir(), "Documents", "Dirac", "Rules") - } - return diracRulesDir +// Expected legacy-path errors: ENOENT (dir missing), EPERM/EACCES (TCC-protected ~/Documents). +function isExpectedMigrationError(error: unknown): boolean { + const code = (error as NodeJS.ErrnoException).code + return code === "ENOENT" || code === "EPERM" || code === "EACCES" } -// Ensures the global Workflows directory exists, falling back to ~/Documents/Dirac/Workflows. -export async function ensureWorkflowsDirectoryExists(): Promise { - const userDocumentsPath = await getDocumentsPath() - const diracWorkflowsDir = path.join(userDocumentsPath, "Dirac", "Workflows") +// Copies a single file, skipping if dest exists. Per-file isolation — one failure doesn't block others. +async function migrateFile(src: string, dest: string): Promise { try { - await fs.mkdir(diracWorkflowsDir, { recursive: true }) - } catch (_error) { - return path.join(os.homedir(), "Documents", "Dirac", "Workflows") + await fs.copyFile(src, dest, fs.constants.COPYFILE_EXCL) + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "EEXIST") Logger.warn(`migration: skipping ${src}: ${error}`) } - return diracWorkflowsDir } -// Ensures the global Hooks directory exists, falling back to ~/Documents/Dirac/Hooks. -export async function ensureHooksDirectoryExists(): Promise { - const userDocumentsPath = await getDocumentsPath() - const diracHooksDir = path.join(userDocumentsPath, "Dirac", "Hooks") +// Recursively migrates src → dest. Each file has its own try/catch — one failure doesn't abort the rest. +async function migrateDir(src: string, dest: string): Promise { + await fs.mkdir(dest, { recursive: true }) + const entries = await fs.readdir(src, { withFileTypes: true }) + await Promise.all( + entries.map(async (entry) => { + const s = path.join(src, entry.name) + const d = path.join(dest, entry.name) + if (entry.isDirectory()) await migrateDir(s, d) + else await migrateFile(s, d) + }), + ) +} + +// Migrates legacy ~/Documents/Dirac/ → ~/.dirac/ recursively, skipping existing files. +// Swallows only ENOENT/EPERM/EACCES (TCC-protected ~/Documents); logs and rethrows unexpected errors. +async function migrateFromDocumentsDir(subdir: string, destDir: string): Promise { + const legacyDir = path.join(await getDocumentsPath(), "Dirac", subdir) try { - await fs.mkdir(diracHooksDir, { recursive: true }) - } catch (_error) { - return path.join(os.homedir(), "Documents", "Dirac", "Hooks") + await migrateDir(legacyDir, destDir) + } catch (error) { + if (isExpectedMigrationError(error)) return + Logger.warn(`migration: failed ${legacyDir}: ${error}`) + throw error } - return diracHooksDir } +// Ensures a ~/.dirac/ directory exists and migrates from the legacy ~/Documents/Dirac/. +async function ensureDiracSubdir(subdir: string): Promise { + const dir = path.join(getDiracHomePath(), subdir) + await fs.mkdir(dir, { recursive: true }) + await migrateFromDocumentsDir(subdir, dir) + return dir +} + +// Ensures the global Rules directory exists at ~/.dirac/Rules (non-TCC-protected). +export const ensureRulesDirectoryExists = (): Promise => ensureDiracSubdir("Rules") + +// Ensures the global Workflows directory exists at ~/.dirac/Workflows (non-TCC-protected). +export const ensureWorkflowsDirectoryExists = (): Promise => ensureDiracSubdir("Workflows") + +// Ensures the global Hooks directory exists at ~/.dirac/Hooks (non-TCC-protected). +export const ensureHooksDirectoryExists = (): Promise => ensureDiracSubdir("Hooks") + // Ensures the global settings directory exists and returns its path. export async function ensureSettingsDirectoryExists(): Promise { return getGlobalStorageDir("settings")