mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-19 16:28:28 +00:00
5770 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
tanzhenxin
|
8beb97eecf
|
feat(tools): keep ask_user_question always-visible to surface clarification UX (#4041)
Previously ask_user_question was deferred behind ToolSearch (shouldDefer: true), so the model only saw it as a name in the deferred-tools section and had to discover it via tool_search before invoking. In practice the model rarely went through that discovery step and instead asked clarifying questions in plain prose, losing the structured multiple-choice UX the tool exists to provide. Flip shouldDefer to false so the schema lives in the initial tool list. Add a unit-test guard so a future refactor doesn't silently put it back behind ToolSearch. |
||
|
tanzhenxin
|
d7a25682e6
|
refactor(core): route side-query LLM calls through runSideQuery chokepoint (#3775)
* refactor(core): route side-query LLM calls through runSideQuery chokepoint
Folds every one-shot side-query call site through a single `runSideQuery`
entry point with `thinkingConfig.includeThoughts: false` and `fastModel`
(falling back to main) as the default policy. Adds a text-mode sibling
to the existing JSON-mode helper, plus a `BaseLlmClient.generateText`
primitive that calls `ContentGenerator.generateContent` directly so
side queries get neither user-memory wrapping nor the main-prompt
fallback that `geminiClient.generateContent` applies.
Migrated call sites: session title, recap, tool-use summary, /rename,
follow-up suggestion (direct path), ACP rewrite, project /summary,
arena approach summary, chat compression, web-fetch, insight analysis,
subagent spec generation. Six call sites override the helper defaults
explicitly (subagent gen, suggestion, ACP rewrite, /summary, compression,
insight) where main-model quality or caller-supplied model matters.
The /summary path additionally fixes a latent bug: text extraction
previously did not strip thought parts, so on thinking models the
saved `.qwen/PROJECT_SUMMARY.md` could leak `reasoning_content` into
the file. The chokepoint now strips thought parts and the request
itself goes out with thinking off.
Best-effort cosmetic callers (recap, tool-use summary, kebab rename,
suggestion) opt into `maxAttempts: 1` so transient outages don't burn
seven retries on output the user will likely never see. `isInternalPromptId`
recognises the `side-query:` prefix automatically so new call sites are
filtered without per-site allowlist updates.
Removes the `getAgentContentGenerator` workaround in `InProcessBackend`
and the `getAgentSummaryGenerator` indirection in `ArenaManager` —
arena approach summaries now run through the chokepoint against
`fastModel`, giving every agent a neutral arbiter rather than a
self-summary on its own model.
* fix(core): guard isInternalPromptId against undefined prompt_id
logToolCall calls isInternalPromptId(event.prompt_id), and tool-call
events from useToolScheduler can carry an undefined prompt_id. The
side-query refactor added promptId.startsWith(SIDE_QUERY_PROMPT_PREFIX)
without a falsy guard, so the missing id crashed the logger and broke
six useToolScheduler tests across all OS / Node matrix entries on CI.
* fix(cli,core): polish runSideQuery callers from review feedback
- Cap web-fetch, chat-compression, and ACP rewrite at maxAttempts: 1.
These paths degrade gracefully on failure (tool error, NOOP fallback,
null return), so 7 retries only delays the user-visible outcome.
- /summary now carries the main session's system instruction so the
summarizer keeps the coding-assistant role, project context, and
user memory instead of summarizing the chat in isolation.
- Add isInternalPromptId tests for the side-query: prefix so future
callers minted via runSideQuery stay filtered out of recordings.
* refactor(core): document runSideQuery defaults and surface promptId in errors
- Add JSDoc on the model and config fields of SideQueryJsonOptions and
SideQueryTextOptions so the fastModel-first defaulting and the
thinkingConfig.includeThoughts: false default are visible at the API
surface, not buried in resolveDefaultModel / applyThinkingDefault.
- BaseLlmClient.generate{Json,Text} error wraps now include promptId
in the message and pass { cause: error }, so a side-query failure
identifies which call site failed and preserves the original stack.
- Add tests covering maxAttempts forwarding (present + omitted) and
rejection propagation for both JSON and text modes — the conditional
spread is non-trivial and was previously unverified.
* fix(core): preserve per-model provider routing in side queries
BaseLlmClient was bound to the main session's ContentGenerator and only
swapped the request `model` field, so side queries targeting a fast or
alternate model inherited the main provider's baseUrl, credentials, and
sampling settings — breaking cross-provider configurations.
Move per-model generator/authType resolution out of GeminiClient and into
BaseLlmClient as `resolveForModel`. Both generateJson and generateText
now build a per-model ContentGenerator (with cache) when the request
targets a non-main model and pass the resolved retry authType through
to retryWithBackoff. GeminiClient.generateContent delegates to the same
resolver so there is a single source of truth.
Also pin the /forget destructive selector to the main model — the
runSideQuery default moved to fast model in this branch, but /forget
acts on the selection without confirmation, so a weaker fast model
could silently delete the wrong managed-memory entries.
* test(core): assert thinkingConfig/maxAttempts/model forwarding in compression
The compression caller of runSideQuery sets thinkingConfig.includeThoughts=true
and maxAttempts=1. A future refactor that silently drops either would degrade
compression quality without test failure; this assertion locks the contract.
* fix(cli): route dynamic localization through side query
* refactor(core): remove unused memory governance review
|
||
BingqingLyu
|
28a3439307
|
feat(skills): Add codegraph skill for PR review risk analysis and conflict detection (#3910)
* feat(skills): add codegraph skill for PR review risk analysis and conflict detection Add .qwen/skills/codegraph/ with PR analysis, bug analysis, schema, patterns, and eval support. Enables per-PR risk scoring (blast radius, interface changes, test coverage), cross-PR conflict detection, and automated GitHub labeling via the codegraph-ai pip package. * chore: ignore .venv and .codegraph directories Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(skills): add HF mirror and ModelScope fallback for embedding model downloads Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(skills): address PR #3910 review feedback for codegraph skill - Rename skill from codegraph-qa to codegraph for consistency - Broaden description to cover index creation and inspection - Add HuggingFace/ModelScope model download tips - Document codegraph ingest command for adding git history - Fix stray '@' in stdout Tee pattern code snippet - Remove dead reference to PRReview.md - Add repo_dir parameter to CrossPRAnalyzer examples - Update CHANGES edge types (hunk/deleted/related/new) and resolve_pr_functions internals - Add Conflict Detection Dimensions table - Replace codegraph query with PRReview Python API examples - Remove OPTIONAL MATCH limitation from schema (now supported) - Update codegraph-ai link to PyPI - Add HF model download entry to Troubleshooting table --------- Co-authored-by: pomelo-nwu <czynwu@outlook.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
ChiGao
|
cadda23782
|
chore(deps): upgrade ink 6.2.3 → 7.0.2 + bump Node engine to 22 (#3860)
* chore(deps): upgrade ink 6.2.3 -> 7.0.2 + bump Node engine to 22
ink 7 requires Node >=22 and react-reconciler 0.33 with React >=19.2,
so this PR also bumps:
- Node engines (root + cli + core) 20 -> 22
- React/react-dom 19.1 -> 19.2.4 (pinned exact via overrides to keep
the transitive React graph deduped to a single instance)
- @types/node pinned to 20.19.1 via overrides to avoid an unrelated
Dirent NonSharedBuffer regression in sessionService tests
- @vitest/eslint-plugin pinned to 1.3.4 to avoid an unrelated lint
regression introduced by the 1.6.x rule additions
- react-devtools-core 4.28 -> 6.1 (ink 7 peerOptional requires >=6.1.2)
- ink hoisted to root devDeps so workspace-private peer-dep contention
doesn't push ink-link/spinner/gradient into nested workspace
installs (which would skip transitive resolution for terminal-link)
Workflow + image + installer alignment:
- .nvmrc 20 -> 22
- Dockerfile node:20-slim -> node:22-slim
- CI test matrix drops 20.x (keeps 22.x + 24.x)
- terminal-bench workflow Node 20 -> 22
- Linux/Windows install scripts upgrade their Node version targets
Documentation alignment:
- README.md badge + prerequisites
- AGENTS.md, CONTRIBUTING.md, docs/users/quickstart.md,
docs/users/configuration/settings.md, docs/developers/contributing.md,
docs/developers/sdk-typescript.md, docs/users/extension/extension-releasing.md,
packages/sdk-typescript/README.md, packages/zed-extension/README.md,
scripts/installation/INSTALLATION_GUIDE.md
Test gating:
- Two AuthDialog/AskUserQuestionDialog tests that drive <SelectInput>
through ink-testing-library now race ink 7's frame-throttled input
delivery and land on the wrong option. The maintainers had already
marked one of them unreliable (skip on Win32 + CI+Node20). Extend
that gate to cover all environments until upstream
ink-testing-library ships an ink-7-compatible release that flushes
input deterministically. The other test now uses it.skip with the
same comment. No business code changes.
Verified locally:
- npm run typecheck across all workspaces: clean
- npm run lint (root): clean
- npm run test --workspaces:
cli 312/312 files, 4918 passed, 9 skipped
core 266/266 files, 6836 passed, 3 skipped
webui 6/6, 201 passed
sdk 40/40, 283 passed, 1 skipped
- npm ls ink: single ink@7.0.2 instance across all peer deps
- single react@19.2.4 instance
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* chore: align Node 22 floor across all shipping artifacts
Reviewer (tanzhenxin) flagged five surfaces where the >=22 engine bump
leaked: SDK package metadata, web-templates engines, /doctor runtime
check, main bundler target, and SDK bundler target. Each was a separate
escape hatch letting Node 18/20 consumers install or run the artifact
on an unsupported runtime.
- packages/sdk-typescript/package.json: engines.node >=18.0.0 -> >=22.0.0
- packages/web-templates/package.json: engines.node >=20 -> >=22
- packages/cli/src/utils/doctorChecks.ts: MIN_NODE_MAJOR 20 -> 22
- esbuild.config.js: target node20 -> node22 (main CLI bundle)
- packages/sdk-typescript/scripts/build.js: target node18 -> node22 (esm + cjs)
- packages/cli/src/utils/doctorChecks.test.ts: rename test label to v22+
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* ci(e2e): bump E2E workflow Node matrix to 22.x
Reviewer (tanzhenxin) flagged that e2e.yml still pinned node-version
20.x while root engines is now >=22, so every E2E run on push would
either fail at npm ci with engine error or silently exercise the bundle
on a runtime that's no longer in ci.yml's test matrix.
The macOS job in the same workflow already reads .nvmrc (which is 22)
so this only updates the Linux matrix.
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(deps): drop root wrap-ansi override so ink 7 gets its declared dep
Reviewer (tanzhenxin) flagged that the root overrides.wrap-ansi: 9.0.2
predates this upgrade and forces every consumer (including ink) to v9,
while ink 7 declares wrap-ansi: ^10.0.0. The lockfile had no nested
install under node_modules/ink/, so ink 7 was running with a transitive
dep one major below its declared minimum.
Dropping the global override lets ink resolve its own wrap-ansi 10
nested install (now visible in the lockfile under
node_modules/ink/node_modules/wrap-ansi), while the cli package's own
direct `wrap-ansi: 9.0.2` dependency keeps the cli code path
(TableRenderer.tsx) on the version it has been tested against. The
nested cliui override is preserved for yargs which still needs v7.
Verified via `npm ls wrap-ansi`:
- ink@7.0.2 -> wrap-ansi@10.0.0 (newly nested)
- @qwen-code/qwen-code -> wrap-ansi@9.0.2 (unchanged)
- yargs/cliui -> wrap-ansi@7.0.0 (unchanged)
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* test(InputPrompt): un-skip placeholder ID reuse after deletion
Reviewer (tanzhenxin) flagged that the new it.skip on the
'should reuse placeholder ID after deletion' test was undisclosed in
the PR description and removed coverage of real product behavior
(freePlaceholderId / bracketed-paste backspace path) without a
TODO(#NNNN) link.
Their argument was sound: the skip rationale pointed at ink 7's input
throttle, but this same file just bumped the wait helper from 50ms to
150ms specifically to give ink 7 frame time. Re-running the test under
the bumped wait shows it passes reliably (5/5 runs in the full-file
context, 9/10 alone), so the skip was masking the throttle-flake that
the wait bump already addresses, not a real product bug.
Drop the it.skip and the now-stale comment so coverage of the
freePlaceholderId reuse logic is restored.
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* test(InputPrompt): bump first prompt-suggestion test wait to 350ms
The "accepts and submits the prompt suggestion on Enter when the buffer
is empty" test is the first in its describe block, so it pays the
renderer cold-start cost. On macOS-22.x CI runners that pushes the
Enter → onSubmit microtask past the default 150ms post-Enter wait. Match
the 350ms initial render wait used immediately above to absorb the cold
start.
* Revert "test(InputPrompt): bump first prompt-suggestion test wait to 350ms"
This reverts commit
|
||
pomelo
|
7e11428545
|
refactor(cli): remove legacy qwen auth CLI subcommand, redirect to /auth TUI dialog (#3959)
The `qwen auth` CLI subcommand (with subcommands like qwen-oauth, coding-plan, api-key, openrouter, status) has been superseded by the richer /auth TUI dialog introduced in the provider-first auth registry (#3864). Running `qwen auth` now prints a deprecation notice pointing users to the /auth TUI dialog (interactive), env vars (CI/headless), or /doctor (status check). Changes: - Replace auth.ts with a stub that prints a removal notice and exits - Delete handler.ts (734 lines), interactiveSelector.ts, and their tests (interactiveSelector.test.ts, openrouter.test.ts, status.test.ts) - Update /auth slash command to handle non-interactive/ACP modes gracefully - Enrich /doctor auth check with provider-aware diagnostics using findProviderByCredentials - Mark `auth` as a subcommand that handles its own exit in config.ts Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
Shaojin Wen
|
84ecb5b8a3
|
feat(cli): add --json-schema for structured output in headless mode (#3598)
* feat(cli): add --json-schema for structured output in headless mode
Registers a synthetic `structured_output` tool whose parameter schema IS the
user-supplied JSON Schema. In headless mode (`qwen -p`), the first successful
call terminates the session and exposes the validated payload via the result
message's `structured_result` field. Invalid schemas are rejected at CLI parse
time via a new strict Ajv compile helper so they can't silently no-op at
runtime.
* fix(cli): honour "first structured_output call ends session" + reject non-object root schemas
Two review fixes for the `--json-schema` feature:
1. `runNonInteractive` now breaks out of the tool-call loop as soon as the
first successful `structured_output` invocation is captured, rather than
continuing to execute any trailing tool calls the model emitted in the
same turn. This restores the documented single-shot contract and prevents
side-effecting tools from running after the final answer has already
been accepted.
2. `resolveJsonSchemaArg` rejects schemas whose root `type` is anything
other than "object" (or a type array including "object"). Function-
calling APIs require tool arguments to be JSON objects, so a schema
like `{"type": "array"}` would have registered an unusable synthetic
tool the model could never satisfy. Absent `type` and `type: "object"`
remain accepted.
Adds tests for both paths and updates the existing Ajv-compile test to
exercise that path without tripping the new root-type guard first.
* fix(cli): also reject root anyOf/oneOf schemas whose branches can't accept objects
Addresses a review follow-up: the previous root-object check only inspected
the top-level `type` keyword, so a schema like
`{"anyOf":[{"type":"array"},{"type":"string"}]}` slipped through even though
none of its branches can ever validate the object-shaped arguments that
function-calling APIs send.
Replace the single `type` check with `schemaRootAcceptsObject`, which
recursively walks root-level anyOf/oneOf branches and requires at least one
to accept objects. Absent `type`, `type: "object"`, `type: ["object", ...]`,
and mixed anyOf branches where one accepts object all still pass. `allOf`
is left to Ajv's runtime behaviour — guessing intent across contradictory
allOf branches at parse time is fragile.
* fix(cli): propagate exitCode from --json-schema failure path + tests
Address two PR-3598 review findings:
1. gemini.tsx unconditionally called process.exit(0) after
runNonInteractive/runNonInteractiveStreamJson, clobbering the
process.exitCode = 1 set by nonInteractiveCli.ts when the model
emits plain text instead of the structured_output tool. Switch
both call sites to process.exit(process.exitCode ?? 0) so CI can
detect the failure via the exit code.
2. nonInteractiveCli.test.ts: strengthen the structured-output
success path to assert registry.abortAll() is called and that the
stdout result envelope carries the JSON-stringified args under
`result` plus the raw object under `structured_result`. Add a
retry-path test that mocks executeToolCall to return an error on
the first structured_output call, then verifies sendMessageStream
is called a second time so the model can retry rather than the
session terminating early.
* fix(cli): suppress non-structured tool calls when structured_output is in the same turn
When --json-schema is active and the model emits a batch like
[write_file(...), structured_output(...)], the previous implementation
ran the leading side-effecting tool before accepting the structured
result, violating the "structured_output is the terminal contract"
guarantee. The trailing-only break also let an invalid first
structured_output fall through to subsequent tools before the retry
turn.
Pre-scan the batch: if a structured_output request is present, execute
ONLY the first one and skip everything else (leading and trailing).
This is consistent with the existing terminal-path semantics — the
suppressed tool_use blocks lack a matching tool_result, the same way
max-turns / cancellation leave the stream.
Adds a test covering the reverse-order [side_effect, structured_output]
case alongside the existing trailing-suppression and retry tests.
* fix(cli): tighten --json-schema root validation per review feedback
Three small holes flagged in the latest pass:
1. `schemaRootAcceptsObject` returned early when a root `type` keyword
was present, ignoring sibling `anyOf`/`oneOf`. JSON Schema applies
keywords at the same level conjunctively, so e.g.
`{type:"object", anyOf:[{type:"string"}]}` is unsatisfiable for any
value but used to pass. Now both `type` AND any sibling
`anyOf`/`oneOf` must independently admit object.
2. The FatalConfigError text said "Every branch of a root anyOf/oneOf
must be satisfiable by an object", but the actual logic only
requires *at least one* branch (and tests still accept
`anyOf:[object, string]`). Reworded to "at least one branch" so the
message matches the behaviour.
3. `compileStrict` used `typeof schema !== 'object'` to gate input,
which lets arrays through (`typeof [] === 'object'`). The contract
says "schema must be a JSON object", so add an `Array.isArray`
check so array input gets the intended error rather than a less
helpful Ajv compile message.
Tests cover the new rejection paths and the array case.
* fix(cli): handle root $ref and allOf in --json-schema accept-object check
`schemaRootAcceptsObject` previously only inspected `type`, `anyOf`,
and `oneOf` at the root, so a couple of unsatisfiable shapes still
slipped through:
1. `{"$ref":"#/$defs/Foo","$defs":{"Foo":{"type":"array"}}}` would be
accepted because we don't follow $refs, but registers a synthetic
tool whose params resolve to "array" — the model can never produce
a valid object. Now reject any root $ref unless the user adds a
sibling `type:"object"` as an explicit anchor.
2. `allOf` was deferred to Ajv runtime, but allOf is conjunctive at
the same level as `type` / `anyOf` / `oneOf`, so an entry like
`{"allOf":[{"type":"object"},{"type":"string"}]}` is unsatisfiable
for any value. Walk it like the others, requiring every branch to
admit object.
Tests cover the new $ref-rejected / $ref+anchor-accepted paths and the
allOf reject/accept paths.
* fix(cli): explicit exit code from runNonInteractive + pair suppressed tool calls
Three review threads on the structured-output flow:
1. The break that ends the for-loop on a successful structured_output
call sat *before* the responseParts.push and modelOverride capture.
SyntheticOutputTool currently returns neither, so it was safe today
— but anyone wiring extra signals into the synthetic tool later
would see them silently dropped. Move the break after both captures
so the contract is explicit, not implicit.
2. The failure path used to set process.exitCode = 1 and return void,
relying on global mutable state across an async boundary. Any
cleanup task between runNonInteractive and process.exit could
silently turn the structured-output failure into exit 0. Switch
runNonInteractive to Promise<number>, return 0 / 1 directly from
each function-level exit, and have gemini.tsx use the captured
return value.
3. The pre-scan from the prior commit suppresses sibling tool calls
when structured_output is in the same turn. On the retry path —
when structured_output fails validation — the next-turn payload
has tool_result for structured_output but no entry for the
suppressed siblings, leaving the prior assistant turn's tool_use
blocks unpaired. Anthropic and OpenAI both reject that batch
shape, so the retry would surface as an opaque provider error.
Synthesize a "skipped" functionResponse for every suppressed call
so every tool_use in the prior assistant message has a matching
tool_result.
Tests cover the new retry-pairing contract and update the existing
plain-text-failure test to assert on the return value rather than
process.exitCode.
* fix: address Copilot follow-up review on --json-schema scaffolding
Five small but real findings flagged on the latest pass:
1. core/src/index.ts re-exported `SyntheticOutputTool` via `export type`,
but it's a runtime class — that erased it from the emitted JS and
would break value imports. Split into a value `export { ... }` and a
`export type { StructuredOutputParams }`.
2. The structured-output success path returned without flushing
`localQueue` notifications or finalising one-shot monitors. If a
background task had already emitted `task_started`, exiting here
could drop its paired `task_notification` and leave SDK consumers
with unpaired lifecycle events. Mirror the regular terminal path's
`flushQueuedNotificationsToSdk` + `finalizeOneShotMonitors` calls
before `emitResult`.
3. `schemaRootAcceptsObject` ignored the `not` keyword, so
`{not:{type:"object"}}` (which forbids every object value) slipped
through. Add a best-effort `not` check that rejects when
`not.type` directly excludes object. Deeper negated patterns still
fall through to Ajv at runtime.
4. `compileStrict`'s JSDoc claimed it errored on "Ajv versions we can't
support", but the function doesn't actually check Ajv versions. Reword
to "malformed or uses unsupported draft/features for our Ajv
configuration" so the contract matches the implementation.
5. The pre-scan suppressed sibling tool calls but only synthesised
tool_result events for them on the retry path — the success path
left those tool_use blocks unpaired in the emitted JSONL/stream-json
event log. Move the synthesis after the for loop so it runs for both
the success break and the validation-failure fall-through; the event
log is now consistent regardless of which path the run takes.
Tests cover the new \`not\`-rejection paths, the success-path tool_result
synthesis, and the existing retry-pairing test still passes against the
restructured emit ordering.
* fix(cli): tighten --json-schema parse-time gate per Copilot review
Two more shapes that used to slip through:
1. `schemaRootAcceptsObject` defaulted to true when no narrowing
keyword was present, so root-value constraints like `{const: 1}`
or `{enum: [1, 2]}` registered an unsatisfiable structured_output
contract — the model could never produce a value matching the
tool's parameter schema, and the run would loop on validation
failures until max-turns. Reject `const` whose literal isn't an
object, and `enum` whose members include no object.
2. The yargs check rejected `--json-schema` with `-i` and with no
prompt, but not with `--input-format stream-json`. Stream-json
keeps the process open waiting for protocol messages, so
"terminate on the first valid structured_output call" silently
drops everything queued after that point. Refuse the combination
at parse time so the contradiction surfaces immediately.
Tests cover the new const/enum reject and accept paths.
* fix(cli): handle empty/boolean subschemas + allow stdin-only prompt
Three more shapes flagged on the latest review pass:
1. `schemaRootAcceptsObject` treated an empty root `anyOf`/`oneOf`
as "no constraint" (skipped when length === 0), but per JSON
Schema an empty union is unsatisfiable — no value can match a
member of the empty set. Reject those at parse time so users
get a clear parse error instead of an opaque runtime
never-validates loop.
2. JSON Schema (draft-06+) allows boolean subschemas anywhere a
schema is accepted: `true` matches every value, `false` matches
nothing. The `anyOf`/`oneOf`/`allOf` walks were rejecting
booleans via the typeof-object guard, which incorrectly
rejected `{anyOf:[true]}` and `{allOf:[true,{type:"object"}]}`
while letting `{anyOf:[false]}` slip through. Replace the
per-branch object guard with a `variantAcceptsObject` helper
that treats `true` as accepting and `false` as rejecting, then
recurses on object subschemas.
3. The yargs `.check` rejected `--json-schema` when no `-p` /
positional prompt was given, but the headless CLI also reads
the prompt from stdin (`cat prompt.txt | qwen --json-schema
'...'`) — a legit usage pattern that was being blocked. Drop
the parse-time no-prompt rejection; the existing runtime "No
input provided via stdin..." error in gemini.tsx still catches
genuinely empty input.
Tests cover the empty-union, all-`false`, mixed-boolean accept,
and `false`-in-allOf reject paths. Live-verified against the
bundled CLI: `echo "..." | qwen --json-schema '...'` now reaches
the model call, and the four schema edge cases all surface the
expected error text or proceed past parse time.
* docs(core): note SyntheticOutputTool as the value-export exception
The block comment above the lazy-load type re-exports said tool classes
"are now lazy-loaded and are not exported as values from the package
root", but `SyntheticOutputTool` was just promoted to a runtime export
in
|
||
|
Shaojin Wen
|
e2f7661ef1
|
feat(cli): Ctrl+B promote keybind (#3831 PR-3 of 3) (#3969)
* feat(cli): Ctrl+B promote keybind — wire UI to PR-2's promoteAbortController (#3831 PR-3 of 3) Final piece of the foreground → background promote feature. PR-1 (#3842) landed the `signal.reason` foundation; PR-2 (#3894) wired `shell.ts` to detect a `{ kind: 'background' }` abort, snapshot output, register a `BackgroundShellEntry`, and stash the promote `AbortController` on `TrackedExecutingToolCall`. This PR exposes the user-visible surface: pressing Ctrl+B during an in-flight foreground shell command transfers ownership to a background task the user can inspect via `/tasks` or stop via `task_stop`. ## Changes - `keyBindings.ts`: new `Command.PROMOTE_SHELL_TO_BACKGROUND` bound to `Ctrl+B`. JSDoc explains the no-shell-running no-op semantics. - `useReactToolScheduler.ts`: project `promoteAbortController` from the core's `ExecutingToolCall` through `TrackedExecutingToolCall` so the React layer (AppContainer keypress handler) can find it by callId without re-plumbing through the scheduler. - `AppContainer.tsx`: `handleGlobalKeypress` gains a `PROMOTE_SHELL_TO_BACKGROUND` branch that walks `pendingToolCallsRef.current` (the ref, not the destructured array — keeps the deps list stable so the handler isn't re-bound on every tool-call status update), finds the executing tool call with a defined `promoteAbortController`, calls `.abort({ kind: 'background' })`, and returns early. No-op when no foreground shell is executing — Ctrl+B then falls through to the input layer's existing cursor-left binding. - `keyboard-shortcuts.md`: documents Ctrl+B with explicit fall-through behavior so the conflict with the prompt-area cursor-left binding is intentional + understandable. ## Tests - `keyMatchers.test.ts` (+1): Ctrl+B positive / bare-b + meta+b + Ctrl+other negatives. - `AppContainer.test.tsx` (+2): - **Ctrl+B promotes** — pendingToolCalls includes an executing shell with a stubbed `AbortController` + spy; firing Ctrl+B asserts `abort({ kind: 'background' })` is called once. - **Ctrl+B no-op** — empty `pendingToolCalls` + Ctrl+B must NOT throw (pins the safety contract for the typing-mid-prompt case where the input layer's own Ctrl+B should still fire). - 37/37 keyMatchers + 58/58 AppContainer pass; tsc + ESLint clean. ## E2E (manual, PR description guidance) The unit / integration tests cover the keybind → abort wiring and the promote handler's downstream behavior (PR-2's tests). Real-PTY E2E is intentionally manual since headless test infrastructure doesn't drive a real shell child + Ctrl+B keystroke; documented in the PR description checklist. Closes the 3-PR sequence for #3831 (Phase D part b of #3634). * fix(cli): #3969 review wave — broadcast comment + debug log + redundancy 5 #3969 review threads addressed: - **AppContainer.tsx Ctrl+B handler**: documented the KeypressContext.broadcast caveat (after `return`, the same Ctrl+B is still dispatched to text-buffer cursor-left + DebugProfiler; visible cursor-left side effect is cosmetic) so future readers understand why the prompt cursor moves on a successful promote. Added `debugLogger.debug` calls on both branches (matched callId on success; streamingState + pendingToolCalls.length on no-op fall-through) so "Ctrl+B doesn't work" reports are debuggable. - **useReactToolScheduler.ts TrackedExecutingToolCall**: dropped the redundant `pid?` and `promoteAbortController?` declarations — both come through the `& ExecutingToolCall` intersection unchanged. Fixed the JSDoc that wrote `{ kind: 'background', shellId }`: callers don't generate `shellId` (it's optional on the abort-reason union and `handlePromotedForeground` produces it downstream). The corresponding executing branch in `toolCallsUpdateHandler` no longer projects pid / promoteAbortController explicitly — `...coreTc` already spreads them; the explicit-undefined clearing in the non-executing branch is also dropped (those fields aren't on coreTc when status !== 'executing', so `...coreTc` doesn't carry them). - **AppContainer.test.tsx**: replaced two `as unknown as Key` double-casts with direct `: Key` annotations on the literal — the object already conforms to the Key interface, double-cast was bypassing type safety needlessly. Tests: 37/37 keyMatchers + 58/58 AppContainer pass; tsc + ESLint clean. No behavior change beyond the new debug log lines. * fix(cli): #3969 wave — tool-name guard + non-shell test + defensive clear 3 #3969 review threads addressed; 1 deferred: - AppContainer.tsx: Ctrl+B `find()` predicate now also checks `tc.request.name === ToolNames.SHELL` before matching the executing tool call. Defense-in-depth — today only the shell tool wires `promoteAbortController`, but a future copy-paste / type confusion that adds the property to a non-shell tool would otherwise let Ctrl+B mistakenly fire `abort({kind:'background'})` on a tool whose service has no promote-handoff handler. - useReactToolScheduler.ts: re-added explicit `pid: undefined` and `promoteAbortController: undefined` to the non-executing return. Previously dropped on the assumption that `...coreTc` doesn't carry these fields when the status isn't `executing` — true today, but the explicit clearing is defense-in-depth against a future core change that adds either field to a non-executing status type (would surface as a stuck PID display or a Ctrl+B handler that matches a no-longer-executing tool call). - AppContainer.test.tsx: replaced the placeholder "no-op when no pending tool calls" framing on the empty-array case (it does exercise the `executing-status` predicate but NOT the tool-name guard) with TWO tests: 1. existing empty-array no-throw test (renamed for clarity) 2. NEW: executing non-shell tool with a hostile-shape `promoteAbortController` — asserts `abortSpy` is NOT called. This is the regression test for the new tool-name guard above. Tests: 61/61 AppContainer.test.tsx pass; tsc + ESLint clean. Deferred to follow-up (replied + tracked): - `debugLogger.debug` is file-only; success-path "agent unblocks + next message says 'promoted to bg_xxx'" is the user-visible signal. Adding a synthetic history item or stderr line for the gap between keypress and agent message conflicts with Ink rendering and is better as a focused UX PR. * test(cli): pin inheritance of pid + promoteAbortController via type assertions #3969 review: the earlier "redundant declaration" review removed the explicit `pid?: number` and `promoteAbortController?: AbortController` from `TrackedExecutingToolCall`, relying on the `& ExecutingToolCall` intersection to inherit them. Current review flags the type-safety regression: if core renames or removes either field, the React-side build won't catch it locally — Ctrl+B handler silently breaks at runtime. Compromise: keep the type minimal (no re-declaration noise the prior review flagged) but add compile-time `extends keyof ExecutingToolCall` assertions that fail loudly + locally if either field disappears. The assertions are evaluated at compile time and zero-cost at runtime; the dummy `const` pins them so they aren't dead code. 61/61 AppContainer tests pass; tsc clean. |
||
易良
|
cb7059f54d
|
feat(installer): add standalone archive installation (#3776)
* feat(installer): add standalone archive installation * fix(installer): harden standalone archive installs * fix(installer): address standalone review findings * chore(installer): clarify review followups * fix(installer): stabilize standalone script checks * chore(installer): remove internal planning docs * chore(installer): simplify standalone release review fixes * test(installer): add Windows batch install smoke * test(installer): fix Windows batch smoke quoting * test(installer): preserve Windows cmd quotes * fix(installer): use robust Windows checksum hashing * ci: narrow installer debug matrix * fix(installer): address standalone review hardening * fix(installer): avoid Windows validation parse errors * fix(installer): simplify Windows option validation * fix(installer): harden standalone review fixes |
||
|
易良
|
576bd8e0a7
|
ci: skip unnecessary release and SDK checks (#3984)
* ci: skip unnecessary release and SDK checks * ci: guard release skip classification for non-pr events * ci: harden release sync skip gate * ci: refine release sync skip fallback |
||
|
Shaojin Wen
|
b949ae070c
|
feat(tools): defer low-frequency built-in tools to reduce initial prompt size (#4022)
Some checks are pending
Qwen Code CI / Lint (push) Waiting to run
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* feat(tools): defer low-frequency built-in tools to reduce initial prompt size
Mark Monitor, SendMessage, Skill, TaskStop, TodoWrite, and WebFetch as
shouldDefer=true so their full schemas are excluded from the initial
function-declaration list. The model discovers them on demand via
ToolSearch, aligning with Claude Code's deferral strategy for
infrequently used tools.
* fix(tools): don't defer TodoWrite/Skill — they're required by the system prompt
TodoWrite is mandated as high-frequency by the core system prompt
("Use VERY frequently", "IMPORTANT: Always use the TODO_WRITE tool to
plan and track tasks throughout the conversation"). Deferring it forces
a ToolSearch round-trip before every required call, adding latency and
hurting prompt compliance.
SkillTool's full description carries the dynamically generated
<available_skills> listing and the BLOCKING invocation rules
("invoke the relevant Skill tool BEFORE generating any other response").
The deferred-tool summary truncates this, so the model loses both the
list of skills and the activation contract from the initial prompt.
Keep Monitor / SendMessage / TaskStop / WebFetch deferred — those are
genuinely infrequent and have no system-prompt requirement.
|
||
jinye
|
0a05ea8004
|
feat(telemetry): inject traceId/spanId into debug log files for OTel correlation (#3847) | ||
Gordon Lam
|
464e4cf343
|
feat(core): write runtime.json sidecar for active sessions (#3714)
Some checks are pending
Qwen Code CI / Lint (push) Waiting to run
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* feat(core): write runtime.json sidecar for active sessions Port kimi-cli PR #2082 part 1 to qwen-code. On interactive session start, atomically write a small JSON sidecar at <projectDir>/chats/<sessionId>.runtime.json recording the (pid, session_id, work_dir, hostname, started_at, qwen_version) tuple. External tools (terminal multiplexers, IDE integrations, status daemons) can map a running PID to its session id and work dir without parsing argv. Write is best-effort: a read-only filesystem must not block UI startup. OS process title (was #3713) and dynamic OSC tab title (kimi #2083) remain out of scope. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(core): refresh runtime.json on same-PID session swap Config.startNewSession() reassigns this.sessionId in the same process, which is reached by /clear, /reset, /new and /resume. Previously the old <oldId>.runtime.json was left behind, falsely claiming the still- live PID for a session no longer being served, and no new sidecar was written for the incoming session. Centralize the swap by clearing the old sidecar and writing a fresh one for the new session id from inside startNewSession itself, so all same-PID transitions are covered. The refresh runs as a fire-and- forget best-effort; failures must not block the session switch. Mirrors the post-merge Codex P1 fix on kimi-cli PR #2082 (the source of the runtime.json sidecar pattern this PR ports). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(core): only refresh runtime.json when this process owns it Mirrors kimi-cli PR #2082 commit e237951f (Codex P1 r3158754463): a short-lived non-interactive invocation (qwen --prompt, ACP, etc.) that runs `/clear` would otherwise call `Config.startNewSession()`, delete a concurrent shell's runtime.json sidecar (same outgoing session id), and never write a replacement — leaving the shell discoverable to nobody. Add a `runtimeStatusEnabled` flag on Config, flipped on by the interactive UI bootstrap immediately after the first successful sidecar write, and gate the swap-time refresh in `startNewSession()` on it. Non-interactive entry points never reach the bootstrap, so they won't touch sibling sidecars. Kimi later reverted the equivalent `write only from shell mode` guard (commit 7083975a) in favor of writing from every long-lived mode, but qwen's wire point is already interactive-only, so the narrower guard is the right shape here. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
Yan Shen
|
9bd5a0180b
|
feat(cli): core built-in i18n coverage (#3871)
* feat(i18n): expand built-in locale coverage * feat(cli): add dynamic slash command translation * test(cli): stabilize session picker assertions * fix(core): close jsonl readers before cleanup * fix: address i18n review regressions * fix(cli): address dynamic i18n review findings * fix(cli): address i18n review follow-ups * fix(cli): address i18n review feedback * test(cli): align i18n parity coverage with strict locales * fix(cli): address i18n review findings |
||
|
易良
|
04729d646c
|
test: stabilize main e2e flakes (#3992)
* test: stabilize main e2e flakes * test: stabilize macos e2e assertions |
||
qqqys
|
1777b20e93
|
perf(core): bound session-list metadata reads to head/tail 64KB; pool buffer; lazy message count (#3897)
Some checks are pending
Qwen Code CI / Lint (push) Waiting to run
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
SDK Python / SDK Python (3.10) (push) Waiting to run
SDK Python / SDK Python (3.11) (push) Waiting to run
SDK Python / SDK Python (3.12) (push) Waiting to run
* perf(core): drop full-file readline + pool tail buffer in listSessions
`listSessions` previously called `countSessionMessages` per file, which
streamed the entire JSONL through `readline` to count unique
user/assistant UUIDs. For a project with N sessions averaging M bytes
each, every /resume open paid O(N · M) wall time before showing the
picker — by far the dominant cost once a project accumulated many
multi-MB sessions.
This change:
- Drops the per-file count from listSessions / findSessionsByTitle.
`SessionListItem.messageCount` is now optional. Callers that need
a count call the new public `SessionService.countSessionMessages
(sessionId)` lazily — typically only when a SessionPreview panel is
about to display the badge.
- Pools a single 64KB tail-read buffer across the per-file metadata
reads in listSessions / findSessionsByTitle, mirroring the pattern
in claude-code's `enrichLogs`. The two helpers in
`sessionStorageUtils` (`readLastJsonStringFieldSync` and
`readLastJsonStringFieldsSync`) accept an optional caller-owned
scratch buffer; one-off callers (rename, single-session lookup) pass
nothing and keep the original alloc behaviour.
- Updates SessionPicker's row metadata to omit the "N messages"
segment when `messageCount` isn't available, keeping the visual
layout intact.
Tests:
- Pin that listSessions does not populate messageCount (regression
guard against silently re-introducing the per-file scan).
- Smoke test the buffer-pool plumbing — same caller-owned buffer
handed to two reads of different file sizes returns both correct
values without state bleed.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* perf(core): re-anchor custom_title; drop Phase-2 full-file scan
Tightens the title-write / title-read invariant so the picker's
metadata read is bounded to a fixed 2 × 64KB per file, regardless
of session length, with no fallback that scales with file size.
Writer (`ChatRecordingService`):
- New `bytesSinceTitleAnchor` counter and `TITLE_REANCHOR_BYTES`
threshold (32 KB, half of LITE_READ_BUF_SIZE).
- `appendRecord` now updates the counter and, once it crosses the
threshold while a title is set, re-appends a fresh `custom_title`
record to EOF (`reanchorTitle`). The recursive append routes
back through the same tracking path, which sees the title
record and resets the counter to zero.
- Sessions that never set a title pay zero overhead — the early
return in `updateTitleAnchorTracking` short-circuits.
Reader (`sessionStorageUtils`):
- `readLastJsonStringFieldSync` / `readLastJsonStringFieldsSync`
now read the file's last 64 KB, fall back to the first 64 KB
on miss, and return `undefined` if neither contains the field.
The previous Phase-2 streaming full-file scan (capped at 64 MB)
is removed entirely.
- The pooled scratch buffer (already optional) now backs both the
tail and head reads — only one allocation per `listSessions`
page even with the head fallback firing.
- `MAX_FULL_SCAN_BYTES` constant deleted (unused).
The two changes are coupled: the reader's tighter bound only works
because the writer guarantees the title stays in tail. A title
buried mid-file is now intentionally `undefined` (the picker
falls back to `firstPrompt` for display) rather than triggering a
full-file scan that would freeze the UI on long agent transcripts.
Tests:
- `chatRecordingService.customTitle.test.ts` — three scenarios
pinning the re-anchor invariant: threshold trigger, no-spurious
on no-title sessions, no-trigger on small write bursts.
- `sessionStorageUtils.test.ts` — replaces Phase-2 tests with
head-window fallback tests; pins the new "buried beyond both
windows returns undefined" contract; updates buffer-pool reuse
test to cover both tail and head reads with the same scratch.
E2E coverage (35/35 scenarios on a separate harness against real
fs): R1–R10 reader contract, M1–M4 multi-field, W1–W7 writer
re-anchor, L1–L6 listing latency + regression pins, S1 stress
(200 × 3 MB), C1 concurrency, I1–I5 writer/reader integration.
Measured 2.6× speedup on listSessions(50) over 50 × 4 MB
sessions vs the legacy `countSessionMessages` baseline; speedup
scales linearly with average file size.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* test(core): cover CR review gaps for session-list perf path
- countSessionMessages: valid id counts unique user/assistant uuids and
ignores other types/malformed lines; invalid id short-circuits without
filesystem access; ENOENT degrades to 0 instead of bubbling.
- readLastJsonStringFieldsSync: mirror the single-field variant's
scratch-buffer reuse test across a tail-hit then head-fallback to
catch any decode that ignores bytesRead.
- ChatRecordingService re-anchor: legacy resumed session (source
undefined) must omit titleSource on threshold-triggered re-anchor,
not silently reclassify as 'manual'.
Co-authored-by: Qwen-Coder <noreply@alibabacloud.com>
* fix(core): count title-anchor bytes as utf-8 + reset on reanchor failure
`String.length` undercounts the on-disk size of multi-byte payloads
(CJK, emoji are 1 UTF-16 unit but 3 UTF-8 bytes), so a session full
of CJK content could push >96KB of writes past the last anchor before
the 32KB threshold thinks it has — silently drifting the title past
the 64KB tail window the picker scans. Switch the byte counter to
`Buffer.byteLength(..., 'utf8')` for parity with `jsonl.writeLine`.
Also reset `bytesSinceTitleAnchor` to 0 in the reanchor catch:
without it, a failing reanchor pins the counter at the threshold
and turns a single transient I/O fault into a per-record retry
storm. One missed anchor is the right tradeoff — finalize() will
re-emit on the next lifecycle event.
Co-authored-by: Qwen-Coder <noreply@alibabacloud.com>
* fix(core): trim head-window to whole lines in lite metadata reads
The 64KB head-window fallback in `readLastJsonStringFieldSync` /
`readLastJsonStringFieldsSync` reads a fixed slice and hands it
straight to the extractor — its trailing bytes can fall mid-record.
A partial line whose `customTitle` value happens to close inside
the buffer but whose body extends past 64KB would otherwise win
the latest-match race and surface as the picker's title.
Drop everything past the last `\n` before extracting (only when the
buffer is shorter than the file — a small file is necessarily whole-
line). Honors the original Phase-2 contract that only complete lines
get a vote, without paying for the deleted full-file scan.
Co-authored-by: Qwen-Coder <noreply@alibabacloud.com>
* fix(core,cli): preview lazy count, project-scope countSessionMessages, pin perf contract
Address #3897 follow-up review findings:
- SessionPreview footer no longer drops the message count when listSessions
omits it. The prop is the override path; default falls back to a unique
user/assistant uuid count derived from the already-loaded conversation,
matching countSessionMessages semantics with zero extra disk I/O.
- countSessionMessages now scopes to the current project, mirroring the
first-record cwd check in deleteSession/renameSession/loadSession. A
valid sessionId from another project sharing the chats dir no longer
bypasses project boundaries on lazy count.
- New regression tests:
- SessionPicker row renders cleanly with messageCount === undefined
(no "messages"/"undefined"/dangling separator)
- findSessionsByTitle perf contract: matches have messageCount
undefined and fs.createReadStream is never called
- countSessionMessages: cross-project sessions return 0 without
streaming, empty file returns 0
- Update corruption-recovery tests to call private countSessionMessagesFromPath
(the streaming entry point) instead of the public sessionId-shaped API the
merge from main pointed them at.
Co-Authored-By: Qwen-Coder <noreply@qwen.com>
* docs(core): correct stale 'full-file fallback' comments to head-window
The session metadata reader uses two bounded 64KB windows (tail + head)
since the perf rework on this branch — never a full-file scan. Two
comments still described the prior behavior. Reported in MR review.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(cli): route ACP renameSession through live ChatRecordingService
ACP renameSession constructed a fresh SessionService and wrote the
custom_title record straight to disk. When the same session was live
in this process, ChatRecordingService.currentCustomTitle stayed at
the old value, so the next title re-anchor (every 32KB) or finalize()
re-emitted the stale title at EOF and silently reverted the rename.
Now we look up the live ChatRecordingService first and route through
recordCustomTitle, which keeps the in-memory cache and the on-disk
record in sync. The SessionService path remains for the non-live case
(e.g., another client renaming a backgrounded session). Reported in
MR review.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(core): harden session title re-anchor invariants
* docs(core): clarify bounded metadata race recovery cost
* test(core): cover bounded multi-field title reads
---------
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <noreply@alibabacloud.com>
Co-authored-by: Qwen-Coder <noreply@qwen.com>
Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com>
|
||
qwen-code-ci-bot
|
f261229ad5
|
chore(release): v0.15.10 [skip ci]
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> |
||
|
Shaojin Wen
|
ecc6828948
|
feat(tools): add ToolSearch for on-demand loading of deferred tool schemas (#3589)
* feat(tools): add ToolSearch for on-demand loading of deferred tool schemas Large MCP deployments push the function-declaration list past 15K tokens per request. This change lets tools opt out of the initial declaration list via `shouldDefer`, and adds a new `ToolSearch` tool the model calls to fetch schemas on demand — either by exact name (`select:Name1,Name2`) or keyword search with name/description/searchHint scoring. - `DeclarativeTool` gains `shouldDefer`, `alwaysLoad`, `searchHint` opts. - MCP tools default to `shouldDefer=true`; lsp, cron_*, ask_user_question, and exit_plan_mode are flagged too. - `ToolRegistry.getFunctionDeclarations()` filters deferred tools by default; `revealDeferredTool()` re-includes them after ToolSearch loads their schemas. - `getCoreSystemPrompt` appends a "Deferred Tools" list (names + first line of description) so the model knows what's reachable. - Subagent wildcard inheritance keeps including deferred tools so existing `tools: ['*']` configs still see MCP schemas. - Resume-session support: `startChat` scans history for prior calls to deferred tools and re-reveals them so the API doesn't reject follow-up calls. `resetChat` clears the revealed set for a clean slate. - Skipped when ToolSearch is filtered out by the permission manager. * feat(cli): add --json-schema for structured output in headless mode Registers a synthetic `structured_output` tool whose parameter schema IS the user-supplied JSON Schema. In headless mode (`qwen -p`), the first successful call terminates the session and exposes the validated payload via the result message's `structured_result` field. Invalid schemas are rejected at CLI parse time via a new strict Ajv compile helper so they can't silently no-op at runtime. * fix(tools): tighten ToolSearch schema + match invocation signature Resolves 2 #3589 review threads: - `max_results` schema: declared as unconstrained `number` but the implementation clamps to the integer range [1, 20]. Updated to `type: 'integer'` with `minimum: 1`, `maximum: HARD_MAX_RESULTS`, `default: DEFAULT_MAX_RESULTS` so the model gets accurate contract guidance and out-of-range values fail validation early instead of silently being clamped after a wasted call. - `execute()` signature now takes `_signal: AbortSignal` to match the base `ToolInvocation.execute` contract. The signal is unused today (the search is sync), but matching the shared signature avoids accidental divergence and makes future cancellation wiring trivial. Test: existing `enforces max_results cap` split into: - schema-rejection (`max_results: 100` → throws at build time) - clamp-on-in-range (`max_results: 20` capped on the candidate side) 21/21 tool-search.test.ts pass; tsc + ESLint clean. * fix(tools,cli): surface ToolSearch reveal failures + dedupe revealed tools Closes 3 #3589 review threads: - Critical: `setTools()` failure during reveal was silently swallowed via `debugLogger.warn` (off in production). Schemas appeared in `llmContent` so the model thought the tools were callable, but the chat's declaration list never updated — the next call surfaced as an "unknown tool" API error, leaving the session in an unrecoverable degraded state. Now returns a proper `ToolResult.error` with the concrete failure reason and instructions to retry; schemas are withheld from `llmContent` so the model doesn't act on a non-ready tool. - Critical: `collectCandidates` returned every deferred tool that matched `shouldDefer && !alwaysLoad` regardless of whether ToolSearch had already revealed it earlier in the session. Already-revealed tools are in the model's declaration list, so re-surfacing them in later keyword searches wasted tokens and risked the model retrying a load it already had. Filter now also skips tools where `registry.isDeferredToolRevealed(name) === true`. `select:<name>` mode is unaffected (the model may legitimately want to re-inspect the schema of a loaded tool). - Suggestion: `--json-schema` plain-text terminal path set `process.exitCode = 1` and emitted `isError: true` to the JSON adapter, but TEXT-mode users only saw a silent exit-code-1 with no visible context (`emitResult` is a no-op for the TEXT-mode error case). Echo the full `'Model produced plain text instead of calling the structured_output tool as required by --json-schema.'` line to stderr so headless runs are debuggable without scraping `--output-format json`. Tests: 2 new in `tool-search.test.ts`: - `keyword search excludes already-revealed deferred tools`: pins the dedupe behavior across two consecutive searches. - `returns an error result when setTools() throws`: pins that failures don't expose schemas as "ready" and the agent gets the underlying message in `error.message`. 23/23 tool-search.test.ts pass; tsc + ESLint clean. DEFERRED to follow-up PRs (replied on threads): - Critical: structured_output + side-effect-tool race in same turn — needs a pre-scan + synthesized "skipped" tool_results, design overlaps with #3598 PR-2's existing skippedOutput pattern. - Suggestion: `+` prefix parsing edge cases (C++, `+ slack`). - Suggestion: `instanceof DiscoveredMCPTool` hard couple — needs a type tag on AnyDeclarativeTool, broader API surface change. - Suggestion: SyntheticOutputTool registered in interactive mode. - Suggestion: resume scan O(history × parts) early-exit. - Suggestion: deferredToolsSection cap. * fix(cli): honor process.exitCode in headless main exit The two non-interactive exit paths in `main()` hardcoded `process.exit(0)` after `runNonInteractive` / `runNonInteractiveStreamJson` returned. This silently overwrote any `process.exitCode = 1` set inside the run — most visibly the `--json-schema` plain-text contract: the JSON adapter emits `isError: true` and stderr gets the explanation, but the shell saw exit code 0 and assumed success. Replace the hardcoded 0 with `process.exit(process.exitCode ?? 0)` on both paths so non-zero exits propagate. The success case is unchanged (exitCode is undefined → exits 0). * test(cli): add integration tests for --json-schema and ToolSearch Closes review-flagged coverage gaps for #3589: `json-schema.test.ts` (6 cases) covers the headless structured-output contract end-to-end: - structured_result emits when the model fills the schema (success path) - @path/to/schema.json file-load works - parse-time validation rejects invalid JSON, invalid JSON Schema, and missing files (no LLM, fast) - plain-text path: when structured_output is not callable (`--exclude-tools structured_output`), the run exits 1 with `is_error: true` and the contract error message — locks in the exit-code fix from the prior commit. `tool-search.test.ts` (3 cases) covers the deferred-tool flow: - select:<name> reveals a tool and the model can invoke it in the same turn (asserts call order so a missed reveal would surface as an unknown-tool API error instead of a silent pass) - keyword query (no select: prefix) hits the tool_search tool - feature-flag-off: with experimental.cron disabled, cron tools are never registered and never appear in tool calls LLM-dependent tests use the cron tools as a deterministic deferred target (gated by experimental.cron, no MCP server required). * fix(cli,core): tighten --json-schema validation Closes 3 #3589 review threads: - Schemas like `{"type":"string"}` and `{"type":"array"}` compiled fine (they're valid JSON Schemas in isolation), but the `--json-schema` value becomes the synthetic structured_output tool's parameter schema and tool-call arguments are object-shaped. Reject any non-undefined top-level type that is not "object" so the user sees the contract violation at parse time, not as an unrecoverable runtime mismatch. - `SchemaValidator.compileStrict` accepted arrays since `typeof [] === 'object'` — Ajv would later emit a confusing error. Add an explicit `Array.isArray` guard so the contract stated by the function name is honored at the boundary. - `compileStrict` shared the project-wide Ajv instances configured with `strictSchema: false` (intentionally lenient so MCP servers can ship custom keywords without breaking runtime validation). That leniency is wrong for the `--json-schema` surface — typos like `propertees` were silently ignored. Compile inside a dedicated `strict: true` Ajv so user-supplied schemas surface mistakes immediately. Tests: - jsonSchemaArg: rejects non-object top-level type ("string"/"array"). - schemaValidator.compileStrict: rejects arrays; flags unknown keywords (typos) under strict mode. * fix(tools): roll back ToolSearch reveals when setTools() throws Closes 1 #3589 review thread. `loadAndReturnSchemas` revealed each requested tool BEFORE calling `setTools()` because `getFunctionDeclarations()` filters by the revealedDeferred set — the reveal has to be in place when setTools() rebuilds the chat's declaration list. But if setTools() throws (e.g. chat not yet initialised), the registry was left holding orphaned reveals: the tool was marked "revealed" while the API never received its schema. Subsequent keyword searches would then exclude that tool from candidates (per `collectCandidates`'s isDeferredToolRevealed filter), making it unreachable until `/clear`. Track the names this call NEWLY revealed (skipping tools that were already revealed by an earlier ToolSearch in the same session) and unreveal them on setTools() failure. Added `unrevealDeferredTool` to the registry as the one-tool inverse of `revealDeferredTool`; `clearRevealedDeferredTools` is unchanged and still wipes the whole set on `/clear`. Test: extends the existing `setTools() throws` test to also assert that (a) the failed call's reveal is rolled back and (b) a tool revealed by an earlier call stays revealed (not whole-set wiped). * test(cli): unit-cover --json-schema runtime branches Closes one of the test-coverage gaps in #3589 reviews (gpt-5.5 review S8). Adds two deterministic L1 unit tests in nonInteractiveCli.test.ts that mock the LLM at sendMessageStream — no model API hit, no flake, ~10ms total. 1. structured_output success path: model fires the synthetic tool once, runtime sets structuredSubmission, aborts background tasks, and emitResult fires exactly once with `structuredResult` matching the model's args. No follow-up turn is issued (single-shot contract). 2. plain-text error path under --json-schema: model emits text only; runtime sets process.exitCode=1, writes the contract-violation line to stderr, and emits an isError result with the canonical "Model produced plain text" message. Both tests inject a stub adapter via runNonInteractive's `options.adapter` hook, so they assert against direct emitResult calls instead of parsing JSON stdout. process.exitCode is snapshot/restored to keep the test hermetic. The L2 integration tests in integration-tests/cli/json-schema.test.ts remain as smoke coverage against a real model. * fix(cli,core): support type-union arrays in --json-schema Resolves 2 regressions introduced by the previous schema-hardening commit ( |
||
|
Shaojin Wen
|
7ba6281b74
|
fix(core): unify Edit/WriteFile prior-read with Claude Code; close #3964 + #3945 (#4002)
* fix(core): decouple cacheable flag from truncation in FileReadCache PR #3774 introduced prior-read enforcement that consults `lastReadCacheable` to discriminate text from binary / image / PDF / notebook payloads. ReadFileToolInvocation derived `cacheable` as `string && originalLineCount && !isTruncated`, conflating two unrelated concerns: "is the content text" and "did we see all the bytes". A partial read (offset/limit) or a truncated full read of a regular `.kt` / `.cpp` / `.py` source file therefore set `cacheable: false`, and priorReadEnforcement.ts mistook that for a non-text payload and rejected the next Edit with the misleading "binary / image / audio / video / PDF / notebook payload" error. PR #3932 split prior-read enforcement so Edit accepts partial reads (`lastReadWasFull`-relaxed for Edit, kept for WriteFile), but the `lastReadCacheable` conflation persisted, so partial / truncated text reads still hit the binary-payload rejection on Edit. Issue #3964 is the resulting field reports: .kt / .cpp / .py / .ts files on both Linux and Windows misclassified as binary across 0.15.7-0.15.9. Decouple the two concerns: - `cacheable` is now purely about content type. A partial or truncated text read records `cacheable: true` because the bytes the model saw were text. - Truncation gating moves to `full`. A request-level full read (no offset/limit/pages) only counts as full at the cache level when the produced content was not truncated; otherwise the model only saw the head of the file. The fast-path `file_unchanged` placeholder still requires both `lastReadWasFull && lastReadCacheable`, so its semantics are unchanged — a truncated full read now fails the AND on the moved flag instead of the original. WriteFile's `requireFullRead` still rejects partial or truncated text reads; it now reports the accurate "partial read" error instead of the wrong "binary payload" message. Also fixes issue #3945 (edit tool unusable for large files) as a side effect: the truncated-full case there hit the same misclassified path before the rejection wording could even surface the truncation question. Tests: 2 regression tests added in read-file.test.ts (partial .kt read and truncated full .cpp read both record `lastReadCacheable: true`). Existing 7386 / 7391 (5 skipped) core tests pass; tsc --noEmit clean. Issue #3964 also reports a separate scenario on Windows encrypted/DRM-protected file systems where .cpp source files are misclassified by `isBinaryFile`'s 4KB content sampling. That path is content-detection-side, not cache-side, and is left to a follow-up (extension- or mime-based override of the content sample for known text types). * fix(core): trust extension/mime over isBinaryFile sampling for known text Issue #3964's first report (Frank-Shaw-FS) describes `.cpp` / `.c` / `.h` source files on a Windows encrypted / DRM-protected file system being misclassified as binary. The OS surfaces encrypted bytes to `fs.open()` random-access reads, so `isBinaryFile`'s 4 KB sample sees nulls / non-printable characters and concludes binary — even though the higher-level `readFile` returns the decrypted text and the extension declares the file as text. Layer-2 fix on top of the cache-side decoupling: change `detectFileType` to trust the registry / curated extension list *before* running the content sample, so a known text extension is not subject to false positives from raw-bytes sampling. - Trust mime types declared as text: `text/*`, `application/*` text-likes (`application/javascript`, `application/json`, `application/toml`, ...), and any mime ending in `+xml` / `+json`. - Trust a curated set of source-code / config / markup extensions whose `mime/lite` registry coverage is patchy (`.py`, `.kt`, `.go`, `.rb`, `.swift`, `.scala`, `.rs`, `.proto`, `.graphql`, `.toml`, `.hcl`, `.tf`, ...). The list is restricted to extensions we have observed to be misclassified by `isBinaryFile` in the field; obscure extensions still go through the content sampler. Order in `detectFileType`: 1. Hardcoded `.ts` / `.svg` / `.ipynb` 2. Mime check (image / audio / video / pdf / declared-text) 3. `BINARY_EXTENSIONS` pre-empt (so `.png` with text-looking content stays binary) 4. Curated text extension override (for mime-less source code) 5. `isBinaryFile` content sampler (final fallback for unrecognised extensions) 6. Default text Tests: 5 new cases in `fileUtils.test.ts` and 1 end-to-end in `read-file.test.ts` covering: text mime override on binary-looking content, application/* text mimes, `+xml` / `+json` suffix match, curated extension override on `.py` / `.kt` / `.go` / `.rb` / `.swift`, and the `BINARY_EXTENSIONS` pre-empt still winning over the new override (a `.png` whose first bytes happen to be ASCII text stays binary). Full core suite passes (7392 / 7397, 5 pre- existing skips); `tsc --noEmit` clean. Together with the earlier commit, this PR closes both arms of issue #3964: the cache-side `cacheable` conflation that affected partial / truncated reads, and the content-detection-side false positive on encrypted file systems. * fix(core): tighten detectFileType after self-review on #4002 Three follow-ups flagged by `/review` on #4002: 1. `KNOWN_TEXT_APPLICATION_MIMES` had 10 dead entries — names like `application/x-sh`, `application/x-perl`, `application/x-yaml`, `application/x-tex`, `application/x-sql`, `application/graphql` are real mimes seen in HTTP `Content-Type` contexts but are not in `mime/lite`'s registry, so `mime.getType()` never returns them and the entries are unreachable. Strip the set to the 6 values the registry actually emits (`javascript`, `ecmascript`, `node`, `json`, `xml`, `toml`); the shells / tex / sql / graphql extensions reach the text fallback through `KNOWN_TEXT_EXTENSIONS` instead. Add a scope rule in the docstring so future additions stay aligned with what mime/lite actually emits. 2. The early-return at the top of `detectFileType` listed `.ts / .mts / .cts / .tsx` in its comment but the array only contained `.ts / .mts / .cts`. `.tsx` was reaching the text verdict via `KNOWN_TEXT_EXTENSIONS`, which works today but would break if a future `mime/lite` update mapped `.tsx` to `video/mp2t` (mirroring `.ts`): the `startsWith('video/')` guard would fire before the text fallback. Move `.tsx` up to the early-return array so the comment matches the code and the defence is consistent across the TypeScript family. Drop the duplicate listing in `KNOWN_TEXT_EXTENSIONS`. 3. `isTextMime()` short-circuits `isBinaryFile` for any `text/*` mime, which is the necessary tradeoff for the encrypted-FS fix but removes the safety net for *corrupted* text files (a binary blob saved with a `.txt` / `.md` extension via redirection). Document the tradeoff explicitly with a concrete counter-example and call out that Edit's `0 occurrences` failure mode is the fallback for the corrupted-text population. Tests: 261 / 262 (1 skipped, pre-existing) on `fileUtils.test.ts` + `read-file.test.ts` + `edit.test.ts` + `write-file.test.ts`. `tsc --noEmit` clean. * fix(core): drop full-read requirement on WriteFile, align with Claude Code PR #3932 deliberately diverged from Claude Code's `readFileState` by keeping `requireFullRead: true` on WriteFile's prior-read enforcement, citing issue #2499 (LLM hallucinates content of an unread file and clobbers user changes) as evidence that the asymmetric stance was justified. In practice that stance leaves a hard deadlock: when a file is larger than `truncate-tool-output- lines`, `read_file` without offset/limit still records `lastReadWasFull: false` (the model only saw the head), and the "only been partially read … re-read without offset / limit / pages" rejection sends the model back to the same truncated read with no escape — the exact deadlock issue #3945 reported. Drop the `requireFullRead` option from `checkPriorRead` and remove all 5 `requireFullRead: true` call sites in WriteFileTool. After this change the contract is identical to Claude Code's: any prior read of an existing file clears enforcement; the mtime/size drift check is the only gate that distinguishes "the model has seen current bytes" from "the model has seen older bytes", and it fires identically for Edit and WriteFile. The residual #2499 risk is acknowledged in the docstring: a model that reads only a slice and then overwrites would necessarily hallucinate the rest of the bytes. Mitigations: - `fileReadCacheDisabled: true` for users who want stricter behaviour (existing escape hatch, unchanged). - The mtime/size drift check still rejects Writes against bytes the model saw at fingerprint X if disk has moved to Y. Cleanup: drops the dedicated "fresh + cacheable + partial + requireFullRead" rejection branch and the `requireFullRead`-aware wording variant in the `unknown` branch — both unreachable now. Tests: - `write-file.test.ts:932` inverted from "rejects a write when the previous read was ranged" to "allows a write after a ranged read", matching the equivalent `edit.test.ts:1077`. - New `write-file.test.ts:961` regression for the issue #3945 deadlock: a `recordRead({ full: false, cacheable: true })` entry (what a truncated full read produces) clears WriteFile enforcement. - 7393 / 7398 (5 skipped, all pre-existing) on the full core suite. `tsc --noEmit` clean. * docs(core): add anti-regression notes locking in the WriteFile relax Three sites a future contributor might naturally try to "tighten up" back into the deadlock-prone shape, now carrying explicit guard comments that name the prior PR (#3932), the issue it broke (#3945), and the residual risk this stance accepts (#2499): - `priorReadEnforcement.ts:CheckPriorReadOptions` — interface-level note: do not re-introduce `requireFullRead` (or any "stricter for WriteFile than Edit") option here. References the function docstring for the full rationale. - `fileReadCache.ts:lastReadWasFull` — field-level note: sole consumer is the Read fast-path; `priorReadEnforcement` does not consult this and must not start. - `write-file.ts` first checkPriorRead call site — anchor comment that explains why no extra option is passed and applies to all 5 call sites in the file. No code changes; test suite unchanged at 7393 / 7398 (5 pre-existing skips); `tsc --noEmit` clean. * fix(core): #4002 review wave — basename allowlist + correct stale comments 3 #4002 review threads addressed: - fileUtils.ts: added KNOWN_TEXT_BASENAMES allowlist for extensionless build / config / lockfiles (Dockerfile, Containerfile, Makefile, GNUmakefile, Jenkinsfile, Vagrantfile, Rakefile, Gemfile, Procfile, BUILD, WORKSPACE, CMakeLists.txt, go.mod, go.sum, go.work, Cargo.lock, Pipfile, Pipfile.lock, poetry.lock, package-lock.json, yarn.lock, pnpm-lock.yaml, requirements.txt, .gitignore, .gitattributes, .dockerignore, .npmignore, .editorconfig, .env, .bashrc, .zshrc, .profile, LICENSE, COPYING, AUTHORS, CHANGELOG, README, NOTICE). `path.extname('Dockerfile')` returns `''`, so the KNOWN_TEXT_EXTENSIONS check above misses these — an encrypted-volume read whose 4 KB sample looks binary would misclassify them as binary. Regression test pinned with fake-encrypted bytes for Dockerfile / Makefile / Jenkinsfile / go.mod / package-lock.json / .gitignore / LICENSE. - priorReadEnforcement.ts: rewrote two misleading comments that pointed users to `fileReadCacheDisabled: true` for "stricter behaviour". That setting actually DISABLES enforcement entirely (skips checkPriorRead). Updated to make the opt-out semantics explicit and clarify that there is no built-in stricter mode — users who want stricter built-in enforcement than the residual #2499 risk accepts have no flag here today and should file a feature request. - read-file.ts: updated the `lastReadWasFull` comment to reflect that PR #4002 removed WriteFile's `requireFullRead`. The flag now gates ONLY the `file_unchanged` fast-path; the stale "and WriteFile's full-read requirement" wording would have confused future readers into thinking WriteFile still consults `lastReadWasFull`. Tests: 89/89 fileUtils.test.ts pass; tsc + ESLint clean. * fix(core): split priorReadEnforcement guidance — partial OK for edit, full for overwrite #4002 review: shared "never read" error said `(a partial read with offset / limit is fine — you only need to have seen the bytes you intend to edit/overwrite)` for BOTH Edit and WriteFile. For Edit this is correct — the model only needs to have seen the `old_string`-bearing bytes; the rest passes through untouched. For WriteFile this is misleading: overwriting replaces EVERY byte, so a partial read leaves any unseen bytes as collateral damage. The mtime/size drift check still catches the worst-case #2499 hallucinated-bytes risk, but recommending a partial read in the WriteFile guidance would actively encourage the footgun. Fix: branch the partial-read guidance on `verb`. Edit keeps the current "partial OK" text. WriteFile gets `(read the full file — overwriting replaces every byte, so any unseen bytes would be discarded)`. 120/120 edit + write-file tests pass; tsc + ESLint clean. * docs(core): finish #4002 review wave — drop two stale "fileReadCacheDisabled is escape hatch" mentions |
||
Edenman
|
6556adcdba
|
feat: add /diff command and git diff statistics utility (#3491)
Some checks are pending
Qwen Code CI / Lint (push) Waiting to run
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* feat(core): add git diff statistics utility
Port numstat + unified-diff parsing into `packages/core/src/utils/gitDiff.ts`
to surface structured working-tree change summaries (files changed, lines
added/removed, per-file hunks) against HEAD. Caps mirror issue #2997:
50 files, 1MB per file, 400 lines per file, with a 500-file short-circuit
via `git diff --shortstat` to avoid expensive work on massive diffs.
- `fetchGitDiff(cwd)` returns stats + per-file summaries (tracked + untracked).
- `fetchGitDiffHunks(cwd)` returns structured hunks on demand.
- `resolveGitDir(cwd)` follows `.git` file indirection so linked worktrees
and submodules report the correct gitdir.
- Transient-state short-circuit covers merge, cherry-pick, revert, and both
`rebase-merge` / `rebase-apply` layouts.
- `core.quotepath=false` is forced so non-ASCII filenames stay as UTF-8.
Refs #2997
* feat(cli): add /diff slash command
Surface the `fetchGitDiff` utility through an interactive `/diff` command.
Prints a header (`N files changed, +A / -R`) followed by per-file rows with
padded add/remove counts. Untracked files are marked `?`, binary files are
marked `~`. When the change set exceeds the per-file cap, a trailing
`…and N more` note tells the user how many entries are hidden.
Returns a `MessageActionReturn` so it renders the same way in interactive
and non-interactive modes.
* fix(cli): harden /diff command after adversarial audit
- Wrap `fetchGitDiff` in try/catch so permission errors on `.git` surface
as a friendly error message instead of crashing the action.
- Declare `supportedModes: ['interactive', 'non_interactive', 'acp']` so
the command is reachable outside the interactive Ink UI — the default
for `commandType: 'local'` is interactive-only.
- Align `?` (untracked) and `~` (binary) markers with the `+X -Y` stat
column via a padded prefix, so filenames line up regardless of row kind.
- Drop the "…and N more" hint when no rows are shown (shortstat fast-path
with >500 files) — the count alone is sufficient and "showing first 0"
is noise.
- Switch header to full-phrase i18n templates (separate singular/plural
variants) instead of word-by-word `t()` calls that don't survive
non-English locales.
- Extend tests to 12 scenarios: empty cwd, fetch rejection, singular
"file" form, mixed untracked/binary/tracked alignment, 4-digit padding,
shortstat fast-path, and supportedModes declaration. Mocks carry a
`satisfies GitDiffResult` annotation so shape drift in core breaks the
test at compile time.
* fix(cli): clean up /diff feature review issues
- Remove invalid `commandType` field from diffCommand (SlashCommand has
no such property; caused a TS build failure).
- Drop duplicate `NumstatResult` interface in gitDiff.ts — it is
structurally identical to `GitDiffResult`.
- Register the 9 missing `/diff` i18n strings in en.js / zh.js so the
command is translatable (previously only `Configuration not available.`
had entries).
* fix(core): harden git diff stats after multi-round review
- fetchUntrackedPaths now uses `ls-files -z` so filenames containing
newlines, tabs, or non-ASCII bytes round-trip cleanly instead of
being C-style quoted and split into phantom entries.
- fetchGitDiff runs the `--shortstat` probe and the untracked-paths
lookup in parallel, since both are needed regardless of which path
the function takes.
- parseGitDiff measures per-file diff size via Buffer.byteLength so
MAX_DIFF_SIZE_BYTES matches its documented meaning on non-ASCII diffs.
- Adds a regression test for an untracked file whose name contains a
literal newline.
* fix(core): address /diff PR review comments
Addresses the five open review threads on #3491:
- parseShortstat: anchored and bounded the regex (`^...$` with `\d{1,10}`)
so adversarial inputs can no longer drive polynomial backtracking. Closes
CodeQL alert #137.
- fetchGitDiff: only parse the untracked-path list when we actually need
it; the fast path now counts NUL bytes in the raw `ls-files -z` stdout
(wenshao P1).
- fetchGitDiff: base the `MAX_FILES_FOR_DETAILS` short-circuit on
`tracked + untracked`, so repos with few edits but many untracked files
still take the summary-only path (wenshao P2).
- fetchGitDiff: count newlines in each untracked text file (binary sniff +
1 MB read cap) and fold that into both the header `+N` and the per-file
row, so a brand-new file no longer renders as `+0 / -0` (BZ-D P2).
- parseGitNumstat: switch to `git diff --numstat -z`. The parser now uses
index-based slicing and a rename-pair state machine, so tracked
filenames containing tabs/newlines/non-ASCII keep their real bytes
(BZ-D P3). Renames collapse into a single `old => new` entry.
UI: untracked rows render as `+N filename (new)` (or
`~ filename (binary, new)`) instead of the placeholder `?` marker;
`/diff` now shows real additions for fresh files.
* fix(core): surface truncated untracked counts and decouple totals from display
Two issues surfaced during a directionless multi-round audit of the /diff
feature:
1. `countUntrackedLines` reads at most `UNTRACKED_READ_CAP_BYTES` (1 MB)
per file, so a 10 MB new log was silently reported as `+~20k` when the
real count is ~10×. The helper now `fstat`s the file and returns a
`truncated: true` flag when size exceeds the read window; `/diff`
surfaces it as `(new, partial)` so the `+N` isn't read as exact.
2. Line-count aggregation was coupled to the per-file display cap: when
tracked changes filled the `MAX_FILES` slot, untracked line counts
beyond the remaining slots were dropped from `stats.linesAdded`
entirely (header under-reported additions). Decoupled: we now read up
to `MAX_FILES` untracked files for their line counts regardless of
display slots, and only restrict the visible rows to `remainingSlots`.
Added regression tests for both: a 1.5 MB new file asserts `truncated:
true` and a lower-bound line count, and a `MAX_FILES`-saturated tracked
set + 5 untracked files asserts that untracked additions still appear in
the header totals even though none of them get displayed.
* fix(core): parse filenames from +++/--- lines to handle paths with ' b/'
`diff --git a/X b/Y` is ambiguous when X contains ` b/` — a file literally
named `a b/c.txt` produces `diff --git a/a b/c.txt b/a b/c.txt` with no
escape or quoting, and the previous regex `^a\/(.+?) b\/(.+)$` keyed the
hunks under the wrong path. Consumers of the exported `fetchGitDiffHunks`
API would then fail to correlate hunks with stats or editor paths.
Introduces `extractFilePath(lines)` which walks the block for the
unambiguous markers (`rename to` / `copy to` / `+++ b/<path>` with a
`/dev/null` fallback to `--- a/<path>`) and strips the trailing TAB git
appends to paths containing whitespace. Adds unit tests for the
`a b/c.txt`, rename, delete, and new-file cases plus an end-to-end test
that creates a real `a b/c.txt` file and asserts `fetchGitDiffHunks`
keys the hunks correctly.
Addresses wenshao review comment #3136657141 on #3491.
* feat(cli): colorize /diff output via a themed Ink component
The /diff stats used to come back as a plain-text MessageActionReturn.
Pipes and ACP still get that, but in interactive terminals we now dispatch
a structured history item so the numbers can carry theme colors.
- packages/cli/src/ui/types.ts — new DiffRenderRow / DiffRenderModel /
HistoryItemDiffStats, MessageType.DIFF_STATS.
- packages/cli/src/ui/components/messages/DiffStatsDisplay.tsx — renders
+N in theme.status.success (green), -M in theme.status.error (red), and
the (new) / (binary) / (new, partial) markers in theme.text.secondary
(dim). Column alignment matches the plain-text fallback.
- packages/cli/src/ui/components/HistoryItemDisplay.tsx — routes the new
item type.
- packages/cli/src/ui/commands/diffCommand.ts — builds a DiffRenderModel
once and fans out: interactive calls context.ui.addItem; other modes
fall through to renderDiffModelText() for the plain-text path. Error
and "clean tree" branches keep the existing info/error
MessageActionReturn in every mode.
- Tests: existing diffCommand suite moved to an explicit non_interactive
context (it was asserting text content); new interactive suite covers
addItem dispatch and model shape; DiffStatsDisplay component tests
cover the four row variants and the "…and N more" note.
* refactor(cli): factor /diff column widths into a shared helper
Audit of the colorize commit found one real DRY hazard: DiffStatsDisplay
and renderDiffModelText each independently re-derived addWidth /
remWidth / statColumnWidth from the same row list. If anyone later
changed one formula, the interactive Ink output and the non-interactive
plain text would silently fall out of column alignment.
Extract the computation into computeDiffColumnWidths() exported from
diffCommand.ts; both renderers now call it. Adds a focused unit test of
the contract (empty rows, widest non-binary row wins, binary rows are
ignored, untracked text rows count). Drop a redundant
`Omit<HistoryItemDiffStats, 'id'>` annotation since the type already has
no id field.
* fix(core): pin /diff git ops to repo root and lstat untracked entries
Two Critical findings on PR #3491:
1. (line 63) When /diff is invoked from a subdirectory of the worktree,
`git diff` emits repo-root-relative paths but `git ls-files --others`
is scoped to cwd and emits cwd-relative paths. Result: mixed path
bases in `perFileStats` and silent omission of untracked files in
sibling directories. Resolve `findGitRoot(cwd)` once and run every
git invocation (and `path.join(...)` for line counting) from there,
so all keys are repo-root-relative and the listing is repo-wide.
2. (line 455) `countUntrackedLines` opened every untracked path with
`open(absPath, 'r')`. Git's `ls-files --others` can list FIFOs
(whose `open()` blocks indefinitely waiting on a writer) and
symlinks (which `open()` dereferences, potentially reading outside
the worktree). Add an `lstat` gate: only regular files are counted;
symlinks and other special files render as binary `~` rows.
Two new integration tests cover both regressions: one creates a
sibling untracked file at the repo root and invokes fetchGitDiff from
a subdir asserting all three changes (root + sub) come back keyed by
repo-root-relative paths; the other creates a symlink pointing at
content outside the worktree and asserts it lands as a binary row
with no contribution to linesAdded.
* chore: revert stray .npmrc/README.md test edits swept into
|
||
|
ChiGao
|
f4d0ad6b7f
|
fix(core): throttle shell tool live text updates (#3902)
* fix(core): throttle shell tool live text updates
The previous lastUpdateTime = Date.now() at function entry meant the
first 'data' chunk's check (Date.now() - lastUpdateTime > INTERVAL)
was always false on first invocation, but shouldUpdate=true was set
unconditionally — so every text chunk forced a React render.
Initialize lastUpdateTime to NEGATIVE_INFINITY so the first chunk
always emits, then throttle subsequent text chunks to OUTPUT_UPDATE_INTERVAL_MS.
ANSI (Array<>) chunks are already throttled and deduped by
ShellExecutionService and continue to update at full rate.
Final ToolResult still carries the complete output after command
completion — only the live preview is throttled.
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(core): add trailing-edge flush to shell throttle
wenshao CHANGES_REQUESTED (2026-05-07T22:42): the leading-edge-only throttle
left the last suppressed plain-text chunk unshown if the command went quiet
within the 1s window (e.g. a status line printed once and then no more output).
Fix: when a plain-text chunk is suppressed, schedule a setTimeout for the
remaining window duration that calls the existing doUpdate() helper. The timer
is cancelled if a subsequent leading-edge update arrives first (preventing a
redundant render), or when the command settles via await resultPromise.
The do-update logic is extracted into a local doUpdate() helper to avoid
duplicating the string/ANSI branching between the immediate path and the timer.
Test changes:
- Updated existing throttle test to reflect new 3-call sequence: leading-edge
('line 1'), trailing flush ('line 2'), leading-edge ('line 3').
- Added 'trailing flush' test: leading update fires, next chunk suppressed,
time advances → trailing flush emits the last suppressed chunk.
- Added 'ANSI passthrough' test: two back-to-back ANSI chunks both trigger
updateOutput immediately (ANSI branch bypasses throttle, regression guard).
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(test): correct AnsiToken shape in shell ANSI throttle test
Use fg/bg string fields and remove non-existent properties
(strikethrough, hidden, blink, foreground, background) so the
object literals satisfy the AnsiToken interface and tsc passes.
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(core): harden shell throttle timer lifecycle + add coverage
Centralizes trailing-flush timer cancellation in `doUpdate()` so every
emit path (leading-edge text, ANSI passthrough, binary_detected,
binary_progress) supersedes a pending timer instead of leaving a stale
one that could double-fire. Adds an abort listener so user-cancel /
timeout cancels the timer before the result settles, and wraps both
`ShellExecutionService.execute()` and `await resultPromise` in
try/finally so a thrown PTY import or rejected result still tears down
the timer + listener.
Adds five regression tests covering the lifecycle invariants flagged
in review:
- 3+ rapid suppressed chunks coalesce into one trailing flush
- command settling cancels a pending trailing-flush timer
- leading-edge update path produces no duplicate trailing flush
- abort signal cancels a pending trailing-flush timer
- execute() rejection cleans up listeners (no late updateOutput)
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>
|
||
|
jinye
|
4e91dbaff0
|
fix(core): harden reactive compression follow-ups (#3985)
Some checks are pending
Qwen Code CI / Lint (push) Waiting to run
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
jinye
|
e11fd3f479
|
fix(core): suppress otel diagnostics in UI (#3986)
Route OpenTelemetry SDK diagnostics through the debug logger instead of console output so exporter warnings and errors do not surface in user-facing UI. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
ChiGao
|
4bab7a1ad6
|
fix(cli): replace clearTerminal with targeted repaint on resize (#3967)
Some checks are pending
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Waiting to run
Qwen Code CI / Lint (push) Waiting to run
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
Add repaintStaticViewport() that uses cursorTo(0,0)+eraseDown instead of the full clearTerminal (ESC[2J ESC[3J ESC[H]) sequence. Hook it to terminal width changes via a new useEffect that guards against no-op repaints when width is unchanged. clearTerminal remains in use for explicit refreshStatic() calls (model switches, /clear, etc.) where a full history remount is intended. previousTerminalWidthRef ensures the resize effect only fires on actual column changes, not height-only resizes. Closes the full-screen flash that occurs on every terminal width change. Generated with AI Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
Dragon
|
8255027426
|
feat(vscode): add message edit/rewind and message metadata UI (#3762)
* feat(vscode): add message edit/rewind and message metadata UI - Add rewindSession extension method to ACP agent for session rewind - Add rewindToTurn method in Session to truncate conversation history - Handle conversationRewound event in webview to reset messages, tool calls, plans, and UI state - Add editMessage flow in VSCode companion: user edit → rewind request → truncated state - New MessageMeta component with timestamp, copy, and edit actions (hover-reveal) - Integrate MessageMeta into AssistantMessage and UserMessage components - Reset task timer on editMessage in WebViewProvider This enables users to edit a previous user message, which rewinds the conversation to that turn and re-submits the edited content. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(webui): assert message datetime attribute * fix: address message edit rewind review feedback * fix(vscode): preserve edit turn indexes on session switch * fix(vscode): reset edit rewind turn indexing * fix(vscode): restore edit rewind state transactionally * fix(vscode): handle edit rewind review feedback --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
tanzhenxin
|
78ad595581
|
feat(core): support QWEN_HOME env var to customize config directory (#2953)
* feat(core): support QWEN_CONFIG_DIR env var to customize config directory Allow users to override the default ~/.qwen config directory location via the QWEN_CONFIG_DIR environment variable. This enables users on dev machines with external disk mounts or custom home directory layouts to persist config at a location of their choosing. Changes: - Add QWEN_CONFIG_DIR check to Storage.getGlobalQwenDir() (absolute and relative path support) - Eliminate 11 redundant '.qwen' constant definitions across packages - Replace 16+ direct os.homedir() + '.qwen' path constructions with Storage.getGlobalQwenDir() calls - Inline env var checks for packages that cannot import from core (channels, vscode-ide-companion, standalone scripts) - Add unit tests for the new env var behavior - Project-level .qwen/ directories are NOT affected Closes #2951 * fix(core): use path.resolve/join in QWEN_CONFIG_DIR tests for Windows compat Hardcoded Unix paths like '/tmp/custom-qwen/settings.json' fail on Windows where path APIs produce backslash separators. Use path.resolve() for inputs and path.join() for assertions so the tests pass cross-platform. * test(cli): remove flaky 'should keep restart prompt when switching scopes' test Timing-sensitive UI test that fails intermittently on Windows CI due to async ANSI output not settling within the wait window. * feat(core): route remaining hardcoded ~/.qwen/ paths through Storage.getGlobalQwenDir() Update channel status, memory command, extension storage, skills discovery, and memory discovery to use Storage.getGlobalQwenDir() instead of hardcoded os.homedir()/.qwen paths, ensuring QWEN_CONFIG_DIR env var is respected throughout the codebase. * fix(tests): mock os.homedir before makeFakeConfig for Storage.getGlobalQwenDir Storage.getGlobalQwenDir() is now called during Config construction, which requires os.homedir() to be mocked before makeFakeConfig() is called. Also mock Storage.getGlobalQwenDir in memoryCommand tests since it uses a cross-package import that vi.spyOn doesn't intercept. * fix(core): respect QWEN_CONFIG_DIR for .env discovery and install source findEnvFile() walk-up would find legacy ~/.qwen/.env before checking QWEN_CONFIG_DIR/.env when the workspace was under $HOME. Skip the legacy path when a custom config dir is set so the fallback picks up the correct file. Also add a legacy fallback in readSourceInfo() since the installer always writes source.json to ~/.qwen/ regardless of QWEN_CONFIG_DIR. * refactor(core): rename QWEN_CONFIG_DIR to QWEN_HOME and fix runtime path resolution Rename the env var before it ships (zero existing users) to match the convention of CARGO_HOME, GRADLE_USER_HOME, etc. — "HOME" means "root of all tool state", not just config. Key changes: - Rename QWEN_CONFIG_DIR → QWEN_HOME across all packages and scripts - Add shared path utils in vscode-ide-companion and channels/base to eliminate scattered inline env var resolution - Fix runtime path mismatch: IDE lock files and session paths in the vscode extension now route through getRuntimeBaseDir() (checking QWEN_RUNTIME_DIR first), matching core Storage behavior - Fix telemetry_utils.js otel path to check QWEN_RUNTIME_DIR for tmp/ - Add E2E integration tests for QWEN_HOME scenarios * fix(core): address critical review issues for QWEN_HOME support Pass resolved QWEN_HOME as a dedicated QWEN_DIR sandbox parameter so macOS Seatbelt profiles allow writes to custom config directories. Fix hookRunner treating signal-killed hooks as success by using ?? -1 instead of || 0. Add QWEN_HOME and QWEN_RUNTIME_DIR to the env vars documentation table. * fix(sandbox): whitelist QWEN_RUNTIME_DIR in macOS Seatbelt profiles When QWEN_RUNTIME_DIR is set separately from QWEN_HOME, the sandbox was blocking writes to the runtime directory (debug logs, chat history, IDE locks, sessions). Pass RUNTIME_DIR as a sandbox parameter and add the corresponding subpath rule to all six .sb profiles. * fix(core): add tilde expansion to QWEN_HOME and align satellite path helpers - Extract resolvePath() from resolveRuntimeBaseDir() so QWEN_HOME gets the same ~/tilde expansion that QWEN_RUNTIME_DIR already had. - Port resolvePath() to vscode-ide-companion and channels/base mirrors, fixing tilde handling in getRuntimeBaseDir() for the IDE companion. - Add missing os.tmpdir() fallback in channels/base getGlobalQwenDir(). - Add unit tests for tilde expansion in QWEN_HOME. - Clarify prompts.ts comment that system.md default is global, not project-level. * fix(core): add tilde expansion to scripts and fix extension cache QWEN_HOME support Add resolvePath() helper to standalone JS scripts (sandbox_command.js, telemetry.js, telemetry_utils.js) so QWEN_HOME=~/custom expands consistently with core Storage.resolvePath(). Fix ExtensionManager.refreshCache() to use ExtensionStorage.getUserExtensionsDir() instead of hardcoded os.homedir(), so extensions installed under a custom QWEN_HOME are discoverable. * test: remove flaky InputPrompt tab-suggestion test on Windows * test: remove flaky tests that fail intermittently on Windows Removes 'does not accept the prompt suggestion on shift+tab' from InputPrompt.test.tsx and 'should keep restart prompt when switching scopes' from SettingsDialog.test.tsx. Both have been observed to fail intermittently on the Windows CI workers; the underlying behaviors are covered by adjacent assertions and end-to-end tests. * revert(core): keep system.md path project-local under .qwen/ The QWEN_HOME refactor incorrectly routed the QWEN_SYSTEM_MD default path through Storage.getGlobalQwenDir() (i.e. ~/.qwen/system.md or $QWEN_HOME/system.md). The original semantics — inherited from the upstream Gemini-CLI sync — are project-local: <cwd>/.qwen/system.md. System-prompt customization is intentionally per-project so that each repository can ship its own override without global side effects. Users who want a global override can still set QWEN_SYSTEM_MD to an absolute path. This revert keeps that behavior intact while leaving the rest of the QWEN_HOME plumbing (settings, credentials, extensions, skills, memory) unchanged. * refactor(core): unify QWEN_CONFIG_DIR into the canonical QWEN_DIR Three definitions of the literal '.qwen' string existed across the codebase: - QWEN_DIR in config/storage.ts (canonical, used by the Storage class) - QWEN_CONFIG_DIR in memory/const.ts - QWEN_CONFIG_DIR in tools/memory-config.ts (a near-clone of the above) The QWEN_CONFIG_DIR name also collided with a former env-var name (now renamed to QWEN_HOME on this branch), making it ambiguous whether call sites referred to a configurable env var or a hardcoded directory name. Drop the duplicates and route the only call sites (prompts.ts and its test) through QWEN_DIR from config/storage.ts. The mock factory in config.test.ts is updated to no longer expose the removed export. * fix(integration-tests): use 'extensions list' to trigger settings migration Tests 2b and 3a in cli/qwen-config-dir.test.ts relied on running \`qwen --help\` to invoke loadSettings() (and thus the V1→V3 settings migration). That worked when loadSettings() ran before parseArguments() in the CLI startup sequence. Main has since flipped the order: parseArguments() runs first, and yargs intercepts --help and exits the process before loadSettings() is reached, so migration never runs and the tests' migration probe always reads back V1. Switch to \`qwen extensions list\` instead. It is a yargs subcommand that runs through main() to loadSettings() without requiring an API key, so migration runs as expected. Update the inline comments to document why --help cannot be used and why this command works. * fix(memory): route auto-memory base dir through Storage.getGlobalQwenDir() The auto-memory subsystem (introduced on main in #3087) computed its base directory by hardcoding path.join(os.homedir(), QWEN_DIR). That bypassed QWEN_HOME entirely, so global auto-memory artifacts always landed in ~/.qwen/projects/... regardless of the user's configured QWEN_HOME path. Route the default through Storage.getGlobalQwenDir() so QWEN_HOME is honored. The QWEN_CODE_MEMORY_BASE_DIR test override stays as the highest-priority short-circuit. Discovered while running the QWEN_HOME e2e test plan against the merged branch — Group B test B3 (memory tool writes to QWEN_HOME) was the only failing scenario across A/B/C/D groups. * fix(cli): treat custom QWEN_HOME .env as user-level When QWEN_HOME points to a directory whose path does not contain `.qwen` (e.g., `/tmp/qwen-home`), the global `.env` was misclassified as a project-level env file. As a result, default-excluded variables such as `DEBUG` and `DEBUG_MODE` were silently dropped even though they came from the user-level config directory. The classification now reuses the same user-level path set computed by `findEnvFile`, so any `.env` inside the resolved global Qwen directory (or directly under `~/`) is recognized as user-level. Also drop the misleading "does not expand `~`" note from the QWEN_HOME documentation — `Storage.getGlobalQwenDir` does expand leading tildes via `Storage.resolvePath`. * fix(cli): drop legacy .qwen substring check from env-file classification The user-level env-file detection now keys solely off the precomputed user-level path set, which already covers ~/.env and ${QWEN_HOME}/.env. The legacy substring fallback misclassified <repo>/.qwen/.env as user-level, so excludedEnvVars no longer applied to it. * fix(core): align plain-text hook output with documented exit-code semantics Per docs/users/features/hooks.md, only exit code 2 is a blocking error; all other non-zero exit codes are non-blocking and execution should continue. The plain-text branch in convertPlainTextToHookOutput previously denied on every non-zero, non-1 exit code (3, 127, signal fallbacks), contradicting the documented behavior. Collapse all non-blocking non-zero codes to EXIT_CODE_NON_BLOCKING_ERROR before passing into the converter so they take the warning path consistently. * chore: trigger CI * fix(cli): pass QWEN_HOME and QWEN_RUNTIME_DIR into docker/podman sandbox The container CLI previously had no awareness of the host's QWEN_HOME or QWEN_RUNTIME_DIR values. The global qwen dir worked only because the mount target happens to match the default fallback inside the sandbox, and the runtime base dir was lost entirely when it diverged from the global qwen dir. * fix(cli): canonicalize sandbox QWEN/RUNTIME paths and pin IDE lock dir Two reviewer-flagged issues from PR #2953: * macOS Seatbelt was passed `path.resolve` for `QWEN_DIR`/`RUNTIME_DIR` while neighbouring directories used `fs.realpathSync`. With a symlinked `QWEN_HOME` or `QWEN_RUNTIME_DIR`, sandbox-exec would compare against the canonical kernel path and deny writes. Create the dirs (so `realpathSync` can succeed on first run) then canonicalize them like the surrounding entries. * The VS Code companion wrote IDE lock files via the runtime base dir while the CLI side resolves the runtime dir from settings too. That divergence silently desynced lock-file discovery whenever a user set `advanced.runtimeOutputDir` without `QWEN_RUNTIME_DIR`. Anchor both sides to `getGlobalQwenDir()` since the companion process can only see env vars, not CLI settings. * fix(cli): finish QWEN_HOME plumbing across env, memory, rules, sandbox Codex review surfaced four user-visible spots where QWEN_HOME wasn't threaded through: * `findEnvFile` walked through the user home dir before consulting the QWEN_HOME fallback, so `~/.env` shadowed `<QWEN_HOME>/.env` and reversed the qwen-specific precedence the default `~/.qwen/.env` path enjoys. Add a home-dir-step check that prefers the custom Qwen dir when set. * `MemoryDialog` displayed and edited `~/.qwen/QWEN.md` regardless of QWEN_HOME. Memory discovery already routes through Storage, so user edits via the dialog were silently ignored at runtime. Route the dialog through `Storage.getGlobalQwenDir()` to match. * `loadRules` looked up global rules at `~/.qwen/rules/`, ignoring QWEN_HOME entirely. Use the global Qwen dir like the rest of the config surfaces. * The Docker/Podman sandbox path called `mkdirSync(userSettingsDir)` without `recursive`. Pre-PR the dir was always `~/.qwen` and the parent existed; with a nested QWEN_HOME like `/tmp/qwen/config` the first run threw ENOENT before the mount could be added. * fix(cli): block project .env from redirecting QWEN_HOME and QWEN_RUNTIME_DIR A project `.env` could set QWEN_HOME after settings were already loaded from the real home, splitting global state: settings.json read from ~/.qwen but later writes (installation_id, OAuth credentials, MCP tokens) landed in the project-controlled directory. The user-configurable excludedEnvVars list isn't the right place for this — it's a correctness boundary, not a preference — so always exclude these two vars from project .env files. User-level .env files (~/.qwen/.env) are unaffected. * fix(cli): keep workspace .qwen/.env unfiltered and pre-resolve user QWEN_HOME The env-file classification conflated two concerns: which paths may override global state vars, and which paths are exempt from the user-configurable excludedEnvVars filter. Splitting them lets a workspace `<repo>/.qwen/.env` carry DEBUG/DEBUG_MODE per the documented contract while still being blocked from redirecting QWEN_HOME or QWEN_RUNTIME_DIR. A QWEN_HOME set in `~/.qwen/.env` or `~/.env` would also previously arrive too late: USER_SETTINGS_PATH was captured at module load and loadSettings migrated `~/.qwen/settings.json` before loadEnvironment applied the override, leaving credentials, MCP tokens, and installation_id pointed at the new directory while settings stayed at the legacy one. A pre-pass now reads those user-level files for the two storage-controlling vars before any user settings are loaded, and the user settings path is re-resolved locally so all global state lands in one place. * fix(cli): make user-settings paths lazy to pick up bootstrapped QWEN_HOME USER_SETTINGS_PATH/USER_SETTINGS_DIR in settings.ts and the duplicate USER_SETTINGS_DIR in trustedFolders.ts were top-level consts evaluated at module load — before preResolveHomeEnvOverrides() reads QWEN_HOME from ~/.env or ~/.qwen/.env. Callers (sandbox launcher, trusted-folders reader) saw the legacy ~/.qwen path while the main CLI had moved to the custom home, splitting state. Convert all three to lazy getter functions and add a regression test that pokes process.env.QWEN_HOME after import and asserts each getter reflects it — any future top-level capture turns the test red. Mirror the same ~/.env / ~/.qwen/.env bootstrap into scripts/sandbox_command.js, which previously only read process.env directly and could disagree with the main CLI on the sandbox setting. Addresses review threads #3159793469, #3177804507, and item #2 of the 2026-05-06 review summary. * fix(cli): address qwen home review follow-ups * test(cli): normalize path in QWEN_HOME freshness assertion for Windows `getUserSettingsDir()` returns `path.dirname(...)`, which on Windows uses backslash separators. The bare string comparison failed on Windows runners ("\tmp\qwen-lazy-test" vs "/tmp/qwen-lazy-test"). Wrap the expected value in `path.normalize()` to match the OS-native separator, mirroring the two sibling assertions that already use `path.join()`. * fix(cli): close storage-routing leaks via settings.env and project sandbox .env settings.env (merged) was being applied to process.env without filtering, so a workspace settings.json could redirect global state by setting env.QWEN_HOME or env.QWEN_RUNTIME_DIR after the home-scoped .env bootstrap ran. Apply PROJECT_ENV_HARDCODED_EXCLUSIONS to the settings.env path too. scripts/sandbox_command.js's project-walk fallback called dotenv.config() to find QWEN_SANDBOX, which injected every parsed key — including QWEN_HOME / QWEN_RUNTIME_DIR the main CLI hard-blocks. Replace with a manual parse that copies only QWEN_SANDBOX. Add a startup migration warning when QWEN_HOME points to a directory with no settings.json while ~/.qwen/settings.json exists, so users notice that their existing OAuth tokens / settings / memory aren't auto-migrated. * test: cover QWEN_HOME / QWEN_RUNTIME_DIR in duplicated path helpers Adds targeted unit tests for the two TypeScript mirrors of Storage.getGlobalQwenDir() / getRuntimeBaseDir() that live outside packages/core to avoid cross-package imports. Covers default, absolute, relative, ~/x, ~\x, and bare ~ inputs, plus the runtime/home priority chain in the IDE companion. * fix: bootstrap QWEN_HOME before yargs handlers and in VS Code companion Two storage-routing leaks surfaced by Codex review of feat/qwen-config-dir: - channel status/stop call readServiceInfo() inside yargs handlers that process.exit before loadSettings() runs, so QWEN_HOME defined only in ~/.qwen/.env or ~/.env never resolved for them. The same race exists for the duplicate-instance check at the top of channel start. Hoist preResolveHomeEnvOverrides() to the top of main() so all subcommand handlers see the bootstrapped env vars. - The VS Code companion's getGlobalQwenDir / getRuntimeBaseDir read process.env directly, missing the same .env pre-pass. If a user only configures QWEN_HOME via ~/.qwen/.env, the CLI looks under the redirected dir while the companion writes IDE lock files under ~/.qwen, breaking IDE discovery. Mirror the CLI pre-pass in the companion (lazy, idempotent) without importing from core. * fix(config): preserve credentials in legacy ~/.qwen/.env when QWEN_HOME redirects When QWEN_HOME is bootstrapped from `~/.qwen/.env`, the home-dir env walk previously skipped that file and never read `<QWEN_HOME>/.env` from the companion. This stranded non-routing credentials (e.g. OPENAI_API_KEY) left in `~/.qwen/.env` and let the companion write IDE lock files into a different runtime dir than the CLI was reading from. - CLI: fall back to `~/.qwen/.env` after `<QWEN_HOME>/.env` at both the home-dir step and the post-walk fallback in findEnvFile, and treat the legacy path as user-level for trust and exclusion semantics. - Companion: after the initial candidate pass discovers QWEN_HOME, also read `<QWEN_HOME>/.env` so QWEN_RUNTIME_DIR sourced from there matches what the CLI's findEnvFile would pick. * refactor(cli): simplify QWEN_HOME plumbing — dedupe helpers, latch, comments - replace local isSameOrChildPath with core's isSubpath in sandbox.ts - latch preResolveHomeEnvOverrides so it runs once per process - pass userLevelPaths from loadEnvironment into findEnvFile (no recompute) - collapse findEnvFile's home-dir branch and post-loop fallback into one shared candidate list (drops duplicate existsSync calls) - factor extensionManager's user-extensions loop into a private helper - use QWEN_DIR constant instead of '.qwen' literal in skill-manager - trim narrative / PR-history comments across changed files * fix(cli): align QWEN_HOME .env bootstrap across CLI, sandbox, telemetry Telemetry scripts previously read process.env.QWEN_HOME directly, so a QWEN_HOME set only in ~/.env or ~/.qwen/.env left telemetry writing to ~/.qwen while the CLI routed elsewhere. Extract the bootstrap into scripts/lib/qwen-home-bootstrap.js and have sandbox_command.js, telemetry.js, and telemetry_utils.js share it. Also add a third-pass <new QWEN_HOME>/.env read in preResolveHomeEnvOverrides so the CLI and VS Code companion agree on QWEN_RUNTIME_DIR when it is configured under the new home dir. * test(integration-tests): update QWEN_HOME assertions for v4 schema Settings schema was bumped to v4 on main (gitCoAuthor migration). The qwen-config-dir tests still asserted post-migration $version === 3, so they failed after the merge. Bump the assertions to 4 and the seed in 3a to match, and point a comment at SETTINGS_VERSION so the next bump is easy to find. |
||
|
ChiGao
|
ccabd9d908
|
fix(cli): unfreeze Ctrl+O compact-mode toggle on long conversations (#3905)
* fix(cli): unfreeze Ctrl+O compact-mode toggle on long conversations Toggling compact mode on a long conversation called refreshStatic(), which clearTerminal'd and forced <Static> to remount every history item synchronously on the input thread — N items × per-item Ink layout/render blocked the keyboard for seconds. Issue #3899. Two narrow changes that preserve current UX: 1. compactToggleHasVisualEffect(history) skips refreshStatic() when no past item would render differently (history without tool_group or gemini_thought*). Future items still pick up the new mode via the live React state — Static is append-only. Plain-chat sessions no longer freeze on Ctrl+O regardless of length. 2. MainContent now feeds <Static> a progressive slice of mergedHistory when the catch-up gap is large. Below 100 items the slice jumps to full length in one render (bit-identical to the previous behavior); above that, it grows in 50-item chunks via setImmediate so the event loop yields between batches and the input thread stays responsive during the post-Ctrl+O remount. Also covers the post-resume initial mount path: if a session resumes with a large history, the first paint is no longer a single blocking render of every item. Tests: 7 new (5 for compactToggleHasVisualEffect, 2 for the progressive Static replay path). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): address review comments on Ctrl+O freeze fix Follow-up to PR review on #3899: - Rewrite compactToggleHasVisualEffect doc comment to give the actual reason for the conservative tool_group → true rule (force-expand detection needs embeddedShellFocused/activePtyId, unavailable cheaply at the keypress call site). - Add a TODO above PROGRESSIVE_REPLAY_* constants noting the unbenchmarked thresholds and the line-budget alternative. - Add a TODO at visibleHistoryItemsWithSourceCopyOffsets documenting the catch-up invisibility window and the tail-buffer follow-up. - Drop redundant useMemo wrapper around the slice — the underlying inputs only change when the slice would change anyway. - Mirror historyManager.history into a ref so the keypress handler reads the latest snapshot at call time without depending on the full historyManager object (which useMemo rebuilds on every history change). Removes historyManager from the keypress callback deps. - Add AppContainer integration tests verifying the Ctrl+O handler skips refreshStatic for plain history and triggers it for tool_group history (uses the same handler-discovery pattern as the existing renderMode toggle test). - Tighten the progressive-replay test: assert monotonic growth per drained setImmediate tick instead of relying on a 'drain enough ticks' upper bound. All 5134 tests green; lint clean. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): reset Static replayCount synchronously on remount key change Wenshao's [Critical] review on #3905: the previous useEffect-based reset fired AFTER the render that already passed the (post-catch-up) full replayCount to <Static>. Because <Static>'s key is bumped in the same render, Ink remounts synchronously with the full history before the effect ever runs — defeating the chunked-replay protection that the PR is supposed to provide for tool/thinking-bearing histories. Switch to the canonical "store previous prop in state" pattern: track the last seen historyRemountKey alongside replayCount and resync both during render. setState during render queues a discard-and-rerun, so <Static> only ever commits with the post-reset (first-chunk) slice on the remount. Refs alone aren't enough — they don't trigger the re-render that lets the slice take effect. Add a regression test that drives a 200-item history to full catch-up, then bumps historyRemountKey and asserts the very next render falls back to the first chunk (53 items, not 203). Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(cli): use ToolCallStatus enum in Ctrl+O fixture Copilot review nit on #3905: the tool_group fixture in the Ctrl+O integration test used the raw string 'Success' for `status` even though `ToolCallStatus` is already imported in this file. Switching to the enum keeps the fixture type-safe and would catch silent regressions if the enum values ever change. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): prevent finalized item from disappearing during progressive replay When a pending item finalizes into mergedHistory while replayCount lags behind, the item was removed from pendingHistoryItems but not yet visible in the Static slice — creating a brief "disappear frame" for one render. Fix: render the full list whenever `historyItemsWithSourceCopyOffsets.length - replayCount <= PROGRESSIVE_REPLAY_CHUNK_SIZE` (small tail gap). This covers the normal append path without blocking the input thread, while still yielding via setImmediate for large remount gaps (the Ctrl+O freeze the PR is fixing). Removes the TODO comment and adds a regression test that pins the "no disappear frame" behaviour for small-delta appends. 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> |
||
顾盼
|
b55b52543a
|
feat(cli): improve slash command discovery (#3736)
* feat(cli): improve slash command discovery Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(cli): update input prompt completion expectations Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): address review feedback on slash command phase 3 - Fix getBestSlashCommandMatch sort order: completionPriority first, recentOrder second — consistent with compareRankedCommandMatches in useSlashCompletion.ts so ghost text and dropdown agree on best match - Fix findSlashCommandTokens to index altNames into commandMap so alias tokens (e.g. /usage for /stats) are highlighted as valid instead of being marked invalid - Fix getRecentScore decay formula: 10 * Math.max(0, 1 - ageMs / RECENT_DECAY_MS) so the recent boost truly decays to 0 within the 10-minute window named by RECENT_DECAY_MS - Fix Help.tsx CommandsHelp scroll indicator to show command count range (e.g. 1-12/49) instead of raw line count (18/108), which was confusing because each command expands into 2-3 render lines - Fix Help.tsx CommandLine key prop: use stable type:text:index key instead of scrollOffset-index to avoid remounting every line on scroll - Internationalize Help.tsx tab labels via t() instead of hardcoded English strings - Add Tab/Shift+Tab to switch tabs hint in Help footer alongside Esc - Add commandMetadata.test.ts with full branch coverage for all 6 exported functions (getCommandSourceBadge, getCommandSourceGroup, formatSupportedModes, getCommandDisplayName, getCommandSubcommandNames, formatCommandSourceLabel) - Add direct unit tests for getBestSlashCommandMatch covering: null on empty input, null on no match, null for non-modelInvocable commands, completionPriority ordering, recentCommands tie-breaking, argumentHint return path, exact-match exclusion without hint, inclusion with hint - Update Session.test.ts expectation for sendAvailableCommandsUpdate to include _meta field that was added in this PR * test(cli): add missing test coverage for slash completion - Add test: midInputGhostText is null when only non-modelInvocable commands match - Add test: recentCommands boosts non-root prefix suggestions via recentScore Both tests address coverage gaps identified in code review. --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
顾盼
|
f5af7fbf95
|
feat(memory): add autoSkill background project skill extraction (#3673)
* feat(memory): add autoSkill background project skill extraction
* fix(test): add missing mock methods for autoSkill (getAutoSkillEnabled, recordCompletedToolCall, consumePendingMemoryTaskPromises)
* fix(test): fix cross-platform path comparison in skillReviewNudge integration test
* fix(autoSkill): address critical review comments
- Fix merged_with_extract silent drop: remove broken merge optimization
in scheduleSkillReview(). When an extract task is pending/running,
skill review is now scheduled independently instead of recording
metadata that no production code ever reads.
- Fix SKILL_MANAGE blocked from skill review agent: prepareTools() now
only enforces the recursion guard (AGENT tool) when the agent has an
explicit tools list. Wildcard/inherit subagents still get the full
EXCLUDED_TOOLS_FOR_SUBAGENTS filter, preventing task subagents from
calling skill_manage. The dedicated skill review agent can now receive
the skill_manage tool it requires.
- Update manager.test.ts: replace merged_with_extract tests with
concurrent-extract independent-scheduling tests.
- Update skill-manage.test.ts: clarify test description to reflect
wildcard-only exclusion semantics.
* fix(autoSkill): reject symlink traversal in skill_manage path guard
assertProjectSkillPath() uses path.resolve() which is purely lexical and
does not dereference symlinks. If any path component inside .qwen/skills/
is a symlink pointing outside the project, fs.writeFile/readFile/rm would
follow the link and mutate files outside the advertised write boundary.
Add assertRealProjectSkillPath() (async) in skill-paths.ts that:
- Resolves the real path of the skills root via fs.realpath()
- Walks up from targetPath to find the nearest existing ancestor
- Resolves that ancestor to its real filesystem path
- Rejects if the real path falls outside the real skills root
skill-manage.ts execute() now calls both the cheap lexical check (fast
fail for obviously wrong paths) and the async real-path check before any
fs.writeFile / fs.rm mutation.
Add three symlink-specific tests in skill-paths.test.ts covering:
- Legitimate path accepted
- Symlinked directory pointing outside skills root rejected
- Skills root itself being a symlink (safe target) accepted
* refactor(autoSkill): remove skill_manage tool, use path-based skill write detection
Address reviewer feedback: instead of keeping skill_manage as the sole
write gate (which still had symlink bypass risk via generic tools), remove
the dedicated tool entirely and replace with a two-layer protection:
1. skillsModifiedInSession (client.ts): detects writes to .qwen/skills/
by inspecting the file_path arg of every completed tool call, replacing
the fragile historyCallsSkillManage() history scan.
2. hasAutoSkillSource + evaluateScopedDecision (skillReviewAgentPlanner.ts):
the review agent's permission sandbox now verifies BOTH that the target
path is inside the skills directory AND that the existing file already
contains 'source: auto-skill' in its frontmatter before allowing edits,
preventing the agent from overwriting user-managed skills.
Changes:
- Delete skill-manage.ts and skill-manage.test.ts
- Remove SKILL_MANAGE from ToolNames, ToolDisplayNames, config registerLazy,
agent-core EXCLUDED_TOOLS comment, and agent.ts comment
- Replace historyCallsSkillManage() with skillsModified: boolean param in
scheduleSkillReview; skip reason renamed skills_modified_in_session
- recordCompletedToolCall(name, filePath?) detects .qwen/skills/ writes;
CLI layers pass file_path arg from tool call request
- Fix buildTaskPrompt frontmatter template to use top-level source: auto-skill
- Update skill-paths.ts error messages to remove skill_manage references
- Update all unit/integration tests accordingly
* fix(autoSkill): deduplicate concurrent skill-review tasks per projectRoot
scheduleSkillReview() was launching a new background task every time the
threshold was reached for the same project, with no guard against multiple
in-flight reviews running concurrently.
Fix: add skillReviewInFlightByProject Map that tracks the taskId of any
running review per projectRoot. A second call while one is in-flight returns
{ status: 'skipped', skippedReason: 'already_running', taskId: <existing> }.
The map entry is cleared in a finally block inside runSkillReview() so the
next session can schedule a fresh review after the current one completes.
Also extend SkillReviewScheduleResult.skippedReason union to include
'already_running', and add a unit test covering the full lifecycle:
first call schedules, second call is skipped with existing taskId, and a
third call after completion schedules a new task.
* fix(autoSkill): address all critical review comments
1. hasAutoSkillSource: narrow catch to ENOENT only (EISDIR/EACCES etc.
return false to deny); tighten frontmatter regex to match opening block only.
2. evaluateScopedDecision: add explicit allow for READ_FILE and LS so they
don't fall to 'default' which the base PermissionManager might widen;
EDIT/WRITE_FILE now call assertRealProjectSkillPath() (async realpath guard)
in addition to the lexical check, closing the symlink traversal hole.
3. isScopedTool / getScopedDenyRule: cover READ_FILE and LS so hasRelevantRules
returns true and findMatchingDenyRule is correctly consulted for them.
4. recordCompletedToolCall (client.ts): broaden tool name set to match
WRITE_TOOL_NAMES in manager.ts (write_file, edit, replace, create_file) and
inspect all three arg keys (file_path, path, target_file). Signature changed
from (name, filePath?) to (name, args?) to carry all args through.
5. client.ts hardcoded literals: replace threshold/maxTurns/timeoutMs with the
named constants AUTO_SKILL_THRESHOLD / DEFAULT_AUTO_SKILL_MAX_TURNS /
DEFAULT_AUTO_SKILL_TIMEOUT_MS imported from manager.ts and
skillReviewAgentPlanner.ts.
6. toolCallCount / skillsModifiedInSession reset: only reset when skill review
is actually scheduled (status === 'scheduled'), not every turn, so the
counter correctly accumulates across turns within a session as per design doc.
7. runSkillReview (manager.ts): rethrow after marking record failed, consistent
with runExtract behavior.
8. skillReviewNudge.integration.test.ts test 5: rewrite to reflect the
in-flight dedup contract (second same-project call returns already_running
with existing taskId; third call after completion gets a new task). Add
vi.mock for runSkillReviewByAgent so the test does not need a full Config.
* fix(autoSkill): address all review comments
- skill-paths: detect dangling symlinks with lstat before treating ENOENT as safe
- skill-paths: fix isProjectSkillPath relative path resolution to use projectRoot
- skillReviewAgentPlanner: restrict READ_FILE/LS to project root only
- skillReviewAgentPlanner: remove SHELL tool from review agent tool list
- skillReviewAgentPlanner: add path import; remove unused shell imports
- skillReviewAgentPlanner: add comment for buildAgentHistory trailing user message
- client: fix runManagedAutoMemoryBackgroundTasks gate widening
- client: fix skillsModifiedInSession deadlock
- client: add .catch() to skill review promise
- client: hoist SKILL_WRITE_TOOL_NAMES to module-level ReadonlySet
- agent-core: use full EXCLUDED_TOOLS_FOR_SUBAGENTS for explicit tool list subagents
- manager: extend notify() signature to accept 'skill-review' taskType
- config: fix JSDoc default value comment (false, not true)
* fix(autoSkill): address second round review comments
- client: reset toolCallCount when scheduleSkillReview returns already_running
and count >= threshold, preventing immediate cascade after in-flight review
- client.test: add autoSkill branch tests (scheduled/already_running/skills_modified)
- client.test: add full recordCompletedToolCall unit tests (skillsModifiedInSession,
toolCallCount increment, skill path detection for write_file/edit/read_file)
- client.test: add scheduleSkillReview mock to mockMemoryManager
- nonInteractiveCli.test: add assertions for recordCompletedToolCall and
consumePendingMemoryTaskPromises in tool-call integration test
|
||
|
jinye
|
3f60e595d8
|
fix(core) monitor notifications for subagents (#3933)
* fix(core): route monitor notifications to owner agent Route subagent-owned monitor notifications back into the owning agent instead of the parent queue. Keep owner agents alive while their monitors can still produce notifications, and clean up owned monitors silently when the owner exits. Fixes #3925 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): address monitor owner routing review Clear owner monitor callbacks on registry reset, avoid owner-helper allocation in the monitor idle-wait path, and guard waitable external-input queues against abort races after listener registration. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): update monitor agent context import Align Monitor owner-context imports with the runtime agent context module added on main, and update the related test to use the current runWithAgentContext signature. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): snapshot owner monitor cancellations Collect owner monitor ids before cancelling so pruning terminal entries during cancellation cannot affect the iteration. Add coverage for cancelling owner monitors while retained terminal entries are pruned. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): wake owner monitor waits on silent stop Wake subagent external-input waits when owner monitors are stopped silently, re-check wait predicates after empty wakes, and add coverage for foreground, fork, and resumed owner-monitor routing cleanup. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): preserve external input wake metadata Make implicit-fork external input waits safe for overlapping waiters and persist external input kind for notification observability in subagent transcripts. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): close monitor cleanup races Avoid owner monitor lost wakeups before idle wait registration, make silent monitor cancellation suppress partial-line notification dispatch, and clean resumed owner monitor callbacks when setup fails before execution. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): preserve deferred approval owner context Restore the logical agent id when approved tool continuations re-enter agent frames, keep external message kinds backwards compatible, and document widened background task inputs. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): clean up monitor owner resources Close the resumed agent transcript writer when setup fails before the run body starts, and cancel owner monitors before unregistering their callbacks to avoid stale notification races.\n\nCo-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
John London
|
b1d33dbda2
|
fix(cli): preserve comments and formatting in settings.json during migration write-back (#3861)
* fix(cli): preserve comments and formatting in settings.json during migration
The persistSettingsObject() helper in loadAndMigrate() previously used
writeWithBackupSync() with raw JSON.stringify(), which stripped all comments
and custom formatting from users' settings.json on every migration or version
normalization.
Fix: Refactored updateSettingsFilePreservingFormat() in commentJson.ts with:
1. Sync mode (sync=true): Removes keys from the original file that are not
present in the migrated object, preventing zombie keys from persisting
after migrations that remove deprecated settings.
2. Atomic writes: Replaced fs.writeFileSync with a writeFileSyncAtomic()
helper that uses temp-file + rename for crash-safe writes. This applies
to both the migration path (persistSettingsObject) and the runtime
setValue path (saveSettings).
3. Comment preservation: Uses comment-json's parse() during the load phase,
so comment metadata is retained in the parsed structure and preserved by
stringify() during write-back. Keys that exist in both the original file
and the migrated object keep their original comments.
persistSettingsObject now calls updateSettingsFilePreservingFormat with
{sync: true} to get all three guarantees: comment preservation, zombie key
removal, and atomic writes.
The test file updates (settings.test.ts, commentJson.test.ts) from the
previous iteration are reverted since the write mechanism now goes through
the same writeFileSyncAtomic path.
Fixes #3843
* fix(cli): remove nested zombie keys during migration sync
The sync mode in updateSettingsFilePreservingFormat only removed top-level
zombie keys. Nested deprecated keys (e.g. general.disableAutoUpdate) survived
because applyUpdates performed a deep merge without removing keys absent from
the migrated object.
Fix: Added a sync parameter to applyUpdates() that recursively deletes keys
not present in the updates object at every nesting level. This ensures
deprecated keys like general.disableAutoUpdate are properly cleaned up during
V2→V3 migration write-back.
Added 3 new tests:
- Nested zombie keys removed in sync mode
- Top-level zombie keys removed in sync mode
- Unrelated keys in nested objects preserved during sync
* fix(pr-3861): address review comments on settings.json comment preservation
- Replace writeFileSyncAtomic with writeWithBackupSync from writeWithBackup.ts
to eliminate duplicate atomic-write implementations
- Simplify UpdateSettingsOptions interface to a plain boolean `sync` parameter
- Include parse error details in stderr output (error.message with position info)
- Add test for written=false path in settings.test.ts
- Add test for sync=true with empty updates={} documenting zombie key removal
- Mock statSync in settings test to support writeWithBackupSync directory check
* fix(pr-3861): add debugLogger mock and assert error on written=false
- Add mockDebugLogger with debug/warn/error/info to settings.test.ts
- Enhance existing written=false test to assert debugLogger.error is called
- All 96 settings tests + 15 commentJson tests pass
|
||
Rayan Salhab
|
7910a5bd4b
|
fix(core): filter Mistral reasoning content at request boundary (#3882)
Co-authored-by: cyphercodes <cyphercodes@users.noreply.github.com> |
||
|
jinye
|
5316edb6d8
|
feat(core): add reactive compression on context overflow (#3879)
* feat(core): add reactive compression on context overflow Force-compress chat history after a provider rejects a request for exceeding the context window, then retry the request once with refreshed history. Add provider-agnostic context overflow classification and focused retry coverage. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): address reactive compression review feedback --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
qqqys
|
0411d05a2a
|
test(cli): drop wait-dependent SessionPicker search tests (#3978)
The Search describe block in StandaloneSessionPicker.test.tsx was
introduced in #3880 and consistently failed on Test (windows-latest,
20.x / 22.x) — six of its tests assumed a 30 ms inter-key wait was
enough for the keypress → useEffect → render chain to commit, which
slow Windows runners regularly missed. The lone fix in
|
||
|
Edenman
|
250d158821
|
fix(core): drop disabled MCP server from health status registry (#3916)
Some checks are pending
Qwen Code CI / Lint (push) Waiting to run
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* fix(core): drop disabled MCP server from status registry The global `serverStatuses` map only added entries — disabling a server via `/mcp` left its `DISCONNECTED` entry in place, so the Footer's MCP health pill kept counting the server as offline. Add `removeMCPServerStatus` and call it from `disableMcpServer` so the server drops out of the health snapshot. `disconnectServer` (used for transient reconnects like OAuth and health-check retries) keeps the entry as before. Fixes #3895 * fix(core): guard MCP status updates after disconnect Address review feedback on #3916: - McpClient.updateStatus now bails out when isDisconnecting is set, so an in-flight connect()'s catch block can no longer resurrect a server that disableMcpServer already removed from the global registry. - disableMcpServer wraps the disconnect call in try/finally so the status entry is cleared even if disconnect throws. - McpClientManager.removeServer also calls removeMCPServerStatus for symmetry — a server removed from configuration should drop out of the health snapshot the same way a disabled one does. - updateMCPServerStatus / removeMCPServerStatus iterate over a snapshot of statusChangeListeners so a listener detaching itself during dispatch doesn't mutate the array we're iterating. Adds two regression tests: one drives the connect/disconnect race and asserts the entry stays removed; another verifies disableMcpServer clears the entry even when disconnect rejects. * fix(core): order MCP exclusion update before status removal Move setExcludedMcpServers ahead of removeMCPServerStatus inside the finally block, so a disabled server is marked excluded before its status entry is dropped. Closes the (currently synchronous) window where doctorChecks would observe a missing status while the exclusion list still hadn't been updated, mis-reporting an intentional disable as a connectivity failure. * fix(core): keep MCP status cleanup running if exclusion update throws Wrap setExcludedMcpServers in a nested try/finally inside disableMcpServer so removeMCPServerStatus still runs if the config write throws. Otherwise a future config impl that fails on write would leave a stale entry in the global status registry, resurrecting the #3895 health-pill bug. Also tighten the existing throw-path test to assert the exclusion list is still updated when disconnect throws (now that the update lives in the finally block), and add a new test for the exclusion-update-throws case. |
||
|
tanzhenxin
|
b87a154760
|
fix(core): log the OpenAI request actually sent on the wire (#3767)
* fix(core): log the OpenAI request actually sent on the wire The --openai-logging capture was a parallel reconstruction that copied only a small subset of fields, silently dropping anything the provider layer injected — extra_body (so enable_thinking/thinking), DashScope metadata, stream/stream_options, and samplingParams pass-through keys like reasoning_effort. Anyone debugging "what did we actually send?" saw a stripped-down view that disagreed with the wire payload. Surface the fully built request from the pipeline to the logging decorator via an AsyncLocalStorage-scoped capture, so the log file mirrors the SDK call. The synthetic reconstruction stays as a fallback for non-OpenAI generators that don't go through the pipeline. * fix(core): isolate OpenAI logging failures from API result Wrap each session.resolve() / logOpenAIInteraction call in LoggingContentGenerator with try/catch so a synthesis failure inside the synthetic-fallback path can no longer mask a successful API response or replace the original API error. Also note the load-bearing position of the capture call in the OpenAI pipeline. * docs(core): broaden logging-isolation comment scope Comment said the try/catch protects against resolve() throws, but the same block also catches logOpenAIInteraction throws. Reword to match. * fix(core): isolate streaming logging failures too Mirror the non-streaming try/catch around `logOpenAIInteraction` inside `loggingStreamWrapper`, so a logger throw on stream completion cannot turn a fully-yielded stream into an error, and a logger throw in the catch path cannot replace the original stream/API error. |
||
|
易良
|
199c0e2902
|
fix(cli): validate /model command arguments (#3963)
Some checks failed
Qwen Code CI / Lint (push) Waiting to run
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Waiting to run
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
SDK Python / SDK Python (3.11) (push) Has been cancelled
SDK Python / SDK Python (3.12) (push) Has been cancelled
SDK Python / SDK Python (3.10) (push) Has been cancelled
* fix(cli): validate model slash command arguments * fix(cli): improve model command validation feedback * fix(cli): keep fast model picker behavior unchanged |
||
|
qwen-code-ci-bot
|
d1a600acc4
|
chore(release): v0.15.9 [skip ci]
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> |
||
|
易良
|
983322b1df
|
ci: reduce PR test matrix runtime (#3962) | ||
|
易良
|
940be2db84
|
feat(skills): reload slash commands when SkillManager fires change event (#3923)
* feat(skills): reload slash commands when SkillManager fires change event SkillManager already rebuilds its own cache and notifies SkillTool when skill files change, but SkillCommandLoader.loadCommands() only runs once during CommandService.create(). A newly added SKILL.md would update <available_skills> via setTools() but leave the slash command list stale until restart. Subscribe slashCommandProcessor to SkillManager.addChangeListener so the existing reloadCommands() path runs on every skill change, mirroring the IdeClient status-change wiring already in place. Progress on #3696. * fix(skills): subscribe slash reload after config init * fix(skills): ignore aborted slash command reloads |
||
|
qqqys
|
c4a54ac96e
|
fix(core): route countSessionMessages through parseLineTolerant (#3692)
* fix(core): route countSessionMessages through parseLineTolerant
`countSessionMessages` had its own readline loop that silently dropped any
line failing `JSON.parse` — including `}{`-glued lines that #3606's
read-side recovery now handles. Effect: a corrupted session's
`listSessions` count was lower than the record count seen on resume.
Export `parseLineTolerant` from `jsonl-utils` (the helper previously
private to `read()` / `readLines()`) and route the count loop through it.
The N records inside a glued physical line now contribute to the uuid
set instead of being dropped, so list-view count and resume count agree.
Stays on the streaming path: listSessions hot-pathes 20 files per page
and active sessions can hold thousands of records, so this avoids the
full-load cost of a `jsonl.read()` + dedupe alternative.
Refs #3681 — Item 1. Item 2 (write-side durability) is intentionally
deferred per the issue.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(core): apply parseLineTolerant to readLastRecordUuid + filter scalars
Address #3692 review feedback:
- `readLastRecordUuid` had the same bare `JSON.parse + catch { continue }`
pattern that this PR fixed in `countSessionMessages`. A `}{`-glued tail
line was silently dropped, so `renameSession` set the synthetic title
record's `parentUuid` to a stale uuid and `reconstructHistory` truncated
the chain on resume. Routed through `parseLineTolerant`, walking
recovered fragments bottom-up so the latest record wins.
- `parseLineTolerant` now filters non-object JSON values (`null`, scalars)
centrally. Previously a bare `null` line would have been forwarded as
`[null]` and the caller's `record.type` access would propagate to the
outer catch in `countSessionMessages`, zeroing the whole file's count —
a regression vs. the old per-line `try/catch { continue }`.
- Added integration tests in a dedicated corruption-recovery file (real
fs, no jsonl-utils mock) covering: `}{`-glued count, scalar-line guard,
uuid dedupe, last-uuid recovery from a glued tail line, malformed-tail
walkback.
- Default-mocked `parseLineTolerant` to `[]` in sessionService.test.ts so
future tests adding code paths through it don't trip on the auto-mock's
`undefined` return.
Telemetry-counter suggestion deferred — wiring metrics for the recovery
warning expands scope beyond this fix; tracked separately if needed.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(core): close partial-tail uuid leak in readLastRecordUuid + tighten parseLineTolerant filter
Address #3692 review (gpt-5.5 + Opus 4.7 via /review):
- Critical (gpt-5.5): readLastRecordUuid ran tolerant recovery on the first
split segment of the 64 KiB tail window, which when readStart > 0 is the
middle of a record that started before the window. _recoverObjectsFromLine
starts brace-counting from depth 0, so a balanced inner {"uuid":...} object
in the record's payload would be parsed as a top-level fragment and
surfaced as the "last record uuid". renameSession would then anchor
custom_title.parentUuid at payload data and reconstructHistory would
truncate the chain on resume after a rename. Drop the first segment when
readStart > 0; complete physical lines after the first '\n' stay safe.
- Nit: parseLineTolerant's typeof === 'object' guard let arrays through
(typeof [] === 'object'). Add !Array.isArray(parsed) so the docstring's
"non-object values are filtered out" claim holds strictly. Behaviorally
no live writers emit array records, but the inconsistency was a future
trap for callers that drop their own Array.isArray guards.
- Nit: drop the now-dead `?.` in readLastRecordUuid (record?.uuid). With
parseLineTolerant's filter, parser-returned records cannot be null /
undefined; aligns with countSessionMessages's plain record.type access.
- Tests:
- jsonl-utils.test: array-filter unit case; read()/readLines() regression
locking the scalar/array drop (broader semantic change beyond #3681
Item 1, called out so future maintainers don't trip).
- sessionService.corruption.test: 160 KB filler-array record with an
embedded trojan {"uuid":"fake-from-payload"} object — asserts the
function does not surface the payload uuid. Filler is unquoted digits
so the parser's inString state stays aligned mid-fragment, keeping the
trojan reachable without the boundary guard.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(core): only shift partial first segment in readLastRecordUuid
The previous guard `if (readStart > 0) lines.shift()` was too eager.
When the tail window happens to start on a newline boundary —
e.g. `prev\n<record>\n` where `record + '\n'` is exactly
TAIL_READ_SIZE — the first split segment is a complete record, not
a partial fragment. Unconditionally dropping it returns `null`
even though the latest record was fully readable, and renameSession
then writes `custom_title.parentUuid` as null and truncates history
on resume.
Peek the byte at `readStart - 1`: if it's `\n` the window is
boundary-aligned and the first segment is whole; otherwise the
window started mid-line and the partial must be discarded so
tolerant recovery doesn't surface a balanced inner `{ "uuid": ... }`
from inside the record's payload as a top-level uuid.
Cover both directions:
- Positive twin of the existing partial-tail test: a giant partial
first line followed by one complete record must return the
complete record's uuid (closes the L151 suggestion that the
existing assertion would still pass if every line were skipped).
- Boundary-aligned case: `prev\n<final>\n` with `final + '\n'` ===
TAIL_READ_SIZE; would return `null` under the old shift.
Co-authored-by: Qwen-Coder <noreply@alibabacloud.com>
---------
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <noreply@alibabacloud.com>
|
||
|
qqqys
|
08b491533e
|
feat(cli): searchable /resume picker with focus-aware modes (#3880)
* feat(cli): searchable /resume picker with focus-aware modes The session picker now has a search input that owns the keyboard exclusively while focused, so users can find a session by typing without losing the existing list-mode shortcuts when they're done. Modes share one query string: - list (default + post-narrow): full shortcut keymap is live — j/k navigate, Space previews (when enablePreview), Ctrl+B toggles branch, '/' enters search (preserves existing query), Esc clears a non-empty query before a second Esc actually cancels. - search: the input owns the keyboard. Printable chars (including Space) append to the query; Backspace pops; Esc clears + exits; Ctrl+U/Ctrl+L wipe + exit; Enter commits the filter (drop to list, query preserved) instead of jumping into the highlighted session — guards against accidental resume while typing. - ↑↓ are routed before either mode: in search they exit to list without advancing the cursor (so ↓ from search lands on the first match, not the second); in list, ↑ at index 0 wraps back into search. Search-input editing keys live in a dedicated useSessionSearchInput hook so the outer picker stays focused on mode dispatch and list state. The query is matched as a case-insensitive substring against customTitle, prompt, and gitBranch (composed AND with the branch toggle). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): tighten session-picker keymap edges Two small cleanups from the #3880 review: - moveSelection: hoist the empty-list early-return so both -1 and +1 branches share it. Previously only the +1 branch had a guard; -1 coasted on Math.max(0, 0-1) === 0, which doesn't crash but signals the empty case wasn't being thought about — a future tweak in either branch could drift past length 0. - j/k handlers: add a comment that they are intentionally claimed before the implicit-search-seed fallback below, so vim users stay in list mode and typing `j` never seeds the query with "j". Co-authored-by: Qwen-Coder <noreply@alibabacloud.com> * refactor(cli): lift onExitToList out of search-query updater Per #3880 review L120: calling `onExitToList()` inside the `setSearchQuery` updater is a React anti-pattern — React 18 StrictMode double-invokes updaters in dev to verify purity, which could fire `onExitToList` twice. Today that's harmless because `setViewMode('list')` is idempotent, but any non-idempotent work the parent grows into the callback later (telemetry, navigation side effects) would silently double up. Converge all four exit paths (Esc, Backspace-to-empty, Ctrl+U, Ctrl+L) on a single `useEffect` watching for a non-empty → empty query transition, gated by a previous-value ref so initial mount (query starts at '') doesn't falsely fire. Each handler now only mutates the query. Cover the new shape with a dedicated test file: 9 cases for `isPrintableSearchChar` (ASCII / SPACE / Ctrl / Meta / paste / multi-char / empty / control / DEL) and 12 cases for the hook itself (initial mount, append, accumulate, Backspace pop / pop-to- empty, Delete, Esc, Ctrl+U, Ctrl+L, silent-swallow of Tab/PageUp/ Ctrl+B, setSearchQuery setter round-trip). Co-authored-by: Qwen-Coder <noreply@alibabacloud.com> * test(cli): bump search→Enter waits to fix Windows CI flake The "↓ from search lands on the first match" test was the lone failure on Test (windows-latest, 22.x) for #3880 — its 30ms inter-key waits between `/login`, ARROW_DOWN, and Enter weren't giving the keypress → useEffect → render chain enough time on slower Windows runners; the Enter event was getting dropped and the spy never saw the selection. Bumped the three intermediate waits to 50ms, matching what the other multi-step tests in this file already use for similar sequences. macOS and Linux runners always passed at 30ms, so this is a Windows-runner timing fix, not a behavior change. Co-authored-by: Qwen-Coder <noreply@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Qwen-Coder <noreply@alibabacloud.com> |
||
|
ChiGao
|
d0eb5c2547
|
fix(cli): trim blank streaming tails from live preview (#3965)
Some models stream long runs of trailing newlines after useful content. Trim them from the pending live viewport so blank rows do not push stable streaming text into scrollback on every repaint. The committed transcript still renders the full assistant message through MarkdownDisplay with isPending=false, so transcript fidelity is preserved. Generated with AI Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
Dragon
|
5534421480
|
fix(cli): persist ACP model selection (#3947) | ||
|
tanzhenxin
|
2538f6d415
|
fix(cli): show tool details in subagent approval banner (#3956)
The compactMode early-return in ToolConfirmationMessage hid the per-type body and question, so the inline subagent banner showed only "Approval requested by <agent>: / Do you want to proceed?" with three options and no indication of which command, file, or MCP tool was being approved. Move the compact-mode handling to the unified return path so per-type body and question render in compact form too. Compact mode also: - Swaps the type-specific exec/mcp question for the generic prompt (the body already shows the command or labeled server + tool, and the exec rootCommand summary surfaces a pre-existing core parser oddity for heredocs that we'd rather not echo into every banner). - Caps the body at 5 lines with MaxSizedBox so a long heredoc can't push other content off-screen; the overflow indicator tells the user content was elided. - Sets MaxSizedBox overflowDirection="bottom" on exec so the head of the command (the action verb + redirection target) stays visible while the tail elides. |
||
|
Shaojin Wen
|
2d222c4206
|
feat(core): foreground → background promote integration (#3831 PR-2 of 3) (#3894)
* feat(core): foreground → background promote integration (#3831 PR-2 of 3) Builds on the \`signal.reason\` foundation merged in #3842 / #3886. Wires the foreground \`shell\` tool to detect a background-promote abort, snapshot the captured output to a \`bg_xxx.output\` file, register a \`BackgroundShellEntry\` in the existing \`BackgroundShellRegistry\`, and return a model-facing \`ToolResult\` pointing at \`/tasks\` / the dialog / \`task_stop\`. Also resolves design question 7 from #3831 (raised by @tanzhenxin in the PR-1 review): set \`result.aborted: false\` when \`result.promoted: true\` so existing \`if (result.aborted)\` consumer branches fall through naturally. ## Changes **\`shellExecutionService.ts\`** — both \`executeWithPty\` and \`childProcessFallback\` background-promote branches now resolve with \`aborted: false, promoted: true\` (was \`aborted: true\`). The flag now answers "should the caller emit a cancel/timeout message?" rather than "did the abort signal fire?" — and a promoted shell is neither cancelled nor timed out (the child is still running, ownership simply transferred). \`ShellExecutionResult.promoted?\` JSDoc updated to document this contract. **\`shell.ts\`** — \`ShellToolInvocation.execute()\` gains a 5th optional parameter \`setPromoteAbortControllerCallback?: (ac: AbortController) => void\`. The foreground path now creates an internal \`promoteAbortController\` and combines its signal into the existing \`signal + timeoutSignal\` AbortSignal.any() chain. Right after \`setPidCallback\` fires, \`setPromoteAbortControllerCallback\` exposes the controller to the scheduler so a UI surface (PR-3 Ctrl+B keybind) can find it by callId and trigger \`abort({ kind: 'background', shellId })\`. When \`result.promoted\` is observed after \`await resultPromise\`, a new \`handlePromotedForeground\` private method: 1. Generates \`bg_xxx\` shellId + on-disk \`outputPath\` under the same project temp dir \`executeBackground\` uses. 2. Writes \`result.output\` (the snapshot the service flushed at promote time) as the file's initial content (best-effort — ENOSPC / EACCES logged + swallowed; the registry entry is valuable on its own). 3. Constructs a \`BackgroundShellEntry\` with the running pid + the same \`promoteAbortController\` already wired into the live child — \`task_stop bg_xxx\` and the dialog's \`x\` key both abort via \`entry.abortController\` and will land on the still-running process. 4. Returns a model-facing \`ToolResult\` pointing at \`/tasks\` / the Background tasks dialog / \`task_stop\` for follow-up. **\`coreToolScheduler.ts\`** — \`TrackedExecutingToolCall\` gains an optional \`promoteAbortController?: AbortController\` field, populated when the shell tool's \`setPromoteAbortControllerCallback\` fires. The scheduler routes only the shell-tool branch to pass this callback, matching the existing \`setPidCallback\` pattern. ## Limitations (deferred to PR-2.5) Two follow-up items intentionally NOT in scope here. Scope discipline keeps PR-2 reviewable while still delivering the user-facing promote flow end-to-end (PR-3's Ctrl+B keybind can wire to this PR's \`promoteAbortController\` to ship a working feature). - **Post-promote stream redirect**: today the \`outputPath\` content is FROZEN at the promote moment. The service detached its data listener as part of PR-1's ownership-transfer contract, so post-promote bytes from the still-running child don't reach the file. \`Read\`-ing the output via \`/tasks\` shows what was captured before promote, not live updates. PR-2.5 will add caller-side \`onPostPromoteData\` callback (or equivalent) so post-promote bytes stream to the file like a normal background shell. - **Natural-exit registry settle**: the registry entry stays \`'running'\` until \`task_stop bg_xxx\` or session-end \`abortAll\` clears it. The service's exit listener was disposed at promote, so there's no observation point for natural child exit. PR-2.5 will keep the exit listener attached post-promote (with a separate \`onPostPromoteSettle\` callback) so the entry transitions to \`completed\` / \`failed\` like a normal background shell. These limitations are visible to users (output frozen, entry stays running until task_stop/session end) but don't break the core promote contract: the agent unblocks, the registry entry is observable, the process stays alive, cancel via \`task_stop\` works. ## Tests **\`shellExecutionService.test.ts\`** — two existing promote tests now assert \`aborted: false\` (per design question 7) instead of \`true\`. \`70 / 70 pass\`. **\`shell.test.ts\`** — three new tests in a \`foreground → background promote (#3831 PR-2)\` describe block: 1. \`setPromoteAbortControllerCallback\` exposes a real \`AbortController\` after spawn. 2. On \`result.promoted: true\`, the registry receives a \`bg_xxx\` entry with pid + abortController + outputPath, the snapshot is written via \`fs.writeFileSync\`, and the model-facing copy references \`/tasks\` + \`task_stop\` + the dialog. 3. A snapshot-write failure (mocked ENOSPC) doesn't break promote — the registry entry still gets registered with the running pid. \`96 / 96 pass\`. **\`coreToolScheduler.test.ts\`** — \`98 / 98 pass\` (no new tests; the new \`promoteAbortController\` field is exercised end-to-end via shell.test.ts). Total: \`264 / 264 affected tests pass\`; tsc + ESLint clean. ## Related - #3831 (Phase D part b — design + 3-PR sequencing; question 7 resolved here) - #3842 (PR-1 — \`signal.reason\` foundation) - #3886 (PR-1 follow-up — Proxy-trap fix + handoff test parity) - #3634 (Background task management roadmap) cc @tanzhenxin * fix(core): give promoted shell entry a FRESH AbortController so task_stop kills the child Real bug found in self-audit of #3894 PR-2: \`entry.abortController\` was being set to the same \`promoteAbortController\` that triggered the promote — which is **already aborted** by the time we reach \`handlePromotedForeground\`. Two consequences: 1. \`task_stop bg_xxx\` calls \`entry.abortController.abort()\`. On an already-aborted controller this is a no-op (the abort event was dispatched once when the controller fired; the second \`abort()\` doesn't re-fire listeners per WHATWG spec). 2. \`ShellExecutionService\` has already detached its own abort listener as part of the PR-1 ownership-transfer contract, so even if the abort COULD re-fire, there's nobody left listening to translate the signal into an actual SIGTERM/SIGKILL on the still- running child. Net effect: a promoted shell would survive \`task_stop\` forever — the agent would think it cancelled, the registry entry would stay \`'running'\`, and the OS process would keep running until the user killed the CLI session. Fix: \`handlePromotedForeground\` now creates a fresh \`AbortController\` for the registry entry and wires its abort listener to: 1. Send SIGTERM → SIGKILL to the still-running child via \`process.kill(-pid, …)\` (Linux/Mac process group, mirroring the \`detached: !isWindows\` spawn the foreground path uses) or \`taskkill /pid /f /t\` (Windows). Reuses the same SIGTERM-then- timeout-then-SIGKILL pattern \`ShellExecutionService.execute()\` uses on the non-promote cancel path; new constant \`PROMOTE_CANCEL_SIGKILL_TIMEOUT_MS = 200ms\` (intentionally separate from the service's \`SIGKILL_TIMEOUT_MS\` so tuning one doesn't silently change the other). 2. Sync-mark the registry entry \`cancelled\` via \`registry.cancel()\` so \`/tasks\` and the dialog reflect the user intent immediately. Added a regression test pinning \`entry.abortController.signal.aborted === false\` at registration time. Without the fix, this asserts \`true\` and the test fails — which is the visible canary for the silent-task_stop-failure mode. 97 / 97 shell.test.ts pass; tsc + ESLint clean. * fix(core): add 'error' listener on Windows taskkill spawn (audit follow-up) Reverse-audit found a Windows-specific crash mode: \`cpSpawn('taskkill', …)\` returns a \`ChildProcess\` whose 'error' event (emitted when the spawn fails — taskkill binary missing, EACCES, etc.) crashes Node by default if no 'error' listener is attached. Same pattern as PR-1's \`@lydell/node-pty\` IPty incident — Web/Node spec quirk easy to miss without specifically thinking about Windows + spawn-failure. Also wrapped the \`cpSpawn\` call itself in try/catch for the rarer sync-throw mode (EMFILE / ENOMEM at spawn-time). Recovery in both cases: log via debugLogger.warn + continue; \`registry.cancel\` below still transitions the entry, and the still-running child becomes an orphan that Windows reaps when the CLI session ends. 97 / 97 shell.test.ts pass; tsc + ESLint clean. * test(core): close 3 test gaps from #3894 review Three [Suggestion] threads from the @tanzhenxin-style review on PR-2, all real test gaps that would have let silent regressions through: 1. **\`setPromoteAbortControllerCallback\` test was too weak.** The old test only asserted that the callback received an \`AbortController\` instance, not that the controller's signal was actually wired into the \`AbortSignal.any(...)\` chain handed to ShellExecutionService. If \`shell.ts\` exposed the controller but forgot to combine its signal, Ctrl+B promotion would never reach the service while the bare-instance test still passed. Strengthened: capture the AbortSignal handed to ShellExecutionService.execute (4th arg), abort the promote controller, and assert the captured signal goes from \`aborted: false\` → \`true\`. 2. **The post-promote cancellation kill path was unverified.** The prior commit added a real-bug fix (fresh \`entryAc\` + abort listener that sends SIGTERM/SIGKILL + sync-marks the registry entry cancelled) but the only test it had was "the controller is fresh, signal not aborted". Reviewer rightly noted that this is the **core operational guarantee** for promoted shells — \`task_stop bg_xxx\` must actually stop the child. Added a test that uses fake timers + a \`process.kill\` spy: register a promoted entry, abort \`entry.abortController\`, flush microtasks (SIGTERM dispatch), advance fake time past \`PROMOTE_CANCEL_SIGKILL_TIMEOUT_MS\` (SIGKILL dispatch + \`registry.cancel\` mark). Pins the entire kill chain. 3. **Scheduler-side wiring of \`promoteAbortController\` was untested.** PR-3's Ctrl+B keybind looks up the executing tool call by callId and aborts \`tc.promoteAbortController\` — if \`CoreToolScheduler\` stops populating that field, the keybind silently breaks. Added a test in \`coreToolScheduler.test.ts\` that uses a \`TestShellInvocation extends ShellToolInvocation\` (so the scheduler's \`instanceof ShellToolInvocation\` check still routes the call through the shell-specific branch that wires the callback) and asserts that an \`onToolCallsUpdate\` batch emitted during the executing window contains a tool call where \`tc.promoteAbortController\` matches the controller the test exposed. 98 / 98 shell.test.ts pass; 99 / 99 coreToolScheduler.test.ts pass; tsc + ESLint clean. * fix(core): use commandToExecute in promoted entry + try/catch register Resolves 3 #3894 review threads: - **Critical**: `entry.command` and `llmContent` for the promoted foreground shell now use `commandToExecute` (post-co-author-rewrite form) instead of raw `this.params.command`. For `git commit -m` invocations that `addCoAuthorToGitCommit()` rewrote, the registry entry now mirrors what actually ran — matching `executeBackground`'s long-standing convention (line 1234). - Defensive try/catch around `registry.register(entry)`: today the call is internally safe (Map.set + emit), but a future implementation that throws would leave a zombie child detached from service listeners with no kill path. Catch path logs, fires `entryAc.abort()` for best-effort kill via the wired listener, and re-throws so the scheduler surfaces the failure. - Updates the misleading comment (line 748) that claimed the registry entry uses "the same `promoteAbortController`" — actual impl uses a fresh `entryAc` (the audit-fix from the previous push). Tests: - `entry.command` git-commit case pinning post-rewrite form - register-throw rejection + SIGTERM/SIGKILL kill via fake timers - 100/100 shell.test.ts pass; tsc + ESLint clean * fix(core): close 2 #3894 review findings — promote refused-race + mkdir orphan Resolves @tanzhenxin's CHANGES_REQUESTED review on #3894. 1. **Refused-promote race no longer reported as "Command timed out"** The combined-abort signal folds in `signal | timeoutSignal | promoteAbortController.signal`, but the timeout discriminator only excluded the user-cancel signal — not the promote signal. When the user fires Ctrl+B (PR-3's keybind) but the service's race guard refuses promotion (the child terminated a beat earlier), the result lands `aborted: true, promoted: false` and the foreground path falsely reported `Command timed out after 120000ms`. Both the agent and the user would see a timeout that didn't happen. Fix: extend the discriminator to ALSO exclude `promoteAbortController.signal.aborted`. Add a `wasPromoteRefused` branch that surfaces the actual cause: "Command finished before the background-promote request could be honoured (the child had already exited)." Same fix applied to both the llmContent path and the returnDisplay path so the model and the visible UI agree. Latent in PR-2 itself (no in-tree caller fires the promote yet), but PR-3's keybind would expose it on first ship. 2. **Unguarded mkdirSync orphans the promoted child** After `result.promoted: true`, ownership of the still-running child has transferred and the service's kill path is detached. The promote handler creates the snapshot output directory next, but the original `fs.mkdirSync(outputDir, { recursive: true })` had no guard — read- only temp mounts, sandboxed perms, full disk on inode/metadata exhaustion would reject the handler BEFORE the registry's kill listener was wired. The still-running child became an orphan zombie with no kill path until the OS reaped it on session end. Fix: wrap mkdirSync in try/catch (matches the safety pattern around `registry.register`). On failure, log + best-effort kill the child (SIGTERM via process.kill(-pid) on POSIX, taskkill /f /t on Windows with an `error` listener so a spawn failure doesn't crash Node) + re-throw so the scheduler surfaces the failure to the agent. Tests: 2 new regressions in `shell.test.ts`: - `mkdirSync(outputDir) throws → child gets SIGTERM, error re-raised` - `promote-refused race (aborted: true, promoted: false after promote signal) is NOT reported as "Command timed out"` 171/171 shell.test.ts pass; tsc + ESLint clean. |
||
|
qqqys
|
35b9cdb22d
|
feat(session): add /branch to fork the current conversation (#3539)
* feat(session): add /branch to fork the current conversation
Introduces `/branch` (alias `/fork`), mirroring Claude Code's fork-session
command. Writes a new JSONL under a fresh sessionId with every record
stamped `forkedFrom: { sessionId, messageUuid }`, rebuilds `parentUuid`
in write order so the fork is a clean linear descendant, and swaps the
CLI into the new session with a Claude-style two-line announcement plus
a `/resume <oldSessionId>` hint.
Core:
- `SessionService.forkSession(src, new)` performs the copy. Uses
`fs.openSync(path, 'wx', 0o600)` for exclusive create — atomic
existence + open in one syscall, no TOCTOU window. Rejects invalid
sessionId patterns, missing/empty sources, cross-project sources, and
pre-existing targets.
- `ChatRecord.forkedFrom` optional field records per-message lineage.
- `SessionStartSource.Branch` lets hook consumers distinguish fork from
resume.
CLI:
- `branchCommand` guards on `isIdleRef` so mid-stream forks can't tear
the parent chain, and on `sessionExists` so empty sessions can't be
forked.
- `useBranchCommand` orchestrates finalize → fork → load → core swap →
init → UI swap, in that order: anything that can still fail runs
while the UI is still on the parent, so a throw leaves the user safely
on the parent session instead of stranded with a cleared history.
- Branch title is `<name> (Branch)` with `(Branch N)` collision bump
(cap 99, then timestamp fallback). When no name is given it's derived
from the first real user `ChatRecord` (skipping cron/notification
subtypes), falling back to `Branched conversation`.
- `/branch` is added to `SLASH_COMMANDS_SKIP_RECORDING` so the command
itself doesn't bleed into the fork's tail.
Tests cover: command guards; hook ordering; title collision bump;
synthetic-record skip; empty-transcript fallback; core-throws-after-fork
UI-preservation invariant; forkSession disk I/O including invalid ids,
cross-project rejection, already-exists rejection.
🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(session): drop stale `commandType` field from branchCommand
The `commandType: 'local'` field was added referencing the Phase 1
slash-command redesign draft, but the field never made it onto
`SlashCommand` — Phase 1 landed with `supportedModes` / `userInvocable`
instead. After merging main, strict tsc rejects the unknown property
with TS2353 and the CLI package fails to build.
🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(session): roll core back to parent when /branch post-fork init throws
`useBranchCommand` swapped core onto the fork via `config.startNewSession`
before `getGeminiClient().initialize()` resolved. If init rejected, the
catch only surfaced an error item: UI was still on the parent, but
`sessionId` + `ChatRecordingService` were already pointing at the orphan
fork JSONL, so the next user message would silently record into the
fork while appearing to belong to the parent conversation.
Snapshot the parent session's `ResumedSessionData` up front, gate the
rollback on a `coreSwapped` flag, and in the catch run
`startNewSession(oldSessionId, prevSessionData)` + re-`initialize()` so
sessionId, recorder (with the correct parentUuid chain tail), and chat
history all return to the parent. Rollback re-init is best-effort — if
it throws again we log and still surface the original failure, since
sessionId + recorder are the load-bearing invariant.
Regression tests: (1) initialize rejects after swap → two
`startNewSessionConfig` calls (fork then rollback-with-parent-data),
two `initialize` calls, no UI swap, original error surfaced; (2)
rollback's own init also rejects → sessionId still lands on parent,
debug logger warns, original error still surfaced.
Reported by gpt-5.5 via Qwen Code `/review` on #3539.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(session): close /branch transactional swap holes flagged in review
Three related correctness issues in the /branch core+UI swap, all reported
by gpt-5.5 via Qwen Code /review on PR #3539:
1. Snapshot-before-finalize. ChatRecordingService.finalize() appends a
trailing `system/custom_title` record that advances `lastRecordUuid`.
Loading the parent ResumedSessionData snapshot before that ran captured
a stale `lastCompletedUuid`; on rollback the restored recorder would
chain its next record's parentUuid to a record that's no longer the
JSONL tail, orphaning the custom_title from the parent chain. Move the
snapshot to AFTER finalize().
2. Reverse split-brain after UI swap. The catch block was gated solely on
`coreSwapped`, so any failure AFTER the UI commits to the branch
(recordCustomTitle, hook fire, remount, announcement render) would
roll core back to the parent — leaving UI on the branch while the
recorder writes new prompts into the parent JSONL. Track `uiSwapped`
separately and skip the rollback once UI is committed; surface the
failure as an error item without unwinding the swap. Pinned by a new
regression test.
3. Slash dispatcher dropped the handleBranch promise. The `branch` case in
slashCommandProcessor returned `{type: 'handled'}` while handleBranch
was still in flight, so a fast follow-up prompt could interleave with
the swap and be recorded against the wrong session. Await it and tighten
the action type from `=> void` to `=> Promise<void>` (both in
SlashCommandProcessorActions and UIActionsContext) so this cannot
silently regress.
Tests:
vitest packages/cli/src/ui/hooks/useBranchCommand.test.ts 15 ✓
vitest packages/cli/src/ui/hooks/slashCommandProcessor.test.ts 41 ✓
vitest packages/cli/src/ui/commands/branchCommand.test.ts 6 ✓
vitest packages/core/src/services/sessionService.test.ts 32 ✓
tsc --noEmit clean
eslint clean
Co-Authored-By: Qwen-Coder <noreply@alibabacloud.com>
* perf(session): fold /branch (Branch N) collision lookup into one scan
`computeUniqueBranchTitle` was probing each `(Branch N)` candidate via
`SessionService.findSessionsByTitle`, and that helper rescans the
project's chats directory on every call. In dense title spaces /branch
could end up doing the scan up to 99 times in a row before settling on
a free suffix, which was visibly stalling the command.
Add `SessionService.findSessionTitlesByPrefix(prefix)` — one project-
wide scan that uses the cheap tail-read to extract each session's
custom_title, filters to titles starting with the prefix, and applies
the same project-scope filter as `findSessionsByTitle`. Heavy hydration
steps (message count, prompt extraction) are skipped because collision
lookup only needs the title.
`computeUniqueBranchTitle` now does ONE call with prefix
`${trimmed} (Branch`, builds an in-memory Set of taken titles, and
picks the first free `(Branch)` / `(Branch N)` slot. Worst-case disk
work drops from O(N) scans to one.
Tests: new `findSessionTitlesByPrefix` describe in sessionService.test
covers prefix match (case-insensitive), missing chats dir, project
isolation, and files without a custom_title. useBranchCommand.test
gains a perf invariant — even when 4 slots are taken, only ONE
prefix-scan is issued.
Reported by gpt-5.5 via Qwen Code \`/review\` on #3539.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* test(cli): tighten mocks and drop dead assertion in slashCommandProcessor tests
Addresses today's review feedback on #3539 plus two tsc gaps the IDE flagged
in the same file.
1. ChatRecordingService cast (TS2352) — route through `unknown` at the two
`recorder = mockConfig.getChatRecordingService() as { recordSlashCommand }`
sites in SLASH_COMMANDS_SKIP_RECORDING. Insufficient overlap between
`ChatRecordingService | undefined` and the inline mock shape; the existing
single-step cast doesn't compile under strict.
2. SlashCommandProcessorActions mock missing `handleBranch` — this PR added
`handleBranch: (name?: string) => Promise<void>` to the actions surface
(commit
|
||
|
pomelo
|
6ca9ca46da
|
feat(cli): add Idealab as third-party provider (#3955)
Add Idealab (Alibaba internal LLM service) to third-party providers with 4 models: Qwen3.6-Plus-DogFooding (default), DeepSeek V4 Pro/Flash, and Kimi K2.6. All models support thinking and multimodal capabilities. Qwen3.6-Plus-DogFooding is free for Alibaba internal users. Closes #3953 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
pomelo
|
7a17c874da
|
chore: Add bilingual requirement to create-issue command (#3952)
Some checks are pending
Qwen Code CI / Lint (push) Waiting to run
Qwen Code CI / Test (push) Blocked by required conditions
Qwen Code CI / Test-1 (push) Blocked by required conditions
Qwen Code CI / Test-2 (push) Blocked by required conditions
Qwen Code CI / Test-3 (push) Blocked by required conditions
Qwen Code CI / Test-4 (push) Blocked by required conditions
Qwen Code CI / Test-5 (push) Blocked by required conditions
Qwen Code CI / Test-6 (push) Blocked by required conditions
Qwen Code CI / Test-7 (push) Blocked by required conditions
Qwen Code CI / Test-8 (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
SDK Python / SDK Python (3.10) (push) Waiting to run
SDK Python / SDK Python (3.11) (push) Waiting to run
SDK Python / SDK Python (3.12) (push) Waiting to run
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Issue body must include both English (top) and Chinese (bottom in a collapsible <details> tag). Title remains English only. |
||
dreamWB
|
e540ec74bb
|
fix(vscode): mark Qwen OAuth coder-model as Discontinued in model picker (#3948)
* fix(vscode): mark Qwen OAuth coder-model as Discontinued in model picker Mirror CLI ModelDialog behavior in the VS Code extension's ModelSelector: render the (Discontinued) badge, replace the description with the migration hint, and block click/Enter selection with an inline error. Add defensive validation in SessionMessageHandler.handleSetModel to reject discontinued model ids that bypass the UI. Runtime OAuth snapshots ($runtime|qwen-oauth|...) are intentionally left selectable, matching the CLI rule that already-cached tokens keep working until the server rejects them. Refs: #3745 * fix(vscode): clear discontinued banner on keyboard arrow navigation too The hover handler clears the inline blocked-selection banner when the user mouses to another row, but the ArrowUp / ArrowDown handlers only updated the selected index — so keyboard navigation left the stale banner visible. Mirror the hover behavior in both arrow-key branches and add a regression test that re-arms the banner and verifies ArrowDown and ArrowUp each clear it. Refs: #3745 |
||
|
易良
|
f43af53a20
|
ci(release): keep skip-ci out of release PR titles (#3950) |