Skip to content

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

Open
thecodedrift wants to merge 3 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 3 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 2 commits August 19, 2026 22:35
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 on lines +155 to +171
versions=$(npm view @taskless/cli-nightly versions --json 2>/dev/null) || versions='[]'
if SHORT_SHA="$short_sha" node -e '
const { hasNightlyForSha } = require("./.github/scripts/nightly-pack.cjs");
let raw = "";
process.stdin
.on("data", (chunk) => { raw += chunk; })
.on("end", () => {
const versions = JSON.parse(raw.trim() || "[]");
process.exit(hasNightlyForSha(versions, process.env.SHORT_SHA) ? 0 : 1);
});
' <<< "$versions"; then
echo "should_publish=false" >> "$GITHUB_OUTPUT"
echo "A nightly ending in x${short_sha} is already published — nothing to do."
else
echo "should_publish=true" >> "$GITHUB_OUTPUT"
echo "Will build a nightly for ${short_sha}."
fi
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

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