mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-11 21:57:14 +03:00
Denoise the design hook and let agents self-serve confident ignores (#508)
* 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 <cursoragent@cursor.com>
* Preserve the policy footer and honor maxChars under constrained budgets
Greptile's runtime check found two pre-existing clamp gaps that matter
more now that the full policy emits once per session: the last-resort
tail slice cut the footer off an over-budget emission, and the DESIGN.md
staleness note was appended after clamping, pushing past maxChars.
The clamp now gives the footer the budget first, clipping the finding
line and downgrading full to short policy when needed. The staleness
note defers, without consuming its session flag, to a later emission
with room.
AI-assisted (Cursor agent), directed and reviewed by @abdulwahabone.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Harden the constrained-budget clamp: keep findings, honest flags, guaranteed note
Review follow-ups from Bugbot and Greptile on the clamp fix:
- The clamp retries with the short policy before dropping finding lines
that fit beside it, and a grouped result that kept only a file header
no longer counts as a fit.
- The full-footer session flag commits only when the full policy
actually survived the clamp, so a downgraded emission does not mark
the session as having seen a policy it never received.
- Render paths reserve room for a pending DESIGN.md staleness note, so
it is delivered inside the budget on the first emission instead of
deferring behind full emissions indefinitely.
AI-assisted (Cursor agent), directed and reviewed by @abdulwahabone.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Route the Cursor deny cap through the clamp and match the whole footer
Bugbot follow-up: cursorBlockMessage tail-sliced at 4000 chars after
render, which the default 8000-char budget made reachable, and a cut
that spared the footer's opening words still committed the session
flag. The 4000 cap now feeds through the renderer's footer-preserving
clamp, and commitFooterShown matches the complete footer text instead
of a sentinel.
AI-assisted (Cursor agent), directed and reviewed by @abdulwahabone.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Reconcile hook.test.mjs expectations with main's per-platform quoting
Three tests fell out of sync when main's quoteCommandArg change (#533,
building on #476) met this branch's footer/hint rework. Test-only
changes; production logic untouched:
- The full-footer test now accepts either close quote after the
hook-admin.mjs path, since quoteCommandArg single-quotes absolute
paths on POSIX and double-quotes them on Windows. The short-footer
guard rejects `node '` and `node "` alike.
- The #476 hostile-value test asserts the new bare
`ignore-value <rule> '<value>'` hint format. The security property is
unchanged: the value still passes through quoteCommandArg, so
$(touch pwned) stays single-quoted and inert.
- The #533 test previously asserted a concrete quoted `--file` path in
the footer; directiveFooter() now carries only literal placeholders,
so that surface is gone. The per-platform assertion moves to the
per-finding ignore hint, the remaining user-visible surface where
scanned file content flows through quoteCommandArg.
Prepared with AI assistance (Claude Code).
Co-Authored-By: Claude <noreply@anthropic.com>
* Fix two Bugbot findings: Cursor prefix budget and footer-cutting tail slice
Both flagged by Cursor Bugbot on PR #508 after the main merge; both real.
1. cursorBlockMessage computed min(maxChars, 4000 - prefix), so a
configured maxChars at or below the Cursor ceiling never charged the
BLOCK_PREFIX against the budget: the final deny text could exceed
maxChars by the prefix length, and appendDesignSystemNoteOnce's size
check lost exactly the room designNoteReserve had held back. The
prefix now comes off whichever limit binds. Default-config behavior
is unchanged (min(8000, 4000) - 60 equals the old 4000 - 60).
2. The note reservation is subtracted after renderTemplate's 500-char
floor, so the clamp can run below the budget clampLastLine assumed
safe, and its last-resort path tail-sliced the rendered text, cutting
the policy footer (the failure mode this PR exists to eliminate) when
a deep file path met a pending DESIGN.md note. The reservation order
stays (the staleness-note delivery guarantee at floor budgets depends
on it); the last resort now drops the finding line and clips the head
instead, so the footer survives every path. New regression test pins
it: 6 findings, 100-char path, maxChars 500, reserveChars 134.
Prepared with AI assistance (Claude Code).
Co-Authored-By: Claude <noreply@anthropic.com>
* Charge the Cursor deny prefix after the renderer's floor, not before
Greptile's runtime check caught the residual from ead0346d: subtracting
BLOCK_PREFIX from the maxChars passed to renderTemplate does nothing at
floor-tier configs, because the renderer re-raises any budget below its
500-char floor. At maxChars 500 with a stale design sidecar, the
prefixed denial landed at 432 chars and appendDesignSystemNoteOnce
could not fit the staleness note inside 500, deferring it (flag
unconsumed) for every equivalent denial in the session.
The prefix now rides in reserveChars, which comes off after the floor,
so it is charged at every config tier and the final prefixed message
plus a pending note closes exactly at the binding limit (499 chars in
the regression scenario). Default-config output is byte-identical:
max(500, min(8000, 4000)) - prefix equals the old min(8000, 4000) -
prefix. New end-to-end Cursor preToolUse test pins the path with a real
stale sidecar at maxChars 500.
AI-assisted (Cursor agent), directed and reviewed by @abdulwahabone.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Paul Bakaus <paul.bakaus@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Abdul Wahab
co-authored by
Cursor
Paul Bakaus
Claude
Cursor
Paul Bakaus
Claude
parent
251135e190
commit
dc4e4a4bd6
@@ -48,14 +48,20 @@ The first argument is the action. Defaults to `status`.
|
||||
5. If `<action>` 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 `<action>` 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 `"<who decided: evidence>"`; 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 <rule> <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 <id> "*" --file <path>`. Run `npx impeccable detect <path>` first to see what actually fires there.
|
||||
- Reach for `ignore-file <path>` 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 <id>` 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:
|
||||
|
||||
@@ -16,11 +16,15 @@ import path from 'node:path';
|
||||
|
||||
import {
|
||||
ALLOWED_EXTS,
|
||||
DEFAULT_CONFIG,
|
||||
EDIT_COUNT_THRESHOLD,
|
||||
GENERATED_PATH,
|
||||
SENSITIVE_PATH,
|
||||
appendDesignSystemNote,
|
||||
appendDesignSystemNoteOnce,
|
||||
commitFooterShown,
|
||||
designNoteReserve,
|
||||
designSystemOptions,
|
||||
footerModeForSession,
|
||||
filterFindings,
|
||||
isNativePlatform,
|
||||
isScanTargetInsideProject,
|
||||
@@ -345,13 +349,32 @@ async function detectProposedHtml(detector, content, filePath, scanOptions) {
|
||||
}
|
||||
}
|
||||
|
||||
function cursorBlockMessage(findings, filePath, config, cwd) {
|
||||
const rendered = renderTemplate(findings, filePath, config, { cwd });
|
||||
const blocked = rendered.replace(
|
||||
// Cursor caps deny messages around 4000 chars. The cap feeds through the
|
||||
// renderer's clamp, which preserves the policy footer, rather than tail-
|
||||
// slicing the rendered text, which cut the footer off any message the
|
||||
// default 8000-char budget let past 4000.
|
||||
const CURSOR_DENY_LIMIT = 4000;
|
||||
const BLOCK_PREFIX = 'Impeccable design hook blocked this write before it landed. ';
|
||||
|
||||
function cursorBlockMessage(findings, filePath, config, cwd, footerMode, reserveChars) {
|
||||
const limits = config?.limits || DEFAULT_CONFIG.limits;
|
||||
// Charge the prefix via reserveChars, not by subtracting from maxChars:
|
||||
// renderTemplate's 500-char floor re-raises any maxChars pushed below it,
|
||||
// un-charging a prefix subtracted from maxChars (Greptile P1 on PR #508).
|
||||
// reserveChars comes off after the floor, so the prefix is charged at every
|
||||
// config tier and the final prefixed message plus a pending staleness note
|
||||
// fits the binding limit. Default-config output is byte-identical.
|
||||
const budget = Math.min(
|
||||
limits.maxChars || DEFAULT_CONFIG.limits.maxChars,
|
||||
CURSOR_DENY_LIMIT,
|
||||
);
|
||||
const rendered = renderTemplate(findings, filePath,
|
||||
{ ...config, limits: { ...limits, maxChars: budget } },
|
||||
{ cwd, footer: footerMode, reserveChars: (reserveChars || 0) + BLOCK_PREFIX.length });
|
||||
return 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',
|
||||
`[impeccable@1] ${BLOCK_PREFIX}Design hook findings requiring review`,
|
||||
);
|
||||
return blocked.length > 4000 ? `${blocked.slice(0, 3984)}\n...(truncated)` : blocked;
|
||||
}
|
||||
|
||||
function findingSignature(findings) {
|
||||
@@ -468,9 +491,16 @@ 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, designNoteReserve(scanOptions, cache, sessionId)),
|
||||
scanOptions, cache, sessionId, config,
|
||||
);
|
||||
commitFooterShown(cache, sessionId, message);
|
||||
const denial = bumpCursorDenial(cache, sessionId, filePath, filtered);
|
||||
persistCache(cwd, cache);
|
||||
if (denial.count > EDIT_COUNT_THRESHOLD) {
|
||||
|
||||
+265
-87
@@ -22,6 +22,9 @@
|
||||
* 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, config)
|
||||
* designNoteReserve(scanOptions, cache, sessionId)
|
||||
* footerModeForSession(cache, sessionId) / commitFooterShown(cache, sessionId, text)
|
||||
* shouldEmitAckForFile(filePath, config?)
|
||||
* writeAuditLog(env, entry)
|
||||
* loadDetector() -> Promise<{ detectText, detectHtml }>
|
||||
@@ -970,7 +973,13 @@ export function renderTemplate(findings, filePath, config, opts = {}) {
|
||||
if (!Array.isArray(findings) || findings.length === 0) return '';
|
||||
const limits = config?.limits || DEFAULT_CONFIG.limits;
|
||||
const cap = Math.max(1, limits.maxFindings || DEFAULT_CONFIG.limits.maxFindings);
|
||||
const maxChars = Math.max(500, limits.maxChars || DEFAULT_CONFIG.limits.maxChars);
|
||||
// reserveChars holds back room for a note the caller appends after render
|
||||
// (the DESIGN.md staleness note), so the final payload stays inside the
|
||||
// configured budget. It comes off after the 500-char floor, so at floor
|
||||
// configs the note keeps guaranteed delivery room; the clamp budget can
|
||||
// therefore sit below 500, which clampLastLine's footer-preserving
|
||||
// fallback handles (Bugbot on PR #508).
|
||||
const maxChars = Math.max(500, limits.maxChars || DEFAULT_CONFIG.limits.maxChars) - (opts.reserveChars || 0);
|
||||
|
||||
const cwd = opts.cwd || process.cwd();
|
||||
const display = relativize(filePath, cwd);
|
||||
@@ -979,11 +988,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);
|
||||
@@ -1007,12 +1017,15 @@ function renderGroupedTemplate(groups, config, opts = {}) {
|
||||
|
||||
const limits = config?.limits || DEFAULT_CONFIG.limits;
|
||||
const cap = Math.max(1, limits.maxFindings || DEFAULT_CONFIG.limits.maxFindings);
|
||||
const maxChars = Math.max(500, limits.maxChars || DEFAULT_CONFIG.limits.maxChars);
|
||||
const maxChars = Math.max(500, limits.maxChars || DEFAULT_CONFIG.limits.maxChars) - (opts.reserveChars || 0);
|
||||
const cwd = opts.cwd || process.cwd();
|
||||
const total = realGroups.reduce((sum, group) => sum + group.findings.length, 0);
|
||||
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 +1033,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 +1042,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);
|
||||
@@ -1037,76 +1050,131 @@ function renderGroupedTemplate(groups, config, opts = {}) {
|
||||
return text;
|
||||
}
|
||||
|
||||
// The clamp contract, shared by both budget functions: the footer is policy,
|
||||
// not detail, so it survives every clamp. Try the requested footer first;
|
||||
// when it cannot fit even after dropping finding lines, retry with the short
|
||||
// policy rather than sacrifice findings that fit beside it. A result that
|
||||
// dropped every finding line (a grouped render can fit a bare file header)
|
||||
// does not count as a fit: findings are why the emission exists.
|
||||
const isFindingLine = (line) => line.startsWith('- ');
|
||||
|
||||
function footerFallbacks(footer) {
|
||||
const short = directiveFooter({ mode: 'short' });
|
||||
return footer === short ? [footer] : [footer, short];
|
||||
}
|
||||
|
||||
function clampGroupedToBudget(header, lines, footer, maxChars) {
|
||||
const assemble = (linesArr, omitted) => [
|
||||
const assemble = (linesArr, omitted, footerText) => [
|
||||
header,
|
||||
...linesArr,
|
||||
...(omitted ? [`... and more (see ${IMPECCABLE_COMMAND} audit).`] : []),
|
||||
'',
|
||||
footer,
|
||||
footerText,
|
||||
].join('\n');
|
||||
|
||||
let working = lines.slice();
|
||||
let omitted = false;
|
||||
let assembled = assemble(working, omitted);
|
||||
while (assembled.length > maxChars && working.length > 1) {
|
||||
working.pop();
|
||||
omitted = true;
|
||||
assembled = assemble(working, omitted);
|
||||
for (const footerText of footerFallbacks(footer)) {
|
||||
let working = lines.slice();
|
||||
let omitted = false;
|
||||
let assembled = assemble(working, omitted, footerText);
|
||||
while (assembled.length > maxChars && working.length > 1) {
|
||||
working.pop();
|
||||
omitted = true;
|
||||
assembled = assemble(working, omitted, footerText);
|
||||
}
|
||||
if (assembled.length <= maxChars && working.some(isFindingLine)) return assembled;
|
||||
}
|
||||
if (assembled.length > maxChars) {
|
||||
assembled = `${assembled.slice(0, maxChars - 1)}…`;
|
||||
}
|
||||
return assembled;
|
||||
return clampLastLine((linesArr, footerText) => assemble(linesArr, true, footerText),
|
||||
lines.find(isFindingLine) || lines[0], maxChars);
|
||||
}
|
||||
|
||||
function clampToBudget(header, lines, more, footer, maxChars) {
|
||||
const assemble = (linesArr, moreText) => {
|
||||
const assemble = (linesArr, moreText, footerText) => {
|
||||
const blocks = [header, ...linesArr];
|
||||
if (moreText) blocks.push(moreText);
|
||||
blocks.push('');
|
||||
blocks.push(footer);
|
||||
blocks.push(footerText);
|
||||
return blocks.join('\n');
|
||||
};
|
||||
|
||||
let working = lines.slice();
|
||||
let moreText = more;
|
||||
let assembled = assemble(working, moreText);
|
||||
while (assembled.length > maxChars && working.length > 1) {
|
||||
working.pop();
|
||||
moreText = `... and more (see ${IMPECCABLE_COMMAND} audit).`;
|
||||
assembled = assemble(working, moreText);
|
||||
let lastMore = more;
|
||||
for (const footerText of footerFallbacks(footer)) {
|
||||
let working = lines.slice();
|
||||
let moreText = more;
|
||||
let assembled = assemble(working, moreText, footerText);
|
||||
while (assembled.length > maxChars && working.length > 1) {
|
||||
working.pop();
|
||||
moreText = `... and more (see ${IMPECCABLE_COMMAND} audit).`;
|
||||
assembled = assemble(working, moreText, footerText);
|
||||
}
|
||||
lastMore = moreText;
|
||||
if (assembled.length <= maxChars) return assembled;
|
||||
}
|
||||
if (assembled.length > maxChars) {
|
||||
assembled = `${assembled.slice(0, maxChars - 1)}…`;
|
||||
}
|
||||
return assembled;
|
||||
return clampLastLine((linesArr, footerText) => assemble(linesArr, lastMore, footerText),
|
||||
lines.find(isFindingLine) || lines[0], maxChars);
|
||||
}
|
||||
|
||||
function formatFindingLine(f) {
|
||||
// Last resort with one finding line left: the short policy gets the budget
|
||||
// first, the line is clipped to what remains. The pre-fix tail-slice cut
|
||||
// whatever happened to be last, which was always the footer.
|
||||
function clampLastLine(build, line, maxChars) {
|
||||
const footerText = directiveFooter({ mode: 'short' });
|
||||
const bare = build([], footerText);
|
||||
// +1 for the newline the line itself brings when it joins the blocks.
|
||||
const room = maxChars - bare.length - 1;
|
||||
if (room >= 24) {
|
||||
const clipped = line.length > room ? `${line.slice(0, room - 1)}…` : line;
|
||||
return build([clipped], footerText);
|
||||
}
|
||||
// No room for even a clipped finding line: the note reservation can pull
|
||||
// the budget below the 500-char floor, and a deep file path can push the
|
||||
// header past what remains beside the short policy (Bugbot on PR #508).
|
||||
// Drop the line, and if the bare header + policy still overflow, clip the
|
||||
// head. Never tail-slice: the footer sits at the end, so a tail slice is
|
||||
// exactly the footer cut this renderer exists to prevent.
|
||||
if (bare.length <= maxChars) return bare;
|
||||
const head = bare.slice(0, Math.max(0, maxChars - footerText.length - 4));
|
||||
return `${head}…\n\n${footerText}`;
|
||||
}
|
||||
|
||||
// `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) {
|
||||
@@ -1606,36 +1674,105 @@ export function designSystemOptions(config, detector, projectCwd) {
|
||||
}
|
||||
}
|
||||
|
||||
const DESIGN_STALE_NOTE = `${ENVELOPE_PREFIX} DESIGN.md is newer than .impeccable/design.json. Run ${IMPECCABLE_COMMAND} document to refresh the design-system sidecar.`;
|
||||
|
||||
export function appendDesignSystemNote(text, scanOptions) {
|
||||
if (!text || !scanOptions?.designSystem?.mdNewerThanJson) return text;
|
||||
return `${text}\n\n${ENVELOPE_PREFIX} DESIGN.md is newer than .impeccable/design.json. Run ${IMPECCABLE_COMMAND} document to refresh the design-system sidecar.`;
|
||||
return `${text}\n\n${DESIGN_STALE_NOTE}`;
|
||||
}
|
||||
|
||||
// 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. The note is
|
||||
// appended after the renderer has clamped to the configured budget: render
|
||||
// paths reserve room for it via designNoteReserve, and the size check here
|
||||
// is the safety net for the ack paths, deferring (without consuming the
|
||||
// flag) to a later emission rather than busting maxChars.
|
||||
export function appendDesignSystemNoteOnce(text, scanOptions, cache, sessionId, config) {
|
||||
if (!text || !scanOptions?.designSystem?.mdNewerThanJson) return text;
|
||||
const maxChars = Math.max(500, config?.limits?.maxChars || DEFAULT_CONFIG.limits.maxChars);
|
||||
if (text.length + DESIGN_STALE_NOTE.length + 2 > maxChars) return text;
|
||||
if (!consumeSessionNoticeFlag(cache, sessionId, 'designNoteShown')) return text;
|
||||
return appendDesignSystemNote(text, scanOptions);
|
||||
}
|
||||
|
||||
// Render-time reservation for the note above: how many characters the
|
||||
// renderer must hold back so a pending staleness note still fits inside the
|
||||
// configured budget. Zero once the session has seen the note. Without the
|
||||
// reservation, a session whose every emission fills the budget would defer
|
||||
// the note forever.
|
||||
export function designNoteReserve(scanOptions, cache, sessionId) {
|
||||
if (!scanOptions?.designSystem?.mdNewerThanJson) return 0;
|
||||
if (ensureSession(cache, sessionId).designNoteShown) return 0;
|
||||
return DESIGN_STALE_NOTE.length + 2;
|
||||
}
|
||||
|
||||
// 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. The mode
|
||||
// is a peek: the clamp can downgrade a requested full footer under a tight
|
||||
// budget, so the flag commits only when the complete full policy actually
|
||||
// reached the output. Matching the whole footer text (not a sentinel) keeps
|
||||
// the flag honest against any truncation that spares the opening words.
|
||||
export function footerModeForSession(cache, sessionId) {
|
||||
return ensureSession(cache, sessionId).footerShown ? 'short' : 'full';
|
||||
}
|
||||
|
||||
export function commitFooterShown(cache, sessionId, text) {
|
||||
if (!text || !text.includes(directiveFooter())) return;
|
||||
const session = ensureSession(cache, sessionId);
|
||||
if (session.footerShown) return;
|
||||
session.footerShown = true;
|
||||
session.updatedAt = Date.now();
|
||||
}
|
||||
|
||||
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 ? '<path>' : quoteCommandArg(display);
|
||||
const fileIgnoreGuidance = `run \`${IMPECCABLE_COMMAND} hooks ignore-value <id> "*" --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 <rule>\` 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 <id>\` 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 <rule> "<value>" --reason "<who decided: evidence>"\` with the pair shown on the finding line, or value "*" plus \`--file <path>\` 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');
|
||||
}
|
||||
|
||||
@@ -1857,20 +1994,23 @@ 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);
|
||||
}
|
||||
|
||||
// The session notice flags mutate the cache, so they must settle before
|
||||
// the persist that makes them 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,
|
||||
reserveChars: designNoteReserve(scanOptions, cache, sessionId),
|
||||
}),
|
||||
scanOptions, cache, sessionId, config,
|
||||
);
|
||||
commitFooterShown(cache, sessionId, text);
|
||||
// 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,
|
||||
@@ -1893,6 +2033,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, config),
|
||||
};
|
||||
} else if (!quietMode && !suppressionWinner && cleanWinner && !cleanAckDeduped && shouldEmitAckForFile(cleanWinner.filePath, config)) {
|
||||
ack = {
|
||||
kind: 'clean',
|
||||
text: appendDesignSystemNoteOnce(renderCleanAck(cleanWinner.filePath, { cwd: projectCwd }), scanOptions, cache, sessionId, config),
|
||||
};
|
||||
}
|
||||
|
||||
// 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 });
|
||||
}
|
||||
@@ -1901,8 +2068,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),
|
||||
@@ -1935,8 +2102,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),
|
||||
@@ -2120,11 +2287,22 @@ 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,
|
||||
reserveChars: designNoteReserve(scanOptions, cache, sessionId),
|
||||
}),
|
||||
scanOptions, cache, sessionId, config,
|
||||
);
|
||||
commitFooterShown(cache, sessionId, text);
|
||||
|
||||
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),
|
||||
|
||||
+328
-41
@@ -50,6 +50,7 @@ import {
|
||||
coLocatedStylesheets,
|
||||
runHook,
|
||||
runStopHook,
|
||||
commitFooterShown,
|
||||
IMMEDIATE_TIER_RULES,
|
||||
splitFindingsByTier,
|
||||
perEditTieringActive,
|
||||
@@ -1093,59 +1094,96 @@ 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 <rule>` 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 <id>/);
|
||||
assert.match(text, /\/impeccable audit/);
|
||||
assert.match(text, /persist the narrowest ignore yourself and disclose it/);
|
||||
// quoteCommandArg quotes the hook-admin.mjs path per platform (single
|
||||
// quotes on POSIX, double on Windows; #533), so match either close quote.
|
||||
assert.match(text, /hook-admin\.mjs['"] ignore-value <rule> "<value>" --reason "<who decided: evidence>"/);
|
||||
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. The quoted
|
||||
// path would render as `node '...'` on POSIX or `node "..."` on Windows,
|
||||
// so reject both.
|
||||
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('single-quotes a hostile font value so the suggestion cannot inject a shell command (#476)', () => {
|
||||
// The suggested command comes straight from scanned file content. A
|
||||
// double-quoted arg would leave $(...) live for whoever runs the
|
||||
// suggestion; single quotes neutralize it.
|
||||
// The suggested pair comes straight from scanned file content. A
|
||||
// double-quoted arg would leave $(...) live for whoever pastes it into
|
||||
// the footer's hook-admin.mjs command; single quotes neutralize it. The
|
||||
// hint format is now the bare `ignore-value <rule> '<value>'` pair (the
|
||||
// runnable command prefix, --shared/--reason contract live in the
|
||||
// footer), but the value still goes through quoteCommandArg.
|
||||
const text = renderTemplate(
|
||||
[finding('overused-font', 1, {
|
||||
name: 'Overused font',
|
||||
@@ -1153,29 +1191,55 @@ describe('renderTemplate()', () => {
|
||||
})],
|
||||
'/x/fonts.css', DEFAULT_CONFIG, { cwd: '/x' }
|
||||
);
|
||||
assert.match(text, /ignore-value overused-font '\$\(touch pwned\)' --shared/);
|
||||
assert.match(text, /ignore-value overused-font '\$\(touch pwned\)'/);
|
||||
assert.doesNotMatch(text, /ignore-value overused-font "\$\(touch pwned\)"/);
|
||||
});
|
||||
|
||||
it('quotes the --file path per platform: single quotes on POSIX, double quotes on Windows (#533)', () => {
|
||||
// The suggested command is run on the same machine the hook fired on.
|
||||
// POSIX needs single quotes so $(...) in a filename cannot execute; Windows
|
||||
// cmd.exe treats single quotes as literal, so a path with spaces must stay
|
||||
// double-quoted or the ignore scope is split at the space.
|
||||
it('quotes a hint value per platform: single quotes on POSIX, double quotes on Windows (#533)', () => {
|
||||
// #533 originally targeted the footer's concrete `--file <path>`
|
||||
// suggestion; directiveFooter() now carries only literal placeholders
|
||||
// (`--file <path>`), so that surface is gone. The quoting-sensitive
|
||||
// surface that remains user-visible is the per-finding ignore hint,
|
||||
// whose value comes straight from scanned file content and is meant to
|
||||
// be pasted into the footer's command on this same machine. POSIX needs
|
||||
// single quotes so $(...) cannot execute; Windows cmd.exe treats single
|
||||
// quotes as literal, so a value with spaces must stay double-quoted or
|
||||
// the ignore scope is split at the space.
|
||||
const original = process.platform;
|
||||
const renderFor = (platform) => {
|
||||
Object.defineProperty(process, 'platform', { value: platform, configurable: true });
|
||||
try {
|
||||
return renderTemplate(
|
||||
[finding('side-tab', 1, { name: 'Side tab' })],
|
||||
'/x/My Components/Card.tsx', DEFAULT_CONFIG, { cwd: '/x' }
|
||||
[finding('overused-font', 1, {
|
||||
name: 'Overused font',
|
||||
snippet: 'h1 { font-family: "Space Grotesk Var", sans-serif; }',
|
||||
})],
|
||||
'/x/fonts.css', DEFAULT_CONFIG, { cwd: '/x' }
|
||||
);
|
||||
} finally {
|
||||
Object.defineProperty(process, 'platform', { value: original, configurable: true });
|
||||
}
|
||||
};
|
||||
assert.match(renderFor('linux'), /--file 'My Components\/Card\.tsx'/);
|
||||
assert.match(renderFor('win32'), /--file "My Components\/Card\.tsx"/);
|
||||
assert.match(renderFor('linux'), /ignore-value overused-font 'Space Grotesk Var'/);
|
||||
assert.match(renderFor('win32'), /ignore-value overused-font "Space Grotesk Var"/);
|
||||
});
|
||||
|
||||
it('keeps the policy footer when reserveChars presses against the 500-char floor', () => {
|
||||
// Bugbot on PR #508: the note reservation used to be subtracted after
|
||||
// the 500-char floor, so the clamp could run at ~366 chars, below the
|
||||
// budget clampLastLine assumes safe, and the hard tail slice cut the
|
||||
// policy footer. The reserve now comes off before the floor; when the
|
||||
// floor wins, the note defers instead.
|
||||
const config = { ...DEFAULT_CONFIG, limits: { ...DEFAULT_CONFIG.limits, maxChars: 500 } };
|
||||
const longPath = `/x/${'deeply-nested/'.repeat(6)}Component.tsx`;
|
||||
const text = renderTemplate(
|
||||
Array.from({ length: 6 }, (_, i) =>
|
||||
finding('side-tab', i + 1, { name: 'Side tab', description: 'Colored side border.' })),
|
||||
longPath, config, { cwd: '/x', reserveChars: 134 }
|
||||
);
|
||||
assert.ok(text.length <= 500, `stays inside the floored budget (got ${text.length})`);
|
||||
assert.match(text, /unsure, ask in one line\.$/);
|
||||
assert.doesNotMatch(text, /…$/);
|
||||
});
|
||||
|
||||
it('drops the L<line> prefix when line is 0', () => {
|
||||
@@ -1205,6 +1269,32 @@ describe('renderTemplate()', () => {
|
||||
{ cwd: '/x' });
|
||||
assert.ok(text.length <= 500);
|
||||
});
|
||||
|
||||
it('keeps a policy footer when the clamp cuts down to one finding line', () => {
|
||||
// At the minimum budget the full footer cannot fit beside a long finding,
|
||||
// so the clamp clips the finding line and downgrades to the short policy
|
||||
// instead of slicing the footer off the tail.
|
||||
const huge = [finding('side-tab', 1, { name: 'X', description: 'y'.repeat(2000) })];
|
||||
const text = renderTemplate(huge, '/x/a.tsx',
|
||||
{ ...DEFAULT_CONFIG, limits: { maxFindings: 5, maxChars: 500 } },
|
||||
{ cwd: '/x' });
|
||||
assert.ok(text.length <= 500);
|
||||
assert.match(text, /\[side-tab\]/, 'the finding is still identified');
|
||||
assert.match(text, /Triage per the session policy/, 'a clamped emission still carries the policy');
|
||||
});
|
||||
|
||||
it('keeps findings that fit beside the short policy instead of dropping them for the full one', () => {
|
||||
const findings = [1, 2, 3].map((line) =>
|
||||
finding('side-tab', line, { name: 'X', description: 'short issue' }));
|
||||
const text = renderTemplate(findings, '/x/a.tsx',
|
||||
{ ...DEFAULT_CONFIG, limits: { maxFindings: 5, maxChars: 500 } },
|
||||
{ cwd: '/x' });
|
||||
assert.ok(text.length <= 500);
|
||||
assert.match(text, /- L1 /);
|
||||
assert.match(text, /- L2 /);
|
||||
assert.match(text, /- L3 /, 'all findings survive; the clamp must not drop lines chasing the full policy');
|
||||
assert.match(text, /Triage per the session policy/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('writeAuditLog()', () => {
|
||||
@@ -1609,7 +1699,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 () => {
|
||||
@@ -2193,6 +2283,116 @@ 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');
|
||||
});
|
||||
|
||||
it('delivers the staleness note inside the budget on a full first emission', async () => {
|
||||
fs.writeFileSync(getConfigPath(cwd), JSON.stringify({
|
||||
hook: { limits: { maxChars: 500 } },
|
||||
}));
|
||||
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', description: 'y'.repeat(600) })]),
|
||||
loadDesignSystemForCwd: () => ({ present: true, mdNewerThanJson: true }),
|
||||
};
|
||||
|
||||
// A finding this long would fill the whole budget; the renderer must
|
||||
// reserve room so the note still lands without busting maxChars.
|
||||
const r1 = await runHook({ stdinJson: event(a), env: {}, cwd, detector: det });
|
||||
const ctx1 = JSON.parse(r1.stdout).hookSpecificOutput.additionalContext;
|
||||
assert.ok(ctx1.length <= 500, `final emission honors maxChars (got ${ctx1.length})`);
|
||||
assert.match(ctx1, /DESIGN\.md is newer/, 'the note is delivered on the first emission, not deferred past it');
|
||||
|
||||
const r2 = await runHook({ stdinJson: event(b), env: {}, cwd, detector: det });
|
||||
const ctx2 = JSON.parse(r2.stdout).hookSpecificOutput.additionalContext;
|
||||
assert.doesNotMatch(ctx2, /DESIGN\.md is newer/, 'one mention per session');
|
||||
});
|
||||
|
||||
it('keeps the full-footer flag unspent when the clamp downgrades the footer', async () => {
|
||||
fs.writeFileSync(getConfigPath(cwd), JSON.stringify({
|
||||
hook: { limits: { maxChars: 500 } },
|
||||
}));
|
||||
const a = path.join(cwd, 'a.css');
|
||||
fs.writeFileSync(a, 'noop');
|
||||
const det = fakeDetector([finding('tiny-text', 1, { name: 'Tiny text', description: 'y'.repeat(600) })]);
|
||||
|
||||
// 500 chars cannot hold the full policy, so the emission carries the
|
||||
// short form. The session must not be marked as having seen the full
|
||||
// footer it never received.
|
||||
const r1 = await runHook({ stdinJson: event(a), env: {}, cwd, detector: det });
|
||||
assert.match(r1.stdout, /Triage per the session policy/);
|
||||
assert.doesNotMatch(r1.stdout, /Triage each finding/);
|
||||
const cache = readCache(cwd);
|
||||
assert.ok(!cache.sessions['sid-1'].footerShown, 'a downgraded footer does not spend the session flag');
|
||||
});
|
||||
|
||||
it('commits the footer flag only for the complete full policy, not its opening words', () => {
|
||||
const cache = { version: 1, sessions: {} };
|
||||
const full = renderTemplate(
|
||||
[finding('tiny-text', 1, { name: 'Tiny text' })],
|
||||
'/x/a.css', DEFAULT_CONFIG, { cwd: '/x' },
|
||||
);
|
||||
|
||||
// A tail truncation can spare "Triage each finding" while cutting the
|
||||
// policy body. That must not count as delivered.
|
||||
commitFooterShown(cache, 'sid-1', full.slice(0, full.length - 40));
|
||||
assert.ok(!cache.sessions['sid-1']?.footerShown, 'a truncated policy must not spend the flag');
|
||||
|
||||
commitFooterShown(cache, 'sid-1', full);
|
||||
assert.ok(cache.sessions['sid-1'].footerShown, 'the intact policy commits the flag');
|
||||
});
|
||||
});
|
||||
|
||||
describe('runHook() — clean-ack noise', () => {
|
||||
let cwd;
|
||||
beforeEach(() => {
|
||||
@@ -2998,7 +3198,9 @@ 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/);
|
||||
assert.match(payload.agent_message, /Full suppression ladder/, 'the deny message carries the complete policy, not a truncated head');
|
||||
assert.ok(payload.agent_message.length <= 4000, 'the deny message respects the Cursor cap');
|
||||
|
||||
const entries = fs.readFileSync(logPath, 'utf-8').trim().split('\n').map((line) => JSON.parse(line));
|
||||
assert.equal(entries[0].event, 'preToolUse');
|
||||
@@ -3006,6 +3208,71 @@ describe('Cursor hook scripts', () => {
|
||||
assert.equal(entries[0].blockedFindings, 1);
|
||||
});
|
||||
|
||||
it('preToolUse delivers the stale-sidecar note within a 500-char budget (PR #508)', () => {
|
||||
fs.mkdirSync(path.join(cwd, '.impeccable'), { recursive: true });
|
||||
fs.writeFileSync(getConfigPath(cwd), JSON.stringify({
|
||||
hook: { limits: { maxChars: 500 } },
|
||||
}));
|
||||
|
||||
const designMd = path.join(cwd, 'DESIGN.md');
|
||||
const sidecarPath = path.join(cwd, '.impeccable', 'design.json');
|
||||
fs.writeFileSync(designMd, `---
|
||||
typography:
|
||||
body:
|
||||
fontFamily: "IBM Plex Sans, Arial, sans-serif"
|
||||
colors:
|
||||
ink: "#241f1a"
|
||||
rounded:
|
||||
"2xl": "80px"
|
||||
---
|
||||
|
||||
# Design System
|
||||
`);
|
||||
fs.writeFileSync(sidecarPath, JSON.stringify({
|
||||
extensions: {
|
||||
colorMeta: {
|
||||
accent: {
|
||||
canonical: '#b8422e',
|
||||
tonalRamp: ['#d55a42'],
|
||||
},
|
||||
},
|
||||
roundedMeta: {
|
||||
lg: { canonical: '24px' },
|
||||
},
|
||||
},
|
||||
}));
|
||||
const past = new Date(Date.now() - 10000);
|
||||
fs.utimesSync(sidecarPath, past, past);
|
||||
|
||||
const filePath = path.join(cwd, 'src/Card.html');
|
||||
const out = execFileSync(process.execPath, [path.join('skill', 'scripts', 'hook-before-edit.mjs')], {
|
||||
cwd: path.resolve('.'),
|
||||
input: JSON.stringify({
|
||||
hook_event_name: 'preToolUse',
|
||||
session_id: 'sid-508',
|
||||
cwd,
|
||||
tool_name: 'Write',
|
||||
tool_input: {
|
||||
file_path: filePath,
|
||||
content: `
|
||||
<style>
|
||||
.card { border-left: 4px solid #7c3aed; border-radius: 16px; }
|
||||
</style>
|
||||
<div class="card">Hello</div>
|
||||
`,
|
||||
},
|
||||
}),
|
||||
env: { ...process.env, IMPECCABLE_HOOK_LOG: '' },
|
||||
encoding: 'utf-8',
|
||||
});
|
||||
|
||||
const payload = JSON.parse(out);
|
||||
assert.equal(payload.permission, 'deny');
|
||||
assert.match(payload.agent_message, /DESIGN\.md is newer/);
|
||||
assert.ok(payload.agent_message.length <= 500, `deny message length ${payload.agent_message.length} exceeds 500-char budget`);
|
||||
assert.equal(readCache(cwd).sessions['sid-508'].designNoteShown, true);
|
||||
});
|
||||
|
||||
it('preToolUse allows writes with findings when the project platform is native', () => {
|
||||
// Same slop content the deny test blocks, but the project declares a
|
||||
// native platform, so the web rule engine must stand aside.
|
||||
@@ -3546,6 +3813,26 @@ describe('runStopHook()', () => {
|
||||
assert.equal(stop.emission.kind, 'stop-deep-pass');
|
||||
});
|
||||
|
||||
it('keeps a policy footer when the grouped Stop render is clamped to the minimum budget', async () => {
|
||||
const sid = 'stop-clamp';
|
||||
fs.mkdirSync(path.join(cwd, '.impeccable'), { recursive: true });
|
||||
fs.writeFileSync(getConfigPath(cwd), JSON.stringify({ hook: { limits: { maxChars: 500 } } }));
|
||||
const a = write('src/A.tsx', 'noop');
|
||||
const b = write('src/B.tsx', 'noop');
|
||||
const det = fakeDetector([finding('side-tab', 1, { name: 'X', description: 'y'.repeat(2000) })]);
|
||||
|
||||
// Two touched files with deferred findings so the Stop pass groups them.
|
||||
await runHook({ stdinJson: JSON.stringify(editEvent(a, sid)), env: {}, cwd, detector: det });
|
||||
await runHook({ stdinJson: JSON.stringify(editEvent(b, sid)), env: {}, cwd, detector: det });
|
||||
|
||||
const stop = await runStopHook({ stdinJson: JSON.stringify(stopEvent(sid)), env: {}, cwd, detector: det });
|
||||
assert.equal(stop.audit.emitted, true);
|
||||
const ctx = JSON.parse(stop.stdout).hookSpecificOutput.additionalContext;
|
||||
assert.ok(ctx.length <= 500, `grouped emission honors maxChars (got ${ctx.length})`);
|
||||
assert.match(ctx, /Triage per the session policy/, 'a clamped grouped emission still carries the policy');
|
||||
assert.match(ctx, /\[side-tab\]/, 'a clamped grouped emission keeps finding detail, not just a file header');
|
||||
});
|
||||
|
||||
it('exits silent and fast when the session touched no UI files', async () => {
|
||||
const r = await runStopHook({ stdinJson: JSON.stringify(stopEvent('stop-untouched')), env: {}, cwd });
|
||||
assert.equal(r.exitCode, 0);
|
||||
|
||||
Reference in New Issue
Block a user