Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
feat(ci): announce a published nightly on the Version Packages PR #133
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.
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
base: main
Are you sure you want to change the base?
feat(ci): announce a published nightly on the Version Packages PR #133
Changes from all commits
8936111File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Unchanged since last review] This
.replaceAll(/\n{3,}/g, "\n\n")is unconditional across the whole body whenever a region was present, not scoped to the seam the removal left behind. If the PR description contains an intentional 3+ newline run elsewhere (e.g. inside a fenced code block that isn't indented, or deliberate spacing), every future republish silently collapses it — content unrelated to the managed region gets rewritten. Restricting the collapse to the boundary immediately around the removed region (or tracking the removal offsets) plus a regression test for unrelated blank-line runs would close this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[New] (Related to a suppressed, non-thread comment on the prior Copilot review; I traced it through
stack-breadcrumb.cjsto confirm.) This places theregion at the true end of the body only as long as nothing else re-lays that body afterward. If the Version Packages PR is also part of a stack that carriesregions (viacarryForward/carryBackwardinstack-breadcrumb.cjs), the next stack reconcile callscanonicalizeBody, which rebuilds the body asbreadcrumb → ownDescription(body) → carried regions (sorted).ownDescriptiononly stripsandregions — it doesn't know about— so the nightly region survives embedded inside "description" and gets reassembled *before* the carriedblocks, no longer at the end of the body. The workflow header (and this file's own doc comment) note that "a Version Packages PR can be in a stack" and that the two region types "never rewrite" each other's markers, but that only covers this script not touching/content — it doesn't coverstack-breadcrumb.cjs's own canonicalizer moving the nightly region once carried regions exist. The requirement text says the region "SHALL be placed at the end of the pull request body" unconditionally; this is the one path where that can be violated, and it isn't covered by either test suite (nightly-breadcrumb.test.cjsonly tests the reverse direction: this script not disturbing an existing stack region).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[New] (Related to a suppressed, non-thread comment on the prior Copilot review, which I independently confirmed by reading the code.) This only matches on
pull.head.ref === branch; it never checkspull.base.ref. GitHub allows multiple open PRs from the same head branch to different base branches, so if a second PR is ever opened fromchangeset-release/maintargeting a non-mainbase (e.g. for testing), thegh apiquery in the workflow (head=${OWNER}:changeset-release/main, nobase=filter) can return both, and.find()here would silently pick whichever comes first — potentially not the real Version Packages PR — and PATCH its body instead. Adding apull.base.ref === "main"check here (orbase=mainin thegh apiquery) would close this.Uh oh!
There was an error while loading. Please reload this page.