diff --git a/packages/cli/src/serve/capabilities.ts b/packages/cli/src/serve/capabilities.ts index 5948a71a186..74862167123 100644 --- a/packages/cli/src/serve/capabilities.ts +++ b/packages/cli/src/serve/capabilities.ts @@ -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' }, diff --git a/packages/cli/src/serve/debugMode.ts b/packages/cli/src/serve/debugMode.ts new file mode 100644 index 00000000000..dd81a5ec5bb --- /dev/null +++ b/packages/cli/src/serve/debugMode.ts @@ -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()); +} diff --git a/packages/cli/src/serve/fs/contract.test.ts b/packages/cli/src/serve/fs/contract.test.ts index 82f5d838d9e..a074f70e7fd 100644 --- a/packages/cli/src/serve/fs/contract.test.ts +++ b/packages/cli/src/serve/fs/contract.test.ts @@ -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'; /** diff --git a/packages/cli/src/serve/httpAcpBridge.test.ts b/packages/cli/src/serve/httpAcpBridge.test.ts index 80a191f23ff..de411f4d2be 100644 --- a/packages/cli/src/serve/httpAcpBridge.test.ts +++ b/packages/cli/src/serve/httpAcpBridge.test.ts @@ -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` so cast through `Set` to attempt + // a mutation; the live registry must stay intact. + (ids as Set).delete(a.clientId!); + const fresh = bridge.knownClientIds(); + expect(fresh.size).toBe(2); + + await bridge.shutdown(); + }); + }); }); diff --git a/packages/cli/src/serve/httpAcpBridge.ts b/packages/cli/src/serve/httpAcpBridge.ts index 9190e8875ae..04ffdff0af5 100644 --- a/packages/cli/src/serve/httpAcpBridge.ts +++ b/packages/cli/src/serve/httpAcpBridge.ts @@ -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): 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; + /** * 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. @@ -3235,6 +3257,61 @@ export function createHttpAcpBridge(opts: BridgeOptions): HttpAcpBridge { }; }, + publishWorkspaceEvent(event) { + // 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 + // 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() { + // Snapshot the union of every live session's stamped client ids. + // 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 + // 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(); + 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), diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index 4f82c4ea69f..7ed73507ea4 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -96,6 +96,8 @@ const EXPECTED_STAGE1_FEATURES = [ 'workspace_mcp', 'workspace_skills', 'workspace_providers', + 'workspace_memory', + 'workspace_agents', 'workspace_env', 'workspace_preflight', 'session_context', @@ -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(); + }, async killSession(sessionId, opts) { killCalls.push({ sessionId, opts }); }, diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index 75006de252e..6323662c83a 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -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, @@ -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, + 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 diff --git a/packages/cli/src/serve/status.test.ts b/packages/cli/src/serve/status.test.ts index 7b776c1c387..0beb6393229 100644 --- a/packages/cli/src/serve/status.test.ts +++ b/packages/cli/src/serve/status.test.ts @@ -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', @@ -27,6 +29,7 @@ describe('SERVE_ERROR_KINDS', () => { 'protocol_error', 'missing_file', 'parse_error', + 'stat_failed', 'budget_exhausted', ]); }); diff --git a/packages/cli/src/serve/status.ts b/packages/cli/src/serve/status.ts index 93d531e7b60..c2fdc733e17 100644 --- a/packages/cli/src/serve/status.ts +++ b/packages/cli/src/serve/status.ts @@ -23,6 +23,7 @@ export const SERVE_ERROR_KINDS = [ 'protocol_error', 'missing_file', 'parse_error', + 'stat_failed', // Issue #4175 PR 14: budget refusal under `--mcp-budget-mode=enforce`. // Surfaced on per-server `mcp_server` cells (refused at discovery) // and on the workspace-level `mcp_budget` cell (any refusal this pass). @@ -51,6 +52,8 @@ export const SERVE_STATUS_EXT_METHODS = { workspaceMcp: 'qwen/status/workspace/mcp', workspaceSkills: 'qwen/status/workspace/skills', workspaceProviders: 'qwen/status/workspace/providers', + workspaceMemory: 'qwen/status/workspace/memory', + workspaceAgents: 'qwen/status/workspace/agents', workspacePreflight: 'qwen/status/workspace/preflight', sessionContext: 'qwen/status/session/context', sessionSupportedCommands: 'qwen/status/session/supported_commands', @@ -246,6 +249,120 @@ export interface ServeSessionSupportedCommandsStatus { availableSkills: string[]; } +/** + * Issue #4175 PR 16: workspace memory + agents read surfaces. + * + * Both shapes mirror the `kind / status / error? / errorKind? / hint?` + * cell pattern that PR 12's mcp/skills/providers status structures use, + * so the SDK reducer can render any of these with one pattern. + */ + +export type ServeContextFileScope = 'workspace' | 'global'; + +export interface ServeWorkspaceMemoryFile { + kind: 'memory_file'; + /** Absolute path to the discovered memory file. */ + path: string; + /** + * 'workspace' for files under the bound workspace tree, 'global' for + * `~/.qwen/QWEN.md` style entries. Helps adapters render scope chips. + */ + scope: ServeContextFileScope; + /** Size in bytes of the file's serialized contents on disk. */ + bytes: number; +} + +export interface ServeWorkspaceMemoryStatus { + v: typeof STATUS_SCHEMA_VERSION; + workspaceCwd: string; + initialized: boolean; + files: ServeWorkspaceMemoryFile[]; + /** Total bytes across all hierarchical files (sum of `files[].bytes`). */ + totalBytes: number; + /** + * Number of merged QWEN.md / AGENTS.md files the loader pulled in. + * Mirrors `LoadServerHierarchicalMemoryResponse.fileCount`. + */ + fileCount: number; + /** Baseline path-rule count from `.qwen/rules/`. */ + ruleCount: number; + errors?: ServeStatusCell[]; +} + +/** + * Storage level for a subagent definition surfaced through + * `GET /workspace/agents` and the per-`agentType` detail route. + * + * `project` / `user` / `builtin` are the values the daemon actually + * returns today. `extension` and `session` are forward-compat slots: + * the daemon-scoped `SubagentManager` runs against a stub `Config` + * whose `getActiveExtensions()` returns `[]`, and session-level + * subagents live in a runtime-only cache no CRUD route reads. + * Mirrors `DaemonAgentLevel` in `@qwen-code/sdk` so route + SDK + * consumers see the same forward-compat union. + */ +export type ServeAgentLevel = + | 'project' + | 'user' + | 'builtin' + | 'extension' + | 'session'; + +export interface ServeWorkspaceAgentSummary { + kind: 'agent'; + name: string; + description: string; + level: ServeAgentLevel; + isBuiltin: boolean; + /** Whether this agent restricts the tool set via `tools:` frontmatter. */ + hasTools: boolean; + model?: string; + color?: string; + background?: boolean; + approvalMode?: string; + extensionName?: string; + /** Absolute path to the file backing this agent (or sentinel for built-ins). */ + filePath?: string; +} + +export interface ServeWorkspaceAgentDetail extends ServeWorkspaceAgentSummary { + systemPrompt: string; + tools?: string[]; + disallowedTools?: string[]; + runConfig?: { max_time_minutes?: number; max_turns?: number }; +} + +export interface ServeWorkspaceAgentsStatus { + v: typeof STATUS_SCHEMA_VERSION; + workspaceCwd: string; + agents: ServeWorkspaceAgentSummary[]; + errors?: ServeStatusCell[]; +} + +export function createIdleWorkspaceMemoryStatus( + workspaceCwd: string, +): ServeWorkspaceMemoryStatus { + return { + v: STATUS_SCHEMA_VERSION, + workspaceCwd, + initialized: false, + files: [], + totalBytes: 0, + fileCount: 0, + ruleCount: 0, + }; +} + +export function createIdleWorkspaceAgentsStatus( + workspaceCwd: string, +): ServeWorkspaceAgentsStatus { + return { + v: STATUS_SCHEMA_VERSION, + workspaceCwd, + agents: [], + }; +} + export function createIdleWorkspaceMcpStatus( workspaceCwd: string, ): ServeWorkspaceMcpStatus { diff --git a/packages/cli/src/serve/workspaceAgents.test.ts b/packages/cli/src/serve/workspaceAgents.test.ts new file mode 100644 index 00000000000..908f68920eb --- /dev/null +++ b/packages/cli/src/serve/workspaceAgents.test.ts @@ -0,0 +1,969 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { promises as fs } from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import express from 'express'; +import request from 'supertest'; +import { + afterEach, + beforeEach, + describe, + expect, + it, + vi, + type MockInstance, +} from 'vitest'; +import { Storage, QWEN_DIR } from '@qwen-code/qwen-code-core'; +import { createMutationGate } from './auth.js'; +import type { HttpAcpBridge } from './httpAcpBridge.js'; +import type { BridgeEvent } from './eventBus.js'; +import { mountWorkspaceAgentsRoutes } from './workspaceAgents.js'; + +type RecordedEvent = Omit; + +function buildBridgeStub( + opts: { knownIds?: Iterable } = {}, +): HttpAcpBridge & { + events: RecordedEvent[]; +} { + const events: RecordedEvent[] = []; + const known = new Set(opts.knownIds ?? []); + return { + events, + publishWorkspaceEvent(event: RecordedEvent) { + events.push(event); + }, + knownClientIds() { + return new Set(known); + }, + spawnOrAttach: () => { + throw new Error('not implemented'); + }, + loadSession: () => { + throw new Error('not implemented'); + }, + resumeSession: () => { + throw new Error('not implemented'); + }, + sendPrompt: () => { + throw new Error('not implemented'); + }, + cancelSession: () => { + throw new Error('not implemented'); + }, + subscribeEvents: () => { + throw new Error('not implemented'); + }, + closeSession: () => { + throw new Error('not implemented'); + }, + updateSessionMetadata: () => { + throw new Error('not implemented'); + }, + respondToPermission: () => { + throw new Error('not implemented'); + }, + respondToSessionPermission: () => { + throw new Error('not implemented'); + }, + listWorkspaceSessions: () => { + throw new Error('not implemented'); + }, + recordHeartbeat: () => { + throw new Error('not implemented'); + }, + getHeartbeatState: () => undefined, + getWorkspaceMcpStatus: async () => { + throw new Error('not implemented'); + }, + getWorkspaceSkillsStatus: async () => { + throw new Error('not implemented'); + }, + getWorkspaceProvidersStatus: async () => { + throw new Error('not implemented'); + }, + getSessionContextStatus: async () => { + throw new Error('not implemented'); + }, + getSessionSupportedCommandsStatus: async () => { + throw new Error('not implemented'); + }, + setSessionModel: async () => { + throw new Error('not implemented'); + }, + killSession: async () => {}, + detachClient: async () => {}, + sessionCount: 0, + pendingPermissionCount: 0, + killAllSync: () => {}, + shutdown: async () => {}, + } as unknown as HttpAcpBridge & { events: RecordedEvent[] }; +} + +function buildApp(opts: { + bridge: HttpAcpBridge; + boundWorkspace: string; + strictNoToken?: boolean; +}) { + const app = express(); + app.use(express.json({ limit: '10mb' })); + const mutate = createMutationGate({ + tokenConfigured: !opts.strictNoToken, + requireAuth: false, + }); + mountWorkspaceAgentsRoutes(app, { + bridge: opts.bridge, + boundWorkspace: opts.boundWorkspace, + mutate, + parseClientId: (req, res) => { + const raw = req.get('x-qwen-client-id'); + if (raw === undefined || raw === '') return undefined; + if (raw.length > 128 || !/^[A-Za-z0-9._:-]+$/.test(raw)) { + res.status(400).json({ + error: '`X-Qwen-Client-Id` must be a non-empty token', + code: 'invalid_client_id', + }); + return null; + } + return raw; + }, + safeBody: (req) => { + const raw = req.body; + if (typeof raw !== 'object' || raw === null || Array.isArray(raw)) { + return Object.create(null) as Record; + } + const out = Object.create(null) as Record; + for (const [k, v] of Object.entries(raw as Record)) { + if (k === '__proto__' || k === 'constructor' || k === 'prototype') { + continue; + } + out[k] = v; + } + return out; + }, + }); + return app; +} + +describe('workspace agents routes', () => { + let tmp: string; + let workspace: string; + let globalDir: string; + let getGlobalQwenDirSpy: MockInstance<() => string>; + + beforeEach(async () => { + tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'qwen-serve-agents-')); + workspace = path.join(tmp, 'workspace'); + globalDir = path.join(tmp, 'global'); + await fs.mkdir(workspace, { recursive: true }); + await fs.mkdir(globalDir, { recursive: true }); + getGlobalQwenDirSpy = vi + .spyOn(Storage, 'getGlobalQwenDir') + .mockReturnValue(globalDir); + }); + + afterEach(async () => { + getGlobalQwenDirSpy.mockRestore(); + await fs.rm(tmp, { recursive: true, force: true }); + }); + + it('lists built-in agents alongside on-disk project agents', async () => { + const projectAgentsDir = path.join(workspace, QWEN_DIR, 'agents'); + await fs.mkdir(projectAgentsDir, { recursive: true }); + await fs.writeFile( + path.join(projectAgentsDir, 'reviewer.md'), + `---\nname: reviewer\ndescription: reviews PRs\n---\nyou are a reviewer agent\n`, + 'utf8', + ); + + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app).get('/workspace/agents'); + + expect(res.status).toBe(200); + const names = (res.body.agents as Array<{ name: string }>).map( + (a) => a.name, + ); + expect(names).toContain('reviewer'); + expect(names).toContain('general-purpose'); + const reviewerEntry = ( + res.body.agents as Array<{ + name: string; + level: string; + systemPrompt?: string; + }> + ).find((a) => a.name === 'reviewer'); + expect(reviewerEntry?.level).toBe('project'); + // Listings exclude the systemPrompt for bounded payload. + expect(reviewerEntry?.systemPrompt).toBeUndefined(); + }); + + it('GET /workspace/agents reflects out-of-band agent file changes', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + + // First call populates SubagentManager's cache. + let res = await request(app).get('/workspace/agents'); + expect(res.status).toBe(200); + const before = (res.body.agents as Array<{ name: string }>).map( + (a) => a.name, + ); + expect(before).not.toContain('fresh-out-of-band'); + + // Out-of-band: a developer / IDE adapter writes a new agent file + // directly to disk, bypassing the daemon's POST route. Without + // `force: true` on the LIST handler, `listSubagents()` would + // serve the stale cache from the first call and silently miss + // the new entry — diverging from the detail route, which always + // re-reads disk. + const projectAgentsDir = path.join(workspace, QWEN_DIR, 'agents'); + await fs.mkdir(projectAgentsDir, { recursive: true }); + await fs.writeFile( + path.join(projectAgentsDir, 'fresh-out-of-band.md'), + `---\nname: fresh-out-of-band\ndescription: out-of-band agent description\n---\nyou are the fresh out-of-band agent\n`, + 'utf8', + ); + + res = await request(app).get('/workspace/agents'); + expect(res.status).toBe(200); + const after = (res.body.agents as Array<{ name: string }>).map( + (a) => a.name, + ); + expect(after).toContain('fresh-out-of-band'); + }); + + it('returns the full detail (with systemPrompt) on GET /workspace/agents/:agentType', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app).get('/workspace/agents/general-purpose'); + expect(res.status).toBe(200); + expect(res.body.name).toBe('general-purpose'); + expect(typeof res.body.systemPrompt).toBe('string'); + expect(res.body.isBuiltin).toBe(true); + expect(res.body.level).toBe('builtin'); + }); + + it('returns 404 agent_not_found for unknown agent', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app).get('/workspace/agents/no-such-agent'); + expect(res.status).toBe(404); + expect(res.body.code).toBe('agent_not_found'); + }); + + it('matches frontmatter name case-insensitively', async () => { + const projectAgentsDir = path.join(workspace, QWEN_DIR, 'agents'); + await fs.mkdir(projectAgentsDir, { recursive: true }); + await fs.writeFile( + path.join(projectAgentsDir, 'casey.md'), + `---\nname: CaseInsensitive-Agent\ndescription: case insensitive lookup test\n---\nyou are a test agent\n`, + 'utf8', + ); + + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app).get( + '/workspace/agents/caseinsensitive-agent', + ); + expect(res.status).toBe(200); + expect(res.body.name).toBe('CaseInsensitive-Agent'); + }); + + it('creates a project-level agent and emits agent_changed', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app).post('/workspace/agents').send({ + name: 'tester', + description: 'runs tests in the project', + systemPrompt: 'you are a tester agent', + scope: 'workspace', + }); + expect(res.status).toBe(201); + expect(res.body.ok).toBe(true); + expect(res.body.agent.name).toBe('tester'); + expect(res.body.agent.level).toBe('project'); + + const events = (bridge as unknown as { events: RecordedEvent[] }).events; + expect(events).toHaveLength(1); + expect(events[0]?.type).toBe('agent_changed'); + expect(events[0]?.data).toMatchObject({ + change: 'created', + name: 'tester', + level: 'project', + }); + + // File was actually written. + const onDisk = await fs.readFile( + path.join(workspace, QWEN_DIR, 'agents', 'tester.md'), + 'utf8', + ); + expect(onDisk).toContain('name: tester'); + }); + + it('creates a user-level agent when scope=global', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app).post('/workspace/agents').send({ + name: 'global-helper', + description: 'cross-workspace helper', + systemPrompt: 'you are a helper agent', + scope: 'global', + }); + expect(res.status).toBe(201); + expect(res.body.agent.level).toBe('user'); + const onDisk = await fs.readFile( + path.join(globalDir, 'agents', 'global-helper.md'), + 'utf8', + ); + expect(onDisk).toContain('name: global-helper'); + }); + + it('returns 409 agent_already_exists when name collides at the same level', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const body = { + name: 'duplicate', + description: 'first description', + systemPrompt: 'you are the duplicate agent', + scope: 'workspace' as const, + }; + const first = await request(app).post('/workspace/agents').send(body); + expect(first.status).toBe(201); + const second = await request(app).post('/workspace/agents').send(body); + expect(second.status).toBe(409); + expect(second.body.code).toBe('agent_already_exists'); + }); + + it('rejects 422 invalid_config when create uses a builtin agent name', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app).post('/workspace/agents').send({ + name: 'general-purpose', + description: 'a description longer than ten chars', + systemPrompt: 'this is a system prompt', + scope: 'workspace', + }); + expect(res.status).toBe(422); + expect(res.body.code).toBe('invalid_config'); + expect(res.body.error).toMatch(/built-in/i); + + // BuiltinAgentRegistry.isBuiltinAgent is case-insensitive — both + // `Explore` and `explore` must reject so a project-level shadow + // can never land regardless of how the client cases the name. + const res2 = await request(app).post('/workspace/agents').send({ + name: 'explore', + description: 'a description longer than ten chars', + systemPrompt: 'this is a system prompt', + scope: 'workspace', + }); + expect(res2.status).toBe(422); + expect(res2.body.code).toBe('invalid_config'); + }); + + it('returns 422 invalid_config for missing required fields', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app) + .post('/workspace/agents') + .send({ scope: 'workspace' }); + expect(res.status).toBe(422); + expect(res.body.code).toBe('invalid_config'); + }); + + it('returns 400 invalid_scope for bad scope value', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app).post('/workspace/agents').send({ + name: 'a-name', + description: 'a description longer than ten chars', + systemPrompt: 'this is the system prompt', + scope: 'project', + }); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_scope'); + }); + + it('updates an existing project-level agent and emits agent_changed', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + await request(app).post('/workspace/agents').send({ + name: 'updatable', + description: 'old description', + systemPrompt: 'you are an updatable agent', + scope: 'workspace', + }); + const res = await request(app) + .post('/workspace/agents/updatable') + .send({ description: 'new description' }); + expect(res.status).toBe(200); + expect(res.body.agent.description).toBe('new description'); + const events = (bridge as unknown as { events: RecordedEvent[] }).events; + const changeEvents = events.filter((e) => e.type === 'agent_changed'); + expect(changeEvents).toHaveLength(2); + expect(changeEvents[1]?.data).toMatchObject({ + change: 'updated', + name: 'updatable', + level: 'project', + }); + }); + + it('returns 404 agent_not_found when updating an unknown agent', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app) + .post('/workspace/agents/no-such-agent') + .send({ description: 'x' }); + expect(res.status).toBe(404); + expect(res.body.code).toBe('agent_not_found'); + }); + + it('returns 403 agent_readonly when updating a built-in agent', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app) + .post('/workspace/agents/general-purpose') + .send({ description: 'rewritten' }); + expect(res.status).toBe(403); + expect(res.body.code).toBe('agent_readonly'); + }); + + it('deletes a project-level agent and emits agent_changed', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + await request(app).post('/workspace/agents').send({ + name: 'temporary', + description: 'temp description', + systemPrompt: 'you are a temp agent', + scope: 'workspace', + }); + const res = await request(app).delete('/workspace/agents/temporary'); + expect(res.status).toBe(204); + const events = (bridge as unknown as { events: RecordedEvent[] }).events; + const changeEvents = events.filter((e) => e.type === 'agent_changed'); + expect(changeEvents.at(-1)?.data).toMatchObject({ + change: 'deleted', + name: 'temporary', + level: 'project', + }); + }); + + it('returns 403 agent_readonly when deleting a built-in agent', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app).delete('/workspace/agents/general-purpose'); + expect(res.status).toBe(403); + expect(res.body.code).toBe('agent_readonly'); + }); + + it('returns 404 when deleting a missing agent', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app).delete('/workspace/agents/no-such-agent'); + expect(res.status).toBe(404); + expect(res.body.code).toBe('agent_not_found'); + }); + + it('refuses POST with 401 token_required on no-token loopback strict mode', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ + bridge, + boundWorkspace: workspace, + strictNoToken: true, + }); + const res = await request(app).post('/workspace/agents').send({ + name: 'a-name', + description: 'a description longer than ten chars', + systemPrompt: 'this is the system prompt', + scope: 'workspace', + }); + expect(res.status).toBe(401); + expect(res.body.code).toBe('token_required'); + }); + + it('rejects 400 invalid_client_id for unknown X-Qwen-Client-Id', async () => { + const bridge = buildBridgeStub({ knownIds: ['client_known'] }); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app) + .post('/workspace/agents') + .set('X-Qwen-Client-Id', 'client_stranger') + .send({ + name: 'a-name', + description: 'a description longer than ten chars', + systemPrompt: 'this is the system prompt', + scope: 'workspace', + }); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_client_id'); + }); + + it('trims leading/trailing whitespace on the agent name', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app).post('/workspace/agents').send({ + name: ' trimmed-name ', + description: 'a description longer than ten chars', + systemPrompt: 'you are a trimmed name agent', + scope: 'workspace', + }); + expect(res.status).toBe(201); + expect(res.body.agent.name).toBe('trimmed-name'); + // File on disk uses the trimmed name; the original-with-spaces + // version must NOT exist (would otherwise be unfindable via + // case-insensitive lookup). + const onDisk = await fs.readFile( + path.join(workspace, QWEN_DIR, 'agents', 'trimmed-name.md'), + 'utf8', + ); + expect(onDisk).toContain('name: trimmed-name'); + }); + + it('returns 422 invalid_config when scalar field has wrong type on create', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app).post('/workspace/agents').send({ + name: 'wrong-type', + description: 'a description longer than ten chars', + systemPrompt: 'you are a wrong-type test agent', + scope: 'workspace', + model: 123, + }); + expect(res.status).toBe(422); + expect(res.body.code).toBe('invalid_config'); + expect(res.body.error).toMatch(/model.*string/); + }); + + it('returns 422 invalid_config for unknown approvalMode', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app).post('/workspace/agents').send({ + name: 'bad-mode', + description: 'a description longer than ten chars', + systemPrompt: 'you are a bad-mode test agent', + scope: 'workspace', + approvalMode: 'rampage', + }); + expect(res.status).toBe(422); + expect(res.body.code).toBe('invalid_config'); + expect(res.body.error).toMatch(/approvalMode/); + }); + + it('strips unknown runConfig keys and rejects malformed values', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + // Unknown keys are silently dropped, valid known keys preserved. + const res = await request(app) + .post('/workspace/agents') + .send({ + name: 'run-config', + description: 'a description longer than ten chars', + systemPrompt: 'you are a run-config test agent', + scope: 'workspace', + runConfig: { max_turns: 5, mystery_field: 'oops' }, + }); + expect(res.status).toBe(201); + expect(res.body.agent.runConfig).toEqual({ max_turns: 5 }); + + // Malformed known field fails closed. + const res2 = await request(app) + .post('/workspace/agents') + .send({ + name: 'run-config-bad', + description: 'a description longer than ten chars', + systemPrompt: 'you are a run-config bad agent', + scope: 'workspace', + runConfig: { max_turns: -1 }, + }); + expect(res2.status).toBe(422); + expect(res2.body.code).toBe('invalid_config'); + }); + + it('rejects 400 invalid_scope on repeated ?scope= query', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + // Express parses repeated query params as an array; we should + // fail-closed rather than treating it as absent. + const res = await request(app).delete( + '/workspace/agents/some-name?scope=workspace&scope=global', + ); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_scope'); + }); + + it('rejects 400 invalid_config for empty update body', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + await request(app).post('/workspace/agents').send({ + name: 'has-fields', + description: 'a description longer than ten chars', + systemPrompt: 'you are a has-fields test agent', + scope: 'workspace', + }); + const res = await request(app) + .post('/workspace/agents/has-fields') + .send({}); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_config'); + }); + + it('stamps originatorClientId on agent_changed for known clients (create / update / delete)', async () => { + const bridge = buildBridgeStub({ knownIds: ['client_audit'] }); + const app = buildApp({ bridge, boundWorkspace: workspace }); + + // Create with a stamped client id. + const createRes = await request(app) + .post('/workspace/agents') + .set('X-Qwen-Client-Id', 'client_audit') + .send({ + name: 'audited', + description: 'a description longer than ten chars', + systemPrompt: 'you are an audited agent', + scope: 'workspace', + }); + expect(createRes.status).toBe(201); + + // Update with the same client id. + const updateRes = await request(app) + .post('/workspace/agents/audited') + .set('X-Qwen-Client-Id', 'client_audit') + .send({ description: 'a NEW description longer than ten chars' }); + expect(updateRes.status).toBe(200); + expect(updateRes.body.changed).toBe(true); + + // Delete with the same client id. + const deleteRes = await request(app) + .delete('/workspace/agents/audited') + .set('X-Qwen-Client-Id', 'client_audit'); + expect(deleteRes.status).toBe(204); + + const events = (bridge as unknown as { events: RecordedEvent[] }).events; + const agentEvents = events.filter((e) => e.type === 'agent_changed'); + expect(agentEvents).toHaveLength(3); + // All three must be stamped with the originator id so audit / + // echo-suppression on the SDK side can attribute them. + for (const evt of agentEvents) { + expect(evt.originatorClientId).toBe('client_audit'); + } + // Sequence: created → updated → deleted. + expect( + agentEvents.map((e) => (e.data as { change: string }).change), + ).toEqual(['created', 'updated', 'deleted']); + }); + + it('returns 400 invalid_agent_type for path-traversal-shaped agentType', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + // The readdir-based scan in `findSubagentByNameAtLevel` already + // protects against path traversal (filenames are matched, not + // joined-and-resolved), but the route-level regex check fails + // fast at the boundary so unsafe-shaped names never reach + // SubagentManager. + const res = await request(app).get('/workspace/agents/..%2Fetc%2Fpasswd'); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_agent_type'); + }); + + it('rejects 400 invalid_agent_type for over-long agentType', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const longName = 'a'.repeat(65); + const res = await request(app).delete(`/workspace/agents/${longName}`); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_agent_type'); + }); + + it('returns 500 agent_delete_partial when one level unlink silently fails', async () => { + // Windows ignores Unix-style permission bits passed to + // `fs.chmod` — the user-agents directory stays writable, the + // unlink succeeds, and the partial-delete path this test + // exercises is unreachable. SubagentManager's `unlink` import + // (`import * as fs from 'fs/promises'`) creates a sealed + // namespace object that vitest can't `spyOn`, so a per-platform + // mock is also off-limits. The route logic itself is + // platform-agnostic; the Ubuntu + macOS runs cover it. Mirrors + // the `process.platform === 'win32'` early-return idiom used in + // `customBanner.test.ts:232`. + if (process.platform === 'win32') return; + + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + + // Set up a project-level agent. + await request(app).post('/workspace/agents').send({ + name: 'partial-target', + description: 'a description longer than ten chars', + systemPrompt: 'you are a partial-target agent', + scope: 'workspace', + }); + // Set up a user-level shadow with the same name. + await request(app).post('/workspace/agents').send({ + name: 'partial-target', + description: 'a description longer than ten chars', + systemPrompt: 'you are a partial-target user agent', + scope: 'global', + }); + + // Lock the user-level agent's containing directory so the unlink + // raises EACCES — `SubagentManager.deleteSubagent` swallows the + // error and returns "success" because the project-level unlink + // worked. Without this PR's per-level `fs.access` verification, + // the route would 204 and publish a misleading `agent_changed` + // event for the user-level file that's still on disk. + const userAgentsDir = path.join(globalDir, 'agents'); + const userPath = path.join(userAgentsDir, 'partial-target.md'); + const originalMode = (await fs.stat(userAgentsDir)).mode; + await fs.chmod(userAgentsDir, 0o555); // r-x: blocks unlink + try { + const res = await request(app).delete('/workspace/agents/partial-target'); + expect(res.status).toBe(500); + expect(res.body.code).toBe('agent_delete_partial'); + expect(res.body.removedLevels).toEqual(['project']); + expect(res.body.remainingLevels).toEqual(['user']); + + // Event fan-out: only one event for the level that actually + // disappeared. The remaining level (still on disk) must NOT + // emit a misleading deleted event. + const events = (bridge as unknown as { events: RecordedEvent[] }).events; + const deletedEvents = events.filter( + (e) => + e.type === 'agent_changed' && + (e.data as { change: string }).change === 'deleted', + ); + expect(deletedEvents).toHaveLength(1); + expect((deletedEvents[0]?.data as { level: string }).level).toBe( + 'project', + ); + + // Verify the user-level file is still on disk. + await expect(fs.access(userPath)).resolves.toBeUndefined(); + } finally { + // Restore permissions so afterEach's rmdir succeeds. + await fs.chmod(userAgentsDir, originalMode); + } + }); + + it('DELETE /workspace/agents/:agentType?scope=workspace removes only the project shadow', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + await request(app).post('/workspace/agents').send({ + name: 'scoped-delete', + description: 'a description longer than ten chars', + systemPrompt: 'you are a scoped-delete project agent', + scope: 'workspace', + }); + await request(app).post('/workspace/agents').send({ + name: 'scoped-delete', + description: 'a description longer than ten chars', + systemPrompt: 'you are a scoped-delete user agent', + scope: 'global', + }); + + const res = await request(app).delete( + '/workspace/agents/scoped-delete?scope=workspace', + ); + expect(res.status).toBe(204); + + // Project file gone; user file still exists. + await expect( + fs.access(path.join(workspace, QWEN_DIR, 'agents', 'scoped-delete.md')), + ).rejects.toMatchObject({ code: 'ENOENT' }); + await expect( + fs.access(path.join(globalDir, 'agents', 'scoped-delete.md')), + ).resolves.toBeUndefined(); + + // Exactly one agent_changed event, at project level. + const events = (bridge as unknown as { events: RecordedEvent[] }).events; + const deleteEvents = events.filter( + (e) => + e.type === 'agent_changed' && + (e.data as { change: string }).change === 'deleted', + ); + expect(deleteEvents).toHaveLength(1); + expect((deleteEvents[0]?.data as { level: string }).level).toBe('project'); + }); + + it('POST /workspace/agents/:agentType?scope=global updates the user shadow', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + await request(app).post('/workspace/agents').send({ + name: 'scoped-update', + description: 'a description longer than ten chars', + systemPrompt: 'you are a scoped-update project agent', + scope: 'workspace', + }); + await request(app).post('/workspace/agents').send({ + name: 'scoped-update', + description: 'a description longer than ten chars', + systemPrompt: 'you are a scoped-update user agent', + scope: 'global', + }); + + const res = await request(app) + .post('/workspace/agents/scoped-update?scope=global') + .send({ description: 'NEW user-level description (longer than ten)' }); + expect(res.status).toBe(200); + expect(res.body.changed).toBe(true); + expect(res.body.agent.level).toBe('user'); + expect(res.body.agent.description).toBe( + 'NEW user-level description (longer than ten)', + ); + + // Project-level definition is untouched. + const projectFile = await fs.readFile( + path.join(workspace, QWEN_DIR, 'agents', 'scoped-update.md'), + 'utf8', + ); + expect(projectFile).toContain('a description longer than ten chars'); + }); + + it('rejects 422 when create has whitespace-only systemPrompt', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app).post('/workspace/agents').send({ + name: 'whitespace-prompt', + description: 'a description longer than ten chars', + systemPrompt: ' \n \t ', + scope: 'workspace', + }); + expect(res.status).toBe(422); + expect(res.body.code).toBe('invalid_config'); + expect(res.body.error).toMatch(/systemPrompt.*non-empty/); + }); + + it('rejects 422 when update has whitespace-only systemPrompt', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + await request(app).post('/workspace/agents').send({ + name: 'prompt-target', + description: 'a description longer than ten chars', + systemPrompt: 'you are a prompt-target agent', + scope: 'workspace', + }); + const res = await request(app) + .post('/workspace/agents/prompt-target') + .send({ systemPrompt: '\n\n \t' }); + expect(res.status).toBe(422); + expect(res.body.code).toBe('invalid_config'); + }); + + it('toDetail.runConfig only emits the documented fields', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + await request(app) + .post('/workspace/agents') + .send({ + name: 'detail-pick', + description: 'a description longer than ten chars', + systemPrompt: 'you are a detail-pick agent', + scope: 'workspace', + runConfig: { max_time_minutes: 5, max_turns: 7 }, + }); + const res = await request(app).get('/workspace/agents/detail-pick'); + expect(res.status).toBe(200); + // Detail must contain ONLY the whitelisted runConfig keys; if + // `SubagentConfig.runConfig` ever gains a new field in core, this + // assertion fails until the route schema is updated explicitly. + expect(Object.keys(res.body.runConfig).sort()).toEqual([ + 'max_time_minutes', + 'max_turns', + ]); + }); + + it('rejects 422 when update body has whitespace-only description', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + await request(app).post('/workspace/agents').send({ + name: 'whitespace-target', + description: 'a description longer than ten chars', + systemPrompt: 'you are a whitespace target agent', + scope: 'workspace', + }); + // Update path used to silently accept " " and overwrite the + // description with blank — divergent from create which 422s. + const res = await request(app) + .post('/workspace/agents/whitespace-target') + .send({ description: ' ' }); + expect(res.status).toBe(422); + expect(res.body.code).toBe('invalid_config'); + expect(res.body.error).toMatch(/non-empty/); + }); + + it('detects no-op partial runConfig update (preserves omitted keys)', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + await request(app) + .post('/workspace/agents') + .send({ + name: 'runconfig-noop', + description: 'a description longer than ten chars', + systemPrompt: 'you are a runconfig-noop agent', + scope: 'workspace', + runConfig: { max_time_minutes: 30, max_turns: 10 }, + }); + const eventsBefore = (bridge as unknown as { events: RecordedEvent[] }) + .events.length; + + // Partial update with the SAME max_time_minutes value. Without the + // fix, isNoOpUpdate compared `undefined !== existing.max_turns` → + // true and re-wrote the file + emitted agent_changed. + const res = await request(app) + .post('/workspace/agents/runconfig-noop') + .send({ runConfig: { max_time_minutes: 30 } }); + expect(res.status).toBe(200); + expect(res.body.changed).toBe(false); + + const events = (bridge as unknown as { events: RecordedEvent[] }).events; + expect(events.length).toBe(eventsBefore); + }); + + it('detects real partial runConfig change and writes', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + await request(app) + .post('/workspace/agents') + .send({ + name: 'runconfig-real', + description: 'a description longer than ten chars', + systemPrompt: 'you are a runconfig-real agent', + scope: 'workspace', + runConfig: { max_time_minutes: 30, max_turns: 10 }, + }); + const res = await request(app) + .post('/workspace/agents/runconfig-real') + .send({ runConfig: { max_time_minutes: 45 } }); + expect(res.status).toBe(200); + expect(res.body.changed).toBe(true); + // Merged result preserves max_turns from existing. + expect(res.body.agent.runConfig).toEqual({ + max_time_minutes: 45, + max_turns: 10, + }); + }); + + it('short-circuits no-op updates with changed: false and no event', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + await request(app).post('/workspace/agents').send({ + name: 'noop-target', + description: 'a description longer than ten chars', + systemPrompt: 'you are a noop-target agent', + scope: 'workspace', + }); + const eventsBefore = (bridge as unknown as { events: RecordedEvent[] }) + .events.length; + + const res = await request(app) + .post('/workspace/agents/noop-target') + .send({ description: 'a description longer than ten chars' }); + expect(res.status).toBe(200); + expect(res.body.changed).toBe(false); + + // No new agent_changed event for the no-op update. + const events = (bridge as unknown as { events: RecordedEvent[] }).events; + expect(events.length).toBe(eventsBefore); + }); +}); diff --git a/packages/cli/src/serve/workspaceAgents.ts b/packages/cli/src/serve/workspaceAgents.ts new file mode 100644 index 00000000000..ec0b7c950de --- /dev/null +++ b/packages/cli/src/serve/workspaceAgents.ts @@ -0,0 +1,1334 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { promises as fs } from 'node:fs'; +import type { Application, Request, RequestHandler, Response } from 'express'; +import { + APPROVAL_MODES, + BuiltinAgentRegistry, + SubagentError, + SubagentErrorCode, + SubagentManager, + type Config, + type SubagentConfig, + type SubagentLevel, +} from '@qwen-code/qwen-code-core'; +import { writeStderrLine } from '../utils/stdioHelpers.js'; +import { isServeDebugMode } from './debugMode.js'; +import { InvalidClientIdError, type HttpAcpBridge } from './httpAcpBridge.js'; + +/** + * Pattern for the route-layer `:agentType` URL parameter. Matches the + * `SubagentValidator.validateName` regex (`^[\p{L}\p{N}_-]+$`) so a + * malformed path component (containing slashes, dots, control chars, + * leading hyphen) is rejected at the boundary instead of trickling + * through `findSubagentByNameAtLevel`'s readdir scan. Defense in + * depth — `findSubagentByNameAtLevel` already prevents path traversal + * via filename matching, but failing fast at the route layer keeps + * surprising inputs out of downstream code paths. + */ +const AGENT_TYPE_PATTERN = /^[\p{L}\p{N}_-]+$/u; + +/** + * Cap on the route-layer name validator. SubagentValidator caps + * payload-side names at 50 chars; the route check uses 64 to leave a + * little headroom for legacy on-disk agents created with a longer + * name (resolved via case-insensitive cascade) that a client tries + * to GET / DELETE through the URL. + */ +const AGENT_TYPE_MAX_LENGTH = 64; + +/** + * Minimum agent-name length. Matches `SubagentValidator.validateName` + * (which requires `trimmedName.length >= 2`). Keeping the same lower + * bound at the route layer surfaces the constraint as a 422 instead + * of letting core throw `VALIDATION_ERROR` (which the route also + * 422s, but with a less specific message). + */ +const AGENT_TYPE_MIN_LENGTH = 2; + +/** + * Per-field size caps for create + update payloads. The Express body + * parser caps the whole request at 10 MB but no per-field guard + * existed, so a single payload could land a multi-megabyte + * `systemPrompt` on disk and balloon every `GET /workspace/agents` + * snapshot in memory. 256 KB is far above any realistic + * user-authored prompt while keeping list-response cost bounded. + */ +const MAX_DESCRIPTION_BYTES = 256 * 1024; +const MAX_SYSTEM_PROMPT_BYTES = 256 * 1024; +const MAX_TOOLS_ENTRIES = 256; +const MAX_TOOL_ID_LENGTH = 256; +import { + STATUS_SCHEMA_VERSION, + type ServeAgentLevel, + type ServeWorkspaceAgentDetail, + type ServeWorkspaceAgentSummary, + type ServeWorkspaceAgentsStatus, +} from './status.js'; + +/** + * Issue #4175 PR 16: workspace subagent CRUD routes. + * + * Wraps `SubagentManager` over five HTTP routes: + * + * GET /workspace/agents — list project + user + builtin + extension + * POST /workspace/agents — create at project or user level (409 on collision) + * GET /workspace/agents/:agentType — full detail incl. systemPrompt + * POST /workspace/agents/:agentType — update existing (404 missing, 403 read-only) + * DELETE /workspace/agents/:agentType — delete (idempotent for SDK callers) + * + * The daemon doesn't have a full `Config` instance, so we instantiate + * `SubagentManager` against a CRUD-scoped `Config` stub that + * implements only `getSdkMode / getProjectRoot / getActiveExtensions` + * — the methods the manager's CRUD paths actually touch (verified + * against `subagent-manager.ts:365,932,954,958`). A `Proxy` makes any + * future use of an unimplemented method throw immediately so a + * silent dependency creep can't ship as a 500. + */ + +export interface WorkspaceAgentsRouteDeps { + bridge: HttpAcpBridge; + boundWorkspace: string; + mutate: (opts?: { strict?: boolean }) => RequestHandler; + parseClientId: (req: Request, res: Response) => string | undefined | null; + safeBody: (req: Request) => Record; +} + +export function mountWorkspaceAgentsRoutes( + app: Application, + deps: WorkspaceAgentsRouteDeps, +): void { + const manager = createDaemonSubagentManager(deps.boundWorkspace); + + app.get('/workspace/agents', async (_req, res) => { + try { + // `force: true` re-walks `.qwen/agents/` on every call so out-of- + // band edits (a developer editing an agent file in their IDE + // while the daemon is running) appear immediately. Without it + // `SubagentManager.listSubagents()` serves a stale cache and + // diverges from `GET /workspace/agents/:agentType`, which always + // reads from disk (`loadSubagent → findSubagentByNameAtLevel → + // listSubagentsAtLevel`). Bringing the LIST route to parity is + // sub-millisecond for the typical 0-50 agents and matches the + // detail route's "filesystem is the source of truth" contract. + // + // No TTL cache or `fs.watch`-based invalidation here despite the + // 4-level walk per request. Reasoning: + // - 4 levels × <50 agents on local SSD = sub-ms IO, well below + // the per-request budget for any client UI. + // - A short-TTL cache would re-introduce the exact stale-list + // bug Codex P2 #2 fixed (a recently-edited file invisible + // until the TTL elapses); invalidation logic adds state to + // the route handler that PR 24 (audit / policy / mediator) + // is the proper home for. + // - `fs.watch` is platform-fragile (recursive watch broken on + // some macOS Node versions, inotify limits on Linux) and the + // daemon's per-request semantics make watchers harder to + // reason about than a fresh disk read. + // - Burst protection lives at `--max-connections` (256) + + // bearer auth on non-loopback, not at the route layer. + // Revisit if profiling shows the LIST route is on the hot path. + const agents = await manager.listSubagents({ force: true }); + const status: ServeWorkspaceAgentsStatus = { + v: STATUS_SCHEMA_VERSION, + workspaceCwd: deps.boundWorkspace, + agents: agents.map(toSummary), + }; + res.status(200).json(status); + } catch (err) { + writeStderrLine( + `qwen serve: GET /workspace/agents failed: ${ + err instanceof Error ? (err.stack ?? err.message) : String(err) + }`, + ); + res.status(500).json({ + error: 'Failed to list workspace agents', + code: 'agent_list_failed', + }); + } + }); + + app.post( + '/workspace/agents', + deps.mutate({ strict: true }), + async (req, res) => { + const body = deps.safeBody(req); + const clientIdResult = resolveOriginatorClientId(deps, req, res); + if (clientIdResult === null) return; + const originatorClientId = clientIdResult; + + const scope = body['scope']; + if (scope !== 'workspace' && scope !== 'global') { + res.status(400).json({ + error: '`scope` must be "workspace" or "global"', + code: 'invalid_scope', + }); + return; + } + const level: SubagentLevel = scope === 'workspace' ? 'project' : 'user'; + + const config = parseAgentConfig(body, level, res); + if (!config) return; + + // `manager.createSubagent` only checks whether the default + // `.md` file path is occupied. If a different on-disk + // file at the same level shares the frontmatter `name`, the + // duplicate-name collision wouldn't surface as 409. Preflight + // through `loadSubagent(name, level)` so a same-name shadow at + // either level returns `agent_already_exists` deterministically. + const collision = await manager.loadSubagent(config.name, level); + if (collision) { + res.status(409).json({ + error: `Subagent "${config.name}" already exists at ${level} level`, + code: 'agent_already_exists', + name: config.name, + level, + }); + return; + } + + try { + await manager.createSubagent(config, { level }); + } catch (err) { + if (err instanceof SubagentError) { + if (err.code === SubagentErrorCode.ALREADY_EXISTS) { + res.status(409).json({ + error: err.message, + code: 'agent_already_exists', + name: err.subagentName ?? config.name, + }); + return; + } + if ( + err.code === SubagentErrorCode.VALIDATION_ERROR || + err.code === SubagentErrorCode.INVALID_CONFIG || + err.code === SubagentErrorCode.INVALID_NAME || + err.code === SubagentErrorCode.TOOL_NOT_FOUND + ) { + res.status(422).json({ + error: err.message, + code: 'invalid_config', + name: err.subagentName ?? config.name, + }); + return; + } + if (err.code === SubagentErrorCode.FILE_ERROR) { + // `SubagentError(FILE_ERROR)` wraps Node fs error + // messages like `"ENOENT: no such file or directory, open + // '/Users//.qwen/agents/foo.md'"` — leaking the + // operator's absolute filesystem layout through an + // authenticated route response. Gate the message behind + // `QWEN_SERVE_DEBUG` so default production responses + // carry only the generic envelope; operators triaging + // locally enable the toggle to get the path back. + // Mirrors the workspaceMemory route's `file_error` + // disclosure posture. + const debug = isServeDebugMode(); + res.status(500).json({ + error: debug + ? err.message + : 'Failed to write workspace agent file', + code: 'file_error', + name: err.subagentName ?? config.name, + }); + return; + } + } + writeStderrLine( + `qwen serve: POST /workspace/agents failed: ${ + err instanceof Error ? (err.stack ?? err.message) : String(err) + }`, + ); + res.status(500).json({ + error: 'Failed to create workspace agent', + code: 'agent_create_failed', + }); + return; + } + + const created = await manager.loadSubagent(config.name, level); + if (!created) { + // Race window: createSubagent already wrote the file to disk, + // but the subsequent loadSubagent walked the cache and found + // nothing — typically a cache-refresh ordering bug. The file + // persists (no rollback) because deleting on a half-failed + // create would lose work for an agent that's actually fine on + // disk. Operators MUST be able to correlate the orphan file + // with the failed POST, so emit a stderr breadcrumb with the + // path; a fresh `GET /workspace/agents` will surface the + // agent on next request. PR 24's PermissionMediator can layer + // a proper rollback policy on top once mutation auditing + // arrives. + writeStderrLine( + `qwen serve: agent_create_reload_failed (name=${safeLogValue(config.name)} ` + + `level=${level}) — file likely persisted on disk; check ` + + `\`GET /workspace/agents\` for a phantom entry`, + ); + res.status(500).json({ + error: 'Agent creation succeeded but reload failed', + code: 'agent_create_reload_failed', + name: config.name, + level, + }); + return; + } + deps.bridge.publishWorkspaceEvent({ + type: 'agent_changed', + data: { change: 'created', name: config.name, level }, + ...(originatorClientId ? { originatorClientId } : {}), + }); + res.status(201).json({ ok: true, agent: toDetail(created) }); + }, + ); + + app.get('/workspace/agents/:agentType', async (req, res) => { + const agentType = validateAgentType(req, res); + if (agentType === null) return; + try { + const config = await manager.loadSubagent(agentType); + if (!config) { + res.status(404).json({ + error: `Subagent "${agentType}" not found`, + code: 'agent_not_found', + name: agentType, + }); + return; + } + res.status(200).json(toDetail(config)); + } catch (err) { + writeStderrLine( + `qwen serve: GET /workspace/agents/${safeLogValue(agentType)} failed: ${ + err instanceof Error ? (err.stack ?? err.message) : String(err) + }`, + ); + res.status(500).json({ + error: 'Failed to read workspace agent', + code: 'agent_read_failed', + }); + } + }); + + app.post( + '/workspace/agents/:agentType', + deps.mutate({ strict: true }), + async (req, res) => { + const agentType = validateAgentType(req, res); + if (agentType === null) return; + const clientIdResult = resolveOriginatorClientId(deps, req, res); + if (clientIdResult === null) return; + const originatorClientId = clientIdResult; + + const body = deps.safeBody(req); + const updates = parseAgentUpdates(body, res); + if (!updates) return; + + const preferredLevel = parseScopeQuery(req, res); + if (preferredLevel === null) return; + + const existing = await manager.loadSubagent(agentType, preferredLevel); + if (!existing) { + res.status(404).json({ + error: `Subagent "${agentType}" not found`, + code: 'agent_not_found', + name: agentType, + }); + return; + } + if (assertMutableLevel(existing, agentType, res)) { + return; + } + + // Empty / no-op update detection. An empty body or a body whose + // recognized fields all match `existing` would otherwise rewrite + // the file (mtime bump) AND fan out an `agent_changed` event for + // a request that didn't change anything — the same misleading + // signal the memory route avoids for whitespace-only appends. + // Reject empty payloads with 400; short-circuit no-op updates + // with 200 + `changed: false` so adapters can suppress redundant + // toasts without re-fetching. + if (Object.keys(updates).length === 0) { + res.status(400).json({ + error: + '`POST /workspace/agents/:agentType` requires at least one updatable field in the body', + code: 'invalid_config', + name: agentType, + }); + return; + } + if (isNoOpUpdate(existing, updates)) { + res.status(200).json({ + ok: true, + agent: toDetail(existing), + changed: false, + }); + return; + } + + try { + await manager.updateSubagent(agentType, updates, existing.level); + } catch (err) { + if (err instanceof SubagentError) { + if (err.code === SubagentErrorCode.NOT_FOUND) { + res.status(404).json({ + error: err.message, + code: 'agent_not_found', + name: err.subagentName ?? agentType, + }); + return; + } + if (err.code === SubagentErrorCode.INVALID_CONFIG) { + res.status(403).json({ + error: err.message, + code: 'agent_readonly', + name: err.subagentName ?? agentType, + }); + return; + } + if ( + err.code === SubagentErrorCode.VALIDATION_ERROR || + err.code === SubagentErrorCode.INVALID_NAME || + err.code === SubagentErrorCode.TOOL_NOT_FOUND + ) { + res.status(422).json({ + error: err.message, + code: 'invalid_config', + name: err.subagentName ?? agentType, + }); + return; + } + if (err.code === SubagentErrorCode.FILE_ERROR) { + // Same path-disclosure gating as the create-path + // FILE_ERROR handler above. Default response is the + // generic envelope; `QWEN_SERVE_DEBUG` re-enables the + // raw `err.message` for local triage. + const debug = isServeDebugMode(); + res.status(500).json({ + error: debug + ? err.message + : 'Failed to write workspace agent file', + code: 'file_error', + name: err.subagentName ?? agentType, + }); + return; + } + } + writeStderrLine( + `qwen serve: POST /workspace/agents/${safeLogValue(agentType)} failed: ${ + err instanceof Error ? (err.stack ?? err.message) : String(err) + }`, + ); + res.status(500).json({ + error: 'Failed to update workspace agent', + code: 'agent_update_failed', + }); + return; + } + + const updated = await manager.loadSubagent(agentType, existing.level); + if (!updated) { + // Symmetric to the create-reload-failure branch above. The + // disk write succeeded but the cache lookup raced; emit a + // breadcrumb so operators can correlate the orphan in-flight + // change with the failed POST. The file is in its updated + // state on disk; subsequent reads will pick it up. + writeStderrLine( + `qwen serve: agent_update_reload_failed (name=${safeLogValue(agentType)} ` + + `level=${existing.level}) — disk write completed; check ` + + `\`GET /workspace/agents/${safeLogValue(agentType)}\` for the new state`, + ); + res.status(500).json({ + error: 'Agent update succeeded but reload failed', + code: 'agent_update_reload_failed', + name: agentType, + level: existing.level, + }); + return; + } + const eventLevel: 'project' | 'user' = + existing.level === 'project' ? 'project' : 'user'; + deps.bridge.publishWorkspaceEvent({ + type: 'agent_changed', + data: { change: 'updated', name: existing.name, level: eventLevel }, + ...(originatorClientId ? { originatorClientId } : {}), + }); + res + .status(200) + .json({ ok: true, agent: toDetail(updated), changed: true }); + }, + ); + + app.delete( + '/workspace/agents/:agentType', + deps.mutate({ strict: true }), + async (req, res) => { + const agentType = validateAgentType(req, res); + if (agentType === null) return; + const clientIdResult = resolveOriginatorClientId(deps, req, res); + if (clientIdResult === null) return; + const originatorClientId = clientIdResult; + + const scopedLevel = parseScopeQuery(req, res); + if (scopedLevel === null) return; + + // Pre-check at every level we're going to try to delete. When + // `scopedLevel` is given we touch just that level; when omitted, + // `SubagentManager.deleteSubagent` iterates both `project` and + // `user`, so we need to look at both to (a) reject built-in / + // extension shadows and (b) emit one `agent_changed` event per + // file actually removed. + const levelsToCheck: SubagentLevel[] = scopedLevel + ? [scopedLevel] + : ['project', 'user']; + const existingAtLevels: SubagentConfig[] = []; + for (const lvl of levelsToCheck) { + const found = await manager.loadSubagent(agentType, lvl); + if (found) existingAtLevels.push(found); + } + for (const found of existingAtLevels) { + if (assertMutableLevel(found, agentType, res)) return; + } + + try { + await manager.deleteSubagent(agentType, scopedLevel); + } catch (err) { + if (err instanceof SubagentError) { + if (err.code === SubagentErrorCode.NOT_FOUND) { + res.status(404).json({ + error: err.message, + code: 'agent_not_found', + name: err.subagentName ?? agentType, + }); + return; + } + if (err.code === SubagentErrorCode.INVALID_CONFIG) { + res.status(403).json({ + error: err.message, + code: 'agent_readonly', + name: err.subagentName ?? agentType, + }); + return; + } + } + writeStderrLine( + `qwen serve: DELETE /workspace/agents/${safeLogValue(agentType)} failed: ${ + err instanceof Error ? (err.stack ?? err.message) : String(err) + }`, + ); + res.status(500).json({ + error: 'Failed to delete workspace agent', + code: 'agent_delete_failed', + }); + return; + } + + // [Critical] gpt-5.5 round-6 finding: + // `SubagentManager.deleteSubagent` swallows per-level + // `fs.unlink()` failures (subagent-manager.ts:332-336) and + // returns success as long as ANY level was removed. Trusting + // that signal would let us publish `agent_changed`/`deleted` + // for a file still on disk (EACCES / EBUSY / EPERM) — the + // client UI would drop a still-active definition from cache. + // Verify each pre-checked level's file is actually gone via + // `fs.access`; only fan out the event for confirmed removals. + // If at least one level still has its file, return 500 with + // the residual list so callers can act. + const removed: SubagentConfig[] = []; + const remaining: SubagentConfig[] = []; + for (const found of existingAtLevels) { + if (!found.filePath) { + // Synthetic / no-file entries (impossible at project / + // user levels, defensive guard) treat as "no verification + // possible" → assume removed to match legacy behavior. + removed.push(found); + continue; + } + try { + await fs.access(found.filePath); + // Still present → unlink failed silently. + remaining.push(found); + } catch { + // Any access error (typically ENOENT) means the file is + // gone — count as successfully removed. + removed.push(found); + } + } + + if (remaining.length > 0) { + writeStderrLine( + `qwen serve: DELETE /workspace/agents/${safeLogValue(agentType)} partial — ` + + `removed=${removed.map((r) => r.level).join(',') || 'none'} ` + + `remaining=${remaining + .map((r) => `${r.level}:${r.filePath}`) + .join(',')}`, + ); + // Still publish events for files we DID remove so subscribers + // get partial-success signals — but emit them BEFORE the 500 + // so a client reading the response can correlate. + for (const found of removed) { + const evtLevel: 'project' | 'user' = + found.level === 'project' ? 'project' : 'user'; + deps.bridge.publishWorkspaceEvent({ + type: 'agent_changed', + data: { + change: 'deleted', + name: found.name, + level: evtLevel, + }, + ...(originatorClientId ? { originatorClientId } : {}), + }); + } + res.status(500).json({ + error: + `Failed to delete every level of subagent "${agentType}" — ` + + `${remaining.length} level(s) still have their file on disk`, + code: 'agent_delete_partial', + name: agentType, + removedLevels: removed.map((r) => r.level), + remainingLevels: remaining.map((r) => r.level), + }); + return; + } + + // Emit one event per level that was deleted so subscribers using + // event metadata for toasts/audit/echo-suppression see the + // complete picture. Without this split, an unscoped DELETE that + // removed both project AND user shadows would publish only one + // event with one level — misleading the receiver about which + // file(s) actually went away. + if (existingAtLevels.length === 0) { + // `deleteSubagent` succeeded with no pre-checked level — could + // happen if a file landed between the loadSubagent check and + // the unlink. Emit a single best-effort event with the level + // hint we know. + const fallbackLevel: 'project' | 'user' = + scopedLevel === 'user' ? 'user' : 'project'; + deps.bridge.publishWorkspaceEvent({ + type: 'agent_changed', + data: { + change: 'deleted', + name: agentType, + level: fallbackLevel, + }, + ...(originatorClientId ? { originatorClientId } : {}), + }); + } else { + for (const found of removed) { + const evtLevel: 'project' | 'user' = + found.level === 'project' ? 'project' : 'user'; + deps.bridge.publishWorkspaceEvent({ + type: 'agent_changed', + data: { + change: 'deleted', + name: found.name, + level: evtLevel, + }, + ...(originatorClientId ? { originatorClientId } : {}), + }); + } + } + res.status(204).end(); + }, + ); +} + +/** + * Wrap a string value for safe interpolation into stderr log lines. + * `JSON.stringify` escapes control characters (`\n`, `\r`, etc.) and + * wraps the result in quotes so any injection attempt surfaces as + * visible-as-quoted-noise rather than a forged log line. Mirrors + * `safeLogValue` in `server.ts` (kept private there); we copy the + * 82-byte truncation budget so attacker-controlled long names can't + * blow up the operator's log shipper. Defense-in-depth — the + * route's `validateAgentType` regex already rejects names with + * control chars, but escaping also covers `agentType` derived from + * sources we don't fully control (legacy on-disk shadows, future + * routes adding new fields). + */ +function safeLogValue(raw: unknown): string { + return JSON.stringify(String(raw)).slice(0, 82); +} + +/** + * Pull `:agentType` off the request and reject malformed values at + * the route boundary. Returns the validated string, or `null` AFTER + * sending its own 400 — caller must short-circuit on `null`. + */ +function validateAgentType(req: Request, res: Response): string | null { + const raw = req.params['agentType']; + if (!raw || raw.length === 0) { + res.status(400).json({ + error: '`agentType` path parameter is required', + code: 'invalid_agent_type', + }); + return null; + } + if (raw.length > AGENT_TYPE_MAX_LENGTH || !AGENT_TYPE_PATTERN.test(raw)) { + res.status(400).json({ + error: + '`agentType` must contain only letters, numbers, hyphens, or underscores (max 64 chars)', + code: 'invalid_agent_type', + agentType: raw, + }); + return null; + } + return raw; +} + +/** + * Read the `?scope=` query, fail-closed on repeated/non-string + * values, and translate `workspace`/`global` into the + * `SubagentLevel` the manager expects. Returns: + * - `undefined` when `scope` is absent (caller falls back to default + * resolution / both levels); + * - the resolved `SubagentLevel` when valid; + * - `null` when the query was malformed AND the response was + * already sent — caller must short-circuit. + * + * Centralizes the duplicated parser block from the POST update + + * DELETE handlers so a future scope addition (e.g. `extension`) + * stays in one place. + */ +function parseScopeQuery( + req: Request, + res: Response, +): SubagentLevel | undefined | null { + const raw = req.query['scope']; + if (raw === undefined) return undefined; + if (typeof raw !== 'string') { + res.status(400).json({ + error: '`scope` query must be a single "workspace" or "global" value', + code: 'invalid_scope', + }); + return null; + } + if (raw !== 'workspace' && raw !== 'global') { + res.status(400).json({ + error: '`scope` query must be "workspace" or "global"', + code: 'invalid_scope', + }); + return null; + } + return raw === 'workspace' ? 'project' : 'user'; +} + +/** + * Reject mutation attempts targeting a read-only agent + * (built-in / extension / session). Returns `true` after sending + * the 403 — caller must short-circuit on `true`. Returns `false` + * when the entry is mutable (`project` / `user`). + * + * Centralizes the duplicated guard from the POST update + DELETE + * handlers; a future PR adding a new mutation route just calls this + * helper instead of re-implementing the predicate. + */ +function assertMutableLevel( + found: SubagentConfig, + agentType: string, + res: Response, +): boolean { + if ( + found.isBuiltin || + found.level === 'builtin' || + found.level === 'extension' || + found.level === 'session' + ) { + res.status(403).json({ + error: `Cannot modify ${found.level}-level subagent "${agentType}"`, + code: 'agent_readonly', + name: found.name, + level: found.level, + }); + return true; + } + return false; +} + +function resolveOriginatorClientId( + deps: WorkspaceAgentsRouteDeps, + req: Request, + res: Response, +): string | undefined | null { + const clientId = deps.parseClientId(req, res); + if (clientId === null) return null; + if (clientId === undefined) return undefined; + if (!deps.bridge.knownClientIds().has(clientId)) { + res.status(400).json({ + error: `Client id "${clientId}" is not registered for this workspace`, + code: 'invalid_client_id', + clientId, + }); + return null; + } + return clientId; +} + +function parseAgentConfig( + body: Record, + level: SubagentLevel, + res: Response, +): SubagentConfig | undefined { + const rawName = body['name']; + if (typeof rawName !== 'string' || rawName.trim().length === 0) { + res.status(422).json({ + error: '`name` is required and must be a non-empty string', + code: 'invalid_config', + }); + return undefined; + } + // Trim leading/trailing whitespace BEFORE storing. Without this, a + // client posting `{ name: " tester " }` would land a file whose + // frontmatter `name` field literally contains the spaces; the + // resolver's case-insensitive cascade still wouldn't match `/agents/ + // tester` because the lookup name and the on-disk name differ. + // Better to normalize at the boundary than carry untrimmed names + // through validation + serialization. + const name = rawName.trim(); + // Apply the same regex + length contract `validateAgentType` uses + // for `:agentType` URL parameters. Without this, a client could + // `POST /workspace/agents` with `name: "my/agent"` or + // `name: "a".repeat(100)` — names that the route's regex would + // reject if echoed back through GET / DELETE, plus the core's + // `SubagentValidator` would reject with a different error shape. + // Failing at the body-validation boundary keeps the round-trip + // (POST → GET → DELETE) coherent under one error shape. + if ( + name.length < AGENT_TYPE_MIN_LENGTH || + name.length > AGENT_TYPE_MAX_LENGTH || + !AGENT_TYPE_PATTERN.test(name) + ) { + res.status(422).json({ + error: `\`name\` must be ${AGENT_TYPE_MIN_LENGTH}-${AGENT_TYPE_MAX_LENGTH} characters of letters, numbers, hyphens, or underscores`, + code: 'invalid_config', + name, + }); + return undefined; + } + // Reject names that shadow a built-in subagent. Without this check a + // client could `POST /workspace/agents { name: "general-purpose" }` + // and write a project-level file at `/.qwen/agents/ + // general-purpose.md`. List/load resolve the project entry first + // (project > builtin), but `SubagentManager.deleteSubagent` rejects + // by name alone (`subagent-manager.ts:302`) — so DELETE returns 403 + // `agent_readonly` and the file becomes undeleteable through the + // API. Surface the conflict at create time instead. The check is + // case-insensitive (`BuiltinAgentRegistry.isBuiltinAgent` lowercases + // both sides), matching `loadSubagent`'s case-insensitive cascade. + if (BuiltinAgentRegistry.isBuiltinAgent(name)) { + res.status(422).json({ + error: `"${name}" shadows a built-in subagent and cannot be used as a project- or user-level agent name. Choose a different name.`, + code: 'invalid_config', + name, + }); + return undefined; + } + const description = body['description']; + if (typeof description !== 'string' || description.trim().length === 0) { + res.status(422).json({ + error: '`description` is required and must be a non-empty string', + code: 'invalid_config', + }); + return undefined; + } + if (Buffer.byteLength(description, 'utf8') > MAX_DESCRIPTION_BYTES) { + res.status(422).json({ + error: `\`description\` exceeds the ${MAX_DESCRIPTION_BYTES}-byte limit`, + code: 'invalid_config', + }); + return undefined; + } + const systemPrompt = body['systemPrompt']; + if (typeof systemPrompt !== 'string' || systemPrompt.trim().length === 0) { + // Reject whitespace-only systemPrompts to match the description + // field's `trim().length === 0` rule. A pure-whitespace prompt + // would land on disk as effectively empty (the YAML serializer + // collapses blank lines), and the agent can't operate without + // instructions, so a 422 at the boundary is friendlier than a + // mysterious downstream "agent does nothing" failure. + res.status(422).json({ + error: + '`systemPrompt` is required and must be a non-empty string (whitespace only is rejected)', + code: 'invalid_config', + }); + return undefined; + } + if (Buffer.byteLength(systemPrompt, 'utf8') > MAX_SYSTEM_PROMPT_BYTES) { + res.status(422).json({ + error: `\`systemPrompt\` exceeds the ${MAX_SYSTEM_PROMPT_BYTES}-byte limit`, + code: 'invalid_config', + }); + return undefined; + } + const tools = parseStringArray(body['tools'], 'tools', res); + if (tools === null) return undefined; + const disallowedTools = parseStringArray( + body['disallowedTools'], + 'disallowedTools', + res, + ); + if (disallowedTools === null) return undefined; + const config: SubagentConfig = { + name, + description, + systemPrompt, + level, + }; + if (tools !== undefined) config.tools = tools; + if (disallowedTools !== undefined) config.disallowedTools = disallowedTools; + + // Optional scalar fields. Present-but-wrong-type fails closed (422) + // rather than silently dropping the field — `SubagentValidator` + // doesn't reject these, and `serializeSubagent` only writes recognized + // values, so without explicit validation a `model: 123` payload would + // 201 with no `model` field on the file (masking client-serialization + // bugs). + if (rejectIfPresentWrongType(body, 'model', 'string', res)) return undefined; + if (typeof body['model'] === 'string') config.model = body['model']; + + if (rejectIfPresentWrongType(body, 'color', 'string', res)) return undefined; + if (typeof body['color'] === 'string') config.color = body['color']; + + if (rejectIfPresentWrongType(body, 'approvalMode', 'string', res)) { + return undefined; + } + if (typeof body['approvalMode'] === 'string') { + if (!APPROVAL_MODES.includes(body['approvalMode'] as never)) { + res.status(422).json({ + error: `\`approvalMode\` must be one of ${JSON.stringify(APPROVAL_MODES)}`, + code: 'invalid_config', + }); + return undefined; + } + config.approvalMode = body['approvalMode']; + } + + if (rejectIfPresentWrongType(body, 'background', 'boolean', res)) { + return undefined; + } + if (typeof body['background'] === 'boolean') { + config.background = body['background']; + } + + const runConfig = body['runConfig']; + if (runConfig !== undefined) { + const sanitized = sanitizeRunConfig(runConfig, res); + if (sanitized === null) return undefined; + config.runConfig = sanitized; + } + return config; +} + +function parseAgentUpdates( + body: Record, + res: Response, +): Partial | undefined { + const updates: Partial = {}; + if ('description' in body) { + const value = body['description']; + // Match the create-side rule: `description` is required and + // non-empty after trim. The previous update path silently + // accepted `" "` and let `mergeConfigurations` write a blank + // description to the file — divergent from create which would + // 422 the same payload. + if (typeof value !== 'string' || value.trim().length === 0) { + res.status(422).json({ + error: + '`description` must be a non-empty string (whitespace only is rejected) when provided', + code: 'invalid_config', + }); + return undefined; + } + if (Buffer.byteLength(value, 'utf8') > MAX_DESCRIPTION_BYTES) { + res.status(422).json({ + error: `\`description\` exceeds the ${MAX_DESCRIPTION_BYTES}-byte limit`, + code: 'invalid_config', + }); + return undefined; + } + updates.description = value; + } + if ('systemPrompt' in body) { + const value = body['systemPrompt']; + if (typeof value !== 'string' || value.trim().length === 0) { + // Mirror create's `systemPrompt.trim().length === 0` check. + // A whitespace-only prompt is effectively empty after YAML + // serialization and the agent can't operate without + // instructions, so reject at the boundary. + res.status(422).json({ + error: + '`systemPrompt` must be a non-empty string (whitespace only is rejected) when provided', + code: 'invalid_config', + }); + return undefined; + } + if (Buffer.byteLength(value, 'utf8') > MAX_SYSTEM_PROMPT_BYTES) { + res.status(422).json({ + error: `\`systemPrompt\` exceeds the ${MAX_SYSTEM_PROMPT_BYTES}-byte limit`, + code: 'invalid_config', + }); + return undefined; + } + updates.systemPrompt = value; + } + if ('tools' in body) { + const tools = parseStringArray(body['tools'], 'tools', res); + if (tools === null) return undefined; + if (tools !== undefined) updates.tools = tools; + } + if ('disallowedTools' in body) { + const disallowedTools = parseStringArray( + body['disallowedTools'], + 'disallowedTools', + res, + ); + if (disallowedTools === null) return undefined; + if (disallowedTools !== undefined) { + updates.disallowedTools = disallowedTools; + } + } + // Optional scalar fields. Match the create-side fail-closed posture + // so a typo like `model: 123` returns 422 instead of silently + // succeeding with no model change. + if (rejectIfPresentWrongType(body, 'model', 'string', res)) return undefined; + if (typeof body['model'] === 'string') updates.model = body['model']; + + if (rejectIfPresentWrongType(body, 'color', 'string', res)) return undefined; + if (typeof body['color'] === 'string') updates.color = body['color']; + + if (rejectIfPresentWrongType(body, 'approvalMode', 'string', res)) { + return undefined; + } + if (typeof body['approvalMode'] === 'string') { + if (!APPROVAL_MODES.includes(body['approvalMode'] as never)) { + res.status(422).json({ + error: `\`approvalMode\` must be one of ${JSON.stringify(APPROVAL_MODES)}`, + code: 'invalid_config', + }); + return undefined; + } + updates.approvalMode = body['approvalMode']; + } + + if (rejectIfPresentWrongType(body, 'background', 'boolean', res)) { + return undefined; + } + if (typeof body['background'] === 'boolean') { + updates.background = body['background']; + } + + if ('runConfig' in body) { + const sanitized = sanitizeRunConfig(body['runConfig'], res); + if (sanitized === null) return undefined; + updates.runConfig = sanitized; + } + return updates; +} + +function parseStringArray( + value: unknown, + field: string, + res: Response, +): string[] | undefined | null { + if (value === undefined) return undefined; + if (!Array.isArray(value) || value.some((v) => typeof v !== 'string')) { + res.status(422).json({ + error: `\`${field}\` must be an array of strings when provided`, + code: 'invalid_config', + }); + return null; + } + if (value.length > MAX_TOOLS_ENTRIES) { + res.status(422).json({ + error: `\`${field}\` exceeds the ${MAX_TOOLS_ENTRIES}-entry limit`, + code: 'invalid_config', + }); + return null; + } + if ((value as string[]).some((v) => v.length > MAX_TOOL_ID_LENGTH)) { + res.status(422).json({ + error: `\`${field}\` entries must be at most ${MAX_TOOL_ID_LENGTH} characters`, + code: 'invalid_config', + }); + return null; + } + return value as string[]; +} + +/** + * Returns `true` and sends a 422 when `body[key]` is present but the + * wrong scalar type. The caller then returns `undefined` to short- + * circuit the route. `false` covers both "absent" and "right type" so + * the caller proceeds. Used to give scalar fields the same fail-closed + * posture as `parseStringArray` / `sanitizeRunConfig`. + */ +function rejectIfPresentWrongType( + body: Record, + key: string, + expected: 'string' | 'boolean', + res: Response, +): boolean { + if (!(key in body)) return false; + if (typeof body[key] === expected) return false; + res.status(422).json({ + error: `\`${key}\` must be a ${expected} when provided`, + code: 'invalid_config', + }); + return true; +} + +/** + * Detect a no-op update — every supplied field already matches the + * existing agent's value. Without this check an empty (or + * value-unchanged) PATCH still rewrites the file, bumps mtime, and + * fans out a misleading `agent_changed` event. The recognized-field + * comparison covers what `parseAgentUpdates` produces; unknown keys + * are dropped upstream so we don't need to handle them here. + */ +function isNoOpUpdate( + existing: SubagentConfig, + updates: Partial, +): boolean { + if ( + updates.description !== undefined && + updates.description !== existing.description + ) { + return false; + } + if ( + updates.systemPrompt !== undefined && + updates.systemPrompt !== existing.systemPrompt + ) { + return false; + } + if ( + updates.tools !== undefined && + !shallowArrayEqual(updates.tools, existing.tools) + ) { + return false; + } + if ( + updates.disallowedTools !== undefined && + !shallowArrayEqual(updates.disallowedTools, existing.disallowedTools) + ) { + return false; + } + if (updates.model !== undefined && updates.model !== existing.model) { + return false; + } + if (updates.color !== undefined && updates.color !== existing.color) { + return false; + } + if ( + updates.approvalMode !== undefined && + updates.approvalMode !== existing.approvalMode + ) { + return false; + } + if ( + updates.background !== undefined && + updates.background !== existing.background + ) { + return false; + } + if (updates.runConfig !== undefined) { + // `SubagentManager.mergeConfigurations` MERGES `updates.runConfig` + // with `existing.runConfig` (existing keys preserved when not in + // updates), so the no-op check must compare only the keys the + // caller actually intends to change. Comparing every known field + // against `existing` would treat any partial update as non-no-op + // because absent keys would be `undefined` while existing has a + // value — a false positive that would re-emit `agent_changed` + // for a request that didn't actually mutate anything. + const e = existing.runConfig ?? {}; + const u = updates.runConfig; + if ('max_time_minutes' in u) { + if (u['max_time_minutes'] !== e['max_time_minutes']) return false; + } + if ('max_turns' in u) { + if (u['max_turns'] !== e['max_turns']) return false; + } + } + return true; +} + +function shallowArrayEqual( + a: readonly string[] | undefined, + b: readonly string[] | undefined, +): boolean { + if (a === b) return true; + if (!a || !b) return false; + if (a.length !== b.length) return false; + for (let i = 0; i < a.length; i++) if (a[i] !== b[i]) return false; + return true; +} + +/** + * Sanitize `runConfig` to only the documented fields. Without this + * filter `SubagentManager.serializeSubagent` writes whatever object the + * client sent into the agent's frontmatter, including unknown or + * YAML-sensitive keys that downstream parsers may choke on. Returning + * a fresh whitelist-shaped object also makes the wire contract + * self-documenting at the route boundary. + * + * - `undefined` is impossible here (caller checks `'runConfig' in body`). + * - `null` (sent) → 422 invalid_config (the route handler converts + * the null sentinel to a short-circuit). + * - Right-shape object → returns a new object with only `max_time_minutes` + * and `max_turns` if they validate as finite positive numbers. + */ +function sanitizeRunConfig( + raw: unknown, + res: Response, +): SubagentConfig['runConfig'] | null { + if (typeof raw !== 'object' || raw === null || Array.isArray(raw)) { + res.status(422).json({ + error: '`runConfig` must be an object when provided', + code: 'invalid_config', + }); + return null; + } + const input = raw as Record; + const out: Record = {}; + + if ('max_time_minutes' in input) { + const v = input['max_time_minutes']; + if (typeof v !== 'number' || !Number.isFinite(v) || v <= 0) { + res.status(422).json({ + error: + '`runConfig.max_time_minutes` must be a positive finite number when provided', + code: 'invalid_config', + }); + return null; + } + out['max_time_minutes'] = v; + } + if ('max_turns' in input) { + const v = input['max_turns']; + if ( + typeof v !== 'number' || + !Number.isFinite(v) || + v <= 0 || + !Number.isInteger(v) + ) { + res.status(422).json({ + error: '`runConfig.max_turns` must be a positive integer when provided', + code: 'invalid_config', + }); + return null; + } + out['max_turns'] = v; + } + return out as SubagentConfig['runConfig']; +} + +function toSummary(config: SubagentConfig): ServeWorkspaceAgentSummary { + const summary: ServeWorkspaceAgentSummary = { + kind: 'agent', + name: config.name, + description: config.description, + level: toServeLevel(config.level), + isBuiltin: config.isBuiltin === true || config.level === 'builtin', + hasTools: Array.isArray(config.tools) && config.tools.length > 0, + }; + if (config.model) summary.model = config.model; + if (config.color) summary.color = config.color; + if (config.background !== undefined) summary.background = config.background; + if (config.approvalMode) summary.approvalMode = config.approvalMode; + if (config.extensionName) summary.extensionName = config.extensionName; + if (config.filePath) summary.filePath = config.filePath; + return summary; +} + +function toDetail(config: SubagentConfig): ServeWorkspaceAgentDetail { + const detail: ServeWorkspaceAgentDetail = { + ...toSummary(config), + systemPrompt: config.systemPrompt, + }; + if (config.tools) detail.tools = [...config.tools]; + if (config.disallowedTools) { + detail.disallowedTools = [...config.disallowedTools]; + } + if (config.runConfig) { + // Explicit field pick rather than spread-with-cast. If + // `SubagentConfig.runConfig` gains new fields in core, the + // spread-then-cast pattern would silently leak them through the + // HTTP response without a compile error. Picking `max_time_minutes` + // and `max_turns` by name forces a deliberate schema bump if a + // future core field needs to surface on the daemon route. + const runConfig: ServeWorkspaceAgentDetail['runConfig'] = {}; + if (typeof config.runConfig.max_time_minutes === 'number') { + runConfig.max_time_minutes = config.runConfig.max_time_minutes; + } + if (typeof config.runConfig.max_turns === 'number') { + runConfig.max_turns = config.runConfig.max_turns; + } + detail.runConfig = runConfig; + } + return detail; +} + +function toServeLevel(level: SubagentLevel): ServeAgentLevel { + return level; +} + +/** + * Build a CRUD-scoped `SubagentManager` for the daemon. The + * underlying manager only touches three `Config` methods on its + * read/write paths (`getSdkMode`, `getProjectRoot`, + * `getActiveExtensions`); a `Proxy` makes any future expansion of + * that surface throw immediately rather than silently produce + * incorrect data. + */ +export function createDaemonSubagentManager( + boundWorkspace: string, +): SubagentManager { + const stub = { + getSdkMode: () => false, + getProjectRoot: () => boundWorkspace, + getActiveExtensions: () => [], + } as unknown as Record; + const guarded = new Proxy(stub, { + get(target, prop) { + if (prop in target) { + return (target as Record)[prop]; + } + // `then` is queried by Promise resolution machinery on object + // returns; returning undefined keeps async paths happy without + // implementing every Config method. + if (prop === 'then') return undefined; + throw new Error( + `qwen serve workspace agents: SubagentManager touched Config.` + + `${String(prop)} which the daemon stub does not implement. ` + + `Add it to createDaemonSubagentManager and audit safety.`, + ); + }, + // Mirror the `get` trap. Without a `has` trap, a SubagentManager + // path that does `if ('someMethod' in this.config)` would consult + // `Reflect.has(target, prop)` directly and silently return false + // for unimplemented methods — bypassing the throw the `get` trap + // is supposed to surface. With the trap, an `in` check on an + // unknown method throws the same way a property access would, so + // both code paths behave consistently. + has(target, prop) { + if (prop in target) return true; + // Allow `'then' in obj` so the runtime's thenable-detection + // continues to behave correctly. + if (prop === 'then') return false; + throw new Error( + `qwen serve workspace agents: SubagentManager probed Config.` + + `${String(prop)} via 'in' check; the daemon stub does not ` + + `implement it. Add it to createDaemonSubagentManager and ` + + `audit safety.`, + ); + }, + }) as unknown as Config; + return new SubagentManager(guarded); +} + +// Re-export the bridge error type used by route helpers so test files +// can import it from a single module without reaching into +// httpAcpBridge directly. +export { InvalidClientIdError }; diff --git a/packages/cli/src/serve/workspaceMemory.test.ts b/packages/cli/src/serve/workspaceMemory.test.ts new file mode 100644 index 00000000000..eadc2a4529d --- /dev/null +++ b/packages/cli/src/serve/workspaceMemory.test.ts @@ -0,0 +1,477 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { promises as fs } from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import express from 'express'; +import request from 'supertest'; +import { + afterEach, + beforeEach, + describe, + expect, + it, + vi, + type MockInstance, +} from 'vitest'; +import { Storage } from '@qwen-code/qwen-code-core'; +import { createMutationGate } from './auth.js'; +import { InvalidClientIdError, type HttpAcpBridge } from './httpAcpBridge.js'; +import type { BridgeEvent } from './eventBus.js'; +import { mountWorkspaceMemoryRoutes } from './workspaceMemory.js'; + +type RecordedEvent = Omit; + +function buildBridgeStub( + opts: { + knownIds?: Iterable; + } = {}, +): HttpAcpBridge & { events: RecordedEvent[] } { + const events: RecordedEvent[] = []; + const known = new Set(opts.knownIds ?? []); + return { + events, + publishWorkspaceEvent(event: RecordedEvent) { + events.push(event); + }, + knownClientIds() { + return new Set(known); + }, + // Methods below are not used by the memory routes; throw to keep + // unrelated tests from accidentally relying on them. + spawnOrAttach: () => { + throw new Error('not implemented'); + }, + loadSession: () => { + throw new Error('not implemented'); + }, + resumeSession: () => { + throw new Error('not implemented'); + }, + sendPrompt: () => { + throw new Error('not implemented'); + }, + cancelSession: () => { + throw new Error('not implemented'); + }, + subscribeEvents: () => { + throw new Error('not implemented'); + }, + closeSession: () => { + throw new Error('not implemented'); + }, + updateSessionMetadata: () => { + throw new Error('not implemented'); + }, + respondToPermission: () => { + throw new Error('not implemented'); + }, + respondToSessionPermission: () => { + throw new Error('not implemented'); + }, + listWorkspaceSessions: () => { + throw new Error('not implemented'); + }, + recordHeartbeat: () => { + throw new Error('not implemented'); + }, + getHeartbeatState: () => undefined, + getWorkspaceMcpStatus: async () => { + throw new Error('not implemented'); + }, + getWorkspaceSkillsStatus: async () => { + throw new Error('not implemented'); + }, + getWorkspaceProvidersStatus: async () => { + throw new Error('not implemented'); + }, + getSessionContextStatus: async () => { + throw new Error('not implemented'); + }, + getSessionSupportedCommandsStatus: async () => { + throw new Error('not implemented'); + }, + setSessionModel: async () => { + throw new Error('not implemented'); + }, + killSession: async () => {}, + detachClient: async () => {}, + sessionCount: 0, + pendingPermissionCount: 0, + killAllSync: () => {}, + shutdown: async () => {}, + } as unknown as HttpAcpBridge & { events: RecordedEvent[] }; +} + +function buildApp(opts: { + bridge: HttpAcpBridge; + boundWorkspace: string; + strictNoToken?: boolean; +}) { + const app = express(); + app.use(express.json({ limit: '10mb' })); + const mutate = createMutationGate({ + tokenConfigured: !opts.strictNoToken, + requireAuth: false, + }); + mountWorkspaceMemoryRoutes(app, { + bridge: opts.bridge, + boundWorkspace: opts.boundWorkspace, + mutate, + parseClientId: (req, res) => { + const raw = req.get('x-qwen-client-id'); + if (raw === undefined || raw === '') return undefined; + if (raw.length > 128 || !/^[A-Za-z0-9._:-]+$/.test(raw)) { + res.status(400).json({ + error: '`X-Qwen-Client-Id` must be a non-empty token', + code: 'invalid_client_id', + }); + return null; + } + return raw; + }, + safeBody: (req) => { + const raw = req.body; + if (typeof raw !== 'object' || raw === null || Array.isArray(raw)) { + return Object.create(null) as Record; + } + const out = Object.create(null) as Record; + for (const [k, v] of Object.entries(raw as Record)) { + if (k === '__proto__' || k === 'constructor' || k === 'prototype') { + continue; + } + out[k] = v; + } + return out; + }, + }); + return app; +} + +describe('workspace memory routes', () => { + let tmp: string; + let workspace: string; + let globalDir: string; + let getGlobalQwenDirSpy: MockInstance<() => string>; + + beforeEach(async () => { + tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'qwen-serve-memory-')); + workspace = path.join(tmp, 'workspace'); + globalDir = path.join(tmp, 'global'); + await fs.mkdir(workspace, { recursive: true }); + getGlobalQwenDirSpy = vi + .spyOn(Storage, 'getGlobalQwenDir') + .mockReturnValue(globalDir); + }); + + afterEach(async () => { + getGlobalQwenDirSpy.mockRestore(); + await fs.rm(tmp, { recursive: true, force: true }); + }); + + describe('GET /workspace/memory', () => { + it('returns idle status when no QWEN.md or AGENTS.md exists anywhere', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app).get('/workspace/memory'); + expect(res.status).toBe(200); + expect(res.body).toEqual({ + v: 1, + workspaceCwd: workspace, + initialized: false, + files: [], + totalBytes: 0, + fileCount: 0, + ruleCount: 0, + }); + }); + + it('reports workspace and global QWEN.md files with byte counts', async () => { + const wsFile = path.join(workspace, 'QWEN.md'); + const wsContent = 'workspace memory\n'; + await fs.writeFile(wsFile, wsContent, 'utf8'); + + await fs.mkdir(globalDir, { recursive: true }); + const globalFile = path.join(globalDir, 'QWEN.md'); + const globalContent = 'global memory\n'; + await fs.writeFile(globalFile, globalContent, 'utf8'); + + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app).get('/workspace/memory'); + + expect(res.status).toBe(200); + expect(res.body.initialized).toBe(true); + expect(res.body.fileCount).toBe(2); + expect(res.body.ruleCount).toBe(0); + expect(res.body.totalBytes).toBe( + Buffer.byteLength(wsContent) + Buffer.byteLength(globalContent), + ); + const paths = (res.body.files as Array<{ path: string }>).map( + (f) => f.path, + ); + expect(paths).toEqual(expect.arrayContaining([wsFile, globalFile])); + }); + }); + + describe('POST /workspace/memory', () => { + it('appends to workspace QWEN.md and emits memory_changed', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app) + .post('/workspace/memory') + .send({ scope: 'workspace', mode: 'append', content: '- entry one' }); + + expect(res.status).toBe(200); + expect(res.body.ok).toBe(true); + expect(res.body.mode).toBe('append'); + expect(res.body.filePath).toBe(path.join(workspace, 'QWEN.md')); + + const written = await fs.readFile( + path.join(workspace, 'QWEN.md'), + 'utf8', + ); + expect(written).toContain('- entry one'); + + const events = (bridge as unknown as { events: RecordedEvent[] }).events; + expect(events).toHaveLength(1); + expect(events[0]?.type).toBe('memory_changed'); + const data = events[0]?.data as Record; + expect(data['scope']).toBe('workspace'); + expect(data['mode']).toBe('append'); + expect(data['filePath']).toBe(path.join(workspace, 'QWEN.md')); + }); + + it('replaces workspace QWEN.md when mode=replace', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const filePath = path.join(workspace, 'QWEN.md'); + await fs.writeFile(filePath, 'old\n', 'utf8'); + + const res = await request(app) + .post('/workspace/memory') + .send({ scope: 'workspace', mode: 'replace', content: 'new\n' }); + + expect(res.status).toBe(200); + const written = await fs.readFile(filePath, 'utf8'); + expect(written).toBe('new\n'); + }); + + it('writes to the global ~/.qwen directory when scope=global', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app) + .post('/workspace/memory') + .send({ scope: 'global', mode: 'append', content: '- global note' }); + + expect(res.status).toBe(200); + expect(res.body.filePath).toBe(path.join(globalDir, 'QWEN.md')); + const written = await fs.readFile( + path.join(globalDir, 'QWEN.md'), + 'utf8', + ); + expect(written).toContain('- global note'); + }); + + it('rejects 400 invalid_scope on unknown scope value', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app) + .post('/workspace/memory') + .send({ scope: 'all', content: 'x' }); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_scope'); + }); + + it('rejects 400 invalid_mode on unknown mode value', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app) + .post('/workspace/memory') + .send({ scope: 'workspace', mode: 'merge', content: 'x' }); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_mode'); + }); + + it('rejects 400 invalid_content for non-string content', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app) + .post('/workspace/memory') + .send({ scope: 'workspace', content: 123 }); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_content'); + }); + + it('rejects 400 content_too_large above the 1 MB limit', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const big = 'x'.repeat(1024 * 1024 + 1); + const res = await request(app) + .post('/workspace/memory') + .send({ scope: 'workspace', content: big }); + expect(res.status).toBe(400); + expect(res.body.code).toBe('content_too_large'); + }); + + it('returns 401 token_required when strict gate fires on no-token loopback', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ + bridge, + boundWorkspace: workspace, + strictNoToken: true, + }); + const res = await request(app) + .post('/workspace/memory') + .send({ scope: 'workspace', content: '- x' }); + expect(res.status).toBe(401); + expect(res.body.code).toBe('token_required'); + }); + + it('rejects 400 invalid_client_id when X-Qwen-Client-Id is unknown', async () => { + const bridge = buildBridgeStub({ knownIds: ['client_known'] }); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app) + .post('/workspace/memory') + .set('X-Qwen-Client-Id', 'client_unknown') + .send({ scope: 'workspace', content: '- x' }); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_client_id'); + }); + + it('suppresses memory_changed event when append content is whitespace only', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app) + .post('/workspace/memory') + .send({ scope: 'workspace', mode: 'append', content: '\n\n \n' }); + expect(res.status).toBe(200); + expect(res.body.changed).toBe(false); + const events = (bridge as unknown as { events: RecordedEvent[] }).events; + expect(events).toHaveLength(0); + }); + + it('returns 413 memory_file_too_large when existing QWEN.md exceeds the 16 MB cap', async () => { + // Write a 17 MB existing QWEN.md, then attempt append. The + // helper's pre-read `fs.stat` must refuse with the typed + // error → the route maps it to 413. + const filePath = path.join(workspace, 'QWEN.md'); + // 17 MB of `x` characters. Bypass the helper's mutex / cap by + // writing directly via fs (simulating an externally-grown file + // outside the daemon's control). + const big = 'x'.repeat(17 * 1024 * 1024); + await fs.writeFile(filePath, big, 'utf8'); + + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app) + .post('/workspace/memory') + .send({ scope: 'workspace', mode: 'append', content: '- entry' }); + + expect(res.status).toBe(413); + expect(res.body.code).toBe('memory_file_too_large'); + expect(res.body.scope).toBe('workspace'); + expect(res.body.mode).toBe('append'); + expect(res.body.bytes).toBe(17 * 1024 * 1024); + expect(res.body.limit).toBe(16 * 1024 * 1024); + // Default response: no filePath, no path-embedding error message. + expect(res.body.filePath).toBeUndefined(); + expect(res.body.error).not.toContain(filePath); + }); + + it('omits errorMessage + filePath in 500/413 responses unless QWEN_SERVE_DEBUG is on', async () => { + // Windows ignores Unix-style permission bits passed to + // `fs.chmod` — the directory stays writable, the POST succeeds + // with 200, and the EACCES path this test exercises is + // unreachable. The route logic itself is platform-agnostic; the + // Ubuntu + macOS runs cover it. Mirrors the + // `process.platform === 'win32'` early-return idiom already used + // in `customBanner.test.ts:232`. + if (process.platform === 'win32') return; + + // Default: production response carries no `errorMessage` or + // `filePath` fields — operators read the daemon stderr log + // for the path. Setting QWEN_SERVE_DEBUG=1 enables both. + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + + // Force a 500 by making the workspace QWEN.md unwritable. We + // chmod the WORKSPACE directory (not the file) so `mkdir` and + // `writeFile` will fail with EACCES. + const before = await fs.stat(workspace); + await fs.chmod(workspace, 0o555); + const prevDebug = process.env['QWEN_SERVE_DEBUG']; + try { + delete process.env['QWEN_SERVE_DEBUG']; + const res = await request(app).post('/workspace/memory').send({ + scope: 'workspace', + mode: 'append', + content: '- entry', + }); + expect(res.status).toBe(500); + expect(res.body.code).toBe('file_error'); + expect(res.body.scope).toBe('workspace'); + expect(res.body.mode).toBe('append'); + // Default response: no errorMessage, no filePath. + expect(res.body.errorMessage).toBeUndefined(); + expect(res.body.filePath).toBeUndefined(); + + // Toggle debug back on; the same payload now carries the + // detail. + process.env['QWEN_SERVE_DEBUG'] = '1'; + const debugRes = await request(app).post('/workspace/memory').send({ + scope: 'workspace', + mode: 'append', + content: '- entry', + }); + expect(debugRes.status).toBe(500); + expect(typeof debugRes.body.errorMessage).toBe('string'); + } finally { + if (prevDebug === undefined) delete process.env['QWEN_SERVE_DEBUG']; + else process.env['QWEN_SERVE_DEBUG'] = prevDebug; + await fs.chmod(workspace, before.mode); + } + }); + + it('returns 500 memory_discovery_failed when GET helper throws unexpectedly', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + // Force the helper to throw by spying on `Storage.getGlobalQwenDir` + // — every call site of the discovery walk uses it. + const failGlobal = vi + .spyOn(Storage, 'getGlobalQwenDir') + .mockImplementation(() => { + throw new Error('boom'); + }); + try { + const res = await request(app).get('/workspace/memory'); + expect(res.status).toBe(500); + expect(res.body.code).toBe('memory_discovery_failed'); + } finally { + failGlobal.mockRestore(); + } + }); + + it('stamps originatorClientId on the memory_changed event for known clients', async () => { + const bridge = buildBridgeStub({ knownIds: ['client_a'] }); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app) + .post('/workspace/memory') + .set('X-Qwen-Client-Id', 'client_a') + .send({ scope: 'workspace', mode: 'append', content: '- x' }); + expect(res.status).toBe(200); + const events = (bridge as unknown as { events: RecordedEvent[] }).events; + expect(events[0]?.originatorClientId).toBe('client_a'); + }); + + // Reference InvalidClientIdError in case future refactors rename + // it — keeps the import non-tree-shakeable surface a real symbol. + it('exposes InvalidClientIdError from the bridge module', () => { + expect(typeof InvalidClientIdError).toBe('function'); + }); + }); +}); diff --git a/packages/cli/src/serve/workspaceMemory.ts b/packages/cli/src/serve/workspaceMemory.ts new file mode 100644 index 00000000000..e7a0e7ef2c6 --- /dev/null +++ b/packages/cli/src/serve/workspaceMemory.ts @@ -0,0 +1,440 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { promises as fs } from 'node:fs'; +import * as path from 'node:path'; +import type { Application, Request, RequestHandler, Response } from 'express'; +import { + Storage, + WorkspaceMemoryFileTooLargeError, + WorkspaceMemoryWriteTimeoutError, + getAllGeminiMdFilenames, + writeWorkspaceContextFile, +} from '@qwen-code/qwen-code-core'; +import { writeStderrLine } from '../utils/stdioHelpers.js'; +import { isServeDebugMode } from './debugMode.js'; +import type { HttpAcpBridge } from './httpAcpBridge.js'; +import { + createIdleWorkspaceMemoryStatus, + STATUS_SCHEMA_VERSION, + type ServeContextFileScope, + type ServeWorkspaceMemoryFile, + type ServeWorkspaceMemoryStatus, +} from './status.js'; + +/** + * Issue #4175 PR 16: workspace memory CRUD routes. + * + * `GET /workspace/memory` returns the daemon's snapshot of explicit + * `QWEN.md` / `AGENTS.md` files reachable from the bound workspace + * plus the user's `~/.qwen/` global. Read-only; returns + * `initialized: false` and an empty `files` list when no files exist + * (no synthetic 500s, mirroring PR 12's read-only routes). + * + * `POST /workspace/memory` accepts `{ scope, content, mode }` and + * forwards to `writeWorkspaceContextFile`. Strict mutation gate; on + * success, fans out a `memory_changed` event onto every active + * session's bus so adapters can refresh cached snapshots. + * + * Both routes are filesystem-only — neither spawns the ACP child. + * + * **Absolute filePath disclosure note**: success / 413 / GET-list + * responses include absolute on-disk paths (`/work//QWEN.md`, + * `/Users//.qwen/QWEN.md`). This is by design for a daemon + * contract: clients pre-flight `caps.workspaceCwd` to learn the + * bound workspace root and can compute relative paths if they + * prefer; the global scope (`~/.qwen/QWEN.md`) is NOT under the + * workspace root, so rewriting to a workspace-relative form would + * lose information. The bearer-token gate + the daemon's loopback- + * default binding already restrict who can see these paths. If a + * future deployment shape needs path redaction (e.g. multi-tenant + * over a shared host), it should land as a `--redact-paths` + * deployment toggle rather than a per-route default flip — tracked + * with PR 24's `--redact-errors` policy work, not in PR 16. + */ + +export interface WorkspaceMemoryRouteDeps { + bridge: HttpAcpBridge; + boundWorkspace: string; + /** + * `mutate({ strict: true })`-style middleware factory from PR 15. + * Passed in so `server.ts` stays the single composition root for + * the mutation-gate decisions. + */ + mutate: (opts?: { strict?: boolean }) => RequestHandler; + /** + * Pre-validated client id parser. Returns `undefined` for absent + * headers, the parsed id for valid ones, and `null` after sending + * its own 400 response (so the route handler must short-circuit). + * Re-uses `parseClientIdHeader` from `server.ts`. + */ + parseClientId: (req: Request, res: Response) => string | undefined | null; + /** `safeBody` from `server.ts` — strips prototype-pollution keys. */ + safeBody: (req: Request) => Record; +} + +const MAX_MEMORY_CONTENT_BYTES = 1024 * 1024; + +/** Mount the two memory routes on the supplied Express app. */ +export function mountWorkspaceMemoryRoutes( + app: Application, + deps: WorkspaceMemoryRouteDeps, +): void { + app.get('/workspace/memory', async (_req, res) => { + try { + const status = await collectWorkspaceMemoryStatus(deps.boundWorkspace); + res.status(200).json(status); + } catch (err) { + // Per-file stat failures are caught inside + // `collectWorkspaceMemoryStatus` and surfaced in-band via + // `errors[]` with `errorKind: 'stat_failed'`. The outer catch + // here only fires on programmer error (an upstream helper + // throws unexpectedly). Return 500 — a 200-with-errors response + // for a complete-discovery failure would silently look healthy + // to status dashboards counting non-2xx as failures, which is + // exactly the silent-failure mode PR 12's read-only routes + // avoided by routing bridge errors through `sendBridgeError`. + writeStderrLine( + `qwen serve: GET /workspace/memory failed: ${ + err instanceof Error ? (err.stack ?? err.message) : String(err) + }`, + ); + res.status(500).json({ + error: 'Failed to discover workspace memory', + code: 'memory_discovery_failed', + }); + } + }); + + app.post( + '/workspace/memory', + deps.mutate({ strict: true }), + async (req, res) => { + const body = deps.safeBody(req); + + const scope = body['scope']; + if (scope !== 'workspace' && scope !== 'global') { + res.status(400).json({ + error: '`scope` must be "workspace" or "global"', + code: 'invalid_scope', + }); + return; + } + + const modeRaw = body['mode']; + if ( + modeRaw !== undefined && + modeRaw !== 'append' && + modeRaw !== 'replace' + ) { + res.status(400).json({ + error: '`mode` must be "append", "replace", or omitted', + code: 'invalid_mode', + }); + return; + } + const mode: 'append' | 'replace' = + modeRaw === 'replace' ? 'replace' : 'append'; + + const content = body['content']; + if (typeof content !== 'string') { + res.status(400).json({ + error: '`content` must be a string', + code: 'invalid_content', + }); + return; + } + if (Buffer.byteLength(content, 'utf8') > MAX_MEMORY_CONTENT_BYTES) { + res.status(400).json({ + error: `\`content\` exceeds the ${MAX_MEMORY_CONTENT_BYTES}-byte limit`, + code: 'content_too_large', + }); + return; + } + + const clientId = deps.parseClientId(req, res); + if (clientId === null) return; + let originatorClientId: string | undefined; + if (clientId !== undefined) { + // Mirror the workspaceAgents.ts `resolveOriginatorClientId` + // posture: validate against `bridge.knownClientIds()`, send + // 400 directly, return `null` so the caller short-circuits. + // Previously this branch threw `InvalidClientIdError` and + // caught it locally — wenshao round-6 flagged the + // throw-vs-direct-400 inconsistency between the two route + // files. Aligning the call sites now removes the surface + // divergence; the deeper DRY refactor (one shared helper + // module) still lands in the cross-Wave-4 sweep with PR + // 17/19/20/21. + const known = deps.bridge.knownClientIds(); + if (!known.has(clientId)) { + res.status(400).json({ + error: `Client id "${clientId}" is not registered for this workspace`, + code: 'invalid_client_id', + clientId, + }); + return; + } + originatorClientId = clientId; + } + + try { + const result = await writeWorkspaceContextFile({ + scope, + mode, + content, + projectRoot: deps.boundWorkspace, + }); + const responseBody = { + ok: true as const, + filePath: result.filePath, + bytesWritten: result.bytesWritten, + mode, + changed: result.changed, + }; + // Only fan out a `memory_changed` event when the helper + // actually mutated the file. Whitespace-only appends short- + // circuit upstream (writeContextFile.ts) and would otherwise + // emit a misleading "memory just changed" toast across every + // SSE subscriber for a request that did nothing. + if (result.changed) { + deps.bridge.publishWorkspaceEvent({ + type: 'memory_changed', + data: { + scope, + filePath: result.filePath, + mode, + bytesWritten: result.bytesWritten, + }, + ...(originatorClientId ? { originatorClientId } : {}), + }); + } + res.status(200).json(responseBody); + } catch (err) { + // 413 + structured fields for the "memory file is past the + // safe-append cap" case so callers can tell pathological + // file size apart from generic file errors. The helper + // refuses to pull >16 MB into memory on append; clients + // either trim the file or switch to mode=replace. + if (err instanceof WorkspaceMemoryWriteTimeoutError) { + writeStderrLine( + `qwen serve: POST /workspace/memory timeout — file lock at ` + + `${err.filePath} did not acquire within ${err.timeoutMs}ms ` + + `(stalled FS / OneDrive / NFS)`, + ); + const debug = isServeDebugMode(); + res.status(500).json({ + error: debug + ? err.message + : 'Workspace memory write timed out waiting for the per-file lock. Retry or restart the daemon.', + code: 'memory_write_timeout', + scope, + mode, + timeoutMs: err.timeoutMs, + ...(debug ? { filePath: err.filePath } : {}), + }); + return; + } + if (err instanceof WorkspaceMemoryFileTooLargeError) { + writeStderrLine( + `qwen serve: POST /workspace/memory refused — existing file ` + + `${err.filePath} is ${err.bytes} bytes (cap ${err.limit})`, + ); + // Path disclosure: both `error` (which embeds the absolute + // file path in the constructor message — see + // `WorkspaceMemoryFileTooLargeError`) and `filePath` are + // gated behind QWEN_SERVE_DEBUG so production responses + // don't include `/Users//.qwen/...` in the body. + // Operators triaging an issue locally enable the debug + // toggle to get the full text; in default mode SDK + // callers branch on `code` + `bytes` / `limit` instead + // (the structured discriminator survives without the + // disclosure). + const debug = isServeDebugMode(); + res.status(413).json({ + error: debug + ? err.message + : 'Existing memory file exceeds the safe-append cap. Trim the file or POST with mode=replace.', + code: 'memory_file_too_large', + scope, + mode, + ...(debug ? { filePath: err.filePath } : {}), + bytes: err.bytes, + limit: err.limit, + }); + return; + } + writeStderrLine( + `qwen serve: POST /workspace/memory failed (scope=${scope} mode=${mode}): ${ + err instanceof Error ? (err.stack ?? err.message) : String(err) + }`, + ); + // Surface enough context for callers to debug without leaking + // absolute paths in the response body. `osCode` (`EACCES` / + // `EROFS` / `EDQUOT` / `ENOSPC` / ...) stays unconditional so + // SDK clients can branch on the failure class. The full + // `errorMessage` (which often embeds the file path on Node's + // ENOENT/EACCES messages) is gated behind `QWEN_SERVE_DEBUG`. + // Without the debug toggle, callers see only the generic + // `error` + `code` + `osCode` envelope; the daemon's stderr + // log has the full message for the operator. + const osCode = + err && typeof err === 'object' && 'code' in err + ? (err as { code?: unknown }).code + : undefined; + const debug = isServeDebugMode(); + res.status(500).json({ + error: 'Failed to write workspace memory', + code: 'file_error', + scope, + mode, + ...(typeof osCode === 'string' ? { osCode } : {}), + ...(debug + ? { + errorMessage: err instanceof Error ? err.message : String(err), + } + : {}), + }); + } + }, + ); +} + +interface DiscoveredFile { + absolutePath: string; + scope: ServeContextFileScope; + bytes: number; +} + +/** + * Filesystem-only discovery of explicit `QWEN.md` / `AGENTS.md` + * files reachable from the daemon's bound workspace plus the user's + * `~/.qwen/` global directory. + * + * Discovers the bound-workspace-root file(s) (no parent-directory + * walk in this version) plus the global dir. `walkWorkspaceForMemory` + * keeps a guarded upward-walk loop body for a future hierarchical + * mode but breaks after iteration 1 today; callers should treat the + * surface as "workspace root + global". Auto-memory (the `MEMORY.md` + * index + per-type files) is intentionally NOT included; that's PR + * 16.5's responsibility per scope decision in issue #4175. Path- + * based rules (`.qwen/rules/`) are also out of scope for v1. + */ +async function collectWorkspaceMemoryStatus( + boundWorkspace: string, +): Promise { + const filenames = new Set(getAllGeminiMdFilenames()); + const files: DiscoveredFile[] = []; + const errors: ServeWorkspaceMemoryStatus['errors'] = []; + + const workspaceFiles = await walkWorkspaceForMemory( + boundWorkspace, + filenames, + errors, + ); + files.push(...workspaceFiles); + + const globalDir = Storage.getGlobalQwenDir(); + for (const filename of filenames) { + const candidate = path.join(globalDir, filename); + try { + const stat = await fs.stat(candidate); + if (stat.isFile()) { + files.push({ + absolutePath: candidate, + scope: 'global', + bytes: stat.size, + }); + } + } catch (err) { + if (!isEnoent(err)) { + errors.push({ + kind: 'memory_file', + status: 'error', + error: err instanceof Error ? err.message : String(err), + errorKind: 'stat_failed', + hint: candidate, + }); + } + } + } + + if (files.length === 0 && errors.length === 0) { + return createIdleWorkspaceMemoryStatus(boundWorkspace); + } + + const totalBytes = files.reduce((acc, f) => acc + f.bytes, 0); + const result: ServeWorkspaceMemoryStatus = { + v: STATUS_SCHEMA_VERSION, + workspaceCwd: boundWorkspace, + initialized: true, + files: files.map( + (f): ServeWorkspaceMemoryFile => ({ + kind: 'memory_file', + path: f.absolutePath, + scope: f.scope, + bytes: f.bytes, + }), + ), + totalBytes, + fileCount: files.length, + ruleCount: 0, + }; + if (errors.length > 0) result.errors = errors; + return result; +} + +/** + * Stat each known memory filename (`QWEN.md`, `AGENTS.md`) at the + * bound workspace root and return the matches. v1 does not walk + * parent directories — that's reserved for PR 16.5's hierarchical + * mode, which will replace this helper with a real upward walk + * (originally drafted in this file but removed at glm-5.1's review + * because the loop body was reachable only on its first iteration + * via `if (cursor === start) break`, making the cap and `seen` set + * dead code that confused reviewers). When PR 16.5 lifts the cap, + * the new implementation lands as a fresh upward walk rather than + * "uncomment lines". + */ +async function walkWorkspaceForMemory( + start: string, + filenames: ReadonlySet, + errors: NonNullable, +): Promise { + const out: DiscoveredFile[] = []; + for (const filename of filenames) { + const candidate = path.join(start, filename); + try { + const stat = await fs.stat(candidate); + if (stat.isFile()) { + out.push({ + absolutePath: candidate, + scope: 'workspace', + bytes: stat.size, + }); + } + } catch (err) { + if (!isEnoent(err)) { + errors.push({ + kind: 'memory_file', + status: 'error', + error: err instanceof Error ? err.message : String(err), + errorKind: 'stat_failed', + hint: candidate, + }); + } + } + } + return out; +} + +function isEnoent(err: unknown): boolean { + return ( + typeof err === 'object' && + err !== null && + (err as { code?: string }).code === 'ENOENT' + ); +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 03fd4867177..55be58312da 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -169,6 +169,10 @@ export * from './memory/types.js'; export * from './memory/paths.js'; export * from './memory/store.js'; export * from './memory/const.js'; +// Issue #4175 PR 16: write helper for hierarchical context files, +// re-exported so the `qwen serve` daemon can mutate workspace memory +// via `POST /workspace/memory` without depending on internal paths. +export * from './memory/writeContextFile.js'; // ============================================================================ // IDE Support diff --git a/packages/core/src/memory/writeContextFile.test.ts b/packages/core/src/memory/writeContextFile.test.ts new file mode 100644 index 00000000000..3664dd8cc15 --- /dev/null +++ b/packages/core/src/memory/writeContextFile.test.ts @@ -0,0 +1,394 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { promises as fs } from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { Storage } from '../config/storage.js'; +import { + AGENT_CONTEXT_FILENAME, + DEFAULT_CONTEXT_FILENAME, + MEMORY_SECTION_HEADER, + setGeminiMdFilename, +} from './const.js'; +import { writeWorkspaceContextFile } from './writeContextFile.js'; + +describe('writeWorkspaceContextFile', () => { + let tmpRoot: string; + let workspace: string; + let globalDir: string; + let getGlobalQwenDirSpy: ReturnType; + + beforeEach(async () => { + tmpRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'qwen-write-context-')); + workspace = path.join(tmpRoot, 'workspace'); + globalDir = path.join(tmpRoot, 'global'); + await fs.mkdir(workspace, { recursive: true }); + getGlobalQwenDirSpy = vi + .spyOn(Storage, 'getGlobalQwenDir') + .mockReturnValue(globalDir); + }); + + afterEach(async () => { + getGlobalQwenDirSpy.mockRestore(); + await fs.rm(tmpRoot, { recursive: true, force: true }); + }); + + it('creates QWEN.md with a fresh section header on first append', async () => { + const result = await writeWorkspaceContextFile({ + scope: 'workspace', + mode: 'append', + content: '- first entry', + projectRoot: workspace, + }); + + expect(result.filePath).toBe( + path.join(workspace, DEFAULT_CONTEXT_FILENAME), + ); + const written = await fs.readFile(result.filePath, 'utf8'); + expect(written).toBe(`${MEMORY_SECTION_HEADER}\n- first entry\n`); + expect(result.bytesWritten).toBe(Buffer.byteLength(written, 'utf8')); + }); + + it('appends under existing section header', async () => { + const initial = `# project notes\n\n${MEMORY_SECTION_HEADER}\n- first entry\n`; + const filePath = path.join(workspace, DEFAULT_CONTEXT_FILENAME); + await fs.writeFile(filePath, initial, 'utf8'); + + await writeWorkspaceContextFile({ + scope: 'workspace', + mode: 'append', + content: '- second entry', + projectRoot: workspace, + }); + + const written = await fs.readFile(filePath, 'utf8'); + expect(written).toBe( + `# project notes\n\n${MEMORY_SECTION_HEADER}\n- first entry\n- second entry\n`, + ); + }); + + it('inserts a section header when file lacks one', async () => { + const initial = '# project notes\n'; + const filePath = path.join(workspace, DEFAULT_CONTEXT_FILENAME); + await fs.writeFile(filePath, initial, 'utf8'); + + await writeWorkspaceContextFile({ + scope: 'workspace', + mode: 'append', + content: '- entry', + projectRoot: workspace, + }); + + const written = await fs.readFile(filePath, 'utf8'); + expect(written).toBe( + `# project notes\n\n${MEMORY_SECTION_HEADER}\n- entry\n`, + ); + }); + + it('replaces file contents in replace mode', async () => { + const filePath = path.join(workspace, DEFAULT_CONTEXT_FILENAME); + await fs.writeFile(filePath, 'old contents\n', 'utf8'); + + const result = await writeWorkspaceContextFile({ + scope: 'workspace', + mode: 'replace', + content: 'replacement\n', + projectRoot: workspace, + }); + + const written = await fs.readFile(filePath, 'utf8'); + expect(written).toBe('replacement\n'); + expect(result.bytesWritten).toBe( + Buffer.byteLength('replacement\n', 'utf8'), + ); + }); + + it('writes to the global ~/.qwen directory when scope=global', async () => { + const result = await writeWorkspaceContextFile({ + scope: 'global', + mode: 'append', + content: '- global entry', + projectRoot: workspace, + }); + + expect(result.filePath).toBe( + path.join(globalDir, DEFAULT_CONTEXT_FILENAME), + ); + expect(getGlobalQwenDirSpy).toHaveBeenCalled(); + const written = await fs.readFile(result.filePath, 'utf8'); + expect(written).toBe(`${MEMORY_SECTION_HEADER}\n- global entry\n`); + }); + + it('creates the parent directory when missing', async () => { + const nested = path.join(workspace, 'nested', 'deep'); + await writeWorkspaceContextFile({ + scope: 'workspace', + mode: 'append', + content: '- entry', + projectRoot: nested, + }); + + const created = await fs.readFile( + path.join(nested, DEFAULT_CONTEXT_FILENAME), + 'utf8', + ); + expect(created).toContain('- entry'); + }); + + it('rejects non-absolute projectRoot', async () => { + await expect( + writeWorkspaceContextFile({ + scope: 'workspace', + mode: 'append', + content: 'x', + projectRoot: 'relative/path', + }), + ).rejects.toThrow(/projectRoot must be absolute/); + }); + + it('skips the write entirely when append content is whitespace only', async () => { + const filePath = path.join(workspace, DEFAULT_CONTEXT_FILENAME); + await fs.writeFile(filePath, 'preserved\n', 'utf8'); + + // Spy on `fs.writeFile` rather than relying on filesystem mtime + // resolution. macOS HFS+ has 1-second mtime resolution; a quick + // re-write inside the same second would leave `mtimeMs` unchanged + // and let a regression slip through. The spy makes the + // "writeFile was never called" invariant explicit and platform- + // independent. + const writeFileSpy = vi.spyOn(fs, 'writeFile'); + try { + const result = await writeWorkspaceContextFile({ + scope: 'workspace', + mode: 'append', + content: '\n\n', + projectRoot: workspace, + }); + + const written = await fs.readFile(filePath, 'utf8'); + expect(written).toBe('preserved\n'); + // `bytesWritten: 0` because the no-op short-circuit wrote zero + // bytes — NOT the existing file size. Earlier revisions returned + // `stat.size` here, which conflated two semantics and let + // clients accumulating `sum(bytesWritten)` count the existing + // file every whitespace POST. + expect(result.bytesWritten).toBe(0); + expect(result.changed).toBe(false); + // The no-op short-circuit must not call writeFile at all. + expect(writeFileSpy).not.toHaveBeenCalled(); + } finally { + writeFileSpy.mockRestore(); + } + }); + + it('serializes concurrent appends so no entry is lost', async () => { + // Spawn 10 parallel appends with unique content. Without the + // per-file mutex, the read-compose-write race in + // `composeAppendedContent` lets later writes overwrite earlier + // ones — at least one entry would be missing from the final file. + const PARALLEL = 10; + const writes = Array.from({ length: PARALLEL }, (_, i) => + writeWorkspaceContextFile({ + scope: 'workspace', + mode: 'append', + content: `- entry ${i}`, + projectRoot: workspace, + }), + ); + const results = await Promise.all(writes); + + const filePath = path.join(workspace, DEFAULT_CONTEXT_FILENAME); + const written = await fs.readFile(filePath, 'utf8'); + for (let i = 0; i < PARALLEL; i++) { + expect(written).toContain(`- entry ${i}`); + } + // All N writes report changed; none short-circuited. + expect(results.every((r) => r.changed)).toBe(true); + // Exactly one section header — the lock keeps the + // "is-section-present" check consistent across the group, so we + // never insert duplicate headers. + const headerCount = written.split(MEMORY_SECTION_HEADER).length - 1; + expect(headerCount).toBe(1); + }); + + it('marks `changed: false` for a no-op append against a missing file', async () => { + const result = await writeWorkspaceContextFile({ + scope: 'workspace', + mode: 'append', + content: ' ', + projectRoot: workspace, + }); + expect(result.changed).toBe(false); + expect(result.bytesWritten).toBe(0); + await expect( + fs.access(path.join(workspace, DEFAULT_CONTEXT_FILENAME)), + ).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('inserts new entries inside the MEMORY section, not past a later heading', async () => { + // File where the MEMORY section is followed by other prose. + // Without the section-boundary fix the new entry would be + // appended to EOF, landing it inside the `## post` section. + const filePath = path.join(workspace, DEFAULT_CONTEXT_FILENAME); + const initial = `# pre\n\n${MEMORY_SECTION_HEADER}\n- first\n\n## post\nstuff\n`; + await fs.writeFile(filePath, initial, 'utf8'); + + await writeWorkspaceContextFile({ + scope: 'workspace', + mode: 'append', + content: '- second', + projectRoot: workspace, + }); + + const written = await fs.readFile(filePath, 'utf8'); + expect(written).toBe( + `# pre\n\n${MEMORY_SECTION_HEADER}\n- first\n- second\n\n## post\nstuff\n`, + ); + // `- second` must be inside the memory block, not after `stuff`. + const memorySection = written.indexOf(MEMORY_SECTION_HEADER); + const postSection = written.indexOf('## post'); + const secondIdx = written.indexOf('- second'); + expect(secondIdx).toBeGreaterThan(memorySection); + expect(secondIdx).toBeLessThan(postSection); + }); + + it('does not split a memory entry that contains `## ` inside a fenced code block', async () => { + // Round-7 [Critical] glm-5.1: the `\n## ` boundary heuristic was + // matching `## ` lines INSIDE user-authored fenced code blocks + // (common in QWEN.md memory entries that quote API docs with + // markdown headings). The old impl would insert the new entry + // mid-fence, splitting the existing entry. Code-fence-aware + // detection skips matches inside ``` ``` `` ` blocks. + const filePath = path.join(workspace, DEFAULT_CONTEXT_FILENAME); + const fencedEntry = [ + `${MEMORY_SECTION_HEADER}`, + '- API example:', + '```markdown', + '## Request Body', + 'POST /api/thing', + '```', + '', + ].join('\n'); + await fs.writeFile(filePath, fencedEntry, 'utf8'); + + await writeWorkspaceContextFile({ + scope: 'workspace', + mode: 'append', + content: '- next entry', + projectRoot: workspace, + }); + + const written = await fs.readFile(filePath, 'utf8'); + // The new entry must land AFTER the fence, not inside it. + const fenceClose = written.lastIndexOf('```'); + const newEntry = written.indexOf('- next entry'); + expect(newEntry).toBeGreaterThan(fenceClose); + // The fenced `## Request Body` must still be intact (no insert + // before / inside the code block). + expect(written).toContain( + '```markdown\n## Request Body\nPOST /api/thing\n```', + ); + }); + + it('still respects real `## ` headings outside code fences', async () => { + const filePath = path.join(workspace, DEFAULT_CONTEXT_FILENAME); + // Memory section, then a fenced `## ` (must be skipped), then a + // real `## post` heading (must be honored as the boundary). + const initial = [ + `${MEMORY_SECTION_HEADER}`, + '- existing', + '```', + '## fake heading inside fence', + '```', + '', + '## post', + 'tail', + '', + ].join('\n'); + await fs.writeFile(filePath, initial, 'utf8'); + + await writeWorkspaceContextFile({ + scope: 'workspace', + mode: 'append', + content: '- new', + projectRoot: workspace, + }); + + const written = await fs.readFile(filePath, 'utf8'); + const realPost = written.indexOf('## post'); + const newEntry = written.indexOf('- new'); + expect(newEntry).toBeLessThan(realPost); + expect(newEntry).toBeGreaterThan(written.indexOf('- existing')); + }); + + it('appends to EOF when the MEMORY section is the last block', async () => { + // Sanity: when no later heading follows, behavior is the + // pre-fix append-to-end path (still inside the section because + // the section IS the tail). + const filePath = path.join(workspace, DEFAULT_CONTEXT_FILENAME); + const initial = `# pre\n\n${MEMORY_SECTION_HEADER}\n- a\n`; + await fs.writeFile(filePath, initial, 'utf8'); + + await writeWorkspaceContextFile({ + scope: 'workspace', + mode: 'append', + content: '- b', + projectRoot: workspace, + }); + + const written = await fs.readFile(filePath, 'utf8'); + expect(written).toBe(`# pre\n\n${MEMORY_SECTION_HEADER}\n- a\n- b\n`); + }); + + it('does not create the parent directory on a no-op append', async () => { + // Whitespace-only append targeting a non-existent nested path + // must NOT call fs.mkdir — the no-op detection short-circuits + // BEFORE acquiring the lock or touching the filesystem. Without + // this, an empty POST would still bump the parent directory's + // mtime even though the helper reports `changed: false`. + const nested = path.join(workspace, 'never-exists'); + const result = await writeWorkspaceContextFile({ + scope: 'workspace', + mode: 'append', + content: '\n\n', + projectRoot: nested, + }); + expect(result.changed).toBe(false); + await expect(fs.access(nested)).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('honors setGeminiMdFilename overrides so POST targets the same file GET surfaces', async () => { + // Round-trip the `setGeminiMdFilename` override: with the prior + // `DEFAULT_CONTEXT_FILENAME` hard-code, a deployment that switched + // the context filename to `AGENTS.md` saw GET list the new file + // but POST keep writing to `QWEN.md`. The fix routes + // `resolveContextFilePath` through `getCurrentGeminiMdFilename()` + // so both surfaces agree. + try { + setGeminiMdFilename(AGENT_CONTEXT_FILENAME); + const result = await writeWorkspaceContextFile({ + scope: 'workspace', + mode: 'append', + content: '- entry', + projectRoot: workspace, + }); + expect(result.filePath).toBe( + path.join(workspace, AGENT_CONTEXT_FILENAME), + ); + const written = await fs.readFile(result.filePath, 'utf8'); + expect(written).toContain('- entry'); + // The legacy QWEN.md must NOT have been written — the prior + // hard-coded behavior would have created it here. + await expect( + fs.access(path.join(workspace, DEFAULT_CONTEXT_FILENAME)), + ).rejects.toMatchObject({ code: 'ENOENT' }); + } finally { + setGeminiMdFilename(DEFAULT_CONTEXT_FILENAME); + } + }); +}); diff --git a/packages/core/src/memory/writeContextFile.ts b/packages/core/src/memory/writeContextFile.ts new file mode 100644 index 00000000000..ac8789a947d --- /dev/null +++ b/packages/core/src/memory/writeContextFile.ts @@ -0,0 +1,428 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { promises as fs } from 'node:fs'; +import * as path from 'node:path'; +import { + E_TIMEOUT, + Mutex, + withTimeout, + type MutexInterface, +} from 'async-mutex'; +import { Storage } from '../config/storage.js'; +import { getCurrentGeminiMdFilename, MEMORY_SECTION_HEADER } from './const.js'; + +/** + * Per-resolved-file mutex map. Two simultaneous `writeWorkspaceContextFile` + * calls targeting the same file would otherwise race read-then-write: + * both reads see the same existing content, both compose new content in + * memory, and the later `fs.writeFile` overwrites the earlier append. + * Result is a silently-lost entry with both callers observing success. + * + * Pattern mirrors `packages/core/src/utils/jsonl-utils.ts:36-46`. The + * Map grows by one entry per unique resolved path; production has at + * most two (workspace QWEN.md + global QWEN.md), so no cleanup is + * required. Tests use tmpdirs and clean up with `afterEach` — the Map + * keeps inert entries between tests but each entry is a single Mutex + * that acquires no resources when idle. + */ +const fileLocks = new Map(); + +/** + * Per-file-mutex acquire deadline. A wedged filesystem (NFS hiccup, + * disk I/O stall, locked OneDrive sync target) would otherwise let + * `runExclusive` hold indefinitely — every subsequent `POST + * /workspace/memory` for the same path queues up with no deadline, + * no abort path, and no diagnostic. 30 s is generous for any sane + * filesystem op while bounded enough that a single stalled write + * doesn't silently consume the daemon's request budget. + * + * On timeout `withTimeout` rejects with the sentinel `E_TIMEOUT`, + * which `writeWorkspaceContextFile` catches and rethrows as the + * typed `WorkspaceMemoryWriteTimeoutError` for the route to map to + * a 500 `memory_write_timeout`. + */ +const FILE_LOCK_TIMEOUT_MS = 30_000; + +function getFileLock(filePath: string): MutexInterface { + let lock = fileLocks.get(filePath); + if (!lock) { + lock = withTimeout(new Mutex(), FILE_LOCK_TIMEOUT_MS); + fileLocks.set(filePath, lock); + } + return lock; +} + +/** + * Thrown when the per-file mutex acquire times out. The route maps + * this to a 500 with `code: 'memory_write_timeout'` so SDK callers + * can branch on a stalled-fs / hung-write condition without parsing + * a generic 500. + */ +export class WorkspaceMemoryWriteTimeoutError extends Error { + readonly filePath: string; + readonly timeoutMs: number; + constructor(filePath: string, timeoutMs: number) { + super( + `Workspace memory write at ${filePath} did not acquire the per-file ` + + `lock within ${timeoutMs}ms — another write may be stalled (NFS / ` + + `OneDrive / locked file). Retry or restart the daemon.`, + ); + this.name = 'WorkspaceMemoryWriteTimeoutError'; + this.filePath = filePath; + this.timeoutMs = timeoutMs; + } +} + +export type WriteContextFileScope = 'workspace' | 'global'; +export type WriteContextFileMode = 'append' | 'replace'; + +export interface WriteContextFileOptions { + scope: WriteContextFileScope; + mode: WriteContextFileMode; + /** + * Content to write. For `append`, this is added under the + * `MEMORY_SECTION_HEADER` block. For `replace`, this becomes the + * file's full contents. + */ + content: string; + /** + * Absolute path to the workspace root (used when `scope === 'workspace'`). + * Ignored for `global` writes. + */ + projectRoot: string; +} + +export interface WriteContextFileResult { + filePath: string; + /** + * Bytes actually written by this call. `0` on the no-op short- + * circuit path (`changed: false`). NOT a measurement of the file's + * on-disk size — callers that need that should `fs.stat` the + * returned `filePath` directly. + */ + bytesWritten: number; + /** + * `true` when the call actually mutated the file on disk; `false` + * when the helper short-circuited because the requested write would + * have been a no-op (e.g. `mode: 'append'` with whitespace-only + * content). Callers like the `qwen serve` POST route use this to + * suppress spurious `memory_changed` events that would otherwise + * fan out for a write that didn't change anything. + */ + changed: boolean; +} + +/** + * Append/replace `QWEN.md` for the workspace or the user's global + * `~/.qwen/` directory. Used by the `qwen serve` daemon's + * `POST /workspace/memory` route (issue #4175 PR 16) and any other + * caller that needs to mutate hierarchical memory through code. + * + * Append mode preserves any prose already in the file: when a + * `## Qwen Added Memories` section exists, the new content is + * appended to the end of the file; when it doesn't, a fresh section + * header is added before the content. This matches the shape the + * agent-side `save_memory` tool produces, so files written through + * the daemon route round-trip cleanly with the existing CLI surface. + * + * Replace mode overwrites the whole file with `content` verbatim. + * Callers should canonicalize/validate `content` before passing. + * + * Path safety: `projectRoot` MUST be absolute. Callers are expected + * to pass a daemon-canonicalized workspace path (the bridge's + * `boundWorkspace`); this helper does not re-canonicalize. + */ +export async function writeWorkspaceContextFile( + options: WriteContextFileOptions, +): Promise { + if (!path.isAbsolute(options.projectRoot)) { + throw new Error( + `writeWorkspaceContextFile: projectRoot must be absolute, got "${options.projectRoot}"`, + ); + } + const filePath = resolveContextFilePath(options.scope, options.projectRoot); + + // Hold the per-file mutex for the entire read-compose-write sequence + // INCLUDING the whitespace-only no-op detection. Two concurrent + // POSTs targeting the same file (one whitespace-only, one with real + // content) would otherwise let the no-op `fs.stat` see a stale size + // — the no-op's `changed: false` would still be correct but + // `bytesWritten` could lag the post-write reality. Holding the + // mutex makes the snapshot consistent. `replace` mode also acquires + // the lock so a concurrent `replace` + `append` against the same + // file produces a deterministic last-write rather than a partial + // composite. + try { + return await getFileLock(filePath).runExclusive( + async () => await runWrite(filePath, options), + ); + } catch (err) { + // `withTimeout` rejects with the `E_TIMEOUT` sentinel when the + // mutex acquire deadline elapses — typically a wedged write on + // a stalled FS (NFS hiccup, locked OneDrive sync, kernel I/O + // hang). Translate to a typed error so the route can map to a + // structured 500 instead of a generic catch-all. + if (err === E_TIMEOUT) { + throw new WorkspaceMemoryWriteTimeoutError( + filePath, + FILE_LOCK_TIMEOUT_MS, + ); + } + throw err; + } +} + +async function runWrite( + filePath: string, + options: WriteContextFileOptions, +): Promise { + if (options.mode === 'append' && isWhitespaceOnly(options.content)) { + // No-op short-circuit. Skip the mkdir + writeFile path entirely + // so the parent dir mtime isn't bumped on a request that + // changed nothing — the whitespace-only `\n\n` case from a + // flaky pipeline must not reach the filesystem at all. + // + // `bytesWritten` is `0` (zero bytes were actually written), + // not the existing file's `stat.size`. Earlier revisions returned + // `stat.size` here so the response carried "current file size", + // but that conflated two semantics under one field: clients + // accumulating writes via `sum(bytesWritten)` got the file size + // added in for every whitespace POST. `changed: false` already + // gives clients the no-op signal; the byte count should remain + // true to its field name. + return { filePath, bytesWritten: 0, changed: false }; + } + + await fs.mkdir(path.dirname(filePath), { recursive: true }); + + if (options.mode === 'replace') { + await fs.writeFile(filePath, options.content, { + encoding: 'utf8', + mode: 0o644, + }); + return { + filePath, + bytesWritten: Buffer.byteLength(options.content, 'utf8'), + changed: true, + }; + } + + const next = await composeAppendedContent(filePath, options.content); + await fs.writeFile(filePath, next, { encoding: 'utf8', mode: 0o644 }); + return { + filePath, + bytesWritten: Buffer.byteLength(next, 'utf8'), + changed: true, + }; +} + +function resolveContextFilePath( + scope: WriteContextFileScope, + projectRoot: string, +): string { + // Honor `setGeminiMdFilename()` overrides so POST writes to the same + // file GET surfaces. With the prior `DEFAULT_CONTEXT_FILENAME` hard- + // code, a deployment that switched the context filename to + // `AGENTS.md` would have GET listing the new file while POST kept + // appending to a stale `QWEN.md` — clients then observed "I just + // wrote content but it's missing from /workspace/memory". Mirrors the + // discovery path's `getAllGeminiMdFilenames()` usage in + // `workspaceMemory.ts:collectWorkspaceMemoryStatus`. + const filename = getCurrentGeminiMdFilename(); + if (scope === 'workspace') { + return path.join(projectRoot, filename); + } + return path.join(Storage.getGlobalQwenDir(), filename); +} + +/** + * Cap on the existing-file size we'll read into memory before + * appending. The POST route caps NEW content at 1 MB but a malicious + * or accidental client could grow the file to arbitrary size over + * time (workspace QWEN.md is operator-controlled but the global + * `~/.qwen/QWEN.md` may have been edited externally). 16 MB sits + * three orders of magnitude above any realistic user-authored + * memory file while still bounding the daemon's transient memory + * cost per append. Hitting this cap means QWEN.md has grown past + * any reasonable size and the operator should clean it up — we + * 500 the route with a structured error rather than try to + * stream-process a corrupted file. + */ +const MAX_EXISTING_FILE_BYTES = 16 * 1024 * 1024; + +export class WorkspaceMemoryFileTooLargeError extends Error { + readonly filePath: string; + readonly bytes: number; + readonly limit: number; + constructor(filePath: string, bytes: number, limit: number) { + super( + `Existing memory file at ${filePath} is ${bytes} bytes, exceeds ` + + `the ${limit}-byte cap for safe append. Trim the file or use ` + + `mode=replace to overwrite it.`, + ); + this.name = 'WorkspaceMemoryFileTooLargeError'; + this.filePath = filePath; + this.bytes = bytes; + this.limit = limit; + } +} + +async function composeAppendedContent( + filePath: string, + newContent: string, +): Promise { + let existing = ''; + try { + // `stat` first so we can refuse pathological files BEFORE pulling + // them into memory. Without this check a 200 MB QWEN.md would + // load fully into the daemon's heap on every append, even though + // the route's 1 MB new-content cap caught the request body. + const stat = await fs.stat(filePath); + if (stat.size > MAX_EXISTING_FILE_BYTES) { + throw new WorkspaceMemoryFileTooLargeError( + filePath, + stat.size, + MAX_EXISTING_FILE_BYTES, + ); + } + existing = await fs.readFile(filePath, 'utf8'); + } catch (err) { + if (err instanceof WorkspaceMemoryFileTooLargeError) throw err; + if (!isEnoent(err)) throw err; + } + + const trimmed = trimNewlines(newContent); + if (trimmed.length === 0) return existing; + + if (existing.length === 0) { + return `${MEMORY_SECTION_HEADER}\n${trimmed}\n`; + } + + const sectionIdx = existing.indexOf(MEMORY_SECTION_HEADER); + if (sectionIdx === -1) { + const sep = existing.endsWith('\n') ? '' : '\n'; + return `${existing}${sep}\n${MEMORY_SECTION_HEADER}\n${trimmed}\n`; + } + + // Section header found. Append the new entry INSIDE the section, not + // necessarily at the end of the file. Without this guard, a file + // whose `## Qwen Added Memories` block is followed by another + // `## ...` heading would land each new entry past the next heading + // — silently moving entries into the wrong section. + // + // The naive `indexOf('\n## ')` scan, however, can match `## ` lines + // INSIDE fenced code blocks (` ``` `) — common in user-authored + // QWEN.md memory entries that quote API documentation containing + // markdown headings. Track fence state while scanning and only + // accept matches outside fences. If no real heading is found + // (memory section is the last block), keep the previous behavior + // of appending to EOF. + const afterHeaderIdx = sectionIdx + MEMORY_SECTION_HEADER.length; + const nextHeaderRel = findNextTopLevelHeading(existing, afterHeaderIdx); + if (nextHeaderRel === -1) { + const sep = existing.endsWith('\n') ? '' : '\n'; + return `${existing}${sep}${trimmed}\n`; + } + const insertAt = afterHeaderIdx + nextHeaderRel; + const before = existing.slice(0, insertAt); + const after = existing.slice(insertAt); + const sep = before.endsWith('\n') ? '' : '\n'; + return `${before}${sep}${trimmed}\n${after}`; +} + +/** + * Find the byte offset of the next `\n## ` heading in `text` starting + * at position `start`, skipping any matches that fall inside a fenced + * code block (lines opening with ``` ``` `` `). Returns the offset + * RELATIVE TO `start` (so callers can do `start + result`), or `-1` + * when no real heading is found. + * + * The fence detector is line-based: a line whose first three + * characters are ``` ``` `` ` toggles fence state. Doesn't model + * indented code blocks (4+ leading spaces) — `## ` inside an + * indented code block is rare enough not to justify the parser + * complexity, and a misclassification only causes us to fall back + * to EOF-append, which is the legacy behavior. + */ +function findNextTopLevelHeading(text: string, start: number): number { + let inFence = false; + let lineStart = start; + for (let i = start; i < text.length; i++) { + if (text.charCodeAt(i) !== 0x0a /* \n */) continue; + const nextLineStart = i + 1; + // Toggle fence state if the JUST-FINISHED line opens/closes a + // fence. Strict prefix check — leading whitespace is intentional + // because a 4-space-indented "```" is markdown code-block content, + // not a fence marker. CommonMark allows both ` ``` ` and `~~~` as + // fence delimiters; both must toggle the inside-fence state so a + // `## heading` inside a `~~~` block isn't treated as a section + // boundary. + if ( + text.startsWith('```', lineStart) || + text.startsWith('~~~', lineStart) + ) { + inFence = !inFence; + } + // Heading match runs against the boundary `\n## ` (the four chars + // starting at the newline we just observed). Skip when in fence. + if (!inFence && text.startsWith('## ', nextLineStart)) { + return i - start; // relative offset of the `\n` separator + } + lineStart = nextLineStart; + } + return -1; +} + +function isEnoent(err: unknown): boolean { + return ( + typeof err === 'object' && + err !== null && + (err as { code?: string }).code === 'ENOENT' + ); +} + +/** + * Hand-rolled `^\s+|\s+$` substitute. CodeQL's polynomial-regex + * detector flags `\s+` with anchors as a ReDoS risk on + * attacker-controlled input; the linear loop sidesteps the rule + * without changing behavior. Mirrors the same pattern used by + * `auth.ts:120-125` for header-credential parsing. + */ +function isWhitespaceOnly(s: string): boolean { + for (let i = 0; i < s.length; i++) { + const c = s.charCodeAt(i); + // ASCII space, tab, line feed, carriage return, form feed, + // vertical tab. All non-printable whitespace control chars the + // route's "no-op append" check should treat as empty content. + if ( + c !== 0x20 && + c !== 0x09 && + c !== 0x0a && + c !== 0x0d && + c !== 0x0c && + c !== 0x0b + ) { + return false; + } + } + return true; +} + +/** + * Hand-rolled `^\n+|\n+$` substitute. Same CodeQL rationale as + * `isWhitespaceOnly`. Trims only `\n` so the section-header insert + * path keeps its newline framing semantics — a leading `\t` in + * `newContent` is preserved as part of the user's bullet, while + * `\n\n- entry\n` collapses to `- entry`. + */ +function trimNewlines(s: string): string { + let start = 0; + let end = s.length; + while (start < end && s.charCodeAt(start) === 0x0a) start++; + while (end > start && s.charCodeAt(end - 1) === 0x0a) end--; + return start === 0 && end === s.length ? s : s.slice(start, end); +} diff --git a/packages/core/src/subagents/index.ts b/packages/core/src/subagents/index.ts index 8194798db77..dd3563f75f5 100644 --- a/packages/core/src/subagents/index.ts +++ b/packages/core/src/subagents/index.ts @@ -20,10 +20,13 @@ export type { ValidationResult, ListSubagentsOptions, CreateSubagentOptions, - SubagentErrorCode, } from './types.js'; -export { SubagentError } from './types.js'; +// `SubagentErrorCode` is both a value (the const enum-like object used +// at runtime) and a type. Re-export both shapes so callers like the +// `qwen serve` workspace-agents route can use it as a value without +// reaching into `./types.js` directly. +export { SubagentError, SubagentErrorCode } from './types.js'; // Built-in agents registry export { diff --git a/packages/sdk-typescript/src/daemon/DaemonClient.ts b/packages/sdk-typescript/src/daemon/DaemonClient.ts index 5c4271c7b3f..68c5ac4c75e 100644 --- a/packages/sdk-typescript/src/daemon/DaemonClient.ts +++ b/packages/sdk-typescript/src/daemon/DaemonClient.ts @@ -6,18 +6,26 @@ import { parseSseStream } from './sse.js'; import type { + DaemonAgentMutationResult, DaemonCapabilities, + DaemonCreateAgentRequest, DaemonEvent, DaemonSessionContextStatus, DaemonRestoredSession, DaemonSession, DaemonSessionSummary, DaemonSessionSupportedCommandsStatus, + DaemonUpdateAgentRequest, + DaemonWorkspaceAgentDetail, + DaemonWorkspaceAgentsStatus, DaemonWorkspaceEnvStatus, DaemonWorkspaceMcpStatus, + DaemonWorkspaceMemoryStatus, DaemonWorkspacePreflightStatus, DaemonWorkspaceProvidersStatus, DaemonWorkspaceSkillsStatus, + DaemonWriteMemoryRequest, + DaemonWriteMemoryResult, HeartbeatResult, PermissionResponse, PromptContentBlock, @@ -342,6 +350,211 @@ export class DaemonClient { ); } + // -- Workspace memory (issue #4175 PR 16) ------------------------------ + + /** + * Fetch the daemon's `QWEN.md` / `AGENTS.md` snapshot. Read-only; + * pre-flight `caps.features.workspace_memory` before calling + * against an unknown daemon. Returns `initialized: false` and an + * empty `files` array when no memory files exist at the bound + * workspace root or `~/.qwen`. + * + * v1 discovers files at the bound workspace ROOT only, plus the + * user's global `~/.qwen` directory — it does NOT walk parent + * directories or recurse into the workspace tree. The route's + * companion helper `walkWorkspaceForMemory` keeps a guarded + * upward-walk loop body for a future hierarchical mode but breaks + * after iteration 1 in this release. PR 16.5 will lift the cap + * once auto-memory CRUD lands. + */ + async workspaceMemory(): Promise { + return await this.fetchWithTimeout( + `${this.baseUrl}/workspace/memory`, + { headers: this.headers() }, + async (res) => { + if (!res.ok) { + throw await this.failOnError(res, 'GET /workspace/memory'); + } + return (await res.json()) as DaemonWorkspaceMemoryStatus; + }, + ); + } + + /** + * Append to or replace `QWEN.md` at workspace or global scope. + * Strict mutation gate (`token_required` on no-token loopback + * defaults). When the daemon advertises `workspace_memory`, expect + * 200 with `{ ok, filePath, bytesWritten, mode }`; older daemons + * without the capability return 404. + */ + async writeWorkspaceMemory( + req: DaemonWriteMemoryRequest, + clientId?: string, + ): Promise { + return await this.fetchWithTimeout( + `${this.baseUrl}/workspace/memory`, + { + method: 'POST', + headers: this.headers({ 'Content-Type': 'application/json' }, clientId), + body: JSON.stringify(req), + }, + async (res) => { + if (!res.ok) { + throw await this.failOnError(res, 'POST /workspace/memory'); + } + return (await res.json()) as DaemonWriteMemoryResult; + }, + ); + } + + // -- Workspace agents (issue #4175 PR 16) ------------------------------ + + async listWorkspaceAgents(): Promise { + return await this.fetchWithTimeout( + `${this.baseUrl}/workspace/agents`, + { headers: this.headers() }, + async (res) => { + if (!res.ok) { + throw await this.failOnError(res, 'GET /workspace/agents'); + } + return (await res.json()) as DaemonWorkspaceAgentsStatus; + }, + ); + } + + /** + * Create a project- or user-level subagent. 409 `agent_already_exists` + * when a same-name agent is already registered at the chosen level; + * 422 `invalid_config` for validation failures. + */ + async createWorkspaceAgent( + req: DaemonCreateAgentRequest, + clientId?: string, + ): Promise { + return await this.fetchWithTimeout( + `${this.baseUrl}/workspace/agents`, + { + method: 'POST', + headers: this.headers({ 'Content-Type': 'application/json' }, clientId), + body: JSON.stringify(req), + }, + async (res) => { + if (!res.ok) { + throw await this.failOnError(res, 'POST /workspace/agents'); + } + return (await res.json()) as DaemonAgentMutationResult; + }, + ); + } + + async getWorkspaceAgent( + agentType: string, + ): Promise { + return await this.fetchWithTimeout( + `${this.baseUrl}/workspace/agents/${encodeURIComponent(agentType)}`, + { headers: this.headers() }, + async (res) => { + if (!res.ok) { + throw await this.failOnError(res, 'GET /workspace/agents/:agentType'); + } + return (await res.json()) as DaemonWorkspaceAgentDetail; + }, + ); + } + + /** + * Update a project- or user-level subagent definition. Built-in / + * extension / session-level agents are read-only and return 403 + * `agent_readonly`; missing agents return 404 `agent_not_found`. + * + * Optional `scope` mirrors the delete helper: when a project agent + * shadows a user-level agent of the same name, pass + * `{ scope: 'global' }` to update the user-level definition + * specifically. Without the scope the daemon resolves through the + * default precedence (project > user) and updates the project entry. + */ + async updateWorkspaceAgent( + agentType: string, + req: DaemonUpdateAgentRequest, + opts: { scope?: 'workspace' | 'global' } = {}, + clientId?: string, + ): Promise { + const url = opts.scope + ? `${this.baseUrl}/workspace/agents/${encodeURIComponent(agentType)}?scope=${encodeURIComponent(opts.scope)}` + : `${this.baseUrl}/workspace/agents/${encodeURIComponent(agentType)}`; + return await this.fetchWithTimeout( + url, + { + method: 'POST', + headers: this.headers({ 'Content-Type': 'application/json' }, clientId), + body: JSON.stringify(req), + }, + async (res) => { + if (!res.ok) { + throw await this.failOnError( + res, + 'POST /workspace/agents/:agentType', + ); + } + return (await res.json()) as DaemonAgentMutationResult; + }, + ); + } + + /** + * Delete a project- or user-level subagent definition. Optional + * `scope` query narrows deletion to one level when the same name + * exists at both. Idempotent for SDK callers — both 204 (deleted) + * and 404 (already gone) resolve successfully. + */ + async deleteWorkspaceAgent( + agentType: string, + opts: { scope?: 'workspace' | 'global' } = {}, + clientId?: string, + ): Promise { + const url = opts.scope + ? `${this.baseUrl}/workspace/agents/${encodeURIComponent(agentType)}?scope=${encodeURIComponent(opts.scope)}` + : `${this.baseUrl}/workspace/agents/${encodeURIComponent(agentType)}`; + return await this.fetchWithTimeout( + url, + { + method: 'DELETE', + headers: this.headers({}, clientId), + }, + async (res) => { + if (res.status === 204) { + try { + await res.body?.cancel(); + } catch { + /* body already consumed or no body */ + } + return; + } + // Treat as idempotent ONLY when the daemon explicitly says + // `agent_not_found`. A bare 404 (e.g. an HTTP proxy returning + // a generic page, an older daemon that doesn't know the + // route, a misrouted load balancer) would otherwise be + // silently swallowed and the SDK caller would believe the + // agent was deleted when the request never reached a route + // that understands workspace agents. Failing on non- + // structured 404s makes routing errors visible. + if (res.status === 404) { + const err = await this.failOnError( + res, + 'DELETE /workspace/agents/:agentType', + ); + const body = err.body as { code?: unknown } | undefined; + if (body && body.code === 'agent_not_found') return; + throw err; + } + throw await this.failOnError( + res, + 'DELETE /workspace/agents/:agentType', + ); + }, + ); + } + async workspaceEnv(): Promise { return await this.fetchWithTimeout( `${this.baseUrl}/workspace/env`, diff --git a/packages/sdk-typescript/src/daemon/events.ts b/packages/sdk-typescript/src/daemon/events.ts index b81bf064d46..aa7e0266ce2 100644 --- a/packages/sdk-typescript/src/daemon/events.ts +++ b/packages/sdk-typescript/src/daemon/events.ts @@ -19,6 +19,12 @@ const DAEMON_KNOWN_EVENT_TYPE_VALUES = [ 'client_evicted', 'slow_client_warning', 'stream_error', + // Issue #4175 PR 16: workspace-level mutation signals fanned out + // through every active session's bus. Non-terminal — informational + // for adapters that want to render "memory just changed" / "agent X + // updated" toasts. Read-after-write remains the correctness contract. + 'memory_changed', + 'agent_changed', ] as const; const DAEMON_KNOWN_EVENT_TYPES: ReadonlySet = new Set( @@ -125,6 +131,33 @@ export interface DaemonStreamErrorData { [key: string]: unknown; } +/** + * Issue #4175 PR 16: a `POST /workspace/memory` write completed + * successfully. `scope` records which file was touched (workspace QWEN.md + * vs global ~/.qwen/QWEN.md), `mode` is the requested write mode, and + * `bytesWritten` is the size of the file post-write. + */ +export interface DaemonMemoryChangedData { + scope: 'workspace' | 'global'; + filePath: string; + mode: 'append' | 'replace'; + bytesWritten: number; + [key: string]: unknown; +} + +/** + * Issue #4175 PR 16: a workspace agent CRUD mutation completed + * successfully. `change` discriminates the operation; `level` records + * whether the project- or user-level definition was touched. Built-in + * and extension agents are read-only and never appear here. + */ +export interface DaemonAgentChangedData { + change: 'created' | 'updated' | 'deleted'; + name: string; + level: 'project' | 'user'; + [key: string]: unknown; +} + export type DaemonSessionUpdateEvent = DaemonEventEnvelope< 'session_update', DaemonSessionUpdateData @@ -173,6 +206,14 @@ export type DaemonStreamErrorEvent = DaemonEventEnvelope< 'stream_error', DaemonStreamErrorData >; +export type DaemonMemoryChangedEvent = DaemonEventEnvelope< + 'memory_changed', + DaemonMemoryChangedData +>; +export type DaemonAgentChangedEvent = DaemonEventEnvelope< + 'agent_changed', + DaemonAgentChangedData +>; export type DaemonSessionEvent = | DaemonSessionUpdateEvent @@ -192,10 +233,20 @@ export type DaemonStreamLifecycleEvent = | DaemonSlowClientWarningEvent | DaemonStreamErrorEvent; +/** + * Issue #4175 PR 16: workspace-level mutation signals fanned out + * through every active session's bus. Non-terminal; clients use them + * to refresh cached views of workspace memory / agents. + */ +export type DaemonWorkspaceMutationEvent = + | DaemonMemoryChangedEvent + | DaemonAgentChangedEvent; + export type KnownDaemonEvent = | DaemonSessionEvent | DaemonControlEvent - | DaemonStreamLifecycleEvent; + | DaemonStreamLifecycleEvent + | DaemonWorkspaceMutationEvent; export interface DaemonSessionViewState { lastEventId?: number; @@ -231,6 +282,16 @@ export interface DaemonSessionViewState { */ slowClientWarningCount: number; lastSlowClientWarning?: DaemonSlowClientWarningData; + /** + * Issue #4175 PR 16: most recent workspace mutation observed on this + * stream (memory or agent change). Non-terminal — adapters render a + * "memory just changed" / "agent X updated" toast and re-fetch the + * relevant workspace status route. Captures only the latest event; + * older events are not retained because the route's read-after-write + * contract makes the event a hint, not the source of truth. + */ + lastWorkspaceMutation?: DaemonMemoryChangedData | DaemonAgentChangedData; + lastWorkspaceMutationType?: 'memory_changed' | 'agent_changed'; } export function createDaemonSessionViewState( @@ -257,6 +318,8 @@ export function createDaemonSessionViewState( seed.lastUnmatchedPermissionResolutionId, slowClientWarningCount: seed.slowClientWarningCount ?? 0, lastSlowClientWarning: seed.lastSlowClientWarning, + lastWorkspaceMutation: seed.lastWorkspaceMutation, + lastWorkspaceMutationType: seed.lastWorkspaceMutationType, }; } @@ -326,6 +389,14 @@ export function asKnownDaemonEvent( return isStreamErrorData(event.data) ? (event as DaemonStreamErrorEvent) : undefined; + case 'memory_changed': + return isMemoryChangedData(event.data) + ? (event as DaemonMemoryChangedEvent) + : undefined; + case 'agent_changed': + return isAgentChangedData(event.data) + ? (event as DaemonAgentChangedEvent) + : undefined; default: return undefined; } @@ -462,6 +533,24 @@ export function reduceDaemonSessionEvent( streamError: event.data, pendingPermissions: {}, }; + case 'memory_changed': + // Non-terminal: adapters render a "memory just changed" hint and + // re-fetch `GET /workspace/memory` to get the canonical state. We + // don't append to a list — the latest event is enough since the + // route's read-after-write contract is the source of truth. + return { + ...base, + lastWorkspaceMutation: event.data, + lastWorkspaceMutationType: 'memory_changed', + }; + case 'agent_changed': + // Same shape as `memory_changed` — non-terminal hint that + // triggers a `GET /workspace/agents` re-fetch. + return { + ...base, + lastWorkspaceMutation: event.data, + lastWorkspaceMutationType: 'agent_changed', + }; default: { const _exhaustive: never = event; return _exhaustive; @@ -619,6 +708,29 @@ function isStreamErrorData(value: unknown): value is DaemonStreamErrorData { return isRecord(value) && isNonEmptyString(value['error']); } +function isMemoryChangedData(value: unknown): value is DaemonMemoryChangedData { + if (!isRecord(value)) return false; + const scope = value['scope']; + const mode = value['mode']; + return ( + (scope === 'workspace' || scope === 'global') && + isNonEmptyString(value['filePath']) && + (mode === 'append' || mode === 'replace') && + isFiniteNumber(value['bytesWritten']) + ); +} + +function isAgentChangedData(value: unknown): value is DaemonAgentChangedData { + if (!isRecord(value)) return false; + const change = value['change']; + const level = value['level']; + return ( + (change === 'created' || change === 'updated' || change === 'deleted') && + isNonEmptyString(value['name']) && + (level === 'project' || level === 'user') + ); +} + function isPermissionOption(value: unknown): value is DaemonPermissionOption { return isRecord(value) && isNonEmptyString(value['optionId']); } diff --git a/packages/sdk-typescript/src/daemon/index.ts b/packages/sdk-typescript/src/daemon/index.ts index a136100d988..d2ed7ccfb40 100644 --- a/packages/sdk-typescript/src/daemon/index.ts +++ b/packages/sdk-typescript/src/daemon/index.ts @@ -33,11 +33,15 @@ export { requireWorkspaceCwd, } from './types.js'; export type { + DaemonAgentChangedData, + DaemonAgentChangedEvent, DaemonClientEvictedData, DaemonClientEvictedEvent, DaemonControlEvent, DaemonEventEnvelope, DaemonKnownEventType, + DaemonMemoryChangedData, + DaemonMemoryChangedEvent, DaemonModelSwitchedData, DaemonModelSwitchedEvent, DaemonModelSwitchFailedData, @@ -65,11 +69,16 @@ export type { DaemonStreamErrorData, DaemonStreamErrorEvent, DaemonStreamLifecycleEvent, + DaemonWorkspaceMutationEvent, KnownDaemonEvent, } from './events.js'; export type { + DaemonAgentLevel, + DaemonAgentMutationResult, DaemonAvailableCommand, DaemonCapabilities, + DaemonContextFileScope, + DaemonCreateAgentRequest, DaemonEnvCell, DaemonEnvKind, DaemonErrorKind, @@ -90,9 +99,15 @@ export type { DaemonPreflightKind, DaemonStatus, DaemonStatusCell, + DaemonUpdateAgentRequest, + DaemonWorkspaceAgentDetail, + DaemonWorkspaceAgentSummary, + DaemonWorkspaceAgentsStatus, DaemonWorkspaceEnvStatus, DaemonWorkspaceMcpServerStatus, DaemonWorkspaceMcpStatus, + DaemonWorkspaceMemoryFile, + DaemonWorkspaceMemoryStatus, DaemonWorkspacePreflightStatus, DaemonWorkspaceProviderCurrent, DaemonWorkspaceProviderModel, @@ -100,6 +115,8 @@ export type { DaemonWorkspaceProvidersStatus, DaemonWorkspaceSkillStatus, DaemonWorkspaceSkillsStatus, + DaemonWriteMemoryRequest, + DaemonWriteMemoryResult, HeartbeatResult, PermissionOutcome, PermissionOutcomeCancelled, diff --git a/packages/sdk-typescript/src/daemon/types.ts b/packages/sdk-typescript/src/daemon/types.ts index ecf23e1d9fe..c6c4110ff0e 100644 --- a/packages/sdk-typescript/src/daemon/types.ts +++ b/packages/sdk-typescript/src/daemon/types.ts @@ -347,6 +347,187 @@ export interface DaemonWorkspaceProvidersStatus { errors?: DaemonStatusCell[]; } +/** + * Issue #4175 PR 16: workspace memory snapshot returned from + * `GET /workspace/memory`. Mirrors the `kind / status / error?` cell + * pattern used by mcp/skills/providers — adapters can render any of + * the four with the same component. + */ +export type DaemonContextFileScope = 'workspace' | 'global'; + +export interface DaemonWorkspaceMemoryFile { + kind: 'memory_file'; + path: string; + scope: DaemonContextFileScope; + bytes: number; +} + +export interface DaemonWorkspaceMemoryStatus { + v: 1; + workspaceCwd: string; + initialized: boolean; + files: DaemonWorkspaceMemoryFile[]; + totalBytes: number; + fileCount: number; + ruleCount: number; + errors?: DaemonStatusCell[]; +} + +/** + * Body of `POST /workspace/memory`. `mode` defaults to `'append'` + * server-side when omitted; clients SHOULD send it explicitly so a + * future server-side default flip doesn't silently change semantics. + */ +export interface DaemonWriteMemoryRequest { + scope: DaemonContextFileScope; + content: string; + mode?: 'append' | 'replace'; +} + +export interface DaemonWriteMemoryResult { + ok: true; + filePath: string; + /** + * Bytes actually written by THIS request. `0` when the daemon + * short-circuited the write (`changed: false`) — e.g. whitespace- + * only append. NOT the on-disk file size; callers needing that + * should issue a `GET /workspace/memory` for the file's current + * `bytes`. + */ + bytesWritten: number; + mode: 'append' | 'replace'; + /** + * `true` when the daemon actually mutated the file on disk. `false` + * for whitespace-only `append` requests that short-circuited + * upstream — the route accepted the request as well-formed (200 + * OK) but the helper detected the trimmed content was empty and + * skipped the write to avoid an mtime bump + a misleading + * `memory_changed` event. SDK consumers can branch on this to + * suppress redundant cache invalidation. Optional at the type + * level for forward-compat with daemons that predate the field — + * those return undefined and callers should treat that as + * `changed: true` (the legacy contract). + */ + changed?: boolean; +} + +/** + * Issue #4175 PR 16: subagent CRUD types. `agentType` on the wire is + * the `name` field from the agent's frontmatter (case-insensitive); + * `level` distinguishes project-/user-/builtin-/extension-level + * registrations. Built-in / extension agents are read-only — POST and + * DELETE return 403 `agent_readonly`. + */ +/** + * Storage level for a subagent definition. + * + * `project` / `user` / `builtin` are the levels the `qwen serve` + * daemon currently surfaces through `GET /workspace/agents` and the + * per-`agentType` detail route. + * + * `extension` and `session` are present on the union for forward- + * compat but the daemon does NOT return them today — the daemon- + * scoped `SubagentManager` is constructed against a stub `Config` + * whose `getActiveExtensions()` returns `[]` (extension plumbing has + * no entry point through the workspace daemon yet) and session-level + * subagents live in a runtime-only cache no CRUD route reads. SDK + * consumers writing exhaustive switches over `DaemonAgentLevel` + * should therefore include arms for both values but treat them as + * unreachable on today's route surface — having them on the type + * avoids a breaking SDK change when a future PR exposes either + * source. + */ +export type DaemonAgentLevel = + | 'project' + | 'user' + | 'builtin' + | 'extension' + | 'session'; + +export interface DaemonWorkspaceAgentSummary { + kind: 'agent'; + name: string; + description: string; + level: DaemonAgentLevel; + isBuiltin: boolean; + hasTools: boolean; + model?: string; + color?: string; + background?: boolean; + approvalMode?: string; + extensionName?: string; + filePath?: string; +} + +export interface DaemonWorkspaceAgentDetail + extends DaemonWorkspaceAgentSummary { + systemPrompt: string; + tools?: string[]; + disallowedTools?: string[]; + runConfig?: { max_time_minutes?: number; max_turns?: number }; +} + +export interface DaemonWorkspaceAgentsStatus { + v: 1; + workspaceCwd: string; + agents: DaemonWorkspaceAgentSummary[]; + errors?: DaemonStatusCell[]; +} + +/** + * Body of `POST /workspace/agents`. The daemon translates `scope` into + * the corresponding `SubagentLevel` (`workspace`→`project`, + * `global`→`user`). + */ +export interface DaemonCreateAgentRequest { + name: string; + description: string; + systemPrompt: string; + scope: 'workspace' | 'global'; + tools?: string[]; + disallowedTools?: string[]; + model?: string; + runConfig?: { max_time_minutes?: number; max_turns?: number }; + color?: string; + approvalMode?: string; + background?: boolean; +} + +/** + * Body of `POST /workspace/agents/:agentType`. `name` / `level` / + * `filePath` / `isBuiltin` are intentionally omitted — agent type + * comes from the URL, level is determined by the existing record, and + * the other two are server-managed. + */ +export interface DaemonUpdateAgentRequest { + description?: string; + systemPrompt?: string; + tools?: string[]; + disallowedTools?: string[]; + model?: string; + runConfig?: { max_time_minutes?: number; max_turns?: number }; + color?: string; + approvalMode?: string; + background?: boolean; +} + +export interface DaemonAgentMutationResult { + ok: true; + agent: DaemonWorkspaceAgentDetail; + /** + * `true` when the daemon actually rewrote the agent definition; + * `false` when the request was a no-op (every supplied field + * already matched the existing record). The update route emits + * the field on every response (introduced alongside the no-op + * short-circuit in PR 16); create responses currently omit it + * because every successful create is a write — typed consumers + * should treat `undefined` as `true` (the legacy contract). This + * mirrors `DaemonWriteMemoryResult.changed`. Optional at the type + * level for forward-compat with daemons that predate the field. + */ + changed?: boolean; +} + export type DaemonEnvKind = | 'runtime' | 'platform' diff --git a/packages/sdk-typescript/test/unit/DaemonClient.test.ts b/packages/sdk-typescript/test/unit/DaemonClient.test.ts index 0341c312e7a..c18a88253ad 100644 --- a/packages/sdk-typescript/test/unit/DaemonClient.test.ts +++ b/packages/sdk-typescript/test/unit/DaemonClient.test.ts @@ -1336,4 +1336,244 @@ describe('DaemonClient', () => { ); }); }); + + describe('workspace memory + agents helpers (issue #4175 PR 16)', () => { + it('GETs /workspace/memory and parses the snapshot', async () => { + const snapshot = { + v: 1, + workspaceCwd: '/work/a', + initialized: true, + files: [ + { + kind: 'memory_file' as const, + path: '/work/a/QWEN.md', + scope: 'workspace' as const, + bytes: 42, + }, + ], + totalBytes: 42, + fileCount: 1, + ruleCount: 0, + }; + const { fetch, calls } = recordingFetch(() => + jsonResponse(200, snapshot), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + await expect(client.workspaceMemory()).resolves.toEqual(snapshot); + expect(calls[0]).toMatchObject({ + method: 'GET', + url: 'http://daemon/workspace/memory', + }); + }); + + it('POSTs /workspace/memory and forwards X-Qwen-Client-Id', async () => { + const reply = { + ok: true, + filePath: '/work/QWEN.md', + bytesWritten: 17, + mode: 'append', + changed: true, + }; + const { fetch, calls } = recordingFetch(() => jsonResponse(200, reply)); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + const result = await client.writeWorkspaceMemory( + { scope: 'workspace', mode: 'append', content: '- entry' }, + 'client-7', + ); + expect(result).toEqual(reply); + expect(calls[0]?.method).toBe('POST'); + expect(calls[0]?.url).toBe('http://daemon/workspace/memory'); + expect(calls[0]?.headers['x-qwen-client-id']).toBe('client-7'); + const body = JSON.parse(calls[0]!.body!); + expect(body).toEqual({ + scope: 'workspace', + mode: 'append', + content: '- entry', + }); + }); + + it('throws DaemonHttpError on non-2xx workspace memory writes', async () => { + const { fetch } = recordingFetch(() => + jsonResponse(401, { error: 'token required', code: 'token_required' }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + await expect( + client.writeWorkspaceMemory({ + scope: 'workspace', + mode: 'append', + content: 'x', + }), + ).rejects.toBeInstanceOf(DaemonHttpError); + }); + + it('GETs /workspace/agents (list) and /workspace/agents/:id (detail)', async () => { + const list = { + v: 1, + workspaceCwd: '/work/a', + agents: [ + { + kind: 'agent' as const, + name: 'reviewer', + description: 'reviews code', + level: 'project' as const, + isBuiltin: false, + hasTools: false, + }, + ], + }; + const detail = { + kind: 'agent' as const, + name: 'reviewer', + description: 'reviews code', + level: 'project' as const, + isBuiltin: false, + hasTools: false, + systemPrompt: 'you are a reviewer', + }; + const { fetch, calls } = recordingFetch((req) => { + if (req.url.endsWith('/workspace/agents')) + return jsonResponse(200, list); + if (req.url.endsWith('/workspace/agents/reviewer')) { + return jsonResponse(200, detail); + } + return jsonResponse(500, { error: `unexpected ${req.url}` }); + }); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + await expect(client.listWorkspaceAgents()).resolves.toEqual(list); + await expect(client.getWorkspaceAgent('reviewer')).resolves.toEqual( + detail, + ); + expect(calls.map((c) => [c.method, c.url])).toEqual([ + ['GET', 'http://daemon/workspace/agents'], + ['GET', 'http://daemon/workspace/agents/reviewer'], + ]); + }); + + it('encodes the agentType path segment', async () => { + const { fetch, calls } = recordingFetch(() => + jsonResponse(404, { error: 'not found', code: 'agent_not_found' }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + await expect( + client.getWorkspaceAgent('with/slash'), + ).rejects.toBeInstanceOf(DaemonHttpError); + expect(calls[0]?.url).toBe('http://daemon/workspace/agents/with%2Fslash'); + }); + + it('createWorkspaceAgent POSTs the body with the client id', async () => { + const reply = { + ok: true, + agent: { + kind: 'agent' as const, + name: 'tester', + description: 'tests', + level: 'project' as const, + isBuiltin: false, + hasTools: false, + systemPrompt: 'run tests', + }, + }; + const { fetch, calls } = recordingFetch(() => jsonResponse(201, reply)); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + const out = await client.createWorkspaceAgent( + { + name: 'tester', + description: 'tests', + systemPrompt: 'run tests', + scope: 'workspace', + }, + 'client-1', + ); + expect(out).toEqual(reply); + expect(calls[0]?.method).toBe('POST'); + expect(calls[0]?.headers['x-qwen-client-id']).toBe('client-1'); + }); + + it('updateWorkspaceAgent forwards the optional scope query', async () => { + const { fetch, calls } = recordingFetch(() => + jsonResponse(200, { + ok: true, + agent: { + kind: 'agent', + name: 'x', + description: 'd', + level: 'user', + isBuiltin: false, + hasTools: false, + systemPrompt: 'p', + }, + }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + await client.updateWorkspaceAgent( + 'x', + { description: 'd' }, + { scope: 'global' }, + ); + expect(calls[0]?.url).toBe( + 'http://daemon/workspace/agents/x?scope=global', + ); + }); + + it('updateWorkspaceAgent surfaces the daemon `changed` flag on the typed result', async () => { + // The route emits `changed: false` on no-op updates so adapters + // can suppress redundant cache invalidation. The SDK type + // exposes the field as optional so typed callers can branch. + const { fetch } = recordingFetch(() => + jsonResponse(200, { + ok: true, + agent: { + kind: 'agent', + name: 'x', + description: 'd', + level: 'project', + isBuiltin: false, + hasTools: false, + systemPrompt: 'p', + }, + changed: false, + }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + const result = await client.updateWorkspaceAgent('x', { + description: 'd', + }); + expect(result.changed).toBe(false); + expect(result.ok).toBe(true); + expect(result.agent.name).toBe('x'); + }); + + it('deleteWorkspaceAgent treats 204 as success and only swallows structured 404', async () => { + // 204 → resolves silently + { + const { fetch } = recordingFetch( + () => new Response(null, { status: 204 }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + await expect(client.deleteWorkspaceAgent('x')).resolves.toBeUndefined(); + } + // 404 with `code: agent_not_found` → idempotent success + { + const { fetch } = recordingFetch(() => + jsonResponse(404, { error: 'not found', code: 'agent_not_found' }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + await expect(client.deleteWorkspaceAgent('x')).resolves.toBeUndefined(); + } + // 404 WITHOUT structured code (proxy / older daemon / wrong route) → throws + { + const { fetch } = recordingFetch( + () => + new Response('Not Found', { + status: 404, + headers: { 'content-type': 'text/plain' }, + }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + await expect(client.deleteWorkspaceAgent('x')).rejects.toBeInstanceOf( + DaemonHttpError, + ); + } + }); + }); }); diff --git a/packages/sdk-typescript/test/unit/daemonEvents.test.ts b/packages/sdk-typescript/test/unit/daemonEvents.test.ts index e58aa6ff745..a3c3287533d 100644 --- a/packages/sdk-typescript/test/unit/daemonEvents.test.ts +++ b/packages/sdk-typescript/test/unit/daemonEvents.test.ts @@ -744,4 +744,131 @@ describe('daemon event schema', () => { // id observed (the original session_update at id=1). expect(state.lastEventId).toBe(1); }); + + it('narrows memory_changed events and rejects malformed payloads', () => { + const valid: DaemonEvent = { + id: 7, + v: 1, + type: 'memory_changed', + data: { + scope: 'workspace', + filePath: '/work/QWEN.md', + mode: 'append', + bytesWritten: 42, + }, + originatorClientId: 'client-mem', + }; + const known = asKnownDaemonEvent(valid); + expect(known?.type).toBe('memory_changed'); + expect(isDaemonEventType(valid, 'memory_changed')).toBe(true); + + // Malformed: scope outside the union → not narrowable. + const bad: DaemonEvent = { + id: 8, + v: 1, + type: 'memory_changed', + data: { + scope: 'remote', + filePath: '/work/QWEN.md', + mode: 'append', + bytesWritten: 1, + }, + }; + expect(asKnownDaemonEvent(bad)).toBeUndefined(); + + // Missing required field (bytesWritten). + const missing: DaemonEvent = { + id: 9, + v: 1, + type: 'memory_changed', + data: { + scope: 'workspace', + filePath: '/work/QWEN.md', + mode: 'append', + }, + }; + expect(asKnownDaemonEvent(missing)).toBeUndefined(); + }); + + it('narrows agent_changed events and rejects malformed payloads', () => { + const valid: DaemonEvent = { + id: 10, + v: 1, + type: 'agent_changed', + data: { change: 'created', name: 'reviewer', level: 'project' }, + }; + expect(asKnownDaemonEvent(valid)?.type).toBe('agent_changed'); + + // change outside union. + const bad: DaemonEvent = { + id: 11, + v: 1, + type: 'agent_changed', + data: { change: 'mutated', name: 'x', level: 'project' }, + }; + expect(asKnownDaemonEvent(bad)).toBeUndefined(); + + // level outside union. + const badLevel: DaemonEvent = { + id: 12, + v: 1, + type: 'agent_changed', + data: { change: 'created', name: 'x', level: 'builtin' }, + }; + expect(asKnownDaemonEvent(badLevel)).toBeUndefined(); + }); + + it('reduces memory_changed and agent_changed into lastWorkspaceMutation', () => { + const state = reduceDaemonSessionEvents([ + { + id: 1, + v: 1, + type: 'memory_changed', + data: { + scope: 'workspace', + filePath: '/work/QWEN.md', + mode: 'append', + bytesWritten: 12, + }, + }, + { + id: 2, + v: 1, + type: 'agent_changed', + data: { change: 'updated', name: 'reviewer', level: 'project' }, + }, + ]); + // Latest event wins; type discriminator follows. + expect(state.lastWorkspaceMutationType).toBe('agent_changed'); + expect(state.lastWorkspaceMutation).toEqual({ + change: 'updated', + name: 'reviewer', + level: 'project', + }); + // Both events are non-terminal. + expect(state.alive).toBe(true); + expect(state.terminalEvent).toBeUndefined(); + expect(state.lastEventId).toBe(2); + }); + + it('preserves memory_changed snapshot when no agent_changed follows', () => { + const state = reduceDaemonSessionEvent(createDaemonSessionViewState(), { + id: 5, + v: 1, + type: 'memory_changed', + data: { + scope: 'global', + filePath: '/home/.qwen/QWEN.md', + mode: 'replace', + bytesWritten: 100, + }, + }); + expect(state.lastWorkspaceMutationType).toBe('memory_changed'); + expect(state.lastWorkspaceMutation).toEqual({ + scope: 'global', + filePath: '/home/.qwen/QWEN.md', + mode: 'replace', + bytesWritten: 100, + }); + }); });