mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-17 03:57:18 +00:00
5852 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Shaojin Wen
|
ef29700bce
|
fix(ui): trim background task results and show newest first (#4094) (#4125)
Some checks are pending
Qwen Code CI / Classify PR (push) Waiting to run
Qwen Code CI / Lint (push) Blocked by required conditions
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Blocked by required conditions
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* fix(ui): trim background task results and show newest first (#4094) Two related improvements to the background task pill and dialog: 1. Trim outdated terminal task results. `BackgroundTaskRegistry` and `BackgroundShellRegistry` now cap retained terminal entries at 32 each (mirroring `MonitorRegistry`'s existing `MAX_RETAINED_TERMINAL_MONITORS` pattern). Running, paused, and cancelled-but-not-yet-notified entries are never evicted — pruning a not-yet-notified entry would break the SDK contract that every `register` pairs with exactly one terminal `task-notification`. 2. Show newest tasks at the top of the dialog. `useBackgroundTaskView` now sorts entries by `startTime` descending so the dialog opens with the cursor on the most recently launched task. `LiveAgentPanel` reverses internally back to ASC for its own visual layout (newest row sits closest to the composer). * perf(shell-registry): batch abortAll prune + statusChange into one pass abortAll() previously delegated to cancel() per entry, so each running shell triggered its own pruneTerminalEntries() and statusChange wakeup. On shutdown / `/clear` with N running shells the only subscriber (useBackgroundTaskView) re-pulled getAll() N times for what is logically a single batch transition. Settle each entry inline via the new private settleAsCancelled() helper, then fire prune + statusChange exactly once after the loop. The split keeps the running-status guard at the public-API boundary so callers can't accidentally re-settle a terminal entry. * fix(ui): two-bucket sort so running tasks outrank fresh terminals The earlier startTime DESC sort surfaced the newest LAUNCH but let an older long-running / paused entry get pushed below a batch of newer terminal entries — the user opening the dialog to check on the running work would find it buried under stale completed rows. Split the merge into two buckets: - active (running + paused): sorted by startTime DESC so the most recent launch sits at the very top of the dialog. - terminal (completed / failed / cancelled): sorted by endTime DESC so the most recently FINISHED entry leads the terminal section (matches "what changed while I wasn't looking" intuition; a long task that just settled outranks an old quick task that finished hours ago). Pin the new behavior with two tests covering active-above-terminal and the endTime-vs-startTime distinction inside the terminal bucket. * fix: add missing outputFile and isBackgrounded to retention cap tests The merge brought in required fields on AgentTaskRegistration that the retention-cap test helpers were not supplying. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
Shaojin Wen
|
5ce2f2854b
|
fix(test): clear boundedPromise timers to prevent unhandled rejections (#4220)
boundedPromise timeouts could fire after test completion, causing vitest to exit with code 1 due to unhandled rejection. Add clear() method and cleanup all pending timers in the finally block. |
||
qqqys
|
07165a095c
|
Add stop hook blocking cap (#4208)
* feat(core): add stop hook blocking cap * fix(core): tighten stop hook cap behavior * fix(cli): show goal judge details * fix(core): bound stop hook blocking cap * fix(core): surface subagent stop cap warnings * fix(core): clean up stop hook cap loop * test(core): cover stop hook cap integrations * test(core): strengthen stop hook cap coverage |
||
jinye
|
0eed884c0b
|
fix(rewind): restore upstream TOCTOU ordering + heal sticky failed marker (#4216)
Some checks are pending
Qwen Code CI / Classify PR (push) Waiting to run
Qwen Code CI / Lint (push) Blocked by required conditions
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Blocked by required conditions
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* fix(rewind): restore upstream TOCTOU ordering + heal sticky failed marker
Two related bugs that landed in #4064:
1. **trackEdit widened the pre-write TOCTOU window**. PR #4064 inserted
`await trackEdit(filePath)` between `checkPriorRead` and
`writeTextFile` in both `edit.ts` and `write-file.ts`. trackEdit does
`stat` + `copyFile` and on large files can take hundreds of
milliseconds. The pre-existing comment above `checkPriorRead`
acknowledged a stat-then-write race window of "two adjacent
syscalls"; the new ordering widened that to "freshness check →
potentially-multi-second backup → write", so an external mutation
landing during the backup would no longer be detected before the
write clobbered it.
The fix is the ordering upstream `claude-code/src/tools/FileEditTool`
uses, with its own explicit comment on the equivalent block:
> These awaits must stay OUTSIDE the critical section below — a
> yield between the staleness check and writeTextContent lets
> concurrent edits interleave.
Moved `trackEdit` to BEFORE `checkPriorRead` in both tools. Backups
are idempotent (deterministic `{hash}@v{version}` filename), so
running the backup on a path that ends up rejected just leaves an
unused-but-correct backup of the pre-edit state, not corrupt state.
2. **The `failed` marker added in
|
||
|
jinye
|
9505246886
|
fix(serve): align integration test + user doc with merged sessionScope override (#4214)
PR #4209 (Wave 2 PR 5) shipped per-request `sessionScope` override and added a `session_scope_override` capability tag to the registry. Two follow-ups from wenshao's review landed unaddressed: 1. `integration-tests/cli/qwen-serve-routes.test.ts` still asserted the pre-PR 9-element `caps.features` list and was named "all 9 Stage 1 features". Running the suite against a real daemon would fail — the daemon now advertises 10 features, with `session_scope_override` between `session_create` and `session_list` per the registry order. PR CI didn't catch this because integration tests need a real `qwen serve` spawn and run only in the release pipeline; the unit-level `EXPECTED_STAGE1_FEATURES` constant in `server.test.ts` was updated, but its integration sibling was missed. 2. `docs/users/qwen-serve.md` "Stage 1.5+ runtime guarantees" still listed per-request `sessionScope` override as item 1 of "Blockers for serious downstream use", saying "today the daemon-wide default is the only setting." Directly contradicts the merged behavior and the protocol doc, so downstream integrators reading the user guide get inverse guidance. Fixes: - Update the integration test name to "all 10 Stage 1 features" and insert `session_scope_override` in the asserted array (matching registry order); add a comment noting the unit/integration/registry triple must stay in lockstep. - Remove the obsolete blocker bullet from the user doc and renumber the remaining items (2/3 → 1/2 in Blockers, 4-7 → 3-6 in Reliability, 8-10 → 7-9 in Integration ergonomics). No production code changes. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) |
||
易良
|
ba77ddd81b
|
fix(lsp): expose status and startup diagnostics (#3649)
* feat(lsp): add /lsp slash command to show server status Implements the /lsp command that displays the status of all configured LSP servers. Previously this was documented in the FAQ but never implemented, leaving users with no way to check if their language servers started successfully. Changes: - Add LspServerStatusInfo interface to lsp/types.ts - Add getServerStatus() to LspClient and NativeLspClient - Expose getServerHandles() from NativeLspService - Create lspCommand.ts with status table output - Register /lsp in BuiltinCommandLoader (only when LSP is enabled) The command shows: server name, command, languages, and status (NOT_STARTED / IN_PROGRESS / READY / FAILED + error message). * fix(lsp): expose status and startup diagnostics * fix(lsp): harden status command diagnostics * fix(lsp): add stderr error listener and harden initialization error handling - Add stderr 'error' event listener in LspConnectionFactory to prevent unhandled stream errors from crashing the process - Wrap setLspInitializationError calls in try-catch in config.ts to guard against post-initialization state changes that would throw |
||
ChiGao
|
57de269f45
|
feat(sdk): add DaemonSessionClient skeleton (#4201)
* feat(sdk): add daemon session client * fix(sdk): harden daemon session event replay * fix(sdk): replay attach-time daemon events --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> |
||
|
jinye
|
0788ed7fb0
|
test(perf): add daemon baseline harness (#4175 Wave 1 PR 1) (#4205)
* test(perf): add daemon baseline harness (#4175 Wave 1 PR 1) First implementation PR of the Mode B v0.16 rollout (issue #4175 Wave 1 PR 1). Captures reference performance metrics for the `qwen serve` daemon so subsequent Mode B PRs (M2 MCP shared pool, M3 architecture refactor, M4 multi-client safety) can be measured against a known baseline rather than guessed-at numbers. ## What it captures The new `integration-tests/cli/qwen-serve-baseline.test.ts` runs five describe blocks against a real `qwen serve` daemon: - RSS scaling across 1 / 5 / 10 same-workspace `createOrAttachSession` calls (sampled via `ps -o rss=`). - Same-workspace attach latency for the 2nd and 5th attach. - MCP child amplification with two configured idle-mcp servers, measured via two-level `pgrep -P` walk (daemon → ACP child → MCP grandchildren). - SSE backpressure invariants exercised at the unit layer by instantiating `EventBus` directly: queue overflow → synthetic `client_evicted` frame; replay across reconnect honors `lastEventId` up to ring size. - Prompt p50 / p99 (skipped when `QWEN_TEST_MODEL_KEY` is unset, with an explicit reason recorded in the snapshot). Each run writes a structured JSON snapshot to `<INTEGRATION_TEST_FILE_DIR>/perf-baseline.json` plus a Markdown summary, with `gitCommit` / platform / config preserved for cross-PR correlation. ## Honest documentation of current limits The captured snapshot includes a `notes` field flagging that with the default `sessionScope: 'single'`, N successive `createOrAttachSession` calls return the same sessionId — so the RSS and MCP metrics here measure "N attaches to one shared session", not "N distinct sessions". Once Wave 2 PR 5 lands per-request `sessionScope: 'thread'` override, the harness will be updated to optionally force distinct sessions and surface the P1 MCP N×M amplification before M2 fixes it. ## Reused / new Reused: existing daemon spawn pattern from `qwen-serve-routes.test.ts` (port-0 + stdout regex + SIGTERM teardown), `pgrep -P` pattern from `qwen-serve-streaming.test.ts:144`, `EventBus` invariants from `eventBus.test.ts`, `DaemonClient` SDK, integration-tests `globalSetup.ts` env var conventions. New (this PR): - `integration-tests/cli/_daemon-harness.ts` (~280 lines) — extracts the inline daemon spawn pattern into a shared helper plus adds `getRssMB`, `startRssPolling`, `countDescendants`, `percentiles`, `consumeSseEvents`, `writeWorkspaceSettings`. Future serve test files can import instead of inlining. - `integration-tests/fixtures/idle-mcp/{server.mjs,package.json}` — a minimal stdio MCP fixture that responds to `initialize` / `tools/list` and idles. Lets the harness count real MCP children via `pgrep` without depending on a network npm package in CI. - `integration-tests/baselines/baseline-stage-1.json` — the first captured baseline at this commit. Future Mode B PRs can diff their run against this file; updating it is a deliberate one-line change in a follow-up PR. ## Reference patterns from opencode JSDoc on the main test file documents the shape borrowed from `opencode/test/memory/abort-leak.test.ts` (forced-GC heap-growth), `opencode/src/cli/heap.ts` (RSS poll + threshold-triggered `writeHeapSnapshot`, useful for Wave 6 production tooling), and `opencode/src/util/cpu-watchdog.ts` (event-loop lag drift sampling). The harness here is daemon-level multi-session — a shape neither opencode nor qwen-code had before. ## Engineering principles checklist - [x] Independently mergeable (test-only; no production code touched) - [x] Backward compatible (no removed routes / event fields / CLI behavior) - [x] Default off (PR CI does not run integration tests; baseline runs in release CI / nightly / manual) - [x] `qwen serve` Stage 1 routes / SDK behavior preserved (no production code changed) - [x] Gradual migration (no client adapter migration in this PR) - [x] Reversible (revert = delete files, no other side effects) - [x] Tests-first (this IS the test PR; harness exercises real daemon end-to-end; Windows skipped via existing `process.platform === 'win32'` precedent) ## Test plan - [x] `KEEP_OUTPUT=true TEST_CLI_PATH=$(pwd)/packages/cli/dist/index.js QWEN_BASELINE_SKIP_PROMPT_LATENCY=1 QWEN_BASELINE_RSS_SAMPLE_DURATION_MS=2000 npx vitest run integration-tests/cli/qwen-serve-baseline.test.ts` — 6 passed / 1 skipped (prompt latency requires model key) - [x] `npx tsc --noEmit -p integration-tests/tsconfig.json` — only pre-existing tsconfig `paths` glob warning remains, no new errors 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix: import exit from node:process in idle-mcp fixture Fixes eslint no-undef error: 'process' is not defined. Replace process.exit(0) with exit(0) from node:process import. * fix(test): remove stale baseline lint disable Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(test): harden daemon baseline harness Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
jinye
|
54fd5c50f0
|
feat(telemetry): add detailed sensitive span attributes (#4097)
Layer detailed content attributes onto the existing hierarchical spans (qwen-code.interaction / qwen-code.llm_request / qwen-code.tool) gated by includeSensitiveSpanAttributes: - Interaction span: user prompt (new_context) - LLM request span: system prompt + hash + preview + length (full text deduped per session via SHA-256), tool schemas (per-tool tool_schema events, also hash-deduped), model output - Tool span: tool input, tool result on every exit path (success + pre-hook block + post-hook stop + tool error + try-block cancel + catch-block cancel + execution exception) All large content truncated at 60KB with *_truncated and *_original_length metadata. Heavy serialization (safeJsonStringify on tool I/O, partToString on user prompt) is guarded by the sensitive flag at the call site so it doesn't run when telemetry is off. Also adds: - getActiveInteractionSpan() helper for client.ts to attach prompt attributes to the interaction span. - Updated config schema description and docs (telemetry.md + settings.md) to reflect expanded scope and add security/cost notes. - 28 unit tests for detailed-span-attributes, 4 tests for getActiveInteractionSpan, integration mocks updated. |
||
|
qqqys
|
daaa85e98e
|
feat(cli): add fork-session resume flag (#4159)
* feat(cli): add fork-session resume flag * fix(cli): address fork-session review feedback * fix(cli): handle fork session copy failures * fix(cli): guard sandbox session handoff flag |
||
|
Shaojin Wen
|
b9590283c0
|
fix(cli): pass rewind selector test props (#4211)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
jinye
|
878f35fc4f
|
feat(serve): per-request sessionScope override on POST /session (#4175 Wave 2 PR 5) (#4209)
* feat(serve): per-request sessionScope override on POST /session Resolves the FIXME at httpAcpBridge.ts:BridgeOptions.sessionScope from #3803 — clients can now override the daemon-wide sessionScope per request instead of being stuck with whatever boot-time value the operator picked. A VSCode window that wants strict isolation can ask for `'thread'` against a default-`'single'` daemon, and vice versa. Wire change: - POST /session body accepts optional `sessionScope: 'single' | 'thread'` - Per-request value wins; daemon-wide default remains the fallback when the field is omitted (bit-for-bit backward compat for every existing caller) - Invalid values yield 400 `{ code: 'invalid_session_scope' }` - New capability tag `session_scope_override` advertised on /capabilities.features for negotiation Bridge changes: - BridgeSpawnRequest gains optional `sessionScope` - spawnOrAttach validates the per-request value and resolves effectiveScope = req.sessionScope ?? defaultSessionScope - doSpawn now takes effectiveScope and only stamps `defaultEntry` (the single-scope attach slot) when the spawn is single-scope — fixes a mixed-scope leak where a thread-first call would let a later omitted-scope call attach to the supposedly-isolated session SDK: - CreateSessionRequest gains optional `sessionScope` - DaemonClient.createOrAttachSession conditionally spreads it into the JSON body so omitted callers send the same wire shape as before Tests: - 4 new bridge tests (override single→thread, override thread→single, mixed-scope leak regression, invalid-value rejection) - 3 new server tests (valid passthrough, invalid 400, omitted backward compat) - 2 new SDK tests (forwards/omits sessionScope on the wire) - EXPECTED_STAGE1_FEATURES updated for the new capability tag 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(serve): address Wave 2 PR 5 review findings Three independent review passes found three real issues: 1. Bridge `TypeError` on invalid `sessionScope` collapsed to opaque 500 in `sendBridgeError` instead of the typed `400 invalid_session_scope` the route layer guarantees. Direct embed / test / future entry-point callers bypassing the route would see a generic 500 with stack noise on stderr — disagreeing with the route contract. Fix: add `InvalidSessionScopeError` class (alongside `SessionNotFoundError` / `WorkspaceMismatchError` / `SessionLimitExceededError`); the `spawnOrAttach` validator now throws it, and `sendBridgeError` translates to the same `{ error, code: 'invalid_session_scope' }` shape. 2. SDK `DaemonClient.createOrAttachSession` used a truthy check (`req.sessionScope ? ...`) for the conditional spread, silently erasing falsy-but-defined values (`''`, `null`, `0`) on the wire. A buggy caller would never see the daemon's 400 — it'd inherit the daemon-wide default while believing it requested a specific scope. Fix: use `!== undefined` (matching the bridge's own validation shape). Same fix to the server-side spread for consistency. 3. JSDoc and docs referenced `serve --sessionScope` as if it were a shipping CLI flag. It isn't — `ServeOptions` has no field, neither `runQwenServe` nor `serve.ts` plumbs one, and the production daemon default is hardcoded to `'single'`. Strike the references; note that #4175 may add the flag in a follow-up. Test coverage expanded: - Cap-bypass guard: per-request `'thread'` overrides cannot bypass `maxSessions` on a daemon-default-`'single'` deployment. Without this, a future refactor that gated the cap on `defaultSessionScope` instead of `effectiveScope` would silently let `'thread'` overrides amplify past the limit — the exact N-amplification cliff #3803 was about. - Symmetric mixed-scope leak: daemon-default-`'thread'` + single-first-call followed by omitted-scope-second-call must produce distinct sessions. Mirrors the existing daemon-default-`'single'` + thread-first leak regression. - Concurrent mixed-scope coalescing: simultaneous single + thread `spawnOrAttach` against the same workspace under slow `initialize` must not collide on `inFlightSpawns` (tracker keys differ by scope). - Updated invalid-scope rejection test to assert `InvalidSessionScopeError` instance + carried `sessionScope` field. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) |
||
|
易良
|
2c200a3d0a
|
fix(core): add heap-pressure auto-compaction safety net (#4186)
* fix(core): add heap pressure compaction safety net Link: #4185 * fix(core): keep heap pressure compaction active Let heap-pressure bypass also bypass the failed-attempt latch and cover the interaction with a regression test. Link: #4185 * test(core): cover raw next-speaker history lookup Verify next-speaker checks use the raw last history entry for function responses while the LLM prompt still uses curated history. Link: #4185 * fix(core): avoid latching heap-pressure compression failures * fix(core): back off failed heap-pressure compaction * fix(core): back off heap-pressure no-op compaction |
||
Dragon
|
8f54ae9c0f
|
feat(cli): add built-in status line presets with interactive dialog (#4120)
* chore(skills): add codex reproduce workflows
* feat(cli): add built-in status line presets with interactive dialog
Replace the shell-command-only status line with a preset system that
renders structured session info (model, context usage, git branch,
token counts, etc.) without external commands. Users can configure
which items to display via a new interactive dialog accessible through
/statusline or the settings UI.
- Add statusLinePresets module with 16 built-in item types
- Add StatusLineDialog component with search, multi-select, and preview
- Update /statusline command to open the preset dialog
- Extend settings schema to support { type: "preset", items: [...] }
- Enhance MultiSelect with separator items, active marker, and
customizable checked text
- Update Footer to support theme-colored preset output
* fix(cli): refresh status line preset after saving
* chore: remove codex reproduce skills
* fix(cli): address status line preset review feedback
|
||
dreamWB
|
966b040359
|
feat(cli): readline Ctrl+P/N for history and selection navigation (#4082)
* feat(cli): readline Ctrl+P/N for history and selection navigation
Adds GNU-readline-style Ctrl+P (previous) and Ctrl+N (next) shortcuts
to the qwen-code TUI so users coming from bash/zsh, Emacs, or Claude
Code feel at home. The change has three orthogonal behavior groups:
1. Input prompt, history-versus-line-motion two-step edge
Ctrl+P / Ctrl+N and the arrow keys behave identically and apply a
two-step edge transition that matches GNU readline and Claude Code:
inside a multi-line buffer they move the cursor between visual
rows; on the top row with the cursor away from column 0 the first
Up press snaps the cursor to column 0 without changing history, and
only the second press walks one entry back. The mirror rule holds
for Down at the last row (snap to end of line, then advance). After
navigateUp the buffer is parked at offset 0 (the "start of older
entry" landing position); after navigateDown setText's default
end-of-text positioning keeps the cursor at the end. The same
two-step rule applies to single-line buffers so the
reverse-direction case the issue called out works: pressing Ctrl+N
immediately after Ctrl+P loaded a single-line older entry (cursor
at col 0) first snaps the cursor to end-of-line, and only the next
Ctrl+N moves forward through the history. Bare k/j inside the
input prompt remain ordinary typed letters — the vim aliases are
selection-list shortcuts, not text-editing ones.
2. Selection lists: arrows, k/j, and Ctrl+P/N are interchangeable
A new pair of Command bindings, SELECTION_UP and SELECTION_DOWN, is
wired into the shared useSelectionList hook and every dialog that
used to hand-roll an "up/down arrow only" or "up/k arrow + vim
only" navigation check. Covered surfaces: the main selection-list
hook itself, the MCP / extensions / agents / hooks / background-
tasks / rewind / plugin-choice / ask-user-question dialogs, the
memory dialog (both its file list and the auto-memory and
auto-cleanup toggle panel above the list), the settings dialog
list (with the in-place value editor's "block other keys while
editing" guard preserved), and the manage-models dialog's top
tabs row. The auth-provider wizard's Advanced Config focus rows
and the resume-session picker's cross-mode arrows are extended
with the readline Ctrl+P / Ctrl+N synonyms while keeping their
existing arrow-key and (for the session picker) vim k/j semantics
intact.
3. Selection surfaces that wrap an active text input
AskUserQuestionDialog's "Other / type a custom answer" field,
manage-models' search input, the resume-session picker's search
field, and the auth-wizard's Context-window number input all
coexist with the selection list on the same screen. In those
surfaces typing k or j has to land in the text buffer, not scroll
the surrounding list. The fix is to scope the input-aware handler
to unambiguous non-letter shortcuts only — arrow keys plus
readline-style Ctrl+P / Ctrl+N escape the text field, while bare
letters (including k / j / p / n) are delivered to the active
input. The keyBinding-level fix that backs this is the
`{ key: 'k', ctrl: false }` / `{ key: 'j', ctrl: false }` clauses
on SELECTION_UP / SELECTION_DOWN, which prevent Ctrl+K from
accidentally matching SELECTION_UP and thereby firing both the
list-up handler and the KILL_LINE_RIGHT handler in the same
keystroke (the P0 finding the quality-gate review surfaced).
Focus-traversal tokens (the agent tab bar and the background-task
pill) and chord shortcuts (Ctrl+Shift+Up/Down for embedded-shell
history) are deliberately left untouched because their existing
"any printable letter yields focus back to the composer" UX would
break under the new vim-style letter bindings, and the Help
viewer's scroll is a viewer rather than a selection list and is
out of this PR's scope.
Documentation: docs/users/reference/keyboard-shortcuts.md is updated
so the Ctrl+P / Ctrl+N entries describe the two-step edge rule and
the radio-button-select table mentions the new k/j and Ctrl+P/N
aliases. Per-dialog on-screen hints (which still read "↑↓ to
navigate") are intentionally not touched so the i18n string surface
stays unchanged; the global reference doc is the authoritative source
for the new shortcuts.
Tests:
- packages/cli/src/ui/keyMatchers.test.ts adds positive cases
covering ↑ / ↓ / bare k / bare j / Ctrl+P / Ctrl+N matching
SELECTION_UP / SELECTION_DOWN and negative cases asserting that
Ctrl+K and Ctrl+J do NOT match (the conflict guard).
- packages/cli/src/ui/components/InputPrompt.test.tsx adds a
"two-step edge transition for history navigation" describe block
with four cases: a mid-line Ctrl+P snaps to col 0 without invoking
navigateUp; an at-col-0 Ctrl+P does invoke navigateUp and then
parks the cursor via moveToOffset(0); a not-at-end Ctrl+N snaps to
end-of-line without invoking navigateDown; and arrow Up obeys the
same rule as Ctrl+P for keyboard-parity. The test file's mock
buffer's setText was also corrected to mirror the real buffer's
"cursor lands at the end of the new text" semantic so the cursor
field is internally consistent during keypress assertions; the
small InputPrompt render-frame snapshot in the same file's
__snapshots__/ directory was regenerated to reflect the now-
accurate cursor render position. Three pre-existing arrow-key
navigation tests were updated to pre-position the mock cursor at
the relevant edge before pressing the arrow, because the new
two-step rule means the first arrow press at a non-edge position
is a cursor snap, not a history step. Multi-line cursor-between-
rows movement is covered indirectly by the keyBinding-level
matcher tests plus the end-to-end manual demo plan.
The work landed in three rounds against the planner's gate: round 1
added the unified SELECTION_UP / SELECTION_DOWN Command binding and
the cursor-first dispatch in the input prompt; round 2 picked up the
quality-gate review's P0 (the Ctrl+K double-fire in the "Other"
custom-input field) and the user's hand-test feedback on the missing
two-step edge in the reverse direction plus the MemoryDialog
top-panel sections that weren't wired through SELECTION_*; round 3
swept the remaining adjacent dialogs (SettingsDialog list,
ManageModelsDialog tabs and search transitions, ProviderSetupSteps
advancedConfig, useSessionPicker's cross-mode arrows) so the
keyboard model is uniform across the TUI.
The original issue also asks for Meta+B / Meta+F word motion and
smarter Ctrl+H token-aware backspace among other readline
conveniences. The user explicitly scoped this PR down to Ctrl+P /
Ctrl+N at the planner approval gate; the remaining wish-list items
are deferred to follow-up issues.
Closes #3821
* docs(cli): refine Ctrl+P/N input-history rows; fix Ctrl+J in selection-list comment
Both items came from a non-blocking COMMENTED review on PR #4082
(https://github.com/QwenLM/qwen-code/pull/4082#pullrequestreview-4271527787),
flagging two polish points in the readline Ctrl+P/Ctrl+N feature the parent
commit `feat(cli): readline Ctrl+P/N for history and selection navigation`
(
|
||
tanzhenxin
|
8d765fec78
|
refactor(core): TaskBase envelope + foreground subagent persistence (#3970)
* refactor(core): TaskBase envelope + foreground subagent persistence
Establishes a shared `TaskBase` envelope across the agent / shell /
monitor task registries with a mandatory `outputFile` field. Brings the
foreground subagent path into compliance with the new contract, so it
now leaves the same JSONL transcript + meta sidecar on disk that
backgrounded subagents have always produced — closing the only gap
where a registered task wrote nothing. Renames the agent-task
discriminator from `flavor: 'foreground' | 'background'` to claw-code's
`isBackgrounded: boolean`; the deprecated names are kept as
one-release type aliases.
PR 1 of the task-registry-unification design. PR 2 will collapse the
three per-kind registries into one thin TaskRegistry plus per-kind
modules.
* refactor(core): drop unused BackgroundTaskFlavor type alias
The alias only preserved the type name; no in-tree caller used it,
and after the field rename no realistic external consumer use survives
(reading entry.flavor / writing { flavor: ... } both fail at the use
site regardless of whether the alias resolves). Drop it instead of
carrying a hollow shim.
* fix(core): tighten foreground subagent launch path
- Register before writing the meta sidecar so a register() failure can't
leave an orphaned 'running' meta file behind. writeAgentMeta is
best-effort and never throws, so the inverse failure mode (registry
entry without sidecar) is a benign degradation.
- Cache getGitBranch by cwd at the agent module level so foreground
launches don't pay a fresh git rev-parse exec each time. Branches
don't change within a process under normal use; the transcript
annotation is best-effort audit metadata.
- Document on cancel() that foreground entries take a partial path
through the method — Map deletion is the caller's responsibility
via unregisterForeground() in the tool-call's finally path.
* fix(agent): correct foreground meta status mapping and register order
The foreground finally block in agent.ts mapped any non-ERROR, non-CANCELLED
terminate mode (including MAX_TURNS, TIMEOUT, SHUTDOWN) to 'completed' in
the sidecar, so post-mortem readers and resume logic saw a successful
status for runs that actually hit a guardrail. Flip the ternary to mirror
the background path: GOAL -> completed, CANCELLED -> cancelled, else ->
failed.
Also reorder the background launch so registry.register() runs before
writeAgentMeta(), matching the foreground path. Both paths now share the
same orphaned-meta guarantee.
* test(agent): rename stale foreground-flavor test
The "default flavor (absent) behaves as background" test name and its
backwards-compat comment referenced the old optional flavor field, but
the registration shape has required isBackgrounded for a while now —
there is no "absent" path to exercise. Rename it to describe what the
assertion actually covers: that background entries fire a task-
notification on complete.
* refactor(core): alias BackgroundTaskStatus to TaskStatus
The local `BackgroundTaskStatus` union was byte-identical to the new
shared `TaskStatus` defined in `tasks/types.ts`. Replace it with a
`@deprecated` type alias so external consumers (notably
`nonInteractiveCli.ts`) keep compiling unchanged while the canonical
name lives in one place.
* refactor(core): tidy monitorRegistry signatures and document cancel ordering
Two small consistency wins flagged in review:
1. `dispatchOwnerLifecycleWake` and `dispatchNotification` were the only
methods on the registry still typed with the deprecated `MonitorEntry`
alias. Rename their parameters to `MonitorTask` to match every other
signature in the file.
2. `cancel()` orders `settle()` and `abort()` differently between its two
branches, which is intentional (silent cancel locks the terminal status
before abort listeners run; default cancel lets a naturally-completing
operation settle through its own terminal path). Document that
asymmetry in a JSDoc on the method so the next reader doesn't have to
reverse-engineer it.
* refactor(core): migrate internal BackgroundTaskStatus refs to TaskStatus
The `BackgroundTaskStatus` alias was introduced in
|
||
|
jinye
|
8dfbdaa5d4
|
feat(telemetry): unify span creation paths for hierarchical trace tree (#4126)
* feat(telemetry): unify span creation paths for hierarchical trace tree (#3731 P3 Phase 1) Replace disconnected withSpan/startSpanWithContext calls in runtime with session-tracing typed helpers so LLM and tool spans become children of the interaction span instead of siblings under the session root. - Add toolContext ALS with runInToolSpanContext() for concurrent-safe tool span scoping (uses AsyncLocalStorage.run, not enterWith) - Wire startLLMRequestSpan/endLLMRequestSpan in loggingContentGenerator for both streaming and non-streaming paths - Wire startToolSpan/endToolSpan + startToolExecutionSpan/endToolExecutionSpan in coreToolScheduler with proper try/finally lifecycle - Remove redundant withSpan('client.generateContent') wrapper from client.ts - Fix endToolSpan to not override pre-set status when metadata is omitted - Change startToolExecutionSpan to read parent from toolContext ALS - Update tests for new span creation APIs and remove dead test infrastructure * fix(telemetry): address CI build errors in session-tracing tests - Remove unused _toolSpan variable (TS6133) - Use bracket notation for index signature property access (TS4111) * fix(telemetry): update coreToolScheduler and loggingContentGenerator test mocks - coreToolScheduler.test.ts: mock startToolSpan/endToolSpan/runInToolSpanContext instead of withSpan; update cancellation tests for restored safeSetStatus call - loggingContentGenerator.test.ts: fix attribute keys in mock, add try/catch in endLLMRequestSpan mock to match production best-effort behavior * fix(telemetry): address review feedback from wenshao - Add debugLogger.warn in catch blocks of endLLMRequestSpan/endToolSpan/ endToolExecutionSpan instead of silent swallowing - Add JSDoc on endToolSpan documenting intentional no-metadata-no-status contract with setToolSpanFailure/setToolSpanCancelled - Add warning in startToolExecutionSpan when called outside runInToolSpanContext (no active toolContext) - Sanitize error message in endToolExecutionSpan: use constant TOOL_SPAN_STATUS_TOOL_EXCEPTION instead of raw error message * fix(telemetry): use partial mock for telemetry/index.js in coreToolScheduler tests The full mock shadowed all re-exports (logToolCall, etc.) causing 49 test failures. Use importActual to preserve other exports, only override span functions. * fix(telemetry): getLastToolSpan must skip tool.execution sub-spans startToolExecutionSpan mock also pushes to toolSpanRecords, so at(-1) returns the execution sub-span instead of the tool span. Use findLast to filter by name. * fix(telemetry): address second round review feedback - Remove redundant safeSetStatus(span, OK) on success path — endToolSpan in finally already sets OK via metadata - Add llm_request.stream attribute (true/false) to distinguish streaming vs non-streaming LLM requests in trace backends * fix(telemetry): endToolSpan mock writes to record directly Bypass span.setStatus() in mock to avoid potential interference from vitest module resolution. Write to statusCalls/ended directly on the ToolSpanRecord. * fix(telemetry): mock session-tracing.js directly instead of telemetry/index.js Mocking the barrel re-export (telemetry/index.js) with importActual was unreliable — vitest's module resolution could bind production code to the real endToolSpan before the mock override took effect. Mock the source module (session-tracing.js) directly to guarantee interception. * fix(telemetry): fix endToolSpan status on success — toolCalls is empty in finally Root cause: checkAndNotifyCompletion clears this.toolCalls before the finally block in executeSingleToolCall runs, so the tc lookup always returns undefined. Fix: set OK status explicitly in _executeToolCallBody's success path via safeSetStatus(span, OK), and call endToolSpan() without metadata in finally (just ends the span, preserves pre-set status from any path). * fix(telemetry): address Codex review — activate OTel context, end span on failure - Wrap non-stream generateContent API call + logging in context.with(spanContext) so nested OTel spans (HTTP instrumentation, log-bridge spans) parent to qwen-code.llm_request instead of session root (matches streaming path). - runInToolSpanContext now also activates OTel context via otelContext.with, not just the custom toolContext ALS. Hooks/HTTP/IO during tool execution now correctly parent to qwen-code.tool span. - Split end*Span helpers: span.end() runs in its own try/catch so a throwing setAttributes/setStatus can't leak unended spans. * fix(telemetry): address Codex review v2 — session-root fallback + execution span timing - start{LLMRequest,Tool,ToolExecution}Span now fall back to getSessionContext() when no parent context, instead of otelContext.active(). Side-query LLM calls (auto-title, recap) now stay in the session trace instead of starting a new detached trace. - Move startToolExecutionSpan() to BEFORE invocation.execute(), matching claude-code. Previously the synchronous setup inside execute (shell command preprocessing, child_process.spawn) ran outside the execution span. * fix(telemetry): address Codex review v3 — sync throw, idle timeout race, test coverage - coreToolScheduler.executeSingleToolCall: move try-block to wrap invocation.execute() so synchronous throws (e.g. shell setup failure) flow into the same catch path as async rejections. Previously a sync throw would leak the execution span and skip failure hooks. - loggingStreamWrapper: track spanEndedByTimeout flag so a stream that resumes after the 5-min idle timeout does not run the final endLLMRequestSpan (which would no-op anyway, but the flag also stops resetSpanTimeout from queuing further timer callbacks). - coreToolScheduler.test: add execution sub-span assertions for success, ToolResult.error, thrown invocation exceptions, and pre-hook denial. - loggingContentGenerator.test: capture setAttribute calls into the mock span attributes record; assert llm_request.stream is false for non-stream and true for stream paths. * fix(telemetry): address Codex review v4 — consistency + test coverage gaps - endLLMRequestSpan now uses spanCtx.span for mutations (matches endToolSpan/endToolExecutionSpan pattern). Same object, but consistent lookup pattern prevents future drift. - Mocks capture endLLMRequestSpan and endToolSpan/endToolExecutionSpan metadata so tests can assert token counts, durationMs, success, error are forwarded correctly. Add assertions on: * Non-stream LLM: inputTokens, outputTokens, success on response path * Non-stream LLM: success: false + sanitized error on rejection * Stream LLM: final lastUsageMetadata reaches endLLMRequestSpan * Tool execution sub-span: success: true on happy path * Tool execution sub-span: success: false on ToolResult.error * Tool execution sub-span: success: false + sanitized error on throw - Add OTel error resilience tests: when setAttributes or setStatus throws, span.end() must still run and the span must be removed from activeSpans. Covered for endLLMRequestSpan, endToolSpan, endToolExecutionSpan. * fix(telemetry): address Codex review v5 — abort distinction + API symmetry - session-tracing.ts SpanContext.type: comment 'tool.blocked_on_user' | 'hook' as Phase 2 forward-declarations (no helpers wired yet). - endToolExecutionSpan: align no-metadata-no-status behavior with endToolSpan. Currently no caller omits metadata, but the asymmetric default (OK vs preserve-pre-set) was a maintenance trap. - loggingContentGenerator generateContent (non-stream) catch block: call endLLMRequestSpan BEFORE the logging block, mirroring the streaming path. Defense-in-depth against logging-side rejections. - loggingContentGenerator: restore abort-specific span status message. All three LLM error paths (non-stream catch, stream eager-error catch, stream loggingStreamWrapper finally) now use API_CALL_ABORTED_SPAN_STATUS_MESSAGE when req.config.abortSignal.aborted, matching the original withSpan('client.generateContent') behavior. Trace backends can now distinguish cancellations from real failures. - coreToolScheduler _executeToolCallBody catch: distinguish abort vs exception in execSpan error message. New constant TOOL_SPAN_STATUS_TOOL_CANCELLED prevents operators filtering exec spans for errors from seeing cancellation false positives. - New test asserting exec span uses cancelled-by-user message when the invocation throws after abort. * fix(telemetry): always write 'success' attribute on tool spans E2E review found qwen-code.tool spans never carry the `success` boolean attribute (the helper only writes it when metadata is passed, and the finally block calls endToolSpan(toolSpan) without metadata). This breaks the most common observability query — filtering tool failures with `success = false` — because tool spans don't have that field at all. Fix: setToolSpanFailure / setToolSpanCancelled now also call span.setAttribute('success', false); the success path in _executeToolCallBody adds span.setAttribute('success', true) after safeSetStatus(span, OK). Mirrors the unconditional `success` attribute on llm_request spans, so backends can use one query for both span types. Add 4 scheduler-level tests asserting the success attribute on: - success path - ToolResult.error path - thrown invocation path - cancellation path |
||
|
jinye
|
784182dfe3
|
feat(skills): add /stuck diagnostic skill for frozen sessions (#4133)
* feat(skills): add /stuck diagnostic skill for frozen sessions Port the /stuck diagnostic capability to qwen-code as a bundled skill. Scans for stuck processes, high CPU/memory, hung subprocesses, and debug logs, then presents a structured diagnostic report. Adapted from claude-code's internal /stuck skill with: - Process identification via command path (node-based CLI, not compiled binary) - Debug log path updated to ~/.qwen/debug/ - Cross-platform stack dump support (macOS sample + Linux /proc/stack) - Direct user-facing output (no Slack dependency) 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(skills): respect QWEN_RUNTIME_DIR/QWEN_HOME for debug log path in /stuck 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(skills): add allowedTools and clarify diagnostic-only boundary in /stuck - Add allowedTools (run_shell_command, read_file) for convention consistency - Rephrase recommended actions as user-facing options, not model-executable commands 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(skills): address review feedback for /stuck — security, accuracy, sidecar - Add explicit PID argument validation (reject shell metacharacters) to prevent the model from substituting injection payloads into shell commands - Mention macOS/BSD `U` state alongside Linux `D` for uninterruptible sleep, so I/O-blocked macOS sessions are not silently missed - Add `-ww` to `ps` to disable column truncation, so long qwen paths don't fall outside the grep window and cause sessions to be missed - Use `~/.qwen/projects/*/chats/*.runtime.json` sidecars as the primary source of (pid, sessionId, workDir) mappings; `ps` is now a supplement for CPU/RSS/state enrichment 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(skills): apply minimal review fixes for /stuck - Filter ps to current UID via -u "$(id -u)" — avoid leaking other users' Qwen processes on shared hosts - Note that ps `rss=` is in KB; divide by 1024 before MB comparison - Replace `pgrep -lP` with `pgrep -P` + `ps -p` so child state shows up - Mention `advanced.runtimeOutputDir` setting alongside QWEN_RUNTIME_DIR / QWEN_HOME in the runtime-base description - Add half-line about PID reuse handling and not quoting secrets from debug logs (without inflating the prompt into a full workflow) 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(skills): round-3 review fixes for /stuck - Resolve RUNTIME_DIR from QWEN_RUNTIME_DIR/QWEN_HOME and use it in the sidecar `ls`, debug log path, and `latest` symlink — the previous round only updated the prose and left the actual commands hardcoded - Add explicit fallthrough: when sidecar enumeration finds nothing, fall through to step 2 instead of getting stuck trying to make sidecar work - Replace metacharacter blacklist with digit-only PID whitelist — safer and shorter; "etc." in a blacklist outsourced completeness to the LLM - Drop `strace -p <pid> -c -f` from the Linux stack-dump branch: `-c` blocks until the target exits, hanging the diagnostic on the very conditions it should diagnose; `ptrace_scope=1` would also misreport permission errors as process symptoms. Keep `cat /proc/<pid>/stack` - Warn that `ps -ww` command lines may include CLI-arg credentials and that `sample` stack frames may include in-memory secrets — redact before quoting in the report - Cover the "no sessions found at all" case so a fresh machine doesn't get reported as "all healthy" when zero data was collected 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(skills): /stuck overview-vs-step3 consistency and self-explanatory state triage - Update "What to look for" overview from `pgrep -lP <pid>` to `pgrep -P <pid>` to match step 3 (overview was left behind in the previous round when step 3 was upgraded to capture child state) - Add a triage sentence to step 3: when the state alone explains the problem (`T` = stopped, `Z` = zombie), skip child/log/stack inspection and go straight to the report 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(skills): correct /stuck runtime base priority order and resolution The actual priority in `Storage.getRuntimeBaseDir()` is `QWEN_RUNTIME_DIR` > `advanced.runtimeOutputDir` setting > `QWEN_HOME` > `~/.qwen`. The previous round merged the `advanced.runtimeOutputDir` mention but listed it after `QWEN_HOME`, and the shell snippet skipped the settings layer entirely — so on a machine where only the setting was configured, the skill would silently look in `~/.qwen` and miss all sessions. - Reorder the prose priority list to match the source - Add a `jq`-based read of `~/.qwen/settings.json` between the env-var and `QWEN_HOME`/default fallbacks. Gracefully degrades if `jq` is absent or the setting is unset. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * feat(skills): improve /stuck diagnostic flow Functional upgrades found in self-review (no reviewer raised these): - Add network-hang detection bullet to step 3. Hung HTTPS requests to the model API are the most common qwen-code "stuck" mode and showed as healthy under all previous heuristics (low CPU + S state). macOS uses `lsof -i -p`, Linux uses `ss -tnp`. - Add a fast path at the top of "Investigation steps": when the user passes a digit-only PID, skip enumeration and go straight to per-PID ps + step 3. Avoids a full sidecar+ps scan in the targeted case. - Replace per-file sidecar liveness check with a single bash loop that emits only live (pid, sidecar-path) pairs. On machines with many stale sidecars this drops 50+ separate reads. - Promote `~/.qwen/debug/latest` to the primary debug-log entry point (it usually points to the suspicious session). Sidecar-derived path becomes the fallback. - Bound the debug-log read with `tail -n 200` so the model doesn't attempt to load multi-GB log files. - Replace the placeholder `<child_pids>` for `ps -p` with a runnable `pgrep -P <pid> | xargs -I{} ps -p {} -o ...` composition. - Drop the redundant "substitute <pid> only after validation" note in step 3 — the digit-only whitelist in Argument validation already enforces this; PIDs from ps/sidecar are inherently digit-only. End-to-end tmux smoke test confirms the flow runs to completion with a correct structured report. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(skills): /stuck — RUNTIME_DIR preamble + jq-free sidecar liveness Two issues caught by Codex review: 1. **PID fast path left $RUNTIME_DIR unset.** Step 3 references `"$RUNTIME_DIR"/debug/<session-id>.txt` but the fast path skipped step 1 where it was resolved, so debug-log lookup degraded to `/debug/latest` (broken absolute path). Fix: extract RUNTIME_DIR resolution into a preamble that runs before both paths. Also add a `grep -l "pid": <PID>` lookup in the fast path so it can match the given PID to its sidecar and recover the session ID for log lookup. 2. **Sidecar liveness loop required `jq`.** Default macOS / minimal Linux images don't ship `jq`, so the loop emitted nothing for every sidecar — the "preferred reliable" path silently failed and the skill fell back to the less accurate `ps | grep`. Replace with a single-spawn `node -e` script: node is guaranteed present (qwen-code itself runs on it). The settings.json `jq` lookup stays — that one gracefully degrades to QWEN_HOME/default if `jq` is missing. Both verified by hand: liveness loop correctly emits live PID/sidecar pairs (56219, 33534), `grep -l` lookup correctly finds the sidecar for a given PID and emits empty for non-matches. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(skills): /stuck — validate fast-path PID is a Qwen Code process Codex review caught that the targeted PID fast path accepted any digit-only PID and dumped its full command line, bypassing the Qwen- process filter that the general scan applies via `grep -E '(qwen|node.*qwen|bun.*qwen)'`. Cross-user PIDs are already filtered (`kill -0` returns EPERM), but **same-user non-Qwen processes** would have their argv (potentially including secret CLI flags) printed into the chat. Fix: add a single-line validation pipeline before the stats dump: `kill -0 <pid> && ps -p <pid> -o command= -ww | grep -qE '(qwen|node.*qwen|bun.*qwen)'`. If it returns non-zero, refuse with "PID is not a current-user Qwen Code session" and stop the diagnostic. Otherwise proceed. Verified by manual test against a real Qwen Code session PID (matches) and PID 1 / launchd (correctly rejected). 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(skills): /stuck — settings path, sidecar grep, ps regex, lsof safety Four issues from PR review: 1. **Settings path honors QWEN_HOME.** The `jq` lookup in the preamble hardcoded `~/.qwen/settings.json`, but `getGlobalSettingsPath()` resolves to `$QWEN_HOME/settings.json` when set. Now uses `"${QWEN_HOME:-$HOME/.qwen}/settings.json"`. 2. **Sidecar grep uses `-El`.** Without `-E`, BSD `grep` on macOS may not treat `\b` as a word boundary in BRE. Also added a note: when PID reuse makes multiple sidecars match, prefer the most recently modified file via `ls -t | head -n 1`. 3. **Process regex tightened to avoid false positives.** The old `(qwen|node.*qwen|bun.*qwen)` matched any path containing "qwen" anywhere — so `qwen-playground/server.js`, `qwen-polyfill.js`, and even unrelated processes that pass a qwen-code path as `--cwd` (e.g., Codex plugin brokers) all matched. Replaced with `(qwen-code/[^ ]*\.(js|ts|mjs|cjs)( |$)|/qwen( |$))` — requires the `qwen-code/` substring to be followed by a script-file path, OR the bin invocation to end in `/qwen`. Verified on the local machine that broker processes are no longer matched while real Qwen sessions (worktree dev, dist/cli.js, qwen serve daemons) all are. 4. **lsof safety.** Added `-nP` to skip reverse-DNS and port lookups which can themselves hang. Mentioned `timeout 10` / `gtimeout 10` as an optional prefix when available — qwen-code's shell tool already has a backstop timeout, so this is belt-and-suspenders. Note: tested `\b` in BSD ERE on macOS — it does work correctly with `-E`, so the `-El` switch alone fully addresses concern #2's portability claim (BRE-without-E remains broken but is no longer used). 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(skills): /stuck — expand ~ and resolve relative paths in RUNTIME_DIR `Storage.resolvePath()` in qwen-code expands `~` and resolves relative paths before using `advanced.runtimeOutputDir`. The shell preamble was reading the raw JSON value via `jq`, so a user with `"runtimeOutputDir": "~/.qwen-runtime"` would pass the literal string to the glob — bash does not expand `~` inside double quotes — and the sidecar scan would silently find nothing and fall back to ps-only mode. Add two bash lines after the jq lookup: - `${RUNTIME_DIR/#\~/$HOME}` to substitute leading tilde - `case ... cd && pwd` to resolve relative paths to absolute (clears RUNTIME_DIR if cd fails so the chain falls through to QWEN_HOME) Smoke tested: tilde paths expand, absolute paths pass through, relative paths resolve, nonexistent dirs clear cleanly, empty stays empty. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(skills): /stuck — round-N review feedback Adopted 9 of the 16 review suggestions; declined 5; 1 already done. - Anchor process regex to `(^|/)qwen-code[^ /]*/`. Now matches renamed clones (`qwen-code-dev`, `qwen-code-x1`, worktrees) AND rejects prefix false positives (`analyze-qwen-code/`, `my-qwen-code-tool/`). Verified against 10 cases. - Clarify RSS unit conversion: KB ÷ 1024 = MB, KB ÷ 1048576 = GB. The 4GB threshold is `4194304` KB raw, or 4 in GB. Prevents the model from dividing once and comparing to 4, which would over-alert by 1024×. - Add `State S with low CPU` to the Signs list so initial triage flags the most common hang signature (hung HTTPS to model API) instead of only catching it inside step 3. - Split fast path validation into two guards with distinct messages: dead/wrong-user vs. yours-but-not-Qwen. Plus add the same credential-redaction note that step 2 already has. - Replace `pgrep | xargs -I{} ps` with a single `ps -p $CHILDREN` call (avoids forking N times) and add `-ww` so long child cmdlines don't truncate. - Wrap macOS `sample <pid> 3` with optional `timeout 15` (or `gtimeout 15`). Same belt-and-suspenders pattern used for `lsof`. - Note that `ss -tnp -p` requires root/CAP_NET_ADMIN; non-root sees `-` in the PID column. Tell the model to fall back to `lsof` instead of concluding "no connections". Declined: self-PID via `$$` (wrong PID — `$$` is the spawned shell, not qwen), pgrep fallback for distroless (over-engineering), `\b` matches negative numbers (false alarm — `:[[:space:]]*` won't match through `-`), regex DRY abstraction (no value in markdown prompts), project-level settings.json read (already declined; same trade-off). 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * test(skills): add integration test that parses every bundled SKILL.md The bundled skill loader (`SkillManager.parseSkillFileInternal`) silently catches and debug-logs frontmatter parse errors, so a typo in any SKILL.md (missing `description`, broken `---` delimiter, `allowedTools` written as a scalar) merges with green CI and only surfaces when a user invokes the skill — at which point the skill is missing from autocomplete with no indication why. Add a tiny integration test that walks `packages/core/src/skills/bundled/`, runs every `SKILL.md` through the real `parseSkillContent` (no mocks), and asserts: name matches the directory, description is non-empty, body is non-empty, and `allowedTools` (if present) is an array. Lives in its own file because `skill-load.test.ts` mocks `fs/promises` and the YAML parser, which would defeat the purpose of an integration test. New file uses real fs and the real loader. Negative-case verified: deliberately corrupting `stuck/SKILL.md`'s frontmatter delimiter makes only that file's test fail; restoring it returns the suite to all-green. Addresses wenshao's standing [Critical] review (2026-05-15 12:29Z) about the bundled skill system lacking automated tests for SKILL.md parsing. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) |
||
|
jinye
|
379d14ad00
|
feat(rewind): add file restoration support to /rewind command (#4064)
* feat(rewind): add file restoration support to /rewind command (#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 #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
|
||
|
qqqys
|
0dde1ad704
|
feat(cli): add session-scoped /goal command with judge-driven turn continuation (#4123)
Some checks are pending
Qwen Code CI / Classify PR (push) Waiting to run
Qwen Code CI / Lint (push) Blocked by required conditions
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Blocked by required conditions
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* feat(cli): 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>
|
||
|
jinye
|
264ed82273
|
[codex] feat(serve): add capability registry protocol versions (#4191)
* feat(serve): add capability registry protocol versions Introduce a serve capability registry and advertise protocolVersions from /capabilities while preserving the existing v1 envelope and Stage 1 feature aliases. Update SDK wire types, docs, and focused tests for old-daemon compatibility. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): clarify capability advertisement semantics Address PR review feedback by preserving historical capability versions, separating registered and advertised feature helpers, testing protocol version metadata directly, and keeping runtime exports out of the serve types module. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
qqqys
|
96b30ee427
|
feat(cli): add baseline /doctor memory diagnostics (#4180)
* 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 |
||
|
qqqys
|
372acf1444
|
feat(cli): argument hint + --auto completion for /rename (#4048)
* feat(cli): argument hint + --auto completion for /rename Closes #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 #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> |
||
|
jinye
|
435f711e33
|
feat(cli): warn users that rewind is disabled in IDE mode (#4122)
Some checks are pending
Qwen Code CI / Classify PR (push) Waiting to run
Qwen Code CI / Lint (push) Blocked by required conditions
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Blocked by required conditions
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
|
||
|
易良
|
df32345d05
|
fix(vscode-ide-companion): use existing editor group for diff instead of forcing a new one (#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> |
||
|
jinye
|
dcf7681d65
|
feat(core,cli): add generic atomicWriteFile, wire into Write/Edit tools, upgrade @types/node (#4096)
* feat(core): add generic atomicWriteFile and wire into Write/Edit tools The Write and Edit tools used bare fs.writeFile, risking half-written corrupt files on crash or power loss. Both tools' source code contained explicit TODOs noting atomic write as the fix. - Add atomicWriteFile() supporting string/Buffer with flush (fsync), permission preservation, symlink resolution, and EXDEV fallback - Wire StandardFileSystemService.writeTextFile() through atomicWriteFile - Refactor atomicWriteJSON to delegate to atomicWriteFile (adds fsync) - Deduplicate renameWithRetry from runtimeStatus.ts - Add flush:true to writeWithBackupSync for settings writes - Upgrade @types/node to ^22.0.0 (flush option type support) Closes the TODO in write-file.ts:371-385 and edit.ts:487-497. Ref: #4095 (Phase 1) 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(core): address review comments on atomicWriteFile - Fix permission window: separate existingMode from desiredMode so mode is set during writeFile (not just chmod after), eliminating the brief window where tmp file has overly permissive defaults - Fix broken symlink handling: use lstat+readlink instead of realpath to correctly resolve symlinks whose targets don't exist yet, preventing the symlink from being replaced by rename - Add test for writing through a broken symlink 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(core): address wenshao review on atomicWriteFile - Fix Windows bug: use path.isAbsolute() instead of startsWith('/') - Hoist path import to top-level static import - Resolve full symlink chains via loop (handles A→B→C), with ELOOP guard at 40 hops matching POSIX SYMLOOP_MAX - Mask stat.mode with 0o7777 to strip file-type bits - Document EXDEV fallback atomicity loss in JSDoc - Add tests for relative symlinks and multi-level symlink chains 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(test): fix CI failures from atomic write changes - edit.test.ts: mock writeTextFile instead of chmod 444 for write error test — atomic write creates tmp file in same dir, so readonly target no longer triggers a write error - atomicFileWrite.test.ts: skip permission tests on Windows — chmod is a no-op and stat.mode always returns 0o666 * fix(core): address deepseek review on atomicWriteFile - Add try/catch around chmod calls to handle FAT/exFAT filesystems where POSIX permissions are not supported - Add explicit type annotation to lstats variable * fix: restore version numbers to 0.15.11 after rebase * fix(core): resolve relative symlinks through directory symlinks resolveSymlinkChain used path.dirname() to resolve relative symlink targets, which is purely string-based. When intermediate path components are themselves directory symlinks, the result would be wrong (e.g. /a/link/file → ../target resolves to /a/target instead of the kernel-resolved /b/target). Use fs.realpath() on the parent directory to get the kernel-resolved base for relative-target resolution. * fix(test): normalize path separators in directory symlink test Windows readlink returns native separators (backslashes), causing the directory-symlink test to fail on Windows CI. Wrap both sides of the symlink-target comparison with path.normalize. * refactor(core): dedupe write/chmod logic in atomicWriteFile - Extract writeOptions construction and tryChmod helper, removing duplication between the main write path and the EXDEV fallback - Document atomicWriteJSON's symlink-preservation behavior Addresses deepseek review on PR #4096. |
||
|
ChiGao
|
41bcdae7d8
|
fix(core): refresh systemInstruction in setTools() so progressive MCP tools reach the model (#4166)
Some checks are pending
Qwen Code CI / Classify PR (push) Waiting to run
Qwen Code CI / Lint (push) Blocked by required conditions
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Blocked by required conditions
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* fix(core): refresh systemInstruction in setTools() so progressive MCP tools reach the model Under PR #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 #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 #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 #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> |
||
|
ChiGao
|
9d20536343
|
perf(cli): code-split lowlight to cut startup V8 parse cost (#4070)
* 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 #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 #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
|
||
DennisYu07
|
57282ebb7d
|
feat(hooks): add prompt hook type with LLM evaluation support (#3388)
* implement prompt hook * resolve comment * resolve comment * resolve comment * resolve comment * fix unit test |
||
|
ChiGao
|
78c65c8dee
|
chore(deps): re-upgrade ink 6 → 7.0.3 (upstream Static remount fix landed) (#4119)
* chore(deps): re-upgrade ink 6 → 7.0.3 (upstream Static remount fix landed) PR #3860 first upgraded ink 6 → 7.0.2. PR #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 #4083) contains the exact fixes: - be9f44cda Fix: <Static> remount via key change drops new items (#948) - 669c4386c Fix: Drop stale <Static> output from fullStaticOutput on identity change (#950) - 7c2267c01 Fix `useBoxMetrics` not accepting ref objects with an initial null value (#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 #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 #3941's overflowY-self-managed viewport. #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 #4119 review correctly flagged that splitting the onModelChange flow into two effects ( |
||
|
tanzhenxin
|
f6315b378d
|
refactor(cli): revert dynamic slash command LLM translation (#4145)
* refactor(cli): revert dynamic slash command LLM translation (#4137) Removes the runtime LLM-translation path for dynamic slash command descriptions added in #3871, along with its `general.dynamicCommandTranslation` setting and the `/language translate` subcommand tree. Keeps the built-in locale coverage from the same PR untouched. Localization of dynamic command descriptions should be solved at the source (manifest fields, not runtime model calls); see #4137 for the proposed alternative. * refactor(cli): drop translate prompts from mustTranslateKeys Follow-up to the dynamic command translation revert: the 7 prompt keys were stripped from every locale file in the previous commit, but the allow-list in mustTranslateKeys still demanded them. * refactor(cli): drop dead CommandService.fromCommands and vacuous tests Follow-up cleanup after the dynamic command translation revert. CommandService.fromCommands was introduced by #3871 solely to wrap the LLM-translated command list. With the LLM-translation path gone, it has no remaining non-test callers — remove it and the matching test mock. Also drop two assertions in languageCommand.test.ts that checked for the absence of a top-level /language cache command. They tested a migration state that never existed in this branch and now pass vacuously. * docs: drop /language translate references after revert Two user-facing docs documented the /language translate subcommands (status/on/off/cache refresh/clear) that were removed in the dynamic command translation revert. Strip them so users following the docs don't hit "Invalid command" errors. * refactor(cli): drop unused localizeDescription field The DynamicCommandLocalizationService that read this flag was removed in the revert, leaving the field with five setters and zero readers. Drop the field, its JSDoc, and the five `localizeDescription: true` assignments. Also tidy the now-misleading `modelDescription` JSDoc and the stale `reloadCommands` comment that referenced the removed feature. * refactor(cli): drop unused getLanguageNameForTranslationTarget The only caller was the removed DynamicCommandLocalizationService. Remove the function from `i18n/languages.ts` and the matching import + re-export from `i18n/index.ts`. |
||
|
DennisYu07
|
1c529e4f0a
|
feat(hooks): Add TodoCreated and TodoCompleted hooks for todo lifecycle events (#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 |
||
MikeWang0316tw
|
02a65f90c4
|
fix(i18n): Correct zh-TW translations to match Traditional Chinese conventions (#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 #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 (#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> |
||
|
DennisYu07
|
7c2b51d28e
|
fix(hooks): inject SessionStart additionalContext into chat context (#4115)
* inject addContext for SessionStart * resolve comment * resolve comment * resolve comment * fix comment * unfiy function and resolve comment * resolve comment |
||
Tyler
|
4c18f13051
|
feat(core): add image+video support for Qwen3.6-35B-A3B quant variants (#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> |
||
kkhomej33-netizen
|
6b55d01968
|
fix(cli): preserve debug session across sandbox relaunch
Preserve the outer debug session ID when relaunching into the sandbox by passing it through an internal sandbox-only session flag. |
||
|
jinye
|
ff63da2652
|
refactor(serve): extract createInMemoryChannel helper (#4156 A1) (#4160)
* refactor(serve): extract createInMemoryChannel helper from httpAcpBridge.test.ts (#4156 A1) Sub-PR A1 of issue #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 #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 #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 #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 (#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) |
||
|
dreamWB
|
da1941c975
|
fix(cli): handle MinTTY Ctrl+Backspace as delete-previous-word
Refs #3926 |
||
|
Dragon
|
a656930e82
|
fix(vscode): preserve thinking state and recover missing edit snapshots (#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 |
||
|
Shaojin Wen
|
790f2d0485
|
refactor(serve): 1 daemon = 1 workspace (#3803 §02) (#4113)
* refactor(serve): 1 daemon = 1 workspace (#3803 §02) Stage 1 shipped with M-workspaces-per-daemon routing (`byWorkspaceChannel` Map keyed by request `cwd`). The §02 architectural revision in `docs/comparison/qwen-code-daemon-design/02-architectural-decisions.md` narrows the bridge to 1 daemon = 1 workspace × N sessions: each daemon binds to one canonical workspace path at boot; `POST /session` with a mismatched `cwd` returns 400 `workspace_mismatch`. Multi-workspace deployments run multiple daemon processes (one per workspace, supervised externally — systemd / docker-compose / k8s / `qwen-coordinator`). Bridge state collapses from maps to single optional slots: - `byWorkspaceChannel: Map<string, ChannelInfo>` → `channelInfo?: ChannelInfo` - `inFlightChannelSpawns: Map<string, Promise>` → `inFlightChannelSpawn?: Promise` - `byWorkspace: Map<string, SessionEntry>` → `defaultEntry?: SessionEntry` - `liveChannels: Set<ChannelInfo>` → not needed; `channelInfo` is the live reference, cleared only by `channel.exited` (preserves the tanzhenxin BkUyD invariant that `killAllSync` finds a target mid-SIGTERM-grace) `BridgeOptions.boundWorkspace` becomes required. `WorkspaceMismatchError` is thrown from `spawnOrAttach` when the request's canonical cwd doesn't match the bound path, translated to 400 `workspace_mismatch` (with both paths in the body) by the route layer. `CapabilitiesEnvelope.workspaceCwd` surfaces the bound path so clients pre-flight check + omit `cwd` from `POST /session` (it falls back to the bound workspace). A new `--workspace <path>` CLI flag lets operators override `process.cwd()` at boot. The previous `--http-bridge` / `--multi-workspace` opt-in was never shipped; nothing changes for default users running `qwen serve` in their project directory. Removed code path: ~150 LOC of multi-workspace map machinery in `httpAcpBridge.ts` plus the test cases that exercised it. Test surgery: - New `makeBridge()` helper in `httpAcpBridge.test.ts` injects `boundWorkspace: WS_A` by default; tests that need a different bind (the mismatch test) pass it explicitly. - `does NOT reuse across workspaces` → `rejects cross-workspace requests with WorkspaceMismatchError` (the new semantics under §02). - `shutdown kills every live channel` retargeted to single-channel multi-session shutdown. - `killAllSync force-kills channels even after shutdown cleared byWorkspaceChannel (BkUyD)` retargeted to single-channel: the invariant is the same (channel reference must outlive eager shutdown clearing), the surface is just smaller. - `listWorkspaceSessions` cross-workspace assertion now expects empty for the un-bound path. - `--max-sessions` cap test uses two thread-scope sessions on `WS_A` instead of WS_A + WS_B. Closes #3803 §02. * fix(serve): address review findings on the §02 refactor Two correctness fixes + four doc/test polish items surfaced by the multi-agent review of #4113: 1. `killSession` → `spawnOrAttach` race (Critical). After killing the last session, `channel.kill()` runs through a 5s SIGTERM grace before SIGKILL. During that window a concurrent `spawnOrAttach` used to hit `ensureChannel`, find `channelInfo` still set, and reuse the dying transport — either landing the caller with a sessionId that 404s on every follow-up once `channel.exited` fires, or hanging until the newSession timeout. Fix: add an `isDying: boolean` flag on `ChannelInfo`, set synchronously by `killSession` / `doSpawn`-newSession-failure / `shutdown` BEFORE awaiting `channel.kill()`. `ensureChannel` treats a dying channel as absent and spawns a fresh one. The tanzhenxin BkUyD invariant ("`channelInfo` reference must outlive the kill-await for `killAllSync` mid-grace") is preserved — we set `isDying` but don't clear `channelInfo` until the OS reaps the child via `channel.exited`. A regression test in `httpAcpBridge.test.ts` pins the invariant: a never-resolving `kill()` keeps the SIGTERM grace open while a concurrent spawn verifies the factory was called twice (two distinct handles). 2. `boundWorkspace` canonicalization divergence (Critical). `server.ts` and `runQwenServe.ts` each computed `opts.workspace ?? process.cwd()` independently. The bridge canonicalized that string via `realpathSync.native` (resolving symlinks, case-folding on case-insensitive filesystems); the callers retained the raw form. On macOS HFS+ / APFS or any symlinked path, `/capabilities.workspaceCwd` advertised one spelling while the bridge enforced against another — clients echoing the advertised path back saw `POST /session` succeed but the response carry a different `workspaceCwd`. Fix: export `canonicalizeWorkspace` from `httpAcpBridge.ts` and call it once in `runQwenServe` (after the existence check) and once in `createServeApp`. Both paths land on the same canonical form; the bridge's own re-canonicalize is now a no-op (idempotent). 3. Reject `--workspace` pointing at non-existent directories at boot (Suggestion). `canonicalizeWorkspace`'s ENOENT fallback to `path.resolve` previously let the daemon boot pointed at a path that didn't exist; every `POST /session` then spawned a `qwen --acp` child with that cwd and the agent failed with an opaque ENOENT. Now `runQwenServe` `statSync`s the bound path at boot and rejects "directory does not exist" / "not a directory" with a clear message. 4. Stale docstrings (Nice to have). `types.ts` `ServeMode` JSDoc said "one `qwen --acp` child PER WORKSPACE" — directly contradicted the new `workspace` field's doc in the same file. `commands/serve.ts` `--http-bridge` description said "per workspace" — directly contradicted the `--workspace` flag's help in the same yargs builder. Both updated to "per daemon (the daemon binds to ONE workspace at boot)". 5. Stale `byWorkspace` comment references (Nice to have). `server.ts:188` ("orphaned in byId / byWorkspace") and `httpAcpBridge.test.ts:1210` ("still in byId/byWorkspace at the moment of crash") referenced the removed Map. Updated to `defaultEntry`. 6. `/capabilities` curl example in the Authentication section of `docs/users/qwen-serve.md` was missing the new `workspaceCwd` field — the Quickstart's curl example was updated but the parallel one in the auth section was not. Synced. Tests added: - `killSession marks the channel dying so concurrent spawnOrAttach gets a fresh channel` — pins fix (1). - `--workspace flows end-to-end and surfaces on /capabilities` — exercises the runQwenServe → server.ts → bridge plumbing that no prior test covered. - `rejects --workspace pointing at a non-existent directory` and `rejects --workspace pointing at a regular file` — pin fix (3). - `rejects relative --workspace at boot` — covers the absoluteness check that exists but was untested. Net: +238 / -24 across 8 files. All 149 serve tests pass. * fix(serve): BkUyD overwrite race + Windows-fragile test + doSpawn-failure coverage Round-2 review of #4113 caught three follow-up issues introduced by or left open after round-1's fixes: 1. **BkUyD invariant overwrite race (Critical).** Round-1's `isDying` flag lets `ensureChannel` skip a dying channel and spawn a fresh one. When the fresh spawn completes, `channelInfo = info` overwrote the dying channel's reference — leaving NO global pointer to it. `killAllSync()` then iterated only `channelInfo` (the fresh one) and missed the dying child entirely. A double-Ctrl+C arriving mid-SIGTERM-grace would call `process.exit(1)` before the dying child's per-channel SIGKILL escalation timer fired, orphaning the child. Restore a `aliveChannels: Set<ChannelInfo>` (parallel to the original Stage 1 design, but justified by single-workspace too). Entries added in `ensureChannel`, removed by each channel's `channel.exited` handler. `killAllSync` iterates the SET, not the single attach-target slot. `shutdown` does the same — snapshots every alive channel and kills each, not just the current `channelInfo`. New regression test pins the invariant: spawn → killSession (channel marked dying, kill hangs) → spawnOrAttach (fresh channel overwrites `channelInfo`) → `killAllSync` — expect BOTH channels' `killSync` to fire. Pre-fix only the fresh one would have fired. 2. **Windows-fragile test path.** The new `rejects --workspace pointing at a regular file` test used `new URL(import.meta.url).pathname` to get a path to the test file. On Windows that returns `/C:/path/...` (leading slash); `fs.statSync` then resolves it as path-from-current-drive-root, fails with ENOENT, and the test sees the "does not exist" error message instead of the expected "not a directory" branch. CI runs `windows-latest`. Fix: `fileURLToPath(import.meta.url)` from `node:url`. 3. **doSpawn newSession-failure isDying path was untested.** The round-1 fix added `ci.isDying = true` to both `killSession` AND `doSpawn`'s newSession-failure catch, but only the killSession path had a regression test. Added a parallel one for the doSpawn path: thread-scope bridge with a `newSessionImpl` that throws on the first call → captures the rejection without awaiting it (the bridge's `await ci.channel.kill()` hangs in the test), yields enough cycles for the `isDying = true` sync prefix to settle, then confirms (a) the next `spawnOrAttach` produces a fresh channel and (b) `killAllSync` finds both channels in `aliveChannels`. Also added a `newSessionImpl` option to the test FakeAgent — the existing `initializeThrows` hook covered handshake-time failures, but post-init `newSession` rejections (auth, bad config, mid-init crashes) had no test affordance. All 151 serve tests pass. * docs(serve): update daemon-client-quickstart for §02 single-workspace Round-3 review caught that the SDK example doc was the only one of the three serve-related docs that the §02 refactor didn't touch. Updated: - Boot log example now shows the `, workspace=/path/to/your-project` suffix that `runQwenServe` emits after the §02 changes. - The "Hello daemon" example now reads `caps.workspaceCwd` off `/capabilities` and passes it back as `workspaceCwd` on session creation — illustrating the documented pre-flight pattern, not a hand-written literal that may not match the daemon's actual bind. - Shared-session example makes the prerequisite explicit: the daemon must be bound to `/work/repo` (via `--workspace` or `cd`); under §02 two clients can only share a session if they're both hitting a daemon already bound to that workspace. - New "Workspace mismatch" section shows how to handle the `400 workspace_mismatch` error class: catching `DaemonHttpError`, branching on `body.code`, surfacing `boundWorkspace` / `requestedWorkspace` for the operator. This is a new error class SDK consumers' error handlers should branch on. No code changes; docs only. * feat(sdk,test): align SDK types + integration tests with §02 single-workspace Round-4 review caught one type-drift gap + a set of integration-test assumptions that the §02 refactor invalidated. **SDK type drift.** `DaemonCapabilities` in `packages/sdk-typescript/src/daemon/types.ts` was the SDK-side mirror of `CapabilitiesEnvelope` on the daemon side. The §02 PR added `workspaceCwd: string` to the daemon envelope (and the round-3 doc example reads `caps.workspaceCwd` off the SDK client) but the SDK type wasn't updated. A TypeScript consumer copying the doc snippet verbatim would hit `TS2339 'workspaceCwd' does not exist on type 'DaemonCapabilities'`. The wire field is present so JS consumers wouldn't notice — but the SDK is marketed as a TypeScript quickstart, so this is a real onboarding break. Fix: add `workspaceCwd: string` to `DaemonCapabilities` (parallel to `DaemonSession.workspaceCwd` which is already there). The SDK unit test for `client.capabilities()` was updated to put the new field in the mocked response. **Integration tests.** `qwen-serve-routes.test.ts` spawns a real `qwen serve` daemon in `beforeAll`. Three breakages exposed: 1. The daemon was launched without `--workspace`, so it inherited the test runner's `cwd`. Tests then POST `workspaceCwd: REPO_ROOT` assuming the daemon is bound to the repo root — true when run via `npm test` from the repo, brittle from IDEs / launchers that have a different `cwd`. Added `'--workspace', REPO_ROOT` to the spawn args so the bound workspace is deterministic regardless of where the test runner is launched. 2. The `bad modelServiceId` test used `cwd: '/tmp'`. Under §02 this would now return 400 workspace_mismatch before the session was spawned. Switched to `REPO_ROOT` and softened the `attached` assertion (REPO_ROOT may already have a session from earlier tests in the suite under sessionScope:single). 3. Added three new integration tests pinning the §02 surface end-to-end through a real daemon process: - `rejects cross-workspace cwd with 400 workspace_mismatch` — posts `/tmp` and asserts the full structured error body (`code`, `boundWorkspace`, `requestedWorkspace`). - `omits cwd → falls back to bound workspace` — posts an empty body and asserts the response's `workspaceCwd` matches REPO_ROOT (verifies the runQwenServe → createServeApp → bridge fallback plumbing). - `GET /capabilities surfaces workspaceCwd` — asserts the new SDK type field is populated correctly off the wire. All 422 unit tests pass (cli serve + sdk). Integration tests typecheck clean. * fix(serve): address /review feedback from gpt-5.5 + deepseek-v4-pro Process the 7 inline /review comments on PR #4113: - C1+C3 (SDK): make `DaemonCapabilities.workspaceCwd` and `CreateSessionRequest.workspaceCwd` optional in the SDK types. `workspaceCwd` is an additive field on the v=1 envelope per #3803 §02; the protocol's "bump v only on incompatible changes" stance is honored by leaving the field optional at the type level. `DaemonClient.createOrAttachSession` now omits `cwd` from the body when `workspaceCwd` isn't passed, matching the PR description's "SDK accepts bound path or none". Adds a unit test pinning the empty-body shape. - C2 (docs/users/qwen-serve.md): the `--http-bridge` row described the pre-§02 per-session model; updated to reflect one child per daemon with N sessions multiplexed via ACP `newSession()`. - C4 (server.ts): `WorkspaceMismatchError` was silently 400'ing without a stderr breadcrumb, leaving operators blind to cross-workspace routing drift. Mirrors the SessionLimitExceeded /InvalidPermissionOption observability pattern. - C5 (server.test.ts): the `/capabilities` fallback test compared `res.body.workspaceCwd` against raw `process.cwd()`; on macOS default tmpdir flows (`/var/folders/...` → `/private/var/...`) the canonicalize-once route value diverges. Use `realpathSync.native(process.cwd())` to match the route's canonicalization. - C6 (server.ts): the cwd-not-absolute error said "cwd is required and must be an absolute path" but cwd is now optional under §02. Tightened wording to "must be an absolute path when provided". - C7 (runQwenServe.ts): the `statSync` catch only wrapped ENOENT with a friendly diagnostic; EACCES / EPERM (typical for SIP-protected dirs on macOS or root-owned paths the daemon's UID can't traverse) re-threw as raw `SystemError`. Wrap both codes with a `--workspace`-context message so the boot failure points at the flag the operator set. Docs: quickstart shows the explicit-pass-or-omit options side by side; protocol reference notes `workspaceCwd` is additive to v=1. * fix(serve/test): make /work/bound literals Windows-portable Windows CI failed on this PR's two new tests because returns (drive-relative absolute), so the route's canonicalize step diverged from the hardcoded literal. Mirror the WS_A/WS_B pattern already used in httpAcpBridge.test.ts: define WS_BOUND / WS_DIFFERENT via `path.resolve(path.sep, …)` and use the constants everywhere. The 400 workspace_mismatch test would still have passed (mock controls both throw + assertion) but I aligned it for consistency. Failures from CI run 25806528710: expected 'D:\work\bound' to be '/work/bound' (Object.is) Affected tests: - createServeApp > GET /capabilities > reports the bound workspace - createServeApp > POST /session > 200 when cwd is omitted * fix(serve): address second /review round (gpt-5.5 + deepseek-v4-pro) Four new inline findings from the latest /review pass: - N1 (integration-tests/cli/qwen-serve-routes.test.ts) — Critical: the `workspace_mismatch` assertion compared `requestedWorkspace` against the literal `'/tmp'`, but the bridge canonicalizes via `realpathSync.native` and on macOS `/tmp` is a symlink to `/private/tmp`. Compare against `realpathSync.native('/tmp')` so the assertion is portable. - N2 (packages/cli/src/serve/types.ts): `CapabilitiesEnvelope.workspaceCwd: string` (server side) diverged from the SDK's `DaemonCapabilities.workspaceCwd?: string`. Made the server type optional too — matches the SDK, matches the protocol doc's "additive to v=1" framing, doesn't change runtime emission (the post-§02 server still always populates the field). - N3 + N4 (packages/cli/src/serve/server.ts + sdk-typescript/.../DaemonClient.ts): the route's `cwd` validation treated every non-string body value (`null`, `123`, `{}`, `[]`) the same as omitted, silently falling back to `boundWorkspace`. That hid client/orchestrator serialization bugs as "session attached to wrong workspace". Now the route uses `'cwd' in body` to detect presence and rejects presence-but-not-a-string with `400 'cwd must be a string absolute path when provided'`. Empty string still hits the existing `path.isAbsolute` branch ("must be an absolute path when provided"), so an SDK caller passing `workspaceCwd: ''` no longer silently lands in the daemon's bound workspace. SDK side: reverted my conditional spread to `cwd: req.workspaceCwd` unconditional. `JSON.stringify` strips `undefined` automatically (so omitted `workspaceCwd` becomes "no `cwd` key" on the wire, as before), but empty-string is now forwarded verbatim and the server's 400 surfaces the bug instead of the SDK swallowing it. Added a unit test pinning the empty-string-forwarded shape. Server tests: - `400 when cwd is present but not a string` covers null / number / object / array via a sub-loop. - `400 when cwd is the empty string` pins the isAbsolute path. bridge: 73/73; server: 80/80 (was 78, +2 new); SDK: 40/40 (was 39, +1 empty-string test). tsc clean for SDK and PR-touched CLI files. * fix(serve): use const cwd in POST /session (prefer-const lint) CI lint failed with packages/cli/src/serve/server.ts:199:9 prefer-const: 'cwd' is never reassigned. The wave-4 rewrite split the original 'let cwd; if (!cwd) cwd = boundWorkspace' into a single ternary, which removes the only mutation path; the variable should be const accordingly. * fix(serve): address third /review round (gpt-5.5 + glm-5.1 + deepseek-v4-pro) Five new inline findings; M1 was already resolved in |
||
|
tanzhenxin
|
fa6f664a6f
|
test(integration): pin simple-mcp-server to legacy MCP discovery path (#4164)
The progressive-MCP rollout (#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-#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 #4163 is fixed. |
||
|
tanzhenxin
|
cc800d0132
|
fix(core): support cross-auth fast side queries (#4117)
Some checks are pending
Qwen Code CI / Classify PR (push) Waiting to run
Qwen Code CI / Lint (push) Blocked by required conditions
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Blocked by required conditions
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* fix(core): support cross-auth fast side queries * refactor(core): hoist resolveForModel selector and refresh side-query docs Compute the model selector once at the top of `resolveForModel` and pass it through to `createContentGeneratorForModel` and `resolveModelAcrossAuthTypes`. This eliminates the redundant selector resolution that happened up to five times per cross-auth side query (once per call, plus once inside each downstream helper). Also update the JSDoc for `SideQueryJsonOptions.model` and `SideQueryTextOptions.model` to reflect the actual fallback chain (`getFastModelForSideQuery` → `getFastModel` → `getModel` → `DEFAULT_QWEN_MODEL`) introduced in this PR. |
||
|
tanzhenxin
|
85c10c1619
|
fix(core): correct context-usage Footer for prompt size and Anthropic caches (#4109)
* fix(core): include cache_creation_input_tokens in Anthropic prompt accounting
Anthropic reports the prompt across three mutually-exclusive fields —
input_tokens, cache_read_input_tokens, cache_creation_input_tokens —
but the adapter only summed input + cache_read, dropping the
cache_creation bucket. On a fresh session that wrote the system prompt
to cache, the reported promptTokenCount was off by the cache-creation
amount.
Extract the normalization into a shared helper used by both streaming
and non-streaming paths, and add a guard for non-conforming providers
that expose the Anthropic protocol but follow OpenAI-style accounting
(input_tokens already covers the cache fields). When input_tokens is at
least as large as both cache fields and at least one cache field is
non-zero, trust input_tokens alone so we don't double-count.
* fix(core): prefer promptTokenCount over totalTokenCount for context display
The Footer's "context used" indicator is meant to track prompt size —
how much of the context window the next request will carry. The current
code preferred totalTokenCount (= prompt + output), so output tokens
generated in the in-flight round were double-counted. Across turns this
caused the % bar to oscillate non-monotonically: it could *decrease*
between turns whenever the prior round's output was large.
Flip the preference at every consumer site that drives the live counter:
the per-stream-chunk update in the main chat, the per-round update in
the subagent runtime (which drives auto-compaction), the session-resume
walk, and the in-process agent panel's listener. Producer sites that
expose total for billing/export are left unchanged.
* fix(core): use cache_creation as the discriminator in Anthropic usage normalization
The previous guard fell back to "input alone" whenever input_tokens was
at least as large as both cache fields. In a real Anthropic conversation
input_tokens grows past cache_creation_input_tokens as history
accumulates, so the guard inevitably mis-classified every later turn as
OpenAI-style and silently dropped the cache_creation portion from the
displayed prompt size. The Footer would show a one-shot drop at the
crossover point and then keep under-reporting by ~32k tokens.
cache_creation_input_tokens is unique to Anthropic's protocol (OpenAI
has no equivalent), so its presence is a strong signal the response
follows real Anthropic semantics. Use that as the primary discriminator
and only fall back to "input alone" when cache_creation is zero, cache
reads are reported, and input already covers them — the actual OpenAI-
on-Anthropic case the guard was meant to catch.
Adds a regression test that locks in the crossover scenario.
* chore(core): address PR review — restore isFinite guard and cover cache-field plumbing
- Restore the `isFinite` guard on `lastPromptTokenCount`: the previous
`if (contextTok)` relaxation accepted `Infinity` (truthy), which a
malformed provider response could otherwise latch and poison the
downstream compaction math.
- Add unit coverage for the cache-field plumbing the PR introduced:
- usage.ts: real-Anthropic warm-turn case where `cache_read > 0` and
`cache_creation > 0` simultaneously (mid-conversation breakpoint
advance over an already-cached prefix).
- converter.ts: `convertAnthropicResponseToGemini` now exercised with
all three prompt buckets present to confirm both cache fields are
forwarded to `usageMetadata`.
- anthropicContentGenerator.ts: streaming pipeline test that includes
`cache_creation_input_tokens` in `message_start` and asserts the
accumulated `usageMetadata` carries it through to the final chunk.
|
||
Coloring
|
dd1d68644d
|
feat(cli): add modelscope api provider (#4150) | ||
顾盼
|
609e05baee
|
feat(tools): add generic worktree support — EnterWorktree/ExitWorktree + Agent isolation (#4073)
* feat(tools): add generic worktree support (Phase A + B of #4056) Adds first-class git worktree as a general-purpose capability: Phase A — User-facing tools - enter_worktree: creates `<projectRoot>/.qwen/worktrees/<slug>` on a `worktree-<slug>` branch and returns the absolute path. Slug auto-generated when omitted; validated against path traversal and disallowed characters. - exit_worktree: keeps or removes the worktree (and its branch). Refuses to remove a worktree with uncommitted tracked changes or untracked files unless `discard_changes: true` is set. Phase B — Agent isolation - Agent tool gains an `isolation: 'worktree'` parameter that provisions a temporary `agent-<7hex>` worktree, prepends a worktree notice to the task prompt, and on completion either removes the worktree (no changes) or preserves it and reports its path/branch in the result. Background and foreground execution paths both wired up; rejected for fork agents. - worktreeCleanup.cleanupStaleAgentWorktrees: fail-closed sweep for ephemeral `agent-<7hex>` worktrees older than 30 days with no tracked changes and no unpushed commits. User-named worktrees are never swept. - buildWorktreeNotice helper for fork subagents (parity with claude-code). Arena compatibility - The existing Arena worktree implementation (GitWorktreeService.setupWorktrees, ArenaManager, agents.arena.worktreeBaseDir) is untouched. Arena uses its own batch APIs and `~/.qwen/arena` base dir; the new general-purpose APIs live alongside under `<projectRoot>/.qwen/worktrees/`. Subagent safety - enter_worktree / exit_worktree are added to EXCLUDED_TOOLS_FOR_SUBAGENTS so a subagent cannot mutate the parent session's worktree state. Refs #4056 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(worktree): use path.join in expected paths so the test passes on Windows The Windows CI run reported `enter-worktree.test.ts` failing because the expected string was hardcoded with `/` while `getUserWorktreesDir()` uses `path.join`, which returns `\\` on Windows. Build the expected path via `path.join` so the platform-correct separator is compared. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(enter-worktree): treat empty name as auto-generate Some models pass `{ "name": "" }` when calling EnterWorktree, because the schema marks `name` as optional and they emit an empty placeholder. The previous validation rejected the empty string with "Worktree name must be a non-empty string", which surprised users running the auto-slug path. Now both `validateToolParams` and `execute` treat `name: ""` as equivalent to `name: undefined` and fall back to the auto-generated `{adj}-{noun}-{4hex}` slug. Explicit invalid slugs (`'../etc'`, `'a/b'`, etc.) are still rejected as before. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(worktree): address review findings 1-6 from PR #4073 Six issues raised on the initial review; each addressed with a verifiable guarantee. 1. Real isolation for `agent isolation: 'worktree'` Before: subagent's Config still resolved `getTargetDir()` to the parent project root, so Edit/Write/Read workspace checks and Shell's default cwd silently operated on the parent tree. The cleanup helper then saw a "clean" worktree and removed it — destroying the evidence. After: the worktree is provisioned BEFORE `createApprovalModeOverride`, and the resulting agent Config has `getTargetDir`/`getCwd`/`getWorkingDir` rebound to the worktree path. Relative paths, unqualified shell commands, and glob/grep roots all confine to the worktree. 2. `exit_worktree action='remove'` now prompts in default/auto-edit modes Added `getDefaultPermission()` on the invocation: `'ask'` when action is `remove`, `'allow'` when `keep`. Brings it in line with edit, write_file, and run_shell_command. 3. Force-delete no longer silently destroys unpushed commits `removeUserWorktree` now uses `git branch -d` (refuses unmerged) by default and surfaces `branchPreserved: true` when git refuses. Added `hasUnmergedWorktreeCommits` (checks if branch tip is reachable from any other local branch or remote ref). Both the agent isolation cleanup and `exit_worktree action='remove'` use this check: if the branch has work not covered elsewhere, the worktree+branch are preserved even when `discard_changes: true` is set (there is no `discard_commits` flag — committed work is rarely what `remove` means to discard). 4. Both new tools are now deferred behind ToolSearch `shouldDefer: true` + `searchHint` on both. Verified via openai-logging: `enter_worktree` and `exit_worktree` no longer appear in the function- declaration list sent on every API request. 5. Stale-worktree cleanup is wired in `Config.initialize()` fires `cleanupStaleAgentWorktrees(targetDir)` as a non-awaited startup sweep (skipped in bare mode). Picks up orphaned `agent-<7hex>` worktrees left by crashed runs. 6. Foreground isolation no longer leaks on uncaught throw The foreground try block tracks whether the cleanup helper ran on the success path; the finally block invokes it as a fallback when the try bailed early. Mirrors the background path's pattern. Verification: - Unit tests: 83 passed (16 worktree + 64 existing agent + 3 cleanup) — no regressions. - E2E #1: agent told to write `hello.txt` via RELATIVE path — file landed at `.qwen/worktrees/agent-XXXXXXX/hello.txt`, NOT at the parent root. - E2E #3: created worktree, committed work inside it, called exit_worktree with `discard_changes=true` — refused with clear message; worktree and branch both preserved. - E2E #4: openai-logging confirms worktree tools absent from API tool list (7 tools sent instead of 9). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(worktree): address review round 2 findings (1 from tanzhenxin, 7+8 from wenshao) The first round closed the data-loss-class issues. This round addresses follow-ups from a deeper audit: 1. Stale-worktree sweep was inert on common-case repos `cleanupStaleAgentWorktrees` previously ran `git log --branches --not --remotes --oneline` from each worktree's directory — that lists unpushed commits across EVERY local branch, not just the worktree's own branch. On any repo with no remote configured (or with stray unpushed branches), the sweep refused to remove every candidate. Replaced with `service.hasUnmergedWorktreeCommits(slug)` which scopes the check to the worktree branch via `for-each-ref --contains <tip>`. Also added the `branchPreserved` warn log requested in M7 and an `fs.access` shortcut for the empty-worktrees-dir case (M8). 2. `cleanupWorktreeIsolation` and `worktreeIsolation` were inside the inner try (~660 lines from the outer catch). Hoisted both to the top of `execute()` so the outer catch can reap or preserve the worktree when anything between provisioning and the inner try throws (e.g. `createApprovalModeOverride`, agent creation). Closure carries the resolved `repoRoot` so cleanup never has to re-resolve. 3. Background error path discarded the cleanup result. Now captures `formatWorktreeSuffix(...)` and appends it to the registry's failure /cancel message, so users see the preserved path/branch even when the agent crashed before reporting. 4. `cleanupWorktreeIsolation` now treats `result.success === false` as "worktree still on disk" and surfaces it as preserved instead of silently dropping it from the result. 5. Override was incomplete. Several Config methods read `this.targetDir` directly (`getProjectRoot`, `getFileService`, etc.) — own-property getter overrides did not redirect them. Now also shadows `targetDir` and `cwd` as own properties on the agent's Config override, swaps in a `FileDiscoveryService` rooted at the worktree, and rebuilds `WorkspaceContext` to point at the worktree only. Verified end-to-end: shell `pwd > pwd-record.txt` (no directory arg) lands at `.qwen/worktrees/agent-<7hex>/pwd-record.txt`, not the parent root. 6. monorepo subdir issue. Both `enter_worktree` and the agent isolation path now resolve `git rev-parse --show-toplevel` first and anchor `.qwen/worktrees/<slug>` at the repo root. Worktrees created from any subdirectory now end up where the startup sweep can find them. 7. Replaced `git worktree add -B` (silent force-reset of pre-existing branches) with `git worktree add -b` plus an explicit existence check via `git for-each-ref` (NOT `show-ref --quiet`, which simple-git swallows). Pre-existing `worktree-<slug>` branches now trigger a clear error instead of clobbering committed work. 8. First worktree creation in a repo writes `<projectRoot>/.qwen/.gitignore` with `worktrees/` so worktree contents stay out of the parent's `git status`, glob/grep results, and bundle tools. Idempotent: never overwrites an existing file. 9. Logging across the failure paths (`enter_worktree` errors, `agent.ts:failWorktreeProvisioning`, `cleanupWorktreeIsolation`, `hasUnmergedWorktreeCommits` swallowed errors, `cleanupStaleAgentWorktrees`'s `branchPreserved` race). 10. `exit_worktree` no longer suggests `discard_changes: true` when the git status check itself fails — that would be advising the user to bypass a safety check whose precondition is unknown. Now points at the underlying repo problem. 11. `generateAutoSlug` switched from `Math.random()` (4 hex, weak RNG, one-in-65k collision) to `randomBytes` (6 hex, ~16M combinations). Two RNG sources in this file collapsed to one. Pushed back: the TOCTOU swap in `removeUserWorktree` (S6 round 1) is left as-is — `git branch -d` is the real safety, and reordering does not eliminate the window. Windows reserved-name validation (M5 round 2) deferred to a follow-up; the current allowlist already rejects path separators, `..`, leading dot/dash, and the >64-char case. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(worktree): use randomInt to silence CodeQL biased-modulo finding CodeQL's `js/biased-cryptographic-random` flagged `randomBytes(4)[i] % ARRAY.length` in `generateAutoSlug`. The math is actually exact for the current word-list lengths (256 % 8 == 0), but the lint rule does not know that — and a future contributor changing the list to a non-power-of-two length would silently introduce bias. Switched the index lookups to `crypto.randomInt(0, length)`, which uses rejection sampling and is uniform by construction. Suffix still uses `randomBytes(3).toString('hex')` since hex encoding is unbiased. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(worktree): address review round 3 findings 1-6 from PR #4073 The previous round added `getRepoTopLevel` for `enter_worktree`'s provisioning, but missed three sibling call sites that still used the raw cwd. The double-cleanup race in the foreground path also leaked stale `[worktree preserved]` suffixes on rejected promises. All six findings from the deeper audit are addressed: 1. exit_worktree now resolves through `getRepoTopLevel()` before building its `GitWorktreeService`, mirroring `enter_worktree`. Without this, launching `qwen` from a monorepo subdirectory created the worktree under the repo root but exit_worktree looked under the subdir's `.qwen/worktrees/` and always returned "Worktree not found". Verified end-to-end: enter + exit from `packages/core/` works. 2. agent.ts cleanup helper now nulls `worktreeIsolation` immediately after capturing the closure value. The previous structure could reach the helper twice — once in the foreground try's success path and once in the foreground finally fallback (or once in the inner try and once in the outer catch on a thrown rejection). The second call would `hasWorktreeChanges()` against a directory the first call already removed, fail-closed, and emit a bogus `[worktree preserved: <missing path>]` suffix. 3. Config.initialize's startup sweep now resolves `getRepoTopLevel()` before invoking `cleanupStaleAgentWorktrees`. Without this, every subdir launch scanned a non-existent `<subdir>/.qwen/worktrees/` and the 30-day expiry sweep was permanently a no-op. 4. agent.ts's `buildWorktreeNotice` now passes `worktreeIsolation.repoRoot` as `parentCwd` instead of `this.config.getTargetDir()`. The notice's path-translation guidance (≈ "translate paths from <parent> to <worktree>") would otherwise misdirect the subagent in a monorepo subdir launch. 5. Removed dead method `GitWorktreeService.listUserWorktrees`. It had no callers anywhere in the codebase and used `execSync` in a loop (would have blocked the event loop if anyone wired it up). 6. `localBranchExists` no longer swallows git failures silently. The defensive `false` default is preserved (so `git worktree add -b` itself surfaces the conflict if the check missed an existing branch), but the catch now logs via `debugLogger.warn` so disk-full / permission / ref-store-corruption cases are visible in debug output instead of being invisible. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(worktree): address review round 4 findings (data-loss + visibility) Seven actionable findings from a deeper audit, all closed: 1. User worktree slugs could collide with ephemeral-agent shape `validateUserWorktreeSlug` did not reject names starting with `agent-`, so a user-named `agent-1234567` matched the cleanup regex `/^agent-[0-9a-f]{7}$/` and would be silently swept after 30 days along with whatever work was in it. Now reserved — clear error message points users at the cause. 2. Slug producer and consumer were string-coupled across files `agent.ts` hardcoded `agent-${hex(7)}` and `worktreeCleanup.ts` independently hardcoded `/^agent-[0-9a-f]{7}$/`. Future change to hex length on one side would silently break the other. Lifted `AGENT_WORKTREE_PREFIX`, `AGENT_WORKTREE_HEX_LENGTH`, `AGENT_WORKTREE_SLUG_PATTERN`, and `generateAgentWorktreeSlug()` to `gitWorktreeService.ts`; both call sites import them. 3. Startup sweep was invisible at default log level Fire-and-forget sweep used `debug` for errors and discarded the success count. A leak-chasing operator had no log breadcrumb. Errors promoted to `warn`; successful removals (count > 0) logged at `info`. 4. `getRepoTopLevel()` silent catch Returned `null` on any git failure with no log. Combined with `?? cwd` fallback in callers, a flaky git would have made worktree creators and the startup sweep disagree silently about which dir to use. Now logs the underlying error. 5. `hasTrackedChanges()` silent catch Cleanup's fail-closed `return true` had no log. Couldn't tell "has real changes — leave alone" from "git index unreadable — repo may be corrupt". Now logs. 6. `cleanupWorktreeIsolation` claimed `preservedPath` for a removed dir When `removeUserWorktree` returns `{ success: true, branchPreserved: true }` it has already deleted the directory and failed only on `git branch -d`. The helper still reported the (now non-existent) path as preserved. Now returns only `preservedBranch` for that case; `formatWorktreeSuffix` emits a distinct message instructing recovery via `git worktree add <new-path> <branch>`. 7. `removeUserWorktree` swallowed branch-delete failures Both `-d` and `-D` catch blocks were empty. Locked refs, perms, disk full all looked identical to "unmerged commits". Both now `debugLogger.warn` with the underlying error. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(worktree): self-review pass — reuse, parallelism, dead code Self-review caught a handful of issues across three categories: Reuse: - `pathExists` in the new code now uses the existing `fileExists` from `utils/fileUtils.ts` instead of duplicating an `fs.access` wrapper. - `worktree-` branch prefix was string-literalled in five places. Added `WORKTREE_BRANCH_PREFIX` and `worktreeBranchForSlug(slug)` exports in `gitWorktreeService.ts`; updated `gitWorktreeService.ts`, `worktreeCleanup.ts`, and `exit-worktree.ts` to use them. Future prefix changes are a single edit. Efficiency: - `Config.initialize` used two `await import(...)` calls inside the startup-sweep IIFE, paying that cost on every CLI start. Switched to static imports at the top of `config.ts` — the modules are tiny and the dynamic indirection bought nothing. - `cleanupWorktreeIsolation` in `agent.ts` ran `hasWorktreeChanges` and `hasUnmergedWorktreeCommits` sequentially. They have no data dependency on each other and each spawns its own `git` invocation; `Promise.all` halves the cleanup wall-clock on the common path. Same fix in `worktreeCleanup.ts`'s per-entry loop. - `ensureWorktreesGitignored` used `fs.access` then `fs.writeFile`, a TOCTOU race when two agent invocations created worktrees concurrently (both could pass the `access` check and the second would clobber the first's `.gitignore`). Now writes with `flag: 'wx'` and treats `EEXIST` as the no-op case — atomic in one syscall. Quality: - Dropped the `worktreeCleanupRan` boolean in the foreground execution path. `cleanupWorktreeIsolation` already nulls its closure variable at the top of every call (see the comment at its definition), so re-entries are no-ops. The boolean and its tracking were dead weight that obscured the real guard. - Trimmed the Phase-2 override comment block to drop the WHAT-stating enumerations (items 3 and 4 just narrated the lines below) and removed a navigation comment about hoisted helpers — the helpers are visible at the top of the same method. 84 unit tests pass; typecheck clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(worktree): address review round 5 — design-doc commitments + correctness Five critical findings + four suggestions, all closed. Critical: 1. Wrong base branch for agent isolation. `createUserWorktree(slug)` with no `baseBranch` arg fell back to `getCurrentBranch()` on the **main** working tree, returning `main` regardless of which branch the user was actually on. A subagent invoked from `feature-x` would silently start from `main` and produce diffs against the wrong baseline. `enter_worktree` had the same bug. Both now resolve the parent's current branch first and pass it explicitly. Verified end-to-end: `git checkout feature-x` → `enter_worktree` → worktree HEAD includes the feature-x commit. 2. `countWorktreeChanges` (used by `exit_worktree`'s dirty-state guard) missed `status.conflicted[]`. In simple-git that array is mutually exclusive with the staged/modified/etc. arrays, so a worktree mid-merge with only conflicts looked `{tracked: 0, untracked: 0}` to the guard and `action='remove'` would proceed without `discard_changes: true`. Added `+ status.conflicted.length`. 3. `exit_worktree` had no session-ownership check, contradicting the design doc's "only operates on worktrees created by THIS session". In yolo mode a prompt injection could enumerate `.qwen/worktrees/` and pass any name to drop another session's work. Now: `enter_worktree` and agent isolation write a `.qwen-session` marker into the worktree at provisioning time; `exit_worktree action='remove'` reads it and refuses if it does not match the current `Config.getSessionId()`. Worktrees from before this guard (no marker file) are treated as "owner unknown" — allowed with a warn log so the change is observable. 4. `enter_worktree` did not refuse nested invocations from inside an existing worktree, contradicting the design doc. Now rejects any cwd containing `.qwen/worktrees/` as a path component, with a clear "Already inside a git worktree…" message. Verified: enter from inside a worktree returns is_error with that text. 6. `hasTrackedChanges` (cleanup sweep) had the same `conflicted[]` gap. Rewrote to use raw `git status --porcelain --untracked-files=no` which lists every tracked change including `UU` conflict markers in a single git call and explicitly skips the untracked walk (the prior comment claimed to skip it, but `status()` always does the scan). Suggestion: 7. `buildWorktreeNotice` now receives the parent agent's actual `getTargetDir()` again (was switched to `repoRoot` in round 3 on a different reviewer's suggestion; round-5 caught that the model's inherited paths reference the parent's cwd, not necessarily the repo root, so the prior behaviour was correct). 8. Startup sweep now does `fs.access(<targetDir>/.qwen/worktrees)` *before* importing GitWorktreeService and spawning `git rev-parse --show-toplevel`. The git probe is reserved for users who actually have a worktrees directory locally — 99% of users pay only one syscall on startup. 9. Tests: - New `exit-worktree.test.ts` covers metadata, validation, `getDefaultPermission` (ask vs allow), and getDescription. - `agent.test.ts` adds three `validateToolParams` cases for the `isolation` parameter (accepted with subagent_type, rejected without, rejected for non-"worktree" values). - `enter-worktree.test.ts` adds round-trip tests for `writeWorktreeSessionMarker` / `readWorktreeSessionMarker` plus a `worktreeBranchForSlug` sanity check. - Total: 101 tests pass (was 86 → +15). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(test): drop unused @ts-expect-error in exit-worktree.test.ts Empty string `''` is a valid `string` type, so the @ts-expect-error directive on `validateToolParams({ name: '', action: 'keep' })` did nothing — TypeScript correctly accepted the line, and `tsc --build` in CI reported TS2578 ("Unused '@ts-expect-error' directive"). The runtime assertion already covers the case; the directive was leftover from an earlier draft. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(test): use importActual in ArenaManager mock to preserve new exports The Arena test mocks `gitWorktreeService.js` with a factory that returns only `{ GitWorktreeService }`. PR #4073 added several other exports to that module (`AGENT_WORKTREE_SLUG_PATTERN`, `WORKTREE_BRANCH_PREFIX`, `worktreeBranchForSlug`, `generateAgentWorktreeSlug`, `writeWorktreeSessionMarker`, `readWorktreeSessionMarker`, `WORKTREE_SESSION_FILE`). Other modules in the dep graph reach the mocked surface — most notably `worktreeCleanup.ts` imports `AGENT_WORKTREE_SLUG_PATTERN` and `worktreeBranchForSlug`, and now reaches the mock via the static `config.ts` → `worktreeCleanup.ts` import chain added in the self-review pass. The Arena test failed at module-load with: Caused by: Error: [vitest] No "AGENT_WORKTREE_SLUG_PATTERN" export is defined on the "../../services/gitWorktreeService.js" mock. Did you forget to return it from "vi.mock"? Use `importOriginal` to capture every real export, spread it into the return object, and only replace `GitWorktreeService` (the class the test actually needs to mock). The class-level mock keeps its existing static-method shims. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(worktree): address review round 6 (5 critical + 6 suggestions) The biggest item — #1 — is a self-inflicted regression from round 5: the new agent- prefix reservation in `validateUserWorktreeSlug` rejected EVERY slug that `generateAgentWorktreeSlug` produces, since that helper emits exactly `agent-<7hex>`. Net effect: every `AgentTool isolation: 'worktree'` invocation failed at validation. The reservation now allows the canonical pattern through (everything the helper can produce) and only rejects user-chosen `agent-*` names that don't match it. Added a round-trip regression guard: 50 `generateAgentWorktreeSlug()` outputs are fed back through `validateUserWorktreeSlug` and must all pass. Other critical fixes: 2. `hasWorktreeChanges` (used by agent isolation cleanup) was the one remaining caller relying solely on `status.isClean()`. Defensive `|| status.conflicted.length > 0` so a future simple-git bookkeeping change can't let a mid-merge worktree appear clean and get auto-deleted. 3. `readWorktreeSessionMarker` swallowed every I/O error as "marker missing", which let a disk error / EACCES silently bypass the session-ownership guard. ENOENT is still treated as missing (legitimate); every other code now logs. 4. `exit_worktree` `fs.stat` catch was the same shape — every error collapsed to "Worktree not found". ENOENT → not found; everything else logs and returns a distinct "cannot access" error. 5. `cleanupStaleAgentWorktrees` `fs.stat` catch was again the same. ENOENT → silently skip (entry vanished between readdir and stat); everything else logs. Suggestions: 6. Startup sweep fast-bail was running BEFORE resolving the repo top-level. For monorepo subdir launches, `targetDir/.qwen/worktrees` never exists and the sweep early-returned — permanently a no-op. Now resolves the root first, then fast-bails against the resolved `<root>/.qwen/worktrees`. Also logs the skip case so operators can tell "skipped" from "ran, found nothing". 7. `.qwen-session` marker was visible to `git add -A` inside the worktree. Now writes a `.git/info/exclude` rule (resolved via `git rev-parse --git-dir`, since worktree `.git` is a file pointing at the parent repo's `.git/worktrees/<name>/`). Best-effort: failure to write the rule does not abort provisioning. 8. Agent isolation now refuses to provision when the parent's cwd is already inside a worktree — same regex guard as `enter_worktree`. 9. `exit_worktree`'s wrapper around `hasUnmergedWorktreeCommits` now logs at the call site so the chain (caller → reason it asked → underlying git error) is complete in operator logs. 10. Sweep now logs unconditionally at `info`. Three distinct messages: "skipped (no worktrees dir)", "ran, nothing to remove", "removed N". Tests: 11. New `execute()` coverage: • exit-worktree: session-ownership refusal, keep happy path, legacy/no-marker fallthrough with warn log, missing-worktree error, unmerged-commits guard with `discard_changes: true`, `writeWorktreeSessionMarker` round-trip. • enter-worktree: nested-guard rejection, non-git-repo error. These spin up real temp git repos (no filesystem mocking) and drive the actual tool invocation pipeline. Total: 135 tests pass (was 101 → +34). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(worktree): demote noise startup-sweep logs to debug Self-review pass applying the round-6 review-triage framework (filter #5: "If a log only fires on the happy path, it's noise.") to my own round-6 changes: - "Stale worktree sweep skipped: <dir> does not exist" — fires on every CLI start for ~99% of users who never use worktrees. - "Stale worktree sweep ran under <root>: nothing to remove" — fires on every CLI start for users who have any worktrees but no stale ones at the moment. Both are happy-path noise at `info`. Demoted to `debug` so an operator can opt in via `--debug` when they want to confirm the sweep is wired up, but normal output stays clean. Only the actually-actionable case ("removed N worktrees") stays at `info` — that's the signal someone chasing a worktree leak would grep for. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(worktree): close AUTO_EDIT bypass + parent-dirty stale-code hazard Round-7 review caught two correctness gaps: 1. exit_worktree action='remove' was still auto-approved in AUTO_EDIT `getDefaultPermission` returning 'ask' is necessary but not sufficient. `permissionFlow.isAutoEditApproved` auto-approves any tool whose `confirmationDetails.type` is 'edit' OR 'info', and `BaseToolInvocation` returns 'info' by default. So a session in AUTO_EDIT could silently destroy a worktree (with branch deletion) without a confirmation prompt — the data-loss path the round-1 `'ask'` switch was meant to close. Now overrides `getConfirmationDetails` to return `type: 'exec'` for action=remove, which keeps the prompt in AUTO_EDIT. The `keep` action still falls through to the base info-type since it is non-destructive. Regression-guard test asserts the type is 'exec' (not 'info') for remove and that the command field describes both the worktree-remove and branch-delete operations. 2. Agent isolation worktrees ran against parent's HEAD, not its working tree `git worktree add -b <branch> <path> <base>` only checks out the base ref's tip — uncommitted edits in the parent's working tree do NOT propagate. The "edit code → ask review/test agent before committing" workflow silently ran the subagent against the pre-edit HEAD and returned results that looked authoritative but reflected stale code. Reviewer offered two options: overlay parent's dirty state à la Arena (~50 LOC, edge cases), or refuse isolation when parent is dirty (~10 LOC, clear UX). Chose the latter for Phase B scope — simpler, decisive, and matches the design-doc's explicit commitment that dirty-state overlay is Arena-specific. Users can commit/stash before re-invoking agent isolation; overlay can be a follow-up if users complain about the friction. Fail-closed on the dirty-check itself (assume dirty rather than silently launch on a possibly-stale tree). Test exercises both "dirty parent → guard fires" and "clean parent → guard passes" against real temp git repos. 139 unit tests pass (was 135, +4 regression guards). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> |
||
|
ChiGao
|
a86404e9ea
|
fix(cli): apply /language output to running session without restart (#4143)
* fix(cli): apply /language output to running session without restart `/language output <lang>` wrote ~/.qwen/output-language.md and persisted the setting, but the rule reaches the model through the system instruction which is bound once when GeminiChat is created at startup. The command therefore had to tell the user "Please restart the application for the changes to take effect." Refresh hierarchical memory after writing the rule file so userMemory re-reads output-language.md, then rebuild and re-bind the main-session system instruction on the live chat via a new GeminiClient.refreshSystemInstruction() helper. The change takes effect on the next turn without restarting the session and without losing conversation history. Drop the restart notice from the success message. Fixes #4142 * test(cli): assert refresh order in /language output and cover failure path - The fix for /language output relies on refreshHierarchicalMemory running *before* refreshSystemInstruction; otherwise the new systemInstruction is rebuilt from stale userMemory and the language switch silently fails to take effect. Assert ordering with invocationCallOrder so a regression cannot pass review. - Add a test that the command still reports success when the in-session refresh throws — the setting is already persisted on disk, so the user-visible message should not surface the refresh failure. - Drop two stale "rule file is updated on restart" comments left over from the pre-fix behavior. --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> |
||
|
易良
|
796de4dfef
|
fix(core): merge IDE context into user prompt (#3980)
Some checks are pending
Qwen Code CI / Classify PR (push) Waiting to run
Qwen Code CI / Lint (push) Blocked by required conditions
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Blocked by required conditions
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* fix(core): merge IDE context into user prompt IDE mode now wraps editor context in a <system-reminder> block and prepends it to the current user request instead of inserting a separate user history entry via addHistory(). This preserves the API history turn shape and avoids extra user turns in IDE mode. Key changes: - IDE context merged into user request via prependToFirstTextPart() - State update deferred until after arena cancellation check - escapeClosingSystemReminderTags() hardens against tag injection including zero-width/control character variants - forceFullIdeContext reset on stream errors for correct resend - Context prompt updated to encourage active use of editor context Refs #3712 * fix(core): restore BaseLlmClient per-model cache clear on session reset resetChat only cleared GeminiClient's new perModelGeneratorCache but dropped the BaseLlmClient.clearPerModelGeneratorCache() call that was present before the refactoring. sideQuery.ts and sessionTitle.ts still route through BaseLlmClient with the fast model, so stale generators survived session reset. * refactor(core): remove duplicated per-model generator code from GeminiClient The per-model ContentGenerator resolution logic (resolveModelAcrossAuthTypes, createRetryAuthTypeForModel, createContentGeneratorForModel, perModelGeneratorCache) was inadvertently duplicated from BaseLlmClient into GeminiClient. This restores the original one-line delegation to BaseLlmClient.resolveForModel() and removes ~130 lines of redundant code to keep the PR focused on IDE context merging only. * fix(core): harden IDE context reminder escaping * fix(core): defer IDE context baseline update * fix(core): use shared escapeSystemReminderTags in tool scheduler Aligns the rule-activation envelope scrub with the IDE-context path — both now route through `escapeSystemReminderTags`, which neutralizes whitespace, zero-width, and control-character variants of `<system-reminder>` tag boundaries. The previous narrow regex only matched the literal `</system-reminder>` sequence, so a rule body containing `</system-reminder >`, `</ system-reminder>`, or `</system-reminder>` could still close the envelope mid-content. * docs(core): clarify request assembly order in IDE merge path Two adjacent comments described the pre-merge model: one called the system-reminder block "append" while the code prepends, and the tryCompressChat note still talked about "the previous context turn" which no longer exists once IDE context is merged into the user prompt. Rewrite both to match what the code actually does so future readers do not get a misleading mental model of prompt assembly or post-compression resend behavior. * docs(core): align scheduler scrub comment with shared helper The block-level comment still labeled the sanitization step as "closing-tag scrub", which described the old narrow regex. The shared escapeSystemReminderTags helper now neutralizes opening / self-closing / obfuscated variants too, so name the helper directly to keep the rationale and the call site in agreement. * test(core): cover escapeSystemReminderTags variants in scheduler The end-to-end scheduler scrub test only exercised a literal </system-reminder> body. Now that the rule-activation envelope routes through escapeSystemReminderTags, extend the integration coverage to the obfuscated closing-tag variants the helper was introduced to catch (whitespace before/after the slash, ZWSP / WJ / VS-16 inside the name) and to opening-tag injection. Each case asserts that the envelope still has exactly one </system-reminder> closer and that the raw obfuscated form (or unescaped opening tag) does not survive into the model-facing payload. Refactor the existing test's mock setup into a shared runSchedulerWithRule helper so the new it.each variants stay focused on the assertion shape. * fix(core): address remaining review feedback - Add debug log when IDE context parts are empty for diagnosability - Add safety comment in xml.ts explaining why no fast-path pre-check is used in getSystemReminderTagKind (zero-width obfuscation bypass) |
||
|
ChiGao
|
3cc66f9ae5
|
ci(deps): bump docker/* actions to Node 24 majors (#4131)
GitHub Actions deprecates the Node 20 runtime; older versions of every docker/* action run on Node 20 and emit the "Node.js 20 actions are deprecated" warning in every release / e2e / image-build run today. Each action shipped a "Node 24 as default runtime" major: - docker/setup-buildx-action v3 → v4 (2026-03-05) - docker/setup-qemu-action v3 → v4 - docker/metadata-action v5 → v6 - docker/login-action v3 → v4 - docker/build-push-action v6 → v7 None of our usages touch the deprecated inputs removed in the bumps — release.yml / e2e.yml call setup-buildx with no `with:` block, and build-and-publish-image.yml only passes the universally-supported `images` / `tags` / `registry` / `username` / `password` / `context` / `platforms` / `push` / `labels` / `build-args` inputs. ESM internal refactor of each action is transparent to consumers. Ratchet-pinned bumps use the v4.0.0 commit SHA `4d04d5d9486b7bd6fa91e7baf45bbb4f8b9deedd` (release.yml + e2e.yml). Other action references in build-and-publish-image.yml use `# ratchet:exclude` per existing convention, so version-string bumps suffice there. Verified runtime hosts (`actions/runner` v2.327.1+) are already in use on github-hosted runners as of 2026-03; no infra bump required. Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> |
||
|
顾盼
|
c512427f93
|
feat(core): strip inline media before chat compaction summary (#4101)
* feat(core): strip inline media before chat compaction summary
Compaction's side-query previously shipped historyToCompress verbatim.
Two related issues degraded summary quality and accuracy:
- Inline image / document bytes (from MCP tool results) leaked into the
summary model's prompt where they could not be interpreted and merely
inflated payload.
- findCompressSplitPoint apportioned chars via JSON.stringify(content),
so a single 1 MB base64 image looked like ~350K tokens and biased
the split point. Real Qwen-VL token cost is at most a few thousand.
This change adds a new compactionInputSlimming module that replaces
inlineData / fileData parts with short [image: <mime>] / [document:
<mime>] placeholders before the side-query, leaving live history
unchanged. The same constant feeds estimateContentChars so the
split-point algorithm sees the budget the summary model actually
consumes downstream. Microcompact is also extended to clear stale
inline images alongside old tool results.
A previous draft of the design also externalized large pastes to a
content-addressable on-disk cache, but it was withdrawn after surveying
claude-code's 2026-03 to 2026-05 releases - upstream consensus is to
keep user input visible to the model and amortize cost via prompt
caching rather than externalize. See the Out-of-scope section of the
design doc for the full rationale.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(core): recurse into functionResponse.parts when stripping media
E2E exposed that `read_file` (and any tool that surfaces an image)
wraps the result in `functionResponse.parts` via
`coreToolScheduler.createFunctionResponsePart`. The slimming module
only walked top-level `part.inlineData` / `part.fileData`, so the
nested base64 bytes leaked into the compaction side-query payload.
The previous design doc incorrectly claimed that no recursive walk
was needed.
Three changes:
- `slimCompactionInput.transformPart` recurses into the nested
`functionResponse.parts` array and replaces each entry via the
same image/document placeholder logic.
- `estimatePartChars` walks the nested array too, so the split-point
algorithm doesn't fall back to `JSON.stringify` and over-count the
base64 bytes.
- `microcompactHistory` drops `functionResponse.parts` when clearing
an old tool result; the previous spread of `...part.functionResponse`
silently carried the original media through.
New unit tests cover (a) nested image / document stripping, (b) the
estimator no longer being skewed by nested base64. The previously
failing E2E now PASSES: side-query payload contains zero `data:image/`
occurrences, zero long base64 runs, and exactly one
`[image: image/png]` placeholder.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(core): address review findings on compaction image stripping
Addresses 8 valid findings from PR review:
- [Critical] estimatePartTokens now handles `fileData` parts (both
top-level and nested under functionResponse.parts). Without this,
microcompact's `tokensSaved === 0` short-circuit silently discarded
every fileData clear.
- estimatePartTokens for binary parts now uses a fixed
MEDIA_PART_TOKEN_ESTIMATE constant (1,600) instead of base64-length
divided by 4. The old formula billed a 1 MB image as ~250K tokens
rather than its actual ~1,280 visual tokens on Qwen-VL, inflating
the saved-token metric by orders of magnitude.
- mimeType values from MCP tool servers are now run through
sanitizeMimeForPlaceholder before being embedded in `[image: …]` /
`[document: …]` placeholders. An adversarial server could otherwise
craft `image/png]\n\n[SYSTEM: …` and inject instructions into the
summary side-query.
- collectCompactablePartRefs now recognizes a third 'nested-media'
kind: functionResponse parts from non-compactable tools (e.g. MCP
screenshots whose names aren't in COMPACTABLE_TOOLS) that carry
images on functionResponse.parts. The nested media is dropped while
the tool's text output is preserved. Previously such media
accumulated forever in live history.
- keepRecent budgets are now per-kind (tool / media / nested-media).
Setting `toolResultsNumToKeep: 1` keeps 1 of each kind rather than 1
entry total across the merged list — matches the natural reading of
the setting name.
- findCompressSplitPoint's `precomputedCharCounts` fallback path is
now documented as test-only; production callers MUST pass the
precomputed array.
- The text-based branch of isAlreadyCleared is gone: with the new
nested-media handling (drops `parts`) and existing media handling
(replaces with `{ text: … }` that is no longer collected) it was
unreachable.
- OpenAI converter (createToolMessage) now passes text parts inside
functionResponse.parts through as text content. The slimmer writes
`{ text: '[image: image/png]' }` placeholders into the nested array;
without this fix the converter dropped them when serializing to the
OpenAI wire format, leaving the summary model with empty tool
responses instead of the placeholder.
Two findings deferred with rationale (see design doc Open Questions):
MIN_COMPRESSION_FRACTION still uses pre-slim counts (acceptable —
"user shared an image" is itself worth summarizing); SlimResult is not
re-exported (round-3 simplify decided to keep core's public surface
minimal).
E2E re-verified end-to-end: side-query payload contains 0 data:image/
occurrences, 0 long base64 runs, and 1 `[image: image/png]` placeholder
in the expected position. 185/185 collocated unit tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(core): tidy compaction slimming after self-review
Three small polishes from a follow-up code review pass:
- `stripNestedMedia` no longer re-casts its return value: after
destructuring `parts` out of the widened input type, TypeScript
infers the original `FunctionResponse` shape without help.
- `isAlreadyCleared` shed a 10-line comment block — the body is now
one line, so one descriptive line above it is enough.
- OpenAI converter's nested-part text check switched from
`(part as { text?: unknown }).text` to
`'text' in part && typeof part.text === 'string'`, dropping the
cast and letting `in` narrow the type.
No behavior change. 185/185 unit tests still pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(core): wire slim stats to debug log; split MicrocompactMeta tools vs media
Addresses two follow-up review suggestions:
- `slimCompactionInput` returned `stats.imagesStripped` and
`stats.documentsStripped` but the orchestrator never consumed them.
Now logged at debug level whenever non-zero so operators can confirm
the slimming pipeline actually fires on image-heavy compactions.
- `MicrocompactMeta.toolsCleared` lost meaning after the recent
refactor: it had grown to count both tool-result clears AND
inline-media / nested-media clears. Renamed:
- `toolsCleared` → only `tool`-kind clears (compactable tool output)
- `mediaCleared` → `media` + `nested-media` clears (new)
- `toolsKept` / `mediaKept` mirror the split, replacing the prior
`toolsKept` that was actually a combined count.
The single non-test consumer (`client.ts` debug log) updated to use
both fields.
185/185 unit tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
tanzhenxin
|
f72a156e13
|
fix(anthropic): allow cache_control on tool_result blocks so the per-turn breakpoint advances (#4121)
The Anthropic adapter places three cache_control breakpoints (system, last tool, last user message), but `addCacheControlToMessages` only attached the third when the last block of the last user message was a non-empty text block. After turn 1 of any agentic conversation, the last user message is a tool_result, so the breakpoint was silently dropped and the cacheable region collapsed back to system+tools. Per-turn history was never cached. Anthropic's docs explicitly list tool_result as a cacheable block type (https://docs.claude.com/en/docs/build-with-claude/prompt-caching). Accepting both text and tool_result keeps the breakpoint moving forward as the conversation grows. |