Skip to content

feat(ci): publish unreleased main as @taskless/cli-nightly - #122

Open
thecodedrift wants to merge 13 commits into
openspec/nightly-1-workflow-splitfrom
openspec/nightly-2-publish
Open

feat(ci): publish unreleased main as @taskless/cli-nightly#122
thecodedrift wants to merge 13 commits into
openspec/nightly-1-workflow-splitfrom
openspec/nightly-2-publish

Conversation

@thecodedrift

@thecodedrift thecodedrift commented Aug 20, 2026

Copy link
Copy Markdown
Member

Stack (root → tip):

PR 2 of 3 for nightly-cli-builds. Adds the nightly publish itself: a pack script, its unit tests, and release-cli-nightly.yml.

Shape

  • .github/scripts/nightly-pack.cjs — pure exported helpers (selectProposedVersion, buildNightlyVersion, applyNightlyIdentity, hasNightlyForSha, isValidVersion) plus a main() that rewrites packages/cli/package.json to @taskless/cli-nightly and the stamped version, packs, and restores the manifest in finally. It asserts the official semver grammar before stamping, so an invalid version is never produced rather than being caught downstream.
  • release-cli-nightly.yml — push to main; a credential-free gate job, then publish under npm-autopublish. The gate/publish split is the same property release-cli.yml relies on: the OIDC identity only exists for a run that will actually publish.
  • README section, extended changeset (same file — the stack merges forward), .gitignore entries.

Against this repo's real state it produces @taskless/cli-nightly@0.11.0-20260820051459x773226a. Tarball verified: name and version rewritten, bin still {"taskless":"./dist/index.js"}, 13 optionalDependencies intact, and git status packages/cli/package.json clean afterwards.

Five things the proposal had wrong, found by building it

The first two were already corrected on the branches below; the last three are new and each would have broken a real run.

  1. Gate 1 was specified as a directory listing. .changeset/ is never empty — README.md and config.json are permanent — so it would have reported "pending" on every push forever, including the Version Packages merge it is supposed to self-handle. Now the require-changeset.yml rule: any .changeset/*.md other than README.md.

  2. The x separator's justification was wrong about the mechanism. The leading-zero rule bites only when the separator is a .; a bare concatenation stays valid semver, because the timestamp's leading digit is never 0. What concatenation actually produces is a 21-digit numeric identifier past exact double precision — 202608181234560123456 reads back as 202608181234560140000, and n === n+1 — so it destroys the chronological ordering the timestamp exists for, silently. The test now covers both alternatives against the semver grammar rather than only the dotted one.

  3. npm view --json on a 404 prints an error object to stdout and exits non-zero. So the natural $(npm view … || echo '[]') yields object-then-[] and crashes JSON.parse. The fallback has to replace the capture, not append to it. This is exactly the bootstrap-day state, when the package does not exist yet.

  4. npm view <pkg> versions --json returns a bare string when exactly one version exists — the state immediately after the manual bootstrap publish. Unhandled, the gate would have skipped every build until a second version landed.

  5. git rev-parse --short auto-scales its length with repo size. Unpinned, it would eventually emit 8 characters and stop matching 7-character suffixes, silently disabling dedupe. Pinned to --short=7, and the same value is passed to the pack script so the gate and the stamp cannot disagree.

What is verified, and what is not

Executed: the pack end-to-end and the resulting tarball's manifest; the version computation against real changeset status output; both gates as shell logic, including the npm view 404 path and the "only README.md and config.json" case; YAML parse of all 10 workflows.

Reasoned only: anything requiring the environment or OIDC — the npm-autopublish branch policy, the trusted-publisher binding, the real publish, and --tag latest actually moving on npm. This workflow cannot run end to end until the two maintainer actions happen, and the first real run is the only proof of them.

Before this merges

Two maintainer actions, in this order:

  1. npm-autopublish needs its deployment branch policy restricted to main. Done — verified as custom_branch_policies: true with exactly one policy, branch: main.
  2. @taskless/cli-nightly must be published once manually from the packed tarball, not the package directory (publishing a directory burns the name on a placeholder 0.0.0 — the trap release-vale.yml documents), then a trusted-publisher binding registered against release-cli-nightly.yml and the npm-autopublish environment. The workflow header carries the exact commands.

Registering that binding is also the first direct evidence of whether npm bindings are environment-scoped — which is what determines whether PR 3's Vale move invalidates its six existing bindings. That is why the nightly goes first.

Verification

pnpm lint clean · pnpm typecheck 1/1 · pnpm test 602 passing · openspec validate --all --strict 24/24 · node --test .github/scripts/*.test.cjs 122 passing

The change is deliberately not archived — PR 3 is the tip.

Refs #111

Refs OSS-34

thecodedrift and others added 11 commits August 18, 2026 23:25
Adds the nightly-cli-builds change: publish @taskless/cli-nightly from
main, and split release.yml into one workflow per release design.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
Swaps the last two units of the nightly stack. The nightly now lands
before Vale moves to `npm-autopublish`, rather than after.

The reason is the trusted-publisher risk the proposal already names.
npm bindings can be scoped to a GitHub environment, and 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. That trades a working release path for a convenience.

The nightly exercises the same environment, the same handshake, and the
same trusted-publishing model on a package where failure is free:
`@taskless/cli-nightly` is new, nothing resolves it, and a failed first
publish blocks nobody. Whatever the binding model turns out to require
is learned there. By the time Vale moves, the destination is proven.

Stated generally, since it outlives this change: prove a new credential
path on something disposable before migrating something that works onto
it.

Both human-gated prerequisites move up accordingly — the environment and
the `@taskless/cli-nightly` binding are now needed before the nightly
unit rather than before the Vale one. The archive moves to the Vale PR,
which is now the tip.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
Two of these were claimed as fixed in the PR description and were not
actually in the artifacts — an implementer following the tasks would
have reproduced both bugs the description said were handled.

Select the proposed version by package name, never by index.
`changeset status` returns an array of every package it releases, and
`[0]` is the CLI only while the CLI is the sole managed package. The six
vale packages already sit in the changesets ignore list, so a second
managed package is not hypothetical. Taking `[0]` would stamp a nightly
with another package's version — a wrong version that publishes cleanly
and looks plausible. Now stated in the task, the design, and the spec,
with a scenario.

Record why there is no tag bookkeeping. `latest` is correct for every
version this package produces, so there is no channel to maintain and no
state to drift. The reviewer's case — a publish that lands while the tag
move does not — is accepted rather than mitigated: the window closes at
the next nightly, and gate 2 skipping a re-run of identical bytes is
right on its own terms. Written down, because "we thought about this"
and "we did not think about this" are indistinguishable from silence.

Scope the PR 1 changeset to PR 1. The stack merges forward, where
CLAUDE.md requires each unit to extend the changeset with what it landed
rather than the base promising everything up front.

Also: the group cross-reference in 1.8 was left pointing at the
pre-reorder numbering, and task 0.3 asked to confirm a "currently
published" name is "unclaimed".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
`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
…ds PR 2

Three findings from the re-review, the first two of which would have
shipped as bugs.

`.changeset/` is never empty. It permanently holds `README.md` and
`config.json` — `changesets init` writes both and nothing removes them —
so a literal emptiness test answers "pending" on every push, including
the Version Packages merge that D4 claims it self-handles. The gate is
"any `.changeset/*.md` other than `README.md`", which is exactly how
`require-changeset.yml` already counts them; reuse that rule rather than
inventing a second one.

Group 3 could not run where it was placed. It publishes the tarball from
the pack script in task 4.1, and 4.1 is part of PR 2 — the PR group 3 is
meant to precede. Introduced when the units were reordered to put the
nightly ahead of the Vale move: before that swap the binding step came
after the nightly PR, so the script existed. It now says what it always
meant — open PR 2, run its pack script from that branch, publish and
bind, then merge. Vale's equivalent reads as a clean prerequisite only
because `vale-prepare.cjs` was already on `main`.

Also the third stale cross-reference from the same renumbering: the
Vale-bindings check is task 5.3, not 4.2.

Found by the first review round that could read whole files rather than
diff hunks, after the review workflow was fixed to check out the PR ref.
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.
Add release-cli-nightly.yml and the pack script behind it. Every push to
main with changesets pending publishes the CLI under a second name at
<next-version>-<yyyymmddhhmmss>x<short-sha>, so merged-but-unreleased work
is installable.

Two credential-free gates, in their own job, decide whether the publish job
exists at all: pending changesets (before any install) and whether the
commit already has a nightly. The rename happens at pack time, so the
committed manifest and @taskless/cli's version history are untouched.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
…y-2-publish

Reconciles two independent fixes of the same defect. Gate 1 was written
as a bare directory listing, which is never true because `.changeset/`
permanently holds `README.md` and `config.json`. That was found twice:
once by review on the proposal branch, and again by measurement while
building the gate here, because this branch was cut before the first fix
landed.

Both descriptions were right and each said something the other did not.
The resolution keeps this branch's wording — it is checked off and folds
in what building the thing actually taught — and grafts on the two
clauses only the other had: that `changesets init` writes both files and
nothing removes them, which is why they are permanent, and that a bare
emptiness test would fire on the Version Packages merge, the one case
the gate exists to handle.

Task 4.3's rationale also survives from this side, since it corrects the
proposal's stated mechanism: the leading-zero rule bites only if the
separator is a `.`, while a bare concatenation stays valid semver and
fails differently — a 21-digit numeric identifier past exact double
precision, which breaks ordering rather than validity.

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 adds the “nightly CLI” publishing path: on pushes to main with pending changesets, it packs the CLI as @taskless/cli-nightly with a stamped prerelease version and publishes it via OIDC trusted publishing in the npm-autopublish environment.

Changes:

  • Adds a reusable pack script (.github/scripts/nightly-pack.cjs) plus unit tests to stamp a nightly semver and pack without leaving packages/cli/package.json modified.
  • Introduces release-cli-nightly.yml with a credential-free gate job (pending changesets + dedupe by SHA) and a credentialed publish job.
  • Documents nightlies and ignores the generated tarball/status artifacts.

Reviewed changes

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

Show a summary per file
File Description
README.md Documents how to install/use @taskless/cli-nightly and collision caveats.
openspec/changes/nightly-cli-builds/tasks.md Marks PR2 nightly tasks as completed and records measured corrections.
openspec/changes/nightly-cli-builds/design.md Refines the “x separator” rationale with measured failure modes.
.gitignore Ignores nightly build artifacts (.nightly-dist/, nightly-status.json).
.github/workflows/release-cli-nightly.yml Adds the nightly gate+publish workflow using OIDC trusted publishing.
.github/scripts/nightly-pack.test.cjs Adds unit tests for version selection/stamping and dedupe logic.
.github/scripts/nightly-pack.cjs Implements version selection/stamping, manifest rewrite/restore, and npm pack.
.changeset/nightly-cli-builds.md Extends the changeset to include the nightly publishing feature.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/release-cli-nightly.yml Outdated
npm always includes README.md in a tarball regardless of `files`, so the
nightly was publishing @taskless/cli's documentation under a different
package name — install instructions for a package the reader did not
install, with nothing saying so. Someone arriving from a search would
follow them and never learn this is a prerelease of something else.

The pack swap now covers the README the same way it covers the manifest:
written, packed, restored in `finally`, so the committed file is never
left rewritten.

Deliberately minimal. It names the package, links to the real one for
documentation and support, explains what the version string encodes, and
carries the one warning a reader can act on destructively — that the
release and the nightly collide on the `taskless` executable and
installing both globally is unsupported. It does not restate anything
from the CLI's README, because a copy would have to be kept in sync with
a file it was copied from.
@thecodedrift
thecodedrift marked this pull request as ready for review August 20, 2026 06:58
@thecodedrift

Copy link
Copy Markdown
Member Author

@claude /review

@claude

claude Bot commented Aug 20, 2026

Copy link
Copy Markdown

Claude finished @thecodedrift's task in 2m 37s —— View job


Claude finished @thecodedrift's task

Reviewed the diff against origin/openspec/nightly-1-workflow-split across correctness, dropped-behavior, reuse/simplification, and altitude/conventions. All findings below were independently verified against the actual file contents.

  • Gather context (PR diff, workflow YAML, pack script, tests)
  • Correctness/security pass
  • Removed-behavior audit (nothing dropped)
  • Reuse/simplification/efficiency pass
  • Altitude & CLAUDE.md/STYLEGUIDE-CODE.md conventions pass
  • Post findings

Overall assessment

Solid piece of work — the design doc's five corrections (leading-zero vs. precision-loss, npm view 404-to-stdout, single-version bare string, pinned --short=7) are all real fixes and all are exercised by tests. No invariant from the base branch was weakened or dropped. Two issues below are worth fixing before merge; the rest are cleanup/simplification.

Findings

1. (Correctness, confirmed) The pre-publish npm view guard doesn't actually close the gate/publish race, because the nightly version isn't deterministic across runs
.github/scripts/nightly-pack.cjs:279 stamps the version with new Date() inside main(), called fresh every time the publish job's pack step runs. The workflow's safety comment (release-cli-nightly.yml:237-241) claims this guard is "the same way release-cli.yml and release-vale.yml guard" — but those two compute a content-derived version (read verbatim from committed package.json, or stamped once upstream under a concurrency: group), so a retry recomputes the identical string and the npm view check is meaningful. Here, two independent executions of the publish job for the same commit — e.g. a duplicate webhook delivery, or re-running a publish step that actually succeeded but whose HTTP response was lost — each call new Date() at a different instant and mint two different versions both ending in the same x<sha> suffix. Gate 2 (hasNightlyForSha) treats "already published" as any version with that suffix; the final guard at release-cli-nightly.yml:248 treats it as that exact string exists. These two checks don't agree, and the missing concurrency: group (deliberate, per D6) relies entirely on them agreeing. Net effect: the "one nightly per commit" invariant the whole x<sha> scheme exists to guarantee can be silently violated.

2. (Correctness, confirmed) The write/pack/restore isn't fully inside the try/finally the comment claims
.github/scripts/nightly-pack.cjs:298-310: the comment above ("the restore is in finally so a failed pack does not strand a rewritten manifest") only covers the spawnSync call. Both writeFileSync(packageJsonPath, ...) (298) and writeFileSync(readmePath, ...) (299) run before the try block starts (300). If the README write throws after the package.json write succeeds (e.g. ENOSPC/EROFS on that specific write), the exception propagates before finally is ever registered, and packages/cli/package.json is left rewritten to the nightly identity in the working tree — contradicting the "committed manifest is never left rewritten" guarantee the PR description itself asserts. Moving both writeFileSync calls inside the try closes the gap.

3. (Test coverage) The gate-2 stdin/JSON-parsing wrapper is untested
release-cli-nightly.yml:156-165 embeds a ~10-line node -e program (stdin listener, JSON.parse, process.exit) inside a bash heredoc inside YAML, wrapping the tested hasNightlyForSha. Only the pure function is covered by nightly-pack.test.cjs; the wrapper — including the SHORT_SHA env round-trip and the 404→'[]' substitution — is exercised only by pushing to main. Every other release script in the repo (vale-gate.cjs, vale-prepare.cjs, nightly-pack.cjs itself) is a standalone .cjs entry point instead. Worth doing the same here (e.g. a small nightly-gate.cjs with its own test), especially since vale-gate.cjs already solves the identical "treat a 404 as empty, not an error" problem via a single in-process fetch() with no subprocess — that pattern could replace the npm view CLI shell-out + heredoc entirely.

4. (Minor/cosmetic) Misplaced JSDoc
.github/scripts/nightly-pack.cjs:173-181: the doc comment for hasNightlyForSha sits directly above buildNightlyReadme's own doc comment and implementation, not above hasNightlyForSha itself (which is defined later, at line 217). A reader following the comment top-to-bottom lands on the wrong function. Likely leftover from reordering during editing.

5. (Simplification, non-blocking) Some duplication with the Vale release scripts

  • parseArguments/setOutput-style CLI parsing (nightly-pack.cjs:233-272) is now a third near-identical copy of helpers already in vale-prepare.cjs and vale-gate.cjs — plausibly worth extracting to a shared .github/scripts/*-util.cjs now that there are three.
  • main()'s write/pack/restore (see finding 2) duplicates the same stamp-and-pack shape as vale-prepare.cjs's per-platform loop, per design.md:38's stated intent to reuse that shape — but the two independently reimplement it and disagree on restore-on-failure behavior (vale-prepare.cjs has no try/finally around its npm pack calls today). Not a blocker for this PR, but a signal a shared helper would pay off for both.
  • pnpm --filter @taskless/cli build (release-cli-nightly.yml:207) runs before the much cheaper pnpm exec changeset status (line 215); reordering would fail fast on a bad changeset state without spending build minutes first.

Nothing else rose to the level of a concrete bug — argument parsing, regex escaping, the npm view 404 handling, and the shell quoting/env: routing for untrusted values all check out.
· Branch: openspec/nightly-2-publish

A nightly shipped @taskless/cli's skill and recipe content verbatim, so
an agent following it ran `npx @taskless/cli` — the released CLI. Someone
installs a nightly to exercise unreleased behavior and their agent
silently uses the released binary. Nothing errors; the instructions are
simply for a different package.

Adds a `nightly` build target beside `dev` and `self`, rewriting the
baked invocation to `npx @taskless/cli-nightly@<version>` through the
same `__TASKLESS_CLI__` define the other targets already use. Target
resolution moves to `scripts/build-target.ts` so it can be unit-tested
over an explicit environment rather than this process.

The version is computed ONCE and shared. `nightly-pack.cjs
--print-version` stamps it; the build reads TASKLESS_NIGHTLY_VERSION and
the pack takes `--version`, which is the only version input pack mode
accepts — it rejects `--status`/`--sha` so it cannot derive a second one.
Two `new Date()` calls a build apart would ship instructions naming a
version that was never published, so this is enforced rather than
observed. A missing or malformed version fails the build; falling back
to `npx @taskless/cli` would silently reintroduce the bug being fixed.

`build:nightly` emits to `dist`, unlike `dev`/`self`, because
`files: ["dist"]` is what npm packs — so it overwrites a local prod
build, which the comment says.

Also closes a fail-open in gate 2, found in review on #122. Any non-zero
exit from the version query landed in the "no nightly found" branch, so
unparseable output meant publish. Since the version carries a timestamp,
a re-run after a parse failure mints a different version for the same
commit and publishes it — two nightlies for one SHA, no error anywhere,
by the gate whose only job is suppression. Now three-way: present means
skip, parsed-and-absent means build, unparseable means fail. The 404
carve-out stays, because on bootstrap day the package genuinely does not
exist.

The bootstrap block in the header gains the build step it was missing;
following it literally packed a tarball with no dist/.
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