From 145445c1fde8902214de2f45ae452ad991948a78 Mon Sep 17 00:00:00 2001 From: tris203 Date: Tue, 27 Oct 2026 20:24:04 +0000 Subject: [PATCH 1/3] fix(scripts): check:changed compares forks against upstream, not a stale origin/main With no ref, check:changed diffed against the merge base with origin/main. In a fork whose main lags upstream, that counted every upstream change as the branch's own. It now takes the closest merge base across each remote's main, without assuming remote names. --- docs/operations/development.md | 3 +- scripts/check-changed.test.ts | 99 ++++++++++++++++++++++++++++++++++ scripts/check-changed.ts | 42 ++++++++++----- 3 files changed, 130 insertions(+), 14 deletions(-) create mode 100644 scripts/check-changed.test.ts diff --git a/docs/operations/development.md b/docs/operations/development.md index 54a1fe9fd9b41..278f71f308523 100644 --- a/docs/operations/development.md +++ b/docs/operations/development.md @@ -152,7 +152,8 @@ for the security model. changed files, `knip:check` when code or a manifest changes, the generated GraphQL checks, typecheck of the affected packages and their dependents, and the test files beside the changed files. It maps files to packages with [ci-changes.cjs](../../.github/scripts/ci-changes.cjs), as CI does. It -compares against the merge base with `origin/main`, or with a ref you pass. CI still runs the full +compares against the closest merge base with any remote's `main`, so a fork whose `main` lags +upstream still compares against upstream, or with a ref you pass. CI still runs the full test suites and the jobs the summary lists, so a local pass does not promise a green CI run. The steps run side by side, so a run takes about as long as its slowest step, usually knip's diff --git a/scripts/check-changed.test.ts b/scripts/check-changed.test.ts new file mode 100644 index 0000000000000..3167bcd42dd9a --- /dev/null +++ b/scripts/check-changed.test.ts @@ -0,0 +1,99 @@ +// @effect-diagnostics nodeBuiltinImport:off +import * as NodeChildProcess from "node:child_process"; +import * as NodeFS from "node:fs"; +import * as NodeOS from "node:os"; +import * as NodePath from "node:path"; +import { expect, it } from "vite-plus/test"; + +// Hooks export GIT_DIR and GIT_INDEX_FILE, and user config or the default +// ~/.config/git/ignore can ignore or sign fixture files. +const fixtureConfig = { + "core.excludesFile": NodeOS.devNull, + "user.name": "Fixture", + "user.email": "fixture@example.invalid", +}; +const env = { + ...Object.fromEntries(Object.entries(process.env).filter(([name]) => !name.startsWith("GIT_"))), + GIT_CONFIG_GLOBAL: NodeOS.devNull, + GIT_CONFIG_NOSYSTEM: "1", + GIT_CONFIG_COUNT: String(Object.keys(fixtureConfig).length), + ...Object.fromEntries( + Object.entries(fixtureConfig).flatMap(([key, value], index) => [ + [`GIT_CONFIG_KEY_${index}`, key], + [`GIT_CONFIG_VALUE_${index}`, value], + ]), + ), +}; + +// History A -> B -> C, with HEAD at C. Each remote's main points at A or B. +function fixture(remotes: Record) { + const root = NodeFS.realpathSync( + NodeFS.mkdtempSync(NodePath.join(NodeOS.tmpdir(), "t3-check-changed-")), + ); + const commits: Record = {}; + try { + const write = (file: string, content: string) => { + const path = NodePath.join(root, file); + NodeFS.mkdirSync(NodePath.dirname(path), { recursive: true }); + NodeFS.writeFileSync(path, content); + }; + for (const file of ["scripts/check-changed.ts", ".github/scripts/ci-changes.cjs"]) { + write(file, NodeFS.readFileSync(NodePath.join(import.meta.dirname, "..", file), "utf8")); + } + write(".gitignore", "bin/\n"); + write("pnpm-workspace.yaml", "packages: []\n"); + write( + "package.json", + JSON.stringify({ type: "module", scripts: { "knip:check": "knip --include files" } }), + ); + write("bin/vp", "#!/bin/sh\nexit 0\n"); + NodeFS.chmodSync(NodePath.join(root, "bin/vp"), 0o755); + const git = (...args: Array) => + NodeChildProcess.execFileSync("git", args, { cwd: root, encoding: "utf8", env }).trim(); + git("init"); + for (const name of ["A", "B", "C"]) { + if (name !== "A") write(`${name}.txt`, name); + git("add", "."); + git("commit", "-m", name); + commits[name] = git("rev-parse", "HEAD"); + } + for (const [remote, commit] of Object.entries(remotes)) { + git("remote", "add", remote, NodePath.join(root, "missing")); + git("update-ref", `refs/remotes/${remote}/main`, commits[commit]!); + } + } catch (error) { + NodeFS.rmSync(root, { recursive: true, force: true }); + throw error; + } + return { + commits, + run: (...args: Array) => + NodeChildProcess.spawnSync(process.execPath, ["scripts/check-changed.ts", ...args], { + cwd: root, + encoding: "utf8", + env: { ...env, PATH: `${NodePath.join(root, "bin")}:${process.env.PATH}` }, + }), + [Symbol.dispose]() { + NodeFS.rmSync(root, { recursive: true, force: true }); + }, + }; +} + +it.each(["upstream", "team/upstream"])( + "compares against %s/main when the fork's origin/main lags it", + (remote) => { + using project = fixture({ origin: "A", [remote]: "B" }); + const result = project.run(); + expect(result.status, result.stdout + result.stderr).toBe(0); + expect(result.stdout).toContain( + `1 files since ${project.commits.B!.slice(0, 12)} (${remote}/main)`, + ); + }, +); + +it("compares against an explicit ref instead of the closest remote main", () => { + using project = fixture({ origin: "A", upstream: "B" }); + const result = project.run("origin/main"); + expect(result.status, result.stdout + result.stderr).toBe(0); + expect(result.stdout).toContain(`2 files since ${project.commits.A!.slice(0, 12)} (origin/main)`); +}); diff --git a/scripts/check-changed.ts b/scripts/check-changed.ts index 25630728e3d66..c4889eee7ff59 100644 --- a/scripts/check-changed.ts +++ b/scripts/check-changed.ts @@ -5,7 +5,7 @@ // A dependency change also gets the unused-patch install check below. CI still // runs the full test suites and the jobs listed in the summary. // -// vp run check:changed compare against the merge base with origin/main +// vp run check:changed compare against the closest merge base with any remote main // vp run check:changed compare against the merge base with // // Uncommitted and untracked files count. Affected packages come from @@ -32,18 +32,34 @@ const git = (...args: Array) => .split("\n") .filter(Boolean); -const ref = process.argv[2] ?? "origin/main"; -const mergeBase = NodeChildProcess.spawnSync("git", ["merge-base", "HEAD", ref], { - cwd: root, - encoding: "utf8", -}); -if (mergeBase.status !== 0) { - console.error( - `check:changed: no merge base with ${ref}. Run \`git fetch origin\` or pass a ref.`, - ); +const mergeBaseWith = (ref: string) => { + const result = NodeChildProcess.spawnSync("git", ["merge-base", "HEAD", ref], { + cwd: root, + encoding: "utf8", + }); + return result.status === 0 ? result.stdout.trim() : undefined; +}; + +// A fork's own main often lags upstream, so take whichever remote main is closest to HEAD. +// Remote names can contain "/", so build each ref from `git remote` rather than a refs glob. +const explicitRef = process.argv[2]; +const candidates = explicitRef + ? [{ ref: explicitRef, rev: explicitRef }] + : git("remote").map((name) => ({ ref: `${name}/main`, rev: `refs/remotes/${name}/main` })); +const closest = candidates + .flatMap(({ ref, rev }) => { + const candidateBase = mergeBaseWith(rev); + if (!candidateBase) return []; + const ahead = Number(git("rev-list", "--count", `${candidateBase}..HEAD`)[0]); + return [{ ref, base: candidateBase, ahead }]; + }) + .sort((a, b) => a.ahead - b.ahead)[0]; +if (!closest) { + const tried = candidates.map((candidate) => candidate.ref).join(", ") || "a remote main"; + console.error(`check:changed: no merge base with ${tried}. Run \`git fetch\` or pass a ref.`); process.exit(1); } -const base = mergeBase.stdout.trim(); +const { ref, base } = closest; // --no-renames lists a rename as its old and new path, as CI does. const untracked = git("ls-files", "--others", "--exclude-standard"); const files = [ @@ -51,10 +67,10 @@ const files = [ ].sort(); if (files.length === 0) { - console.log(`check:changed: no changes since ${base.slice(0, 12)}.`); + console.log(`check:changed: no changes since ${base.slice(0, 12)} (${ref}).`); process.exit(0); } -console.log(`check:changed: ${files.length} files since ${base.slice(0, 12)}`); +console.log(`check:changed: ${files.length} files since ${base.slice(0, 12)} (${ref})`); // Ignored generated sources, declarations, env files, and install state affect check results. // Include them in the key, but skip large ignored folders that no check reads. From 8c4b3408ec0ca292261eeb5408396ff0c145b315 Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Fri, 30 Oct 2026 13:52:02 +0000 Subject: [PATCH 2/3] test(scripts): isolate comparison subprocess environment --- scripts/check-changed.test.ts | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/scripts/check-changed.test.ts b/scripts/check-changed.test.ts index a17361637a43d..b01c3556766c5 100644 --- a/scripts/check-changed.test.ts +++ b/scripts/check-changed.test.ts @@ -11,7 +11,7 @@ import * as Effect from "effect/Effect"; import * as Fiber from "effect/Fiber"; import * as Sink from "effect/Sink"; import * as Stream from "effect/Stream"; -import { ChildProcessSpawner } from "effect/process"; +import { ChildProcess, ChildProcessSpawner } from "effect/process"; import * as TestClock from "effect/testing/TestClock"; import { @@ -224,14 +224,22 @@ function fixture(remotes: Record) { }; } +const compare = (root: string, ref: Option.Option) => + Effect.gen(function* () { + const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; + return yield* comparisonBase(root, ref).pipe( + Effect.provideService( + ChildProcessSpawner.ChildProcessSpawner, + ChildProcessSpawner.make((command) => spawner.spawn(ChildProcess.setEnv(command, env))), + ), + ); + }).pipe(Effect.provide(NodeServices.layer), Effect.runPromise); + it.each(["upstream", "team/upstream"])( "compares against %s/main when the fork's origin/main lags it", async (remote) => { using project = fixture({ origin: "A", [remote]: "B" }); - const result = await comparisonBase(project.root, Option.none()).pipe( - Effect.provide(NodeServices.layer), - Effect.runPromise, - ); + const result = await compare(project.root, Option.none()); assert.strictEqual(result.base, project.commits.B); assert.strictEqual(result.ref, remote + "/main"); }, @@ -239,10 +247,7 @@ it.each(["upstream", "team/upstream"])( it("compares against an explicit ref instead of the closest remote main", async () => { using project = fixture({ origin: "A", upstream: "B" }); - const result = await comparisonBase(project.root, Option.some("origin/main")).pipe( - Effect.provide(NodeServices.layer), - Effect.runPromise, - ); + const result = await compare(project.root, Option.some("origin/main")); assert.strictEqual(result.base, project.commits.A); assert.strictEqual(result.ref, "origin/main"); }); From 3a61e7dc27b5cfddb3ef4dd344ac86353e2e418f Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Fri, 30 Oct 2026 13:55:13 +0000 Subject: [PATCH 3/3] fix(scripts): use direct effect recovery for missing merge bases --- scripts/check-changed.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/check-changed.ts b/scripts/check-changed.ts index fa547b85753f7..b23289211b90e 100644 --- a/scripts/check-changed.ts +++ b/scripts/check-changed.ts @@ -156,7 +156,7 @@ export const comparisonBase = Effect.fn("checkChanged.comparisonBase")(function* for (const { ref, rev } of candidates) { const base = yield* git(root, ["merge-base", "HEAD", rev]).pipe( Effect.map((stdout) => stdout.toString().trim()), - Effect.catch(() => Effect.succeed(undefined)), + Effect.orElseSucceed(() => undefined), ); if (!base) continue; const ahead = Number((yield* git(root, ["rev-list", "--count", base + "..HEAD"])).toString());