Commit graph

155 commits

Author SHA1 Message Date
Thibault Jaigu
a6e206c887
feat(core): add Requesty provider (#5478)
* feat(core): add Requesty provider

Requesty (https://requesty.ai) is an OpenAI-compatible model gateway that uses
the same provider/model identifier format as OpenRouter, so it is added by
mirroring the existing OpenRouter provider.

- RequestyOpenAICompatibleProvider + isRequestyProvider detection, mirroring
  the OpenRouter provider (base https://router.requesty.ai/v1, attribution headers)
- register in the openaiContentGenerator dispatch + provider preset registry
- auth migration entry + docs (auth, model-providers)

Signed-off-by: Thibault Jaigu <thibault.jaigu@gmail.com>

* fix(core): harden Requesty provider hostname detection

Address review feedback on isRequestyProvider:

- Replace substring matching (baseURL.includes) with URL-parsed hostname
  detection (host === 'router.requesty.ai' || host.endsWith('.requesty.ai')),
  matching the ownsModel gate in presets/requesty.ts and the MiMo/MiniMax/
  Mistral providers. Rejects crafted URLs like router.requesty.ai.evil.com.
- Add hostile-hostname rejection tests and determineProvider dispatch tests,
  matching the coverage in the other provider suites.

---------

Signed-off-by: Thibault Jaigu <thibault.jaigu@gmail.com>
2026-06-21 06:37:24 +08:00
tt-a1i
977313b5ae
fix(cli): parse force hyperlink override strictly (#5489) 2026-06-21 01:48:27 +08:00
Gove
0ba245ea3d
feat(cli): add persistent history collapse on resume with refined commands (#4085)
* feat(cli): add --quiet-restore flag to suppress history output on session resume

* fix: preserve history state for /rewind while suppressing rendering

* refactor: model quiet-restore as display policy with shared utilities

* refactor: replace --quiet-restore with /history collapse|expand slash command

* fix: persist history collapse state as user setting

* fix(cli): address maintainer feedback on history collapse persistence and i18n

* test(cli): fix TypeScript compilation errors in historyCommand tests

* fix(cli): address maintainer review feedback on history collapse

* test: fix act() warning in slashCommandProcessor.test.ts

* fix: make applyCollapsePolicyAndSummary pure to avoid React batching bug

* chore: revert unrelated changes to lockfile and NOTICES.txt

* test: verify isRealUserTurn handles suppressOnRestore items correctly

* wip(cli): preserve local history review fixes before redesign

* feat(cli): refine history resume collapse commands

* fix(cli): address maintainer review feedback on history collapse

* test(cli): cover cold-boot collapsed resume

* fix(cli): address reviewer feedback on history collapse

* fix(cli): resolve rebase conflicts and missing imports

* fix(cli): strip suppressOnRestore in handleRewindConfirm

* fix(i18n): add Chinese translations for history collapse commands

* fix: address wenshao review comments on PR #4085

- Restore restoreGoalFromHistory call in cold-boot resume path
- Extract stripSuppressOnRestore to shared utility in resumeHistoryUtils
- Add comment explaining historyRef pattern in slashCommandProcessor
- Use historyCommand.name constant instead of string literal
- Add missing i18n translation for collapse summary message
- Fix pluralization in createHistoryCollapseSummaryItem

* fix(i18n): add missing English translations for history collapse commands

* fix: address wenshao follow-up review comments

- Filter out collapse-summary items in rewind path (AppContainer.tsx)
- Show info messages for collapse-on-resume/expand-on-resume commands (slashCommandProcessor.ts)
- Use visibleHistory instead of uiState.history in summaryByCallId useMemo (MainContent.tsx)
- Remove dead hasHistoryManager guard and optional chaining (useResumeCommand.ts)

* fix: restore optional chaining for remount in useResumeCommand

* test: update slashCommandProcessor tests for history command feedback changes

* chore: remove generated artifact files from branch

- Remove .learnings/LEARNINGS.md (local workflow artifact)
- Remove .pr-body.md (PR description draft)
- Remove build_output.log (build log)
- Remove vscode_test_output.log (test output log)

These files are unrelated to the history-collapse feature and were causing
git diff --check whitespace errors.

* fix: address wenshao review comments on PR #4085

- Remove stray [!tip] file from repo root
- Add selfManaged flag to MessageActionReturn for explicit UI feedback control
- Fix expand-now to return load_history type instead of calling loadHistory directly
- Replace hardcoded isSelfManaged path check with result.selfManaged in processor
- Fix MainContent merge detection to use visibleHistory.length (avoid flicker)
- Refactor applyCollapsePolicyAndSummary to not mutate input array
- Extract expandCollapsedHistory shared helper
- Restore dialog:memory test in slashCommandProcessor.test.ts
- Add stripSuppressOnRestore dedicated tests
- Add visibleHistory filtering test in MainContent.test.tsx
- Update historyCommand tests for new load_history return type
- Fix eslint errors: remove unused imports and fix dependency array

* fix: address wenshao review comments on PR #4085

- Remove stray [!tip] file from repo root
- Add selfManaged flag to MessageActionReturn for explicit UI feedback control
- Fix expand-now to return load_history type instead of calling loadHistory directly
- Replace hardcoded isSelfManaged path check with result.selfManaged in processor
- Fix MainContent merge detection to use visibleHistory.length (avoid flicker)
- Refactor applyCollapsePolicyAndSummary to not mutate input array
- Extract expandCollapsedHistory shared helper
- Restore dialog:memory test in slashCommandProcessor.test.ts
- Add stripSuppressOnRestore dedicated tests
- Add visibleHistory filtering test in MainContent.test.tsx
- Update historyCommand tests for new load_history return type
- Fix eslint errors: remove unused imports and fix dependency array
- Fix TypeScript errors: add useEffect import, fix display.kind type assertions

* fix: add braces around if statement body (eslint curly rule)

Fixes lint error in editorGroupUtils.ts:
- Expected { after 'if' condition on line 32

* fix: address wenshao follow-up review comments on PR #4085

- Revert expand-now to use loadHistory/refreshStatic directly (no load_history return)
- Remove dead selfManaged flag from MessageActionReturn and processor
- Fix visibleHistory filter to also exclude collapse-summary items
- Simplify applyCollapsePolicyAndSummary (return rawItems when !collapseOnResume)
- Fix test assertion shapes (content→text, timestamp as separate arg)
- Add mockClient for expand-now test
- Update historyCommand tests for new behavior
- Add expandCollapsedHistory dedicated tests
- Fix MainContent filtering test to use historyItemDisplayPropsSpy

* fix: address wenshao latest review comments on PR #4085

- Fix visibleHistory filter: remove collapse-summary exclusion (summary should render when all items suppressed)
- Update MainContent test: assert summary item IS rendered alongside unsuppressed items
- Remove dead selfManaged flag from MessageActionReturn type
- Remove dead selfManaged check from slashCommandProcessor.ts
- Add remount?.() to useResumeCommand error path
- Remove unused 'History expanded.' translation key from en.js, zh.js, zh-TW.js
- Fix expand-now test: mock action to return undefined (matching real behavior)

* fix: address wenshao latest review comments on PR #4085

- Add missing i18n translations to ca.js, de.js, fr.js, ja.js, pt.js, ru.js
- Simplify applyResumeDisplayPolicy: remove dead options parameter
- Fix expand-now test: mock action to return undefined (matches real behavior)
- Revert unrelated changes to package-lock.json and editorGroupUtils.ts

* fix: address wenshao latest review comments

- Add openDiffDialog to createMockActions() in slashCommandProcessor.test.ts
- Add sentToModel: false to user message assertions in slashCommandProcessor.test.ts
- Fix useResumeCommand.test.ts mock: spread original @qwen-code/qwen-code-core exports to include createDebugLogger
- Remove dead optional chaining (addItem?., clearItems?., loadHistory?.) in useResumeCommand.ts
- Simplify if (!config || !startNewSession) to if (!config) since startNewSession is required
- Fix truncatedCount off-by-one in AppContainer.tsx rewind path: exclude collapse-summary from effective length
- Apply collapse policy (applyCollapsePolicyAndSummary) in useBranchCommand.ts for /branch
- Add settings to UseBranchCommandOptions with proper useCallback dependency

* fix: add missing mockUpdateItem arg to test renderHook calls, remove stray file

- Add mockUpdateItem (17th arg) to resume-direct and memory dialog test calls
- Remove accidentally committed packages/core/.qwen/computer-use/installed.json
- Add .qwen/computer-use/installed.json to .gitignore

* fix: address review comments (type, gitignore, test, split-brain)

1. UIActionsContext: handleResume return type → Promise<void>
2. .gitignore: split corrupted merged line into .codegraph + .qwen/...
3. useBranchCommand.test: add settings to makeOptions(), add collapseOnResume test
4. useResumeCommand: reorder core-before-UI with rollback (matches branch pattern)
5. useResumeCommand.test: add getSessionId to mocks, add rollback test

* fix: complete history collapse translations in 6 locale files

- Added missing translation text for 'History collapsed' message
- Fixed syntax errors in de, fr, ja, pt, ru, zh-TW locale files

* fix: add missing history parameter in slashCommandProcessor test

- Fixed parameter order in 'should skip reload when consumeSlashReloadSuppression' test
- Added missing empty array for history parameter
- All 58 tests now pass

* merge: resolve conflicts with upstream main

- docs: keep ui.history.collapseOnResume setting, adopt upstream showCitations default
- fix: update rewindRecording call to include file history snapshots parameter

* fix(cli): repair history collapse CI failures

---------

Co-authored-by: qqqys <qys177@gmail.com>
2026-06-19 19:22:48 +00:00
tt-a1i
b773b895c2
feat(cli): show optional response token rate (#5401) 2026-06-19 17:39:59 +08:00
tt-a1i
0430ff7af4
fix(openai): add string tool result compatibility mode (#5399)
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
Qwen Code CI / Integration Tests (CLI, No Sandbox) (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
2026-06-19 16:56:49 +08:00
MikeWang0316tw
26ad36e95b
feat(cli): show follow-up suggestion in input placeholder (#5145)
* feat(cli): show follow-up suggestion in input placeholder

When enableFollowupSuggestions is true, display the generated
follow-up suggestion as the input placeholder text (replacing
the default "Type your message..."). Tab/Enter/Right arrow
accepts the suggestion; typing dismisses it.

Also change the default of enableFollowupSuggestions from false
to true so the feature is on by default.

Key changes:
- AppContainer: dismissPromptSuggestion no longer clears
  promptSuggestion state, preserving it for placeholder restore
  after user types then deletes
- InputPrompt: Tab/Enter/Right arrow/typing handlers check
  promptSuggestion prop as fallback when followup.state is not
  visible (e.g. after 300ms delay or user dismissed)
- Composer: placeholder shows suggestion text when available
- hasTabConsumer: include promptSuggestion to prevent Windows
  bare Tab from cycling approval mode

* chore: update settings.schema.json (enableFollowupSuggestions default: false → true)

* test(cli): add tests for promptSuggestion prop fallback paths (#5145)

- Add unit tests for Tab/Right arrow/Enter accepting promptSuggestion
  when followup.state.suggestion is null (type-then-delete path).
- Add unit test for hasTabConsumer reporting true immediately when
  promptSuggestion prop is set (no followup debounce needed).
- Update stale comment on speculation abort useEffect in AppContainer.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): address PR #5145 review feedback for promptSuggestion

- Fix Enter key to fill buffer instead of submitting suggestion (matches
  Tab/Right-arrow behavior and Claude Code design)
- Add suggestionDismissed state to hasTabConsumer for Windows Tab cycling
- Fix suggestionDismissed to be set to true on user input (paste/typing)
- Add speculation abort to dismissPromptSuggestion callback
- Remove dead placeholder branch from Composer.tsx
- Update tests to reflect Enter no longer auto-submits suggestion

* fix(cli): address PR #5145 review from wenshao + telemetry gap

wenshao's review (posted after the previous fixes) flagged two issues,
both still valid against the current code; doudouOUC's telemetry gap
is addressed too.

- settings description: replace stale "Enter to accept and submit" with
  "Press Tab, Right Arrow, or Enter to accept into the input buffer" in
  both settingsSchema.ts and settings.schema.json (Enter now only fills
  the buffer, and the feature defaults to enabled).

- hasTabConsumer / handler consistency: drop the redundant
  `suggestionDismissed` state and gate hasTabConsumer on
  `buffer.text.length === 0` — the exact condition the Tab/Right/Enter
  handlers already use. Fixes the type-then-delete desync where Windows
  bare Tab would both insert the suggestion and cycle approval mode
  (regression of #4171).

- fallback telemetry: add a `fallbackText` option to the followup
  controller's accept() so the prop-fallback path (no live suggestion,
  e.g. within the show delay or after type-then-delete) routes through
  accept() and logs onOutcome instead of silently bypassing telemetry.
  Tab/Right/Enter handlers now call accept(method, { fallbackText }).

- tests: add core-level coverage for accept() with/without fallbackText,
  and fix the InputPrompt "fallback" tests that advanced 700ms (which
  silently exercised the normal visible-suggestion path) to advance only
  100ms so followup.state.suggestion truly stays null.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(cli): add accept_source telemetry + tests for promptSuggestion fallback

Follow-up to wenshao's second review pass on #5145.

- accept_source telemetry: fallback accepts report time_to_accept_ms: 0
  (the suggestion was never shown via the timer), which is indistinguishable
  from an instant accept. Add an `accept_source: 'live' | 'fallback'` field to
  the followup controller's onOutcome and PromptSuggestionEvent so analytics
  can tell the two apart. The controller derives it from whether a live
  `currentState.suggestion` was present before applying `fallbackText`.

- tests: assert accept_source on the fallback accept; add a test that a live
  suggestion takes priority over fallbackText (guards the `?? fallbackText`
  ordering); add an InputPrompt test pinning the new buffer.text.length === 0
  gate — hasTabConsumer reports false when a promptSuggestion is set but the
  buffer is non-empty (the old Boolean(promptSuggestion) gate wrongly reported
  true). The empty-buffer → true direction stays covered by the existing test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(cli): address doudouOUC review on #5145 (dedupe, rename, telemetry)

Three [Suggestion]-level items from the latest review pass.

- Extract `availableSuggestion`: the compound condition
  `(followup.state.isVisible || promptSuggestion) && (followup.state.suggestion ?? promptSuggestion)`
  was copy-pasted across the Tab/Right/Enter accept guards, both
  typing-dismiss guards, and the placeholder prop. Collapse them into one
  derived value so the sites can't drift apart. Behavior is unchanged
  (the controller keeps `isVisible` and `suggestion` in lockstep).

- Rename `dismissPromptSuggestion` -> `abortPromptSuggestion` across the
  UIState context, AppContainer, Composer, and the MainContent mock. The
  function only aborts in-flight generation/speculation and deliberately
  does NOT clear `promptSuggestion` (so the placeholder can restore it);
  the "dismiss" name implied the suggestion was gone.

- Omit `time_to_first_keystroke_ms` for fallback accepts. With
  `accept_source: 'fallback'` the suggestion was never shown via the timer
  (shownAt stayed 0), so `prevShownAtRef` still holds a previous
  suggestion's timestamp and the delta would be meaningless.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(cli): actually enable followup suggestions by default

PR #5145 changed the schema default to `true`, but `mergeSettings` never
applies SETTINGS_SCHEMA defaults, so the runtime `=== true` gates left the
feature off while the settings panel read it as on (verified by wenshao).

- Flip both runtime gates to treat an unset value as enabled — only an
  explicit `false` opts out: `AppContainer.tsx` and the ACP `Session.ts`
  (`#maybeEmitFollowupSuggestion`).
- Add a Session test for the unset/default-on path.
- Fix the stale `UIStateContext` JSDoc left over from the dismiss→abort
  rename (it no longer clears state).
- Docs: mark the feature on-by-default, correct Enter (fills the input,
  does not submit), ghost-text → placeholder text, and add a cost note that
  `fastModel` forks to a separate cache and can cost more than the default
  main-model + shared-cache path on long conversations.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(core): reject control chars and ANSI escapes in prompt suggestions

The follow-up suggestion is influenceable through conversation history
(tool/file/web output) and is rendered verbatim in the input placeholder
now that enableFollowupSuggestions defaults to on. Raw control bytes (CR,
ESC/CSI, C1) reached the terminal because getFilterReason only rejected
newlines and asterisks. Reject them at the source so the displayed and
inserted text always match.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(cli): clear promptSuggestion on submit and accept paths

Addresses doudouOUC review on #5145. Since abortPromptSuggestion was
changed to preserve `promptSuggestion` for type-then-delete restore, the
submit and accept paths leaked stale suggestion text:

- handleSubmitAndClear only called followup.dismiss(); after a synchronous
  command (/clear, /help) that never triggers AppContainer's streaming
  transition, the placeholder kept showing the old suggestion.
- Tab/Right/Enter accept never cleared the prop, so clearing the buffer
  without submitting (Ctrl+U) made the accepted suggestion reappear as a
  ghost placeholder.

Both now call onPromptSuggestionDismiss?.() after the followup action. Also
reuse the availableSuggestion single-source-of-truth in hasTabConsumer
instead of an inlined parallel expression, and add useFollowupSuggestions
tests asserting the accept_source guard suppresses time_to_first_keystroke_ms
on fallback accepts.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(cli): assert promptSuggestion is cleared on accept and submit

Regression coverage for the state-leak fixed in 04fcffd1c (doudouOUC
Critical #1/#2, confirmed by wenshao's maintainer re-verification): Tab,
Right-arrow and Enter accepts plus message submit must each call
onPromptSuggestionDismiss, so the persisted promptSuggestion can't reappear
as a ghost placeholder when the buffer is next cleared.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-19 13:39:12 +08:00
Dragon
ad6368b3ae
docs: fix stale defaults, CLI syntax, and tool naming drift (#5158)
- common-workflow.md: fix --p flag syntax (→ -p) and --print (→ -p/--prompt)
- settings.md: fix ui.theme default (undefined → "Qwen Dark"),
  skipNextSpeakerCheck (false → true), enableInteractiveShell (false → true),
  add auto approval mode to settings and CLI flag tables
- keyboard-shortcuts.md: fix Shift+Tab cycling (add auto mode), correct
  newline keybinding (Ctrl+Enter/Cmd+Enter/Shift+Enter/Ctrl+J)
- model-providers.md: fix stale codingPlan.region reference → modelProviders
- developers/tools: rename task → agent tool, remove stale save_memory and
  read_many_files references, fix todo_write activeForm → id field
2026-06-15 20:06:34 +08:00
jinye
57a90f7302
fix(core): Bound active tool result history (#5111)
* fix(core): bound active tool result history

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(core): keep tool result budget defaults lightweight

Move the new tool-result history budget default into a lightweight config defaults module so microcompaction does not load the full Config graph during service tests. Update the ACP worktree test mock to include the public default export used by settings schema imports.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(core): address tool result budget review

Handle negative legacy idle thresholds consistently, clarify size compaction diagnostics for pending tool results, promote successful microcompaction logs to info, and strengthen tests/docs around skipped results and soft thresholds.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(core): log protected tool result overages

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-06-15 15:29:12 +08:00
顾盼
bb1e71911c
feat(computer-use): configurable screenshot max dimension (setting + env) (#5122)
* feat(computer-use): configurable screenshot max dimension (setting + env)

Add a user-level knob for cua-driver's screenshot longest-edge cap. The
old open-computer-use backend exposed this via OPEN_COMPUTER_USE_IMAGE_*
env vars; the cua-driver migration dropped them, leaving only the
model-driven set_config tool. This restores deterministic user control.

- Setting tools.computerUse.maxImageDimension (number; default -1 = keep
  cua-driver's built-in default of 1568; 0 disables resizing / full
  resolution; a positive value caps the longest edge).
- Env override QWEN_COMPUTER_USE_MAX_IMAGE_DIMENSION (takes precedence
  over the setting; invalid/negative values fall through).
- Resolution lives in resolveMaxImageDimension(); applied via the
  cua-driver set_config tool once per (re)connect in
  ComputerUseClient.doStart — best-effort, never aborts startup, and
  re-applied after a daemon-restart reconnect.
- Docs: document tools.computerUse.{enabled,maxImageDimension} in
  settings.md (the block was previously undocumented). Refresh stale
  ocu/npx comments left in client.ts + install-state.ts by the migration.

Precedence: env var > setting > cua-driver default.

* chore(computer-use): finish ocu→cua-driver cleanup in schema-sync script

The cua-driver migration (#5051) left scripts/sync-computer-use-schemas.ts
pointing at the old open-computer-use backend: it npx'd
@qwen-code/open-computer-use, hard-coded the 9-tool ocu surface, and emitted an
"open-computer-use" header. Re-running it — which constants.ts' version-bump
procedure tells maintainers to do — would have clobbered the migrated 35-tool
cua-driver schemas.ts.

- Drive the locally-pinned `cua-driver mcp` binary (binaryPath /
  CUA_DRIVER_VERSION from constants.ts) instead of npx'ing ocu; expect 35
  tools and warn (don't fail) on drift.
- Emit the cua-driver-flavored schemas.ts header.
- Refresh install-state.test.ts fixtures from ocu package specs to the
  cua-driver-rs approval-key form the field actually stores now.

Verified the fixed script reproduces the committed 35-tool surface exactly
(modulo prettier formatting). No dead env-var handling remained — the module
reads only QWEN_COMPUTER_USE_{AUTO_APPROVE,DOWNLOAD_HOST,MAX_IMAGE_DIMENSION}.
2026-06-15 15:25:27 +08:00
Dragon
7cb95bebbd
fix(docs): update Coding Plan model list and fix stale references in developer docs (#5054)
* fix(docs): update Coding Plan model list and fix stale references in developer docs

- model-providers.md: expand Coding Plan model table from 3 to 9 models (add qwen3.6-plus, glm-5, kimi-k2.5, MiniMax-M2.5, qwen3-coder-next, glm-4.7) to match the provider registry
- auth.md: update Coding Plan model list to match all 9 models and use the correct date-stamped model ID (qwen3-max-2026-01-23)
- contributing.md: fix Node.js version requirement from 18+ to 22+ to match package.json engines
- sdk-typescript.md: fix nonexistent tool name run_terminal_cmd → run_shell_command
- integration-tests.md: fix file extension from .test.js to .test.ts to match actual test files
- qwen-serve-protocol.md: fix legacy tool name search_file_content → grep_search and remove nonexistent ripgrep tool reference

* fix(docs): add missing qwen3.7-plus to Coding Plan model docs

MODELSTUDIO_MODELS in alibaba-coding-plan.ts defines 10 models, but the
inline list in auth.md and the table in model-providers.md only listed 9,
omitting qwen3.7-plus (1M context, thinking enabled). Add it after
qwen3.6-plus in both, matching the source order.
2026-06-13 01:34:14 +08:00
Dragon
546b2758fb
fix(docs): correct stale settings keys, wrong defaults, and missing commands (#4969) 2026-06-12 14:31:31 +08:00
顾盼
240c99c186
fix(openai): default splitToolMedia so tool-returned images reach strict backends (#4917)
OpenAI Chat Completions only permits text on `role:"tool"` messages, so an image
read via read_file — the only image path available to a subagent — was embedded
there and silently dropped by strict OpenAI-compatible backends (doubao /
new-api / LM Studio). The model never saw the image and returned content
unrelated to it (#4876). Permissive backends (e.g. DashScope) happen to parse
it, which is why the same model worked for the main agent via @-image
(role:"user") but not for the subagent via read_file (role:"tool").

Flip the runtime default of splitToolMedia to true so tool-returned media is
lifted into a follow-up role:"user" message — spec-compliant and visible to all
backends. Opt out via generationConfig.splitToolMedia = false.

Also:
- modalityDefaults: recognize ByteDance Doubao (Seed chat + *vision/*vl => image;
  seedance/seedream generation models => text-only).
- settingsSchema + docs: default true, description corrected to cover the
  built-in read_file (not only MCP tools).

Tests: pipeline default-true regression, modalityDefaults doubao cases, converter
opt-out wording.
2026-06-11 04:51:14 +08:00
jinye
9e4c87a7e4
refactor(core): remove GitService, migrate /restore to FileHistoryService (#4871)
* refactor(core): remove GitService, migrate /restore to FileHistoryService

Remove the shadow-git-based GitService and rewire /restore to use the
existing FileHistoryService for file restoration. This eliminates the
`checkpointing` config flag (off by default) and unifies file recovery
under `fileCheckpointingEnabled` (on by default in interactive mode).

Key changes:
- /restore now calls FileHistoryService.rewind(promptId, true) instead
  of GitService.restoreProjectFromSnapshot(commitHash)
- File restoration runs before conversation history replacement to
  avoid inconsistent state on failure
- Legacy checkpoint files (commitHash format) are explicitly rejected
- Fix EDIT_TOOL_NAMES bug: 'replace' → ToolNames.EDIT, add
  ToolNames.NOTEBOOK_EDIT (checkpoint creation and AUTO_EDIT
  auto-approval were broken for edit tool)
- Add isClientInitiated guard to prevent redundant checkpoint creation
  from /restore re-submitted tool calls
- Remove checkpointing settings schema, CLI flag, docs, and all
  GitService references across 27 files

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* fix: address wenshao review — improve error message, add tests, remove tombstones

- Improve partial-restore warning: show files reverted/failed count
- Add 3 tests: legacy format rejection, rewind partial failure, rewind exception
- Remove dead tombstone comments in config.test.ts

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* fix: align restore success message with turn-level semantics

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
2026-06-09 18:34:31 +08:00
Edenman
6f6b326d63
docs: add /diff command and auto theme detection documentation (#4699)
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
* docs: add /diff command documentation to commands.md

Add section 1.8 documenting the /diff interactive diff viewer,
including source picker (Current + per-turn diffs), keyboard
shortcuts, dialog example, and non-interactive mode output format.
Also add /diff entry to the 1.2 Interface and Workspace Control table.

* docs: add auto theme detection section to themes.md

Document the 'auto' theme setting and its detection fallback chain
(COLORFGBG → OSC 11 → macOS system appearance → default dark),
including notes for tmux/SSH environments.

* docs: fix checkpointing default description in /diff section

Checkpointing defaults to false, not true. Updated from
"on by default" to "disabled by default" per reviewer feedback.

* docs: fix file checkpointing default in /diff section

File checkpointing (used by per-turn diffs and /rewind) defaults to
enabled in interactive mode. Session checkpointing (/restore) is the
one that defaults to disabled. Corrected the description accordingly.

---------

Co-authored-by: 克竟 <dingbingzhi.dbz@alibaba-inc.com>
2026-06-02 21:28:34 +08:00
jinye
13f37dc9f0
feat(cli): background housekeeping for stale file-history dirs (#4414)
PR #4064 introduced ~/.qwen/file-history/{sessionId}/ for /rewind but had
no cross-session cleanup — directories accumulated indefinitely. This adds
a generic background housekeeping framework with file-history cleanup as
its first user.

- 30-day mtime sweep, configurable via general.cleanupPeriodDays
- 10-min startup delay (1-min catch-up if last run >7d ago)
- 24h recurring cadence, idle-gated (defers if user typed in last 1 min)
- O_EXCL lockfile + marker mtime throttle (multi-process safe)
- Current session whitelisted via lazy config.getSessionId() — defends
  against long-idle active sessions and /clear minting a new session
- Negative cleanupPeriodDays values clamp to 1h minimum (defends against
  schema-bypass: a future cutoff would otherwise sweep everything)
- Zero new prod dependencies; ~70 lines of self-written O_EXCL throttle
  primitive in lieu of proper-lockfile (which pulls graceful-fs and
  monkey-patches every fs method on first require)
- All setTimeout(...).unref() — never blocks process exit

Closes #4173.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
2026-06-02 14:36:41 +08:00
yao
2d8052b02c
feat(cli): add respectUserColors and hideContextIndicator options for statusline (#4670)
* feat(cli): add respectUserColors option to preserve ANSI colors in
     statusline command output

* test(cli): add respectUserColors tests for useStatusLine and Footer

* feat(cli): add hideContextIndicator option to hide built-in context usage in footer

* docs: update statusline configuration docs with respectUserColors and hideContextIndicator
2026-06-02 09:31:56 +08:00
Dragon
59c283670e
Hide internal docs from docs site (#4357) 2026-06-01 15:55:14 +08:00
顾盼
365409366d
refactor(core)!: replace tail-preservation compaction with summary + restoration attachments (#4599)
* refactor(core): rewrite compression prompt to 9-section claude-code-style format

Replaces the <state_snapshot> XML template with a numbered 9-section
structure that mandates verbatim preservation of user messages, including
the historical chronological list (section 6). The new format is
designed to pair with post-compact file/image restoration (separate work)
so the agent can resume long single-turn tasks without losing intent.

* refactor(core): align compaction trigger string with new 9-section prompt

The user-turn trigger injected after the system prompt still said
'generate the <state_snapshot>' from the old XML prompt era. Updated to
'produce the 9-section summary' to match Task 1's new prompt format.

Also tightens the prompt test to assert the specific user-message
verbatim mandate (not just the word 'verbatim' anywhere) so a future
regression that drops the mandate won't silently pass.

* feat(core): add postCompactAttachments module with file path extractor

extractRecentFilePaths walks history newest-first and returns the top N
unique file paths touched by read_file/write_file/edit/replace tool calls.
Pure function, no side effects, no state cache — readiness for the next
compaction-rewrite tasks.

* refactor(core): simplify extractRecentFilePaths internals

Three small cleanups from code review:
- Map<string, number> -> Set<string> (the index value was never read)
- Guard against maxFiles <= 0 explicitly (avoids returning 1 result
  when caller passes 0 as a 'disable' sentinel)
- Document 'replace' as a legacy alias for 'edit' so a future cleanup
  pass does not delete it as apparent dead code

Adds one test covering the maxFiles=0 path.

* feat(core): add image extractor with source-tool metadata

extractRecentImages walks history newest-first, collects up to N image
inlineData parts, and attributes each one to the model+functionCall that
preceded it (when one exists). Returns chronological order so callers
can render a meaningful 'last visual state ends here' strip.

* feat(core): add size-adaptive file reader for post-compact restore

readFileSizeAdaptive reads a file and returns one of: embed (full content
for files ≤ maxTokens × 4 chars), reference (path-only for large files),
missing (deleted since last touch), or binary (non-text content). The
embed/reference distinction mirrors claude-code's compact_file_reference
vs file attachment behavior, but without introducing new message types.

* refactor(core): harden readFileSizeAdaptive size accounting

Three corrections from code review:
- Import CHARS_PER_TOKEN from tokenEstimation.ts (canonical) instead of
  redeclaring locally, preventing silent drift between modules.
- Compare decoded character length, not raw byte length, against the
  cap. Otherwise a 10k-char Chinese file would be ~30k bytes and would
  be mis-classified as 'reference' despite fitting the budget.
- Rename FileReadResult -> FileEmbedResult to avoid a name collision
  with the unrelated FileReadResult interface in fileUtils.ts.

Adds a CJK-text test that catches the byte/char regression.

* feat(core): add file restoration block composer

buildFileRestorationBlocks reads each candidate file, classifies it as
embed/reference/missing/binary, and emits one consolidated reference
block (path-only list) plus one user message per embedded small file.
Total embed size is capped at POST_COMPACT_TOKEN_BUDGET; over-budget
files downgrade to reference.

* test(core): make budget test actually exercise the downgrade path

The previous version of this test wrote 3 files totalling 9k chars
against a 200k char budget. The assertions trivially passed regardless
of whether the budget check existed in the implementation.

The new version writes 11 files of 20k chars (each at the per-file cap)
so the budget is exhausted by the 10th and the 11th must downgrade
from embed to reference. Asserts both: file 11 appears in the reference
block, and file 11's content does NOT appear in any embed block.

* feat(core): add image restoration block composer

buildImageRestorationBlock emits a single user message whose first part
is a metadata header (turn index + source tool name + args per image),
followed by the inlineData parts themselves. Handles user-paste images
(no source tool) by labeling them as 'user-provided'.

* feat(core): add composePostCompactHistory orchestrator

Assembles the full post-compact history in order:
  summary → model ack → file references → file embeds → image block.

Each section is built by the per-concern extractors and builders added
in previous tasks. This is the single integration point that
chatCompressionService.compress() will call once the wire-up task lands.

* feat(core)!: rewrite compress() to claude-code-style full-history model

Replaces the split-point + tail-preservation model with full-history
compression + composePostCompactHistory. The entire curated history is
sent to the summary side-query, and the post-compact history is
assembled by the new composer (summary + ack + file restores + image
restore).

BREAKING: the previously-exported findCompressSplitPoint,
splitPointRetainingTrailingPairs, COMPRESSION_PRESERVE_THRESHOLD, and
TOOL_ROUND_RETAIN_COUNT will be removed in the next commit. Tests that
exercise them remain failing temporarily.

* chore(core): remove obsolete split-point compression infrastructure

Deletes findCompressSplitPoint, splitPointRetainingTrailingPairs,
COMPRESSION_PRESERVE_THRESHOLD, MIN_COMPRESSION_FRACTION, and
TOOL_ROUND_RETAIN_COUNT, plus the tests that exercised them. The new
behavior is covered by composePostCompactHistory and its unit tests.

Also cleans up:
- Stale orphan-strip comment in compress() that described the deleted
  manual-trigger orphan-funcCall handling.
- TEST_ONLY.COMPRESSION_PRESERVE_THRESHOLD hatch in client.ts.
- Docstring references in config.ts and compactionInputSlimming.ts.

* test(core): add single-turn computer-use compaction regression

Reproduces the scenario the rewrite targets: one user prompt kicks off
many screenshot tool calls. Asserts that (a) the user prompt is carried
into the summary verbatim and (b) the 3 most recent screenshots are
restored as an image block with source-tool metadata. This is the canary
test for the computer-use UX claim made in the design discussion.

* docs(core): remove stale "split point" references in tokenEstimation comments

Aligns the docstrings with the new compose-based compression flow. The
"split point" and "splitter" concepts no longer exist after the rewrite.

* fix(core): iterate parts reverse so parallel tool calls keep the last N

Real-session E2E surfaced a bug: a model that issues N parallel ReadFile
calls puts all N functionCall parts in ONE model+fc content. The
extractor's outer history walk is newest-first, but the inner parts
walk was forward — so for a 6-parallel batch hitting the cap of 5,
the FIRST 5 parts won and the actually-most-recent (last-listed) file
was dropped.

Fix: walk parts in reverse within each content. Applied symmetrically
to extractRecentImages (same shape, even rarer trigger).

Adds a regression test that hits a 6-parallel batch.

* fix(core): code-review fixes — fence escape, path sanitize, alias removal

- CommonMark-safe fence in file embed blocks. The old 3-backtick fence
  closed prematurely when a file's content contained a triple-backtick
  run (Markdown, CLAUDE.md, JSDoc with code examples) — leaking the
  remainder as unfenced text. Now uses a fence one longer than the
  longest backtick run in the content.

- Strip control characters (\r, \n, \t) from file paths before
  rendering into attachment markdown. Paths come from model-controlled
  history; a \n could inject markdown structure. The actual path stays
  intact for tool calls — only the displayed string is sanitized.

- Remove the historyForCompression alias for curatedHistory in
  compress(). The alias was added as a comment anchor during the
  rewrite but didn't carry semantic information.

* refactor(core): rewrite compression prompt to <state_snapshot> XML with 9 claude-aligned sections

Replaces the 9-section numbered-text prompt with qwen-code's original
<state_snapshot> XML envelope, but with the 9 inner section tags
content-aligned to claude-code:
  <primary_request_and_intent>
  <key_technical_concepts>
  <files_and_code_sections>
  <errors_and_fixes>
  <problem_solving>
  <all_user_messages>
  <pending_tasks>
  <current_work>
  <next_step>

Also:
- <scratchpad> -> <analysis>, stripped by postProcessSummary (saves
  ~600-800 tokens of CoT noise per compaction).
- "Resume directly..." trailer moved out of the prompt body and into
  postProcessSummary (no longer re-generated by the model every
  compaction; lives once in code with our own wording).
- Section 6 verbatim-policed mandate relaxed to "chronological, include
  short messages like 'ok' / 'continue'" — matches claude-code intent
  without forcing the model to literally copy long user messages.

E2E (qwen3.6-plus, 6 substantial .ts files + thorough analysis):
  raw history 6508 -> summary 1513 (after strip ~947), 38% history
  compression. Overall context 24642 -> 20647 reported (-16%), with
  another ~664 tokens actually saved by the post-strip but not
  reflected in the conservative token-math heuristic.

* docs(core): code-review polish on XML prompt rewrite

Four small follow-ups from review of 641a0eadd:

- prompts.ts: rewrite getCompressionPrompt's stale JSDoc — it still
  described the deleted 9-section numbered-text format and the
  verbatim mandate that was relaxed.
- chatCompressionService.ts: clarify the token-math comment so it's
  obvious the ~1000 token deduction covers the full compression
  system prompt + kick-off user turn (not any single instruction)
  and that newTokenCount slightly over-counts because <analysis>
  gets stripped by postProcessSummary downstream.
- postCompactAttachments.ts: add a NOTE comment on the <analysis>
  strip regex covering its strict-tag-match assumption and
  multi-block / non-greedy semantics.
- postCompactAttachments.test.ts: replace the four lazy
  `await import('./postCompactAttachments.js')` calls inside the
  postProcessSummary describe block with one top-level static import
  — consistent with how every other describe in the file imports.

* docs(core): drop stale duplicate sentence left in token-math comment

* fix(core): address wenshao review on PR #4599 (correctness + security + ergonomics)

Seven follow-ups from wenshao's review of the compaction rewrite.

Critical:
- newTokenCount now includes restoration-block tokens via
  estimateContentChars over extraHistory[2..]. Previously the formula
  only counted side-query output, so up to 5 × 5K (files) + 3 × image
  tokens were missing — letting the inflation guard miss and the
  cheap-gate under-estimate the next prompt size (Finding 1).
- composePostCompactHistory now merges every file restoration block
  and the image block into a single user Content following the model
  ack. The previous output had consecutive user roles, which
  geminiChat.test.ts:6289 enforces against and Gemini providers
  reject with 400 "consecutive same-role content" (Finding 2).
- Preserve a trailing model+functionCall through compaction so a
  pending functionResponse (sitting in sendMessageStream's
  pendingUserMessage) has a matching call. Without this, hard-rescue
  auto-compaction mid tool-use loop produces a user+functionResponse
  with no preceding model+functionCall → API 400. This restores the
  protection the split-point in-flight fallback used to provide.
  When the funcCall lands without attachments it folds into the
  ack's own model Content to avoid model→model adjacency (Finding 3).
- composePostCompactHistory now takes an optional workspaceRoot and
  silently skips file paths that resolve outside it.
  extractRecentFilePaths picks up paths from model functionCall args
  regardless of whether the tool execution succeeded; without a
  boundary check, an adversarial model that issued
  read_file('/etc/passwd') — denied by the permission system —
  would still have its path extracted and re-read into the next
  prompt. compress() passes config.getTargetDir() as the boundary
  (Finding 4).

Suggestions:
- composePostCompactHistory + buildFileRestorationBlocks +
  readFileSizeAdaptive all take optional AbortSignal and short-
  circuit / pass it to readFile's { signal } option. Cancelled
  compactions stop on the next file read (Finding 5).
- postProcessSummary fallback no longer re-injects the raw
  <analysis> block when the strip leaves nothing. The new
  stripAnalysisBlock helper runs the closed-tag strip AND an
  unclosed-tag strip (handles 'model ran out of output tokens
  before closing'). If both leave nothing, postProcessSummary
  emits '[Summary unavailable]' rather than leaking scratchpad
  (Finding 6).
- firePostCompactEvent now receives stripAnalysisBlock(summary) so
  hook consumers see the same text that lands in history. The
  resume trailer stays out of the hook payload — that's wrapper
  decoration for the next agent turn, not state for consumers
  (Finding 8a).

Docs:
- Update the geminiChat.ts comment around `trigger: 'auto'` to
  describe what the trigger actually does post-refactor (hook event
  categorization) rather than the deleted manual-only orphan-strip
  it used to guard against (Finding 8b).

Regression tests cover all six fixable code-path changes
(role alternation, trailing funcCall preservation, workspace
boundary, abort propagation, closed-tag fallback strip, unclosed-tag
fallback strip).

* fix(core): add getTargetDir to geminiChat auto-compression test mock

The R3.4 end-to-end auto-compression test drives the real
ChatCompressionService, which reads config.getTargetDir() for the
post-compact file-restoration workspace boundary. The geminiChat mock
config lacked getTargetDir, so the test threw "config.getTargetDir is
not a function" on CI. Add the mock to unblock the failing Test jobs.

* feat(core): configurable compaction retention + computer-use screenshot trigger

Add four env-overridable chatCompression settings (priority env >
settings > default):
- maxRecentFilesToRetain    (QWEN_COMPACT_MAX_RECENT_FILES,     default 5)
- maxRecentImagesToRetain   (QWEN_COMPACT_MAX_RECENT_IMAGES,    default 3)
- enableScreenshotTrigger   (QWEN_COMPACT_SCREENSHOT_TRIGGER,   default true)
- screenshotTriggerThreshold(QWEN_COMPACT_SCREENSHOT_THRESHOLD, default 50)

The screenshot trigger fires auto-compaction once tool-returned images
accumulate to the threshold even when token usage is below the auto tier,
so computer-use sessions don't drown the model in stale screenshots. It
counts only images nested in functionResponse.parts (tool results), not
user pastes, and runs only in the would-be-NOOP path when enabled.

Fix a latent bug surfaced while wiring the trigger: extractRecentImages
only inspected top-level inlineData parts, but convertToFunctionResponse
nests tool media under functionResponse.parts — so post-compact
restoration recovered ZERO tool screenshots in real sessions, while unit
tests stayed green against a fabricated top-level shape. It now walks both
shapes; the image counter and tests use the real nested shape.

Remove the now-defunct contextPercentageThreshold deprecation warning (the
field was already dropped from ChatCompressionSettings) and its tests, and
document the four new settings.

* test(core): assert screenshot trigger can't re-fire post-compaction; fix misleading docs

Code-review follow-up. The screenshot trigger counts only images nested in
functionResponse.parts. Compaction replaces those with the summary and
re-embeds survivors as TOP-LEVEL parts in the restoration block, which the
counter ignores — so the tool-image count always resets to ~0 and the
trigger cannot immediately re-fire, independent of maxRecentImages.

The resolveCompactionTuning JSDoc and the settings.md note previously warned
of a non-existent "maxRecentImages near threshold => compact every turn"
loop. Correct both, and add a regression test asserting
countToolResponseImages() is 0 on composePostCompactHistory output.

* fix(core): guard readFileSizeAdaptive against multi-GB reads; cover composer 4-entry branch

wenshao review round 2 on PR #4599.

- readFileSizeAdaptive now stats the file first and short-circuits to a
  reference when its byte size exceeds maxChars*4 (the safe UTF-8 upper
  bound — a file larger than that cannot fit within maxChars chars). This
  stops a multi-GB file the agent previously touched from being slurped
  into a Buffer and exhausting the heap mid-compaction, exactly when we're
  trying to reduce memory. A large binary file now references rather than
  reading to binary-detect.

- Add a test for composePostCompactHistory's 4-entry branch (attachments +
  trailing model+functionCall) producing [user(summary), model(ack),
  user(attachments), model(fc)]. This is the common mid-tool-loop
  compaction case; a model->model adjacency here is a provider 400. Prior
  tests only covered the 2-entry fold (no attachments) and 3-entry (no
  trailing fc) shapes.

* fix(core): resolve symlinks in workspace boundary; guard compose against throws

wenshao review round 3 on PR #4599 (two Criticals).

- isInsideWorkspace now resolves symlinks via realpathSync (safeRealpath,
  with a lexical fallback for non-existent paths). A symlink living inside
  the workspace but pointing outside (e.g. workspace/.env -> ~/.ssh/id_rsa)
  previously passed the lexical boundary check and had its target read and
  embedded into the post-compact history sent to the provider. Added a
  RED-verified security regression test (secret embedded under the old
  lexical check; rejected under realpath).

- Wrap composePostCompactHistory in try/catch inside compress(). The
  summary side-query has already succeeded at that point, so a
  restoration-assembly throw (disk I/O / malformed history) previously
  escaped to sendMessageStream, crashing the active turn AND bypassing the
  COMPRESSION_FAILED breaker. It now degrades to summary + ack.

* fix(core): close 4 compaction Criticals from review round 4

wenshao review round 4 on PR #4599.

- isSummaryEmpty now checks the STRIPPED summary: a response that is only an
  <analysis> block (no <state_snapshot>) strips to empty, so it takes the
  COMPRESSION_FAILED_EMPTY_SUMMARY path instead of "succeeding" with
  `[Summary unavailable]` as the agent's only context (silent amnesia).
- Manual /compress strips a trailing ORPHANED model+functionCall before
  composing — it has no pending functionResponse, so preserving it would
  emit model[fc] then the next user text turn -> API 400. Auto-compaction
  still keeps it (the pending response pairs with it).
- The restoration-failure catch fallback now folds a trailing
  model+functionCall into the ack turn, so a pending functionResponse
  (auto mid-tool-loop) keeps its matching call even on the degraded path.
- extractRecentFilePaths skips file paths whose tool call FAILED (an error
  functionResponse), so a denied read_file is never re-read off disk during
  compaction — closing a permission-bypass side channel.

RED-verified regression tests for the empty-summary, orphan-strip, and
permission-bypass fixes. Corrected the postProcessSummary comment.

* test(core): cover composePostCompactHistory catch-fallback; document fold text drop

wenshao review round 5 on PR #4599.

- Regression test for the restoration-failure catch fallback: mock
  composePostCompactHistory to reject and assert compaction still returns
  COMPRESSED (no escape to sendMessageStream / breaker bypass) with the
  trailing functionCall folded into the ack and the trailing text dropped.
- Document that the fold branch intentionally keeps only functionCall parts
  (the trailing turn's text is already captured in the summary); the
  asymmetry with the with-attachments branch is deliberate.
2026-05-29 16:20:13 +08:00
顾盼
0c3cd0052f
feat(cli): default auto-dream/auto-skill to on and add /memory toggle (#4547)
* feat(cli): default auto-dream/auto-skill to on and add /memory toggle

Bring the managed memory pipeline closer to its intended out-of-the-box
experience: auto-dream and auto-skill now default to enabled (matching
the existing auto-memory default), so users get summarized memories and
reusable project skills without having to opt in.

The /memory dialog previously only exposed Auto-memory and Auto-dream
toggles. With auto-skill now on by default, users need an equally
discoverable way to opt out, so this adds an Auto-skill row alongside
the existing two with the same focus/Enter toggle semantics and
workspace-scoped persistence (memory.enableAutoSkill).

Default-value updates are kept consistent across all three sources of
truth (settings schema, CLI loader, core Config), and the generated
vscode settings.schema.json is regenerated to match.

* test(cli): add getAutoSkillEnabled to MemoryDialog test mock

The new Auto-skill toggle row reads config.getAutoSkillEnabled() at
render time; without it on the mocked config the component throws and
the existing list-navigation tests assert against an empty frame.

* fix(cli): guard managed auto-dream in bare mode, sync tests and docs

- enableManagedAutoDream in loadCliConfig was missing the bareMode guard
  that its two siblings already had; once the default flipped to true,
  this caused a raw-field inconsistency in bare-mode sessions (the
  getter still returned false via its own !getBareMode() guard, but the
  Config.enableManagedAutoDream field itself was now true).
- docs/users/configuration/settings.md still listed
  enableManagedAutoDream's default as false, and was missing the new
  enableAutoSkill row entirely. Both fixed.
- MemoryDialog.test.tsx now covers the autoSkill row render, the new
  focus chain (list ↑ autoSkill ↑ autoDream and back down), and the
  Enter-toggle path that writes memory.enableAutoSkill to workspace
  settings.
- config.test.ts gains a non-bare default test asserting all three
  getManaged*Enabled() / getAutoSkillEnabled() return true, and the
  bare-mode test now asserts auto-dream/auto-skill also resolve to
  false in bare mode.
2026-05-27 17:25:06 +08:00
Edenman
331f45e907
feat(cli): headless / non-interactive runaway-protection guardrails (#4103) (#4502)
* feat(cli): headless runaway-protection guardrails (#4103)

Adds two opt-in run-level budgets and a startup safety warning for
non-interactive / CI / SDK runs. All defaults preserve existing
behavior; the budgets only fire when the user explicitly sets a limit.

Phase 1 — surface unsafe configs and fix doc drift
- New `--yolo`-without-sandbox stderr warning at startup of every
  non-interactive run, emitted by `getHeadlessYoloSafetyWarning` in
  `packages/cli/src/utils/headlessSafetyWarnings.ts`. Suppressible
  via `QWEN_CODE_SUPPRESS_YOLO_WARNING=1` (strict `1`/`true` match so
  `=0` / `=false` don't silence it). Strict env match also applied to
  the `SANDBOX` check so values like `SANDBOX=0` don't accidentally
  bypass the warning.
- Gated on `!config.isInteractive()` at the gemini.tsx call site so
  TUI users aren't nagged.
- `docs/users/configuration/settings.md`: corrected
  `model.skipLoopDetection` default (`true`, not `false`) and reworded
  the `--yolo`/sandbox section — `--yolo` does NOT auto-enable a
  sandbox; sandboxing must still be opted into explicitly.

Phase 2 — run-level budgets with distinct exit code
- `--max-wall-time` / `model.maxWallTimeSeconds`: wall-clock duration
  for the whole run. Flag accepts `90` (s), `30s`, `5m`, `1h`,
  `500ms`. Settings is plain seconds.
- `--max-tool-calls` / `model.maxToolCalls`: cumulative tool
  executions (success + failure). Ticked BEFORE each `executeToolCall`
  so a budget of N caps the run at exactly N executions.
- New `FatalBudgetExceededError` (exit code 55), distinct from
  `FatalTurnLimitedError` (53) and `FatalCancellationError` (130) so
  CI scripts can branch on the reason. JSON output mirrors the
  `handleMaxTurnsExceededError` / `handleCancellationError` envelope
  convention.
- Enforced via `RunBudgetEnforcer` in
  `packages/cli/src/utils/runBudget.ts`, wired to the same
  `AbortController` as SIGINT so existing cancellation plumbing
  carries the abort. A `routeAbort` helper distinguishes budget vs.
  SIGINT at the abort-check sites and at the outer catch.

Critical correctness fixes (informed by the #4105 review pass)
- Drain-loop fall-through: the inner drain-item `for await` previously
  exited via `finalizeAssistantMessage(); return;`, swallowing a
  budget abort that fires during the last drain item and surfacing
  exit code 0. Now routes through `routeAbort` so exit 55 is
  preserved.
- Settings symmetry: `maxWallTimeSeconds: 0` in settings.json is now
  rejected (same as `--max-wall-time 0`); the enforcer treats `<=0`
  as "no timer" so silent disable would be a foot-gun.
  `validateMaxWallTimeSetting` also rejects `Infinity` / `NaN`.
- `setTimeout` overflow: both parser paths reject durations above
  `Math.floor((2^31 - 1) / 1000)s` (~24.8 days). Node clamps
  oversized delays to 1ms and fires the timer almost immediately;
  fail loud at startup instead.
- First-fence-wins + SIGINT race: `markExceeded` no-ops if the
  controller was already aborted by a third party, so a budget tick
  arriving after user SIGINT doesn't misattribute the abort to exit
  code 55.
- Outer catch re-routes mid-stream `AbortError`s through the budget
  handler so users see "Run aborted: …" instead of raw "AbortError".

Tests
- `runBudget.test.ts` (32 tests): parser happy / reject paths,
  setting validator, post-increment off-by-one, `maxToolCalls=0`
  meaning "disallowed", `-1` meaning unlimited, wall-clock under
  fake timers, `stop()` cancels pending timer, idempotent `start()`,
  first-fence-wins, SIGINT-race protection.
- `headlessSafetyWarnings.test.ts` (7 tests): YOLO + sandbox / env
  matrix; strict-truthy `SANDBOX` check; suppression env.
- Pre-existing suites: `nonInteractiveCli.test.ts` (46),
  `gemini.test.tsx` (23), `config/config.test.ts` (220),
  `core/utils/errors.test.ts` (12), `core/config/config.test.ts`
  (172) all green after picking up the new config getters / CliArgs
  fields.

Backward compatibility
- All budgets default to `-1` (unlimited); existing CLI invocations
  behave identically.
- New stderr warning only fires in the narrow YOLO-no-sandbox case,
  with an explicit suppress env.
- New exit code 55 is purely additive; no existing exit codes change
  meaning.

* fix(cli): address audit findings for headless guardrails (#4103, #4502)

Round-1 audit (3 angles × line-by-line + removed-behavior + cross-file)
plus an open-ended design pass surfaced eight correctness issues. This
commit lands all of them; the larger ACP / serve-mode structural items
are documented for follow-up.

Correctness fixes

- headlessSafetyWarnings: `SANDBOX` env check reverted to plain truthy.
  The sandbox transport sets `SANDBOX` to `sandbox-exec` (macOS
  seatbelt) or the container name (`qwen-code-sandbox`), neither of
  which matches `isTruthyEnv`. The PR's strict-`1`/`true` check was
  emitting the "no sandbox" warning INSIDE real sandboxes. Match the
  rest of the codebase (sandboxConfig.ts, gemini.tsx, Footer.tsx,
  prompts.ts, …) which all treat any non-empty value as "sandboxed".
- nonInteractiveCli main-loop abort: add `finalizeAssistantMessage()`
  before `routeAbort()`. The drain-item loop already had it (PR #4502
  Critical bug #1); the main loop was asymmetric — stream-json
  consumers would see an unterminated `message_start` when a budget /
  SIGINT abort landed mid-stream.
- nonInteractiveCli drain-loop `routeAbort`: also flush
  `flushQueuedNotificationsToSdk(localQueue)` and
  `finalizeOneShotMonitors()` before exiting. The old `return`-and-
  fall-through path went through the outer holdback loop, which did
  this flushing; switching to `routeAbort()` skipped it, so
  `task_started` envelopes lost their paired `task_notification`.
- nonInteractiveCli catch handler: emit `adapter.emitResult({...})`
  BEFORE `handleBudgetExceededError`, with the budget message as
  `errorMessage` when budget tripped. Previously the budget handler
  `process.exit(55)`ed before the adapter could emit a terminal
  `result` envelope, so STREAM_JSON consumers never saw a stream
  terminator on budget exits and hung waiting for one.
- runBudget: new `validateMaxToolCalls` mirrors
  `validateMaxWallTimeSetting`. yargs coerces non-numeric flag values
  (`--max-tool-calls abc`) to `NaN`, and the enforcer's `>= 0` gate
  treats `NaN` and negatives as "no limit", silently disabling the
  budget. Reject `NaN`, `Infinity`, fractional, and negative-other-
  than-`-1` values at both flag and settings layers. `0` remains
  legal (`first tick aborts`), unlike wall-time where 0 is fatal.
- runBudget: new `MIN_WALL_TIME_SECONDS = 1` floor. Previously
  `--max-wall-time 500ms` parsed cleanly and aborted on the next
  event-loop tick before any model round-trip — almost certainly a
  typo (`5m`?) and not a useful guardrail at any rate.
- nonInteractiveCli `tickToolCall`: exempt `ToolNames.STRUCTURED_OUTPUT`.
  Under `--json-schema` this is the terminal "I'm done" contract tool,
  not real work. Without the exemption a budget-edge completion is
  aborted as a false positive (model used N tools then emitted
  structured_output as call N+1 → exit 55 instead of success).
- commands/serve.ts: emit the YOLO-no-sandbox warning at daemon
  startup when settings.json statically configures
  `tools.approvalMode: 'yolo'` with no `tools.sandbox` /
  `SANDBOX` env. The daemon can't use `getHeadlessYoloSafetyWarning`
  (no Config yet — sessions get their own) so we re-derive the
  predicate from settings. Per-session ACP override is documented as
  out of scope.

Documentation

- `docs/users/features/headless.md`: new "Scope" subsection under
  Run-level budgets explaining (a) `--max-tool-calls` counts top-level
  dispatches only — subagent / `agent` tool inner calls are not
  counted, (b) `structured_output` is exempt, (c) stream-json input
  mode resets budgets per user message, (d) `qwen serve` / ACP
  sessions do not currently consult budgets from settings.json.

Tests

- `runBudget.test.ts` grows from 32 → 41 tests: `validateMaxToolCalls`
  (NaN / Infinity / negatives / fractional), `parseDurationSeconds`
  sub-second rejection, `validateMaxWallTimeSetting` sub-second
  rejection.
- `headlessSafetyWarnings.test.ts`: replaced the "still warns when
  SANDBOX is 0/false/no" case (which encoded the strict-check bug) with
  positive coverage for the real sandbox-set values
  (`sandbox-exec`, `qwen-code-sandbox`).

All previously-green suites still green: cli/nonInteractiveCli (46),
cli/gemini.test (23), cli/config/config.test (220), core/utils/errors
(12), core/config/config.test (172). 337 tests across the touched suites.

Won't-fix (out of scope, documented or pre-existing)

- Unpaired `tool_use` in stream-json when a tool is aborted mid-execution
  — pre-existing structural gap (SIGINT mid-tool has the same outcome);
  PR amplifies it but doesn't introduce it.
- Narrow SIGINT-vs-budget-timer race — already mitigated by
  `markExceeded`'s `signal.aborted` check.
- `tickToolCall` increments past abort (cosmetic; only affects the
  `observed` value in the error envelope for a pathological caller).

* fix(cli): round-2 audit fixes for headless guardrails (#4103, #4502)

Round-2 audit (after round-1 commit 40ae6dd0f) surfaced two NEW
correctness issues introduced by the round-1 catch-handler restructure,
plus a handful of polish items from a parallel design pass.

Correctness fixes (new bugs from R1)

- nonInteractiveCli catch handler: wrap `adapter.emitResult` in
  try/catch. R1 moved the emit BEFORE `handleBudgetExceededError` so
  STREAM_JSON consumers see a terminal envelope first. But emitResult
  eventually hits `stdout.write`, which throws on EPIPE /
  ERR_STREAM_WRITE_AFTER_END when a piped consumer closes early
  (`qwen -p ... | head -n 1` is the common CI case). Letting that
  throw bubble out skipped both `handleBudgetExceededError` and
  `handleError`, dropping the documented exit-code-55 contract
  precisely when stdout was in trouble. Best-effort emit and continue
  to the exit handler.
- nonInteractiveCli `structured_output` exemption: also require
  `config.getJsonSchema?.() !== undefined`. Without that guard, an
  MCP server registering an unrelated tool literally named
  `structured_output` would silently bypass `--max-tool-calls`. Also
  documents (in `headless.md` "Scope") the related caveat that failed
  Ajv-validation retries skip the tick too, so a malformed-output
  retry loop is NOT bounded by `--max-tool-calls` — combine with
  `--max-session-turns` or `--max-wall-time`.

Polish

- runBudget `validateMaxToolCalls` upper bound: cap at 1_000_000.
  `1e10` (typo for `1e1`) would otherwise parse cleanly, pass the
  `>= 0` gate forever, and silently disable the budget — the exact
  foot-gun `MAX_WALL_TIME_SECONDS` was built to prevent. Symmetry.
- runBudget `parseDurationSeconds` sub-second hint: only append the
  "did you mean Ns?" suggestion when the input actually contained
  `ms`. Bare `0.5` would otherwise produce a useless "did you mean
  0.5s?" suggestion.
- nonInteractiveCli `routeAbort`: the `throw 'unreachable'` is only
  hit if `handleBudgetExceededError` / `handleCancellationError` ever
  becomes resumable (e.g. mocked `process.exit` in a test). Carry
  the original exceeded.message into the thrown Error so the outer
  catch's `errorMessage` field stays actionable instead of degrading
  to a literal "unreachable" string.
- commands/serve.ts: compare `approvalMode` against `ApprovalMode.YOLO`
  enum instead of the string literal `'yolo'`. If the enum value is
  ever renamed, the startup warning stays in sync with the helper at
  `headlessSafetyWarnings.ts` instead of silently going dead.

Documentation

- `headless.md` "Scope": clarify the `structured_output` exemption is
  unconditional (including failed validations); add explicit note
  that `--max-session-turns` does NOT exempt `structured_output`, so
  size to `N+1` for `N` real-work turns under `--json-schema`.
- `headless.md` flag table: add `1.5h` to the accepted-forms hint for
  `--max-wall-time` (the parser already accepts fractional units).

Tests

- `runBudget.test.ts`: new coverage for the `validateMaxToolCalls`
  ceiling. Total 42 tests across `runBudget.test.ts` (was 41), all
  green. cli/nonInteractiveCli, gemini.test, config/config all
  unchanged and still green.

Won't-fix (documented above or out of scope)

- ACP per-session approval-mode escalation (mid-session flip to YOLO)
  doesn't print the warning — daemon-level wiring; out of scope for
  this PR.
- 1s wall-time floor vs higher (5–10s) — debatable, keeping 1s with
  loud sub-second rejection; can raise later without semver impact.
- Integration test for the full budget-trip → catch → emitResult →
  exit 55 path — requires a process-exit-mocking harness; tracked as
  follow-up.

* docs: align headless guardrails examples with R1 sub-second floor

Round-3 audit caught two stale doc surfaces that R1's 1-second wall-time
floor (and R2's `1.5h` fractional-unit addition) didn't update:

- `docs/users/features/headless.md` budget table: replace stale `500ms`
  example with `1.5h`, add explicit "minimum 1s — sub-second values are
  rejected as typos" note.
- `docs/users/configuration/settings.md` `model.maxWallTimeSeconds` row:
  same fix. Also extend `model.maxToolCalls` row with the structured_output
  exemption note, the `0` semantic, and the 1,000,000 ceiling that R2
  added.

A user copying the documented `--max-wall-time 500ms` example from either
surface would hit a startup error after R1.

Known follow-up (not addressed in this commit)

- No test exercises the R2 `isStructuredOutputExempt` predicate end-to-end.
  Adding one needs the same process-exit-mocking harness called out in the
  R2 commit as a separate follow-up.

* docs: align JSDoc / schema / CLI help with R1+R2 validation rules

Round-4 final-pass audit caught four schema/help-text/JSDoc surfaces
that drifted from the validators introduced in R1 (1s wall-time floor,
24-day ceiling) and R2 (1M tool-call ceiling, structured_output
exemption, `0` sentinel).

- `runBudget.ts` `parseDurationSeconds` JSDoc: replace stale claim
  that `500ms` is accepted and "sub-second precision is preserved"
  with the actual contract — `[MIN_WALL_TIME_SECONDS, MAX_WALL_TIME_SECONDS]`,
  ms suffix only legal when value resolves to >= 1s. Adds `1.5h` to
  the accepted-forms list.
- `settingsSchema.ts` `model.maxWallTimeSeconds` description: now
  documents the 1s minimum and ~24-day ceiling.
- `settingsSchema.ts` `model.maxToolCalls` description: documents the
  structured_output exemption, the `0` sentinel ("no tool calls
  allowed"), and the 1,000,000 ceiling.
- `vscode-ide-companion/schemas/settings.schema.json`: mirrors both
  schema descriptions above so the VS Code settings UI auto-completion
  matches.
- `config.ts` yargs `--max-wall-time` description: documents the 1s
  floor and the ~24-day max.
- `config.ts` yargs `--max-tool-calls` description: documents the
  structured_output exemption, the `0` sentinel, and the 1M ceiling.
  `qwen --help` is the most-read surface for these flags; matches the
  prose docs in headless.md and settings.md.

No code changes — pure doc/help-text alignment.

---------

Co-authored-by: 克竟 <dingbingzhi.dbz@alibaba-inc.com>
2026-05-26 00:06:26 +08:00
顾盼
a8a6ad2d06
feat(core)!: redesign auto-compaction thresholds with three-tier ladder (#4345)
* feat(core)!: redesign auto-compaction thresholds with three-tier ladder

Replaces the single 70% proportional threshold with a three-tier ladder
(warn/auto/hard) that combines proportional fallback with absolute
reservation. Large-window models (>=128K) now reserve ~33K instead of
30% of the window, freeing tens of thousands of context tokens that the
old formula wasted.

Other improvements bundled in the same redesign:

- Compression sideQuery now disables thinking and caps maxOutputTokens
  at 20K, matching claude-code so the buffer math is predictable across
  providers (Anthropic/OpenAI/Gemini handle thinking budgets
  inconsistently)
- Failure handling upgraded from one-shot permanent lock to a 3-strike
  circuit breaker; reactive overflow still latches immediately
- New estimatePromptTokens helper closes the lag-by-one-turn and
  first-send-is-0 gaps in lastPromptTokenCount
- Hard-tier rescue pulls reactive overflow recovery forward to before
  the API call, saving an oversized round-trip
- /context command displays the three-tier ladder + current tier
- tipRegistry's context-* tips track the new thresholds instead of
  fixed 50/80/95 percentages

BREAKING CHANGE: chatCompression.contextPercentageThreshold setting is
removed. Settings files containing the field log a one-line deprecation
warning at startup and the value is ignored; behaviour is now controlled
by built-in thresholds via the new computeThresholds() function.

Design: docs/design/auto-compaction-threshold-redesign.md
Plan: docs/plans/2026-05-14-auto-compaction-threshold-redesign.md

* test(core): fix leftover hasFailedCompressionAttempt option in compress test

A pre-existing test case at chatCompressionService.test.ts:678 still
passed `hasFailedCompressionAttempt: false` in the CompressOptions
shape; rebasing onto current main surfaced this as a typecheck error
because the field was renamed to `consecutiveFailures` (Task 7 of the
three-tier ladder migration). Update to `consecutiveFailures: 0` —
semantically equivalent, the test asserts the side-query is called
when `force: true`, no other behaviour change.

* fix(core): drop compaction summary when output hits maxOutputTokens cap

Adds a defensive guard in ChatCompressionService.compress() that detects
when the side-query summary hit COMPACT_MAX_OUTPUT_TOKENS (20K). In that
case the summary is likely truncated mid-content, so we drop it and
return NOOP rather than persist a half-summary. The next send re-tries;
reactive overflow still catches the catastrophic case where the API
rejects the next request as too large.

Documented in the design doc as risk #2; the bot reviewer on PR #4168
correctly pushed for it to land alongside the threshold redesign rather
than as a follow-up since the new 20K cap is what makes truncation
likely in the first place.

* fix(cli): render three-tier thresholds in /context TUI view

The Task 11 redesign updated the non-interactive text formatter
(formatContextUsageText) but left ContextUsage.tsx — the interactive
React component that real /context users see — unchanged. As a result
the TUI still showed the old single "Autocompact buffer" line and none
of the new warn/auto/hard ladder.

Adds a "Compaction thresholds" section after the per-category breakdown:
  - Effective window
  - Warn / Auto / Hard threshold rows with a ▶ marker on the row the
    current usage has crossed
  - Current tier label coloured by severity (safe→green, warn/auto→
    yellow, hard→red)

The existing progress bar legend (Used / Free / Autocompact buffer)
is preserved because it's tied to the three-segment progress bar
visualisation; the new section adds the absolute numbers + tier badge
on top of that.

Caught by the tmux e2e test (PR #4168 ci-monitor follow-up). Pre-fix
the assertion 'Compaction thresholds' missed completely from the TUI;
post-fix the new section renders correctly for fresh and live sessions
on 1M / 200K / 128K windows.

* fix(core,cli): address PR #4168 review batch 4

Behavior fixes:
- MAX_TOKENS truncation guard now returns COMPRESSION_FAILED_EMPTY_SUMMARY
  instead of NOOP so the consecutive-failure breaker actually trips after
  repeated max-length summaries (R1.1).
- Reactive overflow failure increments consecutiveFailures by 1 instead
  of latching to MAX in one shot, so a transient network blip doesn't
  permanently disable auto-compaction. The hard-tier rescue resets the
  counter, which remains the designated recovery path (R1.2).
- /context current-tier classification uses rawOverhead (system + tools +
  memory + skills) as the tier input when API data is not yet available,
  rather than 0 — large inherited contexts no longer silently show 'safe'
  (R2.2).

Performance:
- sendMessageStream computes effectiveTokens ONCE and passes it through
  TryCompressOptions.precomputedEffectiveTokens, so the cheap-gate inside
  service.compress doesn't redo the estimation. Also fixes the
  imageTokenEstimate inconsistency between the rescue and cheap-gate
  paths (R1.3 + R1.4).
- Steady-state path (lastPromptTokenCount > 0) skips the costly
  getHistory(true) clone — estimatePromptTokens only needs the user
  message in that branch.

Code hygiene:
- BYTES_PER_TOKEN → CHARS_PER_TOKEN (inputs are char counts, not byte
  counts; CJK text would mislead under the old name) (R3.1).
- Drop dead getContextUsagePercent helper + index re-export — no callers
  in source after the threshold rewire (R1.5).
- Add a comment on estimatePromptTokens' first-send fallback documenting
  the ~15-20K under-estimate (system prompt + tools + skills) and that
  reactive overflow is the safety net (R3.3).

Tests:
- New CLI ContextUsage.test.tsx exercises the React renderer for the
  three-tier section: section presence, ▶ marker placement per tier,
  current-tier label coloring (R1.6).
- New chatCompressionService.test.ts case pins that a stale
  contextPercentageThreshold: 0 value in user settings no longer
  short-circuits compaction (R2.1).
- New tokenEstimation.test.ts case covers functionResponse (distinct
  nested-parts branch from functionCall) (R3.5).
- New geminiChat.test.ts integration test exercises the real
  ChatCompressionService — not a mock — for the first-send-after-
  inherited-history scenario where lastPromptTokenCount=0 and only the
  full-history estimate can cross the auto threshold (R3.4).

Declined: R3.2 (change `>=` to `>` on the MAX_TOKENS guard). The current
operator catches the at-cap case as suspicious, which is intentional —
landing exactly at the output cap is far more likely truncation than
clean stop given p99.99 ≈ 17K. With R1.1 in place, persistent truncations
trip the breaker after MAX_CONSECUTIVE_FAILURES so the worst case is
bounded.

* fix(core,cli): address PR #4168 review batch 5

- R5.1: tighten /context tier comment + TODO. The rawOverhead-based fix
  doesn't cover `--continue` restores with many history messages (since
  rawOverhead excludes messagesTokens). UI may still show 'safe' for one
  render until the first send. Documented inline and added a TODO to plumb
  chat history into collectContextData for same-source-of-truth as the
  cheap-gate.
- R5.2a: add TODO(finish_reason) at the truncation guard. The `>= cap`
  heuristic false-positives on legitimate at-cap summaries; the proper
  signal is finish_reason which runSideQuery doesn't surface today.
- R5.2b: split telemetry — new CompressionStatus.COMPRESSION_FAILED_OUTPUT_TRUNCATED
  enum value. Distinct from EMPTY_SUMMARY so logs/telemetry can tell
  prompt-quality failures (tune prompt / splitter) from capacity failures
  (raise cap / shrink splitter input). isCompressionFailureStatus()
  treats both as failures so the breaker behavior is unchanged.
- R5.3: expand consecutiveFailures JSDoc to clarify it tracks
  "non-force, non-hard-rescue consecutive failures" — hard-rescue resets
  the counter and force=true skips increments, so the counter is the
  "regular path" health signal only; reactive overflow is the real
  safety net for the force-only paths.
- R5.4: document the CompressOptions field rename
  (hasFailedCompressionAttempt: boolean → consecutiveFailures: number)
  as an SDK breaking change in the design doc with migration guide.

* fix(core): disambiguate hard-rescue from manual /compress orphan-strip

Self-review (dual reviewer / pr-triage round 1) caught a correctness
regression in the hard-rescue path:

`sendMessageStream` calls `tryCompress(force=true)` from inside the
pre-push window when `effectiveTokens >= hard`. The service's
orphan-strip predicate at `chatCompressionService.ts:426-429` gated on
`force` alone, which conflated two distinct call shapes:

  - manual `/compress` (force=true, trigger='manual'): user-initiated
    between turns; trailing model funcCall IS orphaned because no
    funcResponse is coming
  - hard-rescue (force=true, trigger='auto'): automatic mid-turn;
    trailing model funcCall is ACTIVE because its matching funcResponse
    is sitting in the pending `userContent` waiting to be pushed

The strip fired for both, so a hard-rescue triggered mid tool-use loop
would drop the active funcCall. After compression returned and
`userContent` (the funcResponse) was pushed, the next API request
carried tool_result with no matching tool_use → provider validation
error.

The in-code comment at L422-424 already documented this exact
constraint for the auto-compress case (`force=false`), but reusing
`force=true` for hard-rescue silently violated the same constraint.

Fix:
- Gate `hasOrphanedFuncCall` on `compactTrigger === 'manual'` instead
  of `force`. The trigger field already disambiguates intent.
- `sendMessageStream` hard-rescue now passes `trigger: 'auto'`
  explicitly (without it, `force=true` defaults to `trigger='manual'`
  via the `?? (force ? 'manual' : 'auto')` resolver).

Sibling audit for "force=true non-manual callsites":
- `GeminiClient.tryCompressChat` (manual /compress): correct — manual
- `sendMessageStream` hard-rescue: fixed in this commit
- `sendMessageStream` reactive overflow catch: already passes
  trigger='auto'; runs AFTER API call (userContent in history), so if
  it observes a trailing funcCall it IS orphaned but findCompressSplitPoint
  handles the case without needing the strip

RED-first regression test added:
`preserves trailing model+funcCall under hard-rescue (force=true + trigger=auto)`
in `chatCompressionService.test.ts`. Failed against pre-fix code (the
strip dropped the funcCall); passes against the fix.

Adjacent fixes from the same triage round:

- `docs/users/configuration/settings.md`: the
  `chatCompression.contextPercentageThreshold` row still said "use 0
  to disable compression entirely" — code has ignored the value since
  the removal commit. Marked the row REMOVED with migration guidance
  pointing at the design doc.
- `packages/core/src/config/config.ts`: the deprecation warning now
  tells users how to silence it (remove the key) and where to read
  current behavior, instead of just announcing the removal.
- `docs/design/auto-compaction-threshold-redesign.md`: closed Open
  Question 2 (small-window hard/auto collapse) — decision is to NOT
  annotate `/context`, with rationale on file.

Tests: 2395 core tests passing, typecheck clean.

* docs(core): fix tier-collapse direction in auto-compaction design doc

Self-review on the 50bac974b commit caught a direction error in the
M2a Open Question 2 closure note: said `currentTier` skips `'hard'`
and goes to `'auto'` on collapsed windows, which is backwards.

`contextCommand.ts:43-44` checks `tokens >= thresholds.hard` first
(no `hard > auto` guard — that fix lives in a separate follow-up), so
when `hard === auto` the `'hard'` branch matches first and the
`'auto'` band is the empty one. Updated the rationale to describe the
actual collapse direction and cite the source-of-truth file:line.

Conclusion of the open question (don't annotate `/context`) is
unchanged — only the explanation is corrected.

* refactor(core): extract shared in-flight funcCall fixture in compression tests

The auto-compress and hard-rescue tests for "trailing funcCall is
active, not orphaned" shared a byte-identical 4-message history and
mock setup. Pull both into setupInFlightFuncCallFixture() inside the
describe block so each test only contains the scenario name, the
compress() call shape, and its own assertions.

Net -29 LOC, no behavior change.

* fix(core,cli): address PR #4345 round-2 review feedback

- geminiChat: remove pre-call consecutiveFailures reset in hard-rescue.
  force=true already bypasses the breaker check in chatCompressionService;
  the pre-reset was redundant on success (post-call L614 already handles it)
  and *broke* the breaker on failure paths — hard-rescue failures don't
  increment via tryCompress (force=true skips that branch), only the
  reactive overflow path at L992 explicitly increments. With the pre-reset
  the counter oscillated 0↔1 every send and MAX_CONSECUTIVE_FAILURES=3 was
  unreachable. Wrote a RED test asserting the forwarded counter is the
  latched value, not zero; the test failed against the old code and passes
  with the reset removed.

- geminiChat: log hard-tier-rescue triggers via debugLogger.warn including
  effectiveTokens, hard, and the current consecutiveFailures so operators
  debugging "compaction stopped working" have a breadcrumb.

- chatCompressionService: clamp effectiveWindow to >= 0 in computeThresholds
  so the value surfaced in /context stays meaningful for tiny windows
  (window < SUMMARY_RESERVE). auto/warn/hard outputs are unaffected because
  each is Math.max(proportional, absolute) and the proportional branch
  dominates whenever the absolute branch goes negative.

- turn.ts: rewrite COMPRESSION_FAILED_OUTPUT_TRUNCATED docstring. Drop the
  misleading "compression succeeded" framing (the summary is dropped and
  isCompressionFailureStatus returns true) and reference the full enum name
  COMPRESSION_FAILED_EMPTY_SUMMARY instead of the abbreviation.

- contextCommand.test.ts: reword the no-API-data-session test comment.
  collectContextData classifies estimated sessions against rawOverhead;
  with default fixtures rawOverhead lands in `safe`, but heavy
  system-prompt / skill / MCP loads can push it into warn/auto/hard.

- design doc Background: prepend a blockquote clarifying the section
  describes pre-redesign behavior and that the inline file:line references
  point at code before PR #4345 (which removes them).

- ui/types: replace the duplicated ContextThresholds interface with a
  type alias to the core's CompactionThresholds. Field-by-field copy in
  contextCommand.ts becomes a direct spread. ContextUsage.tsx keeps its
  CompactionThresholds React component name — the alias avoids the
  collision a direct import would have caused.

- contextCommand: interpolate the actual reserve value into the
  "(window − 20K reserve)" annotation so SUMMARY_RESERVE retuning doesn't
  leave the text stale.

* fix(core): address PR #4345 round-3 + round-4 review feedback

R3-1: rewrite the stale "Hard-tier rescue resets the counter" comment in
the reactive-overflow path. The R2 commit removed the pre-call reset
from hard-rescue; the only counter-reset path is now the post-call
COMPRESSED branch in tryCompress. Two contradicting comments in the
same file would mislead a future maintainer tracing the lifecycle.

R3-2: rewrite the JSDoc on CompactionThresholds.hard. The "(resets
failure counter)" phrasing was true under the pre-R2 design; after R2
the hard threshold force-triggers compaction and bypasses the breaker,
but does not reset the counter (which only happens on COMPRESSED
success via the post-call branch). The type is consumed by both
geminiChat and the CLI UI (via ContextThresholds alias), so the
authoritative description had to match the actual contract.

R3-3: add a Step 3 to the hard-rescue regression test. The test title
claims "success recovers via the post-call branch" but the original
Steps 1-2 only verified the latched counter was forwarded INTO the
call. Step 3 follows up with a below-hard send and asserts the
forwarded counter is 0 — proving geminiChat.ts:614 ran on the
COMPRESSED result.

R3-4: assert effectiveWindow === 0 on the existing extreme-small-window
test and add a separate zero-window edge case. The Math.max(0, ...)
clamp from R2 was previously unasserted; a regression that removed
the clamp would go undetected.

R4-1: forward originalTokenCount on the breaker-NOOP path in
chatCompressionService.compress() to match the adjacent
threshold-NOOP path (L368-369). Returning {originalTokenCount: 0,
newTokenCount: 0} masked "breaker tripped at N tokens" as
"empty session" in telemetry dashboards.

R4-2a: add debugLogger.warn at the two consecutiveFailures increment
sites (cheap-gate path L586 and reactive-overflow path L955) when
the counter reaches MAX_CONSECUTIVE_FAILURES. The breaker is one of
the PR's headline safety features but, prior to this round, had zero
observability when it tripped. Required importing MAX_CONSECUTIVE_FAILURES
into geminiChat.ts.

R4-3: programmatically link tokenEstimation.ts's CHARS_PER_TOKEN to
compactionInputSlimming.ts's TOKEN_TO_CHAR_RATIO. Both are 4 today
and represent the same generic char/token conversion. Exporting from
compactionInputSlimming and aliasing in tokenEstimation eliminates
the silent-drift hazard the JSDoc already warned about.

Declined (round-weighted bar at round 4):
- R3-5: debugLogger test for hard-rescue trigger — observability test
  coverage is overthinking at round 3+; the log is informational.
- R4-2b: expose breaker state in /context — new feature; out of scope.
- R4-4: render test for auto-tier marker — test coverage gap on
  working code, defer to follow-up PR per round-weighted bar.
- R4-5a: extract makeFakeChat/makeFakeConfig shared factory — pure
  test refactor at round 4, not a fix.
- R4-5b: direct unit test for precomputedEffectiveTokens — exercised
  indirectly via hard-rescue path tests in geminiChat.test.ts.
- R4-6: truncation-guard fallback test for missing candidatesTokenCount
  — code already has a TODO acknowledging the heuristic is imperfect
  (chatCompressionService.ts:549-553); defer.

* fix(core): address PR #4345 round-5 review feedback

R5-1: assert breaker-NOOP forwards originalTokenCount. R4-1 changed the
breaker-NOOP return from `{0, 0}` to `{originalTokenCount, originalTokenCount}`
so telemetry can distinguish "breaker tripped at N tokens" from
"empty session", but the existing test only checked compressionStatus
and newHistory. Now seeds a non-zero originalTokenCount (120K) and
asserts both fields forward it.

R5-2: forward originalTokenCount on the empty-history NOOP. This was
sibling drift on R4-1 — I fixed the cited breaker-NOOP site but missed
the empty-history NOOP. Of 5 NOOP return sites in chatCompressionService,
4 now forward originalTokenCount (breaker, threshold-gate, post-split,
min-compression-fraction) and 1 (this one) was still returning `{0, 0}`,
breaking the project-wide invariant. Now consistent.

R5-3: replace 10 stale line-number references with semantic anchors.
After the R3+R4 push, the line refs in my R2/R3 comments (`geminiChat.ts:614`,
`chatCompressionService.ts:339`, `line 992`, `L627`, `line 944`) no longer
pointed at their original targets — `geminiChat.ts:614` now points at
`setSystemInstruction`'s body, completely unrelated to compaction. The
pattern itself is fragile; semantic phrasing ("the post-call reset in
tryCompress's COMPRESSED handler") doesn't drift when lines shift.

347/347 affected core tests passing locally; typecheck clean.

* fix(core): address PR #4345 round-6 review feedback (R6 sweep)

R6-1: rewrite the stale JSDoc bullet on `consecutiveFailures` (the
"Hard-tier rescue failures" bullet). The old wording said "the counter
is reset to 0 BEFORE the rescue call" — that contradicted R5 which
explicitly removed the pre-call reset. Now the bullet matches the
actual behavior: counter is NOT pre-reset, force=true bypasses the
breaker, post-call COMPRESSED handler resets on success, reactive
overflow is the explicit-increment safety net.

My R5 stale-comment sweep only grep'd inline `//` comments; this JSDoc
on the field declaration slipped through. Re-audited "reset to 0
BEFORE" / "pre-reset" across both packages — single site remaining.

R6-7: assert `passedOpts.trigger === 'auto'` in the hard-rescue test.
This field is the orphan-strip safety wire added by the C1 fix (the
service's `compactTrigger === 'manual'` check would otherwise strip
the trailing active funcCall mid tool-loop). The test asserted force
and pendingUserMessage but not the trigger; a refactor dropping the
'auto' from `trigger: shouldForceFromHard ? 'auto' : undefined` would
silently break orphan-strip safety. Now regression-guarded with a
single-line expect.

164/164 affected core tests passing locally.

Declined per round-weighted bar (round 6 defaults Suggestion / Test
coverage / Style to overthinking):
- R6-2/3/6: test-coverage gaps on working code — defer to follow-up
- R6-4: redundant truthy guard on always-set fields — style nit
- R6-5: text-vs-UI inconsistency on /context — existing test enforces
  current behavior; treat as design decision (offer follow-up if
  reviewer escalates)
- R6-8 (tipRegistry small-window context-high): explicitly closed in
  design doc's Open Question 2 — small windows have empty context-high
  band by design; UI work is out-of-scope for this PR
- R6-9: wasted clone on rare fallback path — Suggestion-level perf
- R6-10 (CompressionMessage missing case): file not in this PR's diff;
  reviewer themselves proposed it as follow-up
2026-05-25 21:11:08 +08:00
zhangxy-zju
ed14a33064
feat(core): add NotebookEdit tool for Jupyter notebooks
Some checks are pending
Qwen Code CI / Classify PR (push) Waiting to run
Qwen Code CI / CodeQL (push) Blocked by required conditions
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
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
Adds NotebookEdit as the structured write counterpart to existing notebook read support.

Summary:
- Add `notebook_edit` for safe cell-level `.ipynb` replace/insert/delete operations.
- Integrate notebook editing with tool registration, permissions, Claude conversion, prior-read enforcement, IDE/inline modify flow, commit attribution, docs, and SDK permission docs.
- Harden notebook read/edit behavior for truncated notebook renders, ambiguous fallback cell IDs, internal modify metadata, compact JSON, UTF-8 BOM notebooks, and cache behavior after structural edits.
- Add unit and integration coverage for notebook read/edit behavior.

Follow-up work remains for tab-indented notebook formatting preservation, a few low-risk unit-test additions, and non-blocking hardening suggestions from review.
2026-05-21 00:06:15 +08:00
Yan Shen
9985d91e08
feat(cli): add configurable plansDirectory for Plan Mode (#4062)
* feat(cli): add configurable plansDirectory for Plan Mode

Add a plansDirectory setting that allows users to define a custom
directory for approved Plan Mode files. Relative paths are resolved
against the project root and validated to prevent path traversal.

- Storage: add isPathWithinDirectory() with realpathSync-based symlink
  resolution to prevent traversal bypass attacks (direct, intermediate,
  and cross-drive)
- Config: cache plansDir at construction time, use atomic write
  (write-temp then rename) to prevent corrupted plan files on crash
- CLI: respect bareMode by clearing plansDirectory in minimal mode
- Docs: document plansDirectory with requiresRestart and gitignore hint
- Tests: 26 new tests covering path validation, symlink attacks
  (direct and intermediate), Windows cross-drive paths, mixed
  separators, and configuration integration

Closes #3548

* fix(core): align symlink test with return value

* fix(core): harden plans directory handling

* fix(config): address PR #4062 review findings for plansDirectory

- Handle EXDEV during atomic plan writes (cross-device rename fallback)

- Sanitize session IDs to prevent path traversal in plan filenames

- Expand tilde (~) in configured plansDirectory paths

- Preserve plansDirectory in bare mode

- Add EACCES/EPERM handling to getPlanFileNames with user-visible warnings

- Close TOCTOU gap with post-write path containment validation

- Fix docs to clarify plansDirectory is a top-level key

- Add happy-path I/O tests for configured plansDirectory
2026-05-17 19:43:24 +08:00
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.
2026-05-17 00:36:48 +08:00
ChiGao
d343e2c15e
feat(perf): progressive MCP availability — MCP no longer blocks first input (#3994)
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(perf): progressive MCP availability — MCP no longer blocks first input

Today `Config.initialize()` runs MCP discovery synchronously and the cli
can't accept input until every configured MCP server finishes its
discover handshake. One slow or hung server bottlenecks every user with
MCP configured. Validated by the profiler instrumentation added in this
PR (set `QWEN_CODE_PROFILE_STARTUP=1` to reproduce):

| User scenario             | Time to first prompt input |
| ------------------------- | -------------------------- |
| No MCP                    | ~480 ms                    |
| 1 fast MCP                | ~875 ms                    |
| 2 fast + 1 slow MCP       | **~7.1 s**                 |
| 1 hung MCP server         | **~10.5 s**                |

(Measured on macOS arm64 / Node 24.15, n=30/fixture, p50.)

`Config.initialize()` now passes `{ skipDiscovery: true }` to
`createToolRegistry` by default and kicks off MCP discovery in a
fire-and-forget background path. As each server completes discover,
the cli's `AppContainer` debounces `setTools()` calls into one-frame
(16 ms) batches so the model sees the consolidated tool list shortly
after each server settles. Rollback: `QWEN_CODE_LEGACY_MCP_BLOCKING=1`.

- `packages/core/src/config/config.ts` — `Config.initialize` switches
  to `skipDiscovery: true` + new `startMcpDiscoveryInBackground()`
  (defensive against partially-stubbed `ToolRegistry` in tests). Adds
  `MCPServerConfig.discoveryTimeoutMs` (last positional ctor param —
  doesn't shift existing call sites). Tool-call timeout is untouched.
- `packages/core/src/tools/tool-registry.ts` — new
  `getMcpClientManager()` getter so the background path can call the
  incremental discover directly without going through `discoverMcpTools`
  (which would wipe already-registered tools).
- `packages/core/src/tools/mcp-client-manager.ts` —
  `discoverAllMcpToolsIncremental` now: emits `mcp-client-update`
  after IN_PROGRESS transition, wraps each per-server discover in a
  discovery-only timeout (stdio 30s, remote 5s), emits trailing
  `mcp-client-update` after COMPLETED so UI subscribers see the
  terminal state.
- `packages/cli/src/ui/AppContainer.tsx` — new `useEffect` (gated on
  `isConfigInitialized`) subscribes to `mcp-client-update` and
  16ms-batches `setTools()` calls. Same effect also defers
  `finalizeStartupProfile` until MCP settles (or 35s hard cap), so
  startup-perf profiles capture the full MCP timeline.

Activated only by `QWEN_CODE_PROFILE_STARTUP=1`; when unset every
profiler entry point short-circuits in a single null/flag check and
returns. Heisenberg overhead measured at -1.12% Δp50 between
profile-on vs profile-off (Welch p=0.092, n=30/config × 3 configs) —
within statistical noise.

- `packages/cli/src/utils/startupProfiler.ts` — extended with
  `events` array (multi-fire), `recordStartupEvent`,
  `setInteractiveMode`, `derivedPhases`, per-checkpoint heap snapshots,
  `MAX_EVENTS` cap, and `QWEN_CODE_PROFILE_STARTUP_OUTER` / NO_HEAP
  env opt-ins. + 7 new tests.
- `packages/core/src/utils/startupEventSink.ts` (new) — minimal
  cross-package sink so `core` can emit profiler events without
  reverse-depending on `cli`. No-op when no sink registered. + 4 tests.
- `packages/core/src/index.ts` — export `setStartupEventSink` /
  `recordStartupEvent` / type aliases.
- `packages/cli/src/gemini.tsx` — registers the sink at `main()`
  entry, adds `first_paint` checkpoint after Ink render, calls
  `setInteractiveMode(true)` in the interactive branch.
- `packages/core/src/config/config.ts` — emits
  `tool_registry_created`.
- `packages/core/src/core/client.ts` — emits `gemini_tools_updated`
  at the end of `setTools()`.
- `packages/core/src/tools/mcp-client-manager.ts` — emits
  `mcp_discovery_start`, `mcp_server_ready:<name>`,
  `mcp_first_tool_registered`, `mcp_all_servers_settled`.
- `packages/cli/src/ui/AppContainer.tsx` — emits
  `config_initialize_start`, `config_initialize_end`, `input_enabled`.

`Config.initialize()` now returns BEFORE MCP discovery completes.
Things to check:
- Any code path that assumed "after `config.initialize()`, all MCP
  tools exist in the registry" — these will see only built-in tools
  initially; new tools appear via `mcp-client-update` events.
- `MCPDiscoveryState.COMPLETED` is now set asynchronously instead of
  synchronously after `initialize()` resolves.
- Model requests issued before MCP settles see only built-in tools;
  subsequent requests see the full set as servers come online.
- Tests that assert MCP tool count immediately after
  `config.initialize()` should wait for the `mcp-client-update` with
  COMPLETED discoveryState instead.

- 313 impacted-area tests green (config / mcp-client-manager / client
  / startupProfiler 18 / startupEventSink 4).
- `tsc --noEmit` clean for `packages/core` and `packages/cli`.
- `eslint` clean on touched files.
- Manual: `QWEN_CODE_PROFILE_STARTUP=1 SANDBOX=1` interactive run
  produces a JSON profile in `~/.qwen/startup-perf/` containing
  `first_paint`, `config_initialize_start/end`, `input_enabled`,
  MCP per-server events, and `gemini_tools_updated`. See PR
  description's "How to validate" section.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(core): harden progressive MCP discovery against silent regressions

Addresses review feedback on PR #3994:

- Skip user-disabled servers in discoverAllMcpToolsIncremental. The new
  incremental path used to iterate Object.entries(servers) without
  consulting isMcpServerDisabled, so a server the user had explicitly
  turned off would still get connected and its tools registered.
  Mirrors the existing protection in discoverAllMcpTools.

- Disconnect the underlying client when runWithDiscoveryTimeout fires.
  Without this, the inner discoverMcpToolsForServer kept running after
  the timeout rejected the outer promise — if discover() eventually
  succeeded it would register the late server's tools into the live
  toolRegistry (a silent registration vector, especially exploitable
  with a 0/negative discoveryTimeoutMs override).

- Clamp discoveryTimeoutMs to [100ms, 300_000ms]. 0/negative/Infinity
  values previously passed through to setTimeout unvalidated and made
  the silent-registration bug above trivially reachable.

- Classify the `tcp` (WebSocket) transport field as remote so hung WS
  handshakes use the 5s default instead of the 30s stdio default.

- Defensive delete of serverDiscoveryPromises[name] in the per-server
  catch so a doomed/orphan entry can't briefly short-circuit a
  subsequent discoverMcpToolsForServer call.

Adds focused tests for each fix.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): restore runtime.json sidecar and harden non-interactive MCP visibility

Addresses review feedback on PR #3994:

- Restore writeRuntimeStatus + markRuntimeStatusEnabled in
  startInteractiveUI. The progressive-MCP diff inadvertently dropped
  the runtime.json sidecar write from the interactive entry point,
  leaving Config.refreshSessionId()'s session-swap refresh as dead
  code and silently breaking external integrations (terminal
  multiplexers, IDE integrations, status daemons) that map PID →
  sessionId via runtime.json.

- Add Config.getFailedMcpServerNames() and surface a stderr warning
  in --prompt / stream-json / ACP entry points when one or more MCP
  servers failed during background discovery. Per-server errors are
  caught inside discoverAllMcpToolsIncremental and never reached a
  TTY otherwise, so a script using non-interactive mode with broken
  MCP config would silently run with only built-in tools — a
  regression vs the legacy synchronous path.

- Pass the parsed `settings` object through to
  runNonInteractiveStreamJson. The new call site dropped the
  argument, falling back to createMinimalSettings() and losing any
  user-configured permission / approval / hook setup for stream-json
  sessions. Added regression assertion to gemini.test.tsx.

- Move finalizeStartupProfile out of gemini.tsx's stream-json branch
  and into Session.ensureConfigInitialized so it runs AFTER
  config.initialize() / waitForMcpReady() in stream-json. Previously
  the profile was finalized before any MCP / config_initialize_*
  events were emitted, producing empty stream-json profiles.

- Gate setStartupEventSink registration on isStartupProfilerEnabled()
  so core-side recordStartupEvent calls short-circuit at the first
  null-check when profiling is disabled, instead of going through an
  arrow wrapper and the profiler's own enabled gate.

- Tighten the type-unsafe ToolRegistry cast in
  startMcpDiscoveryInBackground to preserve the typed return signature
  so a rename of getMcpClientManager would be flagged at this call
  site (kept the optional-chain guard for tests that stub
  ToolRegistry as a plain object).

- Re-document first_paint as "render call returned" so consumers don't
  confuse Ink's synchronous render() return with literal pixel paint.
  Kept the checkpoint name for backward compatibility with collected
  profiles.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): restore resize repaint and pin gemini_tools_lag capture in AppContainer

Addresses review feedback on PR #3994:

- Restore the terminal-resize useEffect that calls
  repaintStaticViewport() when terminalWidth changes. The progressive-
  MCP diff removed previousTerminalWidthRef + the repaint useCallback
  + the resize useEffect, so tmux pane resizes and fullscreen toggles
  leave the static region rendered at the old width — header content
  visibly tears until something else triggers refreshStatic.

- Pin the gemini_tools_lag startup metric. The previous onMcpUpdate
  handler called finalizeOnce() synchronously when discovery reached
  COMPLETED, but the pending setTools() batch was still 16ms away.
  setTools() emits `gemini_tools_updated` — when finalize ran first
  the profile's `finalized` guard suppressed that event, so
  gemini_tools_lag came out undefined in interactive mode. New
  onMcpUpdate flushes setTools() NOW on COMPLETED and only finalizes
  after the flush resolves, guaranteeing the event lands.

- Log setTools() batch-flush errors via debugLogger instead of
  silently swallowing them. GeminiClient.setTools() has no try/catch
  around warmAll() / getFunctionDeclarations() / getChat().setTools();
  the previous `.catch(() => {})` would have hidden production
  tool-registration regressions completely.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(core): correct MCP failure visibility and incremental cleanup

Addresses three review findings on PR #3994:

- McpClient.discover() now flips the client status to DISCONNECTED before
  re-throwing. Previously, a server that connected successfully but whose
  discoverPrompts / discoverTools then rejected (or that returned no
  prompts and no tools) would remain CONNECTED in the global status
  registry. Config.getFailedMcpServerNames() filters by
  `status !== CONNECTED`, so such servers were silently omitted from the
  non-interactive failure banner and the Footer's MCP health pill kept
  counting them as healthy.

- discoverAllMcpToolsIncremental no longer records `outcome: 'ready'`
  for servers whose connect/discover threw. The inner
  discoverMcpToolsForServerInternal catches errors without re-throwing
  (best-effort discovery semantics), so the try block resolved even for
  failures — only the runWithDiscoveryTimeout path reached the catch.
  Auth errors, server crashes, and missing-tools responses were therefore
  recorded as success in the startup profile. We now consult the actual
  server status (now correctly DISCONNECTED after the first fix) before
  emitting `ready`, and emit `outcome: 'failed'` otherwise.
  `mcp_first_tool_registered` is gated on the same check so a failed
  server can't pollute that user-facing metric.

- discoverAllMcpToolsIncremental tears down enabled→disabled mid-session
  transitions. When a previously-connected server is disabled (e.g. via
  `/mcp disable foo` or by editing settings), the incremental path used
  to just `continue` past it, leaving its client, tools, health check,
  and global status entry in place. Now calls removeServer() for any
  already-known client we encounter in the disabled branch.

Adds focused tests for each fix.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs(core): clarify ToolRegistry cast comment in startMcpDiscoveryInBackground

Addresses review feedback on PR #3994. The previous comment claimed the
call site uses "no defensive cast" but the code still casts via
`as ToolRegistry & { getMcpClientManager?: ... }`. Reword to explain
the cast's actual purpose: it exists only because some tests stub
ToolRegistry as a plain object, so we use optional chaining to avoid
crashing the init path when those tests run. Also note that the inner
shape now uses `ReturnType<ToolRegistry['getMcpClientManager']>` — a
future rename of the production method still surfaces as a type error
at this call site rather than silently falling through to the
`if (!manager)` branch.

Comment-only change; no behavior diff.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(core): close MCP timeout TOCTOU race and propagate disconnect status

Addresses two critical findings on PR #3994 round 6:

- runWithDiscoveryTimeout no longer uses fire-and-forget disconnect. The
  prior `void client.disconnect()` returned before `transport.close()`
  landed, leaving a window where an in-flight `discover()` could pump
  `tools/list` through the transport and synchronously register tools
  into the live registry BEFORE the close took effect. The earlier fix
  comment described this as a "remote-exploitable silent-tool-registration
  vector"; the await closes the timing window but doesn't help if tools
  already landed, so we also drop them with `removeMcpToolsByServer()`
  after the disconnect resolves. No-op when discover hadn't reached
  registration yet.

- McpClient.disconnect() now writes DISCONNECTED to the global registry
  directly. Previously, `isDisconnecting = true` was set BEFORE the
  internal `updateStatus(DISCONNECTED)` call, and `updateStatus`'s guard
  (designed to suppress LATE writes from a stale `connect()` catch)
  silently swallowed the write. The global stayed CONNECTED forever for
  timeout-disconnected servers, so `Config.getFailedMcpServerNames()`
  (which filters `status !== CONNECTED`) omitted them from the
  non-interactive failure banner and the Footer's MCP health pill kept
  counting them as healthy. This invalidated the round-5
  `getMCPServerStatus === CONNECTED` gate, which would always pass the
  "ready" check for timed-out servers. The guard stays in place for its
  original purpose; the legitimate disconnect→DISCONNECTED notification
  now bypasses it by writing the registry directly.

Also adds the `config_initialize_start` / `_end` profiler checkpoints
to `Session.ensureConfigInitialized()` so stream-json startup profiles
include the same derived `config_initialize_dur` phase as the
non-stream-json branch in gemini.tsx (round 6 [Suggestion]).

Tests cover (a) the disconnect-and-cleanup path on timeout and (b) the
intentional-disconnect global registry propagation regression.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(mcp): surface failures + prevent health-check resurrection of timed-out servers

Round-7 review follow-ups:

- AppContainer (interactive): MCP startup failures now route through
  debugLogger.warn on COMPLETED. Was silent — only debug logs / profile
  events surfaced failures, so regular interactive users got no
  indication their MCP servers failed. Mirrors the non-interactive
  stderr warning, adjusted to debugLogger so it doesn't collide with
  Ink's rendered output.

- acpAgent per-session: `QwenAgent.initializeConfig()` now emits the
  same `Warning: MCP server(s) failed to start` stderr line as the
  top-level `runAcpAgent` path. Previously per-session ACP configs
  with failed MCP servers silently fell back to built-in tools.

- mcp-client-manager timeout handler: after disconnecting an
  intentionally timed-out server, also drop it from `this.clients` and
  stop any pending health-check timer. Without this the discovery
  `finally` block would arm a health-check that detected DISCONNECTED
  status and called `reconnectServer()` → `discoverMcpToolsForServer()`
  directly — bypassing `runWithDiscoveryTimeout` entirely and silently
  resurrecting the slow server. `startHealthCheck` also early-returns
  for unknown servers so the trailing finally-block call is a no-op.

- startupEventSink: silent `catch {}` now logs via `debugLogger.error`
  so a corrupted sink doesn't silently drop every subsequent event.
  Quiet by default; visible under `QWEN_CODE_DEBUG=1`.

Tests:
- mcp-client-manager.test.ts: regression for the timeout → no-reconnect
  invariant (clients map purged + health-check timer absent).
- acpAgent.test.ts: per-session newSession surfaces failures to stderr,
  and stays safe when Config lacks `getFailedMcpServerNames`.

Declines (with reasoning in PR reply):
- [Critical] AppContainer batch-flush useEffect untested → re-flag of
  the round-5 deferral that wenshao acknowledged at the time. Lower-
  layer invariants (this PR's mcp-client-manager + mcp-client tests)
  pin the dependent contracts. The component-test harness for timers +
  event emitters in this file is non-trivial and out of scope; tracked
  for a follow-up.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

---------

Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-05-13 22:17:16 +08:00
Edenman
533daac316
feat(cli): wrap markdown links in OSC 8 so wrapped URLs stay clickable (#4037)
* feat(cli): wrap markdown links in OSC 8 so wrapped URLs stay clickable

Long URLs the model emits inside `[label](url)` or as bare `https://...`
get line-wrapped by the terminal, which prevents most emulators from
detecting them as a single clickable region. OSC 8 hyperlinks decouple
the link target from the visible label so the entire label remains one
clickable target regardless of where it wraps.

- Extract the existing OSC 8 helpers from AuthenticateStep into a shared
  packages/cli/src/ui/utils/osc8.ts util, plus a dependency-free
  capability detector that honors NO_COLOR / FORCE_COLOR=0 / CI /
  non-TTY stdout, with FORCE_HYPERLINK=1 and QWEN_DISABLE_HYPERLINKS=1
  overrides for explicit opt-in / opt-out.
- Wire InlineMarkdownRenderer to wrap markdown link labels and bare
  autolinks in an OSC 8 envelope when supported. Wrapping happens after
  the inline link token has been fully matched, so streamed partial
  chunks cannot split an envelope across flushes.
- Fall back to the legacy `label (url)` rendering byte-for-byte when
  the host terminal does not advertise OSC 8 support.

Closes #3954

* fix(cli): harden OSC 8 markdown wrapping after multi-round audit

Address findings from a multi-round design and code audit of the OSC 8
hyperlink feature:

Design fixes:
- Keep the visible `(url)` suffix in supported terminals too — preserves
  copy-paste UX and lets users preview suspicious URLs before clicking.
  OSC 8 is now purely additive (byte-identical unsupported output, plus
  envelope on supported terminals).
- Restrict OSC 8 wrapping to http/https/mailto/ftp/sftp/ssh schemes;
  javascript:/data:/file:/vbscript: fall through unwrapped so the user
  can read the target. Prompt-injection defense for LLM output.
- Reject URLs with whitespace — every terminal treats whitespace in an
  OSC 8 target as truncation/rejection, which would turn the whole
  region into an un-clickable trap.
- Block OSC 8 inside tmux/screen by default; require `FORCE_HYPERLINK=1`
  opt-in. The multiplexer hides the host terminal's capabilities, so
  emitting passthrough escapes on a host without OSC 8 prints garbage.
- Version-gate `supportsHyperlinks()` (iTerm ≥3.1, vscode ≥1.72, WezTerm
  ≥20200620, VTE ≥0.50 with 0.50.0 segfault carve-out), block CI /
  TEAMCITY / win32 (modulo WT_SESSION/Kitty/Ghostty/DOMTERM), mirror
  `supports-hyperlinks` semantics.
- Extend the link regex to allow one level of balanced parens in the
  URL group so `[wiki](https://en.wikipedia.org/wiki/Foo_(bar))` isn't
  truncated at the inner `)`.
- Trim trailing sentence punctuation off the OSC 8 *target* for bare
  URLs (`.`, `,`, `;`, `:`, `!`, `?`, `'`, `"`, `` ` ``) and unbalanced
  trailing `)]}` so the clickable URL resolves to a real page.
- Catch VTE 0.50.0 reported in packed form (`'5000'`) — the original
  string compare missed it and let the segfault through.

Code fixes:
- Consolidate `wrapForMultiplexer` with the pre-existing
  `packages/cli/src/utils/osc.ts` — no more duplicate helpers.
- Drop the `supportsHyperlinks` memoization cache so runtime env changes
  (NO_COLOR / theme toggles) take effect immediately.
- Extract `MD_LINK_PATTERN`, `MD_LINK_CAPTURE`, `shouldWrapMarkdownLink`,
  and `HYPERLINK_ENV_KEYS` into `osc8.ts` so the React and ANSI
  renderers stay in lockstep.
- Hoist `supportsHyperlinks()` once per render (both renderers).
- Apply the same OSC 8 treatment to `TableRenderer` so markdown links
  inside tables are clickable too.
- Rewrite `trimTrailingUrlPunctuation` to O(n) by pre-counting opens.

Tests cover: balanced parens in URL, dangerous-scheme rejection,
whitespace-URL rejection, trailing-punctuation trimming, tmux blocking,
version gating (iTerm/WezTerm/vscode/VTE incl. packed form), platform
fallbacks, mid-stream chunk balance, byte-identical legacy fallback.

* feat(cli): detect Alacritty / Konsole / Warp / JetBrains / mintty for OSC 8

Expand supportsHyperlinks() to recognize five more capable terminals
that the original detector silently treated as unsupported:

- Alacritty ≥ 0.11 via TERM=alacritty (the issue explicitly calls this
  one out)
- Konsole ≥ 21.04 via KONSOLE_VERSION
- WarpTerminal via TERM_PROGRAM=WarpTerminal
- JetBrains JediTerm (IDE integrated terminals) via TERMINAL_EMULATOR
- mintty (Git Bash on Windows, etc.) via TERM_PROGRAM=mintty

Hyper stays auto-detection-off (FORCE_HYPERLINK=1 override) because
plugin chains have a long history of breaking escape passthrough.
Apple_Terminal stays off because it has no OSC 8 support at all.

KONSOLE_VERSION and TERMINAL_EMULATOR added to HYPERLINK_ENV_KEYS so
the test isolation list stays in sync.

* chore(cli): polish OSC 8 detector after another audit round

Address findings from the final multi-round audit pass:

- Document `FORCE_HYPERLINK` and `QWEN_DISABLE_HYPERLINKS` in the
  user-facing env-vars table at docs/users/configuration/settings.md so
  the new opt-in / opt-out surface is discoverable without grepping
  source.

- Detect Alacritty even when the alacritty terminfo entry isn't
  installed (a common Linux distro scenario where Alacritty falls back
  to TERM=xterm-256color). Fall back to ALACRITTY_LOG /
  ALACRITTY_WINDOW_ID / ALACRITTY_SOCKET — Alacritty sets at least one
  of these unconditionally since 0.12.

- Trim a trailing `>` off the OSC 8 target so CommonMark autolinks
  (`<https://example.com>`) produce a clickable target that actually
  resolves instead of 404-ing because of the captured delimiter.

- Add OSC 8 / hyperlink env isolation to TableRenderer.test.tsx so a
  developer running the suite from iTerm2 / WezTerm / Kitty can't leak
  escape bytes into table output.

- Symmetric `isTTY` reset in osc8.test.ts `beforeEach` so the early
  describes (sanitizer, scheme, trim) don't inherit residual TTY state
  from a prior test.

- Document the deliberate security property of keeping the visible
  `(url)` suffix in OSC 8 mode (user always reads the destination
  before clicking) in the SAFE_OSC8_SCHEMES comment.

- Collapse the `wrapForMultiplexer` import + re-export to a single
  `export { wrapForMultiplexer }` after the local import.

- Add ALACRITTY_* keys to HYPERLINK_ENV_KEYS so test isolation lists
  stay complete.

Tests cover the new autolink `>` trim, the Alacritty env-var
fallbacks, and NBSP / Unicode-whitespace URL rejection.

* fix(cli): tighten OSC 8 gating per PR review

Two fixes from chiga0's review on PR #4037:

1. Move the non-TTY check above `FORCE_HYPERLINK` so a user with
   `FORCE_HYPERLINK=1` in their shell profile still gets a clean pipe
   when they run `qwen | cat` or `qwen > out.txt`. The "non-TTY stdout
   must suppress escapes" acceptance criterion now holds even under
   forced enable.

2. Version-gate the Konsole detection at `>= 21.04`. KONSOLE_VERSION
   is set by every Konsole release including ones that pre-date OSC 8
   support, so the existence check alone false-positives on Konsole
   20.x. Parse the packed integer (21.04 → 210400) and let older
   releases fall through to the legacy fallback.

Updates the docs row for FORCE_HYPERLINK to make the non-TTY caveat
explicit. Splits the prior "FORCE_HYPERLINK + isTTY=false" test into
two — one verifying force works on a TTY, one asserting it never
escapes the non-TTY guard. Adds a Konsole < 21.04 regression test.

* fix(cli): stop auto-detecting Warp Terminal as OSC 8 capable

Warp's current rendering engine doesn't honor OSC 8 envelopes — the
escape sequence is printed as visible garbage rather than recognized
as a clickable hyperlink. Falling through to the legacy `label (url)`
rendering avoids the regression on Warp.

Users on a Warp build that ever ships OSC 8 support can opt in with
`FORCE_HYPERLINK=1`; the case will be reinstated in the switch when
Warp lands real support upstream.

Test flipped from "enabled" to "not auto-detected, FORCE_HYPERLINK
opts in" to lock the new behavior.

* feat(cli): drop visible (url) suffix when OSC 8 wrapping is active

In the originally shipped renderer, `[label](url)` was rendered as
`label (url)` even when OSC 8 wrapped the region. With long URLs that's
clutter for no benefit — capable terminals already expose the target
via hover / status bar / right-click "copy link" without needing the
URL in the visible stream.

When `shouldWrapMarkdownLink(url, canHyperlink)` returns true, the
React renderer and the ANSI table renderer now emit only the markdown
label (link-colored), with the OSC 8 envelope pointing at the full URL.
Empty labels (`[](url)`) fall back to using the URL as the visible
label so the link stays discoverable.

When the predicate returns false (unsupported terminal, unsafe scheme,
whitespace URL) the legacy `label (url)` rendering is preserved
byte-for-byte — the scheme allowlist still guarantees the user sees
the destination before any click on a `javascript:` / `data:` / etc.
link.

Tests updated to assert label-only visible bytes in wrap mode and an
empty-label fallback case added. Comment block in `osc8.ts` updated to
reflect the new visibility contract.

* fix(cli): strip C1 controls in OSC 8 sanitizer

sanitizeForOsc() only removed C0 + DEL, so 8-bit ST (\x9c) and 8-bit
OSC (\x9d) bytes could still survive inside an OSC 8 target. On
terminals that honor C1 controls, those bytes act as the same sequence
boundaries as their two-byte ESC counterparts, which defeats the
escape-injection hardening this helper is meant to provide. Extend
the regex to also strip \x80-\x9f and cover the case with a test.

* fix(cli): harden OSC 8 link sanitization and tighten gating

Three independent issues found while auditing the markdown OSC 8 path:

1. sanitizeForOsc() previously left Unicode bidi controls (U+200E/F,
   U+202A-E, U+2066-9) and line/paragraph separators (U+2028/9) intact.
   A model-emitted RLO in a link label visually reverses trailing bytes,
   spoofing the host the user thinks they're clicking — exactly the
   click-deception attack the scheme allowlist is meant to block, just
   moved from the URL into the visible label. Extend the regex to strip
   those bytes too.

2. The visible label rendered inside the OSC 8 envelope went straight
   to the terminal without sanitization, so even with (1) the spoof
   would still land. Wire sanitizeForOsc() over the linkText in both
   InlineMarkdownRenderer and TableRenderer's OSC 8 branches. The
   legacy `label (url)` branches stay untouched so today's
   unsupported-terminal output remains byte-identical.

3. AuthenticateStep emitted osc8Hyperlink(authUrl) unconditionally,
   leaking escape bytes into pipes / non-OSC-8 terminals — inconsistent
   with the suppression contract documented for the rest of the PR.
   Gate it on supportsHyperlinks() so it falls back to the bare URL.

Test coverage added:
- sanitizeForOsc bidi/line-separator strip
- bidi spoof in the rendered markdown label
- byte-equality fallback on unsupported terminals
- TableRenderer markdown link → OSC 8 (positive, fallback, unsafe
  scheme, bidi-spoof) — the table renderer had zero OSC 8 coverage
  before this.

* fix(cli): keep `(url)` visible when an OSC 8 label looks like a different URL

Adversarial round-2 audit identified a label-as-URL deception attack:
when the OSC 8 branch elides the `(url)` suffix and shows only the
clickable label, a model-emitted `[https://google.com](https://attacker.com)`
renders a "google.com" link that resolves to attacker.com. Pre-OSC-8
rendering kept `(url)` visible so the user could see the real target;
hiding it makes the click-deception case land.

Mitigation: a new `labelMayDeceive(label, url)` predicate. When the
label contains a URL-shaped substring AND it doesn't equal the actual
target, both renderers keep the legacy `(url)` suffix while still
emitting the OSC 8 envelope — the link stays clickable, the user
still sees where the click goes.

Heuristic is permissive on purpose: false positives are harmless
(redundant `(url)` on niche labels), false negatives let a real spoof
through.

Tests: positive (mismatched URL labels), negative (label == url, plain
text labels), in both InlineMarkdownRenderer and TableRenderer.

* fix(cli): catch bare-host label deception in OSC 8 wrapping

Round-3 audit caught a false-negative in labelMayDeceive: the
`://` substring check only flagged labels with a fully-qualified URL
shape. The most natural markdown spoof — `[google.com](https://evil.com)`
— uses a bare host as the label and slipped past, so the OSC 8 branch
elided the `(url)` suffix and rendered a clickable "google.com" that
resolved to evil.com.

Add a third detection pattern: extract host-like tokens from the
label (`name.tld` with an alphabetic 2+ char TLD), and flag the link
when any of them doesn't equal the URL's parsed hostname. Plain
labels like `docs` / `click here` don't match the regex, version
strings like `1.2.3` are skipped (last segment is numeric), and
`[google.com](https://google.com)` is honest rendering — none of these
get flagged.

ASCII-only matching means an IDN-homograph attack on a bare-host label
(Cyrillic `о`) still escapes this layer; the fully-qualified form of
the same attack is still caught by the existing `://` rule, which is
the only form an LLM is realistically likely to emit.

Tests cover: bare-host mismatch, punycode IDN target, same-host /
different-path, label==target negative, plain-text labels, version
strings.

* fix(cli): handle mailto: target in labelMayDeceive

Round-4 audit caught a false positive: `new URL('mailto:x@y').hostname`
is empty, so targetHostname() returned undefined and the defensive
`return true` branch fired any time a mailto label contained an
email-shaped string. A perfectly honest
`[support@example.com](mailto:support@example.com)` was being flagged
as deceptive and getting a redundant `(url)` suffix on capable
terminals.

Special-case mailto: by pulling the domain from after the `@` in the
URL pathname, matching what the user would compare against.
A mismatched mailto (e.g. `[support@example.com](mailto:abuse@evil.com)`)
still flags correctly.

Also drop a dead `HOST_LIKE_RE.lastIndex = 0` reset — `.match()` doesn't
consult lastIndex, so the line was a no-op.

* fix(cli): catch IPv4-literal label deception in OSC 8 wrapping

Round-5 audit found another bare-host bypass: a label like
`[1.1.1.1](https://attacker.com)` (or any other dotted-quad such as
`[192.168.1.1]` / `[8.8.8.8]`) escaped labelMayDeceive because the
existing host regex anchors on a 2+ alphabetic TLD. The user would
see a clickable "1.1.1.1" that resolves to attacker.com with no
visible target.

Add a separate dotted-quad pattern and combine it with the host-token
list before comparing against the URL's hostname. False-positive
surface is small (over-permissive on octet ranges is harmless — worst
case is an extra `(url)` suffix on a label like `999.999.999.999`).

Tests cover mismatched IPv4, IPv4 spelled inside surrounding text,
and label-equals-target IPv4 (which must NOT flag).

* fix(cli): sanitize URL when rendered as visible text in OSC 8 path

Two PR review findings:

1. config-utils.ts dropped the `resolvePath(...)` call (and its import)
   that origin/main introduced in #4045 for tilde / relative `cwd` paths
   in channel configs. The auto-merge silently reverted it the same way
   it did `packages/channels/base/src/index.ts`. Restore main's content.

2. Anti-spoof sanitization was only applied to `linkText`, but the
   OSC 8 render path emits the URL as visible text in two places that
   bypassed it:
   - empty-label fallback `safeLabel || url` — `[](https://x/a‮evil)`
     would print the URL with RLO intact even though the OSC target was
     sanitized.
   - deceptive-label `(url)` suffix.

   Compute `safeUrl = sanitizeForOsc(url)` once in the OSC 8 branch and
   use it for both visible-URL renderings. The OSC target inside
   `osc8Open` keeps the raw URL (sanitization happens inside the helper
   anyway). Same fix mirrored in `TableRenderer.tsx`. The legacy
   `label (url)` branch on unsupported terminals stays untouched so its
   byte-identical-fallback contract holds.

Test added: `[](https://example.com/a‮evil)` round-trips through the
renderer with the RLO stripped from both the OSC target and the visible
URL fallback.
2026-05-13 11:37:27 +08:00
jinye
aecea70114
docs(telemetry): align config and docs semantics for target, outfile, and CLI flags (#4066)
* docs(telemetry): align config and docs semantics for target, outfile, and CLI flags

- Remove stale warning note "This feature requires corresponding code
  changes" — the OTLP implementation is now complete (#3779, #4061)
- Clarify that `target` is an informational destination label and does
  not control exporter routing; `otlpEndpoint` or `outfile` must be set
  to configure where data is sent
- Mark `--telemetry-target` CLI flag as deprecated in the configuration
  table to match the deprecateOption() call in cli/src/config/config.ts
- Fix `outfile` / `QWEN_TELEMETRY_OUTFILE` descriptions: remove the
  incorrect "when target is local" qualifier — outfile overrides OTLP
  export regardless of the target value
- Simplify the file-based output example by removing the now-redundant
  `"target": "local"` and `"otlpEndpoint": ""` fields

Closes the "Align telemetry config and docs semantics for target,
useCollector, otlpEndpoint, otlpProtocol, and outfile" checklist item
in #3731.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* docs(telemetry): address Copilot review comments on outfile and target descriptions

- Fix outfile table row in telemetry.md: "overrides `otlpEndpoint`" →
  "overrides OTLP export" (outfile disables all OTLP exporting, not
  just the base endpoint)
- Use fully-qualified setting names (`telemetry.otlpEndpoint`,
  `telemetry.outfile`) in the target description in settings.md for
  consistency with the rest of the table

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)

* docs(telemetry): update QWEN_TELEMETRY_TARGET env var description and add outfile note

- Align QWEN_TELEMETRY_TARGET env var description with the updated
  telemetry.target setting semantics (informational label, not routing)
- Add a note after the file-based output example clarifying that outfile
  automatically disables OTLP export

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
2026-05-13 08:27:41 +08:00
jinye
32a49b4ddb
refactor(telemetry): remove dead useCollector setting and unreachable TelemetryTarget.QWEN (#4061)
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
useCollector was plumbed through config (interface, constructor, getter,
env var resolution) but never consumed by the telemetry SDK — the setting
had no runtime effect. TelemetryTarget.QWEN existed in the enum but
parseTelemetryTargetValue() only accepted 'local' and 'gcp', making
'qwen' unreachable (it would throw FatalConfigError).

Remove both dead code paths along with their tests and documentation.

Part of #3731
2026-05-11 23:22:53 +08:00
ChiGao
cadda23782
chore(deps): upgrade ink 6.2.3 → 7.0.2 + bump Node engine to 22 (#3860)
* chore(deps): upgrade ink 6.2.3 -> 7.0.2 + bump Node engine to 22

ink 7 requires Node >=22 and react-reconciler 0.33 with React >=19.2,
so this PR also bumps:

- Node engines (root + cli + core) 20 -> 22
- React/react-dom 19.1 -> 19.2.4 (pinned exact via overrides to keep
  the transitive React graph deduped to a single instance)
- @types/node pinned to 20.19.1 via overrides to avoid an unrelated
  Dirent NonSharedBuffer regression in sessionService tests
- @vitest/eslint-plugin pinned to 1.3.4 to avoid an unrelated lint
  regression introduced by the 1.6.x rule additions
- react-devtools-core 4.28 -> 6.1 (ink 7 peerOptional requires >=6.1.2)
- ink hoisted to root devDeps so workspace-private peer-dep contention
  doesn't push ink-link/spinner/gradient into nested workspace
  installs (which would skip transitive resolution for terminal-link)

Workflow + image + installer alignment:

- .nvmrc 20 -> 22
- Dockerfile node:20-slim -> node:22-slim
- CI test matrix drops 20.x (keeps 22.x + 24.x)
- terminal-bench workflow Node 20 -> 22
- Linux/Windows install scripts upgrade their Node version targets

Documentation alignment:

- README.md badge + prerequisites
- AGENTS.md, CONTRIBUTING.md, docs/users/quickstart.md,
  docs/users/configuration/settings.md, docs/developers/contributing.md,
  docs/developers/sdk-typescript.md, docs/users/extension/extension-releasing.md,
  packages/sdk-typescript/README.md, packages/zed-extension/README.md,
  scripts/installation/INSTALLATION_GUIDE.md

Test gating:

- Two AuthDialog/AskUserQuestionDialog tests that drive <SelectInput>
  through ink-testing-library now race ink 7's frame-throttled input
  delivery and land on the wrong option. The maintainers had already
  marked one of them unreliable (skip on Win32 + CI+Node20). Extend
  that gate to cover all environments until upstream
  ink-testing-library ships an ink-7-compatible release that flushes
  input deterministically. The other test now uses it.skip with the
  same comment. No business code changes.

Verified locally:

- npm run typecheck across all workspaces: clean
- npm run lint (root): clean
- npm run test --workspaces:
    cli  312/312 files, 4918 passed, 9 skipped
    core 266/266 files, 6836 passed, 3 skipped
    webui 6/6, 201 passed
    sdk  40/40, 283 passed, 1 skipped
- npm ls ink: single ink@7.0.2 instance across all peer deps
- single react@19.2.4 instance

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* chore: align Node 22 floor across all shipping artifacts

Reviewer (tanzhenxin) flagged five surfaces where the >=22 engine bump
leaked: SDK package metadata, web-templates engines, /doctor runtime
check, main bundler target, and SDK bundler target. Each was a separate
escape hatch letting Node 18/20 consumers install or run the artifact
on an unsupported runtime.

- packages/sdk-typescript/package.json: engines.node >=18.0.0 -> >=22.0.0
- packages/web-templates/package.json: engines.node >=20 -> >=22
- packages/cli/src/utils/doctorChecks.ts: MIN_NODE_MAJOR 20 -> 22
- esbuild.config.js: target node20 -> node22 (main CLI bundle)
- packages/sdk-typescript/scripts/build.js: target node18 -> node22 (esm + cjs)
- packages/cli/src/utils/doctorChecks.test.ts: rename test label to v22+

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* ci(e2e): bump E2E workflow Node matrix to 22.x

Reviewer (tanzhenxin) flagged that e2e.yml still pinned node-version
20.x while root engines is now >=22, so every E2E run on push would
either fail at npm ci with engine error or silently exercise the bundle
on a runtime that's no longer in ci.yml's test matrix.

The macOS job in the same workflow already reads .nvmrc (which is 22)
so this only updates the Linux matrix.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(deps): drop root wrap-ansi override so ink 7 gets its declared dep

Reviewer (tanzhenxin) flagged that the root overrides.wrap-ansi: 9.0.2
predates this upgrade and forces every consumer (including ink) to v9,
while ink 7 declares wrap-ansi: ^10.0.0. The lockfile had no nested
install under node_modules/ink/, so ink 7 was running with a transitive
dep one major below its declared minimum.

Dropping the global override lets ink resolve its own wrap-ansi 10
nested install (now visible in the lockfile under
node_modules/ink/node_modules/wrap-ansi), while the cli package's own
direct `wrap-ansi: 9.0.2` dependency keeps the cli code path
(TableRenderer.tsx) on the version it has been tested against. The
nested cliui override is preserved for yargs which still needs v7.

Verified via `npm ls wrap-ansi`:
- ink@7.0.2 -> wrap-ansi@10.0.0 (newly nested)
- @qwen-code/qwen-code -> wrap-ansi@9.0.2 (unchanged)
- yargs/cliui -> wrap-ansi@7.0.0 (unchanged)

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* test(InputPrompt): un-skip placeholder ID reuse after deletion

Reviewer (tanzhenxin) flagged that the new it.skip on the
'should reuse placeholder ID after deletion' test was undisclosed in
the PR description and removed coverage of real product behavior
(freePlaceholderId / bracketed-paste backspace path) without a
TODO(#NNNN) link.

Their argument was sound: the skip rationale pointed at ink 7's input
throttle, but this same file just bumped the wait helper from 50ms to
150ms specifically to give ink 7 frame time. Re-running the test under
the bumped wait shows it passes reliably (5/5 runs in the full-file
context, 9/10 alone), so the skip was masking the throttle-flake that
the wait bump already addresses, not a real product bug.

Drop the it.skip and the now-stale comment so coverage of the
freePlaceholderId reuse logic is restored.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* test(InputPrompt): bump first prompt-suggestion test wait to 350ms

The "accepts and submits the prompt suggestion on Enter when the buffer
is empty" test is the first in its describe block, so it pays the
renderer cold-start cost. On macOS-22.x CI runners that pushes the
Enter → onSubmit microtask past the default 150ms post-Enter wait. Match
the 350ms initial render wait used immediately above to absorb the cold
start.

* Revert "test(InputPrompt): bump first prompt-suggestion test wait to 350ms"

This reverts commit 6add83b62ea80c551c81f54af1fda3e6e7478f55.

* test(InputPrompt): wait for followup suggestion debounce before pressing Enter

Root cause of the failing prompt-suggestion tests on macOS and Windows
CI is not flaky timing of the test post-Enter wait — it's the 300ms
debounce inside createFollowupController.setSuggestion (shared core).
The Enter handler reads followup.state.isVisible synchronously, so if
the debounce timer has not fired before stdin.write('\\r'), the
suggestion path is skipped and onSubmit never runs. No amount of
post-Enter wait can recover from that — the keypress was already
processed against stale state.

The original wait(350) only left ~50ms margin over the 300ms debounce,
which ink 7 / React 19.2 mount overhead consumed on slow Windows
runners. Bump the initial wait to 700ms (named SUGGESTION_VISIBLE_WAIT_MS)
to give the debounce timer + cold-start render a generous buffer.

Apply to the two sibling tests too — without the wait their "does not
accept" assertions pass trivially when suggestion is never visible,
which is a false green that hides regressions in the actual reject path.

* fix(deps): align cli wrap-ansi with ink 7 (9.0.2 -> ^10.0.0)

Ink 7 ships its own wrap-ansi@10. CLI's direct dep was pinned to 9.0.2,
causing two copies of wrap-ansi in node_modules and a potential drift in
CJK width / ANSI handling between ink's internal text wrapping and our
TableRenderer.

Upgrading the CLI's direct dep to ^10.0.0 lets npm dedupe to a single
wrap-ansi@10 used by both ink and TableRenderer. API surface is
identical; the only documented behaviour change is that tabs are
expanded to 8-column tab stops before wrapping, which TableRenderer
doesn't feed in.

TableRenderer test suite (43 tests) passes against wrap-ansi@10.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* chore(deps): document @types/node 20.x pin in overrides

The override pinning @types/node to 20.19.1 (while engines require
Node >=22) is intentional: bumping to @types/node@22.x re-introduces
a Dirent<NonSharedBuffer> type regression that breaks
@qwen-code/qwen-code-core/sessionService tests.

Add a sibling "//@types/node" note inside `overrides` so future
maintainers see the rationale and know when to revisit the pin
without having to dig through PR #3860 history.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* test(AskUserQuestionDialog): link skipped Submit-tab test to tracking issue

The 'shows unanswered questions as (not answered) in Submit tab' test
was switched to `it.skip` in the ink 7 upgrade because
`ink-testing-library@4.0.0` doesn't flush input deterministically
through ink 7's 30fps throttle.

Add a `// TODO(#4036):` marker so the skip is greppable and can be
re-enabled once upstream ships an ink-7-compatible release.

Refs #4036

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(deps): move @types/node pin comment out of overrides block

npm's `overrides` field requires every key to be a real package name —
the `"//@types/node"` comment-key added in 205855875 trips Arborist with
"Override without name" and breaks `npm ci` across all CI jobs.

Move the explanation to a sibling top-level `"//overrides"` key, which
npm ignores at the document root. Same documentation value, no
override-parser collateral damage.

---------

Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-05-11 17:29:50 +08:00
pomelo
7e11428545
refactor(cli): remove legacy qwen auth CLI subcommand, redirect to /auth TUI dialog (#3959)
The `qwen auth` CLI subcommand (with subcommands like qwen-oauth,
coding-plan, api-key, openrouter, status) has been superseded by the
richer /auth TUI dialog introduced in the provider-first auth registry
(#3864). Running `qwen auth` now prints a deprecation notice pointing
users to the /auth TUI dialog (interactive), env vars (CI/headless),
or /doctor (status check).

Changes:
- Replace auth.ts with a stub that prints a removal notice and exits
- Delete handler.ts (734 lines), interactiveSelector.ts, and their
  tests (interactiveSelector.test.ts, openrouter.test.ts, status.test.ts)
- Update /auth slash command to handle non-interactive/ACP modes gracefully
- Enrich /doctor auth check with provider-aware diagnostics using
  findProviderByCredentials
- Mark `auth` as a subcommand that handles its own exit in config.ts

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-05-11 16:44:09 +08:00
tanzhenxin
78ad595581
feat(core): support QWEN_HOME env var to customize config directory (#2953)
* feat(core): support QWEN_CONFIG_DIR env var to customize config directory

Allow users to override the default ~/.qwen config directory location
via the QWEN_CONFIG_DIR environment variable. This enables users on dev
machines with external disk mounts or custom home directory layouts to
persist config at a location of their choosing.

Changes:
- Add QWEN_CONFIG_DIR check to Storage.getGlobalQwenDir() (absolute and
  relative path support)
- Eliminate 11 redundant '.qwen' constant definitions across packages
- Replace 16+ direct os.homedir() + '.qwen' path constructions with
  Storage.getGlobalQwenDir() calls
- Inline env var checks for packages that cannot import from core
  (channels, vscode-ide-companion, standalone scripts)
- Add unit tests for the new env var behavior
- Project-level .qwen/ directories are NOT affected

Closes #2951

* fix(core): use path.resolve/join in QWEN_CONFIG_DIR tests for Windows compat

Hardcoded Unix paths like '/tmp/custom-qwen/settings.json' fail on
Windows where path APIs produce backslash separators. Use path.resolve()
for inputs and path.join() for assertions so the tests pass cross-platform.

* test(cli): remove flaky 'should keep restart prompt when switching scopes' test

Timing-sensitive UI test that fails intermittently on Windows CI due to
async ANSI output not settling within the wait window.

* feat(core): route remaining hardcoded ~/.qwen/ paths through Storage.getGlobalQwenDir()

Update channel status, memory command, extension storage, skills
discovery, and memory discovery to use Storage.getGlobalQwenDir()
instead of hardcoded os.homedir()/.qwen paths, ensuring QWEN_CONFIG_DIR
env var is respected throughout the codebase.

* fix(tests): mock os.homedir before makeFakeConfig for Storage.getGlobalQwenDir

Storage.getGlobalQwenDir() is now called during Config construction,
which requires os.homedir() to be mocked before makeFakeConfig() is
called. Also mock Storage.getGlobalQwenDir in memoryCommand tests
since it uses a cross-package import that vi.spyOn doesn't intercept.

* fix(core): respect QWEN_CONFIG_DIR for .env discovery and install source

findEnvFile() walk-up would find legacy ~/.qwen/.env before checking
QWEN_CONFIG_DIR/.env when the workspace was under $HOME. Skip the
legacy path when a custom config dir is set so the fallback picks up
the correct file.

Also add a legacy fallback in readSourceInfo() since the installer
always writes source.json to ~/.qwen/ regardless of QWEN_CONFIG_DIR.

* refactor(core): rename QWEN_CONFIG_DIR to QWEN_HOME and fix runtime path resolution

Rename the env var before it ships (zero existing users) to match the
convention of CARGO_HOME, GRADLE_USER_HOME, etc. — "HOME" means "root of
all tool state", not just config.

Key changes:
- Rename QWEN_CONFIG_DIR → QWEN_HOME across all packages and scripts
- Add shared path utils in vscode-ide-companion and channels/base to
  eliminate scattered inline env var resolution
- Fix runtime path mismatch: IDE lock files and session paths in the
  vscode extension now route through getRuntimeBaseDir() (checking
  QWEN_RUNTIME_DIR first), matching core Storage behavior
- Fix telemetry_utils.js otel path to check QWEN_RUNTIME_DIR for tmp/
- Add E2E integration tests for QWEN_HOME scenarios

* fix(core): address critical review issues for QWEN_HOME support

Pass resolved QWEN_HOME as a dedicated QWEN_DIR sandbox parameter so
macOS Seatbelt profiles allow writes to custom config directories.
Fix hookRunner treating signal-killed hooks as success by using ?? -1
instead of || 0. Add QWEN_HOME and QWEN_RUNTIME_DIR to the env vars
documentation table.

* fix(sandbox): whitelist QWEN_RUNTIME_DIR in macOS Seatbelt profiles

When QWEN_RUNTIME_DIR is set separately from QWEN_HOME, the sandbox
was blocking writes to the runtime directory (debug logs, chat history,
IDE locks, sessions). Pass RUNTIME_DIR as a sandbox parameter and add
the corresponding subpath rule to all six .sb profiles.

* fix(core): add tilde expansion to QWEN_HOME and align satellite path helpers

- Extract resolvePath() from resolveRuntimeBaseDir() so QWEN_HOME gets
  the same ~/tilde expansion that QWEN_RUNTIME_DIR already had.
- Port resolvePath() to vscode-ide-companion and channels/base mirrors,
  fixing tilde handling in getRuntimeBaseDir() for the IDE companion.
- Add missing os.tmpdir() fallback in channels/base getGlobalQwenDir().
- Add unit tests for tilde expansion in QWEN_HOME.
- Clarify prompts.ts comment that system.md default is global, not
  project-level.

* fix(core): add tilde expansion to scripts and fix extension cache QWEN_HOME support

Add resolvePath() helper to standalone JS scripts (sandbox_command.js,
telemetry.js, telemetry_utils.js) so QWEN_HOME=~/custom expands
consistently with core Storage.resolvePath().

Fix ExtensionManager.refreshCache() to use ExtensionStorage.getUserExtensionsDir()
instead of hardcoded os.homedir(), so extensions installed under a custom
QWEN_HOME are discoverable.

* test: remove flaky InputPrompt tab-suggestion test on Windows

* test: remove flaky tests that fail intermittently on Windows

Removes 'does not accept the prompt suggestion on shift+tab' from
InputPrompt.test.tsx and 'should keep restart prompt when switching
scopes' from SettingsDialog.test.tsx. Both have been observed to fail
intermittently on the Windows CI workers; the underlying behaviors are
covered by adjacent assertions and end-to-end tests.

* revert(core): keep system.md path project-local under .qwen/

The QWEN_HOME refactor incorrectly routed the QWEN_SYSTEM_MD default path
through Storage.getGlobalQwenDir() (i.e. ~/.qwen/system.md or
$QWEN_HOME/system.md). The original semantics — inherited from the
upstream Gemini-CLI sync — are project-local: <cwd>/.qwen/system.md.

System-prompt customization is intentionally per-project so that each
repository can ship its own override without global side effects. Users
who want a global override can still set QWEN_SYSTEM_MD to an absolute
path. This revert keeps that behavior intact while leaving the rest of
the QWEN_HOME plumbing (settings, credentials, extensions, skills, memory)
unchanged.

* refactor(core): unify QWEN_CONFIG_DIR into the canonical QWEN_DIR

Three definitions of the literal '.qwen' string existed across the
codebase:

- QWEN_DIR in config/storage.ts (canonical, used by the Storage class)
- QWEN_CONFIG_DIR in memory/const.ts
- QWEN_CONFIG_DIR in tools/memory-config.ts (a near-clone of the above)

The QWEN_CONFIG_DIR name also collided with a former env-var name (now
renamed to QWEN_HOME on this branch), making it ambiguous whether call
sites referred to a configurable env var or a hardcoded directory name.

Drop the duplicates and route the only call sites (prompts.ts and its
test) through QWEN_DIR from config/storage.ts. The mock factory in
config.test.ts is updated to no longer expose the removed export.

* fix(integration-tests): use 'extensions list' to trigger settings migration

Tests 2b and 3a in cli/qwen-config-dir.test.ts relied on running
\`qwen --help\` to invoke loadSettings() (and thus the V1→V3 settings
migration). That worked when loadSettings() ran before parseArguments()
in the CLI startup sequence. Main has since flipped the order:
parseArguments() runs first, and yargs intercepts --help and exits the
process before loadSettings() is reached, so migration never runs and
the tests' migration probe always reads back V1.

Switch to \`qwen extensions list\` instead. It is a yargs subcommand that
runs through main() to loadSettings() without requiring an API key, so
migration runs as expected. Update the inline comments to document why
--help cannot be used and why this command works.

* fix(memory): route auto-memory base dir through Storage.getGlobalQwenDir()

The auto-memory subsystem (introduced on main in #3087) computed its base
directory by hardcoding path.join(os.homedir(), QWEN_DIR). That bypassed
QWEN_HOME entirely, so global auto-memory artifacts always landed in
~/.qwen/projects/... regardless of the user's configured QWEN_HOME path.

Route the default through Storage.getGlobalQwenDir() so QWEN_HOME is
honored. The QWEN_CODE_MEMORY_BASE_DIR test override stays as the
highest-priority short-circuit.

Discovered while running the QWEN_HOME e2e test plan against the merged
branch — Group B test B3 (memory tool writes to QWEN_HOME) was the only
failing scenario across A/B/C/D groups.

* fix(cli): treat custom QWEN_HOME .env as user-level

When QWEN_HOME points to a directory whose path does not contain
`.qwen` (e.g., `/tmp/qwen-home`), the global `.env` was misclassified
as a project-level env file. As a result, default-excluded variables
such as `DEBUG` and `DEBUG_MODE` were silently dropped even though
they came from the user-level config directory.

The classification now reuses the same user-level path set computed
by `findEnvFile`, so any `.env` inside the resolved global Qwen
directory (or directly under `~/`) is recognized as user-level.

Also drop the misleading "does not expand `~`" note from the
QWEN_HOME documentation — `Storage.getGlobalQwenDir` does expand
leading tildes via `Storage.resolvePath`.

* fix(cli): drop legacy .qwen substring check from env-file classification

The user-level env-file detection now keys solely off the precomputed
user-level path set, which already covers ~/.env and ${QWEN_HOME}/.env.
The legacy substring fallback misclassified <repo>/.qwen/.env as
user-level, so excludedEnvVars no longer applied to it.

* fix(core): align plain-text hook output with documented exit-code semantics

Per docs/users/features/hooks.md, only exit code 2 is a blocking error;
all other non-zero exit codes are non-blocking and execution should
continue. The plain-text branch in convertPlainTextToHookOutput
previously denied on every non-zero, non-1 exit code (3, 127, signal
fallbacks), contradicting the documented behavior.

Collapse all non-blocking non-zero codes to EXIT_CODE_NON_BLOCKING_ERROR
before passing into the converter so they take the warning path
consistently.

* chore: trigger CI

* fix(cli): pass QWEN_HOME and QWEN_RUNTIME_DIR into docker/podman sandbox

The container CLI previously had no awareness of the host's QWEN_HOME or
QWEN_RUNTIME_DIR values. The global qwen dir worked only because the
mount target happens to match the default fallback inside the sandbox,
and the runtime base dir was lost entirely when it diverged from the
global qwen dir.

* fix(cli): canonicalize sandbox QWEN/RUNTIME paths and pin IDE lock dir

Two reviewer-flagged issues from PR #2953:

* macOS Seatbelt was passed `path.resolve` for `QWEN_DIR`/`RUNTIME_DIR`
  while neighbouring directories used `fs.realpathSync`. With a symlinked
  `QWEN_HOME` or `QWEN_RUNTIME_DIR`, sandbox-exec would compare against
  the canonical kernel path and deny writes. Create the dirs (so
  `realpathSync` can succeed on first run) then canonicalize them like
  the surrounding entries.

* The VS Code companion wrote IDE lock files via the runtime base dir
  while the CLI side resolves the runtime dir from settings too. That
  divergence silently desynced lock-file discovery whenever a user set
  `advanced.runtimeOutputDir` without `QWEN_RUNTIME_DIR`. Anchor both
  sides to `getGlobalQwenDir()` since the companion process can only
  see env vars, not CLI settings.

* fix(cli): finish QWEN_HOME plumbing across env, memory, rules, sandbox

Codex review surfaced four user-visible spots where QWEN_HOME wasn't
threaded through:

* `findEnvFile` walked through the user home dir before consulting the
  QWEN_HOME fallback, so `~/.env` shadowed `<QWEN_HOME>/.env` and
  reversed the qwen-specific precedence the default `~/.qwen/.env` path
  enjoys. Add a home-dir-step check that prefers the custom Qwen dir
  when set.

* `MemoryDialog` displayed and edited `~/.qwen/QWEN.md` regardless of
  QWEN_HOME. Memory discovery already routes through Storage, so user
  edits via the dialog were silently ignored at runtime. Route the
  dialog through `Storage.getGlobalQwenDir()` to match.

* `loadRules` looked up global rules at `~/.qwen/rules/`, ignoring
  QWEN_HOME entirely. Use the global Qwen dir like the rest of the
  config surfaces.

* The Docker/Podman sandbox path called `mkdirSync(userSettingsDir)`
  without `recursive`. Pre-PR the dir was always `~/.qwen` and the
  parent existed; with a nested QWEN_HOME like `/tmp/qwen/config` the
  first run threw ENOENT before the mount could be added.

* fix(cli): block project .env from redirecting QWEN_HOME and QWEN_RUNTIME_DIR

A project `.env` could set QWEN_HOME after settings were already loaded
from the real home, splitting global state: settings.json read from
~/.qwen but later writes (installation_id, OAuth credentials, MCP tokens)
landed in the project-controlled directory. The user-configurable
excludedEnvVars list isn't the right place for this — it's a correctness
boundary, not a preference — so always exclude these two vars from
project .env files. User-level .env files (~/.qwen/.env) are unaffected.

* fix(cli): keep workspace .qwen/.env unfiltered and pre-resolve user QWEN_HOME

The env-file classification conflated two concerns: which paths may
override global state vars, and which paths are exempt from the
user-configurable excludedEnvVars filter. Splitting them lets a
workspace `<repo>/.qwen/.env` carry DEBUG/DEBUG_MODE per the documented
contract while still being blocked from redirecting QWEN_HOME or
QWEN_RUNTIME_DIR.

A QWEN_HOME set in `~/.qwen/.env` or `~/.env` would also previously
arrive too late: USER_SETTINGS_PATH was captured at module load and
loadSettings migrated `~/.qwen/settings.json` before loadEnvironment
applied the override, leaving credentials, MCP tokens, and
installation_id pointed at the new directory while settings stayed at
the legacy one. A pre-pass now reads those user-level files for the
two storage-controlling vars before any user settings are loaded, and
the user settings path is re-resolved locally so all global state lands
in one place.

* fix(cli): make user-settings paths lazy to pick up bootstrapped QWEN_HOME

USER_SETTINGS_PATH/USER_SETTINGS_DIR in settings.ts and the duplicate
USER_SETTINGS_DIR in trustedFolders.ts were top-level consts evaluated
at module load — before preResolveHomeEnvOverrides() reads QWEN_HOME
from ~/.env or ~/.qwen/.env. Callers (sandbox launcher, trusted-folders
reader) saw the legacy ~/.qwen path while the main CLI had moved to the
custom home, splitting state.

Convert all three to lazy getter functions and add a regression test
that pokes process.env.QWEN_HOME after import and asserts each getter
reflects it — any future top-level capture turns the test red.

Mirror the same ~/.env / ~/.qwen/.env bootstrap into
scripts/sandbox_command.js, which previously only read process.env
directly and could disagree with the main CLI on the sandbox setting.

Addresses review threads #3159793469, #3177804507, and item #2 of the
2026-05-06 review summary.

* fix(cli): address qwen home review follow-ups

* test(cli): normalize path in QWEN_HOME freshness assertion for Windows

`getUserSettingsDir()` returns `path.dirname(...)`, which on Windows uses
backslash separators. The bare string comparison failed on Windows runners
("\tmp\qwen-lazy-test" vs "/tmp/qwen-lazy-test"). Wrap the expected value
in `path.normalize()` to match the OS-native separator, mirroring the two
sibling assertions that already use `path.join()`.

* fix(cli): close storage-routing leaks via settings.env and project sandbox .env

settings.env (merged) was being applied to process.env without filtering, so
a workspace settings.json could redirect global state by setting
env.QWEN_HOME or env.QWEN_RUNTIME_DIR after the home-scoped .env bootstrap
ran. Apply PROJECT_ENV_HARDCODED_EXCLUSIONS to the settings.env path too.

scripts/sandbox_command.js's project-walk fallback called dotenv.config() to
find QWEN_SANDBOX, which injected every parsed key — including QWEN_HOME /
QWEN_RUNTIME_DIR the main CLI hard-blocks. Replace with a manual parse that
copies only QWEN_SANDBOX.

Add a startup migration warning when QWEN_HOME points to a directory with
no settings.json while ~/.qwen/settings.json exists, so users notice that
their existing OAuth tokens / settings / memory aren't auto-migrated.

* test: cover QWEN_HOME / QWEN_RUNTIME_DIR in duplicated path helpers

Adds targeted unit tests for the two TypeScript mirrors of
Storage.getGlobalQwenDir() / getRuntimeBaseDir() that live outside
packages/core to avoid cross-package imports. Covers default, absolute,
relative, ~/x, ~\x, and bare ~ inputs, plus the runtime/home priority
chain in the IDE companion.

* fix: bootstrap QWEN_HOME before yargs handlers and in VS Code companion

Two storage-routing leaks surfaced by Codex review of feat/qwen-config-dir:

- channel status/stop call readServiceInfo() inside yargs handlers that
  process.exit before loadSettings() runs, so QWEN_HOME defined only in
  ~/.qwen/.env or ~/.env never resolved for them. The same race exists
  for the duplicate-instance check at the top of channel start. Hoist
  preResolveHomeEnvOverrides() to the top of main() so all subcommand
  handlers see the bootstrapped env vars.

- The VS Code companion's getGlobalQwenDir / getRuntimeBaseDir read
  process.env directly, missing the same .env pre-pass. If a user only
  configures QWEN_HOME via ~/.qwen/.env, the CLI looks under the
  redirected dir while the companion writes IDE lock files under
  ~/.qwen, breaking IDE discovery. Mirror the CLI pre-pass in the
  companion (lazy, idempotent) without importing from core.

* fix(config): preserve credentials in legacy ~/.qwen/.env when QWEN_HOME redirects

When QWEN_HOME is bootstrapped from `~/.qwen/.env`, the home-dir env walk
previously skipped that file and never read `<QWEN_HOME>/.env` from the
companion. This stranded non-routing credentials (e.g. OPENAI_API_KEY)
left in `~/.qwen/.env` and let the companion write IDE lock files into a
different runtime dir than the CLI was reading from.

- CLI: fall back to `~/.qwen/.env` after `<QWEN_HOME>/.env` at both the
  home-dir step and the post-walk fallback in findEnvFile, and treat the
  legacy path as user-level for trust and exclusion semantics.
- Companion: after the initial candidate pass discovers QWEN_HOME, also
  read `<QWEN_HOME>/.env` so QWEN_RUNTIME_DIR sourced from there matches
  what the CLI's findEnvFile would pick.

* refactor(cli): simplify QWEN_HOME plumbing — dedupe helpers, latch, comments

- replace local isSameOrChildPath with core's isSubpath in sandbox.ts
- latch preResolveHomeEnvOverrides so it runs once per process
- pass userLevelPaths from loadEnvironment into findEnvFile (no recompute)
- collapse findEnvFile's home-dir branch and post-loop fallback into one
  shared candidate list (drops duplicate existsSync calls)
- factor extensionManager's user-extensions loop into a private helper
- use QWEN_DIR constant instead of '.qwen' literal in skill-manager
- trim narrative / PR-history comments across changed files

* fix(cli): align QWEN_HOME .env bootstrap across CLI, sandbox, telemetry

Telemetry scripts previously read process.env.QWEN_HOME directly, so a
QWEN_HOME set only in ~/.env or ~/.qwen/.env left telemetry writing to
~/.qwen while the CLI routed elsewhere. Extract the bootstrap into
scripts/lib/qwen-home-bootstrap.js and have sandbox_command.js,
telemetry.js, and telemetry_utils.js share it.

Also add a third-pass <new QWEN_HOME>/.env read in
preResolveHomeEnvOverrides so the CLI and VS Code companion agree on
QWEN_RUNTIME_DIR when it is configured under the new home dir.

* test(integration-tests): update QWEN_HOME assertions for v4 schema

Settings schema was bumped to v4 on main (gitCoAuthor migration). The
qwen-config-dir tests still asserted post-migration $version === 3, so
they failed after the merge. Bump the assertions to 4 and the seed in
3a to match, and point a comment at SETTINGS_VERSION so the next bump
is easy to find.
2026-05-09 15:51:52 +08:00
pomelo
997796f532
refactor(cli): provider-first auth registry with unified install pipeline (#3864)
* fix(cli): refresh static header on model switch

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* feat(cli): simplify api key provider registry

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* refactor(cli): split Alibaba auth providers

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* polish(cli): refine auth provider onboarding

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): update OpenRouter free defaults

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): restrict token plan models

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* chore(cli): remove unused third-party providers

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* feat(cli): add regional third-party providers

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* refactor(cli): simplify api key provider endpoints

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* refactor(cli): split auth dialog flows

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* refactor(cli): unify auth around declarative provider config

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

Introduce ProviderConfig abstraction (providerConfig.ts) and a central provider registry (allProviders.ts), replacing the per-flow UI components (AlibabaModelStudioFlow, CustomProviderFlow, OAuthFlow, ThirdPartyProvidersFlow, etc.) with unified ProviderSetupSteps and useProviderSetupFlow.

Key changes:
- Remove setupMethods/apiKey/ directory entirely
- Collapse flow-specific hooks/components into a single generic provider setup flow
- Simplify each provider file to export only a ProviderConfig descriptor
- Add alibabaStandard provider alongside codingPlan/tokenPlan
- Move all baseUrl resolution, install plan building, and settings writing into providerConfig
- Update useAuth, AuthDialog, command handler, and upstream consumers to use the new registry

* refactor(cli): simplify provider setup input flow

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* refactor(cli): remove toLlmProvider and legacy auth wrappers

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* refactor(cli): flatten auth flow files and simplify ProviderSetupSteps props

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* feat(cli): prefill API key from existing env settings in provider setup flow

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): correct third-party provider context windows

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): harden provider auth setup

* feat(cli): support provider modality and context settings

* feat: eable modelsEditable for coding plan

* refactor(cli): auto-derive provider metadata key and state

Move metadataKey and getProviderState from per-provider config to
auto-derived helpers (resolveMetadataKey, resolveProviderState) in
providerConfig.ts. This centralizes version tracking logic and reduces
boilerplate in individual provider definitions.

Add useProviderUpdates hook that detects model template changes across
all version-tracked providers and surfaces update/ignore choices.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

Closes: OSS-1730, OSS-1729

* refactor(cli): namespace provider metadata under providerMetadata key

Introduce PROVIDER_METADATA_NS ('providerMetadata') to avoid top-level
settings key collisions. Provider metadata now lives under
e.g. providerMetadata.coding-plan.version instead of codingPlan.version.

Add migration logic (migrateProviderMetadata) to automatically move
legacy top-level keys (codingPlan, tokenPlan) into the new namespace
on first run.

Update auth handler, useProviderUpdates hook, and all related tests
to use the new namespace structure.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

[skip ci]

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): polish ProviderUpdatePrompt styling and test coverage [skip ci]

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* refactor(auth): simplify auth flows around provider abstraction [skip ci]

- Rewrite motivation.md to document provider-centric architecture
- Remove Alibaba Standard API Key and Coding Plan UI flows from handler
- Update status tests to use providerMetadata instead of codingPlan settings
- Streamline API key auth to show docs link only

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* refactor(auth): update provider models and refine auth infrastructure

- Bump model versions (qwen3.6-plus, glm-5.1) and add deepseek-v4-pro/flash
  with modalities to Alibaba Standard provider
- Reorder DeepSeek models, add thinking+image/video modalities to v4-pro,
  fix v4-flash context window
- Enhance auth tests with provider metadata setValue assertions
- Switch env key generation from hash-based to URL-based with
  trailing-slash normalization
- Remove deprecated codingPlan section from settings schema

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(i18n): add missing zh-TW translations for token plan and subscription providers

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* refactor(auth): improve provider install error recovery and AuthDialog state init

- Restore settings from backup on provider install plan failure
- Fix AuthDialog mainIndex state to null (was 0), preventing stale selection
- Remove ownsModel from customProvider; fall back to id-based filtering
- Change provider migration log from console.error to console.log
- Add sync reminder comments between CLI and VSCode subscription models
- Expand handleApiKeyAuth JSDoc explaining its role as lightweight fallback

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(auth): i18n for step labels, lazy preview JSON, and accurate header label

- Wrap getStepLabel() strings and PROTOCOL_ITEMS in t() for i18n
- Only compute previewJson when on the review step
- Return matched provider's own label in getAuthDisplayType instead
  of hardcoding CODING_PLAN for all managed providers

* fix(auth): address round-3 review blockers

- Fix CI: add missing useProviderUpdates mock in AppContainer.test.tsx
  that caused TypeError breaking React effects (title/height tests)
- Fix half-rollback: snapshot settings + modelProviders before install,
  restore in-memory state (not just disk) on refreshAuth failure
- Fix .orig backup reuse: always create fresh backup (overwrite stale),
  cleanup on success, unlink after restore to prevent data loss
- Fix cross-package key consistency: VS Code settingsWriter now writes
  to providerMetadata namespace matching CLI's new structure
- Fix validateApiKey: remove baseUrl guard so sk-sp- prefix check
  applies to both China and Global Coding Plan endpoints

* fix(cli): stabilize AuthDialog tests for slower CI environments

Increase vi.waitFor timeouts from default 1000ms to 5000ms and replace
unreliable fixed-delay waits with proper render-completion assertions,
preventing flaky failures on Linux/Windows CI runners with Node 22/24.

* fix(core): use id+baseUrl composite key for model identity

Custom provider installs previously used model id alone to determine
ownership, causing the second install to remove the first backend's
model entry when both expose the same model id (e.g. gpt-4o) with
different baseUrls. Use id+baseUrl as the composite identity key
throughout the model registry, ModelDialog, and modelsConfig to
prevent cross-provider model collisions.

* fix(cli): update ModelDialog tests for composite-key model identity

Add missing getModelsConfig and getActiveRuntimeModelSnapshot mocks,
and update switchModel assertion to expect the new { baseUrl } options
object introduced in 4c4ebb81c.

* fix(cli): skip flaky TUI input tests on all CI environments

Multi-step TUI navigation tests exceed 5s timeout on CI runners
regardless of Node version. Extend skip condition from only Node 20
to all CI environments where input simulation is unreliable.

* fix(cli): improve auth/provider edge cases and UX

- Add fallback to non-free models in OpenRouter OAuth when no free models available
- Validate non-empty models list when building install plan
- Fix auth status to use activeConfig instead of iterating all providers
- Clear API key input when switching auth protocol
- Skip unnecessary auth refresh when applying provider updates

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* test(cli): update tests for empty model validation and skip auth refresh

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): skip remaining flaky TUI input AuthDialog tests on CI

8558c49bc only converted part of the tests to itWhenTuiInputReliable,
leaving 9 multi-step keyboard-navigation tests still using bare it().
These tests reliably time out on Linux/Windows CI runners where stdin
simulation timing is unpredictable.

Convert all remaining it() → itWhenTuiInputReliable() so CI skips them,
and add a comment block to clearly demarcate the TUI input section.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-05-08 12:19:28 +08:00
Shaojin Wen
cfbcea1e88
feat: add commit attribution with per-file AI contribution tracking (#3115)
* feat: add commit attribution with per-file AI contribution tracking via git notes

Track character-level AI vs human contributions per file and store
detailed attribution metadata as git notes (refs/notes/ai-attribution)
after each successful git commit. This enables open-source AI disclosure
and enterprise compliance audits without polluting commit messages.

* feat: enhance commit attribution with real AI/human ratios and generated file exclusion

- Replace line-based diff with a prefix/suffix character-level algorithm
  for precise contribution calculation (e.g. "Esc"→"esc" = 1 char, not whole line)
- Compute real AI vs human contribution percentages at commit time by analyzing
  git diff --stat output: humanChars = max(0, diffSize - trackedAiChars)
- Add generated file exclusion (lock files, dist/, .min.js, .d.ts, etc.)
  ported from an existing generatedFiles.ts
- Add file deletion tracking via recordDeletion()
- Update git notes payload format: {aiChars, humanChars, percent} per file
  with real percentages instead of hardcoded 100%

* feat: add surface tracking, prompt counting, session persistence, and PR attribution

Align with the full attribution feature set:
- Surface tracking: read QWEN_CODE_ENTRYPOINT env var (cli/ide/api/sdk),
  include surfaceBreakdown in git notes payload
- Prompt counting: incrementPromptCount() hooked into client.ts message
  loop, tracks promptCount/permissionPromptCount/escapeCount
- Session persistence: toSnapshot()/restoreFromSnapshot() for serializing
  attribution state; ChatRecordingService.recordAttributionSnapshot()
  writes to session JSONL; client.ts restores on session resume
- PR attribution: addAttributionToPR() in shell.ts detects `gh pr create`
  and appends "🤖 Generated with Qwen Code (N-shotted by Qwen-Coder)"
- Session baseline: saves content hash on first AI edit of each file
  for precise human/AI contribution detection
- generatePRAttribution() method for programmatic access

* fix: audit fixes — initial commit handling, cron prompt exclusion, failed commit counter preservation

- Handle initial commit (no HEAD~1) by detecting parent with rev-parse
  and falling back to --root for first commit in repo
- Exclude Cron-triggered messages from promptCount (not user-initiated)
- Add commitSucceeded parameter to clearAttributions() so failed/disabled
  commits don't reset the prompts-since-last-commit counter
- Add test for clearAttributions(false) behavior

* fix: cross-platform and correctness fixes from multi-round audit

- Normalize path.relative() to forward slashes for Windows compatibility
- Use diff-tree --root for initial commits (git diff --root is invalid)
- Replace String.replace() with indexOf+slice to avoid $& special patterns
- Fix clearAttributions(false→true) when co-author disabled but commit succeeded
- Use real newlines instead of literal \n in PR attribution text
- Add surface fallback in restoreFromSnapshot for version compatibility
- Fix single-quote regex to not assume bash supports \' escaping
- Case-insensitive directory matching in generated file detection
- Handle renamed file brace notation in parseDiffStat

* fix(attribution): also snapshot on ToolResult turns so resume keeps tool edits

Previously, recordAttributionSnapshot() only ran at the start of UserQuery
and Cron turns — before the tools for that turn had executed. A session
that wrote a file in turn 1 and committed in turn 2 (across process
boundaries via --resume) lost the tracked edit: the last persisted
snapshot was the turn-1-start snapshot (empty fileStates), so on resume
the attribution service restored empty state and no git notes were
attached to the commit.

Move the snapshot call out of the UserQuery/Cron conditional and run it
on every non-Retry turn. ToolResult turns are scheduled right after
tools execute, so their start-of-turn snapshot now captures any edits
those tools made. Retry turns are skipped since the state is unchanged
from the prior turn.

Added unit tests asserting the snapshot fires for ToolResult/UserQuery
turns and skips Retry turns.

Verified end-to-end in a scratch repo: write-file in turn 1 (no commit)
→ exit → --resume → commit in turn 2 → git notes now contain the
recorded file with correct aiChars and promptCount: 2.

* refactor(attribution): merge duplicate retry guard and update stale doc

Collapse the two back-to-back messageType !== Retry blocks in
sendMessageStream into one, and refresh chatRecordingService's
recordAttributionSnapshot doc comment to reflect that snapshots fire
on every non-retry turn (not just after user prompts).

* feat(attribution): split gitCoAuthor into independent commit and pr toggles

Matches the shape used upstream in Claude Code's `attribution.{commit,pr}`
so users can disable the PR body line without losing the commit-message
Co-authored-by trailer (or vice versa). The previous boolean forced both
to move together, which conflated two different surfaces.

- settingsSchema: gitCoAuthor becomes an object with nested commit/pr
  booleans, each `showInDialog: true` so both appear in /settings.
- Config constructor accepts legacy boolean (coerced to { commit: v, pr: v })
  so stored preferences from the pre-split schema carry over.
- shell.ts: attachCommitAttribution and addCoAuthorToGitCommit read .commit;
  addAttributionToPR reads .pr.

* feat(settings): add v3→v4 migration for gitCoAuthor shape change

Legacy gitCoAuthor was a single boolean and shipped ~4 months ago; the
previous commit split it into { commit, pr } sub-toggles. Without a
migration, users who had set gitCoAuthor: false would see the settings
dialog show the default (true) for both sub-toggles — misleading and
likely to flip their preference on the next save because getNestedValue
returns undefined when asked for .commit on a boolean.

- New v3-to-v4 migration expands boolean → { commit: v, pr: v },
  preserves already-object values, resets invalid values to {} with a
  warning.
- SETTINGS_VERSION bumped 3 → 4; existing integration assertions use the
  constant so the next bump is a single-line change.
- Regenerate vscode-ide-companion settings.schema.json to reflect the
  new nested shape.
- Docs: split the single gitCoAuthor row into .commit and .pr.

* test(migration): cover null/array/number and partial object for v3-to-v4

The migration already treats any non-boolean, non-object value as invalid
(reset to {} with warning), but the existing test only exercised the
string "yes" branch. Add parameterized cases for null, array, and number
so a future regression that accepts these in the valid bucket gets caught.
Also cover partial objects — the migration must not paternalistically
fill defaults; that responsibility lives in normalizeGitCoAuthor at the
Config boundary.

* fix(shell): address PR review for compound commits and PR body escaping

Two critical issues called out in review:

1. attachCommitAttribution treated the final shell exit code as proof
   that `git commit` itself failed. For compound commands like
   `git commit -m "x" && npm test`, the commit can succeed and a later
   step can fail; the previous code then cleared attribution without
   writing the git note. Now we snapshot HEAD before the command (via
   `git rev-parse HEAD` through child_process.execFile, kept independent
   of the mockable ShellExecutionService) and detect commit creation by
   HEAD movement, so attribution lands whenever a new commit was created
   regardless of later steps.

2. addAttributionToPR spliced the configured generator name into the
   user-approved `gh pr create --body "..."` argument verbatim. A name
   containing `"`, `$`, a backtick, or `'` could break the command or be
   evaluated as command substitution. Now we shell-escape the appended
   text per the surrounding quote style before splicing.

Tests cover the new escape paths for both double- and single-quoted
bodies, including a generator name designed to break interpolation
(`$(rm -rf /) "danger" \`eval\``) and one with an apostrophe.

* fix(attribution): address Copilot review on shell, schema, and totals

Six items called out on PR #3115 by Copilot:

- shell.ts: addAttributionToPR's bash quote escaping doesn't apply to
  cmd.exe / PowerShell, where `\$` and `'\''` aren't honored. Skip the
  PR body rewrite entirely on Windows — losing PR attribution there is
  preferable to corrupting the user-approved `gh pr create` command.

- attributionTrailer.ts + shell.ts call site: buildGitNotesCommand used
  bash-style single-quote escaping on the JSON note, which is broken on
  Windows. Switched to argv form (`{ command, args }`) and routed the
  invocation through child_process.execFile so shell quoting is bypassed
  entirely. Tests updated to assert the argv shape.

- commitAttribution.ts: when a tracked file's aiChars exceeded the diff
  --stat-derived diffSize (long-line edits where diffSize ≈ lines * 40),
  humanChars clamped to 0 but aiChars stayed inflated, leaving aiChars +
  humanChars > the committed change magnitude. Clamp aiChars to diffSize
  so the totals stay consistent.

- shell.ts parseDiffStat: only normalized rename brace notation
  (`{old => new}`). Cross-directory renames emit `old/path => new/path`
  without braces, leaving diffSizes keyed by the full string. Added a
  second normalization step.

- shell.ts: addAttributionToPR docstring claimed `(X% N-shotted)` but
  the implementation only emits `(N-shotted by Generator)`. Updated the
  docstring to match the actual behavior.

- settingsSchema.ts + generator: gitCoAuthor went from boolean to object
  in the V4 migration. The exported JSON Schema now wraps the field in
  `anyOf: [boolean, object]` (via a new `legacyTypes` hint on
  SettingDefinition) so users with a stored boolean don't see a spurious
  IDE warning before their next launch runs the migration.

* fix(attribution): parse binary diffs, source generator from model, sync schema $version

Three follow-up review items from Copilot:

- parseDiffStat now handles git's binary-diff format (`path | Bin A ->
  B bytes`) using the byte delta with a floor of 1. Without this,
  binary edits arrived at the attribution payload as diffSize=0 and
  were silently dropped. Also extracted the parser to a top-level
  exported function so the binary path is unit-testable; added five
  targeted cases (text/binary/rename normalisation/summary skip).

- attachCommitAttribution now passes `this.config.getModel()` into
  generateNotePayload instead of the user-configurable
  `gitCoAuthor.name`. The note's `generator` field reflects which
  model produced the changes — and CommitAttributionService's
  sanitizeModelName() actually has the codename to scrub now.

- generate-settings-schema.ts imports SETTINGS_VERSION instead of
  hardcoding `default: 3`, so a future bump propagates to the emitted
  JSON schema in one place. Regenerated settings.schema.json bumps
  $version's default from 3 to 4 to match the V4 migration.

* fix(attribution): repo-root baseDir, escape co-author trailer, switch to numstat

Three Critical items called out by wenshao:

- attachCommitAttribution was passing config.getTargetDir() as `baseDir`
  to generateNotePayload, but getCommittedFileInfo returns paths
  relative to `git rev-parse --show-toplevel`. When the working
  directory was a subdirectory of the repo, path.relative produced
  `../...` keys that never matched in the AI-attribution lookup,
  silently zeroing out attribution for every file outside getTargetDir.
  StagedFileInfo now carries an optional `repoRoot` (filled in by
  getCommittedFileInfo via `git rev-parse --show-toplevel`) and the
  caller prefers it over the target dir.

- addCoAuthorToGitCommit interpolated `gitCoAuthorSettings.name` and
  `.email` into the rewritten command without escaping. A name
  containing `$()`, backticks, or `"` could be evaluated as command
  substitution under double quotes, or break the user-approved
  `git commit -m "..."` quoting. Now escapes per the surrounding quote
  style with the same helpers addAttributionToPR uses, gates on
  non-Windows for the same shell-quoting reason, and fixes the regex
  to accept `-m"msg"` shorthand (no space) so users who type the
  bash-shorthand form aren't silently denied a trailer.

- parseDiffStat used `git diff --stat` output and approximated each
  line as ~40 chars by parsing a graphical text bar. Replaced with
  `git diff --numstat` which gives unambiguous integer
  additions+deletions per file; the heuristic remains but the parser
  is no longer fooled by the visual `++--` markers. Binary entries
  fall back to a fixed estimate so they still land in the map (rather
  than dropping out as diffSize=0).

Suggestions also addressed: stale duplicate JSDoc on
addCoAuthorToGitCommit removed, misleading `clearAttributions`
comments rewritten to describe what the boolean argument actually
does. Tests cover the new shorthand path, escape behavior, and
numstat parsing (text/binary/rename/malformed).

* fix(shell): shell-aware git-commit detection and apostrophe-escape handling

Two more Critical items called out by wenshao plus the matching Copilot
quote-handling notes:

- attachCommitAttribution and addCoAuthorToGitCommit now go through a
  shell-aware `looksLikeGitCommit` helper instead of a raw
  `\bgit\s+commit\b` regex. The helper splits the command on shell
  separators (`splitCommands`) and checks each segment, so `echo "git
  commit"` no longer triggers attribution clearing or trailer
  injection. The same helper bails on any segment that contains `cd`
  or `git -C <path>`, since either could redirect the commit into a
  different repo than our cwd — writing notes or capturing HEAD there
  would corrupt unrelated state.

- The post-command attribution call now runs regardless of whether the
  shell wrapper aborted. `git commit -m "x" && sleep 999` could move
  HEAD and then time out, leaving the new commit without its
  attribution note while the stale per-file attribution stayed around
  for a later unrelated commit. attachCommitAttribution still gates on
  HEAD movement, so it's a no-op when no commit was actually created.

- The `-m '...'` and `--body '...'` regexes used to match only the
  first quote segment, so a command like `git commit -m 'don'\''t'`
  (bash's standard apostrophe-escape form) would have the trailer
  spliced mid-message and break the command's quoting. The single-
  quote patterns now use a negative lookahead / inner alternation to
  either skip those messages entirely (commit path) or match the
  whole escape-aware body (PR path).

Tests cover the new behavior: quoted "git commit" is left alone, the
`cd && git commit` and `git -C` patterns get no trailer, and the
apostrophe-escape form passes through unchanged for both `-m` and
`--body`.

* fix(attribution): drop magic 100 fallback for empty deletions

Deleted files with no AI tracking now use diffSize directly. With
numstat as the input source, diffSize is an exact count, and an
empty-file deletion legitimately reports zero — a magic fallback would
only inflate totals.

* fix(shell): broaden git-commit detection, gate background, drop dead helpers

Five Copilot follow-ups:

- looksLikeGitCommit now strips leading env-var assignments
  (`GIT_COMMITTER_DATE=now git commit ...`) and a small allowlist of
  safe wrappers (`sudo`, `command`) before matching. The previous
  exact-prefix match silently skipped trailer injection on common
  real-world commit forms.

- A new looksLikeGhPrCreate (same shell-aware shape) replaces the raw
  `\bgh\s+pr\s+create\b` regex in addAttributionToPR, so quoted text
  like `echo "gh pr create --body \"x\""` no longer triggers a
  command-string rewrite.

- executeBackground refuses to run `git commit` and tells the user to
  re-run foreground. The BackgroundShellRegistry lifecycle has no
  hook for the post-command pre/post-HEAD comparison or git-notes
  write, so allowing the commit through would create the new commit
  without notes and leak stale per-file attribution into the next
  foreground commit.

- recordDeletion was unused outside its own test — removed (and the
  test). When AI-driven deletions need tracking we'll add it with an
  actual integration point rather than carrying dead API surface.

- generatePRAttribution was likewise unused; addAttributionToPR
  builds the trailer string inline. The two formats had already
  diverged. Removed the helper and its tests; reviving from git
  history is straightforward if a future caller needs it.

Tests: env-var and sudo prefixes now produce trailers; quoted
"gh pr create" leaves the command unchanged; existing 81 shell tests
still pass alongside the trimmed 25 commitAttribution tests.

* fix(shell): unified git-commit detection split by intent

Six items called out across CodeQL, Copilot, and wenshao:

- The earlier `looksLikeGitCommit`/`stripCommandPrefix` returned a
  single yes/no and rejected ANY `cd` in the chain. That fixed the
  wrong-repo case but also disabled attribution for `git commit -m
  "x" && cd ..` (commit already landed safely in our cwd; the cd
  came after). It also conflated three distinct decisions onto one
  predicate.

  New `gitCommitContext` returns both `hasCommit` and
  `attributableInCwd`, walking segments in order so that a `cd`
  AFTER the commit doesn't invalidate it. Callers now pick the right
  arm:
  - background-mode refusal uses `hasCommit` (refuses even
    `cd /elsewhere && git commit` since we can't attribute it
    afterward either way)
  - HEAD snapshot, addCoAuthorToGitCommit, and the
    attachCommitAttribution gate use `attributableInCwd`

- Tokenisation switches from a regex while-loop to `shell-quote`'s
  `parse`. Quoted env values like `FOO="a b" git commit` now skip
  correctly (the old `\S*\s+` form would cut after the opening
  quote). Eliminates the CodeQL polynomial-regex alert at the same
  time since the `\S*\s+` pattern is gone.

- attachCommitAttribution now snapshots prompt counters via
  `clearAttributions(true)` whenever a commit lands, even if no
  per-file attributions were tracked. Previously the early-return
  on `hasAttributions() === false` meant `promptCountAtLastCommit`
  never advanced, so a later `gh pr create` reported an inflated
  N-shotted count spanning multiple commits.

Tests: env-var and sudo prefixes still produce trailers; quoted
"git commit" / "gh pr create" leave commands unchanged; cd BEFORE
commit suppresses the rewrite while cd AFTER commit does not; `git
-C <path> commit` is treated as a commit (refused in background)
but not as attributable.

* fix(shell): position-independent git subcommand detection + bash-shell guard

Six review items, two of them critical:

- gitCommitContext was checking fixed-position tokens (`arg1`, `arg3`)
  and missed every git invocation that puts a global flag between
  `git` and the subcommand: `git -c user.email=x@y commit`,
  `git --no-pager commit`, `git -C /p -c k=v commit`, etc. In
  background mode these would slip past the refusal guard; in
  foreground they got no co-author trailer, no git note, and no
  prompt-counter snapshot. New `parseGitInvocation` walks past
  git's global flags (with their values) before reading the
  subcommand, and reports `changesCwd` for `-C` / `--git-dir` /
  `--work-tree`.

- The Windows guard on addCoAuthorToGitCommit and addAttributionToPR
  used `os.platform() === 'win32'`, which incorrectly skipped Windows
  + Git Bash (`getShellConfiguration().shell === 'bash'`). Switched
  both to gate on `getShellConfiguration().shell !== 'bash'` so Git
  Bash users keep the feature.

- attachCommitAttribution was re-parsing `gitCommitContext(command)`
  even though `execute()` already gates on `commitCtx.attributableInCwd`.
  Removed the redundant re-parse — drift between the two checks would
  silently diverge trailer injection from git-notes writes.

- tokeniseSegment (formerly tokeniseProgram) now logs via debugLogger
  on parse failure instead of swallowing silently. Easier to debug
  if shell-quote ever throws on something unusual.

- Added a comment on `cwdShifted` documenting that it's a one-way
  latch — `cd src && cd ..` will still skip attribution. The
  trade-off matches the wrong-repo guard's "better miss than corrupt
  unrelated repos" intent.

- Stale `--stat` reference in the aiChars-clamp comment updated to
  `--numstat` to match the actual git command in
  ShellToolInvocation.getCommittedFileInfo.

Tests: `git -c key=val commit` and `git --no-pager commit` now
produce a trailer; existing 82 shell tests still pass.

* fix(shell): refuse multi-commit attribution; misc review follow-ups

Five follow-ups from the latest review pass:

- attachCommitAttribution now refuses to write a single git note for
  shell commands that produce more than one commit (e.g.
  `git commit -m a && git commit -m b`). The singleton's per-file
  attribution map can't be partitioned across the individual commits,
  so attaching the combined note to HEAD would mis-attribute earlier
  commits' changes to the last one. Walks `preHead..HEAD` via
  `git rev-list --count`; on multi-commit detection it snapshots the
  prompt counters and bails with a debug warning instead of writing
  a misleading note.

- parseGitInvocation now recognises the attached `-C/path` form
  (e.g. `git -C/path commit -m x`). shell-quote tokenises that as a
  single `-C/path` token which previously fell to the generic flag
  branch with `changesCwd = false`, leaving an out-of-cwd commit
  classified as attributable.

- attachCommitAttribution dropped its unused `command` parameter
  (the caller already gates on `commitCtx.attributableInCwd`, so
  re-parsing was removed earlier; the parameter became dead).

- Added wiring guards in edit.test.ts and write-file.test.ts:
  AI-originated edits/writes hit `CommitAttributionService.recordEdit`,
  `modified_by_user: true` skips, and write-file's distinction
  between a true new file and an overwritten empty file (`null` vs
  `''` old content) is now pinned by `aiCreated` assertions.

* fix(attribution): partial-commit clear, symlink baseDir, gh/git flag handling

Two Critical items, two Copilot, and five wenshao Suggestions:

- attachCommitAttribution's `finally` block used to call
  `clearAttributions()` unconditionally, wiping per-file tracking
  for files the AI had edited but the user excluded from this
  commit. Added `clearAttributedFiles(committedAbsolutePaths)` to
  the service and the call site now passes only the paths that
  actually landed in this commit; entries for un-`add`ed files stay
  pending for a later commit.

- generateNotePayload now runs both `baseDir` and each tracked
  absolute path through `fs.realpathSync` before `path.relative`.
  On macOS in particular `/var` symlinks to `/private/var`, so the
  toplevel from `git rev-parse --show-toplevel` and the absolute
  path captured by edit/write-file tools could diverge — producing
  `../../actual/path` keys in the lookup that never matched and
  silently zeroed all per-file AI attribution.

- tokeniseSegment now consumes value-taking sudo flags (`-u`,
  `-g`, `-h`, `-D`, `-r`, `-t`, `-C`, plus the long forms). Without
  this, `sudo -u other git commit` left `other` standing in for
  the program name and skipped the trailer entirely.

- A duplicate JSDoc block above `countCommitsAfter` (a leftover
  from the earlier extraction of `getGitHead`) was removed; both
  helpers now have one accurate comment each.

- attachCommitAttribution's multi-commit guard now also runs when
  `preHead === null` (brand-new repo), via `git rev-list --count
  HEAD`. A compound `git init && git commit -m a && git commit -m b`
  no longer slips through and mis-attributes combined data to the
  last commit.

- addCoAuthorToGitCommit's `-m` matching switched to `matchAll` and
  takes the LAST match. `git commit -m "title" -m "body"` puts the
  trailer at the end of the body so `git interpret-trailers`
  recognises it; the previous first-match behaviour stuffed the
  trailer in the title where git treats it as plain message text.

- addAttributionToPR's `--body` regex accepts both space and
  `=` separators (`--body "..."` and `--body="..."`); the `=` form
  is common with gh.

- New `parseGhInvocation` walks past gh's global flags
  (`--repo`, `-R`, `--hostname`) so `gh --repo owner/repo pr
  create ...` is detected. The earlier fixed-position check at
  tokens[1]/tokens[2] missed any command with a global flag.

- getCommittedFileInfo now fans out the two `rev-parse` calls and
  the three diff calls with `Promise.all`. They're independent and
  serialising them was paying spawn latency 5× per commit.

Tests: sudo with `-u user`, multi `-m`, `gh --repo owner/repo`,
`--body="..."`, plus the existing 84 shell tests still pass.

* fix(attribution): canonicalize file paths centrally in CommitAttributionService

Two related Copilot follow-ups:

- recordEdit/getFileAttribution/clearAttributedFiles now run input
  paths through fs.realpathSync before storing/looking up, so a
  symlinked path (e.g. macOS /var ↔ /private/var) resolves to the
  same key regardless of which form the caller passes. Previously
  edit.ts/write-file.ts handed in non-realpath'd absolute paths
  while generateNotePayload tried to realpath only inside its
  lookup loop, leaving partial-clear and clear-on-finally paths
  unable to find entries when the forms diverged.

- restoreFromSnapshot also canonicalises on the way in so a
  session resumed from a pre-fix snapshot (where keys may not
  have been canonical) ends up with the same shape as newly
  recorded entries — otherwise a single file could end up with
  two parallel records.

- generateNotePayload's lookup loop dropped its per-entry realpath
  call (now redundant since keys are canonical at write time),
  keeping only the realpath of `baseDir` (which still comes from
  `git rev-parse --show-toplevel` and may be a symlink).

- Updated `clearAttributedFiles` doc to describe the new semantics:
  callers can pass either the resolved repo-relative path or an
  already-canonical absolute path, and either will match.

* fix(attribution): canonicalize-from-root cleanup; fix mixed-quote -m / gh -R=

Five review items, one Critical:

- attachCommitAttribution now canonicalises via the repo *root* (one
  realpath call) and resolves committed paths against that canonical
  root, rather than per-leaf realpath inside clearAttributedFiles.
  At cleanup time the leaf for a just-deleted file no longer exists,
  so per-leaf fs.realpathSync would fail and silently fall back to a
  non-canonical path that misses the stored canonical key — leaving
  stale attributions for deleted files.
  clearAttributedFiles drops its internal realpath and now documents
  the canonical-paths-required precondition explicitly.

- addCoAuthorToGitCommit picks the LAST `-m` regardless of quote
  style. Previously `doubleMatch ?? singleMatch` always preferred
  the last double-quoted match, so `git commit -m "Title" -m
  'Body'` injected the trailer into the title where git
  interpret-trailers would silently ignore it. Now compares match
  indices, and the escape helper follows the actually-selected
  match's quote style.

- parseGhInvocation handles `-R=value` (the equals form of the
  short `--repo` alias). `--repo=...` and `--hostname=...` were
  already covered; `-R=...` previously fell through to the generic
  flag branch and skipped the value.

- New tests for the symlink-aware canonicalisation: macOS-style
  `/var` ↔ `/private/var` mapping is mocked via vi.mock on
  node:fs, with cases for record-then-look-up under either form,
  generateNotePayload with a symlinked baseDir, partial clear via
  the canonical-root-derived path (deleted leaf), and snapshot
  restore canonicalisation.

- Doc-only: integration-test header comments updated from
  "V1 -> V2 -> V3" / "migration to V3" to reflect the actual V4
  end state (assertions already used the literal `4`).

* fix(shell): scope -m rewrite to commit segment, reject nested matches

Two Critical findings on addCoAuthorToGitCommit, plus a Copilot
maintainability nit:

- The `-m` regex used to scan the whole compound command, so
  `git commit -m "fix" && git tag -a v1 -m "release"` would target
  the LATER tag annotation (last -m wins) and splice the trailer
  there instead of the commit message. The rewrite now scopes to
  the actual `git commit` segment via a new
  findAttributableCommitSegment(): same shell-aware walk
  gitCommitContext does, but returning the segment's character
  range so the regex can be run on a slice and spliced back into
  the original command.

- Within the segment, a literal `-m '...'` *inside* a quoted body
  was treated as a real later -m. For
  `git commit -m "docs mention -m 'flag' for completeness"`, the
  inner single-quoted -m sits at a higher index than the real
  outer -m, and the previous index comparison would have it win —
  splicing the trailer mid-message and corrupting the quoting.
  The new code checks whether the candidate is nested inside the
  other quote-style's range (start/end containment) and prefers
  the outer match when so.

- Hoisted three constant Sets (sudo flag list, git global flags
  taking values, git global flags shifting cwd, gh global flags)
  out of the per-call scope to module constants. Functional
  no-op, but keeps the parsing helpers easier to read and avoids
  re-allocating the Sets on every command.

Two regression tests added for the cases above:
- inner `-m '...'` inside the outer message body is preserved
  literally and the trailer lands after the body
- `git tag -a v1 -m "release notes"` after a real
  `git commit -m "fix"` is left untouched, with the trailer
  appended to "fix" only

* fix(attribution): cd-leak, numstat partial failure, $() bailout, gh pr new alias

Five Critical/Suggestion items:

- `cd subdir && git commit` (or any non-attributable commit chain
  whose HEAD movement still happens in our cwd, e.g. cd into a
  subdirectory of the same repo) used to skip attribution AND fail
  to clear pending per-file entries. Those entries then leaked into
  the next foreground commit, inflating its AI percentage. New
  `else if (commitCtx.hasCommit)` branch in execute() compares pre-
  and post-HEAD; if HEAD moved we drop the per-file state. preHead
  is now snapshotted whenever ANY commit was attempted, not only
  attributable ones.

- getCommittedFileInfo's three diff calls run in `Promise.all`. If
  `--numstat` failed while `--name-only` succeeded, every file's
  diffSize would be 0 and generateNotePayload would clamp aiChars
  to 0 — emitting a structurally valid note with all-zero AI
  percentages. Detect the partial-failure shape (files non-empty,
  diffSizes empty) and return empty so no note is written.

- addCoAuthorToGitCommit and addAttributionToPR now bail when the
  captured `-m`/`--body` value contains `$(`. The tool description
  recommends `git commit -m "$(cat <<'EOF' ... EOF)"` for
  multi-line messages, but the regex's `(?:[^"\\]|\\.)*` body group
  stops at the first interior `"` from a nested shell token —
  splicing the trailer there breaks the command before it reaches
  the executor.

- looksLikeGhPrCreate now accepts `gh pr new` as well — it's a
  documented alias for `gh pr create` and was silently skipped.

- Removed `incrementPermissionPromptCount` / `incrementEscapeCount`
  and their getters: they had no production callers, so the backing
  fields just round-tripped through snapshots as 0. The four
  snapshot fields are now optional so pre-fix snapshots that carry
  non-zero values still load cleanly and just get ignored.

Three regression tests added: heredoc-style `-m "$(cat <<EOF...)"`
preserved literally, heredoc-style `--body` likewise, `gh pr new
--body "..."` rewritten with attribution.

* fix(attribution): --amend, --message/-b aliases, .d.ts over-exclusion

Four Copilot follow-ups, three of them user-visible coverage gaps:

- `git commit --amend` was diffing `HEAD~1..HEAD` for attribution,
  which spans the entire amended commit (parent → amended) rather
  than the actual amend delta. A message-only amend would emit a
  note attributing every file in the original commit to this
  amend. New `isAmendCommit` helper detects the flag and
  getCommittedFileInfo switches to `HEAD@{1}..HEAD` (the pre-amend
  HEAD vs the amended HEAD); if the reflog is GC'd we bail with a
  warning rather than over-attribute.

- `git commit --message "..."` and `--message="..."` were silently
  skipped because the regex only recognised the short `-m` form.
  The flag prefix now matches both alternatives via
  `(?:-[a-zA-Z]*m|--message)\s*=?\s*` (non-capturing inner group
  so the existing `[full, prefix, body]` destructure still works).

- `gh pr create -b "..."` (the short alias for `--body`) was the
  same gap on the PR side; `(?:--body|-b)[\s=]+` now covers both
  forms.

- `.d.ts` was an over-broad blanket exclusion in
  EXCLUDED_EXTENSIONS — declaration files are commonly authored
  (ambient declarations, asset shims like `*.d.ts` for
  `import './x.svg'`); the repo even contains
  `packages/vscode-ide-companion/src/assets.d.ts`. Removed `.d.ts`
  from the extensions Set and adjusted the test to assert the new
  behavior. Auto-generated `.d.ts` (e.g. `tsc --declaration`
  output) still gets caught by the build-directory rules.

Tests added: `--amend` plumbing covered by the new branch in
getCommittedFileInfo (no targeted unit test — the diff invocation
goes through ShellExecutionService and is exercised by the existing
post-command path); `--message`/`--message="..."`/-b/-b="..."` all
have positive trailer-injection assertions; `.d.ts` test split into
"hand-authored" (negative) and "in dist" (positive).

* fix(attribution): cd-subdir, scope --body, multi-commit count guard, /clear reset

Four bugs flagged this round:

- gitCommitContext / findAttributableCommitSegment used a blanket
  "any cd shifts cwd" gate, breaking the very common
  `cd subdir && git commit -m "..."` flow even though the commit
  lands in the same repo. New `cdTargetMayChangeRepo` heuristic:
  treat relative paths that don't escape upward (no leading `..`,
  no absolute path, no `~`/`$VAR` expansion, no bare `cd`/`cd -`)
  as in-repo and let attribution proceed. Conservative on anything
  it can't statically verify.

- addAttributionToPR was running the `--body`/`-b` regex against
  the FULL compound command string. In
  `curl -b "session=abc" && gh pr create --body "summary"` the
  regex would match curl's `-b` cookie flag and inject attribution
  into the cookie value, corrupting the curl call. Added
  `findGhPrCreateSegment` (analog of `findAttributableCommitSegment`)
  and scoped the body regex to that segment, splicing back into
  the original command via offsetting the in-segment match index.

- The multi-commit guard treated `runGitCount === 0` as "single
  commit" and bypassed itself. After `commitCreated === true`, a
  count of 0 is impossible in normal operation — it means
  rev-list errored or timed out. Now we bail on `commitCount !== 1`
  with a tailored message: anything other than exactly 1 commit
  is suspicious and refuses the note.

- The CommitAttributionService singleton survives across
  `Config.startNewSession()` (the `/clear` and resume paths). New
  `CommitAttributionService.resetInstance()` call alongside the
  existing chat-recording / file-cache resets in startNewSession
  prevents pending attributions from a prior session attaching to
  a commit in the new one.

Three regression tests added: `cd src && git commit` produces a
trailer (in-repo cd), `cd .. && git commit` does not (could escape
repo root), and `curl -b "..." && gh pr create --body "..."` leaves
curl's cookie value untouched while attribution lands in gh's body.

* fix(attribution): cd embedded .., env wrapper, Windows ARG_MAX, segment-locator warn

Four review items, all small but real:

- cdTargetMayChangeRepo missed embedded `..` traversal — `cd
  foo/../../escape` and similar would slip past the leading-`..`
  check and be treated as in-repo. Added an `includes('/..')` /
  `includes('\\..')` check (catches POSIX and Windows separators
  without false-positiving on `..` chars inside ordinary names,
  which only escape when followed by a separator).

- tokeniseSegment now recognises `env` as a safe wrapper alongside
  `sudo`/`command`, so `env GIT_COMMITTER_DATE=now git commit ...`
  resolves to `git`. After the wrapper detection we also skip any
  `KEY=VALUE` argv entries (env's own argument syntax for setting
  vars before the program).

- buildGitNotesCommand's MAX_NOTE_BYTES dropped from 128 KB to
  30 KB. Windows' CreateProcess lpCommandLine is capped around
  32,768 UTF-16 chars including the executable path and other argv
  entries; a 128 KB note would still fail to spawn even though
  the function returned a command instead of null. 30 KB leaves
  ~2 KB of headroom for the rest of the argv on Windows and is
  larger than any real commit's metadata in practice.

- findAttributableCommitSegment / findGhPrCreateSegment now log a
  debugLogger.warn when `command.indexOf(sub, cursor)` returns -1
  — splitCommands strips line continuations (`\<newline>`), so a
  multi-line command can have the trimmed segment text fail to
  match its source. Previously the segment was silently skipped
  with no signal; the warn makes the failure observable when
  QWEN_DEBUG_LOG_FILE is set.

Two regression tests added: `cd foo/../../escape && git commit`
gets no trailer (embedded-`..` heuristic catches it), and
`env GIT_COMMITTER_DATE=now git commit` does (env wrapper skipped).

* fix(attribution): scope isAmendCommit to attributable segment only

`git -C ../other commit --amend && git commit -m x` would previously
flag the second (fresh) commit as an amend, causing
attachCommitAttribution to diff `HEAD@{1}..HEAD` against an unrelated
reflog entry. Mirror findAttributableCommitSegment's cd/cwd tracking
so only the first commit segment that runs in the original cwd
determines amend status.

* fix(attribution): last-match --body, symlink leaf canonicalisation, scoped prompt count

- addAttributionToPR: use matchAll/last-match for `--body`/`-b` so the
  trailer lands in the gh-honoured (final) body when multiple flags are
  present. Mirrors addCoAuthorToGitCommit. Adds regression test.
- attachCommitAttribution: also fs.realpathSync the per-file resolved
  path (not just the repo root) so files behind intermediate symlinks
  are matched against canonical keys recordEdit stored, instead of
  silently zeroing attribution and leaking entries past commit.
- incrementPromptCount: scope to SendMessageType.UserQuery — ToolResult,
  Retry, Hook, Cron, Notification are model/background re-entries of
  the same logical turn. Tracking them all inflated the "N-shotted"
  trailer (one user message could become 10-shotted with 10 tool calls).
- AttributionSnapshot: add `version: 1` field; restoreFromSnapshot now
  refuses incompatible versions and validates per-field types so a
  partially-written snapshot can't seed `Math.min(undefined, n) === NaN`
  into git-notes payloads.
- Drop unused permission/escape counters (declared, persisted, never
  read or incremented) — fields, snapshot tolerance, and clear-method
  bookkeeping all removed; AttributionSnapshot interface simplifies.
- isGeneratedFile: switch directory rule from substring `.includes('/dist/')`
  to segment-boundary check (split on `/`) so project dirs like
  `my-dist/` or `xbuild/` don't match. `.lock` removed from the blanket
  extension exclusion — well-known lockfiles already covered by
  EXCLUDED_FILENAMES; hand-authored `.lock` files (e.g. `.terraform.lock.hcl`)
  now stay attributable.
- getClientSurface: document `QWEN_CODE_ENTRYPOINT` as the embedder
  override hook so the always-`'cli'` default is intentional.

* fix(attribution): skip values for env -u NAME and -S string

`env`'s value-taking flags (`-u`/`--unset`, `-S`/`--split-string`) were
not in the wrapper's flag-skip allowlist, so `env -u FOO git commit ...`
left FOO as the next token and the parser treated it as the program —
masking the real `git commit` from attribution detection. Add an
ENV_FLAGS_WITH_VALUE table mirroring the sudo allowlist. Regression
test added.

* fix(attribution): submodule leak, PR body nesting, shallow-clone bail, schema default

- attachCommitAttribution: when HEAD didn't move in our cwd, leave
  pending attributions alone instead of dropping them. The case can be
  a failed commit, `git reset HEAD~1`, OR `cd submodule && git commit`
  (inner repo's HEAD moves, ours doesn't). Dropping was overly
  aggressive and silently lost outer-repo edits in the submodule case.
- addAttributionToPR: mirror addCoAuthorToGitCommit's nested-match
  rejection so `gh pr create --body "docs mention -b 'flag'"` picks the
  outer `--body`, not the inner literal `-b`. Splicing into the inner
  match would corrupt the body. Regression test added.
- getCommittedFileInfo: when `rev-parse --verify HEAD~1` fails, also
  check `rev-list --count HEAD === 1` to confirm HEAD is the true
  root commit. In a shallow clone, HEAD~1 is unreadable but the commit
  has a parent recorded — falling back to `diff-tree --root` would
  diff against the empty tree and over-attribute the entire commit.
  Bail with a debug warning instead.
- generate-settings-schema: lift `default` (and `description`) out of
  the inner `anyOf[N]` schema to the outer level when wrapping with
  `legacyTypes`. Most JSON-schema-driven editors only surface
  top-level defaults; burying the default under `anyOf` lost the
  "enabled by default" hint. Also extend the default filter to
  publish non-empty plain objects (so `gitCoAuthor`'s default can
  appear). gitCoAuthor's source default updated to the runtime shape
  `{commit: true, pr: true}` to match `normalizeGitCoAuthor`.

* fix(attribution): drop unsafe full-clear, tag analysis-failure with null

ju1p (Copilot): the `else if (commitCtx.hasCommit)` branch fully
cleared the singleton on `cd /abs/same-repo/subdir && git commit`
(or `git -C . commit`), losing pending AI edits the user hadn't
staged. We can't tell which files were in the commit from this
branch, and the next attributable commit's partial-clear handles
cleanup correctly anyway. Drop the branch entirely.

ju2D (Copilot): `getCommittedFileInfo` returned the same empty
StagedFileInfo for both "could not analyze" (shallow clone, --amend
without reflog, --numstat partial failure, exception) and
"intentionally empty" (--allow-empty). The caller couldn't tell them
apart, so the partial clear became a no-op on analysis failure and
the just-committed AI edits leaked to the next commit. Switch the
return type to `StagedFileInfo | null` and have the caller treat
null as "fall back to full clear" while empty StagedFileInfo
(--allow-empty) leaves attributions intact for the next real commit.

* fix(attribution): dedup snapshot writes, cap excludedGenerated, doc commit toggle scope

rsf- (Copilot): recordAttributionSnapshot wrote a full snapshot to
the JSONL on every non-retry turn, even when the tracked state was
unchanged. Long-running sessions accumulated thousands of identical
snapshot copies, inflating session size and slowing /resume hydrate.
Dedup by JSON-equality with the prior write — first write always
goes through, identical successors are no-ops.

rsgo (Copilot): excludedGenerated path list was unbounded. A commit
churning thousands of generated artifacts (large dist/ rebuild)
could push the JSON note past MAX_NOTE_BYTES (30KB) and lose
attribution for the real source files in the same commit. Cap the
serialized sample at MAX_EXCLUDED_GENERATED_SAMPLE (50) and add
excludedGeneratedCount for the true total.

rsg9 + rshM (Copilot): the gitCoAuthor.commit description claimed
the toggle only controlled the Co-authored-by trailer, but
attachCommitAttribution also gates the per-file git-notes payload
on the same flag. Update both the schema description and the
settings.md table to mention both effects so disabling the option
isn't a silent surprise.

* fix(attribution): depth-1 shallow detection, snapshot dedup post-rewind/post-failure

sfGz (Copilot): rev-list --count HEAD === 1 cannot distinguish a
true root commit from a depth-1 shallow clone — both report 1
because rev-list only walks locally available objects. Switch to
git log -1 --pretty=%P HEAD which reads the parent SHA directly
from commit metadata: empty means a real root, non-empty means a
parent is recorded (whether or not its object is local). The
shallow-clone bail is now reliable.

sfIm (Copilot): the dedup key persisted across rewindRecording, so
the previous snapshot living on the now-abandoned branch would
match the next post-rewind snapshot and silently skip the write,
leaving /resume on the rewound session with no attribution state.
Reset lastAttributionSnapshotJson when rewindRecording fires.

sfJE (Copilot): dedup key was committed before the async write
settled. A transient write failure would update the key, then
permanently suppress all future identical snapshots even though
nothing was ever persisted. Switch to optimistic-set then rollback
on appendRecord rejection — synchronous identical calls dedup
cleanly, but a failed write clears the key so the next identical
snapshot retries. appendRecord now returns the per-record write
promise (writeChain still has its swallow-catch for chain liveness)
so callers needing per-write success can react to it. Tests added
in chatRecordingService.test.ts for both rewind-reset and
rollback-on-failure paths.

* fix(attribution): preHead race, regex apostrophe-escape, surface failures, dead code

t2G0 (deepseek-v4-pro): addCoAuthorToGitCommit single-quote regex now
matches the bash close-escape-reopen apostrophe form using
((?:[^']|'\\'')*) — the same pattern bodySinglePattern uses for
gh pr create. Input like git commit -m 'don'\''t' was previously
silently un-rewritten because the negative lookahead bailed; the
trailer now lands at the FINAL closing quote. Test updated.

tMBP (gpt-5.5): preHead capture switched from concurrent async
getGitHead to a synchronous getGitHeadSync (execFileSync) BEFORE
ShellExecutionService.execute spawns the user's command. A fast
hot-cached git commit could move HEAD before the async rev-parse
resolved, leaving preHead === postHead and silently skipping the
attribution note. Trade ~10–50 ms event-loop block per
commit-shaped command for correctness of the post-command HEAD
comparison.

t2Gv (deepseek-v4-pro): attribution write failures (note exec
non-zero, payload too large, diff-analysis exception, shallow
clone / amend-without-reflog) are now surfaced on the shell tool's
returnDisplay AND llmContent so the user and agent both see when
their commit succeeded but the per-file git note didn't land.
attachCommitAttribution now returns string | null (warning text or
null for intentional skips like no-tracked-edits). Co-authored-by
trailer is unaffected — only the note is gated by these failures.

t2Gy (deepseek-v4-pro): committedAbsolutePaths now matches against
the canonical keys already stored in fileAttributions
(matchCommittedFiles iterates by relative path against the
canonical repo root) instead of re-resolving each diff path
on the fly. realpathSync(resolved) failed for deleted files and
didn't follow intermediate symlinks, leaving stale per-file
attribution alive past commit and inflating AI percentages on
subsequent commits.

t2HI (deepseek-v4-pro): removed dead sessionBaselines /
FileBaseline / contentHash / computeContentHash infrastructure
(~40 lines). The fields were written, persisted, and restored but
never read for any computation or decision. AttributionSnapshot
schema stays at version 1 — restore tolerates pre-fix snapshots
that carried the now-ignored baselines field.

t2HM (deepseek-v4-pro): extracted the duplicated lastMatch helper
in addCoAuthorToGitCommit and addAttributionToPR into a single
module-level lastMatchOf so future fixes can't be applied to only
one copy.

* chore(schema): regenerate settings.schema.json to match gitCoAuthor.commit description

The settingsSchema.ts source for `gitCoAuthor.commit.description` was
updated in 3c0e3293b but the JSON schema only picked up the OUTER
description rewrite and missed this inner property's. The Lint check
("Check settings schema is up-to-date") fails on that drift; this
commit re-runs `npm run generate:settings-schema` to sync them.

* fix(attribution): preserve unstaged AI edits across cleanup branches

uxU5 + uxVQ + uxUO (Copilot): every cleanup branch in
attachCommitAttribution that called clearAttributions(true) was
wholesale-erasing pending AI edits for files the user never staged
in this commit. Reviewer scenarios:
- multi-commit chain (`commit a && commit b`) bails out without
  writing a note, but unstaged edits to file Z (touched by neither
  commit) get cleared along with the chain's committed files.
- attribution toggle off: same — toggling the flag wipes pending
  unstaged work.
- analysis failure (shallow clone, --amend without reflog, partial
  diff failure): the finally-block fallback wholesale-cleared
  every pending file, consuming unrelated AI edits.
- 0%-AI commit: when no file in the commit was AI-touched,
  generateNotePayload was emitting an "0% AI" note attached to a
  commit that legitimately had no AI involvement — actively
  misleading metadata.

Add `noteCommitWithoutClearing()` to the service: snapshots the
prompt counter as the new "at last commit" but leaves the per-file
map alone. Use it in the multi-commit, no-tracked-edits,
toggle-off, and analysis-failure paths. The committed-files
partial-clear (clearAttributedFiles) still runs in the success
path. The 0%-AI no-match case now skips the note write entirely.

* fix(attribution): runGit null-on-failure, versionless v3→v4 migration

z54M (Copilot): runGit returned '' on both successful-empty-output
and silent failure, so a `--name-only` that errored mid-way through
the diff fan-out aliased to a real `--allow-empty` commit. The
empty-commit branch then preserved pending attributions, leaving
the just-committed file's tracked AI edit alive to re-attribute on
the next commit. Switch runGit to `Promise<string | null>`,
distinguishing exit code 0 (any output, including '') from non-zero
(null). The diff-stage fan-out and ancillary probes now treat null
as analysis failure and bail with `return null` instead of falling
into the empty-commit path.

z539 (Copilot): the v3→v4 `shouldMigrate` only fired on
`$version === 3`. A versionless settings file carrying the legacy
`general.gitCoAuthor: false` boolean would skip every migration
(gitCoAuthor isn't in V1_INDICATOR_KEYS — it post-dates V2), get
its `$version` normalized to 4 by the loader, and leave the
boolean in place. The settings dialog then reads the V4
`{commit, pr}` shape, sees missing keys, defaults both to true, and
silently overwrites the user's opt-out on the next save. Also fire
when `$version` is absent AND the value at `general.gitCoAuthor`
is a boolean. Tests cover the new path and confirm the existing
versioned/object-shape paths are untouched.

* fix(attribution): toggle-off partial clear, normalizeGitCoAuthor type-check, terraform lockfile

0oAK (Copilot): the gitCoAuthor.commit toggle-off branch returned
before computing the committed file set, leaving the just-committed
files' tracked AI work in the singleton. Re-enabling the toggle and
committing the same file again would re-attribute earlier (already-
committed) AI edits to the new commit. Move the toggle gate AFTER
matchCommittedFiles so the finally block does a proper partial clear
of the just-committed files even when the note write is skipped.

0oAg (Copilot): normalizeGitCoAuthor copied value?.commit / value?.pr
without type-checking. settings.json is hand-editable; a stored
`{ commit: "false" }` reached runtime as a truthy string and behaved
as if attribution were enabled. Add a per-field bool coercion that
falls back to the schema default (true) for any non-boolean,
matching what the dialog and IDE schema already imply. Tests cover
the string / number / null cases.

0oAo (Copilot): v3→v4 shouldMigrate only special-cased versionless
legacy booleans — versionless files with invalid gitCoAuthor values
(`"off"`, `[]`, etc.) skipped the migration and the loader stamped
`$version: 4` over the bad value. Runtime normalization then
silently re-enabled attribution. Extend shouldMigrate to fire on ANY
versionless non-object value at general.gitCoAuthor; the existing
migrate() body's drop-and-warn path resets it. Already-object
shapes (hand-edited to v4) still skip cleanly. Tests added.

0oAt (Copilot): `.terraform.lock.hcl` got dropped from generated-file
exclusion when `.lock` was removed from the blanket extension list
in 3c0e3293b. It's a generated provider lockfile in the same class
as `package-lock.json` and dominates Terraform-repo commits. Re-add
to EXCLUDED_FILENAMES and add a regression test covering both
repo-root and module-nested locations.

* fix(attribution): harden restoreFromSnapshot against corrupt payloads

1KMY (Copilot): snapshot.surface was copied without type validation.
A corrupted/partially-written snapshot with a non-string surface
(e.g. {}, 42, null) would later be serialized into the git note as
"[object Object]" and used as a Map key downstream, breaking the
expected payload shape. Type-check and fall back to the current
client surface for any non-string (or empty-string) value.

1KLq (Copilot): per-field sanitiseCount enforced
`promptCount >= 0` and `promptCountAtLastCommit >= 0` independently,
but never the cross-field invariant. A snapshot with
promptCountAtLastCommit > promptCount would surface a negative
getPromptsSinceLastCommit() and propagate as a "(-N)-shotted"
trailer into PR text. Clamp atLastCommit to total on restore.

1KL_ (Copilot): when a snapshot carried both the symlinked and
canonical paths for the same file (a session straddling the
canonicalisation fix), `set(realpathOrSelf(k), ...)` overwrote the
first entry with the second, silently dropping the AI contribution
the first form had accumulated. Merge instead: sum aiContribution
and OR aiCreated when collapsing duplicate keys.

Tests cover all three branches: non-string surface fallback,
promptCount clamp, and duplicate-key merge.

* fix(attribution): roll back snapshot dedup key on sync appendRecord failure

1UMh (Copilot): appendRecord can throw synchronously before returning
a promise — e.g. when ensureConversationFile() rethrows a non-EEXIST
writeFileSync error. The async .catch() handler attached to the
promise never runs in that case, so the optimistic dedup-key set
sticks on a write that never landed and permanently suppresses
identical retries. Roll back lastAttributionSnapshotJson in the outer
catch too. Regression test forces writeFileSync to throw EACCES on
the first invocation, then asserts the second identical snapshot
attempt fires a fresh write rather than getting deduped.

* docs(attribution): align cleanup-branch comments with noteCommitWithoutClearing

Three doc/test-fixture stale-after-refactor cleanups (Copilot
4MDx / 4MEI / 4MEa):

- shell.ts:1944 (around the stagedInfo === null branch): the comment
  still claimed the finally block "falls back to a full clear", but
  1ece87438 switched analysis-failure cleanup to
  noteCommitWithoutClearing(). Update the comment so the reasoning
  matches what the code actually does (and so a future reader doesn't
  reintroduce the wholesale clear thinking it's already there).

- shell.ts: getCommittedFileInfo docstring carried the same stale
  "full clear" claim for the `null` return value. Update to describe
  the noteCommitWithoutClearing() fallback and the smaller-evil
  trade-off for the just-committed file.

- chatRecordingService.test.ts: baseSnapshot fixture for the
  recordAttributionSnapshot tests still carried `baselines: {}`,
  even though that field was removed from AttributionSnapshot in
  296fb55ae's dead-code purge. Structural typing let it compile,
  but the fixture didn't reflect the production shape — drop it.

* fix(attribution): restore fire-and-forget appendRecord, route rollback via callback

6OcJ (Copilot): refactor in 715c258fb returned a Promise from
appendRecord so the snapshot dedup-key path could chain rollback —
but recordUserMessage / recordAssistantTurn / recordAtCommand /
recordSlashCommand / rewindRecording all call appendRecord without
await or .catch(). A transient jsonl.writeLine rejection on any of
those would surface as an unhandled-promise-rejection (warning, or
crash on --unhandled-rejections=throw).

Restore the original fire-and-forget semantics: appendRecord again
returns void and internally swallows async failures (logging via
debugLogger). Per-record failure reactions are routed through an
optional onError callback — recordAttributionSnapshot uses this to
roll back lastAttributionSnapshotJson when the write that set it
ends up rejecting.

Tests: add a fire-and-forget regression that mocks writeLine to
reject and asserts no unhandledRejection events fire while the
existing snapshot rollback tests (sync + async) still pass via the
new callback path.

* fix(attribution): GIT_DIR repo-shift bail, snapshot envelope validation, narrow legacyTypes

80ME (gpt-5.5 /review, [Critical]): tokeniseSegment unconditionally
stripped every leading KEY=value token. `GIT_DIR=elsewhere/.git git
commit ...` was therefore treated as an in-cwd commit, picked up the
Co-authored-by trailer, and produced a per-file note that landed
against our cwd's HEAD even though the actual commit went to a
different repo. Define a GIT_ENV_SHIFTS_REPO set (GIT_DIR,
GIT_WORK_TREE, GIT_COMMON_DIR, GIT_INDEX_FILE, GIT_NAMESPACE) and
have tokeniseSegment refuse to parse any segment whose leading env
block (including the env-wrapper's KEY=VALUE block) carries one of
these. Identity / date variables (GIT_AUTHOR_*, GIT_COMMITTER_*) are
deliberately NOT in the set — they tweak metadata but don't relocate
the repo. Tests cover plain prefix, env-wrapped prefix, and a
GIT_COMMITTER_DATE positive control that should still get the trailer.

8EeQ (Copilot): restoreFromSnapshot received `snapshot as
AttributionSnapshot` from a structural cast off `unknown` (the
resume path), so its TS-typed shape was only a hint. A corrupted
JSONL line (non-object / array / wrong type discriminator / missing
type) would skip past the version check straight into
Object.entries(snapshot.fileStates) — and a non-object fileStates
(an array, say) seeded fileAttributions with numeric-string keys.
Add envelope-level shape gates (isPlainObject + type discriminator)
and a fileStates plain-object check before iterating; both bail to a
clean reset rather than poisoning the singleton. Tests added.

8Eej (Copilot): SettingDefinition.legacyTypes was typed as
SettingsType[] which includes 'enum' and 'object' — JSON Schema's
`type` keyword doesn't accept those values. Adding
`legacyTypes: ['enum']` would silently produce an invalid
settings.schema.json. Narrow the field's type to
ReadonlyArray<'boolean' | 'string' | 'number' | 'array'> (the
JSON-Schema-primitive subset). Future complex-shape legacy support
should land its own branch in convertSettingToJsonSchema.

* docs(attribution): correct legacyTypes / EXCLUDED_DIRECTORY_SEGMENTS comments

9Ta_ (Copilot): the JSDoc on legacyTypes claimed JSON Schema's
`type` keyword does not accept `'object'` — that's wrong; `'object'`
IS a valid JSON Schema type. Reword to reflect the actual rationale:
`'enum'` is not a valid JSON Schema `type` value at all (enum
constraints use the `enum` keyword), and a bare `{type: 'object'}`
would accept any object regardless of what the field's pre-expansion
shape actually allowed. The narrowed `boolean | string | number |
array` set is exactly what the one-liner generator can faithfully
emit; richer legacy shapes belong in their own branch of
convertSettingToJsonSchema.

9Tbs (Copilot): the comment in generatedFiles.ts referenced
`EXCLUDED_DIRECTORIES`, but the constant is `EXCLUDED_DIRECTORY_SEGMENTS`
(renamed during the segment-boundary refactor). Update the
reference so a future maintainer scanning for the rule doesn't
chase a non-existent identifier.

* fix(attribution): SHA-pin git notes, on-disk hash divergence detection, env -C cwd-shift

tanzhenxin review #1 — Note targets symbolic HEAD, not captured SHA:
buildGitNotesCommand hard-coded 'HEAD' as the target; postHead was
captured at commit-detection time but only used for the !== preHead
diff. Between that capture and the execFile, three more awaited git
calls run — anything that moves HEAD in the same cwd (post-commit
hook, chained `commit && tag -m`, parallel process) silently lands
the note on the wrong commit because of `-f`. Thread postHead
through buildGitNotesCommand as a required `targetCommit` arg.
Test asserts the targeted SHA, not the symbolic ref.

tanzhenxin review #2 — Accumulator has no baseline:
recordEdit was monotonic per-path with no reset for out-of-band
mutations. Re-instate FileAttribution.contentHash and:
- recordEdit hashes the input `oldContent` and resets the per-file
  accumulator if it doesn't match what AI's last write recorded
  (catches paste-replace via external editor, manual save, etc.
  WHEN AI subsequently edits the same file again).
- New validateOnDiskHashes() rehashes every tracked file's CURRENT
  on-disk content and drops entries whose hash diverged. Called
  from attachCommitAttribution before matchCommittedFiles so a
  commit can never credit AI for a human-only diff. Deleted files
  (readFileSync throws) are left alone — the commit's deletion
  record is what the note should reflect.

tanzhenxin review #4 — Failed-commit / staleness leak:
The recordEdit divergence check above + commit-time
validateOnDiskHashes together catch tanzhenxin's exact scenario
(AI edits a.ts → hook rejects → user manually edits a.ts → user
commits → no AI credit because validateOnDiskHashes drops the
stale entry). The !commitCreated branch still preserves
attributions to keep the submodule case working — the staleness
problem is now solved at the next commit's validation step.

Self-review item — env -C / --chdir treated as repo-shifting:
Added ENV_FLAGS_SHIFT_CWD set covering -C / --chdir. tokeniseSegment
returns null for `env -C DIR git commit ...` segments — same
contract as a leading GIT_DIR=... assignment. Without this we'd
either misidentify /elsewhere as the program (silently dropping
attribution) or, worse if -C went into the value-skip set,
trailer-inject onto a commit that lands in /elsewhere's repo. Tests
added alongside the existing GIT_DIR repo-shift cases.

339 tests pass; typecheck clean.

* fix(attribution): pickBool intent-aware, shouldClear gate, ETIMEDOUT surface, drop dead exports

-wgA + -wg0 (deepseek): pickBool defaulted non-boolean to true,
turning a hand-edited `{ commit: "false" }` into enabled
attribution. Replace with intent-aware parsing: "true"/"yes"/"on"/
"1" → true, "false"/"no"/"off"/"0"/"" → false, anything else
(unknown strings, non-1 numbers, objects, arrays, null) → false.
Genuinely-absent sub-fields still default to true (schema default).
Migration test scenarios covered. Tests now cover ~17 input cases
across both string/number/null/object/unknown forms.

-wgq (deepseek): when buildGitNotesCommand returned null (oversized
payload) or git notes itself failed, the finally block called
clearAttributedFiles(committedAbsolutePaths) — irreversibly
deleting per-file attribution data the user might need to amend &
retry. Introduce a separate `shouldClear` set that's only assigned
on successful note write OR explicit toggle-off. Failure paths
(oversized, exitCode != 0, exception, analysis failure) leave
shouldClear null so the finally block calls noteCommitWithoutClearing
instead — preserving per-file state for the user's recovery.

9p7W (Copilot): execFile callback coerced ETIMEDOUT / SIGTERM
(timeout) into a generic exitCode=1 warning. Detect both
`error.code === 'ETIMEDOUT'` and `error.killed === true &&
error.signal === 'SIGTERM'` so the user-visible warning correctly
names "timed out after 5s" instead of "exited 1".

-wg7 (deepseek): formatAttributionSummary and getAttributionNotesRef
were exported but had zero production callers (only tests). Remove
the dead exports + their tests (~40 LOC). If/when a logging surface
needs them, they can be re-introduced.

-wgb (deepseek): tokeniseSegment doesn't recursively unwrap
`bash -c '...'` / `sh -c` / `zsh -c`, so addCoAuthorToGitCommit
won't splice the trailer into a wrapped command. The background
refusal AND the post-commit note path DO catch the wrapped commit
because stripShellWrapper at the top of execute peels the wrapper
before gitCommitContext / getGitHead run — so the worst-case
("background bash -c 'git commit' bypasses the guard") doesn't
materialize. The remaining gap (no Co-authored-by trailer for
bash -c-wrapped commits) requires recursively splicing into the
inner script with proper bash single-quote re-quoting; significant
enough that it's worth its own PR. Documented as a partial-coverage
limitation.

339 → 325 tests pass after the dead-export removal; typecheck clean.

* fix(attribution): committed-blob validation, deleted-leaf canonicalisation, sudo/env shifts, dir-stack

gpt-5.5 review (issue 4389405179):

1. realpathOrSelf falls back to the non-canonical input when the
   leaf doesn't exist (deleted file). recordEdit stored the entry
   under the canonical path; lookup post-deletion misses on macOS
   where /var ↔ /private/var. Canonicalise the parent and rejoin
   the basename for missing leaves so deleted-file getFileAttribution
   still resolves the canonical key. Test updated to assert the
   lookup-after-unlink path explicitly.

2. validateOnDiskHashes read the LIVE working-tree, so a user who
   `git add`'d AI's content and then made additional unstaged edits
   would have the entry dropped on a commit whose blob still matched
   AI's hash. Replace with `validateAgainst(getContent)` that takes
   a caller-supplied reader; attachCommitAttribution now passes a
   reader that fetches the COMMITTED blob via `git show HEAD:<rel>`.
   Working-tree validation kept as `validateAgainstWorkingTree` for
   code paths without a committed ref. Returns null = no comparison
   signal (entry preserved). Tests cover all three readers
   (committed-blob via stub, working-tree, null-passthrough).

deepseek-v4-pro review #1: sanitiseAttribution defaults missing
contentHash to '' on legacy-snapshot restore. recordEdit's
divergence check would then trip on every subsequent edit and
silently reset all the AI work. Skip the divergence check when
existing.contentHash is empty — we have no baseline to compare
against, so don't drop. Test added covering legacy-snapshot
preservation through validateAgainst.

deepseek #4: validateAgainst now logs every entry drop via
debugLogger.debug so a 3am operator can see WHICH entry got
dropped and tied to which canonical key.

deepseek #8: GIT_NAMESPACE removed from GIT_ENV_SHIFTS_REPO. It
prefixes ref names within the same repo but doesn't redirect git
to a different on-disk repository, so a commit underneath it still
lands in our cwd's repo. Doc comment explains the distinction.

deepseek #9: pushd/popd treated as cwd-shifting alongside cd in
gitCommitContext / isAmendCommit / findAttributableCommitSegment.
pushd reuses cdTargetMayChangeRepo (relative-no-escape stays
in-repo); popd unconditionally flips cwdShifted because we don't
track the bash dir-stack.

deepseek #10: sudo's value-taking flag table now has a parallel
SUDO_FLAGS_SHIFT_CWD set covering -D / --chdir (Linux sudo 1.9.2+).
Any segment whose sudo wrapper sees one of those flags returns null
from tokeniseSegment — same contract as env -C / --chdir and
GIT_DIR=...

328 tests pass; typecheck clean both packages.

* fix(attribution): scope validateAgainst to committed set, SHA-pin reader, intent-aware migration

Round 1 of multi-pass audit on b3a06a7c4. Three correctness fixes:

1. validateAgainst was iterating ALL fileAttributions but the
   committed-blob reader (git show HEAD:<rel>) returns HEAD's
   pre-AI content for files NOT in the just-made commit. Result:
   pending unstaged AI work was silently wiped on every commit
   because the divergence check ran against the wrong baseline
   for unrelated files. Fix: build the committed scope first via
   matchCommittedFiles, scope the reader to that set (return null
   for everything else), validate, then RE-run matchCommittedFiles
   to pick up dropped entries. The validateAgainstWorkingTree
   wrapper had no production caller — removed it and its test.

2. The committed-blob reader used symbolic `HEAD` instead of the
   captured postHead SHA — same TOCTOU concern buildGitNotesCommand
   already addressed. A post-commit hook moving HEAD between
   capture and the reader's `git show` would silently compare
   against the wrong commit's content and trip the divergence
   check spuriously. Pin the reader to `git show <postHead>:<rel>`.

3. v3→v4 migration's invalid-string fallback used to reset to {}.
   Combined with the runtime pickBool's "absent → schema default
   true" rule, that silently re-enabled attribution for users who
   hand-edited `"gitCoAuthor": "off"` to disable. Migration now
   recognises enable-intent strings (true/yes/on/1/enabled) and
   disable-intent strings (false/no/off/0/disabled/'') and maps
   them to {commit, pr} explicitly. Unrecognised strings fall to
   {commit: false, pr: false} with a warning — same safer-by-default
   contract as runtime pickBool. Test grid covers all 11 cases.

Also tidied the FileAttribution.contentHash JSDoc to reference
the renamed `validateAgainst` (was still pointing at the dropped
`validateOnDiskHashes` name).

1085 tests pass; typecheck clean both packages.

* chore(attribution): extract pickOuterLastMatch, log unrecognised pickBool inputs

Round 2 of multi-pass audit. Two cleanups, no behaviour changes:

1. addCoAuthorToGitCommit and addAttributionToPR each carried their
   own copy of the matchRange / isInside / "pick LAST non-nested
   match" logic (~25 LOC duplicated). Extracted to module-level
   helpers `matchSpan`, `isMatchInside`, and `pickOuterLastMatch<T>`
   so a future bug fix can't apply to only one of the two
   rewriters. Behaviour identical — same algorithm, same edge cases.

2. normalizeGitCoAuthor's pickBool silently maps unrecognised
   strings to false (safer-by-default vs the old "default-to-true
   on mismatch" policy, but a user who hand-edited
   `{ commit: "maybe" }` had no signal that their setting was being
   ignored). Add a `gitCoAuthorLogger.warn` listing the accepted
   forms so a debug-mode user can see the actual coercion. Known
   disable-intent strings (false/no/off/0/empty) stay silent —
   they're explicit user intent. Also pass the field name so the
   warning identifies which sub-toggle (commit vs pr) was bad.

1101 tests pass; typecheck clean.

* fix(attribution): canonicalise BOM and CRLF before hashing

Round 3 of multi-pass audit. One real correctness fix.

Edit and WriteFile preserve the file's BOM and CRLF line-ending
choice when writing back, so the on-disk bytes can include a leading
U+FEFF and CRLFs even when AI's recordEdit input was given with LF
and no BOM. The committed-blob reader's `git show <sha>:<rel>`
returns those raw bytes verbatim, and computeContentHash hashed them
as-is — so a UTF-8 BOM file or a CRLF-line-ending file would always
have a mismatch between AI's recorded hash and the on-disk hash, and
validateAgainst would drop the entry on every commit.

Add `canonicaliseForHash`: strips a leading U+FEFF and normalises
CRLF→LF before computing the SHA-256. Both sides (recordEdit when
storing the post-write hash, and validateAgainst when comparing to
the on-disk read) flow through computeContentHash, so the
canonicalisation is symmetric. The hash is metadata used only for
divergence detection — collapsing these visual differences is the
right comparison semantics.

Three regression tests added: BOM-only, CRLF-only, and BOM+CRLF
combined. All exercise the typical case where AI's recordEdit input
is LF + no BOM but the on-disk content (post-writeTextFile) has the
file's preserved BOM/lineEnding choice.

* fix(attribution): reset accumulator when re-creating a deleted tracked file

Round 4 of multi-pass audit + Copilot finding from review 4236842362
(I missed it in the previous refresh).

recordEdit's existing prior-state check was symmetric on diverged
oldContent but ASYMMETRIC on a fresh file lifetime: when AI creates
`foo.ts` (oldContent=null), then user `rm foo.ts`, then AI
re-creates `foo.ts` (oldContent=null again), the second recordEdit
saw `existing` (from the first lifetime) and SKIPPED the divergence
check (because oldContent === null bails out of that branch). The
accumulator carried 100 chars from the deleted file plus 5 chars
from the new content = 155, vs the actual 5 on disk. Subsequent
generateNotePayload's clamp against `(adds+dels) * 40` couldn't
catch this — the diff size for a 1-line addition is 40, far above
the actual content size.

Add a fresh-file-lifetime branch: when `existing` is set AND the
caller reports `oldContent === null`, reset aiContribution and
aiCreated before counting the new contribution. The new edit is
treated as a brand-new file at the same path (which is what the
caller's null oldContent means semantically).

Test added covering the exact `AI create → delete → AI re-create`
flow. Also verified `should treat new files as ai-created` and
`should accumulate contributions across multiple edits` still pass.

* fix(attribution): treat git -C . as in-cwd, gate preHead on attributable

Round 5 of multi-pass audit. Two related correctness/efficiency
fixes around the cwd-shift parser and the preHead capture.

1. `git -C .` (and `-C ./`, `-C.`) is a no-op cwd shift but the
   "any -C → cwd-shifted" rule was treating it the same as
   `-C /tmp/other`, suppressing attribution for what's effectively
   `git commit` with an explicit current-dir marker. Add an
   `isNoopCwdTarget` helper used in both the spaced (`-C .`) and
   attached (`-C.`) branches of `parseGitInvocation`. `--git-dir`
   / `--work-tree` are left unconditional — those aren't cwd in the
   same sense.

2. preHead was being captured for ANY hasCommit, including the
   non-attributable cases (`cd /elsewhere && git commit`,
   `git -C /other commit`). The only consumer of preHead is the
   `attachCommitAttribution` call inside the `attributableInCwd`
   branch — there is intentionally NO cleanup branch for the
   non-attributable case (see the existing comment around the
   `else if (commitCtx.hasCommit)` non-branch). The execFileSync
   for `getGitHeadSync` is dead work in that path: ~10–50 ms
   blocking the event loop before the user's real command spawns.
   Gate the capture on `attributableInCwd` to match the consumer.

Tests added for the three -C dot-form variants. Full suite green:
146 in shell.test.ts, 56 in commitAttribution.test.ts.

* fix(core): preserve attribution across renamed files

* fix(attribution): preserve env-vars in tokens, exclude empty -C targets

Round 7 of multi-pass audit. Two related fixes around how
`shell-quote` handles env-var references and how the cwd-shift
detector reads them.

1. `shell-quote.parse` collapses `$NAME` references it cannot
   resolve to the empty string. The downstream cwd-shift checks
   (`cdTargetMayChangeRepo`'s `target.includes('$')` repo-shift
   detector, and the new `isNoopCwdTarget` no-op detector) were
   designed to catch env-var targets but received `''` instead of
   `$NAME` from `tokeniseSegment` and silently failed. Concretely,
   `cd $HOME && git commit` and `git -C $HOME commit` would both
   pass through as in-cwd attributable, stamping our trailer onto
   commits that land in whatever repo `$HOME`/`$REPO_ROOT`
   resolves to at runtime.

   Pass an env getter `(key) => '$' + key` to `shell-quote.parse`
   inside `tokeniseSegment` so unresolved references stay literal
   in tokens (`['cd', '$HOME']` instead of `['cd', '']`).
   `target.includes('$')` now fires correctly, and the no-op
   detector sees `$HOME` (non-`.`) and rejects it. KEY=value
   leading-env detection is unaffected (shell-quote doesn't
   interpolate inside KEY=value tokens).

2. Even with env preservation, an `''` target can still slip
   through (literal `-C ""`, escaped quotes, edge cases in
   shell-quote). Round 5's `isNoopCwdTarget` accepted `''` as a
   no-op alongside `'.'` / `'./'`, which would re-introduce the
   attribution-on-wrong-repo problem if any path produced an
   empty token. Tighten to `'.'` and `'./'` only — the only
   missed cases are literal `-C ""` (malformed, won't actually
   commit) and the rare `-C $PWD` (now also caught conservatively,
   since `$PWD` becomes literal `$PWD` and isn't `.` or `./`).

Tests added for `cd $HOME` / `cd $REPO_ROOT && git commit` and
`git -C $HOME commit` / `git -C "" commit`. Full suite green
(150 in shell.test.ts, 58 in commitAttribution.test.ts).

* fix(attribution): SHA-pin diff/rev-list phase, document aiChars heuristic

Addresses tanzhenxin's review (4240760004) — two residuals after
the prior pinning round.

1. Diff phase still races against HEAD.

   The note write itself was already pinned to the captured `postHead`
   (`git notes add -f <postHead>`), but the *content* of the note —
   `getCommittedFileInfo`'s probe + diff calls and the multi-commit
   guard's `rev-list --count` — were still going through symbolic
   `HEAD` / `HEAD~1` / `HEAD@{1}`. Several awaited subprocesses run
   between the postHead capture and these reads, so a husky / lefthook
   auto-amender, signed-commits hook, chained `git tag -m`, or
   parallel git process moving HEAD in that window would leave the
   note attached to commit A but describing commit B's contents.
   Same TOCTOU class as the prior critical, half-closed.

   Thread `postHead` (and `preHead` for amend) through
   `getCommittedFileInfo`. Probes become `rev-parse --verify
   ${postHead}~1` and `log -1 --pretty=%P ${postHead}`; diffs become
   `${postHead}~1..${postHead}` (parent case),
   `${preHead}..${postHead}` (amend — preHead is the pre-amend SHA
   captured before the user's command and is exactly what HEAD@{1}
   resolved to at parse time, with the added benefit that it can't be
   GC'd between capture and use), and `diff-tree --root <postHead>`
   (root commit). The amend branch keeps the existing reflog-vs-
   no-reflog warning, just driven off `preHead` instead of HEAD@{1}.

   Same pin applied to `countCommitsAfter` (now `${preHead}..
   ${postHead}`) and `countCommitsFromRoot` (now `${postHead}`).

   Why parent case uses `${postHead}~1` and NOT `${preHead}`: in
   `git reset HEAD~3 && git commit` chains the captured preHead
   points well above postHead's parent, and `${preHead}..${postHead}`
   would describe the reset-away commits as deletions, drastically
   over-attributing. The actual parent of the just-landed commit is
   what we want, and `${postHead}~1` is the SHA-pinned form of that.

2. `aiChars` reads as a literal char count but isn't.

   The field is emitted as a plain integer named `aiChars`; the PR
   description's example shows values like 3200 / 1500 / 4700 that
   anyone parsing the note will read as literal character counts.
   Internally it's `(addedLines + deletedLines) × 40` for text and a
   flat 1024 for binary, with the per-file AI accumulator clamped
   against that ceiling. So 1000 one-character lines and 1000
   thousand-character lines both report aiChars=40000, and a 5 MB
   image change and a 1-byte binary tweak both report 1024. Anyone
   aggregating raw aiChars for compliance reporting gets
   systematically wrong numbers.

   Add a comprehensive doc block on `FileAttributionDetail` (and
   `CommitAttributionNote`) calling out the heuristic explicitly,
   noting that `percent` / `summary.aiPercent` are the correct
   fields for aggregation since both numerator and denominator use
   the same proxy. Also expand the `APPROX_CHARS_PER_LINE` /
   `BINARY_DIFF_SIZE_FALLBACK` const docs to point at the same
   caveat. (Not renaming the fields — that'd break any downstream
   consumer already parsing the existing schema; the doc is the
   minimum-disruption call here.)

208 attribution tests pass; type-check clean.

* fix(attribution): use posix join in applyCommittedRenames for Windows compat

Windows CI failure on the two new rename tests (visible at PR #3115's
`Test (windows-latest, *)` jobs):

  AssertionError: expected undefined to be defined
  ❯ src/services/commitAttribution.test.ts:572:66 (basic move)
  AssertionError: expected 11 to be 22 (merge into existing)

Root cause: `path.join(canonicalRepoRoot, ...renamedRel.split('/'))`
calls `path.win32.join` on Windows, which forces backslash separators
regardless of input form. The test's `fs.realpathSync` mock returns
forward-slash paths (matching the macOS `/var` ↔ `/private/var`
fixture style), so `recordEdit` stores keys like
`/private/var/repo/src/old.ts`. The rename's joined target then came
out as `\\private\\var\\repo\\src\\new.ts`, the mock left it
unchanged (no `/var/` prefix to translate), and the subsequent
`fileAttributions.get(renamedAbs)` / `getFileAttribution(...)` lookups
missed the just-set entry — the rename silently dropped attribution.

The fix: build the joined path with `path.posix.join` against a
forward-slash-normalised `posixRepoRoot`, then let `realpathOrSelf`
canonicalise to the platform's storage form. This way:

  - On real Windows production: posix-joined `D:/repo/src/new.ts` is
    accepted by `fs.realpathSync` (Win32 API takes mixed slashes) and
    returned in backslash form, matching what `recordEdit` stored.
  - On real Linux/macOS production: forward-slash throughout, no-op.
  - In the symlink-aware test (any platform): forward-slash matches
    the mock-fixture storage form.

`matchCommittedFiles` already does the inverse normalisation
(`.split(path.sep).join('/')` for the relative-form check), so the
in/out paths line up either way.

Skipped adding a path.sep-mocked Linux-side regression because the
ESM module namespace doesn't allow `vi.spyOn` on path's exports.
The Windows CI job is the regression catcher; a focused-rerun
should now go green.

* docs(attribution): refresh stale HEAD~1/HEAD@{1} references in comments

The SHA-pinning round (8c3312027) replaced symbolic `HEAD~1..HEAD` /
`HEAD@{1}..HEAD` with `${postHead}~1..${postHead}` and
`${preHead}..${postHead}` in `getCommittedFileInfo` and the rev-list
helpers, but three docstrings / inline comments still described the
old shapes:

- `isAmendCommit` JSDoc said the amend switch goes from `HEAD~1..HEAD`
  to `HEAD@{1}..HEAD`. Updated to reference `${postHead}~1..${postHead}`
  and `${preHead}..${postHead}`, with the why (amended commit's parent
  is the original's parent so the standard parent diff lumps both
  commits' changes).
- `attachCommitAttribution`'s amend branch comment had the same drift;
  updated to mention `${preHead}..${postHead}` directly.
- `getCommittedFileInfo` JSDoc said it diffs "HEAD against its parent
  (HEAD~1)" and listed "--amend with no reflog" as an analysis-failure
  case. Updated to mention postHead-pinning and the preHead-driven
  amend bail (the reflog-GC dependency was dropped in the SHA-pin
  round).

The remaining `HEAD~1..HEAD` references at countCommitsAfter:1959 and
getCommittedFileInfo:2523 are intentional — they describe the old
buggy shape as contrast for why we pin now.

No code change; tests + tsc still clean.

* fix(attribution): catch attached-value forms of env/sudo cwd-shift flags

Round 13 audit found a real bug: `sudo --chdir=/tmp git commit`,
`env -C/tmp git commit`, `env --chdir=/tmp git commit`, and
`sudo -D/tmp git commit` were all silently slipping through the
cwd-shift detector and getting our `Co-authored-by` trailer stamped
onto commits that landed in a different repo.

Root cause: `shell-quote` tokenises both the long attached form
(`--chdir=/tmp`) and the short attached form (`-C/tmp`) as a single
argv entry. The previous SHIFT_CWD detector did set-membership only
against the bare flag (`{'-C', '--chdir'}` for env;
`{'-D', '--chdir'}` for sudo), so the attached-form tokens never
matched and `tokeniseSegment` returned a normally-attributable
`['git', 'commit', ...]` segment.

Fix: introduce `isShiftCwdFlag(flag, set)` that catches:
  - bare set-membership (existing behavior),
  - long attached: `--name=...` when `--name` is in the set,
  - short attached: `-Xanything` when `-X` is in the set and the
    token is longer than the flag itself.

The flag does NOT need to consume an extra value token in the
attached-form case (the value is already embedded), so the existing
TAKES_VALUE bookkeeping is unaffected — we just bail with `null`
from `tokeniseSegment` before reaching the value-skip step.

Tests added: `env --chdir=`, `env -C/...` (attached), `sudo --chdir=`,
`sudo -D/...` (attached) — each is asserted NOT to add a co-author
trailer. 154 shell tests pass; type-check + lint clean.

* test(attribution): cover attached-form git -C/--git-dir/--work-tree

Adds three regression cases to the existing "git -C <path>" suppression
test: the short attached form `-C/path` (single shell-quote token)
and the long attached forms `--git-dir=/path` / `--work-tree=/path`.
parseGitInvocation already had the prefix checks at lines 416/425, but
no test exercised them — paired with the b89b65533 sudo/env attached-
form fix this round closes the family of "shell-quote single-token
flag with embedded value" cases that the bare set-membership checks
would otherwise miss.

157 shell tests pass; type-check clean.

* docs(attribution): document why backtick body doesn't bail like $(

The addCoAuthorToGitCommit body capture has a known truncation case
when an inner unescaped `"` appears inside the captured body — handled
for `$(...)` command substitution with an explicit bailout, but not
for backtick command substitution. The trade-off was unspoken; spell
it out so a future reviewer doesn't read the asymmetry as an
oversight.

Bare-backtick bodies (`\`func()\`` markdown-style) are common in
commit messages, have no inner `"`, and the regex captures them
correctly. Pathological backtick-with-inner-quote bodies (`\`cmd
"with" quotes\``) are a near-zero-traffic case where bash itself
already interprets the backticks as command substitution, so the
user has likely already broken their own command before our rewrite
runs. Bailing on any backtick would lose attribution for the common
case to defend against the rare one.

Also drops a stray blank line in commitAttribution.test.ts left over
from an earlier regression-test attempt.

* fix(attribution): scope trailer rewrite to before unquoted shell comment

Round 13 follow-on. Both `addCoAuthorToGitCommit` and
`addAttributionToPR` ran their `-m` / `--body` regex against the full
segment string, including any trailing shell comment. For a command
like `git commit -m "real" # -m "fake"` (a human-authored script
might leave a comment-out flag in place), `lastMatchOf` would pick
the comment's `-m "fake"`, splice the `Co-authored-by:` trailer in
there, and bash would silently discard the entire segment as a
comment — leaving the actual commit unattributed. Same shape for
`gh pr create --body "real" # --body "fake"`.

Fix: introduce `findUnquotedCommentStart(s)` — a bash-aware position
scanner that tracks single/double-quote state and treats `#` as a
comment marker only when it begins a word (start of input or
preceded by whitespace), not when it appears inside a quoted region
or mid-token like `foo#bar`. Both rewriters slice the segment to
`[0, commentStart)` before running their regex, so the trailer can
only land in the live (pre-comment) part.

Tests added:
  - `git commit -m "real" # -m "fake"` — trailer lands in `"real"`
    body BEFORE the `#`, comment's `-m "fake"` is left untouched.
  - `git commit -m "fix #123 add feature"` — `#` inside the quoted
    body is correctly NOT treated as a comment; the `#123` stays
    inside the body and the trailer is appended.

159 shell tests pass; type-check clean.

* fix(attribution): warn on gh pr create flows that can't be rewritten + cover legacy gitCoAuthor migration end-to-end

Two residuals from this morning's review pass.

1. ANm7O — `addAttributionToPR` silently skipped for `--body-file`,
   `--fill`, and bare `gh pr create` (editor) flows.

   The rewriter only knows how to splice into an inline `--body`/`-b`
   argv entry. For a `gh pr create` that uses `--body-file path`,
   `--fill` (uses commit messages), or no body flag at all (editor
   prompt), there's no inline body to splice into and the function
   returned the unmodified command. Users with `gitCoAuthor.pr`
   enabled would see PRs created without the attribution line and
   have no signal as to why.

   Add a debugLogger.warn at the no-match path naming the unsupported
   flows and pointing the user at the inline form. Don't try to
   handle `--body-file` automatically — that would mean mutating the
   user's file on disk, which is well outside what an unprompted
   command rewriter should do; `--fill` and editor flows have no body
   in argv at all and can't be rewritten without re-architecting.

   Tests added for `--body-file <path>`, `--fill`, and bare
   `gh pr create` — each is asserted to leave the command unchanged
   (no `Generated with Qwen Code` line spliced in).

2. ANm7L — settings-migration integration suite didn't cover the
   exact V3 legacy shape this PR introduces.

   `v3-to-v4.test.ts` already pins the migration body, but the end-
   to-end CLI load → migrate → write path could regress without the
   integration suite noticing. The existing v3LegacyDisableSettings
   fixture has no `general.gitCoAuthor` field, so the V3→V4 step
   technically fires but doesn't exercise the new boolean-expansion
   logic.

   Add a `v3GitCoAuthorBooleanSettings` fixture and a paired test
   case that writes `general: { gitCoAuthor: false }` at $version 3,
   runs the same `mcp list` CLI invocation, and asserts the saved
   file has $version 4 plus `general.gitCoAuthor` exactly
   `{ commit: false, pr: false }` — with sibling general.* keys and
   unrelated top-level sections preserved.

162 shell tests pass; type-check + lint clean.
2026-05-08 09:55:58 +08:00
jinye
df90da6f03
feat(telemetry): add sensitive span attribute opt-in (#3893)
* feat(telemetry): add sensitive span attribute opt-in

Add a telemetry setting and environment override for including sensitive attributes in spans created by the log-to-span bridge. Keep the default filtering behavior for prompt, function_args, and response_text unless explicitly enabled.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(telemetry): clarify span bridge options

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* feat(telemetry): populate api response text

Populate response_text on API response telemetry events for non-internal prompts so opted-in bridge spans can include model response bodies.

Exclude thought text from the recorded response text and keep internal prompt responses omitted.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs(telemetry): clarify sensitive span attribute scope

Clarify that the sensitive span attribute setting only controls log-to-span bridge spans, while response text may still reach other telemetry sinks from API response events.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(telemetry): cap recorded response text

Limit response_text captured for API response telemetry to a bounded length and mark truncated values to avoid oversized OTLP attributes.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-05-08 00:36:08 +08:00
ChiGao
7f0c9791b7
feat(cli): expand TUI markdown rendering (#3680)
Some checks are pending
Qwen Code CI / Lint (push) Waiting to run
Qwen Code CI / Test (push) Blocked by required conditions
Qwen Code CI / Test-1 (push) Blocked by required conditions
Qwen Code CI / Test-2 (push) Blocked by required conditions
Qwen Code CI / Test-3 (push) Blocked by required conditions
Qwen Code CI / Test-4 (push) Blocked by required conditions
Qwen Code CI / Test-5 (push) Blocked by required conditions
Qwen Code CI / Test-6 (push) Blocked by required conditions
Qwen Code CI / Test-7 (push) Blocked by required conditions
Qwen Code CI / Test-8 (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
SDK Python / SDK Python (3.10) (push) Waiting to run
SDK Python / SDK Python (3.11) (push) Waiting to run
SDK Python / SDK Python (3.12) (push) Waiting to run
* feat(cli): expand markdown rendering in tui

* fix(cli): render finalized mermaid images synchronously

* fix(cli): harden markdown rendering paths

* fix(cli): preserve mermaid source fallbacks

* test(cli): make mermaid image renderer mock cross-platform

* fix(cli): run windows mmdc shims through shell

* fix(cli): address markdown rendering review comments

* fix(cli): validate mermaid render timeout

* feat(cli): expose rendered markdown source blocks

* fix(cli): align mermaid source copy controls

* fix(cli): make markdown render toggle visible

* fix(cli): keep markdown render toggle quiet

* fix(cli): generalize markdown render mode

* fix(cli): broaden render mode and copy latex blocks

* fix(cli): align rendered copy source indices

* fix(cli): support copying inline latex expressions

* feat(cli): document markdown render controls

* test(cli): cover markdown render controls

* fix(cli): tighten markdown render fallbacks

* fix(cli): bound mermaid renderer cache

* fix(cli): address markdown render review feedback

* fix(cli): address markdown render review comments

* test(cli): strengthen render mode shortcut coverage

* fix(cli): address mermaid image review comments

* fix(cli): stabilize renderer output truncation

* test(cli): flush fake renderer stderr before exit

* fix(cli): address markdown renderer review feedback

* docs(cli): clarify mermaid image limits

* fix(cli): refresh mermaid images on height resize

* fix(cli): address markdown render review

* chore: revert unrelated review formatting churn

* fix(cli): avoid mermaid regex codeql alert

* fix(cli): silence mermaid operator codeql alert

* fix(cli): render inline math in markdown tables

* fix(cli): harden markdown visual renderers

* fix(cli): strip c1 controls from mermaid previews

---------

Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>
2026-05-07 16:24:13 +08:00
jinye
b1ec8d64c7
fix(cli): warn on ignored provider generation config (#3883)
* fix(cli): warn on ignored provider generation config

Warn when a selected provider model has top-level model.generationConfig fields that will not apply because the provider entry is sealed. Document the required local-model placement for contextWindowSize and related generation config fields.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): address provider config warning review

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): narrow provider config warning fields

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-05-07 14:46:23 +08:00
Shaojin Wen
93139d0eb0
docs(cli): document new banner customization settings (#3885)
Add ui.customBannerTitle, ui.customBannerSubtitle, and ui.customAsciiArt
to the user-facing settings table. Also reword ui.hideBanner to note
that it covers both the logo column and the info panel and that Tips
render independently.

These settings landed in #3710 but only ui.hideBanner was listed in
the table, so users had no way to discover the other three short of
reading the schema or the design doc.
2026-05-07 13:42:22 +08:00
Shaojin Wen
efb7351d58
feat(core): support reasoning effort 'max' tier (DeepSeek extension) (#3800)
* feat(core): support reasoning effort 'max' tier (DeepSeek extension)

DeepSeek's chat-completions endpoint added an extra-strong `max` tier
to `reasoning_effort` (per
https://api-docs.deepseek.com/zh-cn/api/create-chat-completion ; valid
values are now `high` and `max`, with `low`/`medium` mapping to `high`
for backward compat). Plumb it end-to-end:

- `ContentGeneratorConfig.reasoning.effort` union now includes 'max'.
- DeepSeek OpenAI-compat provider: translate the standard nested
  `reasoning: { effort }` shape into DeepSeek's flat `reasoning_effort`
  body parameter so user-configured effort actually takes effect (the
  nested shape was previously sent verbatim and silently ignored,
  defaulting to `high`). low/medium → high mirrors the documented
  server-side behavior so dashboards / logs match wire reality.
  An explicit top-level `reasoning_effort` (via samplingParams or
  extra_body) wins over the nested form.
- Anthropic converter: pass 'max' through to `output_config.effort`
  unchanged and bump the `thinking.budget_tokens` budget for the new
  tier (low 16k / medium 32k / high 64k / max 128k).
- Gemini converter: clamp 'max' to HIGH since Gemini has no higher
  thinking level. Without this, 'max' would silently fall through to
  THINKING_LEVEL_UNSPECIFIED.

Live verification against api.deepseek.com:
- `reasoning_effort: high` → 200
- `reasoning_effort: max`  → 200 (the new tier)
- `reasoning_effort: bogus`→ 400 with valid-set list confirming
  [high, low, medium, max, xhigh]

108 anthropic/openai-deepseek/gemini tests pass; full core suite
(6601 tests) green; lint + typecheck clean.

* fix(core): map xhigh→max + clamp max on non-DeepSeek anthropic + docs

Address PR review (copilot × 2) and add missing user docs:

1. (J698) `translateReasoningEffort` claimed in the PR description that
   it surfaces the DeepSeek backward-compat mapping client-side, but
   only handled `low`/`medium` → `high`. Add `xhigh` → `max` to match
   the doc and stay symmetric with the low/medium branch.

2. (J6-A) `output_config.effort: 'max'` would have been emitted on
   any anthropic-protocol provider whenever a user configured it, even
   when the baseURL points at real `api.anthropic.com` (which only
   accepts low/medium/high and would 400). Reuse the existing
   `isDeepSeekAnthropicProvider` detector to clamp `'max'` → `'high'`
   on non-DeepSeek anthropic backends, with a debugLogger.warn so the
   downgrade is visible. DeepSeek anthropic-compatible endpoints still
   pass through unchanged.

3. New docs:
   - `docs/users/configuration/model-providers.md`: a "Reasoning /
     thinking configuration" section under generationConfig — single
     example targeting DeepSeek + a per-provider behavior table
     (OpenAI/DeepSeek flat reasoning_effort, OpenAI passthrough for
     other servers, real Anthropic clamp, Anthropic-compatible
     DeepSeek passthrough, Gemini thinkingLevel mapping).
   - `docs/users/configuration/settings.md`: extend the
     `model.generationConfig` description to mention `reasoning`
     (the field was undocumented before this PR even though it
     already existed as a typed field) and link to the new section.

96 anthropic + deepseek tests pass; lint + typecheck clean.

* refactor(core): single-source effort normalization for anthropic + doc fix

Address PR review round 2 (copilot × 2):

1. (J8aG) The `contentGenerator.ts` comment claimed passing
   `reasoning.effort: 'max'` to real Anthropic was "up to the user",
   but commit b5b05ae actively clamps 'max' → 'high' (with a debug
   log) on non-DeepSeek anthropic backends. Update the comment to
   describe current runtime behavior.

2. (J8aL) The clamp ran inside `buildOutputConfig()` only — the effort
   label was downgraded but `buildThinkingConfig()` still used the
   raw user value to size the budget, so a non-DeepSeek anthropic
   request could end up with `output_config.effort: 'high'` paired
   with a 'max'-sized 128K thinking budget. Inconsistent label vs.
   budget on the wire.

   Refactor: hoist the normalization into a single
   `resolveEffectiveEffort()` helper that runs once per request in
   `buildRequest()`. Both `buildThinkingConfig` and `buildOutputConfig`
   now consume the same clamped value, so the budget ladder and the
   effort label stay aligned. The debug log fires once per request.

Add a regression test asserting that on a non-DeepSeek anthropic
provider with `effort: 'max'` configured, the wire request carries
both `output_config.effort: 'high'` AND `thinking.budget_tokens:
64_000` (the 'high' tier), not the 128K 'max' budget.

96 tests pass; lint + typecheck clean.

* fix(core): tighten 'max' clamp + warn-once + strip reasoning_effort on side queries

Address PR review round 3 (copilot × 3):

1. (J-2v) When request.config.thinkingConfig.includeThoughts is false,
   pipeline.buildRequest's post-processing only deleted the nested
   `reasoning` key. The DeepSeek provider's translateReasoningEffort
   may have already flattened an extra_body-injected reasoning into
   top-level `reasoning_effort` by that point, so a side query (e.g.
   suggestionGenerator) could still ship reasoning_effort on the wire.
   Extend the post-processing to also delete `reasoning_effort`.

2. (J-2z) The warn for clamping 'max' on non-DeepSeek anthropic ran on
   every request needing the downgrade — the docstring claimed "first
   time only" but the implementation didn't latch. Add a private
   `effortClampWarned` boolean on the generator so the warning fires
   once per generator lifetime.

3. (J-23) `resolveEffectiveEffort` used the broad
   `isDeepSeekAnthropicProvider` detector for the clamp decision, but
   that helper falls back to model-name matching to cover sglang/vllm
   self-hosted DeepSeek deployments. A model configured as e.g.
   "deepseek-distill" but routed to real api.anthropic.com would
   bypass the clamp and trigger HTTP 400. Split the detector: keep
   `isDeepSeekAnthropicProvider` (broad) for the thinking-block
   injection workaround where false-positives are harmless, and add
   `isDeepSeekAnthropicHostname` (hostname-only) for decisions where
   a model-name false-positive would route DeepSeek-only behavior to
   a stricter backend. The clamp now uses the hostname-only check.

New regression test: a config with model name containing "deepseek"
but baseURL pointing at api.anthropic.com still clamps `'max'` to
`'high'`. Existing "passes max through" test updated to set a
DeepSeek baseURL since model name alone no longer suffices for the
clamp bypass.

385 tests pass; lint + typecheck clean.

* docs(core): correct pipeline timing comment + samplingParams caveat

Address PR review round 4 (copilot × 3) — three documentation accuracy
fixes, no behavior change:

1. (KBcw) The post-processing comment in pipeline.ts misdescribed the
   call order ("after this branch already ran during the same
   buildRequest pass") — provider.buildRequest actually runs BEFORE
   the includeThoughts=false post-processing in the same pass.
   Reword to match the actual order: provider hook flattens nested
   reasoning to reasoning_effort first, this cleanup runs after and
   strips both shapes.

2. (KBdC, KBdE) The "Reasoning / thinking configuration" section in
   model-providers.md and the model.generationConfig description in
   settings.md both implied `reasoning` is honored on every provider.
   For OpenAI-compatible providers, when `generationConfig.samplingParams`
   is set, `ContentGenerationPipeline.buildGenerateContentConfig()`
   ships samplingParams verbatim and skips the separate `reasoning`
   injection entirely. Configs like
   `{ samplingParams: { temperature: 0.5 }, reasoning: { effort: 'max' } }`
   would silently drop the reasoning field on OpenAI/DeepSeek
   requests.

   Add an explicit "Interaction with samplingParams" warning section
   in model-providers.md and a parenthetical note in settings.md
   directing users to put `reasoning_effort` inside `samplingParams`
   (or `extra_body`) when both are configured.

385 tests pass; lint + typecheck clean.

* docs(core): clarify explicit budget_tokens bypasses 'max' effort clamp

When user sets `{ effort: 'max', budget_tokens: N }` on a non-DeepSeek
anthropic backend, the effort label gets clamped to 'high' (otherwise
the server 400s on the unknown enum) but the explicit budget_tokens is
preserved verbatim. The wire-shape mismatch is intentional, not a bug:
the clamp only protects the enum field, while budget is a free integer
the server accepts within the context window, so an explicit override
stays explicit. Document the contract on the early-return and add a
regression test that locks it in.

* docs(deepseek): fix comments to match flatten + reasoning-strip behavior

Two doc-only nits called out in review:

1. `buildRequest` JSDoc said non-text parts are "rejected", but
   `flattenContentParts` actually substitutes a textual placeholder
   (`[Unsupported content type: <type>]`) so the request still goes
   through with a breadcrumb. Reword the JSDoc accordingly.

2. `translateReasoningEffort`'s strip comment claimed it strips the
   nested form to avoid shipping both shapes, but it only drops the
   duplicated `effort` key when other keys (e.g. `budget_tokens`) are
   present. Reword to describe the actual selective behavior and why
   keeping orthogonal keys is intentional.

Behavior unchanged.

* fix(deepseek): gate reasoning_effort translation on actual DeepSeek hostname

The provider class is selected via the broader `isDeepSeekProvider`
check, which falls back to model-name matching to cover self-hosted
DeepSeek deployments (sglang/vllm/ollama, see #3613). That fallback is
the right call for content-part flattening — it's a model-format
constraint baked into the model itself, not the API surface.

But the same broad detection was also gating
`translateReasoningEffort`, which rewrites the standard
`reasoning: { effort }` config into DeepSeek's flat `reasoning_effort`
body parameter. That's a wire-shape decision, not a model-format one:
strict OpenAI-compat backends in self-hosted setups may not accept the
DeepSeek extension and would have happily handled the original shape.

Split the two decisions: keep `isDeepSeekProvider` (broad) for
flattening, add a hostname-only `isDeepSeekHostname` and gate the body
rewrite on it. Self-hosted DeepSeek users who actually want the
translation can either use a baseUrl containing api.deepseek.com or
inject `reasoning_effort` directly via `samplingParams`/`extra_body`.

Regression tests:
  - self-hosted (sglang) with deepseek-named model + nested
    `reasoning.effort` → flattening still runs, body shape preserved
  - `isDeepSeekHostname` matches api.deepseek.com but not custom hosts

* fix(deepseek): use URL parsing in isDeepSeekHostname; fix log-level docs

CodeQL flagged a high-severity URL substring sanitization issue on the
new `isDeepSeekHostname` helper. The naive
`baseUrl.includes('api.deepseek.com')` check would false-positive on
hostile hosts like `https://api.deepseek.com.evil.com/v1` and
incorrectly inject the DeepSeek-only `reasoning_effort` body parameter
into requests routed elsewhere. Switch to `new URL(...).hostname` with
exact match against `api.deepseek.com` (and `.api.deepseek.com`
subdomains), mirroring `isDeepSeekAnthropicHostname` on the Anthropic
side. Invalid URLs treated as non-DeepSeek.

`isDeepSeekProvider` already routes through `isDeepSeekHostname`, so
the hardening applies to both decision paths.

Regression tests cover:
  - subdomain match (us.api.deepseek.com)
  - hostile substrings (api.deepseek.com.evil.com,
    evil.com/api.deepseek.com/v1, api.deepseek.comevil.com,
    api-deepseek-com.example.com)
  - invalid / empty baseUrl

Also fix two doc-level mismatches: the `'max'` clamp on Anthropic logs
via `debugLogger.warn` (warning level, once per generator), not "with
a debug log". Update both `ContentGeneratorConfig.reasoning` JSDoc and
the per-provider behavior table in model-providers.md.

* feat(deepseek): emit thinking:disabled signal when reasoning is off

DeepSeek V4+ defaults `thinking.type` to `'enabled'`, so just stripping
`reasoning_effort` from the request leaves the server happily thinking
on side queries — paying full thinking latency/cost without an effort
configured. Per yiliang114's review, emit the explicit
`thinking: { type: 'disabled' }` field on the wire whenever reasoning
is disabled.

Triggered when either:
  - `request.config.thinkingConfig.includeThoughts === false` (forked
    queries, e.g. suggestion generation)
  - `contentGeneratorConfig.reasoning === false` (config-level opt-out)

The previous post-processing block only fired on the per-request opt-out
path, so the config-level case was already leaking. Unify both under a
single `reasoningDisabled` predicate that runs the same strip + signal
logic.

Hostname-gated to `api.deepseek.com` (and subdomains): self-hosted
DeepSeek behind sglang/vllm/ollama, or older DeepSeek versions, may
not accept the V4 thinking parameter — pushing it there could trip an
unknown-key 400. Mirrors the round-7 decision to gate
`reasoning_effort` translation on hostname.

Regression tests cover all four matrix points:
  - DeepSeek hostname + includeThoughts false → emits disabled
  - DeepSeek hostname + reasoning false → emits disabled
  - non-DeepSeek hostname + includeThoughts false → does not emit
  - self-hosted DeepSeek (model-name fallback only) → does not emit

Docs: extend the `reasoning: false` section with the new behavior and
the self-hosted/non-DeepSeek caveat.

* refactor(deepseek): expose isDeepSeek* as free functions; clarify docs

Two doc/coupling nits from review:

1. The pipeline post-processing block was importing the concrete
   `DeepSeekOpenAICompatibleProvider` class just to reach
   `isDeepSeekHostname`. That couples the generic OpenAI pipeline to a
   specific provider implementation. Promote the helper (and its broad
   `isDeepSeekProvider` sibling) to free `export function`s in
   `provider/deepseek.ts` and import them by name. The class keeps thin
   static delegates for backward compat with existing callers and tests.

2. The per-provider behavior table on `model-providers.md` said
   `'low'/'medium' → 'high'` and `'xhigh' → 'max'` "client-side", but
   that normalization only fires inside `translateReasoningEffort`,
   which runs on the nested `reasoning.effort` config path. Explicit
   top-level overrides via `samplingParams.reasoning_effort` or
   `extra_body.reasoning_effort` skip the rewrite and ship verbatim.
   Reword the row to reflect that.

Behavior unchanged.
2026-05-04 22:42:23 +08:00
Rayan Salhab
0b7a569ac7
fix(cli): honor proxy setting (#3753)
Some checks failed
Qwen Code CI / Lint (push) Waiting to run
Qwen Code CI / Test (push) Blocked by required conditions
Qwen Code CI / Test-1 (push) Blocked by required conditions
Qwen Code CI / Test-2 (push) Blocked by required conditions
Qwen Code CI / Test-3 (push) Blocked by required conditions
Qwen Code CI / Test-4 (push) Blocked by required conditions
Qwen Code CI / Test-5 (push) Blocked by required conditions
Qwen Code CI / Test-6 (push) Blocked by required conditions
Qwen Code CI / Test-7 (push) Blocked by required conditions
Qwen Code CI / Test-8 (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
SDK Python / SDK Python (3.10) (push) Has been cancelled
SDK Python / SDK Python (3.11) (push) Has been cancelled
SDK Python / SDK Python (3.12) (push) Has been cancelled
* fix(cli): honor proxy setting

* fix(cli): apply settings proxy to channel start

* test(cli): cover channel start settings proxy

---------

Co-authored-by: cyphercodes <cyphercodes@users.noreply.github.com>
2026-04-30 18:24:59 +08:00
Bramha.dev
414b3304cd
fix(core): split tool-result media into follow-up user message for strict OpenAI compat (#3617)
Fixes #3616.

Adds opt-in `splitToolMedia` flag (default false). When enabled, media parts (image / audio / video / file) returned by MCP tool calls are split into a follow-up `role: "user"` message instead of being embedded in the `role: "tool"` message. Required for strict OpenAI-compatible servers (e.g., LM Studio) that reject non-text content on tool messages with HTTP 400 "Invalid 'messages' in payload".

Media from parallel tool responses is accumulated and emitted as a single follow-up user message after all tool messages, preserving OpenAI's contiguity requirement for tool responses.

Default behavior is unchanged for permissive providers.
2026-04-27 23:01:02 +08:00
jinye
f0e8601982
fix(cli): add API Key option to qwen auth interactive menu (#3624)
* fix(cli): add "API Key" option to `qwen auth` interactive menu

The `qwen auth` CLI command only showed 2 options (Coding Plan, Qwen OAuth),
while the interactive `/auth` dialog showed 3 (Coding Plan, API Key, Qwen OAuth).
Users following the README instructions to configure OpenRouter/Fireworks via
`qwen auth` had no API Key entry point.

- Add "API Key" option to the `runInteractiveAuth` menu with two sub-paths:
  "Alibaba Cloud ModelStudio Standard API Key" (guided flow) and
  "Custom API Key" (prints docs link)
- Add `qwen auth api-key` yargs subcommand for direct access
- Extract `createMinimalArgv` / `loadAuthConfig` helpers to eliminate duplicated
  CliArgs boilerplate
- Extract `promptForInput` to share raw-mode stdin logic between `promptForKey`
  and `promptForModelIds`
- Improve `showAuthStatus` to distinguish Coding Plan, Standard API Key, and
  generic OpenAI-compatible configurations
- Align menu labels and descriptions with the interactive `/auth` dialog

Closes #3413

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs: add `qwen auth api-key` to auth subcommand tables

Update documentation to reflect the new `qwen auth api-key` subcommand:
- auth.md: add to subcommands table, examples, and interactive menu display
- commands.md: add to CLI Auth Subcommands table
- quickstart.md: add to quick-reference command table

* fix(cli): restore incomplete Coding Plan warning in showAuthStatus

When selectedType is USE_OPENAI and Coding Plan metadata exists but
the API key is missing, show the incomplete warning instead of falling
through to the generic "OpenAI-compatible" status.

* refactor(cli): use endpoint constants in region selector and fix status formatting

- Use ALIBABA_STANDARD_API_KEY_ENDPOINTS constants for region
  descriptions instead of hardcoded URLs
- Restore trailing newline in showAuthStatus "no auth" command list
  for consistent spacing

* fix(cli): determine active auth method from model config in showAuthStatus

Previously showAuthStatus checked which env keys exist to determine
the auth method, causing false reports when users switch providers
(e.g., Coding Plan key still present after switching to Standard API Key).

Now it inspects the active model's provider config (baseUrl/envKey) to
determine the actual method, and validates the corresponding key exists:
- Coding Plan: check via isCodingPlanConfig + CODING_PLAN_ENV_KEY
- Standard API Key: check via DASHSCOPE_STANDARD_API_KEY_ENV_KEY + endpoints
- Generic OpenAI-compatible: check if the model's envKey is set

Also clear stale Coding Plan metadata (codingPlan.region/version and
process.env) when switching to Standard API Key.

* fix(cli): add legacy fallback in showAuthStatus and clear persisted Coding Plan env

- When no active model config is found (legacy setups without
  modelProviders), fall back to env key / metadata checks for
  Coding Plan status detection. Fixes CI test failures.
- When activeConfig exists but has no envKey, report incomplete
  status instead of false positive "Configured".
- Clear persisted env.BAILIAN_CODING_PLAN_API_KEY from settings
  when switching to Standard API Key, not just process.env.

* fix(cli): also remove Coding Plan model entries when switching to Standard API Key

When switching to Standard API Key, filter out existing Coding Plan
model entries from modelProviders.openai in addition to old Standard
entries. Previously these were preserved but their credential source
(BAILIAN_CODING_PLAN_API_KEY) was cleared, leaving broken model
entries visible in /model.

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-04-27 22:01:47 +08:00
Shaojin Wen
f420742831
feat(cli,core): LLM-generated summary labels for tool-call batches (#3538)
Some checks are pending
Qwen Code CI / Lint (push) Waiting to run
Qwen Code CI / Test (push) Blocked by required conditions
Qwen Code CI / Test-1 (push) Blocked by required conditions
Qwen Code CI / Test-2 (push) Blocked by required conditions
Qwen Code CI / Test-3 (push) Blocked by required conditions
Qwen Code CI / Test-4 (push) Blocked by required conditions
Qwen Code CI / Test-5 (push) Blocked by required conditions
Qwen Code CI / Test-6 (push) Blocked by required conditions
Qwen Code CI / Test-7 (push) Blocked by required conditions
Qwen Code CI / Test-8 (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* feat(cli,core): generate tool-use summaries for compact mode

After each tool batch completes, fire a parallel fast-model call to
generate a short git-commit-subject-style label summarizing what the
batch accomplished (e.g. "Read txt files", "Searched in auth/"). In
compact mode the label replaces the generic "Tool × N" header so N
parallel tool calls collapse to a single semantic row.

The fast-model call (~1s) runs fire-and-forget, overlapped with the
next turn's API stream, so there is no perceived latency. Missing
fast model, aborted turns, and model failures all degrade silently to
the existing rendering.

The summary is also emitted as a `tool_use_summary` history entry
with `precedingToolUseIds`, keeping the shape compatible with SDK
clients that want to render collapsed tool views on their own.

Gated by `experimental.emitToolUseSummaries` (default on). Can be
overridden per-session with `QWEN_CODE_EMIT_TOOL_USE_SUMMARIES=0|1`.

The system prompt and truncation rules (300 chars per tool field,
200 chars of trailing assistant text as intent prefix) match the
existing behavior seen in other tools that emit the same message
type, so SDK consumers see a consistent shape across clients.

* fix(core): bound cleanSummary quote-strip regex to avoid ReDoS

CodeQL js/polynomial-redos flagged the /^["'`]+|["'`]+$/g pattern in
cleanSummary because its input comes from an LLM (treated as
uncontrolled). The original regex is anchored and linear in practice,
but tightening the quantifier to {1,10} both satisfies the static
check and caps engine work on pathological model output with a long
run of quotes. Ten opening/closing quotes is well past anything a real
label would produce.

* fix(cli): render tool_use_summary inline so full mode also shows the label

The summary was only visible in compact mode because the full-mode
ToolGroupMessage ignored the compactLabel prop. Compact mode got away
with this because mergeCompactToolGroups triggers refreshStatic(),
which re-renders the merged tool_group with its newly-looked-up
label. Full mode has no such refresh path, so when the fast-model
call resolves *after* the tool_group has been committed to the
append-only <Static>, there is no way to retroactively decorate it.

Switch to rendering `tool_use_summary` as its own inline history item
(a single dim `● <label>` line). New items append cleanly to <Static>,
so the summary flows in naturally once the fast-model call resolves.
Compact mode still replaces the merged tool_group header with the
label and hides the standalone summary line via the `compactMode`
guard.

With this, the feature works under the default `ui.compactMode: false`
— not just the opt-in compact view.

* docs: tool-use-summaries feature guide, settings entry, and design doc

Three new docs matching the existing fast-model feature docs layout:

- docs/users/features/tool-use-summaries.md — user-facing guide
  covering full + compact rendering, configuration (settings + env),
  failure modes, cost, and cross-links to followup-suggestions.

- docs/users/configuration/settings.md — register the new
  experimental.emitToolUseSummaries setting next to the other
  fast-model-driven UI settings.

- docs/design/tool-use-summary/tool-use-summary-design.md — deep dive
  matching the compact-mode-design.md competitive-analysis style.
  Documents the Claude Code port (prompt, truncation, timing, gate),
  the deviations (settings layer, default on, cleanSummary, dual
  render paths), and the Ink <Static> append-only rationale that
  drove the inline full-mode render vs header-replacement split.

* docs: add Recommended pairing section to tool-use-summaries

Full-mode rendering of the summary works, but for small same-type
batches (Read × 3 and similar) the label visibly restates what the
tool lines already show. Pairing with ui.compactMode: true folds
the whole batch into a single labeled row, which is the cleanest
transcript shape once the label is available.

Adds a dedicated section showing the paired settings.json snippet
and explicitly calling out when each mode wins (and when to turn
the feature off instead).

* fix: address review feedback on tool-use summary generation

Addresses multiple issues from @chiga0's review:

Blocking — compact-mode label invisible for single-batch turns.
mergeCompactToolGroups's adjacency-only gating left a trailing
tool_use_summary in the merged result whenever there was no second
batch to merge across. That pushed mergedHistory.length lock-step
with history.length and MainContent's refreshStatic heuristic
(currMLen <= prevMLen) never fired, so Ink's append-only <Static>
never repainted the tool_group with its newly-looked-up label.
Drop tool_use_summary items unconditionally now; gemini_thought
still survives to avoid unnecessary repaints. New tests cover
the single-batch case and the summary-before-user-message case.

Blocking — stale summary appears after Ctrl+C on the next turn.
summarySignal captured the CURRENT turn's AbortController, but the
summary resolves during the NEXT turn's streaming window. The next
turn's submitQuery allocates a fresh controller, so the captured
signal was never aborted — Ctrl+C during the new turn used to let
the previous turn's summary land in the transcript seconds later.
Fix: dedicated per-batch AbortController tracked in a ref set,
aborted eagerly from cancelOngoingRequest; resolve-time check reads
the live abort state and turnCancelledRef.

High — summarizer input pollution.
geminiTools contained error/cancelled tools; retry-loop warnings
and "Cancelled by user" strings were feeding the fast model.
cleanSummary can only reject error-shaped output, not prevent the
model from hallucinating a plausible label from bad input (the PR's
own tmux screenshot showed "Read txt files · 5 tools" where 4 of
the 5 were prior-retry failures). Filter to status === 'success'
before building the prompt; skip the call entirely if nothing's
left.

High — unstable label on merged groups.
getCompactLabel iterated all callIds and returned the first hit,
so asynchronous resolution order made the header visibly flip
from SB to SA when batch A resolved after batch B. Lock onto
item.tools[0].callId to keep stable "leading batch governs"
semantics.

High — force-expanded groups in compact mode had no label at all.
Compact mode routes non-force-expand groups through
CompactToolGroupDisplay (consumes compactLabel) and force-expand
groups through the full ToolGroupMessage (ignores compactLabel);
the standalone ● line was gated on !compactMode, creating a dead
zone — exactly the diagnostically valuable case. MainContent now
computes absorbedCallIds (which groups actually consume the
header replacement) and passes summaryAbsorbed to
HistoryItemDisplay; force-expand groups in compact mode get the
standalone line as the label's only path to the screen.

Medium — cleanSummary robustness.
Extend quote-strip to Unicode curly + CJK corner brackets; strip
markdown emphasis (**bold**, _italic_); broaden refusal-prefix
rejection to curly-apostrophe "I can't", Chinese "我无法 / 我不能 /
抱歉 / 无法", and "Failed to / Sorry, / Request failed". 7 new
cleanSummary tests cover the added cases.

Low — concurrent-rendering safety.
Move historyRef.current = history from render phase into
useLayoutEffect so bailed renders can't leave a dropped value.

Low — CompactToolGroupDisplay readability.
Extract renderSummaryHeader / renderDefaultHeader helpers and
document the toolCalls.length > 1 count-suffix guard so a future
"fix" to >= 1 doesn't reintroduce "Read config.json · 1 tools".

Docs — add Scope & Lifecycle section to tool-use-summaries.md
covering (1) one generation per batch shared by both modes,
(2) no backfill on toggle / session resume, (3) main-agent batches
only with the Task-tool clarification.

* fix: address second-round review feedback on tool-use summaries

Critical — force-expand groups lost their summary entirely.
Previous round's "drop tool_use_summary unconditionally" merge fix
also stripped summaries for force-expanded groups, defeating the
exact case (errors, confirmations, focused shell) where the
standalone ● label is the label's only path to the screen. The
merge function now takes an absorbedCallIds set: summaries whose
preceding callIds are all absorbed by a compact tool_group header
are dropped (so refreshStatic still fires), but force-expanded
summaries pass through to be rendered standalone by
HistoryItemDisplay. MainContent computes absorbedCallIds from raw
history and passes it in. New tests cover both the absorbed-drop
and the force-expand-preserve cases plus the empty-set default
for callers that don't compute absorption.

Suggestion — late-arriving summaries could land out of order.
A slow fast-model call could resolve after the next turn's
content was committed, planting the ● label between later items
in full mode. The resolve callback now captures the first batch
callId, locates the corresponding tool_group at resolve time,
and drops the summary if a newer tool_group has already appeared
in history. New test exercises this with a manually-resolved
fast-model promise.

Suggestion — truncateJson allocated full JSON for large strings.
A 10MB ReadFile result was being JSON.stringify'd in full only to
be sliced down to 300 chars. Added preTruncate that walks the
value (depth-bounded to 4) and slices string leaves to maxLength
before serialization. Tests verify the input never reaches its
full pre-cap form.

Suggestion — settings description over-claimed SDK emission.
The description said summaries are emitted to SDK clients as a
tool_use_summary message; the SDK plumbing isn't actually wired
in this PR (the factory is exported for follow-up). Updated
settings.json description and regenerated the vscode schema to
state CLI-only scope explicitly.

Suggestion — fastModel data-boundary not documented.
When fastModel uses a different provider than the main session
model, tool inputs/outputs cross a new auth boundary that users
may not expect. Added "Data flow & privacy" section to the user
feature doc spelling out: same-provider fast model = no scope
change; different-provider = strictly larger sharing scope; two
escape hatches (same-provider fast model OR feature off).
Code-level mitigation (metadata-only mode) deferred.
2026-04-27 16:54:10 +08:00
Fu Yuchen
93cbad24b1
fix(core): preserve reasoning_content during session resume and active sessions (GH#3579) (#3590)
* fix(core): preserve reasoning_content during session resume and active sessions (GH#3579)

* chore(core): remove dead thinkingThresholdMinutes config after latch removal (GH#3579)
2026-04-24 17:49:05 +08:00
顾盼
aeeb2976d6
feat(web-search): remove built-in web_search tool, replace with MCP-based approach (#3502)
* feat(web-search): add GLM (ZhipuAI) web search provider

- Add GlmProvider class implementing BaseWebSearchProvider using the
  ZhipuAI Web Search API (https://open.bigmodel.cn/api/paas/v4/web_search)
- Support multiple search engines: search_std, search_pro, search_pro_sogou,
  search_pro_quark
- Support optional config: maxResults, searchIntent, searchRecencyFilter,
  contentSize, searchDomainFilter
- Truncate query to 70 characters per API limit
- Register 'glm' in the provider discriminated union (types.ts) and
  createProvider() switch (index.ts)
- Add GlmProviderConfig to settingsSchema, ConfigParams, and Config class
- Add --glm-api-key CLI flag and GLM_API_KEY env var support in webSearch.ts
- Forward GLM_API_KEY in sandbox environment
- Update provider priority list: Tavily > Google > GLM > DashScope
- Add 17 unit tests for GlmProvider and 4 integration tests in index.test.ts
- Update docs/developers/tools/web-search.md with GLM configuration,
  env vars, CLI args, pricing, and corrected DashScope billing info
- Fix stale OAuth/free-tier references in web-search.md

Closes #3496

* docs(web-search): fix DashScope note and add GLM server-side limitations

* fix(web-search): make DashScope provider work with standard API key, remove qwen-oauth dependency

- DashScopeProvider.isAvailable() now checks config.apiKey instead of authType
- Remove OAuth credential file reading and resource_url requirement
- Use standard DashScope endpoint: dashscope.aliyuncs.com/api/v1/indices/plugin/web_search
- Read DASHSCOPE_API_KEY env var and --dashscope-api-key CLI flag
- Forward DASHSCOPE_API_KEY into sandbox environment
- Update integration test to detect DASHSCOPE_API_KEY
- Update docs to reflect new API key based configuration

* feat(web-search): remove built-in web search tool

The web_search tool and all related provider implementations are removed.
Web search functionality will be provided via MCP integrations instead,
which is the direction the broader agent ecosystem is moving.

Removed:
- packages/core/src/tools/web-search/ (entire directory)
- packages/cli/src/config/webSearch.ts
- integration-tests/cli/web_search.test.ts
- ToolNames.WEB_SEARCH, ToolErrorCode.WEB_SEARCH_FAILED
- webSearch config in ConfigParams, Config class, settingsSchema
- CLI options: --tavily-api-key, --google-api-key, --google-search-engine-id,
  --glm-api-key, --dashscope-api-key, --web-search-default
- Sandbox env forwarding for TAVILY/GLM/DASHSCOPE/GOOGLE search keys
- web_search from rule-parser, permission-manager, speculation gate,
  microcompact tool set, and builtin-agents tool list

* fix: remove websearch reference

* docs: remove websearch tool

* docs: add break change guide

* fix review
2026-04-24 11:29:02 +08:00
Shaojin Wen
d71f2fab70
feat(cli): cap inline shell output with configurable line limit (#3508)
* feat(cli): cap inline shell output with configurable line limit

Long-running shell commands (npm install, find /, build logs) currently
fill the viewport with the full visible PTY buffer (up to availableHeight,
~24 lines on a typical terminal). The output dominates the screen and
pushes prior context off the top.

This caps inline ANSI shell output to a small window (default 5 lines,
matching Claude Code's ShellProgressMessage). The hidden line count is
already surfaced via the existing `+N lines` indicator in
`ShellStatsBar`, so users still know how much was elided.

The cap applies only when nothing in the existing escape-hatch set is
true:
  - `forceShowResult` (errors, !-prefix user-initiated commands,
    tools awaiting confirmation, agents pending confirmation)
  - `isThisShellFocused` (ctrl+f focus on a running embedded PTY shell)
  - `ui.shellOutputMaxLines = 0` (user opt-out)

Also adds a new `ui.shellOutputMaxLines` setting (default 5) so users
can adjust or disable the cap. The SettingsDialog renders it
automatically via the existing `type: 'number'` schema path.

Notes on scope:
  - Only the `'ansi'` display branch is capped. `'string'`, `'diff'`,
    `'todo'`, `'plan'`, `'task'` renderers are untouched.
  - `AnsiOutputDisplay` is only produced by shell tools (`shell.ts`,
    `shellCommandProcessor.ts`), so other tool outputs are unaffected.
  - The `+N lines` count is bounded by the headless xterm buffer height
    (~30 rows) — a pre-existing limitation of the buffer-based stats,
    not introduced here.

Tests:
  - 4 new ToolMessage tests cover cap default, forceShowResult bypass,
    settings disable (cap=0), and custom cap value.
  - The existing `MockAnsiOutputText` / `MockShellStatsBar` mocks were
    extended to print `availableTerminalHeight` / `displayHeight` so
    the cap behavior is asserted at the prop level.

* fix(cli): apply shell output cap to completed string display too

Initial PR caught only the streaming ANSI branch. AI shell tools emit
the final completed result through `shell.ts:returnDisplayMessage =
result.output`, which is a plain string. That string went through
`StringResultRenderer` with the unmodified `availableHeight`, so the
cap was effectively bypassed for the steady-state display the user
actually sees most of the time.

Verified manually in tmux: a `seq 1 30` invocation by the AI now
collapses to "first 26 lines hidden ... 27 28 29 30" instead of
listing all 30 rows. `!`-prefix `seq 1 30` still expands fully via
the existing `isUserInitiated → forceShowResult` bypass.

Changes:
  - Detect shell tool by name (matches existing `SHELL_COMMAND_NAME` /
    `SHELL_NAME` checks already used in this file)
  - Rename `ansiAvailableHeight` → `shellCapHeight` since it now
    governs the string branch as well
  - Pass `shellCapHeight` to `StringResultRenderer`; the value
    falls back to `availableHeight` for non-shell tools so other
    tools' string output is unaffected
  - Two new tests: shell completed string is capped; non-shell
    string is not
  - Two existing tests updated to use `name="Shell"` so they actually
    exercise the cap path (would previously have passed by accident
    since the original code didn't check tool name)

Also picks up the auto-regenerated VSCode IDE companion settings
schema entry for `ui.shellOutputMaxLines`.

* fix(cli): symmetrize ANSI/string row counts and clamp shell cap input

Addresses two non-blocking review observations on #3508.

Off-by-one between paths:
  MaxSizedBox reserves one row for its overflow banner when content
  exceeds maxHeight (visibleContentHeight = max - 1). The ANSI path
  pre-slices to N in AnsiOutputText so MaxSizedBox sees exactly N
  rows and renders all N — plus the separate ShellStatsBar line.
  The string path passes the raw cap and lets MaxSizedBox handle
  overflow, so it shows N-1 content rows + the banner.

  Result with cap=5: ANSI showed 5+stats, string showed 4+banner.
  Pass shellCapHeight + 1 to StringResultRenderer when capping so
  both paths render N visible content rows. Verified in tmux: the
  completed Shell tool box now reports `... first 25 lines hidden ...`
  followed by lines 26-30 (was 26 + lines 27-30).

Setting validation:
  Schema accepts any number; the dialog only rejects NaN. Negatives
  silently disabled the cap (only 0 is documented as off) and
  fractional values produced fractional slice counts. Added
  Math.max(0, Math.floor(value || 0)) at the use site so:
   - negatives → 0 → cap disabled (matches the documented opt-out)
   - fractions → floor → whole-row cap
   - non-numeric (raw settings.json edits) → 0 → cap disabled
  Schema-level minimum/integer constraints aren't supported by the
  current settings infrastructure (no other number setting uses
  them either), so the guard lives at the use site.

Tests:
  - Updated string-cap test to assert lines 26-30 visible (catches
    the +1 fix; was lines 27-30 before)
  - New parameterized test covers -1, 1.5, and a non-numeric value
2026-04-22 14:37:13 +08:00
zhangxy-zju
ebe364d0b8
feat(retry): add persistent retry mode for unattended CI/CD environments (#3080)
* feat(retry): add persistent retry mode for unattended CI/CD environments

When running in CI/CD pipelines or background daemon mode, transient API
capacity errors (429/529) should not terminate long-running tasks after a
fixed number of retries. This adds an environment-aware persistent retry
mode that retries indefinitely for transient errors, with exponential
backoff capped at 5 minutes and heartbeat keepalives every 30 seconds to
prevent CI runner timeouts.

* docs: add persistent retry mode documentation

Add environment variable entries (QWEN_CODE_UNATTENDED_RETRY, QWEN_CODE_BG)
to the settings reference, and a new "Persistent Retry Mode" section to the
headless mode docs covering activation, behavior, and CI/CD usage examples.

* refactor(retry): simplify to single explicit env var QWEN_CODE_UNATTENDED_RETRY

Remove QWEN_CODE_BG and CI=true as activation triggers for persistent retry.
Having multiple env vars with identical behavior adds confusion, and silently
activating infinite retry on CI=true is dangerous — a regular CI test hitting
a 429 would hang forever instead of failing fast.

* fix(retry): address PR review feedback

- Forward caller's abortSignal into retryWithBackoff in both
  baseLlmClient.ts and geminiChat.ts so persistent waits remain
  cancellable (wenshao)
- Re-apply maxBackoff and capMs after jitter so delays strictly
  respect stated caps (Copilot)
- Respect shouldRetryOnError in persistent mode so callers can
  force fast-fail even for transient 429/529 errors (Copilot)
- Guard sleepWithHeartbeat against infinite loop when heartbeat
  interval is <= 0 via Math.max(1, ...) (Copilot)
- Normalize isEnvTruthy with trim/toLowerCase for robust env
  var parsing across CI conventions (Copilot)

* test(retry): add missing UT for shouldRetryOnError override and heartbeat zero-interval guard

* fix(retry): do not cap Retry-After delays at maxBackoff

Server-specified Retry-After values should only be limited by the
absolute cap (capMs/6h), not the exponential backoff cap (maxBackoff/5min).
Jitter is also skipped for Retry-After since the server already specified
the exact wait time.

* refactor(retry): align isUnattendedMode with project env parsing convention

Replace custom isEnvTruthy (trim + toLowerCase) with strict matching
(val === 'true' || val === '1') to match parseBooleanEnvFlag used
elsewhere in the codebase. Prevents inconsistent behavior where
'TRUE' or ' 1 ' would activate persistent retry here but not in
telemetry or other env-driven features.

* test(retry): add Retry-After handling tests for persistent mode

Cover three key behaviors:
- Retry-After is NOT capped at maxBackoff (only at capMs)
- Retry-After IS capped at persistentCapMs absolute limit
- Retry-After delays have no jitter applied

* fix(test): add isUnattendedMode to retry.js mock in baseLlmClient tests

The existing vi.mock for retry.js only exported retryWithBackoff.
After adding isUnattendedMode to the retry module, baseLlmClient.ts
imports it, causing all 10 generateJson tests to fail with
'No "isUnattendedMode" export is defined on the mock'.

* fix(retry): wire persistent retry mode into client.ts generateContent

Forward persistentMode and abortSignal to retryWithBackoff() in
GeminiClient.generateContent(), matching the existing wiring in
baseLlmClient.ts and geminiChat.ts.
2026-04-21 22:08:11 +08:00
Shaojin Wen
afbb5e71db
fix(cli): rework session recap rendering and add blur threshold setting (#3482)
* feat(cli): make recap away-threshold configurable

The 5-minute blur threshold was hard-coded. Confirmed from Claude
Code's own binary (v2.1.113) that 5 minutes is their default as well
(and that they shift to 60 minutes when 1h prompt-cache is active) —
so the default stays, but expose it as `general.sessionRecapAway
ThresholdMinutes` for users who briefly alt-tab often and don't want
recaps piling up, or who want to lower it for testing.

Non-positive / unset values fall back to the 5-minute default, so
dropping the key has the same behavior as before.

* fix(core): align recap prompt with Claude Code (1-2 sentences, ≤40 words)

The earlier "exactly one sentence, 80-char cap" was an over-correction
to a single in-the-moment ask. Going back to it: the natural shape of
"current task + next action" is two clauses, and forcing them into a
single sentence either crams them with a semicolon or drops the next
action entirely on complex sessions.

Adopt Claude Code's prompt verbatim (extracted from the v2.1.113
binary): "under 40 words, 1-2 plain sentences, no markdown. Lead with
the overall goal and current task, then the one next action. Skip
root-cause narrative, fix internals, secondary to-dos, and em-dash
tangents." Add a Chinese-budget note (~80 chars) and keep the
<recap>...</recap> wrapping that protects against reasoning-model
preambles leaking into the UI.

The sticky banner already re-measures controls height when the
recap toggles, so a 2-line render lays out cleanly.

Sweep "one-line" out of user-facing copy (settings description,
slash-command description, feature docs, design doc) so the
documentation matches the new shape.

* fix(cli): restore "one-line" in user-facing recap copy

Verified from the Claude Code v2.1.113 binary that the slash-command
description IS literally "Generate a one-line session recap now" even
though the underlying prompt allows 1-2 sentences. Claude Code is
deliberately setting a tighter user expectation than the prompt
guarantees, which keeps the surface feel "glanceable".

Mirror that asymmetry: keep the prompt at 1-2 sentences (the previous
commit) for behavioral parity, but put "one-line" back in the user-
visible copy (slash-command description, settings description, user
docs). Internal design doc keeps the accurate "1-2 sentence" wording.

* fix(cli): render recap inline in history to match Claude Code

Earlier I read the user's complaint that the recap "scrolled away" as
"the recap should be sticky above the input box," and built a sticky
banner accordingly. Disassembly of the Claude Code v2.1.113 binary
shows the actual behavior is the opposite: their away_summary is a
plain `type:"system", subtype:"away_summary"` message dispatched
through the standard message renderer (no Static, no anchor, no
flexbox pinning) — it scrolls with the conversation like every other
system message.

Tear out the sticky-banner machinery so recap matches that:

- Recap is back in the `HistoryItemWithoutId` union and `addItem`'d
  into history (both from `/recap` and from auto-trigger), so it
  serializes into session saves and behaves like every other history
  item — no special clear paths, no resume-wrapper, no layout-effect
  re-measure dance.
- `useAwaySummary` takes `addItem` again instead of a setter callback.
- `AwayRecapMessage` renders the way Claude Code does: a 2-column
  gutter with `※`, then bold "recap: " and italic content, all in
  dim color. Drop the prior `StatusMessage`-shaped layout that fused
  prefix and label into "※ recap:".
- Remove the AppContainer plumbing, the slashCommandProcessor state,
  the UIStateContext fields, the DefaultAppLayout / ScreenReader
  placement blocks, the test-utils mocks, and the noninteractive
  stub. Restore `useResumeCommand.handleResume` to a void return
  since callers no longer need the success boolean.

Sweep the design doc so the architecture diagram, files table, and
hook deps reflect the inline-history flow.

* fix(cli): dedupe back-to-back auto-recaps with no new user turns between

Two consecutive blur cycles, each over the threshold but with no new
user activity in between, would each fire their own auto-recap and
add two near-duplicate entries to history (same task, slightly
different wording from temperature-driven LLM variance). Reported
case: leaving the terminal twice while a /review of one PR was
still on screen produced two recaps both about that same review.

Add a `shouldFireRecap` gate before kicking off the LLM call:

- Need at least 3 user messages in history total (don't fire on a
  near-empty session).
- If a previous away_recap is already in history, need at least 2
  new user messages since that one before another can fire.

Same shape as Claude Code's `Ic1` gate (`Sc1=3`, `Rc1=2`). Read
history through a ref so this isn't in the effect's deps and the
effect doesn't re-run on every message.

* fix(cli): type useResumeCommand.handleResume as Promise<void>

Per gemini review on #3482: the interface declared this as `() => void`
but the implementation is `async` and returns `Promise<void>`. The
mismatch silently lost the chainable promise — tests had to launder
it through `as unknown as Promise<void> | undefined` just to await.

Tighten the interface to `Promise<void>` and drop the cast in the
"closes the dialog immediately" test.

* fix(cli): persist auto-fired recap to chat recording so /resume keeps it

Per yiliang114 review on #3482: the manual `/recap` path persists across
`/resume` because the slash-command processor records every output
history item via `chatRecorder.recordSlashCommand({ phase: 'result',
outputHistoryItems })`, but the auto path called `addItem` directly
and bypassed that recorder. The result was an asymmetry where users
who triggered recap manually saw it after `/resume`, while users whose
recap fired automatically lost it.

Mirror the manual recording from useAwaySummary's `.then` callback —
record only the `result` phase (not invocation, since we don't want
a fake `> /recap` user line replayed) with the away-recap item as the
single output. Wrapped in try/catch because recap is best-effort and
must never surface a failure to the user.

Add useAwaySummary.test.ts covering:
- the recording path is taken on a successful auto-trigger
- the dedup gate (`shouldFireRecap`) suppresses the LLM call entirely,
  including the recording, when no new user turns happened since the
  last recap

* fix(cli): cast recap item via spread to satisfy strict tsc --build

CI's `tsc --build` (stricter than local `tsc --noEmit`) rejected the
direct `item as Record<string, unknown>` cast: HistoryItemAwayRecap's
literal `type: 'away_recap'` field doesn't overlap with `unknown`,
TS2352. Use the `{ ...item } as Record<string, unknown>` spread
pattern that the rest of the codebase (arenaCommand,
slashCommandProcessor's serializer) already uses for the same
SlashCommandRecordPayload field.
2026-04-21 14:39:13 +08:00
Shaojin Wen
52c7a3d0ed
fix(cli): pin /recap above input and align defaults with fastModel (#3478)
Some checks are pending
Qwen Code CI / Lint (push) Waiting to run
Qwen Code CI / Test (push) Blocked by required conditions
Qwen Code CI / Test-1 (push) Blocked by required conditions
Qwen Code CI / Test-2 (push) Blocked by required conditions
Qwen Code CI / Test-3 (push) Blocked by required conditions
Qwen Code CI / Test-4 (push) Blocked by required conditions
Qwen Code CI / Test-5 (push) Blocked by required conditions
Qwen Code CI / Test-6 (push) Blocked by required conditions
Qwen Code CI / Test-7 (push) Blocked by required conditions
Qwen Code CI / Test-8 (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* fix(cli): pin /recap above input box and align defaults with fastModel

The recap rendered as a regular history item, so as soon as the model
streamed a new reply the "where you left off" reminder scrolled out of
view. Move it to a sticky banner anchored just above the Composer
(matching how btwItem is rendered) so it stays visible across turns.

While reworking the surface, also:
- Replace the chevron prefix with `※ recap:` so it reads as a labeled
  recap line instead of a generic dim message.
- Mirror the placement in ScreenReaderAppLayout so screen-reader users
  see it in the same logical position.
- Drop HistoryItemAwayRecap from the HistoryItemWithoutId union — it
  is no longer addItem-able, and leaving it in invited silent no-op
  bugs where addItem(awayRecap) would compile but render nothing.
- Clear the banner on /clear, /reset, /new and on /resume into a
  different session, so a recap from a previous context doesn't bleed
  into a freshly started one.
- Re-measure the controls box when the banner appears or disappears
  (its height changes by a couple of lines) so the main content area
  recomputes availableTerminalHeight and stays laid out correctly.

Auto-trigger now defaults to "on iff fastModel is configured" rather
than unconditionally on. Running an ambient background recap on the
main coding model is too costly and slow to be a sane default; tying
it to fastModel means the feature is silently opt-in for users who
have set up a cheap fast model. An explicit `general.showSessionRecap`
override still wins either way, and `/recap` itself is unaffected.

Sharpen the slash-command description to match the new behavior.

* fix(core): silence AbortSignal listener-leak warning in OpenAI pipeline

Every chat.completions.create call wires up an abort listener on the
incoming AbortSignal, and several layers — retryWithBackoff, the
LoggingContentGenerator wrapper, the SDK's own internal stream/fetch
plumbing — register their own listeners against the same signal. Five
retry attempts plus those layers comfortably exceed Node's default
10-listener cap and produce a MaxListenersExceededWarning. With
features that share or compose signals (e.g., recap + followup
speculation firing on the same response cycle), even a higher cap
gets blown past.

The signals here are per-request and short-lived, so the accumulation
is structural rather than a real memory leak — they get GC'd as soon
as the request settles. setMaxListeners(0, signal) at the SDK boundary
disables the warning for these specific signals only, without masking
any genuine leak elsewhere in the process. Idempotent and confined to
the one place where retry-bound API calls cross into the SDK.

* fix(core): tighten recap to a single sentence within 80 chars

The 1-3 sentence budget reliably wrapped onto two lines in the sticky
banner above the input box, which made it visually heavy for what is
supposed to be a glanceable reminder. Constrain the prompt to exactly
one sentence with a hard 80-char cap, and merge the "high-level task
+ next step" rule into a single sentence instead of two adjacent ones.

Also sweep the docs (settings, commands, design) so the user-facing
copy and the internal design notes match the new format.

* fix(cli): apply review feedback for recap PR

Two issues from review:

- The schema description for `general.showSessionRecap` still said
  "1-3 sentence summary" while the prompt, docs, and slash-command
  copy already say "one-line". Aligns the text in settingsSchema.ts
  and the regenerated VSCode JSON schema.

- The /resume wrapper cleared the sticky recap synchronously, before
  the inner handler had a chance to discover that no session data
  was available. On a no-op resume the user would still lose the
  current recap. Make `useResumeCommand.handleResume` return
  Promise<boolean> reporting whether a session actually loaded, and
  only clear the recap on a confirmed switch.

* fix(cli): default showSessionRecap to false and drop fastModel heuristic

The earlier "enabled iff fastModel is configured" default made it hard
for users to answer the simple question "is auto-recap on for me right
now?" — the answer depended on a setting from a different category,
and setting/unsetting fastModel silently changed recap behavior.

Revert to a plain boolean with a conservative off-by-default:

- Auto-trigger fires only when the user explicitly sets
  `general.showSessionRecap: true`.
- Manual `/recap` keeps working regardless (that's a user-initiated
  call, not an ambient one).
- Users never get ambient LLM calls billed to their main coding model
  without having opted in.

Aligns settings.md, design doc, and the regenerated JSON schema.
2026-04-20 23:58:19 +08:00
ihubanov
0b8b3da836
feat(cli): add slashCommands.disabled setting to gate slash commands (#3445)
* feat(cli): add slashCommands.disabled setting to gate slash commands

Introduces a first-class way for operators to hide and refuse to execute
specific slash commands. Useful for multi-tenant / enterprise / sandboxed
deployments where different users should see different command subsets.

The denylist is sourced from three unioned inputs:

  * `slashCommands.disabled` settings key (string[], UNION merge), so
    workspace scopes can only add to a denylist set at user or system
    scope, never shrink it — matching the shape already used by
    `permissions.deny`.
  * `--disabled-slash-commands` CLI flag (comma-separated or repeated).
  * `QWEN_DISABLED_SLASH_COMMANDS` environment variable.

Matching is case-insensitive against the final (post-rename) command
name, so extension commands are addressable by their disambiguated
form (e.g. `firebase.deploy`). Disabled commands are removed from
`CommandService`'s output, so they disappear from autocomplete and
produce the standard unknown-command path in both interactive TUI and
non-interactive (`--prompt`) modes.

The scope of this change is slash commands only: it does not affect
tool permissions (still `permissions.deny`) or keyboard shortcuts.

* chore(cli): regenerate settings.schema.json for slashCommands.disabled

Regenerates the companion JSON schema consumed by the VS Code extension
after adding the `slashCommands.disabled` entry to the TS schema in the
previous commit. Required by the "Check settings schema is up-to-date"
CI lint step.

* fix(cli): route disabled slash commands to unsupported, not no_command

handleSlashCommand was passing the disabled denylist straight into
CommandService.create, so disabled commands disappeared from
`allCommands` too. The fallback existence check that distinguishes
"known but not allowed in non-interactive mode" from "truly unknown"
then failed, and disabled commands like `/help` fell through to
`no_command` — causing the caller to forward them to the model as
plain prompt text.

Keep `allCommands` unfiltered and apply the denylist only when
constructing the executable set and when producing the unsupported
response. A disabled command now returns `unsupported` with a
"disabled by the current configuration" reason and never reaches the
model. Added three regression tests covering the primary case,
case-insensitive match, and the preserved no_command path for
genuinely unknown input.
2026-04-20 11:06:26 +08:00
Shaojin Wen
60a6dfc14c
feat(cli): add session recap with /recap and auto-show on return (#3434)
Some checks are pending
Qwen Code CI / Lint (push) Waiting to run
Qwen Code CI / Test (push) Blocked by required conditions
Qwen Code CI / Test-1 (push) Blocked by required conditions
Qwen Code CI / Test-2 (push) Blocked by required conditions
Qwen Code CI / Test-3 (push) Blocked by required conditions
Qwen Code CI / Test-4 (push) Blocked by required conditions
Qwen Code CI / Test-5 (push) Blocked by required conditions
Qwen Code CI / Test-6 (push) Blocked by required conditions
Qwen Code CI / Test-7 (push) Blocked by required conditions
Qwen Code CI / Test-8 (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* feat(cli): add session recap with /recap and auto-show on return

Users often open an old session days later and need to scroll through
pages to remember where they left off. This change adds a short
"where did I leave off" recap — a 1-3 sentence summary generated by
the fast model — so they can resume without re-reading the history.

Two triggers:
- /recap: manual slash command.
- Auto: when the terminal has been blurred for 5+ minutes and gets
  focused again (uses the existing DECSET 1004 focus protocol via
  useFocus). Gated on streamingState === Idle so it never interrupts
  an active turn. Only fires once per blur cycle.

The recap is rendered in dim color with a chevron prefix, visually
distinct from assistant replies. A new `general.showSessionRecap`
setting controls the auto-trigger (default on). /recap works
independent of the setting.

Implementation notes:
- generateSessionRecap uses fastModel (falls back to main model),
  tools: [], maxOutputTokens: 300, and a tight system prompt. It
  strips tool calls / responses from history before sending — tool
  responses can hold 10K+ tokens of file content that drown the recap
  in irrelevant detail. The 30-message window respects turn boundaries
  (slice never starts on a dangling model/tool response).
- Output is wrapped in <recap>...</recap> tags; the extractor returns
  empty (skips render) if the tag is missing, preventing model
  reasoning from leaking into the UI.
- All failures are silent (return null) and logged via a scoped
  debugLogger; recap is best-effort and must never break main flow.
- /recap refuses to run while a turn is pending.

* fix(cli): abort in-flight recap when showSessionRecap is disabled

If the user disables showSessionRecap while an auto-recap LLM call is
already in flight, the previous code returned early without aborting.
The pending .then would still pass its idle/abort guards and append the
recap, producing an unwanted message after the user has opted out.

Abort the controller and clear it eagerly so the resolved promise no
longer adds to history.

* fix(cli): gate /recap and auto-recap on streaming idle state

Two related issues from review:

1. /recap was only refusing when ui.pendingItem was set, but a normal
   model reply runs with streamingState === Responding and a null
   pendingItem. Invoking /recap mid-stream would generate a recap from
   a partial conversation and insert it between the user prompt and
   the assistant reply.

2. useAwaySummary cleared blurredAtRef before checking isIdle, so if
   focus returned during a still-streaming turn (after a >5min blur)
   the recap was permanently dropped — there was no later retry when
   the turn became idle, because isIdle was not in the effect deps.

Fixes:
- Expose isIdleRef on CommandContext.ui (mirrors btwAbortControllerRef
  pattern). Plumb it from AppContainer through useSlashCommandProcessor.
- recapCommand now refuses when isIdleRef.current is false OR
  pendingItem is non-null.
- useAwaySummary preserves blurredAtRef on the !isIdle bail and adds
  isIdle to the effect deps, so the trigger re-evaluates when the
  current turn finishes.
- Brief blurs (< AWAY_THRESHOLD_MS) still reset blurredAtRef.

Also seeds isIdleRef in nonInteractiveUi and mockCommandContext so the
new field has a sensible default outside the interactive UI.

* docs: document /recap command, showSessionRecap setting, and design

- User docs: add /recap to the Session and Project Management table in
  features/commands.md and a dedicated subsection covering manual use,
  the auto-trigger, the dim-color rendering, and the fast-model tip.
- User docs: add general.showSessionRecap row to the configuration
  settings reference.
- Design doc: docs/design/session-recap/session-recap-design.md covers
  motivation, the two trigger paths, the per-file architecture, prompt
  design with the <recap> tag and three-tier extractor, history
  filtering rationale (functionResponse can be 10K+ tokens), the
  useAwaySummary state machine, the isIdleRef gating for /recap, model
  selection, observability, and out-of-scope items.

* fix(core): exclude thought parts from session recap context

filterToDialog kept any non-empty text part, but @google/genai's Part
type also marks model reasoning with part.thought / part.thoughtSignature.
That hidden chain-of-thought was being fed to the recap LLM and could
get summarized as if it were user-visible dialogue.

Drop parts where either flag is set. Update the design doc's
History 过滤 section to call this out alongside the existing
tool-call/response rationale.

* docs(session-recap): correct debug-logging guidance, fill in state machine, sharpen UX wording

Audit of the session recap docs against the implementation found three
issues worth fixing:

- Design doc claimed debug logs were enabled via a QWEN_CODE_DEBUG_LOGGING
  env var. That var does not exist; debug logs are written to
  ~/.qwen/debug/<sessionId>.txt by default, gated by QWEN_DEBUG_LOG_FILE.
  Replace with the accurate path + opt-out behavior, and tell the reader
  to grep for the [SESSION_RECAP] tag.
- Design doc's useAwaySummary state machine table was missing the
  isFocused && blurredAtRef === null path (taken on first render and
  right after a brief-blur reset). Add the row.
- User doc's "Refuses to run ... failures are silent" line conflated the
  inline-error refusal with silent generation failures, and "(when the
  conversation is idle)" used internal jargon. Split the two cases and
  spell out what "idle" means, including the wait-then-fire behavior
  when focus returns mid-turn.

* docs(session-recap): correctly describe /recap vs auto-trigger failure modes

The previous wording said "Generation/network failures are silent — the
recap simply does not appear", but recapCommand returns a user-facing
info message ("Not enough conversation context for a recap yet.") in
exactly that path, and also returns inline messages for the
config-not-loaded and busy-turn guards.

Only the auto-trigger path is truly silent (it just skips addItem when
generateSessionRecap returns null). Split the two paths in the doc so
the manual command's "always responds with something" behavior is
distinguished from the auto-trigger's no-op-on-failure behavior.

* docs(session-recap): align prompt-rules section with the actual prompt

Two doc-vs-code mismatches in the design doc's "System Prompt" section,
caught with the same lens as yiliang114's failure-mode review:

- The bullet list claimed RECAP_SYSTEM_PROMPT forbids "推测用户意图"
  and "用 'you' 称呼用户". Those rules existed in an early draft but
  were dropped when the <recap> tag rules were added; the current
  prompt has no such restrictions. Replace with the actual rules and
  add a "与 RECAP_SYSTEM_PROMPT 一一对应" marker so future edits stay
  in sync.
- The doc said systemInstruction "覆盖" the main agent prompt. True
  for the agent prompt portion, but GeminiClient.generateContent
  internally calls getCustomSystemPrompt which appends user memory
  (QWEN.md / 自动 memory) as a suffix. Spell that out — the final
  system prompt is recap prompt + user memory, which is actually
  useful project context for the recap.

* docs(session-recap): translate design doc to English

The repo convention for docs/design is English (7 of 8 existing files;
auto-memory/memory-system.md is the only Chinese one). The first version
of this design doc followed the auto-memory example, which turned out
to be the wrong sample.

Translate to English while preserving the existing structure, the
state-machine table, the prompt-vs-doc 1:1 alignment, the
QWEN_DEBUG_LOG_FILE description, and the failure-mode notes added in
prior commits.

* fix(cli): drop empty info return from /recap interactive success path

The interactive success path inserts the away_recap history item
directly via ui.addItem and then returned `{type: 'message',
messageType: 'info', content: ''}`. The slash-command processor's
'message' case unconditionally calls addMessage, which adds another
HistoryItemInfo with empty text. The empty info renders as nothing
(StatusMessage early-returns null), but it still bloats the in-memory
history list and shows up in /export and saved sessions.

Return void on the interactive success path and on the abort path so
the processor's `if (result)` check skips the message-handler branch
entirely. Widen the action's return type to `void | SlashCommandActionReturn`
to match (same shape as btwCommand).
2026-04-19 21:38:48 +08:00