feat(ci): carry PR quality gates onto dev2-go (#648) - #683
Conversation
…n#648) * docs: spec PR quality gates for ancestry and descriptions Capture the approved design for rejecting main-based PRs into dev/dev2-go and empty/thin/malformed PR bodies in the enforcer. * docs: plan PR ancestry and description quality gates Break the approved spec into TDD tasks for pr-quality.cjs, enforcer wiring, harness coverage, and contributing docs. * feat(ci): add pure PR ancestry and description quality checks * test(ci): cover wrong_base plus bad_description together * feat(ci): enforce PR ancestry and description in target gate * fix(ci): strip WRONG BRANCH prefix when base is corrected * test(ci): cover PR ancestry and description enforcement paths * docs: document PR ancestry and description quality gates * fix(ci): address CodeRabbit findings on PR quality gates Treat removed new-form Version/OS headings as missing, soft-fail ancestry compares, tighten harness require/write allowlists, and clarify enforce-target is convention until branch protection. * fix(ci): address Codex findings on PR quality gates Reject untouched PR templates, require low ahead-of-main for ancestry, and checkpoint draft/title ownership before mutations.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Wibias
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4879a54a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try { | ||
| await convertToDraft(); | ||
| state.autoDraftedByBot = true; | ||
| // Checkpoint after a successful draft so ownership survives | ||
| // if the final explanation write fails. | ||
| await upsertComment( |
There was a problem hiding this comment.
If convertToDraft() succeeds but the following checkpoint upsertComment() fails, this shared catch treats the comment error as a conversion failure and sets autoDraftedByBot to false. If the final comment retry succeeds, that false state is persisted even though the bot did draft the PR, so resolving the quality violation will leave the PR stuck in draft. Catch the conversion and checkpoint failures separately, preserving ownership once conversion succeeds.
Useful? React with 👍 / 👎.
| const skipAncestry = | ||
| ancestryLookupFailed || | ||
| (!permissionLookupFailed && authorHasPushPermission(authorPermission)); |
There was a problem hiding this comment.
When either compare API request fails transiently, ancestryLookupFailed makes the ancestry gate skip rather than fail or remain pending. An otherwise valid main-based PR therefore gets a green check; worse, a previously flagged PR is marked ready and its active state is cleared even though its ancestry was never re-evaluated. Treat comparison failure as an explicit gate failure, or at minimum preserve an existing ancestry failure until a successful comparison confirms it is fixed.
AGENTS.md reference: AGENTS.md:L83-L86
Useful? React with 👍 / 👎.
| if (hasEscapedNewlines(body)) { | ||
| return { ok: false, reason: "escaped_newlines" }; | ||
| } | ||
| const withoutTemplate = stripPrTemplateBoilerplate(body); |
There was a problem hiding this comment.
When an otherwise valid description contains generated HTML metadata with several literal \n sequences, this check counts those hidden sequences before clean() strips the comment and can classify the visible description as malformed. Run escaped-newline detection on comment-stripped content so hidden release-note or bot metadata cannot cause a valid PR to be drafted and failed.
Useful? React with 👍 / 👎.
| * Exact instruction / checklist lines from `.github/PULL_REQUEST_TEMPLATE.md`. | ||
| * Untouched templates must not count as substance. | ||
| */ | ||
| const PR_TEMPLATE_BOILERPLATE_LINES = new Set([ |
There was a problem hiding this comment.
This new validator embeds exact lines from .github/PULL_REQUEST_TEMPLATE.md, but neither issue-quality-tests.yml nor the cross-platform CI path filters include that template, and no test reads it dynamically. A template-only wording change can therefore land without running these tests and make an untouched template count as a substantial unstructured description. Add the template to the workflow paths and test that this set remains synchronized with it.
Useful? React with 👍 / 👎.
|
|
||
| ## Pull requests | ||
|
|
||
| - Target **`dev`** (or **`dev2-go`** only for scoped Go native-port work). Do not open ordinary feature or fix pull requests against **`main`**. |
There was a problem hiding this comment.
This bullet repeats the dev/dev2-go/main targeting policy already stated immediately above under Branches, creating two copies on the same page that must remain synchronized. Keep this section focused on the new ancestry and description gates, or link back to the existing branch-policy section instead.
AGENTS.md reference: docs-site/AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
| isNewBugForm && | ||
| version !== null && | ||
| (isEmpty(version) || isRawPlaceholder(version)) | ||
| (version === null || isEmpty(version) || isRawPlaceholder(version)) |
There was a problem hiding this comment.
A new-form bug is identified only by the still-editable Client or integration heading. After submission, an author can remove that heading together with Version and Operating system; the stored kind or bug label keeps the report classified as a bug, but isNewBugForm becomes false and these new missing-field checks are skipped, so the edited report validates successfully. Persist whether the issue originated from the new form, or use immutable workflow state rather than a removable heading to decide whether these fields are required.
Useful? React with 👍 / 👎.
1 participant
Summary
dev2-go(CI workflows/scripts, docs, AGENTS/MAINTAINERS, tests).75bd272dondev.Go port
No Go counterpart. This change is GitHub Actions / Node CI scripts, contributing docs, and maintainer policy — nothing under
go/.Test plan
dev2-go(pre-existing matrix reds on the line are unrelated)75bd272dapplied cleanly