From 9a914859f5a1ea669e841ad617e72a2a00779af5 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Thu, 20 Aug 2026 22:12:55 -0700 Subject: [PATCH 1/2] fix(ci): publish a nightly only from a commit Validate passed on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `release-cli-nightly.yml` and `validate.yml` both listened to `push: branches: [main]`, so they ran in parallel and a nightly could reach npm before CI had formed any opinion about the commit. Observed on 3f114d6: Validate failed at 00:24:27 and @taskless/cli-nightly@0.11.0-20260821002453x3f114d6 published 26 seconds later, taking dist-tags.latest. That particular failure was the OpenSpec hygiene gate rather than a broken build — which is the argument for this change, since nothing distinguished "red for bookkeeping" from "red because the tests fail" and it published on both. Move the nightly onto `workflow_run` on `Validate`, `types: [completed]`. The absence of a verdict is now the absence of a run: there is no "has not reported yet" state to misread, because nothing exists until Validate has reported. The gate job carries a four-clause condition — a positive `conclusion == 'success'`, `event == 'push'` (the actual fork boundary, since a fork's own default branch may also be called `main`), `head_branch == 'main'`, and `head_repository.full_name == github.repository` — and every checkout takes an explicit `ref: workflow_run.head_sha`, because `github.sha` under `workflow_run` is the default-branch tip rather than the tested commit. Both jobs assert HEAD equals that sha: an empty `ref:` makes actions/checkout fall back to the default branch and succeed, which would publish an unvalidated commit while gate 2 deduped against the wrong sha. The credential-free gate / OIDC publish split is unchanged, as is the Version Packages merge, which is an ordinary push to main that Validate runs on and gate 1 still declines. The cost is latency — a nightly now waits the full Validate wall clock — and that is the intended trade. Adds the requirement "A nightly is published only from a commit that passed validation" to openspec/specs/cli-nightly-builds/spec.md, the change having been archived. `validate.yml` gains a comment noting that its `name:` is now an interface — `workflows:` matches the display name, not the path, so renaming it retires the nightly with nothing turning red. Fixes #127 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3 --- .github/workflows/release-cli-nightly.yml | 183 ++++++++++++++++++++-- .github/workflows/validate.yml | 5 + openspec/specs/cli-nightly-builds/spec.md | 38 ++++- 3 files changed, 214 insertions(+), 12 deletions(-) diff --git a/.github/workflows/release-cli-nightly.yml b/.github/workflows/release-cli-nightly.yml index c4ef638..22b961a 100644 --- a/.github/workflows/release-cli-nightly.yml +++ b/.github/workflows/release-cli-nightly.yml @@ -7,14 +7,94 @@ # packages/cli/package.json is never changed, so @taskless/cli's own version # history contains releases and nothing else. # -# PUSH TO main, NEVER A PULL REQUEST. This is a security property, not a -# convenience. A PR-triggered publish would invert the split release-cli.yml -# exists to maintain: contributor-authored text would flow into a job holding an -# OIDC identity, and UNREVIEWED code would be published to npm under the -# @taskless scope. Building from main means the failure cannot arise — the only -# code that can be published is code that already merged. (A PR-side trigger is -# also measurably stale: a changeset is written early and the implementation -# lands after it, by 7 and 11 commits on the two branches measured for D1.) +# A VALIDATED COMMIT ON main, NEVER A PULL REQUEST. This is a security property, +# not a convenience. A PR-triggered publish would invert the split +# release-cli.yml exists to maintain: contributor-authored text would flow into +# a job holding an OIDC identity, and UNREVIEWED code would be published to npm +# under the @taskless scope. Building from main means the failure cannot arise — +# the only code that can be published is code that already merged. (A PR-side +# trigger is also measurably stale: a changeset is written early and the +# implementation lands after it, by 7 and 11 commits on the two branches +# measured for D1.) +# +# TRIGGERED BY `Validate` FINISHING, NOT BY THE PUSH ITSELF (issue #127; the +# requirement is "A nightly is published only from a commit that passed +# validation" in openspec/specs/cli-nightly-builds/spec.md). Publishing has to +# mean "this commit builds, lints, typechecks and tests cleanly." A second +# workflow listening to the same `push` event only means "this commit exists": +# Validate and this file ran in PARALLEL. `workflow_run` is the only trigger +# GitHub offers that fires after another workflow's verdict. +# +# 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, taking dist-tags.latest. Read the caveat honestly: that failure +# was the OpenSpec hygiene gate rather than a broken build, so those bytes were +# probably fine. That is the argument FOR this change, not against it — nothing +# in the old arrangement could tell "red for bookkeeping" from "red because the +# tests fail", and it published on both. +# +# 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. +# +# THE ABSENCE OF A VERDICT IS THE ABSENCE OF A RUN, and that is the property to +# protect above the mechanism. There is no "Validate has not reported yet" state +# for this file to misread, because nothing here comes into existence until +# Validate has reported. The three answers are structurally distinct: Validate +# passed (this run's jobs execute), Validate did not pass (this run exists with +# every job skipped), Validate never reported (no run at all). A gate that +# polled for the check from inside this workflow would have to tell "not started +# yet" from "passed" using the same absent answer, and would be wrong in the +# direction that publishes. +# +# FOUR CONDITIONS ON THE GATE JOB, EACH LOAD-BEARING. Do not collapse them: +# +# * `conclusion == 'success'` — a POSITIVE test. `!= 'failure'` would admit +# `cancelled`, `timed_out`, `skipped`, `neutral`, `action_required`, and the +# null conclusion, i.e. six ways of not having passed read as passing. +# * `event == 'push'` — Validate also runs on every pull request, including +# from forks, and this is the boundary that keeps those out. It is what +# makes the `pull_request_target`-shaped hazard (.github/copilot-instructions.md) +# inapplicable here: no run of this workflow ever checks out a pull request +# head. +# * `head_branch == 'main'` — validate.yml's push trigger is already filtered +# to main, but a trust boundary must not be spread across two files, one of +# which is free to change its filter. Note it CANNOT stand alone: a fork +# whose own default branch is called `main` produces a Validate run with +# `head_branch: main`, which is why the `event` test above is the actual +# fork boundary and this one is the branch one. +# * `head_repository.full_name == github.repository` — redundant given +# `event == 'push'`, kept deliberately as a second, independent reason the +# fork case cannot reach a checkout. The cost is one line. +# +# `github.sha` IS NOT THE TESTED COMMIT under `workflow_run` — it is the tip of +# the default branch when the event fired, which on a busy day is a later commit +# that Validate has said nothing about. Every checkout here therefore carries an +# explicit `ref: github.event.workflow_run.head_sha`, and both jobs then assert +# that HEAD is that commit. That assertion is not ceremony: an EMPTY `ref:` +# makes actions/checkout fall back to the default branch and succeed, so a +# missing head_sha would publish an unvalidated commit with nothing reporting an +# error — the same fail-open shape gate 2 was already fixed for. Gate 2 is +# per-SHA, so a wrong SHA also silently stops deduplicating. +# +# THIS FILE ALWAYS RUNS FROM main's COPY. GitHub loads a `workflow_run` workflow +# from the default branch, whatever the triggering run was. Two consequences: an +# edit here cannot be exercised on its own pull request, so the first proof is +# the first qualifying push after it merges; and the workflow deciding to +# publish is reviewed separately from the code it publishes. +# +# `workflows: [Validate]` MATCHES validate.yml's `name:`, NOT ITS PATH. Renaming +# that string stops every nightly, permanently, with no error anywhere — gate 1 +# is false on most pushes anyway, so nobody would notice the silence. +# validate.yml carries a comment on its `name:` saying so; keep the two in +# agreement. +# +# Every Validate run on a pull request also produces a run of this workflow with +# all jobs skipped. That Actions-tab noise is accepted: a `branches:` filter on +# the trigger would hide it, but it matches `head_branch`, which a fork controls, +# so putting it there would dress a filter up as the security boundary it cannot +# be. Let the workflow run and decide inside it (CLAUDE.md). # # TWO GATES, IN THIS ORDER (design D4): # @@ -39,6 +119,14 @@ # workflow knows about the other, and neither has a rule naming the other's # commit. # +# That survives the move to `workflow_run` unchanged, and it survives it twice +# over. The Version Packages merge is an ordinary push to main, so Validate runs +# on it and this workflow is triggered exactly as before — and gate 1 is still +# false, because that merge consumed the changesets. And in the case where +# Validate does not run at all for some push, the nightly not running is already +# the desired outcome, so the failure mode of the new trigger points the same +# way as gate 1 does. +# # GATES ARE CREDENTIAL-FREE AND LIVE IN THEIR OWN JOB, exactly as in # release-cli.yml. The point is not saving CI minutes: it is that the # OIDC-capable `publish` job is never INSTANTIATED for a run that will not @@ -101,8 +189,13 @@ name: Release CLI Nightly on: - push: - branches: [main] + # Not `push`. See the header: a nightly may only be built from a commit + # `Validate` has already passed on, and this is the only trigger that fires + # after another workflow's verdict. `workflows:` matches validate.yml's + # `name:` — renaming it there silently retires this file. + workflow_run: + workflows: [Validate] + types: [completed] # No workflow-wide grants; each job asks for exactly what it needs. permissions: {} @@ -111,15 +204,52 @@ jobs: gate: name: "nightly gate" runs-on: ubuntu-latest + # GATE 0, and the only one that cannot be expressed as a step: was this a + # SUCCESSFUL Validate run, on a PUSH, to main, in THIS repository? Each + # clause is explained in the header; all four are required and none is + # implied by another. A job-level `if` is where this belongs — there is no + # workflow-level `if`, and `publish` needs `gate`, so a skipped `gate` + # skips the OIDC-capable job with it. + 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 permissions: contents: read # checkout only outputs: should_publish: ${{ steps.gate.outputs.should_publish }} short_sha: ${{ steps.gate.outputs.short_sha }} steps: + # `ref:` is MANDATORY under workflow_run. Without it checkout takes + # `github.sha`, which here is the default branch tip at event time, not + # the commit Validate tested. - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: + ref: ${{ github.event.workflow_run.head_sha }} persist-credentials: false # nothing here writes to git + - name: Confirm the checkout is the commit Validate passed on + env: + VALIDATED_SHA: ${{ github.event.workflow_run.head_sha }} + run: | + set -euo pipefail + + # An empty `ref:` is not an error to actions/checkout — it falls back + # to the default branch and succeeds. So "no head_sha in the payload" + # would otherwise present as a normal run against a commit nobody + # validated, and gate 2 would dedupe against the wrong sha at the same + # time. Absent must not read as fine. + if [ -z "$VALIDATED_SHA" ]; then + echo "::error::The workflow_run payload carried no head_sha; refusing to build a nightly from an unidentified commit." + exit 1 + fi + + actual=$(git rev-parse HEAD) + if [ "$actual" != "$VALIDATED_SHA" ]; then + echo "::error::Checked out ${actual} but Validate passed on ${VALIDATED_SHA}; refusing to build a nightly from a commit that was not validated." + exit 1 + fi + echo "Building from ${actual}, which Validate passed on." - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: 24 @@ -235,9 +365,42 @@ jobs: contents: read # checkout only id-token: write # OIDC → short-lived npm auth + build provenance steps: + # Same explicit `ref:` as the gate job, for the same reason, and asserted + # again here rather than trusted from there. This is the job that produces + # the bytes: `gate` proved that some job checked out the validated commit, + # not that this one did. - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: + ref: ${{ github.event.workflow_run.head_sha }} persist-credentials: false # publish authenticates via OIDC, not git creds + - name: Confirm the checkout is the commit Validate passed on + env: + VALIDATED_SHA: ${{ github.event.workflow_run.head_sha }} + GATE_SHORT_SHA: ${{ needs.gate.outputs.short_sha }} + run: | + set -euo pipefail + + if [ -z "$VALIDATED_SHA" ]; then + echo "::error::The workflow_run payload carried no head_sha; refusing to publish from an unidentified commit." + exit 1 + fi + + actual=$(git rev-parse HEAD) + if [ "$actual" != "$VALIDATED_SHA" ]; then + echo "::error::Checked out ${actual} but Validate passed on ${VALIDATED_SHA}; refusing to publish a commit that was not validated." + exit 1 + fi + + # The gates were evaluated against a short sha, and that same short + # sha is stamped into the version below. If the two jobs somehow saw + # different commits, the dedupe answer belongs to one of them and the + # tarball to the other. + actual_short=$(git rev-parse --short=7 HEAD) + if [ "$actual_short" != "$GATE_SHORT_SHA" ]; then + echo "::error::The gate job decided for ${GATE_SHORT_SHA} but this job holds ${actual_short}; refusing to publish." + exit 1 + fi + echo "Publishing from ${actual}, which Validate passed on." - uses: pnpm/action-setup@b906affcce14559ad1aafd4ab0e942779e9f58b1 # v4 - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 043ef61..315895d 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -1,3 +1,8 @@ +# THIS NAME IS AN INTERFACE, NOT A LABEL. `release-cli-nightly.yml` triggers on +# `workflow_run: workflows: [Validate]`, which matches this string and not this +# file's path — a nightly is only published from a commit this workflow passed +# on. Rename it and the nightly stops firing permanently, with no error +# anywhere and nothing turning red. Rename both together, or neither. name: Validate on: diff --git a/openspec/specs/cli-nightly-builds/spec.md b/openspec/specs/cli-nightly-builds/spec.md index bd6c0c8..faf6877 100644 --- a/openspec/specs/cli-nightly-builds/spec.md +++ b/openspec/specs/cli-nightly-builds/spec.md @@ -12,9 +12,9 @@ The CLI SHALL be published as `@taskless/cli-nightly` from commits on the defaul The nightly SHALL be built from the same source and the same build as the release it anticipates, differing only in the published package name and version. Its executable SHALL remain `taskless`, so every documented invocation works unchanged against a nightly. -#### Scenario: A push to the default branch with unreleased work publishes a nightly +#### Scenario: A validated push to the default branch with unreleased work publishes a nightly -- **WHEN** a commit is pushed to the default branch and release-pending metadata exists for the CLI +- **WHEN** a commit is pushed to the default branch, the repository's validation suite passes for that commit, and release-pending metadata exists for the CLI - **THEN** `@taskless/cli-nightly` SHALL be published for that commit #### Scenario: A pull request publishes nothing @@ -88,6 +88,40 @@ Every nightly version is a semantic-version prerelease, which a registry does no - **WHEN** `@taskless/cli-nightly` is installed with no version or tag specified - **THEN** the most recently published nightly SHALL be installed +### Requirement: A nightly is published only from a commit that passed validation + +A nightly SHALL be published only from a commit for which the repository's validation suite — build, lint, typecheck, and tests — has already reported success. A nightly asserts that the default branch works at that commit; a publish that races validation asserts only that the commit exists. + +The decision SHALL be driven by validation's reported outcome rather than by the event that produced the commit, and SHALL distinguish three states: validation succeeded, validation did not succeed, and validation has not reported. The third SHALL NOT be treated as the first. A non-successful outcome SHALL leave visible evidence that the nightly was considered and declined, so that "declined" is distinguishable from "never triggered". + +Success SHALL be tested for explicitly. An outcome SHALL NOT be accepted on the grounds that it is not a failure, because a validation run may end without either succeeding or failing. + +The commit built and published SHALL be the commit validation reported on, and SHALL NOT be inferred from the state of the default branch at the time the decision is made, which may have advanced. When that commit cannot be identified, the run SHALL fail rather than fall back to any other commit. + +Validation's outcome SHALL gate the publish regardless of why validation failed. No exemption is made for a failure the run's author judges cosmetic: nothing downstream can tell a repository-hygiene failure from a failing test suite, and a rule that publishes on some red states publishes on all of them. + +#### Scenario: Failing validation publishes nothing + +- **WHEN** validation does not succeed for a commit on the default branch +- **THEN** no nightly SHALL be published for that commit +- **AND** the declined publish SHALL be observable + +#### Scenario: An inconclusive validation outcome is not a success + +- **WHEN** validation ends without succeeding — cancelled, timed out, skipped, or otherwise inconclusive +- **THEN** no nightly SHALL be published + +#### Scenario: The published commit is the validated commit + +- **WHEN** further commits reach the default branch while validation of an earlier commit is still running +- **THEN** the nightly SHALL be built from the commit validation reported on, not from the newer tip + +#### Scenario: An unidentifiable commit fails the run + +- **WHEN** the commit validation reported on cannot be determined +- **THEN** the run SHALL fail +- **AND** no nightly SHALL be published + ### Requirement: A nightly build is bounded by pending release metadata and by the commit Two gates SHALL decide whether a nightly is built, evaluated in this order. From fb737cd894cdb63de6d762da74d2821da6bb5b1f Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Thu, 20 Aug 2026 23:24:29 -0700 Subject: [PATCH 2/2] fix(ci): compare the gate's short sha as a prefix, not by re-abbreviating MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Raised in review on #132. `git rev-parse --short=` 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. Test the gate's short sha as a prefix of the full validated sha instead. Every abbreviation of a commit, at any width, is a prefix of its full sha, so the comparison 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. Passing the full 40-char sha through instead, as the review suggested, is not available: the version format is `-x`, so the gate's short sha has to exist and has to be the stamped one. Also corrects the gate job's own comment, which claimed `--short=7` was "A FIXED abbreviation length" — the same overclaim that produced the bug, sitting six lines above where the short sha is computed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3 --- .github/workflows/release-cli-nightly.yml | 30 ++++++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/.github/workflows/release-cli-nightly.yml b/.github/workflows/release-cli-nightly.yml index 22b961a..b950b14 100644 --- a/.github/workflows/release-cli-nightly.yml +++ b/.github/workflows/release-cli-nightly.yml @@ -280,12 +280,19 @@ jobs: echo "Pending changeset(s):" echo "$pending" | sed 's/^/ - /' - # A FIXED abbreviation length. `git rev-parse --short` scales the + # A PINNED abbreviation length. `git rev-parse --short` scales the # length with the size of the repository, so an unpinned length would # eventually produce an 8-character sha that no longer matches the # 7-character suffix of every nightly published before it — gate 2 # would stop deduping, silently, at a moment unrelated to any change # here. + # + # Pinned, not fixed, and the difference is worth knowing: `--short=` + # sets a MINIMUM width and git still lengthens past it to keep the + # abbreviation unambiguous. It holds the floor steady, which is what + # gate 2's suffix match needs; it is not a guarantee of exactly seven + # characters, so nothing downstream may assume the width (raised in + # review on #132 — see the publish job's prefix test). short_sha=$(git rev-parse --short=7 HEAD) echo "short_sha=$short_sha" >> "$GITHUB_OUTPUT" @@ -395,9 +402,24 @@ jobs: # sha is stamped into the version below. If the two jobs somehow saw # different commits, the dedupe answer belongs to one of them and the # tarball to the other. - actual_short=$(git rev-parse --short=7 HEAD) - if [ "$actual_short" != "$GATE_SHORT_SHA" ]; then - echo "::error::The gate job decided for ${GATE_SHORT_SHA} but this job holds ${actual_short}; refusing to publish." + # + # TESTED AS A PREFIX, NOT RE-ABBREVIATED AND COMPARED. `--short=` + # is 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. Two jobs abbreviating the SAME commit can therefore disagree — + # and a string comparison would fail the run with "the gate decided + # for X but this job holds Y" while nothing whatsoever is wrong + # (raised in review on #132). A prefix test cannot do that: every + # abbreviation of this commit, at any width, is a prefix of its full + # sha. It is also the invariant actually wanted — "the short sha about + # to be stamped into the version names THIS commit" — rather than a + # proxy for it. + if [ -z "$GATE_SHORT_SHA" ]; then + echo "::error::The gate job published no short_sha; refusing to publish a version stamped from an unknown commit." + exit 1 + fi + if [ "${VALIDATED_SHA#"$GATE_SHORT_SHA"}" = "$VALIDATED_SHA" ]; then + echo "::error::The gate job decided for ${GATE_SHORT_SHA}, which does not name ${VALIDATED_SHA}; refusing to publish." exit 1 fi echo "Publishing from ${actual}, which Validate passed on."