From b3aa719a57ca7f5b8fe7db38abf3e9dc4b58a4bb Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 10 Oct 2026 01:10:30 -0700 Subject: [PATCH] Preserve Git history lanes through unrelated roots (#26932) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Preserve active graph lanes through unrelated root commits and share node/reference colors. Isolate real-Git fixtures from inherited Git settings and use the existing public process-host import after the current-main package migration. Co-authored-by: Matheus Breguêz --- .../sync/git-history-graph-svg.test.tsx | 110 +++++++++++++ .../sync/git-history-graph-svg.tsx | 10 +- src/shared/git-history-graph.test.ts | 38 ++++- src/shared/git-history-graph.ts | 68 +++++--- .../git-history-real-repository.test.ts | 152 ++++++++++++++++++ 5 files changed, 349 insertions(+), 29 deletions(-) create mode 100644 src/renderer/src/components/right-sidebar/source-control/sync/git-history-graph-svg.test.tsx create mode 100644 src/shared/git-history-real-repository.test.ts diff --git a/src/renderer/src/components/right-sidebar/source-control/sync/git-history-graph-svg.test.tsx b/src/renderer/src/components/right-sidebar/source-control/sync/git-history-graph-svg.test.tsx new file mode 100644 index 00000000000..cda77aebbfc --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control/sync/git-history-graph-svg.test.tsx @@ -0,0 +1,110 @@ +import { renderToStaticMarkup } from 'react-dom/server' +import { describe, expect, it } from 'vitest' +import { + buildDefaultGitHistoryColorMap, + buildGitHistoryViewModels, + type GitHistoryItemViewModel +} from '../../../../../../shared/git-history-graph' +import type { GitHistoryItem } from '../../../../../../shared/git-history' +import { GitHistoryGraphSvg } from './git-history-graph-svg' + +function item(id: string, parentIds: string[]): GitHistoryItem { + return { id, parentIds, subject: id, message: id } +} + +function renderGraph(viewModel: GitHistoryItemViewModel): string { + return renderToStaticMarkup() +} + +describe('Git history graph rendering', () => { + it('shifts a surviving lane past a root to its left without changing the root color', () => { + const markup = renderGraph({ + historyItem: item('root', []), + kind: 'node', + inputSwimlanes: [ + { id: 'root', color: 'git-graph-lane-1' }, + { id: 'other', color: 'git-graph-lane-2' } + ], + outputSwimlanes: [{ id: 'other', color: 'git-graph-lane-2' }] + }) + + expect(markup).toContain('d="M 22 0 V 6') + expect(markup).toContain('cx="11" cy="12" r="3.5" fill="var(--git-graph-lane-1)"') + }) + + it('draws a surviving lane straight past a root to its right', () => { + const markup = renderGraph({ + historyItem: item('root', []), + kind: 'node', + inputSwimlanes: [ + { id: 'other', color: 'git-graph-lane-1' }, + { id: 'root', color: 'git-graph-lane-2' } + ], + outputSwimlanes: [{ id: 'other', color: 'git-graph-lane-1' }] + }) + + expect(markup).toContain('d="M 11 0 V 24"') + expect(markup).toContain('cx="22" cy="12" r="3.5" fill="var(--git-graph-lane-2)"') + }) + + it('retains a lane in the rendered production model through an unrelated root', () => { + const rows = buildGitHistoryViewModels([ + item('merge', ['root', 'other']), + item('root', []), + item('other', ['base']), + item('base', []) + ]) + expect(renderGraph(rows[1]!)).toContain('d="M 22 0 V 6') + expect(renderGraph(rows[2]!)).toContain('d="M 11 0 V 12"') + }) + + it('connects duplicate lanes to one root while shifting the other history', () => { + const rows = buildGitHistoryViewModels([ + item('merge', ['step', 'other', 'root']), + item('step', ['root']), + item('root', []), + item('other', []) + ]) + const markup = renderGraph(rows[2]!) + + expect(markup).toContain('d="M 22 0 V 6') + expect(markup).toContain('d="M 33 0 A 11 11 0 0 1 22 12 H 11"') + expect(markup).toContain('cx="11" cy="12" r="3.5" fill="var(--git-graph-lane-1)"') + expect(renderGraph(rows[3]!)).toContain('d="M 11 0 V 12"') + }) + + it('keeps the normal parent edge when history is truncated before the parent', () => { + const [row] = buildGitHistoryViewModels([item('tip', ['outside-page'])]) + expect(renderGraph(row!)).toContain('d="M 11 12 V 24"') + }) + + it('keeps incoming and outgoing boundary colors and rings', () => { + const currentRef = { id: 'refs/heads/feature', name: 'feature', revision: 'tip' } + const remoteRef = { + id: 'refs/remotes/origin/feature', + name: 'origin/feature', + revision: 'remote' + } + const rows = buildGitHistoryViewModels( + [item('tip', ['base']), item('remote', ['base']), item('base', [])], + buildDefaultGitHistoryColorMap({ currentRef, remoteRef }), + currentRef, + remoteRef, + undefined, + true, + true, + 'base' + ) + + for (const [kind, color] of [ + ['incoming-changes', 'git-graph-remote-ref'], + ['outgoing-changes', 'git-graph-ref'] + ]) { + const boundary = rows.find((row) => row.kind === kind)! + const markup = renderGraph(boundary) + expect(markup).toContain(`fill="var(--${color})"`) + expect(markup).toContain('stroke-dasharray="4 2"') + expect(markup).toContain('V 24"') + } + }) +}) diff --git a/src/renderer/src/components/right-sidebar/source-control/sync/git-history-graph-svg.tsx b/src/renderer/src/components/right-sidebar/source-control/sync/git-history-graph-svg.tsx index 49053d7ecfe..395cc68cbe9 100644 --- a/src/renderer/src/components/right-sidebar/source-control/sync/git-history-graph-svg.tsx +++ b/src/renderer/src/components/right-sidebar/source-control/sync/git-history-graph-svg.tsx @@ -1,6 +1,7 @@ import React from 'react' import type { GitHistoryGraphColorId } from '../../../../../../shared/git-history' import { + getGitHistoryItemColor, getGitHistoryItemLaneIndex, getGitHistoryMergeParentLaneIndex, type GitHistoryItemViewModel @@ -47,12 +48,7 @@ export function GitHistoryGraphSvg({ const outputSwimlanes = viewModel.outputSwimlanes const inputIndex = inputSwimlanes.findIndex((node) => node.id === historyItem.id) const circleIndex = getGitHistoryItemLaneIndex(viewModel) - const circleColor = - circleIndex < outputSwimlanes.length - ? outputSwimlanes[circleIndex]!.color - : circleIndex < inputSwimlanes.length - ? inputSwimlanes[circleIndex]!.color - : 'git-graph-ref' + const circleColor = getGitHistoryItemColor(viewModel) const paths: React.JSX.Element[] = [] let outputSwimlaneIndex = 0 @@ -72,7 +68,7 @@ export function GitHistoryGraphSvg({ ].join(' ')} /> ) - } else { + } else if (historyItem.parentIds.length > 0) { outputSwimlaneIndex += 1 } continue diff --git a/src/shared/git-history-graph.test.ts b/src/shared/git-history-graph.test.ts index dce6e789937..79bd226f5a9 100644 --- a/src/shared/git-history-graph.test.ts +++ b/src/shared/git-history-graph.test.ts @@ -116,6 +116,42 @@ describe('git history graph model', () => { expect(viewModels[0]!.outputSwimlanes[1]!.color).toBe(GIT_HISTORY_REMOTE_REF_COLOR) }) + it.each([ + ['A', 'V'], + ['V', 'A'] + ])('keeps other lanes when parents are %s then %s', (...parents) => { + const rootRef = branch('unrelated', 'V') + const viewModels = buildGitHistoryViewModels( + [item('M', parents), item('V', [], [rootRef]), item('A', ['B']), item('B', [])], + new Map([[rootRef.id, undefined]]) + ) + const root = viewModels[1]! + const rootLane = root.inputSwimlanes.find((lane) => lane.id === 'V')! + + expect(root.inputSwimlanes.map((lane) => lane.id)).toEqual(parents) + expect(root.outputSwimlanes.map((lane) => lane.id)).toEqual(['A']) + expect(root.historyItem.references?.[0]?.color).toBe(rootLane.color) + expect(viewModels[2]!.inputSwimlanes.map((lane) => lane.id)).toEqual(['A']) + expect(viewModels[2]!.outputSwimlanes.map((lane) => lane.id)).toEqual(['B']) + expect(viewModels[3]!.outputSwimlanes).toEqual([]) + }) + + it('ends every converging root lane while retaining another history', () => { + const rootRef = branch('root', 'V') + const rows = buildGitHistoryViewModels( + [item('M', ['A', 'C', 'V']), item('A', ['V']), item('V', [], [rootRef]), item('C', [])], + new Map([[rootRef.id, undefined]]) + ) + const root = rows[2]! + const survivingLane = root.inputSwimlanes[1]! + + expect(root.inputSwimlanes.map((lane) => lane.id)).toEqual(['V', 'C', 'V']) + expect(root.outputSwimlanes).toEqual([survivingLane]) + expect(root.historyItem.references?.[0]?.color).toBe(root.inputSwimlanes[0]!.color) + expect(rows[3]!.inputSwimlanes).toEqual([survivingLane]) + expect(rows[3]!.outputSwimlanes).toEqual([]) + }) + it('avoids building a parent index for linear history', () => { const count = 64 const reads = { count: 0 } @@ -136,7 +172,7 @@ describe('git history graph model', () => { const historyItems: GitHistoryItem[] = [] for (let index = 0; index < mergeCount; index += 1) { historyItems.push(trackedItem(`merge-${index}`, [`missing-${index}`, targetId], reads)) - // Reset swimlanes between merges so this measures parent lookup, not lane growth. + // Roots retain other lanes; growing width must not add history-item reads. historyItems.push(trackedItem(`leaf-${index}`, [], reads)) } historyItems.push(trackedItem(targetId, [], reads)) diff --git a/src/shared/git-history-graph.ts b/src/shared/git-history-graph.ts index f3c6c24ad49..89c230b5fe6 100644 --- a/src/shared/git-history-graph.ts +++ b/src/shared/git-history-graph.ts @@ -33,6 +33,25 @@ function cloneNode(node: GitHistoryGraphNode): GitHistoryGraphNode { return { id: node.id, color: node.color } } +function resolveNodeColor( + id: string, + hasParents: boolean, + inputSwimlanes: GitHistoryGraphNode[], + outputSwimlanes: GitHistoryGraphNode[] +): GitHistoryGraphColorId { + const inputIndex = inputSwimlanes.findIndex((node) => node.id === id) + const circleIndex = inputIndex !== -1 ? inputIndex : inputSwimlanes.length + // A root ends its own lane, so its output slot may now belong to another history. + if (!hasParents && inputIndex !== -1) { + return inputSwimlanes[inputIndex]!.color + } + return ( + outputSwimlanes[circleIndex]?.color ?? + inputSwimlanes[circleIndex]?.color ?? + GIT_HISTORY_REF_COLOR + ) +} + function findLastIndex(items: readonly T[], predicate: (item: T) => boolean): number { for (let index = items.length - 1; index >= 0; index -= 1) { if (predicate(items[index] as T)) { @@ -106,25 +125,25 @@ export function buildGitHistoryViewModels( let historyItemsById: Map | undefined for (const historyItem of historyItems) { - const kind = historyItem.id === currentRef?.revision ? 'HEAD' : 'node' + const historyItemId = historyItem.id + const firstParentId = historyItem.parentIds[0] + const kind = historyItemId === currentRef?.revision ? 'HEAD' : 'node' const inputSwimlanes = (viewModels.at(-1)?.outputSwimlanes ?? []).map(cloneNode) const outputSwimlanes: GitHistoryGraphNode[] = [] let firstParentAdded = false - if (historyItem.parentIds.length > 0) { - for (const node of inputSwimlanes) { - if (node.id === historyItem.id) { - if (!firstParentAdded) { - outputSwimlanes.push({ - id: historyItem.parentIds[0]!, - color: getLabelColorIdentifier(historyItem, colorMap) ?? node.color - }) - firstParentAdded = true - } - continue + for (const node of inputSwimlanes) { + if (node.id === historyItemId) { + if (!firstParentAdded && firstParentId !== undefined) { + outputSwimlanes.push({ + id: firstParentId, + color: getLabelColorIdentifier(historyItem, colorMap) ?? node.color + }) + firstParentAdded = true } - outputSwimlanes.push(cloneNode(node)) + continue } + outputSwimlanes.push(cloneNode(node)) } for (let index = firstParentAdded ? 1 : 0; index < historyItem.parentIds.length; index += 1) { @@ -162,14 +181,12 @@ export function buildGitHistoryViewModels( .map((ref) => { let color = colorMap.get(ref.id) if (colorMap.has(ref.id) && color === undefined) { - const inputIndex = inputSwimlanes.findIndex((node) => node.id === historyItem.id) - const circleIndex = inputIndex !== -1 ? inputIndex : inputSwimlanes.length - color = - circleIndex < outputSwimlanes.length - ? outputSwimlanes[circleIndex]!.color - : circleIndex < inputSwimlanes.length - ? inputSwimlanes[circleIndex]!.color - : GIT_HISTORY_REF_COLOR + color = resolveNodeColor( + historyItemId, + firstParentId !== undefined, + inputSwimlanes, + outputSwimlanes + ) } return { ...ref, color } }) @@ -195,6 +212,15 @@ export function buildGitHistoryViewModels( return viewModels } +export function getGitHistoryItemColor(viewModel: GitHistoryItemViewModel): GitHistoryGraphColorId { + return resolveNodeColor( + viewModel.historyItem.id, + viewModel.historyItem.parentIds.length > 0, + viewModel.inputSwimlanes, + viewModel.outputSwimlanes + ) +} + export function getGitHistoryItemLaneIndex(viewModel: GitHistoryItemViewModel): number { const inputIndex = viewModel.inputSwimlanes.findIndex( (node) => node.id === viewModel.historyItem.id diff --git a/src/shared/git-history-real-repository.test.ts b/src/shared/git-history-real-repository.test.ts new file mode 100644 index 00000000000..f627e34178d --- /dev/null +++ b/src/shared/git-history-real-repository.test.ts @@ -0,0 +1,152 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { mkdtemp, mkdir, readFile, realpath, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { runProcess } from '@orca/process-host' +import { resolveExecutableCommand } from './node-cli-command-resolution' +import { loadGitHistoryFromExecutor } from './git-history' +import { buildDefaultGitHistoryColorMap, buildGitHistoryViewModels } from './git-history-graph' + +const repositories: string[] = [] +const gitBinary = resolveExecutableCommand(process.env.ORCA_TEST_GIT_BINARY ?? 'git') + +afterEach(async () => { + await Promise.all( + repositories.splice(0).map((repo) => rm(repo, { recursive: true, force: true })) + ) +}) + +async function createRepository(inheritedEnv: NodeJS.ProcessEnv = process.env) { + if (!gitBinary) { + throw new Error('Git is required for the real history regression') + } + const repo = await mkdtemp(join(tmpdir(), 'orca-history-roots-')) + repositories.push(repo) + const emptyConfig = join(repo, 'empty-git-config') + const emptyHooks = join(repo, 'empty-hooks') + await writeFile(emptyConfig, '') + await mkdir(emptyHooks) + const fixtureEnv = Object.fromEntries( + Object.entries(inheritedEnv).filter(([key]) => !key.toUpperCase().startsWith('GIT_')) + ) + const git = async (args: string[], cwd = repo, date = '2000-01-04T00:00:00Z') => { + const result = await runProcess({ + program: gitBinary, + args, + cwd, + env: { + ...fixtureEnv, + GIT_CONFIG_NOSYSTEM: '1', + GIT_CONFIG_GLOBAL: emptyConfig, + GIT_AUTHOR_DATE: date, + GIT_COMMITTER_DATE: date + }, + timeoutMs: 10000, + maxOutputBytes: 1024 * 1024 + }) + if (result.code !== 0 || result.timedOut || result.outputTruncated) { + throw new Error(`git ${args.join(' ')} failed: ${result.stderr}`) + } + return { stdout: result.stdout } + } + await git(['init', '--quiet']) + await git(['config', 'user.name', 'Graph Fixture']) + await git(['config', 'user.email', 'graph-fixture@example.test']) + await git(['config', 'commit.gpgsign', 'false']) + await git(['config', 'core.hooksPath', emptyHooks]) + await git(['checkout', '-b', 'first']) + await git(['commit', '--quiet', '--allow-empty', '-m', 'B'], repo, '2000-01-01T00:00:00Z') + await git(['commit', '--quiet', '--allow-empty', '-m', 'A'], repo, '2000-01-02T00:00:00Z') + await git(['checkout', '--orphan', 'unrelated']) + await git(['commit', '--quiet', '--allow-empty', '-m', 'V'], repo, '2000-01-03T00:00:00Z') + return { repo, git } +} + +describe('real unrelated Git histories', () => { + it.each(['repository pointers', 'config overrides'] as const)( + 'ignores inherited Git %s when creating a fixture', + async (contamination) => { + const cleanEnv = Object.fromEntries( + Object.entries(process.env).filter(([key]) => !key.toUpperCase().startsWith('GIT_')) + ) + const caller = await createRepository(cleanEnv) + await caller.git(['config', 'user.name', 'Caller Repository']) + const protectedPaths = ['HEAD', 'config', 'index'].map((name) => + join(caller.repo, '.git', name) + ) + const before = await Promise.all(protectedPaths.map((path) => readFile(path))) + const refsBefore = (await caller.git(['show-ref'])).stdout + const inheritedGitEnv = + contamination === 'repository pointers' + ? { + GIT_DIR: join(caller.repo, '.git'), + GIT_WORK_TREE: caller.repo, + GIT_COMMON_DIR: join(caller.repo, '.git'), + GIT_INDEX_FILE: join(caller.repo, '.git', 'index'), + GIT_OBJECT_DIRECTORY: join(caller.repo, '.git', 'objects'), + GIT_ALTERNATE_OBJECT_DIRECTORIES: join(caller.repo, '.git', 'objects') + } + : { + GIT_CONFIG: join(caller.repo, '.git', 'config'), + GIT_CONFIG_PARAMETERS: "'user.name=Injected Author'", + GIT_CONFIG_COUNT: '1', + GIT_CONFIG_KEY_0: 'core.bare', + GIT_CONFIG_VALUE_0: 'true' + } + const fixture = await createRepository({ ...cleanEnv, ...inheritedGitEnv }) + + expect( + await realpath((await fixture.git(['rev-parse', '--show-toplevel'])).stdout.trim()) + ).toBe(await realpath(fixture.repo)) + expect((await fixture.git(['log', '-1', '--format=%an <%ae>'])).stdout.trim()).toBe( + 'Graph Fixture ' + ) + expect(await Promise.all(protectedPaths.map((path) => readFile(path)))).toEqual(before) + expect((await caller.git(['show-ref'])).stdout).toBe(refsBefore) + } + ) + + it.each([false, true])( + 'keeps the other history when first parent is a root: %s', + async (firstParentIsRoot) => { + const { repo, git } = await createRepository() + if (!firstParentIsRoot) { + await git(['checkout', 'first']) + } + await git([ + 'merge', + '--quiet', + '--no-ff', + '--allow-unrelated-histories', + '-m', + 'M', + firstParentIsRoot ? 'first' : 'unrelated' + ]) + const history = await loadGitHistoryFromExecutor(git, repo, { limit: 10 }) + const rows = buildGitHistoryViewModels( + history.items, + buildDefaultGitHistoryColorMap(history), + history.currentRef + ) + const terminatingRoot = rows.find( + (row) => row.historyItem.subject === (firstParentIsRoot ? 'B' : 'V') + )! + const surviving = history.items.find( + (item) => item.subject === (firstParentIsRoot ? 'V' : 'A') + )! + + expect(history.items).toHaveLength(4) + expect(history.items[0]!.parentIds).toHaveLength(2) + expect(terminatingRoot.historyItem.parentIds).toEqual([]) + expect(terminatingRoot.inputSwimlanes).toHaveLength(2) + expect(terminatingRoot.outputSwimlanes.map((lane) => lane.id)).toEqual([surviving.id]) + const nextRow = rows[rows.indexOf(terminatingRoot) + 1]! + expect(nextRow.inputSwimlanes.map((lane) => lane.id)).toEqual([surviving.id]) + expect(rows.at(-1)!.outputSwimlanes).toEqual([]) + + const truncated = await loadGitHistoryFromExecutor(git, repo, { limit: 2 }) + expect(truncated.hasMore).toBe(true) + expect(truncated.items).toHaveLength(2) + } + ) +})