You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by as...@apache.org on 2026/09/07 12:15:14 UTC
(maka) branch main updated: fix(ci): accept an amended protocol compatibility declaration (#4944)
This is an automated email from the ASF dual-hosted git repository.
Astro-Han pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/maka.git
The following commit(s) were added to refs/heads/main by this push:
new 73f61bb7ff fix(ci): accept an amended protocol compatibility declaration (#4944)
73f61bb7ff is described below
commit 73f61bb7ff553422018dfdd649158e3946059ac4
Author: NekoPunch <95...@users.noreply.github.com>
AuthorDate: Mon Sep 7 05:15:08 2026 -0700
fix(ci): accept an amended protocol compatibility declaration (#4944)
A newly added compatibility declaration became invisible to staged validation after a later edit. Accept added or modified declarations in the staged path while retaining added-only validation against the target base. Shared parsing retains the existing human-reviewed compatibility contract.
Generated-by: Claude Code
---
.../protocol-compatible-changes/README.md | 53 +++++++
scripts/protocol-epoch-check.mjs | 117 +++++++++------
scripts/protocol-epoch-check.test.mjs | 157 +++++++++++++++++++++
3 files changed, 281 insertions(+), 46 deletions(-)
diff --git a/packages/runtime-host/protocol-compatible-changes/README.md b/packages/runtime-host/protocol-compatible-changes/README.md
new file mode 100644
index 0000000000..3a744393bf
--- /dev/null
+++ b/packages/runtime-host/protocol-compatible-changes/README.md
@@ -0,0 +1,53 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+-->
+
+# Compatible protocol changes
+
+`RUNTIME_HOST_COMPATIBILITY_EPOCH` is the generation Client and Host exchange at
+handshake; a mismatch rejects the connection. The guard in
+`scripts/protocol-epoch-check.mjs` therefore treats every touched file under
+`packages/runtime-host/src/protocol/` as a protocol change and demands an epoch the
+base branch has not seen. That proxy is deliberately coarse: a needless bump costs a
+number, a missed one ships two incompatible protocols under one epoch (#3313).
+
+A file here is the exemption, for a change the wire provably cannot observe:
+
+```json
+{
+ "epoch": 121,
+ "files": ["packages/runtime-host/src/protocol/codec.ts"],
+ "reason": "Renames a local helper without changing any codec or message shape"
+}
+```
+
+- `epoch` pins the declaration to the epoch your branch carries. When another change
+ moves it first, re-pin, and re-read the reason: it has to still hold.
+- `files` must name every protocol file your branch changed, or the guard fails on
+ the ones it does not cover.
+- `reason` is read by a human. Nothing checks it, so it earns its keep only by being
+ specific about why no peer can tell the difference.
+
+Legitimate: renaming or un-exporting an internal helper, moving a constant, tightening
+a decoder to reject what it already rejected. Not legitimate: any new or removed field,
+key, error code, or accepted value — those move the epoch, whatever the intent.
+
+CI checks the whole branch against its base, so one declaration covers the PR. When a
+later commit touches another protocol file, amend this file rather than adding a second
+one. Declarations are read only while they are new to the branch; once landed they are
+history, which is why the older files here still pin long-past epochs.
diff --git a/scripts/protocol-epoch-check.mjs b/scripts/protocol-epoch-check.mjs
index d4cf832d06..9c712b4aff 100644
--- a/scripts/protocol-epoch-check.mjs
+++ b/scripts/protocol-epoch-check.mjs
@@ -25,8 +25,10 @@
// incompatible protocols end up advertising one epoch. This check runs on the
// PR merge result and compares it with the synthetic merge's first parent: the
// current base branch. An incompatible change must move the epoch. A compatible
-// extension may keep it only when a newly added declaration names every changed
-// protocol file, keeping that exception explicit and reviewable.
+// extension may keep it only when a declaration added against the base names every
+// changed protocol file, keeping that exception explicit and reviewable. The
+// `--staged` pre-commit mode judges one commit against HEAD and cannot see the base,
+// so it also honors a declaration the branch amends; the merge result decides.
import { execFileSync } from 'node:child_process';
import { dirname, resolve } from 'node:path';
@@ -53,6 +55,46 @@ export function extractCompatibilityEpoch(source) {
return Number(matches[0][1]);
}
+const DECLARATION_KEYS = ['epoch', 'files', 'reason'];
+
+export function parseDeclaration(declarationPath, source, headEpoch) {
+ const fail = (detail) => {
+ throw new Error(`Invalid compatible protocol change declaration ${declarationPath}: ${detail}`);
+ };
+ let value;
+ try {
+ value = JSON.parse(source);
+ } catch (error) {
+ fail(`it is not valid JSON (${error.message})`);
+ }
+ if (!value || typeof value !== 'object' || Array.isArray(value)) fail('expected a JSON object');
+ const unknown = Object.keys(value).filter((key) => !DECLARATION_KEYS.includes(key));
+ if (unknown.length > 0) {
+ fail(
+ `unknown key(s) ${unknown.join(', ')}; a declaration holds only ${DECLARATION_KEYS.join(', ')}`,
+ );
+ }
+ if (value.epoch !== headEpoch) {
+ fail(
+ `it declares epoch ${JSON.stringify(value.epoch)} but this branch is at ${headEpoch}. ` +
+ `Another change moved the epoch, so re-pin this declaration to ${headEpoch} and re-read ` +
+ `its reason: it has to still hold against the protocol as it now stands.`,
+ );
+ }
+ if (!Array.isArray(value.files) || value.files.length === 0) {
+ fail('"files" must name at least one changed protocol file');
+ }
+ if (typeof value.reason !== 'string' || value.reason.trim().length === 0) {
+ fail('"reason" must say why the wire cannot observe the change');
+ }
+ for (const file of value.files) {
+ if (typeof file !== 'string' || !file.startsWith(PROTOCOL_DIR)) {
+ fail(`"files" entry ${JSON.stringify(file)} is not a path under ${PROTOCOL_DIR}`);
+ }
+ }
+ return value.files;
+}
+
export function evaluateEpochCheck({
baseEpoch,
headEpoch,
@@ -71,6 +113,15 @@ export function evaluateEpochCheck({
const compatible = new Set(compatibleProtocolFiles);
const incompatibleChanges = changedProtocolFiles.filter((file) => !compatible.has(file));
if (incompatibleChanges.length > 0 && headEpoch === baseEpoch) {
+ const template = JSON.stringify(
+ {
+ epoch: headEpoch,
+ files: incompatibleChanges,
+ reason: '<why the wire cannot observe this change>',
+ },
+ null,
+ 2,
+ );
return {
ok: false,
reason:
@@ -79,7 +130,10 @@ export function evaluateEpochCheck({
`a git conflict (#3313), so every protocol change must land with an epoch the current ` +
`base has not seen: rebase onto current main and set the epoch past ${baseEpoch}. ` +
`Changed files without a compatible-change declaration:\n` +
- `${incompatibleChanges.map((file) => ` ${file}`).join('\n')}`,
+ `${incompatibleChanges.map((file) => ` ${file}`).join('\n')}\n\n` +
+ `If the wire provably cannot observe this change, declare it instead of bumping: ` +
+ `add one ${COMPATIBLE_CHANGE_DIR}<slug>.json, described by the README in that ` +
+ `directory, holding\n${template}`,
};
}
return {
@@ -109,29 +163,15 @@ export function compatibleProtocolFilesBetween(base, head, headEpoch, exec = exe
exec,
)
.split('\n')
- .filter(Boolean);
+ .filter((file) => file.endsWith('.json'));
const compatibleFiles = new Set();
for (const declaration of declarations) {
- const value = JSON.parse(git(['show', `${head}:${declaration}`], exec));
- if (
- !value ||
- typeof value !== 'object' ||
- Array.isArray(value) ||
- value.epoch !== headEpoch ||
- !Array.isArray(value.files) ||
- value.files.length === 0 ||
- typeof value.reason !== 'string' ||
- value.reason.trim().length === 0 ||
- Object.keys(value).some((key) => !['epoch', 'files', 'reason'].includes(key))
- ) {
- throw new Error(`Invalid compatible protocol change declaration: ${declaration}`);
- }
- for (const file of value.files) {
- if (typeof file !== 'string' || !file.startsWith(PROTOCOL_DIR)) {
- throw new Error(`Invalid protocol file in compatible change declaration: ${declaration}`);
- }
- compatibleFiles.add(file);
- }
+ const files = parseDeclaration(
+ declaration,
+ git(['show', `${head}:${declaration}`], exec),
+ headEpoch,
+ );
+ for (const file of files) compatibleFiles.add(file);
}
return [...compatibleFiles];
}
@@ -152,35 +192,20 @@ export function evaluateStagedEpochCheck(exec = execFileSync) {
.split('\n')
.filter(Boolean)
.filter((file) => !isStagedHeaderOnlyChange(file, exec));
+ // `M` too: a branch amends the declaration it added. The merge-result check counts
+ // only declarations added against the base, so editing a landed one grants nothing.
const declarations = git(
- ['diff', '--cached', '--diff-filter=A', '--name-only', 'HEAD', '--', COMPATIBLE_CHANGE_DIR],
+ ['diff', '--cached', '--diff-filter=AM', '--name-only', 'HEAD', '--', COMPATIBLE_CHANGE_DIR],
exec,
)
.split('\n')
- .filter(Boolean);
+ .filter((file) => file.endsWith('.json'));
const compatibleProtocolFiles = [];
const headEpoch = extractCompatibilityEpoch(stagedFile(EPOCH_FILE, exec));
for (const declaration of declarations) {
- const value = JSON.parse(stagedFile(declaration, exec));
- if (
- !value ||
- typeof value !== 'object' ||
- Array.isArray(value) ||
- value.epoch !== headEpoch ||
- !Array.isArray(value.files) ||
- value.files.length === 0 ||
- typeof value.reason !== 'string' ||
- value.reason.trim().length === 0 ||
- Object.keys(value).some((key) => !['epoch', 'files', 'reason'].includes(key))
- ) {
- throw new Error(`Invalid compatible protocol change declaration: ${declaration}`);
- }
- for (const file of value.files) {
- if (typeof file !== 'string' || !file.startsWith(PROTOCOL_DIR)) {
- throw new Error(`Invalid protocol file in compatible change declaration: ${declaration}`);
- }
- compatibleProtocolFiles.push(file);
- }
+ compatibleProtocolFiles.push(
+ ...parseDeclaration(declaration, stagedFile(declaration, exec), headEpoch),
+ );
}
return evaluateEpochCheck({
baseEpoch: epochAtRevision('HEAD', exec),
diff --git a/scripts/protocol-epoch-check.test.mjs b/scripts/protocol-epoch-check.test.mjs
index 2982a52e72..2c40990a5d 100644
--- a/scripts/protocol-epoch-check.test.mjs
+++ b/scripts/protocol-epoch-check.test.mjs
@@ -32,6 +32,7 @@ import {
evaluateStagedEpochCheck,
extractCompatibilityEpoch,
isHeaderOnlyChange,
+ parseDeclaration,
} from './protocol-epoch-check.mjs';
import { renderHeader } from './asf-license-headers.mjs';
import { dirname, join } from 'node:path';
@@ -369,3 +370,159 @@ test('treats added, deleted, and renamed protocol files as real changes', () =>
rmSync(repo, { recursive: true, force: true });
}
});
+
+/**
+ * A branch declares its protocol files in one declaration, so a later commit
+ * that touches another file amends the declaration it already added rather than
+ * adding a second one. The staged check sees that as a modification.
+ */
+test('accepts a declaration amended by a later commit on the same branch', () => {
+ const repo = mkdtempSync(join(tmpdir(), 'maka-protocol-epoch-amend-'));
+ const epochPath = join(repo, EPOCH_FILE);
+ const protocolDirectory = dirname(epochPath);
+ const declarationPath = join(repo, COMPATIBLE_CHANGE_DIR, 'example.json');
+ const first = 'packages/runtime-host/src/protocol/first.ts';
+ const second = 'packages/runtime-host/src/protocol/second.ts';
+ const runGit = (...args) => execFileSync('git', args, { cwd: repo, encoding: 'utf8' });
+ const runInFixture = (file, args, options) =>
+ execFileSync(file, args, { ...options, cwd: repo, encoding: 'utf8' });
+ const declare = (files) =>
+ writeFileSync(
+ declarationPath,
+ JSON.stringify({ epoch: 27, files, reason: 'Renames a local helper the wire never sees' }),
+ );
+
+ try {
+ runGit('init', '--initial-branch=main');
+ runGit('config', 'user.email', 'epoch-guard@example.invalid');
+ runGit('config', 'user.name', 'Epoch Guard Test');
+ runGit('config', 'commit.gpgSign', 'false');
+ mkdirSync(protocolDirectory, { recursive: true });
+ mkdirSync(join(repo, COMPATIBLE_CHANGE_DIR), { recursive: true });
+ writeFileSync(epochPath, 'export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 27 as const;\n');
+ writeFileSync(join(repo, first), 'export const first = 1;\n');
+ writeFileSync(join(repo, second), 'export const second = 1;\n');
+ runGit('add', '.');
+ runGit('commit', '-m', 'base');
+
+ writeFileSync(join(repo, first), 'export const first = 2;\n');
+ declare([first]);
+ // The directory documents itself; only its .json files are declarations.
+ writeFileSync(join(repo, COMPATIBLE_CHANGE_DIR, 'README.md'), '# Compatible changes\n');
+ runGit('add', '.');
+ assert.equal(evaluateStagedEpochCheck(runInFixture).ok, true);
+ runGit('commit', '-m', 'declare the first file');
+
+ writeFileSync(join(repo, second), 'export const second = 2;\n');
+ runGit('add', second);
+ const undeclared = evaluateStagedEpochCheck(runInFixture);
+ assert.equal(undeclared.ok, false);
+ assert.match(undeclared.reason, /second\.ts/);
+
+ declare([first, second]);
+ runGit('add', '.');
+ assert.equal(evaluateStagedEpochCheck(runInFixture).ok, true);
+ } finally {
+ rmSync(repo, { recursive: true, force: true });
+ }
+});
+
+/**
+ * The staged check accepts an amended declaration; the merge-result check still
+ * counts only declarations added against the base, so editing one that already
+ * landed on the base branch buys no exemption.
+ */
+test('refuses a landed declaration edited to cover a new protocol file', () => {
+ const repo = mkdtempSync(join(tmpdir(), 'maka-protocol-epoch-landed-'));
+ const epochPath = join(repo, EPOCH_FILE);
+ const protocolDirectory = dirname(epochPath);
+ const declarationPath = join(repo, COMPATIBLE_CHANGE_DIR, 'landed.json');
+ const covered = 'packages/runtime-host/src/protocol/covered.ts';
+ const smuggled = 'packages/runtime-host/src/protocol/smuggled.ts';
+ const runGit = (...args) => execFileSync('git', args, { cwd: repo, encoding: 'utf8' });
+ const runInFixture = (file, args, options) =>
+ execFileSync(file, args, { ...options, cwd: repo, encoding: 'utf8' });
+
+ try {
+ runGit('init', '--initial-branch=main');
+ runGit('config', 'user.email', 'epoch-guard@example.invalid');
+ runGit('config', 'user.name', 'Epoch Guard Test');
+ mkdirSync(protocolDirectory, { recursive: true });
+ mkdirSync(join(repo, COMPATIBLE_CHANGE_DIR), { recursive: true });
+ writeFileSync(epochPath, 'export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 27 as const;\n');
+ writeFileSync(join(repo, covered), 'export const covered = 1;\n');
+ writeFileSync(join(repo, smuggled), 'export const smuggled = 1;\n');
+ writeFileSync(
+ declarationPath,
+ JSON.stringify({ epoch: 27, files: [covered], reason: 'Landed on the base branch' }),
+ );
+ runGit('add', '.');
+ runGit('commit', '-m', 'base with a landed declaration');
+ runGit('tag', 'base');
+
+ writeFileSync(join(repo, smuggled), 'export const smuggled = 2;\n');
+ writeFileSync(
+ declarationPath,
+ JSON.stringify({ epoch: 27, files: [covered, smuggled], reason: 'Edited after landing' }),
+ );
+ runGit('add', '.');
+ runGit('commit', '-m', 'edit the landed declaration');
+
+ assert.deepEqual(compatibleProtocolFilesBetween('base', 'HEAD', 27, runInFixture), []);
+ const verdict = evaluateEpochCheck({
+ baseEpoch: 27,
+ headEpoch: 27,
+ changedProtocolFiles: changedProtocolFilesBetween('base', 'HEAD', runInFixture),
+ compatibleProtocolFiles: compatibleProtocolFilesBetween('base', 'HEAD', 27, runInFixture),
+ });
+ assert.equal(verdict.ok, false);
+ assert.match(verdict.reason, /smuggled\.ts/);
+ } finally {
+ rmSync(repo, { recursive: true, force: true });
+ }
+});
+
+test('names what makes a declaration invalid, and tells a stale one to re-pin', () => {
+ const path = `${COMPATIBLE_CHANGE_DIR}example.json`;
+ const declaration = (overrides) =>
+ JSON.stringify({
+ epoch: 28,
+ files: ['packages/runtime-host/src/protocol/turn.ts'],
+ reason: 'Renames a local helper',
+ ...overrides,
+ });
+
+ assert.deepEqual(parseDeclaration(path, declaration({}), 28), [
+ 'packages/runtime-host/src/protocol/turn.ts',
+ ]);
+ assert.throws(() => parseDeclaration(path, declaration({}), 29), /re-pin this declaration to 29/);
+ assert.throws(() => parseDeclaration(path, declaration({ files: [] }), 28), /"files" must name/);
+ assert.throws(
+ () => parseDeclaration(path, declaration({ reason: ' ' }), 28),
+ /"reason" must say/,
+ );
+ assert.throws(
+ () => parseDeclaration(path, declaration({ note: 'x' }), 28),
+ /unknown key\(s\) note/,
+ );
+ assert.throws(
+ () => parseDeclaration(path, declaration({ files: ['packages/core/src/turn.ts'] }), 28),
+ /is not a path under/,
+ );
+ assert.throws(() => parseDeclaration(path, '{', 28), /not valid JSON/);
+});
+
+test('offers a paste-ready declaration when a protocol change has no epoch to show for it', () => {
+ const verdict = evaluateEpochCheck({
+ baseEpoch: 27,
+ headEpoch: 27,
+ changedProtocolFiles: ['packages/runtime-host/src/protocol/codec.ts'],
+ });
+ assert.equal(verdict.ok, false);
+ const template = verdict.reason.slice(verdict.reason.indexOf('{'));
+ assert.deepEqual(JSON.parse(template), {
+ epoch: 27,
+ files: ['packages/runtime-host/src/protocol/codec.ts'],
+ reason: '<why the wire cannot observe this change>',
+ });
+});
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org