-
Notifications
You must be signed in to change notification settings - Fork 0
feat(pr-contract): substance-only verification contract + template/doc reconciliation #100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - **Substance-only PR verification contract.** `scripts/pr-contract.mjs` now counts any non-empty bullet or checkbox in `## Verification` as a verification item (`missing_verification_item` replaces `missing_checked_verification`); unchecked boxes and missing evidence blocks are advisories instead of failures; heading presence/order is advisory while section substance stays enforced; placeholder and generic "tests pass"/"CI green" claims still hard-fail; fenced content inside the Verification section is no longer misread as items; advisories now surface in the job summary on success. Templates (`.github/PULL_REQUEST_TEMPLATE.md`, `contracts/pr-template.md`), README contract description, and pr-policy header docs reconciled; retired persona language removed from AGENTS.md. Caller inputs unchanged — consumers pick this up at the next `@v1` retag. (#99) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,19 +1,50 @@ | ||
| <!-- | ||
| Canonical PR body scaffold for ArchonVII repos (substance-only contract, #99). | ||
|
|
||
| Filling this out verbatim — with every TODO replaced by real content — | ||
| passes `repo-required-gate / pr-contract`. The rules: | ||
| - `## Verification` needs at least one substantive item (bullet or checkbox) | ||
| recording what was actually run or checked. | ||
| - Evidence blocks are the recommended shape; validated when present, | ||
| advisory when absent. | ||
| - Placeholders (TODO/TBD/N-slash-A) and generic claims ("tests pass", | ||
| "CI green") hard-fail. | ||
| - Heading order is advisory; section substance is not. | ||
| --> | ||
|
|
||
| ## Summary | ||
|
|
||
| TODO: Replace with a concise description of the change. | ||
|
|
||
| ## Verification | ||
|
|
||
| - [ ] TODO: Run required verification and replace this line. | ||
| - TODO: The exact command, CI check, or manual smoke test you ran, with its result. | ||
|
|
||
| ```evidence | ||
| command: TODO | ||
| location: local | ||
| result: TODO | ||
| timestamp: TODO | ||
| ``` | ||
|
|
||
| ### Verification Notes | ||
|
|
||
| TODO: Record concrete command output, CI check names, or manual smoke-test notes. | ||
|
|
||
| ## Docs / Changelog | ||
|
|
||
| TODO: Record docs/changelog handling. | ||
| TODO: Record docs/changelog handling (fragment, direct edit, docs update, or no-changelog label). | ||
|
|
||
| Plan/status artifacts: TODO: closed, narrowed, superseded-with-pointer, or not applicable. | ||
|
|
||
| Owner decisions this lane: appended / none | ||
|
|
||
| ## Linked Issue | ||
|
|
||
| TODO: Closes #___ | ||
| TODO: Closes #\_\_\_ | ||
|
|
||
| ## Risks | ||
|
|
||
| - Risk level: | ||
| - Rollback: | ||
| - Follow-ups: |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,8 +17,14 @@ const TITLE_RE = /^(feat|fix|refactor|test|docs|style|chore|perf|ci|build|revert | |
| const ISSUE_RE = /\b(Closes|Fixes|Refs)\s+#\d+\b/i; | ||
| // "placeholder" is valid completed prose; reject explicit unfilled markers. | ||
| const PLACEHOLDER_RE = /\b(TODO|TBD|FIXME|FILL ME|FILL IN|REPLACE THIS|NOT YET|N\/A|NONE YET)\b|#\s*(?:___|<[^>]+>)|<set-before-merge>/i; | ||
| const CHECKED_RE = /^\s*-\s+\[[xX]\]\s+(.+?)\s*$/; | ||
| const UNCHECKED_RE = /^\s*-\s+\[\s\]\s+(.+?)\s*$/; | ||
| const CHECKED_RE = /^\s*[-*]\s+\[[xX]\]\s+(.+?)\s*$/; | ||
| const UNCHECKED_RE = /^\s*[-*]\s+\[\s\]\s+(.+?)\s*$/; | ||
| // Substance-only contract (owner decision 2026-07-01, #99): any non-empty | ||
| // bullet in `## Verification` counts as a verification item — the contract | ||
| // requires that something substantive is recorded, not a specific checkbox | ||
| // format. The negative lookahead keeps checkbox lines out of the plain-item | ||
| // match so each line is classified exactly once. | ||
| const ITEM_RE = /^\s*[-*]\s+(?!\[[ xX]\]\s)(.+?)\s*$/; | ||
| const GENERIC_VERIFICATION_RE = /\b(automated ci checks? green|ci[- ]?green|ci checks? pass(?:ed|es)?|checks? green|all checks? pass(?:ed|es)?|tests? pass(?:ed|es)?)\b/i; | ||
|
|
||
| /** | ||
|
|
@@ -69,29 +75,39 @@ export function validatePrContract(input, options = {}) { | |
| validateBody(data.body, rules, errors, warnings); | ||
| } | ||
|
|
||
| const items = collectVerificationItems(data.body); | ||
| return { | ||
| ok: errors.length === 0, | ||
| errors, | ||
| warnings, | ||
| facts: { | ||
| docsOnly, | ||
| checkedVerificationCount: countCheckedVerificationItems(data.body), | ||
| checkedVerificationCount: items.checked.length, | ||
| verificationItemCount: [...items.checked, ...items.unchecked, ...items.plain] | ||
| .filter((item) => item.claim.length > 0).length, | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| export function formatPrContractResult(result) { | ||
| if (result.ok) { | ||
| const suffix = result.facts.docsOnly ? ' (docs-only body ceremony skipped)' : ''; | ||
| return `PR contract passed${suffix}.`; | ||
| const lines = [`PR contract passed${suffix}.`]; | ||
| if (result.warnings.length > 0) { | ||
| lines.push('', 'Advisories (non-blocking):'); | ||
| for (const item of result.warnings) { | ||
| lines.push(`- [${item.code}] ${item.message}`); | ||
| } | ||
| } | ||
| return lines.join('\n'); | ||
| } | ||
|
|
||
| const lines = ['PR contract failed.', '', 'Required fixes:']; | ||
| for (const item of result.errors) { | ||
| lines.push(`- [${item.code}] ${item.message}`); | ||
| } | ||
| if (result.warnings.length > 0) { | ||
| lines.push('', 'Warnings:'); | ||
| lines.push('', 'Advisories (non-blocking):'); | ||
| for (const item of result.warnings) { | ||
| lines.push(`- [${item.code}] ${item.message}`); | ||
| } | ||
|
|
@@ -188,7 +204,10 @@ function validateBody(body, rules, errors, warnings) { | |
| )); | ||
| } | ||
|
|
||
| validateHeadingOrder(headings, rules.requiredHeadings, errors); | ||
| // Heading presence/order is advisory since #99: sections are located by | ||
| // name, so the substance checks below still hard-fail when a section's | ||
| // content is genuinely missing — only the exact structure is soft. | ||
| validateHeadingOrder(headings, rules.requiredHeadings, warnings); | ||
|
|
||
| const summary = sectionContent(body, headings, 'Summary'); | ||
| if (rules.requireSummary && !hasSubstantiveContent(summary)) { | ||
|
|
@@ -216,44 +235,41 @@ function validateBody(body, rules, errors, warnings) { | |
| errors.push(error('empty_docs_changelog', '`## Docs / Changelog` must describe docs or changelog handling.', 'body')); | ||
| } | ||
|
|
||
| const verification = sectionContent(body, headings, 'Verification', { stopBefore: 'Verification Notes' }); | ||
| const checkedItems = verification.split(/\r?\n/).map((line, index) => { | ||
| const match = line.match(CHECKED_RE); | ||
| return match ? { claim: match[1].trim(), index } : null; | ||
| }).filter(Boolean); | ||
| const uncheckedItems = verification.split(/\r?\n/).map((line) => { | ||
| const match = line.match(UNCHECKED_RE); | ||
| return match ? match[1].trim() : null; | ||
| }).filter(Boolean); | ||
|
|
||
| if (rules.requireCheckedVerification && checkedItems.length === 0) { | ||
| const { verification, checked, unchecked, plain } = collectVerificationItems(body); | ||
| const allItems = [...checked, ...unchecked, ...plain]; | ||
| const substantiveItems = allItems.filter((item) => item.claim.length > 0); | ||
|
|
||
| if (rules.requireCheckedVerification && substantiveItems.length === 0) { | ||
| errors.push(error( | ||
| 'missing_checked_verification', | ||
| '`## Verification` must include at least one checked verification item.', | ||
| 'missing_verification_item', | ||
| '`## Verification` must include at least one verification item — a bullet or checkbox recording what was actually run or checked.', | ||
| 'body', | ||
| )); | ||
| } | ||
|
|
||
| for (const claim of uncheckedItems) { | ||
| errors.push(error( | ||
| 'unchecked_required_box', | ||
| `Unchecked verification item must be completed or removed before ready-for-review: "${claim}".`, | ||
| for (const item of unchecked) { | ||
| warnings.push(error( | ||
| 'unchecked_verification_item', | ||
| `Verification item is unchecked; it still counts as an item, but reads as not-done: "${item.claim}".`, | ||
| 'body', | ||
| )); | ||
| } | ||
|
|
||
| for (const item of checkedItems) { | ||
| for (const item of allItems) { | ||
| if (GENERIC_VERIFICATION_RE.test(item.claim)) { | ||
| errors.push(error( | ||
| 'generic_verification', | ||
| `Checked verification item is too generic: "${item.claim}". Record the actual command, check, or manual action.`, | ||
| `Verification item is too generic: "${item.claim}". Record the actual command, check, or manual action.`, | ||
| 'body', | ||
| )); | ||
| } | ||
| } | ||
|
|
||
| for (const item of checked) { | ||
| if (rules.requireEvidenceBlocks && !hasEvidenceBlockAfter(verification, item.index)) { | ||
| errors.push(error( | ||
| warnings.push(error( | ||
| 'missing_evidence_block', | ||
| `Checked verification item "${item.claim}" must be followed by a fenced \`\`\`evidence block.`, | ||
| `Checked verification item "${item.claim}" is not followed by a fenced \`\`\`evidence block (recommended shape; advisory since #99).`, | ||
|
Comment on lines
+270
to
+272
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This demotes missing evidence to a PR-contract warning, but in Useful? React with 👍 / 👎. |
||
| 'body', | ||
| )); | ||
| } | ||
|
|
@@ -375,10 +391,44 @@ function hasEvidenceBlockAfter(section, checkedLineIndex) { | |
| return false; | ||
| } | ||
|
|
||
| function countCheckedVerificationItems(body) { | ||
| // Classify every line of the `## Verification` section (fenced blocks masked | ||
| // so evidence field values are never misread as items) into checked boxes, | ||
| // unchecked boxes, and plain bullets. Line indexes are preserved through the | ||
| // masking so evidence-adjacency checks still run against the raw section. | ||
| function collectVerificationItems(body) { | ||
| const headings = parseHeadings(body || ''); | ||
| const verification = sectionContent(body || '', headings, 'Verification', { stopBefore: 'Verification Notes' }); | ||
| return verification.split(/\r?\n/).filter((line) => CHECKED_RE.test(line)).length; | ||
| const checked = []; | ||
| const unchecked = []; | ||
| const plain = []; | ||
| maskFencedLines(verification).forEach((line, index) => { | ||
| let match = line.match(CHECKED_RE); | ||
| if (match) { | ||
| checked.push({ claim: match[1].trim(), index }); | ||
| return; | ||
| } | ||
| match = line.match(UNCHECKED_RE); | ||
| if (match) { | ||
| unchecked.push({ claim: match[1].trim(), index }); | ||
| return; | ||
| } | ||
| match = line.match(ITEM_RE); | ||
| if (match) plain.push({ claim: match[1].trim(), index }); | ||
| }); | ||
| return { verification, checked, unchecked, plain }; | ||
| } | ||
|
|
||
| // Blank out fence delimiters and fenced content while preserving line count, | ||
| // so indexes into the raw section stay valid. | ||
| function maskFencedLines(text) { | ||
| let inFence = false; | ||
| return String(text || '').split(/\r?\n/).map((line) => { | ||
| if (/^\s*```/.test(line)) { | ||
| inFence = !inFence; | ||
| return ''; | ||
| } | ||
| return inFence ? '' : line; | ||
| }); | ||
| } | ||
|
|
||
| function isDocsOnly(files, rules) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this regex a plain
- Ran npm testline now satisfies the PR contract, but I checked.github/workflows/pr-body-autoinject.ymland its skip test still requireschecked: /-\s+\[[xX]\]/before it avoids prepending the TODO scaffold. For bot-authored non-doc PRs that already have the new valid plain-bullet verification, the reusable auto-inject workflow will still add TODO placeholders, and the same contract then fails onplaceholder_text; update the auto-inject satisfiability check and stub to use the new item semantics.Useful? React with 👍 / 👎.