diff --git a/.agents/skills/pgsql-lint/SKILL.md b/.agents/skills/pgsql-lint/SKILL.md index 172c7a42..5924439a 100644 --- a/.agents/skills/pgsql-lint/SKILL.md +++ b/.agents/skills/pgsql-lint/SKILL.md @@ -52,8 +52,9 @@ targets, so a CI step is just `pgsql-lint --changed`. `--changed` diffs against `git merge-base HEAD ` (base: explicit → `$GITHUB_BASE_REF` → the repository's default branch), unions in working-tree and untracked changes, drops paths that no longer exist, and falls back to -`git diff HEAD` on a shallow/detached checkout. Modelled on pgpm's bundle-drift -check. Nothing changed → exit 0. +working-tree changes only on a shallow/detached checkout. All of that is the +`git-changed` package — `src/changed.ts` is just the `.sql` filter over it, and +pgpm's bundle-drift check uses the same package. Nothing changed → exit 0. Programmatic entry points (all pure, DB-free): diff --git a/packages/lint/README.md b/packages/lint/README.md index 29cdc903..f88219b9 100644 --- a/packages/lint/README.md +++ b/packages/lint/README.md @@ -62,8 +62,11 @@ request's base branch (`$GITHUB_BASE_REF`) and otherwise to the repository's default branch; the diff is taken against `git merge-base HEAD `, so commits landed on the base branch afterwards don't widen the set. Uncommitted and untracked changes are included, deleted/renamed-away paths are dropped, and a -shallow clone or detached checkout (no resolvable merge base) falls back to the -working-tree diff against `HEAD`. Nothing changed is an exit-0 pass. +shallow clone or detached checkout (no resolvable base) falls back to working-tree +changes only. Nothing changed is an exit-0 pass. + +Detection is [`git-changed`](https://npmjs.com/package/git-changed), shared with +`pgpm package --check`, so the two agree on what "changed" means. ### Config file diff --git a/packages/lint/__tests__/changed.test.ts b/packages/lint/__tests__/changed.test.ts index 4e951f57..7384686c 100644 --- a/packages/lint/__tests__/changed.test.ts +++ b/packages/lint/__tests__/changed.test.ts @@ -124,9 +124,13 @@ describe('resolveChangedBase', () => { expect(resolveChangedBase('release/1.0', repo())).toBe('release/1.0'); }); - it('uses the PR base branch in CI, unprefixed when no remote has it', () => { + it('ignores a PR base branch that names no ref, rather than returning a broken one', () => { + // Neither `origin/develop` nor `develop` exists in this fixture. Handing back + // `develop` anyway would make every later git call fail and quietly reduce the + // gate to working-tree changes — nothing at all in CI, where the work is + // already committed. The default branch is a real ref, so the gate still runs. process.env.GITHUB_BASE_REF = 'develop'; - expect(resolveChangedBase(undefined, repo())).toBe('develop'); + expect(resolveChangedBase(undefined, repo())).toBe('main'); }); it('falls back to the repository default branch', () => { diff --git a/packages/lint/package.json b/packages/lint/package.json index d1e0cc56..7199f332 100644 --- a/packages/lint/package.json +++ b/packages/lint/package.json @@ -36,6 +36,7 @@ "dependencies": { "@pgsql/traverse": "workspace:*", "chalk": "^4.1.0", + "git-changed": "^0.3.0", "libpg-query": "18.1.4", "minimist": "1.2.8", "pgsql-parser": "workspace:*" diff --git a/packages/lint/src/changed.ts b/packages/lint/src/changed.ts index 31165272..e87a5035 100644 --- a/packages/lint/src/changed.ts +++ b/packages/lint/src/changed.ts @@ -2,44 +2,15 @@ * Changed-file detection for `--changed`, so a CI gate (or a human before a * commit) pays only for the SQL that a branch actually touched. * - * Modelled on pgpm's bundle-drift check (`pgpm/core/src/packaging/check.ts` in - * constructive): resolve a base ref (explicit → `origin/$GITHUB_BASE_REF` in a - * PR → the repository's default branch), diff `HEAD` against the **merge base** - * so unrelated commits on the base branch don't widen the set, and union that - * with uncommitted/untracked working-tree changes. Deleted paths are dropped — - * there is nothing left on disk to lint. + * The git plumbing — base resolution, merge-base diff, working-tree union, + * rename targets, dropping paths that no longer exist — lives in `git-changed`, + * which is shared with pgpm's bundle-drift check. This module is the `.sql` + * filter over it, and the place where "no base" stays non-fatal: a lint gate + * that refuses to run on a shallow clone lints nothing, which is worse than + * linting the working tree. */ -import { execFileSync } from 'child_process'; -import { existsSync, statSync } from 'fs'; -import * as path from 'path'; - -function git(args: string[], cwd: string): string { - return execFileSync('git', args, { - cwd, - encoding: 'utf-8', - stdio: ['ignore', 'pipe', 'ignore'], - maxBuffer: 64 * 1024 * 1024 - }); -} - -function tryGit(args: string[], cwd: string): string | null { - try { - return git(args, cwd); - } catch { - return null; - } -} - -/** The repository's default branch as a remote-tracking ref, when discoverable. */ -function defaultBranch(cwd: string): string | undefined { - const head = tryGit(['symbolic-ref', '--short', 'refs/remotes/origin/HEAD'], cwd); - if (head && head.trim()) return head.trim(); - for (const candidate of ['origin/main', 'origin/master', 'main', 'master']) { - if (tryGit(['rev-parse', '--verify', '--quiet', candidate], cwd)) return candidate; - } - return undefined; -} +import { changedFiles as gitChangedFiles, isRepo, resolveBase } from 'git-changed'; /** * Resolve the ref to diff against. An explicit `base` wins; otherwise the PR @@ -48,14 +19,7 @@ function defaultBranch(cwd: string): string | undefined { * the caller falls back to working-tree changes only. */ export function resolveChangedBase(base?: string, cwd: string = process.cwd()): string | undefined { - if (base && base.trim()) return base.trim(); - const prBase = process.env.GITHUB_BASE_REF; - if (prBase && prBase.trim()) { - const ref = `origin/${prBase.trim()}`; - if (tryGit(['rev-parse', '--verify', '--quiet', ref], cwd)) return ref; - return prBase.trim(); - } - return defaultBranch(cwd); + return resolveBase(base, cwd); } export interface ChangedFilesResult { @@ -67,65 +31,23 @@ export interface ChangedFilesResult { mergeBase?: string; } -/** Parse `git status --porcelain` into paths (rename target wins). */ -function workingTreePaths(cwd: string): string[] { - const out: string[] = []; - // `-uall` lists untracked *files*; the default collapses a new directory to - // the directory name, which would hide every file a new module adds. - const status = tryGit(['status', '--porcelain', '-uall'], cwd) ?? ''; - for (const rawLine of status.split('\n')) { - const line = rawLine.trimEnd(); - if (!line) continue; - let p = line.slice(3); - const arrow = p.indexOf(' -> '); - if (arrow !== -1) p = p.slice(arrow + 4); - p = p.replace(/^"|"$/g, ''); - if (p) out.push(p); +function collect(cwd: string, base: string | undefined, ext?: string): ChangedFilesResult { + if (!isRepo(cwd)) { + throw new Error(`--changed needs a git repository; ${cwd} is not inside one`); } - return out; + const result = gitChangedFiles({ cwd, base, ext }); + return { files: result.paths, base: result.base, mergeBase: result.mergeBase }; } /** * Collect the files that differ from `base` (via `git merge-base`) plus any - * uncommitted/untracked working-tree changes. Falls back to `git diff HEAD` - * when no base is resolvable or no merge base exists — a shallow clone or a - * detached CI checkout — rather than failing the run. + * uncommitted/untracked working-tree changes. */ export function changedFiles(options: { cwd?: string; base?: string } = {}): ChangedFilesResult { - const cwd = options.cwd ?? process.cwd(); - if (!tryGit(['rev-parse', '--git-dir'], cwd)) { - throw new Error(`--changed needs a git repository; ${cwd} is not inside one`); - } - - const files = new Set(workingTreePaths(cwd)); - const base = resolveChangedBase(options.base, cwd); - let mergeBase: string | undefined; - - if (base) { - const found = tryGit(['merge-base', 'HEAD', base], cwd); - mergeBase = found?.trim() || undefined; - } - // No base, or no common ancestor (shallow clone / detached checkout): the - // uncommitted diff against HEAD is all the history we can see. - const diffArgs = mergeBase - ? ['diff', '--name-only', '--diff-filter=ACMR', mergeBase, 'HEAD'] - : ['diff', '--name-only', '--diff-filter=ACMR', 'HEAD']; - for (const rawLine of (tryGit(diffArgs, cwd) ?? '').split('\n')) { - const p = rawLine.trim(); - if (p) files.add(p); - } - - const abs: string[] = []; - for (const rel of files) { - const full = path.resolve(cwd, rel); - // Deleted or renamed-away paths have nothing left to lint. - if (existsSync(full) && statSync(full).isFile()) abs.push(full); - } - return { files: abs.sort(), base, mergeBase }; + return collect(options.cwd ?? process.cwd(), options.base); } /** {@link changedFiles}, narrowed to `.sql`. */ export function changedSqlFiles(options: { cwd?: string; base?: string } = {}): ChangedFilesResult { - const result = changedFiles(options); - return { ...result, files: result.files.filter((f) => f.toLowerCase().endsWith('.sql')) }; + return collect(options.cwd ?? process.cwd(), options.base, '.sql'); } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 16a706fb..d9fa3045 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -99,6 +99,9 @@ importers: chalk: specifier: ^4.1.0 version: 4.1.2 + git-changed: + specifier: ^0.3.0 + version: 0.3.0 libpg-query: specifier: 18.1.4 version: 18.1.4 @@ -3612,6 +3615,13 @@ packages: } engines: { node: ">=10" } + git-changed@0.3.0: + resolution: + { + integrity: sha512-tNAKg96UMf8YBlyr7nZFulqS/yEhSQ+Q/1QVFnQ4mUe4f5eIwhK+Oie11x4IixWD+SB4AmSaLR84S3qj2DTkKA==, + } + hasBin: true + git-raw-commits@3.0.0: resolution: { @@ -9014,6 +9024,8 @@ snapshots: get-stream@6.0.1: {} + git-changed@0.3.0: {} + git-raw-commits@3.0.0: dependencies: dargs: 7.0.0