Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
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
11 changes: 11 additions & 0 deletions packages/cli/src/serve/capabilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,17 @@ export const SERVE_CAPABILITY_REGISTRY = {
workspace_mcp: { since: 'v1' },
workspace_skills: { since: 'v1' },
workspace_providers: { since: 'v1' },
// Issue #4175 PR 16: workspace memory CRUD (`GET/POST /workspace/memory`).
// Daemon exposes hierarchical QWEN.md state and accepts append/replace
// writes scoped to either the bound workspace or the global ~/.qwen
// directory. Mutation path is gated by the centralized mutation gate.
workspace_memory: { since: 'v1' },
// Issue #4175 PR 16: workspace agents CRUD (`GET/POST /workspace/agents`
// + `GET/POST/DELETE /workspace/agents/:agentType`). Wraps
// `SubagentManager` over HTTP so remote clients can list / read /
// create / update / delete project- and user-level subagent
// definitions. Built-in / extension agents stay read-only.
workspace_agents: { since: 'v1' },
workspace_env: { since: 'v1' },
workspace_preflight: { since: 'v1' },
session_context: { since: 'v1' },
Expand Down
32 changes: 32 additions & 0 deletions packages/cli/src/serve/debugMode.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* @license
* Copyright 2025 Qwen Team
* SPDX-License-Identifier: Apache-2.0
*/

/**
* Returns whether the daemon should expose verbose error context in
* HTTP responses. Mirrors `isServeDebugLoggingEnabled` in
* `httpAcpBridge.ts` (which gates stderr-side debug log output).
*
* Extracted into its own module in fold-in 2i so route files
* (`workspaceMemory.ts`, `workspaceAgents.ts`, future Wave 4 mutation
* routes) can share one canonical predicate when deciding whether to
* include `errorMessage` / `filePath` in their response bodies.
* Without the toggle, error responses carry only structured fields
* (`code` / `scope` / `mode` / `osCode` / ...) so absolute filesystem
* paths from Node's fs error messages don't leak to authenticated
* remote callers.
*
* Accepts any non-falsy value for the env var; explicit literals
* `"0" / "false" / "off" / "no"` (case-insensitive, with surrounding
* whitespace trimmed) disable. Matches the bridge's existing
* `isServeDebugLoggingEnabled` semantics so the two toggles move in
* lockstep — operators set `QWEN_SERVE_DEBUG=1` and get both stderr
* verbosity and response-body detail.
*/
export function isServeDebugMode(): boolean {
const value = process.env['QWEN_SERVE_DEBUG'];
if (!value) return false;
return !['0', 'false', 'off', 'no'].includes(value.trim().toLowerCase());
}
2 changes: 1 addition & 1 deletion packages/cli/src/serve/fs/contract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { canonicalizeWorkspace, resolveWithinWorkspace } from './paths.js';
// fix and exists because the auto-fix at commit 7b0db4c3a promoted the
// whole line to `import type`, erasing `isFsError` at runtime and
// failing 11 tests in this file alone.

import { isFsError, type FsError, type FsErrorKind } from './errors.js';

/**
Expand Down
93 changes: 93 additions & 0 deletions packages/cli/src/serve/httpAcpBridge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4968,4 +4968,97 @@ describe('createHttpAcpBridge', () => {
await bridge.shutdown();
});
});

describe('publishWorkspaceEvent + knownClientIds (issue #4175 PR 16)', () => {
it('fans out a workspace event onto every active session bus', async () => {
const factory: ChannelFactory = async () => makeChannel().channel;
const bridge = makeBridge({ channelFactory: factory });
const a = await bridge.spawnOrAttach({
workspaceCwd: WS_A,
sessionScope: 'thread',
});
const b = await bridge.spawnOrAttach({
workspaceCwd: WS_A,
sessionScope: 'thread',
});

const aFrames: BridgeEvent[] = [];
const bFrames: BridgeEvent[] = [];
const collect = async (
sessionId: string,
target: BridgeEvent[],
signal: AbortSignal,
) => {
for await (const frame of bridge.subscribeEvents(sessionId, {
signal,
})) {
target.push(frame);
}
};
const ctrl = new AbortController();
const tasks = Promise.all([
collect(a.sessionId, aFrames, ctrl.signal),
collect(b.sessionId, bFrames, ctrl.signal),
]);
// Yield once so the subscribe handlers register.
await new Promise((resolve) => setImmediate(resolve));

bridge.publishWorkspaceEvent({
type: 'memory_changed',
data: {
scope: 'workspace',
filePath: '/work/QWEN.md',
mode: 'append',
bytesWritten: 5,
},
});

// Yield so the bus's async push reaches both subscribers.
await new Promise((resolve) => setImmediate(resolve));

expect(aFrames.some((f) => f.type === 'memory_changed')).toBe(true);
expect(bFrames.some((f) => f.type === 'memory_changed')).toBe(true);

ctrl.abort();
await tasks.catch(() => {});
await bridge.shutdown();
});

it('returns an empty knownClientIds set when no clients are attached', async () => {
const factory: ChannelFactory = async () => makeChannel().channel;
const bridge = makeBridge({ channelFactory: factory });
const ids = bridge.knownClientIds();
expect(ids).toBeInstanceOf(Set);
expect(ids.size).toBe(0);
await bridge.shutdown();
});

it('aggregates clientIds across sessions in knownClientIds()', async () => {
const factory: ChannelFactory = async () => makeChannel().channel;
const bridge = makeBridge({ channelFactory: factory });
const a = await bridge.spawnOrAttach({
workspaceCwd: WS_A,
sessionScope: 'thread',
});
const b = await bridge.spawnOrAttach({
workspaceCwd: WS_A,
sessionScope: 'thread',
});

const ids = bridge.knownClientIds();
expect(ids.size).toBe(2);
expect(ids.has(a.clientId!)).toBe(true);
expect(ids.has(b.clientId!)).toBe(true);

// Snapshot semantics: mutating the returned Set must not
// affect future calls. The interface returns
// `ReadonlySet<string>` so cast through `Set<string>` to attempt
// a mutation; the live registry must stay intact.
(ids as Set<string>).delete(a.clientId!);
const fresh = bridge.knownClientIds();
expect(fresh.size).toBe(2);

await bridge.shutdown();
});
});
});
77 changes: 77 additions & 0 deletions packages/cli/src/serve/httpAcpBridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,28 @@ export interface HttpAcpBridge {
*/
getHeartbeatState(sessionId: string): BridgeHeartbeatState | undefined;

/**
* Issue #4175 PR 16: workspace-level event fan-out for mutations that
* change daemon-wide state (memory writes, agent CRUD). Publishes the
* event onto every live session's `EventBus` so SSE subscribers
* observe it through the existing per-session stream rather than a
* new workspace-level channel. Best-effort per session — a closed
* bus is silently skipped (mirrors the `permission_resolved`
* try/catch). When zero sessions are active the event drops on the
* floor; the route's read-after-write contract remains correct.
*/
publishWorkspaceEvent(event: Omit<BridgeEvent, 'id' | 'v'>): void;

/**
* Issue #4175 PR 16: union of every live session's `clientIds`. Used
* by workspace-level mutation routes (memory write, agent CRUD) to
* validate the optional `X-Qwen-Client-Id` header without requiring
* a session id. Returns a snapshot — callers must not mutate.
* Wave 5 PR 24 will replace this with a workspace-scoped client
* registry decoupled from sessions.
*/
knownClientIds(): ReadonlySet<string>;

/**
* Read daemon-runtime MCP status for the bound workspace. Does not spawn an
* ACP child when the daemon is idle; idle daemons return initialized:false.
Expand Down Expand Up @@ -3235,6 +3257,61 @@ export function createHttpAcpBridge(opts: BridgeOptions): HttpAcpBridge {
};
},

publishWorkspaceEvent(event) {

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] publishWorkspaceEvent 缺少 workspace discriminator 参数 — 未来多 workspace 支持的信息泄露隐患

该方法遍历所有活跃 session 并向每个 bus 发布,无按 session 的 workspace 范围检查。当前 daemon 绑定到单一 workspace(boundWorkspace),因此风险为零。但如果未来添加多 workspace 支持而未改造此 fan-out 路径,workspace A 的 memory 写入会将 memory_changed 事件泄露给 workspace B 的 SSE 订阅者。

Suggested change
publishWorkspaceEvent(event) {
// 添加 workspaceCwd 参数并在 fan-out 循环中过滤:
// if (entry.workspaceCwd !== workspaceCwd) continue;

— DeepSeek/deepseek-v4-pro via Qwen Code /review

// Issue #4175 PR 16. Workspace-level mutations (memory writes /
// agent CRUD) need a fan-out path that doesn't require a session
// id. Iterate every live session's bus best-effort — a closed bus
// (mid-shutdown, or evicted under load) is silently skipped, same
Comment thread
doudouOUC marked this conversation as resolved.
// posture as `permission_resolved` at line 1717.
//
// We deliberately do NOT track delivery success per session here:
// the route handler's contract is "read-after-write" and any SSE
// subscriber that misses the event can re-fetch via the route's
// GET sibling. Stage 5 PR 24 PermissionMediator can layer a
// proper workspace event bus on top if adapters need stricter
// delivery semantics.
//
// Per-entry exceptions go to stderr in normal operation, but
// are downgraded to the debug channel when `shuttingDown` is
// true. `EventBus.publish` is documented never to throw (BX9_p
// contract at eventBus.ts:186), so anything landing here in
// normal ops is by definition unexpected — silencing it via
// QWEN_SERVE_DEBUG would let a true regression succeed at the
// route layer (200 OK) while SSE subscribers stop seeing
// events. The shutdown gate keeps the common race noise out of
// the production log without hiding actual bugs.
for (const entry of byId.values()) {
try {
entry.events.publish(event);
} catch (err) {
const detail =
`publishWorkspaceEvent: bus publish failed for session ` +
`${JSON.stringify(entry.sessionId)} (type=${event.type}): ` +
`${err instanceof Error ? err.message : String(err)}`;
if (shuttingDown) {
writeServeDebugLine(detail);
} else {
writeStderrLine(`qwen serve: ${detail}`);
}
}
}
},

knownClientIds() {

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] knownClientIds() 为单次 Set.has() 检查构建并填充完整 Set,每次 mutation 请求都会触发

resolveWorkspaceClientId(memory route)和 resolveOriginatorClientId(agents route)在每次 POST/DELETE 时都会调用该方法。仅需一次成员检查,却遍历所有 session 及其全部 clientIds

Suggested change
knownClientIds() {
// 添加直接成员检查方法:
// isKnownClientId(clientId: string): boolean
// 在迭代 session 时,首次匹配即返回,避免完整的 Set 分配。

— DeepSeek/deepseek-v4-pro via Qwen Code /review

// Snapshot the union of every live session's stamped client ids.
Comment thread
doudouOUC marked this conversation as resolved.
// Returned as a fresh Set so callers can mutate-safely (the live
// per-session maps stay private). Workspace-level mutation routes
// use this to validate `X-Qwen-Client-Id` without owning a
Comment thread
doudouOUC marked this conversation as resolved.
// session id; PR 24 will replace it with a workspace-scoped
// registry that doesn't conflate session-attach with workspace-
// attach.
const out = new Set<string>();
for (const entry of byId.values()) {
for (const id of entry.clientIds.keys()) out.add(id);
}
return out;
},

async getWorkspaceMcpStatus() {
return requestWorkspaceStatus(SERVE_STATUS_EXT_METHODS.workspaceMcp, () =>
createIdleWorkspaceMcpStatus(boundWorkspace),
Expand Down
14 changes: 14 additions & 0 deletions packages/cli/src/serve/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ const EXPECTED_STAGE1_FEATURES = [
'workspace_mcp',
'workspace_skills',
'workspace_providers',
'workspace_memory',
'workspace_agents',
'workspace_env',
'workspace_preflight',
'session_context',
Expand Down Expand Up @@ -548,6 +550,18 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge {
heartbeatStateCalls.push(sessionId);
return heartbeatStateImpl(sessionId);
},
publishWorkspaceEvent(_event) {
// Issue #4175 PR 16 — fakeBridge default is a no-op. Tests that
// assert on workspace fan-out override this through the dedicated
// route-level test files (workspaceMemory.test.ts /
// workspaceAgents.test.ts) where the real fan-out behavior is
// exercised against a live bridge.
},
knownClientIds() {
// Default empty set — workspace mutation tests opt in by
// overriding the bridge in their suite.
return new Set<string>();
},
async killSession(sessionId, opts) {
killCalls.push({ sessionId, opts });
},
Expand Down
24 changes: 24 additions & 0 deletions packages/cli/src/serve/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ import {
type CapabilitiesEnvelope,
type ServeOptions,
} from './types.js';
import { mountWorkspaceMemoryRoutes } from './workspaceMemory.js';
import { mountWorkspaceAgentsRoutes } from './workspaceAgents.js';
import {
createWorkspaceFileSystemFactory,
type WorkspaceFileSystemFactory,
Expand Down Expand Up @@ -376,6 +378,28 @@ export function createServeApp(
}
});

// Issue #4175 PR 16: workspace memory + agents CRUD. Routes mounted
// through factories so server.ts stays the composition root while
// the feature modules own their own validation, error mapping, and
// event fan-out. Both factories receive the shared `mutate` gate
// and the request-helpers `parseClientIdHeader` / `safeBody` so
// strict mutation gating and pollution-key scrubbing match the
// existing routes bit-for-bit.
mountWorkspaceMemoryRoutes(app, {
bridge,
boundWorkspace,
mutate,
parseClientId: parseClientIdHeader,
Comment thread
doudouOUC marked this conversation as resolved.
safeBody,
});
mountWorkspaceAgentsRoutes(app, {
bridge,
boundWorkspace,
mutate,
parseClientId: parseClientIdHeader,
safeBody,
});

// TODO(#4175 PR 24 — PermissionMediator audit log): emit an
// `audit.diagnostic_read` event from these two routes so a security
// operator can correlate "who read what when". Read-only diagnostic
Expand Down
11 changes: 7 additions & 4 deletions packages/cli/src/serve/status.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ import {
} from './status.js';

describe('SERVE_ERROR_KINDS', () => {
it('exposes the eight roadmap-defined error kinds in stable order', () => {
it('exposes the roadmap-defined error kinds in stable order', () => {
// PR 13 introduced the closed taxonomy with seven preflight/env
// kinds; PR 14 added `'budget_exhausted'` for MCP guardrail
// refusals (see #4175 PR 14). Future additions append to this
// list — the order is part of the contract so SDK consumers
// can pattern-match without per-kind lookups.
// refusals (see #4175 PR 14); PR 16 added `'stat_failed'` for
// non-ENOENT stat failures on workspace memory discovery (see
// #4175 PR 16). Future additions append to this list — the
// order is part of the contract so SDK consumers can pattern-
// match without per-kind lookups.
expect(SERVE_ERROR_KINDS).toEqual([
'missing_binary',
'blocked_egress',
Expand All @@ -27,6 +29,7 @@ describe('SERVE_ERROR_KINDS', () => {
'protocol_error',
'missing_file',
'parse_error',
'stat_failed',
'budget_exhausted',
]);
});
Expand Down
Loading
Loading