feat(web-shell): show current git branch in composer toolbar - #6725
Conversation
|
Thanks for the PR! Template looks good ✓ Problem: This addresses a real feature request (#6702) — showing the current git branch in the Web Shell composer toolbar. The issue has a clear rationale: branch awareness is important for coding agents, especially with worktrees and feature branches. Not a bug fix, so no reproduction needed. Direction: Aligned. A read-only branch indicator is a small, well-scoped UI addition. The Codex desktop client reference in the issue validates the feature direction. CHANGELOG has no direct reference but the area (Web Shell workspace context) is actively being developed (#6699). Size: 351 production lines + 275 test lines across 4 packages (cli, sdk-typescript, web-shell, webui). Cross-package but each touch is minimal and purpose-specific. Under the 500-line threshold. No core infrastructure paths ( Approach: The scope feels right. Each layer does the minimum needed:
One minor note: the Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：这个 PR 对应一个明确的 feature request (#6702) — 在 Web Shell composer toolbar 中显示当前 git 分支。Issue 中有清晰的理由：分支感知对编码 agent 很重要，特别是在使用 worktree 和功能分支时。不是 bug fix，不需要复现。 方向：对齐。只读分支指示器是一个小而范围明确的 UI 新增。Issue 中引用的 Codex 桌面客户端验证了这个方向。CHANGELOG 无直接参考，但该领域（Web Shell workspace 上下文）正在积极开发中（#6699）。 规模：4 个包（cli, sdk-typescript, web-shell, webui）中 351 行生产代码 + 275 行测试代码。跨包但每处改动都很小且目的明确。未达到 500 行阈值。未修改核心基础设施路径。 方案：范围合理。每层都做了最少的必要工作：后端用现有 git 工具函数新建路由和状态类，SDK 加薄封装的客户端方法，UI 用简单的只读 一个小备注： 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewReviewed the full diff across 21 files. The implementation is clean and well-structured. Backend ( SDK plumbing: The Event handling ( UI component: Uses a semantic Session provider: Git status is fetched in both the initial connect and reconnect paths via No critical blockers or AGENTS.md violations found. Test ResultsAll new and existing tests pass: Real-Scenario Testing (backend)Started the daemon via Daemon startup + git routeGit route responseBranch name correctly returned for a git workspace. Route responds in <5ms. Branch switch (watcher update)After Web Shell UIFull Web Shell UI testing requires a browser with an active ACP session — not feasible in this tmux-only CI environment. The author also noted this limitation in the PR body. — Qwen Code · qwen3.7-max |
ReflectionThis PR implements a small, well-defined feature from issue #6702: a read-only git branch indicator in the Web Shell composer toolbar. The implementation closely matches what I would have proposed independently — a backend route with lazy-cached state and file watching, thin SDK client methods, a minimal React component, and proper session-level event handling. The code quality is solid throughout. Each of the four packages touched does the minimum necessary work. No drive-by refactors, no scope creep. The All 315 tests pass (68 core git + 5 CLI routes/state + 14 webui mappers + 2 web-shell component + 226 SDK client). The backend route responds correctly in live testing with sub-5ms latency. The watcher-based branch update didn't fire in this CI environment after a The one testing gap is the full Web Shell UI — it requires a browser with an active ACP session, which neither this CI environment nor the author's Windows setup could provide. The component renders correctly in jsdom tests, and the backend plumbing is verified end-to-end, so the remaining gap is narrow (CSS layout and visual integration in a real browser). This is a clean, focused PR that ships exactly what was requested. Ready to merge. 中文说明总结这个 PR 实现了 #6702 中明确定义的功能：在 Web Shell composer toolbar 中添加只读的 git 分支指示器。实现方式与我的独立方案高度一致——后端路由带懒缓存状态和文件监听、薄封装的 SDK 客户端方法、最小化的 React 组件、以及正确的 session 级事件处理。 代码质量始终很扎实。涉及的四个包都只做了最必要的改动，没有夹带重构或范围蔓延。 全部 315 个测试通过。后端路由在实测中正确响应，延迟在 5ms 以下。CI 环境中 唯一的测试缺口是完整的 Web Shell UI——需要浏览器和已连接的 ACP session，这个 CI 环境和作者的 Windows 环境都无法提供。组件在 jsdom 测试中正确渲染，后端管道已端到端验证，所以剩余缺口很窄（真实浏览器中的 CSS 布局和视觉集成）。 这是一个干净、聚焦的 PR，准确交付了所请求的功能。可以合并。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
| activeSession.context(), | ||
| ]); | ||
| const gitPromise = client | ||
| .workspaceByCwd(activeSession.workspaceCwd) |
There was a problem hiding this comment.
[Critical] The reconnect path calls client.workspaceByCwd(activeSession.workspaceCwd).workspaceGit() without guarding for undefined. activeSession.workspaceCwd is string | undefined (types.ts:60), and workspaceByCwd expects string. When it is undefined, encodeURIComponent produces "undefined", yielding the malformed URL /workspaces/undefined/git. The error is caught by Promise.allSettled, so it won't crash, but the git branch will silently fail to populate on reconnect.
By contrast, the initial connection path (line 461) has an explicit guard:
effectWorkspaceCwd
? client.workspaceByCwd(effectWorkspaceCwd).workspaceGit()
: client.workspaceGit()| .workspaceByCwd(activeSession.workspaceCwd) | |
| const gitPromise = activeSession.workspaceCwd | |
| ? client.workspaceByCwd(activeSession.workspaceCwd).workspaceGit() | |
| : client.workspaceGit(); |
— qwen3.7-max via Qwen Code /review
| supportedCommands: supportedCommands ?? current.supportedCommands, | ||
| context: context ?? current.context, | ||
| gitBranch: | ||
| gitResult.status === 'fulfilled' ? gitBranch : undefined, |
There was a problem hiding this comment.
[Suggestion] When the git fetch is rejected, gitBranch is unconditionally set to undefined, discarding the previously known branch. Every other metadata field in this same state update preserves its current value on failure using value ?? current.fieldName:
providers: providers ?? current.providers,
supportedCommands: supportedCommands ?? current.supportedCommands,
context: context ?? current.context,
capabilities: capabilities ?? current.capabilities,Only gitBranch is wiped. A transient network blip during reconnect will clear the displayed branch, causing the indicator to flicker off and back on.
| gitResult.status === 'fulfilled' ? gitBranch : undefined, | |
| gitBranch: | |
| gitResult.status === 'fulfilled' ? gitBranch : current.gitBranch, |
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
| } | ||
|
|
||
| /** Current Git branch metadata returned from a workspace Git status route. */ | ||
| export interface DaemonWorkspaceGitStatus { |
There was a problem hiding this comment.
[Suggestion] DaemonWorkspaceGitStatus is defined here but not re-exported from the barrel files (packages/sdk-typescript/src/daemon/index.ts and packages/sdk-typescript/src/index.ts). Other workspace status types (e.g. DaemonWorkspaceMcpStatus, DaemonWorkspaceAgentsStatus) are re-exported from both barrels. External consumers of @qwen-code/sdk-typescript who call client.workspaceGit() cannot type-annotate the return value from the public API surface.
| export interface DaemonWorkspaceGitStatus { | |
| export interface DaemonWorkspaceGitStatus { |
Add export type { DaemonWorkspaceGitStatus } to packages/sdk-typescript/src/daemon/index.ts and re-export from the root barrel as well.
— qwen3.7-max via Qwen Code /review
|
|
||
| const pending = this.createEntry(workspaceCwd, bridge).catch((error) => { | ||
| this.entries.delete(workspaceCwd); | ||
| throw error; |
There was a problem hiding this comment.
[Suggestion] The .catch handler in getOrCreateEntry deletes the failed entry from the map, enabling retry on subsequent getStatus calls. No test verifies this retry-after-failure path. If the cleanup logic were incorrect, the entry would remain as a rejected promise and all future calls for that workspace would permanently fail.
Consider adding a test where resolveBranchName rejects on the first call and succeeds on the second:
it('retries after a failed createEntry', async () => {
resolveBranchName
.mockRejectedValueOnce(new Error('git not found'))
.mockResolvedValueOnce('main');
await expect(state.getStatus(cwd, bridge)).rejects.toThrow();
const result = await state.getStatus(cwd, bridge);
expect(result.branch).toBe('main');
});— qwen3.7-max via Qwen Code /review
|
Thanks for the review. I pushed
Verified locally: |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
| gitState: WorkspaceGitState; | ||
| }, | ||
| ): void { | ||
| app.get('/workspace/git', async (_req, res) => { |
There was a problem hiding this comment.
[Suggestion] Both route handlers (/workspace/git and /workspaces/:workspace/git) omit the try/catch + sendBridgeError envelope that every other workspace status route uses (e.g., workspace-status.ts, workspace-tools.ts). If getStatus rejects, Express returns a generic 500 instead of the structured JSON error body that clients expect.
| app.get('/workspace/git', async (_req, res) => { | |
| import { sendBridgeError } from '../server/error-response.js'; | |
| // ... | |
| registerWorkspaceGitRoutes({ | |
| // ... | |
| router: router.get('/workspace/git', async (req, res) => { | |
| try { | |
| const result = await deps.gitState.getStatus(deps.workspaceCwd, deps.bridge); | |
| res.json(result ?? { branch: null }); | |
| } catch (err) { | |
| sendBridgeError(res, err, { route: 'GET /workspace/git' }); | |
| } | |
| }), | |
| }); |
The same pattern should be applied to the workspace-qualified route. Consider also adding a test that mocks getStatus to reject and asserts the error response shape.
— qwen3.7-max via Qwen Code /review
| v: 1 as const, | ||
| workspaceCwd: secondary.workspaceCwd, | ||
| branch: 'feature/web-shell', | ||
| })); |
There was a problem hiding this comment.
[Suggestion] No test covers the workspace-not-found path (e.g., GET /workspaces/nonexistent/git). The resolveWorkspaceRuntimeFromParam utility correctly sends a 400 with workspace_mismatch when the workspace doesn't exist, but this behavior is never exercised for the new git route. Adding a test with a nonexistent workspace ID would guard against future changes to the resolution chain.
— qwen3.7-max via Qwen Code /review
| @@ -274,6 +282,16 @@ const sdkMocks = vi.hoisted(() => { | |||
| channelLive: true, | |||
There was a problem hiding this comment.
[Suggestion] The workspaceGit mock is configured to resolve { branch: 'main' }, but no test asserts that connection.gitBranch equals 'main' after initial connect or reconnect. The integration of git status into DaemonConnectionState — via both the initial-connect and reconnect Promise.allSettled paths — is untested at the provider level. Adding an assertion on connection.gitBranch after the mock resolves would verify the wiring.
— qwen3.7-max via Qwen Code /review
| </div> | ||
| )} | ||
| <div className={styles.toolbarLeft}> | ||
| {gitBranch && showToolbarAction('gitBranch') && ( |
There was a problem hiding this comment.
[Suggestion] The conditional rendering {gitBranch && showToolbarAction('gitBranch') && <GitBranchIndicator ...>} has no integration test. The GitBranchIndicator component itself is unit-tested, but there's no test verifying that it appears when gitBranch is set and the toolbar action is active, or that it's hidden when either condition is false.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
| workspaceCwd: string, | ||
| bridge: AcpSessionBridge, | ||
| ): Promise<WorkspaceGitStatus> { | ||
| const entry = await this.getOrCreateEntry(workspaceCwd, bridge); |
There was a problem hiding this comment.
[Suggestion] The bridge parameter is captured in the watcher closure during the first getStatus call (via getOrCreateEntry), but subsequent calls with a different bridge silently reuse the cached entry. If the workspace bridge rotates on session reconnect, the watcher continues publishing git_branch_changed events to the stale bridge.
Consider storing the bridge on the entry and updating it on each getStatus call, or resolving it dynamically from the registry at publish time. At minimum, a comment documenting this capture behavior would prevent future confusion.
— qwen3.7-max via Qwen Code /review
| }); | ||
| }; | ||
| entry.dispose = await watchRepoBranch(workspaceCwd, () => { | ||
| void refresh().catch(() => {}); |
There was a problem hiding this comment.
[Suggestion] Two related issues in the watcher refresh logic:
entry.branchis updated (line 65) beforepublishWorkspaceEvent(line 67). If publish throws, the branch is already mutated, and the next watcher fire hits thebranch === entry.branchdedup check and returns without re-publishing — the UI stays stuck on the old branch..catch(() => {})swallows all errors with zero observability. IfresolveBranchNameor the publish call fails, there's no log, no counter, no signal.
| void refresh().catch(() => {}); | |
| const refresh = async () => { | |
| const branch = await resolveBranchName(workspaceCwd); | |
| if (branch === entry.branch) return; | |
| bridge.publishWorkspaceEvent({ | |
| type: 'git_branch_changed', | |
| data: { workspaceCwd, branch: branch ?? null }, | |
| }); | |
| entry.branch = branch; | |
| }; | |
| entry.dispose = await watchRepoBranch(workspaceCwd, () => { | |
| void refresh().catch((err) => { | |
| console.debug('git branch refresh failed for %s: %s', workspaceCwd, err); | |
| }); | |
| }); |
— qwen3.7-max via Qwen Code /review
| // Carries `currentModelId` and `currentApprovalMode` so reconnecting | ||
| // clients can seed their reducer without an extra round-trip. | ||
| 'session_snapshot', | ||
| 'git_branch_changed', |
There was a problem hiding this comment.
[Suggestion] git_branch_changed is added to DAEMON_KNOWN_EVENT_TYPE_VALUES but has no corresponding case in asKnownDaemonEvent and no typed interface (e.g., DaemonGitBranchChangedEvent). This means isKnownDaemonEvent() returns false for this event, and the session reducer increments unrecognizedKnownEventCount for every git_branch_changed event.
Functionally harmless (the webui mapper handles the raw event), but inconsistent with other workspace events like memory_changed. Consider adding a typed interface and a case in asKnownDaemonEvent.
— qwen3.7-max via Qwen Code /review
| status: 'connected', | ||
| workspaceCwd: effectWorkspaceCwd, | ||
| gitBranch: | ||
| gitResult.status === 'fulfilled' |
There was a problem hiding this comment.
[Suggestion] When gitResult is rejected in the deferred-connect path, no console.warn is logged — unlike providerResult, skillsResult, and acpStatusResult rejections, which each log a warning. This makes debugging connection issues harder.
| gitResult.status === 'fulfilled' | |
| if (gitResult.status === 'rejected') { | |
| console.warn( | |
| '[DaemonSessionProvider] workspaceGit failed:', | |
| gitResult.reason, | |
| ); | |
| } | |
| if (providerResult.status === 'rejected') { |
— qwen3.7-max via Qwen Code /review
| : client.workspaceProviders(), | ||
| canReuseSessionMetadata | ||
| ? Promise.resolve(undefined) | ||
| : activeSession.supportedCommands(), |
There was a problem hiding this comment.
[Suggestion] Same pattern as the deferred-connect path: when gitResult is rejected in the reconnect path, the failure is silently absorbed via the current.gitBranch fallback. The other three results in this Promise.allSettled are logged when rejected. Adding a console.warn for gitResult rejection would aid debugging.
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /resolve |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge Conflict Resolution SummaryConflictspackages/cli/src/serve/server.tsConflict location: Lines 811-817 (after merge-base line 785) What conflicted:
Resolution:
Rationale: Files Modified
Commit
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
✅ Maintainer local verification — real build, tests & UI captureVerified PR head Results at a glance
Reviewer Test Plan — walked through in the real appI wrote a throwaway Playwright spec that drives the real web-shell App in Chromium (real ① Git workspace → indicator shows the current branch (dark + light)
② Switch branch → indicator updates live — pushing the exact SSE frame the reflog watcher emits,
③ Non-Git workspace → indicator hidden (
The remaining test-plan item — "follows the active session's workspace, not the sidebar selection" — is covered by the mapper's workspace guard, exercised by the unit test Test evidence
Per-package unit-test breakdown (verbatim vitest summaries)Run as
The Notes worth having for merge
Verification verdict: PASS. Functionally correct across the server route/watcher, SDK, webui connection state, and the web-shell UI, with the read-only / hidden-when-non-Git contract holding in a real browser. Good merge candidate from a verification standpoint. Evidence images are hosted on the 🇨🇳 中文版本（点击展开）✅ Maintainer 本地验证 —— 真实构建、测试与 UI 截图在隔离的 git worktree 中，基于 macOS 15.6 / Node 22.23.1 验证了 PR 的 HEAD 结果概览
Reviewer 测试计划 —— 在真实 App 中逐条走查我写了一个一次性的 Playwright 用例，驱动真实的 web-shell App 在 Chromium 中运行（真实的 ① Git workspace → 指示器显示当前分支（深色 + 浅色），见上方第一张图。 ② 切换分支 → 指示器实时更新 —— 推送 reflog watcher 会发出的那一帧 SSE 事件， ③ 非 Git workspace → 指示器隐藏（ 测试计划中剩下的一条 ——"跟随的是活跃 session 的 workspace，而不是侧边栏选中的 workspace"—— 由 mapper 中的 workspace 守卫覆盖，并由单元测试 可供合并参考的补充说明
验证结论：PASS。 在 server 路由 / watcher、SDK、webui 连接状态、web-shell UI 各层均功能正确，"只读 / 非 Git 时隐藏"的约定在真实浏览器中成立。从验证角度看是一个不错的可合并候选。 证据图片托管在 |
…n narrow screens (QwenLM#6753) The Git branch chip kept max-width: 180px on narrow screens while the mode/model buttons collapse to icon-only, so inside the wrapping .toolbarLeft it claimed roughly half the row and pushed mode/model onto a second line — making the composer taller (QwenLM#6725). Within the existing @container (max-width: 699px) block, keep the leading controls on one row (flex-wrap: nowrap) and let the branch chip yield space first, truncating via its existing ellipsis down to just the icon if needed (max-width: 140px; flex-shrink: 1). The mode/model buttons stay fixed-size. Desktop (composer > 699px) is unchanged.
4 participants
What this PR does
Adds a read-only Git branch indicator to the Web Shell composer toolbar. The indicator shows the active session workspace's current branch, hides when the workspace is not inside a Git repository, and updates when the branch changes.
Why it's needed
This implements the Git branch display requested in #6702 and supports the broader Web Shell workspace context improvements tracked in #6699. Showing the active branch in the composer helps users understand which repository state their session is operating in without adding branch-switching behavior or other workflow changes.
Reviewer Test Plan
How to verify
Open Web Shell from a Git workspace and confirm the composer toolbar shows a Git branch indicator with the current branch name. Switch branches in that workspace and confirm the indicator updates. Open Web Shell from a non-Git workspace and confirm the indicator is hidden. The implementation uses the active session workspace cwd rather than the sidebar-selected workspace, so reviewers should also confirm the indicator follows the active session's workspace.
Evidence (Before & After)
No screenshot captured locally. The local Windows development environment could serve the new Web Shell build and the new Git route, but the ACP child process did not establish a connected Web Shell session for a visual capture. I verified the Git route, non-Git null result, watcher-driven branch updates after branch switching, session mapper behavior, component rendering, type checks, and production builds locally.
Commands verified locally:
Tested on
Environment (optional)
Windows 11, Node 22 used for local daemon/API validation where needed. The repository dependencies were installed locally; the full repository prepare/build step is affected by an existing dependency/type issue outside this PR, so validation focused on the changed packages and relevant tests.
Risk & Scope
Linked Issues
Closes #6702
中文说明
What this PR does
这个 PR 在 Web Shell composer toolbar 中新增了一个只读的 Git 分支指示器。它会显示当前活跃 session workspace 的当前分支，在 workspace 不属于 Git 仓库时隐藏，并在分支变化时更新。
Why it's needed
这个 PR 实现了 #6702 中请求的 Git 分支显示能力，也对应 #6699 中 Web Shell workspace 上下文改进的大方向。在 composer 中显示当前分支，可以帮助用户确认当前 session 正在操作哪个仓库状态，同时不会引入切换分支或其他 workflow 行为变化。
Reviewer Test Plan
How to verify
从一个 Git workspace 打开 Web Shell，确认 composer toolbar 中显示 Git 分支指示器和当前分支名。在该 workspace 中切换分支，确认指示器会更新。从一个非 Git workspace 打开 Web Shell，确认该指示器会隐藏。这个实现使用活跃 session 的 workspace cwd，而不是侧边栏当前选中的 workspace，所以 reviewer 也可以确认该指示器跟随的是活跃 session 的 workspace。
Evidence (Before & After)
本地没有截取截图。我的 Windows 本地开发环境可以启动新的 Web Shell build，也可以访问新的 Git route，但 ACP 子进程没有成功建立已连接的 Web Shell session，因此无法截取真实 UI 图。我已经在本地验证了 Git route、非 Git workspace 返回 null、切换分支后的 watcher 更新、session mapper 行为、组件渲染、类型检查和生产构建。
本地验证过的命令：
Tested on
Environment (optional)
Windows 11，本地 daemon/API 验证中按需使用了 Node 22。仓库依赖已在本地安装；完整仓库 prepare/build 步骤受一个不属于本 PR 的既有依赖/类型问题影响，所以验证聚焦在本 PR 改动涉及的 package 和相关测试上。
Risk & Scope
Linked Issues
Closes #6702