From 21462677cb88a1824be57e1da391ce756f5ea928 Mon Sep 17 00:00:00 2001 From: t3code-fork Date: Thu, 12 Nov 2026 09:04:14 +0000 Subject: [PATCH 1/6] fork: CI check that refuses database schema and migration changes T3 has no down-migrations, so a fork patch that changes the schema would strand every install that later returns to upstream. fork-checks.yml runs on every pull request and fails when the change touches the migration files or adds a schema statement to shipped source. The follow workflow runs the same script over the whole patch series. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/fork-schema-check.cjs | 92 ++++++++++++++++++++++ .github/scripts/fork-schema-check.test.cjs | 58 ++++++++++++++ .github/workflows/fork-checks.yml | 28 +++++++ 3 files changed, 178 insertions(+) create mode 100644 .github/scripts/fork-schema-check.cjs create mode 100644 .github/scripts/fork-schema-check.test.cjs create mode 100644 .github/workflows/fork-checks.yml diff --git a/.github/scripts/fork-schema-check.cjs b/.github/scripts/fork-schema-check.cjs new file mode 100644 index 0000000000000..fac080bf7b2cd --- /dev/null +++ b/.github/scripts/fork-schema-check.cjs @@ -0,0 +1,92 @@ +// Refuses fork changes to the database schema: T3 has no down-migrations, so a fork migration +// strands installs that return to upstream. Fork state goes in existing JSON fields instead. +// Usage: node .github/scripts/fork-schema-check.cjs (checks ...). + +const { execFileSync } = require("node:child_process"); + +// Files that define or apply the schema. Any change to them is refused. +const SCHEMA_PATHS = [ + /^apps\/server\/src\/persistence\/Migrations(\/|\.ts$)/, + /^apps\/server\/src\/persistence\/databaseMigrationHead\.ts$/, + /^apps\/server\/src\/persistence\/initializeV2Database\.ts$/, + /^apps\/server\/src\/persistence\/reconcileV2PreviewMigration\.ts$/, +]; + +// Schema statements added anywhere else in shipped source (tests may build their own tables). +const SCHEMA_STATEMENT = + /\b(CREATE\s+(UNIQUE\s+)?(TABLE|INDEX|VIEW|TRIGGER|VIRTUAL\s+TABLE)|ALTER\s+TABLE|DROP\s+(TABLE|INDEX|VIEW|TRIGGER))\b/i; +const SHIPPED_SOURCE = /^(apps|packages)\/.+\.(ts|tsx|js|mjs|cjs|sql)$/; +const TEST_SOURCE = /(\.test\.|\.spec\.|\/testkit\/|\/__tests__\/|\/test\/)/; + +/** + * @param {ReadonlyArray<{ path: string; addedLines: ReadonlyArray }>} files + * @returns {string[]} one reason per refused file + */ +function findSchemaChanges(files) { + const reasons = []; + for (const file of files) { + if (SCHEMA_PATHS.some((pattern) => pattern.test(file.path))) { + reasons.push(`${file.path}: changes the database schema or its migrations`); + continue; + } + if (!SHIPPED_SOURCE.test(file.path) || TEST_SOURCE.test(file.path)) continue; + const statement = file.addedLines.find((line) => SCHEMA_STATEMENT.test(line)); + if (statement) { + reasons.push(`${file.path}: adds a schema statement: ${statement.trim().slice(0, 120)}`); + } + } + return reasons; +} + +/** Parses `git diff --unified=0` output into changed files and their added lines. */ +function parseDiff(diff) { + const files = []; + let oldPath = ""; + let current; + for (const line of diff.split("\n")) { + if (line.startsWith("diff --git ")) { + current = undefined; + oldPath = ""; + } else if (current === undefined && line.startsWith("--- ")) { + oldPath = line.slice(4).replace(/^a\//, ""); + } else if (current === undefined && line.startsWith("+++ ")) { + // A deleted file's new side is /dev/null; it keeps its old path, so deleting is refused too. + const target = line.slice(4); + current = { + path: target === "/dev/null" ? oldPath : target.replace(/^b\//, ""), + addedLines: [], + }; + files.push(current); + } else if (current && line.startsWith("+")) { + current.addedLines.push(line.slice(1)); + } + } + return files; +} + +function main(argv) { + const [base, head] = argv; + if (!base || !head) { + console.error("Usage: fork-schema-check.cjs "); + return 2; + } + const diff = execFileSync( + "git", + ["diff", "--unified=0", "--no-renames", "--no-color", `${base}...${head}`], + { encoding: "utf8", maxBuffer: 256 * 1024 * 1024 }, + ); + const reasons = findSchemaChanges(parseDiff(diff)); + if (reasons.length === 0) { + console.log("No database schema changes."); + return 0; + } + console.error("Fork changes must not change the database schema (no down-migrations exist):"); + for (const reason of reasons) console.error(`- ${reason}`); + return 1; +} + +module.exports = { findSchemaChanges, parseDiff }; + +if (require.main === module) { + process.exitCode = main(process.argv.slice(2)); +} diff --git a/.github/scripts/fork-schema-check.test.cjs b/.github/scripts/fork-schema-check.test.cjs new file mode 100644 index 0000000000000..5e987b924b034 --- /dev/null +++ b/.github/scripts/fork-schema-check.test.cjs @@ -0,0 +1,58 @@ +const assert = require("node:assert/strict"); +const test = require("node:test"); +const { findSchemaChanges, parseDiff } = require("./fork-schema-check.cjs"); + +const file = (path, ...addedLines) => ({ path, addedLines }); + +test("refuses any change to a migration or the schema setup", () => { + const reasons = findSchemaChanges([ + file("apps/server/src/persistence/Migrations/082_New.ts", "export default 1;"), + file("apps/server/src/persistence/Migrations.ts", "import x from './x';"), + file("apps/server/src/persistence/databaseMigrationHead.ts", "export const HEAD = 82;"), + ]); + assert.equal(reasons.length, 3); +}); + +test("refuses a schema statement added in shipped source", () => { + const [reason] = findSchemaChanges([ + file( + "apps/server/src/feature/Store.ts", + " yield* sql`ALTER TABLE threads ADD COLUMN x TEXT`;", + ), + ]); + assert.match(reason, /Store\.ts: adds a schema statement: .*ALTER TABLE/); +}); + +test("allows schema statements in tests and ordinary changes elsewhere", () => { + assert.deepEqual( + findSchemaChanges([ + file("apps/server/src/feature/Store.test.ts", "CREATE TABLE scratch (id TEXT)"), + file("apps/server/src/persistence/testkit/Fake.ts", "CREATE INDEX i ON t (x)"), + file("apps/server/src/feature/Store.ts", "const label = 'create a table view';"), + file("docs/notes.md", "ALTER TABLE in prose"), + file("packages/contracts/src/settings.ts", " mcpProfile: Schema.optional(Schema.String),"), + ]), + [], + ); +}); + +test("parseDiff keeps added lines and the old path of a deleted file", () => { + const diff = [ + "diff --git a/apps/x.ts b/apps/x.ts", + "--- a/apps/x.ts", + "+++ b/apps/x.ts", + "@@ -1,0 +2 @@", + "+const added = 1;", + "-const removed = 1;", + "diff --git a/apps/server/src/persistence/Migrations/001_A.ts b/apps/server/src/persistence/Migrations/001_A.ts", + "--- a/apps/server/src/persistence/Migrations/001_A.ts", + "+++ /dev/null", + "@@ -1 +0,0 @@", + "-export default 1;", + ].join("\n"); + assert.deepEqual(parseDiff(diff), [ + { path: "apps/x.ts", addedLines: ["const added = 1;"] }, + { path: "apps/server/src/persistence/Migrations/001_A.ts", addedLines: [] }, + ]); + assert.equal(findSchemaChanges(parseDiff(diff)).length, 1); +}); diff --git a/.github/workflows/fork-checks.yml b/.github/workflows/fork-checks.yml new file mode 100644 index 0000000000000..4cd7f065bcaa2 --- /dev/null +++ b/.github/workflows/fork-checks.yml @@ -0,0 +1,28 @@ +name: Fork checks + +# Rules for this fork's own changes, run on every pull request. Upstream's CI needs runners a fork +# does not have; fork-tests.yml runs upstream's tests on hosted runners instead. +on: + pull_request: + +permissions: + contents: read + +jobs: + no_schema_changes: + name: No database schema changes + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Checkout + uses: actions/checkout@v6 + with: + fetch-depth: 0 + filter: blob:none + + - name: Test the check + run: node --test .github/scripts/fork-schema-check.test.cjs + + # T3 has no down-migrations, so a fork migration strands installs that return to upstream. + - name: Refuse schema and migration changes + run: node .github/scripts/fork-schema-check.cjs "${{ github.event.pull_request.base.sha }}" "${{ github.event.pull_request.head.sha }}" From e9afd0c479109766f94d53c589f430a78049c6ae Mon Sep 17 00:00:00 2001 From: t3code-fork Date: Thu, 12 Nov 2026 09:40:17 +0000 Subject: [PATCH 2/6] fork: schema check runs from the base branch and reads whole files Review round 1 of the schema check: - The workflow runs on pull_request_target, so its file and the check script come from the default branch; a pull request can no longer weaken, skip or delete the check that judges it. The pull request's commits are only fetched and read as blobs, never checked out or run. - Changes to the check's own files or to any .gitattributes are refused once the base has the check; only the owner can merge such a change. - Files the migrations import (transitively, computed from both trees) count as schema files, and a shipped file whose text holds a schema statement before or after the change is refused; statements match across lines. - Paths come from `git diff --raw -z` and texts from the blobs, so quoted non-ASCII paths and .gitattributes cannot hide a change. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/fork-schema-check.cjs | 234 ++++++++++++--- .github/scripts/fork-schema-check.test.cjs | 331 ++++++++++++++++++--- .github/workflows/fork-checks.yml | 22 +- 3 files changed, 499 insertions(+), 88 deletions(-) diff --git a/.github/scripts/fork-schema-check.cjs b/.github/scripts/fork-schema-check.cjs index fac080bf7b2cd..15d70db1dea25 100644 --- a/.github/scripts/fork-schema-check.cjs +++ b/.github/scripts/fork-schema-check.cjs @@ -1,67 +1,200 @@ // Refuses fork changes to the database schema: T3 has no down-migrations, so a fork migration -// strands installs that return to upstream. Fork state goes in existing JSON fields instead. -// Usage: node .github/scripts/fork-schema-check.cjs (checks ...). +// strands installs that return to upstream. Usage: fork-schema-check.cjs . +// Refused: migrations, schema setup and what they import; shipped files holding a schema statement +// (matched across lines); this check itself and .gitattributes. Files are read as git blobs. const { execFileSync } = require("node:child_process"); +const path = require("node:path"); -// Files that define or apply the schema. Any change to them is refused. -const SCHEMA_PATHS = [ +// Files that define or apply the schema. Any change to them, or to what they import, is refused. +const SCHEMA_ROOTS = [ /^apps\/server\/src\/persistence\/Migrations(\/|\.ts$)/, /^apps\/server\/src\/persistence\/databaseMigrationHead\.ts$/, /^apps\/server\/src\/persistence\/initializeV2Database\.ts$/, /^apps\/server\/src\/persistence\/reconcileV2PreviewMigration\.ts$/, ]; -// Schema statements added anywhere else in shipped source (tests may build their own tables). +// The check's own files. Changing them would let a PR judge itself, so it is never allowed. +const CHECK_SCRIPT = ".github/scripts/fork-schema-check.cjs"; +const CHECK_FILES = [ + CHECK_SCRIPT, + ".github/scripts/fork-schema-check.test.cjs", + ".github/workflows/fork-checks.yml", +]; +const GITATTRIBUTES = /(^|\/)\.gitattributes$/; + +// Schema statements in shipped source (tests may build their own tables). \s spans line breaks. const SCHEMA_STATEMENT = /\b(CREATE\s+(UNIQUE\s+)?(TABLE|INDEX|VIEW|TRIGGER|VIRTUAL\s+TABLE)|ALTER\s+TABLE|DROP\s+(TABLE|INDEX|VIEW|TRIGGER))\b/i; -const SHIPPED_SOURCE = /^(apps|packages)\/.+\.(ts|tsx|js|mjs|cjs|sql)$/; -const TEST_SOURCE = /(\.test\.|\.spec\.|\/testkit\/|\/__tests__\/|\/test\/)/; +const SHIPPED_SOURCE = /^(apps|packages)\/.+\.(ts|tsx|mts|cts|js|mjs|cjs|sql)$/; +const TEST_SOURCE = /(\.test\.|\.spec\.|\.testkit\.|\/testkit\/|\/__tests__\/|\/test\/)/; +const SOURCE_EXTENSIONS = ["", ".ts", ".tsx", ".mts", ".js", "/index.ts", "/index.tsx"]; +const PACKAGE_MANIFEST = /^(apps|packages)\/[^/]+\/package\.json$/; +const IMPORT_SPECIFIER = /\b(?:from|import|require)\s*\(?\s*["']([^"'\n]+)["']/g; + +const isTest = (file) => TEST_SOURCE.test(file); +const isShippedSource = (file) => SHIPPED_SOURCE.test(file) && !isTest(file); + +/** + * Files the schema roots import, transitively: relative and package subpath imports. Package + * roots (`@t3tools/contracts`) re-export the whole package, so they are not followed. + * @param {{ files: string[]; has(file: string): boolean; read(file: string): string }} tree + * @returns {Set} + */ +function schemaClosure(tree) { + const packages = packageDirectories(tree); + const queue = tree.files.filter( + (file) => SCHEMA_ROOTS.some((pattern) => pattern.test(file)) && !isTest(file), + ); + const closure = new Set(queue); + while (queue.length > 0) { + const file = queue.pop(); + const text = tree.read(file); + for (const match of text.matchAll(IMPORT_SPECIFIER)) { + const target = resolveImport(tree, packages, file, match[1]); + if (target && !isTest(target) && !closure.has(target)) { + closure.add(target); + queue.push(target); + } + } + } + return closure; +} + +function packageDirectories(tree) { + const packages = new Map(); + for (const file of tree.files) { + if (!PACKAGE_MANIFEST.test(file)) continue; + try { + const manifest = JSON.parse(tree.read(file)); + if (typeof manifest.name === "string") { + packages.set(manifest.name, { dir: path.posix.dirname(file), exports: manifest.exports }); + } + } catch { + // A broken package.json fails the build elsewhere; it defines no schema imports here. + } + } + return packages; +} + +function resolveImport(tree, packages, from, specifier) { + if (specifier.startsWith(".")) { + return resolveFile(tree, path.posix.join(path.posix.dirname(from), specifier)); + } + const scoped = specifier.match(/^(@[^/]+\/[^/]+)\/(.+)$/); + const pkg = scoped && packages.get(scoped[1]); + if (!pkg || !pkg.exports || typeof pkg.exports !== "object") return undefined; + const entry = pkg.exports[`./${scoped[2]}`]; + const target = + typeof entry === "string" ? entry : entry && (entry.import ?? entry.default ?? entry.types); + return typeof target === "string" + ? resolveFile(tree, path.posix.join(pkg.dir, target)) + : undefined; +} + +function resolveFile(tree, file) { + const candidates = SOURCE_EXTENSIONS.map((extension) => file + extension); + if (file.endsWith(".js")) candidates.unshift(file.replace(/\.js$/, ".ts")); + return candidates.find((candidate) => tree.has(candidate)); +} /** - * @param {ReadonlyArray<{ path: string; addedLines: ReadonlyArray }>} files + * @param {ReadonlyArray<{ path: string; before?: string; after?: string }>} changes + * one entry per changed path; `before`/`after` are the file texts (undefined when absent) + * @param {{ closure: ReadonlySet; baseHasCheck: boolean }} context * @returns {string[]} one reason per refused file */ -function findSchemaChanges(files) { +function findSchemaChanges(changes, { closure, baseHasCheck }) { const reasons = []; - for (const file of files) { - if (SCHEMA_PATHS.some((pattern) => pattern.test(file.path))) { - reasons.push(`${file.path}: changes the database schema or its migrations`); + for (const change of changes) { + const file = change.path; + if (CHECK_FILES.includes(file)) { + if (baseHasCheck) { + reasons.push( + `${file}: changes the schema check itself. Only the owner may do that: the check stays ` + + "red, and the owner merges past it.", + ); + } continue; } - if (!SHIPPED_SOURCE.test(file.path) || TEST_SOURCE.test(file.path)) continue; - const statement = file.addedLines.find((line) => SCHEMA_STATEMENT.test(line)); + if (GITATTRIBUTES.test(file)) { + reasons.push( + `${file}: changes how git treats files, which can hide changes from review. Only the ` + + "owner may do that.", + ); + continue; + } + if (SCHEMA_ROOTS.some((pattern) => pattern.test(file)) || closure.has(file)) { + reasons.push(`${file}: changes the database schema, its migrations or code they import`); + continue; + } + if (!isShippedSource(file)) continue; + const statement = [change.after, change.before] + .map((text) => text?.match(SCHEMA_STATEMENT)?.[0]) + .find(Boolean); if (statement) { - reasons.push(`${file.path}: adds a schema statement: ${statement.trim().slice(0, 120)}`); + reasons.push( + `${file}: holds a schema statement (${statement.replace(/\s+/g, " ")}), so any change ` + + "to it may change the schema", + ); } } return reasons; } -/** Parses `git diff --unified=0` output into changed files and their added lines. */ -function parseDiff(diff) { - const files = []; - let oldPath = ""; - let current; - for (const line of diff.split("\n")) { - if (line.startsWith("diff --git ")) { - current = undefined; - oldPath = ""; - } else if (current === undefined && line.startsWith("--- ")) { - oldPath = line.slice(4).replace(/^a\//, ""); - } else if (current === undefined && line.startsWith("+++ ")) { - // A deleted file's new side is /dev/null; it keeps its old path, so deleting is refused too. - const target = line.slice(4); - current = { - path: target === "/dev/null" ? oldPath : target.replace(/^b\//, ""), - addedLines: [], - }; - files.push(current); - } else if (current && line.startsWith("+")) { - current.addedLines.push(line.slice(1)); +/** + * Parses `git diff --raw -z --no-abbrev` output into { path, oldBlob, newBlob } entries. A side + * that is absent or a submodule (no blob) is undefined; the path is still listed. + */ +function parseRawDiff(raw) { + const fields = raw.split("\0"); + const entries = []; + for (let index = 0; index + 1 < fields.length; index += 2) { + const header = fields[index]; + if (!header.startsWith(":")) break; + const [oldMode, newMode, oldSha, newSha, status] = header.slice(1).split(" "); + if (/^[RC]/.test(status)) throw new Error(`unexpected rename in diff: ${header}`); + const blob = (mode, sha) => (/^10|^12/.test(mode) && !/^0+$/.test(sha) ? sha : undefined); + entries.push({ + path: fields[index + 1], + oldBlob: blob(oldMode, oldSha), + newBlob: blob(newMode, newSha), + }); + } + return entries; +} + +function git(args) { + return execFileSync("git", ["-c", "core.quotePath=false", ...args], { + encoding: "utf8", + maxBuffer: 512 * 1024 * 1024, + }); +} + +/** Source files and package manifests of a commit; texts are read only when the walk asks. */ +function readTree(rev) { + const blobs = new Map(); + for (const record of git(["ls-tree", "-r", "-z", "--full-tree", rev]).split("\0")) { + const match = record.match(/^\d+ blob ([0-9a-f]+)\t(.+)$/s); + if (match && (SHIPPED_SOURCE.test(match[2]) || PACKAGE_MANIFEST.test(match[2]))) { + blobs.set(match[2], match[1]); } } - return files; + return { + files: [...blobs.keys()], + has: (file) => blobs.has(file), + read: (file) => (blobs.has(file) ? readBlob(blobs.get(file)) : ""), + }; +} + +const blobCache = new Map(); +function readBlob(sha) { + if (!blobCache.has(sha)) blobCache.set(sha, git(["cat-file", "blob", sha])); + return blobCache.get(sha); +} + +function treeHas(rev, file) { + return git(["ls-tree", "--full-tree", "--name-only", rev, "--", file]).trim() === file; } function main(argv) { @@ -70,14 +203,27 @@ function main(argv) { console.error("Usage: fork-schema-check.cjs "); return 2; } - const diff = execFileSync( - "git", - ["diff", "--unified=0", "--no-renames", "--no-color", `${base}...${head}`], - { encoding: "utf8", maxBuffer: 256 * 1024 * 1024 }, + const mergeBase = git(["merge-base", base, head]).trim(); + const entries = parseRawDiff( + git(["diff", "--raw", "-z", "--no-abbrev", "--no-renames", "--no-ext-diff", mergeBase, head]), ); - const reasons = findSchemaChanges(parseDiff(diff)); + // Only shipped source is searched for statements, so only its texts are read. + const text = (entry, sha) => (sha && isShippedSource(entry.path) ? readBlob(sha) : undefined); + const changes = entries.map((entry) => ({ + path: entry.path, + before: text(entry, entry.oldBlob), + after: text(entry, entry.newBlob), + })); + const closure = new Set([ + ...schemaClosure(readTree(mergeBase)), + ...schemaClosure(readTree(head)), + ]); + const reasons = findSchemaChanges(changes, { + closure, + baseHasCheck: treeHas(base, CHECK_SCRIPT), + }); if (reasons.length === 0) { - console.log("No database schema changes."); + console.log(`No database schema changes (${changes.length} changed files checked).`); return 0; } console.error("Fork changes must not change the database schema (no down-migrations exist):"); @@ -85,7 +231,7 @@ function main(argv) { return 1; } -module.exports = { findSchemaChanges, parseDiff }; +module.exports = { findSchemaChanges, parseRawDiff, schemaClosure, CHECK_FILES }; if (require.main === module) { process.exitCode = main(process.argv.slice(2)); diff --git a/.github/scripts/fork-schema-check.test.cjs b/.github/scripts/fork-schema-check.test.cjs index 5e987b924b034..7d0ed2ee3f238 100644 --- a/.github/scripts/fork-schema-check.test.cjs +++ b/.github/scripts/fork-schema-check.test.cjs @@ -1,58 +1,307 @@ const assert = require("node:assert/strict"); +const { execFileSync, spawnSync } = require("node:child_process"); +const fs = require("node:fs"); +const os = require("node:os"); +const path = require("node:path"); const test = require("node:test"); -const { findSchemaChanges, parseDiff } = require("./fork-schema-check.cjs"); +const { CHECK_FILES, findSchemaChanges, schemaClosure } = require("./fork-schema-check.cjs"); -const file = (path, ...addedLines) => ({ path, addedLines }); +const SCRIPT = path.join(__dirname, "fork-schema-check.cjs"); +const WORKFLOW = path.join(__dirname, "..", "workflows", "fork-checks.yml"); +const MIGRATIONS = "apps/server/src/persistence/Migrations"; +const SEARCH_INDEX = "apps/server/src/orchestration-v2/messageSearchIndex.ts"; + +const TRIGGER_FILE = [ + 'import * as SqlClient from "effect/sql/SqlClient";', + "export const createMessageSearchTriggers = (sql) => sql`", + " CREATE TRIGGER message_search_update", + " AFTER UPDATE ON messages", + " WHEN (old.streaming = 0 OR new.streaming = 0)", + " BEGIN SELECT 1; END", + "`;", + "", +].join("\n"); + +const treeOf = (files) => ({ + files: Object.keys(files), + has: (file) => Object.hasOwn(files, file), + read: (file) => files[file] ?? "", +}); +const changed = (file, before, after) => ({ path: file, before, after }); +const noClosure = { closure: new Set(), baseHasCheck: true }; test("refuses any change to a migration or the schema setup", () => { - const reasons = findSchemaChanges([ - file("apps/server/src/persistence/Migrations/082_New.ts", "export default 1;"), - file("apps/server/src/persistence/Migrations.ts", "import x from './x';"), - file("apps/server/src/persistence/databaseMigrationHead.ts", "export const HEAD = 82;"), - ]); - assert.equal(reasons.length, 3); + const reasons = findSchemaChanges( + [ + changed(`${MIGRATIONS}/082_New.ts`, undefined, "export default 1;"), + changed("apps/server/src/persistence/Migrations.ts", "a", "b"), + changed("apps/server/src/persistence/databaseMigrationHead.ts", "a", "b"), + changed(`${MIGRATIONS}/001_A.ts`, "export default 1;", undefined), + ], + noClosure, + ); + assert.equal(reasons.length, 4); }); -test("refuses a schema statement added in shipped source", () => { - const [reason] = findSchemaChanges([ - file( - "apps/server/src/feature/Store.ts", - " yield* sql`ALTER TABLE threads ADD COLUMN x TEXT`;", - ), - ]); - assert.match(reason, /Store\.ts: adds a schema statement: .*ALTER TABLE/); +test("refuses a schema statement split over lines, before or after the change", () => { + const added = "export const x = sql`\n CREATE\n TABLE IF NOT EXISTS fork_state (id TEXT)`;"; + const reasons = findSchemaChanges( + [ + changed("apps/server/src/feature/Store.ts", "export {};", added), + changed(SEARCH_INDEX, TRIGGER_FILE, TRIGGER_FILE.replace(" OR new.streaming = 0", "")), + ], + noClosure, + ); + assert.equal(reasons.length, 2); + assert.match(reasons[0], /Store\.ts: holds a schema statement \(CREATE TABLE\)/); + assert.match(reasons[1], /messageSearchIndex\.ts: holds a schema statement \(CREATE TRIGGER\)/); }); test("allows schema statements in tests and ordinary changes elsewhere", () => { assert.deepEqual( - findSchemaChanges([ - file("apps/server/src/feature/Store.test.ts", "CREATE TABLE scratch (id TEXT)"), - file("apps/server/src/persistence/testkit/Fake.ts", "CREATE INDEX i ON t (x)"), - file("apps/server/src/feature/Store.ts", "const label = 'create a table view';"), - file("docs/notes.md", "ALTER TABLE in prose"), - file("packages/contracts/src/settings.ts", " mcpProfile: Schema.optional(Schema.String),"), - ]), + findSchemaChanges( + [ + changed("apps/server/src/feature/Store.test.ts", "", "CREATE TABLE scratch (id TEXT)"), + changed("apps/server/src/persistence/testkit/Fake.ts", "", "CREATE INDEX i ON t (x)"), + changed("apps/server/src/provider/Child.testkit.ts", "", "CREATE TABLE t (x)"), + changed("apps/server/src/feature/Store.ts", "", "const label = 'create a table view';"), + changed("docs/notes.md", "", "ALTER TABLE in prose"), + changed( + "packages/contracts/src/settings.ts", + "", + " mcpProfile: Schema.optional(Schema.String),", + ), + ], + noClosure, + ), [], ); }); -test("parseDiff keeps added lines and the old path of a deleted file", () => { - const diff = [ - "diff --git a/apps/x.ts b/apps/x.ts", - "--- a/apps/x.ts", - "+++ b/apps/x.ts", - "@@ -1,0 +2 @@", - "+const added = 1;", - "-const removed = 1;", - "diff --git a/apps/server/src/persistence/Migrations/001_A.ts b/apps/server/src/persistence/Migrations/001_A.ts", - "--- a/apps/server/src/persistence/Migrations/001_A.ts", - "+++ /dev/null", - "@@ -1 +0,0 @@", - "-export default 1;", - ].join("\n"); - assert.deepEqual(parseDiff(diff), [ - { path: "apps/x.ts", addedLines: ["const added = 1;"] }, - { path: "apps/server/src/persistence/Migrations/001_A.ts", addedLines: [] }, +test("refuses changes to files the migrations import", () => { + const reasons = findSchemaChanges([changed("packages/shared/src/helper.ts", "a", "b")], { + closure: new Set(["packages/shared/src/helper.ts"]), + baseHasCheck: true, + }); + assert.match(reasons[0], /helper\.ts: changes the database schema, its migrations or code/); +}); + +test("refuses changes to the check itself and to .gitattributes once the base has the check", () => { + const changes = [...CHECK_FILES, ".gitattributes", "apps/.gitattributes"].map((file) => + changed(file, "a", "b"), + ); + const reasons = findSchemaChanges(changes, noClosure); + assert.equal(reasons.length, 5); + for (const reason of reasons.slice(0, 3)) assert.match(reason, /Only the owner may do that/); + // The series that first adds the check (base is an upstream tag) may add its files. + assert.equal(findSchemaChanges(changes, { closure: new Set(), baseHasCheck: false }).length, 2); +}); + +test("the closure follows relative and package subpath imports, not package roots or tests", () => { + const closure = schemaClosure( + treeOf({ + "apps/server/src/persistence/Migrations.ts": 'import m from "./Migrations/001_A.ts";', + [`${MIGRATIONS}/001_A.ts`]: `import { t } from "../../orchestration-v2/messageSearchIndex.ts";`, + [`${MIGRATIONS}/001_A.test.ts`]: 'import x from "../../feature/OnlyTests.ts";', + [SEARCH_INDEX]: [ + 'import { a } from "@t3tools/shared/assistantCitations";', + 'import { Thread } from "@t3tools/contracts";', + 'import type { B } from "./types.js";', + ].join("\n"), + "apps/server/src/orchestration-v2/types.ts": "export type B = 1;", + "apps/server/src/feature/OnlyTests.ts": "", + "packages/shared/package.json": JSON.stringify({ + name: "@t3tools/shared", + exports: { "./assistantCitations": { import: "./src/assistantCitations.ts" } }, + }), + "packages/shared/src/assistantCitations.ts": 'import { c } from "./threadLinks.ts";', + "packages/shared/src/threadLinks.ts": "", + "packages/contracts/package.json": JSON.stringify({ + name: "@t3tools/contracts", + exports: { ".": "./src/index.ts" }, + }), + "packages/contracts/src/index.ts": "", + }), + ); + assert.deepEqual([...closure].sort(), [ + "apps/server/src/orchestration-v2/messageSearchIndex.ts", + "apps/server/src/orchestration-v2/types.ts", + "apps/server/src/persistence/Migrations.ts", + `${MIGRATIONS}/001_A.ts`, + "packages/shared/src/assistantCitations.ts", + "packages/shared/src/threadLinks.ts", ]); - assert.equal(findSchemaChanges(parseDiff(diff)).length, 1); +}); + +test("the workflow runs the default branch's copy and never runs pull request code", () => { + const workflow = fs.readFileSync(WORKFLOW, "utf8"); + const code = workflow.replace(/^\s*#.*$/gm, ""); + assert.match(code, /^on:\n {2}pull_request_target:/m); + assert.doesNotMatch(code, /^\s+pull_request:/m); + // No checkout of the pull request: actions/checkout without `ref` takes the default branch. + assert.doesNotMatch(code, /\bref:/); + assert.doesNotMatch(code, /git (checkout|switch|worktree|reset)/); + assert.match(code, /persist-credentials: false/); + // Event values reach the shell only through env, never pasted into the script. + for (const line of code.split("\n").filter((l) => /run:/.test(l))) { + assert.doesNotMatch(line, /\$\{\{/, line); + } + assert.match(code, /node \.github\/scripts\/fork-schema-check\.cjs "\$BASE_SHA" "\$HEAD_SHA"/); +}); + +// End to end: the real git in a scratch repository, the way CI calls the check. + +function scratchRepo() { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "fork-schema-check-")); + const env = { + ...process.env, + GIT_CONFIG_GLOBAL: os.devNull, + GIT_CONFIG_NOSYSTEM: "1", + GIT_AUTHOR_NAME: "test", + GIT_AUTHOR_EMAIL: "test@example.invalid", + GIT_COMMITTER_NAME: "test", + GIT_COMMITTER_EMAIL: "test@example.invalid", + }; + const git = (...args) => execFileSync("git", args, { cwd: dir, env, encoding: "utf8" }).trim(); + const write = (files) => { + for (const [file, text] of Object.entries(files)) { + const target = path.join(dir, file); + if (text === null) { + fs.rmSync(target); + continue; + } + fs.mkdirSync(path.dirname(target), { recursive: true }); + fs.writeFileSync(target, text); + } + }; + const commit = (files) => { + write(files); + git("add", "-A"); + git("commit", "-q", "--allow-empty", "-m", "change"); + return git("rev-parse", "HEAD"); + }; + git("init", "-q", "-b", "main"); + const check = (base, head) => { + const result = spawnSync(process.execPath, [SCRIPT, base, head], { + cwd: dir, + env, + encoding: "utf8", + }); + return { code: result.status, output: result.stdout + result.stderr }; + }; + return { dir, git, commit, check }; +} + +const BASE_TREE = { + [CHECK_FILES[0]]: "// the check\n", + [CHECK_FILES[1]]: "// its tests\n", + [CHECK_FILES[2]]: "name: Fork checks\n", + "apps/server/src/persistence/Migrations.ts": 'import m from "./Migrations/081_Search.ts";\n', + [`${MIGRATIONS}/081_Search.ts`]: + 'import { createMessageSearchTriggers } from "../../orchestration-v2/messageSearchIndex.ts";\n', + [SEARCH_INDEX]: TRIGGER_FILE, + "apps/server/src/feature/Store.ts": "export const x = 1;\n", +}; + +function prAgainstBase(files, baseFiles = BASE_TREE) { + const repo = scratchRepo(); + const base = repo.commit(baseFiles); + const head = repo.commit(files); + try { + return repo.check(base, head); + } finally { + fs.rmSync(repo.dir, { recursive: true, force: true }); + } +} + +test("e2e: an ordinary change passes and says how many files it checked", () => { + const result = prAgainstBase({ "apps/server/src/feature/Store.ts": "export const x = 2;\n" }); + assert.equal(result.code, 0, result.output); + assert.match(result.output, /No database schema changes \(1 changed files checked\)/); +}); + +test("e2e: a pull request that edits the check is refused, however small the edit", () => { + const result = prAgainstBase({ [CHECK_FILES[0]]: "// the check, weakened\n" }); + assert.equal(result.code, 1, result.output); + assert.match(result.output, /fork-schema-check\.cjs: changes the schema check itself/); +}); + +test("e2e: a pull request that deletes the workflow is refused", () => { + const result = prAgainstBase({ [CHECK_FILES[2]]: null }); + assert.equal(result.code, 1, result.output); + assert.match(result.output, /fork-checks\.yml: changes the schema check itself/); +}); + +test("e2e: the series that first adds the check passes (base has no check)", () => { + const { + [CHECK_FILES[0]]: _a, + [CHECK_FILES[1]]: _b, + [CHECK_FILES[2]]: _c, + ...upstream + } = BASE_TREE; + const result = prAgainstBase( + { [CHECK_FILES[0]]: "// the check\n", [CHECK_FILES[2]]: "name: Fork checks\n" }, + upstream, + ); + assert.equal(result.code, 0, result.output); +}); + +test("e2e: an edit inside a trigger body outside the migrations folder is refused", () => { + const result = prAgainstBase({ + [SEARCH_INDEX]: TRIGGER_FILE.replace(" OR new.streaming = 0", ""), + }); + assert.equal(result.code, 1, result.output); + assert.match(result.output, /messageSearchIndex\.ts: changes the database schema/); +}); + +test("e2e: a file the migrations import is refused even without a schema statement", () => { + const files = { + ...BASE_TREE, + [SEARCH_INDEX]: 'export { keyFor } from "./searchKey.ts";\n', + "apps/server/src/orchestration-v2/searchKey.ts": "export const keyFor = (x) => x;\n", + }; + const repo = scratchRepo(); + const base = repo.commit(files); + const head = repo.commit({ + "apps/server/src/orchestration-v2/searchKey.ts": "export const keyFor = (x) => `${x}!`;\n", + }); + const result = repo.check(base, head); + fs.rmSync(repo.dir, { recursive: true, force: true }); + assert.equal(result.code, 1, result.output); + assert.match(result.output, /searchKey\.ts: changes the database schema/); +}); + +test("e2e: a new store with CREATE TABLE split over lines is refused", () => { + const result = prAgainstBase({ + "apps/server/src/feature/ForkState.ts": + "export const init = (sql) => sql`\n CREATE\n TABLE IF NOT EXISTS fork_state (id TEXT)\n`;\n", + }); + assert.equal(result.code, 1, result.output); + assert.match(result.output, /ForkState\.ts: holds a schema statement \(CREATE TABLE\)/); +}); + +test("e2e: a migration with a non-ASCII name is refused", () => { + const result = prAgainstBase({ [`${MIGRATIONS}/082_Forké.ts`]: "export default 1;\n" }); + assert.equal(result.code, 1, result.output); + assert.match(result.output, /Migrations\/082_Forké\.ts: changes the database schema/); +}); + +test("e2e: .gitattributes cannot hide a migration change", () => { + const result = prAgainstBase({ + ".gitattributes": "apps/server/src/persistence/** -diff\n", + [`${MIGRATIONS}/081_Search.ts`]: `${BASE_TREE[`${MIGRATIONS}/081_Search.ts`]}// more\n`, + }); + assert.equal(result.code, 1, result.output); + assert.match(result.output, /\.gitattributes: changes how git treats files/); + assert.match(result.output, /081_Search\.ts: changes the database schema/); +}); + +test("e2e: renaming or deleting a migration is refused", () => { + const renamed = prAgainstBase({ + [`${MIGRATIONS}/081_Search.ts`]: null, + [`${MIGRATIONS}/081_Renamed.ts`]: BASE_TREE[`${MIGRATIONS}/081_Search.ts`], + }); + assert.equal(renamed.code, 1, renamed.output); + assert.match(renamed.output, /081_Search\.ts: changes/); + assert.match(renamed.output, /081_Renamed\.ts: changes/); }); diff --git a/.github/workflows/fork-checks.yml b/.github/workflows/fork-checks.yml index 4cd7f065bcaa2..daedd0abbf9a0 100644 --- a/.github/workflows/fork-checks.yml +++ b/.github/workflows/fork-checks.yml @@ -2,8 +2,14 @@ name: Fork checks # Rules for this fork's own changes, run on every pull request. Upstream's CI needs runners a fork # does not have; fork-tests.yml runs upstream's tests on hosted runners instead. +# +# pull_request_target, not pull_request: GitHub then runs this workflow file and the check script +# from the default branch, so a pull request cannot weaken, skip or delete the check that judges +# it. The pull request's commits are only fetched as git objects and read as text; nothing from +# them is checked out or run, so the base branch's permissions never reach pull request code. on: - pull_request: + pull_request_target: + types: [opened, synchronize, reopened, edited] permissions: contents: read @@ -14,15 +20,25 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 5 steps: - - name: Checkout + - name: Checkout the default branch uses: actions/checkout@v6 with: fetch-depth: 0 filter: blob:none + persist-credentials: false + + - name: Fetch the pull request's commits (not checked out) + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: git fetch --no-tags --filter=blob:none origin "$BASE_SHA" "$HEAD_SHA" - name: Test the check run: node --test .github/scripts/fork-schema-check.test.cjs # T3 has no down-migrations, so a fork migration strands installs that return to upstream. - name: Refuse schema and migration changes - run: node .github/scripts/fork-schema-check.cjs "${{ github.event.pull_request.base.sha }}" "${{ github.event.pull_request.head.sha }}" + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: node .github/scripts/fork-schema-check.cjs "$BASE_SHA" "$HEAD_SHA" From a7328e42ae42141a7a936b31473a2e7337c41a3f Mon Sep 17 00:00:00 2001 From: t3code-fork Date: Thu, 12 Nov 2026 11:19:49 +0000 Subject: [PATCH 3/6] fork: schema check judges its own files against the base's copy - A check file or .gitattributes that the branch carries unchanged from the base passes. After main is rebased onto a new upstream release, a branch from the old main is told to rebase onto main (its commits are matched on main by author, author time and subject), never to merge past the red check. - Package root imports (@t3tools/contracts) are followed to the files that declare the imported names, so migration 064's schemas in threadPullRequest.ts are covered while the rest of the package is not. - Schema statements also match across SQL comments and string joins, and with upper-case keywords around an interpolation (CREATE ${kind}). - parseRawDiff throws on output it cannot read; actions/checkout is pinned by commit SHA. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/fork-schema-check.cjs | 199 +++++++++++++++++---- .github/scripts/fork-schema-check.test.cjs | 169 ++++++++++++++++- .github/workflows/fork-checks.yml | 2 +- 3 files changed, 335 insertions(+), 35 deletions(-) diff --git a/.github/scripts/fork-schema-check.cjs b/.github/scripts/fork-schema-check.cjs index 15d70db1dea25..59e4202b92d25 100644 --- a/.github/scripts/fork-schema-check.cjs +++ b/.github/scripts/fork-schema-check.cjs @@ -1,7 +1,8 @@ // Refuses fork changes to the database schema: T3 has no down-migrations, so a fork migration // strands installs that return to upstream. Usage: fork-schema-check.cjs . -// Refused: migrations, schema setup and what they import; shipped files holding a schema statement -// (matched across lines); this check itself and .gitattributes. Files are read as git blobs. +// Refused: migrations, schema setup and what they import; shipped source files holding a schema +// statement; a PR copy of this check or of .gitattributes that differs from the base's copy; and a +// branch built on an old (since rebased) base. Files are read as git blobs. const { execFileSync } = require("node:child_process"); const path = require("node:path"); @@ -23,21 +24,40 @@ const CHECK_FILES = [ ]; const GITATTRIBUTES = /(^|\/)\.gitattributes$/; -// Schema statements in shipped source (tests may build their own tables). \s spans line breaks. -const SCHEMA_STATEMENT = - /\b(CREATE\s+(UNIQUE\s+)?(TABLE|INDEX|VIEW|TRIGGER|VIRTUAL\s+TABLE)|ALTER\s+TABLE|DROP\s+(TABLE|INDEX|VIEW|TRIGGER))\b/i; +// Schema statements in shipped source (tests may build their own tables). Between the keywords +// may stand line breaks, SQL comments and string joins ("CREATE " + "TABLE"). An interpolation +// where the object kind belongs (`CREATE ${kind}`) counts too, but only with upper-case keywords, +// so UI text such as `Create ${label}` stays free. +const GAP = String.raw`(?:\s|["'\x60+]|/\*[\s\S]*?\*/|--[^\n]*\n)+`; +const HOLE = String.raw`\$\{[^}]*\}`; +const SCHEMA_STATEMENTS = [ + new RegExp( + String.raw`\b(?:CREATE${GAP}(?:UNIQUE${GAP})?(?:TABLE|INDEX|VIEW|TRIGGER|VIRTUAL${GAP}TABLE)|ALTER${GAP}TABLE|DROP${GAP}(?:TABLE|INDEX|VIEW|TRIGGER))\b`, + "i", + ), + new RegExp(String.raw`\b(?:CREATE${GAP}(?:UNIQUE${GAP})?|ALTER${GAP}|DROP${GAP})${HOLE}`), +]; +/** The first schema statement in `text`, or undefined. */ +const schemaStatement = (text) => + text === undefined + ? undefined + : SCHEMA_STATEMENTS.map((pattern) => text.match(pattern)?.[0]).find(Boolean); const SHIPPED_SOURCE = /^(apps|packages)\/.+\.(ts|tsx|mts|cts|js|mjs|cjs|sql)$/; const TEST_SOURCE = /(\.test\.|\.spec\.|\.testkit\.|\/testkit\/|\/__tests__\/|\/test\/)/; const SOURCE_EXTENSIONS = ["", ".ts", ".tsx", ".mts", ".js", "/index.ts", "/index.tsx"]; const PACKAGE_MANIFEST = /^(apps|packages)\/[^/]+\/package\.json$/; const IMPORT_SPECIFIER = /\b(?:from|import|require)\s*\(?\s*["']([^"'\n]+)["']/g; +// `import { A, type B as C } from "x"` and `export { A } from "x"`: the names a static clause takes. +const IMPORT_CLAUSE = + /\b(?:import|export)\s+(?:type\s+)?([\w$*{}\s,]*?)\s*from\s*["']([^"'\n]+)["']/g; const isTest = (file) => TEST_SOURCE.test(file); const isShippedSource = (file) => SHIPPED_SOURCE.test(file) && !isTest(file); /** - * Files the schema roots import, transitively: relative and package subpath imports. Package - * roots (`@t3tools/contracts`) re-export the whole package, so they are not followed. + * Files the schema roots import, transitively: relative and package subpath imports, and for a + * package root (`@t3tools/contracts`) the files that declare the names imported from it. The + * root's barrel itself is not added, so the rest of the package stays free to change. * @param {{ files: string[]; has(file: string): boolean; read(file: string): string }} tree * @returns {Set} */ @@ -50,11 +70,13 @@ function schemaClosure(tree) { while (queue.length > 0) { const file = queue.pop(); const text = tree.read(file); + const names = importedNames(text); for (const match of text.matchAll(IMPORT_SPECIFIER)) { - const target = resolveImport(tree, packages, file, match[1]); - if (target && !isTest(target) && !closure.has(target)) { - closure.add(target); - queue.push(target); + for (const target of resolveImport(tree, packages, file, match[1], names.get(match[1]))) { + if (!isTest(target) && !closure.has(target)) { + closure.add(target); + queue.push(target); + } } } } @@ -77,19 +99,93 @@ function packageDirectories(tree) { return packages; } -function resolveImport(tree, packages, from, specifier) { +/** + * Per specifier, the names static clauses take from it; `*` for a namespace or a clause it cannot + * read. A specifier with no entry (dynamic import, require, side-effect import) takes everything. + * @returns {Map>} + */ +function importedNames(text) { + const names = new Map(); + for (const [, clause, specifier] of text.matchAll(IMPORT_CLAUSE)) { + const set = names.get(specifier) ?? new Set(); + names.set(specifier, set); + const braces = clause.match(/\{([^}]*)\}/); + const outside = clause.replace(/\{[^}]*\}/, "").replace(/,/g, " ").trim(); + if (/\*/.test(outside)) set.add("*"); + else if (outside) set.add("default"); + for (const part of braces ? braces[1].split(",") : []) { + const name = part.trim().replace(/^type\s+/, "").split(/\s+as\s+/)[0].trim(); + if (name) set.add(name); + } + if (!braces && !outside) set.add("*"); + } + return names; +} + +/** The files an import reaches: one file, or for a package root the files declaring `names`. */ +function resolveImport(tree, packages, from, specifier, names) { if (specifier.startsWith(".")) { - return resolveFile(tree, path.posix.join(path.posix.dirname(from), specifier)); + const target = resolveFile(tree, path.posix.join(path.posix.dirname(from), specifier)); + return target ? [target] : []; } - const scoped = specifier.match(/^(@[^/]+\/[^/]+)\/(.+)$/); - const pkg = scoped && packages.get(scoped[1]); - if (!pkg || !pkg.exports || typeof pkg.exports !== "object") return undefined; - const entry = pkg.exports[`./${scoped[2]}`]; + const parts = specifier.match(/^(@[^/]+\/[^/]+)(?:\/(.+))?$/); + const pkg = parts && packages.get(parts[1]); + if (!pkg || !pkg.exports) return []; + const exports = + typeof pkg.exports === "string" || "import" in pkg.exports || "default" in pkg.exports + ? { ".": pkg.exports } + : pkg.exports; + const entry = exports[parts[2] ? `./${parts[2]}` : "."]; const target = typeof entry === "string" ? entry : entry && (entry.import ?? entry.default ?? entry.types); - return typeof target === "string" - ? resolveFile(tree, path.posix.join(pkg.dir, target)) - : undefined; + const file = + typeof target === "string" ? resolveFile(tree, path.posix.join(pkg.dir, target)) : undefined; + if (!file) return []; + if (parts[2] || !names || names.has("*") || names.has("default")) return [file]; + const found = new Set(); + for (const name of names) declaringFiles(tree, packages, file, name, found, new Set()); + return [...found]; +} + +/** Follows `export * from` and `export { name } from` until a file declares `name`. */ +function declaringFiles(tree, packages, file, name, found, seen) { + if (seen.has(`${file}\0${name}`)) return; + seen.add(`${file}\0${name}`); + const text = tree.read(file); + const word = name.replace(/\$/g, "\\$"); + const declares = new RegExp( + String.raw`\bexport\s+(?:declare\s+)?(?:abstract\s+)?(?:const|let|var|function\*?|class|type|interface|enum|namespace)\s+${word}(?![\w$])`, + ); + const local = [...text.matchAll(/\bexport\s+(?:type\s+)?\{([^}]*)\}(?!\s*from)/g)].some( + (match) => match[1].split(",").some((part) => part.trim().split(/\s+as\s+/).pop() === name), + ); + if (declares.test(text) || local) { + found.add(file); + return; + } + for (const match of text.matchAll( + /\bexport\s+(?:type\s+)?(\*(?:\s+as\s+([\w$]+))?|\{[^}]*\})\s*from\s*["']([^"'\n]+)["']/g, + )) { + const [, clause, namespace, specifier] = match; + const targets = resolveImport(tree, packages, file, specifier, new Set(["*"])); + if (clause.startsWith("*") && !namespace) { + for (const target of targets) declaringFiles(tree, packages, target, name, found, seen); + continue; + } + const exported = namespace + ? namespace === name && "*" + : clause + .slice(1, -1) + .split(",") + .map((part) => part.trim().replace(/^type\s+/, "").split(/\s+as\s+/)) + .find((pair) => (pair[1] ?? pair[0]).trim() === name)?.[0] + .trim(); + if (!exported) continue; + for (const target of targets) { + if (exported === "*") found.add(target); + else declaringFiles(tree, packages, target, exported, found, seen); + } + } } function resolveFile(tree, file) { @@ -99,15 +195,31 @@ function resolveFile(tree, file) { } /** - * @param {ReadonlyArray<{ path: string; before?: string; after?: string }>} changes - * one entry per changed path; `before`/`after` are the file texts (undefined when absent) - * @param {{ closure: ReadonlySet; baseHasCheck: boolean }} context + * @param {ReadonlyArray<{ path: string; before?: string; after?: string; sameAsBase?: boolean }>} changes + * one entry per path the branch changes since the merge-base; `before`/`after` are the file + * texts (undefined when absent); `sameAsBase` says the branch's copy equals the base's copy + * @param {{ closure: ReadonlySet; baseHasCheck: boolean; staleCommits?: number }} context + * `staleCommits`: the branch's commits that the base already holds as rebased copies * @returns {string[]} one reason per refused file */ -function findSchemaChanges(changes, { closure, baseHasCheck }) { +function findSchemaChanges(changes, { closure, baseHasCheck, staleCommits = 0 }) { const reasons = []; + const rebase = + "this branch is based on an old main that has since been rebased; rebase it onto main"; + if (staleCommits > 0) { + reasons.push( + `${staleCommits} commit(s) of this branch are already on main as rebased copies, so ${rebase}.`, + ); + } for (const change of changes) { const file = change.path; + const guarded = CHECK_FILES.includes(file) || GITATTRIBUTES.test(file); + // The branch carries main's copy (e.g. it branched before main was rebased): nothing to judge. + if (guarded && change.sameAsBase) continue; + if (guarded && staleCommits > 0) { + reasons.push(`${file}: differs from main's copy because ${rebase}.`); + continue; + } if (CHECK_FILES.includes(file)) { if (baseHasCheck) { reasons.push( @@ -130,11 +242,11 @@ function findSchemaChanges(changes, { closure, baseHasCheck }) { } if (!isShippedSource(file)) continue; const statement = [change.after, change.before] - .map((text) => text?.match(SCHEMA_STATEMENT)?.[0]) + .map(schemaStatement) .find(Boolean); if (statement) { reasons.push( - `${file}: holds a schema statement (${statement.replace(/\s+/g, " ")}), so any change ` + + `${file}: holds a schema statement (${statement.replace(/[^\w${}]+/g, " ")}), so any change ` + "to it may change the schema", ); } @@ -149,9 +261,13 @@ function findSchemaChanges(changes, { closure, baseHasCheck }) { function parseRawDiff(raw) { const fields = raw.split("\0"); const entries = []; - for (let index = 0; index + 1 < fields.length; index += 2) { + if (fields.at(-1) === "") fields.pop(); + if (fields.length % 2 !== 0) throw new Error("cannot parse git diff --raw output (odd fields)"); + for (let index = 0; index < fields.length; index += 2) { const header = fields[index]; - if (!header.startsWith(":")) break; + if (!/^:\d+ \d+ [0-9a-f]+ [0-9a-f]+ [A-Z]\d*$/.test(header)) { + throw new Error(`cannot parse git diff --raw record: ${JSON.stringify(header)}`); + } const [oldMode, newMode, oldSha, newSha, status] = header.slice(1).split(" "); if (/^[RC]/.test(status)) throw new Error(`unexpected rename in diff: ${header}`); const blob = (mode, sha) => (/^10|^12/.test(mode) && !/^0+$/.test(sha) ? sha : undefined); @@ -194,7 +310,27 @@ function readBlob(sha) { } function treeHas(rev, file) { - return git(["ls-tree", "--full-tree", "--name-only", rev, "--", file]).trim() === file; + return blobAt(rev, file) !== undefined; +} + +/** The blob id of `file` in `rev`, or undefined when absent. */ +function blobAt(rev, file) { + const record = git(["ls-tree", "-z", "--full-tree", rev, "--", file]).split("\0")[0]; + const match = record.match(/^\d+ \w+ ([0-9a-f]+)\t(.+)$/s); + return match && match[2] === file ? match[1] : undefined; +} + +/** + * The branch's commits (merge-base..head) that the base already holds as rebased copies, matched + * by author, author time and subject: a rebase keeps all three. Needs no file contents. + */ +function staleCommitCount(mergeBase, base, head) { + const ids = (range) => + git(["log", "--no-merges", "--format=%an%x00%ae%x00%at%x00%s", range]) + .split("\n") + .filter(Boolean); + const onBase = new Set(ids(`${mergeBase}..${base}`)); + return ids(`${mergeBase}..${head}`).filter((id) => onBase.has(id)).length; } function main(argv) { @@ -209,10 +345,12 @@ function main(argv) { ); // Only shipped source is searched for statements, so only its texts are read. const text = (entry, sha) => (sha && isShippedSource(entry.path) ? readBlob(sha) : undefined); + const guarded = (file) => CHECK_FILES.includes(file) || GITATTRIBUTES.test(file); const changes = entries.map((entry) => ({ path: entry.path, before: text(entry, entry.oldBlob), after: text(entry, entry.newBlob), + sameAsBase: guarded(entry.path) ? blobAt(base, entry.path) === entry.newBlob : undefined, })); const closure = new Set([ ...schemaClosure(readTree(mergeBase)), @@ -221,6 +359,7 @@ function main(argv) { const reasons = findSchemaChanges(changes, { closure, baseHasCheck: treeHas(base, CHECK_SCRIPT), + staleCommits: staleCommitCount(mergeBase, base, head), }); if (reasons.length === 0) { console.log(`No database schema changes (${changes.length} changed files checked).`); @@ -231,7 +370,7 @@ function main(argv) { return 1; } -module.exports = { findSchemaChanges, parseRawDiff, schemaClosure, CHECK_FILES }; +module.exports = { findSchemaChanges, parseRawDiff, schemaClosure, CHECK_FILES, schemaStatement }; if (require.main === module) { process.exitCode = main(process.argv.slice(2)); diff --git a/.github/scripts/fork-schema-check.test.cjs b/.github/scripts/fork-schema-check.test.cjs index 7d0ed2ee3f238..1bb8605a2ebea 100644 --- a/.github/scripts/fork-schema-check.test.cjs +++ b/.github/scripts/fork-schema-check.test.cjs @@ -4,7 +4,12 @@ const fs = require("node:fs"); const os = require("node:os"); const path = require("node:path"); const test = require("node:test"); -const { CHECK_FILES, findSchemaChanges, schemaClosure } = require("./fork-schema-check.cjs"); +const { + CHECK_FILES, + findSchemaChanges, + parseRawDiff, + schemaClosure, +} = require("./fork-schema-check.cjs"); const SCRIPT = path.join(__dirname, "fork-schema-check.cjs"); const WORKFLOW = path.join(__dirname, "..", "workflows", "fork-checks.yml"); @@ -57,6 +62,58 @@ test("refuses a schema statement split over lines, before or after the change", assert.match(reasons[1], /messageSearchIndex\.ts: holds a schema statement \(CREATE TRIGGER\)/); }); +test("refuses statements built with interpolation, string joins or SQL comments", () => { + const shapes = [ + "export const make = (sql, kind) => sql`CREATE ${kind} fork_state (id TEXT)`;", + 'export const ddl = "CREATE " +\n "TABLE fork_state (id TEXT)";', + "export const ddl = sql`CREATE /* fork */ TABLE fork_state (id TEXT)`;", + "export const ddl = sql`ALTER -- fork\n TABLE threads ADD COLUMN x`;", + ]; + const reasons = findSchemaChanges( + shapes.map((text, index) => changed(`apps/server/src/fork/state${index}.ts`, "", text)), + noClosure, + ); + assert.equal(reasons.length, shapes.length, reasons.join("\n")); + for (const reason of reasons) assert.match(reason, /holds a schema statement/); + // UI text with the same words in title case stays free. + assert.deepEqual( + findSchemaChanges( + [changed("apps/web/src/Git.tsx", "", "const label = `Create ${terms.shortLabel}`;")], + noClosure, + ), + [], + ); +}); + +test("a check file or .gitattributes the branch carries unchanged from the base passes", () => { + const changes = [...CHECK_FILES, ".gitattributes"].map((file) => ({ + ...changed(file, undefined, "same as main"), + sameAsBase: true, + })); + assert.deepEqual(findSchemaChanges(changes, noClosure), []); +}); + +test("a branch built on an old, rebased main is told to rebase, not to merge past red", () => { + const changes = [changed(CHECK_FILES[0], undefined, "old copy")]; + const reasons = findSchemaChanges(changes, { ...noClosure, staleCommits: 2 }); + assert.equal(reasons.length, 2); + for (const reason of reasons) { + assert.match(reason, /rebase it onto main/); + assert.doesNotMatch(reason, /merges past/); + } +}); + +test("parseRawDiff fails loudly on output it cannot read", () => { + const sha = "a".repeat(40); + const zero = "0".repeat(40); + assert.deepEqual(parseRawDiff(`:000000 100644 ${zero} ${sha} A\0new.ts\0`), [ + { path: "new.ts", oldBlob: undefined, newBlob: sha }, + ]); + assert.deepEqual(parseRawDiff(""), []); + assert.throws(() => parseRawDiff("warning: something\0x\0"), /cannot parse/); + assert.throws(() => parseRawDiff(`:000000 100644 ${zero} ${sha} A\0`), /cannot parse/); +}); + test("allows schema statements in tests and ordinary changes elsewhere", () => { assert.deepEqual( findSchemaChanges( @@ -97,7 +154,7 @@ test("refuses changes to the check itself and to .gitattributes once the base ha assert.equal(findSchemaChanges(changes, { closure: new Set(), baseHasCheck: false }).length, 2); }); -test("the closure follows relative and package subpath imports, not package roots or tests", () => { +test("the closure follows relative and package subpath imports, not root barrels or tests", () => { const closure = schemaClosure( treeOf({ "apps/server/src/persistence/Migrations.ts": 'import m from "./Migrations/001_A.ts";', @@ -133,6 +190,49 @@ test("the closure follows relative and package subpath imports, not package root ]); }); +test("the closure follows a package root only to the files that declare the imported names", () => { + const closure = schemaClosure( + treeOf({ + [`${MIGRATIONS}/064_Links.ts`]: [ + "import {", + " ThreadLinkedPullRequest,", + " type ThreadPullRequestKey,", + '} from "@t3tools/contracts";', + 'import { Renamed } from "@t3tools/contracts";', + ].join("\n"), + "packages/contracts/package.json": JSON.stringify({ + name: "@t3tools/contracts", + exports: { ".": { types: "./src/index.ts", import: "./src/index.ts" } }, + }), + "packages/contracts/src/index.ts": [ + 'export * from "./settings.ts";', + 'export * from "./threadPullRequest.ts";', + 'export { Original as Renamed } from "./aliases.ts";', + ].join("\n"), + "packages/contracts/src/settings.ts": "export const Settings = 1;", + "packages/contracts/src/threadPullRequest.ts": [ + 'import { Url } from "./baseSchemas.ts";', + "export const ThreadLinkedPullRequest = Url;", + "export type ThreadPullRequestKey = string;", + ].join("\n"), + "packages/contracts/src/baseSchemas.ts": "export const Url = 1;", + "packages/contracts/src/aliases.ts": "const Original = 1;\nexport { Original };", + }), + ); + assert.deepEqual([...closure].sort(), [ + `${MIGRATIONS}/064_Links.ts`, + "packages/contracts/src/aliases.ts", + "packages/contracts/src/baseSchemas.ts", + "packages/contracts/src/threadPullRequest.ts", + ]); +}); + +test("the workflow pins actions by commit SHA", () => { + const uses = fs.readFileSync(WORKFLOW, "utf8").match(/^\s*(?:-\s*)?uses:\s*\S+/gm) ?? []; + assert.ok(uses.length > 0); + for (const line of uses) assert.match(line, /@[0-9a-f]{40}$/, line); +}); + test("the workflow runs the default branch's copy and never runs pull request code", () => { const workflow = fs.readFileSync(WORKFLOW, "utf8"); const code = workflow.replace(/^\s*#.*$/gm, ""); @@ -174,10 +274,10 @@ function scratchRepo() { fs.writeFileSync(target, text); } }; - const commit = (files) => { + const commit = (files, message = "change") => { write(files); git("add", "-A"); - git("commit", "-q", "--allow-empty", "-m", "change"); + git("commit", "-q", "--allow-empty", "-m", message); return git("rev-parse", "HEAD"); }; git("init", "-q", "-b", "main"); @@ -305,3 +405,64 @@ test("e2e: renaming or deleting a migration is refused", () => { assert.match(renamed.output, /081_Search\.ts: changes/); assert.match(renamed.output, /081_Renamed\.ts: changes/); }); + +test("e2e: after main is rebased onto a new upstream release, an old branch is told to rebase", () => { + const { + [CHECK_FILES[0]]: script, + [CHECK_FILES[1]]: tests, + [CHECK_FILES[2]]: workflow, + ...upstreamFiles + } = BASE_TREE; + const repo = scratchRepo(); + try { + const upstreamOld = repo.commit(upstreamFiles, "upstream v1"); + const checkPatch = repo.commit( + { [CHECK_FILES[0]]: script, [CHECK_FILES[1]]: tests, [CHECK_FILES[2]]: workflow }, + "fork: schema check", + ); + repo.git("branch", "old-main", checkPatch); + // A pull request opened from the old main. + repo.git("checkout", "-q", "-b", "pr", checkPatch); + const prHead = repo.commit({ "README.md": "fork notes\n" }, "docs: fork notes"); + // The follow workflow: a new upstream release, the patch series replayed on it, main moved. + repo.git("checkout", "-q", "-b", "new-main", upstreamOld); + repo.commit({ "apps/server/src/feature/Other.ts": "export {};\n" }, "upstream v2"); + repo.git("cherry-pick", checkPatch); + const newMain = repo.git("rev-parse", "HEAD"); + + const stale = repo.check(newMain, prHead); + assert.equal(stale.code, 1, stale.output); + assert.match(stale.output, /1 commit\(s\) of this branch are already on main/); + assert.match(stale.output, /rebase it onto main/); + assert.doesNotMatch(stale.output, /merges past it|changes the schema check itself/); + + // The same branch rebased onto the new main passes. + repo.git("checkout", "-q", "pr"); + repo.git("rebase", "-q", "--onto", "new-main", "old-main"); + const rebased = repo.check(newMain, repo.git("rev-parse", "HEAD")); + assert.equal(rebased.code, 0, rebased.output); + + // A branch that really edits the check is still refused against the new main. + const edit = repo.commit({ [CHECK_FILES[0]]: "// weakened\n" }, "weaken the check"); + const refused = repo.check(newMain, edit); + assert.equal(refused.code, 1, refused.output); + assert.match(refused.output, /fork-schema-check\.cjs: changes the schema check itself/); + } finally { + fs.rmSync(repo.dir, { recursive: true, force: true }); + } +}); + +test("e2e: a branch behind a main that moved forward normally is not called stale", () => { + const repo = scratchRepo(); + try { + repo.commit(BASE_TREE, "fork main"); + repo.git("checkout", "-q", "-b", "pr"); + const head = repo.commit({ "README.md": "notes\n" }, "docs: notes"); + repo.git("checkout", "-q", "main"); + const moved = repo.commit({ [CHECK_FILES[0]]: "// the check, v2\n" }, "fork: check v2"); + const result = repo.check(moved, head); + assert.equal(result.code, 0, result.output); + } finally { + fs.rmSync(repo.dir, { recursive: true, force: true }); + } +}); diff --git a/.github/workflows/fork-checks.yml b/.github/workflows/fork-checks.yml index daedd0abbf9a0..e755470fe5993 100644 --- a/.github/workflows/fork-checks.yml +++ b/.github/workflows/fork-checks.yml @@ -21,7 +21,7 @@ jobs: timeout-minutes: 5 steps: - name: Checkout the default branch - uses: actions/checkout@v6 + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 with: fetch-depth: 0 filter: blob:none From ca471421f3583135d973238dfa6ff186e5ce675b Mon Sep 17 00:00:00 2001 From: t3code-fork Date: Thu, 12 Nov 2026 11:40:55 +0000 Subject: [PATCH 4/6] fork: schema check guards re-exporting barrels and fails closed on untraced names A name imported from a package root that no file is seen to declare now guards the whole barrel and everything it re-exports, instead of dropping out of the guarded set. The tracer also knows export async function, a local export { type X }, export { default as X } from, and destructured exports. The barrels on the way to a declaring file are guarded too, so a barrel-only edit cannot re-point a name a migration imports. On a branch holding commits already on main, a real edit to a check file is reported as one, and the message no longer assumes main was rebased. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/fork-schema-check.cjs | 145 ++++++++++++++------- .github/scripts/fork-schema-check.test.cjs | 145 ++++++++++++++++++++- 2 files changed, 244 insertions(+), 46 deletions(-) diff --git a/.github/scripts/fork-schema-check.cjs b/.github/scripts/fork-schema-check.cjs index 59e4202b92d25..db12d53b2294a 100644 --- a/.github/scripts/fork-schema-check.cjs +++ b/.github/scripts/fork-schema-check.cjs @@ -2,7 +2,8 @@ // strands installs that return to upstream. Usage: fork-schema-check.cjs . // Refused: migrations, schema setup and what they import; shipped source files holding a schema // statement; a PR copy of this check or of .gitattributes that differs from the base's copy; and a -// branch built on an old (since rebased) base. Files are read as git blobs. +// branch holding commits already on the base (built on an old, since rebased base, or copying its +// commits). Files are read as git blobs. const { execFileSync } = require("node:child_process"); const path = require("node:path"); @@ -57,7 +58,9 @@ const isShippedSource = (file) => SHIPPED_SOURCE.test(file) && !isTest(file); /** * Files the schema roots import, transitively: relative and package subpath imports, and for a * package root (`@t3tools/contracts`) the files that declare the names imported from it. The - * root's barrel itself is not added, so the rest of the package stays free to change. + * barrels on the way to a declaring file are guarded too (a barrel edit could re-point a name), + * but their other re-exports are not followed, so the rest of the package stays free to change. + * A name that cannot be traced guards the whole barrel and everything it re-exports. * @param {{ files: string[]; has(file: string): boolean; read(file: string): string }} tree * @returns {Set} */ @@ -67,14 +70,18 @@ function schemaClosure(tree) { (file) => SCHEMA_ROOTS.some((pattern) => pattern.test(file)) && !isTest(file), ); const closure = new Set(queue); + const walked = new Set(queue); while (queue.length > 0) { const file = queue.pop(); const text = tree.read(file); const names = importedNames(text); for (const match of text.matchAll(IMPORT_SPECIFIER)) { - for (const target of resolveImport(tree, packages, file, match[1], names.get(match[1]))) { - if (!isTest(target) && !closure.has(target)) { + const reached = resolveImport(tree, packages, file, match[1], names.get(match[1])); + for (const target of reached.barrels) if (!isTest(target)) closure.add(target); + for (const target of reached.files) { + if (!isTest(target) && !walked.has(target)) { closure.add(target); + walked.add(target); queue.push(target); } } @@ -122,15 +129,21 @@ function importedNames(text) { return names; } -/** The files an import reaches: one file, or for a package root the files declaring `names`. */ +/** + * The files an import reaches: one file, or for a package root the files declaring `names` + * (`files`, whose own imports matter) and the barrels re-exporting them (`barrels`, guarded only). + * @returns {{ files: string[]; barrels: string[] }} + */ function resolveImport(tree, packages, from, specifier, names) { + const none = { files: [], barrels: [] }; + const whole = (file) => ({ files: [file], barrels: [] }); if (specifier.startsWith(".")) { const target = resolveFile(tree, path.posix.join(path.posix.dirname(from), specifier)); - return target ? [target] : []; + return target ? whole(target) : none; } const parts = specifier.match(/^(@[^/]+\/[^/]+)(?:\/(.+))?$/); const pkg = parts && packages.get(parts[1]); - if (!pkg || !pkg.exports) return []; + if (!pkg || !pkg.exports) return none; const exports = typeof pkg.exports === "string" || "import" in pkg.exports || "default" in pkg.exports ? { ".": pkg.exports } @@ -140,36 +153,42 @@ function resolveImport(tree, packages, from, specifier, names) { typeof entry === "string" ? entry : entry && (entry.import ?? entry.default ?? entry.types); const file = typeof target === "string" ? resolveFile(tree, path.posix.join(pkg.dir, target)) : undefined; - if (!file) return []; - if (parts[2] || !names || names.has("*") || names.has("default")) return [file]; + if (!file) return none; + if (parts[2] || !names || names.has("*") || names.has("default")) return whole(file); const found = new Set(); - for (const name of names) declaringFiles(tree, packages, file, name, found, new Set()); - return [...found]; + const barrels = new Set(); + for (const name of names) { + // A name no file is seen to declare fails closed: the barrel is walked whole. + if (!declaringFiles(tree, packages, file, name, found, barrels, new Map())) return whole(file); + } + return { files: [...found], barrels: [...barrels] }; } -/** Follows `export * from` and `export { name } from` until a file declares `name`. */ -function declaringFiles(tree, packages, file, name, found, seen) { - if (seen.has(`${file}\0${name}`)) return; - seen.add(`${file}\0${name}`); +/** + * Follows `export * from` and `export { name } from` until a file declares `name`. Adds declaring + * files to `found` and the re-exporting files on the way to `barrels`. + * @returns {boolean} whether `name` was traced to a declaring file from `file` + */ +function declaringFiles(tree, packages, file, name, found, barrels, seen) { + const key = `${file}\0${name}`; + if (seen.has(key)) return seen.get(key); + seen.set(key, false); const text = tree.read(file); - const word = name.replace(/\$/g, "\\$"); - const declares = new RegExp( - String.raw`\bexport\s+(?:declare\s+)?(?:abstract\s+)?(?:const|let|var|function\*?|class|type|interface|enum|namespace)\s+${word}(?![\w$])`, - ); - const local = [...text.matchAll(/\bexport\s+(?:type\s+)?\{([^}]*)\}(?!\s*from)/g)].some( - (match) => match[1].split(",").some((part) => part.trim().split(/\s+as\s+/).pop() === name), - ); - if (declares.test(text) || local) { + if (declaresName(text, name)) { found.add(file); - return; + seen.set(key, true); + return true; } + let traced = false; for (const match of text.matchAll( /\bexport\s+(?:type\s+)?(\*(?:\s+as\s+([\w$]+))?|\{[^}]*\})\s*from\s*["']([^"'\n]+)["']/g, )) { const [, clause, namespace, specifier] = match; - const targets = resolveImport(tree, packages, file, specifier, new Set(["*"])); + const targets = resolveImport(tree, packages, file, specifier, new Set(["*"])).files; if (clause.startsWith("*") && !namespace) { - for (const target of targets) declaringFiles(tree, packages, target, name, found, seen); + for (const target of targets) { + if (declaringFiles(tree, packages, target, name, found, barrels, seen)) traced = true; + } continue; } const exported = namespace @@ -183,9 +202,36 @@ function declaringFiles(tree, packages, file, name, found, seen) { if (!exported) continue; for (const target of targets) { if (exported === "*") found.add(target); - else declaringFiles(tree, packages, target, exported, found, seen); + if (exported === "*" || declaringFiles(tree, packages, target, exported, found, barrels, seen)) { + traced = true; + } } } + if (traced) barrels.add(file); + seen.set(key, traced); + return traced; +} + +/** + * Whether `text` declares and exports `name` itself: a declaration (`export async function`, + * `export const { name } = ...` and the like), a local `export { [type] x as name }`, or for + * `default` an `export default`. + */ +function declaresName(text, name) { + const word = name.replace(/\$/g, "\\$"); + const bound = String.raw`(? new RegExp(pattern).test(text))) return true; + return [...text.matchAll(/\bexport\s+(?:type\s+)?\{([^}]*)\}(?!\s*from)/g)].some((match) => + match[1] + .split(",") + .some((part) => part.trim().replace(/^type\s+/, "").split(/\s+as\s+/).pop().trim() === name), + ); } function resolveFile(tree, file) { @@ -195,36 +241,38 @@ function resolveFile(tree, file) { } /** - * @param {ReadonlyArray<{ path: string; before?: string; after?: string; sameAsBase?: boolean }>} changes + * @param {ReadonlyArray<{ path: string; before?: string; after?: string; sameAsBase?: boolean; fromOldMain?: boolean }>} changes * one entry per path the branch changes since the merge-base; `before`/`after` are the file - * texts (undefined when absent); `sameAsBase` says the branch's copy equals the base's copy + * texts (undefined when absent); `sameAsBase` says the branch's copy equals the base's copy; + * `fromOldMain` says the branch's copy is the one a commit already on main left there * @param {{ closure: ReadonlySet; baseHasCheck: boolean; staleCommits?: number }} context * `staleCommits`: the branch's commits that the base already holds as rebased copies * @returns {string[]} one reason per refused file */ function findSchemaChanges(changes, { closure, baseHasCheck, staleCommits = 0 }) { const reasons = []; - const rebase = - "this branch is based on an old main that has since been rebased; rebase it onto main"; if (staleCommits > 0) { reasons.push( - `${staleCommits} commit(s) of this branch are already on main as rebased copies, so ${rebase}.`, + `${staleCommits} commit(s) of this branch are already on main (it is based on an old main, ` + + "or it copies main's commits), so rebase it onto main.", ); } + const also = staleCommits > 0 ? " (also rebase onto main)" : ""; for (const change of changes) { const file = change.path; const guarded = CHECK_FILES.includes(file) || GITATTRIBUTES.test(file); // The branch carries main's copy (e.g. it branched before main was rebased): nothing to judge. if (guarded && change.sameAsBase) continue; - if (guarded && staleCommits > 0) { - reasons.push(`${file}: differs from main's copy because ${rebase}.`); + // Main's own older copy, carried by commits already on main: only the rebase is needed. + if (guarded && staleCommits > 0 && change.fromOldMain) { + reasons.push(`${file}: differs from main's copy, because this branch needs a rebase onto main.`); continue; } if (CHECK_FILES.includes(file)) { if (baseHasCheck) { reasons.push( `${file}: changes the schema check itself. Only the owner may do that: the check stays ` + - "red, and the owner merges past it.", + `red, and the owner merges past it${also}.`, ); } continue; @@ -232,7 +280,7 @@ function findSchemaChanges(changes, { closure, baseHasCheck, staleCommits = 0 }) if (GITATTRIBUTES.test(file)) { reasons.push( `${file}: changes how git treats files, which can hide changes from review. Only the ` + - "owner may do that.", + `owner may do that${also}.`, ); continue; } @@ -321,16 +369,20 @@ function blobAt(rev, file) { } /** - * The branch's commits (merge-base..head) that the base already holds as rebased copies, matched - * by author, author time and subject: a rebase keeps all three. Needs no file contents. + * The branch's commits (merge-base..head) that the base already holds as copies (rebased or + * cherry-picked), matched by author, author time and subject: both keep all three. Needs no file + * contents. Returns the branch-side commit ids. */ -function staleCommitCount(mergeBase, base, head) { +function staleCommits(mergeBase, base, head) { const ids = (range) => - git(["log", "--no-merges", "--format=%an%x00%ae%x00%at%x00%s", range]) + git(["log", "--no-merges", "--format=%H%x00%an%x00%ae%x00%at%x00%s", range]) .split("\n") - .filter(Boolean); - const onBase = new Set(ids(`${mergeBase}..${base}`)); - return ids(`${mergeBase}..${head}`).filter((id) => onBase.has(id)).length; + .filter(Boolean) + .map((line) => ({ sha: line.slice(0, line.indexOf("\0")), id: line.slice(line.indexOf("\0")) })); + const onBase = new Set(ids(`${mergeBase}..${base}`).map((commit) => commit.id)); + return ids(`${mergeBase}..${head}`) + .filter((commit) => onBase.has(commit.id)) + .map((commit) => commit.sha); } function main(argv) { @@ -346,11 +398,16 @@ function main(argv) { // Only shipped source is searched for statements, so only its texts are read. const text = (entry, sha) => (sha && isShippedSource(entry.path) ? readBlob(sha) : undefined); const guarded = (file) => CHECK_FILES.includes(file) || GITATTRIBUTES.test(file); + const stale = staleCommits(mergeBase, base, head); + // A guarded file's copy that one of those commits left: main's older copy, not a branch edit. + const fromOldMain = (entry) => + stale.some((sha) => blobAt(sha, entry.path) === entry.newBlob); const changes = entries.map((entry) => ({ path: entry.path, before: text(entry, entry.oldBlob), after: text(entry, entry.newBlob), sameAsBase: guarded(entry.path) ? blobAt(base, entry.path) === entry.newBlob : undefined, + fromOldMain: guarded(entry.path) && stale.length > 0 ? fromOldMain(entry) : undefined, })); const closure = new Set([ ...schemaClosure(readTree(mergeBase)), @@ -359,7 +416,7 @@ function main(argv) { const reasons = findSchemaChanges(changes, { closure, baseHasCheck: treeHas(base, CHECK_SCRIPT), - staleCommits: staleCommitCount(mergeBase, base, head), + staleCommits: stale.length, }); if (reasons.length === 0) { console.log(`No database schema changes (${changes.length} changed files checked).`); diff --git a/.github/scripts/fork-schema-check.test.cjs b/.github/scripts/fork-schema-check.test.cjs index 1bb8605a2ebea..0b13a4fb1d716 100644 --- a/.github/scripts/fork-schema-check.test.cjs +++ b/.github/scripts/fork-schema-check.test.cjs @@ -94,15 +94,23 @@ test("a check file or .gitattributes the branch carries unchanged from the base }); test("a branch built on an old, rebased main is told to rebase, not to merge past red", () => { - const changes = [changed(CHECK_FILES[0], undefined, "old copy")]; + const changes = [{ ...changed(CHECK_FILES[0], undefined, "old copy"), fromOldMain: true }]; const reasons = findSchemaChanges(changes, { ...noClosure, staleCommits: 2 }); assert.equal(reasons.length, 2); for (const reason of reasons) { - assert.match(reason, /rebase it onto main/); + assert.match(reason, /rebase/); assert.doesNotMatch(reason, /merges past/); } }); +test("on a stale branch, a real edit to the check is still reported as one", () => { + const changes = [{ ...changed(CHECK_FILES[0], "a", "weakened"), fromOldMain: false }]; + const reasons = findSchemaChanges(changes, { ...noClosure, staleCommits: 1 }); + assert.equal(reasons.length, 2); + assert.match(reasons[0], /already on main/); + assert.match(reasons[1], /changes the schema check itself.*\(also rebase onto main\)/); +}); + test("parseRawDiff fails loudly on output it cannot read", () => { const sha = "a".repeat(40); const zero = "0".repeat(40); @@ -185,6 +193,8 @@ test("the closure follows relative and package subpath imports, not root barrels "apps/server/src/orchestration-v2/types.ts", "apps/server/src/persistence/Migrations.ts", `${MIGRATIONS}/001_A.ts`, + // `Thread` is declared nowhere, so the barrel is guarded whole (fail closed). + "packages/contracts/src/index.ts", "packages/shared/src/assistantCitations.ts", "packages/shared/src/threadLinks.ts", ]); @@ -223,10 +233,53 @@ test("the closure follows a package root only to the files that declare the impo `${MIGRATIONS}/064_Links.ts`, "packages/contracts/src/aliases.ts", "packages/contracts/src/baseSchemas.ts", + // The barrel on the way is guarded (an edit to it could re-point a name), not followed: + // settings.ts stays free. + "packages/contracts/src/index.ts", "packages/contracts/src/threadPullRequest.ts", ]); }); +// A migration importing `S` from a package root whose barrel re-exports `./a.ts` with `reexport`. +const rootImportTree = (reexport, declaring, extra = {}) => + treeOf({ + [`${MIGRATIONS}/001_A.ts`]: 'import { S } from "@t3tools/c";', + "packages/c/package.json": JSON.stringify({ name: "@t3tools/c", exports: "./src/index.ts" }), + "packages/c/src/index.ts": `${reexport}\nexport * from "./other.ts";`, + "packages/c/src/a.ts": declaring, + "packages/c/src/other.ts": "export const Other = 1;", + ...extra, + }); + +test("the closure traces the declaration shapes a name can have", () => { + const shapes = [ + ['export * from "./a.ts";', "export async function S() {}"], + ['export * from "./a.ts";', "const S = 1;\ntype T = 2;\nexport { type T, S };"], + ['export * from "./a.ts";', "type X = 1;\nexport { type X as S };"], + ['export { default as S } from "./a.ts";', "export default function () {}"], + ['export * from "./a.ts";', "const o = { S: 1 };\nexport const { S } = o;"], + ['export * from "./a.ts";', "export const [first, S] = [1, 2];"], + ['export * from "./a.ts";', "export function* S() {}"], + ]; + for (const [reexport, declaring] of shapes) { + assert.deepEqual( + [...schemaClosure(rootImportTree(reexport, declaring))].sort(), + [`${MIGRATIONS}/001_A.ts`, "packages/c/src/a.ts", "packages/c/src/index.ts"], + declaring, + ); + } +}); + +test("a name the closure cannot trace guards the whole barrel and what it re-exports", () => { + const closure = schemaClosure(rootImportTree('export * from "./a.ts";', "export const NotS = 1;")); + assert.deepEqual([...closure].sort(), [ + `${MIGRATIONS}/001_A.ts`, + "packages/c/src/a.ts", + "packages/c/src/index.ts", + "packages/c/src/other.ts", + ]); +}); + test("the workflow pins actions by commit SHA", () => { const uses = fs.readFileSync(WORKFLOW, "utf8").match(/^\s*(?:-\s*)?uses:\s*\S+/gm) ?? []; assert.ok(uses.length > 0); @@ -406,6 +459,54 @@ test("e2e: renaming or deleting a migration is refused", () => { assert.match(renamed.output, /081_Renamed\.ts: changes/); }); +test("e2e: a barrel-only edit that re-points a name a migration imports is refused", () => { + const files = { + ...BASE_TREE, + [`${MIGRATIONS}/064_Links.ts`]: 'import { ThreadLinkedPullRequest } from "@t3tools/contracts";\n', + "packages/contracts/package.json": JSON.stringify({ + name: "@t3tools/contracts", + exports: { ".": "./src/index.ts" }, + }), + "packages/contracts/src/index.ts": + 'export * from "./orchestration.ts";\nexport * from "./threadPullRequest.ts";\n', + "packages/contracts/src/orchestration.ts": "export const ProviderKind = 1;\n", + "packages/contracts/src/threadPullRequest.ts": "export const ThreadLinkedPullRequest = 2;\n", + }; + const repo = scratchRepo(); + try { + const base = repo.commit(files); + const head = repo.commit({ + "packages/contracts/src/index.ts": `${files["packages/contracts/src/index.ts"]}export { ProviderKind as ThreadLinkedPullRequest } from "./orchestration.ts";\n`, + }); + const result = repo.check(base, head); + assert.equal(result.code, 1, result.output); + assert.match(result.output, /contracts\/src\/index\.ts: changes the database schema/); + } finally { + fs.rmSync(repo.dir, { recursive: true, force: true }); + } +}); + +test("e2e: an edit to a declaring file the closure cannot trace is refused (fail closed)", () => { + const files = { + ...BASE_TREE, + [`${MIGRATIONS}/001_A.ts`]: 'import { S } from "@t3tools/c";\n', + "packages/c/package.json": JSON.stringify({ name: "@t3tools/c", exports: "./src/index.ts" }), + "packages/c/src/index.ts": 'export * from "./a.ts";\n', + // A shape the tracer does not know: `S` is declared, but not in a way it recognises. + "packages/c/src/a.ts": "const o = { S: 1 };\nexport default o;\nexport const { S: renamed } = o;\nexports.S = 1;\n", + }; + const repo = scratchRepo(); + try { + const base = repo.commit(files); + const head = repo.commit({ "packages/c/src/a.ts": `${files["packages/c/src/a.ts"]}// edited\n` }); + const result = repo.check(base, head); + assert.equal(result.code, 1, result.output); + assert.match(result.output, /packages\/c\/src\/a\.ts: changes the database schema/); + } finally { + fs.rmSync(repo.dir, { recursive: true, force: true }); + } +}); + test("e2e: after main is rebased onto a new upstream release, an old branch is told to rebase", () => { const { [CHECK_FILES[0]]: script, @@ -434,6 +535,7 @@ test("e2e: after main is rebased onto a new upstream release, an old branch is t assert.equal(stale.code, 1, stale.output); assert.match(stale.output, /1 commit\(s\) of this branch are already on main/); assert.match(stale.output, /rebase it onto main/); + assert.doesNotMatch(stale.output, /rebased copies|has since been rebased/); assert.doesNotMatch(stale.output, /merges past it|changes the schema check itself/); // The same branch rebased onto the new main passes. @@ -466,3 +568,42 @@ test("e2e: a branch behind a main that moved forward normally is not called stal fs.rmSync(repo.dir, { recursive: true, force: true }); } }); + +test("e2e: a stale branch that also edits the check is told both", () => { + const { [CHECK_FILES[0]]: script, ...rest } = BASE_TREE; + const repo = scratchRepo(); + try { + const old = repo.commit(rest, "upstream v1"); + const patch = repo.commit({ [CHECK_FILES[0]]: script }, "fork: schema check"); + repo.git("checkout", "-q", "-b", "pr", patch); + const head = repo.commit({ [CHECK_FILES[0]]: "// weakened\n" }, "weaken the check"); + repo.git("checkout", "-q", "-b", "new-main", old); + repo.commit({ "apps/server/src/feature/Other.ts": "export {};\n" }, "upstream v2"); + repo.git("cherry-pick", patch); + const result = repo.check(repo.git("rev-parse", "HEAD"), head); + assert.equal(result.code, 1, result.output); + assert.match(result.output, /already on main/); + assert.match(result.output, /fork-schema-check\.cjs: changes the schema check itself/); + } finally { + fs.rmSync(repo.dir, { recursive: true, force: true }); + } +}); + +test("e2e: a branch behind main that only cherry-picks a main commit is told to rebase", () => { + const repo = scratchRepo(); + try { + repo.commit(BASE_TREE, "fork main"); + repo.git("checkout", "-q", "-b", "pr"); + repo.commit({ "README.md": "notes\n" }, "docs: notes"); + repo.git("checkout", "-q", "main"); + const fix = repo.commit({ "apps/server/src/feature/Store.ts": "export const x = 3;\n" }, "fix"); + repo.git("checkout", "-q", "pr"); + repo.git("cherry-pick", fix); + const result = repo.check(fix, repo.git("rev-parse", "HEAD")); + assert.equal(result.code, 1, result.output); + assert.match(result.output, /already on main \(it is based on an old main, or it copies/); + assert.doesNotMatch(result.output, /has since been rebased/); + } finally { + fs.rmSync(repo.dir, { recursive: true, force: true }); + } +}); From dee218af792e0cbfce80beaa28575c70a6e1a52b Mon Sep 17 00:00:00 2001 From: t3code-fork Date: Thu, 12 Nov 2026 11:59:00 +0000 Subject: [PATCH 5/6] fork: schema check guards package.json and refuses imports it cannot resolve - The package.json of every workspace package that schema code imports is guarded, so an edit that re-points such an import (another export target, a condition Node picks first, exports removed in favour of main) is refused. Every file any export condition can pick is guarded, and main, module and types are followed when a package has no exports. - An import that should resolve inside the repository but does not is refused, naming the import: a workspace package with no entry file, a relative import of a missing file, a #name import, a computed import() or require(), import.meta.glob. Only packages outside the workspace resolve to nothing; the output lists them. - Workspace packages come from the pnpm workspace list (scoped or not, at any depth), and a change to that list is refused. A test-path file that schema code imports is guarded like any other. - The tracer no longer counts the key in `export const { S: renamed }` as declaring S. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/fork-schema-check.cjs | 432 ++++++++++++++++----- .github/scripts/fork-schema-check.test.cjs | 280 ++++++++++++- 2 files changed, 616 insertions(+), 96 deletions(-) diff --git a/.github/scripts/fork-schema-check.cjs b/.github/scripts/fork-schema-check.cjs index db12d53b2294a..6a230331d75e0 100644 --- a/.github/scripts/fork-schema-check.cjs +++ b/.github/scripts/fork-schema-check.cjs @@ -1,9 +1,11 @@ // Refuses fork changes to the database schema: T3 has no down-migrations, so a fork migration // strands installs that return to upstream. Usage: fork-schema-check.cjs . -// Refused: migrations, schema setup and what they import; shipped source files holding a schema -// statement; a PR copy of this check or of .gitattributes that differs from the base's copy; and a -// branch holding commits already on the base (built on an old, since rebased base, or copying its -// commits). Files are read as git blobs. +// Refused: migrations, schema setup and what they import (with the package.json files that map +// those imports, and the workspace package list); an import of theirs that should resolve inside +// the repository but does not; shipped source files holding a schema statement; a PR copy of this +// check or of .gitattributes that differs from the base's copy; and a branch holding commits +// already on the base (built on an old, since rebased base, or copying its commits). Files are +// read as git blobs. const { execFileSync } = require("node:child_process"); const path = require("node:path"); @@ -46,8 +48,13 @@ const schemaStatement = (text) => const SHIPPED_SOURCE = /^(apps|packages)\/.+\.(ts|tsx|mts|cts|js|mjs|cjs|sql)$/; const TEST_SOURCE = /(\.test\.|\.spec\.|\.testkit\.|\/testkit\/|\/__tests__\/|\/test\/)/; const SOURCE_EXTENSIONS = ["", ".ts", ".tsx", ".mts", ".js", "/index.ts", "/index.tsx"]; -const PACKAGE_MANIFEST = /^(apps|packages)\/[^/]+\/package\.json$/; +// Files whose imports the closure follows; other files it reaches (JSON, SQL) are guarded whole. +const SCRIPT_SOURCE = /\.(ts|tsx|mts|cts|js|jsx|mjs|cjs)$/; +const MANIFEST = /(^|\/)package\.json$/; +const WORKSPACE_FILE = "pnpm-workspace.yaml"; const IMPORT_SPECIFIER = /\b(?:from|import|require)\s*\(?\s*["']([^"'\n]+)["']/g; +// An import whose target is computed (`import(name)`, a template, `import.meta.glob`): not followed. +const COMPUTED_IMPORT = /\b(?:import|require)\s*\(\s*(?!["'\s])|\bimport\.meta\.glob\b/g; // `import { A, type B as C } from "x"` and `export { A } from "x"`: the names a static clause takes. const IMPORT_CLAUSE = /\b(?:import|export)\s+(?:type\s+)?([\w$*{}\s,]*?)\s*from\s*["']([^"'\n]+)["']/g; @@ -56,30 +63,53 @@ const isTest = (file) => TEST_SOURCE.test(file); const isShippedSource = (file) => SHIPPED_SOURCE.test(file) && !isTest(file); /** - * Files the schema roots import, transitively: relative and package subpath imports, and for a - * package root (`@t3tools/contracts`) the files that declare the names imported from it. The - * barrels on the way to a declaring file are guarded too (a barrel edit could re-point a name), - * but their other re-exports are not followed, so the rest of the package stays free to change. - * A name that cannot be traced guards the whole barrel and everything it re-exports. + * Files the schema roots import, transitively: relative imports, and for an import of a workspace + * package its package.json (which maps the import to files) and every file any export condition + * of that entry can pick. For a package root (`@t3tools/contracts`) only the files that declare + * the imported names are followed. The barrels on the way to a declaring file are guarded too (a + * barrel edit could re-point a name), but their other re-exports are not followed, so the rest of + * the package stays free to change. A name that cannot be traced guards the whole barrel and + * everything it re-exports. + * + * Nothing that could be schema code is skipped silently: an import inside the repository that + * resolves to no file is listed in `unresolved` (the check refuses it), and only packages outside + * the workspace resolve to nothing; their names are listed in `outside`. * @param {{ files: string[]; has(file: string): boolean; read(file: string): string }} tree - * @returns {Set} + * @returns {{ closure: Set; unresolved: string[]; outside: Set }} */ -function schemaClosure(tree) { - const packages = packageDirectories(tree); +function traceSchema(tree) { const queue = tree.files.filter( (file) => SCHEMA_ROOTS.some((pattern) => pattern.test(file)) && !isTest(file), ); const closure = new Set(queue); + const unresolved = new Map(); + const ctx = { + tree, + ...workspacePackages(tree), + outside: new Set(), + guard: (file) => closure.add(file), + unresolved: (from, specifier, why) => + unresolved.set(`${from}\0${specifier}`, `${from}: imports "${specifier}", but ${why}.`), + }; const walked = new Set(queue); while (queue.length > 0) { const file = queue.pop(); + if (!SCRIPT_SOURCE.test(file)) continue; const text = tree.read(file); const names = importedNames(text); + for (const match of text.matchAll(COMPUTED_IMPORT)) { + ctx.unresolved( + file, + `${match[0].trim()}...`, + "its target is computed, so it cannot be followed", + ); + } for (const match of text.matchAll(IMPORT_SPECIFIER)) { - const reached = resolveImport(tree, packages, file, match[1], names.get(match[1])); - for (const target of reached.barrels) if (!isTest(target)) closure.add(target); + const reached = resolveImport(ctx, file, match[1], names.get(match[1])); + // A test-path file that schema code imports ships with it, so it is guarded too. + for (const target of reached.barrels) closure.add(target); for (const target of reached.files) { - if (!isTest(target) && !walked.has(target)) { + if (!walked.has(target)) { closure.add(target); walked.add(target); queue.push(target); @@ -87,23 +117,104 @@ function schemaClosure(tree) { } } } - return closure; + const why = + "the check cannot resolve it to a file in this repository, so it cannot tell which files " + + "the schema depends on. Fix the import (or teach the check this import shape)"; + return { + closure, + unresolved: [...unresolved.values()].map((reason) => reason.replace(/\.$/, `: ${why}.`)), + outside: ctx.outside, + }; +} + +/** The files the schema roots import, transitively (see traceSchema). */ +const schemaClosure = (tree) => traceSchema(tree).closure; + +/** + * The workspace's member directories as written in pnpm-workspace.yaml `packages:`, or else the + * root package.json `workspaces`; undefined when neither says (then every package.json counts). + * @returns {{ source?: string; patterns?: string[] }} + */ +function workspacePatterns(tree) { + if (tree.has(WORKSPACE_FILE)) { + const lines = tree.read(WORKSPACE_FILE).split("\n"); + const start = lines.findIndex((line) => /^packages\s*:/.test(line)); + if (start < 0) return { source: WORKSPACE_FILE }; + const inline = lines[start].match(/^packages\s*:\s*\[(.*)\]/); + const items = inline + ? inline[1].split(",") + : lines + .slice(start + 1) + .filter((line, index, rest) => rest.slice(0, index + 1).every((l) => /^(\s|#|$)/.test(l))) + .map((line) => line.match(/^\s+-\s*(.*)$/)?.[1]) + .filter((item) => item !== undefined); + return { + source: WORKSPACE_FILE, + patterns: items + .map((item) => + item + .replace(/\s+#.*$/, "") + .trim() + .replace(/^["']|["']$/g, ""), + ) + .filter(Boolean), + }; + } + try { + const { workspaces } = JSON.parse(tree.read("package.json")); + const patterns = Array.isArray(workspaces) ? workspaces : workspaces?.packages; + if (Array.isArray(patterns)) return { source: "package.json", patterns }; + } catch { + // No root manifest, or not JSON: no workspace list, so every package.json counts. + } + return {}; } -function packageDirectories(tree) { +/** + * Workspace packages by name. A name may have several directories (all are consulted). Manifests + * that are not JSON are listed in `broken`: such a package's name is unknown, so an import that + * matches no package is then not trusted to be from outside the repository. + */ +function workspacePackages(tree) { + const { patterns } = workspacePatterns(tree); + const glob = (pattern) => + new RegExp( + `^${pattern + .replace(/^\.\//, "") + .replace(/\/$/, "") + .replace(/[.+^${}()|[\]\\]/g, "\\$&") + .replace( + /\*\*\/|\*\*|\*/g, + (star) => ({ "**/": "(?:.*/)?", "**": ".*" })[star] ?? "[^/]*", + )}$`, + ); + const include = patterns?.filter((pattern) => !pattern.startsWith("!")).map(glob); + const exclude = patterns + ?.filter((pattern) => pattern.startsWith("!")) + .map((p) => glob(p.slice(1))); + const member = (dir) => + include + ? dir === "" || (include.some((re) => re.test(dir)) && !exclude.some((re) => re.test(dir))) + : !/(^|\/)node_modules(\/|$)/.test(dir); const packages = new Map(); + const broken = []; for (const file of tree.files) { - if (!PACKAGE_MANIFEST.test(file)) continue; + if (!MANIFEST.test(file)) continue; + const dir = file === "package.json" ? "" : path.posix.dirname(file); + if (!member(dir)) continue; + let manifest; try { - const manifest = JSON.parse(tree.read(file)); - if (typeof manifest.name === "string") { - packages.set(manifest.name, { dir: path.posix.dirname(file), exports: manifest.exports }); - } + manifest = JSON.parse(tree.read(file)); } catch { - // A broken package.json fails the build elsewhere; it defines no schema imports here. + broken.push(file); + continue; } + if (typeof manifest?.name !== "string") continue; + const list = packages.get(manifest.name) ?? []; + list.push({ dir, file, manifest }); + packages.set(manifest.name, list); } - return packages; + return { packages, broken }; } /** @@ -117,11 +228,18 @@ function importedNames(text) { const set = names.get(specifier) ?? new Set(); names.set(specifier, set); const braces = clause.match(/\{([^}]*)\}/); - const outside = clause.replace(/\{[^}]*\}/, "").replace(/,/g, " ").trim(); + const outside = clause + .replace(/\{[^}]*\}/, "") + .replace(/,/g, " ") + .trim(); if (/\*/.test(outside)) set.add("*"); else if (outside) set.add("default"); for (const part of braces ? braces[1].split(",") : []) { - const name = part.trim().replace(/^type\s+/, "").split(/\s+as\s+/)[0].trim(); + const name = part + .trim() + .replace(/^type\s+/, "") + .split(/\s+as\s+/)[0] + .trim(); if (name) set.add(name); } if (!braces && !outside) set.add("*"); @@ -130,38 +248,125 @@ function importedNames(text) { } /** - * The files an import reaches: one file, or for a package root the files declaring `names` - * (`files`, whose own imports matter) and the barrels re-exporting them (`barrels`, guarded only). + * The files an import reaches: the files it maps to, or for a package root the files declaring + * `names` (`files`, whose own imports matter) and the barrels re-exporting them (`barrels`, guarded + * only). A workspace package's package.json is guarded through `ctx.guard`; an import that should + * resolve inside the repository but does not is reported through `ctx.unresolved`. * @returns {{ files: string[]; barrels: string[] }} */ -function resolveImport(tree, packages, from, specifier, names) { +function resolveImport(ctx, from, specifier, names) { + const { tree } = ctx; const none = { files: [], barrels: [] }; - const whole = (file) => ({ files: [file], barrels: [] }); - if (specifier.startsWith(".")) { + if (/^\.\.?(\/|$)/.test(specifier)) { const target = resolveFile(tree, path.posix.join(path.posix.dirname(from), specifier)); - return target ? whole(target) : none; + if (target) return { files: [target], barrels: [] }; + ctx.unresolved(from, specifier, "no file in the repository matches it"); + return none; + } + if (/^[a-z][\w+.-]*:/i.test(specifier)) { + // `node:fs` and other scheme imports come from the runtime, not from this repository. + ctx.outside.add(specifier); + return none; + } + // `#name` (package.json "imports") is not traced, so it is reported like any unresolved import. + const parts = specifier.match(/^((?:@[^/@]+\/)?[^/@.#][^/]*)(?:\/(.+))?$/); + if (!parts) { + ctx.unresolved(from, specifier, "it is neither a relative import nor a package name"); + return none; + } + const [, name, subpath] = parts; + const candidates = ctx.packages.get(name); + if (!candidates) { + if (ctx.broken.length > 0) { + ctx.unresolved( + from, + specifier, + `${ctx.broken.join(", ")} is not JSON, so its name is unknown`, + ); + } else { + ctx.outside.add(name); + } + return none; + } + const entries = []; + for (const pkg of candidates) { + ctx.guard(pkg.file); + for (const target of packageTargets(pkg.manifest, subpath)) { + const file = resolveFile(tree, path.posix.join(pkg.dir, target)); + if (file && !entries.includes(file)) entries.push(file); + } + } + if (entries.length === 0) { + ctx.unresolved( + from, + specifier, + `${name} is a package of this repository (${candidates.map((pkg) => pkg.file).join(", ")}) ` + + "and none of the entry points it lists for that import is a file here", + ); + return none; } - const parts = specifier.match(/^(@[^/]+\/[^/]+)(?:\/(.+))?$/); - const pkg = parts && packages.get(parts[1]); - if (!pkg || !pkg.exports) return none; - const exports = - typeof pkg.exports === "string" || "import" in pkg.exports || "default" in pkg.exports - ? { ".": pkg.exports } - : pkg.exports; - const entry = exports[parts[2] ? `./${parts[2]}` : "."]; - const target = - typeof entry === "string" ? entry : entry && (entry.import ?? entry.default ?? entry.types); - const file = - typeof target === "string" ? resolveFile(tree, path.posix.join(pkg.dir, target)) : undefined; - if (!file) return none; - if (parts[2] || !names || names.has("*") || names.has("default")) return whole(file); - const found = new Set(); + if (subpath || !names || names.has("*") || names.has("default")) + return { files: entries, barrels: [] }; + const files = new Set(); const barrels = new Set(); - for (const name of names) { - // A name no file is seen to declare fails closed: the barrel is walked whole. - if (!declaringFiles(tree, packages, file, name, found, barrels, new Map())) return whole(file); + for (const entry of entries) { + const found = new Set(); + // A name no file is seen to declare fails closed: that entry file is walked whole. + const traced = [...names].every((each) => + declaringFiles(ctx, entry, each, found, barrels, new Map()), + ); + for (const file of traced ? found : [entry]) files.add(file); + } + return { files: [...files], barrels: [...barrels] }; +} + +/** + * Every path a package.json can map `name[/subpath]` to, for any export condition (`node`, + * `import`, `types`, `default`, custom ones, nested or in fallback arrays): the check does not + * guess which condition a tool picks, so it guards them all. Without `exports`: `main`, `module`, + * `types`, a string `browser` and `index`, or the subpath itself. + * @returns {string[]} + */ +function packageTargets(manifest, subpath) { + const key = subpath ? `./${subpath}` : "."; + const { exports } = manifest; + if (exports === undefined || exports === null) { + if (subpath) return [key]; + return [ + manifest.main, + manifest.module, + manifest.types, + manifest.typings, + manifest.browser, + "./index", + ].filter((target) => typeof target === "string"); } - return { files: [...found], barrels: [...barrels] }; + const map = + typeof exports === "object" && + !Array.isArray(exports) && + Object.keys(exports).some((each) => each.startsWith(".")) + ? exports + : { ".": exports }; + const targets = []; + const leaves = (value, star) => { + if (typeof value === "string") + targets.push(star === undefined ? value : value.replaceAll("*", star)); + else if (Array.isArray(value)) for (const each of value) leaves(each, star); + else if (value && typeof value === "object") + for (const each of Object.values(value)) leaves(each, star); + }; + if (Object.hasOwn(map, key)) leaves(map[key]); + else { + for (const [pattern, value] of Object.entries(map)) { + const star = pattern.indexOf("*"); + if (star < 0) continue; + const [prefix, suffix] = [pattern.slice(0, star), pattern.slice(star + 1)]; + if (key.length >= pattern.length - 1 && key.startsWith(prefix) && key.endsWith(suffix)) { + leaves(value, key.slice(prefix.length, key.length - suffix.length)); + } + } + } + return targets; } /** @@ -169,11 +374,11 @@ function resolveImport(tree, packages, from, specifier, names) { * files to `found` and the re-exporting files on the way to `barrels`. * @returns {boolean} whether `name` was traced to a declaring file from `file` */ -function declaringFiles(tree, packages, file, name, found, barrels, seen) { +function declaringFiles(ctx, file, name, found, barrels, seen) { const key = `${file}\0${name}`; if (seen.has(key)) return seen.get(key); seen.set(key, false); - const text = tree.read(file); + const text = ctx.tree.read(file); if (declaresName(text, name)) { found.add(file); seen.set(key, true); @@ -184,10 +389,10 @@ function declaringFiles(tree, packages, file, name, found, barrels, seen) { /\bexport\s+(?:type\s+)?(\*(?:\s+as\s+([\w$]+))?|\{[^}]*\})\s*from\s*["']([^"'\n]+)["']/g, )) { const [, clause, namespace, specifier] = match; - const targets = resolveImport(tree, packages, file, specifier, new Set(["*"])).files; + const targets = resolveImport(ctx, file, specifier, new Set(["*"])).files; if (clause.startsWith("*") && !namespace) { for (const target of targets) { - if (declaringFiles(tree, packages, target, name, found, barrels, seen)) traced = true; + if (declaringFiles(ctx, target, name, found, barrels, seen)) traced = true; } continue; } @@ -196,13 +401,18 @@ function declaringFiles(tree, packages, file, name, found, barrels, seen) { : clause .slice(1, -1) .split(",") - .map((part) => part.trim().replace(/^type\s+/, "").split(/\s+as\s+/)) + .map((part) => + part + .trim() + .replace(/^type\s+/, "") + .split(/\s+as\s+/), + ) .find((pair) => (pair[1] ?? pair[0]).trim() === name)?.[0] .trim(); if (!exported) continue; for (const target of targets) { if (exported === "*") found.add(target); - if (exported === "*" || declaringFiles(tree, packages, target, exported, found, barrels, seen)) { + if (exported === "*" || declaringFiles(ctx, target, exported, found, barrels, seen)) { traced = true; } } @@ -220,20 +430,42 @@ function declaringFiles(tree, packages, file, name, found, barrels, seen) { function declaresName(text, name) { const word = name.replace(/\$/g, "\\$"); const bound = String.raw`(? new RegExp(pattern).test(text))) return true; + const declaration = String.raw`\bexport\s+(?:declare\s+)?(?:abstract\s+)?(?:async\s+)?(?:const|let|var|function\s*\*?|class|type|interface|enum|namespace|module)\s*${bound}`; + if (new RegExp(declaration).test(text)) return true; + if (name === "default" && /\bexport\s+default\b/.test(text)) return true; + if (destructuresName(text, word)) return true; return [...text.matchAll(/\bexport\s+(?:type\s+)?\{([^}]*)\}(?!\s*from)/g)].some((match) => - match[1] - .split(",") - .some((part) => part.trim().replace(/^type\s+/, "").split(/\s+as\s+/).pop().trim() === name), + match[1].split(",").some( + (part) => + part + .trim() + .replace(/^type\s+/, "") + .split(/\s+as\s+/) + .pop() + .trim() === name, + ), ); } +/** + * Whether an `export const { ... } = ` or `export const [ ... ] = ` pattern binds `word`. A binding + * stands after `{`, `[`, `,`, `:` or `...` and is not a key: in `{ S: renamed }` only `renamed` is + * bound. + */ +function destructuresName(text, word) { + const binding = new RegExp(String.raw`(?:^|[{[,:]|\.\.\.)\s*${word}(?![\w$])(?!\s*:)`); + for (const match of text.matchAll(/\bexport\s+(?:const|let|var)\s*(?=[{[])/g)) { + let depth = 0; + let end = match.index + match[0].length; + for (; end < text.length; end += 1) { + if (text[end] === "{" || text[end] === "[") depth += 1; + else if ((text[end] === "}" || text[end] === "]") && --depth === 0) break; + } + if (binding.test(text.slice(match.index + match[0].length + 1, end))) return true; + } + return false; +} + function resolveFile(tree, file) { const candidates = SOURCE_EXTENSIONS.map((extension) => file + extension); if (file.endsWith(".js")) candidates.unshift(file.replace(/\.js$/, ".ts")); @@ -265,7 +497,9 @@ function findSchemaChanges(changes, { closure, baseHasCheck, staleCommits = 0 }) if (guarded && change.sameAsBase) continue; // Main's own older copy, carried by commits already on main: only the rebase is needed. if (guarded && staleCommits > 0 && change.fromOldMain) { - reasons.push(`${file}: differs from main's copy, because this branch needs a rebase onto main.`); + reasons.push( + `${file}: differs from main's copy, because this branch needs a rebase onto main.`, + ); continue; } if (CHECK_FILES.includes(file)) { @@ -289,9 +523,7 @@ function findSchemaChanges(changes, { closure, baseHasCheck, staleCommits = 0 }) continue; } if (!isShippedSource(file)) continue; - const statement = [change.after, change.before] - .map(schemaStatement) - .find(Boolean); + const statement = [change.after, change.before].map(schemaStatement).find(Boolean); if (statement) { reasons.push( `${file}: holds a schema statement (${statement.replace(/[^\w${}]+/g, " ")}), so any change ` + @@ -335,14 +567,12 @@ function git(args) { }); } -/** Source files and package manifests of a commit; texts are read only when the walk asks. */ +/** The files of a commit (any import may name any file); texts are read only when the walk asks. */ function readTree(rev) { const blobs = new Map(); for (const record of git(["ls-tree", "-r", "-z", "--full-tree", rev]).split("\0")) { const match = record.match(/^\d+ blob ([0-9a-f]+)\t(.+)$/s); - if (match && (SHIPPED_SOURCE.test(match[2]) || PACKAGE_MANIFEST.test(match[2]))) { - blobs.set(match[2], match[1]); - } + if (match) blobs.set(match[2], match[1]); } return { files: [...blobs.keys()], @@ -378,7 +608,10 @@ function staleCommits(mergeBase, base, head) { git(["log", "--no-merges", "--format=%H%x00%an%x00%ae%x00%at%x00%s", range]) .split("\n") .filter(Boolean) - .map((line) => ({ sha: line.slice(0, line.indexOf("\0")), id: line.slice(line.indexOf("\0")) })); + .map((line) => ({ + sha: line.slice(0, line.indexOf("\0")), + id: line.slice(line.indexOf("\0")), + })); const onBase = new Set(ids(`${mergeBase}..${base}`).map((commit) => commit.id)); return ids(`${mergeBase}..${head}`) .filter((commit) => onBase.has(commit.id)) @@ -400,8 +633,7 @@ function main(argv) { const guarded = (file) => CHECK_FILES.includes(file) || GITATTRIBUTES.test(file); const stale = staleCommits(mergeBase, base, head); // A guarded file's copy that one of those commits left: main's older copy, not a branch edit. - const fromOldMain = (entry) => - stale.some((sha) => blobAt(sha, entry.path) === entry.newBlob); + const fromOldMain = (entry) => stale.some((sha) => blobAt(sha, entry.path) === entry.newBlob); const changes = entries.map((entry) => ({ path: entry.path, before: text(entry, entry.oldBlob), @@ -409,15 +641,30 @@ function main(argv) { sameAsBase: guarded(entry.path) ? blobAt(base, entry.path) === entry.newBlob : undefined, fromOldMain: guarded(entry.path) && stale.length > 0 ? fromOldMain(entry) : undefined, })); - const closure = new Set([ - ...schemaClosure(readTree(mergeBase)), - ...schemaClosure(readTree(head)), - ]); - const reasons = findSchemaChanges(changes, { - closure, - baseHasCheck: treeHas(base, CHECK_SCRIPT), - staleCommits: stale.length, - }); + const trees = [readTree(mergeBase), readTree(head)]; + const traces = trees.map(traceSchema); + const closure = new Set(traces.flatMap((trace) => [...trace.closure])); + const reasons = [...new Set(traces.flatMap((trace) => trace.unresolved))]; + // Which directories are workspace packages decides what a package import resolves to. + const [before, after] = trees.map(workspacePatterns); + if (JSON.stringify(before) !== JSON.stringify(after)) { + reasons.push( + `${after.source ?? before.source}: changes which directories are workspace packages, which ` + + "can re-point a package the schema code imports", + ); + } + reasons.push( + ...findSchemaChanges(changes, { + closure, + baseHasCheck: treeHas(base, CHECK_SCRIPT), + staleCommits: stale.length, + }), + ); + const outside = [...new Set(traces.flatMap((trace) => [...trace.outside]))].sort(); + console.log( + `Schema code: ${closure.size} guarded files. Imports from outside this repository, not ` + + `guarded: ${outside.join(", ") || "none"}.`, + ); if (reasons.length === 0) { console.log(`No database schema changes (${changes.length} changed files checked).`); return 0; @@ -427,7 +674,14 @@ function main(argv) { return 1; } -module.exports = { findSchemaChanges, parseRawDiff, schemaClosure, CHECK_FILES, schemaStatement }; +module.exports = { + findSchemaChanges, + parseRawDiff, + schemaClosure, + traceSchema, + CHECK_FILES, + schemaStatement, +}; if (require.main === module) { process.exitCode = main(process.argv.slice(2)); diff --git a/.github/scripts/fork-schema-check.test.cjs b/.github/scripts/fork-schema-check.test.cjs index 0b13a4fb1d716..f4d9740462d85 100644 --- a/.github/scripts/fork-schema-check.test.cjs +++ b/.github/scripts/fork-schema-check.test.cjs @@ -9,6 +9,7 @@ const { findSchemaChanges, parseRawDiff, schemaClosure, + traceSchema, } = require("./fork-schema-check.cjs"); const SCRIPT = path.join(__dirname, "fork-schema-check.cjs"); @@ -162,12 +163,15 @@ test("refuses changes to the check itself and to .gitattributes once the base ha assert.equal(findSchemaChanges(changes, { closure: new Set(), baseHasCheck: false }).length, 2); }); -test("the closure follows relative and package subpath imports, not root barrels or tests", () => { +test("the closure follows relative and package subpath imports, not root barrels or test-only files", () => { const closure = schemaClosure( treeOf({ - "apps/server/src/persistence/Migrations.ts": 'import m from "./Migrations/001_A.ts";', [`${MIGRATIONS}/001_A.ts`]: `import { t } from "../../orchestration-v2/messageSearchIndex.ts";`, [`${MIGRATIONS}/001_A.test.ts`]: 'import x from "../../feature/OnlyTests.ts";', + // A test-path file that shipped schema code imports is guarded like any other. + "apps/server/src/persistence/Migrations.ts": + 'import m from "./Migrations/001_A.ts";\nimport "./testkit/seed.ts";', + "apps/server/src/persistence/testkit/seed.ts": "", [SEARCH_INDEX]: [ 'import { a } from "@t3tools/shared/assistantCitations";', 'import { Thread } from "@t3tools/contracts";', @@ -193,8 +197,12 @@ test("the closure follows relative and package subpath imports, not root barrels "apps/server/src/orchestration-v2/types.ts", "apps/server/src/persistence/Migrations.ts", `${MIGRATIONS}/001_A.ts`, + "apps/server/src/persistence/testkit/seed.ts", + // A package's manifest maps the import to files, so it is guarded too. + "packages/contracts/package.json", // `Thread` is declared nowhere, so the barrel is guarded whole (fail closed). "packages/contracts/src/index.ts", + "packages/shared/package.json", "packages/shared/src/assistantCitations.ts", "packages/shared/src/threadLinks.ts", ]); @@ -231,6 +239,7 @@ test("the closure follows a package root only to the files that declare the impo ); assert.deepEqual([...closure].sort(), [ `${MIGRATIONS}/064_Links.ts`, + "packages/contracts/package.json", "packages/contracts/src/aliases.ts", "packages/contracts/src/baseSchemas.ts", // The barrel on the way is guarded (an edit to it could re-point a name), not followed: @@ -264,22 +273,181 @@ test("the closure traces the declaration shapes a name can have", () => { for (const [reexport, declaring] of shapes) { assert.deepEqual( [...schemaClosure(rootImportTree(reexport, declaring))].sort(), - [`${MIGRATIONS}/001_A.ts`, "packages/c/src/a.ts", "packages/c/src/index.ts"], + [ + `${MIGRATIONS}/001_A.ts`, + "packages/c/package.json", + "packages/c/src/a.ts", + "packages/c/src/index.ts", + ], declaring, ); } }); test("a name the closure cannot trace guards the whole barrel and what it re-exports", () => { - const closure = schemaClosure(rootImportTree('export * from "./a.ts";', "export const NotS = 1;")); + const closure = schemaClosure( + rootImportTree('export * from "./a.ts";', "export const NotS = 1;"), + ); assert.deepEqual([...closure].sort(), [ `${MIGRATIONS}/001_A.ts`, + "packages/c/package.json", "packages/c/src/a.ts", "packages/c/src/index.ts", "packages/c/src/other.ts", ]); }); +test("a destructuring key is not a binding: `{ S: renamed }` declares renamed, not S", () => { + const traced = (declaring) => + !schemaClosure(rootImportTree('export * from "./a.ts";', declaring)).has( + "packages/c/src/other.ts", + ); + assert.equal(traced("const o = { S: 1 };\nexport const { S: renamed } = o;"), false); + assert.equal(traced("export const { a = { S: 1 } } = o;"), false); + for (const shape of [ + "export const { a: S } = o;", + "export const { a: { S } } = o;", + "export const { S = 1 } = o;", + "export const { a, ...S } = o;", + "export const [a, [S]] = list;", + ]) { + assert.equal(traced(shape), true, shape); + } +}); + +// A migration importing `S` from the workspace package `@t3tools/c`, described by `manifest`. +const packageTree = (manifest, extra = {}) => + treeOf({ + [`${MIGRATIONS}/001_A.ts`]: + 'import { S } from "@t3tools/c";\nimport { U } from "@t3tools/c/util";', + "packages/c/package.json": JSON.stringify({ name: "@t3tools/c", ...manifest }), + "packages/c/src/index.ts": "export const S = 1;", + "packages/c/src/node.ts": "export const S = 2;", + "packages/c/src/util.ts": "export const U = 1;", + ...extra, + }); + +test("a package import guards every file any export condition can pick, and its package.json", () => { + const trace = traceSchema( + packageTree({ + exports: { + ".": { node: "./src/node.ts", import: ["./src/index.ts", "./dist/index.js"] }, + "./*": { worker: { default: "./src/*.ts" } }, + }, + }), + ); + assert.deepEqual(trace.unresolved, []); + assert.deepEqual([...trace.closure].sort(), [ + `${MIGRATIONS}/001_A.ts`, + "packages/c/package.json", + "packages/c/src/index.ts", + "packages/c/src/node.ts", + "packages/c/src/util.ts", + ]); +}); + +test("a package without exports resolves through main, module, types and index", () => { + const closure = schemaClosure( + packageTree({ main: "./src/node.ts", module: "./src/index.ts" }, { "packages/c/util.ts": "" }), + ); + assert.deepEqual([...closure].sort(), [ + `${MIGRATIONS}/001_A.ts`, + "packages/c/package.json", + "packages/c/src/index.ts", + "packages/c/src/node.ts", + // Without exports, a subpath is a path inside the package. + "packages/c/util.ts", + ]); +}); + +test("an in-repository import that resolves to no file is reported, not skipped", () => { + const cases = [ + [ + { exports: { ".": { import: "./src/missing.ts" } } }, + /"@t3tools\/c", but @t3tools\/c is a package of this repository/, + ], + [{ exports: { "./other": "./src/util.ts" } }, /"@t3tools\/c", but @t3tools\/c is a package/], + [{}, /"@t3tools\/c\/util", but @t3tools\/c is a package/], + ]; + for (const [manifest, pattern] of cases) { + const trace = traceSchema(packageTree(manifest)); + assert.ok( + trace.unresolved.some((reason) => pattern.test(reason)), + trace.unresolved.join("\n"), + ); + for (const reason of trace.unresolved) + assert.match(reason, /^apps\/server\/.*001_A\.ts: imports/); + } + const relative = traceSchema( + treeOf({ [`${MIGRATIONS}/001_A.ts`]: 'import x from "./gone.ts";' }), + ); + assert.match( + relative.unresolved.join(""), + /imports "\.\/gone\.ts", but no file in the repository/, + ); + const shapes = traceSchema( + treeOf({ + [`${MIGRATIONS}/001_A.ts`]: [ + 'import { a } from "#internal/a";', + "const m = await import(`./${name}.ts`);", + "const n = require(name);", + 'const all = import.meta.glob("./*.ts");', + ].join("\n"), + }), + ).unresolved.join("\n"); + assert.match(shapes, /imports "#internal\/a", but it is neither a relative import nor a package/); + assert.match(shapes, /imports "import\(\.\.\.", but its target is computed/); + assert.match(shapes, /imports "require\(\.\.\.", but its target is computed/); + assert.match(shapes, /imports "import\.meta\.glob\.\.\.", but its target is computed/); + // A manifest that is not JSON hides its name, so no import is trusted to be from outside. + const broken = traceSchema( + treeOf({ + [`${MIGRATIONS}/001_A.ts`]: 'import { S } from "@t3tools/d";', + "packages/d/package.json": "{ not json", + }), + ); + assert.match(broken.unresolved.join(""), /packages\/d\/package\.json is not JSON/); +}); + +test("workspace packages come from the workspace list, scoped or not, at any depth", () => { + const trace = traceSchema( + treeOf({ + "pnpm-workspace.yaml": + 'packages:\n - "packages/*"\n - apps/**\n - "!apps/skip"\n\ncatalog:\n x: 1\n', + [`${MIGRATIONS}/001_A.ts`]: [ + 'import { a } from "plain-pkg/schema";', + 'import { b } from "@t3tools/deep";', + 'import { c } from "@t3tools/skipped";', + 'import { d } from "effect/Schema";', + 'import * as fs from "node:fs";', + ].join("\n"), + "packages/plain/package.json": JSON.stringify({ + name: "plain-pkg", + exports: { "./schema": { import: "./src/schema.ts" } }, + }), + "packages/plain/src/schema.ts": "export const a = 1;", + "apps/mobile/modules/deep/package.json": JSON.stringify({ + name: "@t3tools/deep", + main: "index.ts", + }), + "apps/mobile/modules/deep/index.ts": "export const b = 1;", + // Not a workspace member, so not what the import resolves to. + "apps/skip/package.json": JSON.stringify({ name: "@t3tools/skipped", main: "index.ts" }), + "apps/skip/index.ts": "", + "vendor/effect/package.json": JSON.stringify({ name: "effect", main: "index.ts" }), + }), + ); + assert.deepEqual(trace.unresolved, []); + assert.deepEqual([...trace.closure].sort(), [ + "apps/mobile/modules/deep/index.ts", + "apps/mobile/modules/deep/package.json", + `${MIGRATIONS}/001_A.ts`, + "packages/plain/package.json", + "packages/plain/src/schema.ts", + ]); + assert.deepEqual([...trace.outside].sort(), ["@t3tools/skipped", "effect", "node:fs"]); +}); + test("the workflow pins actions by commit SHA", () => { const uses = fs.readFileSync(WORKFLOW, "utf8").match(/^\s*(?:-\s*)?uses:\s*\S+/gm) ?? []; assert.ok(uses.length > 0); @@ -462,7 +630,8 @@ test("e2e: renaming or deleting a migration is refused", () => { test("e2e: a barrel-only edit that re-points a name a migration imports is refused", () => { const files = { ...BASE_TREE, - [`${MIGRATIONS}/064_Links.ts`]: 'import { ThreadLinkedPullRequest } from "@t3tools/contracts";\n', + [`${MIGRATIONS}/064_Links.ts`]: + 'import { ThreadLinkedPullRequest } from "@t3tools/contracts";\n', "packages/contracts/package.json": JSON.stringify({ name: "@t3tools/contracts", exports: { ".": "./src/index.ts" }, @@ -493,12 +662,15 @@ test("e2e: an edit to a declaring file the closure cannot trace is refused (fail "packages/c/package.json": JSON.stringify({ name: "@t3tools/c", exports: "./src/index.ts" }), "packages/c/src/index.ts": 'export * from "./a.ts";\n', // A shape the tracer does not know: `S` is declared, but not in a way it recognises. - "packages/c/src/a.ts": "const o = { S: 1 };\nexport default o;\nexport const { S: renamed } = o;\nexports.S = 1;\n", + "packages/c/src/a.ts": + "const o = { S: 1 };\nexport default o;\nexport const { S: renamed } = o;\nexports.S = 1;\n", }; const repo = scratchRepo(); try { const base = repo.commit(files); - const head = repo.commit({ "packages/c/src/a.ts": `${files["packages/c/src/a.ts"]}// edited\n` }); + const head = repo.commit({ + "packages/c/src/a.ts": `${files["packages/c/src/a.ts"]}// edited\n`, + }); const result = repo.check(base, head); assert.equal(result.code, 1, result.output); assert.match(result.output, /packages\/c\/src\/a\.ts: changes the database schema/); @@ -607,3 +779,97 @@ test("e2e: a branch behind main that only cherry-picks a main commit is told to fs.rmSync(repo.dir, { recursive: true, force: true }); } }); + +// The base of the package.json tests: migration 001 imports `S` from `@t3tools/c`, whose barrel +// re-exports it from a.ts; settings.ts, also in the package, exports an `S` of its own. +const PACKAGE_BASE = { + ...BASE_TREE, + "pnpm-workspace.yaml": "packages:\n - packages/*\n", + [`${MIGRATIONS}/001_A.ts`]: 'import { S } from "@t3tools/c";\n', + "packages/c/package.json": JSON.stringify({ + name: "@t3tools/c", + exports: { ".": { types: "./src/index.ts", import: "./src/index.ts" } }, + }), + "packages/c/src/index.ts": 'export * from "./a.ts";\n', + "packages/c/src/a.ts": "export const S = 1;\n", + "packages/c/src/settings.ts": "export const S = 2;\n", +}; +const manifest = (fields) => JSON.stringify({ name: "@t3tools/c", ...fields }); + +test("e2e: a package.json edit that re-points a name a migration imports is refused", () => { + const tricks = { + "the import target swapped to another file exporting the name": { + "packages/c/package.json": manifest({ exports: { ".": { import: "./src/settings.ts" } } }), + }, + "a node condition that Node picks before import": { + "packages/c/package.json": manifest({ + exports: { ".": { node: "./src/evil.ts", import: "./src/index.ts" } }, + }), + "packages/c/src/evil.ts": "export const S = 3;\n", + }, + "exports removed in favour of main": { + "packages/c/package.json": manifest({ main: "./src/evil.ts" }), + "packages/c/src/evil.ts": "export const S = 3;\n", + }, + }; + const passed = Object.entries(tricks).filter(([, files]) => { + const result = prAgainstBase(files, PACKAGE_BASE); + return !( + result.code === 1 && + /packages\/c\/package\.json: changes the database schema/.test(result.output) + ); + }); + assert.deepEqual( + passed.map(([trick]) => trick), + [], + ); + // The rest of the package stays free. + const free = prAgainstBase( + { "packages/c/src/settings.ts": "export const S = 4;\n" }, + PACKAGE_BASE, + ); + assert.equal(free.code, 0, free.output); +}); + +test("e2e: an import of a workspace package the check cannot resolve is refused, naming it", () => { + const broken = { + ...PACKAGE_BASE, + "packages/c/package.json": manifest({ exports: { ".": { import: "./src/missing.ts" } } }), + }; + const result = prAgainstBase( + { "apps/server/src/feature/Store.ts": "export const x = 2;\n" }, + broken, + ); + assert.equal(result.code, 1, result.output); + assert.match( + result.output, + /001_A\.ts: imports "@t3tools\/c", but @t3tools\/c is a package of this repository \(packages\/c\/package\.json\)/, + ); + // A PR that removes the file the entry points to is refused the same way. + const removed = prAgainstBase({ "packages/c/src/index.ts": null }, PACKAGE_BASE); + assert.equal(removed.code, 1, removed.output); + assert.match(removed.output, /imports "@t3tools\/c", but/); +}); + +test("e2e: the output names the imports from outside the repository that are not guarded", () => { + const result = prAgainstBase({ "apps/server/src/feature/Store.ts": "export const x = 2;\n" }); + assert.equal(result.code, 0, result.output); + assert.match(result.output, /Imports from outside this repository, not guarded: effect\./); +}); + +test("e2e: a change to the workspace package list is refused", () => { + const result = prAgainstBase( + { "pnpm-workspace.yaml": "packages:\n - packages/*\n - vendor/*\n" }, + PACKAGE_BASE, + ); + assert.equal(result.code, 1, result.output); + assert.match( + result.output, + /pnpm-workspace\.yaml: changes which directories are workspace packages/, + ); + const catalog = prAgainstBase( + { "pnpm-workspace.yaml": "packages:\n - packages/*\ncatalog:\n x: 1\n" }, + PACKAGE_BASE, + ); + assert.equal(catalog.code, 0, catalog.output); +}); From 5c2688eabd653063b68a7eb0749a4b419376161f Mon Sep 17 00:00:00 2001 From: t3code-fork Date: Thu, 12 Nov 2026 12:23:03 +0000 Subject: [PATCH 6/6] fork: schema check guards the importer's dependency specs and judges only new unfollowable imports - The dependency specs of every name schema code imports (workspace packages and outside names such as effect) are compared in the package.json files of the packages holding schema code and the root one, plus pnpm-workspace.yaml catalog/catalogs/overrides/patchedDependencies and the root pnpm.overrides/patchedDependencies/resolutions. Patch files of those names are guarded. Other dependency changes stay free. - A package.json is judged by the fields that map the imports schema code makes through it (name, type, main/module/types/browser, imports, the export entries answering those keys), so adding a module to a package stays free. - An import the check cannot follow is refused only when the branch brings it; ones already on the merge-base are listed, not refused. Imports are read from code with comments and string contents blanked, so prose like "require (in order)" is not an import. - Tests: the browser fallback, the four importer-side probes and more, manifest field judging, the lexer and the merge-base rule. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/fork-schema-check.cjs | 502 ++++++++++++++++++--- .github/scripts/fork-schema-check.test.cjs | 274 ++++++++++- 2 files changed, 690 insertions(+), 86 deletions(-) diff --git a/.github/scripts/fork-schema-check.cjs b/.github/scripts/fork-schema-check.cjs index 6a230331d75e0..3f98dbcf40a34 100644 --- a/.github/scripts/fork-schema-check.cjs +++ b/.github/scripts/fork-schema-check.cjs @@ -1,11 +1,12 @@ // Refuses fork changes to the database schema: T3 has no down-migrations, so a fork migration // strands installs that return to upstream. Usage: fork-schema-check.cjs . -// Refused: migrations, schema setup and what they import (with the package.json files that map -// those imports, and the workspace package list); an import of theirs that should resolve inside -// the repository but does not; shipped source files holding a schema statement; a PR copy of this -// check or of .gitattributes that differs from the base's copy; and a branch holding commits -// already on the base (built on an old, since rebased base, or copying its commits). Files are -// read as git blobs. +// Refused: migrations, schema setup and what they import; a change to how those imports map to +// files (the package.json fields that map them, the dependency specs, catalogs and overrides of +// the imported names, the workspace package list); a new import of theirs that should resolve +// inside the repository but does not; shipped source files holding a schema statement; a PR copy +// of this check or of .gitattributes that differs from the base's copy; and a branch holding +// commits already on the base (built on an old, since rebased base, or copying its commits). +// Files are read as git blobs. const { execFileSync } = require("node:child_process"); const path = require("node:path"); @@ -52,30 +53,47 @@ const SOURCE_EXTENSIONS = ["", ".ts", ".tsx", ".mts", ".js", "/index.ts", "/inde const SCRIPT_SOURCE = /\.(ts|tsx|mts|cts|js|jsx|mjs|cjs)$/; const MANIFEST = /(^|\/)package\.json$/; const WORKSPACE_FILE = "pnpm-workspace.yaml"; -const IMPORT_SPECIFIER = /\b(?:from|import|require)\s*\(?\s*["']([^"'\n]+)["']/g; +// These run on code whose comments are removed and whose string contents are blanked (see lex), so +// prose such as `require (in order)` in a comment or a string is not taken for an import; the +// specifier itself is then read back from the same offsets of the text with its strings intact. +const IMPORT_SPECIFIER = /\b(?:from|import|require)\s*\(?\s*["']([^"'\n]+)["']/dg; // An import whose target is computed (`import(name)`, a template, `import.meta.glob`): not followed. const COMPUTED_IMPORT = /\b(?:import|require)\s*\(\s*(?!["'\s])|\bimport\.meta\.glob\b/g; // `import { A, type B as C } from "x"` and `export { A } from "x"`: the names a static clause takes. const IMPORT_CLAUSE = - /\b(?:import|export)\s+(?:type\s+)?([\w$*{}\s,]*?)\s*from\s*["']([^"'\n]+)["']/g; + /\b(?:import|export)\s+(?:type\s+)?([\w$*{}\s,]*?)\s*from\s*["']([^"'\n]+)["']/dg; +const REEXPORT = + /\bexport\s+(?:type\s+)?(\*(?:\s+as\s+([\w$]+))?|\{[^}]*\})\s*from\s*["']([^"'\n]+)["']/dg; +const DEPENDENCY_FIELDS = [ + "dependencies", + "devDependencies", + "peerDependencies", + "optionalDependencies", +]; const isTest = (file) => TEST_SOURCE.test(file); const isShippedSource = (file) => SHIPPED_SOURCE.test(file) && !isTest(file); /** * Files the schema roots import, transitively: relative imports, and for an import of a workspace - * package its package.json (which maps the import to files) and every file any export condition - * of that entry can pick. For a package root (`@t3tools/contracts`) only the files that declare - * the imported names are followed. The barrels on the way to a declaring file are guarded too (a - * barrel edit could re-point a name), but their other re-exports are not followed, so the rest of - * the package stays free to change. A name that cannot be traced guards the whole barrel and - * everything it re-exports. + * package every file any export condition of that entry can pick. For a package root + * (`@t3tools/contracts`) only the files that declare the imported names are followed. The barrels + * on the way to a declaring file are guarded too (a barrel edit could re-point a name), but their + * other re-exports are not followed, so the rest of the package stays free to change. A name that + * cannot be traced guards the whole barrel and everything it re-exports. + * + * Besides the files: `mappings` names, per package.json, the entries (`.`, `./sub`) the schema + * code imports through it (only those fields are compared, so a new export elsewhere stays free); + * `imported` holds every package name the schema code imports, from the workspace or outside, and + * `holders` the package.json files of the packages holding schema code, plus the root one: their + * dependency specs for those names decide what the names resolve to. * * Nothing that could be schema code is skipped silently: an import inside the repository that - * resolves to no file is listed in `unresolved` (the check refuses it), and only packages outside - * the workspace resolve to nothing; their names are listed in `outside`. + * resolves to no file is listed in `unresolved` (the check refuses it when the PR brings it), and + * only packages outside the workspace resolve to nothing; their names are listed in `outside`. * @param {{ files: string[]; has(file: string): boolean; read(file: string): string }} tree - * @returns {{ closure: Set; unresolved: string[]; outside: Set }} + * @returns {{ closure: Set; unresolved: string[]; outside: Set; + * imported: Set; mappings: Map>; holders: Set }} */ function traceSchema(tree) { const queue = tree.files.filter( @@ -83,11 +101,14 @@ function traceSchema(tree) { ); const closure = new Set(queue); const unresolved = new Map(); + const mappings = new Map(); + const workspace = workspacePackages(tree); const ctx = { tree, - ...workspacePackages(tree), + ...workspace, outside: new Set(), - guard: (file) => closure.add(file), + imported: new Set(), + map: (file, key) => mappings.set(file, (mappings.get(file) ?? new Set()).add(key)), unresolved: (from, specifier, why) => unresolved.set(`${from}\0${specifier}`, `${from}: imports "${specifier}", but ${why}.`), }; @@ -95,17 +116,24 @@ function traceSchema(tree) { while (queue.length > 0) { const file = queue.pop(); if (!SCRIPT_SOURCE.test(file)) continue; - const text = tree.read(file); - const names = importedNames(text); - for (const match of text.matchAll(COMPUTED_IMPORT)) { + const source = lexed(tree.read(file)); + if (source.open) { + unresolved.set( + `${file}\0`, + `${file}: a comment, string or template in it does not end, so its imports cannot be read.`, + ); + } + const names = importedNames(source); + for (const match of source.code.matchAll(COMPUTED_IMPORT)) { ctx.unresolved( file, `${match[0].trim()}...`, "its target is computed, so it cannot be followed", ); } - for (const match of text.matchAll(IMPORT_SPECIFIER)) { - const reached = resolveImport(ctx, file, match[1], names.get(match[1])); + for (const match of source.code.matchAll(IMPORT_SPECIFIER)) { + const specifier = source.group(match, 1); + const reached = resolveImport(ctx, file, specifier, names.get(specifier)); // A test-path file that schema code imports ships with it, so it is guarded too. for (const target of reached.barrels) closure.add(target); for (const target of reached.files) { @@ -120,12 +148,140 @@ function traceSchema(tree) { const why = "the check cannot resolve it to a file in this repository, so it cannot tell which files " + "the schema depends on. Fix the import (or teach the check this import shape)"; + // The package holding a file is its deepest workspace member directory. + const holders = new Set(tree.has("package.json") ? ["package.json"] : []); + for (const file of closure) { + const holder = workspace.members + .filter((member) => member.dir !== "" && file.startsWith(`${member.dir}/`)) + .sort((a, b) => b.dir.length - a.dir.length)[0]; + if (holder) holders.add(holder.file); + } return { closure, unresolved: [...unresolved.values()].map((reason) => reason.replace(/\.$/, `: ${why}.`)), outside: ctx.outside, + imported: ctx.imported, + mappings, + holders, + }; +} + +/** + * The text with comments and the contents of strings, templates and regular expressions blanked + * (`code`), at the same offsets as the text. `open` says the text ends inside a block comment or a + * template. `group(match, n)` reads a match's group `n` (found in `code`, `d` flag) from the text, + * where strings are intact. + */ +function lex(text) { + const code = text.split(""); + const blank = (array, from, to) => { + for (let k = from; k < Math.min(to, text.length); k += 1) { + if (array[k] !== "\n") array[k] = " "; + } + }; + const templates = []; // the brace depth at each open `${` + let depth = 0; + let prev = ""; // the last code token: a punctuation character or a word + let open = false; + let i = 0; + // From just after a backtick, or the `}` that closes an interpolation, to the next `${` or `. + const template = () => { + const start = i; + while (i < text.length) { + if (text[i] === "\\") i += 2; + else if (text[i] === "`") { + blank(code, start, i); + i += 1; + prev = ")"; + return; + } else if (text[i] === "$" && text[i + 1] === "{") { + blank(code, start, i); + i += 2; + templates.push(depth); + depth += 1; + prev = "{"; + return; + } else i += 1; + } + blank(code, start, text.length); + open = true; + }; + while (i < text.length) { + const c = text[i]; + if (c === "/" && text[i + 1] === "/") { + const end = text.indexOf("\n", i); + const stop = end < 0 ? text.length : end; + blank(code, i, stop); + i = stop; + } else if (c === "/" && text[i + 1] === "*") { + const end = text.indexOf("*/", i + 2); + if (end < 0) open = true; + const stop = end < 0 ? text.length : end + 2; + blank(code, i, stop); + i = stop; + } else if (c === "'" || c === '"' || (c === "/" && REGEX_AFTER.has(prev))) { + // A string or a regular expression literal: both end at the line's end at the latest. + let j = i + 1; + let inClass = false; + while (j < text.length && text[j] !== "\n") { + if (text[j] === "\\") j += 1; + else if (c === "/" && text[j] === "[") inClass = true; + else if (c === "/" && text[j] === "]") inClass = false; + else if (text[j] === c && !inClass) break; + j += 1; + } + blank(code, i + 1, j); + i = j + 1; + prev = ")"; + } else if (c === "`") { + i += 1; + template(); + } else if (c === "}" && templates.at(-1) === depth - 1) { + templates.pop(); + depth -= 1; + i += 1; + template(); + } else if (/[\w$]/.test(c)) { + const word = text.slice(i).match(/^[\w$]+/)[0]; + prev = word; + i += word.length; + } else { + if (c === "{") depth += 1; + if (c === "}") depth -= 1; + if (!/\s/.test(c)) prev = c; + i += 1; + } + } + if (templates.length > 0) open = true; + return { + code: code.join(""), + open, + group: (match, n) => match.indices[n] && text.slice(...match.indices[n]), }; } +// After these tokens a `/` starts a regular expression, not a division. +const REGEX_AFTER = new Set([ + ..."(,=:[!&|?{};+-*%<>~^", + "", + "return", + "typeof", + "case", + "in", + "of", + "new", + "delete", + "void", + "throw", + "yield", + "await", + "else", + "do", +]); +const lexCache = new Map(); +function lexed(text) { + if (!lexCache.has(text)) lexCache.set(text, lex(text)); + return lexCache.get(text); +} /** The files the schema roots import, transitively (see traceSchema). */ const schemaClosure = (tree) => traceSchema(tree).closure; @@ -198,10 +354,12 @@ function workspacePackages(tree) { : !/(^|\/)node_modules(\/|$)/.test(dir); const packages = new Map(); const broken = []; + const members = []; for (const file of tree.files) { if (!MANIFEST.test(file)) continue; const dir = file === "package.json" ? "" : path.posix.dirname(file); if (!member(dir)) continue; + members.push({ dir, file }); let manifest; try { manifest = JSON.parse(tree.read(file)); @@ -214,17 +372,20 @@ function workspacePackages(tree) { list.push({ dir, file, manifest }); packages.set(manifest.name, list); } - return { packages, broken }; + return { packages, broken, members }; } /** * Per specifier, the names static clauses take from it; `*` for a namespace or a clause it cannot * read. A specifier with no entry (dynamic import, require, side-effect import) takes everything. + * @param {ReturnType} source * @returns {Map>} */ -function importedNames(text) { +function importedNames(source) { const names = new Map(); - for (const [, clause, specifier] of text.matchAll(IMPORT_CLAUSE)) { + for (const match of source.code.matchAll(IMPORT_CLAUSE)) { + const clause = match[1]; + const specifier = source.group(match, 2); const set = names.get(specifier) ?? new Set(); names.set(specifier, set); const braces = clause.match(/\{([^}]*)\}/); @@ -275,6 +436,7 @@ function resolveImport(ctx, from, specifier, names) { return none; } const [, name, subpath] = parts; + ctx.imported.add(name); const candidates = ctx.packages.get(name); if (!candidates) { if (ctx.broken.length > 0) { @@ -290,7 +452,7 @@ function resolveImport(ctx, from, specifier, names) { } const entries = []; for (const pkg of candidates) { - ctx.guard(pkg.file); + ctx.map(pkg.file, subpath ? `./${subpath}` : "."); for (const target of packageTargets(pkg.manifest, subpath)) { const file = resolveFile(tree, path.posix.join(pkg.dir, target)); if (file && !entries.includes(file)) entries.push(file); @@ -341,12 +503,6 @@ function packageTargets(manifest, subpath) { "./index", ].filter((target) => typeof target === "string"); } - const map = - typeof exports === "object" && - !Array.isArray(exports) && - Object.keys(exports).some((each) => each.startsWith(".")) - ? exports - : { ".": exports }; const targets = []; const leaves = (value, star) => { if (typeof value === "string") @@ -355,18 +511,174 @@ function packageTargets(manifest, subpath) { else if (value && typeof value === "object") for (const each of Object.values(value)) leaves(each, star); }; - if (Object.hasOwn(map, key)) leaves(map[key]); - else { - for (const [pattern, value] of Object.entries(map)) { - const star = pattern.indexOf("*"); - if (star < 0) continue; - const [prefix, suffix] = [pattern.slice(0, star), pattern.slice(star + 1)]; - if (key.length >= pattern.length - 1 && key.startsWith(prefix) && key.endsWith(suffix)) { - leaves(value, key.slice(prefix.length, key.length - suffix.length)); + for (const [, value, star] of exportEntries(exports, key)) leaves(value, star); + return targets; +} + +/** + * The `exports` entries that can answer `key`: the exact key, or else every `*` pattern that + * matches it, as [pattern, value, what the star stands for]. + */ +function exportEntries(exports, key) { + const map = + typeof exports === "object" && + !Array.isArray(exports) && + Object.keys(exports).some((each) => each.startsWith(".")) + ? exports + : { ".": exports }; + if (Object.hasOwn(map, key)) return [[key, map[key], undefined]]; + const entries = []; + for (const [pattern, value] of Object.entries(map)) { + const star = pattern.indexOf("*"); + if (star < 0) continue; + const [prefix, suffix] = [pattern.slice(0, star), pattern.slice(star + 1)]; + if (key.length >= pattern.length - 1 && key.startsWith(prefix) && key.endsWith(suffix)) { + entries.push([pattern, value, key.slice(prefix.length, key.length - suffix.length)]); + } + } + return entries; +} + +/** + * What a package.json says about the import `key` (`.` or `./sub`): its name, type, the export + * entries that answer that key (and whether `exports` is a subpath map), the fallback fields and + * `imports`. Two trees whose values differ map the import differently; other export entries do not + * count, so adding a module to the package stays free. A file that is absent or not JSON gives + * a value of its own. + */ +function manifestMapping(text, key) { + if (text === undefined) return "(absent)"; + let manifest; + try { + manifest = JSON.parse(text); + } catch { + return `(not JSON) ${text}`; + } + if (!manifest || typeof manifest !== "object") return `(not an object) ${text}`; + const { exports } = manifest; + const fields = ["name", "type", "main", "module", "types", "typings", "browser", "imports"]; + return JSON.stringify({ + ...Object.fromEntries(fields.map((field) => [field, manifest[field]])), + exports: + exports === undefined || exports === null + ? exports + : { + map: + typeof exports === "object" && + !Array.isArray(exports) && + Object.keys(exports).some((each) => each.startsWith(".")), + entries: exportEntries(exports, key).map(([pattern, value]) => [pattern, value]), + }, + }); +} + +/** The package name an override, resolution, catalog or patch key is about (`a>b@^1` -> `b`). */ +function keyNames(key, names) { + const last = key.split(">").pop().trim(); + const bare = last.replace(/(.)@[^/]*$/, "$1"); + return [...names].filter((name) => bare === name || bare.endsWith(`/${name}`)); +} + +/** + * Top-level YAML map entries as { path, value } (`catalog:` / ` effect: 4.1.2` gives + * { path: ["catalog", "effect"], value: "4.1.2" }); enough for pnpm-workspace.yaml, whose + * catalogs and overrides are plain maps. List items and other lines are skipped. + */ +function yamlEntries(text) { + const entries = []; + const stack = []; + for (const line of text.split("\n")) { + if (/^\s*(#|-|$)/.test(line)) continue; + const match = line.match( + /^(\s*)(?:"([^"]*)"|'([^']*)'|([^\s#'"][^:#]*?))\s*:(?:\s+(.*?))?\s*$/, + ); + if (!match) continue; + const indent = match[1].length; + const key = match[2] ?? match[3] ?? match[4]; + const value = (match[5] ?? "").replace(/\s+#.*$/, "").replace(/^(["'])(.*)\1$/, "$2"); + while (stack.length > 0 && stack.at(-1).indent >= indent) stack.pop(); + entries.push({ path: [...stack.map((each) => each.key), key], value }); + stack.push({ indent, key }); + } + return entries; +} + +/** + * The dependency specs that decide what the imported `names` resolve to: their entries in the + * dependency fields of the `holders` (the package.json files of the packages holding schema code, + * and the root one), the root's `pnpm.overrides`, `pnpm.patchedDependencies` and `resolutions`, + * and pnpm-workspace.yaml's `catalog`, `catalogs`, `overrides` and `patchedDependencies`. Specs of + * other names are not read, so other dependency bumps stay free. `patches` are the patch files of + * those names, guarded like schema code. + * @returns {{ specs: Map; patches: string[] }} + */ +function dependencySpecs(tree, names, holders) { + const specs = new Map(); + const patches = []; + const add = (file, where, name, value) => { + const text = typeof value === "string" ? value : JSON.stringify(value); + specs.set(`${file}\0${where}\0${name}`, { file, where, name, value: text }); + }; + const keyed = (file, where, map, isPatch) => { + if (map === undefined) return; + if (!map || typeof map !== "object") return add(file, where, "*", map); + for (const [key, value] of Object.entries(map)) { + for (const name of keyNames(key, names)) { + add(file, `${where} ${JSON.stringify(key)}`, name, value); + if (isPatch && typeof value === "string") patches.push(path.posix.normalize(value)); } } + }; + for (const file of holders) { + if (!tree.has(file)) continue; + let manifest; + try { + manifest = JSON.parse(tree.read(file)); + } catch { + add(file, "(not JSON)", "*", tree.read(file)); + continue; + } + for (const field of DEPENDENCY_FIELDS) { + const deps = manifest?.[field]; + if (deps === undefined) continue; + if (!deps || typeof deps !== "object") { + add(file, field, "*", deps); + continue; + } + for (const name of names) { + if (Object.hasOwn(deps, name)) add(file, field, name, deps[name]); + } + } + if (file === "package.json") { + keyed(file, "pnpm.overrides", manifest?.pnpm?.overrides); + keyed(file, "pnpm.patchedDependencies", manifest?.pnpm?.patchedDependencies, true); + keyed(file, "resolutions", manifest?.resolutions); + } } - return targets; + if (tree.has(WORKSPACE_FILE)) { + const sections = ["catalog", "catalogs", "overrides", "patchedDependencies"]; + for (const { path: keys, value } of yamlEntries(tree.read(WORKSPACE_FILE))) { + const [section] = keys; + if (!sections.includes(section)) continue; + // A section written inline (`catalog: { ... }`) is compared whole. + if (keys.length === 1) { + if (value) add(WORKSPACE_FILE, section, "*", value); + continue; + } + if (section === "catalogs" && keys.length === 2) { + if (value) add(WORKSPACE_FILE, `catalogs.${keys[1]}`, "*", value); + continue; + } + const key = section === "catalogs" ? keys[2] : keys[1]; + if (keys.length > (section === "catalogs" ? 3 : 2)) continue; + const where = section === "catalogs" ? `catalogs.${keys[1]}` : section; + for (const name of keyNames(key, names)) { + add(WORKSPACE_FILE, `${where} ${JSON.stringify(key)}`, name, value); + if (section === "patchedDependencies") patches.push(path.posix.normalize(value)); + } + } + } + return { specs, patches }; } /** @@ -378,17 +690,17 @@ function declaringFiles(ctx, file, name, found, barrels, seen) { const key = `${file}\0${name}`; if (seen.has(key)) return seen.get(key); seen.set(key, false); - const text = ctx.tree.read(file); + const source = lexed(ctx.tree.read(file)); + const text = source.code; if (declaresName(text, name)) { found.add(file); seen.set(key, true); return true; } let traced = false; - for (const match of text.matchAll( - /\bexport\s+(?:type\s+)?(\*(?:\s+as\s+([\w$]+))?|\{[^}]*\})\s*from\s*["']([^"'\n]+)["']/g, - )) { - const [, clause, namespace, specifier] = match; + for (const match of text.matchAll(REEXPORT)) { + const [, clause, namespace] = match; + const specifier = source.group(match, 3); const targets = resolveImport(ctx, file, specifier, new Set(["*"])).files; if (clause.startsWith("*") && !namespace) { for (const target of targets) { @@ -618,6 +930,71 @@ function staleCommits(mergeBase, base, head) { .map((commit) => commit.sha); } +/** + * Judges what the branch changes in how schema code is found, between the merge-base tree and the + * head tree: the guarded closure (both trees), imports the head cannot follow that the merge-base + * could (`known`: the ones already on the merge-base, which the branch did not bring), and changes + * to the workspace list, to the package.json fields that map schema imports, and to the dependency + * specs of the names schema code imports. + * @returns {{ closure: Set; reasons: string[]; known: string[]; outside: string[] }} + */ +function compareTrees(before, after) { + const trees = [before, after]; + const traces = trees.map(traceSchema); + const union = (pick) => new Set(traces.flatMap((trace) => [...pick(trace)])); + const closure = union((trace) => trace.closure); + const reasons = []; + // An import the merge-base already could not follow is not the branch's doing: listed, not + // refused. Only the ones the branch brings (a changed or newly guarded file, or a target it + // moved or broke) are refused. + const onBase = new Set(traces[0].unresolved); + const known = traces[1].unresolved.filter((reason) => onBase.has(reason)); + reasons.push(...traces[1].unresolved.filter((reason) => !onBase.has(reason))); + // Which directories are workspace packages decides what a package import resolves to. + const [listBefore, listAfter] = trees.map(workspacePatterns); + if (JSON.stringify(listBefore) !== JSON.stringify(listAfter)) { + reasons.push( + `${listAfter.source ?? listBefore.source}: changes which directories are workspace ` + + "packages, which can re-point a package the schema code imports", + ); + } + // The package.json fields that map a schema import to files. + const mappings = new Map(); + for (const trace of traces) { + for (const [file, keys] of trace.mappings) { + mappings.set(file, new Set([...(mappings.get(file) ?? []), ...keys])); + } + } + for (const [file, keys] of [...mappings].sort()) { + const read = (tree) => (tree.has(file) ? tree.read(file) : undefined); + const changed = [...keys] + .sort() + .filter((key) => manifestMapping(read(before), key) !== manifestMapping(read(after), key)); + if (changed.length > 0) { + reasons.push( + `${file}: changes how it maps ${changed.map((key) => `"${key}"`).join(", ")}, which the ` + + "schema code imports (the fields for other entries stay free)", + ); + } + } + // The dependency specs of the names schema code imports. + const names = union((trace) => trace.imported); + const holders = union((trace) => trace.holders); + const [specsBefore, specsAfter] = trees.map((tree) => dependencySpecs(tree, names, holders)); + for (const key of new Set([...specsBefore.specs.keys(), ...specsAfter.specs.keys()])) { + const [old, now] = [specsBefore.specs.get(key), specsAfter.specs.get(key)]; + if (old?.value === now?.value) continue; + const { file, where, name } = old ?? now; + const show = (spec) => (spec ? JSON.stringify(spec.value) : "(none)"); + reasons.push( + `${file}: changes what "${name}" resolves to (${where}: ${show(old)} -> ${show(now)}), ` + + "which the schema code imports", + ); + } + for (const patch of [...specsBefore.patches, ...specsAfter.patches]) closure.add(patch); + return { closure, reasons, known, outside: [...union((trace) => trace.outside)].sort() }; +} + function main(argv) { const [base, head] = argv; if (!base || !head) { @@ -641,18 +1018,9 @@ function main(argv) { sameAsBase: guarded(entry.path) ? blobAt(base, entry.path) === entry.newBlob : undefined, fromOldMain: guarded(entry.path) && stale.length > 0 ? fromOldMain(entry) : undefined, })); - const trees = [readTree(mergeBase), readTree(head)]; - const traces = trees.map(traceSchema); - const closure = new Set(traces.flatMap((trace) => [...trace.closure])); - const reasons = [...new Set(traces.flatMap((trace) => trace.unresolved))]; - // Which directories are workspace packages decides what a package import resolves to. - const [before, after] = trees.map(workspacePatterns); - if (JSON.stringify(before) !== JSON.stringify(after)) { - reasons.push( - `${after.source ?? before.source}: changes which directories are workspace packages, which ` + - "can re-point a package the schema code imports", - ); - } + const judged = compareTrees(readTree(mergeBase), readTree(head)); + const { closure, outside } = judged; + const reasons = [...judged.reasons]; reasons.push( ...findSchemaChanges(changes, { closure, @@ -660,11 +1028,17 @@ function main(argv) { staleCommits: stale.length, }), ); - const outside = [...new Set(traces.flatMap((trace) => [...trace.outside]))].sort(); console.log( - `Schema code: ${closure.size} guarded files. Imports from outside this repository, not ` + - `guarded: ${outside.join(", ") || "none"}.`, + `Schema code: ${closure.size} guarded files. Imports from outside this repository (their ` + + `dependency specs are guarded, their code is not): ${outside.join(", ") || "none"}.`, ); + if (judged.known.length > 0) { + console.log( + "Imports of the schema code the check cannot follow, already on the merge-base (not this " + + "branch's doing; the check needs teaching, the branch needs no change):", + ); + for (const reason of judged.known) console.log(`- ${reason}`); + } if (reasons.length === 0) { console.log(`No database schema changes (${changes.length} changed files checked).`); return 0; @@ -675,7 +1049,9 @@ function main(argv) { } module.exports = { + compareTrees, findSchemaChanges, + lex, parseRawDiff, schemaClosure, traceSchema, diff --git a/.github/scripts/fork-schema-check.test.cjs b/.github/scripts/fork-schema-check.test.cjs index f4d9740462d85..cd66805db7a7a 100644 --- a/.github/scripts/fork-schema-check.test.cjs +++ b/.github/scripts/fork-schema-check.test.cjs @@ -6,6 +6,7 @@ const path = require("node:path"); const test = require("node:test"); const { CHECK_FILES, + compareTrees, findSchemaChanges, parseRawDiff, schemaClosure, @@ -198,11 +199,8 @@ test("the closure follows relative and package subpath imports, not root barrels "apps/server/src/persistence/Migrations.ts", `${MIGRATIONS}/001_A.ts`, "apps/server/src/persistence/testkit/seed.ts", - // A package's manifest maps the import to files, so it is guarded too. - "packages/contracts/package.json", // `Thread` is declared nowhere, so the barrel is guarded whole (fail closed). "packages/contracts/src/index.ts", - "packages/shared/package.json", "packages/shared/src/assistantCitations.ts", "packages/shared/src/threadLinks.ts", ]); @@ -239,7 +237,6 @@ test("the closure follows a package root only to the files that declare the impo ); assert.deepEqual([...closure].sort(), [ `${MIGRATIONS}/064_Links.ts`, - "packages/contracts/package.json", "packages/contracts/src/aliases.ts", "packages/contracts/src/baseSchemas.ts", // The barrel on the way is guarded (an edit to it could re-point a name), not followed: @@ -273,12 +270,7 @@ test("the closure traces the declaration shapes a name can have", () => { for (const [reexport, declaring] of shapes) { assert.deepEqual( [...schemaClosure(rootImportTree(reexport, declaring))].sort(), - [ - `${MIGRATIONS}/001_A.ts`, - "packages/c/package.json", - "packages/c/src/a.ts", - "packages/c/src/index.ts", - ], + [`${MIGRATIONS}/001_A.ts`, "packages/c/src/a.ts", "packages/c/src/index.ts"], declaring, ); } @@ -290,7 +282,6 @@ test("a name the closure cannot trace guards the whole barrel and what it re-exp ); assert.deepEqual([...closure].sort(), [ `${MIGRATIONS}/001_A.ts`, - "packages/c/package.json", "packages/c/src/a.ts", "packages/c/src/index.ts", "packages/c/src/other.ts", @@ -327,7 +318,7 @@ const packageTree = (manifest, extra = {}) => ...extra, }); -test("a package import guards every file any export condition can pick, and its package.json", () => { +test("a package import guards every file any export condition can pick, and its mapping", () => { const trace = traceSchema( packageTree({ exports: { @@ -339,11 +330,16 @@ test("a package import guards every file any export condition can pick, and its assert.deepEqual(trace.unresolved, []); assert.deepEqual([...trace.closure].sort(), [ `${MIGRATIONS}/001_A.ts`, - "packages/c/package.json", "packages/c/src/index.ts", "packages/c/src/node.ts", "packages/c/src/util.ts", ]); + // The package.json is judged by the entries that map these imports, not as a whole file. + assert.deepEqual( + [...trace.mappings].map(([file, keys]) => [file, [...keys].sort()]), + [["packages/c/package.json", [".", "./util"]]], + ); + assert.deepEqual([...trace.imported], ["@t3tools/c"]); }); test("a package without exports resolves through main, module, types and index", () => { @@ -352,12 +348,13 @@ test("a package without exports resolves through main, module, types and index", ); assert.deepEqual([...closure].sort(), [ `${MIGRATIONS}/001_A.ts`, - "packages/c/package.json", "packages/c/src/index.ts", "packages/c/src/node.ts", // Without exports, a subpath is a path inside the package. "packages/c/util.ts", ]); + // A string `browser` field is a fallback entry as well. + assert.ok(schemaClosure(packageTree({ browser: "./src/node.ts" })).has("packages/c/src/node.ts")); }); test("an in-repository import that resolves to no file is reported, not skipped", () => { @@ -440,9 +437,7 @@ test("workspace packages come from the workspace list, scoped or not, at any dep assert.deepEqual(trace.unresolved, []); assert.deepEqual([...trace.closure].sort(), [ "apps/mobile/modules/deep/index.ts", - "apps/mobile/modules/deep/package.json", `${MIGRATIONS}/001_A.ts`, - "packages/plain/package.json", "packages/plain/src/schema.ts", ]); assert.deepEqual([...trace.outside].sort(), ["@t3tools/skipped", "effect", "node:fs"]); @@ -816,7 +811,7 @@ test("e2e: a package.json edit that re-points a name a migration imports is refu const result = prAgainstBase(files, PACKAGE_BASE); return !( result.code === 1 && - /packages\/c\/package\.json: changes the database schema/.test(result.output) + /packages\/c\/package\.json: changes how it maps "\."/.test(result.output) ); }); assert.deepEqual( @@ -833,13 +828,9 @@ test("e2e: a package.json edit that re-points a name a migration imports is refu test("e2e: an import of a workspace package the check cannot resolve is refused, naming it", () => { const broken = { - ...PACKAGE_BASE, "packages/c/package.json": manifest({ exports: { ".": { import: "./src/missing.ts" } } }), }; - const result = prAgainstBase( - { "apps/server/src/feature/Store.ts": "export const x = 2;\n" }, - broken, - ); + const result = prAgainstBase(broken, PACKAGE_BASE); assert.equal(result.code, 1, result.output); assert.match( result.output, @@ -851,10 +842,78 @@ test("e2e: an import of a workspace package the check cannot resolve is refused, assert.match(removed.output, /imports "@t3tools\/c", but/); }); +test("e2e: an import the merge-base already cannot follow is listed, not blamed on the PR", () => { + // As after an upstream release that brings an import shape the check does not know. + const upstream = { + ...PACKAGE_BASE, + "packages/c/package.json": manifest({ exports: { ".": { import: "./src/missing.ts" } } }), + [`${MIGRATIONS}/002_B.ts`]: "export const load = (name) => import(name);\n", + "apps/server/src/persistence/Migrations.ts": `${BASE_TREE["apps/server/src/persistence/Migrations.ts"]}import b from "./Migrations/002_B.ts";\n`, + }; + const result = prAgainstBase( + { "apps/server/src/feature/Store.ts": "export const x = 2;\n" }, + upstream, + ); + assert.equal(result.code, 0, result.output); + assert.match(result.output, /already on the merge-base \(not this branch's doing/); + assert.match(result.output, /002_B\.ts: imports "import\(\.\.\.", but its target is computed/); + assert.match(result.output, /001_A\.ts: imports "@t3tools\/c", but/); +}); + +test("an unfollowable import is refused when the branch brings it, not when the base has it", () => { + const files = (loader) => ({ + [`${MIGRATIONS}/001_A.ts`]: 'import { load } from "../loader.ts";', + "apps/server/src/persistence/loader.ts": loader, + }); + const plain = "export const load = () => 1;"; + const computed = "export const load = (name) => require(name);"; + const brought = compareTrees(treeOf(files(plain)), treeOf(files(computed))); + assert.match(brought.reasons.join("\n"), /loader\.ts: imports "require\(\.\.\.", but/); + assert.deepEqual(brought.known, []); + const kept = compareTrees(treeOf(files(computed)), treeOf(files(computed))); + assert.deepEqual(kept.reasons, []); + assert.equal(kept.known.length, 1); +}); + +test("imports are read from code: comments and strings do not count, specifiers do", () => { + const trace = traceSchema( + treeOf({ + [`${MIGRATIONS}/001_A.ts`]: [ + "// each step must require (in order) the step before it", + "/* import x from './gone.ts'; import(name) */", + 'const note = "require (anything) or import (this)";', + "const re = /import\\(/;", + "const sql = `SELECT 1 -- require (x) ${'import(y)'}`;", + 'import { b } from "./b.ts"; // import { c } from "./c.ts";', + "const url = 'https://example.invalid/require(';", + ].join("\n"), + [`${MIGRATIONS}/b.ts`]: "export const b = 1;", + }), + ); + assert.deepEqual(trace.unresolved, []); + assert.ok(trace.closure.has(`${MIGRATIONS}/b.ts`)); + // A real computed import, also inside a template's interpolation, is still seen. + const computed = traceSchema( + treeOf({ [`${MIGRATIONS}/001_A.ts`]: "const s = `${require(name)}`;" }), + ); + assert.match(computed.unresolved.join(""), /require\(\.\.\.", but its target is computed/); + // A file whose comment or template never ends cannot be read, so it is reported. + const open = traceSchema( + treeOf({ [`${MIGRATIONS}/001_A.ts`]: 'const s = `never ends\nimport x from "./x.ts";' }), + ); + assert.match( + open.unresolved.join(""), + /001_A\.ts: a comment, string or template in it does not end/, + ); +}); + test("e2e: the output names the imports from outside the repository that are not guarded", () => { const result = prAgainstBase({ "apps/server/src/feature/Store.ts": "export const x = 2;\n" }); assert.equal(result.code, 0, result.output); - assert.match(result.output, /Imports from outside this repository, not guarded: effect\./); + assert.match( + result.output, + /Imports from outside this repository \(their dependency specs are guarded, their code is not\): effect\./, + ); }); test("e2e: a change to the workspace package list is refused", () => { @@ -873,3 +932,172 @@ test("e2e: a change to the workspace package list is refused", () => { ); assert.equal(catalog.code, 0, catalog.output); }); + +// The importer's side: apps/server holds the migrations and depends on `@t3tools/c` and `effect`, +// which the migrations import; `other` is a dependency the schema code does not import. +const SPEC_BASE = { + ...PACKAGE_BASE, + "package.json": JSON.stringify({ name: "root", private: true }), + "pnpm-workspace.yaml": [ + "packages:", + " - apps/*", + " - packages/*", + "catalog:", + " effect: 4.1.2", + " other: 1.0.0", + "catalogs:", + " next:", + " effect: 4.2.0", + "overrides:", + ' "dbus-next>usocket": "-"', + "patchedDependencies:", + " other@1.0.0: patches/other@1.0.0.patch", + " effect@4.1.2: patches/effect@4.1.2.patch", + "", + ].join("\n"), + "patches/other@1.0.0.patch": "--- a\n+++ b\n", + "patches/effect@4.1.2.patch": "--- a\n+++ b\n", + [`${MIGRATIONS}/001_A.ts`]: + 'import { S } from "@t3tools/c";\nimport * as Sql from "effect/sql";\n', + "apps/server/package.json": JSON.stringify({ + name: "server", + dependencies: { effect: "catalog:", other: "catalog:" }, + devDependencies: { "@t3tools/c": "workspace:*" }, + }), +}; +const json = (fields) => JSON.stringify(fields); +const yamlWith = (from, to) => SPEC_BASE["pnpm-workspace.yaml"].replace(from, to); + +test("e2e: re-pointing a name the schema code imports from the importer's side is refused", () => { + const server = JSON.parse(SPEC_BASE["apps/server/package.json"]); + const tricks = { + "link: spec in the importer": { + "apps/server/package.json": json({ + ...server, + devDependencies: { "@t3tools/c": "link:../../x/evil" }, + }), + "x/evil/package.json": json({ name: "@t3tools/c", main: "index.ts" }), + "x/evil/index.ts": "export const S = 3;\n", + }, + "npm: alias in the importer": { + "apps/server/package.json": json({ + ...server, + devDependencies: { "@t3tools/c": "npm:left-pad@1.3.0" }, + }), + }, + "file: spec for an outside name": { + "apps/server/package.json": json({ + ...server, + dependencies: { ...server.dependencies, effect: "file:../../x/effect" }, + }), + }, + "the name added to the root manifest": { + "package.json": json({ name: "root", dependencies: { effect: "link:./x/effect" } }), + }, + "workspace overrides": { + "pnpm-workspace.yaml": yamlWith( + "overrides:\n", + 'overrides:\n "@t3tools/c": "link:./x/evil"\n', + ), + }, + "a nested override of the name": { + "pnpm-workspace.yaml": yamlWith("overrides:\n", "overrides:\n foo>effect@^4: 4.0.0\n"), + }, + catalog: { "pnpm-workspace.yaml": yamlWith(" effect: 4.1.2", " effect: link:./x/effect") }, + "named catalog": { + "pnpm-workspace.yaml": yamlWith(" effect: 4.2.0", " effect: npm:evil@1"), + }, + "a patch of the name re-pointed": { + "pnpm-workspace.yaml": yamlWith( + "effect@4.1.2: patches/effect@4.1.2.patch", + "effect@4.1.2: patches/effect-fork.patch", + ), + "patches/effect-fork.patch": "--- a\n+++ b\n", + }, + "root pnpm.overrides": { + "package.json": json({ name: "root", pnpm: { overrides: { effect: "link:./x" } } }), + }, + "root resolutions": { + "package.json": json({ name: "root", resolutions: { "**/@t3tools/c": "link:./x" } }), + }, + "the importer's package.json made unreadable": { "apps/server/package.json": "{ broken" }, + }; + const passed = Object.entries(tricks).filter(([, files]) => { + const result = prAgainstBase(files, SPEC_BASE); + return !(result.code === 1 && /changes what "[^"]+" resolves to/.test(result.output)); + }); + assert.deepEqual( + passed.map(([trick]) => trick), + [], + ); + // The patch file of a name the schema code imports is guarded like schema code. + const patched = prAgainstBase( + { "patches/effect@4.1.2.patch": "--- a\n+++ b\n@@ fork @@\n" }, + SPEC_BASE, + ); + assert.equal(patched.code, 1, patched.output); + assert.match(patched.output, /patches\/effect@4\.1\.2\.patch: changes the database schema/); +}); + +test("e2e: dependency changes of names the schema code does not import stay free", () => { + const server = JSON.parse(SPEC_BASE["apps/server/package.json"]); + const result = prAgainstBase( + { + "pnpm-workspace.yaml": yamlWith(" other: 1.0.0", " other: 1.1.0\n added: 2.0.0").replace( + "overrides:\n", + "overrides:\n other: 1.1.0\n", + ), + "patches/other@1.0.0.patch": "--- a\n+++ b\n@@ changed @@\n", + "apps/server/package.json": json({ + ...server, + dependencies: { ...server.dependencies, other: "2.0.0", added: "1.0.0" }, + }), + // A package that holds no schema code may depend on the guarded names as it likes. + "packages/d/package.json": json({ name: "@t3tools/d", dependencies: { effect: "link:x" } }), + }, + SPEC_BASE, + ); + assert.equal(result.code, 0, result.output); +}); + +test("e2e: a package.json is judged by the entries schema code imports through it", () => { + const base = { + ...SPEC_BASE, + [`${MIGRATIONS}/001_A.ts`]: 'import { U } from "@t3tools/c/util";\n', + "packages/c/package.json": manifest({ + exports: { ".": "./src/index.ts", "./util": "./src/util.ts", "./ui/*": "./src/ui/*.ts" }, + }), + "packages/c/src/util.ts": "export const U = 1;\n", + }; + const exportsWith = (exports) => ({ "packages/c/package.json": manifest({ exports }) }); + const kept = { ".": "./src/index.ts", "./util": "./src/util.ts", "./ui/*": "./src/ui/*.ts" }; + const free = [ + // A new module, an edit of an entry no schema code imports, unrelated fields. + { + ...exportsWith({ ...kept, "./newThing": "./src/newThing.ts" }), + "packages/c/src/newThing.ts": "", + }, + exportsWith({ ...kept, ".": "./src/settings.ts" }), + { + "packages/c/package.json": manifest({ exports: kept, scripts: { test: "x" }, version: "2" }), + }, + ]; + for (const files of free) { + const result = prAgainstBase(files, base); + assert.equal(result.code, 0, result.output); + } + const refused = [ + exportsWith({ ...kept, "./util": "./src/settings.ts" }), + exportsWith({ ...kept, "./util": { node: "./src/evil.ts", default: "./src/util.ts" } }), + // A pattern added in front of the entry, or the entry turned into a pattern. + exportsWith({ ".": "./src/index.ts", "./*": "./src/*.ts" }), + { "packages/c/package.json": manifest({ exports: kept, imports: { "#u": "./x.ts" } }) }, + { "packages/c/package.json": JSON.stringify({ name: "@t3tools/other", exports: kept }) }, + { "packages/c/package.json": null }, + ]; + for (const files of refused) { + const result = prAgainstBase(files, base); + assert.equal(result.code, 1, `${JSON.stringify(files)}\n${result.output}`); + assert.match(result.output, /packages\/c\/package\.json: changes how it maps "\.\/util"/); + } +});