ci: pf-4128 contribution policy workflow by cdcabrera · Pull Request #194 · patternfly/patternfly-mcp

cdcabrera · 2026-05-11T20:31:39Z

What is it?

ci: pf-4128 contribution policy workflow
- build, workflow package audit, combined PR gatekeeper, commits, integration
- testing, scripts testing for workflows

Notes

workflow scripting broken out to help reviews
the integration workflow and support scripts require being checked in before the Gatekeeper policy will pass

Updates issue/story

related #193
updates #190
pf-4128

nicolethoen

Took a look primarily at the copy

nicolethoen · 2026-05-13T15:17:22Z

+  if (codeSignature.hasTell) {
+    const botComment = `### 🤖 PR Quality Guidance\n` +
+      `I've moved this to **Draft** due to the scope of changes, and to avoid confusion.\n` +
+      `Once you've focused your changes I'll take another look.\n\n` +


Once you've addressed these items, I'll re-check your changes. Thank you for your contribution!

nicolethoen · 2026-05-13T15:17:43Z

+  } else {
+    // Or confirm the work has passed pre-check
+    const successComment = `### 🤖 PR Quality Guidance\n` +
+      `I finished my scan and all pre-checks pass!\n\n` +


All pre-checks pass! Your contribution is queued for review. Thank you for following our guidelines!

* build, workflow package audit, policy PR gatekeeper, commits, integration * testing, scripts testing for workflows

cdcabrera · 2026-05-13T16:48:59Z

So we ended up refactoring the messages again @nicolethoen ... it may alter your comments

the draft variation had to be removed, it just melted in testing
outside contributors received nothing in testing, no labels, no comments. what we're about to add into this is they now receive logging with the same "comments" and a "label" subset. this should improve the overall experience
and we ended up refactor messages after a bot reviewer implied the content was written by a soulless machine ... whoops

dlabaj

Looks good

nicolethoen

I added suggestions back with suggestions for the messages.

I tried removing 'you' and 'i' statements to make it sound more direct/actionable and a bit less first person.

nicolethoen · 2026-05-13T19:13:53Z

+    const errors = [];
+
+    if (isMaxFilesUpdated === true) {
+      errors.push(`⚠️ You've updated a lot of files (${fileCount}/${fileChangeLimit}). **Resolution:** To ensure a smooth review, please consider reducing the scope of these changes by splitting them into smaller, more focused PR contributions.`);


⚠️ Large changeset detected (${fileCount} files, guideline: ${fileChangeLimit}). Resolution: Smaller PRs are easier to review and merge faster. Please split these changes into focused PRs, each addressing a single concern.

nicolethoen · 2026-05-13T19:14:07Z

+    }
+
+    if (isCoreModified) {
+      errors.push(`⚠️ I detected core file modifications. To help us track and plan your work, please ensure these updates are associated with a GitHub issue (${coreModified.join(', ')}). **Resolution:** Link a GitHub issue in your PR description. Starting with an issue ensures your work is recognized and aligned with the roadmap.`);


⚠️ Core file modifications detected (${coreModified.join(', ')}). Resolution: Please link the related issue in your PR description. If no issue exists, please create one first so we can discuss the approach. How to create an issue

nicolethoen · 2026-05-13T19:14:53Z

+    }
+
+    if (isSecModified) {
+      errors.push(`⚠️ I've found updates to security-sensitive files (${secModified.join(', ')}). **Resolution:** These changes require a core contributor's review. You can remove these changes or provide a clear explanation for these updates in your PR description.`);


⚠️ Changes detected to security-sensitive files... Resolution: These files (workflows, scripts, package configuration) require a core contributor's security review before merging.

nicolethoen · 2026-05-13T19:16:30Z

+    }
+
+    if (isAgentModified) {
+      errors.push(`⚠️ I found modifications to agent guidelines (${agentModified.join(', ')}). **Resolution:** Changes to shared guidelines require architectural alignment. Please coordinate these updates through a GitHub issue first.`);


⚠️ Changes to agent instruction guidelines detected (${guidelinesModified.join(', ')}). Resolution: Modifications to AI agent instructions require maintainer review for security and quality. Please explain your changes in the PR description, attach the related issue, and a maintainer will review.

nicolethoen · 2026-05-13T19:17:49Z

+      errors.push(`⚠️ I detected core file modifications. To help us track and plan your work, please ensure these updates are associated with a GitHub issue (${coreModified.join(', ')}). **Resolution:** Link a GitHub issue in your PR description. Starting with an issue ensures your work is recognized and aligned with the roadmap.`);
+    }
+
+    if (isExtraModified) {


⚠️ Changes detected to test fixture/mock directories (${extraModified.join(', ')}). These don't match our testing structure. Resolution: Please review our testing guidelines or ask in your PR description how to structure your tests. A maintainer will provide guidance.

This one we're going to have to tweak more... these lists we're leveraging can include both "test" and "code".

Inviting discussion on aligning to an established pattern vs creating an entirely new one based on an internal preference feels like an unresolvable thing (def way more talking than we want). I'll tweak it to remove the "I" for sure, but I'll fiddle with it

jpuzz0 · 2026-05-14T13:55:56Z

With CODEOWNERS requiring maintainer approval on every file in this repo, what is this script catching that the existing review process doesn't already cover?

If I'm understanding this correctly, someone can submit a working, tested fix — and their build fails. Not because their code is broken, but because of which files they touched. What are they supposed to do next?

I think this is a possible scenario — a contributor adds a new MCP tool with full test coverage, updates snapshots, writes a detailed PR description by hand. They've seemingly done everything right, but because they touched 16 files, hit a few paths on the core list, and didn't preserve a hidden metadata string from the PR template — hasTell fires, CI fails, and a bot tells them their work doesn't meet "quality, security, and architectural standards." How do we expect someone to respond to that? That doesn't guide contributions — it discourages them.

cdcabrera · 2026-05-14T13:58:55Z

@jpuzz0 in meeting we noted #193

I recommend you review the doc updates there for the explanation you want

nicolethoen

Jeff raised good point. The scenario he described is real — contributor does everything right (working code, tests, snapshots) but CI fails on metadata/file count. CODEOWNERS already enforces maintainer review on every file, so automation doesn't add protection — just friction.

Some things we could consider:

Keep warnings/labels as guidance
Remove hard CI failure from hasTell
Drop isPrTemplateModified and isGenModified from failure conditions
CODEOWNERS is actual gate, let automation inform maintainer review instead of blocking it

Our own bot messaging in this PR emphasizes "starting with a conversation helps ensure your contribution is integrated smoothly." Fewer CI failure gates communicates we're open to conversation. Contributors still get all feedback and warnings — just not blocked from maintainer review.

nicolethoen · 2026-05-14T14:19:36Z

+      `- Consider splitting your changes into smaller, focused PR contributions.\n\n` +
+      `Starting with a conversation helps ensure your contribution is integrated smoothly. Once you've focused your changes, I'll take another look.\n\n` +
+      `**Labels**: \`${LABEL_NEEDS_CLEANUP}\`, \`${LABEL_PRECHECKS_FAIL}\` \n\n` +
+      `_Read our [contribution guidelines](https://github.com/patternfly/patternfly-mcp/blob/main/CONTRIBUTING.md). This comment updates automatically._`;


### 🤖 PR Quality Guidance\n +
This PR has been flagged for a **Policy Hold** to ensure alignment with quality, security, and architectural standards.\n\n +
**To resolve this hold and move forward**:\n +
- Associate updates with a GitHub issue (Step 1 of the [Contributor's Journey (https://github.com/patternfly/patternfly-mcp/blob/main/CONTRIBUTING.md#step-1-start-a-conversation)).\n +
- Align changes with codebase style and remove excessive scope.\n +
- Consider splitting changes into smaller, focused PR contributions.\n\n +
Starting with a conversation helps ensure contributions are integrated smoothly.\n\n +
**Labels**: \${LABEL_NEEDS_CLEANUP}`, `${LABEL_PRECHECKS_FAIL}` \n\n+Read our contribution guidelines. This comment updates automatically.`

nicolethoen · 2026-05-14T14:19:38Z

+      `I found some issues with your work. Once the following updates are addressed, you'll be queued for review:\n\n` +
+      `${codeSignature.errors.map(err => `- ${err}`).join('\n')}\n\n` +
+      `**Labels**: \`${LABEL_NEEDS_CLEANUP}\` \n\n` +
+      `_Read our [contribution guidelines](https://github.com/patternfly/patternfly-mcp/blob/main/CONTRIBUTING.md). This comment updates automatically._`;


### 🤖 PR Quality Guidance\n +
The following issues need to be addressed before this PR can be queued for review:\n\n +
${codeSignature.errors.map(err => - ${err}).join('\n')}\n\n +
**Labels**: \${LABEL_NEEDS_CLEANUP}` \n\n+Read our contribution guidelines. This comment updates automatically.`

cdcabrera mentioned this pull request May 11, 2026

docs(contributing): pf-4128 automation, testing standards #193

Draft

cdcabrera force-pushed the pf-4128-workflows branch 2 times, most recently from 56a7eb6 to 8674b84 Compare May 12, 2026 18:28

nicolethoen requested review from dlabaj, jpuzz0 and nicolethoen May 12, 2026 18:42

cdcabrera force-pushed the pf-4128-workflows branch from 8674b84 to 27b74a2 Compare May 12, 2026 20:40

cdcabrera marked this pull request as ready for review May 12, 2026 20:41

cdcabrera mentioned this pull request May 12, 2026

build(node.js)!: pf-3843 remove 20 support #169

Draft

2 tasks

nicolethoen reviewed May 13, 2026

View reviewed changes

Comment thread scripts/workflow.preCheck.js

ci: pf-4128 contribution policy workflow

b60aa20

* build, workflow package audit, policy PR gatekeeper, commits, integration * testing, scripts testing for workflows

cdcabrera force-pushed the pf-4128-workflows branch from 27b74a2 to b60aa20 Compare May 13, 2026 16:52

cdcabrera changed the title ~~build: pf-4128 contribution workflow actions~~ build: pf-4128 contribution policy workflow May 13, 2026

cdcabrera changed the title ~~build: pf-4128 contribution policy workflow~~ ci: pf-4128 contribution policy workflow May 13, 2026

dlabaj previously approved these changes May 13, 2026

View reviewed changes

nicolethoen reviewed May 13, 2026

View reviewed changes

fix: review update

95e0e24

cdcabrera dismissed dlabaj’s stale review via 95e0e24 May 13, 2026 22:21

nicolethoen reviewed May 14, 2026

View reviewed changes

Conversation

cdcabrera commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is it?

Notes

Updates issue/story

Uh oh!

nicolethoen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cdcabrera commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dlabaj left a comment

Choose a reason for hiding this comment

Uh oh!

nicolethoen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cdcabrera May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jpuzz0 commented May 14, 2026

Uh oh!

cdcabrera commented May 14, 2026

Uh oh!

nicolethoen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cdcabrera commented May 11, 2026 •

edited

Loading

cdcabrera commented May 13, 2026 •

edited

Loading

cdcabrera May 13, 2026 •

edited

Loading