Commit graph

261 commits

Author SHA1 Message Date
易良
b5577b7d11
fix(sdk): route unrecognized diagnostics onto a bounded transcript sidechannel (#9202)
* fix(sdk): route unrecognized diagnostics onto a bounded transcript sidechannel

Normalizer-classified unrecognized_event / unrecognized_session_update debug events no longer enter transcript blocks[]: they are mirrored onto a capped unrecognizedDiagnostics sidechannel instead. This stops them from finalizing a streaming assistant/thought block (which dropped a following assistant.usage frame) and from consuming the maxBlocks budget (which let repeated noise evict real conversation content). malformed_payload diagnostics and client-dispatched debug events keep their existing block semantics.

* fix(sdk): align browser bundle budget

* fix(sdk): close the sidechannel review round (#8823)

- export the sidechannel API through the daemon barrel
  (selectUnrecognizedDiagnostics, UNRECOGNIZED_DIAGNOSTICS_LIMIT,
  DAEMON_UI_UNRECOGNIZED_DIAGNOSTIC_REASONS + types) and pin the
  reachability in daemon-public-surface.test.ts
- restore the MAX_TEXT_BLOCK_LENGTH cap on sidechannel text, mirroring
  truncateText exactly (suffix fits within the cap)
- ship the unrecognized reason subset as a runtime const array and route
  by membership, so a new reason cannot fall through to appendStatusBlock
- copy the correlation fields createBase stamps (promptId, sourceRecordIds,
  branchRecordId, originatorClientId) onto sidechannel entries; drop the
  dead source/data switches
- un-fuse the budget-history comment chain in scripts/build.js
- update docs/developers/daemon-ui for the split routing
- tests: full entry shape, text cap, block-path debugReason counterpart,
  and a webui malformed_payload interleave sibling so the #7012
  flush-before-guard keeps a discriminating stimulus

* fix(sdk): address round-2 sidechannel review for #8823

- build.js: bump daemon browser bundle budget 191KB -> 192KB
  (195,591 bytes measured > 195,584 cap; build failed at head)
- webui: narrow the observer-mode debug guard so unrecognized_*
  diagnostics reach the reducer sidechannel; only block-path debug
  events are dropped
- webui: merge history-store unrecognizedDiagnostics in
  applyTranscriptHistory so paged-back sessions keep diagnostics
- transcript: extract truncateTextAtLimit shared by the block and
  sidechannel truncation paths
- transcript: reset unrecognizedDiagnostics on rewind alongside the
  sibling per-turn state resets
- types: rename DaemonUnrecognizedDiagnostic.receivedAt to
  clientReceivedAt (matches the sibling block projection)
- tests: reason-prefix conformance pin, rewind reset, narrowed guard,
  history pagination merge

* fix(webui): avoid flushing sidechannel diagnostics

* fix(sdk): preserve diagnostics across rewind

* fix(webui): dedupe sidechannel history records

* fix(webui): align the paging sidechannel test with the normalizer keys

The paging test added in e6b40e5c failed deterministically (webui
suite red, CI Test job red) for two reasons:

1. The fixtures stamped only _meta['qwen.session.recordId'], but the
   SDK normalizer's extractSourceRecordIds reads
   _meta.qwenTranscript.sourceRecordIds — no sidechannel entry ever
   carried sourceRecordIds, so the dedupe assertion could not pass and
   the new displayedRecordIds loop was never exercised by a passing
   test. Stamp BOTH keys, matching production replay frames
   (acp-bridge buildUpdateMeta) and the sibling dedupe test.
2. Cap arithmetic: LIMIT-1 live entries + 2 fresh history entries =
   LIMIT+1, so the newest-wins slice evicted record-old-1 which the
   test asserted present. Emit LIMIT-2 live events so the post-merge
   total lands exactly on the cap.

Also correct the post-merge index assertions: history entries come
first (old-1, old-2), then the deduped-once live overlap, then the
first live mystery event. Suite 506/506, eslint + prettier clean.

* fix(sdk): raise diagnostic sidechannel bundle budget

* fix(sdk): raise the daemon browser bundle budget to 198KB and pin the diagnostics selector

- The sidechannel routing + selector cost ~1037 B over the 197KB cap
  (bundle measured 201893 B), failing the browser-bundle size gate; bump
  MAX_DAEMON_BROWSER_BUNDLE_BYTES to 198 * 1024.
- Fold the rebase-residue 190→191→192 KB ledger entries into the accurate
  190→195→196→197→198 lineage so the next bump has one canonical history.
- Add a behavioral pin for selectUnrecognizedDiagnostics: it must return
  the routed sidechannel itself (toBe), discriminating a `return []` or
  shallow-copy regression that the typeof-only surface test cannot see;
  flip-verified.

* fix(sdk): reset the user pointer on sidechanneled diagnostics, share the routing predicate

appendUnrecognizedDiagnostic left activeUserBlockId untouched while the
replaced appendStatusBlock path reset it for every non-user block; a
later mergeable user.text.delta with no promptId stamp (e.g. a peer
client's $ <cmd> echo) then appended onto the earlier user block
across the diagnostic, collapsing two user turns into one and skewing
rewindTranscriptToUserTurn's kind==='user' turn indexing. Keep the
reset (assistant/thought pointers stay untouched, the point of the
sidechannel); witness test flip-verified red without the one-line reset.

Also export isUnrecognizedDiagnosticReason from types.ts next to
DAEMON_UI_UNRECOGNIZED_DIAGNOSTIC_REASONS and call it at all three
routing-guard sites (reducer, provider flush condition, provider drop
filter) so the #7012/#8823 guard pair classifies every debug event
against one source instead of three hand-written copies.

* fix(ci): prevent bite harness SIGPIPE

---------

Co-authored-by: yiliang114 <yiliang114@users.noreply.github.com>
2026-08-19 14:38:00 +00:00
ChiGao
d96f264de7
feat(telemetry): link daemon HTTP request spans to inbound W3C traceparent (#9391)
Some checks are pending
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 1/2 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 2/2 (push) Waiting to run
E2E Tests / channel-plugin E2E (nightly) (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
npm cache producer / Save npm cache (push) Waiting to run
SDK Java / ubuntu-latest / Java 11 (push) Waiting to run
SDK Java / ubuntu-latest / Java 17 (push) Waiting to run
SDK Java / macos-latest / Java 21 (push) Waiting to run
SDK Java / ubuntu-latest / Java 21 (push) Waiting to run
SDK Java / windows-latest / Java 21 (push) Waiting to run
SDK Java / Real daemon E2E / Java 11 (push) Waiting to run
Security Checks / Dependency CVE audit (push) Waiting to run
Security Checks / Secret scan (TruffleHog) (push) Waiting to run
* feat(telemetry): link daemon HTTP request spans to inbound W3C traceparent

The daemon HTTP surface records a request span per request, but every span
starts a new trace: a caller forwarding the standard W3C traceparent header
(OTel-instrumented clients, proxies, gateways) gets no linkage back to its
own trace.

Extract traceparent/tracestate from inbound request headers in the daemon
telemetry middleware and parent the request span to that remote context.
Extraction reuses the same path as the existing JSON-RPC _meta extraction
(global propagator first, strict manual fallback so behavior is identical
without a registered SDK) and fails closed: requests without a valid header
keep the exact current span shape.

* fix(telemetry): guard inbound traceparent sampling and align W3C fallback

- Force TraceFlags.SAMPLED on inbound HTTP parents via the existing
  shouldForceSampled() matrix: an unsampled remote parent under the
  default parentbased_always_on sampler silently dropped the request
  span, the whole next() subtree, and the session-subprocess spans
  forwarded via _meta (review C1).
- Replace the hand-rolled manual fallback parser with a direct
  W3CTraceContextPropagator instance so acceptance rules (future
  versions, tracestate, all-zero ids, version-00 extension field)
  match the registered path with or without an initialized SDK.
- Gate middleware extraction behind isTelemetrySdkInitialized() to
  skip the hot-path parse when telemetry is off, and emit a debug
  daemon log when a present-but-invalid traceparent header is
  rejected.
- Re-export DaemonRequestSpanOptions from the core barrel and add a
  type-level guard so the parentContext field cannot silently
  disappear (vitest alone cannot catch its removal).

* chore(vscode): regenerate companion NOTICES.txt for @opentelemetry/core

* fix(telemetry): lazy-load OTel core fallback propagator behind SDK init

Address review feedback on the inbound traceparent linkage:

- Keep @opentelemetry/core out of the static graph. The module-level
  W3CTraceContextPropagator in daemon-tracing.ts pulled the CJS barrel
  (bot-measured +65,046 bytes) into every closure loading that module,
  including telemetry-off deployments. daemon-tracing.ts now keeps only a
  holder + setter (setDaemonFallbackPropagator, typed against
  @opentelemetry/api — type imports stay free at runtime); the lazy
  sdk-impl.ts chunk, whose closure already contains @opentelemetry/core via
  sdk-node/resources, constructs and injects the W3C instance on the
  successful SDK assembly path. Until injection, extraction returns no
  parent context: the HTTP edge is already gated on
  isTelemetrySdkInitialized (nothing changes when telemetry is off), and
  the _meta edge's consumers (withDaemonSpan / withInteractionSpan)
  short-circuit on the same flag, so an unresolved pre-init parent never
  had an observable effect.
- Add the mutation-verified fail-closed test for the header-extraction
  try/catch in daemonTelemetryMiddleware: a throwing extractor leaves the
  request settling normally (recordDaemonHttpRequest still fires once)
  with no parentContext on the span options.
- Record the rejected traceparent value (truncated to 128 chars) as
  http.request.header.traceparent on the invalid-header breadcrumb —
  traceparent only carries trace-id/span-id/flags, so this is
  privacy-safe and makes broken cross-service joins diagnosable.

Also document why the _meta extraction path deliberately skips
shouldForceSampled (trusted in-process bridge vs external HTTP input).

* feat(telemetry): carry inbound trace id into daemon access log with telemetry off

Telemetry off (the default) left daemon logs without any trace id: with no
request span, the log trace prefix never fires, so a caller forwarding W3C
traceparent could not be joined to its daemon log lines.

The middleware now parses the header with a plain regex
(extractInboundTraceId — same shape/all-zero/ff rejections as the W3C
propagator, no OTel machinery) and stores the trace id on the per-response
telemetry context. The access log emits it as the camelCase traceId field
of "request completed", keeping the log-based join alive with no telemetry
config and no trace backend. With telemetry on nothing changes: the request
span already carries the caller's trace id into the log prefix.

* fix(telemetry): unify _meta/HTTP sampling and repair build export

- Export extractInboundTraceId from the core barrel: the previous commit
  exported it from daemon-tracing.ts only, so downstream package builds
  failed with TS2305.
- extractDaemonTraceContext now applies the same shouldForceSampled()
  matrix as the HTTP edge: the _meta path is also reachable from direct
  ACP clients (acpAgent newSession/loadSession/unstable_resumeSession
  and Session.prompt pass caller-controlled _meta), so an external
  sampled=0 parent no longer silences daemon spans there either. The
  in-process bridge is unaffected (its injected values are already
  SAMPLED).
- The rejected-header breadcrumb now goes through sanitizeLogText so a
  crafted traceparent cannot forge log line structure with control
  characters.
- Add the sdk-impl wiring test: after initializeTelemetry the injected
  W3C fallback propagator resolves inbound HTTP parents.

* fix(telemetry): align log-path traceparent parsing and emit traceId in both modes

- extractInboundTraceId now mirrors the vendored W3C propagator's
  acceptance exactly: single optional leading/trailing whitespace and
  trailing extension fields above version 00 (version 00 must stay
  four fields). Previously the strict four-field anchor made the two
  paths disagree on the same forward-compatible header, silently
  dropping the access-log traceId for exactly the callers the
  propagator path supports.
- The camelCase traceId access-log field is now captured whenever a
  valid header parses, regardless of telemetry mode, so one saved log
  query / alert shape works for every deployment; with telemetry on the
  snake_case span prefix carries the same id redundantly.

* fix(telemetry): move inbound trace id getter out of the middleware module

52d572c0f2 made the access log statically import the telemetry
middleware module to read the captured inbound trace id. The access log
sits inside the serve fast-path pre-listen closure (run-qwen-serve
imports it directly), so the middleware's core-barrel import graph came
along for the ride and check-serve-fast-path-bundle started failing:
the 5.6MB core chunk (shell tool, glob, chokidar, @iarna/toml, fzf)
became statically reachable from run-qwen-serve.

Move the response-context symbol, its type, and the
getDaemonTelemetryInboundTraceId getter into a new import-light
telemetry-context.ts; the middleware imports the symbol from there and
re-exports the getter, so the access log no longer links against the
telemetry module at all.

* fix(telemetry): capture inbound trace id pre-auth under a dedicated symbol

* test(telemetry): pin the trace id seam through the context module getter

---------

Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>
2026-08-19 12:21:49 +00:00
callmeYe
e4f5504e9f
feat(extensions): support authenticated HTTPS Git installs (#9458)
* feat(extensions): support authenticated HTTPS Git installs

* test(serve): update capability integration baseline
2026-08-19 09:06:54 +00:00
callmeYe
daa7d61990
feat(daemon): add batch extension activation APIs (#8788)
* feat(daemon): add batch extension activation APIs

* fix(daemon): focus extension batch API on V2

* fix(sdk): export extension batch types

* fix(core): reject empty extension batches

* test(extensions): strengthen batch regression fences

* feat(extensions): allow batch activation declarations

* fix(extensions): preserve legacy activation declarations

* fix(extensions): reject ambiguous legacy identities

* fix(extensions): preserve batch activation lifecycle

* feat(extensions): key batch activation by name

* fix(extensions): harden batch activation lifecycle

* fix(extensions): preserve declared activation lifecycle

* fix(extensions): reconcile renamed and legacy policies

* fix(extensions): preserve renamed artifact lifecycle

* fix(extensions): validate persisted artifact paths

* fix(extensions): preserve re-keyed artifact directory
2026-08-19 06:42:48 +00:00
jinye
83fc634f61
feat(serve): measure ACP child peak old-generation heap (#9380)
* feat(serve): measure ACP child peak old-generation heap

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(cli): Omit unmeasured child heap reports and pin the GC observer path

Self-review round 1 on the child heap measurement:

- A child whose every getHeapSpaceStatistics() call throws (restricted
  container) reported a zeroed heap object with unclassifiedSpaceNames: [],
  which downstream reads as measured, needs-nothing, and coverage-complete.
  The probe now reports no heap at all until its first successful space
  read, matching what a child without the probe already sends.
- The GC observer callback — the only writer of peakLiveSetBytes,
  majorGcCount, and majorGcMs — had no test delivering a gc entry, so a
  wrong detail.kind check or a callback that never runs stayed green. The
  observer is now injectable and a test pins the major/minor split.
- The status/protocol/design text said the aggregate maximum "names the
  single worst-off child", but the aggregation takes Math.max per field
  independently. Wording now says each field is an independent maximum.

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-19 05:00:07 +00:00
jinye
dd82ba404e
feat(serve): Add live-state session activity watermark (#9396)
* docs(serve): Design live-state session activity timestamps

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

* docs(serve): Clarify live-state timestamp semantics

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

* feat(serve): Add live-state session activity watermark

Advance a bridge-local per-session activity watermark once when a prompt
that reached the running state publishes its formal terminal, project it
as the existing optional BridgeSessionSummary.updatedAt, and expose it on
the workspace live-state route. The advance is written before the
terminal is published so a client that observes the terminal cannot read
a stale value, and the extra millisecond keeps the watermark strictly
increasing when several terminals share a wall-clock millisecond or the
clock moves backward. A queued-only terminal, heartbeat, attach/detach,
or streamed update never advances it, and turn activity does not change
the session catalog version.

Populating the already-typed summary field lets full workspace session
lists merge live and persisted timestamps. Because the mtime and the
running-turn watermark are different authorities and the recorder writes
asynchronously, the merge picks the later valid timestamp instead of
blindly preferring the live value, so a row cannot move backward when an
async transcript write lands after the terminal.

Extend the response schema documentation for the live-state route and
GET /session/:id/status, and add the optional field on the TypeScript
SDK DaemonSessionLiveState type so consumers can pre-flight the tag once
and read the recency directly.

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

* fix(serve): Apply the later-valid activity rule on Live Task read paths

read_thread and wait_threads read the bridge summary directly, so once the
summary began carrying the running-turn watermark their fallbacks stopped
consulting the persisted transcript timestamp. Because the recorder writes
asynchronously, one task could report a later recency from the thread list than
from a thread read, and a wait cursor keyed on the live value alone stopped
changing when only the transcript advanced, so a consumer waiting for that
flush saw an unchanged cursor and exited early. Move the merge rule into a
shared helper and apply it at all three read points, including the revision
fallback used for a session with no attached client.

Add the watermark cases the design doc enumerates but the previous commit did
not ship: the deadline path publishes its terminal twice and must still advance
exactly once, a corrected forward clock jump must never decrease the value, and
a clock that advances between terminals must be reported instead of the logical
tie-breaker. Cover the single-session status route's verbatim pass-through of
the field, and cover the helper's both-invalid tail directly because no route
can supply two invalid candidates.

Correct two design-doc test-plan claims that did not match the code: the
teardown paths advance a watermark no consumer can read, because the entry
leaves live state in the same operation, and the duplicate deadline terminal
comes from the raced rejection reaching the settle handler rather than from a
late agent result.

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

* fix(serve): Merge live state on every organized session page

The organized view applied the live merge only on the first page, so page 1
sorted rows and encoded its cursor from merged activity keys while later pages
keyed the same rows by persisted mtime alone. That was harmless while bridge
summaries never carried an activity timestamp, because both keys were the mtime.
Now that a settled turn advances a watermark that leads storage until the
recorder flushes, a live row ordered onto page 1 by its watermark falls behind
the page-1 cursor boundary on page 2 and is returned a second time, displacing a
genuinely new row. Merge live state on every page so both pages key rows the
same way; a live-only row still has no persisted key to page by and stays a
first-page insertion.

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

* docs(serve): Document activity-cursor duplication on live retirement

The activity key merges the live watermark, which is in-memory only, so a
session whose live entry retires mid-pagination falls back to its transcript
mtime and can be admitted again by a cursor encoded from the higher watermark.
Pre-change keys came from mtime alone and only advanced, so pages could skip a
row but never repeat one. Name that mode in the design doc and warn SDK
consumers of the activity-ordered cursors to key accumulated pages by
sessionId.

* fix(serve): Exclude emitted identities from activity-cursor re-admission

An activity key merges the bridge's in-memory watermark, so it is not a
stable property of a row: when a live entry retires mid-pagination the key
regresses to the transcript mtime, and a live-only row that persists mid-pass
re-enters the scan keyed by its first flush. Either way a row already emitted
on an earlier page could pass the strictly-older cursor filter again and
displace a genuinely new row, which was structurally impossible while activity
keys came from mtime alone.

The organized and metadata activity cursors now carry the identities already
emitted at a live-derived key, and the after-cursor filter excludes them, so
one pass returns a session at most once. The list prunes itself: an identity
is dropped once its persisted floor alone can no longer pass the key filter or
once the row leaves the filtered collection while not live. Past a 64-identity
cap the highest floors are dropped first, degrading to the previous at-most-
once duplicate instead of failing the pass. Cursors minted before the field
existed stay valid, and the field is omitted when empty.

* fix(serve): Close carried-identity drop paths in activity-cursor pagination

The emitted-identity carry could still drop a carried session mid-pass and
re-admit it later: an identity absent from a page's collection was discarded
even though absence can be transient (pre-flush TTL cache, mid-pass group
movement), organized re-entry was evaluated under the row's current pin state
only, and the live-only cursor key could move backward when a wall-clock
rollback landed the first watermark behind createdAt.

Retain absent carried identities at a negative-infinity floor, test organized
re-entry under both pin states, and floor the first watermark advance at the
entry's createdAt. Extend the retire test to a three-page pass so carried-set
propagation through an intermediate cursor is pinned, probe scan visibility in
the mid-pass-flush tests, and cover the live-list-failure and unpin paths.
Scope the at-most-once pagination wording in the design and protocol docs to
what the carry actually guarantees.

* docs(design): floor the first watermark advance at createdAt in the normative formula

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-08-19 03:31:44 +00:00
Shaojin Wen
da26cffc36
feat(review): Aone Code read path (second review-platform provider) (#9226)
* docs(design): /review Aone Code read path (Phase 2)

* feat(review): Aone Code read path (Phase 2)

Adds an Aone Code provider so /review can review a MaxCompute CR locally.
The read path works end to end against a real odps_src CR (verified E2E):
fetch-pr fetches `refs/merge-requests/<global-id>/head` and builds the
worktree + diff (stats computed locally, Aone advertises none); meta /
issue-context / fetch-diff resolve identity, Aone workitem evidence, and the
diff via the a1 CLI. The four reader-backed subcommands and fetch-pr select
the provider from the clone's remote (or an Aone host), so a GitHub clone is
unchanged.

Read-only this phase: pr-context / comment-status / presubmit have no Aone
backing yet (the run degrades to context-unavailable), and --comment is
refused on an Aone target. SKILL.md + code-review.md document the Aone
target and the degradations. See docs/design/2026-08-15-review-aone-provider.md.

* fix(review): address PR #9226 round-1 Aone review findings

Critical:
- match-remote: Aone CR URLs use the WEB host (code.alibaba-inc.com) while
  a clone's remote uses the GIT host (gitlab.alibaba-inc.com) — treat them as
  one equivalence class (hostsEquivalent) so a codereview URL matches its
  clone's remote and the worktree flow is reachable; nested-group remotes
  (group/subgroup/project) now collapse to the last two segments instead of
  failing to match
- registry: an explicit non-Aone host/remote now beats the cwd probe, so an
  explicitly-GitHub subcommand run from an Aone clone is not hijacked to
  Aone; hint host is trimmed; the four reader-backed subcommands thread
  --host into detection (previously dropped); dropped the unwired --platform
  dead switch
- aone parseRemoteUrl: user-less scp remotes (ssh-config/insteadOf), nested
  groups, a trailing slash after .git, and empty segments now parse; the
  parse-failure message redacts a user:token@ origin (no credential leak)
- aone getCommentBody throws on a missing id (was an indistinguishable empty
  string); fetchDiff uses gitRaw (512 MiB buffer, no CRLF rewrite, latin1)
  instead of git() (1 MiB ENOBUFS, CJK byte loss)

Suggestions:
- aone-client: 120 s timeout, ENOENT branch in the auth check ("install the
  a1 CLI"), TRANSIENT_RE anchored to HTTP 5xx (bare 502/503/504 misfired on
  command lines containing those digits), one stderr trace line per retry
- fetch-pr: validate pr_number before Number() coercion (1e3 fetched PR
  1000), trim --host before detection; submit guard trims --host;
  comment-body --pr help notes the Aone per-MR requirement
- parse-args: nested-group codereview URL grammar; invalid-url warning names
  both grammars
- SKILL.md + code-review.md: Aone paragraph corrected (clone-origin trigger,
  Agent 0 skipped, test-plan/publish-assets unbacked, pass --host), design
  doc updated (detection, Agent 0 gating)
- tests: aone.test.ts, registry cwd-mock + precedence + parseRemoteUrl cases,
  remote-match hostsEquivalent + nested collapse, parse-args nested codereview,
  submit-aone refusal

* fix(review): address PR #9226 round-2 Aone review findings

Critical:
- parse-args: the Aone CR URL grammar is now constrained to Aone hosts
  (*.alibaba-inc.com) — a /codereview/ URL on any other host hits the
  fail-closed invalid-url refusal instead of becoming a live PR target
  (unlike …/pull/<n>, which any GHE host legitimately serves)
- aone fetchDiff: merge-bases against a fetched target branch (not a
  present-but-stale origin/<target>), and the MR-head refspec is now
  force-fetched (+) so a stale throwaway ref from an interrupted run does
  not fail the fetch when the head was rewritten (normal AGit-Flow iteration)

Suggestions:
- fetch-pr: countDiffChangedLines now delegates to the single hunk-state
  walker in computeDiffStats (the two could not disagree silently); the
  changedFiles count is pinned on `diff --git` via a binary-file fixture
- aone parseRemoteUrl scheme case made explicit + pinned (RFC 3986)
- submit.test.ts pins the platform registry to GitHub so the Aone refusal
  guard neither spawns a real git in the vitest cwd nor couples to the
  machine's clone origin

* fix(review): address PR #9226 round-3 Aone review findings

Critical:
- registry: hostOfRemoteUrl now makes `user@` optional in the scp branch
  (user-less scp remotes from ssh-config/insteadOf no longer misroute an
  Aone clone to GitHub); the token-bearing scp userinfo parses to the host,
  not an owner
- parse-args: the Aone CR-URL host group now requires a REAL subdomain dot
  boundary (`(?:[A-Za-z0-9-]+\.)+alibaba-inc.com`), so lookalikes
  (`evilalibaba-inc.com`) hit the fail-closed refusal; a `/pull/<n>` URL on
  an Aone host is refused too (Aone serves no /pull/ pages)
- meta: on a non-GitHub platform an explicit `--repo` without `--host` is
  refused (no default host off GitHub) instead of emitting the contradictory
  `platform:aone` + `host:github.com`
- aone fetchDiff: spreads PINNED_DIFF_CONFIG/PINNED_DIFF_FLAGS (an un-pinned
  color.diff=always zeroes computeDiffStats), discloses a failed target-branch
  fetch via a stderr WARNING, and refuses to diff from a clone of a different
  repo; scp-form userinfo is redacted in the parse-failure message

Suggestions handled:
- comment-body: the Aone per-MR `--pr` requirement is enforced before the
  auth gate (usage errors precede auth)
- aone-client: the auth-failure diagnostic surfaces a1's real first stderr
  line (not the execFileSync preamble) and reports a timeout/kill distinctly
- remote-match docstring + registry precedence comment updated to the
  implemented behavior

Deferred to follow-up #9194: the 16 test-gap patterns, headRefOid dead-field
removal, MAX_SAFE_INTEGER digit guard, and the refusal-message host branch.

* fix(review): address PR #9226 round-4 Aone review findings

Critical:
- aone fetchDiff + fetch-pr merge-base fetch: the server-controlled
  target/base branch reached `git fetch` bare — a dash-leading branch name
  (creatable by full-refname push) parses as an option, so
  `--upload-pack=<payload>` executed attacker-named code with the
  reviewer's credentials. Pass `--` to end option parsing and refuse
  dash-leading values outright on both providers
- meta: the no-default-host guard now gates on the FLAG, not the resolved
  value — a GH_HOST export no longer bypasses it (and an empty-string
  --host counts as missing); the whole --repo branch's pure resolution
  moves above the auth gate (usage errors precede auth)
- skill: pass --host for EVERY pr-url target including github.com — an
  omitted hint falls back to the cwd origin probe, which hijacked a
  github.com review run from an Aone clone (and vice versa); lightweight
  fetch-diff/pr-context carry the host too
- submit: the Aone refusal moves BELOW the authorisation gate and takes
  the exit-3 + {"posted": false} shape instead of throwing — an
  unauthorised Aone run now ends as the skill's contract defines, and
  detection reads the effective host (flag → GH_HOST), so an Aone-pointing
  GH_HOST export is refused instead of dying opaque inside gh

Suggestions handled:
- parseRemoteUrl: strip query/fragment (credential channel into repo
  identity), fix the cleaning order for two-plus trailing slashes after
  .git, and discard an explicit port instead of folding it into the path
- registry: isAoneHost normalizes the trailing-dot FQDN spelling; the cwd
  probe delegates to lib/git's gitOpt (shared git policy)
- aone: the MR-head refspec is stated once (mrHeadRefSpec); resolveRepo
  quotes git's real error line, not the execFileSync preamble
- aone-client: the auth fall-through message is neutral (covers
  non-auth failures the login hint cannot fix)
- fetch-pr: pr_number guard tightened to ^[1-9]\d*$ (no PR zero, no
  leading zeros, no side effects before the refusal)
- the five detection-consuming subcommands' --host describes now state
  the implemented semantics; SKILL.md/code-review.md read-only phrasing
  corrected and the false "detection reads the clone's remote, not the
  URL" claim fixed

Tests: dash-leading refusal on both providers, meta guard flip tests
(GH_HOST bypass, empty flag, pre-auth), submit exit-3 shape (authorised,
unauthorised, padded host, GH_HOST), trailing-dot and /pull/-on-Aone
parse refusals, port/query/slash parse cases, fetch-pr zero-number and
base-ref refusals.

Deferred to follow-up #9194: the single-branch merge-base disclosure
(R3-9), cleanup audit skip-in-code (R3-13), URL-form --remote hint
(R3-19), publish-assets refusal parity (R3-22), and the data-path
deadline translation (R3-25).

* chore(review): re-push to re-link PR head after branch recreation

* test(review): repin SKILL.md host-rule wording in SKILL.test.ts

The round-4 fix rewrote the skill's --host notes (pass --host for every
pr-url target, github.com included); three revert-guard tests pinned the
old 'add --host <host> for Enterprise' phrasing and reddened the core
suite in CI. Repin them at the new wording.

* fix(review): address PR #9226 round-5 Critical findings

- aone resolveRepo: redactUrl now strips the query/fragment channel too —
  a ?private_token=… origin carries no @ for the userinfo redaction, so the
  parse-refusal message echoed the secret the success path strips (test
  pins the refusal message secret-free)
- aone fetchDiff: the merge-base fallback (base = ref~1) DISCLOSES via a
  stderr WARNING — previously silent, a multi-commit MR got only its last
  commit served as the complete diff (shallow/single-branch clones hit
  this; the GitHub path is loud about the same class)
- submit: the Aone write-refusal binds the platform in BOTH directions —
  the authorisation gate now surfaces the recorded target's host, so a
  recorded Aone host refuses whatever the runtime-effective host resolves
  to (an ambient GH_HOST export can no longer steer an Aone review into
  posting at a same-named repo), while a recorded non-Aone pr-url binding
  is no longer vetoed by the cwd probe from an Aone-origin clone

Tests: refusal-message redaction, fallback disclosure (spy calls captured
before mockRestore — vitest's restore clears them), bidirectional refusal
arms (recorded-Aone + GHE env refuses; recorded-github + Aone cwd posts).

* fix(review): address PR #9226 round-6 Critical findings

- authorization: the --user-authorized fast path now surfaces the recorded
  target's host too (best-effort read of the recorded args) — it returned
  before the args file was read, so recordedHost was always undefined on
  that path and the 'a recorded Aone host always refuses' invariant leaked:
  a user-authorised post of a recorded Aone codereview review from a
  non-Aone cwd with no --host/GH_HOST posted at github.com's same-named
  repo. Tests pin the fast-path host through the REAL gate and the
  end-to-end refusal (the witness scenario)
- aone: the query/fragment strip now uses [\s\S]* in both redactUrl and
  parseRemoteUrl — git stores newline-bearing remote URLs, and a plain .
  stopped at the first \n, letting ?private_token=SECRET\nx smuggle the
  token past the strip into the parse-refusal message. Tests cover both
  the parse-success and refusal paths of the smuggle

Round-6 is Critical-only per the ~5-round policy (user-confirmed for
convergence); the 13 Suggestions are deferred to follow-up #9194.

* fix(review): address PR #9226 round-7 Critical findings

- fetchDiff's throwaway ref now carries a pid suffix — two concurrent runs
  for the same MR in one clone shared the name: one session's finally-
  delete killed the other mid-review (unknown revision), and a
  pre-existing local branch of the reserved name was force-moved then
  deleted, reflog and all (race probe: 12/60 failures → 0 with the
  per-run unique name)
- the target/base-ref guards close the refspec channel the dash-only
  check left open after `--`: a leading `+` parses as a force refspec
  (fetches the wrong head — stale evidence, no WARNING) and a colon as
  src:dst (force-moves the throwaway ref or a reviewer-local branch).
  Both providers now refuse '-', '+', and ':' shapes (probe-confirmed on
  real fetchDiff incl. the served-wrong-diff and local-branch-overwrite
  witnesses); tests pin the new channels on both guards
- redactUrl and parseRemoteUrl clean userinfo BEFORE the query/fragment
  strip: a userinfo that itself contains '?' or '#' was truncated
  mid-credential, leaking the username+secret prefix into the refusal
  message and making parseable origins unparseable (flip-verified on the
  witness shapes)

Round-7 is Critical-only per the convergence directive; the 8
Suggestions (incl. the 4 bot findings) are deferred to #9194.

* fix(review): address PR #9226 round-8 Critical findings

- the server-controlled branch-name guards now validate ALLOWLIST-style on
  both providers (aone.fetchDiff's target, fetch-pr's baseRefName): the
  denylist admitted HEAD (silent fetch + merge-base through the stale
  clone-time symref), rev-parse metasyntax (wrong base under a
  misdescribing warning), ranges, and the empty string (garbled diff-less
  fallback) — a plain-branch-name shape closes every channel
- parseRemoteUrl/redactUrl consume userinfo GREEDILY up to the last @ of
  the authority — multi-@ and :-/-bearing token userinfo no longer leaks
  cleartext residue through the refusal messages or folds into the parsed
  host (take() fails closed on any surviving @); the scp strip admits only
  a removal that leaves a host: shape behind
- fetch-pr's Aone stats backfill moves AFTER the plan/rescue, where
  diffText is final — the partition-rescue republishing the full range no
  longer leaves delta-scoped numbers beside a full-range diffPath — and
  isCollapsedFromUpstream is skipped when the stats are locally derived
  (one source, not two: the disclosure needs an independent advertised
  fact, and a delta-scoped round beside the full-range count fired a
  false collapse)
- remote identity is injective again: Aone nested-group targets carry the
  full group path (parse-args → match-remote --group-path → matchRemotes
  compares every segment when both sides have three or more), and
  fetchDiff's origin guard adds the origin's host (Aone family) — a
  same-named repo in another group or on another platform can no longer
  pass either gate; SKILL.md passes --group-path for nested targets
- meta's discovery branch drops GH_HOST inheritance off GitHub — an
  ambient GHE export beside an Aone-origin clone no longer vetoes the
  valid invocation at HOSTNAME_RE; only an explicit --host steers routing

Round-8 is Critical-only per the convergence directive; all five findings
fixed, no deferrals this round.

* fix(review): address PR #9226 round-9 Critical findings

- redactUrl is fail-closed BY CONSTRUCTION: split at the last @, redact
  everything before it — the per-regex redaction kept missing shapes
  (round-9: URL userinfo with a / in the secret, scp userinfo with a
  newline, residues with no host: shape all leaked verbatim through the
  parse-refusal message)
- parseRemoteUrl cleans per form and fails CLOSED: URL-form userinfo is
  bounded to the authority (greedy within it — multi-@ and ?/# inside
  secrets consumed whole, /-bearing secrets left to fail closed in take),
  scheme inputs never fall through to the scp grammar (a malformed
  https://user:pa/ss no longer parses host user); the round-8 scp-strip
  firing on scheme URLs fabricated coordinates from query-borne and
  path-borne @ witnesses — all witnesses now parse correctly or refuse
- registry hostOfRemoteUrl consumes token-bearing userinfo (':' AND '/'
  in the secret) on both branches, mirroring aone.parseRemoteUrl —
  detection no longer parses the credential prefix as the host and
  misroutes Aone clones to GitHub; detectPlatformKind ranks an explicit
  --host above the remote-URL hint in BOTH directions (an Aone origin
  can no longer hijack an explicitly-GitHub invocation into fetching a
  global MR id from the wrong remote)
- nested-group identity is injective in both directions: matchRemotes
  compares the full group path exactly whenever the target carries one
  (any length — a 3+-segment target no longer matches a two-segment
  remote sharing its tail, nor the reverse); Aone CR targets carry the
  path even at two segments and the canonicalized URL keeps the full
  path; fetchDiff's origin guard compares the origin's full path against
  the MR's own detailUrl path (authoritative repo identity, where the
  seam's ownerRepo is collapsed); the rescue pool keys on the full path
  and same-id cross-group CR URLs are refused as ambiguous

Round-9 is Critical-only per the convergence directive; the 8
Suggestions (R8-6..R8-13) are deferred to follow-up #9194.

* fix(review): address PR #9226 round-10 Critical findings

- aone.fetchDiff's host arm keys on the CANONICAL Aone-family predicate
  (new remote-match isAoneHostFamily: port/trailing-dot/case normalized;
  registry.isAoneHost now delegates to it) — a trailing-dot FQDN clone
  that detection accepts as Aone can no longer be refused by the diff gate
  with a misdirecting remedy
- the URL cleaning/redaction class is closed structurally, not per shape
  (sixth consecutive round a new entrance was found): parseRemoteUrl's
  URL-form userinfo is consumed whole WITHIN the authority (span between
  // and the first /), and the scp-form userinfo strip + its lookahead are
  bounded at ?/# — an @ inside a query or fragment value is the
  credential's own character and can no longer fabricate coordinates from
  the query tail; redactUrl fails the DISPLAY closed with a constant when
  the last @ sits after a ?/# marker — the token tail can no longer reach
  the refusal message (URL/scp/fragment witnesses all pinned)
- isPlainBranchName rejects git's pseudo-ref set (FETCH_HEAD/ORIG_HEAD/
  MERGE_HEAD/…) on both guards — FETCH_HEAD resolves to the just-fetched
  PR head (empty diff beside full-range metadata), ORIG_HEAD to an
  arbitrary ancestor; both shape-legal, both silently wrong
- fetch-pr's merge-base probe requires the fetch to have produced the
  tracking ref — a tag-only baseRefName exits 0 writing only FETCH_HEAD,
  and the bare-name fallback once merge-based against the reviewer's
  local tag with baseFetchFailed falsely false; the tag shape now lands
  in the disclosed state
- parse-args: the repo-qualified CR URL outranks a same-number bare
  spelling as the target in BOTH the rescue pool and positional order —
  the bare number carries no host, and letting it win flipped detection
  onto the cwd fallback (a loud refusal at the merge base had degraded to
  a silent wrong-platform retarget); bare restatements of the URL target
  are skipped silently, matching the rescue loop's restatement handling

Round-10 is Critical-only per the convergence directive (the bot's own
ledger is at its round cap); the 6 convergence-posture deferrals named in
the review body join follow-up #9194.

* fix(review): address PR #9226 round-11 Critical findings

- the URL cleaning/redaction surface is closed STRUCTURALLY: one parser,
  one source of truth — registry.hostOfRemoteUrl now delegates to the
  canonical aone.parseRemoteUrl (detection and the identity parser can no
  longer disagree), and the scp branch reads GIT'S OWN grammar
  (GIT_TRACE-probed: hostinfo ends at the FIRST ':', userinfo carries no
  ':' or '/') — the last-'@' consumption once parsed a different host than
  git connects to, letting fetchDiff's same-repo guard pass while git
  fetched from another server; token-bearing scp shapes now fail closed,
  and the round-8 detection tests are re-blessed onto shapes git reads
  that way
- the pseudo-ref allowlist is CASE-INSENSITIVE on both twins: on
  case-insensitive filesystems (macOS/Windows defaults) fetch_head folds
  onto FETCH_HEAD, resolving the merge-base to the just-fetched MR head
  (empty diff beside full-range metadata); lowercase spellings refused,
  pinned
- submit.test.ts's file-level setup now saves/clears/restores GH_HOST —
  the Aone refusal reads the ambient env, and the org's standard intranet
  export pattern (an Aone-family host) turned 50 of 69 posting tests into
  refusals
- the --user-authorized fast path binds the recorded host to THIS write
  (same-PR number only — a stale recording of another PR must not supply
  a host) and scans SIBLING session recordings when the session-scoped
  args file is absent — the characteristic cross-session publish shape
  otherwise lost the host and posted a recorded Aone review at
  github.com's same-named repo (real-gate witness: exit 0, COMMENT filed);
  tests drive the real gate through a sibling-session fixture
- the tracking-ref requirement and both merge-base sites are FULLY
  QUALIFIED (refs/remotes/…): git resolves unqualified origin/<name> in
  refs/tags and refs/heads first, so a tag or branch literally named
  origin/<baseRefName> — a PUSHABLE, server-controlled refname a plain
  clone auto-carries — shadowed the just-fetched tracking ref and moved
  the merge base with no disclosure; shadow-tag tests pinned on the
  resolveMergeBase probe, the fetch-pr seam, and aone.fetchDiff

Round-11 is Critical-only per the convergence directive; the bot's own
ledger is at its round cap and this round still produced findings —
recommend freezing the bot loop and moving to human security review.

* fix(review): address PR #9226 round-12 Critical findings

- the recorded-args host lookup is HARDENED — the store lives under
  .qwen/tmp/ beside review worktrees checked out from the PR's own tree,
  so its content is attacker-influenceable: only s-* session directories
  are scanned (a malicious PR can no longer plant a root-level args file
  that binds a host), symlinks are skipped at both the directory and file
  levels (mirroring writeSkillArgs' O_NOFOLLOW write-side policy), reads
  are size-bounded, and the host binds only when the recording names the
  same PR number AND the same repo
- the canonical Aone invocation shape (bare global MR id, no URL) can no
  longer post cross-session without host evidence: a same-number
  recording with no host binds the recorded --host flag when present
  (parse-args now records it), and without one the write gate FAILS
  CLOSED with the exit-3 shape and names the remedy — instead of posting
  the review at github.com's same-named repo (the probe-verified witness
  once exited 0 and POSTed)
- parse-args: the URL-outranks-bare-number invariant now holds for MIXED
  shapes — a positional bare number restating the rescue pool's single
  PR is carved out of hasValidCandidate, so --effort <cr-url> 7 (and
  both orderings/equals-form) target the CR URL instead of silently
  retargeting onto the cwd clone's same-number PR; a different number
  still outranks
- aoneReader.resolveRepo refuses an origin outside the Aone host family —
  an explicit --host can steer detection onto this reader while the cwd
  clone is a GitHub mirror (the dual-remote migration setup), which once
  emitted {platform:'aone', host:'github.com'} and queried a1 with the
  mirror's coordinates; same predicate fetchDiff's origin guard applies
- isPlainBranchName (both twins) rejects refs/-prefixed names: legal
  branch names (check-ref-format --branch) that resolve qualified refs
  the server controls as fetch/merge-base arguments (refs/remotes/origin/
  HEAD is the clone's default-branch symref — wrong base, misdescribing
  WARNING)
- the ref-dwim class is closed at the verified sites: fetch-pr's base
  probe fetches an EXPLICIT branch refspec (bare names dwim onto
  same-named tags — exit 0, tracking ref untouched, stale base passing
  the freshness guard it never refreshed), its fetchedSha/merge-base head
  reads are refs/heads-qualified (a planted same-name tag can no longer
  shadow the real head), and aone.fetchDiff's target fetch + merge-base +
  diff-range reads are qualified the same way

Round-12 is Critical-only per the convergence directive (the bot's own
ledger is past its round cap).
2026-08-18 09:19:52 +00:00
BaboBen
9a64c0a963
feat(serve): add pollable daemon turn status (#9080)
* feat(serve): add pollable turn-status endpoints for daemon sessions

Add GET /session/:id/turns/current and GET /session/:id/turns/:promptId
so external callers can poll a turn's lifecycle state (queued / running /
completed / cancelled / error) and result instead of holding the SSE
stream for the whole turn lifetime.

- Live state comes from the bridge's pending prompt queue; settled
  outcomes from persisted turn_result transcript records, so results
  survive daemon restarts and the daemon keeps no per-turn memory
- Each prompt captures its own recording and settles exactly that one,
  so overlapping turns (DAEMON-003 deadline overlap) can never
  misattribute one turn's outcome to another promptId
- Enforces the same client authorization as POST /session/:id/prompt

Refs #8680

* test(serve): update telemetry route count

* fix(serve): prefer settled turn outcome over deadline error overlay

When the prompt-deadline path latches an error terminal in the overlay and the child later settles and persists a non-error turn_result for the same promptId, the poll surface previously kept the overlay error while enriching it with the successful resultText, and flipped to completed only after overlay eviction or restart. Merge via mergeTerminalWithPersisted at the two enrich call sites so the persisted outcome supersedes a bridge-synthesized error terminal once it exists; the exactly-once turn_error event publication and FIFO release are unchanged. The different-promptId endedAt tie-break is intentionally untouched.

* fix(serve): pin turn-start session identity for turn-result settle

R4-1: settle resolved the ChatRecordingService at settle time, so a startNewSession rotation mid-turn could land the turn_result record in the new session's transcript while the poll surface kept reading the old one. Capture the recorder at turn start and settle on that instance; pin the outgoing service's session identity at rotation so the late append keeps the pre-rotation sessionId.

R4-2: reject empty error.message/error.code in turn_result payloads, mirroring the existing empty-promptId rejection.

Adds rotation/pin regression tests plus the round-4 test suggestions (extractor fallback, startedAt, cancel/error race matrix, resultCode defaulting, removed-prompt projections).

* fix(serve): enforce the turn_result bounded contract on the write path

R3-3: cap promptId, stopReason, and originatorClientId at 256 chars in isTurnResultRecordPayload, closing the unbounded echo of corrupted-transcript values through GET /session/:id/turns/:promptId; recordTurnResult now validates payloads against the same contract before appending, so type-correct but invalid shapes (error state without error, error on non-error states) can no longer produce records invisible to the restart scan.

Also lands the four round-5 test assertions: merged-payload error-leak pin, multi-model-call settle count, successor attribution in the superseded-throws test, and the early session-mismatch guard pin.

* fix(serve): address round-6 review findings on daemon turn status

- Session: settle a successor-aborted turn as cancelled only when the
  thrown error is the abort itself; genuine failures after a NEW_PROMPT
  abort surface as error, matching the send-loop contract
- bridge: serve repeat polls of a settled promptId from the enriched
  overlay instead of re-scanning the child transcript, and give the
  turn-status read the transcript timeout instead of the 10s init default
- bridge: forward the channel display text unchanged; Session treats an
  empty display text as absent for the turn record ([image] fallback)
- Session: cap streamed-response accumulation for turns without a
  channel delivery at the turn-result bound
- docs: document the bounded non-monotonicity of poll terminals

* fix(serve): guard turn-status reads against rewind races and keep the trusted prompt projection

A successful rewind that completes while a getSessionTurnStatus child
transcript scan is in flight could let the pre-rewind record be cached
into the freshly cleared overlay and served forever. Track a per-session
rewind generation captured before the scan and discard the scanned
outcome when it moved.

enrichTerminalTurnStatus and the deadline-supersede merge returned the
child-recorded promptText ahead of the bridge's trusted display
projection, leaking hidden channel context on the poll surface. Make
promptText/promptTextTruncated backfill-only and keep the terminal's
projection in the supersede path. Make the pinning test adversarial and
correct a false comment about the child's ''-as-absent fallback.

---------

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: qqqys <qys177@gmail.com>
2026-08-18 06:35:17 +00:00
易良
99a3a08e17
feat(daemon): make serve new-file mode configurable (QWEN_SERVE_NEW_FILE_MODE) (#9364)
* feat(daemon): make serve new-file mode configurable (QWEN_SERVE_NEW_FILE_MODE)

qwen serve's atomic text writers created every NEW file at 0600
unconditionally, ignoring the daemon process umask with no way to opt
out (issue reporter runs the daemon under a systemd UMask=0002 drop-in
and every agent-created file diverged from the group-readable repo
convention).

Add a NewFileModePolicy ('owner' = 0600 default, 'system' = standard
0o666 & ~umask) on createWorkspaceFileSystemFactory, threaded through
writeTextAtomic / writeTextOverwrite / edit / editAtomic and the
same-host external tool-write route. resolveBridgeFsFactory reads
QWEN_SERVE_NEW_FILE_MODE ('owner' | '0600' | 'system',
case-insensitive); unrecognized values warn on stderr and keep the
fail-closed 0600 default.

Existing-file mode preservation is unchanged, binary uploads stay
0600, and the default behavior is bit-for-bit unchanged.

Closes #9250

* fix(serve): register the new-file-mode env access and harden the knob's docs and wiring

- process-env guard: register the whole-object process.env access in
  fs-factory.ts (the parseNewFileModePolicy default parameter) so the
  serve process.env guard suite passes — the PR-caused CI failure.
- resolveNewFileModeBits: read the umask lazily only when the 'system'
  policy consumes it; the default 'owner' path no longer issues two
  umask(2) syscalls per write.
- docs: state that the literal `0600` is an alias for `owner` (no other
  octal modes) in both tables; correct the binary-upload route to
  POST /file/upload; replace the phantom per-write mode-override clause
  with the factual statement that agents cannot pass one.
- test: pin the resolveBridgeFsFactory env seam — with newFileMode
  uninjected, the policy must come from process.env.QWEN_SERVE_NEW_FILE_MODE
  (regression-mutates to a hard-coded default are now caught).

* fix(serve): keep QWEN_SERVE_NEW_FILE_MODE out of project .env files

R2-1: the daemon boot path loads the primary workspace .env into
process.env before any fs factory is built, and the new-file-mode key was
not in PROJECT_ENV_HARDCODED_EXCLUSIONS — a project-controlled file could
flip the documented fail-closed 0600 posture to umask-derived modes
daemon-wide with no warning (system is a valid value), widening the
visibility of agent-created files on a multi-user host. Register it as a
process-scoped operator knob like the other daemon posture keys, with a
security test pinning the exclusion.

* test(serve): pin the fail-closed 0600 default through the resolveBridgeFsFactory seam

The env-wiring test only covered the 'system' half of the seam; the
unset-env default (owner -> 0600) had no coverage through the same
production path — a regression making the unset default resolve to
'system' would flip every agent-created new file to umask-derived
modes with no test failing (mutant verified surviving all 13 prior
tests; this mirror test fails it with 0o664 vs 0o600).

---------

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-08-18 03:42:35 +00:00
易良
04043e555d
feat: consolidate Local Control into one daemon-owned implementation (#9106)
* feat(cli): add daemon-owned Local Control service

Local Control is implemented twice today — once in the CLI, once as an
830-line Rust TCP proxy in the Tauri shell — with two divergent security
models. This adds the daemon-side service both can collapse onto.

The Rust proxy exists only because `qwen serve` fixes its bind address at
startup and cannot add a listener later; everything it does (Host/Origin
rewriting, CRLF rejection, connection caps) is compensation for that one
fact. `LocalControlService` attaches a second `http.Server` over the same
Express app at runtime, so there is no hop to rewrite.

Phases 1-3 of docs/plans/2026-08-13-local-control-consolidation.md:

- Listener identity tagged on the `http.Server`, resolved per request, so
  credentials scope to the listener a request arrived on.
- `CredentialStore` replaces `bearerAuth`'s single pre-hashed token. The
  runtime token is rejected on the LAN listener and the pairing token on
  loopback — the invariant the Rust proxy enforced by rejecting requests
  carrying the runtime token. Fixes the CLI path handing the LAN the
  full-strength daemon token with no revocation short of restart.
- `hostAllowlist` now gates the LAN listener against its advertised
  authority. Previously it opted out entirely off loopback, leaving the
  CLI path with no DNS-rebinding defense.
- `MutableOriginAllowlist` lets the LAN origin be added and removed at
  runtime; the middleware is still installed once. Empty-allowlist
  behavior is identical to the `denyBrowserOriginCors` wall it replaces.
- ACP WS upgrade tracks a set of servers instead of one, and scopes the
  subprotocol credential the same way as the REST gate.
- LAN selection advertises private/link-local IPv4 only, and surfaces
  ambiguity to the caller instead of failing (Rust) or emitting a QR per
  interface (CLI).

Refs #9075

* feat(cli): wire Local Control into the daemon boot sequence

Constructs the service in `createServeApp`, where the credential store and
the CORS allowlist it mutates already live, and publishes it on
`app.locals` alongside `acpHandle` — the channel `runQwenServe` already
uses to reach into the built app for lifecycle work.

- `bearerAuth` now takes the listener-scoped `CredentialStore`, and the
  ACP WS mount takes the same one so the `qwen-bearer.*` subprotocol
  cannot sidestep the scoping the REST gate enforces.
- The CORS middleware is installed unconditionally over a
  `MutableOriginAllowlist`. With no `--allow-origin` this returns the same
  403 envelope as the `denyBrowserOriginCors` wall it replaces, so the
  default posture is unchanged.
- Daemon teardown disables Local Control before disposing the ACP handle,
  since detaching the LAN listener's upgrade registration goes through it.
  Token revocation and origin removal are synchronous, so they complete
  even though the enclosing dispose scope cannot await the socket close.
- The LAN listener honors `--tls-cert` / `--tls-key`, reading them at
  enable time so a renewed certificate is picked up. Serving plaintext off
  a daemon deliberately put behind TLS would downgrade the more exposed of
  the two surfaces; `status.encrypted` reports which it is.

Refs #9075

* feat(cli): repoint --local-control at the daemon service

The flag stops being a second implementation and becomes a caller.

Previously `--local-control` commandeered the daemon: it bound to
`0.0.0.0`, generated a token that WAS the daemon token, and rewrote the
origin allowlist — which is why it conflicted with `--token`,
`--hostname`, `--allow-origin`, and an ephemeral port. The daemon now owns
a separate LAN listener with a separate revocable credential, so none of
those are in tension. A daemon can serve authenticated loopback and run a
Local Control session at the same time, and `--no-web` is the only
remaining conflict.

- `localControlUrls` is deleted. Its "every non-internal IPv4" policy is
  the bug the service's private/link-local selection replaces; it would
  put a VPN or public address in a QR code.
- Ambiguous multi-network hosts get `--local-control-address <ip>` instead
  of a QR per interface.
- Sleep inhibition moves into the service, so it is held while the LAN
  listener is up and released when it goes down rather than for the
  lifetime of the process.
- The pairing line now reports actual sleep-inhibition and encryption
  state instead of asserting the common case.
- `RunHandle.getLocalControl()` reaches the service; a getter because the
  runtime app is mounted after the listener is up.

Refs #9075

* fix(cli): harden daemon-owned Local Control

* fix(cli): flush Local Control disable response

* feat(desktop): move Local Control into Settings

* fix(local-control): close listener lifecycle gaps

* fix(cli): resolve local control review comments

* fix(local-control): align route lifecycle

* fix(serve): close local control review gaps

* fix(serve): close Local Control QR and bridge-filter review blockers

QR rendering in the Local Control routes is now best-effort: an
over-capacity pairing URL (the target deep-link is caller-influenced)
no longer turns enable/status into a 500 while the LAN listener stays
live, which wedged the Web Shell card with no disable path. The
interface denylist also stops rejecting physical LAN bridges (br0,
Windows "Network Bridge") and only filters the virtual bridge shapes
(Docker br-<hex>, macOS bridge<N>), matching the deleted Rust filter's
per-platform behavior. Adds regression tests for both.

* fix(serve): close round-5 Local Control review findings

- Card: reconcile the selected LAN address on every status update, so a
  stale selection cannot survive a network change when only one candidate
  remains (the selector is hidden in that case and gave no affordance).
- Interface filter: fold the hex run into the Docker bridge token
  (br-[0-9a-f]+) so bridge IDs starting with a letter stop escaping the
  shared boundary check.
- LAN listener: drop the whole-request timeout budget; Node never resets
  it on body chunks, so it 408'd phones trickling large uploads through
  the shared Express app. Header and keep-alive timeouts stay.
- Copy: Ctrl+C ends the whole daemon, not just Local Control (design
  doc, terminal banner, --local-control description).
- Accessibility: aria-live on the card, role=alert on its error line.
- Tests: QR happy path, listen-error handler cleanup, strict
  error-handler count after enable, letter-starting Docker bridge.

* fix(web-shell): preserve local control base paths

* fix(local-control): close round-7 review findings

- card: keep the 409 candidate list on the error path — requestLocalControl
  attaches the parsed payload to the thrown error and toggle reconciles
  status/selection from it, so a stale address after a DHCP change recovers
  without a page remount (R7-3)
- lan-interfaces: match `vpn` as a substring and add a `wintun` token,
  closing the OpenVPN Wintun escape (boundary semantics let `vpn` sit
  inside "openvpn" unmatched) plus mid-word names like vpnkit; regression
  test covers the adapter family (R7-1 demonstrated entrance; structural
  per-platform classification stays a follow-up)
- drop the orphaned strictPort ServeOptions field, the EADDRINUSE-bump
  condition reading it, and its test — no production entry point sets it
  anymore (R7-5)
- docs: refresh 12-auth-security.md / 02-serve-runtime.md for the new
  middleware topology — unconditional allowOriginCors over the mutable
  allowlist on the runtime app, deny wall only in the bootstrap app,
  listener-scoped pairing credential on the LAN listener, and the new
  mutation-gate row (R7-2)
- remove the dead selectLanAddress barrel re-export

* fix(local-control): enforce the loopback-bind precondition on runtime enable

The LAN listener binds the primary listener's port on the selected LAN
address, so a wildcard or LAN primary bind already owns it — the
`--local-control` CLI flag refuses that configuration at boot, but the
runtime enable route (driven by the Web Shell Settings card) skipped the
check and surfaced a 500 `listen EADDRINUSE` with no remediation,
silently unusable for the whole class of non-loopback deployments.

Return 409 `local_control_non_loopback_bind` with the actionable
restart hint instead; loopback binds (127.0.0.1/localhost/::1) stay
enabled via the shared `isLoopbackBind` helper.

* fix(local-control): close round-8 demonstrated escapes + doc/test gaps

R7-1 (demonstrated false negatives): Docker veth peer IDs are
veth<hex> and may be letter-led (vethd4a1b2c), which the bare token's
digit boundary let escape — the token takes the same shape as br-<hex>.
Corporate SSL-VPN adapters (Cisco AnyConnect, GlobalProtect, Pulse
Secure, FortiClient, Cloudflare WARP) carry no `vpn` substring, so
their vendor names are listed explicitly; a sole-candidate VPN address
is no longer silently auto-advertised in the QR. Regression tests cover
all six shapes. The vEthernet-external false positive and the class fix
(structural classification instead of name matching) remain under #9158.

Also: the settings card's status-fetch effect clears a stale error on
re-run and ignores superseded responses; the detach test now connects a
primary-listener client and asserts it survives detachServer (the
per-server filter previously survived a mutation probe); the flags table
gains the --local-control-address row; the design doc states that
--allow-origin origins stay admitted alongside the LAN origin; the three
Host-gate doc surfaces note that the LAN listener always enforces its
advertised-authority Host check.

* fix(local-control): bound slow-body slots + close round-7 adapter escapes (#9106)

- service: replace requestTimeout=0 with a bounded 30-minute whole-request
  budget; an unlimited budget let an unauthenticated LAN client trickle
  bodies and hold every pre-auth connection slot open indefinitely
  (headersTimeout covers only headers, keepAliveTimeout only idle sockets)
- lan-interfaces: add interim vendor tokens for post-rename SSL-VPN
  successors (ivanti, cisco secure, citrix, sonicwall); Ivanti Connect
  Secure (Pulse Secure renamed) escaped the enumerated list and was
  auto-advertised in the QR. Class fix stays tracked in #9158
- auth: document the MutationGateOptions caveat that on a no-token daemon
  the Local Control pairing credential admits loopback callers to the
  strict surface (round-7 design decision still open)

* fix(local-control): stop serving the pairing secret to unauthenticated callers (#9106)

Probe-verified hole: on a tokenless daemon any local process could POST
/workspace/local-control/enable (or GET the unguarded status route) and
read status.url — the pairing token in the fragment — then present it on
the LAN listener, where the strictDenier passthrough admitted it to the
whole strict mutation surface (file writes, memory CRUD, git push/pull,
extension/MCP control) without the operator ever scanning anything.

Close the acquisition step:
- GET status / POST enable / POST disable now return url + qrText only to
  requests bearerAuth actually authenticated (requestWasAuthenticated);
  unauthenticated callers get the status with the secret stripped and
  urlRedacted: true while active
- on an unauthenticated enable the pairing URL is printed to the daemon's
  own terminal instead — the one channel a local attacker cannot read over
  HTTP
- web-shell Settings card renders a terminal hint when urlRedacted (en/zh)
- MutationGateOptions caveat rewritten to the resolved state

Authenticated callers (daemon token) are unchanged. Route tests: redaction
for unauthenticated GET/enable, full payload for authenticated callers,
terminal print on enable; suites 42/42, eslint/prettier clean, Codex
security review CLEAN.

* fix(local-control): close round-9/10/11 review findings (#9106)

- write the pairing URL with writeStdoutLineSafe so a dead/full stdout
  cannot wedge enable into a false 500
- reject an empty --local-control-address instead of silently dropping it
- pin --token/--allow-origin composition through to runQwenServe
- drop stale serve.ts file:line references in credentials/lan-interfaces
- correct the CORS caveat and the design doc's flag/origin claims

* test(serve): drop stale strict-port assertion

* fix(local-control): close round-12 review findings (#9106)

- give the composition test a full enable payload and a handle.close so
  the detached handler cannot leak a real process.exit(1)
- wrap the QR dynamic import + setErrorLevel in withUiData's fault
  isolation so a broken qrcode-terminal degrades to the raw URL instead
  of 500ing every status/enable
- fix the ZH urlRedacted copy (it prints a URL, not a QR)
- finish the denyBrowserOriginCors -> allowOriginCors doc sweep in
  01-architecture.md and 18-error-taxonomy.md

---------

Co-authored-by: yiliang114 <yiliang114@users.noreply.github.com>
2026-08-17 16:44:48 +00:00
jinye
f16975ee56
feat(serve): Add workspace session live-state endpoint and catalog version (#9261)
* docs(serve): Design workspace session live-state protocol

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

* docs(serve): Refine workspace session live-state design

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

* feat(serve): Add workspace session live-state endpoint and catalog version

Add GET /workspaces/:workspace/sessions/live-state: a memory-only
volatile snapshot (clientCount, hasActivePrompt, waiting flags) plus an
in-memory catalog version (generation+revision equality token), so
clients stop polling the persisted catalog for volatile state.

The bridge owns the clock: registration/removal marks flow through the
emitSessionLifecycle choke point; rename, automatic title, worktree,
and persisted branch commits mark at exact points; serve-layer REST/ACP
mutations share an invalidate-then-mark helper with exact no-op
semantics (deleted:false group deletes, removeSession:false cleanups,
no-op renames). The route exposes a new version only after
invalidating both persisted catalog scopes, enabling the client
live-A -> full catalog -> live-B reconciliation handshake.

Wire-additive: new unconditional capability
workspace_session_live_state, TypeScript SDK types and
DaemonClient/WorkspaceDaemonClient methods (native REST, no per-poll
capability preflight), telemetry label, and protocol/capability/SDK
docs. Required clock methods on AcpSessionBridge are a source-level
contract change for external structural implementations; in-repo fakes
updated.

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

* test(serve): Cover workspace_session_live_state in the serve integration baseline

The capabilities envelope E2E asserts the exact advertised feature list;
the new unconditional live-state capability must appear after the
archived-export tag, matching registry declaration order.

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

* docs(serve): Close the handshake-vs-single-request argument gap

Separate the two claims the earlier paragraph conflated: the catalog
path cost is irrelevant to carrying a version (it runs anyway on a full
reload), but stamp placement decides consistency. Stamp-after-scan can
silently accept a bundle missing a mid-scan mutation; stamp-first is
safe and self-heals within one poll cycle, which a client may
legitimately choose. The A/B handshake buys provable consistency for
one extra cheap live-state read; the server supports both and the
Web Shell PR picks per product tolerance.

* fix(serve): Mark catalog version on persisted session renames

The metadata route's SessionNotFoundError fallback renamed persisted
sessions without advancing the catalog revision, so version-watching
clients kept the stale display name. Mark after a successful persisted
rename (parity with the live path, which marks on an actual change).

Also reconcile the design doc summary with its Implementation
Boundaries (the implementation ships in this PR, not a follow-up) and
spell out the child-recording persistence mechanism behind the
auto-title catalog mark.

* test(serve): Pin catalog-mark and live-state behaviors from the review round

- Assert markSessionCatalogChanged in the scheduled-task rollback
  (including the no-op-removal negative case), the sub-session and
  Live coordinator rollback paths, and the never-live orphan
  deletion; previously each mark could regress with suites green.
- Cover the live-state route's ?? false projection for both wait
  flags, and its first-exposure invalidation arm (revision
  unchanged, both organized scopes refilled).
- Cover the side-task generation-closed rollback arm (kill, remove,
  catalog mark).
- Compile the SDK live-state type fence via tsconfig.test-fence.json
  so shape assertions really pin the wire contract; the default
  tsconfig excludes test/.
- Align the design doc's cache-consistency goal with the cache
  mechanics (waiters joined before an invalidation may resolve, but
  cannot install).

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-08-17 15:01:50 +00:00
samuelhsin
5abd367f47
feat(daemon): attach skill-toggle mutation metadata to settings_changed (#9051)
* feat(daemon): attach skill-toggle mutation metadata to settings_changed

Hosts can apply Skill toggles incrementally without a full task reload or suppressing skills.* events.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(review): fit skill-toggle mutation metadata in the SDK bundle budget

The new normalizer parser pushed the browser daemon bundle over the 186KB cap. Raise it to 187KB and pin the review gaps that were cheap to close.

Co-authored-by: Cursor <cursoragent@cursor.com>

* test(daemon): pin skill-toggle mutation event count and parser edges

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(sdk): raise daemon browser bundle budget for skill-toggle metadata

The 190KB cap overflowed by 491 bytes after merging main, so the SDK build fails before tests run.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-08-17 05:01:41 +00:00
jinye
889f0d8bbd
feat(daemon): Isolate the Conversations runtime boundary (#9181)
* feat(daemon): isolate the Conversations runtime boundary

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

* codex: fix CI failure on PR #9181

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

* codex: fix CI failure on PR #9181

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

* codex: address PR review feedback (#9181)

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

* codex: address PR review feedback (#9181)

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

* codex: address PR review feedback (#9181)

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

* codex: address PR review feedback (#9181)

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

* codex: address PR review feedback (#9181)

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

* codex: address PR review feedback (#9181)

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

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-08-16 17:32:45 +00:00
Heyang Wang
9f8f65dde0
feat: support fork from any conversation (#8817)
Some checks are pending
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 1/2 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 2/2 (push) Waiting to run
E2E Tests / channel-plugin E2E (nightly) (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
SDK Java / ubuntu-latest / Java 11 (push) Waiting to run
SDK Java / ubuntu-latest / Java 17 (push) Waiting to run
SDK Java / macos-latest / Java 21 (push) Waiting to run
SDK Java / ubuntu-latest / Java 21 (push) Waiting to run
SDK Java / windows-latest / Java 21 (push) Waiting to run
SDK Java / Real daemon E2E / Java 11 (push) Waiting to run
Security Checks / Dependency CVE audit (push) Waiting to run
Security Checks / Secret scan (TruffleHog) (push) Waiting to run
* feat(web-shell): branch from completed assistant responses

Add durable response checkpoints so Web Shell sessions can branch from
eligible completed Assistant turns without mutating the source history.

- Record and validate checkpoints behind serialized topology fences
- Preserve historical anchors through replay, daemon, SDK, and UI layers
- Publish bounded forks with crash-safe ownership and referenced backups
- Serialize prompt, rewind, branch, automatic turn, and close mutations
- Cover stale anchors, replay pagination, cleanup, and pending UI states

Note: Responses recorded before this change remain non-branchable.

# Conflicts:
#	packages/acp-bridge/src/bridge.ts
#	packages/acp-bridge/src/bridgeTypes.ts
#	packages/cli/src/acp-integration/acpAgent.test.ts
#	packages/cli/src/acp-integration/acpAgent.ts
#	packages/cli/src/serve/routes/session.ts
#	packages/cli/src/serve/server.test.ts
#	packages/core/src/services/chatRecordingService.ts
#	packages/core/src/services/sessionService.test.ts
#	packages/core/src/services/sessionService.ts
#	packages/sdk-typescript/src/daemon/DaemonClient.ts
#	packages/web-shell/client/components/MessageItem.tsx
#	packages/web-shell/client/components/MessageList.tsx

* fix(session): preserve historical branch checkpoints

Keep Assistant-response branching intact across the daemon stack after
rebases, including history serialization and persisted-session ownership.

- Forward durable checkpoint IDs through Bridge, SDK, and UI layers
- Serialize live history mutations and retain valid nested branch anchors
- Preserve persisted branches during generation cleanup
- Add cross-layer regression tests for replay and stale checkpoints

* fix(web-shell): harden response session branching

* chore: remove PR comment evaluation artifact

Keep the PR review report as a local ignored backup instead of
shipping it with the feature branch.

- Remove the generated PR comment evaluation from tracked files
- Preserve the report under the ignored analyze directory

* fix(web-shell): guard historical branch mutations

Historical branch requests could outlive the client timeout during an
active turn, and interactive forks lacked the recorder's cross-process
writer-lease barrier.

- Hide Assistant Branch actions while a turn is active
- Run interactive fork creation inside the recorder write barrier
- Use the concrete checkpoint recorder contract in Session
- Document committed-session ownership and implemented design status

* perf(core): index historical branch points during transcript scan

Build branch catalogs during the frozen index scan so the first history
page no longer reopens and materializes the complete active chain.

- Retain a compact projection for shared branch-point resolution
- Correlate live branch anchors with the completed prompt and final reply
- Complete recorder mocks required by the concrete Session contract
- Update the reviewed design with performance and correlation invariants

* fix(core): address review findings — dead code, boundary remap, promptId guard, stale toast (#8274)

* fix(core): address review findings — dead code, boundary remap, promptId guard, stale toast (#8274)

* fix(core): address review findings — archived GC, subtype registration, UUID validation, dead code (#8274)

* test: strengthen branch-point and fork coverage from review (#8274)

Add focused tests requested in PR review:
- branch catalog resolves checkpoints that fall on a later page
- accept a parallel tool batch closed within a single turn
- exercise the linkSync->copyFileSync fork backup fallback success path
- prove a remapped checkpoint stays usable via a nested fork
- isolate each branch-point validation conjunct across bridge and SDK

* fix: address round-4 review feedback for session branching (#8274)

- Make the directory-fsync durability test platform-aware (skip on win32),
  since fsyncDirectoryBestEffort swallows the injected error on Windows and
  the rejection path is non-Windows by design.
- Reject atRecordId on the side-task fork path instead of silently discarding
  it, so the API surface no longer implies acceptance.
- Correct the design doc: name the real promptQueue FIFO (not the nonexistent
  historyMutationQueue) and describe filtered checkpoint boundaries as
  remapped to the nearest retained predecessor, not unconditionally null.
- Add focused tests: branch-point assistantRecordUuid mismatch rejection, and
  insight-block branchRecordId anchoring (insight-only block must not anchor
  onto the previous reply).

* fix: address round-5 review feedback for session branching (#8274)

* fix: address round-6 review feedback for session branching (#8274)

* fix: address round-7 review feedback for session branching (#8274)

* fix(core): harden branch-point resolution against malformed transcript shapes (#8274)

- Filter null/non-object part elements in the shared branch resolver so a
  transcript containing null parts no longer makes forkSession throw a
  TypeError for every checkpoint.
- Tag tool calls carried in from the pre-boundary prefix so a dangling call
  left by a crashed turn no longer permanently disables checkpoint
  recording; only calls issued inside the turn must close.
- Merge duplicate-uuid records first-wins for identity fields in the
  transcript reader, matching the byUuid index and fork aggregation, so the
  reader never advertises a branch marker the fork path must reject.

* fix: address round-8 review feedback for session branching (#8274)

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

* fix: address round-9 review feedback for session branching (#8274)

* test(core): pin branch GC isolation from throwing warning callbacks (#8274)

* fix(acp-bridge): reject rewind at admission while a prompt is active (#8274)

* fix(web-shell): harden session branch publication

Preserve direct ACP prompt preemption while fencing branch and rewind history mutations at the Session boundary. Convert branch publication, backup staging, cleanup, and stale-claim GC to asynchronous filesystem APIs, and surface unsupported hard-link commits as typed ACP and HTTP errors. Expand regression coverage and update the reviewed design contract.

* fix(web-shell): harden historical response branching

Reject branches during prompt admission and keep dispatched mutations
owned until their real outcome is known.

- remove detached timeouts across ACP, SDK, and WebUI
- bound branch cleanup and make title scans asynchronous
- avoid full branch-point scans during transcript pagination
- add regression coverage from Core through the real daemon and browser

* fix: address round-11 review feedback for session branching (#8274)

* fix: address round-12 review feedback for session branching (#8274)

* refactor(branching): remove branch-specific overdesign

Simplify historical session branching around the minimum persistence,
recording, and navigation invariants required by the Web Shell flow.

- Replace branch claims and garbage collection with staged publication
- Validate completed turns incrementally instead of reloading transcripts
- Separate persisted branch creation from live session restoration
- Bound SDK waits and prevent late results from replacing navigation
- Remove unused checkpoint prompt IDs while reading legacy records

Note: A pre-commit crash may leave hidden staging or orphan backups.

* chore(sdk): update browser bundle budget

Account for the combined historical branching and transcript projection APIs after merging main while keeping the browser bundle size guard narrowly bounded.

* fix(branching): address review lifecycle gaps

Harden historical session branching against cancellation, observer,
navigation, and shutdown races found during review.

- Normalize cancellation keys and bound close-time mutation waits
- Preserve anchors after observer completion and load persisted forks
- Report success only when the guarded session switch starts
- Cover recorder cursors, fork cleanup, admission, and rollback
- Align daemon events and branch errors with runtime behavior

* refactor(session): simplify branching safeguards

Reduce the session branching surface after review while preserving the
critical concurrency, durability, and ownership guarantees.

- Remove the unused full-chain resolver and test production entry points
- Copy backups from verified open handles instead of using hard links
- Reuse the bounded title scan instead of maintaining an async mirror
- Deduplicate UI branch requests and fail fast for busy automatic turns
- Consolidate repeated mutation tests and retain critical race coverage
- Document the retained invariants and rejected overdesign explicitly

* test(branching): simplify regression coverage

Reduce duplicated branching tests while retaining regression coverage for
the safety, concurrency, and lifecycle fixes introduced by this feature.

- Consolidate symmetric bridge and agent scenarios with table-driven cases
- Remove repeated cross-layer assertions and brittle implementation spies
- Drop redundant UI permutations and branch-only visual snapshots

* fix(serve): handle branch busy admission

* fix(sdk): preserve v1 branch session contract

Keep existing latest-state branch callers source- and wire-compatible while
retaining the persisted-only behavior for historical checkpoint branches.

- Restore no-anchor branches before returning their live client identity
- Add a separate typed result for persisted historical branch requests
- Clean up restored attachments on stale navigation and disconnect races
- Cover immediate continuation and historical persistence independently

* fix(daemon): guard branching history mutations

Prevent branch creation and automatic Goal turns from racing session
teardown or interactive history mutations.

- Reject branch admission while a conditional close is authorized
- Serialize Goal continuations behind the history mutation gate
- Limit branch checkpoints to interactive prompts
- Add regressions for close and Goal scheduling races

* fix(branching): preserve fork and checkpoint semantics

Keep branch checkpoints and file-history snapshots correct across resumed,
forked, and non-interactive session flows.

- Track the restored active-chain base before the first appended turn
- Preserve backup file modes during fork publication
- Exclude authenticated channel prompts from checkpoint recording
- Add regressions for all three review failures

* fix(branching): harden branch and rewind behavior

Handle the remaining branch and rewind review findings without widening
the feature contract.

- Ignore benign concurrent branch rejections in the Web Shell
- Validate rewind prompt IDs before using string operations
- Pin mutation ordering, cleanup, compaction, and checkpoint invariants
- Align sourced-fork fixtures with the canonical side_task value

---------

Co-authored-by: heyang.why <heyang.why@alibaba-inc.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: 易良 <1204183885@qq.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: qwen-code-bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-08-15 10:01:49 +00:00
Shaojin Wen
4257916e7e
feat(daemon): guard cross-worktree Git mutations (#8687)
* feat(daemon): guard cross-worktree Git mutations

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

* fix(daemon): keep Git guard off serve fast path

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

* fix(serve): close daemon Git guard parser bypasses

Rebuild the daemon-side Git relocation guard parser so the runtime-verified
bypasses from review are closed: comment/glob tokens, backslash
continuations, shell wrapper and path-qualified invocations, cwd-shifting
builtins, env-var relocations, gitfile/symlink/worktree-admin indirection,
-C vs relative git-dir ordering, --output and textconv-capable read-only
subcommands, command-valued -c config, and dynamic expansion forms all fail
closed for mutations outside the session working directory. Command
splitting, canonicalization, and containment now reuse the core helpers.

Key the child-side v1 restrictions (/fork, agent-backed workspace memory)
and per-call daemon round trips on a real external provider being attached
instead of on guard plumbing presence: under the built-in guard alone,
hidden-agent tool calls traverse the same daemon-side policy, so those
features stay available and non-shell tools resolve locally.

Denial reasons are length-clamped and control-character-stripped so they
always satisfy the guard result validation.

* fix(serve): keep daemon Git guard out of serve fast-path closure

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

* fix(serve): close re-reviewed daemon Git guard bypasses and subagent regression

Address the re-review at b1b7606: keep the outermost entry cwd as the
containment basis inside shell wrappers, fail closed on unrecognized
programs that still reference a relocated Git command, skip leading
shell keywords, deny undecidable and fused `-c` payloads, inspect
command-executing `-c` config before the read-only allowance, drop
`grep`/`status` from the relocated read-only set, validate the
model-supplied `directory` against the effective working directory,
and stop modelling `--exec-path`/`--list-cmds` as value-taking.

Context-less shell paths (subagents, cron turns, background
notifications, resumed background agents) previously failed closed
under the now-unconditional managed guard: fall back to the
scheduler-owned session id and validate those requests by session
ownership, while external-provider consultation still requires a
prompt binding. Move the guard's canonicalization off the daemon
event loop with a promise-based realpathNearestExisting, drop the
unread workspaceCwd request field, and restore the top-level guard
import in run-qwen-serve.

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

* fix(serve): restore lazy daemon Git guard import

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

* fix(serve): close daemon Git guard shell front-end bypasses

All six forms below were reproduced against the real guard and confirmed to
really escape the boundary with a real shell and git 2.47.3 (the outside
worktree was reset, or the textconv marker file was created outside).

- `cat-file --textconv`/`--filters` run programs configured by the *target*
  repository, so the relocated read-only allowance no longer applies when a
  `--textconv`, `--filters`, or `--output` flag is present, wherever it
  appears in the invocation.
- `export GIT_WORK_TREE=<outside> && git reset --hard` hid the relocation in
  its own segment. `export`/`declare`/`typeset`/`readonly` operands (and plain
  assignments after `set -a`) are now recorded as exported relocations that
  apply to every later command in the chain, including wrapper payloads and
  substitution bodies.
- `builtin cd <outside>` masked cwd tracking; `builtin` now takes the same
  prefix-skipping path as `command`.
- `cd -P <outside>` consumed the flag as the directory operand, so containment
  was evaluated against `<cwd>/-P` — inside the boundary whenever such a
  directory exists. `cd`/`pushd` option words are skipped when locating the
  operand.
- `cd <outside> && nice git reset --hard` passed because an unrecognized
  program word was only checked for relocation markers. It is now also denied
  when the tracked working directory is unknown or already outside the
  boundary, while the same command inside the boundary stays allowed.
- `echo $(git -C <outside> reset --hard)` was folded into an opaque token.
  `$(…)` and backtick bodies are extracted from the raw segment and analysed
  as nested commands; `$((…))` is stepped over as arithmetic and an
  unterminated substitution is denied.

The guard also covers the `monitor` tool, which spawns its `command` through
the same shell with the same `directory` argument and was previously
short-circuited to allow by the child before the daemon ever saw it. The
shell-executing tool set is shared through acp-bridge so the child and the
daemon policy cannot drift apart.

* fix(serve): restore the shell-wrapper analysis return shape

The unrecognized-shell-wrapper branch of `analyzeRun` still returned the
pre-`assignmentsOnly` `other` shape, so `tsc --build` failed on the union.
Every CI job that installs dependencies runs that build, so all three went
red on it.

* fix(serve): close the remaining daemon Git guard front-end gaps

Round-4 review findings, each reproduced against the real guard before the
fix and pinned by a regression test with an in-boundary control.

- An unrecognized program word only failed closed on Git-flag markers, so a
  `cd` inside its quoted payload slipped through (`su -c 'cd <outside> && git
  reset --hard'`, the same through `xargs … sh -c`). `cd`/`pushd` now count as
  relocation markers in that scan.
- The Git word was matched case-sensitively while program classification
  lowercases, so `nice GIT reset --hard` escaped on a case-insensitive
  filesystem.
- A program word the daemon cannot read at all (`cd <outside> && $CMD git
  reset --hard`, also behind `command`) skipped the containment check that
  unrecognized program words get.
- `export GIT_DIR` with no `=` exported an earlier shell-local assignment
  invisibly; shell-local GIT_* assignments are now tracked and promoted on a
  name-only export. `+=` appends and `set -o $OPT` are recorded as unresolved.
- `eval` propagated only its cwd, so `eval 'export GIT_WORK_TREE=<outside>'`
  lost the export it performed in the current shell.
- `--shallow-file` and `--attr-source` were not modelled as value-taking, so
  their value was read as the subcommand — which ends option parsing and hid
  every relocation after it.
- The command-executing config set missed `core.sshCommand`,
  `diff.<driver>.textconv`/`command`, `merge.<driver>.driver`,
  `sequence.editor`, `gpg.program`, `pager.*`, `core.askPass` and
  `uploadpack.packObjectsHook`, and matched case-sensitively although Git
  config keys are not.
- `env -S` accepted a dynamic payload as literal text and ignored the fused
  `env -S'cmd'` form.

Docs: the Non-goals list claimed `CoreToolScheduler` was untouched although
this PR adds the scheduler-owned `sessionId` to the guard context; the
protocol capability row and the user guide still described the guard as
`run_shell_command`-only and mis-stated the denial-message prefixes; and the
relocation-never-revoked over-approximation (`unset GIT_DIR` does not clear a
recorded relocation) is now stated in Limitations.

* fix(serve): resolve the repository Git discovers, not just the directory

Two more escapes from the round-4 review, both reproduced with a real shell
and git 2.47.3 before the fix.

- A `.git` gitfile inside the boundary redirects Git to an outside
  repository: `git -C <inside>/decoy commit` moved the outside repo's HEAD
  while the directory itself passed containment. A `cwd` target now resolves
  the first `.git` between it and the boundary through the same
  `resolveGitDirRepository` path `--git-dir` targets use, which keeps a
  linked-worktree session working because its own gitfile resolves back to
  that worktree's checkout — pinned by a test that runs a session whose `.git`
  points at an outside admin directory.
- `cd -P <symlink>/..` lands the shell in the parent of the symlink's real
  target, which a lexical resolve places back inside the boundary. A `-P` cd
  whose target contains `..` now drops the tracked directory. The default
  logical form is unchanged and still allowed, because bash resolves it
  against the logical path and really does stay inside.

* fix(serve): match chdir, brace-expansion and here-string semantics

Three more escapes from the round-4 review, each reproduced against the real
guard and then confirmed with a real shell and git 2.47.3.

- `git -C <symlink>/.. reset --hard` escaped: git's `-C` reaches the kernel as
  a chdir, which resolves each component's symlinks, while the guard collapsed
  `..` lexically and landed back inside the boundary. `-C`, `env -C`,
  `sudo -D` and `cd -P` now resolve physically, component by component; bash's
  default `cd` stays lexical because that is what the shell itself does.
- `git {-C,<outside>} reset --hard` escaped: brace expansion happens after
  this parse, so the tokens git receives were never the tokens the guard saw.
  A brace-expansion token now marks the invocation unresolved.
- `sh <<< 'git -C <outside> reset --hard'` escaped: the tokenizer dropped
  redirect operands, and a here-string carries its whole payload in the
  command line. Redirect operands stay in the run, so the here-string is
  scanned like any other token; ordinary `>`/`2>` targets are inert text and
  a regression test keeps them allowed.

Checked and not reproduced, so left alone: `describe --dirty` did not rewrite
the target index, `GIT_OBJECT_DIRECTORY=<outside>` did not write objects
there, and `bash -o allexport -c '…'` cannot export into the parent shell
because the payload runs in a subprocess.

* fix(serve): treat relocated git describe as a target-repo write

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

* docs(serve): state what git describe actually rewrites

Keeping `describe` out of the relocated read-only set is right, but the
reason as written does not match git 2.47.3. Measured against a real
repository with a stale stat cache (mtime-only touch), `describe --dirty`,
`--broken` and `--always --dirty` rewrite the target repository's
`.git/index`, while a plain `describe`, `--tags` and `--always` leave it
untouched. The subcommand still belongs outside the set — the flag is one
token away from any describe a model writes — so only the comments and the
design doc change.

* test(serve): cover the provider-attached marker with a real handshake

The only assertion on the provider marker was the negative case, so a break
in the attached path — the marker comparison or the conditional child-env
spread — would have gone unnoticed. This drives a loopback provider through
the real `/v1/handshake` and asserts the child env carries the attached
marker alongside the plumbing one. Verified load-bearing: forcing the marker
to `undefined` fails it.

* fix(serve): close the round-3 shell and repository-discovery gaps

Every payload below was reproduced against the real guard first; the two
that turn on git's own behaviour were measured with git 2.47.3.

Repository discovery
- `ls-files` executes the target repository's `core.fsmonitor` — the exact
  property that removed `status` — so it leaves the relocated read-only set.
  Measured: `git -C <outside> ls-files` runs the hook; `rev-parse` and
  `cat-file` remain side-effect free.
- The discovery check was tied to a *cwd* relocation, so a `--work-tree`-only
  or bare relocation skipped it. Git discovers its repository from the cwd
  whenever no `--git-dir` names one, so the check now runs on that basis, and
  an unrecognized program (`cd sub && nice git branch`) gets it too. A linked
  worktree whose own gitfile points at an outside admin directory stays
  allowed — pinned from both sides.

Shell front-end
- `eval > /dev/null '…'` swallowed the redirection into its payload and lost
  the command. Redirect operands (and an `N>` descriptor prefix) are now
  flagged: still scanned for markers, never joined into argv.
- `cd` glued to a control operator (`true;cd <outside>`) was not a marker.
- Letters after `c` in a short bundle are more flags, not a fused payload:
  `bash -cx 'cd <outside> && …'` and `sh -co ignoreeof '…'` took their real
  payload from a later argv entry the guard never read.
- `( … )` is now scoped like a subshell: `(cd <outside>); git commit` is
  allowed again, while `(cd <outside> && git reset --hard)` still denies.
- `sudo -R <rootfs>`/`--chroot=` and a `PATH=`/`GIT_EXEC_PATH=` assignment
  make every path the daemon resolves meaningless, so they fail closed.
- `env --unset=NAME`, `-uNAME` and `--split-string=` in their attached forms
  no longer read as unrecognized options, which was denying decidable
  commands.
- Values assigned earlier in the same command are substituted before the
  dynamic-program check (`X=git; Y='-C <outside> …'; $X $Y`), `eval` carries
  its shell locals back out, and `export $NAME` fails closed.
- A command that relinks a path (`ln`, `mv`) invalidates containment proved
  afterwards: `ln -s <outside> bait && git -C bait reset --hard` is checked
  while `bait` is still the original directory.
- `resolvePhysicalPath` treated `\` as a separator on POSIX, where it is an
  ordinary filename character.
- `gpg.<format>.program` and `core.hooksPath` join the command-executing
  config keys.

* fix(serve): scope the relink invalidation to path-resolving Git runs

The rule I added a commit ago denied every Git run that followed an `ln` or
`mv` in the same command, which takes out `mv old new && git add -A` — as
ordinary as it gets. The invalidation only makes sense for an invocation that
resolves a path, so it now requires a relocation target or a shifted cwd:
`ln -s <outside> bait && git -C bait reset --hard` still denies, while
staging renamed files does not.

* fix(serve): rebuild the relink defense and close the round-4 gaps

Reproduced against the real guard first; the two environment claims were
measured with git 2.47.3.

Fixing my own two previous commits
- The relink invalidation was both too wide and too narrow. It now records
  which paths a run may have re-pointed instead of setting one flag: a
  relinked `.git` invalidates repository discovery for every later command
  (`ln -s <outside>/.git .git && git status` mutated the outside repo), while
  a relinked directory only affects a run that resolves that very path, so
  `mv old new && git add -A` stays allowed. The scan no longer keys on
  `run[0]`, which `env ln …`, `X=1 ln …` and `nice ln …` walked straight past,
  and `cp -s` joins `ln`/`mv`.
- Flagging redirect operands (the here-string fix) left `consumeShellWrapper`
  reading one as the `-c` payload: `sh -c > /dev/null 'git -C <outside> …'`
  dropped the real payload from analysis.
- Leaving a subshell rolled back only the tracked cwd, so exports and
  shell locals made inside `( … )` kept denying later commands.
- Shell-local reconstruction was last-assignment-wins, losing `X+=` appends.

Repository discovery
- A non-relocated Git command never reached discovery, so a planted `.git`
  gitfile at the session root redirected plain `git commit` outside. Discovery
  now runs for those too; a session bound below its repository is unaffected
  because the walk stops at the boundary, and a linked worktree still resolves
  back to its own checkout — both pinned.

Environment and parsing
- `GIT_OBJECT_DIRECTORY`, `GIT_ALTERNATE_OBJECT_DIRECTORIES`, `GIT_CONFIG*`
  and `SHELLOPTS` mark the invocation unresolved. Measured:
  `GIT_OBJECT_DIRECTORY=<outside>/.git/objects git add` writes the blob there,
  and `GIT_CONFIG_GLOBAL=<outside>/cfg` makes git read that config.
- `$'…'` is ANSI-C quoting where a backslash escapes, so `$'a\'b'` no longer
  leaves the substitution scanner a quote out of phase — which had hidden a
  following `$(git -C <outside> …)` from every analysis pass.

Over-denial
- A program with its own `-C` (`grep -C 5 git`, `tar -C dir`) no longer reads
  as a Git relocation.

* fix(serve): carry relink and shell state across nested scopes

Round-5 findings, all reproduced against the real guard. Four of the five
are in the machinery I added over the last two commits.

- Relink state was local to one `evaluateCommandWithCwd` call, so a symlink
  created inside `sh -c '…'`, `eval '…'` or a `$(…)` body was invisible to the
  parent, and a relink made in the parent was invisible to a nested Git run.
  It is now shared by reference in both directions.
- Nothing consulted it for an unrecognized or dynamic program word, so
  `… && nice git add -A` after a relinked `.git` was allowed, and
  `X=ln; $X -s <outside>/.git .git` recorded nothing at all. A dynamic program
  word may itself be `ln`, so its operands are recorded too.
- `<(…)` opens a paren that shell-quote reports without one, while its `)`
  still arrives — so `(cd <outside>; <(true); git reset --hard)` popped the
  subshell early and lost the `cd`. My round-4 triage called this one "not
  reproduced" because the probe used the top-level shape, which survives on
  the `Math.max(0, …)` clamp; the nested shape does not.
- `eval` ran with an empty shell-local map, so
  `GIT_DIR=<outside>/meta; eval 'export GIT_DIR'` promoted invisibly. Locals
  now flow into `eval` and into substitution subshells (by copy, since their
  own assignments die with them); a `sh -c` subprocess still gets none.
- Any unreadable word in a shell wrapper's argv can be the `-c` that carries
  the command, so `bash $A "$P"` is undecidable rather than absent.

* fix(daemon): evaluate the guard against the directory the tool runs in

A sub-agent pinned to a worktree — `working_dir`, or `isolation`, which sets
`ov.targetDir` on the child Config — executes at `config.getTargetDir()` while
still reporting the parent session id. The guard context carried only that
session id, so the daemon evaluated every such call against the parent
session's `effectiveCwd`: a plain `git commit` from an isolated sub-agent was
judged in-boundary and allowed while running somewhere else entirely, and a
relative `-C` resolved against a directory the command would never be in.

The invocation now carries the directory it will run in, from
`config.getTargetDir()` through the child guard to the daemon. It is
explicitly untrusted, so the daemon accepts it only where it can verify it
from state it owns: inside the session's effective working directory, or
inside the worktree tree that session owns — worktrees live under
`GitWorktreeService.getWorktreesDir(<session id>)`, and the session id is
already validated by `ownsSession`. Anywhere else the scope cannot be
established and the call fails closed.

When an owned worktree is accepted it becomes the boundary, so an isolated
sub-agent is contained to its own worktree instead of to its parent's
checkout — reaching back into the parent is now denied, which is the escape
this was reported for.

* fix(daemon): reach the real guard path and close the round-6 escapes

The headline finding is that the previous round's fix never reached the code
it was written for. `Session.runTool` — the path daemon ACP sessions actually
execute tools through — built the guard context without `sessionId` and
without `cwd`, so both the session fallback this PR added and the execution
directory added last round were unreachable there. Both are now supplied,
exactly as `CoreToolScheduler` does.

Escapes, each reproduced against the real guard first:
- `GIT_SSH_COMMAND`, `GIT_EDITOR`, `GIT_SEQUENCE_EDITOR`, `GIT_ASKPASS`,
  `GIT_PAGER`, `GIT_EXTERNAL_DIFF`, `GIT_SSH` are programs git executes, and
  `GIT_CONFIG_PARAMETERS`/`GIT_CONFIG_COUNT`/`GIT_CONFIG_KEY_<n>` are its
  environment config channel — none were modelled.
- `diff.external`, `core.gitProxy`, `interactive.diffFilter`,
  `credential.<url>.helper`, `remote.<name>.uploadpack`/`receivepack`/`proxy`,
  `tar.<format>.command`, `browser.<tool>.cmd`, `web.browser`,
  `help.browser`, `gc.recentObjectsHook` and `ssh.variant` join the
  command-executing config keys.
- An unrecognized wrapper laundered that config: `nice git -c alias.pwn='!…'`
  never reached the git analysis. It is checked there too now.
- `find <outside> -execdir git reset --hard` relocates through the program's
  own flag, leaving no marker.
- An archive decides where it writes, so `tar`/`unzip`/`cpio`/`rsync` make
  their extraction directory suspect rather than their operands.
- `alias g='git reset --hard'; cd <outside>; g` and the function-definition
  form both defer a body to wherever the bare word is later used.

Over-denials, all introduced by earlier rounds of this PR:
- `SHELLOPTS=errexit git status` (SHELLOPTS is bash's own options state — the
  rationale I gave for listing it was simply wrong, and it never reproduced
  as an escape), `env --ignore-environment`/`--null`/`--debug`,
  `curl -C - …`, `env -iS 'cmd'`, `d=<inside>; cd $d; git status`, and
  `set +a` turning allexport back off.

Also: the sub-agent worktree test created and recursively deleted a directory
under the user's global Qwen dir, keyed on a session id a real session could
own. It now uses a process-unique id and cleans up in `finally`.

* fix(daemon): contain a sub-agent to an in-project agent worktree

`AgentTool` with `isolation: 'worktree'` provisions under
`<projectRoot>/.qwen/worktrees/`, which is inside the session — so the
acceptance rule's "inside the effective working directory" branch left the
boundary alone and one sub-agent could still reach into a sibling's worktree,
the very thing this PR is named for.

A reported directory that is a checkout root in its own right now becomes the
boundary wherever it lives, not only under the session-owned worktree tree.
An ordinary subdirectory resolves to the session's own repository and changes
nothing, which is what keeps `cd packages/cli && git commit` working.

Also drops the unread `entryCwd` parameter from `evaluateUnrecognizedRun`,
whose signature implied containment behaviour that function never had, and
covers the child-side `invocationCwd` forwarding with a direct test — it is
the only link between a pinned sub-agent's real execution directory and the
daemon's check.

* test(acp): assert the session identity and cwd the guard now receives

Adding `sessionId`/`cwd` to the guard context in `Session.runTool` changed
the shape these two assertions pin, and I ran the guard, acpAgent and serve
suites but not this one — CI caught what I should have.

* fix(daemon): record ordinary targets a dynamic relinker re-points

The dynamic-program branch resolved the operands of an unreadable program
word but only used them to raise the `.git` flag, so an ordinary target was
never added to the relink set: `X=ln; $X -s <outside> src && git -C src reset
--hard` validated `src` against what it pointed at before the same command
replaced it.

Ordinary operands are now recorded alongside the `.git` case. Verified
load-bearing: dropping the new line fails the added regression test and
nothing else.

* fix(daemon): close the round-7 escapes, verified with the reported payloads

The reviewers were right that my previous round's "denies as written" replies
were built on non-equivalent counter-probes: a `.git` inside the path, or a
literal `-C`, tripped an unrelated marker rule while the reported mechanism
went untouched. Re-run with each payload verbatim — against a path with no
Git word in it — five of them were allowed. All are fixed, and the new tests
use those exact payloads.

- `$'…'` is ANSI-C quoting only OUTSIDE double quotes, so the substitution in
  `echo "$'$(GIT_DIR=<outside>/.git git reset --hard HEAD~1)'"` is live. Both
  scanners now gate the skip on `!single && !double`.
- The export attribute sticks to the name: after `export GIT_DIR`, a LATER
  assignment to it reaches the git subprocess. Name-only exports of a
  relocation key are recorded so those assignments count as exported.
- Both sides of a pipe run in subshells, so a pipe-side `cd` must not move
  the shell. Segments that are pipeline components restore the directory they
  started with; the top-level separators are read with the same quoting rules
  `splitCommands` uses, and any disagreement falls back to treating every
  segment of a piped command as a component.
- A bare digit before a *spaced* redirect is a real argv word, not a file
  descriptor, and nothing in the token stream distinguishes it — so it is
  marked ambiguous and a payload built from it fails closed instead of
  silently dropping the word.
- `-o`/`-O` before `c` in a short bundle does not cancel the `c`: bash still
  executes it, taking the command from a later argv entry. The bundle is now
  parsed for how many entries the value flags consume on either side.
- Rebuilding a command line out of separate argv words re-quotes anything
  that would otherwise split, so a path with a space stays one word and a
  `-C` value cannot shrink. `eval` keeps the verbatim join it needs, since it
  re-parses its argument as shell text.

* fix(daemon): repair the round-7 patch and bound what this guard promises

Two halves.

First, the round-7 patch introduced seven defects of its own, six of them
reproduced here before fixing:

- the fd digit of `2>…` was eligible as a `-c` payload, because
  `nextArgvIndex` skipped only redirect-flagged tokens;
- `o`/`O` letters after `c` in a bundle were counted by presence rather than
  per letter, shifting the extracted payload left;
- `env --split-string=` still rebuilt its payload with the verbatim join
  while both sibling branches had moved to the re-quoting one;
- the separator scan recorded no lone `&` and mistook `>|` for a pipe, and
  its disagreement fallback scoped nothing instead of everything;
- the export-attribute set neither crossed `eval` nor rolled back with a
  subshell;
- deferred alias and function bodies were keyed on `run[0]` rather than on
  the program word, so any prefix hid them.

Second, and more important than any single rule: the docs now bound what
this control claims. It is reliable against Git relocation written in the
literal forms the design doc lists — the mis-targeted command it exists for
— and best-effort, not a boundary, against shell text written to defeat it.

Seven rounds of adversarial review support that framing rather than
contradict it: each round closed the reported bypasses and the next found
more, several inside the rules the previous round added. The gap is
structural — the guard reads command text before a shell interprets it — so
the honest fix is to move the decision off the text, deciding where a command
may write when it runs rather than predicting it beforehand. That is a
separate change with its own design, and this one should not grow into it by
accretion. Saying so plainly is itself a safety property: an operator who
believes the daemon cannot reach a sibling worktree would grant it more trust
than the mechanism earns.

* fix(daemon): close the round-9 critical forms an agent may actually emit

Scoped to the Critical findings that reproduced against the real guard with
the reviewer's payloads verbatim (Git-word-free path). Common shell forms,
not adversarial exotica; the parser-edge tail stays under the bounded promise
this PR now documents.

- `&>`/`&>>` is a redirect operator, no longer read as a background `&`.
- `function NAME { … }` (the keyword form, `()` optional) is recognised as a
  definition.
- `git -c include.path=`/`includeIf.<cond>.path=` pull in a config file the
  guard cannot read; it can carry a `core.worktree` redirect or executable
  config, so it is treated as dangerous config and fails closed.
- `imap.tunnel`, `instaweb.httpd` join the command-executing config keys, and
  `GIT_DIFFTOOL_EXTCMD` the executed-env keys.
- `GIT_DIR=… set -a` persists (a prefix assignment on the special builtin
  `set`) and exports; that leading assignment is now carried, not dropped.
- alias/function recognition starts at the program word — past a leading
  redirect (`2>/dev/null alias …`), keyword (`if …; then alias …`) or
  assignment — and records every pair of a multi-alias statement.
- a heredoc body is stdin data, not commands: it is stripped before command
  splitting so a body `cd` cannot launder the tracked directory.
- a function body that `splitCommands` cuts across segments is now captured
  whole and replayed, so a `-C <outside>` inside it is seen, not just the
  name.

Two round-9 Criticals are deliberately not "fixed" here: `cd <outside> &
git …` runs git in the parent shell at the in-boundary cwd, so allowing it is
correct; and an archive that plants a `.git` for a later path-less discovery
is a TOCTOU (the unpack happens after the decision), left to the same
limitation as the symlink race rather than denying every `tar && git commit`.

* fix(daemon): replay an alias with the args its invocation appends

`alias gg='git'; gg -C <outside> reset --hard` ran `git -C <outside> reset
--hard`, but the guard replayed only the recorded body (`git`) and dropped
the appended argv, so the relocation was invisible and the command was
allowed. An alias now replays as `body + trailing args`, so the invocation's
own `-C <outside>` is seen. A function is unchanged: its args arrive through
`$@` inside the body, which the recorded body already carries.

Verified with in-boundary controls (`alias gg='git'; gg status`,
`alias gg='git commit'; gg -m x`) staying allowed.

* fix(daemon): carry a function/alias body's cwd and exports to the caller

A shell function and an alias both run in the current shell, so a `cd` or an
export inside the recorded body survives the call. The replay discarded
`nested.cwdAfter` and the exported state, so `f() { cd <outside>; }; f; git
reset --hard` kept the old in-boundary tracked cwd and the path-free git
mutation was judged inside while the real shell had moved outside. The nested
cwd, exports and shell-locals now propagate back, exactly as an `eval`
payload already does. Distinct from the earlier case where git appeared in
the body itself.

Verified: `f() { cd nested; }; f; git status` and `f() { echo hi; }; f; git
commit` stay allowed.

* fix(daemon): inherit the caller's allexport into a same-shell body

A body run in the current shell — `eval`, an alias, or a function — inherits
the enclosing `set -a`, so a plain `GIT_WORK_TREE=<outside>` assignment there
is exported to the following git. The nested evaluation initialized
`allExport` to false instead of the caller's value, so with allexport on the
assignment was treated as shell-local, no relocation was recorded, and the
path-free mutation was allowed. `allExport` now flows into the same-shell
scopes (and back out). An unexported assignment stays shell-local and is
still ignored.

* fix(daemon): complete the same-shell state model for bodies and substitutions

Three related gaps, all in the shell-state sharing this PR has been building:

- A command substitution inherits the enclosing `set -a` but did not carry
  it in, so `set -a; echo $(GIT_WORK_TREE=<outside>; git reset --hard)` was
  allowed. The substitution scope now inherits allexport (by copy — its own
  changes still die with the subshell).
- A same-shell body could turn allexport on but not off: the merge-back only
  handled the truthy result, so `set -a; f() { set +a; }; f;
  GIT_WORK_TREE=<outside>; git status` denied even though bash leaves the
  later assignment unexported. Both the function and eval merges now
  propagate the boolean in both directions.
- Recorded function/alias definitions were local to each evaluator, so a
  body could not see a function the caller had already defined:
  `inner() { cd <outside>; }; outer() { inner; }; outer; git reset --hard`
  ran `inner` as an opaque command and lost the cwd. The definition tables
  are now shared by reference with same-shell bodies (`eval`, function/alias
  replay) and copied for substitution subshells.

* fix(daemon): resolve shadowing and exported functions; isolate pipe subshells

Four related function-model findings, all reproduced first:

- A recorded function shadows the git program or a builtin, and bash resolves
  it before either — `git() { cd <outside>; command git status; }; git` and
  `cd() { command cd <outside>; }; cd nested; git reset --hard` were allowed
  because `analyzeRun` classified `git`/`cd` before the body lookup. Recorded
  bodies are now resolved before program/builtin dispatch, via a shared
  `invokeDefinedBody`; `command`/`builtin` name a different program word and
  bypass it as bash does.
- A function/alias redefinition in a pipeline component runs in a subshell and
  must not persist, but sharing `definedBodies` (previous commit) let it leak:
  `f() { cd <outside>; }; f() { :; } | cat; f` was modelled as a no-op. Pipe
  and background components no longer record a definition into the parent, and
  their cwd/allexport are already rolled back.
- `export -f f` makes a function visible inside a `bash -c` subprocess, unlike
  an ordinary function. Those names are tracked and the subprocess payload is
  seeded with only the exported subset; an unexported function stays invisible
  to `bash -c`.

* fix(daemon): close the interlocking gaps in my function-model work

Four gaps in the recorded-body machinery the last commits built, all
reproduced first:

- `invokeDefinedBody` did not carry `exportedFunctions` into the replayed
  body, so a `export -f`'d function invoked from another function's body was
  invisible to its `bash -c`.
- A prefix assignment on the invocation (`GIT_WORK_TREE=<outside> gg`) was
  dropped, because the defined-body gate skips `analyzeRun`; the run's leading
  assignments are now applied to the body as ambient relocations.
- The pipe-component rollback restored cwd/allexport/definitions but leaked
  the subshell's exports, export attributes and shell-locals into the parent;
  all of them now roll back.

* fix(daemon): deny relocations disguised by a redirection

Two reachable escapes with ordinary (non-adversarial) commands:

- `cd <outside> >&2; git reset --hard` — a stderr redirect on the `cd`, whose
  `&` was read as a background separator so the tracked cwd was rewound while
  the real shell had moved outside. `>&`/`<&` file-descriptor redirects are no
  longer treated as backgrounding.
- `git 2>/dev/null -C <outside> reset --hard` — the redirect operand among the
  git args ended `readGitInvocation`'s option parsing before the `-C`, so the
  relocation was invisible. It now skips redirect/fd-flagged tokens. Ordinary
  trailing redirects (`git status 2>/dev/null`) stay allowed.

* fix(daemon): deny relocation hidden by a leading redirect or a background &

Two more reachable escapes with ordinary commands, triaged out of the R8
batch (the rest of which is Windows paths, docs wording, test coverage or
adversarial parser edges under the documented best-effort promise):

- `2>/dev/null gg` where `gg` is a recorded alias/function ran the body in
  bash, but `readProgramWord` returned the fd token instead of the program
  word, so the invocation was not resolved. It now skips redirect/fd operands.
- `true & cd <outside>; git reset --hard` — only the segment a `&` follows is
  backgrounded (a subshell); the segment after it runs in the foreground, so
  its `cd` persists. The pipe-component test now treats a segment as a
  subshell only when it precedes `&`, while both sides of a `|` still are.

* fix(daemon): don't let a harmless or removed shadow mask a relocation

Two escapes where the guard replayed a recorded body while the real
interpreter ran a relocating external git, both reproduced first:

- `export -f` functions were seeded into every subprocess shell, but only
  bash imports them. `git() { :; }; export -f git; dash -c "git -C <outside>
  reset --hard"` was allowed because the guard replayed the harmless `:` for
  dash, while real dash resolves the external git and relocates. Exported
  functions are now seeded only for a bash child.
- `definedBodies`/`gitShapedNames`/`exportedFunctions` only ever gained
  entries, so a removed shadow still replayed. `unset -f`/`unalias` now drop
  the function/alias (and `-a` clears all), and `export -n -f` clears the
  export attribute — `git() { :; }; unset -f git; git -C <outside> reset
  --hard` and the `unalias git` form now deny, while a live compatible shadow
  (bash-imported function, an alias still in effect) stays modelled.

* fix(daemon): drop exported functions when env clears the child environment

Two follow-ups to the per-interpreter shadow modelling:

- The `unalias`/`unset -f` removal branch compared `removalProgram` against
  `'unalias'` after `isFunctions` had already narrowed it to `'unset'`, which
  `tsc --build` rejects as a no-overlap comparison (TS2367). `isFunctions`
  already covers every `unalias` case, so drop the redundant term.
- `env -i` / `-` / `--ignore-environment` start the child from an empty
  environment, so a bash `-c` payload no longer inherits the parent's
  `export -f` functions. The env wrapper now records that the environment was
  cleared and the bash payload stops importing exported functions when it was,
  so `git() { :; }; export -f git; env -i bash -c "git -C <outside> reset
  --hard"` denies while `env -i bash -c "... rev-parse HEAD"` and an
  un-cleared `env FOO=bar bash -c` stay allowed. Regressions added.

* test(daemon): pin the sh-wrapper fail-closed contract and document it

`sh` is bash on macOS and dash elsewhere, so its `export -f` import behaviour
cannot be decided from the basename. The guard already treats `sh` as
non-importing — it never replays an exported shadow for `sh -c`, because doing
so on a dash-backed `sh` would recreate the relocation escape. Pin that
fail-closed contract with a regression (`export -f git; sh -c "git -C
<outside> reset --hard"` denies) so a future change that widens the bash gate
to include `sh` breaks a test, and record the deliberate over-denial in the
design doc's non-goals.

* fix(daemon): model shell-definition removal the way the real shell does

The removal-builtin handling added earlier was too broad and dropped live
relocating shadows, and the exported-function set was shared into subprocess
scopes by reference. Each escape below was reproduced against the guard first.

- `unset` has no `-a` option and `unalias -a` clears only aliases, yet both
  were treated as "clear every definition", so `pwn(){ git -C <outside> reset
  --hard; }; unset -a; pwn` (and the `unalias -a` form) wiped the function and
  ran it unrecognized. Removal is now kind-aware: `unalias` touches only
  aliases, `unset -f`/bare `unset` only functions.
- A function shadowing `unset`/`unalias`/`export` runs instead of the builtin,
  so the removal never happens; the branch now fires only when the name is not
  itself a recorded shadow, and otherwise falls through to replay the shadow.
- The bash `-c` subprocess and command-substitution scopes received the
  parent's `exportedFunctions` set by reference (or, for `$( )`, not at all),
  so a child `unset -f` retracted the parent's export and a substitution saw
  none. Both now take a copy.

Adds regressions for each and keeps the existing shadow/removal cases green.

* fix(daemon): bare unset keeps the function and env -u strips exported functions

Two more escapes doudouOUC reproduced in the removal model, both verified
against the guard first.

- A bare `unset NAME` unsets a same-name variable first and removes the
  function only when none exists. This evaluator tracks no ordinary variables,
  so it cannot tell the two apart; treating every bare `unset NAME` as a
  function removal dropped a live relocating shadow
  (`pwn(){ git -C <outside> …; }; pwn=1; unset pwn; pwn`). Only `unset -f`
  now removes a function; a bare `unset` leaves it, the safe over-deny choice.
- A bash `export -f foo` travels as a `BASH_FUNC_foo%%` environment entry, so
  `env -u BASH_FUNC_foo%%` (and the `--unset=` / attached forms) strips it
  before `bash -c` and the child runs the real program. The env wrapper now
  records unset keys in PrefixState and the payload seeding drops functions
  whose `BASH_FUNC_*` entry was removed, so a stripped harmless `git` shadow no
  longer masks the real relocation.

Adds regressions for both; keeps `unset -f`, unrelated `env -u`, and live
shadows behaving as before.

* fix(daemon): fail closed when a removal builtin could retract a tracked shadow

Modelling exactly which definition an `unset`/`unalias`/`export -n` removes is
general shell semantics this guard does not attempt: a bare `unset NAME` drops
a same-name variable before the function, `enable -n unset` turns the builtin
into a no-op, a `command`/`builtin` prefix or a `( … )` subshell changes what
runs, and fused flag clusters (`-nf`) hide the mode. Every attempt to model
these precisely left a live relocating shadow reachable through a form it did
not cover.

Collapse the whole removal path to one rule: when a removal references a name
tracked as a shadow (a defined body, a git-shaped name, or an exported
function) — or clears all while any shadow exists — fail closed. This denies
the previously-allowed `git(){ :; }; unset git; git -C <outside> …`,
`export -nf`, `command unset -f`, `enable -n unset; unset -f`, and
`( unset -f git ); git` forms, while a removal of an untracked name and every
live-shadow replay behave exactly as before. Removes the earlier kind-aware
bookkeeping the same escapes kept slipping through.

* fix(daemon): skip leading redirections before the removal-builtin prefix scan

The `command`/`builtin` strip in the shadow-removal guard started at raw token
zero, but bash strips redirections from argv. A leading `2>/dev/null` before
`command unset -f <tracked-function>` left the scan looking at the redirect
operand, so `command` was never consumed, `readProgramWord` returned `command`
rather than `unset`, the removal went unrecorded, and the stale harmless
function masked the later external Git relocation.

Skip redirect/fd operands before and between the `command`/`builtin` prefixes,
the same normalization `readProgramWord` applies. Adds the leading-redirection
variant to the command-prefix regression.

* fix(daemon): replay a shadowed removal builtin and drop unset variables

Two more escapes doudouOUC reproduced in the fail-closed removal rule.

- The rule's early `continue` fired even when `unset`/`unalias`/`export` was
  itself a recorded function and the operands named only untracked state, so a
  shadowing `unset(){ git -C <outside> …; }; unset other` was classified as a
  harmless builtin removal and never reached the shadow dispatch that replays
  the relocating body. The branch now runs only when the program is not a
  shadowed function (a `command`/`builtin` prefix still forces the builtin).
- The removal never dropped tracked variables, so `A=nested; unset A; cd $A`
  kept expanding the stale in-bounds value while bash's `unset A` leaves `$A`
  empty and `cd $A` lands at $HOME. `unset NAME`/`unset -v NAME` now deletes
  the shell-local, turning the later `$A` into an unresolved reference the cd
  fails closed on. `unset -f` is functions-only and leaves variables intact.

Adds regressions for both.

* fix(daemon): honor shadowed command/builtin prefixes and PATH-based relocation

Two escapes surfaced by the round-11 review, both reproduced against the guard.

- The shadow-removal prefix scan trusted a literal `command`/`builtin` word to
  force the real builtin, but bash resolves a function of that name first. A
  `command(){ git -C <outside> …; }; command unset other` therefore
  early-continued as a harmless builtin removal and never replayed the
  relocating body. The prefix loop now stops when the prefix word is itself a
  recorded shadow, leaving it for the normal shadow dispatch.
- The unrecognized-program marker scans covered GIT_DIR/GIT_WORK_TREE-family
  assignments but not GIT_PROGRAM_ENV_KEYS (`PATH`/`GIT_EXEC_PATH`), which
  decide which git binary runs. The direct `PATH=/evil git …` was denied while
  `find … -exec sh -c 'PATH=/evil git …'` slipped through. Both marker scans
  now include those keys, and they remain gated on a co-present git word so an
  ordinary `PATH=… make` is unaffected.

Adds regressions for the shadowed prefixes and the wrapped PATH/GIT_EXEC_PATH
forms.

* fix(daemon): catch delimiter-glued relocations and redirect-decoy prefix drops

Two escapes surfaced by the round-12 review, both reproduced against the guard.

- The env-assignment arm of both text marker patterns required `(^|\s)` before
  the key, while the sibling `cd`/`pushd` arm already allowed `;&|(){}`
  boundaries. A relocation glued to a delimiter inside a quoted wrapper payload
  (`su -c 'true;GIT_DIR=<outside> git reset --hard'`) therefore evaded the
  unrecognized-program backstop. Both arms now share the same boundary class.
- `invokeDefinedBody` located the invoked name with a raw `findIndex` that
  also matched redirect operands, so a decoy `> g` whose target equals the
  function name truncated the prefix-assignment scan to empty and dropped the
  call's `GIT_DIR=` relocation. The lookup now skips redirect/fd operands like
  `readProgramWord` does.

Adds regressions for the delimiter-glued and redirect-decoy forms.

* fix(daemon): deny trailer/man/sendemail command-executing config keys

The dangerous-config model already denies `git -c <key>=<command>` for the
command-executing config families, but omitted three documented ones:
`trailer.<token>.command`, `man.<tool>.cmd`, and
`sendemail.(sendmailcmd|tocmd|cccmd)`. `git -c trailer.sign.command='…'
interpret-trailers` (and the man/sendemail forms) ran the configured shell
command while the guard allowed it. Adds the three patterns and regressions.

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
2026-08-14 09:55:59 +00:00
qqqys
8517fa9d47
feat(web-shell): redesign Channel policy and workspace management (#8848)
* feat(web-shell): expose channel access policies

* test(cli): cover shared Channel management fields

* feat(web-shell): clarify channel policy controls

* feat(web-shell): select channel workspace

* feat(web-shell): redesign channel management

* fix(web-shell): align channel manager with shell tabs

* fix(web-shell): prioritize conversation settings

* fix(web-shell): preserve legacy channel defaults

* fix(channels): address management review blockers

* fix(channels): address editor review blockers

* fix(channels): preserve workspace action and route state

* fix(web-shell): prevent stale channel editor state

* fix(channels): preserve stored group settings

* fix(channels): preserve group behavior settings

* fix(web-shell): reset channel workspace UI state

* test(web-shell): assert restored channel scope

* fix(web-shell): preserve legacy channel scope

* fix(web-shell): preserve inherited channel defaults

* fix(channels): preserve compatible legacy settings

* fix(web-shell): keep workspace navigation available

* fix(channels): default new channels to pairing

---------

Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com>
2026-08-14 07:41:44 +00:00
jinye
53a7f2fd1b
feat(daemon): track background shells in activeWork (#9042)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-08-14 03:25:50 +00:00
jinye
d670bb8109
feat(telemetry): Trace main agent invocations (#9107)
Some checks failed
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 1/2 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 2/2 (push) Waiting to run
E2E Tests / channel-plugin E2E (nightly) (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
SDK Java / ubuntu-latest / Java 11 (push) Waiting to run
SDK Java / ubuntu-latest / Java 17 (push) Waiting to run
SDK Java / macos-latest / Java 21 (push) Waiting to run
SDK Java / ubuntu-latest / Java 21 (push) Waiting to run
SDK Java / windows-latest / Java 21 (push) Waiting to run
SDK Java / Real daemon E2E / Java 11 (push) Waiting to run
Security Checks / Dependency CVE audit (push) Waiting to run
Security Checks / Secret scan (TruffleHog) (push) Waiting to run
SDK Python / Classify PR (push) Has been cancelled
SDK Python / SDK Python (3.10) (push) Has been cancelled
SDK Python / SDK Python (3.11) (push) Has been cancelled
SDK Python / SDK Python (3.12) (push) Has been cancelled
* feat(telemetry): Trace main agent invocations

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

* codex: address PR review feedback (#9107)

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

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-08-14 02:50:22 +00:00
jinye
3378212b5f
feat(cli): correlate daemon logs with OpenTelemetry spans (#9084)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-08-13 16:33:12 +00:00
Shaojin Wen
407cf0a7f8
feat(serve): adaptively grow live-journal caps before truncating mid-turn replay (#8905)
* feat(serve): adaptively grow live-journal caps before truncating mid-turn replay

A single turn fanning out many concurrent subagents (e.g. a /review run) can emit hundreds of thousands of source events, far past the per-session live-journal baseline caps (10 000 entries / 8 MiB), so a mid-turn (re)load silently shows a truncated replay until the turn finishes. Before evicting, the engine now asks a growth advisor: caps double (entries scaled proportionally) while the growth granted across the bridge's live sessions fits in a pool derived from the daemon memory budget (5%, clamped to [32, 1024] MB), never past a per-session hard cap of 256 MiB. Growth is on demand, throttled after a refusal, and accounted statelessly from the current caps of all live sessions, so granted headroom dies with its session. An operator-pinned --max-journal-events/--max-journal-bytes disables growth; without a pool the fixed-cap eviction behavior is unchanged.

* fix(serve): address adaptive live-journal growth review feedback (#8905)

* fix(serve): account in-flight restores in the journal growth pool (#8905)

Concurrent restores hold their buses in pendingRestoreEvents rather than
byId, so each advisor ask only saw its own caps and concurrent restores
could each draw a full doubling from the same pool. Sum the current caps
of every in-flight restore bus into allSessionLimitBytes.

Also skip the growth ask when the breaching append is a turn boundary —
compactCurrentTurn discards the journal immediately afterwards, so the
grant would be charged to the pool while buying zero eviction.

Pin the previously untested contracts with tests: restore-window
accounting, concurrent-restore accounting, headroom release on session
close, the hard-cap clamp term, partial-grant eviction, requester
discrimination in the policy fixtures, the maxEvents safe-integer
conjunct, and the dynamic-workspace bridge pool wiring. Fix the docs:
add the missing journal-flag rows to the daemon configuration and
operations pages, and correct the effective-budget definition.

* test(serve): request 'response' replay in the transport-failure test (#8905)

The merge of main pulled in #8933, which gates historyPageSize on
historyReplay === 'response'. The 'transport failure marks the channel
dying before process exit' test (from #8947) passes historyPageSize with
the default stream replay, so the paged transcript fetch it waits on is
never issued and the test times out — a cross-PR interaction between two
main commits, failing deterministically on main. Pin the response replay
mode the paged fetch requires.

* fix(serve): share one daemon-wide journal growth pool (#8905)

Address the automated review of adaptive live-journal growth:

- The growth pool is now one daemon-wide aggregate shared by every
  workspace bridge instead of a full pool per bridge, and growth is
  disabled when the budget is insufficient or leaves no headroom after
  the root reserve.
- Grants that cannot retain any additional journal entries (an oversized
  event survives as the sole entry either way) are refused so the pool
  is never charged for growth that preserves no replay.
- The refusal throttle defaults to a monotonic clock and treats a
  backward clock jump as an elapsed window.
- The proportional event hard cap is clamped to MAX_SAFE_INTEGER so a
  valid-but-extreme baseline cannot poison every grant.
- /daemon/status reports the growth semantics: limits.memory.journalGrowth
  (pool size, hard cap, baselines), per-session effective caps in full
  diagnostics, and enforced:false scoped to the child-heap model.
- Validation-boundary tests for the growth-pool normalizer and doc fixes
  (positive safe integer types; growth toward double, limited by pool
  headroom).

* fix(serve): align growth-pool docs and harden growth tests (#8905)

* fix(serve): account growth per session baseline and walk intermediate grants (#8905)

* fix(serve): harden growth-pool tests and derive help figures from constants (#8905)

* fix(serve): reject valueless journal cap flags and harden growth tests (#8905)

---------

Co-authored-by: qwen-code-ci-bot <qwen-code-ci@service.alibaba.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
2026-08-13 11:35:39 +00:00
ytahdn
a8bcaefea7
feat(web-shell): support workspace file uploads (#8874)
* feat: add web shell workspace file uploads

* fix(serve): accept plain string targets in shared atomic publisher (#8874)

* fix(review): bound web shell related paths

* fix(review): address web shell file upload review findings (#8874)

* test(serve): include upload capability in baseline

* fix(review): pin workspace_file_upload in the serve capabilities integration baseline (#8874)

* fix(review): address round-2 web shell file upload review findings (#8874)

* fix(review): address round-3 web shell file upload review findings (#8874)

* fix(review): address round-4 web shell file upload review findings (#8874)

* fix(review): address round-5 web shell file upload review findings (#8874)

* fix(review): address remaining file upload findings (#8874)

* fix(review): address round-6 web shell file upload review findings (#8874)

---------

Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
2026-08-13 06:42:07 +00:00
Nothing Chan
de470b95b4
feat(telemetry): align session lifecycle with OpenTelemetry (#8616)
* 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>
2026-08-12 02:53:06 +00:00
qqqys
de48637aa0
refactor(serve): default project memory to workspace scope (#8856)
* refactor(serve): default project memory to workspace scope

* fix(serve): preserve launch env access guard

* fix(serve): harden project memory scope resolution

* fix(serve): harden project memory scope diagnostics

* fix(serve): keep memory scope operator-owned

---------

Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com>
2026-08-12 02:38:09 +00:00
jinye
542ef73fd3
chore(serve): Log session continuation admissions (#8932)
* chore(serve): Log session continuation admissions

Refs #8923

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

* test(serve): Tighten continuation log assertions

Refs #8923

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

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-08-11 14:34:08 +00:00
易良
89708569f7
fix: add structured error code to SessionNotFoundError for session-closing retry (#8884)
* 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
2026-08-11 13:01:25 +00:00
ytahdn
1a2c5026b2
fix(web-shell): reconcile mid-turn messages with daemon state (#8798)
* fix(web-shell): reconcile mid-turn messages with daemon state

* test(serve): update mid-turn capability expectation

* test(mid-turn): cover reconciliation mutants and restore serve protocol docs (#8798)

* fix(serve): close mid-turn promotion admission and delivery gaps (#8798)

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

* fix(serve): keep anonymous mid-turn enqueues off the shared queue surface (#8798)

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

* fix(sdk): account for mid-turn APIs in bundle budget

* test(acp-bridge): use vi.waitFor for async prompt-drain assertions (#8798)

* fix(serve): reconcile mid-turn steering safely

* fix: reconcile mid-turn messages safely

---------

Co-authored-by: ytahdn <ytahdn@users.noreply.github.com>
Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.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: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
2026-08-11 07:24:56 +00:00
Shaojin Wen
95e17691a9
chore(serve): remove the /demo debug page (#8805)
* 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>
2026-08-10 13:31:15 +00:00
jinye
fa8cae5418
fix(serve): Allow approved external built-in text writes (#8852)
* fix(serve): allow approved external built-in text writes

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

* fix(serve): keep write provenance off startup bundle

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

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-08-10 12:19:58 +00:00
Shaojin Wen
77bd04bd61
fix(acp-bridge): bound live journal replay chunks (#8801)
* 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>
2026-08-10 09:52:17 +00:00
Shaojin Wen
b314d01f2d
fix(web-shell): stop rendering unrecognized daemon events in transcripts (#8812)
Some checks are pending
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 1/2 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 2/2 (push) Waiting to run
E2E Tests / channel-plugin E2E (nightly) (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
* 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>
2026-08-10 00:51:57 +00:00
jinye
a810f7e16c
fix(serve): Make session restore timeouts safe and observable (#8691)
* 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>
2026-08-09 15:47:09 +00:00
jinye
60458f5e37
fix(serve): Coordinate caller-supplied session IDs (#8415)
* fix(serve): coordinate caller-supplied session IDs

Complete daemon-wide admission across REST, ACP, workspace generations, SDKs, and MCP.

Closes #8411

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

* test(serve): wire session bridges in hot-reload harness

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

* fix(serve): address review round for caller-supplied session IDs (#8415)

Restore the observability and fail-loud guarantees flagged in review:
log every session-id admission routing failure, name the live foreign
owner workspace in restore conflicts, make the ACP dispatcher's
admission dependency required so load/resume cannot run on a mount
without one, and require mountAcpHttp hosts to inject the daemon-wide
admission instead of silently building a weak fallback. Harden the SDK
WS transport against environments without global fetch and against
non-capabilities 200 envelopes, and align the design doc with the
implemented restore-sharing and persistence-failure semantics.

* fix(sdk): harden session ID capability fallback

Preserve REST capability errors, fail closed on malformed envelopes, retain restore routing diagnostics, and align retry documentation.

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

* fix(serve): normalize restored session IDs

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

* fix(session): preserve mixed-case legacy session access

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

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
2026-08-09 07:31:30 +00:00
callmeYe
39377fcff3
feat(daemon): add batch skill toggle API (#8664)
* feat(daemon): add batch skill toggle API

* test(serve): update capability integration baseline

* fix(daemon): apply skill batches atomically

* test(daemon): pin Skill batch toggle contracts and fix docs examples

* test(daemon): pin Skill batch toggle mutants flagged in review

* test(daemon): cover Skill batch toggle edge cases

* docs(daemon): clarify Skill batch toggle contract notes from review

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

* test(daemon): pin Skill batch toggle cap semantics and SDK surface shape

* test(daemon): pin Skill batch toggle mutants flagged in round-5 review

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
2026-08-08 23:22:04 +00:00
jinye
59b750fc4d
feat(serve): Expose active work state (#8588)
Some checks are pending
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 1/2 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 2/2 (push) Waiting to run
E2E Tests / channel-plugin E2E (nightly) (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
SDK Java / ubuntu-latest / Java 11 (push) Waiting to run
SDK Java / ubuntu-latest / Java 17 (push) Waiting to run
SDK Java / macos-latest / Java 21 (push) Waiting to run
SDK Java / ubuntu-latest / Java 21 (push) Waiting to run
SDK Java / windows-latest / Java 21 (push) Waiting to run
SDK Java / Real daemon E2E / Java 11 (push) Waiting to run
* feat(serve): expose active work state

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

* refactor(serve): rebuild active-work reporting on channel-wide snapshots

Reworks the active-work signal after review. Three changes of substance.

Drops the 45s heartbeat watchdog entirely. It inferred "this channel is
dead" from "one Session stopped reporting" and killed the whole channel,
taking every Session on that process with it — including on a suspend,
a long event-loop stall, or a single dropped notification. Channel
liveness is a transport concern and gets its own mechanism.

Replaces the per-Session boolean with a channel-wide snapshot of named
holds, derived on every report from the owners of the work (the
registry's unfinalized set, the notification queue) rather than from a
ledger kept alongside them. Full snapshots make a dropped report
self-correcting in both directions, and a Session's absence from one is
positive evidence the child released it. Agent holds now use
hasUnfinalizedTasks()'s predicate, closing the cancel to
finalizeCancelled() window where a cancelled agent looked idle and its
terminal notification could be stranded.

Leaves prompts out of the child's report: the daemon accepts, queues,
dispatches, and settles them, so its own count is authoritative and
covers the FIFO wait the child cannot see. A snapshot is flushed ahead
of the prompt response so a hold the prompt left behind is on the wire
before the daemon drops that count.

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

* feat(serve): confirm idle before closing, and grade the health signal

Completes the active-work rework with the two facts a restart controller
was still missing and the one guarantee automatic cleanup was missing.

Automatic cleanup no longer destroys a Session on the strength of a
cached snapshot. It asks the child to close only if unheld, and the
child answers under its own close gate — with the gate held no prompt is
admitted and no automatic turn starts, so a hold cannot appear between
the check and the teardown. A refusal hands back the current holds and
the daemon adopts them. An unanswered request is neither retried nor
assumed: the Session stays, and the next snapshot settles it, because a
Session absent from one has provably been released. Every automatic path
— detach, attach rollback, prompt settle, notification settle, a child
reporting itself idle — now funnels through one decision point instead
of four near-copies.

Health gains activeWorkReporting and activeWorkStaleMs. Without them
activeWork:false cannot be told apart from "no child told me anything",
which is the one case where acting on it is unsafe. Freshness is graded
by the daemon rather than the controller, since the cadence is negotiated
per channel; a stale snapshot or a child omitting a category degrades the
grade instead of silently narrowing what the boolean covers.

Tests: acp-bridge 489/489, acpAgent 383/383, Session 534/534,
serve suites 1188 with one pre-existing cross-file flake in the Live
Appshot integration tests (reproduces on the unmodified tree, failing a
different test each run).

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

* fix(serve): contain snapshot-collection failures, and repair two Session mocks

CI caught two things the local runs missed.

The reporter's snapshot construction was unguarded. Only the send was
wrapped, so a throw while collecting a Session's holds escaped through
setInterval and queueMicrotask as an uncaught exception — capable of
taking down the ACP child — and through flush() into the prompt path,
turning a reporting problem into a failed prompt. Collection is now
wrapped and a failed snapshot is abandoned whole rather than sent
partially: a Session missing from a report reads as released, and one
reported with no holds reads as safe to close, so publishing a partial
snapshot would actively invite the daemon to destroy live work. Sending
nothing lets the daemon's copy age instead, which its freshness grading
already treats as untrustworthy and retains. flush() no longer rejects.

Session.review-lease and Session.worktree mock the background-task
registry without setStatusChangeCallback, so constructing a Session threw.
That break arrived with the original commit, which verified only
Session.test.ts; the sibling Session.*.test.ts files were never run. Both
mocks now carry the methods the constructor and the hold collector need.

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

* test(serve): prove the reporter contains collection and transport failures

The previous commit added the guard but could not have demonstrated it:
the same commit also gave the acpAgent Session mock a
collectActiveWorkHolds, removing the very condition that triggered the
throw. The unhandled error disappearing was therefore explained by the
mock alone, and active-work-reporter.ts had no tests at all.

These cover the escape routes that matter — the interval timer, the
coalescing microtask, and flush() on the prompt path — plus the choice to
abandon a whole snapshot rather than send a partial one, since a session
omitted from a report reads as released and one reported with no holds
reads as safe to close.

Verified by removing the guard: five of the nine fail with the collection
error escaping, and pass again once it is restored.

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

* fix(serve): make every automatic teardown ask before destroying

Self-review of the previous revision found that this PR had promoted a
cached child report from a hint into the authority that permits destroying
a Session. Four teardown paths consulted it, their guards disagreed with
each other, and each was weaker than what main had. The four are one
defect with four exits, so they are fixed as one change.

Absence from a snapshot no longer authorizes teardown. Because reports
are complete, a Session the child omits holds nothing on the child side —
so absence and reported-with-no-holds are the same fact and now take the
same path. The separate absence loop is gone; it lacked the subscriber
and client guards `maybeCloseIdleSession` applies, so one snapshot could
destroy a Session with a live SSE subscriber and a registered client.
That contradicted this PR's own claim that an unreported Session is
retained, and the old test asserted the destruction. Both are corrected.

A conditional close is now marked in flight across the whole confirm-then-
teardown span, and attach, prompt, and rewind refuse a Session in that
state exactly as they refuse one already closing. `closeSessionImpl` sets
`closing` synchronously, but the round trip in front of it is an await of
up to ten seconds; on main the guard sequence ran straight into teardown,
so splitting it is what opened the window.

A snapshot older than the freshness window stops counting as evidence.
Staleness was already computed, but only to grade health, never to gate
destruction — so a child that went quiet after one empty report left a
cache that permitted reaping indefinitely. Never-reported and gone-quiet
now land in the same retained bucket. Reclaiming a channel that has truly
stopped answering belongs to transport liveness, not here.

The idle reaper asks the child too. Its TTL says the client stopped
caring, which is not the same as the child having nothing left to run.

Health coverage is exposed as counts and graded once daemon-wide, because
grades do not compose: a runtime with zero Sessions is vacuously `full`,
and folding that in let an empty workspace vouch for another workspace's
unreported Sessions. `activeWorkStaleMs` now measures only covered
Sessions, so it can no longer report positive staleness beside a grade
saying nothing is covered.

Also: bound snapshot `sessions[]` and `holds[]` so a buggy child cannot
make the daemon walk an unbounded structure per report, and retract the
background-task status callback by identity rather than blanking a
single-slot setter the TUI also uses.

Tests: the absence test now asserts retention under a registered client
and under a live subscriber; new regressions cover the recovered lost
close response, the stale-snapshot gate, admission refusal during a
conditional close, the reaper's confirmation, the oversized-snapshot
discard, and the mixed empty/uncovered health aggregate.

* fix(serve): make unknown a reason to ask, not a reason to skip

Triage review found that the design doc, the PR description, and the
comment on `entryHasActiveWork` all promised the daemon *asks* the child
about a Session it has not heard about, while no code path ever did:
`entryHasActiveWork` returns true when the child's side is unknown, and
the cleanup path returned early on exactly that. The finding predates the
guard rework and survived it unchanged.

Skipping on unknown looks like the safe direction and is in fact the worse
failure. Nothing resolves it — a Session on a channel that went quiet is
retained forever, and the idle reaper skips it too, so there is no path
out at all. Asking resolves it definitively: the child answers under its
own close gate whether or not its snapshots are arriving, the round trip
is bounded, and every non-answer still retains.

So the predicate is split by what it actually knows. `childReportsHeldWork`
is positive knowledge only; `childWorkIsUnknown` is the absence of a
gradeable report. The health surface ORs both, because a controller must
never read "nobody told me" as "nothing is running". Automatic cleanup
blocks only on known work and lets unknown through to `confirmChildUnheld`.

Also moves `parseActiveWorkSnapshot` out from between two import blocks
(pure relocation, no logic change) and aligns the doc wording, including
the shared-guard table, with what the code now does.

* fix(serve): close three teardown races the confirm window opened

Review found three ways the conditional close can still destroy a live
Session. All three share a cause: the round trip turned a synchronous
guard-then-teardown into an awaited span, and three things that were
previously impossible to observe mid-teardown now are.

**A restore in flight looks exactly like an abandoned Session.**
`session/load` registers the entry before awaiting `artifacts.restore()`
and `seedSessionUpdates()`, and registers its first client only after —
so for that whole window there are no clients, no subscribers, nothing
held, and the child answers the conditional close truthfully. The
snapshot trigger this PR added fires inside it. Excluded in
`entryIsAutoCloseCandidate` rather than at the snapshot trigger, so the
reaper's TTL elapsing inside a slow restore is covered too.
`pendingRestoreIds` already existed but was read only by
`hasNoChannelWork`, never by the close funnel.

**Teardown re-resolved the target by id without re-checking identity.**
`closeSessionImpl` does a fresh `byId.get`, and the id can be
re-registered to a different entry during the round trip: an explicit
kill removes this one (kill ignores the in-flight flag by design, keeping
its force semantics) and a `session/load` for the same persisted id
registers a fresh one. The stale continuation then tore down the newly
restored Session under its just-attached client. One identity re-check
after the await.

**The restore path was not upgraded to the new admission predicate.**
`sendPrompt`, `rewindSession`, and single-scope attach check
`isClosingOrAuthorizingClose`; `restoreSession` still checked bare
`closing` at both its guards, so a client could attach inside the window
and lose the session under it. That directly contradicted the
`closeIfChildUnheld` comment claiming every admission path checks the
flag. Its `racedEntry` branch had no closing guard at all — a narrower
pre-existing hole, same defect, same predicate.

Regression test covers the restore-path admission refusal. The other two
need a mid-restore snapshot and a kill-then-reload interleave that the
mocked-channel harness cannot stage honestly; both are pinned by reading
the code paths, which is weaker and worth saying.

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-08 06:34:20 +00:00
Dragon
681e30d54f
docs: clarify SDK interrupt behavior (#8711) 2026-08-08 01:48:16 +00:00
BaboBen
edb420393e
fix(channels): manage DingTalk interactive card config (#8517)
Some checks are pending
E2E Tests / E2E Test (Linux) - sandbox:none - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 1/2 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 2/2 (push) Waiting to run
E2E Tests / channel-plugin E2E (nightly) (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
SDK Java / ubuntu-latest / Java 17 (push) Waiting to run
SDK Java / macos-latest / Java 21 (push) Waiting to run
SDK Java / ubuntu-latest / Java 21 (push) Waiting to run
SDK Java / windows-latest / Java 21 (push) Waiting to run
SDK Java / Real daemon E2E / Java 11 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 3/3 (push) Waiting to run
SDK Java / ubuntu-latest / Java 11 (push) Waiting to run
* fix(channels): manage DingTalk interactive card config

* test(cli): cover nested channel object validation

* fix(channels): harden nested management metadata

* fix(channels): isolate invalid management descriptors

* fix(channels): isolate invalid management metadata from channel runtime

* fix(channels): reject reserved unknown keys in management config upserts

* fix(channels): harden channel management validation and editor checks

Reject management descriptors that lack a fields array at registration so
broken plugins are stripped to unmanageable instead of being advertised as
manageable and failing every upsert with an unmapped TypeError. Reserve the
top-level "type" field key and require enum fields to declare at least one
option, both of which the settings store could never accept. Treat
whitespace-only number drafts as empty in the channel editor, consistent
with the module's other emptiness checks.

Also give the SDK descriptor mirror test a runtime wire-shape walk over the
built-in catalog, add the parser's timeout rejection boundary, and restore
the exact built-in catalog membership assertion.

* fix(channels): validate management field shapes and editor bounds (#8517)

* fix(channels): align management validation layers and pin gate behavior (#8517)

Read envResolvable by truthiness in the settings store so it matches the
registration gate and the editor, instead of rejecting the advertised
environment references of untyped plugins. Fail closed at registration on
non-finite exclusiveMinimum values, empty object property lists, and async
validateConfig functions, all of which would otherwise advertise a field
or save path that can never succeed. Strip invalid management metadata
over a prototype-preserving copy so class-instance plugins keep their
createChannel implementation.

Move the unchanged-value preservation exemption ahead of the object shape
rejection so a stored non-record value (for example a hand-written null)
no longer locks every unrelated management edit of that channel. Clamp
DingTalk question-card timeouts at the maximum setTimeout delay, since
Node treats larger delays as one millisecond and would expire cards
instantly.

Pin the previously untested load-bearing behaviors: per-key previous
threading in the recursive validation, the preservation exemption's
precedence over nested required enforcement, nested "type" properties,
depth-2 nesting rules, and the nested-only constraints of the daemon
descriptor wire contract.

* fix(channels): close reserved-key preservation gaps and pin gate behavior (#8517)

* test(cli): tolerate IPv6-less hosts in serve ::1 bind tests (#8517)

The self-hosted CI containers can have no IPv6 loopback, where the two
runQwenServe tests that bind ::1 fail with EADDRNOTAVAIL. Probe the
interfaces once and skip only the IPv6-dependent binds there; every
assertion still runs on IPv6-capable hosts.

* fix(channels): align descriptor type contracts with runtime validation (#8517)

The registry already rejects object fields without a non-empty
properties array and enums without unique options, but the descriptor
types still admitted both, so TS-authored plugins only learned about
it when registration stripped their management surface. Make
`properties` required, give enums a dedicated descriptor member with
required `options`, and drop the never-honored `envResolvable` flag
from number descriptors, in both channel-base and the SDK mirror, and
export the descriptor sub-types through the webui barrels. Also map a
throwing `validateConfig` to the usual invalid-config error and pin
the store contracts that had no distinguishing tests: omitting a
parent object drops the stored object without checking its nested
required, writes replace nested values wholesale, unchanged stored
scalars are still re-validated, and valid plugins register by
original reference.

* fix(channels): defuse validateConfig rejection leak and close descriptor gate gaps (#8517)

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

---------

Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-08-07 15:47:20 +00:00
qqqys
3edecac116
feat(channels): support group pairing (#8440)
* feat(channels): support group pairing

* fix(channels): address group pairing review

* fix(web-shell): show group pairing management

* fix(channels): recheck group pairing before history backfill

* test(channels): verify group approval isolation

* fix(channels): address group pairing review findings and pin behaviors (#8440)

- Grandfather the group allowlist file in PairingStore legacy migration
- Offer the pairing groupPolicy option in github/gitlab descriptors
- Re-export DaemonChannelPairingSubject from the webui barrels
- Refresh the GroupGate doc comment and channel docs rows
- Pin the unpinned group pairing behaviors called out in review:
  subject dedup, trigger matrix, notification content/cap/failure/
  thread routing, DM negative space under groupPolicy pairing,
  stored DM loop authz, pairing-enabled guard negative space,
  approval/revocation HTTP bodies, descriptor-driven gate branch,
  and the web-shell group approval mirrors
- Add a compile-time assertion for the revocation request union

* fix(channels): address group pairing review findings (#8440)

- Accept 'pairing' in the GitLab connect warning, descriptor help text,
  and gitlab.md: todos dispatch after one-time group approval.
- Model group approvals in the web-shell e2e mock daemon (approve by
  subject type, GET returns senderIds+groupIds, DELETE accepts groupId)
  and exercise the group pairing flow in the channels spec.
- Add 'pairing' to the groupPolicy enumerations in the plugins and
  per-channel docs (telegram, feishu, dingtalk, qqbot, wecom).
- Update the channel pairing CLI help to cover group requests.
- Cap pending pairing requests at one per sender so a single member
  cannot occupy every shared pending slot.

* fix(channels): address group pairing review findings round 7 (#8440)

* fix(channels): address group pairing review findings round 8 (#8440)

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

* fix(channels): address group pairing review findings round 9 (#8440)

---------

Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.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: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-08-07 09:20:18 +00:00
jinye
bf3abdee81
fix(serve): Allow approved same-host text reads outside workspace (#8620)
Some checks failed
npm cache producer / Save npm cache (push) Has been cancelled
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 1/2 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 2/2 (push) Waiting to run
E2E Tests / channel-plugin E2E (nightly) (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
SDK Java / ubuntu-latest / Java 11 (push) Waiting to run
SDK Java / ubuntu-latest / Java 17 (push) Waiting to run
SDK Java / macos-latest / Java 21 (push) Waiting to run
SDK Java / ubuntu-latest / Java 21 (push) Waiting to run
SDK Java / windows-latest / Java 21 (push) Waiting to run
SDK Java / Real daemon E2E / Java 11 (push) Waiting to run
* fix(serve): allow same-host daemon text reads

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

* docs(serve): address review on same-host text reads

Record what the read capability does not fix: #8618 still reproduces for
the write and edit family, whose delegated writes are refused after the
user has already approved the diff. Give the daemon's pre-approval SSE
fan-out its own bullet in the user-facing security section, restore the
sentence stating that environment isolation is not an OS security
boundary, and make the design doc the single owner of the tradeoff list
so tuning a limit cannot leave stale copies behind.

Test fixtures no longer land in the developer's real home directory, the
assertion pinned to localized rejection copy is dropped, and the combined
capability case is split so deleting the write half cannot silently
remove read coverage.

* fix(test): declare REPO_ROOT and bind the external-read session to the daemon's workspace

The external-read regression test referenced REPO_ROOT twice without
declaring it, which made it unrunnable everywhere:

- On a developer box the ReferenceError was swallowed by the bare catch
  in findExternalReadBase(), every candidate was discarded, and the test
  reported a green skip -- exactly the silently-disabled security test
  the CI loud-fail added last round was meant to prevent. The guard was
  defeated three lines above itself.
- On CI that loud-fail branch threw at module scope, so the file failed
  to collect and took the four pre-existing tests down with it.

Declare REPO_ROOT the way every other daemon integration test does.

The session also asked for `workspaceCwd: REPO_ROOT` while beforeAll
binds the daemon with `--workspace workspaceDir`, so the create returned
400 Workspace mismatch even once the constant existed. The read under
test is external because externalReadDir sits outside the bound
workspace, not because the session claims a wider one.

Finally, collect each candidate's rejection reason instead of dropping
it, and fold it into both branches: the CI throw names why every
candidate failed and the developer-box skip warns with the same text.
A bare catch cannot tell "no /var/tmp on this image" from a bug in the
function, and the second reads as a green skip.

Reported by @wenshao, who reproduced all three consequences against a
real qwen serve daemon on Linux and supplied the repair.

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

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-07 07:33:16 +00:00
jinye
2eb5cd6df5
feat(serve): observe daemon and child memory against real denominators (#8423)
* feat(serve): observe daemon memory pressure against a real denominator

The daemon samples its own RSS and heap but has nothing to divide them
by, so nothing in `/daemon/status` says whether a figure is fine or
nearly fatal. #8245 landed the denominator (`limits.memory`); this turns
it into a reading.

`runtime.memory.pressure` reports `level`, `ratio`, `source`, and the six
raw figures behind them. The level is the worse of two independent
ratios, because the two failure modes are independent: a container dies
by RSS against its cgroup limit, while a process on a large host can
exhaust V8's heap long before RSS is a meaningful fraction of the
machine. Reporting only one hides whichever failure the deployment is
actually heading for. `source` names which ratio produced the level, and
`unknown` says the daemon could not measure itself — which a consumer
must not read as healthy.

The denominator is `availableMemoryMb`, not `effectiveBudgetMb`: pressure
asks how close this process is to being killed, and what kills it is the
cgroup limit or host memory. An operator's budget is a policy number, so
classifying against it would report `critical` for a daemon in no danger.

`--memory-pressure-mode` is `off | observe`, default `observe`. Both
modes report every figure; only `observe` also raises the
`daemon_memory_pressure` warning, so `off` leaves the top-level `status`
rollup untouched — the thresholds are inherited from an interactive-CLI
monitor and are not yet calibrated for a long-running daemon, and a
deployment that alerts on `status` needs the reading without the verdict.
There is deliberately no `enforce`: nothing here remediates, and a value
a caller can pass but never use is a dead switch. It arrives with the
enforcement.

Scope is the daemon root process only. `childRssCoverage` still reads
`primary_only` and says so on the wire; aggregate child RSS and channel
workers are separate measurements and land separately.

Severity is `warning` at every level including `critical`, because
`error` would make `rollupStatus` return `error` for the whole daemon —
too strong a claim to stake on uncalibrated thresholds.

Refs #8051.

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

* feat(serve): report aggregate ACP child RSS, not just the primary's (#8462)

* test(serve): close the under-determined assertions review probed

The automated review mutation-probed this diff and found several
assertions that were live but under-determined — each mutant it names
kept the whole suite green. All confirmed locally, and all now fail:

- Deleting `level !== 'normal'` from the issue gate raised
  daemon_memory_pressure on a healthy daemon and flipped top-level
  status to warning on every response — the exact false positive
  `--memory-pressure-mode off` exists to opt out of. Now covered on both
  sides: nothing raised at a realistic denominator, exactly one warning
  at a denominator sized to land this process in `soft`.
- Summing children over `list()` instead of `listManaged()` dropped a
  draining-but-process-holding workspace while `activeAcpChildren` still
  counted it. The draining bridge now reports RSS, so the byte count can
  only come from that child.
- The message's denominator ternary had no coverage; inverting it sent
  an operator hunting RSS growth during a heap-driven incident.
- A truthiness guard on `ageMs` turned a measured-fresh reading (age
  exactly 0, when a status read lands in the sampler's millisecond) into
  `null`, which the field's own docs say never means fresh.
- The multi-contributor age test listed ages ascending, so a
  plain-overwrite accumulator produced the same answer as Math.max.
  Reordered descending, which kills last-wins and first-wins both.

Two declaration-only hunks — the issue-code union member and the
`pressure` field — were guarded by tsc alone, which vitest does not run.
Both are now pinned at runtime by asserting the code string and the full
key set.

Also fixes a real display defect: `toFixed(0)` renders a ratio of 0.795
as "hard at 80%", and 80% is critical's documented threshold. One
decimal, so the number and the level cannot contradict each other.

And corrects a JSDoc claim of mine that was simply wrong: `pressure` is
absent not only for direct-embed but on the bootstrap /daemon/status
route, which omits runtime.memory wholesale even though the budget is
resolved — and that window is not just startup, since a daemon whose
runtime fails to start serves the bootstrap app for its lifetime.

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

* refactor(serve): model a per-child heap partition of the daemon budget (#8508)

* feat(serve): add the child-heap admission primitives, unwired

Groundwork for #8182 step 2. Nothing calls any of this yet, so no child
is sized differently and no spawn is refused.

`ProcessRegistry.committedProcessCount` counts attached children plus
reservations that have not attached. That is the figure admission has to
key on: `reserve()` inserts its token synchronously before `spawn()`, so
two racing spawns each see the other, while neither appears in
`activeProcessCount` until its child attaches. A child leaves the count
on exit rather than when `terminate()` starts, so a channel swap counts
twice while the old process winds down — deliberate, since its memory is
still resident.

`getAcpMemoryArgs(explicitMb?)` takes an optional share that bypasses
both the module cache and the raise-only guard. Both bypasses are
load-bearing. The cache, because the share depends on how many children
are live now rather than on the host. The guard, because a
budget-derived share is normally *below* the daemon's own heap limit, so
routing it through `targetMB > currentLimitMB` would drop the flag,
silently restore the overcommit, and leave every test green — the trap
against a multi-GB runner, and mutation-checking it by reinstating the
guard fails two tests.

`createChildHeapPolicy` holds the mode, the budget, and the would-be
refusal counter, and answers `decide(concurrentChildren)`. The refusal
is derived from the unclamped quotient, not from
`recommendedChildShareMb`, because that function clamps *up* to the
512 MB floor: past the point where the pool stops covering the count its
answer saturates and can no longer distinguish "barely does not fit"
from "wildly does not fit".

`ChildHeapPoolExhaustedError` with both transport mappings — REST 503
with Retry-After, ACP `child_heap_pool_exhausted` — added together,
since the two mappings are hand-written and drift silently otherwise.
Refusing at spawn rather than at registration is the correction #8182
demands: registration allocates nothing, so this surfaces as "no new
session in this workspace right now", which is true and retryable.

Refs #8182.

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

* feat(serve): size each ACP child by concurrently live children

Wires the primitives from the previous commit into the spawn path, behind
`--child-heap-mode off | observe | enforce`, default `observe`.

Under `enforce` a child's `--max-old-space-size` is a share of the child
pool divided by the children concurrently committed at the moment it
spawns — read from the shared ProcessRegistry after `reserve()`, so two
racing spawns each see the other. When the pool cannot cover another
child at the 512 MB floor the spawn is refused with
ChildHeapPoolExhaustedError, which is what turns a per-child ceiling into
an aggregate bound: concurrent children can never exceed pool/512.

Keyed on concurrency, never on registrations. A dormant workspace has no
child, so it costs nothing — the specific correction #8182 records
against the withdrawn proposal, which would have shrunk a lone live child
to 614 MB because of 24 idle registrations.

Default `observe` computes the share and the admission decision and
applies neither, counting the refusals that would have happened. The
divisor has never been checked against a real multi-workspace deployment,
and a non-zero count is how an operator learns enforcement would have
broken them without being broken. It also catches the case worth
worrying about: a channel swap counts the dying child alongside its
replacement, so on a saturated pool enforcement could refuse a restart
and leave that workspace with no child at all. Excluding terminating
children would authorise real overcommit to dodge a hypothetical
refusal, so the count reports it instead.

Ceilings already granted are not revisited — V8 cannot lower them — so
granted ceilings transiently exceed the pool. Acceptable: the flag is a
ceiling, not a reservation, and a workspace with no live sessions has no
child and picks up the current share on its next spawn.

`limits.memory.enforced` stops being a required literal `false`. #8245
made it one so a client could never mistake that namespace for
enforcement that had not shipped; it has now, so the field is a boolean
derived from the mode — and stays `false` under `observe`, which applies
nothing.

Refs #8182.

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

* docs(serve): correct the claims child-heap enforcement makes false

Two sentences in the protocol doc described the memory section as
unconditionally observational: "a required `enforced: false`", and "no
child spawn argument derives from these values, and no request is
refused on their basis". Both are false under `--child-heap-mode
enforce`, so both are rewritten rather than left to rot — `enforced` is
now documented as the boolean that answers exactly this, and the refusal
is documented with its wire shape on both transports.

Also documents `childHeap.refusals` as the calibration signal, since a
would-be-refusal count is useless if operators do not know to read it
before switching to `enforce`; the flag row in the three operator docs;
and the design doc's Part 1, which listed applying a share as a
compatibility risk without recording how that was resolved.

The end-to-end test asserts the policy reaches a real booted daemon's
status with `enforced: false` under the default mode — the wire type in
that test is a hand-written mirror, so its `enforced: false` literal had
to widen too, which is the check that caught the type not being widened
everywhere.

Refs #8182.

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

* test(serve): cover both branches of the enforced tripwire

`enforced` was only ever asserted false — the unit tests build no policy
and the end-to-end daemon runs the default `observe` mode, so the branch
that makes the field worth having was untested. Hardcoding it back to
`false` passed everything.

Also pins `childHeap: null` as distinct from a policy in `off` mode: the
first says no policy exists (direct-embed, or the bootstrap window before
the runtime is built), the second says one exists and computes nothing.

Refs #8182.

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

* fix(serve): partition the child pool so granted ceilings stay inside it

Review was right that the previous design did not deliver the aggregate
bound it claimed. Sizing each child by the count live at *its* spawn
bounds the child count but not the memory: V8 cannot lower a running
child's ceiling, so grants accumulate as P + P/2 + P/3 + ... = P x H(n).
Reproduced exactly — 9557 MB authorised against a 3687 MB pool at seven
children on an 8 GB host, and 61355 MB against 15360 MB at the limit on
32 GB. That is 2.6x and 4x the pool, which is what the policy exists to
prevent.

Grant accounting alone does not fix it: the first child would take the
whole pool and the second would be refused immediately. Keeping the
invariant requires early children not to receive the whole pool, so the
ceiling is now a fixed partition — childPoolMb / maxConcurrentChildren,
constant for every child, with maxConcurrentChildren itself derived from
the pool and capped at MAX_DAEMON_WORKSPACES. The sum is then
n x ceiling <= pool by construction, with no ledger of outstanding
grants and no dependence on arrival order. Tested as an invariant across
four host sizes: fill the daemon to its admission limit and the
authorised total still fits.

The cost is deliberate and now documented rather than hidden: a lone
workspace on a 32 GB host gets 614 MB rather than the pool, because any
child may still be running when the house fills. An 8 GB host admits
seven concurrent children at 526 MB each.

Also from review:

- The policy is no longer built for an injected `deps.bridge`. That
  bridge carries its own channel and never reaches the factory the
  policy rides on, so status could report `enforced: true` while nothing
  was being sized.
- Both transport mappings now have direct tests. They are hand-written
  beside each other and drift silently; the spawn-policy tests cannot
  catch a wire regression.
- Swept the "does not size any child" claim, which enforce makes false,
  out of the CLI help text, ServeOptions docs, the two operator tables,
  and the e2e header comment. The 17-configuration table realigns
  wholesale because that cell was its widest — whitespace only.

Refs #8182.

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

* refactor(serve): model the child heap partition, defer applying it

Review established that the refusal counter cannot tell an operator
whether enforcement is safe, and that is the ground the enforcing mode
stood on. While observing, children run on the host-derived ceiling
(16384 MB on a 32 GB host), so a workload needing 2 GB of old space is
healthy with zero refusals and OOMs the moment a 614 MB partition is
applied. The counter measures admission pressure, not ceiling adequacy.

Rather than ship a switch with no safe way to decide when to turn it on,
`enforce` is removed. `--child-heap-mode` is `off | observe`, and the
mode that would apply the partition arrives with the measurement that
justifies it: peak old-space per child, compared against the modeled
ceiling. That is a real measurement chain — the child reports rss and
cpu today, and `--max-old-space-size` bounds old space specifically, so
neither rss nor heapUsed answers the question.

With nothing applying the partition, the machinery that existed only to
apply it goes too rather than shipping unreachable:
`getAcpMemoryArgs(explicitMb?)`, `ChildHeapPoolExhaustedError` and both
transport mappings, and `limits.memory.enforced` reverts to the required
literal `false` it was before. The spawn path is untouched again; the
factory asks the policy what it would decide purely so the count is
real.

Also fixes the zero-pool defect review found, which the removed clamp
caused: forcing at least one admissible child on a 512 MB host — where
the root reserve consumes the whole 256 MB budget — produced a ceiling
of 0, and `--max-old-space-size=0` is V8's *default* heap, not a zero
ceiling. A pool that cannot cover one child at the floor now reports
`maxConcurrentChildren: 0` and `perChildCeilingMb: null`, and the test
that enshrined the old behaviour is inverted.

Status now publishes `maxConcurrentChildren` and `perChildCeilingMb`, so
an operator can judge the partition against their own workload — the
substitute for a counter that cannot judge it for them. Every claim that
a zero refusal count means the partition is safe to apply is removed
from the flag help, the operator docs, the protocol doc, and the design
doc.

Refs #8182.

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

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

* fix(serve): repair the child-heap assertion and the reservation leak

Three findings review raised against #8508 after the partition became
observation-only, all still live on this branch now that it has merged.

The status assertion in `run-qwen-serve.test.ts` failed on head: it used
`toEqual` against `{ mode, refusals }` while the wire also carries
`maxConcurrentChildren` and `perChildCeilingMb`, so the suite was red at
217 passed / 1 failed. The local type restating the wire shape was short
the same two fields. Both are filled in, and the assertion stays `toEqual`
so an unannounced field still fails it — the two derived figures get
matchers because this suite boots a real daemon and the pool follows the
machine. What they have to satisfy is now pinned separately: a fixed
ceiling times the number admitted must fit inside the pool it partitions,
which is the whole reason the partition bounds anything.

`decide()` and `getAcpMemoryArgs()` ran between `reserve()` and the `try`
that cancels the reservation. `childHeapPolicy` is a public
`createSpawnChannelFactory` option, so `decide()` is caller code and may
throw; the spawn then rejected with the token held for the process
lifetime, inflating `committedProcessCount` for every later spawn. Both
calls move inside the `try`. The regression test is mutation-verified —
reverting the move gives `expected 1 to be +0`.

`ServeOptions.memoryBudgetMb` still promised a `childHeapMode: 'enforce'`
that sizes children and refuses spawns. No such mode exists.

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

* fix(serve): report no child-heap partition under `off`

`snapshot()` returned `maxConcurrentChildren` and `perChildCeilingMb`
unconditionally, so a daemon run with `--child-heap-mode off` still
published a partition — 7 children at 526 MB on an 8 GB host — under a
mode whose documentation says "do not model it". Review raised it, and it
mattered more than it looked: with `enforce` gone, `off` and `observe`
differed only in whether `refusals` incremented, so nothing on the wire
distinguished a model that was switched off from one in force.

Both figures are now `null` under `off`, which required widening
`maxConcurrentChildren` to `number | null` in the daemon type and the SDK
mirror. `null` rather than `0`: zero is already the computed answer for a
pool too small to host one child at the 512 MB floor, and collapsing the
two would tell an operator who disabled the model that their host cannot
run anything. That leaves three distinguishable states — no policy at all
(`childHeap: null`), a policy modeling nothing (`mode: 'off'` with null
figures), and a live model — and each now has a test.

The `off` unit test previously asserted only `refusals`, so its name
("models nothing at all when off") promised more than it checked. It now
covers the figures, with a sibling test pinning 7 / 526 under `observe` on
the same budget so nulling them unconditionally cannot satisfy both.
Mutation-verified in both directions.

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

* fix(serve): never model a child heap ceiling below the documented minimum

`perChildCeilingMb` is `min(floor(pool / maxConcurrentChildren),
legacyChildCeilingMb)`. The first term is at least `MIN_CHILD_HEAP_MB` by
construction; the second is `floor(available / 2)` and is not, so the
`Math.min` could publish a ceiling *below* the `minChildHeapMb` sitting beside
it in the same snapshot:

    avail=768  --memory-budget-mb 1024  pool=512  legacyCeil=384  perChild=384
    avail=1023 --memory-budget-mb 1024  pool=767  legacyCeil=511  perChild=511

Unreachable from a derived budget — the pool reaches 0 first — but an explicit
budget has a floor of 1024 while available memory does not, and
`docs/users/qwen-serve.md` tells operators on exactly these hosts to pass that
flag. The documented remedy is what reaches the band.

Refuse the model rather than shrink under the floor, with
`maxConcurrentChildren` zeroed in lockstep: a ceiling no child may run at is
not a partition, and "one child fits" beside a null ceiling is the same
contradiction from the other side. Nothing is applied today so the impact was a
wrong published figure, but this is the number the partition asks to be judged
by and the one an `enforce` mode would hand to `--max-old-space-size`.

The existing matrix resolves derived budgets only, which is why the mutation
sweep came back clean; add the `budgetMb` axis, asserting in each case the
shape that makes it reachable, and pin the inclusive boundary (1024/1024 ->
one child at 512) so nulling unconditionally cannot pass instead.

Also, in the same review pass:

- Split usable-gauge handling into numerator and denominator. Coercing an
  unusable numerator to 0 published `rssBytes: 0, rssRatio: 0, level: 'normal',
  source: 'rss'` — a daemon that measured nothing, indistinguishable from an
  idle one, which is the confusion `source: 'unknown'` and `sampled: 0` exist
  to prevent everywhere else here. An unusable numerator now retires its own
  side. Zero stays a reading for a numerator and not for a denominator.
- Document that `rssRatio` divides by host total under
  `availableMemorySource: 'host'`, so it is a lower bound on real pressure
  there — a denominator problem no threshold calibration addresses.
- Document that `refusals` counts channel swaps at full occupancy (the
  terminating child is counted until it exits) and equals the total spawn count
  on a host too small to model a partition. Deliberately not fixed by giving
  the comparison swap headroom, which would admit a 26th ceiling against a
  25-child pool.
- Keep the sampler's rejection handler as a documented backstop — the shipped
  `refreshChildResource` never rejects, but it is an optional `async` interface
  member, so a foreign implementation throwing early would otherwise surface as
  an unhandled rejection — and give it the workspace so it is attributable
  across the fan-out.
- Test hygiene: drop a duplicated `enforced` assertion; replace a host-
  dependent `expect.any(Number)` with a key-set pin plus a branch, since a
  small host now legitimately reports no partition; use `vi.spyOn(Date, 'now')`
  over direct assignment; reuse the exported `ChildHeapMode` on the child-heap
  side, leaving the independent `memoryPressureMode` switch alone.

Reported by @wenshao.

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

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-08-07 06:10:37 +00:00
jinye
037b4d9c03
feat(daemon): Add SSE stream and client observability (#8572)
* feat(daemon): add SSE stream observability

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

* fix(daemon): Address SSE observability review feedback

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

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-08-06 16:44:15 +00:00
顾盼
a5c637b749
feat(web-shell): add native Live Voice (#7859)
* feat(web-shell): add native Live Voice

* fix(web-shell): address review feedback for Live Voice PR (#7859)

- Quote all strings in electron-builder.yml to fix yamllint CI failure
- Gate discovery publish on liveVoiceEnabledAtBoot to avoid writing
  bearer token to disk when Live Voice is disabled (M1)
- Add child identity guard to CommandMonitor stdout/stderr handlers
  to prevent stale helper output from corrupting the new buffer (M4)
- Add exponential backoff to sent-completion delivery retry (M3)
- Skip broadcastState when setCallState/setTranscript value is
  unchanged to reduce per-audio-delta overhead (H1)
- Document sent-mode completion notification in module docstring (H2)
- Remove dead protocol/nonce aliases from readDiscoveryFile
- Fix single instance lock fall-through with process.exit(0)

* fix(cli): register realtime_voice in docs contract and env guard (#7859)

* fix(web-shell): address review feedback for Live Voice PR (#7859)

* fix(cli): discard orphaned isolated dir when parent restore fails (#7859)

* fix(web-shell): address review feedback for Live Voice PR (#7859)

* fix(serve): harden live turn recovery

* fix(desktop): restore Live Host native build

* fix(live): align native host and session isolation

* fix(acp): preserve live worker continuation lineage

* fix(live): classify provider close reasons

* fix(serve): discard unused recovered conversation dirs

* fix(live): isolate authorized realtime responses

* fix(live): preserve realtime response authority

* feat(web-shell): complete Live Voice onboarding

* fix(live): persist realtime-owned dialogue

* fix(live): preserve final speech while stopping

* Revert "fix(web-shell): address review feedback for Live Voice PR (#7859)"

This reverts commit 7110bec6b034c702bca6e28e35b93c7f70e729cd.

* Revert "fix(cli): discard orphaned isolated dir when parent restore fails (#7859)"

This reverts commit 85165f1b2ddfaa311b8be91acdd76a6f388f6204.

* Revert "fix(web-shell): address review feedback for Live Voice PR (#7859)"

This reverts commit 9199fa633e102bb8f24e4b216d322be4323eb3fc.

* Revert "fix(cli): register realtime_voice in docs contract and env guard (#7859)"

This reverts commit 6b6b1718352ef01a98a73976b5c7c4433fd14c35.

* Revert "fix(web-shell): address review feedback for Live Voice PR (#7859)"

This reverts commit e083779105199d26de3afd8ad00719a08efe3099.

* revert(live): remove remaining takeover behavior

* revert(live): restore pre-rollback implementation

* test(cli): align Live diagnostics env guard

* test(release): cover Live Host publication

* fix(ci): re-sign Live Host package before verification

* fix(serve): scope sent completion notifications to Live

* fix(web-shell): preserve live setup errors

* fix(live): align realtime backend speech lifecycle

* ci(live): publish Live Host independently

* test(cli): mock Live speech bridge handler

* test(release): align Live Host workflow contract

* fix(live): address release and lifecycle review findings

* fix(live): release completed call tracking

---------

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
2026-08-05 08:33:22 +00:00
ChiGao
5631f4b112
feat(serve): add a required external tool guard provider (#8125)
* feat(serve): add required external tool guard

* fix(serve): keep guard constants off fast path closure

* test(serve): cover guard startup options

* refactor(acp-bridge): centralize external tool guard validation and ack value (#8125)

* fix(core): align MCP reconnect timeout test with safe replay policy (#8125)

The reconnect-on-timeout test still built its mock tools without server
trust or tool annotations, which the safe replay change now requires
before automatically replaying a connection-loss failure. Update the
fixtures the same way the surrounding reconnect tests were updated,
keeping the test's original assertion that a timeout on a known
disconnected server goes through the reconnect path. Mirrors the same
alignment already landed on main.

* fix(cli): alias externalToolGuard subpath for vitest source resolution (#8125)

This PR added `@qwen-code/acp-bridge/externalToolGuard` imports to cli
serve/acp modules but not the vitest source alias every other acp-bridge
subpath carries. Without it, any vitest run whose acp-bridge dist is
stale or absent fails to resolve the import and the five serve test
files die at transform time. Add the alias following the documented
convention in the config so tests read the live source.

* fix(serve): reject non-ASCII external tool guard bearer tokens

A token outside the ASCII range passed construction but made the
handshake throw ERR_INVALID_CHAR when interpolated into the
Authorization header, blocking qwen serve startup in required mode
with an unexplained error. Enforce printable ASCII (0x21-0x7E) at
validation time so the configuration fails fast with a clear message.

---------

Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.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: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-08-04 14:37:13 +00:00
jinye
0cb109f513
fix(core): Avoid replaying unsafe MCP tool calls (#8387)
* fix(core): Avoid replaying unsafe MCP tool calls

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

* fix(core): Revalidate MCP replay after reconnect

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

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-08-03 11:04:38 +00:00
jinye
d1648b3af9
feat(telemetry): Track tool execution outcomes (#8180)
* feat(telemetry): track tool execution outcomes

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

* fix(telemetry): address execution-status review feedback (#8180)

- Update stale nonInteractiveToolExecutor expectations for executionStatus (red CI)
- Scope the cancelled span-status short-circuit to tool_call events so other
  cancelled events carrying an error keep ERROR status
- Record loop-detection skips as UNKNOWN, not EXECUTION_DENIED, keeping the
  denial metric accurate
- Assert execution_status on the resolved-with-error PostToolBatch path
- Raise tool-call observer-failure logging from warn to error
- Clarify subagent-projection exclusion and JSONL compatibility in design doc

* fix(core): address review findings 3-6 on tool execution status (#8180)

- recordToolExecutionMetrics now merges common attributes (session.id
  opt-in) like every other counter in metrics.ts
- Lift TOOL_FAILURE_KIND_ATTRIBUTE / TOOL_FAILURE_KIND_CANCELLED into
  telemetry/constants.ts so coreToolScheduler and session-tracing share
  one definition
- Add debugLogger to runToolTelemetrySink catch (was silent)
- Replace delete-based absence in withPostToolBatchStop with conditional
  spread

* fix(core): address remaining review findings on tool execution status (#8180)

- Pass the frozen executionStatus variable instead of the literal
  'success' in the post-hook-stop error response, keeping the frozen
  value the single source of truth (finding 4)
- Force-finalize the deferred PostToolBatch parent span in the abort
  drain, since that terminal path cancels the batch hook that otherwise
  owns the span; documents the invariant at the call site (finding 6)
- Comment the loop-detection guard so the permission-cancellation
  exclusion from invalid-param loop detection is explicit (finding 9)
- Rename the design doc to the dated docs/design convention and note
  the schedule()/handleConfirmationResponse() resolution contract
  change for embedders (finding 2, doc convention)

* docs(core): note schedule() resolution contract in tool execution status design (#8180)

Record the embedder-facing behavior change that schedule() and
handleConfirmationResponse() resolve with a terminal error call rather
than rejecting, so a failing tool no longer aborts its siblings.

* fix(telemetry): address review feedback for tool execution status (#8180)

- Document the new tool_call attributes (call_id, execution_status), the
  qwen-code.tool.execution.count metric, the tool.execution span attributes,
  and the tool.failure_kind=cancelled span field in telemetry.md.
- Pass ToolErrorType explicitly at loop-detection skip sites instead of
  inferring it from the skip message string, so copy edits cannot silently
  reclassify loop skips as approval denials.
- Simplify withPostToolBatchStop response construction (drop the
  destructure-and-reattach used to preserve a missing execution status).
- Add a debug breadcrumb when a PostToolBatch stop has no span to attach to,
  and a one-time warning when PostToolBatch hook detection fails open.
- Drop the try/catch wrapping the pure isTelemetrySdkInitialized getter.
- Clarify the design doc invalid-combination wording and note that the
  execution-failure SLI cannot be attributed to a specific tool.
- Add a regression test pinning that schedule() resolves (not rejects) when a
  tool execution throws.

* fix(telemetry): address round-7 review feedback for tool execution status (#8180)

* fix(telemetry): restore type-safety fallback for executionErrorType (#8180)

* fix(telemetry): align tool execution failure outcomes

Keep Core and ACP cancellation arbitration consistent, preserve structured post-processing errors, and restore QwenLogger MCP metadata privacy.

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

* fix(core): address review suggestions for tool execution status (#8180)

* test(core,cli): strengthen test-efficacy for tool execution status (#8180)

* fix(core): address review suggestions for tool execution status (#8180)

- Improve post-processing cancellation message to indicate the tool had
  already completed, preventing silent model redo of completed work
- Remove dead !isExecutionTimeout conjunct in Session.ts PostToolUse
  cancellation check (unreachable: timeout always sets toolResult.error)
- Replace construct-then-delete with destructuring in withPostToolBatchStop
- Move all failure-kind constants to telemetry/constants.ts so the full
  documented vocabulary lives in one place
- Re-export StructuredToolError from tool-error.ts instead of importing
  from the unrelated priorReadEnforcement module
- Add JSDoc to normalizeToolCallEvent documenting key-absent semantics
- Add ordering-safety comment to createParentAbortRace microtask guarantee
- Document endToolExecutionSpan not_started guard as defence-in-depth
- Document PostToolBatch span leak window in finalizeToolSpan
- Add design doc note about hand-placed cancellation check invariant
- Add test for unknown execution_status normalization path
- Revert unrelated generate-notices.js formatting change

* fix(core): address review feedback for tool execution status (#8180)

- Gate cancel message on executionThrew so the model sees 'User
  cancelled tool execution.' when execute() rejected under abort,
  reserving 'already completed' wording for post-processing cancels
- Move StructuredToolError into tool-error.ts to break the
  tool-error ↔ priorReadEnforcement module cycle
- Revert unrelated Prettier reformat in generate-notices.js

* test(core): pin both tool cancellation notices; extract them as constants

afd349ca gated the cancel message on executionThrew but left the two
wordings as bare literals at four sites and added no test. That is the
exact shape the bug had: it was introduced by editing one literal and
missing the others.

Extract TOOL_CANCELLED_{BEFORE,AFTER}_COMPLETION_MESSAGE so the four
sites cannot drift, and add regression tests for both paths — a tool
interrupted mid-flight (execute() rejected under abort) must report
"User cancelled tool execution.", while a cancel after execute()
returned must report that the output was discarded. The mid-flight test
fails against the pre-afd349ca behaviour.

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

* fix(core): keep MCP reconnect for a timeout on a dead transport

Classifying every `-32001` as EXECUTION_TIMEOUT skips
handleReconnectOnError, which previously recovered one real case: the
transport dies mid-request, the SDK request times out because no
response will ever arrive, and the server is already recorded
DISCONNECTED. That reconnected and retried; now it hard-fails and the
user has to retry by hand.

Divert back to the reconnect path only on positive evidence the
transport is dead. Note that getMCPServerStatus() reports DISCONNECTED
for servers it has never seen, so the guard checks for a *recorded*
DISCONNECTED — the naive comparison misroutes every timeout from a
server whose status was never registered, which broke four existing
timeout tests when tried.

A timeout on a healthy server is still EXECUTION_TIMEOUT: retrying it
after a reconnect would just double the wait. The client-side idle
timeout keeps classifying unconditionally; it is our own timer, not a
transport signal.

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

* fix(core): address blocking review feedback for tool execution status (#8180)

Two blocking items from the maintainer review:

1. Post-processing cancellations dropped persistedOutputFiles (and
   visionBridgeNotice) along with the model-visible output, orphaning
   files the tool had already spilled to disk. createCancelledResponse
   now carries both, and every cancelAfterPostProcessing site passes
   what it has; the settle-then-abort and hook-stop paths do the same.

2. A -32001 that lands while the parent signal is aborted is the SDK's
   abort rejection or a timeout that raced with a cancel; classifying
   it EXECUTION_TIMEOUT would count user cancels against the timeout
   SLI. isExecutionTimeoutFailure now defers to the abort in both
   catch blocks, regardless of which side settled the race first. The
   two tests that pinned the opposite timeout-wins ordering are
   updated to the abort-wins semantics the review asked for.

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

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.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-code-bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: qwen-code-ci-bot <253268222+qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Qwen Code <noreply@alibaba-inc.com>
2026-08-03 10:21:44 +00:00
Shaojin Wen
72bd3dccc2
ci: remove broken legacy scheduled PR triage workflow (#8434)
The Gemini-era scheduled PR triage workflow has been dead weight for a
long time:

- Its only business value — syncing labels from the linked issue to the
  PR — never fires: gh exports closingIssuesReferences as a flat array,
  so the script's '.closingIssuesReferences.nodes[0].number' jq path
  always errors, the error is swallowed by 2>/dev/null, and every PR
  falls into the "No linked issue found" branch. The latest production
  run logged 157 "No linked issue" hits and zero label syncs, despite
  many of those PRs having linked issues.
- LABELS_TO_REMOVE is computed but never applied, PRS_NEEDING_COMMENT is
  never appended to, and the prs_needing_comment job output has no
  consumer — the rest of the script is dead code.
- It burns 1+N API calls against every open PR every 15 minutes.
- The id-token: write permission is a leftover from the Gemini/GCP OIDC
  era; nothing in the bash script uses it.

Real PR triage lives in qwen-triage.yml. Remove the workflow and its
script, drop the stale docs section describing behavior it never had,
and pin the file into the legacy-workflow regression list.

Co-authored-by: verify <verify@local>
2026-08-03 10:20:42 +00:00
jinye
4338120100
feat(serve): resolve and report the daemon memory budget (#8245)
Some checks are pending
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 1/2 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 2/2 (push) Waiting to run
E2E Tests / channel-plugin E2E (nightly) (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
SDK Java / ubuntu-latest / Java 11 (push) Waiting to run
SDK Java / ubuntu-latest / Java 17 (push) Waiting to run
SDK Java / macos-latest / Java 21 (push) Waiting to run
SDK Java / ubuntu-latest / Java 21 (push) Waiting to run
SDK Java / windows-latest / Java 21 (push) Waiting to run
SDK Java / Real daemon E2E / Java 11 (push) Waiting to run
* feat(serve): resolve and report the daemon memory budget

The daemon has no notion of how much memory it has. It samples its own
RSS and heap every five seconds, and polls the primary ACP child's RSS,
but there is no limit anywhere to divide those by: no cgroup read, no
heap-size limit, no ratio, no `limits.*` memory field. Every number it
reports is an absolute byte count with nothing to compare against, so
"how close to exhaustion is this daemon" cannot be answered from
`/daemon/status` at all.

Resolve one set of figures at boot and report them. Configured and
effective budgets are separate: the effective value is capped at resolved
cgroup or host memory, so an operator passing a budget larger than the
machine gets a denominator the machine can actually back, with the
discrepancy visible rather than silently resolved. A derived budget below
the documented minimum is reported as `insufficientMemory` rather than
clamped upward, which would have invented capacity that does not exist —
a 768 MB host would otherwise report a 1 GB budget and poison every ratio
computed from it.

`limits.memory` carries the static figures, including `legacyCeilingMb`:
the ceiling an ACP child receives today with no budget involved, so the
gap between current behavior and any future policy is measurable before
that policy exists. `runtime.memory` carries live counts and the advisory
per-child share at both the registered and the live child count.

Nothing here sizes a child. Dividing the pool by a workspace count is not
a sound policy on its own, and the advisory shares exist to show why: on
a 32 GB host with 25 registered workspaces and only the preheated primary
live, a registered-count divisor would cut that child from 16384 MB to
614 MB for memory no dormant workspace is holding, while the per-child
floor still lets 25 children authorise more than the pool. Registration
is not allocation; a real policy needs admission at spawn time keyed on
concurrently live children, and it needs this data to be designed
against.

Applying such a share is also a compatibility change even with no
refusals, since it alters child GC and OOM behavior — so it is not
something this change should slip in under the heading of reporting.

Refs #8182
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(serve): report honest memory counts and guard the registered share (#8245)

* fix(serve): reuse workspace snapshots when counting active children

Counting active ACP children from `listManaged()` is right — `list()` only
returns entries in `active` state, so a workspace mid-drain, mid-replacement,
or blocked still holds a live child that `list()` drops. But taking the count
by calling `getDaemonStatusSnapshot()` again per managed runtime undoes the
existing reuse of the primary bridge's snapshot, and `getDaemonStatusSnapshot`
rebuilds the whole session array on every call.

The second pass also reads the tree at a different instant than the rest of
the response, so `activeAcpChildren` could disagree with the session and
channel figures beside it.

Reuse the snapshots already taken instead, keyed by bridge, and fall back to a
fresh call only for a managed runtime the first pass missed — which is exactly
the non-`active` entry the `listManaged()` change exists to catch.

The reuse this restores was already guarded by a test asserting one snapshot
call per bridge, but that test resolves no memory budget, and the second pass
ran only on the budget path — so it stayed green while every production
`/daemon/status` call did the work twice. Added a case that resolves a budget
and asserts the same property; it fails against the previous commit with
"expected 1 times, but got 2 times".

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

* fix(serve): address review feedback on daemon memory budget (#8245)

* fix(serve): import isValidMemoryBudgetMb in serve command (#8245)

* fix(serve): address review feedback on daemon memory budget (#8245)

* fix(serve): stub isChannelLive on serve test fake bridges (#8245)

* fix(serve): address review feedback on daemon memory budget (#8245)

- Make the stderr-gate test host-independent by pinning os.totalmem
  through a vi.mock toggle instead of reading the runner's cgroup
- Add a spawn-path constant parity test enforcing that
  getAcpMemoryArgs and legacyChildCeilingMb agree on the fraction
  and cap, converting the comment-only invariant into a test
- Narrow the mirror comment to name only the two constants that
  actually have spawn-path counterparts
- Accept availableMemorySource through the resolveDaemonMemoryBudget
  seam so the constrained path is testable end-to-end
- Report maxChildHeapMb alongside minChildHeapMb on the wire so
  clients can distinguish the 16 GB cap from a large host
- Move the listManaged/list comment to the computation it describes
- Add a cross-reference at the opts literal for the late-assigned
  daemonMemoryBudget field

* fix(serve): address review feedback on daemon memory budget (#8245)

* fix(serve): address review — split fraction constant, sharpen parity test, deduplicate error, populate bootstrap memory (#8245)

* fix(serve): address review — document maxChildHeapMb, make parity test order-independent (#8245)

* fix(serve): address review — split parity test into two files to avoid cold re-import timeout (#8245)

* fix(serve): address review — correct session count, compatibility scope, bootstrap memory docs (#8245)

* fix(serve): address review — align help text framing, document default cap, fix stale comment (#8245)

* fix(serve): address review — add positive memory-budget validation test, correct activeAcpChildren docs (#8245)

* fix(serve): address review — document childRssBytes stale tail after watcher detach (#8245)

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Qwen Autofix <qwen-autofix@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-08-02 04:19:31 +00:00
Dragon
4c6e2518a3
docs: refresh architecture overview (#8325)
* docs: refresh architecture overview

* docs: complete architecture package table
2026-08-02 02:45:00 +00:00
ytahdn
554c5e44ba
feat(web-shell): support mutable default mid-turn messages (#8229)
* feat(web-shell): support mutable default mid-turn messages

* fix(serve): register mid-turn removal telemetry route

* test(serve): update telemetry route totals

* fix(test): add session_mid_turn_message_mutation to expected features list

* fix(webui): forward clientId on cross-session mid-turn removal (#8229)

- Forward the session clientId in the cross-session removeMidTurnMessage
  branch so the bridge's exact-originator match can succeed; without it the
  removal resolved to an undefined originator and could never remove the
  message stamped at enqueue.
- Strip a misaligned/malformed messageIds from mid_turn_message_injected in
  asKnownDaemonEvent instead of rejecting the whole event, mirroring the
  sidechannel parser so a buggy daemon can't silently lose the injection
  signal.
- Log a mid-turn removal miss in the bridge like the enqueue/pending-removal
  siblings, to make removal races diagnosable from daemon logs.

* fix(web-shell): exclude annotations from mid-turn path and harden idle cleanup (#8229)

* fix(web-shell): add container-type to .queuedPrompts so @container query applies (#8229)

* fix(web-shell): harden mid-turn dedupe and capability gate per review (#8229)

- removeInjectedFromQueue now matches by id first (position-independent)
  and falls back to text only when no id match exists, so two same-text
  sends can't remove the wrong row and double-deliver.
- Thread canMutateMidTurn into useQueuedPrompts and gate the mid-turn
  delete/edit mutation on it, so the keyboard path can't hit a DELETE
  route the daemon doesn't advertise.
- asMidTurnMessageInjectedData omits a malformed messageIds key instead
  of leaving a present undefined, matching the sidechannel parser.
- Narrow MidTurnQueueItem.midTurnState, document the load-bearing effect
  order, and make clearQueuedPrompts return false on a no-op clear.

* fix: harden mid-turn removal per review (log escape, cross-session client id) (#8229)

- Escape the caller-controlled messageId (and sessionId) in the mid-turn
  removal-miss stderr line to prevent log injection (CWE-117).
- Forward the target session's persisted client id on cross-session mid-turn
  removal so the bridge's exact-originator match no longer rejects valid
  removals after a session switch with per-session client ids.
- Strengthen tests: distinct-id independence for two queued messages, deferred
  removal proving the composer waits for daemon removal, and the active-turn
  delete failed-action flag.

---------

Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
2026-08-01 10:41:29 +00:00
destire-mio
412eae24b4
feat(core): add project-level fork profiles (#8148)
* feat(core): add project-level fork profiles

* fix(core): harden fork profile loading

---------

Co-authored-by: destire-mio <248462155+destire-mio@users.noreply.github.com>
2026-08-01 02:20:51 +00:00
Shaojin Wen
77d8a27eda
feat(daemon): raise default max sessions from 20 to 32 (#8235)
* feat(daemon): raise default max sessions from 20 to 32

* fix(daemon): update test assertion and docs for new default max sessions (32)

* fix(daemon): sync default max sessions

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

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-07-31 17:47:51 +00:00