diff --git a/skill/scripts/hook-before-edit.mjs b/skill/scripts/hook-before-edit.mjs index 5b8b13087..acf8dec52 100644 --- a/skill/scripts/hook-before-edit.mjs +++ b/skill/scripts/hook-before-edit.mjs @@ -358,10 +358,16 @@ 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; + // Subtract the prefix from whichever limit binds, not just the Cursor + // ceiling: with a configured maxChars at or below the ceiling, the old + // `min(maxChars, ceiling - prefix)` never charged the prefix against the + // budget, so the final deny text could exceed maxChars by the prefix + // length and appendDesignSystemNoteOnce's size check lost exactly the + // room designNoteReserve had held back (Bugbot on PR #508). const budget = Math.min( limits.maxChars || DEFAULT_CONFIG.limits.maxChars, - CURSOR_DENY_LIMIT - BLOCK_PREFIX.length, - ); + CURSOR_DENY_LIMIT, + ) - BLOCK_PREFIX.length; const rendered = renderTemplate(findings, filePath, { ...config, limits: { ...limits, maxChars: budget } }, { cwd, footer: footerMode, reserveChars }); diff --git a/skill/scripts/hook-lib.mjs b/skill/scripts/hook-lib.mjs index b5e4570b1..794ac59a1 100644 --- a/skill/scripts/hook-lib.mjs +++ b/skill/scripts/hook-lib.mjs @@ -975,7 +975,10 @@ export function renderTemplate(findings, filePath, config, opts = {}) { const cap = Math.max(1, limits.maxFindings || DEFAULT_CONFIG.limits.maxFindings); // 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. + // 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(); @@ -1115,15 +1118,22 @@ function clampToBudget(header, lines, more, footer, maxChars) { // 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 - build([], footerText).length - 1; + const room = maxChars - bare.length - 1; if (room >= 24) { const clipped = line.length > room ? `${line.slice(0, room - 1)}…` : line; return build([clipped], footerText); } - // 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], footerText).slice(0, maxChars - 1)}…`; + // 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 diff --git a/tests/hook.test.mjs b/tests/hook.test.mjs index ee9eb340a..1e250e6d8 100644 --- a/tests/hook.test.mjs +++ b/tests/hook.test.mjs @@ -1224,6 +1224,24 @@ describe('renderTemplate()', () => { 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 prefix when line is 0', () => { const text = renderTemplate( [finding('side-tab', 0, { name: 'X' })],