From 936e8c761b97ee4d671c9843e2b459adf01f7507 Mon Sep 17 00:00:00 2001 From: scuttletoby <123843277+scuttletoby@users.noreply.github.com> Date: Thu, 12 Nov 2026 13:34:34 +0000 Subject: [PATCH 1/2] fork: green main again (schema-check comments, checkout without submodule cleanup) Lint: eight comments in fork-schema-check.cjs were over the four-line cap of t3code/no-low-value-comments. Cut to the constraints; the restating parts and JSDoc type blocks are gone (the file is not type-checked). No database schema changes: actions/checkout with persist-credentials: false runs `git submodule foreach` to remove auth, and upstream's tree has gitlinks under .repos/alchemy-effect/submodules/ with no .gitmodules, so it died with "No url found for submodule path" before any script ran. The job now uses plain git: init, an anonymous blob:none fetch of the default branch, base and head, a sparse checkout of .github at github.sha. Still pull_request_target, still no checkout of PR code, any git error fails the step. Tests: a gitlink with no .gitmodules in the e2e set, and the workflow's own checkout step run against such a repository (plus fail-closed on a missing commit). The workflow shape test now allows only a checkout of github.sha. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/fork-schema-check.cjs | 85 +++++--------------- .github/scripts/fork-schema-check.test.cjs | 92 ++++++++++++++++++++-- .github/workflows/fork-checks.yml | 22 +++--- 3 files changed, 119 insertions(+), 80 deletions(-) diff --git a/.github/scripts/fork-schema-check.cjs b/.github/scripts/fork-schema-check.cjs index 3f98dbcf40a34..21ba11a6359c8 100644 --- a/.github/scripts/fork-schema-check.cjs +++ b/.github/scripts/fork-schema-check.cjs @@ -1,12 +1,7 @@ // 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; 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. +// strands installs that return to upstream. Refused: schema code and anything that changes what its +// imports resolve to, a PR copy of this check or .gitattributes, and branches holding commits +// already on the base. Files are read as git blobs. Usage: fork-schema-check.cjs . const { execFileSync } = require("node:child_process"); const path = require("node:path"); @@ -75,25 +70,10 @@ 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 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 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; - * imported: Set; mappings: Map>; holders: Set }} + * Files the schema roots import, transitively, plus what maps those imports (`mappings`, `imported`, + * `holders`). Barrels on the way to a declaring file are guarded (an edit could re-point a name), + * but their other re-exports are not followed. Nothing is skipped silently: an in-repo import with + * no file goes in `unresolved`; only packages outside the workspace go in `outside`. */ function traceSchema(tree) { const queue = tree.files.filter( @@ -409,11 +389,8 @@ function importedNames(source) { } /** - * 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[] }} + * The files an import reaches. For a package root: the files declaring `names` (`files`, whose own + * imports matter) and the barrels re-exporting them (`barrels`, guarded only). */ function resolveImport(ctx, from, specifier, names) { const { tree } = ctx; @@ -483,11 +460,8 @@ function resolveImport(ctx, from, specifier, names) { } /** - * 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[]} + * Every path a package.json can map `name[/subpath]` to, under any export condition, nested or in + * fallback arrays: the check does not guess which condition a tool picks, so it guards them all. */ function packageTargets(manifest, subpath) { const key = subpath ? `./${subpath}` : "."; @@ -540,11 +514,9 @@ function exportEntries(exports, key) { } /** - * 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. + * What a package.json says about the import `key` (`.` or `./sub`). Other export entries do not + * count, so adding a module to the package stays free. An absent or non-JSON file gives a value of + * its own, so it never compares equal to a real mapping. */ function manifestMapping(text, key) { if (text === undefined) return "(absent)"; @@ -604,13 +576,9 @@ function yamlEntries(text) { } /** - * 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[] }} + * The specs that decide what the imported `names` resolve to (the `holders`' dependency fields, root + * overrides and resolutions, pnpm-workspace.yaml catalogs and overrides). Specs of other names are + * not read, so other dependency bumps stay free. `patches` of those names are guarded like schema. */ function dependencySpecs(tree, names, holders) { const specs = new Map(); @@ -785,13 +753,9 @@ function resolveFile(tree, file) { } /** - * @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; - * `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 + * One reason per refused file. `changes`: one entry per path changed since the merge-base, with its + * `before`/`after` texts (undefined when absent), `sameAsBase` (equals the base's copy) and + * `fromOldMain` (a commit already on main left it). `staleCommits`: commits the base holds rebased. */ function findSchemaChanges(changes, { closure, baseHasCheck, staleCommits = 0 }) { const reasons = []; @@ -930,14 +894,7 @@ 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[] }} - */ +/** Judges what the branch changes in how schema code is found: merge-base tree against head tree. */ function compareTrees(before, after) { const trees = [before, after]; const traces = trees.map(traceSchema); diff --git a/.github/scripts/fork-schema-check.test.cjs b/.github/scripts/fork-schema-check.test.cjs index cd66805db7a7a..7a079ba440d0b 100644 --- a/.github/scripts/fork-schema-check.test.cjs +++ b/.github/scripts/fork-schema-check.test.cjs @@ -443,9 +443,8 @@ test("workspace packages come from the workspace list, scoped or not, at any dep assert.deepEqual([...trace.outside].sort(), ["@t3tools/skipped", "effect", "node:fs"]); }); -test("the workflow pins actions by commit SHA", () => { +test("any action the workflow uses is pinned 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); }); @@ -454,10 +453,12 @@ test("the workflow runs the default branch's copy and never runs pull request co 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/); + // The only checkout is of the default branch (`github.sha` under pull_request_target). + assert.doesNotMatch(code, /actions\/checkout|\bref:|secrets\.|GITHUB_TOKEN/); + assert.deepEqual(code.match(/git (checkout|switch|worktree|reset|read-tree)\b.*/g), [ + 'git checkout -q --detach "$DEFAULT_SHA"', + ]); + assert.deepEqual(code.match(/DEFAULT_SHA:.*/g), ["DEFAULT_SHA: ${{ github.sha }}"]); // 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); @@ -505,7 +506,34 @@ function scratchRepo() { }); return { code: result.status, output: result.stdout + result.stderr }; }; - return { dir, git, commit, check }; + return { dir, env, git, commit, check }; +} + +// The workflow's checkout step, run as CI runs it, against `origin` (a local repository). +function runCheckoutStep(origin, shas) { + const workflow = fs.readFileSync(WORKFLOW, "utf8"); + const block = workflow.match(/^ {8}run: \|\n((?: {10}.*\n)+)/m); + assert.ok(block && /git init/.test(block[1]), "the first run block is the checkout step"); + const script = block[1].replace(/^ {10}/gm, ""); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "fork-checks-runner-")); + const result = spawnSync("bash", ["--noprofile", "--norc", "-eo", "pipefail", "-c", script], { + cwd: dir, + env: { + ...origin.env, + GITHUB_SERVER_URL: `file://${path.dirname(origin.dir)}`, + GITHUB_REPOSITORY: path.basename(origin.dir), + ...shas, + }, + encoding: "utf8", + }); + return { dir, code: result.status, output: result.stdout + result.stderr }; +} + +/** A gitlink with no .gitmodules, as upstream's tree has under .repos/alchemy-effect/submodules/. */ +function addGitlink(repo, file, sha) { + repo.git("update-index", "--add", "--cacheinfo", `160000,${sha},${file}`); + repo.git("commit", "-q", "-m", "gitlink"); + return repo.git("rev-parse", "HEAD"); } const BASE_TREE = { @@ -1101,3 +1129,53 @@ test("e2e: a package.json is judged by the entries schema code imports through i assert.match(result.output, /packages\/c\/package\.json: changes how it maps "\.\/util"/); } }); + +test("e2e: a tree with a gitlink and no .gitmodules is checked like any other", () => { + const repo = scratchRepo(); + try { + repo.commit(BASE_TREE); + const base = addGitlink(repo, ".repos/vendor/submodules/dep", "1".repeat(40)); + const head = repo.commit({ "apps/server/src/feature/Store.ts": "export const x = 2;\n" }); + const bumped = addGitlink(repo, ".repos/vendor/submodules/dep", "2".repeat(40)); + for (const result of [repo.check(base, head), repo.check(base, bumped)]) { + assert.equal(result.code, 0, result.output); + assert.match(result.output, /No database schema changes/); + } + } finally { + fs.rmSync(repo.dir, { recursive: true, force: true }); + } +}); + +test("e2e: the workflow's checkout works on a tree with gitlinks and no .gitmodules", () => { + const origin = scratchRepo(); + origin.git("config", "uploadpack.allowFilter", "true"); + origin.git("config", "uploadpack.allowAnySHA1InWant", "true"); + const runners = []; + try { + origin.commit(BASE_TREE); + const main = addGitlink(origin, ".repos/vendor/submodules/dep", "1".repeat(40)); + const pr = origin.commit({ "apps/server/src/feature/Store.ts": "export const x = 2;\n" }); + origin.git("checkout", "-q", "--detach", main); + + const run = runCheckoutStep(origin, { DEFAULT_SHA: main, BASE_SHA: main, HEAD_SHA: pr }); + runners.push(run.dir); + assert.equal(run.code, 0, run.output); + const git = (...args) => + execFileSync("git", args, { cwd: run.dir, env: origin.env, encoding: "utf8" }).trim(); + assert.equal(git("rev-parse", "HEAD"), main, "the default branch is checked out, not the PR"); + assert.equal(git("cat-file", "-t", pr), "commit", "the PR head is fetched as objects"); + assert.ok(fs.existsSync(path.join(run.dir, CHECK_FILES[0]))); + assert.ok(!fs.existsSync(path.join(run.dir, "apps")), "only .github and root files"); + + // Fail closed: a commit the remote does not have stops the job. + const missing = runCheckoutStep(origin, { + DEFAULT_SHA: main, + BASE_SHA: main, + HEAD_SHA: "3".repeat(40), + }); + runners.push(missing.dir); + assert.notEqual(missing.code, 0, missing.output); + } finally { + for (const dir of [origin.dir, ...runners]) fs.rmSync(dir, { recursive: true, force: true }); + } +}); diff --git a/.github/workflows/fork-checks.yml b/.github/workflows/fork-checks.yml index e755470fe5993..f710599c59d19 100644 --- a/.github/workflows/fork-checks.yml +++ b/.github/workflows/fork-checks.yml @@ -20,18 +20,22 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 5 steps: - - name: Checkout the default branch - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 - with: - fetch-depth: 0 - filter: blob:none - persist-credentials: false - - - name: Fetch the pull request's commits (not checked out) + # Plain git, not actions/checkout: upstream's tree has gitlinks under .repos/ but no + # .gitmodules, and checkout's credential cleanup runs `git submodule foreach`, which fails on + # them. Nothing is authenticated (public repository) and only the default branch is checked out. + - name: Check out the default branch and fetch the pull request's commits (not checked out) env: + DEFAULT_SHA: ${{ github.sha }} 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" + run: | + set -euo pipefail + git init -q + git remote add origin "$GITHUB_SERVER_URL/$GITHUB_REPOSITORY" + git fetch -q --no-tags --no-recurse-submodules --filter=blob:none origin \ + "$DEFAULT_SHA" "$BASE_SHA" "$HEAD_SHA" + git sparse-checkout set --cone .github + git checkout -q --detach "$DEFAULT_SHA" - name: Test the check run: node --test .github/scripts/fork-schema-check.test.cjs From e76c87755efab2875c95d1ee99da48373e4114d1 Mon Sep 17 00:00:00 2001 From: scuttletoby <123843277+scuttletoby@users.noreply.github.com> Date: Thu, 12 Nov 2026 13:39:04 +0000 Subject: [PATCH 2/2] fork-checks: full checkout of the default branch, so the check does not fetch blobs one by one A sparse checkout left every blob the check reads to a separate lazy fetch from the partial clone (about 170 round trips on the real tree), close to the job's five-minute timeout. A full checkout fetches the default branch's blobs in one batch, as actions/checkout did before. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/fork-schema-check.test.cjs | 1 - .github/workflows/fork-checks.yml | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/scripts/fork-schema-check.test.cjs b/.github/scripts/fork-schema-check.test.cjs index 7a079ba440d0b..4d6e3ebe7b017 100644 --- a/.github/scripts/fork-schema-check.test.cjs +++ b/.github/scripts/fork-schema-check.test.cjs @@ -1165,7 +1165,6 @@ test("e2e: the workflow's checkout works on a tree with gitlinks and no .gitmodu assert.equal(git("rev-parse", "HEAD"), main, "the default branch is checked out, not the PR"); assert.equal(git("cat-file", "-t", pr), "commit", "the PR head is fetched as objects"); assert.ok(fs.existsSync(path.join(run.dir, CHECK_FILES[0]))); - assert.ok(!fs.existsSync(path.join(run.dir, "apps")), "only .github and root files"); // Fail closed: a commit the remote does not have stops the job. const missing = runCheckoutStep(origin, { diff --git a/.github/workflows/fork-checks.yml b/.github/workflows/fork-checks.yml index f710599c59d19..46f0e986a2def 100644 --- a/.github/workflows/fork-checks.yml +++ b/.github/workflows/fork-checks.yml @@ -22,7 +22,8 @@ jobs: steps: # Plain git, not actions/checkout: upstream's tree has gitlinks under .repos/ but no # .gitmodules, and checkout's credential cleanup runs `git submodule foreach`, which fails on - # them. Nothing is authenticated (public repository) and only the default branch is checked out. + # them. The full checkout fetches the default branch's blobs in one batch; a sparse one leaves + # the check to fetch every blob it reads one at a time. - name: Check out the default branch and fetch the pull request's commits (not checked out) env: DEFAULT_SHA: ${{ github.sha }} @@ -34,7 +35,6 @@ jobs: git remote add origin "$GITHUB_SERVER_URL/$GITHUB_REPOSITORY" git fetch -q --no-tags --no-recurse-submodules --filter=blob:none origin \ "$DEFAULT_SHA" "$BASE_SHA" "$HEAD_SHA" - git sparse-checkout set --cone .github git checkout -q --detach "$DEFAULT_SHA" - name: Test the check