* feat(cli): report retained tool-result stats in /doctor memory
Add a tool-result retention section to /doctor memory (and --json) covering
the phase-1 diagnostics from #4184: retained tool-result count/total/largest,
oversized flagging against the 30k widest legal per-tool budget, plus
duplication signals for UI history and compression input. Sizes and counts
only (never content); history is scanned by reference so the diagnostic adds
no memory pressure.
* docs(design): document tool-output offload/preview state transitions and privacy model
Satisfies acceptance criterion 2 of #4184: state transitions of the layered
truncation/offload mitigation (implemented in #4880) and its privacy model,
plus the tmux E2E report for the /doctor memory retention diagnostics.
* fix(cli): align retention diagnostics with per-tool budgets and raw-char sizes
Address review round 1: measure retained tool results with the compression
pipeline's estimatePartChars (raw chars for string outputs, image estimate
for nested media) instead of JSON.stringify, fixing false oversized flags on
newline-dense compliant outputs; compare each result against its own tool's
declared budget resolved from the tool registry instead of a fixed 30k
threshold, so compliant high-budget results (e.g. MCP) are never flagged;
scope the UI-history duplication scan to tool_group result displays so model
text is excluded and rendered tool outputs are actually detected; omit
toolResultRetention from --json when unavailable; log retention collection
failures via debug logger; fix the design note's IO-error persistence bullet;
re-verify the E2E report with the new measurement basis.
* fix(cli): calibrate oversized detection against truncation layers
- Canonicalize tool names before registry lookup (legacy aliases resolve)
- Skip sentinel-prefixed results and apply combined-pass 2x tolerance
- Fall back to configured global threshold for tools declaring no budget
- Compare UI history displays against per-tool budgets
- Document persistence gate in design note; re-verify e2e scenario 3
* fix(core): share tolerance constant and align imageTokenEstimate with compression pipeline
Review (round 4) fixes:
- R4-3: Extract COMBINED_PASS_TOLERANCE_FACTOR into truncation.ts and
share it between the scheduler's combined pass and the retention
diagnostics so both use the same tolerance factor.
- R4-4: Add imageTokenEstimate option to analyzeToolResultRetention
(defaults to DEFAULT_IMAGE_TOKEN_ESTIMATE); doctorCommand now resolves
it via resolveSlimmingConfig (env > settings > default), the same
source the compression pipeline uses. Export resolveSlimmingConfig from
the core barrel.
- R4-5/R4-6: Add tests for configurable imageTokenEstimate and Infinity
threshold guard.
- R4-7: Guard oversizedThresholdChars against Infinity — when truncation
is disabled (threshold <= 0 → Infinity), report 0 instead of Infinity
so JSON.stringify does not drop the key to null.
- R3-3: Fix UI history budget resolution — IndividualToolCallDisplay.name
stores the tool's displayName (e.g. 'Shell'), not the registry key (e.g.
'shell'), so getTool(displayName) returned undefined and budgets fell
back to the global threshold. Build a displayName → maxOutputChars map
from getAllTools() at scan time.
- R4-12: Declare string-only scope for UI history scanning in code
comment and design doc §5 — structured display objects (file diffs,
ANSI captures, agent result summaries) are out of scope for phase 1.
- 3750020138: Update design doc §5 to reflect resolveSlimmingConfig
alignment and display-name map.
* fix(doctor): align UI scan to 2x tolerance, fix threshold/error/sentinel gaps
R5-1: threshold uses config.getTruncateToolOutputThreshold() first so
disabled (Infinity) doesn't fall to 0
R5-3: UI scan uses budget * COMBINED_PASS_TOLERANCE_FACTOR to match
API-history 2x tolerance
R5-7: oversized check uses rawChars (output.length) instead of
estimatePartChars (which adds 64 wrapper floor)
R5-8: read output ?? error and check <persisted-output> sentinel
R5-12: UI lookup consults ToolDisplayNamesMigration for legacy names
Tests: 3 core + 1 CLI updated for rawChars/2x; 3 core + 2 CLI added
Doc: test counts (core 14→19, CLI 9→11), scenario 4 JSON, 2x wording
* docs: fix design note accuracy (R5-9/R5-15/R5-10)
R5-9: clarify shell/MCP truncate in-tool before gate; split recovery paths
R5-15: note sentinel skip in combined-pass re-check; fix mermaid routing
R5-10: describe two-stage persist failure (fallback saves full payload)
* fix(doctor): add envelope slack for token-aware truncation fallback
R6 review: truncateAndSaveToFile's token-aware fallback returns the
original content (sentinel-less) when the wrapped form would not be
smaller, so the diagnostic's oversized check must tolerate that band.
- Export TRUNCATION_FALLBACK_ENVELOPE_SLACK=500 from truncation.ts
- Apply slack to the rawChars > 2x budget comparison
- Adjust 4 test fixtures broken by the new threshold boundary
- Fix 7 factual errors in design/E2E docs (mermaid, project-hash,
web-search 102k, re-entrancy guard, shell/MCP bullet)
- Add JSDoc caveat for MCP-disconnect limitation (R6-16)
* docs: align §5 sentinel description with diagnostic code
Bot R6-10 re-report: design doc §5 claimed three markers are skipped
(prefix, in-body marker, <persisted-output> stub), but the diagnostic
only checks two (prefix and <persisted-output> stub). Aligned the doc
to match the code. The re-entrancy guard bullet still describes the
scheduler's three-marker isAlreadyTruncated check, which is accurate.
---------
Co-authored-by: 俊良 <zzj542558@alibaba-inc.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
* fix(cli): improve slash command history feedback
* fix(cli): centralize auxiliary model guard
* test(cli): cover model picker left key
* fix(cli): persist slash command invocation hiding across resume
* fix: keep model picker feedback consistent
* docs(cli): align design doc with /stats root-level hiding
* fix(cli): preserve model picker history safely
* fix: preserve hidden command feedback on resume
* fix(desktop): match slash results to invocations
* fix: keep slash command feedback paired with invocations
- Record message-type command results through the recording wrapper so
rejections and errors land in the result record and replay on resume
- Move the NO_COLOR /theme rejection into the command action so the result
record is written after the invocation, and keep that invocation visible
because the command prints feedback instead of opening the dialog
- Skip the /auth result record when the dialog auto-opened without an
/auth invocation to pair with
- Guard the model picker against re-entering a selection while a switch
is already in flight
- Walk only uuid -> parentUuid links when matching desktop slash results
to invocations instead of retaining every parsed transcript record
* fix(desktop): bound slash result lookup to user turn
* test(cli): cover hidden command result replay on resume
* fix(cli): guard dialog result recording
* fix(cli,desktop): tighten slash-command feedback recording
- Pin the revealed invocation as the first history item in the
argument-validation reveal test, matching the processor's ordering.
- Derive the /model picker-flag pattern from a single shared flag list.
- Pair each transcript invocation with at most one result during desktop
reconstruction, so a later same-name orphan result (e.g. an auto-fired
recap) cannot re-emit an already-paired invocation's user row.
- Retarget the textElements guard at the synthesized slash user row.
* fix(cli): guard partially persisted model switches
* test(cli): cover isPickerOnlyModelInvocation regex boundaries directly
---------
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: Qwen Autofix <autofix@qwen-code.dev>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* feat(web-shell): show channel sessions in sidebar
* feat(web-shell): configure channel session scope
* fix(web-shell): refresh channel session transcripts
* feat(web-shell): group channel sessions by platform
* fix(channels): address session review feedback
* test(web-shell): harden channel session coverage
* test: pin remaining channel session review findings
Close the round-1 review gaps the previous fixes left unpinned: the
channel-base/acp-bridge display-text wire key now has a cross-package
equality assertion, the secondary-workspace catalog test covers pinned
channel sessions, the collect-coalescing test pins the raw display text
when buffered messages carry model-only metadata, the editor descriptor
labels diverge from the i18n values so missing keys surface, and the
channels e2e asserts the legacy fixture is absent from the Tasks tab.
* fix: harden channel session sidebar and display projections (#8457)
Round-2 review fixes:
- Guard the sidebar channel-instance lookup with Object.hasOwn so a
sourceId like 'constructor' cannot crash the Channels view through
Object.prototype (R2-1).
- Drop the {text:''} part synthesis for empty displayText replays so
image-only channel prompts stop emitting a spurious
unknown_record_or_part diagnostic and complete:false (R2-2).
- Route the webhook display projection through the same
truncate+sanitize treatment as the model prompt via a shared helper
(R2-3).
- Refresh the per-workspace channel catalog on the session poll tick
and render the channel list flat — never under organization groups —
while the catalog is unavailable (R2-4).
- Remove the dead prompt re-truncation in sessionService (R2-5).
- Extract getChannelPromptDisplayText as the single source of truth for
the echo, pending-entry, and re-arm display-text reads (R2-7).
- Add workspaceCwd to the channel e2e session fixtures (R2-8).
- Pin the archived source switch, the deleteSessionData validation
contract, and the stale-response ordering guard with tests
(R2-10, R2-11, R2-13).
* fix(channels): address session review feedback
* fix(channels): address channel session review feedback
* fix(channels): harden channel session review round-7 criticals (#8457)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(web-shell): harden first-visit session section latches (#8457)
* fix(channels): address review round-8 findings (#8457)
* test(web-shell): pin round-8 sidebar behavior fixes (#8457)
* fix(channels): address review round-9 findings (#8457)
* fix(web-shell): pin Other channels group after platform sections (#8457)
---------
Co-authored-by: 克竟 <dingbingzhi.dbz@alibaba-inc.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: 丁炳智 <dingbingzhi@Edenmans-Office-Mac.local>
Co-authored-by: qwen-code-autofix[bot] <qwen-code-autofix[bot]@users.noreply.github.com>
* feat(desktop): mirror releases to Aliyun OSS
* fix(desktop): harden OSS mirror workflow and tests
- Add ref guard to sync-desktop-to-oss.yml (dispatch only from main)
- Add diagnostic error messages for missing Windows/Linux installers
- Harden test: pin verify-index > 0 before ordering comparison
- Harden test: pin confirm-before-publish ordering and source comparison
- Add test: stable-only release validation in reusable sync job
* fix(desktop): harden OSS mirror permissions, stable-version guard, and non-latest repair
- Remove workflow-level actions:read; grant it only to the sync-oss caller job
- Reject suffixed versions for published stable releases in prepare
- Turn latest-feed comparison into a non-fatal check; condition publish/verify on match
- Assert both check_for_update call sites in release test
- Add jq stable-only guard assertion and endpoint default alignment test
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
---------
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(telemetry): align session lifecycle with OTel
* feat(telemetry): complete session lifecycle coverage
* fix(telemetry): deduplicate deferred session starts
* test(telemetry): cover duplicate session starts
* fix(serve): emit daemon session starts
* fix(telemetry): repair session lifecycle test wiring and record attributes (#8616)
Restore the missing logSessionEnd export in the config-session-env mock
(the Test-check failure), add event.timestamp to the session lifecycle
records to match every sibling emitter, and pin the previously untested
wiring: end-before-start ordering and the /clear non-continuation rule at
the Config level, the deferred-init catch-up guard, the shutdown emission,
and the loggers->session-events link. Correct the telemetry docs claims
about session.previous_id, end_session, and the log event catalog.
* fix(telemetry): skip session lifecycle transition on same-id resume (#8616)
* test(telemetry): pin session lifecycle behaviors per review (#8616)
Mutation testing in review round 4 showed three properties survived the
whole suite unguarded: the session-start guard resetting on session.end,
the daemon runtime config's isTelemetryInitializationDeferred flag, and
the catch-up session.start emitting only after NodeSDK.start(). Add
focused assertions for each, and replace a duplicated config mock literal
with the makeFakeConfig factory.
* fix(telemetry): emit session start catch-up on every init path (#8616)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
---------
Co-authored-by: zjunothing <zjunothing@users.noreply.github.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(core): resolve Qwen 3.8 reasoning budget conflicts
* fix(core): cover unconfigured Qwen 3.8 conflicts
* chore: preserve latest main formatting
* fix(core): harden DashScope thinking precedence
* fix: honor DashScope thinking knob precedence
* test(core): assert same-layer thinking knob drop warning for request pairs (#8525)
* fix: align effort override reporting with wire resolution
* fix: resolve thinking knob review findings
* fix: sort Python SDK test imports
* fix(core): ignore null thinking knobs
* fix(core): register enable_thinking true in thinking knob selection (#8525)
selectFromLayer only registered enable_thinking === false, so a
higher-priority enable_thinking: true was invisible to cross-layer
resolution: a lower-priority samplingParams disable won selection and
rewrote the shipping tier to reasoning_effort 'none', inverting the
documented extra_body > samplingParams precedence. Register the
on-switch as the weakest knob in its own layer (an off-switch rewrites
the tier, an on-switch never does) and make the drop branch
value-aware: true keeps the shipping tier and drops only the redundant
knobs, false keeps the canonical 'none' disable.
getReasoningEffortOverride no longer reports an on-switch as shadowing
the tier (the wire drops the switch and ships the tier), except for a
request-level effort override that still shadows from under it.
Also corrects the dropConflictingThinkingKnobs contract comment (only
effort tiers ship alone; the 'none' disable and a winning budget keep
a co-present enable_thinking) and the model-providers.md precedence
callout, which overstated samplingParams precedence for older qwen
hybrids where the reasoning-derived enable_thinking: true overrides it.
* fix(core): preserve budget beneath thinking on-switch
* fix(core): canonicalize disabled thinking knobs
* fix: resolve round-6 thinking knob review findings (#8525)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(external-context): read the response body with a reader, not for-await (#8525)
Port of #8764 (10621b3a93) to this branch. Async-iterating a
ReadableStream needs [Symbol.asyncIterator] on the TYPE, and whether it
is there depends on which lib set the program resolves — @types/node's
stream has it, the DOM lib's needs lib.dom.asynciterable. This branch
predates #8693, whose tsconfig "types" guard keeps @types/jsdom's
lib.dom out of the package program; the autofix verification build
resolves node_modules from the trusted base (which has @types/jsdom),
so the guardless branch fails the build with TS2504 on the `for await`.
The reader loop types identically in every lib set, so the build no
longer depends on that resolution. Behavior is unchanged and pinned by
the regression tests ported from the same commit: multi-chunk assembly,
the exact MAX_RESPONSE_BYTES boundary, oversize rejection with stream
cancellation, deferred-cancel sequencing, mid-stream read failure, and
invalid-UTF-8 rejection. The Mem0-related changes that share main's
http-client.ts (#8507) are intentionally not ported.
* fix(sdk-python): expose effort status reason from CLI (#8525)
The CLI emits a human-readable reason on effort_status and the
TypeScript SDK surfaces it, but the Python EffortStatus TypedDict and
_parse_effort_status dropped it, leaving Python callers to reconstruct
the reason from override. Add reason as an optional field and pass it
through, mirroring the TypeScript parser.
* test(core): add direct unit tests for selectDashScopeThinkingKnob (#8525)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
---------
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
* fix: add structured error code to SessionNotFoundError responses
PR #8864 retried session switches while the target session is closing,
but relied on fragile string matching against the daemon's error message.
This commit:
1. Adds a `code` property to `SessionNotFoundError` — automatically set
to `'session_closing'` when the extra message mentions "closing",
otherwise `'session_not_found'`.
2. Includes `code` in the HTTP JSON response body so clients can
distinguish closing (transient) from genuinely missing sessions
without depending on error message text.
3. Updates the WebUI retry check in `DaemonSessionProvider` to use
`errorBody.code === 'session_closing'` instead of matching
`endsWith('The session is closing; retry after close completes')`.
4. Fixes an inconsistent error message in `rewindSession` that used the
short `'The session is closing'` without the retry suffix.
Closes: #8864 (follow-up)
* fix(daemon): expose session closing code
* docs(serve): document session closing codes
* fix(acp): preserve closing code after restore waits
* fix: restore class pin in bridge test and update error taxonomy
- Add toBeInstanceOf(SessionNotFoundError) alongside toMatchObject
to preserve the envelope type assertion
- Document session_closing code in 18-error-taxonomy.md
* chore: drop unrelated merge formatting
* fix(cli): avoid duplicate context usage in footer and status line
The built-in default status line preset includes `context-used`, and the
footer renders its own context indicator unless `hideContextIndicator` is
set, so context usage was shown twice out of the box.
Treat `ui.statusLine.hideContextIndicator` as tri-state: an explicit
boolean still wins in both directions, and when it is unset a preset
status line containing `context-used` or `context-remaining` hides the
footer indicator. Command status lines are unchanged — their output is
opaque, so it is never inspected for context information.
Fixes#8695
* fix(cli): preserve status line context override
* fix(cli): preserve status line context semantics
* fix(cli): keep context visible in narrow footers
* fix(cli): keep context visible when status line clips
* fix(cli): preserve context indicator visibility
* fix(cli): match status line wrap layout
* refactor(review): extract the toolchain adapter boundary
`review build-test` combined three responsibilities in one module: reading
the review plan, deciding which toolchain can be verified deterministically,
and implementing npm workspace installation, affected-package selection,
dependency widening, build execution, test execution, and reporting.
Move the npm implementation behind an internal `ReviewToolchainAdapter`
contract. `build-test.ts` keeps CLI routing, plan reading, output trimming,
env shaping, and the spawn boundary; `lib/npm-toolchain.ts` owns npm
detection and the verification algorithm; `lib/disk.ts` holds the shared
free-disk floors.
This is a move, not a rewrite: 95% of the lines removed from build-test.ts
reappear verbatim in the new files. The CLI arguments, the BuildTestReport
JSON shape, and every npm behaviour are unchanged, and the existing
build-test suite is the compatibility oracle for that.
Selection requires exactly one applicable adapter and fails closed to the
`unsupported` handoff otherwise, so a second toolchain lands as a
registration rather than another branch in this file.
* docs(review): record the toolchain adapter boundary design
---------
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
* chore(serve): remove the /demo debug page
The daemon has shipped a real browser UI for a while: `resolveWebShellDir()`
finds the bundled Web Shell assets and `mountWebShellAssets()` serves them at
`/`, so `qwen serve` already opens onto a full client. `/demo` stayed behind as
a 663-line inline-HTML console covering the same ground with none of the
reach — nobody drives the daemon through it, and `npm run dev:daemon` starts
the Web Shell dev server rather than the demo page.
Keeping it around costs more than the dead code. It is the only file in the
tree that pairs an event log with daemon HTTP, so work that starts as a Web
Shell observation lands there instead: #8762 was found while running `/review`
through the Web Shell and was fixed entirely inside the demo page's rendering,
with "no Web Shell changes" in its own risk note. Deleting the page removes
that decoy.
Nothing is lost for protocol-level debugging: `GET /session/:id/events`
streams the same raw frames the Events tab printed.
`/health` shared `routes/health-demo.ts` with the demo handler, so the module
is now `routes/health.ts` / `createHealthRoutes()` and drops its `getPort`
dependency. The rate-limit exemption, the boot breadcrumb, and the daemon docs
lose their `/demo` arms; the loopback self-origin shim regression test already
asserted through `/health` and only needed its title corrected.
* test(serve): pin the removed /demo contract and the pre-auth surface
Review follow-up. Three of the removal hunks shipped ungated, and two doc
sentences the removal rewrote were describing the pre-auth surface wrong —
both before and after the edit.
Deleting the `/demo` route took its assertions with it, so nothing failed if
the handler came back: the Web Shell suite only exercised a generic deep link,
and the rate-limit exemption could be widened again with the suite still green.
`/demo` is now pinned as what it became — an ordinary unknown path: a
non-navigation request 404s, a browser navigation is answered by the SPA
fallback like any other deep link, and once a token is configured (with or
without `--require-auth`) that navigation is refused with 401, because the
fallback sits behind the bearer. The rate-limit test pins that `/health` is the
only exempt GET, so re-adding a second pre-auth page to the predicate fails
instead of silently escaping the limiter. Each new assertion was checked by
reverting the hunk it guards and confirming it goes red.
The `--allow-origin '*'` warning and both `--allow-origin` doc paragraphs
enumerated `/health` as the residual tokenless surface and said nothing about
the Web Shell static assets, which are mounted before the bearer in every
launch mode and stay reachable even under `--require-auth` — the enumeration
also claimed `/health` stays pre-auth on non-loopback binds, where it is
registered behind the bearer and 401s. A probe across all three launch modes
established the actual matrix; the warning and the docs now match it and name
`--no-web` as the way to remove the residual browser surface. The warning text
is asserted by a test for the first time.
* fix(serve): correct Web Shell doc claims and re-pin the pre-auth CORS wall
Review follow-up. The removal rewrote the daemon docs around the Web
Shell, and three of the rewritten claims did not match what the runtime
actually does: §1 never said how the bearer reaches the browser (with
auth on, the plain URL loads a shell whose every API call 401s), §8
called the shell writable on any bind (on a non-loopback bind without
`--allow-origin` its POSTs hit the CORS wall and 403), and §8 served
`/session/:id` without the document-navigation qualifier its own code
enforces. The §9 call-chain diagram also still listed the deleted
`/demo` route, the developer flag references had no `--web`/`--no-web`
row despite the new guidance pointing at the flag, and both design docs
listed the JSON body parser ahead of post-auth `/health` while
`createServeApp()` registers them the other way round.
The deleted `/demo` CORS test was also the only assertion that a
pre-auth page sits behind the Origin wall — every surviving Origin test
targets an API path. Re-pin it for the shell root so a mount-order
regression fails instead of exposing the pre-auth HTML surface
cross-origin.
* fix(serve): finish demo rename sweep and scope pre-auth shell claims to loopback
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
---------
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* feat(cli): add background cleanup for OpenAI API logs
With model.enableOpenAILogging on, every API call appends a full
request/response JSON under logs/openai with no rotation — heavy usage
accumulates hundreds of thousands of files (tens of GB) within months.
Register a third cleaner in the existing background housekeeping
pipeline that sweeps openai-*.json files older than the new
model.openAILogRetentionDays setting (default 7 days). The
filename-embedded UTC date is used as a fast path to avoid one stat()
per file; the boundary day and unparseable names fall back to mtime.
Throttling is keyed on the resolved log dir, so both the default
per-CWD layout and a shared custom openAILoggingDir are swept at most
once a day. The sweep runs regardless of whether logging is currently
enabled, so residue from earlier debugging sessions still gets cleaned.
Scope note: housekeeping only starts for interactive sessions, so
headless (-p) / SDK processes are not covered yet.
* codex: address PR review feedback (#8862)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
---------
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(cli): extend the #8663 loader denylist and harden its scrub lifecycle
Follow-up to #8663. Its inherited-env denylist closed the NODE_OPTIONS/
NODE_PATH class but left sibling code-execution and TLS-trust-anchor vars
that reach the same #8653 cross-workspace outcome — an untrusted workspace
`.env` is frozen into daemonRuntimeBaseEnv and distributed to every
workspace's session subprocesses.
Denylist additions, split by the PR's own tiering:
- Scrubbed loader tier (INHERITED_LOADER_ENV_KEYS — scrubbed from the
inherited launch env and rejected from every `.env`/settings.env scope),
for pure-injection vars with no legitimate operator-shell use:
OPENSSL_CONF (startup dlopen of an attacker OpenSSL engine),
NODE_REPL_EXTERNAL_MODULE, npm_config_node_gyp, npm_config_init_module.
- Reject-from-project-`.env` tier (PROJECT_ENV_HARDCODED_EXCLUSIONS —
rejected from project files, preserved from the shell / home `.env`), for
vars with a legitimate operator-shell use whose only exposed vector is an
untrusted project file:
* TLS trust anchors SSL_CERT_FILE, SSL_CERT_DIR, CURL_CA_BUNDLE,
REQUESTS_CA_BUNDLE, GIT_SSL_CAINFO (siblings of NODE_EXTRA_CA_CERTS;
an attacker CA MITMs a session's git/npm/pip/curl traffic).
* git command-execution family GIT_SSH_COMMAND, GIT_EXTERNAL_DIFF,
GIT_CONFIG_GLOBAL/SYSTEM/COUNT and the numbered GIT_CONFIG_KEY_<n>/
GIT_CONFIG_VALUE_<n> pairs (matched by prefix). core/utils/git-branches.ts
already scrubs these from the repo's own git invocations.
* node-gyp interpreter selection NODE_GYP_FORCE_PYTHON, npm_config_python,
PYTHON (run as the build Python during native-addon installs).
Concurrency: the daemon's process.env scrub/restore and the loader-key
rejection reporter were process-global with no guard for concurrent embedded
daemons in one process (a documented supported config). The first daemon's
close() restored loader vars into the shared env, re-poisoning a still-live
sibling's sessions, and dropped its reporter. The scrub is now reference
counted (acquireInheritedLoaderEnvScrub — snapshot on first acquire, restore
only on last release) and the reporter is cleared only when still active.
Test hardening from the same review: pin the daemon-worker scrub breadcrumb
(not just key removal); pin the fast-path settings.env case-folded
hardcoded-exclusion gate; drain the module-global fast-path stash so the
accumulate assertion is order-independent. Docs updated for the new keys.
* fix(cli): keep the loader-scrub process.env access in the serve guard surface
The refcounted acquireInheritedLoaderEnvScrub read/wrote process.env from
config/shared-env-keys.ts, which the serve process.env guard does not scan —
moving the access out of run-qwen-serve.ts dropped its allowlisted count and
failed process-env-guard.test.ts. Pass the env into the coordinator instead so
run-qwen-serve.ts still owns the process.env reference (matching the existing
scrub helpers), and update the allowlist to the new count.
* fix(cli): block GIT_SSH and GIT_CONFIG_PARAMETERS in the project-env denylist
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(cli): extend the project-env denylist across git exec, TLS, and rc-file tiers
Close the round-2 review findings: block the remaining git
command-execution siblings (GIT_EXEC_PATH, GIT_TEMPLATE_DIR, GIT_ASKPASS,
GIT_PROXY_COMMAND, GIT_EDITOR), the npm/pip TLS trust knobs
(npm_config_cafile, npm_config_ca, npm_config_strict_ssl, PIP_CERT,
GIT_SSL_CAPATH), and the curl/wget rc-file redirects (CURL_HOME, WGETRC)
from project .env files. Freeze the numbered GIT_CONFIG_KEY_/VALUE_ pairs
on reload together with GIT_CONFIG_COUNT, and sync the qwen-serve.md
loader-key enumeration with settings.md.
* fix(cli): harden the project-env denylist and nested scrub snapshot (#8763)
* fix(cli): merge the loader-env scrub snapshot into one pass (#8763)
acquireInheritedLoaderEnvScrub iterated process.env twice (a snapshot
pass, then the scrub); record the originals inside the scrub's single
pass instead. Drop the acquire-time snapshot clear, which the
release-time clear made unreachable defense, and add tests that kill
the previously surviving mutants on the release-time clear, the
test-only reset, and the undefined-value guard.
* fix(cli): block the round-4 exec-redirect env keys from project files (#8763)
---------
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(acp-bridge): bound live journal replay chunks
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* test(core): isolate shell retention sidecars
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* test(integration): cover aggregated live journal replay
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* test(core): isolate registry sidecars
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(acp-bridge): keep unmodeled chunk keys out of live journal merges
The merged live-journal entry is rebuilt by spread-merging the first and
last source events, which was only safe because producers happen to emit
exactly {sessionUpdate, content, _meta?} on mergeable chunks. Gate the
merge on that key set so unmodeled data/update fields keep entries
discrete instead of leaking into the aggregate. Also clarify the
live-journal truncation marker: its retained/truncated counts describe
source events, while the limits count replay entries.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(acp-bridge): align replay boundaries for discrete and meta-shaped chunks
Turn compaction folded discrete thought chunks (and non-todo-stop-guard
discrete messages) into one text slot with the last chunk's meta, while
the live journal keeps every discrete chunk separate — resyncing from
compactedReplay mis-attributed text across background tasks. Guard both
chunk paths with the same hasDiscreteMessageMeta predicate the live
journal already uses. Also align the merge gate with the shapes the
shared meta builder emits: tolerate update-level timestamp/
serverTimestamp and qwenTranscript.planToolCallId, and treat an
empty-string parentToolCallId as top-level the way the extractor does.
Document that byte-cap truncation drops whole entries, so the retained
tail can be much smaller than the cap, and tighten the integration
assertion that became vacuous once entries merge source chunks.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(acp-bridge): merge subagent chunks in live journal replay
SubAgentTracker stamps every streamed subagent fragment with
{ parentToolCallId, subagentType }, but the live-journal merge gate
only modeled parentToolCallId, so subagent chunks stayed discrete and
a high-fragment subagent stream could still trip history_truncated.
Model subagentType as a carried label (like the completed-turn path,
which merges by parentToolCallId alone) and cover the producer wire
shape in the merge tests.
* fix(acp-bridge): preserve TextContent metadata in live journal replay (#8801)
---------
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* feat(web-shell): support image drag and drop
Allow Web Shell composers to ingest image files reliably while
preserving the existing multimodal prompt protocol.
- Share ordered image ingestion across desktop and mobile editors
- Support image-only prompts and BMP preview and provider-safe handling
- Preserve queued payloads across retries and uncertain outcomes
- Add lifecycle guards, user feedback, unit coverage, and browser tests
* fix(web-shell): harden image prompt admission recovery
Preserve complete prompt payloads and prevent duplicate or uncertain
delivery states when admission responses race with queue lifecycle
events.
- Correlate admission, queue, and terminal events by prompt ID
- Restore images and input annotations across retry and edit flows
- Bound image reader concurrency and encoded attachment memory
- Reconcile confirmed removals and explain ambiguous queue entries
* docs(web-shell): align image drag design with review fixes
Document the reviewed admission, recovery, and resource invariants.
Keep the design aligned with the hardened Web Shell implementation.
- Record bounded image ingestion and encoded-data budgeting
- Clarify prompt lifecycle correlation and confirmed removal behavior
- Describe annotation restoration and internal action boundaries
- Update focused validation evidence and acceptance criteria
* fix(web-shell): avoid duplicate restored attachments
Skip payload attachments when restoring text is a no-op because the
same prompt text already exists in the composer.
- Restore images and annotations only when their text is inserted
- Preserve image-only restoration regardless of the current draft
- Add regression coverage for duplicate text with attachments
---------
Co-authored-by: heyang.why <heyang.why@alibaba-inc.com>
* perf(review): guarantee compose survives a reverse-audit budget stop
PR #8687 — a 4,269-line cross-worktree git guard — timed out after six
hours and posted nothing, holding ~20 E2E-confirmed Critical bypasses.
The deadline gate worked: it refused round 3 correctly with ~110
minutes and the whole reserve in hand. The tail after the stop was the
killer — a single hand-rolled verification agent re-running a 15-family
shell/git bypass battery with real filesystem E2E consumed all of it,
and the wall hit mid-verification before compose-review ever ran.
The reserve was one number covering "verification + compose + submit",
which is right for a normal per-finding re-trace but wrong for a
security PR where verification cost is unbounded (real E2E per finding)
while compose and submit stay bounded. So a distinct, smaller compose
FLOOR is carved out and the VERIFIER — not the reverse-audit builder —
is gated on it: below the floor `agent-prompt --role verify` refuses to
build (VERIFY BUDGET, exit 4), the findings keep their `— [unverified]`
tag for compose-review to cap, and compose runs. The floor is strictly
below the reserve, so a healthy run reaches the reverse-audit gate first
and never sees it; it is the cover for the one span the reserve cannot
bound.
The prose closes the bypass the gate cannot see: the post-stop tail
verifies only through the gated builder, never a hand-rolled agent, and
invents no fresh re-verification pass for findings already confirmed —
compose and submit are non-negotiable. DESIGN.md records the incident;
the RA budget message and SKILL Step 5 tail are rewritten to match.
* fix(review): close the round-1 gaps in the compose-floor gate
- R2-2 (Critical): the documented `0` escape hatch did not disable the
verify gate past the deadline — `remainingSeconds` goes negative there
and `negative >= 0` is false, firing the supposedly-disabled gate.
verifyBudgetExhausted now returns null the moment the effective floor is
0, before the comparison. Pinned with a past-deadline case.
- R2-1 (Critical): the gate bounds prompt CONSTRUCTION, not the wall time
of an already-admitted verifier that then runs a long E2E past the
floor — and agent-prompt builds prompts, it cannot cancel a running
agent. The SKILL tail now tells the orchestrator to bound the WAIT: when
the deadline is within the compose floor and a verifier batch has not
returned, stop waiting on it, keep its findings unverified, and compose.
The remaining execution-time cancellation is a harness capability, noted
as such (same layer boundary as the hand-rolled-agent caveat).
- R2-3: the agent-prompt exit-code help now documents both the BUDGET and
VERIFY BUDGET exit-4 refusals.
- R2-4: the reverseAuditBudgetMessage test now pins the new tail rules
(gated verifier only, no hand-rolled agent, no re-verification).
- R2-5: docs/users/features/code-review.md documents the compose floor —
default, env var, reserve nesting, exit-4 behaviour, zero hatch.
* fix(review): round-2 fixes for the compose-floor gate
- R3-1 (Critical): the verify gate admitted at exactly the floor, where
the first work crosses below it — the floor is compose-only with no
margin, so it now refuses at equality (`> floor`, unlike the RA reserve
which admits at exact cover). Exact-boundary test flipped.
- R3-2 (Critical): the refusal message and SKILL claimed unverified
findings "post as needing human review", but the confirmed-only rule
keeps tagged details terminal-only. Reworded to the true contract:
compose-review caps the verdict and discloses the verification gap; the
tagged details stay terminal-only; what posts is the earlier rounds'
confirmed findings plus that gap.
- R3-5: extracted readDeadlineSeconds / readNonNegativeSeconds, shared by
both gates so the fail-open contract lives in one place.
- R3-3: pinned the verify gate's fail-open branches (malformed/non-positive
deadline, past-deadline negative remaining, negative-floor fallback).
- R3-4: pinned the floor-minutes rendering (a field swap to remainingSeconds
would misstate the protected floor).
- R3-7: pinned that a refused verifier writes no budget-stop marker and no
admission stamp.
- R3-8: pinned validation-before-gate (a malformed verify call under the
floor throws, not exit 4).
R3-6 needs no change: the SKILL.test pointer<->heading gate already covers
the DESIGN section (a dangling pointer fails it).
* fix(review): round-3 cheap fixes for the compose-floor gate
Low-risk corrections; the two edge-case Criticals (R4-1 broken-plan
masking, shared with the RA gate; R4-2 compose-review relaunch FIX) are
left as follow-ups — noted on the threads.
- R4-4: the readDeadlineSeconds extraction stranded reverseAuditBudgetExhausted's
contract JSDoc above the helper; moved it back onto the function.
- R4-5: the round-2 "terminal-only, never posted" wording contradicted
compose-review's own verdict line ("posted, disclosed as unverified") —
a pre-existing contract ambiguity this PR should not relitigate. Reworded
the message and SKILL to the invariant both readings share: an unverified
finding is never treated as a confirmed blocker; the verdict is capped.
- R4-7: "below the N-minute floor" contradicted the exact-equality refusal
(the gate admits on `> floor`); now "at or below", in the message and the
user docs.
- R4-3: pinned that a blank/whitespace floor override falls back to the
default (only explicit 0 disables).
- R4-6: pinned the negative-remaining clamp in verifyBudgetMessage.
---------
Co-authored-by: verify <verify@local>
* feat(desktop): create default workspace on first launch
* test(desktop): cover default workspace failure
* feat(desktop): honor QWEN_DEFAULT_WORKSPACE_DIR for the default workspace
* fix(desktop): defer default workspace creation; kill in-flight runtime on stop
Move default-workspace directory creation out of setup into the runtime
start blocking task, so the first touch of ~/Documents (which can raise
the macOS TCC prompt) no longer blocks the main thread. Path resolution
(initial_workspace/default_workspace) is now pure and reports whether the
derived default directory must be created; creation failures surface
through the existing runtime-failed path.
Register the spawned runtime child in a shared pending handle before the
startup wait, so app exit, restart, and generation switches can kill an
in-flight daemon instead of orphaning it in its own process group. The
startup wait loops poll the shared handle and treat a taken child as a
stop during startup.
* fix(desktop): close runtime startup cancellation race
* fix(desktop): preserve default workspace retries
* fix(web-shell): stop rendering unrecognized daemon events in transcripts
The daemon UI normalizer projects any frame it has no case for into a
`debug` event carrying a raw JSON dump. webui's ChatViewer drops those
blocks, but Web Shell renders `status` and `debug` together as system
info, so every event kind the daemon ships ahead of the UI surfaces as
unreadable JSON in the middle of the conversation. This has been patched
per-symptom three times now: two string-prefix suppressions inside
`isIgnoredWebShellStatus`, plus #8790 for `usage_update`.
Give the normalizer's debug events a structured `debugReason` and let
Web Shell branch on it instead of pattern-matching text:
- `unrecognized_event` / `unrecognized_session_update` — the daemon runs
ahead of this client; developer diagnostics, not conversation content.
Web Shell no longer renders them.
- `malformed_payload` — a frame the client does know arrived unusable.
That is a real defect signal, so it stays visible.
Debug events dispatched by clients themselves, such as Web Shell's own
model-switch summary, carry no `debugReason` and keep rendering.
The two `(unrecognized daemon event)` prefix checks are now covered by
`debugReason` and are removed; the `Model switched: ` check stays, since
`model.changed` projects to a `status` block rather than a debug one.
* fix(sdk): classify a discriminator-less session_update as malformed
Review of #8812 caught a hole in the new classification: `session_update`
payloads such as `{}` or `{ sessionUpdate: 42 }` reach the default branch
with `kind === undefined`, and stamping them `unrecognized_session_update`
made Web Shell hide the only diagnostic a malformed frame produces.
Reserve the unrecognized reason for a real unknown string kind.
Also update the top-level default-case comment, which still pointed
adapters at the debug text prefix, and add a reducer-level test proving
`debugReason` survives the UI-event → transcript-block boundary: the
normalizer tests inspect events and the Web Shell tests build blocks by
hand, so dropping the spread in transcript.ts would leave both green.
* fix(web-shell): keep filtering legacy debug blocks, tighten the reason split
Four review findings from #8812:
- `WebShellTranscript` is a public entry point taking already-projected
blocks, so blocks from an SDK predating `debugReason` still arrive with
no reason and started rendering again when the prefix checks were
removed. Fall back to the stable ` (unrecognized daemon event): ` marker
when no reason is present — which covers every unrecognized event type,
not just the two previously suppressed by name. The old-shape fixture is
restored (adding `debugReason` to it had hidden this path) and a
dedicated legacy test now pins it.
- A whitespace-only discriminator is truthy, so `sessionUpdate: ' '` was
classified unrecognized and hidden. Gate on `trim()`, matching the
convention `getFirstString` already uses.
- Add the mirror invariant for the reducer: a client-dispatched debug
event must produce a block with no `debugReason`. Defaulting the field
in `appendStatusBlock` otherwise passes every other test while tagging
the model-switch summary unrecognized.
- Guard the outermost public re-export. A type-only guard would not hold —
vitest erases `export type` through esbuild and this package's tsconfig
excludes `test/` — so ship the union as `DAEMON_UI_DEBUG_REASONS`,
matching `DAEMON_ERROR_KINDS`, and assert it at runtime.
* fix(web-shell): suppress legacy usage_update/a2ui blocks with no debugReason
Follow-up verification on #8812 pointed out the marker fallback does not
close the original report. #8790 stopped the SDK inserting new
`usage_update` blocks, but `WebShellTranscript` renders whatever blocks its
caller passes, so a transcript persisted or projected before that still
holds them and the spam returns after upgrade.
The legacy `session_update` projection is `<kind>: <json>` with no marker to
key on, so match those by kind name instead. The list is closed on purpose —
`usage_update` and `a2ui`, the two known to have leaked — and requires the
`: {` shape, because a generic `<word>: {` rule would swallow legitimate
diagnostics. Blocks the normalizer classified still win on `debugReason`,
so `malformed_payload` and client-dispatched debug blocks stay visible.
Mutation-checked in both directions: dropping the fallback fails the legacy
test, and loosening the prefix to bare `usage_update:` fails the test that
pins prose and classified blocks staying visible.
* fix(web-shell): match the legacy projection shape, not a quoted marker
The legacy fallback was too broad in two ways, both reachable. It ran for
`status` blocks as well as `debug` ones, because this helper is called from
the shared `case 'status': case 'debug':` arm, and it matched the marker as
a substring anywhere in the text.
Probed at df0b757c3c: all four of these were dropped — a status line
quoting the marker, a legacy malformed payload relaying an upstream peer's
message that contains it, a client-dispatched summary quoting it, and a
status block whose text starts with `usage_update: {`. Exactly the
diagnostics this PR promises to keep.
Scope the text match to `debug` blocks, and anchor it to the whole legacy
projection (`<event-type> (unrecognized daemon event): <json>`) instead of
the bare marker. Classified blocks still win on `debugReason` before any of
this runs.
Mutation-checked: dropping the `kind === 'debug'` guard and restoring the
substring match each fail the new negative test.
* fix(web-shell): match legacy projections with non-object payloads
The anchored legacy pattern required the payload to start with `[`, `{` or
`"`, which only holds for objects and arrays. `DaemonEvent.data` is
`unknown`, and `stringifyJson` returns strings verbatim, serializes
primitives as `42` / `true` / `null`, and yields `''` for `undefined` — so
every non-object payload bypassed the compatibility fallback and rendered.
Drop the leading-character constraint. The event-type prefix plus the fixed
phrase, anchored at the start, is specific enough on its own, and the
negative cases for quoted markers and status blocks still pass.
Regression test covers object, array, string, number, boolean, null and
empty payloads. Mutation-checked: restoring the character class fails it.
* fix(web-shell): hide debug blocks by the unrecognized_ reason category
The debugReason filter enumerated the two current `unrecognized_*` values,
but the SDK contract this PR adds names reasons by category: `unrecognized_*`
is forward-compat noise to hide, `malformed_*` a defect signal to keep
visible. A reason a newer SDK adds would compile silently against the
two-literal comparison and render raw JSON again with both suites green.
Match the category prefix instead. Also drop the now-dead marker branch in
MessageList's mid-turn hide check: every block carrying that prefix is
filtered upstream in the adapter (reason-stamped via `debugReason`, legacy
via the anchored pattern), and the dedicated normalizer case emits a status
event keyed by `source`. Document `DaemonUiDebugReason` beside the sibling
closed enums in the daemon-ui docs, whose forward-compat bullet still
described the unstamped projection.
Regression test pins both directions of the category contract with reasons
outside the current enum.
---------
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
* fix(serve): make session restore timeouts safe
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(cli): restore missing core mock exports in the ACP worktree suite
The restore-tracing change added `extractDaemonTraceContext` and
`withDaemonSpan` to `acpAgent.ts`, but `acpAgent.worktree.test.ts`
replaces `@qwen-code/qwen-code-core` with a full mock factory that never
listed them. `loadSession` then failed on an undefined export, taking all
three cases down and producing teardown rejections from the half-built
agent. The sibling suite was updated; this one was missed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(serve): bound and disambiguate the abandoned restore lifecycle
Four follow-ups from review of the restore timeout work.
A startup budget may now raise the restore budget but never lower it.
Taking an explicitly configured `initializeTimeoutMs` as the restore
fallback meant a deployment that tightened its child-initialize check
still inherited a sub-default restore deadline — exactly the failure this
change exists to remove. An explicit `sessionRestoreTimeoutMs` still wins
outright, including below the default, for deployments that want restore
to fail fast. Validation now names the field actually at fault.
A restore fenced behind a timed-out predecessor is no longer reported as
an ordinary in-flight restore. It carries `reason:
awaiting_abandoned_cleanup` and a retry hint of one restore budget
(capped at 120s) instead of the ordinary 5 seconds, because the fence
cannot clear until the non-cancellable ACP request settles and a 5-second
cadence just spins the caller against a 409 it cannot resolve.
Whether a channel is condemned is now derived rather than sticky. A
timeout recorded `emptyReapPending` permanently, so any channel that had
ever seen one was guaranteed to be reaped once its remaining work
drained, forcing a cold respawn even when the late restore had landed and
closed cleanly. The reap condition is now computed from an outstanding
`unsettledAbandonedRestores` set, quarantine, or an ordinary pending
empty reap; real settlement clears the entry and hands the channel back
to the configured idle policy.
Abandonment no longer retains ownership without bound. One further
restore budget after the deadline, a still-unsettled restore marks the
channel `restoreSettlementOverdue`: existing sessions and workspace
control keep working, but fresh session work is refused so the channel
can drain, since closing the transport is the only lever that releases a
permanently hung request. Releasing capacity while hidden work runs would
allow unbounded oversubscription, and force-killing a channel with live
siblings would reintroduce the failure this work removes, so neither is
done. Fresh-admission blocking is now scanned across alive channels
rather than tracked in a single reference, so a second condemned channel
cannot silently displace the first.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(serve): keep the abandoned restore lifecycle off ids it no longer owns
Two correctness gaps in the abandoned-restore machinery introduced by this
PR, both reported by automated review and both confirmed by mutation
testing (each new test fails when its fix is reverted).
A caller-supplied `sessionId` is used verbatim by the agent, but
`spawnOrAttach` never consulted `inFlightRestores`. A fresh spawn could
therefore take an id that a restore still owns, in either lifecycle phase.
The consequences were silent: `abandonedRestoreIds` suppresses session
updates, guardrail events, and child notifications, so the new session
would have registered successfully and then emitted nothing; and a late
`settleAbandonedRestore` would have closed and tombstoned it out from
under its owner. Such a spawn is now rejected with the same
`RestoreInProgressError` and reason the restore path uses, so the caller
gets the correct retry hint for whichever phase is holding the id.
The cleanup path is guarded independently, because the request-level check
only covers the id the caller asked for and a session registers under the
id the child returns. An abandoned restore never reaches
`createSessionEntry` — the deadline rejects before registration — so any
live entry under that id belongs to someone else. Cleanup now detects that
and returns without closing or tombstoning, releasing its own bookkeeping
instead.
The notification fence has no TTL and was only cleared by
`markRestoreInFlight`, which covers a subsequent restore and nothing else.
`createSessionEntry` now clears it for every registration route, so a
legitimate owner of the id is never handed a session that silently drops
everything the child sends it.
Also tightens two tests that could not observe the values they pin. The
SDK default restore timeout admitted any value in (30s, 70s]; it is now
split at the exact boundary, so collapsing the default onto the 60s server
budget — which would make the client abort race the daemon's own deadline
and cost the caller its structured 504 — fails. And the advertised-budget
propagation from capabilities through to the SDK call had no live-path
assertion; dropping the capabilities argument at the real call site left
every existing test green. The `as never` casts are replaced with typed
`DaemonCapabilities` values so a field rename fails typecheck.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(serve): let a condemned channel drain without its wedged child
Merging main's active-work close protocol (#8588) into this PR's abandoned
restore bound produced a deadlock that neither side has on its own, and the
conflict resolution was committed without running tests.
`maybeCloseIdleSession` now routes through `confirmChildUnheld`, which asks
the child whether it still holds work before closing a session nobody is
attached to. That is right in general and wrong for a channel this PR has
already condemned. `restoreSettlementOverdue` and quarantine exist precisely
because the child stopped being answerable, and their whole premise is that
visible work drains so the channel can be reaped — closing the transport is
the only thing that can release a restore we cannot cancel. Making that
drain depend on a round trip to the wedged child inverts it: a child stuck
in a non-cancellable restore is exactly the one that cannot reply inside
`ACTIVE_WORK_CLOSE_TIMEOUT_MS`, so the sessions never close, the channel
never drains, the reap never fires, and the bound never takes effect.
A channel condemned by the restore lifecycle now skips the round trip and
proceeds to local teardown. Nothing is attached to the session by then —
`maybeCloseIdleSession` gates on that — and the sibling-safety invariant is
untouched: this closes sessions whose clients have already left, it does not
force-kill a channel that still has live ones.
The regression test drives an overdue channel whose child never answers the
close-if-unheld probe and asserts the detach still reaps it. Reverting the
guard reproduces the deadlock as a test timeout.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(serve): pin the restore-timeout contract the review found unasserted
Automated review identified eleven places where the restore-timeout work's
behavior was correct but unpinned — each with a mutation that ships green.
Every fix below was verified the same way: apply the mutation, watch the new
assertion fail, revert, watch it pass.
The timeout path's telemetry had no coverage at all, which is the sharpest
gap given that observability is what this work exists to deliver. A shared
recorder now asserts the public timeout result and its kill_empty-vs-
fence_shared signal, the late arrival, and the cleanup outcome for both the
closed and quarantined cases.
The deadline timer's cancellation on a successful restore was likewise
unpinned: deleting both `clearTimeout` calls kept the whole suite green,
while in production the stale timer fires one budget after a successful
restore and abandons a live session — fencing its frames, closing its event
bus, and emitting a spurious timeout. A success-path test now advances past
the deadline and asserts no second public result.
Three more bridge assertions proved less than they claimed: the concurrent-
restore case never checked that the abandoned restore settles, the
workspace-control case never checked that the deferred reap eventually
fires, and the resolver never pinned the accepting side of the MAX boundary
(a `>` to `>=` mutation rejects the largest legal delay at boot). The
workspace-control case also needed a positive channel idle budget, since
with the default zero the idle-timer kill substitutes for the reap junction
under test; its assertions are rewritten around the derived reap semantics
rather than the sticky flag they predate.
Outside the bridge: the scheduled-task timeout wiring had no test, so
deleting the arguments silently fell back to the helpers' own defaults; the
cold restore path never asserted that `live_restore_ms` is absent; the SDK's
per-request validation and its over-ceiling clamp were untested; the WebUI
watchdog test jumped straight to its own value, staying green for any
watchdog at or below it, including the 30s attach value that would recreate
the original symptom in the browser; and the two new known error types were
unexercised, so dropping either would relabel every restore-timeout and
quarantine error as unknown.
Two review items are deliberately not taken here and are recorded in the
design doc's non-goals instead: transcript materialization is still not
separately attributable from `config_setup`, which needs instrumentation
inside the core session loader that P1/P2 restructures anyway, and sibling
event-loop latency during a large restore remains unmeasured.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(serve): bound the condemned-channel close and complete the fence contract
Second automated review round, on the code the first round produced. One
Critical and twelve suggestions; all verified by mutation before and after.
**The Critical is a regression I introduced.** Letting a condemned channel
skip the bounded hold probe routed it into `closeSessionImpl`, whose agent
close is unbounded when it throws on failure — so the fix traded a bounded
wait on a wedged child for an unbounded one. A settlement-overdue channel
with an unresponsive child would hang `detachClient` forever, strand the
session in `closing`, never drain, never reap, and 503 every new session
until restart: strictly worse than before. `CloseSessionOpts` now carries an
`agentCloseTimeoutMs` that the condemned path sets, so a hang lands in the
existing unknown-outcome recovery, which kills the channel — the teardown
the drain was waiting for. The earlier test missed this because its fake
child still answered the plain close; it now answers nothing at all, and
asserts the detach itself returns.
**The fence was invisible on the transports clients actually use.**
`toRpcError` had no `RestoreInProgressError` case, so over acp-http and
acp-ws — which SDK negotiation prefers over REST — the fence degraded to an
opaque internal 500 with no code, reason, or hint, and the backoff contract
this work documents was impossible to honor.
**Two retry hints still advertised five seconds for states that outlive a
budget.** The restore 504 creates the fence, and quarantine lasts until the
channel drains; a fresh-id caller never reaches the 409 that carries the
real hint, so its header was the only signal it got. Both now derive from
the budget through one shared clamp helper, which also replaces the formula
that was inlined in the bridge and gives the documented 5-120s bounds a
test.
**A spawn collision reported an operation the caller never issued**, naming
the restore owner's action as both the active and the requested one and
telling the caller to retry an endpoint it never called.
The rest: five places still described the initialize-timeout fallback as a
plain chain rather than raise-only, contradicting sibling docs shipped in
this same PR; the design doc omitted the retry-hint clamp; the protocol
reference omitted the new spawn emission site; the error taxonomy omitted
`restore_settlement_overdue`, which matters because its audience is
monitoring. Test-only gaps: the dynamic 409 had no HTTP-layer coverage, the
120-second cap was unpinned, and the SDK's precedence of an explicit global
timeout over the advertised budget was pinned only branch-by-branch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(serve): preserve restore session ownership handoff
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
---------
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
* feat(cli): add Local Control pairing
* fix(cli): address Local Control review feedback
* fix(cli): allow Local Control loopback origin
* feat(desktop): add Local Control pairing
* fix(local-control): bound unauthenticated connections
* test(desktop): allow Windows proxy cleanup
* test(desktop): avoid socket cleanup timing
* fix(desktop): surface Local Control status
* fix(desktop): simplify Local Control window
* fix(desktop): harden Local Control pairing
* fix(desktop): bind Mac wake lock to app
