Reference the Freeze Doctor contract as a package instead of copying it (BL-16719)

[JohnThomson]

Read this for shape, not to merge yet

Stacked on #8218 — review that one first; this branch's base is that branch, so its diff shows only the migration.

Nothing is published. The package is restored from a local folder, so this branch is buildable and reviewable before anything irreversible happens. Companion PR: BloomBooks/bloom-freeze-doctor#1.

Why

Three files describing the wire format between Bloom and the Freeze Doctor — the shared-memory layout, the session file, the named events — were 750 lines of hand-maintained duplicates across two repos. They had already drifted, and a fix in one had to be carried across the other by hand.

Drift there fails silently and expensively: Bloom writes one set of offsets, the Doctor reads another, and the reports look plausible and are wrong.

What this removes

193 insertions, 1095 deletions. Three things go away, and the third is the one worth noting:

• The three copied files.
• The drift check and its workflow — they existed only to catch divergence between copies. With one definition there is nothing to diverge.
• The RobustFile pre-commit exemption. It was only needed because the copies couldn't call RobustFile and still match the originals. The files have left this repo, so that shared quality gate is back to full strength with nothing excused from it — the check passes on its own merits. This is the exemption that needed approving in the last preflight; it's now moot.

What arrives

One PackageReference on BloomBooks.FreezeDoctor.Protocol — with no transitive dependencies at all, just a 26KB assembly, which is most of what makes this reasonable to add to Bloom — two using lines, and temporary local-feed scaffolding (NuGet.Config at the root, build/pack-freeze-doctor-protocol.ps1, localpackages/) that all disappears when the package is published.

To build it

build/pack-freeze-doctor-protocol.ps1

It packs the protocol from a clone of the Doctor's repo beside this one (or BLOOM_FREEZE_DOCTOR_REPO) into localpackages/. That project only exists on the Doctor repo's contract-package branch until it merges.

Why "Protocol" and not "Contract"

These files are not a contract in any sense a reader would recognize from the word — no interface, no assertions, nothing enforced. They are the wire format of the shared-memory channel between two processes: byte offsets, a schema version, event names. "Protocol" says that, so the package, its namespace, its folder in the Doctor's repo, and the test fixture here all say it.

The BloomBooks. prefix follows the house style already in production: BloomBooks.Velopack is a Bloom-only package published the same way (nuget.org, then unlisted — all 11 versions currently unlisted, 0 search hits), which also settles the earlier open question about a SIL. prefix. There isn't one to seek.

The pinned test, with its purpose rewritten — and a hole in it closed

FreezeDoctorProtocolTests (was DoctorChannelContractTests) used to guard against two copies diverging. That reason is gone, but it still earns its place for a different one: it pins the layout Bloom expects against the layout the referenced version actually has. A package upgrade that moved an offset would otherwise be silent — Bloom would compile, run, and publish its health where the Doctor no longer looks.

It wasn't actually doing that. It pinned SchemaVersion, Size, ActivityMaxBytes and the section name — not a single field offset. So the one failure it existed to prevent was the one it did not cover, and it couldn't have: the offsets are internal to the package. It now works from a Fields table the layout publishes about itself, and asserts every field's name, offset and size by value. Its comment says explicitly not to update the numbers to make it pass — a moved field means every Doctor already installed is misreading Bloom, so the numbers are the symptom.

Two tests join it. One pins the invariant rather than the numbers, which is what lets the first be strict without becoming a nuisance: PayloadBytes must be exactly the end of the last field, and at least the frozen generation-1 baseline. The other checks Bloom's extent actually reaches the shared page rather than trusting the constant.

PayloadBytes, and why the version moved to 0.2.0-alpha

The protocol now records how far the writer wrote. This is Bloom's side of bloom-freeze-doctor@aaec915, and for Bloom it is only a reference bump — Bloom writes the same fields either way.

Worth knowing why it exists, since it changes how this format is allowed to evolve: adding a field is the likeliest change, and until now there was no way to make one. SchemaVersion is part of the section's name, so bumping it makes an old Doctor stop finding the channel altogether rather than merely ignoring a field it doesn't know. With an extent recorded, fields can be appended within a generation, and a newer Doctor reading an older Bloom can tell a field that Bloom never wrote from a real zero — the page is zero-filled, so without it a report would confidently say "0 blocked workers" where the truth is "this Bloom is too old to say". That direction is the common one in the field, not a curiosity: the Doctor updates itself through Velopack while Bloom versions linger, and the 6.4 backport guarantees more than one vintage writing this page.

Several offsets moved to make room for it in the header. That was free only because nothing is published yet, which is a reason to settle the package id and get it published rather than to keep the local-folder arrangement going.

The version bump is not cosmetic: NuGet caches by id and version, so re-packing 0.1.0-alpha with a changed layout would let a machine that had already restored it keep the old assembly, with nothing looking wrong. Bump the version for any layout change, even while restoring from a local folder.

Decided, so not up for review here

The feed: publish to nuget.org, then unlist the version — absent from search and browse, still restorable when a consumer pins an exact version, and no feed credentials anywhere. A private feed was considered and rejected: it hides nothing (the Doctor's repo is public MIT, so the package is a compiled copy of readable source) and would cost credentials on every dev machine, in TeamCity, and in CI. Reasoning lives in the protocol .csproj.

The id, likewise: BloomBooks.FreezeDoctor.Protocol. An id on nuget.org is claimed permanently, which is exactly why it is being reviewed before anything is pushed rather than after.

Still open

• Who owns the nuget.org API key for the Doctor's repo. BloomBooks.Velopack's fork uses a repo-level BLOOMBOOKS_NUGET secret; whether that is an org-level key we can reuse couldn't be checked from here. Note the gotcha for whoever holds it: pushing a new package id needs a key scoped to "Push new packages and package versions", not just new versions of existing ones.

Verified

Bloom builds against the package with the copies deleted, and the full suite is 3155 passed, 13 skipped, 0 failed — the two extra over the pre-package count are the new layout tests.

The Doctor-side sentinel test (the writer never touches a byte past PayloadBytes) was verified by mutation rather than by passing: a stray write past the extent makes it fail with the intended message, and the mutation was reverted.

🤖 Generated with Claude Code

Devin review

---

This change is

[greptile-apps]

Your trial has ended. Reactivate Greptile to resume code reviews.

[JohnThomson]

[Devin — Investigate, current commit] A fresh checkout cannot build until someone runs the pack script.

This PackageReference can only be restored from the localpackages folder that the new root NuGet.Config adds, and that folder is empty on a fresh clone — only .gitkeep is tracked. Nothing is published to nuget.org yet, so restore fails on any machine that has not run build/pack-freeze-doctor-protocol.ps1 against a clone of the Doctor repo. That includes CI and TeamCity.

Devin's own read is that this is intentional and documented rather than a bug, and adds that a reviewer should confirm the package is published and the temporary scaffolding removed before this actually merges. Agreed on both counts — which is why this thread is being left open.

Leaving open deliberately: this is a merge precondition, not a settled point. Before #8219 merges, three things have to go: the root NuGet.Config, build/pack-freeze-doctor-protocol.ps1, and localpackages/ — replaced by the package being restorable from nuget.org. Resolving this thread now would delete the only durable record of that.

Worth noting for anyone reading the checks: GitHub Actions does not build Bloom on PRs, so this will not appear as a red check here. It would surface in TeamCity.

[greptile-apps]

Your trial has ended. Reactivate Greptile to resume code reviews.