Skip to content

fix(pr-contract): relax marker prose guard #91

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.

Sign up for GitHub

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

Merged
ArchonVII merged 1 commit into from
Jun 20, 2026
Merged

fix(pr-contract): relax marker prose guard #91

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions .changelog/unreleased/90-pr-contract-placeholder-matching.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
### Fixed

- Relaxed PR contract placeholder matching so completed prose may use the word
"placeholder" while explicit unfilled markers still fail validation.
9 changes: 9 additions & 0 deletions docs/repo-update-log.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ This log records agent-visible repository changes that should be easy to audit l
- **Propagation:** none | pending <repo/path> | completed <repo/path>
```

## 2026-06-20 - PR contract placeholder prose matching

- **Issue/PR:** #90 / #91
- **Branch:** agent/codex/90-pr-contract-placeholder-matching
- **Changed paths:** scripts/pr-contract.mjs, scripts/pr-contract.test.mjs, .changelog/unreleased/90-pr-contract-placeholder-matching.md, docs/repo-update-log.md
- **What changed:** Relaxed the shared PR contract validator so normal completed prose can use the word "placeholder". Explicit unfilled markers such as TODO, TBD, N/A, unset issue links, and `<set-before-merge>` remain rejected.
- **Verification:** `npx vitest run scripts/pr-contract.test.mjs` passed 23/23 tests. `git diff --check` exited 0 with CRLF warnings. `npm test` on local Windows failed before executing unchanged `scripts/doc-policy-lint.test.mjs` with `SyntaxError: Invalid or unexpected token`; latest upstream `Self-test (scripts)` on `main` passed on GitHub Actions run 27698751593, and this PR should use the PR self-test for full-suite Linux signal.
- **Propagation:** pending `v1` tag movement after merge.

## 2026-06-15 - Warning-only document policy lint workflow

- **Issue/PR:** #70 / (pending)
Expand Down
32 changes: 30 additions & 2 deletions scripts/pr-contract.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ const DEFAULT_REQUIRED_HEADINGS = [

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;
const PLACEHOLDER_RE = /\b(TODO|TBD|FIXME|FILL ME|FILL IN|REPLACE THIS|PLACEHOLDER|NOT YET|N\/A|NONE YET)\b|#\s*(?:___|<[^>]+>)|<set-before-merge>/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;
Comment thread
ArchonVII marked this conversation as resolved.
const CHECKED_RE = /^\s*-\s+\[[xX]\]\s+(.+?)\s*$/;
const UNCHECKED_RE = /^\s*-\s+\[\s\]\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;
Expand Down Expand Up @@ -399,7 +400,34 @@ function hasSubstantiveContent(text) {
}

function hasPlaceholder(text) {
return PLACEHOLDER_RE.test(text || '');
return PLACEHOLDER_RE.test(text || '') || hasLiteralPlaceholderFiller(text);
}

function hasLiteralPlaceholderFiller(text) {
const raw = stripHtmlComments(text);
const candidates = [
raw,
...raw.split(/\r?\n/),
];
return candidates.some((candidate) => {
const cleaned = normalizePlaceholderCandidate(candidate);
const words = cleaned.toLowerCase().match(/[a-z]+/g) || [];
return words.length > 0
&& (
words.every((word) => word === 'placeholder')
|| words.join(' ') === 'placeholder text'
Comment on lines +417 to +418

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 20, 2026

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 Reject explicit placeholder prose beyond bare filler

When a generated body leaves text such as This is a placeholder for the summary in ## Summary, ### Verification Notes, or ## Docs / Changelog, this helper no longer treats it as a placeholder because it only rejects lines whose words are all placeholder or exactly placeholder text; hasSubstantiveContent() then sees enough characters and the non-doc PR passes the ready-for-review contract. This is a fresh scenario beyond the earlier bare placeholder placeholder placeholder case, and it regresses explicit placeholder filler that the contract still says must be removed.

Useful? React with 👍 / 👎.

);
});
}

function normalizePlaceholderCandidate(text) {
return String(text || '')
.replace(/^[-*]\s+\[[ xX]\]\s+/, '')
.replace(/^[-*]\s+/, '')
.replace(/^(?:feat|fix|refactor|test|docs|style|chore|perf|ci|build|revert)(?:\([^)]+\))?:\s*/i, '')
.replace(/^[A-Za-z][A-Za-z -]{0,40}:\s*/, '')
.replace(/[`"'[\]{}()<>]/g, ' ')
.trim();
}

// Remove HTML comments before placeholder scanning so a template's own
Expand Down
34 changes: 34 additions & 0 deletions scripts/pr-contract.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,23 @@ describe('validatePrTemplate', () => {
// 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 visible completed prose uses the word "placeholder"', () => {
const body = validBody
.replace(
'- Add strict PR contract validation before ready-for-review.',
'- Allow completed PR prose to mention placeholder matching without failing the contract.',
)
.replace(
'Validated the reusable workflow tests locally with `npm test`; no warnings were emitted.',
'Validated that placeholder wording in visible completed prose is accepted.',
);
const result = validatePrContract(input({
title: 'fix(policy): allow placeholder prose',
body,
}));
expect(result.ok).toBe(true);
});

it('passes when the template HTML comment (with the word "placeholder") survives but visible fields are filled', () => {
const body = [
'<!--',
Expand All @@ -229,13 +246,30 @@ describe('context-aware parser (acceptance table)', () => {
expect(result.errors.map((e) => e.code)).toContain('placeholder_text');
});

it('fails when ## Summary is only literal placeholder filler', () => {
const body = validBody.replace(
'- Add strict PR contract validation before ready-for-review.',
'placeholder placeholder placeholder',
);
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 when an evidence-block field value is literal placeholder filler', () => {
const body = validBody.replace('command: npm test', 'command: placeholder');
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 }));
Expand Down
Loading