diff --git a/skill/scripts/hook-before-edit.mjs b/skill/scripts/hook-before-edit.mjs index c9d31b5d8..e2d897b11 100644 --- a/skill/scripts/hook-before-edit.mjs +++ b/skill/scripts/hook-before-edit.mjs @@ -27,6 +27,7 @@ import { readCache, readConfig, renderTemplate, + resolveCacheCwd, resolveProjectCwd, truthy, writeAuditLog, @@ -379,9 +380,12 @@ async function main() { return allow({ skipped: 'stdin-empty' }); } - const cwd = resolveProjectCwd(event); + const sessionCwd = resolveProjectCwd(event); const started = Date.now(); - const filePath = proposedFilePath(event, cwd); + const filePath = proposedFilePath(event, sessionCwd); + // Re-key config/cache to the edited file's project root when the session + // was launched from a non-project umbrella directory (issue #305). + const cwd = resolveCacheCwd(filePath, sessionCwd); const audit = { harness: 'cursor', cwd, diff --git a/skill/scripts/hook-lib.mjs b/skill/scripts/hook-lib.mjs index 32232c88b..bcbf617c7 100644 --- a/skill/scripts/hook-lib.mjs +++ b/skill/scripts/hook-lib.mjs @@ -10,7 +10,7 @@ * truthy(value) * readConfig(cwd) / DEFAULT_CONFIG / getConfigPath(cwd) / getLocalConfigPath(cwd) * normalizeIgnoreValue(value) - * readCache(cwd) / persistCache(cwd, cache) + * readCache(cwd) / persistCache(cwd, cache) / resolveCacheCwd(primaryFile, sessionCwd) * bumpEditCount(cache, sessionId, filePath) -> number * suppressionNotice(filePath) * filterFindings(findings, content, ext, config) @@ -35,6 +35,7 @@ */ import fs from 'node:fs'; +import os from 'node:os'; import path from 'node:path'; import { pathToFileURL, fileURLToPath } from 'node:url'; @@ -134,6 +135,39 @@ export function resolveProjectCwd(event, fallback = process.cwd()) { || fallback; } +function looksLikeProjectRoot(dir) { + return ['.git', 'package.json', '.impeccable'].some((marker) => { + try { return fs.existsSync(path.join(dir, marker)); } catch { return false; } + }); +} + +// Where `.impeccable/` (cache + config) lives for this event. Normally the +// session cwd, untouched. But when the agent was launched from an umbrella +// directory that is not itself a project (no .git, package.json, or +// .impeccable), key to the edited file's nearest project root instead, so a +// multi-project launch dir doesn't accumulate a shared cross-project cache +// (issue #305). Climbing stops at the home dir, falling back to the session +// cwd when no marker is found. +export function resolveCacheCwd(primaryFile, sessionCwd) { + const base = path.resolve(sessionCwd || process.cwd()); + if (!primaryFile || typeof primaryFile !== 'string' || hasPathTraversal(primaryFile)) return base; + if (looksLikeProjectRoot(base)) return base; + let dir; + try { + dir = path.dirname(path.resolve(primaryFile)); + } catch { + return base; + } + const home = path.resolve(os.homedir()); + while (true) { + if (dir === home) return base; + if (looksLikeProjectRoot(dir)) return dir; + const parent = path.dirname(dir); + if (parent === dir) return base; + dir = parent; + } +} + export function readConfig(cwd) { const config = cloneDefaultConfig(); // Hook runtime settings live under `hook`; detector filters live under @@ -1402,9 +1436,10 @@ export async function runHook({ stdinJson, env = {}, cwd = process.cwd(), now = event = normalizeHookEvent(event, cwd, harness); audit.harness = harness; - const projectCwd = event.cwd || cwd; + const sessionCwd = event.cwd || cwd; + const primaryFiles = normalizeScanTargets(resolveTargetFiles(event, sessionCwd), sessionCwd); + const projectCwd = resolveCacheCwd(primaryFiles[0], sessionCwd); audit.cwd = projectCwd; - const primaryFiles = normalizeScanTargets(resolveTargetFiles(event, projectCwd), projectCwd); const primaryFileSet = new Set(primaryFiles); const targetFiles = expandScanTargets(primaryFiles, projectCwd); audit.session = event.session_id || null; @@ -1423,7 +1458,7 @@ export async function runHook({ stdinJson, env = {}, cwd = process.cwd(), now = const sessionId = event.session_id || 'unknown'; const det = detector || await loadDetector(); if (!det || typeof det.detectText !== 'function') { - persistCache(projectCwd, cache); + // Cache is not mutated yet at this point; nothing to persist. return result({ skipped: 'detector-missing', durationMs: Date.now() - started }); } const scanOptions = designSystemOptions(config, det, projectCwd); @@ -1435,6 +1470,7 @@ export async function runHook({ stdinJson, env = {}, cwd = process.cwd(), now = let detectorThrewAny = false; let lastSkip = 'no-scannable-file'; let suppressedHit = false; + let cacheDirty = false; for (const filePath of targetFiles) { audit.file = filePath; @@ -1467,6 +1503,7 @@ export async function runHook({ stdinJson, env = {}, cwd = process.cwd(), now = if (primaryFileSet.has(filePath)) { const editCount = bumpEditCount(cache, sessionId, filePath); + cacheDirty = true; audit.editCount = editCount; if (editCount > EDIT_COUNT_THRESHOLD) { @@ -1496,6 +1533,7 @@ export async function runHook({ stdinJson, env = {}, cwd = process.cwd(), now = if (fresh.length > 0) { rememberFindings(cache, sessionId, filePath, fresh); + cacheDirty = true; freshGroups.push({ filePath, findings: fresh }); continue; } @@ -1513,7 +1551,15 @@ export async function runHook({ stdinJson, env = {}, cwd = process.cwd(), now = } } - persistCache(projectCwd, cache); + // Persist only when the write is earned: fresh findings justify creating + // `.impeccable/` (dedup and suppression need it), and an already-present + // `.impeccable/` dir marks a project that opted in. A non-UI edit, or a + // clean UI edit in a project with no Impeccable footprint, must be a + // no-op on disk (issues #344, #305). + if (freshGroups.length > 0 + || (cacheDirty && fs.existsSync(path.join(projectCwd, '.impeccable')))) { + persistCache(projectCwd, cache); + } if (freshGroups.length > 0) { const firstGroup = freshGroups[0]; diff --git a/tests/hook.test.mjs b/tests/hook.test.mjs index 30631f9b6..ec3d9ec56 100644 --- a/tests/hook.test.mjs +++ b/tests/hook.test.mjs @@ -28,6 +28,7 @@ import { readConfig, readCache, persistCache, + resolveCacheCwd, bumpEditCount, rememberFindings, dedupeAgainstCache, @@ -1326,6 +1327,132 @@ rounded: }); }); +describe('runHook() — cache write gating (issues #344, #305)', () => { + // The hook must be a no-op on disk in projects that never earned an + // `.impeccable/` footprint: skipped files never dirty the cache, and a + // dirty cache is only persisted when there are fresh findings or the + // project already opted in (an `.impeccable/` dir exists). + let cwd; + beforeEach(() => { cwd = mkTmp(); }); + afterEach(() => fs.rmSync(cwd, { recursive: true, force: true })); + + function eventFor(file, sessionId = 'gate-sid') { + return { + session_id: sessionId, + cwd, + hook_event_name: 'PostToolUse', + tool_name: 'Edit', + tool_input: { file_path: file }, + }; + } + + function write(rel, body, base = cwd) { + const abs = path.join(base, rel); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, body); + return abs; + } + + it('non-UI edit (.md) does not create .impeccable/', async () => { + const file = write('notes/todo.md', '# notes'); + const r = await runHook({ + stdinJson: JSON.stringify(eventFor(file)), + env: {}, cwd, detector: fakeDetector([finding('side-tab', 1)]), + }); + assert.equal(r.audit.skipped, 'extension'); + assert.ok(!fs.existsSync(path.join(cwd, '.impeccable')), '.impeccable should not exist'); + }); + + it('clean UI edit in a project with no footprint does not create .impeccable/, still acks', async () => { + const file = write('src/Card.tsx', 'noop'); + const r = await runHook({ + stdinJson: JSON.stringify(eventFor(file)), + env: {}, cwd, detector: fakeDetector([]), + }); + assert.match(r.stdout, /No deterministic design-quality issues found/); + assert.ok(!fs.existsSync(path.join(cwd, '.impeccable')), '.impeccable should not exist'); + }); + + it('detector-missing path does not create .impeccable/', async () => { + const file = write('src/Card.tsx', 'noop'); + const r = await runHook({ + stdinJson: JSON.stringify(eventFor(file)), + env: {}, cwd, detector: {}, + }); + assert.equal(r.audit.skipped, 'detector-missing'); + assert.ok(!fs.existsSync(path.join(cwd, '.impeccable')), '.impeccable should not exist'); + }); + + it('fresh findings create the cache, and dedup works on the next run', async () => { + const file = write('src/Card.tsx', 'noop'); + const det = fakeDetector([finding('side-tab', 1)]); + const first = await runHook({ stdinJson: JSON.stringify(eventFor(file)), env: {}, cwd, detector: det }); + assert.match(first.stdout, /Design hook findings requiring review/); + assert.ok(fs.existsSync(path.join(cwd, '.impeccable', 'hook.cache.json')), 'cache should exist'); + + const second = await runHook({ stdinJson: JSON.stringify(eventFor(file)), env: {}, cwd, detector: det }); + assert.doesNotMatch(second.stdout, /Design hook findings requiring review/); + assert.match(second.stdout, /flagged earlier this session/); + }); + + it('clean UI edit in an opted-in project (existing .impeccable/) still persists editCount', async () => { + fs.mkdirSync(path.join(cwd, '.impeccable'), { recursive: true }); + const file = write('src/Card.tsx', 'noop'); + await runHook({ stdinJson: JSON.stringify(eventFor(file)), env: {}, cwd, detector: fakeDetector([]) }); + + const cache = readCache(cwd); + assert.equal(cache.sessions['gate-sid'].files[file].editCount, 1); + }); + + it('umbrella launch keys the cache to the edited file\'s project root', async () => { + // cwd is the umbrella: no .git / package.json / .impeccable of its own. + write('app/package.json', '{"name":"child"}'); + const file = write('app/src/Card.tsx', 'noop'); + const child = path.join(cwd, 'app'); + const r = await runHook({ + stdinJson: JSON.stringify(eventFor(file)), + env: {}, cwd, detector: fakeDetector([finding('side-tab', 1)]), + }); + assert.match(r.stdout, /Design hook findings requiring review/); + assert.equal(r.audit.cwd, child); + assert.ok(fs.existsSync(path.join(child, '.impeccable', 'hook.cache.json')), 'cache should land in the child project'); + assert.ok(!fs.existsSync(path.join(cwd, '.impeccable')), 'umbrella root should stay clean'); + }); +}); + +describe('resolveCacheCwd()', () => { + let cwd; + beforeEach(() => { cwd = mkTmp(); }); + afterEach(() => fs.rmSync(cwd, { recursive: true, force: true })); + + it('keeps the session cwd when it already looks like a project root', () => { + for (const marker of ['.git', '.impeccable']) { + const dir = path.join(cwd, `root-${marker}`); + fs.mkdirSync(path.join(dir, marker), { recursive: true }); + const file = path.join(dir, 'nested', 'app', 'src', 'Card.tsx'); + assert.equal(resolveCacheCwd(file, dir), dir); + } + const pkgDir = path.join(cwd, 'root-pkg'); + fs.mkdirSync(pkgDir, { recursive: true }); + fs.writeFileSync(path.join(pkgDir, 'package.json'), '{}'); + assert.equal(resolveCacheCwd(path.join(pkgDir, 'src', 'Card.tsx'), pkgDir), pkgDir); + }); + + it('climbs to the nearest marker root when the session cwd is a bare umbrella', () => { + const child = path.join(cwd, 'app'); + fs.mkdirSync(path.join(child, 'src'), { recursive: true }); + fs.writeFileSync(path.join(child, 'package.json'), '{}'); + assert.equal(resolveCacheCwd(path.join(child, 'src', 'Card.tsx'), cwd), child); + }); + + it('falls back to the session cwd when no marker is found or the path is unsafe', () => { + const file = path.join(cwd, 'app', 'src', 'Card.tsx'); + assert.equal(resolveCacheCwd(file, cwd), cwd); + assert.equal(resolveCacheCwd('', cwd), cwd); + assert.equal(resolveCacheCwd(`${cwd}/../etc/Card.tsx`, cwd), cwd); + }); +}); + describe('suppressionNotice()', () => { it('starts with envelope and mentions /impeccable audit', () => { const text = suppressionNotice('src/Card.tsx');