refactor(lint): use git-changed for --changed detection

Author: pyramation

.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>` (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):
 

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 <base>`, 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
 

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', () => {

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:*"

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<string>(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');
 }