mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-14 15:16:35 +03:00
feat(cli): interactive hook consent + unified .impeccable/config.json (#245)
* feat(cli): interactive hook consent + unified .impeccable/config.json Make the design-hook install a conscious choice and unify scattered config into one file. Interactive consent - On an interactive `skills install`/`update`, the CLI explains what the hook does and offers to install it (default yes), then records the per-developer decision in the gitignored `.impeccable/config.local.json`, so it never re-asks. A recorded decision or an already-installed hook short-circuits; `-y`/non-TTY keeps the historical install-by-default behavior; `--no-hooks` is a one-off skip that records nothing. The trigger keys on "is the hook installed?" + "is there a recorded decision?", not a brittle version check. Unified config - `.impeccable/config.json` (shared) and `.impeccable/config.local.json` (gitignored) now hold all Impeccable settings: hook settings under a `hook` key, plus top-level `updateCheck`. `/impeccable hooks` writes the `hook` subtree, preserving siblings. The hook runtime reads `hook.quiet` and `hook.auditLog`; context boot reads `updateCheck`. The legacy `IMPECCABLE_HOOK_DISABLED|QUIET|LOG` and `IMPECCABLE_NO_UPDATE_CHECK` env vars still work and override config; docs now lead with config and treat env vars as a legacy note. - No backward compat for the pre-unification `hook.json`/`hook.local.json` (the hook shipped an hour ago; nothing in the wild uses it). This repo's own hook config is migrated to `.impeccable/config.json`. The CLI and skill scripts are separate trees, so a small CLI-side config module (cli/lib/impeccable-config.mjs) duplicates the config-path and .git/info/exclude handling; comments flag the duplication. Tests: new cli config unit test; skills-cli consent tests (declined skips, accepted installs, --no-hooks records nothing); hook.test.mjs back-compat removed and quiet/auditLog-from-config + gitexclude coverage added. Full suite green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(hooks): preserve sibling config fields + resolve audit log from event cwd (Bugbot) Two Bugbot findings: - High: `/impeccable hooks` edits replaced the whole `hook` object with the merge-helper output, dropping fields those helpers don't manage — so an `ignore-value --local` could wipe the recorded install consent and make the CLI re-prompt. writeConfig now merges over the existing hook object, keeping consent/quiet/auditLog. - Medium: config-based audit logging resolved hook.auditLog from process.cwd(), which can differ from the hook event's project root (and Cursor's pre-edit hook passed no cwd). The hook now stamps the resolved project root on the audit entry, and writeAuditLog reads config from entry.cwd when present. Tests: a /impeccable hooks edit preserves consent + quiet; writeAuditLog resolves config auditLog from entry.cwd, not the fallback cwd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(hooks): resolve a relative auditLog path against the project root (Bugbot) A relative hook.auditLog was read from the project root but written relative to the hook process cwd, so when those differ the log went to the wrong place. writeAuditLog now resolves a relative target (from env or config) against the same project root it reads config from. Absolute and ~/ paths are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix hook consent recovery and smoke config * Fix hook consent explainer for Cursor * Fix empty hook target consent --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
9c0012d4e1
commit
8cf2be110d
@@ -54,8 +54,11 @@ const smokeFiles = {
|
||||
};
|
||||
|
||||
const results = [];
|
||||
const hookConfigFiles = ['.impeccable/config.json', '.impeccable/config.local.json'];
|
||||
const originalHookConfigFiles = new Map();
|
||||
|
||||
main().catch((error) => {
|
||||
restoreHookConfigFiles();
|
||||
if (!results.some((result) => !result.pass)) {
|
||||
record('fatal', false, String(error?.message || error), 'fatal');
|
||||
}
|
||||
@@ -67,6 +70,7 @@ main().catch((error) => {
|
||||
async function main() {
|
||||
assertPath(targetRepo, 'target repo');
|
||||
assertPath(bundlePath, 'universal bundle');
|
||||
snapshotHookConfigFiles();
|
||||
mkdirSync(smokeDir, { recursive: true });
|
||||
ensureTargetGitExclude();
|
||||
|
||||
@@ -85,6 +89,7 @@ async function main() {
|
||||
|
||||
cleanSmokeFiles();
|
||||
clearRuntimeState();
|
||||
restoreHookConfigFiles();
|
||||
writeSummary();
|
||||
|
||||
const failed = results.filter((result) => !result.pass);
|
||||
@@ -238,7 +243,7 @@ function cleanInstalledImpeccable() {
|
||||
rmSync(join(targetRepo, rel), { recursive: true, force: true });
|
||||
}
|
||||
|
||||
for (const rel of ['.claude/settings.json', '.cursor/hooks.json', '.codex/hooks.json']) {
|
||||
for (const rel of ['.claude/settings.json', '.claude/settings.local.json', '.cursor/hooks.json', '.codex/hooks.json']) {
|
||||
stripManifest(rel);
|
||||
}
|
||||
|
||||
@@ -319,6 +324,47 @@ function stripManifest(rel) {
|
||||
}
|
||||
}
|
||||
|
||||
function snapshotHookConfigFiles() {
|
||||
for (const rel of hookConfigFiles) {
|
||||
const file = join(targetRepo, rel);
|
||||
originalHookConfigFiles.set(rel, existsSync(file) ? readFileSync(file, 'utf8') : null);
|
||||
}
|
||||
}
|
||||
|
||||
function restoreHookConfigFiles() {
|
||||
if (originalHookConfigFiles.size === 0) return;
|
||||
for (const [rel, content] of originalHookConfigFiles.entries()) {
|
||||
const file = join(targetRepo, rel);
|
||||
if (content === null) {
|
||||
rmSync(file, { force: true });
|
||||
} else {
|
||||
mkdirSync(dirname(file), { recursive: true });
|
||||
writeFileSync(file, content);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function resetHookConfigForSmoke() {
|
||||
for (const rel of hookConfigFiles) {
|
||||
const file = join(targetRepo, rel);
|
||||
if (!existsSync(file)) continue;
|
||||
let raw;
|
||||
try {
|
||||
raw = JSON.parse(readFileSync(file, 'utf8'));
|
||||
} catch {
|
||||
rmSync(file, { force: true });
|
||||
continue;
|
||||
}
|
||||
if (!raw || typeof raw !== 'object' || Array.isArray(raw) || !('hook' in raw)) continue;
|
||||
const { hook, ...rest } = raw;
|
||||
if (Object.keys(rest).length === 0) {
|
||||
rmSync(file, { force: true });
|
||||
} else {
|
||||
writeFileSync(file, `${JSON.stringify(rest, null, 2)}\n`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function stripImpeccableHookEntry(entry) {
|
||||
if (!entry || typeof entry !== 'object') return entry;
|
||||
if (containsImpeccableHook(entry)) return null;
|
||||
@@ -338,7 +384,7 @@ function containsImpeccableHook(value) {
|
||||
}
|
||||
|
||||
function verifyInstallShape() {
|
||||
const claude = readText('.claude/settings.json');
|
||||
const claude = readText('.claude/settings.local.json');
|
||||
const codex = readText('.codex/hooks.json');
|
||||
const cursor = readText('.cursor/hooks.json');
|
||||
assertCount(claude, '.claude/skills/impeccable/scripts/hook.mjs', 1, 'Claude hook.mjs');
|
||||
@@ -464,14 +510,13 @@ function runConfirmedExceptionPersistenceChecks() {
|
||||
for (const provider of providers) {
|
||||
runConfirmedExceptionForProvider(provider);
|
||||
}
|
||||
record('confirmed exception persistence', true, `${providers.join(', ')} ignored confirmed overused-font values through shared hook.json, not source comments`);
|
||||
record('confirmed exception persistence', true, `${providers.join(', ')} ignored confirmed overused-font values through shared config.json, not source comments`);
|
||||
}
|
||||
|
||||
function runConfirmedExceptionForProvider(provider) {
|
||||
clearRuntimeState();
|
||||
const rel = confirmedSmokeFile(provider);
|
||||
const file = writeConfirmedFixture(rel);
|
||||
const configPath = join(targetRepo, '.impeccable', 'hook.json');
|
||||
const beforeLog = `${provider}-confirmed-before.ndjson`;
|
||||
const afterLog = `${provider}-confirmed-after.ndjson`;
|
||||
|
||||
@@ -480,9 +525,7 @@ function runConfirmedExceptionForProvider(provider) {
|
||||
|
||||
const first = runInstalledProviderHook(provider, file, beforeLog);
|
||||
requireRuleFinding(`${provider} confirmed exception first hook`, `${first.stdout}\n${first.stderr}\n${readMaybe(join(smokeDir, beforeLog))}`, 'overused-font');
|
||||
if (existsSync(configPath)) {
|
||||
throw new Error(`${provider} hook wrote .impeccable/hook.json before explicit confirmation`);
|
||||
}
|
||||
assertNoSpecificFontIgnoreConfig(provider);
|
||||
|
||||
run('node', [
|
||||
providerAdminScript(provider),
|
||||
@@ -498,7 +541,7 @@ function runConfirmedExceptionForProvider(provider) {
|
||||
timeoutMs: 60 * 1000,
|
||||
});
|
||||
|
||||
const config = readJson(configPath);
|
||||
const config = readSharedHookConfig();
|
||||
assertSpecificFontIgnoreConfig(provider, config);
|
||||
|
||||
clearTransientHookState();
|
||||
@@ -539,7 +582,6 @@ function runAgentChosenFontExceptionForProvider(provider) {
|
||||
clearRuntimeState();
|
||||
const rel = agentChoiceSmokeFile(provider);
|
||||
const file = writeConfirmedFixture(rel);
|
||||
const configPath = join(targetRepo, '.impeccable', 'hook.json');
|
||||
const beforeLog = `${provider}-agent-choice-before.ndjson`;
|
||||
const afterLog = `${provider}-agent-choice-after.ndjson`;
|
||||
|
||||
@@ -548,13 +590,11 @@ function runAgentChosenFontExceptionForProvider(provider) {
|
||||
|
||||
const first = runInstalledProviderHook(provider, file, beforeLog);
|
||||
requireRuleFinding(`${provider} agent-choice first hook`, `${first.stdout}\n${first.stderr}\n${readMaybe(join(smokeDir, beforeLog))}`, 'overused-font');
|
||||
if (existsSync(configPath)) {
|
||||
throw new Error(`${provider} hook wrote .impeccable/hook.json before explicit confirmation`);
|
||||
}
|
||||
assertNoSpecificFontIgnoreConfig(provider);
|
||||
|
||||
runProviderAgentFontException(provider, rel);
|
||||
|
||||
const config = readJson(configPath);
|
||||
const config = readSharedHookConfig();
|
||||
assertSpecificFontIgnoreConfig(provider, config);
|
||||
|
||||
clearTransientHookState();
|
||||
@@ -648,6 +688,30 @@ function assertSpecificFontIgnoreConfig(provider, config) {
|
||||
}
|
||||
}
|
||||
|
||||
function assertNoSpecificFontIgnoreConfig(provider) {
|
||||
const file = join(targetRepo, '.impeccable', 'config.json');
|
||||
if (!existsSync(file)) return;
|
||||
const raw = readJson(file);
|
||||
const config = raw && typeof raw === 'object' && !Array.isArray(raw) && raw.hook && typeof raw.hook === 'object'
|
||||
? raw.hook
|
||||
: null;
|
||||
if (!config) return;
|
||||
const broad = Array.isArray(config.ignoreRules) && config.ignoreRules.includes('overused-font');
|
||||
const specific = Array.isArray(config.ignoreValues)
|
||||
&& config.ignoreValues.some((entry) => entry.rule === 'overused-font' && entry.value === 'roboto');
|
||||
if (broad || specific) {
|
||||
throw new Error(`${provider} hook config already suppressed overused-font=roboto before explicit confirmation`);
|
||||
}
|
||||
}
|
||||
|
||||
function readSharedHookConfig() {
|
||||
const raw = readJson(join(targetRepo, '.impeccable', 'config.json'));
|
||||
if (!raw || typeof raw !== 'object' || Array.isArray(raw) || !raw.hook || typeof raw.hook !== 'object') {
|
||||
throw new Error('Missing .impeccable/config.json hook config');
|
||||
}
|
||||
return raw.hook;
|
||||
}
|
||||
|
||||
function runInstalledProviderHook(provider, file, logName) {
|
||||
const env = { IMPECCABLE_HOOK_LOG: join(smokeDir, logName) };
|
||||
if (provider === 'claude') {
|
||||
@@ -891,7 +955,7 @@ function fontExceptionPrompt(provider, rel) {
|
||||
return [
|
||||
`Read the installed Impeccable hooks reference for ${provider}, then persist a confirmed hook exception for Roboto specifically in ${rel}.`,
|
||||
'The user confirms Roboto is intentional for this fixture, but did not ask to ignore overused fonts generally.',
|
||||
'Use the /impeccable hooks / hook-admin flow; do not edit .impeccable/hook.json by hand and do not edit the source fixture.',
|
||||
'Use the /impeccable hooks / hook-admin flow; do not edit .impeccable/config.json by hand and do not edit the source fixture.',
|
||||
'The final config must use ignoreValues for overused-font=roboto and must not add overused-font to ignoreRules.',
|
||||
'After updating the config, stop.',
|
||||
].join(' ');
|
||||
@@ -959,6 +1023,7 @@ function cleanSmokeFiles() {
|
||||
}
|
||||
|
||||
function clearRuntimeState() {
|
||||
resetHookConfigForSmoke();
|
||||
for (const rel of [
|
||||
'.impeccable/hook.cache.json',
|
||||
'.impeccable/hook.pending.json',
|
||||
|
||||
@@ -41,6 +41,7 @@ export const SUITES = {
|
||||
'tests/lib/provider-blocks.test.js',
|
||||
'tests/lib/transformers/provider-blocks.test.js',
|
||||
'tests/lib/utils.test.js',
|
||||
'tests/lib/impeccable-config.test.js',
|
||||
'tests/lib/transformers/factory.test.js',
|
||||
'tests/lib/transformers/providers.test.js',
|
||||
'tests/docs-integrity.test.js',
|
||||
|
||||
Reference in New Issue
Block a user