fix(ci): carry PR-target/issue-quality harden onto dev2-go (#631) - #677
Conversation
…gates (lidge-jun#631) Hardens wrong-base PR drafting so the required check fails even when draft GraphQL soft-fails, and tightens bug-form Version/OS/reproduction gates. Go port: none — CI/workflows and issue-quality scripts only; no Go counterpart.
|
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a25d4236ea
ℹ️ 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".
| 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.
Keep draft ownership when checkpointing fails
If convertToDraft() succeeds but this checkpoint comment update fails, the surrounding catch treats the comment failure as a failed conversion and resets state.autoDraftedByBot to false even though the PR is now a draft. Whether the final update recovers or a later event reruns the workflow, the stored false state means correcting the target branch will no longer restore the PR to ready-for-review. Catch only the GraphQL mutation failure and preserve ownership after a successful conversion.
Useful? React with 👍 / 👎.
| } else if ( | ||
| !softPass && | ||
| isNewBugForm && | ||
| version !== null && | ||
| (isEmpty(version) || isRawPlaceholder(version)) |
There was a problem hiding this comment.
Reject removed required environment headings
When a contributor edits an otherwise valid new-form bug and deletes the entire Version heading, extractSection returns null and this version !== null guard skips the newly added missing-version error, so the report still validates; the Operating system branch repeats the same bypass. For isNewBugForm, treat null headings as missing too rather than only rejecting headings whose contents were cleared.
Useful? React with 👍 / 👎.
| core.setFailed( | ||
| `Pull request targets ${pr.base.ref}; allowed bases are ${ALLOWED_BASES.join(", ")}.` | ||
| ); |
There was a problem hiding this comment.
Exempt maintainer promotions from the failing target check
For the documented release flow where a maintainer promotes dev to main, wrongBase is true and this unconditional failure makes the required target check impossible to satisfy even though that PR is an allowed exception. Add a securely verified maintainer-promotion exception while continuing to fail ordinary feature PRs aimed at main.
AGENTS.md reference: AGENTS.md:L55-L56
Useful? React with 👍 / 👎.
| */ | ||
| function isTooTerseBugReproduction(text) { | ||
| if (isEmpty(text) || isPlaceholder(text)) return false; | ||
| if (hasConcreteDetail(text)) return false; |
There was a problem hiding this comment.
Require substance alongside concrete-detail tokens
A reproduction consisting only of 1, 502, ocx, or config passes because any digit or product keyword makes hasConcreteDetail() true and returns before the word-count check. These inputs remain non-actionable despite the new vague-reproduction gate, so require a minimum amount of surrounding content even when a concrete-detail token is present.
Useful? React with 👍 / 👎.
Summary
14dde562ontodev2-go(PR-target draft soft-fail + issue-quality Version/OS/reproduction gates).tests/ci-workflows.test.ts: keptdev2-go-only release/go-ci assertions after the shared enforce-pr-target tests.Go port
None — CI/workflows and issue-quality scripts only. No Go counterpart.
Test plan
bun test tests/ci-workflows.test.tsnode --test .github/scripts/enforce-pr-target.test.cjsnode --test .github/scripts/issue-quality.test.cjsdev2-go