From 6509b1497efc1ef281439e33c5c22afd95ad945b Mon Sep 17 00:00:00 2001 From: Abdul Wahab Date: Tue, 4 Aug 2026 14:11:17 +0500 Subject: [PATCH] Denoise the design hook and let agents self-serve confident ignores (#497) The directive footer now emits in full once per session (a one-line reminder after), the DESIGN.md staleness note is mentioned once per session, rule descriptions dedupe within an emission, and the per-line ignore suggestion shrinks to the bare rule/value pair. The footer and hooks.md replace the confirmation-gated ignore policy with a three-way triage: fix real problems, self-serve the narrowest ignore for confident false positives or sanctioned exceptions and disclose it (with an honest --reason), ask when unsure. Self-serve stops at ignore-value, and the footer now gives a runnable hook-admin.mjs command instead of a slash command agents cannot execute. Measured on a seeded lab session replaying 11 hook events: 33,658 to 14,063 chars of agent-visible output (-58%). AI-assisted (Cursor agent), directed and reviewed by @abdulwahabone. Co-authored-by: Cursor --- skill/reference/hooks.md | 18 ++- skill/scripts/hook-before-edit.mjs | 15 ++- skill/scripts/hook-lib.mjs | 206 +++++++++++++++++++++-------- tests/hook.test.mjs | 137 +++++++++++++++---- 4 files changed, 280 insertions(+), 96 deletions(-) diff --git a/skill/reference/hooks.md b/skill/reference/hooks.md index fa6222274..d6df929de 100644 --- a/skill/reference/hooks.md +++ b/skill/reference/hooks.md @@ -48,14 +48,20 @@ The first argument is the action. Defaults to `status`. 5. If `` is `ignore-value`, `ignore-file`, or `ignore-rule`, just print the script output. The default scope is shared `.impeccable/config.json`; add `--local` only when the user explicitly asks for a private exception. 6. If `` is `status`, just print the script output. Do not add commentary unless the user asked a follow-up question. -## Intentional findings +## Triage findings -The hook itself never writes ignore config. Persist an exception only after the user explicitly confirms the flagged issue is intentional, and always go through `hook-admin.mjs`. +The hook itself never writes ignore config; every exception goes through `hook-admin.mjs`. Triage each finding into one of three outcomes: + +- **Real design problem**: fix it. Never add an ignore to skip a fix or to push a blocked write through. +- **Confident false positive or sanctioned exception**: persist the narrowest ignore yourself and disclose it in your reply. The bar is evidence you can name: an intentional demo or fixture, documentation of bad design, literal or domain-appropriate motion (a ball that bounces), or a choice the user already confirmed. Put that evidence in `--reason` as `""`; write "user confirmed" only when the user actually did. +- **Unsure**: leave the finding standing and ask the user in one line. Ask once; a one-line question costs less than the hook re-firing on every later edit. + +Self-serve stops at `ignore-value`. `ignore-file` and `ignore-rule` silence too much to add on your own judgment; ask the user first. Prefer the narrowest exception: -- If the finding line shows an exact `ignore-value` command, run that command. This writes shared `.impeccable/config.json` by default. -- For value-specific findings such as `overused-font` and `bounce-easing`, use `ignore-value` when the user confirms the specific value. Do not use `ignore-rule overused-font` for a specific font. +- If the finding line shows an `ignore-value ` pair, pass it to `hook-admin.mjs ignore-value` with your `--reason`. This writes shared `.impeccable/config.json` by default. +- For value-specific findings such as `overused-font` and `bounce-easing`, use `ignore-value` for the specific value. Do not use `ignore-rule overused-font` for a specific font. - If the finding has no value-specific command, such as `side-tab`, scope that one rule to the file: `ignore-value "*" --file `. Run `npx impeccable detect ` first to see what actually fires there. - Reach for `ignore-file ` only when the whole file is out of scope for design review: a fixture, a generated artifact, a deliberate slop demo. It silences every rule for that file permanently, including rules that have not been written yet. A real UI surface with one noisy rule wants the file-scoped value ignore above. - Use `ignore-rule ` only when the user asks to suppress that whole rule across the project. For broad overused-font suppression, use `ignore-rule overused-font --all-values` only when the user asks to ignore overused fonts generally. @@ -67,10 +73,10 @@ Example value-specific exception: node {{scripts_path}}/hook-admin.mjs ignore-value overused-font Inter --shared --reason "User confirmed Inter is intentional" ``` -Example intentional motion exception: +Example self-served exception, with the evidence named: ```bash -node {{scripts_path}}/hook-admin.mjs ignore-value bounce-easing bounce-ball --shared --reason "User confirmed ball bounce animation is intentional" +node {{scripts_path}}/hook-admin.mjs ignore-value bounce-easing bounce-ball --shared --reason "Agent: literal ball-bounce animation, bounce easing is the subject" ``` Example whole-rule font exception: diff --git a/skill/scripts/hook-before-edit.mjs b/skill/scripts/hook-before-edit.mjs index 1dcde6ef3..9398df93b 100644 --- a/skill/scripts/hook-before-edit.mjs +++ b/skill/scripts/hook-before-edit.mjs @@ -19,8 +19,9 @@ import { EDIT_COUNT_THRESHOLD, GENERATED_PATH, SENSITIVE_PATH, - appendDesignSystemNote, + appendDesignSystemNoteOnce, designSystemOptions, + footerModeForSession, filterFindings, isNativePlatform, isScanTargetInsideProject, @@ -345,8 +346,8 @@ async function detectProposedHtml(detector, content, filePath, scanOptions) { } } -function cursorBlockMessage(findings, filePath, config, cwd) { - const rendered = renderTemplate(findings, filePath, config, { cwd }); +function cursorBlockMessage(findings, filePath, config, cwd, footerMode) { + const rendered = renderTemplate(findings, filePath, config, { cwd, footer: footerMode }); const blocked = rendered.replace( '[impeccable@1] Design hook findings requiring review', '[impeccable@1] Impeccable design hook blocked this write before it landed. Design hook findings requiring review', @@ -468,9 +469,15 @@ async function main() { }); } - const message = appendDesignSystemNote(cursorBlockMessage(filtered, filePath, config, cwd), scanOptions); const sessionId = event.session_id || event.conversation_id || 'unknown'; const cache = readCache(cwd); + // Repeated denials for the same session repeat the findings, not the + // policy: the full footer emits once per session, the short form after. + const footerMode = footerModeForSession(cache, sessionId); + const message = appendDesignSystemNoteOnce( + cursorBlockMessage(filtered, filePath, config, cwd, footerMode), + scanOptions, cache, sessionId, + ); const denial = bumpCursorDenial(cache, sessionId, filePath, filtered); persistCache(cwd, cache); if (denial.count > EDIT_COUNT_THRESHOLD) { diff --git a/skill/scripts/hook-lib.mjs b/skill/scripts/hook-lib.mjs index b874985a6..3d034d063 100644 --- a/skill/scripts/hook-lib.mjs +++ b/skill/scripts/hook-lib.mjs @@ -22,6 +22,8 @@ * dedupeAgainstCache(findings, cache, sessionId, filePath) * renderTemplate(findings, filePath, config, opts) * renderCleanAck(filePath, opts) / renderPendingAck(filePath, known, opts) + * appendDesignSystemNote(text, scanOptions) / appendDesignSystemNoteOnce(text, scanOptions, cache, sessionId) + * footerModeForSession(cache, sessionId) * shouldEmitAckForFile(filePath, config?) * writeAuditLog(env, entry) * loadDetector() -> Promise<{ detectText, detectHtml }> @@ -979,11 +981,12 @@ export function renderTemplate(findings, filePath, config, opts = {}) { const remaining = total - shown.length; const header = `${ENVELOPE_PREFIX} Design hook findings requiring review in ${display} (${total} issue(s)):`; - const lines = shown.map((f) => formatFindingLine(f)); + const seenRules = new Set(); + const lines = shown.map((f) => formatDedupedFindingLine(f, seenRules)); const more = remaining > 0 ? `... and ${remaining} more (see ${IMPECCABLE_COMMAND} audit).` : null; - const footer = directiveFooter(display); + const footer = directiveFooter({ mode: opts.footer }); const blocks = [header, ...lines]; if (more) blocks.push(more); @@ -1013,6 +1016,9 @@ function renderGroupedTemplate(groups, config, opts = {}) { const header = `${ENVELOPE_PREFIX} Design hook findings requiring review across ${realGroups.length} files (${total} issue(s)):`; const lines = []; let shownCount = 0; + // One seen-set across all groups: a rule already described under one file + // is not re-described under the next. + const seenRules = new Set(); for (const group of realGroups) { const display = relativize(group.filePath, cwd); @@ -1020,7 +1026,7 @@ function renderGroupedTemplate(groups, config, opts = {}) { const remainingCap = Math.max(0, cap - shownCount); const shown = group.findings.slice(0, remainingCap); for (const finding of shown) { - lines.push(formatFindingLine(finding)); + lines.push(formatDedupedFindingLine(finding, seenRules)); } shownCount += shown.length; const hidden = group.findings.length - shown.length; @@ -1029,7 +1035,7 @@ function renderGroupedTemplate(groups, config, opts = {}) { } } - const footer = directiveFooter('the affected files', { grouped: true }); + const footer = directiveFooter({ mode: opts.footer }); let text = [header, ...lines, '', footer].join('\n'); if (text.length > maxChars) { text = clampGroupedToBudget(header, lines, footer, maxChars); @@ -1083,30 +1089,45 @@ function clampToBudget(header, lines, more, footer, maxChars) { return assembled; } -function formatFindingLine(f) { +// `compact` drops the registry description: within one emission the first +// occurrence of a rule carries the full description and repeats keep only the +// rule id, name, and their own ignore hint (values differ per line, so the +// hint must survive the dedupe). +function formatFindingLine(f, opts = {}) { const prefix = f.line && f.line > 0 ? `- L${f.line}` : '-'; - const desc = (f.description || '').trim(); + const desc = opts.compact ? '' : (f.description || '').trim(); const name = (f.name || '').trim(); // Description from the registry already ends in punctuation; join with a // single space. `name` may have a trailing period already, keep it clean. const nameSegment = name ? `${name.replace(/\.+\s*$/, '')}.` : ''; - const ignoreCommand = formatFindingIgnoreCommand(f); - const ignoreSegment = ignoreCommand - ? ` If the user explicitly confirms this value is intentional: \`${ignoreCommand}\`.` - : ''; + const ignoreHint = formatFindingIgnoreHint(f); + const ignoreSegment = ignoreHint ? ` If intentional: \`${ignoreHint}\`.` : ''; return `${prefix} [${f.antipattern}] ${nameSegment} ${desc}${ignoreSegment}`.replace(/\s+/g, ' ').trim(); } -function formatFindingIgnoreCommand(finding) { +// Dedupe applied in shown-line order, so the first rendered occurrence of a +// rule always carries the description. The budget clamps pop lines from the +// end, which can never orphan a compact repeat before its described first +// occurrence. +function formatDedupedFindingLine(finding, seenRules) { + const rule = normalizeIgnoreRule(finding?.antipattern); + const compact = rule ? seenRules.has(rule) : false; + if (rule) seenRules.add(rule); + return formatFindingLine(finding, { compact }); +} + +// The rule/value pair the footer's `hook-admin.mjs ignore-value` command +// takes. Deliberately just the args: the executable prefix, the --reason +// contract, and the disclosure rule live in the directive footer, stated once +// instead of per line. +function formatFindingIgnoreHint(finding) { if (!finding || typeof finding !== 'object') return ''; const rule = normalizeIgnoreRule(finding.antipattern); if (!rule) return ''; const normalizedValue = extractFindingIgnoreValue(finding); if (!normalizedValue) return ''; - const value = extractFindingIgnoreValueRaw(finding); - const valueArg = quoteCommandArg(value); - const reason = quoteCommandArg(`User confirmed ${value} is intentional`); - return `${IMPECCABLE_COMMAND} hooks ignore-value ${rule} ${valueArg} --shared --reason ${reason}`; + const valueArg = quoteCommandArg(extractFindingIgnoreValueRaw(finding)); + return `ignore-value ${rule} ${valueArg}`; } function quoteCommandArg(value) { @@ -1599,31 +1620,69 @@ export function appendDesignSystemNote(text, scanOptions) { return `${text}\n\n${ENVELOPE_PREFIX} DESIGN.md is newer than .impeccable/design.json. Run ${IMPECCABLE_COMMAND} document to refresh the design-system sidecar.`; } +// Session-scoped once-only gate for repeat-prone message parts. Returns true +// the first time a flag is consumed in a session and false after, mirroring +// the `cleanAcked` mechanic: the mtime skew (and the policy footer) do not +// change between edits, so re-stating them on every emission spends context +// to say nothing new. Callers must persist the cache for the flag to stick. +function consumeSessionNoticeFlag(cache, sessionId, flag) { + const session = ensureSession(cache, sessionId); + if (session[flag]) return false; + session[flag] = true; + session.updatedAt = Date.now(); + return true; +} + +// Once-per-session variant of appendDesignSystemNote for the emission paths +// that have cache access. The staleness note names standing project state, +// not new information, so one mention per session is enough. +export function appendDesignSystemNoteOnce(text, scanOptions, cache, sessionId) { + if (!text || !scanOptions?.designSystem?.mdNewerThanJson) return text; + if (!consumeSessionNoticeFlag(cache, sessionId, 'designNoteShown')) return text; + return appendDesignSystemNote(text, scanOptions); +} + +// Full directive footer once per session, the short reminder after. Fresh +// emissions and Cursor denials share the session flag (`footerShown`), so a +// session pays the full policy exactly once however it first fires. +export function footerModeForSession(cache, sessionId) { + return consumeSessionNoticeFlag(cache, sessionId, 'footerShown') ? 'full' : 'short'; +} + +const HOOK_ADMIN_COMMAND = `node ${quoteCommandArg(path.join(__dirname, 'hook-admin.mjs'))}`; + // The directive footer is the part of the hook output that steers model -// behavior. Three intentional moves: -// 1. **Imperative, not advisory.** "Handle these..." beats "Consider -// revising..." which the model treats as a soft suggestion it can -// override when the user asked for any kind of throwaway / demo UI. -// 2. **Explicit judgment clause.** Without it, the model will try to -// "fix" intentional motion, bad fixtures, anti-pattern examples in -// docs, or test cases. Naming the judgment inline beats hoping the -// model infers it from context. -// 3. **Acknowledgement instruction.** Hook output is injected as -// developer-role context, not a chat turn, so the user never sees the -// raw envelope. Asking the model to surface the resolution in its -// reply is the cheapest way to make the feedback loop visible. -function directiveFooter(display, opts = {}) { - // Offer the rule-scoped-to-file form first. `ignore-file` silences every rule - // for the path forever, which is far more than one noisy rule on a real UI - // surface justifies, and it was previously the only option named here. - const target = opts.grouped ? '' : quoteCommandArg(display); - const fileIgnoreGuidance = `run \`${IMPECCABLE_COMMAND} hooks ignore-value "*" --file ${target}\` to scope just that rule to the file, or \`${IMPECCABLE_COMMAND} hooks ignore-file ${target}\` only when the whole file is out of scope for design review (a fixture, a generated artifact, a deliberate demo)`; +// behavior. Intentional moves, in order: +// 1. **Imperative, not advisory.** "Triage each finding..." beats +// "Consider revising...", which the model treats as a soft suggestion. +// 2. **Positive triage branches.** Fix / suppress-and-disclose / ask. The +// suppress branch names the calibration examples (demo, fixture, +// documented bad design, user-confirmed choice) because the agent now +// acts on its own confidence and needs the bar stated. +// 3. **Executable ignore path.** The old footer named only the slash +// command, which an agent reacting to hook output cannot run; the +// hook-admin.mjs invocation is runnable as-is and keeps agents out of +// hand-editing config.json. +// 4. **Honest provenance.** The --reason is the audit trail; "user +// confirmed" appears only when the user actually did. +// 5. **Acknowledgement instruction.** Hook output is injected as +// developer-role context, so the reply is where the user sees the +// resolution, including any ignore the agent persisted. +// 6. **Once per session.** The full policy emits on the session's first +// fire; later emissions carry the one-line short form (mode 'short'). +function directiveFooter(opts = {}) { + if (opts.mode === 'short') { + // No command path here: the session's first emission already gave the + // runnable hook-admin.mjs invocation, and restating ~70 chars of absolute + // path on every repeat is the duplication this mode exists to cut. + return 'Triage per the session policy: fix real problems; persist confident false-positive or sanctioned-exception ignores via `hook-admin.mjs ignore-value` and disclose them in your reply; unsure, ask in one line.'; + } return [ - 'Handle these before finalizing: fix findings that are real design problems, or explicitly classify contextually intentional findings as false positives. Acknowledge what you changed or why you are leaving a finding unchanged.', - '', - 'Use context judgment before editing. A finding is not automatically a defect; literal or domain-appropriate motion, intentional demos or fixtures, documentation of bad design, and user-confirmed choices can be valid as-is.', - '', - `Do not change intentional design just to satisfy the hook, and do not silence a real finding with an inline ignore comment to skip fixing it. Suppress a finding only after the user explicitly confirms it is intentional. Prefer a config ignore (one reviewable place, the commands below); reach for an inline \`impeccable-disable \` comment only when the waiver must travel with a file that leaves the repo, such as an exported or standalone document. Prefer the narrowest persisted exception: run the exact \`${IMPECCABLE_COMMAND} hooks ignore-value ... --shared\` command shown next to a value-specific finding. For \`overused-font\`, use \`ignore-value\` for a specific font and use \`${IMPECCABLE_COMMAND} hooks ignore-rule overused-font --all-values\` only when the user asks to ignore overused fonts generally. For a finding whose line shows no exact ignore-value command, such as \`side-tab\`, ${fileIgnoreGuidance}; use \`${IMPECCABLE_COMMAND} hooks ignore-rule \` only when the user asks to suppress the whole non-value-specific rule. Run ${IMPECCABLE_COMMAND} audit for the full pass.`, + 'Triage each finding, then state in your reply what you fixed, what you suppressed, and what you left standing:', + '- Real design problem: fix it. Keep intentional design as designed.', + `- Confident false positive or sanctioned exception (an intentional demo or fixture, documentation of bad design, literal or domain-appropriate motion, a choice the user confirmed): persist the narrowest ignore yourself and disclose it. Run \`${HOOK_ADMIN_COMMAND} ignore-value "" --reason ""\` with the pair shown on the finding line, or value "*" plus \`--file \` when the line shows none. Write "user confirmed" in a reason only when the user did.`, + '- Unsure: leave it as is and ask the user in one line.', + `Self-serve ends at ignore-value: \`ignore-file\` and \`ignore-rule\` need the user's explicit approval, and never add an ignore to push a blocked write through. Full suppression ladder: ${IMPECCABLE_COMMAND} hooks.`, ].join('\n'); } @@ -1845,20 +1904,18 @@ export async function runHook({ stdinJson, env = {}, cwd = process.cwd(), now = } } - // Persist only when the write is earned: fresh findings justify creating - // `.impeccable/` (dedup and suppression need it), deferred findings do - // too (the Stop deep pass needs the touched-file list to surface them), - // 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 || deferredTotal > 0 - || (cacheDirty && fs.existsSync(path.join(projectCwd, '.impeccable')))) { - persistCache(projectCwd, cache); - } - + // Consuming a session notice flag mutates the cache, so both must happen + // before the persist that makes the flag stick across events. if (freshGroups.length > 0) { const firstGroup = freshGroups[0]; - const text = appendDesignSystemNote(renderGroupedTemplate(freshGroups, config, { cwd: projectCwd }), scanOptions); + const footerMode = footerModeForSession(cache, sessionId); + const text = appendDesignSystemNoteOnce( + renderGroupedTemplate(freshGroups, config, { cwd: projectCwd, footer: footerMode }), + scanOptions, cache, sessionId, + ); + // Fresh findings always earn the cache write, including creating + // `.impeccable/`: dedup, suppression, and the notice flags need it. + persistCache(projectCwd, cache); const allFindings = freshGroups.flatMap((group) => group.findings); return { exitCode: 0, @@ -1881,6 +1938,33 @@ export async function runHook({ stdinJson, env = {}, cwd = process.cwd(), now = }; } + // Resolve the ack emission before the persist below: appendDesignSystem- + // NoteOnce consumes a session flag, and the flag only sticks when the + // write happens after it. Quiet mode emits nothing, so it consumes + // nothing. The clean arm mirrors the branch order further down: pending + // outranks suppression, suppression outranks clean. + let ack = null; + if (!quietMode && pendingWinner && shouldEmitAckForFile(pendingWinner.filePath, config)) { + ack = { + kind: 'pending', + text: appendDesignSystemNoteOnce(renderPendingAck(pendingWinner.filePath, pendingWinner.known, { cwd: projectCwd }), scanOptions, cache, sessionId), + }; + } else if (!quietMode && !suppressionWinner && cleanWinner && !cleanAckDeduped && shouldEmitAckForFile(cleanWinner.filePath, config)) { + ack = { + kind: 'clean', + text: appendDesignSystemNoteOnce(renderCleanAck(cleanWinner.filePath, { cwd: projectCwd }), scanOptions, cache, sessionId), + }; + } + + // Persist only when the write is earned: deferred findings need the + // 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')))) { + persistCache(projectCwd, cache); + } + if (detectorThrewAny && !pendingWinner && !cleanWinner) { return result({ emitted: false, error: 'detector-threw', durationMs: Date.now() - started }); } @@ -1889,8 +1973,8 @@ export async function runHook({ stdinJson, env = {}, cwd = process.cwd(), now = return result({ emitted: false, quiet: true, durationMs: Date.now() - started }); } - if (pendingWinner && shouldEmitAckForFile(pendingWinner.filePath, config)) { - const text = appendDesignSystemNote(renderPendingAck(pendingWinner.filePath, pendingWinner.known, { cwd: projectCwd }), scanOptions); + if (ack?.kind === 'pending') { + const text = ack.text; return { exitCode: 0, stdout: payload(text, 'PostToolUse', harness), @@ -1923,8 +2007,8 @@ export async function runHook({ stdinJson, env = {}, cwd = process.cwd(), now = }; } - if (cleanWinner && !cleanAckDeduped && shouldEmitAckForFile(cleanWinner.filePath, config)) { - const text = appendDesignSystemNote(renderCleanAck(cleanWinner.filePath, { cwd: projectCwd }), scanOptions); + if (ack?.kind === 'clean') { + const text = ack.text; return { exitCode: 0, stdout: payload(text, 'PostToolUse', harness), @@ -2108,11 +2192,17 @@ export async function runStopHook({ stdinJson, env = {}, cwd = process.cwd(), no return result({ emitted: false, skipped: 'stop-clean', durationMs: Date.now() - started }); } - // Fresh findings earn the cache write so the next Stop fire is silent - // unless new issues appear. - persistCache(projectCwd, cache); + // A per-edit fire earlier in this session already consumed the footer + // flag, so the Stop wall of text carries the one-line short footer. + const footerMode = footerModeForSession(cache, sessionId); + const text = appendDesignSystemNoteOnce( + renderGroupedTemplate(freshGroups, config, { cwd: projectCwd, footer: footerMode }), + scanOptions, cache, sessionId, + ); - const text = appendDesignSystemNote(renderGroupedTemplate(freshGroups, config, { cwd: projectCwd }), scanOptions); + // Fresh findings earn the cache write so the next Stop fire is silent + // unless new issues appear; the notice flags ride along. + persistCache(projectCwd, cache); return { exitCode: 0, stdout: payload(text, 'Stop', harness), diff --git a/tests/hook.test.mjs b/tests/hook.test.mjs index 631fc245d..e809c2832 100644 --- a/tests/hook.test.mjs +++ b/tests/hook.test.mjs @@ -1093,53 +1093,83 @@ describe('renderTemplate()', () => { const text = renderTemplate(findings, '/x/Card.tsx', DEFAULT_CONFIG, { cwd: '/x' }); assert.ok(text.startsWith(`${ENVELOPE_PREFIX} Design hook findings requiring review in Card.tsx (12 issue(s)):`)); assert.match(text, /\.\.\. and 7 more \(see \/impeccable audit\)\./); - // Exactly 5 finding lines. - const lines = text.split('\n').filter((l) => l.startsWith('- ')); + // Exactly 5 finding lines. The footer's triage bullets also start with + // "- ", so count only lines carrying a rule id. + const lines = text.split('\n').filter((l) => /^- L\d+ \[/.test(l)); assert.equal(lines.length, 5); assert.ok(text.length <= DEFAULT_CONFIG.limits.maxChars); }); - it('emits a directive footer (imperative + judgment clause + confirmed ignore guidance)', () => { - // Steers the model: imperative "handle", explicit context judgment - // before editing, and "acknowledge" so the user sees the resolution - // in the chat reply. See `directiveFooter()` in hook-lib.mjs for - // the rationale. + it('emits a directive footer (triage branches + executable self-serve ignore + honest provenance)', () => { + // Steers the model: imperative triage into fix / suppress-and-disclose / + // ask, a runnable hook-admin.mjs path for the self-served ignore, and the + // provenance rule for --reason. See `directiveFooter()` in hook-lib.mjs + // for the rationale. const text = renderTemplate( [finding('side-tab', 1, { name: 'X' })], '/x/Card.tsx', DEFAULT_CONFIG, { cwd: '/x' } ); - assert.match(text, /Handle these before finalizing/); - assert.match(text, /fix findings that are real design problems/); - assert.match(text, /classify contextually intentional findings as false positives/); - assert.match(text, /Use context judgment before editing/); - assert.match(text, /not automatically a defect/); + assert.match(text, /Triage each finding/); + assert.match(text, /what you fixed, what you suppressed, and what you left standing/); + assert.match(text, /Real design problem: fix it\. Keep intentional design as designed\./); + assert.match(text, /Confident false positive or sanctioned exception/); assert.match(text, /literal or domain-appropriate motion/); - assert.match(text, /Do not change intentional design just to satisfy the hook/); - assert.match(text, /Suppress a finding only after the user explicitly confirms it is intentional/); - assert.match(text, /do not silence a real finding with an inline ignore comment/); - assert.match(text, /inline `impeccable-disable ` comment only when the waiver must travel with a file/); - assert.match(text, /ignore-value \.\.\. --shared/); - assert.match(text, /ignore-rule overused-font --all-values/); - assert.match(text, /\/impeccable hooks ignore-file Card\.tsx/); - assert.match(text, /ignore-rule /); - assert.match(text, /\/impeccable audit/); + assert.match(text, /persist the narrowest ignore yourself and disclose it/); + assert.match(text, /hook-admin\.mjs" ignore-value "" --reason ""/); + assert.match(text, /Write "user confirmed" in a reason only when the user did/); + assert.match(text, /Unsure: leave it as is and ask the user in one line/); + assert.match(text, /Self-serve ends at ignore-value/); + assert.match(text, /never add an ignore to push a blocked write through/); + assert.match(text, /Full suppression ladder: \/impeccable hooks/); }); - it('shows the exact value-specific command for overused-font findings', () => { + it('renders the one-line short footer when opts.footer is "short"', () => { + const text = renderTemplate( + [finding('side-tab', 1, { name: 'X' })], + '/x/Card.tsx', DEFAULT_CONFIG, { cwd: '/x', footer: 'short' } + ); + assert.match(text, /Triage per the session policy/); + // The short form names the tool without the absolute path; the runnable + // invocation lives only in the session's first (full) footer. + assert.match(text, /`hook-admin\.mjs ignore-value`/); + assert.doesNotMatch(text, /node "/); + assert.match(text, /unsure, ask in one line/); + assert.doesNotMatch(text, /Triage each finding/); + assert.doesNotMatch(text, /Self-serve ends at ignore-value/); + }); + + it('dedupes rule descriptions within one emission, keeping per-line ignore hints', () => { + const desc = 'Long registry description that should appear once.'; + const text = renderTemplate( + [ + finding('overused-font', 2, { name: 'Overused font', description: desc, snippet: 'font-family: "Roboto"' }), + finding('overused-font', 9, { name: 'Overused font', description: desc, snippet: 'font-family: "Inter"' }), + ], + '/x/fonts.css', DEFAULT_CONFIG, { cwd: '/x' } + ); + const occurrences = text.split(desc).length - 1; + assert.equal(occurrences, 1); + // The repeat keeps the rule id, name, and its own value-specific hint. + assert.match(text, /- L9 \[overused-font\] Overused font\. If intentional: `ignore-value overused-font Inter`\./); + assert.match(text, /`ignore-value overused-font Roboto`/); + }); + + it('shows the value-specific ignore hint for overused-font findings', () => { const text = renderTemplate( [finding('overused-font', 1, { name: 'Overused font', snippet: 'body { font-family: "Roboto", sans-serif; }' })], '/x/fonts.css', DEFAULT_CONFIG, { cwd: '/x' } ); - assert.match(text, /\/impeccable hooks ignore-value overused-font Roboto --shared/); - assert.match(text, /ignore-rule overused-font --all-values/); + // The line carries just the rule/value pair; the runnable hook-admin.mjs + // prefix and the --reason contract are stated once in the footer. + assert.match(text, /If intentional: `ignore-value overused-font Roboto`\./); }); - it('shows the exact value-specific command for bounce-easing findings', () => { + it('shows the value-specific ignore hint for bounce-easing findings', () => { const text = renderTemplate( [finding('bounce-easing', 1, { name: 'Bounce or elastic easing', snippet: 'animation: bounce-ball' })], '/x/main.css', DEFAULT_CONFIG, { cwd: '/x' } ); - assert.match(text, /\/impeccable hooks ignore-value bounce-easing bounce-ball --shared/); + assert.match(text, /If intentional: `ignore-value bounce-easing bounce-ball`\./); }); it('drops the L prefix when line is 0', () => { @@ -1573,7 +1603,7 @@ rounded: }); assert.match(withDesign.stdout, /Design hook findings requiring review/); assert.match(withDesign.stdout, /design-system-font/); - assert.match(withDesign.stdout, /ignore-value design-system-font Poppins --shared/); + assert.match(withDesign.stdout, /If intentional: `ignore-value design-system-font Poppins`/); }); it('respects detector.designSystem.enabled=false', async () => { @@ -2157,6 +2187,57 @@ describe('runHook() — the session cache tracks the current scan', () => { }); }); +describe('runHook() — session-scoped notices', () => { + let cwd; + beforeEach(() => { + cwd = mkTmp(); + fs.mkdirSync(path.join(cwd, '.impeccable'), { recursive: true }); + }); + afterEach(() => fs.rmSync(cwd, { recursive: true, force: true })); + + const event = (file, sessionId = 'sid-1') => JSON.stringify({ + session_id: sessionId, cwd, hook_event_name: 'PostToolUse', + tool_name: 'Edit', tool_input: { file_path: file }, + }); + + it('emits the full directive footer once per session, then the short reminder', async () => { + const a = path.join(cwd, 'a.css'); + const b = path.join(cwd, 'b.css'); + fs.writeFileSync(a, 'noop'); + fs.writeFileSync(b, 'noop'); + const det = fakeDetector([finding('tiny-text', 1, { name: 'Tiny text' })]); + + const r1 = await runHook({ stdinJson: event(a), env: {}, cwd, detector: det }); + assert.match(r1.stdout, /Triage each finding/, 'first fresh emission carries the full policy'); + + const r2 = await runHook({ stdinJson: event(b), env: {}, cwd, detector: det }); + assert.match(r2.stdout, /Triage per the session policy/); + assert.doesNotMatch(r2.stdout, /Triage each finding/, 'repeat emissions carry the short reminder'); + + // A new session pays the full policy again. + const r3 = await runHook({ stdinJson: event(a, 'sid-2'), env: {}, cwd, detector: det }); + assert.match(r3.stdout, /Triage each finding/); + }); + + it('mentions the DESIGN.md staleness note once per session', async () => { + const a = path.join(cwd, 'a.css'); + const b = path.join(cwd, 'b.css'); + fs.writeFileSync(a, 'noop'); + fs.writeFileSync(b, 'noop'); + const det = { + ...fakeDetector([finding('tiny-text', 1, { name: 'Tiny text' })]), + loadDesignSystemForCwd: () => ({ present: true, mdNewerThanJson: true }), + }; + + const r1 = await runHook({ stdinJson: event(a), env: {}, cwd, detector: det }); + assert.match(r1.stdout, /DESIGN\.md is newer than \.impeccable\/design\.json/); + + const r2 = await runHook({ stdinJson: event(b), env: {}, cwd, detector: det }); + assert.ok(r2.audit.emitted, 'second file still emits findings'); + assert.doesNotMatch(r2.stdout, /DESIGN\.md is newer/, 'the staleness note does not repeat within a session'); + }); +}); + describe('runHook() — clean-ack noise', () => { let cwd; beforeEach(() => { @@ -2962,7 +3043,7 @@ describe('Cursor hook scripts', () => { assert.equal(payload.permission, 'deny'); assert.match(payload.user_message, /blocked this write/); assert.match(payload.user_message, /side-tab/); - assert.match(payload.agent_message, /Handle these before finalizing/); + assert.match(payload.agent_message, /Triage each finding/); const entries = fs.readFileSync(logPath, 'utf-8').trim().split('\n').map((line) => JSON.parse(line)); assert.equal(entries[0].event, 'preToolUse');