Support submodule diffs and upstream-base compares in Source Control#6350
Support submodule diffs and upstream-base compares in Source Control#6350lvfen wants to merge 7 commits into
Conversation
Dirty submodules now expand inline in Source Control to reveal their inner changes, with file-level diffs that are read-only from the parent worktree. Inner status is fetched lazily only when a submodule is expanded, so status polling never recurses into (possibly nested) submodules. Adds a submodule-status path across local and SSH runtimes and git providers.
Adds a global setting (default off) that defaults the Source Control compare base to the current branch's upstream so the panel prioritizes local changes instead of the full delta versus the repository default branch. When the branch has no upstream, the compare view falls back to working-tree-only. This affects only the compare/diff view; the Pull Request and rebase merge target are unchanged.
…ion gates Moves the lazy submodule-expansion state into a useSourceControlSubmoduleStatus hook and centralizes per-row stage/unstage/discard eligibility into source-control-entry-actions, shrinking SourceControl.tsx and keeping the read-only submodule rules consistent across the row UI, bulk actions, and tests. The hook adds a generation guard so a slow submodule-status response from a previous worktree (common over SSH) can't write stale status into the current panel. On the relay side, configured submodule paths are read through a short-TTL per-instance cache so a burst of diff clicks does not re-read .gitmodules over the SSH link. Adds tests for the new modules.
📝 WalkthroughWalkthroughThe PR adds a new 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/main/providers/ssh-git-provider.ts (1)
98-103: 🩺 Stability & Availability | 🔵 TrivialAdd an older-relay guard to
getSubmoduleStatus.The
git.submoduleStatusRPC call lacks anisJsonRpcMethodNotFoundErrorcheck. If a new client connects to an older hosted relay, the error propagates to the UI. Currently, the consuming hook (useSourceControlSubmoduleStatus) simply displays the raw error message (e.g., "method not found"), which is confusing.Align this wrapper with existing patterns (e.g.,
getCloneStatus,getWorktreeIsClean): catch the specific incompatibility error and re-throw a user-friendly message instructing the user to reconnect or updateorcaon the host.src/main/providers/ssh-git-provider.ts:98-103
async getSubmoduleStatus(worktreePath: string, submodulePath: string): Promise<GitStatusResult> { return (await this.mux.request('git.submoduleStatus', { worktreePath, submodulePath })) as GitStatusResult }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3a9f8c69-001e-4415-8c62-7e80a55e0a84
📒 Files selected for processing (44)
src/main/codex-accounts/runtime-home-service.test.tssrc/main/codex-accounts/service.test.tssrc/main/git/status.test.tssrc/main/git/status.tssrc/main/ipc/filesystem.tssrc/main/providers/ssh-git-provider.tssrc/main/providers/types.tssrc/main/runtime/orca-runtime-git.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/rpc/methods/git-params.tssrc/main/runtime/rpc/methods/git.tssrc/preload/api-types.tssrc/preload/index.tssrc/relay/git-handler-submodule-ops.test.tssrc/relay/git-handler-submodule-ops.tssrc/relay/git-handler.test.tssrc/relay/git-handler.tssrc/renderer/src/components/right-sidebar/SourceControl.compare-summary.test.tssrc/renderer/src/components/right-sidebar/SourceControl.open-file-highlight.test.tsxsrc/renderer/src/components/right-sidebar/SourceControl.preview-open.test.tsxsrc/renderer/src/components/right-sidebar/SourceControl.tsxsrc/renderer/src/components/right-sidebar/discard-all-sequence.test.tssrc/renderer/src/components/right-sidebar/discard-all-sequence.tssrc/renderer/src/components/right-sidebar/source-control-entry-actions.test.tssrc/renderer/src/components/right-sidebar/source-control-entry-actions.tssrc/renderer/src/components/right-sidebar/source-control-submodule-expansion.test.tssrc/renderer/src/components/right-sidebar/source-control-submodule-expansion.tssrc/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.test.tsxsrc/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.tssrc/renderer/src/components/settings/CompareAgainstUpstreamSetting.tsxsrc/renderer/src/components/settings/GitPane.test.tssrc/renderer/src/components/settings/GitPane.tsxsrc/renderer/src/components/settings/git-search.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.jsonsrc/renderer/src/runtime/runtime-git-client.tssrc/renderer/src/store/slices/editor.tssrc/renderer/src/web/web-preload-api.tssrc/shared/constants.tssrc/shared/git-status-types.tssrc/shared/types.ts
- Degrade git.submoduleStatus to an actionable reconnect hint when an older SSH relay lacks the RPC, mirroring clone()/worktreeIsClean fallbacks. - Keep the branch-compare summary while upstream status is still loading so it no longer flickers when switching worktrees with prefer-upstream on. - Mark the compare-base switch as type="button" to avoid form submission. - Add diff base / source control keywords to the Git settings search catalog. - Assert the compare-base toggle's own switch state and updateSettings call.
…e-support # Conflicts: # src/main/git/status.test.ts # src/main/git/status.ts # src/relay/git-handler.ts
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/renderer/src/components/right-sidebar/SourceControl.tsx (2)
1716-1728: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep list-view selection in sync with injected submodule rows.
Line 1716 renders injected list rows, but list-mode selection still uses pre-injection
flatEntriesat Line 1741. Expanded submodule child files can render with row handlers but stay absent from selection/range/open-key bookkeeping.Proposed fix
const visibleSelectionEntries = useMemo(() => { if (sourceControlViewMode === 'list') { - return flatEntries + const arr: FlatEntry[] = [] + for (const section of displaySections) { + if (collapsedSections.has(section.id)) { + continue + } + for (const row of visibleListRowsBySection[section.id] ?? []) { + if (row.type === 'entry') { + arr.push({ + key: `${row.entry.area}::${row.entry.path}`, + entry: row.entry, + area: row.entry.area + }) + } + } + } + return arr } const arr: FlatEntry[] = []Also update the memo dependencies:
- flatEntries, sourceControlViewMode, + visibleListRowsBySection, visibleTreeRowsBySection
4777-4785: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRefresh commit history when the compare base changes.
Line 4683 fetches history with
compareBaseRef, but this effect still depends oneffectiveBaseRef. If only the upstream compare base changes, the visible history can remain stale until another dependency changes.Proposed fix
}, [ activeWorktreeId, - effectiveBaseRef, + compareBaseRef, isBranchVisible, isFolder, isGitHistoryExpanded,src/main/git/status.ts (1)
1069-1074: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse the validated submodule path resolver here, not raw
path.join().
matchedSubmoduleultimately comes from.gitmodules, so joining it directly bypasses the new worktree-boundary validation added earlier in this PR. A crafted submodule path can make inner diff reads escape the selected repo. Please route this through the existingresolveSubmoduleWorktreePath(...)guard before readingHEADor recursing intogetDiff().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 722a0521-426d-423a-bf28-42e89c639f81
📒 Files selected for processing (16)
src/main/codex-accounts/runtime-home-service.test.tssrc/main/codex-accounts/service.test.tssrc/main/git/status.test.tssrc/main/git/status.tssrc/main/providers/ssh-git-provider.test.tssrc/main/providers/ssh-git-provider.tssrc/main/runtime/orca-runtime.tssrc/preload/api-types.tssrc/preload/index.tssrc/relay/git-handler.test.tssrc/relay/git-handler.tssrc/renderer/src/components/right-sidebar/SourceControl.compare-summary.test.tssrc/renderer/src/components/right-sidebar/SourceControl.tsxsrc/renderer/src/components/settings/CompareAgainstUpstreamSetting.tsxsrc/renderer/src/components/settings/GitPane.test.tssrc/renderer/src/components/settings/git-search.ts
💤 Files with no reviewable changes (3)
- src/renderer/src/components/settings/git-search.ts
- src/renderer/src/components/settings/CompareAgainstUpstreamSetting.tsx
- src/renderer/src/components/settings/GitPane.test.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- src/preload/index.ts
- src/preload/api-types.ts
- src/main/runtime/orca-runtime.ts
- src/main/codex-accounts/service.test.ts
- src/renderer/src/components/right-sidebar/SourceControl.compare-summary.test.ts
- src/main/codex-accounts/runtime-home-service.test.ts
- src/main/git/status.test.ts
- src/relay/git-handler.test.ts
- src/relay/git-handler.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/renderer/src/components/right-sidebar/SourceControl.tsx (2)
1716-1728: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep list-view selection in sync with injected submodule rows.
Line 1716 renders injected list rows, but list-mode selection still uses pre-injection
flatEntriesat Line 1741. Expanded submodule child files can render with row handlers but stay absent from selection/range/open-key bookkeeping.Proposed fix
const visibleSelectionEntries = useMemo(() => { if (sourceControlViewMode === 'list') { - return flatEntries + const arr: FlatEntry[] = [] + for (const section of displaySections) { + if (collapsedSections.has(section.id)) { + continue + } + for (const row of visibleListRowsBySection[section.id] ?? []) { + if (row.type === 'entry') { + arr.push({ + key: `${row.entry.area}::${row.entry.path}`, + entry: row.entry, + area: row.entry.area + }) + } + } + } + return arr } const arr: FlatEntry[] = []Also update the memo dependencies:
- flatEntries, sourceControlViewMode, + visibleListRowsBySection, visibleTreeRowsBySection
4777-4785: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRefresh commit history when the compare base changes.
Line 4683 fetches history with
compareBaseRef, but this effect still depends oneffectiveBaseRef. If only the upstream compare base changes, the visible history can remain stale until another dependency changes.Proposed fix
}, [ activeWorktreeId, - effectiveBaseRef, + compareBaseRef, isBranchVisible, isFolder, isGitHistoryExpanded,src/main/git/status.ts (1)
1069-1074: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse the validated submodule path resolver here, not raw
path.join().
matchedSubmoduleultimately comes from.gitmodules, so joining it directly bypasses the new worktree-boundary validation added earlier in this PR. A crafted submodule path can make inner diff reads escape the selected repo. Please route this through the existingresolveSubmoduleWorktreePath(...)guard before readingHEADor recursing intogetDiff().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 722a0521-426d-423a-bf28-42e89c639f81
📒 Files selected for processing (16)
src/main/codex-accounts/runtime-home-service.test.tssrc/main/codex-accounts/service.test.tssrc/main/git/status.test.tssrc/main/git/status.tssrc/main/providers/ssh-git-provider.test.tssrc/main/providers/ssh-git-provider.tssrc/main/runtime/orca-runtime.tssrc/preload/api-types.tssrc/preload/index.tssrc/relay/git-handler.test.tssrc/relay/git-handler.tssrc/renderer/src/components/right-sidebar/SourceControl.compare-summary.test.tssrc/renderer/src/components/right-sidebar/SourceControl.tsxsrc/renderer/src/components/settings/CompareAgainstUpstreamSetting.tsxsrc/renderer/src/components/settings/GitPane.test.tssrc/renderer/src/components/settings/git-search.ts
💤 Files with no reviewable changes (3)
- src/renderer/src/components/settings/git-search.ts
- src/renderer/src/components/settings/CompareAgainstUpstreamSetting.tsx
- src/renderer/src/components/settings/GitPane.test.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- src/preload/index.ts
- src/preload/api-types.ts
- src/main/runtime/orca-runtime.ts
- src/main/codex-accounts/service.test.ts
- src/renderer/src/components/right-sidebar/SourceControl.compare-summary.test.ts
- src/main/codex-accounts/runtime-home-service.test.ts
- src/main/git/status.test.ts
- src/relay/git-handler.test.ts
- src/relay/git-handler.ts
🛑 Comments failed to post (2)
src/main/git/status.ts (1)
73-91: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Invalidate
statusReadsInFlighton mutations too.The new
getStatus()coalescing can now return stale results after a mutation: ifstageFile()/commitChanges()starts while a matching status read is still in flight, the nextgetStatus()joins that pre-mutation promise because onlygitDiffReadDedupeis cleared.statusReadsInFlightneeds to be cleared in the same mutation invalidation path.Suggested direction
+function clearGitReadInvalidationState(): void { + gitDiffReadDedupe.clear() + statusReadsInFlight.clear() +} + export async function stageFile( worktreePath: string, filePath: string, options: GitRuntimeOptions = {} ): Promise<void> { - gitDiffReadDedupe.clear() + clearGitReadInvalidationState() try { await gitExecFileAsync( ['add', '--', literalPathspec(filePath)], gitOptionsForWorktree(worktreePath, options) ) } finally { - gitDiffReadDedupe.clear() + clearGitReadInvalidationState() } }Also applies to: 1727-1740, 1751-1758, 1814-1838, 1849-1882, 1951-1990, 2015-2028, 2040-2053
src/main/providers/ssh-git-provider.ts (1)
113-130: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make
getSubmoduleStatus()clear diff dedupe likegetStatus().
getStatus()already clearsgitDiffReadDedupebefore refreshing repo state so later diff reads can’t join stale in-flight work. The new SSH submodule-status path skips that step, so after a submodule refresh the next diff can still reuse an older pending RPC.
…edback - Route submodule inner diffs through resolveSubmoduleWorktreePath so a crafted .gitmodules path can't escape the selected worktree - Clear statusReadsInFlight alongside the diff dedupe on git mutations so a post-mutation getStatus() can't join a stale in-flight read - Clear the SSH diff dedupe in getSubmoduleStatus to mirror getStatus - Derive list-view selection from the submodule-injected rows so expanded submodule children are selectable - Refresh commit history when the upstream compare base changes
|
Thanks for the thorough review! All actionable comments from the latest round have been addressed in commit 6888dcb. Details below.
Fixed. loadDiff() now validates the matched submodule before any inner read: as soon as findContainingSubmodule() returns a match, we call
Fixed. Introduced a single clearGitReadInvalidationState() helper that clears both gitDiffReadDedupe and statusReadsInFlight, and routed every
Fixed. getSubmoduleStatus() now calls this.gitDiffReadDedupe.clear() before issuing the RPC, mirroring getStatus(). After a submodule status refresh,
Fixed. In list mode, visibleSelectionEntries now derives from the submodule-injected visibleListRowsBySection instead of the pre-injection
Fixed. The history effect that calls refreshGitHistoryRef.current() now depends on compareBaseRef instead of effectiveBaseRef. Since history is Verification: pnpm typecheck, pnpm lint, and the affected test suites all pass (status, ssh-git-provider, and the full right-sidebar suite — 985 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/git/status.ts (1)
1084-1098: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the correct commit endpoints for staged and compare-against-HEAD inner submodule diffs.
The root pointer diff handles
stagedandcompareAgainstHead, but inner file diffs always compareindex || HEADto the submodule workingHEAD. For staged submodule pointer changes, this can show the wrong range; for compare-against-HEAD, it can incorrectly use the index as the left side.Proposed fix
- const fromOid = - (await readGitlinkOidFromIndex(worktreePath, matchedSubmodule, options)) || - (await readGitlinkOidFromTree(worktreePath, 'HEAD', matchedSubmodule, options)) - const toOid = await readWorkingSubmoduleHead(submoduleWorktreePath, options) + const headOid = await readGitlinkOidFromTree(worktreePath, 'HEAD', matchedSubmodule, options) + const indexOid = await readGitlinkOidFromIndex(worktreePath, matchedSubmodule, options) + const workingHeadOid = await readWorkingSubmoduleHead(submoduleWorktreePath, options) + const fromOid = staged || compareAgainstHead ? headOid : indexOid || headOid + const toOid = staged ? indexOid : workingHeadOid
🧹 Nitpick comments (1)
src/renderer/src/components/right-sidebar/SourceControl.tsx (1)
4318-4323: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared unstage eligibility helper for bulk selection.
bulkUnstagePathscurrently reimplements only part of the row/action eligibility rule. Switching this tocanUnstageStatusEntry(entry.entry)keeps the bulk bar aligned with per-row unstage behavior if more read-only cases are added.♻️ Proposed refactor
const bulkUnstagePaths = useMemo( () => selectedEntries - // Why: submodule-internal rows are read-only from the parent worktree. - .filter((entry) => entry.area === 'staged' && !entry.entry.submoduleRoot) + .filter((entry) => canUnstageStatusEntry(entry.entry)) .map((entry) => entry.entry.path), [selectedEntries] )
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0d92cd5a-b7e4-491d-b7fc-b8b3cba6419d
📒 Files selected for processing (7)
src/main/git/status.test.tssrc/main/git/status.tssrc/main/providers/ssh-git-provider.test.tssrc/main/providers/ssh-git-provider.tssrc/renderer/src/components/right-sidebar/SourceControl.tsxsrc/renderer/src/components/right-sidebar/source-control-submodule-expansion.test.tssrc/renderer/src/components/right-sidebar/source-control-submodule-expansion.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/providers/ssh-git-provider.test.ts
- src/main/providers/ssh-git-provider.ts
- src/renderer/src/components/right-sidebar/source-control-submodule-expansion.ts
- src/main/git/status.test.ts
- Support expanding and diffing staged submodule changes (HEAD vs index) independently of unstaged changes (index vs worktree). - Track submodule expansion states using a compound key of area and path to prevent conflicts between staged and unstaged listings. - Update the compare-against-upstream setting to a segmented control for the "Default Compare Base" policy. - Fall back to the repository default branch when comparing a branch with no upstream, preventing comparison views from unexpectedly disappearing.
|
感谢PR！I reviewed and changed some design around the "staged" schema (I directly pushed my commit) will finish the rest of the review tmr (California time) |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/renderer/src/i18n/locales/en.json (1)
7275-7286: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the missing
diffBasecatalog key.
src/renderer/src/components/settings/git-search.tsLine 117 now looks upauto.components.settings.git.search.diffBase, but thisgit.searchblock never defines it. English falls back to"diff base", yet the key will be missing from catalog sync and other locale files will drift from the rest of the compare-base search terms.Suggested fix
"branchUpstream": "branch upstream", + "diffBase": "diff base", "currentBranch": "current branch", "upstream": "upstream", "localChanges": "local changes",Based on learnings, feature PRs should keep new locale keys in catalog parity via
pnpm run sync:localization-catalog.Source: Learnings
src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.ts (1)
55-105: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard same-key fetches against out-of-order responses.
This only rejects replies after a worktree/path switch. Because the effect below refetches every expanded key whenever
entriesrefresh, two requests for the sameexpansionKeycan overlap and an older slower response can overwrite newer child rows after a poll or mutation refresh.Suggested fix
+ const requestSeqRef = useRef<Record<string, number>>({}) + useEffect(() => { generationRef.current += 1 + requestSeqRef.current = {} setExpandedSubmoduleKeys(new Set()) setSubmoduleStatusByKey({}) }, [activeWorktreeId, worktreePath]) ... const { area, path: submodulePath } = parsed const generation = generationRef.current + const requestSeq = (requestSeqRef.current[expansionKey] ?? 0) + 1 + requestSeqRef.current[expansionKey] = requestSeq ... - if (generationRef.current !== generation) { + if ( + generationRef.current !== generation || + requestSeqRef.current[expansionKey] !== requestSeq + ) { return } ... - if (generationRef.current !== generation) { + if ( + generationRef.current !== generation || + requestSeqRef.current[expansionKey] !== requestSeq + ) { return }
🧹 Nitpick comments (1)
src/renderer/src/components/settings/CompareAgainstUpstreamSetting.tsx (1)
24-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare the compare-base metadata from one source.
The title, description, and keywords are duplicated here and in
src/renderer/src/components/settings/git-search.tsLines 73-123. That duplication already caused the search path to drift once; exporting one shared metadata helper/constants would keep the rendered setting and the search catalog in lockstep.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2b662e07-8c52-48ec-b98c-f0cf6d426d0e
📒 Files selected for processing (36)
src/main/git/status-porcelain-parser.test.tssrc/main/git/status-porcelain-parser.tssrc/main/git/status.test.tssrc/main/git/status.tssrc/main/ipc/filesystem.tssrc/main/providers/ssh-git-provider.test.tssrc/main/providers/ssh-git-provider.tssrc/main/providers/types.tssrc/main/runtime/orca-runtime-git.tssrc/main/runtime/rpc/methods/git-params.tssrc/main/runtime/rpc/methods/git.test.tssrc/main/runtime/rpc/methods/git.tssrc/preload/api-types.tssrc/preload/index.tssrc/relay/git-handler-submodule-ops.tssrc/relay/git-handler.test.tssrc/relay/git-handler.tssrc/relay/git-status-output-parser.test.tssrc/relay/git-status-output-parser.tssrc/renderer/src/components/right-sidebar/SourceControl.compare-summary.test.tssrc/renderer/src/components/right-sidebar/SourceControl.tsxsrc/renderer/src/components/right-sidebar/source-control-submodule-expansion.test.tssrc/renderer/src/components/right-sidebar/source-control-submodule-expansion.tssrc/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.test.tsxsrc/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.tssrc/renderer/src/components/settings/CompareAgainstUpstreamSetting.tsxsrc/renderer/src/components/settings/GitPane.test.tssrc/renderer/src/components/settings/git-search.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.jsonsrc/renderer/src/runtime/runtime-git-client.test.tssrc/renderer/src/runtime/runtime-git-client.tssrc/renderer/src/web/web-preload-api.ts
✅ Files skipped from review due to trivial changes (2)
- src/renderer/src/i18n/locales/ko.json
- src/renderer/src/i18n/locales/ja.json
🚧 Files skipped from review as they are similar to previous changes (20)
- src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.test.tsx
- src/main/runtime/rpc/methods/git-params.ts
- src/preload/api-types.ts
- src/main/providers/types.ts
- src/main/providers/ssh-git-provider.ts
- src/main/runtime/rpc/methods/git.ts
- src/renderer/src/runtime/runtime-git-client.ts
- src/renderer/src/components/right-sidebar/SourceControl.compare-summary.test.ts
- src/renderer/src/web/web-preload-api.ts
- src/main/ipc/filesystem.ts
- src/renderer/src/components/right-sidebar/source-control-submodule-expansion.test.ts
- src/preload/index.ts
- src/main/providers/ssh-git-provider.test.ts
- src/renderer/src/components/right-sidebar/source-control-submodule-expansion.ts
- src/relay/git-handler.test.ts
- src/relay/git-handler.ts
- src/relay/git-handler-submodule-ops.ts
- src/main/git/status.test.ts
- src/main/git/status.ts
- src/renderer/src/components/right-sidebar/SourceControl.tsx
AmethystLiang
requested review from
AmethystLiang
and removed request for
brennanb2025
3 participants
Summary
Source Control now supports expanding dirty submodules inline so projects that use git submodules can inspect submodule file diffs directly from the
parent repository. Previously, submodule changes could not be viewed in detail, which made submodule-heavy projects harder to review and iterate on.
This also adds a setting to compare changes against the current branch's upstream base instead of always defaulting to
main. For branch-baseddevelopment, this keeps the diff focused on local branch work, avoids noisy unrelated changes, and removes the need to reset the compare base every
time the project is opened.
The submodule diff path is lazy-loaded and read-only from the parent worktree, with support across local and SSH runtimes.
Screenshots
Testing
pnpm lintpnpm typecheckpnpm testpnpm buildAdded or updated high-quality tests that would catch regressions, or explained why tests were not needed
AI Review Report
Reviewed with an AI coding agent. Risks checked:
git.submoduleStatus; now degrades to an actionable reconnect message viaisJsonRpcMethodNotFoundError(mirrorsclone()/worktreeIsClean), instead of surfacing a raw JSON-RPC method-not-found.remoteStatus === undefined);clearing only happens once upstream is loaded and no base resolves.
diff baseandsource control; the compare-base toggle test asserts the specificswitch and its
updateSettingscall instead of scanning full markup.Cross-platform: changes are platform-agnostic TypeScript (renderer effects, settings UI, SSH provider RPC forwarding). No
path/shell/shortcut/Electron platform-specific code was added; behavior is identical on macOS, Linux, and Windows. SSH hosted-runtime path was
specifically considered (the relay compatibility fix).
Security Audit
submodulePath/worktreePathare forwarded to existing relay RPCs unchanged; no new shell construction or path concatenationintroduced.
Notes
No additional platform-specific risks. Compare-base behavior is opt-in via the existing setting and only affects the compare view.