mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-19 07:54:38 +00:00
105 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
jinye
|
93ad0ff131
|
fix(serve): auth device-flow follow-up for #4255 review threads (#4291) | ||
|
jinye
|
3ffe321cfd
|
feat(serve): MCP guardrail push events + hysteresis (#4175 Wave 3 PR 14b) (#4271) | ||
|
jinye
|
6f7a48936f
|
feat(serve): approval / tools / init / MCP-restart mutation routes (#4175 Wave 4 PR 17) (#4282)
* feat(core): introduce TrustGateError for setApprovalMode (#4175 Wave 4 PR 17) Adds a named subclass `TrustGateError` thrown by `Config.setApprovalMode` when the requested mode would grant privileged tool autonomy in a folder the user has not marked as trusted. Daemon mutation routes can now recognize this rejection class without depending on message text. Extends `mapDomainErrorToErrorKind` in `packages/cli/src/serve/status.ts` to map `TrustGateError → 'auth_env_error'`. Matches by `err.name` rather than `instanceof` because cross-package bundling can produce duplicate class instances where `instanceof` returns false. Test covers both the real class and a name-synthesized instance. Foundation for the `POST /session/:id/approval-mode` route landing in a follow-up commit in this PR. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * feat(core): add disabledTools workspace setting (#4175 Wave 4 PR 17) Introduces a per-workspace skip-registration mechanism for tool names, distinct from `permissions.deny` (which keeps the tool registered and blocks invocation). Tools listed in `disabledTools` are not registered at all and never appear in `/tools`, `getAllTools()`, or function-call discovery — both built-ins and MCP-discovered tools flow through `ToolRegistry.registerTool` / `registerFactory`, so gating there covers every registration path. - `ConfigParameters.disabledTools?: string[]` (frozen into a `ReadonlySet` at Config construction; queried via `Config.getDisabledTools()`) - `ToolRegistry.registerTool` and `ToolRegistry.registerFactory` skip when the tool name is in the disabled set, with a debug log line - New `settings.tools.disabled: string[]` (UNION merge across scopes), wired from `loadCliConfig` into ConfigParameters - Tests pin the contract: skip at register, lazy factory skip, and the "next refresh" semantic (already-registered tools are unaffected by a subsequent toggle — the disabled set is consulted at register time, not at lookup time) Foundation for the `POST /workspace/tools/:name/enable` route in a follow-up commit; the bridge will write the settings file directly, and the next ACP child spawn will pick up the change. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * feat(serve): add session approval-mode mutation route (#4175 Wave 4 PR 17) Adds POST /session/:id/approval-mode — the first strict-gated session mutation surface introduced in Wave 4 alongside PR 16 / PR 21. Remote clients can switch a live session's approval mode (plan / default / auto-edit / yolo) without touching the user's host CLI. Routing: - Route handler validates `mode` against the closed `APPROVAL_MODES` enum and an optional `persist: boolean` flag (400 on either) - Bridge `setSessionApprovalMode` forwards through the new `qwen/control/session/approval_mode` ACP extMethod (introduced in a new `SERVE_CONTROL_EXT_METHODS` namespace) so the change lands inside the ACP child's per-session `Config` - `persist: true` writes `tools.approvalMode` to workspace settings via a new `BridgeOptions.persistApprovalMode` callback wired in `runQwenServe`. Default is ephemeral so a remote caller does not pollute the user's host settings unless asked Trust gate translation: - ACP child catches `TrustGateError` from `Config.setApprovalMode` and re-raises as a JSON-RPC error with `data.errorKind: 'trust_gate'` - Bridge detects the structured payload and re-instantiates the typed `TrustGateError` (since the class name does not survive the wire) - `sendBridgeError` translates to HTTP 403 with the closed PR-13 `errorKind: 'auth_env_error'` taxonomy SDK additions: - `DaemonClient.setSessionApprovalMode(sessionId, mode, opts?, clientId?)` mirrors the route shape and forwards `X-Qwen-Client-Id` - New `DaemonApprovalMode` literal union and `DAEMON_APPROVAL_MODES` const tuple; `DaemonApprovalModeResult` for the route response - New `approval_mode_changed` typed event on `DaemonControlEvent`, reducer integration on `DaemonSessionViewState` (`approvalMode` / `approvalModeChangedCount` / `lastApprovalModeChange`) - Drift detector `approvalMode.test.ts` walks core's `ApprovalMode` enum and fails CI if `APPROVAL_MODES` or `DAEMON_APPROVAL_MODES` drift in either direction New capability tag `session_approval_mode_control` (always-on, since v1). 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * feat(serve): add workspace tool toggle route (#4175 Wave 4 PR 17) Adds POST /workspace/tools/:name/enable — strict-gated mutation route that toggles a tool name in the workspace's `tools.disabled` settings list. Pure file IO + workspace-scoped event fan-out; no ACP roundtrip. - Bridge `setWorkspaceToolEnabled(toolName, enabled, originatorClientId)` invokes the new `BridgeOptions.persistDisabledTools` callback. The default `runQwenServe` wires it to `loadSettings(workspace).setValue( 'tools.disabled', merged)` with a fresh load on each call so concurrent edits from other writers stay safe across the read/modify/write window - New private `broadcastWorkspaceEvent` helper fan-outs to every live session SSE bus, swallowing per-bus errors so a single torn-down session can't block its peers. Naming mirrors PR 21 #4255 (the post- PR-16 fold-in will collapse the two helpers) - Unknown tool names are accepted: the daemon has no authoritative tool registry to validate against (built-ins live inside the ACP child, MCP tools are discovered post-spawn). Pre-disabling a not-yet-installed MCP tool is a legitimate use case - Live ACP children retain already-registered tools — the toggle takes effect on the next ACP child spawn (`tools.disabled` is consulted at Config construction time, gated in ToolRegistry.registerTool by PR 17 commit 2) SDK additions: - `DaemonClient.setWorkspaceToolEnabled(toolName, enabled, clientId?)` with URL-encoded tool name - `DaemonToolToggleResult` + `DaemonToolToggledEvent` typed event, reducer integration on `DaemonSessionViewState` (`toolToggleCount` / `lastToolToggle`) - `asKnownDaemonEvent` runtime guard for `tool_toggled` AND `approval_mode_changed` (the latter was missed in commit 3 — without this entry the events were silently filed as `unrecognizedKnownEvent` by `reduceDaemonSessionEvent`, never reaching the typed reducer cases) New capability tag `workspace_tool_toggle` (always-on, since v1). 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * feat(serve): add workspace init route (#4175 Wave 4 PR 17) Adds POST /workspace/init — strict-gated mutation route that scaffolds an empty `QWEN.md` (or whatever `getCurrentGeminiMdFilename()` returns under `--memory-file-name` overrides) at the daemon's bound workspace root. Mechanical only — does NOT invoke the LLM. Clients that want AI-driven content fill should follow up with POST /session/:id/prompt. Behavior: - Default refuses to overwrite when the target file exists with non- whitespace content; the bridge throws `WorkspaceInitConflictError` which the route translates to HTTP 409 `workspace_init_conflict` with the resolved path + size in the body - `body: {force: true}` overwrites unconditionally; response carries `action: 'overwrote'` vs `'created'` so SDK consumers can render the difference - Whitespace-only existing content is treated as absent (no 409), matching the local `/init` slash command's behavior so a half- broken init left with an empty file doesn't trap the user - Pure file IO + workspace-scoped event fan-out — no ACP roundtrip; works regardless of whether an ACP child is alive - Fan-outs `workspace_initialized` event with `{path, action}` to every live session SSE bus via the `broadcastWorkspaceEvent` helper introduced in commit 4 SDK additions: - `DaemonClient.initWorkspace(opts?, clientId?)` with conditional body emission (omits `force` unless explicitly true so older daemons that reject unknown body fields stay compatible) - `DaemonInitWorkspaceResult` + `DaemonWorkspaceInitializedEvent` typed event with runtime guard (`isWorkspaceInitializedData`), reducer integration on `DaemonSessionViewState` (`workspaceInitCount` / `lastWorkspaceInit`) New typed error class `WorkspaceInitConflictError` exported from `packages/cli/src/serve/index.ts` so direct embeds can match it via `instanceof`. New capability tag `workspace_init` (always-on, since v1). 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * feat(serve): add MCP server restart route with budget guard (#4175 Wave 4 PR 17) Adds POST /workspace/mcp/:server/restart — strict-gated mutation route that performs a single-server MCP restart through the ACP child's `McpClientManager.discoverMcpToolsForServer`. Pre-checks the live budget snapshot from PR 14 v1 (#4247) so a restart on a budget-saturated workspace returns a soft refusal rather than triggering a `BudgetExhaustedError` cascade through the discovery loop. Decision logic (ACP-side, in `qwen/control/workspace/mcp/restart` extMethod): - Server not in `getMcpServers()` → JSON-RPC `resourceNotFound` → HTTP 404 - Server in `excludedMcpServers` → 200 with `{skipped:true, reason:'disabled'}` - `manager.isServerDiscovering(name)` → 200 with `{reason:'in_flight'}` - Mode is `enforce`, server not in `reservedSlots`, total ≥ budget → 200 with `{reason:'budget_would_exceed'}` - Otherwise: `discoverMcpToolsForServer(name, config)`, return `{restarted:true, durationMs}` Soft refusals still return 200 because the route understood the request and reached a deterministic answer about why no restart happened. Only hard "we cannot answer" cases (unknown server, no live ACP child) escalate to non-2xx. This mirrors PR 14 v1's discovery-time refusal contract: refusals don't throw, they get recorded. Bridge: - New `restartMcpServer(serverName, originatorClientId)` forwards through the new `SERVE_CONTROL_EXT_METHODS.workspaceMcpRestart` extMethod against the live `liveChannelInfo()` channel - Throws `SessionNotFoundError` (mapped to HTTP 404) when no ACP child is alive — restart inherently requires a live `McpClientManager` instance - Fan-outs `mcp_server_restarted` (success) or `mcp_server_restart_refused` (skip) to every live session SSE bus Core: - New public `McpClientManager.isServerDiscovering(serverName): boolean` — reads `serverDiscoveryPromises.has(name)` so the daemon can short-circuit a redundant restart with `skipped:in_flight` instead of awaiting the original discovery promise (HTTP latency stays bounded) SDK additions: - `DaemonClient.restartMcpServer(serverName, clientId?)` with URL-encoded server name - `DaemonMcpRestartResult` discriminated union, two new typed events (`DaemonMcpServerRestartedEvent`, `DaemonMcpServerRestartRefusedEvent`) with runtime guards, reducer integration on `DaemonSessionViewState` (`mcpRestartCount` / `lastMcpRestart` / `mcpRestartRefusedCount` / `lastMcpRestartRefused`) New capability tag `workspace_mcp_restart` (always-on, since v1). 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * docs(serve): mutation control routes protocol section (#4175 Wave 4 PR 17) Adds a "Mutation: approval, tools, init, MCP restart" section to the developer protocol doc covering all four PR 17 routes: - POST /session/:id/approval-mode — `{mode, persist?}` request, four closed-enum modes, trust-gate 403 with `errorKind: 'auth_env_error'`, `approval_mode_changed` SSE event (session-scoped) - POST /workspace/tools/:name/enable — `{enabled}` request, unknown names accepted, "next-spawn semantics" call-out, `tool_toggled` SSE event (workspace-scoped fan-out) - POST /workspace/init — `{force?}` request, scaffold-only contract (no LLM call), 409 with `path` + `existingSize` body when the target exists with non-whitespace content, `workspace_initialized` SSE event (workspace-scoped) - POST /workspace/mcp/:server/restart — empty body, soft-skip decision table (in_flight / disabled / budget_would_exceed), `mcp_server_restarted` and `mcp_server_restart_refused` SSE events Capability list at the top of the file updated with the four new tags (and a missing-from-PR-13 fix for `workspace_env` / `workspace_preflight`). User-facing `qwen-serve.md` gains a one-line "Remote runtime control" bullet under "What it gives you" pointing to the four routes and clarifying that `/workspace/init` is mechanical only. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(serve): fold-in 1 — wenshao + gpt-5.5 review (#4175 Wave 4 PR 17) Addresses 5 critical / 4 high / 2 medium items from #4282 review. CI blocker (wenshao H1) - Move `approvalMode.test.ts` from `packages/cli/src/acp-integration/` to `packages/sdk-typescript/test/unit/approval-mode-drift.test.ts`. The CLI package has no `@qwen-code/sdk` dep and the tsconfig has no path mapping for it, so `tsc --build` failed `Cannot find module '@qwen-code/sdk'` on Lint + Test (mac/linux/windows). The SDK package is the right host: it already depends on `@qwen-code/qwen-code-core`, and the test pins the SDK ↔ core contract directly. Also drop the tautological `APPROVAL_MODES contains every ApprovalMode enum value` check — `APPROVAL_MODES` is defined as `Object.values(ApprovalMode)` in core, so that assertion can never fire. Critical (gpt-5.5 via wenshao /review) - C1 (`initWorkspace` path traversal): `getCurrentGeminiMdFilename()` is settings-controlled. A daemon configured with `context.fileName: "../outside.md"` could resolve outside `boundWorkspace` and let this strict-gated mutation create or truncate a file outside the workspace boundary. Resolve and verify the joined path stays within `boundWorkspace`; reject otherwise. - C2 (`X-Qwen-Client-Id` forgery): the 3 workspace mutation routes (`/workspace/init`, `/workspace/tools/:name/enable`, `/workspace/mcp/:server/restart`) accepted any syntactically valid client id and stamped it onto fan-out events without checking `bridge.knownClientIds()`. Mirrors the inline validation pattern PR 16 already uses for `/workspace/memory` and `/workspace/agents`. Add `parseAndValidateWorkspaceClientId` shared helper in `server.ts` (collapses with PR 16's pattern when the Wave-4-wide DRY refactor lands). - C3 (MCP restart budget under-count): the pre-check used `accounting.total >= budget`, but enforce-mode capacity is reserved by `tryReserveSlot` via `reservedSlots` (which counts configured + in-flight + disconnected slot holders). `total` only counts CONNECTED, so a restart on a budget-saturated workspace passed the pre-check while the manager refused internally and the route reported `restarted: true`. Mirror the manager's policy by checking `reservedSlots.length`. - C4 (false `restarted: true` on broken MCP): `discoverMcpToolsForServer` catches reconnect/discovery errors internally (logs and resolves void), so the route reported `restarted: true` while the server stayed disconnected. After the call, verify the live `getMCPServerStatus(name)` is `MCPServerStatus.CONNECTED`; throw a structured JSON-RPC error otherwise. New typed bridge error `McpServerRestartFailedError` → HTTP 502 with `errorKind: 'protocol_error'`. - C5 (unknown MCP server falls through as 500): the agent-side `RequestError.resourceNotFound` was not specially handled by `sendBridgeError`, so a typo in the server name returned 500 indistinguishable from an internal daemon failure. Re-raise with structured `data.errorKind: 'mcp_server_not_found'`; bridge re-instantiates as `McpServerNotFoundError`; route maps to a stable 404 with `code: 'mcp_server_not_found'` and `serverName` in the body. High (wenshao) - H2 (`persistDisabledTools` scope leak): the callback read `fresh.merged.tools?.disabled` (UNION across System / SystemDefaults / User / Workspace) and wrote the result back into `SettingScope.Workspace`, copying entries from higher scopes into the workspace file on the first toggle. Subsequent removals at the originating scope (e.g. User) would no longer take effect. Read from the WORKSPACE-scope `LoadedSettings` only via `fresh.forScope(SettingScope.Workspace).settings.tools?.disabled`. - H3 (silent persist no-op): `setSessionApprovalMode` with `persist: true` returned HTTP 200 + `persisted: false` when no `persistApprovalMode` callback was wired, indistinguishable from "hook ran but failed" or genuine `persisted: true`. Throw asymmetrically with the sibling `setWorkspaceToolEnabled` (which already throws in the same situation). - H4 (whitespace-only init clobber): `/workspace/init` overwrote a whitespace-only `QWEN.md` with `action: 'created'` despite `force` not being passed, destroying the user's whitespace content (template, half-written init, intentional newline) without a signal. Treat existing-and-whitespace-only as a no-op; return `action: 'noop'` and skip the write. Adds `'noop'` to the discriminator union on `DaemonInitWorkspaceResult` and the `workspace_initialized` event payload. Medium - M1 (SDK `clientId` position consistency): the four new mutation helpers placed `clientId` inconsistently (4th vs 3rd vs 2nd). Fold `clientId` into the trailing options bag for all four. Matches the existing `context: { clientId }` argument the bridge layer already uses internally; reduces caller boilerplate for callers that always stamp clientId for audit. - M2 (dead `instanceof String` branch): drop the no-op `instanceof String` clause in `setSessionApprovalMode`'s wire-error reconstruction — `Error.message` is always a primitive string. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * chore(vscode): regenerate settings.schema.json for tools.disabled (#4175 PR 17 fold-in) Picked up by `Check settings schema is up-to-date` lint step (the only red CI step on `3f63ad435`). PR 17 commit 2 added `tools.disabled` to `packages/cli/src/config/settingsSchema.ts` but didn't run `npm run generate:settings-schema`, so the JSON-schema mirror used by the VSCode IDE companion drifted. Regenerating now picks up the new entry verbatim — no behavior change. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(serve): fold-in 2 — gpt-5.5 + deepseek review (#4175 Wave 4 PR 17) Addresses 3 critical / 3 suggestion items from #4282 round-2 review. Critical (gpt-5.5) - CV1 (`initWorkspace` symlink escape): the textual `withinWorkspace` check on the joined path doesn't see through symlinks. A `QWEN.md` symlink inside the workspace pointing outside it would still get followed by `fs.readFile` / `writeFile`; under `force: true` the route would truncate the external target, and a dangling symlink could create outside the workspace. Add an `lstat(target)` check before the read/write and reject when `isSymbolicLink()`. The proper long-term fix routes through PR 18's `WorkspaceFileSystem` boundary (chain-aware resolution + audit hooks); tracked under the SV2 TODO comment below. - CV2 (MCP restart timeout vs MCP discovery deadline): bridge raced against `initTimeoutMs` (10s) but `McpClientManager`'s per-server discovery deadline can be up to 5 minutes (`MAX_DISCOVERY_TIMEOUT_MS = 300_000`). A valid restart returned HTTP timeout to the client while the ACP child kept reconnecting in the background, leaving daemon and client state divergent. Add a dedicated `MCP_RESTART_TIMEOUT_MS = 300_000` constant and use it for the bridge race. The bridge race remains a safety net against a wedged ACP channel; per-server discovery deadlines stay owned by the manager. - CV3 (`disabledTools` rename ordering bug): the gate ran on `tool.name` BEFORE the MCP collision-rename branch. An MCP tool that collided with a lazy factory and got renamed via `asFullyQualifiedTool()` (e.g. `structured_output` → `mcp__rogue-server__structured_output`) bypassed the disabled set if the operator disabled the renamed-and-exposed name. Re-check `isToolDisabled` after the rename, before inserting into `this.tools`. New regression test pins the contract. Suggestion - SV1 (deepseek): cap `:name` path parameter at 256 chars so an extremely long tool name can't bloat the workspace settings file. Mirrors `MAX_CLIENT_ID_LENGTH = 128` and `MAX_WORKSPACE_PATH_LENGTH = 4096` siblings. - SV2 (deepseek): `initWorkspace` uses `node:fs/promises` directly instead of routing through `WorkspaceFileSystem`. Bridge layer doesn't have `fsFactory` plumbed today (PR 18 boundary is per-request inside `createServeApp`); a separate plumbing PR will hoist it into `BridgeOptions`. Added a FIXME pointing to that follow-up. CV1's symlink reject covers the immediate boundary-escape concern. - SV3 (gpt-5.5): the daemon stamps `originatorClientId` on the SSE envelope, but reducer snapshots stored only `event.data`. Consumers of `lastApprovalModeChange` / `lastToolToggle` / `lastWorkspaceInit` / `lastMcpRestart{,Refused}` couldn't tell whether the mutation originated from themselves. New `mergeOriginator` helper copies the envelope's `originatorClientId` onto the stored snapshot when `data.originatorClientId` is unset (the daemon does not currently populate `data.originatorClientId`, but the field exists on the Data interfaces — preserve it if a future daemon version does). 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(serve): fold-in 3 — gpt-5.5 round-3 review (#4175 Wave 4 PR 17) Addresses 2 suggestion items from #4282 round-3 review (post-rebase onto PR 21). - C7 (`docs/developers/qwen-serve-protocol.md`): protocol doc showed built-in display labels (`Bash`, `Read`, `Write`) as disable-able, but `ToolRegistry.isToolDisabled` checks the actual registered tool name. The shell tool registers as `run_shell_command`, so a `POST /workspace/tools/Bash/enable {enabled:false}` would persist + emit `tool_toggled` while the next session still registers `run_shell_command`. Updated the doc to use the canonical registry name in the example body and added a ⚠️ block explaining that names must match the registry's exposed identifier exactly. The daemon route deliberately does not alias-resolve (it accepts unknown names for forward-looking MCP pre-disable, so any alias map would be incomplete). - C8 (`packages/sdk-typescript/test/unit/daemonEvents.test.ts`): the 5 PR 17 reducer cases (`approval_mode_changed`, `tool_toggled`, `workspace_initialized`, `mcp_server_restarted`, `mcp_server_restart_refused`) had no SDK-side coverage. Added 7 tests covering happy-path counter + last-snapshot accumulation, malformed-payload rejection (rounds through `asKnownDaemonEvent → undefined` and increments `unrecognizedKnownEventCount` rather than the event-specific counter), all 3 refused-reason literals, the `noop` action literal added in fold-in 1, and the `mergeOriginator` precedence rule (data-level wins over envelope-level when both present). 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(serve): fold-in 4 — qwen-latest review (#4175 Wave 4 PR 17) Round-4 reviewer adoption (qwen-latest-series-invite-beta-v28): - C1: hoist `persistApprovalMode` guard before the ACP roundtrip so a missing callback no longer leaves the daemon's mode shifted while the caller observes a 500 (httpAcpBridge.ts). - C2: serialize `persistApprovalMode` and `persistDisabledTools` through a per-workspace promise chain (`withSettingsLock`) so concurrent toggles can't lose updates in the read-modify-write window (runQwenServe.ts). - C3: trim `toolName` before persisting in `/workspace/tools/:name/enable` so the write path matches `loadCliConfig`'s `.trim()` on read. Re-validates empty-after-trim with 400 `invalid_tool_name`. - S1: cap `serverName` at `MAX_SERVER_NAME_LENGTH=256` on `/workspace/mcp/:server/restart` for parity with the tool-toggle cap. - S2: when `persist:true` succeeds, mirror `approval_mode_changed` via `broadcastWorkspaceEvent` so peer sessions in the same workspace observe the new default before their next ACP child spawn. - S3: `'noop'` added to `FakeBridge.initWorkspaceImpl` return type. - S5: `qwen-serve-protocol.md` action enumeration now includes `'noop'` and notes how the SSE event mirrors the response action. S4 (sync IO inside async persist callbacks) is acknowledged but deferred — `loadSettings` is the project-wide read path and the H2 fold-in already restricted us to workspace-scope-only consumption, keeping the sync window bounded. Fully eliminating it requires swapping `loadSettings` to async across the CLI, which is out of scope. 7 new tests: - server.test.ts × 3: tool-name trim, whitespace-only 400, server-name 256 cap. - httpAcpBridge.test.ts × 4: pre-call guard ordering for persist:true (no callback), persist:false bypasses guard, persist:true broadcasts to peer sessions, persist:false stays session-scoped. Typecheck clean across cli / sdk-typescript / core. 1599/1599 unit tests pass. |
||
|
jinye
|
688d64416e
|
feat(serve): add workspace file write/edit routes (#4175 PR20) (#4280)
Some checks are pending
Qwen Code CI / Classify PR (push) Waiting to run
Qwen Code CI / Lint (push) Blocked by required conditions
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Blocked by required conditions
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* feat(serve): add workspace file write/edit routes Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): bind file hashes to text snapshots Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): tighten read-bytes snapshot and create-mode publish - readBytesWindow: re-stat the open fd after read and require unchanged ino+size+mtime before emitting the response. Mirrors the hardened text-snapshot path so the full-window hash can no longer pair with bytes that drifted under in-place rewrite or append. Surface drift as retryable hash_mismatch. - atomicWriteTextResolvedFile: reject a symlinked parent up-front as defense-in-depth ahead of the parent-fd publish follow-up referenced by assertInodeStableAfterRead. - atomicWriteTextResolvedFile: publish create-mode writes via link()+unlink() instead of rename(). POSIX rename() overwrites an existing regular file, so a racing external process could break the public create contract; link() returns EEXIST atomically and is portable across POSIX/NTFS. The early assertCreateTargetAbsent check stays for friendlier errors on the non-racing path. --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
jinye
|
36760ca63c
|
feat(serve): auth device-flow route (#4175 Wave 4 PR 21) (#4255)
* feat(serve): auth device-flow route
Implements issue #4175 Wave 4 PR 21. Brokers OAuth 2.0 Device
Authorization Grant (RFC 8628) through the `qwen serve` daemon so a
remote SDK client can trigger a Qwen-account login whose tokens land
on the **daemon** filesystem, not on the client. The daemon polls the
IdP itself; the client's only job is to display the verification URL +
user code.
Runtime locality (#4175 §11): the daemon NEVER spawns a browser or
calls `open(url)` — even when running locally. Static-source grep
test fails the build on `node:child_process` / `open` / `xdg-open` /
`shell.openExternal` / `execa` / `shelljs` / `process.spawn` and
their dynamic-import / require variants.
- `POST /workspace/auth/device-flow` — strict mutation gate; returns
201 fresh / 200 idempotent take-over with `attached: true`. Per
per-`providerId` singleton: a second POST while pending takes over
rather than allocating a new `device_code`.
- `GET /workspace/auth/device-flow/:id` — public state read. Pending
entries echo `userCode/verificationUri/expiresAt/intervalMs`;
terminal entries (5-min grace) drop them and surface
`status/errorKind/hint`.
- `DELETE /workspace/auth/device-flow/:id` — strict; idempotent
(terminal → 204 no-op; unknown → 404).
- `GET /workspace/auth/status` — pending flows + supported providers
snapshot. v1 stub for `providers: []` (populated in fold-in 1).
`DeviceFlowRegistry` (`packages/cli/src/serve/auth/deviceFlow.ts`)
is the in-memory state holder:
- per-`providerId` singleton with idempotent take-over
- workspace-wide cap of 4 active flows (abuse defense)
- 5-min terminal grace so SDK reconnects can still observe results
- TTL sweeper evicts grace-expired entries every 30s
- in-flight `Promise` map coalesces concurrent `start()` calls so two
parallel POSTs don't double-allocate IdP `device_code`
- `transitionTerminal` returns `boolean` so caller-side emit/audit
guard prevents sweeper × poll-tick double-fire
- `dispose()` wired into `runQwenServe.close()`'s shutdown drain;
cancels `provider.poll()` mid-flight via `cancelController`,
records `lost_success` audit when an IdP-minted token is dropped
by transition
`DeviceFlowProvider` interface accepts `start({signal})` +
`poll(state, {signal})`. `QwenOAuthDeviceFlowProvider` wraps the
existing `QwenOAuth2Client.requestDeviceAuthorization` /
`pollDeviceToken` primitives directly (NOT
`authWithQwenDeviceFlow`, which calls `open(url)`). PKCE is
provider-required by Qwen but optional in the interface for future
non-PKCE providers. `success.persist()` writes to disk FIRST, then
updates the in-process client — a failed disk write no longer
leaves the daemon with a zombie in-memory token. Maps RFC 8628
errors via an anchored regex (`^Device token poll failed:
(expired_token|access_denied|invalid_grant)`) so an
`error_description` containing one of those literals can't
mis-classify an unrelated upstream error.
`BrandedSecret<T extends string>` holds the `device_code` and PKCE
verifier. Earlier draft used `new String()` wrapper which leaked
through `+` / template literals (`Symbol.toPrimitive` →
`valueOf` returned the primitive). Final shape: frozen plain object
+ `WeakMap` indirection + 4-way redaction
(`toString` / `toJSON` / `Symbol.toPrimitive` / numeric coercion →
`'[redacted]'` or `NaN`) + `unique symbol` brand. 6 leak-path
tests: `JSON.stringify` / `String()` / concat / template / `+x` /
reveal-roundtrip.
5 new daemon events (workspace-scoped, fanned out to every active
session bus via `bridge.broadcastWorkspaceEvent`):
- `auth_device_flow_started` — `{deviceFlowId, providerId, expiresAt}`
(no userCode/verificationUri — see PR 21 design §3)
- `auth_device_flow_throttled` — `{deviceFlowId, intervalMs}`,
emitted only on upstream `slow_down` interval bumps
- `auth_device_flow_authorized` — `{deviceFlowId, providerId,
expiresAt?, accountAlias?}`; `accountAlias` is best-effort
non-PII (never email/phone)
- `auth_device_flow_failed` — `{deviceFlowId, errorKind, hint?}`
with `errorKind ∈ {expired_token, access_denied, invalid_grant,
upstream_error, persist_failed}`
- `auth_device_flow_cancelled` — `{deviceFlowId}` (DELETE on pending)
Workspace-scoped reducer `reduceDaemonAuthEvent` produces
`DaemonAuthState { flows: Partial<Record<ProviderId, ...>> }` —
parallel to `reduceDaemonSessionEvent`. Session reducer no-ops on
auth events (workspace-scoped state belongs in its own reducer).
`bridge.broadcastWorkspaceEvent` is intentionally distinct from PR
16's `publishWorkspaceEvent` to avoid merge conflict; collapses to
the shared helper as a fold-in once #4249 lands (~25 LoC).
`@qwen-code/sdk` (`packages/sdk-typescript/`):
- 4 new `DaemonClient` methods: `startDeviceFlow`, `getDeviceFlow`,
`cancelDeviceFlow`, `getAuthStatus` — typed against the wire
shapes, errors mapped through the existing `DaemonHttpError`.
- High-level `client.auth` getter (lazy `DaemonAuthFlow` singleton)
exposes a `start(...).awaitCompletion()` shape mirroring `gh auth
login`'s UX: print code first, let the SDK consumer decide where
to open the browser. `awaitCompletion` polls GET on the
daemon-supplied `intervalMs`, honors `slow_down` bumps, and
fall-back-recovers from 404 (entry evicted post-grace).
POST + DELETE flow through PR 15's `mutate({strict: true})` —
401 `token_required` on token-less loopback defaults. GET routes
use only the global `bearerAuth`. Every state transition
(`started/authorized/failed/cancelled/expired/lost_success`)
records a structured stderr breadcrumb (`[serve] auth.device-flow:
provider=... deviceFlowId=abc12... clientId=... status=...`)
since `mutate()` doesn't carry an audit hook — events alone aren't
enough since SDK can silently drop them; stderr → journald/docker
logs is the unfalsifiable record.
`auth_device_flow` advertised unconditionally on
`/capabilities.features`. Supported providers list lives on
`/workspace/auth/status` to keep the registry descriptor uniform.
- `packages/core/src/qwen/qwenOAuth2.ts`:
- exports `cacheQwenCredentials` (was a private function; needed
by the daemon's device-flow registry)
- `cacheQwenCredentials` now calls `SharedTokenManager.clearCache()`
after writing, folding what was previously a paired call site at
L820+L829. Idempotent change.
- file mode `0o600` on `oauth_creds.json` (was default 0o666 +
umask). Mirrors opencode's `auth/index.ts`.
- `packages/cli/src/serve/runQwenServe.ts`: device-flow registry
`dispose()` wired into the shutdown drain (BEFORE
`bridge.shutdown()`).
- `auth/deviceFlow.test.ts` — 21 tests: BrandedSecret leak paths,
state machine (slow_down / success / error), terminal grace,
concurrent-start coalescing, dispose, cancel idempotency, static-
source grep against browser-spawn primitives.
- `server.test.ts` — 10 device-flow integration tests:
POST 201/200 take-over, strict 401, 400 `unsupported_provider`,
GET / DELETE / `/workspace/auth/status`, 502 `upstream_error`
mapping, sweeper-driven auto-expiry with controlled clock,
capability advertisement.
- `daemonEvents.test.ts` — 5 SDK reducer tests: type guards, per-
provider state projection, `failed` always → `status: 'error'`
(errorKind carries the kind, including new `persist_failed`),
session reducer no-ops on auth events.
369/369 serve + SDK tests pass; typecheck + `eslint
--max-warnings 0` clean across 14 PR 21 files.
- [x] Independently mergeable (depends only on merged PR 4 / PR 7 /
PR 12 / PR 15)
- [x] Backward compatible (4 new routes + 1 capability tag + 5 typed
events + 4 SDK helpers; existing routes/events untouched)
- [x] Default off (capability advertised but no client is forced to
use it; CLI `qwen` OAuth flow unchanged)
- [x] `qwen serve` Stage 1 routes / SDK behavior preserved
- [x] Gradual migration (v1 only `qwen-oauth`; future providers
register through the `DeviceFlowProvider` interface)
- [x] Reversible (revert removes 4 routes + 1 tag + 5 events with no
schema migration)
- [x] Tests-first (28 new tests across 3 layers)
- Inline `bridge.broadcastWorkspaceEvent` → fold-in to PR 16 (#4249)
`publishWorkspaceEvent` once that lands
- `/workspace/auth/status` vs PR 12 `/workspace/providers` boundary
— separate route in v1; merge alternative discussed
- Wave 4 PRs 17/19/20 should adopt the same mutate-strict +
workspace event-fan-out pattern
5 items from pre-PR specialist passes parked for a focused
follow-up: `DeviceFlowEntry` discriminated union, single-source SDK
status / ProviderId unions, `awaitCompletion` memoization,
broadcast-100%-fail stderr elevation, SDK 404 →
`not_found_or_evicted` errorKind.
Refs: #4175
🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
* fixup(serve): address PR #4255 round-1 review feedback
Eleven items from copilot-pull-request-reviewer's round-1 pass on
#4255 — 4 inline threads + 7 from the PR-level review summary.
## Adopted (11 items, code/doc changes)
- **`lastSeenAt` → `lastSeenEventId`** (`events.ts`,
`DaemonDeviceFlowReducerState`). The field was set from
`rawEvent.id` (SSE event id) but documented as "epoch ms" — a real
semantic mismatch that would mislead consumers into time-based
logic against a monotonic counter. Rename + tighten the JSDoc to
describe it as an event-id counter; reducer cases updated.
- **`DEVICE_FLOW_EXPIRY_GRACE_MS = 30_000` extracted** in
`DaemonAuthFlow.ts` (was a magic number on `start.expiresAt +
30_000`). `AwaitCompletionOptions.timeoutMs` doc now describes the
actual grace-past-expiry behavior + the rationale (clock skew +
daemon sweeper interval + network latency) instead of the wrong
"defaults to expiresAt - Date.now()" claim.
- **Explicit `chmod 0o600`** in `cacheQwenCredentials` after every
write. `fs.writeFile`'s `mode` only applies on file creation; a
pre-existing `oauth_creds.json` written under a broader umask kept
its old permissions across upgrades. The chmod now tightens it on
every write; chmod failure (Windows / hardened FS) surfaces via
`debugLogger.warn` instead of silently dropping the invariant.
- **`SharedTokenManager.clearCache()` failure now logs**
`debugLogger.warn` (was a silent `try { } catch { }`). In
production a swallowed clearCache means in-process callers serve
stale credentials until the SharedTokenManager mtime watcher
catches up — a recoverable degradation worth a log line.
- **Protocol doc** lists `persist_failed` in the
`auth_device_flow_failed.errorKind` union (was added to the type
but missed in the doc).
- **`pollDeviceToken({signal})`** plumbed through
`IQwenOAuth2Client` interface + `QwenOAuth2Client` impl + the Qwen
device-flow provider. Cancel / dispose during a slow IdP response
now aborts the in-flight HTTP socket immediately instead of
waiting for the upstream timeout. Two new registry tests assert
`cancel()` / `dispose()` propagate abort to the signal observed by
`provider.poll`.
- **`revealSecret` error message** clarified: was "secret has been
GC-evicted" (impossible — WeakMap doesn't evict reachable keys).
Now points at the actual reachable failure modes (forged shape /
serialize+reparse losing the WeakMap binding).
- **`transitionTerminal` JSDoc** clarifies that the PRIMARY guard
against late timer secret leaks is the `entry.status !== 'pending'`
check at the top of `runPollTick`; secret-clearing here is
defense-in-depth.
- **`DeviceFlowErrorKind` JSDoc'd per variant** so consumers can tell
when each fires (RFC 8628 distinctions + `persist_failed` vs
`upstream_error` boundary).
- **Stale "PR 16 / PR 21 §3" temporal references** in
`DaemonAuthFlow.ts:124` rephrased to be timeless ("workspace-scoped
events fan out through whatever session buses happen to be live"
— no PR number references that rot when those PRs merge).
## Not adopted (4 items, replied to in-thread)
- **`authWithQwenDeviceFlow` browser-launch separation** — correct
architectural advice but out of #4255 scope (would refactor a CLI
auth UX module that PR 21 only touched additively). Tracked as a
Wave 5 follow-up.
- **Copyright header year range** — repo-wide convention "2025"; not
introduced by this PR.
- **Spread `...(x ? {x} : {})` → `x: x ?? undefined`** — the two are
not semantically equivalent. The current form omits the key
entirely on falsy `x`; the suggested form always includes the key.
Tests assert object shape and would break under the change.
- **Eager `client.auth` getter** — public API boundary. Lazy
construction matches `DaemonSessionClient` precedent + saves the
module load for SDK consumers that never touch auth.
Refs: #4175 #4255
🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
* fixup(serve): address PR #4255 wenshao round-1 review feedback
15 items from @wenshao's review batches on #4255. Catches a handful
of real bugs that the earlier round (commit 3d9f082f5) didn't
surface.
## Critical fixes
- **C1 — `pollUntilTerminal` providerId pass-through**
(`DaemonAuthFlow.ts:185`). The synthetic 404 fallback hardcoded
`providerId: 'qwen-oauth'`; the parent `awaitCompletion` already
receives the real providerId via `start.providerId` but
`pollUntilTerminal`'s parameter type stripped it. Add the field to
the param type, propagate.
- **C2 — open `errorKind` allowlist** (`events.ts`). The closed
5-value union in the type guard silently dropped any `failed`
event whose errorKind the daemon added without mirroring SDK-side
(e.g. a future `rate_limited`). The flow's reducer state would
never transition to terminal, leaving SDK consumers stuck on
`pending` forever. Open the union with `(string & {})` and accept
any non-empty string in the runtime guard. Updated test asserts
forward-compat behavior + still rejects the truly-malformed
empty-string case.
- **C3 — `persist()` timeout + signal**
(`deviceFlow.ts`). A wedged disk I/O (NFS stall, encrypted-volume
contention) without bounds would pin the entry in `pending` until
the upstream `expires_in` elapsed (potentially minutes). The
registry now passes its `cancelController.signal` AND arms a hard
`DEVICE_FLOW_PERSIST_TIMEOUT_MS = 30_000` timer; persist failure
surfaces as `persist_failed` immediately. The
`DeviceFlowPollResult` `success` variant signature changed to
`persist({signal})`.
- **C4 — cancel × success race rollback**
(`deviceFlow.ts` + Qwen provider). Today, if `cancel()`
transitions while `persist()` is in flight, the credentials get
written but the flow's status is `cancelled`. User sees cancelled,
daemon disk has a valid token. `DeviceFlowPollResult.success`
gains an optional `unpersist()` callback the registry calls when
`transitionTerminal(authorized)` fails — the Qwen provider wires
it to `clearQwenCredentials()`. Rollback failure is audited but
not propagated (re-running auth would overwrite anyway).
- **C5 — don't `unref()` the `awaitCompletion` sleep timer**
(`DaemonAuthFlow.ts`). On a standalone Node CLI/script doing just
`client.auth.start().awaitCompletion()`, the unref'd between-poll
timer was the only event-loop handle, so Node could exit before
the user finished authorization. The poll wait is foreground work
the caller explicitly awaits — keep it ref'd.
## Information-leak fixes
- **S1 — sanitize `persist_failed` hint**. `err.message` from
`cacheQwenCredentials` embeds the full `~/.qwen/oauth_creds.json`
path. Broadcast via SSE, that path leaks the daemon's home layout
to every connected session subscriber. Replace user-facing hint
with `"credentials could not be written to the daemon filesystem
— check disk space and permissions"`; full err goes to stderr
audit only.
- **S2 — sanitize upstream `pollDeviceToken` hint**. The class
embedded the entire raw IdP response body (which can be an HTML
error page from a reverse proxy) into the thrown message. Same
broadcast leak path. Replace upstream-error hint with
`"unexpected response from identity provider"`; RFC 8628 errors
use `"Qwen IdP returned ${kind}"`.
## Cleanup / forward-compat
- **D1 — drop duplicate `clearCache()`** at `qwenOAuth2.ts:840`. The
paired call became redundant once `cacheQwenCredentials` folded
the clearCache in (PR #4255 fold-in 1). The fold-in 1 message
said this would be done; the duplicate slipped through.
- **S3 — drop unused `DeviceFlowNotFoundError`** (`deviceFlow.ts`).
Exported but never imported; route handlers do inline 404 JSON.
- **S4 — single-source SDK status / errorKind unions**
(`types.ts`). `DaemonAuthDeviceFlowSdkStatus` /
`DaemonAuthDeviceFlowSdkErrorKind` were parallel literal copies
of the canonical events.ts definitions — drift waiting to happen.
Now imported + aliased as type-only re-exports.
- **S5 — broadcast 100% fail elevates to stderr**
(`httpAcpBridge.ts`). Per-session bus failures stay debug-only,
but a broadcast where EVERY session bus refused is operationally
interesting (clients won't see the event). Track success / fail
counts; `writeStderrLine` when `successCount === 0`.
- **S6 — `this.disposed` check after `await provider.start()`**
(`deviceFlow.ts`). `dispose()` mid-start would orphan the freshly-
inserted entry (`schedulePoll` guards on `disposed` so no poll
fires; the entry never transitions). Throw post-await if disposed.
- **W1 — thread `signal` into `requestDeviceAuthorization`**
(`qwenOAuth2.ts` + Qwen provider). `start()` had the same
cancellation gap that `pollDeviceToken` had — a slow
device-authorization request couldn't be aborted during shutdown.
Now plumbed end-to-end.
- **W2 — split `invalid_request` from `unsupported_provider`**
(`server.ts`). Conflating them surfaced misleading remediation
hints to SDK consumers branching on `code` ("this provider isn't
supported here" when the real cause was a serializer dropping the
field). Bad-shape now returns `code: 'invalid_request'`;
unknown-but-well-formed stays `unsupported_provider`.
- **W3 — drop never-populated `accountAlias`**
(Qwen provider). The field was wired through types / events /
reducer / audit but the Qwen IdP's token response doesn't carry
one (no `name` / `email` / `sub`). Returning only `{expiresAt}`
makes the field type-honestly absent rather than always-undefined.
Future provider with an alias-bearing response can populate it.
- **W4 — `DaemonAuthFlow` JSDoc accuracy**. Doc claimed "first
attempts to consume an SSE event stream … falls back to GET-based
polling"; actual is GET-only with SSE as a real-time hint for
clients already subscribed to a session stream.
- **W5 — clearer unit arithmetic** in interval normalization. The
`(_INTERVAL_MS / 1000) * 1000` cancelation hid the s↔ms boundary;
expanded form makes both branches unit-explicit.
## Test changes
- `daemonEvents.test.ts` updated to match the now-OPEN errorKind
union (forward-compat assertion + empty-string still rejected).
- `deviceFlow.test.ts` `FakeProvider.poll` aligned with the new
`persist({signal})` signature + optional `unpersist`.
## Validation
- `npm run typecheck --workspace packages/cli --workspace
packages/sdk-typescript --workspace packages/core` — clean
- `npx vitest run packages/cli/src/serve/
packages/sdk-typescript/test/unit/daemonEvents.test.ts` — 368/368
- `npx eslint --max-warnings 0` over the 11 PR 21 surface files —
clean
Refs: #4175 #4255
🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
* fixup(serve): address PR #4255 wenshao round-2 review feedback
10 new threads from @wenshao's second deep-review pass on #4255.
Verified status: 5 real issues, 1 improvement, 3 stale (already
fixed; comments lagged), 1 false alarm (typecheck demonstrably
clean).
## Critical fixes
- **fold-in 2 C4 REVERSED**: when `provider.poll()` returns success
AND `cancel()` / `dispose()` transitioned the entry mid-`persist()`,
the registry now FORCES the entry to `authorized` and keeps the
on-disk credentials. The earlier rollback (`unpersist()`) wasted
the user's IdP approval because the RFC 8628 `device_code` is
single-use — re-running the flow would force them through the
whole browser-prompt + paste-code dance again for a click whose
intent was likely "stop the wait" rather than "undo my already-
completed approval". Aligns with gh CLI / Auth0 SDK / git-
credential-manager. Audit captures the race via `hint:
'lost_success_kept ...'`. `DeviceFlowPollResult.success.unpersist`
field + Qwen provider's `clearQwenCredentials` rollback removed.
- **#1 GET /workspace/auth/device-flow/:id strict gate**: this GET
surfaces `userCode` / `verificationUri` for pending entries, which
on the loopback no-token default were readable by any local
process. POST + DELETE were already strict; aligning GET closes
the information-disclosure asymmetry. `/workspace/auth/status`
stays bearer-only (its `pendingDeviceFlows` entries intentionally
omit `userCode`).
- **#2 `inFlightStarts` hard timeout**: a hung `provider.start()`
(network partition, unresponsive IdP) used to leave the per-
`providerId` slot in `inFlightStarts` occupied forever, blocking
every subsequent POST until daemon restart. New
`DEVICE_FLOW_START_TIMEOUT_MS = 30_000` arms a timer that
`cancelController.abort()`s the start; the rejected promise
unwinds through the `try/finally` clearing the slot.
- **#10 chain-completing the C3 persist-timeout**: the earlier C3
fix armed a 30s timer that fired `cancelController.abort()` then
`await result.persist({signal})`, but the chain ended at the
registry boundary — `cacheQwenCredentials` didn't take a signal,
so `fs.writeFile` couldn't be aborted. Now `cacheQwenCredentials`
accepts an optional `{signal}` and threads it into
`fs.writeFile(..., {signal})` (Node native). The Qwen provider's
`persist({signal})` forwards the entry's
`cancelController.signal` end-to-end.
## Improvement (#4): 404 fallback errorKind
`pollUntilTerminal`'s 404 catch used to synthesize
`{status: 'expired'}` for ALL evicted entries — conflating "your
flow expired during your disconnect", "the daemon was restarted",
and "your deviceFlowId was wrong". Now returns
`status: 'error'` + `errorKind: 'not_found_or_evicted'` + a `hint`
so SDK consumers branching on errorKind can distinguish.
## Information leak (#9): start() path raw IdP message
S2 (fold-in 2) sanitized `poll()`'s upstream-error hint, but
`start()` still embedded the raw `err.message` (full IdP response,
potentially HTML from a reverse proxy / WAF) into the
`UpstreamDeviceFlowError` that flowed to SDK clients via the 502.
Now uses static messages for the SDK-visible errors; raw detail
goes through `writeStderrLine` for operator audit only. Mirrors
S2's approach.
## Stale comments cleaned (#5, #7)
`qwenDeviceFlowProvider.ts:177` claimed
`cacheQwenCredentials` "doesn't currently take a signal — that's
a follow-up". After #10 above, that's no longer true; the comment
is replaced with the actual end-to-end signal-threading note.
## Not adopted (1 false alarm)
- Thread on `types.ts:330` claimed type-only-import-after-
declarations breaks `tsc` and fails `daemonEvents.test.ts:670`
with TS2345. Demonstrably false: `npx tsc -p
packages/sdk-typescript/tsconfig.json --noEmit` exits 0;
`daemonEvents.test.ts` is the post-fold-in-2 file with the
open-allowlist assertion (test 28/28 passes). The reviewer may
have been looking at a transient state during their analysis.
## Validation
- `npm run typecheck --workspace packages/cli --workspace
packages/sdk-typescript --workspace packages/core` — clean
- `npx vitest run packages/cli/src/serve/
packages/sdk-typescript/test/unit/daemonEvents.test.ts` — 398/398
pass
- `npx eslint --max-warnings 0` over the PR 21 surface — clean
Refs: #4175 #4255
🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
* fixup(serve): address PR #4255 wenshao round-3 review feedback
5 new threads from the third deep-review pass on #4255. 3 real
issues fixed; 1 stale (already done in fold-in 3); 1 deferred as
non-blocking design suggestion.
- **A — `expiresIn` / `interval` non-finite guard**
(`deviceFlow.ts`). The provider contract types both as `number`,
but a misbehaving / future provider could hand `undefined` /
`NaN` / `Infinity`. `Math.max(0, NaN) * 1000` is `NaN`, then
`now() + NaN` is `NaN`, then `now >= NaN` is always `false` —
the sweeper would NEVER evict the entry, pinning an upstream
`device_code` slot until daemon restart. Same hazard on
`interval * 1000` (NaN → `setTimeout(NaN)` fires immediately,
Infinity → scheduler clamps to TIMEOUT_MAX). Now both fields go
through `Number.isFinite(x) && x > 0`; missing/bad values fall
back to RFC 8628's recommended ceilings (10 min for expiry, 5s
for interval).
- **D — typed `app.locals` accessor**
(`deviceFlow.ts` + writer/reader call sites). The
`app.locals['deviceFlowRegistry']` string key was shared between
`createServeApp` (writer) and `runQwenServe` (reader); a typo on
either side would compile cleanly and the shutdown dispose call
would silently no-op, leaving polling timers running until the
`unref()` rescue. New `setDeviceFlowRegistry(app, registry)` /
`getDeviceFlowRegistry(app)` pair gives both call sites
type-checked access; the string literal is encapsulated in one
module.
- **E — `UnsupportedDeviceFlowProviderError` docstring**
(`deviceFlow.ts`). After fold-in 2's W2 fix split
`invalid_request` from `unsupported_provider`, the route layer
screens unknown ids against `DEVICE_FLOW_SUPPORTED_PROVIDERS`
before reaching the registry — so this error is now reachable
ONLY on a daemon-internal invariant violation (id is declared
supported but not registered in the runtime provider map).
Docstring + thrown message updated to reflect that this branch
signals a programmer error, not user input.
- **B** claimed `cacheQwenCredentials(credentials)` doesn't forward
signal to `fs.writeFile`. Verified: fold-in 3 (#10) at
`qwenDeviceFlowProvider.ts:204` calls
`cacheQwenCredentials(credentials, { signal: persistOpts.signal })`
and the core helper threads it into `fs.writeFile(..., {mode,
signal})`. The reviewer was looking at the comment block above
(lines 174-181) without scrolling to the actual call site.
- **C — SDK `cancelDeviceFlow` lossy 204/404 collapse**.
Suggested returning `{existed: boolean; alreadyTerminal: boolean}`
instead of resolving void on both 204 and 404. Real signal-loss
but tagged "[非阻塞]" by the reviewer; changing requires a
daemon route shape change (200 + body instead of 204) which is
better as a focused follow-up PR. Acknowledged in-thread;
deferred to a fold-in PR after #4255 lands.
- `npm run typecheck` — clean across `packages/{cli,sdk-typescript,core}`
- `npx vitest run packages/cli/src/serve/
packages/sdk-typescript/test/unit/daemonEvents.test.ts` — 398/398
- `npx eslint --max-warnings 0` over the PR 21 surface — clean
Refs: #4175 #4255
🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
* fixup(serve): address PR #4255 wenshao round-4 review feedback
4 threads from the fourth review pass on #4255. 3 adopted + 1
deferred (out-of-scope rename of PR 15's `mutate` helper).
## Adopted
### #1 — `persistInFlight` flag suppresses cancel × persist event-stream UX trap
When `provider.poll()` returns success and we await `persist()`, a
concurrent `cancel()` would synchronously transition the entry to
`cancelled` and emit `auth_device_flow_cancelled` — then `persist()`
resolves and (per fold-in 3 C4) force-overrides to `authorized` +
emits `auth_device_flow_authorized`. The reducer state correctly
last-write-wins on `authorized`, but DIRECT event-stream consumers
(close-dialog handlers, telemetry, UI cleanup) race onto an unmounted
UI when the second event lands.
Now: while persist is in-flight, `cancel()` and the sweeper SKIP the
state transition + event emit. They register intent (set
`cancelRequestedDuringPersist=true` for cancel; sweeper just no-ops)
and let the persist resolution decide:
- persist succeeds → `authorized` (IdP wins per fold-in 3 C4)
- persist fails AND cancel was requested → `cancelled`
- persist fails AND `now >= expiresAt` → `expired` / `expired_token`
- persist fails otherwise → `error` / `persist_failed`
Result: at most one terminal event per flow. Imperative SSE
consumers no longer see oscillating terminal states. Audit captures
the race (`hint: 'lost_success_kept ...'`) for incident-response
correlation.
### #2 — `revealSecret` → `unsafeRevealSecret` rename
The earlier JSDoc claimed "the `unsafeReveal_` naming is intentional:
greppable in code review, easy to allowlist in lint rules, hard to
invoke by accident" — but the actual function was named
`revealSecret`. The promised safety properties didn't exist; a code
reviewer wouldn't single out `revealSecret` as suspicious, and a
`no-restricted-syntax` ESLint rule wouldn't flag it.
Renamed to `unsafeRevealSecret` so the JSDoc-promised "greppable" /
"lintable" property is now actually true. Two call sites in the
Qwen provider + 4 test references updated. Internal symbol; not
exposed through the SDK package.
### #4 — `QwenOAuthPollError` typed class replaces substring regex
The earlier RFC 8628 error mapper used an anchored regex against the
thrown error message text — an implicit cross-file string contract
between `qwenOAuth2.ts` (throws) and `qwenDeviceFlowProvider.ts`
(matches). If `qwenOAuth2.ts` ever changed its message format, ALL
RFC 8628 errors (`expired_token` / `access_denied` / `invalid_grant`)
would silently fall through to `upstream_error` — wrong errorKind
flowing through telemetry with no test or type-system check to catch
the drift.
Now `QwenOAuth2Client.pollDeviceToken` throws a structured
`QwenOAuthPollError extends Error` with `oauthError` / `description`
/ `status` fields. The provider branches on `instanceof
QwenOAuthPollError` and reads `.oauthError` directly via a
dedicated `mapRfc8628OAuthCode(code)` switch. The drift hazard is
gone: a future code change that touches the typed class will
fail tsc until both sides are updated. Message format preserved
for any pre-existing log-parsing / substring matchers.
## Not adopted
### #3 — `mutate({strict:true})` semantic awkwardness on GET
Reviewer correctly noted that `mutate` is named for state-changing
routes, but `GET /workspace/auth/device-flow/:id` uses it for an
information-disclosure defense (only reachable code path is reading
state). Suggested rename: `mutate` → `strictHttpGate`.
Deferred: the rename touches PR 15's helper which has many call
sites in `server.ts` and is shared infrastructure for Wave 4 PRs
17/19/20. PR 21 is the first / only consumer of the strict-on-GET
form so far; widening the rename to a Wave 4 follow-up keeps the
fold-in scope tight. Replied in-thread.
## Validation
- `npm run typecheck` — clean across `packages/{cli,sdk-typescript,core}`
- `npx vitest run packages/cli/src/serve/
packages/sdk-typescript/test/unit/daemonEvents.test.ts` — 544/544
- `npx eslint --max-warnings 0` over the PR 21 surface — clean
Refs: #4175 #4255
🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
* fixup(serve): address PR #4255 wenshao round-5 review feedback
Five small adopt items from the round-5 review pass; one stale thread
already addressed in
|
||
|
jinye
|
103090669e
|
feat(serve): workspace memory and agents CRUD (#4175 Wave 4 PR 16) (#4249)
* feat(serve): workspace memory and agents CRUD (#4175 Wave 4 PR 16)
Adds the first Wave 4 mutation route surface: workspace-scoped memory
and subagent CRUD over HTTP. Remote clients (TUI / channels / web /
IDE adapters) can now list, read, create, update, and delete subagent
definitions and read / append / replace QWEN.md without disturbing
session state.
Routes:
- GET /workspace/memory (read-only snapshot)
- POST /workspace/memory (append/replace, strict-gated)
- GET /workspace/agents (list project + user + builtin)
- POST /workspace/agents (create-only; 409 on collision)
- GET /workspace/agents/:agentType (full detail incl. systemPrompt)
- POST /workspace/agents/:agentType (update; 403 read-only on builtin)
- DELETE /workspace/agents/:agentType (idempotent for SDK callers)
Mutation paths use mutate({ strict: true }) from PR 15 so they refuse
unauthenticated requests even on no-token loopback defaults. Workspace
mutations validate X-Qwen-Client-Id against bridge.knownClientIds() and
stamp originatorClientId on emitted events.
Capability tags added: workspace_memory, workspace_agents.
New typed events fanned out via bridge.publishWorkspaceEvent (best-
effort to every active session bus; read-after-write is the contract):
- memory_changed { scope, filePath, mode, bytesWritten }
- agent_changed { change, name, level }
writeContextFile.ts is the new core helper that resolves
QWEN.md placement (workspace vs ~/.qwen) and append-vs-replace
semantics. Whitespace-only appends short-circuit before fs.writeFile,
so a no-op POST does not bump mtime or fan out a misleading event.
SubagentManager is wrapped with a CRUD-scoped Config stub via Proxy:
only getSdkMode / getProjectRoot / getActiveExtensions are stubbed
(verified against subagent-manager.ts; getToolRegistry is execution-
path only). Any future Config method touched on a CRUD path throws
immediately so dependency creep is visible.
Auto-memory CRUD, persistent audit log, and the EACCES → NOT_FOUND
unlink mapping in core SubagentManager.deleteSubagent are explicit
follow-ups (PR 16.5 / PR 24 / separate fix).
Validation:
- typecheck: cli + sdk-typescript clean
- vitest: serve 348/348, writeContextFile 10/10, SDK 335/335
- eslint: clean
* fix(serve): address Codex P2 review on PR 16 (#4175 Wave 4 PR 16 follow-up)
Three correctness issues Codex flagged on the just-shipped workspace
memory + agents CRUD surface:
1. Concurrent POST /workspace/memory append no longer loses writes.
Two simultaneous appends would each read the same existing file,
compose new content in JS memory, then race the fs.writeFile —
the later write silently overwrote the earlier appended entry.
Add a per-resolved-path Mutex map (mirroring jsonl-utils.ts's
fileLocks pattern) and wrap the entire read-compose-write
sequence in runExclusive.
2. GET /workspace/agents now reflects out-of-band file changes.
SubagentManager.listSubagents() default served the in-memory cache;
developer / IDE adapter edits to .qwen/agents/*.md never appeared
even though GET /workspace/agents/:agentType always reads disk.
Pass { force: true } so the LIST route walks disk every call,
matching the detail route's "filesystem is the source of truth"
contract.
3. Reject builtin agent names on POST /workspace/agents to prevent
undeleteable shadow files. A client could write a project-level
agent named "general-purpose" — list/load resolved the shadow
first, but SubagentManager.deleteSubagent's name-based builtin
guard (subagent-manager.ts:302) rejected DELETE forever. Add a
BuiltinAgentRegistry.isBuiltinAgent check in parseAgentConfig
so the conflict surfaces at create time instead of trapping the
file beyond the API. The check is case-insensitive, matching the
resolver's case-insensitive cascade.
New tests:
- writeContextFile.test.ts: 10 parallel appends, all 10 entries
must survive in the final file (would fail without the mutex).
- workspaceAgents.test.ts: GET /workspace/agents observes a
freshly-written agent file on the second call (force-refresh
proof); POST with name="general-purpose" returns 422 + the
case-insensitive variant "explore" too.
Validation:
- typecheck: cli + sdk-typescript clean
- vitest: serve 351/351 (was 348, +3 new), writeContextFile 11/11
- eslint: clean
* fix(serve): apply round-1 review fold-in 2a (HIGH + CodeQL) on PR 16
Round-1 inline review (#4249) flagged ~28 items across Copilot,
wenshao, and CodeQL. This commit lands the HIGH-severity correctness
fixes plus the two CodeQL polynomial-regex warnings.
Validation tighten — `parseAgentConfig` + `parseAgentUpdates`:
- Trim leading/trailing whitespace on `name` before passing to
SubagentManager. `" tester "` would otherwise create a frontmatter
name with spaces that case-insensitive lookups can never find.
- Fail-closed (422 invalid_config) on present-but-wrong-type optional
scalars: `model`, `color`, `approvalMode`, `background`. Previously
malformed values silently dropped through validation, masking
client-serialization bugs.
- Validate `approvalMode` against the `APPROVAL_MODES` enum on both
create and update; an unknown value used to 201 with the field
silently omitted from the saved file.
- `runConfig` is now whitelist-sanitized to `{ max_time_minutes,
max_turns }` only; unknown keys are dropped, malformed values
return 422. Previously the whole input object was persisted
verbatim into YAML frontmatter.
- `?scope=` query is fail-closed for repeated values
(`?scope=workspace&scope=global`) — Express parses these as arrays
which the previous `typeof === 'string'` check silently treated as
absent, broadening DELETE/UPDATE semantics from one level to both.
- Empty update body returns 400 invalid_config (previously rewrote
the file + emitted a misleading `agent_changed` event).
- No-op updates (every supplied field already matches `existing`)
return 200 + `changed: false` and SKIP the file rewrite + event
fan-out.
Memory write helper — `writeContextFile.ts`:
- Move whitespace-only no-op detection BEFORE `fs.mkdir`. Without
this, an empty POST still created the parent directory and bumped
its mtime even though `changed: false` was reported.
- Replace two polynomial regex patterns flagged by CodeQL
(`/^\s+|\s+$/g` and `/^\n+|\n+$/g`) with hand-rolled `while` loops.
Same pattern auth.ts:120-125 already uses for the same CodeQL rule.
SDK — `DaemonClient.ts` + `types.ts`:
- `DaemonWriteMemoryResult` gains optional `changed?: boolean` so
typed callers can suppress redundant cache invalidation on no-op
appends. Optional for forward-compat with daemons that predate the
field — undefined treats as "changed: true" (legacy contract).
- `deleteWorkspaceAgent` only swallows 404 when the body's `code`
is `agent_not_found`. A bare 404 (older daemon, misrouted proxy,
generic gateway page) now throws — previously the SDK silently
reported success even when the request never reached a route that
understands workspace agents.
- `updateWorkspaceAgent` adds an optional `scope` parameter
mirroring `deleteWorkspaceAgent`, so callers can target the user-
level definition when a project-level agent shadows it.
Validation:
- typecheck: cli + sdk-typescript clean
- vitest: serve 357/357 + writeContextFile 12/12 = 369/369 passing
(was 362; +7 new)
- eslint: clean
Explicitly NOT applying (out of scope per issue #4175 PR 16
review-resolution policy):
- Copilot's "strict gate after body parser" finding — already
documented as PR 15 review-resolved tradeoff at auth.ts:256-269.
* fix(serve): apply round-1 review fold-in 2b (MEDIUM + tests) on PR 16
MEDIUM hardening:
- Fix the JSDoc on `collectWorkspaceMemoryStatus` to match the
workspace-root-only discovery the implementation actually does
today. The 32-iteration upward walk is reserved for a future
hierarchical mode but breaks after iteration 1 in v1.
- Lower the depth limit on `walkWorkspaceForMemory` from 32 → 12.
Realistic project depth sits well below 8; 12 leaves headroom
without amplifying blast radius from symlink cycles.
- Daemon `Config` Proxy now defines a `has` trap symmetric to the
existing `get` trap. Without it, a future SubagentManager path
doing `'someMethod' in this.config` would silently get `false` and
bypass the safety net the throw-on-unknown-property design
installed.
- Preflight `manager.loadSubagent(name, level)` before
`manager.createSubagent`. The default-path collision check inside
SubagentManager would otherwise miss same-frontmatter-name +
different-filename collisions; the preflight makes 409
agent_already_exists deterministic.
- Multi-level DELETE now emits one `agent_changed` event per level
that actually had a file removed. Previously an unscoped DELETE
removing both project and user shadows would publish only one
event with one level — misleading subscribers using event metadata
for toasts / audit / echo-suppression.
Test additions (covers the new event types + bridge fan-out + SDK
helpers):
- `daemonEvents.test.ts`: predicate narrowing for `memory_changed` /
`agent_changed` (rejects malformed scope/mode/level), reducer
records `lastWorkspaceMutation` + `lastWorkspaceMutationType` with
latest-event-wins semantics and stays non-terminal.
- `httpAcpBridge.test.ts`: `publishWorkspaceEvent` fans out to every
active session bus; `knownClientIds()` aggregates clientIds across
sessions and the returned set is a snapshot (mutating it does not
affect future calls).
- `workspaceAgents.test.ts`: success-path test stamping
`originatorClientId` on the create / update / delete events for a
known client.
- `DaemonClient.test.ts`: 7 round-trip tests for the new SDK helpers
(workspaceMemory, writeWorkspaceMemory, listWorkspaceAgents,
getWorkspaceAgent, createWorkspaceAgent, updateWorkspaceAgent with
scope query, deleteWorkspaceAgent: 204 / structured 404 / bare 404
triage).
- `writeContextFile.test.ts`: replace the 30ms-mtime test with a
`vi.spyOn(fs, 'writeFile')` assertion that the no-op path never
invokes writeFile. Deterministic on every filesystem.
Validation:
- typecheck: cli + sdk-typescript clean
- vitest: serve 363/363 + writeContextFile 12/12 + SDK 347/347
- eslint: clean
Reviewer guide: combined with fold-in 2a (commit
|
||
|
jinye
|
96219924a0
|
feat(serve): MCP client guardrails (#4175 Wave 3 PR 14) (#4247)
* feat(serve): MCP client guardrails (#4175 Wave 3 PR 14) Adds an in-process MCP client counter, slot-reservation enforcement at all 3 spawn sites (discoverAllMcpTools / discoverAllMcpToolsIncremental / readResource), new `--mcp-client-budget=N` + `--mcp-budget-mode={enforce,warn,off}` CLI flags forwarded to the ACP child via env, and additive `clientCount` / `clientBudget` / `budgetMode` / `budgets[]` fields plus `disabledReason: 'budget'` tagging on `GET /workspace/mcp`. Always-on capability tag `mcp_guardrails` with `modes: ['warn', 'enforce']` so SDK clients can pre-flight refusal semantics. Typed SSE push events (`mcp_budget_warning` / `mcp_child_refused_batch`) intentionally deferred to a small follow-up PR — the snapshot already exposes `budgets[0].status: 'warning'|'error'` + `refusedCount` so operator visibility isn't blocked. * fixup(serve): address PR 14 review (#4247) findings 1-7 Addresses Codex + Copilot review feedback on #4247. Seven functional and forward-compat fixes; (8) `tcp` transport mapper vs createTransport deferred pending @wenshao direction (separate core/protocol decision). 1. **Single-server rediscovery bypass** — add `tryReserveSlot` at the top of `discoverMcpToolsForServerInternal`. Pre-fix a server refused at startup could be brought online later via `/mcp reconnect <name>` and exceed the cap in enforce mode. 2. **Empty `budgets[]` when mode=off** — early `return []` in `buildBudgetCells` when mode is `off`. Protocol docs / SDK types promise empty array; pre-fix emitted a synthetic noisy cell. 3. **runQwenServe validation + env leakage** — mirror CLI budget validation in `runQwenServe` (the embedded entry point); explicitly delete `QWEN_SERVE_MCP_*` env vars when options are undefined so multiple daemons in one process don't leak prior budget config to subsequent ACP children. 4. **Disabled-vs-refused precedence + stale refusal log** — config-disable wins over budget refusal in the per-server cell; `removeServer` + `disconnectServer` drop the entry from `lastRefusedServerNames` so operator action immediately clears the budget tag. 5. **Incremental remove-before-reserve ordering** — process config-removed servers FIRST in `discoverAllMcpToolsIncremental` so freed slots are visible to subsequent `tryReserveSlot` calls. Pre-fix scenario {a,b}→{a,c} with budget=2 wasted a slot. 6. **`scope` forward-compat type widening** — `'workspace' | (string & {})` on both `ServeMcpBudgetStatusCell` and `DaemonMcpBudgetStatusCell` so SDK consumers don't break when PR 23 adds `scope: 'pool'` per the documented no-schema-bump contract. 7. **Test comment alignment** — fix "With budget=1" comment to match `clientBudget: 2` code. Plus 4 new core regression tests covering #1/#2/#4/#5, and 4 new serve tests covering #3 (boot rejection + env cleanup). 237/237 pass across the affected files (36 core mcp-client-manager + 50 acpAgent + 151 serve). * docs(serve): clarify v1 snapshot-based budget warning detection (#4247) Address github-actions review-summary finding (I) on PR #4247: v1 operators have no SSE push event for budget pressure yet (deferred to PR 14b), so the protocol doc should explicitly say how to detect warning / error states from the snapshot. Adds the three-way mapping `budgets[0].status` ↔ live/refused counts. * fixup(serve): address PR 14 review round 2 (#4247 wenshao) Addresses @wenshao review on PR #4247. Three critical safety fixes + four suggestion-level improvements. Critical (zombie slot leaks — would break `enforce` mode for the rest of the daemon's lifetime): - C2: `discoverAllMcpTools` connect() catch now releases reservedSlots + clients entry. Pre-fix one failed connect permanently consumed a budget slot. - C3: `readResource` wraps client.connect() in try/catch; on throw the slot + client entry are cleaned up before re-raising. Tracked `weReservedSlot` so the cleanup only fires for newly-created lazy spawns (reused already-CONNECTED clients are untouched). - (wenshao C1 was the rediscovery-bypass also caught by Codex + Copilot — already addressed in fixup |
||
|
jinye
|
f44ed09412
|
feat(serve): preflight and env diagnostics routes (#4175 Wave 3 PR 13) (#4251)
* feat(serve): introduce ServeErrorKind and BridgeTimeoutError (#4175 Wave 3 PR 13) Lay the type foundation for `/workspace/preflight` and `/workspace/env` (and the eventual MCP guardrails route) so cells emitted by all three share a closed `errorKind` taxonomy: - `SERVE_ERROR_KINDS` literal-list + `ServeErrorKind` union — the seven values from #4175 (`missing_binary`, `blocked_egress`, `auth_env_error`, `init_timeout`, `protocol_error`, `missing_file`, `parse_error`). - `BridgeTimeoutError` typed class — `withTimeout` now rejects with this rather than a plain `Error`, letting `mapDomainErrorToErrorKind` recognize init / heartbeat / extMethod timeouts via `instanceof` instead of regex-matching message strings. Message format is preserved bit-for-bit. - `mapDomainErrorToErrorKind` helper — one place to classify `BridgeTimeoutError`, `SkillError`, fs ENOENT/EACCES/EPERM, ModelConfigError subclasses (recognized by `name` field — they aren't on the public surface of `@qwen-code/qwen-code-core`), `SyntaxError`, plus message-regex fallbacks for legacy throw sites (`agent channel closed`, missing CLI entry path). - `ServeStatusCell.errorKind` tightened from open `string` to the closed `ServeErrorKind` union. Backward compatible — PR 12 never assigned the field. - SDK mirrors: `DAEMON_ERROR_KINDS` const + `DaemonErrorKind` type; `DaemonStatusCell.errorKind` tightened. Tests: 11 new unit tests in `status.test.ts` covering each mapping rule plus the BridgeTimeoutError shape. No route changes; no behavior changes for any existing path. * feat(serve): add buildEnvStatusFromProcess helper (#4175 Wave 3 PR 13) Pure helper that constructs the `/workspace/env` payload from `process.*` state. No I/O, no ACP roundtrip, no globals beyond `process.env`. The route itself lands in the next commit. - `ServeEnvKind` discriminant: `runtime | platform | sandbox | proxy | env_var` - `ServeEnvCell extends ServeStatusCell` with `name` + optional `present` / `value`. Cells with `kind: 'env_var'` are presence-only — `value` is ALWAYS omitted to keep secret env vars off the wire even by accident. - `ServeWorkspaceEnvStatus` envelope: `{ v, workspaceCwd, initialized: true, acpChannelLive, cells, errors? }`. `initialized` is structurally `true` because env answers from the daemon process directly; `acpChannelLive` reports whether a child is up but does not change the payload shape. Whitelist policy: - Auth/secret keys (presence-only): OPENAI/ANTHROPIC/GEMINI/GOOGLE/DASHSCOPE/ OPENROUTER `_API_KEY`, `QWEN_SERVER_TOKEN`. - Non-secret keys (also presence-only for shape uniformity): base URLs, locale, TZ, NODE_EXTRA_CA_CERTS, QWEN_CLI_ENTRY. - Proxy vars (`HTTP_PROXY`/`HTTPS_PROXY`/`NO_PROXY`/`ALL_PROXY` + lowercase variants): credentials stripped via `redactProxyCredentials`, then `URL().host` so the wire only carries `host:port`. NO_PROXY is a host list rather than a URL so we pass the redacted form verbatim. SDK mirrors: `DaemonEnvKind`, `DaemonEnvCell`, `DaemonWorkspaceEnvStatus`. Tests: 9 unit tests covering the proxy-credential redaction, lowercase env fallback, NO_PROXY pass-through, presence-only `env_var` invariant (`'value' in cell === false`), whitelist enforcement, runtime tag detection, and envelope shape. * feat(serve): add GET /workspace/env route (#4175 Wave 3 PR 13) Wire `buildEnvStatusFromProcess` from the previous commit through the bridge, server, and SDK so remote clients can pre-flight the daemon's runtime environment without spawning an ACP child. - `workspace_env` capability tag (always advertised on a current daemon). - `bridge.getWorkspaceEnvStatus()` answers entirely from `process.*` — the route never consults ACP. `acpChannelLive` reflects whether a child exists but does not change the payload, so an idle daemon and a busy one return the same env shape. - `app.get('/workspace/env', ...)` mirrors PR 12's one-liner pattern. - SDK: `DaemonClient.workspaceEnv()` returning `DaemonWorkspaceEnvStatus`. - Docs: bullet in `docs/users/qwen-serve.md` calling out the presence-only redaction policy and the no-ACP-spawn guarantee. Tests: server-level (env returned + `'value' in env_var === false` assertion), bridge-level (idle and live both answer locally without hitting ACP extMethod), SDK-level (recording-fetch round-trip on `/workspace/env`). The `workspace_env` tag is added to the `EXPECTED_STAGE1_FEATURES` capability list assertion. * feat(serve): add /workspace/preflight daemon-cells path (#4175 Wave 3 PR 13) Wire the preflight route. Daemon-level cells are populated unconditionally from `process.*` and `node:fs`; ACP-level cells fall back to `not_started` placeholders when no child is alive so a poll never spawns one. - `workspace_preflight` capability tag. - `ServePreflightKind` discriminant (12 values: node_version, cli_entry, workspace_dir, ripgrep, git, npm — daemon-level — plus auth, mcp_discovery, skills, providers, tool_registry, egress — ACP-level). - `ServePreflightCell extends ServeStatusCell` with `locality: 'daemon' | 'acp'` + free-form `detail`. `ServeWorkspacePreflightStatus` envelope. - `createIdleAcpPreflightCells()` factory: emits the six ACP-level cells with `status: 'not_started'` + a uniform `hint` so the bridge can stitch them in alongside daemon cells without ever calling ACP. - `bridge.getWorkspacePreflightStatus()`: - Daemon cells via `buildDaemonPreflightCells` (Promise.all over Node-version, CLI-entry resolution mirroring `defaultSpawnChannelFactory`, `fs.stat` on `boundWorkspace` with ENOENT/EACCES/EPERM mapped to `missing_file`, best-effort `canUseRipgrep` / `getGitVersion` / `getNpmVersion` warnings). - ACP cells via `requestWorkspaceStatus` — idle factory returns the `not_started` placeholders; live path delegates to ACP via the `qwen/status/workspace/preflight` ext method (handler lands in next commit). Bridge-side timeout / channel-close while consulting ACP folds into envelope `errors[]` with `mapDomainErrorToErrorKind` classification; daemon cells still render. - `app.get('/workspace/preflight', ...)` route + JSDoc bullet. - SDK: `DaemonPreflightKind` / `DaemonPreflightCell` / `DaemonWorkspacePreflightStatus` mirrors; `DaemonClient.workspacePreflight()`. Tests: server-level (route returns the bridge payload), bridge-level (idle returns 6 daemon + 6 ACP `not_started` cells without spawning a channel), SDK-level (`workspacePreflight()` round-trip). Capability test updated. * feat(serve): wire ACP-side preflight cells (#4175 Wave 3 PR 13) Populate the six ACP-level preflight cells inside the ACP child so `/workspace/preflight` returns real values for live sessions. - `extMethod(qwen/status/workspace/preflight, ...)` dispatches to a new `buildAcpPreflightCells(config)` private method. - Five cell builders, each returning a `ServePreflightCell` with `locality: 'acp'`: - `auth`: `validateAuthMethod(authType, config)` returning non-null string → `auth_env_error`. Missing auth method → warning. Throws classified via `mapDomainErrorToErrorKind` with `auth_env_error` fallback. - `mcp_discovery`: rolls up `getMCPDiscoveryState()` + per-server `getMCPServerStatus(name)` counts. `connecting > 0` or in-progress discovery → warning + `init_timeout`; `disconnected > 0` post-discovery → error + `protocol_error`. - `skills`: `SkillManager.listSkills()`; SkillError throws are mapped via the helper (`PARSE_ERROR` → `parse_error`, `FILE_ERROR` → `missing_file`). - `providers`: `getAllConfiguredModels()`; empty list with a configured `authType` → warning + `auth_env_error`. ModelConfigError throws map to `auth_env_error`. - `tool_registry`: null registry → error + `protocol_error`. Otherwise surfaces tool count. - `egress`: stays `not_started`. PR 14 plugs in the real probe. - `errorCell` private helper extended with optional `errorKind` parameter; defaults to `mapDomainErrorToErrorKind(error)` so existing call sites (`mcp` / `skills` / `providers` envelope errors) automatically gain classification. Tests: 2 new acpAgent tests — preflight returns the six expected ACP cells with correct locality + statuses; preflight surfaces a `SkillError` (`PARSE_ERROR`) on the `skills` cell as `errorKind: 'parse_error'`. The core `vi.mock` block adds a SkillError class for `instanceof` matching inside `mapDomainErrorToErrorKind`. * docs(serve): preflight and env protocol section (#4175 Wave 3 PR 13) Document `/workspace/env` and `/workspace/preflight` end-to-end: - Common-cell shape: tighten `errorKind` from open `string` to the closed `DaemonErrorKind` enum (seven literals from #4175). Add an explicit redaction-policy paragraph covering env-var presence-only, proxy host:port reduction, and the whitelisted-secrets list. - Capability-tag list: add `workspace_env` and `workspace_preflight`. - New `### GET /workspace/env` section with sample payload, `DaemonEnvKind` / `DaemonEnvCell` types, and the redaction-policy paragraph spelling out which secret env vars are enumerated and how proxy URLs are reduced to `host:port`. - New `### GET /workspace/preflight` section with idle sample payload, `DaemonPreflightKind` / `DaemonPreflightCell` types, the seven-value `errorKind` semantics table, and the bridge-error fallback contract (mid-request ACP channel close → cells drop to `not_started` + envelope carries one `errors[]` entry). - Source-layout table: extend the `status.ts` row to mention the new `ServeErrorKind` / `BridgeTimeoutError` / `mapDomainErrorToErrorKind` surface; add a new `envSnapshot.ts` row. |
||
|
jinye
|
60fe594e8f
|
feat(serve): add read-only status routes (#4241)
* feat(serve): add read-only status routes Add read-only daemon status endpoints for workspace MCP, skills, providers, session context, and session supported commands. Expose matching typed SDK helpers and document the new additive v1 status surface. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): harden read-only status snapshots Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): address read-only status review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
jinye
|
aef35c390e
|
feat(serve): session metadata and close/delete lifecycle (#4175 Wave 2.5 PR 11) (#4240)
* feat(serve): session metadata and close/delete lifecycle (#4175 Wave 2.5 PR 11) Add explicit session close and metadata management to the daemon serve infrastructure, closing the Stage 1 limitation that sessions could only end via child crash or daemon shutdown. - DELETE /session/:id — force-closes a live session (cancels active prompt, resolves pending permissions, publishes session_closed event) - PATCH /session/:id/metadata — update mutable displayName - Enriched GET /workspace/:id/sessions with createdAt, displayName, clientCount, hasActivePrompt - session_closed + session_metadata_updated SDK event types with validation, reducer, and terminal event priority - DaemonClient.closeSession / updateSessionMetadata + session client wrappers - Capabilities: session_close, session_metadata * fix(serve): address review feedback on session lifecycle PR - Fix JSDoc on closeSession: clarify that bridge throws SessionNotFoundError (SDK absorbs 404 for client-side idempotency) - Tighten event validators: isSessionClosedData checks closedBy type, isSessionMetadataUpdatedData checks displayName type - PATCH /session/:id/metadata now returns effective stored metadata instead of echoing request fields, avoiding ambiguous no-op responses - Only publish session_metadata_updated event when displayName changes - Update chooseTerminalEvent comment to reflect session_closed * fix: address PR 4240 review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix: address remaining PR 4240 suggestions Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix: update serve sessions test mock Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
jinye
|
d2d426fad0
|
feat(serve): SSE replay sizing + slow_client_warning backpressure (#4175 Wave 2.5 PR 10) (#4237)
* feat(serve): SSE replay sizing + slow_client_warning backpressure #4175 Wave 2.5 PR 10. Closes the SSE replay / backpressure knobs called out in #3803 §02 so chatty Stage 1 sessions get an honest reconnect window and operators get a heads-up signal before clients are summarily evicted. - **`DEFAULT_RING_SIZE` 4000 → 8000.** Per-session replay ring depth now matches the #3803 §02 target for chatty sessions. - **`--event-ring-size <n>`** CLI flag (default 8000) lets operators tune the ring per daemon. Threaded `ServeOptions` → `BridgeOptions.eventRingSize` → both `new EventBus()` construction sites (fresh sessions + restore path). Validation is fail-CLOSED (positive finite integer; 0 / NaN / negative throw at boot). - **`slow_client_warning` SSE frame.** When a subscriber's queue crosses 75% full the bus force-pushes a synthetic `slow_client_warning` to that subscriber once per overflow episode, carrying `{queueSize, maxQueued, lastEventId}`. The flag re-arms after the queue drains below 37.5% (hysteresis, no flap near threshold). If the queue actually overflows after the warning, the existing `client_evicted` terminal frame path still fires. Like `client_evicted`, the warning has no `id` (synthetic frame; must not burn a sequence slot for other subscribers). - **`?maxQueued=N`** query param on `GET /session/:id/events` (range `[16, 2048]`, default 256). Lets cold reconnect clients pre-size their per-subscriber backlog so a large `Last-Event-ID: 0` replay doesn't trip the warning on the first publish. Range rationale: lower bound 16 (smaller is useless for any replay); upper bound 2048 (so a single subscriber can't pin ~1 MB just by asking). Out-of-range / non-decimal returns `400 invalid_max_queued` BEFORE opening the SSE stream — clean 4xx beats half-opening a stream + emitting a `stream_error` (which EventSource would auto-reconnect on). - **`slow_client_warning` capability tag** — single source of truth for the warning frame + `?maxQueued` query param + ring-size knob. Old daemons silently lack all of these; pre-flight via `caps.features`. - **SDK extensions** (`@qwen-code/sdk`): typed `DaemonSlowClientWarningEvent` (added to known event union and `DaemonStreamLifecycleEvent`); schema-validated by a new `isSlowClientWarningData` predicate; reducer (`reduceDaemonSessionEvent`) increments `slowClientWarningCount` + stores `lastSlowClientWarning`. Warning is **non-terminal** — `alive` stays true (only `client_evicted` / `stream_error` / `session_died` close the stream). Re-exported from the public SDK entry. - **Docs**: `qwen-serve-protocol.md` updates the features list (adds `slow_client_warning` and the previously-missing `client_identity` to match reality post-#4231), documents the `?maxQueued` query param, adds the warning frame to the event table, and notes the new default ring size. `qwen-serve.md` adds the `--event-ring-size` flag row. Tests: 19 eventBus (4 new: warning at 75%, once per episode, no `id` on the synthetic frame, hysteresis re-arm), 106 bridge (2 new: validate eventRingSize accept/reject), 111 server (4 new: ?maxQueued accept/absent/non-decimal/out-of-range + EXPECTED_STAGE1_FEATURES update), 14 SDK daemonEvents (2 new: schema validation + non-terminal reducer behavior). 321 focused tests total, all green. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * refactor(serve): adopt PR #4237 review feedback (eventBus polish) Address the actionable items from the Qwen Code review bot's pass on PR #4237: - Pre-compute `warnThreshold` / `warnResetThreshold` per `InternalSub` at `subscribe()` time so `publish()`'s per-event hot path is one integer compare per subscriber instead of a multiply + compare. The `!warned` short-circuit still collapses the steady state to a single boolean read; this just shaves a multiply when the threshold check actually fires. - Document the back-of-queue ordering choice for the synthetic `slow_client_warning` frame in `EventBus.publish()`: front-push was considered but mid-stream front-insertion would mis-count `forcedInBuf` in `BoundedAsyncQueue.next()`, and `forcePush` already short-circuits via `resolvers.shift()` for the active-consumer case — the back-of-queue path only matters for stalled consumers, who can't drain regardless of warning position. - Reuse the existing `collect()` helper in the "default ring size 8000" test for consistency with the rest of the file; the new test also tightens the assertion by checking that the first retained event id is 2 (id=1 dropped by the ring) and the last is 8001. - Soften the "~500 B per session" magic number in `BridgeOptions.eventRingSize`'s JSDoc to a qualitative description (each retained `BridgeEvent` is a reference plus its serialized payload; ceiling scales as `ringSize × average-event-size`). Rejected: - Bot's claim that the error JSON contains `\`...\`` escape sequences — bot misread the JS template-literal source as the wire output; `JSON.stringify` does not escape backticks, and the existing `cwd` error messages use the same style. - Bot's "use `Record<string, never>` instead of `[key: string]: unknown`" suggestion on `DaemonSlowClientWarningData` — every other event-data type in `sdk-typescript/src/daemon/events.ts` carries the same index signature for additive-field compatibility. - Bot's "features list breaks alphabetical order" — the capability list is grouped by protocol lifecycle (health → capabilities → session lifecycle → events → permissions), not alphabetical. Tests: 139 focused tests across eventBus + httpAcpBridge + SDK daemon events — all passing. Behavior unchanged; this is hot-path micro-opt + comment polish only. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(serve): correct queue tagging + plumb maxQueued through SDK Address both P2 findings from the Codex review pass on PR #4237. **Bug 1: `BoundedAsyncQueue.forcedInBuf` position-invariant break** The previous `forcedInBuf` counter only tracked LIVE-vs-FORCED correctly when all forced entries lived at the FRONT of the buffer (subscribe-time `Last-Event-ID` replay). The new mid-stream `slow_client_warning` path force-pushes to the BACK of the queue while the queue is still open, which the existing accounting was not designed for: - publish 6 events at maxQueued=8 → 75% threshold trips → force-push warning at the back → buf=[1..6, warning], forcedInBuf=1. - consumer shifts `1` → forcedInBuf decremented to 0 (incorrect: `1` was a live frame, not the forced one). - consumer drains 2..6 + warning → buf=[], forcedInBuf=0, true live count = 0, but `size` getter and `push()` cap check then use `buf.length - forcedInBuf` which drifts over subsequent refills, causing premature warn / eviction before the cap is actually reached. Replace the position-dependent counter with a per-entry `{value, forced}` tag. `liveCount` is incremented in `push()` / decremented in `next()` only when the shifted entry was non-forced — position becomes irrelevant. `size` getter returns `liveCount` directly. The class doc comment is rewritten to call out that the new tag is the position-independent replacement for the old "forced frames must stay at the front" invariant. Regression test in `eventBus.test.ts` reproduces the codex trace (warn at 75%, drain past warning, refill to cap) and asserts no premature eviction. **Bug 2: SDK does not expose `?maxQueued`** `docs/users/qwen-serve.md` and `docs/developers/qwen-serve-protocol.md` both document `?maxQueued=N` as something SDK clients can request, but `SubscribeOptions` on `DaemonClient` only declared `lastEventId` + `signal`, and `subscribeEvents()` always fetched `/events` without a query string. Typed-SDK consumers had no way to opt in without hand-crafting URLs. - Add `SubscribeOptions.maxQueued?: number` with JSDoc noting the daemon range `[16, 2048]` and the pre-flight requirement on `caps.features.slow_client_warning`. - `DaemonClient.subscribeEvents` builds the URL with an optional `?maxQueued=<n>` segment. No client-side range validation — the daemon's `parseMaxQueuedQuery` is the source of truth and returns structured `400 invalid_max_queued`; duplicating the bounds in two layers would diverge on the next tweak. - `DaemonSessionSubscribeOptions extends SubscribeOptions` so the new field flows through `DaemonSessionClient` automatically. Three new SDK tests: - subscribeEvents appends `?maxQueued=N` when set - omits the query string when absent (existing behavior preserved) - propagates a `400 invalid_max_queued` unchanged Tests: 214 focused tests across eventBus / bridge / SDK DaemonClient / DaemonSessionClient / daemonEvents, plus 111 in the server suite. All green; the new eventBus regression case proves the position-invariant fix. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * refactor(serve): adopt PR #4237 copilot review feedback Address 6 of 8 copilot-reviewer findings on PR #4237; the other 2 (#1 forcedInBuf live-size corruption, #5 SDK lacks maxQueued) were already fixed in |
||
|
jinye
|
0a4a08e443
|
feat(serve): add client heartbeat (#4175 Wave 2.5 PR 9) (#4235)
* feat(serve): add client heartbeat route Adds POST /session/:id/heartbeat plus SDK helpers so long-lived adapters (TUI/IDE/web) can refresh the daemon's last-seen bookkeeping. Bridge stores per-session and per-client timestamps behind a getHeartbeatState() snapshot accessor that PR 12 read-only diagnostics and PR 24 revocation policy will consume. - Capability tag: client_heartbeat (advertised on /capabilities.features) - Identified clients must echo X-Qwen-Client-Id; the bridge validates the id BEFORE bumping any timestamp so a forged id can't mask client absence - Per-client entries are dropped together with the registration ref-count in unregisterClient, so churn doesn't leak stale ids - getHeartbeatState returns a snapshot Map; mutating it does not leak into bridge state - Anonymous heartbeats bump only the per-session watermark Errors mirror the rest of the routes — 404 SessionNotFoundError, 400 invalid_client_id (header malformed or unknown for this session). Roadmap PR 9 from #4175. Depends on PR 7 (#4231 client identity, merged) for the trusted clientId registry. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * feat(sdk): re-export HeartbeatResult from package root The published @qwen-code/sdk only exposes the root entrypoint via `exports`; daemon subpath imports are not part of the public API. Adding HeartbeatResult to packages/sdk-typescript/src/daemon/index.ts made it reachable internally but not for downstream consumers writing `import type { HeartbeatResult } from '@qwen-code/sdk'` — every other daemon result type (PromptResult, SetModelResult, DaemonSession, etc.) is forwarded through the root barrel, so HeartbeatResult was the only hole in the heartbeat helper's public surface. Inserted alphabetically between DaemonStreamLifecycleEvent and KnownDaemonEvent to match the existing ordering convention. |
||
|
jinye
|
07e0e82258
|
feat(serve): advertise typed_event_schema + pin SDK public surface (#4175 PR 4 follow-up) (#4226)
* feat(serve): advertise typed_event_schema capability Follow-up to #4217 (`feat(protocol): add typed daemon event schema v1`, Wave 1 PR 4 of #4175), which landed the SDK-side typed schema + `KnownDaemonEvent` union + reducer but did not register a daemon-side capability tag for it. Without the tag, non-SDK clients (web debug UI, third-party adapters, channel/IDE backends not yet on `@qwen-code/sdk`) have no way to detect at the protocol envelope level that the daemon promises to emit only `KnownDaemonEvent`-shaped frames — they would either pin against SDK version, or pre-flight every frame defensively. Add `typed_event_schema: { since: 'v1' }` to `SERVE_CAPABILITY_REGISTRY`, inserted right after `session_events` (the route that delivers the frames whose schema this tag describes). The capability is purely informational — `narrowDaemonEvent`/`asKnownDaemonEvent` already fall back to "unknown" for older daemons that don't advertise the tag, so the SDK does not gate any behavior off the tag. Sync `EXPECTED_STAGE1_FEATURES` (server.test.ts) and the integration test array (qwen-serve-routes.test.ts) with the registry order, the same lockstep discipline #4214 codified. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * test(sdk): pin typed event surface at the public SDK entry, point DaemonSessionClient docstring at it Two small follow-ups to #4217 (Wave 1 PR 4 of #4175). 1. Public-entry regression fence `@qwen-code/sdk` is a single-entry package: `package.json.exports` only exposes `.` (`dist/index.{cjs,mjs,d.ts}`), and the bundle is built from `src/index.ts`. Symbols re-exported only from `src/daemon/index.ts` are unreachable to consumers unless they are also forwarded by `src/index.ts`. #4217 forwards the typed event schema correctly today, but the two-layer chain has no compile-time test pinning it — a future daemon export that lands in `src/daemon/index.ts` but is missed by `src/index.ts` would ship invisibly. Add `test/unit/daemon-public-surface.test.ts` that imports `* as Public from '../../src/index.js'`, asserts at runtime that every PR 4 value is `typeof === 'function'` (or a primitive of the expected shape), round-trips a raw `DaemonEvent` through the public `asKnownDaemonEvent` to prove the wire-up actually works, and compile-imports every PR 4 type so any drift fails to build. 2. DaemonSessionClient docstring pointer The class docstring already deferred typed event consumption to "the protocol schema layer" without a concrete pointer. Now that #4217 has put `asKnownDaemonEvent` and `reduceDaemonSessionEvent` in `./events.js`, name them so future readers can find the typed surface without grepping. No code change. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) |
||
ChiGao
|
c25e22b575
|
feat(serve): add session-scoped permission route (#4232)
Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> |
||
|
ChiGao
|
4d9cbe49c0
|
feat(serve): add daemon-stamped client identity (#4231)
* feat(serve): add daemon-stamped client identity * fix(serve): harden daemon client identity handling --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> |
||
|
ChiGao
|
b90a2c91c9
|
feat(sdk): harden daemon session client (#4225)
Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> |
||
|
jinye
|
2453b82add
|
[codex] Add daemon session load/resume (#4222)
* feat(serve): add daemon session load resume Adds HTTP and SDK support for restoring persisted daemon sessions through load/resume routes, including replay buffering for load and guarded concurrent restore handling. Refs #4175 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): address review feedback on daemon session load/resume - Gate `defaultEntry` claim in `restoreSession` on `defaultSessionScope === 'single'`, mirroring `doSpawn`. Without the gate, a restored session silently became the omitted-scope attach target on `'thread'`-default daemons. - Rename advertised capability `session_resume` to `unstable_session_resume` to match the underlying ACP method (`connection.unstable_resumeSession`). `session_load` stays stable. - Seed `lastEventId: 0` in `DaemonSessionClient.resume`, symmetric with `load`. The agent's `unstable_resumeSession` schedules an `available_commands_update` via `setTimeout(0)`; without the seed the SDK consumer would miss that frame. - Add HTTP-level test for the `RestoreInProgressError → 409` envelope. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * docs(serve): adopt review feedback comments on session load/resume - Cross-reference the `POST /session` disconnect-cleanup rationale from `restoreSessionHandler`'s `!res.writable` branch so future maintainers find the BQ9tV race + tanzhenxin attach-rollback context without grep. - Document `DaemonSessionState.{models, modes, configOptions}` in the SDK so callers can narrow to the ACP `SessionModelState` / `SessionModeState` / `SessionConfigOption` shapes. - Add JSDoc on `DaemonClient.restoreSession` explaining why `loadSession` and `resumeSession` collapse into one transport. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(serve): preserve restore state and harden in-flight restore races Address the four Critical findings from PR #4222 review (wenshao): - Coalesced restore waiters now observe the same ACP state the original restore caller did. `state: {}` in `restoreSession`'s coalesce branch was clobbering the spread `restored.state`, so concurrent callers got different payloads based purely on timing. Cache the load/resume response on `SessionEntry.restoreState` and return it from both the existing-byId early return and the coalesce branch. - Drop the `defaultEntry` promotion on restore. Explicit `session/load` / `session/resume` is "give me THIS id"; it must not become the implicit attach target for subsequent omitted-id `POST /session` callers under `single` scope. Reserves `defaultEntry` for sessions created through `doSpawn` only. - Reserve coalesced attaches synchronously via `InFlightRestore.coalesceState.count` so the spawn owner's `requireZeroAttaches` disconnect-reaper sees a non-zero `attachCount` on the freshly registered entry and skips the kill. Without this, B's `attachCount++` happened after `await inFlight.promise`, leaving a window where A's HTTP-disconnect cleanup could reap the session out from under B. - Include `pendingRestoreIds` in the `killSession` channel-teardown decision. The last live session leaving while a restore is in-flight on the same channel would otherwise SIGTERM the channel mid-restore. - Bump `RestoreInProgressError`'s `Retry-After` from 1s to 5s (matches `SessionLimitExceededError`); under the default `initTimeoutMs` of 10s, 1s pushed clients into tight loops. Tests: new bridge cases covering state propagation through coalesce, the spawn-owner-disconnect race, the pendingRestoreIds-aware channel teardown, and the no-promote- on-restore invariant. Existing "attaches twice" test rewritten to assert the cached restore state propagates. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * test(serve): cover acpAgent load/resume + restore route error mappings Close the test-coverage gaps wenshao called out in PR #4222 review: - acpAgent.test.ts gains a `QwenAgent loadSession / unstable_resumeSession` block that locks down the new contract end-to-end at the agent layer: * `loadSession` missing persisted session → throws `RequestError.resourceNotFound("session:<id>")` (code -32002 + `data.uri`). * `loadSession` existing session → returns LoadSessionResponse AND triggers `session.replayHistory(messages)` so SSE subscribers see the persisted turns. * `unstable_resumeSession` missing session → same resourceNotFound contract. * `unstable_resumeSession` existing session → returns the response WITHOUT replaying history (resume restores model context internally; UI replay is intentionally suppressed). Required extending the mocked `RequestError` with `resourceNotFound`, and mocking `SessionService` per case. - server.test.ts adds the missing restore-route wire mappings: `WorkspaceMismatchError → 400 workspace_mismatch` and `SessionLimitExceededError → 503 + Retry-After: 5`. Combined with the existing 409 case for `RestoreInProgressError`, the route layer now has full structured-error coverage. - Updated the 409 test's `Retry-After` expectation from `1` to `5` to match the bumped retry hint. Disconnect-cleanup tests for the restore route were intentionally not added — the cleanup branch is line-for-line identical to `POST /session`'s handler (which itself ships without route-level disconnect tests due to flaky supertest + Node http close-event timing). 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * docs(serve): document daemon session load/resume routes Sync the docs to the routes that landed via PR #4222: - `docs/developers/qwen-serve-protocol.md`: * Add `session_load` and `unstable_session_resume` to the advertised features list, with a note on the `unstable_` prefix mirroring ACP's underlying method name. * Document `POST /session/:id/load` and `POST /session/:id/resume` — request body, response shape (including the cached `state` field that late attachers observe), and the full error envelope: 404 unknown id, 400 workspace_mismatch, 503 session_limit_exceeded (counts in-flight restores), 409 restore_in_progress (cross-action race). * Note the SSE replay ring bound (4000 frames default) and the "subscribe immediately after load" guidance for long histories. - `docs/users/qwen-serve.md`: * Add a "Loading and resuming a persisted session" section with the SDK example (`DaemonSessionClient.load` / `DaemonSessionClient.resume`) and the load-vs-resume decision table. * Update the durability model — sessions are still ephemeral across daemon restarts in Stage 1, but persisted sessions on disk can now be reloaded. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(test): use _meta payload to satisfy ACP SessionConfigOption types The two new state-propagation tests in `httpAcpBridge.test.ts` used `{ id, name, value }` as a `SessionConfigOption`, but ACP's actual `SessionConfigSelect` shape requires `currentValue` + `options`. vitest runs through esbuild and skips strict typechecking, so the local `vitest run` passed; CI's `tsc --build` (run during `npm run prepare`) caught it. Switch the fixture to `_meta: { tag: '...' }` instead — `_meta` is typed as `Record<string, unknown> | null` on the ACP response shapes, so any payload survives. The assertions only need the bridge to forward the state object intact, which `_meta` proves equally well without committing the test to the full SessionConfigOption union. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(serve): symmetric restore coalesce guard + transportClosed leak + defensive cleanup Address the two new Critical findings + the test/cosmetic gaps from wenshao's second review pass on PR #4222 (`a3f38da3a`): - **[Critical] Symmetric coalesce guard.** The previous guard only rejected `load`-on-`resume`; `resume` arriving while a `load` was in flight silently coalesced and inherited the load's history- replay frames over SSE — directly violating resume's "no UI replay" contract (made worse by `DaemonSessionClient.resume()` seeding `lastEventId: 0`). Tighten the guard to `action !== inFlight.action` so any cross-action race throws `RestoreInProgressError`. Same-action coalescing is unaffected. - **[Critical] `transportClosed` dangling rejection.** When `withTimeout` wins the `Promise.race` against `channel.exited`, the `.then(throw)` chain on `channel.exited` stays pending. A later channel exit (next session boundary, daemon shutdown, agent crash) fires the `throw` with no observer attached — Node 22 logs `unhandledRejection`, and `--unhandled-rejections=throw` deployments crash the daemon. Add `transportClosed.catch(() => {})` to suppress the dangling rejection after the race settles. - **`isAcpSessionResourceNotFound` exact-match fallback.** The message-fallback path used `message.includes(expectedUri)`, which would falsely match a sessionId of `"a"` against a message containing `"session:abc"`. Tighten to exact equality on the canonical `Resource not found: <uri>` form. The primary `data.uri` path remains the dominant code path. - **`loadSession` mcpServers default symmetry.** `loadSession` now uses `params.mcpServers ?? []` to mirror `unstable_resumeSession`. Defends against a future ACP schema loosening that makes `LoadSessionRequest.mcpServers` optional — without the null-coalesce, `newSessionConfig` would `TypeError` on iteration. Tests added: - `httpAcpBridge.test.ts`: `resume`-on-`load` rejection (mirror of the existing `load`-on-`resume` test); regression for the dangling `unhandledRejection` (resolves `channel.exited` after the restore promise has already settled and asserts no `unhandledRejection` event); shutdown-awaits-restore via `Promise.race`-based ordering. - `server.test.ts`: 400 for non-string and over-length `cwd` on the restore routes (mirroring the equivalent `POST /session` cases for `parseOptionalWorkspaceCwd`). - `acpAgent.test.ts`: load with `getResumedSessionData()` returning `undefined` — distinct code path that does NOT call `replayHistory`. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
ChiGao
|
80f1e266ba
|
feat(protocol): add typed daemon event schema v1 (#4217)
Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> |
||
|
ChiGao
|
57de269f45
|
feat(sdk): add DaemonSessionClient skeleton (#4201)
* feat(sdk): add daemon session client * fix(sdk): harden daemon session event replay * fix(sdk): replay attach-time daemon events --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> |
||
|
jinye
|
878f35fc4f
|
feat(serve): per-request sessionScope override on POST /session (#4175 Wave 2 PR 5) (#4209)
* feat(serve): per-request sessionScope override on POST /session Resolves the FIXME at httpAcpBridge.ts:BridgeOptions.sessionScope from #3803 — clients can now override the daemon-wide sessionScope per request instead of being stuck with whatever boot-time value the operator picked. A VSCode window that wants strict isolation can ask for `'thread'` against a default-`'single'` daemon, and vice versa. Wire change: - POST /session body accepts optional `sessionScope: 'single' | 'thread'` - Per-request value wins; daemon-wide default remains the fallback when the field is omitted (bit-for-bit backward compat for every existing caller) - Invalid values yield 400 `{ code: 'invalid_session_scope' }` - New capability tag `session_scope_override` advertised on /capabilities.features for negotiation Bridge changes: - BridgeSpawnRequest gains optional `sessionScope` - spawnOrAttach validates the per-request value and resolves effectiveScope = req.sessionScope ?? defaultSessionScope - doSpawn now takes effectiveScope and only stamps `defaultEntry` (the single-scope attach slot) when the spawn is single-scope — fixes a mixed-scope leak where a thread-first call would let a later omitted-scope call attach to the supposedly-isolated session SDK: - CreateSessionRequest gains optional `sessionScope` - DaemonClient.createOrAttachSession conditionally spreads it into the JSON body so omitted callers send the same wire shape as before Tests: - 4 new bridge tests (override single→thread, override thread→single, mixed-scope leak regression, invalid-value rejection) - 3 new server tests (valid passthrough, invalid 400, omitted backward compat) - 2 new SDK tests (forwards/omits sessionScope on the wire) - EXPECTED_STAGE1_FEATURES updated for the new capability tag 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(serve): address Wave 2 PR 5 review findings Three independent review passes found three real issues: 1. Bridge `TypeError` on invalid `sessionScope` collapsed to opaque 500 in `sendBridgeError` instead of the typed `400 invalid_session_scope` the route layer guarantees. Direct embed / test / future entry-point callers bypassing the route would see a generic 500 with stack noise on stderr — disagreeing with the route contract. Fix: add `InvalidSessionScopeError` class (alongside `SessionNotFoundError` / `WorkspaceMismatchError` / `SessionLimitExceededError`); the `spawnOrAttach` validator now throws it, and `sendBridgeError` translates to the same `{ error, code: 'invalid_session_scope' }` shape. 2. SDK `DaemonClient.createOrAttachSession` used a truthy check (`req.sessionScope ? ...`) for the conditional spread, silently erasing falsy-but-defined values (`''`, `null`, `0`) on the wire. A buggy caller would never see the daemon's 400 — it'd inherit the daemon-wide default while believing it requested a specific scope. Fix: use `!== undefined` (matching the bridge's own validation shape). Same fix to the server-side spread for consistency. 3. JSDoc and docs referenced `serve --sessionScope` as if it were a shipping CLI flag. It isn't — `ServeOptions` has no field, neither `runQwenServe` nor `serve.ts` plumbs one, and the production daemon default is hardcoded to `'single'`. Strike the references; note that #4175 may add the flag in a follow-up. Test coverage expanded: - Cap-bypass guard: per-request `'thread'` overrides cannot bypass `maxSessions` on a daemon-default-`'single'` deployment. Without this, a future refactor that gated the cap on `defaultSessionScope` instead of `effectiveScope` would silently let `'thread'` overrides amplify past the limit — the exact N-amplification cliff #3803 was about. - Symmetric mixed-scope leak: daemon-default-`'thread'` + single-first-call followed by omitted-scope-second-call must produce distinct sessions. Mirrors the existing daemon-default-`'single'` + thread-first leak regression. - Concurrent mixed-scope coalescing: simultaneous single + thread `spawnOrAttach` against the same workspace under slow `initialize` must not collide on `inFlightSpawns` (tracker keys differ by scope). - Updated invalid-scope rejection test to assert `InvalidSessionScopeError` instance + carried `sessionScope` field. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) |
||
|
jinye
|
264ed82273
|
[codex] feat(serve): add capability registry protocol versions (#4191)
* feat(serve): add capability registry protocol versions Introduce a serve capability registry and advertise protocolVersions from /capabilities while preserving the existing v1 envelope and Stage 1 feature aliases. Update SDK wire types, docs, and focused tests for old-daemon compatibility. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): clarify capability advertisement semantics Address PR review feedback by preserving historical capability versions, separating registered and advertised feature helpers, testing protocol version metadata directly, and keeping runtime exports out of the serve types module. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
jinye
|
dcf7681d65
|
feat(core,cli): add generic atomicWriteFile, wire into Write/Edit tools, upgrade @types/node (#4096)
* feat(core): add generic atomicWriteFile and wire into Write/Edit tools The Write and Edit tools used bare fs.writeFile, risking half-written corrupt files on crash or power loss. Both tools' source code contained explicit TODOs noting atomic write as the fix. - Add atomicWriteFile() supporting string/Buffer with flush (fsync), permission preservation, symlink resolution, and EXDEV fallback - Wire StandardFileSystemService.writeTextFile() through atomicWriteFile - Refactor atomicWriteJSON to delegate to atomicWriteFile (adds fsync) - Deduplicate renameWithRetry from runtimeStatus.ts - Add flush:true to writeWithBackupSync for settings writes - Upgrade @types/node to ^22.0.0 (flush option type support) Closes the TODO in write-file.ts:371-385 and edit.ts:487-497. Ref: #4095 (Phase 1) 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(core): address review comments on atomicWriteFile - Fix permission window: separate existingMode from desiredMode so mode is set during writeFile (not just chmod after), eliminating the brief window where tmp file has overly permissive defaults - Fix broken symlink handling: use lstat+readlink instead of realpath to correctly resolve symlinks whose targets don't exist yet, preventing the symlink from being replaced by rename - Add test for writing through a broken symlink 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(core): address wenshao review on atomicWriteFile - Fix Windows bug: use path.isAbsolute() instead of startsWith('/') - Hoist path import to top-level static import - Resolve full symlink chains via loop (handles A→B→C), with ELOOP guard at 40 hops matching POSIX SYMLOOP_MAX - Mask stat.mode with 0o7777 to strip file-type bits - Document EXDEV fallback atomicity loss in JSDoc - Add tests for relative symlinks and multi-level symlink chains 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(test): fix CI failures from atomic write changes - edit.test.ts: mock writeTextFile instead of chmod 444 for write error test — atomic write creates tmp file in same dir, so readonly target no longer triggers a write error - atomicFileWrite.test.ts: skip permission tests on Windows — chmod is a no-op and stat.mode always returns 0o666 * fix(core): address deepseek review on atomicWriteFile - Add try/catch around chmod calls to handle FAT/exFAT filesystems where POSIX permissions are not supported - Add explicit type annotation to lstats variable * fix: restore version numbers to 0.15.11 after rebase * fix(core): resolve relative symlinks through directory symlinks resolveSymlinkChain used path.dirname() to resolve relative symlink targets, which is purely string-based. When intermediate path components are themselves directory symlinks, the result would be wrong (e.g. /a/link/file → ../target resolves to /a/target instead of the kernel-resolved /b/target). Use fs.realpath() on the parent directory to get the kernel-resolved base for relative-target resolution. * fix(test): normalize path separators in directory symlink test Windows readlink returns native separators (backslashes), causing the directory-symlink test to fail on Windows CI. Wrap both sides of the symlink-target comparison with path.normalize. * refactor(core): dedupe write/chmod logic in atomicWriteFile - Extract writeOptions construction and tryChmod helper, removing duplication between the main write path and the EXDEV fallback - Document atomicWriteJSON's symlink-preservation behavior Addresses deepseek review on PR #4096. |
||
tanzhenxin
|
f6315b378d
|
refactor(cli): revert dynamic slash command LLM translation (#4145)
* refactor(cli): revert dynamic slash command LLM translation (#4137) Removes the runtime LLM-translation path for dynamic slash command descriptions added in #3871, along with its `general.dynamicCommandTranslation` setting and the `/language translate` subcommand tree. Keeps the built-in locale coverage from the same PR untouched. Localization of dynamic command descriptions should be solved at the source (manifest fields, not runtime model calls); see #4137 for the proposed alternative. * refactor(cli): drop translate prompts from mustTranslateKeys Follow-up to the dynamic command translation revert: the 7 prompt keys were stripped from every locale file in the previous commit, but the allow-list in mustTranslateKeys still demanded them. * refactor(cli): drop dead CommandService.fromCommands and vacuous tests Follow-up cleanup after the dynamic command translation revert. CommandService.fromCommands was introduced by #3871 solely to wrap the LLM-translated command list. With the LLM-translation path gone, it has no remaining non-test callers — remove it and the matching test mock. Also drop two assertions in languageCommand.test.ts that checked for the absence of a top-level /language cache command. They tested a migration state that never existed in this branch and now pass vacuously. * docs: drop /language translate references after revert Two user-facing docs documented the /language translate subcommands (status/on/off/cache refresh/clear) that were removed in the dynamic command translation revert. Strip them so users following the docs don't hit "Invalid command" errors. * refactor(cli): drop unused localizeDescription field The DynamicCommandLocalizationService that read this flag was removed in the revert, leaving the field with five setters and zero readers. Drop the field, its JSDoc, and the five `localizeDescription: true` assignments. Also tidy the now-misleading `modelDescription` JSDoc and the stale `reloadCommands` comment that referenced the removed feature. * refactor(cli): drop unused getLanguageNameForTranslationTarget The only caller was the removed DynamicCommandLocalizationService. Remove the function from `i18n/languages.ts` and the matching import + re-export from `i18n/index.ts`. |
||
Shaojin Wen
|
790f2d0485
|
refactor(serve): 1 daemon = 1 workspace (#3803 §02) (#4113)
* refactor(serve): 1 daemon = 1 workspace (#3803 §02) Stage 1 shipped with M-workspaces-per-daemon routing (`byWorkspaceChannel` Map keyed by request `cwd`). The §02 architectural revision in `docs/comparison/qwen-code-daemon-design/02-architectural-decisions.md` narrows the bridge to 1 daemon = 1 workspace × N sessions: each daemon binds to one canonical workspace path at boot; `POST /session` with a mismatched `cwd` returns 400 `workspace_mismatch`. Multi-workspace deployments run multiple daemon processes (one per workspace, supervised externally — systemd / docker-compose / k8s / `qwen-coordinator`). Bridge state collapses from maps to single optional slots: - `byWorkspaceChannel: Map<string, ChannelInfo>` → `channelInfo?: ChannelInfo` - `inFlightChannelSpawns: Map<string, Promise>` → `inFlightChannelSpawn?: Promise` - `byWorkspace: Map<string, SessionEntry>` → `defaultEntry?: SessionEntry` - `liveChannels: Set<ChannelInfo>` → not needed; `channelInfo` is the live reference, cleared only by `channel.exited` (preserves the tanzhenxin BkUyD invariant that `killAllSync` finds a target mid-SIGTERM-grace) `BridgeOptions.boundWorkspace` becomes required. `WorkspaceMismatchError` is thrown from `spawnOrAttach` when the request's canonical cwd doesn't match the bound path, translated to 400 `workspace_mismatch` (with both paths in the body) by the route layer. `CapabilitiesEnvelope.workspaceCwd` surfaces the bound path so clients pre-flight check + omit `cwd` from `POST /session` (it falls back to the bound workspace). A new `--workspace <path>` CLI flag lets operators override `process.cwd()` at boot. The previous `--http-bridge` / `--multi-workspace` opt-in was never shipped; nothing changes for default users running `qwen serve` in their project directory. Removed code path: ~150 LOC of multi-workspace map machinery in `httpAcpBridge.ts` plus the test cases that exercised it. Test surgery: - New `makeBridge()` helper in `httpAcpBridge.test.ts` injects `boundWorkspace: WS_A` by default; tests that need a different bind (the mismatch test) pass it explicitly. - `does NOT reuse across workspaces` → `rejects cross-workspace requests with WorkspaceMismatchError` (the new semantics under §02). - `shutdown kills every live channel` retargeted to single-channel multi-session shutdown. - `killAllSync force-kills channels even after shutdown cleared byWorkspaceChannel (BkUyD)` retargeted to single-channel: the invariant is the same (channel reference must outlive eager shutdown clearing), the surface is just smaller. - `listWorkspaceSessions` cross-workspace assertion now expects empty for the un-bound path. - `--max-sessions` cap test uses two thread-scope sessions on `WS_A` instead of WS_A + WS_B. Closes #3803 §02. * fix(serve): address review findings on the §02 refactor Two correctness fixes + four doc/test polish items surfaced by the multi-agent review of #4113: 1. `killSession` → `spawnOrAttach` race (Critical). After killing the last session, `channel.kill()` runs through a 5s SIGTERM grace before SIGKILL. During that window a concurrent `spawnOrAttach` used to hit `ensureChannel`, find `channelInfo` still set, and reuse the dying transport — either landing the caller with a sessionId that 404s on every follow-up once `channel.exited` fires, or hanging until the newSession timeout. Fix: add an `isDying: boolean` flag on `ChannelInfo`, set synchronously by `killSession` / `doSpawn`-newSession-failure / `shutdown` BEFORE awaiting `channel.kill()`. `ensureChannel` treats a dying channel as absent and spawns a fresh one. The tanzhenxin BkUyD invariant ("`channelInfo` reference must outlive the kill-await for `killAllSync` mid-grace") is preserved — we set `isDying` but don't clear `channelInfo` until the OS reaps the child via `channel.exited`. A regression test in `httpAcpBridge.test.ts` pins the invariant: a never-resolving `kill()` keeps the SIGTERM grace open while a concurrent spawn verifies the factory was called twice (two distinct handles). 2. `boundWorkspace` canonicalization divergence (Critical). `server.ts` and `runQwenServe.ts` each computed `opts.workspace ?? process.cwd()` independently. The bridge canonicalized that string via `realpathSync.native` (resolving symlinks, case-folding on case-insensitive filesystems); the callers retained the raw form. On macOS HFS+ / APFS or any symlinked path, `/capabilities.workspaceCwd` advertised one spelling while the bridge enforced against another — clients echoing the advertised path back saw `POST /session` succeed but the response carry a different `workspaceCwd`. Fix: export `canonicalizeWorkspace` from `httpAcpBridge.ts` and call it once in `runQwenServe` (after the existence check) and once in `createServeApp`. Both paths land on the same canonical form; the bridge's own re-canonicalize is now a no-op (idempotent). 3. Reject `--workspace` pointing at non-existent directories at boot (Suggestion). `canonicalizeWorkspace`'s ENOENT fallback to `path.resolve` previously let the daemon boot pointed at a path that didn't exist; every `POST /session` then spawned a `qwen --acp` child with that cwd and the agent failed with an opaque ENOENT. Now `runQwenServe` `statSync`s the bound path at boot and rejects "directory does not exist" / "not a directory" with a clear message. 4. Stale docstrings (Nice to have). `types.ts` `ServeMode` JSDoc said "one `qwen --acp` child PER WORKSPACE" — directly contradicted the new `workspace` field's doc in the same file. `commands/serve.ts` `--http-bridge` description said "per workspace" — directly contradicted the `--workspace` flag's help in the same yargs builder. Both updated to "per daemon (the daemon binds to ONE workspace at boot)". 5. Stale `byWorkspace` comment references (Nice to have). `server.ts:188` ("orphaned in byId / byWorkspace") and `httpAcpBridge.test.ts:1210` ("still in byId/byWorkspace at the moment of crash") referenced the removed Map. Updated to `defaultEntry`. 6. `/capabilities` curl example in the Authentication section of `docs/users/qwen-serve.md` was missing the new `workspaceCwd` field — the Quickstart's curl example was updated but the parallel one in the auth section was not. Synced. Tests added: - `killSession marks the channel dying so concurrent spawnOrAttach gets a fresh channel` — pins fix (1). - `--workspace flows end-to-end and surfaces on /capabilities` — exercises the runQwenServe → server.ts → bridge plumbing that no prior test covered. - `rejects --workspace pointing at a non-existent directory` and `rejects --workspace pointing at a regular file` — pin fix (3). - `rejects relative --workspace at boot` — covers the absoluteness check that exists but was untested. Net: +238 / -24 across 8 files. All 149 serve tests pass. * fix(serve): BkUyD overwrite race + Windows-fragile test + doSpawn-failure coverage Round-2 review of #4113 caught three follow-up issues introduced by or left open after round-1's fixes: 1. **BkUyD invariant overwrite race (Critical).** Round-1's `isDying` flag lets `ensureChannel` skip a dying channel and spawn a fresh one. When the fresh spawn completes, `channelInfo = info` overwrote the dying channel's reference — leaving NO global pointer to it. `killAllSync()` then iterated only `channelInfo` (the fresh one) and missed the dying child entirely. A double-Ctrl+C arriving mid-SIGTERM-grace would call `process.exit(1)` before the dying child's per-channel SIGKILL escalation timer fired, orphaning the child. Restore a `aliveChannels: Set<ChannelInfo>` (parallel to the original Stage 1 design, but justified by single-workspace too). Entries added in `ensureChannel`, removed by each channel's `channel.exited` handler. `killAllSync` iterates the SET, not the single attach-target slot. `shutdown` does the same — snapshots every alive channel and kills each, not just the current `channelInfo`. New regression test pins the invariant: spawn → killSession (channel marked dying, kill hangs) → spawnOrAttach (fresh channel overwrites `channelInfo`) → `killAllSync` — expect BOTH channels' `killSync` to fire. Pre-fix only the fresh one would have fired. 2. **Windows-fragile test path.** The new `rejects --workspace pointing at a regular file` test used `new URL(import.meta.url).pathname` to get a path to the test file. On Windows that returns `/C:/path/...` (leading slash); `fs.statSync` then resolves it as path-from-current-drive-root, fails with ENOENT, and the test sees the "does not exist" error message instead of the expected "not a directory" branch. CI runs `windows-latest`. Fix: `fileURLToPath(import.meta.url)` from `node:url`. 3. **doSpawn newSession-failure isDying path was untested.** The round-1 fix added `ci.isDying = true` to both `killSession` AND `doSpawn`'s newSession-failure catch, but only the killSession path had a regression test. Added a parallel one for the doSpawn path: thread-scope bridge with a `newSessionImpl` that throws on the first call → captures the rejection without awaiting it (the bridge's `await ci.channel.kill()` hangs in the test), yields enough cycles for the `isDying = true` sync prefix to settle, then confirms (a) the next `spawnOrAttach` produces a fresh channel and (b) `killAllSync` finds both channels in `aliveChannels`. Also added a `newSessionImpl` option to the test FakeAgent — the existing `initializeThrows` hook covered handshake-time failures, but post-init `newSession` rejections (auth, bad config, mid-init crashes) had no test affordance. All 151 serve tests pass. * docs(serve): update daemon-client-quickstart for §02 single-workspace Round-3 review caught that the SDK example doc was the only one of the three serve-related docs that the §02 refactor didn't touch. Updated: - Boot log example now shows the `, workspace=/path/to/your-project` suffix that `runQwenServe` emits after the §02 changes. - The "Hello daemon" example now reads `caps.workspaceCwd` off `/capabilities` and passes it back as `workspaceCwd` on session creation — illustrating the documented pre-flight pattern, not a hand-written literal that may not match the daemon's actual bind. - Shared-session example makes the prerequisite explicit: the daemon must be bound to `/work/repo` (via `--workspace` or `cd`); under §02 two clients can only share a session if they're both hitting a daemon already bound to that workspace. - New "Workspace mismatch" section shows how to handle the `400 workspace_mismatch` error class: catching `DaemonHttpError`, branching on `body.code`, surfacing `boundWorkspace` / `requestedWorkspace` for the operator. This is a new error class SDK consumers' error handlers should branch on. No code changes; docs only. * feat(sdk,test): align SDK types + integration tests with §02 single-workspace Round-4 review caught one type-drift gap + a set of integration-test assumptions that the §02 refactor invalidated. **SDK type drift.** `DaemonCapabilities` in `packages/sdk-typescript/src/daemon/types.ts` was the SDK-side mirror of `CapabilitiesEnvelope` on the daemon side. The §02 PR added `workspaceCwd: string` to the daemon envelope (and the round-3 doc example reads `caps.workspaceCwd` off the SDK client) but the SDK type wasn't updated. A TypeScript consumer copying the doc snippet verbatim would hit `TS2339 'workspaceCwd' does not exist on type 'DaemonCapabilities'`. The wire field is present so JS consumers wouldn't notice — but the SDK is marketed as a TypeScript quickstart, so this is a real onboarding break. Fix: add `workspaceCwd: string` to `DaemonCapabilities` (parallel to `DaemonSession.workspaceCwd` which is already there). The SDK unit test for `client.capabilities()` was updated to put the new field in the mocked response. **Integration tests.** `qwen-serve-routes.test.ts` spawns a real `qwen serve` daemon in `beforeAll`. Three breakages exposed: 1. The daemon was launched without `--workspace`, so it inherited the test runner's `cwd`. Tests then POST `workspaceCwd: REPO_ROOT` assuming the daemon is bound to the repo root — true when run via `npm test` from the repo, brittle from IDEs / launchers that have a different `cwd`. Added `'--workspace', REPO_ROOT` to the spawn args so the bound workspace is deterministic regardless of where the test runner is launched. 2. The `bad modelServiceId` test used `cwd: '/tmp'`. Under §02 this would now return 400 workspace_mismatch before the session was spawned. Switched to `REPO_ROOT` and softened the `attached` assertion (REPO_ROOT may already have a session from earlier tests in the suite under sessionScope:single). 3. Added three new integration tests pinning the §02 surface end-to-end through a real daemon process: - `rejects cross-workspace cwd with 400 workspace_mismatch` — posts `/tmp` and asserts the full structured error body (`code`, `boundWorkspace`, `requestedWorkspace`). - `omits cwd → falls back to bound workspace` — posts an empty body and asserts the response's `workspaceCwd` matches REPO_ROOT (verifies the runQwenServe → createServeApp → bridge fallback plumbing). - `GET /capabilities surfaces workspaceCwd` — asserts the new SDK type field is populated correctly off the wire. All 422 unit tests pass (cli serve + sdk). Integration tests typecheck clean. * fix(serve): address /review feedback from gpt-5.5 + deepseek-v4-pro Process the 7 inline /review comments on PR #4113: - C1+C3 (SDK): make `DaemonCapabilities.workspaceCwd` and `CreateSessionRequest.workspaceCwd` optional in the SDK types. `workspaceCwd` is an additive field on the v=1 envelope per #3803 §02; the protocol's "bump v only on incompatible changes" stance is honored by leaving the field optional at the type level. `DaemonClient.createOrAttachSession` now omits `cwd` from the body when `workspaceCwd` isn't passed, matching the PR description's "SDK accepts bound path or none". Adds a unit test pinning the empty-body shape. - C2 (docs/users/qwen-serve.md): the `--http-bridge` row described the pre-§02 per-session model; updated to reflect one child per daemon with N sessions multiplexed via ACP `newSession()`. - C4 (server.ts): `WorkspaceMismatchError` was silently 400'ing without a stderr breadcrumb, leaving operators blind to cross-workspace routing drift. Mirrors the SessionLimitExceeded /InvalidPermissionOption observability pattern. - C5 (server.test.ts): the `/capabilities` fallback test compared `res.body.workspaceCwd` against raw `process.cwd()`; on macOS default tmpdir flows (`/var/folders/...` → `/private/var/...`) the canonicalize-once route value diverges. Use `realpathSync.native(process.cwd())` to match the route's canonicalization. - C6 (server.ts): the cwd-not-absolute error said "cwd is required and must be an absolute path" but cwd is now optional under §02. Tightened wording to "must be an absolute path when provided". - C7 (runQwenServe.ts): the `statSync` catch only wrapped ENOENT with a friendly diagnostic; EACCES / EPERM (typical for SIP-protected dirs on macOS or root-owned paths the daemon's UID can't traverse) re-threw as raw `SystemError`. Wrap both codes with a `--workspace`-context message so the boot failure points at the flag the operator set. Docs: quickstart shows the explicit-pass-or-omit options side by side; protocol reference notes `workspaceCwd` is additive to v=1. * fix(serve/test): make /work/bound literals Windows-portable Windows CI failed on this PR's two new tests because returns (drive-relative absolute), so the route's canonicalize step diverged from the hardcoded literal. Mirror the WS_A/WS_B pattern already used in httpAcpBridge.test.ts: define WS_BOUND / WS_DIFFERENT via `path.resolve(path.sep, …)` and use the constants everywhere. The 400 workspace_mismatch test would still have passed (mock controls both throw + assertion) but I aligned it for consistency. Failures from CI run 25806528710: expected 'D:\work\bound' to be '/work/bound' (Object.is) Affected tests: - createServeApp > GET /capabilities > reports the bound workspace - createServeApp > POST /session > 200 when cwd is omitted * fix(serve): address second /review round (gpt-5.5 + deepseek-v4-pro) Four new inline findings from the latest /review pass: - N1 (integration-tests/cli/qwen-serve-routes.test.ts) — Critical: the `workspace_mismatch` assertion compared `requestedWorkspace` against the literal `'/tmp'`, but the bridge canonicalizes via `realpathSync.native` and on macOS `/tmp` is a symlink to `/private/tmp`. Compare against `realpathSync.native('/tmp')` so the assertion is portable. - N2 (packages/cli/src/serve/types.ts): `CapabilitiesEnvelope.workspaceCwd: string` (server side) diverged from the SDK's `DaemonCapabilities.workspaceCwd?: string`. Made the server type optional too — matches the SDK, matches the protocol doc's "additive to v=1" framing, doesn't change runtime emission (the post-§02 server still always populates the field). - N3 + N4 (packages/cli/src/serve/server.ts + sdk-typescript/.../DaemonClient.ts): the route's `cwd` validation treated every non-string body value (`null`, `123`, `{}`, `[]`) the same as omitted, silently falling back to `boundWorkspace`. That hid client/orchestrator serialization bugs as "session attached to wrong workspace". Now the route uses `'cwd' in body` to detect presence and rejects presence-but-not-a-string with `400 'cwd must be a string absolute path when provided'`. Empty string still hits the existing `path.isAbsolute` branch ("must be an absolute path when provided"), so an SDK caller passing `workspaceCwd: ''` no longer silently lands in the daemon's bound workspace. SDK side: reverted my conditional spread to `cwd: req.workspaceCwd` unconditional. `JSON.stringify` strips `undefined` automatically (so omitted `workspaceCwd` becomes "no `cwd` key" on the wire, as before), but empty-string is now forwarded verbatim and the server's 400 surfaces the bug instead of the SDK swallowing it. Added a unit test pinning the empty-string-forwarded shape. Server tests: - `400 when cwd is present but not a string` covers null / number / object / array via a sub-loop. - `400 when cwd is the empty string` pins the isAbsolute path. bridge: 73/73; server: 80/80 (was 78, +2 new); SDK: 40/40 (was 39, +1 empty-string test). tsc clean for SDK and PR-touched CLI files. * fix(serve): use const cwd in POST /session (prefer-const lint) CI lint failed with packages/cli/src/serve/server.ts:199:9 prefer-const: 'cwd' is never reassigned. The wave-4 rewrite split the original 'let cwd; if (!cwd) cwd = boundWorkspace' into a single ternary, which removes the only mutation path; the variable should be const accordingly. * fix(serve): address third /review round (gpt-5.5 + glm-5.1 + deepseek-v4-pro) Five new inline findings; M1 was already resolved in |
||
|
Shaojin Wen
|
870bdf2a9d
|
feat(cli,sdk): qwen serve daemon (Stage 1) (#3889)
Some checks are pending
Qwen Code CI / Classify PR (push) Waiting to run
Qwen Code CI / Lint (push) Blocked by required conditions
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Blocked by required conditions
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* feat(cli): scaffold `qwen serve` HTTP daemon (Stage 1, #3803) Adds a `serve` subcommand that boots an Express 5 listener with bearer auth, host allowlist, and CORS modeled on `vscode-ide-companion/src/ ide-server.ts`. Ships only `/health` and `/capabilities` to begin with; session/prompt/event routes will land in follow-up PRs once the per- session ACP child-process bridge in `httpAcpBridge.ts` is wired. Defaults to 127.0.0.1 with auth disabled so local development needs no configuration. Binding beyond loopback (e.g. `--hostname 0.0.0.0`) refuses to start without a token (`--token` or `QWEN_SERVER_TOKEN`). Capabilities envelope versioned at v=1 with a `features` array — clients should gate UI off `features`, never off `mode`, so subsequent PRs can add capability tags without breaking older clients. Per design issue's Stage 1 scope (~700-1000 LOC). Adds ~430 LOC of implementation + tests in this scaffold; the remaining budget belongs to the route wiring + bridge implementation in follow-ups. * feat(cli): wire HttpAcpBridge + POST /session for `qwen serve` (#3803) Stage 1 follow-up to the scaffold. Implements the bridge between the HTTP daemon and the existing ACP child agent, plus the first session endpoint. `HttpAcpBridge.spawnOrAttach`: - Spawns `node $cliEntry --acp` per workspace via an injectable `ChannelFactory` (default uses `process.argv[1]`; tests use an in-memory `TransformStream` pair so they don't fork real processes). - Drives the ACP `initialize` + `newSession` handshake via the SDK's `ClientSideConnection`, with a 10s timeout that kills the channel. - Under `sessionScope: 'single'` (default), reuses the live session when the same canonical workspace cwd is requested again — backs the `attached: true` flag. - The `Client` impl on the bridge side proxies file reads/writes to local fs (daemon and agent share the host) and buffers `sessionUpdate` notifications for the SSE wiring in the next PR. `requestPermission` returns `cancelled` until the `/permission/:requestId` route lands. `POST /session`: - 400 on missing or relative `cwd`. - 200 with `{sessionId, workspaceCwd, attached}` on success. - 500 on bridge failure (the failing channel is killed, not leaked). `runQwenServe` constructs the bridge and ties `bridge.shutdown()` into the listener-close path so SIGINT/SIGTERM drain children before the socket closes. Tests (14 new, 0 regressions in the 4967-test baseline): - 9 bridge cases over an in-memory channel — fresh spawn, single-scope reuse, cross-workspace isolation, thread-scope independence, path canonicalization, relative-path rejection, init failure cleanup, init timeout, multi-channel shutdown. - 4 route cases for /session (missing/relative/200/500). - 1 lifecycle case asserting `runQwenServe.close()` calls `bridge.shutdown()` before closing the listener. Verified end-to-end: `qwen serve` boots, `POST /session` spawns a real `qwen --acp` child and returns the SDK-assigned `sessionId`, repeat calls under the same cwd return `attached: true`, `SIGTERM` reaps the child along with the listener. * feat(cli): wire POST /session/:id/prompt + /cancel for `qwen serve` (#3803) Stage 1 follow-up after the bridge scaffold. Adds the two routes a client needs to actually run a turn against the daemon. Bridge: - `sendPrompt(sessionId, req)` looks up the session, FIFO-queues the call against the per-session prompt queue, and forwards through the SDK `ClientSideConnection.prompt`. Concurrent calls observe ACP's "one active prompt per session" invariant — second waits for first. - A failed prompt does NOT poison the queue; the tail catches and keeps draining so the next caller still runs (the original caller still sees its own rejection). - `cancelSession(sessionId, req?)` bypasses the queue and forwards the ACP notification immediately. ACP semantics: the agent winds down the *currently active* prompt; queued work is unaffected. - Both methods throw `SessionNotFoundError` (a typed Error subclass) when the id is unknown so route handlers can map cleanly to 404 without brittle message matching. - Both methods overwrite the `sessionId` field in the request body with the routing id — a stale or spoofed body would otherwise be dispatched to the wrong agent process. Routes: - `POST /session/:id/prompt` → 200 with PromptResponse, 400 on missing/non-array prompt, 404 on unknown session, 500 on agent error. - `POST /session/:id/cancel` → 204 always (cancel is a notification), 404 on unknown session. Tests (14 new — 7 bridge + 7 route, 0 regressions in the 4981 baseline): - sendPrompt: success forwards & returns response · routing-id overrides body sessionId · concurrent prompts FIFO-serialize (verified via per-prompt start/end ordering with a release latch) · failed prompt doesn't block subsequent prompts · 404 for unknown id. - cancelSession: forwards with routing id · 404 for unknown id. - Routes: 200/400/404/500 paths for prompt; 204 with body or empty + 404 for cancel. Verified end-to-end against a real `qwen --acp` child: - POST /session/:id/prompt with `[{type:'text',text:'hi'}]` → 200 `{"stopReason":"end_turn"}` in ~3.4s. - POST /session/:id/cancel → 204. - POST /session/does-not-exist/prompt → 404 with the unknown id surfaced in the body. * feat(cli): wire SSE streaming for `qwen serve` events (#3803) Stage 1 follow-up that turns prompt into a real streaming experience. Replaces the in-memory `notifications: SessionNotification[]` buffer on each session with a per-session EventBus and exposes it through `GET /session/:id/events` as an `text/event-stream` SSE feed. EventBus (`packages/cli/src/serve/eventBus.ts`): - Monotonic per-session ids (`v: 1` schema). Each `publish` chains an id, returning the materialized BridgeEvent. - Bounded ring (default 1000) backs `Last-Event-ID` reconnect — a consumer that drops can resume from `lastEventId` and replay any still-buffered events before live events flow. - Per-subscriber bounded queue (default 256). When a slow consumer overruns its queue, the bus appends a synthetic `client_evicted` terminal frame and closes that subscription so it can't hold the daemon hostage. Other subscribers are unaffected. - `subscribe()` returns an AsyncIterable — registration is synchronous so events `publish`ed immediately after the subscribe land in the queue (a generator-style implementation deferred registration to first `next()` and raced with publishes). - AbortSignal-aware: aborting the signal closes the iterator promptly. Bridge (`httpAcpBridge.ts`): - `BridgeClient.sessionUpdate` now publishes onto the session's EventBus instead of pushing to a plain array — every ACP notification the agent emits becomes a stream event automatically. - New `subscribeEvents(sessionId, opts?)` returns the bus's AsyncIterable; throws `SessionNotFoundError` for unknown ids. - Shutdown closes every live event bus before killing channels so pending consumers unwind cleanly. Route (`server.ts`): - `GET /session/:id/events` sets the SSE content type, advertises a 3s reconnect hint, and writes a 15s heartbeat comment frame to keep proxy/NAT connections alive. - Forwards the `Last-Event-ID` header to the bus. - `req.on('close')` triggers an AbortController that propagates into the bridge subscription so disconnects don't leak subscribers. - 404 when the bridge can't find the session. Capabilities envelope: `STAGE1_FEATURES` now advertises `session_create`, `session_prompt`, `session_cancel`, `session_events` in addition to `health`/`capabilities` so clients can light up UI for the routes that have actually shipped. Tests (16 new, 0 regressions in the 4995 baseline): - 9 EventBus unit cases — id sequencing, live delivery, replay, replay+live splice, fan-out to N subscribers, eviction on overflow, abort-signal unsubscribe, bus.close() drains subscribers, ring-size eviction. - 4 bridge subscribe cases — 404, sessionUpdate→event publishing via real ACP fake-agent, shutdown closes live subscriptions. - 4 SSE route cases against a live HTTP listener — frame format, Last-Event-ID forwarding, 404, abort propagation on disconnect. Verified end-to-end against a real `qwen --acp` child: - Subscribed to `/session/$SID/events`, fired `POST /session/$SID/prompt` with text content. Captured 13 distinct `event: session_update` SSE frames in real time during the model's response — `available_ commands_update` metadata, 9 `agent_thought_chunk` frames carrying the model's chain-of-thought, 3 `agent_message_chunk` frames with the actual reply, and a final usage frame with token totals. - Frames carry monotonic ids 1..13, the daemon-side counter, and are valid SSE per the EventSource spec. * feat(cli): wire POST /permission/:requestId for `qwen serve` (#3803) Stage 1 follow-up that turns `BridgeClient.requestPermission` from a hardcoded `cancelled` placeholder into a real first-responder vote loop, and ships the HTTP route any attached client uses to cast the deciding vote. Bridge: - `requestPermission` generates a UUID requestId, registers a pending entry on a daemon-wide map (and the owning session's `pendingPermissionIds` set), publishes a `permission_request` event onto the session's EventBus (so SSE subscribers see it), and awaits the resolution. - New `respondToPermission(requestId, response)` resolves the pending promise with the supplied outcome. First call wins — subsequent calls return false. On success the bridge publishes a `permission_resolved` event so other attached clients can update their UI when the race is decided. - `cancelSession` and `shutdown` both resolve every still-pending permission for the affected session(s) as `{ outcome: { outcome: 'cancelled' } }` per the ACP spec requirement that a cancelled prompt MUST resolve outstanding requestPermission calls with cancelled. - New `pendingPermissionCount` getter exposes inflight count for inspection / tests. Route (`server.ts`): - `POST /permission/:requestId` validates the body's `outcome` is either `{ outcome: 'cancelled' }` or `{ outcome: 'selected', optionId: string }`, then forwards to `bridge.respondToPermission`. - 200 on accepted vote, 404 when the requestId is unknown or already resolved (Stage 1 doesn't differentiate), 400 on a malformed outcome. Capabilities envelope: STAGE1_FEATURES gains `permission_vote`. Tests (14 new — 9 bridge + 5 route, 0 regressions in the 5011 baseline): - Bridge: publishes permission_request with a generated requestId and waits; respondToPermission first-responder wins; publishes permission_resolved on vote; respondToPermission false for unknown requestId; cancelSession resolves outstanding as cancelled; shutdown resolves outstanding as cancelled. - Route: 200 on selected outcome; 200 on cancelled outcome; 404 on unknown requestId; 400 on malformed outcome; 400 on missing outcome. Verified end-to-end against a real `qwen --acp` child: - Subscribed to /session/$SID/events, sent a prompt asking the agent to write a file at /tmp/qwen-serve-permission-e2e-test.txt. - The agent triggered a permission_request via the bus, surfacing the three options Qwen Code presents (Allow Always / Allow / Reject) with their option ids. - POSTed `{outcome:{outcome:"selected",optionId:"proceed_once"}}` to /permission/$requestId — got HTTP 200. - Bus published the matching permission_resolved event. - Agent proceeded with the writeTextFile tool call; file was actually created on disk with the expected content. * feat(sdk): add DaemonClient for the qwen serve HTTP API (#3803) Stage 1 follow-up that proves the cross-mode protocol-isomorphism design assumption: an SDK client can drive the daemon's HTTP routes end-to-end without going through ProcessTransport's stdio + stream-json path. DaemonClient is a sibling of ProcessTransport, not a replacement. The two speak different protocols (ACP NDJSON over HTTP vs stream-json over stdio). Existing `query()` users keep getting subprocess-mode unchanged; applications that want daemon-mode (cross-client attach, shared MCP pool, network reachability, first-responder permissions) opt in by constructing a DaemonClient against a running `qwen serve`. API surface (`packages/sdk-typescript/src/daemon/`): - `new DaemonClient({ baseUrl, token?, fetch? })`. The `fetch` override is for tests; defaults to `globalThis.fetch`. Trailing slashes on `baseUrl` are stripped. - `health()`, `capabilities()` — discovery. - `createOrAttachSession({ workspaceCwd, modelServiceId? })` — `attached: true` on the response indicates a session was reused under sessionScope:single. - `prompt(sessionId, { prompt: ContentBlock[] })` — returns PromptResult with stopReason. - `cancel(sessionId)` — tolerates 204; throws on 404. - `subscribeEvents(sessionId, { lastEventId?, signal? })` — async iterator over parsed SSE frames; AbortSignal-aware. Native Node AbortController only — jsdom polyfills are incompatible with undici. - `respondToPermission(requestId, response)` — first-responder vote; returns true on 200, false on 404 (lost the race or unknown id), throws on 400/500. `DaemonHttpError` is thrown for any non-2xx (besides the 404 "already-resolved" case on permission votes); carries `status` and `body` so callers can branch on standard daemon HTTP semantics. `parseSseStream(body)` is the underlying SSE parser; exported separately so applications can consume daemon SSE outside the DaemonClient surface. Handles split-chunk frames, comment/retry directives, malformed JSON (skip), trailing frame without final newline. Wire types live SDK-side (no SDK→CLI dep); the capabilities envelope's `v` field signals breaking changes. Tests (26 new, 0 regressions in the 201 baseline): - 7 SSE parser cases — single frame, multiple frames, comments, chunked-split frame, malformed JSON skip, trailing frame on close, empty stream. - 19 DaemonClient cases — health success/error, capabilities, bearer auth presence/absence, createOrAttachSession success/400, prompt body shape + sessionId url-encoding, cancel 204/404, permission 200/400/404, subscribeEvents header forwarding + 404, baseUrl normalization. Verified end-to-end against a real `qwen serve` daemon driving a real `qwen --acp` child: - `client.capabilities()` returned `{v:1, mode:"http-bridge", features: [...7 tags]}`. - First `createOrAttachSession` returned `attached:false`; second returned `attached:true` with the same sessionId. - `client.prompt(...)` with text content yielded `{stopReason: "end_turn"}` while the parallel `subscribeEvents` iterator streamed 10 distinct frames during the same turn. - AbortController on the events iterator cleanly severed the SSE connection. * feat(cli,sdk): list workspace sessions + set session model (#3803) Closes the §04 Stage-1 routes table for `qwen serve` with the two remaining endpoints, plus matching SDK methods. `GET /workspace/:id/sessions` - `:id` is the URL-encoded canonical absolute workspace path (Express decodes path params automatically; clients pass `encodeURIComponent(cwd)`). - Returns `{ sessions: [{ sessionId, workspaceCwd }, ...] }` for live sessions whose canonical workspace matches. - Empty array (not 404) when the workspace is idle so picker UIs don't have to special-case "no sessions yet". - 400 when the decoded path isn't absolute. `POST /session/:id/model` - Body: `{ modelId: string, ... }`. The route's `:id` overrides any spoofed sessionId in the body. - Forwards to ACP's `unstable_setSessionModel` and publishes a `model_switched` event onto the session bus so cross-client UIs update. - 200 with the agent's response on success, 400 on missing/empty modelId, 404 on unknown session. - The SDK method is currently unstable; documented in the bridge comment in case the spec renames the method when it stabilizes. Bridge: - New `listWorkspaceSessions(workspaceCwd)` iterates `byId.values()` and filters by canonical workspace path; works for both `single` and `thread` session scopes. - New `setSessionModel(sessionId, req)` forwards through `connection.unstable_setSessionModel`, normalizes sessionId, publishes `model_switched`, throws SessionNotFoundError on unknown ids. `STAGE1_FEATURES` capabilities envelope grows to 9 tags, adding `session_list` and `session_set_model`. SDK (`DaemonClient`): - `listWorkspaceSessions(workspaceCwd)` URL-encodes the cwd and returns the parsed `sessions` array directly. - `setSessionModel(sessionId, modelId)` POSTs the body and returns the agent response (currently opaque per ACP unstable spec). - Wire types `DaemonSessionSummary` and `SetModelResult` exported from the SDK barrel. Tangential cleanup: `sendBridgeError` now extracts a useful message from non-Error values via a small `errorMessage` helper. JSON-RPC errors from the agent (`{code, message, data}`) used to surface as `"[object Object]"` in the 500 response body; they now show the inner `message` field. Caught while running the model-set e2e. Tests (17 new — 9 bridge + 7 route + 4 SDK, 0 regressions in the 5022 + 227 baselines): - Bridge listWorkspaceSessions: matching cwd returns the live sessions; canonicalizes the lookup; empty for relative paths. - Bridge setSessionModel: forwards modelId + overrides body sessionId; publishes model_switched event; 404 unknown session. - Route /workspace/:id/sessions: returns the bridge list; empty for idle workspace; 400 for relative path. - Route /session/:id/model: 200 success; 400 missing modelId; 400 empty modelId; 404 unknown session. - SDK listWorkspaceSessions: URL-encodes the cwd; throws on 400. - SDK setSessionModel: posts body; throws on 404. Verified end-to-end against a real `qwen serve`: - SDK reports 9 capability features, list returns the existing session, attached:true on repeat create, and `setSessionModel` rejects with HTTP 500 when the modelId isn't registered (with the daemon now surfacing "Internal error" instead of "[object Object]"). - 404 path through SDK on unknown sessionId works. * fix(cli,sdk): audit round 1 follow-ups for `qwen serve` (#3803) Self-review pass on PR #3889. Two real correctness bugs and an ergonomics gap, plus the test-coverage holes the audit surfaced. The loudest finding ("host allowlist no-op when bind=localhost") was a false positive — the conditional was misread; existing tests already prove the validator is active on `localhost` binds. Real fixes: - Bearer-auth timing-attack: `parts[1] !== token` short-circuits per byte, leaking which prefix is correct via response latency. Replace with SHA-256 of both sides + `crypto.timingSafeEqual` so comparison is constant-time regardless of token length. - Concurrent `spawnOrAttach` race in single-scope: two parallel callers for the same workspace both passed the `byWorkspace.get` check, both spawned, and one entry ended up orphaned in `byId` while the other won `byWorkspace`. Violates the "at most one session per workspace" invariant. Coalesce via an `inFlightSpawns` map: parallel callers attach to the in-flight promise and report `attached: true`. The slot is cleared on both success and rejection so a failed spawn doesn't poison the workspace forever. New test asserts ONE channel spawns under parallel calls and that retry works after rejection. - `Number.parseInt('1.5e10z', 10)` returns 1, so a malformed `Last-Event-ID` header silently passes through. Tighten `parseLastEventId` to `^\d+$` so anything not a pure decimal integer is dropped. New test exercises 'abc', '-1', '1.5e10z'. Ergonomics: - `LOOPBACK_BINDS` and `LOOPBACK_HOST_BINDS` now include `::1` and `[::1]`. IPv6 loopback users no longer have to set a token. Host-allowlist allows `[::1]:port` Host headers. Documentation: - `BridgeClient` doc-comment now states the Stage 1 trust model explicitly: agent runs as the same UID, the file-proxy methods are NOT a workspace-cwd sandbox, restricting them would be theatre. The audit flagged this as a "design gap" but the daemon-and-agent-on-same-host posture makes a sandbox here redundant — Stage 4+ remote-sandbox swaps the Client for a sandbox-aware variant. SDK fix: - `DaemonClient.failOnError` previously called `res.json()`, which consumes the body even on parse-failure; the subsequent `res.text()` returned empty. New impl reads once as text and attempts JSON-parse; raw text is the fallback. New test asserts a `text/plain` 502 surfaces the body verbatim. Test gap fills (audit-flagged): - Bridge: in-memory file-proxy tests for `BridgeClient.{read,write} TextFile` including line/limit slicing. - SSE route: `stream_error` synthetic frame on iterator throw mid-stream; numeric Last-Event-ID forwarded; malformed Last-Event-ID dropped. - DaemonClient: text/plain error body coerced to `body` field; `respondToPermission` 5xx throws; `subscribeEvents` null-body throws; `cancel`/`respondToPermission` URL-encode session/request ids that contain slashes. Verified end-to-end with a token-required daemon: right token → 200, wrong/missing/malformed → 401. All paths return uniform 401 messages so a side-channel can't distinguish between "no header", "bad scheme", and "wrong token". Test counts: cli serve **89** (was 81, +8), sdk daemon **35** (was 30, +5). Full suites still green. * fix(cli): audit round 2 follow-ups for `qwen serve` (#3803) Second self-review pass on PR #3889. Three real bugs (one correctness, one resource-cleanup, one cosmetic) plus consolidation of the loopback bindings into a single source of truth. Real fixes: - Shutdown could hang forever on a long-lived SSE consumer: `server.close` waits for every in-flight connection to drain, and a paused EventSource client never disconnects. Added a `SHUTDOWN_FORCE_CLOSE_MS` (5s) timer that calls `server.closeAllConnections()` to force-destroy stuck sockets, then resolves so `process.exit(0)` can run. New test asserts close completes well under 5.5s even when an SSE GET is in flight. - Signal-handler race during shutdown: round 1 detached the SIGINT/SIGTERM listeners *up front* in `handle.close()`. If a second SIGTERM arrived during the drain, no handler existed and Node's default termination ran, orphaning agent children. Switch to detaching at the *end* of the close path (in `finish()`): during the drain window the handler is still attached and the `if (shuttingDown) return` guard makes a second signal a no-op; after drain completes we can safely remove the listeners (this also fixes a test-suite MaxListenersExceededWarning that fired once we ran the runQwenServe tests >10 times in a single process). - SSE response had no `error` listener. When the underlying TCP socket died (RST, kill -9 on the client), the next `res.write` threw EPIPE and Express forwarded it to the default error handler, logging noisily. Added `res.on('error', cleanup)` so the failure is absorbed and triggers the same teardown path the `req.on('close')` handler uses. Validation: - `createHttpAcpBridge` now throws on invalid `sessionScope` (anything other than `'single'` or `'thread'`) and on `initializeTimeoutMs <= 0`. Misconfigured callers used to silently degrade to thread behavior; now they fail loudly. Cleanup: - The `LOOPBACK_BINDS` set was duplicated between `auth.ts` and `runQwenServe.ts` (round 1 missed this). Extracted into `packages/cli/src/serve/loopbackBinds.ts` with a single `isLoopbackBind(hostname)` helper. Both files now import; drift is impossible. - `res.flushHeaders?.()` lost the optional chaining. The method is on `http.ServerResponse` since Node 1.6; our `engines` floor is 20. Tests added: - bridge: `sessionScope` validation, `initializeTimeoutMs` validation. - server: shutdown force-close timeout, SIGINT/SIGTERM listener detach-after-drain. False positives from the round 2 audit (verified and dismissed): - "EventBus nextId overflow at 2^53" — theoretical only (would require ~9 quadrillion publishes per session). No code change. - "Subscribe-during-close race" — JS is single-threaded; the close() flag is set synchronously before the loop touches state. - "Queued prompts on shutdown" — by design; documented via the promptQueue tail comment. - "10MB body parser limit" — design choice for Stage 1's in-memory buffering model; revisit if ACP streaming lands in Stage 2. - "Unbounded body read in DaemonClient.failOnError" — daemon is local in Stage 1; the threat surface for adversarial-large error bodies is the same as the daemon's other unbounded buffers. Test counts: cli serve **93** (was 89, +4), full cli **5047** (no regressions), sdk **236** (no regressions). * docs(cli): audit rounds 3 + 4 follow-ups for `qwen serve` (#3803) Two more self-review passes on PR #3889. No correctness bugs surfaced this time — round 3 found a HIGH-severity Windows-path claim that turned out to be a false positive (`path.win32.isAbsolute('/foo/bar')` returns true; verified against Node 20). Round 4 confirmed every prior decision and surfaced one latent-but-not-currently-triggered concurrency note. Changes are pure documentation + a tiny optional-chain cleanup: - Drop `?.` on `server.closeAllConnections()` in runQwenServe.ts — the method exists since Node 18.2 and our `engines` floor is 20. The optional chain dated from before round 2's force-close timer landed; clean it up. - Help text for `qwen serve --port` now documents that port 0 means "OS-assigned ephemeral port" (which the implementation has always supported but never advertised). - `defaultSpawnChannelFactory` gains a comment near the spawn site documenting the FD-budget implication (~3 FDs per session, bump `ulimit -n` for many concurrent sessions) and the `stdio: ['pipe', 'pipe', 'inherit']` choice (child stderr lands in the daemon's stderr, interleaved across sessions). Both are Stage-1-accepted; Stage 2/4+ revisit each. - Comment on the bridge's `byWorkspace`/`byId` Maps documenting the known gap that a child crashing between requests leaves a garbage SessionEntry until daemon shutdown — surfaced as a per-prompt failure when the dead session is touched, not a hang. Stage 2's in-process bridge eliminates the spawned-child failure mode entirely so this gap goes away naturally. - `EventBus.subscribe` doc-comment now states explicitly that the returned iterator is NOT safe to drive from concurrent `.next()` callers — the underlying queue isn't atomic. Daemon usage is the sequential `for await ... of` inside the SSE route, so this is safe in production. Documented so a future fan-out consumer doesn't accidentally rely on undefined behavior. False positives verified and dismissed (round 3 + 4 combined): - `path.isAbsolute('/foo/bar')` Windows breakage — `path.win32. isAbsolute('/foo/bar')` is true; verified empirically. - "Windows drive divergence" causing duplicate sessions — different drives are different on-disk paths; sessions intentionally differ. - "parseSseStream early-break leaks reader" — `for await ... break` triggers `iterator.return()` which runs the generator's `finally` that calls `releaseLock`. Standard JS semantics. - "Promise executor sync-throw fragility in requestPermission" — sync throws inside `new Promise(executor)` reject the outer promise; functionally correct, just stylistic. - "Force-close timeout test elapsed assertion flakiness" — assertion is `< 5500ms` but the natural happy-path is sub-100ms. Generous headroom; not flake-prone in practice. - "fetch reference stale after polyfill" — `globalThis.fetch.bind` captures at construction; tests inject `opts.fetch` instead of polyfilling, which is the correct pattern. Test counts unchanged (cli serve **93**, sdk **236**); typecheck + lint clean. STAGE1_FEATURES still matches every implemented route 1:1, fakeBridge in tests implements every HttpAcpBridge method. * fix(cli): PR #3889 review round 1 — critical correctness (#3803) Addresses the four critical findings from the PR #3889 reviewer pass: 1. ACP `ReadTextFileRequest.line` is 1-based per spec, but the bridge's `BridgeClient.readTextFile` was treating it as a 0-based slice index. A client asking for `{line:1, limit:2}` ("first two lines") was getting lines 2-3 — a sign-off-by-one bug that breaks every editor / SDK client following the ACP schema. Convert to 0-based via `Math.max(0, line - 1)`. The existing slice test was asserting the wrong behavior; updated to expect the spec-correct result and added a second `line:3, limit:2` case to lock in the offset. 2. `modelServiceId` was accepted by the SDK + server `POST /session` path, forwarded into `bridge.spawnOrAttach`, and then silently dropped: `doSpawn` never wired it into the agent. Callers requesting a specific model got the agent's default and no indication anything was wrong. Now `doSpawn` issues `unstable_setSessionModel` immediately after `newSession`. If the agent rejects the model id, the half-initialized session is torn down and the spawn rejects so the caller can retry cleanly instead of inheriting silent drift. Three new bridge tests: happy path, omit-when-undefined, agent-rejection cleanup. 3. The CORS middleware used `cors({ origin: (o, cb) => cb(new CORSError(...), false) })` for browser-Origin requests. `cors` flows the Error into Express's error chain; without an explicit error handler that produces a 500 + HTML body, which is misleading for what is really a deterministic 403 denial. Replace with a tiny `RequestHandler` that checks `req.headers.origin` directly and returns `403 { error: 'Request denied by CORS policy' }` JSON. Drops the `cors` and `@types/cors` dependencies — there's no other consumer in the cli package. 4. The SSE `stream_error` synthetic frame hard-coded `id: 0`, which would regress the client's `Last-Event-ID` tracker and trigger duplicate replays on reconnect. The frame is terminal and daemon-emitted — it has no place in the per-session monotonic sequence. Refactor `formatSseFrame` to omit the `id:` line when the input event has no id field, and emit `stream_error` without one. Test updated to assert `frames[1].id === undefined` while the preceding `session_update` still carries its monotonic id. Tangential cleanup: `errorMessage` now formats the SSE error body (was `err.message` only — would have shown `[object Object]` for JSON-RPC errors mid-stream, mirroring the round-1 SDK fix). Test counts: cli serve **96** (was 93, +3 modelServiceId cases); existing readTextFile slice test rewritten in place. Full typecheck + lint + suite green. * fix(cli,sdk): PR #3889 review round 2 — SSE robustness + EventBus polish (#3803) Second batch of reviewer-flagged fixes for PR #3889. Addresses 7 robustness issues across the daemon's SSE pipeline + the bus + the SDK's stream parser. Daemon SSE (`server.ts`): - SSE writes now respect backpressure. `res.write` returns false when the kernel send buffer is full; the previous code ignored that and Node accumulated payloads in user-space memory unboundedly. A slow consumer on a chatty session could balloon daemon RSS. New `writeWithBackpressure` helper awaits `drain` (or `close`/`error`) before scheduling the next write — for both per-frame writes and heartbeats. - `parseLastEventId` rejects values > `Number.MAX_SAFE_INTEGER`. With the prior `^\d+$` regex a malicious 25-digit value would parse to a number that loses precision and confuses replay comparisons. EventBus (`eventBus.ts`): - `Last-Event-ID` replay events now `forcePush` past `maxQueued`. A client reconnecting with a 1000-event gap on a subscriber whose cap is 256 was silently losing entries 257-1000 — a sign-off-by- nothing breakage of the resume contract. Live publishes still go through the normal cap (slow live consumer must be evictable); historical replay is bypassed. - `onAbort` now disposes the subscription immediately instead of only closing the queue. An aborted-but-never-iterated subscriber used to linger in `bus.subs` until the consumer drove `next()` / `return()`. New tests cover both abort-after-subscribe and already-aborted-at-subscribe paths. - `BoundedAsyncQueue.next` now checks `buf.length > 0` before shifting instead of `buf.shift() !== undefined`. The bus never pushes `undefined` today but the queue is generic — the prior pattern would mis-handle a queue whose element type legitimately includes undefined. SDK SSE parser (`sse.ts`): - Now flushes the TextDecoder on stream close. Without the final `decoder.decode()`, an incomplete multi-byte UTF-8 sequence at the tail of the last chunk was silently dropped — corrupting any frame whose JSON ended mid-character. New test feeds a stream split mid-byte through "中" (3-byte UTF-8) and asserts the character round-trips. - Frame separators now accept both `\n\n` and `\r\n\r\n`. SSE spec allows CRLF, and intermediaries (corporate proxies, some Node http servers) sometimes normalize. Frame field splitter also accepts `\r?\n`. Two new tests cover pure CRLF + mixed-LF/CRLF. Test counts: cli serve **99** (was 96, +3 EventBus); sdk daemon-sse **10** (was 7, +3). Full typecheck + lint + suite green. * docs(cli,sdk): PR #3889 review round 3 — minor + docs (#3803) Last batch from the PR #3889 reviewer pass: mostly docs + a ReDoS-tooling-silencing rewrite + a yargs-key cleanup. - `commands/serve.ts` ServeArgs interface dropped the camelCase `httpBridge` mirror; the handler now reads `argv['http-bridge']` matching the declared option name. The dual surface relied on yargs's camelCase expansion behavior — fragile if yargs config ever changes. - `DaemonClient` constructor's `baseUrl.replace(/\/+$/, '')` (which is end-anchored and linear, but CodeQL's polynomial-regex detector flags any `\/+$` pattern on attacker-controlled input) swapped for a hand-rolled `stripTrailingSlashes` loop. Same behavior, no rule trigger. - `defaultSpawnChannelFactory`'s `cwd: workspaceCwd` flow into `spawn` is the second CodeQL finding ("uncontrolled data used in path expression"). It IS user-controlled, by design — that's the Stage 1 trust model. Added a `// lgtm[js/shell-command- constructed-from-input]` suppression with a comment explaining the model and pointing at issue #3803 §11 for the Stage 4+ remote- sandbox replacement. - Stale doc comment on `createServeApp` that still listed only `/health`, `/capabilities`, `POST /session` as shipped — now enumerates all 9 routes that match §04 of the design. - Stale doc comment on `HttpAcpBridge` saying "Stage 1 buffers them in-memory; SSE wiring lands in the next PR" — SSE wiring landed in commit |
||
|
ChiGao
|
cadda23782
|
chore(deps): upgrade ink 6.2.3 → 7.0.2 + bump Node engine to 22 (#3860)
* chore(deps): upgrade ink 6.2.3 -> 7.0.2 + bump Node engine to 22
ink 7 requires Node >=22 and react-reconciler 0.33 with React >=19.2,
so this PR also bumps:
- Node engines (root + cli + core) 20 -> 22
- React/react-dom 19.1 -> 19.2.4 (pinned exact via overrides to keep
the transitive React graph deduped to a single instance)
- @types/node pinned to 20.19.1 via overrides to avoid an unrelated
Dirent NonSharedBuffer regression in sessionService tests
- @vitest/eslint-plugin pinned to 1.3.4 to avoid an unrelated lint
regression introduced by the 1.6.x rule additions
- react-devtools-core 4.28 -> 6.1 (ink 7 peerOptional requires >=6.1.2)
- ink hoisted to root devDeps so workspace-private peer-dep contention
doesn't push ink-link/spinner/gradient into nested workspace
installs (which would skip transitive resolution for terminal-link)
Workflow + image + installer alignment:
- .nvmrc 20 -> 22
- Dockerfile node:20-slim -> node:22-slim
- CI test matrix drops 20.x (keeps 22.x + 24.x)
- terminal-bench workflow Node 20 -> 22
- Linux/Windows install scripts upgrade their Node version targets
Documentation alignment:
- README.md badge + prerequisites
- AGENTS.md, CONTRIBUTING.md, docs/users/quickstart.md,
docs/users/configuration/settings.md, docs/developers/contributing.md,
docs/developers/sdk-typescript.md, docs/users/extension/extension-releasing.md,
packages/sdk-typescript/README.md, packages/zed-extension/README.md,
scripts/installation/INSTALLATION_GUIDE.md
Test gating:
- Two AuthDialog/AskUserQuestionDialog tests that drive <SelectInput>
through ink-testing-library now race ink 7's frame-throttled input
delivery and land on the wrong option. The maintainers had already
marked one of them unreliable (skip on Win32 + CI+Node20). Extend
that gate to cover all environments until upstream
ink-testing-library ships an ink-7-compatible release that flushes
input deterministically. The other test now uses it.skip with the
same comment. No business code changes.
Verified locally:
- npm run typecheck across all workspaces: clean
- npm run lint (root): clean
- npm run test --workspaces:
cli 312/312 files, 4918 passed, 9 skipped
core 266/266 files, 6836 passed, 3 skipped
webui 6/6, 201 passed
sdk 40/40, 283 passed, 1 skipped
- npm ls ink: single ink@7.0.2 instance across all peer deps
- single react@19.2.4 instance
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* chore: align Node 22 floor across all shipping artifacts
Reviewer (tanzhenxin) flagged five surfaces where the >=22 engine bump
leaked: SDK package metadata, web-templates engines, /doctor runtime
check, main bundler target, and SDK bundler target. Each was a separate
escape hatch letting Node 18/20 consumers install or run the artifact
on an unsupported runtime.
- packages/sdk-typescript/package.json: engines.node >=18.0.0 -> >=22.0.0
- packages/web-templates/package.json: engines.node >=20 -> >=22
- packages/cli/src/utils/doctorChecks.ts: MIN_NODE_MAJOR 20 -> 22
- esbuild.config.js: target node20 -> node22 (main CLI bundle)
- packages/sdk-typescript/scripts/build.js: target node18 -> node22 (esm + cjs)
- packages/cli/src/utils/doctorChecks.test.ts: rename test label to v22+
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* ci(e2e): bump E2E workflow Node matrix to 22.x
Reviewer (tanzhenxin) flagged that e2e.yml still pinned node-version
20.x while root engines is now >=22, so every E2E run on push would
either fail at npm ci with engine error or silently exercise the bundle
on a runtime that's no longer in ci.yml's test matrix.
The macOS job in the same workflow already reads .nvmrc (which is 22)
so this only updates the Linux matrix.
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(deps): drop root wrap-ansi override so ink 7 gets its declared dep
Reviewer (tanzhenxin) flagged that the root overrides.wrap-ansi: 9.0.2
predates this upgrade and forces every consumer (including ink) to v9,
while ink 7 declares wrap-ansi: ^10.0.0. The lockfile had no nested
install under node_modules/ink/, so ink 7 was running with a transitive
dep one major below its declared minimum.
Dropping the global override lets ink resolve its own wrap-ansi 10
nested install (now visible in the lockfile under
node_modules/ink/node_modules/wrap-ansi), while the cli package's own
direct `wrap-ansi: 9.0.2` dependency keeps the cli code path
(TableRenderer.tsx) on the version it has been tested against. The
nested cliui override is preserved for yargs which still needs v7.
Verified via `npm ls wrap-ansi`:
- ink@7.0.2 -> wrap-ansi@10.0.0 (newly nested)
- @qwen-code/qwen-code -> wrap-ansi@9.0.2 (unchanged)
- yargs/cliui -> wrap-ansi@7.0.0 (unchanged)
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* test(InputPrompt): un-skip placeholder ID reuse after deletion
Reviewer (tanzhenxin) flagged that the new it.skip on the
'should reuse placeholder ID after deletion' test was undisclosed in
the PR description and removed coverage of real product behavior
(freePlaceholderId / bracketed-paste backspace path) without a
TODO(#NNNN) link.
Their argument was sound: the skip rationale pointed at ink 7's input
throttle, but this same file just bumped the wait helper from 50ms to
150ms specifically to give ink 7 frame time. Re-running the test under
the bumped wait shows it passes reliably (5/5 runs in the full-file
context, 9/10 alone), so the skip was masking the throttle-flake that
the wait bump already addresses, not a real product bug.
Drop the it.skip and the now-stale comment so coverage of the
freePlaceholderId reuse logic is restored.
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* test(InputPrompt): bump first prompt-suggestion test wait to 350ms
The "accepts and submits the prompt suggestion on Enter when the buffer
is empty" test is the first in its describe block, so it pays the
renderer cold-start cost. On macOS-22.x CI runners that pushes the
Enter → onSubmit microtask past the default 150ms post-Enter wait. Match
the 350ms initial render wait used immediately above to absorb the cold
start.
* Revert "test(InputPrompt): bump first prompt-suggestion test wait to 350ms"
This reverts commit
|
||
|
jinye
|
2c93fd670c
|
refactor: extract shared release helper utilities (#3834)
Move four duplicated utility functions (getArgs, readJson,
validateVersion, isExpectedMissingGitHubRelease) from the three
get-release-version.js scripts into a shared module at
scripts/lib/release-helpers.js so that changes only need to happen
in one place.
Also fixes a pre-existing bug in getArgs where argument values
containing '=' were silently truncated (e.g. --msg=a=b produced
{msg:'a'} instead of {msg:'a=b'}).
Closes #3795
🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
Co-authored-by: jinye.djy <jinye.djy@alibaba-inc.com>
|
||
|
jinye
|
03f66bada5
|
feat(sdk-python): add PyPI release workflow (#3685)
* feat(sdk-python): add pypi release workflow Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk-python): build cli before smoke test Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk-python): tighten release conflict handling Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk-python): harden python release workflow Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk-python): tighten stable release guards Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk-python): harden prerelease publish flow Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk-python): reuse release branches on rerun Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk-python): resume incomplete releases Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(release): tighten missing-release checks Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk-python): resume stable release reruns Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk-python): tighten release recovery guards Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(sdk-python): cover release version edge cases Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk-python): address release workflow review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * refactor(sdk-python): address review feedback on release version script - Remove unreachable `if (type === 'stable')` branch in bumpVersion(); the stable path was dead code since getVersion() throws for all stable conflicts before calling bumpVersion(). Move nightly conflict throw to the call site for symmetry. - Rename getNextPatchBaseVersion → getNextBaseVersion to reflect that the function can return a prerelease base without incrementing patch. - Add test for preview+nightly coexistence where nightly base is higher. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(sdk-python): address remaining review feedback on release workflow - Fix failure-issue gate to read github.event.inputs.dry_run directly instead of steps.vars.outputs.is_dry_run (which is empty when early steps fail). Add --repo flag for gh issue create when checkout failed. - Add diagnostic state table to failure-issue body (RELEASE_TAG, PACKAGE_VERSION, PUBLISH_CHANNEL, RESUME_EXISTING_RELEASE, etc.) - Fix release-notes error swallow: only silence release not found / Not Found / HTTP 404, emit :⚠️: for other gh release view errors. - Improve validateVersion error messages to use human-readable format keys (X.Y.Z, X.Y.Z-preview.N) matching TS sibling convention. - Filter fully-yanked versions in getAllVersionsFromPyPI. - Add console.error log when stable is derived from nightly. - Add bash regex guard for inputs.version to prevent shell injection. - Use per-release-type concurrency groups (nightly/preview/stable). - Add jq null-guard checks for all 6 field extractions. - Remove misleading --follow-tags from git push (lightweight tags). 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(sdk-python): rename misleading test description The test asserts that preview/nightly releases return empty previousReleaseTag, but the name said "same-channel previous release tags" which implied non-empty values. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(sdk-python): address unresolved review comments on release workflow - Remove -z check in extract_field() that blocked preview/nightly releases (previousReleaseTag is legitimately empty for non-stable releases) - Use static environment.url since step outputs aren't available at job startup - Use skip-existing for resumed PyPI publish to fill in missing artifacts - Add AbortSignal.timeout(30s) to PyPI fetch to prevent indefinite hangs - Add downgrade guard for stable_version_override - Use GHA :⚠️: annotation instead of console.error for visibility - Separate yanked/non-yanked version lists so conflict detection includes yanked versions (PyPI still reserves those slots) - Filter current release from previousReleaseTag to avoid self-reference on resume - Add tests for yanked conflict detection, downgrade guard, and resume previousReleaseTag Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk-python): address final review round on release version script - Fix getNextBaseVersion() first-release skip: use pyproject.toml version directly when PyPI has no stable versions instead of unconditionally incrementing - Fix getNextBaseVersion() off-by-one: change > to >= so equal prerelease base continues the existing line instead of incrementing patch - Add :⚠️: annotation when preview auto-bumps due to orphan git tags (tag exists without PyPI version or GitHub release) - Add set -euo pipefail to 5 workflow steps missing it: release_branch, persist_source, Create GitHub release, Delete prerelease branch, Create issue on failure - Fix 2 existing tests affected by first-release change, add 4 new tests 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(sdk-python): use stderr for GHA warning annotations to avoid corrupting JSON stdout console.log writes to stdout, which gets captured by VERSION_JSON=$(node ...) in the workflow and corrupts the JSON output for jq. Switch to console.error so :⚠️: annotations go to stderr (GHA recognizes workflow commands on both streams). Also add set -euo pipefail to the "Get the version" step for consistency with other workflow steps. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) --------- Co-authored-by: jinye.djy <jinye.djy@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
qwen-code-ci-bot
|
96116dc76f
|
chore(release): sdk-typescript v0.1.7 (#3688)
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> |
||
顾盼
|
aeeb2976d6
|
feat(web-search): remove built-in web_search tool, replace with MCP-based approach (#3502)
* feat(web-search): add GLM (ZhipuAI) web search provider - Add GlmProvider class implementing BaseWebSearchProvider using the ZhipuAI Web Search API (https://open.bigmodel.cn/api/paas/v4/web_search) - Support multiple search engines: search_std, search_pro, search_pro_sogou, search_pro_quark - Support optional config: maxResults, searchIntent, searchRecencyFilter, contentSize, searchDomainFilter - Truncate query to 70 characters per API limit - Register 'glm' in the provider discriminated union (types.ts) and createProvider() switch (index.ts) - Add GlmProviderConfig to settingsSchema, ConfigParams, and Config class - Add --glm-api-key CLI flag and GLM_API_KEY env var support in webSearch.ts - Forward GLM_API_KEY in sandbox environment - Update provider priority list: Tavily > Google > GLM > DashScope - Add 17 unit tests for GlmProvider and 4 integration tests in index.test.ts - Update docs/developers/tools/web-search.md with GLM configuration, env vars, CLI args, pricing, and corrected DashScope billing info - Fix stale OAuth/free-tier references in web-search.md Closes #3496 * docs(web-search): fix DashScope note and add GLM server-side limitations * fix(web-search): make DashScope provider work with standard API key, remove qwen-oauth dependency - DashScopeProvider.isAvailable() now checks config.apiKey instead of authType - Remove OAuth credential file reading and resource_url requirement - Use standard DashScope endpoint: dashscope.aliyuncs.com/api/v1/indices/plugin/web_search - Read DASHSCOPE_API_KEY env var and --dashscope-api-key CLI flag - Forward DASHSCOPE_API_KEY into sandbox environment - Update integration test to detect DASHSCOPE_API_KEY - Update docs to reflect new API key based configuration * feat(web-search): remove built-in web search tool The web_search tool and all related provider implementations are removed. Web search functionality will be provided via MCP integrations instead, which is the direction the broader agent ecosystem is moving. Removed: - packages/core/src/tools/web-search/ (entire directory) - packages/cli/src/config/webSearch.ts - integration-tests/cli/web_search.test.ts - ToolNames.WEB_SEARCH, ToolErrorCode.WEB_SEARCH_FAILED - webSearch config in ConfigParams, Config class, settingsSchema - CLI options: --tavily-api-key, --google-api-key, --google-search-engine-id, --glm-api-key, --dashscope-api-key, --web-search-default - Sandbox env forwarding for TAVILY/GLM/DASHSCOPE/GOOGLE search keys - web_search from rule-parser, permission-manager, speculation gate, microcompact tool set, and builtin-agents tool list * fix: remove websearch reference * docs: remove websearch tool * docs: add break change guide * fix review |
||
chinesepowered
|
ed6f9e056e
|
fix(sdk): settle pending next() promise in Stream.return() to prevent hangs (#2981)
* fix(sdk): settle pending next() promise in Stream.return() to prevent hangs * test(sdk): add regression tests for Stream.return() pending-promise cleanup |
||
Reid
|
d439e7d738
|
fix(sdk): avoid leaking process exit listeners in ProcessTransport (#3295)
* fix(sdk): avoid leaking process exit listeners in ProcessTransport * Strengthen ProcessTransport cleanup during process exit. This updates the shared process-exit cleanup path to use a best-effort SIGTERM/SIGKILL sequence and adds coverage to verify the global exit handler terminates all active child processes. It keeps the listener leak fix in place while closing the remaining gaps found in review. |
||
|
Shaojin Wen
|
1486e85385
|
feat(cli/sdk): expose /context usage data in non-interactive mode and SDK API (#2916)
Some checks are pending
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / Lint (push) Waiting to run
Qwen Code CI / Test (push) Blocked by required conditions
Qwen Code CI / Test-1 (push) Blocked by required conditions
Qwen Code CI / Test-2 (push) Blocked by required conditions
Qwen Code CI / Test-3 (push) Blocked by required conditions
Qwen Code CI / Test-4 (push) Blocked by required conditions
Qwen Code CI / Test-5 (push) Blocked by required conditions
Qwen Code CI / Test-6 (push) Blocked by required conditions
Qwen Code CI / Test-7 (push) Blocked by required conditions
Qwen Code CI / Test-8 (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* feat(cli): implement non-interactive /context output and diagnostic - Extract collectContextData() from contextCommand.ts for shared usage. - Register /context in ALLOWED_BUILTIN_COMMANDS_NON_INTERACTIVE. - Extend SDK control protocol with GET_CONTEXT_USAGE request. - Implement handleGetContextUsage in SystemController for programmatic token queries. - Expose getContextUsage() method in the TypeScript SDK Query interface. * fix: address review feedback and fix critical bugs in context usage feature - Add missing `get_context_usage` route in ControlDispatcher (SDK calls would throw) - Fix `executionMode` defaulting: use `?? 'interactive'` to match other commands - Validate dynamic import of `collectContextData` before invoking - Preserve original error message in handleGetContextUsage catch block - Add ControlDispatcher test for get_context_usage routing - Add JSDoc comment for context command in non-interactive allowlist Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: re-check abort signal after async operations in handleGetContextUsage Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: add getContextUsage() to SDK TypeScript documentation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: clarify getContextUsage showDetails is a display hint, not a data filter Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: make showDetails affect response shape, add getContextUsage test - When showDetails is false, return empty detail arrays instead of full data so /context and /context detail produce different payloads - Add unit test for Query.getContextUsage() covering request payload and response handling * fix: strip UI type from SDK response, sync Java SDK protocol - Remove leaked `type: 'context_usage'` from control response payload - Add GET_CONTEXT_USAGE to Java SDK protocol mirror (enum, interface, union type) --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
tanzhenxin
|
63f1963377
|
Merge pull request #2698 from QwenLM/refactor/subagent-model-selection
feat: add cross-provider model selection for subagents |
||
github-actions[bot]
|
c7faae7b6e | chore(release): sdk-typescript v0.1.6 | ||
|
tanzhenxin
|
ad20049a4e | Merge remote-tracking branch 'origin/main' into refactor/subagent-model-selection | ||
|
tanzhenxin
|
5d58b2f112 |
feat: simplify subagent model configuration with model selector
Refactor subagent model configuration from nested modelConfig object to a simple model string field for better UX and clarity. Changes: - Replace modelConfig object with model string in SubagentConfig interface - Add model-selection.ts utility for parsing and validating model selectors - Support 'inherit' keyword and bare model IDs (e.g., 'glm-5', 'claude-sonnet-4-6') - Maintain backward compatibility by parsing legacy modelConfig frontmatter - Update validation to reject cross-provider authType-prefixed selectors - Update SDK types (TypeScript and Java) to reflect new schema - Add comprehensive tests for model selection and validation - Update documentation with model selection examples Breaking changes: - modelConfig.frontmatter field deprecated in favor of model field - Cross-provider model selectors (e.g., 'openai:gpt-4') not supported for subagents Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
mingholy.lmh
|
43bb14ddc9 |
docs(sdk): enhance coreTools/excludeTools/allowedTools documentation with permissions reference
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
DragonnZhang
|
ee33a3c35e | feat: add system prompt customization options in SDK and CLI | ||
|
mingholy.lmh
|
82dc79629c | feat: enhance session ID handling and error propagation | ||
|
mingholy.lmh
|
5d939fdb83 |
feat: add --session-id support for CLI and SDK
- Add --session-id flag to CLI for specifying custom session ID - Add sessionId option to SDK QueryOptions - Implement UUID validation for session IDs - Pass session ID from SDK to CLI via --session-id argument - Add integration tests for session-id functionality - Update unit tests for ProcessTransport Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
LaZzyMan
|
3f04217458 |
fix: prevent AbortSignal listener memory leak
- Add abort listener cleanup in Query.close() to prevent memory leak - Add abort listener cleanup in ControlDispatcher.shutdown() - Remove AbortController recreation in Session.handleInterrupt() This fixes the MaxListenersExceededWarning that occurred when: - Creating 11+ Query instances in SDK/non-interactive mode - Multiple user interrupts (Ctrl+C) in interactive mode - Intensive control request scenarios |
||
|
tanzhenxin
|
4d29d1fbb5
|
Merge pull request #1738 from QwenLM/feat/promote-skills-to-stable
feat: promote Agent Skills from experimental to stable |
||
|
tanzhenxin
|
a4ffc6eb24 |
feat: promote Agent Skills from experimental to stable
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
Mingholy
|
5c582e98c6
|
Merge pull request #1732 from QwenLM/mingholy/fix/sdk-release-workflow
Some checks are pending
Qwen Code CI / Lint (push) Waiting to run
Qwen Code CI / Test (push) Blocked by required conditions
Qwen Code CI / Test-1 (push) Blocked by required conditions
Qwen Code CI / Test-2 (push) Blocked by required conditions
Qwen Code CI / Test-3 (push) Blocked by required conditions
Qwen Code CI / Test-4 (push) Blocked by required conditions
Qwen Code CI / Test-5 (push) Blocked by required conditions
Qwen Code CI / Test-6 (push) Blocked by required conditions
Qwen Code CI / Test-7 (push) Blocked by required conditions
Qwen Code CI / Test-8 (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
Add CLI source selection for SDK releases and fix subagent output handler |
||
|
mingholy.lmh
|
77a8698bc5 |
ci(sdk-release): fix nightly and preview version calculation based on npm latest+1 patch
- Add getNextPatchVersion() to calculate next patch version from npm latest - Fix getNightlyVersion() to use npm latest + 1 patch instead of package.json - Fix getPreviewVersion() to use npm latest + 1 patch instead of nightly version - Add version info logging in workflow for dry-run verification Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
mingholy.lmh
|
39884cc6a1 |
ci(sdk-release): fix CLI package path handling
- Remove dist/ directory requirement from workflow (CLI files are in package root) - Update bundle-cli-from-npm.js to use package root directly instead of package/dist Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
mingholy.lmh
|
1e05359881 |
ci(sdk-release): add CLI source selection for SDK releases
- Replace cli_ref input with cli_source choice (build_from_source or npm_latest) - Add support for bundling latest stable CLI from npm - Add bundle-cli-from-npm.js script for npm-based CLI bundling - Fix property naming in nonInteractiveCli.ts (outputUpdateHandler) - Improve integration tests for subagents - Skip creating issue on failure during dry-run mode Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
tanzhenxin
|
7d50f5363f
|
Merge pull request #1726 from QwenLM/mingholy/fix/sdk-resume-usage
feat(sdk): add resume, continue options and extend authType support |
||
|
tanzhenxin
|
03b2cfbbde
|
Merge pull request #1719 from QwenLM/mingholy/fix/patch-process-transport-for-electron
Add FORK_MODE support to ProcessTransport for Electron IPC integration |