feat(web-shell): add git commit history browser - #7204
Merged
Conversation
Collaborator
|
Thanks for the PR! Template looks good ✓ — all required sections present with detailed test plan and screenshots. Problem: This is a feature addition, not a bug fix — the "problem" is a UX gap: viewing commit history in the Web Shell currently requires asking the agent to run Direction: Aligned. The Web Shell is an active investment area, and commit history is a fundamental developer workflow view. The existing Size: Core paths touched ( Approach: The scope feels right for a commit history browser. Each layer follows the established pattern from the existing git diff feature: core git utilities → daemon REST routes → SDK client methods → React dialog component. The refactoring of Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ — 所有必填章节齐全，附有详细测试计划和截图。 问题： 这是功能新增，不是 bug 修复——"问题"是体验缺口：在 Web Shell 中查看提交历史目前只能让 agent 在聊天中跑 方向： 对齐。Web Shell 是活跃投资方向，提交历史是开发者基础工作流视图。现有 规模： 触及核心路径（ 方案： 对于提交历史浏览器来说范围合理。每层都遵循现有 git diff 功能的既定模式。 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / after✅ No screenshot changes against the PR base. Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Collaborator
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 4 scenario(s). — Qwen Code · serve A/B |
Collaborator
Code ReviewIndependent proposal: For a commit history browser in the Web Shell, I would follow the exact same layered pattern the existing git diff feature uses: add Comparison with the diff: The PR's approach matches this proposal almost exactly. The implementation is clean and well-structured:
No critical blockers found. No AGENTS.md violations. The code follows project conventions (ESM, kebab-case files, collocated tests, CSS Modules, no sequenceDiagram
participant P1 as User
participant P2 as Web Shell
participant P3 as SDK Client
participant P4 as Daemon Route
participant P5 as Core gitDiff
P1->>P2: /log command or History tab
P2->>P3: workspaceGitLog(limit, skip)
P3->>P4: GET /workspace/git/log
P4->>P5: fetchGitLog(cwd, limit, skip)
P5-->>P4: GitLogResult (entries, hasMore)
P4-->>P3: DaemonGitLog JSON
P3-->>P2: render commit list
P1->>P2: click commit row (expand)
P2->>P3: workspaceGitCommitDetail(sha)
P3->>P4: GET /workspace/git/log/commit?sha=
P4->>P5: fetchGitCommitDetail(cwd, sha)
P5-->>P4: GitCommitDetail (body, files, stats)
P4-->>P3: DaemonGitCommitDetail JSON
P3-->>P2: render detail panel
Files changed (24 of 24 shown)
Real-Scenario TestingBuilt the PR branch ( Daemon startup (tmux capture-pane)GET /workspace/git/log?limit=5{
"v": 1,
"workspaceCwd": "/home/github-runner/actions-runner-18/_work/qwen-code/qwen-code",
"available": true,
"entries": [
{
"sha": "54dfda1b8c07e2d7bf6254c5436cea36d0e9648a",
"shortSha": "54dfda1",
"authorName": "Shaojin Wen",
"authorEmail": "szujobs@gmail.com",
"authorDate": 1784556330,
"subject": "fix(core): harden git log metadata parsing",
"refs": "grafted, HEAD -> pr-7204",
"parents": []
}
],
"hasMore": false
}GET /workspace/git/log/commit?sha=54dfda1{
"v": 1,
"workspaceCwd": "/home/github-runner/actions-runner-18/_work/qwen-code/qwen-code",
"available": true,
"sha": "54dfda1b8c07e2d7bf6254c5436cea36d0e9648a",
"shortSha": "54dfda1",
"authorName": "Shaojin Wen",
"subject": "fix(core): harden git log metadata parsing",
"body": "Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>",
"refs": "grafted, HEAD -> pr-7204",
"parents": [],
"files": [ ... ],
"filesCount": 24,
"linesAdded": 3021,
"linesRemoved": 54,
"hiddenCount": 0
}Edge casesDaemon request log (tmux capture-pane)Unit tests中文说明代码审查独立方案： 对于 Web Shell 中的提交历史浏览器，我会采用与现有 git diff 功能完全相同的分层模式：在 与 diff 的比较： PR 的方案与此提案几乎完全一致。实现干净、结构良好。核心层正确使用 NUL 字段分隔符，daemon 路由有 SHA 验证和信任门控，SDK 层遵循现有模式，Web Shell 组件有完整的 ARIA 标签语义和 i18n 支持。未发现关键阻断问题。 真实场景测试构建 PR 分支，在 tmux 中启动 daemon，用 curl 测试新的 REST 端点。所有端点正常工作：提交列表、commit 详情、SHA 验证（无效 SHA 返回 400）、不存在的 SHA 返回 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Confidence: 4/5 — Clean feature addition that follows established patterns, solid test coverage, endpoints verified end-to-end. Only non-blocking nit: This is a well-executed feature PR. The approach matches what I'd propose independently — same layered architecture, same git plumbing commands, same pagination strategy. Nothing surprising, nothing over-engineered. The code reads like a natural extension of the existing git diff feature, which is exactly what you want from a four-package change. What I liked: the NUL-delimited format parsing is correct (commit subjects can contain anything), merge commits get first-parent diffs instead of empty output, renames are handled via The one thing I'd watch in six months: if users start paginating deep into history (thousands of commits), the 中文说明信心：4/5 — 干净的功能新增，遵循既定模式，测试覆盖扎实，端点已端到端验证。唯一非阻断的小问题： 这是一个执行良好的功能 PR。方案与我独立提出的完全一致——相同的分层架构、相同的 git 底层命令、相同的分页策略。没有意外，没有过度设计。代码读起来像现有 git diff 功能的自然扩展，这对于跨四个包的改动来说正是你想要的。 亮点：NUL 分隔格式解析正确（commit subject 可以包含任何字符），merge commit 使用 first-parent diff 而非空输出，通过 — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
approved these changes
Jul 19, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
requested changes
Jul 19, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
pushed a commit
that referenced
this pull request
Jul 19, 2026
pull Bot
pushed a commit
to bit-cook/qwen-code
that referenced
this pull request
Jul 19, 2026
…e-flagging (QwenLM#7210) The before/after preview flagged text-heavy views (e.g. workspace-sidebar) as ~0.1-0.3% "changed" on PRs that do not touch them — QwenLM#7204 is a live example (the two panels are pixel-for-pixel indistinguishable). Root cause: base and head render in SEPARATE CI jobs, so Linux font anti-aliasing is not bit-identical between them, and the naive per-pixel diff counts the scatter of isolated / 1px-wide glyph-edge pixels that leaves. At the tight 0.02% threshold that scatter crosses the line. Measure the changed fraction AFTER a cluster denoise: a differing pixel counts only when at least 4 of its 8 neighbours also differ. AA scatter (isolated = 0 neighbours, a 1px line = 2) erodes to ~zero, while a real change — a badge, chip, icon, panel — is a solid block whose interior keeps 5-8 and easily clears the threshold, so the threshold stays tight without raising it (which would miss small real changes like a workspace badge). The browser now returns a compact bit-mask; the denoise + count run in node against the unit-tested countDenoisedChanges, so there is one tested implementation of the metric. Co-authored-by: wenshao <wenshao@example.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.8-max-preview via Qwen Code /review
Comment on lines
+1998
to
+2005
| it('clamps limit to MAX_LOG_LIMIT', async () => { | ||
| await fs.writeFile(path.join(repo, 'a.txt'), 'x\n'); | ||
| await git(repo, 'add', '.'); | ||
| await git(repo, 'commit', '-q', '-m', 'c1'); | ||
|
|
||
| const result = await fetchGitLog(repo, { limit: 9999 }); | ||
| expect(result!.entries).toHaveLength(1); | ||
| }); |
Collaborator
There was a problem hiding this comment.
[Suggestion] This test is titled clamps limit to MAX_LOG_LIMIT but seeds only one commit, so toHaveLength(1) passes whether or not the clamp exists — Failure scenario: if Math.min(…, MAX_LOG_LIMIT) in fetchGitLog regressed (limit passed through unclamped), git log -n 10000 on this single-commit repo still returns exactly 1 entry and this test stays green, so the behavior named in the title ships unverified. The daemon-level test does pin the clamp (it mocks fetchGitLog and asserts limit: 200); this core-level one does not.
Seed MAX_LOG_LIMIT + 1 commits (or stub runGit) and assert entries.length === MAX_LOG_LIMIT, so an unclamped limit would actually return more entries than a clamped one.
— qwen3.8-max-preview via Qwen Code /review
Comment on lines
+445
to
+450
| {onOpenLog && ( | ||
| <div className={styles.tabBar}> | ||
| <button | ||
| type="button" | ||
| className={`${styles.tab} ${styles.tabActive}`} | ||
| > |
Collaborator
There was a problem hiding this comment.
[Suggestion] The new tab bar lacks ARIA tab semantics (role="tablist" on the container, role="tab" + aria-selected on each button) — Failure scenario: a screen-reader user encounters two plain buttons (Changes, History) with no indication they behave as tabs or which is active; the active state is conveyed only by the visual tabActive class. DaemonStatusDialog.tsx applies exactly this pattern (role="tablist"/role="tab"/aria-selected + arrow-key handling), so the new bar is the only tab-like widget in the codebase without it. The same gap exists in the GitLogDialog.tsx tab bar.
Suggested change
| {onOpenLog && ( | |
| <div className={styles.tabBar}> | |
| <button | |
| type="button" | |
| className={`${styles.tab} ${styles.tabActive}`} | |
| > | |
| {onOpenLog && ( | |
| <div className={styles.tabBar} role="tablist"> | |
| <button | |
| type="button" | |
| role="tab" | |
| aria-selected="true" | |
| className={`${styles.tab} ${styles.tabActive}`} | |
| > |
— qwen3.8-max-preview via Qwen Code /review
Comment on lines
+26
to
+28
| .tab:hover { | ||
| background: var(--subtle-bg); | ||
| } |
Collaborator
There was a problem hiding this comment.
[Suggestion] --subtle-bg (and --success-bg used by .refHead) are not defined in the dialog's theme scope — Failure scenario: the dialog portals to document.body, outside the app root in App.module.css where these variables live; DialogShell.module.css re-declares the dialog's theme variables but omits these two, so every background: var(--subtle-bg) (.tab:hover, .tabActive, .commitHeader:hover, .commitDetail, .loadMore:hover) and var(--success-bg) (.refHead) resolves to transparent. Hover feedback is invisible, the active tab has no background distinction, and the HEAD ref tag loses its green badge. (The new GitDiffDialog.module.css tab rules inherit the same gap.)
Add the missing variables to both .themeDark and .themeLight in DialogShell.module.css, mirroring App.module.css.
— qwen3.8-max-preview via Qwen Code /review
Comment on lines
+78
to
+81
| void navigator.clipboard.writeText(entry.sha).then(() => { | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false), 1500); | ||
| }); |
Collaborator
There was a problem hiding this comment.
[Suggestion] Unhandled promise rejection when the Clipboard API fails — navigator.clipboard.writeText(...).then(...) has no .catch() — Failure scenario: when the page is in a non-secure context, a Permissions-Policy blocks clipboard-write, the document is unfocused, or the user denies the permission, every click on copy-SHA produces an unhandled rejection and the user gets no feedback that the copy failed.
Suggested change
| void navigator.clipboard.writeText(entry.sha).then(() => { | |
| setCopied(true); | |
| setTimeout(() => setCopied(false), 1500); | |
| }); | |
| void navigator.clipboard.writeText(entry.sha).then(() => { | |
| setCopied(true); | |
| setTimeout(() => setCopied(false), 1500); | |
| }).catch(() => {}); |
— qwen3.8-max-preview via Qwen Code /review
Comment on lines
+39
to
+41
| const months = Math.floor(days / 30); | ||
| if (months < 12) return `${months}mo ago`; | ||
| return `${Math.floor(days / 365)}y ago`; |
Collaborator
There was a problem hiding this comment.
[Suggestion] timeAgo renders "0y ago" for commits ~360–364 days old — Failure scenario: at days = 360, months = floor(360/30) = 12 fails the < 12 guard and falls through to floor(360/365) = 0, producing the nonsensical string "0y ago" instead of "12mo ago"/"1y ago". The window is ~5 days per year.
Suggested change
| const months = Math.floor(days / 30); | |
| if (months < 12) return `${months}mo ago`; | |
| return `${Math.floor(days / 365)}y ago`; | |
| const months = Math.floor(days / 30); | |
| if (months < 12) return `${months}mo ago`; | |
| return `${Math.max(1, Math.floor(days / 365))}y ago`; |
— qwen3.8-max-preview via Qwen Code /review
Comment on lines
+239
to
+242
| useEffect(() => { | ||
| let cancelled = false; | ||
| setLoading(true); | ||
| setError(false); |
Collaborator
There was a problem hiding this comment.
[Suggestion] The initial-fetch effect resets loading and error but never loadMoreError — Failure scenario: a Load-more request fails (loadMoreError = true), then the client/workspaceCwd effect re-fires (e.g. daemon reconnect) and reloads page 0 successfully; because loadMoreError is still true and the fresh page has hasMore, the stale "Failed to load history" placeholder renders above the Load-more button even though the history loaded without error.
Suggested change
| useEffect(() => { | |
| let cancelled = false; | |
| setLoading(true); | |
| setError(false); | |
| useEffect(() => { | |
| let cancelled = false; | |
| setLoading(true); | |
| setError(false); | |
| setLoadMoreError(false); |
— qwen3.8-max-preview via Qwen Code /review
Comment on lines
+6145
to
+6147
| const cwd = diffWorkspaceCwd; | ||
| setDiffWorkspaceCwd(undefined); | ||
| window.setTimeout(() => setLogWorkspaceCwd(cwd), 300); |
Collaborator
There was a problem hiding this comment.
[Suggestion] The dialog-switch setTimeout(…, 300) is a magic number that is neither tied to any animation nor cleared/guarded — Failure scenario: (1) DialogShell.module.css defines no transition/animation, so the 300 ms is a perceptible blank gap between dialogs rather than waiting on a close animation; (2) during that window both diffWorkspaceCwd and logWorkspaceCwd are undefined, so dialogOpen is false and the composer/sidebar are live — if the user clicks a sidebar git chip or runs /diff in those 300 ms, that sets one dialog state immediately and the pending timer then sets the other, mounting GitDiffDialog and GitLogDialog simultaneously (or opening one for a stale cwd).
Track the timer in a ref and clear it in each onClose/onOpen* handler before setting new state (or switch on a transitionend/setTimeout(…, 0)), and skip the deferred set if another dialog was opened in the meantime.
— qwen3.8-max-preview via Qwen Code /review
Comment on lines
+257
to
+260
| const response = await request(app).get('/workspaces/secondary/git/log'); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(fetchGitLogMock).toHaveBeenCalledWith('/work/secondary', { |
Collaborator
There was a problem hiding this comment.
[Suggestion] Workspace isolation is guarded asymmetrically: this positive resolution test covers the qualified list route (fetchGitLog called with /work/secondary), but the qualified commit-detail route (GET /workspaces/:workspace/git/log/commit) has no positive test that it resolves the selected workspace's cwd — only the untrusted-rejection case touches it (asserting 403). Failure scenario: if a refactor of the qualified detail handler regresses to pass the wrong cwd (e.g. the primary runtime's), every existing test still passes and a multi-workspace daemon returns another workspace's commit detail (body, file paths, line counts) for a trusted secondary id — the exact cross-workspace exposure these tests exist to prevent.
Mirror this test for the detail route: register two trusted workspaces, GET /workspaces/secondary/git/log/commit?sha=abcdef1, and assert fetchGitCommitDetailMock was called with '/work/secondary'.
— qwen3.8-max-preview via Qwen Code /review
Comment on lines
+270
to
+271
| ...result, | ||
| entries: [...prev.entries, ...result.entries], |
Collaborator
There was a problem hiding this comment.
[Suggestion] The loadMore merge spreads ...result as the base object, overwriting the existing log.available (and workspaceCwd, v) with the new page's values; only entries is preserved from prior state — Failure scenario: page 0 loads (available: true, 50 entries); before Load-more, the repo becomes unavailable and the route maps it to { available: false, entries: [] }; the merge yields { ...result, entries: [...prev.entries, ...[]] }, so available flips to false and the render guard (!log.available) replaces the 50 browsed rows with the "Git is not available" placeholder, discarding the user's already-loaded history.
Suggested change
| ...result, | |
| entries: [...prev.entries, ...result.entries], | |
| ...prev, | |
| hasMore: result.hasMore, | |
| entries: [...prev.entries, ...result.entries], |
— qwen3.8-max-preview via Qwen Code /review
Comment on lines
+191
to
+193
| <span className={styles.copyBtn} onClick={copySha} aria-hidden="true"> | ||
| {copied ? <CheckIcon size={12} /> : <CopyIcon size={12} />} | ||
| </span> |
Collaborator
There was a problem hiding this comment.
[Suggestion] The copy-SHA control is a non-focusable, aria-hidden <span> with only an onClick (no tabIndex, role, or onKeyDown) — Failure scenario: a keyboard-only or screen-reader user tabs through the commit list; the commit-header <button> receives focus, but the copy control inside it can never be focused and is removed from the accessibility tree, so these users cannot copy a SHA at all (WCAG 2.1.1 Keyboard). This is the root cause behind the hover-only CSS reveal (.commitHeader:hover .copyBtn): adding :focus-visible alone won't help because a <span> without tabIndex never receives focus.
Make it a real control (e.g. a <button type="button" aria-label=…> as a sibling of the header button, since a button nested in a button is invalid HTML) and drop aria-hidden.
— qwen3.8-max-preview via Qwen Code /review
Collaborator
|
Qwen Code review timed out. Qwen review timed out after 180 minutes. For large PRs, retry with a longer timeout by commenting: |
qwen-code-ci-bot
pushed a commit
that referenced
this pull request
Jul 19, 2026
Collaborator
|
Qwen Code review timed out. Qwen review timed out after 10800 seconds (of the 180-minute budget). For large PRs, retry with a longer timeout by commenting: |
Add a read-only Git Log dialog to the Web Shell, accessible via /log command or the History tab in the Changes dialog. Full-stack implementation across core, daemon, SDK, and web-shell: - core: fetchGitLog (paginated commit list) and fetchGitCommitDetail (message body + per-file numstat) with 12 integration tests - daemon: GET /workspace/git/log and /workspace/git/log/commit routes with bound + qualified dual registration - SDK: DaemonGitLog/DaemonGitCommitDetail types and client methods - web-shell: GitLogDialog with commit list, expandable details, SHA copy icon, Load more pagination, and Changes/History tab switching in both dialogs
- Critical: use first-parent diff for merge commits (diff-tree without -c or explicit parent outputs nothing for merges) - Remove dead embedded prop from GitDiffDialog and GitLogDialog - Replace span role=button with aria-hidden for copy icon (a11y) - Add loadMore error feedback instead of silent catch - Refresh relative timestamps every 60s (useState + interval) - Remove dead branch param from subtitle i18n call
- Use bounded split in parseLogFields (first 7 separators) to prevent subject containing literal \x1f from shifting the parents field - Add SHA hex format validation at daemon route layer (400 for invalid) - Add rendering branch for detail.available === false (error message instead of empty content)
…oute & dialog Follows the R2 review-feedback commit (which fixed the bounded parse, the non-hex SHA 400, and the unavailable-detail render). Remaining items: - Commit detail counts renamed files. diff-tree is plumbing and does not honour diff.renames, so a `git mv` split into a delete + add pair (or an empty-path entry) instead of one file — understating filesCount / linesAdded / linesRemoved. Run diff-tree with -M and give the inline numstat parser the same pending-rename state machine as parseGitNumstat, so a rename is one file keyed by its new path. Covered by a real-repo rename test, plus a merge-commit test that locks the first-parent diff. - Tests for the two previously-untested modules: the workspace-git-log route (list shape, pagination clamping, sha-required + non-hex 400, trust gating) and GitLogDialog (all five list state paths, load-more offset + error, detail expand + both failure branches incl. available:false, and the relative-time render).
- Fix timeAgo '0y ago' for commits ~360-364 days old (Math.max(1, ...)) - Add .catch() to clipboard writeText to prevent unhandled rejection - Reset loadMoreError on initial re-fetch (daemon reconnect) - Preserve prev.available in loadMore merge instead of overwriting - Add ARIA tab semantics (role=tablist/tab, aria-selected) to both Changes and History tab bars
- Fix vacuous limit-clamp test: seed 3 commits, verify limit=2 returns 2 + hasMore, limit=0 clamps to 1 - Add CSS var fallbacks for --subtle-bg and --success-bg (dialog portals outside App.module.css scope) - Extract GIT_DIALOG_SWITCH_DELAY_MS constant with doc comment - Make copy-SHA control keyboard accessible (tabIndex, onKeyDown, aria-label instead of aria-hidden)
wenshao
force-pushed
the
feat/webshell-git-log
branch
from
July 19, 2026 23:51
361b444 to
ca08ca7
Compare
Contributor
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
Collaborator
Author
Review —
|
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Collaborator
Author
|
Addressed in Implemented the review feedback as follows:
I intentionally did not change the deep- Verification:
|
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Collaborator
Author
CI Failure AnalysisRun: https://github.com/QwenLM/qwen-code/actions/runs/29731096240/job/88315824186 Root CauseThe PR-related failure was a stale module mock in Fixed in Failing Tests1. Web Shell —
2. Core —
3. WebUI —
Verification
🇨🇳 中文分析CI 失败分析运行记录： https://github.com/QwenLM/qwen-code/actions/runs/29731096240/job/88315824186 根本原因与本 PR 直接相关的失败是 已在 失败测试1. Web Shell —
2. Core —
3. WebUI —
验证结果
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
Comment on lines
+441
to
459
| export function GitDiffDialog({ | ||
| workspaceCwd, | ||
| onClose, | ||
| }: { | ||
| workspaceCwd: string; | ||
| onClose: () => void; | ||
| }) { | ||
| const { t } = useI18n(); | ||
| return ( | ||
| <DialogShell | ||
| title={t('gitDiff.title')} | ||
| subtitle={subtitle} | ||
| size="xl" | ||
| allowFullscreen | ||
| onClose={onClose} | ||
| > | ||
| {body} | ||
| <GitDiffContent workspaceCwd={workspaceCwd} /> | ||
| </DialogShell> | ||
| ); | ||
| } |
Collaborator
There was a problem hiding this comment.
[Suggestion] The standalone GitDiffDialog wrapper no longer displays the subtitle (file count / line stats summary) — Failure scenario: the pre-refactor GitDiffDialog computed a subtitle locally and passed it to DialogShell. The refactoring moved subtitle computation into GitDiffContent, which communicates it via onSubtitleChange. GitDialog (used by App.tsx) wires this correctly, but the standalone wrapper does not pass onSubtitleChange to GitDiffContent, so its DialogShell never receives the subtitle. The same pattern also appears in GitLogDialog (line 354).
Suggested change
| export function GitDiffDialog({ | |
| workspaceCwd, | |
| onClose, | |
| }: { | |
| workspaceCwd: string; | |
| onClose: () => void; | |
| }) { | |
| const { t } = useI18n(); | |
| return ( | |
| <DialogShell | |
| title={t('gitDiff.title')} | |
| subtitle={subtitle} | |
| size="xl" | |
| allowFullscreen | |
| onClose={onClose} | |
| > | |
| {body} | |
| <GitDiffContent workspaceCwd={workspaceCwd} /> | |
| </DialogShell> | |
| ); | |
| } | |
| export function GitDiffDialog({ | |
| workspaceCwd, | |
| onClose, | |
| }: { | |
| workspaceCwd: string; | |
| onClose: () => void; | |
| }) { | |
| const { t } = useI18n(); | |
| const [subtitle, setSubtitle] = useState<string>(); | |
| return ( | |
| <DialogShell | |
| title={t('gitDiff.title')} | |
| subtitle={subtitle} | |
| size="xl" | |
| allowFullscreen | |
| onClose={onClose} | |
| > | |
| <GitDiffContent workspaceCwd={workspaceCwd} onSubtitleChange={setSubtitle} /> | |
| </DialogShell> | |
| ); | |
| } |
— qwen3.7-max via Qwen Code /review
Comment on lines
+354
to
+372
| export function GitLogDialog({ | ||
| workspaceCwd, | ||
| onClose, | ||
| }: { | ||
| workspaceCwd: string; | ||
| onClose: () => void; | ||
| }) { | ||
| const { t } = useI18n(); | ||
| return ( | ||
| <DialogShell | ||
| title={t('gitLog.title')} | ||
| size="xl" | ||
| allowFullscreen | ||
| onClose={onClose} | ||
| > | ||
| <GitLogContent workspaceCwd={workspaceCwd} /> | ||
| </DialogShell> | ||
| ); | ||
| } |
Collaborator
There was a problem hiding this comment.
[Suggestion] The standalone GitLogDialog wrapper does not wire the subtitle through — same pattern as GitDiffDialog (line 441). GitLogContent computes and emits the subtitle via onSubtitleChange, but the standalone wrapper neither provides a callback nor forwards the result to DialogShell.
Suggested change
| export function GitLogDialog({ | |
| workspaceCwd, | |
| onClose, | |
| }: { | |
| workspaceCwd: string; | |
| onClose: () => void; | |
| }) { | |
| const { t } = useI18n(); | |
| return ( | |
| <DialogShell | |
| title={t('gitLog.title')} | |
| size="xl" | |
| allowFullscreen | |
| onClose={onClose} | |
| > | |
| <GitLogContent workspaceCwd={workspaceCwd} /> | |
| </DialogShell> | |
| ); | |
| } | |
| export function GitLogDialog({ | |
| workspaceCwd, | |
| onClose, | |
| }: { | |
| workspaceCwd: string; | |
| onClose: () => void; | |
| }) { | |
| const { t } = useI18n(); | |
| const [subtitle, setSubtitle] = useState<string>(); | |
| return ( | |
| <DialogShell | |
| title={t('gitLog.title')} | |
| subtitle={subtitle} | |
| size="xl" | |
| allowFullscreen | |
| onClose={onClose} | |
| > | |
| <GitLogContent workspaceCwd={workspaceCwd} onSubtitleChange={setSubtitle} /> | |
| </DialogShell> | |
| ); | |
| } |
— qwen3.7-max via Qwen Code /review
Comment on lines
+76
to
+84
| const copySha = () => { | ||
| void navigator.clipboard | ||
| .writeText(entry.sha) | ||
| .then(() => { | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false), 1500); | ||
| }) | ||
| .catch(() => {}); | ||
| }; |
Collaborator
There was a problem hiding this comment.
[Suggestion] copySha accesses navigator.clipboard.writeText without a null guard — Failure scenario: a user accesses the web shell over HTTP from a remote host (e.g. http://server.lan:4170). navigator.clipboard is undefined in non-secure contexts, so .writeText throws a synchronous TypeError before the promise chain's .catch() is reached. The same codebase guards against this consistently: MessageTimestamp.tsx uses navigator.clipboard?.writeText(), EnhancedMarkdownTable.tsx uses if (!navigator.clipboard) return;.
Suggested change
| const copySha = () => { | |
| void navigator.clipboard | |
| .writeText(entry.sha) | |
| .then(() => { | |
| setCopied(true); | |
| setTimeout(() => setCopied(false), 1500); | |
| }) | |
| .catch(() => {}); | |
| }; | |
| const copySha = () => { | |
| void navigator.clipboard | |
| ?.writeText(entry.sha) | |
| .then(() => { | |
| setCopied(true); | |
| setTimeout(() => setCopied(false), 1500); | |
| }) | |
| .catch(() => {}); | |
| }; |
— qwen3.7-max via Qwen Code /review
Comment on lines
+272
to
+274
| const loadMore = useCallback(() => { | ||
| if (!log || loadingMore) return; | ||
| setLoadingMore(true); |
Collaborator
There was a problem hiding this comment.
[Suggestion] The loadingMore guard in loadMore is a React state value captured in a closure — Failure scenario: a double-click on "Load more" can fire two click events within a single render cycle, both seeing loadingMore === false. Both fetches fire with the same nextSkipRef.current value. SHA-based dedup prevents duplicate rows, but nextSkipRef.current is incremented by result.entries.length in both .then() callbacks, overshooting by one page — silently skipping entries on the next click.
Suggested change
| const loadMore = useCallback(() => { | |
| if (!log || loadingMore) return; | |
| setLoadingMore(true); | |
| const loadingMoreRef = useRef(false); | |
| const loadMore = useCallback(() => { | |
| if (!log || loadingMoreRef.current) return; | |
| loadingMoreRef.current = true; | |
| setLoadingMore(true); |
— qwen3.7-max via Qwen Code /review
Collaborator
Author
|
@qwen-code /review |
Contributor
| _Qwen Code review request accepted. Review is queued in [workflow run](https://github.com/QwenLM/qwen-code/actions/runs/29743374471)._ |
wenshao
commented
Jul 20, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
Author
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
— qwen3.8-max-preview via Qwen Code /review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
ytahdn
approved these changes
Jul 20, 2026
ytahdn
left a comment
ytahdn
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. 增量 review 无 Critical 缺陷。
核心确认：
fetchGitLog/fetchGitCommitDetail实现清晰，SHA 验证、merge commit 处理、空仓库探测均正确parsePagination正确 clamp limit/skip，qualified 路由有 trust gatingGitDialogARIA tablist（Arrow/Home/End/aria-selected）实现到位CommitRow懒加载 +cancelledRef防 unmount setState，timeAgo使用Intl.RelativeTimeFormat- 分页
nextSkipRef正确，load more 带错误处理 - 测试覆盖 12 个路由用例 + 组件渲染/分页/复制/展开测试
— qwen3.7-plus via Qwen Code /review
Collaborator
Review 总结变更概述为 Web Shell 添加只读 Git commit 历史浏览器，通过 代码质量评估
结论无 Critical 缺陷，实现质量高，可以合并。👍 |
Collaborator
Author
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: removed-behavior — Agent 1b subagent execution failed. Not reviewed: every dimension — none of the 17 required agents is on record as launched with a prompt this skill built, so this diff was reviewed, if at all, from prompts the run wrote for itself: no record shows the severity bar, the finding format or this project's own rules reaching an agent. Not reviewed: reverse audit — an auditor ran and opened its brief, but no agent was launched with the prompt the CLI built — the launch was written by hand, and what the agent was actually asked is not what this skill certifies. Not reviewed: verification — the review posts findings, but no verifier was launched with a prompt this skill builds — they were ruled on, if at all, without the verdict bar its brief carries.
— qwen3.7-max via Qwen Code /review
Comment on lines
+209
to
+211
| .loadMore:hover { | ||
| background: var(--subtle-bg, rgba(128, 128, 128, 0.06)); | ||
| } |
Collaborator
There was a problem hiding this comment.
[Suggestion] .loadMore sets cursor: pointer but has no :disabled override — Failure scenario: when disabled={loadingMore} is active during pagination, the button still shows a pointer cursor because the author CSS cursor: pointer overrides the browser UA stylesheet's :disabled { cursor: default }, providing no visual feedback that the button is non-interactive.
.loadMore:disabled {
cursor: default;
opacity: 0.5;
}— qwen3.7-max via Qwen Code /review
|
|
||
| function timeAgo(timestamp: number, now: number, language: string): string { | ||
| const seconds = Math.max(0, Math.floor(now - timestamp)); | ||
| const formatter = new Intl.RelativeTimeFormat(language, { numeric: 'auto' }); |
Collaborator
There was a problem hiding this comment.
[Suggestion] timeAgo() creates new Intl.RelativeTimeFormat(language, ...) on every call — Failure scenario: with PAGE_SIZE=50, each render of the commit list creates 50+ Intl.RelativeTimeFormat instances. Construction is non-trivial (locale resolution, internal table lookup). Cache the formatter per language (e.g., a module-level Map<string, Intl.RelativeTimeFormat>) to avoid repeated construction on every render.
— qwen3.7-max via Qwen Code /review
| setGitDialog({ workspaceCwd: gitDiffWorkspaceCwd, view: 'diff' }); | ||
| return true; | ||
| } | ||
| if (cmd === 'log') { |
Collaborator
There was a problem hiding this comment.
[Suggestion] The /log local command intercept (cmd === 'log') has no test coverage in App.test.tsx — Concrete cost: the new branch (lines 4575–4583) that guards on gitDiffWorkspaceCwd, toasts when absent, and opens the dialog with view: 'log' is entirely untested. A regression (e.g., the command being forwarded to the agent instead of intercepted locally) would go undetected. The parallel /diff intercept at line 4567 follows the same pattern and could be tested with the same approach.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
approved these changes
Jul 20, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Collaborator
|
Qwen Code review timed out. Qwen review timed out after 300 seconds (of the 180-minute budget). For large PRs, retry with a longer timeout by commenting: |
Collaborator
|
Released in v0.20.1. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Adds a read-only Git commit history browser to the Web Shell. Users can view recent commits in a graphical dialog with expandable details (full message body + per-file change stats), copy commit SHAs with one click, and paginate through history. The feature is accessible via the
/logslash command and through a History tab in the existing Changes (/diff) dialog.The implementation spans four packages: new core git utilities for log retrieval, new daemon REST routes, new SDK types and client methods, and a new React dialog component with bilingual (EN/zh-CN) i18n support.
Why it's needed
Previously, viewing commit history in the Web Shell required asking the agent to run
git log --onelineand reading plain text output in the chat. For a graphical interface, this is a significant UX gap — commit history is a fundamental view for code review, change tracking, and understanding project evolution. The existing Changes dialog (/diff) shows working-tree state but provides no way to look at past commits.Reviewer Test Plan
How to verify
npm run dev:daemonand open the Web Shell athttp://localhost:5173//login the composer (press Escape to dismiss autocomplete, then Enter to submit)/diff(Changes dialog) — verify the tab bar shows "Changes" and "History" tabsEvidence (Before & After)
Before: No way to view commit history except asking the agent to run
git login chat.After:
History dialog with commit list:
Expanded commit detail with message body and file stats:
Tested on
Environment
npm run dev:daemonwith Vite dev server, Playwright headless browser verification.Risk & Scope
git log --skipis O(skip) for deep pagination; acceptable for typical usage (50 commits per page, manual paging). Can be replaced with--before=<timestamp>cursor if needed later./diffbehavior unchanged.Linked Issues
Design doc:
docs/design/2026-07-19-webshell-git-log.md中文说明
本 PR 做了什么
为 Web Shell 新增只读的 Git 提交历史浏览器。用户可以在图形化弹窗中查看最近的提交记录，展开查看完整的提交信息和文件变更统计，一键复制 commit SHA，并通过分页加载更多历史。该功能可通过
/log斜杠命令和现有 Changes（/diff）弹窗中的 History 标签页访问。实现横跨四个包：core 层新增 git log 工具函数，daemon 层新增 REST 路由，SDK 层新增类型和客户端方法，web-shell 层新增 React 弹窗组件，支持中英双语。
为什么需要
此前在 Web Shell 中查看提交历史只能让 agent 跑
git log --oneline，然后在聊天中读纯文本。对于图形界面来说这是明显的体验缺口——提交历史是代码审查、回溯变更、理解项目演进的基础视图。现有的 Changes 弹窗（/diff）只展示工作区状态，无法查看过去的提交。审阅者测试计划
如何验证
npm run dev:daemon，打开http://localhost:5173//log（按 Escape 关闭补全，再按 Enter 提交）/diff（Changes 弹窗）——确认顶部有 Changes / History 标签栏测试环境
风险与范围
git log --skip深分页为 O(skip)，对典型使用场景（每页 50 条、手动翻页）可接受。后续可改为--before=<timestamp>游标。