refactor(serve): extract DaemonWorkspaceService from AcpSessionBridge (issue #4542, 方案 C)#4563
Conversation
📋 Review SummaryThis PR extracts workspace-scoped capabilities from 🔍 General Feedback
🎯 Specific Feedback🟡 High
🟢 Medium
🔵 Low
✅ Highlights
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the qwen serve daemon layering by renaming the bridge (HttpAcpBridge → AcpSessionBridge) and introducing a new DaemonWorkspaceService facade intended to own workspace-scoped operations (status/env/preflight/tool toggle/init/MCP restart + sub-services for file/auth/agents/memory), while keeping the HTTP REST surface unchanged.
Changes:
- Introduces
packages/cli/src/serve/workspace-service/(types + facade + sub-services) and rewires workspace status/mutation REST routes inserver.tsto call the workspace service. - Renames and re-exports bridge types/factory (
AcpSessionBridge/createAcpSessionBridge), and updates tests/imports accordingly across CLI and@qwen-code/acp-bridge. - Updates Vitest aliases to ensure monorepo tests resolve
@qwen-code/acp-bridge/*subpath exports to source.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/cli/vitest.config.ts | Adds aliases so CLI tests resolve @qwen-code/acp-bridge/* subpaths to live source files. |
| packages/cli/src/serve/workspaceMemory.ts | Updates bridge type import to AcpSessionBridge. |
| packages/cli/src/serve/workspaceMemory.test.ts | Updates test stubs/types to AcpSessionBridge. |
| packages/cli/src/serve/workspaceAgents.ts | Updates bridge import and comment to acpSessionBridge. |
| packages/cli/src/serve/workspaceAgents.test.ts | Updates test stubs/types to AcpSessionBridge. |
| packages/cli/src/serve/workspace-service/types.ts | Adds workspace-service public types: request context, sub-service interfaces, facade deps. |
| packages/cli/src/serve/workspace-service/index.ts | Implements createDaemonWorkspaceService facade and workspace status/mutation methods. |
| packages/cli/src/serve/workspace-service/fileService.ts | Adds FileService wrapper over WorkspaceFileSystemFactory. |
| packages/cli/src/serve/workspace-service/authService.ts | Adds AuthService wrapper over DeviceFlowRegistry. |
| packages/cli/src/serve/workspace-service/agentsService.ts | Adds AgentsService wrapper over SubagentManager + event fan-out. |
| packages/cli/src/serve/workspace-service/memoryService.ts | Adds MemoryService for QWEN.md/AGENTS.md discovery + write/delete fan-out. |
| packages/cli/src/serve/workspace-service/tests/fileService.test.ts | Unit tests for FileService context mapping and delegation. |
| packages/cli/src/serve/workspace-service/tests/authService.test.ts | Unit tests for AuthService delegation and clientId threading. |
| packages/cli/src/serve/workspace-service/tests/agentsService.test.ts | Unit tests for AgentsService CRUD behavior and event emission. |
| packages/cli/src/serve/workspace-service/tests/memoryService.test.ts | Unit tests for MemoryService discovery/mutation + event emission. |
| packages/cli/src/serve/workspace-service/tests/facade.test.ts | Unit tests for facade wiring + status/mutation behavior (incl. init workspace). |
| packages/cli/src/serve/workspace-service/tests/integration.test.ts | Integration tests verifying Express routes delegate into injected workspace service. |
| packages/cli/src/serve/server.ts | Rewires workspace status/init/tool/MCP-restart routes to call DaemonWorkspaceService; adds workspace ctx helper. |
| packages/cli/src/serve/server.test.ts | Updates server tests for new workspace-service behavior and wiring. |
| packages/cli/src/serve/runQwenServe.ts | Constructs and injects DaemonWorkspaceService into createServeApp; refactors tool-disable persistence. |
| packages/cli/src/serve/routes/workspaceFileWrite.ts | Updates bridge type import to AcpSessionBridge. |
| packages/cli/src/serve/permissionAudit.ts | Updates doc references from createHttpAcpBridge/HttpAcpBridge to createAcpSessionBridge/AcpSessionBridge. |
| packages/cli/src/serve/index.ts | Updates serve barrel exports to include createAcpSessionBridge and new bridge types. |
| packages/cli/src/serve/daemonStatusProvider.test.ts | Rewrites daemon status provider integration tests to exercise workspace-service env/preflight. |
| packages/cli/src/serve/acpSessionBridge.ts | Updates shim docs and re-exports to include createAcpSessionBridge while preserving legacy exports. |
| packages/acp-bridge/src/status.ts | Updates BridgeTimeoutError message prefix to AcpSessionBridge. |
| packages/acp-bridge/src/status.test.ts | Updates tests for new BridgeTimeoutError message prefix. |
| packages/acp-bridge/src/internal/testUtils.ts | Switches test helper bridge factory to createAcpSessionBridge + AcpSessionBridge type. |
| packages/acp-bridge/src/bridgeTypes.ts | Renames interface to AcpSessionBridge, removes workspace methods, adds generic workspace query/command methods, keeps HttpAcpBridge as deprecated alias. |
| packages/acp-bridge/src/bridgeOptions.ts | Updates docstrings to reference createAcpSessionBridge. |
| packages/acp-bridge/src/bridge.test.ts | Renames suite + removes tests for workspace methods migrated out of bridge. |
| docs/superpowers/specs/2026-05-27-daemon-workspace-service-design.md | Adds implementation design doc for the refactor (方案 C). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wenshao
left a comment
There was a problem hiding this comment.
[Critical] [typecheck] tsc --noEmit reports 2 errors on unchanged lines that reference a now-missing symbol:
bridge.ts(2674,14): error TS2304: Cannot find name 'STATUS_SCHEMA_VERSION'.
bridge.ts(2696,14): error TS2304: Cannot find name 'STATUS_SCHEMA_VERSION'.
The PR removed the import or definition of STATUS_SCHEMA_VERSION but two references remain in getWorkspaceMcpToolsStatus and getWorkspaceToolsStatus. This blocks the CI build step.
3e6bd9e to
5882d7f
Compare
chiga0
left a comment
There was a problem hiding this comment.
Deep Review — PR #4563: refactor(serve): extract DaemonWorkspaceService from AcpSessionBridge
1. 架构设计评估 — 与文档一致性
PR 完整实现了参考文档 §6.2 推荐的方案 C：
| 文档要求 | PR 实现 | 状态 |
|---|---|---|
新建 DaemonWorkspaceService 作为 L2 兄弟 |
✅ workspace-service/index.ts facade |
一致 |
HttpAcpBridge → AcpSessionBridge 改名 |
✅ 带 @deprecated 兼容别名 |
一致 |
| 拆分原则 = 是否依赖 ACP child | ✅ | 基本一致 |
| 两组件互不依赖 | 部分偏离 | |
| 跨切需求用单向回调注入 | ✅ publishWorkspaceEvent / queryWorkspaceStatus / invokeWorkspaceCommand |
一致 |
| trust/context 单点封装 | ✅ WorkspaceRequestContext |
一致 |
2. 关键发现 (Findings)
Finding 1 [Critical] — "互不依赖"承诺被打破：workspace service 实际上深度依赖 bridge 的 live channel
// runQwenServe.ts
queryWorkspaceStatus: (method, idle) =>
bridge.queryWorkspaceStatus(method, idle),
invokeWorkspaceCommand: (method, params, invokeOpts) =>
bridge.invokeWorkspaceCommand(method, params, invokeOpts),
publishWorkspaceEvent: (event) => bridge.publishWorkspaceEvent(event),
knownClientIds: () => bridge.knownClientIds(),
isChannelLive: () => bridge.sessionCount > 0,文档 §6.2 说："两组件互不依赖"，但 workspace service 的 5 个核心 deps 全部闭包 bridge 实例。这不是"互不依赖"——这是 workspace service 运行时单向依赖 bridge。
影响：
- 如果 bridge 未初始化 / shutdown，workspace service 的 status / restart 全部不可用
- 构造顺序被锁死（bridge 先于 service）
- 未来如果要把 workspace service 独立测试/部署，必须 mock 全部 5 个回调
建议：文档中"互不依赖"的说法应调整为"单向依赖（service → bridge callbacks），无反向"。架构上仍然合格（bridge 不知道 service 存在），但应诚实描述。或者考虑把 queryWorkspaceStatus / invokeWorkspaceCommand 的实现从 bridge 抽成独立的 ChannelAccessor，让 service 和 bridge 都依赖这个更低层的组件——真正实现"互不依赖"。
Finding 2 [Critical] — invokeWorkspaceCommand 返回 as any 类型擦除
// bridge.ts
async invokeWorkspaceCommand(method, params, invokeOpts) {
...
return response as any; // ← 类型安全丧失
},bridge 的 invokeWorkspaceCommand<T> 签名承诺返回 T，但实现用 as any 消除了类型约束。如果 ACP child 返回了不匹配 T 的结构，编译期和运行期都不会报错——下游 workspace service 会拿到一个 shape 不对的对象，直到某个字段访问 .restarted / .durationMs 爆 undefined。
建议：
- 至少在 workspace service 侧对
restartMcpServer结果做 runtime shape check - 或者把泛型
<T>降级为unknown并在 caller 侧显式 narrow——把 unsafe cast 收到离业务最近的地方
Finding 3 [Critical] — deviceFlowRegistry: undefined / subagentManager: undefined + as 强转 = 运行时 NPE 地雷
// runQwenServe.ts
deviceFlowRegistry: undefined,
subagentManager: undefined,但 facade 内部：
const auth = createAuthService({
registry: deviceFlowRegistry as DeviceFlowRegistry, // undefined as DeviceFlowRegistry
});
const agents = createAgentsService({
subagentManager: subagentManager as SubagentManager, // undefined as SubagentManager
});facade 在构造时就创建了 sub-service 并暴露了 .auth / .agents 属性。如果任何代码路径意外调了 workspace.auth.startDeviceFlow(ctx, ...) 或 workspace.agents.list(ctx)，就会在 undefined.someMethod() 上 NPE——无任何有意义的错误消息。
建议（三选一）：
- sub-service factory 内加 guard：
if (!registry) throw new Error('AuthService: DeviceFlowRegistry not wired') .auth/.agents做成 lazy getter，缺依赖时抛 "not available" 错误DaemonWorkspaceService接口上把auth/agents标为 optional（auth?: AuthService）
Finding 4 [Suggestion] — getWorkspaceEnvStatus 丢失了 bridge 原版的 error 日志
原 bridge 实现：
} catch (err) {
writeStderrLine(`qwen serve: statusProvider.getEnvStatus failed; falling back...`);
return createIdleEnvStatus(boundWorkspace, acpChannelLive);
}新 workspace service 实现：
} catch {
return createIdleEnvStatus(boundWorkspace, false);
}诊断信息丢失。生产中如果 statusProvider.getEnvStatus 持续报错，operator 无法从 stderr 看到原因。getWorkspacePreflightStatus 的 daemon cells catch 也有同样问题。
建议：注入一个 logger 回调到 deps，在 catch 中输出错误信息。
Finding 5 [Suggestion] — getWorkspacePreflightStatus 降低了 error 结构精度
原 bridge 实现用 mapDomainErrorToErrorKind(err) 给 error cell 加了 errorKind 字段（区分 timeout / channel-close / unknown）。新实现只有 { kind, status, error } — errorKind 字段丢失。SDK client 侧可能依赖此字段做差异化 UI。
建议：从 bridge 错误映射中提取 mapDomainErrorToErrorKind，在 workspace service 里复用。
Finding 6 [Suggestion] — REST routes 实际迁移范围 < PR body 描述
PR body 说 "REST routes rewired to call workspace service"。从 diff 看，实际迁移了 env/preflight/init/tool-toggle/restart。但 file / agents / memory 路由仍然在原文件里直接用 bridge（只改了类型名）。
这作为分期实施是合理的，但 PR body 应明确为 "status/init/tool-toggle/restart routes rewired; file/agents/memory route migration deferred"。
3. 分层合理性总评
| 维度 | 评价 |
|---|---|
| L1 薄 | ✅ 路由 handler 只做 ctx 构造 + 调 service + 错误映射 |
| L2 聚焦 | ✅ 两个 L2 门面各管一域 |
| trust/audit 单点 | ✅ WorkspaceRequestContext 贯穿所有 sub-service |
| bridge 瘦身 | ✅ 移除了 ~750 行 workspace-local 逻辑 |
| 向后兼容 | ✅ HttpAcpBridge type alias + createHttpAcpBridge re-export |
4. Client 接入友好度
- SDK 无修改：REST 接口 surface 未变，对现有 client 零影响
WorkspaceRequestContext设计合理：originatorClientIdoptional（GET 路由不必传）、route为 audit、workspaceCwd为 trust boundaryvalidateClientId独立模块，sub-service 可选用——良好的 opt-in 模式
5. 建议行动项
| 优先级 | 项目 |
|---|---|
| Must Fix | invokeWorkspaceCommand 的 as any — service 侧加 runtime shape guard |
| Must Fix | undefined as DeviceFlowRegistry 强转 — 改为 fail-early guard 或 optional 接口 |
| Should Fix | 保留 env/preflight 的 error 日志（原 bridge 有 writeStderrLine） |
| Should Fix | PR body 修正迁移范围描述 |
| Nice to Have | preflight error cell 保留 errorKind 字段 |
| Nice to Have | 文档修正："互不依赖"→"单向回调依赖，bridge 不知道 service 存在" |
补充：关于 Finding #1 "互不依赖" — 当前实现优于文档字面描述经过进一步分析，当前 PR 的 callback injection 实现是正确的架构选择，不需要为了满足"互不依赖"的字面描述去抽出第三方组件。 为什么当前实现更好如果要实现字面意义上的"互不依赖"，就得抽出
PR 当前的 callback injection 模式已经达到了架构上有意义的独立：
这正是 hexagonal architecture 里"通过端口解耦"的标准做法——workspace service 依赖的是 contract（函数签名），不是 concrete（bridge 类型）。实质上的 DIP（依赖倒置）已经到位。 文档建议修正原文档 §6.2 中"两组件互不依赖"建议改为：
这样既诚实描述了运行时的数据流方向，又不会误导读者觉得需要再抽一层。 总结：PR 在这个点上的实现选择是 pragmatic + architecturally sound 的，值得肯定。 callback injection 既保持了 bridge 的内聚性（不为外部消费者拆碎自己的内部状态），又给 workspace service 提供了干净的测试边界和未来的替换自由度。这比追求图上的"对称独立"更有工程价值。 |
Fix Summary — addressing wenshao CHANGES_REQUESTED + chiga0 reviewAll 12 wenshao [Critical] items + chiga0 findings fixed in
Deferred (noted, not blocking):
🤖 Generated with Qwen Code |
Fix Summary R2 — addressing wenshao's second CHANGES_REQUESTEDFixed in
Deferred (Suggestions):
|
|
[Critical] The rename from Generated By GPT-5 model |
|
Agreed, fixed in 25b0661 — updated runQwenServe.test.ts import to ./acpSessionBridge.js. |
170dfa5 to
024caa8
Compare
wenshao
left a comment
There was a problem hiding this comment.
[Critical] [build] npm run build fails with 8 TS2551/TS2339 errors in packages/cli/src/serve/acpHttp/dispatch.ts (lines 751, 757, 764, 768, 774, 793, 815, 856). The workspace-service extraction removed getWorkspaceMcpStatus, getWorkspaceSkillsStatus, getWorkspaceProvidersStatus, getWorkspaceEnvStatus, getWorkspacePreflightStatus, initWorkspace, setWorkspaceToolEnabled, and restartMcpServer from the AcpSessionBridge interface and implementation, but dispatch.ts (the ACP northbound JSON-RPC handler) still calls this.bridge.<method>(). Either wire WorkspaceService into dispatch and route through it, or keep the methods on the bridge interface until the dispatch migration PR.
|
Agreed, fixed in 5f09914 — injected DaemonWorkspaceService into AcpDispatcher. All 8 workspace method calls now route through the service, matching the REST surface pattern. |
wenshao
left a comment
There was a problem hiding this comment.
[Critical] [typecheck] server.test.ts lines 796, 804, 811 — 8 mock function parameters implicitly have any type (TS7006). These are on unchanged lines outside the diff hunks, so they can't be posted as inline comments. The mock helpers at lines 796 (toolName, enabled, originatorClientId), 804 (initOpts, originatorClientId), and 811 (serverName, originatorClientId, restartOpts) need explicit type annotations to satisfy noImplicitAny.
— qwen3.7-max via Qwen Code /review
|
Fixed in af56189 — added explicit type annotations to mock helpers at lines 796/804/811. |
4272001 to
6300583
Compare
R35 Review Response
|
wenshao
left a comment
There was a problem hiding this comment.
[Critical] forceFlushMetrics() runs sequentially before bridge.shutdown() with no timeout (runQwenServe.ts:1130). If the OTLP collector is unreachable, shutdown blocks for the exporter's full timeout (typically 30s gRPC). In k8s with 30s terminationGracePeriodSeconds, the pod gets SIGKILL before bridge.shutdown() starts, orphaning agent children. Fix: add Promise.race([forceFlushMetrics(), sleep(5000)]) or flush in parallel with shutdown. This also caused 10 runQwenServe tests to hang in afterEach (server.test.ts).
— qwen3.7-max via Qwen Code /review
| }); | ||
| } | ||
|
|
||
| return result; |
There was a problem hiding this comment.
[Critical] as cast bypasses discriminated union narrowing on RestartMcpServerResult. By the else branch, TypeScript has already narrowed result to the second variant via 'entries' in result + result.restarted === true checks. The cast is unnecessary and erases the field's required-ness and literal type.
| return result; | |
| reason: result.reason, |
— qwen3.7-max via Qwen Code /review
| const idleCells = createIdleAcpPreflightCells(); | ||
|
|
||
| // Get daemon cells (local, no ACP query). | ||
| let daemonCells: ServeWorkspacePreflightStatus['cells'] = []; |
There was a problem hiding this comment.
[Suggestion] getDaemonPreflightCells() and queryWorkspaceStatus() are independent (daemon cells from local state, ACP cells from child process) but execute sequentially. Parallelize with Promise.allSettled to halve /workspace/preflight latency for SDK polling.
— qwen3.7-max via Qwen Code /review
| const idleCells = createIdleAcpPreflightCells(); | ||
|
|
||
| // Get daemon cells (local, no ACP query). | ||
| let daemonCells: ServeWorkspacePreflightStatus['cells'] = []; |
There was a problem hiding this comment.
[Suggestion] getDaemonPreflightCells() and queryWorkspaceStatus() are independent (daemon cells from local state, ACP cells from child process) but execute sequentially. /workspace/preflight is polled by SDKs on a timer — parallelize with Promise.allSettled to halve response latency.
— qwen3.7-max via Qwen Code /review
| createDaemonWorkspaceService({ | ||
| boundWorkspace, | ||
| contextFilename: deps.contextFilename ?? 'QWEN.md', | ||
| statusProvider: createDaemonStatusProvider(), |
There was a problem hiding this comment.
[Suggestion] createDaemonStatusProvider() is called twice in createServeApp — once here for the workspace service and once at bridge construction (line ~554). runQwenServe.ts correctly creates a single shared instance. Extract a local to match:
const statusProvider = createDaemonStatusProvider();
// pass to both bridge and workspace service— qwen3.7-max via Qwen Code /review
R36 Review ResponseThe review body's [Critical] about
No code changes needed this round — all findings are pre-existing patterns or deferred type improvements. |
| // Env status is answered daemon-locally from process state — no ACP | ||
| // query needed. The old bridge used statusProvider.getEnvStatus() | ||
| // directly; replicate that behavior here. | ||
| const acpChannelLive = isChannelLive?.() ?? false; |
There was a problem hiding this comment.
[Suggestion] getWorkspaceEnvStatus silently returns idle placeholders on error with only writeStderrLine. No programmatic signal distinguishes "daemon working, ACP cold" from "status provider threw." Compare with getWorkspacePreflightStatus which surfaces errors in the response body's errors array. Consider mirroring that pattern.
— qwen3.7-max via Qwen Code /review
| acpChannelLive, | ||
| cells: [...daemonCells, ...acpCells], | ||
| ...(errors && errors.length > 0 ? { errors } : {}), | ||
| } as ServeWorkspacePreflightStatus; |
There was a problem hiding this comment.
[Suggestion] as ServeWorkspacePreflightStatus cast masks potential drift — if the interface gains a new required field, this compiles cleanly but produces an incomplete response. Build without the cast using a typed local variable with conditional errors assignment.
— qwen3.7-max via Qwen Code /review
✅ Local Verification Report — PR #4563refactor: extract DaemonWorkspaceService from AcpSessionBridge Environment
Build Verification
Test Results
Pre-existing Failures (not introduced by this PR)
None of these failures touch files modified by this PR ( Critical Review Issues — All Addressed
Minor Note
Verdict✅ PASS — Architecture extraction is clean, all workspace-service tests pass, Critical review items addressed, no regressions introduced. Verified by wenshao |
Rebased on daemon_mode_b_main (post #4563 merge). Adds all wave 1+2 methods in a single commit: - Session (6): recap, btw, shell, detach, context_usage, tasks - Memory (2): workspace/memory read + write (1MB limit, scope/mode validation) - Files (7): read, read_bytes, stat, list, glob, write, edit (via WorkspaceFileSystem) - Auth (4): status, device_flow start/get/cancel (projected, no verification leak) - Workspace (5): tools, mcp/tools, mcp/servers add/remove, sessions/delete (100 cap, dedup) - Agents (5): list, get, create, update, delete (SubagentManager) Production hardening: - toRpcError: FsError, MemoryError, AuthError, SubagentError → structured errorKind - Error data propagation: catch blocks forward data to JSON-RPC error frames - BTW_MAX_INPUT_LENGTH validation, shell audit logging - sessions/delete: 100 cap + dedup + strict types + error preservation - auth/status: verification material stripped (security) Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Rebased on daemon_mode_b_main (post #4563 merge). Adds all wave 1+2 methods in a single commit: - Session (6): recap, btw, shell, detach, context_usage, tasks - Memory (2): workspace/memory read + write (1MB limit, scope/mode validation) - Files (7): read, read_bytes, stat, list, glob, write, edit (via WorkspaceFileSystem) - Auth (4): status, device_flow start/get/cancel (projected, no verification leak) - Workspace (5): tools, mcp/tools, mcp/servers add/remove, sessions/delete (100 cap, dedup) - Agents (5): list, get, create, update, delete (SubagentManager) Production hardening: - toRpcError: FsError, MemoryError, AuthError, SubagentError → structured errorKind - Error data propagation: catch blocks forward data to JSON-RPC error frames - BTW_MAX_INPUT_LENGTH validation, shell audit logging - sessions/delete: 100 cap + dedup + strict types + error preservation - auth/status: verification material stripped (security) Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Rebased on daemon_mode_b_main (post #4563 merge). Adds all wave 1+2 methods in a single commit: - Session (6): recap, btw, shell, detach, context_usage, tasks - Memory (2): workspace/memory read + write (1MB limit, scope/mode validation) - Files (7): read, read_bytes, stat, list, glob, write, edit (via WorkspaceFileSystem) - Auth (4): status, device_flow start/get/cancel (projected, no verification leak) - Workspace (5): tools, mcp/tools, mcp/servers add/remove, sessions/delete (100 cap, dedup) - Agents (5): list, get, create, update, delete (SubagentManager) Production hardening: - toRpcError: FsError, MemoryError, AuthError, SubagentError → structured errorKind - Error data propagation: catch blocks forward data to JSON-RPC error frames - BTW_MAX_INPUT_LENGTH validation, shell audit logging - sessions/delete: 100 cap + dedup + strict types + error preservation - auth/status: verification material stripped (security) Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Rebased on daemon_mode_b_main (post #4563 merge). Adds all wave 1+2 methods in a single commit: - Session (6): recap, btw, shell, detach, context_usage, tasks - Memory (2): workspace/memory read + write (1MB limit, scope/mode validation) - Files (7): read, read_bytes, stat, list, glob, write, edit (via WorkspaceFileSystem) - Auth (4): status, device_flow start/get/cancel (projected, no verification leak) - Workspace (5): tools, mcp/tools, mcp/servers add/remove, sessions/delete (100 cap, dedup) - Agents (5): list, get, create, update, delete (SubagentManager) Production hardening: - toRpcError: FsError, MemoryError, AuthError, SubagentError → structured errorKind - Error data propagation: catch blocks forward data to JSON-RPC error frames - BTW_MAX_INPUT_LENGTH validation, shell audit logging - sessions/delete: 100 cap + dedup + strict types + error preservation - auth/status: verification material stripped (security) Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Rebased on daemon_mode_b_main (post #4563 merge). Adds all wave 1+2 methods in a single commit: - Session (6): recap, btw, shell, detach, context_usage, tasks - Memory (2): workspace/memory read + write (1MB limit, scope/mode validation) - Files (7): read, read_bytes, stat, list, glob, write, edit (via WorkspaceFileSystem) - Auth (4): status, device_flow start/get/cancel (projected, no verification leak) - Workspace (5): tools, mcp/tools, mcp/servers add/remove, sessions/delete (100 cap, dedup) - Agents (5): list, get, create, update, delete (SubagentManager) Production hardening: - toRpcError: FsError, MemoryError, AuthError, SubagentError → structured errorKind - Error data propagation: catch blocks forward data to JSON-RPC error frames - BTW_MAX_INPUT_LENGTH validation, shell audit logging - sessions/delete: 100 cap + dedup + strict types + error preservation - auth/status: verification material stripped (security) Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Rebased on daemon_mode_b_main (post #4563 merge). Adds all wave 1+2 methods in a single commit: - Session (6): recap, btw, shell, detach, context_usage, tasks - Memory (2): workspace/memory read + write (1MB limit, scope/mode validation) - Files (7): read, read_bytes, stat, list, glob, write, edit (via WorkspaceFileSystem) - Auth (4): status, device_flow start/get/cancel (projected, no verification leak) - Workspace (5): tools, mcp/tools, mcp/servers add/remove, sessions/delete (100 cap, dedup) - Agents (5): list, get, create, update, delete (SubagentManager) Production hardening: - toRpcError: FsError, MemoryError, AuthError, SubagentError → structured errorKind - Error data propagation: catch blocks forward data to JSON-RPC error frames - BTW_MAX_INPUT_LENGTH validation, shell audit logging - sessions/delete: 100 cap + dedup + strict types + error preservation - auth/status: verification material stripped (security) Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Rebased on daemon_mode_b_main (post #4563 merge). Adds all wave 1+2 methods in a single commit: - Session (6): recap, btw, shell, detach, context_usage, tasks - Memory (2): workspace/memory read + write (1MB limit, scope/mode validation) - Files (7): read, read_bytes, stat, list, glob, write, edit (via WorkspaceFileSystem) - Auth (4): status, device_flow start/get/cancel (projected, no verification leak) - Workspace (5): tools, mcp/tools, mcp/servers add/remove, sessions/delete (100 cap, dedup) - Agents (5): list, get, create, update, delete (SubagentManager) Production hardening: - toRpcError: FsError, MemoryError, AuthError, SubagentError → structured errorKind - Error data propagation: catch blocks forward data to JSON-RPC error frames - BTW_MAX_INPUT_LENGTH validation, shell audit logging - sessions/delete: 100 cap + dedup + strict types + error preservation - auth/status: verification material stripped (security) Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Rebased on daemon_mode_b_main (post #4563 merge). Adds all wave 1+2 methods in a single commit: - Session (6): recap, btw, shell, detach, context_usage, tasks - Memory (2): workspace/memory read + write (1MB limit, scope/mode validation) - Files (7): read, read_bytes, stat, list, glob, write, edit (via WorkspaceFileSystem) - Auth (4): status, device_flow start/get/cancel (projected, no verification leak) - Workspace (5): tools, mcp/tools, mcp/servers add/remove, sessions/delete (100 cap, dedup) - Agents (5): list, get, create, update, delete (SubagentManager) Production hardening: - toRpcError: FsError, MemoryError, AuthError, SubagentError → structured errorKind - Error data propagation: catch blocks forward data to JSON-RPC error frames - BTW_MAX_INPUT_LENGTH validation, shell audit logging - sessions/delete: 100 cap + dedup + strict types + error preservation - auth/status: verification material stripped (security) Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Rebased on daemon_mode_b_main (post #4563 merge). Adds all wave 1+2 methods in a single commit: - Session (6): recap, btw, shell, detach, context_usage, tasks - Memory (2): workspace/memory read + write (1MB limit, scope/mode validation) - Files (7): read, read_bytes, stat, list, glob, write, edit (via WorkspaceFileSystem) - Auth (4): status, device_flow start/get/cancel (projected, no verification leak) - Workspace (5): tools, mcp/tools, mcp/servers add/remove, sessions/delete (100 cap, dedup) - Agents (5): list, get, create, update, delete (SubagentManager) Production hardening: - toRpcError: FsError, MemoryError, AuthError, SubagentError → structured errorKind - Error data propagation: catch blocks forward data to JSON-RPC error frames - BTW_MAX_INPUT_LENGTH validation, shell audit logging - sessions/delete: 100 cap + dedup + strict types + error preservation - auth/status: verification material stripped (security) Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Rebased on daemon_mode_b_main (post #4563 merge). Adds all wave 1+2 methods in a single commit: - Session (6): recap, btw, shell, detach, context_usage, tasks - Memory (2): workspace/memory read + write (1MB limit, scope/mode validation) - Files (7): read, read_bytes, stat, list, glob, write, edit (via WorkspaceFileSystem) - Auth (4): status, device_flow start/get/cancel (projected, no verification leak) - Workspace (5): tools, mcp/tools, mcp/servers add/remove, sessions/delete (100 cap, dedup) - Agents (5): list, get, create, update, delete (SubagentManager) Production hardening: - toRpcError: FsError, MemoryError, AuthError, SubagentError → structured errorKind - Error data propagation: catch blocks forward data to JSON-RPC error frames - BTW_MAX_INPUT_LENGTH validation, shell audit logging - sessions/delete: 100 cap + dedup + strict types + error preservation - auth/status: verification material stripped (security) Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Summary
HttpAcpBridge→AcpSessionBridge(now honestly session-only)DaemonWorkspaceServicefacadequeryWorkspaceStatus+invokeWorkspaceCommandso the facade reaches the ACP child via callback injection (no direct bridge reference)server.ts+acpHttp/dispatch.tsroute those workspace ops through the facade; HTTP/ACP API surface unchangedArchitecture
Splitting principle: workspace-scoped → service, session-scoped → bridge.
Key Design Decisions
publishWorkspaceEvent,queryWorkspaceStatus,invokeWorkspaceCommand) — the facade holds function refs, not the bridge type (one-way dependency inversion).originatorClientIdis optional — matches existingAuditContextbehavior (reads work without clientId).isChannelLive()on the bridge — wraps the internalliveChannelInfo()probe so the facade'sacpChannelLivefield is accurate during the cold-spawn / post-kill windows (wheresessionCount > 0would mislead).DaemonStatusProvider(daemon-local) not an ACP query.What's Deferred
deviceFlowRegistry/subagentManagerinjection, thevalidateClientIdfacade gate, and route→service e2e tests./workspace/memory,/workspace/agentsroute migration — still served by the existingworkspaceMemory.ts/workspaceAgents.tshandlers.initWorkspace→fsFactory/WorkspaceFileSystem— still uses the carried-over rawnode:fsimplementation (with the existing TOCTOU / symlink guards); the fsFactory trust-gate + audit migration is a follow-up (no regression vs. the old bridge)./acpnorthboundqwen/workspace/*methods — blocked on PR feat(daemon): ACP Streamable HTTP transport at /acp [RFD #721] #4472 (ACP Streamable HTTP transport).Test Plan
cd packages/acp-bridge && npx vitest run— 309 tests passcd packages/cli && npx vitest run src/serve/workspace-service/ src/serve/daemonStatusProvider.test.ts— 40 tests passcd packages/cli && npx vitest run src/serve/server.test.ts— 373 tests passtsc --noEmit— 0 new errors in modified packagesCloses #4542
🤖 Generated with Qwen Code