refactor(ci): split release.yml so each workflow carries one design - #119
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the release CI configuration by splitting the former monolithic release.yml into dedicated workflows per trust model (CLI version PR, CLI publish, Vale publish), and updates docs/spec material to reflect the new workflow filenames and corrected changeset status --output output shape.
Changes:
- Split
release.ymlintorelease-cli-changeset.yml(Version Packages PR) andrelease-cli.yml(check + gated publish), and delete the originalrelease.yml. - Rename the Vale workflow file to
release-vale.ymland update references in Vale package READMEs and the Vale manifest comment. - Update OpenSpec
design.md/tasks.mdand add a changeset entry describing the workflow split.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/vale-win32-x64/README.md |
Update workflow reference from vale-binaries.yml to release-vale.yml. |
packages/vale-win32-arm64/README.md |
Update workflow reference from vale-binaries.yml to release-vale.yml. |
packages/vale-linux-x64/README.md |
Update workflow reference from vale-binaries.yml to release-vale.yml. |
packages/vale-linux-arm64/README.md |
Update workflow reference from vale-binaries.yml to release-vale.yml. |
packages/vale-darwin-x64/README.md |
Update workflow reference from vale-binaries.yml to release-vale.yml. |
packages/vale-darwin-arm64/README.md |
Update workflow reference from vale-binaries.yml to release-vale.yml. |
openspec/changes/nightly-cli-builds/tasks.md |
Mark PR1 tasks complete and update measured facts for later PRs. |
openspec/changes/nightly-cli-builds/design.md |
Correct the documented changeset status --output JSON shape and related guidance. |
.github/workflows/release.yml |
Removed legacy combined release workflow. |
.github/workflows/release-vale.yml |
Rename/reference updates to reflect the split CLI workflows. |
.github/workflows/release-cli.yml |
New: CLI check + gated publish workflow. |
.github/workflows/release-cli-changeset.yml |
New: Version Packages PR workflow (credential-free). |
.github/scripts/vale-manifest.json |
Update comment reference to release-vale.yml. |
.changeset/nightly-cli-builds.md |
New changeset documenting the workflow split and header correction. |
Suppressed comments (1)
.github/workflows/release-vale.yml:36
- The comment refers to the “STAMPED version” as what
release-cli.ymluses, butrelease-cli.ymlchecks the manifest (exact) version frompackages/cli/package.json; it doesn’t stamp versions the way Vale does. Rewording avoids implying the CLI workflow stamps versions too.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@claude /review |
|
Claude finished @thecodedrift's task in 4m 25s —— View job Review completeTodo list
SummaryThis is a clean, well-reasoned split of One real discrepancy found (posted inline on Second finding (posted inline on Everything else checked out:
Nothing here blocks merging; both findings are about the accuracy of the PR narrative/changeset text rather than the shipped workflow behavior, which looks correct and appropriately cautious. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Both findings were real and are addressed: the PR body now describes the Vale — AI Coding Agent |
Both inline comments are answered on their threads (the changeset overclaim is fixed in 78cf458; the — AI Coding Agent |
|
@claude /review |
|
Claude finished @thecodedrift's task in 5m 54s —— View job Review completeTodo list
SummaryI re-diffed the extracted One new finding, posted inline on Everything else checked out:
Nothing here blocks merging; the one new finding is a documentation/behavior-mismatch issue rather than a functional risk to the release itself. |
512110b to
d4a3b6a
Compare
cc8e4ca to
2225291
Compare
d4a3b6a to
2dd8494
Compare
2225291 to
e4f95f2
Compare
`release.yml` held two jobs with opposite trust properties behind a single
header. Split it, with no change to what any job does:
- `release-cli-changeset.yml` — the `version` job, keeping
`concurrency: release-${{ github.ref }}`. It consumes untrusted changeset
text and holds no credential and no OIDC identity.
- `release-cli.yml` — the `check` and `publish` jobs, deliberately in one
file: the credential-free gate is what keeps an OIDC-capable job from being
instantiated on an ordinary merge, and it must not be separated from the job
it protects. Deliberately no concurrency group; the gate makes a duplicate
run a no-op and the residual TOCTOU is treated as possibly-already-published.
- `vale-binaries.yml` → `release-vale.yml`, rename only; its workflow name and
concurrency group are unchanged, so its check name still reports.
Also corrects a header claim that had been false since the environment was
created: `npm-production` has a required reviewer and a protected-branches
deployment policy, so a release has always waited on a human approval the
comment said was not there.
Verified while implementing, and corrected in the change's design/tasks:
`changeset status --output=` writes an object whose per-package entries live
under `releases` (not a bare array), and an absolute `--output` path is
resolved against the working directory rather than ignored.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
The rename left `name: Vale Binaries` and `concurrency: vale-binaries` inside `release-vale.yml`, on the reasoning that task 1.5 asked for no behavior change. That preserved exactly the mismatch this split exists to remove: a file named for one thing announcing itself as another, in a change whose whole subject is making each workflow say what it is. Safe because branch protection requires only `Validate` (confirmed in task 0.1), so no required check is keyed to the old display name. The concurrency group is renamed with it; the only effect is that a run already in flight under the old group would not serialize against a new one, which is a single transition, not a standing property. Task 1.5 now says to do this, resolving its tension with 1.7 rather than leaving the next reader to rediscover it.
The release note claimed nothing about how the CLI is built, versioned, or published changes. Two things do: check/publish no longer share the release-* concurrency group, and the release runs as two workflow runs, so its check contexts are renamed. The publish mechanics are what is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
Removing the concurrency group was justified by a mitigation that lives in a different file. `release-vale.yml` runs `npm view` immediately before each `npm publish` and skips when the version is already there; `release-cli.yml` had a bare `npm publish` with no pre-check and no error handling. The header, design D6, and task 1.3 all claimed otherwise. The `check` job does query npm, but it is a separate job — which is exactly where the window is. A gate in another job cannot close the gap between its own answer and the publish. So the guard moves to where it can do the work, immediately before the publish. That is better than serializing rather than merely equivalent: it is idempotent instead of ordered, so it also absorbs a re-run against a version an earlier attempt already shipped, which a concurrency group does nothing about. Without it the losing run of a race fails with npm's "cannot publish over the previously published version" — a duplicate that reads as a broken release. The three places that asserted the old story now describe this one, and say that the omitted concurrency group is safe *because of* the guard rather than on its own.
e4f95f2 to
137b943
Compare
#131 `main` had been failing the `Check for unarchived OpenSpec changes on main` step in `validate.yml` on every push since 162afa0, because this change directory was still sitting under `openspec/changes/`. Issue #127 will make a red `main` suppress nightly publishing outright, so a red `main` had stopped being cosmetic. Every requirement in the change's spec delta shipped in #116, #119 and #122 — the workflow split, the pack script, the build target, both publish gates and the version stamp. What remained was one environment move and a set of verification steps, neither of which any delivered requirement depends on. Both move to #131 rather than holding `main` red. Removes the `vale-binary-packages` delta from the change. Applying it would have written into the published spec that the Vale workflow "SHALL publish without a human approval step, using the reviewer-free publishing environment", which is false: `release-vale.yml` still names `npm-production`, which has a required reviewer. A requirement describing deferred work is worse than no requirement, because a spec is what later readers trust. Also corrects task 3.2, briefly marked done on the strength of `npm view` showing a published version. That is evidence of the manual bootstrap in 3.1, not of a trusted-publisher binding; the two are independent. Refs #131 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
Stack (root → tip):
PR 1 of 3 for the
nightly-cli-buildschange. Behavior-preserving apart from one correction, and it exists so PRs 2 and 3 add one flow each to a file that already has one job.The split
release-cli-changeset.ymlversion— opens the Version Packages PRconcurrency: release-${{ github.ref }}release-cli.ymlcheck+publish, in one filenpm-production, approvalrelease-vale.ymlvale-binaries.ymlrelease.ymlis deleted. Each new file carries a header written for its own trust story rather than a copy of the original —release.yml's header is the reason its security properties survived this long, and splitting it without splitting the reasoning would have thrown that away.checkandpublishstay together deliberately: the credential-free gate is what keeps an OIDC-capable job from existing on an ordinary push, and separating it from the job it protects is the arrangement most likely to be broken by a later partial edit.release-cli.ymlhas no concurrency group, also deliberately, and the header says why — the gate makes a duplicate publish a no-op, and the residual TOCTOU is handled the way the Vale workflow already handles it, treating a publish failure as possibly-already-published.Verified equivalence
A parsed-config diff of old versus new shows
version,check, andpublishbyte-identical, with the same triggers and the samepermissions: {}. All 9 workflow files parse underjs-yaml.Two intentional behavior differences:
check/publishno longer share therelease-*concurrency group (D6).Release CLI Version PR / …andRelease CLI / …instead ofRelease / ….Vale Binaries→Release Vale,vale-binaries→release-vale), so its check context changes too and anything keyed on the old name (badges, notification rules,gh run list --workflow) stops matching.None of these is a required check. Branch protection on
mainrequires onlyValidate, fromci.yml— confirmed via the API, so no repo-settings change is needed.pr-check-openspec.yml,require-changeset.yml,stack-breadcrumb.yml, and.github/scripts/*.cjskey on nothing workflow-name-specific.The renamed Vale workflow is renamed through:
name:becomesRelease Valeandconcurrency:becomesrelease-vale. A file calledrelease-vale.ymlthat still announced itself asVale Binarieswould reproduce the naming mismatch this split exists to remove, so task 1.5 calls for both. Its whole header comment is preserved verbatim apart from therelease.yml→release-cli.ymlreference.The one behavior-adjacent correction
release.yml's header claimed "No required reviewers (fully automatic once the Version Packages PR merges), by design." That is false:npm-productionhasrequired_reviewers(reviewerthecodedrift) plus a protected-branches policy. A release has always waited on an approval the file denied existed. The new header states what is configured, and why.Three things measurement corrected in the proposal
Task group 0 is verification, and it contradicted the design in three places. All three are fixed in
design.mdandtasks.mdon this branch, since PR 2 codes against them:changeset status --outputreturns an object, not an array. It is{changesets: [...], releases: [{name, type, oldVersion, changesets, newVersion}]}. The bump isdata.releases.find(r => r.name === "@taskless/cli").newVersion. The proposal described a bare top-level array — so the review finding about not indexing with[0]was right about the hazard and wrong about the shape.--output=/tmp/x.jsonresolves against cwd to<cwd>/tmp/x.jsonand fails loudly withENOENTand exit 1. It does not silently write nothing. It would write to the wrong place silently only if that directory happened to exist.npm-autopublishalready exists, created 2026-08-18 — but with no protection rules and no deployment branch policy, meaning any branch may deploy to it. Group 2 is half-done and the missing half is the branch restriction. Noted inline on task 2.1 so nobody assumes it is complete. Inert today since no workflow references it; load-bearing the moment PR 3 does.@taskless/cli-nightlyis confirmed unclaimed on npm.Verification
pnpm lintclean ·pnpm typecheck1/1 ·pnpm test602 passing ·openspec validate --all --strict24/24 ·.github/scriptstests 113 passing across 4 files (turbo does not cover these, so they were run directly)Refs #111
Refs OSS-34
Built on top of #116
Proposal only — no implementation. Artifacts for a nightly build of the CLI published from
mainas a separate package, plus a split of the release workflows so each carries one design.What it proposes
@taskless/cli-nightly, published frommain, versionn.m.k-yyyymmddhhmmssx<sha>.changeset/empty (a directory listing, before any install), and is this SHA already builtnpm-autopublishenvironment for flows that publish without a human clickrelease.ymlandvale-binaries.yml, each with one trust storyThe design is argued in #111;
design.mdrecords the decisions with their reasoning rather than restating the conclusions.Built from
main, not from PRsThe original framing was "on PR update". That is what the title of #111 said until this proposal was written, and it does not survive contact with
release.yml's security model: a PR-triggered publish would route contributor-authored changeset text into a credentialed job, inverting the split that file exists to maintain, and would publish unreviewed code under the@tasklessscope.Tying builds to changeset edits does not work either. Measured on the #71→#106 stack,
add-vale-rule-enginehad 7 commits after its last changeset edit andagent-command-and-vale-authoringhad 11 — a nightly stamped at changeset-edit time would omit all of them while looking current.Delivery: stacked, merging forward, three PRs
release.yml's header, which claims "No required reviewers … by design" whilenpm-productiondemonstrably has a required reviewer.npm-autopublishand registering trusted publishing.npm-autopublish— tip, archives the change.The nightly deliberately precedes the Vale move. npm trusted-publisher bindings can be scoped to a GitHub environment. If the six
@taskless/vale-*bindings are, moving Vale first invalidates all of them and the next publish fails the OIDC handshake with no stored token to fall back on — trading a working release path for a convenience. The nightly exercises the same environment, handshake, and binding model on a package where failure is free: nothing resolves@taskless/cli-nightly, so a failed first publish blocks nobody. Stated generally indesign.md, because it outlives this change: prove a new credential path on something disposable before migrating something that works onto it.Two prerequisites no implementer can satisfy
Called out in the proposal body rather than buried in tasks, because they gate merges:
npm-autopublishenvironment must be created in repo settings — no required reviewers, branch policy onmain.@taskless/cli-nightlymust be published once manually and a trusted-publisher binding registered. Trusted publishing is per-package, and the package does not exist yet.Both sit as maintainer task groups before the nightly unit. A PR waits rather than merging with a dangling environment reference.
Gaps this surfaced in #111
Writing the design down found four things the discussion had left implicit, all captured as tasks:
changeset statusreturns an array; taking[0]breaks the day a second changesets-managed package appears. Filter by name.npm view … versionslists versions regardless of tag, so a version that published but failed to get tagged reads as already-built and is skipped forever. The Vale workflow handles that case explicitly; nightly's was left implicit.mainrequiresValidate; any branch-protection or breadcrumb reference to an old name silently stops reporting.openspec validate --all --strict: 24 passed, 0 failed.No changeset — a proposal ships nothing.
Refs #111
Refs OSS-34