diff --git a/.changelog/unreleased/81-pr-contract-body-file.md b/.changelog/unreleased/81-pr-contract-body-file.md new file mode 100644 index 0000000..4d3be8c --- /dev/null +++ b/.changelog/unreleased/81-pr-contract-body-file.md @@ -0,0 +1,14 @@ +### Added + +- `pr-contract.mjs` gains a `--body-file ` (stdin) input mode so the PR + contract can validate a locally drafted body before a PR exists — the same + validator now runs identically before and after PR creation. (#81) +- `--help`/usage output for `pr-contract.mjs`, `agent-pr-ready.mjs`, and + `agent-close-preflight.mjs`. (#81) + +### Changed + +- The PR-body parser is now context-aware: HTML comments are ignored by the + placeholder scan, and fenced/inline code and quoted/blockquoted text are + ignored by the generic-verification scan on free prose. Checked checkbox + claims and evidence-block field placeholders still hard-fail. (#81) diff --git a/scripts/agent-close-preflight.mjs b/scripts/agent-close-preflight.mjs index 9d5db32..431ed09 100644 --- a/scripts/agent-close-preflight.mjs +++ b/scripts/agent-close-preflight.mjs @@ -10,10 +10,14 @@ function parseArgs(argv) { const args = {}; for (let i = 0; i < argv.length; i++) { const item = argv[i]; + if (item === '-h' || item === '--help') { + args.help = true; + continue; + } if (!item.startsWith('--')) continue; const key = item.slice(2); - if (key === 'json' || key === 'allow-ready' || key === 'skip-git') { + if (key === 'json' || key === 'allow-ready' || key === 'skip-git' || key === 'help') { args[key] = true; continue; } @@ -23,6 +27,23 @@ function parseArgs(argv) { return args; } +function printUsage() { + process.stdout.write(`Usage: agent-close-preflight.mjs --repo --pr [options] + +Validate an existing draft PR against the contract AND check local git state +(clean tree, branch matches PR head, branch pushed) before closeout. To validate +a drafted body BEFORE the PR exists, use \`pr-contract.mjs --body-file -\` instead. + +Options: + --repo Target repository (required). + --pr PR number (required). + --allow-ready Do not fail if the PR is already ready for review. + --skip-git Skip the local git-state checks. + --json Emit the full result object as JSON. + -h, --help Show this help. +`); +} + function git(args, options = {}) { return execFileSync('git', args, { encoding: 'utf8', @@ -68,6 +89,10 @@ function collectGitFailures({ expectedBranch }) { function main() { const args = parseArgs(process.argv.slice(2)); + if (args.help) { + printUsage(); + return; + } const pr = loadPrFromGh({ repo: args.repo, pr: args.pr }); const contract = validatePrContract(pr, { branchPattern: args['branch-pattern'], diff --git a/scripts/agent-pr-ready.mjs b/scripts/agent-pr-ready.mjs index c7573be..b1202c6 100644 --- a/scripts/agent-pr-ready.mjs +++ b/scripts/agent-pr-ready.mjs @@ -10,10 +10,14 @@ function parseArgs(argv) { const args = {}; for (let i = 0; i < argv.length; i++) { const item = argv[i]; + if (item === '-h' || item === '--help') { + args.help = true; + continue; + } if (!item.startsWith('--')) continue; const key = item.slice(2); - if (key === 'json' || key === 'dry-run') { + if (key === 'json' || key === 'dry-run' || key === 'help') { args[key] = true; continue; } @@ -23,8 +27,28 @@ function parseArgs(argv) { return args; } +function printUsage() { + process.stdout.write(`Usage: agent-pr-ready.mjs --repo --pr [--dry-run] [--json] + +Validate a PR against the ready-for-review contract and, if it passes, promote +the draft PR to ready (gh pr ready). Refuses to promote a PR that fails the +contract. With --dry-run it only reports what it would do. + +Options: + --repo Target repository (required). + --pr PR number (required). + --dry-run Report only; do not run gh pr ready. + --json Emit the full result object as JSON. + -h, --help Show this help. +`); +} + function main() { const args = parseArgs(process.argv.slice(2)); + if (args.help) { + printUsage(); + return; + } const pr = loadPrFromGh({ repo: args.repo, pr: args.pr }); const result = validatePrContract(pr, { branchPattern: args['branch-pattern'], diff --git a/scripts/pr-contract.mjs b/scripts/pr-contract.mjs index 95128dc..f72d2b5 100644 --- a/scripts/pr-contract.mjs +++ b/scripts/pr-contract.mjs @@ -179,7 +179,7 @@ function validateBody(body, rules, errors, warnings) { )); } - if (rules.rejectPlaceholders && hasPlaceholder(body)) { + if (rules.rejectPlaceholders && hasPlaceholder(stripHtmlComments(body))) { errors.push(error( 'placeholder_text', 'PR body contains placeholder text such as TODO, TBD, N/A, or an unset issue marker.', @@ -202,7 +202,7 @@ function validateBody(body, rules, errors, warnings) { 'body', )); } - if (rules.rejectGenericVerificationNotes && GENERIC_VERIFICATION_RE.test(notes.trim())) { + if (rules.rejectGenericVerificationNotes && GENERIC_VERIFICATION_RE.test(maskCodeAndQuotes(notes).trim())) { errors.push(error( 'generic_verification_notes', '`### Verification Notes` must not be a generic CI/tests-passed statement.', @@ -389,7 +389,7 @@ function isDocsOnly(files, rules) { } function hasSubstantiveContent(text) { - const cleaned = text + const cleaned = stripHtmlComments(text) .split(/\r?\n/) .map((line) => line.replace(/^[-*]\s+\[[ xX]\]\s+/, '').trim()) .filter((line) => line && !line.startsWith('/g, ' '); +} + +// Mask fenced code/evidence blocks, inline code spans, and blockquoted lines +// from FREE PROSE before the generic-verification scan, so an explanatory note +// may quote a command or diagnostic (e.g. a cited `npm test` line) without being +// rejected as a generic "tests passed" claim. Checked checkbox claims are scanned +// raw (not masked), so `- [x] tests passed` still fails. +// Source: forensic analysis of session 019eccc1 (F7) + owner refinement 5. +function maskCodeAndQuotes(text) { + return String(text || '') + .replace(/```[\s\S]*?```/g, ' ') + .replace(/`[^`\n]*`/g, ' ') + .split(/\r?\n/) + .map((line) => (/^\s*>/.test(line) ? ' ' : line)) + .join('\n'); +} + function sameHeading(left, right) { return cleanHeadingText(left).toLowerCase() === cleanHeadingText(right).toLowerCase(); } @@ -418,11 +442,15 @@ function parseArgs(argv) { const args = {}; for (let i = 0; i < argv.length; i++) { const item = argv[i]; + if (item === '-h' || item === '--help') { + args.help = true; + continue; + } if (!item.startsWith('--')) continue; const key = item.slice(2); - if (key === 'json') { - args.json = true; + if (key === 'json' || key === 'help') { + args[key] = true; continue; } args[key] = argv[i + 1]; @@ -448,11 +476,68 @@ function loadInputFromEvent(eventPath, filesJson) { }; } +// Pre-publish input adapter: validate a locally drafted body with NO remote PR. +// `--body-file -` reads the body from stdin so the agent never needs a temp file. +// Pair with --title / --branch / --files-json so the SAME contract that CI runs +// after creation can be run locally before `gh pr create`. +// Source: forensic analysis of session 019eccc1 (F2/F3) — the keystone that +// removes the push -> create -> amend -> re-scan loop. +function loadInputFromBodyFile(args) { + const source = args['body-file']; + const body = readFileSync(source === '-' ? 0 : source, 'utf8'); + const files = args['files-json'] ? JSON.parse(args['files-json']) : []; + return { + title: args.title || '', + body, + branch: args.branch || '', + files, + isDraft: true, + number: null, + url: null, + }; +} + +function printUsage() { + process.stdout.write(`Usage: pr-contract.mjs [options] + +Validate a PR against the ArchonVII ready-for-review contract. The same +validator runs locally (before a PR exists) and in CI (after) — identical rules. + +Input modes (choose one): + --body-file Validate a locally drafted body. '-' reads stdin. + Pair with --title, --branch, --files-json. + --repo --pr Validate an existing remote PR (via gh pr view). + --event-path Validate a pull_request event payload (CI). + +Options: + --title PR title (body-file mode). + --branch Head branch (body-file mode). + --files-json JSON array of changed file paths (docs-only detection). + --branch-pattern Override the allowed head-branch pattern. + --doc-only-extensions Override the docs-only file extensions. + --doc-only-path-prefixes

Override docs-only path prefixes (newline-separated). + --json Emit the full result object as JSON. + -h, --help Show this help. + +Exit code: 0 = contract passes, 1 = fails. +`); +} + function main() { const args = parseArgs(process.argv.slice(2)); - const input = args['event-path'] - ? loadInputFromEvent(args['event-path'], args['files-json'] || process.env.PR_CONTRACT_FILES_JSON) - : loadPrFromGh({ repo: args.repo, pr: args.pr }); + if (args.help) { + printUsage(); + return; + } + + let input; + if (args['body-file']) { + input = loadInputFromBodyFile(args); + } else if (args['event-path']) { + input = loadInputFromEvent(args['event-path'], args['files-json'] || process.env.PR_CONTRACT_FILES_JSON); + } else { + input = loadPrFromGh({ repo: args.repo, pr: args.pr }); + } const result = validatePrContract(input, { branchPattern: args['branch-pattern'], diff --git a/scripts/pr-contract.test.mjs b/scripts/pr-contract.test.mjs index 5a9984b..855d08d 100644 --- a/scripts/pr-contract.test.mjs +++ b/scripts/pr-contract.test.mjs @@ -1,6 +1,10 @@ +import { execFileSync } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; import { describe, expect, it } from 'vitest'; import { validatePrContract, validatePrTemplate, formatPrTemplateResult } from './pr-contract.mjs'; +const scriptPath = fileURLToPath(new URL('./pr-contract.mjs', import.meta.url)); + const validBody = [ '## Summary', '', @@ -199,3 +203,126 @@ describe('validatePrTemplate', () => { .toContain('conforms'); }); }); + +// Owner acceptance table from the closeout-contract plan (session 019eccc1 F4/F7). +// The "still fails" cases matter as much as the "now passes" cases: the parser is +// context-aware, not weaker. +describe('context-aware parser (acceptance table)', () => { + it('passes when the template HTML comment (with the word "placeholder") survives but visible fields are filled', () => { + const body = [ + '', + '', + validBody, + ].join('\n'); + expect(validatePrContract(input({ body })).ok).toBe(true); + }); + + it('fails when ## Summary still contains a TODO placeholder', () => { + const body = validBody.replace( + '- Add strict PR contract validation before ready-for-review.', + 'TODO: Fill in summary.', + ); + const result = validatePrContract(input({ body })); + expect(result.ok).toBe(false); + expect(result.errors.map((e) => e.code)).toContain('placeholder_text'); + }); + + it('fails when an evidence-block field value is a placeholder (command: TODO)', () => { + const body = validBody.replace('command: npm test', 'command: TODO'); + const result = validatePrContract(input({ body })); + expect(result.ok).toBe(false); + expect(result.errors.map((e) => e.code)).toContain('placeholder_text'); + }); + + it('fails a checked verification claim of "tests passed"', () => { + const body = validBody.replace('- [x] npm test', '- [x] tests passed'); + const result = validatePrContract(input({ body })); + expect(result.ok).toBe(false); + expect(result.errors.map((e) => e.code)).toContain('generic_verification'); + }); + + it('passes a note that cites a command in inline code', () => { + const body = validBody.replace( + 'Validated the reusable workflow tests locally with `npm test`; no warnings were emitted.', + 'Direct `npm test` evidence is recorded in the block above; no warnings were emitted.', + ); + expect(validatePrContract(input({ body })).ok).toBe(true); + }); + + it('passes a note that quotes a "tests passed" diagnostic inside a fenced block', () => { + const body = validBody.replace( + 'Validated the reusable workflow tests locally with `npm test`; no warnings were emitted.', + [ + 'Close-scan hit a Windows buffer limit; the direct run is cited below and GitHub checks succeeded.', + '', + '```', + 'npm test -> 129 tests passed, 0 failed', + '```', + ].join('\n'), + ); + expect(validatePrContract(input({ body })).ok).toBe(true); + }); + + it('passes a note that quotes a "tests passed" diagnostic on a blockquote line', () => { + const body = validBody.replace( + 'Validated the reusable workflow tests locally with `npm test`; no warnings were emitted.', + [ + 'Close-scan hit a Windows buffer limit; the direct run is cited below and GitHub checks succeeded.', + '', + '> npm test -> 129 tests passed, 0 failed', + ].join('\n'), + ); + expect(validatePrContract(input({ body })).ok).toBe(true); + }); + + it('fails a Docs / Changelog section that is only "N/A"', () => { + const body = validBody.replace( + '- [x] README and reusable workflow examples updated for the new contract.', + 'N/A', + ); + const result = validatePrContract(input({ body })); + expect(result.ok).toBe(false); + expect(result.errors.map((e) => e.code)).toContain('placeholder_text'); + }); +}); + +// The keystone: the same validator runs on a drafted body BEFORE a PR exists. +describe('pr-contract --body-file (pre-publish, no PR)', () => { + const runBodyFile = (body, extraArgs = []) => { + try { + const stdout = execFileSync('node', [scriptPath, '--body-file', '-', '--json', ...extraArgs], { + input: body, + encoding: 'utf8', + }); + return { code: 0, result: JSON.parse(stdout) }; + } catch (err) { + return { code: err.status ?? 1, result: JSON.parse(err.stdout || '{}') }; + } + }; + + it('validates a complete drafted body from stdin and exits 0', () => { + const { code, result } = runBodyFile(validBody, [ + '--title', 'feat(policy): enforce PR contract before ready', + '--branch', 'agent/codex/36-pr-contract-gate', + '--files-json', JSON.stringify(['scripts/pr-contract.mjs']), + ]); + expect(code).toBe(0); + expect(result.ok).toBe(true); + }); + + it('rejects a drafted body with placeholders and exits 1', () => { + const { code, result } = runBodyFile( + validBody.replace('- Add strict PR contract validation before ready-for-review.', 'TODO: fill me'), + [ + '--title', 'feat(policy): enforce PR contract before ready', + '--branch', 'agent/codex/36-pr-contract-gate', + '--files-json', JSON.stringify(['scripts/pr-contract.mjs']), + ], + ); + expect(code).toBe(1); + expect(result.ok).toBe(false); + expect(result.errors.map((e) => e.code)).toContain('placeholder_text'); + }); +});