gh-prs-merge --fix: resolve lockfile-only conflicts instead of skipping (0.69.0)

bin/gh-prs-merge.ts

@@ -15,6 +15,7 @@
 import { csv, integer, parseArgs, UsageError } from '../src/args.ts';
 import { isMain } from '../src/is-main.ts';
 import { Gh } from '../src/gh.ts';
+import { resolveLockfileConflicts } from '../src/lockfile-conflicts.ts';
 import { defaults, render, renderSummary, sweep, type MergeOptions } from '../src/prs-merge.ts';
 
 const USAGE = `Usage:
@@ -74,11 +75,19 @@ Fixing (--fix):
   parked PRs are polled together and they are all running their suites at the
   same time anyway.
 
+    lockfile-only conflict  mergeStateStatus=DIRTY where every conflicting
+                            file is bun.lock(b), pnpm-lock.yaml or
+                            package-lock.json: merges the base in a blobless
+                            clone under ~/.cache, takes the base's lockfile,
+                            rebuilds it (lockfile-only, no scripts), pushes a
+                            merge commit (never a force push) and parks it.
+                            Same-repo PRs only; forks stay FIXME
+
   Never repaired:
-    mergeStateStatus=DIRTY is GitHub's settled verdict that the branch
-    conflicts with its base. update-branch cannot merge over a conflict, so it
-    is not attempted; the PR is reported once as FIXME and left alone.
-    Resolving a conflict means choosing between two authors' intent. A check
+    Any other DIRTY PR. update-branch cannot merge over a conflict, so it is
+    not attempted; the PR is reported once as FIXME, naming the conflicting
+    files, and left alone. Resolving a conflict in code means choosing
+    between two authors' intent. A check
     that ran and failed is a result, not an obstacle.
 `;
 
@@ -104,6 +113,7 @@ async function main(argv: string[]): Promise<number> {
     fixWaitMs:
       integer(parsed.values, '--fix-wait', defaults.fixWaitMs / 1000, { max: 86_400 }) * 1000,
     pollMs: defaults.pollMs,
+    resolveLockfiles: (pr) => resolveLockfileConflicts(pr),
   };
 
   if (options.orgs.length === 0 && options.users.length === 0) {

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@profullstack/cli-tools",
-  "version": "0.68.4",
+  "version": "0.69.0",
   "private": true,
   "description": "Local command-line tools, in TypeScript, exposed on PATH.",
   "type": "module",

src/lockfile-conflicts.ts

@@ -0,0 +1,141 @@
+import { mkdir, mkdtemp, rm } from 'node:fs/promises';
+import { homedir } from 'node:os';
+import { basename, dirname, join } from 'node:path';
+import { run, type RunResult } from './exec.ts';
+import { parsePullRequestUrl, type PullRequest } from './gh.ts';
+
+/**
+ * Lockfiles a package manager can rebuild from the manifests beside it.
+ *
+ * A conflict in one of these is not two authors disagreeing: each side bumped
+ * a dependency, and the merged package.json already says what both wanted. Take
+ * the base's lockfile and let the tool that owns it re-resolve. Lockfile-only
+ * and no scripts, so nothing from the branch is executed.
+ *
+ * yarn.lock is absent on purpose: classic yarn has no lockfile-only mode, and
+ * guessing which yarn a repo uses is worse than a FIXME.
+ */
+export const LOCKFILES: Readonly<Record<string, readonly [string, readonly string[]]>> = {
+  'bun.lock': ['bun', ['install', '--lockfile-only', '--ignore-scripts']],
+  'bun.lockb': ['bun', ['install', '--lockfile-only', '--ignore-scripts']],
+  'pnpm-lock.yaml': ['pnpm', ['install', '--lockfile-only', '--ignore-scripts']],
+  'package-lock.json': [
+    'npm',
+    ['install', '--package-lock-only', '--ignore-scripts', '--no-audit', '--no-fund'],
+  ],
+};
+
+export const isLockfile = (path: string): boolean => basename(path) in LOCKFILES;
+
+export type LockfileOutcome =
+  | { kind: 'resolved'; files: string[] }
+  | { kind: 'manual'; reason: string };
+
+/** Resolves a DIRTY PR when every conflicting file is a lockfile. */
+export type LockfileResolver = (pr: PullRequest) => Promise<LockfileOutcome>;
+
+const tail = (result: RunResult): string =>
+  (result.stderr.trim() || result.stdout.trim()).replace(/\s+/g, ' ').slice(-300);
+
+/**
+ * Merge the base into the head in a throwaway blobless clone, rebuild the
+ * conflicting lockfiles, and push the merge commit. Never a force push: if the
+ * branch moved underneath us the push is refused and the PR stays a FIXME.
+ *
+ * The clone lives under ~/.cache, not os.tmpdir(): on these boxes /tmp is a
+ * quota'd tmpfs and a repo checkout is exactly what fills it.
+ */
+export async function resolveLockfileConflicts(
+  pr: PullRequest,
+  exec: typeof run = run,
+): Promise<LockfileOutcome> {
+  const parsed = parsePullRequestUrl(pr.url);
+  if (!parsed) return { kind: 'manual', reason: `cannot parse ${pr.url}` };
+
+  const env = { ...process.env, GH_PAGER: 'cat', GIT_TERMINAL_PROMPT: '0' };
+  const view = await exec(
+    'gh',
+    ['pr', 'view', pr.url, '--json', 'headRefName,baseRefName,isCrossRepository'],
+    { env },
+  );
+  if (view.code !== 0) return { kind: 'manual', reason: `gh pr view failed: ${tail(view)}` };
+
+  const refs = JSON.parse(view.stdout) as {
+    headRefName: string;
+    baseRefName: string;
+    isCrossRepository: boolean;
+  };
+  // A fork's branch is not ours to push to, and its manifests are not ours to trust.
+  if (refs.isCrossRepository) return { kind: 'manual', reason: 'PR is from a fork' };
+
+  const root = join(process.env.XDG_CACHE_HOME || join(homedir(), '.cache'), 'gh-prs-merge');
+  await mkdir(root, { recursive: true });
+  const dir = await mkdtemp(join(root, `${parsed.slug.replace('/', '-')}-${parsed.number}-`));
+
+  const git = (args: string[], cwd = dir) =>
+    exec('git', ['-c', 'advice.mergeConflict=false', ...args], { env, cwd, timeoutMs: 600_000 });
+
+  try {
+    const clone = await exec(
+      'gh',
+      ['repo', 'clone', parsed.slug, dir, '--', '--filter=blob:none', '--branch', refs.headRefName],
+      { env, timeoutMs: 600_000 },
+    );
+    if (clone.code !== 0) return { kind: 'manual', reason: `clone failed: ${tail(clone)}` };
+
+    const head = await git(['rev-parse', 'HEAD']);
+    if (head.stdout.trim() !== pr.headRefOid) {
+      return { kind: 'manual', reason: 'head moved since it was judged' };
+    }
+
+    const fetch = await git(['fetch', '--quiet', 'origin', refs.baseRefName]);
+    if (fetch.code !== 0) return { kind: 'manual', reason: `fetch failed: ${tail(fetch)}` };
+
+    const message = `Merge ${refs.baseRefName} into ${refs.headRefName}`;
+    const merge = await git(['merge', '--no-ff', '--no-commit', '-m', message, 'FETCH_HEAD']);
+
+    const unmerged = await git(['diff', '--name-only', '--diff-filter=U']);
+    const conflicted = unmerged.stdout.split('\n').filter(Boolean);
+
+    if (merge.code !== 0 && conflicted.length === 0) {
+      return { kind: 'manual', reason: `git merge failed: ${tail(merge)}` };
+    }
+
+    const other = conflicted.filter((f) => !isLockfile(f));
+    if (other.length > 0) {
+      return { kind: 'manual', reason: `conflicts outside lockfiles: ${other.join(', ')}` };
+    }
+
+    for (const file of conflicted) {
+      // Merging base into head, "theirs" is the base: the lockfile main already
+      // resolved, which the branch's own changes are then applied on top of.
+      const take = await git(['checkout', '--theirs', '--', file]);
+      if (take.code !== 0) return { kind: 'manual', reason: `checkout ${file}: ${tail(take)}` };
+
+      const [bin, args] = LOCKFILES[basename(file)]!;
+      const regen = await exec(bin, [...args], {
+        env,
+        cwd: join(dir, dirname(file)),
+        timeoutMs: 600_000,
+      });
+      if (regen.code !== 0) {
+        return { kind: 'manual', reason: `${bin} could not rebuild ${file}: ${tail(regen)}` };
+      }
+
+      const add = await git(['add', '--', file]);
+      if (add.code !== 0) return { kind: 'manual', reason: `git add ${file}: ${tail(add)}` };
+    }
+
+    const body =
+      conflicted.length > 0 ? `\n\nRegenerated ${conflicted.join(', ')} by gh-prs-merge --fix.` : '';
+    const commit = await git(['commit', '--no-verify', '-m', `${message}${body}`]);
+    if (commit.code !== 0) return { kind: 'manual', reason: `commit failed: ${tail(commit)}` };
+
+    const push = await git(['push', '--quiet', 'origin', `HEAD:refs/heads/${refs.headRefName}`]);
+    if (push.code !== 0) return { kind: 'manual', reason: `push refused: ${tail(push)}` };
+
+    return { kind: 'resolved', files: conflicted };
+  } finally {
+    await rm(dir, { recursive: true, force: true });
+  }
+}

src/prs-merge.ts

@@ -1,5 +1,6 @@
 import { Gh, RateLimitError, type Check, type MergeState, type PullRequest, type WorkflowRun } from './gh.ts';
 import { sleep } from './exec.ts';
+import type { LockfileResolver } from './lockfile-conflicts.ts';
 
 export interface MergeOptions {
   orgs: string[];
@@ -12,6 +13,11 @@ export interface MergeOptions {
   fixWaitMs: number;
   /** Poll interval while waiting on running checks. Shortened by tests. */
   pollMs: number;
+  /**
+   * With --fix, rebuilds a DIRTY PR whose only conflicts are lockfiles. Left
+   * out (as in tests), every DIRTY PR is a FIXME.
+   */
+  resolveLockfiles?: LockfileResolver;
 }
 
 export interface Summary {
@@ -363,6 +369,41 @@ export async function sweep(
         continue;
       }
 
+      if (pr.mergeStateStatus === 'DIRTY' && options.resolveLockfiles) {
+        // Both sides bumped dependencies and only the lockfile collided. That
+        // is not a disagreement: the merged manifests say what both wanted.
+        emit({ kind: 'fixing', url, text: `${reason}; trying a lockfile-only conflict resolution` });
+        let outcome: Awaited<ReturnType<LockfileResolver>>;
+        try {
+          outcome = await options.resolveLockfiles(pr);
+        } catch (error) {
+          outcome = {
+            kind: 'manual',
+            reason: error instanceof Error ? error.message : String(error),
+          };
+        }
+
+        if (outcome.kind === 'resolved') {
+          emit({
+            kind: 'fixing',
+            url,
+            text: `merged the base in and regenerated ${outcome.files.join(', ') || 'nothing'}; parked for the second pass`,
+          });
+          summary.fixed += 1;
+          parked.push({ url, title: pr.title, repaired: true, awaitingChecks: true });
+          continue;
+        }
+
+        emit({
+          kind: 'fixme',
+          url,
+          text: `${reason}; the branch conflicts with its base and needs resolving by hand (${outcome.reason})`,
+          title: pr.title,
+        });
+        summary.skipped += 1;
+        continue;
+      }
+
       if (pr.mergeStateStatus === 'DIRTY') {
         // A real conflict. Say so once — as FIXME, not as a second SKIP line
         // repeating it — and leave it alone: choosing between two authors'

test/lockfile-conflicts.test.ts

@@ -0,0 +1,105 @@
+import { describe, expect, it } from 'vitest';
+import type { RunResult } from '../src/exec.ts';
+import type { PullRequest } from '../src/gh.ts';
+import { isLockfile, resolveLockfileConflicts } from '../src/lockfile-conflicts.ts';
+
+const pr: PullRequest = {
+  url: 'https://github.com/acme/repo/pull/7',
+  title: 't',
+  state: 'OPEN',
+  isDraft: false,
+  mergeable: 'CONFLICTING',
+  mergeStateStatus: 'DIRTY',
+  headRefOid: 'deadbeef',
+};
+
+/** A scripted gh/git/bun; `conflicted` is what `git diff --diff-filter=U` reports. */
+function stubExec(options: {
+  conflicted: string[];
+  fork?: boolean;
+  head?: string;
+  pushFails?: boolean;
+}) {
+  const calls: string[] = [];
+  const ok = (stdout = ''): RunResult => ({ code: 0, stdout, stderr: '' });
+  const exec = async (file: string, args: readonly string[]): Promise<RunResult> => {
+    const argv = args.filter((a, i) => !(a === '-c' || args[i - 1] === '-c'));
+    calls.push(`${file} ${argv.join(' ')}`);
+    if (file === 'gh' && argv[0] === 'pr') {
+      return ok(
+        JSON.stringify({
+          headRefName: 'feat',
+          baseRefName: 'main',
+          isCrossRepository: !!options.fork,
+        }),
+      );
+    }
+    if (file === 'git' && argv[0] === 'rev-parse') return ok(`${options.head ?? 'deadbeef'}\n`);
+    if (file === 'git' && argv[0] === 'merge') {
+      return options.conflicted.length ? { code: 1, stdout: 'CONFLICT', stderr: '' } : ok();
+    }
+    if (file === 'git' && argv[0] === 'diff') return ok(options.conflicted.join('\n'));
+    if (file === 'git' && argv[0] === 'push' && options.pushFails) {
+      return { code: 1, stdout: '', stderr: '! [rejected] (fetch first)' };
+    }
+    return ok();
+  };
+  return { exec, calls };
+}
+
+describe('lockfile conflict resolution', () => {
+  it('recognises lockfiles anywhere in the tree, and nothing else', () => {
+    expect(isLockfile('bun.lock')).toBe(true);
+    expect(isLockfile('apps/web/pnpm-lock.yaml')).toBe(true);
+    expect(isLockfile('package-lock.json')).toBe(true);
+    expect(isLockfile('yarn.lock')).toBe(false);
+    expect(isLockfile('package.json')).toBe(false);
+  });
+
+  it('takes the base lockfile, rebuilds it lockfile-only, and pushes without force', async () => {
+    const { exec, calls } = stubExec({ conflicted: ['bun.lock'] });
+    const outcome = await resolveLockfileConflicts(pr, exec);
+
+    expect(outcome).toEqual({ kind: 'resolved', files: ['bun.lock'] });
+    expect(calls).toContain('git checkout --theirs -- bun.lock');
+    expect(calls).toContain('bun install --lockfile-only --ignore-scripts');
+    const push = calls.find((c) => c.startsWith('git push'))!;
+    expect(push).toContain('HEAD:refs/heads/feat');
+    expect(push).not.toMatch(/--force|\+HEAD/);
+  });
+
+  it('declines when a code file conflicts too, before rebuilding anything', async () => {
+    const { exec, calls } = stubExec({ conflicted: ['bun.lock', 'src/index.ts'] });
+    const outcome = await resolveLockfileConflicts(pr, exec);
+
+    expect(outcome).toEqual({
+      kind: 'manual',
+      reason: 'conflicts outside lockfiles: src/index.ts',
+    });
+    expect(calls.some((c) => c.startsWith('bun '))).toBe(false);
+    expect(calls.some((c) => c.startsWith('git push'))).toBe(false);
+  });
+
+  it('never touches a fork', async () => {
+    const { exec, calls } = stubExec({ conflicted: ['bun.lock'], fork: true });
+    expect(await resolveLockfileConflicts(pr, exec)).toEqual({
+      kind: 'manual',
+      reason: 'PR is from a fork',
+    });
+    expect(calls.some((c) => c.includes('clone'))).toBe(false);
+  });
+
+  it('declines when the head moved after the PR was judged', async () => {
+    const { exec, calls } = stubExec({ conflicted: ['bun.lock'], head: 'cafe' });
+    const outcome = await resolveLockfileConflicts(pr, exec);
+    expect(outcome.kind).toBe('manual');
+    expect(calls.some((c) => c.startsWith('git push'))).toBe(false);
+  });
+
+  it('reports a refused push instead of claiming the repair', async () => {
+    const { exec } = stubExec({ conflicted: ['bun.lock'], pushFails: true });
+    const outcome = await resolveLockfileConflicts(pr, exec);
+    expect(outcome.kind).toBe('manual');
+    expect(outcome.kind === 'manual' && outcome.reason).toContain('push refused');
+  });
+});