Commit graph

5966 commits

Author SHA1 Message Date
顾盼
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
Dragon
56522bd89c
fix(core): enable cache control for Token Plan (#4495) 2026-05-25 20:14:37 +08:00
易良
5493888c15
ci: split Aliyun OSS sync into a separate post-release workflow (#4492)
* ci: split Aliyun OSS sync into a separate post-release workflow

The OSS upload and verification steps were adding significant time to the
release workflow's critical path. Move them into a new `sync-release-to-oss.yml`
workflow that triggers on `release: published`, running asynchronously after
the release completes.

Key changes:
- Extract all OSS steps (ossutil install, credential config, asset upload,
  verification, hosted installation sync, latest VERSION pointer) into
  `sync-release-to-oss.yml`
- Switch `gh release create` to use CI_BOT_PAT so the release event can
  trigger the new downstream workflow (GITHUB_TOKEN events don't trigger
  other workflows)
- Add `workflow_dispatch` input for manual re-runs on failure
- New workflow downloads release assets from GitHub Release instead of
  rebuilding them

This decouples publishing from CDN distribution: the release finishes as
soon as npm publish + GitHub Release are done, and China CDN sync happens
in parallel without blocking.

* fix(test): update install-script test to check sync-release-to-oss.yml

The test asserts OSS sync steps exist in the workflow. Now that these
steps live in sync-release-to-oss.yml instead of release.yml, update the
test to read from the correct file and add assertions that release.yml
no longer contains OSS logic.

* fix(ci): address review feedback for OSS sync split

- Add 'Verify Standalone Archives' step before gh release create in
  release.yml as a pre-publish safety gate (wenshao)
- Add concurrency group to sync-release-to-oss.yml to prevent race
  conditions when multiple releases publish close together (wenshao)
- Update test to assert verify step exists in release.yml

* chore: add comment explaining CI_BOT_PAT requirement [skip ci]
2026-05-25 19:34:21 +08:00
pomelo
7cb017d4b0
docs(agents,pr-template): add Working Principles and restructure PR template (#4496)
* docs(agents): add Working Principles and file/comment conventions

Add a "Working Principles" section at the top of AGENTS.md, with
Simplicity First (adapted from Andrej Karpathy's CLAUDE.md) as the lead
principle. Extend Code Conventions with two new entries:
- File naming: PascalCase for React components, kebab-case preferred for
  new non-component files, existing camelCase stays as-is.
- Comments: default to none; explain why, not what.

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

* docs(agents): link Karpathy's CLAUDE.md in attribution

Per review feedback, make the source attribution clickable so reviewers
can reach the original document in one hop.

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

* docs(agents): align comments guidance — "default to none"

Raise the bar for code comments from "add sparingly" to "default to
none" in the runtime prompt, matching the AGENTS.md convention. Add a
preservation clause to AGENTS.md so agents do not strip existing
high-value comments during cleanup passes. Update snapshots.

Co-Authored-By: Qwen Code <noreply@alibaba-inc.com>

* docs(pr-template): restructure for reviewer test plan clarity

- Reorganize PR template around a Reviewer Test Plan section with How to verify, Before/After, and Tested on
- Add collapsible Chinese description section for bilingual PRs
- Simplify create-pr command guidance to match the new template
- Tighten AGENTS.md file naming and comments conventions; align PR submission guide with the new template

This makes PRs easier to review by focusing contributors on the evidence reviewers need most.

* docs(pr-template): merge Before/After into Evidence and require full Chinese translation

- Consolidate Before and After sections into a single Evidence (Before & After) section
- Update Chinese summary comment to require full paragraph-by-paragraph translation instead of abbreviated bullets

This reduces template redundancy for non-UI changes and ensures the Chinese block is a proper translation, not a summary.

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen Code <noreply@alibaba-inc.com>
2026-05-25 19:15:35 +08:00
Dragon
35e6963285
docs(tools): document monitor tool (#4356) 2026-05-25 15:25:07 +08:00
qqqys
05458d59ec
fix(core): strip additional dangerous interpreter rules (#4371)
* fix(core): strip additional dangerous interpreter rules

* test(core): clarify dangerous interpreter coverage

* chore(core): group bash.exe with windows shells

* fix(core): normalize dangerous interpreter tokens

* test(core): normalize dangerous exe interpreter rules

* fix(core): detect windows interpreter path allows

* fix(core): detect windows interpreter path allows
2026-05-25 14:33:05 +08:00
kkhomej33-netizen
632865c0df
feat(core): limit background agent concurrency (#4324)
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
* feat(core): limit background agent concurrency

* fix(core): handle background agent cap on resume
2026-05-25 14:02:45 +08:00
qqqys
45a51185eb
fix(extension): redact credentialed source diagnostics (#4426)
* fix(extension): redact credentialed source diagnostics

* fix(extension): avoid leaking redacted URL causes

* fix(extension): close credential redaction gaps
2026-05-25 13:07:08 +08:00
ihubanov
9363879ea1
fix(core): preserve duplicate object references in safeJsonStringify (#4407)
* fix(core): preserve duplicate object references in safeJsonStringify

The replacer kept a WeakSet of every object it had ever seen. JSON.stringify
calls the replacer for every key in a DFS walk, siblings included, and the
set was never trimmed when the walk unwound. So the second sibling that
pointed at the same object got replaced with [Circular]. Not a cycle, just
a duplicate reference. Same false positive for repeated array elements and
for any shared leaf that appears on more than one branch.

Track the current ancestor path instead. The replacer's `this` is the parent
of `value`, so on each call pop the stack back to wherever the walk currently
is, then check the remaining ancestors for membership. Only real cycles get
flagged.

Existing cycle tests still pass. Added five regression tests covering shared
siblings, repeated array elements, shared subtree leaves, indirect cycles,
and a mix of duplicate ref + real cycle in the same graph.

* test(core): cover deep unwinding and toJSON paths in safeJsonStringify

Four regression tests covering corners the initial five missed:

- Shared leaf reached through five levels of nesting plus a sibling branch.
  Exercises the unwind loop popping multiple frames between the deep arm
  and the sibling arm of the walk.

- Real cycle (root referenced back from depth 5). Same depth as above but
  the deep arm closes the loop, so the ancestor check must still fire.

- Shared object returned by toJSON from two sibling positions. The replacer
  sees the post-toJSON value, so duplicate-ref handling has to recognize
  these as duplicates even though the carriers are different objects.

- Cycle through a toJSON that returns an ancestor. Confirms the ancestor
  check fires on the toJSON return value, not the toJSON-bearing carrier.

Per review feedback on #4407.
2026-05-25 13:06:55 +08:00
顾盼
24ebfbc13e
feat(memory): load .qwen/QWEN.local.md as project-local context (#4091) (#4394)
* feat(memory): load .qwen/QWEN.local.md as project-local context (#4091)

Adds a per-developer, project-scoped context file slot at
`<projectRoot>/.qwen/QWEN.local.md`. Loaded after all hierarchical
QWEN.md / AGENTS.md files so local instructions can supplement or
override shared ones.

Use case: project-specific but personal instructions (local cluster IDs,
container registry namespaces, accounts) that shouldn't live in the
shared root `QWEN.md` (exposes them to the team) or in the global
`~/.qwen/QWEN.md` (applies to every project). Mirrors Claude Code's
`.claude/CLAUDE.local.md` convention.

The slot is single and fixed (project root only — not searched in CWD
subdirectories or via upward traversal), gated by the same trust and
explicit-only checks as the rest of project-level discovery, and counted
in `fileCount` so the `/memory` panel surfaces it. Users must gitignore
the file themselves; `.qwen/` is not auto-ignored and `.qwen/settings.json`
is commonly committed.

* fix(memory): support .git-file repos when locating QWEN.local.md slot

`findProjectRoot()` only accepted `.git` as a directory, so in git
worktrees and submodules (where `.git` is a file containing a `gitdir:`
pointer) it returned `null`. The new `.qwen/QWEN.local.md` slot then
fell back to `<cwd>/.qwen/QWEN.local.md`, silently breaking the
documented "single fixed slot at project root" behavior for users
inside worktrees — including the developer of this feature.

Two changes:

1. `findProjectRoot()` now accepts `.git` as either a directory or a
   regular file. This also incidentally repairs pre-existing breakage
   in `rulesDiscovery` / hierarchical-search stop boundary, both of
   which consume the same helper.

2. The local-context-file slot now requires a real `foundRoot` (the
   `null` case is no longer covered by the `effectiveRoot` fallback).
   Without this guard:
     - a deep cwd in a non-git workspace turned the slot into a
       per-cwd file, opposite the design;
     - `cwd === homedir` resolved the slot to `~/.qwen/QWEN.local.md`,
       colliding with the global Qwen directory.

Three regression tests pin the new behavior: `.git`-as-file is
recognized, no-`.git`-ancestor skips the slot, `cwd === homedir`
without `.git` does not promote a global file to project-local.

* refactor(memory): extract findProjectRoot to shared utility (#4091)

Two duplicate `findProjectRoot` helpers existed in
`packages/core/src/utils/`: one in `memoryDiscovery.ts` (returns
`Promise<string | null>`) and one in `memoryImportProcessor.ts`
(returns `Promise<string>`, falls back to startDir). The previous fix
in 97c6fb41f only updated the first copy for `.git`-file support, so
`@import` resolution under git worktrees and submodules was still
silently broken — the QWEN.local.md file would load, but its imports
would resolve against the wrong root.

Extract the helper into `utils/projectRoot.ts`, with the unified
nullable return type. Rewire both call sites; `memoryImportProcessor`
preserves its previous fallback semantics at the call site
(`?? path.resolve(basePath)`). Adds 5 unit tests for the utility
(directory / file / null / deep / symlink) and 1 test for the
previously-unverified dedup guard in `memoryDiscovery.ts` (exercised
via `extensionContextFilePaths`).

Addresses inline + cross-file findings from wenshao on PR #4394.
2026-05-25 11:22:55 +08:00
易良
94da486e19
fix(weixin): send decryptable image payloads (#4464) 2026-05-25 11:16:33 +08:00
易良
8ef73599db
fix(weixin): allow Windows image paths inside workspace (#4465) 2026-05-25 11:16:00 +08:00
胡玮文
ab26a5ab72
fix(cli): resolve stale closure race in text buffer submit handler (#4470)
Replace useReducer with useRef + useState + synchronous dispatch so that
event handlers always read the latest buffer state. Previously, rapid
input via tmux send-keys could deliver characters and Enter in the same
event loop tick; the Enter handler read buffer.text from a stale render
closure (empty string) because useReducer's dispatch only enqueues
actions for the next render pass.

The fix runs the reducer synchronously at dispatch time, stores results
in a useRef for immediate reads, and calls setState to trigger
re-renders. The returned TextBuffer object exposes text, lines, and
cursor as getters reading from stateRef.current, so all consumers
(BaseTextInput, InputPrompt, vim hook) automatically get fresh values
without code changes.
2026-05-25 10:59:02 +08:00
胡玮文
84f408017a
feat(skills): add memory-leak-debug skill for heap snapshot diagnosis (#4468)
Some checks failed
Qwen Code CI / Classify PR (push) Has been cancelled
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Has been cancelled
E2E Tests / E2E Test (Linux) - sandbox:none (push) Has been cancelled
E2E Tests / E2E Test - macOS (push) Has been cancelled
Qwen Code CI / Lint (push) Has been cancelled
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Has been cancelled
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Has been cancelled
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Has been cancelled
Qwen Code CI / Post Coverage Comment (push) Has been cancelled
Qwen Code CI / CodeQL (push) Has been cancelled
Provides a step-by-step workflow for diagnosing memory leaks in the CLI
using Node.js heap snapshots and the chrome-devtools CLI memory tools.
Includes a helper script for tmux PID discovery and a worked example
from the react-reconciler PerformanceMeasure leak (dbdc94be9).
2026-05-24 02:46:16 +08:00
dykebo
4dc98484fd
feat(cli): do not append trailing space for directory completions (#4092) (#4288)
* feat(cli): do not append trailing space for directory completions (#4092)

## What

在 @路径补全和 /dir add 命令的目录补全中不再追加尾部空格。这样可以允许用户在补全目录后直接按 Tab 继续深入下一级子目录,无需先删除空格。

## Examples

- Input: `@src/com` + Tab → Output: `@src/components/` (no trailing space)

- Input: `/dir add ./pac` + Tab → Output: `/dir add ./packages/` (no trailing space)

- File completions still append a space (e.g., `@src/file.txt `)

## Changes

- Added `isDirectory` flag to `Suggestion` and `CommandCompletionItem` interfaces

- Updated `handleAutocomplete` to skip trailing space when `isDirectory === true`

- Modified `getDirPathCompletions` to return `CommandCompletionItem[]` with `isDirectory: true`

- Added test case for directory completion behavior

* fix(cli): append trailing / to directory completions for deeper navigation

* fix(cli): propagate isDirectory and fix JSDoc comment

## Comment 2: Fix JSDoc in SuggestionsDisplay

Removed "(ends with /)" from isDirectory description since it was factually incorrect.

## Comment 3: Add test for isDirectory propagation

- Added test suite in useSlashCompletion.test.ts to verify directory command structure

- Real filesystem testing is done in directoryCommand.test.tsx

* fix(cli): add comprehensive isDirectory propagation tests

Added getDirPathCompletions unit tests that verify:
- Directory suggestions include isDirectory: true
- Directory values end with / for continued navigation
- Prefix filtering preserves isDirectory flag
- Comma-separated path completion works correctly
- Deeply nested directories maintain isDirectory flag

This closes the testing gap identified in review comment 3.

* fix(cli): address wenshao feedback - lint rules, real test, cross-platform

Fixes 4 new review comments from wenshao:

- [Critical] Empty catch {} blocks: guarded with if (tempTestDir) + void err

- [Critical] useSlashCompletion.no-op test: replaced with real integration test that

  verifies isDirectory propagation through toSuggestion pass-through

- [Suggestion] Windows path separator: using path.sep instead of hardcoded /

  in both directoryCommand.tsx and related test assertions

* fix(cli): remove unused import and fix Windows path separator in tests

- Remove unused directoryCommand import in useSlashCompletion.test.ts (TS6133)
- Replace hardcoded / regex with path.sep-aware assertions in
  directoryCommand.test.tsx to fix Windows CI failures

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

* Apply suggestion from @wenshao

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

* Update packages/cli/src/ui/commands/directoryCommand.test.tsx

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

* Update packages/cli/src/ui/commands/directoryCommand.tsx

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

* Update packages/cli/src/ui/commands/directoryCommand.tsx

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

* fix(cli): normalize isDirectory to explicit boolean in toSuggestion

Normalize isDirectory from three-state (true/false/undefined) to explicit
boolean (true/false) to prevent latent bugs in future code that might
distinguish between false and undefined.

Fixes review comment: isDirectory normalization is inconsistent across
completion paths.

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

* Update packages/cli/src/ui/hooks/useSlashCompletion.ts

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

* chore: remove accidentally committed pr_body.md

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

* chore: add pr_body.md to .gitignore

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

* fix(cli): remove duplicate .slice and orphaned test code from directoryCommand.tsx

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

* fix(cli): only suppress trailing space for dir completions at end-of-line

When isDirectory is true, the trailing space was suppressed unconditionally,
even when the cursor is mid-line. This caused directory completions to merge
directly with following text (e.g. '@src/components/something').

Now only suppress the space when the cursor is at end-of-line, allowing
continued Tab navigation into subdirectories.

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

* docs(cli): document crawler path separator dependency for isDirectory check

The isDirectory detection uses p.endsWith('/') which depends on the
crawler in @qwen-code/qwen-code-core normalizing paths with posix '/'
(fdir.withPathSeparator('/') in crawler.ts). Add a comment to make this
implicit coupling explicit.

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

* test(cli): add mid-line directory completion test

Verify that directory completions append a trailing space when the cursor
is mid-line, preventing the completed path from merging with following text.

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

* Update packages/cli/src/ui/hooks/useCommandCompletion.test.ts

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

---------

Co-authored-by: 方磊 <fanglei@192.168.1.11>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-05-23 23:37:23 +08:00
qwen-code-ci-bot
394e2a3fa8
chore(release): v0.16.1 [skip ci]
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
2026-05-23 23:09:48 +08:00
jinye
94982c6a91
fix(build): clean stale outputs before tsc --build to prevent TS5055 (#4453)
* fix(build): clean stale outputs before tsc --build to prevent TS5055

Run `tsc --build --clean` before `tsc --build` in build_package.js so a
stale tsconfig.tsbuildinfo (e.g. after a version bump, branch switch, or
a prior `npm ci` prepare) cannot collide with composite project
references emitting back into packages/core/dist.

Closes #4447

* fix(build): scope clean step to current package only

Replace `tsc --build --clean` with direct `rmSync` of `dist` and
`tsconfig.tsbuildinfo`. `tsc -b --clean` walks project references, so
when scripts/build.js builds packages in dependency order, cleaning
from a downstream package (e.g. cli) would also wipe upstream outputs
(core, acp-bridge, channels) that were just built — a major perf
regression.

Spotted by Copilot review on #4453.
2026-05-23 23:06:31 +08:00
易良
a41afc465d
fix(release): move constants above entry point to avoid TDZ error (#4398)
MAX_UPLOAD_ATTEMPTS and INITIAL_BACKOFF_MS were declared after the
isMainModule() guard that calls main(). In ES modules, const bindings
are not initialized until the declaration is reached, so the runtime
threw "Cannot access 'MAX_UPLOAD_ATTEMPTS' before initialization"
during the Release workflow.
2026-05-23 22:21:33 +08:00
Shaojin Wen
e43852f769
fix(cli): gate mintty OSC 8 detection on TERM_PROGRAM_VERSION ≥ 3.3 (#4420) (#4451)
* fix(cli): gate mintty OSC 8 detection on TERM_PROGRAM_VERSION ≥ 3.3 (#4420)

mintty added OSC 8 in 3.1 and hardened it in 3.3. Older builds — still
bundled with some Git-for-Windows distros and developer environments like
Laragon — print the raw `\x1b]8;;url\x07` bytes as visible garbage instead
of silently ignoring them.

The previous unconditional `case 'mintty': return true` deviated from the
upstream `supports-hyperlinks` library (which rejects all of win32 outside
WT_SESSION) and let those old mintty users see escape bytes in their UI.

Gate on TERM_PROGRAM_VERSION (set by mintty since 2.7 in 2017 — a missing
value implies an ancient build, so we refuse rather than guess). Users on
mintty 3.1–3.2.x who know their build works can still opt in with
FORCE_HYPERLINK=1.

This fixes the OSC 8 component of #4420 (the "garbled UI on Windows + Git
Bash" report). The Ink 7 render interaction and terminalRedrawOptimizer
angles flagged in the same triage need separate Windows-environment
testing; `QWEN_CODE_LEGACY_ERASE_LINES=1` remains the documented escape
hatch for those.

* test(cli): assert FORCE_HYPERLINK=1 escape hatch works on gated mintty

Mirrors the Warp/Hyper pattern: after asserting auto-detection rejects an
older mintty build, set FORCE_HYPERLINK=1 and verify it opts back in. The
PR description for #4451 documents this contract for users on mintty
3.1–3.2 who know their build's OSC 8 implementation works; pinning it as
a test guards against a future refactor reordering the early-exit checks.

Addresses review feedback on #4451.
2026-05-23 22:19:23 +08:00
pomelo
b602a72e86
fix(cli): stabilize flaky sticky-todo remeasure test (#4416)
* fix(cli): stabilize flaky sticky-todo remeasure test (#4415)

Replace absolute mock.calls.length assertion with mockClear() +
not.toHaveBeenCalled() in the sticky todo status-only update test.

The previous assertion captured the total measureElement call count
after initial render, rerendered, and checked the count was unchanged.
This was flaky on CI (macOS runner) because React 19's Ink test
renderer can invoke useLayoutEffect a variable number of times during
mount (StrictMode double-invoke, multiple reconciliation passes),
making the absolute count unreliable across environments.

The new approach resets the mock after initial render and asserts no
new calls occur during rerender — clearly expressing the test intent
and eliminating environment-dependent flakiness.

* fix(cli): stabilize flaky sticky-todo remeasure test

Replace fragile measureElement call-count assertion with a behavioral
assertion on availableTerminalHeight stability, wrapped in act() to
flush useLayoutEffect timing.

The original test asserted that measureElement was not called after
rerender when only todo status changed (pending -> in_progress). This
was flaky because:

1. The absolute mock.calls.length count was environment-dependent
   (React 19 StrictMode double-invoke, variable reconciliation passes)
2. Even with mockClear(), the useLayoutEffect fires for legitimate
   reasons (buffer ref, btwItem) unrelated to sticky todo status,
   especially on Windows CI runners
3. The controlsHeight state (useState(0)) races with useLayoutEffect's
   first measurement — mainControlsRef.current may be null on initial
   render, causing controlsHeight to settle at different times

The fix:
- Assert on availableTerminalHeight (the behavioral outcome exposed via
  UIState context) rather than measureElement call count
- Wrap render + rerender in act() to ensure useLayoutEffect and
  setControlsHeight fully settle before capturing the baseline
- Consolidate duplicate react imports

* test(cli): address review feedback on sticky-todo remeasure test

- Narrow the `mockConfig.initialize` stub from `beforeEach` (which flipped
  `isConfigInitialized` for ~75 tests in the block) back to the single test
  that needs it. Other tests now exercise the real init gate as before.
- Strengthen the behavioral assertion: switch `measureElement`'s mocked
  return value between the settle phase and the status-only rerender, so
  any re-measurement triggered by the status change would change
  `controlsHeight` and break the equality assertion. Without this, the
  production same-value short-circuit on `setControlsHeight` made the
  assertion pass even when the optimization regressed.

The core layout-key contract (status-only changes return the same key) is
already directly covered by `todoSnapshot.test.ts` — this integration test
provides layered protection on top.

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

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-05-23 22:15:00 +08:00
易良
15247f4bce
chore(deps): update express from 4.21.2 to 5.2.1 (#4458)
express@4.21.2 was a stale peer dependency residual at the top-level
node_modules. It was originally pulled in to satisfy express-rate-limit's
peerDependency "express >= 4.11" when express@4 was still the latest tag.

Since express@5 is now latest and equally satisfies ">= 4.11", updating
removes ~1200 lines of unused express@4 dependency tree from the lockfile.

Closes #4457
2026-05-23 21:39:13 +08:00
胡玮文
61d91ad716
fix(build): tree-shake React reconciler dev build to prevent PerformanceMeasure leak (#4462)
The ink 6→7 upgrade (v0.15.11) pulled in react-reconciler 0.33, whose
development build calls performance.measure() on every component render.
Since NODE_ENV was never set to "production" in the esbuild define map,
the bundle shipped both dev and prod builds and selected dev at runtime,
causing an unbounded measureEntryBuffer leak (~45% of heap after moderate
use, confirmed via heap snapshots).

Set process.env.NODE_ENV to "production" at build time so esbuild
statically resolves the conditional require and tree-shakes the entire
15k-line dev build. Bundle shrinks by ~700 KB / 15,800 lines.
2026-05-23 21:00:32 +08:00
Shaojin Wen
0cb9ff0a23
fix: renormalize CRLF storage for install-qwen-standalone.bat (#4427) [skip ci]
The blob in HEAD stored raw CRLF bytes while .gitattributes declared
'text eol=crlf', which expects LF in the object database and CRLF on
checkout. The mismatch caused git status to permanently report the
file as modified on every working tree, with neither reset --hard nor
checkout fixing it.
2026-05-22 17:48:13 +08:00
jinye
fd75f77e19
feat(telemetry): Phase 4a — TTFT capture + GenAI semconv dual-emit (#3731) (#4417)
Some checks failed
Qwen Code CI / Classify PR (push) Has been cancelled
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Has been cancelled
E2E Tests / E2E Test (Linux) - sandbox:none (push) Has been cancelled
E2E Tests / E2E Test - macOS (push) Has been cancelled
Qwen Code CI / Lint (push) Has been cancelled
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Has been cancelled
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Has been cancelled
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Has been cancelled
Qwen Code CI / Post Coverage Comment (push) Has been cancelled
Qwen Code CI / CodeQL (push) Has been cancelled
2026-05-22 10:54:11 +08:00
zhangxy-zju
48b0a8bfce
fix(core): preserve tab-indented notebook formatting (#4373)
Some checks are pending
Qwen Code CI / Classify PR (push) Waiting to run
Qwen Code CI / Lint (push) Blocked by required conditions
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Blocked by required conditions
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* fix(core): preserve tab-indented notebook formatting

* test(core): cover mixed notebook indentation
2026-05-21 22:19:04 +08:00
易良
94f7cb5785
fix(vscode): skip redundant tsc build in prepackage to prevent TS5055 (#4401)
The prepackage script called `npm run build` (full tsc compilation of
all workspace packages) before bundling. This second tsc pass fails
with TS5055 in CI because:

1. `npm ci` already built everything via the `prepare` lifecycle
2. `release:version` bumps package versions, making tsbuildinfo stale
3. The redundant `npm run build` triggers tsc --build which attempts a
   full rebuild, but composite project references cause dist/*.d.ts to
   be resolved as input files → TS5055

The fix removes the redundant `npm run build` call. The CLI bundle is
produced by esbuild directly from TypeScript source files (entry:
packages/cli/index.ts), so compiled dist/ artifacts are not needed.

Root cause introduced by #4295 (composite project references).
2026-05-21 21:08:50 +08:00
Shaojin Wen
6b0e75816a
fix(core,cli): close tool_use↔tool_result invariant across all failure paths (#4176)
* fix(core): persist partial assistant turn when stream errors mid tool_use

Weak-network failures during an Anthropic-compatible stream (DeepSeek,
api.anthropic.com, etc.) can drop the SSE between a tool_use
`content_block_stop` and the terminal `message_stop`. The functionCall
chunk is already yielded at content_block_stop, so:

  - Turn.run records a ToolCallRequest event.
  - useGeminiStream's for-await exits and schedules the tool.
  - handleCompletedTools eventually fires submitQuery(..., ToolResult)
    and pushes a user[functionResponse] into history.
  - But processStreamResponse's history.push for the model turn never
    ran (the for-await threw first), so the matching tool_use is gone.

The next request body has `user → user[tool_result]` with no tool_use
in between, and the server rejects with HTTP 400:
"tool_use_id ... must have a corresponding tool_use block in the
previous message". Ctrl+Y can't recover because
stripOrphanedUserEntriesFromHistory only strips trailing user
entries — the lost tool_use is unrecoverable, and the session is
wedged.

Wrap the for-await loop in processStreamResponse with try/catch.
When the stream throws AND any functionCall chunk was already
yielded (hasToolCall=true), persist the partial assistant turn to
history before re-throwing. The eventual tool_result submission
then has a matching tool_use and the session can continue.

Plain-text partial turns (no functionCall yielded) are intentionally
NOT persisted: the Retry path pops the trailing user prompt and
re-issues it, so a stale partial-text model turn between them would
either bias the retry or surface as duplicate output.

* test(core): cover thinking+tool_use mid-stream throw in partial-history fix

Adds a third case to the partial-history persistence test suite for
reasoning-mode providers (DeepSeek thinking, Claude 4.6+ adaptive): when
the assistant turn streams a thinking block AND a tool_use before the
SSE drops, the partial push must keep the thinking part before the
functionCall so DeepSeek's `injectThinkingOnToolUseTurns` converter
pass sees an existing block on the replayed turn and does not pre-pend
a synthetic empty one (which would discard the model's original
reasoning text).

* fix(core,cli): close tool_use↔tool_result invariant at failure points

Extends the partial-history fix in fe35e3778 to cover the residual race
paths surfaced in PR #4176 review:

  - Race A: Ctrl+Y while in-flight tool hasn't finished. History is
    [user, model(tool_use)] — `stripOrphanedUserEntriesFromHistory` only
    pops trailing user entries, so the retry payload lands as a fresh
    user turn after the orphan tool_use and API rejects. Meanwhile the
    scheduler's `onAllToolCallsComplete` is single-shot and gated on
    `isResponding`, so the eventual tool_result is silently swallowed.
  - Race B: process crash / OOM / SIGKILL between the partial-tool_use
    push and the React scheduler's tool_result submission. On `--resume`
    the dangling model(tool_use) wedges the first API call.
  - Race C: external tooling / manual JSONL edits leaving the same
    dangling shape.

The fix has three pieces working together:

1. `repairOrphanedToolUseTurns(history)` in geminiChat.ts walks history
   left-to-right and synthesizes an `error`-typed functionResponse for
   every functionCall whose id is not echoed back in the next user
   turn. Appends to an existing user turn when present, otherwise
   inserts a new one. Returns the injected (callId, name) list.

2. `GeminiClient.repairOrphanedToolUseTurnsInHistory()` wraps the helper
   and is called from three points:
     - `startChat()` after loading the transcript (Race B/C, --resume).
     - `sendMessageStream` Retry branch after stripOrphans (Race A).
     - `sendMessageStream` UserQuery/Cron branch (defensive belt-and-
       suspenders for anything that slipped past 1 and 2).

3. `handleCompletedTools` in useGeminiStream.ts dedupes against
   chat.history before submitting tool_results — if a synthetic
   functionResponse for the same callId is already present (planted by
   the repair pass), the in-flight scheduler's late result is dropped
   and the call is `markToolsAsSubmitted` so the UI advances. Same
   trade-off upstream Claude Code's `StreamingToolExecutor.discard()`
   makes — late real results are dropped on the wire after synthesis,
   the model sees the synthetic error and can retry the tool if it
   still wants the result.

Together with the partial-history push from fe35e3778, every tool_use
that ever streamed to the consumer is guaranteed to have a matching
tool_result on the wire — regardless of whether the stream errored,
the user retried mid-flight, the process crashed, or the session is
later resumed. This is the qwen-code analogue of upstream Claude Code's
`yieldMissingToolResultBlocks` (query.ts:123-149), but split across
the core/cli boundary because the React tool scheduler runs out-of-band
from the stream loop (so the synthesis path can't atomically discard
in-flight tools the way upstream's StreamingToolExecutor can; the
history-dedup at handleCompletedTools fills that gap instead).

Tests:
  - 8 new repair-helper tests in geminiChat.test.ts cover Race A,
    Race B, partial coverage of parallel tool_use, idempotence on
    already-paired history, no-op on tool-free history, caller-
    supplied reason text, multiple non-adjacent dangling rounds, and
    routes through the GeminiChat instance-method wrapper.
  - client.test.ts mocks updated for the new GeminiChat method.
  - All 88 geminiChat tests pass; 131 client tests pass; 91
    useGeminiStream tests pass.

* fix(core,cli): close tool_use↔tool_result invariant at failure points

Post-review audit of 3de3241a2 surfaced two real races the earlier fix
did not actually close:

  - Race A (the one yiliang114 originally flagged): handleCompletedTools
    is gated on `isResponding` (useGeminiStream:1971) BEFORE the dedup
    branch ran, so when the user Ctrl+Y'd mid-tool, the dedup never
    fired and the in-flight tool stayed permanently stuck in
    `completed-but-not-submitted` (the scheduler's
    `allToolCallsCompleteHandler` is single-shot). The dedup branch was
    structurally unreachable on the very race it was meant to defend
    against.

  - Race in the Retry path: the repair pass in
    `client.sendMessageStream` Retry branch ran BEFORE the chat-internal
    `push(userContent)`, so a Retry of a previous ToolResult submission
    (lastPrompt is a functionResponse part array) raced its own real
    `functionResponse` against the synthesized error one. The synthesis
    was winning and pre-empting the real result, producing two
    `functionResponse` entries with the same callId on the wire.

This commit relocates both pieces to where the contract actually holds:

1. Move the dedup branch in `handleCompletedTools` to BEFORE the
   `isResponding` early-return so `markToolsAsSubmitted` always runs
   for callIds already paired in history, unblocking the UI/scheduler
   even when a new stream is in flight. The submission-side `geminiTools
   = completedAndReadyToSubmitTools.filter(... && !inHistory ...)` then
   covers the no-double-submit case in one expression.

2. Move the repair call from `client.sendMessageStream` Retry branch
   into `chat.sendMessageStream` immediately AFTER the user-supplied
   turn is pushed. The user's own tool_result (when the Retry payload
   carries one) gets the first chance to close the pair before the
   synthesizer sees it as dangling. `client.startChat()` keeps a
   belt-and-suspenders pass at session-load time so any pre-send code
   reading `chat.history` sees a well-formed shape.

Adds three integration tests covering the new wiring:

  - geminiChat.test.ts: chat.sendMessageStream synthesizes a
    functionResponse when history carries a dangling tool_use AND the
    user-supplied content doesn't already close it (Race B/C plus
    Race A from the chat side).
  - geminiChat.test.ts: chat.sendMessageStream does NOT synthesize
    when the user-supplied content IS a matching functionResponse
    (the Retry-of-ToolResult race the previous wiring tripped on).
  - useGeminiStream.test.tsx: handleCompletedTools dedups a late real
    result whose callId already has a functionResponse in chat.history
    (Race A end-to-end: markToolsAsSubmitted fires but no
    submitQuery is dispatched).

Test summary: 90/90 geminiChat, 131/131 client, 92/92 useGeminiStream;
all 8186 core tests pass; tsc clean.

Known limitation logged for follow-up: a Retry of ToolResult whose
intervening stream itself partial-pushed a SECOND tool_use produces a
trailing user turn with both the stale re-pushed `fr_A` and the
synthesized `fr_B` for the second call — `fr_A` retry lands as an
orphan because `fc_A` was already paired earlier in history. Triggered
only when partial-push fires on the second stream AND user retries the
tool_result rather than the user prompt; extremely low frequency in
practice. Cleanest fix is in the retryLastPrompt layer (don't re-push
an already-paired tool_result), which is out of scope for this PR.

* fix(core,cli): close tool_use↔tool_result invariant at failure points

Review surfaced four issues on top of b2fed61cd. All addressed here:

  - [P0] Synthetic functionResponse was appended to the trailing user
    turn's parts; on a Ctrl+Y race the shape became `user[text, fr]` and
    Anthropic-compatible backends still 400 because tool_result blocks
    must come FIRST in a user message (Claude Code does the same hoist
    in utils/messages.ts:hoistToolResults). Repair now slots synthetic
    fr in before the first non-functionResponse part, after any
    pre-existing real fr parts (which preserves real-tool-result order
    on a parallel-partial-submit). Tests asserting the part order added.

  - [P0] The dedup test fixture in useGeminiStream.test.tsx forced an
    incompatible object through a direct cast to TrackedCompletedToolCall;
    `tsc` rejected it with TS2352. Fixture restructured to match the
    type's actual shape (matches the existing `Mocked` patterns at
    L597-606 / L2396-2401) so the changed-file typecheck stays green.

  - [P1] On stream error, `recordAssistantTurn` ran unconditionally
    (gated only on parts being non-empty), so partial text-only turns
    we intentionally do NOT push into in-memory history were still
    written to the chat-recording JSONL — leaving --resume's
    transcript-load path to re-inject a model turn the in-session run
    discarded. Gate now matches the `history.push` decision: record
    iff we will persist (success path OR stream-error-with-tool_use).

  - [P2] Added client-level tests for the startChat repair wiring:
    extraHistory ending in `model[functionCall]` triggers synthetic fr
    injection on init; happy-path resume is a no-op. Defends against a
    future reorder/removal of the call in startChat() regressing the
    --resume recovery path.

All 91 geminiChat, 133 client, 92 useGeminiStream tests pass. Full
core suite (8186 tests) green. Changed-file tsc clean except for the
pre-existing `recordCompletedToolCall` typing gap that exists on main.

* fix(core): roll back partial assistant push on retryable mid-stream errors

Regression surfaced by @yiliang114 on PR #4176: a stream attempt that
yields a `functionCall` and then throws a retryable error (e.g.
`StreamContentError` with a 429 payload, or `InvalidStreamError`)
triggers the partial-assistant-turn push in `processStreamResponse`,
but the outer `sendMessageStream` retry loop catches the error and
issues a fresh attempt. The failed-attempt `model[functionCall]` was
left in history, and the successful retry's response landed as a
SECOND consecutive `model` entry — invalid user/model alternation
AND the failed-attempt `tool_use` is orphan on the wire (no matching
tool_result). The very wedge this PR is meant to escape.

Track the index of the pushed partial in a new
`pendingPartialAssistantTurnIndex` field, reset it on every
`sendMessageStream` entry, and pop it before each retry-and-continue
path in the catch block (rate-limit, reactive-compression, transient
stream anomaly, content-validation retry). Paths that `break`
(unretryable) keep the partial — the caller will see it as part of
the error surface and the existing repair / dedup machinery in
`useGeminiStream.handleCompletedTools` handles it correctly.

The pop is defensive (index-bounds + role check) so a hypothetical
intervening `setHistory` / `truncateHistory` can't cause an out-of-
bounds splice.

Adds a regression test in `geminiChat.test.ts` that reproduces
@yiliang114's exact shape: yield `functionCall`, throw
`StreamContentError(429)`, second attempt yields `Success after retry`
plain text + STOP. Asserts the final history is exactly
`[user, model(success text)]` — no leading failed-attempt model turn,
no orphan `functionCall` anywhere.

All 92 geminiChat, 133 client, 92 useGeminiStream tests pass; full
core suite green; tsc clean.

* fix(core): clear pendingPartialAssistantTurnIndex on history replacement

Audit follow-up to db344a403. The retry-rollback marker
`pendingPartialAssistantTurnIndex` captures an absolute index into
`this.history`, but `setHistory` / `clearHistory` / `truncateHistory`
wipe or shift the underlying array without touching the marker. Two
paths where this can bite:

  - Reactive compression on contextOverflow calls `this.setHistory(
    newHistory)` inside `tryCompress`. If a partial push has happened
    earlier (extreme edge case — would require a partial push followed
    by a context-overflow on retry), the marker's stale index could
    coincide with a model entry in the compressed history, and
    `popPartialIfPushed` would splice the wrong turn.

  - `/clear` / `--resume` / programmatic `setHistory` / external
    `chat.truncateHistory()` (Session.ts:244) wipe the basis the marker
    was captured against, leaving the next send's
    `popPartialIfPushed` with a meaningless index.

The fix is mechanical: each history-replacement method clears the
marker. The marker is per-send and ephemeral, so losing it across a
history replacement is safe (the next send either has no partial yet,
or will push a fresh one with a fresh index).

`popPartialIfPushed` already bounds-checks + role-checks before
splicing, so this is defense-in-depth — the bug above requires the
defensive check to ALSO line up. But cheap to harden and the comment
documents the invariant for future readers. 235 core/geminiChat +
core/client tests still pass.

* fix(core,cli): close mimo-v2.5-pro review gaps on partial-tool_use repair

Four review items from the mimo-v2.5-pro pass on #4176:

S1 — `addHistory` and `stripThoughtsFromHistory` did not reset
`pendingPartialAssistantTurnIndex`. `setHistory`/`truncateHistory`/
`clearHistory` already do; consistency with the defensive pattern
matters because `stripThoughtsFromHistory` filter+map produces a new
array that makes the marker stale, and any future addHistory variant
that splices into the middle could leave `popPartialIfPushed` looking
at a wrong index. Comments updated to record the actual semantics
(plain push() is safe; a future splicing variant would not be).

S2 — When the history-based dedup in `useGeminiStream.handleCompletedTools`
filters a tool out of `geminiTools` because its callId is already paired
in chat.history, `recordCompletedToolCall` was never called for it.
That silently skips the `toolCallCount` increment and the
`skillsModifiedInSession` flip — the latter gates the skills-reload
prompt at end-of-turn, so a deduped `write_file` under a project
SKILLS path would never trigger a reload. Now records inline with the
dedup mark, filtered to non-client-initiated to match the original
geminiTools loop's shape.

S3 — Existing Race A dedup test never set `isResponding=true`, so a
future refactor that moves the dedup block below the `if (isResponding)
return` guard would pass every test while silently leaving deduped
tools stuck in `completed-but-not-submitted` (the scheduler's
`onAllToolCallsComplete` is single-shot per batch). New test holds a
stream open via `submitQuery` to pin `isResponding=true`, then triggers
`onComplete` and asserts `markToolsAsSubmitted` still fires. Verified
the test fails when the dedup is moved below the guard. Existing
Race A test also gained a `recordCompletedToolCall` assertion locking
in the S2 fix.

N1 — Hoisting comment in `repairOrphanedToolUseTurns` documented the
why (mirror upstream `hoistToolResults`) but not the consequence of
removal. Added "CONSEQUENCE OF REMOVAL" paragraph spelling out the
exact 400 a naive `next.parts = [...existing, ...syntheticParts]`
re-introduces, so the constraint is unmissable for future maintainers.

Verified: 94/94 useGeminiStream, 102/102 geminiChat, 133/133 client
tests pass. tsc clean both packages, eslint + prettier clean on all
three changed files. S3 regression-injection check confirmed the new
test catches the dedup-below-guard regression.

* fix(core): defer chat-recording flush until partial-turn rollback decision

Yiliang114's PR #4176 follow-up: `popPartialIfPushed()` rolls a
failed-attempt's `model[functionCall]` out of in-memory `this.history`
before retrying, but the same partial turn was already appended to the
chat-recording JSONL by `recordAssistantTurn()`. After a successful
retry, live history was clean while the durable transcript still
carried two assistant entries — the failed attempt and the success.
`--resume` then rehydrated the failed `model[functionCall]` and the
resumed model context picked up a tool_use the live session correctly
discarded.

Fix: the recording call on the stream-error + hasToolCall path is now
deferred. processStreamResponse stashes the would-be record on a new
`pendingPartialAssistantRecord` field instead of immediately appending.
The retry loop's `popPartialIfPushed` clears the stash alongside the
in-memory splice — no flush, no leak. Once the retry loop has settled
(success-break: stash already null; unretryable-break: partial
survived), a flush hook right after the for-loop appends the stashed
record to JSONL so the transcript matches whatever lives in
`this.history`. The success path (streamError === null) still records
immediately as before — only the at-risk-of-rollback path is deferred.

Lifecycle is paired one-to-one with `pendingPartialAssistantTurnIndex`:
set together, popped together, flushed together, and reset in every
history-replacement method (sendMessageStream entry, setHistory,
clearHistory, truncateHistory, addHistory, stripThoughtsFromHistory)
so an exotic path can't leak a stale stash into a future send's flush.

Two regression tests in geminiChat.test.ts:

  1. "rolls back the chat-recording entry too when the retry succeeds"
     — failing-then-succeeding stream sequence asserts
     `recordAssistantTurn` is called exactly once with the success
     text, no functionCall part anywhere in the recorded message.
     Verified the test fails (mock called twice) when the deferral
     branch is bypassed.

  2. "flushes the chat-recording entry on the unretryable break path"
     — synthetic non-retryable error after a tool_use chunk asserts
     the partial IS recorded so the JSONL stays aligned with the
     in-memory partial that survives. Verified the test fails
     (mock called zero times) when the post-loop flush is removed.

Tests: 104/104 geminiChat (+2 new), 133/133 client, 94/94
useGeminiStream. tsc + eslint + prettier clean.

* fix(core,cli): close 4 deepseek-v4-pro review threads on PR #4176

Four follow-ups from the deepseek-v4-pro pass on commit 2dbfc4e3b:

[A] useGeminiStream.ts — dedup recordCompletedToolCall now skips
cancelled tools. `dedupedTools` includes anything in a terminal state
(success | error | cancelled), but cancelled means the tool never
actually produced model-visible output. Counting it via
`recordCompletedToolCall` would inflate `toolCallCount` and could flip
`skillsModifiedInSession` for a never-executed skill-write. The
markToolsAsSubmitted call still fires so the scheduler unblocks.
Mirrors the rationale of the existing `allToolsCancelled` branch which
surfaces non-deduped cancellations via addHistory + reportCancelled
rather than the completed-call metric.

[B] client.ts — JSDoc on `repairOrphanedToolUseTurnsInHistory` claimed
three call points (startChat, Retry submit path, defensive
UserQuery/Cron pass) but in practice this method is only called once
from `startChat()`. The other two coverage points live one layer down
inside `GeminiChat.sendMessageStream` and call the standalone
`repairOrphanedToolUseTurns(history)` function directly without
routing through this wrapper. Updated to reflect the actual coupling.

[C] geminiChat.ts — `addHistory` now warns via debugLogger if it
clears a non-null partial-push marker. Today's callers
(useGeminiStream cancelled-tool synthesis, ACP session injects,
shellCommandProcessor) only run between sends, so the marker is null
on entry. If a future code path calls addHistory between the partial
push and the retry attempt, the silent clear would strand the
partial: popPartialIfPushed would no-op, the failed model[functionCall]
would survive into the retry, and a successful retry's response would
land as a SECOND consecutive model turn — the wedge this whole
subsystem exists to prevent. The warn surfaces the offending caller in
the log instead of forcing a blind trace through the marker lifecycle.

[D] geminiChat.ts — extracted `clearPendingPartialState()` helper
covering all 7 sites that need to reset both
`pendingPartialAssistantTurnIndex` and `pendingPartialAssistantRecord`
in lockstep (sendMessageStream entry, popPartialIfPushed, the
post-loop flush hook, clearHistory, addHistory, setHistory,
truncateHistory, stripThoughtsFromHistory). The two fields ARE always
paired by lifecycle (set together on stream-error stash, popped
together on retry, flushed together at the rethrow site), so any
single-field reset would be a bug. The helper makes the lockstep
invariant explicit and means a future history-mutating method can't
forget to clear one field.

Regression test: useGeminiStream.test.tsx adds
"skips recordCompletedToolCall for deduped CANCELLED tools" — sets up
a deduped cancelled tool with callId paired in history, asserts
markToolsAsSubmitted IS called (scheduler unblocks) but
recordCompletedToolCall is NOT called. Verified the test fails when
the cancelled-filter line is removed (regression-injection check).

Tests: 237/237 core, 95/95 useGeminiStream (+1 new). tsc + eslint +
prettier clean. Existing CI failure on Test (ubuntu-latest, Node 22.x)
is an unrelated timing flake in promptHookRunner.test.ts
("expected 49 to be greater than or equal to 50") — macOS and Windows
both pass.

* fix(core): close 3 review threads on partial-tool_use repair (PR #4176)

Three follow-ups from the qwen-latest-series-invite-beta-v28 pass on
commit fd12639c9:

[E] CORRECTNESS — `repairOrphanedToolUseTurns` now scans EVERY
consecutive `user` turn after a `model[functionCall]` (not just
`history[i+1]`) when building the matched-id set. The shape
`model[fc], user[text], user[fr_real]` arises when a user aborts a
long-running tool, types a follow-up text turn, and the React
scheduler's late `submitQuery` appends the real `tool_result` as a
SEPARATE user entry. Previously the repair saw only the immediate
follow-up text turn, found no matching fr, and synthesized a duplicate
`error` `functionResponse` for a callId that already had a real result
two turns down. The downstream dedup in
`useGeminiStream.handleCompletedTools` then dropped the REAL result on
the next submission (its callId was now "already in history" thanks to
the synthetic), and the model only ever saw the error placeholder —
silently swapping a real tool success for an error.

Two regression tests in `geminiChat.test.ts`:
  - "does NOT synthesize when the real functionResponse lives in a
    non-adjacent later user turn" — proves the forward-scan finds the
    real fr at history[i+2]+ and skips synthesis.
  - "still synthesizes for a partial mismatch when forward scan misses
    one callId" — counterpart that confirms parallel tool_use shapes
    where only some callIds have real fr's still get synthetic ones
    hoisted onto the immediate next user turn.

Verified via regression-injection (revert to scanning only
`history[i+1]`): the non-adjacent test fails with `injected =
[{callId:'call_nonadjacent_real',...}]` — proves the test catches the
bug, not just verifies a no-op.

[F] OBSERVABILITY — added a `[PARTIAL_PUSH]` `debugLogger.warn` at the
partial-turn push site in `processStreamResponse`. The repair lifecycle
already had push/pop/repair logging at the dedup and synthesis ends
(`[REPAIR] Dropping ...`, `[REPAIR] Synthesized ...`), but the original
PUSH event — root cause of every downstream recovery — was unlogged.
At 3 AM investigating a stale-partial wedge, the trace now anchors at
the exact moment the partial was created with pendingIndex, callIds,
and the originating error message.

[G] INVARIANT — `stripOrphanedUserEntriesFromHistory` now calls
`clearPendingPartialState()` after popping. Today this is safe even
without the reset (only trailing `user` entries are popped, which
can't shift the index of an earlier `model` partial), but every other
history-mutation method in the class — `clearHistory`, `addHistory`,
`setHistory`, `truncateHistory`, `stripThoughtsFromHistory` — now
clears the partial-push state in lockstep. Omitting it here would be
a silent exception to the uniform invariant the helper extraction
established. A future caller invoking this between the deferred JSONL
flush and the next `sendMessageStream` would otherwise leave a stale
marker that happens to line up with whatever model entry is at that
index in the meanwhile.

Tests: 239/239 core (+2 new), 95/95 useGeminiStream. tsc + eslint +
prettier clean. Latest CI run: all three platforms (Linux/macOS/
Windows) green.

* fix(core): close 6 review threads on partial-tool_use repair (PR #4176)

Six follow-ups from the qwen-latest-series-invite-beta-v28 + gpt-5.5
passes on commit 2880de577:

[A] CORRECTNESS — `repairOrphanedToolUseTurns` now HOISTS a real
`functionResponse` from a non-adjacent later user turn into the
IMMEDIATE next user turn (placed before non-fr parts), in addition to
the forward-scan dedup added in 2880de577. The shape `model[fc],
user[text], user[fr_real]` arises when a user aborts a long-running
tool, types a follow-up text turn, and the React scheduler's late
`submitQuery` appends the real `tool_result` as a SEPARATE user entry.
Forward-scan alone correctly skipped the synthesis duplicate, but the
wire layout still serialized `model[tool_use] → user[text] →
user[tool_result]`, which Anthropic-compatible backends reject with
"tool_use_id ... must have a corresponding tool_use block in the
previous message". The hoist physically moves the real fr part out of
its original turn into history[i+1], drops the source turn if it
becomes empty, and reuses the existing front-of-user-turn
insertion logic so caller-supplied fr ordering stays preserved.
Hoisted ids are NOT added to `injected` — the real fr is already in
history (just relocated), so the React scheduler's history-based
dedup handles them naturally without an extra entry that would
double-trigger the dedup-drop log.

Three new tests in `geminiChat.test.ts`:
  - "hoists the real functionResponse from a non-adjacent later user
    turn into the adjacent one" — pure relocate case (source turn had
    only the fr → removed).
  - "synthesizes missing fr AND hoists real fr in a parallel tool_use
    mismatch" — parallel `model[fc_a, fc_b]` with real fr_a in a
    non-adjacent turn: cid_b synthesized, cid_a hoisted, source turn
    removed.
  - "hoists real fr but preserves the source user turn when it
    carries other content" — pins the empty-turn cleanup so it only
    drops turns whose parts list goes to zero (mixed text + fr source
    keeps its text after the fr is extracted).

[B] CRITICAL — moved the deferred chat-recording flush from BEFORE the
max-tokens escalation block into the outer `finally`. The escalated
`makeApiCallAndProcessStream → processStreamResponse` can set a NEW
`pendingPartialAssistantRecord` if it errors mid-tool_use, and that
throw escapes through the for-await without touching the (now-passed)
retry-loop catch. Before this fix the new record was never appended
to JSONL: live history retained the partial `model[functionCall]`
that the escalated processStreamResponse pushed, but the durable
transcript silently dropped it; on `--resume` the transcript-load
path didn't see the partial, `repairOrphanedToolUseTurnsInHistory`
found nothing to repair, and the React scheduler's late real result
became a permanent orphan — reproducing the exact wedge this PR
prevents. Putting the flush in `finally` covers all throw paths
(escalation, post-retry-loop `throw lastError`, consumer `.return()`)
and the normal completion path with one statement, while keeping the
"marker and stash cleared in lockstep" invariant.

One new regression test: "flushes the JSONL record when escalated
stream throws mid-tool_use" — pins the flush behavior under the
specific shape (initial MAX_TOKENS success → escalation cuts mid-
tool_use → throw). Asserts both that the partial survives in
`this.history` and that `recordAssistantTurn` got called with the
partial functionCall id.

[C] DOC — updated `repairOrphanedToolUseTurns` doc comment to spell
out both fix-ups (synthesize + hoist), the wire-format consequence of
removal ("tool_use_id ... must have a corresponding tool_use block"),
and the rule that hoisted ids are not in the returned `injected` list.
The previous "immediately following user turn" wording contradicted
the forward-scan implementation; the new wording matches the actual
behavior across both fix-up paths.

[D] OBSERVABILITY — inline `repairOrphanedToolUseTurns` call in
`sendMessageStream` now logs `[REPAIR] sendMessageStream inline pass
synthesized N functionResponse(s) ...` when synthesis fires. The
startChat() path already logs via `repairOrphanedToolUseTurnsInHistory`
(`[REPAIR] Synthesized ...`), and `useGeminiStream.handleCompletedTools`
logs `[REPAIR] Dropping ...` on dedup. Without a tagged log at the
inline call site, an investigator looking at a dedup-drop had no way
to tell whether the synthetic was planted at session-load or at the
per-send pass.

[E] OBSERVABILITY — `popPartialIfPushed` now logs `[PARTIAL_POP]
Splice skipped` when the marker is set but `history.length <= idx` or
`history[idx]?.role !== 'model'`. This can't happen today (every
history-mutation method clears the marker in lockstep), but the warn
makes any future regression observable rather than silent: a
mutation path that forgets to clear would otherwise leave a stale
partial in history with no diagnostic trace.

[F] CLARITY — added a comment at the post-`tryCompress`
`popPartialIfPushed()` call explaining the intentional defense-in-
depth no-op. `tryCompress()` succeeds → `setHistory()` →
`clearPendingPartialState()`, so the marker is null by the time the
pop runs. The call is kept (not removed) so a future refactor that
switches `tryCompress` to in-place mutation would still drop the
stale partial before `requestContents` is rebuilt.

Tests: 336/336 (108 in geminiChat.test.ts + 4 new = +4; 95
useGeminiStream + 133 client unchanged). tsc + eslint + prettier
clean on changed files.

* fix(core,cli): close 10 review threads on partial-tool_use repair (PR #4176)

Five Critical + four Suggestion threads from the gpt-5.5 +
qwen-latest-series-invite-beta-v34 passes on commit ce68749b5, plus
one N/A documented in code/test comments where the requested test
target is provably unreachable.

[A] CORRECTNESS — `repairOrphanedToolUseTurns` now collects EVERY
location for each `functionResponse` id, not just the first.
Previously the `matched` Map stored only the first occurrence per
callId, so when the same id was echoed back more than once across
the consecutive user turns (shape `model[fc id=cid], user[text],
user[fr cid], user[fr cid]` — possible when the React scheduler
retries the late submitQuery after the orphan repair already planted
one, or two parallel late-submit paths land), hoisting only the
first left a duplicate behind. The wire payload then serialized
`model[tool_use] -> user[tool_result] -> user[tool_result]`, the
backend rejected the trailing block as an orphan, and the session
stayed wedged for the same 400 class this whole repair pass exists
to escape. Fix: track all locations per id; pick the first as the
canonical survivor (hoisted into history[i+1] when non-adjacent, or
left in place when adjacent), and drop EVERY duplicate. New tests:
"drops duplicate functionResponse entries for the same callId across
user turns" (canonical non-adjacent + duplicate further down →
3-entry result), "drops duplicate fr even when the canonical copy is
already in the adjacent turn" (no hoist needed but duplicate cleanup
must still fire). gpt-5.5 review thread on PR #4176.

[B] CORRECTNESS — Max-tokens recovery catch (`recoveryError` block)
now pops the partial `model` turn FIRST, then the
OUTPUT_RECOVERY_MESSAGE user turn. Before the fix, when a recovery
stream errored AFTER yielding a `functionCall` chunk,
`processStreamResponse` pushed a partial `model` turn into history
before re-throwing — so by the time the catch ran, the trailing
entries were `[..., user(OUTPUT_RECOVERY_MESSAGE), model(partial fc)]`
and the naive `if last is user, pop` check no-op'd, stranding the
control prompt as a real user turn. Two consequences: the recovery
prompt's instructions polluted later turns, and the inline repair
on the next sendMessageStream synthesized an `error` `functionResponse`
for the dangling `functionCall` — which the dedup then dropped when
the React scheduler's REAL tool result arrived, so the model saw an
"execution result was not recorded" error for a tool that actually
succeeded. Fix: pop the partial model turn, clear the partial-push
markers, then pop the recovery user turn. New regression test:
"should pop both the partial model turn AND the recovery user
message when recovery throws after a functionCall".
qwen-latest-series-invite-beta-v34 review thread.

[C] PERFORMANCE — added `getHistoryFunctionResponseIds(): Set<string>`
to `GeminiChat` and a wrapper on `GeminiClient`. The dedup pass in
`useGeminiStream.handleCompletedTools` previously called
`geminiClient.getHistory()`, which returns `structuredClone(this.history)`
— a recursive deep clone of every part including large tool outputs.
On long sessions (200+ entries) running on every tool-completion
batch, this caused multi-millisecond stalls on the React UI thread
during streaming. The new accessor walks `this.history` in place
and only collects the id strings the dedup actually needs.
`useGeminiStream` prefers it; falls back to the cloning getHistory()
for older mocks that don't expose it (tests stay green).
qwen-latest-series-invite-beta-v34 review thread.

[D] ROBUSTNESS — wrapped the `finally`-block JSONL flush in
try/catch. Recording-service errors (disk full, write permission,
serialization failure) MUST NOT propagate out of the generator's
`finally` — that would mask the real send outcome (success or
original throw) with a JSONL-write error the caller can't usefully
act on. On failure the partial is dropped from JSONL but stays
durable in `this.history`, so live behavior is unaffected; eventual
consistency is restored on the next successful flush.
qwen-latest-series-invite-beta-v34 review thread.

[E] TEST COVERAGE — added an InvalidStreamError transient-retry
rollback test exercising `popPartialIfPushed()` on the
NO_FINISH_REASON / NO_RESPONSE_TEXT path (separate budget from the
already-tested rate-limit branch). Stream yields a functionCall
then throws InvalidStreamError; retry succeeds; assert no orphan
functionCall remains in history. qwen-latest-series-invite-beta-v34
review thread.

[F] N/A WITH RATIONALE — verified that the InvalidStreamError
content-retry branch (geminiChat.ts ~line 1399) is unreachable for
that error class: `isTransientStreamError` and `isContentError` are
the same predicate (`error instanceof InvalidStreamError`), so the
transient branch above always either continues or breaks before
control reaches the content branch. The `popPartialIfPushed()` call
is preserved as defense-in-depth for a future error class that
should diverge the predicates. Documented at the call site (code
comment) and at the would-be test site (test-block comment) so
future readers see the unreachability analysis instead of repeating
the review-comment ask.

[G] DRIFT GUARD — `recordAssistantTurn` and `this.history.push`
gates now share the single `willPersistToHistory` binding instead of
re-deriving the same `hasToolCall && (thoughtContentPart || ...)`
expression. Drift risk: tightening one without the other would
silently desync the JSONL transcript from in-memory history.
qwen-latest-series-invite-beta-v34 review thread.

[H] DOUBLE-DISPATCH — `clientTools` filter in
`useGeminiStream.handleCompletedTools` now excludes callIds whose
fr is already in history. Previously a deduped client-initiated
tool would have `markToolsAsSubmitted` called twice (once in the
dedup block, once in the clientTools block), triggering an extra
React render cycle. qwen-latest-series-invite-beta-v34 review thread.

[I] TEST COVERAGE — added six tests under "partial-push marker
invariants on history mutation" pinning that each of the six
mutation methods (clearHistory, addHistory, setHistory,
truncateHistory, stripThoughtsFromHistory,
stripOrphanedUserEntriesFromHistory) calls clearPendingPartialState()
in lockstep. Markers planted directly via private-field assignment
(the lifecycle would otherwise clear them in finally before the
test could observe them). Future refactors that drop a
clearPendingPartialState() call from one site are now caught at
test time rather than as a silent stale-partial wedge in production.
qwen-latest-series-invite-beta-v34 review thread.

[J] TEST COVERAGE — added a mixed-batch dedup test in
useGeminiStream.test.tsx: scheduler completes two tools in the same
batch, one whose callId already has an fr in history (deduped), one
fresh. Asserts (a) markToolsAsSubmitted called with both, (b)
recordCompletedToolCall fires exactly once per tool (deduped via
the dedup-loop, fresh via the geminiTools loop — no double-record
on the deduped id), (c) sendMessageStream IS called for the fresh
tool's real result. The `!historyCallIdsWithResponse.has(callId)`
filter on `geminiTools` is the only guard against double-counting
telemetry; existing tests supplied only deduped tools.
qwen-latest-series-invite-beta-v34 review thread.

Tests: 347/347 (118 in geminiChat.test.ts, 96 in
useGeminiStream.test.tsx, 133 in client.test.ts; net +8 new tests).
tsc + eslint + prettier clean on changed files.

* fix(core,cli): close 3 review threads on partial-tool_use repair (PR #4176)

Three Suggestion threads from the qwen-latest-series-invite-beta-v34
pass on commit c8fa3143f.

[A] DIAGNOSTICS — `repairOrphanedToolUseTurns` return type now
carries a `droppedDuplicates: Array<{ callId; name }>` field
alongside `injected`. Previously a duplicate-only repair (no
synthesis, no hoist — the main scenario the prior commit added)
returned `injected.length === 0` and both call sites
(`repairOrphanedToolUseTurnsInHistory` in client.ts,
`sendMessageStream` inline in geminiChat.ts) only logged on
synthesis, leaving zero diagnostic trail. If a future callId
collision bug caused the wrong `functionResponse` to be dropped,
there was no breadcrumb pointing back to the repair function. Both
call sites now emit a `[REPAIR] Dropped N duplicate functionResponse(s)`
warn line tagged with their origin (session-load vs inline per-send)
so investigators can anchor to the exact pass.

[B] DEFENSIVE INVARIANT — max-tokens recovery catch now uses an
index-checked pop instead of a positional `history.pop()`. The
semantically equivalent `popPartialIfPushed` uses
`splice(idx, 1)` with an explicit bounds/role check and a
`debugLogger.warn` on mismatch; the recovery catch was the only
rollback site that pop'd whatever model entry happened to be last,
with zero diagnostic output. Today the invariant holds (nothing
mutates `this.history` between `processStreamResponse`'s push and
the for-await catch), but a future change that inserts a mutation
in that window — compression side-effect, abort-signal handler,
telemetry hook — would silently pop the wrong entry while
`clearPendingPartialState()` cleared markers for the actual
partial, leaving it permanently stranded. The new
`[RECOVERY_POP] Marker/last-index mismatch` warn surfaces any
future violation immediately.

[C] TEST COVERAGE — added 6 unit tests for
`GeminiChat.getHistoryFunctionResponseIds()` (empty, mixed
user/model, model[fc] ignored, duplicate collapse to Set,
malformed parts, no aliasing of internal state). Also wired the
mixed-batch dedup test in `useGeminiStream.test.tsx` through the
fast-path accessor (mock returns the dedup Set directly) and
asserted that `getHistoryFunctionResponseIds` IS called and the
legacy cloning `getHistory()` is NOT. The earlier version of that
test only mocked `getHistory()`, so the optimization the previous
commit added was never actually exercised — a regression that
dropped the fast-path branch from the dispatcher would re-route
every batch onto the slow `structuredClone` path with no test
failure. The new assertion makes that regression visible.

Tests: 353/353 (124 in geminiChat.test.ts +6, 96 in
useGeminiStream.test.tsx unchanged but mock surface expanded,
133 in client.test.ts unchanged). tsc + eslint + prettier clean
on changed files.

* test(cli): route existing dedup tests through fast-path accessor (PR #4176)

One Suggestion thread from the deepseek-v4-pro pass on commit
c30bba6e7.

`MockedGeminiClientClass` did not expose `getHistoryFunctionResponseIds`,
so 3 of the 4 dedup tests in `useGeminiStream.test.tsx` fell through
to the `else if (typeof geminiClient.getHistory === 'function')`
branch — the `structuredClone(this.history)` slow path. Only the
mixed-batch test added in c30bba6e7 wired the fast path explicitly.
Net effect: a regression in `getHistoryFunctionResponseIds` (wrong
ids, missing ids, exception) would silently re-route every dedup
batch onto the slow clone path with all 4 tests still green, while
production paid the multi-millisecond UI-thread stall this PR
specifically added the accessor to avoid.

Fix in two parts:

1. Add `getHistoryFunctionResponseIds = vi.fn().mockReturnValue(new
   Set<string>())` to the `MockedGeminiClientClass` default. Every
   test now exposes the fast-path accessor, so the dispatcher takes
   the `getHistoryFunctionResponseIds` branch by default — matching
   production. Tests that need a non-empty dedup set override the
   mock explicitly.

2. Override the mock in each of the three previously-affected dedup
   tests with a `Set` containing the callId(s) their `getHistory()`
   fixture had paired. The dedup assertions stay identical — but
   the path the production code takes to reach them now matches
   the path under test.

Tests: 96/96 useGeminiStream (no new tests; existing dedup tests
now exercise the fast path the previous commit added). 353/353
across core + cli. tsc + eslint + prettier clean.

* refactor(core,cli): address yiliang114 review observations (PR #4176)

Five items from the yiliang114 review on commit 06a695156. Two P1
(observability + structural maintainability) and three editorial.

[A] P1 — repairOrphanedToolUseTurns refactored into three pure
phases: scanModelTurn / planRepair / applyRepair. Each runs in
isolation against narrow `ScanResult` / `RepairPlan` types, and only
the last mutates `history`. The outer forward-walk loop is now a
~25-line orchestrator: scan → bail on empty expected → plan → bail
on empty plan → apply → record `injected` + `droppedDuplicates` →
advance cursor past any freshly-inserted user turn. Index drift can
only happen in applyRepair, so an audit narrows to one function.
Behavior identical to the pre-refactor monolith — all 13 existing
repair tests pass unchanged.

[B] P1 — finally-block JSONL flush error log promoted from
`debugLogger.warn` to `debugLogger.error`. A persistent write
failure (disk full, permission denied, serialization broken)
silently loses every deferred partial after the first occurrence;
that's the class of failure that warrants monitoring attention, not
the warn category that fades into log noise. Single-occurrence
transient failures still surface as one error per occurrence.

[C] P2 — addHistory partial-marker log promoted to `debugLogger.error`
with `[INVARIANT_VIOLATION]` tag. The existing call graph cannot
legitimately hit this branch (it only fires when addHistory runs
mid-sendMessageStream, which no current caller does); any future
hit is a true bug in a new caller, not noise.

[D] P2 — removed the `getHistory()` fallback branch in
`useGeminiStream.handleCompletedTools`. Verified via grep: every
GeminiClient consumer (real production code + MockedGeminiClientClass
+ every individual dedup test) now exposes
`getHistoryFunctionResponseIds`, so the three-branch duck-type
dispatch was dead-code at production level and only existed to
support an interim cycle when the fast-path accessor was being
introduced. The dispatcher is now a one-liner; production path is
strictly the fast accessor, with an empty Set returned if `geminiClient`
itself is missing (only happens in hook-level unit tests with no
client at all).

[E] P2 — cleanup pass dropping reviewer/thread citations from code
comments (e.g. "qwen-latest-series-invite-beta-v34 thread on PR
#4176", "gpt-5.5 review thread", "deepseek-v4-pro thread",
"yiliang114 repro"). The "why" explanations stay — those are the
load-bearing context. Reviewer/thread attribution lives in the
PR history; embedding it in long-term comments was noise that
ages poorly. Touched 9 sites in geminiChat.ts, 2 in client.ts,
1 in useGeminiStream.ts, 4 in useGeminiStream.test.tsx, and 8 in
geminiChat.test.ts (test names + comment blocks).

Also: P2.4 (yiliang114 flagged a potential test-coverage regression
on TPM throttling + Retry-After delay) verified as a false alarm
— `should retry on TPM throttling StreamContentError with initial
delay` (geminiChat.test.ts:2959) and `should use Retry-After delay
for streamed rate-limit errors` (line 3035) both still exist.

Tests: 353/353 (no new tests; all existing pass through the
refactored repair function and the simplified dispatcher). tsc +
eslint + prettier clean.

* refactor(core): consolidate partial-tool_use repair docs into one design block (PR #4176)

Addresses the pomelo-nwu review observation on c30bba6e7:
~60% of the +918 lines added to geminiChat.ts were comments, with
several 20–40 line prose blocks repeating the same race-class /
tool_use_id wedge analysis at every call site. Net –139 / +117
(net delete ~22 lines but mostly redistributing weight from per-site
blocks to a single canonical note).

Changes:

* Add one canonical design block above ORPHAN_TOOL_USE_REPAIR_REASON
  covering: the tool_use_id 400 wedge, the three race classes (A
  Ctrl+Y mid-flight, B process crash, C SSE drop), the two-layer fix
  (partial-push + repair), and the partial-push marker lifecycle.
  Every per-site comment that used to repeat this now points back.

* Trim popPartialIfPushed COMPRESSED-branch comment from 14 lines to
  4 (no-op-today + reason for keeping).

* Trim recovery-catch comment block from ~38 lines to ~7 (pop-order
  matters + index-checked, pointer to canonical note).

* Trim addHistory invariant comment from ~22 lines to ~5 (when this
  fires + why error-level, pointer to canonical note).

* Trim processStreamResponse partial-push comment from ~22 lines to
  ~8 (Race C name + signal + plain-text-not-persisted rule).

* Replace tool_use_id wedge re-explanations at 5 sites (repair doc,
  scanModelTurn doc, applyRepair doc, popPartialIfPushed comment,
  processStreamResponse comment) with single-line pointers. Canonical
  block at line 411 is now the only authoritative copy.

Reviewer marked this as non-blocking ("shouldn't gate the merge");
done as cleanup polish while LGTM is in.

Tests: 375/375 (no test changes). tsc + eslint + prettier clean.

* refactor(core): further trim partial-tool_use repair comments (PR #4176)

Second cleanup pass on pomelo-nwu's feedback. Previous commit
(298588190) cut net –22 lines but several blocks were still
20–25 lines of prose; this pass takes the rest down to the
one-or-two-line pointer pattern the reviewer suggested.

geminiChat.ts: 2384 → 2207 lines (-177). Comment density 37% → 32%.

Trimmed:

  * pendingPartialAssistantTurnIndex + pendingPartialAssistantRecord
    field doc blocks (16 + 22 lines → one combined 5-line block
    pointing at the canonical note).
  * clearPendingPartialState method doc (10 → 4 lines).
  * sendMessageStream inline-repair comment block (29 → 7 lines).
  * `finally` JSONL flush block (32 → 8 lines).
  * repairOrphanedToolUseTurns function doc (39 → 14 lines).
  * scanModelTurn / planRepair / applyRepair phase docs (11–22 → 4–9
    lines each).
  * RepairPlan interface doc (14 → 1 line).
  * GeminiChat.repairOrphanedToolUseTurns wrapper doc (9 → 3 lines).
  * GeminiChat.getHistoryFunctionResponseIds doc (13 → 6 lines).

What stayed (intentional load-bearing context):

  * The canonical block above ORPHAN_TOOL_USE_REPAIR_REASON — that's
    the consolidation point every per-site one-liner points back to.
  * Pre-existing upstream JSDoc (ctor, sendMessageStream, getHistory,
    redactStructuredOutputArgsForRecording).
  * The Reactive compression no-op note (already at the 4-line cap).
  * The recovery-catch 7-line block (already at the cap).
  * The addHistory invariant 5-line block (already at the cap).

Tests: 375/375 pass on the affected suites. tsc + eslint + prettier
clean on changed files. Behavior unchanged.
2026-05-21 19:52:57 +08:00
易良
fc15b3312d
chore(release): v0.16.0 (#4404)
* chore: bump version to 0.16.0 and normalize bat line endings

* revert: restore install-qwen-standalone.bat to original CRLF encoding

The previous bump commit inadvertently normalized line endings from
CRLF to LF. Windows batch files must retain CRLF in the repository
to work correctly with cmd.exe.

* revert: remove spurious NOTICES.txt change from version bump
2026-05-21 19:49:49 +08:00
顾盼
ce82d65aa1
Revert "fix(core): set x-api-key alongside Authorization on Anthropic outbound (#4323) (#4342)" (#4385)
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
This reverts commit 4b25f9c05c.

PR #4342 unconditionally injected `x-api-key` alongside `Authorization:
Bearer` on every proxy-branch request. This broke IdeaLab-style proxies
(e.g. `idealab.alibaba-inc.com/api/anthropic`) which reject requests
carrying both headers with HTTP 401:

  鉴权header, x-api-key和Authorization不可以同时存在
  (auth header: x-api-key and Authorization cannot coexist)

The two proxy families have mutually exclusive header contracts —
OpenCode-Go-style servers want `x-api-key` only, IdeaLab-style servers
want `Authorization` only — so a one-size-fits-all default cannot
satisfy both at once.

Reverting restores the pre-#4342 default (Bearer only) so IdeaLab users
are unblocked. OpenCode-Go-style users can opt in via `customHeaders`:

  {
    "customHeaders": {
      "x-api-key": "<key>"
    }
  }

`buildHeaders` already merges customHeaders into defaultHeaders (only
`anthropic-beta` is reserved for per-request handling), and on the
proxy branch the SDK is constructed with `apiKey: null` so it does
not emit its own `x-api-key` — the value on the wire comes solely
from the user's explicit customHeaders entry, preserving the #4020
env-leak guard.

Reopens #4323.
2026-05-21 16:13:22 +08:00
易良
262a15e22e
fix(ci): resolve TS5055 release build failure since May 19 (#4383)
The nightly/preview release workflow has been failing for 3 days with
`TS5055: Cannot write file ... because it would overwrite input file`
in packages/core during the version bump step.

Root cause: `npm install --package-lock-only` in version.js triggers
the root `prepare` lifecycle, which re-runs `tsc --build` while
packages/core/dist/ already exists from the initial `npm ci`. The
unbuilt acp-bridge reference (added in #4295 but missing from
build.js) corrupts TypeScript's incremental project graph resolution.

Fixes:
1. Add --ignore-scripts to the lock-file-only install in version.js
2. Add packages/acp-bridge to the build order in build.js

Closes #4368, closes #4339, closes #4307
2026-05-21 16:01:35 +08:00
易良
d2ece83726
feat(skills): support priority field in SKILL.md for sorting skill display order (#4155)
* feat(skills): support priority field in SKILL.md for sorting skill display order

Closes #4136

* fix(skills): make /skills respect priority and treat unset as 0

- /skills was re-sorting alphabetically after listSkills(), masking the
  new priority order. Drop the redundant sort and reuse the manager's
  output directly.
- Treat missing priority as 0 instead of -Infinity so an explicit
  negative priority (e.g. -1) sorts below unset skills, which matches
  user intent.

* fix(skills): harden priority parsing and ordering

* fix(skills): warn when extension supplies invalid priority

Extension-provided skills bypass parseSkillContent / validateConfig, so a
non-number `priority` was silently normalized to 0 in the sort with zero
diagnostic. Match the SKILL.md author signal: warn at load time so the
extension author can see and fix the typo.

Addresses PR #4155 review (the extension-bypass-validation point).

* test(skills): direct unit tests for parsePriorityField and normalizeSkillPriority

Both helpers are exported but previously had no direct tests — coverage
came only via parseSkillContent and listSkills. Adds inputs the
integration paths can't surface cleanly: -0 / NaN / Infinity, numeric
strings, objects, arrays, and the boolean coercion regression that
motivated the strict typecheck.

Also adds a NOTE on parsePriorityField warning future contributors that
SKILL.md frontmatter parsing lives in two places (parseSkillContent here
and SkillManager.parseSkillContent), so any new field must be wired into
both — the same regression that previously hit whenToUse,
disable-model-invocation, paths, and priority. Full dedup of the two
parseSkillContent bodies is left as a follow-up refactor.

Addresses the remaining two [Suggestion] items from PR #4155 review.

* fix(skills): scope priority to /skills listing only

Earlier in this PR, `skill.priority` was mapped into `SlashCommand.completionPriority`
on both bundled and non-bundled skill loaders, so a high-priority skill
also bubbled up in the slash-completion menu and the `/help` custom-commands
tab. That was broader than intended — the design goal is for `priority:`
to control the `/skills` listing only, with everything else (typing `/`,
mid-input completion, `/help`) staying purely alphabetical so a skill
can't reorder built-in commands.

Changes:
- BundledSkillLoader / SkillCommandLoader: drop the
  `completionPriority: skill.priority` mapping. Skill commands now have
  no `completionPriority`, falling back to alphabetical+recency in the
  shared completion comparator.
- Help.tsx: revert the per-group sort to `localeCompare` and remove the
  `compareCommandsForHelp` helper. `/help` is again purely alphabetical
  within each group.
- Tests:
  - Both loader tests assert `completionPriority` is `undefined` when
    a skill has a `priority` set, locking the non-leakage in.
  - Help.test.tsx's "orders by completionPriority" case is replaced
    with "orders alphabetically regardless of completionPriority", so a
    future change that re-introduces the leak fails the test.
- Extension-skill validation also normalizes `skill.priority` to 0 (in
  addition to the existing sort-time normalization) so downstream
  consumers see a clean value matching the emitted warning.

Validation:
- 177/177 unit tests pass across the 5 affected test files
- core typecheck clean
- bundled CLI built (`npm run bundle`) and exercised via tmux E2E:
  E1 /skills sorted by priority, E2 / completion menu unaffected,
  E3 mid-input alphabetical, E4 invalid priority warns + skill loads,
  E5 order stable across restart — all 5 pass.

* fix(skills): tag priority warning with calling module's namespace

`parsePriorityField` previously hardcoded `debugLogger.warn` from
skill-load, so a warning emitted from `SkillManager.parseSkillContent`
(project / user / bundled skills) was tagged `[SKILL_LOAD]` instead of
`[SKILL_MANAGER]`. Annoying for log filtering and slightly misleading
about which parse path actually surfaced the bad priority.

Added an optional `warn` callback parameter; the existing extension
call site keeps the default skill-load logger, while skill-manager
passes its own. Behavior is otherwise unchanged.

* docs(skills): correct priority scope description

Earlier doc said priority sorts "in /skills, slash-command completion,
and the /help custom commands view." After the scope-narrowing in
96722aa67, priority only affects /skills. Updating the doc to match
the actual behavior so readers don't expect cross-surface ordering.

* fix(skills): keep listSkills() alphabetical, sort priority at /skills display

`listSkills()` previously returned priority-desc order for every consumer,
including `SkillTool.refreshSkills()` which builds the model-facing
`<available_skills>` description. That contradicted the stated design goal
(`priority:` controls the `/skills` listing only) and the user docs, which
say everything outside `/skills` stays alphabetical.

- skill-manager.ts: `listSkills()` now sorts name-asc only, giving all
  programmatic consumers (SkillTool, contextCommand, loaders) a stable
  alphabetical order unaffected by `priority:`.
- skillsCommand.ts: apply the priority-desc, name-asc sort at the display
  layer using the shared `normalizeSkillPriority`.
- skills/index.ts: export `normalizeSkillPriority` for the CLI display sort.
- Tests: core tests now lock in that `listSkills()` stays alphabetical
  regardless of priority; new skillsCommand.test.ts covers the display sort.

* fix: correct copyright year 2025 -> 2026 in new file [skip ci]
2026-05-21 14:49:22 +08:00
ChiGao
80895d540e
fix(core): deduplicate geminiChat recovery continuation text (#3966)
* fix(core): deduplicate geminiChat recovery continuation text

When a provider hits MAX_TOKENS and the model resumes via the recovery
loop, the continuation stream sometimes re-sends characters from the end
of the previous response as a context anchor. Without deduplication this
causes repeated Markdown tables/prose in the final history even if the
live UI suppresses them.

Add getRecoveryContinuationSuffix / findContainedRecoveryPrefixReplayLength
to strip the replayed prefix before appending the continuation parts.
Also include the last 1200 chars of the previous response in the recovery
prompt so the model can see where it left off.

Two new tests cover:
- exact suffix overlap (shared recovery suffix and continuation)
- contained tail anchor replay (Markdown table prefix replayed mid-text)

Generated with AI

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

* fix(core): tighten contained-prefix recovery dedup to avoid prose loss

Address review feedback on PR #3966: the contained-prefix fallback in
geminiChat recovery dedup was too permissive — a 6-byte minimum plus a
4000-char lookahead window allowed common opener phrases ("In summary,",
"In conclusion,", "Here is the …") to silently strip legitimate
continuation text whenever they happened to coincide with any substring
in the previous turn. Silent loss is a worse failure mode than the
duplication we were fixing.

Constrain the fallback to its real intended use case — replayed
Markdown blocks that providers re-emit at the start of a recovery
continuation (table headers, headings, fenced code, lists, blockquotes):

- Require the continuation to *open* with a Markdown structural anchor
  before considering any contained-prefix replay; plain prose openers
  fall through with no dedup attempted.
- Restrict the substring search to the immediate truncation tail
  (last 400 chars) so a coincidental match far above the truncation
  point cannot win.
- Raise the contained-prefix byte floor (12 bytes) above the suffix-
  overlap floor.

Also add coverage for the previously-untested guard branches
(empty input, full-overlap drop, empty previous-text path that skips
the <previous_response_suffix> block) and regression tests for the
prose-loss scenarios called out in review.

Generated with AI

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

* fix(core): handle leading whitespace in structural anchor + cover tail truncation

Address wenshao review on PR #3966:

- `startsWithMarkdownStructuralAnchor` now strips all leading whitespace
  (`/^\s+/`) instead of only newlines (`/^\n+/`). Some providers re-emit
  a recovered Markdown block with leading spaces or tabs, not just
  newlines; the old regex caused the structural-anchor gate to fail and
  the contained-prefix dedup path was silently skipped.
- Add a regression test for `buildOutputRecoveryMessage` that exercises
  the `previousText.slice(-OUTPUT_RECOVERY_TAIL_CHARS)` truncation
  branch with a 1300-char previous response, asserting the
  <previous_response_suffix> block contains exactly the trailing 1200
  chars and that the dropped head does not leak.

Generated with AI

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

* fix(core): unify plain-text predicate and harden recovery delimiter

Address two review concerns on geminiChat output-recovery:

- `isPlainTextPart` was a near-duplicate of `isValidNonThoughtTextPart`
  with subtly weaker guards (missing thoughtSignature/inlineData/fileData
  and using `!== true` vs `!part.thought`). Delegate to the shared
  predicate so the recovery-merge and consolidated-history paths agree
  on what counts as plain text.
- `buildOutputRecoveryMessage` embedded the previous response inside a
  `<previous_response_suffix>` pseudo-XML block without sanitization. If
  the model's own truncated output contained the literal closing tag
  (e.g. while generating XML/HTML examples), the recovery prompt's
  structure would break. Neutralize literal opening/closing delimiters
  inside the tail with a zero-width space so the prompt always has
  exactly one well-formed block; add a regression test that asserts the
  delimiter pair count stays at 1/1 even when the tail contains a raw
  `</previous_response_suffix>`.

Generated with AI

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

* test(core): cover opening-tag branch of sanitizeRecoverySuffixTail

The existing prompt-recovery-delimiter-collision test only exercises
the closing-tag (`</previous_response_suffix>`) neutralization path.
Add a sibling test that emits a literal opening tag in the previous
model turn so the opening-tag replace branch is also covered. Asserts
exactly one opening/closing delimiter pair in the recovery message and
that the neutralized variant (with zero-width space) appears in the
embedded tail.

* docs(core): document recovery-dedup constants and tighten contained-prefix anchor

Address PR #3966 review polish items from wenshao:
- Add JSDoc rationale to each magic constant (OUTPUT_RECOVERY_TAIL_CHARS,
  RECOVERY_OVERLAP_MAX_SCAN_CHARS, RECOVERY_OVERLAP_MIN_BYTES,
  RECOVERY_STRUCTURAL_OVERLAP_MIN_BYTES) so future tuning is grounded.
- Make the contained-prefix scan symmetric: require the match inside
  previousTail to begin at index 0 or immediately after a newline, mirroring
  the structural-anchor check on the continuation side. All occurrences are
  walked so a benign mid-paragraph hit doesn't shadow a real line-anchored
  match later in the 400-char tail window.
- Document the suffix-anchored overlap loop's O(n^2) bound and the bounded
  scan cap so the perf characteristic is explicit rather than reverse-
  engineered.
- Explain why appendRecoveryContinuationParts always shifts the first
  continuation text part even when the dedup suffix is empty (empty suffix
  means a pure replay that must be discarded).

All 68 tests in geminiChat.test.ts still pass; typecheck and lint clean.

Generated with AI

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

* fix(core): scan recovery parts for plain-text + CJK-safe overlap floor

`appendRecoveryContinuationParts` previously only inspected the boundary
parts (last of previous, first of continuation). `processStreamResponse`
orders parts as `[thoughtPart?, ...consolidatedHistoryParts]`, so for
thinking models the first continuation part is the recovery turn's
thought — the plain-text predicate failed on it and the entire dedup
block was skipped, leaking the replayed overlap into durable history.
Now scan both sides for the plain-text anchor and splice the matched
text part rather than shifting the head. Allocate a fresh merged part
instead of mutating `mergedParts[i].text` in place so callers caching
part references never observe a half-merged turn.

Two additional hardening fixes on the overlap path:

- `isSignificantRecoveryOverlap` adds a 4-code-point floor on top of
  the 6-byte floor for prose. CJK characters are 3 UTF-8 bytes each,
  so the byte-only floor admitted 2-character coincidences like
  "我们" / "但是" that recur constantly across unrelated Chinese
  turns. The structural-anchor branch is exempted (those collisions
  are far rarer and the structural floor already governs them).
- `findContainedRecoveryPrefixReplayLength` now strips leading
  whitespace from the continuation before matching. The structural-
  anchor check already tolerated leading spaces/tabs (some providers
  re-emit replayed blocks with extra indentation), but the substring
  scan still used the un-trimmed prefix and silently failed to match
  the corresponding `previousTail` occurrence.

Adds three regression tests covering: a thinking-model recovery
continuation whose first part is a thought, a 2-CJK-character
coincidence that must NOT be dedup'd, and a leading-whitespace
structural replay that must be dedup'd.

Generated with AI

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

* docs(core): cover recovery-dedup line-boundary + normalization branches

Add JSDoc to getRecoveryContinuationSuffix calling out that its empty-input
guard is defensive-only (the production caller already filters both sides),
and document appendRecoveryContinuationParts' implicit coupling with
processStreamResponse's text-part consolidation plus its return-shape
convention that coalesceRecoveryPairs relies on for multi-iteration recovery.

Add two regression tests:
- mid-paragraph match rejection: a structural anchor that appears in the
  previous tail but is not preceded by a newline must NOT trigger the
  contained-prefix strip, so legitimate continuation survives verbatim.
- newline-normalization branch: when the replayed prefix ends with \n but
  the previous tail does not and the suffix does not start with \n, the
  helper must insert a separator so the coalesced text keeps its block
  boundary.

Generated with AI

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

* fix(core): tighten table-row anchor + document structural-class scope

Tightens `startsWithMarkdownStructuralAnchor`'s table-row alternation so a
bare `|expression|` (2 pipes) in technical prose no longer qualifies as a
Markdown block anchor — real GFM table rows have ≥3 pipes (≥2 cells) or a
separator row like `|---|`. Without this, prose continuation starting with
a 2-pipe expression that re-appears at a line boundary mid-tail of the
previous response would be silently stripped by the contained-prefix path,
contradicting the JSDoc's stated invariant that "incidental `|` characters
in prose do not count."

Also adds an inline comment to `isSignificantRecoveryOverlap` documenting
why the structural-class detection (`[#|`\n]`) is intentionally loose —
the 2-byte gap between the 4-byte structural floor and the 6-byte prose
floor only matters for 4–5 byte fragments that coincide on both sides of
a truncation boundary, which is far rarer than the structural-replay
scenarios the lower floor exists to catch.

Adds a regression test asserting that a continuation opening with
`|expression| ...` is left intact even when it matches at a line boundary
in the previous tail.

Generated with AI

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

* test(core): pin recovery thought-before-text ordering

Adds a regression test for @tanzhenxin's review comment: the existing
`prompt-recovery-thinking-continuation` test only asserts joined non-
thought text, so a regression where the recovery turn's leading thought
ends up *after* the merged text part slips through. The new test
explicitly asserts `thoughtIdx < mergedTextIdx` in the final history
entry.

Thinking-model providers (Gemini 2.5+, Anthropic, OpenAI o-series)
validate thought-signature provenance and expect a thought to precede
the content it generated; without an ordering assertion the dedup path
could silently violate that invariant.

The new test fails on the current implementation
(`appendRecoveryContinuationParts` appends the leftover leading thought
at the end of the part list). Fix follows in a separate commit so the
red → green transition is reviewable.

Generated with AI

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

* fix(core): keep recovery thought before merged text part

The recovery dedup path in `appendRecoveryContinuationParts` previously
spliced only the matched continuation text part out of `nextParts` and
appended the leftover parts (including any leading thought) after the
merged text. For thinking-model providers (Gemini 2.5+, Anthropic,
OpenAI o-series) that validate thought-signature provenance, this
violated the invariant that a thought precedes the content it
generated: durable history ended up as `[..., previousText + suffix,
recoveryThought]`, with the recovery turn's thought trailing its own
text.

Hoist any non-text parts that preceded the matched text on the
continuation side (typically the recovery turn's thought) into
`mergedParts` directly before the merged text part. Trailing non-text
parts (tool calls etc.) keep their position via the final concat.
Existing `prompt-recovery-thinking-continuation` test still passes
because it only asserts joined non-thought text; the new
`...-order` test now passes as well.

Reported by @tanzhenxin in PR review on commit 556b015.

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-21 14:25:42 +08:00
jinye
64401e1d17
feat(telemetry): support custom resource attributes and add metric cardinality controls (#4367)
* feat(telemetry): support custom resource attributes and add metric cardinality controls

Resolves #4365.

Adds two coupled OpenTelemetry capabilities to make qwen-code's telemetry
production-ready in multi-team / multi-tenant deployments:

1. Custom resource attributes via standard `OTEL_RESOURCE_ATTRIBUTES` and
   `OTEL_SERVICE_NAME` env vars and a new `telemetry.resourceAttributes`
   setting. Operators can now tag every span / log / metric with `team`,
   `env`, `cost_center`, or anything else their backend needs.
2. Metric cardinality controls. `session.id` is moved off the OpenTelemetry
   Resource (where it auto-attached to every metric data point and caused
   unbounded time-series fan-out on Prometheus / ARMS Metric / etc.) and
   gated behind a new opt-in `telemetry.metrics.includeSessionId` toggle.
   Spans and logs still carry `session.id` for trace and log correlation.

Reserved keys (`service.version`, `session.id`) are stripped from both env
and settings sources with a `diag.warn`. `OTEL_SERVICE_NAME` follows the
OTel spec precedence (highest priority for `service.name`). Settings JSON
values are runtime-coerced to strings as defense against hand-edited
non-conforming JSON.

Breaking change: metrics no longer carry `session.id` by default. Operators
who need it can restore the previous behavior with
`QWEN_TELEMETRY_METRICS_INCLUDE_SESSION_ID=true` or
`telemetry.metrics.includeSessionId: true` in settings.json; recommended
only for short-term debugging since it re-introduces the cardinality
problem. For long-term session-level analysis, prefer trace and log
backends which handle per-event data without cardinality pressure.

Design doc: docs/design/telemetry-resource-attributes-design.md

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

* docs(telemetry): align reserved-key descriptions with implementation

Round 1 review fixes (#4367). After session.id was added to
RESERVED_RESOURCE_ATTRIBUTE_KEYS in Codex review, four user-facing
descriptions still claimed only service.version was reserved:

- packages/core/src/telemetry/config.ts (merge comment)
- packages/core/src/config/config.ts (TelemetrySettings JSDoc)
- packages/cli/src/config/settingsSchema.ts (schema description)
- packages/vscode-ide-companion/schemas/settings.schema.json (regenerated)

Also corrects scope claim: resource attributes apply to every signal
the SDK exports (OTLP and file outfile share the same Resource), not
just OTLP.

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

* docs(telemetry): clarify warning destination and surface percent-encoding hint

Round 2 self-review fixes (#4367). Two small but real UX gaps:

1. Reserved-key / malformed-pair / coerce warnings route to the debug
   log (per #3986), not the console — so a user who types
   `OTEL_RESOURCE_ATTRIBUTES=service.version=2.0` sees no feedback that
   the value was silently dropped. Adds a "Troubleshooting" section in
   telemetry.md telling users where to look, and a note in the parser
   docstring documenting where warns go.

2. A literal (unencoded) comma in an env var value is a common foot-gun:
   the parser splits on it, producing a malformed second half that is
   silently dropped. Updates the warn text to include a "hint:
   percent-encode literal commas as %2C" callout, and adds the same
   guidance to the docs.

Deferred to a follow-up: startup-time stderr summary of dropped
attributes. Stderr during TUI render could break Ink rendering, so the
right surface needs separate design.

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

* test(telemetry): cover first-`=` split contract in OTEL_RESOURCE_ATTRIBUTES parser

Per review feedback on #4367. The parser uses `indexOf('=')` so
the first `=` separates key and value while subsequent `=` stay in
the value. The behavior was correct but untested; a future refactor
to `split('=')` would silently break base64-padded, JWT, or
connection-string values.

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

* feat(telemetry): tighten resource-attribute input validation + startup summary

Adopts review feedback from #4367 (wenshao via Qwen Code /review).

Five accepted suggestions, bundled because they all touch the same
parse/coerce/strip pipeline:

1. Key percent-decoding (CRITICAL). `parseOtelResourceAttributes` now
   percent-decodes both keys and values per the OTel / W3C Baggage spec.
   Without this, `OTEL_RESOURCE_ATTRIBUTES=service%2Eversion=99` lands
   on Resource as the literal key `service%2Eversion`, bypassing the
   reserved-key filter; a collector that decodes keys downstream could
   then resurrect `service.version` and spoof the version label.

2. Startup summary of dropped attributes. Every `diag.warn` in
   resource-attributes.ts routes only to the OTel debug log (per
   #3986), giving operators zero feedback when their attributes are
   silently dropped. Helpers now optionally accumulate diagnostics
   into a `ResourceAttributeWarnings` array; the resolver collects
   them and the SDK emits a one-time console summary at init (before
   Ink renders, so no TUI conflict).

3. `||` instead of `??` for service.name fallback. Settings can put
   an empty string through `??`, producing a blank `service.name`
   that some backends reject. `||` falls through to the default.

4. `coerceStringResourceAttributes` now trims keys and skips
   empty/whitespace-only keys, matching `parseOtelResourceAttributes`.
   Previously `{"  ": "x"}` or `{"team ": "y"}` from settings.json
   would land as malformed Resource attributes.

5. `OTEL_SERVICE_NAME` is trimmed before the truthy check, so values
   like `'  '` or `'\t'` are treated as unset rather than producing
   a whitespace-only service name on Resource.

One suggestion declined (in-thread reply on PR):

- "Redundant `?? {}` in sdk.ts:160" — intentional defense-in-depth for
  `vi.mock('../config/config.js')` callers in `telemetry.test.ts` where
  auto-stub returns undefined. The reviewer is right that production
  code paths never hit it, but tests do.

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

* fix(telemetry): trim whitespace-only service.name + add invalid-key-encoding test

Adopts two review suggestions on #4367 (wenshao via Qwen Code /review):

1. `service.name` fallback uses `.trim() || SERVICE_NAME` instead of plain
   `||`. Plain `||` lets whitespace-only values (`" "`, `"\t"`) through as
   truthy, producing a blank service name on Resource that some backends
   reject. Both settings (no value trimming) and env (`%20` decodes to `" "`)
   can deliver such values. Test added.

2. Adds `key%ZZ=val` to the parameterized parser test to cover the
   invalid-percent-encoding-on-key catch branch. Previously only the
   value-side catch was tested.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
2026-05-21 13:54:37 +08:00
Dragon
59d5c1f16f
fix(core): handle MiMo tool-result media (#4281) 2026-05-21 13:18:25 +08:00
易良
d59c9e7b77
feat(installer): add standalone hosted install and uninstall flow (#3828)
* feat(installer): add standalone archive installation

* fix(installer): harden standalone archive installs

* fix(installer): address standalone review findings

* chore(installer): clarify review followups

* fix(installer): stabilize standalone script checks

* chore(installer): remove internal planning docs

* chore(installer): simplify standalone release review fixes

* test(installer): add Windows batch install smoke

* test(installer): fix Windows batch smoke quoting

* test(installer): preserve Windows cmd quotes

* fix(installer): use robust Windows checksum hashing

* ci: narrow installer debug matrix

* fix(installer): address standalone review hardening

* fix(installer): avoid Windows validation parse errors

* fix(installer): simplify Windows option validation

* fix(installer): harden standalone review fixes

* feat(installer): publish release installer assets

* fix(installer): address release asset review feedback

* fix(installer): avoid prerelease installer asset links

* test(installer): isolate standalone dist fixture

* feat(installer): add hosted install release alias

* chore: no changes - code review requested

Agent-Logs-Url: https://github.com/QwenLM/qwen-code/sessions/38467aec-15b9-4b76-9139-0b2cfe40477a

* fix(installer): pin versioned installer assets

* fix: parallelize Node.js binary downloads in standalone release build

Use Promise.all instead of sequential for...of+await for
the 5 independent Node.js runtime downloads, reducing CI
release build time by ~4-5x.

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

* fix(installer): address release asset review followups

* refactor(installer): share release CLI parsing

* fix(installer): address release asset review followups

- sh: reject CR/LF in archive entry names before the literal `..` glob so
  a `..\r` entry cannot bypass path validation.
- bat: prefer Tls12+Tls13 in PowerShell helpers, fall back to Tls12 alone
  on older .NET Framework where the Tls13 enum is missing.
- bat: document the implicit `:ValidateOptions` dependency next to the
  qwen.cmd wrapper writer so loosening the validator stays a conscious
  choice.
- build-standalone-release: surface the `xz-utils` host requirement for
  Linux Node downloads in `--help`.
- release-script-utils: support `--key=value` form in `parseCliArgs`.
- tests: cover the new CRLF message, TLS string, and `--key=value` parsing;
  register process-level signal/exit handlers in `ensureMinimalDist` so a
  crashed test still restores `dist/`.

* fix(installer): unblock Windows CI for standalone install path

Three CI failures and a few review followups in one pass.

- ensureMinimalDist places its dist/ backup beside dist/ instead of
  under os.tmpdir(). On Windows GitHub runners the workspace lives on
  D: while os.tmpdir() is on C:, so renameSync raised EXDEV for every
  test that needed to swap dist/ in.
- create-standalone-package.js and the matching test fixture build
  win-x64 zips with [IO.Compression.ZipFile]::CreateFromDirectory.
  Compress-Archive emits backslash entry names that the .bat
  installer's path-traversal guard then rejected, so every freshly
  built archive failed the standalone install path on Windows.
- :ValidateArchiveContents normalizes entry separators to '/' before
  checking for '..', absolute paths, and drive prefixes - archives
  from any Windows zip tool still install while real traversal
  entries remain rejected.
- createWindowsTraversalStandaloneArchive runs PowerShell via -File
  instead of a single -Command line; the joined-with-'; ' form had a
  function definition the runner's PowerShell refused to parse.

Drive-by review followups:

- replaceRequired uses replaceAll so a future duplicate placeholder
  cannot silently keep the trailing copy as 'latest'.
- :ValidateOptions runs the unsafe-character check on SOURCE
  alongside the other variables.
- build-installation-assets.js drops a dead INSTALLATION_ASSETS
  re-export; consumers already import from release-asset-config.js.
- .gitignore covers the new sibling .qwen-dist-backup-* directory.

* fix(installer): address release asset review findings

* fix(installer): keep installer entrypoint hosted

* fix(installer): reject stale hosted assets

* fix(installer): refine hosted asset staging

* fix(installer): tighten hosted default-version check, flag legacy URL

- Replace the loose `latest` fragment check with per-format regex patterns
  in HOSTED_INSTALLER_DEFAULT_VERSION_PATTERNS so an unrelated occurrence
  of `latest` (comment, help text) cannot satisfy the staging guard. The
  patterns still tolerate whitespace variation, only the default-version
  assignment itself must be intact.
- Add a "Hosted endpoint status" callout in INSTALLATION_GUIDE.md before
  the curl examples. The documented `--version` flow does not work against
  the OSS URL today because it currently serves the legacy NVM-based
  installer; the callout points users at a local checkout until the next
  release sync.
- Tests: drop `latest` from the fragments equality assertion, add positive
  and negative regex coverage, add a failure-path case for sources whose
  default version is not `latest`, and pin the new guide markers so the
  callout cannot silently disappear.

* feat(installer): verify installation release assets

Adds `npm run verify:installation-release` and wires it into the release
workflow after `Build Standalone Archives`, so a broken release directory
fails CI before publishing.

Local mode (`--dir PATH`) checks:
- All five `qwen-code-{platform}.{ext}` standalone archives exist.
- `SHA256SUMS` covers exactly those five — missing or unexpected entries fail.
- Each archive's actual SHA256 matches its `SHA256SUMS` entry.

Remote mode (`--base-url URL`) checks:
- `SHA256SUMS` is downloadable, parseable, and contains exactly the expected
  archive entries.
- Each archive URL is reachable via HEAD, with a 1-byte ranged GET fallback
  for hosts that disable HEAD.

Hosted installer scripts (`install-qwen.sh` / `install-qwen.bat`) are
intentionally out of scope here — they are served from the hosted endpoint
prepared by `package:hosted-installation` (PR #3853), not from the GitHub
Release surface this verifier targets.

* fix(installer): tighten verifier base-url + clarify test helper

Three small refinements from the second review pass:

- normalizeHttpsBaseUrl rejects everything except https, since real release
  URLs are always HTTPS. Accepting http previously would let an operator
  silently target a stale or attacker-controlled mirror.
- Drop EXPECTED_RELEASE_ASSET_NAMES from the public exports; it was only
  used internally for the verification log line.
- Rename the test helper standaloneChecksumContent to
  placeholderChecksumContent and document that the hashes in its output are
  placeholders — the remote verifier does not download archives or compare
  hashes, it only validates that SHA256SUMS lists the expected names and
  that each archive URL is reachable.

The non-https rejection test now also covers `http://` in addition to the
existing `file://` case.

* fix(installer): address standalone review follow-ups

* fix(installer): repair Windows installer tests

* fix(release): tighten standalone asset checks

* fix(installer): stabilize Windows managed install checks

* test(installer): relax Windows installer timeout

* fix(test): escape release asset regex

* test(cli): avoid POSIX node path in relaunch test

* fix(installer): align npm fallback node gate with engines

* test(installer): allow Windows archive validation more time

* fix(installer): remove stale node 20 installer references

* docs(installer): clarify hosted endpoint sync requirement

* refactor(installer): reuse standaloneArchiveName in release verifier

The verify-installation-release script was duplicating the archive name
derivation logic with a hardcoded ternary instead of reusing the
standaloneArchiveName helper from build-standalone-release. Export the
helper and import it so the extension mapping lives in one place.

* fix(scripts): address release verifier review feedback

* feat(installer): add standalone archive installer with multi-platform release workflow

- Add standalone archive installer (bat/sh) that downloads platform binaries
  from GitHub/Aliyun without requiring Node.js or npm on the target machine
- Add fork-friendly release-test workflow for manual GitHub Release creation
  covering all 5 platforms (darwin-arm64/x64, linux-arm64/x64, win-x64)
- Add OSS upload/mirror tools for staging and release distribution
- Update .gitignore to exclude generated build artifacts (release-staging/,
  hosted-staging/)
- Fix Windows PowerShell test command in copy-release-to-latest tool

* feat(installer): support QWEN_INSTALL_GITHUB_REPO env var for custom repo

* chore(installer): exclude local-only staging tools from PR

The tools/ directory contained personal staging-OSS upload helpers
(upload-staging, upload-release-mirror, copy-release-to-latest,
test-upload-one) that should not ship in the public PR. They reference
a personal staging bucket and only exist to validate the installer
end-to-end before production release.

Removes them from git tracking via `git rm --cached` (files stay on
disk for the author's local use) and adds /tools/ to root .gitignore
so they cannot be re-added accidentally.

No runtime / installer code change. Production CI on ubuntu-latest is
unaffected.

* fix(installer): enforce CRLF line endings for .bat files via gitattributes

cmd.exe requires CRLF in batch scripts; the global eol=lf was causing
every line to be misparsed on Windows, producing errors like
'QWEN_VALIDATE_METHOD=detect is not recognized as a command'.

* fix(installer): store .bat files with CRLF in git blob for raw GitHub downloads

GitHub raw file serving bypasses gitattributes eol conversion and serves
blob bytes directly, so eol=crlf alone was not enough. Use -text to disable
normalization and commit with actual CRLF so raw downloads work on Windows.

* fix(installer): follow HTTP redirects in UrlExists and RaceMirrorHead probes

GitHub release asset URLs return HTTP 302 to objects.githubusercontent.com.
[Net.WebRequest] with HEAD does not auto-redirect by default, so the
existence check and mirror-race probe both incorrectly reported the file
as missing. Set AllowAutoRedirect=true on HttpWebRequest instances.

* fix(installer): surface download errors and add MaximumRedirection 10

* feat(installer): add hosted install-qwen.ps1 shim for irm|iex one-liner

The previous Windows quick-install one-liner used `Invoke-WebRequest -OutFile
(Join-Path $env:TEMP 'install-qwen.bat'); & (Join-Path …)`. When pasted into a
narrow terminal, line wrap could land on `-OutFile`, orphaning the parameter
from its value and producing the "missing argument for OutFile" failure
followed by a "file not found" when the second `&` ran. PowerShell's line
continuation rules cannot resolve this for parameter-name-at-EOL.

Add `install-qwen.ps1` as a thin hosted entrypoint that downloads
`install-qwen.bat` into TEMP, runs it, and cleans up. Documented one-liner
becomes the standard pattern used by bun, uv, scoop, deno, pnpm:

    powershell -ExecutionPolicy Bypass -c "irm <url>/install-qwen.ps1 | iex"

The `.bat` remains the source of truth for installer behavior; `.ps1` is just
the modern hosted entrypoint. Version pinning via `$env:QWEN_INSTALL_VERSION`
flows through unchanged. Stored with `*.ps1 -text` so CRLF survives both
GitHub raw and OSS uploads, matching the existing `.bat` handling.

* fix(installer): stage direct hosted install scripts

* chore(installer): trim hosted release diff scope

* chore(installer): narrow hosted release diff

* feat(installer): restore hosted PowerShell entrypoint

* chore(installer): stage standalone hosted entrypoints

* fix(installer): address hosted installer review followups

* fix(installer): stabilize Windows installer tests

* fix(installer): make Windows option validation readable

* feat(installer): wire Aliyun OSS sync, address review followups

- Add Aliyun OSS sync steps to release workflow: package hosted assets,
  install pinned ossutil, configure credentials, upload versioned and
  latest paths, and verify upload via verify:installation-release plus
  curl probes against the hosted installer endpoint.
- Document required production-release environment secrets and bucket
  variables in INSTALLATION_GUIDE.md.
- Restructure hosted endpoint guidance to lead with the pre-sync
  warning, splitting "Run today" (local checkout) from "After the OSS
  sync" (hosted one-liners) so users no longer copy a one-liner that
  silently installs latest.
- Distinguish mirror auto-selection timeout from successful selection
  in install-qwen-standalone.sh and install-qwen-standalone.bat: emit
  a "timed out; defaulting to github" log instead of pretending the
  HEAD probe picked github.
- Support QWEN_INSTALLER_BAT_URL override (https only) in the
  PowerShell shim so staging mirrors can be exercised without forking
  the file.
- Strip a leading UTF-8 BOM in verify-installation-release.js
  parseSha256Sums so BOM-prefixed SHA256SUMS reports a useful
  "Missing checksum entry" error instead of "Malformed SHA256SUMS
  line 1".
- Add tests for verifier HEAD→Range fallback, partial-failure
  formatting, all-failure wording, and BOM tolerance.

* ci(installer): add temporary OSS smoke test

* fix(installer): make OSS release assets public-readable

* chore(installer): remove temporary OSS smoke workflow

* fix(installer): address hosted installer review gaps

* feat(installer): refactor argument parsing and utility functions for release scripts

* fix(installer): harden hosted release script checks

* fix(installer): suppress PowerShell progress bar in hosted entrypoint shim

Add $ProgressPreference = 'SilentlyContinue' to the .ps1 wrapper so
Invoke-WebRequest downloads don't render a progress bar when invoked
via the irm | iex one-liner.

* fix(installer): suppress PowerShell progress bar in bat installer downloads

Add $ProgressPreference = 'SilentlyContinue' to DownloadFile so the
full-screen progress UI does not appear during archive downloads in
interactive PowerShell sessions, consistent with the .ps1 shim.

* fix(installer): use curl.exe -# progress bar in Windows downloads

Prefer curl.exe with -# (hash-mark progress bar) for archive and installer
downloads on Windows 10+. Falls back to Invoke-WebRequest (which shows its
own progress bar) when curl.exe is unavailable. Matches the approach used
by code-server (curl -#fL) and bun.sh (curl.exe -#SfLo).

* fix(installer): suppress progress bars for small downloads and Expand-Archive

- .ps1: replace curl.exe -# with silent mode, suppress Invoke-WebRequest
  progress bar; save/restore $global:ProgressPreference
- .bat: add $ProgressPreference = 'SilentlyContinue' before Expand-Archive
  to prevent full-screen extraction progress UI
- .sh: remove --progress-bar / --show-progress from download_file, always
  use silent curl/wget

* fix(installer): auto-backup non-qwen directories and simplify output

- ensure_managed_install_dir / :EnsureManagedInstallDir now back up
  non-qwen directories instead of refusing to install, so users
  upgrading from npm or old installers don't hit a hard error
- Simplify header/footer output: remove banner bars, verbose INFO
  lines, and redundant "Installation completed!" message
- Match bun.sh / code-server style: minimal, to the point

* fix(installer): revert Expand-Archive progress suppression in bat

The inline $ProgressPreference = 'SilentlyContinue' caused a cmd.exe
parsing error ("此时不应有 >") on Chinese Windows. Revert to the
original Expand-Archive invocation.

* fix(installer): fix cmd.exe parsing error in backup fallback code

The %s in the for /f fallback command string was interpreted as a variable
reference by cmd.exe, causing "此时不应有 >" on Chinese Windows. Replace
with a safe fallback and re-enable Expand-Archive progress suppression.

* fix(installer): always persist install bin to user PATH

Previously MaybeUpdateUserPath was only called when shadow qwen
executables were detected. When no shadow was found, the PATH update
was skipped entirely, leaving the user without qwen on PATH after
restarting their terminal.

Now always persist the bin directory to PATH (unless --no-modify-path
is set), regardless of whether other qwen installations exist.

* fix(installer): persist PATH to current terminal session on Windows

Use the `endlocal & set` trick (same as bun/Rust installers) to export
the install bin directory from the setlocal scope to the current cmd
session. qwen is now usable immediately without restarting the terminal.

* docs(installer): document cmd.exe one-liner for immediate PATH availability

Add curl-based one-liner for cmd.exe users. Running the .bat directly
in the current cmd session makes `qwen` available immediately via the
`endlocal & set` trick. The `powershell -c "irm | iex"` path creates
a child process so PATH changes cannot propagate to the parent.

* feat(installer): make qwen usable immediately from PowerShell after install

- .ps1: detect parent process, update current session PATH, and for
  cmd.exe parents emit a `set PATH=...` command
- .bat: skip final instructions when called from PowerShell to avoid
  duplicate "Run: qwen" output

* fix(installer): remove non-functional doskey approach for cmd parent

doskey /exename from a child PowerShell process cannot modify the
parent cmd.exe session. Replace with a simple set PATH=... command
that the user can copy-paste.

* fix(installer): make Windows standalone shim available in cmd

* feat(installer): add standalone uninstall scripts

* fix(uninstall): match shell-quoted paths when removing the wrapper

The installer's write_unix_wrapper shell-quotes the binary path, so
paths containing single quotes (or other shell metacharacters) appear
as shell-quoted strings in the generated wrapper file. The uninstall
script's literal grep -qF missed these, leaving the wrapper orphaned.

Add shell_quote to the uninstall script and match against both the raw
and shell-quoted forms before removing the wrapper.

* fix(installer): update download commands to use progress indicators for curl and wget

* fix(installer): resolve Aliyun latest via version pointer

* fix(installer): cleanup mirror probe temp dirs

* fix(installer): harden standalone release fallback

* fix(installer): address standalone review feedback

* style(installer): align standalone install output

* fix(installer): print standalone uninstall commands

* fix(installer): address release review follow-ups

* fix(installer): harden Windows target detection

* test(installer): stabilize Windows fake tool path

* fix(installer): allow explicit Windows curl path

* test(installer): use cmd fake curl on Windows

* test(installer): cover Windows fake curl helper

* test(installer): inject Windows arch overrides in cmd

* test(cli): wait for prompt suggestion render

* test(cli): revert prompt suggestion wait tweak

* fix(installer): harden hosted release publishing

* fix(installer): harden Windows latest pointer parsing

* fix(installer): bound Windows download timeouts

* fix(installer): bound hosted installer probes

* fix(release): make ossutil download configurable

* fix(installer): address hosted release review feedback

* test(installer): keep dist backup on same filesystem

* fix(installer): address remaining review feedback on PR #3828

- Remove REQUIRE_CHECKSUM dead code, always hard-fail on checksum issues
- Add JSDoc to HOSTED_INSTALLER_BEHAVIOR_PATTERNS explaining its purpose
- Add credential cleanup trap for ossutilconfig in release workflow
- Add 3-attempt retry with exponential backoff for OSS uploads
- Tighten findstr SOURCE regex to require leading letter

* fix(release): correct OSS credentials lifetime and mirror probe fallback

- release.yml: remove `trap EXIT` inside the Configure step; it deleted
  ${RUNNER_TEMP}/.ossutilconfig as soon as the configure shell exited,
  so every subsequent step (publish/sync/verify) lost the credentials.
  Move credential cleanup to a final `if: always()` step at the job tail.
- install-qwen-standalone.sh: drop the predictable PID-based mktemp -d
  fallback in race_mirror_head; if mktemp fails, return "github" instead
  of using /tmp/qwen-mirror.$$ which a local attacker could pre-create
  to bias mirror selection.

* fix(installer): address review feedback round 2

Workflow:
- Move 'Publish Aliyun OSS Latest VERSION' to run after the hosted installer
  assets are uploaded and verified, so the latest/VERSION pointer only flips
  once every release artifact is in place. Previously a hosted-sync failure
  could leave the pointer ahead of the actual installer scripts.

upload-aliyun-oss-assets.js:
- Replace `spawnSync('sleep', ...)` retry backoff with an Atomics.wait-based
  cross-platform sleep so retries also work on Windows runners.

install-qwen-standalone.bat:
- :DetectTarget no longer emits TARGET=win-arm64 because RELEASE_TARGETS has
  no win-arm64 archive; ARM64 hosts now fall through to the unsupported-arch
  branch and (in detect mode) get the npm fallback instead of a 404.
- Add QWEN_INSTALL_CURL_EXE to :ValidateRawEnvironmentOptions so this curl
  override is checked for shell metacharacters like every other knob.
- Replace `call echo %%i>>...` with plain `echo %%i>>...` when capturing
  pre-install qwen.cmd paths; `call` triggered an extra parse pass that
  could interpret &/|/<,>/etc. inside a directory name as command separators.
- Add `--retry 2` to curl.exe downloads (`:DownloadFile` / `:DownloadFileQuiet`)
  to match the shell installer.
- Include expected vs actual hash in the checksum-mismatch error message.

install-qwen-standalone.ps1:
- Stage the downloaded installer at a cryptographically random temp path
  (`qwen-installer-<random>.bat`) so a same-user attacker cannot pre-stage a
  malicious .bat at a predictable path and race the verify/execute window.
- Atomically install the current-session cmd shim by writing to a sibling
  `.new` temp file then renaming, so a partial write cannot leave a
  half-written shim on PATH.
- Add `--retry 2` to the curl.exe download path.
- Include expected vs actual hash in the checksum-mismatch error message.

install-qwen-standalone.sh:
- Include expected vs actual hash in the checksum-mismatch error message.

uninstall-qwen-standalone.ps1:
- Accept `-Purge` and `-Help` parameters; previously every CLI flag was
  silently dropped, so users running with `-Purge` got no purge and no error.
  `-Purge` maps to `QWEN_UNINSTALL_PURGE=1`.

uninstall-qwen-standalone.sh:
- `remove_install_wrapper` additionally requires the wrapper file to start
  with a `#!` shebang before it deletes it; a user-authored script that just
  happens to mention the install path now stays untouched.

verify-installation-release.js, build-hosted-installation-assets.js:
- Include expected vs actual hash in the checksum-mismatch error messages.

scripts/tests/install-script.test.js:
- Update assertions for the new error wording, the curl `--retry 2` flag,
  the dropped ARM64 detection, and the new release-step ordering.

* fix(installer): address review feedback round 3

Workflow:
- Configure Aliyun OSS Credentials: write the ossutil config file directly
  with restricted umask instead of invoking `ossutil config -k <secret>`.
  Passing the access-key secret via argv made it visible in /proc/<pid>/cmdline
  for the lifetime of that step; writing the INI file in-process keeps the
  secret out of the process table.

upload-aliyun-oss-assets.js:
- Upload assets in parallel with `Promise.all` + async `spawn` instead of a
  sequential `spawnSync` loop. Each asset keeps its own retry budget; failures
  are aggregated so one flaky upload does not mask a separate failure.
- Replace the bespoke `Atomics.wait` retry sleep with `timers/promises#setTimeout`
  now that the loop is async.

INSTALLATION_GUIDE.md:
- Drop the misleading "instead of overwriting the global installation/
  entrypoint objects" sentence; the workflow has always also refreshed the
  global versionless objects so curl|bash links keep resolving without a
  version segment. Document the rollback story instead.

* test(installer): add parseUploadArgs unit tests and align verify derivation

- scripts/tests/upload-aliyun-oss-assets.test.js: cover --help short-circuit,
  required-option validation (--bucket/--config/--prefix/empty assets),
  unknown options, missing option values, and trailing-slash prefix
  normalization.
- scripts/verify-installation-release.js: switch the win-only zip branch
  from `startsWith('win-')` to the strict `=== 'win-x64'` check used by
  build-standalone-release.js, and add a comment recording that the two
  derivations must stay aligned. Without this the helpers would diverge
  the moment a non-x64 win target gets added.

* test(installer): add uploadAssets integration tests with fake ossutil

Add two integration tests that route a temp-directory ossutil shim onto
PATH so uploadAssets actually spawns the real binary with the real cp
argv:

- happy-path test asserts the destination URI, `-c <config>`, `--acl
  public-read`, and per-asset cp invocations land for both inputs.
- failure-path test asserts non-zero ossutil exits surface as an
  aggregate `asset uploads failed` error after the retry budget runs out.

* revert(installer): drop over-engineered ossutil/upload changes

Roll back two changes from a1ef8697b/0a5d308c9 that were not justified
by the actual threat model or release-pipeline needs:

- .github/workflows/release.yml: restore the supported `ossutil config -k`
  invocation. The earlier switch to writing the .ossutilconfig INI file
  in-process was meant to keep the access-key out of /proc/<pid>/cmdline,
  but GitHub-hosted runners are single-tenant ephemeral VMs where no other
  user can read that namespace. The benefit was theoretical; the cost was
  taking on a brittle dependency on ossutil's undocumented config format.

- scripts/upload-aliyun-oss-assets.js: revert the uploadAssets parallel
  rewrite (Promise.all + spawn + setTimeout) back to the original sync
  spawnSync loop with retry. Release-time uploads of ~6 small files do
  not need parallelism, and the async refactor changed the public
  contract (sync→async) for no real wall-clock win.

Kept from those commits:
- The cleanup `if: always()` step that removes RUNNER_TEMP/.ossutilconfig
  at the end of the publish job.
- The cross-platform sleepSync(ms) helper, since `spawnSync('sleep', ...)`
  still does not work on Windows runners.
- The INSTALLATION_GUIDE.md doc fix.
- All other round-2 fixes.

Test assertions updated for the restored sync uploadAssets contract.

* test(installer): cover Windows release script regressions

* test(release): avoid Windows shim lookup in oss upload tests

* test(installer): use stable fake Aliyun version on Windows

* fix(installer): parse Aliyun latest version in batch

* fix(installer): validate Aliyun latest version without findstr

* fix(installer): normalize Aliyun latest version via PowerShell

* fix(installer): avoid captured PowerShell output in batch latest parsing

* fix(installer): normalize Aliyun latest pointer from file

* test(installer): fix fake Windows curl output parsing

* fix(installer): print checksum path on miss, gate hardcoded version pin in ps1 [skip ci]

Address two narrow follow-ups from PR #3828 review:

- build-hosted-installation-assets.js: add a HOSTED_INSTALLER_FORBIDDEN_PATTERNS guard for install-qwen-standalone.ps1. The ps1 shim has no VERSION variable of its own (it forwards @args to the .bat), so the existing default-version positive-match patterns don't apply. The new guard fails the build if a $env:QWEN_INSTALL_VERSION assignment or a --version flag prepended to the forwarded argument list ever lands in the shim. Patterns are line-anchored with /m so the documented usage examples in the header docstring stay valid. Two vitest cases cover the reject and allow paths.

- install-qwen-standalone.sh / .bat: include the searched checksum-file path in the "SHA256SUMS not found" error. Operators triaging --archive failures could not tell from the prior message whether the fallback path (next to the archive) or the remote URL was being looked up. Existing test assertions updated to match the new wording.

Local validation: npm run test:scripts -> 160 passed | 9 skipped (was 158 | 9).

* fix: stamp release version in hosted installers and add Zip Slip protection [skip ci]

1. The hosted installation asset build now accepts --version and stamps it
   into the copied .sh/.bat installers so they default to the tagged release
   version instead of 'latest'. The release workflow passes the version.

2. install-qwen-with-source.bat now validates archive entries before calling
   Expand-Archive, rejecting paths with '..', leading '/', drive-rooted
   paths, empty names, or control characters — matching the protection
   already present in install-qwen-standalone.bat and the .sh installer.

* fix(installer): add SOURCE to PowerShell unsafe-character validation [skip ci]

The SOURCE variable is user-provided and used in path operations but was
not included in the :ValidateOptions unsafe-character check. Add it
alongside the other validated variables.

* fix: correct copyright year 2025 -> 2026 in new files [skip ci]

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: yiliang114 <effortyiliang@gmail.com>
2026-05-21 11:57:10 +08:00
jinye
b58fe19c3a
feat(telemetry): Phase 2 — tool.blocked_on_user + hook spans (#3731) (#4321)
* feat(telemetry): Phase 2 — tool.blocked_on_user + hook spans

Adds two OTel span types under the existing hierarchical session-tracing
infrastructure (#3731 Phase 2; depends on Phase 1 #4126 and Phase 1.5 #4302):

1. `qwen-code.tool.blocked_on_user` — brackets the time a tool spends in
   awaiting_approval waiting for the user. Child of the tool span. Records
   decision (proceed_once / proceed_always / cancel / aborted /
   auto_approved) and source (cli / ide / hook / auto / system). Status
   stays UNSET — waiting is neither OK nor ERROR.

2. `qwen-code.hook` — wraps each pre/post-hook fire site so a slow hook can
   be told from a slow tool. Records hook_event (PreToolUse / PostToolUse /
   PostToolUseFailure), tool_name, shouldProceed, shouldStop, blockType,
   hasAdditionalContext. Status stays UNSET on intentional blocking
   decisions; ERROR only when the hook itself throws.

To make blocked_on_user a child of the tool span, the tool span lifecycle
moved from `executeSingleToolCall` to `_schedule`'s validating-loop —
covering validating → awaiting_approval → executing in one span. Two new
private Maps on CoreToolScheduler hold span refs across method boundaries
(callId-keyed). Centralized cleanup via `finalizeToolSpan` /
`finalizeBlockedSpan` private helpers ensures every terminal status path
also ends the corresponding span.

Eight terminal sites now finalize the tool span: signal.aborted at loop
entry, hard deny, plan-mode block, non-interactive deny, permission-hook
deny, background-agent deny, _schedule catch, executeSingleToolCall
finally. Five blocked_on_user end sites: handleConfirmationResponse cancel
and proceed branches, autoApproveCompatiblePendingTools, _schedule catch
under signal.aborted, and the global-error catch. ModifyWithEditor stays
inside one blocked_on_user span until the final proceed/cancel — the
duration_ms reflects total user think-time including editor side trips.

Six hook fire sites are wrapped: firePreToolUseHook, firePostToolUseHook,
and four safelyFirePostToolUseFailureHook variants (success-path
interrupt, toolResult.error path, catch-path interrupt, catch-path real
exception). fireNotificationHook is intentionally NOT wrapped — it's
fire-and-forget and the duration is meaningless.

Mirrors claude-code's session-tracing pattern but deliberately diverges on
one point: every end-helper takes the span object explicitly via
`getSpanId(span)` lookup instead of `findLast`-by-type. Under concurrent
tool calls, claude-code's findLast can end the wrong blocked span; passing
the ref directly is concurrency-safe.

Tests:
- session-tracing.test.ts: 11 new tests covering parent resolution
  (explicit parent for blocked_on_user, ALS-based for hook), idempotent
  end, NOOP behavior, error-status mapping, and a concurrency regression
  test (two parallel blocked spans ended in reverse order).
- coreToolScheduler.test.ts: mock extended with the four new helpers and
  two new metadata fields. New tests cover the tool span outliving a
  pre-hook deny path, blocked_on_user ending with cancel via the
  awaiting_approval flow, hook span recording shouldProceed=false /
  blockType='denied' on pre-hook block and shouldStop=true /
  blockType='stop' on post-hook stop, and a leak guard that asserts
  every recorded lifecycle span is ended after a successful tool call.

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

* fix(telemetry): address #4321 review — Copilot inline + code-reviewer + silent-failure-hunter

Eight discrete fixes plus two new tests, all surfaced in the Phase 2 review
rounds. Grouped here because they touch the same handful of code paths.

Copilot inline (#4321 PR):
1. startToolSpan attrs naming: drop redundant `tool_name` (helper already
   sets `'tool.name'` from the first arg) and rename `call_id` to the
   namespaced `'tool.call_id'`. Two sites: `_schedule` validating-loop
   start, and the defensive fallback in executeSingleToolCall. Without
   this, traces emit non-namespaced `tool_name` / `call_id` attributes
   that consumers grepping for `tool.call_id` miss.
2. PreToolUse hook span: propagate the actual `preHookResult.blockType`
   ('denied' / 'ask' / 'stop') instead of collapsing every block to
   'denied'. Also record `hasAdditionalContext` for parity with the
   PostToolUse / failure-hook spans.
3. blocked_on_user `source` detection: use `config.getIdeMode()` (best-
   effort) so IDE-driven decisions don't all show up as `'cli'`.
   Centralized in a new `getBlockedSource()` helper.

silent-failure-hunter / code-reviewer:
4. Hook span error-tracking is dead code. firePreToolUseHook /
   firePostToolUseHook / safelyFirePostToolUseFailureHook all swallow
   throws internally — every `catch (e) { endMeta = { error, ... };
   throw e }` block in the scheduler was unreachable. Simplify all 6
   sites to `try { ... } finally { endHookSpan(...) }`. The default
   `endMeta = { success: false }` keeps the span sensible if a future
   hook impl decides to throw.
5. handleConfirmationResponse had no error handling. modifyWithEditor /
   _applyInlineModify / attemptExecutionOfScheduledCalls can throw and
   would otherwise leak both the tool span and the blocked_on_user span
   until the 30-min TTL fires. Wrap the body in a try/catch that
   finalizes both spans on rethrow. Extracted the body to
   `_handleConfirmationResponseInner` for clarity.
6. Add `'error'` to the `ToolBlockedDecision` union for system-error
   closes, so dashboards counting `decision: 'cancel'` don't get
   polluted by thrown exceptions.
7. _schedule's outer catch was labelling its non-aborted close as
   `'cancel'`. Switch to `'error'` (uses #6).
8. signal.aborted vs explicit user Cancel: when both are true, the old
   code reported `'aborted'/'system'` even though the user actually
   clicked Cancel. Reverse the precedence so `outcome === Cancel`
   wins, with `getBlockedSource()` for the source.

Tests:
- T1: extend the existing ProceedAlways auto-approve test to assert the
  two siblings' blocked spans end with `decision: 'auto_approved'`,
  `source: 'auto'`, while the first tool ends as `'proceed_always'`/cli.
- T2: existing cancel-during-confirmation test now also asserts exactly
  one blocked span is recorded for the lifecycle — the same invariant
  ModifyWithEditor's intentional preservation across editor side trips
  must not break.

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

* fix(telemetry): close autoApprove blocked-span leak + cover three new behaviors

Two follow-ups from the post-#6767469b2 review pass on PR #4321:

1. autoApproveCompatiblePendingTools error path was logging-only and
   leaving the sibling tool's blocked_on_user span open until the 30-min
   TTL fires. Symmetric with the success branch's
   finalizeBlockedSpan('auto_approved', 'auto'), the catch now finalizes
   with ('error', 'system') so the trace deterministically explains why
   the sibling didn't auto-approve.

2. Three behaviors introduced by 6767469b2 had no test coverage:
   - decision='error' from _schedule's outer catch when
     getConfirmationDetails throws (asserts tool span ends, no blocked
     span ever opens since the throw happens pre-awaiting_approval).
   - source='ide' when getBlockedSource() honors getIdeMode (Cancel
     path with getIdeMode: () => true).
   - Explicit Cancel takes precedence over a concurrent signal.aborted
     in the decision label — the bug the precedence flip was meant to
     fix is now regression-tested.

Extracted a small `buildApprovalScheduler` helper for the two
awaiting_approval-flow tests; the throw-on-confirmation test reuses
StructuredErrorOnConfirmationTool.

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

* fix(telemetry): revert autoApprove catch finalizeBlockedSpan (#4321 codex P3)

The previous commit 32f94d348 added a `finalizeBlockedSpan(callId, 'error',
'system')` to the autoApproveCompatiblePendingTools catch in the name of
"symmetry with the success branch". Codex review pointed out the bug:
that catch fires when evaluatePermissionFlow throws for a SIBLING tool,
but the sibling itself is still in `awaiting_approval` — the user can
still respond. By closing the blocked span at the catch, the eventual
handleConfirmationResponse → finalizeBlockedSpan call becomes a no-op
(Map.delete already cleared it), and the user's actual decision /
source attributes are lost from the trace.

Revert that line. The previous behavior was correct: log the error,
leave the span open, let the user's eventual decision close it
correctly. If the user never responds, the 30-min TTL in
session-tracing.ts cleans up the orphan span — same fallback that
already covered every other "user walks away" scenario.

The "leak" the original change was trying to fix was a phantom: the
span IS finalized once the user (or the abort signal) drives the tool
to a terminal state. The TTL is just the safety net.

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

* fix(telemetry): split tool.failure_kind labels + cover proceed_once decision

Two #4321 review comments from wenshao, both Critical:

1. `TOOL_FAILURE_KIND_PRE_HOOK_BLOCKED` was being emitted for FIVE distinct
   non-PreToolUse-hook deny paths in `_schedule`:
   - finalPermission === 'deny' (hard deny)
   - plan-mode block
   - non-interactive deny
   - permission_request hook deny
   - background-agent deny
   Dashboards filtering by `failure_kind = 'pre_hook_blocked'` were
   silently picking up all of these, undermining the attribute. Add
   distinct constants + status messages for each path. The original
   PRE_HOOK_BLOCKED label is now used at exactly one site — the actual
   PreToolUse hook deny in `_executeToolCallBody`.

2. `decision: 'proceed_once'` was untested. Existing tests covered
   'cancel' and 'proceed_always' (auto-approve) but not the most common
   user interaction. Add a test that schedules an approval-required tool,
   confirms with ProceedOnce, and asserts the blocked span ends with
   `decision: 'proceed_once'`, `source: 'cli'`.

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

* fix(telemetry): address #4321 wenshao Critical + bot summary nits

Three review items folded into one follow-up:

1. wenshao Critical (`coreToolScheduler.ts:1851`) — `ModifyWithEditor`
   path silently returned when `getPreferredEditor()` was undefined,
   leaking blocked + tool spans on user-walks-away. Add a
   `debugLogger.warn` so the silent failure is at least visible in debug
   telemetry. Deliberately do NOT finalize spans here, matching the
   Codex P3 / autoApprove decision: ModifyWithEditor stays inside one
   awaiting period, the user can still recover via Cancel/Proceed which
   closes the spans correctly, and the 30-min TTL is the safety net for
   give-up scenarios. Finalizing prematurely would make the user's
   eventual decision a no-op (Map already cleared) and lose the actual
   decision/source attributes.

2. Bot summary Medium (`session-tracing.ts:557-562`) — add a
   `debugLogger.debug` when `startToolBlockedOnUserSpan` falls back to
   `resolveParentContext` because the tool span isn't in `activeSpans`
   anymore. Helps diagnose unexpected ordering during development.

3. Bot summary Low (`constants.ts`) — JSDoc the two new span name
   constants.

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

* refactor(telemetry): extract withHookSpan helper + drop dead finalizeToolSpan param

Two #4321 review Suggestions from wenshao:

1. The 6 hook fire sites (PreToolUse, PostToolUse, 4× PostToolUseFailure)
   each repeated the same try/finally + endMeta init + endHookSpan
   pattern. Future hook span protocol changes had to be made in lockstep.
   Extract a private generic helper:

       withHookSpan<T>(opts, fn, toEndMeta): Promise<T>

   Each fire site collapses from ~12 lines of try/finally scaffolding to
   ~3 lines passing in the fire callback + endMeta builder. The
   `let postHookResult!:` definite-assignment hack at the PostToolUse
   site is gone because the helper returns the awaited result directly.

2. `finalizeToolSpan(callId, metadata?)` had a dead `metadata`
   parameter — every caller pre-sets the span status via
   `setToolSpan{Failure,Cancelled}` and called `finalizeToolSpan` with no
   argument. Removed the parameter.

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

* fix(telemetry): hook span error tracking + TTL cleanup safety + call_id back-compat

Three #4321 review threads from wenshao (#4321 codex P3-equivalent +
two structural concerns):

1. **[Critical] Hook spans reported success on swallowed hook failures.**
   firePreToolUseHook / firePostToolUseHook /
   firePostToolUseFailureHook (and the safelyFire wrapper in
   coreToolScheduler) all catch transport / dispatch errors internally
   and return safe defaults. Before this fix, withHookSpan's `toEndMeta`
   ran on the safe default and recorded `success: true` — a crashing
   hook was indistinguishable from one that allowed execution.
   Add a `hookError?: string` field to the three result types, populate
   it in each catch, and have all 6 toEndMeta callbacks return
   `{ success: false, error: hookError }` when present.
   Existing "graceful error" tests updated to expect the new field.

2. **[Suggestion] ensureCleanupInterval not kicked from new helpers.**
   The 30-min TTL cleanup safety net for leaked spans only starts when
   `startInteractionSpan` is first called. Sub-agent or side-query code
   paths that call `startToolBlockedOnUserSpan` / `startHookSpan`
   without an interaction span first never trigger cleanup. Both
   helpers now call the (idempotent) `ensureCleanupInterval()` early.

3. **[Suggestion] `call_id` → `'tool.call_id'` rename is breaking for
   downstream consumers.** Phase 1's `startToolSpan(name, { tool_name,
   call_id })` shipped non-namespaced attribute keys. My Phase 2 #4321
   review-fix dropped both. Dual-emit `call_id` (legacy alias) +
   `'tool.call_id'` for one release cycle so existing dashboards /
   alerts don't silently return zero. Comment notes the legacy key is
   removed in the next release.

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

* fix(telemetry): close hookError plumbing gaps from final pre-merge audit

Final-pass review surfaced two gaps in the hookError contract added in
eafe68820:

1. **Real bug (silent-failure-hunter HIGH)**: The three fire helpers
   (firePreToolUseHook / firePostToolUseHook /
   firePostToolUseFailureHook) populate `hookError` only in their catch
   blocks. But the `if (!response.success || !response.output)`
   short-circuit at lines 121 / 220 / 299 silently dropped
   `response.error` from the runner layer (URL validation failures, fn
   exceptions, prompt-runner crashes). Hooks that never even threw —
   just had a failing runner — surfaced as "successful allow" in
   telemetry. Forward `response.error?.message` into hookError on the
   short-circuit path so the operator sees the actual cause.

2. **Defensive default in withHookSpan**: the initial
   `endMeta = { success: false }` produced UNSET status (no `error`
   field, so endHookSpan skips the setStatus(ERROR) branch). Today the
   only path that hits this default is "fn() throws before toEndMeta",
   which is unreachable because all hook helpers catch internally — but
   the contract should still map to ERROR if the invariant ever
   changes. Default now carries an explanatory error string.

Test: new `coreToolScheduler.test.ts` case where messageBus.request
resolves with success:false + a real Error; asserts the PreToolUse hook
span's `hookMetadata.error` is the runner's message (instead of being
silently absent).

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

* test(telemetry): cover #4321 rethrow path + 2 of the new failure_kind labels

Two test gaps surfaced by wenshao [Suggestion] threads:

1. **handleConfirmationResponse outer catch was untested.** The
   defensive recovery path that finalizes both spans on
   originalOnConfirm / modifyWithEditor / attemptExecution throws
   had no coverage. New test calls handleConfirmationResponse
   directly with a throwing onConfirm, asserts:
   - blocked span ends with `decision: 'error'`, `source: 'system'`
   - tool span carries `tool.failure_kind: 'tool_exception'`
   - the original error is rethrown to the caller

2. **5 new permission-flow failure_kind labels had zero
   coverage.** Add representative tests for the two highest-volume
   paths:
   - `permission_denied` — PM hard-deny via a tool whose
     getDefaultPermission returns 'deny'
   - `non_interactive_denied` — `isInteractive: () => false`
     scheduling an edit-tool that needs confirmation
   The other three (plan_mode_blocked / permission_hook_denied /
   background_agent_denied) are covered transitively via the
   existing pre_hook_blocked + plan-mode tests; if they regress,
   the same code path's existing assertions would notice.

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

* fix(telemetry): adopt 4 wenshao Critical/Suggestion findings on PR #4321

Inline review findings:
- coreToolScheduler.ts: signal.abort drains scheduler-local
  toolSpans/blockedSpans Maps via deferred setTimeout(0) — bridges the
  gap between session-tracing's 30-min TTL (which ends underlying spans
  but cannot reach the Maps) and walk-away-during-awaiting_approval. The
  drain is deferred so explicit Cancel via handleConfirmationResponse
  and mid-execution setToolSpanCancelled paths still win the race and
  set canonical labels.
- coreToolScheduler.test.ts: regression test for permission_hook_denied
  (firePermissionRequestHook deny branch at _schedule:1683) and
  background_agent_denied (getShouldAvoidPermissionPrompts auto-deny at
  _schedule:1697). Both branches were untested — silently dropping
  setToolSpanFailure on either would lose attribution.
- coreToolScheduler.ts: defensive-fallback span in executeSingleToolCall
  uses canonicalToolName(toolName) so dashboards grouping by span name
  don't see two entries for migrated/MCP tools whose canonical and raw
  names differ.

Review-body finding:
- session-tracing.ts: TTL safety net stamps qwen-code.span.ttl_expired
  + qwen-code.span.duration_ms attributes and emits a debug log before
  ending stale spans. Operators can now distinguish "abandoned and
  garbage-collected by the safety net" from "deliberately ended without
  status/attrs". Refactored cleanup loop into sweepStaleSpans(now) and
  exposed runTTLSweepForTesting for unit coverage.

Tests: +3 scheduler tests (~220 LOC), +2 session-tracing tests (~36
LOC). 247/247 in affected files.

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

* fix(telemetry): adopt 7 DeepSeek /review findings on PR #4321

Adopted ([Critical]):
- coreToolScheduler.ts: ModifyWithEditor `!editorType` path now sets
  `qwen-code.tool.modify_with_editor_unavailable: true` on the live tool
  span so operators can detect the silent-bail-out state in production
  traces without enabling debug logging.
- coreToolScheduler.test.ts: regression test for plan_mode_blocked
  failure_kind path (ApprovalMode.PLAN + non-read-only confirmation
  tool).
- coreToolScheduler.test.ts: regression test for the pre-aborted
  signal early-exit in `_schedule` — asserts
  setToolSpanCancelled (UNSET status) without entering execution.

Adopted ([Suggestion]):
- coreToolScheduler.ts: `withHookSpan` now `catch`-es and surfaces the
  actual thrown message instead of the hardcoded
  `'hook fn threw before toEndMeta'` sentinel. Currently unreachable
  (hook helpers swallow internally) but defensive against contract
  drift.
- coreToolScheduler.ts: re-add `tool_name` (non-namespaced) as a legacy
  alias on both startToolSpan call sites, mirroring the `call_id` /
  `tool.call_id` dual-emit window so pre-Phase-2 dashboards filtering
  on `tool_name` don't silently stop matching during the rollout.
- coreToolScheduler.test.ts: regression test for the
  `_schedule`-driven aborted decision label on the blocked_on_user
  span (companion to the existing tool-span drain test).
- coreToolScheduler.ts: PreToolUse / PostToolUse `toEndMeta` now
  include `shouldProceed: true` / `shouldStop: false` when `hookError`
  is set, mirroring the runtime's allow-on-hook-failure semantics.

Pushed back (separate PR-level reply):
- "sibling failure prematurely closes confirmed tool span" — not
  reachable: `_executeToolCallBody` swallows execution errors so the
  only paths into `handleConfirmationResponse`'s catch are
  `originalOnConfirm` / `modifyWithEditor` / `_applyInlineModify`,
  none of which run after `attemptExecutionOfScheduledCalls` started
  any sibling.
- "PostToolUseFailure hook spans not asserted" — broader scope, defer.
- "finalizeToolSpan accept required metadata" — invariant-redesign,
  out of scope for this PR.

Tests: +3 scheduler tests; 250/250 green in affected files
(coreToolScheduler 154 + session-tracing 49 + toolHookTriggers 47).

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

* fix(telemetry): adopt 3 wenshao /review findings on PR #4321

- coreToolScheduler.ts: handleConfirmationResponse outer catch now
  branches on signal.aborted — a throw caused by the abort signal
  (e.g. ModifyWithEditor child interrupted by Ctrl+C) lands as
  decision:'aborted'/UNSET status instead of 'error'/tool_exception,
  matching the sister catch in `_schedule` and keeping dashboard
  abort-vs-error counts honest (Critical-shaped Suggestion).

- coreToolScheduler.ts: drop the per-batch abort listener at the end
  of `_schedule` when no batch entries remain in toolSpans /
  blockedSpans. Prevents Node's MaxListenersExceededWarning in
  long-lived sessions where the same AbortSignal sees many _schedule
  batches without a real abort. Listeners that still cover
  awaiting_approval entries stay attached — the user's eventual
  decision closes the spans, and the listener becomes a no-op when it
  later fires (or auto-removes via `{ once: true }` on real abort).

- coreToolScheduler.test.ts: 2 regression tests for PostToolUseFailure
  hook span variants — `is_interrupt:true` on user-abort vs
  `is_interrupt:false` on real-exception. Operators rely on this flag
  to separate user-initiated cancellations from system errors in
  dashboards; a copy-paste regression flipping the value across the 4
  PostToolUseFailure call sites was previously invisible.

Tests: 252/252 across affected files (coreToolScheduler 156 +
session-tracing 49 + toolHookTriggers 47).

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

* fix(telemetry): adopt 7 wenshao /review round-3 findings on PR #4321

Adopted ([Critical]):

- coreToolScheduler.ts: full per-batch abort listener cleanup. Replaced
  the closure-local Set + end-of-_schedule cleanup with a class-level
  callIdToBatch Map keyed off a shared BatchAbortState. The listener is
  now released by `finalizeToolSpan` → `releaseBatchListenerIfDrained`
  whenever the last live batch entry drains, regardless of whether
  finalize happens synchronously inside _schedule, later via
  handleConfirmationResponse, or via executeSingleToolCall. Closes
  the awaiting_approval-batches-leak-listeners gap from the previous
  partial fix.

- coreToolScheduler.ts: re-check signal.aborted in the _schedule
  for-loop after `evaluatePermissionFlow`/`getConfirmationDetails`/
  `firePermissionRequestHook` and BEFORE setting awaiting_approval +
  starting the blocked span. Without this, a signal that aborts during
  one of those awaits opens a blocked span on an already-aborted
  signal whose drainSpansForBatch may have already fired, leaving the
  new entry permanently orphaned.

- session-tracing.ts: introduce truncateSpanError(s) (1KB cap) and
  apply it to every endXSpan site that writes metadata.error to span
  attributes / status messages (LLM, tool, tool execution, hook).
  Hook server responses, raw exception stacks, or hostile inputs can
  be unbounded; some OTel backends drop the entire span when any
  field exceeds their limit.

Adopted ([Suggestion]):

- coreToolScheduler.ts: per-callId try/catch inside drainSpansForBatch.
  One bad finalize no longer skips the rest of the batch; failures
  are logged via debugLogger.warn instead of bubbling up as an
  unhandled timer-callback exception.

- session-tracing.ts: TTL sweep robustness — wraps setAttributes and
  span.end() in separate try/catch blocks so a setAttributes throw
  can't leak the OTel span; stamps `decision: 'aborted'`/
  `source: 'system'` on TTL-expired blocked_on_user spans so
  dashboards filtering by decision count walk-aways consistently with
  explicit user aborts; includes tool.name + tool.call_id in the
  warn log so it's actionable in production without a trace-backend
  lookup.

- coreToolScheduler.ts: extract the 4 byte-identical PostToolUseFailure
  toEndMeta lambdas into a single `postToolUseFailureEndMeta` member.
  Future protocol changes only need to touch one place.

- coreToolScheduler.test.ts: 3 new tests
  * outer-catch aborted branch — pre-aborted signal + throwing
    onConfirm asserts decision='aborted'/source='system' and
    failure_kind='cancelled'.
  * ModifyWithEditor !editorType — uses a getModifyContext-shimmed
    MockEditTool to enter the modifiable branch and asserts
    qwen-code.tool.modify_with_editor_unavailable=true.
  * per-batch listener removed when batch drains synchronously —
    asserts AbortSignal listenerCount and `callIdToBatch` size.

Pushed back (deferred):

- "firePermissionRequestHook in withHookSpan + hookError field" —
  same as previous deferral. Touches the public PermissionRequestHookResult
  type re-exported from packages/core/src/index.ts; declined per the
  guardrail on public-API changes.

Tests: 255/255 across affected files (coreToolScheduler 159 +
session-tracing 49 + toolHookTriggers 47).

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

* fix(telemetry): polish 2 wenshao /review round-4 nits on PR #4321

- session-tracing.ts: rename `SPAN_ERROR_MAX_BYTES` → `SPAN_ERROR_MAX_CHARS`
  and update the JSDoc to be honest that `truncateSpanError` truncates by
  UTF-16 code units rather than bytes. CJK/emoji-heavy errors land in the
  ~2-3KB UTF-8 range under the same code-unit cap, but that's still well
  under all major OTel backends' per-attribute limits (Jaeger/Honeycomb
  ~64KB, OTLP default ~32KB), so we keep the simpler char-count bound
  rather than paying the encoder cost on every endXSpan.

- coreToolScheduler.ts: move the `withHookSpan` JSDoc block to sit
  directly above the method. The previous order had two consecutive
  JSDoc blocks separated by `postToolUseFailureEndMeta`, which orphaned
  the `withHookSpan` doc — IDE hover tooltips would surface the wrong
  documentation.

Tests: 208/208 in affected files; tsc --noEmit clean.

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

* fix(telemetry): adopt 4 wenshao /review round-5 findings on PR #4321

Adopted ([Suggestion]):

- coreToolScheduler.ts: `setToolSpanFailure` now applies
  `truncateSpanError` to the status message at this single ingress
  point. Many of its 10+ call sites pass raw `error.message` which can
  be unbounded — the same backend-drop risk that drove
  `truncateSpanError` for the endXSpan attribute writes. Static-
  constant callers see no change since their messages are well under
  the 1024-char cap. Required exporting `truncateSpanError` from
  `session-tracing.ts` and re-exporting from `telemetry/index.ts`.

- coreToolScheduler.ts: in `_schedule`, after the for-loop runs to
  completion, drop the abort listener if `batchState.callIds.size === 0`.
  Closes the all-error-batch leak path: if every newToolCall had
  `status !== 'validating'` (e.g., invalid params, tool not registered,
  queue full), no `finalizeToolSpan` ever fires for the batch and
  `releaseBatchListenerIfDrained` is never invoked. Without this drop,
  one dead listener accumulates per all-error batch.

- coreToolScheduler.ts: `handleConfirmationResponse` outer catch now
  emits a `debugLogger.warn` before rethrowing. Without it, if the
  caller (CLI confirmation UI layer) doesn't log the rejection, the
  error disappears from application logs entirely — operators
  grepping by callId would see nothing despite the trace backend
  showing `failure_kind: tool_exception`.

- session-tracing.test.ts: 4 new tests
  * `truncateSpanError` returns short strings unchanged
  * `truncateSpanError` truncates over 1024 chars + appends sentinel
  * `truncateSpanError` boundary at exactly 1024 chars
  * TTL sweep stamps `decision: 'aborted'` + `source: 'system'` on
    blocked_on_user spans (covers the branch added in review-3 round)

Pushed back ([Suggestion]):

- "TTL sweep can't reach scheduler-local Maps" — accurate but the fix
  is non-trivial: a parallel scheduler-side TTL sweep duplicates the
  session-tracing sweep's bookkeeping, and the practical impact is
  bounded (Maps die with the scheduler instance, which is per-session
  in CLI mode). The bigger leak (listener accumulation on shared
  signals) is already covered by `releaseBatchListenerIfDrained`.
  Marking as out-of-scope architectural follow-up.

Tests: 259/259 across affected files (coreToolScheduler 159 +
session-tracing 53 + toolHookTriggers 47). `tsc --noEmit` clean.

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

* fix(telemetry): adopt 1 wenshao /review round-6 finding on PR #4321

- coreToolScheduler.test.ts: convert the `truncateSpanError` mock from
  an inline identity function to `vi.fn(identity)` so individual tests
  can substitute a sentinel return. Added regression test
  `setToolSpanFailure forwards the truncateSpanError result to the span
  status (#4321)` that overrides the spy with `<<TRUNCATED-SENTINEL>>`,
  drives the scheduler through the pre-hook deny path, and asserts the
  span's ERROR status message equals the sentinel — locks the
  integration so a regression dropping the `truncateSpanError(message)`
  call inside `setToolSpanFailure` is caught at the scheduler boundary
  rather than only at the utility's unit test.

Tests: 213/213 across `coreToolScheduler.test.ts` (160) +
`session-tracing.test.ts` (53). `tsc --noEmit` clean.

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

* fix(telemetry): close 4 silent-failure + test-gap findings from final review on PR #4321

Comprehensive self-review (code-reviewer + silent-failure-hunter +
type-design-analyzer + pr-test-analyzer agents) after 6 rounds of bot
feedback turned up 4 remaining actionable items. Addressed:

[silent-failure-hunter HIGH-1] toolHookTriggers.ts: when the hook
runner returns `{ success: false }` (or missing output) with no
`error.message`, the 3 fire helpers used to silently return the safe
default — `{ shouldProceed: true }` / `{ shouldStop: false }` / `{}` —
producing a hook span that reads `success: true` and looked like a
clean allow in dashboards. Now synthesizes a sentinel hookError
describing the contract violation so the span records the failure.
Three existing test cases updated to assert the new sentinel-bearing
shape.

[silent-failure-hunter HIGH-2] coreToolScheduler.ts: synchronous
throws in `_executeToolCallBody`'s prelude (addToolInputAttributes,
getMessageBus, startToolExecutionSpan, etc.) propagated up to
`executeSingleToolCall`'s `finally` without ever hitting setToolSpan*,
so the tool span ended UNSET with no failure_kind AND the tool call
stayed in 'executing' forever (checkAndNotifyCompletion never sees
terminal state, scheduler hangs). Added a catch in
executeSingleToolCall that pre-sets failure status + an error response
before the finally finalizes — guards every prelude path the body's
own try/catch doesn't cover.

[silent-failure-hunter MEDIUM-3] session-tracing.ts: the empty catch
on `sweepStaleSpans` `setAttributes` lost the `ttl_expired` +
`decision: 'aborted'` sentinel attrs silently if setAttributes ever
threw. Now matches the sibling `span.end()` catch and surfaces via
`debugLogger.warn` — TTL-leaked blocked spans stay distinguishable
from deliberately-UNSET ones in dashboards.

[pr-test-analyzer Gap1, severity 7] coreToolScheduler.test.ts: the
`signal.aborted` re-check at `_schedule:1834` (round-3 fix that
prevents opening a blocked span on an already-aborted signal between
the for-loop's await points and the awaiting_approval transition) had
no regression test. Added one that uses a tool whose
`getConfirmationDetails` aborts the signal before returning — top of
loop check passes, getConfirmationDetails resolves and aborts, re-check
fires the cancel path. Asserts `tool.failure_kind === 'cancelled'` AND
that NO blocked_on_user span was ever started.

Tests: 261/261 across affected files (coreToolScheduler 161 +
session-tracing 53 + toolHookTriggers 47). `tsc --noEmit` clean.

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

* fix(telemetry): adopt 3 wenshao /review round-8 findings on PR #4321

All three from the same /review run; all valid (the Critical is a real
bug in the SF-H2 fix from review-7 that this commit fixes).

[Critical] coreToolScheduler.ts:2407 — the `c.status === 'executing'`
guard on the prelude-throw catch was wrong. Prelude throws happen
BEFORE the `scheduled → executing` transition in `_executeToolCallBody`
(getMessageBus is called at line 2460, scheduled→executing flips at
line 2522). The `find(... 'executing')` skipped the setStatusInternal,
so the toolCall stayed in `scheduled` forever and
checkAndNotifyCompletion never fired — exactly the stall the SF-H2 fix
was supposed to prevent. Drop the guard; setStatusInternal already
no-ops on terminal states (success/error/cancelled) so the
unconditional call covers both scheduled-prelude and executing-body
paths. Added regression test that makes getMessageBus throw and
asserts onAllToolCallsComplete fires with status='error'.

[Suggestion] session-tracing.ts:222 — truncateSpanError used
`slice(0, 1024)` on UTF-16 code units, which splits surrogate pairs
when an emoji (e.g. 🚀) or rare CJK character sits at the boundary.
The result was a lone high surrogate followed by `'…[truncated]'` —
strict OTLP/gRPC collectors reject batches with invalid UTF-8 (a lone
high surrogate encodes to an invalid byte sequence). Back up one code
unit when the cut lands on a high surrogate. Added regression test
that constructs the boundary case (1023 'a' + 🚀 + padding) and
asserts the truncated string is valid UTF-16.

[Suggestion] toolHookTriggers.ts:133/240/319 — switched `||` to `??`
in the 3 hookError sentinel sites. `||` treats empty string as falsy
so a runner returning `{ error: { message: "" } }` triggered the
sentinel instead of preserving the (unhelpful but distinct) empty
message — a runner contract violation that's worth distinguishing
from a missing-message case. `??` synthesizes only when the message
is truly absent (undefined / null).

Tests: 263/263 across affected files (coreToolScheduler 162 +
session-tracing 54 + toolHookTriggers 47). `tsc --noEmit` clean.

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

* fix(telemetry): adopt 3 wenshao /review round-9 findings on PR #4321

[Critical] coreToolScheduler.ts — `handleConfirmationResponse`'s catch
was misattributing sister-tool prelude throws to the confirmed tool's
span. The catch wrapped `_handleConfirmationResponseInner`, which
called `attemptExecutionOfScheduledCalls` at its tail. If the user
proceeds tool A with ProceedAlways, `autoApproveCompatiblePendingTools`
transitions sister tools B/C to `scheduled`, and B has a prelude
throw, the SF-H2 catch in `executeSingleToolCall` re-throws → the
throw propagates up through `attemptExecutionOfScheduledCalls` → into
the outer catch keyed on A.callId, where `setToolSpanFailure(A.span,
TOOL_EXCEPTION, B.error.message)` corrupts A's span and
`finalizeToolSpan(A.callId)` ends A's span prematurely. A's actual
result later disappears from telemetry. Fix: move
`attemptExecutionOfScheduledCalls` out of
`_handleConfirmationResponseInner` and into
`handleConfirmationResponse` after the try/catch. The catch now
covers only confirmation logic; each tool's
`executeSingleToolCall` already handles its own span lifecycle via
its own catch.

[Suggestion] toolHookTriggers.ts — reverted the round-8 `??` change
back to `||`. Downstream consumers in coreToolScheduler.ts gate on
`r.hookError ? ...`, so an empty-string `hookError` preserved by
`??` was silently dropped — the change defeated its own stated
intent. Empty-string runner error messages carry no operator value;
the sentinel ("hook runner returned ... without error detail") is
more actionable, and `||` matches existing downstream truthiness
semantics.

[Suggestion] session-tracing.test.ts — replaced the vacuous
`Buffer.from(truncated, 'utf16le')` assertion (which never throws
because Node's Buffer copies raw 16-bit code units without validating
surrogate pairs) with the suggested regex
`/[\uD800-\uDBFF](?![\uDC00-\uDFFF])/` that actually checks for
orphan high surrogates anywhere in the string.

Tests: 263/263 across affected files (coreToolScheduler 162 +
session-tracing 54 + toolHookTriggers 47). `tsc --noEmit` clean.

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

* test(telemetry): pin empty-string runner error sentinel behavior on PR #4321

[Suggestion] gpt-5.5 review-10: the round-9 `??` → `||` revert was
correct, but the existing tests only covered the missing-error case
(`success: false` with no `error` field). A future regression back to
`??` would still pass those tests while reintroducing the silent-drop
behavior the revert was guarding against.

Add 3 explicit tests — one per fire helper (PreToolUse, PostToolUse,
PostToolUseFailure) — that pass `{ error: { message: '' } }` and
assert the sentinel hookError is synthesized (not the empty string).
Pins the `||` semantics so any future `??` change fails the suite.

Tests: 50/50 in toolHookTriggers.test.ts (47 → 50). `tsc --noEmit`
clean.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
2026-05-21 11:55:50 +08:00
dreamWB
60d8ffae27
feat(cli): respect /editor preference in Ctrl+X external editor (#4310)
* feat(cli): respect /editor preference in Ctrl+X external editor

The Ctrl+X external editor prompt previously ignored the
general.preferredEditor setting, always falling back to $VISUAL/$EDITOR
env vars. Now it consults the preferred editor first, using the correct
--wait flags for GUI editors, and falls back to env vars only when no
preference is set or the preferred editor is unavailable.

Closes #4165

* fix(cli): address review feedback on external editor feature

- Fix command injection risk: quote args when needsShell is true
- Move writeFileSync inside try/finally with mode 0o600
- Change temp file extension from .md to .txt
- Extend needsShell check to cover .bat extension
- Fix import formatting in AgentComposer.tsx
- Extract usePreferredEditor hook to deduplicate validation
- Add 12 tests for openInExternalEditor covering all branches

* test(cli): add missing vi.mock for usePreferredEditor and useWorktreeSession

AppContainer.test.tsx mocks every hook that AppContainer.tsx imports,
but the two new hooks (usePreferredEditor from this PR,
useWorktreeSession from main's #4174) were not mocked — causing the
real hooks to execute during tests, crash on missing context, and fail
all 47 downstream assertions.

* fix(cli): address review feedback on env-var fallback and spawnSync timeout

- Detect .cmd/.bat in env-var fallback path on Windows and enable shell
  mode with quoted args, matching the preferred-editor path behavior
- Add 30-minute timeout to spawnSync to prevent terminal freeze when a
  GUI editor hangs
- Add test cases for both changes

* fix(cli): propagate preferredEditor to TextInput component

TextInput creates its own useTextBuffer but was not passing
preferredEditor, so Ctrl+X in secondary inputs (dialogs, settings
prompts, etc.) silently ignored the /editor preference.

* fix(cli): document why simple double-quoting is safe for shell args

The args passed to cmd.exe are program-controlled (tmpdir path + fixed
flags), never arbitrary user input. cmd.exe does not expand $() or
backticks inside double quotes. This matches Claude Code's approach.

* fix(cli): handle signal-killed editor and defer undo snapshot

- Check spawnSync signal field to avoid reading stale temp file
  when editor is killed by SIGTERM/SIGKILL
- Move undo snapshot creation after successful file read to prevent
  phantom no-op undo entries on editor failure

* fix(cli): restore private tmpdir, skip undo on unchanged content

- Restore mkdtempSync isolation directory (was flattened to os.tmpdir)
- Skip undo snapshot when editor content is unchanged
- Update JSDoc to reflect deferred-snapshot behavior
- Remove unused crypto import
- Add tests: unchanged content skip, tmpDir cleanup, undo precision

* fix(cli): use path.join in external editor tests for Windows compat

Tests hardcoded forward-slash paths which fail on Windows where
path.join produces backslashes. Use pathMod.join for the expected
temp file path so assertions pass on all platforms.

* fix(cli): quote editorCmd in shell mode, wrap setRawMode, improve logging

- Quote editorCmd along with args when shell: true, so Windows paths
  with spaces (e.g. C:\Program Files\...\code.cmd) survive cmd.exe.
- Wrap setRawMode restore in try/catch so a destroyed stdin doesn't
  skip temp file cleanup.
- Include command, shell mode, and resolution source in error log.
- Add tests: CRLF normalization, readFileSync failure, editorCmd quoting.

* refactor(core): remove unused isTerminal from ExternalEditorCommand

The field was never consumed by any caller — only command, args, and
needsShell are destructured. The standalone isTerminalEditor() function
already serves the same purpose for openDiff.

* docs(cli): update stale JSDoc on openInExternalEditor

Reflect the new editor resolution order (/editor → $VISUAL → $EDITOR → vi)
and the moved undo-snapshot timing (after editor exit, not before).

* fix(cli): address review round 3 — temp dir leak, mkdtemp safety, TextInput stdin

- Split unlinkSync/rmdirSync into separate try/catch blocks to prevent
  temp directory leak when unlinkSync throws (regression from main)
- Move mkdtempSync inside try block with early return on failure
- Pass stdin/setRawMode from TextInput to useTextBuffer so terminal
  editors (vim/neovim/emacs) correctly toggle raw mode via Ctrl+X

* test(cli): add undo-after-successful-edit test for external editor

* fix(cli): opts.editor priority, filePath in error log, warn on invalid editor

* fix(cli): address sandbox gap and Windows env-var safety in external editor

- usePreferredEditor now checks allowEditorTypeInSandbox() and returns
  undefined for GUI editors when SANDBOX env is set
- env/default editor fallback rejects commands containing " or | before
  enabling shell mode on Windows

* fix(cli): address wenshao review — unsafe-char guard, debug logs, test coverage

- Add unsafe-character rejection for opts.editor .cmd paths on Windows
- Change env-var unsafe-char handling from throw to graceful return + cleanup
- Add debug logging before spawnSync and in setRawMode catch block
- Add tests for opts.editor path, .cmd shell mode, and unsafe-char rejection

* fix(cli): expand unsafe-char guard, remove stale comment, add tests

- Expand Windows unsafe-character regex to include % and ! (cmd.exe
  variable expansion and delayed expansion)
- Remove stale "no hooks needed" comment in TextInput.tsx
- Add setRawMode lifecycle test (disable before editor, restore after)
- Add default fallback tests for vi (linux) and notepad (win32)

* fix(cli): remove explicit type annotation on mock.calls.findIndex callback

The `[boolean]` tuple annotation conflicts with vitest's `any[][]`
mock.calls type, causing TS2345 in CI.

* fix(cli): replace unlinkSync+rmdirSync with recursive rmSync for temp cleanup

Leftover swap files from vim/neovim would cause rmdirSync to silently
fail on non-empty directories, leaking temp dirs. Use rmSync with
recursive+force to handle this. Also fix stale JSDoc fallback comment.

* test(cli): add % and ! unsafe-char coverage and error-path raw mode test

- Expand opts.editor and env-var unsafe-char tests to cover %, !, and "
  independently via it.each, preventing silent regex regressions
- Add error-path test verifying setRawMode restore when editor exits
  with non-zero status
2026-05-21 10:50:10 +08:00
qqqys
c4421acd53
Expose active goal in stream JSON (#4314)
* feat(cli): expose active goal in stream json

* fix(cli): support goal clear messages in acp

* docs(cli): explain active goal stream events
2026-05-21 10:31:39 +08:00
qqqys
b588f74f64
fix(core): align session hook matcher targets (#4354)
* fix(core): align session hook matcher targets

* fix(core): share hook matcher target mapping

* fix(core): satisfy hook matcher exhaustiveness lint
2026-05-21 10:30:06 +08:00
易良
a3037889a6
fix(core): replace structuredClone with shallow copy to prevent OOM in long sessions (#4286)
* docs: add OOM investigation reports and auto-compaction redesign proposal

- Runtime memory investigation plan
- Non-interactive memory benchmark report
- OOM reproduction report with 2GiB/4GiB synthetic tests
- Runtime diagnostics benchmark report
- Auto-compaction threshold redesign proposal

* fix(core): replace structuredClone with shallow copy to prevent OOM

Replace `structuredClone(this.history)` (called up to 4x per turn on the
send path) with a lightweight shallow copy via `copyContentContainer()`.
This eliminates the OOM root cause in long tool-heavy sessions where the
full deep clone exceeded remaining V8 heap headroom.

Key changes:
- Add `copyContentContainer()` helper ({...content, parts: [...parts]})
- Add `getRequestHistory()` private method for the send path
- Add `getHistoryShallow()`, `getHistoryTailShallow()`,
  `peekLastHistoryEntry()`, `getLastModelMessageText()`,
  `getHistoryLength()` for read-only callers
- Remove HEAP_PRESSURE_COMPRESSION_RATIO safety net (no longer needed
  now that the underlying OOM cause is fixed)
- Update chatCompressionService to use getHistoryShallow(true)
- Update nextSpeakerChecker to send only lastMessage (not full history)
- Update memoryDiagnostics with process-tree RSS measurement

* feat(core): add runtimeDiagnostics utility for heap/memory instrumentation

Required by content generators (anthropic, openai, logging) which import
runtimeDiagnostics for optional heap-pressure telemetry during streaming.
Gated by QWEN_CODE_PROFILE_RUNTIME=1 environment variable.

* fix(cli): update doctorCommand test mocks for new MemoryDiagnostics interface

Add missing maxRSSRaw, maxRSSUnit, and processTree fields to test fixtures
to match the updated MemoryResourceUsage and MemoryDiagnostics interfaces.

* fix(vscode-ide-companion): use public core imports

* fix: address review comments — type guards, dead fallbacks, and doc accuracy

Code:
- Fix unsound type guard: `'text' in part` → `typeof part.text === 'string'`
  in geminiChat.ts and client.ts (Copilot + wenshao feedback)
- Remove unnecessary optional chaining and dead fallback chains in client.ts
  (getHistoryShallow, peekLastHistoryEntry, getHistoryLength, etc. now call
  GeminiChat methods directly)
- Add 5s timeout to `execFileAsync('ps', ...)` in memoryDiagnostics.ts

Docs:
- Fix GiB conversion accuracy and add single-run caveat to summary
- Add Node.js version to test environment table
- Fix auto-compaction attempt count (5→4) in OOM report
- Soften root-cause attribution certainty
- Add MCP child process context to investigation plan
- Clarify "Codex" reference (→ OpenAI Codex)
- Fix truncated MCP server name (chrome → chrome-devtools)
- Remove duplicate verification commands in benchmark table
- Clarify thread exhaustion vs V8 heap OOM distinction
- Add workload confound caveat to before/after comparison
- Fix SUMMARY_RESERVE "hard relationship" vs thinking budget contradiction

* fix(core): restore fallback chains in client.ts for mock compatibility

The previous commit removed optional chaining from client.ts wrapper
methods, but client.test.ts mocks getChat() with partial objects that
lack the new shallow methods. Restore ?. fallback chains so both
production (GeminiChat) and test (mock) paths work correctly.

* docs: clarify memory review follow-ups

* docs: fix runtime benchmark unit conversion

* docs: add default-heap OOM stress report

* fix: update copyright year to 2026 in new files [skip ci]

New files added in this PR had 2025 copyright headers. Updated to 2026
to reflect the current year.
2026-05-21 10:28:59 +08:00
pomelo
7c4b7f582a
fix(cli): remove QWEN_OAUTH gate from feedback dialog (#4316)
The feedback dialog (point-up/point-down) was only shown to users
authenticated via QWEN_OAUTH. With the QWEN_OAUTH free tier closed
on 2026-04-15 (#3203), the active user pool that can produce
feedback events has effectively drained, leaving the user_feedback
telemetry signal blind.

The reported payload only contains session_id, rating, model,
approval_mode, and prompt_id — no prompt content or other PII —
so there is no privacy reason to scope it to a specific auth
provider. Keep the existing usageStatisticsEnabled and
enableUserFeedback opt-ins, which already gate all telemetry.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-21 10:25:35 +08:00
顾盼
4b25f9c05c
fix(core): set x-api-key alongside Authorization on Anthropic outbound (#4323) (#4342)
Some checks are pending
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
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 / CodeQL (push) Blocked by required conditions
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* fix(core): set x-api-key alongside Authorization on Anthropic outbound (#4323)

On the IdeaLab-style proxy branch, the Anthropic SDK is constructed with
`authToken: <key>, apiKey: null` so it emits `Authorization: Bearer <key>`
and suppresses the ANTHROPIC_API_KEY env back-fill (the #4020 leak fix).
That covers IdeaLab and CherryStudio-style proxies, but standards-
compliant Anthropic-compatible servers (OpenCode-Go, Claude proxy
products) authenticate only on the canonical `x-api-key` header and
reject the request with "Missing API key" even though the bearer token
is present.

Inject `x-api-key: <key>` into `defaultHeaders` on the proxy branch
(post-`buildHeaders`, so customHeaders cannot override it). The value
is the user's already-configured `apiKey` — never an env-resolved one —
so the #4020 env-leak vector stays closed. The Anthropic-native branch
is untouched: the SDK's apiKey path already emits the header, and
duplicating it via defaultHeaders would risk stale-value drift.

Verified:
- new unit test pins `x-api-key: <key>` on every proxy-branch case
  (config-baseUrl, malformed baseUrl, DeepSeek anthropic-compat,
  ANTHROPIC_BASE_URL env-pointed-at-proxy); a negative test pins that
  the native branch does NOT add the header.
- E2E: spun up a local `http.createServer`, pointed the SDK at it the
  same way `AnthropicContentGenerator` does, and dumped the captured
  wire headers — `Authorization: Bearer` and `x-api-key` both arrive
  alongside the existing X-Stainless-* / x-app / claude-cli UA trio.

Fixes #4323

* fix(core): clarify x-api-key comment + cover guard branch & customHeaders ordering (#4323)

Address review feedback on #4342:

- Source comment claimed the apiKey value was "never an env-resolved
  one"; that's wrong — `resolveCredentialField` in
  content-generator-config.ts:178 falls through to env vars when the
  explicit and inherited values are unset. The security reasoning
  doesn't actually depend on that claim (the same value already ships
  as `Authorization: Bearer` via `authToken` on the same request), so
  re-anchor the comment on that fact and drop the misleading "never
  env-resolved" framing.

- Add test pinning the `&& contentGeneratorConfig.apiKey` guard: a
  falsy apiKey on the proxy branch must NOT inject `x-api-key:` (empty
  string would otherwise ship a meaningless header). The TypeScript
  signature `apiKey?: string` keeps the guard needed at the type level,
  but a future loosen-the-type refactor would silently re-enable the
  empty ship; the test catches that.

- Add test pinning the post-buildHeaders ordering: a user-supplied
  `customHeaders: { 'x-api-key': … }` must NOT win against the
  canonical key. The source comment promises this invariant but no
  test pinned it; a refactor that moved the injection above the
  customHeaders merge would silently let user config swap the auth
  header, defeating the dual-auth contract.

Declined two suggestions:
- Bot suggested extracting the 3-line injection into a `buildApiKeyHeader()`
  helper for consistency. Declined: adds indirection without abstraction
  win, and the inline form keeps the post-buildHeaders ordering visible
  at the call site (the ordering IS the invariant the comment promises).
- Bot suggested asserting `Authorization` is absent from `defaultHeaders`
  on the native path. Declined: the constructor-options pins
  (`apiKey: 'test-key'`, `authToken: null`) already document the
  SDK-driven auth mode; asserting on the absence of a header we never
  set in defaultHeaders is redundant given the existing assertions.

68 tests pass (66 + 2 new). tsc + eslint clean.
2026-05-21 10:06:25 +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
pomelo
a552df8998
refactor(auth): unify provider config in core, simplify /auth as "Connect a Provider" (#4287)
* refactor(providers): unify provider config into core, remove CLI re-exports

Move all ProviderConfig definitions, registry (ALL_PROVIDERS), and
utility functions (buildInstallPlan, resolveBaseUrl, etc.) from
packages/cli/src/auth/ into packages/core/src/providers/ so both
CLI and VSCode can share the same provider system.

- Add core providers module with types, presets, install logic
- Rewrite VSCode AuthMessageHandler to dynamically generate provider
  choices from ALL_PROVIDERS instead of hardcoding 3 providers
- Add applyProviderInstallPlanToFile in VSCode settingsWriter using
  the ProviderSettingsAdapter abstraction
- Delete 11 CLI re-export wrapper files, update ~20 import sites
- Keep CLI-specific applyProviderInstallPlan (uses LoadedSettings)
  and openrouterOAuth.ts (CLI-only OAuth runtime)

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

* refactor(cli): drop OpenRouter OAuth + /manage-models, simplify /auth

OpenRouter now uses the standard API-key flow under "Third-party Providers"
(issue #4108). The whole OpenRouter OAuth implementation (PKCE, callback
server, model auto-install) and the /manage-models command (only OpenRouter
was wired in; /auth Step 2 already covers model selection) are removed.

/auth is renamed around the "Connect a Provider" mental model:
- Dialog title is now "Connect a Provider"; the OAuth main entry is gone
- handleAuthSelect (mixed close + auth trigger) is split into a single-purpose
  closeAuthDialog; legacy wrappers (handleSubscriptionPlanSubmit,
  handleApiKeyProviderSubmit, handleCustomApiKeySubmit, ...) are dropped in
  favor of the unified handleProviderSubmit

Core: openRouterProvider switches to authMethod='input', uiGroup='third-party',
ships with two recommended free models, and is reordered to the end of the
third-party list to keep DeepSeek as the default highlight.

Net diff: 34 files, +124 / -3835.

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

* refactor(auth): unify applyProviderInstallPlan in core, drop cli/auth

CLI and vscode now share core's applyProviderInstallPlan instead of keeping
two parallel implementations. The CLI-only env rollback (snapshot
process.env, restore on error) is folded into the core version so vscode
also benefits from it.

CLI ships a LoadedSettingsAdapter that maps LoadedSettings to core's
ProviderSettingsAdapter contract. Backup/restore is layered: write a .orig
file, structuredClone settings + originalSettings, then recomputeMerged()
on restore — same guarantees as before, just routed through the adapter.

Tests for the install logic are migrated to core and rewritten against the
adapter mock (more focused than the previous LoadedSettings/Config mocks).

packages/cli/src/auth/ is gone entirely.

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

* refactor(providers): drop unused authMethod field from ProviderConfig

Every preset has had authMethod='input' since OpenRouter switched to the
standard API-key flow, making the field a dead dimension. Removing it
cleans up three never-taken branches and aligns the type with reality:
connecting a provider always means entering an API key.

- core: remove ProviderConfig.authMethod; shouldShowStep('apiKey') is
  now unconditionally true; drop authMethod from 9 presets
- vscode AuthMessageHandler: drop the OAuth branch in handleAuthInteractive
- vscode WebViewProvider: simplify the apiKey-required guard
- tests: update provider-config.test and custom-provider.test

If a future provider needs a browser-based flow, the field can be
re-introduced; for now the smaller surface is worth more.

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

* refactor(providers): prefix Alibaba plan presets with alibaba-

Rename coding-plan.{ts,test.ts} → alibaba-coding-plan.{ts,test.ts} and
token-plan.{ts,test.ts} → alibaba-token-plan.{ts,test.ts} so the file
names line up with the existing alibaba-standard preset and make it
obvious at a glance which presets belong to Alibaba ModelStudio.

Export names (codingPlanProvider, tokenPlanProvider, TOKEN_PLAN_*,
CODING_PLAN_*) are unchanged — only the file paths and the two
imports in all-providers.ts / index.ts move.

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

* fix(vscode): guard ProviderSettingsAdapter against prototype pollution

The dotted-key writer in createFileSettingsAdapter walked through any
segment, including __proto__/constructor/prototype, which would let a
malicious or malformed ProviderInstallPlan reach Object.prototype.

Refuse to write paths containing reserved segments and use
hasOwnProperty when traversing intermediate objects so that inherited
properties cannot redirect the walk.

Addresses CodeQL alert #226 surfaced on PR #4287.

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

* fix(auth): default Audio modality to off in provider advanced config

In the /auth Custom Provider advanced-config step, "Enable modality"
should default to Image + Video only. Audio was on by default, which
implied the model accepts audio input even though most providers
people configure here don't.

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

* fix(auth): show base URL default as placeholder, not prefilled value

In Custom Provider Step 2/6 (and on protocol switch), the base URL
input started with the protocol's default URL pre-filled. Users who
wanted a non-default endpoint had to manually clear the field first.

Switch to placeholder semantics: the input starts empty, the default
URL is shown as a hint, and submitting blank falls back to that
default (then writes it back to baseUrl so downstream steps see a
real value).

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

* refactor(cli): rename /auth description to "Connect an LLM provider"

The old description ("Configure authentication information for login")
implied a Qwen-account login. After the /auth refactor it's really
about picking an LLM provider and entering credentials, so the menu
entry should say that.

Also add 'connect' as an alt-name alongside the existing 'login' so
users can type /connect when 'auth' feels wrong. Keep 'login' for
muscle-memory compatibility.

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

* i18n(cli): translate "Connect an LLM provider" in all locales

Strict-parity locales (zh, zh-TW) require every built-in command
description to be translated; the renamed /auth description was
falling back to English and breaking the must-translate test.

Add translations for zh / zh-TW (required) and refresh the other
seven locales (en, ru, de, ja, fr, ca, pt) so the old
"Configure authentication information for login" key is removed
everywhere rather than left as a dangling dictionary entry.

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

* fix(vscode): await applyProviderInstallPlanToFile and grow test coverage

Critical: applyProviderInstallPlanToFile fired the install plan with
`void`, so any rejection (EACCES from persist(), prototype-pollution
guard throw, etc.) was silently swallowed and WebViewProvider proceeded
to disconnect/reconnect the agent as if the write had succeeded.
Make the wrapper `async` and `await` it in the only caller.

Tests added:
- core/install.test: isSameModelIdentity fallback path
  (prepend-and-remove-owned with no ownsModel) — verifies models are
  matched on id+baseUrl, not just id.
- vscode/AuthMessageHandler.test: happy-path with a fixed-baseUrl
  third-party provider, validateApiKey error branch, and BaseUrlOption
  picker presentation.

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

* fix(auth): address PR #4287 review (critical + suggestion)

vscode AuthMessageHandler (Critical):
- Add the missing protocol-selection step so custom-provider users can
  pick Anthropic/Gemini instead of being silently locked to OpenAI.
- Validate free-form base URL with the same /^https?:\/\// check the
  CLI uses; reject file:/javascript: schemes.

vscode AuthMessageHandler (Suggestion):
- Stop filtering separator entries from the provider QuickPick so
  groups (Alibaba Cloud / Third Party / Custom) actually show as
  headers instead of a flat list.
- Treat a null authInteractiveHandler as an error: surface an
  authError + cancellation notification instead of silently dropping
  the user's input.
- Call notifyAuthCancelled when validateApiKey rejects so the
  webview state resets and the user can retry.

core/providers/presets/openrouter.ts (Critical):
- Replace the substring includes() in ownsModel with a URL-hostname
  match so paths like https://api.example.com/openrouter.ai/v1 stop
  being misidentified as OpenRouter models (and getting removed on
  re-install).

vscode/services/settingsWriter.ts (Critical):
- stripTrailingCommas() so JSONC files with trailing commas (VSCode's
  default style) parse instead of silently returning {} and then
  overwriting the entire settings file.
- readSettings() distinguishes ENOENT (return {}) from parse errors
  (log + rethrow) so a malformed file never gets clobbered.
- writeSettings() writes through a temp file + fs.renameSync atomic
  rename, eliminating the half-written file window on EACCES /
  disk-full / crash.
- setValue() refuses to overwrite a scalar at an intermediate path
  segment (would have silently destroyed e.g. {"env": "legacy-string"}).

core/providers/install.ts (Suggestion):
- Move settings.backup?.() inside the try block so a backup failure
  still triggers the env-rollback path in catch.

cli/config/loadedSettingsAdapter.ts (Suggestion):
- Add the same UNSAFE_KEY_PARTS guard the vscode adapter has, so
  __proto__/constructor/prototype segments are rejected before
  reaching the underlying setNestedPropertySafe walker. Defense in
  depth: not exploitable today but the utility has no built-in guard.

vscode/webview/providers/WebViewProvider.ts (Suggestion):
- Hoist buildInstallPlan / applyProviderInstallPlanToFile to static
  imports (both modules already top-level imported); drops two
  per-call await import() round-trips.

cli/utils/doctorChecks.ts (Suggestion):
- Whitespace nit before the comma in the qwen-code-core import.

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

* fix(auth): second round of PR #4287 review fixes

Critical:
- settingsWriter: stripTrailingCommas now uses a char-by-char scanner so
  literal ",]" inside a string value is preserved (the previous regex
  silently corrupted it).
- install.ts: wrap settings.restore() in try/catch so a restore failure
  doesn't mask the original error or skip the env-rollback loop.
- install.ts: snapshot the runtime ModelProvidersConfig before applying
  patches and reload it in the catch path, so an in-flight refreshAuth()
  failure doesn't leave the live session holding providers that were
  never successfully installed.
- AuthMessageHandler: custom-provider Base URL is now a placeholder
  instead of a pre-filled value, with the default selected by the
  user's chosen protocol (openai/anthropic/gemini). Empty input falls
  back to the protocol-appropriate URL, preventing the
  pick-Anthropic-but-keep-OpenAI-URL footgun.

Suggestion:
- AuthDialog: replace the isCurrentlyCodingPlan misnomer with a uiGroup
  check — resolveMetadataKey returns config.id for *any* provider with
  a static models[], so the old guard made DeepSeek/MiniMax/OpenRouter
  users land on the Alibaba tab instead of Third-party Providers.
- AuthMessageHandler: guard against modelIds being [] after splitting
  comma input (matches the CLI's "Model IDs cannot be empty.").
- WebViewProvider: restore the explanatory comment for the
  authState === true success-toast guard that the previous diff
  accidentally dropped.

Tests:
- settingsWriter.test: new applyProviderInstallPlanToFile suite covering
  happy path, prototype-pollution guard (built via Object.defineProperty
  to bypass __proto__ literal semantics), intermediate-scalar rejection,
  malformed-file no-clobber, JSONC-with-trailing-commas parsing
  (including a string containing ",]"), and the atomic-write tmp-file
  cleanup.
- loadedSettingsAdapter.test: new file — forwarding, UNSAFE_KEY_PARTS
  rejection, getValue against merged settings, backup/restore round-trip,
  cleanupBackup semantics.
- provider-config.test: added findProviderByCredentials and
  getAllProviderBaseUrls coverage (preset hits, unknown-key misses,
  BaseUrlOption[] preset expansion).

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

* fix(cli): satisfy strict tsc --build in loadedSettingsAdapter.test

CI's `tsc --build` (with emit) enforced two strict checks that
`tsc --noEmit` had been letting through:

- `noPropertyAccessFromIndexSignature` flagged `file.settings['env']`
  reads against `Record<string, unknown>`. Switched the test fixture
  shape to a named `SettingsShape` interface with explicit `env` and
  `modelProviders` keys (plus an index signature for setValue's
  arbitrary writes), so dot access on the known keys is no longer
  "through" the index signature.
- Calling optional methods via `adapter.backup?.()` produced TS2722
  (`Cannot invoke an object which is possibly 'undefined'`) under the
  build flags. createLoadedSettingsAdapter always installs
  backup/restore/cleanupBackup, so the tests now assert
  `toBeTypeOf('function')` first and then call via non-null assertion,
  which both documents the invariant and makes the call typesafe.
- Dropped the `({} as Record<string, unknown>)['polluted']` sanity
  check; `expect(setValue).not.toHaveBeenCalled()` already proves the
  guard short-circuits before any write reaches LoadedSettings.

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

* fix(cli): guard mock setValue against prototype pollution in adapter test

CodeQL flagged the mock setValue's recursive property assignment as a prototype-pollution sink. Add UNSAFE_KEY_PARTS check at the top of the mock to align with the real setNestedPropertySafe contract.

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

* fix(cli): use literal === guards for CodeQL prototype-pollution sanitiser

CodeQL re-flagged the mock setValue write even after the Set.has guard added in 2e6adf8a6d — the scanner only recognises inline literal === comparisons as prototype-pollution sanitisers, not Set lookups.

Reworked the mock to (1) merge the guard into the loop so every current[part] write is preceded by a literal === check against '__proto__'/'constructor'/'prototype', and (2) collapse the dual leaf/branch logic into a single loop body. Runtime behaviour is identical; CodeQL should now treat the write as sanitised.

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

* fix(auth): third round of PR #4287 review fixes (8 comments)

Critical:
- useAuth: handleProviderSubmit now calls setPendingAuthType at the start
  of the try, so handleAuthFailure can record the AuthEvent telemetry on
  applyProviderInstallPlan rejection (previously dropped silently because
  pendingAuthType was undefined).
- settingsWriter: readQwenSettingsForVSCode wraps readSettings in
  try/catch so a malformed settings.json no longer crashes the VSCode
  extension on activation; the write paths (writeCodingPlanConfig,
  writeModelProvidersConfig) deliberately keep propagating to avoid
  silently overwriting a corrupt file with partial data.

Suggestions:
- settingsWriter.setValue: intermediate-segment guard now also rejects
  arrays (typeof [] === 'object' previously slipped through and would
  let us set string keys on an array). Loop restructured so the
  literal-=== prototype-pollution guard runs at every step, satisfying
  CodeQL's sanitiser detector on both the leaf and intermediate writes.
- settingsWriter atomic write: SETTINGS_FILE_MODE = 0o600 +
  SETTINGS_DIR_MODE = 0o700 + best-effort chmod on existing files. API
  keys persisted into env.* are no longer world-readable on multi-user
  systems.
- loadedSettingsAdapter: switched its prototype-pollution guard to the
  same inline literal === pattern so the two adapters stay symmetric
  and CodeQL recognises both as sanitisers (Comment 6 — explicit
  'keep in sync' comment + same shape rather than a shared helper that
  CodeQL wouldn't trace through).
- AuthMessageHandler: protocol QuickPick now shows 'OpenAI Compatible'
  / 'Anthropic' / 'Gemini' instead of the raw AuthType enum values.
- WebViewProvider: authInteractive log now records only the parsed
  hostname, not the full inputs.baseUrl, so credentials embedded in
  userinfo or query strings don't leak into extension-host logs.

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

* test(auth): cover the rollback safety nets in applyProviderInstallPlan + useAuth failure path

Addresses the missing-coverage points in the latest review pass: every deliberately-engineered rollback path in install.ts and the visible side effects of handleAuthFailure now have a regression test, so a future refactor that 'simplifies' these paths can't silently break them.

applyProviderInstallPlan (install.test.ts, +4 cases):
- restores runtime model providers when refreshAuth rejects after
  reloadModelProviders ran (asserts the second reloadModelProviders call
  receives the pre-install snapshot).
- still rolls back env vars when backup() throws before persist (pins
  the 'backup inside try' invariant added in 38a214d0ec).
- continues env rollback even when settings.restore itself throws
  (pins the nested try/catch around restore added in 38a214d0ec).
- continues throw + env rollback when the rollback-time
  reloadModelProviders itself throws (the original error must still
  surface; env vars must still revert).

useAuth (useAuth.test.ts, +1 case):
- surfaces install-plan rejection as an auth error and records
  telemetry — refreshAuth throws, the test asserts authError is set,
  the dialog reopens, isAuthenticating clears, no success toast is
  added, and pendingAuthType is populated (which is what the new
  setPendingAuthType call lets handleAuthFailure key the AuthEvent on).
- createSettings now mocks recomputeMerged + forScope.settings so the
  loaded-settings-adapter restore() path doesn't emit a noisy stderr.

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

* fix(auth): fourth round of PR #4287 review fixes

Critical:
- settingsWriter JSONC scanner: \uXXXX is a 6-char escape, not 2.
  The previous stripJsonComments / stripTrailingCommas used j+=2 for
  every backslash, so a value containing \u0022 would let the embedded
  quote terminate the string early — turning a single string value into
  multiple top-level keys after the strip passes. That's a parser
  differential vs JSON.parse and enables settings.json key injection
  (e.g. an attacker-controlled API_KEY string could inject env.NODE_OPTIONS).
  Now we branch on text[j+1] === 'u' and skip 6, satisfying both scanners.
- resolveBaseUrl no longer crashes on an empty baseUrl array. The
  previous config.baseUrl[0].url threw 'Cannot read undefined.url' on []
  and brought down the whole install flow. Falls back to selectedBaseUrl
  or '' instead.
- providerMatchesCredentials now resolves function-typed envKey by
  calling it with (protocol, baseUrl). The previous typeof-string gate
  made the custom provider invisible to findProviderByCredentials —
  /doctor and system-info diagnostics couldn't see custom-provider users.
  Catches the function call so a misbehaving custom envKey can't crash
  the matcher.

Suggestions:
- AuthDialog: defaultMainIndex now also returns 2 for uiGroup === 'custom'
  so a custom-provider user lands on the Custom Provider tab instead of
  Alibaba ModelStudio.
- install.ts: env-var rollback loop is now wrapped in try/catch matching
  the same shape as the settings.restore() and reloadModelProviders
  rollbacks. A process.env write throwing (custom property descriptors,
  some sandboxes) won't skip the runtime-providers rollback below.
- readSettings: SyntaxError is now wrapped in an actionable Error
  ('Cannot parse ~/.qwen/settings.json ($name: $message). Standard
  JSONC is supported... Please fix or delete $path...') so users facing
  a corrupt file get a clear message instead of a bare SyntaxError. The
  cause is preserved via Error.cause.

Tests:
- settingsWriter: new \u0022 injection regression — asserts that a
  string containing \u0022 stays a single string and no injected key
  lands at the top level.
- provider-config: new edge-case suite for resolveBaseUrl with [] and
  providerMatchesCredentials with function-typed envKey (matching path,
  wrong-key path, function-throws path). Re-imports via the relative
  source path so the new behaviour is exercised even before dist/ is
  rebuilt.

Not addressed:
- handleProviderSubmit error-path test (Comment 3264567491) was already
  added in 7d8b4785ad — same test, same surface (refreshAuth rejection
  + authError set + dialog reopen + isAuthenticating false + no success
  toast + pendingAuthType populated).

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

* fix(vscode): import AuthType as value not type

AuthMessageHandler now references AuthType.USE_OPENAI etc. as enum values (for the protocolLabels map added in cdc17cbba0), but the import was 'import type AuthType' which strips the runtime binding. TS1361 fired in CI's emitting build even though --noEmit was happy locally.

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

* fix(providers): restore modelscope test + tighten openrouter ownsModel

Two findings from the latest /review pass that survived earlier rounds:

1. modelscope.test.ts was deleted in the move-from-CLI step (60 lines / 4 cases under packages/cli/src/auth/providers/thirdParty/) but never recreated in core's preset test folder. Re-added a 3-case suite (config shape, install plan with per-model metadata for known IDs, graceful fallback for unknown IDs) so the third-party preset coverage is symmetric again. Also exported modelscopeProvider from packages/core/src/providers/index.ts so the public API matches the other presets.

2. openrouter.ts ownsModel previously claimed any model on an openrouter.ai hostname, which would silently delete a user's hand-added entry that happened to route through openrouter.ai under a different envKey (e.g. a personal gateway). Now requires both model.envKey === OPENROUTER_ENV_KEY AND the openrouter.ai hostname match. Existing openrouter.test.ts updated and extended to cover: matching path, envKey mismatch path, host mismatch path, missing/malformed baseUrl.

The remaining findings in that /review were either already addressed in earlier rounds (custom provider visibility / resolveBaseUrl empty array / useAuth telemetry / TS4111 errors — verified 0 locally) or architectural concerns beyond this PR's scope (LoadedSettings.setValue's per-call saveSettings).

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

* fix(auth): fifth round of PR #4287 review fixes

Critical:
- provider-config.ts providerMatchesCredentials: iterate config.protocolOptions
  when resolving a function-typed envKey instead of relying on the default
  config.protocol. A custom provider configured under USE_ANTHROPIC or
  USE_GEMINI persists an envKey derived from THAT protocol, not from
  USE_OPENAI — without iteration the matcher silently misses them and
  custom-provider users disappear from /doctor + AppHeader +
  systemInfoFields + AuthDialog.defaultMainIndex.
- provider-config.test.ts: the existing test asserting 'returns false for
  function-typed envKey' was holding on the old broken behaviour. Flipped
  to assert toBe(true) for the matching path, and routed it through the
  relative source import so it doesn't run against stale dist.

Suggestions:
- settingsWriter.clearPersistedAuth: now wipes every preset's string envKey
  (iterates ALL_PROVIDERS, plus the existing subscription-plan loop kept
  for explicitness) and every QWEN_CUSTOM_API_KEY_* key by prefix match.
  Previously DeepSeek / MiniMax / Z.AI / IdeaLab / ModelScope / OpenRouter
  / custom keys lingered on disk after clearing auth.
- custom-provider.ts generateCustomEnvKey: the readable-only normalization
  collapsed 'api.example.com', 'api-example.com', and 'api_example.com'
  into the same env key, so two structurally different custom providers
  would overwrite each other's API key. Now appends a 6-hex-char SHA-256
  suffix derived from (protocol, baseUrl-with-trailing-slash-stripped).
  The trailing-slash invariant from the prior implementation is preserved
  (api/v1 and api/v1/ still hash equal). Suffix collision probability at
  6 hex chars is ~1/16M per pair — fine for an interactive flow.

Tests:
- provider-config.test.ts: added a 'iterates protocolOptions' case that
  configures a custom-style provider, derives the key under
  USE_ANTHROPIC, and asserts the matcher finds it.
- custom-provider.test.ts: regex-matches the new readable+hash format
  for the deterministic / special-character / empty-string cases, and a
  new 'disambiguates structurally distinct URLs that normalize
  identically' case that pins down the collision fix
  (api.example.com vs api-example.com vs api_example.com all differ).

Not addressed:
- TS1361 'type AuthType' import — already fixed in 8f94b018bd
- modelscope re-export — already fixed in 7228d73d80

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

* fix(custom-provider): replace polynomial regex with linear char scans

CodeQL alerts 225 + 232 flagged `/_+/g`, `/^_+|_+$/g`, and `/\/+$/` in generateCustomEnvKey as polynomial regex on user input. V8 handles these patterns linearly in practice, but the scanner can't see that and any baseUrl with many '_' or '/' would be flagged as a theoretical worst case.

Replaced both passes with single-pass character scans:

- normalizeEnvSegment: walks the string once, emits alphanumerics verbatim, collapses any non-alphanumeric run to a single '_', then trims leading/trailing underscores via charCodeAt index walks. Equivalent to the prior three regexes but with no quantifier backtracking surface.

- stripTrailingSlashes: walks backwards from the end while charCodeAt === 47, then slices. Equivalent to `replace(/\/+$/, '')`.

All 11 custom-provider tests still pass — output format and invariants (trailing-slash equivalence, hash suffix, protocol/URL disambiguation) are unchanged.

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

* fix(auth): seventh round of PR #4287 review fixes

Critical:
- i18n: 9 locale files updated to replace orphaned 'Select Authentication
  Method' / 'You must select an auth method...' keys with the new
  'Connect a Provider' / 'You must connect a provider...' keys the
  AuthDialog actually references. Non-English users no longer see the
  English fallback for the main heading + exit-prevention warning.
- settingsWriter.writeSettings: renameSync is now wrapped in try/catch
  that unlinks the temp file on failure (EPERM/EBUSY on Windows from
  watchers/AV would otherwise orphan a secret-bearing .tmp file in
  ~/.qwen on every failed write).
- settingsWriter.restore(): write to disk FIRST, then update in-memory
  data. The previous order left memory clean while disk retained the
  failed install's partial state if writeSettings threw. Now matches
  the CLI adapter's order.
- AuthMessageHandler custom-provider tests: added 4 cases covering
  protocol picker → free-form URL → API key → comma-split model IDs →
  advanced config (one happy path), plus the http(s) scheme guard, the
  protocol-aware blank-URL fallback, and the whitespace-only model
  IDs guard. Previously the entire custom path through
  runProviderSetupFlow had zero coverage.
- settingsWriter clearPersistedAuth tests: added cases for the
  expanded preset/custom/subscription cleanup (asserts NODE_OPTIONS
  survives, every QWEN_CUSTOM_API_KEY_* is wiped, providerMetadata
  entries for every preset are gone) plus a no-settings-file no-op.

Suggestions:
- loadedSettingsAdapter.restore(): now checks restoreSettingsFromBackup's
  boolean return value and logs an explicit warning when on-disk rollback
  fails (EACCES / missing .orig). Previously the failure was silent and
  the next CLI restart would read a corrupted file.
- generateCustomEnvKey: hash suffix lengthened from 6 → 12 hex chars
  (24 → 48 bits). Brings collision search out of milliseconds-range
  enumeration; offline 'pick a URL that collides' attack is no longer
  practical at interactive setup time.
- getDefaultBaseUrlForProtocol: new shared helper in core consumed by
  both the CLI (useProviderSetupFlow) and VS Code (AuthMessageHandler)
  flows. Removes the duplicated DEFAULT_BASE_URLS map; one source of
  truth for the OpenAI/Anthropic/Gemini placeholder URLs.
- settingsWriter.clearPersistedAuth: providerMetadata cleanup now
  iterates ALL_PROVIDERS with resolveMetadataKey instead of hardcoding
  coding-plan/token-plan. Stale metadata for deepseek/minimax/zai/
  idealab/modelscope/openrouter no longer lingers after logout.
- resolveMetadataKey: explicit guard against provider ids containing
  '.'. A dotted id would split into multiple nested objects under
  providerMetadata, silently corrupting the settings tree. Now throws
  loudly at registration time.
- customProvider: added explicit ownsModel that prefix-matches against
  QWEN_CUSTOM_API_KEY_*. Reinstalling a custom provider under a
  different baseUrl now reliably replaces (not accumulates) the old
  entries.

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

* fix(auth): eighth round of PR #4287 review fixes

Suggestions:
- clearPersistedAuth metadata cleanup loop: per-iteration try/catch
  around resolveMetadataKey so a future dotted-id provider can't abort
  the loop and leave secrets on disk.
- VS Code AuthMessageHandler: removed the hardcoded
  || 'https://api.openai.com/v1' fallback after
  getDefaultBaseUrlForProtocol — defaults must live in core. The CLI
  flow has no such fallback, and the silent OpenAI default would mask
  a new AuthType core hadn't been taught about.
- settingsWriter restore() comment: clarified the deliberate divergence
  from the CLI adapter's trade-off (disk-fail-throws here, disk-fail-
  logs-and-continues there) so the comment doesn't read 'same order'.
- useAuth handleAuthFailure: closure staleness — setPendingAuthType
  queues an async React update, so handleAuthFailure's pendingAuthType
  read could see undefined when a synchronous throw beats the next
  render. Added an optional protocolForTelemetry argument that the new
  handleProviderSubmit passes explicitly; closure fallback kept for
  legacy callers. AuthEvent error telemetry is no longer silently
  dropped.
- install.ts: track currentStep before each phase (backup → env →
  modelProviders → authType → legacyCredentials → modelSelection →
  providerState → persist → reloadModelProviders → syncAuthState →
  refreshAuth → cleanupBackup) and annotate the rethrown error with
  the failing step + authType. Original error preserved via Error.cause
  so callers matching on err.code still work.
- custom-provider.ts: stale '6-hex-char' comment updated to 12. Added
  a migration note explaining that old 6-char keys persist as harmless
  orphan disk state until clear-auth.
- settingsUtils.restoreSettingsFromBackup: was swallowing fs errors
  with catch(_e); now logs the underlying cause so the adapter's
  on-disk-rollback-failed warning has something specific to point at.

Tests:
- useAuth: new cancelAuthentication case asserts isAuthenticating
  clears, externalAuthState clears, dialog opens, authError clears.
- provider-config: new resolveMetadataKey suite — normal id, no-models
  → undefined, dotted id → throws.
- install: new case asserting the rethrown error names the failing
  step ('refreshAuth') + authType and preserves the original error
  via Error.cause.

Not addressed:
- 6→12 hash backward compat (Comment 3267562667): The 6-char keys are
  orphan disk state — never read by applyProviderInstallPlan (the new
  model provider entries reference the new 12-char key), so no security
  or correctness issue, just disk noise that clears on next sign-out.
  Documented in custom-provider.ts. A full clean-up pass would need a
  new ProviderSettingsAdapter delete API + a migration scan — better
  as its own PR.
- writeSettings renameSync error path test + loadedSettingsAdapter
  restore-failure log test (terminal-only findings): adding these
  requires fs mocking surgery that's worth its own PR.

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

* fix(format): four prettier/JSDoc nitpicks from review

All four are Critical-tagged formatter / docs issues caught by the latest /review pass:

- AppHeader.tsx: `AuthType ,` (stray space before comma) → standard newline-after-{ form. Was breaking CI Lint.
- useProviderUpdates.test.ts: same `AuthType ,` pattern → standard form.
- apiPreconnect.ts: double blank line after the closing `}` of the
  import block (left behind when getAllProviderBaseUrls was removed
  from the old auth/allProviders path) → single blank line.
- types.ts (Suggestion): JSDoc for `modelsEditable` said
  "false → skip model step; use models as-is (e.g. Coding Plan)" but
  codingPlanProvider actually sets modelsEditable: true (every preset
  in the registry does), so the example contradicts the registry.
  Dropped the parenthetical.

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

* test(scripts): raise install-script suite timeout to survive Windows

Windows CI flaked on `standalone release packaging > rejects unexpected dist assets` with a 5000ms timeout. The test shells out to `node scripts/create-standalone-package.js` which produces a tar.gz; observed real runtimes from sibling tests in the same run: 4780ms / 1666ms / 1079ms — the 4.8s case is already at vitest's default 5s limit, so a slightly slower subprocess startup (antivirus inspection, contended runner) tips it over.

Pre-existing test (added 2026-05-11 in cb7059f54d), unrelated to this PR's auth refactor. Bumped the suite-wide testTimeout to 30s in scripts/tests/vitest.config.ts — the tests still complete in seconds when subprocess startup is healthy; the headroom only kicks in to cover Windows-slow variance.

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

* fix(auth): ninth round of PR #4287 review fixes

Critical:
- WebViewProvider.handleAuthInteractive: roll back bad credentials when the
  agent reconnect rejects them. applyProviderInstallPlanToFile commits the
  key + calls cleanupBackup before the disconnect/reconnect runs, so the
  plan's own rollback can't cover an authState=false outcome. Now snapshot
  settings before the write (snapshotSettingsForRollback) and restore it
  (restoreSettingsSnapshot) on both the authState!=true branch and the
  catch branch. Without this a rejected key persisted and every VS Code
  restart retried it. Two new helpers added to settingsWriter; never-throw
  snapshot so a malformed pre-state degrades to a no-op restore.

Suggestions:
- AuthMessageHandler: trim the API key before validateApiKey + persistence,
  matching the CLI flow (useProviderSetupFlow trims in two places). A key
  pasted with trailing whitespace no longer causes silent auth failures or
  VS-Code-only validateApiKey rejections.
- install.ts: the annotated rethrow no longer bakes 'step "persist"' into
  the user-facing message. Step + authType are now structured properties on
  a new exported ProviderInstallError (message stays the underlying error
  text, cause preserved). Callers can show a clean message and log
  err.step/err.authType to the dev console.
- provider-config.ts: providerMatchesCredentials no longer swallows a throw
  from a function-typed envKey — console.warn surfaces the programming
  error so a custom provider silently vanishing from /doctor has a trace.
- types.ts: documented that ProviderSettingsAdapter.setValue MAY flush to
  disk eagerly (the CLI LoadedSettings adapter does) and that persist() can
  be a no-op for such adapters — so future authors don't insert pre-persist
  steps assuming atomicity.
- settingsWriter: moved the orphaned stripJsonComments JSDoc off
  jsonEscapeLength (the \u-escape helper inserted between the doc and its
  function) back onto stripJsonComments itself.

Tests:
- settingsWriter: snapshot/restore round-trip, malformed→null→no-op-restore,
  no-file→{} snapshot.
- install: updated the step-annotation test to assert err.step/err.authType
  structured properties + clean message instead of the embedded string.
- WebViewProvider.test: settingsWriter mock extended with
  applyProviderInstallPlanToFile/snapshotSettingsForRollback/
  restoreSettingsSnapshot.

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

* fix(auth): tenth round of PR #4287 review fixes

Critical (both from the previous round's own changes):
- WebViewProvider.handleAuthInteractive: restoreSettingsSnapshot →
  writeSettings can throw (EPERM on Windows renameSync / disk full /
  EACCES). Both rollback call sites are now routed through a local
  safeRollback() that try/catches and logs, so a rollback failure can
  never (a) re-throw out of the else-branch into the outer catch and
  trigger a second rollback that skips the error message, nor (b) throw
  out of the catch-branch and leave the webview auth dialog hanging with
  no feedback.
- provider-config.providerMatchesCredentials: the new envKey-throw
  console.warn logged the full baseUrl, which can embed credentials
  (https://user:sk-secret@host). Now logs only new URL(baseUrl).hostname
  (with an [invalid] fallback) and err.message, matching the
  sanitization WebViewProvider already uses.

Tests:
- WebViewProvider.test: new 'credential rollback' describe with three
  cases — (1) authState!==true after reconnect → restoreSettingsSnapshot
  called with the snapshot, (2) authState===true → restore NOT called,
  (3) restore throws (EPERM) → handleAuthInteractive still resolves and
  the authError message is still sent. Hoisted mocks extended with
  applyProviderInstallPlanToFile / snapshotSettingsForRollback /
  restoreSettingsSnapshot refs so the scenario is controllable.

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

* fix(auth): eleventh round of PR #4287 review fixes

Critical:
- AuthMessageHandler: validation-failure paths (bad URL scheme, invalid
  API key, empty model IDs, handler-not-set) no longer call
  notifyAuthCancelled after sendToWebView({authError}). The webview's
  ProviderSetupForm clears the error on authCancelled, so the two
  messages raced and the error flashed away before the user could read
  it. authCancelled is now reserved for genuine user dismissals (Escape
  on a QuickPick/InputBox); authError already clears the connecting state.
- WebViewProvider: after rolling back rejected credentials, also
  disconnect the agent. The reconnect spawned a process holding the bad
  key in memory; without disconnect a subsequent chat message hit a
  stale-credential error unrelated to the original auth failure. Now
  agentManager.disconnect() + agentInitialized=false so the next /auth
  reconnects cleanly.

Suggestions:
- install.ts: added a DENY_ENV_KEYS denylist (NODE_OPTIONS, NODE_PATH,
  LD_PRELOAD, LD_LIBRARY_PATH, DYLD_INSERT_LIBRARIES, DYLD_LIBRARY_PATH,
  PATH, HOME, TMPDIR), checked case-insensitively before writing any
  plan.env entry to settings + process.env. Defense in depth: all callers
  go through buildInstallPlan with hardcoded keys today, but
  ProviderInstallPlan is exported.
- settingsUtils: setNestedPropertySafe AND setNestedPropertyForce now
  refuse __proto__/constructor/prototype path segments (inline literal
  === so CodeQL recognises the sanitiser). migrateProviderMetadata feeds
  field names from Object.entries on user settings.json, and JSON.parse
  keeps __proto__ as an own property — guarding at the utility protects
  every caller, not just the adapters.

Already fixed in f31224bac1 (review ran against 9f45a7536b):
- restoreSettingsSnapshot throw masking the original error → safeRollback.
- baseUrl logged verbatim in providerMatchesCredentials → hostname only.

Tests:
- install: NODE_OPTIONS rejected + not leaked to process.env/settings;
  case-insensitive Path rejection.
- AuthMessageHandler: validation authError is NOT followed by
  authCancelled.
- WebViewProvider: rollback path disconnects the agent + clears
  agentInitialized.
- settingsUtils: setNestedPropertySafe/Force refuse __proto__/
  constructor/prototype and don't pollute Object.prototype.

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

* fix(test): use bracket access in settingsUtils prototype-pollution tests

The new setNestedProperty guard tests asserted obj.a.b.c / obj.x.y dot-access on Record<string, unknown>, which trips noPropertyAccessFromIndexSignature (TS4111) under the emitting tsc --build the CI 'Install dependencies' step runs. Local npm run typecheck (--noEmit) had a stale tsbuildinfo and didn't re-check the file. Switched to bracket access (obj['a']['b']['c']) to match the strict option. Behaviour unchanged; 78 settingsUtils tests still pass.

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

* test(vscode): cover the outer-catch rollback path in handleAuthInteractive

All prior rollback tests exercised the else-branch (authState !== true). The outer catch — reached when applyProviderInstallPlanToFile or doInitializeAgentConnection throws (disk errors, partial writes) — had no coverage, and that's the higher-risk path. New test makes doInitializeAgentConnection reject and asserts (1) restoreSettingsSnapshot called with the snapshot, (2) authError sent containing 'Configuration failed', (3) handleAuthInteractive resolves without throwing. Guards against a regression that drops the safeRollback wrapper in the catch.

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

* test(providers): make NODE_OPTIONS denylist test env-independent

The test asserted process.env.NODE_OPTIONS toBeUndefined after the rejected plan, but CI sets NODE_OPTIONS (--max-old-space-size=3072 from the build script), so it failed there while passing locally where NODE_OPTIONS is unset. Snapshot the original value and assert the rejected plan left it UNCHANGED (and specifically not the evil --require value) — that's the actual invariant: the denylist throws before mutating process.env.

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

* fix(vscode): disconnect stale agent in handleAuthInteractive catch block too

The else-branch (authState !== true) disconnected the agent after rollback, but the outer catch only rolled back. If doInitializeAgentConnection partially initializes (agentInitialized=true, agent process spawned) then throws — e.g. a disk error during post-connect setup — the stale-credential agent stayed connected.

Extracted a disconnectStaleAgent() local helper (alongside safeRollback) and called it in both the else-branch and the catch, so the two paths are symmetric. Extended the outer-catch test to spawn a partial agent before the throw and assert disconnect() is called + agentInitialized cleared.

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

* fix(auth): twelfth round of PR #4287 review fixes (5 suggestions)

All from DeepSeek's pass, all on recent commits:
- settingsUtils: stale comment referenced a non-existent UNSAFE_PATH_SEGMENTS const; the actual guard is pathHasUnsafeSegment(). Fixed both comment sites.
- settingsWriter.snapshotSettingsForRollback: was silently returning null on a readSettings throw (disabling credential rollback with no signal). Now console.warn's the cause so oncall can tie repeated cross-restart auth failures back to a transient unreadable settings file.
- provider-config.providerMatchesCredentials: the envKey-throw warn logged err.message, which a user-defined envKey fn could populate with the API key (new Error(`bad config: ${apiKey}`)). Now logs only err.constructor.name — no message, no URL.
- install.ProviderInstallError: was an interface (erased at compile time → instanceof always false). Converted to a class extending Error so instanceof works at runtime; exported as a value (not type) from the barrel. Construction simplified to new ProviderInstallError(msg, step, authType, { cause }).
- install.DENY_ENV_KEYS: added Windows TMP/TEMP alongside TMPDIR so a crafted plan can't redirect temp-file creation on Windows.

Tests:
- install: assert the thrown error is instanceof ProviderInstallError; new it.each covering TMP/TEMP/tmp rejection (case-insensitive).

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

* fix(vscode): log error class name not message in snapshotSettingsForRollback

Consistency with the err.constructor.name approach applied in provider-config.providerMatchesCredentials. The risk here is lower (the catch is filesystem errors from readSettings/structuredClone, not user-defined functions), but logging only the class name keeps the security stance uniform across the codebase.

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

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-05-20 23:48:52 +08:00
DennisYu07
aedbf37b9d
feat(core): inject git status into system prompt and refine Explore/git-log guidance (#4110)
* add system prompt for codebase task

* update prompt snapshot

* fix test

* resolve comment
2026-05-20 23:27:00 +08:00
易良
1a59f207bf
chore: add .github/release.yml to support skip-changelog label (#4327)
* chore: add .github/release.yml to support skip-changelog label

* chore: add comments explaining release.yml purpose

* fix(lint): quote string value in release.yml for yamllint
2026-05-20 22:30:52 +08:00
Shaojin Wen
96fa0616c6
fix(review): harden SKILL.md against weak-model rule skipping (#4340)
* fix(review): harden SKILL.md against weak-model rule skipping

Weak models often skip parts of the long /review prompt and fall back
to familiar defaults — `gh pr checkout` instead of the worktree flow,
or running the autofix prompt even when the user passed `--comment`
(which means "only post inline comments, don't mutate code").

Three reinforcements, all in SKILL.md (no CLI changes):

- Promote the two most commonly violated rules to the top of the
  "Critical rules" list: worktree is mandatory for PR reviews, and
  `--comment` skips Step 8 entirely.
- Add an inline blockquote at the top of the Step 1 PR branch that
  names the specific forbidden commands (`gh pr checkout`,
  `git checkout`, `git switch`, `git pull`, `git reset --hard`).
- Add an explicit skip block at the top of Step 8 listing the three
  conditions that bypass autofix — `--comment`, cross-repo lightweight
  mode, or no fixable findings — so a weak model doesn't have to
  infer them from scattered earlier text.

* fix(review): address /review comments on rule scope + Step 8 dedup

Follow-up to the initial harden pass, addressing the inline review
comments on PR #4340.

Rule #1 (worktree mandatory):
- Scope it to **same-repo PR reviews** so cross-repo PRs running in
  lightweight mode (no matching local remote, no worktree) don't read
  as a contradiction.
- Replace "Your very first action" with "After argument parsing and
  remote detection, the first command that touches code state" — the
  literal "very first" was wrong since `--comment` parsing and
  URL/remote disambiguation legitimately run before `fetch-pr`.
- Align the forbidden-command list with the Step 1 blockquote (add
  `git pull` and `git reset --hard`) so a weak model that only reads
  the Critical rules section sees the same five commands as a model
  that reaches the blockquote at the point of use.
- Add an explicit "cross-repo PRs use lightweight mode" parenthetical
  so the same model knows where to look for the alternative path.

Step 8 skip block:
- Drop the redundant third bullet ("no Critical or Suggestion findings
  with concrete, applicable fixes") — it was both logically equivalent
  to the "Otherwise" clause below and used a different qualifier
  ("concrete, applicable" vs "clear, unambiguous"), risking a weak
  model treating them as two distinct thresholds.
- "ANY of the following" → "EITHER" since only two bullets remain.
- Fold the no-findings case into the Otherwise clause as a no-op note.
2026-05-20 22:29:59 +08:00
Shang Yuanchun
d97b85f2cf
Pin fetch to bundled undici for undici higher versions compatibility (#4238)
* fix: pin fetch to bundled undici for Node.js 26 (undici 8.x) compat

Node.js 26 bundles undici 8.x, which differs from the project's undici 6.x.
Using Node's built-in fetch mixed with ProxyAgent/Client from the bundled
undici causes handler-interface mismatches (e.g. 'invalid onError method').

* fix(core): export undici fetch alongside proxy dispatcher to avoid version mismatch

for review of #4238

When a custom dispatcher (ProxyAgent) is passed, pin fetch to the bundled undici's implementation so both share the same undici version. Without this, Node's built-in fetch (e.g. undici v8) rejects a ProxyAgent from the bundled undici (e.g. v6) with "invalid onError method".

* fix: move pinning fetch alongside with dispatcher in runtimeOptions, change back default.ts

* docs(core): update code comment reference in runtimeFetchOptions test
2026-05-20 22:26:05 +08:00
Shaojin Wen
16f0fde19a
fix(test): raise timeout for Windows installer end-to-end tests (#4352)
* fix(test): raise timeout for Windows installer end-to-end tests

The Windows-only end-to-end installer tests spawn cmd.exe to run the
.bat installer and then qwen.cmd --version, which boots a Node process.
On GitHub's windows-latest runners that chain regularly takes >5s, so
the default 5s vitest timeout makes them flaky (recently observed at
5804ms on CI). Bump the describe-block timeout to 30s, which leaves
headroom without masking real regressions.

* fix(test): raise timeout for Linux/macOS installer end-to-end tests

Match the timeout already applied to the Windows e2e block: the
Linux/macOS installer tests also spawn child processes via
execFileSync, so they share the same flake risk near the default 5s
vitest timeout. 15s leaves ample headroom without Windows' cmd.exe
overhead.

Addresses review feedback on #4352.
2026-05-20 17:42:29 +08:00
kkhomej33-netizen
dc6a5ad50a
feat(cli): add session path status command (#4124)
Some checks are pending
Qwen Code CI / Classify PR (push) Waiting to run
Qwen Code CI / Lint (push) Blocked by required conditions
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Blocked by required conditions
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* feat(cli): add session path status command

* fix(cli): add status paths translations

* fix(core): use secure subagent id suffix

* fix(cli): harden status paths log lookup

* fix(cli): use secure prompt id randomness

* test(cli): cover status paths formatting
2026-05-20 16:33:19 +08:00