Harden IMPECCABLE_CACHE_ROOT edges: normalization, opt-in gate, failure path

- hookStateDir now trims the env value (stray whitespace in env files)
  and path.resolve()s both the root and the cwd, so trailing separators
  and relative segments slug to the same per-project dir.
- The #344/#305 persist gate also treats an existing (possibly
  redirected) cache file as the opted-in marker. Without this, once
  state relocated, clean-edit editCount bumps stopped persisting because
  the project-local .impeccable/ dir never appears. No-op under stock
  paths, where the cache file lives inside .impeccable/.
- New tests: slug normalization equivalences, whitespace trim, graceful
  persistCache failure on an unusable root, and three runHook
  end-to-end cases (findings persist + dedup through the redirect,
  clean-edit editCount persistence, and the no-footprint no-op gate
  holding under redirect).

Prepared with AI assistance (Claude Code) under direction of
0xDarkMatter, per the maintainer-approved issue #422.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

skill/scripts/hook-lib.mjs

@@ -218,11 +218,17 @@ export function getLocalConfigPath(cwd) {
 // artifacts (issue #422). User-authored config (config.json,
 // config.local.json, design.json) deliberately stays project-local — only
 // disposable state relocates.
+// Read from process.env (not runHook's injected env): the cache root is a
+// machine-scoped setting like CURSOR_PROJECT_DIR, not a per-invocation
+// switch. Trim guards against stray whitespace in env files; resolving both
+// sides makes the slug deterministic when callers hand in a trailing
+// separator or unnormalized cwd.
 function hookStateDir(cwd) {
-  const root = process.env.IMPECCABLE_CACHE_ROOT;
-  if (root && typeof root === 'string' && root.trim()) {
-    const slug = String(cwd).replace(/[:\\/.]/g, '-');
-    return path.join(root, slug);
+  const raw = process.env.IMPECCABLE_CACHE_ROOT;
+  const root = typeof raw === 'string' ? raw.trim() : '';
+  if (root) {
+    const slug = path.resolve(String(cwd)).replace(/[:\\/.]/g, '-');
+    return path.join(path.resolve(root), slug);
   }
   return path.join(cwd, '.impeccable');
 }
@@ -2139,8 +2145,13 @@ export async function runHook({ stdinJson, env = {}, cwd = process.cwd(), now =
     // touched-file list for the Stop deep pass, 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 (deferredTotal > 0 || (cacheDirty && fs.existsSync(path.join(projectCwd, '.impeccable')))) {
+    // no-op on disk (issues #344, #305). An existing cache file also counts
+    // as opted in: under IMPECCABLE_CACHE_ROOT (issue #422) state lives
+    // outside the project, so the project dir alone can't carry the marker —
+    // without this, clean-edit editCount bumps would stop persisting the
+    // moment state relocates. Under stock paths the cache sits inside
+    // `.impeccable/`, so the extra check changes nothing there.
+    if (deferredTotal > 0 || (cacheDirty && (fs.existsSync(path.join(projectCwd, '.impeccable')) || fs.existsSync(getCachePath(projectCwd))))) {
       persistCache(projectCwd, cache);
     }
 

tests/hook.test.mjs

@@ -457,11 +457,36 @@ describe('IMPECCABLE_CACHE_ROOT relocates hook state (issue #422)', () => {
     assert.equal(getPendingPath(cwd), path.join(cacheRoot, slug, 'hook.pending.json'));
   });
 
-  it('slug maps colons, slashes, backslashes, and dots to hyphens', () => {
+  it('slug maps separators, colons, and dots to hyphens', () => {
     process.env.IMPECCABLE_CACHE_ROOT = cacheRoot;
-    const cachePath = getCachePath('C:\\work\\my.app/sub');
-    const slugDir = path.basename(path.dirname(cachePath));
-    assert.equal(slugDir, 'C--work-my-app-sub');
+    const proj = path.join(cwd, 'my.app', 'v2');
+    const slugDir = path.basename(path.dirname(getCachePath(proj)));
+    assert.doesNotMatch(slugDir, /[:\\/.]/, 'no path-significant chars survive');
+    assert.ok(slugDir.endsWith('my-app-v2'), `dots and separators map to hyphens (got ${slugDir})`);
+  });
+
+  it('trailing separators and relative segments slug to the same dir', () => {
+    process.env.IMPECCABLE_CACHE_ROOT = cacheRoot;
+    const canonical = getCachePath(cwd);
+    assert.equal(getCachePath(cwd + path.sep), canonical);
+    assert.equal(getCachePath(path.join(cwd, 'sub', '..')), canonical);
+  });
+
+  it('trims stray whitespace from the env value', () => {
+    process.env.IMPECCABLE_CACHE_ROOT = `  ${cacheRoot}  `;
+    const slug = path.resolve(cwd).replace(/[:\\/.]/g, '-');
+    assert.equal(getCachePath(cwd), path.join(cacheRoot, slug, 'hook.cache.json'));
+  });
+
+  it('persistCache degrades gracefully when the cache root is unusable', () => {
+    // Point the root at an existing FILE so mkdir of the slug dir must fail.
+    const blocker = path.join(cacheRoot, 'not-a-dir');
+    fs.writeFileSync(blocker, 'x');
+    process.env.IMPECCABLE_CACHE_ROOT = blocker;
+    const cache = readCache(cwd);
+    bumpEditCount(cache, 'sid-1', '/x/a.tsx');
+    assert.equal(persistCache(cwd, cache), false, 'returns false instead of throwing');
+    assert.equal(fs.existsSync(path.join(cwd, '.impeccable')), false);
   });
 
   it('config paths stay project-local even when the redirect is active', () => {
@@ -483,6 +508,79 @@ describe('IMPECCABLE_CACHE_ROOT relocates hook state (issue #422)', () => {
     const reloaded = readCache(cwd);
     assert.equal(reloaded.sessions['sid-1'].files['/x/a.tsx'].editCount, 1);
   });
+
+  function redirectEventFor(file, sessionId = 'redir-sid') {
+    return {
+      session_id: sessionId,
+      cwd,
+      hook_event_name: 'PostToolUse',
+      tool_name: 'Edit',
+      tool_input: { file_path: file },
+    };
+  }
+
+  function writeProjectFile(rel, body) {
+    const abs = path.join(cwd, rel);
+    fs.mkdirSync(path.dirname(abs), { recursive: true });
+    fs.writeFileSync(abs, body);
+    return abs;
+  }
+
+  it('runHook end-to-end: findings persist under the redirect root, project root stays clean', async () => {
+    process.env.IMPECCABLE_CACHE_ROOT = cacheRoot;
+    const file = writeProjectFile('src/Card.tsx', 'noop');
+    const det = fakeDetector([finding('text-overflow', 1)]);
+
+    const first = await runHook({
+      stdinJson: JSON.stringify(redirectEventFor(file)),
+      env: {}, cwd, detector: det,
+    });
+    assert.match(first.stdout, /Design hook findings requiring review/);
+    assert.equal(fs.existsSync(path.join(cwd, '.impeccable')), false, 'no project-local footprint');
+    assert.equal(fs.existsSync(getCachePath(cwd)), true, 'cache lands under the redirect root');
+
+    // Session dedup still works across runs through the redirected cache.
+    const second = await runHook({
+      stdinJson: JSON.stringify(redirectEventFor(file)),
+      env: {}, cwd, detector: det,
+    });
+    assert.doesNotMatch(second.stdout, /Design hook findings requiring review/);
+    assert.match(second.stdout, /flagged earlier this session/);
+  });
+
+  it('runHook end-to-end: clean edits keep persisting editCount once redirected state exists', async () => {
+    process.env.IMPECCABLE_CACHE_ROOT = cacheRoot;
+    const file = writeProjectFile('src/Card.tsx', 'noop');
+
+    // Earn the footprint (in the redirect dir) with a real finding first.
+    await runHook({
+      stdinJson: JSON.stringify(redirectEventFor(file)),
+      env: {}, cwd, detector: fakeDetector([finding('text-overflow', 1)]),
+    });
+    assert.equal(fs.existsSync(getCachePath(cwd)), true);
+
+    // A clean follow-up edit must still persist its editCount bump — the
+    // opted-in check has to see the redirected cache, not just `<cwd>/.impeccable/`.
+    await runHook({
+      stdinJson: JSON.stringify(redirectEventFor(file)),
+      env: {}, cwd, detector: fakeDetector([]),
+    });
+    const cache = readCache(cwd);
+    assert.equal(cache.sessions['redir-sid'].files[file].editCount, 2);
+    assert.equal(fs.existsSync(path.join(cwd, '.impeccable')), false, 'project root still clean');
+  });
+
+  it('runHook end-to-end: a no-footprint clean edit writes nothing anywhere (gates hold under redirect)', async () => {
+    process.env.IMPECCABLE_CACHE_ROOT = cacheRoot;
+    const file = writeProjectFile('src/Card.tsx', 'noop');
+    const r = await runHook({
+      stdinJson: JSON.stringify(redirectEventFor(file)),
+      env: {}, cwd, detector: fakeDetector([]),
+    });
+    assert.match(r.stdout, /No deterministic design-quality issues found/);
+    assert.equal(fs.existsSync(path.join(cwd, '.impeccable')), false);
+    assert.equal(fs.existsSync(getCachePath(cwd)), false, 'redirect root also stays empty');
+  });
 });
 
 describe('ensureHookGitExcludes()', () => {