diff --git a/src/main/codex-accounts/runtime-home-service.test.ts b/src/main/codex-accounts/runtime-home-service.test.ts index 555e19175e..d8e7a48e13 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 ad5b5976a5..00f0f4f604 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/main/git/status-porcelain-parser.test.ts b/src/main/git/status-porcelain-parser.test.ts index bba3840930..4f9347b64d 100644 --- a/src/main/git/status-porcelain-parser.test.ts +++ b/src/main/git/status-porcelain-parser.test.ts @@ -40,6 +40,24 @@ describe('StatusPorcelainParser', () => { ]) }) + it('marks staged S... submodule rows as commit-changed gitlinks', () => { + const parser = new StatusPorcelainParser() + parser.update( + '1 M. S... 160000 160000 160000 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb flutter_mine\n', + 0 + ) + parser.finish() + + expect(parser.entries).toEqual([ + { + path: 'flutter_mine', + status: 'modified', + area: 'staged', + submodule: { commitChanged: true, trackedChanges: false, untrackedChanges: false } + } + ]) + }) + it('collects unmerged lines for async resolution rather than parsing inline', () => { const parser = new StatusPorcelainParser() parser.update('u UU N... 100644 100644 100644 100644 aa bb cc both.ts\n', 0) diff --git a/src/main/git/status-porcelain-parser.ts b/src/main/git/status-porcelain-parser.ts index 3807743fbf..3353275c08 100644 --- a/src/main/git/status-porcelain-parser.ts +++ b/src/main/git/status-porcelain-parser.ts @@ -132,7 +132,6 @@ export class StatusPorcelainParser { // "2 XY sub mH mI mW hH X path\torigPath" const parts = line.split(' ') const xy = parts[1] - const submodule = parseSubmoduleStatus(parts[2]) const indexStatus = xy[0] const worktreeStatus = xy[1] @@ -149,7 +148,7 @@ export class StatusPorcelainParser { status: parseStatusChar(indexStatus), area: 'staged', oldPath, - ...(submodule ? { submodule } : {}) + ...submoduleStatusField(parts[2], indexStatus) }) } if (worktreeStatus !== '.') { @@ -158,7 +157,7 @@ export class StatusPorcelainParser { status: parseStatusChar(worktreeStatus), area: 'unstaged', oldPath, - ...(submodule ? { submodule } : {}) + ...submoduleStatusField(parts[2], worktreeStatus) }) } return @@ -170,7 +169,7 @@ export class StatusPorcelainParser { path, status: parseStatusChar(indexStatus), area: 'staged', - ...(submodule ? { submodule } : {}) + ...submoduleStatusField(parts[2], indexStatus) }) } if (worktreeStatus !== '.') { @@ -178,7 +177,7 @@ export class StatusPorcelainParser { path, status: parseStatusChar(worktreeStatus), area: 'unstaged', - ...(submodule ? { submodule } : {}) + ...submoduleStatusField(parts[2], worktreeStatus) }) } } @@ -207,14 +206,23 @@ export function parseStatusChar(char: string): GitStatusEntry['status'] { } export function parseSubmoduleStatus( - submoduleField: string | undefined + submoduleField: string | undefined, + statusChar = '.' ): GitStatusEntry['submodule'] { if (!submoduleField?.startsWith('S')) { return undefined } return { - commitChanged: submoduleField[1] === 'C', + commitChanged: submoduleField[1] === 'C' || (submoduleField === 'S...' && statusChar === 'M'), trackedChanges: submoduleField[2] === 'M', untrackedChanges: submoduleField[3] === 'U' } } + +function submoduleStatusField( + submoduleField: string | undefined, + statusChar: string +): { submodule: GitStatusEntry['submodule'] } | {} { + const submodule = parseSubmoduleStatus(submoduleField, statusChar) + return submodule ? { submodule } : {} +} diff --git a/src/main/git/status.test.ts b/src/main/git/status.test.ts index b91c8e06a0..858d4e51a6 100644 --- a/src/main/git/status.test.ts +++ b/src/main/git/status.test.ts @@ -69,6 +69,7 @@ import { bulkDiscardChanges, bulkUnstageFiles, clearEffectiveUpstreamStatusCacheForTests, + clearSubmodulePathsCacheForTests, detectConflictOperation, getBranchDiff, discardChanges, @@ -78,7 +79,10 @@ import { getDiff, getStagedCommitContext, getStatus, + getSubmoduleStatus, isWithinWorktree, + listSubmodulePaths, + resolveSubmoduleWorktreePath, stageFile } from './status' @@ -702,6 +706,312 @@ 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('diffs staged inner files from parent HEAD to parent index', async () => { + 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 ${NEW_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: '' }) + }) + 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', true) + + 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') + }) + + it('rejects inner submodule diffs whose .gitmodules path escapes the worktree', async () => { + // A crafted .gitmodules path must not let the inner diff read escape the + // selected worktree; loadDiff routes through resolveSubmoduleWorktreePath. + gitExecFileAsyncMock.mockImplementation((args: string[], options?: { cwd?: string }) => { + if (args[0] === 'config' && args.includes('.gitmodules')) { + return Promise.resolve({ + stdout: options?.cwd === PARENT ? 'submodule.evil.path ../evil\n' : '' + }) + } + return Promise.resolve({ stdout: '' }) + }) + + await expect(getDiff(PARENT, '../evil/secret.txt', false)).rejects.toThrow('Access denied') + }) + + it('rejects gitlink pointer diffs whose .gitmodules path escapes the worktree', async () => { + gitExecFileAsyncMock.mockImplementation((args: string[], options?: { cwd?: string }) => { + if (args[0] === 'config' && args.includes('.gitmodules')) { + return Promise.resolve({ + stdout: options?.cwd === PARENT ? 'submodule.evil.path ../evil\n' : '' + }) + } + return Promise.resolve({ stdout: '' }) + }) + + await expect(getDiff(PARENT, '../evil', false)).rejects.toThrow('Access denied') + }) +}) + +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('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() + 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' }) + ) + }) + + it('includes staged commit-range entries from parent HEAD to parent index', 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 submodule worktree: the staged parent gitlink still has files to show. + if (args.includes('--name-status')) { + return Promise.resolve({ stdout: 'M\tlib/main.dart\n' }) + } + if (args[0] === 'ls-files') { + return Promise.resolve({ stdout: `160000 ${NEW_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: '' }) + }) + + const result = await getSubmoduleStatus('/repo', 'flutter_mine', { staged: true }) + + expect(result.entries).toContainEqual( + expect.objectContaining({ path: 'lib/main.dart', status: 'modified', area: 'unstaged' }) + ) + }) +}) + describe('getStatus', () => { beforeEach(() => { clearEffectiveUpstreamStatusCacheForTests() @@ -785,6 +1095,36 @@ describe('getStatus', () => { expect(statusCommandCalls).toBe(2) }) + it('clears in-flight status reads when a mutation runs', async () => { + readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n') + existsSyncMock.mockReturnValue(false) + let statusCommandCalls = 0 + const releaseStatusReads: (() => void)[] = [] + gitExecFileAsyncMock.mockImplementation((args: string[]) => { + if (args.includes('status')) { + statusCommandCalls += 1 + return new Promise<{ stdout: string }>((resolve) => { + releaseStatusReads.push(() => resolve({ stdout: '' })) + }) + } + return Promise.resolve({ stdout: '' }) + }) + + const first = getStatus('/repo') + await vi.waitFor(() => expect(statusCommandCalls).toBe(1)) + + // A mutation must invalidate the in-flight status read so the next getStatus + // starts fresh instead of joining a pre-mutation promise and going stale. + await stageFile('/repo', 'src/file.ts') + + const second = getStatus('/repo') + await vi.waitFor(() => expect(statusCommandCalls).toBe(2)) + + releaseStatusReads.splice(0).forEach((release) => release()) + await Promise.all([first, second]) + expect(statusCommandCalls).toBe(2) + }) + it('parses unmerged porcelain v2 entries into unresolved conflict rows', async () => { readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n') existsSyncMock.mockImplementation((target: string) => target.endsWith('MERGE_HEAD')) diff --git a/src/main/git/status.ts b/src/main/git/status.ts index 4902a23e59..5073da3421 100644 --- a/src/main/git/status.ts +++ b/src/main/git/status.ts @@ -61,6 +61,10 @@ 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>() @@ -68,19 +72,37 @@ const gitDiffReadDedupe = new InFlightPromiseDedupe() const effectiveUpstreamStatusWriteGeneration = new Map() const statusReadsInFlight = new Map>() +// Why: a mutation invalidates both in-flight diff reads and in-flight status +// coalescing; clearing only the diff dedupe would let a post-mutation +// getStatus() join a pre-mutation read and return stale entries. +function clearGitReadInvalidationState(): void { + gitDiffReadDedupe.clear() + statusReadsInFlight.clear() + submodulePathsCache.clear() +} + +export function clearSubmodulePathsCacheForTests(): void { + submodulePathsCache.clear() +} + 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 { effectiveUpstreamStatusCache.clear() effectiveUpstreamStatusInFlight.clear() retiredEffectiveUpstreamStatusInFlight.clear() - gitDiffReadDedupe.clear() effectiveUpstreamStatusWriteGeneration.clear() - statusReadsInFlight.clear() + clearGitReadInvalidationState() } export function getEffectiveUpstreamStatusCacheCountForTests(): number { @@ -280,6 +302,125 @@ async function runGetStatus( } } +/** + * 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 & { staged?: boolean } = {} +): 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 = options.staged + ? await readGitlinkOidFromTree(worktreePath, 'HEAD', submodulePath, options) + : (await readGitlinkOidFromIndex(worktreePath, submodulePath, options)) || + (await readGitlinkOidFromTree(worktreePath, 'HEAD', submodulePath, options)) + const toOid = options.staged + ? await readGitlinkOidFromIndex(worktreePath, submodulePath, options) + : await readWorkingSubmoduleHead(submoduleWorktreePath, options) + if (fromOid && toOid && fromOid !== toOid) { + const rangeEntries = await computeSubmoduleRangeEntries( + submoduleWorktreePath, + fromOid, + toOid, + options + ) + if (options.staged) { + return { ...workingResult, entries: rangeEntries } + } + 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 } + } + if (options.staged) { + 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, @@ -725,6 +866,186 @@ 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 cacheKey = getSubmodulePathsCacheKey(worktreePath, options) + const cached = submodulePathsCache.get(cacheKey) + 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(cacheKey, { 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, + // Why: default to the validated resolver so every caller (not just loadDiff) + // is protected from a .gitmodules path escaping the parent worktree. + submoduleWorktreePath = resolveSubmoduleWorktreePath(worktreePath, submodulePath) +): Promise { + 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. */ @@ -735,6 +1056,8 @@ export async function getDiff( compareAgainstHead = false, options: GitRuntimeOptions = {} ): Promise { + // Why: register the in-flight dedupe synchronously (before any await) so + // concurrent identical reads coalesce; submodule routing happens inside. return gitDiffReadDedupe.run( stableInFlightKey([ 'diff', @@ -755,6 +1078,53 @@ async function loadDiff( compareAgainstHead: boolean, 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) { + // Why: matchedSubmodule originates from .gitmodules, so validate it against + // the worktree boundary before any inner read — a crafted submodule path + // must not let the diff escape the selected repo. + const submoduleWorktreePath = resolveSubmoduleWorktreePath(worktreePath, matchedSubmodule) + const normalizedFilePath = filePath.replace(/\\/g, '/').replace(/\/+$/, '') + if (normalizedFilePath === matchedSubmodule) { + return buildSubmodulePointerDiff( + worktreePath, + matchedSubmodule, + staged, + compareAgainstHead, + options, + submoduleWorktreePath + ) + } + const innerPath = normalizedFilePath.slice(matchedSubmodule.length + 1) + const fromOid = staged + ? await readGitlinkOidFromTree(worktreePath, 'HEAD', matchedSubmodule, options) + : (await readGitlinkOidFromIndex(worktreePath, matchedSubmodule, options)) || + (await readGitlinkOidFromTree(worktreePath, 'HEAD', matchedSubmodule, options)) + const toOid = staged + ? await readGitlinkOidFromIndex(worktreePath, matchedSubmodule, options) + : 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 @@ -1396,14 +1766,14 @@ export async function stageFile( filePath: string, options: GitRuntimeOptions = {} ): Promise { - gitDiffReadDedupe.clear() + clearGitReadInvalidationState() try { await gitExecFileAsync( ['add', '--', literalPathspec(filePath)], gitOptionsForWorktree(worktreePath, options) ) } finally { - gitDiffReadDedupe.clear() + clearGitReadInvalidationState() } } @@ -1415,13 +1785,13 @@ export async function unstageFile( filePath: string, options: GitRuntimeOptions = {} ): Promise { - gitDiffReadDedupe.clear() + clearGitReadInvalidationState() try { await gitExecFileAsync(['restore', '--staged', '--', literalPathspec(filePath)], { ...gitOptionsForWorktree(worktreePath, options) }) } finally { - gitDiffReadDedupe.clear() + clearGitReadInvalidationState() } } @@ -1478,7 +1848,7 @@ export async function commitChanges( message: string, options: GitRuntimeOptions = {} ): Promise<{ success: boolean; error?: string }> { - gitDiffReadDedupe.clear() + clearGitReadInvalidationState() try { await gitExecFileAsync(['commit', '-m', message], gitOptionsForWorktree(worktreePath, options)) return { success: true } @@ -1501,7 +1871,7 @@ export async function commitChanges( (error instanceof Error ? error.message : 'Commit failed') return { success: false, error: errorMessage } } finally { - gitDiffReadDedupe.clear() + clearGitReadInvalidationState() } } @@ -1513,7 +1883,7 @@ export async function discardChanges( filePath: string, options: GitRuntimeOptions = {} ): Promise { - gitDiffReadDedupe.clear() + clearGitReadInvalidationState() const resolvedWorktree = path.resolve(worktreePath) const resolvedTarget = path.resolve(worktreePath, filePath) try { @@ -1545,7 +1915,7 @@ export async function discardChanges( cleanUntrackedPaths(worktreePath, [targetPath], options) ) } finally { - gitDiffReadDedupe.clear() + clearGitReadInvalidationState() } } @@ -1615,7 +1985,7 @@ export async function bulkDiscardChanges( filePaths: string[], options: GitRuntimeOptions = {} ): Promise { - gitDiffReadDedupe.clear() + clearGitReadInvalidationState() if (filePaths.length === 0) { return } @@ -1653,7 +2023,7 @@ export async function bulkDiscardChanges( } ) } finally { - gitDiffReadDedupe.clear() + clearGitReadInvalidationState() } } @@ -1679,7 +2049,7 @@ export async function bulkStageFiles( filePaths: string[], options: GitRuntimeOptions = {} ): Promise { - gitDiffReadDedupe.clear() + clearGitReadInvalidationState() if (filePaths.length === 0) { return } @@ -1692,7 +2062,7 @@ export async function bulkStageFiles( ) } } finally { - gitDiffReadDedupe.clear() + clearGitReadInvalidationState() } } @@ -1704,7 +2074,7 @@ export async function bulkUnstageFiles( filePaths: string[], options: GitRuntimeOptions = {} ): Promise { - gitDiffReadDedupe.clear() + clearGitReadInvalidationState() if (filePaths.length === 0) { return } @@ -1716,6 +2086,6 @@ export async function bulkUnstageFiles( }) } } finally { - gitDiffReadDedupe.clear() + clearGitReadInvalidationState() } } diff --git a/src/main/ipc/filesystem.ts b/src/main/ipc/filesystem.ts index 9e9ff27670..29ee293059 100644 --- a/src/main/ipc/filesystem.ts +++ b/src/main/ipc/filesystem.ts @@ -17,6 +17,7 @@ import type { GitForkSyncExpectedUpstream, GitForkSyncResult, GlobalSettings, + GitStagingArea, GitPushTarget, GitUpstreamStatus, GitStatusResult, @@ -37,6 +38,7 @@ import { } from '../../shared/text-search' import { getStatus, + getSubmoduleStatus, abortMerge, abortRebase, detectConflictOperation, @@ -1051,6 +1053,40 @@ 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 + area?: GitStagingArea + } + ): 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, args.area) + } + const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store) + const gitOptions = getLocalGitOptionsForRegisteredWorktree( + store, + args.worktreePath, + worktreePath + ) + return getSubmoduleStatus(worktreePath, args.submodulePath, { + ...gitOptions, + ...(args.area === 'staged' ? { staged: true } : {}) + }) + } + ) + ipcMain.handle( 'git:checkIgnored', async ( diff --git a/src/main/providers/ssh-git-provider.test.ts b/src/main/providers/ssh-git-provider.test.ts index 01736fb2de..cce6813c22 100644 --- a/src/main/providers/ssh-git-provider.test.ts +++ b/src/main/providers/ssh-git-provider.test.ts @@ -93,6 +93,53 @@ 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', + area: 'unstaged' + }) + expect(result).toEqual(statusResult) + }) + + it('getSubmoduleStatus forwards the requested source-control area', async () => { + const statusResult = { entries: [], conflictOperation: 'unknown' } + mux.request.mockResolvedValue(statusResult) + + await provider.getSubmoduleStatus('/home/user/repo', 'vendor/lib', 'staged') + + expect(mux.request).toHaveBeenCalledWith('git.submoduleStatus', { + worktreePath: '/home/user/repo', + submodulePath: 'vendor/lib', + area: 'staged' + }) + }) + + 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']) @@ -943,6 +990,31 @@ describe('SshGitProvider', () => { expect(mux.request).toHaveBeenCalledTimes(3) }) + it('clears pending diff RPCs when submodule status runs', async () => { + const diff = { + kind: 'text', + originalContent: 'old', + modifiedContent: 'new', + originalIsBinary: false, + modifiedIsBinary: false + } + const pendingDiff = deferredValue(diff) + mux.request.mockReturnValueOnce(pendingDiff.promise) + + const first = provider.getDiff('/home/user/repo', 'src/file.ts', false, true) + await waitForRequestCount(mux.request, 1) + + mux.request.mockResolvedValueOnce({ entries: [], conflictOperation: 'unknown' }) + await provider.getSubmoduleStatus('/home/user/repo', 'vendor/lib') + + mux.request.mockResolvedValueOnce(diff) + const second = provider.getDiff('/home/user/repo', 'src/file.ts', false, true) + + pendingDiff.resolve() + await expect(Promise.all([first, second])).resolves.toEqual([diff, diff]) + expect(mux.request).toHaveBeenCalledTimes(3) + }) + it('clears pending diff RPCs when a mutation runs', async () => { const diff = { kind: 'text', diff --git a/src/main/providers/ssh-git-provider.ts b/src/main/providers/ssh-git-provider.ts index 563f3e60b5..8f07490480 100644 --- a/src/main/providers/ssh-git-provider.ts +++ b/src/main/providers/ssh-git-provider.ts @@ -13,6 +13,7 @@ import type { GitForkSyncExpectedUpstream, GitForkSyncResult, GitPushTarget, + GitStagingArea, GitUpstreamStatus, GitWorktreeInfo, RemoveWorktreeResult @@ -113,6 +114,33 @@ export class SshGitProvider implements IGitProvider { : this.mux.request('git.status', request))) as GitStatusResult } + async getSubmoduleStatus( + worktreePath: string, + submodulePath: string, + area: GitStagingArea = 'unstaged' + ): Promise { + // Why: mirror getStatus() — refreshing submodule state must invalidate + // in-flight diff reads so a later diff can't reuse a stale pending RPC. + this.gitDiffReadDedupe.clear() + try { + return (await this.mux.request('git.submoduleStatus', { + worktreePath, + submodulePath, + area + })) 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 { return (await this.mux.request('git.checkIgnored', { worktreePath, diff --git a/src/main/providers/types.ts b/src/main/providers/types.ts index 6561d12024..eeca8fda89 100644 --- a/src/main/providers/types.ts +++ b/src/main/providers/types.ts @@ -9,6 +9,7 @@ import type { GitForkSyncExpectedUpstream, GitForkSyncResult, GitPushTarget, + GitStagingArea, GitUpstreamStatus, GitWorktreeInfo, RemoveWorktreeResult, @@ -174,6 +175,11 @@ export type GitProviderStatusOptions = { export type IGitProvider = { getStatus(worktreePath: string, options?: GitProviderStatusOptions): Promise + getSubmoduleStatus( + worktreePath: string, + submodulePath: string, + area?: GitStagingArea + ): 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..7aaa8ede56 100644 --- a/src/main/runtime/orca-runtime-git.ts +++ b/src/main/runtime/orca-runtime-git.ts @@ -7,6 +7,7 @@ import type { GitForkSyncExpectedUpstream, GitForkSyncResult, GitPushTarget, + GitStagingArea, GitStatusResult, GitUpstreamStatus, GitWorktreeInfo, @@ -41,6 +42,7 @@ import { getDiff, getStagedCommitContext, getStatus as getGitStatus, + getSubmoduleStatus as getGitSubmoduleStatus, stageFile, unstageFile } from '../git/status' @@ -183,6 +185,25 @@ export class RuntimeGitCommands { : getGitStatus(target.worktree.path, gitOptions) } + async getRuntimeGitSubmoduleStatus( + worktreeSelector: string, + submodulePath: string, + area: GitStagingArea = 'unstaged' + ): 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, area) + } + return getGitSubmoduleStatus(target.worktree.path, submodulePath, { + ...localGitOptionsForTarget(target), + ...(area === 'staged' ? { staged: true } : {}) + }) + } + async checkRuntimeGitIgnoredPaths( worktreeSelector: string, relativePaths: string[] diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 46361aa7ba..673631def1 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -4771,6 +4771,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..8f9d511dfc 100644 --- a/src/main/runtime/rpc/methods/git-params.ts +++ b/src/main/runtime/rpc/methods/git-params.ts @@ -16,6 +16,22 @@ 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 -') + ), + // Why: submodule expansion is requested from a Source Control row; the row + // area determines whether the gitlink range is HEAD->index or index->worktree. + area: z.enum(['staged', 'unstaged', 'untracked']).optional() +}) + export const GitFilePath = WorktreeSelector.extend({ filePath: z .unknown() diff --git a/src/main/runtime/rpc/methods/git.test.ts b/src/main/runtime/rpc/methods/git.test.ts index c6307c0aa6..850276906a 100644 --- a/src/main/runtime/rpc/methods/git.test.ts +++ b/src/main/runtime/rpc/methods/git.test.ts @@ -105,6 +105,35 @@ describe('git RPC methods', () => { }) }) + it('returns submodule status for a selected worktree area', async () => { + const runtime = { + getRuntimeId: () => 'test-runtime', + getRuntimeGitSubmoduleStatus: vi.fn().mockResolvedValue({ + entries: [{ path: 'lib.ts', status: 'modified', area: 'unstaged' }], + conflictOperation: 'unknown' + }) + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: GIT_METHODS }) + + const response = await dispatcher.dispatch( + makeRequest('git.submoduleStatus', { + worktree: 'id:wt-1', + submodulePath: 'vendor/lib', + area: 'staged' + }) + ) + + expect(runtime.getRuntimeGitSubmoduleStatus).toHaveBeenCalledWith( + 'id:wt-1', + 'vendor/lib', + 'staged' + ) + expect(response).toMatchObject({ + ok: true, + result: { entries: [{ path: 'lib.ts' }] } + }) + }) + it('returns a worktree file diff', async () => { const runtime = { getRuntimeId: () => 'test-runtime', diff --git a/src/main/runtime/rpc/methods/git.ts b/src/main/runtime/rpc/methods/git.ts index 964f7d09ce..1e4923f672 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, params.area) + }), defineMethod({ name: 'git.history', params: GitHistory, diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index a25297106c..af164a9f73 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -48,6 +48,7 @@ import type { GitForkSyncExpectedUpstream, GitForkSyncResult, GitPushTarget, + GitStagingArea, GitStatusResult, GitUpstreamStatus, GitHubAssignableUser, @@ -2189,6 +2190,12 @@ export type PreloadApi = { includeIgnored?: boolean bypassEffectiveUpstreamNegativeCache?: boolean }) => Promise + submoduleStatus: (args: { + worktreePath: string + submodulePath: string + connectionId?: string + area?: GitStagingArea + }) => Promise checkIgnored: (args: { worktreePath: string paths: string[] diff --git a/src/preload/index.ts b/src/preload/index.ts index bac40a93a9..f3710a0f3c 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -26,6 +26,7 @@ import type { GitHubCommentResult, GitHubWorkItem, GitPushTarget, + GitStagingArea, GitForkSyncExpectedUpstream, GitForkSyncResult, GitUpstreamStatus, @@ -2593,6 +2594,12 @@ const api = { includeIgnored?: boolean bypassEffectiveUpstreamNegativeCache?: boolean }): Promise => ipcRenderer.invoke('git:status', args), + submoduleStatus: (args: { + worktreePath: string + submodulePath: string + connectionId?: string + area?: GitStagingArea + }): Promise => ipcRenderer.invoke('git:submoduleStatus', args), checkIgnored: (args: { worktreePath: string paths: string[] 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..be60aa34d5 --- /dev/null +++ b/src/relay/git-handler-submodule-ops.test.ts @@ -0,0 +1,91 @@ +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, + resolveSubmoduleWorktreePath +} 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) + }) +}) + +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 new file mode 100644 index 0000000000..088e59e9fe --- /dev/null +++ b/src/relay/git-handler-submodule-ops.ts @@ -0,0 +1,248 @@ +/** + * 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' + +/** + * 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. + */ +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 +} + +// 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, + 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, + staged = false +): Promise<{ fromOid: string; toOid: string }> { + const submoduleWorktreePath = resolveSubmoduleWorktreePath(worktreePath, submodulePath) + const fromOid = staged + ? await readGitlinkOidFromTree(git, worktreePath, 'HEAD', submodulePath) + : (await readGitlinkOidFromIndex(git, worktreePath, submodulePath)) || + (await readGitlinkOidFromTree(git, worktreePath, 'HEAD', submodulePath)) + const toOid = staged + ? await readGitlinkOidFromIndex(git, worktreePath, submodulePath) + : 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 = resolveSubmoduleWorktreePath(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-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 } diff --git a/src/relay/git-handler.test.ts b/src/relay/git-handler.test.ts index 90b516a947..e0af8e07a5 100644 --- a/src/relay/git-handler.test.ts +++ b/src/relay/git-handler.test.ts @@ -592,6 +592,167 @@ 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') + }) + + it('lists and diffs staged submodule pointer changes from parent HEAD to index', 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') + execFileSync('git', ['add', 'flutter_mine'], { cwd: tmpDir, stdio: 'pipe' }) + + const status = (await dispatcher.callRequest('git.submoduleStatus', { + worktreePath: tmpDir, + submodulePath: 'flutter_mine', + area: 'staged' + })) 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: true + })) 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 31c796418f..da3820d4e4 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -18,6 +18,17 @@ import { branchDiffEntries, validateGitExecArgs } from './git-handler-ops' +import { + buildSubmoduleInnerCommitRangeDiff, + computeSubmodulePointerDiff, + computeSubmoduleRangeEntries, + createSubmodulePathsCache, + findContainingSubmodule, + listSubmodulePathsCached, + resolveSubmoduleWorktreePath, + resolveSubmoduleCommitRange, + type SubmodulePathsCache +} from './git-handler-submodule-ops' import { commitCompare as commitCompareOp, commitDiffEntry } from './git-handler-commit-diff-ops' import { areRelayWorktreePathsEqual, @@ -53,6 +64,15 @@ const execFileAsync = promisify(execFile) const MAX_GIT_BUFFER = 10 * 1024 * 1024 const BULK_CHUNK_SIZE = 100 +function resolveSubmoduleStatusArea( + params: Record +): 'staged' | 'unstaged' | 'untracked' { + if (params.area === 'staged' || params.area === 'unstaged' || params.area === 'untracked') { + return params.area + } + return 'unstaged' +} + function getErrorText(error: unknown): string { if (typeof error === 'object' && error !== null) { const parts: string[] = [] @@ -150,6 +170,11 @@ export class GitHandler { private dispatcher: RelayDispatcher private readonly gitDiffReadDedupe = new InFlightPromiseDedupe() + // 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) { @@ -159,6 +184,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)) @@ -263,6 +289,52 @@ 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 area = resolveSubmoduleStatusArea(params) + const staged = area === 'staged' + const resolved = resolveSubmoduleWorktreePath(worktreePath, submodulePath) + 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, + staged + ) + if (fromOid && toOid && fromOid !== toOid) { + const rangeEntries = await computeSubmoduleRangeEntries( + this.git.bind(this), + resolved, + fromOid, + toOid + ) + if (staged) { + return { ...workingResult, entries: rangeEntries } + } + const rangePaths = new Set(rangeEntries.map((entry) => entry.path)) + const entries = [ + ...rangeEntries, + ...workingResult.entries.filter((entry) => !rangePaths.has(entry.path)) + ] + return { ...workingResult, entries } + } + if (staged) { + return { ...workingResult, entries: [] } + } + return workingResult + } + private async checkIgnored(params: Record) { return checkIgnoredPathsOp(this.git.bind(this), params) } @@ -285,22 +357,74 @@ 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: register the in-flight dedupe synchronously (before any await) so + // concurrent identical reads coalesce; submodule routing happens inside. return this.gitDiffReadDedupe.run( - stableInFlightKey([ - 'diff', - worktreePath, - filePath, - params.staged as boolean, - params.compareAgainstHead as boolean | undefined - ]), - () => - computeDiff( + stableInFlightKey(['diff', worktreePath, filePath, staged, compareAgainstHead]), + async () => { + // 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 local). + const submodulePaths = await listSubmodulePathsCached( + this.git.bind(this), + worktreePath, + this.submodulePathsCache + ) + 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 = resolveSubmoduleWorktreePath( + worktreePath, + matchedSubmodule + ) + const innerPath = normalizedFilePath.slice(matchedSubmodule.length + 1) + const { fromOid, toOid } = await resolveSubmoduleCommitRange( + this.git.bind(this), + worktreePath, + matchedSubmodule, + staged + ) + // 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/relay/git-status-output-parser.test.ts b/src/relay/git-status-output-parser.test.ts new file mode 100644 index 0000000000..ec25e9b72b --- /dev/null +++ b/src/relay/git-status-output-parser.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from 'vitest' +import { parseStatusOutput } from './git-status-output-parser' + +describe('parseStatusOutput', () => { + it('marks staged S... submodule rows as commit-changed gitlinks', () => { + const parsed = parseStatusOutput( + '1 M. S... 160000 160000 160000 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb flutter_mine\n' + ) + + expect(parsed.entries).toEqual([ + { + path: 'flutter_mine', + status: 'modified', + area: 'staged', + submodule: { commitChanged: true, trackedChanges: false, untrackedChanges: false } + } + ]) + }) +}) diff --git a/src/relay/git-status-output-parser.ts b/src/relay/git-status-output-parser.ts index 22d55e652d..9bb1d34407 100644 --- a/src/relay/git-status-output-parser.ts +++ b/src/relay/git-status-output-parser.ts @@ -69,7 +69,6 @@ export function parseStatusOutput(stdout: string): { if (line.startsWith('1 ') || line.startsWith('2 ')) { const parts = line.split(' ') const xy = parts[1] - const submodule = parseSubmoduleStatus(parts[2]) const indexStatus = xy[0] const worktreeStatus = xy[1] @@ -85,7 +84,7 @@ export function parseStatusOutput(stdout: string): { status: parseStatusChar(indexStatus), area: 'staged', oldPath, - ...(submodule ? { submodule } : {}) + ...submoduleStatusField(parts[2], indexStatus) }) } if (worktreeStatus !== '.') { @@ -94,7 +93,7 @@ export function parseStatusOutput(stdout: string): { status: parseStatusChar(worktreeStatus), area: 'unstaged', oldPath, - ...(submodule ? { submodule } : {}) + ...submoduleStatusField(parts[2], worktreeStatus) }) } } else { @@ -104,7 +103,7 @@ export function parseStatusOutput(stdout: string): { path: filePath, status: parseStatusChar(indexStatus), area: 'staged', - ...(submodule ? { submodule } : {}) + ...submoduleStatusField(parts[2], indexStatus) }) } if (worktreeStatus !== '.') { @@ -112,7 +111,7 @@ export function parseStatusOutput(stdout: string): { path: filePath, status: parseStatusChar(worktreeStatus), area: 'unstaged', - ...(submodule ? { submodule } : {}) + ...submoduleStatusField(parts[2], worktreeStatus) }) } } @@ -143,18 +142,29 @@ export function parseStatusOutput(stdout: string): { } function parseSubmoduleStatus( - submoduleField: string | undefined + submoduleField: string | undefined, + statusChar = '.' ): { commitChanged: boolean; trackedChanges: boolean; untrackedChanges: boolean } | undefined { if (!submoduleField?.startsWith('S')) { return undefined } return { - commitChanged: submoduleField[1] === 'C', + commitChanged: submoduleField[1] === 'C' || (submoduleField === 'S...' && statusChar === 'M'), trackedChanges: submoduleField[2] === 'M', untrackedChanges: submoduleField[3] === 'U' } } +function submoduleStatusField( + submoduleField: string | undefined, + statusChar: string +): + | { submodule: { commitChanged: boolean; trackedChanges: boolean; untrackedChanges: boolean } } + | {} { + const submodule = parseSubmoduleStatus(submoduleField, statusChar) + return submodule ? { submodule } : {} +} + function parseBranchAheadBehind(line: string): { ahead: number; behind: number } | null { const match = line.match(/^# branch\.ab \+(\d+) -(\d+)$/) if (!match) { 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 ff850d554e..f00bbe9d78 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,12 +5,14 @@ import { CompareSummaryToolbarButton, refreshSourceControlAfterRemoteAction, resolveSourceControlBaseRef, + resolveSourceControlCompareBaseRef, resolveSourceControlPickerBaseRef, + shouldClearBranchCompareForMissingBase, shouldRefreshBranchCompareForRemoteStatus, shouldRefreshBranchCompareForStatusHead, shouldShowCompareSummary } from './SourceControl' -import type { GitBranchCompareSummary } from '../../../../shared/types' +import type { GitBranchCompareSummary, GitUpstreamStatus } from '../../../../shared/types' type ReactElementLike = { type: unknown @@ -209,6 +211,137 @@ 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('falls back to the merge target when on and the branch has no upstream', () => { + expect( + resolveSourceControlCompareBaseRef({ + enabled: true, + worktreeBaseRef: null, + repoBaseRef: null, + upstreamName: null, + fallbackBaseRef: 'origin/master' + }) + ).toBe('origin/master') + }) + + it('returns null only when no upstream or fallback base exists', () => { + expect( + resolveSourceControlCompareBaseRef({ + enabled: true, + worktreeBaseRef: null, + repoBaseRef: null, + upstreamName: null, + fallbackBaseRef: null + }) + ).toBeNull() + }) + + it('keeps the branch compare while upstream status is still loading', () => { + // remoteStatus undefined means upstream status has not loaded yet; the + // upstream policy can still make compareBaseRef momentarily null when no + // fallback base is available. + 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.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 f8031e09dd..21cdfb98e2 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, @@ -84,6 +89,16 @@ import { namespaceSourceControlTreeDirectoryKeys, type SourceControlTreeNode } from './source-control-tree' +import { + collectListSelectionEntries, + getSubmoduleExpansionKey, + injectExpandedSubmoduleEntries, + injectExpandedSubmoduleRows, + isExpandableSubmoduleEntry, + type RenderableSourceControlNode, + type RenderableSubmoduleListItem +} from './source-control-submodule-expansion' +import { useSourceControlSubmoduleStatus } from './useSourceControlSubmoduleStatus' import { buildSourceControlDisplaySections, getSourceControlSectionViewAction, @@ -341,6 +356,45 @@ 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. Branches without an +// upstream fall back to effectiveBaseRef so the automatic policy never makes +// the committed-changes comparison disappear unexpectedly. 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() || input.fallbackBaseRef?.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 @@ -484,6 +538,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) @@ -805,6 +862,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) @@ -1099,6 +1157,13 @@ function SourceControlInner(): React.JSX.Element { const isFolder = activeRepo ? isFolderRepo(activeRepo) : false const worktreePath = activeWorktree?.path ?? null + const { expandedSubmoduleKeys, submoduleStatusByKey, toggleSubmodule } = + useSourceControlSubmoduleStatus({ + activeWorktreeId, + worktreePath, + activeRepoSettings, + entries + }) const activeCommitMessageGenerationKey = getCommitMessageGenerationRecordKey( activeWorktreeId, worktreePath @@ -1373,6 +1438,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 @@ -1656,18 +1730,6 @@ function SourceControlInner(): React.JSX.Element { [branchEntries, fileFilterState] ) - const flatEntries = useMemo(() => { - const arr: FlatEntry[] = [] - for (const section of displaySections) { - if (!collapsedSections.has(section.id)) { - for (const entry of section.items) { - arr.push({ key: `${entry.area}::${entry.path}`, entry, area: entry.area }) - } - } - } - return arr - }, [collapsedSections, displaySections]) - const treeRootsBySection = useMemo(() => { const roots: Partial> = {} @@ -1688,16 +1750,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), + expandedSubmoduleKeys, + submoduleStatusByKey, + SUBMODULE_LOADING_LABEL, + SUBMODULE_EMPTY_LABEL + ) + } + return rows + }, [ + collapsedTreeDirs, + displaySections, + treeRootsBySection, + expandedSubmoduleKeys, + submoduleStatusByKey + ]) + + // 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, + expandedSubmoduleKeys, + submoduleStatusByKey, + SUBMODULE_LOADING_LABEL, + SUBMODULE_EMPTY_LABEL ) } return rows - }, [collapsedTreeDirs, displaySections, treeRootsBySection]) + }, [displaySections, expandedSubmoduleKeys, submoduleStatusByKey]) const branchTreeRoots = useMemo( () => compactSourceControlTree(buildSourceControlTree('branch', filteredBranchEntries)), @@ -1709,11 +1795,20 @@ function SourceControlInner(): React.JSX.Element { ) const visibleSelectionEntries = useMemo(() => { + const arr: FlatEntry[] = [] + // Why: list view splices lazily-loaded submodule child rows into the + // rendered list, so selection/range/open-key bookkeeping must read the same + // injected rows instead of the pre-injection flat entries. if (sourceControlViewMode === 'list') { - return flatEntries + for (const section of displaySections) { + if (collapsedSections.has(section.id)) { + continue + } + arr.push(...collectListSelectionEntries(visibleListRowsBySection[section.id] ?? [])) + } + return arr } - const arr: FlatEntry[] = [] for (const section of displaySections) { if (collapsedSections.has(section.id)) { continue @@ -1728,8 +1823,8 @@ function SourceControlInner(): React.JSX.Element { }, [ collapsedSections, displaySections, - flatEntries, sourceControlViewMode, + visibleListRowsBySection, visibleTreeRowsBySection ]) @@ -1994,11 +2089,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) { @@ -2023,7 +2118,7 @@ function SourceControlInner(): React.JSX.Element { activeWorktreeId, beginGitBranchCompareRequest, commitMessage, - effectiveBaseRef, + compareBaseRef, grouped.staged.length, refreshActiveGitStatusAfterMutation, updateCommitDrafts, @@ -4294,7 +4389,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] ) @@ -4519,11 +4617,11 @@ function SourceControlInner(): React.JSX.Element { const branchCompareRemoteStatusRef = 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] @@ -4534,12 +4632,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 }) } @@ -4554,13 +4652,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, @@ -4577,7 +4675,7 @@ function SourceControlInner(): React.JSX.Element { activeWorktreeId, beginGitBranchCompareRequest, branchName, - effectiveBaseRef, + compareBaseRef, isFolder, setGitBranchCompareResult, worktreePath @@ -4653,7 +4751,7 @@ function SourceControlInner(): React.JSX.Element { worktreePath, connectionId }, - { limit: 50, baseRef: effectiveBaseRef } + { limit: 50, baseRef: compareBaseRef } ) if (gitHistoryRequestByWorktreeRef.current[worktreeId] !== requestId) { return @@ -4677,7 +4775,7 @@ function SourceControlInner(): React.JSX.Element { }, [ activeRepoSettings, activeWorktreeId, - effectiveBaseRef, + compareBaseRef, isBranchVisible, isFolder, isGitHistoryExpanded, @@ -4689,13 +4787,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 } @@ -4707,14 +4805,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) { branchCompareRemoteStatusRef.current = null return } @@ -4723,7 +4821,7 @@ function SourceControlInner(): React.JSX.Element { // without changing local HEAD, so the HEAD-change effect alone misses it. const current = { ahead: remoteStatus?.ahead ?? null, - baseRef: effectiveBaseRef, + baseRef: compareBaseRef, behind: remoteStatus?.behind ?? null, hasUpstream: remoteStatus?.hasUpstream ?? null, upstreamName: remoteStatus?.upstreamName ?? null, @@ -4736,7 +4834,7 @@ function SourceControlInner(): React.JSX.Element { } }, [ activeWorktreeId, - effectiveBaseRef, + compareBaseRef, isBranchVisible, isFolder, remoteStatus?.ahead, @@ -4747,7 +4845,7 @@ function SourceControlInner(): React.JSX.Element { ]) useEffect(() => { - if (!activeWorktreeId || !worktreePath || !isBranchVisible || !effectiveBaseRef || isFolder) { + if (!activeWorktreeId || !worktreePath || !isBranchVisible || !compareBaseRef || isFolder) { return } @@ -4757,7 +4855,21 @@ 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 the compare-base policy resolves to no base, runBranchCompare + // bails out; drop any stale summary so the + // committed-changes section and "vs" row disappear and only the working tree + // 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, remoteStatus]) useEffect(() => { // Why: history shells out to git. Defer the first load until the user @@ -4767,8 +4879,10 @@ function SourceControlInner(): React.JSX.Element { } void refreshGitHistoryRef.current() }, [ + // Why: history is fetched with compareBaseRef, so re-run when the upstream + // compare base changes — effectiveBaseRef can stay put while it moves. activeWorktreeId, - effectiveBaseRef, + compareBaseRef, isBranchVisible, isFolder, isGitHistoryExpanded, @@ -5299,7 +5413,7 @@ function SourceControlInner(): React.JSX.Element { diffCommentCount={diffCommentCount} onExpandNotes={() => setDiffCommentsExpanded(true)} branchSummary={branchSummary} - compareBaseRef={effectiveBaseRef} + compareBaseRef={compareBaseRef} upstreamStatus={remoteStatus} /> @@ -5785,6 +5899,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: expandedSubmoduleKeys.has( + getSubmoduleExpansionKey(node.entry) + ), + onToggle: () => toggleSubmodule(node.entry) + } + : 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: expandedSubmoduleKeys.has( + getSubmoduleExpansionKey(entry) + ), + onToggle: () => toggleSubmodule(entry) + } + : undefined return ( ) }))} @@ -7784,6 +7938,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, @@ -7801,7 +7986,8 @@ const UncommittedEntryRow = React.memo(function UncommittedEntryRow({ onUnstage, onDiscard, commentCount, - showPathHint = true + showPathHint = true, + submoduleExpansion }: { entryKey: string entry: GitStatusEntry @@ -7820,13 +8006,15 @@ 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) 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) @@ -7843,12 +8031,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.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 ( { + 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 + } if (onSelect) { onSelect(e, entryKey, entry) } else { @@ -7897,9 +8093,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-entry-actions.test.ts b/src/renderer/src/components/right-sidebar/source-control-entry-actions.test.ts new file mode 100644 index 0000000000..f9f3bcba20 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control-entry-actions.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from 'vitest' +import type { GitStatusEntry } from '../../../../shared/types' +import { + canDiscardStatusEntry, + canStageStatusEntry, + canUnstageStatusEntry +} from './source-control-entry-actions' + +function entry(overrides: Partial): 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/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..f6de3f25a3 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control-submodule-expansion.test.ts @@ -0,0 +1,445 @@ +import { describe, expect, it } from 'vitest' +import type { GitStatusEntry } from '../../../../shared/types' +import { + buildSubmoduleChildNodes, + collectListSelectionEntries, + getSubmoduleExpansionKey, + injectExpandedSubmoduleEntries, + injectExpandedSubmoduleRows, + isExpandableSubmoduleEntry, + type SubmoduleSectionTreeNode, + type SubmoduleStatusState +} from './source-control-submodule-expansion' + +const LOADING = 'Loading submodule changes…' +const EMPTY = 'No changes in submodule' +const FLUTTER_KEY = 'unstaged::flutter_mine' + +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', + 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('renamed') + 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_KEY]), {}, 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_KEY]: { status: 'error', error: 'boom' } + } + const result = injectExpandedSubmoduleRows( + [node], + new Set([FLUTTER_KEY]), + 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_KEY]: { status: 'loaded', entries: [] } + } + const result = injectExpandedSubmoduleRows( + [node], + new Set([FLUTTER_KEY]), + 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_KEY]: { + status: 'loaded', + entries: [{ path: 'lib/main.dart', status: 'modified', area: 'unstaged' }] + } + } + const result = injectExpandedSubmoduleRows( + [node], + new Set([FLUTTER_KEY]), + 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('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({ + path: 'flutter_mine', + submodule: { commitChanged: true, trackedChanges: false, untrackedChanges: false } + }) + ) + const statuses: Record = { + [FLUTTER_KEY]: { + status: 'loaded', + entries: [{ path: 'lib/main.dart', status: 'modified', area: 'unstaged' }] + } + } + const result = injectExpandedSubmoduleRows( + [node], + new Set([FLUTTER_KEY]), + 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(['unstaged::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_KEY]), + {}, + 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_KEY]: { + status: 'loaded', + entries: [{ path: 'lib/main.dart', status: 'modified', area: 'unstaged' }] + } + } + const result = injectExpandedSubmoduleEntries( + [entry], + new Set([FLUTTER_KEY]), + 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('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 = { + [FLUTTER_KEY]: { status: 'loaded', entries: [] } + } + const result = injectExpandedSubmoduleEntries( + [entry], + new Set([FLUTTER_KEY]), + statuses, + LOADING, + EMPTY + ) + expect(result[1]).toMatchObject({ + type: 'submodule-placeholder', + state: 'empty', + message: EMPTY + }) + }) +}) + +describe('collectListSelectionEntries', () => { + it('includes injected submodule child entries and skips placeholders', () => { + const entry = submoduleEntry({ + path: 'flutter_mine', + submodule: { commitChanged: true, trackedChanges: false, untrackedChanges: false } + }) + const statuses: Record = { + [FLUTTER_KEY]: { + status: 'loaded', + entries: [{ path: 'lib/main.dart', status: 'modified', area: 'unstaged' }] + } + } + const rows = injectExpandedSubmoduleEntries( + [entry], + new Set([FLUTTER_KEY]), + statuses, + LOADING, + EMPTY + ) + + // The expanded submodule's child file must be selectable, mirroring the rows + // that actually render in list view. + expect(collectListSelectionEntries(rows)).toEqual([ + { key: 'unstaged::flutter_mine', entry, area: 'unstaged' }, + { + key: 'unstaged::flutter_mine/lib/main.dart', + entry: { + path: 'flutter_mine/lib/main.dart', + status: 'modified', + area: 'unstaged', + submoduleRoot: 'flutter_mine' + }, + area: 'unstaged' + } + ]) + }) + + it('drops loading placeholders from selection entries', () => { + const entry = submoduleEntry({ path: 'flutter_mine' }) + const rows = injectExpandedSubmoduleEntries([entry], new Set([FLUTTER_KEY]), {}, LOADING, EMPTY) + + expect(collectListSelectionEntries(rows)).toEqual([ + { key: 'unstaged::flutter_mine', entry, area: 'unstaged' } + ]) + }) +}) + +describe('getSubmoduleExpansionKey', () => { + it('separates staged and unstaged rows for the same submodule path', () => { + expect(getSubmoduleExpansionKey(submoduleEntry({ path: 'flutter_mine', area: 'staged' }))).toBe( + 'staged::flutter_mine' + ) + expect( + getSubmoduleExpansionKey(submoduleEntry({ path: 'flutter_mine', area: 'unstaged' })) + ).toBe(FLUTTER_KEY) + }) + + it('keeps staged children in the staged area for diff routing', () => { + const entry = submoduleEntry({ + path: 'flutter_mine', + area: 'staged', + submodule: { commitChanged: true, trackedChanges: false, untrackedChanges: false } + }) + const result = injectExpandedSubmoduleEntries( + [entry], + new Set(['staged::flutter_mine']), + { + 'staged::flutter_mine': { + status: 'loaded', + entries: [{ path: 'lib/main.dart', status: 'modified', area: 'unstaged' }] + } + }, + LOADING, + EMPTY + ) + + expect(result[1]).toMatchObject({ + type: 'entry', + entry: { + path: 'flutter_mine/lib/main.dart', + area: 'staged', + submoduleRoot: 'flutter_mine' + } + }) + }) +}) 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..0cd0ca92ca --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control-submodule-expansion.ts @@ -0,0 +1,267 @@ +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' +import type { FlatEntry } from './useSourceControlSelection' + +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 } + +export function getSubmoduleExpansionKey(entry: Pick): string { + return `${entry.area}::${entry.path}` +} + +export function parseSubmoduleExpansionKey( + key: string +): { area: GitStatusEntry['area']; path: string } | null { + const separatorIndex = key.indexOf('::') + if (separatorIndex <= 0) { + return null + } + const area = key.slice(0, separatorIndex) + if (area !== 'staged' && area !== 'unstaged' && area !== 'untracked') { + return null + } + const path = key.slice(separatorIndex + 2) + return path ? { area, path } : null +} + +/** + * 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). 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}`, + ...(innerEntry.oldPath ? { oldPath: `${submodulePath}/${innerEntry.oldPath}` } : {}), + area, + 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, parent.entry.area) + return { + type: 'file', + key: `${childEntry.area}::${childEntry.path}`, + name: basename(childEntry.path), + path: childEntry.path, + entry: childEntry, + area: childEntry.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[], + expandedSubmoduleKeys: ReadonlySet, + submoduleStatusByKey: Readonly>, + loadingMessage: string, + emptyMessage: string +): RenderableSubmoduleListItem[] { + const result: RenderableSubmoduleListItem[] = [] + for (const entry of entries) { + result.push({ type: 'entry', entry }) + const expansionKey = getSubmoduleExpansionKey(entry) + if (!isExpandableSubmoduleEntry(entry) || !expandedSubmoduleKeys.has(expansionKey)) { + continue + } + const submodulePath = entry.path + const state = submoduleStatusByKey[expansionKey] + 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, entry.area) + }) + } + } + return result +} + +/** + * Map injected list rows to selection entries. List-view selection/range/open-key + * bookkeeping must read the same submodule-injected rows it renders; otherwise an + * expanded submodule's child files render with handlers but stay unselectable. + * Placeholders are skipped because they are not selectable. + */ +export function collectListSelectionEntries( + rows: readonly RenderableSubmoduleListItem[] +): FlatEntry[] { + const result: FlatEntry[] = [] + for (const row of rows) { + if (row.type === 'entry') { + result.push({ + key: `${row.entry.area}::${row.entry.path}`, + entry: row.entry, + area: row.entry.area + }) + } + } + 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[], + expandedSubmoduleKeys: ReadonlySet, + submoduleStatusByKey: Readonly>, + loadingMessage: string, + emptyMessage: string +): RenderableSourceControlNode[] { + const result: RenderableSourceControlNode[] = [] + for (const node of nodes) { + result.push(node) + if ( + node.type !== 'file' || + !isExpandableSubmoduleEntry(node.entry) || + !expandedSubmoduleKeys.has(getSubmoduleExpansionKey(node.entry)) + ) { + continue + } + const submodulePath = node.entry.path + const state = submoduleStatusByKey[getSubmoduleExpansionKey(node.entry)] + 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/components/right-sidebar/useSourceControlSubmoduleStatus.test.tsx b/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.test.tsx new file mode 100644 index 0000000000..6df07a3684 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.test.tsx @@ -0,0 +1,259 @@ +// @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, + settings = null +}: { + worktreeId: string + worktreePath: string + entries: GitStatusEntry[] + settings?: { activeRuntimeEnvironmentId: string | null } | null +}): null { + latest = useSourceControlSubmoduleStatus({ + activeWorktreeId: worktreeId, + worktreePath, + activeRepoSettings: settings, + entries + }) + return null +} + +function innerEntry(path: string): GitStatusEntry { + return { path, status: 'modified', area: 'unstaged' } as GitStatusEntry +} + +function submoduleEntry(area: GitStatusEntry['area'] = 'unstaged'): GitStatusEntry { + return { + path: 'sub', + status: 'modified', + area, + submodule: { commitChanged: true, trackedChanges: false, untrackedChanges: false } + } 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(submoduleEntry()) + }) + await flush() + + // Switch to worktree B (same submodule path) and expand it there. + await act(async () => { + root.render() + }) + await act(async () => { + latest?.toggleSubmodule(submoduleEntry()) + }) + 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?.submoduleStatusByKey['unstaged::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(submoduleEntry()) + }) + await flush() + + await act(async () => { + root.render() + }) + await act(async () => { + latest?.toggleSubmodule(submoduleEntry()) + }) + 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?.submoduleStatusByKey['unstaged::sub']).toEqual({ + status: 'loaded', + entries: [innerEntry('from-b.ts')] + }) + }) + + 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')] }) + + const container = document.createElement('div') + const root = createRoot(container) + roots.push(root) + const stagedEntry = submoduleEntry('staged') + + await act(async () => { + root.render() + }) + await act(async () => { + latest?.toggleSubmodule(stagedEntry) + }) + await flush() + + expect(mocks.getRuntimeGitSubmoduleStatus).toHaveBeenCalledWith( + expect.objectContaining({ worktreeId: 'A', worktreePath: '/a' }), + 'sub', + 'staged' + ) + expect(latest?.submoduleStatusByKey['staged::sub']).toEqual({ + status: 'loaded', + 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 new file mode 100644 index 0000000000..971fbe7602 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.ts @@ -0,0 +1,136 @@ +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 { + getSubmoduleExpansionKey, + isExpandableSubmoduleEntry, + parseSubmoduleExpansionKey, + 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 = { + expandedSubmoduleKeys: Set + submoduleStatusByKey: Record + toggleSubmodule: (entry: Pick) => 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 [expandedSubmoduleKeys, setExpandedSubmoduleKeys] = useState>(() => new Set()) + 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/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({}) + }, [activeConnectionRouteKey, activeRuntimeRouteKey, activeWorktreeId, worktreePath]) + + const fetchSubmoduleStatus = useCallback( + async (expansionKey: string): Promise => { + if (!worktreePath) { + return + } + const parsed = parseSubmoduleExpansionKey(expansionKey) + if (!parsed) { + return + } + const { area, path: submodulePath } = parsed + 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. + setSubmoduleStatusByKey((prev) => + prev[expansionKey] ? prev : { ...prev, [expansionKey]: { 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, + area + ) + if (generationRef.current !== generation) { + return + } + setSubmoduleStatusByKey((prev) => ({ + ...prev, + [expansionKey]: { status: 'loaded', entries: result.entries } + })) + } catch (error) { + if (generationRef.current !== generation) { + return + } + setSubmoduleStatusByKey((prev) => ({ + ...prev, + [expansionKey]: { + status: 'error', + error: error instanceof Error ? error.message : String(error) + } + })) + } + }, + [activeRepoSettings, activeWorktreeId, worktreePath] + ) + + const toggleSubmodule = useCallback((entry: Pick) => { + const expansionKey = getSubmoduleExpansionKey(entry) + setExpandedSubmoduleKeys((prev) => { + const next = new Set(prev) + if (next.has(expansionKey)) { + next.delete(expansionKey) + } else { + next.add(expansionKey) + } + 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(() => { + const visibleExpandableKeys = new Set( + entries.filter(isExpandableSubmoduleEntry).map(getSubmoduleExpansionKey) + ) + for (const expansionKey of expandedSubmoduleKeys) { + if (visibleExpandableKeys.has(expansionKey)) { + void fetchSubmoduleStatus(expansionKey) + } + } + }, [expandedSubmoduleKeys, entries, fetchSubmoduleStatus]) + + return { expandedSubmoduleKeys, submoduleStatusByKey, toggleSubmodule } +} diff --git a/src/renderer/src/components/settings/CompareAgainstUpstreamSetting.tsx b/src/renderer/src/components/settings/CompareAgainstUpstreamSetting.tsx new file mode 100644 index 0000000000..0fb266867b --- /dev/null +++ b/src/renderer/src/components/settings/CompareAgainstUpstreamSetting.tsx @@ -0,0 +1,103 @@ +import type { GlobalSettings } from '../../../../shared/types' +import { translate } from '@/i18n/i18n' +import { SearchableSetting } from './SearchableSetting' +import { SettingsRow, SettingsSegmentedControl } from './SettingsFormControls' +import { matchesSettingsSearch } from './settings-search' + +type SourceControlCompareBasePolicy = 'repository-default' | 'branch-upstream' + +export const COMPARE_AGAINST_UPSTREAM_KEYWORDS = [ + 'compare base', + 'default compare base', + 'default branch', + 'repository default', + 'branch upstream', + 'current branch', + 'upstream', + 'local changes', + 'origin/master', + 'committed changes', + 'diff base', + 'source control' +] + +function getCompareAgainstUpstreamTitle(): string { + return translate( + 'auto.components.settings.GitPane.compareAgainstUpstreamTitle', + 'Default Compare Base' + ) +} + +function getCompareAgainstUpstreamDescription(): string { + return translate( + 'auto.components.settings.GitPane.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." + ) +} + +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() + const value: SourceControlCompareBasePolicy = settings.sourceControlCompareAgainstUpstream + ? 'branch-upstream' + : 'repository-default' + + return ( + + + value={value} + onChange={(nextValue) => { + if (nextValue !== value) { + void updateSettings({ + sourceControlCompareAgainstUpstream: nextValue === 'branch-upstream' + }) + } + }} + ariaLabel={title} + size="sm" + options={[ + { + value: 'repository-default', + label: translate( + 'auto.components.settings.GitPane.compareBaseRepositoryDefault', + 'Repository default' + ) + }, + { + value: 'branch-upstream', + label: translate( + 'auto.components.settings.GitPane.compareBaseBranchUpstream', + 'Branch upstream' + ) + } + ]} + /> + } + /> + + ) +} diff --git a/src/renderer/src/components/settings/GitPane.test.ts b/src/renderer/src/components/settings/GitPane.test.ts index ab2ef19c1b..3275eb9a18 100644 --- a/src/renderer/src/components/settings/GitPane.test.ts +++ b/src/renderer/src/components/settings/GitPane.test.ts @@ -15,6 +15,7 @@ import { import { TooltipProvider } from '../ui/tooltip' import { matchesSettingsSearch } from './settings-search' import { SettingsSegmentedControl } from './SettingsFormControls' +import { CompareAgainstUpstreamSetting } from './CompareAgainstUpstreamSetting' type ReactElementLike = { type: unknown @@ -56,6 +57,23 @@ function findSegmentedControl(node: unknown): ReactElementLike { return found } +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 === SettingsSegmentedControl && entry.props.ariaLabel === label) { + found = entry + } + }) + if (!found) { + throw new Error('compare-base segmented control not found') + } + return found +} + function renderGitPane(searchQuery: string): string { useAppStore.setState({ settingsSearchQuery: searchQuery }) return renderToStaticMarkup( @@ -163,4 +181,88 @@ describe('GitPane', () => { expect(matchesSettingsSearch('staged', getGitPaneSearchEntries())).toBe(true) expect(matchesSettingsSearch('group order', getGitPaneSearchEntries())).toBe(true) }) + + it('renders the default compare base setting in Git settings', () => { + const markup = renderGitPane('compare base') + + expect(markup).toContain( + translate( + 'auto.components.settings.GitPane.compareAgainstUpstreamTitle', + '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 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 default compare base policy in its own segmented control', () => { + const repositoryDefaultControl = findCompareBaseSegmentedControl( + CompareAgainstUpstreamSetting({ + settings: { + ...getDefaultSettings(os.homedir()), + sourceControlCompareAgainstUpstream: false + }, + updateSettings: () => {} + }) + ) + expect(repositoryDefaultControl.props.value).toBe('repository-default') + + const branchUpstreamControl = findCompareBaseSegmentedControl( + CompareAgainstUpstreamSetting({ + settings: { + ...getDefaultSettings(os.homedir()), + sourceControlCompareAgainstUpstream: true + }, + updateSettings: () => {} + }) + ) + expect(branchUpstreamControl.props.value).toBe('branch-upstream') + }) + + it('updates the default compare base policy from its segmented control', () => { + const updateSettings = vi.fn() + const repositoryDefaultControl = findCompareBaseSegmentedControl( + CompareAgainstUpstreamSetting({ + settings: { + ...getDefaultSettings(os.homedir()), + sourceControlCompareAgainstUpstream: false + }, + updateSettings + }) + ) + ;(repositoryDefaultControl.props.onChange as (value: string) => void)('branch-upstream') + expect(updateSettings).toHaveBeenCalledWith({ sourceControlCompareAgainstUpstream: true }) + + updateSettings.mockClear() + const branchUpstreamControl = findCompareBaseSegmentedControl( + CompareAgainstUpstreamSetting({ + settings: { + ...getDefaultSettings(os.homedir()), + sourceControlCompareAgainstUpstream: true + }, + updateSettings + }) + ) + ;(branchUpstreamControl.props.onChange as (value: string) => void)('repository-default') + expect(updateSettings).toHaveBeenCalledWith({ sourceControlCompareAgainstUpstream: false }) + }) }) 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', + 'Default Compare Base' + ), + description: translate( + 'auto.components.settings.git.search.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." + ), + 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' + ), + ...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' + ), + ...translateSearchKeyword('auto.components.settings.git.search.diffBase', 'diff base'), + ...translateSearchKeyword( + 'auto.components.settings.git.search.sourceControl', + 'source control' + ) + ] + }, ...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 5a4f04e6e1..9a69ddcaff 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -5227,7 +5227,11 @@ "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": "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", @@ -7362,7 +7366,19 @@ "stagedFirst": "staged first", "untrackedFirst": "untracked first", "sourceControl": "source control", - "gitChanges": "git changes" + "gitChanges": "git changes", + "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", + "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 0b5d62fa80..e8c9c4de92 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -5190,7 +5190,11 @@ "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": "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", @@ -7325,7 +7329,19 @@ "stagedFirst": "cambios preparados primero", "untrackedFirst": "sin seguimiento primero", "sourceControl": "control de código fuente", - "gitChanges": "cambios de git" + "gitChanges": "cambios de git", + "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", + "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 b6de9e836d..1958ab3267 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -5212,7 +5212,11 @@ "sourceControlGroupOrderDescription": "ソース管理で「変更」「ステージ済みの変更」「未追跡ファイル」のどれを先に表示するかを選択します。", "changesFirst": "変更を先頭", "stagedFirst": "ステージ済みを先頭", - "untrackedFirst": "未追跡を先頭" + "untrackedFirst": "未追跡を先頭", + "compareAgainstUpstreamTitle": "既定の比較ベース", + "compareAgainstUpstreamDescription": "ソース管理がコミット済み変更の比較に既定で使うベースを選択します。ブランチの上流は現在のブランチに自動的に追従し、上流がない場合はリポジトリの既定ブランチにフォールバックします。比較ベースは各 worktree の Git パネルから worktree ごとに変更できます。Pull Request やリベースの対象は変更されません。", + "compareBaseRepositoryDefault": "リポジトリ既定", + "compareBaseBranchUpstream": "ブランチの上流" }, "HiddenExperimentalGroup": { "d0f914a528": "プレースホルダーの切り替え", @@ -7347,7 +7351,19 @@ "stagedFirst": "ステージ済みを先頭", "untrackedFirst": "未追跡を先頭", "sourceControl": "ソース管理", - "gitChanges": "git の変更" + "gitChanges": "git の変更", + "compareAgainstUpstreamTitle": "既定の比較ベース", + "compareAgainstUpstreamDescription": "ソース管理がコミット済み変更の比較に既定で使うベースを選択します。ブランチの上流は現在のブランチに自動的に追従し、上流がない場合はリポジトリの既定ブランチにフォールバックします。比較ベースは各 worktree の Git パネルから worktree ごとに変更できます。Pull Request やリベースの対象は変更されません。", + "compareBase": "比較基準", + "currentBranch": "現在のブランチ", + "upstream": "上流", + "localChanges": "ローカルの変更", + "originMaster": "origin/master", + "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 888eb9e193..aa5b4dc53e 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -5175,7 +5175,11 @@ "sourceControlGroupOrderDescription": "소스 제어에서 변경 사항, 스테이징된 변경 사항 또는 추적되지 않는 파일 중 무엇을 먼저 표시할지 선택합니다.", "changesFirst": "변경 사항 먼저", "stagedFirst": "스테이징된 항목 먼저", - "untrackedFirst": "추적되지 않는 파일 먼저" + "untrackedFirst": "추적되지 않는 파일 먼저", + "compareAgainstUpstreamTitle": "기본 비교 기준", + "compareAgainstUpstreamDescription": "소스 제어가 커밋된 변경 사항 비교에 기본으로 사용할 기준을 선택합니다. 브랜치 업스트림은 현재 브랜치를 자동으로 따라가며, 업스트림이 없으면 저장소 기본 브랜치로 돌아갑니다. 비교 기준은 해당 worktree의 Git 패널에서 worktree별로 변경할 수 있습니다. Pull Request 및 리베이스 대상은 변경되지 않습니다.", + "compareBaseRepositoryDefault": "저장소 기본값", + "compareBaseBranchUpstream": "브랜치 업스트림" }, "HiddenExperimentalGroup": { "d0f914a528": "자리 표시자 토글", @@ -7310,7 +7314,19 @@ "stagedFirst": "스테이징된 항목 먼저", "untrackedFirst": "추적되지 않는 파일 먼저", "sourceControl": "소스 제어", - "gitChanges": "git 변경 사항" + "gitChanges": "git 변경 사항", + "compareAgainstUpstreamTitle": "기본 비교 기준", + "compareAgainstUpstreamDescription": "소스 제어가 커밋된 변경 사항 비교에 기본으로 사용할 기준을 선택합니다. 브랜치 업스트림은 현재 브랜치를 자동으로 따라가며, 업스트림이 없으면 저장소 기본 브랜치로 돌아갑니다. 비교 기준은 해당 worktree의 Git 패널에서 worktree별로 변경할 수 있습니다. Pull Request 및 리베이스 대상은 변경되지 않습니다.", + "compareBase": "비교 기준", + "currentBranch": "현재 브랜치", + "upstream": "업스트림", + "localChanges": "로컬 변경 사항", + "originMaster": "origin/master", + "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 5f3fa0b1d0..497addb6f1 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -5175,7 +5175,11 @@ "sourceControlGroupOrderDescription": "选择在源代码管理中优先显示“更改”、“已暂存更改”还是“未跟踪文件”。", "changesFirst": "更改优先", "stagedFirst": "已暂存优先", - "untrackedFirst": "未跟踪优先" + "untrackedFirst": "未跟踪优先", + "compareAgainstUpstreamTitle": "默认对比基准", + "compareAgainstUpstreamDescription": "选择源代码管理在比较已提交变更时默认使用的基准。分支上游会自动跟随当前分支；如果没有上游，则回退到仓库默认分支。你仍然可以在对应 worktree 的 Git 面板中按 worktree 更改对比基准。Pull Request 和变基目标不会改变。", + "compareBaseRepositoryDefault": "仓库默认", + "compareBaseBranchUpstream": "分支上游" }, "HiddenExperimentalGroup": { "d0f914a528": "占位符切换", @@ -7310,7 +7314,19 @@ "stagedFirst": "已暂存优先", "untrackedFirst": "未跟踪优先", "sourceControl": "源代码管理", - "gitChanges": "git 更改" + "gitChanges": "git 更改", + "compareAgainstUpstreamTitle": "默认对比基准", + "compareAgainstUpstreamDescription": "选择源代码管理在比较已提交变更时默认使用的基准。分支上游会自动跟随当前分支；如果没有上游，则回退到仓库默认分支。你仍然可以在对应 worktree 的 Git 面板中按 worktree 更改对比基准。Pull Request 和变基目标不会改变。", + "compareBase": "对比基准", + "currentBranch": "当前分支", + "upstream": "上游", + "localChanges": "本地变更", + "originMaster": "origin/master", + "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 b0795718ac..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 @@ -148,6 +149,32 @@ export async function getRuntimeGitStatus( ) } +export async function getRuntimeGitSubmoduleStatus( + context: RuntimeGitContext, + 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, + area + }) + } + return callRuntimeRpc( + target, + 'git.submoduleStatus', + { + worktree: toRuntimeWorktreeSelector(context.worktreeId), + submodulePath, + area + }, + { timeoutMs: 15_000 } + ) +} + export async function getRuntimeGitIgnoredPaths( context: RuntimeGitContext, paths: string[] diff --git a/src/renderer/src/store/slices/editor.ts b/src/renderer/src/store/slices/editor.ts index 92d0371f68..6fd0ef6b9a 100644 --- a/src/renderer/src/store/slices/editor.ts +++ b/src/renderer/src/store/slices/editor.ts @@ -658,6 +658,7 @@ export type EditorSlice = { requestKey: string, result: { summary: GitBranchCompareSummary; entries: GitBranchChangeEntry[] } ) => void + clearGitBranchCompare: (worktreeId: string) => void // File search state fileSearchStateByWorktree: Record< @@ -3870,6 +3871,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/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index b80b50fb38..2fecb65037 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -1483,6 +1483,14 @@ function createGitApi(): NonNullable['git']> { includeIgnored }) }, + submoduleStatus: async ({ worktreePath, submodulePath, area }) => { + const worktree = await resolveRuntimeWorktreeByPath(worktreePath) + return callRuntimeResult('git.submoduleStatus', { + worktree: toRuntimeWorktreeSelector(worktree.id), + submodulePath, + area + }) + }, checkIgnored: async ({ worktreePath, paths }) => { const worktree = await resolveRuntimeWorktreeByPath(worktreePath) return callRuntimeResult('git.checkIgnored', { diff --git a/src/shared/constants.ts b/src/shared/constants.ts index 4b03a57271..82886ceef7 100644 --- a/src/shared/constants.ts +++ b/src/shared/constants.ts @@ -274,6 +274,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/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. diff --git a/src/shared/types.ts b/src/shared/types.ts index b927e00289..156436c99c 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -2577,6 +2577,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