Commit graph

5885 commits

Author SHA1 Message Date
秦奇
b8a2cf3df5 feat(ide): add experimental daemon webview path 2026-05-18 13:10:50 +08:00
秦奇
e9ce860909 feat(ide): add daemon smoke wire-up 2026-05-18 11:07:21 +08:00
ChiGao
4ab20ff6b8
feat(ide): add daemon connection spike (#4199)
* docs(ide): draft daemon adapter plan

* feat(ide): add daemon connection spike

* fix(ide): harden daemon connection lifecycle

* fix(ide): harden daemon permission replay

* fix(ide): harden daemon connection lifecycle

* fix(ide): harden daemon connection adapter

* fix(ide): harden daemon permission routing

* fix(ide): tighten daemon adapter review gaps

---------

Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>
2026-05-18 10:38:26 +08:00
ChiGao
11ba3856df
feat(channel): add daemon bridge spike (#4203)
* docs(channel): draft daemon adapter plan

* feat(channel): add daemon bridge spike

* fix(channel): harden daemon bridge session lifecycle

* fix(channel): handle daemon terminal frames

* fix(channel): harden daemon bridge lifecycle

* fix(channel): harden daemon bridge isolation

* fix(channel): harden daemon bridge cancellation

* fix(channels): close daemon bridge review nits

---------

Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>
2026-05-18 10:21:22 +08:00
tanzhenxin
d3cbe7614e
fix(core): preserve read-before-write state across idle microcompaction (#4243)
* fix(core): preserve read-before-write state across idle microcompaction (#4239)

Idle microcompaction blanked old tool outputs and then wiped the entire
file-read tracking cache to keep the read_file "unchanged" fast path from
pointing at content no longer in history. That wipe also destroyed the
read-before-write record, so after any idle break the model was falsely
told a file it had already read "has not been read in this session" and
forced into a redundant re-read of an unchanged file before it could
edit it.

Replace the blanket wipe with per-file targeting: when microcompaction
blanks a read/edit/write result, only that file's fast-path eligibility
is disarmed; the on-disk fingerprint and "seen this session" markers are
preserved, so edits/overwrites of an unchanged file proceed without a
re-read while a file that actually changed on disk is still correctly
caught. If a blanked result's file path cannot be resolved back to its
cache entry, fall back to the old blanket wipe so a stale "unchanged"
placeholder can never be served.

* refactor(core): harden microcompaction fast-path eviction (PR #4243 review)

- buildCallIdToFilePath now accumulates paths per callId (Map<string,
  string[]>) so a reused/duplicate functionCall.id disarms ALL candidate
  files instead of silently dropping one — over-disarming costs at most a
  redundant re-read, whereas keeping the wrong file armed would resurrect
  the issue #4239 dangling-placeholder hazard.
- Add a guard-rail comment tying COMPACTABLE_TOOLS to FILE_PATH_TOOLS.
- Document why readResidentInHistory is intentionally not reset on the
  fingerprint-drift branch.
- Add tests: duplicate-id disarm, and a mixed on-disk/ghost evicted batch
  forcing the safe blanket wipe.
2026-05-18 10:05:44 +08:00
jinye
f44ed09412
feat(serve): preflight and env diagnostics routes (#4175 Wave 3 PR 13) (#4251)
* feat(serve): introduce ServeErrorKind and BridgeTimeoutError (#4175 Wave 3 PR 13)

Lay the type foundation for `/workspace/preflight` and `/workspace/env` (and
the eventual MCP guardrails route) so cells emitted by all three share a
closed `errorKind` taxonomy:

- `SERVE_ERROR_KINDS` literal-list + `ServeErrorKind` union — the seven
  values from #4175 (`missing_binary`, `blocked_egress`, `auth_env_error`,
  `init_timeout`, `protocol_error`, `missing_file`, `parse_error`).
- `BridgeTimeoutError` typed class — `withTimeout` now rejects with this
  rather than a plain `Error`, letting `mapDomainErrorToErrorKind` recognize
  init / heartbeat / extMethod timeouts via `instanceof` instead of
  regex-matching message strings. Message format is preserved bit-for-bit.
- `mapDomainErrorToErrorKind` helper — one place to classify
  `BridgeTimeoutError`, `SkillError`, fs ENOENT/EACCES/EPERM, ModelConfigError
  subclasses (recognized by `name` field — they aren't on the public surface
  of `@qwen-code/qwen-code-core`), `SyntaxError`, plus message-regex fallbacks
  for legacy throw sites (`agent channel closed`, missing CLI entry path).
- `ServeStatusCell.errorKind` tightened from open `string` to the closed
  `ServeErrorKind` union. Backward compatible — PR 12 never assigned the
  field.
- SDK mirrors: `DAEMON_ERROR_KINDS` const + `DaemonErrorKind` type;
  `DaemonStatusCell.errorKind` tightened.

Tests: 11 new unit tests in `status.test.ts` covering each mapping rule plus
the BridgeTimeoutError shape.

No route changes; no behavior changes for any existing path.

* feat(serve): add buildEnvStatusFromProcess helper (#4175 Wave 3 PR 13)

Pure helper that constructs the `/workspace/env` payload from `process.*`
state. No I/O, no ACP roundtrip, no globals beyond `process.env`. The route
itself lands in the next commit.

- `ServeEnvKind` discriminant: `runtime | platform | sandbox | proxy | env_var`
- `ServeEnvCell extends ServeStatusCell` with `name` + optional `present` /
  `value`. Cells with `kind: 'env_var'` are presence-only — `value` is
  ALWAYS omitted to keep secret env vars off the wire even by accident.
- `ServeWorkspaceEnvStatus` envelope: `{ v, workspaceCwd, initialized: true,
  acpChannelLive, cells, errors? }`. `initialized` is structurally `true`
  because env answers from the daemon process directly; `acpChannelLive`
  reports whether a child is up but does not change the payload shape.

Whitelist policy:
- Auth/secret keys (presence-only): OPENAI/ANTHROPIC/GEMINI/GOOGLE/DASHSCOPE/
  OPENROUTER `_API_KEY`, `QWEN_SERVER_TOKEN`.
- Non-secret keys (also presence-only for shape uniformity): base URLs, locale,
  TZ, NODE_EXTRA_CA_CERTS, QWEN_CLI_ENTRY.
- Proxy vars (`HTTP_PROXY`/`HTTPS_PROXY`/`NO_PROXY`/`ALL_PROXY` + lowercase
  variants): credentials stripped via `redactProxyCredentials`, then
  `URL().host` so the wire only carries `host:port`. NO_PROXY is a host list
  rather than a URL so we pass the redacted form verbatim.

SDK mirrors: `DaemonEnvKind`, `DaemonEnvCell`, `DaemonWorkspaceEnvStatus`.

Tests: 9 unit tests covering the proxy-credential redaction, lowercase env
fallback, NO_PROXY pass-through, presence-only `env_var` invariant
(`'value' in cell === false`), whitelist enforcement, runtime tag detection,
and envelope shape.

* feat(serve): add GET /workspace/env route (#4175 Wave 3 PR 13)

Wire `buildEnvStatusFromProcess` from the previous commit through the
bridge, server, and SDK so remote clients can pre-flight the daemon's
runtime environment without spawning an ACP child.

- `workspace_env` capability tag (always advertised on a current daemon).
- `bridge.getWorkspaceEnvStatus()` answers entirely from `process.*` —
  the route never consults ACP. `acpChannelLive` reflects whether a child
  exists but does not change the payload, so an idle daemon and a busy
  one return the same env shape.
- `app.get('/workspace/env', ...)` mirrors PR 12's one-liner pattern.
- SDK: `DaemonClient.workspaceEnv()` returning `DaemonWorkspaceEnvStatus`.
- Docs: bullet in `docs/users/qwen-serve.md` calling out the
  presence-only redaction policy and the no-ACP-spawn guarantee.

Tests: server-level (env returned + `'value' in env_var === false`
assertion), bridge-level (idle and live both answer locally without
hitting ACP extMethod), SDK-level (recording-fetch round-trip on
`/workspace/env`). The `workspace_env` tag is added to the
`EXPECTED_STAGE1_FEATURES` capability list assertion.

* feat(serve): add /workspace/preflight daemon-cells path (#4175 Wave 3 PR 13)

Wire the preflight route. Daemon-level cells are populated unconditionally
from `process.*` and `node:fs`; ACP-level cells fall back to `not_started`
placeholders when no child is alive so a poll never spawns one.

- `workspace_preflight` capability tag.
- `ServePreflightKind` discriminant (12 values: node_version, cli_entry,
  workspace_dir, ripgrep, git, npm — daemon-level — plus auth, mcp_discovery,
  skills, providers, tool_registry, egress — ACP-level).
- `ServePreflightCell extends ServeStatusCell` with `locality: 'daemon' | 'acp'`
  + free-form `detail`. `ServeWorkspacePreflightStatus` envelope.
- `createIdleAcpPreflightCells()` factory: emits the six ACP-level cells with
  `status: 'not_started'` + a uniform `hint` so the bridge can stitch them in
  alongside daemon cells without ever calling ACP.
- `bridge.getWorkspacePreflightStatus()`:
  - Daemon cells via `buildDaemonPreflightCells` (Promise.all over Node-version,
    CLI-entry resolution mirroring `defaultSpawnChannelFactory`, `fs.stat` on
    `boundWorkspace` with ENOENT/EACCES/EPERM mapped to `missing_file`,
    best-effort `canUseRipgrep` / `getGitVersion` / `getNpmVersion` warnings).
  - ACP cells via `requestWorkspaceStatus` — idle factory returns the
    `not_started` placeholders; live path delegates to ACP via the
    `qwen/status/workspace/preflight` ext method (handler lands in next
    commit). Bridge-side timeout / channel-close while consulting ACP folds
    into envelope `errors[]` with `mapDomainErrorToErrorKind` classification;
    daemon cells still render.
- `app.get('/workspace/preflight', ...)` route + JSDoc bullet.
- SDK: `DaemonPreflightKind` / `DaemonPreflightCell` / `DaemonWorkspacePreflightStatus`
  mirrors; `DaemonClient.workspacePreflight()`.

Tests: server-level (route returns the bridge payload), bridge-level (idle
returns 6 daemon + 6 ACP `not_started` cells without spawning a channel),
SDK-level (`workspacePreflight()` round-trip). Capability test updated.

* feat(serve): wire ACP-side preflight cells (#4175 Wave 3 PR 13)

Populate the six ACP-level preflight cells inside the ACP child so
`/workspace/preflight` returns real values for live sessions.

- `extMethod(qwen/status/workspace/preflight, ...)` dispatches to a new
  `buildAcpPreflightCells(config)` private method.
- Five cell builders, each returning a `ServePreflightCell` with
  `locality: 'acp'`:
  - `auth`: `validateAuthMethod(authType, config)` returning non-null
    string → `auth_env_error`. Missing auth method → warning. Throws
    classified via `mapDomainErrorToErrorKind` with `auth_env_error`
    fallback.
  - `mcp_discovery`: rolls up `getMCPDiscoveryState()` + per-server
    `getMCPServerStatus(name)` counts. `connecting > 0` or in-progress
    discovery → warning + `init_timeout`; `disconnected > 0` post-discovery
    → error + `protocol_error`.
  - `skills`: `SkillManager.listSkills()`; SkillError throws are mapped
    via the helper (`PARSE_ERROR` → `parse_error`, `FILE_ERROR` →
    `missing_file`).
  - `providers`: `getAllConfiguredModels()`; empty list with a configured
    `authType` → warning + `auth_env_error`. ModelConfigError throws map
    to `auth_env_error`.
  - `tool_registry`: null registry → error + `protocol_error`. Otherwise
    surfaces tool count.
- `egress`: stays `not_started`. PR 14 plugs in the real probe.
- `errorCell` private helper extended with optional `errorKind` parameter;
  defaults to `mapDomainErrorToErrorKind(error)` so existing call sites
  (`mcp` / `skills` / `providers` envelope errors) automatically gain
  classification.

Tests: 2 new acpAgent tests — preflight returns the six expected ACP cells
with correct locality + statuses; preflight surfaces a `SkillError`
(`PARSE_ERROR`) on the `skills` cell as `errorKind: 'parse_error'`. The
core `vi.mock` block adds a SkillError class for `instanceof` matching
inside `mapDomainErrorToErrorKind`.

* docs(serve): preflight and env protocol section (#4175 Wave 3 PR 13)

Document `/workspace/env` and `/workspace/preflight` end-to-end:

- Common-cell shape: tighten `errorKind` from open `string` to the closed
  `DaemonErrorKind` enum (seven literals from #4175). Add an explicit
  redaction-policy paragraph covering env-var presence-only, proxy
  host:port reduction, and the whitelisted-secrets list.
- Capability-tag list: add `workspace_env` and `workspace_preflight`.
- New `### GET /workspace/env` section with sample payload, `DaemonEnvKind`
  / `DaemonEnvCell` types, and the redaction-policy paragraph spelling
  out which secret env vars are enumerated and how proxy URLs are
  reduced to `host:port`.
- New `### GET /workspace/preflight` section with idle sample payload,
  `DaemonPreflightKind` / `DaemonPreflightCell` types, the seven-value
  `errorKind` semantics table, and the bridge-error fallback contract
  (mid-request ACP channel close → cells drop to `not_started` + envelope
  carries one `errors[]` entry).
- Source-layout table: extend the `status.ts` row to mention the new
  `ServeErrorKind` / `BridgeTimeoutError` / `mapDomainErrorToErrorKind`
  surface; add a new `envSnapshot.ts` row.
2026-05-18 07:29:05 +08:00
qqqys
f84ddd434b
feat(core): fail impossible goals (#4230)
Some checks are pending
Qwen Code CI / Classify PR (push) Waiting to run
Qwen Code CI / Lint (push) Blocked by required conditions
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Blocked by required conditions
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* feat(core): fail impossible goals

* fix(core): refine impossible goal judgement

* fix(core): include goal feedback when continuing

* fix(core): clarify impossible goal terminal state

* fix(core): harden impossible goal feedback

* fix(core): log suppressed impossible verdicts

* fix(goal): address review suggestions

* test(goal): cover impossible parsing suggestions
2026-05-18 00:31:51 +08:00
Shaojin Wen
c93d66cd23
fix(serve): align build and integration test coverage (#4248)
* fix(serve): align test coverage with build inputs

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

* test(serve): address review feedback

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

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-05-18 00:01:47 +08:00
Shaojin Wen
ad23c7ab34
docs: user + design docs for --json-schema structured output (#4051)
* docs: add user + design docs for --json-schema structured output

Follows up #3598 (cli/core feature shipped to main, no docs).

**User doc** `docs/users/features/structured-output.md` — covers
quick-start, schema input forms (inline + `@path`), output shapes per
`--output-format`, parse-time restrictions, retry/failure modes,
privacy redaction, permission gating, MCP shadow-tool handling, and a
worked `jq`-piped pipeline example. Registered under the existing
`features/_meta.ts` so it shows up in the docs sidebar between
"Headless Mode" and "Dual Output".

**Design doc** `docs/design/structured-output/structured-output.md` —
why the synthetic-tool-whose-param-schema-is-the-user-schema approach,
the four-stage parse-time validation pipeline,
`schemaRootAcceptsObject`'s decided-vs-deferred boundaries, main-turn
vs drain-turn parity via `processToolCallBatch`, the structured-
success terminal block, the cross-surface privacy redaction sharing
`STRUCTURED_OUTPUT_REDACTED_ARGS`, subagent context handling
(`forSubAgent`), MCP shadow-tool guard, the compatibility surface,
alternatives considered (and why rejected), and a file-by-file index.

Both docs are English-only — repo convention is English-only for
both `docs/users/features/` (zero zh-CN siblings) and `docs/design/`
(only `customize-banner-area/` has a zh-CN twin). Open to adding
zh-CN translations as a separate PR if there's demand.

* docs(structured-output): address PR review feedback

User doc:
- explicit stdout-vs-stderr contract and `{}`-schema behavior.
- 500 ms shutdown-holdback latency note.
- ReDoS warning for user-supplied `pattern` keywords.
- root `$ref` rejection + `allOf` workaround.
- per-retry token cost note.
- sibling-suppression success vs retry paths split out.
- numeric exit codes (1 / 53 / 130) for every failure mode.
- new "Session resumption" section for --continue / --resume.

Design doc:
- gloss the ToolSearch on-demand-loading reference.
- `not` row: drop the array-indexing-lookalike `[…]`.
- 500 ms holdback is best-effort, not guaranteed.
- redaction rationale extends to validation-failure retries.
- `CORE_TOOLS` phrasing: structured_output is excluded FROM the set;
  skill is in a separate dynamically-discovered category.
- subagent suppression maintainer note (single brittle call path).
- `--bare` parenthetical lists the three retained core tools.
- PR #4001 status (closed 2026-05-11, superseded).

* docs(structured-output): correct empty-schema / holdback / SIGINT claims

Three doc claims were stronger than the actual code behaviour:

- **Empty schema produces `{}`, not `null`.** `turn.ts` normalises
  the tool args via `(fnCall.args || {})` before they land in
  `structuredSubmission`, so a zero-arg call against `{}` is emitted
  as `{}` on stdout. The `?? null` in the adapter is defence-in-depth
  for the strictly-undefined case, which the upstream path doesn't
  produce.

- **Holdback is a cap, not a fixed wait.** The loop guard is
  `Date.now() < deadline && registry.hasUnfinalizedTasks()`, so it
  exits immediately when nothing is in flight. Reword as "capped at
  ~500 ms" with an early-exit note.

- **SIGINT can still flush a captured result.** The holdback loop
  does not poll the abort signal, so a SIGINT after the structured
  call is captured but before `adapter.emitResult` finishes may
  still land on stdout. Treat exit code 130 as the source of truth.

Also addresses the new auto-review summary suggestion about per-turn
schema cost: pull the cost callout up out of the bullet list (so it
covers both retry cost and schema-embedded-every-turn cost), since
the schema-embedding cost isn't retry-specific.

* docs(structured-output): correct stdout/stderr + json-mode envelope claims

Two doc claims didn't match `JsonOutputAdapter.emitResult`:

- **Model prose doesn't go to stderr in text mode.** Only error
  messages and log lines do. Successful runs emit just the
  JSON-stringified payload on stdout; accumulated assistant prose
  is discarded entirely (not mirrored to stderr). Point users at
  `--output-format json` / `stream-json` when they need the prose.

- **`--output-format json` emits a JSON array, not a single
  document with top-level fields.** The adapter calls
  `JSON.stringify(this.messages)` where `messages` is an array of
  message objects. `structured_result` lives on the final
  `type: "result"` element of that array, not at the document
  root, so consumers must read `.[-1].structured_result` rather
  than `.structured_result`.

* docs(structured-output): note schema-itself reaches the provider

The Privacy section so far only described `structured_output` *args*
being redacted from local on-device surfaces (telemetry + chat
recording). The schema body is a separate exposure surface — it
ships as the function declaration's `parameters` block on every
model request, so `enum`, `const`, `default`, `examples`,
`description`, `$comment`, etc. travel to the provider in
cleartext. Users defaulting to "redaction covers everything"
could legitimately leak secrets via schema-literal fields.

Add a callout in the user doc, plus a parallel paragraph in the
design doc explaining why the redaction stops at on-device
surfaces (the model needs the schema to satisfy the tool-call
contract, so provider-side redaction isn't possible).

* docs(structured-output): correct stdout-on-failure / ReDoS example / hooks / --bare deny / typo

Five issues from the latest /qreview pass:

- **stdout-vs-stderr is text-mode only.** In `--output-format json`
  and `stream-json`, the failure result message is emitted on
  stdout (final element of the JSON array, or the terminating
  `result` line on the JSONL stream). Wrappers in those modes must
  switch on `is_error`, not on whether stdout is empty.

- **ReDoS example didn't actually demonstrate the threat.** JSON
  Schema `pattern` only fires on string instances, and tool args
  are always objects, so the bare `{"pattern": "(a+)+b"}` schema
  doesn't constrain anything the model can supply. Move the
  pattern inside a string-typed property.

- **Hooks see raw `tool_input`.** `PreToolUse` / `PostToolUse` /
  `PostToolUseFailure` receive the unredacted args — including
  HTTP hooks that can forward off-device. Call this out
  explicitly so users with audit-style catch-all hooks know to
  filter or add hook-side redaction.

- **`--bare` drops settings-level deny.** Bare mode builds
  `mergedDeny` as `[...(bareMode ? [] : settings.permissions.deny), …]`
  — settings-level denies are skipped while the synthetic tool
  stays registered. Argv-level `--exclude-tools` still applies.
  Document this exception in the user doc and the design doc.

- **`maxSessionTurns` hint typo.** The hint points at "schema is
  unsatisfiable" — the original text inverted the polarity.

* feat(core): PR-2.5 — post-promote stream redirect + natural-exit registry settle

Closes the two limitations PR-2 (#3894) deferred for the Phase D part
(b) Ctrl+B promote flow (#3831):

1. **Post-promote stream redirect**: today the `bg_xxx.output` file
   is frozen at promote time because `ShellExecutionService` detaches
   its data listener as part of PR-1's ownership-transfer contract.
   PR-2.5 wires a caller-side `onPostPromoteData` callback so bytes
   from the still-running child append to the file via an
   `fs.createWriteStream` opened in `handlePromotedForeground`.
2. **Natural-exit registry settle**: today the registry entry stays
   `'running'` until `task_stop` / session-end `abortAll` fires its
   abort listener. PR-2.5 wires `onPostPromoteSettle` so natural
   child exit transitions the entry to `'completed'` / `'failed'`
   with the right exitCode / signal / error message.

## Service (`shellExecutionService.ts`)

- New exported types: `ShellExecuteOptions`, `ShellPostPromoteHandlers`,
  `ShellPostPromoteSettleInfo`.
- `execute()` options bag now accepts `postPromote?: { onData, onSettle }`.
  Threaded through to both `executeWithPty` and `childProcessFallback`.
- PTY's `performBackgroundPromote` (line ~1159): after disposing
  the foreground data + exit + error listeners, RE-ATTACH minimal
  forwarders that call `postPromote.onData` / `postPromote.onSettle`
  when the caller opted in. Backwards compat: when `postPromote` is
  unset the PR-2 detach-everything contract is preserved (the
  re-attach is gated on each callback being defined).
- `childProcessFallback`'s `performBackgroundPromote` (line ~706):
  same pattern — re-attach `stdout.on('data', ...)`, `stderr.on('data',
  ...)`, `child.once('exit', ...)`, `child.once('error', ...)` when
  the caller opted in. `error` listener routes through `onSettle`
  with `error` populated, so spawn-side errors after the foreground
  errorHandler detached don't crash the daemon via the default
  unhandled `'error'` event.
- Both paths wrap caller callbacks in try/catch so a thrown handler
  doesn't crash the child's data loop / unhandled-rejection the
  service.

## Shell tool (`shell.ts`)

- New `PromoteArtifacts` type — slots shared between the foreground
  `execute()` postPromote handlers (which fire on the service side
  as soon as promote happens) and the post-resolve
  `handlePromotedForeground` finalizer (which runs after
  `await resultPromise` returns). The two race; the buffer +
  settle-queue absorb that race so neither chunks nor the eventual
  exit info are lost.
- `executeForeground` wires `postPromote` handlers that route data
  to either `promoteArtifacts.stream` (if open) or
  `promoteArtifacts.buffer` (drained when the stream opens), and
  queue settle info if the wired handler isn't yet installed.
- `handlePromotedForeground` opens `fs.createWriteStream(outputPath,
  { flags: 'w' })`, writes the initial snapshot first, drains the
  buffer, then registers the entry and wires `onSettleWired` with
  the full registry decision table:
    - `error` set → `registry.fail(shellId, error.message, endTime)`
    - `exitCode === 0` → `registry.complete(shellId, 0, endTime)`
    - non-zero exitCode → `registry.fail(shellId, "Exited with code N", endTime)`
    - signal !== null → `registry.fail(shellId, "Terminated by signal N", endTime)`
    - all-null fallback → `registry.fail(shellId, "Exited with unknown status", endTime)`
- Fires queued settle synchronously after wiring so a fast command
  that exits between promote and finalizer doesn't get lost.
- Self-audit catch: closes the output stream on the
  `registry.register` throw path so the FD doesn't leak past the
  orphan-child kill.

## Tests

- 3 new in `shellExecutionService.test.ts`:
  - `post-promote bytes route to postPromote.onData when callback provided`
  - `postPromote.onSettle fires on natural child exit after promote`
  - `backwards compat: without postPromote, listeners stay fully detached`
- 3 new in `shell.test.ts` under a `foreground → background promote
  PR-2.5` describe block:
  - `post-promote bytes APPEND to bg_xxx.output via write stream`
  - `natural child exit transitions registry entry to "completed"`
  - `non-zero exit / signal / error → "failed" with descriptive message`
- Bulk-replaced 50 prior `{},` (empty 6th-arg shellExecutionConfig)
  with `expect.objectContaining({}),` + added `expect.objectContaining({
  postPromote: expect.any(Object) }),` as the 7th-arg expectation for
  the foreground execute call.
- Updated the existing `registers a bg_xxx entry on result.promoted`
  test to assert on `fs.createWriteStream` + `stream.write` instead
  of the now-removed `fs.writeFileSync` snapshot path.

182/182 shell.test.ts pass + 73/73 shellExecutionService.test.ts pass
+ 111/111 coreToolScheduler.test.ts pass + 60/60 AppContainer.test.tsx
pass; tsc + ESLint clean.

Self-audit: 3 rounds (positive / reverse / cross-file) found one
issue — output stream FD leak on `registry.register` throw — and
fixed it before flagging complete. All flagged edge cases (stream
errors, child-exits-before-wire-up race, task_stop during natural-
exit window, promote-never-happens cleanup, backwards compat
without callbacks) have explicit handling and / or test pinning.

* fix(core): #4102 review wave — 3 Critical + UTF-8 + tests

3 Critical race/correctness issues + 1 multibyte-corruption suggestion
+ 3 test coverage gaps addressed:

**Critical 1 — child_process late-chunk drop (service)**
Settle was fired on 'exit', but stdout/stderr can emit buffered data
between 'exit' and 'close'. Late chunks landed in
`promoteArtifacts.buffer` after shell.ts had already closed the
stream + transitioned the registry → silently dropped → truncated
`bg_xxx.output`. Switched to listening on 'close' which guarantees
all stdio is fully drained. (code, signal) payload is identical to
'exit', just with proper ordering.

**Critical 2 — stream-flush wait before registry transition (shell)**
`stream.end()` is asynchronous; pending writes can still be in the
libuv queue when it returns. The old code transitioned the registry
immediately after `.end()`, so a /tasks consumer could observe a
`completed` entry and read the output file BEFORE the trailing
bytes were on disk. Fixed: wired settle now `stream.once('finish',
...)` BEFORE calling `registry.complete/fail`. `error` event also
short-circuits to the transition so a late ENOSPC doesn't hang the
settle path forever.

**Critical 3 — stream-open-fail buffer leak (shell)**
If `fs.createWriteStream` threw, the catch path set `stream = null`
but the foreground `onData` handler would still take the
`stream === null` branch and push chunks into `promoteArtifacts.buffer`
— unbounded growth under a sustained child whose output file
couldn't be opened. Added a `streamFailed: boolean` latch on
`PromoteArtifacts`. When set, `onData` drops chunks (with a debug
log) instead of buffering. The catch branch sets the latch.

**Suggestion — shared TextDecoder corrupts multibyte UTF-8 (service)**
child_process post-promote used ONE TextDecoder for both stdout AND
stderr. The decoder's continuation-byte state machine assumes one
byte source; interleaved multibyte chunks corrupted. Now uses
separate decoders + flushes both with `decode()` (no `stream: true`)
on settle so trailing bytes surface as their final characters.

**Suggestion — llmContent reflects already-settled status (shell)**
When the queued-settle drain transitions the registry synchronously
(fast-exit race), the model-facing copy was still saying "Status:
running. … task_stop({...})". Updated to branch on
`postPromoteAlreadySettled` / `postPromoteFinalStatus` — when the
process is already gone, the copy says "Status: completed/failed"
and replaces the `task_stop` suggestion with "Process has already
exited; no `task_stop` needed".

**Suggestion — test coverage gaps**
Added: (a) `queued-settle race: onSettle BEFORE
handlePromotedForeground completes` — custom service impl fires
onSettle synchronously before resolving the promote promise, pins
the drain path. (b) child_process post-promote tests for stdout/stderr
forwarding + 'close'-not-'exit' settle + spawn-error settle.

**Self-audit**: Round 1 + reverse audit. Stream.once mock added to
fire 'finish' synchronously so existing tests don't hang on the new
flush wait. 76/76 shellExecutionService.test.ts (+3) + 183/183
shell.test.ts (+1) pass; tsc + ESLint clean.

* fix(core): #4102 review wave-2 — 3 more from gpt-5.5

C1 (shell.ts:2227): the WriteStream `'error'` event handler only
logged. `fs.createWriteStream` reports common open failures
(ENOENT / EACCES / ENOSPC) asynchronously via that event rather
than throwing. Result: `promoteArtifacts.stream` kept pointing at
the failed stream; `onSettleWired` attached a `.once('finish')`
listener that would never fire → registry stuck on `running`
forever. Latch the failure (null the shared `stream` slot,
set `streamFailed`); `onSettleWired`'s existing `if (!stream)`
branch then transitions the registry immediately.

C2 (shellExecutionService.ts:1468): the promote handoff removes the
foreground `ptyErrorHandler` and only re-attaches data + exit
listeners. A subsequent PTY `error` event had no listener — Node
treats an unhandled `error` from an EventEmitter as a fatal
exception that takes the whole CLI down. Attach a post-promote
forwarder that ignores expected PTY read-exit codes (EIO / EAGAIN,
same filter the foreground handler uses) and routes unexpected
errors through `postPromote.onSettle` with `error` populated.
Single-fire latch shared with `onExit` so settle never fires twice.

C3 (shell.ts:2503): `onSettleWired` waits for the stream's
asynchronous `'finish'` event before flipping
`postPromoteAlreadySettled`, but the model-facing `statusLine` was
built immediately after invoking `onSettleWired` on the queued
settle. A fast-exited promoted command could therefore land
"Status: running" + a `task_stop` instruction in production even
though settle was already observed. Split into two flags:
`postPromoteSettleObserved` (set synchronously when settle is
classified) drives the model copy; the registry transition stays
behind the stream flush.

Tests: +1 PR-2.5 wave-2 PTY error-routing test; +2 shell.ts tests
(stream open async error → registry still transitions; async
`'finish'` after queued-settle drain → llmContent says 'completed'
before registry transition fires).

* fix(core): #4102 review wave-3 — 4 actionable from deepseek-v4-pro

T2 (shell.ts:2456) — Critical buffer-leak race
`onSettleWired` previously set `promoteArtifacts.stream = null`
BEFORE calling `stream.end()`. Any `postPromote.onData` chunk that
landed between that null assignment and the actual flush completing
saw `stream === null && streamFailed === false` and pushed into
`promoteArtifacts.buffer` — a buffer that has no further drain path
(the foreground finalizer has already returned). Result: chunks
stranded indefinitely; PTY mode in particular hits this because
`onExit` can fire while kernel buffers still hold data. Fix drains
the pre-settle buffer to the stream BEFORE nulling AND latches
`streamFailed = true` so any subsequent chunk drops via the
existing `else if (streamFailed)` arm in `onData` instead of
leaking. Updates the `streamFailed` doc to cover both setters
(open-fail and settle-done) so the dual semantic is explicit.

T3 (shell.ts:2262) — silent chunk-drop in catch path
When `fs.createWriteStream` throws synchronously (rare: ENOENT on
a vanished tmpdir), chunks already in `promoteArtifacts.buffer`
were silently lost with no observability — oncall reading a
truncated `bg_xxx.output` had no way to distinguish "stream open
failed" from "child produced nothing." Logs the dropped chunk
count and empties the buffer.

T5 (shell.ts:2443) — opaque all-null fallback
The "Exited with unknown status" fallback fired the registry to
'failed' without any context about which fields were null. This
branch is meant to be unreachable; hitting it indicates the
service emitted a defective settle info object. Includes the
field values in both the fail message and a warn log so the
oncall engineer can tell this path apart from the other "failed"
branches.

T6 (shellExecutionService.ts:1452) — leaked PTY post-promote listeners
`ptyProcess.onData(...)` returns an `IDisposable` that was being
discarded; same for `onExit`. The `'error'` listener function was
also not captured (no way to `removeListener` it). EventEmitter
holds refs to listener closures, which transitively hold refs to
`onPostData` / `onPostSettle` / the caller's `promoteArtifacts`.
While bounded by the PTY's lifetime, the closures keep the
caller's state pinned for the post-settle delay window. Captures
all three handles into `postPromoteDataDisposable` /
`postPromoteExitDisposable` / `postPromoteErrorListener`, then
releases them via a shared `disposePostPromoteListeners()` call
from `firePostSettle` (idempotent — each slot null-checked and
nulled after disposal).

Tests: +1 service test for IDisposable + error-listener cleanup;
+2 shell.ts tests for buffer drain race and catch-path snapshot
fallback. Existing tests stay green (262 → 265 in the touched
suites; 7819 → 7822 across the core package).

* fix(core/test): drop unused 'registry' in wave-3 T2 test (TS6133)

CI build failed across all platforms with src/tools/shell.test.ts(4395,15): error TS6133. The variable was a leftover from copying the queued-settle test pattern; the wave-3 T2 test inspects writeStreamMock.write call history directly and never reads the registry, so the assignment is dead code. Drop it.

* fix(core): #4102 review wave-4 — 6 actionable from gpt-5.5 + deepseek-v4-pro

T1 (Critical, shellExecutionService.ts:860 child_process onSettle
exactly-once)
The PTY path used a `firePostSettle` latch but child_process wired
`close` and `error` independently to `onPostSettle`. A spawn-side
error followed by Node's auto-emitted `'close'` would call the
caller's settle TWICE, racing the registry transition. Added the
same single-fire latch on the child_process path.

T2 (Critical, shell.ts:2264 handoff race reorder)
Original order was `write(snapshot) -> drain buffer -> assign stream`.
Synchronous today (no race in current code), but assign-after-drain
leaves a hazard for any future refactor that adds an `await` inside
the drain loop — a chunk arriving in that window would land in
`promoteArtifacts.buffer`, then post-assign chunks would write to
the stream first, producing out-of-order bytes until the settle
drain. Reordered to `write(snapshot) -> assign stream -> drain
buffer`, which closes the hazard regardless of future async
additions.

T3 (Suggestion, shellExecutionService.ts:816 decoder flush gated
on onSettle)
The trailing-multibyte flush ran inside the `child.once('close', ...)`
handler, which was only installed when `onSettle` was set. An
`onData`-only caller (no onSettle) lost trailing continuation
bytes silently. Hoisted flush into `flushPostPromoteDecoders`
called from `firePostSettle`, and made `firePostSettle` available
on the `'close'` path independent of onSettle (T6 install).

T4 (Suggestion, shell.ts:1700 promoted ANSI passthrough)
The regular `executeBackground` path strips ANSI before writing to
`bg_xxx.output`; the promoted-foreground onData path appended raw
chunks. Reading `bg_xxx.output` after Ctrl+B showed plain text up
to the snapshot then raw `\x1b[31m` / cursor-move / clear-screen
sequences for the post-promote tail — unreadable. Apply
`stripAnsi(rawChunk)` before write/buffer, matching the
executeBackground contract.

T5 (Suggestion, shellExecutionService.ts:786 UTF-8 hardcoded)
The post-promote child_process decoders were hard-coded to
`new TextDecoder('utf-8')`, but the foreground decoder runs
encoding detection via `getCachedEncodingForBuffer`. On a non-UTF-8
child (e.g. GBK on a Chinese Windows shell), the snapshot decoded
correctly but the post-promote tail was mojibake. Capture the
foreground decoder's `.encoding` property and reuse it for
post-promote (with utf-8 fallback if foreground hadn't seen any
bytes yet, and a try/catch around `new TextDecoder` for the rare
unsupported-encoding case).

T6 (Suggestion, shellExecutionService.ts:1540 `error` listener
gated on onSettle)
The post-promote `error` listener was attached only when `onSettle`
was set. An `onData`-only caller still had the foreground
errorHandler detached; a post-promote spawn error would then crash
the CLI via Node's unhandled-error default. Hoisted the close +
error listeners into `if (postPromote)` so any caller opting into
post-promote gets crash protection; if `onSettle` is absent the
listeners log + drop instead of routing.

T7 (Suggestion, shellExecutionService.ts:791 onSettle-only
pipe-block deadlock)
Same root cause as T6: when only `onSettle` is set, the foreground
`stdout`/`stderr` 'data' listeners are detached and no post-promote
listener replaces them. The Readables stay paused, the OS pipe
buffer fills (~64KB on Linux), the child blocks on `stdout.write`,
'close' never fires, onSettle never fires. Added `child.stdout?.resume()`
and `child.stderr?.resume()` in the no-onData branch so the child
can drain its pipes and reach exit.

T8 (Suggestion, shell.ts:2614 dead inspectLine ternary)
`inspectLine`'s ternary returned the same string on both sides —
copy-paste leftover from when the other two adjacent ternaries
(statusLine / stopLine) were correctly varied. Collapsed to a
single string assignment.

Tests: +5 regression tests (4 child_process: T1 double-fire latch,
T3 onData-only flush, T6 onData-only error survives, T7 onSettle-
only resume; +1 shell.ts: T4 ANSI strip).

265 -> 270 in the touched suites; 7822 -> 7827 across the core
package; full suite green.

* fix(core/test): use ShellOutputEvent type in wave-4 onData callbacks (TS2345)

CI lint failed on the wave-4 (T3 / T6) tests with TS2345: pushing
ShellOutputEvent into Array<{type:string;chunk:unknown}> narrows
incompatibly. Switch to ShellOutputEvent[] (matches earlier helpers
at lines 758/966) and discriminate the union via .type === 'data'
when reading .chunk so the narrowed multibyte assertion still
type-checks.

* docs(structured-output): address doudouOUC's four review findings

- Tighten JSON/stream-json paragraph: not all failures emit a result
  to stdout (exit 53 / exit 130 are stderr-only); check exit code first
- Fix suppressed-sibling retry guidance: re-issue in a separate turn
  that does not include structured_output (avoids re-suppression)
- Distinguish settings-deny (exit 53) from --exclude-tools (exit 1)
  in Permission gating section
- Replace <projectDir> placeholder with actual path
  ~/.qwen/projects/<sanitized-cwd>/chats/<sessionId>.jsonl in both docs

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

* docs(structured-output): fix Permission gating — both deny paths strip registration

Forward audit against source code found that the Permission gating
section incorrectly distinguished settings.permissions.deny (claiming
tool stays visible, exit 53) from --exclude-tools (claiming
declaration stripped, exit 1). Both go through the same mergedDeny →
isToolEnabled path and both prevent registration — the model never
sees the tool. Corrected both docs to reflect the actual mechanism:
typical outcome is plain text (exit 1), with maxSessionTurns (exit 53)
as the fallback if the model loops through other tools.

* docs(structured-output): address doudouOUC's May 17 review (5 items)

- Clarify validation is client-side Ajv, not provider-side
- Qualify "same way" with DeclarativeTool abstraction parenthetical
- Match symptom→cause structure for maxSessionTurns hint
- Expand $ref workaround with concrete $defs example
- Clarify Dual Output See Also doesn't require --json-schema

* docs(structured-output): address 2 unresolved design-doc suggestions

1. Privacy/redaction section: note hooks as intentionally non-redacted
   surface (matches user-doc "Hooks see raw args" callout).
2. Dual call-site section: clarify differing post-helper termination
   flow between main-turn (direct return) and drain-turn (sentinel hop).

* docs(structured-output): address doudouOUC's May 17 review (2 nits)

1. Failure-paths table: align "three common causes" cell with the
   symptom→cause framing already used at parse-time validation pipeline
   section ("common stuck-run symptom and its two likely causes").
2. Dual call-site section: fix factual inaccuracy from prior commit —
   `drainOneItem` is `async (): Promise<void>` and returns nothing.
   The two-hop termination is via closure-mutated `structuredSubmission`
   (set by `processToolCallBatch`, checked by `drainLocalQueue` and the
   holdback loop), not a return-value sentinel.

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-05-17 23:10:34 +08:00
xMillogx
79f38e0701
fix: add cache limits to prevent OOM during build/test (#4188)
* fix: add cache limits to prevent OOM during build/test

* chore: remove intermediate OOM analysis document

* fix(core): enforce MAX_TOTAL_PATHS cap when updating existing crawlCache key

Before: !crawlCache.has(key) guard in the MAX_TOTAL_PATHS eviction loop
short-circuited eviction when updating an existing key, allowing cache to
grow beyond 50,000 paths (F1 bug - OOM protection bypassed).

After: totalPaths is calculated excluding the current key, and the eviction
loop protects the key being written from being evicted as "largest".
FIFO bump (delete+set) ensures frequently updated keys move to end of queue.

Per @wenshao review: MAX_CACHE_ENTRIES guard on line 59 is preserved
(updating existing key doesn't increase entry count).

* test(core): add cache eviction tests and fix MAX_TOTAL_PATHS loop guard

* test(core): add eviction coverage for fileReadCache and crawlCache

* test(core): unskip bumped entries eviction test now that upsert bump is implemented

---------

Co-authored-by: Юрий Острожных <millog@mail.ru>
2026-05-17 23:01:23 +08:00
jinye
672de88a47
fix(serve): align integration test mirrors with merged capability + EventBus changes (#4245)
- qwen-serve-routes.test.ts: expand expected features list to 24, adding
  slow_client_warning (#4237) and workspace_mcp/workspace_skills/
  workspace_providers/session_context/session_supported_commands (#4241).
  Matches EXPECTED_STAGE1_FEATURES in server.test.ts:76-101.
- qwen-serve-baseline.test.ts: update SSE backpressure assertion from 3
  to 4 frames (tick, tick, slow_client_warning, client_evicted). PR #4237
  changed EventBus to force-push a slow_client_warning synthetic frame
  when the per-subscriber queue reaches the 75% warn threshold, before
  the client_evicted terminal frame fires on overflow. Mirrors the unit
  test at eventBus.test.ts:103-122.

Both integration mirrors drifted because integration tests only run on
schedule / workflow_dispatch (release.yml:4-9), not PR CI. Fixes the
release run 25992130532 failure in both Docker and No-Sandbox jobs.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
2026-05-17 22:53:11 +08:00
jinye
60fe594e8f
feat(serve): add read-only status routes (#4241)
* feat(serve): add read-only status routes

Add read-only daemon status endpoints for workspace MCP, skills, providers, session context, and session supported commands.

Expose matching typed SDK helpers and document the new additive v1 status surface.

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

* fix(serve): harden read-only status snapshots

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

* fix(serve): address read-only status review feedback

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

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-05-17 21:37:20 +08:00
jinye
aef35c390e
feat(serve): session metadata and close/delete lifecycle (#4175 Wave 2.5 PR 11) (#4240)
* feat(serve): session metadata and close/delete lifecycle (#4175 Wave 2.5 PR 11)

Add explicit session close and metadata management to the daemon serve
infrastructure, closing the Stage 1 limitation that sessions could only
end via child crash or daemon shutdown.

- DELETE /session/:id — force-closes a live session (cancels active
  prompt, resolves pending permissions, publishes session_closed event)
- PATCH /session/:id/metadata — update mutable displayName
- Enriched GET /workspace/:id/sessions with createdAt, displayName,
  clientCount, hasActivePrompt
- session_closed + session_metadata_updated SDK event types with
  validation, reducer, and terminal event priority
- DaemonClient.closeSession / updateSessionMetadata + session client
  wrappers
- Capabilities: session_close, session_metadata

* fix(serve): address review feedback on session lifecycle PR

- Fix JSDoc on closeSession: clarify that bridge throws SessionNotFoundError
  (SDK absorbs 404 for client-side idempotency)
- Tighten event validators: isSessionClosedData checks closedBy type,
  isSessionMetadataUpdatedData checks displayName type
- PATCH /session/:id/metadata now returns effective stored metadata
  instead of echoing request fields, avoiding ambiguous no-op responses
- Only publish session_metadata_updated event when displayName changes
- Update chooseTerminalEvent comment to reflect session_closed

* fix: address PR 4240 review feedback

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

* fix: address remaining PR 4240 suggestions

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

* fix: update serve sessions test mock

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

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-05-17 20:42:15 +08:00
JerryLee
9abd704e09
fix(cli): record mid-turn queued user prompts (#4215) 2026-05-17 20:21:06 +08:00
jinye
4e06967c2b
feat(serve): mutation gating helper and --require-auth (#4236)
Some checks are pending
Qwen Code CI / Classify PR (push) Waiting to run
Qwen Code CI / Lint (push) Blocked by required conditions
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Blocked by required conditions
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* feat(serve): mutation gating helper and --require-auth

Implements issue #4175 Wave 4 PR 15. Adds the centralized
state-changing-route gate that Wave 4 follow-ups (memory CRUD, file
edit, MCP restart, device-flow auth) will reuse, plus the
`--require-auth` deployment knob that hardens the loopback developer
default for shared dev hosts / CI runners.

- `createMutationGate({ tokenConfigured, requireAuth })` factory in
  serve/auth.ts — per-route middleware with a 4-cell behavior matrix:
  pass-through under `requireAuth` or any token configured;
  `401 token_required` for `strict: true` routes on no-token loopback
  defaults; baseline pass-through otherwise.
- Existing Wave 1-2 mutation routes (POST /session, /session/:id/{load,
  resume,prompt,cancel,model}, /permission/:requestId) opt into the
  default non-strict factory call as the centralization marker. Wave 4
  routes will pass `{ strict: true }` to require a token even on
  loopback.
- `--require-auth` CLI flag + `ServeOptions.requireAuth`. Boot refuses
  without a token; closes the `/health` exemption when on so loopback
  `/health` also requires bearer auth; stderr breadcrumb so the
  hardened mode is visible in journald/docker logs.
- Conditional `require_auth` capability tag advertised only when the
  flag is on. New `CONDITIONAL_SERVE_FEATURES` registry primitive so
  future per-deployment toggles follow the same shape.
- 5 new unit tests in auth.test.ts covering the gate matrix; 5 added
  in server.test.ts for capability advertisement, conditional tag,
  /health 401 under --require-auth, and runQwenServe boot
  refusal + happy path. 245/245 serve tests pass; typecheck + eslint
  clean.

Refs: #4175

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

* fixup(serve): address PR #4236 review feedback

Three small follow-ups from the automated reviewers on PR #4236:

1. **Drop misleading `--require-auth` from `token_required` error
   message** (Copilot inline auth.ts:262). The strict-mode 401
   listed three remediations but `--require-auth` is paired-required
   with a token at boot — naming it standalone would loop the operator
   into a different boot error. Keep the two valid standalone fixes
   (env var, --token); add inline note explaining the omission.
   `auth.test.ts` regex updated to `not.toMatch(/--require-auth/)`
   to anchor the new wording.

2. **Mention `/health` gating in `--require-auth` CLI description**
   (auto-reviewer Medium #2). Operators flipping the flag without
   reading the protocol doc would get paged when k8s/Compose probes
   start 401-ing. One sentence in the yargs description prevents that.

3. **Drift insurance comment between registry and
   `CONDITIONAL_SERVE_FEATURES`** (auto-reviewer Low #3). Document
   the four-step procedure for adding a new conditional tag so a
   future contributor doesn't update only the registry and silently
   advertise the tag unconditionally. Notes the Map<predicate>
   refactor as the right move when a second tag lands.

Deferred (not in this fix-up):
- Module-level PASSTHROUGH singleton (High #1) — micro-optimization,
  unmeasurable.
- Map<feature, predicate> for conditional features (High #2) —
  premature abstraction with one tag.
- Per-route `// non-strict marker` comments (Medium #1) — noise.
- `@see` cross-ref in types.ts (Low #2) — sugar.
- JSDoc bullet-list vs table (Low #1) — current format is fine.

Refs: #4175 #4236

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

* fixup(serve): address PR #4236 round-2 review feedback

Five small follow-ups from @wenshao + DeepSeek (via Qwen Code /review)
on PR #4236:

1. **Map<predicate> refactor for `CONDITIONAL_SERVE_FEATURES`**
   (review threads #3254467192 + #3254485912). Two reviewers asked
   for the same shape on the grounds that the `Set` + per-feature
   `if`-branch needed FOUR coordinated changes per new conditional
   tag and silently fail-CLOSED when the branch was missed. The Map
   collapses the predicate-decision and the set-membership into one
   entry per feature — adding a new conditional tag is now two
   coordinated changes (registry + Map entry) and a missing predicate
   is a TypeScript error rather than a silent omission. JSDoc
   updated.

2. **Drift-insurance test that iterates `CONDITIONAL_SERVE_FEATURES`**
   (review thread #3254467192 option 1, layered on top of #1).
   `server.test.ts` now walks every Map entry and asserts the
   predicate accepts/rejects as expected; future entries that don't
   add an assertion branch fail the test loudly so a missing
   predicate cannot ship silently. Adoption-of-record for the Map
   shape rather than relying on a hand-maintained invariant.

3. **Cache `strictDenier` for allocation symmetry** (review thread
   #3254467193). Wave 4 PRs will mount strict mode on multiple
   routes; without the cache each `mutate({strict:true})` call would
   allocate a fresh 401 closure. Now both the passthrough and the
   strict denier are pre-built singletons. Identity assertion in
   `auth.test.ts` anchors the cache so a future change that loses it
   surfaces in CI.

4. **Doc cosmetic — extra blank line in qwen-serve.md** (review
   thread #3254467198). Single blank line between the `>` quoted
   example and the following non-quoted bash block now.

5. **Doc correctness — `require_auth` is post-auth confirmation**
   (review thread #3254485910 from DeepSeek). When `--require-auth`
   is on, the global `bearerAuth` middleware gates every route
   including `/capabilities`, so an unauthenticated client cannot
   pre-flight `caps.features` to discover that auth is required —
   the discovery surface is the 401 response body itself. Both
   `qwen-serve.md` and `qwen-serve-protocol.md` rewritten to
   describe the tag as a post-authentication confirmation, matching
   the auth.ts JSDoc which already stated this correctly.

Trade-offs documented (no code change):

- **Body-parser ordering** (review thread #3254485915 from DeepSeek)
  noted as a comment block in `auth.ts`. Strict-mode 401 fires AFTER
  `express.json()` because the gate is per-route middleware. On
  loopback no-token defaults a strict route therefore parses the
  request body before refusing it — bounded by
  `express.json({limit: '10mb'})` × `--max-connections` (256
  default). Strict routes Wave 4 actually adds carry small bodies in
  legitimate use, so this isn't a production hot path. Future routes
  accepting large bodies should lift the gate to app-level (maintain
  a strict-path Set in `createServeApp`); flagged as a Wave 4
  follow-up rather than re-architecting the helper.

- **`bearerAuth` body-shape inconsistency** (review thread
  #3254467197 from @wenshao) flagged as a Wave 4 cross-PR
  follow-up. `bearerAuth` returns `{error: 'Unauthorized'}` while
  the strict gate returns `{code: 'token_required', error: '...'}`;
  SDK clients have to branch on both shapes. Standardizing
  `bearerAuth` to also carry a `code` field is orthogonal to this
  PR's scope.

Validation: 260/260 cli serve tests pass (was 258 — added the drift
insurance test + strict denier identity test); typecheck + eslint
clean.

Refs: #4175 #4236

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

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-05-17 20:10:37 +08:00
易良
eef06ce376
feat(cli): add structured memory diagnostics JSON (#3785)
* feat(cli): add memory diagnostics doctor command

* fix(core): platform-aware maxRSS conversion and accurate risk message

- Extract platform detection before building diagnostics so the correct
  unit conversion can be applied: multiply by 1024 on Linux (where
  process.resourceUsage().maxRSS is in KB) but leave the value unchanged
  on macOS/Windows (where it is already in bytes).
- Correct the native-memory-pressure risk message to accurately state
  that the threshold is 2× heap used, not just "larger than heapUsed".
- Add a dedicated test to assert that maxRSS is not multiplied on a
  non-Linux platform (darwin).

All 3 core and 9 CLI tests pass; typecheck clean.

Agent-Logs-Url: https://github.com/QwenLM/qwen-code/sessions/9b413337-68ed-4d5c-af99-0d42378900c3

* test(core): cover active request memory risk

* fix(cli): address memory diagnostics review feedback

* fix(cli): harden memory diagnostics review fixes

* fix(memory-diagnostics): tighten risk thresholds and expand readable output

- Add 64MB absolute floor on native-memory-pressure so cold processes don't trip
  the 2x ratio check; raise active-handles threshold from 100 to 256
- Show detachedContexts, nativeContexts, maxRSS, CPU times, smapsRollup
  availability, and v8HeapSpaces summary in the readable /doctor memory output
- Validate unknown memory subcommand args with a usage hint instead of silently
  dropping them
- Wrap human-readable strings in t(...) for i18n parity with the rest of doctor
- Advertise the memory subcommand via /doctor argumentHint while keeping
  acceptsInput false so the parent still auto-submits
- Document _getActiveHandles/_getActiveRequests as undocumented Node internals
- Update tests for new thresholds, expanded output, unknown-arg path, and
  abort-during-json

* fix(cli): harden memory doctor diagnostics

* fix(core): correct maxRSS byte handling and heapRatio consistency

- Remove incorrect * 1024 multiplier for maxRSS on Linux (Node.js >=14.10 returns bytes on all platforms)
- Use v8HeapStats.usedHeapSize for heapRatio to avoid cross-API inconsistency
- Update test expectations and rename "does not multiply" test

* fix(cli): resolve rebase conflicts in memory diagnostics

- Rename local formatMemoryDiagnostics to formatCoreDiagnostics to avoid
  naming conflict with the imported utility from memoryDiagnostics.js
- Update Session.test.ts to use objectContaining for _meta field added
  in recent main commits
- Align doctorCommand.test.ts assertions with current parent command
  state (argumentHint includes --sample/--snapshot from main)

* fix(core): use null instead of undefined for optional probes, deduplicate active count helpers

- optionalProbe/optionalSyncProbe now return null on failure so
  JSON.stringify preserves the keys instead of silently omitting them.
- Merge getActiveHandlesCount/getActiveRequestsCount into a single
  parameterized getProcessInternalCount helper.
- Update MemoryDiagnostics interface: v8HeapSpaces, openFileDescriptors,
  smapsRollup are now T | null instead of T | undefined.

* fix(cli): finish memory diagnostics review fixes

* fix(cli): address memory diagnostics review feedback

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
2026-05-17 19:52:46 +08:00
Yan Shen
9985d91e08
feat(cli): add configurable plansDirectory for Plan Mode (#4062)
* feat(cli): add configurable plansDirectory for Plan Mode

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

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

Closes #3548

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

* fix(core): harden plans directory handling

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

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

- Sanitize session IDs to prevent path traversal in plan filenames

- Expand tilde (~) in configured plansDirectory paths

- Preserve plansDirectory in bare mode

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

- Close TOCTOU gap with post-write path containment validation

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

- Add happy-path I/O tests for configured plansDirectory
2026-05-17 19:43:24 +08:00
jinye
d2d426fad0
feat(serve): SSE replay sizing + slow_client_warning backpressure (#4175 Wave 2.5 PR 10) (#4237)
* feat(serve): SSE replay sizing + slow_client_warning backpressure

#4175 Wave 2.5 PR 10. Closes the SSE replay / backpressure knobs
called out in #3803 §02 so chatty Stage 1 sessions get an honest
reconnect window and operators get a heads-up signal before clients
are summarily evicted.

- **`DEFAULT_RING_SIZE` 4000 → 8000.** Per-session replay ring depth
  now matches the #3803 §02 target for chatty sessions.
- **`--event-ring-size <n>`** CLI flag (default 8000) lets operators
  tune the ring per daemon. Threaded `ServeOptions` →
  `BridgeOptions.eventRingSize` → both `new EventBus()` construction
  sites (fresh sessions + restore path). Validation is fail-CLOSED
  (positive finite integer; 0 / NaN / negative throw at boot).
- **`slow_client_warning` SSE frame.** When a subscriber's queue
  crosses 75% full the bus force-pushes a synthetic
  `slow_client_warning` to that subscriber once per overflow
  episode, carrying `{queueSize, maxQueued, lastEventId}`. The flag
  re-arms after the queue drains below 37.5% (hysteresis, no flap
  near threshold). If the queue actually overflows after the
  warning, the existing `client_evicted` terminal frame path still
  fires. Like `client_evicted`, the warning has no `id` (synthetic
  frame; must not burn a sequence slot for other subscribers).
- **`?maxQueued=N`** query param on `GET /session/:id/events`
  (range `[16, 2048]`, default 256). Lets cold reconnect clients
  pre-size their per-subscriber backlog so a large `Last-Event-ID:
  0` replay doesn't trip the warning on the first publish. Range
  rationale: lower bound 16 (smaller is useless for any replay);
  upper bound 2048 (so a single subscriber can't pin ~1 MB just by
  asking). Out-of-range / non-decimal returns `400
  invalid_max_queued` BEFORE opening the SSE stream — clean 4xx
  beats half-opening a stream + emitting a `stream_error` (which
  EventSource would auto-reconnect on).
- **`slow_client_warning` capability tag** — single source of truth
  for the warning frame + `?maxQueued` query param + ring-size
  knob. Old daemons silently lack all of these; pre-flight via
  `caps.features`.
- **SDK extensions** (`@qwen-code/sdk`): typed
  `DaemonSlowClientWarningEvent` (added to known event union and
  `DaemonStreamLifecycleEvent`); schema-validated by a new
  `isSlowClientWarningData` predicate; reducer
  (`reduceDaemonSessionEvent`) increments `slowClientWarningCount`
  + stores `lastSlowClientWarning`. Warning is **non-terminal** —
  `alive` stays true (only `client_evicted` / `stream_error` /
  `session_died` close the stream). Re-exported from the public
  SDK entry.
- **Docs**: `qwen-serve-protocol.md` updates the features list (adds
  `slow_client_warning` and the previously-missing `client_identity`
  to match reality post-#4231), documents the `?maxQueued` query
  param, adds the warning frame to the event table, and notes the
  new default ring size. `qwen-serve.md` adds the `--event-ring-size`
  flag row.

Tests: 19 eventBus (4 new: warning at 75%, once per episode,
no `id` on the synthetic frame, hysteresis re-arm), 106 bridge
(2 new: validate eventRingSize accept/reject), 111 server (4 new:
?maxQueued accept/absent/non-decimal/out-of-range +
EXPECTED_STAGE1_FEATURES update), 14 SDK daemonEvents (2 new:
schema validation + non-terminal reducer behavior). 321 focused
tests total, all green.

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

* refactor(serve): adopt PR #4237 review feedback (eventBus polish)

Address the actionable items from the Qwen Code review bot's pass
on PR #4237:

- Pre-compute `warnThreshold` / `warnResetThreshold` per
  `InternalSub` at `subscribe()` time so `publish()`'s per-event
  hot path is one integer compare per subscriber instead of a
  multiply + compare. The `!warned` short-circuit still collapses
  the steady state to a single boolean read; this just shaves a
  multiply when the threshold check actually fires.
- Document the back-of-queue ordering choice for the synthetic
  `slow_client_warning` frame in `EventBus.publish()`: front-push
  was considered but mid-stream front-insertion would mis-count
  `forcedInBuf` in `BoundedAsyncQueue.next()`, and `forcePush`
  already short-circuits via `resolvers.shift()` for the
  active-consumer case — the back-of-queue path only matters for
  stalled consumers, who can't drain regardless of warning
  position.
- Reuse the existing `collect()` helper in the "default ring size
  8000" test for consistency with the rest of the file; the new
  test also tightens the assertion by checking that the first
  retained event id is 2 (id=1 dropped by the ring) and the last
  is 8001.
- Soften the "~500 B per session" magic number in
  `BridgeOptions.eventRingSize`'s JSDoc to a qualitative
  description (each retained `BridgeEvent` is a reference plus its
  serialized payload; ceiling scales as
  `ringSize × average-event-size`).

Rejected:
- Bot's claim that the error JSON contains `\`...\`` escape
  sequences — bot misread the JS template-literal source as the
  wire output; `JSON.stringify` does not escape backticks, and
  the existing `cwd` error messages use the same style.
- Bot's "use `Record<string, never>` instead of `[key: string]:
  unknown`" suggestion on `DaemonSlowClientWarningData` — every
  other event-data type in `sdk-typescript/src/daemon/events.ts`
  carries the same index signature for additive-field
  compatibility.
- Bot's "features list breaks alphabetical order" — the
  capability list is grouped by protocol lifecycle (health →
  capabilities → session lifecycle → events → permissions), not
  alphabetical.

Tests: 139 focused tests across eventBus + httpAcpBridge + SDK
daemon events — all passing. Behavior unchanged; this is
hot-path micro-opt + comment polish only.

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

* fix(serve): correct queue tagging + plumb maxQueued through SDK

Address both P2 findings from the Codex review pass on PR #4237.

**Bug 1: `BoundedAsyncQueue.forcedInBuf` position-invariant break**

The previous `forcedInBuf` counter only tracked LIVE-vs-FORCED
correctly when all forced entries lived at the FRONT of the buffer
(subscribe-time `Last-Event-ID` replay). The new mid-stream
`slow_client_warning` path force-pushes to the BACK of the queue
while the queue is still open, which the existing accounting was
not designed for:

  - publish 6 events at maxQueued=8 → 75% threshold trips →
    force-push warning at the back → buf=[1..6, warning],
    forcedInBuf=1.
  - consumer shifts `1` → forcedInBuf decremented to 0 (incorrect:
    `1` was a live frame, not the forced one).
  - consumer drains 2..6 + warning → buf=[], forcedInBuf=0, true
    live count = 0, but `size` getter and `push()` cap check then
    use `buf.length - forcedInBuf` which drifts over subsequent
    refills, causing premature warn / eviction before the cap is
    actually reached.

Replace the position-dependent counter with a per-entry
`{value, forced}` tag. `liveCount` is incremented in `push()` /
decremented in `next()` only when the shifted entry was non-forced
— position becomes irrelevant. `size` getter returns `liveCount`
directly. The class doc comment is rewritten to call out that the
new tag is the position-independent replacement for the old
"forced frames must stay at the front" invariant.

Regression test in `eventBus.test.ts` reproduces the codex trace
(warn at 75%, drain past warning, refill to cap) and asserts no
premature eviction.

**Bug 2: SDK does not expose `?maxQueued`**

`docs/users/qwen-serve.md` and `docs/developers/qwen-serve-protocol.md`
both document `?maxQueued=N` as something SDK clients can request,
but `SubscribeOptions` on `DaemonClient` only declared `lastEventId`
+ `signal`, and `subscribeEvents()` always fetched `/events` without
a query string. Typed-SDK consumers had no way to opt in without
hand-crafting URLs.

  - Add `SubscribeOptions.maxQueued?: number` with JSDoc noting the
    daemon range `[16, 2048]` and the pre-flight requirement on
    `caps.features.slow_client_warning`.
  - `DaemonClient.subscribeEvents` builds the URL with an optional
    `?maxQueued=<n>` segment. No client-side range validation —
    the daemon's `parseMaxQueuedQuery` is the source of truth and
    returns structured `400 invalid_max_queued`; duplicating the
    bounds in two layers would diverge on the next tweak.
  - `DaemonSessionSubscribeOptions extends SubscribeOptions` so the
    new field flows through `DaemonSessionClient` automatically.

Three new SDK tests:
  - subscribeEvents appends `?maxQueued=N` when set
  - omits the query string when absent (existing behavior preserved)
  - propagates a `400 invalid_max_queued` unchanged

Tests: 214 focused tests across eventBus / bridge / SDK
DaemonClient / DaemonSessionClient / daemonEvents, plus 111 in the
server suite. All green; the new eventBus regression case proves
the position-invariant fix.

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

* refactor(serve): adopt PR #4237 copilot review feedback

Address 6 of 8 copilot-reviewer findings on PR #4237; the other 2
(#1 forcedInBuf live-size corruption, #5 SDK lacks maxQueued) were
already fixed in bae42c88b — replied on the threads with the
commit hash.

- **[2] server.ts:1068** — `?maxQueued=` (present-but-empty) now
  fails closed with `400 invalid_max_queued` instead of silently
  falling back to the default queue cap. The API documents
  fail-closed for any malformed value before opening SSE, so an
  empty string is unambiguously malformed. New server.test.ts
  case locks this in.
- **[3] commands/serve.ts:93** — CLI help text for
  `--event-ring-size` no longer mis-shapes `Last-Event-ID` as a
  query parameter. It is an HTTP header, and the daemon's SSE
  route does not parse a `?Last-Event-ID=` query.
- **[4] docs/developers/qwen-serve-protocol.md:351** — clarify
  that `?maxQueued=N` controls the LIVE-event backlog cap.
  Replay frames are force-pushed and exempt from the cap; what
  consumes it is live events that arrive while the subscriber is
  still draining a cold-reconnect replay. Bumping for cold
  reconnects is still the right answer, but for the live tail,
  not for the replay frames themselves.
- **[6] eventBus.ts:214** — stale `ringSize=4000` performance
  comment updated to the new `ringSize=8000` default with a note
  about the O(n) `shift()` cost scaling.
- **[7] sdk-typescript events.ts:492** — `isSlowClientWarningData`
  now uses the existing `isFiniteNumber` helper instead of bare
  `typeof === 'number'`. Mirrors the sibling predicates and
  rejects `NaN` / `Infinity` payloads as schema garbage. New
  daemonEvents.test.ts assertions cover both.
- **[8] server.ts:127** — `createServeApp`'s default-bridge
  construction now also forwards `opts.eventRingSize` to
  `createHttpAcpBridge`, symmetric with the `runQwenServe.ts`
  path. Direct embeds / tests that called `createServeApp`
  without supplying their own bridge but did pass
  `ServeOptions.eventRingSize` were silently getting the
  default 8000 ring.

Tests: 326 focused tests across eventBus / bridge / SDK
DaemonClient / DaemonSessionClient / daemonEvents / server. All
green; the new server.test.ts case + the extended
daemonEvents.test.ts assertions cover the tightened guards.

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

* refactor(serve): adopt PR #4237 wenshao round-2 review feedback

Six adopted findings from @wenshao's second review pass on
PR #4237. The seventh ([10] forcedInBuf 3rd case invariant) was
already fixed in bae42c88b — replied on that thread.

- **[9] + [14] server.ts** — Sanitize attacker-controlled values
  before stderr interpolation in both `parseMaxQueuedQuery` and
  `parseLastEventId`. New `safeLogValue()` helper uses
  `JSON.stringify` to escape control characters (`\n`/`\r`/…) so a
  URL-encoded newline in `?maxQueued=%0a` can't inject extra log
  lines into journald/Loki/Splunk pipelines. Matches the
  `workspace_mismatch` sanitization style in `sendBridgeError`.
  Fixed in both helpers (the sibling pre-existing
  `parseLastEventId` had the same shape) so the file stays
  consistent.

- **[11] httpAcpBridge.ts** — `!Number.isFinite(eventRingSize)`
  was redundant: `Number.isInteger(NaN)` and
  `Number.isInteger(Infinity)` both return `false`, so the sibling
  `!Number.isInteger` already catches both. Drop the dead guard.

- **[12] httpAcpBridge.ts** — Add soft upper bound
  `MAX_EVENT_RING_SIZE = 1_000_000` on `eventRingSize` to catch
  operator typos (`--event-ring-size 80000000` vs `8000000`). At
  ~500 B per `BridgeEvent` an 1M-frame ring already pins ~500 MB
  per session — well past any realistic workload. Not a security
  boundary (operator-controlled flag), pure typo defense. Existing
  bridge construction test extended with an `80_000_000` case.

- **[13] commands/serve.ts** — CLI `--event-ring-size` flag now
  sources its default from `DEFAULT_RING_SIZE` (imported from
  `serve/eventBus.js`) instead of the hardcoded literal `8000`.
  Without this, a future bump of the bus default would silently
  not take effect for daemons launched through the CLI because
  the flag always overrides — single source of truth fixes that.

- **[15] eventBus.ts** — Drop unreachable `event.id ?? this.lastEventId`
  fallback in the `slow_client_warning` frame. `event` is locally
  constructed at the top of `publish()` with `id: this.nextId++`
  and is guaranteed defined. Use `event.id as number` directly +
  an inline note about the invariant.

Tests: 197 (eventBus 20 / bridge 107 / SDK DaemonClient 57 / SDK
daemonEvents 14) + 112 server. All green; the new upper-bound
bridge case + the existing log assertions pin the changed
behaviors.

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

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-05-17 19:30:43 +08:00
JerryLee
6a1d55c737
fix(core): apply tool name migrations at dispatch (#4213) 2026-05-17 19:04:08 +08:00
jinye
0a4a08e443
feat(serve): add client heartbeat (#4175 Wave 2.5 PR 9) (#4235)
* feat(serve): add client heartbeat route

Adds POST /session/:id/heartbeat plus SDK helpers so long-lived
adapters (TUI/IDE/web) can refresh the daemon's last-seen
bookkeeping. Bridge stores per-session and per-client timestamps
behind a getHeartbeatState() snapshot accessor that PR 12
read-only diagnostics and PR 24 revocation policy will consume.

- Capability tag: client_heartbeat (advertised on /capabilities.features)
- Identified clients must echo X-Qwen-Client-Id; the bridge validates
  the id BEFORE bumping any timestamp so a forged id can't mask
  client absence
- Per-client entries are dropped together with the registration
  ref-count in unregisterClient, so churn doesn't leak stale ids
- getHeartbeatState returns a snapshot Map; mutating it does not
  leak into bridge state
- Anonymous heartbeats bump only the per-session watermark

Errors mirror the rest of the routes — 404 SessionNotFoundError, 400
invalid_client_id (header malformed or unknown for this session).

Roadmap PR 9 from #4175. Depends on PR 7 (#4231 client identity,
merged) for the trusted clientId registry.

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

* feat(sdk): re-export HeartbeatResult from package root

The published @qwen-code/sdk only exposes the root entrypoint via
`exports`; daemon subpath imports are not part of the public API.
Adding HeartbeatResult to packages/sdk-typescript/src/daemon/index.ts
made it reachable internally but not for downstream consumers writing
`import type { HeartbeatResult } from '@qwen-code/sdk'` — every other
daemon result type (PromptResult, SetModelResult, DaemonSession, etc.)
is forwarded through the root barrel, so HeartbeatResult was the only
hole in the heartbeat helper's public surface.

Inserted alphabetically between DaemonStreamLifecycleEvent and
KnownDaemonEvent to match the existing ordering convention.
2026-05-17 18:57:28 +08:00
jinye
07e0e82258
feat(serve): advertise typed_event_schema + pin SDK public surface (#4175 PR 4 follow-up) (#4226)
* feat(serve): advertise typed_event_schema capability

Follow-up to #4217 (`feat(protocol): add typed daemon event schema v1`,
Wave 1 PR 4 of #4175), which landed the SDK-side typed schema +
`KnownDaemonEvent` union + reducer but did not register a daemon-side
capability tag for it. Without the tag, non-SDK clients (web debug
UI, third-party adapters, channel/IDE backends not yet on
`@qwen-code/sdk`) have no way to detect at the protocol envelope
level that the daemon promises to emit only `KnownDaemonEvent`-shaped
frames — they would either pin against SDK version, or pre-flight
every frame defensively.

Add `typed_event_schema: { since: 'v1' }` to `SERVE_CAPABILITY_REGISTRY`,
inserted right after `session_events` (the route that delivers the
frames whose schema this tag describes). The capability is purely
informational — `narrowDaemonEvent`/`asKnownDaemonEvent` already
fall back to "unknown" for older daemons that don't advertise the
tag, so the SDK does not gate any behavior off the tag.

Sync `EXPECTED_STAGE1_FEATURES` (server.test.ts) and the integration
test array (qwen-serve-routes.test.ts) with the registry order, the
same lockstep discipline #4214 codified.

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

* test(sdk): pin typed event surface at the public SDK entry, point DaemonSessionClient docstring at it

Two small follow-ups to #4217 (Wave 1 PR 4 of #4175).

1. Public-entry regression fence

   `@qwen-code/sdk` is a single-entry package: `package.json.exports`
   only exposes `.` (`dist/index.{cjs,mjs,d.ts}`), and the bundle
   is built from `src/index.ts`. Symbols re-exported only from
   `src/daemon/index.ts` are unreachable to consumers unless they
   are also forwarded by `src/index.ts`. #4217 forwards the typed
   event schema correctly today, but the two-layer chain has no
   compile-time test pinning it — a future daemon export that lands
   in `src/daemon/index.ts` but is missed by `src/index.ts` would
   ship invisibly.

   Add `test/unit/daemon-public-surface.test.ts` that imports
   `* as Public from '../../src/index.js'`, asserts at runtime that
   every PR 4 value is `typeof === 'function'` (or a primitive of
   the expected shape), round-trips a raw `DaemonEvent` through the
   public `asKnownDaemonEvent` to prove the wire-up actually works,
   and compile-imports every PR 4 type so any drift fails to build.

2. DaemonSessionClient docstring pointer

   The class docstring already deferred typed event consumption to
   "the protocol schema layer" without a concrete pointer. Now that
   #4217 has put `asKnownDaemonEvent` and `reduceDaemonSessionEvent`
   in `./events.js`, name them so future readers can find the
   typed surface without grepping. No code change.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
2026-05-17 18:43:38 +08:00
易良
ab03badaae
fix(core): extend DashScope provider detection with additional hostname rules (#4157)
* fix(core): extend DashScope provider detection with additional hostname rules

Add internal domain patterns (*.dw.alibaba-inc.com and
*.data.aliyun-inc.com) to isDashScopeProvider() so that
DashScope-compatible private gateways are recognized
without requiring DASHSCOPE_PROXY_BASE_URL.

Closes #4138

* feat(core): add providerType field for explicit DashScope provider opt-in

Allow modelProviders entries to declare providerType: 'dashscope'
so that custom endpoints on private gateways are recognized as
DashScope-compatible without relying solely on hostname detection.

Closes #4138

* revert(core): remove DashScope providerType opt-in

* refactor(core): broaden dashscope internal domain match to *.alibaba-inc.com / *.aliyun-inc.com

Drop the business-specific dw./data. subdomain checks and treat any
*.alibaba-inc.com or *.aliyun-inc.com host as DashScope-compatible.
If a future internal service under these domains is not compatible,
narrow it via a runtime provider property rather than re-introducing
business-name subdomains.

* fix(core): clarify internal dashscope origin matching

* docs(core): clarify DashScope internal host matching
2026-05-17 18:36:42 +08:00
kkhomej33-netizen
a5e4839e07
fix(cli): restore ACP prompt counter on resume (#4233) 2026-05-17 18:32:44 +08:00
Shaojin Wen
0240c310fd
feat(core): PR-2.5 — post-promote stream redirect + natural-exit registry settle (#3831 follow-up) (#4102)
* feat(core): PR-2.5 — post-promote stream redirect + natural-exit registry settle

Closes the two limitations PR-2 (#3894) deferred for the Phase D part
(b) Ctrl+B promote flow (#3831):

1. **Post-promote stream redirect**: today the `bg_xxx.output` file
   is frozen at promote time because `ShellExecutionService` detaches
   its data listener as part of PR-1's ownership-transfer contract.
   PR-2.5 wires a caller-side `onPostPromoteData` callback so bytes
   from the still-running child append to the file via an
   `fs.createWriteStream` opened in `handlePromotedForeground`.
2. **Natural-exit registry settle**: today the registry entry stays
   `'running'` until `task_stop` / session-end `abortAll` fires its
   abort listener. PR-2.5 wires `onPostPromoteSettle` so natural
   child exit transitions the entry to `'completed'` / `'failed'`
   with the right exitCode / signal / error message.

- New exported types: `ShellExecuteOptions`, `ShellPostPromoteHandlers`,
  `ShellPostPromoteSettleInfo`.
- `execute()` options bag now accepts `postPromote?: { onData, onSettle }`.
  Threaded through to both `executeWithPty` and `childProcessFallback`.
- PTY's `performBackgroundPromote` (line ~1159): after disposing
  the foreground data + exit + error listeners, RE-ATTACH minimal
  forwarders that call `postPromote.onData` / `postPromote.onSettle`
  when the caller opted in. Backwards compat: when `postPromote` is
  unset the PR-2 detach-everything contract is preserved (the
  re-attach is gated on each callback being defined).
- `childProcessFallback`'s `performBackgroundPromote` (line ~706):
  same pattern — re-attach `stdout.on('data', ...)`, `stderr.on('data',
  ...)`, `child.once('exit', ...)`, `child.once('error', ...)` when
  the caller opted in. `error` listener routes through `onSettle`
  with `error` populated, so spawn-side errors after the foreground
  errorHandler detached don't crash the daemon via the default
  unhandled `'error'` event.
- Both paths wrap caller callbacks in try/catch so a thrown handler
  doesn't crash the child's data loop / unhandled-rejection the
  service.

- New `PromoteArtifacts` type — slots shared between the foreground
  `execute()` postPromote handlers (which fire on the service side
  as soon as promote happens) and the post-resolve
  `handlePromotedForeground` finalizer (which runs after
  `await resultPromise` returns). The two race; the buffer +
  settle-queue absorb that race so neither chunks nor the eventual
  exit info are lost.
- `executeForeground` wires `postPromote` handlers that route data
  to either `promoteArtifacts.stream` (if open) or
  `promoteArtifacts.buffer` (drained when the stream opens), and
  queue settle info if the wired handler isn't yet installed.
- `handlePromotedForeground` opens `fs.createWriteStream(outputPath,
  { flags: 'w' })`, writes the initial snapshot first, drains the
  buffer, then registers the entry and wires `onSettleWired` with
  the full registry decision table:
    - `error` set → `registry.fail(shellId, error.message, endTime)`
    - `exitCode === 0` → `registry.complete(shellId, 0, endTime)`
    - non-zero exitCode → `registry.fail(shellId, "Exited with code N", endTime)`
    - signal !== null → `registry.fail(shellId, "Terminated by signal N", endTime)`
    - all-null fallback → `registry.fail(shellId, "Exited with unknown status", endTime)`
- Fires queued settle synchronously after wiring so a fast command
  that exits between promote and finalizer doesn't get lost.
- Self-audit catch: closes the output stream on the
  `registry.register` throw path so the FD doesn't leak past the
  orphan-child kill.

- 3 new in `shellExecutionService.test.ts`:
  - `post-promote bytes route to postPromote.onData when callback provided`
  - `postPromote.onSettle fires on natural child exit after promote`
  - `backwards compat: without postPromote, listeners stay fully detached`
- 3 new in `shell.test.ts` under a `foreground → background promote
  PR-2.5` describe block:
  - `post-promote bytes APPEND to bg_xxx.output via write stream`
  - `natural child exit transitions registry entry to "completed"`
  - `non-zero exit / signal / error → "failed" with descriptive message`
- Bulk-replaced 50 prior `{},` (empty 6th-arg shellExecutionConfig)
  with `expect.objectContaining({}),` + added `expect.objectContaining({
  postPromote: expect.any(Object) }),` as the 7th-arg expectation for
  the foreground execute call.
- Updated the existing `registers a bg_xxx entry on result.promoted`
  test to assert on `fs.createWriteStream` + `stream.write` instead
  of the now-removed `fs.writeFileSync` snapshot path.

182/182 shell.test.ts pass + 73/73 shellExecutionService.test.ts pass
+ 111/111 coreToolScheduler.test.ts pass + 60/60 AppContainer.test.tsx
pass; tsc + ESLint clean.

Self-audit: 3 rounds (positive / reverse / cross-file) found one
issue — output stream FD leak on `registry.register` throw — and
fixed it before flagging complete. All flagged edge cases (stream
errors, child-exits-before-wire-up race, task_stop during natural-
exit window, promote-never-happens cleanup, backwards compat
without callbacks) have explicit handling and / or test pinning.

* fix(core): #4102 review wave — 3 Critical + UTF-8 + tests

3 Critical race/correctness issues + 1 multibyte-corruption suggestion
+ 3 test coverage gaps addressed:

**Critical 1 — child_process late-chunk drop (service)**
Settle was fired on 'exit', but stdout/stderr can emit buffered data
between 'exit' and 'close'. Late chunks landed in
`promoteArtifacts.buffer` after shell.ts had already closed the
stream + transitioned the registry → silently dropped → truncated
`bg_xxx.output`. Switched to listening on 'close' which guarantees
all stdio is fully drained. (code, signal) payload is identical to
'exit', just with proper ordering.

**Critical 2 — stream-flush wait before registry transition (shell)**
`stream.end()` is asynchronous; pending writes can still be in the
libuv queue when it returns. The old code transitioned the registry
immediately after `.end()`, so a /tasks consumer could observe a
`completed` entry and read the output file BEFORE the trailing
bytes were on disk. Fixed: wired settle now `stream.once('finish',
...)` BEFORE calling `registry.complete/fail`. `error` event also
short-circuits to the transition so a late ENOSPC doesn't hang the
settle path forever.

**Critical 3 — stream-open-fail buffer leak (shell)**
If `fs.createWriteStream` threw, the catch path set `stream = null`
but the foreground `onData` handler would still take the
`stream === null` branch and push chunks into `promoteArtifacts.buffer`
— unbounded growth under a sustained child whose output file
couldn't be opened. Added a `streamFailed: boolean` latch on
`PromoteArtifacts`. When set, `onData` drops chunks (with a debug
log) instead of buffering. The catch branch sets the latch.

**Suggestion — shared TextDecoder corrupts multibyte UTF-8 (service)**
child_process post-promote used ONE TextDecoder for both stdout AND
stderr. The decoder's continuation-byte state machine assumes one
byte source; interleaved multibyte chunks corrupted. Now uses
separate decoders + flushes both with `decode()` (no `stream: true`)
on settle so trailing bytes surface as their final characters.

**Suggestion — llmContent reflects already-settled status (shell)**
When the queued-settle drain transitions the registry synchronously
(fast-exit race), the model-facing copy was still saying "Status:
running. … task_stop({...})". Updated to branch on
`postPromoteAlreadySettled` / `postPromoteFinalStatus` — when the
process is already gone, the copy says "Status: completed/failed"
and replaces the `task_stop` suggestion with "Process has already
exited; no `task_stop` needed".

**Suggestion — test coverage gaps**
Added: (a) `queued-settle race: onSettle BEFORE
handlePromotedForeground completes` — custom service impl fires
onSettle synchronously before resolving the promote promise, pins
the drain path. (b) child_process post-promote tests for stdout/stderr
forwarding + 'close'-not-'exit' settle + spawn-error settle.

**Self-audit**: Round 1 + reverse audit. Stream.once mock added to
fire 'finish' synchronously so existing tests don't hang on the new
flush wait. 76/76 shellExecutionService.test.ts (+3) + 183/183
shell.test.ts (+1) pass; tsc + ESLint clean.

* fix(core): #4102 review wave-2 — 3 more

C1 (shell.ts:2227): the WriteStream `'error'` event handler only
logged. `fs.createWriteStream` reports common open failures
(ENOENT / EACCES / ENOSPC) asynchronously via that event rather
than throwing. Result: `promoteArtifacts.stream` kept pointing at
the failed stream; `onSettleWired` attached a `.once('finish')`
listener that would never fire → registry stuck on `running`
forever. Latch the failure (null the shared `stream` slot,
set `streamFailed`); `onSettleWired`'s existing `if (!stream)`
branch then transitions the registry immediately.

C2 (shellExecutionService.ts:1468): the promote handoff removes the
foreground `ptyErrorHandler` and only re-attaches data + exit
listeners. A subsequent PTY `error` event had no listener — Node
treats an unhandled `error` from an EventEmitter as a fatal
exception that takes the whole CLI down. Attach a post-promote
forwarder that ignores expected PTY read-exit codes (EIO / EAGAIN,
same filter the foreground handler uses) and routes unexpected
errors through `postPromote.onSettle` with `error` populated.
Single-fire latch shared with `onExit` so settle never fires twice.

C3 (shell.ts:2503): `onSettleWired` waits for the stream's
asynchronous `'finish'` event before flipping
`postPromoteAlreadySettled`, but the model-facing `statusLine` was
built immediately after invoking `onSettleWired` on the queued
settle. A fast-exited promoted command could therefore land
"Status: running" + a `task_stop` instruction in production even
though settle was already observed. Split into two flags:
`postPromoteSettleObserved` (set synchronously when settle is
classified) drives the model copy; the registry transition stays
behind the stream flush.

Tests: +1 PR-2.5 wave-2 PTY error-routing test; +2 shell.ts tests
(stream open async error → registry still transitions; async
`'finish'` after queued-settle drain → llmContent says 'completed'
before registry transition fires).

* fix(core): #4102 review wave-3 — 4 actionable

T2 (shell.ts:2456) — Critical buffer-leak race
`onSettleWired` previously set `promoteArtifacts.stream = null`
BEFORE calling `stream.end()`. Any `postPromote.onData` chunk that
landed between that null assignment and the actual flush completing
saw `stream === null && streamFailed === false` and pushed into
`promoteArtifacts.buffer` — a buffer that has no further drain path
(the foreground finalizer has already returned). Result: chunks
stranded indefinitely; PTY mode in particular hits this because
`onExit` can fire while kernel buffers still hold data. Fix drains
the pre-settle buffer to the stream BEFORE nulling AND latches
`streamFailed = true` so any subsequent chunk drops via the
existing `else if (streamFailed)` arm in `onData` instead of
leaking. Updates the `streamFailed` doc to cover both setters
(open-fail and settle-done) so the dual semantic is explicit.

T3 (shell.ts:2262) — silent chunk-drop in catch path
When `fs.createWriteStream` throws synchronously (rare: ENOENT on
a vanished tmpdir), chunks already in `promoteArtifacts.buffer`
were silently lost with no observability — oncall reading a
truncated `bg_xxx.output` had no way to distinguish "stream open
failed" from "child produced nothing." Logs the dropped chunk
count and empties the buffer.

T5 (shell.ts:2443) — opaque all-null fallback
The "Exited with unknown status" fallback fired the registry to
'failed' without any context about which fields were null. This
branch is meant to be unreachable; hitting it indicates the
service emitted a defective settle info object. Includes the
field values in both the fail message and a warn log so the
oncall engineer can tell this path apart from the other "failed"
branches.

T6 (shellExecutionService.ts:1452) — leaked PTY post-promote listeners
`ptyProcess.onData(...)` returns an `IDisposable` that was being
discarded; same for `onExit`. The `'error'` listener function was
also not captured (no way to `removeListener` it). EventEmitter
holds refs to listener closures, which transitively hold refs to
`onPostData` / `onPostSettle` / the caller's `promoteArtifacts`.
While bounded by the PTY's lifetime, the closures keep the
caller's state pinned for the post-settle delay window. Captures
all three handles into `postPromoteDataDisposable` /
`postPromoteExitDisposable` / `postPromoteErrorListener`, then
releases them via a shared `disposePostPromoteListeners()` call
from `firePostSettle` (idempotent — each slot null-checked and
nulled after disposal).

Tests: +1 service test for IDisposable + error-listener cleanup;
+2 shell.ts tests for buffer drain race and catch-path snapshot
fallback. Existing tests stay green (262 → 265 in the touched
suites; 7819 → 7822 across the core package).

* fix(core/test): drop unused 'registry' in wave-3 T2 test (TS6133)

CI build failed across all platforms with src/tools/shell.test.ts(4395,15): error TS6133. The variable was a leftover from copying the queued-settle test pattern; the wave-3 T2 test inspects writeStreamMock.write call history directly and never reads the registry, so the assignment is dead code. Drop it.

* fix(core): #4102 review wave-4 — 6 actionable

T1 (Critical, shellExecutionService.ts:860 child_process onSettle
exactly-once)
The PTY path used a `firePostSettle` latch but child_process wired
`close` and `error` independently to `onPostSettle`. A spawn-side
error followed by Node's auto-emitted `'close'` would call the
caller's settle TWICE, racing the registry transition. Added the
same single-fire latch on the child_process path.

T2 (Critical, shell.ts:2264 handoff race reorder)
Original order was `write(snapshot) -> drain buffer -> assign stream`.
Synchronous today (no race in current code), but assign-after-drain
leaves a hazard for any future refactor that adds an `await` inside
the drain loop — a chunk arriving in that window would land in
`promoteArtifacts.buffer`, then post-assign chunks would write to
the stream first, producing out-of-order bytes until the settle
drain. Reordered to `write(snapshot) -> assign stream -> drain
buffer`, which closes the hazard regardless of future async
additions.

T3 (Suggestion, shellExecutionService.ts:816 decoder flush gated
on onSettle)
The trailing-multibyte flush ran inside the `child.once('close', ...)`
handler, which was only installed when `onSettle` was set. An
`onData`-only caller (no onSettle) lost trailing continuation
bytes silently. Hoisted flush into `flushPostPromoteDecoders`
called from `firePostSettle`, and made `firePostSettle` available
on the `'close'` path independent of onSettle (T6 install).

T4 (Suggestion, shell.ts:1700 promoted ANSI passthrough)
The regular `executeBackground` path strips ANSI before writing to
`bg_xxx.output`; the promoted-foreground onData path appended raw
chunks. Reading `bg_xxx.output` after Ctrl+B showed plain text up
to the snapshot then raw `\x1b[31m` / cursor-move / clear-screen
sequences for the post-promote tail — unreadable. Apply
`stripAnsi(rawChunk)` before write/buffer, matching the
executeBackground contract.

T5 (Suggestion, shellExecutionService.ts:786 UTF-8 hardcoded)
The post-promote child_process decoders were hard-coded to
`new TextDecoder('utf-8')`, but the foreground decoder runs
encoding detection via `getCachedEncodingForBuffer`. On a non-UTF-8
child (e.g. GBK on a Chinese Windows shell), the snapshot decoded
correctly but the post-promote tail was mojibake. Capture the
foreground decoder's `.encoding` property and reuse it for
post-promote (with utf-8 fallback if foreground hadn't seen any
bytes yet, and a try/catch around `new TextDecoder` for the rare
unsupported-encoding case).

T6 (Suggestion, shellExecutionService.ts:1540 `error` listener
gated on onSettle)
The post-promote `error` listener was attached only when `onSettle`
was set. An `onData`-only caller still had the foreground
errorHandler detached; a post-promote spawn error would then crash
the CLI via Node's unhandled-error default. Hoisted the close +
error listeners into `if (postPromote)` so any caller opting into
post-promote gets crash protection; if `onSettle` is absent the
listeners log + drop instead of routing.

T7 (Suggestion, shellExecutionService.ts:791 onSettle-only
pipe-block deadlock)
Same root cause as T6: when only `onSettle` is set, the foreground
`stdout`/`stderr` 'data' listeners are detached and no post-promote
listener replaces them. The Readables stay paused, the OS pipe
buffer fills (~64KB on Linux), the child blocks on `stdout.write`,
'close' never fires, onSettle never fires. Added `child.stdout?.resume()`
and `child.stderr?.resume()` in the no-onData branch so the child
can drain its pipes and reach exit.

T8 (Suggestion, shell.ts:2614 dead inspectLine ternary)
`inspectLine`'s ternary returned the same string on both sides —
copy-paste leftover from when the other two adjacent ternaries
(statusLine / stopLine) were correctly varied. Collapsed to a
single string assignment.

Tests: +5 regression tests (4 child_process: T1 double-fire latch,
T3 onData-only flush, T6 onData-only error survives, T7 onSettle-
only resume; +1 shell.ts: T4 ANSI strip).

265 -> 270 in the touched suites; 7822 -> 7827 across the core
package; full suite green.

* fix(core/test): use ShellOutputEvent type in wave-4 onData callbacks (TS2345)

CI lint failed on the wave-4 (T3 / T6) tests with TS2345: pushing
ShellOutputEvent into Array<{type:string;chunk:unknown}> narrows
incompatibly. Switch to ShellOutputEvent[] (matches earlier helpers
at lines 758/966) and discriminate the union via .type === 'data'
when reading .chunk so the narrowed multibyte assertion still
type-checks.

* fix(core): address PR #4102 review — PTY error guard, flush timeout, diagnostic marker, failed-settle test

- Move PTY post-promote error listener from `if (postPromote?.onSettle)` to
  `if (postPromote)` to match child_process path and prevent unhandled error
  crashes for onData-only callers
- Add 10s flush timeout in onSettleWired so stalled streams don't leave
  registry entries stuck on 'running' forever
- Append diagnostic marker to output file on stream error so truncation
  is visible without debug logging
- Add queued-settle test with exitCode:1 asserting 'Status: failed.' in
  llmContent

* fix(core): address PR #4102 review — align PTY/child_process guards, add flush timeout, diagnostic marker, and tests

- Widen PTY post-promote onExit + error listener guard from
  `if (postPromote?.onSettle)` to `if (postPromote)` to match
  child_process path — prevents unhandled error crash and listener
  leak for onData-only callers
- Add 10s flush timeout in onSettleWired so stalled streams don't
  leave registry entries stuck on 'running' indefinitely
- Append diagnostic marker to output file on stream error so
  truncation is visible without debug logging
- Remove model name references from code comments
- Add tests: PTY onData-only error/exit, flush timeout fallback,
  appendFileSync diagnostic marker, queued-settle with failed exit code

* fix(core): address PR #4102 review round 2 — listener cleanup, rename, constant hoist

- Fix expect.objectContaining({}) misused as runtime arg in 2 execute() call sites
- Add child_process post-promote stdout/stderr listener cleanup in firePostSettle
- Rename streamFailed → streamClosed to reflect its overloaded semantics
- Hoist FLUSH_TIMEOUT_MS to module-level PROMOTE_FLUSH_TIMEOUT_MS constant
- Fix dangling FLUSH_TIMEOUT_MS reference (was undefined at runtime)
- Add Windows note to streams pause/resume comment
- Document PTY onData dispose-before-settle as known limitation
2026-05-17 17:57:08 +08:00
tanzhenxin
cc32ef2ff9
test(perf): skip daemon baseline harness under sandbox (#4234)
The qwen-serve-baseline harness walks the daemon process tree using
host-side `pgrep -P`. Under the Docker/Podman sandbox the daemon's
`qwen --acp` child and its MCP grandchildren run inside the container's
PID namespace, which host `pgrep` cannot observe, so the MCP-grandchild
descendant walk always sees zero and times out. The test passes in the
no-sandbox job but failed every retry in the Docker release job.

Extend the existing Windows `SKIP` gate to also skip when sandbox is
enabled, matching the precedent in acp-integration.test.ts and
cron-tools.test.ts.

Refs #4205
2026-05-17 17:52:34 +08:00
ChiGao
c25e22b575
feat(serve): add session-scoped permission route (#4232)
Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>
2026-05-17 17:48:30 +08:00
ChiGao
4d9cbe49c0
feat(serve): add daemon-stamped client identity (#4231)
* feat(serve): add daemon-stamped client identity

* fix(serve): harden daemon client identity handling

---------

Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>
2026-05-17 16:19:30 +08:00
kkhomej33-netizen
605e5eea16
fix(cli): include skill base dir in slash commands (#4224) 2026-05-17 15:52:29 +08:00
kkhomej33-netizen
d6914bdfd6
fix(core): align shell tool description with configured shell (#4170) 2026-05-17 15:36:59 +08:00
ChiGao
b90a2c91c9
feat(sdk): harden daemon session client (#4225)
Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>
2026-05-17 15:05:37 +08:00
jinye
2453b82add
[codex] Add daemon session load/resume (#4222)
* feat(serve): add daemon session load resume

Adds HTTP and SDK support for restoring persisted daemon sessions through load/resume routes, including replay buffering for load and guarded concurrent restore handling.

Refs #4175

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

* fix(serve): address review feedback on daemon session load/resume

- Gate `defaultEntry` claim in `restoreSession` on
  `defaultSessionScope === 'single'`, mirroring `doSpawn`. Without the
  gate, a restored session silently became the omitted-scope attach
  target on `'thread'`-default daemons.
- Rename advertised capability `session_resume` to
  `unstable_session_resume` to match the underlying ACP method
  (`connection.unstable_resumeSession`). `session_load` stays stable.
- Seed `lastEventId: 0` in `DaemonSessionClient.resume`, symmetric with
  `load`. The agent's `unstable_resumeSession` schedules an
  `available_commands_update` via `setTimeout(0)`; without the seed the
  SDK consumer would miss that frame.
- Add HTTP-level test for the `RestoreInProgressError → 409` envelope.

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

* docs(serve): adopt review feedback comments on session load/resume

- Cross-reference the `POST /session` disconnect-cleanup rationale
  from `restoreSessionHandler`'s `!res.writable` branch so future
  maintainers find the BQ9tV race + tanzhenxin attach-rollback
  context without grep.
- Document `DaemonSessionState.{models, modes, configOptions}` in
  the SDK so callers can narrow to the ACP `SessionModelState` /
  `SessionModeState` / `SessionConfigOption` shapes.
- Add JSDoc on `DaemonClient.restoreSession` explaining why
  `loadSession` and `resumeSession` collapse into one transport.

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

* fix(serve): preserve restore state and harden in-flight restore races

Address the four Critical findings from PR #4222 review (wenshao):

- Coalesced restore waiters now observe the same ACP state the
  original restore caller did. `state: {}` in `restoreSession`'s
  coalesce branch was clobbering the spread `restored.state`, so
  concurrent callers got different payloads based purely on timing.
  Cache the load/resume response on `SessionEntry.restoreState` and
  return it from both the existing-byId early return and the
  coalesce branch.
- Drop the `defaultEntry` promotion on restore. Explicit
  `session/load` / `session/resume` is "give me THIS id"; it must
  not become the implicit attach target for subsequent omitted-id
  `POST /session` callers under `single` scope. Reserves
  `defaultEntry` for sessions created through `doSpawn` only.
- Reserve coalesced attaches synchronously via
  `InFlightRestore.coalesceState.count` so the spawn owner's
  `requireZeroAttaches` disconnect-reaper sees a non-zero
  `attachCount` on the freshly registered entry and skips the
  kill. Without this, B's `attachCount++` happened after `await
  inFlight.promise`, leaving a window where A's HTTP-disconnect
  cleanup could reap the session out from under B.
- Include `pendingRestoreIds` in the `killSession` channel-teardown
  decision. The last live session leaving while a restore is
  in-flight on the same channel would otherwise SIGTERM the
  channel mid-restore.
- Bump `RestoreInProgressError`'s `Retry-After` from 1s to 5s
  (matches `SessionLimitExceededError`); under the default
  `initTimeoutMs` of 10s, 1s pushed clients into tight loops.

Tests: new bridge cases covering state propagation through
coalesce, the spawn-owner-disconnect race, the
pendingRestoreIds-aware channel teardown, and the no-promote-
on-restore invariant. Existing "attaches twice" test rewritten
to assert the cached restore state propagates.

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

* test(serve): cover acpAgent load/resume + restore route error mappings

Close the test-coverage gaps wenshao called out in PR #4222 review:

- acpAgent.test.ts gains a `QwenAgent loadSession /
  unstable_resumeSession` block that locks down the new contract
  end-to-end at the agent layer:
  * `loadSession` missing persisted session → throws
    `RequestError.resourceNotFound("session:<id>")` (code -32002
    + `data.uri`).
  * `loadSession` existing session → returns LoadSessionResponse
    AND triggers `session.replayHistory(messages)` so SSE
    subscribers see the persisted turns.
  * `unstable_resumeSession` missing session → same
    resourceNotFound contract.
  * `unstable_resumeSession` existing session → returns the
    response WITHOUT replaying history (resume restores model
    context internally; UI replay is intentionally suppressed).
  Required extending the mocked `RequestError` with
  `resourceNotFound`, and mocking `SessionService` per case.
- server.test.ts adds the missing restore-route wire mappings:
  `WorkspaceMismatchError → 400 workspace_mismatch` and
  `SessionLimitExceededError → 503 + Retry-After: 5`. Combined with
  the existing 409 case for `RestoreInProgressError`, the route
  layer now has full structured-error coverage.
- Updated the 409 test's `Retry-After` expectation from `1` to `5`
  to match the bumped retry hint.

Disconnect-cleanup tests for the restore route were intentionally
not added — the cleanup branch is line-for-line identical to
`POST /session`'s handler (which itself ships without route-level
disconnect tests due to flaky supertest + Node http close-event
timing).

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

* docs(serve): document daemon session load/resume routes

Sync the docs to the routes that landed via PR #4222:

- `docs/developers/qwen-serve-protocol.md`:
  * Add `session_load` and `unstable_session_resume` to the
    advertised features list, with a note on the `unstable_`
    prefix mirroring ACP's underlying method name.
  * Document `POST /session/:id/load` and `POST /session/:id/resume`
    — request body, response shape (including the cached `state`
    field that late attachers observe), and the full error
    envelope: 404 unknown id, 400 workspace_mismatch, 503
    session_limit_exceeded (counts in-flight restores), 409
    restore_in_progress (cross-action race).
  * Note the SSE replay ring bound (4000 frames default) and the
    "subscribe immediately after load" guidance for long histories.
- `docs/users/qwen-serve.md`:
  * Add a "Loading and resuming a persisted session" section with
    the SDK example (`DaemonSessionClient.load` /
    `DaemonSessionClient.resume`) and the load-vs-resume
    decision table.
  * Update the durability model — sessions are still ephemeral
    across daemon restarts in Stage 1, but persisted sessions on
    disk can now be reloaded.

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

* fix(test): use _meta payload to satisfy ACP SessionConfigOption types

The two new state-propagation tests in `httpAcpBridge.test.ts` used
`{ id, name, value }` as a `SessionConfigOption`, but ACP's actual
`SessionConfigSelect` shape requires `currentValue` + `options`. vitest
runs through esbuild and skips strict typechecking, so the local
`vitest run` passed; CI's `tsc --build` (run during `npm run prepare`)
caught it.

Switch the fixture to `_meta: { tag: '...' }` instead — `_meta` is
typed as `Record<string, unknown> | null` on the ACP response shapes,
so any payload survives. The assertions only need the bridge to
forward the state object intact, which `_meta` proves equally well
without committing the test to the full SessionConfigOption union.

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

* fix(serve): symmetric restore coalesce guard + transportClosed leak + defensive cleanup

Address the two new Critical findings + the test/cosmetic gaps from
wenshao's second review pass on PR #4222 (`a3f38da3a`):

- **[Critical] Symmetric coalesce guard.** The previous guard only
  rejected `load`-on-`resume`; `resume` arriving while a `load` was
  in flight silently coalesced and inherited the load's history-
  replay frames over SSE — directly violating resume's "no UI
  replay" contract (made worse by `DaemonSessionClient.resume()`
  seeding `lastEventId: 0`). Tighten the guard to
  `action !== inFlight.action` so any cross-action race throws
  `RestoreInProgressError`. Same-action coalescing is unaffected.

- **[Critical] `transportClosed` dangling rejection.** When
  `withTimeout` wins the `Promise.race` against `channel.exited`,
  the `.then(throw)` chain on `channel.exited` stays pending. A
  later channel exit (next session boundary, daemon shutdown, agent
  crash) fires the `throw` with no observer attached — Node 22 logs
  `unhandledRejection`, and `--unhandled-rejections=throw`
  deployments crash the daemon. Add `transportClosed.catch(() => {})`
  to suppress the dangling rejection after the race settles.

- **`isAcpSessionResourceNotFound` exact-match fallback.** The
  message-fallback path used `message.includes(expectedUri)`, which
  would falsely match a sessionId of `"a"` against a message
  containing `"session:abc"`. Tighten to exact equality on the
  canonical `Resource not found: <uri>` form. The primary
  `data.uri` path remains the dominant code path.

- **`loadSession` mcpServers default symmetry.** `loadSession` now
  uses `params.mcpServers ?? []` to mirror `unstable_resumeSession`.
  Defends against a future ACP schema loosening that makes
  `LoadSessionRequest.mcpServers` optional — without the
  null-coalesce, `newSessionConfig` would `TypeError` on iteration.

Tests added:
- `httpAcpBridge.test.ts`: `resume`-on-`load` rejection (mirror of
  the existing `load`-on-`resume` test); regression for the
  dangling `unhandledRejection` (resolves `channel.exited` after
  the restore promise has already settled and asserts no
  `unhandledRejection` event); shutdown-awaits-restore via
  `Promise.race`-based ordering.
- `server.test.ts`: 400 for non-string and over-length `cwd` on
  the restore routes (mirroring the equivalent `POST /session`
  cases for `parseOptionalWorkspaceCwd`).
- `acpAgent.test.ts`: load with `getResumedSessionData()` returning
  `undefined` — distinct code path that does NOT call
  `replayHistory`.

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

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-05-17 12:58:47 +08:00
qqqys
2e773b0e60
[codex] Allow custom output directory for /export (#4193)
* feat(cli): support export output directories

* fix(cli): address export review feedback

* test(cli): cover JSON export directory handling

* fix(cli): constrain export output directories

* test(cli): cover export edge cases

* fix(cli): address export directory review feedback

* fix(cli): revalidate export directory before write

* fix(cli): validate export directory before mkdir

* fix(cli): harden export target writes

* fix(cli): refine export failure handling

* fix(cli): clarify export directory mode

* fix(cli): include export path context in errors

* fix(cli): add export debug logging

* fix(cli): make export tests path portable

* fix(cli): refine export validation diagnostics

* test(cli): cover export validation failures
2026-05-17 12:33:02 +08:00
ChiGao
80f1e266ba
feat(protocol): add typed daemon event schema v1 (#4217)
Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>
2026-05-17 12:31:16 +08:00
Shaojin Wen
ef29700bce
fix(ui): trim background task results and show newest first (#4094) (#4125)
Some checks are pending
Qwen Code CI / Classify PR (push) Waiting to run
Qwen Code CI / Lint (push) Blocked by required conditions
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Blocked by required conditions
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* fix(ui): trim background task results and show newest first (#4094)

Two related improvements to the background task pill and dialog:

1. Trim outdated terminal task results.
   `BackgroundTaskRegistry` and `BackgroundShellRegistry` now cap
   retained terminal entries at 32 each (mirroring `MonitorRegistry`'s
   existing `MAX_RETAINED_TERMINAL_MONITORS` pattern). Running, paused,
   and cancelled-but-not-yet-notified entries are never evicted —
   pruning a not-yet-notified entry would break the SDK contract that
   every `register` pairs with exactly one terminal `task-notification`.

2. Show newest tasks at the top of the dialog.
   `useBackgroundTaskView` now sorts entries by `startTime` descending
   so the dialog opens with the cursor on the most recently launched
   task. `LiveAgentPanel` reverses internally back to ASC for its own
   visual layout (newest row sits closest to the composer).

* perf(shell-registry): batch abortAll prune + statusChange into one pass

abortAll() previously delegated to cancel() per entry, so each running
shell triggered its own pruneTerminalEntries() and statusChange wakeup.
On shutdown / `/clear` with N running shells the only subscriber
(useBackgroundTaskView) re-pulled getAll() N times for what is logically
a single batch transition.

Settle each entry inline via the new private settleAsCancelled() helper,
then fire prune + statusChange exactly once after the loop. The split
keeps the running-status guard at the public-API boundary so callers
can't accidentally re-settle a terminal entry.

* fix(ui): two-bucket sort so running tasks outrank fresh terminals

The earlier startTime DESC sort surfaced the newest LAUNCH but let an
older long-running / paused entry get pushed below a batch of newer
terminal entries — the user opening the dialog to check on the running
work would find it buried under stale completed rows.

Split the merge into two buckets:
  - active (running + paused): sorted by startTime DESC so the most
    recent launch sits at the very top of the dialog.
  - terminal (completed / failed / cancelled): sorted by endTime DESC
    so the most recently FINISHED entry leads the terminal section
    (matches "what changed while I wasn't looking" intuition; a long
    task that just settled outranks an old quick task that finished
    hours ago).

Pin the new behavior with two tests covering active-above-terminal
and the endTime-vs-startTime distinction inside the terminal bucket.

* fix: add missing outputFile and isBackgrounded to retention cap tests

The merge brought in required fields on AgentTaskRegistration that the
retention-cap test helpers were not supplying.

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-05-17 09:13:24 +08:00
Shaojin Wen
5ce2f2854b
fix(test): clear boundedPromise timers to prevent unhandled rejections (#4220)
boundedPromise timeouts could fire after test completion,
causing vitest to exit with code 1 due to unhandled
rejection. Add clear() method and cleanup all pending
timers in the finally block.
2026-05-17 08:28:22 +08:00
qqqys
07165a095c
Add stop hook blocking cap (#4208)
* feat(core): add stop hook blocking cap

* fix(core): tighten stop hook cap behavior

* fix(cli): show goal judge details

* fix(core): bound stop hook blocking cap

* fix(core): surface subagent stop cap warnings

* fix(core): clean up stop hook cap loop

* test(core): cover stop hook cap integrations

* test(core): strengthen stop hook cap coverage
2026-05-17 06:52:56 +08:00
jinye
0eed884c0b
fix(rewind): restore upstream TOCTOU ordering + heal sticky failed marker (#4216)
Some checks are pending
Qwen Code CI / Classify PR (push) Waiting to run
Qwen Code CI / Lint (push) Blocked by required conditions
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Blocked by required conditions
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* fix(rewind): restore upstream TOCTOU ordering + heal sticky failed marker

Two related bugs that landed in #4064:

1. **trackEdit widened the pre-write TOCTOU window**. PR #4064 inserted
   `await trackEdit(filePath)` between `checkPriorRead` and
   `writeTextFile` in both `edit.ts` and `write-file.ts`. trackEdit does
   `stat` + `copyFile` and on large files can take hundreds of
   milliseconds. The pre-existing comment above `checkPriorRead`
   acknowledged a stat-then-write race window of "two adjacent
   syscalls"; the new ordering widened that to "freshness check →
   potentially-multi-second backup → write", so an external mutation
   landing during the backup would no longer be detected before the
   write clobbered it.

   The fix is the ordering upstream `claude-code/src/tools/FileEditTool`
   uses, with its own explicit comment on the equivalent block:

   > These awaits must stay OUTSIDE the critical section below — a
   > yield between the staleness check and writeTextContent lets
   > concurrent edits interleave.

   Moved `trackEdit` to BEFORE `checkPriorRead` in both tools. Backups
   are idempotent (deterministic `{hash}@v{version}` filename), so
   running the backup on a path that ends up rejected just leaves an
   unused-but-correct backup of the pre-edit state, not corrupt state.

2. **The `failed` marker added in d59838338 stayed sticky in `trackEdit`**.
   When `makeSnapshot` recorded `failed: true` for a file (transient
   I/O error during per-file backup), the next `trackEdit` for that
   file skipped because the early guard only checked existence, not
   the failed bit. The pre-edit state never got re-captured even
   after the underlying I/O recovered, leaving the snapshot marked
   filesFailed forever for that path.

   The guard now skips only on confirmed (non-failed) entries:
   `if (existing && !existing.failed) return;`. The re-check guard
   at the end is updated symmetrically so the fresh backup actually
   replaces the failed entry (instead of being silently dropped by
   the "already present" branch).

Tests:

- `edit.test.ts`: pin the TOCTOU ordering via spy on `fileReadCache.check`
  and `mockFileHistoryService.trackEdit`. Verifies `trackEdit` is called
  before the last `cache.check` (the pre-write freshness guard). Same
  pattern in `write-file.test.ts`. Both tests fail on the old ordering
  (verified by stashing the production change).
- `fileHistoryService.test.ts`: `heals a failed entry on the next
  trackEdit attempt` — sabotage storage so makeSnapshot records
  failed, restore storage, run trackEdit, assert the entry is now
  non-failed with a real backupFileName.

* test(rewind): switch TOCTOU tests to behavioral assertions + harden heal test

Address review feedback from code-reviewer agent:

- The TOCTOU ordering tests previously asserted ordering via
  `callOrder.lastIndexOf('cache.check')`, which would silently degrade
  to a tautology if a future refactor added any `cache.check` call
  AFTER the pre-write check (e.g. the deferred atomic-write + content-
  hash post-check explicitly mentioned in the comment above the
  freshness guard).

  Switch to a behavioral assertion that directly tests the invariant:
  install a `trackEdit` mock that mutates the file on disk (bumps
  mtime) before returning. The pre-write `checkPriorRead` must catch
  the in-trackEdit mutation — that only happens if `trackEdit` runs
  BEFORE the pre-write check. The broken ordering would run pre-write
  check first (passing on pre-mutation stats), then trackEdit (which
  mutates), then write (which clobbers the external mutation).

  Asserting on `result.error?.type === FILE_CHANGED_SINCE_READ`
  exercises the actual race protection the fix exists to preserve,
  and survives any future refactor that keeps the invariant intact
  regardless of how many `cache.check` calls happen on the way.

  Verified by reverting just the production change to HEAD~1: both
  behavioral tests fail under the broken ordering (file is silently
  overwritten, no FILE_CHANGED_SINCE_READ error), and pass under the
  fix.

- The heal test (`heals a failed entry on the next trackEdit attempt`)
  previously asserted `backupFileName !== null` but not that the file
  on disk at that name actually contained the current content. Add a
  `readFile + expect(...).toBe('p2-content')` assertion so a regression
  that wrongly reuses `previous.backupFileName` (pointing to
  `p1-content`) fails loudly instead of silently passing.
2026-05-17 02:01:58 +08:00
jinye
9505246886
fix(serve): align integration test + user doc with merged sessionScope override (#4214)
PR #4209 (Wave 2 PR 5) shipped per-request `sessionScope` override and
added a `session_scope_override` capability tag to the registry. Two
follow-ups from wenshao's review landed unaddressed:

1. `integration-tests/cli/qwen-serve-routes.test.ts` still asserted
   the pre-PR 9-element `caps.features` list and was named "all 9
   Stage 1 features". Running the suite against a real daemon would
   fail — the daemon now advertises 10 features, with
   `session_scope_override` between `session_create` and
   `session_list` per the registry order. PR CI didn't catch this
   because integration tests need a real `qwen serve` spawn and run
   only in the release pipeline; the unit-level
   `EXPECTED_STAGE1_FEATURES` constant in `server.test.ts` was
   updated, but its integration sibling was missed.

2. `docs/users/qwen-serve.md` "Stage 1.5+ runtime guarantees" still
   listed per-request `sessionScope` override as item 1 of "Blockers
   for serious downstream use", saying "today the daemon-wide default
   is the only setting." Directly contradicts the merged behavior and
   the protocol doc, so downstream integrators reading the user guide
   get inverse guidance.

Fixes:
- Update the integration test name to "all 10 Stage 1 features" and
  insert `session_scope_override` in the asserted array (matching
  registry order); add a comment noting the unit/integration/registry
  triple must stay in lockstep.
- Remove the obsolete blocker bullet from the user doc and renumber
  the remaining items (2/3 → 1/2 in Blockers, 4-7 → 3-6 in Reliability,
  8-10 → 7-9 in Integration ergonomics).

No production code changes.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
2026-05-17 01:51:25 +08:00
易良
ba77ddd81b
fix(lsp): expose status and startup diagnostics (#3649)
* feat(lsp): add /lsp slash command to show server status

Implements the /lsp command that displays the status of all configured
LSP servers. Previously this was documented in the FAQ but never
implemented, leaving users with no way to check if their language
servers started successfully.

Changes:
- Add LspServerStatusInfo interface to lsp/types.ts
- Add getServerStatus() to LspClient and NativeLspClient
- Expose getServerHandles() from NativeLspService
- Create lspCommand.ts with status table output
- Register /lsp in BuiltinCommandLoader (only when LSP is enabled)

The command shows: server name, command, languages, and status
(NOT_STARTED / IN_PROGRESS / READY / FAILED + error message).

* fix(lsp): expose status and startup diagnostics

* fix(lsp): harden status command diagnostics

* fix(lsp): add stderr error listener and harden initialization error handling

- Add stderr 'error' event listener in LspConnectionFactory to prevent
  unhandled stream errors from crashing the process
- Wrap setLspInitializationError calls in try-catch in config.ts to guard
  against post-initialization state changes that would throw
2026-05-17 01:42:28 +08:00
ChiGao
57de269f45
feat(sdk): add DaemonSessionClient skeleton (#4201)
* feat(sdk): add daemon session client

* fix(sdk): harden daemon session event replay

* fix(sdk): replay attach-time daemon events

---------

Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>
2026-05-17 01:01:12 +08:00
jinye
0788ed7fb0
test(perf): add daemon baseline harness (#4175 Wave 1 PR 1) (#4205)
* test(perf): add daemon baseline harness (#4175 Wave 1 PR 1)

First implementation PR of the Mode B v0.16 rollout (issue #4175 Wave 1
PR 1). Captures reference performance metrics for the `qwen serve`
daemon so subsequent Mode B PRs (M2 MCP shared pool, M3 architecture
refactor, M4 multi-client safety) can be measured against a known
baseline rather than guessed-at numbers.

## What it captures

The new `integration-tests/cli/qwen-serve-baseline.test.ts` runs five
describe blocks against a real `qwen serve` daemon:

- RSS scaling across 1 / 5 / 10 same-workspace `createOrAttachSession`
  calls (sampled via `ps -o rss=`).
- Same-workspace attach latency for the 2nd and 5th attach.
- MCP child amplification with two configured idle-mcp servers,
  measured via two-level `pgrep -P` walk (daemon → ACP child → MCP
  grandchildren).
- SSE backpressure invariants exercised at the unit layer by
  instantiating `EventBus` directly: queue overflow → synthetic
  `client_evicted` frame; replay across reconnect honors
  `lastEventId` up to ring size.
- Prompt p50 / p99 (skipped when `QWEN_TEST_MODEL_KEY` is unset, with
  an explicit reason recorded in the snapshot).

Each run writes a structured JSON snapshot to
`<INTEGRATION_TEST_FILE_DIR>/perf-baseline.json` plus a Markdown
summary, with `gitCommit` / platform / config preserved for cross-PR
correlation.

## Honest documentation of current limits

The captured snapshot includes a `notes` field flagging that with the
default `sessionScope: 'single'`, N successive
`createOrAttachSession` calls return the same sessionId — so the RSS
and MCP metrics here measure "N attaches to one shared session", not
"N distinct sessions". Once Wave 2 PR 5 lands per-request
`sessionScope: 'thread'` override, the harness will be updated to
optionally force distinct sessions and surface the P1 MCP N×M
amplification before M2 fixes it.

## Reused / new

Reused: existing daemon spawn pattern from `qwen-serve-routes.test.ts`
(port-0 + stdout regex + SIGTERM teardown), `pgrep -P` pattern from
`qwen-serve-streaming.test.ts:144`, `EventBus` invariants from
`eventBus.test.ts`, `DaemonClient` SDK, integration-tests
`globalSetup.ts` env var conventions.

New (this PR):

- `integration-tests/cli/_daemon-harness.ts` (~280 lines) — extracts
  the inline daemon spawn pattern into a shared helper plus adds
  `getRssMB`, `startRssPolling`, `countDescendants`, `percentiles`,
  `consumeSseEvents`, `writeWorkspaceSettings`. Future serve test
  files can import instead of inlining.
- `integration-tests/fixtures/idle-mcp/{server.mjs,package.json}` — a
  minimal stdio MCP fixture that responds to `initialize` /
  `tools/list` and idles. Lets the harness count real MCP children
  via `pgrep` without depending on a network npm package in CI.
- `integration-tests/baselines/baseline-stage-1.json` — the first
  captured baseline at this commit. Future Mode B PRs can diff their
  run against this file; updating it is a deliberate one-line change
  in a follow-up PR.

## Reference patterns from opencode

JSDoc on the main test file documents the shape borrowed from
`opencode/test/memory/abort-leak.test.ts` (forced-GC heap-growth),
`opencode/src/cli/heap.ts` (RSS poll + threshold-triggered
`writeHeapSnapshot`, useful for Wave 6 production tooling), and
`opencode/src/util/cpu-watchdog.ts` (event-loop lag drift sampling).
The harness here is daemon-level multi-session — a shape neither
opencode nor qwen-code had before.

## Engineering principles checklist

- [x] Independently mergeable (test-only; no production code touched)
- [x] Backward compatible (no removed routes / event fields / CLI behavior)
- [x] Default off (PR CI does not run integration tests; baseline
      runs in release CI / nightly / manual)
- [x] `qwen serve` Stage 1 routes / SDK behavior preserved (no production
      code changed)
- [x] Gradual migration (no client adapter migration in this PR)
- [x] Reversible (revert = delete files, no other side effects)
- [x] Tests-first (this IS the test PR; harness exercises real daemon
      end-to-end; Windows skipped via existing `process.platform === 'win32'`
      precedent)

## Test plan

- [x] `KEEP_OUTPUT=true TEST_CLI_PATH=$(pwd)/packages/cli/dist/index.js
      QWEN_BASELINE_SKIP_PROMPT_LATENCY=1 QWEN_BASELINE_RSS_SAMPLE_DURATION_MS=2000
      npx vitest run integration-tests/cli/qwen-serve-baseline.test.ts`
      — 6 passed / 1 skipped (prompt latency requires model key)
- [x] `npx tsc --noEmit -p integration-tests/tsconfig.json` — only
      pre-existing tsconfig `paths` glob warning remains, no new errors

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

* fix: import exit from node:process in idle-mcp fixture

Fixes eslint no-undef error: 'process' is not defined.
Replace process.exit(0) with exit(0) from node:process import.

* fix(test): remove stale baseline lint disable

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

* fix(test): harden daemon baseline harness

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

---------

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-05-17 00:41:26 +08:00
jinye
54fd5c50f0
feat(telemetry): add detailed sensitive span attributes (#4097)
Layer detailed content attributes onto the existing hierarchical spans
(qwen-code.interaction / qwen-code.llm_request / qwen-code.tool) gated
by includeSensitiveSpanAttributes:

- Interaction span: user prompt (new_context)
- LLM request span: system prompt + hash + preview + length (full text
  deduped per session via SHA-256), tool schemas (per-tool tool_schema
  events, also hash-deduped), model output
- Tool span: tool input, tool result on every exit path (success +
  pre-hook block + post-hook stop + tool error + try-block cancel +
  catch-block cancel + execution exception)

All large content truncated at 60KB with *_truncated and
*_original_length metadata. Heavy serialization (safeJsonStringify on
tool I/O, partToString on user prompt) is guarded by the sensitive
flag at the call site so it doesn't run when telemetry is off.

Also adds:
- getActiveInteractionSpan() helper for client.ts to attach prompt
  attributes to the interaction span.
- Updated config schema description and docs (telemetry.md +
  settings.md) to reflect expanded scope and add security/cost notes.
- 28 unit tests for detailed-span-attributes, 4 tests for
  getActiveInteractionSpan, integration mocks updated.
2026-05-17 00:36:48 +08:00
qqqys
daaa85e98e
feat(cli): add fork-session resume flag (#4159)
* feat(cli): add fork-session resume flag

* fix(cli): address fork-session review feedback

* fix(cli): handle fork session copy failures

* fix(cli): guard sandbox session handoff flag
2026-05-17 00:27:52 +08:00
Shaojin Wen
b9590283c0
fix(cli): pass rewind selector test props (#4211)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-05-16 23:57:50 +08:00
jinye
878f35fc4f
feat(serve): per-request sessionScope override on POST /session (#4175 Wave 2 PR 5) (#4209)
* feat(serve): per-request sessionScope override on POST /session

Resolves the FIXME at httpAcpBridge.ts:BridgeOptions.sessionScope from
#3803 — clients can now override the daemon-wide sessionScope per
request instead of being stuck with whatever boot-time value the
operator picked. A VSCode window that wants strict isolation can ask
for `'thread'` against a default-`'single'` daemon, and vice versa.

Wire change:
- POST /session body accepts optional `sessionScope: 'single' | 'thread'`
- Per-request value wins; daemon-wide default remains the fallback when
  the field is omitted (bit-for-bit backward compat for every existing
  caller)
- Invalid values yield 400 `{ code: 'invalid_session_scope' }`
- New capability tag `session_scope_override` advertised on
  /capabilities.features for negotiation

Bridge changes:
- BridgeSpawnRequest gains optional `sessionScope`
- spawnOrAttach validates the per-request value and resolves
  effectiveScope = req.sessionScope ?? defaultSessionScope
- doSpawn now takes effectiveScope and only stamps `defaultEntry`
  (the single-scope attach slot) when the spawn is single-scope —
  fixes a mixed-scope leak where a thread-first call would let a
  later omitted-scope call attach to the supposedly-isolated session

SDK:
- CreateSessionRequest gains optional `sessionScope`
- DaemonClient.createOrAttachSession conditionally spreads it into the
  JSON body so omitted callers send the same wire shape as before

Tests:
- 4 new bridge tests (override single→thread, override thread→single,
  mixed-scope leak regression, invalid-value rejection)
- 3 new server tests (valid passthrough, invalid 400, omitted backward
  compat)
- 2 new SDK tests (forwards/omits sessionScope on the wire)
- EXPECTED_STAGE1_FEATURES updated for the new capability tag

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

* fix(serve): address Wave 2 PR 5 review findings

Three independent review passes found three real issues:

1. Bridge `TypeError` on invalid `sessionScope` collapsed to opaque 500
   in `sendBridgeError` instead of the typed `400 invalid_session_scope`
   the route layer guarantees. Direct embed / test / future entry-point
   callers bypassing the route would see a generic 500 with stack noise
   on stderr — disagreeing with the route contract.

   Fix: add `InvalidSessionScopeError` class (alongside
   `SessionNotFoundError` / `WorkspaceMismatchError` /
   `SessionLimitExceededError`); the `spawnOrAttach` validator now
   throws it, and `sendBridgeError` translates to the same
   `{ error, code: 'invalid_session_scope' }` shape.

2. SDK `DaemonClient.createOrAttachSession` used a truthy check
   (`req.sessionScope ? ...`) for the conditional spread, silently
   erasing falsy-but-defined values (`''`, `null`, `0`) on the wire.
   A buggy caller would never see the daemon's 400 — it'd inherit the
   daemon-wide default while believing it requested a specific scope.
   Fix: use `!== undefined` (matching the bridge's own validation
   shape). Same fix to the server-side spread for consistency.

3. JSDoc and docs referenced `serve --sessionScope` as if it were a
   shipping CLI flag. It isn't — `ServeOptions` has no field, neither
   `runQwenServe` nor `serve.ts` plumbs one, and the production daemon
   default is hardcoded to `'single'`. Strike the references; note
   that #4175 may add the flag in a follow-up.

Test coverage expanded:
- Cap-bypass guard: per-request `'thread'` overrides cannot bypass
  `maxSessions` on a daemon-default-`'single'` deployment. Without
  this, a future refactor that gated the cap on `defaultSessionScope`
  instead of `effectiveScope` would silently let `'thread'` overrides
  amplify past the limit — the exact N-amplification cliff #3803 was
  about.
- Symmetric mixed-scope leak: daemon-default-`'thread'` +
  single-first-call followed by omitted-scope-second-call must produce
  distinct sessions. Mirrors the existing daemon-default-`'single'` +
  thread-first leak regression.
- Concurrent mixed-scope coalescing: simultaneous single + thread
  `spawnOrAttach` against the same workspace under slow `initialize`
  must not collide on `inFlightSpawns` (tracker keys differ by scope).
- Updated invalid-scope rejection test to assert
  `InvalidSessionScopeError` instance + carried `sessionScope` field.

🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
2026-05-16 23:54:20 +08:00
易良
2c200a3d0a
fix(core): add heap-pressure auto-compaction safety net (#4186)
* fix(core): add heap pressure compaction safety net

Link: #4185

* fix(core): keep heap pressure compaction active

Let heap-pressure bypass also bypass the failed-attempt latch and cover the interaction with a regression test.

Link: #4185

* test(core): cover raw next-speaker history lookup

Verify next-speaker checks use the raw last history entry for function responses while the LLM prompt still uses curated history.

Link: #4185

* fix(core): avoid latching heap-pressure compression failures

* fix(core): back off failed heap-pressure compaction

* fix(core): back off heap-pressure no-op compaction
2026-05-16 23:37:05 +08:00
Dragon
8f54ae9c0f
feat(cli): add built-in status line presets with interactive dialog (#4120)
* chore(skills): add codex reproduce workflows

* feat(cli): add built-in status line presets with interactive dialog

Replace the shell-command-only status line with a preset system that
renders structured session info (model, context usage, git branch,
token counts, etc.) without external commands. Users can configure
which items to display via a new interactive dialog accessible through
/statusline or the settings UI.

- Add statusLinePresets module with 16 built-in item types
- Add StatusLineDialog component with search, multi-select, and preview
- Update /statusline command to open the preset dialog
- Extend settings schema to support { type: "preset", items: [...] }
- Enhance MultiSelect with separator items, active marker, and
  customizable checked text
- Update Footer to support theme-colored preset output

* fix(cli): refresh status line preset after saving

* chore: remove codex reproduce skills

* fix(cli): address status line preset review feedback
2026-05-16 23:22:11 +08:00
dreamWB
966b040359
feat(cli): readline Ctrl+P/N for history and selection navigation (#4082)
* feat(cli): readline Ctrl+P/N for history and selection navigation

Adds GNU-readline-style Ctrl+P (previous) and Ctrl+N (next) shortcuts
to the qwen-code TUI so users coming from bash/zsh, Emacs, or Claude
Code feel at home. The change has three orthogonal behavior groups:

1. Input prompt, history-versus-line-motion two-step edge

   Ctrl+P / Ctrl+N and the arrow keys behave identically and apply a
   two-step edge transition that matches GNU readline and Claude Code:
   inside a multi-line buffer they move the cursor between visual
   rows; on the top row with the cursor away from column 0 the first
   Up press snaps the cursor to column 0 without changing history, and
   only the second press walks one entry back. The mirror rule holds
   for Down at the last row (snap to end of line, then advance). After
   navigateUp the buffer is parked at offset 0 (the "start of older
   entry" landing position); after navigateDown setText's default
   end-of-text positioning keeps the cursor at the end. The same
   two-step rule applies to single-line buffers so the
   reverse-direction case the issue called out works: pressing Ctrl+N
   immediately after Ctrl+P loaded a single-line older entry (cursor
   at col 0) first snaps the cursor to end-of-line, and only the next
   Ctrl+N moves forward through the history. Bare k/j inside the
   input prompt remain ordinary typed letters — the vim aliases are
   selection-list shortcuts, not text-editing ones.

2. Selection lists: arrows, k/j, and Ctrl+P/N are interchangeable

   A new pair of Command bindings, SELECTION_UP and SELECTION_DOWN, is
   wired into the shared useSelectionList hook and every dialog that
   used to hand-roll an "up/down arrow only" or "up/k arrow + vim
   only" navigation check. Covered surfaces: the main selection-list
   hook itself, the MCP / extensions / agents / hooks / background-
   tasks / rewind / plugin-choice / ask-user-question dialogs, the
   memory dialog (both its file list and the auto-memory and
   auto-cleanup toggle panel above the list), the settings dialog
   list (with the in-place value editor's "block other keys while
   editing" guard preserved), and the manage-models dialog's top
   tabs row. The auth-provider wizard's Advanced Config focus rows
   and the resume-session picker's cross-mode arrows are extended
   with the readline Ctrl+P / Ctrl+N synonyms while keeping their
   existing arrow-key and (for the session picker) vim k/j semantics
   intact.

3. Selection surfaces that wrap an active text input

   AskUserQuestionDialog's "Other / type a custom answer" field,
   manage-models' search input, the resume-session picker's search
   field, and the auth-wizard's Context-window number input all
   coexist with the selection list on the same screen. In those
   surfaces typing k or j has to land in the text buffer, not scroll
   the surrounding list. The fix is to scope the input-aware handler
   to unambiguous non-letter shortcuts only — arrow keys plus
   readline-style Ctrl+P / Ctrl+N escape the text field, while bare
   letters (including k / j / p / n) are delivered to the active
   input. The keyBinding-level fix that backs this is the
   `{ key: 'k', ctrl: false }` / `{ key: 'j', ctrl: false }` clauses
   on SELECTION_UP / SELECTION_DOWN, which prevent Ctrl+K from
   accidentally matching SELECTION_UP and thereby firing both the
   list-up handler and the KILL_LINE_RIGHT handler in the same
   keystroke (the P0 finding the quality-gate review surfaced).
   Focus-traversal tokens (the agent tab bar and the background-task
   pill) and chord shortcuts (Ctrl+Shift+Up/Down for embedded-shell
   history) are deliberately left untouched because their existing
   "any printable letter yields focus back to the composer" UX would
   break under the new vim-style letter bindings, and the Help
   viewer's scroll is a viewer rather than a selection list and is
   out of this PR's scope.

Documentation: docs/users/reference/keyboard-shortcuts.md is updated
so the Ctrl+P / Ctrl+N entries describe the two-step edge rule and
the radio-button-select table mentions the new k/j and Ctrl+P/N
aliases. Per-dialog on-screen hints (which still read "↑↓ to
navigate") are intentionally not touched so the i18n string surface
stays unchanged; the global reference doc is the authoritative source
for the new shortcuts.

Tests:
 - packages/cli/src/ui/keyMatchers.test.ts adds positive cases
   covering ↑ / ↓ / bare k / bare j / Ctrl+P / Ctrl+N matching
   SELECTION_UP / SELECTION_DOWN and negative cases asserting that
   Ctrl+K and Ctrl+J do NOT match (the conflict guard).
 - packages/cli/src/ui/components/InputPrompt.test.tsx adds a
   "two-step edge transition for history navigation" describe block
   with four cases: a mid-line Ctrl+P snaps to col 0 without invoking
   navigateUp; an at-col-0 Ctrl+P does invoke navigateUp and then
   parks the cursor via moveToOffset(0); a not-at-end Ctrl+N snaps to
   end-of-line without invoking navigateDown; and arrow Up obeys the
   same rule as Ctrl+P for keyboard-parity. The test file's mock
   buffer's setText was also corrected to mirror the real buffer's
   "cursor lands at the end of the new text" semantic so the cursor
   field is internally consistent during keypress assertions; the
   small InputPrompt render-frame snapshot in the same file's
   __snapshots__/ directory was regenerated to reflect the now-
   accurate cursor render position. Three pre-existing arrow-key
   navigation tests were updated to pre-position the mock cursor at
   the relevant edge before pressing the arrow, because the new
   two-step rule means the first arrow press at a non-edge position
   is a cursor snap, not a history step. Multi-line cursor-between-
   rows movement is covered indirectly by the keyBinding-level
   matcher tests plus the end-to-end manual demo plan.

The work landed in three rounds against the planner's gate: round 1
added the unified SELECTION_UP / SELECTION_DOWN Command binding and
the cursor-first dispatch in the input prompt; round 2 picked up the
quality-gate review's P0 (the Ctrl+K double-fire in the "Other"
custom-input field) and the user's hand-test feedback on the missing
two-step edge in the reverse direction plus the MemoryDialog
top-panel sections that weren't wired through SELECTION_*; round 3
swept the remaining adjacent dialogs (SettingsDialog list,
ManageModelsDialog tabs and search transitions, ProviderSetupSteps
advancedConfig, useSessionPicker's cross-mode arrows) so the
keyboard model is uniform across the TUI.

The original issue also asks for Meta+B / Meta+F word motion and
smarter Ctrl+H token-aware backspace among other readline
conveniences. The user explicitly scoped this PR down to Ctrl+P /
Ctrl+N at the planner approval gate; the remaining wish-list items
are deferred to follow-up issues.

Closes #3821

* docs(cli): refine Ctrl+P/N input-history rows; fix Ctrl+J in selection-list comment

Both items came from a non-blocking COMMENTED review on PR #4082
(https://github.com/QwenLM/qwen-code/pull/4082#pullrequestreview-4271527787),
flagging two polish points in the readline Ctrl+P/Ctrl+N feature the parent
commit `feat(cli): readline Ctrl+P/N for history and selection navigation`
(f66427b) introduced.

The `Up Arrow`, `Down Arrow`, `Ctrl+P`, and `Ctrl+N` rows of the Input
Prompt table in `docs/users/reference/keyboard-shortcuts.md` are reworded
to describe the three-phase keystroke sequence the implementation walks
through — an intra-buffer visual-row step (a no-op in a single-line
buffer, where there's exactly one visual row), a column-edge snap when
the cursor reaches the buffer's first or last visual row with the
cursor not already at column 0 (for the up-direction pair) or
end-of-line (for the down-direction pair), and the readline-style
previous-history or next-history walk on the press after the snap. The
reviewer specifically pointed out that the prior wording described
single-line input as "navigates the input history directly", which no
longer matches the post-PR-#4082 behavior: single-line input also goes
through the snap-then-walk two-press rule (the snap is a no-op when
the cursor is already at the line's edge column, in which case the
keystroke does the history walk on its first press). The new sentence
covers the single-line and multi-line cases in one shape — single-line
is the degenerate zero-row-walk-prefix instance of the same rule. The
up-direction text is shared verbatim between the `Up Arrow` row (L31)
and the `Ctrl+P` row (L43), and the down-direction text between the
`Down Arrow` row (L27) and the `Ctrl+N` row (L42), so the keyboard-
parity alias relationship is signaled by source-side text duplication
rather than a prose cross-reference. The Input Prompt table's 234-byte
canonical row width (the separator row's `| <50-dash> | <177-dash> |`
template, which sets the column-1 and column-2 source-side widths the
file's existing untouched rows already align to) is preserved by
trailing-ASCII-space padding inside the description column.

The comment above `[Command.SELECTION_UP]` and `[Command.SELECTION_DOWN]`
in `packages/cli/src/config/keyBindings.ts` previously read

    // Selection list navigation — up/k/Ctrl+P move selection up; down/j/Ctrl+N move selection down
    // ctrl: false on k/j ensures Ctrl+K (kill-line) and Ctrl+N (history-down) are not captured here

The `Ctrl+N` half of the second line is wrong: `Ctrl+N` is intentionally
matched here as the selection-down readline alias — the
`{ key: 'n', ctrl: true }` entry in the `SELECTION_DOWN` array literal
directly below the comment, mirroring the input-prompt-side
`[Command.HISTORY_DOWN]: [{ key: 'n', ctrl: true }]` binding at L134 of
the same file. The Ctrl-modified key the bare-letter `k` and `j`
matchers actually guard against — the one already bound elsewhere
whose double-match with the bare-letter selection-key the `ctrl: false`
opt-out is preventing — is `Ctrl+J`, the ASCII line-feed (0x0A) encoding
of the Enter family that appears as `{ key: 'j', ctrl: true }` inside
the four-alternative `[Command.NEWLINE]` array a few lines below. The
corrected one-liner is

    // Selection-list nav: arrows + k/j + Ctrl+P/Ctrl+N
    // ctrl: false on bare k/j skips Ctrl+K and Ctrl+J

in the same terse no-trailing-period section-label style as the file's
adjacent `// Screen control` (L129), `// History navigation` (L132),
`// Auto-completion` (L213, post-edit numbering), and `// Text input`
(L219) header comments. A 64-line block-comment that earlier in the
review-fix cycle wrapped this same correct fact in dispatch-broadcast-
model prose plus `keyMatchers.test.ts` backreferences was condensed to
those two lines for cell-budget consistency with the rest of the file.

No code behavior change. The local verification surface the reviewer
named at the bottom of the review summary stays green: from
`packages/cli`,

    npx vitest run \
        src/ui/keyMatchers.test.ts \
        src/config/keyBindings.test.ts \
        src/ui/components/InputPrompt.test.tsx

runs 178 cases with 177 passed and one unrelated skip (the
implementation file `InputPrompt.tsx`'s feature flag for the keyboard-
queue-input-editing case that was already skipped on the parent commit),
including all four cases inside the `InputPrompt > two-step edge
transition for history navigation` describe-block — `Ctrl+P with cursor
mid-line snaps to col 0 without touching history`, `Ctrl+N with cursor
not at end-of-line snaps to end without touching history`, `Ctrl+P at
col 0 walks history and parks the cursor at offset 0`, and `arrow Up
applies the same two-step rule as Ctrl+P (snap before navigate)`. Those
four test-case names are the implementation-side anchors the new docs
wording verbally mirrors. `npx tsc --noEmit -p .` in the same package
directory reports zero diagnostics.

* fix(cli): align readline history shortcuts with dialogs

* test(cli): cover readline navigation aliases

* fix(cli): guard readline shortcuts in dialog inputs

* test(cli): cover readline aliases in more dialogs
2026-05-16 23:07:25 +08:00
tanzhenxin
8d765fec78
refactor(core): TaskBase envelope + foreground subagent persistence (#3970)
* refactor(core): TaskBase envelope + foreground subagent persistence

Establishes a shared `TaskBase` envelope across the agent / shell /
monitor task registries with a mandatory `outputFile` field. Brings the
foreground subagent path into compliance with the new contract, so it
now leaves the same JSONL transcript + meta sidecar on disk that
backgrounded subagents have always produced — closing the only gap
where a registered task wrote nothing. Renames the agent-task
discriminator from `flavor: 'foreground' | 'background'` to claw-code's
`isBackgrounded: boolean`; the deprecated names are kept as
one-release type aliases.

PR 1 of the task-registry-unification design. PR 2 will collapse the
three per-kind registries into one thin TaskRegistry plus per-kind
modules.

* refactor(core): drop unused BackgroundTaskFlavor type alias

The alias only preserved the type name; no in-tree caller used it,
and after the field rename no realistic external consumer use survives
(reading entry.flavor / writing { flavor: ... } both fail at the use
site regardless of whether the alias resolves). Drop it instead of
carrying a hollow shim.

* fix(core): tighten foreground subagent launch path

- Register before writing the meta sidecar so a register() failure can't
  leave an orphaned 'running' meta file behind. writeAgentMeta is
  best-effort and never throws, so the inverse failure mode (registry
  entry without sidecar) is a benign degradation.
- Cache getGitBranch by cwd at the agent module level so foreground
  launches don't pay a fresh git rev-parse exec each time. Branches
  don't change within a process under normal use; the transcript
  annotation is best-effort audit metadata.
- Document on cancel() that foreground entries take a partial path
  through the method — Map deletion is the caller's responsibility
  via unregisterForeground() in the tool-call's finally path.

* fix(agent): correct foreground meta status mapping and register order

The foreground finally block in agent.ts mapped any non-ERROR, non-CANCELLED
terminate mode (including MAX_TURNS, TIMEOUT, SHUTDOWN) to 'completed' in
the sidecar, so post-mortem readers and resume logic saw a successful
status for runs that actually hit a guardrail. Flip the ternary to mirror
the background path: GOAL -> completed, CANCELLED -> cancelled, else ->
failed.

Also reorder the background launch so registry.register() runs before
writeAgentMeta(), matching the foreground path. Both paths now share the
same orphaned-meta guarantee.

* test(agent): rename stale foreground-flavor test

The "default flavor (absent) behaves as background" test name and its
backwards-compat comment referenced the old optional flavor field, but
the registration shape has required isBackgrounded for a while now —
there is no "absent" path to exercise. Rename it to describe what the
assertion actually covers: that background entries fire a task-
notification on complete.

* refactor(core): alias BackgroundTaskStatus to TaskStatus

The local `BackgroundTaskStatus` union was byte-identical to the new
shared `TaskStatus` defined in `tasks/types.ts`. Replace it with a
`@deprecated` type alias so external consumers (notably
`nonInteractiveCli.ts`) keep compiling unchanged while the canonical
name lives in one place.

* refactor(core): tidy monitorRegistry signatures and document cancel ordering

Two small consistency wins flagged in review:

1. `dispatchOwnerLifecycleWake` and `dispatchNotification` were the only
   methods on the registry still typed with the deprecated `MonitorEntry`
   alias. Rename their parameters to `MonitorTask` to match every other
   signature in the file.

2. `cancel()` orders `settle()` and `abort()` differently between its two
   branches, which is intentional (silent cancel locks the terminal status
   before abort listeners run; default cancel lets a naturally-completing
   operation settle through its own terminal path). Document that
   asymmetry in a JSDoc on the method so the next reader doesn't have to
   reverse-engineer it.

* refactor(core): migrate internal BackgroundTaskStatus refs to TaskStatus

The `BackgroundTaskStatus` alias was introduced in 91b59a8fb as a
`@deprecated` synonym for external SDK consumers (notably
`nonInteractiveCli.ts`). New internal references in this PR's own
file kept the old name; migrate them so the only remaining usage of
the deprecated alias is the alias declaration itself.

No behavior change — the alias is `= TaskStatus` so the union is
identical.

* test(agent): cover foreground failed-mode terminal status mapping

The foreground finally block maps GOAL→completed, CANCELLED→cancelled,
and everything else (ERROR, MAX_TURNS, TIMEOUT, SHUTDOWN) → failed.
Only the GOAL branch was asserted; the failed-mode fallback had no
coverage even though the same mapping recently regressed (d67da6d50)
and had to be fixed by review.

Adds a table-driven case mocking getTerminateMode to ERROR /
MAX_TURNS / TIMEOUT and asserting patchAgentMeta receives
status: 'failed'. CANCELLED is already covered by the
"foreground CANCELLED prefixes the partial result" test below.

* test(agent): cover foreground CANCELLED → cancelled meta mapping

Extends the foreground terminate-mode it.each to assert that
CANCELLED is recorded as `cancelled` in the on-disk sidecar — the
existing cancel-prefix test only verified the LLM-visible payload,
leaving the patchAgentMeta mapping uncovered. A regression flipping
CANCELLED → 'failed' would now fail this case.

* test(agent): make registry path assertions platform-agnostic

The outputFile/metaPath regexes hardcoded forward slashes, so the
foreground JSONL+meta reservation test failed on Windows where paths
use backslashes. Accept either separator.

* fix(core): guard executeBackground register-throw window; correct outputFile contract

A throwing register() subscriber in executeBackground() would leak the
already-spawned child + open output stream, unreachable by /tasks /
task_stop. Mirror the promote path's defensive try/catch: abort the
entry's controller, destroy the stream, and rethrow so the launch fails
visibly.

Also correct the TaskBase.outputFile contract: agent JSONL is
materialized on the writer's first append, which is the launch prompt
at attach time — not the first runtime event. A subagent cancelled
before any event still leaves a prompt-only JSONL plus meta, not meta
alone.
2026-05-16 22:53:08 +08:00
jinye
8dfbdaa5d4
feat(telemetry): unify span creation paths for hierarchical trace tree (#4126)
* feat(telemetry): unify span creation paths for hierarchical trace tree (#3731 P3 Phase 1)

Replace disconnected withSpan/startSpanWithContext calls in runtime with
session-tracing typed helpers so LLM and tool spans become children of
the interaction span instead of siblings under the session root.

- Add toolContext ALS with runInToolSpanContext() for concurrent-safe
  tool span scoping (uses AsyncLocalStorage.run, not enterWith)
- Wire startLLMRequestSpan/endLLMRequestSpan in loggingContentGenerator
  for both streaming and non-streaming paths
- Wire startToolSpan/endToolSpan + startToolExecutionSpan/endToolExecutionSpan
  in coreToolScheduler with proper try/finally lifecycle
- Remove redundant withSpan('client.generateContent') wrapper from client.ts
- Fix endToolSpan to not override pre-set status when metadata is omitted
- Change startToolExecutionSpan to read parent from toolContext ALS
- Update tests for new span creation APIs and remove dead test infrastructure

* fix(telemetry): address CI build errors in session-tracing tests

- Remove unused _toolSpan variable (TS6133)
- Use bracket notation for index signature property access (TS4111)

* fix(telemetry): update coreToolScheduler and loggingContentGenerator test mocks

- coreToolScheduler.test.ts: mock startToolSpan/endToolSpan/runInToolSpanContext
  instead of withSpan; update cancellation tests for restored safeSetStatus call
- loggingContentGenerator.test.ts: fix attribute keys in mock, add try/catch in
  endLLMRequestSpan mock to match production best-effort behavior

* fix(telemetry): address review feedback from wenshao

- Add debugLogger.warn in catch blocks of endLLMRequestSpan/endToolSpan/
  endToolExecutionSpan instead of silent swallowing
- Add JSDoc on endToolSpan documenting intentional no-metadata-no-status
  contract with setToolSpanFailure/setToolSpanCancelled
- Add warning in startToolExecutionSpan when called outside
  runInToolSpanContext (no active toolContext)
- Sanitize error message in endToolExecutionSpan: use constant
  TOOL_SPAN_STATUS_TOOL_EXCEPTION instead of raw error message

* fix(telemetry): use partial mock for telemetry/index.js in coreToolScheduler tests

The full mock shadowed all re-exports (logToolCall, etc.) causing 49 test
failures. Use importActual to preserve other exports, only override span
functions.

* fix(telemetry): getLastToolSpan must skip tool.execution sub-spans

startToolExecutionSpan mock also pushes to toolSpanRecords, so at(-1)
returns the execution sub-span instead of the tool span. Use findLast
to filter by name.

* fix(telemetry): address second round review feedback

- Remove redundant safeSetStatus(span, OK) on success path — endToolSpan
  in finally already sets OK via metadata
- Add llm_request.stream attribute (true/false) to distinguish streaming
  vs non-streaming LLM requests in trace backends

* fix(telemetry): endToolSpan mock writes to record directly

Bypass span.setStatus() in mock to avoid potential interference from
vitest module resolution. Write to statusCalls/ended directly on the
ToolSpanRecord.

* fix(telemetry): mock session-tracing.js directly instead of telemetry/index.js

Mocking the barrel re-export (telemetry/index.js) with importActual was
unreliable — vitest's module resolution could bind production code to the
real endToolSpan before the mock override took effect. Mock the source
module (session-tracing.js) directly to guarantee interception.

* fix(telemetry): fix endToolSpan status on success — toolCalls is empty in finally

Root cause: checkAndNotifyCompletion clears this.toolCalls before the
finally block in executeSingleToolCall runs, so the tc lookup always
returns undefined.

Fix: set OK status explicitly in _executeToolCallBody's success path
via safeSetStatus(span, OK), and call endToolSpan() without metadata
in finally (just ends the span, preserves pre-set status from any path).

* fix(telemetry): address Codex review — activate OTel context, end span on failure

- Wrap non-stream generateContent API call + logging in context.with(spanContext)
  so nested OTel spans (HTTP instrumentation, log-bridge spans) parent to
  qwen-code.llm_request instead of session root (matches streaming path).
- runInToolSpanContext now also activates OTel context via otelContext.with,
  not just the custom toolContext ALS. Hooks/HTTP/IO during tool execution
  now correctly parent to qwen-code.tool span.
- Split end*Span helpers: span.end() runs in its own try/catch so a throwing
  setAttributes/setStatus can't leak unended spans.

* fix(telemetry): address Codex review v2 — session-root fallback + execution span timing

- start{LLMRequest,Tool,ToolExecution}Span now fall back to getSessionContext()
  when no parent context, instead of otelContext.active(). Side-query LLM calls
  (auto-title, recap) now stay in the session trace instead of starting a new
  detached trace.
- Move startToolExecutionSpan() to BEFORE invocation.execute(), matching
  claude-code. Previously the synchronous setup inside execute (shell command
  preprocessing, child_process.spawn) ran outside the execution span.

* fix(telemetry): address Codex review v3 — sync throw, idle timeout race, test coverage

- coreToolScheduler.executeSingleToolCall: move try-block to wrap
  invocation.execute() so synchronous throws (e.g. shell setup failure)
  flow into the same catch path as async rejections. Previously a sync
  throw would leak the execution span and skip failure hooks.

- loggingStreamWrapper: track spanEndedByTimeout flag so a stream that
  resumes after the 5-min idle timeout does not run the final
  endLLMRequestSpan (which would no-op anyway, but the flag also stops
  resetSpanTimeout from queuing further timer callbacks).

- coreToolScheduler.test: add execution sub-span assertions for success,
  ToolResult.error, thrown invocation exceptions, and pre-hook denial.

- loggingContentGenerator.test: capture setAttribute calls into the mock
  span attributes record; assert llm_request.stream is false for non-stream
  and true for stream paths.

* fix(telemetry): address Codex review v4 — consistency + test coverage gaps

- endLLMRequestSpan now uses spanCtx.span for mutations (matches
  endToolSpan/endToolExecutionSpan pattern). Same object, but consistent
  lookup pattern prevents future drift.

- Mocks capture endLLMRequestSpan and endToolSpan/endToolExecutionSpan
  metadata so tests can assert token counts, durationMs, success, error
  are forwarded correctly. Add assertions on:
    * Non-stream LLM: inputTokens, outputTokens, success on response path
    * Non-stream LLM: success: false + sanitized error on rejection
    * Stream LLM: final lastUsageMetadata reaches endLLMRequestSpan
    * Tool execution sub-span: success: true on happy path
    * Tool execution sub-span: success: false on ToolResult.error
    * Tool execution sub-span: success: false + sanitized error on throw

- Add OTel error resilience tests: when setAttributes or setStatus throws,
  span.end() must still run and the span must be removed from activeSpans.
  Covered for endLLMRequestSpan, endToolSpan, endToolExecutionSpan.

* fix(telemetry): address Codex review v5 — abort distinction + API symmetry

- session-tracing.ts SpanContext.type: comment 'tool.blocked_on_user' |
  'hook' as Phase 2 forward-declarations (no helpers wired yet).

- endToolExecutionSpan: align no-metadata-no-status behavior with
  endToolSpan. Currently no caller omits metadata, but the asymmetric
  default (OK vs preserve-pre-set) was a maintenance trap.

- loggingContentGenerator generateContent (non-stream) catch block:
  call endLLMRequestSpan BEFORE the logging block, mirroring the
  streaming path. Defense-in-depth against logging-side rejections.

- loggingContentGenerator: restore abort-specific span status message.
  All three LLM error paths (non-stream catch, stream eager-error catch,
  stream loggingStreamWrapper finally) now use
  API_CALL_ABORTED_SPAN_STATUS_MESSAGE when req.config.abortSignal.aborted,
  matching the original withSpan('client.generateContent') behavior.
  Trace backends can now distinguish cancellations from real failures.

- coreToolScheduler _executeToolCallBody catch: distinguish abort vs
  exception in execSpan error message. New constant
  TOOL_SPAN_STATUS_TOOL_CANCELLED prevents operators filtering exec
  spans for errors from seeing cancellation false positives.

- New test asserting exec span uses cancelled-by-user message when the
  invocation throws after abort.

* fix(telemetry): always write 'success' attribute on tool spans

E2E review found qwen-code.tool spans never carry the `success` boolean
attribute (the helper only writes it when metadata is passed, and the
finally block calls endToolSpan(toolSpan) without metadata). This breaks
the most common observability query — filtering tool failures with
`success = false` — because tool spans don't have that field at all.

Fix: setToolSpanFailure / setToolSpanCancelled now also call
span.setAttribute('success', false); the success path in
_executeToolCallBody adds span.setAttribute('success', true) after
safeSetStatus(span, OK). Mirrors the unconditional `success` attribute
on llm_request spans, so backends can use one query for both span types.

Add 4 scheduler-level tests asserting the success attribute on:
- success path
- ToolResult.error path
- thrown invocation path
- cancellation path
2026-05-16 22:29:55 +08:00