From debf4affe7ed2dcbb4ad9fdad94a87bd92745831 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Wed, 5 Aug 2026 14:46:00 -0700 Subject: [PATCH] Display total lines of code change in branch header (#12771) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add branch line total chip to source control header Display the total lines added and removed across a branch from its fork point, measured via `git diff `. Only computed when the chip is visible (request gate on merge base OID), with 500ms soft deadline to protect status latency and 15s hard timeout. Deduplicated across concurrent pollers and cached alongside line stats. Omitted on failure — always shows exact or nothing, never a partial estimate. Updates throughout the stack: native git status, relay, renderer store/API, and UI components. * Pin branch line total to app locale Format line counts using the app's configured locale instead of the system locale, ensuring consistent cross-platform display and test reliability. * test: wait for coalescer joins instead of fixed sleep Hold the diff until the second status pass actually takes the branch-total coalescer lease instead of using a fixed 400ms sleep. Fixes timing-dependent flakiness on slow machines. --- src/main/git/git-status-read-lease-owner.ts | 104 +--- ...us-branch-line-total-exec-contract.test.ts | 330 ++++++++++++ .../status-branch-line-total-real-git.test.ts | 203 ++++++++ ...tus-branch-line-total-relay-parity.test.ts | 172 +++++++ src/main/git/status.ts | 68 ++- src/main/ipc/filesystem.ts | 4 + .../providers/git-provider-status-options.ts | 2 + src/main/providers/ssh-git-provider.ts | 7 +- src/main/runtime/rpc/methods/git-params.ts | 4 +- src/main/runtime/rpc/methods/git.ts | 4 + src/preload/api-types.ts | 2 + src/preload/index.ts | 1 + src/relay/git-handler-status-ops.ts | 33 +- src/relay/git-handler.ts | 2 + .../git-status-branch-line-total.test.ts | 475 ++++++++++++++++++ src/relay/git-status-branch-line-total.ts | 44 ++ .../SourceControl.branch-line-total.test.tsx | 340 +++++++++++++ ...SourceControl.open-file-highlight.test.tsx | 1 + .../SourceControl.preview-open.test.tsx | 1 + .../right-sidebar/SourceControl.tsx | 27 + .../SourceControl.virtual-file-list.test.tsx | 1 + .../branch-line-total-request-gate.test.ts | 52 ++ .../branch-line-total-request-gate.ts | 23 + ...t-status-refresh-branch-line-total.test.ts | 224 +++++++++ .../right-sidebar/git-status-refresh.ts | 23 +- ...source-control-branch-context-row.test.tsx | 142 ++++++ .../source-control-branch-context-row.tsx | 31 +- .../source-control-branch-context-stats.ts | 12 +- .../source-control-branch-line-total-chip.tsx | 74 +++ ...e-control-header-toolbar-identity.test.tsx | 16 + .../source-control-header-toolbar.tsx | 6 +- src/renderer/src/i18n/locales/en.json | 11 + ...ntime-git-client-branch-line-total.test.ts | 133 +++++ .../src/runtime/runtime-git-client.ts | 10 +- .../slices/editor-branch-line-total.test.ts | 225 +++++++++ src/renderer/src/store/slices/editor.ts | 28 ++ .../src/store/slices/worktrees.test.ts | 21 + src/renderer/src/store/slices/worktrees.ts | 5 + src/renderer/src/web/web-preload-api.test.ts | 93 ++++ src/renderer/src/web/web-preload-api.ts | 4 +- ...it-branch-line-total-soft-deadline.test.ts | 62 +++ src/shared/git-branch-line-total.test.ts | 434 ++++++++++++++++ src/shared/git-branch-line-total.ts | 214 ++++++++ ...git-status-branch-line-total-cache.test.ts | 236 +++++++++ src/shared/git-status-line-stats-cache.ts | 220 +++++--- .../git-status-line-stats-write-token.ts | 69 +++ src/shared/git-status-read-lease-owner.ts | 101 ++++ src/shared/git-status-types.ts | 12 + 48 files changed, 4112 insertions(+), 194 deletions(-) create mode 100644 src/main/git/status-branch-line-total-exec-contract.test.ts create mode 100644 src/main/git/status-branch-line-total-real-git.test.ts create mode 100644 src/main/git/status-branch-line-total-relay-parity.test.ts create mode 100644 src/relay/git-status-branch-line-total.test.ts create mode 100644 src/relay/git-status-branch-line-total.ts create mode 100644 src/renderer/src/components/right-sidebar/SourceControl.branch-line-total.test.tsx create mode 100644 src/renderer/src/components/right-sidebar/branch-line-total-request-gate.test.ts create mode 100644 src/renderer/src/components/right-sidebar/branch-line-total-request-gate.ts create mode 100644 src/renderer/src/components/right-sidebar/git-status-refresh-branch-line-total.test.ts create mode 100644 src/renderer/src/components/right-sidebar/source-control-branch-line-total-chip.tsx create mode 100644 src/renderer/src/runtime/runtime-git-client-branch-line-total.test.ts create mode 100644 src/renderer/src/store/slices/editor-branch-line-total.test.ts create mode 100644 src/shared/git-branch-line-total-soft-deadline.test.ts create mode 100644 src/shared/git-branch-line-total.test.ts create mode 100644 src/shared/git-branch-line-total.ts create mode 100644 src/shared/git-status-branch-line-total-cache.test.ts create mode 100644 src/shared/git-status-line-stats-write-token.ts create mode 100644 src/shared/git-status-read-lease-owner.ts diff --git a/src/main/git/git-status-read-lease-owner.ts b/src/main/git/git-status-read-lease-owner.ts index a0012da7d..eeae5eedf 100644 --- a/src/main/git/git-status-read-lease-owner.ts +++ b/src/main/git/git-status-read-lease-owner.ts @@ -1,101 +1,3 @@ -type StatusReadEntry = { - controller: AbortController - promise: Promise - liveLeases: number - settled: boolean -} - -function getAbortReason(signal: AbortSignal): unknown { - try { - signal.throwIfAborted() - } catch (error) { - return error - } - return new DOMException('This operation was aborted', 'AbortError') -} - -export class GitStatusReadLeaseOwner { - private readonly entries = new Map>() - - lease( - key: string, - signal: AbortSignal | undefined, - load: (sharedSignal: AbortSignal) => Promise - ): Promise { - if (signal?.aborted) { - return Promise.reject(getAbortReason(signal)) - } - - let entry = this.entries.get(key) - if (!entry) { - const controller = new AbortController() - const promise = load(controller.signal) - const createdEntry = { controller, promise, liveLeases: 0, settled: false } - entry = createdEntry - this.entries.set(key, createdEntry) - void promise.then( - () => this.settle(key, createdEntry), - () => this.settle(key, createdEntry) - ) - } - - entry.liveLeases += 1 - return this.createLease(key, entry, signal) - } - - invalidate(): void { - this.entries.clear() - } - - private createLease( - key: string, - entry: StatusReadEntry, - signal: AbortSignal | undefined - ): Promise { - return new Promise((resolve, reject) => { - let active = true - const release = (abortReason?: unknown): boolean => { - if (!active) { - return false - } - active = false - signal?.removeEventListener('abort', onAbort) - entry.liveLeases -= 1 - if (abortReason !== undefined && entry.liveLeases === 0 && !entry.settled) { - if (this.entries.get(key) === entry) { - this.entries.delete(key) - } - entry.controller.abort(abortReason) - } - return true - } - const onAbort = (): void => { - const reason = getAbortReason(signal!) - if (release(reason)) { - reject(reason) - } - } - - signal?.addEventListener('abort', onAbort, { once: true }) - void entry.promise.then( - (value) => { - if (release()) { - resolve(value) - } - }, - (error: unknown) => { - if (release()) { - reject(error) - } - } - ) - }) - } - - private settle(key: string, entry: StatusReadEntry): void { - entry.settled = true - if (this.entries.get(key) === entry) { - this.entries.delete(key) - } - } -} +// Why: the class moved to src/shared so the relay host can coalesce the same +// reads; this re-export keeps main's import path (and its tests) stable. +export { GitStatusReadLeaseOwner } from '../../shared/git-status-read-lease-owner' diff --git a/src/main/git/status-branch-line-total-exec-contract.test.ts b/src/main/git/status-branch-line-total-exec-contract.test.ts new file mode 100644 index 000000000..13ece36da --- /dev/null +++ b/src/main/git/status-branch-line-total-exec-contract.test.ts @@ -0,0 +1,330 @@ +import { execFileSync } from 'node:child_process' +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import * as path from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + buildGitBranchLineTotalDiffArgs, + invalidateGitBranchLineTotalInFlight +} from '../../shared/git-branch-line-total' + +import type * as BranchLineTotal from '../../shared/git-branch-line-total' +import type * as GitRunner from './runner' + +// Why: real git, spied argv. Every exec is recorded so the performance contract +// ("no ranged diff unless asked", "one exec for concurrent callers") can be +// asserted on the actual command list rather than on a stubbed return value. +const { gitExecCalls, execHooks, coalescerJoins } = vi.hoisted(() => ({ + gitExecCalls: [] as string[][], + execHooks: { beforeExec: undefined as undefined | ((args: string[]) => Promise | void) }, + coalescerJoins: { count: 0, onJoin: undefined as undefined | (() => void) } +})) + +vi.mock('../../shared/git-branch-line-total', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + computeGitBranchLineTotal: ( + input: Parameters[0] + ) => { + // The lease is taken synchronously inside, so counting after the call + // means a join is observable the instant it has happened. + const total = actual.computeGitBranchLineTotal(input) + coalescerJoins.count += 1 + coalescerJoins.onJoin?.() + return total + } + } +}) + +vi.mock('./runner', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + gitExecFileAsync: async (...args: Parameters) => { + gitExecCalls.push(args[0]) + await execHooks.beforeExec?.(args[0]) + return actual.gitExecFileAsync(...args) + }, + gitExecFileAsyncBuffer: async ( + ...args: Parameters + ) => { + gitExecCalls.push(args[0]) + return actual.gitExecFileAsyncBuffer(...args) + }, + gitStreamStdout: async (...args: Parameters) => { + gitExecCalls.push(args[0]) + return actual.gitStreamStdout(...args) + } + } +}) + +import { + clearEffectiveUpstreamStatusCacheForTests, + clearSubmodulePathsCacheForTests, + getStatus, + invalidateGitReadCaches +} from './status' + +const BOGUS_MERGE_BASE = 'deadbeef'.repeat(5) +const tempRoots: string[] = [] + +function git(repo: string, args: string[]): string { + return execFileSync('git', args, { + cwd: repo, + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'] + }).trim() +} + +async function createFixtureRepo(): Promise<{ repo: string; mergeBase: string }> { + const root = await mkdtemp(path.join(tmpdir(), 'orca-branch-line-total-exec-')) + tempRoots.push(root) + const repo = path.join(root, 'repo') + execFileSync('git', ['init', '-q', repo]) + git(repo, ['config', 'user.email', 'test@example.com']) + git(repo, ['config', 'user.name', 'Test User']) + git(repo, ['config', 'commit.gpgSign', 'false']) + await write(repo, 'f.txt', 'a\nb\n') + git(repo, ['add', '-A']) + git(repo, ['commit', '-q', '-m', 'base']) + await write(repo, 'f.txt', 'a\nb\nc\n') + return { repo, mergeBase: git(repo, ['rev-parse', 'HEAD']) } +} + +async function write(repo: string, relativePath: string, contents: string): Promise { + const target = path.join(repo, relativePath) + await mkdir(path.dirname(target), { recursive: true }) + await writeFile(target, contents) +} + +/** The `mergeBase → worktree` diff, distinguished from the per-area numstats by its rev + `--`. */ +function rangedDiffCalls(): string[][] { + return gitExecCalls.filter((args) => args.includes('--numstat') && args.at(-1) === '--') +} + +function numstatCalls(): string[][] { + return gitExecCalls.filter((args) => args.includes('--numstat')) +} + +/** Resolves once `count` status passes have entered the branch-total coalescer. */ +function waitForCoalescerJoins(count: number): Promise { + if (coalescerJoins.count >= count) { + return Promise.resolve() + } + return new Promise((resolve) => { + coalescerJoins.onJoin = () => { + if (coalescerJoins.count >= count) { + coalescerJoins.onJoin = undefined + resolve() + } + } + }) +} + +function resetGitReadCaches(): void { + clearEffectiveUpstreamStatusCacheForTests() + clearSubmodulePathsCacheForTests() + invalidateGitReadCaches() + invalidateGitBranchLineTotalInFlight() +} + +beforeEach(() => { + gitExecCalls.length = 0 + execHooks.beforeExec = undefined + coalescerJoins.count = 0 + coalescerJoins.onJoin = undefined + resetGitReadCaches() +}) + +afterEach(async () => { + execHooks.beforeExec = undefined + coalescerJoins.onJoin = undefined + resetGitReadCaches() + await Promise.all(tempRoots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) +}) + +describe('branch line total completeness', () => { + it('omits the total when the status listing hit its limit', async () => { + const { repo, mergeBase } = await createFixtureRepo() + await write(repo, 'u1.txt', 'a\n') + await write(repo, 'u2.txt', 'b\n') + await write(repo, 'u3.txt', 'c\n') + + const result = await getStatus(repo, { branchLineTotalMergeBase: mergeBase, limit: 1 }) + + expect(result.didHitLimit).toBe(true) + expect(result.branchLineTotal).toBeUndefined() + expect(rangedDiffCalls()).toEqual([]) + }) + + it('omits the total when the ranged numstat fails, rather than reporting zero', async () => { + const { repo } = await createFixtureRepo() + + const result = await getStatus(repo, { branchLineTotalMergeBase: BOGUS_MERGE_BASE }) + + expect(result.entries.length).toBeGreaterThan(0) + expect(result.branchLineTotal).toBeUndefined() + // The diff really was attempted, so this is a failure path and not a gate. + expect(rangedDiffCalls()).toHaveLength(1) + }) + + it('omits the total for a directory that is not a git worktree', async () => { + const root = await mkdtemp(path.join(tmpdir(), 'orca-branch-line-total-folder-')) + tempRoots.push(root) + + const result = await getStatus(root, { branchLineTotalMergeBase: BOGUS_MERGE_BASE }) + + expect(result.branchLineTotal).toBeUndefined() + expect(rangedDiffCalls()).toEqual([]) + }) + + it('rejects with an AbortError when the pass is cancelled mid-diff', async () => { + const { repo, mergeBase } = await createFixtureRepo() + const controller = new AbortController() + execHooks.beforeExec = (args) => { + if (args.includes('--numstat') && args.at(-1) === '--') { + controller.abort() + } + } + + await expect( + getStatus(repo, { branchLineTotalMergeBase: mergeBase, signal: controller.signal }) + ).rejects.toMatchObject({ name: 'AbortError' }) + // The pass had already streamed status and reached the ranged diff, so this + // is a cancellation in flight rather than a pre-flight rejection. + expect(rangedDiffCalls()).toHaveLength(1) + }) +}) + +describe('branch line total merge-base gate', () => { + it('runs no ranged diff and returns no total without a merge base', async () => { + const { repo } = await createFixtureRepo() + + const result = await getStatus(repo) + + expect(result.branchLineTotal).toBeUndefined() + expect(rangedDiffCalls()).toEqual([]) + }) + + it.each(['--upload-pack=x', 'HEAD', '', 'origin/main', '-M', 'A1B2C3D'])( + 'never spawns a ranged diff for a merge base of %j', + async (branchLineTotalMergeBase) => { + const { repo } = await createFixtureRepo() + + const result = await getStatus(repo, { branchLineTotalMergeBase }) + + expect(result.branchLineTotal).toBeUndefined() + expect(rangedDiffCalls()).toEqual([]) + expect(gitExecCalls).not.toContainEqual( + buildGitBranchLineTotalDiffArgs(branchLineTotalMergeBase) + ) + } + ) + + it('never leaks a flag-shaped merge base into any git argv', async () => { + const { repo } = await createFixtureRepo() + + await getStatus(repo, { branchLineTotalMergeBase: '--upload-pack=/tmp/evil' }) + + for (const args of gitExecCalls) { + expect(args).not.toContain('--upload-pack=/tmp/evil') + } + }) +}) + +describe('branch line total exec budget', () => { + it('costs exactly one extra exec versus the same pass with the chip hidden', async () => { + const { repo, mergeBase } = await createFixtureRepo() + + await getStatus(repo) + const withoutChip = gitExecCalls.map((args) => args.join(' ')) + + gitExecCalls.length = 0 + resetGitReadCaches() + await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) + const withChip = gitExecCalls.map((args) => args.join(' ')) + + expect(withChip.filter((args) => !args.endsWith(`${mergeBase} --`))).toEqual(withoutChip) + expect(withChip).toHaveLength(withoutChip.length + 1) + }) + + it('shares one ranged diff between two concurrent identical status passes', async () => { + const { repo, mergeBase } = await createFixtureRepo() + + const [first, second] = await Promise.all([ + getStatus(repo, { branchLineTotalMergeBase: mergeBase }), + getStatus(repo, { branchLineTotalMergeBase: mergeBase }) + ]) + + expect(first.branchLineTotal).toEqual({ added: 1, removed: 0, mergeBase }) + expect(second.branchLineTotal).toEqual(first.branchLineTotal) + expect(rangedDiffCalls()).toHaveLength(1) + }) + + it('shares one ranged diff across status passes the status lease cannot merge', async () => { + const { repo, mergeBase } = await createFixtureRepo() + // Why: `limit` is part of the status read key, so these two passes each run + // their own `git status`; only the branch-total coalescer can merge the diff. + // Hold the first diff exactly until the second pass has taken the lease: a + // fixed sleep would either release too early or, on a slow machine, outrun + // GIT_BRANCH_LINE_TOTAL_SOFT_DEADLINE_MS and publish without an exact total. + execHooks.beforeExec = async (args) => { + if (args.includes('--numstat') && args.at(-1) === '--') { + await waitForCoalescerJoins(2) + } + } + + const [first, second] = await Promise.all([ + getStatus(repo, { branchLineTotalMergeBase: mergeBase, limit: 0 }), + getStatus(repo, { branchLineTotalMergeBase: mergeBase, limit: 4096 }) + ]) + + expect(first.branchLineTotal).toEqual({ added: 1, removed: 0, mergeBase }) + expect(second.branchLineTotal).toEqual(first.branchLineTotal) + // Proves the saving came from the branch-total coalescer: both passes really + // ran their own `git status`, so the status lease merged nothing. + expect(gitExecCalls.filter((args) => args.includes('status'))).toHaveLength(2) + expect(rangedDiffCalls()).toHaveLength(1) + }) + + it('reuses the cached total when the pass reuses cached line stats', async () => { + const { repo, mergeBase } = await createFixtureRepo() + + const first = await getStatus(repo, { + branchLineTotalMergeBase: mergeBase, + reuseLineStats: true + }) + expect(rangedDiffCalls()).toHaveLength(1) + expect(numstatCalls()).toHaveLength(2) + + gitExecCalls.length = 0 + invalidateGitBranchLineTotalInFlight() + const second = await getStatus(repo, { + branchLineTotalMergeBase: mergeBase, + reuseLineStats: true + }) + + expect(second.branchLineTotal).toEqual(first.branchLineTotal) + expect(second.branchLineTotal).toEqual({ added: 1, removed: 0, mergeBase }) + expect(numstatCalls()).toEqual([]) + }) + + it('recomputes rather than reusing a total measured against another merge base', async () => { + const { repo, mergeBase } = await createFixtureRepo() + git(repo, ['add', '-A']) + git(repo, ['commit', '-q', '-m', 'second']) + const laterMergeBase = git(repo, ['rev-parse', 'HEAD']) + + await getStatus(repo, { branchLineTotalMergeBase: mergeBase, reuseLineStats: true }) + gitExecCalls.length = 0 + invalidateGitBranchLineTotalInFlight() + const result = await getStatus(repo, { + branchLineTotalMergeBase: laterMergeBase, + reuseLineStats: true + }) + + expect(result.branchLineTotal).toEqual({ added: 0, removed: 0, mergeBase: laterMergeBase }) + expect(rangedDiffCalls()).toHaveLength(1) + }) +}) diff --git a/src/main/git/status-branch-line-total-real-git.test.ts b/src/main/git/status-branch-line-total-real-git.test.ts new file mode 100644 index 000000000..80426c76e --- /dev/null +++ b/src/main/git/status-branch-line-total-real-git.test.ts @@ -0,0 +1,203 @@ +import { execFileSync } from 'node:child_process' +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import * as path from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { invalidateGitBranchLineTotalInFlight } from '../../shared/git-branch-line-total' +import { getStatus, invalidateGitReadCaches } from './status' + +const tempRoots: string[] = [] + +function git(repo: string, args: string[]): string { + return execFileSync('git', args, { + cwd: repo, + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'] + }).trim() +} + +async function createFixtureRepo(): Promise { + const root = await mkdtemp(path.join(tmpdir(), 'orca-branch-line-total-')) + tempRoots.push(root) + const repo = path.join(root, 'repo') + execFileSync('git', ['init', '-q', repo]) + git(repo, ['config', 'user.email', 'test@example.com']) + git(repo, ['config', 'user.name', 'Test User']) + git(repo, ['config', 'commit.gpgSign', 'false']) + return repo +} + +async function write(repo: string, relativePath: string, contents: string | Buffer): Promise { + const target = path.join(repo, relativePath) + await mkdir(path.dirname(target), { recursive: true }) + await writeFile(target, contents) +} + +/** The forked-from commit the chip measures against, as branch compare would resolve it. */ +function commitAll(repo: string, message: string): string { + git(repo, ['add', '-A']) + git(repo, ['commit', '-q', '-m', message]) + return git(repo, ['rev-parse', 'HEAD']) +} + +/** What a plain `git diff ` reports, i.e. the number the chip must not disagree with. */ +function rangedDiffTotal(repo: string, mergeBase: string): { added: number; removed: number } { + let added = 0 + let removed = 0 + for (const line of git(repo, ['diff', '--numstat', '-M', mergeBase, '--']).split(/\r?\n/)) { + if (!line) { + continue + } + const [rawAdded, rawRemoved] = line.split('\t') + added += rawAdded === '-' ? 0 : Number.parseInt(rawAdded ?? '0', 10) + removed += rawRemoved === '-' ? 0 : Number.parseInt(rawRemoved ?? '0', 10) + } + return { added, removed } +} + +beforeEach(() => { + invalidateGitReadCaches() + invalidateGitBranchLineTotalInFlight() +}) + +afterEach(async () => { + invalidateGitReadCaches() + invalidateGitBranchLineTotalInFlight() + await Promise.all(tempRoots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) +}) + +describe('branch line total against a real repository', () => { + it('measures a partially staged line once, not once per status area', async () => { + const repo = await createFixtureRepo() + await write(repo, 'f.txt', 'line1\n') + const mergeBase = commitAll(repo, 'base') + // Stage "+foo", then edit that same line to "bar" without staging it: the + // per-area rows read +1/-0 staged and +1/-1 unstaged, summing to +2/-1. + await write(repo, 'f.txt', 'line1\nfoo\n') + git(repo, ['add', 'f.txt']) + await write(repo, 'f.txt', 'line1\nbar\n') + + const result = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) + + expect(rangedDiffTotal(repo, mergeBase)).toEqual({ added: 1, removed: 0 }) + expect(result.branchLineTotal).toEqual({ added: 1, removed: 0, mergeBase }) + expect(result.entries.map((entry) => [entry.area, entry.added, entry.removed])).toEqual([ + ['staged', 1, 0], + ['unstaged', 1, 1] + ]) + }) + + it('nets a line added in a branch commit and deleted in the working tree to zero', async () => { + const repo = await createFixtureRepo() + await write(repo, 'f.txt', 'a\n') + const mergeBase = commitAll(repo, 'base') + await write(repo, 'f.txt', 'a\nb\n') + commitAll(repo, 'add b') + await write(repo, 'f.txt', 'a\n') + + const result = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) + + expect(result.branchLineTotal).toEqual({ added: 0, removed: 0, mergeBase }) + }) + + it('keeps the total across a commit and updates it on the next status call after an edit', async () => { + const repo = await createFixtureRepo() + await write(repo, 'f.txt', 'a\n') + const mergeBase = commitAll(repo, 'base') + await write(repo, 'f.txt', 'a\nb\nc\n') + + const beforeCommit = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) + expect(beforeCommit.branchLineTotal).toEqual({ added: 2, removed: 0, mergeBase }) + + commitAll(repo, 'commit the edit') + const afterCommit = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) + expect(afterCommit.branchLineTotal).toEqual({ added: 2, removed: 0, mergeBase }) + + await write(repo, 'f.txt', 'a\nb\nc\nd\n') + const afterSecondEdit = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) + expect(afterSecondEdit.branchLineTotal).toEqual({ added: 3, removed: 0, mergeBase }) + }) + + it('counts a rename across the commit boundary once, using the post-rename path', async () => { + const repo = await createFixtureRepo() + await write(repo, 'old.txt', 'a\nb\nc\nd\ne\n') + const mergeBase = commitAll(repo, 'base') + git(repo, ['mv', 'old.txt', 'new.txt']) + commitAll(repo, 'rename') + await write(repo, 'new.txt', 'a\nb\nc\nd\ne\nf\n') + + const result = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) + + expect(rangedDiffTotal(repo, mergeBase)).toEqual({ added: 1, removed: 0 }) + expect(result.branchLineTotal).toEqual({ added: 1, removed: 0, mergeBase }) + }) + + it('counts an untracked-only branch from the untracked file contents', async () => { + const repo = await createFixtureRepo() + await write(repo, 'kept.txt', 'a\n') + const mergeBase = commitAll(repo, 'base') + await write(repo, 'src/new.ts', 'one\ntwo\nthree\n') + + const result = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) + + expect(result.entries.map((entry) => entry.area)).toEqual(['untracked']) + expect(result.branchLineTotal).toEqual({ added: 3, removed: 0, mergeBase }) + }) + + it('excludes a binary-only change, matching numstat reporting it as "-"', async () => { + const repo = await createFixtureRepo() + await write(repo, 'blob.bin', Buffer.from([0, 1, 2, 3, 10, 65, 66])) + const mergeBase = commitAll(repo, 'base') + await write(repo, 'blob.bin', Buffer.from([0, 9, 10, 11, 12, 67, 68, 69, 70])) + + const result = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) + + expect(git(repo, ['diff', '--numstat', mergeBase, '--'])).toMatch(/^-\t-\t/) + expect(result.branchLineTotal).toEqual({ added: 0, removed: 0, mergeBase }) + }) + + it('contributes nothing for a pure rename', async () => { + const repo = await createFixtureRepo() + await write(repo, 'f.txt', 'a\nb\nc\nd\n') + const mergeBase = commitAll(repo, 'base') + git(repo, ['mv', 'f.txt', 'g.txt']) + + const result = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) + + expect(result.branchLineTotal).toEqual({ added: 0, removed: 0, mergeBase }) + }) + + it('includes untracked additions alongside tracked range changes', async () => { + const repo = await createFixtureRepo() + await write(repo, 'f.txt', 'a\nb\n') + const mergeBase = commitAll(repo, 'base') + await write(repo, 'f.txt', 'a\nb\nc\n') + commitAll(repo, 'append c') + await write(repo, 'untracked.txt', 'u1\nu2\nu3\nu4\n') + + const result = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) + + expect(rangedDiffTotal(repo, mergeBase)).toEqual({ added: 1, removed: 0 }) + expect(result.branchLineTotal).toEqual({ added: 5, removed: 0, mergeBase }) + }) + + // Documented deviation from a pure `git diff`: the untracked half is added on + // top of the tracked range, so a deleted-then-recreated path is counted twice. + // Locked in deliberately — changing it must be a deliberate spec change. + it('counts both the deletion and the recreated untracked file for the same path', async () => { + const repo = await createFixtureRepo() + await write(repo, 'gone.txt', 'a\nb\nc\n') + const mergeBase = commitAll(repo, 'base') + git(repo, ['rm', '-q', 'gone.txt']) + commitAll(repo, 'delete gone.txt') + await write(repo, 'gone.txt', 'n1\nn2\n') + + const result = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) + + expect(result.entries.map((entry) => [entry.area, entry.path])).toEqual([ + ['untracked', 'gone.txt'] + ]) + expect(rangedDiffTotal(repo, mergeBase)).toEqual({ added: 0, removed: 3 }) + expect(result.branchLineTotal).toEqual({ added: 2, removed: 3, mergeBase }) + }) +}) diff --git a/src/main/git/status-branch-line-total-relay-parity.test.ts b/src/main/git/status-branch-line-total-relay-parity.test.ts new file mode 100644 index 000000000..e9f524072 --- /dev/null +++ b/src/main/git/status-branch-line-total-relay-parity.test.ts @@ -0,0 +1,172 @@ +/** + * Main and relay must publish the same `branchLineTotal` for the same fixture + * repo. Both call sites + * share `src/shared/git-branch-line-total.ts`; this is the test that catches one + * of them wiring it up differently — different flags, a different untracked + * source, a different completeness gate. + */ +import { execFile, execFileSync } from 'node:child_process' +import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import * as path from 'node:path' +import { promisify } from 'node:util' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { getStatus } from './status' +import type { GitExec } from '../../relay/git-handler-ops' +import { getStatusOp } from '../../relay/git-handler-status-ops' +import type { RelayGitStreamExec } from '../../relay/git-stdout-stream' +import { invalidateGitBranchLineTotalInFlight } from '../../shared/git-branch-line-total' +import { clearGitStatusLineStatsCache } from '../../shared/git-status-line-stats-cache' + +const execFileAsync = promisify(execFile) + +// Fork point → working tree for the fixture below: +// tracked.txt +2 (branch commit only) +// partial.txt +1 (staged +2 then one of those lines removed unstaged) +// flip.txt 0 (added in the branch commit, removed again in the worktree) +// moved.txt 0 (pure rename) +// fresh.txt +3 (untracked) +const EXPECTED_TOTAL = { added: 6, removed: 0 } +// Summing the per-area status rows instead would give this — the wrong answer +// the shared module exists to avoid. +const AREA_ROW_SUM = { added: 5, removed: 2 } + +const relayGit: GitExec = async (args, cwd, opts) => { + const { stdout, stderr } = await execFileAsync('git', args, { + cwd, + encoding: 'utf8', + ...(opts?.signal ? { signal: opts.signal } : {}), + ...(opts?.timeout ? { timeout: opts.timeout } : {}) + }) + return { stdout, stderr } +} + +const relayStreamGit: RelayGitStreamExec = async (args, cwd, options) => { + const { stdout } = await relayGit(args, cwd, { + disableOptionalLocks: options.disableOptionalLocks, + signal: options.signal + }) + return { stoppedEarly: options.onStdout(stdout) === true } +} + +function runFixtureGit(repo: string, args: string[]): string { + return execFileSync( + 'git', + [ + '-c', + 'user.email=test@test.com', + '-c', + 'user.name=Test', + '-c', + 'commit.gpgSign=false', + ...args + ], + { cwd: repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] } + ).trim() +} + +/** Returns the merge-base OID the chip is measured against. */ +async function seedParityFixture(repo: string): Promise { + execFileSync('git', ['init', '-q', repo], { stdio: 'pipe' }) + await writeFile(path.join(repo, 'tracked.txt'), 'a\nb\n') + await writeFile(path.join(repo, 'partial.txt'), 'one\ntwo\nthree\n') + await writeFile(path.join(repo, 'renamed.txt'), 'stable\n') + await writeFile(path.join(repo, 'flip.txt'), 'p\n') + runFixtureGit(repo, ['add', '.']) + runFixtureGit(repo, ['commit', '-m', 'base']) + const mergeBase = runFixtureGit(repo, ['rev-parse', 'HEAD']) + + runFixtureGit(repo, ['checkout', '-q', '-b', 'feature']) + await writeFile(path.join(repo, 'tracked.txt'), 'a\nb\nc\nd\n') + await writeFile(path.join(repo, 'flip.txt'), 'p\nq\n') + runFixtureGit(repo, ['add', '-A']) + runFixtureGit(repo, ['commit', '-m', 'branch commit']) + + runFixtureGit(repo, ['mv', 'renamed.txt', 'moved.txt']) + // Staged and unstaged hunks land on the same added lines, so an area sum + // double-counts them. + await writeFile(path.join(repo, 'partial.txt'), 'one\ntwo\nthree\nfoo\nbaz\n') + runFixtureGit(repo, ['add', 'partial.txt']) + await writeFile(path.join(repo, 'partial.txt'), 'one\ntwo\nthree\nfoo\n') + // The branch commit's line, taken back out in the worktree: net zero. + await writeFile(path.join(repo, 'flip.txt'), 'p\n') + await writeFile(path.join(repo, 'fresh.txt'), 'n1\nn2\nn3\n') + return mergeBase +} + +function sumAreaRows(entries: readonly { added?: number; removed?: number }[]): { + added: number + removed: number +} { + let added = 0 + let removed = 0 + for (const entry of entries) { + added += entry.added ?? 0 + removed += entry.removed ?? 0 + } + return { added, removed } +} + +describe('branch line total parity between main and relay', () => { + let repo: string + + beforeEach(async () => { + clearGitStatusLineStatsCache() + invalidateGitBranchLineTotalInFlight() + repo = await mkdtemp(path.join(tmpdir(), 'branch-line-total-parity-')) + }) + + afterEach(async () => { + clearGitStatusLineStatsCache() + invalidateGitBranchLineTotalInFlight() + await rm(repo, { recursive: true, force: true }) + }) + + it('produces identical totals for the same fixture repo', async () => { + const mergeBase = await seedParityFixture(repo) + + const mainStatus = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) + clearGitStatusLineStatsCache() + const relayStatus = await getStatusOp(relayGit, relayStreamGit, { + worktreePath: repo, + branchLineTotalMergeBase: mergeBase + }) + + expect(mainStatus.branchLineTotal).toEqual({ ...EXPECTED_TOTAL, mergeBase }) + expect(relayStatus.branchLineTotal).toEqual(mainStatus.branchLineTotal) + // Both sides model the same worktree, so a parity pass on a mismatched + // entry list would be meaningless. + expect(relayStatus.entries.map((entry) => entry.path).sort()).toEqual( + mainStatus.entries.map((entry) => entry.path).sort() + ) + }) + + it('agrees on a number no per-area row sum could produce', async () => { + const mergeBase = await seedParityFixture(repo) + + const mainStatus = await getStatus(repo, { branchLineTotalMergeBase: mergeBase }) + clearGitStatusLineStatsCache() + const relayStatus = await getStatusOp(relayGit, relayStreamGit, { + worktreePath: repo, + branchLineTotalMergeBase: mergeBase + }) + + expect(sumAreaRows(mainStatus.entries)).toEqual(AREA_ROW_SUM) + expect(sumAreaRows(relayStatus.entries as { added?: number; removed?: number }[])).toEqual( + AREA_ROW_SUM + ) + expect(mainStatus.branchLineTotal).not.toMatchObject(AREA_ROW_SUM) + expect(relayStatus.branchLineTotal).toEqual(mainStatus.branchLineTotal) + }) + + it('omits the total on both sides when no merge base is requested', async () => { + await seedParityFixture(repo) + + const mainStatus = await getStatus(repo) + clearGitStatusLineStatsCache() + const relayStatus = await getStatusOp(relayGit, relayStreamGit, { worktreePath: repo }) + + expect(Object.hasOwn(mainStatus, 'branchLineTotal')).toBe(false) + expect(Object.hasOwn(relayStatus, 'branchLineTotal')).toBe(false) + }) +}) diff --git a/src/main/git/status.ts b/src/main/git/status.ts index 120c84869..c03d93071 100644 --- a/src/main/git/status.ts +++ b/src/main/git/status.ts @@ -54,6 +54,13 @@ import type { GitRuntimeOptions } from './git-runtime-options' import { gitOptionsForWorktree } from './git-runtime-options' import { GitStatusReadLeaseOwner } from './git-status-read-lease-owner' import { parseGitRevListFirstParentOid } from '../../shared/git-rev-list-output' +import { + computeGitBranchLineTotal, + invalidateGitBranchLineTotalInFlight, + readGitBranchLineTotalMergeBaseParam, + GIT_BRANCH_LINE_TOTAL_TIMEOUT_MS, + type GitBranchLineTotal +} from '../../shared/git-branch-line-total' import { beginGitStatusLineStatsCacheWrite, clearGitStatusLineStatsCache, @@ -95,10 +102,12 @@ const gitDiffReadDedupe = new InFlightPromiseDedupe() const effectiveUpstreamStatusWriteGeneration = new Map() const statusReadLeaseOwner = new GitStatusReadLeaseOwner() -// Why: clear both diff and status in-flight caches; clearing only diff would let getStatus() join a pre-mutation read. +// Why: clear every in-flight git read cache; clearing only some would let a post-mutation +// getStatus() join a pre-mutation read and publish it as current. export function invalidateGitReadCaches(): void { gitDiffReadDedupe.clear() statusReadLeaseOwner.invalidate() + invalidateGitBranchLineTotalInFlight() clearGitStatusLineStatsCache() clearSubmodulePathsCache() resolvedUpstreamNameCache.clear() @@ -195,6 +204,9 @@ export function getEffectiveUpstreamStatusGenerationCountForTests(): number { export type GetStatusOptions = GitRuntimeOptions & { includeIgnored?: boolean reuseLineStats?: boolean + /** Merge-base OID the caller wants the branch line total measured against; + * omitted means the chip is hidden, so no ranged diff runs at all. */ + branchLineTotalMergeBase?: string /** * Max changed-file entries before git is stopped and the result is marked * `didHitLimit`. Defaults to DEFAULT_GIT_STATUS_LIMIT; 0 disables the cap. @@ -232,6 +244,9 @@ function getStatusReadKey(worktreePath: string, options: GetStatusOptions): stri options.wslDistro ?? '', options.includeIgnored === true, options.reuseLineStats === true, + // Why: the result carries a total only for callers who asked, and only for + // this fork point, so a shared lease must never serve one to the other. + options.branchLineTotalMergeBase ?? '', options.bypassEffectiveUpstreamNegativeCache === true, limit, // Why: this changes which entries survive, so it must not share a cache slot. @@ -371,16 +386,25 @@ async function runGetStatus( } // Why: line counts run only for areas with entries (clean tree = 0 calls); skip past the limit to avoid numstat over a huge set. + let branchLineTotal: GitBranchLineTotal | undefined if (!didHitLimit) { - await reuseOrRecomputeGitStatusLineStats({ + const branchLineTotalInput = createBranchLineTotalInput( + worktreePath, + entries, + options, + statusSucceeded + ) + const lineStats = await reuseOrRecomputeGitStatusLineStats({ cacheKey: lineStatsCacheKey, head, entries, writeToken: lineStatsWriteToken, reuse: options.reuseLineStats === true, isAborted: () => options.signal?.aborted === true, - recompute: () => attachLineStats(worktreePath, entries, options) + recompute: () => attachLineStats(worktreePath, entries, options), + ...(branchLineTotalInput ? { branchLineTotal: branchLineTotalInput } : {}) }) + branchLineTotal = lineStats.branchLineTotal } else { clearGitStatusLineStatsCacheKey(lineStatsCacheKey, lineStatsWriteToken) } @@ -398,6 +422,7 @@ async function runGetStatus( head, branch, ...(options.includeIgnored ? { ignoredPaths: parser.ignoredPaths } : {}), + ...(branchLineTotal ? { branchLineTotal } : {}), ...(didHitLimit ? { didHitLimit: true, statusLength: parser.statusLength } : {}), ...(statusSucceeded ? { @@ -416,6 +441,43 @@ async function runGetStatus( } } +/** Undefined — and therefore zero extra work — unless the caller asked for a total we can know exact. */ +function createBranchLineTotalInput( + worktreePath: string, + entries: GitStatusEntry[], + options: GetStatusOptions, + statusSucceeded: boolean +): { mergeBase: string; compute: () => Promise } | undefined { + const mergeBase = readGitBranchLineTotalMergeBaseParam(options.branchLineTotalMergeBase) + // Why: a failed status scan leaves the untracked list untrustworthy, so the + // total would silently under-count rather than be absent. + if (mergeBase === undefined || !statusSucceeded) { + return undefined + } + return { + mergeBase, + compute: () => + computeGitBranchLineTotal({ + worktreePath, + // Why: the same path can be a different filesystem per WSL distro. + hostKey: options.wslDistro ?? 'native', + mergeBase, + untrackedPaths: entries + .filter((entry) => entry.area === 'untracked') + .map((entry) => entry.path), + runDiffNumstat: (args, signal) => + gitExecFileAsync(args, { + ...gitOptionsForWorktree(worktreePath, options), + // Why: after the spread, so the shared lease signal wins over this caller's own. + signal, + env: gitOptionalLocksDisabledEnv(), + timeout: GIT_BRANCH_LINE_TOTAL_TIMEOUT_MS + }).then((result) => result.stdout), + ...(options.signal ? { signal: options.signal } : {}) + }) + } +} + function getStatusLineStatsCacheKey(worktreePath: string, options: GitRuntimeOptions = {}): string { // Why: identical paths can map to different WSL-distro filesystems, so key stats by Git's execution host. return `${options.wslDistro ?? 'native'}\0${worktreePath}` diff --git a/src/main/ipc/filesystem.ts b/src/main/ipc/filesystem.ts index f3c91088d..bf4aff54f 100644 --- a/src/main/ipc/filesystem.ts +++ b/src/main/ipc/filesystem.ts @@ -1127,6 +1127,7 @@ export function registerFilesystemHandlers( includeIgnored?: boolean bypassEffectiveUpstreamNegativeCache?: boolean reuseLineStats?: boolean + branchLineTotalMergeBase?: string requestToken?: string } ): Promise => { @@ -1134,6 +1135,9 @@ export function registerFilesystemHandlers( const options = { includeIgnored: args.includeIgnored ?? false, ...(args.reuseLineStats === true ? { reuseLineStats: true } : {}), + ...(args.branchLineTotalMergeBase === undefined + ? {} + : { branchLineTotalMergeBase: args.branchLineTotalMergeBase }), ...(args.bypassEffectiveUpstreamNegativeCache === true ? { bypassEffectiveUpstreamNegativeCache: true } : {}), diff --git a/src/main/providers/git-provider-status-options.ts b/src/main/providers/git-provider-status-options.ts index 3d03ca6a2..a8924924f 100644 --- a/src/main/providers/git-provider-status-options.ts +++ b/src/main/providers/git-provider-status-options.ts @@ -3,5 +3,7 @@ export type GitProviderStatusOptions = { includeIgnored?: boolean bypassEffectiveUpstreamNegativeCache?: boolean reuseLineStats?: boolean + /** Merge-base OID to measure the branch line total against; omit to skip the work. */ + branchLineTotalMergeBase?: string signal?: AbortSignal } diff --git a/src/main/providers/ssh-git-provider.ts b/src/main/providers/ssh-git-provider.ts index 2ec2fbbfc..e4c306529 100644 --- a/src/main/providers/ssh-git-provider.ts +++ b/src/main/providers/ssh-git-provider.ts @@ -129,11 +129,16 @@ export class SshGitProvider implements IGitProvider { ? { bypassEffectiveUpstreamNegativeCache: true } : {} const lineStatsReuseArgs = options?.reuseLineStats ? { reuseLineStats: true } : {} + const branchLineTotalArgs = + options?.branchLineTotalMergeBase === undefined + ? {} + : { branchLineTotalMergeBase: options.branchLineTotalMergeBase } const request = { worktreePath, ...includeIgnoredArgs, ...upstreamCacheBypassArgs, - ...lineStatsReuseArgs + ...lineStatsReuseArgs, + ...branchLineTotalArgs } return (await (options?.signal ? this.mux.request('git.status', request, { signal: options.signal }) diff --git a/src/main/runtime/rpc/methods/git-params.ts b/src/main/runtime/rpc/methods/git-params.ts index 7e13e584c..0fe273108 100644 --- a/src/main/runtime/rpc/methods/git-params.ts +++ b/src/main/runtime/rpc/methods/git-params.ts @@ -10,7 +10,9 @@ export const WorktreeSelector = z.object({ export const GitStatusParams = WorktreeSelector.extend({ includeIgnored: z.boolean().optional(), bypassEffectiveUpstreamNegativeCache: z.boolean().optional(), - reuseLineStats: z.boolean().optional() + reuseLineStats: z.boolean().optional(), + // Shape is re-validated host-side before it reaches a git argv. + branchLineTotalMergeBase: z.string().optional() }) export const GitCheckIgnored = WorktreeSelector.extend({ diff --git a/src/main/runtime/rpc/methods/git.ts b/src/main/runtime/rpc/methods/git.ts index c4ccdf071..c30e80f43 100644 --- a/src/main/runtime/rpc/methods/git.ts +++ b/src/main/runtime/rpc/methods/git.ts @@ -93,6 +93,7 @@ export const GIT_METHODS: RpcMethod[] = [ params.includeIgnored === undefined && params.bypassEffectiveUpstreamNegativeCache === undefined && params.reuseLineStats === undefined && + params.branchLineTotalMergeBase === undefined && signal === undefined ? undefined : { @@ -103,6 +104,9 @@ export const GIT_METHODS: RpcMethod[] = [ ? { bypassEffectiveUpstreamNegativeCache: true } : {}), ...(params.reuseLineStats === true ? { reuseLineStats: true } : {}), + ...(params.branchLineTotalMergeBase === undefined + ? {} + : { branchLineTotalMergeBase: params.branchLineTotalMergeBase }), ...(signal ? { signal } : {}) } return options === undefined diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 0dc78393f..094a93c34 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -2898,6 +2898,8 @@ export type PreloadApi = { includeIgnored?: boolean bypassEffectiveUpstreamNegativeCache?: boolean reuseLineStats?: boolean + /** Merge-base OID to measure the branch line total against; omit to skip the work. */ + branchLineTotalMergeBase?: string requestToken?: string }) => Promise cancelStatus: (args: { requestToken: string }) => Promise diff --git a/src/preload/index.ts b/src/preload/index.ts index 36e268708..8b3863dcf 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -3230,6 +3230,7 @@ const api = { includeIgnored?: boolean bypassEffectiveUpstreamNegativeCache?: boolean reuseLineStats?: boolean + branchLineTotalMergeBase?: string requestToken?: string }): Promise => ipcRenderer.invoke('git:status', args), cancelStatus: (args: { requestToken: string }): Promise => diff --git a/src/relay/git-handler-status-ops.ts b/src/relay/git-handler-status-ops.ts index 880e690ee..60ea59131 100644 --- a/src/relay/git-handler-status-ops.ts +++ b/src/relay/git-handler-status-ops.ts @@ -24,6 +24,11 @@ import { clearGitStatusLineStatsCacheKey, reuseOrRecomputeGitStatusLineStats } from '../shared/git-status-line-stats-cache' +import { + readGitBranchLineTotalMergeBaseParam, + type GitBranchLineTotal +} from '../shared/git-branch-line-total' +import { buildBranchLineTotalInput } from './git-status-branch-line-total' export async function resolveGitDir(worktreePath: string): Promise { const dotGitPath = path.join(worktreePath, '.git') @@ -74,11 +79,16 @@ export async function getStatusOp( ignoredPaths?: string[] didHitLimit?: boolean statusLength?: number + branchLineTotal?: GitBranchLineTotal }> { const worktreePath = params.worktreePath as string const lineStatsCacheKey = `relay\0${worktreePath}` const lineStatsWriteToken = beginGitStatusLineStatsCacheWrite(lineStatsCacheKey) const includeIgnored = params.includeIgnored === true + // Why: untrusted RPC input spliced into a git argv — only an OID shape may pass. + const branchLineTotalMergeBase = readGitBranchLineTotalMergeBaseParam( + params.branchLineTotalMergeBase + ) // Why: reject NaN/negative limits — NaN would silently disable capping, negatives would over-truncate. const limit = resolveGitStatusLimit(params.limit) const conflictOperation = await detectConflictOperation(worktreePath) @@ -89,6 +99,8 @@ export async function getStatusOp( let ignoredPaths: string[] = [] let didHitLimit = false let statusLength = 0 + let statusSucceeded = false + let branchLineTotal: GitBranchLineTotal | undefined try { // Why: core.quotePath=false keeps non-ASCII filenames as raw UTF-8 instead of octal escapes that render as gibberish. @@ -118,6 +130,7 @@ export async function getStatusOp( ignoredPaths = parser.ignoredPaths statusLength = parser.statusLength didHitLimit = stoppedEarly + statusSucceeded = true const { upstreamName, upstreamAheadBehind } = parser.branch upstreamStatus = upstreamName ? { @@ -173,15 +186,26 @@ export async function getStatusOp( // Why: skip numstat after the limit to avoid reintroducing its cost. if (!didHitLimit) { - await reuseOrRecomputeGitStatusLineStats({ + const branchLineTotalInput = buildBranchLineTotalInput( + git, + worktreePath, + entries, + // Why: a failed scan leaves the untracked list untrustworthy, so the total + // would under-count — omit it rather than publish a confident wrong number. + statusSucceeded ? branchLineTotalMergeBase : undefined, + options.signal + ) + // Why: passed in so the ranged diff runs alongside the per-area numstats, not after them. + ;({ branchLineTotal } = await reuseOrRecomputeGitStatusLineStats({ cacheKey: lineStatsCacheKey, head, entries, writeToken: lineStatsWriteToken, reuse: params.reuseLineStats === true, isAborted: () => options.signal?.aborted === true, - recompute: () => attachLineStats(git, worktreePath, entries, options.signal) - }) + recompute: () => attachLineStats(git, worktreePath, entries, options.signal), + ...(branchLineTotalInput ? { branchLineTotal: branchLineTotalInput } : {}) + })) } else { clearGitStatusLineStatsCacheKey(lineStatsCacheKey, lineStatsWriteToken) } @@ -200,7 +224,8 @@ export async function getStatusOp( branch, upstreamStatus, ...(includeIgnored ? { ignoredPaths } : {}), - ...(didHitLimit ? { didHitLimit: true, statusLength } : {}) + ...(didHitLimit ? { didHitLimit: true, statusLength } : {}), + ...(branchLineTotal ? { branchLineTotal } : {}) } } diff --git a/src/relay/git-handler.ts b/src/relay/git-handler.ts index 6674500c6..0abc9f5d5 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -89,6 +89,7 @@ import { GitResponseStreamRegistry } from './git-response-stream' import { GIT_RESPONSE_STREAM_THRESHOLD } from './protocol' import { endSubprocessStdin } from '../shared/subprocess-stdin-write' import { clearGitStatusLineStatsCache } from '../shared/git-status-line-stats-cache' +import { invalidateGitBranchLineTotalInFlight } from '../shared/git-branch-line-total' import { streamRelayGitStdout } from './git-stdout-stream' const execFileAsync = promisify(execFile) @@ -298,6 +299,7 @@ export class GitHandler { private clearGitMutationReadCaches(): void { this.gitDiffReadDedupe.clear() + invalidateGitBranchLineTotalInFlight() clearGitStatusLineStatsCache() clearSubmodulePathsCache(this.submodulePathsCache) } diff --git a/src/relay/git-status-branch-line-total.test.ts b/src/relay/git-status-branch-line-total.test.ts new file mode 100644 index 000000000..ceb04351e --- /dev/null +++ b/src/relay/git-status-branch-line-total.test.ts @@ -0,0 +1,475 @@ +/** + * Relay-side contract for the status pass's branch line total: the chip is an + * exact number or absent, and costs nothing when nobody asked for it. + */ +import { execFile, execFileSync } from 'node:child_process' +import { mkdtempSync } from 'node:fs' +import * as fs from 'node:fs/promises' +import { tmpdir } from 'node:os' +import * as path from 'node:path' +import { promisify } from 'node:util' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { invalidateGitBranchLineTotalInFlight } from '../shared/git-branch-line-total' +import { clearGitStatusLineStatsCache } from '../shared/git-status-line-stats-cache' +import type { GitExec } from './git-handler-ops' +import { getStatusOp } from './git-handler-status-ops' +import type { RelayGitStreamExec } from './git-stdout-stream' +import { clearNoEffectiveUpstreamStatusCache } from './git-status-upstream-negative-cache' + +type GitCall = Parameters + +const execFileAsync = promisify(execFile) +const MERGE_BASE = '0123456789abcdef0123456789abcdef01234567' +const OTHER_MERGE_BASE = 'fedcba9876543210fedcba9876543210fedcba98' +// Untracked additions ride on top of the ranged diff, so the fixture needs a real file. +const UNTRACKED_FILE = 'notes.md' +const UNTRACKED_LINES = 4 +const STATUS_OUTPUT = [ + '# branch.oid 1111111111111111111111111111111111111111', + '# branch.head (detached)', + '1 .M N... 100644 100644 100644 aaaa aaaa src/a.ts', + `? ${UNTRACKED_FILE}` +].join('\n') + +function isRangedNumstat(args: string[]): boolean { + return args.includes('diff') && args.includes('--numstat') && args.includes('-z') +} + +function rangedDiffCalls(calls: readonly GitCall[]): string[][] { + return calls.map(([args]) => args).filter((args) => isRangedNumstat(args)) +} + +function streamGitFromCapture(git: GitExec): RelayGitStreamExec { + return async (args, cwd, options) => { + const { stdout } = await git(args, cwd, { + disableOptionalLocks: options.disableOptionalLocks, + signal: options.signal + }) + return { stoppedEarly: options.onStdout(stdout) === true } + } +} + +/** Mock host; the fixture-repo cases below run real git instead. */ +function createMockGit(overrides: { + status?: string + ranged?: () => Promise<{ stdout: string; stderr: string }> + areaNumstat?: string +}) { + return vi.fn(async (args) => { + if (args.includes('status')) { + return { stdout: overrides.status ?? STATUS_OUTPUT, stderr: '' } + } + if (isRangedNumstat(args)) { + return overrides.ranged ? overrides.ranged() : { stdout: '12\t5\tsrc/a.ts\n', stderr: '' } + } + if (args.includes('diff')) { + return { stdout: overrides.areaNumstat ?? '3\t2\tsrc/a.ts\n', stderr: '' } + } + throw new Error(`Unexpected git command: ${args.join(' ')}`) + }) +} + +const realGitExec: GitExec = async (args, cwd, opts) => { + const { stdout, stderr } = await execFileAsync('git', args, { + cwd, + encoding: 'utf8', + ...(opts?.signal ? { signal: opts.signal } : {}), + ...(opts?.timeout ? { timeout: opts.timeout } : {}) + }) + return { stdout, stderr } +} + +function runFixtureGit(repo: string, args: string[]): string { + return execFileSync( + 'git', + [ + '-c', + 'user.email=test@test.com', + '-c', + 'user.name=Test', + '-c', + 'commit.gpgSign=false', + ...args + ], + { cwd: repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] } + ).trim() +} + +/** + * Fork point → working tree: +3 tracked (2 committed, 1 unstaged), -2 from the + * file the branch deleted, +4 untracked. The pure rename contributes nothing. + */ +async function seedBranchFixture(repo: string): Promise { + await fs.rm(path.join(repo, UNTRACKED_FILE), { force: true }) + execFileSync('git', ['init', '-q', repo], { stdio: 'pipe' }) + await fs.writeFile(path.join(repo, 'tracked.txt'), 'a\nb\nc\n') + await fs.writeFile(path.join(repo, 'doomed.txt'), 'x\ny\n') + await fs.writeFile(path.join(repo, 'old-name.txt'), 'stable\n') + runFixtureGit(repo, ['add', '.']) + runFixtureGit(repo, ['commit', '-m', 'base']) + const mergeBase = runFixtureGit(repo, ['rev-parse', 'HEAD']) + + runFixtureGit(repo, ['checkout', '-q', '-b', 'feature']) + await fs.writeFile(path.join(repo, 'tracked.txt'), 'a\nb\nc\nd\ne\n') + await fs.rm(path.join(repo, 'doomed.txt')) + runFixtureGit(repo, ['add', '-A']) + runFixtureGit(repo, ['commit', '-m', 'branch commit']) + + runFixtureGit(repo, ['mv', 'old-name.txt', 'new-name.txt']) + await fs.writeFile(path.join(repo, 'tracked.txt'), 'a\nb\nc\nd\ne\nf\n') + await fs.writeFile(path.join(repo, UNTRACKED_FILE), 'n1\nn2\nn3\nn4\n') + return mergeBase +} + +describe('getStatusOp branch line total', () => { + let tmpDir: string + + beforeEach(async () => { + clearNoEffectiveUpstreamStatusCache() + clearGitStatusLineStatsCache() + invalidateGitBranchLineTotalInFlight() + tmpDir = mkdtempSync(path.join(tmpdir(), 'relay-branch-line-total-')) + await fs.writeFile(path.join(tmpDir, UNTRACKED_FILE), 'n1\nn2\nn3\nn4\n') + }) + + afterEach(async () => { + clearNoEffectiveUpstreamStatusCache() + clearGitStatusLineStatsCache() + invalidateGitBranchLineTotalInFlight() + await fs.rm(tmpDir, { recursive: true, force: true }) + }) + + it('runs no ranged diff and returns no total when the merge base param is absent', async () => { + const git = createMockGit({}) + + const result = await getStatusOp(git, streamGitFromCapture(git), { worktreePath: tmpDir }) + + expect(rangedDiffCalls(git.mock.calls)).toEqual([]) + expect(Object.hasOwn(result, 'branchLineTotal')).toBe(false) + expect(result.branchLineTotal).toBeUndefined() + // The per-area numstats the CHANGES rows need still ran, so this is a + // zero-cost omission rather than a disabled status pass. + expect(result.entries).toContainEqual( + expect.objectContaining({ path: 'src/a.ts', added: 3, removed: 2 }) + ) + }) + + it('sums the ranged diff and untracked additions against a real fixture repo', async () => { + const mergeBase = await seedBranchFixture(tmpDir) + const git = vi.fn(realGitExec) + + const result = await getStatusOp(git, streamGitFromCapture(git), { + worktreePath: tmpDir, + branchLineTotalMergeBase: mergeBase + }) + + expect(result.branchLineTotal).toEqual({ + added: 3 + UNTRACKED_LINES, + removed: 2, + mergeBase + }) + expect(rangedDiffCalls(git.mock.calls)).toEqual([ + ['-c', 'core.quotePath=false', 'diff', '-z', '--numstat', '-M', mergeBase, '--'] + ]) + }) + + it('omits the total when the status listing hit its limit', async () => { + const manyEntries = Array.from( + { length: 6 }, + (_, index) => `1 A. N... 100644 100644 100644 000000 111111 generated-${index}.txt` + ).join('\n') + const git = createMockGit({ status: manyEntries }) + + const result = await getStatusOp(git, streamGitFromCapture(git), { + worktreePath: tmpDir, + limit: 3, + branchLineTotalMergeBase: MERGE_BASE + }) + + expect(result.didHitLimit).toBe(true) + // The untracked list is truncated, so any total would silently under-count. + expect(Object.hasOwn(result, 'branchLineTotal')).toBe(false) + expect(rangedDiffCalls(git.mock.calls)).toEqual([]) + }) + + it('omits the total, never zero, when the ranged diff fails', async () => { + const git = createMockGit({ + ranged: () => Promise.reject(new Error('fatal: bad object 0123456789abcdef')) + }) + + const result = await getStatusOp(git, streamGitFromCapture(git), { + worktreePath: tmpDir, + branchLineTotalMergeBase: MERGE_BASE + }) + + expect(Object.hasOwn(result, 'branchLineTotal')).toBe(false) + expect(result.branchLineTotal).toBeUndefined() + expect(rangedDiffCalls(git.mock.calls)).toHaveLength(1) + // A failed ranged diff must not take the per-file rows down with it. + expect(result.entries).toContainEqual(expect.objectContaining({ path: 'src/a.ts', added: 3 })) + }) + + it('omits the total, never zero, for a well-formed but unknown merge-base oid', async () => { + await seedBranchFixture(tmpDir) + const git = vi.fn(realGitExec) + + const result = await getStatusOp(git, streamGitFromCapture(git), { + worktreePath: tmpDir, + branchLineTotalMergeBase: 'deadbeefdeadbeefdeadbeefdeadbeefdeadbeef' + }) + + expect(rangedDiffCalls(git.mock.calls)).toHaveLength(1) + expect(Object.hasOwn(result, 'branchLineTotal')).toBe(false) + expect(result.entries.length).toBeGreaterThan(0) + }) + + it('omits the total when the ranged diff exceeds its time budget', async () => { + const git = createMockGit({ + ranged: () => { + const error: Error & { killed?: boolean } = new Error('spawn git ETIMEDOUT') + error.killed = true + return Promise.reject(error) + } + }) + + const result = await getStatusOp(git, streamGitFromCapture(git), { + worktreePath: tmpDir, + branchLineTotalMergeBase: MERGE_BASE + }) + + expect(Object.hasOwn(result, 'branchLineTotal')).toBe(false) + expect(rangedDiffCalls(git.mock.calls)).toHaveLength(1) + // The budget is handed to the subprocess rather than raced on a wall clock. + const rangedOptions = git.mock.calls.find(([args]) => isRangedNumstat(args))?.[2] + expect(rangedOptions).toMatchObject({ disableOptionalLocks: true, timeout: expect.any(Number) }) + }) + + it('omits the total when the status scan itself failed', async () => { + const git = vi.fn(async (args) => { + if (args.includes('status')) { + throw new Error('fatal: not a git repository') + } + throw new Error(`Unexpected git command: ${args.join(' ')}`) + }) + + const result = await getStatusOp(git, streamGitFromCapture(git), { + worktreePath: tmpDir, + branchLineTotalMergeBase: MERGE_BASE + }) + + expect(result.entries).toEqual([]) + // A failed scan yields an empty untracked list, which would read as "no + // untracked additions" rather than "unknown". + expect(Object.hasOwn(result, 'branchLineTotal')).toBe(false) + expect(rangedDiffCalls(git.mock.calls)).toEqual([]) + }) + + // Relay params are an untyped bag, so the merge base must be proven to be an + // object name before it can be spliced into an argv. + it.each([ + ['a flag-shaped value', '--upload-pack=x'], + ['a rev name', 'HEAD'], + ['a ref path', 'refs/heads/main'], + ['a range', `${MERGE_BASE}..HEAD`], + ['a number', 123], + ['null', null], + ['an empty string', ''] + ])('rejects %s before it reaches a git argv', async (_label, value) => { + const git = createMockGit({}) + + const result = await getStatusOp(git, streamGitFromCapture(git), { + worktreePath: tmpDir, + branchLineTotalMergeBase: value + }) + + expect(Object.hasOwn(result, 'branchLineTotal')).toBe(false) + expect(rangedDiffCalls(git.mock.calls)).toEqual([]) + for (const [args] of git.mock.calls) { + expect(args).not.toContain(String(value)) + } + }) + + it('answers a rejected merge base with the byte-identical no-merge-base response', async () => { + const gitWithout = createMockGit({}) + const withoutParam = await getStatusOp(gitWithout, streamGitFromCapture(gitWithout), { + worktreePath: tmpDir + }) + clearGitStatusLineStatsCache() + const gitRejected = createMockGit({}) + const rejectedParam = await getStatusOp(gitRejected, streamGitFromCapture(gitRejected), { + worktreePath: tmpDir, + branchLineTotalMergeBase: '--upload-pack=x' + }) + + expect(JSON.stringify(rejectedParam)).toBe(JSON.stringify(withoutParam)) + expect(gitRejected.mock.calls.map(([args]) => args)).toEqual( + gitWithout.mock.calls.map(([args]) => args) + ) + }) + + it('rejects an aborted scan instead of resolving a partial total', async () => { + const controller = new AbortController() + const git = vi.fn(async (args) => { + if (args.includes('status')) { + return { stdout: STATUS_OUTPUT, stderr: '' } + } + if (isRangedNumstat(args)) { + controller.abort() + const error = new Error('The operation was aborted.') + error.name = 'AbortError' + throw error + } + if (args.includes('diff')) { + return { stdout: '3\t2\tsrc/a.ts\n', stderr: '' } + } + throw new Error(`Unexpected git command: ${args.join(' ')}`) + }) + + await expect( + getStatusOp( + git, + streamGitFromCapture(git), + { worktreePath: tmpDir, branchLineTotalMergeBase: MERGE_BASE }, + { signal: controller.signal } + ) + ).rejects.toThrow(/abort/i) + expect(rangedDiffCalls(git.mock.calls)).toHaveLength(1) + }) + + it('reuses the cached total instead of re-running the ranged diff', async () => { + const git = createMockGit({}) + + const first = await getStatusOp(git, streamGitFromCapture(git), { + worktreePath: tmpDir, + branchLineTotalMergeBase: MERGE_BASE + }) + const reused = await getStatusOp(git, streamGitFromCapture(git), { + worktreePath: tmpDir, + branchLineTotalMergeBase: MERGE_BASE, + reuseLineStats: true + }) + + expect(first.branchLineTotal).toEqual({ + added: 12 + UNTRACKED_LINES, + removed: 5, + mergeBase: MERGE_BASE + }) + expect(reused.branchLineTotal).toEqual(first.branchLineTotal) + expect(rangedDiffCalls(git.mock.calls)).toHaveLength(1) + }) + + it('coalesces concurrent status passes onto one ranged diff', async () => { + // The renderer's own in-flight refs only dedupe one renderer; a second + // window or an fs-watcher burst must not run the ranged diff twice. + const git = vi.fn(async (args) => { + if (args.includes('status')) { + return { stdout: STATUS_OUTPUT, stderr: '' } + } + if (isRangedNumstat(args)) { + await new Promise((resolve) => setTimeout(resolve, 20)) + return { stdout: '12\t5\tsrc/a.ts\n', stderr: '' } + } + if (args.includes('diff')) { + return { stdout: '3\t2\tsrc/a.ts\n', stderr: '' } + } + throw new Error(`Unexpected git command: ${args.join(' ')}`) + }) + + const [first, second] = await Promise.all([ + getStatusOp(git, streamGitFromCapture(git), { + worktreePath: tmpDir, + branchLineTotalMergeBase: MERGE_BASE + }), + getStatusOp(git, streamGitFromCapture(git), { + worktreePath: tmpDir, + branchLineTotalMergeBase: MERGE_BASE + }) + ]) + + expect(rangedDiffCalls(git.mock.calls)).toHaveLength(1) + expect(first.branchLineTotal).toEqual(second.branchLineTotal) + expect(first.branchLineTotal).toEqual({ + added: 12 + UNTRACKED_LINES, + removed: 5, + mergeBase: MERGE_BASE + }) + }) + + it('recomputes rather than reusing a total measured against another fork point', async () => { + const git = createMockGit({}) + + await getStatusOp(git, streamGitFromCapture(git), { + worktreePath: tmpDir, + branchLineTotalMergeBase: MERGE_BASE + }) + const moved = await getStatusOp(git, streamGitFromCapture(git), { + worktreePath: tmpDir, + branchLineTotalMergeBase: OTHER_MERGE_BASE, + reuseLineStats: true + }) + + expect(moved.branchLineTotal).toEqual({ + added: 12 + UNTRACKED_LINES, + removed: 5, + mergeBase: OTHER_MERGE_BASE + }) + expect(rangedDiffCalls(git.mock.calls)).toHaveLength(2) + }) + + // Rule 1 of docs/reference/remote-wire-compatibility.md: a new optional field + // is safe only while every reader survives its absence. + describe('wire compatibility', () => { + it('drops the key from the wire payload so a reader sees undefined, not 0 or NaN', async () => { + const git = createMockGit({ ranged: () => Promise.reject(new Error('fatal: bad object')) }) + + const result = await getStatusOp(git, streamGitFromCapture(git), { + worktreePath: tmpDir, + branchLineTotalMergeBase: MERGE_BASE + }) + // An old server's payload and a new server's "not known exact" payload are + // the same thing on the wire: no key at all. + const overTheWire = JSON.parse(JSON.stringify(result)) as { + branchLineTotal?: { added: number; removed: number } + } + + expect(Object.hasOwn(overTheWire, 'branchLineTotal')).toBe(false) + expect(overTheWire.branchLineTotal).toBeUndefined() + expect(overTheWire.branchLineTotal?.added).toBeUndefined() + expect(overTheWire.branchLineTotal?.removed).toBeUndefined() + }) + + it('keeps the new request param out of the status argv an old server would run', async () => { + const git = createMockGit({}) + + await getStatusOp(git, streamGitFromCapture(git), { + worktreePath: tmpDir, + branchLineTotalMergeBase: MERGE_BASE + }) + + const statusArgs = git.mock.calls + .map(([args]) => args) + .filter((args) => args.includes('status')) + expect(statusArgs).toHaveLength(1) + expect(statusArgs[0]).not.toContain(MERGE_BASE) + }) + + it('publishes finite counts when the ranged diff reports only binaries', async () => { + const git = createMockGit({ + ranged: () => Promise.resolve({ stdout: '-\t-\tlogo.png\n', stderr: '' }) + }) + + const result = await getStatusOp(git, streamGitFromCapture(git), { + worktreePath: tmpDir, + branchLineTotalMergeBase: MERGE_BASE + }) + + expect(result.branchLineTotal).toEqual({ + added: UNTRACKED_LINES, + removed: 0, + mergeBase: MERGE_BASE + }) + expect(Number.isFinite(result.branchLineTotal?.added)).toBe(true) + expect(Number.isFinite(result.branchLineTotal?.removed)).toBe(true) + }) + }) +}) diff --git a/src/relay/git-status-branch-line-total.ts b/src/relay/git-status-branch-line-total.ts new file mode 100644 index 000000000..c2fe6556a --- /dev/null +++ b/src/relay/git-status-branch-line-total.ts @@ -0,0 +1,44 @@ +/** + * Relay-side wiring for the status pass's branch line total. + * Why: separate file so git-handler-status-ops.ts stays under oxlint max-lines (300). + */ +import { + computeGitBranchLineTotal, + GIT_BRANCH_LINE_TOTAL_TIMEOUT_MS, + type GitBranchLineTotal +} from '../shared/git-branch-line-total' +import type { GitExec } from './git-handler-ops' + +/** Undefined when no valid merge base was requested, which keeps the ranged diff entirely off. */ +export function buildBranchLineTotalInput( + git: GitExec, + worktreePath: string, + entries: Record[], + mergeBase: string | undefined, + signal?: AbortSignal +): { mergeBase: string; compute: () => Promise } | undefined { + if (!mergeBase) { + return undefined + } + return { + mergeBase, + compute: () => + computeGitBranchLineTotal({ + worktreePath, + // Matches the `relay\0` line-stats cache-key convention. + hostKey: 'relay', + mergeBase, + untrackedPaths: entries + .filter((entry) => entry.area === 'untracked') + .map((entry) => entry.path as string), + runDiffNumstat: (args, diffSignal) => + git(args, worktreePath, { + // Why: a working-tree diff must not take index.lock away from terminal Git. + disableOptionalLocks: true, + signal: diffSignal, + timeout: GIT_BRANCH_LINE_TOTAL_TIMEOUT_MS + }).then(({ stdout }) => stdout), + ...(signal ? { signal } : {}) + }) + } +} diff --git a/src/renderer/src/components/right-sidebar/SourceControl.branch-line-total.test.tsx b/src/renderer/src/components/right-sidebar/SourceControl.branch-line-total.test.tsx new file mode 100644 index 000000000..c9a7f24be --- /dev/null +++ b/src/renderer/src/components/right-sidebar/SourceControl.branch-line-total.test.tsx @@ -0,0 +1,340 @@ +// @vitest-environment happy-dom + +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { TooltipProvider } from '@/components/ui/tooltip' +import type { GitBranchCompareSummary } from '../../../../shared/types' +import { + clearBranchLineTotalRequestGateForTests, + getBranchLineTotalMergeBase +} from './branch-line-total-request-gate' +import SourceControl from './SourceControl' + +const MERGE_BASE = '1f3c0d9a5b6e7f8091a2b3c4d5e6f708192a3b4c' + +const mocks = vi.hoisted(() => { + const activeRepo = { + id: 'repo-1', + path: '/repo', + displayName: 'Repo', + badgeColor: '#000', + addedAt: 0, + kind: 'git' as string + } + const activeWorktree = { + id: 'wt-1', + repoId: 'repo-1', + path: '/repo/wt', + head: 'head-1', + branch: 'refs/heads/feature/line-total', + isBare: false, + isMainWorktree: false, + displayName: 'feature/line-total', + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + linkedGitLabMR: null, + linkedGitLabIssue: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 0 + } + return { + activeRepo, + activeWorktree, + state: {} as Record + } +}) + +vi.mock('@/store', () => { + const useAppStore = Object.assign( + (selector?: (state: Record) => unknown) => + selector ? selector(mocks.state) : mocks.state, + { + getState: () => mocks.state + } + ) + return { useAppStore } +}) + +vi.mock('@/store/selectors', () => ({ + useActiveWorktree: () => mocks.activeWorktree, + useRepoById: (repoId: string | null) => + repoId === mocks.activeRepo.id ? mocks.activeRepo : null, + useWorktreeMap: () => new Map([[mocks.activeWorktree.id, mocks.activeWorktree]]) +})) + +vi.mock('@/components/confirmation-dialog-context', () => ({ + useConfirmationDialog: () => vi.fn().mockResolvedValue(true) +})) + +vi.mock('./git-status-refresh', () => ({ + refreshGitStatusForWorktree: vi.fn().mockResolvedValue(undefined), + refreshGitStatusForWorktreeStrict: vi.fn().mockResolvedValue(undefined) +})) + +function noopAsync(value: unknown = undefined): () => Promise { + return vi.fn().mockResolvedValue(value) +} + +const readySummary: GitBranchCompareSummary = { + baseRef: 'refs/remotes/origin/main', + baseOid: 'base-oid', + compareRef: 'feature/line-total', + headOid: 'head-1', + mergeBase: MERGE_BASE, + changedFiles: 2, + commitsAhead: 1, + status: 'ready' +} + +function resetState(overrides: Partial> = {}): void { + vi.clearAllMocks() + mocks.activeRepo.kind = 'git' + mocks.state = { + activeWorktreeId: mocks.activeWorktree.id, + activeGroupIdByWorktree: { [mocks.activeWorktree.id]: 'group-1' }, + groupsByWorktree: { [mocks.activeWorktree.id]: [{ id: 'group-1', activeTabId: null }] }, + repos: [mocks.activeRepo], + worktreesByRepo: { [mocks.activeRepo.id]: [mocks.activeWorktree] }, + rightSidebarOpen: true, + rightSidebarTab: 'source-control', + gitStatusByWorktree: { [mocks.activeWorktree.id]: [] }, + gitBranchChangesByWorktree: { [mocks.activeWorktree.id]: [] }, + gitBranchCompareSummaryByWorktree: { [mocks.activeWorktree.id]: readySummary }, + gitBranchLineTotalByWorktree: {}, + gitConflictOperationByWorktree: {}, + remoteStatusesByWorktree: {}, + isRemoteOperationActive: false, + inFlightRemoteOpKind: null, + settings: null, + hostedReviewCache: {}, + prCache: {}, + commitMessageGenerationRecords: {}, + pullRequestGenerationRecords: {}, + openFiles: [], + activeFileIdByWorktree: {}, + activeTabTypeByWorktree: {}, + getDiffComments: vi.fn(() => []), + updateSettings: noopAsync(), + openSettingsTarget: vi.fn(), + openSettingsPage: vi.fn(), + fetchHostedReviewForBranch: noopAsync(), + getHostedReviewCreationEligibility: noopAsync(null), + createHostedReview: noopAsync({ ok: false, error: 'not available' }), + updateWorktreeMeta: noopAsync(), + fetchPRForBranch: noopAsync(), + enqueueGitHubPRRefresh: vi.fn(), + updateRepo: noopAsync(), + setGitStatus: vi.fn(), + updateWorktreeGitIdentity: vi.fn(), + beginGitBranchCompareRequest: vi.fn(() => 'request-key'), + setGitBranchCompareResult: vi.fn(), + clearGitBranchCompare: vi.fn(), + fetchUpstreamStatus: noopAsync(), + setUpstreamStatus: vi.fn(), + pushBranch: noopAsync(), + pullBranch: noopAsync(), + fastForwardBranch: noopAsync(), + syncBranch: noopAsync(), + rebaseFromBase: noopAsync(), + fetchBranch: noopAsync(), + revealInExplorer: vi.fn(), + trackConflictPath: vi.fn(), + openDiff: vi.fn(), + openFile: vi.fn(), + setEditorViewMode: vi.fn(), + setMarkdownViewMode: vi.fn(), + setPendingEditorReveal: vi.fn(), + openConflictFile: vi.fn(), + openConflictReview: vi.fn(), + openBranchDiff: vi.fn(), + createEmptySplitGroup: vi.fn(() => 'group-2'), + openAllDiffs: vi.fn(), + openBranchAllDiffs: vi.fn(), + openCommitAllDiffs: vi.fn(), + deleteDiffComment: noopAsync(true), + clearDiffComments: noopAsync(true), + clearDiffCommentsForFile: noopAsync(true), + setScrollToDiffCommentId: vi.fn(), + setRightSidebarOpen: vi.fn(), + setRightSidebarTab: vi.fn(), + allocateCommitMessageGenerationRequestId: vi.fn(() => 'commit-generation-1'), + setCommitMessageGenerationRecord: vi.fn(), + updateCommitMessageGenerationRecord: vi.fn(), + pruneCommitMessageGenerationRecords: vi.fn(), + allocatePullRequestGenerationRequestId: vi.fn(() => 'pr-generation-1'), + setPullRequestGenerationRecord: vi.fn(), + updatePullRequestGenerationRecord: vi.fn(), + prunePullRequestGenerationRecords: vi.fn(), + ...overrides + } +} + +let container: HTMLDivElement +let root: Root + +beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true + clearBranchLineTotalRequestGateForTests() + resetState() + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) +}) + +afterEach(() => { + act(() => root.unmount()) + container.remove() + clearBranchLineTotalRequestGateForTests() +}) + +function renderSourceControl(): void { + act(() => { + root.render( + + + + ) + }) +} + +function chip(): HTMLElement | null { + return container.querySelector('[data-testid="source-control-branch-line-total"]') +} + +describe('SourceControl branch line total request gate', () => { + it('asks for a total when the panel is visible and compare is ready', () => { + renderSourceControl() + + expect(getBranchLineTotalMergeBase(mocks.activeWorktree.id)).toBe(MERGE_BASE) + }) + + it('asks for nothing while the sidebar is closed', () => { + resetState({ rightSidebarOpen: false }) + renderSourceControl() + + expect(getBranchLineTotalMergeBase(mocks.activeWorktree.id)).toBeUndefined() + }) + + it('asks for nothing while another right-sidebar tab is showing', () => { + resetState({ rightSidebarTab: 'checks' }) + renderSourceControl() + + expect(getBranchLineTotalMergeBase(mocks.activeWorktree.id)).toBeUndefined() + }) + + it('asks for nothing until branch compare is ready', () => { + resetState({ + gitBranchCompareSummaryByWorktree: { + [mocks.activeWorktree.id]: { ...readySummary, status: 'loading' } + } + }) + renderSourceControl() + + expect(getBranchLineTotalMergeBase(mocks.activeWorktree.id)).toBeUndefined() + }) + + it('asks for nothing when compare has no merge base', () => { + resetState({ + gitBranchCompareSummaryByWorktree: { + [mocks.activeWorktree.id]: { ...readySummary, status: 'invalid-base' } + } + }) + renderSourceControl() + + expect(getBranchLineTotalMergeBase(mocks.activeWorktree.id)).toBeUndefined() + }) + + it('asks for nothing in a folder workspace', () => { + resetState() + mocks.activeRepo.kind = 'folder' + renderSourceControl() + + expect(getBranchLineTotalMergeBase(mocks.activeWorktree.id)).toBeUndefined() + }) + + it('releases the gate when the panel unmounts', () => { + renderSourceControl() + expect(getBranchLineTotalMergeBase(mocks.activeWorktree.id)).toBe(MERGE_BASE) + + act(() => root.render({null})) + + expect(getBranchLineTotalMergeBase(mocks.activeWorktree.id)).toBeUndefined() + }) +}) + +describe('SourceControl branch line total chip', () => { + it('renders a total measured against the current fork point', () => { + resetState({ + gitBranchLineTotalByWorktree: { + [mocks.activeWorktree.id]: { added: 8259, removed: 670, mergeBase: MERGE_BASE } + } + }) + renderSourceControl() + + expect(chip()?.getAttribute('aria-label')).toBe('8259 additions, 670 deletions') + // Grouping is pinned to the app locale (`en`), not the runner's host locale. + expect(chip()?.textContent).toBe('+8,259-670') + }) + + it('drops a total whose fork point has since moved', () => { + // Why: status and branch compare refresh on different cadences, so a total + // can outlive the merge base it measured. Hidden beats a stale number. + resetState({ + gitBranchLineTotalByWorktree: { + [mocks.activeWorktree.id]: { added: 8259, removed: 670, mergeBase: 'stale-merge-base' } + } + }) + renderSourceControl() + + expect(chip()).toBeNull() + }) + + it('drops a published total while branch compare has no ready summary', () => { + resetState({ + gitBranchCompareSummaryByWorktree: { + [mocks.activeWorktree.id]: { ...readySummary, status: 'loading', mergeBase: '' } + }, + gitBranchLineTotalByWorktree: { + [mocks.activeWorktree.id]: { added: 8259, removed: 670, mergeBase: MERGE_BASE } + } + }) + renderSourceControl() + + expect(chip()).toBeNull() + }) + + it('renders nothing when no total was published', () => { + renderSourceControl() + + expect(chip()).toBeNull() + }) + + it('renders nothing for an exact zero total', () => { + resetState({ + gitBranchLineTotalByWorktree: { + [mocks.activeWorktree.id]: { added: 0, removed: 0, mergeBase: MERGE_BASE } + } + }) + renderSourceControl() + + expect(chip()).toBeNull() + }) + + it('omits the zero half of a one-sided total', () => { + resetState({ + gitBranchLineTotalByWorktree: { + [mocks.activeWorktree.id]: { added: 42, removed: 0, mergeBase: MERGE_BASE } + } + }) + renderSourceControl() + + expect(chip()?.textContent).toBe('+42') + expect(chip()?.getAttribute('aria-label')).toBe('42 additions') + }) +}) diff --git a/src/renderer/src/components/right-sidebar/SourceControl.open-file-highlight.test.tsx b/src/renderer/src/components/right-sidebar/SourceControl.open-file-highlight.test.tsx index 8624d25fa..d3125ee86 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.open-file-highlight.test.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.open-file-highlight.test.tsx @@ -104,6 +104,7 @@ function resetState(overrides: Partial> = {}): void { gitStatusByWorktree: { [mocks.activeWorktree.id]: [] }, gitBranchChangesByWorktree: { [mocks.activeWorktree.id]: [] }, gitBranchCompareSummaryByWorktree: { [mocks.activeWorktree.id]: null }, + gitBranchLineTotalByWorktree: {}, gitConflictOperationByWorktree: {}, remoteStatusesByWorktree: {}, isRemoteOperationActive: false, diff --git a/src/renderer/src/components/right-sidebar/SourceControl.preview-open.test.tsx b/src/renderer/src/components/right-sidebar/SourceControl.preview-open.test.tsx index 22bc4954b..13dc24798 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.preview-open.test.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.preview-open.test.tsx @@ -156,6 +156,7 @@ function resetState(overrides: Partial> = {}): void { gitStatusByWorktree: { [mocks.activeWorktree.id]: [] }, gitBranchChangesByWorktree: { [mocks.activeWorktree.id]: [] }, gitBranchCompareSummaryByWorktree: { [mocks.activeWorktree.id]: null }, + gitBranchLineTotalByWorktree: {}, gitConflictOperationByWorktree: {}, remoteStatusesByWorktree: {}, isRemoteOperationActive: false, diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index 68b55f9bd..68cabc5e0 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -296,6 +296,7 @@ import { } from './source-control-hosted-review-push-target' import { buildSourceControlManualReviewUrlFromContext } from './source-control-manual-review-url' import { parseRemoteRepo } from './source-control-remote-repo' +import { setBranchLineTotalMergeBase } from './branch-line-total-request-gate' export { HostedReviewHeaderLink } from './hosted-review-header-chrome' import { createRunningCommitMessageGenerationRecord, @@ -835,6 +836,15 @@ function SourceControlInner(): React.JSX.Element { const branchSummary = useAppStore((s) => activeWorktreeId ? (s.gitBranchCompareSummaryByWorktree[activeWorktreeId] ?? null) : null ) + const publishedBranchLineTotal = useAppStore((s) => + activeWorktreeId ? (s.gitBranchLineTotalByWorktree?.[activeWorktreeId] ?? null) : null + ) + // Why: status and branch compare refresh on different cadences, so a total can + // outlive the fork point it measured. Drop it rather than render a stale number. + const branchLineTotal = + publishedBranchLineTotal && publishedBranchLineTotal.mergeBase === branchSummary?.mergeBase + ? publishedBranchLineTotal + : null const conflictOperation = useAppStore((s) => activeWorktreeId ? (s.gitConflictOperationByWorktree[activeWorktreeId] ?? 'unknown') : 'unknown' ) @@ -1252,6 +1262,22 @@ function SourceControlInner(): React.JSX.Element { // Why: the sidebar stays mounted when closed, so gate polling on tab AND open or branchCompare/PR fetch would run with no visible consumer. const isBranchVisible = rightSidebarTab === 'source-control' && rightSidebarOpen + // Why: the merge base IS the request gate — no OID on the status request means + // the host runs no ranged diff, so a hidden chip costs a background worktree nothing. + const requestedBranchLineTotalMergeBase = + isBranchVisible && !isFolder && branchSummary?.status === 'ready' + ? branchSummary.mergeBase + : null + useEffect(() => { + if (!activeWorktreeId) { + return + } + setBranchLineTotalMergeBase(activeWorktreeId, requestedBranchLineTotalMergeBase) + return () => { + setBranchLineTotalMergeBase(activeWorktreeId, null) + } + }, [activeWorktreeId, requestedBranchLineTotalMergeBase]) + const refreshActiveGitStatus = useCallback( async (signal?: AbortSignal): Promise => { if (!activeWorktreeId || !worktreePath || isFolder) { @@ -5549,6 +5575,7 @@ function SourceControlInner(): React.JSX.Element { diffCommentCount={diffCommentCount} onExpandNotes={() => setDiffCommentsExpanded(true)} branchSummary={branchSummary} + branchLineTotal={branchLineTotal} compareBaseRef={compareBaseRef} headDisplay={gitIdentityDisplay} upstreamStatus={remoteStatus} diff --git a/src/renderer/src/components/right-sidebar/SourceControl.virtual-file-list.test.tsx b/src/renderer/src/components/right-sidebar/SourceControl.virtual-file-list.test.tsx index 7e1d4c7d2..5ff61b17d 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.virtual-file-list.test.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.virtual-file-list.test.tsx @@ -116,6 +116,7 @@ function resetState(overrides: Partial> = {}): void { gitStatusByWorktree: { [mocks.activeWorktree.id]: [] }, gitBranchChangesByWorktree: { [mocks.activeWorktree.id]: [] }, gitBranchCompareSummaryByWorktree: { [mocks.activeWorktree.id]: null }, + gitBranchLineTotalByWorktree: {}, gitConflictOperationByWorktree: {}, remoteStatusesByWorktree: {}, isRemoteOperationActive: false, diff --git a/src/renderer/src/components/right-sidebar/branch-line-total-request-gate.test.ts b/src/renderer/src/components/right-sidebar/branch-line-total-request-gate.test.ts new file mode 100644 index 000000000..600a3cf8f --- /dev/null +++ b/src/renderer/src/components/right-sidebar/branch-line-total-request-gate.test.ts @@ -0,0 +1,52 @@ +import { beforeEach, describe, expect, it } from 'vitest' +import { + clearBranchLineTotalRequestGateForTests, + getBranchLineTotalMergeBase, + setBranchLineTotalMergeBase +} from './branch-line-total-request-gate' + +describe('branch line total request gate', () => { + beforeEach(() => { + clearBranchLineTotalRequestGateForTests() + }) + + it('reports no merge base for a worktree that never opened the chip', () => { + expect(getBranchLineTotalMergeBase('wt-never-gated')).toBeUndefined() + }) + + it('stores the merge base a visible chip asked for', () => { + setBranchLineTotalMergeBase('wt-1', 'merge-base-1') + + expect(getBranchLineTotalMergeBase('wt-1')).toBe('merge-base-1') + }) + + it('replaces the merge base when the fork point moves', () => { + setBranchLineTotalMergeBase('wt-1', 'merge-base-1') + setBranchLineTotalMergeBase('wt-1', 'merge-base-2') + + expect(getBranchLineTotalMergeBase('wt-1')).toBe('merge-base-2') + }) + + it('deletes the entry when the chip is hidden', () => { + setBranchLineTotalMergeBase('wt-1', 'merge-base-1') + setBranchLineTotalMergeBase('wt-1', null) + + expect(getBranchLineTotalMergeBase('wt-1')).toBeUndefined() + }) + + it('treats an empty merge base as no gate rather than an empty request param', () => { + setBranchLineTotalMergeBase('wt-1', 'merge-base-1') + setBranchLineTotalMergeBase('wt-1', '') + + expect(getBranchLineTotalMergeBase('wt-1')).toBeUndefined() + }) + + it('keeps entries independent per worktree', () => { + setBranchLineTotalMergeBase('wt-1', 'merge-base-1') + setBranchLineTotalMergeBase('wt-2', 'merge-base-2') + setBranchLineTotalMergeBase('wt-1', null) + + expect(getBranchLineTotalMergeBase('wt-1')).toBeUndefined() + expect(getBranchLineTotalMergeBase('wt-2')).toBe('merge-base-2') + }) +}) diff --git a/src/renderer/src/components/right-sidebar/branch-line-total-request-gate.ts b/src/renderer/src/components/right-sidebar/branch-line-total-request-gate.ts new file mode 100644 index 000000000..c3fa7b983 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/branch-line-total-request-gate.ts @@ -0,0 +1,23 @@ +// Why: several independent code paths call `git.status`; if only some carried +// the merge base the branch line-total chip would blank out between polls. One +// registry keeps every path asking for the same thing. +// Only the visible worktree's SourceControl is mounted and its effect cleanup +// deletes on every change, so this holds ~1 entry and needs no eviction. +const branchLineTotalMergeBaseByWorktree = new Map() + +/** `null` deletes the entry — that absence is the visibility gate (no merge base ⇒ no host cost). */ +export function setBranchLineTotalMergeBase(worktreeId: string, mergeBase: string | null): void { + if (!mergeBase) { + branchLineTotalMergeBaseByWorktree.delete(worktreeId) + return + } + branchLineTotalMergeBaseByWorktree.set(worktreeId, mergeBase) +} + +export function getBranchLineTotalMergeBase(worktreeId: string): string | undefined { + return branchLineTotalMergeBaseByWorktree.get(worktreeId) +} + +export function clearBranchLineTotalRequestGateForTests(): void { + branchLineTotalMergeBaseByWorktree.clear() +} diff --git a/src/renderer/src/components/right-sidebar/git-status-refresh-branch-line-total.test.ts b/src/renderer/src/components/right-sidebar/git-status-refresh-branch-line-total.test.ts new file mode 100644 index 000000000..3437d8d22 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/git-status-refresh-branch-line-total.test.ts @@ -0,0 +1,224 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + clearBranchLineTotalRequestGateForTests, + setBranchLineTotalMergeBase +} from './branch-line-total-request-gate' +import { + clearGitStatusRefreshOrderingForTests, + refreshGitStatusForWorktree, + refreshGitStatusForWorktreeStrict, + type GitStatusRefreshDeps +} from './git-status-refresh' +import type { GitStatusResult } from '../../../../shared/types' + +const MERGE_BASE = '1f3c0d9a5b6e7f8091a2b3c4d5e6f708192a3b4c' + +function makeDeps(): GitStatusRefreshDeps { + return { + setGitStatus: vi.fn(), + updateWorktreeGitIdentity: vi.fn(), + setUpstreamStatus: vi.fn(), + fetchUpstreamStatus: vi.fn().mockResolvedValue(null) + } +} + +function stubGitStatus(): ReturnType { + const status: GitStatusResult = { + entries: [], + conflictOperation: 'unknown', + upstreamStatus: { hasUpstream: false, ahead: 0, behind: 0 } + } + const gitStatus = vi.fn().mockResolvedValue(status) + vi.stubGlobal('window', { + api: { + git: { + status: gitStatus, + cancelStatus: vi.fn().mockResolvedValue(undefined), + upstreamStatus: vi.fn().mockResolvedValue({ hasUpstream: false, ahead: 0, behind: 0 }) + } + } + }) + return gitStatus +} + +describe('branch line total request gate on git status refreshes', () => { + beforeEach(() => { + vi.unstubAllGlobals() + clearGitStatusRefreshOrderingForTests() + clearBranchLineTotalRequestGateForTests() + }) + + it('omits the merge base when no chip is asking for a total', async () => { + const gitStatus = stubGitStatus() + + await refreshGitStatusForWorktree({ + worktreeId: 'wt-hidden', + worktreePath: '/repo', + connectionId: 'ssh-1', + deps: makeDeps() + }) + + // Why: no OID on the request is the whole performance contract — the host + // runs no ranged diff, so a background worktree costs nothing. + expect(gitStatus).toHaveBeenCalledWith({ + worktreePath: '/repo', + connectionId: 'ssh-1' + }) + }) + + it('omits the merge base for a worktree whose chip was hidden again', async () => { + const gitStatus = stubGitStatus() + setBranchLineTotalMergeBase('wt-1', MERGE_BASE) + setBranchLineTotalMergeBase('wt-1', null) + + await refreshGitStatusForWorktree({ + worktreeId: 'wt-1', + worktreePath: '/repo', + deps: makeDeps() + }) + + expect(gitStatus).toHaveBeenCalledWith({ + worktreePath: '/repo', + connectionId: undefined + }) + }) + + it('does not send one worktree gate on another worktree status pass', async () => { + const gitStatus = stubGitStatus() + setBranchLineTotalMergeBase('wt-visible', MERGE_BASE) + + await refreshGitStatusForWorktree({ + worktreeId: 'wt-background', + worktreePath: '/other-repo', + deps: makeDeps() + }) + + expect(gitStatus).toHaveBeenCalledWith({ + worktreePath: '/other-repo', + connectionId: undefined + }) + }) + + it('sends the merge base on a plain refresh that passes no request options', async () => { + const gitStatus = stubGitStatus() + setBranchLineTotalMergeBase('wt-1', MERGE_BASE) + + // Why: this path used to build no options object at all, so the gate was + // silently dropped on the most common (fs-watcher) refresh. + await refreshGitStatusForWorktree({ + worktreeId: 'wt-1', + worktreePath: '/repo', + connectionId: 'ssh-1', + deps: makeDeps() + }) + + expect(gitStatus).toHaveBeenCalledWith({ + worktreePath: '/repo', + connectionId: 'ssh-1', + branchLineTotalMergeBase: MERGE_BASE + }) + }) + + it('sends the merge base alongside line-stat reuse', async () => { + const gitStatus = stubGitStatus() + setBranchLineTotalMergeBase('wt-1', MERGE_BASE) + + await refreshGitStatusForWorktree({ + worktreeId: 'wt-1', + worktreePath: '/repo', + deps: makeDeps(), + request: { reuseLineStats: true } + }) + + expect(gitStatus).toHaveBeenCalledWith({ + worktreePath: '/repo', + connectionId: undefined, + reuseLineStats: true, + branchLineTotalMergeBase: MERGE_BASE + }) + }) + + it('sends the merge base on an abortable refresh', async () => { + const gitStatus = stubGitStatus() + setBranchLineTotalMergeBase('wt-1', MERGE_BASE) + const controller = new AbortController() + + await refreshGitStatusForWorktree({ + worktreeId: 'wt-1', + worktreePath: '/repo', + deps: makeDeps(), + request: { signal: controller.signal } + }) + + expect(gitStatus).toHaveBeenCalledWith( + expect.objectContaining({ + worktreePath: '/repo', + branchLineTotalMergeBase: MERGE_BASE + }) + ) + }) + + it('re-reads the gate on every pass so a moved fork point is not sent twice', async () => { + const gitStatus = stubGitStatus() + setBranchLineTotalMergeBase('wt-1', MERGE_BASE) + + await refreshGitStatusForWorktree({ + worktreeId: 'wt-1', + worktreePath: '/repo', + deps: makeDeps() + }) + setBranchLineTotalMergeBase('wt-1', 'rebased-merge-base') + await refreshGitStatusForWorktree({ + worktreeId: 'wt-1', + worktreePath: '/repo', + deps: makeDeps() + }) + + expect(gitStatus).toHaveBeenNthCalledWith(1, { + worktreePath: '/repo', + connectionId: undefined, + branchLineTotalMergeBase: MERGE_BASE + }) + expect(gitStatus).toHaveBeenNthCalledWith(2, { + worktreePath: '/repo', + connectionId: undefined, + branchLineTotalMergeBase: 'rebased-merge-base' + }) + }) + + it('omits the merge base from a strict refresh when no chip is visible', async () => { + const gitStatus = stubGitStatus() + + await refreshGitStatusForWorktreeStrict({ + worktreeId: 'wt-strict', + worktreePath: '/repo', + deps: { ...makeDeps(), fetchUpstreamStatus: undefined } + }) + + expect(gitStatus).toHaveBeenCalledWith({ + worktreePath: '/repo', + connectionId: undefined, + bypassEffectiveUpstreamNegativeCache: true + }) + }) + + // Why: a strict refresh is exactly the commit/push/sync that moves the number; + // skipping the gate there would blank the chip until the next automatic poll. + it('sends the merge base on a strict refresh', async () => { + const gitStatus = stubGitStatus() + setBranchLineTotalMergeBase('wt-strict', MERGE_BASE) + + await refreshGitStatusForWorktreeStrict({ + worktreeId: 'wt-strict', + worktreePath: '/repo', + deps: { ...makeDeps(), fetchUpstreamStatus: undefined } + }) + + expect(gitStatus).toHaveBeenCalledWith({ + worktreePath: '/repo', + connectionId: undefined, + bypassEffectiveUpstreamNegativeCache: true, + branchLineTotalMergeBase: MERGE_BASE + }) + }) +}) diff --git a/src/renderer/src/components/right-sidebar/git-status-refresh.ts b/src/renderer/src/components/right-sidebar/git-status-refresh.ts index 596a68f8d..50f2cf068 100644 --- a/src/renderer/src/components/right-sidebar/git-status-refresh.ts +++ b/src/renderer/src/components/right-sidebar/git-status-refresh.ts @@ -1,4 +1,5 @@ import { getRuntimeGitStatus, getRuntimeGitUpstreamStatus } from '@/runtime/runtime-git-client' +import { getBranchLineTotalMergeBase } from './branch-line-total-request-gate' import { clearAutomaticPushTargetUpstreamStatusCache, getCachedAutomaticPushTargetUpstreamStatus, @@ -121,6 +122,9 @@ export async function refreshGitStatusForWorktree({ } }): Promise { const refreshOrder = beginAutomaticUpstreamRefresh(worktreeId) + // Why: every status path must carry the merge base, or the chip blanks out on + // whichever poll happened to omit it. + const branchLineTotalMergeBase = getBranchLineTotalMergeBase(worktreeId) try { const status = (await getRuntimeGitStatus( { @@ -129,12 +133,11 @@ export async function refreshGitStatusForWorktree({ worktreePath, connectionId }, - request - ? { - ...(request.reuseLineStats === true ? { reuseLineStats: true } : {}), - ...(request.signal ? { signal: request.signal } : {}) - } - : undefined + { + ...(request?.reuseLineStats === true ? { reuseLineStats: true } : {}), + ...(request?.signal ? { signal: request.signal } : {}), + ...(branchLineTotalMergeBase ? { branchLineTotalMergeBase } : {}) + } )) as GitStatusResult if (!claimAutomaticUpstreamRefreshApply(worktreeId, refreshOrder, request?.shouldApply)) { @@ -247,6 +250,7 @@ export async function refreshGitStatusForWorktreeStrict({ }): Promise<{ status: GitStatusResult; upstreamStatus: GitUpstreamStatus }> { beginStrictUpstreamRefresh(worktreeId) clearAutomaticPushTargetUpstreamStatusCache() + const strictBranchLineTotalMergeBase = getBranchLineTotalMergeBase(worktreeId) const status = (await getRuntimeGitStatus( { settings, @@ -257,7 +261,12 @@ export async function refreshGitStatusForWorktreeStrict({ { // Why: strict refreshes are user-triggered reconciliation and must not reuse // automatic polling's no-upstream backoff window. - bypassEffectiveUpstreamNegativeCache: true + bypassEffectiveUpstreamNegativeCache: true, + // Why: an absent total clears the chip, so a path that skipped the gate + // would blank it right after the commit or push that moved the number. + ...(strictBranchLineTotalMergeBase + ? { branchLineTotalMergeBase: strictBranchLineTotalMergeBase } + : {}) } )) as GitStatusResult diff --git a/src/renderer/src/components/right-sidebar/source-control-branch-context-row.test.tsx b/src/renderer/src/components/right-sidebar/source-control-branch-context-row.test.tsx index 4a4542249..0172dab5d 100644 --- a/src/renderer/src/components/right-sidebar/source-control-branch-context-row.test.tsx +++ b/src/renderer/src/components/right-sidebar/source-control-branch-context-row.test.tsx @@ -3,6 +3,7 @@ import type { ReactNode } from 'react' import { describe, expect, it, vi } from 'vitest' import { SourceControlBranchContextRow } from './source-control-branch-context-row' import type { GitBranchCompareSummary } from '../../../../shared/types' +import type { GitBranchLineTotal } from '../../../../shared/git-status-types' vi.mock('@/components/ui/tooltip', () => ({ Tooltip: ({ children }: { children: ReactNode }) => <>{children}, @@ -215,3 +216,144 @@ describe('SourceControlBranchContextRow', () => { expect(markup).toContain('aria-label="Open review page in browser"') }) }) + +function renderWithLineTotal( + branchLineTotal: GitBranchLineTotal | null | undefined, + summary: GitBranchCompareSummary | null = readySummary +): string { + return renderToStaticMarkup( + + ) +} + +describe('SourceControlBranchContextRow branch line total', () => { + it('renders both halves with grouped digits and a spoken label', () => { + const markup = renderWithLineTotal({ added: 8259, removed: 670, mergeBase: 'base' }) + + expect(markup).toContain('+8,259') + expect(markup).toContain('-670') + // Label reads raw digits; the grouped spans are decoration. + expect(markup).toContain('aria-label="8259 additions, 670 deletions"') + expect(markup).toContain('tabular-nums') + expect(markup).toContain('text-[color:var(--git-decoration-added)]') + expect(markup).toContain('text-[color:var(--git-decoration-deleted)]') + // Not clickable in v1 — the chip's scope differs from openBranchAllDiffs. + expect(markup).not.toContain('