Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .changelog/unreleased/81-pr-contract-body-file.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
### Added

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Add the required repo update log entry

This commit changes scripts and tests, so the repo-level AGENTS.md rule under “Repo update log” applies: code/config/behavior changes must append an entry to docs/repo-update-log.md before review. The diff adds a changelog fragment but no operational ledger entry, which leaves the PR failing the repo’s documented closeout/audit requirement; add the entry with issue/PR, branch, changed paths, verification, and propagation status.

Useful? React with 👍 / 👎.


- `pr-contract.mjs` gains a `--body-file <path|->` (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)
27 changes: 26 additions & 1 deletion scripts/agent-close-preflight.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -23,6 +27,23 @@ function parseArgs(argv) {
return args;
}

function printUsage() {
process.stdout.write(`Usage: agent-close-preflight.mjs --repo <owner/name> --pr <n> [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 <owner/name> Target repository (required).
--pr <n> 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',
Expand Down Expand Up @@ -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'],
Expand Down
26 changes: 25 additions & 1 deletion scripts/agent-pr-ready.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -23,8 +27,28 @@ function parseArgs(argv) {
return args;
}

function printUsage() {
process.stdout.write(`Usage: agent-pr-ready.mjs --repo <owner/name> --pr <n> [--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 <owner/name> Target repository (required).
--pr <n> 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'],
Expand Down
101 changes: 93 additions & 8 deletions scripts/pr-contract.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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.',
Expand All @@ -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.',
Expand Down Expand Up @@ -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('<!--') && !line.startsWith('```'))
Expand All @@ -402,6 +402,30 @@ function hasPlaceholder(text) {
return PLACEHOLDER_RE.test(text || '');
}

// Remove HTML comments before placeholder scanning so a template's own
// instructional comment (which legitimately contains words like "placeholder"
// or "TODO") does not trip the contract. Evidence-block field values are NOT
// stripped, so `command: TODO` inside a ```evidence block still fails.
// Source: forensic analysis of session 019eccc1 (F4) + owner refinement 5.
function stripHtmlComments(text) {
return String(text || '').replace(/<!--[\s\S]*?-->/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();
}
Expand All @@ -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];
Expand All @@ -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 <input-mode> [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 <path|-> Validate a locally drafted body. '-' reads stdin.
Pair with --title, --branch, --files-json.
--repo <owner/name> --pr <n> Validate an existing remote PR (via gh pr view).
--event-path <path> Validate a pull_request event payload (CI).

Options:
--title <text> PR title (body-file mode).
--branch <name> Head branch (body-file mode).
--files-json <json> JSON array of changed file paths (docs-only detection).
--branch-pattern <regex> Override the allowed head-branch pattern.
--doc-only-extensions <exts> Override the docs-only file extensions.
--doc-only-path-prefixes <p> 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'],
Expand Down
127 changes: 127 additions & 0 deletions scripts/pr-contract.test.mjs
Original file line number Diff line number Diff line change
@@ -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',
'',
Expand Down Expand Up @@ -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 = [
'<!--',
' Replace every placeholder before marking the PR ready for review.',
'-->',
'',
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');
});
});
Loading