refactor(cli): remove unused EnumSelector component - #15
Closed
qqqys wants to merge 412 commits into
Closed
Conversation
* 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
QwenLM#9464) The "Record changed test files for the flakiness gate" step runs its `git diff 'HEAD^1' HEAD` inside the env -i scrubbed child. On the persistent pool, resolving the `^1` parent there intermittently fails with "Could not access 'HEAD^1'" — the shallow merge-ref object store is left unreadable by prior `--depth=2` fetches — which takes down the whole verify lane. The "Pin agent inputs" step already captures the base OID to $RUNNER_TEMP/verify-base-oid while .git is still root-owned. Diff against that content-addressed OID instead of re-resolving the parent: it needs no parent walk and is the same value the workflow already trusts for its post-build re-pin. Add a pin asserting the record step reads the recorded OID rather than re-deriving HEAD^1.
… stop (QwenLM#9435) The ACP daemon Session was the only duplicate-provider-id circuit breaker path (PR QwenLM#5657) that terminated silently: the turn ended as a normal end_turn with nothing in the transcript and no telemetry, so the session looked hung. Route the breaker through recordDaemonLoopDetected with LoopType.GLOBAL_TOOL_CALL_DUPLICATE — the same loop type the non-interactive CLI reports — so foreground turns fail with the visible LOOP_DETECTED turn error, the context message is preserved for the next turn, and the LoopDetectedEvent telemetry is emitted. The bespoke repeatedDuplicateProviderToolCall result flag and its dead consumer branch are removed in favor of the existing loopDetected plumbing.
* feat(providers): add qwen3.8-max to Token Plan model list The stable qwen3.8-max has graduated from preview and is listed in the Bailian model catalog, but the Token Plan preset still only ships qwen3.8-max-preview. Mirror the preview entry's capabilities (1M context, thinking mandatory, image + video understanding) and pin the new entry in the preset unit tests. Closes QwenLM#9376 * test(core): pin qwen3.8-max generationConfig in Token Plan preset test (QwenLM#9383) --------- Co-authored-by: qwen-code-dev-bot <269191875+qwen-code-dev-bot@users.noreply.github.com>
…ompress-fast (QwenLM#9331) * fix(cli): prevent /rewind from dropping history after /compress-fast computeApiTruncationIndex treated every successful compression marker as a summarizing boundary. /compress-fast emits the same marker but removes no user prompts from the API history and inserts no summary prefix, so rewinding to a post-marker turn collapsed the truncation anchor to the startup context and silently dropped the whole pre-marker conversation. Tag compression UI items with the path that produced them (CompressionProps.compressionKind) and make the rewind mapping ignore 'fast' markers as boundaries. Legacy markers without a kind keep prior behavior. Fixes QwenLM#9320 * fix(cli): exclude microcompact media placeholders from rewind prompt count Microcompaction's media clearing replaces the top-level inlineData/fileData parts of user entries with text placeholders. A media-only user entry (e.g. an image-only ACP prompt) never produced a UI user turn, but once cleared it satisfied isUserTextContent's naive text check, desynchronizing the API prompt count from the UI turn count. Rewinding across a fast marker then returned one prompt early and truncateHistory silently dropped a turn the UI still shows — the same hazard this PR fixes, newly reachable through the cross-fast-marker rewinds it enables. Exclude placeholder-prefixed text parts from the prompt count (entries mixing placeholders with real prompt text still count), and mirror the compressionKind persistence caveat onto ChatCompressionRecordPayload. Addresses review R1-1 (Critical) and R1-3 (minimal option). * fix(cli): match full microcompact placeholder shape in rewind count - R3-1: exclude media-clear placeholders by the full generated shape (`[Old inline media cleared: <mime>]`) instead of the bare prefix, so a genuine user prompt that merely begins with the prefix still counts as a prompt. Adds a shared isClearedMediaPlaceholder() helper next to the producer constant and regression tests for both the truncation-late and spurious-compressed-error witness cases. - R3-2: document why the ACP session's private #isUserTextContent twin deliberately keeps the prefix check (ACP rewind maps against per-prompt file-history snapshots, which include media-only prompts). - R3-3: drop the zero-consumer MICROCOMPACT_CLEARED_MESSAGE and MICROCOMPACT_CLEARED_IMAGE_PREFIX barrel exports added by the prior commit; export only the new isClearedMediaPlaceholder() helper. * fix(core): match empty-mime media-clear placeholders in rewind count sanitizeMimeForPlaceholder returns '' for empty/whitespace-only/ bracket-only mimeTypes and the producer's ?? fallback only covers null/undefined, so microcompaction can emit `[Old inline media cleared: ]`. The consumer regex required one-or-more mime chars and rejected that shape, leaving a cleared media-only entry counted as a genuine prompt and desynchronizing the rewind prompt count. Accept the empty-mime shape and pin the producer/consumer round-trip for degenerate mimeTypes. Also correct the ACP-twin note in historyMapping.ts (the twin keeps the bare text-presence check, not a prefix check) and document the deliberate divergence reciprocally on Session.ts #isUserTextContent, with a Session.test.ts pin that cleared placeholders stay rewindable on the ACP side. * docs(cli): document and pin exact-match placeholder collision in rewind mapping Records the known exact-match collision corner next to the media-clear placeholder exclusion and pins its fail-safe behavior with a test: a genuine prompt whose entire text equals a generated placeholder is excluded from the API prompt count, so later rewind targets abort loudly (-1) instead of truncating against a misaligned count. A durable fix needs a structural sentinel on cleared parts, which changes the persisted API history shape and stays out of scope for this PR. * fix(cli): tighten rewind placeholder match and pin mid-history collision Address the round-5 review thread within the scope that is not pending the maintainer structural decision: - The media-clear placeholder regex no longer accepts \r/\n/\t interiors: sanitizeMimeForPlaceholder normalizes them to spaces, so the producer can never emit them, and accepting them let multi-line user text that merely starts with the prefix be misclassified as a cleared-media placeholder. Every producible shape (including the empty-mime one) still matches. - The exact-match collision doc no longer claims "never silent history loss": mid-history, rewinding TO a colliding turn truncates one turn late (its prompt+response stays in model context while the UI removes the turn - under-deletion, not loss of context the UI keeps). The mid-history shape is now pinned next to the first-turn pin so a future structural sentinel updates both expectations. * fix(cli): fail loud on rewind when marker-less auto-compaction left a compressed prefix (QwenLM#9437 entrance 3)
…LM#9433) A named Agent Team teammate accepted run_in_background: false but silently ignored it: the teammate route runs before the regular foreground/background classification and always returns right after spawn, so the flag had no effect. A structured local run showed five named teammates spawned concurrently — including two launched with an explicit false — each continuing to consume model tokens while the accepted flag said not to background the launch. Named teammates are inherently concurrent (persistent team identity, messaging, automatic final-report delivery), so foreground semantics cannot apply. Make the incompatible combination hard to generate and impossible to spawn: - Tool prose: the team guidance, the name property, and the run_in_background schema description now state that teammates always run concurrently, that callers should omit run_in_background, and that an inline blocking result requires omitting name and using a regular agent with run_in_background: false. - validateToolParams rejects run_in_background: false when name routes to an active team, before any spawn. Without an active team the name falls through to a regular agent, where the foreground request stays valid. - The execute-time team-routing branch carries the same rejection as defense in depth, mirroring the existing model/isolation blocks. Fixes QwenLM#9430
…t is written (QwenLM#9460) * fix(review): clamp the posting volume at its origin, not only where it is written Follow-up to QwenLM#9413, from a maintainer's independent pass. The terminal `VOLUME:` line was the one read site of the round's count that skipped the shared volume reader: the serializer, the parser and the side-file recovery all clamp, but `result.postedInline` carried the raw drafted- comment length, so in the defensive over-cap case the line would print an uncapped number beside a marker recording the capped one — the two- outputs-disagree failure that reader's own docstring exists to prevent. Clamping once where the count is derived puts every surface on the same value. Not reachable with any real producer (no round drafts 100k inline comments), so this closes a consistency gap rather than a live bug. Also corrects a fixture comment that survived the round-4 semantics reversal: `postedInline`'s absence is preserved, not defaulted to zero, and the comment beside it still claimed the opposite of what the validator, the type and two tests all say. * docs(review): re-anchor the bodyTrim fixture comment to its own semantics The rewritten `postedInline` comment broke what the line below it refers to. The fixture's comments used to read as one chain — every field non-default because absence defaults to something — so `bodyTrim`'s "for the same reason" resolved correctly. With `postedInline` now documenting the OPPOSITE reason (its absence is preserved, not defaulted), the nearest antecedent teaches a maintainer that `bodyTrim` is preserved too, while the validator defaults it and always emits it. The comment now states its own side of the split instead of pointing at whichever comment happens to precede it.
* feat(extensions): support authenticated HTTPS Git installs * test(serve): update capability integration baseline
…M#9432) The empty list_agents result ("No background agents are available in this session") is true for the ordinary background-subagent roster but reads as team status while named Agent Team teammates are active, which can trigger duplicate launches or a false "no workers running" conclusion. Make the control-plane boundary explicit: - tool description states that named teammates are NOT listed, deliver their final reports automatically, and that list_agents/task_list polling must not be used to wait for them; - empty result names the roster it reports on and the teammate exclusion. The registries stay separate; this is a tool-contract wording fix only. Fixes QwenLM#9431
* feat(core): reject upstream fail-fast placeholder responses
Upstream endpoints occasionally fail fast with an HTTP 200 whose entire
body is the placeholder text '(request timeout)'. The response passes
all existing stream validation and gets persisted as a normal assistant
reply, polluting subsequent request context and surfacing to users as a
lost reply.
- Throw InvalidStreamError('UPSTREAM_DEGRADED_RESPONSE') at stream end
when the whole response body is exactly the placeholder, reusing the
transient retry budget so the turn is rolled back and re-attempted.
- Treat placeholder-only model turns as invalid in extractCuratedHistory
so already-polluted sessions (including --resume) stop replaying them.
Both checks are exact whole-turn matches to avoid false positives on
legitimate mentions of the text.
Fixes QwenLM#8916
* fix(core): prevent placeholder text from reaching display before retry (QwenLM#8938)
- Defer the first chunk with a finishReason in non-continuation turns so
post-stream validation (placeholder check, empty-text check) can reject
it before consumers see the content. Without this a single-chunk
fail-fast placeholder (text + finishReason: 'STOP') was yielded to the
TUI / headless adapter before the throw, and neither consumer rolled
back the committed output.
- Add exhaustion test for UPSTREAM_DEGRADED_RESPONSE retry budget.
- Add curation test for whitespace-padded placeholder (.trim() guard).
- Add curation test for placeholder turn carrying a functionCall.
- Assert the placeholder text is absent from emitted stream events.
* fix(core): block degraded continuation leaks
* fix(core): catch split degraded placeholders
* fix(core): withhold degraded placeholder chunks
* fix(core): preserve current image payloads after placeholder curation
* fix(core): preserve deferred stream chunks safely
* fix(core): fold held placeholder-prefix text into continuation accounting
When a stream is cut while degraded-placeholder-prefix chunks are held
back (never yielded), the send loop's delivery accounting saw nothing
delivered and replayed fresh — letting a placeholder split across the
cut ('(request ' + 'timeout)') slip through as the harmless-looking
fragment. Stage the held text on the pipeline's error path and fold it
in the attempt's catch so the continuation gate resumes from the prefix
and the reassembled text is still rejected as a completed placeholder.
A completed placeholder is the exception: there is nothing honest to
resume from, so that recovery stays a fresh replay.
Fixes the split-placeholder regression test; all 332 geminiChat tests
green.
* fix(core): latch deferredFirstChunk so duplicate finishReason chunks are not dropped
A second finishReason-bearing chunk arriving before any yield overwrote
deferredFirstChunk (plain assignment at both deferral sites), so only the
slot's FINAL value was yielded post-stream while every overwritten chunk's
parts still landed in allModelParts -> history/JSONL: text persisted but
never displayed, or a functionCall persisted but never dispatched. Latch
the first deferred chunk with ??= at both sites (R8-1).
* fix(core): judge deferred chunk against accumulated text on stream error
The error-path escape check judged the deferred chunk by its own parts
only. A placeholder split into a held prefix and a finishReason-bearing
tail ('(request ' + 'timeout)') failed that per-chunk prefix check on
the tail alone: the fragment leaked to consumers while the held prefix
was staged after it, scrambling the continuation handoff. Judge the
deferred chunk against the accumulated attempt text — the in-loop hold's
own expression — and hand the combined text off in delivery order.
Also drop dead stores: the yieldedAnyChunk write after the loop's last
read and the trailing local resets after the final yields.
* fix(core): deliver withheld placeholder-defense chunks in arrival order, drop the invisible continuation handoff
Three review findings on the placeholder-defense machinery:
1. The deferred first chunk reached consumers out of arrival order — the
post-loop flush yielded held chunks first, and any inline-yielded chunk
preceded it, so displayed text diverged from arrival-order history/JSONL
(and consolidation's in-place part merge rewrote the shared part the
deferred chunk carried, duplicating text on delivery).
2. The continuation handoff staged never-delivered held text into the
transport-continuation buffer; a continuation that diverged from
completing the placeholder persisted the invisible prefix into
history/JSONL while the UI showed only the remainder.
3. The staged handoff survived the turn through the MAX_TOKENS recovery
wrapper (which rethrows non-InvalidStreamError without the folding
catch) and could be folded into a later turn's continuation buffer.
Changes:
- Unify the deferred chunk and held placeholder-prefix chunks into one
arrival-ordered pendingPreValidationChunks list; every flush (in-loop
divergence release, stream-error delivery, post-loop flush) yields it in
arrival order, so deferral can no longer reorder delivery.
- Snapshot the pending chunks with detachChunkParts before history
consolidation merges adjacent text parts in place, so post-consolidation
flushes deliver what actually arrived instead of rewritten text.
- On a stream error, deliver the pending chunks (mirrored by the send loop
into the continuation buffer, keeping delivery and persistence aligned)
unless they complete the placeholder — a completed placeholder is still
dropped silently for a clean fresh replay. This replaces the
pendingHeldPlaceholderText field, its staging, the send-loop fold and the
entry reset: never-delivered text no longer enters the continuation
buffer, which also closes the cross-turn leak.
Tests: arrival-order delivery of a deferred chunk + later held chunks;
in-order delivery of a diverged placeholder prefix without retry; a lone
placeholder-prefix chunk delivered and persisted without rejection;
delivery/persistence alignment when a held-prefix continuation diverges.
Existing split-placeholder continuation coverage still green (340/340).
* fix(core): propagate image eviction to durable history, harden placeholder gates
Round-10 review findings:
- replaceImagePayloadsInPlace now rewrites the shared Part object itself
(part.text = reference; part.inlineData = undefined) instead of only
replacing the content.parts[i] slot: when placeholder curation makes two
user turns adjacent, appendCuratedContent merges them into a fresh
Content whose parts array reuses the durable history's Part objects, and
a slot-only swap left the inline base64 payload alive in history — the
eviction pass re-ran every request (countAllInlineImages stayed at the
threshold) and store.put() re-hashed the multi-MB payload each time, so
the memory this mechanism exists to shed never left history. Nested
functionResponse parts get the same treatment (getFunctionResponseParts
returns the live array). Regression test pins the durable side.
- Extract nonThoughtText(parts) and use it at both placeholder gates (the
per-chunk hold and the stream-error completed-placeholder drop) so the
two decisions cannot drift when the text-accumulation semantics change.
- Log the one deliberate discard: the completed-placeholder drop on the
stream-error path now emits a debugLogger.warn naming the withheld chunk
count, so a benign transport-cut recovery is distinguishable from lost
output after the fact.
- Comment at the eviction call site names skipParts as the load-bearing
protection for the current message's images (the identity skipEntry find
misses whenever curation merged the user turns into a fresh object).
* fix(core): preserve deferred stream metadata
* fix(core): close round-12 findings on degraded-placeholder defense
- XML tool-call recovery now aligns the withheld pre-validation chunks
with the rewritten turn: their plain text is rewritten to the stripped
remainder so consumers no longer receive the raw `<invoke>` markup
while history persists the recovered shape (fixes the deterministic
'retains a short text prefix' failure red-lining CI)
- completed-placeholder detection is now cross-attempt: a placeholder
that completes across a transport cut (prefix before the cut,
remainder before the error) is discarded instead of persisting via
the continuation-prefix fold, and the send loop refuses to continue
from a delivered text that equals the completed placeholder — it
routes through the invalid-stream budget and retries fresh
- mid-stream flush of held chunks yields detached copies so post-stream
consolidation cannot rewrite already-delivered part objects
- image reattach now resolves `Image #<id>` references from the store
even below the eviction threshold, so fallback/recovery rebuilds
after an in-place eviction pass don't carry markers without pixels
* fix(core): close round-13 findings on degraded-placeholder defense
R13-1/R13-4: the completed-placeholder conversion no longer throws
from inside the send loop's catch (which escaped the loop and skipped
the invalid-stream budget — zero retries, no telemetry, no fallback
chain). Both conversion sites now fall through to the budget handler:
the send-loop guard converts when the folded buffer completes the
placeholder, and the stream-error discard converts before rethrowing,
so a placeholder completed across a transport cut retries fresh with
the continuation reset instead of continuing from the garbage prefix.
R13-2: the fork boundary (copyHistoryContainers) now shallow-clones
Part objects — including nested functionResponse.parts — so a forked
chat's in-place image eviction can no longer strip payloads out of the
MAIN conversation's durable history (the snapshot chain shares part
objects by reference; the ids would exist only in the discarded fork's
store). The stale contract test is updated to pin the new isolation.
R13-3: the cross-attempt discard merges the UNTRIMMED attempt text
(trim only after the merge), matching the persistence merge so a
remainder opening with whitespace cannot slip past the discard.
R13-8: the below-threshold reference scan is skipped entirely when the
image payload store is empty (the common steady state).
R13-10: reattach candidates exclude ids already carried inline in the
same request (content-hash ids would otherwise ship the same payload
twice per send).
R13-15: the per-chunk placeholder hold compares the MERGED delivered
text when a continuation prefix is in flight, so a remainder that
finishes the placeholder over several chunks is withheld instead of
yielded inline before the post-stream gate throws.
Tests: double-cut regressions for both remainder shapes (with/without
finish frame), e2e below-threshold reattach, fork-boundary isolation
(top-level + nested), inline-id dedupe, split-tail converted to the
retry path, plus the R13-7/9/11/12/13/14 assertion strengthenings.
* test(core): pin full history in the second tool-result continuation test
R13-14 follow-up: the 'deliver every deferred chunk in a tool result
continuation' test also pinned history only via at(-1); replace with the
full expected history so a drifted partial-turn pop orphaning the
functionResponse turn cannot hide.
* fix(core): read the converted placeholder error type in budget telemetry
Round-13 follow-up: the invalid-stream budget handler's debug log and
ContentRetryEvent still read (error as InvalidStreamError).type, but on
the converted-placeholder path `error` is the raw transport error and
the InvalidStreamError lives in budgetError — the retry log printed
[undefined] and the telemetry event carried a wrong type. Read
budgetError.type at both sites.
* fix(core): exempt last-referenced images from reattach cap + harden tests
Round-14 review of the degraded-placeholder defense (QwenLM#8938).
Critical fix (image-payload-references): buildReattachParts ranked ids
referenced in the user's LAST message by their original eviction-marker
position, so under the maxRecentImages cap an explicitly-referenced
older image lost its slot to a stale, never-requested marker — the
reference-resolution feature silently failed in its primary scenario.
Ids referenced in the last content are now collected separately and
reattached unconditionally, outside the recency cap, mirroring
prepareImagePayloadsForRequest's unconditional referencedIds reattach.
Doc (forkedAgent): the CacheSafeParams.history contract now states the
shallow-clone behavior — consumers may rewrite part objects in place,
but deeper payload objects (inlineData, functionResponse.response)
remain shared and must not be mutated.
Test-efficacy hardening (all mutation- or trace-verified gaps from the
review): nested functionResponse inline-twin dedupe; cap-exempt
referenced-image regression; marker-present + placeholder-absent pins
on the eviction test; below-threshold e2e cap-binding, all-images
eviction, post-send-2 marker state, exactly-once reattach; mid-history
and consecutive-run placeholder curation semantics; shape-A double-cut
record/history agreement + corrected mechanism comment; diverged
continuation second-half delivery; plain-RETRY-after-rejection pin;
UPSTREAM_DEGRADED_RESPONSE routing pin on cut-before-finish;
deferred-chunk ORDER pin (finish chunk must be last); retry-exhaustion
no-CHUNK-to-consumer pin (via an expectStreamExhaustion collector);
and a hold-liveness test pinning that non-placeholder text is released
inline before stream end.
* refactor(core): dedupe placeholder-error and reattach-append sites
Ponytail review follow-ups on the degraded-placeholder defense (QwenLM#8938):
- Extract a degradedPlaceholderError() factory: four call sites
(send-loop catch-convert, stream-error completed-placeholder discard,
and the two post-stream gates) constructed the identical
InvalidStreamError literal; the factory keeps the message/type strings
from drifting between sites.
- Extract appendReattachParts(): both request-history branches ended in
the same 7-line "append onto last user turn, else push a fresh one"
block; the shared helper keeps the append shape from drifting.
* fix(core): gate placeholder conversion on !streamYieldedFunctionCall (QwenLM#8938)
- The send-loop placeholder->InvalidStreamError conversion was the only
placeholder-retry producer that could fire after a functionCall chunk
had been delivered: a folded '(request timeout)' completed on the same
attempt that emitted a tool call would schedule a fresh retry after
the consumer received the call, orphaning the tool_use/tool_result
pairing. Add the same point-of-no-return guard every sibling path
enforces; the original transport error now propagates instead.
- Regression test: folded placeholder + delivered functionCall => no
third attempt, stream terminates, last chunk is the tool call.
- Test pins from review: budget-routing assertions (mockLogContentRetry
with UPSTREAM_DEGRADED_RESPONSE) on both double-cut tests, a
non-vacuous retry guard on the prefix+remainder rejection test, and
the throwing structuredClone spy on the getHistoryLength no-clone
contract (matching the sibling walk-only accessors).
* test(core): attach the rejection handler before timer flush (QwenLM#8938)
The folded-placeholder/functionCall regression test rejects the consumer
stream mid timer-advance; attaching the expect().rejects handler only
after advancing left a handlerless rejection at the timer checkpoint,
which vitest's unhandled-error gate turns into a red run under parallel
CI load (all 20094 core tests passed; exit 1 came from the single
unhandled rejection). Attach the expectation before advancing timers so
the handler is on the promise regardless of when the send loop rejects.
* test(core): satisfy vitest/valid-expect on the pre-timer rejection handler (QwenLM#8938)
The previous fix assigned expect().rejects to a variable, which
eslint's vitest/valid-expect rule rejects (async assertions must be
awaited or returned) — CI lint failed before the suite even ran. Keep
the unhandled-rejection protection by registering a plain catch handler
before advancing timers, and leave the awaited expect().rejects
assertion as the lint-compliant pin.
* fix(core): scope placeholder run invalidation to the degraded turns (QwenLM#8938)
R16-1: extractCuratedHistory dropped the WHOLE consecutive model run
when any turn was a degraded placeholder, so a sibling functionCall
turn was dropped while the following user(functionResponse) survived —
the outgoing request carried an orphaned functionResponse, exactly the
invalid pairing the continuation gate and the MAX_TOKENS hasFunctionCall
check exist to avoid (pre-PR the run stayed whole and provider-valid).
- curation now drops only the placeholder turns of a run and keeps
valid siblings (invalid-content runs keep the pre-existing
whole-run semantics)
- repairOrphanedToolUseTurns scan no longer lets placeholder turns
split the model<->user pairing adjacency: an fr right after a
placeholder sibling counts as adjacent (the mirror run
[functionCall, placeholder] + fr previously synthesized a duplicate
error-fr because the placeholder broke adjacency)
- the whole-run drop test is rewritten to the revised per-turn
semantics; new regression tests pin both run directions through the
public API (mutation-verified: the old whole-run drop fails them).
350/350 green, tsc + eslint clean.
* refactor(core): narrow degraded placeholder defense
* fix(core): preserve placeholder-aware tool adjacency
---------
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: yiliang114 <yiliang114@users.noreply.github.com>
* fix(core): isolate image payload eviction state * fix(core): clone history parts for background agents before in-place image eviction * fix(core): protect image history snapshots * fix(core): bound referenced image reattachment * fix(core): anchor image-marker matching to the full eviction marker
…n arguments match (QwenLM#9436) * fix(core): treat duplicate provider tool-call ids as replays only on matching args The duplicate provider tool-call guard (QwenLM#5038/QwenLM#5657) keyed on the id alone, so models whose ids are only unique within a single response — e.g. Kimi emits {name}_{index} and the index can restart at 0 on any round — had fresh calls misclassified as replays: the second collision got a synthetic duplicate error and the third tripped the circuit breaker, killing every turn by round three. A handled id now maps to a (name, canonical args) fingerprint — the same sha256 repeat key the loop guards use, moved to a leaf module so toolCallIdUtils can share it without an import cycle. An incoming call is a replay only when its fingerprint matches the call that first executed under that provider id; id collisions with different args execute normally under the unique suffixed id that normalization already assigns. Exact same-args replays keep the unchanged QwenLM#5014 suppression and QwenLM#5657 breaker behavior at all four entry points (AgentCore, TUI stream, non-interactive CLI, ACP daemon session). The synthetic duplicate message now tells the model to re-issue with a fresh tool-call id when a new invocation was intended, giving id-emitting models a recovery path. * qwen: address PR review feedback (QwenLM#9436) - Fingerprint each incoming call once per carrier object: the WeakMap cache now keys on any stable carrier (FunctionCall part or ToolCallRequestInfo), and the replay predicate / recording helpers take the precomputed fingerprint instead of rehashing (name, args) on every breaker scan, admission pass, and record. - Move the getToolCallRepeatKey tests next to the extracted leaf module instead of exercising it through the loop detection service's compatibility re-export. - Restore the assertion pinning that runToolCalls never mutates the history accessor's returned fingerprint map, now that the defensive copy is load-bearing. * qwen: address PR review feedback (QwenLM#9436) Criticals: - Canonicalize onto a null-prototype object so a literal __proto__ own key (preserved by JSON.parse) stays a data property instead of vanishing through the inherited setter — two calls differing only in __proto__ no longer collide on one repeat key, which the replay oracle would have turned into a wrongly suppressed execution. - Clone request args at scheduler intake: callers pass args that can alias the model-emitted functionCall part stored in chat history, and the executor rewrites PATH_ARG_KEYS on request.args in place (a persistence the post-'ask' bounce re-execution relies on). Without the clone those rewrites leak into history and skew the replay fingerprints derived from it, letting genuine replays of path-carrying calls re-execute in multi-round agent runtimes. Suggestions: - Complete the duplicate message with the different-arguments recovery path required by design decision D3, for models whose provider assigns ids. - Fix the copy-convention comments (the accessor returns a fresh map per call; copies are future-proofing) and align all four entry points on copying the accessor result. - Pin the untested branches: history first-occurrence-wins for reused ids and orphan response-id exclusion, the fingerprint cache-hit path, the __proto__ distinction, the caller-args no-mutation invariant, and a cross-round runtime test that a replay of the original call stays suppressed after an id-colliding execution.
…arent (QwenLM#9391) * 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 52d572c 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>
* test(web-shell): add transcript contract prevalidation Freeze reproducible evidence for current transcript paths before any VS Code or HTML export production migration. - Add versioned fixtures, closed export schema, and capability gates - Probe direct-daemon and ACP identity under partial history prepend - Preserve raw adapter semantics and full write_file Turn Output diffs - Document the two-MR architecture, security constraints, and blockers * fix(web-shell): harden transcript prevalidation gates Make the evidence-only contract suite enforce the review assumptions it documents while preserving the existing runtime transcript behavior. - Run the contract suite in the required no-AK integration job - Fail closed on ambiguous identity probes and deduplicate gate kinds - Enforce manifest, hash, export safety, and renderer version boundaries - Cover visible transcript text and stable Desktop packaging semantics - Record the complete PR comment evaluation and verification outcome * fix(web-shell): close transcript prevalidation gaps * fix(web-shell): remove brittle Desktop wiring probe Keep transcript contract prevalidation at the evidence level it can actually prove. The previous source-text assertion could both reject equivalent formatting and pass unreachable packaging code. - Remove the Desktop script parser and its false behavioral claim - Mark installed-artifact verification as deferred to Desktop smoke tests - Clarify MR1 matrix, CI wiring, and provenance evidence boundaries - Refresh the hash-locked capability matrix fixture Note: This does not change Web Shell or Desktop production behavior. --------- Co-authored-by: heyang.why <heyang.why@alibaba-inc.com>
…wenLM#9468) * test(triage): pin the parent-computed diff and the child copy Update the record-step pins to the RUNNER_TEMP-staged file and add a pin asserting the scrubbed child copies it rather than re-running git. * fix(triage): compute the flake-gate diff before the env -i re-exec The scrubbed (env -i) child cannot read the shallow merge-ref objects: git global safe.directory lives under HOME, which env -i strips, so git refuses to read the base commit and the diff fails with "Could not access <base-oid>". Compute the NUL-delimited diff in the parent (normal environment, no PR code executes) and stage it under RUNNER_TEMP; the clean child copies it into the root-only gate home. * fix(triage): rm -f before the parent diff redirect, slash-path the parent commands * test(triage): pin diff-before-re-exec ordering and slash-pathed commands * fix(triage): harden the flake-record staging path against directory and symlink plants
…dechannel (QwenLM#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 (QwenLM#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 QwenLM#7012 flush-before-guard keeps a discriminating stimulus * fix(sdk): address round-2 sidechannel review for QwenLM#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 e6b40e5 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 QwenLM#7012/QwenLM#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>
) The Actions backend wedged the original qwen-autofix.yml workflow entity on 2026-08-19: runs stick "queued" with zero jobs and cannot be cancelled or deleted via API, schedule ticks stopped being created, and event- triggered runs are dropped. Same-repo control workflows run normally, so the failure is bound to that one workflow entity. A byte-identical copy under a new path registers as a fresh entity and resumes the loop; the original file stays untouched so the revert is deleting this one file.
…kill (QwenLM#9345) * refactor(cli): consolidate shared helpers ahead of the legacy audit skill Move the pieces the upcoming /audit skill needs out of command-group ownership so no skill imports across command groups: - the findings schema moves from commands/review/ to cli/src/utils/ as-is; every review consumer imports it from the new home, and the stale-bundle digest, bundle-asset list, and artifact comment track the move - safeTarget (traversal-safe slug) and tokenizeArgs (quoted argument splitting) lift to cli/src/utils/paths.ts and shell-args.ts, with review's copies re-exporting/redirecting - the two private git check-ignore copies (review test-plan, team memory) consolidate into one fresh-by-default helper in core utils; the memo stays caller-side so a remedy re-check observes the flip Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): restore review ledger machinery dropped by the consolidation The shared-helper consolidation silently reverted three behaviors PR QwenLM#9175 added to compose-review, against the PR's stated "no behavior change" intent: the unreviewed-dimension anchor exemption (scopeUnproven / dimensionGapsAreDepthOnly / isNonDiffDimensionGap), the LEDGER_MAX_ROUND stamp clamp, and the bilingual budget-stop phrase splice. Restore them with the tests that pin them; SKILL.md, ledger.ts, and deadline.ts still document all three. Also harden the new helper tests: - safeTarget: the deep-path fixtures now share a flattened prefix longer than the kept window, so a truncation-only slug (no digest) collides instead of shipping green. - isGitIgnored: each GIT_* scrub arm now carries a discriminating fixture (three arms previously passed with their scrub line deleted), and every foreign git init scrubs ambient repo-placement selectors. * fix(core): scrub git config-injection channels from the ignore probe Ambient GIT_CONFIG_COUNT (inline KEY/VALUE injection) and GIT_CONFIG_GLOBAL/GIT_CONFIG_SYSTEM (config-file redirects) can aim core.excludesFile at a foreign rules file and flip the probe's verdict for the -C worktree — the same leak class the existing selector scrubs close. Measured on the pristine probe: both channels turn a not-ignored path into an ignored one. Also pin the guards the review found unpinned: the two config channels, the `--` separator for dash-leading paths, the timeoutMs wiring, safeTarget's hash-of-original-target property, and tree-existence of the lifted review helpers. * fix(core): scrub pathspec-magic env channels from the ignore probe * fix(core): drop the whole GIT_* env family from the ignore probe Two more leak channels surfaced on the probe's per-variable scrub list: GIT_ICASE_PATHSPECS (the fourth pathspec modifier — ambient, makes check-ignore reject every pathspec with exit 128, which the catch reads as not-ignored) and GIT_CONFIG_PARAMETERS (the inline -c channel git itself uses to propagate config to children — ambient, can aim core.excludesFile at a foreign rules file). Measured through the real function: the first flips a genuinely ignored path to not-ignored, the second flips a not-ignored path to ignored. Since the channel list grew by one leak per review round, drop the whole GIT_* family instead of enumerating, and close the system config tier explicitly (GIT_CONFIG_NOSYSTEM=1) so host policy in /etc/gitconfig can no longer answer for the -C worktree — that ambient dependency also made the config-redirect arm red on any host whose system config matches the probe path. Pin the icase member, the PARAMETERS channel, the default 5 s deadline (previously unpinned), and add a lower timing bound to the caller-deadline arm so it cannot pass vacuously when the shim is not executable. * fix(cli): make the safeTarget slug space prefix-free at the dash boundary Review's cleanup sweeps .qwen/tmp/ by qwen-review-<slug>- prefix. A slug that itself carried '-' — natively (pr-6771 vs pr) or via the truncation join — could extend a shorter slug, letting one target's cleanup delete a DISTINCT target's artifacts (R8-7). The truncation branch this PR carries newly lands deep targets inside the cap instead of dying ENAMETOOLONG, which turned the latent collision live. Drop '-' from the slug alphabet entirely (dashes flatten like separators) and join the truncation digest with '_': with '-' out of every slug, the qwen-review-<slug>- boundary is unambiguous by construction — no slug can start with another slug plus '-'. prev-ledger side files keep their hardcoded dashed name on both writer and reader, untouched by the slug. Tests pin the prefix-free property (short-vs-short, short-vs-truncated) and the fixture names follow the new slugs. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * revert(cli): restore safeTarget byte-identical to the pre-lift behavior Reverts the slug alphabet change (5394952) and drops the deep-target truncation branch, per review round findings R12-1 and R8-7: - R12-1: flattening dashes renamed only one side of review's two-sided artifact-naming contract — bundled-skill templates, composed names, prev-ledger and brief/report producers hardcode the dash spelling, so the bypass-audit tripwire would silently skip and the cleanup sweep would leak ~15 dash-form artifacts per review. - R8-7: the prefix-sweep hazard only exists because of the truncation branch this lift carried; main's safeTarget has no truncation, so a behavior-preserving lift must not add it. Deep-target support belongs in a follow-up paired with the sweep-side structural fix it needs. safeTarget is now byte-identical to the pre-lift implementation (moved, not modified); tests pin the dash spelling and the leading-strip rule as they behave on main. Full review suite green (5022). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci@service.alibaba.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
…enLM#9479) Since ~2026-08-19 00:00 UTC the workflow's schedule, issue_comment, and pull_request triggers stopped creating runs while pull_request_review kept working; dispatch runs were created but never expanded into jobs (three sat queued with zero jobs for 4-11 hours). The on: block was unchanged throughout and a disable/enable cycle did not restore dispatch, consistent with a stale trigger registration on the Actions backend. Any content change forces a re-parse; this commit is that change (one comment line).
…reaches (QwenLM#9416) * fix(review): keep the blocker in a COMMENT body every softening path reaches `compose-review` renders the body copy of an unanchorable blocker on a COMMENT only when that COMMENT stands where a REQUEST_CHANGES would have been — the body copy is its ONLY copy, and softening the event must never erase it. The condition listing which softenings qualify was written as an enumeration of the two flags known at the time: if (downgradedFrom === 'Request changes' || criticalsUnverified) A third path shipped past it. The findings-file `— [unverified]` tag softens a Request changes at the event line, and it sets neither flag: not `downgradedFrom` (only the presubmit carve-out sets that) and not `criticalsUnverified` (only the verification-delivery gate sets that). So a run whose coverage was PROVEN and whose verifier ran posted a 239-character body — the opener, the tag disclosure, the footer — with the blocker nowhere in it, while the verdict line and the artifact both counted it. The condition is derived now rather than enumerated: `baseEvent` is the row before every cap and downgrade, so `baseEvent === 'REQUEST_CHANGES' && event === 'COMMENT'` asks the question the clause is actually about, and answers it for softening paths that do not exist yet. Same closure the module applied to the deferral channel after QwenLM#9095. The regression test builds the isolating shape — covered plan, verifier on record, findings file still carrying a tag — and asserts both flags the old condition read are unset while the blocker rides the body. Reverting to the enumeration reddens it; so does dropping the clause. * fix(review): keep the presubmit downgrade reasons on every softened Request changes * fix(review): keep the presubmit downgrade reasons on every softened Approve
…LM#9476) * feat(web-shell): Consume live-state session activity timestamps * qwen: address PR review feedback (QwenLM#9476)
…enLM#9473) The two-author certification bar — a CLI-built prompt delivered verbatim, plus the agent demonstrably opening its brief, the diff, or the findings list its prompt named — was re-implemented in four places: the coverage walk, the layer-audit gate, retirement, and the resume recovery command. Re-implementing a bar means drifting from it, and each copy had drifted in its own direction. Extract the atoms into `lib/certification.ts` — `chunkOfKey`, `declaresOwnUncoverable`, `openedBrief`, `readBrief`, `readFindingsPointer` — and have coverage, layer-audit-gate, retirement and `recover-findings` compose the same functions. No behavior change: the atoms are the exact predicates the live pipeline already ran, now spelled once so a future edit to the bar reaches every consumer.
…LM#8855) * feat(cli): show loaded context files alongside the first prompt When context files (QWEN.md / context.fileName) are attached to the system prompt, surface a one-shot INFO line above the user's first prompt listing exactly which files were loaded, so users can verify discovery (e.g., catch typos in context.fileName) without digging into debug logs. Also shorten display paths for files under the user home to `~/...` in both the announcement and the /context detail breakdown. Fixes QwenLM#5267 * fix(cli): address review feedback on context file visibility - Resolve memory-marker paths against the session working directory instead of process.cwd() in /context detail (ACP/daemon sessions) - Sanitize display paths with stripAnsiAndControl before they reach the terminal - Drop empty context files from the announced list to match concatenateInstructions' empty-content filter - Delegate home-dir shortening to tildeifyPath (with an optional home override for tests) instead of a second prefix-check implementation - Align the one-shot announcement guard with downstream input classification (trim, /btw, shell mode) - Add tests for the announcement latch, parseMemoryFiles ~ shortening, CWD-under-home and home-prefix-collision cases; use shared-volume fixtures so tests hold on Windows * fix(cli): address second-round review feedback on context file visibility - Skip the announcement latch for blank submissions (dropped by the message queue before reaching the model) - Treat Windows cross-drive relative results (absolute paths) as outside the CWD tree so home-dir files still get `~` shortening - Extract the hasAttachedContent predicate shared by concatenateInstructions and contextFilePaths so "displayed = attached" holds by construction - Thread the loader's resolved userHomePath through formatContextFileDisplayPath into tildeifyPath so discovery and display agree on the home directory - Pin ordering (announcement precedes submission admission), add a whitespace-only-file filter test, exercise workingDir != cwd in the /context detail test, and cover the /directory add reload wiring * fix(cli): address third-round review feedback on context file visibility - Consume the one-shot latch on model-invocable slash commands: skills and MCP prompts are expanded into a submit_prompt that reaches the model, so excluding all slash commands deferred the announcement to a later plain prompt (or hid it entirely in skill-only sessions). - Build contextFilePaths from every attached file, not just memory-named ones, so the announcement matches what concatenateInstructions injects (extension context files with custom basenames were attached but unannounced while /context detail listed them). - Correct the contextFilePaths JSDoc: entries are display paths (CWD-relative or ~/... shortcuts), not paths to resolve against the CWD. - Tests: model-invocable skill first turn consumes the latch; shell-mode submissions do not; extension files with custom basenames are announced. * fix(cli): address fourth-round review feedback on context file visibility - Replace the order-dependent shell-mode test with a pure predicate (consumesContextAnnouncementLatch) and hermetic unit tests: the old test failed deterministically when run in isolation because the React harness state toggle depended on earlier tests' side effects. - Keep latch consumption correct while shell mode is active for slash commands, which route before the shell intercept: model-invocable ones (skills, MCP prompts) still consume it. - Consume the latch only when something was actually announced, so files attached later in the session still get their one-shot notice. - Skip latch consumption for queued (deferUntilIdle) submissions, which are admitted only after the queue drains. - Re-arm the latch on same-process session switches (/clear) via an effect keyed on the session id. - Classify CWD containment with isSubpath instead of startsWith('..'), which misclassified in-tree directories named like '..cfg'. - Document the deliberate exclusions: baseline rules are not announced (see ruleCount), and the Windows cross-drive arm is consciously untested on POSIX CI. * fix(cli): address fifth-round review feedback on context file visibility - Revert the deferUntilIdle exclusion (R4-8): it silently broke the announcement for queue-only submissions, which never pass back through handleFinalSubmit once the drain admits them. The latch is now consumed at queue time; emitting at the drain admission choke point is a deeper refactor deferred for this feature. - Re-arm the latch on Ctrl-L clear-screen, which wipes the emitted INFO item without a session switch (completes the screen-clear case R4-13 named). - Soften the predicate docstring and the guard comment to be honest about the heuristic: btw is deliberately exempt (a side question that doesn't advance the main conversation, even though it may fork a model call), not "bypassing the model"; consumption is a prediction, not an admission guarantee. - Correct the ANSI-stripping test comment to the real mechanism (stripVTControlCharacters matches the ESC[2Jb…BEL run as one BEL-terminated sequence), so a future maintainer doesn't misdiagnose. * fix(cli): re-arm context-file announcement latch on conversation rewind - Rewind (/rewind / double-Esc) filters history to before the target turn via loadHistory, wiping the emitted INFO item without a session switch. Add a conditional re-arm in handleRewindConfirm: the latch re-arms iff the rewound history no longer contains the announcement, so rewinding past it re-announces on the next prompt while rewinding to a later turn doesn't duplicate it. Completes the wipe-path coverage alongside /clear (session id effect) and Ctrl-L (handleClearScreen). * fix(cli): correct rewind latch polarity and cover Ctrl-L re-arm - R6-1 (Critical): the rewind latch re-arm shipped inverted. Latch semantics are true=consumed / false=armed; the correct assignment is truncatedUi.some(announcement) — true (stay consumed) when the INFO survives the rewind, false (re-arm) when it was filtered out. The negation inverted both branches: rewinding past the announcement left the latch consumed (no re-announce), rewinding to a later turn armed it (duplicate). Drop the negation. - R6-2: add 're-arms the latch after Ctrl-L wipes the INFO' — submit, handleClearScreen, submit again, assert two announcements. Locks the R5-8 Ctrl-L re-arm; removing that line now fails the suite. * fix(cli): address R8 review on context-file announcement - AppContainer.tsx: performMemoryRefresh anchors on getWorkingDir() not process.cwd(), matching the /context read site (R1) and ACP/daemon under skipProcessChdir (R8-5). - commandUtils.ts: docstring drops the false "MCP prompts" claim (McpPromptLoader has no modelInvocable field) and the inline comment names the disableModelInvocation / description-less exemptions instead of a false exclusivity (R8-4, R8-7). - AppContainer.test.tsx: renderRewindHarness takes optional history / contextFilePaths and gains two rewind tests — past the announcement re-arms (re-announces), retaining it stays consumed (no dup); each fails under its !some/some mutation, covering the R6-1 inversion (R8-1). - directoryCommand.test.tsx: strengthen the loadServerHierarchicalMemory assertion to pin the getWorkingDir() anchor (R8-6). R8-2 (defensive reset) and R8-9 (/cd plumbing) declined: the reset is on a path that can't hold stale values (safe-mode early return); /cd needs cross-file plumbing, same class as the deferred /restore — both are subsumed by a self-healing latch follow-up. * fix(cli): address R9 review on context-file announcement - commandUtils.ts: narrow the btw exemption to /btw only. ?btw is not a slash command and goes to the main model as a plain query, so it must consume the latch. Updated 2 tests that pinned the old ?btw exemption (R9-2). - AppContainer.tsx: document the /cd latch gap in the consume-point comment — relocateWorkingDirectory swaps the file set without re-arming, deferred to the self-healing latch follow-up (R8-9). - AppContainer.test.tsx: add performMemoryRefresh anchor test that mocks loadHierarchicalGeminiMemory, captures the callback from useGeminiStream mock args, and asserts the first arg is config.getWorkingDir() (not process.cwd()) and setContextFilePaths received the loader's paths (R9-1). * fix(cli): address R10 review — shared constant, project-local marker test, latch-consuming rewind test, gap comment fix - Extract CONTEXT_FILES_ANNOUNCEMENT_PREFIX constant + isContextFilesAnnouncement predicate in commandUtils.ts; emission site, rewind matcher, and all test fixtures/helpers now reference them, eliminating exact-spelling coupling - Add project-local marker test (QWEN.md, docs/QWEN.md) with getWorkingDir != process.cwd() to kill anchor-divergence mutation - Rewind re-arm test now submits before rewinding to consume the latch, asserting 2 total announcements so the deletion mutation is killed - Correct consume-point comment and add /directory add + performMemoryRefresh to the known-gap list alongside /cd * fix(cli): widen isContextFilesAnnouncement param type to string for tsc --build HistoryItem union includes variants whose type is not in the MessageType enum (e.g. 'about', 'stats'), so { type: MessageType } is structurally incompatible. Widen to { type: string } so the predicate accepts all HistoryItem variants; the runtime check (=== MessageType.INFO) is unchanged. * fix(cli): address R11 review — stale mock params, type-check test, ordering assertion, positional pinning - Correct loadServerHierarchicalMemory mock parameter list in config.test.ts to match real signature (extensionContextFilePaths is slot 4, not slot 5) - Add 3 unit tests for isContextFilesAnnouncement type discriminant (non-INFO item with prefix must not match) - Replace invocationCallOrder[0] with findIndex-based assertion to isolate the announcement's own call index - Pin folderTrust slot to true in directoryCommand.test.tsx - Pin extensionContextFilePaths and contextRuleExcludes with distinct sentinels in performMemoryRefresh test to catch same-typed swap * fix(cli): add /resume to known-gap list in latch comment /resume of the current session wipes UI history (announcement INFO not persisted) without re-arming the latch — same class as /cd, /directory add, and performMemoryRefresh. Documented as a known gap; self-healing latch follow-up covers all centrally. * fix(cli): reconcile announcement latch on any history replacement Wrap loadHistory with latch reconciliation: after rewind, /restore, or same-id /resume, the latch is set from whether the restored history contains a context-files announcement. Fixes the /restore duplicate announcement and the same-id /resume missing-announcement gap; replaces the rewind path's explicit hunk with the same shape. Also document the intentional fileCount vs contextFilePaths criteria difference in the loader, and fix a pre-existing test bug: the remount-only refresh test's mock destructuring was missing the history parameter, so it never actually captured refreshStatic. * fix(cli): route interactive /resume through latch wrapper and sanitize markers useResumeCommand now accepts an optional loadHistory override; AppContainer passes the latch-reconciling wrapper so interactive /resume (including same-id) reconciles the latch instead of leaving it consumed with no announcement in the rebuilt history. The wrapper also destructures loadHistory so its useCallback deps hold the stable function reference rather than the per-mutation historyManager identity, keeping history out of commandContext's rebuild path. concatenateInstructions sanitizes the marker displayPath with stripAnsiAndControl: newline/control characters in directory names could previously forge or hide entries in the /context parser, contradicting the sanitized announcement surface. Documented that contextFilePaths lists top-level files only (@import content is inlined into its importer). Added tests for the sessionId re-arm effect, the startup-resume armed latch, and the resume override. * test(cli): stub initialize in sessionId re-arm test to stop unhandled rejection --------- Co-authored-by: 俊良 <zzj542558@alibaba-inc.com>
…ms (QwenLM#9445) * feat(review): add runtime-axis, table-sweep and isolation witness forms The witness rule's forms all answer "what does this input produce": probe runs a unit harness, base-tree runs one input on two trees, extract-step executes a lifted run: body, the sweep iterates a population. Three common claims fall outside that shape. A forward-compatibility claim is unfalsifiable on the one runtime the harness happens to be running, and a green matrix is only evidence about the versions in it. base-tree's version axis is git; this adds the runtime axis, capped at one other version and priced at a tarball rather than an install and a build. A hardcoded table mirroring another system's namespace looks like data rather than logic, so it gets checked by reading it against a list somebody retypes — which is the mirrored oracle the sweep already rejects, wearing a disguise. Parsing the literal out of the source and diffing it against the authority at runtime makes both directions visible, and the direction that ships is the one no test written against the table can see. A claim about an aggregate invites a per-component dump, after which the verdict rests on code the review itself wrote. Removing one contributor and re-reading leaves both numbers coming out of unmodified code, and settles sum-versus-maximum in a single pair of readings. None of the three needs a new CLI surface: the witness field already carries free text, so the witness rule's enumeration of forms is the only other line that had to move. * fix(review): teach the three new witness forms to the verify brief Round 1 review, R1-1: the Step 4 verifier never reads SKILL.md — its whole instruction set is the brief agent-briefs.ts builds — and every pre-existing form in the witness rule's enumeration has a capability paragraph there, including the impact sweep (phrased "sweep the real population", which is why a search for the SKILL.md name missed it). The three new forms had none, so the only agent that produces witnesses could not reach them. They now sit next to their kin: the table sweep after the population sweep, the version axis after the base-tree A/B, elimination after the drive block, and all three in the brief's own witness-form list. Also from round 1: - The subtraction rule was wrong for the aggregate the worked example actually is. A difference is the removed contributor's value only under a sum; under a maximum, removing a non-holder moves nothing and removing the holder exposes the next-largest, so the stated rule would have had a verifier quote 0.2 MB as a child's heap. Both files now read the pair for the combining rule, and name reducing the population to one contributor as what yields a value outright (R1-10). - Whether a silent contributor is skipped or counted as zero is invisible under a sum and a maximum alike; it shows only where a zero moves the figure. Scoped to counts, denominators and averages (R1-11). - The two-child reading is quoted the same way in both files now, and no longer claims to prove more than it does: flat under addition excludes a sum, not every alternative (R1-8, R1-9). - Node 22 exposes eight old-generation names, not ten; the ten-name set is the classifier's, and the two Node 24 adds are named (R1-7). - One name for the capability, "version axis", matching its defining paragraph and the DESIGN.md heading (R1-3), and the witness rule no longer points below for a form that is above it (R1-2). - The enumeration trap is conditional on an unbounded entrance space, and most of these tables are bounded — the lens's own carve-out (R1-5). - "Installs the other version" against "no install and no build" read as a contradiction; the cost is a download, no dependency install (R1-6). - The one-version cap and "every version the repo supports" contradicted each other. Both now say: the versions the claim names, which for a support range is the floor and the newest (R1-4). --------- Co-authored-by: wenshao <nigolaschao777@gmail.com>
…QwenLM#9517) GitHub does not start runs for a workflow file larger than 500 KB (512,000 bytes) and reports nothing when it stops. qwen-autofix.yml crossed that line on 2026-08-19 at 512,782 bytes: schedule ticks stopped firing, every workflow_dispatch sat "queued" forever with zero jobs and could not be cancelled, and issues/issue_comment went quiet — while pull_request_review runs kept succeeding, because a PR event resolves the workflow from the PR's own branch and those carry older, smaller copies of this file. The loop therefore looked half-alive and stayed dark for a day. Move 75 long comment blocks (1,326 lines) verbatim into a sibling design record, .github/workflows/qwen-autofix.md, leaving each block's opening lines plus a `qwen-autofix.md#af-NNN` pointer where it sat: 518,055 -> 426,437 bytes. No executable line changes — the YAML parses to an identical document outside `run:`, every `run:` script still passes `bash -n`, and the only lines removed anywhere are comments. Steps that are duplicated verbatim across jobs share one pointer so they stay byte-identical. Add .github/scripts/check-workflow-size.sh (gate at 470,000 bytes), wired into CI on every profile: a .github-only PR classifies as `github_ci_only` and skips the `full`-only checks, which is exactly the PR that can trip this. Tests pin the gate, every workflow's size, and pointer/section symmetry. Delete qwen-autofix-recovery.yml. It was cloned during the incident on the theory that the workflow ENTITY was wedged, but it carried the same oversized file, so its dispatches queued identically and its schedule never fired.
* feat(cli): add agent view supervisor runtime * feat(cli): add agent view pty workers * fix(cli): harden agent view supervisor runtime * fix(cli): harden agent view pty workers * fix(cli): surface supervisor startup failures * fix(cli): handle supervisor subscription failures * fix(cli): address review feedback on agent-view supervisor (QwenLM#7799) - Narrow AgentViewSupervisorSubscriptionOptions to omit timeoutMs, which the subscription never reads - Throw when DEV=true with a .ts entrypoint but tsx is missing, instead of silently spawning a process that will crash - Add test for the qwen fallback path when argv[1] is undefined - Add test for worker sideband auth bypass (workerEvent without token) * fix(cli): address review feedback on agent-view supervisor (QwenLM#7799) * fix(cli): address review feedback on agent-view supervisor (QwenLM#7799) * fix(cli): address review feedback on agent-view supervisor (QwenLM#7799) * fix(cli): bind streaming supervisor handlers and keep attach stream 8-bit clean (QwenLM#7799) * fix(cli): pause attach socket during leftover write and harden error handlers (QwenLM#7799) * fix(cli): harden supervisor shutdown tests and clean up signal listeners (QwenLM#7799) Pass the auth token to expectSupervisorUnreachable so it distinguishes a live auth-gated supervisor from a closed socket, remove SIGTERM/SIGINT listeners on the non-signal shutdown path, exercise prepareSocketPath's mkdir via a nested socket path in the permission test, and add coverage for the 1 MB request-line size guard. * fix(cli): guard fire-and-forget promises and add supervisor timeout test (QwenLM#7799) * fix(cli): normalize socket path test prefix and force credential file mode (QwenLM#7799) * fix(cli): address agent view runtime review feedback * test(cli): cover agent view bridge edge cases * test(core): isolate worktree symlink traversal fixture * Revert "test(core): isolate worktree symlink traversal fixture" This reverts commit 09f6d44. * fix(cli): harden agent view supervisor store, auth, and streaming (QwenLM#7799) * fix(cli): harden agent view store joins, writes, and coverage (QwenLM#7799) * fix(cli): harden agent view credential writes and subscription handshake (QwenLM#7799) * fix(cli): harden agent view store normalization, roster safety, and bridge backpressure (QwenLM#7799) * fix(cli): guard response serialization in supervisor server (QwenLM#7799) * fix(cli): harden agent view pty host teardown * fix(cli): fail fast on pty host auth rejection * test(cli): cover pty host remote exit polling * fix(cli): address agent view pty worker review nits * fix(cli): harden pty host socket fallback * fix(cli): harden agent view pty workers * fix(cli): harden agent view pty workers * test(cli): cover pty host spawn contract * fix(cli): guard agent view pty host socket takeover and races * fix(cli): fit agent view pty logs under wire cap, strip host token after merge * fix(cli): harden pty host fallback selection * fix(cli): address agent view worker review blockers * fix(cli): bound attach lease ttl and surface pty host input/listen failures * fix(cli): serialize pty host socket ownership and keep attach transport byte-transparent * fix(cli): escalate pty host shutdown and harden socket path fallback * fix(cli): harden win32 kill, decoder reset, and exit tracker - kill/shutdown pass no signal on Windows (WindowsTerminal.kill throws for any signal string; argument-less kill terminates conpty tree) - input StringDecoder is reset at attach boundaries so incomplete UTF-8 bytes from a prior session cannot leak into the next - createRemoteExitTracker requires two consecutive probe failures before resolving exited, preventing false death on a single transient timeout - defaultSpawnPtyHost pairs detached with windowsHide: true - stringifySandbox object branch test, shutdownHost else-branch test, win32 kill guard test, decoder reset test * fix(cli): skip POSIX signal tests on Windows and reclaim empty lock files R9-1: 'passes kill signals' and 'gracefully shuts down with SIGTERM' assert POSIX signal pass-through, but the R8-36 fix made Windows pass undefined. Skip both on win32 to prevent merge-queue CI regression. R9-2: A zero-byte lock (process killed between writeFile open and pid write) yielded parseInt('') = NaN, which failed Number.isInteger and never entered the reclaim branch. Flip the condition so non-integer/empty locks are also treated as stale. * fix(cli): remove abort listener on delay timeout path R10-1: delay() added an abort listener with { once: true } but only removed it when abort fired. When the timer resolved first, the listener stayed registered on the shared AbortSignal. Extract onAbort into a named function and removeEventListener on the timeout path. * fix(cli): don't optimistically resolve exited on SIGINT for remote handles wenshao's local verification (probe P19) showed that kill('SIGINT') on a reconnected handle (child === undefined) resolves exitTracker immediately, even though the worker can trap SIGINT and survive. Only resolve for terminal signals (SIGKILL, SIGTERM, default); let createRemoteExitTracker's status poller observe whether the worker actually exits after SIGINT. * test(cli): add SIGINT carve-out test for remote handle kill Exercise the 'allowedSignal !== SIGINT' guard: connect a childless handle, kill('SIGINT'), assert exited does not settle within 100ms, then kill('SIGTERM') and assert it resolves. Mutation-verified: deleting the guard makes this test fail. * fix(cli): Only resolve exited on SIGKILL for remote handle kill SIGINT/SIGTERM can be trapped and survived; the server-side kill op has no SIGKILL escalation, so optimistically resolving exited for them could report a still-alive worker as dead. Only SIGKILL is untrappable and guarantees the worker is gone, so resolve immediately for it alone and defer other signals to the remote exit poller. * fix(cli): harden agent view PTY host socket lock and kill default The pid lockfile reclaim path could double-acquire: two concurrent reclaimers of a stale lock could both proceed (the later rm deleting the rival's fresh lock), and a rival could reclaim a live writer's lockfile during the O_EXCL create-then-write window — letting two hosts race through prepare→listen and orphan the displaced one. Re-read before removing a stale lock, only release a lock that still contains this pid, and re-verify ownership after listen, failing closed with EADDRINUSE when displaced so at most one host serves the socket. Also pin the signal-less kill op to SIGTERM instead of node-pty's POSIX SIGHUP fallback, which bypasses ALLOWED_KILL_SIGNALS and is commonly ignored. * fix(cli): fix agent view PTY host dispose signal, ring trim, and lock loop dispose() used node-pty's signal-less kill, which falls back to SIGHUP on POSIX and is ignored by nohup-style workers; align it with shutdown() by passing SIGTERM. BoundedOutputRing.trim() ran the leading-continuation-byte re-sync unconditionally, discarding leading bytes (and inflating droppedBytes) when the window never overflowed; gate it on a size trim actually running. The socket lock loop's attempt bound made a successful reclaim on the final iteration throw EADDRINUSE instead of retrying the freed lockfile; loop until the create wins or a live holder is confirmed. Also consume the required-but-unused ACTIVE_CWD sideband field as the state report's cwd fallback, which cannot throw ENOENT like process.cwd(). * fix(cli): settle remote pty host exit only after the RPC lands * fix(cli): optional-call shutdown in the lost-RPC regression test * fix(cli): wait for PTY host endpoint shutdown --------- Co-authored-by: 俊良 <zzj542558@alibaba-inc.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.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> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: yiliang114 <effortyiliang@gmail.com>
* fix(dingtalk): attach media from quoted messages * fix(dingtalk): keep the reply text when attaching quoted media R1-1: `attachMedia`'s placeholder cleanup was written for the direct-media path, where `extractContent` generates `(audio)` / `(video)` / `(file: name)` itself. This PR made the quoted-media path reach it, and there `envelope.text` is the user's own reply — so a reply reading exactly like one of those placeholders was blanked and the agent got an attachment with no prompt. A group `@Bot (audio)` arrives here as exactly `(audio)`, the mention having been stripped upstream. `attachMedia` now takes the placeholder to erase as a parameter; only the direct-media call site passes one. R1-2: the same path newly routes text-only replies through the unguarded `mkdirSync`/`writeFileSync`/`basename` block. Those are synchronous throw sites — ENOSPC on a write of up to 50 MB, ENAMETOOLONG from a quoted fileName over 255 bytes (`basename` does not truncate), a TypeError from a truthy non-string fileName. An escape rejects `processMessage`, whose catch sends the generic error reply and never calls `handleInbound`; the msgId is already in `seenMessages`, so DingTalk's retry is deduped and the prompt is lost for good. The block now degrades the way a failed download already does: log, skip the attachment, deliver the text. This also covers the pre-existing direct-media path. Verified: dingtalk 310/310. Both mutation-checked — restoring the caller-blind cleanup fails 3 tests, letting the fs block throw fails 1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(dingtalk): address quoted-media review findings Unify the msgType->mediaType mapping in a shared helper, make extractContent the single source of truth for the placeholder text cleaned on attach, and remove the store directory when a media write fails so failed stores no longer leak into tmpdir. Merge the stacked attachMedia JSDoc blocks, document quoted media downloads, and pin the previously uncovered paths: unmapped quoted msgTypes with a downloadCode, own-media + quoted-media combinations, direct placeholder cleaning, and the degraded-store attachment shapes. * fix(dingtalk): file-back a quoted image colliding with the own image * fix(dingtalk): give generated media store names a mime-derived extension (QwenLM#9347) --------- Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot[bot]@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
* feat(core): expose workflow execution state * fix(cli): initialize workflow snapshot state * fix(core): preserve workflow client gating * fix(core): snapshot workflows before journal drain * fix(core): sanitize workflow replay logs * fix(core): preserve workflow failure trace integrity * fix(core): validate workflow snapshot run IDs * test(core): cover workflow event boundaries * fix(core): isolate workflow run callbacks * fix(core): align workflow log projections and dependency tails (QwenLM#9034) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): keep persisted workflow projections in agreement (QwenLM#9034) --------- Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…le (QwenLM#9484) * refactor(review): route the certification path atoms through one needle `openedBrief`, `readBrief` and `readFindingsPointer` each inlined the same two lines — `JSON.stringify(path)` then `args.some(a => a.includes(...))` — which is the single mechanism enforcing the bar's "exact path, not a `${path}.bak` look-alike" guarantee. Three copies is the exact drift class this module was created to remove, turned on the module itself: a fix to the match (a serialization edge, path normalization, a stricter compare) would have to land in three places in lockstep or leave one atom silently weaker than its siblings. Extract a private `namesPath(args, path)` and have all three route through it. No behavior change — the existing `.bak`-trap pins in `certification.test.ts` and `check-coverage.test.ts` cover each atom and pass unchanged. * refactor(review): finish the needle consolidation, and keep its name its own Two findings on the first round, both about the helper this PR introduced. The consolidation stopped one copy short. `parseTranscript` still inlined the same exact-path predicate for the diff-read half of the bar, so the count went from four copies to two — while the new comment claimed a fix to the match reaches all of them at once. The match now lives in `transcripts.ts`, beside the code that serializes the args, and both halves call it: the diff-read half directly, the three brief/findings atoms through a wrapper that spreads it over a record's call list. `certification.ts` already imported from `transcripts.ts`, so this needs no layering inversion. The helper also took the name of a module-private `namesPath` in `utils/findings.ts` whose semantics are deliberately different — boundary matching over PROSE, which credits `rm /plan/chunk-3.brief.md` for naming the brief. Unifying the two the obvious way would make `openedBrief` credit an agent for deleting a file it never opened. They keep distinct names now (`serializedArgsNamePath`, `argsNameExactPath`), and both doc comments say why, so a grep for "the path matcher" cannot land a reader on the wrong one. Tests: the needle's own cases (whole JSON string value, no `.bak` sibling, no shell mention) and the `rm <brief>` shape at the `openedBrief` atom. Relaxing the needle to a bare substring now reddens BOTH suites from one edit — the property the consolidation exists to buy, and the one the second copy denied. * test(review): pin the shared needle's two untested arms, restore parseTranscript's doc Three review Suggestions on the needle consolidation: - `serializedArgsNamePath` had been inserted between `parseTranscript`'s JSDoc and its declaration, orphaning the doc onto the wrong function. Move it above the JSDoc so each doc sits with its own function. - The diff-read half (`parseTranscript` with a `diffPath`) was exercised by no test — every `readTranscripts` call site omits `diffPath` and nothing set `diffToolCalls` non-zero, so an arg swap at the call site shipped green. Pin it: the exact diff read counted, a `.bak` sibling and a shell command that only names the diff refused, and `diffToolCalls: 0` without a `diffPath`. - `argsNameExactPath`'s existential (`some`) was only ever run on 0/1-element arrays, so a first-element-only regression shipped green. Pin a match in the second position for `openedBrief` and `readBrief`. * test(review): pin the diff read's RANGE, not only its count The new diff-read fixture asserts `diffToolCalls`, which a mutation dropping the `range: namedTheDiff ? rangeOf(args) : null` wiring survives — the count stays right while every chunk-coverage ruling, which reads the lines rather than the tally, is handed an empty list. The fixture's exact read now carries an `offset`/`limit` and the test asserts `diffReads` equals `[[1, 40]]`; dropping the wiring reddens it.
…wenLM#9532) * chore(ci): Add --provenance to npm publish and id-token permission * test(scripts): expect --provenance in npm publish step assertion PR QwenLM#9532 adds --provenance to every npm publish in the release pipeline. Update the workflow-pinning test to match the new command so the helper test suite stays green.
…tainer (QwenLM#9556) (QwenLM#9723) * feat(review): run the reviewed repository's own commands behind a container (QwenLM#9556) A review executes the code it is reviewing. `build-test` runs whatever the reviewed repository's `package.json` names — `npm ci` with its `preinstall` and `postinstall` scripts, the build, the suite — and `test-efficacy` runs that suite again once per baseline, control, mutant, hunk probe and revert. Both did it as the invoking identity, and both handed the PR's code `process.env` entire: on CI that carries `OPENAI_API_KEY` and `GH_TOKEN`. Reading them is one line in a `postinstall`, and it needs none of the git-config machinery the pipeline's threat findings are built on. The boundary goes around the executions, not around the review agent. Wrapping the agent was tried first and is the wrong shape: its secrets do not survive the container's env allowlist, its `timeout` reaps the host-side client rather than the container, its CLI version stops matching the runner's — and after all of that the mount is the whole checkout, so `<repo>/.git` stays writable anyway. Three decisions the argv encodes, each measured rather than assumed: - **The mount is the review temp dir, not the tree the command runs in.** The dependency farm links OUT of every tree — `exposeDependencies` points each package in the probe tree's `node_modules` at the review worktree's copy, 1 722 of them on a live CI review. Mounting one tree would leave every link dangling. Every tree the pipeline builds is a sibling under `.qwen/tmp`, so one mount covers both ends while `<repo>/.git` stays outside it. - **The environment is an allowlist**, not the inherited one. - **The network is per command kind.** An install needs the registry; a build and a suite do not, and `--network none` keeps loopback so a suite that stands up a local fixture server still runs. One ephemeral container per command. A long-lived one per phase would save about 1–2% of the 540-second efficacy budget and would re-introduce exactly the cross-run state QwenLM#9221 spent rounds closing. Off by default: containerising a build by surprise changes what native modules compile against. `review.sandbox` is `off` | `auto` | `required`, read through `operatorReviewSettings` — which skips the workspace scope, so a repository cannot ship a `.qwen/settings.json` that switches off the containment existing to contain it. `QWEN_REVIEW_SANDBOX` outranks it so CI can require containment without depending on a settings file the runner may not carry. Each of the three decisions is pinned by a test that goes red when that decision alone is reverted. * chore(review): regenerate settings.schema.json for review.sandbox The generated JSON Schema is checked in and CI diffs it against a fresh run (`npm run generate:settings-schema`). Adding `review.sandbox` to `settingsSchema.ts` without regenerating left the two out of step, which is what the "settings.schema.json is out of date" gate is for. The second red check, `Post Coverage Comment`, failed at "Download coverage reports artifact" — a consequence of the test job dying before it uploaded one, not an independent failure. * fix(review): make `required` actually refuse, and fix four wiring bugs the first cut shipped Five Criticals from the review, all of them real, and the first two would have made the feature not work at all. - **`required` failed open.** `sandboxVerdict` produced a `refused` verdict and nothing consumed it: both call sites tested `kind !== 'container'` and fell through to the direct spawn with the full environment. Refusal is now decided ONCE, at the top of each phase, before anything executes — which is also the only place that can cover the route that never reaches a spawn: a repo whose toolchain cannot be scoped is handed to the AGENT's own shell, and a gate at the spawn would leave that wide open under the very policy forbidding it. - **`SANDBOX` was a shortcut past the policy.** The first cut returned `direct` when the session was already sandboxed, reasoning that the outer boundary is the one the operator asked for. Wrong for this property: the CLI's own sandbox constrains the filesystem and hands the child `process.env` entire, and stripping the secrets is half of what `required` promises. - **The probe suite baked in the host's Node path.** `process.execPath` does not exist inside the image, so every sandboxed probe would exit 127 and map baseline, control, every mutant, every hunk and the revert to inconclusive — zero evidence exactly when containment is on. It uses the image's `node` now; the vitest path resolves because it lives under the mount. - **The mount root took the first `.qwen/tmp`, not the deepest.** A review run from inside another review's worktree nests them, and the first occurrence widens the mount to the outer temp dir — pulling `<repo>/.git` and every sibling checkout in, which is the one property the mount exists for. - **No UID/GID mapping.** The default image runs as root, so the container's writes into the mounted trees were root-owned and every later host-side cleanup — the install-timeout `rmSync`, `discardWorktree`, the sweeps — hit EACCES, accumulating residue across reviews. Two things the fixes themselves needed, found by checking them rather than by being told: - **Refusing with `toolchain: 'unsupported'` would have caused the regression it was closing.** That value has a documented meaning — the brief reads it as "build-test could not scope this repo, install and build it yourself" — so a refusal routed into it would have sent the agent to run the reviewed code by hand, unsandboxed. It is a distinct `refused` now, with a brief rule that says the evidence is unavailable and must NOT be reconstructed by hand. - **A bare `--user uid:gid` resets `$HOME` to `/`**, which the mapped user cannot write, so npm fails before the install starts — `utils/sandbox.ts` copies the host's `$HOME` for exactly this. The container gets a writable HOME inside the mount, with the npm cache under it. The duplicated mount-root arithmetic became one exported `mountRootFor`, which is both how the copies stopped drifting and how the nested case got a test. Every fix above is pinned by a test that goes red when that fix alone is reverted. * fix(review): stop the reviewed repository from deciding its own containment (QwenLM#9556) Round 2. The sharpest finding is that the guarantee this PR advertises was true of one route and false through another. **A repository could switch off the containment that exists to contain it.** `operatorReviewSettings` skips the workspace scope precisely so a `.qwen/settings.json` cannot set review policy — but the env layer that outranked it is repository-controlled too: `loadEnvironment` walks up from cwd and applies `<repo>/.qwen/.env`, from the very checkout under review, admitted by default because folder trust starts off. `QWEN_REVIEW_SANDBOX=off` in a committed `.env` disabled it. Three siblings were worse, because they have no ordering to fall back on: `QWEN_REVIEW_SANDBOX_IMAGE` chooses the image the reviewed code runs *inside*; `SANDBOX_SET_UID_GID=false` puts the container back to root; `DOCKER_HOST` chooses which daemon answers, so `required` reads as satisfied and whatever that daemon returns is scored as evidence. `environment.ts` gains `isFileSourcedEnvKey`, and containment now reads only the operator's settings or a real process variable. The policy additionally only ever tightens, so even a genuine env value cannot lower a settings `required` — which, as a mutation showed, is what actually protects the policy; the file-source check is what protects the other three. **`required` still failed open where the mount could not be built.** The gate asked "did a runtime answer", never "can this phase be contained": with a healthy daemon and a cwd outside `.qwen/tmp` — a `/review` of a local checkout — the command fell through to the direct spawn with the full environment and a report indistinguishable from a contained run. It asks the second question now. **A refusal on `--resume` destroyed the run it was asked to continue.** Returning a report let the handler's unconditional write overwrite the in-flight one, and the refusal carries no run identity, so every later resume failed the identity check even after the runtime recovered. It throws on a continuation, which is the invariant the `!adapter` branch states in its own words. **The HOME added last round was itself cross-run state.** It lived on the shared mount, and `sh -lc` sources `$HOME/.profile` while npm reads `$HOME/.npmrc` — so one run's postinstall could plant what the next review's install executes, with the network on. That contradicted this module's own `--rm` "isolation by construction" claim, and it arrived with the fix for the `$HOME` problem rather than in the original. HOME is a tmpfs now: discarded with the container, never on the host. The npm cache stays on the mount, and the comment says plainly that npm's integrity check is what stands between a poisoned cache and a bad install. **The mount root was lexical.** `resolve` never touches the filesystem, so a symlink at or above `.qwen/tmp` — committable as mode 120000 — would have widened a read-write bind mount to wherever it pointed. Every other creating or destroying path in this pipeline refuses that; this one does now too. Also: the new tests were platform-fragile in three places (Windows has no `process.getuid`, and the documented `SANDBOX_SET_UID_GID` opt-out could fail them on a developer's box), and the secret-leak check asserted against whatever the runner happened to export rather than a planted canary. Both fixed, and the mount-root tests now build real directories, which is how the symlink refusal got pinned at all. Every fix here is pinned by a test that goes red when that fix alone is reverted — except the policy's file-source check, which a mutation showed is redundant with the tightening rule, and which is documented as defence in depth rather than claimed as load-bearing. * fix(review): close the hand-off arm, and three fixes that were wrong at their edges Round 3. Six Criticals, all of them real, and two were introduced by round 2's own fixes. - **The hand-off arm of the original blocker was still open.** The phase gate refuses when containment is impossible — but a repo the npm adapter cannot scope (yarn/pnpm/bun, no lockfile) reaches `unsupportedReport` with the gate satisfied: a runtime answered and the tree is mountable. That report tells the agent to install and build with its own shell, which nothing here contains. An inapplicable adapter is now a refusal under `required`, not a hand-off. - **The mount-failure refusal fired under `auto` too**, and its message hardcoded `required`. Under `auto` the contract is "contain it when that is possible", so an unmountable tree falls back to the direct spawn — refusing there would have taken build/test and efficacy evidence away from every local review the moment a daemon happened to be running. Mine, from round 2. - **`--workdir` got the lexical path while the mount got the realpath.** Round 2 made `mountRootFor` canonicalise; the workdir did not follow, so on any layout where the two spellings differ — `/var` against `/private/var` is the everyday one — the container was handed a directory it does not have and every command would fail before starting. Also mine, from round 2. - **The daemon scrub missed the indirection selectors.** `DOCKER_CONFIG`, `CONTAINERS_CONF` and friends name a config file that in turn names the daemon, the registries and the runtime: scrubbing the direct selectors and leaving these moves the same steering one level down. - **`isFileSourcedEnvKey` matched case-sensitively.** Windows env lookup is case-insensitive, so a `.env` committed as `docker_host=…` reaches the child exactly as `DOCKER_HOST` would while the exact-case test answers "not from a file" about a value that is. Same class as `sanitizedGitEnv`'s case fold. The gitfile finding stays deferred with its reasoning in the thread — it is not a hole this PR opens, and closing it belongs to the identity gates rather than to the sandbox. Two mutations came back green on the first pass and were the useful part of this round: the `DOCKER_CONFIG` scrub had no test because the fixture pinned one key rather than the set, and the operator's `SANDBOX_SET_UID_GID=false` opt-out had no test at all — both uid tests asserted the flag was PRESENT. The scrub test now asserts the whole set, and the opt-out has its own. * fix(review): the hand-off gate was dead code, and three more edges Round 4. - **Last round's hand-off gate never ran.** It tested `!applicable`, and `applicable` is the filtered adapter ARRAY returned by `selectToolchainAdapter` — never falsy. The gate shipped green and closed nothing. It is judged on the RESULT now (`toolchain === 'unsupported'`), which also covers the second route to a hand-off — an adapter that applies and cannot scope, from inside the npm one — and the predicate is exported and tested rather than living inline where no test could see it. - **The boxed farm dangled under a symlinked ancestor.** Round 3 made the mount and `--workdir` canonical; `exposeDependencies` still built its link targets from the lexical root, so on the everyday macOS `/tmp` → `/private/tmp` layout every farm link resolved to a path the container does not have. The phase would then report "every file was red or collected nothing" — a wiring failure published as a statement about the PR's own suite. Canonicalised on the sandboxed path only; the direct path keeps the caller's spelling. - **A timed-out boxed run leaked its container.** `--rm` fires only on a self-exit, and the deadline kills the runtime CLIENT — so a suite whose own trap ignores the forwarded signal keeps running with the review temp dir writable, past the budget and past the end of the review. Containers get a unique `--name`, and both spawn sites `rm -f` it when the deadline fires. - **The daemon scrub deleted case-sensitively.** Round 3 taught `isFileSourcedEnvKey` to fold case on Windows and left the deletion exact — so a `docker_host` written by a repo `.env` was correctly detected and then not removed. Two mutations came back green again, and both were the round's real lesson: the hand-off refusal had no test (which is how its dead-code predecessor shipped), and the farm canonicalisation still has none — it needs a symlinked-ancestor fixture with a live runtime, which this machine cannot provide, and it is listed with the other integration gaps rather than claimed. * fix(review): put the hand-off conversion at the one exit, and make the reaper reachable Round 5. The first finding is the same one for the third time, and the third time is the one worth explaining: I had been guarding routes rather than the exit. - **Attempt one** tested `!applicable` — the filtered adapter ARRAY, never falsy — and was dead code. - **Attempt two** wrapped the two `adapter.run` returns and missed the `!adapter` branch's own `unsupported` report. - **Now** the conversion sits at the single place a report can reach a caller, and it is an exported `applyHandOffPolicy` rather than a branch inside a long function, so a test can reach it without a live container runtime. Both previous attempts failed the same way — a guard placed where no test could see it, in a function with more exits than the author was holding in mind. **The container reaper added last round was unreachable, not wrong.** `spawnSync` sends its `killSignal` at the deadline and then WAITS for the child to exit, so an attached runtime client that forwards SIGTERM to a workload whose own trap ignores it never returns — and the `killContainer` after it never runs. The boxed spawns use `killSignal: 'SIGKILL'` now: the client cannot ignore it, the call returns, and the container is reaped by name at the daemon, which is where the deadline had to be enforced. **The scrub missed the proxy family.** `HTTP_PROXY`/`HTTPS_PROXY`/`ALL_PROXY`/ `NO_PROXY` (and their lowercase spellings) are honoured by both clients for every daemon call, so a repo-shipped one interposes on the connection the direct selectors were scrubbed to protect. One test was removed rather than added: a `expect(killContainer).toBeTypeOf( 'function')` I wrote to "document" the SIGKILL reasoning. It asserts nothing and would have read as coverage; the reasoning belongs in the comment where it now lives alone. The `killSignal` choice is reasoned but not pinned — it needs a live runtime and a TERM-ignoring workload — and joins the integration gaps already listed in the PR body rather than being claimed. * fix(review): the third continuation exit destroyed the report it was continuing Round 6, two Criticals: the deferred gitfile one, and this. Round 5's single-exit conversion runs on `--resume` answers too. A resumed report whose toolchain is `unsupported`, under a policy that tightened between the first call and the continuation, was replaced by a fresh refusal — which the handler writes unconditionally, over the report the call was asked to continue. That refusal carries no run identity, so every later `--resume` fails the identity check and the round redoes install, build and every suite. "A continuation must never answer with a FRESH report" is enforced by a throw at the refusal gate and at `!adapter`. This conversion was added after both and did not have it. It does now. The trigger is ordinary rather than adversarial: the policy is read per call, so an operator raising it — or a workflow's `env:` — between call one and the resume is enough, on exactly the unscopeable repo shapes (yarn/pnpm/bun) that reach a hand-off at all. **The first test I wrote for this passed without the fix.** It drove `runBuildTest` with an incomplete argument object and asserted `.toThrow()`; the throw it saw was `--timeout must be a finite number of seconds`, from validation long before the code under test. A mutation caught it. Making the fixture faithful means reproducing the whole resume-identity apparatus, so the decision is a predicate instead — `resumeWouldDestroyReport`, tested directly, with the wiring left as one visible line. Same shape as round 5's `applyHandOffPolicy`, and for the same reason: a guard a test cannot reach is a guard that has already been wrong twice here. * fix(review): reap the container on every abnormal exit, not only the timeout Round 7, one Critical. `build-test` reaped the boxed container when `spawnTimedOut(r)`, which is true for ETIMEDOUT and false for a `maxBuffer` overflow — and a reviewed command writing 64 MB to one stream is a postinstall away. The client dies with ENOBUFS, the reap is skipped, and the container keeps the review temp dir mounted read-write past the per-command deadline and past the end of the review. The sibling in `test-efficacy` already reaped on the broader `r.error || r.signal`. That the two had drifted to different conditions is how one came to miss a case the other caught, so they now share one exported predicate: `status === null` — exactly "the client did not exit normally", covering ETIMEDOUT, ENOBUFS and signal kills in one condition rather than a list of causes to keep in sync. A normal exit needs no reaping (`--rm` has fired), and a client that never spawned has no container, where the reap is a silent no-op by `killContainer`'s construction. * fix(review): scrub the container client env by provenance, not by name The scrub list named the daemon selectors, then the proxy family, and still missed DOCKER_API_VERSION — which selects no daemon at all. It just makes every client call fail, and under `sandbox: auto` a failing availability probe reads as "no runtime here", so a repository that ships one value in its .env turns containment off without a word. The class has no last entry. Replace the list with the rule it was trying to approximate: a key the loader wrote from a file the reviewed checkout supplies does not reach the process that decides whether containment happened. The client needs no repository-provided environment for anything, and the workload's environment is an allowlist either way. Deleting restores rather than approximates: the loader records a key as file-sourced only where the real environment had nothing, so a file value never shadows an inherited one and the client keeps the operator's own DOCKER_HOST, PATH and HOME. * fix(review): drop --user on rootless runtimes, and take the base-side rerun into the box Two blockers from round 9. R9-1. `--user uid:gid` is right on a rootful engine and wrong on a rootless one. Rootless podman — its default install — and rootless docker run the engine inside the invoking user's namespace, so container uid 0 IS the invoking host user and every other container uid lands on a subuid near 100000. The availability probe answers fine, the phase gate passes, and then every boxed command meets a tree it does not own: `npm ci` cannot create node_modules, and what the container does create comes out unsweepable — the cross-run residue `--user` exists to prevent. Detect rootlessness from the runtime's own `info` document and drop the flag there, where the container's root is already the invoking user and the flag has nothing left to do. Unknown answers rootful. The two wrong guesses are not symmetric: guessing rootful on a rootless host breaks the run loudly, guessing rootless on a rootful one silently runs reviewed code as real uid 0 on a writable mount. R2-21. test-delta kept a private copy of build-test's run(). The copy was correct until build-test's grew a container, and then the two sides of the measurement stopped being comparable: the PR side in the image with an env allowlist and no network, the base side on the host with both. A test that reads an env var or opens a socket flips on one side, and the file whose only job is to say which side a failure belongs to says "the PR's". The duplicate is deleted rather than re-synchronised, and `required` now refuses the base-side rerun instead of running it on the host. A refusal returns an empty report, so the brief now says in as many words that an empty `netNew` beside such a note is the absence of a measurement, not the absence of a regression. * fix(review): refuse mount roots the -v grammar cannot spell, and stop the suite inheriting the operator's policy R10-1. `-v src:dst` has exactly one separator, so a checkout at `/…/my:repo` produces a spec docker answers with `invalid spec … too many colons`. `mountRootFor` called that root mountable, which bypassed both designed degradations: under `auto` every install/build/test command surfaced a raw mount error the report attributed to the PR instead of falling back to the direct spawn, and under `required` the phase gate passed and the refusal that should have explained it never happened. Classify such a root as unmountable and it rejoins the path every other unmountable root already takes. Measured rather than assumed, including the suggested alternative: `--mount type=bind` fails on the same colon AND on a comma that `-v` takes without complaint, so it would trade one unspellable path for two. On Windows this refuses every absolute path, deliberately: a drive letter is a colon, and a mount whose source and target are the same path cannot be a Windows path at all. Containment is not available there, and saying so is what gives `auto` its fallback and `required` its refusal. Hermeticity, which this PR broke. The phase gates read the operator's own `review.sandbox`, so a maintainer who turns the feature on and runs the suite watched 101 review tests report that setting back at them instead of what they measure. Both routes are closed: the shared setup drops the environment variables, and the two suites whose gates resolve settings isolate `QWEN_HOME` the way they already isolate the host git config — measured at 101 → 0 from each route independently. test-delta's gate is injectable for the same reason its `exec` is. Also from the round-10 list, all self-inflicted: the `--user` test pinned the documented opt-out it asserts against, `runtimeIsRootless`'s unknown case is now structural (an empty document carries no marker) rather than a literal a mutant can flip, and the dead `resetContainerRuntimeProbe`/`force` pair is gone. Two doc corrections: `containerPathFor` names the null it actually produces, and the brief's `refused` bullet covers all three routes that produce it rather than one. * test(review): gate the mount-root cases Windows cannot be asked Round 10 refused every absolute Windows path from `mountRootFor` — a drive letter is a colon — and said so as intended behaviour. It is; what that reply missed is that this file then asserts the opposite. Three cases here turn on a root being MOUNTABLE, which is the one question Windows has no answer to: the comma control added last round, the deepest-temp-dir selection, and the symlink refusal's honest half. All three assert non-null against real absolute paths, and the colon case also builds `my:repo`, a component name Win32 rejects outright. `test_windows` is merge_group-only and a required check, so nothing here would have gone red until the merge queue itself. Gated off win32 with the convention this directory already uses in 37 places, and the shipped semantics pinned by a win32-only case rather than left to the comment that describes them. * test(review): make the Windows pin actually reach the check it pins The win32 case added last round named a path that does not exist, so `mountRootFor` returned null out of its realpath catch and the assertion held whether or not the drive-letter check was there at all — probe-verified: the layout is absent and the answer is null anyway. It pinned nothing. Build the layout first and assert it exists, so the catch is closed and the colon is the only thing left that can produce the null. Two more from the same list. The uid cases returned early instead of skipping, which reports PASSED with zero assertions on the lane where the condition bites — the reading "this held" for something never checked. And the install-network assertion looked only for a bare `none`, blind to the joined `--network=none` a refactor could switch to; it now rejects the flag in either shape. * test(review): skip the second uid case instead of returning, and restore its stub in finally The sibling got `it.skipIf` last round and this one did not, so on a lane where `getuid` is undefined it still reports PASSED having asserted nothing — which reads as "this held" for something never checked. Its `SANDBOX_SET_UID_GID` stub was also cleared on the last line rather than in a `finally`. There is no `unstubEnvs` in the vitest config and no file-level hook that would catch it, so a failing assertion left the stub set for whatever ran next — one red test quietly becoming two. * test(review): drop the comment last round's edit left stranded Wrapping the uid case in try/finally moved its opt-out note above the stub and left the original copy inside the try, so the same three lines now appear twice a few lines apart. Delete the stranded one. * test(review): pin the four decisions this feature is sold on Four cells the suite reached only by accident of the machine it ran on, each carrying a live mutant on a property the PR description states as a guarantee. All four functions already took their ambient dependency as an injectable parameter, so this is assertions, not seams. - sandboxPolicy: strictest-wins in BOTH directions, and a file-sourced value counting for nothing. "A repository cannot switch off the containment that exists to contain it" was described in a comment and asserted nowhere; env-overrides-settings, a looser strictest, and a dropped file-sourced guard all shipped green. - sandboxVerdict: `auto` with nothing answering runs DIRECTLY. Every other case here either has a runtime or is `required`, so a mutant refusing instead — turning `auto` into `required` on every machine without docker — survived. - refuseUnsandboxedPhase: the PASS path. Every other assertion about this gate is a refusal, so unconditional refusal under `required` — every review on a perfectly good host — survived. - containerPathFor: it feeds `--workdir` at both spawn sites, and had no test at all. A lexical spelling names a directory the container does not have; the parent fallback is what lets a probe tree be named before it is built. Mutation-checked, one at a time: seven mutants, seven reds. * fix(review): read the policy setting the way an operator writes it `"Required"` in settings.json — or a stray trailing space — matched no policy, resolved to `off`, and disabled the containment the operator had just asked for. Silently. The environment value was already normalised; the settings value was not, and the asymmetry fell on the wrong side: settings is the documented place to turn this ON, since the environment can only tighten. So the unnormalised half was the half operators actually use, and a fail-open on the one setting whose whole purpose is to fail closed. Reported as a suggestion in rounds 8, 9 and 16 and deferred each time under the critical-only posture. Measured this round: both `"Required"` and `"required "` returned `off`. Also pins two shapes the new tests left open. `sandboxPolicy`'s settings default is the production path every real caller takes, and with it swapped for `{}` the whole settings half stopped being consulted while every assertion — each passing settings explicitly — stayed green; it is now driven through an isolated settings file. And the `SANDBOX_SET_UID_GID` opt-out parses case- and space-insensitively, which nothing asserted. The remaining deferral, a fixture literal pasted twice, is duplication rather than a defect and is left alone. * test(review): finish the policy table instead of one row of it Last round's normalisation fix was asserted only against `required` spellings, which is the shape a mutant keyed on that one value walks straight through — leaving an operator's `"Auto"` resolving to `off`, the same silent downgrade one rung lower. Assert every policy on both sides. Two adjacent cells with it. A garbled ENVIRONMENT value must be dropped on its own rather than taking the operator's setting down with it: the environment is the half a repository can reach, so garbage there must never answer for the half it cannot. And `sandboxPolicy`'s env default is the twin of the settings default pinned last round — every assertion here hands it an env literal, so `env = {}` as the default stops the environment half being read at all and nothing notices. Three mutants, three reds. * test(review): exercise the reap and the runtime probe, and stop leaking a fixture Three of the four recorded this round; the fourth is duplication, not a defect, and is left alone for the third time. `killContainer` is the whole answer to a container that outlived the client's deadline, and nothing anywhere ran it — a garbled argv, or a dropped `-f`, shipped green, and what survives is a container holding the review tree open past the end of the run. It now takes its spawn as a parameter, so the argv and the swallow-and-continue contract are both asserted. `containerRuntime`'s probe had the same hole with a memo and a real daemon call in the way. The decision is now separated from both: `firstAnsweringRuntime` is order and nothing else, which is the content — a client installed but not running must never shadow one that is. And the `containerPathFor` case left a temp tree behind on every run. In a change about not leaving residue behind, that one is just embarrassing.
) * feat(core): add a per-project outside-repo artifact landing `/audit` (landing in follow-ups) writes reports, sidecars, and intermediates that quote the audited module verbatim. When the audited repository's ignore state cannot keep those out of version control, they have to land somewhere the repository can never commit — this is that landing: per user and per project under QWEN_HOME, 0700, so quoted (possibly exploitable) module content stays private. Adoption is validated rather than assumed. The path is fully predictable — the project hash is a pure function of the root — and 0700 does not exclude the user's own other processes, so an existing leaf is not evidence this tool created it. A leaf planted as a symlink is refused, and a permissive mode is tightened, since mkdirSync's mode only applies to directories it creates. The contents are validated too, because artifacts land BELOW the leaf and an O_NOFOLLOW open only guards the final component: a symlink child is a complete escape (mkdirSync treats a symlink-to-directory as the directory, so everything written "inside" goes wherever it points) while the leaf keeps passing every check on itself. A hardlinked file is the same story through O_TRUNC. Both are refused. The landing is reused across runs — the report and its sidecar are the durable artifacts — so a merely non-empty landing stays adoptable. * fix(core): validate every component of the fallback landing path The leaf was validated but the path walked to reach it was not, which left the whole landing relocatable by a single symlink. mkdirSync(recursive) follows symlinks in every component ABOVE the final one, and lstat refuses to follow only the final one — so a leaf-only check cannot see a redirected parent. Planting `audits` needs one `ln -s` and no race, because ~/.qwen exists long before `audits` does: the leaf is then created inside the planter's directory, reports as a perfectly real directory, passes the contents check, and every artifact written "into the landing" lands wherever the link points. Probed against the built module: before, the landing resolved inside the planted directory and a written report appeared there; after, the call refuses by name and the planter's directory stays empty. Each component this method creates is now created non-recursively and adopted only after its own lstat — recursive creation is precisely what walks a planted component unchecked. QWEN_HOME itself stays unvalidated: it is the user's own configured location, not a path this method invents. Also closes the unknown-dirent-type gap in the contents check: on a filesystem that does not report entry types, isSymbolicLink() and isFile() both answer false, so an entry slipped past every arm; those entries now get an explicit lstat. * fix(core): close audit landing adoption escapes (QwenLM#9776) Adoption now normalizes the full mode — a 0300 plant kept owner write and blinded the content check while writes still succeeded — and an unlistable landing fails closed instead of skipping validation. The landing is refused before creation when QWEN_HOME resolves inside the audited repository, a missing QWEN_HOME base is created instead of throwing ENOENT, and the mutating getter is renamed to ensureAuditFallbackDir so the filesystem side effect is visible. * fix(core): close audit landing race seams with re-validation (QwenLM#9776) * fix(core): harden audit fallback validation per review (QwenLM#9776) * fix(core): re-check landing contents in pre-return re-validation (QwenLM#9776) * fix(core): drive landing content check from fresh per-entry lstat (QwenLM#9776) The content check decided typed dirents' entry type from the readdir snapshot while consulting a fresh lstat only for nlink, so a same-UID swap of a listed entry for a symlink between the snapshot and the loop passed every arm and returned a landing holding a symlink child. Lstat every entry once and drive the symlink, directory, special-file, and hardlink arms from that single fresh stat — the same single-stat shape the unknown-dirent-type path already used, so the two paths no longer disagree about what "validated" means. * fix(core): re-lstat audit landing after follow-based validation steps (QwenLM#9776) * fix(core): fold case in audit landing containment on case-insensitive platforms (QwenLM#9776) --------- 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@service.alibaba.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
* fix(web-shell): reduce streaming thought render jank * test(web-shell): expand historical question result * fix(web-shell): address streaming review findings * test(web-shell): strengthen streaming review follow-up tests * test(web-shell): pin structural snapshot opt-in and matched insight path (QwenLM#9914) --------- Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com>
…9794) * feat(review): report findings to clients as a typed contract Add a report_findings core tool: one {level, findings[]} call whose field names and enum spellings match the review findings artifact, so /review hands live clients (TUI, Web Shell transcript, ACP hosts) the findings as data instead of a Markdown restatement. The tool sorts by severity/confidence/location, derives shortSummary (<= 60 chars) for compact list UIs, and refuses duplicate ids, control characters, and a partial outcome set - after --fix the skill re-issues the call with a fixed/skipped/no_change_needed outcome per finding, mirroring the artifact's own --outcomes completeness rule, and the rule stays live for later in-session disposition changes. The finding enums now live in core; packages/cli/utils/findings.ts re-exports them under its historical names (the Web Shell renderer keeps its deliberate browser-side copy). The TUI renders the new findings_list display as per-finding rows with severity color, id, file:line, confidence marker and outcome badge; the daemon TUI adapter passes it through and history/recording compaction truncates only the free-text fields. * fix(i18n): cover report_findings in tool display-name maps The two CI drift gates caught what the feature commit missed: the CLI requires a zh translation for every core tool display name, and the web-shell requires a display-name entry (and its own zh translation) for every core wire tool name. Add toolDisplayName.ReportFindings to the zh/zh-TW/ca/en locales, report_findings to the web-shell TOOL_DISPLAY_NAMES map, and toolName.report_findings to the web-shell zh strings. * test(cli): pin ToolMessage routing for findings_list displays Stage-2 review observation: FindingsDisplay had direct render tests, but nothing pinned the ToolMessage discriminator, so removing the routing branch kept every test green while findings fell through to the JSON-string fallback. The new case asserts the joined file:line row and the low-confidence marker, which the fallback never produces; verified by mutation (disabling the branch turns the test red). * fix(review): address automatic-review round 1 on the findings contract Two behavior fixes: compressFindingSummary backs its hard cut off a surrogate pair instead of emitting an unpaired high surrogate, and sortReportedFindings now matches the artifact's sortFindings exactly (code-unit file/id comparison, missing line ranked first) as its doc comment already claimed. SKILL.md Step 6 gains the bounded-contract rule: the tool refuses over-cap calls whole, so an artifact past 50 findings reports the most-severe 50 with the cut disclosed, and over-cap prose is shortened rather than dropped. The rest closes the mutation gaps the review demonstrated: fixtures where shortSummary differs from summary (pinning that rows render the compact label), the summary blank-guard case, exact-value assertions for shortSummary derivation and the word-boundary cut, per-field control-character coverage, line/id passthrough, and the artifact-order tiebreaks (missing line first, id by code units under a stable sort). Every new assertion was mutation-verified: each documented mutant now turns at least one test red. * test(core): pin code-unit file/id sort order in report_findings * fix(review): close report_findings contract gaps from review round 5 (QwenLM#9794) - Align the tool's `file` cap with the artifact path domain (PATH_MAX, 4096) and refuse line numbers outside JavaScript's safe integer range. - Refuse `outcome: "skipped"` without a non-empty `outcomeNote`, in the tool and in the `review findings --outcomes` ledger that feeds it. - Hold an outcome re-report to the active report's identity — same ids, none dropped, none added — so a partial fix run cannot silently shorten the client's list. - Render the report-level `level: "low"` state in FindingsDisplay and sanitize every interpolated row value to one terminal-safe line. - Validate the full findings_list shape at the daemon TUI boundary and fall back to plain text for malformed payloads instead of crashing. - SKILL.md: extend the sanctioned over-cap shortening to `outcomeNote` on the Step 6B re-report; the artifact keeps the full-length text. * fix(review): implement report_findings replacement semantics and close round 6 contract gaps (QwenLM#9794) * fix(cli): close findings_list boundary bypass and restore superseded reports on rewind (QwenLM#9794) --------- 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-code-dev-bot <qwen-code-dev@service.alibaba.com>
…nLM#9871) * fix(ci): neutralize legacy ##[ commands in autofix stdout echoes A review finding about workflow-command injection carried its payload strings (::error::forged, ##[add-matcher]) verbatim as example text. Prepare echoed feedback.md raw, the runner parsed the mid-line ##[add-matcher] and failed the step before the agent ran, and the consecutive-failure breaker burned the takeover window of QwenLM#9761 in about 70 minutes while reporting it as a 100/100 round cap. The existing neutralization covered only the modern :: syntax; the runner also parses the legacy ##[name] form, even mid-line. Extend every untrusted-content echo in the autofix family (prepare feedback echo, both artifact dump loops, the gate's failure/handoff/no-action echoes, the deferred-findings dumps and upsert re-emit loops) to neutralize both prefixes, and pin the census plus a behavioral ##[ case in the contract tests. * fix(ci): unify the autofix neutralizer on one canonical spelling Review feedback on QwenLM#9871: the two upsert re-emit loops neutralized with a hybrid spelling (bash expansion for :: plus a single-syntax sed for ##[) while the other fourteen sites used the canonical two-expression sed, establishing two implementations of one normalization. A future edit fixing one spelling could miss the other and recreate the single-syntax half-guard that let QwenLM#9761 through. Unify both loops on the canonical sed and consolidate the pinned test shapes. The census test's comment also promised more than its assertions deliver: it pins the known enumeration, it does not auto-detect new un-neutralized echoes. Reword it to state the actual guarantee, ban the ##[-only sed and the bare bash expansion alongside the existing ::-only ban, and fold the script-side re-emit loop into the census. * test(ci): assert the neutralizer pairing property instead of banning shapes The three shape bans only matched the no--e spellings, so the canonical line's natural half-copies (sed -e 's/::/;;/g' alone, or the ##[-only -e mirror) passed every ban while guarding only one syntax. Assert the pairing property instead: every occurrence of either substitution expression must belong to the canonical two-expression pair, so a half-guard in ANY spelling unbalances the count and fails the census — verified by probe against all four half-guard shapes. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…nLM#9873) * fix(core): refuse a workflow script before it can spend anything `WorkflowRunner.start` minted a runId, opened a journal and registered the run before a single byte was parsed; the first compile happened downstream inside the sandbox. One TypeScript annotation therefore produced a fully registered, failed run — a phantom row in `/workflows`, a snapshot on disk and a telemetry event, for a workflow that never began. Compile first. `compileWorkflowScript` is exported from the sandbox and called by `start()` before anything is minted; the sandbox's own `run()` now calls the same function rather than a lookalike, because a gate that compiled even slightly differently from the run would wave through scripts that then fail after registration, which is the failure the gate exists to prevent. A refusal throws `WorkflowScriptNotLaunchedError`, which the tool turns into a not-launched result rather than a workflow failure: there is no runId to hand back, and the model should re-send a corrected script rather than go looking for a run that does not exist. The message points at the author's own line. V8 already renders the offending source line with a caret under the exact column and puts it at the head of the stack, so reuse that instead of reconstructing a caret from column numbers, and correct the one thing it gets wrong here: the IIFE wrapper shifts every body line by one, so V8's line 2 is the script's line 1. Long lines are windowed with the caret shifted to match. The hint that follows names the usual cause, which is TypeScript syntax. The wrapper also gains `'use strict'`, sharing its line with the arrow so the line offset stays at one. In sloppy mode an undeclared assignment in model-authored code silently created a property on the sandbox global instead of throwing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MDuRWMhKbqEYZXbbLhn3en * fix(core): preserve workflow compile line numbers * fix(core): bound workflow compile error lines * fix(core): preserve workflow compile error frames * fix(core): clarify incomplete workflow syntax --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com>
…nce signal (QwenLM#9903) (QwenLM#9938) * feat(review): engage the severity floor early on a sustained convergence signal (QwenLM#9903) The convergence diagnosis prints the remedy — drop to a critical posting floor — whenever the first-time-finding rate stops falling, but the floor itself engaged on a fixed round-6 schedule, so rounds 3-5 kept posting Suggestions inline at full cost while re-deriving the same root-cause cluster. Record the consecutive not-falling rounds in a new ledger field (flatRounds); at two — the shortest window in which "not falling" is an observation — engage the floor on the firing round, ahead of schedule, latched for the rest of the loop, and disclosed in the posted body with the streak that armed it. The streak rides the churn streak's trust group (foreign markers stripped at the recovery seam, planted values clamped to the rounds the PR actually ran); an explicit suggestion floor still overrides the latch and an unknowable round still fails open. Fixes QwenLM#9903 * fix(review): gate the early-floor streak measurement on the auto posture (QwenLM#9903) * docs(review): align the early-floor docs with the shipped behavior (QwenLM#9903) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(serve): repair persisted session lifecycle Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: fix CI failure on PR QwenLM#9626 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: fix CI failure on PR QwenLM#9626 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (QwenLM#9626) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (QwenLM#9626) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (QwenLM#9626) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (QwenLM#9626) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (QwenLM#9626) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (QwenLM#9626) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (QwenLM#9626) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (QwenLM#9626) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (QwenLM#9626) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (QwenLM#9626) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (QwenLM#9626) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (QwenLM#9626) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (QwenLM#9626) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
… read (QwenLM#9753) * fix(cli): degrade gracefully when messageRewrite.promptFile cannot be read * docs(cli): document rewrite prompt fallback
Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com>
…fety comment (QwenLM#8861) * test(core): pin transport retry diagnostics and correct the replay-safety comment Follow-up to the QwenLM#7938 maintainer verification, addressing both non-blocking findings. The comments justifying the thinking-phase replay claimed thought parts are never recorded in history. That is not the invariant: the successful attempt's thoughts are recorded. What makes the replay safe is that the failed attempt's accumulated partial turn is discarded wholesale before the retry (popPendingPartialAssistantTurn) and thought parts are never user-visible content. Both comment sites now state that. The two retry diagnostics were unpinned: hardcoding yieldedNonContentChunks on the scheduled log or relabeling the skipped_after_content decision on the not-taken log survived the suite. The scheduled-log field is now asserted in the thinking-only replay test, and a new test covers the path on current main that still emits skipped_after_content — a cut after a delivered functionCall, where the replay gate and the continuation gate are both closed. Each mutant now fails exactly one test. * test(core): close the review's four gaps on the retry diagnostics Address the inline review on the follow-up: - Correct the last surviving copy of the stale replay-safety rationale (the thinking-phase test's header comment still claimed thoughts never enter history). - Hoist socketCut/cutAfter out of the continuation suite and reuse them in the function-call cut test instead of a character-for-character inline copy, so the transport error shape has a single producer. - Pin the false side of yieldedNonContentChunks in the tool-preparation retry test, and the 'exhausted' arm of retryDecision in the budget exhaustion test. Hardcoding the diagnostic true or collapsing the ternary now each fail exactly one test. * test(core): unify socket-cut fixtures and pin retry exhaustion with thinking (QwenLM#8861) * docs(core): ground thinking-replay safety in the partial-turn discard (QwenLM#8861) * fix(core): ground replay rationale and pin retry-decision ternary (QwenLM#8861) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: ComplexSimply <rudy.arrowsong@gmail.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…Tauri upgrade bridge (QwenLM#9085) OpenWork (modelstudioai/openwork) has forked the Electron desktop code and is self-contained now, so retire the Electron package and its release/sync machinery from this repo: - Delete packages/desktop (Electron app, live-host app, bun workspace). - Retire scripts/desktop-openwork-sync.ts and the desktop-openwork-sync root script; the OpenWork sync is no longer needed. - Retire .github/workflows/live-host.yml, live-host-release.yml and sync-live-host-to-oss.yml; live-host releases now live in OpenWork. The CLI-side packages/cli/src/serve/live code stays for now (separate cleanup). - Retire scripts/check-voice-guard-sync.js (cli<->desktop parity only) and its CI step. - Clean up remaining references: root package.json workspaces negation and package-lock.json, eslint/prettier/yamllint ignores, architecture docs, web-shell skill descriptions, and review-lib workspace fixtures/comments (renamed to point at packages/desktop-shell, the remaining negation). Deliberately kept: the Electron->Tauri upgrade bridge — desktop-release.yml (incl. the electron_bridge input), create-electron-bridge-manifest.mjs, sync-desktop-to-oss.yml (mirrors Tauri desktop-shell artifacts only) and everything under packages/desktop-shell.
…M#9840) * fix(goal): resume an evidence-limited Goal from a fresh window A Goal stopped at one of the evidence bounds was the only Goal state the reducer refused to resume: `/goal resume` threw, and the sole way forward was to edit or replace the objective — which mints a new revision and, for a replace, a new Goal. An unattended run therefore ended for good, and the user was told to rewrite the objective they had not changed their mind about. Resume now restarts the evidence window instead. The objective, the revision and the accumulated turn count carry over; the cursor moves to the resume boundary and the checkpoint is dropped. That is the same reset `/goal edit` already performs, minus the parts a resuming user did not ask for. Keeping the exhausted window would only exhaust it again on the next turn, which is why the refusal existed at all. Dropping it has a real cost and it belongs to the user who asked to resume: evidence recorded before the resume is no longer citable, so a terminal proposal must prove itself from what the resumed run produces. Only the enumerated evidence bounds reset the window — a `usage_limited` Goal stopped by a transient operational failure keeps its cursor and checkpoint, since nothing was wrong with them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(web-shell): offer resume for evidence-limited goals * refactor(goal): retire the client evidence-limit mirror, pin the resume cursor The previous commit made `canResumeGoal` status-only, which left the whole client-side evidence-limit apparatus -- `isGoalEvidenceLimited`, the `GOAL_EVIDENCE_LIMIT_REASONS` sentinel mirror, and the cross-package drift guard that kept the mirror honest -- consumed by nothing but its own tests. Delete it instead of documenting it: the reducer resumes an evidence-limited Goal by restarting its window, so no client decision hangs on telling those stops apart, and a sentinel copy with no consumer is pure drift surface. The component tests keep one legacy-prose case each, with the sentinel inlined as inert fixture text. Also close the review findings on the same review round: - mappers.test.ts justified carrying `limitKind` with the old refusal story; restate the pin as what it actually guards (a field-by-field mapper rebuild silently narrowing the record). - The fake goal journal answered `getTranscriptCursor` with a fixed `{ recordId: null }`, which made the resume-cursor assertion pass trivially -- and null is the one value the real evidence pipeline hard-rejects (`cursor_unset`). It now answers with the transcript tail, as the real journal does, and the test asserts the resumed cursor IS that tail. Mutation probe: reverting the fake to `{ recordId: null }` fails exactly the resume-cursor test (108 others green). --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(core): allow scoped Git credential config * fix(core): handle Git config URL false positives * docs(core): clarify Git client safety invariant * fix(core): reject unsafe Git extension inputs * docs(core): reattach cloneFromGit JSDoc and note blocklist mirror (QwenLM#9870) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): inherit wrapper environment for restricted Git clients (QwenLM#9870) The restricted child environment replaced the spawn environment wholesale with a small allowlist, so a PATH-resolved git wrapper that needs a non-allowlisted indirection variable failed every spawned command ("exec: : not found"), breaking both the unit suite and real credentialed installs in such environments. Inherit the parent environment minus Git behavior variables (git_*), home locations, proxies, TLS trust overrides, loader-injection keys, and the non-GIT_* keys @simple-git/argv-parser flags as unsafe. This keeps wrapper indirection variables flowing while ambient Git configuration, program overrides, and network redirection still never reach the spawned client. Adds a regression test that spawns git through a PATH wrapper requiring a non-allowlisted variable. * fix(core): reject ambiguous credentialed Git sources and block ambient tokens (QwenLM#9870) * fix(core): preserve Git credential isolation --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* feat(channels): add DingTalk Workspace channel
Add a DingTalk Workspace (DWS) channel package so a workspace can be
driven from DingTalk alongside the existing channels.
- packages/channels/dws: new workspace holding the DWS client, event
stream, environment resolution and channel implementation, with the
event-source fixtures used by its tests.
- cli: register DWS in the channel registry and its builtin list.
- web-shell: recognise the DWS platform in the channels UI.
- docs: document the channel and its configuration under
docs/users/features/channels.
- build/release: include the new workspace in the build, clean and
release-version scripts and the vitest project list.
The channel watches native DingTalk todos, routes document and todo
replies back to their originating conversation, bounds notification
retries, and keeps sender identity authoritative for direct messages.
* fix(dws): classify spawn-resource errnos as not sent, and test against base's source
Round 1 review, two Critical findings.
vitest.config.ts — the new package's config was the only channel config
without the `@qwen-code/channel-base` → source alias its five siblings carry,
so `cd packages/channels/dws && npx vitest run` (the workflow AGENTS.md
prescribes) depended on a prior `tsc --build` of base. Reproduced the
reviewer's witness in this worktree with base/dist moved aside: without the
alias vitest dies in `packageEntryFailure` and runs zero tests; with it,
63/63 pass. Even when dist exists it may lag base's source — it did here, by
four days.
dws-client.ts — `DWS_NOT_SENT_ERROR_CODES` listed only the path errnos, so a
`dws` process that never started because of fd or memory exhaustion
(`EMFILE`/`ENFILE`/`ENOMEM`/`EAGAIN` and family) was classified `unknown`.
The todo and document reply paths in dws-channel.ts swallow `unknown` as
"the originating task will not be rerun", so a user's final reply was dropped
permanently on one log line instead of being retried — and the retry is safe,
since the fingerprint is not persisted when delivery fails. The set now
carries the whole `uv_spawn` pre-exec family. Everything else the callback
reports — a non-zero exit (numeric `code`), a timeout kill (`code === null`),
`ABORT_ERR`, a `maxBuffer` overrun — happened with a child already running
and stays `unknown`, because a retry there could duplicate a delivery.
The classification moved into an exported `classifyDwsCommandFailure` so the
table can be driven directly: the resource errnos need real fd or memory
exhaustion to reproduce through a spawn, which no unit test can stage safely.
The existing missing-executable test still covers the wiring end to end.
Verified: packages/channels/dws — 191 passed (5 files). Mutation-verified:
reverting the errno set turns exactly the 12 added codes red (12 failed /
51 passed); dropping the vitest alias with base/dist absent turns the suite
from 63 passed into a collection failure. eslint and prettier clean. The one
tsc error on this branch (`displayText` missing from `Envelope`) is worktree
build skew — base/dist was built 2026-08-10, base/src changed 2026-08-14, and
the field is present in the source; it reproduces identically with these
changes stashed.
* fix(dws): stop a denied sender from consuming a document comment's dedup slot
Round-2 review, R2-4 (Critical).
`notificationKey` is `documentNotificationKey(documentId, commentKey)` — no
sender in it — so a `'denied'` outcome falling into the `else` branch marked
that (document, comment) pair processed for good. Every later notification for
the same comment, live or polled, then hit
`processedMessages.includes(notificationKey)` and returned silently, including
one from a sender who IS allowed. The cursor persists, so the drop survived
restarts.
Concretely, with `senderPolicy: 'allowlist'` and `allowedUsers: ['open-bob']`:
Alice (not allowlisted) @-mentions the bot in a document comment and is denied;
Bob then mentions the bot on the same comment thread — the ordinary
multi-reviewer document flow — and is dropped forever, with no dispatch, no
pairing and no log.
A denied notification is now parked with `rememberPendingDocumentNotification`
like a `'pairing'` one rather than consuming the slot. Replay already skips a
pending entry whose sender fails `gate.isAllowed`, so a denied sender does not
get retried in; and an allowed sender reaching the same comment clears the
entry on the way through.
The existing `applies sender access policy to document mention notifications`
cannot cover this — its denied and allowed notifications are on DIFFERENT
comments, so the shared key is never exercised. New test puts both on the same
comment. Mutation-checked: restoring the old condition reddens it with
`bridge.prompt` called 0 times against an expected 1, reproducing the review's
own witness.
Verification: `npm run build` and `tsc --noEmit` clean in packages/channels/dws;
eslint clean on both changed files; full package suite 192/192 (118 in
dws-channel.test.ts, 1 new).
* fix(dws): stop a poison message, a full pending queue, and an unreachable
replay from pinning the watermark (R2-1, R2-2, R2-4 queue)
Three ways history polling could stall forever, each measured:
**R2-2, poison message.** A message whose turn threw was never marked
processed, so the watermark never advanced and every poll re-ran it as a
full agent turn — one model call per iteration, no cap, no backoff —
while the pinned watermark grew the query window without bound and the
throw starved every newer message behind it. Pending-document replay
already had retry accounting; this path had none. Inbound failures are
now counted per message and persisted in the cursor: under budget the
error still propagates (redelivery retry and the concurrent-duplicate
contract depend on that, and their tests pin it), and once the budget is
spent the message is marked processed and dropped with a logged reason.
**Pending-queue cap.** `rememberPendingDocumentNotification` threw at
MAX_PROCESSED_ITEMS, and the throw aborted the direct-message loop
before the checkpoint, the watermark and `markProcessedMessage` — so
every later poll re-scanned a growing window and re-threw on the same
never-marked message, surviving restarts in the cursor. The queue's only
drain is an allowed sender later processing the same comment, so entries
parked for unapproved senders never leave: one unpaired member
@-mentioning the bot in 5,000 distinct comments broke document history
polling until manual cursor surgery. It now evicts the oldest instead,
which costs at most a pairing prompt nobody approved.
**R2-1, the replay the fixture could not recover.** The test fake
ignored its `startTime`/`endTime`, so it certified a recovery the
production arithmetic cannot perform. Fixed on both sides: the fake now
filters by its window like the real client (and `message()` defaults
`eventTime` to now, since real messages always carry one — six fixtures
were silently relying on epoch 0), and the stale-replay guard now pulls
`notificationWatermark` back to the parked notification's event time. It
parks document notifications UNMARKED on purpose, "for polling to
recover"; on a fresh cursor the watermark started at
`connectionStartedAt` and the window opened at `watermark − 5s` —
exactly the guard's own drop boundary — so everything it parked was
strictly outside every window that watermark would ever produce.
Every fix is mutation-verified: reverting the retry budget re-runs the
poison turn once per poll (8 polls, 8 turns), restoring the queue throw
reproduces the reviewer's stderr and the pinned watermark, and dropping
the watermark pull-back leaves the replayed notification unrecovered.
Suite 194/194 green; `tsc -p packages/channels/dws` clean.
R1-2 (self-identity degradation) is not in this commit — both fixes the
review proposes collide with behaviour this suite pins deliberately; see
the thread.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(dws): budget every inbound surface, and stop restarting a dead source (R4-1, R4-3)
R4-1: round 3 added an inbound failure budget, but wired it into one of the
three `handleInbound` call sites — the mention/live-IM path. The other two kept
the exact unbounded-retry mode the budget's own doc comment says it exists to
close.
- Document notifications (`processDocumentNotification`): a throw escapes
`pollOnce`'s sorted loop and is swallowed by the outer catch, so nothing is
marked processed and `notificationCheckpoint`/`notificationWatermark` — both
assigned after the loop — never advance. Every 5s poll re-ran the same full
agent turn, forever, starving every newer notification behind it.
- Native todos (`pollTodos`): the fingerprint is remembered only on success, so
a todo whose turn keeps throwing was re-fetched and re-run every poll,
forever.
`recordInboundFailure` now takes the drop action as a parameter, because "stop
re-running this" differs per surface: marking the key processed is right for a
message, a document notification carries its own `notificationKey` (and a
pending entry to clear), and a todo is re-fetched by fingerprint. The default
keeps the mention path byte-identical.
R4-3: `retryable: false` is terminal before ready — `retryLimit` returns 0 —
but `scheduleImRestart` never consulted it, and `startImSource` resets
`restartAttempts` to 0 every time a subscription becomes ready. The backoff
exponent therefore stayed at 0, so a permanently denied consumer (permission
revoked, subscription not allowed) was respawned at a constant ~3s forever —
one `dws event consume` child every 2-3s per affected source — while the
channel reported itself connected and delivered nothing for that source.
Post-ready now matches pre-ready: terminal, with a log line saying so.
Verification (`cd packages/channels/dws`):
- `npx vitest run` — 197 passed (was 194; three new tests).
- `npx tsc -p tsconfig.json --noEmit` — clean.
- Mutation checks, one per fix, each turning exactly its own test red and
leaving the other 122 green:
- drop the `retryable === false` guard -> `stops restarting a source that
died permanently after becoming ready` fails.
- drop the document-path budget -> `drops a document notification whose turn
keeps failing, and stops starving newer ones` fails (the newer
notification is never reached).
- drop the todo-path budget -> `drops a native todo whose turn keeps
failing` fails (8 turns instead of 5).
- eslint + prettier clean.
Not addressed in this commit: R4-2 (checkpoint drain overwriting the stale
replay pull-back), R4-4, R1-2, and R4-5..R4-8.
* fix(dws): stop an in-flight poll from clobbering the stale-replay pullback (R4-4)
`handleImMessage` leaves a replayed document notification UNMARKED on purpose,
for history polling to pick up, and pulls `notificationWatermark` back to the
replay's `eventTime` so a future window can reach it. `pollOnce` then wrote
`checkpoint.endTime` over that watermark unconditionally when its own window
finished — and `checkpoint.endTime` is always past the replay's `eventTime`.
The race is not hairline: `runLoop` polls immediately on connect and the IM
subscriptions start before the poll loop, so a startup replay arrives precisely
while poll #1's `listDirectMessages` is awaiting. One clobber puts the parked
replay outside every window the watermark will ever produce — no turn, no log,
no error, and it survives restarts because `saveCursor()` persists it.
`pollOnce` now records whether the watermark was pulled back while its
direct-message fetch was in flight, and on that path drops the window instead of
finishing it: neither the advance nor the paginated checkpoint resume is safe,
because the checkpoint was itself derived from the pre-pullback watermark. The
next poll re-derives a window from the pulled-back value.
Test: `keeps the stale-replay pullback when a poll was already in flight` emits
the replay from inside `listDirectMessages`. Mutation-checked — forcing the
guard false reddens it with `inbound` empty, matching the reviewer's witness
(`dispatched = 0`). It also asserts the second query window opens at or before
the replay's `eventTime`, so a fake that ignored its window could not certify it.
* fix(dws): stop three silent, permanent losses of a document mention (R6-1/R6-2/R6-3)
All three Criticals round 6 raised share a failure shape: a document comment is
consumed by something that had no right to consume it, the user gets no reply,
and nothing is logged. Each is fixed at the point that consumes the slot.
R6-1 — `handleImMessage` pullback (dws-channel.ts): R4-4 rescued a stale replay
by pulling the notification watermark back, but the flag `pollOnce` consults is
cleared at the top of every fetch, so it only ever covered a replay that landed
DURING one. A pullback arriving in the gap between two polls is reset before it
is read; a persisted multi-page `notificationCheckpoint` then resumes a window
that starts after the replay and finishes by writing `checkpoint.endTime` back
over the pulled-back watermark. The replay was left unmarked on purpose, so
after that no window ever reaches it again. The pullback branch now drops the
checkpoint as well, which makes the rescue durable regardless of when the
replay arrived; the in-flight flag still guards the during-a-fetch case.
R6-2 — in-flight awaiter (dws-channel.ts): a pending entry means the in-flight
turn PARKED the comment for a sender it would not serve, which says nothing
about the caller waiting behind it. Marking unconditionally consumed an ALLOWED
sender's mention outright — replay only re-drives a parked entry whose own
`senderId` passes the gate (the denied one never will), and the allowed
sender's marked message key is skipped by every later history poll. The awaiter
now marks only when the comment is genuinely processed, or when this caller is
no more entitled to it than the sender already parked. This is what the
denied-sender comment further down already claimed happened ("an allowed sender
reaching the same comment clears the entry on the way through") — the awaiter
was the path that never let them reach it.
R6-3 — failure-budget drop closure (dws-channel.ts): the closure marked the
sender-agnostic `notificationKey` (`document\0comment`, no sender), so five
failed turns — about 25s of transient model or bridge trouble, since each 5s
poll re-runs an unmarked notification — dropped every FUTURE mention of that
comment from anyone, permanently and across restarts. It now marks only the
failing message's own `key`, which is what stops the window re-running it, so
the R4-1 starvation this budget closes stays closed.
Tests (dws-channel.test.ts), each mutation-verified against the pre-fix code:
- `keeps a stale-replay pullback that arrives between two polls` — persists a
bounded checkpoint, emits the replay with no poll in flight, asserts the
checkpoint is released and the next window reaches back over the replay.
Reverting R6-1: `expected { startTime: … } to be undefined`.
- `lets an allowed sender through while a denied turn on the same comment is in
flight` — the concurrent counterpart to the existing R2-4 test, which lets
the denied turn finish first and so cannot reach the awaiter. Reverting R6-2:
the allowed sender's prompt is never called.
- `lets a later mention of a dropped comment retry with a fresh budget` — five
failing polls, then a different reviewer on the same comment after the
outage. Reverting R6-3: `expected [] to deeply equal [ ObjectContaining{…} ]`.
Verification: `npx vitest run` in packages/channels/dws — 201 passed (5 files);
`npx tsc --noEmit -p packages/channels/dws/tsconfig.json` clean; `npm run build`
in that package clean; eslint and prettier clean on both changed files.
R1-2 is untouched: it still needs a maintainer call on which pinned contract
gives, and is not something this commit should decide.
* fix(dws): resolve the sender gate before reading a mentioned document (R7-1)
`parseDocumentMentionNotification` reconstructs `(documentId, commentKey)`
from rendered message text, so a bare alidocs URL in an ordinary DM forges a
mention card the channel cannot tell apart from a genuine platform
notification. `processDocumentNotification` then called
`readDocumentContext` on that attacker-named document BEFORE `handleInbound`
resolved the sender gate, so under the documented default
`senderPolicy: 'pairing'` an unpaired stranger could force this profile to
perform an authenticated read of any document it can reach — a turn the
channel would never serve them.
Resolve `gate.isAllowed(message.senderId)` first and read only for a sender
this channel will actually answer. The envelope already carries a "Document
Markdown was unavailable" fallback, the `preflightInbound` document branch
still parks the mention exactly as before, and
`replayPendingDocumentNotifications` re-enters this path once the sender is
approved, so an approved turn still gets its document context — just after
the gate instead of before it.
BEHAVIOR FLIP: `replays a pairing-pending document mention after approval`
pinned `readDocument` being called once for the still-unpaired sender and
twice overall. That pinned expectation was the defect: it asserted an
authenticated read driven by a sender the gate had already refused. It now
expects zero reads before approval and one after. Verified by mutation —
reverting the guard turns both this test and the new forged-mention test red.
Still open on this class and NOT addressed here: the pairing-code write into
the attacker-named comment thread. Closing that needs either fail-closed
verification that `commentKey` is a real comment on `documentId` mentioning
this profile (no DWS CLI surface exposes it — `listMentionedMessages` covers
group IM, not document comments) or structured mention events, so it is a
maintainer contract call rather than a local fix.
Verification:
- packages/channels/dws: 202 passed (5 files), including the new
`does not read a forged document mention before the sender gate resolves`
- tsc --noEmit -p packages/channels/dws/tsconfig.json: clean
- eslint + prettier --check on both changed files: clean
* fix(dws): list the dws channel as a cli test build prerequisite
`channel-registry.ts` dynamically imports `@qwen-code/channel-dws`, whose
package.json resolves the bare specifier to `dist/index.js` and which
`packages/cli/vitest.config.ts` does not alias to source. It therefore
belongs in `DIST_PREREQUISITES['packages/cli']` alongside every other
builtin channel, so a cli test run on an unbuilt checkout reports the
actionable "run npm run build" message instead of a raw resolution error.
This is what the required `Test (ubuntu-latest, Node 22.x)` check caught
on 4bf0407: scripts/tests/vitest-global-setup.test.js asserts the list
stays in sync with the registry, and dws was the one registry import
missing from it.
Verified: `npx vitest run scripts/tests/vitest-global-setup.test.js`
29 passed; reverting this one line reproduces the CI assertion exactly
("missing prerequisite entry for packages/channels/dws"), 1 failed | 28
passed. prettier --check and eslint clean.
* fix(dws): close current review blockers
* test(dws): pin fail-closed self identity gate
* fix(dws): preserve retryable inbound work
* fix(dws): preserve in-flight catch-up mentions
* fix(dws): align channel-base on the workspace version so npm ci resolves
`Dependency CVE audit` has failed every run with:
npm ci can only install packages when your package.json and
package-lock.json are in sync.
Missing: @qwen-code/channel-base@0.21.11 from lock file
The diagnosis of "stale base" was right, but the stale file is this PR's
own. `packages/channels/dws` was written when the workspace was at
0.21.11 and pins that version; every sibling channel — dingtalk, feishu,
github, gitlab, qqbot, telegram, wecom, weixin — now says 0.21.14, which
is what `packages/channels/base` actually publishes. A workspace package
cannot satisfy 0.21.11, so npm resolved `@qwen-code/channel-base` for dws
from the REGISTRY instead of linking the sibling, leaving a nested
`packages/channels/dws/node_modules/@qwen-code/channel-base` entry that
`npm ci` refuses. Merging current main cannot fix it: main is not where
the pin lives.
Bump dws to 0.21.14 for both its own version and its channel-base
dependency, matching every sibling, and regenerate the lockfile. The
nested registry entry is gone and dws now links the workspace like the
others. `npm ci --dry-run` completes, and dws typechecks and passes all
211 tests against the workspace channel-base rather than the published
0.21.11 it was resolving before.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VgTjRF91xANQh6SY9YGyCf
* fix(dws): replay failed direct messages, and close open review items
* fix(deps): bump tar to 7.5.22 to unblock CVE audit (QwenLM#9394)
The 2026-08-21 advisory GHSA-r292-9mhp-454m flags tar <= 7.5.20 as
high severity, failing the Dependency CVE audit gate. Main already
moved to 7.5.22 in QwenLM#9703, but that landed after this branch's last
merge of main. Bump the lockfile entry in-range (core/cli declare
^7.5.19) to match main, and regenerate the committed NOTICES.txt
artifact whose freshness is enforced by CI.
* fix(dws): unblock npm ci, add publish metadata, and keep todo fetch failures out of the turn budget (R13-1, R13-2, R14-1)
* fix(dws): dedup threaded pairing comments on a persisted marker instead of the rotating code (R15-1)
* fix(dws): clear the todo pairing marker when pairing resolves, not on turn success (R16-1)
* fix(dws): address current review blockers
* fix(dws): satisfy event fixture lint
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
* fix(session): preserve source titles when branching Forked sessions could fall back to a UUID fragment when the source name came from its first prompt rather than a custom title. - Forward the active picker name through Web Shell branch requests - Resolve prompt-backed display names and include them in collision scans - Allocate the first free numeric suffix and normalize nested forks - Preserve explicit side-task names and add regression coverage * fix(session): align branch title behavior across clients Review found that CLI and ACP still derived branch names differently, while the Web Shell smoke test asserted the previous request body. - Preserve explicit names while normalizing only derived title suffixes - Reuse Core display-name and title helpers across branch entry points - Cover legacy, fallback, and isolation paths and update the smoke E2E * fix(session): harden branch title fallbacks Round-two review found empty normalized titles could bypass fallbacks, while client-echoed names made explicit-name semantics ambiguous. - Treat empty derived titles as absent across branch entry points - Stop Web Shell name echoing and align CLI fallback with the picker - Pin missing-session, invalid-id, and bounded-scan behavior * fix(session): pin branch title normalization and document fallback divergence Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(session): fall back on empty branch-title bases --------- Co-authored-by: heyang.why <heyang.why@alibaba-inc.com> Co-authored-by: qwen-code-autofix <qwen-code-autofix@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: 易良 <1204183885@qq.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
…enLM#9844) * fix(telemetry): restore usage aggregate when a session swap fails A /resume or /branch that fails after GeminiClient.initialize() replayed the incoming session's stored telemetry leaves that whole replay in the process-wide usage aggregate, and the /branch rollback's own re-initialize() adds a second copy of the parent's history on top — persistSessionUsage then writes the surviving session out inflated for the life of the process (QwenLM#9833). Make the replay undoable with an explicit per-swap transaction: - UiTelemetryService.snapshotForReplay / restoreFromReplaySnapshot capture and put back the process-wide metrics, the incoming and outgoing session buckets, the closed-session flags and both cached token counts. Restore overwrites rather than subtracts, so it stays correct after the rollback's own replay landed on top. - GeminiClient.beginTelemetrySwap / commitTelemetrySwap / abortTelemetrySwap own the snapshot for exactly one swap. The snapshot is armed inside initialize()'s replay branches — keyed off the client-private initializedSessionId that actually decides whether a replay happens, never a caller's config-id guess — so replays outside a transaction (process startup, ACP) arm nothing and cannot be restored by a later failed swap. - abort forgets initializedSessionId when it still names the abandoned session, so a retry replays again instead of early-returning into a permanently under-counted session. - The /resume and /branch hooks open the transaction before the core swap, commit it at the UI re-key (moving uiSwapped up to that point: a failure after the stats provider re-keyed must not roll core back or drop the bucket the display now reads), and abort it on the rollback path after core is restored. Usage accounting only: Goal state, the recorder's on-disk transcript and what a session resumes with are untouched. * fix(telemetry): reject concurrent session swaps to protect the swap slot A second /resume or /branch started while the first is still in flight entangles the single telemetry-swap slot: the second beginTelemetrySwap was a ??= no-op while both replays mutated the same aggregate, so the first swap's stale settlement either no-oped or restored its snapshot over the second swap's committed state — permanently double-counted usage or a zeroed re-keyed display with core rolled back underneath. Nothing else serializes the swaps: the session picker fires them fire-and-forget and no input gate covers them. beginTelemetrySwap now returns false when a transaction is already open (the slot doubles as the session-switch latch); both hooks reject the concurrent attempt with a visible error and never settle a slot they do not own. Pins the review's SCENARIO A interleaving as a regression test. * fix(telemetry): capture the outgoing session when the swap transaction opens The replay snapshot's outgoing session was keyed on initializedSessionId at arm time, but abortTelemetrySwap clears that field after a failed swap (it names the abandoned session). The NEXT swap then armed with outgoing = undefined, so its snapshot never captured the live session's bucket — and the rollback's re-initialize wiped its never-persisted state (skill invocations, live events) for the life of the process. beginTelemetrySwap now stores outgoingHint from the config session id (the hooks call it before startNewSession, so it still names the session the process is on); armTelemetrySwapUndo prefers the hint and falls back to initializedSessionId. Doc comments updated to match. * fix(telemetry): log the session-swap transaction lifecycle The swap transaction ran with zero logging, so a recurrence of the double-count class after a flaky /resume could not be diagnosed without reproducing the exact failure ordering — there was no record of whether the transaction armed, whether abort applied a restore or no-op'd unarmed, or which sessions were involved. Add [TELEMETRY_SWAP_*] debug lines for begin (including latch rejection), arm, commit, and abort, matching the file's existing [RESET_CHAT_*] guarded-debug convention. * fix(telemetry): engage the swap latch before touching the outgoing session The session-switch latch (beginTelemetrySwap) opened only at the core-swap step, after the outgoing session id was captured and the pre-swap work (finalize/snapshot/fork/title persistence for /branch, loadSession for /resume) had run. Two defects on concurrent fire-and-forget swaps (QwenLM#9844 review): - A pre-latch failure reached the catch's settle branch with no ownership check: commitTelemetrySwap clears the shared single slot unconditionally, so it settled whatever transaction the slot held — possibly a DIFFERENT in-flight swap — discarding its armed undo and reintroducing the QwenLM#9833 double-count when that swap later failed. - A concurrent swap that rolled back during the pre-latch window freed the slot, and the stale outgoing-session id captured before the window made the fork/rollback land on a session the UI never shows. Move the latch open to the top of both hooks, recapture the outgoing session under it, and guard both catch settle branches so an attempt only ever settles the transaction it opened itself (swapOpened). The /resume missing-session early return now settles its own opened transaction so the slot is never left stuck. The duplicated multi-line transaction rationale comments collapse into cross-references to the lifetime contract's single home (beginTelemetrySwap's JSDoc in core client.ts). * test(telemetry): guard the fork-bucket cleanup and the branch latch rejection - The failed-/branch test asserted the fork's replay bucket cleanup on SESSION_B, which never receives an event there: getMetricsForSession returns fresh empty metrics for unknown ids, so the assertion passed unconditionally. Assert on the fork's actual derived id — probe-verified to flip red when the incoming-bucket delete is skipped (QwenLM#9844 review). - Add the /branch latch-rejection coverage the hook never had: a /branch submitted while a /resume swap is in flight is rejected at the latch with no fork created and the in-flight transaction untouched, and the in-flight swap's later failure still restores exactly. A hook-level unit test pins that a rejection settles nothing (no commit/abort). - Pin the latch ordering for /resume (rejection happens before the incoming loadSession) and that resuming a missing session settles the transaction it opened, leaving the single slot free for the next swap. * fix(telemetry): drop the inert swapRejected guard from the branch settle path swapRejected was set true only on the line immediately before the step-0 throw, which runs before swapOpened = true — so the catch's settle guard `!swapRejected && swapOpened` was provably equivalent to `swapOpened` alone (the shape the sibling useResumeCommand catch already uses). Remove the dead flag and correct the comments that misattributed the invariant to it: the statement ordering (throw before `swapOpened = true`) is what keeps a latch-rejected attempt from settling the in-flight swap's slot. Suggested in the QwenLM#9844 review. * test(telemetry): pin the swap-slot settle on failed /resume and /branch paths The catch-block settle (`else if (swapOpened) commitTelemetrySwap()`) had no coverage in either hook, and the fork-bucket regression guard derived its session id through a non-null assertion that could pass vacuously: - /resume: add tests for both settle subpaths — a pre-core-swap failure (session load rejects) and a post-UI-commit failure (loadHistory throws after the re-key) — each asserting the catch committed (never aborted) the transaction and that the next swap is admitted instead of rejected with "already in progress". Deleting the settle block fails both. - /branch: the existing failure tests use a client mock without commitTelemetrySwap, so the settle was an optional-chained no-op there. Add a forkSession-rejects test against a stateful swap-slot fake that observes the slot release as the next begin being accepted. - Harden the fork-bucket guard to require the fork key to actually exist in the session store before asserting on it: getMetricsForSession returns fresh empty metrics for unknown ids, so a missing forkId (e.g. once removeSession becomes realistic) passed vacuously. Making the fake removeSession delete the fork now fails the test loudly. Suggested in the QwenLM#9844 review. * fix(telemetry): keep the live parent initialized when the branch rollback armed the undo abortTelemetrySwap assumed swap.undo.sessionId always names the abandoned incoming session, but when /branch fails between startNewSession(fork) and initialize() (waitForGoalRuntime rethrows), the forward replay never runs and the rollback's own re-initialize arms the undo with the live parent's id. The blind clear then forgets a correctly-initialized session, so the next same-session initialize re-replays stored telemetry on top of the live aggregate — a permanent double-count. Guard the clear on the undo actually belonging to the incoming session (QwenLM#9844 review). * test(telemetry): derive the fork id pre-cleanup and make removeSession realistic The fork-bucket anti-vacuity guard derived the fork id from post-cleanup store membership, asserting the fork survives a failed /branch — the exact opposite of production, which calls removeSession(newSessionId) in that same failure branch. Make the fake removeSession delete the entry like the real SessionService, return the shared session-service object from makeFakeEnv, and derive the fork id from forkSession's call args instead. The bucket assertion stays non-vacuous without requiring the fork to outlive the cleanup the production path performs (QwenLM#9844 review). * test(telemetry): share the swap-slot client fake and model the abort boolean The stateful makeSwapSlotClient fake — the only place the hook tests model the client's single-slot begin/commit/abort contract — existed twice, byte-for-byte identical, in useResumeCommand.test.ts and useBranchCommand.test.ts, and under-modelled abortTelemetrySwap(): boolean (both copies returned nothing). Extract it to a shared test-utils module and model the boolean return — true when the abort settled an open transaction, false when nothing was open — so 'abort ran but restored nothing' is observable. The two core-swap rollback tests now observe the abort through it (QwenLM#9844 review). * fix(telemetry): re-initialize the outgoing session when a failed /resume rolls back Mirror /branch's rollback in useResumeCommand: after startNewSession(oldSessionId, prevSessionData) re-run client.initialize() on a best-effort reload of the outgoing session's persisted state, so initializedSessionId names the live session again. Without it a subsequent same-session /resume skips initialize()'s early return, and its replay wipes the live bucket (skill invocations are never persisted) while re-adding stored telemetry on top of the aggregate that already contains it — the QwenLM#9833 double-count class reintroduced by failed /resume -> same-session /resume (QwenLM#9844 review). Also make the shared swap-slot fake honest about abortTelemetrySwap's boolean: it over-approximates the open-but-unarmed case (real client returns false there), so the malformed-Goal test no longer asserts the return value and the fake's JSDoc says what it models. Remaining true-return assertions sit only on shapes where initialize() ran during the open transaction, which arms the real undo.
…not streamState (QwenLM#9971) handleCronTextChunk used streamState.has(sessionId) to tell prompt-response chunks from cron/non-prompt chunks. That discriminator fails in both directions (QwenLM#6094 items 1 and 2): - With blockStreaming: 'on', onResponseChunk early-returns and never populates streamState, so during an active cron flow every prompt-response chunk leaked into cronBuffer and was re-sent by the 2s idle flush on top of the BlockStreamer delivery (duplicate messages). - A residual streamState entry from a finished turn whose flush has not settled (e.g. cancelled/errored prompt) kept the guard true, silently dropping all subsequent cron textChunks for that session. Track sessions with a prompt turn in flight in activePromptSessions, maintained by the onPromptStart/onPromptEnd hooks. ChannelBase always pairs the two per turn (onPromptEnd runs in the prompt path's finally, even on error/cancel), and the marker is independent of streaming config, so it is a reliable discriminator. The set is also cleared in disconnect() and onSessionDied alongside the existing streaming-state cleanup. Items 3 and 4 from the issue are already addressed on main (plain-text fallback for no-msgId sends; setBridge re-attaches _cronTextHandler). Items 5 and 6 remain open as separate low-priority work.
…9568) * fix(cli): mark estimated compression token counts in banners /compress-fast anchors its banner on the API-reported prompt count (system prompt + tools + history) minus a locally estimated reduction, while a later /compress re-estimates history-only once the stored count is estimate-derived. Consecutive banners therefore show numbers on different scales (e.g. 170k -> 75.8k followed by 26.6k -> 14.5k), which reads as ~50k of context silently lost even though nothing was dropped. Expose per-side token-count provenance on ChatCompressionInfo (originalTokenCountIsEstimated, mirroring newTokenCountIsEstimated) from both tryCompress and compressFast, plumb it through the compression history item, and render estimated figures with a '~' prefix in the CompressionMessage banner, the auto-compaction notice, and the headless/ACP messages. Display-only: no compression behavior, thresholds, or recorded counts change. Fixes QwenLM#9309 * fix(cli): mark estimated counts in the ACP auto-compaction notice The ACP session auto-compaction diagnostic interpolated raw token counts and ignored the QwenLM#9309 provenance flags, so estimated figures reached ACP clients unmarked. Route both counts through the same '~'-prefix formatting as the TUI notice and cover it with an asymmetric-flag regression test. Also harden the marker tests per review: asymmetric estimation flags in compressCommand/useGeminiStream fixtures so a swapped flag-argument mutation is detectable, an executionMode:'acp' case for the /compress stream_messages branch, and a null-event ChatCompressed case pinning the 'unknown' fallback. * fix(core): derive compression provenance from the selected count source The estimated flag published on compression banners was computed from the stored count's provenance or the mere presence of an override, not from the count source selected for the attempt: - Auto-compaction / hard-tier rescue: a previously API-reported stored count forced the flag false even though the attempt publishes the precomputed effective count, which always folds in locally estimated parts (pending user message, previous output). - Reactive overflow: a limit/config/default fallback passed as the override forced the flag false whenever the provider did not report actualTokens. Both paths omitted the `~` marker and presented projected or fallback values as API-reported counts (review on QwenLM#9568). Derive the flag from the selected source: a caller-precomputed effective count is always estimated, and the override now carries explicit provenance ({ count, isEstimated }) so only a provider-reported actualTokens override marks the count authoritative. * test(core): pin send-path effective token counts exactly (QwenLM#9309)
…wenLM#9758) * fix(core): emit OpenRouter's reasoning disable when thinking is off The AUTO-mode permission classifier's stage-1 side query forces a respond_in_schema tool call (tool_choice: 'required') with a 256-token budget and includeThoughts: false. On OpenRouter endpoints the thinking-disable rendered only into shapes the gateway ignores (chat_template_kwargs.enable_thinking for qwen-family models), and the pipeline's unconditional strip then removed the `reasoning` object — OpenRouter's native thinking knob. Thinking stayed on, the model spent the whole budget on reasoning, never emitted the tool call, and the classifier fail-closed with "Classifier stage 1 unavailable" (QwenLM#9757). Mirror the isDeepSeekHostname precedent: hostname-gated detection (openrouter.ai / *.openrouter.ai) and emit `reasoning: { enabled: false }` in the reasoningDisabled branch after the strip — the provider buildRequest hook runs before the strip, so emitting earlier would be removed again. Applied endpoint-wide rather than qwen-family-gated: `reasoning` is an OpenRouter provider-level parameter, unlike `enable_thinking`, which is a qwen wire field that leaks upstream on non-qwen routings. thinkingMandatory models stay exempt; DashScope (both shapes), vLLM/SGLang, DeepSeek hostname, and the official OpenAI endpoint are untouched. Repro + regression coverage added in pipeline.test.ts (red before the fix, green after). * test(core): cover OpenRouter reasoning guard
…ve late attach (QwenLM#9824) * fix(core): validate skill commands against the live provider to survive late attach SkillTool caches the model-invocable command set during construction, but in interactive mode the provider is only registered after CommandService initialisation resolves — after Config.initialize() has already warmed the tool registry. The cache then stays empty until an unrelated SkillManager change event re-runs refreshSkills(), so validateToolParams intermittently rejects commands announced by the per-turn skills drain (issue QwenLM#9821). Consult the synchronous provider live in validateToolParams instead, applying the same file-based-skill name shadowing as collectAvailableSkillEntries so path-gated skills stay gated. The execute path already reads the executor live. Regression tests cover late attach, paths gating, and the no-provider SDK path. * fix(core): keep hidden skills from shadowing commands * fix(core): default hidden skill collection in tests * fix(core): record hidden skill fallback failures * test(core): cover hidden skill fallback args --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qqqys
force-pushed
the
simplify/enum-selector
branch
from
August 25, 2026 08:20
8473eb4 to
db1b9cc
Compare
Owner
Author
|
Opened in the wrong repo by mistake — this change belongs upstream: QwenLM#9997. Closing. |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Removes a UI selector component that is no longer referenced anywhere in the codebase, together with the test and snapshot that existed solely to cover it. The component renders a left-right value picker for enum-typed settings, but no screen or component mounts it; its only callers were its own unit test and snapshot. Nothing else changes.
Why it's needed
Dead surface is a cost: every contributor who reads it has to work out what wires it up, and the answer here is "nothing". The component arrived with an upstream sync and was never referenced in this repository's history — no production code, no string-keyed registry entry, no dynamic import path. Its dedicated test and snapshot exist only to cover the component itself, so they go with it.
Reviewer Test Plan
How to verify
This is a pure deletion of code nothing references. Verify the absence of consumers rather than behavior:
rg -n '\bEnumSelector\b' packages integrations integration-tests scripts .github docs-site— expect zero hits after this PR (the only three before it were the component, its test, and its snapshot).npm run build && npm run bundle && npm run typecheck— all green locally with this change (a type-only break from a removed export would surface here; typecheck does not run in CI).npm run lint:ci— green locally with this change.No user-visible behavior changes, so no before/after TUI evidence applies.
Evidence (Before & After)
N/A — non-UI change (dead-code deletion; nothing rendered this component).
Tested on
Environment (optional)
Local
npm run build && npm run bundle && npm run typecheckandnpm run lint:ciin a clean worktree offorigin/main.Risk & Scope
Linked Issues
Part of the dead-surface cleanup tracked in #14.
Machine details (find-simplifications)
enum-selector— class 2, dead component with its test and snapshot.packages/cli/src/ui/components/shared/EnumSelector.tsx(87 lines),EnumSelector.test.tsx(152 lines),__snapshots__/EnumSelector.test.tsx.snap(9 lines) — 248 lines deleted, 0 added (git diff --numstat)..github,.husky,.vscode,patches, root build/lint manifests, tracked.qwenfiles) resolves to exactly three paths — the component itself (declaration), its test (test-only), and its snapshot (snapshot). Zero production consumers; zero string-keyed references (checkedstring-keys,build-graph,generated,vi-mock,mirrors,dyn-import,cli-flags,assetschecklist rows); no integration-test reference (second pass without test exclusions); no design-doc or plan mentions it.packages/cli/src— landable, notpackages/core/channels/SDK/acp-bridge), full-corpus grep, own-file check (no in-file caller beyond the declaration), hidden-consumer checklist, test-only check, unwire history (no binding text ever existed — it was never wired), design-doc ownership (none).origin/main(4a281f2efc): same three paths, no commits touching the symbol since the survey, branch base freshly fetched.npm run build && npm run bundle && npm run typecheckgreen;npm run lint:cigreen; targeted vitest skipped by design (the deletion carries the surface's only test; corpus re-grep on the landing checkout returned zero surviving references). Self-audit: two clean passes.eslint.legacy-filenames.mjsentry (.tsxis outside the kebab-case rule), no i18n locale keys (component took display strings via props), no docs rows.中文说明
本 PR 做了什么
删除一个在代码库中已无任何引用的 UI 选择器组件，以及仅为覆盖它而存在的测试和快照。该组件渲染一个用于枚举类设置项的左右切换选择器，但没有任何界面或组件挂载它；它唯一的调用方就是它自己的单元测试和快照。除此之外没有任何其他改动。
为什么需要
死代码是一种成本：每个读到它的人都得弄清是什么在用它，而这里的答案是“没有任何东西”。该组件随一次上游同步进入本仓库，在整个历史中从未被引用——没有生产代码、没有字符串键注册项、没有动态导入路径。它专属的测试和快照只为覆盖组件本身而存在，因此一并删除。
评审者测试计划
如何验证
这是一次对无任何引用代码的纯删除。请验证消费者的缺失，而不是行为：
rg -n '\bEnumSelector\b' packages integrations integration-tests scripts .github docs-site—— 本 PR 之后应无任何命中（之前仅有的三处即组件本身、其测试与其快照）。npm run build && npm run bundle && npm run typecheck—— 本地在包含本改动的情况下全部通过（删除导出可能引发的纯类型问题会在这里暴露；typecheck 不在 CI 中运行）。npm run lint:ci—— 本地在包含本改动的情况下通过。由于没有用户可见的行为变化，无需前后对比的 TUI 证据。
证据（改动前后）
N/A —— 非 UI 改动（死代码删除；该组件从未被渲染）。
测试环境
环境（可选）
在基于
origin/main的干净 worktree 中本地运行npm run build && npm run bundle && npm run typecheck与npm run lint:ci。风险与范围
关联 Issue
属于 #14 跟踪的死代码清理工作。
机器细节见上方英文 “Machine details (find-simplifications)” 折叠块：候选编号
enum-selector（第 2 类：死组件连同其测试与快照），共删除 248 行、新增 0 行；全语料库搜索证明消费者恰为组件自身、其测试与其快照三处；调查协议各步骤及落地前的重新核验均已执行；构建、打包、类型检查与lint:ci全部通过；因删除本身带走了该表面唯一的测试，按流程跳过定向 vitest，改以落地检出上的语料库复扫（零残留引用）兜底；无允许列表条目、无 i18n 键、无文档行等连带清理项。