Skip to content

Harden merge gate + prerelease guard, and de-prose comments (#188, #189) - #190

Merged
ptr727 merged 5 commits into
developfrom
harden-merge-gate-and-prerelease-guard
Jun 23, 2026
Merged

Harden merge gate + prerelease guard, and de-prose comments (#188, #189)#190
ptr727 merged 5 commits into
developfrom
harden-merge-gate-and-prerelease-guard

Conversation

@ptr727

@ptr727 ptr727 commented Jun 23, 2026

Copy link
Copy Markdown
Owner

Addresses two issues filed upstream from ptr727/ESPHome-NonRoot, plus a comment-hygiene pass requested in the same review session.

#188 — merge gate keeps getting misread as merge-ready

An agent read mergeStateStatus: CLEAN as "safe to merge" while a Copilot finding was still open. The contract was correct but easy to misapply.

  • Added an explicit "Merge Gate (read this first)" block at the top of PR Review Etiquette with a four-precondition ALL-must-hold checklist (required checks green, Copilot review on current head SHA, zero open/unresolved Copilot threads, explicit maintainer permission).
  • Restated the trap directly: CLEAN reflects only required statuses, never open bot review comments, so it is never sufficient on its own.
  • Repointed the loop's closing paragraph at the gate.

Carried verbatim into every derived repo, so the fix lands everywhere on re-sync.

#189 — prerelease guard *-* false-positives on SemVer2 build metadata

The public-release guard tested the raw SemVer2 string for *-*, which matches a hyphen anywhere — including legitimate SemVer2 build metadata after + (e.g. 1.7.0+gabc-def).

Call: the report is correct. The guard's intent is to reject prerelease versions; in SemVer2 the prerelease separator is the - in the core/prerelease segment, not anything in build metadata. Strip build metadata (${SEMVER2%%+*}) before the test. Latent today (NBGV emits no - in metadata) but removes the fragility. Verified: plain / +gabcdef / +gabc-def allowed; -g1234abc / -preview+gabc rejected.

Comment hygiene — stop the prose creep

Comments had been accreting a clause of rationale per editing iteration, drifting into multi-paragraph design narratives.

  • Strengthened the AGENTS.md "Comments" rule: one line is the default; a comment must come out same-length-or-shorter when nearby code is edited; over-grown blocks get cut to their single load-bearing point. Verbosity creep is named as the regression to prevent.
  • Applied it repo-wide: collapsed the multi-sentence prose blocks in the workflow YAML to their load-bearing why, and trimmed/deleted restate-the-code comments in CodeGen and NuGetLibrary plus the narrative _version.py docstring. Load-bearing CI rationale (version guards, concurrency exceptions, OIDC scopes, cache scoping) preserved, just de-prosed. Comment-only; line endings unchanged; all workflow YAML re-validated.

🤖 Generated with Claude Code

#188: Add an explicit four-precondition "Merge Gate (read this first)"
checklist to the PR Review Etiquette section so `mergeStateStatus: CLEAN`
can no longer be misread as merge-ready. Restate the trap directly -
CLEAN reflects only required statuses, never open bot review threads, so
it is never sufficient on its own. Repoint the loop's closing paragraph
at the gate instead of repeating the easy-to-misapply phrasing.

#189: The public-release prerelease guard tested the raw SemVer2 string
for `*-*`, which matches a hyphen anywhere - including legitimate SemVer2
build metadata after `+`. Strip build metadata before the test so a `-`
in the prerelease segment is what's checked, matching the guard's intent.
Latent today (NBGV emits no `-` in build metadata) but fragile.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 23, 2026 15:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens two cross-cutting template safeguards: (1) clarifies the PR review-loop contract so agents do not confuse mergeStateStatus: CLEAN with merge readiness, and (2) hardens the main public-release prerelease guard to avoid false positives from SemVer2 build metadata.

Changes:

  • Add an explicit "Merge Gate (read this first)" checklist to the top of the PR Review Etiquette contract in AGENTS.md.
  • Update the review-loop closing guidance to refer back to the new Merge Gate rather than repeating the previously misread wording.
  • Fix the public-release prerelease guard by stripping SemVer2 build metadata (+...) before checking for a prerelease separator (-).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
AGENTS.md Adds an explicit merge gate checklist and rewords the review-loop closeout to reduce misinterpretation of mergeStateStatus: CLEAN.
.github/workflows/build-release-task.yml Makes the prerelease guard SemVer2-correct by ignoring build metadata when detecting prerelease versions.

Comment thread AGENTS.md
Strengthen the AGENTS.md "Comments" rule against verbosity creep: one
line is the default, a comment must come out same-length-or-shorter when
code near it is edited, and over-grown blocks get cut to their single
load-bearing point. Comments had been accreting a clause of rationale per
iteration.

Apply the rule across the repo: collapse the multi-sentence prose blocks
in the workflow YAML (release/publish/merge-bot/upstream-check/docker/
executable/datebadge/pypi/test) to their load-bearing why, and trim or
delete restate-the-code comments in CodeGen and NuGetLibrary plus the
narrative _version.py docstring. Load-bearing CI rationale (version
guards, concurrency exceptions, OIDC scopes, cache scoping) is preserved,
just de-prosed. Comment-only changes; line endings unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ptr727 ptr727 changed the title Harden merge gate wording and prerelease guard (#188, #189) Harden merge gate + prerelease guard, and de-prose comments (#188, #189) Jun 23, 2026
Copilot review threads can be resolved, but issue-level Copilot comments
have no resolve action - so "zero open threads" could be satisfied while
an issue-level finding is still outstanding, reintroducing the misread
the gate prevents. Reword condition 3 in terms of all findings: threads
resolved and issue-level comments triaged/replied to.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 23, 2026 16:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Comment thread CodeGen/AssemblyInfo.cs Outdated
Split('+', '-') strips prerelease and build metadata; the example now
covers both separators so it can't read as prerelease-not-handled.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Comment thread .github/workflows/build-pypilibrary-task.yml Outdated
"non-develop never publishes" was wrong (main publishes too) and out of
place in a version-compute step; keep only the version/--pre logic.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

@ptr727
ptr727 merged commit 150353e into develop Jun 23, 2026
20 checks passed
ptr727 added a commit that referenced this pull request Jun 23, 2026
Promotes the current `develop` head to `main` for release.

Contents:
- **#190** — Harden merge gate + prerelease guard, and de-prose comments
(closes #188, #189).
- **#186** — Update codegen files (already on develop).

Standard release promotion: merge (not squash) so develop's history
carries to main.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@ptr727
ptr727 deleted the harden-merge-gate-and-prerelease-guard branch July 7, 2026 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants