From d4e1b0902fad59fcd52afa4bb5f8798a8c0a6b35 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Thu, 13 Aug 2026 13:24:29 -0400 Subject: [PATCH] Enforce the ask_instruction sentence-initial contract Review on #576 caught document.md:71 splicing {{ask_instruction}} after "then", which is the same defect this branch set out to fix. Rendered for Codex it produced "Show the user the existing file, then STOP and use Codex's structured user-input/question tool...". The line now starts a new sentence. The comment added to PROVIDER_PLACEHOLDERS asserted the contract without enforcing it, which is exactly how four reference files shipped the splice in the first place. validateAskInstructionSites() in build.js now checks every call site and fails the build on a mid-sentence interpolation, and the comment points at the gate instead of asking authors to remember. Prepared with AI assistance (Claude Code). Co-Authored-By: Claude Opus 5 (1M context) --- scripts/build.js | 57 ++++++++++++++++++++++++++++++++++++- scripts/lib/utils.js | 6 ++-- skill/reference/document.md | 2 +- 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/scripts/build.js b/scripts/build.js index 94257eb85..5d3e07cac 100644 --- a/scripts/build.js +++ b/scripts/build.js @@ -371,6 +371,57 @@ function validateSkillProse(rootDir) { return errors; } +/** + * Validate that every `{{ask_instruction}}` interpolation starts a sentence. + * + * The placeholder's per-provider values are complete capitalized sentences + * ("STOP and call the AskUserQuestion tool to clarify."), so a call site that + * splices it mid-sentence ships malformed guidance to every provider at once: + * `stop and STOP and call the AskUserQuestion tool to clarify. before expanding + * it`. Four reference files shipped exactly that before this gate existed, and + * a comment in PROVIDER_PLACEHOLDERS asking authors to keep the contract is + * what failed to prevent it. + * + * Returns the number of validation errors. Build fails if > 0. + */ +function validateAskInstructionSites(rootDir) { + const dir = path.join(rootDir, 'skill', 'reference'); + const token = '{{ask_instruction}}'; + let errors = 0; + let sites = 0; + + if (!fs.existsSync(dir)) return 0; + + for (const file of fs.readdirSync(dir)) { + if (path.extname(file) !== '.md') continue; + const rel = path.join('skill/reference', file); + fs.readFileSync(path.join(dir, file), 'utf-8') + .split('\n') + .forEach((line, i) => { + let idx = line.indexOf(token); + while (idx !== -1) { + sites++; + // Bold/italic markers may sit between the punctuation and the token. + const before = line.slice(0, idx).replace(/[*_`]+\s*$/, '').trimEnd(); + if (before !== '' && !/[.!?:]$/.test(before)) { + console.error(` āŒ ${rel}:${i + 1}: ${token} is spliced mid-sentence`); + console.error(` ...${before.slice(-60)} ${token}`); + console.error(` Provider values are full sentences. Start a new one.`); + errors++; + } + idx = line.indexOf(token, idx + 1); + } + }); + } + + if (errors === 0) { + console.log(`āœ“ ask_instruction call sites: ${sites} sentence-initial`); + } else { + console.error(`\nāŒ ${errors} of ${sites} {{ask_instruction}} site(s) spliced mid-sentence.`); + } + return errors; +} + /** * Validate that every hand-authored HTML page carries the shared site header. * The partial is stamped with `` so drift is loud. @@ -738,7 +789,11 @@ async function build() { // that has no technical reading. Hardening repetition is intentionally allowed. const skillProseErrors = validateSkillProse(ROOT_DIR); - if (countErrors > 0 || versionErrors > 0 || manifestShapeErrors > 0 || proseErrors > 0 || skillProseErrors > 0) { + // Placeholder values are full sentences; a mid-sentence splice ships broken + // guidance to every provider at once. + const askSiteErrors = validateAskInstructionSites(ROOT_DIR); + + if (countErrors > 0 || versionErrors > 0 || manifestShapeErrors > 0 || proseErrors > 0 || skillProseErrors > 0 || askSiteErrors > 0) { process.exit(1); } diff --git a/scripts/lib/utils.js b/scripts/lib/utils.js index a3fc3c03d..f74da76f5 100644 --- a/scripts/lib/utils.js +++ b/scripts/lib/utils.js @@ -477,8 +477,10 @@ export const PROVIDER_PLACEHOLDERS = { 'codex': { model: 'GPT', config_file: 'AGENTS.md', - // Every {{ask_instruction}} call site is sentence-initial, so each value is a - // complete capitalized sentence. Keep it that way when adding a provider. + // Each value is a complete capitalized sentence, because every + // {{ask_instruction}} call site is sentence-initial. That is enforced by + // validateAskInstructionSites() in scripts/build.js, not left to authors: + // four reference files had already spliced the placeholder mid-sentence. ask_instruction: "STOP and use Codex's structured user-input/question tool when available; if unavailable, ask directly in chat to clarify what you cannot infer.", command_prefix: '$' }, diff --git a/skill/reference/document.md b/skill/reference/document.md index 5a7f348b7..90b963b1a 100644 --- a/skill/reference/document.md +++ b/skill/reference/document.md @@ -68,7 +68,7 @@ Omit irrelevant sections rather than filling them with invented rules. Put respo - An existing `DESIGN.md` is stale (the design has drifted). - Before a large redesign, to capture the current state as a reference. -If a `DESIGN.md` already exists, **do not silently overwrite it**. Show the user the existing file, then {{ask_instruction}} The choice is refresh, overwrite, or merge. +If a `DESIGN.md` already exists, **do not silently overwrite it**. Show the user the existing file first. {{ask_instruction}} The choice is refresh, overwrite, or merge. ## Two paths