fix(git): use local-first base ref resolution in getLog, port to VS Code (#1284)

* chore: add .worktrees/ to gitignore for worktree workflow * fix: resolve remote-tracking base ref in getLog for PR description generation getLog was calling git log <base>..<head> with a bare branch name that often doesn't exist locally (e.g. main when only origin/main is present), causing a fatal 'unknown revision' error and HTTP 500. Apply the same origin/<base> resolution already used in getRangeDiff and getRangeFiles: check refs/remotes/origin/<base> first and prefer that ref if it exists. Also fix getGitLog in gitApiHttp.ts to read the JSON error body on failure instead of falling back to response.statusText, so the actual git error message surfaces in the toast instead of 'Internal Server Error'. * fix(git): use local-first ref resolution in getLog and port to VS Code - Replace unconditional origin/<from> preference in getLog() with a local-first fallback: prefer the local ref, only use origin/<from> when the local ref cannot be resolved, and pass through unchanged when neither resolves so git surfaces a meaningful error. - Extract the logic into an exported resolveBaseRefForLog(from, checkRef) helper so it is unit-testable without a real git repo. - Add service.test.js with 6 cases covering local-wins, origin-fallback, neither-exists passthrough, and falsy/empty inputs. - Port the same local-first resolution to packages/vscode/src/gitService.ts getGitLog() to close the cross-runtime parity gap; also handles from-only ranges as from..HEAD, matching the web service contract. * fix(vscode): add missing to-only range branch in getGitLog When only 'to' is supplied (no 'from'), the web service appends it as a positional git-log argument. The VS Code port was missing this branch and silently returned unbounded history instead. Adds the else-if to restore full cross-runtime parity. * fix(vscode): surface git log errors --------- Co-authored-by: Bohdan Triapitsyn <artmore@protonmail.com>
2026-05-17 19:28:08 +03:00
parent 77b4f27053
commit fa8fac2590
5 changed files with 125 additions and 9 deletions
@@ -62,3 +62,4 @@ Thumbs.db
 data/
 workspaces/
 *.pid
+.worktrees/
@@ -662,7 +662,8 @@ export async function getGitLog(
    })
  );
  if (!response.ok) {
-    throw new Error(`Failed to get git log: ${response.statusText}`);
+    const errorBody = await response.json().catch(() => ({ error: response.statusText }));
+    throw new Error(`Failed to get git log: ${errorBody.error || response.statusText}`);
  }
  return response.json();
 }
@@ -2515,6 +2515,34 @@ export interface GitLogEntry {
  deletions: number;
 }

+/**
+ * Resolve a log base ref using local-first semantics (mirrors web service.js).
+ *
+ * - Returns undefined when `from` is falsy/whitespace.
+ * - Returns `from` unchanged when the local ref resolves.
+ * - Returns `origin/<from>` when local is absent but the remote-tracking ref exists.
+ * - Returns `from` unchanged when neither resolves (lets git surface the error).
+ */
+async function resolveBaseRefForLog(
+  from: string | undefined,
+  directory: string
+): Promise<string | undefined> {
+  const normalized = typeof from === 'string' ? from.trim() : undefined;
+  if (!normalized) return undefined;
+
+  const checkRef = async (ref: string): Promise<boolean> => {
+    const result = await execGit(['rev-parse', '--verify', ref], directory);
+    return result.exitCode === 0 && Boolean(result.stdout.trim());
+  };
+
+  if (await checkRef(normalized)) return normalized;
+
+  const originRef = `refs/remotes/origin/${normalized}`;
+  if (await checkRef(originRef)) return `origin/${normalized}`;
+
+  return normalized;
+}
+
 /**
 * Get git log
 */
@@ -2523,6 +2551,11 @@ export async function getGitLog(
  options?: { maxCount?: number; from?: string; to?: string; file?: string }
 ): Promise<{ all: GitLogEntry[]; latest: GitLogEntry | null; total: number }> {
  const maxCount = options?.maxCount || 50;
+
+  // Prefer the local ref; fall back to origin/<from> only when the local ref
+  // cannot be resolved (e.g. user has never checked out the base branch).
+  const resolvedFrom = await resolveBaseRefForLog(options?.from, directory);
+
  const args = [
    'log',
    `--max-count=${maxCount}`,
@@ -2530,8 +2563,12 @@ export async function getGitLog(
    '--shortstat',
  ];
  
-  if (options?.from && options?.to) {
-    args.push(`${options.from}..${options.to}`);
+  if (resolvedFrom && options?.to) {
+    args.push(`${resolvedFrom}..${options.to}`);
+  } else if (resolvedFrom) {
+    args.push(`${resolvedFrom}..HEAD`);
+  } else if (options?.to) {
+    args.push(options.to);
  }
  
  if (options?.file) {
@@ -2541,7 +2578,7 @@ export async function getGitLog(
  const result = await execGit(args, directory);
  
  if (result.exitCode !== 0) {
-    return { all: [], latest: null, total: 0 };
+    throw new Error(result.stderr.trim() || result.stdout.trim() || 'Failed to get git log');
  }

  const entries: GitLogEntry[] = [];
@@ -2683,14 +2683,52 @@ export async function deleteBranch(directory, branch, options = {}) {
  }
 }

+/**
+ * Resolve a log base ref using local-first semantics.
+ *
+ * - If `from` is falsy / whitespace → return undefined.
+ * - If the local ref resolves → return it unchanged (caller's intent preserved).
+ * - If the local ref is absent but `origin/<from>` exists → return `origin/<from>`
+ *   (common when the user has never checked out the base branch locally).
+ * - If neither resolves → return `from` unchanged so git surfaces a meaningful error.
+ *
+ * @param {string | undefined} from   - The raw `from` option value.
+ * @param {(ref: string) => Promise<boolean>} checkRef - Returns true when the ref resolves.
+ * @returns {Promise<string | undefined>}
+ */
+export async function resolveBaseRefForLog(from, checkRef) {
+  const normalized = typeof from === 'string' ? from.trim() : undefined;
+  if (!normalized) return undefined;
+
+  if (await checkRef(normalized)) return normalized;
+
+  const originRef = `refs/remotes/origin/${normalized}`;
+  if (await checkRef(originRef)) return `origin/${normalized}`;
+
+  return normalized;
+}
+
 export async function getLog(directory, options = {}) {
  const git = await createGit(directory);

  try {
    const maxCount = options.maxCount || 50;
+
+    // Prefer the local ref; fall back to origin/<from> only when the local ref
+    // cannot be resolved (e.g. user has never checked out the base branch).
+    const checkRef = async (ref) => {
+      try {
+        const out = await git.raw(['rev-parse', '--verify', ref]);
+        return Boolean(out && out.trim());
+      } catch {
+        return false;
+      }
+    };
+    const resolvedFrom = await resolveBaseRefForLog(options.from, checkRef);
+
    const baseLog = await git.log({
      maxCount,
-      from: options.from,
+      from: resolvedFrom,
      to: options.to,
      file: options.file
    });
@@ -2703,10 +2741,10 @@ export async function getLog(directory, options = {}) {
      '--shortstat'
    ];

-    if (options.from && options.to) {
-      logArgs.push(`${options.from}..${options.to}`);
-    } else if (options.from) {
-      logArgs.push(`${options.from}..HEAD`);
+    if (resolvedFrom && options.to) {
+      logArgs.push(`${resolvedFrom}..${options.to}`);
+    } else if (resolvedFrom) {
+      logArgs.push(`${resolvedFrom}..HEAD`);
    } else if (options.to) {
      logArgs.push(options.to);
    }
@@ -0,0 +1,39 @@
+import { describe, expect, it } from 'vitest';
+
+import { resolveBaseRefForLog } from './service.js';
+
+describe('resolveBaseRefForLog', () => {
+  it('returns the local ref unchanged when it exists, even if origin also exists', async () => {
+    // Both local 'main' and 'refs/remotes/origin/main' are present.
+    // The local ref takes precedence — callers that ask for 'main' get 'main'.
+    const checkRef = async (ref) => ref === 'main' || ref === 'refs/remotes/origin/main';
+    expect(await resolveBaseRefForLog('main', checkRef)).toBe('main');
+  });
+
+  it('falls back to origin/<from> when local ref cannot be resolved but origin can', async () => {
+    // Local 'main' is absent (e.g. user never checked it out), but origin/main exists.
+    const checkRef = async (ref) => ref === 'refs/remotes/origin/main';
+    expect(await resolveBaseRefForLog('main', checkRef)).toBe('origin/main');
+  });
+
+  it('returns the original ref when neither local nor origin ref can be resolved', async () => {
+    // Neither ref exists; return as-is so git surfaces a meaningful error.
+    const checkRef = async () => false;
+    expect(await resolveBaseRefForLog('nonexistent-branch', checkRef)).toBe('nonexistent-branch');
+  });
+
+  it('returns undefined when from is undefined', async () => {
+    const checkRef = async () => true;
+    expect(await resolveBaseRefForLog(undefined, checkRef)).toBeUndefined();
+  });
+
+  it('returns undefined when from is an empty string', async () => {
+    const checkRef = async () => true;
+    expect(await resolveBaseRefForLog('', checkRef)).toBeUndefined();
+  });
+
+  it('returns undefined when from is a whitespace-only string', async () => {
+    const checkRef = async () => true;
+    expect(await resolveBaseRefForLog('   ', checkRef)).toBeUndefined();
+  });
+});