mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-14 07:06:45 +03:00
Fix: stop the design hook from creating .impeccable/ in unrelated projects (#346)
The PostToolUse hook was writing hook.cache.json after every edit, even when nothing was scanned or recorded. Gate the persist to earned writes only, and key the cache to the edited file's project root when the session starts from an umbrella directory. Fixes #344, #305 Co-authored-by: Abdul Wahab <abdulwahab@Abduls-MacBook-Pro-2.local> Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
co-authored by
Abdul Wahab
Cursor
parent
582f23eae3
commit
751ec31dd6
@@ -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,
|
||||
|
||||
@@ -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];
|
||||
|
||||
@@ -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');
|
||||
|
||||
Reference in New Issue
Block a user