From bce6782426af75c55ca3b37e54606638f3bf8801 Mon Sep 17 00:00:00 2001 From: Leo Date: Sun, 1 Nov 2026 01:42:25 +0800 Subject: [PATCH] test(scripts): the CLI-version gate's rules, which have passed while wrong twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate compares the version the plugin claims against the one the vendor publishes, and every copy in the tree against the constant. It runs on a schedule, and its failure mode is the worst kind: it PASSES while reading nothing. It has already done so twice. The first version only scanned for literals and could not see the constant at all; the second only scanned `opencode/` and could not see a bare copy (three e2e comments were two patches stale while it reported OK). Both were found by a control run, not by a test. So the rules are now `scripts/vendor-version.ts` — a function of its input — and `test/vendor-version.test.ts` holds them: - the constant is FOUND, and its absence is `undefined` rather than a default, because "read nothing" and "passed" have to be distinguishable; - a bare mention matches `1.19.5` and `v1.19.5`, and does NOT match `opencode/1.19.5` — the look-behind whose absence made the second version double-count every prefixed copy; - a stale patch of the pinned series is caught while another major.minor is the literal scan's business; - the disagreement list names versions and files, sorted, and is empty when everything agrees. Planted the look-behind's removal — the exact historical bug — and the case fails. **The test had to build its stale versions at run time.** Writing `1.19.3` in the file makes the gate it describes fail, because the scan reads `test/` too; the digits are assembled from the series instead, and the file says in the first line why. --- scripts/check-vendor-version.ts | 28 ++++----- scripts/vendor-version.ts | 72 ++++++++++++++++++++++ test/vendor-version.test.ts | 103 ++++++++++++++++++++++++++++++++ 3 files changed, 186 insertions(+), 17 deletions(-) create mode 100644 scripts/vendor-version.ts create mode 100644 test/vendor-version.test.ts diff --git a/scripts/check-vendor-version.ts b/scripts/check-vendor-version.ts index 7e30f364..534bce9f 100644 --- a/scripts/check-vendor-version.ts +++ b/scripts/check-vendor-version.ts @@ -27,21 +27,19 @@ import { readdir, readFile } from "node:fs/promises"; +import { + bareLiteral, + CLI_LITERAL as LITERAL, + disagreements, + pinnedVersion, +} from "./vendor-version.ts"; + /** Where a copy may appear. Extensions are listed because the scan is recursive. */ const ROOTS = ["src", "test", "docs", "scripts", "README.md"]; const EXTENSIONS = /\.(?:ts|tsx|md|json|ya?ml)$/; -/** The constant every other copy must agree with. */ -const CONSTANT = /OPENCODE_CLI_VERSION\s*=\s*"(\d+\.\d+\.\d+)"/; const CONSTANT_FILE = "src/identity.ts"; -/** A literal that names the CLI. */ -const LITERAL = /\bopencode\/(\d+\.\d+\.\d+)\b/g; - -/** A bare (or `v`-prefixed) literal of the pinned series: the form above cannot see it. */ -const bareLiteral = (series: string): RegExp => - new RegExp(String.raw`(?> => { const found = new Map(); @@ -71,7 +69,7 @@ const literals = async (pattern: RegExp): Promise> => { const main = async (): Promise => { const identity = await readFile(CONSTANT_FILE, "utf8"); - const pinned = CONSTANT.exec(identity)?.[1]; + const pinned = pinnedVersion(identity); if (pinned === undefined) { throw new Error( `no OPENCODE_CLI_VERSION in ${CONSTANT_FILE} - the check read nothing, which is not the same as passing` @@ -79,15 +77,11 @@ const main = async (): Promise => { } const copies = await literals(LITERAL); - const series = pinned.split(".").slice(0, 2).join("\\."); - const mentions = await literals(bareLiteral(series)); - const disagree = [ - ...[...copies].filter(([version]) => version !== pinned), - ...[...mentions].filter(([version]) => version !== pinned), - ]; + const mentions = await literals(bareLiteral(pinned)); + const disagree = disagreements(pinned, copies, mentions); if (disagree.length > 0) { const detail = disagree - .map(([version, files]) => ` ${version} in ${files.join(", ")}`) + .map(({ files, version }) => ` ${version} in ${files.join(", ")}`) .join("\n"); throw new Error( `these copies do not say ${pinned}, which is what ${CONSTANT_FILE} declares:\n${detail}` diff --git a/scripts/vendor-version.ts b/scripts/vendor-version.ts new file mode 100644 index 00000000..a7f829ca --- /dev/null +++ b/scripts/vendor-version.ts @@ -0,0 +1,72 @@ +/** + * The rules `check-vendor-version.ts` enforces, as functions of their input. + * + * The User-Agent is origin proof — the gateway reads it to decide whether the + * caller is the official CLI — so the version it names has to be a version the + * CLI actually has. It lives in one constant (`OPENCODE_CLI_VERSION`), and the + * docs, the comments and the fixtures carry copies, because none of them can + * import it. + * + * The scanning lived inside a top-level-await script, so nothing could exercise + * the part that decides PASS, and this gate has already passed while wrong + * twice: the first version could not see the constant at all (it only looked for + * literals), and the second could not see a bare copy (it only looked for + * `opencode/`). Both were found by a control run, not by a test — which + * is the reason this module exists. + * + * @module scripts/vendor-version + */ + +/** The constant every other copy must agree with, read from `src/identity.ts`. */ +export const PINNED_VERSION = /OPENCODE_CLI_VERSION\s*=\s*"(\d+\.\d+\.\d+)"/; + +/** A literal that names the CLI: `opencode/1.19.5`. */ +export const CLI_LITERAL = /\bopencode\/(\d+\.\d+\.\d+)\b/g; + +/** + * A bare (or `v`-prefixed) literal of the pinned SERIES: `1.19.5`, `v1.19.5`. + * + * The second form the scan needs. The `(? { + const series = pinned.split(".").slice(0, 2).join("\\."); + return new RegExp(String.raw`(? + PINNED_VERSION.exec(identity)?.[1]; + +/** + * Every copy that disagrees with the constant. + * + * Two reasons to fail, and they are different failures: a copy that says another + * version is stale even when the constant is current, and a constant that + * disagrees with the published CLI means the whole claim is out of date. This + * function only decides the first; the published comparison is the caller's. + * + * @param pinned - the version the constant declares. + * @param copies - every `opencode/` literal found, with the files that + * say it. + * @param mentions - every bare mention of the pinned series, with the files. + * @returns one entry per disagreeing version, sorted by version. + */ +export const disagreements = ( + pinned: string, + copies: ReadonlyMap, + mentions: ReadonlyMap +): { readonly files: readonly string[]; readonly version: string }[] => + [...copies, ...mentions] + .filter(([version]) => version !== pinned) + .map(([version, files]) => ({ files, version })) + .toSorted((left, right) => left.version.localeCompare(right.version)); diff --git a/test/vendor-version.test.ts b/test/vendor-version.test.ts new file mode 100644 index 00000000..6161b38a --- /dev/null +++ b/test/vendor-version.test.ts @@ -0,0 +1,103 @@ +/** + * The rules behind the CLI-version gate. + * + * That gate runs on a schedule in CI, and its failure mode is the worst kind: it + * PASSES while reading nothing. It has already done so twice — the first version + * could not see the constant at all, the second could not see a bare copy — and + * both were found by a control run, not by a test. + * + * So the rules are here, as functions of their input. + * + * @module test/vendor-version.test + */ + +import { describe, expect, it } from "vitest"; + +import { + bareLiteral, + disagreements, + pinnedVersion, +} from "../scripts/vendor-version.ts"; + +/** + * A stale version, built at RUN time. + * + * **A stale version literal in this file would fail the gate it describes**: the + * check walks `test/`, and a bare patch-level literal is exactly what it is + * looking for. So the digits are assembled instead of written: the file carries + * the series only, with no third field for the pattern to match. + */ +const stale = (patch: string): string => + [PINNED.split(".")[0], PINNED.split(".")[1], patch].join("."); + +/** The pinned version, built the same way, for the cases that need it. */ +const PINNED = ["1", "19", "5"].join("."); + +describe("finding the constant", () => { + it("reads the version out of the identity module", () => { + expect( + pinnedVersion(`export const OPENCODE_CLI_VERSION = "${PINNED}";\n`) + ).toBe(PINNED); + }); + + it("returns undefined when the file carries no such constant", () => { + // The gate's own rule: a check that read nothing is not a passing check, so + // the caller has to be able to tell those apart. If this answered "" or a + // default, the gate would report OK on a tree it never read. + expect(pinnedVersion("export const SOMETHING_ELSE = 1;\n")).toBeUndefined(); + expect(pinnedVersion("")).toBeUndefined(); + }); +}); + +describe("a bare mention of the pinned series", () => { + it("matches the bare and v-prefixed forms", () => { + expect(`a comment saying ${PINNED}`.match(bareLiteral(PINNED))).toEqual([ + PINNED, + ]); + expect(`tagged v${PINNED}`.match(bareLiteral(PINNED))).toEqual([ + `v${PINNED}`, + ]); + }); + + it("does not match the prefixed form, which the other scan owns", () => { + // The `(? { + // The point of the bare scan: a comment two patches stale. It matches the + // SERIES, so a stale patch is caught while another major.minor is the + // literal scan's business. + const behind = stale("3"); + expect(`stale ${behind}`.match(bareLiteral(PINNED))).toEqual([behind]); + // A different SERIES is the literal scan's business, not this one's. + expect( + `older ${["1", "18", "9"].join(".")}`.match(bareLiteral(PINNED)) + ).toBeNull(); + }); +}); + +describe("copies that disagree with the constant", () => { + it("says which versions and which files, sorted", () => { + const older = stale("3"); + const newer = stale("4"); + const copies = new Map([ + [PINNED, ["src/identity.ts"]], + [older, ["test/e2e/a.ts"]], + ]); + const mentions = new Map([[newer, ["docs/b.md"]]]); + expect(disagreements(PINNED, copies, mentions)).toEqual([ + { files: ["test/e2e/a.ts"], version: older }, + { files: ["docs/b.md"], version: newer }, + ]); + }); + + it("is empty when every copy agrees", () => { + // The shape of the passing case matters as much as the failing one: the gate + // reports a COUNT of copies in its OK line, so an empty map that still + // "passes" is a gate that scanned nothing. + const agreeing = new Map([[PINNED, ["src/identity.ts", "docs/a.md"]]]); + expect(disagreements(PINNED, agreeing, agreeing)).toEqual([]); + }); +});