feat(cli): add /directory remove subcommand - #9
Closed
B-A-M-N wants to merge 113 commits into
Closed
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 37 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/cli/src/i18n/locales/zh.js">
<violation number="1" location="packages/cli/src/i18n/locales/zh.js:1471">
P0: Remove the stray `=======` merge-marker line; it causes a syntax error in this locale file.</violation>
</file>
<file name="packages/cli/src/i18n/locales/en.js">
<violation number="1" location="packages/cli/src/i18n/locales/en.js:1549">
P0: The added `=======` merge marker makes this locale file invalid JavaScript and causes a syntax error on load.</violation>
</file>
<file name=".github/workflows/ci.yml">
<violation number="1" location=".github/workflows/ci.yml:56">
P2: The new test matrix drops Node 20 coverage, so CI no longer validates the minimum supported runtime (>=20). Add at least one Node 20 lane to prevent compatibility regressions.</violation>
</file>
<file name="packages/cli/src/ui/commands/directoryCommand.tsx">
<violation number="1" location="packages/cli/src/ui/commands/directoryCommand.tsx:425">
P2: Only the first persisted match is removed; duplicate entries for the same directory remain and can cause it to reappear on restart.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
| uses: 'actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020' # ratchet:actions/setup-node@v4.4.0 | ||
| with: | ||
| node-version-file: '.nvmrc' | ||
| node-version: '22.x' |
There was a problem hiding this comment.
P2: The new test matrix drops Node 20 coverage, so CI no longer validates the minimum supported runtime (>=20). Add at least one Node 20 lane to prevent compatibility regressions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci.yml, line 56:
<comment>The new test matrix drops Node 20 coverage, so CI no longer validates the minimum supported runtime (>=20). Add at least one Node 20 lane to prevent compatibility regressions.</comment>
<file context>
@@ -50,13 +50,13 @@ jobs:
uses: 'actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020' # ratchet:actions/setup-node@v4.4.0
with:
- node-version-file: '.nvmrc'
+ node-version: '22.x'
cache: 'npm'
</file context>
There was a problem hiding this comment.
7 issues found across 37 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/cli/src/i18n/locales/zh.js">
<violation number="1" location="packages/cli/src/i18n/locales/zh.js:1471">
P2: This edit removed an existing zh locale entry that is still referenced by the UI, causing that message to lose Chinese translation.</violation>
</file>
<file name="packages/cli/src/ui/commands/directoryCommand.test.tsx">
<violation number="1" location="packages/cli/src/ui/commands/directoryCommand.test.tsx:82">
P2: The `forScope` test mock checks `'user'` (lowercase), but production passes `SettingScope.User` (`'User'`), so the mock always returns `workspace` and does not test user-scope removal logic correctly.</violation>
</file>
<file name=".github/workflows/ci.yml">
<violation number="1" location=".github/workflows/ci.yml:53">
P2: PR CI dropped minimum-version coverage by running only Node 22.x, despite the project declaring support for Node >=20.</violation>
</file>
<file name="packages/cli/src/ui/hooks/useSessionSearchInput.ts">
<violation number="1" location="packages/cli/src/ui/hooks/useSessionSearchInput.ts:97">
P2: Esc in search mode fails to return to list mode when the query is already empty, so `/` then `Esc` leaves the picker stuck in search until another key path exits.</violation>
</file>
<file name="packages/cli/src/ui/commands/directoryCommand.tsx">
<violation number="1" location="packages/cli/src/ui/commands/directoryCommand.tsx:427">
P2: Persisted directory removal can fail for `$HOME`/non-canonical entries, causing removed directories to reappear on restart.</violation>
</file>
<file name="packages/cli/src/ui/commands/modelCommand.ts">
<violation number="1" location="packages/cli/src/ui/commands/modelCommand.ts:232">
P2: Registry-only model validation rejects valid runtime snapshot model IDs before switchModel can handle them.</violation>
</file>
<file name="packages/cli/src/ui/components/SessionPicker.tsx">
<violation number="1" location="packages/cli/src/ui/components/SessionPicker.tsx:261">
P2: The search query is rendered unbounded in the new fixed-height search row, so long input can wrap and break the visible-item height calculation.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
| fetch-depth: 0 | ||
|
|
||
| - name: 'Set up Node.js' | ||
| - name: 'Set up Node.js 22.x' |
There was a problem hiding this comment.
P2: PR CI dropped minimum-version coverage by running only Node 22.x, despite the project declaring support for Node >=20.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci.yml, line 53:
<comment>PR CI dropped minimum-version coverage by running only Node 22.x, despite the project declaring support for Node >=20.</comment>
<file context>
@@ -50,13 +50,13 @@ jobs:
fetch-depth: 0
- - name: 'Set up Node.js'
+ - name: 'Set up Node.js 22.x'
uses: 'actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020' # ratchet:actions/setup-node@v4.4.0
with:
</file context>
| const handleSearchKey = useCallback((key: Key): void => { | ||
| const { name, sequence, ctrl } = key; | ||
|
|
||
| if (name === 'escape') { |
There was a problem hiding this comment.
P2: Esc in search mode fails to return to list mode when the query is already empty, so / then Esc leaves the picker stuck in search until another key path exits.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/ui/hooks/useSessionSearchInput.ts, line 97:
<comment>Esc in search mode fails to return to list mode when the query is already empty, so `/` then `Esc` leaves the picker stuck in search until another key path exits.</comment>
<file context>
@@ -0,0 +1,148 @@
+ const handleSearchKey = useCallback((key: Key): void => {
+ const { name, sequence, ctrl } = key;
+
+ if (name === 'escape') {
+ // Drop the query; the empty-query effect below routes the exit.
+ // The list-mode Esc handler then implements the second-stage cancel.
</file context>
| const targetAuthType = parsed.authType ?? authType; | ||
| const availableModels = | ||
| config.getAvailableModelsForAuthType(targetAuthType); | ||
| if (!availableModels.some((model) => model.id === parsed.modelId)) { |
There was a problem hiding this comment.
P2: Registry-only model validation rejects valid runtime snapshot model IDs before switchModel can handle them.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/ui/commands/modelCommand.ts, line 232:
<comment>Registry-only model validation rejects valid runtime snapshot model IDs before switchModel can handle them.</comment>
<file context>
@@ -130,67 +216,46 @@ export const modelCommand: SlashCommand = {
+ const targetAuthType = parsed.authType ?? authType;
+ const availableModels =
+ config.getAvailableModelsForAuthType(targetAuthType);
+ if (!availableModels.some((model) => model.id === parsed.modelId)) {
return {
type: 'message',
</file context>
Suggested change
| if (!availableModels.some((model) => model.id === parsed.modelId)) { | |
| const isRuntimeSnapshot = parsed.modelId.startsWith('$runtime|'); | |
| if ( | |
| !isRuntimeSnapshot && | |
| !availableModels.some((model) => model.id === parsed.modelId) | |
| ) { |
| <> | ||
| <Text color={theme.text.secondary}>{t('Search: ')}</Text> | ||
| <Text color={theme.text.primary}> | ||
| {picker.searchQuery} |
There was a problem hiding this comment.
P2: The search query is rendered unbounded in the new fixed-height search row, so long input can wrap and break the visible-item height calculation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/ui/components/SessionPicker.tsx, line 261:
<comment>The search query is rendered unbounded in the new fixed-height search row, so long input can wrap and break the visible-item height calculation.</comment>
<file context>
@@ -234,6 +237,39 @@ export function SessionPicker(props: SessionPickerProps) {
+ <>
+ <Text color={theme.text.secondary}>{t('Search: ')}</Text>
+ <Text color={theme.text.primary}>
+ {picker.searchQuery}
+ <Text color={theme.text.secondary}>▌</Text>
+ </Text>
</file context>
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/cli/src/ui/commands/directoryCommand.tsx">
<violation number="1" location="packages/cli/src/ui/commands/directoryCommand.tsx:399">
P1: Rollback state is captured with shallow copies, so nested settings mutations are not actually reversible on failure.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/cli/src/ui/commands/directoryCommand.tsx">
<violation number="1" location="packages/cli/src/ui/commands/directoryCommand.tsx:433">
P2: `/directory remove` now incorrectly requires a persisted settings match before removing from the active workspace. This can reject directories that are actually in the current workspace.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
There was a problem hiding this comment.
1 issue found across 7 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/ci.yml">
<violation number="1" location=".github/workflows/ci.yml:53">
P2: PR CI dropped minimum-version coverage by running only Node 22.x, despite the project declaring support for Node >=20.</violation>
<violation number="2" location=".github/workflows/ci.yml:56">
P2: The new test matrix drops Node 20 coverage, so CI no longer validates the minimum supported runtime (>=20). Add at least one Node 20 lane to prevent compatibility regressions.</violation>
</file>
<file name="packages/cli/src/ui/hooks/useSessionSearchInput.ts">
<violation number="1" location="packages/cli/src/ui/hooks/useSessionSearchInput.ts:97">
P2: Esc in search mode fails to return to list mode when the query is already empty, so `/` then `Esc` leaves the picker stuck in search until another key path exits.</violation>
</file>
<file name="packages/cli/src/ui/commands/modelCommand.ts">
<violation number="1" location="packages/cli/src/ui/commands/modelCommand.ts:232">
P2: Registry-only model validation rejects valid runtime snapshot model IDs before switchModel can handle them.</violation>
</file>
<file name="packages/cli/src/ui/components/SessionPicker.tsx">
<violation number="1" location="packages/cli/src/ui/components/SessionPicker.tsx:261">
P2: The search query is rendered unbounded in the new fixed-height search row, so long input can wrap and break the visible-item height calculation.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
…QwenLM#4164) The progressive-MCP rollout (QwenLM#3994) regressed non-interactive MCP tool visibility on the first `--prompt` request — the model never sees the configured MCP tool and answers from its own knowledge, so the test's `waitForToolCall('mcp__addition-server__add')` assertion times out on all three retries. Reproduced locally: 167s 3/3-fail without the rollback flag, 22s pass with it. Set `QWEN_CODE_LEGACY_MCP_BLOCKING=1` in the test's `beforeAll` so the spawned CLI uses the pre-QwenLM#3994 synchronous discovery path. Scoped to this single test rather than the workflow env so other integration tests keep exercising the new progressive-MCP code path. Temporary workaround. Remove once QwenLM#4163 is fixed.
* refactor(serve): 1 daemon = 1 workspace (QwenLM#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 QwenLM#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 QwenLM#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 QwenLM#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 QwenLM#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 QwenLM#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 1c7f5f0. - M2 (httpAcpBridge.ts): drop the dead `ChannelInfo.workspaceCwd` field. Pre-§02 it was the routing key for `byWorkspaceChannel.get`; after the §02 collapse all reads target `SessionEntry.workspaceCwd` and `ChannelInfo.workspaceCwd` was only written, never read. Per- channel storage also suggests variance the "1 daemon = 1 workspace" model forbids. Removing the field encodes the single-workspace invariant in the type itself; left a stub comment so future readers don't reintroduce it. - M3 (httpAcpBridge.ts): fast-path `canonicalizeWorkspace` when `req.workspaceCwd === boundWorkspace`. The §02 recommended client flow is `caps.workspaceCwd` → POST `cwd: caps.workspaceCwd`, and the omit-cwd route in server.ts synthesizes the same equality. Both hit the equality check and skip the sync `realpathSync.native` syscall. Non-equal inputs fall through to the full canonicalize (clients sending `/work/./bound`, mixed casing on case-insensitive FS, symlink aliases) so correctness is unchanged. - M4 (httpAcpBridge.ts): operator stderr breadcrumb in the `channel.exited` handler. An agent crash (OOM / segfault) used to be silent on the daemon side — the child-stderr forwarder caught whatever the child wrote before dying (often nothing on SIGKILL/segfault), and SSE subscribers saw `session_died` frames but operators reading `qwen serve`'s own output had no signal that the agent process was gone. Log code+signal+affected-session-count so the line is the canonical "agent disappeared" indicator. - M5 (server.ts): documentation-only. The reviewer wanted `createServeApp` to validate `opts.workspace` exists + is a directory (currently only `runQwenServe` does). Trade-off: doing that breaks 4 existing tests which pass synthetic `/work/bound` on purpose to exercise route-layer behavior without a real directory. Deferred the helper extraction; added a JSDoc note pinning the contract so future entry points binding `createServeApp` to user input know to replicate the validation. - M6 (runQwenServe.ts): pass the already-canonical `boundWorkspace` into `createServeApp` via `opts.workspace`. `canonicalizeWorkspace` is idempotent so the server-side recanonicalize is a no-op today, but if a future refactor ever makes it non-idempotent the values the route advertises on `/capabilities` and the bridge enforces would diverge — landing clients in a "/capabilities says X, POST /session/X returns workspace_mismatch" contradiction. Removes the drift risk. bridge: 73/73; server: 80/80; tsc clean for PR-touched files. * fix(serve,sdk): address fourth /review round (deepseek-v4-pro x2) Two new inline findings: - O1 (server.ts): the POST /session route uses `'cwd' in body` against `safeBody`'s `Object.create(null)` output to distinguish "client omitted cwd" from "client sent cwd". The semantics quietly couple to `safeBody`'s literal strip list (`__proto__/constructor/prototype`). If a future maintainer adds a user-facing key (e.g. `cwd`) to that strip list, the route's presence-check would silently flip to "absent → fallback", masking the bug as "wrong workspace bound." Extracted `PROTOTYPE_POLLUTION_KEYS: ReadonlySet<string>` as a named module-scope constant; safeBody uses `.has()` on it (behavior unchanged); the route's comment now cross-references the const so the coupling is documented at both ends. The const's JSDoc spells out what to do if the strip set ever has to grow into user-key territory. - O2 (sdk-typescript): `DaemonCapabilities.workspaceCwd` is `string | undefined` (additive to v=1; pre-§02 daemons omit). SDK consumers that pass it into a `string` context get a TS strict error or, against an old daemon, a runtime `Cannot read properties of undefined`. Added a `requireWorkspaceCwd` helper + `DaemonCapabilityMissingError` so consumers can opt into an actionable `DaemonCapabilities.workspaceCwd is missing — introduced in QwenLM#3803 §02 …` error instead. Exported both from `@qwen-code/sdk`'s top-level module + the `daemon/` sub-module. Unit tests cover populated, missing, and empty-string inputs. bridge: 73/73; server: 80/80; SDK DaemonClient: 43/43 (was 40, +3 new requireWorkspaceCwd cases). tsc clean for SDK and PR-touched CLI files. * fix(serve): address tanzhenxin REQUEST_CHANGES (cold-spawn + streaming-test bind) Two findings from the CHANGES_REQUESTED review on PR QwenLM#4113. - T1 (integration-tests/cli/qwen-serve-streaming.test.ts) — high severity: the daemon spawn in `beforeAll` did not pass `--workspace REPO_ROOT`, so under §02 the daemon bound to whatever cwd the test runner was invoked from. Every later `createOrAttachSession({ workspaceCwd: REPO_ROOT })` then 400'd with `workspace_mismatch`, and the entire file — child-crash recovery, multi-client first-responder permission, Last-Event-ID resume — silently no-op'd once `SKIP_LLM_TESTS` was unset. The sibling `qwen-serve-routes.test.ts` got the same fix earlier in this PR; this file was missed in that pass. Added the flag with a comment pointing at the rationale so the omission can't recur. - T2 (packages/cli/src/serve/httpAcpBridge.ts) — medium severity: cold-spawn window orphans the agent child on double-Ctrl+C. The `qwen --acp` child exists from the moment `channelFactory` spawns it, but pre-fix the bridge only added the channel to `aliveChannels` AFTER `connection.initialize()` returned. During the up-to-`initTimeoutMs` (default 10s) handshake window `aliveChannels` was empty, and a double-Ctrl+C in that window played out as: first SIGINT entered `shutdown()` and awaited the in-flight spawn; second SIGINT called `killAllSync()` against an empty set; `process.exit(1)` orphaned the child. Same class of bug the BkUyD invariant set out to close — the post-init overwrite race was covered, the pre-init handshake window wasn't. Fix: move `info` creation + `aliveChannels.add(info)` + the `channel.exited` handler registration BEFORE the `initialize` await. Init-failure / late-shutdown / child-crash-during-handshake all converge on the same cleanup path: mark `isDying = true`, `await channel.kill()`, let the exited handler `aliveChannels .delete(info)` once the OS reaps the process. `channelInfo` (the attach target) is still assigned LAST so `ensureChannel`'s fast-path never returns a still-handshaking channel. Regression test: `killAllSync force-kills the channel during the initialize handshake` uses a bespoke factory whose agent's `initialize` never resolves and asserts `killAllSync` fires killSync against the channel during the handshake window. Pre-fix the test would observe an empty `killSyncCalls` array. bridge: 74/74 (was 73, +1 cold-spawn test); server: 80/80; tsc clean for PR-touched files. * fix(serve): address third /review round (gpt-5.5 + glm-5.1 + deepseek-v4-pro) Eight new inline findings; six applied, two deferred-with-reply. - P1 (httpAcpBridge.ts init-failure isDying comment): my comment overstated what `info.isDying` accomplishes on the init-failure path — concurrent `ensureChannel()` callers don't bypass via `isDying`, they coalesce on `inFlightChannelSpawn` and observe the same rejection. Reworded to describe the actual cross-path invariant marker. - P2 (server.ts workspace_mismatch log injection): doudouOUC flagged log injection via `err.requested` (user-controlled). `path.resolve` + `realpathSync.native` preserve control chars in path segments, so a body `{"cwd": "/legit/path\nqwen serve: FAKE LOG"}` would emit two valid-looking daemon log lines on stderr — weaponizing line-based log shippers (Splunk / Loki / journald → SIEM). `JSON.stringify` both `err.bound` and `err.requested` in the log line escapes control chars + quotes the values, making any injection attempt visible-as-quoted-noise rather than forged-line. Bound is operator-controlled and inherently safe but quoted symmetrically for readability. The defense-in-depth alternative (reject control chars in canonicalizeWorkspace) is deferred — this single log site was the actionable interpolation; future workspace-path-into-stderr / -JSON / -templated-SQL flows can pick up the rejection if they ship. - P3 (httpAcpBridge.test.ts): refactor the cross-workspace WorkspaceMismatchError test to a single `.catch((e) => e)` capture rather than firing the rejection twice (once for the `rejects .toBeInstanceOf` matcher, once for the field assertions). Logic unchanged. - P4 (httpAcpBridge.ts channel.exited log): the `qwen serve: channel exited (...)` line fired on every channel exit including planned shutdown — alarming for operators who Ctrl+C'd a healthy daemon. Guarded with `if (!shuttingDown)` so the planned-shutdown case (operator already saw `received SIGINT, draining...`) stays silent. The killSession path (last session leaves, daemon stays up — no top-level context line) still logs, since the line is the only signal that the cleanup actually ran. - P5 (httpAcpBridge.ts): light trim of the "pre-fix" narrative voice in two comment blocks (cold-spawn ensureChannel layout + BkUyD killAllSync aliveChannels iteration). Kept the invariant explanations — those carry maintenance value — dropped the "pre-fix the code did X" framing that's review-context not future-reader context. - P6 (server.ts + runQwenServe.ts): `createServeApp` now accepts a pre-canonicalized `deps.boundWorkspace` to skip its own `canonicalizeWorkspace` syscall when the caller (runQwenServe) already did the work. Replaces my earlier `{...opts, workspace: boundWorkspace}` opts-mutation hack — cleaner separation of concerns + drops one `realpathSync.native` per boot. Direct callers (tests, embeds) that omit `deps.boundWorkspace` still get the in-body canonicalize path. - P8 (httpAcpBridge.ts): defensive `aliveChannels.size > 2` warning. The set is intentionally multi-entry to cover the killSession-then-spawnOrAttach overlap window (size 2 is legitimate). Anything higher implies a `channel.exited` handler never fired for a prior channel — a real leak we'd otherwise catch only as gradually-growing RSS. The warning surfaces it the moment it happens. - P7 (CreateSessionRequest.workspaceCwd optional): deferred with reply rationale. Making the field optional is the §02 design ("SDK accepts bound path or none"); the JSDoc already explains the omit-vs-explicit choice; Stage 1 has no shipping SDK consumers so there's no breakage to call out in a changelog file. No code change. bridge: 74/74 (cross-workspace test refactor + behavioral assertions unchanged); server: 80/80; SDK 43/43. tsc clean for PR-touched files. * fix(serve): apply auto-fixes from /review (QwenLM#4113) - canonicalizeWorkspace: narrow catch to ENOENT only, propagate other filesystem errors - listWorkspaceSessions: add fast-path string equality to avoid realpathSync on every poll - GET /workspace/:id/sessions: return 400 workspace_mismatch for cross-workspace queries - SessionNotFoundError: accept optional extra message; clarify agent-crash-on-spawn case - requireWorkspaceCwd: distinguish empty-string (post-§02 bug) from absent (pre-§02 daemon) * fix(serve/test): bind workspace explicitly in GET /workspace tests Wave-5 commit 0c6e963 ("apply auto-fixes from /review (QwenLM#4113)") added a 400 workspace_mismatch reject path to GET /workspace/:id/sessions for cross-workspace queries, but the existing two happy-path tests queried `/work/a` / `/work/idle` against an unbound daemon (which falls back to `process.cwd()`). Both turned to 400 in CI. Bind the daemon to WS_BOUND in both happy-path tests and query the same path. Add a third regression test that pins the §02 cross-workspace rejection contract — `code: workspace_mismatch`, both paths in the body, bridge.listCalls untouched (no silent fallback regression). Brings server.test.ts from 80 → 82 tests, all passing. * fix(serve,sdk): address fourth /review round (deepseek-v4-pro x2) Six new inline findings; five applied, one defer-with-reply. - Q1 (httpAcpBridge.ts + server.ts + tests): cwd length amplification through WorkspaceMismatchError. The error constructor interpolates `requested` into `.message` TWICE; `sendBridgeError` echoes it on stderr (now JSON.stringify-wrapped); `res.json` echoes it again — a ~10 MB `cwd` body (right under express.json's 10 MB cap) would amplify to ~60 MB per request × maxConnections (default 256). On loopback-default-no-token deployments this is pre-auth. Added `MAX_WORKSPACE_PATH_LENGTH = 4096` (Linux PATH_MAX); route rejects oversized `cwd` with a 400 BEFORE the bridge is touched, and the `WorkspaceMismatchError` constructor truncates `requested` as defense-in-depth for non-route callers (tests, embeds, future entry points that throw the error directly). Three new tests pin the route 400, the constructor truncation, and the normal-path passthrough. - Q2 + Q5 (httpAcpBridge.ts docs): the `channelInfo` declaration comment + `ChannelInfo.sessionIds` JSDoc + `ChannelInfo.isDying` JSDoc all overstated when `channelInfo` is cleared. Post-§02 the BkUyD invariant is "ONLY `channel.exited` clears `channelInfo`" — teardown initiators (killSession last-session-leaving, doSpawn-newSession-failure, ensureChannel init-failure/late- shutdown, shutdown) set `isDying = true` but LEAVE `channelInfo` pointing at the dying channel until OS reap, so `killAllSync` can still reach it through `aliveChannels`. A future maintainer reading the old phrasing might "fix" killSession to also clear `channelInfo` and silently break the double-Ctrl+C force-kill path. Rewrote all three sites to describe the actual invariant + enumerate the 5 isDying set-sites + spell out the BkUyD rationale in one place (the `isDying` JSDoc) that other comments point at. - Q3 (runQwenServe.ts): the "listening on …" boot summary goes to stdout but every other operational diagnostic (bearer auth, the workspace_mismatch breadcrumb, channel-exited, bridge errors) goes to stderr. Operators capturing only stderr (systemd / docker / k8s default) miss the `workspace=` indicator, which is the single piece of information they need most when triaging §02 migration issues. Added a `qwen serve: bound to workspace "X"` stderr line alongside the stdout one — keeps stdout untouched (integration tests + scripts parse it) while making the breadcrumb visible to stderr-only log shippers. `JSON.stringify` the boundWorkspace value (operator-controlled but cheap defense-in-depth against any future flow that lands a control char in the path). - Q4 (integration-tests/tsconfig.json): the `paths` entry resolved `@qwen-code/sdk` to the SDK's built `dist/` directory; `dist/` is gitignored and stale dist (no `npm run build` first) yields TS2339 errors on the integration tests' imports of new SDK fields. Pointed `paths` at SDK source instead — `tsc -p integration-tests/tsconfig.json` no longer requires a prior rebuild. The vitest config's runtime alias still resolves to `dist/index.mjs` so the actual test execution exercises the published-bundle shape; this paths entry only affects type resolution. - Q6 (httpAcpBridge.ts): `createHttpAcpBridge` constructor called `canonicalizeWorkspace(opts.boundWorkspace)` even when the caller (`runQwenServe`) had already canonicalized and threaded the same value through `deps.boundWorkspace` into `createServeApp`. Two independent `realpathSync.native` calls can theoretically diverge on NFS-transient / mid-rename filesystems, landing the bridge with a canonical form different from what `/capabilities` advertises and from `createServeApp`'s view. Dropped the bridge's re-canonicalize; kept `path.isAbsolute` (structural, not a syscall); documented the caller contract on `BridgeOptions .boundWorkspace` ("MUST be pre-canonicalized; tests/embeds call `canonicalizeWorkspace` first"). Tests use `path.resolve(path.sep, ...)` which is already canonical-or- fallback for non-existent paths, so no test changes needed. bridge: 76/76 (was 74, +2 WorkspaceMismatchError truncation tests); server: 82/82 (was 80, +2 length cap + the auto-applied helper). tsc clean for SDK, CLI PR-touched files, and integration-tests' qwen-serve-*.
…ts (QwenLM#4147) * fix(vscode): allow editing sessions without local snapshots * fix(vscode): keep thinking after edited user message * fix(vscode): sync conversation id alignment through router
QwenLM#4160) * refactor(serve): extract createInMemoryChannel helper from httpAcpBridge.test.ts (QwenLM#4156 A1) Sub-PR A1 of issue QwenLM#4156 (Stage 1.5b Mode A daemon). Pure refactor with zero behavior change. Extracts the inline paired NDJSON channel construction (`new TransformStream` × 2 + `ndJsonStream` × 2) that was duplicated across `httpAcpBridge.test.ts` into a production helper `createInMemoryChannel()` at `packages/cli/src/serve/inMemoryChannel.ts`. The helper is added to `packages/cli/src/serve/index.ts`'s barrel export alongside the rest of the serve module's public API. The helper is intentionally bare — it returns only the stream pair, no lifecycle / teardown surface. Two reasons: 1. Consumer behavior diverges widely (stuck channel, crashable child simulation, no-op, real in-process termination); a one-size-fits-all `close()` would either pull test-fixture concerns into a production module or force a single shape on consumers that don't want it. 2. The SDK's `ndJsonStream` outer wrapper does not reliably propagate close on `Stream.writable` to the opposite `Stream.readable`; consumers needing to simulate a child exit hold their own underlying `TransformStream` references and close those directly. 10 of 11 inline call sites in `httpAcpBridge.test.ts` migrate cleanly to the new helper. The 11th (`makeChannel` at line 151) keeps the inline 4-line construction because its `kill()` closure needs the underlying `ab` / `ba` writables to simulate child-process termination — a comment above the function explains the asymmetry. The helper is also a primitive for the future A2 PR's `inProcessAcpBridge.ts`, which will use it to wrap an in-process `QwenAgent` without spawning a `qwen --acp` child (see issue QwenLM#4156 §3 decision 1 and §8). Test plan: - New `inMemoryChannel.test.ts`: 5 tests covering bidirectional round-trip, ordering preservation, and bidirectional direction isolation - Existing `httpAcpBridge.test.ts`: 70 tests, identical count and behavior before vs after migration - `vitest run packages/cli/src/serve/inMemoryChannel.test.ts packages/cli/src/serve/httpAcpBridge.test.ts` — 75/75 pass - `tsc --noEmit -p packages/cli/tsconfig.json` — clean for changed files 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * refactor(serve): address Copilot review feedback on createInMemoryChannel Two small follow-ups from QwenLM#4160 review: 1. inMemoryChannel.test.ts:113,137 — handle the pending `reader.read()` that the isolation tests intentionally leave hanging when the timeout wins the race. `reader.releaseLock()` in `finally` rejects that pending read per Web Streams spec; without a rejection handler this could surface as an unhandled rejection / flaky test signal. Added a no-op rejection handler via the two-arg `.then(onResolve, onReject)` form so the cleanup-path rejection settles cleanly. 2. inMemoryChannel.ts:11 — the JSDoc said "two `TransformStream<...>` pairs" which reads ambiguously as "two pairs of TransformStream" (i.e., 4 streams). The implementation creates exactly two TransformStreams (one per direction). Reworded to "two `TransformStream<...>` instances (one per direction)" to disambiguate. Tests still 5/5 pass. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * refactor(serve): expose abort() teardown primitive on createInMemoryChannel + route test through barrel Two follow-ups from QwenLM#4160 review: 1. Expose `abort(reason?)` on the helper return value (per @wenshao critical comment). Reasoning: the helper previously returned only the `Stream` pair, leaving consumers no way to tear the channel down. `ndJsonStream`'s outer wrapper does not reliably propagate `close()`, but `abort()` on the underlying byte-level `TransformStream` is forceful-by-spec — pending reads on both sides settle immediately so GC can reclaim. This unblocks the future Stage 1.5b in-process bridge (QwenLM#4156, sub-PR A2) which needs teardown on daemon shutdown. The settlement shape is documented honestly in JSDoc: at the inner byte-level layer pending reads reject with the supplied reason; at the outer SDK-wrapped `Stream` the wrapper translates that into a clean `{done: true}` signal. Either way, pending operations no longer hang — that's the teardown invariant we care about. 2. Route the test's import through the `serve/index.js` barrel rather than the source file (per @wenshao suggestion). Without a test that exercises the public API path, a typo or missing re-export in the barrel would go undetected in CI. Tests: 8/8 helper tests pass (5 existing + 3 new abort tests covering teardown invariant + idempotency + no-reason variant). 70/70 existing httpAcpBridge tests still pass. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
Preserve the outer debug session ID when relaunching into the sandbox by passing it through an internal sandbox-only session flag.
QwenLM#4106) Add modality pattern for qwen3.6-35b model names, enabling image and video input for locally-hosted Qwen3.6-35B-A3B models (e.g. SGLang's default model name: Qwen3.6-35B-A3B-NVFP4). Previously these fell through to the text-only catch-all, blocking all image content. Co-authored-by: Tyler <tyler@dinsmoor.us> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…wenLM#4115) * inject addContext for SessionStart * resolve comment * resolve comment * resolve comment * fix comment * unfiy function and resolve comment * resolve comment
…nventions (QwenLM#4129) * fix(i18n): Correct zh-TW translations to match Traditional Chinese conventions Fix ~131 lines of Traditional Chinese (zh-TW) translations that used Simplified Chinese character forms instead of standard Traditional Chinese usage. Changes: - 文件 → 檔案 (47 occurrences) - 爲 → 為 (45 occurrences) - 啓 → 啟 (44 occurrences) - 曆史 → 歷史 (6 occurrences) - 鏈接 → 連結 (4 occurrences) - 菜單 → 選單 (3 occurrences) * fix(i18n): Replace 服務器 with 伺服器 (15 occurrences) Align with Traditional Chinese convention where 伺服器 is the standard term for 'server' in computing contexts. * fix(i18n): Update zh-TW.js header comment to prevent accidental overwrite Clarify that the file is the authoritative source and should not be overwritten with auto-generated output, to prevent future maintainers from regenerating with raw OpenCC and losing manual corrections. * fix(i18n): Add zh-TW regression check and maintenance docs Addresses reviewer feedback on PR QwenLM#4129 (points 2 and 3): - scripts/check-i18n.ts: Iterate over parsed zh-TW translation values (not raw file content) and report the offending key. Replace the earlier substring list with ZH_TW_FORBIDDEN_PATTERNS, which targets the three real regression categories: variant Traditional characters produced by OpenCC s2t (爲, 啓), Mainland-Chinese vocabulary (服務器, 菜單, 鏈接), and pure Simplified characters. Excludes 禁用 / 配置 / 文件 / 打開 to avoid false positives on Taiwan-valid usage. - scripts/tests/check-i18n.test.ts: Cover the new check, including negative cases for Taiwan-valid vocabulary. - docs/users/features/language.md: Document zh-TW maintenance — the vocabulary table, why raw OpenCC s2t output is not acceptable, and where the CI-enforced list lives. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(i18n): Address review feedback on zh-TW check (QwenLM#4129) - check-i18n.ts: Sort ZH_TW_FORBIDDEN_PATTERNS longest-first and break on first match so e.g. `历史` reports the specific bigram instead of also firing the bare `历` rule (no duplicate CI errors). - check-i18n.ts: Add ZH_TW_ALLOWED_EXCEPTIONS escape hatch so a future legitimate translation (e.g. 區塊鏈 in a UI string) can opt out by key without weakening the global pattern list. - docs/users/features/language.md: Add a "CI enforced?" column so contributors can tell which rows block CI vs. which are review-only style guidance. Replace bare `曆` in the table with the `曆史` bigram and note that `曆` is correct in calendar terms (日曆, 農曆, 西曆) — prevents a future maintainer from globally replacing 曆→歷. - Tests: Cover the dedup behavior on overlapping patterns. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(i18n): Note word-boundary limitation of zh-TW substring check Document the known limitation that `includes()`-based pattern matching does not respect Chinese word boundaries — a bigram like `鏈接` will false-positive on `區塊鏈接口` (區塊鏈 + 接口). Direct contributors to `ZH_TW_ALLOWED_EXCEPTIONS` when this happens instead of weakening the pattern list. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
…le events (QwenLM#3378) * add TaskCreated and TaskCompleted * resolve comment * resolve lint * change merge logic from simple to or * resolve lint error * reslove commnent * fix i18n key mismatch and malformed imports * resolve comment
) * refactor(cli): revert dynamic slash command LLM translation (QwenLM#4137) Removes the runtime LLM-translation path for dynamic slash command descriptions added in QwenLM#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 QwenLM#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 QwenLM#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`.
…nded) (QwenLM#4119) * chore(deps): re-upgrade ink 6 → 7.0.3 (upstream Static remount fix landed) PR QwenLM#3860 first upgraded ink 6 → 7.0.2. PR QwenLM#4083 reverted because of a TUI regression: `<Static>` did not re-emit items when its `key` prop was bumped, so `/clear` / Ctrl+O / refreshStatic left the history area blank under ink 7.0.2. ink 7.0.3 (released after QwenLM#4083) contains the exact fixes: - be9f44cda Fix: <Static> remount via key change drops new items (QwenLM#948) - 669c4386c Fix: Drop stale <Static> output from fullStaticOutput on identity change (QwenLM#950) - 7c2267c01 Fix `useBoxMetrics` not accepting ref objects with an initial null value (QwenLM#945) Changes: - `ink` ^6.2.3 → ^7.0.3 (root hoist + cli direct) - `react` ^19.1.0 → ^19.2.4 (cli direct; ink 7.0.3 peerDeps requires >=19.2.0) - `react`/`react-dom` overrides ^19.2.4 added so the transitive graph stays deduped to a single instance (avoids `Invalid hook call` from multiple React copies, the classic ink-upgrade hazard) - `wrap-ansi` already on ^10.0.0 from QwenLM#4083's partial-revert (no change) Verified: - `npm ls ink` → single `ink@7.0.3` across all peer deps - `npm ls react` → single `react@19.2.4` - `npm run typecheck --workspace=@qwen-code/qwen-code` clean - `npm run typecheck --workspace=@qwen-code/qwen-code-core` clean - Composer.test.tsx 20/20, MainContent.test.tsx 6/6, TableRenderer.test.tsx 59/59 + 1 skipped — all key UI components green on the new ink The Static-remount regression is upstream-fixed in 7.0.3, so the runtime path is restored without needing QwenLM#3941's overflowY-self-managed viewport. QwenLM#3941 (virtual viewport) remains an opt-in performance feature on top. * fix(deps,cli): add @types/react overrides + move refreshStatic out of setCurrentModel updater Two follow-ups from the multi-round audit of the ink 7.0.3 re-upgrade: 1. @types/react / @types/react-dom now pinned to ^19.2.0 in root overrides. packages/web-templates still declares @types/react ^18.2.0 in its devDeps. Today the CLI build is unaffected (web-templates's 18.x types are nested in its own node_modules and the React-using src/insight and src/export-html files are excluded from its tsconfig build), but a future reincludes-or-hoist accident would land conflicting global JSX namespaces in the CLI compile graph. Match the dep dedup we already enforce for `react` and `react-dom` so the type graph stays as deduped as the runtime graph. 2. AppContainer's onModelChange handler was calling refreshStatic() as a side-effect inside the setCurrentModel updater. React.StrictMode double-invokes state updaters in dev, so model swaps fired two clearTerminal writes + two <Static> key bumps. The double work was masked under ink 6 (key changes were no-ops on <Static>), but ink 7.0.3 honors key changes — the doubled work is now potentially visible as a faster flash-flash on every model switch. Refactor: setCurrentModel becomes a pure setter; refreshStatic moves into a useEffect keyed on currentModel with a ref-comparison guard so the first render doesn't fire. Single clearTerminal write per real model change, even under StrictMode. Verified: npm ls ink → single 7.0.3, npm ls react → single 19.2.4, npm ls @types/react → 19.2.10 hoisted (npm flags web-templates's 18.x constraint as overridden, which is the intended behavior). Typecheck clean across cli + core workspaces. * fix(cli): collapse model-change effect back into one batched handler wenshao's PR QwenLM#4119 review correctly flagged that splitting the onModelChange flow into two effects (b25831b) reintroduced the issue QwenLM#3899 freeze regression on every model switch: 1. setCurrentModel(model) commits first, with the OLD historyRemountKey. 2. <Static key={`${historyRemountKey}-${currentModel}`}> sees its key change (because currentModel did) and remounts immediately. 3. MainContent's render-phase progressive-replay reset only fires when historyRemountKey changes, so replayCount is still the full mergedHistory.length from any prior catch-up. 4. The remounted Static dumps the entire history in one synchronous layout pass — exactly the freeze progressive replay was added to avoid (QwenLM#3899). The second effect's refreshStatic() bump arrives a render too late. Fix: do not split. Both side effects (refreshStatic, which writes clearTerminal + bumps historyRemountKey, and setCurrentModel) live in the event handler again, with a ref guard for same-model notifications. The React.StrictMode concern that motivated b25831b is addressed by keeping the side effect OUT of the setState updater (it now runs once per event-handler invocation, not once per double-invoked updater call). Both setState calls land in the same React batch, so historyRemountKey and currentModel update together — MainContent's render-phase reset sees the new key, replayCount drops to the first chunk, and Static remounts with chunked replay intact. Tests: - AppContainer.test.tsx: 4 new tests covering the synchronous refreshStatic side-effect contract, same-model no-op, ref-guarded StrictMode double-invoke, and unsubscribe-on-unmount. - MainContent.test.tsx: new regression guard — when currentModel changes but historyRemountKey is held constant, progressive replay must NOT reset (pins the MainContent invariant the two-effect refactor accidentally relied on). Verified: vitest packages/cli AppContainer + MainContent green (82/82). Typecheck clean. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…#3388) * implement prompt hook * resolve comment * resolve comment * resolve comment * resolve comment * fix unit test
) * perf(cli): code-split lowlight to cut startup V8 parse cost Move the syntax-highlight engine out of the synchronously-parsed cli.js entry into a separately-emitted chunk and load it via dynamic import on the first code-block render. Until the chunk arrives, code blocks render as plain text; the next React commit of the surrounding subtree picks up the highlighted version, so users never see incorrect highlighting – just an imperceptibly later transition for the very first code block. Mechanics: - esbuild config: switch entry to outdir + splitting:true so that `await import('lowlight')` produces an actual on-disk chunk that's only parsed by V8 when first needed. - esbuild-shims: rename injected __dirname/__filename to qwen-prefixed symbols + use `define` to redirect free references. Previous inject collided with vendored libraries (yargs) that ship their own `var __dirname` ESM-compat polyfill once splitting flattens chunks. - prepare-package: include the new chunks/ directory in the published package's files list. - CodeColorizer: keep the public colorize{Code,Line} signatures and HAST rendering identical; on first call when the chunk hasn't loaded it returns the plain line and fires the dynamic import via a tiny standalone loader module. - lowlightLoader (new): isolates the lazy-load surface to a module with zero transitive imports (no themeManager, settings, or core). This lets test-setup prime the cache without dragging the whole UI module graph into every test file, which was observed to perturb theme and settings test outcomes when CodeColorizer was imported directly. - test-setup: await loadLowlight() once via the standalone loader so synchronous snapshot tests see the highlighted output deterministically. Measurements (real $HOME, n=15 interleaved A/B vs main HEAD, macOS): | Metric | Before (mean±sd ms) | After (mean±sd ms) | Δ | t | p | | ------------------ | ------------------- | ------------------ | -------- | ------ | -------- | | firstByte (wall) | 1633.5 ± 88.7 | 1475.8 ± 73.3 | -157.7 | 5.31 | 1.33e-5 | | idle (wall) | 2048.7 ± 93.6 | 1902.3 ± 80.2 | -146.3 | 4.60 | 8.71e-5 | | cli.js size | 25 MB | 6.9 MB | -18.1 MB | — | — | Both metrics clear the +50ms-or-10% Welch's t-test bar by an order of magnitude. cli.js drops 72%; total payload (cli.js + chunks/) is similar but only cli.js is parsed at module-eval time, which is the phase that dominates the user-visible startup gap. How to validate: npm run bundle ls dist/ # cli.js + chunks/lowlight-*.js node dist/cli.js -y # interactive UI still renders Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): resolve chunk-relative sibling paths under esbuild splitting With `splitting: true`, esbuild hoists modules with shared dependencies into `dist/chunks/`. Three modules derived runtime paths from `import.meta.url` assuming they were co-located with `cli.js`; once hoisted, `path.dirname(fileURLToPath(import.meta.url))` resolved to `dist/chunks/` and sibling-asset lookups silently missed: - `skill-manager.ts`: bundledSkillsDir → `dist/chunks/bundled` (actual `dist/bundled/`). The `existsSync` guard swallowed the miss, dropping all four bundled skills (`/review`, `/qc-helper`, `/batch`, `/loop`) with no user-visible signal. - `ripgrepUtils.ts`: `getBuiltinRipgrep()` → `dist/chunks/vendor/...`. Falls back to system rg if installed, otherwise null on minimal hosts — degrading grep to the slow internal scanner. - `i18n/index.ts`: `getBuiltinLocalesDir()` → `dist/chunks/locales`. User-visible behavior survives via the static glob import in `tryImportBundledTranslations`, but the loose-on-disk override path is dead. Each module now strips a trailing `chunks` segment when present, so the lookup resolves under `dist/`. In source / transpiled modes the basename is never `chunks`, so the fallback is a no-op. Also: - Add `chunks` to `DIST_REQUIRED_PATHS` in `create-standalone-package.js` so a regressed bundle that produces only `cli.js` fails the pre-packaging check instead of shipping a broken archive. - Expand `esbuild-shims.js` header so future contributors understand that `__qwen_filename` / `__qwen_dirname` always resolve to the shim's chunk file (dist/chunks/) and that sibling-asset lookups must strip the `chunks` segment. Reported by claude-opus-4-7 via Qwen Code /qreview on QwenLM#4070. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * perf(cli): prefetch lowlight from AppContainer + harden loader Three follow-ups to the lowlight code-split: - AppContainer fires `loadLowlight()` from a mount effect so the dynamic import is already in flight before any code block needs colorizing. Without this, code blocks committed to ink's append-only `<Static>` region before the import resolves stay plain text for the rest of the session — Static can only be re-rendered via `refreshStatic`, which is not wired to lowlight load completion. Common reachable paths: short `--prompt -p` runs that finalize quickly, Ctrl+C- cancelled first turns, and the first-paint history replay on `--resume`. The startup parse-cost win is preserved (V8 still parses off the critical path). - `lowlightLoader.ts` latches the first import failure so subsequent calls short-circuit to a rejected promise instead of re-attempting `import('lowlight')` on every keystroke. The colorizer already falls back to plain text on miss; recovery requires a fresh process anyway. - `test-setup.ts` wraps the top-level `await loadLowlight()` in try/catch. A transient import failure no longer crashes the entire vitest run — tests that hit a code block render the plain-text fallback and surface a warning. - `CodeColorizer.tsx` header comment updated to point at the AppContainer prefetch instead of claiming first-paint always sees a loaded instance. Reported by DeepSeek/deepseek-v4-pro and claude-opus-4-7 via Qwen Code /review and /qreview on QwenLM#4070. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * refactor(bundle): extract resolveBundleDir helper, apply to extensions/new Centralises the `chunks/` strip pattern that three sites (`i18n/index.ts`, `skills/skill-manager.ts`, `utils/ripgrepUtils.ts`) each duplicated after the round-3 fix in d581da0. The implicit coupling to `esbuild.config.js`'s `chunkNames: 'chunks/[name]-[hash]'` now lives in a single helper (`packages/core/src/utils/bundlePaths.ts`), so a future rename only needs updating in one place. Also applies the same anchor to `commands/extensions/new.ts:EXAMPLES_PATH`. That module is currently bundled into `cli.js` (so the strip is a no-op today), but `qwen extensions new --help` always reads the examples directory in its yargs `builder` — confirmed against the built bundle that the lookup hits `dist/examples/` (sibling of `cli.js`). Using the helper future-proofs against esbuild later hoisting the module into a shared chunk, where the bare `__dirname`/`import.meta.url` lookup would silently break the command for every end user. While here, surface lowlight-load failures from `AppContainer`'s prefetch effect to the debug channel (`debugLogger.warn`) instead of swallowing them silently. The loader already latches failures permanently, so this fires at most once per session; `CodeColorizer` continues to fall back to plain text on miss, so user-visible behaviour is unchanged. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(bundle): restore __filename shadow in ripgrepUtils; harden lowlight loader Round-4 review (wenshao 2026-05-13 13:12) flagged five issues in the recent code-split work. This commit addresses all of them. CRITICAL — `packages/core/src/utils/ripgrepUtils.ts`: the round-3 `resolveBundleDir` refactor removed the local `__filename` declaration but `getBuiltinRipgrep` still references bare `__filename` to decide how many `..` segments to walk. In `npm run dev` (tsx, ESM) `__filename` is undefined so the function throws `ReferenceError`. In the bundle esbuild's `define` rewrites it to `__qwen_filename` (the shim chunk path), which is the wrong string but happens to short-circuit to `levelsUp = 0` — accidentally correct only because the chunk-path string never contains `path.join('src', 'utils')`. Reproduced via tsx: `__filename is not defined`; fixed by re-introducing the explicit local shadow plus a comment explaining why centralising both helpers into `resolveBundleDir` cannot replace the per-file shadow. `packages/cli/src/ui/utils/lowlightLoader.ts`: the previous permanent `lowlightFailed` latch left syntax highlighting dead for the entire process lifetime on transient errors (EMFILE, antivirus locks, slow-disk-after-wake). Replaced with a 30-second cooldown — within the window subsequent calls return the cached rejection synchronously (keeps the per-render short-circuit that protects against permanently-broken installs); after the cooldown the next call retries the dynamic import. Exposes `isLowlightCoolingDown()` so render-hot callers can also skip duplicate failure logging. `packages/cli/src/ui/utils/CodeColorizer.tsx`: hoisted `loadLowlight()` + log out of the per-line render loop into a single `ensureLowlightLoading()` call at the top of `colorizeCode`. In the failure case this collapses hundreds of duplicate debug entries (one per line) to one per block. The instance is now passed down to `highlightAndRenderLine` as a parameter. `packages/core/src/utils/bundlePaths.ts` + `esbuild.config.js`: exposed `BUNDLE_CHUNK_DIR = 'chunks'` as a named constant and updated `esbuild.config.js` to interpolate the same name into `chunkNames` (plus an explicit "MUST stay in sync" comment). Renaming on one side without the other now stands out at review time. Also expanded the `define` comment with a contributor-facing warning describing exactly why bare `__dirname` / `__filename` in source files becomes the shim chunk path, and pointing future contributors at the `fileURLToPath(import.meta.url)` shadow pattern (and `resolveBundleDir` for sibling-asset lookups). Verified: - typecheck (all 4 workspaces): clean - packages/core tests: 7747 passing (no regressions) - packages/cli tests: only the pre-existing `useAtCompletion.test.ts` filesystem-order failures remain (confirmed against `git stash`) - `npm run bundle` succeeds; `node dist/cli.js --version` returns `0.15.10`; `node dist/cli.js --help` renders normally - `npx tsx <call getBuiltinRipgrep>` now returns the vendored path instead of throwing `ReferenceError` Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(bundle): validate lowlight API shape; sync doc-comment drift; add tests - lowlightLoader: validate runtime shape of createLowlight() before the `as Lowlight` cast so an upstream API rename routes through the cooldown latch instead of silently degrading every code block to plain text. - bundlePaths: correct doc comment — esbuild.config.js maintains its own `BUNDLE_CHUNK_DIR` constant rather than importing this one (it runs before any TS compile step). - AppContainer: update prefetch-failure comment to reference the cooldown symbols (`LOWLIGHT_RETRY_COOLDOWN_MS` / `lowlightLastFailureAt`) that replaced the removed `lowlightFailed` latch. - New unit tests covering the lowlightLoader state machine (success, in-flight dedup, shape mismatch, cooldown skip, post-cooldown retry) and `resolveBundleDir`'s strip-only-on-exact-match contract. * test(bundlePaths): use path.resolve for Windows-compatible absolute paths CI failure on Windows: the new `resolveBundleDir` tests built expected values with `path.join(path.sep, ...)` (e.g. `\tmp\dist`), but `pathToFileURL` resolves drive-less paths against the current drive on Windows. The URL -> `fileURLToPath` round-trip returned `D:\tmp\dist`, while the expectation stayed `\tmp\dist`, tripping all three new assertions. Switched both the URL source and the expected value to a single `path.resolve(path.sep, ...)` anchor per test so both sides absorb whatever the platform considers absolute. POSIX behaviour is unchanged (`/tmp/dist` -> `/tmp/dist`). --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
… tools reach the model (QwenLM#4166) * fix(core): refresh systemInstruction in setTools() so progressive MCP tools reach the model Under PR QwenLM#3994's progressive MCP path, Config.initialize() runs startChat() BEFORE MCP discovery starts, then kicks discovery off in the background and re-runs setTools() once it settles. But setTools() only updated chat.generationConfig.tools — not systemInstruction — and MCP tools are shouldDefer=true, so they were filtered out of declarations anyway. The prompt's "Deferred Tools" listing was frozen at the built-in-only snapshot from the initial startChat(), and the model had no signal that any MCP tool existed. Headless --prompt runs silently regressed to built-ins (issue QwenLM#4163); interactive mode had the same gap but was masked by retries. setTools() now rebuilds the system instruction with the up-to-date deferred summary and re-binds it to the live chat. The eager-reveal guard for "ToolSearch unavailable + deferred tools present" moves with it so a freshly-arrived MCP tool in `--exclude-tools tool_search` sessions still lands in declarations instead of disappearing silently. Shared with startChat() / refreshSystemInstruction() via a new private resolveDeferredToolsForSystemPrompt() helper so the three paths cannot drift apart again. The legacy synchronous path (QWEN_CODE_LEGACY_MCP_BLOCKING=1) was incidentally correct because discovery happened before startChat(); it remains correct. Test plan: - packages/core/src/core/client.test.ts — three new cases covering newly-arrived MCP tools, already-revealed filtering, and the no-ToolSearch eager-reveal path. - Full client.test.ts (107 tests) green. - tool-search / skill-manager / agent / mcp-client-manager / AppContainer test suites green (callers of setTools()). - CI integration: integration-tests/cli/simple-mcp-server.test.ts is expected to pass on first try without QWEN_CODE_LEGACY_MCP_BLOCKING. Fixes QwenLM#4163 Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(core): lock in SessionStart preservation across setTools refresh Adds the regression test chiga0 asked for in the PR QwenLM#4166 review: proves that setTools()'s setSystemInstruction-then-reapply pattern keeps the SessionStart hook's additionalContext intact, so progressive-MCP refreshes (AppContainer batch flush + the trailing setTools after waitForMcpReady) don't silently strip hook context from the system instruction. Generated by claude-opus-4-7 Co-authored-by: Claude <claude-opus-4-7@anthropic.com> --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Claude <claude-opus-4-7@anthropic.com>
…ls, upgrade @types/node (QwenLM#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: QwenLM#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 QwenLM#4096.
… of forcing a new one (QwenLM#4130) * fix(vscode-ide-companion): use existing editor group for diff instead of forcing a new one When the chat webview is in the leftmost group, opening a diff previously called ensureLeftGroupOfChatWebview() which forcibly created a new editor group. This was disruptive UX — there is often an existing empty group to the right that could be reused. Change the fallback chain from "left neighbor → force-create → Beside" to "left neighbor → right neighbor → Beside". Also apply the same fix to the readonly file opener in FileMessageHandler. * fix(vscode-ide-companion): address review feedback — explicit Beside fallback, shared scan helper, comment accuracy - Add ?? vscode.ViewColumn.Beside to targetViewColumn declaration so the fallback is explicit even if the downstream usage is reached without it - Extract findNeighborGroup helper to de-duplicate the near-identical scan loops in findLeftGroupOfChatWebview and findRightGroupOfChatWebview - Update stale comment in FileMessageHandler to reflect that the readonly document may open in the right group, not only the left * fix(vscode-ide-companion): remove dead ensureLeftGroupOfChatWebview, fix param naming, add tests - Delete ensureLeftGroupOfChatWebview and waitForTabGroupsCondition which are no longer called by any code path - Remove now-unused openChatCommand import - Rename _cur → cur in findNeighborGroup callbacks (param was used, the underscore prefix was misleading) - Add editorGroupUtils.test.ts with 12 unit tests for findLeft/findRight - Add createAndOpenTempFile viewColumn tests to FileMessageHandler.test.ts covering left-neighbor, right-neighbor, and Beside fallback cases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(vscode-ide-companion): align Beside fallback placement in FileMessageHandler with diff-manager Move vscode.ViewColumn.Beside fallback to the targetViewColumn declaration so both diff-manager and FileMessageHandler follow the same pattern: left ?? right ?? Beside at declaration, plain viewColumn at usage. * fix(vscode-ide-companion): fix TS2322 type error in FileMessageHandler.test.ts vscodeMock.window.tabGroups.all was initialized as plain [] which TypeScript infers as never[], causing assignment errors in CI. Add explicit type annotation to match the objects assigned in tests. * fix(vscode-ide-companion): clarify Beside fallback comment — covers missing webview too The fallback chain left ?? right ?? Beside also falls through to Beside when the chat webview group is not found (both helpers return undefined). Update comments in both diff-manager and FileMessageHandler. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* feat(cli): argument hint + --auto completion for /rename Closes QwenLM#4047. The /rename command supports a structured --auto flag (let the fast model generate a sentence-case title from the conversation), but unlike /model — which advertises --fast via argumentHint and a completion entry — /rename's flag was undocumented inline. Users had to either run the command incorrectly or check the docs to learn about --auto. - argumentHint: '[--auto] [<name>]' so the completion menu shows the shape when the user types `/rename` and tabs. - completion: returns null on empty / free-text input (don't shadow the user typing a title) and surfaces --auto when the partial arg is a prefix of it ('-', '--', '--a', '--au', '--auto'). Same shape as /model's --fast handling. Free-text titles intentionally don't auto-complete — there's nothing meaningful to suggest, and offering --auto on every keystroke would feel like noise on `/rename my-feature`. Tests: - pins argumentHint shape - empty partial → null - '-' / '--' / '--a' / '--au' / '--auto' all return the --auto suggestion - 'my-feature' / 'fix bug' / '-x' return null (free-text path) Co-Authored-By: Qwen-Coder <noreply@qwen.ai> * fix(core): fall back to text JSON when generateJson gets no tool call generateJson registers schemas as a respond_in_schema function declaration and walks parts[].functionCall for the result. When no tool_choice is set (the OpenAI-compatible converter never sets one) and the system prompt explicitly asks for text JSON — e.g. session-title generation's "Return ONLY a JSON object..." — some models honor the prompt and emit the answer as a plain text part instead of calling the tool. The answer is semantically correct; we just weren't reading it. This bottoms out in /rename --auto as "The fast model returned no usable title" on qwen3.6-max-preview, and likely affects every other generateJson caller (next-speaker checker, edit corrector, etc.) on the same class of model. Add a tolerant fallback: when no function call comes back, parse getResponseText(result) — which already skips thought parts — with a JSON-object extractor that strips optional ```json fences and reads the outermost {...} block. Strictly additive; the function-call path stays primary. Closes QwenLM#4057. Co-Authored-By: Qwen-Coder <noreply@qwen.ai> * refactor(cli): unify /rename and /rename --auto pipelines Bare /rename (no args) used to call a private generateKebabTitle path that asked the fast model (or main-model fallback) for a 2-4 word kebab-case name via a plain text call. /rename --auto used the schema-enforced tryGenerateSessionTitle path for a 3-7 word sentence- case title. Two code paths, two prompts, two failure-message formats, two sanitizers — with the kebab path consistently lagging on history filtering, surrogate handling, and error specificity. Collapse to a single fast-model schema-enforced pipeline. Both bare /rename and /rename --auto now call tryGenerateSessionTitle and both record titleSource: 'auto' on success. The --auto flag stays as an explicit user-intent marker (preserves the existing argumentHint / completion / parseArgs surface) but no longer diverges semantically. Bare /rename now also hard-requires fastModel; users who relied on the main-model fallback need to either /model --fast <name> or pass a name explicitly (/rename <name>). The new failure message points at both options. Co-Authored-By: Qwen-Coder <noreply@qwen.ai> * fix(cli): clarify rename title failure * test(core): cover loose json fallback --------- Co-authored-by: Qwen-Coder <noreply@qwen.ai>
* feat(cli): add baseline doctor memory diagnostics * fix(cli): address doctor memory review feedback * feat(cli): add doctor memory assessment * feat(cli): support doctor memory heap snapshots * feat(cli): add doctor memory sampling * fix(cli): harden doctor memory heap snapshots * fix(cli): harden doctor memory heap snapshots * fix(cli): harden memory heap snapshot diagnostics * fix(cli): harden doctor memory snapshots * fix(cli): stabilize heap snapshot cleanup ordering * fix(cli): harden heap snapshot cleanup * test(cli): cover memory snapshot fallbacks * fix(cli): harden doctor memory abort and disk checks
…M#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>
…ntinuation (QwenLM#4123) * feat(cli): add session-scoped /goal command with judge-driven turn continuation `/goal <condition>` pins a free-form objective for the rest of the session. While a goal is active, an LLM judge runs at every Stop boundary and either lets the turn end (condition met) or feeds the judge's reason back as the next user prompt to keep the model working. Auto-clears on success; `/goal clear` cancels early. Same primitive as Anthropic's Claude Code 2.1.140 `/goal`, built on qwen-code's existing Stop-hook + function-hook plumbing — no new subsystem. Core (packages/core/src/goals/): - activeGoalStore: per-session active goal + last-terminal cache, with a terminal-observer channel the CLI subscribes to so achieved/aborted cards land in history. - goalJudge: side-query against a fast model, transcript-grounded system prompt + json_schema response + disabled thinking. Tolerant JSON extraction with fallback so a flaky judge can't kill the loop; 30s default timeout (vs. the 5s function-hook default that was silently killing real-world judge calls). - goalHook: function hook on Stop. Returns {decision:'block', reason} when not met (reusing client.ts's existing recursive continuation), {continue:true} when met. Self-clears active goal + notifies the terminal observer on met/aborted. MAX_GOAL_ITERATIONS=50 backstop. CLI: - goalCommand: /goal | /goal <cond> | /goal clear|stop|off|reset|none| cancel. 4000-char cap, trust + disableAllHooks gates. Empty /goal shows running status, falls back to the last completed summary. - GoalPill: footer chip "◎ /goal active (12s)" — terse, claude-aligned. - GoalStatusMessage: set / checking / achieved / cleared / aborted history cards. "checking" replaces the generic stop_hook_loop chip for goal-driven iterations. - restoreGoal: on session resume, rehydrate the active goal hook + last-terminal cache from transcript so /goal survives /resume. Cross-cutting fixes: - HookSystem.hasHooksForEvent(eventName, sessionId?): also consults SessionHooksManager. Previously SDK / programmatic Stop function hooks were silently gated out by client.ts's fast-path check, so they never fired. - client.ts: yield StopHookLoop on every continuation iteration (was iter > 1) — first not-met turn is now visible in the UI. - useGeminiStream: commit pending item + clear thoughtBuffer / geminiMessageBuffer on every Finished event. Fixes a UI bug where a Stop-hook continuation's text bled into the prior turn's pending history item (cumulative "te" / "tes" rendering), even though the persisted transcript was clean. Co-authored-by: Qwen-Coder <noreply@qwen.ai> * test(cli): fix footer goal pill mock * fix(goal): persist terminal status on restore * fix(goal): harden judge hook * fix(goal): sanitize condition in instruction prompt and update matcher test - goalCommand.ts: collapse newlines and downgrade embedded double-quotes in the condition before splicing into the instruction prompt so the wrapping quote structure stays intact. - goalLoop.integration.test.ts: matcher assertion updated to '*' to match the current registerGoalHook contract (previously ''). Co-authored-by: Qwen-Coder <noreply@alibabacloud.com> * feat(goal): surface judge reason on terminal cards Renders `Last check: <reason>` on the achieved / aborted history card and on the empty-`/goal` summary so the final view records *why* the judge ruled the goal complete. Uses a single inline-label Text instead of the flex-row split used for `Goal:` — the reason is capped at 240 chars and almost always wraps; the flex-row variant hangs the continuation at the value column's left edge (~12 cols of blank space, easily mistaken for a stray empty line). Single Text + natural wrap keeps the continuation flush. Co-authored-by: Qwen-Coder <noreply@alibabacloud.com> * fix(goal): re-arm /goal on runtime /resume and /branch Cold boot path in AppContainer already calls restoreGoalFromHistory after loading session data, but the runtime /resume and /branch paths skipped it entirely. After /new + /resume back to a session that had an active /goal, the in-memory activeGoalStore entry still held the pre-/new setAt and a hookId pointing to a hook that config.startNewSession() had torn down — leaving the footer pill ticking from the original setAt (observable as "几十秒" elapsed immediately after resume) while the Stop hook was silently dead. Wire restoreGoalFromHistory into both handlers right after the session data lands so unregisterGoalHook clears the stale entry and registerGoalHook re-arms with a fresh setAt / hookId and re-installs the terminal observer. Co-authored-by: Qwen-Coder <noreply@alibabacloud.com> * refactor(goal): reuse shared formatDuration utility Drop the duplicated local formatDuration from goalCommand.ts and GoalStatusMessage.tsx in favor of the shared formatters.ts version, called with { hideTrailingZeros: true }. The shared util already has its own test suite and matches Claude Code's ShellTimeDisplay style (round values drop zero-unit tails: `5m 0s` → `5m`). Co-authored-by: Qwen-Coder <noreply@alibabacloud.com> * fix(goal): abort judge API call on judge timeout The judge-timeout path in judgeGoalWithTimeout only resolved a fallback verdict; the underlying judgeGoal generateContent call kept running because the hook context signal is never aborted by the timeout. Each timeout leaked one in-flight request that accumulated across goal-loop iterations. Link an AbortController into the judge signal and abort it when the timeout fires. Co-Authored-By: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(goal): harden judge continuation feedback * test(goal): align loop integration with safe continuation * fix(cli): harden goal resume lifecycle * fix(cli): address goal review blockers * fix(goal): guard stale same-condition callbacks --------- Co-authored-by: Qwen-Coder <noreply@qwen.ai> Co-authored-by: Qwen-Coder <noreply@alibabacloud.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…#4064) * feat(rewind): add file restoration support to /rewind command (QwenLM#3697) Previously /rewind only truncated conversation history — files modified by the assistant remained on disk. This adds a file-copy-based backup system (ported from claude-code's fileHistory) so users can optionally roll back file changes when rewinding. Core changes: - New FileHistoryService with snapshot/backup/restore lifecycle - trackEdit() called before each file write in edit and write-file tools - makeSnapshot() at each user turn boundary in client.ts - Three-phase RewindSelector UI: pick turn → choose restore option → execute - RestoreOption type: 'both' | 'conversation' | 'code' | 'cancel' Closes QwenLM#3697 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(rewind): replace findLast with reverse loop for ES2022 compat vscode-ide-companion targets ES2022 which lacks Array.findLast. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(rewind): add missing i18n translations and fix test expectation - Add file restore i18n keys to all 8 locale files (zh-TW, ca, de, fr, ja, pt, ru were missing) - Update useGeminiStream test to expect promptId in user history item 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(rewind): add getFileHistoryService mock to tool tests edit.test.ts and write-file.test.ts mock configs lacked the new getFileHistoryService method, causing trackEdit calls to throw. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(rewind): allow Esc during diff loading and add missing i18n footer strings Allow users to press Esc/Ctrl+C to cancel during diff stats loading phase. Add three missing footer navigation strings to all 9 locale files. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(rewind): address review feedback — restoreBackup correctness, missing promptId warning, dead code removal - restoreBackup now returns boolean; applySnapshot only counts a file as restored when the backup was actually applied (fixes misleading "Restored N file(s)" when backup is missing on disk) - Show warning when user selects file restore on a turn created before file checkpointing was enabled (promptId undefined) - Remove unused snapshotSequence field, canRestore(), and hasAnyChanges() methods that had no callers * fix(rewind): correct diff direction, truncate snapshots on rewind, add zero-files feedback - Swap diffLines args to diffLines(backup, current) so +/- stats match git convention (insertions = lines added since checkpoint) - Truncate snapshots after rewind to discard stale timeline state, preventing makeSnapshot from using wrong baseline - Show "No files needed restoration." when rewind finds files already at target state (all 9 locales) * test(tools): assert trackEdit is called before file writes * fix(i18n): add missing rewind UI locale keys across all 9 locales * fix(core): reset fileHistoryService on session change, clean up dead code - Reset fileHistoryService in startNewSession() so /clear gets a fresh instance with the new sessionId - Rebuild trackedFiles after rewind() to avoid stale stat() calls - Remove unused setCurrentPromptId/getCurrentPromptId dead API * fix(rewind): validate conversation before file restore, preserve snapshots for code-only - For 'both': validate conversation can be truncated before restoring files to prevent inconsistent state (files rolled back but conversation stays at newer state) - For 'code'-only: pass truncateHistory=false so snapshot timeline is preserved — conversation turns remain visible and their snapshots stay available for future rewinds * fix: correct trackEdit race comment — overwrite not orphan * fix(types): use HistoryItemWithoutId for addItem to preserve union member properties * fix(types): revert addItem type change, use cast at call site for promptId * fix(rewind): guard onRewind calls with .catch() to prevent unhandled rejection * fix(rewind): only truncate snapshot timeline when conversation truncation will execute * fix(rewind): address tanzhenxin review - gate, partial failure, tests 1. Disable file checkpointing for non-interactive (-p) mode by gating on `params.interactive !== false` in addition to `!params.sdkMode`. 2. Surface partial restore failures: `rewind()` now returns `RewindResult { filesChanged, filesFailed }`. In "both" mode, conversation truncation is skipped when any file fails to restore, preventing inconsistent state. 3. Add comprehensive unit tests for FileHistoryService (17 tests covering trackEdit, makeSnapshot, rewind, eviction, diffStats). * fix(rewind): defensive trackEdit + fix version collision on re-track 1. Wrap trackEdit calls in edit.ts and write-file.ts with try/catch so file history failures never break core tool operations. 2. Replace hardcoded version:1 in trackEdit with max-version lookup across all snapshots. Prevents backup file overwrite when the same file is re-tracked after a code-only rewind (truncateHistory=false). * fix(rewind): add missing i18n keys + fix makeSnapshot version collision 1. Add 'Failed to restore {{count}} file(s): {{files}}' to all 7 missing locales (ca, de, fr, ja, pt, ru, zh-TW). 2. Use global max-version scan in makeSnapshot (same as trackEdit) to prevent backup filename collisions after snapshot eviction. * fix(rewind): set hasRestoreFailure when promptId is missing In "both" mode, if the target turn has no promptId, conversation truncation was still proceeding because hasRestoreFailure was not set. Now correctly blocks truncation to prevent inconsistent state. * fix(rewind): show loading state during async restore, close selector in finally Defer setIsRewindSelectorOpen(false) to a try/finally block so the selector stays visible during async file restore. RewindSelector now manages its own isRestoring state: shows "Restoring..." text and disables all keypress handlers while the restore is in progress. This prevents the user from seeing a bare prompt with no progress indicator during slow restores, and eliminates the race where typing during restore could clobber the pre-filled prompt. * fix(rewind): skip timeline truncation on partial failure + fix wording 1. rewind() now only truncates the snapshot timeline when filesFailed is empty, preventing loss of future checkpoints when the caller skips conversation truncation due to failures. 2. Change "No files needed restoration." to the more idiomatic "No files needed to be restored." across all 9 locales. * fix(rewind): address review — TOCTOU in createBackup + outer catch in handleRewindConfirm - Extract safeCopyFile(src, dst) helper that distinguishes source-missing (TOCTOU: file deleted between stat and copyFile) from target-dir-missing, so trackEdit no longer silently fails when a file disappears mid-backup. Same helper now covers restoreBackup. - Wrap handleRewindConfirm with an outer catch that surfaces unexpected failures via historyManager error item; previously a sync throw from the post-rewind block would silently close the selector and leave 'both' mode in a half-applied state. - Add 'Rewind failed: {{error}}' i18n key in all 9 locales. * test(rewind): cover restoreFromSnapshots, trackEdit no-snapshot path, partial-failure timeline guard - restoreFromSnapshots: assert relative-path shortening + external-path preservation - trackEdit before any makeSnapshot: assert no-op early return - rewind truncation guard: assert snapshot timeline is preserved when filesFailed > 0 * fix(rewind): clean up orphaned backups, surface no-client states, polish - Per-eviction backup cleanup: when MAX_SNAPSHOTS overflow or rewind truncation drops snapshots, remove backup files no longer referenced by any surviving snapshot (best-effort, ENOENT-tolerant). Backup files are content-deduplicated across snapshots, so the live-set is computed from survivors before deletion. - Surface no-client failure modes in handleRewindConfirm: 'conversation' mode now shows an error instead of silently returning; 'both' mode shows an info message after restore so the user knows the conversation half was skipped. - i18n the previously hardcoded 'Conversation rewound...' message and add 3 new keys to all 9 locales. - Tighten createBackup signature (drop unreachable null branch). - Extract getMaxVersion helper to deduplicate identical loops in trackEdit and makeSnapshot. Tests added: orphan-cleanup on overflow, dedupe preservation, rewind truncation cleanup. All existing tests continue to pass (23 core, 71 AppContainer, 27 i18n). * fix(rewind): use path separator constant in maybeShortenFilePath The hardcoded '/' check meant Windows absolute paths (with '\') never matched the cwd prefix, so the shortening was a no-op on Windows. The new cleanup tests revealed this by asserting on the relative-path key: on Windows the key was the full absolute path, so trackedFileBackups lookups returned undefined. Switching to the platform sep also makes Windows snapshots use the relative key like POSIX, improving portability if cwd moves later. restoreFromSnapshots re-runs maybeShortenFilePath on every key, so existing on-disk sessions migrate transparently on resume. * test(rewind): cover trackEdit best-effort guarantees and unchanged-file rewind - edit.test.ts: assert tool still completes (file written, llmContent reflects the edit) when FileHistoryService.trackEdit rejects. - write-file.test.ts: same for the write_file tool. - fileHistoryService.test.ts: assert trackEdit swallows createBackup failures (forced via storageDir-replaced-with-file → ENOTDIR in recursive mkdir) without recording any backup. - fileHistoryService.test.ts: assert applySnapshot leaves a file untouched (mtime unchanged, filesChanged empty) when its content already matches the target backup — covers the checkOriginFileChanged short-circuit. * fix(rewind): align fileCheckpointing default + surface backup-missing on rewind Two issues from a Codex review pass: - Config: `fileCheckpointingEnabled` defaulted via `params.interactive !== false`, which resolves truthy when the caller omits `interactive` — but `this.interactive` itself defaults to `false`. Headless/programmatic callers that did not set `interactive` would silently start writing file-history backups under `~/.qwen/file-history/`. Use the same `?? false` default so the gate matches the resolved interactive value. - checkOriginFileChanged: when the on-disk backup AND the working file have both been removed externally, the function returned `false` ("unchanged"), so `applySnapshot` skipped `restoreBackup` and rewind reported success even though the target snapshot expected the file to exist. Treat any failure to stat the backup as "changed" so callers attempt the restore: applySnapshot surfaces the missing backup via restoreBackup → filesFailed, makeSnapshot creates a fresh backup. Added a regression test for the both-missing path. * fix(rewind): mark per-file backup failures so rewind surfaces them Two related issues from a /review pass: 1. Silent data loss in makeSnapshot inheritance: when the per-file backup attempt threw inside makeSnapshot, the catch block left the path missing from `trackedFileBackups`, and the inheritance loop then copied the previous snapshot's backup into the new snapshot. A later rewind to that snapshot would restore older content while reporting success. Now the catch records `{ failed: true, ... }` for the path. The inheritance loop skips paths already present in trackedFileBackups, so failed paths are no longer paved over by stale carryover. Both applySnapshot and getDiffStats honor `failed` — rewind pushes the path to filesFailed and the diff preview omits it. 2. Marketing/scope mismatch: the rewind UI offers "Restore code" but the feature only tracks edits made via the `edit` and `write_file` tools — shell-mediated changes (`sed -i`, `cp`, `rm`, `mv`, `npm`, etc.) and out-of-tool manual edits are not captured. Added a class-level JSDoc on FileHistoryService spelling out the scope, and an inline footer in the restore-options panel: "Rewinding does not affect files edited manually or via shell commands." (matching the upstream claude-code MessageSelector wording). New i18n key in all 9 locales. Test added: trackEdit/makeSnapshot per-file failure path. Asserts the new snapshot has `failed: true`, and that rewind to that snapshot reports the file as filesFailed instead of silently restoring the inherited stale backup. * fix(rewind): polish — i18n, type tightening, resumed-session UX hint Several small wins from the latest /review pass plus a UX mitigation for turns whose file-history snapshot is not present in memory (most often because the conversation came from a resumed session, but also when a turn has no captured edits): - AppContainer: wrap the "Cannot rewind to a turn that was compressed" error in t(); add the new key to all 9 locales. - RewindSelector: replace the inline `(+N -M in K file/files)` template literal with t() using two plural-aware keys; add to all 9 locales. - DiffStats.filesChanged: tighten from optional to required to match reality (every code path that returns a DiffStats sets it). Drops the `!.filesChanged!` non-null cascade in RewindSelector. - RewindSelector phase 2: when the option list does not contain code/both (i.e. no file-restore is actionable for this turn), show an explicit hint instead of leaving the user to guess why those options are missing. Same i18n key in all 9 locales. The mitigation hint covers the resumed-session case Tan raised (snapshots are not rehydrated by `/resume` today) without changing behavior — `getRestoreOptions` already gracefully degrades to conversation-only when `getDiffStats` returns undefined for a snapshot that is not in memory; we just surface the "why" to the user. * fix(rewind): unstick failed marker on the unchanged-file fast path The `failed: true` marker added in d598383 was sticky: once set, the no-change optimization in `makeSnapshot` would copy the failed entry forward into every subsequent snapshot for as long as the file stayed unchanged. A single transient I/O error therefore poisoned `/rewind` for that file until the user happened to modify the content again. Add `!latestBackup.failed` to the no-change reuse guard so a failed entry is never copied forward — the next snapshot retries the backup, which either heals (when the underlying I/O has recovered) or honestly records another failed entry. New regression test (`does not carry a failed marker forward when the file is unchanged`): - Snapshot p1 with file content X - Sabotage the storage dir → p2's per-file backup throws → p2 records failed: true - Restore the storage dir; file still equals X - p3 must NOT copy p2's failed entry; it must retry createBackup and produce a fresh non-failed entry that allows rewind to p3 to succeed
QwenLM#4299) (QwenLM#4300) Replace regex-on-`.message` classification in `mapDomainErrorToErrorKind` with two new typed error classes living next to `BridgeTimeoutError` in `status.ts`: - `BridgeChannelClosedError(context)` — replaces three `throw new Error('agent channel closed …')` sites in `httpAcpBridge.ts` (`getTransportClosedReject`, `getChannelClosedReject`, restore-time `transportClosed`). The `context` field preserves the legacy message wording verbatim so log greps and existing diagnostics keep working. - `MissingCliEntryError()` — replaces the generic `Error` thrown by `defaultSpawnChannelFactory` when neither `QWEN_CLI_ENTRY` nor `process.argv[1]` resolves. Message bytes preserved. `mapDomainErrorToErrorKind` now branches on `instanceof` for these two kinds, and the regex fallbacks (plus the TODO that called them out) are removed entirely — there are no remaining throw sites we don't control. The test suite gains a regression case: wrapping or foreign errors whose `.message` happens to contain "agent channel closed" or "Cannot determine CLI entry path" no longer misclassify (returns `undefined`, the correct behavior per the issue). Bridge-internal classes don't cross the JSON-RPC boundary into the ACP child, so `instanceof` symmetry is preserved at every consumer of `mapDomainErrorToErrorKind` (3 sites in `httpAcpBridge.ts`, 6 in `acpAgent.ts`). Closes QwenLM#4299 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
QwenLM#4306) Two regressions introduced by QwenLM#4271 (MCP guardrail push events) had been failing every main E2E run since the PR landed. Both fixes are in integration tests; no source changes. 1. `qwen serve — capabilities envelope > advertises all baseline capabilities`. `mcp_guardrail_events` was added to `SERVE_CAPABILITY_REGISTRY` and to the unit baseline list (`packages/cli/src/serve/server.test.ts:119`) but not to the integration test's hand-maintained list. Same drift class as QwenLM#4268 / QwenLM#4284. Fix: append the tag in registry order. 2. `MCP child amplification (P1 baseline) > clientCount matches external pgrep observation`. The test (added by QwenLM#4271, never passed CI) asserted `pgrep_observed === MCP_SERVERS_CONFIGURED`, ignoring that an ACP child runs TWO `Config` objects — bootstrap (`runAcpAgent` → `config.initialize`) + per-session (`newSessionConfig` → `config.initialize`) — each with its own `McpClientManager`. After one session, pgrep observes 2×N grandchildren while `/workspace/mcp` snapshot (`buildWorkspaceMcpStatus(this.config)`) reads only the bootstrap manager (=N). Fix: encode the 2× architectural amplification literally so a future follow-up that unifies the managers fails this assertion and forces an explicit update; keep `clientCount === MCP_SERVERS_CONFIGURED` and the original `clientCount ≤ pgrep` over-report guard intact. Verified locally: both tests pass on first attempt (no retries) via `vitest run --root ./integration-tests cli/qwen-serve-routes.test.ts cli/qwen-serve-baseline.test.ts`. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
…og/span consistency (QwenLM#4302) * fix(telemetry): Phase 1.5 polish — fallback order, abort-as-result, log/span consistency Follow-up to QwenLM#4126 (Phase 1: unify span creation paths). Four small fixes surfaced in the late review rounds; tracked in QwenLM#4212. 1. session-tracing fallback order — add resolveParentContext() helper that mirrors tracer.ts:getParentContext(): when interactionContext / toolContext ALS is empty, prefer the active OTel span over the synthetic session-root fallback. Without this, an LLM or tool call nested inside another OTel span (subagent inside a tool, instrumented HTTP path, etc.) re-parented to session root and flattened the trace tree. Three sites unified. 2. tool.execution span misreporting cancellation as success — when a tool observes signal.aborted and resolves with a normal ToolResult (no .error), the execution sub-span used to close as success while the parent tool span ended cancelled. Snapshot signal.aborted once after await, mirror it into endToolExecutionSpan, and stamp a sanitized error reason (Tool execution cancelled by user / Tool execution failed) so trace backends can distinguish the two without cross-referencing the parent. 3. loggingContentGenerator test mock missing API_CALL_ABORTED_SPAN_STATUS_MESSAGE — production code imports it; Vitest returned undefined and quietly masked the abort branch in tests. 4. Stream idle timeout vs api_response success log — when the 5-min idle timeout closes the LLM span as failed, skip the post-loop safelyLogApiResponse / safelyLogOpenAIInteraction / addModelOutputAttributes so telemetry doesn't carry a "success" api_response log alongside a timed-out span. Closes QwenLM#4212. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(telemetry): address QwenLM#4302 review — catch-path gating, exec span cancelled status, sync tags Four follow-ups from the QwenLM#4302 review on top of the Phase 1.5 polish: 1. loggingContentGenerator.ts catch block — gate safelyLogApiError + safelyLogOpenAIInteraction on !spanEndedByTimeout. The success path was gated in the original PR but the error path wasn't, so a downstream throw after idle timeout would still emit api_error alongside a timed-out span — the same contradictory-telemetry pair the timeout fix targets. 2. endToolExecutionSpan now accepts a `cancelled` discriminator. Without it, success: false unconditionally sets SpanStatusCode.ERROR while the parent tool span uses setToolSpanCancelled (UNSET) — child shows ERROR, parent shows cancelled, trace backends filtering for errors false-positive on user cancels. Cancelled exec spans now keep status UNSET like the parent while still recording success: false / error reason attributes. 3. coreToolScheduler.ts catch block now snapshots signal.aborted into a local `aborted` constant and passes it through endToolExecutionSpan + the if-branch — same idiom as the success path (QwenLM#4212), eliminating the divergent style. 4. SYNC tags on tracer.ts:getParentContext and session-tracing.ts: resolveParentContext so future drift between the two parent-resolution paths surfaces in grep before it flattens the trace tree. Tests: - session-tracing.test.ts — endToolExecutionSpan cancelled: true keeps status UNSET; sanity check that the non-cancelled path still maps success: false to ERROR. - coreToolScheduler.test.ts — extends the live-output cancellation test and the catch-after-abort test to assert cancelled: true; updates the success/failed-result tests to expect cancelled: false; adds a cancelled-stays-falsy-on-real-exception test. - loggingContentGenerator.test.ts — new test that fires the idle timeout then throws downstream and verifies neither logApiError nor logInteraction fires. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
…enLM#4273) * feat(cli): emit active goal stream events * feat(cli): support goal in non-interactive mode * fix(core): dedupe active goal stream updates * test(core): stabilize prompt hook duration test * fix(core): handle active goal stop hook abort * fix(core): keep active goal sync on aborted stop hook * fix(core): emit active goal before stop abort
) (QwenLM#4265) The guard at the top of `submitQuery` cleared `pendingRetryErrorItem` via `clearRetryCountdown()` without first persisting it to history, so any new user turn — including informational slash commands like `/status`, `/about`, or `/help` — would silently discard a visible API error. The user could not refer back to the failure they were investigating. The inline comment already described the intended behavior ("Commit any pending retry error to history (without hint) since the user is starting a new conversation turn") but the commit step was missing. Add the `addItem` call before clearing, stripping the now-stale "Press Ctrl+Y to retry" hint so the persisted history entry reads cleanly. Fixes QwenLM#4169
…ider seam (QwenLM#4175 PR 22b/2) (QwenLM#4304) * refactor(acp-bridge): lift BridgeOptions + introduce DaemonStatusProvider seam (QwenLM#4175 PR 22b/2) Design slice of QwenLM#4175 Wave 5 PR 22b. Lifts the bridge's construction contract (`BridgeOptions`) to `@qwen-code/acp-bridge` and introduces a new `DaemonStatusProvider` interface so the bridge factory no longer hard-imports daemon-host-specific helpers. The mechanical bulk lift of `BridgeClient` + spawn factory + `createHttpAcpBridge` factory closure (~3000 LOC) lands in PR 22b/3 against this stable contract. What's lifted - `BridgeOptions` interface (~150 LOC) from `httpAcpBridge.ts:189-325` → `acp-bridge/src/bridgeOptions.ts`. Adds new optional `statusProvider?: DaemonStatusProvider` field; everything else preserved verbatim. - `buildDaemonPreflightCells` + `safeCheck` (~210 LOC) deleted from `httpAcpBridge.ts:4002-4214` → moved to new `cli/src/serve/daemonStatusProvider.ts`. Logic byte-for-byte identical (slot-by-slot allocation, `Promise.allSettled` fan-out, classified error cells); only the call-site changes from `await buildDaemonPreflightCells(boundWorkspace)` to `await opts.statusProvider?.getDaemonPreflightCells(boundWorkspace) ?? []`. What's new - `DaemonStatusProvider` interface in `acp-bridge/src/bridgeOptions.ts`: two methods (`getEnvStatus`, `getDaemonPreflightCells`) covering the full set of daemon-host cells the bridge currently delegates. Direct positional args (boundWorkspace + acpChannelLive); both methods return `Promise<>` so future async impls (config-file reads, network probes) get the seam without breaking the bridge. No abort signal — YAGNI; future async needs can pull in a typed extension. Single combined interface (vs split into two) because env + preflight are conceptually one daemon-host status surface and the production impl ships them as one factory. - `createDaemonStatusProvider()` in `cli/src/serve/daemonStatusProvider.ts`: thin wrapper that calls `buildEnvStatusFromProcess` (still in `envSnapshot.ts`) and the moved `buildDaemonPreflightCells` body. Stateless; production callers can re-allocate per-request without performance cost. Bridge factory body changes - `getWorkspaceEnvStatus`: routes through `opts.statusProvider?.getEnvStatus(...)`. When `statusProvider` is omitted (Mode A in-process consumers, tests that don't care about env cells) the bridge returns an idle envelope `{ v, workspaceCwd, initialized: true, acpChannelLive, cells: [] }` matching the "queryable but empty" pattern PR 12 / 13 established for diagnostic routes. - `getWorkspacePreflightStatus`: routes the daemon-half through `opts.statusProvider?.getDaemonPreflightCells(...) ?? []`. ACP-side cells (auth / mcp_discovery / skills / providers / tool_registry / egress) keep being fetched separately via the existing `requestWorkspaceStatus` extMethod path; the bridge stitches the two halves together as before. Wiring - `runQwenServe.ts` passes `statusProvider: createDaemonStatusProvider()` into the bridge factory by default. - `httpAcpBridge.test.ts`: `makeBridge` helper now defaults to providing the production status provider so existing 4 env / preflight tests (which exercise the bridge's delegation path) keep seeing populated cells. Tests that want to exercise the no-provider idle fallback can override with `{ statusProvider: undefined }`. - `acp-bridge/package.json`: adds `./bridgeOptions` subpath export. - `acp-bridge/src/index.ts`: barrel re-exports `bridgeOptions.js`. Backward compatibility - `httpAcpBridge.ts` still re-exports `BridgeOptions` and `DaemonStatusProvider` types for callers via `from './httpAcpBridge.js'` (existing pattern from PR 22b/1). server.ts:31, runQwenServe.ts:16, workspaceAgents.ts, workspaceMemory.ts all keep resolving without churn. - Zero `/capabilities`, route, SDK, or behavior changes when the default `runQwenServe` is in play (production wiring is identical to pre-PR; the default `createDaemonStatusProvider()` reproduces the pre-injection cell shapes byte-for-byte). - Direct embeds / tests that omit `statusProvider` see new idle fallback semantics; this is the documented Mode A integration pattern and matches PR 12 / 13's "idle status is queryable" contract. Verification - `cd packages/core && npx tsc --build` clean - `cd packages/acp-bridge && npx tsc --noEmit` clean - `cd packages/cli && npx tsc --noEmit` clean (no errors in `src/serve/`; pre-existing tsconfig-excluded `mcp.test.ts` / `check-i18n.ts` errors unrelated to this PR) - `cd packages/cli && npx vitest run src/serve/` — 701/701 pass (16 test files; 4 env / preflight tests reuse the production provider via updated `makeBridge` helper) - `npm ci --dry-run` clean (no lockfile drift; new file imports resolve via existing `@qwen-code/qwen-code-core` and `@qwen-code/acp-bridge` workspace deps already present from PR 22b/1) Remaining PR 22b checkpoints - 22b/3 (mechanical bulk lift): `BridgeClient` (~400 LOC) + `defaultSpawnChannelFactory` (~250 LOC) + `createHttpAcpBridge` factory closure (~2240 LOC) + 5064-LOC test file move + final `httpAcpBridge.ts` shim. Zero new design decisions — factory consumes `DaemonStatusProvider` per the contract this PR froze. - 22b' (small follow-up): `WorkspaceFileSystem` injection into `BridgeClient.writeTextFile/readTextFile`. Closes PR 18 ws.ts:613 TOCTOU thread. - QwenLM#4299: `mapDomainErrorToErrorKind` regex → typed errors (parallel follow-up tracked from PR 22b/1 review). * docs(acp-bridge): catch README up to PR 22b/1 + 22b/2 module additions Self-review found the package README still listed only PR 22a's four modules (`eventBus`, `inMemoryChannel`, `channel`, `permission`) and hadn't been updated for the eight new ones added across the two follow-up slices. The "What's here today" section now covers all nine modules; consumers reading the README to figure out what to import know the full surface. Targeted minimal update — major restructure (drop the obsolete "What's not here yet" section + the "PR 22a (this)" status table) deferred to PR 22b/3 when the package surface stabilizes. Modules added to the list: - `status` (PR 22b/1) — wire contract status types + 27-symbol acp-integration import surface; mapDomainErrorToErrorKind classifier (regex → typed after QwenLM#4299/QwenLM#4300). - `workspacePaths` (PR 22b/1) — canonicalizeWorkspace + MAX_WORKSPACE_PATH_LENGTH. - `bridgeErrors` (PR 22b/1) — 11 typed bridge Error subclasses. - `bridgeTypes` (PR 22b/1) — BridgeSession*, HttpAcpBridge interface. - `bridgeOptions` (PR 22b/2) — BridgeOptions + DaemonStatusProvider seam. * fix(acp-bridge): address PR 22b/2 review feedback Four review-driven fixes from wenshao + gpt-5.5 + Copilot: 1. **wenshao [Critical] @ httpAcpBridge.ts** — `getEnvStatus` and `getDaemonPreflightCells` calls now wrapped in try/catch with stderr-logged fallback to the idle envelope / empty cells. The pre-injection `buildDaemonPreflightCells` used `Promise.allSettled` internally and was effectively unthrowable; the route's "daemon cells always render" invariant would have silently broken for any custom `DaemonStatusProvider` that throws. Fallback preserves the route-level guarantee structurally. 2. **wenshao/gpt-5.5 [Suggestion] @ httpAcpBridge.ts** — wired `createDaemonStatusProvider()` into the default bridge constructed by `createServeApp()` (in `server.ts`). Pre-fix, direct embeds / tests that didn't inject `deps.bridge` would silently lose the daemon env + preflight cells the default server app reported pre-injection. Now symmetric with `runQwenServe.ts`. 3. **wenshao [Suggestion] @ httpAcpBridge.ts** — extracted `createIdleEnvStatus(workspaceCwd, acpChannelLive)` helper into `acp-bridge/src/status.ts` (matches existing `createIdleAcpPreflightCells` pattern). Single construction site for `ServeWorkspaceEnvStatus`'s idle shape — future optional fields added to the production builder in `envSnapshot.ts buildEnvStatusFromProcess` won't silently diverge from the bridge's fallback. 4. **wenshao [Suggestion] @ httpAcpBridge.test.ts** — added 4 new tests covering the previously-untested provider-omitted / provider-throws paths: - `returns idle env envelope when statusProvider is omitted` - `returns empty daemon preflight cells when statusProvider is omitted` - `falls back to idle env envelope when statusProvider.getEnvStatus throws` - `falls back to empty daemon cells when statusProvider.getDaemonPreflightCells throws` Verification - `cd packages/core && npx tsc --build` clean - `cd packages/acp-bridge && npx tsc --build` clean - `cd packages/cli && npx tsc --noEmit` clean (no errors in `src/serve/`) - `cd packages/cli && npx vitest run src/serve/httpAcpBridge.test.ts` — 165/165 pass (4 new tests + 161 existing) - `cd packages/cli && npx vitest run src/serve/` — 705/705 pass on deterministic isolated runs; full-suite has known flakes unrelated to this PR (server.test.ts state pollution; pre-existing) Declined - **Copilot @ httpAcpBridge.ts:200** — `import type` after `export` statements claim against `import/first` ESLint rule. Verified `npx eslint src/serve/httpAcpBridge.ts` exits 0 — the rule isn't enforced on this file. False positive. * fix(acp-bridge): SkillError detection survives cross-package bundling (QwenLM#4298 review) Wenshao [Suggestion] on the merged PR QwenLM#4298 (QwenLM#4298 thread r3262781757): the same cross-package class-duplication problem the function already documents and works around for `TrustGateError` via `.name` matching applies to `SkillError` — and the existing code uses raw `instanceof SkillError`, which silently returns `false` when bundling produces duplicate class instances (a real risk once `acp-bridge` ever gets consumed outside the monorepo or under a bundler that doesn't dedupe `file:` workspace deps). Fix mirrors the `TrustGateError` pattern: `instanceof SkillError || .name === 'SkillError'`. When the synthesized branch fires we cast to read `.code` (TS can't narrow through the OR), then dispatch to the same `SKILL_PARSE_CODES` / `SKILL_FILE_CODES` lookup the genuine branch already used. Folding into PR 22b/2 rather than a standalone tiny PR because: - `status.ts` is already touched by 22b/2 (added `createIdleEnvStatus`) - the change is 4 LOC + 1 regression test, well below scope - one fewer review cycle Regression test in `status.test.ts` constructs synthesized SkillError instances (`Object.assign(new Error(...), { name: 'SkillError', code: ... })`) that fail `instanceof` but carry the right `.name` + `.code`, then asserts classification still resolves to `parse_error` / `missing_file` / `undefined`. Mirrors the existing `TrustGateError` regression test that pinned this exact pattern. Verification - `npx tsc --build` clean (core + acp-bridge) - `cd packages/cli && npx tsc --noEmit` clean - `cd packages/acp-bridge && npx vitest run` — all tests pass including new SkillError cross-bundle regression - `cd packages/cli && npx vitest run src/serve/httpAcpBridge.test.ts` — 161 pass * docs(acp-bridge): clarify env vs preflight fallback asymmetry (self-review) Self-review fold-in. Inline comment at the no-provider branch of `getWorkspacePreflightStatus` explains why the fallback shape differs from `getWorkspaceEnvStatus`: env is a single envelope (idle helper); preflight is the union of daemon-locality + ACP-locality cells stitched below, so an empty daemon slice IS the right fallback (the ACP slice fills in independently). Comment-only; zero behavior change. Caught during the comprehensive self-review pass between commits — the asymmetry is correct but a future maintainer might find it confusing without context.
…wenLM#4172) * docs: add async memory recall design spec and implementation plan * refactor(core): introduce MemoryPrefetchHandle, replace pendingRecallAbortController field * refactor(core): fire memory recall as non-blocking prefetch with settledAt flag * refactor(core): replace blocking await with zero-wait settledAt poll at UserQuery consume point Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(core): inject recalled memory on first ToolResult when UserQuery consume point misses Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(core): replace pendingRecallAbortController with pendingMemoryPrefetch in all cleanup paths Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(memory): remove 1s AbortSignal.timeout from relevanceSelector — caller controls lifetime Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(core): update auto-memory tests for async prefetch pattern — drop fake timers and deadline references Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(core): add ToolResult inject test — memory injected on first ToolResult when recall settles after UserQuery Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(core): address codex review findings on async memory recall Three findings fixed: 1. Abort previous prefetch before installing a new one (line 1059): A new UserQuery/Cron used to overwrite pendingMemoryPrefetch without aborting the old controller, leaking an unbounded background recall now that the 1s side-query timeout is gone. 2. Move the UserQuery consume poll AFTER the async reminder setup: ensureTool + listSubagents are awaited between the old poll location and the final assembly, so recalls that settled during those awaits used to be missed (and a tool-less turn never got a ToolResult retry). The poll now runs immediately before requestToSend assembly, and unshifts memory to the front of systemReminders to preserve ordering. 3. Append memory after functionResponse on ToolResult turns: The Qwen API requires the functionResponse part to immediately follow the model's functionCall (see lines 1209-1213). Prepending memory text risked breaking that pairing on the native Gemini path. Appending keeps the pair intact on Gemini and produces the same OpenAI output (text becomes a separate user message after the tool messages). Tests: - Updated ToolResult inject test to assert memory index > functionResponse - Added abort-previous-prefetch test (mid-flight UserQuery aborts old handle) 224/224 tests pass; tsc clean on changed files. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(core): add JSDoc + clarifying comments per review feedback Annotations only, no behavior change: - MemoryPrefetchHandle: full JSDoc covering lifecycle (create → consume → discard) - UserQuery consume site: explain why we unshift (front of systemReminders) - ToolResult inject site: reference hasPendingToolCall pattern instead of brittle line numbers when citing the Qwen functionCall/Response constraint - relevanceSelector.ts: explain why the side-query has no inline timeout (caller controls lifetime via MemoryPrefetchHandle.controller) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(core): bridge caller abort signal into memory prefetch + doc accuracy fixes Behavior fix (addresses copilot review on client.ts:1071): - When the parent sendMessageStream signal aborts (user Ctrl-C / Esc), the prefetch controller now aborts too. Previously the recall side-query would keep running until a later cleanup (next UserQuery / /clear / etc), wasting fast-model tokens on work whose result no one would consume. - Listener uses { once: true } and is also removed in the promise's finally() so a long-lived parent signal doesn't accumulate listeners across many turns under normal completion. - Edge case: if signal is already aborted when fire runs, abort the controller synchronously instead of attaching a listener. Test: - New regression guard: "should abort the pending prefetch when the caller signal aborts" — verifies the abort handler installed on the recall side fires once the parent signal aborts. Doc accuracy (addresses copilot review on the design spec): - ToolResult inject: was documented as "prepend", actual implementation appends to preserve functionCall/functionResponse pairing. Updated both the prose summary and the code sample. - Cleanup section: was documented as 6 abort-locations including the "post-consume clear"; the consume sites don't actually abort (the promise has already settled). Reorganized as 5 abort-and-clear sites + 2 clear-only sites with the distinction made explicit. - Fire path snippet: added the abort-previous-prefetch line and the caller-signal bridge so the spec matches the current implementation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(core): consolidate memory-prefetch lifecycle + safety nets per round-3 review Architectural (root-cause fix for cleanup-path sibling drift): - New private cancelPendingMemoryPrefetch() consolidates the abort+clear idiom (was duplicated across 6 sites). Logs at debug when discarding a settled-but-unconsumed handle so missing-memory scenarios are diagnosable. - New private tryConsumeMemoryPrefetch() consolidates the consume-and-mark-consumed dance (was duplicated UserQuery + ToolResult). - All existing cleanup sites + the two newly-flagged early-return sites (LoopDetected, Error) now use the helper; future early-returns can rely on the finally-block safety net. - sendMessageStream try-finally now uses a `normalCompletion` flag: only the bottom-of-try return path preserves the prefetch (intentional — next ToolResult turn may consume it); every other exit (uncaught exception, abnormal early-return) goes through cancelPendingMemoryPrefetch in finally. Diagnostics: - Restored AbortError debug log in fire-path catch (was silent after removing the deadline mechanism; aborts now come from 4+ sources so a trace is valuable). - Updated stale "deadline" log in recall.ts to reflect current abort sources (caller signal / new UserQuery / cleanup / 30 s safety timeout). Safety net: - Added 30 s ceiling in relevanceSelector via AbortSignal.any(...). Generous enough that normal ~1 s recalls don't trip it; bounds zombie side-queries if the model API hangs and the caller never aborts. Replaces the uncancellable `new AbortController().signal` fallback that would have left callerless invocations running indefinitely. Doc sync: - Design doc updated: UserQuery consume code sample now shows `unshift` (matches implementation) with an inline note on the prepend-vs-append contrast. Tests: - New regression guard: resetChat aborts pending prefetch and clears the handle. - New regression guard: LoopDetected mid-stream aborts pending prefetch and clears the handle (catches the sibling-drift bug this round caught). 227/227 tests pass; tsc clean on changed files. Declined from this round: - `await Promise.resolve()` after fire path: defensive — current code has multiple natural microtask drains before consume point. Added comment documenting the dependency instead. - Renaming `settledAt: number | null` to `settled: boolean`: timestamp has diagnostic value for future instrumentation; current consumers' null-check usage is documented in the JSDoc. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(test): correct getLastLoopType mock return type — null, not undefined CI tsc --build (stricter than --noEmit) caught: src/core/client.test.ts(2996,65): error TS2345: Argument of type 'undefined' is not assignable to parameter of type 'LoopType | null'. getLastLoopType()'s contract returns LoopType | null; the test mock was returning undefined. Switched to null to match the type. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(core): preserve memory prefetch across hook/next-speaker continuations + accurate recall abort log Round-4 review findings (self-inflicted regression from round-3): 1. Preserve pending prefetch on `return hookTurn` (Stop-hook continuation) and `return continueTurn` (next-speaker continuation). The round-3 `normalCompletion = true` was only set at the bottom-of-try `return turn`, leaving these two recursive-yield paths to trip the finally cleanup. When the inner Hook turn produced tool calls, the subsequent ToolResult turn found `pendingMemoryPrefetch === undefined` and memory was silently dropped. 2. recall.ts catch log distinguishes caller-driven aborts (heuristic genuinely skipped below) from the 30s safety-net timeout in relevanceSelector (the caller's signal is NOT aborted by that path, so the heuristic fallback actually runs). Regression guard added: - "should PRESERVE the pending prefetch when next-speaker continueTurn returns" — was red before this commit, green after. 258/258 tests pass; tsc --build clean. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…rktreeExitDialog, three-mode --resume restore (QwenLM#4174) * docs(worktree): update design doc — split Phase C/D, add Future section - Phase C: session persistence + hooksPath + StatusLine + WorktreeExitDialog - Phase D: --worktree CLI flag + symlinkDirectories - Future: sparse checkout, .worktreeinclude, tmux, PR reference parsing - Feature comparison table updated with Phase A/B completion status Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(worktree): add Phase C implementation plan 8 tasks: WorktreeSession sidecar storage, hooksPath setup, EnterWorktree/ExitWorktree session wiring, useWorktreeSession hook, Footer display, --resume context injection, WorktreeExitDialog. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(worktree): update Phase C plan after claude-code comparison - WorktreeSession: add originalHeadCommit field - hooksPath: add .husky/ detection + skip-if-already-set logic - StatusLine payload: expand worktree field to match claude-code schema - WorktreeExitDialog: load dirty state on mount, display counts in dialog - UIState.activeWorktree: add originalCwd, originalBranch, originalHeadCommit Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(worktree): add WorktreeSession sidecar storage New worktreeSessionService.ts exposes read/write/clear functions for the sidecar JSON file at <chatsDir>/<sessionId>.worktree.json. SessionService gains getWorktreeSessionPath() so callers don't need to know the layout. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(worktree): configure core.hooksPath after worktree creation createUserWorktree() now sets `core.hooksPath` inside the new worktree to the main repo's hooks directory (.husky preferred, .git/hooks fallback) so commits inside the worktree run the same pre-commit checks as the main repo. Mirrors claude-code's performPostCreationSetup logic — skips the subprocess when the value already matches to avoid ~14ms spawn overhead. Failures are non-fatal: the worktree is still usable without hooks. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(worktree): persist WorktreeSession sidecar in EnterWorktreeTool After creating a worktree, EnterWorktreeTool now writes a sidecar JSON file at <chatsDir>/<sessionId>.worktree.json with the full session state (slug, paths, branches, original HEAD SHA). --resume reads this in Phase C task 7 to restore worktree context. Best-effort: write failures don't abort the creation. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(worktree): clear WorktreeSession sidecar in ExitWorktreeTool After successful keep or remove, ExitWorktreeTool now clears the sidecar JSON file iff its slug matches the worktree being exited. The slug check prevents wiping the sidecar when the user exits a worktree that isn't currently tracked (multiple worktrees on disk, sidecar tracks one). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(worktree): expose active worktree via useWorktreeSession + UIState New useWorktreeSession hook watches the sidecar JSON file (created by EnterWorktreeTool, deleted by ExitWorktreeTool) and returns the current WorktreeSession or null. AppContainer wires it into a new UIState.activeWorktree field consumed by Footer (Task 6) and WorktreeExitDialog (Task 8). A showWorktreeExitDialog state placeholder is added too, hardcoded false until Task 8 wires the dialog trigger. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(worktree): show active worktree in Footer + StatusLine payload Footer renders `⎇ <branch> (<slug>)` when activeWorktree != null, but only when the user has no custom statusline (their script likely handles it from the stdin payload itself). useStatusLine's StatusLineCommandInput gains a `worktree` field with {name, path, branch, original_cwd, original_branch} — matches claude-code's schema so statusline scripts can be shared across both CLIs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(worktree): inject context hint on --resume when worktree is active On --resume, if the session has a WorktreeSession sidecar, append an INFO history item pointing the model at the worktree path so it continues using it for file operations. Stale sidecars (worktree dir deleted out-of-band) are cleaned up so the Footer indicator doesn't go stale. qwen-code can't process.chdir() the way claude-code does because Config.targetDir is immutable; the context hint is the equivalent behavioral cue. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(worktree): add WorktreeExitDialog with dirty-state inspection WorktreeExitDialog renders when the user double-presses Ctrl+C inside a worktree. On mount it runs `git status --porcelain` and `git rev-list --count <originalHeadCommit>..HEAD` to show how many uncommitted files and new commits the user would discard by choosing "Remove". The dialog never auto-removes — every exit goes through explicit user confirmation per requirements. handleExit in AppContainer intercepts the second-press quit when activeWorktree is set and shows the dialog instead. A new UIAction handleWorktreeExit(choice) routes the user's choice through removal (via GitWorktreeService.removeUserWorktree) + sidecar cleanup + /quit. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(worktree): add Phase C E2E test plan Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(worktree): fix E2E test plan sidecar path + jq selector - sidecar lives at ~/.qwen/projects/<sanitized-cwd>/chats/, not ~/.qwen/tmp/<hash>/ - qwen --output-format json emits a JSON array, not NDJSON — jq needs .[] Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(worktree): add showWorktreeExitDialog to dialogsVisible Phase C task 8 introduced showWorktreeExitDialog state and the dialog render in DialogManager, but missed adding the flag to the dialogsVisible OR expression. DefaultAppLayout only renders DialogManager when dialogsVisible is true, so the dialog was never shown — second Ctrl+C in a worktree silently absorbed instead of triggering the prompt. Caught by Group E E2E tests. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(worktree): extend --resume context restore to headless + ACP modes Phase C task 7 originally placed the worktree-restore logic in AppContainer.tsx (TUI only). E2E Group C exposed that headless and ACP modes never run AppContainer, so stale sidecars accumulate and the model loses worktree context after --resume. Refactor to a shared `restoreWorktreeContext` helper in core, then wire the three entry points: - TUI (AppContainer): keep historyManager.addItem(INFO) UX, route via the helper. - Headless (nonInteractiveCli): prepend the notice as a system-reminder block on the user prompt; emit a `worktree_restored` system message to the JSON adapter so SDK consumers can react. - ACP (Session.pendingWorktreeNotice): set by acpAgent.loadSession on resume, consumed and cleared exactly once on the next #executePrompt. All three modes call the same helper, so stale-sidecar cleanup is consistent. Helper covers: missing sidecar, live worktree dir, deleted worktree dir, regular file at worktreePath, malformed JSON. 5 new unit tests for restoreWorktreeContext (13/13 pass total). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(worktree): add ACP-mode integration tests for --resume context Covers: - acpAgent.worktree.test.ts (3 tests): loadSession sets pendingWorktreeNotice only when worktree dir is live, clears stale sidecar otherwise, swallows restoreWorktreeContext errors. - Session.worktree.test.ts (4 tests): #executePrompt prepends the system-reminder block exactly once on first prompt, clears the pending notice, second prompt sees no leakage, no-op when nothing was set. E2E via real ACP protocol is impractical without a Zed client; these tests cover the integration boundaries directly. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(worktree): clarify hooksPath comment + pendingWorktreeNotice one-shot rationale Two doc-only fixes from PR QwenLM#4174 review: - gitWorktreeService.ts: previous hooksPath comment overstated the optimization (claimed claude-code's ~14ms saving but we still do a read subprocess). Rewrite to be explicit: write-skip only, read retained, parseGitConfigValue's full optimization deliberately not ported because the read happens once per worktree creation. - Session.ts: pendingWorktreeNotice doc now explains why it's one-shot (after the first prompt the worktree path is already in conversation context; re-injecting would clutter history without adding signal). No behavior change. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(test): add getResumedSessionData to nonInteractiveCli mock Config CI surfaced TypeError: config.getResumedSessionData is not a function across 12 tests in nonInteractiveCli.test.ts. The Phase C ada0837 commit added a worktree-restore call in the headless path that probes config.getResumedSessionData(); the mock Config never had that method. Return undefined to short-circuit the restore block — these tests don't exercise --resume. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(worktree): address PR QwenLM#4174 reviewer findings Bundled response to the two review rounds. Per-thread replies follow. CORE — worktree sidecar robustness (Findings 3252368644, 3252368651, 3255171690): - atomicWriteJSON instead of fs.writeFile (no more half-written sidecar after a crash) - readWorktreeSession now schema-validates the parsed object and returns null on missing/wrong-type fields instead of propagating undefined into consumers - restoreWorktreeContext clears the sidecar on JSON parse failure / read I/O error so a corrupted file doesn't block every subsequent --resume CORE — hooksPath setup (Finding 3252368645): - configureHooksPath distinguishes ENOENT (benign "candidate not present") from real stat errors (EACCES/EIO/ENOTDIR); the latter are warn-logged so a silently-degraded hooksPath is visible to operators CLI — handleWorktreeExit Remove path (Findings 3252368637, 3252368640 a+b): - Anchor GitWorktreeService at activeWorktree.originalCwd (the captured repo root), not config.getTargetDir() — fixes monorepo-subdirectory launches where the worktree lives under the repo root but getTargetDir points at a subpackage - Check removeUserWorktree return value; on failure, leave the sidecar intact so --resume can recover (previous code cleared it regardless) - Pass forceDeleteBranch:true to honour the dialog's "discards N commits" label — without it `git branch -d` refused unmerged commits and the branch was silently preserved CLI — useWorktreeSession watcher (Finding 3252368648): - Normalize fs.watch filename via toString() so the Linux-Buffer code path triggers reloads (previous comparison silently never matched) - Treat null filename as "unknown, reload to be safe" (recursive watchers on some platforms emit events without a payload) CLI — WorktreeExitDialog (Findings 3252368650, 3255171694): - execGit now correctly reads numeric exit codes from .code/.status (NodeJS.ErrnoException.code is a string for spawn errors, number for subprocess exits); previous typeof === 'number' check always missed - Dialog body shows an "⚠ Could not measure worktree state (...)" banner when git status / rev-list failed, so the user doesn't see a misleading "0 files, 0 commits" before choosing Remove CLI — closeAnyOpenDialog (Round 2 review body): - Wire WorktreeExitDialog into the standard dialog-dismissal path so Ctrl+C dismisses it the same way it dismisses every other dialog TEST FIXES — vitest timeouts: - Real git invocations + user-global hooks (e.g. trustup post-commit webhooks) can take 10–20s per setUp on CI. Bump testTimeout + hookTimeout to 30s for the three integ test suites that spawn git (Phase B/C worktree integ tests) so the suite isn't flaky. NEW TESTS: - worktreeSessionService.test: 3 new cases covering malformed JSON, missing required fields, wrong-type fields, malformed sidecar cleanup, partial sidecar cleanup (16 total, up from 13). - useWorktreeSession.test.tsx: 4 new cases — null when no sidecar, parsed sidecar at mount, reacts to delete, reacts to creation. - WorktreeExitDialog.test.tsx: 1 new case — loading frame renders before git probes resolve. (Async dialog states tested via E2E — vi.mock of execFile in ink-testing-library doesn't fire mock impl reliably.) - nonInteractiveCli.test: 3 new "Phase C --resume" cases — system-reminder injection on live worktree, no injection when sidecar absent, stale sidecar cleanup when worktree dir is gone. DECLINED FINDINGS (replied on threads): - 3252368642 (Dialog Keep clears sidecar) — declined-design. Dialog Keep = "exit app, keep worktree for next --resume"; tool Keep = "I'm done with this worktree". Intentionally different semantics. - 3252368643 (originalHeadCommit base branch) — false-positive. There is no base_branch parameter; getCurrentCommitHash() returns HEAD which equals the tip of the current branch (== baseBranch in createUserWorktree). - 3252368640 part c (bypass safety guards) — declined-design. The dialog IS the safety affordance for this path — it shows dirty-state counts and asks for explicit user confirmation before removal. - 3255171696 (DialogManager async fire-and-forget) — false-positive. handleSlashCommand('/quit') is inside the await chain in handleWorktreeExit, so the described race ("process.exit before remove completes") cannot occur. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(test): correct linter-mangled imports in useWorktreeSession.test Pre-commit hook auto-fixed imports collapsed value imports (writeWorktreeSession, clearWorktreeSession) into an `import type` block, breaking runtime resolution. Split back into value + type imports. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(test): normalize path separators for Windows in worktree session integ Windows CI failure: `repoRoot` from Node's `fs.mkdtemp` returns backslash-separated paths (`C:\Users\runneradmin\…`), but `originalCwd` in the sidecar comes from `getRepoTopLevel()` which delegates to `git rev-parse --show-toplevel` — git on Windows returns forward slashes (`C:/Users/runneradmin/…`). The Windows-only assertion `expect(originalCwd).toBe(repoRoot)` was comparing two different representations of the same canonical path and rightly failed on `Object.is` equality. Compare via path.normalize on both sides so the assertion holds across platforms without changing the runtime path (originalCwd still records git's output verbatim, which is what consumers expect since other places in the codebase that read `getRepoTopLevel()` also work with that shape). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(worktree): address PR QwenLM#4174 round 4 findings Finding #3256237933 (Critical, follow-up to #3252368640 part 1): handleWorktreeExit silently /quit'd when removeUserWorktree returned {success:false}, contradicting the user's intent after they clicked "Remove worktree and branch (discards N commits, M files)". Now surfaces an ERROR history item with the underlying error message and STAYS in the session so the user can decide what to do (retry via exit_worktree, fix the lock/permission/corruption issue, or quit anyway). Same treatment applied to the hard-failure catch block — previously it caught the throw and proceeded to /quit with no log; now it emits the error and stays alive. Finding #3256236050 (Nit): originalCwd field name implies "user's launch cwd" but actually stores `getRepoTopLevel()` (different in monorepo subdir launches — the gap closed by #3252368637). Renaming the field would force on-disk migration of every existing sidecar (every active --resume breaks until users wipe the old file). Doc-only fix: WorktreeSession.originalCwd now carries an explicit JSDoc explaining the semantics and warning consumers expecting process.cwd() to NOT use this field. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(worktree): address PR QwenLM#4174 round 5 findings Finding #3256241831 (Nit, but awareness UX): the built-in `⎇` indicator used to disappear whenever `statusLineLines.length > 0`, on the assumption that the user's custom statusline rendered worktree itself. That assumption is unsafe — scripts written before Phase C don't know about `payload.worktree`, scripts can deliberately ignore the field, and partial scripts may render some fields but not worktree. In any of those cases the user sees no worktree UI while having an active worktree, risking destructive operations in the wrong cwd. New behavior: indicator shows by default regardless of statusline. Added an opt-out setting `ui.hideBuiltinWorktreeIndicator` (default false) for users whose custom statusline already renders worktree and want to avoid duplication. Finding #3256239608 (Nit): `fs.watch` in useWorktreeSession holds an inode handle to `chatsDir` at mount time. If the directory is deleted out-of-band (manual cleanup, antivirus quarantine, reset scripts) and recreated, the watcher does NOT re-attach to the new inode and the Footer indicator stops reacting to sidecar changes. Reviewer explicitly accepted this as a documented limitation rather than adding polling-fallback or error-event-handler complexity for an edge case that doesn't arise in normal use. Added a JSDoc block on the hook explaining the limitation and pointing to the future fix shapes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(worktree): regenerate settings.schema.json for hideBuiltinWorktreeIndicator CI Lint step caught that the JSON schema mirror in packages/vscode-ide-companion was out of date after adding the new ui.hideBuiltinWorktreeIndicator setting in 80f9cb4. Regenerated via `npm run generate:settings-schema`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(worktree): address PR QwenLM#4174 round 6 findings Critical fixes: - #3259975247: TUI dialog Remove now reads the in-worktree session marker and refuses to delete a worktree owned by a different session — same ownership guard ExitWorktreeTool already applies. Stale/copied sidecars can no longer destroy another session's work. - #3259975249: TUI --resume queues a one-shot pendingWorktreeNotice ref consumed by handleFinalSubmit; the user's first prompt is prefixed with the same <system-reminder> block headless/ACP use. Previously only the INFO history item showed in the transcript (UI-only), so resumed models could silently edit the parent checkout. - #3259975245: exit_worktree action='keep' no longer clears the sidecar. `keep` means "preserve the worktree for later"; clearing the persisted binding broke --resume / Footer / WorktreeExitDialog for kept worktrees. Now matches the Dialog keep semantics. Test updated to assert preservation instead of clearing. - ACP unstable_resumeSession parity: factored the worktree restore block into #restoreWorktreeOnResume() and called from both loadSession() and unstable_resumeSession(). ACP clients using resume no longer miss the worktree context. Suggestion-level fixes: - #3259975237: configureHooksPath now resolves the canonical hooks dir via `git rev-parse --git-common-dir` instead of constructing `<sourceRepoPath>/.git/hooks`. The construction assumed .git is a directory, but when Qwen runs from a linked worktree it's a file pointing at the real gitdir → ENOTDIR → silent no-hooks worktree. - #3259975242: only writes core.hooksPath when the key is unset. A non-empty inherited or user-configured value is preserved instead of being silently replaced. - #3256839787: restoreWorktreeContext adds a structural invariant check — worktreePath must live under <originalCwd>/.qwen/worktrees/. A tampered/copied sidecar pointing at an arbitrary existing dir is rejected and cleared so the model can't be redirected. Tests: - worktreeSessionService.test: 17/17 (added prefix-escape rejection case + restructured the existing live-worktree case to satisfy the new structural invariant). - exit-worktree.session.integ.test: rewrote keep test to assert preservation (matches new behavior). - nonInteractiveCli.test: updated fixture worktreeDir to live under <originalCwd>/.qwen/worktrees/ for the prefix invariant. - All other suites pass without modification. Test coverage gap acknowledgement (no comment_id reply): per-handler unit tests for handleWorktreeExit + dialog post-load states remain covered by the E2E Group E suite in docs/e2e-tests/worktree-phase-c.md. The execFile mock path in ink-testing-library still doesn't deliver async useEffect state transitions reliably, so unit testing those states adds more harness than signal; deferring. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…enLM#4219) (QwenLM#4262) * fix(core): apply defaultModalities() on env-var-only model config (QwenLM#4219) When qwen-code is configured only via env vars (OPENAI_API_KEY / OPENAI_BASE_URL / OPENAI_MODEL) with no modelProviders entry, resolveGenerationConfig() never invoked defaultModalities(), so generationConfig.modalities stayed undefined for image-capable models. The two other config paths (modelRegistry.resolveModelConfig and modelsConfig.applyResolvedModelDefaults) already call it. This aligns the env-var-only path with both so multimodal models like qwen3.6-35b-a3b correctly accept @image attachments. Fixes QwenLM#4219 * test(core): lock modalities fallback invariants on env-var-only path Address review feedback on PR QwenLM#4262: - Strengthen the positive regression test to also assert video:true and source kind ('computed'), matching the source-tracking convention used elsewhere in this file and catching regex regressions in modalityDefaults. - Add negative case: unknown model → modalities resolves to {} (text-only), never undefined — the key invariant introduced by the fix. - Add negative case: explicit settings.generationConfig.modalities is not clobbered by the fallback (lock the `=== undefined` guard). - Extend the fallback's comment to document the undefined → {} semantic so future maintainers don't reintroduce `modalities === undefined` branches. No behavior change. * test(core): pin Qwen OAuth modalities auto-detect for coder-model Round-2 review feedback on QwenLM#4262: `resolveGenerationConfig` is shared by both the OpenAI/env-var-only path and `resolveQwenOAuthConfig`, which passes `resolvedModel` (defaults to 'coder-model') as modelId. So the new modalities fallback also activates for Qwen OAuth — a real behavior change (was undefined, now { image: true, video: true }). The change is desired (coder-model supports vision per the existing warning text in resolveQwenOAuthConfig), but no test pinned it down. Add a regression test so future MODALITY_PATTERNS edits can't silently shift Qwen OAuth behavior.
… consumer (QwenLM#4308) * fix(cli): block Windows Tab approval-mode toggle when input has a Tab consumer Closes QwenLM#4171. On Windows, Shift+Tab is indistinguishable from a bare Tab in many terminals, so useAutoAcceptIndicator accepts a bare Tab as the approval-mode cycle shortcut. To avoid double-firing with the input area, AppContainer passes a `shouldBlockTab` callback that suppresses the cycle when the input has its own Tab handler. Until now that callback only tracked the autocomplete dropdown (`shouldShowSuggestions`). When the buffer was empty and the followup prompt-suggestion ("input prediction") was visible, pressing Tab on Windows accepted the suggestion *and* cycled approval mode at the same time — the exact behaviour reported in QwenLM#4171. The mid-input ghost-text and reverse/command-search paths had the same gap. Broaden the signal: compute `hasTabConsumer` from every Tab consumer inside InputPrompt — autocomplete dropdown, followup suggestion, mid-input ghost text, reverse-search, command-search — and feed that into `shouldBlockTab`. A single Tab keystroke now triggers exactly one action on Windows; macOS and Linux behaviour is unchanged. Tests cover the four states (followup visible, ghost text visible, autocomplete visible, idle). * fix(cli): tighten hasTabConsumer, add unmount cleanup + tests (QwenLM#4308 review) Three review findings on PR QwenLM#4308 addressed together — all touch the same `hasTabConsumer` signal surface exposed from InputPrompt to AppContainer. 1. **Tighten signal semantics (Copilot)**: drop the standalone `reverseSearchActive || commandSearchActive` terms. When those overlays have matches, their `showSuggestions` flag already flows into `shouldShowSuggestions` and Tab is consumed via `ACCEPT_SUGGESTION_REVERSE_SEARCH`. When they're active without matches, Tab is NOT consumed — including the bare flags misrepresented the signal as "Tab consumer present" when it really meant "modal overlay open". `hasTabConsumer` now strictly matches its name. 2. **useEffect cleanup on unmount (wenshao)**: previously, if any Tab consumer was active when InputPrompt unmounted (e.g. streaming begins while autocomplete is open), AppContainer's `hasTabConsumer` state retained the stale `true` value and kept blocking Windows Tab approval-mode cycling for the entire unmount window. Effect now resets to `false` on cleanup. The pre-existing code had the same gap with one trigger; expanding to 3 triggers materially raised the likelihood. 3. **JSDoc on prop name (wenshao)**: `onSuggestionsVisibilityChange` now carries broader "Tab consumer" semantics than the name suggests. Cross-file rename across UIActionsContext + Composer + AppContainer is too much churn for QwenLM#4308's scope; add JSDoc on the prop declaration documenting the broader signal and that the name is retained for backward compatibility. 4. **Test coverage (wenshao)**: add two tests — autocomplete dismissal reports `false` (true→false transition); unmount-while-active reports `false` (cleanup regression guard). * fix(cli): split Tab-consumer signal so it doesn't hide Footer (QwenLM#4308 review) Self-inflicted regression caught by wenshao: the previous round broadened `onSuggestionsVisibilityChange` from "autocomplete dropdown visible" to "any Tab consumer present", but Composer.tsx was using that same callback for a different purpose — hiding the Footer / KeyboardShortcuts when the dropdown would overlap their vertical space. As a result, followup prompt suggestions and mid-input ghost text (both inline within the input box, neither competing for vertical space) were also hiding the Footer on every platform. Split into two signals: - `onSuggestionsVisibilityChange` — narrow, autocomplete dropdown only. Kept local to Composer for Footer hiding. Restored to pre-PR semantics; no cleanup-on-unmount needed (the entire conditional in Composer.tsx is already gated by `uiState.isInputActive`, which goes false when InputPrompt unmounts). - `onTabConsumerChange` — broad, any input-side Tab consumer (autocomplete + followup + ghost text). Plumbed through UIActionsContext to AppContainer's `hasTabConsumer` state → useAutoAcceptIndicator's `shouldBlockTab`. Retains the cleanup-on-unmount wenshao added last round (the broad signal IS read while InputPrompt is unmounted). Tests: - All 6 broad-signal regression tests renamed to assert `onTabConsumerChange`. - 3 new narrow-signal regression tests pin that `onSuggestionsVisibilityChange` does NOT fire `true` for followup or ghost text. Catches the exact shape of my regression.
* feat(core): extend cross-auth fast models to agents * fix(core): tighten cross-auth model resolution fallbacks When a forked-agent caller passes a selector that cannot resolve (e.g. `fast` with no fast model configured), fall back to the parent session model instead of forwarding the raw selector string to the provider. Matches the subagent path, where unresolvable selectors mean "inherit parent". In BaseLlmClient.createContentGeneratorForModel, do not cache the unregistered-model fallback. getCurrentContentGenerator() reads the runtime view from AsyncLocalStorage, which can differ between calls; caching would pin the first call's view-bound generator under the selector key and reuse it on later calls after that view has unwound. * docs(core): drop stale getFastModelForSideQuery from sideQuery JSDoc The function was removed when fast-model resolution collapsed onto getFastModel(); the JSDoc fallback chain still mentioned it.
B-A-M-N
force-pushed
the
feat/directory-remove-v2
branch
2 times, most recently
from
May 20, 2026 00:04
6f91cf9 to
55ead30
Compare
Add /directory remove subcommand with tab-completion, initial directory guards, workspace settings persistence, and restrictive sandbox check. Warn on startup when --add-dir paths don't exist or aren't readable. Changes: - directoryCommand.tsx: new 'remove' subcommand (action, completion, error handling) - directoryCommand.tsx: remove persists to context.includeDirectories in settings - directoryCommand.tsx: scope-aware settings lookup (User + Workspace) - directoryCommand.tsx: resolve persisted raw entries via expandHomeDir/realpath - directoryCommand.tsx: warn if no persisted entry found after removal - directoryCommand.tsx: restrictive sandbox check (consistent with add) - directoryCommand.test.tsx: 5 new tests for remove subcommand - config.ts (cli): improved --add-dir help text description - config.ts (core): startup warning via process.stderr for invalid --add-dir paths - workspaceContext.ts: track skipped directories, expose getSkippedDirectories() - workspaceContext.test.ts: 4 new tests for getSkippedDirectories() - en.js, zh.js, zh-TW.js: 8 new i18n strings for remove subcommand
Critical fixes: - Fix noImplicitThis in test by converting forScope to arrow function - Restore deleted i18n key 'Already covered by existing directory' in en/zh/zh-TW - Persist settings before in-memory removal; skip removal if persist fails - Replace ?? with || for initial-directory guard (dead code fix) - Use filter instead of findIndex to remove all duplicate entries - Expand raw path in realpathSync fallback for path format mismatch Suggestion fixes: - Remove dead code getSkippedDirectories (zero production consumers)
- Make multi-scope settings update transactional: snapshot in-memory
state before the loop and rollback on partial failure so settings
stay consistent with disk.
- Treat failed in-memory removal as error even when a persisted
settings entry was found, so the user knows the directory is still
accessible in the current session.
- Add missing i18n keys: 'Error updating settings: {{error}}' and
'Directory removed from settings but could not be removed from the
active workspace...' in en/zh/zh-TW.
- Add tests for user-only removal, both-scope removal, partial-failure
rollback, and failed in-memory removal with settings updated.
Address 15 review comments from three review rounds: Critical fixes: - Replace shallow copy rollback with structuredClone for deep copy (nested context.includeDirectories was mutated in-place, making rollback ineffective) - Two-phase settings commit: compute all scope changes first, then atomically commit. Track committed scopes for partial rollback. - Guard memory refresh error path: return after error so success message is not shown when refresh fails - Replace N+1 sync fs.realpathSync with async parallel fs.promises.realpath - Remove redundant dual initial-directory guard (isInitialDirectory already handles realpath resolution) - Add sandbox check to completion function - Convert O(n×m) completion filter to Set-based O(n) lookup Test fixes: - Change mockSettings type from Record<string, unknown> to any (resolves TS4111 noPropertyAccessFromIndexSignature errors) - Mock loadServerHierarchicalMemory for memory refresh tests - Add mockContext.config updates in tests that create new mockConfig - Add setGeminiMdFileCount to mockContext.ui New tests: - Sandbox mode removal: verifies error and no settings mutation - Memory refresh success: verifies setUserMemory, setGeminiMdFileCount, setConditionalRulesRegistry called - Memory refresh error: verifies error message and no success message - Deep copy rollback: verifies committed scopes are rolled back on later-scope failure
- Fix C3: Add disk-level rollback via setValueFullSave when partial scope commit fails - Fix C5: Move success message before memory refresh; make refresh best-effort with WARNING - Fix S1: Use canonicalDirectory in removeDirectory() call - Fix S4: Add normalized path fallback for initial-directory guard - Fix S5: Expand home dir and normalize raw paths in Phase 1 filter - Fix S6: Verify actual in-memory state restoration and disk rollback in rollback test - Add i18n key 'Directory removed but memory refresh failed' to en/zh/zh-TW - Add setValueFullSave mock to test setup and rollback tests - Update memory refresh failure test: expects WARNING + success instead of ERROR - Remove duplicate rollback test and fix describe block closing
…ollback - Replace non-existent setValueFullSave with exported saveSettings function - Import saveSettings from config/settings.js - In catch block: restore in-memory state then call saveSettings() to re-write disk file for committed scopes - Mock saveSettings in tests via vi.mock - Update rollback test assertions to verify saveSettings called with correct scope snapshot - Remove setValueFullSave mock from test setup
…18n keys. Heartfelt apologies for the delays in addressing the final suggestions!
B-A-M-N
force-pushed
the
feat/directory-remove-v2
branch
from
May 20, 2026 00:07
55ead30 to
d2f2a48
Compare
- Remove .gitnexus/ artifacts from commit - Add gemini.addDirectoryContext() after removal (mirrors add subcommand) - Wrap saveSettings() rollback in try/catch to prevent silent abort on double-failure - Roll back persisted settings when removeDirectory() returns false - Fix unsafe error casts to use instanceof Error - Update i18n keys (remove dead keys, add rollback message, update sandbox text) - Add partialArg filtering to completion function
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
19 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add /directory remove subcommand with tab-completion, initial directory
guards, workspace settings persistence, and restrictive sandbox check.
Warn on startup when --add-dir paths don't exist or aren't readable.
Changes
Verification
All 18 directoryCommand tests pass. All 48 workspaceContext tests pass.
Summary by cubic
Large Mode B upgrade: introduce a shared ACP bridge and significantly expand
qwen servewith single‑workspace binding, richer session lifecycle, read‑only file APIs, workspace memory/agents CRUD, status/preflight/env diagnostics, typed events/backpressure, auth hardening, MCP guardrails, and a built‑in demo page. SDK and tests were updated to match; no breaking changes for existing callers.New Features
@qwen-code/acp-bridge(EventBus, channel/types/status, helpers) shared by serve, channels, IDE, and TUI.--workspace, advertised on/capabilities); per‑requestsessionScopeoverride./workspace/env,/workspace/preflight, and read‑only status snapshots (MCP, skills, providers, session context/commands)./file,/list,/glob,/stat.QWEN.md; subagent list/get/create/update/delete.slow_client_warningand configurable ring (--event-ring-size, SSE?maxQueued).--require-auth; OAuth device‑flow broker;/demodebug page.--mcp-client-budgetand--mcp-budget-mode(warn/enforce), with snapshot/status.DaemonSessionClient, typed daemon events/reducer, helpers for new routes (capabilities, status, load/resume, heartbeat, workspace memory/agents, file read), andrequireWorkspaceCwd.Migration
/capabilities.--workspace,--event-ring-size,--require-auth, and MCP flags (--mcp-client-budget,--mcp-budget-mode).sessionScopeperPOST /session, seed SSE withLast-Event-IDand optionalmaxQueued.Written for commit 98d009d. Summary will update on new commits.