diff --git a/skill/scripts/hook-before-edit.mjs b/skill/scripts/hook-before-edit.mjs index 9398df93b..af003105a 100644 --- a/skill/scripts/hook-before-edit.mjs +++ b/skill/scripts/hook-before-edit.mjs @@ -476,7 +476,7 @@ async function main() { const footerMode = footerModeForSession(cache, sessionId); const message = appendDesignSystemNoteOnce( cursorBlockMessage(filtered, filePath, config, cwd, footerMode), - scanOptions, cache, sessionId, + scanOptions, cache, sessionId, config, ); const denial = bumpCursorDenial(cache, sessionId, filePath, filtered); persistCache(cwd, cache); diff --git a/skill/scripts/hook-lib.mjs b/skill/scripts/hook-lib.mjs index 3d034d063..4372e4eac 100644 --- a/skill/scripts/hook-lib.mjs +++ b/skill/scripts/hook-lib.mjs @@ -22,7 +22,7 @@ * 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) + * appendDesignSystemNote(text, scanOptions) / appendDesignSystemNoteOnce(text, scanOptions, cache, sessionId, config) * footerModeForSession(cache, sessionId) * shouldEmitAckForFile(filePath, config?) * writeAuditLog(env, entry) @@ -1044,49 +1044,68 @@ function renderGroupedTemplate(groups, config, opts = {}) { } 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); + let assembled = assemble(working, omitted, footer); while (assembled.length > maxChars && working.length > 1) { working.pop(); omitted = true; - assembled = assemble(working, omitted); + assembled = assemble(working, omitted, footer); } - if (assembled.length > maxChars) { - assembled = `${assembled.slice(0, maxChars - 1)}…`; - } - return assembled; + if (assembled.length <= maxChars) return assembled; + return clampLastLine((linesArr, footerText) => assemble(linesArr, true, footerText), working[0], footer, 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); + let assembled = assemble(working, moreText, footer); while (assembled.length > maxChars && working.length > 1) { working.pop(); moreText = `... and more (see ${IMPECCABLE_COMMAND} audit).`; - assembled = assemble(working, moreText); + assembled = assemble(working, moreText, footer); } - if (assembled.length > maxChars) { - assembled = `${assembled.slice(0, maxChars - 1)}…`; + if (assembled.length <= maxChars) return assembled; + return clampLastLine((linesArr, footerText) => assemble(linesArr, moreText, footerText), working[0], footer, maxChars); +} + +// Last resort when a single finding line still busts the budget. The footer +// is policy, not detail, so it survives every clamp: give it the budget +// first, clip the finding line to what remains, and when the full policy is +// itself what does not fit, downgrade to the short form rather than emit +// findings with no policy at all. The old tail-slice cut whatever happened +// to be last, which was always the footer. +function clampLastLine(build, line, footer, maxChars) { + const footerCandidates = footer === directiveFooter({ mode: 'short' }) + ? [footer] + : [footer, directiveFooter({ mode: 'short' })]; + for (const footerText of footerCandidates) { + // +1 for the newline the line itself brings when it joins the blocks. + const room = maxChars - build([], footerText).length - 1; + if (room >= 24) { + const clipped = line.length > room ? `${line.slice(0, room - 1)}…` : line; + return build([clipped], footerText); + } } - return assembled; + // maxChars is floored at 500 and header + short footer fit well inside + // that, so this is unreachable; keep the hard slice as the safety net. + return `${build([line], footer).slice(0, maxChars - 1)}…`; } // `compact` drops the registry description: within one emission the first @@ -1615,9 +1634,11 @@ 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 @@ -1635,9 +1656,14 @@ function consumeSessionNoticeFlag(cache, sessionId, flag) { // 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) { +// not new information, so one mention per session is enough. The note is +// appended after the renderer has clamped to the configured budget, so when +// it would push the emission past maxChars, defer it (without consuming the +// flag) to a later, smaller emission in the session. +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); } @@ -1911,7 +1937,7 @@ export async function runHook({ stdinJson, env = {}, cwd = process.cwd(), now = const footerMode = footerModeForSession(cache, sessionId); const text = appendDesignSystemNoteOnce( renderGroupedTemplate(freshGroups, config, { cwd: projectCwd, footer: footerMode }), - scanOptions, cache, sessionId, + scanOptions, cache, sessionId, config, ); // Fresh findings always earn the cache write, including creating // `.impeccable/`: dedup, suppression, and the notice flags need it. @@ -1947,12 +1973,12 @@ export async function runHook({ stdinJson, env = {}, cwd = process.cwd(), now = if (!quietMode && pendingWinner && shouldEmitAckForFile(pendingWinner.filePath, config)) { ack = { kind: 'pending', - text: appendDesignSystemNoteOnce(renderPendingAck(pendingWinner.filePath, pendingWinner.known, { cwd: projectCwd }), scanOptions, cache, sessionId), + 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), + text: appendDesignSystemNoteOnce(renderCleanAck(cleanWinner.filePath, { cwd: projectCwd }), scanOptions, cache, sessionId, config), }; } @@ -2197,7 +2223,7 @@ export async function runStopHook({ stdinJson, env = {}, cwd = process.cwd(), no const footerMode = footerModeForSession(cache, sessionId); const text = appendDesignSystemNoteOnce( renderGroupedTemplate(freshGroups, config, { cwd: projectCwd, footer: footerMode }), - scanOptions, cache, sessionId, + scanOptions, cache, sessionId, config, ); // Fresh findings earn the cache write so the next Stop fire is silent diff --git a/tests/hook.test.mjs b/tests/hook.test.mjs index e809c2832..48c9096c0 100644 --- a/tests/hook.test.mjs +++ b/tests/hook.test.mjs @@ -1199,6 +1199,20 @@ 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'); + }); + }); describe('writeAuditLog()', () => { @@ -2236,6 +2250,32 @@ describe('runHook() — session-scoped notices', () => { 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('defers the staleness note past an emission it would push over budget', async () => { + fs.writeFileSync(getConfigPath(cwd), JSON.stringify({ + hook: { limits: { maxChars: 500 } }, + })); + const a = path.join(cwd, 'a.css'); + fs.writeFileSync(a, 'noop'); + let current = [finding('tiny-text', 1, { name: 'Tiny text', description: 'y'.repeat(600) })]; + const det = { + detectText: () => current.slice(), + detectHtml: () => current.slice(), + loadDesignSystemForCwd: () => ({ present: true, mdNewerThanJson: true }), + }; + + // The fresh emission fills the whole budget; appending the note here + // would bust maxChars, so it must wait without burning the session flag. + 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.doesNotMatch(ctx1, /DESIGN\.md is newer/); + + // The next emission is a small clean ack with room to spare. + current = []; + const r2 = await runHook({ stdinJson: event(a), env: {}, cwd, detector: det }); + assert.match(r2.stdout, /DESIGN\.md is newer/, 'the deferred note lands on the next emission with room'); + }); }); describe('runHook() — clean-ack noise', () => { @@ -3591,6 +3631,25 @@ 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'); + }); + 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);