-
Notifications
You must be signed in to change notification settings - Fork 3k
feat(web-shell): git status chip, visual working-tree diff, and sidebar git status #7054
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
18f8a9d
895a682
c25ec11
52e4df2
192db5f
d3a0bff
0c9edbd
9786618
a27840e
d3df892
39e9714
62379e9
ab3f3e8
6c690c5
76bfedb
70dbc9a
fa6c4e0
b122c9f
0f138bd
7fc2656
bf25097
d3b4f12
114dd33
09e3562
7740a9b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,386 @@ | ||
| /** | ||
| * @license | ||
| * Copyright 2026 Qwen Team | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import express from 'express'; | ||
| import request from 'supertest'; | ||
| import { beforeEach, describe, expect, it, vi } from 'vitest'; | ||
| import { | ||
| fetchGitDiff, | ||
| fetchGitDiffHunksForFile, | ||
| } from '@qwen-code/qwen-code-core'; | ||
| import type { AcpSessionBridge } from '../acp-session-bridge.js'; | ||
| import { sendBridgeError } from '../server/error-response.js'; | ||
| import { | ||
| createWorkspaceRegistry, | ||
| type WorkspaceRegistry, | ||
| type WorkspaceRuntime, | ||
| } from '../workspace-registry.js'; | ||
| import { | ||
| registerWorkspaceGitDiffRoutes, | ||
| registerWorkspaceQualifiedGitDiffRoutes, | ||
| } from './workspace-git-diff.js'; | ||
|
|
||
| vi.mock('@qwen-code/qwen-code-core', () => ({ | ||
| fetchGitDiff: vi.fn(), | ||
| fetchGitDiffHunksForFile: vi.fn(), | ||
| })); | ||
|
|
||
| const fetchGitDiffMock = vi.mocked(fetchGitDiff); | ||
| const fetchGitDiffHunksForFileMock = vi.mocked(fetchGitDiffHunksForFile); | ||
|
|
||
| function runtime( | ||
| workspaceId: string, | ||
| workspaceCwd: string, | ||
| trusted: boolean, | ||
| ): WorkspaceRuntime { | ||
| return { | ||
| workspaceId, | ||
| workspaceCwd, | ||
| primary: workspaceId === 'primary', | ||
| trusted, | ||
| bridge: { publishWorkspaceEvent: vi.fn() } as unknown as AcpSessionBridge, | ||
| } as WorkspaceRuntime; | ||
| } | ||
|
|
||
| function registry(runtimes: WorkspaceRuntime[]): WorkspaceRegistry { | ||
| return createWorkspaceRegistry(runtimes); | ||
| } | ||
|
|
||
| describe('workspace Git diff routes', () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('returns the diff file list for the bound workspace', async () => { | ||
| fetchGitDiffMock.mockResolvedValue({ | ||
| stats: { filesCount: 2, linesAdded: 5, linesRemoved: 1 }, | ||
| perFileStats: new Map([ | ||
| ['src/a.ts', { added: 4, removed: 1, isBinary: false }], | ||
| [ | ||
| 'new.txt', | ||
| { added: 1, removed: 0, isBinary: false, isUntracked: true }, | ||
| ], | ||
| ]), | ||
| }); | ||
| const app = express(); | ||
| registerWorkspaceGitDiffRoutes(app, { | ||
| boundWorkspace: '/work/main', | ||
| sendBridgeError, | ||
| }); | ||
|
|
||
| const response = await request(app).get('/workspace/git/diff'); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(response.headers['cache-control']).toBe('no-store'); | ||
| expect(response.body).toEqual({ | ||
| v: 1, | ||
| workspaceCwd: '/work/main', | ||
| available: true, | ||
| filesCount: 2, | ||
| linesAdded: 5, | ||
| linesRemoved: 1, | ||
| files: [ | ||
| { | ||
| path: 'src/a.ts', | ||
| added: 4, | ||
| removed: 1, | ||
| isBinary: false, | ||
| isUntracked: false, | ||
| isDeleted: false, | ||
| truncated: false, | ||
| }, | ||
| { | ||
| path: 'new.txt', | ||
| added: 1, | ||
| removed: 0, | ||
| isBinary: false, | ||
| isUntracked: true, | ||
| isDeleted: false, | ||
| truncated: false, | ||
| }, | ||
| ], | ||
| hiddenCount: 0, | ||
| }); | ||
| expect(fetchGitDiffMock).toHaveBeenCalledWith('/work/main'); | ||
| }); | ||
|
|
||
| it('carries the pre-rename oldPath through the file list', async () => { | ||
| fetchGitDiffMock.mockResolvedValue({ | ||
| stats: { filesCount: 1, linesAdded: 2, linesRemoved: 1 }, | ||
| perFileStats: new Map([ | ||
| [ | ||
| 'src/new.ts', | ||
| { added: 2, removed: 1, isBinary: false, oldPath: 'src/old.ts' }, | ||
| ], | ||
| ]), | ||
| }); | ||
| const app = express(); | ||
| registerWorkspaceGitDiffRoutes(app, { | ||
| boundWorkspace: '/work/main', | ||
| sendBridgeError, | ||
| }); | ||
|
|
||
| const response = await request(app).get('/workspace/git/diff'); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| // The rename must survive serialization keyed by the new path with the old | ||
| // path carried alongside, so both the Web Shell dialog and CLI can render | ||
| // `old → new`. | ||
| expect(response.body.files).toEqual([ | ||
| { | ||
| path: 'src/new.ts', | ||
| oldPath: 'src/old.ts', | ||
| added: 2, | ||
| removed: 1, | ||
| isBinary: false, | ||
| isUntracked: false, | ||
| isDeleted: false, | ||
| truncated: false, | ||
| }, | ||
| ]); | ||
| }); | ||
|
|
||
| it('reports available=false when the bound workspace is not a repo', async () => { | ||
| fetchGitDiffMock.mockResolvedValue(null); | ||
| const app = express(); | ||
| registerWorkspaceGitDiffRoutes(app, { | ||
| boundWorkspace: '/work/main', | ||
| sendBridgeError, | ||
| }); | ||
|
|
||
| const response = await request(app).get('/workspace/git/diff'); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(response.body).toMatchObject({ available: false, files: [] }); | ||
| }); | ||
|
|
||
| it('returns single-file hunks for the bound workspace', async () => { | ||
| fetchGitDiffHunksForFileMock.mockResolvedValue({ | ||
| hunks: [ | ||
| { | ||
| oldStart: 1, | ||
| oldLines: 2, | ||
| newStart: 1, | ||
| newLines: 2, | ||
| lines: ['-one', '+ONE', ' two'], | ||
| }, | ||
| ], | ||
| truncated: false, | ||
| }); | ||
| const app = express(); | ||
| registerWorkspaceGitDiffRoutes(app, { | ||
| boundWorkspace: '/work/main', | ||
| sendBridgeError, | ||
| }); | ||
|
|
||
| const response = await request(app).get( | ||
| '/workspace/git/diff/file?path=src/a.ts', | ||
| ); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| // `truncated` is intentionally ABSENT (not false) on an untruncated diff. | ||
| expect(response.body).toEqual({ | ||
| v: 1, | ||
| workspaceCwd: '/work/main', | ||
| path: 'src/a.ts', | ||
| available: true, | ||
| hunks: [ | ||
| { | ||
| oldStart: 1, | ||
| oldLines: 2, | ||
| newStart: 1, | ||
| newLines: 2, | ||
| lines: ['-one', '+ONE', ' two'], | ||
| }, | ||
| ], | ||
| }); | ||
| expect(fetchGitDiffHunksForFileMock).toHaveBeenCalledWith( | ||
| '/work/main', | ||
| 'src/a.ts', | ||
| undefined, | ||
| ); | ||
| }); | ||
|
|
||
| it('forwards the oldPath query to fetchGitDiffHunksForFile', async () => { | ||
| fetchGitDiffHunksForFileMock.mockResolvedValue({ | ||
| hunks: [], | ||
| truncated: false, | ||
| }); | ||
| const app = express(); | ||
| registerWorkspaceGitDiffRoutes(app, { | ||
| boundWorkspace: '/work/main', | ||
| sendBridgeError, | ||
| }); | ||
|
|
||
| const response = await request(app).get( | ||
| '/workspace/git/diff/file?path=src/new.ts&oldPath=src/old.ts', | ||
| ); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| // The route must parse ?oldPath= and forward it so the core diff is | ||
| // computed old→new (rename detection) instead of new-path-as-added. | ||
| expect(fetchGitDiffHunksForFileMock).toHaveBeenCalledWith( | ||
| '/work/main', | ||
| 'src/new.ts', | ||
| 'src/old.ts', | ||
| ); | ||
| }); | ||
|
|
||
| it('surfaces a traversal oldPath as unavailable via core normalization', async () => { | ||
| // The route forwards oldPath verbatim; fetchGitDiffHunksForFile rejects `..` | ||
| // traversal (returns null) and the route surfaces that as available:false | ||
| // rather than erroring or escaping the workspace. | ||
| fetchGitDiffHunksForFileMock.mockResolvedValue(null); | ||
| const app = express(); | ||
| registerWorkspaceGitDiffRoutes(app, { | ||
| boundWorkspace: '/work/main', | ||
| sendBridgeError, | ||
| }); | ||
|
|
||
| const response = await request(app).get( | ||
| '/workspace/git/diff/file?path=ok.ts&oldPath=../../etc/passwd', | ||
| ); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(fetchGitDiffHunksForFileMock).toHaveBeenCalledWith( | ||
| '/work/main', | ||
| 'ok.ts', | ||
| '../../etc/passwd', | ||
| ); | ||
| expect(response.body.available).toBe(false); | ||
| expect(response.body.hunks).toEqual([]); | ||
| }); | ||
|
|
||
| it('surfaces the truncated flag when the diff was capped', async () => { | ||
| fetchGitDiffHunksForFileMock.mockResolvedValue({ | ||
| hunks: [ | ||
| { | ||
| oldStart: 0, | ||
| oldLines: 0, | ||
| newStart: 1, | ||
| newLines: 1, | ||
| lines: ['+head'], | ||
| }, | ||
| ], | ||
| truncated: true, | ||
| }); | ||
| const app = express(); | ||
| registerWorkspaceGitDiffRoutes(app, { | ||
| boundWorkspace: '/work/main', | ||
| sendBridgeError, | ||
| }); | ||
|
|
||
| const response = await request(app).get( | ||
| '/workspace/git/diff/file?path=big.txt', | ||
| ); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(response.body).toMatchObject({ available: true, truncated: true }); | ||
| }); | ||
|
|
||
| it('reports available=false when the file has no diff', async () => { | ||
| fetchGitDiffHunksForFileMock.mockResolvedValue(null); | ||
| const app = express(); | ||
| registerWorkspaceGitDiffRoutes(app, { | ||
| boundWorkspace: '/work/main', | ||
| sendBridgeError, | ||
| }); | ||
|
|
||
| const response = await request(app).get( | ||
| '/workspace/git/diff/file?path=src/a.ts', | ||
| ); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(response.body).toMatchObject({ available: false, hunks: [] }); | ||
| }); | ||
|
|
||
| it('rejects a missing path query with 400', async () => { | ||
| const app = express(); | ||
| registerWorkspaceGitDiffRoutes(app, { | ||
| boundWorkspace: '/work/main', | ||
| sendBridgeError, | ||
| }); | ||
|
|
||
| const response = await request(app).get('/workspace/git/diff/file'); | ||
|
|
||
| expect(response.status).toBe(400); | ||
| expect(response.body).toMatchObject({ errorKind: 'parse_error' }); | ||
| expect(fetchGitDiffHunksForFileMock).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('uses the selected trusted workspace runtime for the file route', async () => { | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] This proves selected-runtime ownership only for the file route. A list-handler regression to the bound/primary cwd would stay green. Add the analogous qualified list request and assert — Codex GPT-5 via Qwen Code /review |
||
| fetchGitDiffHunksForFileMock.mockResolvedValue({ | ||
| hunks: [], | ||
| truncated: false, | ||
| }); | ||
| const app = express(); | ||
| const primary = runtime('primary', '/work/main', true); | ||
| const secondary = runtime('secondary', '/work/secondary', true); | ||
| registerWorkspaceQualifiedGitDiffRoutes(app, { | ||
| workspaceRegistry: registry([primary, secondary]), | ||
| sendBridgeError, | ||
| }); | ||
|
|
||
| const response = await request(app).get( | ||
| '/workspaces/secondary/git/diff/file?path=b.ts', | ||
| ); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(fetchGitDiffHunksForFileMock).toHaveBeenCalledWith( | ||
| '/work/secondary', | ||
| 'b.ts', | ||
| undefined, | ||
| ); | ||
| }); | ||
|
|
||
| it('rejects an untrusted workspace before diffing', async () => { | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The trust regression covers only — Codex GPT-5 via Qwen Code /review |
||
| const app = express(); | ||
| const primary = runtime('primary', '/work/main', true); | ||
| const untrusted = runtime('untrusted', '/work/untrusted', false); | ||
| registerWorkspaceQualifiedGitDiffRoutes(app, { | ||
| workspaceRegistry: registry([primary, untrusted]), | ||
| sendBridgeError, | ||
| }); | ||
|
|
||
| const response = await request(app).get('/workspaces/untrusted/git/diff'); | ||
|
wenshao marked this conversation as resolved.
|
||
|
|
||
| expect(response.status).toBe(403); | ||
| expect(response.body.code).toBe('untrusted_workspace'); | ||
| expect(fetchGitDiffMock).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('rejects an untrusted workspace on the single-file endpoint too', async () => { | ||
| const app = express(); | ||
| const primary = runtime('primary', '/work/main', true); | ||
| const untrusted = runtime('untrusted', '/work/untrusted', false); | ||
| registerWorkspaceQualifiedGitDiffRoutes(app, { | ||
| workspaceRegistry: registry([primary, untrusted]), | ||
| sendBridgeError, | ||
| }); | ||
|
|
||
| const response = await request(app).get( | ||
| '/workspaces/untrusted/git/diff/file?path=a.ts', | ||
| ); | ||
|
|
||
| expect(response.status).toBe(403); | ||
| expect(response.body.code).toBe('untrusted_workspace'); | ||
| expect(fetchGitDiffHunksForFileMock).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('rejects an unknown workspace', async () => { | ||
| const app = express(); | ||
| const primary = runtime('primary', '/work/main', true); | ||
| registerWorkspaceQualifiedGitDiffRoutes(app, { | ||
| workspaceRegistry: registry([primary]), | ||
| sendBridgeError, | ||
| }); | ||
|
|
||
| const response = await request(app).get('/workspaces/missing/git/diff'); | ||
|
|
||
| expect(response.status).toBe(400); | ||
| expect(response.body).toMatchObject({ code: 'workspace_mismatch' }); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] No route-level test verifies that a traversal-style
oldPathquery parameter is rejected or handled safely. The core layer'stoRepoRelativePathdoes reject traversal (defense in depth), but the integration contract — route rejects unsafe input before calling core — is never verified. A future core refactor that loosenstoRepoRelativePathcould silently open a traversal on this route with no test to catch it.— qwen3.7-max via Qwen Code /review