fix(ci): publish a nightly only from a commit Validate passed on

[thecodedrift]

Release CLI Nightly and Validate both listened to push: branches: [main], so they ran in parallel. A nightly could reach npm before CI had formed any opinion about the commit — publishing asserted only "this commit exists on main", not "this commit builds, lints, typechecks and tests cleanly".

This is observed, not theoretical. On 3f114d6, Validate failed at 00:24:27 and @taskless/cli-nightly@0.11.0-20260821002453x3f114d6 published 26 seconds later at 00:24:53. It is on npm right now holding dist-tags.latest.

The honest caveat: that failure was the OpenSpec hygiene gate, not a broken build, so those bytes were probably fine. That is the argument for this change rather than against it — nothing in the old arrangement could tell "red for bookkeeping" from "red because the tests fail", and it published on both.

The mechanism

The nightly now triggers on workflow_run against Validate, types: [completed]. workflow_run is the only trigger GitHub offers that fires after another workflow's verdict.

The property this buys is structural rather than mechanical: the absence of a verdict is the absence of a run. There is no "Validate has not reported yet" state for the nightly to misread, because nothing comes into existence until Validate has reported. The three outcomes are distinguished by construction:

Validate	Nightly
passed	a run whose jobs execute, then the two existing gates decide
did not pass (any conclusion)	a run exists with every job skipped — visible evidence it was considered and declined
never reported	no run at all, and no publish

That mattered here because the repo has been bitten by this exact shape before: gate 2's old \|\| versions='[]' fell back to "no nightly found" on any unreadable registry response, so a re-run stamped a fresh timestamp and published a duplicate. Under workflow_run the "unknown" state is not something this workflow has to classify — it never runs.

The gate job's four-clause condition

if: >-
  github.event.workflow_run.conclusion == 'success'
  && github.event.workflow_run.event == 'push'
  && github.event.workflow_run.head_branch == 'main'
  && github.event.workflow_run.head_repository.full_name == github.repository

• conclusion == 'success' is a positive test. != 'failure' would admit cancelled, timed_out, skipped, neutral, action_required, and the null conclusion — six ways of not having passed, read as passing.
• event == 'push' is the actual fork boundary. Validate runs on every pull request including fork PRs, and — this is the trap — a fork whose own default branch is called main produces a Validate run with head_branch: main. Branch alone would let it through. This clause is also why the pull_request_target-shaped hazard in .github/copilot-instructions.md does not apply: no run of this workflow ever checks out a pull request head.
• head_branch == 'main' is kept anyway, because validate.yml's own branches: filter lives in a different file that is free to change. A trust boundary spread across two files is one edit from being gone.
• head_repository.full_name == github.repository is redundant given event == 'push', kept as a second independent reason the fork case cannot reach a checkout. One line.

A job-level if is the right home: there is no workflow-level if, and publish needs: gate, so a skipped gate skips the OIDC-capable job with it. The credential-free gate / OIDC publish split is unchanged.

github.sha is not the tested commit

Under workflow_run, github.sha is the default branch tip at event time — on a busy day, a later commit Validate has said nothing about. Both checkouts now carry an explicit ref: ${{ github.event.workflow_run.head_sha }}, and both jobs then assert HEAD equals it.

That assertion is not ceremony. An empty ref: is not an error to actions/checkout — it falls back to the default branch and succeeds. So a missing head_sha would publish an unvalidated commit with nothing reporting an error, and because gate 2 is per-SHA, dedup would silently break at the same moment. The publish job additionally asserts its short sha matches the one the gate decided on, since that same short sha is stamped into the published version.

The cost is latency, and it is the intended trade

The two workflows used to start together. A nightly now waits the full Validate wall clock (~1m10s on recent runs) before its gates even begin. Publishing an artifact nobody has checked is not faster, it is just earlier.

Alternatives rejected

Alternative	Why not
Add the build/lint/typecheck/test steps to the nightly's gate job	Duplicates Validate and pays for it on every push to main, including the overwhelming majority where gate 1 exits immediately. A second copy of the checks is a second thing to keep in agreement with the first.
Poll for the Validate check run from inside the gate job	The one that looks most reasonable, and the one the issue's failure mode is really about. It needs a token, where the design is that the gate job holds none; and it must distinguish "not started yet" from "passed", which the check-runs API cannot help with because both are the absence of a failure. Every retry budget is a guess, and the guess that is too short publishes.
needs: across workflows	Does not exist — needs is job-scoped. Merging the nightly into validate.yml would put an OIDC-capable job in the file that runs on every pull request, inverting the split the release workflows exist to maintain.
Keep push, add a branches: filter or a wait-for-check action	Both leave the publish decision keyed to the push rather than to the verdict.
A branches: [main] filter on the workflow_run trigger	It matches head_branch, which a fork controls, so it cannot be the security boundary — putting it there would dress a filter up as one. Accepted cost: every PR's Validate run also produces a nightly run with all jobs skipped. Actions-tab noise, no runner time.

Does this still fire for the Version Packages merge? Yes — and it fails safe twice

The merge of changeset-release/main into main is an ordinary push to main, so Validate runs on it and this workflow is triggered exactly as before. Gate 1 still handles it with no special case: that merge consumes every changeset, so .changeset/ holds only README.md and config.json, gate 1 is false, and no nightly is built — while release-cli.yml (untouched, still on push) sees a version npm has not got and publishes the real release.

The second layer: in any scenario where Validate does not run for a push, the nightly not running is already the desired outcome. The new trigger's failure mode points the same direction gate 1 does. Established by reading the payload semantics, not by observing a live Version Packages merge — see "not verified" below.

Interaction with a red main

Once this lands, a red main means no nightly publishes at all. That is intended, and it is the point of the issue rather than a side effect: a nightly's claim is "this is main, and it works." main running red while a forward-merging stack drains is a known, documented state, and there is nothing worth publishing during it. (main was red on the OpenSpec archive gate until 423363a cleared it 40 minutes before this PR was rebased — nothing here touches that.)

Is any failure mode silent?

Two, and both are named rather than hidden:

1. workflows: [Validate] matches validate.yml's name: string, not its path. Renaming that string retires the nightly permanently, with nothing turning red — and since gate 1 is false on most pushes, the silence looks normal. Mitigated only by documentation: validate.yml now carries a comment on its name: line saying the string is an interface. That is weaker than a check, and it is called out as chosen rather than missed.
2. A red main suppressing nightlies is itself quiet — no error, just no artifact. Accepted, because main being red is already a loud standing signal on its own.

Everything else fails closed and loudly: an unidentifiable commit, a checkout that does not match the validated sha, and a gate/publish sha disagreement all ::error:: and exit non-zero.

Spec

nightly-cli-builds was archived in 423363a while this branch was in flight, so there is no open change to amend and re-opening one would put main back into the state that archive just cleared. The requirement therefore goes straight into the live spec, openspec/specs/cli-nightly-builds/spec.md:

• New requirement — A nightly is published only from a commit that passed validation — with scenarios for a failing outcome, an inconclusive outcome, the tip advancing during validation, and an unidentifiable commit. It also states that no exemption is made for a failure judged cosmetic, which is the 3f114d6 case above.
• The existing publish scenario now names validation.
• openspec/specs/vale-binary-packages/spec.md is deliberately untouched.

Post-merge verification belongs on #131, which already carries the nightly's unobserved acceptance criteria; the items specific to this change are added there as a comment.

Verification (all run in the worktree, after the rebase onto a8b4037)

Command	Result
pnpm lint	pass, no output
pnpm typecheck	pass, 1/1 task
pnpm build	pass, 1/1 task
pnpm test	pass, 51 files / 630 tests
node --test .github/scripts/*.test.cjs	pass, 156/156 (includes the workflow-outputs checker main added in a8b4037)
pnpm openspec validate --all --strict	pass, 24/24
node .github/scripts/openspec-visibility.cjs	pass, every requirement visible in 24 specs
prettier --check on changed files	clean

Both workflow files were additionally parsed with yaml to confirm the trigger and the folded if resolve as intended.

Not verified, and cannot be from a PR: that the trigger actually fires. workflow_run loads the workflow from the default branch, so no run of this file happens until it is on main. The first qualifying push after merge is the proof — check that a run exists, that its gate job checked out the pushed commit rather than a later one, and that a failing Validate yields a run with every job skipped rather than no run at all.

Review round

claude[bot] reviewed and approved with one low-severity inline finding, fixed in the second commit.

The finding: git rev-parse --short=<n> sets a minimum width, not a fixed one — git lengthens an abbreviation whenever it is ambiguous in the object database of the job that runs it. The publish job re-derived --short=7 and string-compared it against the gate job's output, so two jobs abbreviating the same commit could disagree and fail the run with "the gate decided for X but this job holds Y" while nothing was wrong. It failed closed, so it was a spurious-failure risk rather than a publish risk.

The fix: test the gate's short sha as a prefix of the full validated sha. Every abbreviation of a commit, at any width, is a prefix of its full sha, so the guard cannot spuriously fail — and it is the invariant actually wanted ("the short sha about to be stamped into the version names this commit") rather than a proxy for it. An empty gate output is classified separately so it cannot pass as a zero-length prefix. Exercised in bash across matching-7, git-lengthened-8, full-sha, wrong-commit, empty, and glob-metacharacter inputs.

The reviewer's alternative — pass the full 40-char head_sha through instead — is not available here: the version format is <n.m.k>-<timestamp>x<short-sha>, so the gate's short sha has to exist and has to be the stamped one.

A second instance of the same wrong belief was six lines above where the short sha is computed: the gate job's own comment asserted --short=7 was "A FIXED abbreviation length". Corrected to say it pins the floor (which is what gate 2's suffix match needs) and that nothing downstream may assume the width.

Overlap with #133

#133 (feat/nightly-region-on-changeset-pr) adds a third breadcrumb job to release-cli-nightly.yml and an outputs.version on publish. This PR touches only the on: block, the gate job's if: and first two steps, the publish job's first two steps, and the header comment — so whichever merges second should have a small rebase. Note for that rebase: any job #133 adds needs the same explicit ref: github.event.workflow_run.head_sha on its checkout.

Fixes #127

[Copilot]

Pull request overview

This PR ensures the CLI nightly publish workflow only runs for commits on main that have already passed the repository’s Validate workflow, eliminating the race where a nightly could be published before CI finishes.

Changes:

• Switch Release CLI Nightly from push to workflow_run (completed) for the Validate workflow, with a job-level gate that only admits successful push runs on main in this repository.
• Ensure both the gate and publish jobs explicitly check out and assert the validated commit SHA (github.event.workflow_run.head_sha) to avoid silently publishing the wrong tip.
• Update OpenSpec tasks/spec/design documentation to capture the new requirement and operational implications (including the “skipped jobs” evidence model).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
openspec/changes/nightly-cli-builds/tasks.md	Adds tasks for switching the trigger to workflow_run and verifying post-merge behavior.
openspec/changes/nightly-cli-builds/specs/cli-nightly-builds/spec.md	Updates acceptance criteria to require validation success before publishing and documents key scenarios.
openspec/changes/nightly-cli-builds/design.md	Documents the design rationale, trust boundaries, and trade-offs for workflow_run-based gating.
.github/workflows/validate.yml	Adds an explicit comment that the workflow name: is an interface relied upon by the nightly trigger.
.github/workflows/release-cli-nightly.yml	Moves to workflow_run trigger, adds strict job-level gating, and pins checkout to the validated SHA with explicit assertions.

---

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

[thecodedrift]

@claude /review

[claude]

Claude finished @thecodedrift's task in 8m 14s —— View job

---

Review of PR #132

• Gather context (PR body, diff, changed files)
• Review .github/workflows/release-cli-nightly.yml
• Review .github/workflows/validate.yml
• Review openspec/specs/cli-nightly-builds/spec.md
• Post inline comments for concrete issues
• Post final top-level assessment

Summary

This is a well-reasoned, well-documented fix, and the diff matches what the PR body describes. I traced the workflow_run semantics against GitHub's actual webhook payload schema, walked both new "confirm the checkout matches the validated commit" bash steps line by line, and cross-checked the job-level if: gate's four clauses (conclusion, event, head_branch, head_repository.full_name) — field names, YAML folded-scalar joining, and &&/== precedence all check out. I found no correctness or security bugs.

Specific things verified:

• Fork/PR boundary: event == 'push' is genuinely the load-bearing clause (not head_branch, since a fork's own default branch can also be named main), and no code path in either job ever checks out a PR head — confirmed no pull_request_target-shaped hazard exists here.
• Stale-tip protection: every checkout is pinned to ref: github.event.workflow_run.head_sha, and both jobs assert HEAD matches it before doing anything else — an empty ref: would otherwise silently fall back to the default branch tip (actions/checkout's documented behavior), which the explicit empty-string check correctly guards against.
• Permissions: gate (contents: read) and publish (contents: read, id-token: write) are sufficient; neither job needs actions: read since neither downloads artifacts from the triggering run.
• npm-autopublish environment / branch policy: under workflow_run, github.ref reflects the default branch regardless of the explicitly checked-out sha, so the deployment branch policy (restricted to main) is trivially satisfied whenever this job runs at all — no gap there.
• Injection safety: attacker-adjacent values (head_sha, short_sha) are passed via env: into the bash steps rather than interpolated directly into the script body, avoiding the classic GitHub Actions expression-injection pitfall.

Inline comment posted

One low-severity, non-blocking nit on .github/workflows/release-cli-nightly.yml (lines 394-402): git rev-parse --short=7 HEAD is a minimum length, not fixed — under an astronomically unlikely hex-prefix collision, gate's and publish's independently-computed short shas for the same commit could differ by a character, tripping the new cross-job sha-agreement guard. This fails closed (spurious job failure, never a bad publish), so it's a reliability nit, not a correctness/security issue — flagged for awareness only.

Not flagged (already justified in the file's own comments, and I agree with the reasoning)

• workflows: [Validate] matching by workflow name: rather than path, and the fragility that creates if the name is ever changed — already called out explicitly in both files' headers as an accepted, documented risk.
• The inability to exercise this trigger from within the PR itself (workflow_run always loads from the default branch) — correctly called out as "not verified, and cannot be from a PR," with a clear post-merge verification plan.
• Redundancy of head_repository.full_name == github.repository given event == 'push' — intentional defense-in-depth, reasoning holds up.

Test adequacy

No new automated tests were added, but that's appropriate here: the changed logic lives in workflow YAML if: conditions and inline bash gates, which aren't unit-testable the way nightly-pack.cjs is (and the PR body already documents this limitation honestly rather than papering over it with a brittle test). The existing node --test .github/scripts/*.test.cjs suite (145/145 per the PR's verification table) is untouched and still relevant, since gate 2's dedup logic (hasNightlyForSha/parseVersionsResponse) is unchanged by this diff.

Overall: approve. No blocking issues found.
· Branch

[thecodedrift]

Re: @claude[bot] — "Claude finished @thecodedrift's task in 8m 14s —— [View job]… ### Review of PR #132 … Overall: approve. No blocking issues found."
#132 (comment)

Thanks — the one inline finding is fixed in 638c2b7, and it turned out to have a second instance.

The short-sha nit (fixed, not just noted). You were right that --short=<n> is a minimum width rather than a fixed one. The publish job no longer re-abbreviates and string-compares; it tests the gate's short sha as a prefix of the full validated sha, which cannot spuriously fail because every abbreviation of a commit is a prefix of its full sha at any width. I did not take the "pass the full 40-char head_sha through" option, and not only for diff size — the version format is <n.m.k>-<timestamp>x<short-sha>, so the gate's short sha has to exist and has to be the stamped one. The prefix test validates that value rather than replacing it.

The second instance: the gate job's own comment claimed --short=7 was "A FIXED abbreviation length" — the same wrong belief that produced the bug, six lines above where the short sha is computed. Corrected to say it pins the floor (what gate 2's suffix match needs) and that nothing downstream may assume the width.

On the three things you deliberately did not flag — the name:-string coupling, the untestable-from-a-PR trigger, and the redundant head_repository.full_name clause — that matches how they are intended, and I'd rather they stay documented risks than get quietly "simplified" later. Two remain genuinely unclosed and are called out in the PR body rather than resolved: the name: coupling is mitigated only by a comment, and the trigger cannot be exercised until this is on main.

Verification after the fix: pnpm lint, pnpm typecheck, pnpm test (51 files / 630 tests), node --test .github/scripts/*.test.cjs (145/145), pnpm openspec validate --all --strict (24/24), openspec-visibility.cjs (24 specs), prettier --check — all green. The new prefix test was additionally exercised in bash across matching-7, git-lengthened-8, full-sha, wrong-commit, empty, and glob-metacharacter inputs.

— AI Coding Agent

[thecodedrift]

Re: @copilot-pull-request-reviewer — "## Pull request overview — This PR ensures the CLI nightly publish workflow only runs for commits on main that have already passed the repository's Validate workflow…"

Acknowledged, with nothing to action: this review is a descriptive overview and explicitly reports "Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments." No findings, so nothing was changed on its account.

Recording it rather than dropping it silently, since a zero-finding review and an unread one look identical after the fact. One correction for anyone reading the file table: it lists openspec/changes/nightly-cli-builds/{tasks,design,specs}.md, which was the pre-rebase shape of this branch. That change was archived on main in 423363a while this PR was in flight, so the spec edit now lands in openspec/specs/cli-nightly-builds/spec.md and there are no openspec/changes/ edits at all.

— AI Coding Agent