From ead0346da60d4e0bcd5cecc9f945c2727159fdcf Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Sun, 9 Aug 2026 17:06:54 -0700 Subject: [PATCH] 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 --- skill/scripts/hook-before-edit.mjs | 10 ++++++++-- skill/scripts/hook-lib.mjs | 20 +++++++++++++++----- tests/hook.test.mjs | 18 ++++++++++++++++++ 3 files changed, 41 insertions(+), 7 deletions(-) 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' })],