Commit graph

6036 commits

Author SHA1 Message Date
yao
77458ad21f
feat(input): move physical cursor to visual cursor for IME input (#4652)
* feat(input): add useCursor hook for IME physical cursor tracking

* refactor(input): optimize cursor positioning effect

* feat(input): move setCursorPosition to render phase for immediate cursor positioning

* fix(input): calculate absolute cursor position by walking yoga tree

* fix(input): use addLayoutListener instead of useCursor for zero-jitter cursor positioning

* perf(input): stable addLayoutListener subscription and skip redundant cursor updates

* fix(input): revert lastPos dedup that broke cursorDirty one-shot flag

* feat(input): use patch-package to expose Ink internals for IME cursor positioning

* fix(input): address review feedback — prefixWidth, remove useBoxMetrics, pin ink version

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

---------

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-06-03 09:45:59 +08:00
Yufeng He
7ee37277de
fix(cli): honor list extensions flag (#4673) 2026-06-03 09:23:47 +08:00
顾盼
c03610b7a7
feat(core,cli): auto-compact follow-up — /compress instructions, PreCompact hook plumb, plan/subagent attachments (#4688)
* feat(cli,core): /compress accepts custom focus instructions

Extends /compress to take a trailing instruction string (max 2000 chars)
that is passed through tryCompressChat → tryCompress → CompressOptions
and appended to the compression side-query system prompt as an
"Additional Instructions:" block. Mirrors claude-code /compact <text>.

Empty / whitespace-only args fall back to the prior behaviour.

* test(core): cover /compress customInstructions + PreCompact hook merge

* feat(core): restore plan-mode + subagent snapshot after compaction

Adds two optional ComposePostCompactOptions:
- planModeActive: when true, emits a <plan-mode-active> reminder so the
  post-compact agent does not forget destructive tools remain gated.
- runningSubagents: when non-empty, emits a <background-tasks> block
  listing each running/paused task by id, status, and description.

Both blocks are spliced into the merged user attachment Content before
file/image restorations. XML-significant characters in descriptions are
escaped to prevent an adversarial subagent description from closing the
wrapper tag.

Wiring at the call site arrives in the next commit.

* feat(core): wire plan-mode + subagent snapshot into post-compact attachments

ChatCompressionService.compress now passes:
- planModeActive: derived from config.getApprovalMode() === ApprovalMode.PLAN
- runningSubagents: filtered from BackgroundTaskRegistry to agent-kind tasks
  in 'running' or 'paused' state

into composePostCompactHistory. Adds collectActiveSubagents() helper that
returns [] when the registry is absent so older SDK consumers without it
keep working.

* fix(core,test): use HookSystem.getAdditionalContext accessor; smoke-test /compress

- chatCompressionService: read PreCompact hook output via
  result.getAdditionalContext() — the wrapper returns DefaultHookOutput
  (not the raw AggregatedHookResult), and the accessor sanitises < / >
  consistently with every other call-site in the repo.
- Test mocks now return a DefaultHookOutput-shaped stub via a tiny
  makeHookOutput() helper rather than the aggregator shape.
- New integration smoke test for `/compress focus on the scientist
  mentioned` exercising the args plumbing end-to-end.

* test(core): update client.test.ts to match new tryCompressChat signature

* feat(core): cap subagent snapshot at 30 entries with overflow notice

Code-review follow-up. Pathological sessions with hundreds of
backgrounded agents could otherwise produce a multi-KB block. Newest 30
rows are kept (highest startTime); older ones are summarised on a
trailing line so the model knows the snapshot is partial.

* fix(core,test): flatten subagent description newlines; type-safe ApprovalMode in tests

Second code-review pass found two real issues:

1. Subagent descriptions containing `\n`/`\r`/`\t` would split across
   multiple lines inside the `<background-tasks>` bullet list, letting
   the second line read as a sibling row (or worse, an orphan paragraph
   between two `- [..]` entries). Flatten whitespace before the slice so
   each task stays on one line.

2. The plan-mode wiring tests passed `'plan'` / `'auto-edit'` as plain
   strings instead of `ApprovalMode.PLAN` / `ApprovalMode.AUTO_EDIT`.
   Source code compares against the enum; a future enum value change
   would have silently passed the tests. Import and use the enum.

* fix(core): move PreCompact hook fire after length-guard; align plan-mode tool names

Round 3 code review surfaced two issues:

1. PreCompact hook fired BEFORE the curatedHistory.length < 2 guard, so
   a single-message session would trigger any hook side effects
   (transcript dump, external notification, etc.) and then NOOP. Move
   the hook fire below the guard so hooks only run when compression is
   actually possible. New regression test asserts the contract.

2. PLAN_MODE_REMINDER_TEXT said "shell mutations" but the real qwen-code
   tool is `run_shell_command` (tool-names.ts:26). Use the verbatim
   tool names so a future rename is grep-discoverable.

* refactor(core): share escapeXml, drive plan-mode names from ToolNames, extract reminder builder

Code-review follow-ups on post-compact attachments:
- Replace the local 3-char escapeForXmlText with the shared 5-char
  escapeXml from utils/xml.ts, and apply it to the subagent id and
  status as well as the description. Subagent ids derive from a
  user-configurable subagentConfig.name, so an unescaped `<`/`&` there
  could close the <background-tasks> wrapper or forge sibling markup.
- PLAN_MODE_REMINDER_TEXT now interpolates ToolNames.WRITE_FILE / .EDIT /
  .SHELL instead of retyping the names, so a future rename stays in sync.
- Extract buildStateReminderParts() as the single source of truth for the
  plan-mode + subagent reminder blocks, used by both composePostCompactHistory
  and (next commit) its catch-fallback so the two paths can't drift.

* fix(core): scope subagent snapshot to backgrounded tasks; restore reminders on fallback; cap hook context

Three code-review fixes in the compaction service:
- collectActiveSubagents now also requires isBackgrounded — foreground
  agents are the parent's synchronously-awaited tool call and don't belong
  in a <background-tasks> roster. Mirrors getRunningBackgroundCount.
- The composePostCompactHistory catch-fallback re-applies the plan-mode +
  subagent reminders via the shared buildStateReminderParts (pure, no I/O),
  so a restoration failure no longer silently drops plan-mode enforcement
  and the subagent roster.
- The PreCompact hook's additionalContext is capped at
  MAX_HOOK_INSTRUCTIONS_CHARS before entering the side-query prompt,
  closing the unbounded-input hole the user-text cap was meant to prevent.

The fallback and hook-cap fixes have RED-verified regression tests.

* feat(cli): warn on /compress instruction truncation; fix integration-test pty typing

- /compress now emits an INFO notice (interactive), a stream message (acp),
  and a prefixed return message (non-interactive) when the instruction
  string exceeds MAX_COMPRESS_INSTRUCTIONS_CHARS, so the silent 2000-char
  clip is no longer invisible to the user.
- Annotate the three `ptyProcess.onData((data: string) => ...)` callbacks
  in the compress integration test to clear the TS7006 implicit-any the
  reviewer's typecheck flagged (fixed all three occurrences, not only the
  one inside this PR's diff).
2026-06-03 09:23:00 +08:00
Yan Shen
68408c30c3
feat(cli): Add searchable MiniMax-M3 model setup (#4668)
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(cli): unblock third-party model setup for MiniMax-M3

Keep CLI provider setup focused on issue #4663 by combining free-form model entry with searchable recommended selections, preserving custom-provider multi-model entry, and carrying saved credentials through model switches.

Constraint: Issue #4663 requires MiniMax-M3 metadata, searchable recommended model selection, and manual model IDs without VS Code scope changes.

Rejected: VS Code auth-flow edits | User narrowed scope to CLI-only behavior.

Confidence: high

Scope-risk: moderate

Directive: Keep Custom Provider multi-model entry separate from built-in provider recommended-model selectors.

Tested: cd packages/cli && npx vitest run src/config/auth.test.ts src/ui/auth/ProviderSetupSteps.test.tsx src/ui/components/ModelDialog.test.tsx src/ui/components/shared/TextInput.test.tsx

Tested: cd packages/core && npx vitest run src/core/modalityDefaults.test.ts src/core/tokenLimits.test.ts src/models/modelRegistry.test.ts src/models/modelsConfig.test.ts src/providers/__tests__/presets/minimax.test.ts src/providers/__tests__/provider-config.test.ts

Tested: git diff --check; npm run lint; npm run typecheck; npm run build

Not-tested: Full package test suite on Windows due existing symlink permission / unrelated failures noted in review.

* test(cli): prevent Windows CI races in prompt suggestion submit

Constraint: Windows CI can lag Ink/React render settling after follow-up debounce.

Rejected: Longer real-time sleeps | still flaky and slower under runner load

Confidence: high

Scope-risk: narrow

Directive: Prefer timer-driven state transitions over fixed sleeps in InputPrompt tests.

Tested: cd packages/cli && npx vitest run src/ui/components/InputPrompt.test.tsx

Not-tested: Full cross-platform CI matrix
2026-06-03 02:11:18 +08:00
Edenman
6f6b326d63
docs: add /diff command and auto theme detection documentation (#4699)
Some checks are pending
Qwen Code CI / Classify PR (push) Waiting to run
Qwen Code CI / Lint (push) Blocked by required conditions
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Blocked by required conditions
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* docs: add /diff command documentation to commands.md

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

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

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

* docs: fix checkpointing default description in /diff section

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

* docs: fix file checkpointing default in /diff section

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

---------

Co-authored-by: 克竟 <dingbingzhi.dbz@alibaba-inc.com>
2026-06-02 21:28:34 +08:00
jinye
27a7e8a781
feat(core): atomic write rollout for credentials, memory, config, JSONL (closes #3681, #4095 Phase 2) (#4333)
* feat(core): add atomicWriteFileSync + forceMode option

Sync mirror of atomicWriteFile for code paths that can't await
(settings persistence on exit, sync config writers). Same semantics:
symlink chain resolution, permission preservation, fsync via flush:true,
EPERM/EACCES rename retry, EXDEV fallback to direct write.

Add forceMode option on AtomicWriteFileOptions — when true, ignore the
existing target's permission bits and apply options.mode regardless.
Needed for credential files that must heal historically over-permissive
files (e.g. a 0o644 token restored from backup must be forced to 0o600).
Honored by both async and sync paths. Default false preserves existing
behavior.

Reuses Atomics.wait for true blocking sleep in renameWithRetrySync —
no busy-wait, no extra dep.

Refs: #4095 Phase 2

* refactor(core): migrate credential writes to atomicWriteFile (#4095 Tier 1)

Route all OAuth credential persistence through atomicWriteFile with
forceMode: true, so a process crash mid-write cannot leave the user
with a half-written token file, and historically over-permissive files
(e.g. 0o644 from a manual restore) are healed to 0o600 on the next
write.

- oauth-token-storage.ts: setCredentials, deleteCredentials
- file-token-storage.ts: saveTokens (encrypted MCP token storage)
- qwenOAuth2.ts: cacheQwenCredentials (also fixes missing mode — was
  inheriting 0o644 from umask, now forced to 0o600)
- sharedTokenManager.ts: saveCredentialsToFile — drops ~15 lines of
  hand-rolled tmp + rename in favor of the shared helper

Lock-file writes using flag: 'wx' (sharedTokenManager.ts:720) are
intentionally left untouched — they rely on exclusive-create semantics
that atomic write does not preserve.

Tests updated to mock atomicWriteFile instead of fs.writeFile.

Refs: #4095 Phase 2

* refactor(core): migrate memory state writes to atomicWriteFile (#4095 Tier 2)

Route all auto-memory state persistence through atomicWriteFile so a
process crash during a dream/extract/forget cycle cannot corrupt the
metadata sidecar, extraction cursor, or topic body files.

Touched: manager (writeDreamMetadata), extract (writeExtractCursor +
bumpMetadata), indexer (rebuild), dream (bumpDreamMetadata), forget
(bumpMetadata + topic body rewrite).

manager.ts:362 acquireDreamLock uses flag: 'wx' for exclusive create —
left untouched, atomic write does not preserve that semantic.

Uses atomicWriteFile (not atomicWriteJSON) to preserve the trailing
newline these files have always had.

Refs: #4095 Phase 2

* refactor: migrate config + logger + state writes to atomic helpers (#4095 Tier 3a)

Route the remaining state-file write paths through atomic helpers so a
crash mid-write cannot corrupt config, log, or session-scoped state:

- trustedFolders.ts (sync): atomicWriteFileSync — sole path that flips
  workspace trust, must not half-write
- logger.ts (4 sites): atomicWriteFile — full-file JSON rewrites for
  logs.json and per-checkpoint files
- tipHistory, installationManager, projectSummary, todoWrite,
  trustedHooks: bonus sites with the same shape (state JSON written
  multiple times per session)

todoWrite is on the hot path — writes every time the todo list mutates
— so the added rename + fsync cost is measurable (a few ms per write
on SSD). Trade-off accepted to avoid a half-written todos file
silently breaking the next session's resume.

Export atomicWriteFile / atomicWriteFileSync from the core public
index so CLI-side callers (trustedFolders, tipHistory) can reach them.

Tests updated:
- logger.test.ts uses vi.importActual to re-export the real helper and
  override per-test via vi.mocked(atomicWriteFile).mockRejectedValueOnce
- trustedFolders.test.ts and todoWrite.test.ts mock the helper directly

Refs: #4095 Phase 2

* fix(core): flush JSONL appends to disk (#4095 Tier 3b, closes #3681)

#3656 fixed the read side of glued '}{' JSONL records — when a process
was killed mid-appendFile, the trailing '\n' was lost and the next
record was concatenated. The write side was left for a follow-up
(#3681).

This adds flush:true (fsync) to every per-line append:
- jsonl-utils.ts writeLine / writeLineSync (session transcripts,
  auto-titles, prompt history)
- debugLogger.ts appendFile (per-session debug log)

jsonl-utils.ts write() (full-file replace) now goes through
atomicWriteFileSync so a crash during overwrite cannot corrupt the
session transcript either.

Trade-off: fsync on every append adds disk-sync latency (single-digit
ms on SSD, more on spinning disk / network FS). Acceptable for a few
writes per turn; the alternative is silently losing the last record
of every interrupted session, which #3681 explicitly flagged.

Refs: #4095 Phase 2 Tier 3b
Closes: #3681

* refactor(core): migrate extension config + LSP edit to atomic write

Catch up two sites where Claude Code's equivalent path is atomic but
qwen-code's isn't (verified against
/Users/jinye.djy/Projects/claude-code on 2026-05-19):

- extension/extensionManager.ts:533, :1073 — enablement config and
  install metadata writes. Claude Code's plugin install-counts and zip
  cache use atomic temp+rename via writeFileSyncAndFlush_DEPRECATED.
- lsp/NativeLspService.ts:1351 — applying an LSP edit to a user file.
  Claude Code's FileWriteTool/FileEditTool both route through atomic
  writeTextContent → writeFileSyncAndFlush_DEPRECATED. A bare
  writeFileSync here could half-write the user's source file if the
  process is killed during an LSP-driven rename or quick-fix.

Also clean up stale fs.rename mock setups in sharedTokenManager.test.ts
that became no-ops after Tier 1 migration (rename is no longer called
by saveCredentialsToFile). The fs.writeFile mocks stay because the
wx-flag lock path still uses them.

Refs: #4095 Phase 2

* chore: cosmetic cleanups from PR review

- packages/core/src/index.ts: move atomicFileWrite export to its
  alphabetical position (before browser.js)
- tipHistory.ts: add forceMode: true to atomicWriteFileSync for
  consistency with other 0o600 sites — heals legacy 0o644 files even
  though tips are non-critical

Refs: #4095 Phase 2

* fix(core): address Codex review findings on Phase 2 PR

Three issues caught by post-merge Codex review of the #4095 Phase 2
branch — none had user-visible symptoms yet but all were latent bugs.

1. atomicFileWrite: forceMode without mode silently downgraded perms
   `if (!options?.forceMode)` skipped the existing-mode stat whenever
   forceMode was true, regardless of whether `mode` was also supplied.
   Calling `atomicWriteFile(p, data, { forceMode: true })` (no mode) on
   an existing 0o600 file produced 0o644 (umask default) instead of
   preserving 0o600. Tightened the guard to also require `options.mode`
   to be defined; mirrored fix in atomicWriteFileSync. Added two
   regression tests (async + sync) that assert mode preservation.

2. logger.test.ts: vi.resetAllMocks() blanked the atomicWriteFile shim
   The vi.fn(actual.atomicWriteFile) factory implementation gets reset
   to a no-op by `vi.resetAllMocks()` in beforeEach, which would make
   `logger.initialize()` silently skip creating logs.json on disk.
   Tests passed by coincidence (file pre-existence from prior runs).
   Captured the real implementation at module load and re-attach it via
   `mockImplementation` after each reset.

3. NativeLspService.applyTextEdits: atomic write bypassed file unwritability
   The read catch swallowed every error and treated it as "new empty
   file". With atomic write (tmp + rename), an unreadable target on a
   writable parent could be replaced with edits applied to an empty
   buffer — the old fs.writeFileSync would have errored on the target
   permission. Now only ENOENT is treated as new-file; other read
   errors (EACCES, EISDIR, etc.) propagate.

Refs: #4095 Phase 2

* fix(lsp): refuse LSP edits to chmod 0444 files (Codex round 2)

The previous fix only handled "read failed → propagate the error".
Codex round 2 caught the remaining gap: a file that's readable but
chmod 0444 (read-only) would still be replaced by the atomic rename,
because rename only needs parent-directory write access.

Add an explicit fs.accessSync(W_OK) check before the atomic write.
ENOENT is allowed through so LSP can still create new files via edits.

Refs: #4095 Phase 2

* fix(core): drop withTimeout around atomic credential write (Codex round 3)

`saveCredentialsToFile` wrapped `atomicWriteFile` in `withTimeout(5000)`.
If the call hits the 5s budget (e.g. slow NFS home, network-backed
storage, fsync added by Phase 2), withTimeout rejects but the
atomicWriteFile internal write+rename keeps running unobserved:

  1. withTimeout rejects → saveCredentialsToFile throws
  2. performTokenRefresh `finally` releases the refresh lock
  3. Another process acquires the lock and writes newer credentials
  4. The original atomicWriteFile finally completes its rename and
     overwrites the newer credentials — silent token rollback

Pre-migration the code awaited the tmp write and the rename in two
separate withTimeout calls; a timed-out tmp write never reached the
rename so there was no race against the target file. The migration
collapsed both into one inseparable atomicWriteFile, which made the
timeout actively unsafe (the work cannot be cancelled after the
timeout fires — fs.rename is not abortable).

Atomic write is durable by design — accept the I/O latency. The
mkdir and stat timeouts are kept (idempotent and read-only
respectively, no corruption risk on late completion).

Refs: #4095 Phase 2

* test(core): add rename-retry + EXDEV-fallback coverage (#4333 review)

Address PR review suggestions from wenshao (via qwen-latest /review):
neither renameWithRetry/Sync nor the EXDEV cross-device fallback had
direct test coverage. Both paths are critical (Windows AV contention,
Docker tmpfs /tmp) and a regression would degrade silently.

Vitest can't spy on ESM exports of `node:fs` (`Cannot redefine property:
renameSync`), so add narrow internal test seams instead:
- renameWithRetry / renameWithRetrySync take an optional `_renameImpl`
  parameter, defaulting to fs.rename / fs.renameSync.
- atomicWriteFile / atomicWriteFileSync take an optional `_testFs`
  parameter with `rename` and `writeFile` overrides, forwarded to the
  retry helper and used in the EXDEV fallback branch.

The seams are underscore-prefixed and JSDoc-tagged as "Internal test
seam — production callers never pass this", which keeps the public API
clean while making the behavior testable.

New coverage (+9 tests, 36 → 45):
- renameWithRetry: retry-EPERM-then-succeed, give-up after retries,
  no-retry on non-retryable (ENOSPC)
- renameWithRetrySync: same 3 patterns (EACCES, EPERM exhausted, EINVAL)
- EXDEV fallback: async direct write + tmp cleanup, sync ditto,
  non-EXDEV failure propagates without fallback (rejects EIO + tmp cleanup)

Refs: #4333, #4095 Phase 2

* fix(test): update telemetry sdk.test.ts appendFile assertions for flush:true

CI failure on all 3 OSes (macos / ubuntu / windows): sdk.test.ts asserted
`fs.appendFile` was called with `'utf8'` as the 3rd positional argument,
but commit b7badc7 (#4095 Tier 3b — JSONL fsync) changed the
`debugLogger.ts` appendFile call from string-form to options-form
`{ encoding: 'utf8', flush: true }` to enable per-line fsync. Update the
3 assertions in the telemetry diagnostics test to match the new shape.

No behavior change — debugLogger still flushes per append; only the
assertion in this previously-unrelated suite needed updating.

Refs: #4333, #4095 Phase 2 Tier 3b

* test: cover LSP error branches + sync EXDEV cleanup + JSONL writes (#4333 review)

Address three review suggestions from wenshao (via qwen-latest /review),
each pointing at a real coverage gap introduced by this PR:

1. NativeLspService.applyTextEdits error branches (round-2 LSP fix):
   the ENOENT-only read guard and the fs.accessSync(W_OK) refusal had
   no automated coverage. Added 3 tests accessing applyTextEdits via a
   typed cast (the method is private; making it public for one
   verification inflates API surface). Tests use chmod 0000 / chmod 0444
   reproducers and assert (a) read failure propagates EACCES without
   silently overwriting with empty content, (b) W_OK rejects with
   EACCES/EPERM before the atomic rename touches the target,
   (c) nonexistent files are still accepted so LSP can create via edits.

2. atomicWriteFileSync non-EXDEV rename failure cleanup: the async
   counterpart had an explicit EIO-rename test asserting tmp cleanup;
   the sync variant did not. Added the mirror — injects a sync rename
   throwing EIO via the existing _testFs seam and asserts
   `readdirSync(tmpDir).length === 0`.

3. jsonl-utils writeLine / writeLineSync / write smoke tests: the three
   write paths are the core fix for #3681 (the PR's headline goal) but
   downstream callers (chatRecordingService, sessionService) mock them
   entirely. Without direct unit tests, a regression that dropped
   `flush: true` or reverted `write()` to bare writeFileSync would go
   undetected. Added 3 real-fs roundtrip tests.

Test count delta:
- NativeLspService.test.ts: 15 → 18
- atomicFileWrite.test.ts: 45 → 46
- jsonl-utils.test.ts: 22 → 25

Refs: #4333, #4095 Phase 2

* fix(core,test): wrap atomic write errors + guard root user + cover save failure (#4333 review)

Three review fold-ins from wenshao (via qwen-latest /review):

1. atomicFileWrite: error messages reference the random `.tmp.<hex>`
   path, not the logical target — many callers (memory subsystem,
   extension manager) don't wrap the error, making debug logs unhelpful.
   Add `annotateWriteError(error, targetPath)` that mutates the error
   message in-place to prefix `atomicWriteFile("<targetPath>"): ` while
   preserving `code` / `errno` / `syscall` / `stack` / the prototype
   chain so downstream `err.code === 'ENOENT'` checks and `instanceof`
   narrowing keep working. Applied to both async and sync variants;
   only the final propagated throw (not the EXDEV fallback path) is
   annotated.

2. NativeLspService.test.ts: the chmod 0444 and chmod 0000 tests rely
   on `accessSync(W_OK)` and `readFileSync` failing — but on POSIX with
   UID 0 (root, including most Docker CI runners), permission bits are
   bypassed and `accessSync` always succeeds. The tests would silently
   pass even with the W_OK guard removed entirely. Add
   `process.getuid?.() === 0` to the skip guard on both tests.

3. sharedTokenManager.test.ts: the catch block in saveCredentialsToFile
   that maps disk-full / permission-denied to
   `TokenManagerError(FILE_ACCESS_ERROR)` was never exercised — every
   prior test mocked atomicWriteFile as always-successful. Added a
   regression test that rejects atomicWriteFile with ENOSPC and asserts
   the wrapped TokenManagerError surfaces with the right type and
   carries the original message.

Refs: #4333, #4095 Phase 2

* fix(core,lsp): annotate sync errors correctly + cover EXDEV fallback + async LSP IO (#4333 review)

Three review fold-ins from wenshao (via qwen-latest /review), all real
correctness/consistency issues:

1. annotateWriteError hardcoded "atomicWriteFile" prefix regardless of
   caller. Sync write failures produced misleading
   `atomicWriteFile("/path"):` prefixes in incident logs. Add optional
   `fnName` parameter (defaults to async name) and have sync call sites
   pass "atomicWriteFileSync".

2. EXDEV fallback path in both async and sync variants did NOT route
   the inner writeFileImpl/tryChmod failures through annotateWriteError.
   On a cross-device write that subsequently hit ENOSPC, the propagated
   error had a bare syscall message without the target-path prefix —
   breaking the function's documented error-shape contract. Wrap both
   fallback branches in try/catch + annotate.

3. NativeLspService.applyTextEdits is declared `async` and all callers
   `await` it, but the round-2 fix mixed in sync IO: readFileSync,
   accessSync, atomicWriteFileSync. The sync helper's renameWithRetrySync
   blocks the event loop up to ~350ms under Atomics.wait EPERM backoff
   — particularly bad for LSP workspace edits that loop over many files.
   Switch to async throughout: fsp.readFile, fsp.access, atomicWriteFile.
   Behavior preserved (same ENOENT-vs-other distinction, same W_OK gate).
   Existing tests pass unchanged (they already use the async typed-cast
   entry point).

Refs: #4333, #4095 Phase 2

* test(core): cover EXDEV-fallback-write-failure annotation path (#4333 review)

The previous fold-in added try/catch around the EXDEV fallback write in
both async and sync variants, routing fallback failures through
`annotateWriteError(err, target, fnName)`. The sync-specific
`fnName='atomicWriteFileSync'` differentiation is ONLY exercised on
that path, so a regression that dropped or misapplied the annotation
on sync would otherwise go undetected.

Two new tests inject both a failing rename (EXDEV) and a failing
writeFile (ENOSPC) via the `_testFs` seam, then assert (a) the
original `code === 'ENOSPC'` propagates intact, and (b) the message
matches `/atomicWriteFile(Sync)?\(<target>\):.*ENOSPC/` — verifying
target-path prefix AND correct fn-name differentiation.

Refs: #4333, #4095 Phase 2

* fix(core): annotate guard, drop debug fsync, add noFollow for creds (#4333 review)

Four review fold-ins from wenshao (via qwen-latest /review):

1. annotateWriteError guard `!error.message.includes(targetPath)` was
   always false in production. tmpPath is `${targetPath}.<hex>.tmp`, so
   any real fs error like `ENOSPC ... '/path/creds.json.a1b2c3.tmp'`
   *contains* targetPath as substring → guard returned false → the
   annotation prefix was silently skipped on every real failure path.
   Tests passed only because mock errors used bare `new Error('ENOSPC')`
   messages without paths. Change guard to idempotency check on our
   own prefix (`startsWith(`${fnName}(`)`), and update the two existing
   EXDEV-fallback tests to use realistic path-embedding fs messages.

2. debugLogger.appendFile dropped `flush: true`. 1050+ call sites,
   default-enabled, fire-and-forget — per-line fsync creates sustained
   I/O pressure / SSD wear with no user benefit (debug logs are
   best-effort, the module already tracks `hasWriteFailure` for the
   degraded-mode UI). Kept the kernel page-cache flush; revert
   debugLogger.test.ts and telemetry/sdk.test.ts assertions back to
   plain `'utf8'`. The #3681 closure target is jsonl-utils writeLine,
   not debug logs.

3. Symlink security regression: the old
   `fs.writeFile(tmp) + fs.rename(tmp, filePath)` pattern atomically
   *replaced* a pre-placed symlink at `filePath`. atomicWriteFile's
   default `resolveSymlinkChain(filePath)` follows the link and writes
   through, redirecting tokens to wherever the link points (real
   concern on shared hosts with weaker-than-expected dir perms). Add
   `noFollow?: boolean` option that skips chain resolution; apply
   `noFollow: true` to all 4 credential write sites
   (oauth-token-storage [2 sites], file-token-storage, sharedTokenManager)
   to match the pre-migration replace-symlink semantics.

4a. Test seam `_testFs` was a 4th positional arg → considered moving
    into options. Punted: positional with underscore + JSDoc is
    materially the same surface as options field with underscore,
    and the only realistic collision (future production option as
    5th arg) is bounded by review.
4b. Sync/async code duplication (~110 lines mirror) → DECLINED.
    Refactoring to a sync/async-polymorphic helper introduces a new
    abstraction layer with worse type ergonomics; the duplication is
    mechanical and lined up for easy diffing. Tracked as Phase 2.5
    candidate if divergence actually accumulates.

Refs: #4333, #4095 Phase 2

* fix(core): EXDEV fallback honors noFollow + fix test correctness (#4333 review)

Three review fold-ins from wenshao (via qwen-latest /review):

1. **[CRITICAL]** EXDEV fallback path silently bypassed `noFollow`
   protection. The fallback `writeFileImpl(targetPath, ...)` is
   `fs.writeFile` / `fs.writeFileSync`, both of which follow symlinks.
   When a credential write site set `noFollow: true` and rename threw
   EXDEV (realistic on Docker OverlayFS / union mounts), the fallback
   would write credentials *through* a pre-placed symlink to an
   attacker-controlled target — the exact attack noFollow was meant
   to prevent. Fix: when `noFollow` is set, the EXDEV fallback now
   `unlink`s any existing entry, then opens with `O_WRONLY|O_CREAT|O_EXCL`
   to refuse to write through a symlink that races back. Applied to
   both async and sync variants.

2. **Test correctness**: the two EXDEV-fallback-write-failure tests
   added in the previous round had a bug — the `failingWrite` mock
   threw on every call, including the first call which is the
   tmp-file write. The tmp write failed → outer catch caught
   ENOSPC → EXDEV check returned false (code is ENOSPC) → fell
   through to the outer annotateWriteError. The inner
   EXDEV-fallback annotation was never exercised. The assertions
   passed only because annotateWriteError produces the same format
   in both catch blocks. Fix: selective-failure mock that succeeds
   on the first call (tmp write) and fails on the second (fallback
   write), genuinely reaching the EXDEV branch.

3. **Behavioral noFollow tests**: previously `noFollow: true` was
   only verified at the "option is passed to mock" level. Added 4
   real-fs tests (async + sync × happy-path + EXDEV-fallback) that
   pre-place a symlink, call atomicWriteFile with noFollow, and
   assert: (a) the symlink is replaced by a regular file, (b) the
   new file holds the new data, (c) the real file behind the
   symlink is untouched. A regression flipping the noFollow ternary
   or skipping the noFollow-aware EXDEV fallback now fails directly.

Refs: #4333, #4095 Phase 2

* fix(core): close TOCTOU window on noFollow EXDEV fallback + cover ENOENT path

Two PR #4333 round-7 review items folded in:

1. TOCTOU race: the path-based `tryChmod(targetPath)` after the EXDEV
   noFollow branch ran AFTER `fd.close()` released the inode reference.
   Between close and chmod, an attacker with parent-directory write access
   could replace the regular file with a symlink, redirecting the
   `chmod 0o600` onto an attacker-chosen target — silently defeating the
   `noFollow` protection that the `unlink + O_EXCL` pattern was added to
   provide. Fix: switch to `fd.chmod`/`fchmodSync` on the open fd before
   close (operates on the inode, immune to symlink swap), and skip the
   path-based chmod for the noFollow branch (path-based chmod remains
   for the non-noFollow direct-write branch, where following symlinks
   was already in scope).

2. Missing test coverage: all 4 existing noFollow EXDEV tests pre-place
   a symlink at the target, so `unlink(targetPath)` always succeeded —
   the ENOENT-swallow branch (first-write scenarios, e.g. initial
   credential provisioning on a cross-device mount) had no coverage.
   Added 2 tests (async + sync) verifying the fallback creates a new
   file with the requested mode when the target never existed.

Test results: 54/54 atomicFileWrite tests pass (was 52). Async + sync
parity preserved.

Refs: #4333, #4095 Phase 2

* test(core): skip new noFollow EXDEV-fallback tests on Windows (NTFS perm bits)

The two new-file noFollow EXDEV tests added in 7c476648e assert the file
ends up at 0o600. NTFS reports 0o666 for any file that isn't read-only
regardless of chmod/fchmod, so the assertion fails on Windows runners
(`AssertionError: expected 438 to be 384`). Match the existing
`it.skipIf(process.platform === 'win32')` pattern already used by all
mode-asserting tests in this file. Linux/macOS coverage of the
ENOENT-swallow + new-file path is unchanged.

Refs: #4333, #4095 Phase 2

* fix(core): narrow fchmod catch + verify mechanism on noFollow EXDEV

Two PR #4333 round-8 review items:

1. The `catch {}` around `fd.chmod(desiredMode)` (and `fchmodSync`) was
   intended to tolerate filesystems without POSIX permissions (FAT/exFAT)
   but silently swallowed every error code: EPERM under a hardened
   sandbox, EIO on flaky NFS/CIFS, EROFS if the filesystem is remounted
   read-only mid-write. Because this path operates on the *final
   credential file* — not a tmp file — a silent fchmod failure leaves
   the target at the umask-masked open() mode with no diagnostic trail.
   Narrowed the catch to ENOSYS/ENOTSUP so the FAT/exFAT case still
   tolerates failure but security-relevant errors propagate.

2. The round-7 noFollow-EXDEV-new-file tests asserted the final mode
   (0o600) but didn't verify the *mechanism*. Under typical umask 0o022,
   `open(O_EXCL, 0o600)` already creates the file at 0o600, so a
   regression that swapped `fd.chmod()` back to a path-based
   `tryChmod(targetPath)` (the pre-fix TOCTOU-vulnerable form) would
   leave the mode assertion passing — defeating the round-7 fix
   undetected. Added a `chmod` test seam to `atomicWriteFile` /
   atomicWriteFileSync` (mirroring the existing `rename` / `writeFile`
   seams) and asserted that path-based chmod is never invoked against
   the credential target on this code path.

54/54 atomicFileWrite tests pass.

Refs: #4333, #4095 Phase 2

* fix(core): clean up O_EXCL orphan on noFollow EXDEV fchmod failure + cover both branches

Two PR #4333 round-9 review items, one critical correctness bug
introduced by the round-8 catch narrowing:

1. **[Critical]** When `fd.chmod(desiredMode)` (or `fchmodSync`) on the
   noFollow EXDEV fallback throws a propagating error (EPERM under
   seccomp, EIO on flaky NFS, EROFS after a remount), the file created
   by `open(targetPath, O_CREAT|O_EXCL)` remained on disk after the
   error rethrew. Every subsequent credential write retry then hit
   EEXIST from O_EXCL and failed permanently — turning a transient
   sandbox EPERM into a permanent OAuth-refresh deadlock that requires
   manual file removal. All three credential write sites
   (`sharedTokenManager`, `oauth-token-storage` save+delete,
   `file-token-storage`) hit this code path. Fix: a `writeOk` flag plus
   nested try/catch — fd is closed in the inner finally, then if write/
   sync/fchmod failed, the orphan is unlinked best-effort before the
   error rethrows.

2. The fchmod catch-narrowing (the headline behavior of round 8) had
   zero test coverage on either branch — `_testFs` exposed `rename`,
   `writeFile`, and path-based `chmod`, but no hook for the open-fd
   `fchmod`. A one-line revert from the narrowed catch back to
   `catch {}` would pass every existing test. Added `fchmod` field to
   `_testFs` (async + sync) and four tests:
   - ENOSYS swallowed → write succeeds (FAT/exFAT happy path)
   - EPERM propagates AND `targetPath` is absent (regression-tests #1)
   for both async and sync.

58/58 atomicFileWrite tests pass (was 54). Async + sync parity preserved.

Refs: #4333, #4095 Phase 2

* fix(core): annotate symlink errors + cover ENOTSUP/EACCES + log orphan unlink

Four PR #4333 round-10 review items, all small bounded fixes:

1. Parameterized the FAT/exFAT fchmod-swallow tests over both ENOSYS
   (Linux) and ENOTSUP (macOS). Round-9 only covered ENOSYS — a
   one-token regression dropping `ENOTSUP` from the catch condition
   would have passed every existing test.

2. The orphan-unlink catch block was empty (`/* best effort */`),
   leaving no diagnostic trail when the cleanup itself fails (EROFS,
   immutable flag, sandboxed container). Added a `createDebugLogger`
   import (`'ATOMIC_WRITE'` category) and a debug log so incident
   response can correlate the original write error with a subsequent
   EEXIST loop. Async + sync.

3. The pre-open `unlink(targetPath)` correctly propagates non-ENOENT
   errors (EACCES on parent dir, EROFS), but no test exercised that
   path. Added an `unlink` field to the `_testFs` seam (async + sync,
   matching the existing `rename` / `writeFile` / `chmod` / `fchmod`
   pattern) and two tests verifying EACCES propagates instead of
   getting hidden behind a downstream EEXIST from O_EXCL.

4. `resolveSymlinkChain(filePath)` ran before the function's main
   try-block, so symlink-resolution errors (EACCES on intermediate
   dir, ELOOP from circular chain) bypassed the
   `atomicWriteFile("path"): ...` annotation that every other failure
   path applies — leaving `err.path` referencing an internal
   intermediate directory the caller never asked about. Wrapped with
   `.catch(err => throw annotateWriteError(err, filePath))` (async)
   and the equivalent try/catch for sync. Added a real-fs ELOOP
   regression test for both variants (skipped on Windows).

64/64 atomicFileWrite tests pass (was 58).

Refs: #4333, #4095 Phase 2

* fix(core): widen atomicWriteJSON options + tighten trustedHooks perms + cover backoff/mkdir branches

Five PR #4333 round-11 review items, all small and bounded:

1. **atomicWriteJSON option-type widening**: Was typed as the narrower
   `AtomicWriteOptions` (retries / delayMs only), so credential-grade
   options added in this PR (`mode`, `forceMode`, `noFollow`) and
   pre-existing ones (`flush`, `encoding`) were silently dropped at the
   type level even though the body spread them at runtime. A future
   maintainer calling `atomicWriteJSON(credPath, creds, {noFollow:
   true, mode: 0o600, forceMode: true})` would have typechecked but
   silently lost noFollow + forceMode + mode. Widened to
   `AtomicWriteFileOptions`.

2. **trustedHooks 0o600 + forceMode**: This file lists user-approved
   *executable hook commands* — strictly more sensitive than the
   sibling state files (`trustedFolders.json`, `tipHistory.json`) that
   already use `{mode: 0o600, forceMode: true}`. Was dropping to the
   process umask (0o644 by default), and a backup-restored looser mode
   was never healed. Now matches the sibling pattern.

3. **renameWithRetry exponential-backoff coverage**: Existing tests
   covered retry count and error propagation but not the
   `delayMs * 2 ** attempt` curve itself. A regression to linear,
   constant, or — worst — regressive backoff (which intensifies under
   Windows AV-scan stress) would have passed every existing test.
   Added a test using `vi.useFakeTimers()` that records gaps between
   mock-rename invocations and asserts `[delayMs, 2*delayMs,
   4*delayMs]` for `(retries=3, delayMs=50)`.

4. **jsonl-utils write() parent-dir creation**: The other `write()`
   test targets a path inside the pre-created `tmpRoot`, so the
   `!existsSync(dir) → mkdirSync(dir, {recursive: true})` branch was
   never exercised. Added a one-liner test that targets a deeply
   nested non-existent path.

5. **writeLineSync docstring accuracy**: The docstring claimed "uses
   a simple flag-based locking mechanism (less robust than async
   version)" but there is no flag-based locking — and `writeLine`
   serializes via per-file `Mutex` that this function bypasses. Now
   accurately documents the lack of locking, the bypass, and the
   `flush: true` rationale (closes #3681).

Test results: 65/65 atomicFileWrite tests pass (was 64), 26/26
jsonl-utils tests pass (was 25). Typecheck clean.

Refs: #4333, #4095 Phase 2

* fix(core): force JS slow path for appendFile flush + correct debugLogger format

Two PR #4333 round-12 review items, both real bugs:

1. **flush:true was silently a no-op for string payloads on appendFile**.
   Node's C++ fast path (binding.writeFileUtf8) for string + utf8 +
   appendFile bypasses the JS-side flush/fsync logic entirely —
   empirically string+flush:true takes ~0.05ms/op (identical to no
   flush) while Buffer+flush:true takes ~4.9ms/op (91× slower,
   proving fsync only runs for Buffer payloads). The data still
   reaches the kernel page cache (the syscall is synchronous), so
   `kill -9` is fine, but power-loss durability — the actual #3681
   guarantee — was silently absent.

   Fix: pass `Buffer.from(line, 'utf8')` to both writeLine (async)
   and writeLineSync. This forces the JS slow path that honors
   `flush: true` and actually fsyncs the file. Updated the JSDoc
   on both functions to document the C++ fast-path bypass so a
   future maintainer doesn't revert to the simpler string form.

2. **`debugLogger.debug` doesn't do printf substitution**.
   `debugLogger`'s `formatArgs` (debugLogger.ts:67-77) just joins
   args with spaces — no `util.format()`. The round-10 calls used
   `'orphan unlink failed for %s: %s'` which rendered the literal
   `%s` markers in the log:
       orphan unlink failed for %s: %s /path/to/target Error: EACCES
   instead of:
       orphan unlink failed for /path/to/target: Error: EACCES
   Switched both async and sync sites to template literals, matching
   every other `debugLogger` call site in the codebase.

Refs: #4333, #4095 Phase 2

* fix(core): narrow tryChmod catch + writeFileSync(fd) + cover credential write failures

Three PR #4333 round-13 review items + one comment-wording softening:

1. **`tryChmod`/`tryChmodSync` catch narrowed (wenshao)** — was bare
   `catch {}` swallowing all errors, while the round-8 `fchmod` catch
   was already narrowed to ENOSYS/ENOTSUP. Same security rationale
   applies — and *specifically* on the EXDEV non-noFollow fallback,
   `tryChmod(targetPath)` is the *sole* mode-setting mechanism for an
   existing target (writeFile ignores `mode` when the target exists),
   so a silent EPERM/EROFS would leave credentials at the old mode.
   Non-credential callers don't pass `mode` → `desiredMode === undefined`
   short-circuits, so they're unaffected.

2. **Sync EXDEV `writeSync` → `writeFileSync(fd)` (yiliang114)** —
   `fsSync.writeSync(fd, buf)` returns bytes-actually-written and can
   short-write; the current code ignored the return, so a partial write
   would silently truncate the credential file with the call still
   returning success after fsync+fchmod. Switched to
   `fsSync.writeFileSync(fd, buf)` which loops internally per Node spec.
   The async sibling (`fd.writeFile`) already handles short-writes;
   this brings sync parity.

3. **`file-token-storage` failure-path coverage (wenshao)** — both
   `setCredentials` and `deleteCredentials` propagate `atomicWriteFile`
   rejections (no try/catch around the call), but no test exercised
   that path. Added two tests mirroring the round-1 sharedTokenManager
   precedent: ENOSPC on `setCredentials` and EROFS on `deleteCredentials`
   both rethrow.

4. **Round-12 comment wording softened (wenshao verification report)** —
   strace on Node v22/v24 confirms string + utf8 + flush:true does
   fsync correctly today, counter to my round-12 "silent no-op"
   framing. Buffer is still the safer documented form (forward-compat
   insurance against any future fast-path optimization), but the
   commit's claim that it was *fixing* a confirmed bug overstated what
   reproduces. Reframed both writeLine and writeLineSync comments
   accordingly without changing the code behavior.

Test results: 109/109 affected suites pass (atomicFileWrite 65,
jsonl-utils 26, file-token-storage 18). Broader credential/state
suites also green: 216/216 across sharedTokenManager + oauth-token-storage
+ qwenOAuth2 + logger + trustedFolders. Typecheck clean.

Refs: #4333, #4095 Phase 2

* fix(core): route orphan unlink through seam + cover tryChmod + trustedHooks tests

Three PR #4333 round-14 review items:

1. **Path-level tryChmod / tryChmodSync catch narrowing had zero direct
   coverage**. Round-13 narrowed the bare `catch {}` to ENOSYS/ENOTSUP
   only (matching the round-8 fd-level fchmod narrowing), but every
   existing EXDEV test passed `options: undefined`, so `desiredMode ===
   undefined` short-circuited before any chmod was attempted. A
   regression that inverted the catch condition (missing `!` prefix)
   would silently swallow EPERM on every sync credential write
   (trustedHooks, tipHistory, trustedFolders, etc.) with zero
   diagnostic signal. Added 6 tests via the existing `_testFs.chmod`
   seam: parameterized ENOSYS/ENOTSUP swallow + EPERM propagation, for
   both async (`atomicWriteFile`) and sync (`atomicWriteFileSync`).

2. **Orphan-cleanup unlink on the noFollow EXDEV failure path was
   using raw `fs.unlink` / `fsSync.unlinkSync` instead of the
   injected `unlinkImpl` seam**. The pre-open unlink correctly used
   the seam, but the round-9 orphan cleanup added later bypassed it,
   making it the only fs operation in `atomicWriteFile` not flowing
   through the test seam. Routed both async and sync orphan cleanup
   through `unlinkImpl`, and added 2 tests that inject a spy and
   assert orphan cleanup is invoked against targetPath after a
   simulated fchmod EPERM.

3. **`trustedHooks.ts` had no test coverage**. Round-11 migrated it to
   `atomicWriteFileSync` with `{ mode: 0o600, forceMode: true }` —
   strictly the most security-sensitive write in the PR since the
   file stores user-approved executable hook commands — but unlike
   the sibling files (trustedFolders, tipHistory) it had no test
   file. A regression that dropped `forceMode: true` or weakened
   the mode would have passed all existing tests. Created
   `trustedHooks.test.ts` covering: write goes through
   atomicWriteFileSync with `{ mode: 0o600, forceMode: true }`,
   write targets the global qwen dir path, the persisted content
   matches the hook key derived from the hook config, and round-trip
   trust/untrust behavior.

Test results: 73/73 atomicFileWrite tests (was 65) + 5/5
trustedHooks (new). Typecheck clean.

Refs: #4333, #4095 Phase 2

* fix(core): add noFollow to trustedHooks/trustedFolders + route tmp cleanup through seam

Two PR #4333 round-15 review items:

1. **`trustedHooks.ts` and `trustedFolders.ts` were missing `noFollow:
   true`**. The credential write sites
   (`sharedTokenManager`/`oauth-token-storage`/`file-token-storage`)
   all pass `{ mode: 0o600, forceMode: true, noFollow: true }` to
   prevent pre-placed symlink attacks. The trustedHooks comment
   already called it "strictly more sensitive than trustedFolders /
   tipHistory" — yet credential paths got symlink protection it
   didn't. A pre-placed symlink at `~/.qwen/trusted_hooks.json` (or
   `trustedFolders.json`) could redirect the atomic write to an
   attacker-controlled path, either leaking the executable-trust
   list / trusted-folder list, or leaving the user's real config
   silently stale. Added `noFollow: true` to both write sites and
   updated the assertions in `trustedHooks.test.ts` and
   `trustedFolders.test.ts`.

2. **Tmp-file cleanup at L240 (async) and L568 (sync) used raw
   `fs.unlink` / `fsSync.unlinkSync` instead of the injected
   `unlinkImpl` seam**. Pre-open unlink and orphan cleanup correctly
   routed through `unlinkImpl`, making the tmp-cleanup branch the
   only outlier — `_testFs.unlink`-injecting tests couldn't intercept
   this path, weakening the seam abstraction. Behavioral impact is
   nil (cleanup is best-effort, errors swallowed), but consistency
   matters for future test authors. Routed both async and sync
   variants through `unlinkImpl`.

Test results: 99/99 affected suites pass (atomicFileWrite 73,
trustedHooks 5, trustedFolders 21). Typecheck clean on core.

Refs: #4333, #4095 Phase 2

* fix(core): use path.join in trustedHooks test to fix Windows CI

PR #4333 round-15 review item (Critical, from claude-opus-4-8 /qreview):

trustedHooks.test.ts:68 hardcoded a POSIX path
('/mock/home/.qwen/trusted_hooks.json'), but production builds the path
with path.join(Storage.getGlobalQwenDir(), 'trusted_hooks.json')
(trustedHooks.ts:29-31). On Windows path.join emits '\' separators, so
the mocked atomicWriteFileSync receives
'\mock\home\.qwen\trusted_hooks.json' and the toBe assertion fails — the
cause of the red Test (windows-latest, Node 22.x) check. macOS/Linux
runs are green because forward slashes match.

Build the expected path with path.join so it matches the platform
separator, and add the node:path import.

Refs: #4333, #4095 Phase 2

* fix(core,cli): round-16 review — tipHistory noFollow + atomicWrite test coverage + LSP concurrency note

Four PR #4333 follow-up review items from wenshao:

1. tipHistory.ts was missing noFollow: true while the sibling
   0o600+forceMode config sites (trustedFolders.ts:192,
   trustedHooks.ts:63) both set it. The cleanup commit added forceMode
   "for consistency with other 0o600 sites" but stopped short of
   noFollow. All three store user-trusted paths and should share the
   same pre-placed-symlink protection. Added noFollow: true.

2. annotateWriteError idempotency guard had no direct coverage. The
   guard switched from includes(targetPath) to startsWith(fnName+"(")
   because real syscall errors embed the *tmp* path (which contains the
   target as a substring), so the old guard silently skipped annotation
   on every real failure. Added a test where the rename error message
   embeds a tmp-style path containing the target; the correct startsWith
   guard still annotates it, and reverting to includes would fail it.

3. non-EXDEV rename failure tests only asserted /EIO/, not the
   atomicWriteFile(...): / atomicWriteFileSync(...): annotation prefix
   that the production re-throw applies on that path. Tightened both
   async and sync assertions to match the prefix.

4. applyTextEdits concurrency constraint was undocumented. The async
   read-modify-write has await points between read and write;
   atomicWriteFile prevents torn files but does not serialize writers,
   so concurrent same-path edits can lose updates. Latent today (no
   production caller / no workspace/applyEdit handler), so documented
   the per-file-serialization requirement instead of adding a lock.

Not taking: extending the _testFs seam with open/openSync to fault-inject
O_EXCL EEXIST — the noFollow/O_EXCL security behavior is already covered
by the real-fs behavioral tests (noFollow EXDEV fallback refuses to
follow symlinks); a seam-injected EEXIST would only exercise error
propagation, not the guarantee.

Test results: atomicFileWrite 74/74 pass, eslint clean.

Refs: #4333, #4095 Phase 2

* test(core): route atomicWriteFile open through _testFs seam + assert O_EXCL (PR #4333 review)

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
2026-06-02 19:36:49 +08:00
易良
c4a8e606f3
feat(core): auto-dump memory diagnostics to disk on pressure detection (#4654)
* feat(core): auto-dump memory diagnostics to disk on pressure detection

When the MemoryPressureMonitor (#4403) detects hard or critical pressure,
write a lightweight diagnostics JSON to .qwen/<project>/diagnostics/ before
running cleanup. The file survives even if a subsequent operation triggers
OOM, giving maintainers actionable data from bug reports without requiring
the user to manually run /doctor memory after a crash.

Design follows Claude Code's heapDumpService approach: write the cheap JSON
first (small write, won't OOM), heavy snapshot second. Diagnostics include
process memory stats, V8 heap stats, session history size, and an actionable
suggestion for the user.

Per-session limits: max 3 dumps, 30s cooldown between dumps.

Closes #4651

* ci: retrigger CI after Windows flaky failure

* test(core): use path.join in memoryDiagnosticsDumper test for cross-platform

The assertion hard-coded POSIX separators ('/tmp/test-project/diagnostics/'),
which fails on Windows where path.join produces backslashes. Build the
expected substring with path.join + path.sep so it matches the dumper's
actual output on every platform.

* fix(core): two-phase memory diagnostics write to survive OOM

Two critical issues from review:

1. The async collectMemoryDiagnostics() runs before writeFileSync, but it
   spawns a `ps` subprocess and reads /proc — fork() under critical memory
   pressure can fail or be OOM-killed, leaving no file on disk despite the
   "cheap write first" design comment.

2. dumpCount and lastDumpTime were updated after the await, so concurrent
   dumps (e.g. hard→critical escalation) would both pass the cap/cooldown
   guards and overwrite each other.

Fix:

- Reserve the dump slot synchronously (++dumpCount, lastDumpTime) before
  any await, so concurrent calls correctly hit the cap.
- Phase 1: synchronously write a minimal JSON (process.memoryUsage +
  v8.getHeapStatistics, no fork/exec) with collectionComplete=false.
  Because async functions execute synchronously up to the first await,
  this is guaranteed on disk before the caller's next statement runs.
- Phase 2: enrich with full diagnostics asynchronously and overwrite the
  file with collectionComplete=true. If Phase 2 crashes, the minimal
  Phase 1 file still survives for debugging.

Tests updated for the two-phase write and gain two new cases covering the
sync-Phase-1 guarantee and the synchronous slot reservation.

* fix(core): point memory diagnostics suggestion at /compress (the actual command)

The suggestion text told users to run /compact, which does not exist in
this repository — the actual command is /compress (see compressCommand.ts).
Pointing users at a nonexistent slash command in a diagnostics report
makes the suggestion unactionable.
2026-06-02 16:05:58 +08:00
qqqys
1a5b9792d0
Add AUTO mode denial observability and caps (#4476)
* feat(core): add auto-mode denial observability

* fix(core): prune unused auto denial reason

* fix(core): scope auto fallback counter resets

* test(core): cover denied fallback cancellation

* fix(core): tighten auto-mode denial fallback tracking

* test(core): cover permission denied hook events

* fix(core): preserve auto fallback reasons

* test(core): cover auto mode denial helpers

* fix(core): reset auto fallback after hook approval

* fix(core): log auto fallback reset recovery

* fix(core): clarify auto fallback reset logging

* test(core): assert auto fallback reset logging
2026-06-02 15:31:39 +08:00
易良
237bf879c1
feat(core): inject context env vars (session/agent/prompt ID) into shell subprocesses (#4649)
* feat(core): inject context env vars (session/agent/prompt ID) into shell subprocesses

When SubAgents execute SQL or Python scripts via Bash tool, the scripts
have no way to know their execution context. This adds automatic injection
of QWEN_CODE_SESSION_ID, QWEN_CODE_AGENT_ID, and QWEN_CODE_PROMPT_ID
into all shell subprocess environments, enabling downstream scripts to
perform trace correlation, audit logging, and business context attribution.

Closes #4645

* fix(core): use module-level flag to guard session env claim

Prevents nested qwen-code processes from inheriting the parent's
session ID. The previous `if (!process.env[...])` check would keep
the parent's value; a module-level flag ensures each process claims
its own session ID on first Config construction.

* fix(test): use bracket notation for index signature properties

CI tsc --build enforces noPropertyAccessFromIndexSignature; use
env['KEY'] instead of env.KEY to satisfy strict type checking.

* fix(core): guard process.env assignment for mocked process environments

Some test suites mock node:process without providing env, causing
TypeError when Config constructor assigns QWEN_CODE_SESSION_ID.
Add defensive check before env writes.

* test(config): add coverage for sessionEnvClaimed guard

Addresses reviewer feedback (wenshao) requesting test coverage for the
module-level `sessionEnvClaimed` guard in Config constructor.

Tests verify:
1. First Config instance sets process.env['QWEN_CODE_SESSION_ID']
2. Subsequent Config instances do not overwrite the env var

* test(config): add startNewSession env var test and clarify comment

- Add test verifying startNewSession updates process.env to new session ID
- Add comment explaining why startNewSession bypasses sessionEnvClaimed guard
  (only callable on the canonical Config instance that already claimed)
2026-06-02 15:31:09 +08:00
Dragon
40f061a6a5
fix(core): coerce hostile-provider usage token counts (#4350 part 1) (#4439)
Some checks are pending
Qwen Code CI / Lint (push) Blocked by required conditions
Qwen Code CI / Classify PR (push) Waiting to run
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): coerce hostile-provider usage token counts (#4350 part 1)

Hostile providers (broken upstream, OpenAI-compat proxy returning
null/NaN, misconfigured override) can emit non-finite or negative
values for `usageMetadata.{prompt,candidates,cached,total}TokenCount`.
Captured unguarded in `processStreamResponse`, these poison the
compaction gate arithmetic:

- `lastPromptTokenCount + NaN >= hard` is always false → hard-rescue
  is silently disabled, eventually OOMing the V8 heap.
- `Infinity >= hard` is always true → hard-rescue fires every send.

Route the four API capture sites through a `coerceUsageCount` helper
that maps unknown / non-finite / negative to 0. `Number.isFinite(-1)`
is true, so an explicit `>= 0` is needed in addition to `isFinite`.

Part 1 of the hostile-provider hardening from #4350. The companion
`computeThresholds` guard depends on the un-merged three-tier ladder
in #4345 and is deferred until that lands.

Covered by parametrized tests in `geminiChat.test.ts` over NaN,
±Infinity, negative, null, undefined, and string inputs, plus a
fallback test asserting a hostile `promptTokenCount` falls through
to a coerced `totalTokenCount`.

* docs(core): narrow coerceUsageCount JSDoc to fields it actually coerces

The JSDoc for coerceUsageCount enumerated
{prompt,candidates,cached,total}TokenCount as the protected fields, but
candidatesTokenCount is never passed through coerceUsageCount in this PR --
only promptTokenCount, totalTokenCount, and cachedContentTokenCount are.
The original wording created a misleading impression of coverage.

Rewrite the JSDoc to (a) list exactly the three fields the function is
called on, (b) state explicitly that candidatesTokenCount is intentionally
left raw because it does not feed the compaction gate, and (c) call out
that candidatesTokenCount still flows unguarded into OTel spans via
loggingContentGenerator -- to be tightened separately.

No behavior change. Resolves the geminiChat.ts:109 review thread on this
PR.

* fix(core): address review feedback on coerceUsageCount

- Fix JSDoc: 'This PR' → 'This function' for long-term documentation
- Add optional field name parameter to coerceUsageCount for debug logging
- Log hostile values via debugLogger.warn when coercion fires
- Coerce candidatesTokenCount (was missing despite JSDoc mentioning it)
- Build sanitized usageMetadata with coerced values for recordAssistantTurn
  so JSONL persistence and --resume don't re-poison the compaction gate

* fix(core): reuse coerced usage values in recordAssistantTurn

The streaming loop was calling coerceUsageCount twice for the same
usageMetadata: once for telemetry updates and again inline when building
the tokens object for recordAssistantTurn. This left candidatesTokenCount
declared but unused in the first block (TS6133).

Now stashes all four coerced values in a coercedUsage object during the
streaming loop, then spreads it into the tokens object when recording.
This eliminates the duplicate coercion calls and uses candidatesTokenCount
consistently.

Resolves review thread: PRRT_kwDOPB-92c6Ed4WT

* test(core): assert debugLogger.warn for hostile usage counts

Address PR #4439 review thread on geminiChat.test.ts: the hostile-provider
parametrized test asserted token-count coercion behaviour but never verified
the operator-facing diagnostic emitted by `coerceUsageCount`. A future
refactor could remove or misformat the warning with no test failure.

- Mock `createDebugLogger` via vi.hoisted/vi.mock so the module-level
  `debugLogger` in geminiChat.ts routes warnings to a spy.
- Inside the existing it.each, assert that hostile defined values
  (NaN/Infinity/-Infinity/negative/string) emit warns mentioning both
  `hostile promptTokenCount` and `hostile totalTokenCount`, and that the
  stringified bad value is included so logs are actionable.
- Add a negative assertion that the field-omitted cases (null, undefined)
  do NOT trigger any warn — verifying the `value != null` guard.
2026-06-02 14:59:44 +08:00
qqqys
1a6b4d3e0a
fix(core): loosen auto-mode classifier timeouts, disable stage-2 thinking (#4680)
* fix(core): loosen auto-mode classifier timeouts, disable stage-2 thinking

The AUTO-mode classifier fails closed on timeout — a timed-out judge call
blocks the action as "unavailable". The tight 3s/10s stage budgets turned
transient slowness (slow network, large transcript, model queueing) into
spurious blocks of otherwise-valid actions. Raise them to 10s/30s so a
slow-but-healthy call is not treated as a hard block.

Also disable thinking in stage 2 (previously the only stage with
includeThoughts: true). This is a latency-sensitive permission gate the
user is actively waiting on; allocating a reasoning budget made the review
path slower and more expensive, which directly worsened the fail-closed
timeout. The model still records its reasoning in the structured
`thinking` output field — it just no longer gets an allocated budget.

Closes #4676

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

* docs(core): trim verbose comments in auto-mode classifier

Condense the three comments touched by this change (module docstring
stage-2 note, timeout-budget rationale, stage-2 thinkingConfig) while
keeping the essential "why". No logic changes.

Co-authored-by: Qwen-Coder <noreply@qwenlm.ai>

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <noreply@qwenlm.ai>
2026-06-02 14:53:05 +08:00
jinye
13f37dc9f0
feat(cli): background housekeeping for stale file-history dirs (#4414)
PR #4064 introduced ~/.qwen/file-history/{sessionId}/ for /rewind but had
no cross-session cleanup — directories accumulated indefinitely. This adds
a generic background housekeeping framework with file-history cleanup as
its first user.

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

Closes #4173.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
2026-06-02 14:36:41 +08:00
ChiGao
1285214d10
feat(cli): virtual viewport for long conversations on ink 7 (#4146)
* chore(deps): re-upgrade ink 6 → 7.0.3 (upstream Static remount fix landed)

PR #3860 first upgraded ink 6 → 7.0.2. PR #4083 reverted because of a
TUI regression: `<Static>` did not re-emit items when its `key` prop
was bumped, so `/clear` / Ctrl+O / refreshStatic left the history area
blank under ink 7.0.2.

ink 7.0.3 (released after #4083) contains the exact fixes:

  - be9f44cda Fix: <Static> remount via key change drops new items (#948)
  - 669c4386c Fix: Drop stale <Static> output from fullStaticOutput on identity change (#950)
  - 7c2267c01 Fix `useBoxMetrics` not accepting ref objects with an initial null value (#945)

Changes:
  - `ink` ^6.2.3 → ^7.0.3 (root hoist + cli direct)
  - `react` ^19.1.0 → ^19.2.4 (cli direct; ink 7.0.3 peerDeps requires >=19.2.0)
  - `react`/`react-dom` overrides ^19.2.4 added so the transitive graph
    stays deduped to a single instance (avoids `Invalid hook call` from
    multiple React copies, the classic ink-upgrade hazard)
  - `wrap-ansi` already on ^10.0.0 from #4083's partial-revert (no change)

Verified:
  - `npm ls ink` → single `ink@7.0.3` across all peer deps
  - `npm ls react` → single `react@19.2.4`
  - `npm run typecheck --workspace=@qwen-code/qwen-code` clean
  - `npm run typecheck --workspace=@qwen-code/qwen-code-core` clean
  - Composer.test.tsx 20/20, MainContent.test.tsx 6/6, TableRenderer.test.tsx
    59/59 + 1 skipped — all key UI components green on the new ink

The Static-remount regression is upstream-fixed in 7.0.3, so the
runtime path is restored without needing #3941's overflowY-self-managed
viewport. #3941 (virtual viewport) remains an opt-in performance
feature on top.

* fix(deps,cli): add @types/react overrides + move refreshStatic out of setCurrentModel updater

Two follow-ups from the multi-round audit of the ink 7.0.3 re-upgrade:

1. @types/react / @types/react-dom now pinned to ^19.2.0 in root
   overrides. packages/web-templates still declares @types/react ^18.2.0
   in its devDeps. Today the CLI build is unaffected (web-templates's
   18.x types are nested in its own node_modules and the React-using
   src/insight and src/export-html files are excluded from its tsconfig
   build), but a future reincludes-or-hoist accident would land
   conflicting global JSX namespaces in the CLI compile graph. Match
   the dep dedup we already enforce for `react` and `react-dom` so the
   type graph stays as deduped as the runtime graph.

2. AppContainer's onModelChange handler was calling refreshStatic() as
   a side-effect inside the setCurrentModel updater. React.StrictMode
   double-invokes state updaters in dev, so model swaps fired two
   clearTerminal writes + two <Static> key bumps. The double work was
   masked under ink 6 (key changes were no-ops on <Static>), but ink
   7.0.3 honors key changes — the doubled work is now potentially
   visible as a faster flash-flash on every model switch.

   Refactor: setCurrentModel becomes a pure setter; refreshStatic
   moves into a useEffect keyed on currentModel with a ref-comparison
   guard so the first render doesn't fire. Single clearTerminal write
   per real model change, even under StrictMode.

Verified: npm ls ink → single 7.0.3, npm ls react → single 19.2.4,
npm ls @types/react → 19.2.10 hoisted (npm flags web-templates's 18.x
constraint as overridden, which is the intended behavior). Typecheck
clean across cli + core workspaces.

* docs(design): virtual viewport on ink 7 — analysis + PR sequence

Captures the architectural analysis of how to thoroughly close the
flicker / refresh-storm class of issues (#2950, #3118, #3007, #3838 UI
side, #3899 follow-on) using a virtualized history viewport.

- Surveys claude-code (forked ink) and gemini-cli (@jrichman/ink +
  ScrollableList + VirtualizedList) reference implementations.
- Confirms ink 7 already exposes the primitives needed
  (`useBoxMetrics`, `measureElement`, `useWindowSize`,
  `useAnimation`) — no fork swap required.
- Picks porting gemini-cli's virtualized list components to ink 7 with
  `ResizeObserver` -> `useBoxMetrics` and a custom `StaticRender`.
- Splits the work into V.0..V.4 PRs with scope, dependencies, risk.
- Lists open questions + 11-item approval checklist that must clear
  before V.0 implementation begins.

This is a docs-only PR per the project's design-first workflow. No
runtime code changes.

Generated with AI

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

* feat(cli): virtual viewport for long conversations on ink 7

Port gemini-cli's VirtualizedList + ScrollableList to stock ink 7,
adapting for ink 7's available primitives:

- `overflowY="hidden"` + `marginTop={-scrollTop}` instead of ink-fork's
  `overflowY="scroll"` (ink 7 has proper clip/unclip in render-node-to-output)
- `useBoxMetrics` inside each VirtualizedListItem (Option A) instead of a
  single ResizeObserver WeakMap; reports height changes via onHeightChange
  callback so the parent can update its heights record
- Custom `StaticRender` as `React.memo` with a reference-equality comparator,
  keyed on `itemKey-static-{width}` to freeze completed conversation items
- Character scrollbar column (`│` track / `█` thumb) since ink 7 has no
  native scrollbar prop
- No ScrollProvider / mouse drag (deferred to a follow-up PR)

Wire into MainContent.tsx behind `ui.useTerminalBuffer` setting (Settings
dialog → UI → Virtualized History; default false — opt-in).

Key bindings: Shift+↑/↓ (line), PgUp/PgDn (page), Ctrl+Home/End (top/bottom).

Re-render optimisations:
- renderItem wrapped in useCallback so renderedItems useMemo only recomputes
  when actual deps change (not on every streaming tick)
- Completed history items passed by original object reference so
  VirtualHistoryItem = memo(HistoryItemDisplay) can bail out on stable props
- estimatedItemHeight / keyExtractor / isStaticItem defined as module-level
  constants with no closure deps

Generated with AI

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

* test(cli): add test coverage for virtual viewport scroll bindings and settings

- keyMatchers.test.ts: 6 new test cases for SCROLL_UP/DOWN, PAGE_UP/DOWN,
  SCROLL_HOME/END commands (41 tests total)
- settingsSchema.test.ts: assert ui.useTerminalBuffer is boolean, default false,
  showInDialog true, requiresRestart false

Generated with AI

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

* feat(cli): use ink 7 native overflow for VP pending items

In VP mode, pending items are rendered inside VirtualizedList's
overflowY="hidden" container, which uses ink 7's native clipping
as the viewport guard. Remove the availableTerminalHeight JS-
truncation bound from pending items in renderVirtualItem:

- JS truncation at terminal height would silently cut off content
  the user could scroll to read within the virtual viewport.
- ink 7 overflowY="hidden" on the VirtualizedList container is the
  correct clip guard — no JS line-counting workaround needed.
- Remove uiState.constrainHeight from renderVirtualItem deps (no
  longer referenced in the VP rendering path).

The legacy <Static> path is unchanged.

Generated with AI

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

* perf(cli): binary-search offsets in virtualized list hot path

Replace linear findLastIndex / findIndex scans on the offsets array with
upperBound. Offsets are monotonic by construction, so the lookups inside
the render body and getAnchorForScrollTop drop from O(n) to O(log n).
Material for thousand-turn sessions where the lookup runs on every frame.

* fix(cli): wire ShowMoreLines + skip clearTerminal in VP mode

Two audit-found bugs in the VP path:

1. `<ShowMoreLines>` was outside the `<OverflowProvider>` that wraps
   `<ScrollableList>` in VP mode. `useOverflowState()` returns
   `undefined` outside the provider, so the component returned `null`
   and the "press ctrl-s to show more lines" affordance silently
   disappeared. Move `<ShowMoreLines>` inside the provider so the hook
   sees the live overflow state, matching the legacy path.

2. `refreshStatic()` and `repaintStaticViewport()` wrote
   `clearTerminal` / `cursorTo+eraseDown` to the host terminal
   unconditionally. In VP mode the React tree owns the visible region
   via ink 7's native `overflowY="hidden"` clipping — the physical
   write is a wasted flash on Ctrl+O / Alt+M / model change / resize.
   Guard both writes on `useTerminalBuffer === false`. The
   `historyRemountKey` bump still fires so the legacy `<Static>`
   fallback would still remount if someone toggled the setting mid-
   session.

Extends the targeted-repaint pattern introduced in #3967 to all
refreshStatic call sites, gated by the VP setting instead of by event
type.

* fix(cli): VP renderItem stability + source-copy offsets + heights GC

Three audit-found regressions tightened, in order of severity:

1. **Source-copy index offsets missing in VP** — legacy `<Static>` path
   threads per-item `sourceCopyIndexOffsets` so `/copy mermaid N` /
   `/copy latex N` hints stay stable across continuation messages. VP
   `renderVirtualItem` was not passing this prop, so the copy hints
   shown under each diagram drifted on every `gemini_content` chunk
   (the clipboard mechanism itself still worked from raw history; only
   the displayed number was wrong). Add two lookup tables —
   identity-keyed for static items, index-keyed for pending — without
   changing the VirtualizedList data signature, and thread offsets in
   both render branches.

2. **`renderVirtualItem` callback invalidated on every streaming tick**
   — its deps included `activePtyId` / `embeddedShellFocused` /
   `isEditorDialogOpen`, all of which flip mid-stream when a shell
   tool runs or a dialog opens. Each flip rebuilt the callback,
   invalidated `VirtualizedList.renderedItems`'s useMemo, and forced
   every static item to re-render through `<StaticRender>` — defeating
   the very memoization the design relies on. Move the three pending-
   only fields into a ref read inside the callback. Static-item closure
   now depends only on inputs that legitimately affect static output
   (terminalWidth, slashCommands, getCompactLabel, …). Pending items
   still re-render correctly because their item identity changes per
   tick, so the callback is called fresh each time and reads the
   latest ref.

3. **`pending` items now honour `constrainHeight`** in VP, matching the
   legacy path. Previously VP unconditionally passed `undefined` for
   `availableTerminalHeight` on pending, relying on the viewport
   `overflowY="hidden"` clip to limit visible size — but that hid the
   `<ShowMoreLines>` affordance from the user. Now that ShowMoreLines
   is correctly wired (previous commit), restore parity.

4. **Heights map memory leak** in `VirtualizedList` — `setHeights` only
   grew. Each `/clear` left orphan `h-N` keys; each pending → completed
   transition left orphan `p-N` keys. Add a `useLayoutEffect` that
   prunes entries whose keys are not in the current `data`. Runs in
   layout phase so the prune commits in the same paint as the data
   change — no stale-offsets frame.

* test+fix(cli): VP path coverage + stabilize absorbedCallIds empty Set

Completion-pass artifacts driven by the multi-agent audit:

- Settings description rewritten to enumerate the symptoms VP fixes so
  users with active flicker reports can find the toggle without reading
  the design doc.
- `absorbedCallIds` returns a module-level constant Set when compact mode
  is off, instead of a fresh `new Set()` per render. Fixes a hidden
  cascade: `activePtyId` flip mid-stream → useMemo runs → returns a new
  empty Set → `isSummaryAbsorbed` rebuilds → `renderVirtualItem`
  rebuilds → `VirtualizedList.renderedItems` recomputes → every static
  item re-renders. With the constant, the cascade dies at the source.
  Helps both VP and legacy paths.
- VP-path unit tests for MainContent (4 cases): ScrollableList mounts
  and Static does not when `useTerminalBuffer: true`; ShowMoreLines is
  reachable in VP mode (regression of the OverflowProvider mis-wrap);
  source-copy index offsets thread into renderItem for static items;
  renderItem callback identity is stable across `activePtyId` flips
  (proves the ref-based read keeps StaticRender memo effective).

* fix(cli): stabilize absorbedCallIds in compact mode + gate heights prune + tighten ShowMoreLines test

Round-2 audit follow-ups. Three real findings addressed; one flagged
false positive documented separately.

1. **absorbedCallIds Set identity now content-stable when compact mode is
   on.** The earlier EMPTY constant only short-circuited the compactMode=
   false path; when compact mode is enabled (some users default-on it),
   activePtyId / embeddedShellFocused flips during streaming still
   produced fresh Sets per render even when membership was unchanged,
   restarting the same cascade the pendingStateRef fix was meant to
   avoid. Compare-and-reuse via a ref: if the new Set has identical
   membership to the previous one, return the previous reference.

2. **`heights` map prune in `VirtualizedList` is gated.** Previously
   every streaming tick rebuilt an N-key Set and walked all heights,
   even on the steady-state path where nothing changes. Now only fires
   when the heights record has clearly outpaced live data
   (`size > max(8, 2 × data.length)`) — covers `/clear` and accumulated
   pending → completed transitions, skips the 30-Hz hot path entirely.

3. **VP ShowMoreLines test now actually verifies overflow connectivity.**
   Previous mock unconditionally rendered "SHOW_MORE", so the test only
   proved the JSX mounted — it would still pass if a future refactor
   moved `<OverflowProvider>` out of the VP tree again. The mock now
   reads `useOverflowState()` and emits "OVERFLOW_DISCONNECTED" when the
   context is missing. The VP test asserts both presence of "SHOW_MORE"
   and absence of the disconnected marker, so the regression is now
   caught.

Not addressed:
- Audit P0-1 claim that `renderMode` (Alt+M) / model-change updates
  don't reach VP static items: false positive. `renderMode` is a React
  Context (`RenderModeContext`), and Context propagation traverses the
  tree past `memo` boundaries — MarkdownDisplay's `useRenderMode()`
  consumer re-renders on context change regardless of whether
  `StaticRender` bails out. Verified by reading
  `packages/cli/src/ui/contexts/RenderModeContext.tsx` and
  `MarkdownDisplay.tsx:172`. No code change.
- Audit P1-2 pendingStateRef write-during-render race: speculative,
  relies on a multi-pass render path React 18+ does not currently use.
  Documented assumption in the existing inline comment.

* fix(cli): isolate renderItem errors + defensive height coerce + compact-mode mergedHistory stability

Round-3 audit follow-ups. Three real findings; the rest verified clean.

1. **`renderItem` errors no longer crash the CLI.** Previously a throw
   inside a per-item render propagated through `VirtualizedList`'s
   useMemo into React's commit phase, tearing down the whole Ink tree —
   one bad history record could nuke the session. Wrap each call in a
   try/catch and substitute a small red `[render error] …` text box on
   failure. The row stays in the viewport so the user can scroll past
   it.

2. **Defensive height coerce in offset accumulation.** A buggy
   `estimatedItemHeight` returning NaN / negative / Infinity would
   poison every downstream offset and break the `upperBound` /
   `findLastLE` binary search (which assumes monotonic offsets). Clamp
   to `Number.isFinite(raw) && raw > 0 ? raw : 0`. No-op for the
   in-tree estimators that return 3; insurance against future
   consumers.

3. **`mergedHistory` is content-stable when compact mode is on.** The
   Round-2 absorbedCallIds stability fix didn't reach this path:
   `mergeCompactToolGroups` always allocates a fresh array, and
   `mergedHistory`'s useMemo lists `activePtyId` / `embeddedShellFocused`
   as deps, so every streaming tick mid-shell-tool produced a new array
   even when items aligned. Cascade went `mergedHistory` → offsets map
   → `renderVirtualItem` → every static item re-rendered. Pair-wise
   compare new vs previous and return the previous reference when items
   align. Restores StaticRender memo effectiveness for compact-mode
   users.

Not addressed (audit findings deemed not worth fixing in this PR):
- `scrollToItem` silently no-ops when item is not in data — no current
  caller checks the return value, low impact.
- `allVirtualItems` array spread is O(n) per streaming tick — real but
  not a crash; revisit in a perf-focused follow-up.
- `itemRefs.current` is dead surface (never read) — cosmetic.
- StrictMode-only-in-DEBUG double-invoke paths verified safe.

* test+chore(cli): VP review round 4 — VirtualizedList/useBatchedScroll coverage + cleanups

Addresses wenshao's CHANGES_REQUESTED review on PR #3941.

- Add focused unit tests for `VirtualizedList` (9 cases) covering empty
  data, `renderStatic` full-render, `initialScrollIndex` with
  `SCROLL_TO_ITEM_END`, `targetScrollIndex` anchoring, imperative
  `scrollToEnd` / `scrollToIndex`, per-item `renderItem` error isolation,
  NaN/negative estimator coercion, and out-of-range `initialScrollIndex`
  clamping.
- Add `useBatchedScroll` unit tests (4 cases) covering initial reads,
  pending-value reads in the same tick, post-commit pending reset, and
  callback identity stability across rerenders.
- Remove dead `itemRefs` / `onSetRef` plumbing (declared, written, never
  read; `useCallback` with empty deps was also a stale-closure trap).
- Remove unused `isStatic?: boolean` from `VirtualizedListProps`
  (only `isStaticItem` is actually consumed).
- Tighten the render-phase setState block: each setter is now guarded
  by an equality check so React bails out of redundant updates, and a
  comment documents that this is the React-endorsed "adjusting state
  while rendering" pattern (the synchronous update avoids a one-frame
  flash at the previous position when `targetScrollIndex` changes).

Generated with AI

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

* chore(cli): remove dead `dataRef` from VirtualizedList (round-4 followup)

Declared and written in a `useLayoutEffect` on every `data` change but
never read anywhere in the component. Flagged in wenshao's round-4 review
of PR #3941.

Generated with AI

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

* fix(cli): collapse model-change effect back into one batched handler

wenshao's PR #4119 review correctly flagged that splitting the
onModelChange flow into two effects (b25831b0e) reintroduced the
issue #3899 freeze regression on every model switch:

  1. setCurrentModel(model) commits first, with the OLD
     historyRemountKey.
  2. <Static key={`${historyRemountKey}-${currentModel}`}> sees its
     key change (because currentModel did) and remounts immediately.
  3. MainContent's render-phase progressive-replay reset only fires
     when historyRemountKey changes, so replayCount is still the
     full mergedHistory.length from any prior catch-up.
  4. The remounted Static dumps the entire history in one synchronous
     layout pass — exactly the freeze progressive replay was added
     to avoid (#3899). The second effect's refreshStatic() bump
     arrives a render too late.

Fix: do not split. Both side effects (refreshStatic, which writes
clearTerminal + bumps historyRemountKey, and setCurrentModel) live
in the event handler again, with a ref guard for same-model
notifications. The React.StrictMode concern that motivated b25831b0e
is addressed by keeping the side effect OUT of the setState updater
(it now runs once per event-handler invocation, not once per
double-invoked updater call). Both setState calls land in the same
React batch, so historyRemountKey and currentModel update together —
MainContent's render-phase reset sees the new key, replayCount drops
to the first chunk, and Static remounts with chunked replay intact.

Tests:
- AppContainer.test.tsx: 4 new tests covering the synchronous
  refreshStatic side-effect contract, same-model no-op, ref-guarded
  StrictMode double-invoke, and unsubscribe-on-unmount.
- MainContent.test.tsx: new regression guard — when currentModel
  changes but historyRemountKey is held constant, progressive replay
  must NOT reset (pins the MainContent invariant the two-effect
  refactor accidentally relied on).

Verified: vitest packages/cli AppContainer + MainContent green (82/82).
Typecheck clean.

Generated with AI

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

* fix+docs(cli): VP review round 5 — typecheck, doc drift, scroll keys

PR #4146 review feedback (wenshao + Claude Opus 4.7 audit) addressed:

Code:
- MainContent.test: activePtyId typed as number (was 'pty-xyz' string,
  broke tsc with TS2322 — the test only relies on reference change so
  any number works).
- VirtualizedList: sanitize renderItem error path. Display becomes the
  generic `[render error]` marker; full err goes to debugLogger.debug
  so file paths / partial tool state don't leak to scrollback.
- MainContent: move pendingSourceCopyOffsetsByIndex into a ref so it
  no longer rebuilds renderVirtualItem identity every streaming tick.
  Without this, VirtualizedList.renderedItems useMemo invalidated
  per-tick → JSX rebuilt for every visible item → memo(HistoryItem
  Display) was still bailing but allocations were O(visible) per tick.
- AppContainer: drop the misleading "state-driven scroll reset" claim
  in the VP refreshStatic comment. VP is intentionally near-no-op:
  the React tree owns the visible region, mergedHistory mutation is
  what refreshes the screen, and the remount-key bump is preserved
  only to keep the legacy Static branch in sync if the user toggles
  the flag off mid-session.
- StaticRender: rewrite JSDoc to match reality. The custom React.memo
  is NOT output caching like @jrichman/ink's StaticRender export;
  the comparator rarely matches (parent allocates fresh JSX); the
  real skip happens at memo(HistoryItemDisplay) one level deeper.

Docs:
- docs/design/virtual-viewport: sync file map (drop non-existent
  ScrollProvider.tsx / useAnimatedScrollbar.ts), PR sequence (one PR
  #4146, V.3-V.5 deferred), open-question + checklist resolution for
  #3905 (superseded) and base branch rename.
- docs/users/reference/keyboard-shortcuts: document the 6 VP scroll
  keys (Shift+↑/↓, PgUp/PgDn, Ctrl+Home/End) under a "History
  scrollback (when ui.useTerminalBuffer is on)" section. Previously
  the only discovery path was the Settings dialog description.

Verified: tsc --noEmit -p packages/cli ✓, vitest 160/160 ✓ across
AppContainer / MainContent / VirtualizedList / useBatchedScroll /
keyMatchers / settingsSchema, eslint clean on touched files.

Generated with AI

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

* feat(cli): SGR mouse wheel scroll in VP mode

Recovers the most-felt UX regression vs legacy `<Static>` mode: when
`ui.useTerminalBuffer` is on, legacy users lose mouse wheel as a way
to scroll history (the host terminal stopped seeing the conversation
in its scrollback buffer). This PR enables button-event tracking
(`?1002h`) + SGR coordinates (`?1006h`) while the ScrollableList has
focus, parses wheel events off stdin, and routes them to scrollBy.

Scope kept tight on purpose:
- Wheel only. Hit-testing for scrollbar drag / click-to-position
  needs screen-absolute element coords; stock ink 7's useBoxMetrics
  returns yoga's parent-relative layout. Deferred to V.4 with two
  exit paths (upstream getBoundingBox to ink 7, or local yoga walker).
- Mouse mode is enabled only while ScrollableList is mounted; non-VP
  users never see their terminal flipped into button-event tracking.
- Side effect: native click-and-drag text selection is captured by
  the program. Docs + settings dialog description now spell out the
  Shift / Option (macOS) bypass.

Implementation:
- `ui/utils/mouse.ts` — SGR + X11 parser, ported and trimmed from
  gemini-cli (Google LLC, Apache-2.0). Single-consumer.
- `ui/hooks/useMouseEvents.ts` — enable/parse/disable lifecycle
  hook. Listens on stdin via `useStdin().stdin`, runs handler
  through a ref so callers don't have to memoize.
- `ui/components/shared/ScrollableList.tsx` — subscribe to mouse
  events, route wheel → `scrollBy(±3)`. Also drops a dead outer
  `<Box flexGrow={1}>` wrapper that held an unread containerRef
  and collapsed to zero height in ink-testing-library (the test
  renderer has no flex parent, so flexGrow=1 → 0 height → no items
  ever rendered, which is how this dead code was exposed).

Tests:
- `ui/utils/mouse.test.ts` — 14 cases: SGR parsing (wheel, presses,
  modifiers, move), X11 parsing, fallback chain, incomplete-sequence
  guard (including the >50-byte garbage cap).
- `ui/components/shared/ScrollableList.test.tsx` — 3 cases: wheel
  events shift the rendered window; hasFocus=false makes the mouse
  pipeline inactive (no throw); non-wheel events leave the window
  unchanged. Renders are wrapped in `<KeypressProvider>` (required
  by useKeypress in production but easy to forget in standalone
  tests).

Docs:
- `docs/users/reference/keyboard-shortcuts.md` — adds "Mouse wheel"
  row + the Shift/Option-to-select note.
- `packages/cli/src/config/settingsSchema.ts` — the in-app dialog
  description now mentions mouse wheel and the text-select bypass.
- `docs/design/virtual-viewport/README.md` — §1 status, §5 file map,
  §7 PR sequence all reflect mouse wheel landing in #4146 and the
  V.4–V.7 follow-up split (scrollbar drag / in-app search / alt-
  buffer / host-scrollback dual-write research).

Verified: tsc --noEmit -p packages/cli ✓, vitest 182/182 ✓ across
AppContainer / MainContent / VirtualizedList / ScrollableList /
useBatchedScroll / mouse / keyMatchers / settingsSchema.

Generated with AI

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

* feat(cli): auto-hide animation for VP scrollbar thumb

Pairs with the SGR mouse-wheel work from the previous commit:
when the user actually scrolls, the thumb pops bright; after a
1.5s idle it fades into the dim track so the bar stops competing
with the conversation. The track column itself stays in layout
regardless, so the viewport never reflows mid-flash (which would
trigger per-item re-measure and a visible jitter).

Implementation kept minimal for stock ink 7:
- gemini-cli's `useAnimatedScrollbar` interpolates RGB colors via
  a theme + per-frame setInterval. The terminal can't render
  smooth fades anyway, so this hook collapses the state to a
  binary `isVisible` flag with a single setTimeout. ~75 LoC.
- `VirtualizedList` calls `flashScrollbar()` from a useLayoutEffect
  keyed on `clampedScrollTop`. The very first commit is skipped
  via a ref so initial mount doesn't paint a flash.
- The render switches the thumb glyph (`█` vs `│`) and `dimColor`
  based on `isVisible && inThumb`. Width stays 1 either way.

Tests (6 new):
- initial mount stays hidden (no spurious mount flash)
- flash → visible, hides after idle timeout, successive flashes
  reset the timer (no premature hide), idleHideMs<=0 disables
  auto-hide for tests that want to assert on the visible state,
  unmount cleans up the pending timer.

Doc updates:
- `docs/design/virtual-viewport/README.md` §1 status, §5 file map,
  §7 PR sequence — V.4 row now scopes only the drag/click-jump
  work (still coord-blocked); animated scrollbar moved out of
  deferred and into shipped.
- PR #4146 body — architecture table mentions the auto-hide, new
  files list adds `useAnimatedScrollbar.ts`, test count refreshed
  to 188/188.

Verified: tsc --noEmit -p packages/cli ✓, vitest 188/188 ✓.

Generated with AI

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

* fix(cli): VP review round 6 — ESC bug, CI lint, scope-controlled cleanup

Triage of /review feedback from 2026-05-18 + 2026-05-19. Took the
ones that are real and small; declined the ones that are
false-positive / out-of-scope so this PR stops expanding.

Must-fix:
- CI Lint failure: vscode-ide-companion/schemas/settings.schema.json
  was stale after the keyboard-shortcuts description bump. Regenerated
  via `npm run generate:settings-schema`.
- useMouseEvents.ts had `const ESC = '';` (literal empty string after
  the raw 0x1B byte got stripped somewhere in the source pipeline).
  `buffer.indexOf('', 1) === 1` would have degraded garbage skipping
  to a one-byte scan, and the `else { buffer = ''; break }` branch
  could never run. Fixed by switching to the `'\x1b'` text escape and
  doing the same in `mouse.ts` (which had the raw byte, also fragile).
  Comment explains why.

Small wins (one-liners taken from the review batch):
- ScrollableList: rest-spread separates `hasFocus` from the props
  forwarded to VirtualizedList. Latent collision risk; no behaviour
  change today.
- VirtualizedList: `debugLogger.debug` when isReady=false so blank-
  viewport edge cases (tiny terminal / mid-resize race) become
  diagnosable from the debug log instead of looking like a hang.

Real perf (VP-only):
- MainContent: gated the progressive-Static-replay machinery behind
  `!useVirtualScroll`. The render-phase reset still consumes the
  remount-key bump so flag-off toggles mid-session catch up cleanly,
  but `setReplayCount` and the setImmediate chunking effect are now
  skipped for VP users. Saves ~M/CHUNK_SIZE wasted re-renders per
  Ctrl+O / model change on a 1000-turn session.

Belt-and-braces:
- useMouseEvents: added a `process.on('exit')` handler that writes
  the SGR mouse disable seq again. The React cleanup already covers
  normal unmount, but Ctrl+C / SIGTERM / parent kill bypass it and
  the terminal would otherwise stay in button-event-tracking mode
  after qwen exits.

Explicitly declined / deferred (with reasoning logged on the PR):
- requestAnimationFrame wheel throttle: rAF doesn't exist in Node;
  React 19 already batches state updates within a tick, and the
  renderedItems memo bounds the actual work to visible items. Will
  revisit if profiling shows it.
- Stable pending-item IDs (`p-N` keys shifting on completion): the
  observable jitter is at most one frame of estimated-vs-actual
  height delta. Moderate scope (creation-time ID allocation); fits
  better in a focused follow-up than in this PR.

Verified: tsc --noEmit -p packages/cli ✓, vitest 188/188 ✓ across
the full VP suite.

Generated with AI

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

* fix(cli): scrollBy bottom uses live end anchor in virtualized list

When keyboard scroll reaches the bottom, scrollBy set isStickingToBottom
but anchored via getAnchorForScrollTop(maxScroll), a fixed {index,offset}
pixel anchor. scrollTo/scrollToEnd instead use {index: last, offset:
SCROLL_TO_ITEM_END}, which recomputes the bottom from live item heights
each render. The fixed anchor did not track the last item growing during
streaming, so scroll-to-bottom via keyboard lagged behind new tokens.
Align scrollBy's bottom branch with the sibling methods.

Reported by wenshao in PR review.

* fix(cli): parse mouse events via ink useInput, not a stdin data listener

useMouseEvents attached its own stdin.on('data', ...) listener. Adding a
'data' listener switches stdin into flowing mode, which drains the buffer
before ink's readable + stdin.read() reader (ink App) can consume it, so
all keyboard input routed through useInput was silently starved while
mouse mode was active.

Parse mouse sequences from ink's existing input pipeline via useInput
instead, so there is only one stdin reader. ink captures a full SGR
sequence (ESC [ < .. M/m) as a single CSI event and delivers it with the
leading ESC stripped, so we re-prepend it before parsing. Non-mouse input
does not match and is ignored; ink still routes input to the app's other
useInput handlers, so keyboard navigation keeps working.

Only SGR mode (1006h, which we enable) is parsed via this path; the legacy
X11 encoding is not recoverable through ink's CSI parser, which is the
encoding modern terminals stop emitting once 1006h is set.

Reported by wenshao in PR review.

* fix(cli): parse only SGR in mouse hook to avoid X11 paste misfire

The useInput-based mouse hook called parseMouseEvent, which also tries the
X11 fallback (parseX11MouseEvent). An X11 prefix (ESC [ M + 3 bytes) can
reach the handler via pasted text — ink emits paste content as input when
no paste listener is registered — and would misfire a spurious mouse event.
Call parseSGRMouseEvent directly so only the SGR encoding we enable (1006h)
is parsed, matching the hook's documented contract.

Reported by wenshao in PR review.

* test(cli): assert SGR mouse parser rejects X11 sequences

Locks in the security property behind the parseMouseEvent ->
parseSGRMouseEvent switch in useMouseEvents: an X11 sequence arriving as
pasted text must not misfire a mouse event. Asserts a well-formed X11
sequence is a valid X11 event yet returns null from parseSGRMouseEvent, so
a future revert to parseMouseEvent fails this test.

Reported by wenshao in PR review.

* test(cli): add VP scroll coverage + eslint-disable for useBatchedScroll

Cover keyboard scroll commands (Shift+Up/Down, PageUp/Down, Ctrl+Home/End),
scrollBy/scrollTo imperative API (positive/negative/overflow/clamp), and
auto-scroll-during-streaming state machine (stick-to-bottom, disengage on
user scroll, re-engage on scrollToEnd). Add missing eslint-disable-next-line
for intentionally dep-free useLayoutEffect in useBatchedScroll.

Generated with AI

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

* chore(cli): remove trailing whitespace in useBatchedScroll

The eslint-disable-next-line comment was removed by eslint --fix as an
unused directive (exhaustive-deps does not flag a useLayoutEffect with
no dependency array). Clean up the residual blank line.

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-06-02 13:57:17 +08:00
Dragon
bec97f445d
feat(skills): add agent reproduction workflows (#4118)
* chore(skills): add codex reproduce workflows

* feat(agent-reproduce): implement agent reproduction workflow and supporting scripts

* feat(skills): capture reference agent state diffs
2026-06-02 12:10:54 +08:00
pomelo
5d05cded3e
feat(core): add simplify bundled skill (#3570)
* feat(core): add simplify bundled skill

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

* test(cli): stabilize SettingsDialog restart prompt test

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

* fix(skills): use agent tool instead of task in simplify skill

The simplify skill referenced the 'task' tool for launching review passes,
but Qwen Code exposes 'agent' as the callable subagent tool ('task' is only
a legacy permission alias). Using 'task' would cause /simplify to stall when
trying to launch parallel review passes.

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

* docs: document simplify bundled skill

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

* Update packages/core/src/skills/skill-manager.test.ts

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

* fix(core): repair simplify skill tests

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

* Update packages/core/src/skills/bundled/simplify/SKILL.md

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

* fix(skills): address simplify review feedback (read-only passes, gitignore scope, safer dead-code removal)

- drop inert `argument-hint` frontmatter (argumentHint is never parsed or
  rendered anywhere; no other bundled skill uses it)
- mark Step 2 review passes read-only so edits stay isolated to Step 4
- narrow the no-diff fallback to `git ls-files --modified --others
  --exclude-standard` so ignored build output is excluded
- require a repo-wide caller check before removing code
- make the commands.md row state it edits code directly
- assert non-conflicting bundled skills survive cross-level dedup

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

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: wenshao <wenshao@U-K7F6PQY3-2157.local>
2026-06-02 11:45:09 +08:00
Dragon
aaab5f2027
feat(cli): notify when background shells finish (#4355) 2026-06-02 11:38:52 +08:00
Dragon
c52c9f172b
fix(insight): Harden insight facet normalization and empty qualitative handling (#3557)
* Harden insight facet normalization and empty qualitative handling

* feat: enhance AtAGlance component to accept target sections for dynamic rendering
2026-06-02 10:51:15 +08:00
JerryLee
4e1b3827e9
fix(core): tolerate unsupported Streamable HTTP GET SSE (#4521)
Fixes #4326
2026-06-02 10:09:01 +08:00
yao
2d8052b02c
feat(cli): add respectUserColors and hideContextIndicator options for statusline (#4670)
* feat(cli): add respectUserColors option to preserve ANSI colors in
     statusline command output

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

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

* docs: update statusline configuration docs with respectUserColors and hideContextIndicator
2026-06-02 09:31:56 +08:00
易良
cea15a118b
fix(core,cli): replace full-history structuredClone with shallow/tail variants to prevent OOM on resume (#4644)
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,cli): replace full-history structuredClone with shallow/tail variants to prevent OOM on resume

Several UI and service call sites clone the entire chat history via
structuredClone(getHistory()) every turn. On a resumed session with
thousands of entries, each clone allocates 150-200 MB transiently.
When multiple async side-requests overlap (suggestion generation,
auto-title, checkpointing), multiple clones coexist on the heap,
pushing V8 past its limit within 10 turns (2 GB heap cap).

Changes:
- AppContainer.tsx: use getHistoryTail(40, true) instead of
  getHistory(true) + slice(-40)
- btwCommand.ts: same pattern, use getHistoryTail(40, true)
- sessionTitle.ts: use getHistoryShallow() (read-only filtering)
- sessionRecap.ts: use getHistoryShallow() (read-only filtering)
- useGeminiStream.ts: use getHistoryShallow() for checkpoint
  serialization (only needs to survive JSON.stringify)

Closes #4624

* fix(test): update mocks for getHistoryShallow/getHistoryTail in sessionTitle and btwCommand tests

* fix(cli): migrate remaining getHistory() clone sites to shallow/tail variants

- AppContainer.tsx rewind path: getHistory() → getHistoryShallow()
  (only used read-only by computeApiTruncationIndex)
- Session.ts ACP rewind: getHistory() → getHistoryShallow()
  (only walks entries to compute truncation index)
- Session.ts stop-hook: getHistory() + filter(.model).pop() →
  getLastModelMessageText() (O(1) backward scan, no clone)

* fix(core): use client-level getHistoryShallow with fallback

sessionTitle.ts and sessionRecap.ts were calling
chat.getHistoryShallow() directly, bypassing the client-level
wrapper that provides a getHistory() fallback when the chat
implementation doesn't support shallow reads. Use
geminiClient.getHistoryShallow() instead.

Update test mocks to match the new call site.

* fix(test): add getHistoryShallow and getLastModelMessageText to Session test mocks

Session.ts now calls chat.getHistoryShallow() in rewindToTurn and
chat.getLastModelMessageText() in the Stop hook. Update all mockChat
instances in Session.test.ts to provide these methods.
2026-06-01 22:53:48 +08:00
JerryLee
68e4819d73
fix(cli): use session channel when closing ACP sessions (#4522)
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
Detach closeSession/killSession from the session entry's owning channel instead of the current attach target, so the correct channel is decremented and killed during channel overlap (old channel dying while a fresh channel is current). Extracts findChannelInfoForEntry/detachSessionIdFromEntryChannel helpers with unit + integration coverage. Fixes #4325.
2026-06-01 16:23:42 +08:00
qqqys
788f215117
Improve hooks matcher display (#4545)
* feat(cli): improve hooks matcher display

* test(cli): cover hooks navigation levels
2026-06-01 16:18:10 +08:00
jinye
7448724d8a
fix(core): preserve uid in atomicWriteFile to avoid breaking shared-write files (#4431)
* fix(core): preserve uid/gid in atomicWriteFile to avoid breaking shared-write files

atomicWriteFile uses write-to-tmp + rename for crash atomicity. POSIX
rename creates a new inode owned by the calling process's euid/egid, so
the rename silently strips the original uid/gid. On shared-write setups
(e.g. a group-writable file owned by another user in a shared workspace
where the current user has group-write access), every Write/Edit/
NotebookEdit through qwen-code would reset ownership to the running
user and effectively revoke write access for the original collaborators.

The fix:

1. If the target exists and is owned by a different uid/gid than the
   process's effective uid/gid (and we are not root), fall back to
   in-place writeFile. This truncates the existing inode in place,
   preserving uid/gid. The trade-off is loss of crash atomicity for
   this specific case — an acceptable trade for not silently breaking
   shared-write file ownership.

2. If running as root, atomic rename is still used, and ownership is
   restored via chown(uid, gid) after the rename. Root can chown back;
   non-root cannot, hence the in-place fallback for non-root.

3. Windows is unaffected (no POSIX ownership semantics).

Tests:

- New: in-place fallback on uid mismatch — verify content updates, mode
  preserved, and inode unchanged (the inode is the signal that the
  fallback path ran rather than rename).
- New: same scenario triggered via gid mismatch.
- New: positive case — ownership matches → atomic rename → inode changes.

Regression: a v0.16.0 user reported "every write turns a world-writable
file into one other users can no longer write." Bisected to #4096 which
introduced atomicWriteFile + write-to-tmp + rename.

* fix(core): route root through in-place fallback + doc/test follow-ups

Review follow-ups on the atomic-write ownership fix:

1. Remove the root-special-case (rename + post-rename chown). chown
   silently fails inside user-namespaced or CAP_CHOWN-stripped Docker
   containers, which re-triggers the original bug for root-in-Docker
   users — exactly the scenario this fix was reported against. Routing
   root through the same in-place fallback as non-root eliminates this
   failure mode and drops an untestable branch (chown-back can't be
   exercised under non-root CI).

2. Document the three properties traded away by the in-place fallback:
   crash atomicity, concurrent-reader isolation, inotify watcher
   semantics (MODIFY vs MOVED_TO).

3. Document that the in-place fallback surfaces EACCES when the file's
   mode forbids the current user from writing — this is correct
   behavior (atomic rename used to silently replace files the user had
   no permission on, which was arguably a privilege issue).

4. Replace the brittle "see step 6 in the function doc" comment with a
   step-number-independent reference.

5. New test covering the EACCES path: chmod 0o444 + mocked geteuid
   triggers the fallback, fallback hits the read-only file, EACCES
   propagates cleanly, original content is preserved.

* fix(core): harden in-place fallback against symlink/unlink/inode races + doc/test follow-ups

Review follow-ups on #4431 ownership-preservation fix:

CRITICAL — in-place fallback security hardening (wenshao review):

The path-based `fs.writeFile(targetPath, ...)` fallback introduced
three races that the prior `rename(tmp, target)` form did not have:

1. Non-regular files (FIFO/socket/device): fs.writeFile calls
   open(O_WRONLY|O_CREAT|O_TRUNC). On a FIFO this blocks forever
   waiting for a reader. On a character/block device it writes to
   the actual device. The rename path replaced these with a
   regular file.

2. Symlink-swap TOCTOU: an attacker with parent-dir write can swap
   targetPath for a symlink between our stat and our writeFile.
   fs.writeFile follows symlinks at the destination; POSIX rename
   does not. In the very "shared-write workspace / Docker bind-mount"
   scenarios this PR targets, this lets a directory-writable
   attacker redirect agent writes elsewhere (e.g. /etc/passwd if
   the agent runs as root).

3. Unlink race: if targetPath is unlinked between stat and write,
   O_CREAT silently recreates it owned by the calling user — the
   exact ownership change the fallback was designed to prevent.
   Silent regression to the pre-fix bug under this race.

Fix: extract the fallback into writeInPlaceWithFdGuards():

  - open(target, O_WRONLY | O_TRUNC | O_NOFOLLOW) — no O_CREAT, so
    unlink-race surfaces ENOENT instead of silently recreating; and
    O_NOFOLLOW rejects symlink-swaps with ELOOP.
  - fstat(fd) verifies the bound inode's uid/gid still match
    existingStat — refuses the write if an inode-swap happened
    between stat and open.
  - Write through the fd (locked to the verified inode), chmod
    through the fd, close.

Caller now gates the fallback on existingStat.isFile() — non-regular
targets fall through to the atomic path which has well-defined
"replace special-file with regular-file" semantics.

DOC / TEST follow-ups:

- Add hardlink-propagation as a 4th trade-off in the in-place
  fallback JSDoc (review comment #4): rename creates a new inode so
  sibling hardlinks keep old content; in-place truncate+write keeps
  the inode so all hardlinks see new content.

- Update atomicWriteJSON JSDoc to note the write is now
  *conditionally* atomic (review comment #5): atomic when uid/gid
  matches the process, in-place when ownership differs. Previously
  the JSDoc still claimed unconditional atomicity.

- Update caller comments at runtimeStatus.ts and
  worktreeSessionService.ts that advertised crash-atomic writes via
  tmp+rename — those guarantees are now conditional (review
  comment #6).

- Add mode + tmp-leftover assertions to the gid-mismatch test to
  match the uid-mismatch test (review comment #2 — test
  consistency). Without these, a gid-fallback regression that
  silently dropped permissions or left a tmp file would not be
  caught.

- New test: FIFO + ownership mismatch must take the atomic path,
  not in-place (verifies the existingStat.isFile() guard works;
  hang on in-place would trip vitest timeout).

- New test: writing through a symlink with ownership mismatch
  exercises the resolve-then-stat-then-open flow and verifies the
  symlink itself is preserved.

Tests: 192/192 pass (atomicFileWrite + write-file + edit +
fileSystemService).

* fix(core): defer O_TRUNC and verify dev+ino in writeInPlaceWithFdGuards

PR #4431 review follow-up (wenshao critical):

The previous form opened with `O_WRONLY | O_TRUNC | O_NOFOLLOW`, which
truncated the bound file *before* the fd-bound fstat verification ran.
If an attacker swapped the path between the caller's stat and our
open, we would truncate the attacker's substituted inode (destroying
unrelated content) before detecting the swap.

Two fixes:

1. Open without O_TRUNC. Verify dev+ino+uid+gid+isFile match
   expectedStat through fh.stat(). Only then call fh.truncate(0)
   through the validated fd.

2. Expand the verification beyond uid+gid to include dev+ino+isFile.
   uid+gid alone misses a same-owner inode swap (attacker replaces
   the path with a different inode they own). dev+ino is the strong
   identity check; isFile catches a swap to FIFO/socket/device after
   the caller's existingStat.isFile() gate.

JSDoc updated to enumerate the four guards (NOFOLLOW, no CREAT, no
TRUNC at open, dev+ino+uid+gid+isFile via fstat) and explain why
truncation must wait until after verification.

192/192 tests pass.

* fix(core): close FIFO swap race with O_NONBLOCK + cover EOWNERSHIP_CHANGED path

PR #4431 review follow-up (deepseek-v4-pro via /review):

CRITICAL — FIFO swap TOCTOU:

The caller's `existingStat.isFile()` gate uses stat data captured
earlier. An attacker with parent-dir write can swap the regular file
for a FIFO between the caller's stat and our open inside
`writeInPlaceWithFdGuards`. The previous `O_WRONLY | O_NOFOLLOW` open
would then block indefinitely waiting for a FIFO reader; O_NOFOLLOW
only catches symlinks.

Fix: add O_NONBLOCK to the open flags. Defense in depth:

- On a reader-less FIFO, `open(O_WRONLY | O_NONBLOCK)` returns ENXIO
  immediately — no hang.
- If the FIFO has a reader (open succeeds), the subsequent fstat
  isFile() check still refuses the write via EOWNERSHIP_CHANGED.
- For regular files, O_NONBLOCK is a no-op.

CRITICAL test gap — EOWNERSHIP_CHANGED branch untested:

The primary TOCTOU defense (fdStat dev/ino/uid/gid/isFile vs
expectedStat) had no coverage. Exported `writeInPlaceWithFdGuards` so
it can be unit-tested directly:

- New test: simulate post-stat inode swap (unlink + recreate at same
  path), call helper with stale stat, assert EOWNERSHIP_CHANGED and
  that the attacker's content survives.
- New test: simulate post-stat regular→FIFO swap, assert open fails
  fast (ENXIO) or fstat catches it — either way no hang, no write.

DOC fix:

JSDoc said "we open read-write without truncating" but the code uses
O_WRONLY. Wording corrected to "write-only".

194/194 tests pass.

* fix(core): fix flaky inode-swap test + apply review follow-ups

PR #4431 review follow-up (glm-5.1 via /review) — 7 suggestions adopted,
1 partially adopted, 0 rejected:

CI FIX (Ubuntu test failure on tmpfs inode reuse):

The EOWNERSHIP_CHANGED inode-swap test used unlink+create to simulate
a post-stat swap. On Linux tmpfs the freshly-freed inode number is
often reused by the immediately-following create, so dev+ino remained
identical and the guard didn't trip (intermittent on Ubuntu CI; macOS
APFS happened to allocate different inodes). Switched to rename(decoy,
target) which moves an existing distinct inode into place, guaranteed
to differ from the original.

CODE:

- Wrap fh.writeFile failure after fh.truncate(0) with
  EINPLACE_WRITE_FAILED + cause, so callers see explicitly that the
  file was truncated and the write didn't complete (otherwise they
  see raw ENOSPC/EIO and may wrongly assume the original is intact
  given this lives in atomicFileWrite.ts).
- Skip fh.chmod when euid is neither root nor expectedStat.uid —
  chmod is guaranteed to fail with EPERM in that case (POSIX requires
  owner or root). Avoids a guaranteed-failing syscall on every call.
- Caller catches ENOENT from writeInPlaceWithFdGuards and falls
  through to atomic rename path. If the file was deleted between
  caller's stat and our open there is no ownership to preserve; the
  rename path correctly creates a new file at targetPath.

DOC:

- Replaced "defends against four races" with "hardened against
  post-stat races" (the bullet list has 5 items, the count was wrong).
- Reworded "non-regular targets must not reach this function" to
  describe defense-in-depth — O_NONBLOCK + !fdStat.isFile() reject
  post-stat regular→FIFO/socket/device swaps. The old wording made
  it look like O_NONBLOCK was redundant.
- Documented the dual chmod behavior (root vs non-root with foreign
  uid) inline.

TESTS:

- Added happy-path test for writeInPlaceWithFdGuards (write succeeds,
  inode preserved, mode preserved).
- Added ENOENT regression test (verifies the missing-O_CREAT
  property — if file unlinked between stat and open, no silent
  recreate with caller's uid).
- Renamed the misleading "O_NOFOLLOW guard" test (it actually tests
  resolve-through-symlink, not O_NOFOLLOW) to reflect what it does,
  and added a direct ELOOP test that drives writeInPlaceWithFdGuards
  with a path whose final component is a symlink — that's the real
  O_NOFOLLOW exercise.
- Fixed the FIFO test to pass a stat captured from the FIFO itself
  (not a stale regular-file stat) so only the FIFO-specific defense
  fires, not the inode/dev mismatch from a different file.

NOT ADOPTED:

- Skip-when-non-root chmod optimization adopted (small, useful), but
  the larger "structured chmod error model" deferred — best-effort
  matches the existing tryChmod pattern at file scope.

197/197 tests pass.

* fix(core): wrap truncate err + post-write nlink check + guard close + chmod sync

PR #4431 review follow-up (qwen-latest-series-invite-beta-v34 via /review)
— 7 of 10 suggestions adopted, 3 deferred:

CODE:

- **EINPLACE_TRUNCATE_FAILED wrap** (review #3291863048): symmetric to
  the existing EINPLACE_WRITE_FAILED — distinguishes "truncate failed,
  original intact" from "write failed post-truncate, original lost".

- **Post-write nlink === 0 check** (review #3291863059):
  EINODE_UNLINKED_DURING_WRITE detects the fstat-to-close window where
  a concurrent rename-over drops our bound inode's link count to zero
  and our write goes to an anonymous inode close will free. Silent
  data loss path now surfaces.

- **fh.close() guarded in finally** (review #3291863044): close failure
  on NFS/FUSE was masking the original try-body exception (including
  the meaningful EOWNERSHIP_CHANGED, EINPLACE_*, EINODE_*). flush:true
  already fsync'd, so close-after-flush is best-effort.

- **fdStat.uid in canChmod** (review #3291863055 part 1): use the
  fd-bound verified value instead of expectedStat.uid. Defense in depth
  — a future weakening of the fstat guard won't silently widen chmod
  privilege.

- **fh.sync() after chmod** (review #3291863053): chmod is metadata,
  not covered by writeFile({ flush: true }). A crash before lazy
  metadata flush would lose the mode restoration (matters for
  setuid/setgid). One extra syscall, best-effort.

- **@remarks freshness contract** (review #3291863051 partial): JSDoc
  now spells out that expectedStat MUST be a fresh stat captured
  immediately before the call. Stale stats nullify every guard.

- **Concurrent-writer limitation noted** (review #3291863061 partial):
  added a "Known limitation — no advisory locking" paragraph to JSDoc
  rather than adopting flock (Linux-specific, NFS issues, scope
  expansion). Callers needing multi-process coordination should layer
  their own lockfile.

- **@throws documentation** (review #3291863051 partial): four
  documented error codes (EOWNERSHIP_CHANGED, EINODE_UNLINKED_DURING_WRITE,
  EINPLACE_TRUNCATE_FAILED, EINPLACE_WRITE_FAILED).

TESTS:

- **EINPLACE_WRITE_FAILED via FileHandle.prototype.writeFile monkey-patch**
  (review #3291863040): triggers the data-loss path, asserts the wrapped
  code + message + cause, and verifies the file is empty (truncate ran).

- **canChmod=false actually skips chmod** (review #3291863055 part 2):
  prior uid-mismatch test had desiredMode === current mode, couldn't
  distinguish "skipped" from "no-op". New test uses desiredMode=0o755
  on a 0o644 file under canChmod=false → asserts mode stays 0o644.

NOT ADOPTED:

- ENOENT/ELOOP/ENXIO catch extension (review #3291863043): keeping the
  strict refusal for swap-to-special-file. Silent fallthrough-to-replace
  was pre-PR atomic-rename behavior, but in shared-write workspaces
  (this PR's target users) a special-file appearing at the target path
  is a signal worth surfacing, not papering over.

- Diagnostic logging (review #3291863049): the function has no logger
  dependency today; adding one is an architecture decision outside
  this PR's scope. The path taken is implied by the side effects
  (inode preserved vs new) but agreed: out-of-band telemetry would
  help ops. Defer to follow-up.

- flock advisory locking (review #3291863061 main): scope expansion;
  Linux-specific semantics, NFS edge cases. Documented as known
  limitation instead.

- Integration test for ENOENT fallthrough at atomicWriteFile level
  (review #3291863043 part 1): ESM module bindings prevent monkey-
  patching writeInPlaceWithFdGuards from outside. The unit test for
  the helper's ENOENT path covers the throwing behavior; the catch is
  3 lines and review-visible. Defer until a refactor opens an
  injection seam.

- Error code string constants export (review #3291863051 part 3): two
  codes don't merit a constant module. Magic strings are fine at this
  size.

199/199 tests pass.

* docs(core): sync writeRuntimeStatus JSDoc with conditional-atomic contract

PR #4431 review follow-up: function-level JSDoc still claimed
unconditional "Atomically write" and "never sees a partially written
file", inconsistent with the module-level docblock updated in earlier
commits. Updated to describe the conditional-atomic behavior (atomic
when uid/gid matches, in-place fallback when ownership differs) and
explicitly note the concurrent-reader visibility trade-off in the
fallback path. Links to atomicWriteJSON for the full contract.

Doc-only change. 199/199 tests pass.

* fix(core): add explicit fh.sync() — FileHandle.writeFile ignores flush option

PR #4431 review follow-up (qwen3.7-max via /review):

CRITICAL — FileHandle.writeFile silently ignores flush:

Node.js FileHandle.writeFile takes an early-return path that bypasses
the flush option entirely (the option is only honored on the
path-based fs.writeFile form). Our previous code passed
{ flush: true } to fh.writeFile and relied on the implicit fsync.
The only explicit fh.sync() was nested in the chmod block guarded by
canChmod — which is FALSE precisely when a non-root group member
writes to a group-writable file they don't own (the exact shared-write
scenario this PR targets). Net effect: in that branch, zero fsync.
Data sits in the kernel page cache; a crash before lazy flush leaves
the file empty (truncate succeeded) or partially written.

Fix:
- Drop flush from the fhWriteOptions object (silently ignored anyway).
- Add an explicit `fh.sync()` after writeFile succeeds, gated on
  options.flush. Runs BEFORE the chmod block so the canChmod=false
  branch also fsyncs.
- The chmod-block fh.sync() becomes metadata-only (covers the mode
  change), as the data is already on disk.

Updated comments to reflect the actual semantics rather than the
incorrect "writeFile({ flush: true }) fsyncs" assumption.

TESTS (partial adoption of review #3293252349):

- EINPLACE_TRUNCATE_FAILED: sibling test to EINPLACE_WRITE_FAILED.
  Monkey-patches FileHandle.prototype.truncate to throw EIO; asserts
  err.code + cause + "original content is intact" message, and
  verifies the file's original bytes are unchanged (truncate didn't
  run).
- Buffer in in-place fallback: locks in binary fidelity (byte-exact
  comparison) so a future encoding-passthrough regression for Buffer
  data would be caught.

NOT ADOPTED in this commit:

- EINODE_UNLINKED_DURING_WRITE test: requires post-write fh.stat()
  mocking with call-count discrimination (first call: real stat for
  verification; second call: nlink=0). The monkey-patch pattern works
  but is fragile; deferred to a follow-up that may also refactor the
  helper to accept an injectable stat fn for cleaner testability.

201/201 tests pass.

* fix: correct stale flush comment + add fh.sync() regression test

- Fix misleading close() comment that said "flush:true already
  fsync'd" — the explicit fh.sync() does the actual fsync, not the
  flush option (which is silently ignored on FileHandle.writeFile).
- Add regression test verifying fh.sync() is called when flush:true
  and skipped when flush is absent, preventing silent removal of the
  core durability fix.

Addresses wenshao review threads from 2026-05-23.

* test: add EINODE_UNLINKED_DURING_WRITE regression test

Monkey-patches FileHandle.stat to return nlink:0 on the post-write
check, verifying the nlink guard throws with the correct error code.
Addresses wenshao review from 2026-05-28.

* simplify: replace writeInPlaceWithFdGuards with plain fs.writeFile

Address yiliang114's review (CHANGES_REQUESTED):

1. [Critical] Remove ~120 lines of fd-level TOCTOU hardening
   (writeInPlaceWithFdGuards) — over-engineering for a local CLI.
   The in-place fallback now uses plain fs.writeFile + tryChmod,
   matching the EXDEV fallback pattern.

2. [Suggestion] Fix macOS GID false-positive: only compare uid in
   ownershipWouldChange(). macOS inherits parent dir GID for new
   files, so egid !== file.gid was a false positive that needlessly
   dropped crash atomicity.

3. [Suggestion] Trim 60+ lines of JSDoc to project style (AGENTS.md:
   "default to none, add only when WHY is non-obvious").

Net: -748 lines. 24 tests pass.

* fix: restore Stats type import (TS2304 build failure)

* docs: narrow scope from uid/gid to uid-only preservation

The gid check is intentionally skipped because macOS inherits the
parent directory's GID for new files, making egid !== file.gid a
false positive. Update comments and PR description to match the
actual implementation scope.

* test: add inode assertion to symlink ownership-mismatch test

Proves the in-place fallback actually ran instead of atomic rename.
2026-06-01 16:15:15 +08:00
Dragon
59c283670e
Hide internal docs from docs site (#4357) 2026-06-01 15:55:14 +08:00
顾盼
707f1bdb5f
fix(cli): persist /memory toggle state across dialog reopen (#4650)
Some checks are pending
Qwen Code CI / Lint (push) Blocked by required conditions
Qwen Code CI / Classify PR (push) Waiting to run
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
The Auto-memory / Auto-dream / Auto-skill rows initialized their state
from Config getters, which are frozen at startup and never reflect a
setValue() write. Each /memory reopen re-mounts the dialog and re-reads
that stale snapshot, so a just-flipped toggle appeared to revert. Read the
initial state from the live merged settings instead, matching the existing
write path (bareMode semantics preserved).

Also switch the test's `act` import to `react` — the previously used
@testing-library/react is declared in package.json but not installed, so
the suite could not run — and add a mount/unmount/remount regression test.
2026-06-01 11:29:02 +08:00
Yufeng He
1c48e4121b
fix(core): apply output language to side queries (#4636)
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
2026-05-31 18:03:05 +08:00
Yufeng He
a3bc42dc55
fix(core): harden context error text collection (#4632) 2026-05-31 18:02:39 +08:00
yao
d074ed8434
feat(cli): Add settings JSON corrupted warning dialog (#4560)
* feat(cli): add settings corruption recovery dialog

* fix(cli): address review1 comments – copy instead of rename, clean DEBUG logs & lint

* test(cli): add SettingsCorruptedDialog keyboard navigation & callback coverage

* Update packages/cli/src/config/settings.test.ts

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

* Update packages/cli/src/config/settings.ts

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

* fix(cli): drop corrupted-path filter, use corruptedPath as sole signal

* fix(test): add missing truncateToItem to mockUIState

* fix(cli): prevent env var stale state and unnecessary normalization writes

* Apply suggestions from code review

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

* fix(cli): resolve merge conflict and fix formatting errors from PR review update

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

* fix(cli): prevent validateAuthMethod from consuming corruption env vars

* test(settings): fix scope guard test to actually set env vars

* fix(cli): separate corruption warning from migrationWarnings

* fix(ui): extract CORRUPTED_SUFFIX constant to avoid magic string

* fix(ui): emit error on restore failure in onExit handler

* test(ui): strengthen up/down arrow test to assert selection on target line

* test(settings): strengthen double-corruption and scope guard assertions

* test(ui): assert selection moves in onContinue test before pressing Enter

* fix(ui): include corruption dialog in dialogsVisible to suppress global keypress

* fix(ui): resolve type incompatibility in dialogsVisible and test helpers

* fix(cli): block corruption env var injection and harden error handling

Co-Authored-By: wenshao <shaojin.wensj@alibaba-inc.com>

* fix(cli): harden corruption env var guard with helper, comment, and regression test

* fix(cli): add afterSpawn callback to clear corruption env vars after spawn

Co-Authored-By: qwen3.7-max <noreply@qwen-code.ai>

* fix(cli): share mockSpawn instance between default and named exports in relaunch test

* Update packages/cli/src/utils/relaunch.ts

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

---------

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: qwen3.7-max <noreply@qwen-code.ai>
2026-05-31 18:02:20 +08:00
Yufeng He
093545b71f
fix(cli): hide completed sticky todos (#4635)
* fix(cli): hide completed sticky todos

* fix(cli): remeasure sticky todos on status changes
2026-05-31 17:59:12 +08:00
Yufeng He
9dafd60d5d
fix(core): enforce adjacent tool results (#4622)
* fix(core): enforce adjacent tool results

* fix(core): handle orphan cleanup edge cases
2026-05-31 17:57:22 +08:00
Yufeng He
5743c2a05c
fix(acp): drop discontinued Qwen OAuth method (#4639) 2026-05-31 17:56:05 +08:00
Kagura
1014848234
fix(config): load home .env vars before settings ${VAR} resolution (#4466) (#4474)
* fix(config): load home .env vars before settings ${VAR} resolution (#4466)

${VAR} placeholders in settings.json (e.g. MCP server headers) could not
reference variables defined in ~/.qwen/.env because resolveEnvVarsInObject()
ran before loadEnvironment() loaded the .env file into process.env.

Add preLoadHomeEnvVars() that loads all variables from home-level .env files
(~/.qwen/.env, ~/.env) into process.env in no-override mode before settings
env var resolution. Workspace .env files and settings.env are still handled
by the existing loadEnvironment() call after settings merge.

Fixes #4466

* test: add env var resolution from .env file test (#4466)

* fix(config): use customEnv fallback for home .env resolution (#4466)

Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
Signed-off-by: kagura-agent <kagura.agent.ai@gmail.com>

* fix: align .env precedence and fix test mock

- Use ??= in getHomeEnvFallbackVars() so the more-specific file
  (~/.qwen/.env) wins over ~/.env, matching dotenv first-occurrence-wins
- Fix test 'QWEN_HOME is set': use customSettingsPath matching the
  runtime QWEN_HOME instead of the module-level USER_SETTINGS_PATH
  constant (fixes mock mismatch that cascaded into 7 test failures)
- Remove unused homeEnvPath variable (tsc/eslint warning)
- Add debugLogger.warn in catch block for I/O errors
- Document that the dict intentionally skips
  PROJECT_ENV_HARDCODED_EXCLUSIONS (substitution scope is narrower
  than process.env population)

Signed-off-by: kagura-agent <kagura.agent.ai@gmail.com>

* test: add .env fallback, precedence, and error path tests

Address R2 reviewer suggestions:
- Add ~/.env fallback positive path test (no QWEN_HOME)
- Add precedence test: ~/.qwen/.env wins over ~/.env (first-write-wins)
- Add error path test: readFileSync throws, loadSettings still succeeds
- Add code comment explaining intentional path discrepancy between
  getHomeEnvFallbackVars() and getUserLevelEnvPaths()

* docs: clarify customEnv precedence comment per reviewer suggestion

---------

Signed-off-by: kagura-agent <kagura.agent.ai@gmail.com>
Co-authored-by: Claude Opus 4 (1M context) <noreply@anthropic.com>
2026-05-31 15:03:46 +08:00
Yan Shen
54b6b204d2
fix(cli): stabilize statusline preset ordering (#4634)
* fix(cli): stabilize statusline preset ordering

* test(cli): make statusline helper contracts explicit

Add direct coverage for exported statusline preset helper behavior requested during PR review.

Constraint: Address PR #4634 review feedback for direct exported helper tests.
Rejected: Relying on existing integration coverage | Direct tests were requested for exported helper contracts.
Confidence: high
Scope-risk: narrow
Directive: Keep preset item ordering and reasoning formatting contracts directly covered when these helpers change.
Tested: cd packages/cli && npx vitest run src/ui/statusLinePresets.test.ts; cd packages/cli && npx vitest run src/ui/statusLinePresets.test.ts src/ui/components/StatusLineDialog.test.tsx src/ui/hooks/useStatusLine.test.ts; cd packages/cli && npx eslint src/ui/statusLinePresets.ts src/ui/statusLinePresets.test.ts; git diff --check
Not-tested: Full repository test suite.
2026-05-31 14:44:22 +08:00
JerryLee
7a31c80f0e
fix(core): guard oversized resumed history sends (#4531)
* fix(core): guard oversized resumed history sends

* fix(core): preserve history on hard rescue stop

* fix(core): defer hard-rescue compression recording until guard passes

* test(core): clarify hard-rescue compression status

* test(core): cover hard rescue rollback invariants

* docs(core): clarify hard rescue rollback comment

* docs(core): document deferred compression recording
2026-05-31 14:27:35 +08:00
ZevGit
3fc1849892
feat(core): add memory pressure monitor (#4403)
* feat(core): add memory pressure monitor

* fix(core): address memory pressure review

* fix(core): isolate session memory cleanup
2026-05-31 13:57:52 +08:00
Dragon
60f6c5aba8
fix(core): surface Anthropic empty stream provider errors (#4540)
* fix(core): surface Anthropic empty stream provider errors

* fix(core): address PR #4540 review feedback

- Move convertAnthropicResponseToGemini inside try/catch so converter
  errors are redacted the same way as API errors (Critical fix)
- Add regression guard: assert createImpl called once on normal streams
  to catch accidental double-request regressions
- Replace provider-specific Chinese mock error with generic '400 quota
  exceeded' for portability
- Add comment explaining why test uses message_delta instead of bare
  message_stop
2026-05-31 13:15:55 +08:00
jinye
a1043ee3c9
fix(core): emit enable_thinking on DashScope when reasoning is disabled (#4505)
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
2026-05-31 12:32:52 +08:00
jinye
c699738f9a
fix(rewind): use notification type for mid-turn messages to fix count mismatch (#4580)
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
Mid-turn user messages (typed during tool execution) were added to UI
history as type 'user', causing isRealUserTurn to count them. But in
the API history, they are merged into the preceding tool_result Content
(alongside functionResponse), making them invisible to isUserTextContent.

This UI/API count mismatch caused computeApiTruncationIndex to return -1,
producing a false "Cannot rewind to a compressed turn" error.

Fix: change mid-turn messages from type 'user' to type 'notification'
in both live and resume paths so isRealUserTurn no longer counts them.

Closes #4579
2026-05-29 21:32:11 +08:00
qwen-code-ci-bot
54fc50360c
chore(release): v0.17.0 [skip ci]
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
2026-05-29 18:02:11 +08:00
顾盼
2a2f92aafd
fix(core,cli): label screenshot-triggered compaction accurately in the auto-compact notice (#4623)
The auto-compaction notice hardcoded "approached the input token limit"
even when the screenshot-overflow trigger fired. In computer-use sessions
that's misleading: compaction can fire on accumulated tool screenshots
while token usage is far below the window limit (observed: the notice
claimed "approached the input token limit" at ~116K/1M tokens when it was
actually the image-count trigger).

Add ChatCompressionInfo.triggerReason ('token_limit' | 'image_overflow' |
'manual'); compress() sets it to 'image_overflow' when the screenshot
trigger is what let it through the cheap gate. Both the TUI
(useGeminiStream) and ACP (Session) notices now show an accurate clause.
2026-05-29 17:35:57 +08:00
Yufeng He
a6f640a941
fix(core): use undici fetch for IDE proxy requests (#4607) 2026-05-29 17:16:17 +08:00
qwen-code-ci-bot
efba25f063
chore(release): v0.16.2 [skip ci]
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
2026-05-29 17:10:07 +08:00
qqqys
96f08ebe22
Emit PermissionDenied hooks for AUTO classifier blocks (#4376)
* fix(core): emit permission denied hooks for auto blocks

* test(cli): cover permission denied hook display

* docs(core): clarify permission denied hook semantics

* fix(core): respect disabled hooks for permission denied

* test(core): cover auto permission denied hooks

* test(cli): cover acp permission denied hooks

* test(core): cover unavailable permission denial hooks

* test(core): cover auto-approved permission hooks
2026-05-29 17:09:07 +08:00
顾盼
365409366d
refactor(core)!: replace tail-preservation compaction with summary + restoration attachments (#4599)
* refactor(core): rewrite compression prompt to 9-section claude-code-style format

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

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

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

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

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

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

* refactor(core): simplify extractRecentFilePaths internals

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

Adds one test covering the maxFiles=0 path.

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

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

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

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

* refactor(core): harden readFileSizeAdaptive size accounting

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

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

* feat(core): add file restoration block composer

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

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

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

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

* feat(core): add image restoration block composer

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

* feat(core): add composePostCompactHistory orchestrator

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Four small follow-ups from review of 641a0eadd:

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

wenshao review round 2 on PR #4599.

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

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

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

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

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

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

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

wenshao review round 4 on PR #4599.

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

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

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

wenshao review round 5 on PR #4599.

- Regression test for the restoration-failure catch fallback: mock
  composePostCompactHistory to reject and assert compaction still returns
  COMPRESSED (no escape to sendMessageStream / breaker bypass) with the
  trailing functionCall folded into the ack and the trailing text dropped.
- Document that the fold branch intentionally keeps only functionCall parts
  (the trailing turn's text is already captured in the summary); the
  asymmetry with the with-attachments branch is deliberate.
2026-05-29 16:20:13 +08:00
顾盼
39cc9b3e6f
feat(computer-use): zero-config built-in via open-computer-use MCP (#4590)
* feat(computer-use): add tool name constants

* feat(computer-use): hardcode upstream tool schemas

* feat(computer-use): add enableComputerUse setting (default true)

* chore(vscode-ide-companion): sync settings schema for computerUse

* feat(computer-use): MCP stdio client for upstream binary

* feat(computer-use): ComputerUseTool wrapper + bootstrap stub

* feat(computer-use): register 9 deferred tools when enabled

* feat(computer-use): persist install approval state under ~/.qwen

* feat(computer-use): detect upstream permission errors

* feat(computer-use): bootstrap state machine (install + permissions)

* feat(computer-use): wire install approval to qwen-code confirm UX

* chore(computer-use): script to sync schemas from upstream

* fix(computer-use): consolidate package spec, surface download progress, correct version comment

* docs(computer-use): implementation plan

* fix(computer-use): forward image content parts to the model

* fix(computer-use): coerce string numbers to integers + clarify required fields

* fix(computer-use): detect missing Screen Recording + re-spawn doctor across permission transitions

* fix(computer-use): auto-reconnect on transport-closed errors

* fix(computer-use): sync schemas with upstream canonical contract

Regenerated schemas.ts from upstream open-computer-use@latest via
scripts/sync-computer-use-schemas.ts. Key contract fixes:

- element_index: type integer → string (upstream reads via optionalString)
- x/y/from_x/from_y/to_x/to_y: type integer → number (upstream uses optionalDouble)
- scroll: adds required direction enum + requires element_index (not pages)
- click: adds optional mouse_button string enum (left/right/middle)
- Descriptions updated to upstream verbatim text (no "REQUIRED:" prefix)

* fix(computer-use): bidirectional type coercion for string element_index

Rename coerceNumericStrings → coerceTypes and add Direction 2:
when schema declares type: "string" and model sends a number,
stringify it (e.g. element_index: 2 → "2"). This fixes the upstream
runtime error where optionalString returns nil for numeric element_index.

Direction 1 (string → number for integer/number fields) is preserved
unchanged for x/y coordinate fields.

Update tests: element_index coercion tests now reflect string schema
type; add new "coerces integer element_index to string" test cases.

* feat(prompts): strengthen deferred-tools guidance to prevent param guessing

* fix(computer-use): clearer wording for permission-transition onboarding message

* fix(computer-use): only probe permissions on fresh client start, not every tool call

* docs(computer-use): correct comment about permission-revocation recovery behavior

* fix(computer-use): pin upstream version exactly to prevent schema drift

* fix(computer-use): use pinned package spec in client singleton to prevent schema drift

* fix(computer-use): decouple install gate from per-action permission grant

* fix(computer-use): route registration through PermissionManager-aware registerLazy

* fix(computer-use): probe via upstream doctor instead of get_app_state on Finder

The previous probe called get_app_state on Finder, which has the side
effect of activating the target app via upstream's unhide / open -b /
AXRaise logic. Result: Finder popped to the foreground once per fresh
session even when the user's task had nothing to do with it.

The doctor CLI reads TCC + runtime preflight and prints a summary to
stdout, exiting silently when permissions are granted. When any
permission is missing, doctor launches the onboarding window via
LaunchServices (which dedups so repeated invocations focus the
existing window). We parse the stdout summary and rely on doctor's
own window-launching for the UX trigger — no separate spawnDoctor
call needed.

Side effect for steady-state sessions (permissions already granted):
ZERO Finder activation. The probe spawns npx -y doctor once per fresh
client start (~200-500ms), and that's it.

Also bumped pollIntervalMs default from 2s to 5s to amortize the
npx-spawn overhead during the rare permission-grant flow.
2026-05-29 10:39:51 +08:00
易良
27b06290c9
fix(cli): track model-sent slash command history (#3826)
* fix(cli): prevent file paths from being treated as slash commands (#1804)

When users input file paths starting with '/' (e.g. '/api/apiFunction/...',
'/Users/name/path'), they were incorrectly parsed as slash commands, resulting
in "Unknown command" errors. The input was discarded instead of being sent to
the model for processing.

Root cause: isSlashCommand() only checked for a '/' prefix without validating
whether the first token actually looks like a command name. Any '/' prefix
triggered the slash command flow, and when no matching command was found, the
error was shown with no fallback.

Fix: Add looksLikeCommandName() that validates command names contain only
[a-zA-Z0-9:_-]. Both isSlashCommand() and handleSlashCommand() now check the
first token — if it contains path separators, dots, or non-ASCII characters,
the input falls through to normal model processing instead of the command
dispatcher.

Closes #1804

* fix(cli): allow dots in command names and fix prettier formatting

Address review feedback:
- Allow '.' in looksLikeCommandName() regex to support extension-qualified
  commands like gcp.deploy (CommandService renames conflicts as ext.cmd)
- Add regression tests for dot-named commands in both commandUtils and
  slashCommandProcessor
- Fix prettier formatting in slashCommandProcessor test file

* fix(cli): handle slash command review edge cases

* docs(cli): align slash command validation comment

* fix(cli): preserve slash prompt ordering

* fix(cli): reject shell-metacharacter slash tokens

* test(cli): align slash command action mocks

* fix(cli): track model-sent user turns

* fix(cli): narrow slash path handling scope

* fix(cli): split slash routing follow-up

* fix(cli): tighten slash routing follow-up scope

* fix(cli): address slash routing review gaps

* fix(cli): keep slash history follow-up focused

* fix(cli): align resume history item typing

* fix(cli): address slash history review suggestions

* test(cli): harden slash command history item ids

* chore(cli): use HistoryItemWithoutId in useEditorSettings.test

* refactor(cli): keep setSessionName optional in useSlashCommandProcessor

main currently exposes setSessionName as an optional trailing parameter.
The earlier addition of updateItem pushed it before updateItem and
forced every caller — including AppContainer and all tests — to pass an
explicit undefined. Reorder so updateItem stays required and
setSessionName remains optional, preserving the prior ergonomic.

* test(cli): fix slash command processor hook args

* fix(cli): harden slash command history metadata
2026-05-29 10:27:17 +08:00
gwinthis
7bed56b9b6
feat(telemetry): foundation for skill-based RT optimization (P0+P1) (#4565)
Some checks are pending
Qwen Code CI / Classify PR (push) Waiting to run
Qwen Code CI / Lint (push) Blocked by required conditions
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Blocked by required conditions
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* docs(design): add RT optimization design doc

Two-round review trail documenting the analysis path: original D1-D4
proposal, code-level verification in §6 that recanted the cost estimates,
and §7 ROI reordering after DashScope ephemeral cache implementation was
confirmed already in place — which collapsed D2's net benefit and led to
deferring D2 and D4 as won't-fix.

The doc is preserved as the canonical record of why the obvious-looking
directions (fast-model routing, prevalidate scheduling) turn out to be
dead ends, so future work doesn't relitigate the same conclusions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(design): add reduce-rounds-via-skill-design with spec-first gating

Companion design to rt-optimization-design.md. The core argument:
the real lever for reducing agent loop rounds is at the skill/tool
design layer, not the agent framework. Round 2 in §1.2's baseline
exists because Round 1's skill didn't return a complete answer —
fixing that per-skill collapses 3 rounds into 2, an angle the
original framework-centric proposal completely missed.

Layout:
- §0 acceptance spec is the front-loaded gate: engineering specs
  lock at P-1, statistical thresholds lock at P1.5 (after baseline),
  per-skill specs are data-driven and live in PR descriptions
- §3-§4 three-layer plan: telemetry → per-skill rewrites → prompt
  guidance for concurrent tool calls; each layer is independently
  measurable and reversible
- §5.3 stop-loss lines split into result + process metrics to catch
  the "looks like progress, no actual ROI" failure mode early

The doc was reviewed by codex twice — once on initial draft (caught
qwen-logger dead-code path, batch_size state-passing cost, prompts.ts
line drift) and once after §0 was added (caught spec rigidity, missing
per-skill template, framework boundary case). Both rounds' findings
were either applied or explicitly recorded as not-adopted with reasons
inline.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(telemetry): connect logSkillLaunch to QwenLogger

logSkillLaunch in loggers.ts went only through the OTLP path, while
QwenLogger.logSkillLaunchEvent in qwen-logger.ts had no callers anywhere
in the repo — leaving the skill_launch event invisible to any backend
that consumes from the qwen-logger pipeline rather than OTLP.

Mirror the logToolCall pattern at loggers.ts:230: forward the event to
QwenLogger before the OTLP path so the call still reaches QwenLogger when
the OTEL SDK is not initialized.

This is P0 of docs/design/rt-optimization/reduce-rounds-via-skill-design.md
§4.1.1b — a prerequisite for the prompt_id propagation in P1 so the
SkillLaunchEvent / ToolCallEvent join in §4.1.2 has data to query against.

Tests: 2 new cases under describe('logSkillLaunch') covering forwarding
to QwenLogger plus the OTLP-uninitialized branch; loggers.test.ts now
47/47 pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(telemetry): thread prompt_id through SkillLaunchEvent

To join skill_launch events with the subsequent tool_call events they
trigger, SkillLaunchEvent now carries the prompt_id of the user turn
that fired the skill. The scheduler already holds the request and its
prompt_id; the missing piece was getting that id into the invocation
that does the actual logSkillLaunch call.

Wiring:
- SkillLaunchEvent constructor adds a required prompt_id parameter so
  the field can never be silently undefined in a backend join.
- SkillToolInvocation exposes setPromptId(id) and stores the value;
  the four logSkillLaunch sites in execute() pass this.promptId through.
- CoreToolScheduler.buildInvocation grew an optional fourth promptId
  argument and duck-types setPromptId on the freshly-built invocation,
  mirroring the existing setCallId hook. The two callers (setArgs path
  at L1036 and the main schedule path at L1497) pass
  request.prompt_id / reqInfo.prompt_id.
- qwen-logger.logSkillLaunchEvent forwards prompt_id in the RUM event
  properties so the join works on the qwen-logger pipeline too.

The empty-string default on SkillToolInvocation.promptId is deliberate:
direct invocations (e.g. buildAndExecute in tests) that skip the
scheduler still log a valid event, and downstream queries can filter
prompt_id != '' to exclude non-scheduled launches from joins.

Implements P1 of docs/design/rt-optimization/reduce-rounds-via-skill-design.md
§4.1.1 — required prerequisite for the SkillFollowupRecord SQL in §4.1.2.

Tests: 2 new cases in skill.test.ts cover the setPromptId path and the
empty-default path; loggers.test.ts updated for the new 3-arg signature.
256 tests pass across loggers / skill / coreToolScheduler suites.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(scheduler): cover prompt_id propagation through buildInvocation

The duck-typed setPromptId hook added in the previous commit went
through unit tests on each side independently — SkillToolInvocation
tests verified that setting the field changes the logged event, and
the loggers tests verified the SkillLaunchEvent shape — but the
integration point in CoreToolScheduler.buildInvocation that wires the
two together was only exercised indirectly. Same is true of the older
setCallId hook it mirrors, which had no test at all.

Two cases here close that gap on the scheduler side:

- A purpose-built PromptIdAwareTool whose invocation records every
  setPromptId call; the test schedules a request with a known
  prompt_id and asserts the invocation captured it. This is the
  positive contract.

- The existing TestApprovalTool (no setPromptId) scheduled through
  the same path to confirm the duck-type guard does not throw when
  the method is absent. This is the backward-compatibility contract
  that lets every existing tool keep working unchanged.

The two cases together pin both branches of the typeof check in
buildInvocation, so future refactors of that hook cannot regress
silently. 165 tests in the suite still pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(telemetry,scheduler,skill): close P0/P1 coverage gaps

Three blind spots remained after the initial P0+P1 work — each one
was a path that the production code change had already touched
mechanically but no test was pinning it against future regressions.

qwen-logger.ts logSkillLaunchEvent now has two cases asserting that
prompt_id reaches the RUM event properties, on both the success and
failure branch. Previously the loggers.test.ts spy stopped at "method
was called" and never inspected the payload qwen-logger built.

skill.ts had four logSkillLaunch sites, but only the happy path and
the empty-default path were tested. The commandExecutor-success
branch (L386), not-found branch (L399), and thrown-exception branch
(L482) now each have a test that sets promptId, drives execute()
through that specific path, and asserts the emitted event carries
both the right success flag and the right prompt_id. This catches
the failure mode where someone later edits one of those branches
and forgets the promptId argument — replace_all guaranteed today's
correctness but no test would catch a regression tomorrow.

CoreToolScheduler.buildInvocation now has two direct unit tests
that exercise the method through a type-assertion cast. Reaching
the L1036 setArgs path through the public API would require mocking
modifyWithEditor + the filesystem + an editor type, which would
dwarf the change under test. The direct call covers both L1036 and
L1497 simultaneously: when promptId is supplied the duck-typed
setPromptId is invoked; when it is omitted, the captured field
stays undefined and no throw happens.

298 tests pass across loggers / qwen-logger / skill / scheduler suites.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* review(PR #4565): address Copilot + github-actions feedback

Five spots flagged by the automated review on #4565. Three were real
and worth fixing; two were comment-quality touch-ups that traveled
along with the same patch.

- SkillLaunchEvent.prompt_id is now optional with a default of '',
  removing the breaking-change footprint on the exported telemetry API.
  All current internal callers still pass the value explicitly through
  the SkillToolInvocation.promptId field, so the §0.1 spec ("prompt_id
  串联") is still enforced in production paths — type-level enforcement
  just steps aside in favor of API stability, with §0.5 治理 covering
  the discipline at the process layer.

- The skill-design doc §4.1.1 used to claim "BaseToolInvocation 已有
  request.prompt_id" which is wrong: BaseToolInvocation only holds
  params, and the prompt_id flows through CoreToolScheduler's duck-typed
  setPromptId hook (mirroring setCallId). The doc now reflects the
  actual implementation and notes that the earlier text was the bug.

- CoreToolScheduler.buildInvocation gained a short JSDoc explaining
  why the two extra args (callId, promptId) are optional — they
  match the existing duck-type pattern that lets older tools and
  non-scheduler call sites work without implementing the setters.

- skill.test.ts adds a one-comment note next to the first setPromptId
  cast explaining that setPromptId is a scheduler-only hook, not part
  of the public ToolInvocation interface.

- SkillToolInvocation.promptId field comment shrank from 8 lines to 2
  with a pointer to the design doc so the inline noise drops without
  losing the empty-string semantics.

Pre-existing scope-creep findings (Chinese-only doc, mock-config
duplication in scheduler tests, redundant optional-chain comment,
prompt_id sanitization for an internally-generated UUID) are
deliberately not addressed here — see the reply on PR #4565 for
disposition per item.

298 tests still pass across loggers / qwen-logger / skill / scheduler.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-29 02:17:40 +08:00
yuanyuanAli
7732554805
feat(channels): add Feishu (Lark) channel adapter (#4379)
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(channels): add Feishu (Lark) channel adapter

* fix(channels/feishu): fix webhook stop button, memory leak, spin-wait timeout, and reaction cleanup

* fix(channels/feishu): fix security, stability and build issues from PR review

* fix(channels/feishu): fix card lifecycle, streaming limits, and download safety from CR round 2

* fix(channels/feishu): harden webhook, card lifecycle, and disconnect cleanup from CR round 3

* fix(feishu): clarify stoppedMessages JSDoc to match actual cleanup behavior

* fix(channels/feishu): handle post messages without language key wrapper in quote context

* fix(channels/feishu): fix webhook signature bypass, stop-button double-send, and blockStreaming duplicates from CR round 4

* fix(channels/feishu): harden card lifecycle, markdown splitting, and defensive guards from CR round 5

* fix(channels/feishu): harden card lifecycle, markdown splitting, and defensive guards from CR round 5

- Set cardCreationFailed on onPromptStart failure to prevent retry spiral
- Skip throttle updates when card creation permanently failed
- Handle code fences in hard-split and table-stripping fallbacks
- Use parity-based fence detection in splitByTables (align with splitChunks)
- Add cs.stopped and else branch in onPromptEnd to prevent timer race and state leak
- Mark cardState.stopped after busy-wait timeout to abandon orphaned in-flight creation
- Apply MAX_CARD_CHARS truncation with fence parity in onResponseComplete
- Sanitize senderId before <at> tag interpolation
- Use replaceAll + callback form for mention replacement
- Floor token expiry to prevent thundering herd on expire:0
- Add log for stop-button auth rejection
- Fix stoppedMessages JSDoc to match actual cleanup lifecycle
- Fix test fixture to match "still creating" scenario
- Fix typecheck errors in test file (TS2571, TS4111)
- Add stop-button auth negative path tests (operator mismatch, missing operator, missing sender)
- Replace spanning regex in table-stripping with line-by-line stripTables() to resolve CodeQL ReDoS warning

* fix(channels/feishu): fix HMAC bypass, prompt injection, SSRF, and card lifecycle from CR round 5-6

Security:
- Fix webhook HMAC bypass: use defineProperty(non-enumerable) for headers instead of prototype shadowing
- Fix cross-user prompt injection: mark quoted content as untrusted with explicit marker
- Fix SSRF: validate all Feishu IDs with FEISHU_ID_RE before URL interpolation in 6 endpoints
- Fix safeSenderId regex: add hyphen to character class so ou_abc-def-123 is not rejected

Card lifecycle:
- Set cardCreationFailed on onPromptStart failure to prevent retry spiral
- Skip throttle updates when card creation permanently failed
- Fallback to plain message delivery when cardCreationFailed with accumulated text
- Track creationTimer in CardSessionState so cleanupCard/disconnect can cancel orphaned card creation
- Add cs.stopped and else branch in onPromptEnd to prevent timer race and state leak
- Mark cardState.stopped after busy-wait timeout to abandon orphaned in-flight creation
- Apply MAX_CARD_CHARS truncation with fence parity in onResponseComplete
- Preserve atPrefix in streaming truncation to prevent @mention visual snap
- Account for suffix and fence reserve in truncation maxBody calculation
- Clean up auxiliary maps after handleInbound when gate rejects the message
- Clean up blockStreaming mode Map entries in onPromptEnd
- Skip bare @mention without question text

Markdown:
- Handle code fences in hard-split and table-stripping fallbacks
- Use parity-based fence detection in splitByTables (align with splitChunks)
- Replace spanning regex in table-stripping with line-by-line stripTables() to resolve CodeQL ReDoS warning

Defensive guards:
- Sanitize senderId before <at> tag interpolation
- Use replaceAll + callback form for mention replacement
- Floor token expiry to prevent thundering herd on expire:0
- Add log for stop-button auth rejection

Tests:
- Fix stoppedMessages JSDoc to match actual cleanup lifecycle
- Fix test fixture to match "still creating" scenario
- Fix typecheck errors in test file (TS2571, TS4111)
- Add stop-button auth negative path tests (operator mismatch, missing operator, missing sender)
- Assert cancelSession called in stop-button happy-path test

* fix(channels/feishu): add request timeouts, token dedup, and harden file/quote sanitization

* fix(channels/feishu): harden card lifecycle, webhook auth, and resource cleanup from CR round 7

* fix(channels/feishu): harden card lifecycle, mention handling, and error recovery
2026-05-28 20:11:00 +08:00
jinye
34b7d472ef
fix(telemetry): improve LogToSpan bridge error info and TUI handling (#4482)
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(telemetry): improve LogToSpan bridge error info and TUI handling

The OTel `LogToSpanProcessor` bridge (used when traces+metrics are over
OTLP but logs aren't, e.g. Alibaba Cloud ARMS) had two diagnostic issues:

1. Empty error messages. When the OTLP HTTP exporter callback returned
   `{ code: FAILED, error }`, `error.message` is the HTTP reason-phrase —
   always empty on HTTP/2. The bridge printed literally
   `[LogToSpan] export failed: code=1 error=` with zero actionable info.
   Now we surface `name`, `httpCode` (only when numeric), and a 200-byte
   `data` snippet from the underlying OTLPExporterError, with JSON-escape
   on user content so embedded newlines can't tear the log line.

2. TUI pollution. The processor wrote diagnostics to `process.stderr`
   directly. Ink only manages stdout, so those writes punched through
   into the rendered terminal area. The processor now accepts an
   injectable `diagnosticsSink`; in interactive mode `sdk.ts` injects a
   sink that routes through `debugLogger.warn` (file-backed). Non-
   interactive runs (CI/scripts) keep the default stderr sink so export
   failures remain visible on the canonical batch-diagnostic channel.

Backward compatibility is preserved: the legacy numeric-arg constructor
keeps stderr behavior; the options-object overload gains the new field.
Other raw `process.stderr.write` sites in the CLI (errors.ts,
startupProfiler.ts, useGeminiStream.ts, etc.) have the same TUI-leak
pattern but are intentionally left out of this PR.

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

* fix(telemetry): address PR #4482 review comments

- Fix TS2353 compile error in keeps-processing-after-sink-throw test:
  the mock callback type was narrowed to `{ code: number }` and rejected
  the `error?: Error` field that `ExportResult` actually carries. Widen
  the type. (wenshao Critical — this was the root cause of CI lint/test
  failures across all 3 OS.)

- JSON.stringify the payload of the `export threw` diagnostic so a
  synchronously-thrown error with embedded newlines stays on one line,
  same single-line invariant enforced by `formatExportError`. Add
  coverage for both the newline case and the non-Error throw branch.
  (wenshao Suggestion)

- Remove the dead `makeFailingProcessor(err)` call in the JSON-escape
  test that was immediately overwritten — the orphaned processor
  retained a live `setInterval` timer with no cleanup. (wenshao
  Suggestion)

- Rename the "200 bytes" test name and comment to "200 characters" to
  match the actual `string.slice(0, 200)` (UTF-16 code units) behavior;
  add a note on the cap being a leak/noise budget, not a hard byte
  limit. (Copilot 2x)

- Strengthen the non-interactive test to actually trigger a failed
  export against the real `LogToSpanProcessor` and assert the default
  sink writes to stderr, not just that `diagnosticsSink === undefined`.
  (github-actions High #2)

- Reword the "shell-active bytes" comment to "characters that would
  break log parsing" — the actual concern is log-line tearing, not
  shell semantics. (github-actions Medium)

- Update class JSDoc to mention the diagnostics-sink responsibility
  alongside the bridge purpose. (github-actions Low)

- Minor JSDoc wording fix on `LogToSpanDiagnosticsSink` type for
  clarity around the no-trailing-newline contract. (github-actions Low)

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

* test(telemetry): cover two unreachable formatExportError branches

Add coverage for two paths flagged by the DEEP-tier review on #4482:

- `err.message || err.name || 'unknown'` chain: the third branch (both
  message and name empty) was never exercised. Scenario: minified
  environments that strip `Error.name`. Test constructs
  `Object.assign(new Error(''), { name: '' })` and asserts the output
  contains `error="unknown"`.

- `typeof extra.data === 'string' && extra.data.length > 0` guard: the
  empty-string case (HTTP response with empty body) was never tested,
  so a future loosening to `!== undefined` would silently start
  emitting `data=""`. Test asserts `data=` is absent.

Both branches are real and reachable in production failure modes; the
tests are guards for the documented intent.

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

* fix(telemetry): tighten LogToSpan diagnostics per wenshao review

- Quote `error="unknown"` in the err-missing early return so it matches
  the JSON.stringify output produced when message+name fall back to
  'unknown'. Two paths now emit identical greppable output for
  semantically identical "unknown error" states.

- Widen the duck-typed cast to `code?: number | string` and add a
  load-bearing comment on the `typeof === 'number'` guard. The type now
  matches reality (Node networking errors surface string codes like
  ECONNREFUSED), preventing a future simplification to `if (extra.code)`
  that would mislabel networking errors as HTTP statuses.

- Reuse `formatExportError` in the sync-throw path so a synchronously-
  thrown OTLPExporterError surfaces its httpCode and data, matching the
  callback-failure path. Non-Error throws still fall back to
  JSON.stringify on String(err) to preserve the single-line invariant.

- Include batch span count in the timeout diagnostic
  ("(N span(s))") — lets an operator distinguish slow network from
  oversized batch when troubleshooting timeouts.

- Add a test for non-string truthy err.data (Buffer) — the `typeof ===
  'string'` guard's false branch was only covered for undefined and
  empty string, so a future refactor relaxing the guard would silently
  start emitting binary garbage with no test to catch it.

- Document the QWEN_DEBUG_LOG_FILE=0 trade-off at the sink wiring site:
  interactive mode plus disabled debug log = full diagnostic silence.
  This is an accepted user opt-in trade-off; falling back to stderr
  would re-introduce the TUI pollution this injection prevents.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
2026-05-27 20:13:51 +08:00
Kagura
c425037998
fix(cli): surface startup warnings on stderr before TUI render (#4448) (#4461)
* fix(cli): surface startup warnings on stderr before TUI render (#4448)

When settings.json has invalid JSON, the file is renamed to
.corrupted.<timestamp> and a warning is added to startupWarnings.
Previously these warnings were only rendered inside the TUI's
Notifications component, which can be obscured by the onboarding
flow ('Connect a provider') — leaving users unaware their settings
were silently reset.

Now all startup warnings are written to stderr before the TUI takes
over. This ensures the message appears in the terminal scrollback
regardless of what the TUI shows. In non-interactive mode (--prompt,
piped stdin) this is the *only* output channel for these warnings,
closing a gap where they were collected but never emitted.

* fix(cli): emit settings warnings before relaunch to ensure parent surfaces them

Move getSettingsWarnings() stderr emission to right after loadSettings(),
before the sandbox/relaunch block. This ensures the parent process prints
corruption warnings before relaunchAppInChildProcess() spawns the child
and exits.

Add regression test verifying getSettingsWarnings returns non-empty,
human-readable warnings containing 'invalid JSON' when settings.json
has broken content.

Addresses review feedback from @wenshao on #4461.
Fixes #4448

Signed-off-by: kagura-agent <kagura@openclaw.dev>

---------

Signed-off-by: kagura-agent <kagura@openclaw.dev>
2026-05-27 19:46:03 +08:00