From 31027df81bf77c6d3edf36e68050a4bbd8e5bfa8 Mon Sep 17 00:00:00 2001 From: Abdul Wahab Date: Thu, 6 Aug 2026 15:10:51 +0500 Subject: [PATCH] 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 --- skill/scripts/hook-before-edit.mjs | 2 +- skill/scripts/hook-lib.mjs | 72 ++++++++++++++++++++---------- tests/hook.test.mjs | 59 ++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 24 deletions(-) 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);