Skip to content

fix(ci): keep the canary dist-tag from moving back to an older build - #3784

Open
armando-navarro wants to merge 1 commit into
angular:mainfrom
armando-navarro:a61-main-run-concurrency
Open

armando-navarro wants to merge 1 commit into
angular:mainfrom
armando-navarro:a61-main-run-concurrency

Conversation

@armando-navarro

Copy link
Copy Markdown
Collaborator

Fixes #3783

The publish job now publishes a canary only when its commit comes after the commit of the canary already on npm, and canary publishes run one at a time.

Changes

All in the publish job of .github/workflows/test.yml.

  • New step, "Skip a canary that is not newer than npm's". It runs for canary versions only, without the npm token.
    • It reads the current canary from npm's dist-tags endpoint, which is not CDN-cached. The package data that npm view reads is cached for 5 minutes.
    • It clones the repository's commit history with git clone --bare --filter=tree:0, which takes about half a second, and finds the commit named at the end of that canary's version.
    • Then it decides:
      • This build's commit comes after that commit: publish.
      • Same commit and same version: skip with a notice.
      • Same commit under a different version: publish. This can't happen today. It will after fix(build): name canaries above every published release of their major #3780, when the scheduled run rebuilds an unchanged main after a release.
      • This build's commit comes before that commit: skip with a warning.
      • Neither commit comes before the other, or the canary's commit can't be found: fail the job.
  • concurrency on the job. Canary publishes share the group canary-publish with cancel-in-progress: false and queue: max, so they run one at a time and a waiting one is not replaced. Each release run gets a group of its own.
  • The Publish step runs unless the new step asked to skip.
  • Releases and release candidates publish exactly as before.

Behavior to be aware of

  • The weekday scheduled run stops failing. On an unchanged main its publish fails with E403 today, as it did on 2026-09-30 and 2026-10-02. It now skips with a notice.
  • A failed check needs a person. If the canary dist-tag ever points at a build that is not from main, every canary run fails until the tag is moved to a build from main.
  • queue: max is recent. GitHub added it on 2026-05-07 (changelog). actionlint 1.7.12 does not know the key yet (Actionlint does not know about queue: key for concurrency rhysd/actionlint#657) and reports it as its only finding on this file.
  • Runs from before this change are not covered. A re-run uses the original run's commit, so I expect it to publish without the check.
  • The check trusts npm's answer. If npm returned the previous canary a few seconds after a publish, an older build queued right behind it would still publish. I have not measured whether npm lags like that.

Verification

I ran the new step's script, taken from the YAML, with bash -e against stand-in built packages. It cloned from GitHub and read npm for real, except where a case needed a different canary on npm.

  • A build of an earlier commit on main is skipped with a warning, also when its version is higher.
  • The same commit and version is skipped with a notice.
  • The same commit under a higher version publishes.
  • A build of a later commit publishes, also when npm's canary has the old 21.0.0-canary.a2662fe name.
  • A build from a commit that is not on main, a canary on npm from a commit that is not on main, an unknown hash, and an npm reply without canary each fail the job.
  • 21.0.0 and 21.0.0-rc.2 publish without the check.
  • A failing curl or git clone fails the job, and nothing publishes.
  • The clone holds only this repository's branches and tags, and after cloning the check needs no network.
  • npm publish --dry-run packs the same files, with the same checksum, with and without the clone in the workspace.

The concurrency queue can't be run locally. This PR's CI shows whether GitHub accepts the workflow file. The publish job itself only runs on main and on releases.

Every canary publish moved the `canary` dist-tag, whatever commit it
was built from. Two merges close together could publish out of order,
and a re-run of an older run could publish its build last.

The publish job now clones the repository's commit history and
publishes a canary only when its commit comes after the commit of the
canary on npm, or is the same commit under a new version. A build of
an earlier commit is skipped with a warning. Commits are compared
rather than versions, because a version can be higher for an older
commit.

Canary publishes also run one at a time, queued, so each check reads
what the previous publish left on npm. Release publishes are unchanged.

A scheduled run on an unchanged main now skips with a notice instead
of failing on the duplicate version.

Fixes angular#3783

@tyler-reitz tyler-reitz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. queue: max checks out against GitHub's workflow-syntax docs: it is a real key, single is the default, and max holds up to 100 pending runs and releases them FIFO, so the serialization this check depends on does hold. The sha extraction also works on the live dist-tag, 21.0.0-canary.20260930011755.sha-59d44e7 to 59d44e7, and on the older a2662fe shape. And publish is a leaf job, so a skip breaks nothing downstream.

One thing the body understates. It says a failed check needs a person, but not the consequence: in the two cases you tested as failures, a missing canary dist-tag and a canary built from a commit that is not on main, the step exits 1, so every later push to main shows a red run until someone moves the tag. Worth a line in the body, or a comment in the step itself, since whoever hits it will be reading the red run rather than this PR.

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.

The canary dist-tag can move back to an older build

2 participants