Skip to content

Commit 240e6e6

Browse files
committed
feat(updates): dev takes the newest bundle, because only its number is behind
The compatibility gate compares a bundle's stated minimum against the core's package version, and on `dev` that version is the one thing that lags: the branch carries the endpoints of the next release while still calling itself the last one. So a console built against those endpoints — which dev can serve perfectly well, having the endpoints — was refused by a check that is, on this branch alone, measuring the wrong thing. It showed up immediately. With adminui 6.1.0 published asking for 4.0.0-beta.22, the next push to dev would have failed the dev image build at `fetch:admin`, and a dev install pressing update in the console would have been told to update a server that already had what it needed. Dev is therefore exempt outright, at every point the gate exists: the build-time fetch, the in-place update's preflight, the release resolver (which takes the newest of its channel rather than walking for a compatible one, saving a request per candidate), and the status the console reads. What dev is for is trying a fix, and what that needs is the newest of everything. `isDevBuild` is deliberately not `readBuildChannel() === 'dev'`. That function answers `dev` for anything that does not claim otherwise, which is the right way round for a warning and the wrong way round for a permission — a server-dist tarball unpacked outside a repository declares nothing, and must not inherit what dev may do. It has to be said: BUILD_CHANNEL, the build stamp, or a checkout literally on the branch. Said in three places because none of them can read the others. The dev job sets it on the build step, since a CI checkout is on a detached HEAD with no branch to read; the Dockerfile declares it in the builder stage, since ARGs do not cross stages and `.git` is not copied into the image; and the branch is the fallback for a developer building in their own working copy. Release builds are untouched. A beta or stable build still fails when its bundles ask for a core it is not — which, since a release bumps the version in the same commit that tags it, only fires when that bump is missing.
1 parent 79fd3e8 commit 240e6e6

6 files changed

Lines changed: 158 additions & 5 deletions

File tree

.github/workflows/release.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,14 @@ jobs:
134134
- name: Install dependencies
135135
run: npm ci
136136

137+
# Declared here as well as on the image below, because `npm run build` fetches the web
138+
# bundles and checks them against this checkout's version — and on dev that version is
139+
# behind the code by design, so the newest bundle is what this image should carry.
140+
# A CI checkout is on a detached HEAD, so the branch cannot be read; it has to be said.
137141
- name: Build
138142
run: npm run build
143+
env:
144+
BUILD_CHANNEL: dev
139145

140146
- name: Run tests
141147
run: npm test

Dockerfile

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,14 @@ RUN npm config delete proxy \
1818
&& npm config delete https-proxy \
1919
&& npm ci
2020
COPY . .
21+
22+
# The build fetches the Admin UI and Player bundles and checks them against this source
23+
# tree's version. On dev that version is behind the code by design, so the check is waived
24+
# there and the image carries the newest bundles — which is the point of a dev image.
25+
# It has to be declared in this stage: ARGs do not cross stages, `.git` is not copied in, so
26+
# the branch cannot be read here either, and without it every build looks like "not dev".
27+
ARG BUILD_CHANNEL
28+
ENV BUILD_CHANNEL=${BUILD_CHANNEL}
2129
RUN npm run build
2230
RUN npm prune --omit=dev
2331

scripts/bundleCompat.mjs

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@
1010
* Deliberately a hard failure rather than a warning. The asymmetry is what makes that safe:
1111
* it can only fire when the *bundle* is ahead of the core being built, never when the core
1212
* is ahead of the bundle, so an ordinary release — core moves first, bundles follow — never
13-
* sees it. `SONN_SKIP_BUNDLE_COMPAT=1` is there for the deliberate exception.
13+
* sees it. `SONN_SKIP_BUNDLE_COMPAT=1` is there for the deliberate one-off.
14+
*
15+
* Dev is exempt outright — see `isDevBuild`. It is the one branch where the version number is
16+
* routinely behind the code, so the comparison is measuring the wrong thing there and only
17+
* there.
1418
*
1519
* A plain `.mjs` duplicate of `src/shared/semver.ts` on purpose: these scripts run before
1620
* `tsc` has produced `dist/`, so they cannot import the compiled module they mirror. The
@@ -78,6 +82,33 @@ export function satisfiesMin(running, minimum) {
7882
return compareVersions(have, min) >= 0;
7983
}
8084

85+
/**
86+
* Whether this build is explicitly a development one, in which case minimums do not apply.
87+
*
88+
* On `dev` the version number is the only thing behind: the branch carries the endpoints of
89+
* the next release while still calling itself the last one, so a bundle built against those
90+
* endpoints fails a check that is, on this branch alone, measuring the wrong thing. Dev takes
91+
* the newest of everything — nobody runs it except to try a fix.
92+
*
93+
* Declared rather than assumed. `BUILD_CHANNEL` is what CI sets, and it is read first because
94+
* a CI checkout is on a detached HEAD and has no branch to read. Falling back to the branch is
95+
* for the developer running `npm run build` in their own working copy. A checkout that is
96+
* neither is not dev, and stays gated.
97+
*/
98+
export async function isDevBuild(cwd) {
99+
const declared = (process.env.BUILD_CHANNEL ?? '').trim().toLowerCase();
100+
if (declared) return declared === 'dev';
101+
const stamp = (process.env.BUILD_TIMESTAMP ?? '').trim().toLowerCase();
102+
if (stamp.startsWith('dev-')) return true;
103+
if (stamp.startsWith('testing-')) return false;
104+
try {
105+
const head = await fs.readFile(join(cwd, '.git', 'HEAD'), 'utf8');
106+
return /^ref:\s*refs\/heads\/dev\s*$/i.test(head.trim());
107+
} catch {
108+
return false;
109+
}
110+
}
111+
81112
export async function readCoreVersion(cwd) {
82113
try {
83114
const pkg = JSON.parse(await fs.readFile(join(cwd, 'package.json'), 'utf8'));
@@ -96,6 +127,10 @@ export async function readCoreVersion(cwd) {
96127
*/
97128
export async function assertBundleFitsCore(dir, coreVersion, label) {
98129
if ((process.env.SONN_SKIP_BUNDLE_COMPAT ?? '').trim() === '1') return;
130+
if (await isDevBuild(process.cwd())) {
131+
console.log(`[bundle-compat] dev build, taking ${label} as published`);
132+
return;
133+
}
99134
let manifest;
100135
try {
101136
manifest = JSON.parse(await fs.readFile(join(dir, 'version.json'), 'utf8'));

src/adapters/http/adminApi/misc/miscHandlers.ts

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import https from 'node:https';
66
import os from 'node:os';
77
import { join, resolve } from 'node:path';
88
import { pipeline } from 'node:stream/promises';
9-
import { readBuildChannel, readBuildVersion, readGitBranch, readPackageVersion } from '@/shared/serverVersion';
9+
import { isDevBuild, readBuildChannel, readBuildVersion, readGitBranch, readPackageVersion } from '@/shared/serverVersion';
1010
import { compareVersions, satisfiesMin } from '@/shared/semver';
1111
import {
1212
describeBundle,
@@ -426,8 +426,13 @@ function handleInfo(
426426
// stamp distinguishes two builds of the same core and must not change whether a
427427
// bundle fits it.
428428
const publicDir = deps.runtimeConfig.http.publicDir;
429-
const player = describeBundle(publicDir, 'player', pkgVersion);
430-
const adminUi = describeBundle(publicDir, 'admin', pkgVersion);
429+
// On dev every bundle fits, because dev installs every bundle. Reporting otherwise would
430+
// put a permanent "built for a newer server" note on a branch that is meant to run ahead.
431+
const dev = isDevBuild();
432+
const fitsAnything = (bundle: ReturnType<typeof describeBundle>) =>
433+
dev ? { ...bundle, satisfied: true } : bundle;
434+
const player = fitsAnything(describeBundle(publicDir, 'player', pkgVersion));
435+
const adminUi = fitsAnything(describeBundle(publicDir, 'admin', pkgVersion));
431436
// The oldest, because that is what "the speakers run X" has to mean when they disagree.
432437
const runningClients = (deps.sonnClientVersions?.() ?? []).filter(Boolean).sort(compareVersions);
433438
const sonnClient = { installed: runningClients[0] ?? null };
@@ -886,6 +891,10 @@ async function resolveWebBundleRelease(
886891

887892
const channel = detectReleaseChannel();
888893
const runningCore = readPackageVersion();
894+
// Dev takes the newest of its channel outright. Walking for a compatible release would be
895+
// walking past bundles this server can serve, on the strength of a version string that is
896+
// stale by design between releases — and it would spend a request per candidate doing it.
897+
const newestWins = isDevBuild();
889898

890899
let releases: BundleRelease[] = [];
891900
try {
@@ -905,6 +914,19 @@ async function resolveWebBundleRelease(
905914
? [...releases.filter((r) => r.prerelease), ...releases.filter((r) => !r.prerelease)]
906915
: releases.filter((r) => !r.prerelease);
907916

917+
const first = candidates[0];
918+
if (newestWins && first) {
919+
log.debug(`${spec.label} taking the newest release, minimums are waived on dev`, {
920+
tag: first.tag,
921+
});
922+
return {
923+
release: first.tag,
924+
distUrl: bundleAssetUrl(spec, first.tag),
925+
channel,
926+
picked: 'newest',
927+
};
928+
}
929+
908930
for (const candidate of candidates.slice(0, BUNDLE_LOOKBACK)) {
909931
let manifest: BundleManifest;
910932
try {
@@ -1008,7 +1030,10 @@ async function performWebBundleUpdate(
10081030
* also what makes it free: nothing is rolled back because nothing moved.
10091031
*/
10101032
const staged = readBundleManifest(stagingDir);
1011-
const rejection = rejectIfCoreTooOld(runningCore, staged);
1033+
// Waived on dev, where the version number is the only thing behind: the branch already
1034+
// carries the endpoints of the next release while still calling itself the last one, so
1035+
// the minimum would refuse a bundle this server can serve perfectly well.
1036+
const rejection = isDevBuild() ? null : rejectIfCoreTooOld(runningCore, staged);
10121037
if (rejection) {
10131038
await fs.rm(stagingDir, { recursive: true, force: true }).catch(() => {});
10141039
deps.log.warn(`${spec.label} update refused, needs a newer core`, { release, ...rejection });

src/shared/serverVersion.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,36 @@ export function readBuildChannel(): BuildChannel {
8989
return 'dev';
9090
}
9191

92+
/**
93+
* Whether this is explicitly a development build.
94+
*
95+
* Separate from `readBuildChannel` because of that function's default: it answers `dev`
96+
* for anything that does not claim otherwise, which is the right way round for a warning
97+
* ("this might not be a release") and the wrong way round for a permission. A deployment
98+
* that simply carries no channel — a server-dist tarball unpacked outside a repository —
99+
* must not inherit whatever dev is allowed to do.
100+
*
101+
* What it is allowed to do is skip the bundle compatibility gate. On `dev` the version
102+
* number is the only thing that lags: the branch carries the endpoints of the next release
103+
* while still calling itself the last one, so a bundle built against those endpoints is
104+
* refused by a check that is, on this branch alone, measuring the wrong thing. Nobody runs
105+
* dev except to try a fix, and what they need there is the newest of everything.
106+
*/
107+
export function isDevBuild(): boolean {
108+
const declared = process.env.BUILD_CHANNEL?.trim().toLowerCase();
109+
if (declared && (BUILD_CHANNELS as readonly string[]).includes(declared)) {
110+
return declared === 'dev';
111+
}
112+
const stamp = process.env.BUILD_TIMESTAMP?.trim().toLowerCase() ?? '';
113+
if (stamp.startsWith('dev-')) {
114+
return true;
115+
}
116+
if (stamp.startsWith('testing-')) {
117+
return false;
118+
}
119+
return readGitBranch()?.trim().toLowerCase() === 'dev';
120+
}
121+
92122
/** Appends the CI build stamp when present, so nightly builds are distinguishable. */
93123
export function readBuildVersion(pkgVersion: string = readPackageVersion()): string {
94124
const tsRaw = process.env.BUILD_TIMESTAMP?.trim();

tests/bundleCompat.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
rejectIfCoreTooOld,
1414
} from '../src/shared/bundleManifest';
1515
import { buildMiscRoutes } from '../src/adapters/http/adminApi/misc/miscHandlers';
16+
import { isDevBuild } from '../src/shared/serverVersion';
1617

1718
/*
1819
* The Admin UI and the Player are separate repositories, updated by separate buttons, in
@@ -173,3 +174,51 @@ test('info reports both bundles, so the console is no longer the one part unname
173174
assert.equal(sent.body.player.satisfied, true);
174175
assert.ok('requires' in sent.body, 'the other direction is reported even when unset');
175176
});
177+
178+
/*
179+
* Dev is exempt. It is the one branch whose version number is behind its own code — it
180+
* carries the endpoints of the next release while still calling itself the last one — so a
181+
* minimum written against those endpoints refuses a bundle the server can serve. Nobody runs
182+
* dev except to try a fix, and what they need there is the newest of everything.
183+
*/
184+
185+
function withEnv<T>(env: Record<string, string | undefined>, fn: () => T): T {
186+
const previous = new Map(Object.keys(env).map((key) => [key, process.env[key]]));
187+
for (const [key, value] of Object.entries(env)) {
188+
if (value === undefined) delete process.env[key];
189+
else process.env[key] = value;
190+
}
191+
try {
192+
return fn();
193+
} finally {
194+
for (const [key, value] of previous) {
195+
if (value === undefined) delete process.env[key];
196+
else process.env[key] = value;
197+
}
198+
}
199+
}
200+
201+
test('a dev build is one that says so, never one that failed to say otherwise', () => {
202+
const clean = { BUILD_CHANNEL: undefined, BUILD_TIMESTAMP: undefined };
203+
assert.equal(withEnv({ ...clean, BUILD_CHANNEL: 'dev' }, isDevBuild), true);
204+
assert.equal(withEnv({ ...clean, BUILD_CHANNEL: 'beta' }, isDevBuild), false);
205+
assert.equal(withEnv({ ...clean, BUILD_CHANNEL: 'stable' }, isDevBuild), false);
206+
// Older images predate BUILD_CHANNEL and stamp the channel into the build id.
207+
assert.equal(withEnv({ ...clean, BUILD_TIMESTAMP: 'dev-20260912' }, isDevBuild), true);
208+
assert.equal(withEnv({ ...clean, BUILD_TIMESTAMP: 'testing-20260912' }, isDevBuild), false);
209+
210+
/*
211+
* The one that matters. `readBuildChannel` answers `dev` for anything that does not claim
212+
* otherwise, which is right for a warning and wrong for a permission: a server-dist tarball
213+
* unpacked outside a repository declares nothing, and must not inherit what dev may do.
214+
* Asserted from a working directory with no `.git`, which is exactly that deployment.
215+
*/
216+
const nowhere = mkdtempSync(join(tmpdir(), 'sonn-nogit-'));
217+
const cwd = process.cwd();
218+
process.chdir(nowhere);
219+
try {
220+
assert.equal(withEnv(clean, isDevBuild), false, 'silence is not a claim to be dev');
221+
} finally {
222+
process.chdir(cwd);
223+
}
224+
});

0 commit comments

Comments
 (0)