Skip to content

Commit 7f8c697

Browse files
doudouOUCdoudouOUC
committed
feat(serve): wire ACP-side preflight cells (#4175 Wave 3 PR 13)
Populate the six ACP-level preflight cells inside the ACP child so `/workspace/preflight` returns real values for live sessions. - `extMethod(qwen/status/workspace/preflight, ...)` dispatches to a new `buildAcpPreflightCells(config)` private method. - Five cell builders, each returning a `ServePreflightCell` with `locality: 'acp'`: - `auth`: `validateAuthMethod(authType, config)` returning non-null string → `auth_env_error`. Missing auth method → warning. Throws classified via `mapDomainErrorToErrorKind` with `auth_env_error` fallback. - `mcp_discovery`: rolls up `getMCPDiscoveryState()` + per-server `getMCPServerStatus(name)` counts. `connecting > 0` or in-progress discovery → warning + `init_timeout`; `disconnected > 0` post-discovery → error + `protocol_error`. - `skills`: `SkillManager.listSkills()`; SkillError throws are mapped via the helper (`PARSE_ERROR` → `parse_error`, `FILE_ERROR` → `missing_file`). - `providers`: `getAllConfiguredModels()`; empty list with a configured `authType` → warning + `auth_env_error`. ModelConfigError throws map to `auth_env_error`. - `tool_registry`: null registry → error + `protocol_error`. Otherwise surfaces tool count. - `egress`: stays `not_started`. PR 14 plugs in the real probe. - `errorCell` private helper extended with optional `errorKind` parameter; defaults to `mapDomainErrorToErrorKind(error)` so existing call sites (`mcp` / `skills` / `providers` envelope errors) automatically gain classification. Tests: 2 new acpAgent tests — preflight returns the six expected ACP cells with correct locality + statuses; preflight surfaces a `SkillError` (`PARSE_ERROR`) on the `skills` cell as `errorKind: 'parse_error'`. The core `vi.mock` block adds a SkillError class for `instanceof` matching inside `mapDomainErrorToErrorKind`.
1 parent 5441387 commit 7f8c697

3 files changed

Lines changed: 588 additions & 1 deletion

File tree

‎packages/cli/src/acp-integration/acpAgent.test.ts‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,17 @@ vi.mock('@qwen-code/qwen-code-core', () => ({
103103
CONNECTING: 'connecting',
104104
CONNECTED: 'connected',
105105
},
106+
// SkillError is referenced by status.ts's `mapDomainErrorToErrorKind`
107+
// helper for `instanceof` classification. The mock must surface it as
108+
// a real class so that `instanceof` works inside the helper.
109+
SkillError: class SkillError extends Error {
110+
code: string;
111+
constructor(message: string, code: string) {
112+
super(message);
113+
this.name = 'SkillError';
114+
this.code = code;
115+
}
116+
},
106117
getMCPDiscoveryState: vi.fn().mockReturnValue('completed'),
107118
getMCPServerStatus: vi.fn().mockReturnValue('connected'),
108119
MCPServerConfig: vi.fn().mockImplementation((...args: unknown[]) => ({
@@ -1095,6 +1106,172 @@ describe('QwenAgent MCP SSE/HTTP support', () => {
10951106
await agentPromise;
10961107
});
10971108

1109+
it('extMethod qwen/status/workspace/preflight returns 6 ACP-side cells', async () => {
1110+
mockConfig = {
1111+
...mockConfig,
1112+
getTargetDir: vi.fn().mockReturnValue('/work/status'),
1113+
getMcpServers: vi.fn().mockReturnValue({}),
1114+
getAuthType: vi.fn().mockReturnValue('qwen'),
1115+
getActiveRuntimeModelSnapshot: vi.fn().mockReturnValue(undefined),
1116+
getModel: vi.fn().mockReturnValue('qwen-plus'),
1117+
getSkillManager: vi.fn().mockReturnValue({
1118+
listSkills: vi.fn().mockResolvedValue([]),
1119+
}),
1120+
getAllConfiguredModels: vi.fn().mockReturnValue([
1121+
{
1122+
id: 'qwen-plus',
1123+
label: 'Qwen Plus',
1124+
authType: 'qwen',
1125+
baseUrl: 'https://api.example.com',
1126+
isRuntimeModel: false,
1127+
},
1128+
]),
1129+
getToolRegistry: vi
1130+
.fn()
1131+
.mockReturnValue({ getAllTools: () => [{ name: 'rg' }] }),
1132+
} as unknown as Config;
1133+
1134+
const agentPromise = runAcpAgent(
1135+
mockConfig,
1136+
makeSessionSettings(),
1137+
mockArgv,
1138+
);
1139+
await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined());
1140+
const agent = capturedAgentFactory!({
1141+
get closed() {
1142+
return mockConnectionState.promise;
1143+
},
1144+
}) as AgentLike;
1145+
1146+
const preflight = (await agent.extMethod(
1147+
SERVE_STATUS_EXT_METHODS.workspacePreflight,
1148+
{},
1149+
)) as { cells: Array<{ kind: string; locality: string; status: string }> };
1150+
1151+
expect(preflight.cells.map((c) => c.kind)).toEqual([
1152+
'auth',
1153+
'mcp_discovery',
1154+
'skills',
1155+
'providers',
1156+
'tool_registry',
1157+
'egress',
1158+
]);
1159+
for (const cell of preflight.cells) {
1160+
expect(cell.locality).toBe('acp');
1161+
}
1162+
expect(preflight.cells.find((c) => c.kind === 'egress')?.status).toBe(
1163+
'not_started',
1164+
);
1165+
expect(
1166+
preflight.cells.find((c) => c.kind === 'mcp_discovery')?.status,
1167+
).toBe('ok');
1168+
expect(
1169+
preflight.cells.find((c) => c.kind === 'tool_registry')?.status,
1170+
).toBe('ok');
1171+
1172+
mockConnectionState.resolve();
1173+
await agentPromise;
1174+
});
1175+
1176+
it('extMethod preflight surfaces SkillError as parse_error errorKind', async () => {
1177+
const skillError = new (
1178+
await import('@qwen-code/qwen-code-core')
1179+
).SkillError('bad frontmatter', 'PARSE_ERROR');
1180+
mockConfig = {
1181+
...mockConfig,
1182+
getTargetDir: vi.fn().mockReturnValue('/work/status'),
1183+
getMcpServers: vi.fn().mockReturnValue({}),
1184+
getAuthType: vi.fn().mockReturnValue('qwen'),
1185+
getModel: vi.fn().mockReturnValue('qwen-plus'),
1186+
getSkillManager: vi.fn().mockReturnValue({
1187+
listSkills: vi.fn().mockRejectedValue(skillError),
1188+
}),
1189+
getAllConfiguredModels: vi.fn().mockReturnValue([]),
1190+
getToolRegistry: vi.fn().mockReturnValue({ getAllTools: () => [] }),
1191+
} as unknown as Config;
1192+
1193+
const agentPromise = runAcpAgent(
1194+
mockConfig,
1195+
makeSessionSettings(),
1196+
mockArgv,
1197+
);
1198+
await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined());
1199+
const agent = capturedAgentFactory!({
1200+
get closed() {
1201+
return mockConnectionState.promise;
1202+
},
1203+
}) as AgentLike;
1204+
1205+
const preflight = (await agent.extMethod(
1206+
SERVE_STATUS_EXT_METHODS.workspacePreflight,
1207+
{},
1208+
)) as {
1209+
cells: Array<{
1210+
kind: string;
1211+
status: string;
1212+
errorKind?: string;
1213+
}>;
1214+
};
1215+
const skillsCell = preflight.cells.find((c) => c.kind === 'skills');
1216+
expect(skillsCell?.status).toBe('error');
1217+
expect(skillsCell?.errorKind).toBe('parse_error');
1218+
1219+
mockConnectionState.resolve();
1220+
await agentPromise;
1221+
});
1222+
1223+
it('extMethod preflight returns 6 cells even when a Config getter throws synchronously', async () => {
1224+
// Regression guard: `getSkillManager()` is invoked by `buildSkillsPreflightCell`.
1225+
// Before the fix it ran OUTSIDE the try block, so a sync throw escaped
1226+
// out of `buildAcpPreflightCells` → the whole envelope 500'd. The
1227+
// wrapped variant should produce a `skills` error cell instead and
1228+
// keep the other five cells intact.
1229+
mockConfig = {
1230+
...mockConfig,
1231+
getTargetDir: vi.fn().mockReturnValue('/work/status'),
1232+
getMcpServers: vi.fn().mockReturnValue({}),
1233+
getAuthType: vi.fn().mockReturnValue('qwen'),
1234+
getModel: vi.fn().mockReturnValue('qwen-plus'),
1235+
getSkillManager: vi.fn(() => {
1236+
throw new Error('config getter exploded mid-eval');
1237+
}),
1238+
getAllConfiguredModels: vi.fn().mockReturnValue([]),
1239+
getToolRegistry: vi.fn().mockReturnValue({ getAllTools: () => [] }),
1240+
} as unknown as Config;
1241+
1242+
const agentPromise = runAcpAgent(
1243+
mockConfig,
1244+
makeSessionSettings(),
1245+
mockArgv,
1246+
);
1247+
await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined());
1248+
const agent = capturedAgentFactory!({
1249+
get closed() {
1250+
return mockConnectionState.promise;
1251+
},
1252+
}) as AgentLike;
1253+
1254+
const preflight = (await agent.extMethod(
1255+
SERVE_STATUS_EXT_METHODS.workspacePreflight,
1256+
{},
1257+
)) as { cells: Array<{ kind: string; status: string; error?: string }> };
1258+
1259+
expect(preflight.cells.map((c) => c.kind)).toEqual([
1260+
'auth',
1261+
'mcp_discovery',
1262+
'skills',
1263+
'providers',
1264+
'tool_registry',
1265+
'egress',
1266+
]);
1267+
const skillsCell = preflight.cells.find((c) => c.kind === 'skills');
1268+
expect(skillsCell?.status).toBe('error');
1269+
expect(skillsCell?.error).toContain('config getter exploded');
1270+
1271+
mockConnectionState.resolve();
1272+
await agentPromise;
1273+
});
1274+
10981275
it('provider status marks current only for matching models', async () => {
10991276
mockConfig = {
11001277
...mockConfig,

0 commit comments

Comments
 (0)