diff --git a/skill/scripts/hook-admin.mjs b/skill/scripts/hook-admin.mjs index 8e1230d9f..ac3363d27 100644 --- a/skill/scripts/hook-admin.mjs +++ b/skill/scripts/hook-admin.mjs @@ -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,15 @@ 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) { + next.detector = mergeDetectorConfig(detectorSection(existing), mergeDetectorConfig(legacyDetector)); + } fs.mkdirSync(path.dirname(filePath), { recursive: true }); fs.writeFileSync(filePath, JSON.stringify(next, null, 2) + '\n'); return filePath; diff --git a/tests/hook.test.mjs b/tests/hook.test.mjs index f7b39d2d7..87c55183a 100644 --- a/tests/hook.test.mjs +++ b/tests/hook.test.mjs @@ -1007,6 +1007,35 @@ describe('hook-admin.mjs', () => { 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' }, + })); + + runAdmin(['on']); + + const config = JSON.parse(fs.readFileSync(getConfigPath(cwd), 'utf-8')); + assert.equal(config.hook.advisoryRules, undefined); + assert.equal(config.detector.advisoryRules, 'exclude'); + }); + it('ignore-file refuses unsupported reasons and unknown flags', () => { assert.throws( () => runAdmin(['ignore-file', 'src/legacy/**', '--reason', 'machine-local path']),