From 9b45846584024c1cb0752cb44a0ce82c767944b4 Mon Sep 17 00:00:00 2001 From: lvfen <284437334@qq.com> Date: Thu, 25 Jun 2026 15:14:11 +0800 Subject: [PATCH 1/9] feat(source-control): show submodule diffs with lazy expansion Dirty submodules now expand inline in Source Control to reveal their inner changes, with file-level diffs that are read-only from the parent worktree. Inner status is fetched lazily only when a submodule is expanded, so status polling never recurses into (possibly nested) submodules. Adds a submodule-status path across local and SSH runtimes and git providers. --- src/main/git/status.test.ts | 182 +++++++++- src/main/git/status.ts | 335 ++++++++++++++++++ src/main/ipc/filesystem.ts | 27 ++ src/main/providers/ssh-git-provider.ts | 7 + src/main/providers/types.ts | 1 + src/main/runtime/orca-runtime-git.ts | 20 ++ src/main/runtime/orca-runtime.ts | 2 + src/main/runtime/rpc/methods/git-params.ts | 13 + src/main/runtime/rpc/methods/git.ts | 7 + src/preload/api-types.ts | 5 + src/preload/index.ts | 5 + src/relay/git-handler-submodule-ops.ts | 196 ++++++++++ src/relay/git-handler.test.ts | 131 +++++++ src/relay/git-handler.ts | 100 +++++- .../right-sidebar/SourceControl.tsx | 213 ++++++++++- .../discard-all-sequence.test.ts | 13 + .../right-sidebar/discard-all-sequence.ts | 3 + ...source-control-submodule-expansion.test.ts | 295 +++++++++++++++ .../source-control-submodule-expansion.ts | 215 +++++++++++ .../src/runtime/runtime-git-client.ts | 23 ++ src/renderer/src/web/web-preload-api.ts | 7 + src/shared/git-status-types.ts | 5 + 22 files changed, 1793 insertions(+), 12 deletions(-) create mode 100644 src/relay/git-handler-submodule-ops.ts create mode 100644 src/renderer/src/components/right-sidebar/source-control-submodule-expansion.test.ts create mode 100644 src/renderer/src/components/right-sidebar/source-control-submodule-expansion.ts diff --git a/src/main/git/status.test.ts b/src/main/git/status.test.ts index 97b4c40d8e..8967cbf412 100644 --- a/src/main/git/status.test.ts +++ b/src/main/git/status.test.ts @@ -68,6 +68,7 @@ import { bulkDiscardChanges, bulkUnstageFiles, clearEffectiveUpstreamStatusCacheForTests, + clearSubmodulePathsCacheForTests, detectConflictOperation, discardChanges, getBranchCompare, @@ -75,7 +76,9 @@ import { getDiff, getStagedCommitContext, getStatus, - isWithinWorktree + getSubmoduleStatus, + isWithinWorktree, + resolveSubmoduleWorktreePath } from './status' describe('discardChanges', () => { @@ -446,6 +449,183 @@ describe('getDiff', () => { }) }) +describe('submodule diff routing', () => { + const OLD_OID = 'a'.repeat(40) + const NEW_OID = 'b'.repeat(40) + const PARENT = path.resolve('/repo-sm') + const SUBMODULE = path.join(PARENT, 'flutter_mine') + + beforeEach(() => { + clearSubmodulePathsCacheForTests() + gitExecFileAsyncMock.mockReset() + gitExecFileAsyncBufferMock.mockReset() + lstatMock.mockReset() + readFileMock.mockReset() + statMock.mockReset() + existsSyncMock.mockReset() + statMock.mockResolvedValue({ isFile: () => true, size: 12 }) + // Route git invocations by argv/cwd so the routing logic can resolve the + // gitlink oids without touching a real repo. + gitExecFileAsyncMock.mockImplementation((args: string[], options?: { cwd?: string }) => { + if (args[0] === 'config' && args.includes('.gitmodules')) { + // Only the parent worktree declares the submodule; the recursion into the + // submodule worktree must see no nested submodules. + return Promise.resolve({ + stdout: options?.cwd === PARENT ? 'submodule.flutter_mine.path flutter_mine\n' : '' + }) + } + if (args[0] === 'ls-files') { + return Promise.resolve({ stdout: `160000 ${OLD_OID} 0\tflutter_mine\n` }) + } + if (args[0] === 'ls-tree') { + return Promise.resolve({ stdout: `160000 commit ${OLD_OID}\tflutter_mine\n` }) + } + if (args[0] === 'rev-parse') { + return Promise.resolve({ stdout: `${NEW_OID}\n` }) + } + return Promise.resolve({ stdout: '' }) + }) + }) + + it('synthesizes a Subproject commit pointer diff for the gitlink root', async () => { + const result = await getDiff(PARENT, 'flutter_mine', false) + + expect(result.kind).toBe('text') + expect(result.originalContent).toBe(`Subproject commit ${OLD_OID}\n`) + expect(result.modifiedContent).toBe(`Subproject commit ${NEW_OID}\n`) + }) + + it('diffs inner files across the two commits when the gitlink moved', async () => { + gitExecFileAsyncBufferMock.mockImplementation((args: string[]) => { + const spec = String(args.at(-1)) + if (spec.startsWith(`${OLD_OID}:`)) { + return Promise.resolve({ stdout: Buffer.from('v1\n') }) + } + if (spec.startsWith(`${NEW_OID}:`)) { + return Promise.resolve({ stdout: Buffer.from('v2\n') }) + } + return Promise.resolve({ stdout: Buffer.from('') }) + }) + + const result = await getDiff(PARENT, 'flutter_mine/lib/main.dart', false) + + expect(gitExecFileAsyncBufferMock).toHaveBeenCalledWith( + ['show', '--end-of-options', `${OLD_OID}:lib/main.dart`], + { cwd: SUBMODULE, maxBuffer: 10 * 1024 * 1024 } + ) + expect(gitExecFileAsyncBufferMock).toHaveBeenCalledWith( + ['show', '--end-of-options', `${NEW_OID}:lib/main.dart`], + { cwd: SUBMODULE, maxBuffer: 10 * 1024 * 1024 } + ) + expect(result.kind).toBe('text') + expect(result.originalContent).toBe('v1\n') + expect(result.modifiedContent).toBe('v2\n') + }) + + it('reads inner files from the working tree when the commit is unchanged', async () => { + // Override the gitlink oids so recorded == checked-out (no pointer move), + // routing the inner diff back to the index/working-tree blob read. + gitExecFileAsyncMock.mockImplementation((args: string[], options?: { cwd?: string }) => { + if (args[0] === 'config' && args.includes('.gitmodules')) { + return Promise.resolve({ + stdout: options?.cwd === PARENT ? 'submodule.flutter_mine.path flutter_mine\n' : '' + }) + } + if (args[0] === 'ls-files') { + return Promise.resolve({ stdout: `160000 ${OLD_OID} 0\tflutter_mine\n` }) + } + if (args[0] === 'rev-parse') { + return Promise.resolve({ stdout: `${OLD_OID}\n` }) + } + return Promise.resolve({ stdout: '' }) + }) + gitExecFileAsyncBufferMock.mockResolvedValueOnce({ stdout: Buffer.from('old\n') }) + readFileMock.mockResolvedValue(Buffer.from('new')) + + const result = await getDiff(PARENT, 'flutter_mine/lib/main.dart', false) + + expect(gitExecFileAsyncBufferMock).toHaveBeenCalledWith(['show', ':lib/main.dart'], { + cwd: SUBMODULE, + maxBuffer: 10 * 1024 * 1024 + }) + expect(readFileMock).toHaveBeenCalledWith(path.join(SUBMODULE, 'lib/main.dart')) + expect(result.kind).toBe('text') + expect(result.originalContent).toBe('old\n') + expect(result.modifiedContent).toBe('new') + }) +}) + +describe('resolveSubmoduleWorktreePath', () => { + it('resolves a relative submodule path inside the worktree', () => { + expect(resolveSubmoduleWorktreePath('/repo', 'flutter_mine')).toBe( + path.resolve('/repo', 'flutter_mine') + ) + }) + + it('rejects absolute and escaping submodule paths', () => { + expect(() => resolveSubmoduleWorktreePath('/repo', path.resolve('/etc'))).toThrow( + 'Access denied' + ) + expect(() => resolveSubmoduleWorktreePath('/repo', '../outside')).toThrow('Access denied') + }) +}) + +describe('getSubmoduleStatus', () => { + beforeEach(() => { + clearEffectiveUpstreamStatusCacheForTests() + gitExecFileAsyncMock.mockReset() + gitExecFileAsyncBufferMock.mockReset() + lstatMock.mockReset() + readFileMock.mockReset() + existsSyncMock.mockReset() + gitExecFileAsyncMock.mockResolvedValue({ stdout: '' }) + }) + + it('runs status inside the submodule worktree and returns inner entries', async () => { + readFileMock.mockResolvedValue('gitdir: /repo/flutter_mine/.git\n') + existsSyncMock.mockReturnValue(false) + gitExecFileAsyncMock.mockResolvedValueOnce({ + stdout: + '1 .M N... 100644 100644 100644 ce013625030ba8dba906f756967f9e9ca394464a ce013625030ba8dba906f756967f9e9ca394464a lib/main.dart\n' + }) + + const result = await getSubmoduleStatus('/repo', 'flutter_mine') + + expect(result.entries).toContainEqual({ + path: 'lib/main.dart', + status: 'modified', + area: 'unstaged' + }) + }) + + it('includes commit-range entries when the submodule pointer moved', async () => { + const OLD_OID = 'a'.repeat(40) + const NEW_OID = 'b'.repeat(40) + readFileMock.mockResolvedValue('gitdir: /repo/flutter_mine/.git\n') + existsSyncMock.mockReturnValue(false) + gitExecFileAsyncMock.mockReset() + gitExecFileAsyncMock.mockImplementation((args: string[]) => { + // Clean worktree: the inner status stream returns nothing. + if (args.includes('--name-status')) { + return Promise.resolve({ stdout: 'M\tlib/main.dart\n' }) + } + if (args[0] === 'ls-files') { + return Promise.resolve({ stdout: `160000 ${OLD_OID} 0\tflutter_mine\n` }) + } + if (args[0] === 'rev-parse') { + return Promise.resolve({ stdout: `${NEW_OID}\n` }) + } + return Promise.resolve({ stdout: '' }) + }) + + const result = await getSubmoduleStatus('/repo', 'flutter_mine') + + expect(result.entries).toContainEqual( + expect.objectContaining({ path: 'lib/main.dart', status: 'modified', area: 'unstaged' }) + ) + }) +}) + describe('getStatus', () => { beforeEach(() => { clearEffectiveUpstreamStatusCacheForTests() diff --git a/src/main/git/status.ts b/src/main/git/status.ts index 8876fb18f5..e39607ac39 100644 --- a/src/main/git/status.ts +++ b/src/main/git/status.ts @@ -60,11 +60,19 @@ type EffectiveUpstreamStatusCacheEntry = { status: GitUpstreamStatus } +const SUBMODULE_PATHS_CACHE_TTL_MS = 5_000 +type SubmodulePathsCacheEntry = { paths: string[]; expiresAt: number } +const submodulePathsCache = new Map() + const effectiveUpstreamStatusCache = new Map() const effectiveUpstreamStatusInFlight = new Map>() const retiredEffectiveUpstreamStatusInFlight = new Map>() const effectiveUpstreamStatusWriteGeneration = new Map() +export function clearSubmodulePathsCacheForTests(): void { + submodulePathsCache.clear() +} + export function clearEffectiveUpstreamStatusCacheForTests(): void { effectiveUpstreamStatusCache.clear() effectiveUpstreamStatusInFlight.clear() @@ -226,6 +234,116 @@ export async function getStatus( } } +/** + * Resolve a submodule's own worktree path from a parent worktree + relative + * submodule path, rejecting anything that escapes the parent. Shared by the + * on-demand submodule status query and the submodule-aware diff router. + */ +export function resolveSubmoduleWorktreePath(worktreePath: string, submodulePath: string): string { + if (!submodulePath || submodulePath.includes('\0') || path.isAbsolute(submodulePath)) { + throw new Error('Access denied: invalid submodule path') + } + const resolved = path.resolve(worktreePath, submodulePath) + const rel = path.relative(worktreePath, resolved) + if (!rel || rel === '..' || rel.startsWith(`..${path.sep}`) || path.isAbsolute(rel)) { + throw new Error('Access denied: submodule path escapes the selected worktree') + } + return resolved +} + +/** + * Run a plain status inside a submodule's own worktree. Used by the lazy + * "expand submodule" flow — the parent status only reports a single gitlink + * row, so the inner per-file changes are fetched on demand here. Entry paths + * are relative to the submodule root; the renderer prefixes them with the + * submodule path. + */ +export async function getSubmoduleStatus( + worktreePath: string, + submodulePath: string, + options: GitRuntimeOptions = {} +): Promise { + const submoduleWorktreePath = resolveSubmoduleWorktreePath(worktreePath, submodulePath) + const workingResult = await getStatus(submoduleWorktreePath, options) + // Why: a moved gitlink (clean worktree) has no uncommitted status rows; its + // real changes live between the parent-recorded commit and the checked-out + // commit. Surface those as inner rows so the expansion isn't empty. + const fromOid = + (await readGitlinkOidFromIndex(worktreePath, submodulePath, options)) || + (await readGitlinkOidFromTree(worktreePath, 'HEAD', submodulePath, options)) + const toOid = await readWorkingSubmoduleHead(submoduleWorktreePath, options) + if (fromOid && toOid && fromOid !== toOid) { + const rangeEntries = await computeSubmoduleRangeEntries( + submoduleWorktreePath, + fromOid, + toOid, + options + ) + const rangePaths = new Set(rangeEntries.map((entry) => entry.path)) + // Range rows win on overlap so the diff matches getDiff's commit-range route. + const entries = [ + ...rangeEntries, + ...workingResult.entries.filter((entry) => !rangePaths.has(entry.path)) + ] + return { ...workingResult, entries } + } + return workingResult +} + +/** + * List files changed between two submodule commits as status rows. Used when a + * gitlink pointer moved so the expanded submodule shows the committed file + * changes (each row diffs the file across the two commits). + */ +async function computeSubmoduleRangeEntries( + submoduleWorktreePath: string, + fromOid: string, + toOid: string, + options: GitRuntimeOptions = {} +): Promise { + const gitOptions = { + ...gitOptionsForWorktree(submoduleWorktreePath, options), + env: gitOptionalLocksDisabledEnv() + } + let nameStatus = '' + let numstat = '' + try { + const [statusResult, numstatResult] = await Promise.all([ + gitExecFileAsync( + ['-c', 'core.quotePath=false', 'diff', '--name-status', '-M', '-C', fromOid, toOid], + gitOptions + ), + gitExecFileAsync( + ['-c', 'core.quotePath=false', 'diff', '-z', '--numstat', '-M', '-C', fromOid, toOid], + gitOptions + ) + ]) + nameStatus = statusResult.stdout + numstat = numstatResult.stdout + } catch { + return [] + } + const statsByPath = parseNumstat(numstat) + const entries: GitStatusEntry[] = [] + for (const line of nameStatus.split(/\r?\n/)) { + if (!line) { + continue + } + const change = parseBranchChangeLine(line) + if (!change) { + continue + } + entries.push({ + path: change.path, + status: change.status, + area: 'unstaged', + ...(change.oldPath ? { oldPath: change.oldPath } : {}), + ...statsByPath.get(change.path) + }) + } + return entries +} + async function runNumstat( worktreePath: string, cached: boolean, @@ -671,6 +789,183 @@ export async function resolveGitDir(worktreePath: string): Promise { return dotGitPath } +/** + * List configured submodule paths (relative, forward-slash) for a worktree, + * cached briefly. Read from `.gitmodules` so a single diff click doesn't pay + * for an index-wide `ls-files` scan. Used to route gitlink/inner diffs. + */ +export async function listSubmodulePaths( + worktreePath: string, + options: GitRuntimeOptions = {} +): Promise { + const now = Date.now() + const cached = submodulePathsCache.get(worktreePath) + if (cached && cached.expiresAt > now) { + return cached.paths + } + let paths: string[] = [] + try { + const { stdout } = await gitExecFileAsync( + ['config', '--file', '.gitmodules', '--get-regexp', '^submodule\\..*\\.path$'], + { ...gitOptionsForWorktree(worktreePath, options), env: gitOptionalLocksDisabledEnv() } + ) + paths = stdout + .split(/\r?\n/) + .map((line) => { + const spaceIndex = line.indexOf(' ') + return spaceIndex === -1 + ? '' + : line + .slice(spaceIndex + 1) + .trim() + .replace(/\/+$/, '') + }) + .filter((value) => value.length > 0) + } catch { + // No .gitmodules (or git config failure) — treat as a repo without submodules. + paths = [] + } + submodulePathsCache.set(worktreePath, { paths, expiresAt: now + SUBMODULE_PATHS_CACHE_TTL_MS }) + return paths +} + +/** + * Find the submodule whose root equals or contains `filePath`. Returns the + * submodule path (forward-slash) or null when the path is not in a submodule. + */ +function findContainingSubmodule(submodulePaths: string[], filePath: string): string | null { + const normalized = filePath.replace(/\\/g, '/').replace(/\/+$/, '') + let best: string | null = null + for (const sub of submodulePaths) { + if (normalized === sub || normalized.startsWith(`${sub}/`)) { + // Prefer the longest match to support nested submodule roots. + if (!best || sub.length > best.length) { + best = sub + } + } + } + return best +} + +async function readGitlinkOidFromTree( + worktreePath: string, + ref: string, + submodulePath: string, + options: GitRuntimeOptions +): Promise { + try { + const { stdout } = await gitExecFileAsync(['ls-tree', ref, '--', submodulePath], { + ...gitOptionsForWorktree(worktreePath, options), + env: gitOptionalLocksDisabledEnv() + }) + return stdout.match(/^160000 commit ([0-9a-f]+)\t/m)?.[1] ?? '' + } catch { + return '' + } +} + +async function readGitlinkOidFromIndex( + worktreePath: string, + submodulePath: string, + options: GitRuntimeOptions +): Promise { + try { + const { stdout } = await gitExecFileAsync(['ls-files', '-s', '--', submodulePath], { + ...gitOptionsForWorktree(worktreePath, options), + env: gitOptionalLocksDisabledEnv() + }) + return stdout.match(/^160000 ([0-9a-f]+) /m)?.[1] ?? '' + } catch { + return '' + } +} + +async function readWorkingSubmoduleHead( + submoduleWorktreePath: string, + options: GitRuntimeOptions +): Promise { + try { + const { stdout } = await gitExecFileAsync(['rev-parse', 'HEAD'], { + ...gitOptionsForWorktree(submoduleWorktreePath, options), + env: gitOptionalLocksDisabledEnv() + }) + return stdout.trim() + } catch { + return '' + } +} + +/** + * Synthesize a gitlink pointer diff. Git represents submodule commit changes as + * a one-line `Subproject commit ` swap, so feeding the old/new oids through + * the normal text differ matches git's own rendering. + */ +async function buildSubmodulePointerDiff( + worktreePath: string, + submodulePath: string, + staged: boolean, + compareAgainstHead: boolean, + options: GitRuntimeOptions +): Promise { + const submoduleWorktreePath = path.join(worktreePath, submodulePath) + let leftOid = '' + let rightOid = '' + if (staged) { + leftOid = await readGitlinkOidFromTree(worktreePath, 'HEAD', submodulePath, options) + rightOid = await readGitlinkOidFromIndex(worktreePath, submodulePath, options) + } else if (compareAgainstHead) { + leftOid = await readGitlinkOidFromTree(worktreePath, 'HEAD', submodulePath, options) + rightOid = await readWorkingSubmoduleHead(submoduleWorktreePath, options) + } else { + leftOid = + (await readGitlinkOidFromIndex(worktreePath, submodulePath, options)) || + (await readGitlinkOidFromTree(worktreePath, 'HEAD', submodulePath, options)) + rightOid = await readWorkingSubmoduleHead(submoduleWorktreePath, options) + } + return buildDiffResult( + leftOid ? `Subproject commit ${leftOid}\n` : '', + rightOid ? `Subproject commit ${rightOid}\n` : '', + false, + false, + submodulePath + ) +} + +/** + * Diff a file inside a submodule across two of its commits. Used when the parent + * gitlink moved but the submodule worktree is clean — the change is committed, + * so compare the recorded commit's blob against the checked-out commit's blob. + */ +async function buildSubmoduleInnerCommitRangeDiff( + submoduleWorktreePath: string, + innerPath: string, + fromOid: string, + toOid: string, + options: GitRuntimeOptions +): Promise { + let originalContent = '' + let modifiedContent = '' + let originalIsBinary = false + let modifiedIsBinary = false + try { + const left = await readGitBlobAtOidPath(submoduleWorktreePath, fromOid, innerPath, options) + originalContent = left.content + originalIsBinary = left.isBinary + const right = await readGitBlobAtOidPath(submoduleWorktreePath, toOid, innerPath, options) + modifiedContent = right.content + modifiedIsBinary = right.isBinary + } catch { + // Fallback to empty content; a missing blob (add/delete) reads as one side. + } + return buildDiffResult( + originalContent, + modifiedContent, + originalIsBinary, + modifiedIsBinary, + innerPath + ) +} + /** * Get original and modified content for diffing a file. */ @@ -681,6 +976,46 @@ export async function getDiff( compareAgainstHead = false, options: GitRuntimeOptions = {} ): Promise { + // Why: gitlink paths can't be read as blobs (`git show HEAD:` is a "bad + // object") and a submodule working dir reads as empty, so route submodule + // diffs explicitly: the gitlink root → pointer diff, inner files → recurse + // into the submodule's own worktree. + const submodulePaths = await listSubmodulePaths(worktreePath, options) + if (submodulePaths.length > 0) { + const matchedSubmodule = findContainingSubmodule(submodulePaths, filePath) + if (matchedSubmodule) { + const normalizedFilePath = filePath.replace(/\\/g, '/').replace(/\/+$/, '') + if (normalizedFilePath === matchedSubmodule) { + return buildSubmodulePointerDiff( + worktreePath, + matchedSubmodule, + staged, + compareAgainstHead, + options + ) + } + const innerPath = normalizedFilePath.slice(matchedSubmodule.length + 1) + const submoduleWorktreePath = path.join(worktreePath, matchedSubmodule) + const fromOid = + (await readGitlinkOidFromIndex(worktreePath, matchedSubmodule, options)) || + (await readGitlinkOidFromTree(worktreePath, 'HEAD', matchedSubmodule, options)) + const toOid = await readWorkingSubmoduleHead(submoduleWorktreePath, options) + // Why: when the gitlink moved but the submodule worktree is clean, the + // file's change lives in committed history — diff the two commits. Only + // fall back to the working-tree blob read when the commit didn't move. + if (fromOid && toOid && fromOid !== toOid) { + return buildSubmoduleInnerCommitRangeDiff( + submoduleWorktreePath, + innerPath, + fromOid, + toOid, + options + ) + } + return getDiff(submoduleWorktreePath, innerPath, staged, compareAgainstHead, options) + } + } + let originalContent = '' let modifiedContent = '' let originalIsBinary = false diff --git a/src/main/ipc/filesystem.ts b/src/main/ipc/filesystem.ts index 021ee8f4f1..46f0b4d2c6 100644 --- a/src/main/ipc/filesystem.ts +++ b/src/main/ipc/filesystem.ts @@ -36,6 +36,7 @@ import { } from '../../shared/text-search' import { getStatus, + getSubmoduleStatus, abortMerge, abortRebase, detectConflictOperation, @@ -859,6 +860,32 @@ export function registerFilesystemHandlers( } ) + // Why: the parent status only reports one gitlink row per submodule. When the + // user expands a dirty submodule, this fetches the inner per-file changes by + // running a plain status inside the submodule's own worktree (read-only). + ipcMain.handle( + 'git:submoduleStatus', + async ( + _event, + args: { worktreePath: string; submodulePath: string; connectionId?: string } + ): Promise => { + if (args.connectionId) { + const provider = getSshGitProvider(args.connectionId) + if (!provider) { + throw new Error(SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE) + } + return provider.getSubmoduleStatus(args.worktreePath, args.submodulePath) + } + const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store) + const gitOptions = getLocalGitOptionsForRegisteredWorktree( + store, + args.worktreePath, + worktreePath + ) + return getSubmoduleStatus(worktreePath, args.submodulePath, gitOptions) + } + ) + ipcMain.handle( 'git:checkIgnored', async ( diff --git a/src/main/providers/ssh-git-provider.ts b/src/main/providers/ssh-git-provider.ts index 9c93c46e6d..2194cb16ef 100644 --- a/src/main/providers/ssh-git-provider.ts +++ b/src/main/providers/ssh-git-provider.ts @@ -95,6 +95,13 @@ export class SshGitProvider implements IGitProvider { })) as GitStatusResult } + async getSubmoduleStatus(worktreePath: string, submodulePath: string): Promise { + return (await this.mux.request('git.submoduleStatus', { + worktreePath, + submodulePath + })) as GitStatusResult + } + async checkIgnoredPaths(worktreePath: string, relativePaths: string[]): Promise { return (await this.mux.request('git.checkIgnored', { worktreePath, diff --git a/src/main/providers/types.ts b/src/main/providers/types.ts index c8a090e59f..572d64151c 100644 --- a/src/main/providers/types.ts +++ b/src/main/providers/types.ts @@ -172,6 +172,7 @@ export type GitProviderStatusOptions = { export type IGitProvider = { getStatus(worktreePath: string, options?: GitProviderStatusOptions): Promise + getSubmoduleStatus(worktreePath: string, submodulePath: string): Promise checkIgnoredPaths(worktreePath: string, relativePaths: string[]): Promise getHistory(worktreePath: string, options?: GitHistoryOptions): Promise commit(worktreePath: string, message: string): Promise<{ success: boolean; error?: string }> diff --git a/src/main/runtime/orca-runtime-git.ts b/src/main/runtime/orca-runtime-git.ts index e39ef000ee..e7b64cd860 100644 --- a/src/main/runtime/orca-runtime-git.ts +++ b/src/main/runtime/orca-runtime-git.ts @@ -41,6 +41,7 @@ import { getDiff, getStagedCommitContext, getStatus as getGitStatus, + getSubmoduleStatus as getGitSubmoduleStatus, stageFile, unstageFile } from '../git/status' @@ -183,6 +184,25 @@ export class RuntimeGitCommands { : getGitStatus(target.worktree.path, gitOptions) } + async getRuntimeGitSubmoduleStatus( + worktreeSelector: string, + submodulePath: string + ): Promise { + const target = await this.host.resolveRuntimeGitTarget(worktreeSelector) + const provider = target.connectionId ? getSshGitProvider(target.connectionId) : null + if (target.connectionId) { + if (!provider) { + throw new Error(SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE) + } + return provider.getSubmoduleStatus(target.worktree.path, submodulePath) + } + return getGitSubmoduleStatus( + target.worktree.path, + submodulePath, + localGitOptionsForTarget(target) + ) + } + async checkRuntimeGitIgnoredPaths( worktreeSelector: string, relativePaths: string[] diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index fef93b96e1..9959e24208 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -4627,6 +4627,8 @@ export class OrcaRuntimeService { getRuntimeGitStatus: RuntimeGitCommands['getRuntimeGitStatus'] = this.gitCommands.getRuntimeGitStatus.bind(this.gitCommands) + getRuntimeGitSubmoduleStatus: RuntimeGitCommands['getRuntimeGitSubmoduleStatus'] = + this.gitCommands.getRuntimeGitSubmoduleStatus.bind(this.gitCommands) checkRuntimeGitIgnoredPaths: RuntimeGitCommands['checkRuntimeGitIgnoredPaths'] = this.gitCommands.checkRuntimeGitIgnoredPaths.bind(this.gitCommands) getRuntimeGitHistory: RuntimeGitCommands['getRuntimeGitHistory'] = diff --git a/src/main/runtime/rpc/methods/git-params.ts b/src/main/runtime/rpc/methods/git-params.ts index c94c3349c6..7814a0f221 100644 --- a/src/main/runtime/rpc/methods/git-params.ts +++ b/src/main/runtime/rpc/methods/git-params.ts @@ -16,6 +16,19 @@ export const GitCheckIgnored = WorktreeSelector.extend({ paths: z.array(z.string().min(1, 'Missing path')).max(2000) }) +export const GitSubmoduleStatus = WorktreeSelector.extend({ + submodulePath: z + .unknown() + .transform((v) => (typeof v === 'string' ? v : '')) + .pipe( + z + .string() + .min(1, 'Missing submodule path') + // Why: never let a submodule path be parsed as a git flag (arg injection). + .refine((value) => !value.startsWith('-'), 'Submodule path must not start with -') + ) +}) + export const GitFilePath = WorktreeSelector.extend({ filePath: z .unknown() diff --git a/src/main/runtime/rpc/methods/git.ts b/src/main/runtime/rpc/methods/git.ts index 964f7d09ce..b37d44ac71 100644 --- a/src/main/runtime/rpc/methods/git.ts +++ b/src/main/runtime/rpc/methods/git.ts @@ -23,6 +23,7 @@ import { GitRemoteCommitUrl, GitRemoteFileUrl, GitStatusParams, + GitSubmoduleStatus, GitTargetedRemote, WorktreeSelector } from './git-params' @@ -111,6 +112,12 @@ export const GIT_METHODS: RpcMethod[] = [ handler: async (params, { runtime }) => runtime.checkRuntimeGitIgnoredPaths(params.worktree, params.paths) }), + defineMethod({ + name: 'git.submoduleStatus', + params: GitSubmoduleStatus, + handler: async (params, { runtime }) => + runtime.getRuntimeGitSubmoduleStatus(params.worktree, params.submodulePath) + }), defineMethod({ name: 'git.history', params: GitHistory, diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 55c32a9a5d..e4a7dc4084 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -2110,6 +2110,11 @@ export type PreloadApi = { includeIgnored?: boolean bypassEffectiveUpstreamNegativeCache?: boolean }) => Promise + submoduleStatus: (args: { + worktreePath: string + submodulePath: string + connectionId?: string + }) => Promise checkIgnored: (args: { worktreePath: string paths: string[] diff --git a/src/preload/index.ts b/src/preload/index.ts index 7936edb853..dbaaf98a1b 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -2538,6 +2538,11 @@ const api = { includeIgnored?: boolean bypassEffectiveUpstreamNegativeCache?: boolean }): Promise => ipcRenderer.invoke('git:status', args), + submoduleStatus: (args: { + worktreePath: string + submodulePath: string + connectionId?: string + }): Promise => ipcRenderer.invoke('git:submoduleStatus', args), checkIgnored: (args: { worktreePath: string paths: string[] diff --git a/src/relay/git-handler-submodule-ops.ts b/src/relay/git-handler-submodule-ops.ts new file mode 100644 index 0000000000..fb0b2290a2 --- /dev/null +++ b/src/relay/git-handler-submodule-ops.ts @@ -0,0 +1,196 @@ +/** + * Submodule diff routing for the SSH relay. + * + * Why: the parent repo lists a submodule as a single gitlink row, and a gitlink + * path can't be read as a blob (`git show HEAD:` is a "bad object"). These + * helpers route a gitlink root to a synthesized pointer diff and resolve the + * configured submodule paths so inner files recurse into the submodule worktree. + * Split from git-handler-ops.ts to keep that file under the max-lines budget. + */ +import * as path from 'path' +import { buildDiffResult } from './git-diff-result' +import { parseBranchDiff } from './git-handler-utils' +import { parseNumstat } from '../shared/git-uncommitted-line-stats' +import { readBlobAtOid, type GitBufferExec, type GitExec } from './git-handler-ops' + +/** + * Configured submodule paths (relative, forward-slash) read from `.gitmodules`. + * Used to route gitlink/inner diffs without an index-wide `ls-files` scan. + */ +export async function listSubmodulePaths(git: GitExec, worktreePath: string): Promise { + try { + const { stdout } = await git( + ['config', '--file', '.gitmodules', '--get-regexp', '^submodule\\..*\\.path$'], + worktreePath + ) + return stdout + .split(/\r?\n/) + .map((line) => { + const spaceIndex = line.indexOf(' ') + return spaceIndex === -1 + ? '' + : line + .slice(spaceIndex + 1) + .trim() + .replace(/\/+$/, '') + }) + .filter((value) => value.length > 0) + } catch { + return [] + } +} + +export function findContainingSubmodule(submodulePaths: string[], filePath: string): string | null { + const normalized = filePath.replace(/\\/g, '/').replace(/\/+$/, '') + let best: string | null = null + for (const sub of submodulePaths) { + if (normalized === sub || normalized.startsWith(`${sub}/`)) { + if (!best || sub.length > best.length) { + best = sub + } + } + } + return best +} + +async function readGitlinkOidFromTree( + git: GitExec, + worktreePath: string, + ref: string, + submodulePath: string +): Promise { + try { + const { stdout } = await git(['ls-tree', ref, '--', submodulePath], worktreePath) + return stdout.match(/^160000 commit ([0-9a-f]+)\t/m)?.[1] ?? '' + } catch { + return '' + } +} + +async function readGitlinkOidFromIndex( + git: GitExec, + worktreePath: string, + submodulePath: string +): Promise { + try { + const { stdout } = await git(['ls-files', '-s', '--', submodulePath], worktreePath) + return stdout.match(/^160000 ([0-9a-f]+) /m)?.[1] ?? '' + } catch { + return '' + } +} + +async function readWorkingSubmoduleHead( + git: GitExec, + submoduleWorktreePath: string +): Promise { + try { + const { stdout } = await git(['rev-parse', 'HEAD'], submoduleWorktreePath) + return stdout.trim() + } catch { + return '' + } +} + +/** + * Resolve the submodule's recorded commit (parent index, falling back to HEAD) + * and its checked-out worktree commit. When these differ the gitlink moved. + */ +export async function resolveSubmoduleCommitRange( + git: GitExec, + worktreePath: string, + submodulePath: string +): Promise<{ fromOid: string; toOid: string }> { + const submoduleWorktreePath = path.join(worktreePath, submodulePath) + const fromOid = + (await readGitlinkOidFromIndex(git, worktreePath, submodulePath)) || + (await readGitlinkOidFromTree(git, worktreePath, 'HEAD', submodulePath)) + const toOid = await readWorkingSubmoduleHead(git, submoduleWorktreePath) + return { fromOid, toOid } +} + +/** + * List files changed between two submodule commits as status rows (area + * `unstaged`), so an expanded moved-pointer submodule shows its committed file + * changes instead of an empty working-tree status. + */ +export async function computeSubmoduleRangeEntries( + git: GitExec, + submoduleWorktreePath: string, + fromOid: string, + toOid: string +): Promise[]> { + let nameStatus = '' + let numstat = '' + try { + const [statusResult, numstatResult] = await Promise.all([ + git( + ['-c', 'core.quotePath=false', 'diff', '--name-status', '-M', '-C', fromOid, toOid], + submoduleWorktreePath + ), + git( + ['-c', 'core.quotePath=false', 'diff', '-z', '--numstat', '-M', '-C', fromOid, toOid], + submoduleWorktreePath + ) + ]) + nameStatus = statusResult.stdout + numstat = numstatResult.stdout + } catch { + return [] + } + return parseBranchDiff(nameStatus, parseNumstat(numstat)).map((entry) => ({ + ...entry, + area: 'unstaged' + })) +} + +/** + * Diff a file inside a submodule across two of its commits (recorded vs + * checked-out), mirroring the local handler's commit-range route. + */ +export async function buildSubmoduleInnerCommitRangeDiff( + gitBuffer: GitBufferExec, + submoduleWorktreePath: string, + innerPath: string, + fromOid: string, + toOid: string +) { + const left = await readBlobAtOid(gitBuffer, submoduleWorktreePath, fromOid, innerPath) + const right = await readBlobAtOid(gitBuffer, submoduleWorktreePath, toOid, innerPath) + return buildDiffResult(left.content, right.content, left.isBinary, right.isBinary, innerPath) +} + +/** + * Synthesize a gitlink pointer diff (one-line `Subproject commit ` swap), + * matching git's own rendering of submodule commit changes. + */ +export async function computeSubmodulePointerDiff( + git: GitExec, + worktreePath: string, + submodulePath: string, + staged: boolean, + compareAgainstHead = false +) { + const submoduleWorktreePath = path.join(worktreePath, submodulePath) + let leftOid = '' + let rightOid = '' + if (staged) { + leftOid = await readGitlinkOidFromTree(git, worktreePath, 'HEAD', submodulePath) + rightOid = await readGitlinkOidFromIndex(git, worktreePath, submodulePath) + } else if (compareAgainstHead) { + leftOid = await readGitlinkOidFromTree(git, worktreePath, 'HEAD', submodulePath) + rightOid = await readWorkingSubmoduleHead(git, submoduleWorktreePath) + } else { + leftOid = + (await readGitlinkOidFromIndex(git, worktreePath, submodulePath)) || + (await readGitlinkOidFromTree(git, worktreePath, 'HEAD', submodulePath)) + rightOid = await readWorkingSubmoduleHead(git, submoduleWorktreePath) + } + return buildDiffResult( + leftOid ? `Subproject commit ${leftOid}\n` : '', + rightOid ? `Subproject commit ${rightOid}\n` : '', + false, + false, + submodulePath + ) +} diff --git a/src/relay/git-handler.test.ts b/src/relay/git-handler.test.ts index c2a8eab2bf..ad00af575d 100644 --- a/src/relay/git-handler.test.ts +++ b/src/relay/git-handler.test.ts @@ -561,6 +561,137 @@ describe('GitHandler', () => { }) }) + describe('submodule', () => { + const extraDirs: string[] = [] + + afterEach(async () => { + await Promise.all( + extraDirs.splice(0).map((dir) => fs.rm(dir, { recursive: true, force: true })) + ) + }) + + // Why: `git submodule add` against a local path is blocked since git 2.38 + // unless protocol.file.allow=always is set explicitly. + function addSubmodule(parent: string, name: string): string { + const src = mkdtempSync(path.join(tmpdir(), 'relay-subsrc-')) + extraDirs.push(src) + gitInit(src) + writeFileSync(path.join(src, 'lib.txt'), 'v1\n') + gitCommit(src, 'sub initial') + execFileSync('git', ['-c', 'protocol.file.allow=always', 'submodule', 'add', src, name], { + cwd: parent, + stdio: 'pipe' + }) + execFileSync('git', ['commit', '-m', 'add submodule'], { cwd: parent, stdio: 'pipe' }) + return path.join(parent, name) + } + + it('returns inner per-file changes via git.submoduleStatus', async () => { + gitInit(tmpDir) + writeFileSync(path.join(tmpDir, 'root.txt'), 'root') + gitCommit(tmpDir, 'initial') + const sub = addSubmodule(tmpDir, 'flutter_mine') + writeFileSync(path.join(sub, 'lib.txt'), 'v2\n') + + const result = (await dispatcher.callRequest('git.submoduleStatus', { + worktreePath: tmpDir, + submodulePath: 'flutter_mine' + })) as { entries: { path?: unknown; status?: unknown; area?: unknown }[] } + + const inner = result.entries.find((e) => e.path === 'lib.txt') + expect(inner).toBeDefined() + expect(inner!.status).toBe('modified') + expect(inner!.area).toBe('unstaged') + }) + + it('rejects submoduleStatus paths that escape the worktree', async () => { + gitInit(tmpDir) + await expect( + dispatcher.callRequest('git.submoduleStatus', { + worktreePath: tmpDir, + submodulePath: '../outside' + }) + ).rejects.toThrow('outside the worktree') + }) + + it('routes inner submodule files into the submodule worktree diff', async () => { + gitInit(tmpDir) + writeFileSync(path.join(tmpDir, 'root.txt'), 'root') + gitCommit(tmpDir, 'initial') + const sub = addSubmodule(tmpDir, 'flutter_mine') + writeFileSync(path.join(sub, 'lib.txt'), 'v2\n') + + const result = (await dispatcher.callRequest('git.diff', { + worktreePath: tmpDir, + filePath: 'flutter_mine/lib.txt', + staged: false + })) as { kind: string; originalContent: string; modifiedContent: string } + + expect(result.kind).toBe('text') + expect(normalizeGitFileText(result.originalContent)).toBe('v1\n') + expect(normalizeGitFileText(result.modifiedContent)).toBe('v2\n') + }) + + it('synthesizes a Subproject commit pointer diff for the gitlink root', async () => { + gitInit(tmpDir) + writeFileSync(path.join(tmpDir, 'root.txt'), 'root') + gitCommit(tmpDir, 'initial') + const sub = addSubmodule(tmpDir, 'flutter_mine') + const oldOid = execFileSync('git', ['rev-parse', 'HEAD'], { + cwd: sub, + encoding: 'utf-8' + }).trim() + writeFileSync(path.join(sub, 'lib.txt'), 'v2\n') + execFileSync('git', ['add', 'lib.txt'], { cwd: sub, stdio: 'pipe' }) + gitCommit(sub, 'sub second') + const newOid = execFileSync('git', ['rev-parse', 'HEAD'], { + cwd: sub, + encoding: 'utf-8' + }).trim() + + const result = (await dispatcher.callRequest('git.diff', { + worktreePath: tmpDir, + filePath: 'flutter_mine', + staged: false + })) as { kind: string; originalContent: string; modifiedContent: string } + + expect(result.kind).toBe('text') + expect(result.originalContent).toBe(`Subproject commit ${oldOid}\n`) + expect(result.modifiedContent).toBe(`Subproject commit ${newOid}\n`) + }) + + // Why: a moved gitlink with a clean submodule worktree has no uncommitted + // rows, so status/diff must surface the committed file changes between the + // recorded and checked-out commits. + it('lists commit-range files and diffs them when the pointer moved', async () => { + gitInit(tmpDir) + writeFileSync(path.join(tmpDir, 'root.txt'), 'root') + gitCommit(tmpDir, 'initial') + const sub = addSubmodule(tmpDir, 'flutter_mine') + writeFileSync(path.join(sub, 'lib.txt'), 'v2\n') + execFileSync('git', ['add', 'lib.txt'], { cwd: sub, stdio: 'pipe' }) + gitCommit(sub, 'sub second') + + const status = (await dispatcher.callRequest('git.submoduleStatus', { + worktreePath: tmpDir, + submodulePath: 'flutter_mine' + })) as { entries: { path?: unknown; status?: unknown; area?: unknown }[] } + const ranged = status.entries.find((e) => e.path === 'lib.txt') + expect(ranged).toBeDefined() + expect(ranged!.status).toBe('modified') + expect(ranged!.area).toBe('unstaged') + + const diff = (await dispatcher.callRequest('git.diff', { + worktreePath: tmpDir, + filePath: 'flutter_mine/lib.txt', + staged: false + })) as { kind: string; originalContent: string; modifiedContent: string } + expect(diff.kind).toBe('text') + expect(normalizeGitFileText(diff.originalContent)).toBe('v1\n') + expect(normalizeGitFileText(diff.modifiedContent)).toBe('v2\n') + }) + }) + describe('discard', () => { it('discards changes to tracked file', async () => { gitInit(tmpDir) diff --git a/src/relay/git-handler.ts b/src/relay/git-handler.ts index 46dd750f2a..edf363eac4 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -18,6 +18,14 @@ import { branchDiffEntries, validateGitExecArgs } from './git-handler-ops' +import { + buildSubmoduleInnerCommitRangeDiff, + computeSubmodulePointerDiff, + computeSubmoduleRangeEntries, + findContainingSubmodule, + listSubmodulePaths, + resolveSubmoduleCommitRange +} from './git-handler-submodule-ops' import { commitCompare as commitCompareOp, commitDiffEntry } from './git-handler-commit-diff-ops' import { commitChangesRelay, @@ -98,6 +106,7 @@ export class GitHandler { private registerHandlers(): void { this.dispatcher.onRequest('git.status', (p) => this.getStatus(p)) + this.dispatcher.onRequest('git.submoduleStatus', (p) => this.getSubmoduleStatus(p)) this.dispatcher.onRequest('git.checkIgnored', (p) => this.checkIgnored(p)) this.dispatcher.onRequest('git.history', (p) => this.history(p)) this.dispatcher.onRequest('git.commit', (p) => this.commit(p)) @@ -190,6 +199,47 @@ export class GitHandler { return getStatusOp(this.git.bind(this), params) } + // Why: the parent status only lists a single gitlink row per submodule. The + // renderer fetches inner per-file changes on demand by running a plain status + // inside the submodule's own worktree. Reject paths escaping the worktree to + // match the diff handler's traversal guard. + private async getSubmoduleStatus(params: Record) { + const worktreePath = params.worktreePath as string + const submodulePath = params.submodulePath as string + const resolved = path.resolve(worktreePath, submodulePath) + const rel = path.relative(path.resolve(worktreePath), resolved) + if (!rel || rel === '..' || rel.startsWith(`..${path.sep}`) || path.isAbsolute(rel)) { + throw new Error(`Submodule path "${submodulePath}" resolves outside the worktree`) + } + const workingResult = await getStatusOp(this.git.bind(this), { + ...params, + worktreePath: resolved + }) + // Why: a moved gitlink (clean worktree) has no uncommitted rows; surface the + // files changed between the recorded and checked-out commits so the expanded + // submodule isn't empty. Mirrors getSubmoduleStatus in the local handler. + const { fromOid, toOid } = await resolveSubmoduleCommitRange( + this.git.bind(this), + worktreePath, + submodulePath + ) + if (fromOid && toOid && fromOid !== toOid) { + const rangeEntries = await computeSubmoduleRangeEntries( + this.git.bind(this), + resolved, + fromOid, + toOid + ) + const rangePaths = new Set(rangeEntries.map((entry) => entry.path)) + const entries = [ + ...rangeEntries, + ...workingResult.entries.filter((entry) => !rangePaths.has(entry.path)) + ] + return { ...workingResult, entries } + } + return workingResult + } + private async checkIgnored(params: Record) { return checkIgnoredPathsOp(this.git.bind(this), params) } @@ -212,12 +262,58 @@ export class GitHandler { if (rel === '..' || rel.startsWith(`..${path.sep}`) || path.isAbsolute(rel)) { throw new Error(`Path "${filePath}" resolves outside the worktree`) } + const staged = params.staged as boolean + const compareAgainstHead = params.compareAgainstHead as boolean | undefined + // Why: gitlink paths can't be read as blobs and submodule working dirs read + // as empty, so route the gitlink root → pointer diff and inner files → + // recurse into the submodule's own worktree (mirrors the local handler). + const submodulePaths = await listSubmodulePaths(this.git.bind(this), worktreePath) + if (submodulePaths.length > 0) { + const matchedSubmodule = findContainingSubmodule(submodulePaths, filePath) + if (matchedSubmodule) { + const normalizedFilePath = filePath.replace(/\\/g, '/').replace(/\/+$/, '') + if (normalizedFilePath === matchedSubmodule) { + return computeSubmodulePointerDiff( + this.git.bind(this), + worktreePath, + matchedSubmodule, + staged, + compareAgainstHead + ) + } + const submoduleWorktreePath = path.join(worktreePath, matchedSubmodule) + const innerPath = normalizedFilePath.slice(matchedSubmodule.length + 1) + const { fromOid, toOid } = await resolveSubmoduleCommitRange( + this.git.bind(this), + worktreePath, + matchedSubmodule + ) + // Why: a moved gitlink (clean worktree) keeps inner changes in committed + // history, so diff the two commits; otherwise read the working-tree blob. + if (fromOid && toOid && fromOid !== toOid) { + return buildSubmoduleInnerCommitRangeDiff( + this.gitBuffer.bind(this), + submoduleWorktreePath, + innerPath, + fromOid, + toOid + ) + } + return computeDiff( + this.gitBuffer.bind(this), + submoduleWorktreePath, + innerPath, + staged, + compareAgainstHead + ) + } + } return computeDiff( this.gitBuffer.bind(this), worktreePath, filePath, - params.staged as boolean, - params.compareAgainstHead as boolean | undefined + staged, + compareAgainstHead ) } diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index 45da0012dd..78f837f6b6 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -84,6 +84,14 @@ import { namespaceSourceControlTreeDirectoryKeys, type SourceControlTreeNode } from './source-control-tree' +import { + injectExpandedSubmoduleEntries, + injectExpandedSubmoduleRows, + isExpandableSubmoduleEntry, + type RenderableSourceControlNode, + type RenderableSubmoduleListItem, + type SubmoduleStatusState +} from './source-control-submodule-expansion' import { buildSourceControlDisplaySections, getSourceControlSectionViewAction, @@ -159,6 +167,7 @@ import { generateRuntimePullRequestFields, getRuntimeGitBranchCompare, getRuntimeGitHistory, + getRuntimeGitSubmoduleStatus, stageRuntimeGitPath, unstageRuntimeGitPath, type RuntimeGitContext, @@ -484,6 +493,9 @@ const EMPTY_GIT_HISTORY_STATE: GitHistoryPanelState = { status: 'idle' } const DEFAULT_COLLAPSED_SECTIONS = ['history'] as const const SUBMODULE_WORKTREE_ONLY_LABEL = 'Submodule changes - stage inside submodule' const SUBMODULE_WORKTREE_ONLY_STAGE_TOOLTIP = 'Stage these changes inside the submodule' +const SUBMODULE_LOADING_LABEL = 'Loading submodule changes…' +const SUBMODULE_EMPTY_LABEL = 'No changes in submodule' +const SUBMODULE_ERROR_LABEL = 'Failed to load submodule changes' function createDefaultCollapsedSections(): Set { return new Set(DEFAULT_COLLAPSED_SECTIONS) @@ -948,6 +960,13 @@ function SourceControlInner(): React.JSX.Element { const sourceControlViewMode = persistedSourceControlViewMode const sourceControlGroupOrder = resolveSourceControlGroupOrder(settings?.sourceControlGroupOrder) const [collapsedTreeDirs, setCollapsedTreeDirs] = useState>(new Set()) + // Why: dirty submodules are shown collapsed by default and only query their + // inner status when the user expands them, so the status poll never recurses + // into (potentially nested) submodules. Results are cached per submodule path. + const [expandedSubmodulePaths, setExpandedSubmodulePaths] = useState>(new Set()) + const [submoduleStatusByPath, setSubmoduleStatusByPath] = useState< + Record + >({}) const [baseRefDialogOpen, setBaseRefDialogOpen] = useState(false) const [pendingDiscard, setPendingDiscard] = useState(null) // Why: start null rather than 'origin/main' so branch compare doesn't fire @@ -1620,16 +1639,40 @@ function SourceControlInner(): React.JSX.Element { }, [displaySections]) const visibleTreeRowsBySection = useMemo(() => { - const rows: Partial> = - {} + const rows: Partial> = {} + for (const section of displaySections) { + rows[section.id] = injectExpandedSubmoduleRows( + flattenSourceControlTree(treeRootsBySection[section.id] ?? [], collapsedTreeDirs), + expandedSubmodulePaths, + submoduleStatusByPath, + SUBMODULE_LOADING_LABEL, + SUBMODULE_EMPTY_LABEL + ) + } + return rows + }, [ + collapsedTreeDirs, + displaySections, + treeRootsBySection, + expandedSubmodulePaths, + submoduleStatusByPath + ]) + + // List view needs the same lazy submodule expansion as the tree view, just + // spliced into the flat entry list instead of the tree-row list. + const visibleListRowsBySection = useMemo(() => { + const rows: Partial> = {} for (const section of displaySections) { - rows[section.id] = flattenSourceControlTree( - treeRootsBySection[section.id] ?? [], - collapsedTreeDirs + rows[section.id] = injectExpandedSubmoduleEntries( + section.items, + expandedSubmodulePaths, + submoduleStatusByPath, + SUBMODULE_LOADING_LABEL, + SUBMODULE_EMPTY_LABEL ) } return rows - }, [collapsedTreeDirs, displaySections, treeRootsBySection]) + }, [displaySections, expandedSubmodulePaths, submoduleStatusByPath]) const branchTreeRoots = useMemo( () => compactSourceControlTree(buildSourceControlTree('branch', filteredBranchEntries)), @@ -1813,6 +1856,8 @@ function SourceControlInner(): React.JSX.Element { setFilterExpanded(false) setCollapsedSections(createDefaultCollapsedSections()) setCollapsedTreeDirs(new Set()) + setExpandedSubmodulePaths(new Set()) + setSubmoduleStatusByPath({}) setBaseRefDialogOpen(false) setPendingDiscard(null) setPendingDiffCommentsClear(null) @@ -4223,7 +4268,10 @@ function SourceControlInner(): React.JSX.Element { const bulkUnstagePaths = useMemo( () => - selectedEntries.filter((entry) => entry.area === 'staged').map((entry) => entry.entry.path), + selectedEntries + // Why: submodule-internal rows are read-only from the parent worktree. + .filter((entry) => entry.area === 'staged' && !entry.entry.submoduleRoot) + .map((entry) => entry.entry.path), [selectedEntries] ) @@ -4721,6 +4769,66 @@ function SourceControlInner(): React.JSX.Element { }) }, []) + const fetchSubmoduleStatus = useCallback( + async (submodulePath: string): Promise => { + if (!worktreePath) { + return + } + // Why: keep any already-loaded children visible during a poll-driven + // refetch so expanding then refreshing doesn't flash a loading row. + setSubmoduleStatusByPath((prev) => + prev[submodulePath] ? prev : { ...prev, [submodulePath]: { status: 'loading' } } + ) + try { + const connectionId = getConnectionId(activeWorktreeId ?? null) ?? undefined + const result = await getRuntimeGitSubmoduleStatus( + { + // Why: route by the repo OWNER host, matching the rest of this panel. + settings: activeRepoSettings, + worktreeId: activeWorktreeId, + worktreePath, + connectionId + }, + submodulePath + ) + setSubmoduleStatusByPath((prev) => ({ + ...prev, + [submodulePath]: { status: 'loaded', entries: result.entries } + })) + } catch (error) { + setSubmoduleStatusByPath((prev) => ({ + ...prev, + [submodulePath]: { + status: 'error', + error: error instanceof Error ? error.message : String(error) + } + })) + } + }, + [activeRepoSettings, activeWorktreeId, worktreePath] + ) + + const toggleSubmodule = useCallback((submodulePath: string) => { + setExpandedSubmodulePaths((prev) => { + const next = new Set(prev) + if (next.has(submodulePath)) { + next.delete(submodulePath) + } else { + next.add(submodulePath) + } + return next + }) + }, []) + + // Why: (re)load inner status only for currently-expanded submodules. Re-runs + // when the parent status poll refreshes `entries` so expanded children stay + // fresh, while collapsed submodules never trigger any extra git work. + useEffect(() => { + for (const submodulePath of expandedSubmodulePaths) { + void fetchSubmoduleStatus(submodulePath) + } + }, [expandedSubmodulePaths, entries, fetchSubmoduleStatus]) + const openCommittedDiff = useCallback( (entry: GitBranchChangeEntry, event?: SourceControlRowOpenEvent) => { if ( @@ -5680,6 +5788,16 @@ function SourceControlInner(): React.JSX.Element { {!isCollapsed && (sourceControlViewMode === 'tree' ? (visibleTreeRowsBySection[id] ?? []).map((node) => { + if (node.type === 'submodule-placeholder') { + return ( + + ) + } if (node.type === 'directory') { return ( ) } + const submoduleExpansion = isExpandableSubmoduleEntry(node.entry) + ? { + isExpanded: expandedSubmodulePaths.has(node.entry.path), + onToggle: () => toggleSubmodule(node.entry.path) + } + : undefined return ( ) }) - : items.map((entry) => { + : (visibleListRowsBySection[id] ?? []).map((row) => { + if (row.type === 'submodule-placeholder') { + return ( + + ) + } + const entry = row.entry const key = `${entry.area}::${entry.path}` + const submoduleExpansion = isExpandableSubmoduleEntry(entry) + ? { + isExpanded: expandedSubmodulePaths.has(entry.path), + onToggle: () => toggleSubmodule(entry.path) + } + : undefined return ( ) }))} @@ -7653,6 +7797,37 @@ function DiffLineCounts({ ) } +function SubmodulePlaceholderRow({ + depth, + state, + message +}: { + depth: number + state: 'loading' | 'empty' | 'error' + message?: string +}): React.JSX.Element { + const fallback = + state === 'error' + ? SUBMODULE_ERROR_LABEL + : state === 'empty' + ? SUBMODULE_EMPTY_LABEL + : SUBMODULE_LOADING_LABEL + return ( +
+ {state === 'loading' && } + {message ?? fallback} +
+ ) +} + const UncommittedEntryRow = React.memo(function UncommittedEntryRow({ entryKey, entry, @@ -7670,7 +7845,8 @@ const UncommittedEntryRow = React.memo(function UncommittedEntryRow({ onUnstage, onDiscard, commentCount, - showPathHint = true + showPathHint = true, + submoduleExpansion }: { entryKey: string entry: GitStatusEntry @@ -7689,6 +7865,9 @@ const UncommittedEntryRow = React.memo(function UncommittedEntryRow({ onDiscard: (entry: GitStatusEntry) => void commentCount: number showPathHint?: boolean + // When set, the row is a dirty submodule: clicking toggles lazy expansion of + // its inner changes instead of opening a (uninformative) gitlink diff. + submoduleExpansion?: { isExpanded: boolean; onToggle: () => void } }): React.JSX.Element { const FileIcon = getFileTypeIcon(entry.path) const fileName = basename(entry.path) @@ -7715,6 +7894,7 @@ const UncommittedEntryRow = React.memo(function UncommittedEntryRow({ const canDiscard = !isUnresolvedConflict && !isResolvedLocally && + !entry.submoduleRoot && (entry.area === 'unstaged' || entry.area === 'untracked') const canStage = isStageableStatusEntry(entry) const canUnstage = entry.area === 'staged' @@ -7759,6 +7939,10 @@ const UncommittedEntryRow = React.memo(function UncommittedEntryRow({ e.dataTransfer.effectAllowed = 'copy' }} onClick={(e) => { + if (submoduleExpansion) { + submoduleExpansion.onToggle() + return + } if (onSelect) { onSelect(e, entryKey, entry) } else { @@ -7766,9 +7950,20 @@ const UncommittedEntryRow = React.memo(function UncommittedEntryRow({ } }} onDoubleClick={(e) => { + if (submoduleExpansion) { + return + } onOpen(entry, toPermanentSourceControlRowOpenEvent(e)) }} > + {submoduleExpansion && ( + + )}
diff --git a/src/renderer/src/components/right-sidebar/discard-all-sequence.test.ts b/src/renderer/src/components/right-sidebar/discard-all-sequence.test.ts index b0a1bef1e4..e808af6da2 100644 --- a/src/renderer/src/components/right-sidebar/discard-all-sequence.test.ts +++ b/src/renderer/src/components/right-sidebar/discard-all-sequence.test.ts @@ -155,6 +155,19 @@ describe('status entry stageability', () => { expect(isSubmoduleWorktreeOnlyChange(changedGitlink)).toBe(false) expect(isStageableStatusEntry(changedGitlink)).toBe(true) }) + + it('marks lazily-expanded submodule-internal entries as not stageable', () => { + const innerEntry = entry({ + path: 'flutter_mine/lib/main.dart', + area: 'unstaged', + submoduleRoot: 'flutter_mine' + }) + + // Why: changes inside a submodule are read-only from the parent — staging + // them via `git add` from the parent worktree is a no-op/footgun. + expect(isStageableStatusEntry(innerEntry)).toBe(false) + expect(getStageAllPaths([innerEntry], 'unstaged')).toEqual([]) + }) }) describe('getUnstageAllPaths', () => { diff --git a/src/renderer/src/components/right-sidebar/discard-all-sequence.ts b/src/renderer/src/components/right-sidebar/discard-all-sequence.ts index 5384830603..c52854fcd9 100644 --- a/src/renderer/src/components/right-sidebar/discard-all-sequence.ts +++ b/src/renderer/src/components/right-sidebar/discard-all-sequence.ts @@ -40,6 +40,9 @@ export function isStageableStatusEntry(entry: GitStatusEntry): boolean { return ( (entry.area === 'unstaged' || entry.area === 'untracked') && entry.conflictStatus !== 'unresolved' && + // Why: changes inside a submodule (lazily expanded) are read-only from the + // parent — `git add` here can't stage the submodule's own working tree. + !entry.submoduleRoot && !isSubmoduleWorktreeOnlyChange(entry) ) } diff --git a/src/renderer/src/components/right-sidebar/source-control-submodule-expansion.test.ts b/src/renderer/src/components/right-sidebar/source-control-submodule-expansion.test.ts new file mode 100644 index 0000000000..44658139d8 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control-submodule-expansion.test.ts @@ -0,0 +1,295 @@ +import { describe, expect, it } from 'vitest' +import type { GitStatusEntry } from '../../../../shared/types' +import { + buildSubmoduleChildNodes, + injectExpandedSubmoduleEntries, + injectExpandedSubmoduleRows, + isExpandableSubmoduleEntry, + type SubmoduleSectionTreeNode, + type SubmoduleStatusState +} from './source-control-submodule-expansion' + +const LOADING = 'Loading submodule changes…' +const EMPTY = 'No changes in submodule' + +function submoduleEntry(partial: Partial & { path: string }): GitStatusEntry { + return { + status: 'modified', + area: 'unstaged', + submodule: { commitChanged: false, trackedChanges: true, untrackedChanges: false }, + ...partial + } +} + +function fileNode(entry: GitStatusEntry, depth = 0): SubmoduleSectionTreeNode & { type: 'file' } { + return { + type: 'file', + key: `unstaged::${entry.path}`, + name: entry.path.split('/').pop() ?? entry.path, + path: entry.path, + entry, + area: 'unstaged', + depth + } +} + +describe('isExpandableSubmoduleEntry', () => { + it('is expandable when the submodule has tracked or untracked changes', () => { + expect( + isExpandableSubmoduleEntry( + submoduleEntry({ + path: 'flutter_mine', + submodule: { commitChanged: false, trackedChanges: true, untrackedChanges: false } + }) + ) + ).toBe(true) + expect( + isExpandableSubmoduleEntry( + submoduleEntry({ + path: 'flutter_mine', + submodule: { commitChanged: true, trackedChanges: false, untrackedChanges: true } + }) + ) + ).toBe(true) + }) + + it('is expandable for a pointer-only (commit) change so its files can be inspected', () => { + expect( + isExpandableSubmoduleEntry( + submoduleEntry({ + path: 'flutter_mine', + submodule: { commitChanged: true, trackedChanges: false, untrackedChanges: false } + }) + ) + ).toBe(true) + }) + + it('is not expandable when the submodule has no changes at all', () => { + expect( + isExpandableSubmoduleEntry( + submoduleEntry({ + path: 'flutter_mine', + submodule: { commitChanged: false, trackedChanges: false, untrackedChanges: false } + }) + ) + ).toBe(false) + }) + + it('is not expandable for non-submodule entries or already-inner entries', () => { + expect( + isExpandableSubmoduleEntry({ path: 'src/a.ts', status: 'modified', area: 'unstaged' }) + ).toBe(false) + expect( + isExpandableSubmoduleEntry( + submoduleEntry({ path: 'flutter_mine/lib/main.dart', submoduleRoot: 'flutter_mine' }) + ) + ).toBe(false) + }) +}) + +describe('buildSubmoduleChildNodes', () => { + it('prefixes inner paths, stamps submoduleRoot, and nests one level deeper', () => { + const parent = fileNode(submoduleEntry({ path: 'flutter_mine' }), 2) + const inner: GitStatusEntry[] = [ + { path: 'lib/main.dart', status: 'modified', area: 'unstaged' } + ] + + const [child] = buildSubmoduleChildNodes(parent, inner) + + expect(child.path).toBe('flutter_mine/lib/main.dart') + expect(child.name).toBe('main.dart') + expect(child.entry.submoduleRoot).toBe('flutter_mine') + expect(child.entry.status).toBe('modified') + expect(child.depth).toBe(3) + expect(child.area).toBe('unstaged') + }) +}) + +describe('injectExpandedSubmoduleRows', () => { + it('passes through unexpanded nodes untouched', () => { + const node = fileNode(submoduleEntry({ path: 'flutter_mine' })) + const result = injectExpandedSubmoduleRows([node], new Set(), {}, LOADING, EMPTY) + expect(result).toEqual([node]) + }) + + it('emits a loading placeholder when status is missing or loading', () => { + const node = fileNode(submoduleEntry({ path: 'flutter_mine' })) + const result = injectExpandedSubmoduleRows( + [node], + new Set(['flutter_mine']), + {}, + LOADING, + EMPTY + ) + expect(result).toHaveLength(2) + expect(result[1]).toMatchObject({ + type: 'submodule-placeholder', + state: 'loading', + message: LOADING, + submodulePath: 'flutter_mine' + }) + }) + + it('emits an error placeholder carrying the error message', () => { + const node = fileNode(submoduleEntry({ path: 'flutter_mine' })) + const statuses: Record = { + flutter_mine: { status: 'error', error: 'boom' } + } + const result = injectExpandedSubmoduleRows( + [node], + new Set(['flutter_mine']), + statuses, + LOADING, + EMPTY + ) + expect(result[1]).toMatchObject({ + type: 'submodule-placeholder', + state: 'error', + message: 'boom' + }) + }) + + it('emits an empty placeholder when the submodule has no inner entries', () => { + const node = fileNode(submoduleEntry({ path: 'flutter_mine' })) + const statuses: Record = { + flutter_mine: { status: 'loaded', entries: [] } + } + const result = injectExpandedSubmoduleRows( + [node], + new Set(['flutter_mine']), + statuses, + LOADING, + EMPTY + ) + expect(result[1]).toMatchObject({ + type: 'submodule-placeholder', + state: 'empty', + message: EMPTY + }) + }) + + it('injects child file rows when inner status is loaded', () => { + const node = fileNode(submoduleEntry({ path: 'flutter_mine' })) + const statuses: Record = { + flutter_mine: { + status: 'loaded', + entries: [{ path: 'lib/main.dart', status: 'modified', area: 'unstaged' }] + } + } + const result = injectExpandedSubmoduleRows( + [node], + new Set(['flutter_mine']), + statuses, + LOADING, + EMPTY + ) + expect(result).toHaveLength(2) + expect(result[1]).toMatchObject({ + type: 'file', + path: 'flutter_mine/lib/main.dart' + }) + const child = result[1] as SubmoduleSectionTreeNode & { type: 'file' } + expect(child.entry.submoduleRoot).toBe('flutter_mine') + }) + + it('expands a pointer-only (commit) submodule into its commit-range files', () => { + const node = fileNode( + submoduleEntry({ + path: 'flutter_mine', + submodule: { commitChanged: true, trackedChanges: false, untrackedChanges: false } + }) + ) + const statuses: Record = { + flutter_mine: { + status: 'loaded', + entries: [{ path: 'lib/main.dart', status: 'modified', area: 'unstaged' }] + } + } + const result = injectExpandedSubmoduleRows( + [node], + new Set(['flutter_mine']), + statuses, + LOADING, + EMPTY + ) + expect(result).toHaveLength(2) + expect(result[1]).toMatchObject({ type: 'file', path: 'flutter_mine/lib/main.dart' }) + }) + + it('never expands a non-submodule entry that is in the expanded set', () => { + const node = fileNode({ path: 'src/a.ts', status: 'modified', area: 'unstaged' }) + const result = injectExpandedSubmoduleRows([node], new Set(['src/a.ts']), {}, LOADING, EMPTY) + expect(result).toEqual([node]) + }) +}) + +describe('injectExpandedSubmoduleEntries (list view)', () => { + it('passes through unexpanded entries untouched', () => { + const entry = submoduleEntry({ path: 'flutter_mine' }) + const result = injectExpandedSubmoduleEntries([entry], new Set(), {}, LOADING, EMPTY) + expect(result).toEqual([{ type: 'entry', entry }]) + }) + + it('emits a loading placeholder when status is missing', () => { + const entry = submoduleEntry({ path: 'flutter_mine' }) + const result = injectExpandedSubmoduleEntries( + [entry], + new Set(['flutter_mine']), + {}, + LOADING, + EMPTY + ) + expect(result).toHaveLength(2) + expect(result[1]).toMatchObject({ + type: 'submodule-placeholder', + state: 'loading', + submodulePath: 'flutter_mine', + depth: 1 + }) + }) + + it('injects child entries (with submoduleRoot) for a pointer-only commit change', () => { + const entry = submoduleEntry({ + path: 'flutter_mine', + submodule: { commitChanged: true, trackedChanges: false, untrackedChanges: false } + }) + const statuses: Record = { + flutter_mine: { + status: 'loaded', + entries: [{ path: 'lib/main.dart', status: 'modified', area: 'unstaged' }] + } + } + const result = injectExpandedSubmoduleEntries( + [entry], + new Set(['flutter_mine']), + statuses, + LOADING, + EMPTY + ) + expect(result).toHaveLength(2) + expect(result[0]).toEqual({ type: 'entry', entry }) + expect(result[1]).toMatchObject({ + type: 'entry', + entry: { path: 'flutter_mine/lib/main.dart', submoduleRoot: 'flutter_mine' } + }) + }) + + it('emits an empty placeholder when loaded with no inner entries', () => { + const entry = submoduleEntry({ path: 'flutter_mine' }) + const statuses: Record = { + flutter_mine: { status: 'loaded', entries: [] } + } + const result = injectExpandedSubmoduleEntries( + [entry], + new Set(['flutter_mine']), + statuses, + LOADING, + EMPTY + ) + expect(result[1]).toMatchObject({ + type: 'submodule-placeholder', + state: 'empty', + message: EMPTY + }) + }) +}) diff --git a/src/renderer/src/components/right-sidebar/source-control-submodule-expansion.ts b/src/renderer/src/components/right-sidebar/source-control-submodule-expansion.ts new file mode 100644 index 0000000000..502fccdaff --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control-submodule-expansion.ts @@ -0,0 +1,215 @@ +import type { GitStatusEntry } from '../../../../shared/types' +import { basename } from '@/lib/path' +import type { SourceControlSectionArea } from './source-control-section-order' +import type { SourceControlTreeNode } from './source-control-tree' + +export type SubmoduleSectionTreeNode = SourceControlTreeNode< + GitStatusEntry, + SourceControlSectionArea +> + +/** + * Loading/empty/error placeholder shown beneath an expanded submodule while its + * inner status is fetched on demand. Kept separate from real tree nodes so it + * never leaks into selection or bulk path collection. + */ +export type SubmodulePlaceholderNode = { + type: 'submodule-placeholder' + key: string + submodulePath: string + depth: number + state: 'loading' | 'empty' | 'error' + message?: string +} + +export type RenderableSourceControlNode = SubmoduleSectionTreeNode | SubmodulePlaceholderNode + +export type SubmoduleStatusState = + | { status: 'loading' } + | { status: 'loaded'; entries: GitStatusEntry[] } + | { status: 'error'; error: string } + +/** + * Any changed submodule is shown as an expandable row: worktree dirtiness + * (tracked/untracked) expands to the inner `git status`, and a moved commit + * pointer expands to the files changed between the recorded and checked-out + * commits. Already-inner rows (`submoduleRoot` set) never expand again. + */ +export function isExpandableSubmoduleEntry(entry: GitStatusEntry): boolean { + const submodule = entry.submodule + if (!submodule || entry.submoduleRoot) { + return false + } + return submodule.commitChanged || submodule.trackedChanges || submodule.untrackedChanges +} + +/** + * Build the read-only inner entry for a submodule child row. The inner path is + * relative to the submodule root, so it is prefixed with the submodule path + * (drives diff routing) and stamped with `submoduleRoot` (drives read-only + * gating). The inner entry's own status/area are preserved. + */ +export function buildSubmoduleChildEntry( + submodulePath: string, + innerEntry: GitStatusEntry +): GitStatusEntry { + return { + ...innerEntry, + path: `${submodulePath}/${innerEntry.path}`, + submoduleRoot: submodulePath + } +} + +/** + * Build the child file rows for an expanded submodule (tree view). + */ +export function buildSubmoduleChildNodes( + parent: SubmoduleSectionTreeNode & { type: 'file' }, + innerEntries: GitStatusEntry[] +): (SubmoduleSectionTreeNode & { type: 'file' })[] { + const submodulePath = parent.entry.path + return innerEntries.map((innerEntry) => { + const childEntry = buildSubmoduleChildEntry(submodulePath, innerEntry) + return { + type: 'file', + key: `${parent.area}::${childEntry.path}`, + name: basename(childEntry.path), + path: childEntry.path, + entry: childEntry, + area: parent.area, + depth: parent.depth + 1 + } + }) +} + +/** + * Flat-list (non-tree) variant of an expanded source-control row: either a real + * status entry or a submodule placeholder. Used by the list view, which renders + * raw entries instead of tree nodes. + */ +export type RenderableSubmoduleListItem = + | { type: 'entry'; entry: GitStatusEntry } + | SubmodulePlaceholderNode + +/** + * Splice lazily-loaded submodule children into a flat list of status entries + * (list view). Mirrors injectExpandedSubmoduleRows but operates on entries. + */ +export function injectExpandedSubmoduleEntries( + entries: readonly GitStatusEntry[], + expandedSubmodulePaths: ReadonlySet, + submoduleStatusByPath: Readonly>, + loadingMessage: string, + emptyMessage: string +): RenderableSubmoduleListItem[] { + const result: RenderableSubmoduleListItem[] = [] + for (const entry of entries) { + result.push({ type: 'entry', entry }) + if (!isExpandableSubmoduleEntry(entry) || !expandedSubmodulePaths.has(entry.path)) { + continue + } + const submodulePath = entry.path + const state = submoduleStatusByPath[submodulePath] + if (!state || state.status === 'loading') { + result.push({ + type: 'submodule-placeholder', + key: `submodule-loading::${entry.area}::${submodulePath}`, + submodulePath, + depth: 1, + state: 'loading', + message: loadingMessage + }) + continue + } + if (state.status === 'error') { + result.push({ + type: 'submodule-placeholder', + key: `submodule-error::${entry.area}::${submodulePath}`, + submodulePath, + depth: 1, + state: 'error', + message: state.error + }) + continue + } + if (state.entries.length === 0) { + result.push({ + type: 'submodule-placeholder', + key: `submodule-empty::${entry.area}::${submodulePath}`, + submodulePath, + depth: 1, + state: 'empty', + message: emptyMessage + }) + continue + } + for (const innerEntry of state.entries) { + result.push({ type: 'entry', entry: buildSubmoduleChildEntry(submodulePath, innerEntry) }) + } + } + return result +} + +/** + * Splice lazily-loaded submodule children into a flattened tree row list. Only + * expanded submodules are touched; everything else passes through untouched so + * the status poll stays free of submodule recursion. + */ +export function injectExpandedSubmoduleRows( + nodes: SubmoduleSectionTreeNode[], + expandedSubmodulePaths: ReadonlySet, + submoduleStatusByPath: Readonly>, + loadingMessage: string, + emptyMessage: string +): RenderableSourceControlNode[] { + const result: RenderableSourceControlNode[] = [] + for (const node of nodes) { + result.push(node) + if ( + node.type !== 'file' || + !isExpandableSubmoduleEntry(node.entry) || + !expandedSubmodulePaths.has(node.entry.path) + ) { + continue + } + const submodulePath = node.entry.path + const state = submoduleStatusByPath[submodulePath] + if (!state || state.status === 'loading') { + result.push({ + type: 'submodule-placeholder', + key: `submodule-loading::${node.area}::${submodulePath}`, + submodulePath, + depth: node.depth + 1, + state: 'loading', + message: loadingMessage + }) + continue + } + if (state.status === 'error') { + result.push({ + type: 'submodule-placeholder', + key: `submodule-error::${node.area}::${submodulePath}`, + submodulePath, + depth: node.depth + 1, + state: 'error', + message: state.error + }) + continue + } + if (state.entries.length === 0) { + result.push({ + type: 'submodule-placeholder', + key: `submodule-empty::${node.area}::${submodulePath}`, + submodulePath, + depth: node.depth + 1, + state: 'empty', + message: emptyMessage + }) + continue + } + for (const childNode of buildSubmoduleChildNodes(node, state.entries)) { + result.push(childNode) + } + } + return result +} diff --git a/src/renderer/src/runtime/runtime-git-client.ts b/src/renderer/src/runtime/runtime-git-client.ts index b0795718ac..b170f4e2a8 100644 --- a/src/renderer/src/runtime/runtime-git-client.ts +++ b/src/renderer/src/runtime/runtime-git-client.ts @@ -148,6 +148,29 @@ export async function getRuntimeGitStatus( ) } +export async function getRuntimeGitSubmoduleStatus( + context: RuntimeGitContext, + submodulePath: string +): Promise { + const target = getActiveRuntimeTarget(context.settings) + if (target.kind === 'local' || !context.worktreeId) { + return window.api.git.submoduleStatus({ + worktreePath: context.worktreePath, + submodulePath, + connectionId: context.connectionId + }) + } + return callRuntimeRpc( + target, + 'git.submoduleStatus', + { + worktree: toRuntimeWorktreeSelector(context.worktreeId), + submodulePath + }, + { timeoutMs: 15_000 } + ) +} + export async function getRuntimeGitIgnoredPaths( context: RuntimeGitContext, paths: string[] diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index 4eaf43372f..84e087d32c 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -1393,6 +1393,13 @@ function createGitApi(): NonNullable['git']> { includeIgnored }) }, + submoduleStatus: async ({ worktreePath, submodulePath }) => { + const worktree = await resolveRuntimeWorktreeByPath(worktreePath) + return callRuntimeResult('git.submoduleStatus', { + worktree: toRuntimeWorktreeSelector(worktree.id), + submodulePath + }) + }, checkIgnored: async ({ worktreePath, paths }) => { const worktree = await resolveRuntimeWorktreeByPath(worktreePath) return callRuntimeResult('git.checkIgnored', { diff --git a/src/shared/git-status-types.ts b/src/shared/git-status-types.ts index 8784731fd2..8f1a7008c6 100644 --- a/src/shared/git-status-types.ts +++ b/src/shared/git-status-types.ts @@ -38,6 +38,11 @@ export type GitUncommittedEntry = { conflictStatus?: GitConflictResolutionStatus conflictStatusSource?: GitConflictStatusSource submodule?: GitSubmoduleStatus + // Set on entries that live INSIDE a submodule (lazily loaded when the user + // expands a dirty submodule row). Holds the submodule's path relative to the + // parent worktree. Drives diff routing into the submodule and read-only + // gating — submodule-internal changes are never stageable from the parent. + submoduleRoot?: string // Working-tree line counts for this entry's staging area (staged vs unstaged // diffs are reported separately). Untracked files count their full contents // as additions. Undefined for binary files and when the diff is unavailable. From 24c8c2dbc235622329a221da646febdf2d847b94 Mon Sep 17 00:00:00 2001 From: lvfen <284437334@qq.com> Date: Thu, 25 Jun 2026 15:15:06 +0800 Subject: [PATCH 2/9] feat(source-control): add compare-against-current-branch setting Adds a global setting (default off) that defaults the Source Control compare base to the current branch's upstream so the panel prioritizes local changes instead of the full delta versus the repository default branch. When the branch has no upstream, the compare view falls back to working-tree-only. This affects only the compare/diff view; the Pull Request and rebase merge target are unchanged. --- .../runtime-home-service.test.ts | 1 + src/main/codex-accounts/service.test.ts | 1 + .../SourceControl.compare-summary.test.ts | 67 +++++++++++++++ ...SourceControl.open-file-highlight.test.tsx | 1 + .../SourceControl.preview-open.test.tsx | 1 + .../right-sidebar/SourceControl.tsx | 85 ++++++++++++++----- .../CompareAgainstUpstreamSetting.tsx | 81 ++++++++++++++++++ .../src/components/settings/GitPane.test.ts | 55 ++++++++++++ .../src/components/settings/GitPane.tsx | 11 +++ .../src/components/settings/git-search.ts | 30 +++++++ src/renderer/src/i18n/locales/en.json | 14 ++- src/renderer/src/i18n/locales/es.json | 14 ++- src/renderer/src/i18n/locales/ja.json | 14 ++- src/renderer/src/i18n/locales/ko.json | 14 ++- src/renderer/src/i18n/locales/zh.json | 14 ++- src/renderer/src/store/slices/editor.ts | 29 +++++++ src/shared/constants.ts | 1 + src/shared/types.ts | 5 ++ 18 files changed, 408 insertions(+), 30 deletions(-) create mode 100644 src/renderer/src/components/settings/CompareAgainstUpstreamSetting.tsx diff --git a/src/main/codex-accounts/runtime-home-service.test.ts b/src/main/codex-accounts/runtime-home-service.test.ts index fd09a86fc0..cf08917a19 100644 --- a/src/main/codex-accounts/runtime-home-service.test.ts +++ b/src/main/codex-accounts/runtime-home-service.test.ts @@ -89,6 +89,7 @@ function createSettings(overrides: Partial = {}): GlobalSettings rightSidebarOpenByDefault: true, sourceControlViewMode: 'list', sourceControlGroupOrder: 'changes-first', + sourceControlCompareAgainstUpstream: false, showTitlebarAppName: true, showTasksButton: true, floatingTerminalEnabled: false, diff --git a/src/main/codex-accounts/service.test.ts b/src/main/codex-accounts/service.test.ts index 3d899b25ea..af93168606 100644 --- a/src/main/codex-accounts/service.test.ts +++ b/src/main/codex-accounts/service.test.ts @@ -93,6 +93,7 @@ function createSettings(overrides: Partial = {}): GlobalSettings rightSidebarOpenByDefault: true, sourceControlViewMode: 'list', sourceControlGroupOrder: 'changes-first', + sourceControlCompareAgainstUpstream: false, showTitlebarAppName: true, showTasksButton: true, floatingTerminalEnabled: false, diff --git a/src/renderer/src/components/right-sidebar/SourceControl.compare-summary.test.ts b/src/renderer/src/components/right-sidebar/SourceControl.compare-summary.test.ts index 69a1f3646f..a513471a49 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.compare-summary.test.ts +++ b/src/renderer/src/components/right-sidebar/SourceControl.compare-summary.test.ts @@ -5,6 +5,7 @@ import { CompareSummaryToolbarButton, refreshSourceControlAfterRemoteAction, resolveSourceControlBaseRef, + resolveSourceControlCompareBaseRef, resolveSourceControlPickerBaseRef, shouldRefreshBranchCompareForStatusHead, shouldShowCompareSummary @@ -208,6 +209,72 @@ describe('SourceControl compare summary', () => { ).toBe('origin/main') }) + it('keeps the compare base equal to the merge target when the setting is off', () => { + expect( + resolveSourceControlCompareBaseRef({ + enabled: false, + worktreeBaseRef: null, + repoBaseRef: null, + upstreamName: 'origin/feature', + fallbackBaseRef: 'origin/master' + }) + ).toBe('origin/master') + + expect( + resolveSourceControlCompareBaseRef({ + enabled: false, + upstreamName: 'origin/feature', + fallbackBaseRef: null + }) + ).toBeNull() + }) + + it('prefers a pinned base over the upstream when the setting is on', () => { + expect( + resolveSourceControlCompareBaseRef({ + enabled: true, + worktreeBaseRef: 'refs/remotes/origin/release', + repoBaseRef: 'origin/main', + upstreamName: 'origin/feature', + fallbackBaseRef: 'origin/master' + }) + ).toBe('refs/remotes/origin/release') + + expect( + resolveSourceControlCompareBaseRef({ + enabled: true, + worktreeBaseRef: null, + repoBaseRef: ' origin/main ', + upstreamName: 'origin/feature', + fallbackBaseRef: 'origin/master' + }) + ).toBe('origin/main') + }) + + it('uses the current branch upstream when on and no base is pinned', () => { + expect( + resolveSourceControlCompareBaseRef({ + enabled: true, + worktreeBaseRef: null, + repoBaseRef: null, + upstreamName: 'origin/feature', + fallbackBaseRef: 'origin/master' + }) + ).toBe('origin/feature') + }) + + it('returns null (working-tree only) when on and the branch has no upstream', () => { + expect( + resolveSourceControlCompareBaseRef({ + enabled: true, + worktreeBaseRef: null, + repoBaseRef: null, + upstreamName: null, + fallbackBaseRef: 'origin/master' + }) + ).toBeNull() + }) + it('wires toolbar actions without rendering the dead view-mode toggle', () => { const onChangeBaseRef = vi.fn() const onRetry = vi.fn() 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 6d43872ba6..bddef9ef29 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 @@ -132,6 +132,7 @@ function resetState(overrides: Partial> = {}): void { updateWorktreeGitIdentity: vi.fn(), beginGitBranchCompareRequest: vi.fn(() => 'request-key'), setGitBranchCompareResult: vi.fn(), + clearGitBranchCompare: vi.fn(), fetchUpstreamStatus: noopAsync(), setUpstreamStatus: vi.fn(), pushBranch: noopAsync(), 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 857d043174..61bb6ac363 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 @@ -177,6 +177,7 @@ function resetState(overrides: Partial> = {}): void { updateWorktreeGitIdentity: vi.fn(), beginGitBranchCompareRequest: vi.fn(() => 'request-key'), setGitBranchCompareResult: vi.fn(), + clearGitBranchCompare: vi.fn(), fetchUpstreamStatus: noopAsync(), setUpstreamStatus: vi.fn(), pushBranch: noopAsync(), diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index 78f837f6b6..184a3a341c 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -350,6 +350,30 @@ export function resolveSourceControlBaseRef(input: { return worktreeBaseRef || input.repoBaseRef?.trim() || input.defaultBaseRef?.trim() || null } +// Why: the compare/diff view's base is conceptually distinct from the PR/rebase +// merge target (effectiveBaseRef). When the setting is on, default the compare +// base to the current branch's upstream so the panel surfaces local changes +// instead of the full delta vs the repo default branch. `null` means "no +// committed-changes comparison" (only the working tree), used when a branch +// has no upstream. When the setting is off, fall back to effectiveBaseRef so +// behavior is unchanged. +export function resolveSourceControlCompareBaseRef(input: { + enabled: boolean + worktreeBaseRef?: string | null + repoBaseRef?: string | null + upstreamName?: string | null + fallbackBaseRef?: string | null +}): string | null { + if (!input.enabled) { + return input.fallbackBaseRef?.trim() || null + } + const pinned = input.worktreeBaseRef?.trim() || input.repoBaseRef?.trim() + if (pinned) { + return pinned + } + return input.upstreamName?.trim() || null +} + export function resolveSourceControlPickerBaseRef(input: { pinnedBaseRef?: string | null effectiveBaseRef?: string | null @@ -810,6 +834,7 @@ function SourceControlInner(): React.JSX.Element { const updateWorktreeGitIdentity = useAppStore((s) => s.updateWorktreeGitIdentity) const beginGitBranchCompareRequest = useAppStore((s) => s.beginGitBranchCompareRequest) const setGitBranchCompareResult = useAppStore((s) => s.setGitBranchCompareResult) + const clearGitBranchCompare = useAppStore((s) => s.clearGitBranchCompare) const fetchUpstreamStatus = useAppStore((s) => s.fetchUpstreamStatus) const ensureHostedReviewPushTarget = useAppStore((s) => s.ensureHostedReviewPushTarget) const setUpstreamStatus = useAppStore((s) => s.setUpstreamStatus) @@ -1377,6 +1402,15 @@ function SourceControlInner(): React.JSX.Element { repoBaseRef: normalizedRepoBaseRef, defaultBaseRef }) + // Why: the compare/diff view uses this base; the PR/rebase merge target keeps + // using effectiveBaseRef. When the setting is off, this equals effectiveBaseRef. + const compareBaseRef = resolveSourceControlCompareBaseRef({ + enabled: settings?.sourceControlCompareAgainstUpstream ?? false, + worktreeBaseRef: normalizedWorktreeBaseRef, + repoBaseRef: normalizedRepoBaseRef, + upstreamName: remoteStatus?.upstreamName ?? null, + fallbackBaseRef: effectiveBaseRef + }) const pickerBaseRef = resolveSourceControlPickerBaseRef({ pinnedBaseRef, effectiveBaseRef @@ -1971,11 +2005,11 @@ function SourceControlInner(): React.JSX.Element { // compound flows (runCompoundCommitAction) need handleCommit to // resolve immediately so the push step starts without delay. Errors // here are best-effort — the polling tick will retry. - if (!options?.target && effectiveBaseRef) { + if (!options?.target && compareBaseRef) { beginGitBranchCompareRequest( target.worktreeId, - `${target.worktreeId}:${effectiveBaseRef}:${Date.now()}:post-commit`, - effectiveBaseRef + `${target.worktreeId}:${compareBaseRef}:${Date.now()}:post-commit`, + compareBaseRef ) } if (!options?.target) { @@ -2000,7 +2034,7 @@ function SourceControlInner(): React.JSX.Element { activeWorktreeId, beginGitBranchCompareRequest, commitMessage, - effectiveBaseRef, + compareBaseRef, grouped.staged.length, refreshActiveGitStatusAfterMutation, updateCommitDrafts, @@ -4495,11 +4529,11 @@ function SourceControlInner(): React.JSX.Element { const branchCompareStatusHeadRef = useRef(null) const runBranchCompare = useCallback(async () => { - if (!activeWorktreeId || !worktreePath || !effectiveBaseRef || isFolder) { + if (!activeWorktreeId || !worktreePath || !compareBaseRef || isFolder) { return } - const requestKey = `${activeWorktreeId}:${effectiveBaseRef}:${Date.now()}` + const requestKey = `${activeWorktreeId}:${compareBaseRef}:${Date.now()}` const existingSummary = useAppStore.getState().gitBranchCompareSummaryByWorktree[activeWorktreeId] @@ -4510,12 +4544,12 @@ function SourceControlInner(): React.JSX.Element { // current UI visible until the new IPC result arrives. Resetting to // 'loading' on every poll when the compare is in an error state caused a // visible loading→error→loading→error flicker. - const baseRefChanged = existingSummary && existingSummary.baseRef !== effectiveBaseRef + const baseRefChanged = existingSummary && existingSummary.baseRef !== compareBaseRef const shouldResetToLoading = !existingSummary || baseRefChanged if (shouldResetToLoading) { - beginGitBranchCompareRequest(activeWorktreeId, requestKey, effectiveBaseRef) + beginGitBranchCompareRequest(activeWorktreeId, requestKey, compareBaseRef) } else { - beginGitBranchCompareRequest(activeWorktreeId, requestKey, effectiveBaseRef, { + beginGitBranchCompareRequest(activeWorktreeId, requestKey, compareBaseRef, { preserveExistingSummary: true }) } @@ -4530,13 +4564,13 @@ function SourceControlInner(): React.JSX.Element { worktreePath, connectionId }, - effectiveBaseRef + compareBaseRef ) setGitBranchCompareResult(activeWorktreeId, requestKey, result) } catch (error) { setGitBranchCompareResult(activeWorktreeId, requestKey, { summary: { - baseRef: effectiveBaseRef, + baseRef: compareBaseRef, baseOid: null, compareRef: branchName, headOid: null, @@ -4553,7 +4587,7 @@ function SourceControlInner(): React.JSX.Element { activeWorktreeId, beginGitBranchCompareRequest, branchName, - effectiveBaseRef, + compareBaseRef, isFolder, setGitBranchCompareResult, worktreePath @@ -4629,7 +4663,7 @@ function SourceControlInner(): React.JSX.Element { worktreePath, connectionId }, - { limit: 50, baseRef: effectiveBaseRef } + { limit: 50, baseRef: compareBaseRef } ) if (gitHistoryRequestByWorktreeRef.current[worktreeId] !== requestId) { return @@ -4653,7 +4687,7 @@ function SourceControlInner(): React.JSX.Element { }, [ activeRepoSettings, activeWorktreeId, - effectiveBaseRef, + compareBaseRef, isBranchVisible, isFolder, isGitHistoryExpanded, @@ -4665,13 +4699,13 @@ function SourceControlInner(): React.JSX.Element { refreshGitHistoryRef.current = refreshGitHistory useEffect(() => { - if (!activeWorktreeId || !worktreePath || !isBranchVisible || !effectiveBaseRef || isFolder) { + if (!activeWorktreeId || !worktreePath || !isBranchVisible || !compareBaseRef || isFolder) { branchCompareStatusHeadRef.current = null return } const current = { - baseRef: effectiveBaseRef, + baseRef: compareBaseRef, statusHead: activeGitStatusHead, worktreeId: activeWorktreeId } @@ -4683,14 +4717,14 @@ function SourceControlInner(): React.JSX.Element { }, [ activeGitStatusHead, activeWorktreeId, - effectiveBaseRef, + compareBaseRef, isBranchVisible, isFolder, worktreePath ]) useEffect(() => { - if (!activeWorktreeId || !worktreePath || !isBranchVisible || !effectiveBaseRef || isFolder) { + if (!activeWorktreeId || !worktreePath || !isBranchVisible || !compareBaseRef || isFolder) { return } @@ -4700,7 +4734,18 @@ function SourceControlInner(): React.JSX.Element { run: () => void refreshBranchCompareRef.current(), intervalMs: BRANCH_REFRESH_INTERVAL_MS }) - }, [activeWorktreeId, effectiveBaseRef, isBranchVisible, isFolder, worktreePath]) + }, [activeWorktreeId, compareBaseRef, isBranchVisible, isFolder, worktreePath]) + + useEffect(() => { + // Why: when there is no compare base (prefer-upstream setting on, branch has + // no upstream), runBranchCompare bails out; drop any stale summary so the + // committed-changes section and "vs" row disappear and only the working tree + // shows. + if (!activeWorktreeId || isFolder || compareBaseRef) { + return + } + clearGitBranchCompare(activeWorktreeId) + }, [activeWorktreeId, clearGitBranchCompare, compareBaseRef, isFolder]) useEffect(() => { // Why: history shells out to git. Defer the first load until the user @@ -5302,7 +5347,7 @@ function SourceControlInner(): React.JSX.Element { diffCommentCount={diffCommentCount} onExpandNotes={() => setDiffCommentsExpanded(true)} branchSummary={branchSummary} - compareBaseRef={effectiveBaseRef} + compareBaseRef={compareBaseRef} upstreamStatus={remoteStatus} /> diff --git a/src/renderer/src/components/settings/CompareAgainstUpstreamSetting.tsx b/src/renderer/src/components/settings/CompareAgainstUpstreamSetting.tsx new file mode 100644 index 0000000000..9792390503 --- /dev/null +++ b/src/renderer/src/components/settings/CompareAgainstUpstreamSetting.tsx @@ -0,0 +1,81 @@ +import type { GlobalSettings } from '../../../../shared/types' +import { Label } from '../ui/label' +import { translate } from '@/i18n/i18n' +import { SearchableSetting } from './SearchableSetting' +import { matchesSettingsSearch } from './settings-search' + +export const COMPARE_AGAINST_UPSTREAM_KEYWORDS = [ + 'compare base', + 'current branch', + 'upstream', + 'local changes', + 'origin/master', + 'committed changes', + 'diff base', + 'source control' +] + +function getCompareAgainstUpstreamTitle(): string { + return translate( + 'auto.components.settings.GitPane.compareAgainstUpstreamTitle', + 'Compare Against Current Branch' + ) +} + +function getCompareAgainstUpstreamDescription(): string { + return translate( + 'auto.components.settings.GitPane.compareAgainstUpstreamDescription', + "Default the Source Control compare base to the current branch's upstream so it prioritizes local changes, instead of the repository default branch. Only affects the compare view, not the Pull Request or rebase target." + ) +} + +export function compareAgainstUpstreamMatchesSearch(searchQuery: string): boolean { + return matchesSettingsSearch(searchQuery, { + title: getCompareAgainstUpstreamTitle(), + description: getCompareAgainstUpstreamDescription(), + keywords: COMPARE_AGAINST_UPSTREAM_KEYWORDS + }) +} + +export function CompareAgainstUpstreamSetting({ + settings, + updateSettings +}: { + settings: GlobalSettings + updateSettings: (updates: Partial) => void | Promise +}): React.JSX.Element { + const title = getCompareAgainstUpstreamTitle() + const description = getCompareAgainstUpstreamDescription() + + return ( + +
+ +

{description}

+
+ + + ) +} diff --git a/src/renderer/src/components/settings/GitPane.test.ts b/src/renderer/src/components/settings/GitPane.test.ts index ab2ef19c1b..8bc7adc31f 100644 --- a/src/renderer/src/components/settings/GitPane.test.ts +++ b/src/renderer/src/components/settings/GitPane.test.ts @@ -163,4 +163,59 @@ describe('GitPane', () => { expect(matchesSettingsSearch('staged', getGitPaneSearchEntries())).toBe(true) expect(matchesSettingsSearch('group order', getGitPaneSearchEntries())).toBe(true) }) + + it('renders the compare-against-current-branch setting in Git settings', () => { + const markup = renderGitPane('compare base') + + expect(markup).toContain( + translate( + 'auto.components.settings.GitPane.compareAgainstUpstreamTitle', + 'Compare Against Current Branch' + ) + ) + }) + + it('includes compare-against-current-branch search metadata', () => { + expect(matchesSettingsSearch('compare base', getGitPaneSearchEntries())).toBe(true) + expect(matchesSettingsSearch('upstream', getGitPaneSearchEntries())).toBe(true) + }) + + it('reflects the compare-against-current-branch setting state in the toggle', () => { + useAppStore.setState({ settingsSearchQuery: 'compare base' }) + const off = renderToStaticMarkup( + React.createElement( + TooltipProvider, + null, + React.createElement(GitPane, { + settings: { + ...getDefaultSettings(os.homedir()), + sourceControlCompareAgainstUpstream: false + }, + updateSettings: () => {}, + writeSourceControlAiSettings: async () => {}, + displayedGitUsername: 'brennan', + settingsSearchQuery: 'compare base' + }) + ) + ) + const on = renderToStaticMarkup( + React.createElement( + TooltipProvider, + null, + React.createElement(GitPane, { + settings: { + ...getDefaultSettings(os.homedir()), + sourceControlCompareAgainstUpstream: true + }, + updateSettings: () => {}, + writeSourceControlAiSettings: async () => {}, + displayedGitUsername: 'brennan', + settingsSearchQuery: 'compare base' + }) + ) + ) + + expect(off).toContain('aria-checked="false"') + expect(on).toContain('aria-checked="true"') + }) }) diff --git a/src/renderer/src/components/settings/GitPane.tsx b/src/renderer/src/components/settings/GitPane.tsx index 2579350fe7..6d346fb1de 100644 --- a/src/renderer/src/components/settings/GitPane.tsx +++ b/src/renderer/src/components/settings/GitPane.tsx @@ -8,6 +8,10 @@ import { getGitPaneSearchEntries } from './git-search' import { SearchableSetting } from './SearchableSetting' import { matchesSettingsSearch } from './settings-search' import { AutoRenameBranchFromWorkSetting } from './AutoRenameBranchFromWorkSetting' +import { + CompareAgainstUpstreamSetting, + compareAgainstUpstreamMatchesSearch +} from './CompareAgainstUpstreamSetting' import { getAutoRenameBranchSearchEntries } from './auto-rename-branch-search' import { KEEP_LOCAL_MAIN_UP_TO_DATE_SECTION_ID, @@ -285,6 +289,13 @@ export function GitPane({ updateSettings={updateSettings} /> ) : null, + compareAgainstUpstreamMatchesSearch(searchQuery) ? ( + + ) : null, shouldShowAutoRenameBranchSetting(searchQuery, hasUnsavedBranchPromptChanges) ? ( [ ...translateSearchKeyword('auto.components.settings.git.search.gitChanges', 'git changes') ] }, + { + title: translate( + 'auto.components.settings.git.search.compareAgainstUpstreamTitle', + 'Compare Against Current Branch' + ), + description: translate( + 'auto.components.settings.git.search.compareAgainstUpstreamDescription', + "Default the Source Control compare base to the current branch's upstream so it prioritizes local changes, instead of the repository default branch. Only affects the compare view, not the Pull Request or rebase target." + ), + keywords: [ + ...translateSearchKeyword('auto.components.settings.git.search.compareBase', 'compare base'), + ...translateSearchKeyword( + 'auto.components.settings.git.search.currentBranch', + 'current branch' + ), + ...translateSearchKeyword('auto.components.settings.git.search.upstream', 'upstream'), + ...translateSearchKeyword( + 'auto.components.settings.git.search.localChanges', + 'local changes' + ), + ...translateSearchKeyword( + 'auto.components.settings.git.search.originMaster', + 'origin/master' + ), + ...translateSearchKeyword( + 'auto.components.settings.git.search.committedChanges', + 'committed changes' + ) + ] + }, ...getAutoRenameBranchSearchEntries(), { title: translate('auto.components.settings.git.search.bc7d9f69ce', 'Orca Attribution'), diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 7e004df669..939a115b85 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -5118,7 +5118,9 @@ "sourceControlGroupOrderDescription": "Choose whether Changes, Staged Changes, or Untracked Files appear first in Source Control.", "changesFirst": "Changes first", "stagedFirst": "Staged first", - "untrackedFirst": "Untracked first" + "untrackedFirst": "Untracked first", + "compareAgainstUpstreamTitle": "Compare Against Current Branch", + "compareAgainstUpstreamDescription": "Default the Source Control compare base to the current branch's upstream so it prioritizes local changes, instead of the repository default branch. Only affects the compare view, not the Pull Request or rebase target." }, "HiddenExperimentalGroup": { "d0f914a528": "Placeholder toggle", @@ -7231,7 +7233,15 @@ "stagedFirst": "staged first", "untrackedFirst": "untracked first", "sourceControl": "source control", - "gitChanges": "git changes" + "gitChanges": "git changes", + "compareAgainstUpstreamTitle": "Compare Against Current Branch", + "compareAgainstUpstreamDescription": "Default the Source Control compare base to the current branch's upstream so it prioritizes local changes, instead of the repository default branch. Only affects the compare view, not the Pull Request or rebase target.", + "compareBase": "compare base", + "currentBranch": "current branch", + "upstream": "upstream", + "localChanges": "local changes", + "originMaster": "origin/master", + "committedChanges": "committed changes" } }, "input": { diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index d01aecb641..47146cd45b 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -5081,7 +5081,9 @@ "sourceControlGroupOrderDescription": "Elige si Cambios, Cambios preparados o Archivos sin seguimiento aparecen primero en Control de código fuente.", "changesFirst": "Cambios primero", "stagedFirst": "Cambios preparados primero", - "untrackedFirst": "Archivos sin seguimiento primero" + "untrackedFirst": "Archivos sin seguimiento primero", + "compareAgainstUpstreamTitle": "Comparar con la rama actual", + "compareAgainstUpstreamDescription": "Establece la base de comparación de Control de código fuente en el upstream de la rama actual para priorizar los cambios locales, en lugar de la rama predeterminada del repositorio. Solo afecta a la vista de comparación, no al objetivo de Pull Request o rebase." }, "HiddenExperimentalGroup": { "d0f914a528": "Alternar marcador de posición", @@ -7194,7 +7196,15 @@ "stagedFirst": "cambios preparados primero", "untrackedFirst": "sin seguimiento primero", "sourceControl": "control de código fuente", - "gitChanges": "cambios de git" + "gitChanges": "cambios de git", + "compareAgainstUpstreamTitle": "Comparar con la rama actual", + "compareAgainstUpstreamDescription": "Establece la base de comparación de Control de código fuente en el upstream de la rama actual para priorizar los cambios locales, en lugar de la rama predeterminada del repositorio. Solo afecta a la vista de comparación, no al objetivo de Pull Request o rebase.", + "compareBase": "base de comparación", + "currentBranch": "rama actual", + "upstream": "upstream", + "localChanges": "cambios locales", + "originMaster": "origin/master", + "committedChanges": "cambios confirmados" } }, "input": { diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 138e10813f..48a129d607 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -5103,7 +5103,9 @@ "sourceControlGroupOrderDescription": "ソース管理で「変更」「ステージ済みの変更」「未追跡ファイル」のどれを先に表示するかを選択します。", "changesFirst": "変更を先頭", "stagedFirst": "ステージ済みを先頭", - "untrackedFirst": "未追跡を先頭" + "untrackedFirst": "未追跡を先頭", + "compareAgainstUpstreamTitle": "現在のブランチと比較", + "compareAgainstUpstreamDescription": "ソース管理の比較基準を既定で現在のブランチの上流に設定し、リポジトリの既定ブランチではなくローカルの変更を優先表示します。比較ビューのみに影響し、Pull Request やリベース先には影響しません。" }, "HiddenExperimentalGroup": { "d0f914a528": "プレースホルダーの切り替え", @@ -7216,7 +7218,15 @@ "stagedFirst": "ステージ済みを先頭", "untrackedFirst": "未追跡を先頭", "sourceControl": "ソース管理", - "gitChanges": "git の変更" + "gitChanges": "git の変更", + "compareAgainstUpstreamTitle": "現在のブランチと比較", + "compareAgainstUpstreamDescription": "ソース管理の比較基準を既定で現在のブランチの上流に設定し、リポジトリの既定ブランチではなくローカルの変更を優先表示します。比較ビューのみに影響し、Pull Request やリベース先には影響しません。", + "compareBase": "比較基準", + "currentBranch": "現在のブランチ", + "upstream": "上流", + "localChanges": "ローカルの変更", + "originMaster": "origin/master", + "committedChanges": "コミット済みの変更" } }, "input": { diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 64bf5cd45d..db7494670a 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -5066,7 +5066,9 @@ "sourceControlGroupOrderDescription": "소스 제어에서 변경 사항, 스테이징된 변경 사항 또는 추적되지 않는 파일 중 무엇을 먼저 표시할지 선택합니다.", "changesFirst": "변경 사항 먼저", "stagedFirst": "스테이징된 항목 먼저", - "untrackedFirst": "추적되지 않는 파일 먼저" + "untrackedFirst": "추적되지 않는 파일 먼저", + "compareAgainstUpstreamTitle": "현재 브랜치와 비교", + "compareAgainstUpstreamDescription": "소스 제어 비교 기준을 기본적으로 현재 브랜치의 업스트림으로 설정하여 저장소 기본 브랜치 대신 로컬 변경 사항을 우선 표시합니다. 비교 보기에만 영향을 주며 Pull Request 또는 리베이스 대상에는 영향을 주지 않습니다." }, "HiddenExperimentalGroup": { "d0f914a528": "자리 표시자 토글", @@ -7179,7 +7181,15 @@ "stagedFirst": "스테이징된 항목 먼저", "untrackedFirst": "추적되지 않는 파일 먼저", "sourceControl": "소스 제어", - "gitChanges": "git 변경 사항" + "gitChanges": "git 변경 사항", + "compareAgainstUpstreamTitle": "현재 브랜치와 비교", + "compareAgainstUpstreamDescription": "소스 제어 비교 기준을 기본적으로 현재 브랜치의 업스트림으로 설정하여 저장소 기본 브랜치 대신 로컬 변경 사항을 우선 표시합니다. 비교 보기에만 영향을 주며 Pull Request 또는 리베이스 대상에는 영향을 주지 않습니다.", + "compareBase": "비교 기준", + "currentBranch": "현재 브랜치", + "upstream": "업스트림", + "localChanges": "로컬 변경 사항", + "originMaster": "origin/master", + "committedChanges": "커밋된 변경 사항" } }, "input": { diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 1e46ef2fb9..e60267d1f8 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -5066,7 +5066,9 @@ "sourceControlGroupOrderDescription": "选择在源代码管理中优先显示“更改”、“已暂存更改”还是“未跟踪文件”。", "changesFirst": "更改优先", "stagedFirst": "已暂存优先", - "untrackedFirst": "未跟踪优先" + "untrackedFirst": "未跟踪优先", + "compareAgainstUpstreamTitle": "对比当前分支", + "compareAgainstUpstreamDescription": "将源代码管理的对比基准默认设为当前分支的上游，从而优先显示本地变更，而不是仓库的默认分支。仅影响对比视图，不影响 Pull Request 或变基目标。" }, "HiddenExperimentalGroup": { "d0f914a528": "占位符切换", @@ -7179,7 +7181,15 @@ "stagedFirst": "已暂存优先", "untrackedFirst": "未跟踪优先", "sourceControl": "源代码管理", - "gitChanges": "git 更改" + "gitChanges": "git 更改", + "compareAgainstUpstreamTitle": "对比当前分支", + "compareAgainstUpstreamDescription": "将源代码管理的对比基准默认设为当前分支的上游，从而优先显示本地变更，而不是仓库的默认分支。仅影响对比视图，不影响 Pull Request 或变基目标。", + "compareBase": "对比基准", + "currentBranch": "当前分支", + "upstream": "上游", + "localChanges": "本地变更", + "originMaster": "origin/master", + "committedChanges": "已提交的更改" } }, "input": { diff --git a/src/renderer/src/store/slices/editor.ts b/src/renderer/src/store/slices/editor.ts index 213f575e25..d45144e387 100644 --- a/src/renderer/src/store/slices/editor.ts +++ b/src/renderer/src/store/slices/editor.ts @@ -657,6 +657,7 @@ export type EditorSlice = { requestKey: string, result: { summary: GitBranchCompareSummary; entries: GitBranchChangeEntry[] } ) => void + clearGitBranchCompare: (worktreeId: string) => void // File search state fileSearchStateByWorktree: Record< @@ -3843,6 +3844,34 @@ export const createEditorSlice: StateCreator = (s : { ...s.gitBranchCompareSummaryByWorktree, [worktreeId]: result.summary } } }), + // Why: when the compare base resolves to "no base" (e.g. the prefer-upstream + // setting is on and the branch has no upstream), drop any stale summary so the + // committed-changes section and "vs" row disappear instead of lingering. + clearGitBranchCompare: (worktreeId) => + set((s) => { + if ( + s.gitBranchCompareSummaryByWorktree[worktreeId] === undefined && + s.gitBranchChangesByWorktree[worktreeId] === undefined && + s.gitBranchCompareRequestKeyByWorktree[worktreeId] === undefined && + s.gitBranchCompareRequestStatusHeadByWorktree[worktreeId] === undefined + ) { + return s + } + const nextSummary = { ...s.gitBranchCompareSummaryByWorktree } + const nextChanges = { ...s.gitBranchChangesByWorktree } + const nextRequestKey = { ...s.gitBranchCompareRequestKeyByWorktree } + const nextRequestHead = { ...s.gitBranchCompareRequestStatusHeadByWorktree } + delete nextSummary[worktreeId] + delete nextChanges[worktreeId] + delete nextRequestKey[worktreeId] + delete nextRequestHead[worktreeId] + return { + gitBranchCompareSummaryByWorktree: nextSummary, + gitBranchChangesByWorktree: nextChanges, + gitBranchCompareRequestKeyByWorktree: nextRequestKey, + gitBranchCompareRequestStatusHeadByWorktree: nextRequestHead + } + }), // File search fileSearchStateByWorktree: {}, diff --git a/src/shared/constants.ts b/src/shared/constants.ts index 5c82398ff9..c1203a48d4 100644 --- a/src/shared/constants.ts +++ b/src/shared/constants.ts @@ -270,6 +270,7 @@ export function getDefaultSettings(homedir: string): GlobalSettings { showGitIgnoredFiles: true, sourceControlViewMode: 'list', sourceControlGroupOrder: DEFAULT_SOURCE_CONTROL_GROUP_ORDER, + sourceControlCompareAgainstUpstream: false, showTitlebarAppName: true, showTasksButton: true, showAutomationsButton: true, diff --git a/src/shared/types.ts b/src/shared/types.ts index 9d904f1c6a..162a8b2a0c 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -2515,6 +2515,11 @@ export type GlobalSettings = { sourceControlViewMode: SourceControlViewMode /** Preferred Source Control group order. Per-user, not per-workspace. */ sourceControlGroupOrder: SourceControlGroupOrder + /** When enabled, the Source Control compare base defaults to the current + * branch's upstream (prioritizing local changes) instead of the repo + * default branch. Only affects the compare/diff view, not the PR/rebase + * merge target. Per-user, not per-workspace. */ + sourceControlCompareAgainstUpstream: boolean /** Whether to show the Orca app name in the titlebar. */ showTitlebarAppName: boolean /** Why: some users do not use the Tasks feature and prefer to keep the From a4cfe12756eb1d50323851c25cc9a4eefaceaf40 Mon Sep 17 00:00:00 2001 From: lvfen <284437334@qq.com> Date: Thu, 25 Jun 2026 17:18:14 +0800 Subject: [PATCH 3/9] refactor(source-control): extract submodule status hook and entry-action gates Moves the lazy submodule-expansion state into a useSourceControlSubmoduleStatus hook and centralizes per-row stage/unstage/discard eligibility into source-control-entry-actions, shrinking SourceControl.tsx and keeping the read-only submodule rules consistent across the row UI, bulk actions, and tests. The hook adds a generation guard so a slow submodule-status response from a previous worktree (common over SSH) can't write stale status into the current panel. On the relay side, configured submodule paths are read through a short-TTL per-instance cache so a burst of diff clicks does not re-read .gitmodules over the SSH link. Adds tests for the new modules. --- src/relay/git-handler-submodule-ops.test.ts | 72 ++++++++ src/relay/git-handler-submodule-ops.ts | 34 ++++ src/relay/git-handler.ts | 17 +- .../right-sidebar/SourceControl.tsx | 99 ++--------- .../source-control-entry-actions.test.ts | 48 +++++ .../source-control-entry-actions.ts | 27 +++ .../useSourceControlSubmoduleStatus.test.tsx | 164 ++++++++++++++++++ .../useSourceControlSubmoduleStatus.ts | 118 +++++++++++++ 8 files changed, 496 insertions(+), 83 deletions(-) create mode 100644 src/relay/git-handler-submodule-ops.test.ts create mode 100644 src/renderer/src/components/right-sidebar/source-control-entry-actions.test.ts create mode 100644 src/renderer/src/components/right-sidebar/source-control-entry-actions.ts create mode 100644 src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.test.tsx create mode 100644 src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.ts diff --git a/src/relay/git-handler-submodule-ops.test.ts b/src/relay/git-handler-submodule-ops.test.ts new file mode 100644 index 0000000000..e03bd29890 --- /dev/null +++ b/src/relay/git-handler-submodule-ops.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from 'vitest' +import type { GitExec } from './git-handler-ops' +import { + SUBMODULE_PATHS_CACHE_TTL_MS, + createSubmodulePathsCache, + listSubmodulePathsCached +} from './git-handler-submodule-ops' + +function gitmodulesExec(paths: string[]): { git: GitExec; calls: () => number } { + let calls = 0 + const git: GitExec = async (args) => { + if (args[0] === 'config' && args.includes('.gitmodules')) { + calls += 1 + return { + stdout: paths.map((p, i) => `submodule.sub${i}.path ${p}`).join('\n'), + stderr: '' + } + } + return { stdout: '', stderr: '' } + } + return { git, calls: () => calls } +} + +describe('listSubmodulePathsCached', () => { + it('reads .gitmodules once for repeated diffs on the same worktree within TTL', async () => { + const { git, calls } = gitmodulesExec(['vendor/lib']) + const cache = createSubmodulePathsCache() + + const first = await listSubmodulePathsCached(git, '/repo', cache, 1_000) + const second = await listSubmodulePathsCached(git, '/repo', cache, 1_500) + + expect(first).toEqual(['vendor/lib']) + expect(second).toEqual(['vendor/lib']) + expect(calls()).toBe(1) + }) + + it('re-reads after the TTL expires', async () => { + const { git, calls } = gitmodulesExec(['vendor/lib']) + const cache = createSubmodulePathsCache() + + await listSubmodulePathsCached(git, '/repo', cache, 1_000) + await listSubmodulePathsCached(git, '/repo', cache, 1_000 + SUBMODULE_PATHS_CACHE_TTL_MS + 1) + + expect(calls()).toBe(2) + }) + + it('reads separately for different worktrees', async () => { + const { git, calls } = gitmodulesExec(['vendor/lib']) + const cache = createSubmodulePathsCache() + + await listSubmodulePathsCached(git, '/repo-a', cache, 1_000) + await listSubmodulePathsCached(git, '/repo-b', cache, 1_000) + + expect(calls()).toBe(2) + }) + + it('caches an empty result so a submodule-free repo is not re-read', async () => { + let calls = 0 + const git: GitExec = async () => { + calls += 1 + throw new Error('fatal: No such file or directory') + } + const cache = createSubmodulePathsCache() + + const first = await listSubmodulePathsCached(git, '/repo', cache, 1_000) + const second = await listSubmodulePathsCached(git, '/repo', cache, 1_200) + + expect(first).toEqual([]) + expect(second).toEqual([]) + expect(calls).toBe(1) + }) +}) diff --git a/src/relay/git-handler-submodule-ops.ts b/src/relay/git-handler-submodule-ops.ts index fb0b2290a2..d414cc6757 100644 --- a/src/relay/git-handler-submodule-ops.ts +++ b/src/relay/git-handler-submodule-ops.ts @@ -13,6 +13,40 @@ import { parseBranchDiff } from './git-handler-utils' import { parseNumstat } from '../shared/git-uncommitted-line-stats' import { readBlobAtOid, type GitBufferExec, type GitExec } from './git-handler-ops' +/** + * Short TTL for the configured-submodule-paths cache, matching the local + * handler (src/main/git/status.ts) so a burst of diff clicks on a worktree + * doesn't re-read `.gitmodules` over the (possibly high-latency) SSH link. + */ +export const SUBMODULE_PATHS_CACHE_TTL_MS = 5_000 +type SubmodulePathsCacheEntry = { paths: string[]; expiresAt: number } +export type SubmodulePathsCache = Map + +export function createSubmodulePathsCache(): SubmodulePathsCache { + return new Map() +} + +/** + * Cached variant of {@link listSubmodulePaths}. The cache is passed in (held by + * the GitHandler instance) so it is naturally bound to the connection lifecycle + * and never leaks across relay instances or tests. An empty result is cached + * too, so a submodule-free repo doesn't re-read `.gitmodules` on every diff. + */ +export async function listSubmodulePathsCached( + git: GitExec, + worktreePath: string, + cache: SubmodulePathsCache, + now: number = Date.now() +): Promise { + const cached = cache.get(worktreePath) + if (cached && cached.expiresAt > now) { + return cached.paths + } + const paths = await listSubmodulePaths(git, worktreePath) + cache.set(worktreePath, { paths, expiresAt: now + SUBMODULE_PATHS_CACHE_TTL_MS }) + return paths +} + /** * Configured submodule paths (relative, forward-slash) read from `.gitmodules`. * Used to route gitlink/inner diffs without an index-wide `ls-files` scan. diff --git a/src/relay/git-handler.ts b/src/relay/git-handler.ts index edf363eac4..5de2a133d1 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -22,9 +22,11 @@ import { buildSubmoduleInnerCommitRangeDiff, computeSubmodulePointerDiff, computeSubmoduleRangeEntries, + createSubmodulePathsCache, findContainingSubmodule, - listSubmodulePaths, - resolveSubmoduleCommitRange + listSubmodulePathsCached, + resolveSubmoduleCommitRange, + type SubmodulePathsCache } from './git-handler-submodule-ops' import { commitCompare as commitCompareOp, commitDiffEntry } from './git-handler-commit-diff-ops' import { @@ -97,6 +99,11 @@ function execFileWithStdin( export class GitHandler { private dispatcher: RelayDispatcher + // Why: configured submodule paths change rarely; an instance-level TTL cache + // avoids re-reading `.gitmodules` on every diff click over SSH, and being + // per-instance it stays bound to the connection lifecycle (no cross-test leak). + private submodulePathsCache: SubmodulePathsCache = createSubmodulePathsCache() + // Why: RelayContext is accepted for protocol back-compat (see // docs/relay-fs-allowlist-removal.md) but no longer consulted on git ops. constructor(dispatcher: RelayDispatcher, _context: RelayContext) { @@ -267,7 +274,11 @@ export class GitHandler { // Why: gitlink paths can't be read as blobs and submodule working dirs read // as empty, so route the gitlink root → pointer diff and inner files → // recurse into the submodule's own worktree (mirrors the local handler). - const submodulePaths = await listSubmodulePaths(this.git.bind(this), worktreePath) + const submodulePaths = await listSubmodulePathsCached( + this.git.bind(this), + worktreePath, + this.submodulePathsCache + ) if (submodulePaths.length > 0) { const matchedSubmodule = findContainingSubmodule(submodulePaths, filePath) if (matchedSubmodule) { diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index 184a3a341c..8f96e8c5bf 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -73,6 +73,11 @@ import { runDiscardAllForArea, type DiscardAllArea } from './discard-all-sequence' +import { + canDiscardStatusEntry, + canStageStatusEntry, + canUnstageStatusEntry +} from './source-control-entry-actions' import { getFileTypeIcon } from '@/lib/file-type-icons' import { buildGitStatusSourceControlTree, @@ -89,9 +94,9 @@ import { injectExpandedSubmoduleRows, isExpandableSubmoduleEntry, type RenderableSourceControlNode, - type RenderableSubmoduleListItem, - type SubmoduleStatusState + type RenderableSubmoduleListItem } from './source-control-submodule-expansion' +import { useSourceControlSubmoduleStatus } from './useSourceControlSubmoduleStatus' import { buildSourceControlDisplaySections, getSourceControlSectionViewAction, @@ -167,7 +172,6 @@ import { generateRuntimePullRequestFields, getRuntimeGitBranchCompare, getRuntimeGitHistory, - getRuntimeGitSubmoduleStatus, stageRuntimeGitPath, unstageRuntimeGitPath, type RuntimeGitContext, @@ -985,13 +989,6 @@ function SourceControlInner(): React.JSX.Element { const sourceControlViewMode = persistedSourceControlViewMode const sourceControlGroupOrder = resolveSourceControlGroupOrder(settings?.sourceControlGroupOrder) const [collapsedTreeDirs, setCollapsedTreeDirs] = useState>(new Set()) - // Why: dirty submodules are shown collapsed by default and only query their - // inner status when the user expands them, so the status poll never recurses - // into (potentially nested) submodules. Results are cached per submodule path. - const [expandedSubmodulePaths, setExpandedSubmodulePaths] = useState>(new Set()) - const [submoduleStatusByPath, setSubmoduleStatusByPath] = useState< - Record - >({}) const [baseRefDialogOpen, setBaseRefDialogOpen] = useState(false) const [pendingDiscard, setPendingDiscard] = useState(null) // Why: start null rather than 'origin/main' so branch compare doesn't fire @@ -1130,6 +1127,13 @@ function SourceControlInner(): React.JSX.Element { const isFolder = activeRepo ? isFolderRepo(activeRepo) : false const worktreePath = activeWorktree?.path ?? null + const { expandedSubmodulePaths, submoduleStatusByPath, toggleSubmodule } = + useSourceControlSubmoduleStatus({ + activeWorktreeId, + worktreePath, + activeRepoSettings, + entries + }) const activeCommitMessageGenerationKey = getCommitMessageGenerationRecordKey( activeWorktreeId, worktreePath @@ -1890,8 +1894,6 @@ function SourceControlInner(): React.JSX.Element { setFilterExpanded(false) setCollapsedSections(createDefaultCollapsedSections()) setCollapsedTreeDirs(new Set()) - setExpandedSubmodulePaths(new Set()) - setSubmoduleStatusByPath({}) setBaseRefDialogOpen(false) setPendingDiscard(null) setPendingDiffCommentsClear(null) @@ -4814,66 +4816,6 @@ function SourceControlInner(): React.JSX.Element { }) }, []) - const fetchSubmoduleStatus = useCallback( - async (submodulePath: string): Promise => { - if (!worktreePath) { - return - } - // Why: keep any already-loaded children visible during a poll-driven - // refetch so expanding then refreshing doesn't flash a loading row. - setSubmoduleStatusByPath((prev) => - prev[submodulePath] ? prev : { ...prev, [submodulePath]: { status: 'loading' } } - ) - try { - const connectionId = getConnectionId(activeWorktreeId ?? null) ?? undefined - const result = await getRuntimeGitSubmoduleStatus( - { - // Why: route by the repo OWNER host, matching the rest of this panel. - settings: activeRepoSettings, - worktreeId: activeWorktreeId, - worktreePath, - connectionId - }, - submodulePath - ) - setSubmoduleStatusByPath((prev) => ({ - ...prev, - [submodulePath]: { status: 'loaded', entries: result.entries } - })) - } catch (error) { - setSubmoduleStatusByPath((prev) => ({ - ...prev, - [submodulePath]: { - status: 'error', - error: error instanceof Error ? error.message : String(error) - } - })) - } - }, - [activeRepoSettings, activeWorktreeId, worktreePath] - ) - - const toggleSubmodule = useCallback((submodulePath: string) => { - setExpandedSubmodulePaths((prev) => { - const next = new Set(prev) - if (next.has(submodulePath)) { - next.delete(submodulePath) - } else { - next.add(submodulePath) - } - return next - }) - }, []) - - // Why: (re)load inner status only for currently-expanded submodules. Re-runs - // when the parent status poll refreshes `entries` so expanded children stay - // fresh, while collapsed submodules never trigger any extra git work. - useEffect(() => { - for (const submodulePath of expandedSubmodulePaths) { - void fetchSubmoduleStatus(submodulePath) - } - }, [expandedSubmodulePaths, entries, fetchSubmoduleStatus]) - const openCommittedDiff = useCallback( (entry: GitBranchChangeEntry, event?: SourceControlRowOpenEvent) => { if ( @@ -7919,7 +7861,6 @@ const UncommittedEntryRow = React.memo(function UncommittedEntryRow({ const parentDir = dirname(entry.path) const dirPath = parentDir === '.' ? '' : parentDir const isUnresolvedConflict = entry.conflictStatus === 'unresolved' - const isResolvedLocally = entry.conflictStatus === 'resolved_locally' const isSubmoduleWorktreeOnly = isSubmoduleWorktreeOnlyChange(entry) const conflictLabel = entry.conflictKind ? getLocalizedConflictKindLabel(entry.conflictKind) @@ -7936,13 +7877,11 @@ const UncommittedEntryRow = React.memo(function UncommittedEntryRow({ // For unresolved: discarding is too easy to misfire on a high-risk file. // For resolved_locally: discarding can silently re-create the conflict or // lose the resolution, and v1 does not have UX to explain this clearly. - const canDiscard = - !isUnresolvedConflict && - !isResolvedLocally && - !entry.submoduleRoot && - (entry.area === 'unstaged' || entry.area === 'untracked') - const canStage = isStageableStatusEntry(entry) - const canUnstage = entry.area === 'staged' + const canDiscard = canDiscardStatusEntry(entry) + const canStage = canStageStatusEntry(entry) + // Why: a submodule-internal staged row is read-only from the parent worktree, + // so the parent repo's Unstage must not be offered (mirrors bulk unstage). + const canUnstage = canUnstageStatusEntry(entry) return ( ): GitStatusEntry { + return { + path: 'file.ts', + status: 'modified', + area: 'unstaged', + ...overrides + } as GitStatusEntry +} + +describe('source control entry actions', () => { + it('hides Unstage for submodule-internal staged rows but keeps it for normal staged rows', () => { + expect(canUnstageStatusEntry(entry({ area: 'staged' }))).toBe(true) + expect(canUnstageStatusEntry(entry({ area: 'staged', submoduleRoot: 'vendor/lib' }))).toBe( + false + ) + expect(canUnstageStatusEntry(entry({ area: 'unstaged' }))).toBe(false) + }) + + it('hides Discard for submodule-internal rows and conflict rows, keeps it for normal rows', () => { + expect(canDiscardStatusEntry(entry({ area: 'unstaged' }))).toBe(true) + expect(canDiscardStatusEntry(entry({ area: 'untracked', status: 'untracked' }))).toBe(true) + expect(canDiscardStatusEntry(entry({ area: 'unstaged', submoduleRoot: 'vendor/lib' }))).toBe( + false + ) + expect(canDiscardStatusEntry(entry({ area: 'staged' }))).toBe(false) + expect(canDiscardStatusEntry(entry({ area: 'unstaged', conflictStatus: 'unresolved' }))).toBe( + false + ) + expect( + canDiscardStatusEntry(entry({ area: 'unstaged', conflictStatus: 'resolved_locally' })) + ).toBe(false) + }) + + it('hides Stage for submodule-internal rows', () => { + expect(canStageStatusEntry(entry({ area: 'unstaged' }))).toBe(true) + expect(canStageStatusEntry(entry({ area: 'unstaged', submoduleRoot: 'vendor/lib' }))).toBe( + false + ) + }) +}) diff --git a/src/renderer/src/components/right-sidebar/source-control-entry-actions.ts b/src/renderer/src/components/right-sidebar/source-control-entry-actions.ts new file mode 100644 index 0000000000..b9bd2fe6a5 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control-entry-actions.ts @@ -0,0 +1,27 @@ +import type { GitStatusEntry } from '../../../../shared/types' +import { isStageableStatusEntry } from './discard-all-sequence' + +/** + * Per-row Source Control action eligibility, centralized so the stage/unstage/ + * discard gates stay consistent between the row UI, bulk selection, and tests. + * A submodule-internal row (`submoduleRoot` set) is read-only from the parent + * worktree: the parent repo's git can't stage/unstage/discard changes that live + * in the submodule's own working tree, so those actions are suppressed here. + */ + +export function canStageStatusEntry(entry: GitStatusEntry): boolean { + return isStageableStatusEntry(entry) +} + +export function canUnstageStatusEntry(entry: GitStatusEntry): boolean { + return entry.area === 'staged' && !entry.submoduleRoot +} + +export function canDiscardStatusEntry(entry: GitStatusEntry): boolean { + return ( + entry.conflictStatus !== 'unresolved' && + entry.conflictStatus !== 'resolved_locally' && + !entry.submoduleRoot && + (entry.area === 'unstaged' || entry.area === 'untracked') + ) +} diff --git a/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.test.tsx b/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.test.tsx new file mode 100644 index 0000000000..90563c6c49 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.test.tsx @@ -0,0 +1,164 @@ +// @vitest-environment happy-dom + +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ getRuntimeGitSubmoduleStatus: vi.fn() })) + +vi.mock('@/runtime/runtime-git-client', () => ({ + getRuntimeGitSubmoduleStatus: mocks.getRuntimeGitSubmoduleStatus +})) +vi.mock('@/lib/connection-context', () => ({ getConnectionId: () => undefined })) + +import { + useSourceControlSubmoduleStatus, + type UseSourceControlSubmoduleStatusResult +} from './useSourceControlSubmoduleStatus' +import type { GitStatusEntry } from '../../../../shared/types' + +const roots: Root[] = [] + +function deferred(): { + promise: Promise + resolve: (v: T) => void + reject: (e: unknown) => void +} { + let resolve!: (v: T) => void + let reject!: (e: unknown) => void + const promise = new Promise((res, rej) => { + resolve = res + reject = rej + }) + return { promise, resolve, reject } +} + +async function flush(): Promise { + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) +} + +let latest: UseSourceControlSubmoduleStatusResult | null = null + +function Probe({ + worktreeId, + worktreePath, + entries +}: { + worktreeId: string + worktreePath: string + entries: GitStatusEntry[] +}): null { + latest = useSourceControlSubmoduleStatus({ + activeWorktreeId: worktreeId, + worktreePath, + activeRepoSettings: null, + entries + }) + return null +} + +function innerEntry(path: string): GitStatusEntry { + return { path, status: 'modified', area: 'unstaged' } as GitStatusEntry +} + +afterEach(() => { + act(() => { + for (const root of roots.splice(0)) { + root.unmount() + } + }) + mocks.getRuntimeGitSubmoduleStatus.mockReset() + latest = null +}) + +describe('useSourceControlSubmoduleStatus', () => { + it('drops a late response from a previous worktree when the active worktree changed', async () => { + const a = deferred<{ entries: GitStatusEntry[] }>() + const b = deferred<{ entries: GitStatusEntry[] }>() + mocks.getRuntimeGitSubmoduleStatus.mockImplementation((ctx: { worktreeId?: string | null }) => + ctx.worktreeId === 'A' ? a.promise : b.promise + ) + + const container = document.createElement('div') + const root = createRoot(container) + roots.push(root) + + await act(async () => { + root.render() + }) + // Expand a submodule in worktree A -> issues the (slow) A request. + await act(async () => { + latest?.toggleSubmodule('sub') + }) + await flush() + + // Switch to worktree B (same submodule path) and expand it there. + await act(async () => { + root.render() + }) + await act(async () => { + latest?.toggleSubmodule('sub') + }) + await flush() + + // B resolves first, then the stale A response arrives late. + await act(async () => { + b.resolve({ entries: [innerEntry('from-b.ts')] }) + }) + await flush() + await act(async () => { + a.resolve({ entries: [innerEntry('from-a.ts')] }) + }) + await flush() + + expect(latest?.submoduleStatusByPath.sub).toEqual({ + status: 'loaded', + entries: [innerEntry('from-b.ts')] + }) + }) + + it('does not let a late error from a previous worktree overwrite the current status', async () => { + const a = deferred<{ entries: GitStatusEntry[] }>() + const b = deferred<{ entries: GitStatusEntry[] }>() + mocks.getRuntimeGitSubmoduleStatus.mockImplementation((ctx: { worktreeId?: string | null }) => + ctx.worktreeId === 'A' ? a.promise : b.promise + ) + + const container = document.createElement('div') + const root = createRoot(container) + roots.push(root) + + await act(async () => { + root.render() + }) + await act(async () => { + latest?.toggleSubmodule('sub') + }) + await flush() + + await act(async () => { + root.render() + }) + await act(async () => { + latest?.toggleSubmodule('sub') + }) + await flush() + + await act(async () => { + b.resolve({ entries: [innerEntry('from-b.ts')] }) + }) + await flush() + await act(async () => { + a.reject(new Error('stale worktree failed')) + }) + await flush() + + expect(latest?.submoduleStatusByPath.sub).toEqual({ + status: 'loaded', + entries: [innerEntry('from-b.ts')] + }) + }) +}) diff --git a/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.ts b/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.ts new file mode 100644 index 0000000000..a8fe4de850 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.ts @@ -0,0 +1,118 @@ +import { useCallback, useEffect, useRef, useState } from 'react' +import type { GitStatusEntry } from '../../../../shared/types' +import { getConnectionId } from '@/lib/connection-context' +import { getRuntimeGitSubmoduleStatus, type RuntimeGitContext } from '@/runtime/runtime-git-client' +import type { SubmoduleStatusState } from './source-control-submodule-expansion' + +export type UseSourceControlSubmoduleStatusInput = { + activeWorktreeId: string | null | undefined + worktreePath: string | null + activeRepoSettings: RuntimeGitContext['settings'] + // Why: re-fetch expanded children whenever the parent status poll refreshes + // its entries, so an expanded submodule's inner changes stay fresh. + entries: readonly GitStatusEntry[] +} + +export type UseSourceControlSubmoduleStatusResult = { + expandedSubmodulePaths: Set + submoduleStatusByPath: Record + toggleSubmodule: (submodulePath: string) => void +} + +/** + * Owns the lazy submodule-expansion state for Source Control: which dirty + * submodules are expanded and the on-demand inner status for each. Dirty + * submodules start collapsed and only query their inner `git status` when + * expanded, so the parent status poll never recurses into (possibly nested) + * submodules. + */ +export function useSourceControlSubmoduleStatus( + input: UseSourceControlSubmoduleStatusInput +): UseSourceControlSubmoduleStatusResult { + const { activeWorktreeId, worktreePath, activeRepoSettings, entries } = input + const [expandedSubmodulePaths, setExpandedSubmodulePaths] = useState>(() => new Set()) + const [submoduleStatusByPath, setSubmoduleStatusByPath] = useState< + Record + >({}) + + // Why: a monotonically increasing generation invalidates in-flight requests + // when the active worktree/path changes, so a slow response from a previous + // worktree (common over SSH) can't write stale submodule status into the + // current panel — even when both worktrees share the same submodule path. + const generationRef = useRef(0) + + useEffect(() => { + generationRef.current += 1 + setExpandedSubmodulePaths(new Set()) + setSubmoduleStatusByPath({}) + }, [activeWorktreeId, worktreePath]) + + const fetchSubmoduleStatus = useCallback( + async (submodulePath: string): Promise => { + if (!worktreePath) { + return + } + const generation = generationRef.current + // Why: keep any already-loaded children visible during a poll-driven + // refetch so expanding then refreshing doesn't flash a loading row. + setSubmoduleStatusByPath((prev) => + prev[submodulePath] ? prev : { ...prev, [submodulePath]: { status: 'loading' } } + ) + try { + const connectionId = getConnectionId(activeWorktreeId ?? null) ?? undefined + const result = await getRuntimeGitSubmoduleStatus( + { + // Why: route by the repo OWNER host, matching the rest of this panel. + settings: activeRepoSettings, + worktreeId: activeWorktreeId, + worktreePath, + connectionId + }, + submodulePath + ) + if (generationRef.current !== generation) { + return + } + setSubmoduleStatusByPath((prev) => ({ + ...prev, + [submodulePath]: { status: 'loaded', entries: result.entries } + })) + } catch (error) { + if (generationRef.current !== generation) { + return + } + setSubmoduleStatusByPath((prev) => ({ + ...prev, + [submodulePath]: { + status: 'error', + error: error instanceof Error ? error.message : String(error) + } + })) + } + }, + [activeRepoSettings, activeWorktreeId, worktreePath] + ) + + const toggleSubmodule = useCallback((submodulePath: string) => { + setExpandedSubmodulePaths((prev) => { + const next = new Set(prev) + if (next.has(submodulePath)) { + next.delete(submodulePath) + } else { + next.add(submodulePath) + } + return next + }) + }, []) + + // Why: (re)load inner status only for currently-expanded submodules. Re-runs + // when the parent status poll refreshes `entries` so expanded children stay + // fresh, while collapsed submodules never trigger any extra git work. + useEffect(() => { + for (const submodulePath of expandedSubmodulePaths) { + void fetchSubmoduleStatus(submodulePath) + } + }, [expandedSubmodulePaths, entries, fetchSubmoduleStatus]) + + return { expandedSubmodulePaths, submoduleStatusByPath, toggleSubmodule } +} From 1e4d519806efdbdc18de2e66e7a12b3d4b22cb0a Mon Sep 17 00:00:00 2001 From: lvfen <284437334@qq.com> Date: Fri, 26 Jun 2026 10:06:00 +0800 Subject: [PATCH 4/9] fix(source-control): address submodule/compare review feedback - Degrade git.submoduleStatus to an actionable reconnect hint when an older SSH relay lacks the RPC, mirroring clone()/worktreeIsClean fallbacks. - Keep the branch-compare summary while upstream status is still loading so it no longer flickers when switching worktrees with prefer-upstream on. - Mark the compare-base switch as type="button" to avoid form submission. - Add diff base / source control keywords to the Git settings search catalog. - Assert the compare-base toggle's own switch state and updateSettings call. --- src/main/providers/ssh-git-provider.test.ts | 33 +++++++ src/main/providers/ssh-git-provider.ts | 20 +++- .../SourceControl.compare-summary.test.ts | 55 ++++++++++- .../right-sidebar/SourceControl.tsx | 24 ++++- .../CompareAgainstUpstreamSetting.tsx | 1 + .../src/components/settings/GitPane.test.ts | 99 ++++++++++++------- .../src/components/settings/git-search.ts | 5 + 7 files changed, 195 insertions(+), 42 deletions(-) diff --git a/src/main/providers/ssh-git-provider.test.ts b/src/main/providers/ssh-git-provider.test.ts index a6fd5911fa..f6480dec37 100644 --- a/src/main/providers/ssh-git-provider.test.ts +++ b/src/main/providers/ssh-git-provider.test.ts @@ -85,6 +85,39 @@ describe('SshGitProvider', () => { }) }) + it('getSubmoduleStatus sends git.submoduleStatus request', async () => { + const statusResult = { entries: [], conflictOperation: 'unknown' } + mux.request.mockResolvedValue(statusResult) + + const result = await provider.getSubmoduleStatus('/home/user/repo', 'vendor/lib') + + expect(mux.request).toHaveBeenCalledWith('git.submoduleStatus', { + worktreePath: '/home/user/repo', + submodulePath: 'vendor/lib' + }) + expect(result).toEqual(statusResult) + }) + + it('reports an actionable reconnect message when the relay lacks submodule status', async () => { + const methodNotFound = new Error('Method not found: git.submoduleStatus') as Error & { + code?: number + } + methodNotFound.code = -32601 + mux.request.mockRejectedValueOnce(methodNotFound) + + await expect(provider.getSubmoduleStatus('/home/user/repo', 'vendor/lib')).rejects.toThrow( + 'SSH submodule diff support is unavailable on this relay. Reconnect the SSH target to update Orca on the host, then try again.' + ) + }) + + it('rethrows non-method-not-found submodule status errors unchanged', async () => { + mux.request.mockRejectedValueOnce(new Error('fatal: not a submodule')) + + await expect(provider.getSubmoduleStatus('/home/user/repo', 'vendor/lib')).rejects.toThrow( + 'fatal: not a submodule' + ) + }) + it('checkIgnoredPaths sends git.checkIgnored request', async () => { mux.request.mockResolvedValue(['dist/bundle.js']) diff --git a/src/main/providers/ssh-git-provider.ts b/src/main/providers/ssh-git-provider.ts index 2194cb16ef..091fa5c9d1 100644 --- a/src/main/providers/ssh-git-provider.ts +++ b/src/main/providers/ssh-git-provider.ts @@ -96,10 +96,22 @@ export class SshGitProvider implements IGitProvider { } async getSubmoduleStatus(worktreePath: string, submodulePath: string): Promise { - return (await this.mux.request('git.submoduleStatus', { - worktreePath, - submodulePath - })) as GitStatusResult + try { + return (await this.mux.request('git.submoduleStatus', { + worktreePath, + submodulePath + })) as GitStatusResult + } catch (error) { + // Why: a newer desktop client may talk to an older relay that predates + // git.submoduleStatus; surface an actionable reconnect hint instead of a + // raw JSON-RPC method-not-found error in Source Control. + if (isJsonRpcMethodNotFoundError(error)) { + throw new Error( + 'SSH submodule diff support is unavailable on this relay. Reconnect the SSH target to update Orca on the host, then try again.' + ) + } + throw error + } } async checkIgnoredPaths(worktreePath: string, relativePaths: string[]): Promise { diff --git a/src/renderer/src/components/right-sidebar/SourceControl.compare-summary.test.ts b/src/renderer/src/components/right-sidebar/SourceControl.compare-summary.test.ts index a513471a49..f468f24f15 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.compare-summary.test.ts +++ b/src/renderer/src/components/right-sidebar/SourceControl.compare-summary.test.ts @@ -7,10 +7,11 @@ import { resolveSourceControlBaseRef, resolveSourceControlCompareBaseRef, resolveSourceControlPickerBaseRef, + shouldClearBranchCompareForMissingBase, shouldRefreshBranchCompareForStatusHead, shouldShowCompareSummary } from './SourceControl' -import type { GitBranchCompareSummary } from '../../../../shared/types' +import type { GitBranchCompareSummary, GitUpstreamStatus } from '../../../../shared/types' type ReactElementLike = { type: unknown @@ -275,6 +276,58 @@ describe('SourceControl compare summary', () => { ).toBeNull() }) + it('keeps the branch compare while upstream status is still loading', () => { + // remoteStatus undefined means upstream status has not loaded yet; the + // prefer-upstream setting makes compareBaseRef momentarily null in this gap. + expect( + shouldClearBranchCompareForMissingBase({ + isFolder: false, + compareBaseRef: null, + remoteStatus: undefined + }) + ).toBe(false) + }) + + it('clears the branch compare once upstream loads with no upstream and no base', () => { + const loadedNoUpstream: GitUpstreamStatus = { + hasUpstream: false, + ahead: 0, + behind: 0 + } + expect( + shouldClearBranchCompareForMissingBase({ + isFolder: false, + compareBaseRef: null, + remoteStatus: loadedNoUpstream + }) + ).toBe(true) + }) + + it('keeps the branch compare when a compare base is resolved', () => { + expect( + shouldClearBranchCompareForMissingBase({ + isFolder: false, + compareBaseRef: 'origin/main', + remoteStatus: undefined + }) + ).toBe(false) + }) + + it('never clears the branch compare in folder mode', () => { + const loadedNoUpstream: GitUpstreamStatus = { + hasUpstream: false, + ahead: 0, + behind: 0 + } + expect( + shouldClearBranchCompareForMissingBase({ + isFolder: true, + compareBaseRef: null, + remoteStatus: loadedNoUpstream + }) + ).toBe(false) + }) + it('wires toolbar actions without rendering the dead view-mode toggle', () => { const onChangeBaseRef = vi.fn() const onRetry = vi.fn() diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index 8f96e8c5bf..e7cb856d8a 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -378,6 +378,21 @@ export function resolveSourceControlCompareBaseRef(input: { return input.upstreamName?.trim() || null } +// Why: only drop a stale branch-compare summary once we know there is truly no +// compare base. While upstream status is still loading (remoteStatus undefined) +// compareBaseRef can momentarily resolve to null, so clearing then would make +// the committed-changes summary flicker until upstream loads. +export function shouldClearBranchCompareForMissingBase(input: { + isFolder: boolean + compareBaseRef: string | null + remoteStatus: GitUpstreamStatus | undefined +}): boolean { + if (input.isFolder || input.compareBaseRef) { + return false + } + return input.remoteStatus !== undefined +} + export function resolveSourceControlPickerBaseRef(input: { pinnedBaseRef?: string | null effectiveBaseRef?: string | null @@ -4742,12 +4757,15 @@ function SourceControlInner(): React.JSX.Element { // Why: when there is no compare base (prefer-upstream setting on, branch has // no upstream), runBranchCompare bails out; drop any stale summary so the // committed-changes section and "vs" row disappear and only the working tree - // shows. - if (!activeWorktreeId || isFolder || compareBaseRef) { + // shows. Wait until upstream status has loaded so the summary doesn't flicker. + if ( + !activeWorktreeId || + !shouldClearBranchCompareForMissingBase({ isFolder, compareBaseRef, remoteStatus }) + ) { return } clearGitBranchCompare(activeWorktreeId) - }, [activeWorktreeId, clearGitBranchCompare, compareBaseRef, isFolder]) + }, [activeWorktreeId, clearGitBranchCompare, compareBaseRef, isFolder, remoteStatus]) useEffect(() => { // Why: history shells out to git. Defer the first load until the user diff --git a/src/renderer/src/components/settings/CompareAgainstUpstreamSetting.tsx b/src/renderer/src/components/settings/CompareAgainstUpstreamSetting.tsx index 9792390503..b0ece6eea8 100644 --- a/src/renderer/src/components/settings/CompareAgainstUpstreamSetting.tsx +++ b/src/renderer/src/components/settings/CompareAgainstUpstreamSetting.tsx @@ -59,6 +59,7 @@ export function CompareAgainstUpstreamSetting({

{description}

+ /> ) } diff --git a/src/renderer/src/components/settings/GitPane.test.ts b/src/renderer/src/components/settings/GitPane.test.ts index d7852c9f81..3275eb9a18 100644 --- a/src/renderer/src/components/settings/GitPane.test.ts +++ b/src/renderer/src/components/settings/GitPane.test.ts @@ -57,15 +57,19 @@ function findSegmentedControl(node: unknown): ReactElementLike { return found } -function findCompareAgainstUpstreamSwitch(node: unknown): ReactElementLike { +function findCompareBaseSegmentedControl(node: unknown): ReactElementLike { let found: ReactElementLike | null = null + const label = translate( + 'auto.components.settings.GitPane.compareAgainstUpstreamTitle', + 'Default Compare Base' + ) visit(node, (entry) => { - if (entry.type === 'button' && entry.props.role === 'switch') { + if (entry.type === SettingsSegmentedControl && entry.props.ariaLabel === label) { found = entry } }) if (!found) { - throw new Error('compare-against-upstream switch not found') + throw new Error('compare-base segmented control not found') } return found } @@ -178,26 +182,40 @@ describe('GitPane', () => { expect(matchesSettingsSearch('group order', getGitPaneSearchEntries())).toBe(true) }) - it('renders the compare-against-current-branch setting in Git settings', () => { + it('renders the default compare base setting in Git settings', () => { const markup = renderGitPane('compare base') expect(markup).toContain( translate( 'auto.components.settings.GitPane.compareAgainstUpstreamTitle', - 'Compare Against Current Branch' + 'Default Compare Base' ) ) + expect(markup).toContain( + translate( + 'auto.components.settings.GitPane.compareBaseRepositoryDefault', + 'Repository default' + ) + ) + expect(markup).toContain( + translate('auto.components.settings.GitPane.compareBaseBranchUpstream', 'Branch upstream') + ) + expect(markup).toContain('worktree's Git panel') }) - it('includes compare-against-current-branch search metadata', () => { + it('includes default compare base search metadata', () => { expect(matchesSettingsSearch('compare base', getGitPaneSearchEntries())).toBe(true) + expect(matchesSettingsSearch('worktree', getGitPaneSearchEntries())).toBe(true) + expect(matchesSettingsSearch('Git panel', getGitPaneSearchEntries())).toBe(true) + expect(matchesSettingsSearch('default branch', getGitPaneSearchEntries())).toBe(true) + expect(matchesSettingsSearch('branch upstream', getGitPaneSearchEntries())).toBe(true) expect(matchesSettingsSearch('upstream', getGitPaneSearchEntries())).toBe(true) expect(matchesSettingsSearch('diff base', getGitPaneSearchEntries())).toBe(true) expect(matchesSettingsSearch('source control', getGitPaneSearchEntries())).toBe(true) }) - it('reflects the compare-against-current-branch setting state in its own switch', () => { - const offSwitch = findCompareAgainstUpstreamSwitch( + it('reflects the default compare base policy in its own segmented control', () => { + const repositoryDefaultControl = findCompareBaseSegmentedControl( CompareAgainstUpstreamSetting({ settings: { ...getDefaultSettings(os.homedir()), @@ -206,9 +224,9 @@ describe('GitPane', () => { updateSettings: () => {} }) ) - expect(offSwitch.props['aria-checked']).toBe(false) + expect(repositoryDefaultControl.props.value).toBe('repository-default') - const onSwitch = findCompareAgainstUpstreamSwitch( + const branchUpstreamControl = findCompareBaseSegmentedControl( CompareAgainstUpstreamSetting({ settings: { ...getDefaultSettings(os.homedir()), @@ -217,14 +235,12 @@ describe('GitPane', () => { updateSettings: () => {} }) ) - expect(onSwitch.props['aria-checked']).toBe(true) - // Why: the switch must not default to type="submit" inside a form. - expect(onSwitch.props.type).toBe('button') + expect(branchUpstreamControl.props.value).toBe('branch-upstream') }) - it('toggles the compare-against-current-branch setting to its negated value on click', () => { + it('updates the default compare base policy from its segmented control', () => { const updateSettings = vi.fn() - const offSwitch = findCompareAgainstUpstreamSwitch( + const repositoryDefaultControl = findCompareBaseSegmentedControl( CompareAgainstUpstreamSetting({ settings: { ...getDefaultSettings(os.homedir()), @@ -233,11 +249,11 @@ describe('GitPane', () => { updateSettings }) ) - ;(offSwitch.props.onClick as () => void)() + ;(repositoryDefaultControl.props.onChange as (value: string) => void)('branch-upstream') expect(updateSettings).toHaveBeenCalledWith({ sourceControlCompareAgainstUpstream: true }) updateSettings.mockClear() - const onSwitch = findCompareAgainstUpstreamSwitch( + const branchUpstreamControl = findCompareBaseSegmentedControl( CompareAgainstUpstreamSetting({ settings: { ...getDefaultSettings(os.homedir()), @@ -246,7 +262,7 @@ describe('GitPane', () => { updateSettings }) ) - ;(onSwitch.props.onClick as () => void)() + ;(branchUpstreamControl.props.onChange as (value: string) => void)('repository-default') expect(updateSettings).toHaveBeenCalledWith({ sourceControlCompareAgainstUpstream: false }) }) }) diff --git a/src/renderer/src/components/settings/git-search.ts b/src/renderer/src/components/settings/git-search.ts index 9fc367208a..70b67a7086 100644 --- a/src/renderer/src/components/settings/git-search.ts +++ b/src/renderer/src/components/settings/git-search.ts @@ -73,14 +73,30 @@ export const getGitPaneSearchEntries = createLocalizedCatalog(() => [ { title: translate( 'auto.components.settings.git.search.compareAgainstUpstreamTitle', - 'Compare Against Current Branch' + 'Default Compare Base' ), description: translate( 'auto.components.settings.git.search.compareAgainstUpstreamDescription', - "Default the Source Control compare base to the current branch's upstream so it prioritizes local changes, instead of the repository default branch. Only affects the compare view, not the Pull Request or rebase target." + "Choose which base Source Control uses by default for committed-change comparisons. Branch upstream follows the current branch automatically and falls back to the repository default branch when no upstream exists. You can still change the compare base per worktree from that worktree's Git panel. Pull Request and rebase targets don't change." ), keywords: [ ...translateSearchKeyword('auto.components.settings.git.search.compareBase', 'compare base'), + ...translateSearchKeyword( + 'auto.components.settings.git.search.defaultCompareBase', + 'default compare base' + ), + ...translateSearchKeyword( + 'auto.components.settings.git.search.defaultBranch', + 'default branch' + ), + ...translateSearchKeyword( + 'auto.components.settings.git.search.repositoryDefault', + 'repository default' + ), + ...translateSearchKeyword( + 'auto.components.settings.git.search.branchUpstream', + 'branch upstream' + ), ...translateSearchKeyword( 'auto.components.settings.git.search.currentBranch', 'current branch' diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index e1bb56646e..122db3be13 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -5147,8 +5147,10 @@ "changesFirst": "Changes first", "stagedFirst": "Staged first", "untrackedFirst": "Untracked first", - "compareAgainstUpstreamTitle": "Compare Against Current Branch", - "compareAgainstUpstreamDescription": "Default the Source Control compare base to the current branch's upstream so it prioritizes local changes, instead of the repository default branch. Only affects the compare view, not the Pull Request or rebase target." + "compareAgainstUpstreamTitle": "Default Compare Base", + "compareAgainstUpstreamDescription": "Choose which base Source Control uses by default for committed-change comparisons. Branch upstream follows the current branch automatically and falls back to the repository default branch when no upstream exists. You can still change the compare base per worktree from that worktree's Git panel. Pull Request and rebase targets don't change.", + "compareBaseRepositoryDefault": "Repository default", + "compareBaseBranchUpstream": "Branch upstream" }, "HiddenExperimentalGroup": { "d0f914a528": "Placeholder toggle", @@ -7270,9 +7272,13 @@ "untrackedFirst": "untracked first", "sourceControl": "source control", "gitChanges": "git changes", - "compareAgainstUpstreamTitle": "Compare Against Current Branch", - "compareAgainstUpstreamDescription": "Default the Source Control compare base to the current branch's upstream so it prioritizes local changes, instead of the repository default branch. Only affects the compare view, not the Pull Request or rebase target.", + "compareAgainstUpstreamTitle": "Default Compare Base", + "compareAgainstUpstreamDescription": "Choose which base Source Control uses by default for committed-change comparisons. Branch upstream follows the current branch automatically and falls back to the repository default branch when no upstream exists. You can still change the compare base per worktree from that worktree's Git panel. Pull Request and rebase targets don't change.", "compareBase": "compare base", + "defaultCompareBase": "default compare base", + "defaultBranch": "default branch", + "repositoryDefault": "repository default", + "branchUpstream": "branch upstream", "currentBranch": "current branch", "upstream": "upstream", "localChanges": "local changes", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 928e759916..bfeeb98826 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -5110,8 +5110,10 @@ "changesFirst": "Cambios primero", "stagedFirst": "Cambios preparados primero", "untrackedFirst": "Archivos sin seguimiento primero", - "compareAgainstUpstreamTitle": "Comparar con la rama actual", - "compareAgainstUpstreamDescription": "Establece la base de comparación de Control de código fuente en el upstream de la rama actual para priorizar los cambios locales, en lugar de la rama predeterminada del repositorio. Solo afecta a la vista de comparación, no al objetivo de Pull Request o rebase." + "compareAgainstUpstreamTitle": "Base de comparación predeterminada", + "compareAgainstUpstreamDescription": "Elige qué base usa Control de código fuente de forma predeterminada para las comparaciones de cambios confirmados. El upstream de la rama sigue automáticamente la rama actual y recurre a la rama predeterminada del repositorio cuando no hay upstream. Aun así, puedes cambiar la base de comparación por worktree desde el panel Git de ese worktree. Los objetivos de Pull Request y rebase no cambian.", + "compareBaseRepositoryDefault": "Predeterminada del repositorio", + "compareBaseBranchUpstream": "Upstream de la rama" }, "HiddenExperimentalGroup": { "d0f914a528": "Alternar marcador de posición", @@ -7233,14 +7235,18 @@ "untrackedFirst": "sin seguimiento primero", "sourceControl": "control de código fuente", "gitChanges": "cambios de git", - "compareAgainstUpstreamTitle": "Comparar con la rama actual", - "compareAgainstUpstreamDescription": "Establece la base de comparación de Control de código fuente en el upstream de la rama actual para priorizar los cambios locales, en lugar de la rama predeterminada del repositorio. Solo afecta a la vista de comparación, no al objetivo de Pull Request o rebase.", + "compareAgainstUpstreamTitle": "Base de comparación predeterminada", + "compareAgainstUpstreamDescription": "Elige qué base usa Control de código fuente de forma predeterminada para las comparaciones de cambios confirmados. El upstream de la rama sigue automáticamente la rama actual y recurre a la rama predeterminada del repositorio cuando no hay upstream. Aun así, puedes cambiar la base de comparación por worktree desde el panel Git de ese worktree. Los objetivos de Pull Request y rebase no cambian.", "compareBase": "base de comparación", "currentBranch": "rama actual", "upstream": "upstream", "localChanges": "cambios locales", "originMaster": "origin/master", - "committedChanges": "cambios confirmados" + "committedChanges": "cambios confirmados", + "defaultCompareBase": "base de comparación predeterminada", + "defaultBranch": "rama predeterminada", + "repositoryDefault": "predeterminada del repositorio", + "branchUpstream": "upstream de la rama" } }, "input": { diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 2d9a30f3ee..8d5ccc7541 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -5132,8 +5132,10 @@ "changesFirst": "変更を先頭", "stagedFirst": "ステージ済みを先頭", "untrackedFirst": "未追跡を先頭", - "compareAgainstUpstreamTitle": "現在のブランチと比較", - "compareAgainstUpstreamDescription": "ソース管理の比較基準を既定で現在のブランチの上流に設定し、リポジトリの既定ブランチではなくローカルの変更を優先表示します。比較ビューのみに影響し、Pull Request やリベース先には影響しません。" + "compareAgainstUpstreamTitle": "既定の比較ベース", + "compareAgainstUpstreamDescription": "ソース管理がコミット済み変更の比較に既定で使うベースを選択します。ブランチの上流は現在のブランチに自動的に追従し、上流がない場合はリポジトリの既定ブランチにフォールバックします。比較ベースは各 worktree の Git パネルから worktree ごとに変更できます。Pull Request やリベースの対象は変更されません。", + "compareBaseRepositoryDefault": "リポジトリ既定", + "compareBaseBranchUpstream": "ブランチの上流" }, "HiddenExperimentalGroup": { "d0f914a528": "プレースホルダーの切り替え", @@ -7255,14 +7257,18 @@ "untrackedFirst": "未追跡を先頭", "sourceControl": "ソース管理", "gitChanges": "git の変更", - "compareAgainstUpstreamTitle": "現在のブランチと比較", - "compareAgainstUpstreamDescription": "ソース管理の比較基準を既定で現在のブランチの上流に設定し、リポジトリの既定ブランチではなくローカルの変更を優先表示します。比較ビューのみに影響し、Pull Request やリベース先には影響しません。", + "compareAgainstUpstreamTitle": "既定の比較ベース", + "compareAgainstUpstreamDescription": "ソース管理がコミット済み変更の比較に既定で使うベースを選択します。ブランチの上流は現在のブランチに自動的に追従し、上流がない場合はリポジトリの既定ブランチにフォールバックします。比較ベースは各 worktree の Git パネルから worktree ごとに変更できます。Pull Request やリベースの対象は変更されません。", "compareBase": "比較基準", "currentBranch": "現在のブランチ", "upstream": "上流", "localChanges": "ローカルの変更", "originMaster": "origin/master", - "committedChanges": "コミット済みの変更" + "committedChanges": "コミット済みの変更", + "defaultCompareBase": "既定の比較ベース", + "defaultBranch": "既定ブランチ", + "repositoryDefault": "リポジトリ既定", + "branchUpstream": "ブランチ上流" } }, "input": { diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 671f9a46ca..63d086259a 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -5095,8 +5095,10 @@ "changesFirst": "변경 사항 먼저", "stagedFirst": "스테이징된 항목 먼저", "untrackedFirst": "추적되지 않는 파일 먼저", - "compareAgainstUpstreamTitle": "현재 브랜치와 비교", - "compareAgainstUpstreamDescription": "소스 제어 비교 기준을 기본적으로 현재 브랜치의 업스트림으로 설정하여 저장소 기본 브랜치 대신 로컬 변경 사항을 우선 표시합니다. 비교 보기에만 영향을 주며 Pull Request 또는 리베이스 대상에는 영향을 주지 않습니다." + "compareAgainstUpstreamTitle": "기본 비교 기준", + "compareAgainstUpstreamDescription": "소스 제어가 커밋된 변경 사항 비교에 기본으로 사용할 기준을 선택합니다. 브랜치 업스트림은 현재 브랜치를 자동으로 따라가며, 업스트림이 없으면 저장소 기본 브랜치로 돌아갑니다. 비교 기준은 해당 worktree의 Git 패널에서 worktree별로 변경할 수 있습니다. Pull Request 및 리베이스 대상은 변경되지 않습니다.", + "compareBaseRepositoryDefault": "저장소 기본값", + "compareBaseBranchUpstream": "브랜치 업스트림" }, "HiddenExperimentalGroup": { "d0f914a528": "자리 표시자 토글", @@ -7218,14 +7220,18 @@ "untrackedFirst": "추적되지 않는 파일 먼저", "sourceControl": "소스 제어", "gitChanges": "git 변경 사항", - "compareAgainstUpstreamTitle": "현재 브랜치와 비교", - "compareAgainstUpstreamDescription": "소스 제어 비교 기준을 기본적으로 현재 브랜치의 업스트림으로 설정하여 저장소 기본 브랜치 대신 로컬 변경 사항을 우선 표시합니다. 비교 보기에만 영향을 주며 Pull Request 또는 리베이스 대상에는 영향을 주지 않습니다.", + "compareAgainstUpstreamTitle": "기본 비교 기준", + "compareAgainstUpstreamDescription": "소스 제어가 커밋된 변경 사항 비교에 기본으로 사용할 기준을 선택합니다. 브랜치 업스트림은 현재 브랜치를 자동으로 따라가며, 업스트림이 없으면 저장소 기본 브랜치로 돌아갑니다. 비교 기준은 해당 worktree의 Git 패널에서 worktree별로 변경할 수 있습니다. Pull Request 및 리베이스 대상은 변경되지 않습니다.", "compareBase": "비교 기준", "currentBranch": "현재 브랜치", "upstream": "업스트림", "localChanges": "로컬 변경 사항", "originMaster": "origin/master", - "committedChanges": "커밋된 변경 사항" + "committedChanges": "커밋된 변경 사항", + "defaultCompareBase": "기본 비교 기준", + "defaultBranch": "기본 브랜치", + "repositoryDefault": "저장소 기본값", + "branchUpstream": "브랜치 업스트림" } }, "input": { diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 1e8ab24547..1fb70c4e7a 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -5095,8 +5095,10 @@ "changesFirst": "更改优先", "stagedFirst": "已暂存优先", "untrackedFirst": "未跟踪优先", - "compareAgainstUpstreamTitle": "对比当前分支", - "compareAgainstUpstreamDescription": "将源代码管理的对比基准默认设为当前分支的上游，从而优先显示本地变更，而不是仓库的默认分支。仅影响对比视图，不影响 Pull Request 或变基目标。" + "compareAgainstUpstreamTitle": "默认对比基准", + "compareAgainstUpstreamDescription": "选择源代码管理在比较已提交变更时默认使用的基准。分支上游会自动跟随当前分支；如果没有上游，则回退到仓库默认分支。你仍然可以在对应 worktree 的 Git 面板中按 worktree 更改对比基准。Pull Request 和变基目标不会改变。", + "compareBaseRepositoryDefault": "仓库默认", + "compareBaseBranchUpstream": "分支上游" }, "HiddenExperimentalGroup": { "d0f914a528": "占位符切换", @@ -7218,14 +7220,18 @@ "untrackedFirst": "未跟踪优先", "sourceControl": "源代码管理", "gitChanges": "git 更改", - "compareAgainstUpstreamTitle": "对比当前分支", - "compareAgainstUpstreamDescription": "将源代码管理的对比基准默认设为当前分支的上游，从而优先显示本地变更，而不是仓库的默认分支。仅影响对比视图，不影响 Pull Request 或变基目标。", + "compareAgainstUpstreamTitle": "默认对比基准", + "compareAgainstUpstreamDescription": "选择源代码管理在比较已提交变更时默认使用的基准。分支上游会自动跟随当前分支；如果没有上游，则回退到仓库默认分支。你仍然可以在对应 worktree 的 Git 面板中按 worktree 更改对比基准。Pull Request 和变基目标不会改变。", "compareBase": "对比基准", "currentBranch": "当前分支", "upstream": "上游", "localChanges": "本地变更", "originMaster": "origin/master", - "committedChanges": "已提交的更改" + "committedChanges": "已提交的更改", + "defaultCompareBase": "默认对比基准", + "defaultBranch": "默认分支", + "repositoryDefault": "仓库默认", + "branchUpstream": "分支上游" } }, "input": { diff --git a/src/renderer/src/runtime/runtime-git-client.test.ts b/src/renderer/src/runtime/runtime-git-client.test.ts index d4f1f57abc..909ca60c22 100644 --- a/src/renderer/src/runtime/runtime-git-client.test.ts +++ b/src/renderer/src/runtime/runtime-git-client.test.ts @@ -13,6 +13,7 @@ import { getRuntimeGitHistory, getRuntimeGitIgnoredPaths, getRuntimeGitStatus, + getRuntimeGitSubmoduleStatus, pushRuntimeGit, rebaseRuntimeGitFromBase } from './runtime-git-client' @@ -24,6 +25,7 @@ import { clearRuntimeCompatibilityCacheForTests } from './runtime-rpc-client' const gitStatus = vi.fn() const gitCheckIgnored = vi.fn() +const gitSubmoduleStatus = vi.fn() const gitDiff = vi.fn() const gitHistory = vi.fn() const gitBulkStage = vi.fn() @@ -44,6 +46,7 @@ beforeEach(() => { clearRuntimeCompatibilityCacheForTests() gitStatus.mockReset() gitCheckIgnored.mockReset() + gitSubmoduleStatus.mockReset() gitDiff.mockReset() gitHistory.mockReset() gitBulkStage.mockReset() @@ -67,6 +70,7 @@ beforeEach(() => { git: { status: gitStatus, checkIgnored: gitCheckIgnored, + submoduleStatus: gitSubmoduleStatus, diff: gitDiff, history: gitHistory, bulkStage: gitBulkStage, @@ -185,6 +189,29 @@ describe('runtime git client', () => { expect(runtimeEnvironmentCall).not.toHaveBeenCalled() }) + it('passes submodule status area through local git IPC', async () => { + gitSubmoduleStatus.mockResolvedValue({ entries: [], conflictOperation: 'unknown' }) + + await getRuntimeGitSubmoduleStatus( + { + settings: { activeRuntimeEnvironmentId: null }, + worktreeId: 'wt-1', + worktreePath: '/repo', + connectionId: 'ssh-1' + }, + 'vendor/lib', + 'staged' + ) + + expect(gitSubmoduleStatus).toHaveBeenCalledWith({ + worktreePath: '/repo', + submodulePath: 'vendor/lib', + connectionId: 'ssh-1', + area: 'staged' + }) + expect(runtimeEnvironmentCall).not.toHaveBeenCalled() + }) + it('uses local git IPC for history when no remote runtime is active', async () => { gitHistory.mockResolvedValue({ items: [], @@ -268,6 +295,32 @@ describe('runtime git client', () => { }) }) + it('passes submodule status area through the active runtime environment', async () => { + runtimeEnvironmentCall.mockResolvedValue({ + id: 'rpc-1', + ok: true, + result: { entries: [], conflictOperation: 'unknown' }, + _meta: { runtimeId: 'remote-runtime' } + }) + + await getRuntimeGitSubmoduleStatus( + { + settings: { activeRuntimeEnvironmentId: 'env-1' }, + worktreeId: 'wt-1', + worktreePath: '/repo' + }, + 'vendor/lib', + 'staged' + ) + + expect(runtimeEnvironmentCall).toHaveBeenCalledWith({ + selector: 'env-1', + method: 'git.submoduleStatus', + params: { worktree: 'id:wt-1', submodulePath: 'vendor/lib', area: 'staged' }, + timeoutMs: 15_000 + }) + }) + it('forwards includeIgnored through the active runtime environment', async () => { runtimeEnvironmentCall.mockResolvedValue({ id: 'rpc-1', diff --git a/src/renderer/src/runtime/runtime-git-client.ts b/src/renderer/src/runtime/runtime-git-client.ts index b170f4e2a8..a1768bef5c 100644 --- a/src/renderer/src/runtime/runtime-git-client.ts +++ b/src/renderer/src/runtime/runtime-git-client.ts @@ -9,6 +9,7 @@ import type { GitForkSyncExpectedUpstream, GitForkSyncResult, GitPushTarget, + GitStagingArea, GitStatusResult, GitUpstreamStatus, GlobalSettings @@ -150,14 +151,16 @@ export async function getRuntimeGitStatus( export async function getRuntimeGitSubmoduleStatus( context: RuntimeGitContext, - submodulePath: string + submodulePath: string, + area: GitStagingArea = 'unstaged' ): Promise { const target = getActiveRuntimeTarget(context.settings) if (target.kind === 'local' || !context.worktreeId) { return window.api.git.submoduleStatus({ worktreePath: context.worktreePath, submodulePath, - connectionId: context.connectionId + connectionId: context.connectionId, + area }) } return callRuntimeRpc( @@ -165,7 +168,8 @@ export async function getRuntimeGitSubmoduleStatus( 'git.submoduleStatus', { worktree: toRuntimeWorktreeSelector(context.worktreeId), - submodulePath + submodulePath, + area }, { timeoutMs: 15_000 } ) diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index ce1d2a255d..23e6ab3589 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -1393,11 +1393,12 @@ function createGitApi(): NonNullable['git']> { includeIgnored }) }, - submoduleStatus: async ({ worktreePath, submodulePath }) => { + submoduleStatus: async ({ worktreePath, submodulePath, area }) => { const worktree = await resolveRuntimeWorktreeByPath(worktreePath) return callRuntimeResult('git.submoduleStatus', { worktree: toRuntimeWorktreeSelector(worktree.id), - submodulePath + submodulePath, + area }) }, checkIgnored: async ({ worktreePath, paths }) => { From 747863d2df8d829a62841d83db11d30ddec8d0f5 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sun, 28 Jun 2026 19:13:38 -0700 Subject: [PATCH 7/9] Fix submodule staging behavior, WSL caching, and double-click toggles - Namespace submodule path cache per WSL distro to prevent cross-distro collisions. - Preserve the staged area of child entries when expanding unstaged submodules so staged inner changes do not open empty diffs. - Prefix oldPath with the submodule path for renamed inner entries. - Ignore click events where detail > 1 to prevent double-clicks from instantly collapsing newly expanded submodules. --- src/main/git/status.test.ts | 28 ++++++++ src/main/git/status.ts | 11 +++- .../right-sidebar/SourceControl.tsx | 5 ++ ...source-control-submodule-expansion.test.ts | 64 ++++++++++++++++++- .../source-control-submodule-expansion.ts | 13 ++-- 5 files changed, 112 insertions(+), 9 deletions(-) diff --git a/src/main/git/status.test.ts b/src/main/git/status.test.ts index b17a535ebe..858d4e51a6 100644 --- a/src/main/git/status.test.ts +++ b/src/main/git/status.test.ts @@ -81,6 +81,7 @@ import { getStatus, getSubmoduleStatus, isWithinWorktree, + listSubmodulePaths, resolveSubmoduleWorktreePath, stageFile } from './status' @@ -898,6 +899,33 @@ describe('resolveSubmoduleWorktreePath', () => { }) }) +describe('listSubmodulePaths', () => { + beforeEach(() => { + clearSubmodulePathsCacheForTests() + gitExecFileAsyncMock.mockReset() + }) + + it('keeps cached .gitmodules paths separate per WSL distro', async () => { + gitExecFileAsyncMock.mockImplementation((_args: string[], options?: { wslDistro?: string }) => + Promise.resolve({ + stdout: + options?.wslDistro === 'debian' + ? 'submodule.lib.path debian-lib\n' + : 'submodule.lib.path ubuntu-lib\n' + }) + ) + + await expect(listSubmodulePaths('/repo', { wslDistro: 'ubuntu' })).resolves.toEqual([ + 'ubuntu-lib' + ]) + await expect(listSubmodulePaths('/repo', { wslDistro: 'debian' })).resolves.toEqual([ + 'debian-lib' + ]) + + expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(2) + }) +}) + describe('getSubmoduleStatus', () => { beforeEach(() => { clearEffectiveUpstreamStatusCacheForTests() diff --git a/src/main/git/status.ts b/src/main/git/status.ts index 3705a0db27..cf002f3031 100644 --- a/src/main/git/status.ts +++ b/src/main/git/status.ts @@ -89,6 +89,12 @@ function gitRuntimeOptionsKey(options: GitRuntimeOptions): readonly unknown[] { return [options.wslDistro ?? null] } +function getSubmodulePathsCacheKey(worktreePath: string, options: GitRuntimeOptions): string { + // Why: the same path string can refer to different filesystem views across + // WSL distros, so the `.gitmodules` cache must follow runtime routing. + return [worktreePath, ...gitRuntimeOptionsKey(options)].join('\0') +} + // Why: status tests reuse this reset hook, so every cross-call memoization layer // must reset together even though the historical name mentions upstream only. export function clearEffectiveUpstreamStatusCacheForTests(): void { @@ -866,7 +872,8 @@ export async function listSubmodulePaths( options: GitRuntimeOptions = {} ): Promise { const now = Date.now() - const cached = submodulePathsCache.get(worktreePath) + const cacheKey = getSubmodulePathsCacheKey(worktreePath, options) + const cached = submodulePathsCache.get(cacheKey) if (cached && cached.expiresAt > now) { return cached.paths } @@ -892,7 +899,7 @@ export async function listSubmodulePaths( // No .gitmodules (or git config failure) — treat as a repo without submodules. paths = [] } - submodulePathsCache.set(worktreePath, { paths, expiresAt: now + SUBMODULE_PATHS_CACHE_TTL_MS }) + submodulePathsCache.set(cacheKey, { paths, expiresAt: now + SUBMODULE_PATHS_CACHE_TTL_MS }) return paths } diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index a3f93b3de0..00fe96b51c 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -7949,6 +7949,11 @@ const UncommittedEntryRow = React.memo(function UncommittedEntryRow({ }} onClick={(e) => { if (submoduleExpansion) { + // Why: a double-click emits two click events; without this guard it + // expands and immediately collapses the submodule row. + if (e.detail > 1) { + return + } submoduleExpansion.onToggle() return } diff --git a/src/renderer/src/components/right-sidebar/source-control-submodule-expansion.test.ts b/src/renderer/src/components/right-sidebar/source-control-submodule-expansion.test.ts index b7d47d7f1c..f6de3f25a3 100644 --- a/src/renderer/src/components/right-sidebar/source-control-submodule-expansion.test.ts +++ b/src/renderer/src/components/right-sidebar/source-control-submodule-expansion.test.ts @@ -94,15 +94,21 @@ describe('buildSubmoduleChildNodes', () => { it('prefixes inner paths, stamps submoduleRoot, and nests one level deeper', () => { const parent = fileNode(submoduleEntry({ path: 'flutter_mine' }), 2) const inner: GitStatusEntry[] = [ - { path: 'lib/main.dart', status: 'modified', area: 'unstaged' } + { + path: 'lib/main.dart', + oldPath: 'lib/old-main.dart', + status: 'renamed', + area: 'unstaged' + } ] const [child] = buildSubmoduleChildNodes(parent, inner) expect(child.path).toBe('flutter_mine/lib/main.dart') + expect(child.entry.oldPath).toBe('flutter_mine/lib/old-main.dart') expect(child.name).toBe('main.dart') expect(child.entry.submoduleRoot).toBe('flutter_mine') - expect(child.entry.status).toBe('modified') + expect(child.entry.status).toBe('renamed') expect(child.depth).toBe(3) expect(child.area).toBe('unstaged') }) @@ -189,6 +195,34 @@ describe('injectExpandedSubmoduleRows', () => { expect(child.entry.submoduleRoot).toBe('flutter_mine') }) + it('keeps inner staged rows staged for tree-view diff routing', () => { + const node = fileNode(submoduleEntry({ path: 'flutter_mine' })) + const statuses: Record = { + [FLUTTER_KEY]: { + status: 'loaded', + entries: [{ path: 'lib/main.dart', status: 'modified', area: 'staged' }] + } + } + const result = injectExpandedSubmoduleRows( + [node], + new Set([FLUTTER_KEY]), + statuses, + LOADING, + EMPTY + ) + + expect(result[1]).toMatchObject({ + type: 'file', + key: 'staged::flutter_mine/lib/main.dart', + area: 'staged', + entry: { + path: 'flutter_mine/lib/main.dart', + area: 'staged', + submoduleRoot: 'flutter_mine' + } + }) + }) + it('expands a pointer-only (commit) submodule into its commit-range files', () => { const node = fileNode( submoduleEntry({ @@ -277,6 +311,32 @@ describe('injectExpandedSubmoduleEntries (list view)', () => { }) }) + it('keeps staged-only inner entries staged in list view', () => { + const entry = submoduleEntry({ path: 'flutter_mine' }) + const statuses: Record = { + [FLUTTER_KEY]: { + status: 'loaded', + entries: [{ path: 'lib/main.dart', status: 'modified', area: 'staged' }] + } + } + const result = injectExpandedSubmoduleEntries( + [entry], + new Set([FLUTTER_KEY]), + statuses, + LOADING, + EMPTY + ) + + expect(result[1]).toMatchObject({ + type: 'entry', + entry: { + path: 'flutter_mine/lib/main.dart', + area: 'staged', + submoduleRoot: 'flutter_mine' + } + }) + }) + it('emits an empty placeholder when loaded with no inner entries', () => { const entry = submoduleEntry({ path: 'flutter_mine' }) const statuses: Record = { diff --git a/src/renderer/src/components/right-sidebar/source-control-submodule-expansion.ts b/src/renderer/src/components/right-sidebar/source-control-submodule-expansion.ts index e1002eb0a8..0cd0ca92ca 100644 --- a/src/renderer/src/components/right-sidebar/source-control-submodule-expansion.ts +++ b/src/renderer/src/components/right-sidebar/source-control-submodule-expansion.ts @@ -67,18 +67,21 @@ export function isExpandableSubmoduleEntry(entry: GitStatusEntry): boolean { * Build the read-only inner entry for a submodule child row. The inner path is * relative to the submodule root, so it is prefixed with the submodule path * (drives diff routing) and stamped with `submoduleRoot` (drives read-only - * gating). The parent area is preserved so staged submodule children open - * staged diffs against the parent index instead of unstaged worktree diffs. + * gating). Staged parent gitlink rows force staged children because their + * expansion represents HEAD->index, but worktree-only rows keep the inner + * entry's own area so staged-only submodule changes don't open empty diffs. */ export function buildSubmoduleChildEntry( submodulePath: string, innerEntry: GitStatusEntry, parentArea: GitStatusEntry['area'] = innerEntry.area ): GitStatusEntry { + const area = parentArea === 'staged' ? 'staged' : innerEntry.area return { ...innerEntry, path: `${submodulePath}/${innerEntry.path}`, - area: parentArea, + ...(innerEntry.oldPath ? { oldPath: `${submodulePath}/${innerEntry.oldPath}` } : {}), + area, submoduleRoot: submodulePath } } @@ -95,11 +98,11 @@ export function buildSubmoduleChildNodes( const childEntry = buildSubmoduleChildEntry(submodulePath, innerEntry, parent.entry.area) return { type: 'file', - key: `${parent.area}::${childEntry.path}`, + key: `${childEntry.area}::${childEntry.path}`, name: basename(childEntry.path), path: childEntry.path, entry: childEntry, - area: parent.area, + area: childEntry.area, depth: parent.depth + 1 } }) From 8396bf315eed53796a5246e21124029ef546f2e7 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sun, 28 Jun 2026 21:20:36 -0700 Subject: [PATCH 8/9] Secure submodule path resolution and prevent stale status updates * Extract and centralize submodule path validation into a new `resolveSubmoduleWorktreePath` helper to prevent path traversal exploits when resolving paths from untrusted `.gitmodules` files. * Invalidate submodule expansion state and increment the query generation whenever the active runtime environment or connection route changes, preventing out-of-order responses from writing stale data. --- src/relay/git-handler-submodule-ops.test.ts | 21 ++++++- src/relay/git-handler-submodule-ops.ts | 18 +++++- src/relay/git-handler.ts | 12 ++-- .../useSourceControlSubmoduleStatus.test.tsx | 63 ++++++++++++++++++- .../useSourceControlSubmoduleStatus.ts | 9 +-- 5 files changed, 108 insertions(+), 15 deletions(-) diff --git a/src/relay/git-handler-submodule-ops.test.ts b/src/relay/git-handler-submodule-ops.test.ts index e03bd29890..be60aa34d5 100644 --- a/src/relay/git-handler-submodule-ops.test.ts +++ b/src/relay/git-handler-submodule-ops.test.ts @@ -1,9 +1,11 @@ import { describe, expect, it } from 'vitest' +import * as path from 'path' import type { GitExec } from './git-handler-ops' import { SUBMODULE_PATHS_CACHE_TTL_MS, createSubmodulePathsCache, - listSubmodulePathsCached + listSubmodulePathsCached, + resolveSubmoduleWorktreePath } from './git-handler-submodule-ops' function gitmodulesExec(paths: string[]): { git: GitExec; calls: () => number } { @@ -70,3 +72,20 @@ describe('listSubmodulePathsCached', () => { expect(calls).toBe(1) }) }) + +describe('resolveSubmoduleWorktreePath', () => { + it('resolves relative submodule paths inside the selected worktree', () => { + expect(resolveSubmoduleWorktreePath('/repo', 'vendor/lib')).toBe( + path.resolve('/repo', 'vendor/lib') + ) + }) + + it('rejects empty, absolute, null-byte, and escaping paths', () => { + expect(() => resolveSubmoduleWorktreePath('/repo', '')).toThrow('Access denied') + expect(() => resolveSubmoduleWorktreePath('/repo', path.resolve('/tmp/outside'))).toThrow( + 'Access denied' + ) + expect(() => resolveSubmoduleWorktreePath('/repo', 'vendor\0lib')).toThrow('Access denied') + expect(() => resolveSubmoduleWorktreePath('/repo', '../outside')).toThrow('Access denied') + }) +}) diff --git a/src/relay/git-handler-submodule-ops.ts b/src/relay/git-handler-submodule-ops.ts index 260711c381..088e59e9fe 100644 --- a/src/relay/git-handler-submodule-ops.ts +++ b/src/relay/git-handler-submodule-ops.ts @@ -87,6 +87,20 @@ export function findContainingSubmodule(submodulePaths: string[], filePath: stri return best } +// Why: .gitmodules is repo-controlled; validate its paths before relay reads +// or diffs inside a submodule worktree. +export function resolveSubmoduleWorktreePath(worktreePath: string, submodulePath: string): string { + if (!submodulePath || submodulePath.includes('\0') || path.isAbsolute(submodulePath)) { + throw new Error('Access denied: invalid submodule path') + } + const resolved = path.resolve(worktreePath, submodulePath) + const rel = path.relative(path.resolve(worktreePath), resolved) + if (!rel || rel === '..' || rel.startsWith(`..${path.sep}`) || path.isAbsolute(rel)) { + throw new Error('Access denied: submodule path resolves outside the worktree') + } + return resolved +} + async function readGitlinkOidFromTree( git: GitExec, worktreePath: string, @@ -136,7 +150,7 @@ export async function resolveSubmoduleCommitRange( submodulePath: string, staged = false ): Promise<{ fromOid: string; toOid: string }> { - const submoduleWorktreePath = path.join(worktreePath, submodulePath) + const submoduleWorktreePath = resolveSubmoduleWorktreePath(worktreePath, submodulePath) const fromOid = staged ? await readGitlinkOidFromTree(git, worktreePath, 'HEAD', submodulePath) : (await readGitlinkOidFromIndex(git, worktreePath, submodulePath)) || @@ -209,7 +223,7 @@ export async function computeSubmodulePointerDiff( staged: boolean, compareAgainstHead = false ) { - const submoduleWorktreePath = path.join(worktreePath, submodulePath) + const submoduleWorktreePath = resolveSubmoduleWorktreePath(worktreePath, submodulePath) let leftOid = '' let rightOid = '' if (staged) { diff --git a/src/relay/git-handler.ts b/src/relay/git-handler.ts index 04a311df38..da3820d4e4 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -25,6 +25,7 @@ import { createSubmodulePathsCache, findContainingSubmodule, listSubmodulePathsCached, + resolveSubmoduleWorktreePath, resolveSubmoduleCommitRange, type SubmodulePathsCache } from './git-handler-submodule-ops' @@ -297,11 +298,7 @@ export class GitHandler { const submodulePath = params.submodulePath as string const area = resolveSubmoduleStatusArea(params) const staged = area === 'staged' - const resolved = path.resolve(worktreePath, submodulePath) - const rel = path.relative(path.resolve(worktreePath), resolved) - if (!rel || rel === '..' || rel.startsWith(`..${path.sep}`) || path.isAbsolute(rel)) { - throw new Error(`Submodule path "${submodulePath}" resolves outside the worktree`) - } + const resolved = resolveSubmoduleWorktreePath(worktreePath, submodulePath) const workingResult = await getStatusOp(this.git.bind(this), { ...params, worktreePath: resolved @@ -388,7 +385,10 @@ export class GitHandler { compareAgainstHead ) } - const submoduleWorktreePath = path.join(worktreePath, matchedSubmodule) + const submoduleWorktreePath = resolveSubmoduleWorktreePath( + worktreePath, + matchedSubmodule + ) const innerPath = normalizedFilePath.slice(matchedSubmodule.length + 1) const { fromOid, toOid } = await resolveSubmoduleCommitRange( this.git.bind(this), diff --git a/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.test.tsx b/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.test.tsx index ee920e3f1b..6df07a3684 100644 --- a/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.test.tsx +++ b/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.test.tsx @@ -45,16 +45,18 @@ let latest: UseSourceControlSubmoduleStatusResult | null = null function Probe({ worktreeId, worktreePath, - entries + entries, + settings = null }: { worktreeId: string worktreePath: string entries: GitStatusEntry[] + settings?: { activeRuntimeEnvironmentId: string | null } | null }): null { latest = useSourceControlSubmoduleStatus({ activeWorktreeId: worktreeId, worktreePath, - activeRepoSettings: null, + activeRepoSettings: settings, entries }) return null @@ -171,6 +173,63 @@ describe('useSourceControlSubmoduleStatus', () => { }) }) + it('drops a late response from a previous runtime target on the same worktree', async () => { + const envA = deferred<{ entries: GitStatusEntry[] }>() + const envB = deferred<{ entries: GitStatusEntry[] }>() + mocks.getRuntimeGitSubmoduleStatus.mockImplementation( + (ctx: { settings?: { activeRuntimeEnvironmentId?: string | null } | null }) => + ctx.settings?.activeRuntimeEnvironmentId === 'env-b' ? envB.promise : envA.promise + ) + + const container = document.createElement('div') + const root = createRoot(container) + roots.push(root) + + await act(async () => { + root.render( + + ) + }) + await act(async () => { + latest?.toggleSubmodule(submoduleEntry()) + }) + await flush() + + await act(async () => { + root.render( + + ) + }) + await act(async () => { + latest?.toggleSubmodule(submoduleEntry()) + }) + await flush() + + await act(async () => { + envB.resolve({ entries: [innerEntry('from-env-b.ts')] }) + }) + await flush() + await act(async () => { + envA.resolve({ entries: [innerEntry('from-env-a.ts')] }) + }) + await flush() + + expect(latest?.submoduleStatusByKey['unstaged::sub']).toEqual({ + status: 'loaded', + entries: [innerEntry('from-env-b.ts')] + }) + }) + it('passes the row area when expanding a staged submodule row', async () => { mocks.getRuntimeGitSubmoduleStatus.mockResolvedValue({ entries: [innerEntry('from-index.ts')] }) diff --git a/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.ts b/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.ts index f3f23fe042..971fbe7602 100644 --- a/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.ts +++ b/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.ts @@ -39,18 +39,19 @@ export function useSourceControlSubmoduleStatus( const [submoduleStatusByKey, setSubmoduleStatusByKey] = useState< Record >({}) + const activeRuntimeRouteKey = activeRepoSettings?.activeRuntimeEnvironmentId?.trim() ?? '' + const activeConnectionRouteKey = getConnectionId(activeWorktreeId ?? null) ?? '' // Why: a monotonically increasing generation invalidates in-flight requests - // when the active worktree/path changes, so a slow response from a previous - // worktree (common over SSH) can't write stale submodule status into the - // current panel — even when both worktrees share the same submodule path. + // when the active worktree/path/runtime/SSH route changes, so a slow response + // from a previous target can't write stale submodule status into this panel. const generationRef = useRef(0) useEffect(() => { generationRef.current += 1 setExpandedSubmoduleKeys(new Set()) setSubmoduleStatusByKey({}) - }, [activeWorktreeId, worktreePath]) + }, [activeConnectionRouteKey, activeRuntimeRouteKey, activeWorktreeId, worktreePath]) const fetchSubmoduleStatus = useCallback( async (expansionKey: string): Promise => { From 1a46d887fc5b21af81060d1d2af78b22c5343e15 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sun, 28 Jun 2026 23:18:38 -0700 Subject: [PATCH 9/9] Set git identity via CLI config options in test commits - Extract test email and name into constants. - Use `-c` config flags to pass user identity to `git commit` dynamically. - This ensures commits succeed in submodule checkouts or CI environments where a local or global identity is not configured. --- src/relay/git-handler-test-setup.ts | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/src/relay/git-handler-test-setup.ts b/src/relay/git-handler-test-setup.ts index f7213c9353..512b55d309 100644 --- a/src/relay/git-handler-test-setup.ts +++ b/src/relay/git-handler-test-setup.ts @@ -9,6 +9,9 @@ import { vi } from 'vitest' import { execFileSync } from 'child_process' import type { RelayDispatcher } from './dispatcher' +const TEST_GIT_USER_EMAIL = 'test@test.com' +const TEST_GIT_USER_NAME = 'Test' + // Why: declare an explicit type so the inferred return type of // createMockDispatcher doesn't transitively reference `@vitest/spy`'s // internal `Procedure` type (from `vi.fn(...)`). Without this annotation, @@ -78,13 +81,28 @@ export function createMockDispatcher(): MockDispatcher { export function gitInit(dir: string): void { execFileSync('git', ['init'], { cwd: dir, stdio: 'pipe' }) - execFileSync('git', ['config', 'user.email', 'test@test.com'], { cwd: dir, stdio: 'pipe' }) - execFileSync('git', ['config', 'user.name', 'Test'], { cwd: dir, stdio: 'pipe' }) + execFileSync('git', ['config', 'user.email', TEST_GIT_USER_EMAIL], { cwd: dir, stdio: 'pipe' }) + execFileSync('git', ['config', 'user.name', TEST_GIT_USER_NAME], { cwd: dir, stdio: 'pipe' }) } export function gitCommit(dir: string, message: string): void { execFileSync('git', ['add', '.'], { cwd: dir, stdio: 'pipe' }) - execFileSync('git', ['commit', '-m', message, '--allow-empty'], { cwd: dir, stdio: 'pipe' }) + // Why: `git submodule add` creates a checkout that does not inherit the + // source repo's local identity config, and CI may have no global identity. + execFileSync( + 'git', + [ + '-c', + `user.email=${TEST_GIT_USER_EMAIL}`, + '-c', + `user.name=${TEST_GIT_USER_NAME}`, + 'commit', + '-m', + message, + '--allow-empty' + ], + { cwd: dir, stdio: 'pipe' } + ) } export type { RelayDispatcher }