Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
187 changes: 187 additions & 0 deletions packages/cli/src/serve/routes/workspace-git.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
/**
* @license
* Copyright 2026 Qwen Team
* SPDX-License-Identifier: Apache-2.0
*/

import express from 'express';
import request from 'supertest';
import { describe, expect, it, vi } from 'vitest';
import type { AcpSessionBridge } from '../acp-session-bridge.js';
import { sendBridgeError } from '../server/error-response.js';
import type { WorkspaceGitState } from '../workspace-git-state.js';
import type {
WorkspaceRegistry,
WorkspaceRuntime,
} from '../workspace-registry.js';
import {
registerWorkspaceGitRoutes,
registerWorkspaceQualifiedGitRoutes,
} from './workspace-git.js';

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 {
primary: runtimes[0]!,
list: () => runtimes,
getByWorkspaceCwd: (cwd) =>
runtimes.find((item) => item.workspaceCwd === cwd),
getByWorkspaceId: (id) => runtimes.find((item) => item.workspaceId === id),
resolveWorkspaceCwd: (cwd) =>
cwd === undefined
? runtimes[0]
: runtimes.find((item) => item.workspaceCwd === cwd),
resolveLiveSessionOwner: () => ({ kind: 'not_found' }),
add: () => {},
};
}

describe('workspace Git routes', () => {
it('returns Git status for the bound workspace', async () => {
const app = express();
const bridge = runtime('primary', '/work/main', true).bridge;
const getStatus = vi.fn(async () => ({
v: 1 as const,
workspaceCwd: '/work/main',
branch: 'main',
}));
registerWorkspaceGitRoutes(app, {
boundWorkspace: '/work/main',
bridge,
gitState: { getStatus } as unknown as WorkspaceGitState,
sendBridgeError,
});

const response = await request(app).get('/workspace/git');

expect(response.status).toBe(200);
expect(response.body).toEqual({
v: 1,
workspaceCwd: '/work/main',
branch: 'main',
});
expect(getStatus).toHaveBeenCalledWith('/work/main', bridge);
});

it('returns a structured error when bound Git status fails', async () => {
const app = express();
const bridge = runtime('primary', '/work/main', true).bridge;
const getStatus = vi.fn(async () => {
throw Object.assign(new Error('git failed'), {
code: 'git_status_failed',
});
});
registerWorkspaceGitRoutes(app, {
boundWorkspace: '/work/main',
bridge,
gitState: { getStatus } as unknown as WorkspaceGitState,
sendBridgeError,
});

const response = await request(app).get('/workspace/git');

expect(response.status).toBe(500);
expect(response.body).toMatchObject({
error: 'git failed',
code: 'git_status_failed',
});
});

it('uses the selected trusted workspace runtime', async () => {
const app = express();
const primary = runtime('primary', '/work/main', true);
const secondary = runtime('secondary', '/work/secondary', true);
const getStatus = vi.fn(async () => ({
v: 1 as const,
workspaceCwd: secondary.workspaceCwd,
branch: 'feature/web-shell',
}));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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

registerWorkspaceQualifiedGitRoutes(app, {
workspaceRegistry: registry([primary, secondary]),
gitState: { getStatus } as unknown as WorkspaceGitState,
sendBridgeError,
});

const response = await request(app).get('/workspaces/secondary/git');

expect(response.status).toBe(200);
expect(response.body.branch).toBe('feature/web-shell');
expect(getStatus).toHaveBeenCalledWith(
secondary.workspaceCwd,
secondary.bridge,
);
});

it('rejects an untrusted workspace before reading Git status', async () => {
const app = express();
const primary = runtime('primary', '/work/main', true);
const untrusted = runtime('untrusted', '/work/untrusted', false);
const getStatus = vi.fn();
registerWorkspaceQualifiedGitRoutes(app, {
workspaceRegistry: registry([primary, untrusted]),
gitState: { getStatus } as unknown as WorkspaceGitState,
sendBridgeError,
});

const response = await request(app).get('/workspaces/untrusted/git');

expect(response.status).toBe(403);
expect(response.body.code).toBe('untrusted_workspace');
expect(getStatus).not.toHaveBeenCalled();
});

it('rejects an unknown workspace before reading Git status', async () => {
const app = express();
const primary = runtime('primary', '/work/main', true);
const getStatus = vi.fn();
registerWorkspaceQualifiedGitRoutes(app, {
workspaceRegistry: registry([primary]),
gitState: { getStatus } as unknown as WorkspaceGitState,
sendBridgeError,
});

const response = await request(app).get('/workspaces/missing/git');

expect(response.status).toBe(400);
expect(response.body).toMatchObject({
code: 'workspace_mismatch',
});
expect(getStatus).not.toHaveBeenCalled();
});

it('returns a structured error when qualified Git status fails', async () => {
const app = express();
const primary = runtime('primary', '/work/main', true);
const getStatus = vi.fn(async () => {
throw Object.assign(new Error('qualified git failed'), {
data: { reason: 'watcher' },
});
});
registerWorkspaceQualifiedGitRoutes(app, {
workspaceRegistry: registry([primary]),
gitState: { getStatus } as unknown as WorkspaceGitState,
sendBridgeError,
});

const response = await request(app).get('/workspaces/primary/git');

expect(response.status).toBe(500);
expect(response.body).toMatchObject({
error: 'qualified git failed',
data: { reason: 'watcher' },
});
});
});
72 changes: 72 additions & 0 deletions packages/cli/src/serve/routes/workspace-git.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/**
* @license
* Copyright 2026 Qwen Team
* SPDX-License-Identifier: Apache-2.0
*/

import type { Application, Request, Response } from 'express';
import type { AcpSessionBridge } from '../acp-session-bridge.js';
import type { SendBridgeError } from '../server/error-response.js';
import type { WorkspaceGitState } from '../workspace-git-state.js';
import type {
WorkspaceRegistry,
WorkspaceRuntime,
} from '../workspace-registry.js';
import {
requireTrustedWorkspaceRuntime,
resolveWorkspaceRuntimeFromParam,
} from '../workspace-route-runtime.js';

export function registerWorkspaceGitRoutes(
app: Application,
deps: {
boundWorkspace: string;
bridge: AcpSessionBridge;
gitState: WorkspaceGitState;
sendBridgeError: SendBridgeError;
},
): void {
app.get('/workspace/git', async (_req, res) => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Suggested change
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

try {
res
.status(200)
.json(await deps.gitState.getStatus(deps.boundWorkspace, deps.bridge));
} catch (err) {
deps.sendBridgeError(res, err, { route: 'GET /workspace/git' });
}
});
}

function resolveTrustedRuntime(
registry: WorkspaceRegistry,
req: Request,
res: Response,
): WorkspaceRuntime | null {
const runtime = resolveWorkspaceRuntimeFromParam(registry, req, res);
if (!runtime) return null;
return requireTrustedWorkspaceRuntime(runtime, res) ? runtime : null;
}

export function registerWorkspaceQualifiedGitRoutes(
app: Application,
deps: {
workspaceRegistry: WorkspaceRegistry;
gitState: WorkspaceGitState;
sendBridgeError: SendBridgeError;
},
): void {
app.get('/workspaces/:workspace/git', async (req, res) => {
const runtime = resolveTrustedRuntime(deps.workspaceRegistry, req, res);
if (!runtime) return;
const route = 'GET /workspaces/:workspace/git';
try {
res
.status(200)
.json(
await deps.gitState.getStatus(runtime.workspaceCwd, runtime.bridge),
);
} catch (err) {
deps.sendBridgeError(res, err, { route });
}
});
}
3 changes: 3 additions & 0 deletions packages/cli/src/serve/run-qwen-serve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4430,6 +4430,9 @@ export async function runQwenServe(
(
app.locals as { stopScheduledTaskKeepalive?: () => void }
).stopScheduledTaskKeepalive?.();
(
app.locals as { stopWorkspaceGitState?: () => void }
).stopWorkspaceGitState?.();
// Same rationale for the create_sub_session launchers: stop accepting
// new sub-session spawns before the bridges are torn down. Calls
// every workspace's launcher stop (primary + secondaries).
Expand Down
19 changes: 19 additions & 0 deletions packages/cli/src/serve/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,11 @@ import {
} from './routes/workspace-lifecycle.js';
import { registerWorkspaceManagementRoutes } from './routes/workspace-management.js';
import type { WorkspaceRegistrationStore } from './workspace-registration-store.js';
import {
registerWorkspaceGitRoutes,
registerWorkspaceQualifiedGitRoutes,
} from './routes/workspace-git.js';
import { WorkspaceGitState } from './workspace-git-state.js';
import {
registerWorkspaceMcpControlRoutes,
registerWorkspaceQualifiedMcpControlRoutes,
Expand Down Expand Up @@ -803,6 +808,9 @@ export function createServeApp(
const primaryBridge = primaryRuntime.bridge;
const primaryWorkspace = primaryRuntime.workspaceService;
const primaryRouteFileSystemFactory = primaryRuntime.routeFileSystemFactory;
const workspaceGitState = new WorkspaceGitState();
(app.locals as { stopWorkspaceGitState?: () => void }).stopWorkspaceGitState =
() => workspaceGitState.dispose();
const workspaceQualifiedAcpEnabled = resolveAcpHttpEnabled();

// Order matters: rejection guards (CORS / Host allowlist / bearer auth)
Expand Down Expand Up @@ -1002,6 +1010,17 @@ export function createServeApp(
workspaceRegistry,
sendBridgeError,
});
registerWorkspaceGitRoutes(app, {
boundWorkspace: primaryBoundWorkspace,
bridge: primaryBridge,
gitState: workspaceGitState,
sendBridgeError,
});
registerWorkspaceQualifiedGitRoutes(app, {
workspaceRegistry,
gitState: workspaceGitState,
sendBridgeError,
});

// Workspace memory + agents CRUD routes.
mountWorkspaceMemoryRoutes(app, {
Expand Down
Loading
Loading