Merge pull request #491 from pbakaus/codex/issue-485-ignore-file-flags

Honor scope flags for ignore-file
2026-09-12 06:06:37 +03:00 · 2026-08-03 10:54:43 -07:00
parent 5dfeba6d3e 2345868c7b
commit d69bd093f9
2 changed files with 144 additions and 10 deletions
@@ -10,7 +10,7 @@
 *   node hook-admin.mjs off                            # set enabled: false
 *   node hook-admin.mjs ignore-rule <rule-id>          # append to ignoreRules
 *   node hook-admin.mjs ignore-rule overused-font --all-values
- *   node hook-admin.mjs ignore-file <glob>             # append to ignoreFiles
+ *   node hook-admin.mjs ignore-file <glob> [--shared|--local]   # append to ignoreFiles
 *   node hook-admin.mjs ignore-value <rule> <value>    # append to shared ignoreValues
 *   node hook-admin.mjs ignore-value <rule> <value> --local
 *   node hook-admin.mjs ignore-value <rule> "*" --file <glob>   # rule off in <glob> only
@@ -166,7 +166,7 @@ function readRawConfigFile(filePath) {
  }
 }

-const DETECTOR_CONFIG_KEYS = new Set(['ignoreRules', 'ignoreFiles', 'ignoreValues', 'designSystem']);
+const DETECTOR_CONFIG_KEYS = new Set(['ignoreRules', 'ignoreFiles', 'ignoreValues', 'designSystem', 'advisoryRules']);

 function hookSection(unified) {
  return unified && typeof unified === 'object' && !Array.isArray(unified) && unified.hook && typeof unified.hook === 'object' && !Array.isArray(unified.hook)
@@ -200,6 +200,15 @@ function stripDetectorKeys(raw) {
  return out;
 }

+function pickDetectorKeys(raw) {
+  if (!raw || typeof raw !== 'object' || Array.isArray(raw)) return {};
+  const out = {};
+  for (const [key, value] of Object.entries(raw)) {
+    if (DETECTOR_CONFIG_KEYS.has(key)) out[key] = value;
+  }
+  return out;
+}
+
 // Write hook runtime config under `hook`, leaving detector filters in
 // `detector` and preserving sibling keys such as updateCheck.
 function writeHookConfig(cwd, hookConfig, opts = {}) {
@@ -207,10 +216,19 @@ function writeHookConfig(cwd, hookConfig, opts = {}) {
  if (opts.local) ensureHookGitExcludes(cwd);
  const existingRaw = readRawConfigFile(filePath).raw;
  const existing = existingRaw && typeof existingRaw === 'object' && !Array.isArray(existingRaw) ? existingRaw : {};
-  const existingHook = stripDetectorKeys(hookSection(existing));
+  const existingHookSection = hookSection(existing);
+  const existingHook = stripDetectorKeys(existingHookSection);
+  const legacyDetector = pickDetectorKeys(existingHookSection);
  // Merge over the existing hook object so fields the merge helpers don't manage
  // (consent, quiet, auditLog) survive an Impeccable hooks edit.
  const next = { ...existing, hook: { ...existingHook, ...hookConfig } };
+  if (Object.keys(legacyDetector).length > 0) {
+    const existingDetector = detectorSection(existing) || {};
+    next.detector = {
+      ...existingDetector,
+      ...mergeDetectorConfig(existingDetector, mergeDetectorConfig(legacyDetector)),
+    };
+  }
  fs.mkdirSync(path.dirname(filePath), { recursive: true });
  fs.writeFileSync(filePath, JSON.stringify(next, null, 2) + '\n');
  return filePath;
@@ -222,10 +240,14 @@ function writeDetectorConfig(cwd, detectorConfig, opts = {}) {
  const existingRaw = readRawConfigFile(filePath).raw;
  const existing = existingRaw && typeof existingRaw === 'object' && !Array.isArray(existingRaw) ? existingRaw : {};
  const nextHook = stripDetectorKeys(hookSection(existing));
-  const existingDetector = mergeDetectorConfig(detectorSection(existing));
+  const existingDetectorSection = detectorSection(existing) || {};
+  const existingDetector = mergeDetectorConfig(existingDetectorSection);
  const next = {
    ...existing,
-    detector: mergeDetectorConfig(detectorConfig, existingDetector),
+    detector: {
+      ...existingDetectorSection,
+      ...mergeDetectorConfig(detectorConfig, existingDetector),
+    },
  };
  if (Object.keys(nextHook).length > 0) next.hook = nextHook;
  else delete next.hook;
@@ -259,12 +281,18 @@ function mergeDetectorConfig(existing, seed = null) {
  if (seed?.designSystem && typeof seed.designSystem === 'object' && !Array.isArray(seed.designSystem)) {
    out.designSystem = { ...seed.designSystem };
  }
+  if (seed?.advisoryRules === 'include' || seed?.advisoryRules === 'exclude') {
+    out.advisoryRules = seed.advisoryRules;
+  }
  if (base.designSystem && typeof base.designSystem === 'object' && !Array.isArray(base.designSystem)) {
    out.designSystem = {
      ...(out.designSystem || {}),
      enabled: base.designSystem.enabled === false ? false : true,
    };
  }
+  if (base.advisoryRules === 'include' || base.advisoryRules === 'exclude') {
+    out.advisoryRules = base.advisoryRules;
+  }
  if (Array.isArray(base.ignoreRules)) {
    out.ignoreRules = Array.from(new Set([...out.ignoreRules, ...base.ignoreRules.map(String)]));
  }
@@ -558,12 +586,44 @@ function addIgnoreRule(cwd, args) {
  return `Added "${rule}" to detector.ignoreRules. Current: ${config.ignoreRules.join(', ')}`;
 }

-function addIgnoreFile(cwd, glob) {
+function parseIgnoreFileArgs(args) {
+  const positionals = [];
+  let shared = false;
+  let local = false;
+
+  for (const raw of args) {
+    const arg = String(raw || '');
+    if (arg === '--shared') {
+      shared = true;
+    } else if (arg === '--local') {
+      local = true;
+    } else if (arg === '--reason' || arg.startsWith('--reason=')) {
+      throw new Error('--reason is not supported for ignore-file because detector.ignoreFiles stores globs only; use ignore-value when a documented rule-specific exception fits');
+    } else if (arg.startsWith('--')) {
+      throw new Error(`Unknown ignore-file flag: ${arg}`);
+    } else {
+      positionals.push(arg);
+    }
+  }
+
+  if (shared && local) throw new Error('Pass only one scope flag: --shared or --local');
+  if (positionals.length > 1) throw new Error('Pass exactly one glob to ignore-file');
+
+  return {
+    glob: positionals[0],
+    local,
+  };
+}
+
+function addIgnoreFile(cwd, args) {
+  const parsed = parseIgnoreFileArgs(args);
+  const glob = parsed.glob;
  if (!glob) throw new Error(`Pass a glob, e.g. ${IMPECCABLE_COMMAND} hooks ignore-file "src/legacy/**"`);
-  const config = mergeDetectorConfig(readRawDetectorConfig(cwd));
+  const config = mergeDetectorConfig(readRawDetectorConfig(cwd, { local: parsed.local }));
  if (!config.ignoreFiles.includes(glob)) config.ignoreFiles.push(glob);
-  writeDetectorConfig(cwd, config);
-  return `Added "${glob}" to detector.ignoreFiles. Current: ${config.ignoreFiles.join(', ')}`;
+  const target = writeDetectorConfig(cwd, config, { local: parsed.local });
+  const scope = parsed.local ? 'local detector.ignoreFiles' : 'shared detector.ignoreFiles';
+  return `Added "${glob}" to ${scope} (${path.relative(cwd, target) || target}). Current: ${config.ignoreFiles.join(', ')}`;
 }

 // An empty glob used to be dropped by filter(Boolean), so `--file=` reported
@@ -727,7 +787,7 @@ function main() {
      case 'on':     out = setEnabled(cwd, true); break;
      case 'off':    out = setEnabled(cwd, false); break;
      case 'ignore-rule': out = addIgnoreRule(cwd, rest); break;
-      case 'ignore-file': out = addIgnoreFile(cwd, rest[0]); break;
+      case 'ignore-file': out = addIgnoreFile(cwd, rest); break;
      case 'ignore-value': out = addIgnoreValue(cwd, rest); break;
      case 'reset':  out = reset(cwd); break;
    }
@@ -985,14 +985,88 @@ describe('hook-admin.mjs', () => {
    );
  });

+  it('ignore-file --local writes only the private detector config', () => {
+    const out = runAdmin(['ignore-file', '/abs/path/personal.html', '--local']);
+
+    assert.equal(fs.existsSync(getConfigPath(cwd)), false);
+    const local = JSON.parse(fs.readFileSync(getLocalConfigPath(cwd), 'utf-8')).detector;
+    assert.deepEqual(local.ignoreFiles, ['/abs/path/personal.html']);
+    assert.match(out, /local detector\.ignoreFiles/);
+  });
+
+  it('ignore-file --local preserves the local advisory-rule preference', () => {
+    fs.mkdirSync(path.dirname(getLocalConfigPath(cwd)), { recursive: true });
+    fs.writeFileSync(getLocalConfigPath(cwd), JSON.stringify({
+      detector: { advisoryRules: 'include' },
+    }));
+
+    runAdmin(['ignore-file', '/abs/path/personal.html', '--local']);
+
+    const local = JSON.parse(fs.readFileSync(getLocalConfigPath(cwd), 'utf-8')).detector;
+    assert.equal(local.advisoryRules, 'include');
+    assert.deepEqual(local.ignoreFiles, ['/abs/path/personal.html']);
+  });
+
+  for (const command of ['on', 'off']) {
+    it(`hooks ${command} migrates a legacy hook advisory-rule preference`, () => {
+      fs.mkdirSync(path.dirname(getConfigPath(cwd)), { recursive: true });
+      fs.writeFileSync(getConfigPath(cwd), JSON.stringify({
+        hook: { advisoryRules: 'include' },
+      }));
+
+      runAdmin([command]);
+
+      const config = JSON.parse(fs.readFileSync(getConfigPath(cwd), 'utf-8'));
+      assert.equal(config.hook.advisoryRules, undefined);
+      assert.equal(config.detector.advisoryRules, 'include');
+    });
+  }
+
+  it('hooks on keeps the canonical advisory-rule preference during legacy migration', () => {
+    fs.mkdirSync(path.dirname(getConfigPath(cwd)), { recursive: true });
+    fs.writeFileSync(getConfigPath(cwd), JSON.stringify({
+      hook: { advisoryRules: 'include' },
+      detector: {
+        advisoryRules: 'exclude',
+        extensions: [{ ext: '.blade.php', engine: 'html' }],
+      },
+    }));
+
+    runAdmin(['on']);
+
+    const config = JSON.parse(fs.readFileSync(getConfigPath(cwd), 'utf-8'));
+    assert.equal(config.hook.advisoryRules, undefined);
+    assert.equal(config.detector.advisoryRules, 'exclude');
+    assert.deepEqual(config.detector.extensions, [{ ext: '.blade.php', engine: 'html' }]);
+  });
+
+  it('ignore-file refuses unsupported reasons and unknown flags', () => {
+    assert.throws(
+      () => runAdmin(['ignore-file', 'src/legacy/**', '--reason', 'machine-local path']),
+      /--reason is not supported for ignore-file/,
+    );
+    assert.throws(
+      () => runAdmin(['ignore-file', 'src/legacy/**', '--shard']),
+      /Unknown ignore-file flag: --shard/,
+    );
+    assert.equal(fs.existsSync(getConfigPath(cwd)), false);
+    assert.equal(fs.existsSync(getLocalConfigPath(cwd)), false);
+  });
+
  it('ignore-file writes shared config that suppresses a later hook run', async () => {
    const file = path.join(cwd, 'src/ConfirmedCard.html');
    fs.mkdirSync(path.dirname(file), { recursive: true });
    fs.writeFileSync(file, '<div style="border-left: 4px solid #7c3aed; border-radius: 16px; padding: 16px;">Card</div>');

+    fs.mkdirSync(path.dirname(getConfigPath(cwd)), { recursive: true });
+    fs.writeFileSync(getConfigPath(cwd), JSON.stringify({
+      detector: { extensions: [{ ext: '.blade.php', engine: 'html' }] },
+    }));
+
    runAdmin(['ignore-file', 'src/ConfirmedCard.html']);

    const shared = JSON.parse(fs.readFileSync(getConfigPath(cwd), 'utf-8')).detector;
+    assert.deepEqual(shared.extensions, [{ ext: '.blade.php', engine: 'html' }]);
    assert.deepEqual(shared.ignoreFiles, ['src/ConfirmedCard.html']);

    const r = await runHook({