From 51eb024cc697a257a451132fe1c5a666c12a1df5 Mon Sep 17 00:00:00 2001 From: realDiligent Date: Fri, 10 Jul 2026 12:21:30 +0800 Subject: [PATCH] fix(engine): gate-decision twin version-bump tripwire (#4518) Extend engine-parity checks with advisory.ts/gate-advisory.ts monitoring and fail PRs that touch only one gate-decision twin without bumping packages/gittensory-engine. Co-authored-by: Cursor --- scripts/check-engine-parity.ts | 248 ++++++++++++++++++- test/unit/check-engine-parity-script.test.ts | 96 +++++++ 2 files changed, 340 insertions(+), 4 deletions(-) diff --git a/scripts/check-engine-parity.ts b/scripts/check-engine-parity.ts index a9fbfa969a..12e08337b6 100644 --- a/scripts/check-engine-parity.ts +++ b/scripts/check-engine-parity.ts @@ -5,10 +5,27 @@ // the normalized bodies diverge. Also compares the workspace-installed @jsonbored/gittensory-engine semver against // the monorepo engine package's declared version (version-skew tripwire; no live-gate round-trip). import { existsSync, readdirSync, readFileSync } from "node:fs"; +import { execFileSync } from "node:child_process"; import { join } from "node:path"; import { fileURLToPath } from "node:url"; export const ENGINE_PARITY_AREAS = Object.freeze(["review", "settings", "signals"] as const); + +/** Hand-duplicated gate-decision twins live outside src/{review,settings,signals} (#4518). */ +export const GATE_DECISION_TWIN_PAIR = Object.freeze({ + area: "gate-decision", + hostRelative: "src/rules/advisory.ts", + engineRelative: "packages/gittensory-engine/src/advisory/gate-advisory.ts", + hostFileName: "advisory.ts", + engineFileName: "gate-advisory.ts", +} as const); + +export const GATE_DECISION_CORE_MARKERS = Object.freeze([ + "function evaluateGateCheckCore", + "function isConfiguredGateBlocker", + "export function buildPullRequestAdvisory", + "export function evaluateGateCheck", +] as const); const ENGINE_SRC_ROOT = "packages/gittensory-engine/src"; const HOST_SRC_ROOT = "src"; const ENGINE_PACKAGE_JSON = "packages/gittensory-engine/package.json"; @@ -113,6 +130,186 @@ export function discoverEngineParityPairs({ return pairs; } +/** Normalize git diff paths for stable comparisons across platforms. */ +export function normalizeChangedPath(path: string): string { + return String(path ?? "") + .trim() + .replace(/\\/g, "/") + .replace(/^\.\//, ""); +} + +/** Load the explicit gate-decision twin pair for monitoring and PR version-bump enforcement. */ +export function discoverGateDecisionTwinPair({ + root, + readFile = defaultReadFile, + pair = GATE_DECISION_TWIN_PAIR, +}: { + root: string; + readFile?: EngineParityReadFile; + pair?: typeof GATE_DECISION_TWIN_PAIR; +}): EngineParityPair { + return { + area: pair.area, + fileName: `${pair.hostFileName}<->${pair.engineFileName}`, + hostRelative: pair.hostRelative, + engineRelative: pair.engineRelative, + hostText: readFile(root, pair.hostRelative), + engineText: readFile(root, pair.engineRelative), + }; +} + +/** Structural guard: both gate-decision twins still expose the core gate entrypoints (#4518). */ +export function checkGateDecisionTwinPresence({ + root, + readFile = defaultReadFile, + pair = GATE_DECISION_TWIN_PAIR, + markers = GATE_DECISION_CORE_MARKERS, +}: { + root: string; + readFile?: EngineParityReadFile; + pair?: typeof GATE_DECISION_TWIN_PAIR; + markers?: readonly string[]; +}): { failures: string[]; pairChecked: EngineParityPair } { + let twin: EngineParityPair; + try { + twin = discoverGateDecisionTwinPair({ root, readFile, pair }); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + return { + failures: [`Could not load gate-decision twin files: ${message}`], + pairChecked: { + area: pair.area, + fileName: `${pair.hostFileName}<->${pair.engineFileName}`, + hostRelative: pair.hostRelative, + engineRelative: pair.engineRelative, + hostText: "", + engineText: "", + }, + }; + } + const failures: string[] = []; + for (const marker of markers) { + if (!twin.hostText.includes(marker)) { + failures.push(`${pair.hostRelative} is missing gate-decision marker ${JSON.stringify(marker)}.`); + } + if (!twin.engineText.includes(marker)) { + failures.push(`${pair.engineRelative} is missing gate-decision marker ${JSON.stringify(marker)}.`); + } + } + return { failures, pairChecked: twin }; +} + +export function parseEnginePackageVersion(text: string): string | null { + try { + const version = JSON.parse(text).version; + return typeof version === "string" && version.trim() ? version.trim() : null; + } catch { + return null; + } +} + +/** True when the head engine package version is strictly greater than the base version. */ +export function enginePackageVersionIncreased(baseVersion: string | null, headVersion: string | null): boolean { + if (!baseVersion || !headVersion) return false; + return compareSemver(headVersion, baseVersion) > 0; +} + +/** + * Fail PRs that touch only one gate-decision twin without bumping packages/gittensory-engine/package.json. + * Updating both twins together is allowed without a version bump; a single-sided edit requires a bump. + */ +export function checkGateDecisionVersionBump({ + changedFiles, + pair = GATE_DECISION_TWIN_PAIR, + enginePackageJson = ENGINE_PACKAGE_JSON, + baseEngineVersion, + headEngineVersion, +}: { + changedFiles: readonly string[]; + pair?: typeof GATE_DECISION_TWIN_PAIR; + enginePackageJson?: string; + baseEngineVersion: string | null; + headEngineVersion: string | null; +}): { failures: string[] } { + const normalized = changedFiles.map(normalizeChangedPath).filter(Boolean); + const touchedHost = normalized.includes(pair.hostRelative); + const touchedEngine = normalized.includes(pair.engineRelative); + const touchedEnginePackage = normalized.includes(enginePackageJson); + const failures: string[] = []; + + if (!touchedHost && !touchedEngine) return { failures }; + if (touchedHost && touchedEngine) return { failures }; + if (touchedEnginePackage && enginePackageVersionIncreased(baseEngineVersion, headEngineVersion)) { + return { failures }; + } + + const touched = touchedHost ? pair.hostRelative : pair.engineRelative; + failures.push( + [ + `Gate-decision logic change in ${touched} requires either:`, + ` • a matching edit to the other twin (${touchedHost ? pair.engineRelative : pair.hostRelative}), or`, + ` • a version bump in ${enginePackageJson} (currently ${headEngineVersion ?? "unknown"} vs base ${baseEngineVersion ?? "unknown"}).`, + ].join("\n"), + ); + return { failures }; +} + +export type EngineParityExecGit = (args: string[], cwd: string) => string; + +function defaultExecGit(args: string[], cwd: string): string { + return execFileSync("git", args, { cwd, encoding: "utf8" }).trim(); +} + +/** Best-effort changed-file list for PR/push validation; returns [] outside git/PR contexts. */ +export function listChangedEngineParityFiles({ + root, + execGit = defaultExecGit, + baseRef = process.env.GITTENSORY_ENGINE_PARITY_BASE_REF ?? process.env.GITHUB_BASE_SHA ?? "", + headRef = process.env.GITTENSORY_ENGINE_PARITY_HEAD_REF ?? "HEAD", +}: { + root: string; + execGit?: EngineParityExecGit; + baseRef?: string; + headRef?: string; +}): string[] { + try { + const base = + baseRef || + execGit(["merge-base", headRef, "origin/main"], root) || + execGit(["merge-base", headRef, "upstream/main"], root); + if (!base) return []; + return execGit(["diff", "--name-only", `${base}...${headRef}`], root) + .split("\n") + .map(normalizeChangedPath) + .filter(Boolean); + } catch { + return []; + } +} + +export function readEnginePackageVersionAtRef({ + root, + ref, + enginePackageJson = ENGINE_PACKAGE_JSON, + execGit = defaultExecGit, + readFile = defaultReadFile, +}: { + root: string; + ref: string; + enginePackageJson?: string; + execGit?: EngineParityExecGit; + readFile?: EngineParityReadFile; +}): string | null { + try { + if (ref === "HEAD" || ref === "WORKTREE") { + return parseEnginePackageVersion(readFile(root, enginePackageJson)); + } + return parseEnginePackageVersion(execGit(["show", `${ref}:${enginePackageJson}`], root)); + } catch { + return null; + } +} + /** * Compare normalized bodies of every discovered pair. Returns `{ failures, pairsChecked }` — pure given injectable IO. */ @@ -259,31 +456,74 @@ export function checkMinerEngineVersionPinSync({ return { failures, expected, pin }; } -/** Run both the file-pair drift check and the version-skew check. */ +/** Run drift, gate-decision, version-skew, and optional PR version-bump checks. */ export function runEngineParityChecks(options: { root: string; readFile?: EngineParityReadFile; listDir?: EngineParityListDir; resolveInstalled?: (root: string) => string | null; readExpected?: (root: string) => string | null; + changedFiles?: readonly string[]; + baseEngineVersion?: string | null; + headEngineVersion?: string | null; }): { failures: string[]; pairsChecked: EngineParityPair[]; versionSkew: EngineVersionSkewResult; } { const drift = checkEngineParityDrift(options); + const gateDecision = checkGateDecisionTwinPresence(options); const skew = checkEngineVersionSkew(options); const pinSync = checkMinerEngineVersionPinSync(options); + const readFile = options.readFile ?? defaultReadFile; + let headEngineVersion = options.headEngineVersion; + if (headEngineVersion === undefined) { + try { + headEngineVersion = parseEnginePackageVersion(readFile(options.root, ENGINE_PACKAGE_JSON)); + } catch { + headEngineVersion = null; + } + } + const baseEngineVersion = options.baseEngineVersion ?? headEngineVersion; + const changedFiles = options.changedFiles ?? listChangedEngineParityFiles({ root: options.root }); + const versionBump = + changedFiles.length > 0 && headEngineVersion + ? checkGateDecisionVersionBump({ + changedFiles, + baseEngineVersion, + headEngineVersion, + }) + : { failures: [] as string[] }; return { - failures: [...drift.failures, ...skew.failures, ...pinSync.failures], - pairsChecked: drift.pairsChecked, + failures: [ + ...drift.failures, + ...gateDecision.failures, + ...versionBump.failures, + ...skew.failures, + ...pinSync.failures, + ], + pairsChecked: [...drift.pairsChecked, gateDecision.pairChecked], versionSkew: skew, }; } /** @internal Exported for subprocess-free unit tests of the CLI success/failure paths. */ export function runEngineParityMain(root: string = process.cwd()): number { - const { failures, pairsChecked, versionSkew } = runEngineParityChecks({ root }); + const changedFiles = listChangedEngineParityFiles({ root }); + const headEngineVersion = readEnginePackageVersionAtRef({ root, ref: "HEAD" }); + const baseEngineVersion = + changedFiles.length > 0 + ? readEnginePackageVersionAtRef({ + root, + ref: process.env.GITTENSORY_ENGINE_PARITY_BASE_REF ?? process.env.GITHUB_BASE_SHA ?? "origin/main", + }) ?? headEngineVersion + : headEngineVersion; + const { failures, pairsChecked, versionSkew } = runEngineParityChecks({ + root, + changedFiles, + baseEngineVersion, + headEngineVersion, + }); if (failures.length > 0) { console.error(`Engine-parity check found ${failures.length} issue(s):`); diff --git a/test/unit/check-engine-parity-script.test.ts b/test/unit/check-engine-parity-script.test.ts index 8173fbbe1c..cc7d4a4740 100644 --- a/test/unit/check-engine-parity-script.test.ts +++ b/test/unit/check-engine-parity-script.test.ts @@ -5,6 +5,8 @@ import { join } from "node:path"; import { describe, expect, it, vi } from "vitest"; import { checkEngineParityDrift, + checkGateDecisionTwinPresence, + checkGateDecisionVersionBump, checkEngineVersionSkew, checkMinerEngineVersionPinSync, compareSemver, @@ -12,11 +14,16 @@ import { defaultResolveInstalledEngineVersion, describeEngineVersionSkew, discoverEngineParityPairs, + discoverGateDecisionTwinPair, + enginePackageVersionIncreased, + GATE_DECISION_TWIN_PAIR, type EngineParityPair, isEngineStubPair, isThinEngineReExportShim, + normalizeChangedPath, normalizeEngineParityText, normalizeImportSpec, + parseEnginePackageVersion, runEngineParityChecks, runEngineParityMain, } from "../../scripts/check-engine-parity"; @@ -88,6 +95,89 @@ describe("check-engine-parity script", () => { expect(result.failures).toEqual([]); }); + describe("gate-decision twin coverage (#4518)", () => { + it("discovers the advisory.ts <-> gate-advisory.ts pair outside ENGINE_PARITY_AREAS", () => { + const pair = discoverGateDecisionTwinPair({ root: process.cwd() }); + expect(pair.hostRelative).toBe(GATE_DECISION_TWIN_PAIR.hostRelative); + expect(pair.engineRelative).toBe(GATE_DECISION_TWIN_PAIR.engineRelative); + expect(pair.hostText).toContain("function evaluateGateCheckCore"); + expect(pair.engineText).toContain("function evaluateGateCheckCore"); + }); + + it("passes when both gate-decision twins change together without a version bump", () => { + const result = checkGateDecisionVersionBump({ + changedFiles: [GATE_DECISION_TWIN_PAIR.hostRelative, GATE_DECISION_TWIN_PAIR.engineRelative], + baseEngineVersion: "0.2.0", + headEngineVersion: "0.2.0", + }); + expect(result.failures).toEqual([]); + }); + + it("fails when only one gate-decision twin changes without an engine package version bump", () => { + const hostOnly = checkGateDecisionVersionBump({ + changedFiles: [GATE_DECISION_TWIN_PAIR.hostRelative], + baseEngineVersion: "0.2.0", + headEngineVersion: "0.2.0", + }); + expect(hostOnly.failures).toHaveLength(1); + expect(hostOnly.failures[0]).toContain(GATE_DECISION_TWIN_PAIR.hostRelative); + + const engineOnly = checkGateDecisionVersionBump({ + changedFiles: [GATE_DECISION_TWIN_PAIR.engineRelative], + baseEngineVersion: "0.2.0", + headEngineVersion: "0.2.0", + }); + expect(engineOnly.failures).toHaveLength(1); + expect(engineOnly.failures[0]).toContain(GATE_DECISION_TWIN_PAIR.engineRelative); + }); + + it("passes when a single-sided gate-decision edit includes an engine package version bump", () => { + const result = checkGateDecisionVersionBump({ + changedFiles: [GATE_DECISION_TWIN_PAIR.hostRelative, "packages/gittensory-engine/package.json"], + baseEngineVersion: "0.2.0", + headEngineVersion: "0.2.1", + }); + expect(result.failures).toEqual([]); + expect(enginePackageVersionIncreased("0.2.0", "0.2.1")).toBe(true); + expect(parseEnginePackageVersion(JSON.stringify({ version: "0.2.1" }))).toBe("0.2.1"); + expect(normalizeChangedPath(".\\src\\rules\\advisory.ts")).toBe("src/rules/advisory.ts"); + }); + + it("includes the gate-decision twin in runEngineParityChecks pair coverage", () => { + const gateBody = [ + "export function evaluateGateCheck() {}", + "function evaluateGateCheckCore() {}", + "function isConfiguredGateBlocker() {}", + "export function buildPullRequestAdvisory() {}", + ].join("\n"); + const combined = runEngineParityChecks({ + root: "/fake", + readFile: (_root, relativePath) => { + if (relativePath === "packages/gittensory-engine/package.json") return JSON.stringify({ version: "0.2.0" }); + if (relativePath === GATE_DECISION_TWIN_PAIR.hostRelative) return gateBody; + if (relativePath === GATE_DECISION_TWIN_PAIR.engineRelative) return gateBody; + throw new Error(`unexpected read: ${relativePath}`); + }, + listDir: () => [], + resolveInstalled: () => "0.2.0", + readExpected: () => "0.2.0", + changedFiles: [GATE_DECISION_TWIN_PAIR.hostRelative], + baseEngineVersion: "0.2.0", + headEngineVersion: "0.2.0", + }); + expect(combined.pairsChecked.some((pair) => pair.area === "gate-decision")).toBe(true); + expect(combined.failures.some((failure) => failure.includes("Gate-decision logic change"))).toBe(true); + expect(checkGateDecisionTwinPresence({ + root: "/fake", + readFile: (_root, relativePath) => { + if (relativePath === GATE_DECISION_TWIN_PAIR.hostRelative) return gateBody; + if (relativePath === GATE_DECISION_TWIN_PAIR.engineRelative) return gateBody; + throw new Error(`unexpected read: ${relativePath}`); + }, + }).failures).toEqual([]); + }); + }); + describe("engine version skew", () => { it("classifies equal, behind, and ahead boundary cases", () => { expect(compareSemver("0.2.0", "0.2.0")).toBe(0); @@ -233,6 +323,12 @@ describe("check-engine-parity script", () => { if (relativePath === "packages/gittensory-engine/package.json") return JSON.stringify({ version: "0.2.0" }); if (relativePath === "src/settings/autonomy.ts") return "export const MODE = 'strict';\n"; if (relativePath === "packages/gittensory-engine/src/settings/autonomy.ts") return "export const MODE = 'relaxed';\n"; + if (relativePath === GATE_DECISION_TWIN_PAIR.hostRelative) { + return "export function evaluateGateCheck() {}\nfunction evaluateGateCheckCore() {}\nfunction isConfiguredGateBlocker() {}\nexport function buildPullRequestAdvisory() {}\n"; + } + if (relativePath === GATE_DECISION_TWIN_PAIR.engineRelative) { + return "export function evaluateGateCheck() {}\nfunction evaluateGateCheckCore() {}\nfunction isConfiguredGateBlocker() {}\nexport function buildPullRequestAdvisory() {}\n"; + } throw new Error(`unexpected read: ${relativePath}`); }, listDir: (_root: string, relativePath: string) => {