Skip to content

fix(source-control): 修复 submodule 分支切换出现 HEAD 及不实时刷新的问题#339

Merged
J3n5en merged 2 commits intoJ3n5en:mainfrom
lwt-sadais:fix/submodule-branch-checkout
Mar 5, 2026
Merged

fix(source-control): 修复 submodule 分支切换出现 HEAD 及不实时刷新的问题#339
J3n5en merged 2 commits intoJ3n5en:mainfrom
lwt-sadais:fix/submodule-branch-checkout

Conversation

@lwt-sadais
Copy link
Contributor

问题描述

版本管理面板中切换 submodule 分支存在两个 bug:

  1. 切换后显示 HEAD:选择远程分支(如 remotes/origin/dev)后,submodule 进入 detached HEAD 状态,分支名显示为 HEAD
  2. 切换后不实时刷新:分支切换成功后 UI 需要等待约 10 秒轮询才显示新分支名

根因分析

Bug 1 — detached HEAD

GitService.checkout() 对远程分支名的处理不完整:

// 修复前
"remotes/origin/dev".slice(8) → "origin/dev"
git checkout origin/dev  → detached HEAD ❌

// 修复后
"remotes/origin/dev" → localBranch="dev", remoteBranch="origin/dev"
git checkout dev              (已有本地分支)✅
git checkout -b dev --track origin/dev  (本地不存在时创建)✅

checkoutSubmoduleBranch() 完全没有 normalize 逻辑,同样修复。

Bug 2 — 缓存 key 不匹配

handleBranchCheckout 对 submodule 误用了 useGitCheckout,其 onSuccess 以 submodule 绝对路径失效缓存,但 useSubmodules 的 queryKey 是主仓库路径,两者永不匹配,只能靠 10s 轮询补救。

修复方案

src/main/services/git/GitService.ts

  • checkout()checkoutSubmoduleBranch() 统一处理 remotes/ 前缀:提取本地分支名,try checkout / catch 创建追踪分支

src/renderer/components/source-control/SourceControlPanel.tsx

  • handleBranchCheckout 根据 submodule 列表判断目标仓库类型
  • submodule 改用 useCheckoutSubmoduleBranch,其 onSuccessrootPath 正确失效 ['git', 'submodules', rootPath] 缓存,切换后立即刷新

测试方式

  1. 打开含 submodule 的仓库,进入版本管理面板
  2. 选中 submodule,点击分支切换器,选择一个远程分支(如 remotes/origin/xxx
  3. 验证:切换后显示本地分支名而非 HEAD,且分支名立即更新无需等待

- GitService.checkout/checkoutSubmoduleBranch: 切换远程分支时提取本地分支名,
  优先切换已有本地分支,不存在则通过 -b --track 创建追踪分支,避免进入 detached HEAD
- SourceControlPanel: submodule 分支切换改用 useCheckoutSubmoduleBranch,
  使 onSuccess 以 rootPath 为 key 正确失效缓存,切换后立即刷新分支名显示

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

This comment has been minimized.

- 提取重复的远程分支处理逻辑为私有方法 checkoutRemoteBranch
- 改进错误处理:区分分支不存在和其他错误(如未提交变更),避免掩盖真实异常
- 修复 Toast 提示:显示实际切换到的本地分支名而非原始远程分支名

解决 PR J3n5en#339 代码审查中提出的 3 个问题
@github-actions
Copy link
Contributor

github-actions bot commented Mar 4, 2026

Claude finished @lwt-sadais's task in 2m 4s —— View job


🔍 代码审查报告

👋 欢迎查看审查结果! 本次审查已完成,以下是详细分析和建议。
💡 使用提示:点击 📁 文件链接可直接跳转到代码位置,建议按优先级处理问题。

📊 审查概览 · 一目了然

📋 审查项目 🔢 发现数量 📈 占比 🎯 处理状态
📁 审查文件 2 100% 已完成
🚨 严重问题 0 0%
⚠️ 中等问题 1 33% 🟡 本版本修复
轻微问题 2 67% 🔵 后续优化

🏆 代码质量评分

📊 综合评分: 85/100  🌟🌟🌟🌟⭐
📋 本次等级: 良好

📋 问题分布

🚨 严重: ░░░░░░░░░░  0%
⚠️ 中等: ███░░░░░░░ 33%
⚡ 轻微: ███████░░░ 67%
总体评分: ⭐⭐⭐⭐☆ (4/5)

🎯 关键发现

🏷️ 类别 🔍 发现数量 🎯 优先级 📈 趋势
🔒 安全性 0 个 ✅ 无
⚡ 性能 0 个 ✅ 无
🎨 代码质量 1 个 🟡 中 📊 平稳
🛠️ 健壮性 2 个 🔵 低 📊 平稳

✅ 亮点

本次 PR 解决了两个明确的 bug,修复思路清晰,代码组织良好:

  1. 提取公共方法 checkoutRemoteBranchcheckout()checkoutSubmoduleBranch() 共享远程分支处理逻辑,避免了代码重复,设计合理。
  2. 缓存失效策略修正 — submodule 分支切换改用 useCheckoutSubmoduleBranch,其 onSuccess 正确使用 rootPath 作为 queryKey 来失效 ['git', 'submodules', rootPath] 缓存,这是问题的根因修复。
  3. Toast 显示优化 — 对 remotes/origin/dev 格式的分支名提取为 dev 显示,用户体验良好。

🎯 问题详细分析 · 📊 点击展开查看

📊 **👆 点击此处展开详细统计和图表分析**

📈 问题趋势分析

本次审查: 🟡🔵🔵⚪⚪ (发现 3 个问题)
代码质量: ████████░░ 85%
安全评分: ██████████ 100% ✅
性能评分: ██████████ 100% ✅
🏆 质量指标 📊 当前分数 🎯 目标分数 📈 改进方向
代码质量 85/100 90+ ⬆️ 小幅改进
安全等级 100/100 100 ✅ 优秀
性能优化 100/100 100 ✅ 优秀

🐛 问题清单与修复指南 · 📋 点击展开查看

📋 **👆 点击此处展开详细问题列表 (共 3 个问题) · 一键复制追踪 📌**

🚀 快速开始

  • 📍 点击文件路径可直接跳转到问题代码
  • ⏰ 建议按 P1 → P2 优先级顺序处理
  • 📝 每个问题都包含具体修复方案

⚠️ 中等级别 (Major) 建议修复

🟡 #1 src/main/services/git/GitService.ts:1131-1132

🏷️ 类型: 功能缺陷 | ⏰ 优先级: P1 - 本版本修复 | 🎯 影响范围: 分支切换

  • 🔍 问题描述:当远程分支名包含多层路径时(如 remotes/origin/feature/login),localBranch 提取逻辑只会取第一个 / 之后的部分,结果为 feature/login,这在大多数场景是正确的。但需要注意 remoteBranch 的提取同样使用 slice(8) 硬编码偏移,这假定前缀始终是 remotes/(8 个字符)——目前调用方通过 branch.startsWith('remotes/') 已保证了这一点,所以功能上是安全的。

    然而,真正的问题是:如果本地已有同名分支但追踪的是不同的远程(如本地 dev 追踪 origin/dev,用户选择切换到 remotes/upstream/dev),checkoutRemoteBranch 会直接 checkout 到本地 dev,不会切换到 upstream/dev 的追踪分支。

// src/main/services/git/GitService.ts:1128-1152
private async checkoutRemoteBranch(git: SimpleGit, branch: string): Promise<void> {
    const remoteBranch = branch.slice(8);
    const slashIdx = remoteBranch.indexOf('/');
    const localBranch = slashIdx >= 0 ? remoteBranch.slice(slashIdx + 1) : remoteBranch;

    try {
      // If local "dev" exists but tracks origin/dev,
      // and user wants upstream/dev — this silently switches to origin/dev's local branch
      await git.checkout(localBranch);
    } catch (error) {
      // ...
    }
}
  • 🛠️ 解决方案:这是一个边缘场景(多 remote 的 submodule 较少见),当前修复已覆盖主要用例。如需完善,可在 checkout 成功后检查本地分支的上游是否匹配预期 remote,若不匹配则创建带 remote 前缀的本地分支名(如 upstream-dev)。可作为后续优化。

  • ⚠️ 潜在影响:多 remote 环境下切换到错误的追踪分支


⚡ 轻微级别 (Minor) 优化建议

🔵 #2 src/main/services/git/GitService.ts:1130

🏷️ 类型: 代码优化 | ⏰ 优先级: P2 - 后续版本 | 🎯 影响范围: 局部

  • 🔍 问题描述branch.slice(8) 中的 8'remotes/' 的长度硬编码。虽有注释说明,但建议用常量或 'remotes/'.length 替代魔法数字,提升可读性。
// src/main/services/git/GitService.ts:1130
const remoteBranch = branch.slice(8);
  • 🛠️ 解决方案
const REMOTES_PREFIX = 'remotes/';
const remoteBranch = branch.slice(REMOTES_PREFIX.length);

🔵 #3 src/renderer/components/source-control/SourceControlPanel.tsx:419-424

🏷️ 类型: 代码优化 | ⏰ 优先级: P2 - 后续版本 | 🎯 影响范围: 局部

  • 🔍 问题描述:Toast 中的分支名 normalize 逻辑与后端 checkoutRemoteBranch 中的逻辑相同(从 remotes/origin/xxx 提取本地分支名),存在轻微重复。另外前端的提取方式 branch.slice(branch.indexOf('/', 8) + 1) 和后端的 remoteBranch.slice(slashIdx + 1) 虽然结果一致,但写法不同,后续维护需注意保持同步。
// src/renderer/components/source-control/SourceControlPanel.tsx:419-421
const displayBranch = branch.startsWith('remotes/')
  ? branch.slice(branch.indexOf('/', 8) + 1)
  : branch;
  • 🛠️ 解决方案:可以考虑将分支名 normalize 提取为共享工具函数(如 normalizeRemoteBranch),供前后端统一使用。这是可选的优化,当前实现功能正确。

📊 修复优先级矩阵

🎯 优先级 🚨 严重 ⚠️ 中等 ⚡ 轻微 📈 总计
🔴 P0 0 个 - - 0 个
🟡 P1 - 1 个 - 1 个
🔵 P2 - - 2 个 2 个
📊 合计 0 1 2 3 个

🎯 合并决策建议

📋 合并评估结果

📊 评估维度 ⭐ 得分 🎯 状态 💭 说明
🔐 安全性 10/10 ✅ 通过 无安全风险,仅涉及 git 分支操作
📊 代码质量 8/10 ✅ 优秀 结构清晰,公共方法提取合理,注释规范
⚡ 性能影响 10/10 ✅ 无影响 仅修改分支切换逻辑,无额外性能开销
🛠️ 功能完整性 8/10 ✅ 完整 修复了两个明确 bug,多 remote 边缘场景可后续优化
🧪 测试覆盖 6/10 ⚠️ 基本 无自动化测试覆盖,需手动验证

📈 综合评分: 42/50 分 · 等级: 良好


🚦 最终建议

建议合并

📝 详细理由

  • 🟢 修复思路正确,准确定位了 detached HEAD 和缓存 key 不匹配两个根因
  • 🟢 提取 checkoutRemoteBranch 公共方法,消除了代码重复
  • 🟢 useCheckoutSubmoduleBranchonSuccess 正确失效 ['git', 'submodules', workdir]['git', 'submodule', 'branches', ...] 缓存
  • 🟢 无安全或性能问题
  • 🟡 多 remote 边缘场景为已知限制,不影响当前主流用例

🎯 具体行动建议

  1. 本版本可选改进

    • 考虑将 slice(8) 替换为 slice('remotes/'.length) 提升可读性
  2. 后续版本改进

    • 补充 checkoutRemoteBranch 的单元测试(正常切换、新建追踪分支、多层路径分支名)
    • 考虑多 remote 场景下的分支名冲突处理
    • 提取分支名 normalize 为共享工具函数

@J3n5en J3n5en merged commit d0101cf into J3n5en:main Mar 5, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants