feat(ci): announce a published nightly on the Version Packages PR - #133
feat(ci): announce a published nightly on the Version Packages PR#133thecodedrift wants to merge 1 commit into
Conversation
thecodedrift
added
the
skip-changeset
There was a problem hiding this comment.
Pull request overview
Adds a post-publish nightly build breadcrumb to the pending Version Packages PR.
Changes:
- Adds a least-privileged breadcrumb workflow job.
- Implements breadcrumb parsing, rendering, upsert logic, and tests.
- Updates OpenSpec design, requirements, and tasks.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Summary | Review comments |
|---|---|---|
openspec/changes/nightly-cli-builds/tasks.md |
Adds task 4.14. | No final comments. |
openspec/changes/nightly-cli-builds/specs/cli-nightly-builds/spec.md |
Adds announcement requirements and scenarios. | No final comments. |
openspec/changes/nightly-cli-builds/design.md |
Documents design D11. | No final comments. |
.github/workflows/release-cli-nightly.yml |
Adds the post-publish PR annotation job. | Critical: Full-body updates can lose concurrent changes; use re-fetch-and-merge or coordinate writers. |
.github/scripts/nightly-breadcrumb.test.cjs |
Tests parsing, rendering, replacement, and edge cases. | No final comments. |
.github/scripts/nightly-breadcrumb.cjs |
Implements version parsing and breadcrumb upsertion. | Moderate: Global newline normalization can alter unrelated user-authored content; limit it to removed-region boundaries and add a regression test. |
Suppressed comments (3)
.github/scripts/nightly-breadcrumb.cjs:183
- This helper places the nightly region at the end only for the body it currently receives. If the Version Packages PR also has carried
<!-- PR:N -->regions, the existing stack canonicalizer treats<!-- nightly -->as description and appends carried regions after it, so the next stack reconcile/carry moves nightly away from the required final position. Preserve this marker in the shared canonical layout and cover the carried-region case.
function upsertRegion(body, version) {
const region = renderRegion(version);
const description = stripRegion(body);
return description.length === 0 ? region : `${description}\n\n${region}`;
.github/workflows/release-cli-nightly.yml:386
- The
headfilter does not constrain the base branch. GitHub permits multiple open PRs from the same head branch to different bases, so a PR fromchangeset-release/maintargeting another branch could be returned and selected for this PATCH instead of the Version Packages PR. Addbase=mainto the query (or validatepull.base.refin the selector) before writing.
gh api "repos/${REPO}/pulls?state=open&head=${OWNER}:changeset-release/main" > nightly-pulls.json
.github/workflows/release-cli-nightly.yml:357
- This workflow deliberately has no cross-run concurrency, but this job writes the single shared “latest nightly” region. Two pushes can publish concurrently; if the older run reaches this job after the newer run, its PATCH will leave the Version Packages PR pointing at the stale build.
needs: publishonly orders jobs within one run, so serialize or cancel stale breadcrumb jobs (or reject an older stamped version before writing).
breadcrumb:
name: Link the nightly on the Version Packages PR
needs: publish
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| .replaceAll(ALL_REGIONS_PATTERN, "") | ||
| .replaceAll(/\n{3,}/g, "\n\n") |
| gh api -X PATCH "repos/${REPO}/pulls/${PULL_NUMBER}" \ | ||
| -f body="$(cat nightly-body.md)" > /dev/null |
After a nightly publishes, the open `changeset-release/main` pull request gains a `<!-- nightly -->` region at the end of its body naming the build: the install line, the commit it was built from, and the time it was built. Reviewers of that PR are exactly the audience for a build of the work its changesets describe. The region is removed and re-appended on every publish, so a hand-deleted region comes back, repeated publishes replace rather than accumulate, and a body someone has reordered converges. It never touches the `<!-- stack -->` region stack-breadcrumb.yml may maintain on the same body. A THIRD JOB, holding `pull-requests: write` and no credential — never a step in `publish`, which would then hold both an npm OIDC identity and the ability to rewrite pull request text. It `needs: publish`, so a suppressed nightly never reaches it, and the version arrives as a job output: the stamp already encodes the build time and the sha, so both are parsed back out rather than read from a second clock. No open Version Packages PR exits 0 — the branch exists only while changesets are pending — but a failed `gh api` query still fails the step. Note: the issue writes the install line as `npx @taskless/cli@<version>`. Nightlies publish as `@taskless/cli-nightly`, and that version does not exist under the released name, so the region names the package that will actually install. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
413c039 to
8936111
Compare
|
Rebased onto The The spec content is re-homed accordingly:
The diff against Re-verified after the rebase: |
|
@claude /review |
|
Claude finished @thecodedrift's task in 5m 43s —— View job Review mode: incremental — read 2 prior review thread(s) before reviewing.Both prior threads (from
The prior review also had 3 suppressed (collapsed, non-thread) Copilot comments — lower confidence, never posted as inline threads, so they don't carry
Findings (5 inline comments posted)
Everything else — version parsing/rendering, the upsert idempotency, the I did not run lint/typecheck/tests myself — CI ( |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| return ( | ||
| text | ||
| .replaceAll(ALL_REGIONS_PATTERN, "") | ||
| .replaceAll(/\n{3,}/g, "\n\n") |
There was a problem hiding this comment.
[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.
| run: | | ||
| set -euo pipefail | ||
| gh api -X PATCH "repos/${REPO}/pulls/${PULL_NUMBER}" \ | ||
| -f body="$(cat nightly-body.md)" > /dev/null |
There was a problem hiding this comment.
[Unchanged since last review] This PATCHes the full body from a snapshot fetched earlier in the job (nightly-pulls.json). Both the changesets bot (pushing new release notes to this same PR) and stack-breadcrumb.yml (if this PR is stacked) can rewrite the body between the gh api GET here and this PATCH, and whichever writes last wins — the nightly region or the concurrently-written content can be silently dropped. A re-fetch-and-merge (or optimistic concurrency check) before the PATCH would close the window; at minimum this is worth an explicit acknowledged-risk note if it's accepted as-is.
| (pull) => | ||
| pull && | ||
| pull.head && | ||
| pull.head.ref === branch && |
There was a problem hiding this comment.
[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 checks pull.base.ref. GitHub allows multiple open PRs from the same head branch to different base branches, so if a second PR is ever opened from changeset-release/main targeting a non-main base (e.g. for testing), the gh api query in the workflow (head=${OWNER}:changeset-release/main, no base= 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 a pull.base.ref === "main" check here (or base=main in the gh api query) would close this.
| # answers and only one of them is fine. | ||
| breadcrumb: | ||
| name: Link the nightly on the Version Packages PR | ||
| needs: publish |
There was a problem hiding this comment.
[New] (Related to a suppressed, non-thread comment on the prior Copilot review.) needs: publish only orders jobs within a single workflow run. There's no concurrency group on this workflow (deliberately, per the header comment — but that reasoning covers publish's own idempotent npm view guard, not this job). Two pushes to main in quick succession start two independent runs; if the older run's breadcrumb job happens to execute after the newer run's, its PATCH will leave the Version Packages PR pointing at the stale build even though a newer nightly already published. Worth either a concurrency group scoped to this workflow (cancelling in-progress older runs) or comparing the stamped version's timestamp against whatever is already on the PR before writing.
| function upsertRegion(body, version) { | ||
| const region = renderRegion(version); | ||
| const description = stripRegion(body); | ||
| return description.length === 0 ? region : `${description}\n\n${region}`; |
There was a problem hiding this comment.
[New] (Related to a suppressed, non-thread comment on the prior Copilot review; I traced it through stack-breadcrumb.cjs to confirm.) This places the region 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 carries regions (via carryForward/carryBackward in stack-breadcrumb.cjs), the next stack reconcile calls canonicalizeBody, which rebuilds the body as breadcrumb → ownDescription(body) → carried regions (sorted). ownDescription only strips and regions — it doesn't know about — so the nightly region survives embedded inside "description" and gets reassembled *before* the carried blocks, 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 cover stack-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.cjs only tests the reverse direction: this script not disturbing an existing stack region).
When a nightly publishes, the open changesets "Version Packages" pull request (head
changeset-release/main, currently #74) gains a build-info region at the end of its body:Those reviewers are exactly the audience for a build of the work the pending changesets describe.
The issue writes the install line as
npx @taskless/cli@<version>. Nightlies publish under@taskless/cli-nightly(.github/scripts/nightly-pack.cjs, design D2 — the pack rewritesnamebefore packing), and@taskless/cli@0.11.0-20260818123456x05b3c88does not exist on npm. That line would 404, or at best send a reviewer to the last release.So the region names
@taskless/cli-nightly— the package that will actually install. Everything else matches the issue's format byte for byte. If you want the literal text instead, it is one constant innightly-breadcrumb.cjs.How it is built
A third job,
breadcrumb, holdingpull-requests: writeand no credential. Thepublishjob holds an OIDC identity authorized to publish under the@tasklessscope; addingpull-requests: writeto it would widen what a compromised step there can reach from "publish a package" to "publish a package and rewrite pull request text" — including the text of the PR that gates the next release. Same boundary the file already draws betweengateandpublish, drawn once more. Noid-token, no environment, no contents write beyond checkout-read.needs: publishis what "block on a successful publish" means mechanically — a job whose dependency was skipped does not run, so a suppressed nightly (either gate false) never reaches it.The version arrives as a job output, and every fact is parsed back out of it. The stamp is
<n.m.k>-<yyyymmddhhmmss>x<sha>, so it already carries the build time and the commit. A freshDate.now()would print a time that disagrees with the version on the line above it, and a freshgit rev-parsewould print a sha the published tarball does not carry. The "stamped exactly once" rule is already load-bearing here (--print-version); this is its third consumer and it obeys it.No open Version Packages PR exits 0.
changeset-release/mainexists only while changesets are pending, and a nightly can publish in the seconds before changesets opens it — a cosmetic breadcrumb must never fail a run that already published to npm. That is deliberately not the same branch as a failed API call:gh apiis allowed to exit non-zero and fail the step, with no|| echo '[]'fallback collapsing the two (the same fail-open closed in gate 2 by task 4.13).gh api -X PATCH repos/{owner}/{repo}/pulls/<n>, nevergh pr edit— its GraphQL path is broken by the Projects (classic) deprecation (CLAUDE.md).Upsert semantics, and what the tests cover
The region is removed and re-appended at the end rather than replaced in place, so it converges from any starting state.
.github/scripts/nightly-breadcrumb.test.cjs(16 tests) covers:<!-- stack -->— that region is left byte-for-byte alone across two publishes, and a stack region containing the word "nightly" is not mistaken for oneNot reusing
stack-breadcrumb.cjsIts
REGION_PATTERNhardcodes the namestackandupsertCarriedRegionis keyed to PR numbers. Neither generalizes to a singleton region under another name, and widening them would make a working, well-tested file serve two callers with different invariants. Fresh zero-dependency helpers in the same style, with a.test.cjssibling picked up byvalidate.yml'snode --test .github/scripts/*.test.cjs.Spec
openspec/changes/nightly-cli-buildsis still open, so this adds D11 todesign.md, task 4.14, and a requirement (A published nightly is announced on the pending release pull request) to thecli-nightly-buildsdelta.Verification (all run locally in the worktree)
pnpm lintpnpm typecheckpnpm testnode --test .github/scripts/*.test.cjspnpm openspec validate --all --strictnode .github/scripts/openspec-visibility.cjsAlso smoke-tested the script end to end against fixture
pulls.jsonpayloads: an existing region is replaced, an empty list exits 0 withchanged=false, and an error object exits 1.Conflict heads-up
The PR for #127 is being written in parallel and also touches
.github/workflows/release-cli-nightly.yml(it changes how the workflow triggers). Edits here are localized — anoutputs:block onpublishand one appended job — but whichever merges second will need a rebase.CI-only, no user-facing release note →
skip-changeset. Per.github/workflows/changeset.ymlthe check is advisory now; the label suppresses the warning rather than unblocking a merge.Fixes #128