Skip to content

[Split 3/3] Lockstep release pre-flight, release dry-run CI job, and publish workflow - #1335

Open
umair-ably wants to merge 5 commits into
integration/split-2-door-packagesfrom
integration/split-3-release-tooling
Open

umair-ably wants to merge 5 commits into
integration/split-2-door-packagesfrom
integration/split-3-release-tooling

Conversation

@umair-ably

@umair-ably umair-ably commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Third and final PR in the PDR-091b split stack (plan steps 14 packaging assertions, 16, 16b). Stacked on #1334 (integration/split-2-door-packages), which is its base, so the diff shows only this PR's changes. Siblings: ably-java#1234, ably-ruby#454/#455, ably-js#2296.

Today a release is a human dispatching package.yml, downloading an artifact zip and running dotnet nuget push by hand. Three packages that must land in dependency order, at one version, where a half-finished release cannot be undone and a published version can never be changed, is not a process anyone should be asked to execute correctly under pressure. This PR replaces it.

What this PR does

  • cake-build/tasks/release.cake — three tasks that bracket the pack, all wired into Package so an ordinary pack run enforces them, and all runnable standalone:

    task when asserts
    _Release_Preflight before anything is built source-only assertions (below)
    _Release_Verify_Files after the build, before the pack every <file src> glob resolves to a real file
    _Release_Verify_Packages after the pack id, version, one lib/<tfm> assembly per declared target, and the packed door → core pin

    The pre-flight is a dependency of _Version, not of _Package_Build_All, because _Version overwrites src/CommonAssemblyInfo.cs with the --version input — running after it would make the version assertion a tautology. As a side effect Package now fails loudly without --version instead of silently producing nothing.

  • nuget/*.nuspec — the two TODO(verify on the release dry run) comments from [Split 2/3] Add Ably.PubSub.Device and Ably.PubSub.Server door packages with side-declaring agents #1334 are removed and replaced with the verified answer; see "The [$version$] question, closed" below.

  • src/Ably.PubSub.Tests.DotNET/PubSub/PackagingSpecs.cs — 14 xunit specs duplicating the source-level assertions, so a PR that edits a nuspec fails the ordinary unit run without waiting for a packaging job. nuget/ is located by walking up from the test assembly looking for nuget/ably.pubsub.core.nuspec, not by counting directories.

  • .github/workflows/release-dry-run.yml — on pull_request and pushes to main / integration/**. Reads the version from the committed CommonAssemblyInfo.cs (never by hand — the point is to check what a release of this commit would do), runs the pre-flight, packs all three to dry-run-packages/, re-runs the post-pack assertions as their own named step, and uploads the .nupkgs.

  • .github/workflows/publish.yml — dispatch-only, version + dry_run (default true). Pre-flight → PackageUnityPackage → post-pack assertions → publish core → device → server, then a separate job creates the tag and release. Details under "Guardrails".

  • .github/workflows/package.yml deleted. It built the same artifacts with the same Cake targets and then asked a human to push them; that is now publish.yml with dry_run=true, plus assertions. package.cmd, package-unity.sh and build.sh are unchanged and still work.

  • DocsCONTRIBUTING.md's release process rewritten (bump one version in two files → PR → merge → dispatch dry run → dispatch for real; no manual push, no artifact zip), with a new "nuget.org Trusted Publishing setup" section; cake-build/README.md documents the new targets, --packageOutput and the -- separator; a README paragraph on why the exact pin means all Ably.PubSub.* references in a project move together.

The [$version$] question, closed

#1334 shipped the door → core pins as version="[$version$]" with an open item: nobody could confirm that nuget substitutes the $version$ token inside a dependency's version attribute rather than only in <version>, because there is no nuget.exe and no Mono on the machine that branch was prepared on.

It does. Proven twice — locally first, and then authoritatively by the release-dry-run CI run on this branch (green, 3m32s), which packs with the real nuget.exe on windows-2022 with real net462 heads, exactly as a release will:

Every nuspec <file src> glob resolves against the Release build output.
  Ably.PubSub.Core 2.0.0: 34 files, lib/net46, lib/netstandard2.0, lib/net6.0, lib/net7.0
  Ably.PubSub.Device: packed pin <dependency id="Ably.PubSub.Core" version="[2.0.0]" />
  Ably.PubSub.Device: packed pin <dependency id="Ably.PubSub.Core" version="[2.0.0]" />
  Ably.PubSub.Device: packed pin <dependency id="Ably.PubSub.Core" version="[2.0.0]" />
  Ably.PubSub.Device 2.0.0: 15 files, lib/netstandard2.0, lib/net6.0, lib/net7.0
  Ably.PubSub.Server: packed pin <dependency id="Ably.PubSub.Core" version="[2.0.0]" />   (x4, one per group)
  Ably.PubSub.Server 2.0.0: 18 files, lib/net46, lib/netstandard2.0, lib/net6.0, lib/net7.0
All 3 packages are at 2.0.0 and both doors pin Ably.PubSub.Core as [2.0.0].

Ably.PubSub.Core.2.0.0.nupkg    1,478,712 bytes
Ably.PubSub.Device.2.0.0.nupkg     43,281 bytes
Ably.PubSub.Server.2.0.0.nupkg     51,120 bytes

So: the TODO(verify on the release dry run) comments are gone from both nuspecs, replaced by the answer; and this is also the first end-to-end confirmation that #1333's and #1334's nuspecs pack correctly at all — every files glob resolved, both net462 heads built, and the lib/net46 assets are present in the core and server packages. The pin line is printed by _Release_Verify_Packages, so it is in the log of every release and every dry run from now on.

For the record, the local route (before CI existed on this branch): dotnet pack -p:NuspecFile=<nuspec> -p:NuspecBasePath=nuget -p:NuspecProperties="version=2.0.0;configuration=Release" packs a .nuspec with no Mono and no nuget.exe, and produced the same version="[2.0.0]". Two footnotes on it, since it is a useful thing to know: NuspecProperties must be set in the project file, not on the command line, or MSBuild eats the ; separator; and the two net46 output directories had to be faked (copies of the netstandard2.0 assemblies) to exercise the verifier over the full set locally.

dotnet pack on a nuspec being Mono-free does not move the dry run to Linux. Packing was never what needed Windows — the net462 build is. A Linux run would pack a core and a server silently missing their lib/net46 asset; _Release_Verify_Files fails rather than allowing that (guardrail 5 below is exactly that failure, observed). The Mono-free route is worth knowing about but is not wired into the build: one pack implementation is better than two.

Guardrails

Every one was broken deliberately and the failure observed. --version needs a -- separator because Cake reserves that flag: ./build.sh -- --target=Release.Preflight --version=2.0.0.

# Guardrail How it was broken Result
1 --version == all three CommonAssemblyInfo.cs attributes dispatched --version=2.0.1 4 problems, one per attribute plus version.txt: "src/CommonAssemblyInfo.cs AssemblyVersion is '2.0.0' but --version is '2.0.1'. Bump the version files in their own commit and release the merged version; the release input never overrides what is committed."
2 --version == unity/Assets/Ably/version.txt set version.txt to 2.0.1 "unity/Assets/Ably/version.txt is '2.0.1' but --version is '2.0.0'. Both version files must be bumped together: the .unitypackage ships from the same run as the NuGet packages…"
3 nuget/ holds exactly the lockstep set added nuget/io.ably.nuspec lists expected / found / unexpected, and names the 1.x case explicitly: "If this is a 1.x checkout (nuget/io.ably.nuspec), it cannot be released by this workflow"
4 door → core pin is the exact range changed the device pin to version="2.0.0" 3 problems, one per group: "…expected the exact range token "[$version$]". Square brackets are what make it exact: '2.0.0' without them is a minimum…"
5 every <file src> glob resolves ran it on macOS, where the net462 heads cannot build 5 problems naming exactly the 5 missing net46 assets and the resolved absolute path. This one caught a real condition rather than a synthetic one — nuget omits an unresolvable files entry silently, so this is the only thing standing between a stale build and an empty package
6 packed pin == [<version>] packed a door with the pin hard-coded to [9.9.9] 3 problems: "Ably.PubSub.Device.2.0.0.nupkg pins Ably.PubSub.Core as '[9.9.9]', expected '[2.0.0]'"
7 all three packages present post-pack verified an output dir with no .nupkgs "…was not produced. All 3 packages of the lockstep set are packed from one run; a missing one means the release is partial before it even reaches the registry."
8 --version is required and well-formed omitted it "No --version was supplied. The release version is an explicit input so that it can be checked against the committed version files"

Guardrails not exercised, because they need a real dispatch (see "Follow-up PR to main"): the skip-if-already-published query, the wait-for-core-to-be-listed loop, and the OIDC login. Their logic is small and self-contained but it has not run.

Publish-side design, for review rather than for verification:

  • Order. Core, then device, then server. A door pinned to [2.0.0] reaching the registry before Ably.PubSub.Core 2.0.0 is unresolvable for as long as the gap lasts, and nuget.org validates and indexes asynchronously, so after the core push the workflow polls https://api.nuget.org/v3-flatcontainer/ably.pubsub.core/index.json for up to 10 minutes and refuses to push the doors if the version never appears — the same shape as ably-ruby's release.yml.
  • Idempotence. Before each push, the flat container is queried (404 = the id has never been published) and an already-listed version is skipped, plus --skip-duplicate. A run that dies after the core push is completed by re-dispatching the same version. The GitHub release step is idempotent the same way (gh release viewupload --clobber, else create).
  • Rollback. Printed on every run, successful or not: NuGet cannot delete and the number can never be reused, so the guidance is dotnet nuget delete to unlist — doors first, then the core, the reverse of the publish order, so no door is ever resolvable without the core version it pins.
  • Permissions. permissions: {} at workflow level. The publish job takes contents: read + id-token: write; the release job is separate purely so contents: write is scoped to the only thing that needs it. Third-party actions are pinned to commit SHAs, persist-credentials: false throughout.

Trusted publishing / registry-side setup

Implemented with the official NuGet/login action, pinned to 8d196754b4036150537f80ac539e15c2f1028841 (v1.2.0). Confirmed from its README and from the nuget.org trusted-publishing docs:

  • The action takes one required input, user — the nuget.org username (profile name, not an email address) — and outputs NUGET_API_KEY. The job needs permissions: id-token: write. user is passed as ${{ secrets.NUGET_USER }}; it is not a credential, it is a secret only to keep the account name out of public logs.
  • The key is valid for one hour and each OIDC token buys exactly one key, so the login step sits immediately before the push rather than at the top of the job.
  • A policy binds Repository Owner + Repository + Workflow Filethe file name only (publish.yml, not .github/workflows/publish.yml) — plus an optional Environment, which we leave empty because this workflow uses none.
  • On the plan's open question — whether a brand-new package id needs an API key for its first publish: no, it does not. Policies have scopes which explicitly govern "publishing new packages" as distinct from "publishing new versions of existing packages", with a glob pattern to target which ids the policy covers. A policy scoped to allow new packages with a glob over Ably.PubSub.* is what claims all three names on the first dispatch. This contradicts the assumption in plan step 16b; nothing in the plan depends on it beyond not needing an API-key detour.
  • A policy on a private repository starts temporarily active for 7 days and becomes permanent on the first successful publish, because nuget.org needs the GitHub repository and owner IDs from a real token to lock the policy against repo-resurrection attacks. The 7-day window can be restarted at will. This repo is public, so it should not apply — worth watching for on the first dispatch anyway.
  • A policy is owned by a user or an organization, and applies to all packages owned by that owner. If an org-owned policy's creator leaves the org it goes inactive until they are re-added. Prefer an org-owned policy over one owned by an individual.
  • The policy matches the repository by name, so it must be created (or re-created) after the repo rename — which is what plan steps 16–18 already sequence. Creating it now against ably/ably-dotnet would mean redoing it.

What a repo admin has to do, once, after the rename: nuget.org → username → Trusted Publishing → add a policy owned by the account/org that owns the Ably.PubSub.* ids, Repository Owner: ably, Repository: ably-pubsub-dotnet, Workflow File: publish.yml, Environment empty, scope allowing new packages with glob Ably.PubSub.*; then add the NUGET_USER repository secret. If that cannot be arranged in time, the fallback is documented in a comment at the login step in publish.yml and in CONTRIBUTING.md: delete the login step, drop id-token: write, add a NUGET_API_KEY secret, use it in the push. Nothing else changes.

Note that this makes PDR-091b's "NuGet — nothing changes" line false for .NET, as plan step 16 anticipated. The 1.x maintenance line keeps publishing ably.io with an API key and is unaffected.

Follow-up PR to main

workflow_dispatch only reads workflow files from the default branch, and this work merges to integration/v2 until Phase 7. This PR does not touch main. The follow-up, as ably-ruby did in #455:

git fetch origin
git checkout -b release-tooling-on-main origin/main
git cherry-pick f48c7973 7f3658e2   # release-dry-run.yml, then publish.yml (+ package.yml removal)
# resolve: main has no cake-build/tasks/release.cake, so publish.yml's and
# release-dry-run.yml's Cake targets do not exist there. That is fine and
# deliberate - see below - but keep package.yml on main until 1.x stops
# releasing from it, i.e. drop the package.yml deletion from the cherry-pick.
git push -u origin release-tooling-on-main
gh pr create --base main --title "Cherry-pick the release workflows to main so they can be dispatched" --draft

Why it is safe to live on main while main is still 1.x. The pre-flight requires nuget/ to hold exactly ably.pubsub.{core,device,server}.nuspec. A 1.x checkout has nuget/io.ably.nuspec and none of those, so a dispatch against a 1.x ref aborts in the pre-flight with the message in guardrail 3, which names the 1.x case explicitly — before anything is built, packed or pushed. (On main itself the Cake target does not even exist yet, so it fails harder still.) release-dry-run.yml runs on pushes to main, so it would go red there until integration/v2 merges; either land it in the same follow-up as a known-red check, or hold it back and cherry-pick only publish.ymlrecommend the latter.

Claiming the package names (plan step 1), once the follow-up is on main and the trusted-publishing policy exists:

git checkout -b claim-package-names origin/integration/v2
# set 0.0.1-alpha.0 in src/CommonAssemblyInfo.cs (all three attributes)
# and unity/Assets/Ably/version.txt
git commit -am "Claim the package names with placeholder versions" && git push -u origin claim-package-names

gh workflow run publish.yml --ref claim-package-names -f version=0.0.1-alpha.0 -f dry_run=true   # read the output first
gh workflow run publish.yml --ref claim-package-names -f version=0.0.1-alpha.0 -f dry_run=false

# then unlist all three (doors first, then the core)
dotnet nuget delete Ably.PubSub.Server 0.0.1-alpha.0 --source https://api.nuget.org/v3/index.json --api-key <key> --non-interactive
dotnet nuget delete Ably.PubSub.Device 0.0.1-alpha.0 --source https://api.nuget.org/v3/index.json --api-key <key> --non-interactive
dotnet nuget delete Ably.PubSub.Core   0.0.1-alpha.0 --source https://api.nuget.org/v3/index.json --api-key <key> --non-interactive
git push origin --delete claim-package-names

One run claims all three names and proves auth, ordering, the wait-for-core loop and the skip logic. Then the two guardrail tests the plan asks for:

  1. Re-run the same versiongh workflow run publish.yml --ref claim-package-names -f version=0.0.1-alpha.0 -f dry_run=false. Expect SKIP Ably.PubSub.Core 0.0.1-alpha.0 is already on nuget.org three times and a green run. (Note: unlisted versions still appear in the flat container, so this holds after unlisting too — which is the behaviour we want.)
  2. Mismatched versiongh workflow run publish.yml --ref claim-package-names -f version=9.9.9 -f dry_run=false. Expect the pre-flight to abort with the four version-file problems from guardrail 1, and nothing pushed.

Verification

Run on macOS with the .NET SDKs at $HOME/.dotnet (6.0.428 / 7.0.410 / 8.0.424 / 9.0.317). No Mono, no MSBuild, no nuget.exe.

Command Result
dotnet build src/Ably.PubSub.NetStandard.sln -c Release succeeded, 0 warnings, 0 errors
./build.sh -- --target=Release.Preflight --version=2.0.0 passed — "--version, src/CommonAssemblyInfo.cs and unity/Assets/Ably/version.txt all say 2.0.0; nuget/ holds exactly the lockstep set and each door pins Ably.PubSub.Core as [$version$]"
the 8 guardrail breaks in the matrix above each aborted with the intended message; all reverted
dotnet pack -p:NuspecFile=… (Mono-free pack investigation) works; produced version="[2.0.0]" for the device door
./build.sh -- --target=_Release_Verify_Files --version=2.0.0 correctly failed on the 5 unbuildable net46 assets; passed once those directories were populated
./build.sh -- --target=Release.VerifyPackages --version=2.0.0 --packageOutput=dryrun-out passed over all three packages (see the log excerpt above)
./build.sh -- --target=Test.NetStandard.Unit.WithRetry --framework=net6.0 1139 total, 1120 passed, 19 skipped, 0 failed
Build.NetFramework, Test.NetFramework.*, ./package.cmd 2.0.0 not run — no Mono/MSBuild/nuget.exe locally; CI covers them
.github/workflows/*.yml all 8 parse under Ruby YAML.safe_load
release-dry-run on this branch (windows-2022, real nuget.exe) green in 3m32srun 33643588695. Pre-flight passed, every nuspec glob resolved, both net462 heads built, all three packages packed and asserted, packed pin [2.0.0]
publish.yml dispatch not rungh workflow run only reads workflow files from the default branch, so the real smoke test is the follow-up above

#1334's baseline was 1106 passed / 19 skipped; 1120 − 1106 = exactly the 14 new packaging specs, so nothing regressed.

Notes for reviewers

  • _Release_Preflight hangs off _Version, which is unusual and deliberate. _Version regenerates CommonAssemblyInfo.cs from the --version input, so any check placed after it compares the input against itself. Nothing else in the build depends on _Version, so this does not affect the Build.* or Test.* targets.
  • The pre-flight's file-glob check is a separate task on purpose. The version and nuspec assertions are source-only and run anywhere; the glob check needs a completed Release build of every head, including the net462 ones, and therefore fails by design on a machine without MSBuild. Folding it into Release.Preflight would have made the standalone pre-flight unusable on macOS and Linux.
  • The assertions are duplicated between release.cake and PackagingSpecs.cs. Deliberate: the Cake tasks are what guard the release, the specs are what fail a PR in seconds. Both are ~40 lines of the same three rules and they cannot drift far without one of them going red.
  • _Release_Verify_Packages asserts one lib/<tfm>/<Id>.dll per lib target declared in the source nuspec. That is the check that would have caught [Split 1/3] Restructure into Ably.PubSub.Core (assembly rename, drop Xamarin heads and push satellites, version 2.0.0) #1333's IO.Ably.DeltaCodec near-miss (a glob that stopped matching after a rename) automatically.
  • The .unitypackage is still incomplete until someone regenerates unity/Assets/Ably/Plugins/Ably.PubSub.Core.dll with ./unity-plugins-updater.sh 2.0.0 on a machine with Mono ([Split 1/3] Restructure into Ably.PubSub.Core (assembly rename, drop Xamarin heads and push satellites, version 2.0.0) #1333's open item). publish.yml will happily package whatever is in unity/Assets/Ably; there is no assertion on the Unity payload, and adding one is arguably a follow-up.
  • Package now hard-fails without --version instead of silently producing nothing. A behaviour change, and an improvement, but worth knowing if anything scripted relied on the no-op.
  • --packageOutput resolves relative paths against the repository root, not against Cake's working directory (which is cake-build/). This bit me; it is commented at the resolver.
  • The 10-minute wait for the core to be listed is a guess, copied from ably-ruby's 5 minutes and doubled because NuGet validation is slower than RubyGems'. If it proves too short the failure is safe — nothing further is pushed and re-running completes the release.
  • release-dry-run.yml runs on pushes to main as well as PRs, which means it will be red on main until integration/v2 merges. See the recommendation in the follow-up section: cherry-pick only publish.yml.

Where the stack deliberately stops

  • Nothing is published. No package name is claimed, no trusted-publishing policy exists, no version reaches nuget.org from this PR. The name claim needs the workflow on main and a registry-side policy first.
  • No repo rename (Phase 5), so features.yml still says repository-name: ably-dotnet and every nuspec URL still says ably-dotnet. The trusted-publisher binding is sequenced after the rename on purpose — it matches the repository by name.
  • No push to main or integration/v2 from this PR.
  • No ⚠️091d renames. Still in review; Phase 6 is a separate mechanical pass on the integration branch.

Remaining before GA, per plan.md: ably-common#361 merged (blocks #1334); integration/v2 branch protection (needs a repo admin); the 1.x maintenance branch cut before the integration merge; the migration guide (UPDATING.md); the 2.0.0-rc.1 prerelease through this workflow plus the clean-container consumer check that the exact pin resolves from the public registry; and the coordinated GA window keyed to the MAU pricing date.

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: ad847ccb-f9c4-4397-86e1-8eb3457a6b6d

📥 Commits

Reviewing files that changed from the base of the PR and between 11df62b and b2c9e77.

📒 Files selected for processing (26)
  • .github/workflows/features.yml
  • .github/workflows/package.yml
  • .github/workflows/publish.yml
  • .github/workflows/release-dry-run.yml
  • .github/workflows/run-tests-linux.yml
  • .github/workflows/run-tests-macos-mono.yml
  • .github/workflows/run-tests-macos.yml
  • .github/workflows/run-tests-windows-netframework.yml
  • .github/workflows/run-tests-windows.yml
  • CHANGELOG.md
  • CONTRIBUTING.md
  • README.md
  • UPDATING.md
  • cake-build/README.md
  • cake-build/build.cake
  • cake-build/helpers/utils.cake
  • cake-build/tasks/build.cake
  • cake-build/tasks/package.cake
  • cake-build/tasks/release.cake
  • nuget/ably.pubsub.core.nuspec
  • nuget/ably.pubsub.device.nuspec
  • nuget/ably.pubsub.server.nuspec
  • src/Ably.PubSub.Core/Ably.PubSub.Core.csproj
  • src/Ably.PubSub.Device/Ably.PubSub.Device.csproj
  • src/Ably.PubSub.Server/Ably.PubSub.Server.csproj
  • src/Ably.PubSub.Tests.DotNET/PubSub/PackagingSpecs.cs

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

umair-ably and others added 4 commits September 17, 2026 20:45
Three packages ship at one version and both doors pin the core with an
exact version range. Nothing in the build enforced any of that, so a
loose pin, a stale build or a version-file skew would first become
visible to a consumer after the version had been published and could
never be changed again.

cake-build/tasks/release.cake adds three tasks that bracket the pack:

  _Release_Preflight       source-only assertions: the --version input
                           equals all three attributes in
                           src/CommonAssemblyInfo.cs and
                           unity/Assets/Ably/version.txt; nuget/ holds
                           exactly the lockstep set with the expected
                           ids and the $version$ token; every dependency
                           group of every door pins Ably.PubSub.Core as
                           [$version$]; nothing depends on ably.io or
                           the 1.x push satellites.
  _Release_Verify_Files    every <file src> glob resolves to a real
                           file. nuget silently omits a files entry it
                           cannot resolve, so this is the last point at
                           which an empty or partial package can be
                           caught.
  _Release_Verify_Packages post-pack assertions against the produced
                           .nupkg files: id, version, one lib/<tfm>
                           assembly per declared target, and the packed
                           door -> core pin.

The pre-flight is a dependency of _Version rather than of
_Package_Build_All because _Version overwrites CommonAssemblyInfo.cs
with the --version input; running after it would make the version
assertion a tautology. Package now fails loudly without --version
instead of silently producing nothing.

The packed-pin assertion is what proves the open question from the door
packages PR: NuGet does substitute $version$ inside a dependency's
version attribute, and the packed pin comes out as [2.0.0]. Both door
nuspecs lose their TODO and say so.

The same source-level assertions are duplicated as xunit specs in
Ably.PubSub.Tests.DotNET/PubSub/PackagingSpecs.cs, so a PR that edits a
nuspec fails the ordinary unit run without waiting for a packaging job.
They locate nuget/ by walking up from the test assembly rather than by
counting directories.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The lockstep invariants live in the version files and the nuspecs, none
of which the compiler reads, and a NuGet version once published can
never be changed. This packs all three packages from every PR commit
and runs the pre-flight and the post-pack assertions over them, so a
version-file skew, a loosened pin, a renamed id or a nuspec files entry
that no longer resolves fails on the PR that introduced it rather than
on release day.

Windows, not Linux, because the core and server packages carry a
lib/net46 asset built by an old-style MSBuild head. `dotnet pack
-p:NuspecFile=...` does pack a nuspec Mono-free, so packing is not what
needs Windows - the net462 build is - and _Release_Verify_Files fails
loudly rather than letting a Linux run ship packages silently missing
that asset.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Today's release is a human downloading an artifact zip and running
`dotnet nuget push` by hand. With three packages that must land in
dependency order, at one version, and where a half-finished release
cannot be undone, that is not a process anyone should be asked to
execute correctly under pressure.

publish.yml is dispatch-only, takes `version` and `dry_run` (default
true), and:

  - runs the pre-flight before anything is built, so a mismatched
    version or a loosened pin aborts the run with nothing produced;
  - builds, packs and creates the .unitypackage in one job from one
    build, so the Unity asset and the .nupkg files can never come from
    different builds of the same version;
  - pushes core -> device -> server, and waits for the core version to
    be listed on nuget.org before the doors that pin it go out, because
    nuget.org validates and indexes asynchronously;
  - queries the flat container before each push and skips a version
    already published, so a run that fails partway is completed by
    re-running it with the same version rather than by hand;
  - creates the tag and GitHub release, with all four artifacts
    attached, from a separate job so that contents: write is scoped to
    the only step that needs it;
  - prints the unlist guidance on every run, because NuGet cannot
    delete and the moment it is needed is the moment nobody wants to be
    reading a workflow file.

Authentication is nuget.org Trusted Publishing: GitHub OIDC exchanged
by NuGet/login for an API key valid for one hour, requested immediately
before the push, so the repository holds no long-lived NuGet
credential. The fallback to a NUGET_API_KEY secret is documented in a
comment at the step, and the registry-side setup in CONTRIBUTING.md.

Because the pre-flight demands the 2.0 nuspec set, this workflow is
inert on a 1.x checkout: it can be cherry-picked to the default branch
(which is where `gh workflow run` reads workflow files from) and
dispatched against integration refs without being able to release 1.x.

package.yml is deleted rather than kept alongside: it built the same
artifacts by the same Cake targets and then asked a human to push them,
which is now `dry_run=true` with assertions. package.cmd,
package-unity.sh and build.sh are unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CONTRIBUTING.md's release process was ten manual steps ending in a
hand-run `dotnet nuget push` of an artifact zip. It is now: bump one
version in two files, PR, merge, dispatch publish.yml with
dry_run=true, dispatch it again with dry_run=false. The tag, the GitHub
release and the .unitypackage attachment are the workflow's job. The
changelog and Headway steps are unchanged, and the re-run-to-complete
and unlist-to-abandon behaviours are written down where someone
mid-incident will find them.

Adds the nuget.org Trusted Publishing setup as its own section: what
the policy binds (owner, repository, the workflow file name only), that
its scope has to allow new packages if a brand-new id is being claimed,
that a private-repo policy is provisional for 7 days until the first
publish gives nuget.org the repository ids, that it matches by
repository name and so must be created after any rename, and how to
fall back to an API key.

cake-build/README.md documents Release.Preflight and
Release.VerifyPackages, the three packages that package.cmd now
produces, --packageOutput, and the `--` separator that Cake's reserved
--version flag forces.

README gains a paragraph on why the exact core pin means all
Ably.PubSub.* references in a project move together.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sacOO7
sacOO7 force-pushed the integration/split-3-release-tooling branch from 6356ce3 to f2823d2 Compare September 17, 2026 15:16
…tooling

- D17 follow-up (composition break surfaced by the release dry run): under Package builds the signed core references the unsigned submodule-built DeltaCodec on the SDK TFMs - a pre-existing mix the deleted global ruleset suppression had been masking. Scope a CS8002 NoWarn to the core's Package-conditional signing group only (the doors compile clean and get nothing); the plain-Release guard stays fully live; proper cure is signing the submodule build (follow-up)
- D25: make 2.0.0-beta.N buildable and publishable - _Version stamps numeric identity into AssemblyVersion/AssemblyFileVersion (CS7034 otherwise) and the full string into AssemblyInformationalVersion; ReleaseAssertVersionFilesAgree asserts per-attribute expectations and its success message reworded; gh release create gains --prerelease for hyphenated versions; PackagingSpecs.TheTwoVersionFiles_Agree mirror-patched to the same split; README exact-pin paragraph gains the prerelease-pinning sentence
- D26: publish.yml gains a workflow-level concurrency group keyed on the version (never cancel in-progress) and the publish job runs in the `release` environment as the required-reviewer gate
- D7: new pre-flight assertion ReleaseAssertUnityPluginVersion - the committed unity/Assets/Ably/Plugins/Ably.PubSub.Device.dll must exist and carry the release's numeric identity and full informational version, so a stale merged plugin can never ship inside the .unitypackage
- D1 (CI): publish.yml regenerates the Unity plugin from the release checkout before UnityPackage and cross-checks identity/informational version against the committed DLL, which stays canonical and ships (it is what the Unity test pass validated)
- D12: nuspec projectUrl/releaseNotes/repository URLs moved to ably-pubsub-dotnet in all three nuspecs (metadata freezes per version at push time)
- D13: CHANGELOG gains the 2.0.0 entry and UPDATING.md is added as the 1.x -> 2.0 migration guide (door factories, mixed-graph rule, exact pinning, MAU forcing function, Xamarin push-receive gap, Unity door usage)
- D30: the five run-tests workflows also trigger on pushes to integration/** branches (previously only release-dry-run.yml did)
- D31: RestoreSolution stops swallowing restore failures - Windows NuGetRestore throws; the non-Windows nuget CLI path probes for the tool, warns-and-skips when absent (Mono-less macOS/Linux legs) and throws when present-and-failing; DotNetRestore now throws unconditionally
- D5: features.yml repository-name updated to ably-pubsub-dotnet - the check stays red regardless until the ops-side AWS IAM trust-policy re-bind allows the renamed repo
- D18 (optional polish, included - builds clean): Microsoft.SourceLink.GitHub 8.0.0 with PublishRepositoryUrl/EmbedUntrackedSources on the three SDK csprojs

Deferred/ops notes: the Windows-pack halves remain for CI (net46 pack/install
check, sn -v on packed assemblies, the publish.yml regenerate/cross-check dry
run); the `release` GitHub environment must be created and given required
reviewers plus a deployment-branch restriction in repo Settings - referencing
it auto-creates it unprotected; D13's entry needs its release date and compare
link when the release is cut; D25's wire-agent consequence (a beta build
reports ably-pubsub-dotnet/2.0.0 without the label via Defaults.GetVersion())
needs rollout-owner sign-off; Unity EditMode/PlayMode smoke of the committed
plugin remains for the Windows/Unity pass.

PubSub device/server split rollout (PDR-091b2) review follow-ups.

@maratal maratal left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: this is in good shape; comments only, nothing here blocks merge. The three-stage bracket around the pack (pre-flight before _Version, file-glob check between build and pack, post-pack assertions on the produced .nupkg) is the right design, the failure messages read like they were written for the person who will hit them at 6pm on release day, and the dry run is green end to end on windows-2022 with real net462 heads. I read the dry-run log rather than the description: every glob resolved, all three packages packed, and both doors' packed pin is [2.0.0] in every group. That closes the $version$ question properly.

Two things this PR fixes that I asked for on the earlier PRs, and which I would move down the stack so those PRs stand on their own:

  • features.ymlrepository-name: ably-pubsub-dotnet is the correct fix, not a regression. integration/v2, split-1 and split-2 all still say ably-dotnet, and every Features run on those branches fails with Not authorized to perform sts:AssumeRoleWithWebIdentity because the old role's trust policy no longer matches the renamed repository. This branch flips it and the check goes green. I had this backwards in my reviews of #1333 and #1334 and have corrected myself there. It belongs in #1333.
  • The scoped NoWarn CS8002 on Ably.PubSub.Core.csproj is what makes the Package build survive the ruleset change from #1334. Without it, #1334's Package build fails on the signed core referencing the unsigned DeltaCodec. It belongs in #1334. Note the dry-run log still shows the same CS8002 from the Ably.PubSub.Core.NETFramework head, which passes only because that csproj's Release config has no TreatWarningsAsErrors. Either add the same scoped suppression there or, better, do the follow-up the comment already names and sign the submodule build under Package.

Worth fixing before or soon after merge

  • Trusted-publishing environment mismatch. publish.yml sets environment: release on the publish job (good, that is the human gate), but the workflow comment, CONTRIBUTING.md and the PR body all say "leave Environment empty; this workflow uses no GitHub environment". A job with an environment gets an OIDC sub of repo:ably/ably-pubsub-dotnet:environment:release. nuget.org's environment field is optional, so an empty policy should still match, but the docs are wrong about what the workflow does and the first real dispatch is where a mismatch would surface. Fix the text, and add "create the release environment with required reviewers" to the one-time admin setup list. Without that environment existing with reviewers configured, GitHub auto-creates an unprotected one and the gate is a no-op.
  • Prerelease versions lose their label in the agent string. _Version now stamps AssemblyFileVersion as numeric and puts the full 2.0.0-beta.1 only in AssemblyInformationalVersion, but Defaults.GetVersion() reads AssemblyFileVersion, so a beta identifies itself on the wire as ably-pubsub-dotnet/2.0.0. Pre-existing, but this PR is what makes prereleases a first-class release path, and the 2.0.0-rc.1 you plan to ship will be indistinguishable from GA in the agent data. Reading the informational version (stripping + metadata) is a two-line change in the core.
  • SourceLink is new and undocumented. Microsoft.SourceLink.GitHub 8.0.0 plus PublishRepositoryUrl and EmbedUntrackedSources were added to the three SDK-style projects and the PR body does not mention it. It is a good addition, but it should be in the description, and Ably.PubSub.Server.NETFramework did not get it, so the net46 Server asset ships without source link while the others have it.
  • The description is stale in two places. It says the .unitypackage is still incomplete pending a regenerated Ably.PubSub.Core.dll, but #1334 committed Ably.PubSub.Device.dll and this PR's pre-flight asserts its version. It also says features.yml still says ably-dotnet; this PR changes it.

Smaller points

  • The "MAU rejection" claim now appears in three user-facing places (CHANGELOG.md, UPDATING.md twice). Same request as on #1333: verify against PDR-091b or soften to the classification and billing dependency. UPDATING.md also commits to a net8.0-android/net8.0-ios push port; decision 6 only says parked.
  • Hand-written CHANGELOG.md 2.0.0 section versus the generator. The rewritten release process still runs github_changelog_generator at release time and inserts its output at the top, so this section will need merging by hand then. Fine if intended; say so in the release steps.
  • The Unity cross-check compares versions, not content. A committed plugin at the right version but built from different source passes. The comment says the committed copy is canonical because it is what Unity testing validated, which is a reasonable position, but it means the check catches a forgotten bump and nothing else. Worth stating that limit in the step comment.
  • _Clean inside the Unity regeneration step. Build.NetStandard runs _Clean, which wipes bin/ after the packages are already in release-packages/. I traced it and nothing later reads bin/, so it is safe today. A one-line comment in the step saying the pack must stay before this step would stop someone reordering it.
  • features.yml did not get the integration/** push trigger that every other workflow gained. Minor, but it is the one workflow that will now go quiet on the integration branch.
  • Verified: the NuGet/login pin 8d196754… is the commit v1.2.0 points at, and the action's only required input is user, as described.

Not verified, as the PR itself says

The publish path (OIDC exchange, skip-if-published, wait-for-core) has not run and cannot until the workflow is on the default branch. The follow-up plan (cherry-pick only publish.yml to main, claim the names with 0.0.1-alpha.0, then run the re-dispatch and mismatched-version checks) is the right shape. I would add one item to it: run the very first real dispatch as a prerelease rather than the alpha placeholder if the name claim can wait, so that the first thing nuget.org lists under Ably.PubSub.* is something you would be comfortable leaving unlisted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants