mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-10 09:15:24 +00:00
1032 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
qqqys
|
fb0239eab4
|
feat(channels): add structured channel memory management (#6860)
* feat(core): add structured channel memory document * fix(core): preserve legacy channel memory whitespace * feat(core): structure channel memory storage * fix(core): close channel memory races * test(core): cover channel memory read failures * feat(channels): parse channel memory item intents * test(channels): cover memory intent precedence * feat(channels): manage structured channel memory * fix(channels): address structured memory review * feat(cli): wire structured channel memory * docs(channels): document structured channel memory * fix(core): harden channel memory persistence * fix(core): preserve channel memory uniqueness * fix(channels): prioritize channel memory item updates |
||
ranzhenyu
|
cf42ab6b7e
|
feat(acp): expose tool-call preparation lifecycle (#6819)
* feat(acp): expose tool-call preparation lifecycle Why: ACP clients receive no signal while providers stream tool arguments, making long calls appear stalled and delaying tool-identity policy decisions. What: - attach transient preparation metadata for Anthropic and OpenAI-compatible streams - emit correlated ACP pending, execution, and discarded lifecycle updates - preserve normalized call IDs across partial chunks and provider ID reuse - clear abandoned retry calls and keep cleanup failures from terminating healthy retry/fallback streams - deduplicate suppressed preparations and protect completed remapped parser buffers - cover multi-tool Anthropic streams, ID reservation, TodoWrite suppression, retry cleanup, cancellation, and stream failure Impact: The metadata is additive and consumed only by ACP. It exposes no partial arguments, is not persisted to conversation history, and does not move permissions, hooks, scheduling, or execution ahead of complete function calls. Tests: - Core provider and stream suites: 649 passed - ACP lifecycle suites: 316 passed - npm run build - npm run typecheck - npm run lint:ci - changed-file Prettier and git diff checks Refs: #6775 * fix(acp): stabilize tool preparation lifecycle updates Why: - ACP cleanup failures must not convert a successful model stream into a failed prompt. - A prepared tool call must be updated in place when execution starts instead of creating a second card. What: - Preserve the primary stream outcome when preparation cleanup fails and remove duplicate message display finalization. - Track prepared call IDs so execution starts use tool_call_update, guard empty preparation metadata, and cover late stable IDs. Impact: - Ordinary tool calls keep their existing tool_call start frame. - Streaming parser production behavior is unchanged. * Update packages/cli/src/acp-integration/session/Session.test.ts overrides参数在createPreparationResponse中被声明但从未使用——所有11个调用点仅传递callId且toolName. 该as GenerateContentResponse强制类型转换会绕过对始终为空对象的结构化类型检查。 Co-authored-by: qwen-code-ci-bot <qwen-code-ci@service.alibaba.com> * fix(acp): harden preparation lifecycle handling Why: - A malformed test helper prevented the preparation lifecycle suite from compiling. - Duplicate preparing frames and state cleanup need direct regression coverage. What: - Repair the preparation response helper and isolate cleanup warning assertions. - Suppress duplicate preparing frames and cover terminal cleanup plus missing tool call IDs. Impact: - Normal preparation and execution transitions remain unchanged. - Repeated preparation frames for the same call ID are now ignored. --------- Co-authored-by: qwen-code-ci-bot <qwen-code-ci@service.alibaba.com> |
||
易良
|
d7e2892a7c
|
fix(cli): avoid updating active CLI processes (#6874)
* fix(cli): avoid updating active processes * fix(cli): close update relaunch gaps * test(cli): fix standalone update source path * fix(cli): reset deferred update per relaunch |
||
jinye
|
c538bd70d2
|
feat(core): emit liveness heartbeats for silent foreground shell commands (#6876)
* feat(core): emit liveness heartbeats for silent foreground shell commands Silent foreground commands previously produced no events between spawn and settle, so ACP gateways and stream-json consumers could not tell a long-running command from a dead session. The shell tool now emits a structured ShellProgressData through the existing updateOutput channel whenever no display update has fired for tools.shell.heartbeatIntervalMs (default 10s, 0 disables). Heartbeats carry liveness stats only - never command output - and never enter model context. Consumers: the ACP session forwards heartbeats as meta-only tool_call_update frames (gated so a tick racing the settle path cannot regress status after completion) and records heartbeat span attributes; stream-json forwards them as tool_progress events behind includePartialMessages; the TUI scheduler, React hook, and subagent runtime ignore them so live output views are not replaced by stats objects. * docs(design): add silent command heartbeat design doc * fix(acp): keep tool_call_update heartbeats from breaking in-repo consumers Codex review of the heartbeat change found that in-repo ACP consumers did not tolerate the new meta-only in_progress frames. A full sweep of tool_call_update consumers found three that mishandled them, each now guarded with a regression test: - The desktop agent converted every tool_call_update into a terminal tool_result, so the first heartbeat would prematurely complete the command with an empty result. It now skips in_progress updates. - DaemonChannelBridge requires kind on tool_call_update and flagged the kind-less heartbeat as a malformed-protocol error every interval. It now drops kind-less in_progress frames silently. - The web-shell daemon UI normalizer derived the tool block title from _meta.toolName, overwriting the human-readable title on every heartbeat. It now drops heartbeat frames outright. The remaining consumers (VS Code companion, acp-bridge compaction, session export, daemon TUI adapter) merge updates conditionally and are heartbeat-safe without changes. * fix(core): address PR review — heartbeat monotonic gate, guard scope, telemetry Review round 1 on #6876 (yiliang114, wenshao, chiga0, qwen3.7-max): - shell.ts: the silent-idle gate now uses the monotonic performance.now() clock (via lastOutputPerfTime, falling back to spawn time) instead of the Date.now()-based lastUpdateTime, so an NTP step can neither skew the payload nor misfire a heartbeat — matching the design doc's monotonic commitment. It also keys off actual output arrival rather than the throttled display update. - session-tracing.ts: endToolExecutionSpan now applies caller-supplied attributes BEFORE the canonical keys (duration_ms, success, error) so a passthrough attribute can never mask the span's own outcome fields. - desktop qwen-agent.ts: the in_progress drop guard is now scoped to frames carrying _meta.shellProgress, matching the daemon bridge and web-shell normalizer guards, so a future non-heartbeat in_progress frame is not silently swallowed. - Tests: the desktop regression test now pins result==='done' (previously it stayed green even with the guard removed); added a Session.test assertion that heartbeat counts reach the tool-execution span attributes. * fix(acp): align desktop heartbeat guard with normalizer; test kind pass-through Review round 2 on #6876 (qwen3.7-max via ci-bot): - The desktop qwen-agent in_progress drop guard was broader than the web-shell normalizer's: it dropped any in_progress + shellProgress frame regardless of kind, while the normalizer only drops kind-less ones. The comment claimed they matched. Added the kind-absent check so the desktop guard matches the normalizer exactly — a kind-bearing frame now passes through on both platforms (heartbeats emitted by the ACP session never carry a kind, so real behavior is unchanged). - Added pass-through tests on both sides (daemonUi + desktop) asserting an in_progress frame WITH a kind normalizes to a tool.update / tool_result rather than being dropped, so the load-bearing kind-absent condition is no longer only exercised on the drop path. * fix(channels): scope daemon bridge heartbeat drop to shellProgress frames Review round 3 on #6876 (qwen3.7-max via ci-bot): the DaemonChannelBridge heartbeat guard lived in the shared tool_call / tool_call_update case and dropped ANY kind-less in_progress frame, so a genuinely malformed kind-less tool_call (status in_progress, no shellProgress) was silently swallowed instead of reaching emitProtocolError. Gate the drop on _meta.shellProgress — matching the qwen-agent and web-shell normalizer guards — so real heartbeats are still dropped while malformed frames are flagged. Added a regression test for the malformed path. |
||
ytahdn
|
b59b341a0a
|
feat(web-shell): add extension management page (#6815)
* feat(daemon): support interactive extension installs * feat(web-shell): add extension management page * fix(web-shell): align extension update behavior * fix(web-shell): polish extension management UI * fix(extensions): harden interactive operations * fix(web-shell): address extension review suggestions * fix(web-shell): refine extension interaction handling * fix(web-shell): resolve extension operation races * fix(web-shell): harden extension action admission * fix(web-shell): surface extension recovery failures * fix(web-shell): preserve extension card titles * fix(web-shell): refine extension card layout * fix(extensions): address operation review findings * test(extensions): close remaining review gaps --------- Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com> Co-authored-by: ytahdn <ytahdn@gmail.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
jifeng
|
079ba35207
|
feat(web-shell): add selection statistics to markdown tables (#6838)
* feat(web-shell): add selection statistics * fix(web-shell): clear stale table selections * fix(web-shell): address selection statistics review |
||
|
jinye
|
1f0078c7a2
|
feat(serve): Add workspace-qualified session export (#6844)
* feat(serve): add workspace-qualified session export Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6844) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> |
||
|
jinye
|
c7250df8ea
|
feat(serve): Add workspace-qualified Voice (#6839)
* feat(serve): add workspace-qualified voice Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): harden workspace voice lifecycle Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6839) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6839) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(cli): address workspace Voice review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6839) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): clean up Voice lifecycle resources Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): address Voice review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> |
||
|
jinye
|
fea3ab3854
|
feat(serve): add extension management v2 (#6825)
* feat(cli): workspace-qualified extensions REST (daemon multi-workspace) Mirror the daemon extension-management REST surface to per-workspace routes, reusing the Phase 3 runtime resolver and trust gate. Extract a per-workspace extensions controller so the primary workspace shares one install queue, operation history, and status cache across the legacy and workspace-qualified routes. Reads resolve the target runtime only; mutations require a trusted workspace. Advertise a new baseline capability so clients can discover the surface, and add matching SDK client methods. Refs #6378. * qwen: address PR review feedback (#6638) Align the new extensions controller file's copyright year with the other new files added in this change. * qwen: address PR review feedback (#6638) Redact credentials from the extension source on the two success-path fan-outs (session refresh and refresh-failure broadcast), matching the operation record and failure broadcast. Document the non-cancellation semantics of the extension timeout wrapper. * qwen: address PR review feedback (#6638) Share the queue-full sentinel message via an exported constant so the throw site (controller) and the 429 match site (routes) cannot drift after the module split. Include the bound workspace in the extension operation log prefixes so concurrent per-workspace controllers are distinguishable in stderr. * feat(cli): add concurrent extension preparation Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): remove redundant extension context build Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6638) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6638) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): address extension review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): address final review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): address latest review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6638) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): reject links in npm extension archives Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): limit npm extension archive downloads Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): address review follow-ups Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): address latest review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): release rejected operation slots Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): address operation review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): align archive handling contracts Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): preserve watcher generation state Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(extensions): align management contracts Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk): bound extension operation polls Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(core): cover forged prepared commits Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(core): assert activation generation increment Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): close archive and polling gaps Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): retry suppressed extension generations Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(cli): cover archive URL extension updates Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk): preserve unbounded operation waits Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): share npm redirect download deadline Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): preserve extension reload diagnostics Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): preserve installed Claude plugin paths Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): return committed activation state Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): preserve extension preparation errors Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): validate extension setting env vars Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): target extension reconciliation Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6638) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6638) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6638) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6638) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6638) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6638) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6638) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(cli): cover resultless legacy commit warnings Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): retain suppressed extension generations Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): record legacy runtime reconciliation Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): validate extension clients by runtime Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): record workspace activation refresh Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(cli): stop extension reconcilers after cases Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): resolve global runtimes at reconciliation Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): reconcile newly registered runtimes Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): prevent overlapping runtime reconciliation Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): dispose late runtime apps during shutdown Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): keep projection repair best effort Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): preserve committed store results Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): quarantine corrupt store journals Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): harden npm download redirects Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): address review edge cases Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): honor cancellation between preparation stages Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): retry prepared cleanup failures Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(extensions): cover committed artifact recovery boundary Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): release extension refresh queue on timeout Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6638) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6638) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6638) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6638) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix: address extension review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6638) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6638) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6638) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6638) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): reconcile extension store compatibility state Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): bound npm redirects and isolate extension tests Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): make extension uninstall store-authoritative Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): defer prepared extension secret mutations Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): validate staged extensions before commit Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): enforce public extension network policy Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): handle extension response failures Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): surface committed refresh warnings Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): guard timer unref calls Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): release commit lane after durable writes Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(serve): update mutation callback assertions Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): refresh live extension instructions Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): address latest review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): address follow-up review findings Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): address remaining activation feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): preserve preparation queue status Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): enforce network request deadlines Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(serve): clarify single-workspace capabilities Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): guard deferred settings commit Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): cancel archive extraction Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): harden refresh recovery Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): serialize extension reconciliation Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(extensions): address post-commit review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6825) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6825) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6825) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: fix CI failure on PR #6825 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: fix CI failure on PR #6825 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6825) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix: address critical PR review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): bound legacy extension update checks Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(acp): deduplicate extension refresh requests Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6825) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6825) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6825) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6825) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(sdk): update browser bundle budget after main merge Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> |
||
callmeYe
|
53468cd8af
|
feat(daemon): add workspace skill toggle API (#6816)
* feat(daemon): add workspace skill toggle API * test(daemon): cover skill toggle capability integration * fix(daemon): harden skill refresh handling * fix(daemon): improve skill refresh diagnostics * test(daemon): expand skill toggle coverage --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> |
||
tanzhenxin
|
220fba7917
|
feat(subagents): make Explore inherit the main model by default (#6807) | ||
|
jinye
|
9dd8389ebe
|
fix(serve): Route session continue, language, and artifacts by owner (#6833)
* fix(serve): route session mutations by owner Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6833) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
jinye
|
13c224f5e9
|
feat(serve): support runtime workspace removal (#6745)
* feat(serve): support runtime workspace removal Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): address workspace removal review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(cli): strengthen workspace removal regressions Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(webui): fix timeout assertion lint Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): address workspace removal review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): update workspace Git test registry Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(daemon): address workspace removal review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6745) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6745) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6745) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6745) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6745) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6745) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6745) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6745) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6745) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: fix CI failure on PR #6745 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6745) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6745) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(web-shell): cover workspace removal after sidebar rebase Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6745) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
jinye
|
b933b90172
|
feat(serve): support multi-workspace rewind and shell (#6826)
* feat(serve): support multi-workspace rewind and shell Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6826) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6826) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
callmeYe
|
be4c46d085
|
feat(serve): expose skill installation paths (#6811) | ||
|
jinye
|
3d5dd41bc7
|
fix(serve): route session actions to the owning workspace (#6798)
* fix(serve): route session actions to the owning workspace Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6798) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
ytahdn
|
e6bd1b1c12
|
feat(web-shell): add session created callback (#6703)
* feat(web-shell): add session created callback * fix(web-shell): bound session callback setup * fix(web-shell): serialize session preparation * fix(web-shell): clarify session preparation diagnostics * fix(web-shell): handle session preparation races --------- Co-authored-by: ytahdn <ytahdn@gmail.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> |
||
|
jinye
|
6d286149d6
|
feat(serve): Bound persisted transcript pages (#6769)
* feat(serve): bound persisted transcript pages Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): address transcript page review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
jinye
|
98f2bb37ec
|
feat(cli): Add runtime daemon channel control (#6741)
Some checks are pending
* feat(cli): add runtime daemon channel control Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): address daemon channel review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(daemon): align channel control timeout budget Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): preserve serve fast-path import boundary Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): distinguish pending channel generations Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): use workspace env for deferred webhook auth Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6741) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6741) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6741) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6741) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6741) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6741) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6741) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> |
||
BaboBen
|
1dd984509b
|
fix(feishu): validate credentials before WebSocket startup (#6780)
* docs(channels): design Feishu worker credential validation * docs(channels): plan Feishu worker credential fix * fix(feishu): wait for authenticated WebSocket startup * fix(feishu): close late WebSocket readiness * fix(feishu): validate credentials before WebSocket startup |
||
dreamWB
|
460c92102e
|
feat(web-shell): support custom composer placeholders (#6765) | ||
Shaojin Wen
|
d1b2a7fb72
|
feat(review): procedural correctness finders, effort levels, and posting/verify guardrails (#6711)
* feat(review): procedural correctness finders, effort levels, and posting/verify guardrails
Rework the /review skill's finder layer and add precision and cost controls,
informed by dogfooding the skill against real PRs.
Recall:
- Split Agent 1 (Correctness) into three procedural finders defined by how they
walk the diff — 1a line-by-line (incl. language-pitfall and wrapper-routing
checks), 1b removed-behavior audit, 1c cross-file tracer — so coverage is
complementary instead of overlapping. Bump the 3A dimension fan-out to 12
agents and shift the 3A/3B gate to 3200 diff lines.
- Add Agent 8: up to two diff-specialized finders written per-review when the
diff concentrates in a domain with a known failure grammar.
- Fold altitude into Code Quality and a quote-the-rule discipline into the
conventions pass.
Precision:
- Every finding must state a concrete failure scenario (trigger to wrong
outcome, or concrete cost); findings that can't are dropped at the source,
and verification re-traces the scenario rather than judging prose.
- Verification checks a finding against the diff's own documented intent: a
"regression" the diff deliberately changes and documents is a design
decision, not a defect.
Cost and safety:
- Add --effort low|medium|high: cheap inline passes with no subagents (default
high for PRs, medium for local changes).
- Hard-gate PR posting: never submit a review unless --comment was passed or the
user explicitly asked, regardless of verdict.
- Add a substantive-return check for whole-diff agents (invariant, cross-file,
test-coverage matrix) so a silently whiffing agent is caught like a missing
chunk receipt.
DESIGN.md records the rationale and dogfooding cases behind each change; user
docs updated with the effort levels and the new agent roster.
* docs(review): fix stale topology numbers flagged in review
- Define H in the 3B pipeline diagram cost annotation (3 invariant agents
per heavy file).
- Annotate the 40-PR re-gating cost figures with the roster they were
measured under (22 agents / ~5% at 10 agents; ~34 / ~7% at 12).
- Correct the fork-subagent savings estimate to ~88-92% (~750-950K →
~80-88K); the previous range predated the updated totals.
- "None or nine" receipts under 3A is eleven under the 12-agent roster
(every agent except Build & Test walks the diff).
* fix(review): address review feedback on effort/verify/lightweight edge cases
Criticals from review:
- Apply the --comment→high-effort override only after target disambiguation;
an ignored --comment (non-PR target) no longer silently forces high.
- The documented-intent gate caps confidence only when the rationale makes the
harm uncertain; a traced harm that survives the rationale keeps high
confidence, and rejection is reserved for pure re-descriptions.
- Lightweight cross-repo mode degrades Agents 1a/1b to diff-only and routes
unverifiable re-establishment claims to low confidence instead of asserting
them, matching the verifier's limits.
Suggestions:
- Agent 0's empty-scope exit now carries its evidence and the whiff check
recognizes it, so a legitimate no-linked-issue return is not relaunched.
- Reframe the 3200-line clause as an attention bound (3B is not guaranteed
cheaper with heavy files or specialized finders).
- Fix call-budget notation: F for findings vs N for chunks; correct the 3B
budget to rounds × chunks for the reverse audit (~70 calls on the 19-chunk
example, not ~28-30); state the runtime concurrency cap (10) instead of
claiming ~1x wall time.
- Preserve the failure scenario through pattern aggregation and posted inline
comments; extend quality-finding verification to check the named helper
does what the finding claims.
- Document medium effort's roster (no dedicated security/test-coverage pass);
qualify the cross-effort scope note (incremental cache is high-only); define
"lenses" on first use; fix tense and the stale 9-agent line in commands.md;
clarify the 1b skip condition (no removed/replaced lines).
* fix(review): close 422-relocation verdict hole and lightweight-mode context gap
From review feedback (one human, two model reviews):
- 422 recovery: Criticals relocated into the review body now keep the event at
REQUEST_CHANGES — the event/body table counted comments only, so a review
whose blockers were all relocated could submit as APPROVE/no-blockers
COMMENT with blocker text in the body.
- Lightweight cross-repo mode now runs pr-context (pure GitHub API): Agent 0
and the open-Critical re-check need the PR body and open threads, which the
bare gh-pr-diff setup never captured.
- Define --effort value parsing so a non-enum next token (e.g. a PR number) is
never consumed as the value.
- Add the missing test-coverage-matrix definition section; mark agent counts
as maxima (1b skipped on no-deletion diffs).
- Sync DESIGN's documented-intent paragraph with the corrected confidence
policy; align 3A/3B budget headings with the dual-trigger gate and state the
38-95 reverse-audit range explicitly.
- Docs: dual-trigger diagram labels, attention-bound wording, diff-reading
lenses phrasing, per-stage-bounded (not fixed-total) cost claim, effort
table qualifications, failure-scenario in the Step 7 JSON samples.
* docs(review): reconcile verifier rejection rule and close remaining edge notes
- State the Critical-rejection bar once, without the self-contradicting
"never reject / to reject" phrasing: rejection requires quoting the
contradicting code, and the floor verdict is confirmed (low confidence)
when it cannot be quoted.
- Note the one sanctioned exception to the empty REQUEST_CHANGES body:
unmappable or 422-relocated Criticals.
- Give the callee-direction check a concrete procedure (walk the other
changed symbols this territory calls, re-read their post-change
contracts).
- Define lightweight-mode pr-context failure handling: continue diff-only,
skip Agent 0, open-Critical re-checks become "cannot tell" (no Approve).
- Clarify Agent 8 applies in every mode (it needs only the diff) and that
Step 6 follow-up tips are high-effort only.
* fix(review): close flag-parse, context-unavailable, and downgrade edge cases
Address the latest review round on the skill text:
- An invalid spaced --effort value is discarded (with the warning) whenever
another token is the target, so `/review 6711 --effort typo` reviews PR
6711 instead of leaking `typo` into target disambiguation; the token is
kept only when it is itself the sole target candidate.
- The lightweight-mode pr-context failure now names a context-unavailable
state with a defined Step 7 serialization: never APPROVE, submit COMMENT
with a diff-only body, findings or not.
- Step 6's open-Critical re-check draws from both context sections; a reply
alone ("I disagree") no longer retires a blocker — only a code-verified
"fixed by this diff" does.
- DESIGN and user docs now state the same rejection bar as the skill:
rejecting a Critical requires quoted contradiction (or a documented-intent
re-description); anything less certain downgrades.
- Downgrading a REQUEST_CHANGES that carries body-relocated Criticals keeps
those descriptions after the downgrade sentence, so the self-PR downgrade
can no longer erase the only copy of a blocker.
* fix(review): close verdict-upgrade and body-Critical re-check gaps
Third review round on the skill text:
- 422 recovery may never upgrade the event: a Suggestion-only review whose
anchors all failed resubmits as COMMENT with the could-not-anchor body,
never as APPROVE/"No issues found" — the verdict reflects confirmed
findings, not surviving anchors.
- Step 6's open-Critical re-check now also walks the Review summaries
section: an unmappable or 422-relocated blocker lives only in a review
body, and pr-context truncates summaries to ~240 chars, so a summary
showing (or cut where one could hide) a Critical marker is fetched in
full via the reviews API before ruling.
- The context-unavailable cap now applies to every C=0 row of the invariant
table, not just the empty one: Suggestion-only results post a diff-only
body instead of a "no blockers" claim the run cannot certify.
* fix(review): compose COMMENT bodies from clauses and harden the body-Critical re-check fetch
Fourth review round found four pairwise collisions between rules that each
set "the" COMMENT body, plus four execution gaps in the Step 6 full-body
fetch. Close the class, not the instances:
- Replace the fixed-sentence bodies with an ordered clause composition rule
(downgrade reasons, context-unavailable warning, suggestions disclosure,
uncoverable chunks, body Criticals) — each clause present iff its state
holds, free prose still banned, single-state case identical to the table.
- Define C once, globally: Criticals the review posts anywhere (inline or
body), so no downstream C=0 rule can erase a body-only blocker, and 422
relocation keeps REQUEST_CHANGES by definition rather than by patch.
- 422 recovery re-derives bodies via the composition rule, so a
context-unavailable run can never restore a "no blockers" certification.
- Step 6's full-body fetch is paginated (--paginate; the endpoint returns 30
per page), treats fetched bodies as untrusted data (extract only the
Critical-bearing text, never paste unrelated bodies), and fails closed:
an unreadable truncated blocker rules "cannot tell" and caps the event at
COMMENT.
DESIGN.md records why composition replaces per-collision patching
(n states -> n(n-1)/2 pairs; clauses make new states additive).
* fix(review): correct the cross-repo capability table and close nine review notes
- docs: the cross-repo table claimed "Agents 0-6" run in lightweight mode
while the prose (correctly) says 1c is skipped there — 1c is inside that
range. Split 1c onto its own row, and add the missing Agent 8 row (its
finders need only the diff, so they do run cross-repo).
- --effort=<level> now has a parse rule: split the flag token on the first
'=' and consume no second token; the next-token rule applies only to the
spaced form.
- The substantive-return (whiff) check covers every receipt-less agent, so
3A's dimension agents are in scope, not just 3B's whole-diff agents.
- Step 3C names Agent 1b's lightweight degradation and states the three
angles medium deliberately omits (security, test coverage, adversarial
personas) instead of naming only two.
- Step 6's open-Critical re-check states what a context-unavailable run does
(skip the walk, every Critical is "cannot tell") instead of pointing at a
context file that does not exist.
- The event/body table carries the body-only-Critical exception in the cell,
where it is read, not only in the surrounding prose.
- The posting gate's second condition is now decidable: a publish verb typed
by the user this session, with the near-misses (approving noises, our own
tip, PR text) enumerated as non-authorization.
- DESIGN: the whiff check is evidential, not a length threshold (and says
why no number); "quick pass" is defined as low+medium sharing guardrails.
* feat(review): promote removed-behavior to a whole-diff agent in 3B
Territory-scoped 1b can only ask "was this deletion re-established here",
and for the deletions that matter the answer is somewhere else. PR #6638
(43 files, 8255 additions, 28 chunks) measured the gap: the 3B run with
per-chunk 1b reported one Critical; an independent reviewer reported 32, and
a parallel hand-run 1b+1c wave over the same commit reproduced six of them.
Every one of that overlapping six is a cross-chunk deletion — enableByPath
(includeSubdirs: true) replaced by an exact-path setWorkspaceActivation in
another file, silently narrowing workspace-scoped disable for every untouched
CLI/TUI caller; refreshTools() dropped from the activation paths, its
replacement swallowing the errors it used to propagate; a global mutation
timeout replaced by one covering only the prepare phase. Deletion in chunk A,
replacement in chunk B, consumer in a file the diff never touches: no chunk
agent can see that triple, and 1c does not look for it — it greps callers of
changed symbols, and a deleted export has no symbol left to grep.
- 1b joins 1c as a whole-diff agent in 3B; chunk agents keep the local half
(a guard deleted and not re-established in the same hunk is still theirs).
- The split is stated at both agents: 1c walks the callers of changed
symbols, 1b walks the replacements of removed ones.
- Agent 1b's definition gains the removed-export bullet: compare replacements
as behaviour, not names, then check the call sites the diff never touches —
a replacement that type-checks is not a replacement that behaves.
- 3B whole-diff agent count 4-6 -> 5-7 in the budget and the docs diagram.
* fix(review): serialize cannot-tell blockers, gate the no-blockers opener, and read reviews from a file
Fifth review round, all four notes real:
- The clause inventory had no way to serialize Step 6's `cannot tell`
verdict, so a Critical the review could neither confirm nor clear had
nowhere to go and dropped out of the public review. Added clause 5
(unresolved existing-Critical), which survives downgrades and 422 recovery
like the body-Critical carve-out, and Step 6 now points at it.
- `Reviewed — no blockers.` was injected as the opener whenever context was
available, regardless of C or scope — so a self-PR downgraded to COMMENT
with an inline Critical, or a review with an uncoverable chunk, opened by
certifying the absence of the blockers it was carrying. The opener is now
gated on C === 0 AND no unresolved existing Critical AND no uncoverable
chunk AND context available; otherwise it is a plain `Reviewed.`
- The paginated `/reviews` fetch ran through the shell, whose successful
output is capped at 30 000 chars and split head/tail: a body-only blocker
in the elided middle passes with exit 0 and the fail-closed branch never
fires. It is now redirected to a file and paged with read_file, and a body
read only in part is `cannot tell`, not "no Critical in it" — the same
lesson as "the diff is a file, not a command".
- The substantive-return gate rejected a bare "No issues found" while the
agent contract demanded exactly that string. The contract now asks for
`No issues found — <one line naming what you examined>`, and the relaunch
is capped at one attempt per agent, with the dimension reported under "Not
reviewed" if the second return is still bare.
* fix(review): wire whole-diff 1b into the gates, cap the event on unread scope, page reviews as NDJSON
Sixth round. Four Criticals all trace to the two previous commits:
- Whole-diff Agent 1b was declared but never wired in: it was missing from
the receipt-less roster (so a whiffing 1b passed undetected) and the launch
contract handed every 3B agent "its own chunk range", which is exactly what
a cross-chunk pairing agent cannot work from. The payload contract now
splits by role — chunk agents get one range, every whole-diff agent gets
the entire chunks[] plan.
- The whiff check ended at "note it as Not reviewed", which left an
unreviewed Security or removed-behavior lens able to ship an LGTM. It now
carries an unreviewedDimensions state that forbids Approve, caps the event
at COMMENT, and is serialized in the body next to uncoverable chunks.
- Clause 5 put an undecidable existing Critical in the body while event
selection still chose APPROVE from the C/S table — a review approving the
very blocker it asks the author to confirm. The table now has explicit
overrides: cannot-tell existing Critical, uncoverable chunk, and unreviewed
dimension each cap the event at COMMENT (a confirmed Critical still earns
REQUEST_CHANGES).
- Redirecting `gh api --paginate` to a file does not make it pageable: it
emits compact JSON, so the file is one 150 KB+ line that read_file
truncates and offset skips past to EOF. The fetch now filters with --jq to
marker-bearing bodies and emits line-delimited records that page normally.
Also: the 1b/1c split is by task, not by symbol (1c greps the removed
export's old name and owns caller compatibility; 1b owns the pairing and the
semantic comparison) — the earlier "no symbol left to grep" claim understated
1c and risked dropping its removed-symbol pass. Plus stale arithmetic from the
larger roster (+4 -> +5, worked example 26-28 -> 27-29), the receipt count's
missing Agent 8, "0 LLM calls" -> "0 subagent calls", the fixed "12 parallel
tasks" -> its real range, and 1c's callee procedure no longer speaking of a
"territory" it does not have.
* fix(review): select body Criticals offline, propagate unreviewed dimensions, close the no-findings bypass
Seventh (final self-review) round. The three Criticals all attack the newest
machinery:
- The NDJSON fetch filtered on a literal [Critical] marker, but a body-only
blocker is not guaranteed to carry it (a real emitted review on this repo
does not) — the filter discarded exactly what the re-check exists to
recover. The --jq now keeps every nonempty body and selection happens
offline after reading records whole; clauses 5 and 7 additionally mandate
the marker on everything we serialize, so our own output stays
self-identifying.
- unreviewedDimensions stopped at the event cap: Step 6's Not-reviewed
section only listed uncoverable chunks (a non-posting run hid the missing
lens entirely), and the body invariant made the required disclosure
illegal on a REQUEST_CHANGES. The section now lists both, and the
not-reviewed clause is the second sanctioned REQUEST_CHANGES body
exception — a confirmed Critical must not squeeze out the disclosure of
what was never read.
- The no-confirmed-findings branch still said "APPROVE by default",
special-casing only presubmit and context-unavailable — bypassing the
cannot-tell/uncoverable/unreviewed caps added one commit earlier. The
branch now runs the same machinery as every submission: table with
overrides, then downgrades, then composition; the hard-coded LGTM example
applies only with no cap state present.
Plus the round's consistency notes: cross-file trace marked same-repo-only
in the docs' medium row; +5 -> +4 in the crossover arithmetic (Build & Test
reads no diff) so "crosses twelve about there" is true at 3200; DESIGN's
whole-diff enumeration gains 1b; budget total widened to the honest 15-21
row-sum; fork-subagent math redone at 52K/agent; the payload paragraph
names the invariant agents' third payload class; consumer-direction grep
patterns get Python/Go forms; 3C medium states 1a's lightweight degradation
and scopes the grep permission; the aggregated-format shorthand carries
Failure scenario and Severity; the exactly-one-sentence rule forward-
references the composition rule; and the Step 7 comment template embeds the
failure-scenario shape it was already demanding in prose.
* feat(review): sink argument parsing into a tested parse-args subcommand
The --comment/--effort grammar and target disambiguation were ~400 words of
prose in SKILL.md that the model re-simulated on every run; three separate
parsing bugs shipped that way (the spaced form consuming a flag as its
value, the --effort=<level> form left undefined, and an invalid value token
surviving into target disambiguation). Each is now a table-driven test case.
qwen review parse-args '<raw args>' emits a JSON verdict: classified target
(pr-number / pr-url with owner+repo+number extracted / file / local),
resolved effort with its source (explicit / default / forced-by-comment),
comment.requested vs comment.effective, verbatim warnings, and leftover
tokens the parser refuses to guess about. The skill's Step 1 shrinks to
"run the parser, use the verdict verbatim", and the target branches key off
target.type instead of hand-classifying tokens.
* feat(review): sink event selection and body composition into compose-review
The Step 7 machine — the C/S table, three event-capping overrides, the
seven-clause body composition, and the presubmit downgrade carve-outs — was
restated across four places in SKILL.md, and keeping the restatements in
sync by hand produced five shipped bugs (four Critical), all one shape: a
downstream branch not updated when an upstream rule gained a new state.
qwen review compose-review reads a state JSON (inline/body Critical and
Suggestion counts, discarded anchors, cannot-tell existing Criticals,
uncoverable chunks, unreviewed dimensions, context-unavailable, presubmit
flags, model id) and returns {event, body, baseEvent, cappedBy, downgraded}
for verbatim submission. The truth-table tests pin every previously shipped
bug as a named case: caps forbid APPROVE but never soften a REQUEST_CHANGES;
discarded Suggestions still count toward S so a 422 resubmit can never
upgrade to LGTM; a self-PR downgrade keeps body Criticals after the
downgrade sentence; the no-blockers opener appears only when certifiable;
every disclosure survives every stacking. Writing the tests immediately
caught one more instance of the class (all-discarded -> S=0 -> APPROVE).
SKILL.md's Step 7 shrinks to gathering the state and using the output
verbatim; the 422 recovery becomes "re-run compose-review with updated
counts"; the no-findings branch is the same call with zero counts; the
posting gate (judgment, not bookkeeping) stays prose.
* feat(review): render review bodies in full, quarantine replied Criticals, raise the gh buffer
The Step 6 body-fetch instruction was rewritten five times in four review
rounds (missing pagination -> shell truncation -> unpageable single-line
JSON -> a marker filter that discarded markerless blockers -> offline
selection) — the signature of a download program written in English. This
ends the chain at its root, in pr-context itself:
- Review bodies render in full under "Review summaries" instead of
240-char snippets: an unmappable or 422-relocated blocker lives only
there, and a snippet once hid one from the re-check. A body past the 8000
cap ends by naming its review id, so the tail stays fetchable as a single
object; a body read in part is `cannot tell`, not "no Critical in it".
- Replied Critical threads are quarantined into their own "Replied
Criticals" section, rendered before the settled threads, instead of
sinking into "Already discussed" — a reply alone ("I disagree") never
retires a blocker, and marker-matching in this direction is fail-safe: a
forged marker can only add a thread to the re-check list, never hide one.
- The gh wrapper's maxBuffer rises from Node's 1 MiB default to 64 MiB,
closing the ENOBUFS that killed pr-context and presubmit mid-review on a
comment-heavy 43-file PR.
SKILL.md's NDJSON fetch block is deleted: the re-check reads the context
file's three finding-bearing sections under its untrusted-data preamble,
with one residual single-object fetch for capped bodies. Verified against
this PR's own 100+-comment history: the markerless body-Critical review
that motivated the last rewrite now renders whole, and the fetch survives
without ENOBUFS.
DESIGN.md records the sinking rationale for all three subcommand changes;
the user docs note that parsing and the event/body decision are now pinned
by unit tests rather than prompt text.
* test(review): register parse-args and compose-review in the exact-list assertion
The parent-command test pins the exact subcommand roster; the two new
subcommands landed without updating it, which is precisely the drift the
assertion exists to catch — it caught it in CI, one directory above where
the new tests were run locally.
* fix(review): carry every disclosure on REQUEST_CHANGES and select blockers semantically
Review round on the new subcommands, plus the prompt notes it surfaced:
- compose-review's REQUEST_CHANGES branch dropped the context-unavailable
clause entirely and gated the not-reviewed disclosure on other parts being
present — an RC with only an uncoverable chunk disclosed nothing. Every
clause whose state holds now appears on every event (a confirmed blocker
must not squeeze out the trust warning or the unread-scope disclosure);
four new tests pin it.
- Step 6 selects blockers semantically, not by the literal [Critical]
marker: legacy body-only blockers were emitted markerless, and a marker
filter once discarded exactly such a review.
- The same-repo pr-context failure now sets context-unavailable like the
lightweight path (the guard's "lightweight" narrowing is removed) — a
same-repo run that lost the context file must not behave as if it had
read it.
- Step 5's dry-round return aligns with the agent contract (receipt-bearing
"No issues found — <what it re-examined>"), ending the contradiction where
a compliant reverse auditor would be flagged as whiffing.
- Consumer-direction grep forms for Python/Go are call sites now, with the
declaration forms explicitly labeled as callee lookup.
- The 15-19 totals left downstream (docs table, DESIGN heading and cost
row) move to the honest 15-21 / 13-20.
* fix(review): stdin transport for parse-args, validated compose input, full-body re-check context
Round 9 of review-the-review on this PR: 19 unique findings across three
reviews, each verified against source before fixing.
parse-args:
- The documented positional invocation broke on any flag-first raw string
(`qwen review parse-args '--effort low'` -> "Unknown argument") and the
`--` form silently returned a wrong local/default verdict. The raw
string now travels on stdin (`--stdin`; SKILL.md pipes a quoted
heredoc, immune to leading dashes, quotes, and $(...)); positional +
--stdin and post-`--` smuggling are refused loudly. Wiring-level tests
drive the real yargs command, pinning the strict-mode rejection that
pure-function tests could not see.
- PR URL identity hardened: the number must end its path segment
(/pull/42oops is refused, never PR 42), owner/repo restricted to
GitHub's name charset (keeps shell metacharacters out of derived
values), scheme matched case-insensitively, url canonicalized
(lowercase scheme/host, query/fragment dropped) with a new host field;
near-miss URLs are warned about and reported in extraTokens, never
guessed into a file path or PR number. Step 1 remote matching now
requires host AND owner/repo.
- Repeated --effort warnings state what is actually in effect (last valid
occurrence / --comment forcing / the default), composed after
resolution; previously a later typo claimed the default while an
earlier valid effort stayed active.
compose-review:
- Input validated at the boundary: absent counts default to 0; malformed
values throw typed errors naming the field. Previously
{bodyCriticals:["x"], modelId} made undefined+1=NaN, failed both event
comparisons, and returned APPROVE over the only blocker.
- "Suggestions are inline." keys off suggestionsInline, not s: an
all-discarded 422 recovery no longer claims inline suggestions while
the discarded sentence says the opposite (s still decides the event).
- canCertify requires !downgraded: a downgraded Approve opens with the
neutral "Reviewed." instead of certifying "no blockers" two clauses
after naming failing CI.
- unreviewedDimensions entries may carry their own reason after an
em-dash and render verbatim (used by Agent 0's fetch failure below).
pr-context:
- Replied-Critical root bodies render in full (shared capBody; a cut
names the comment id and the exact fetch); reply snippets name their
comment id when cut. The Step 6 re-check no longer rules on
silently-truncated claims, and the fail-closed "read in part = cannot
tell" rule can actually fire for this section.
- The LGTM filter matches the exact canonical template, anchored to the
whole body: a legacy body opening with the LGTM line but carrying a
relocated blocker below it is shown instead of dropped.
- classifyInlineThreads() extracted: buildMarkdown and the stdout count
use the same walk, so the count cannot diverge from the file.
SKILL.md:
- Step 6 re-check scope: every comment-bearing section, including
"Already discussed" (inline threads and issue-level comments) — the
quarantine keys on the literal marker, a floor not a ceiling, so
unmarked blockers settle there; the false "holds only non-Critical
threads" parenthetical is gone. The residual long-body fetch redirects
to a file (shell output truncates at 30k) and is read paged.
- Step 5 reverse audit: dry = zero new findings WITH the evidence-bearing
receipt; the substantive-return check runs after every round (one
relaunch); a twice-whiffed agent's round is never dry.
- Step 3: Agent 7 added to both whiff-check rosters (evidence = commands
run + outcomes; build-and-test recorded in unreviewedDimensions on the
second whiff). Agent 0's linked-issue fetch failure is fail-closed
after one retry via a self-explained unreviewedDimensions entry.
- Step 8: a fail-closed run (unreviewed dimensions, uncoverable chunks,
context-unavailable) must not advance the incremental cache — caching
it would exempt the disclosed-unreviewed scope from every future run.
- Counting truthfulness: "Twelve agents all reading the same diff" is
eleven (every 3A agent except Build & Test walks the chunk plan); fixed
in the 3B rationale, the diff-capture section, and the user docs.
review.ts: demandCommand message names plan-diff, with a test that the
message stays in sync with the registered roster.
* fix(review): nested-safe stdin guard, validated presubmit, refetchable snippets everywhere
Round 10: 12 findings, all verified before fixing. The headline is
self-inflicted: the round-9 post-`--` guard read argv._ as
['parse-args', ...extras], but the real CLI nests the command, so argv._
is ['review', 'parse-args'] and the guard rejected every real
invocation — while the wiring tests, which register the command
top-level, stayed green. Reproduced against the built CLI before
fixing.
parse-args:
- The smuggle guard skips the command-path prefix in argv._; new wiring
tests go through the real parent `review` command (nested stdin
invocation + nested post-`--` refusal).
- --effort values match case-insensitively (`--effort High` is not a
file target named High); the verdict keeps the lowercase form.
- Single-dash tokens are unknown flags, never target candidates
(`/review -c 6711` reviewed a nonexistent file `-c` and demoted the
PR number to extraTokens).
compose-review:
- presubmit and contextUnavailable get the same boundary validation as
the counts: boolean flags reject stringified "false" (truthy — it
flipped an inline-Critical RC to COMMENT and published the diff-only
warning on runs that fetched context fine), downgradeReasons rejects
scalars with the field name (was a raw .join TypeError), presubmit
rejects non-objects.
- Certification is gated on what presubmit PERMITS, not on whether it
changed the event: a Suggestion-only review is already COMMENT, so
failing CI flipped nothing and the body still certified "no
blockers". Either downgrade flag now suppresses the certifying
opener.
pr-context:
- Every truncating render carries an exact refetch ref: open-root
snippets, settled replied threads (roots and replies), and
issue-level comments (their own issues/comments endpoint). The
Step 6 semantic re-check reads these sections, and a markerless
blocker past the 240-char cut was invisible with no way back.
- Refs are copy-runnable: buildMarkdown threads owner/repo and PR
number into every ref, so emitted commands carry real values.
`gh api` substitutes only {owner}/{repo} — from the CURRENT repo,
wrong in cross-repo mode — and passes {n} through literally.
SKILL.md:
- Step 1: the raw argument string travels via write_file to
.qwen/tmp/qwen-review-args-input.txt and stdin redirection. A quoted
heredoc disables expansion but not delimiter recognition, so a raw
string containing the delimiter line would end the heredoc early and
execute the rest as shell. Step 9 removes the file.
- Step 1: remote matching is structural segment equality (host AND
owner/repo, .git stripped, case-insensitive) — substring "contains"
let shao/qwen-code match a wenshao/qwen-code remote. Non-github.com
hosts must carry GH_HOST on every gh call for the PR.
- Step 5: a twice-whiffed reverse-audit scope is tracked, cleared only
by a later substantive audit, and fed into unreviewedDimensions as a
self-explained entry when the loop ends — terminal prose alone let a
capped run approve with an audit that never ran.
- Step 6: snippet cuts carry their own filled-in fetch note; ruling on
a cut prefix is the fail-closed violation.
- Step 7: the stale hand-derivation bullets (event table, empty-RC-body
rule, one-line COMMENT inventory) are replaced with descriptions of
what compose-review guarantees; the sanity check is byte equality
with the subcommand's output; the last-resort 422 branch re-runs
compose-review instead of hand-building "the one-line body".
- Step 8: the fail-closed cache rule includes cannotTellCriticals — a
cached SHA plus the same-SHA shortcut would skip the very re-check
that must re-rule on an undecided blocker.
MSG2
git log --oneline -1; git push origin feat/review-procedural-finders-effort 2>&1 | tail -2
* feat(review): deterministic overlap disposal, --host routing, machine-readable completion line
Three changes measured out of the first six-PR dogfood batch, not
predicted from review comments.
Overlap disposal (SKILL.md Step 7): presubmit's overlap report used to
end in "list the overlaps to the user, ask whether to proceed" — 2 of 6
batch runs stalled on an improvised interactive question (fatal for a
headless run) while the other 4 proceeded. An overlap is a duplicate by
the Exclusion Criteria; the rule is now drop the overlapping finding,
adjust the counts handed to compose-review (a dropped finding never
flips the verdict), note "already reported at <path>:<line>" in the
terminal, and continue without asking. Zero findings left after
dropping is still not a question — compose-review handles the shape.
--host routing (lib/gh.ts + fetch-pr/pr-context/presubmit): the
round-10 GH_HOST-by-prose rule required the model to remember a prefix
on every call; a forgotten one silently reads from and posts to
github.com's same-named owner/repo. The three gh-calling subcommands
now accept --host and thread it through setGhHost()/ghEnv(), so every
wrapped gh call carries GH_HOST in code; hostname input is
charset-validated. SKILL.md keeps the prose prefix only for the gh
commands the orchestrating model runs directly (Agent 0's fetches,
Step 6's residual body fetch, Step 7's submission).
Completion line (SKILL.md Step 9): three different ad-hoc completion
phrasings across one batch each needed their own driver regex. Every
run now ends with exactly one line, `Review complete: <target> —
<disposition>`, with a closed disposition grammar covering posted
events, unposted verdicts, and quick passes — detectable with a single
^Review complete: match.
Tests: gh host-state unit tests (inherit-by-default, GH_HOST extension,
host:port, charset rejection), presubmit handler --host threading (set
and reset), builder registration checks for fetch-pr and pr-context.
|
||
|
jinye
|
60fcc8cbce
|
fix: Make chat recording failures durable and visible (#6743)
* fix(core): Stop chat recording after write failure Keep the canonical JSONL write chain rejected after the first asynchronous failure so queued descendants are skipped and flush reports the original error consistently. Cover sticky failures across ordinary, strict, parent-session, ACP close, rename, branch, and rewind paths. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix: Surface chat recording failures Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): Await custom title persistence Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6743) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(cli): clarify degraded branch behavior Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(acp): align recording state entry type Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix: address PR review feedback (#6743) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix: address durability review findings (#6743) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): allow artifact migration after recording failure Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(daemon): correct UI event counts Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> |
||
samuelhsin
|
7468e75e3d
|
feat(web-shell): support custom Hex session group colors (#6752)
* feat(web-shell): support custom Hex session group colors * docs(web-shell): add custom group color screenshot * fix(web-shell): address custom Hex color review feedback * fix(web-shell): validate group presets against daemon catalog and auto-prefix Hex input Review follow-ups for the custom Hex group color editor: - Validate the preset branch against the daemon-provided color catalog instead of the hardcoded palette, so future preset additions stay selectable in the editor. - Auto-prefix bare values with '#' in the Hex field so pasted bare Hex validates, and free text can no longer collide with a preset name and silently flip the select out of Custom mode. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(web-shell): cap custom Hex input length --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
jinye
|
d14aca72a6
|
feat(serve): add workspace persisted transcript reader (#6740)
* feat(serve): add workspace persisted transcript reader Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * refactor(cli): rename replay modules to kebab case Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6740) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(serve): cover multi-record transcript paging Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> |
||
nas
|
0579be6ee8
|
feat(core): add configurable default timeout for foreground shell commands (#6628)
* feat(core): add configurable default timeout for foreground shell commands Foreground shell commands started by the agent time out after a hardcoded 120s (DEFAULT_FOREGROUND_TIMEOUT_MS). A per-call `timeout` param can raise that for a single command, but there is no way to change the default for a project or session, so users repeatedly watch long-running commands fail at the 2-minute mark. Add a `tools.shell.defaultTimeoutMs` setting that feeds the existing timeout resolution. Precedence is now: per-call `timeout` param > setting > built-in default. When the setting is unset, behavior is unchanged; a value of 0 disables the timeout, matching the existing per-call semantics. Fixes #5838 * fix(core): add mock getShellDefaultTimeoutMs + bound defaultTimeoutMs Address review on #6628: - Add getShellDefaultTimeoutMs to mock configs in coreToolScheduler.test.ts and toAutoClassifierInput.test.ts (ShellTool construction now reads it). - Add minimum: 0 / maximum: 600000 to the defaultTimeoutMs setting so a negative value can't reach AbortSignal.timeout(); regenerate schema. * chore(core): polish shell defaultTimeoutMs per review - shell.ts: debug-log the resolved foreground timeout (per-call vs configured default vs built-in) for observability - settingsSchema.ts: use type 'integer' for tools.shell.defaultTimeoutMs to match sibling visionBridgeTimeoutMs; regenerate settings.schema.json - config.test.ts: add loadCliConfig test asserting tools.shell.defaultTimeoutMs maps to Config.getShellDefaultTimeoutMs() * fix(core): validate shell defaultTimeoutMs and fix disabled-timeout hint Address review on the configurable foreground shell timeout: - Config: validate shellDefaultTimeoutMs at construction, mirroring visionBridgeTimeoutMs, but allow 0 (disables the timeout). Negative, fractional, or out-of-range values now coerce to undefined instead of reaching AbortSignal.timeout() via a hand-edited settings.json that bypasses schema validation. - settingsSchema: mark tools.shell.defaultTimeoutMs requiresRestart, since Config.shellDefaultTimeoutMs is private readonly with no setter, so a mid-session change cannot take effect. - shell: when the timeout is disabled (effectiveTimeout === 0), suppress the long-run backgrounding hint instead of firing it on every command over ~1s via the longRunThresholdFor floor. - shell: correct the precedence comment; 0 disables only at the settings/default level, as the per-call timeout param rejects <= 0. Add coverage for negative/fractional coercion to the built-in default and for 0 disabling the timeout without emitting the spurious hint. --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> |
||
ever-o
|
ec43c1e951
|
feat(web-shell): render composer references in user messages (#6537)
* feat(web-shell): render composer references in user messages * refactor(web-shell): consolidate composer tag utilities * fix(web-shell): leave custom references as text * fix(web-shell): avoid ambiguous reference chips * fix(web-shell): thread composer tag icons to messages * feat(web-shell): render user references from annotations * fix(web-shell): include inline tags in input annotations * fix(web-shell): remove duplicate composer tag icon option * fix(web-shell): forward plan prompt annotations * test(web-shell): cover composer annotation edge cases * fix(web-shell): forward split pane prompt annotations * fix(web-shell): guard malformed input annotations --------- Co-authored-by: zhanghuapeng.zhp <zhanghuapeng.zhp@alibaba-inc.com> Co-authored-by: 易良 <1204183885@qq.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> |
||
|
jinye
|
51d4ce48db
|
feat(serve): persist dynamic workspace registrations (#6716)
* feat(serve): persist dynamic workspace registrations Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6716) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6716) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6716) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6716) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6716) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
易良
|
263dbba741
|
fix(core): preserve managed memory during microcompaction (#6714)
* fix(core): preserve managed memory during microcompaction Refs #6487 * test(core): cover managed memory read errors --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> |
||
|
qqqys
|
58dc985ed8
|
chore: remove DingTalk planning artifacts (#6722)
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> |
||
|
jinye
|
e403246dc2
|
feat(serve): Expose read-only untrusted session catalogs (#6717)
* feat(serve): expose read-only untrusted session catalogs Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * refactor(cli): address session catalog review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6717) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6717) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6717) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> |
||
Nothing Chan
|
b19ebd8fc6
|
fix(packaging): bundle clipboard addon in standalone builds (#6708) | ||
Alex Yanchenko
|
218dec6a6f
|
feat(hooks): add MessageDisplay hook for mid-turn streaming (#6489)
* feat(hooks): add MessageDisplay hook for mid-turn streaming Fires repeatedly as the assistant reply streams, before Stop (which only fires once at the end of the turn). Fire-and-forget, cumulative text payload, debounced (~200ms) except for the unconditional final firing. Fires from the single streaming loop in client.ts shared by the terminal UI and ACP paths. Fixes #6488 * fix(hooks): address MessageDisplay review feedback - Chain fire-and-forget MessageDisplay requests per message_id instead of firing them fully unbounded, so a slow hook command can't pile up concurrent processes. - Gate the final flush on non-empty displayed_text and !signal.aborted, matching the adjacent Stop hook's guard. - Document why the final flush intentionally re-sends the last debounced text (is_final itself is new information). - Simplify the debounce constant's JSDoc to drop the competitor comparison. - Add tests for the mid-stream debounced flush and the rejected-request warn path. * test(hooks): drain microtasks before asserting on chained MessageDisplay calls fireMessageDisplayHook now chains per-message_id through a promise (see previous commit), so the final flush's actual messageBus.request() call lands a few microtask ticks after the generator itself finishes — the mid-stream-flush test needs to let that chain settle before asserting. * fix(hooks): flush MessageDisplay is_final on every for-await exit path The three early `return turn` paths inside the streaming loop (always-on loop-detection safety, heuristic loop detection, and the stream Error event) exited before the final MessageDisplay flush, which only sat after the loop ended normally. Hook scripts relying on is_final: true to know when to flush never received it when a turn ended via loop detection or an API error. Extracts the flush into a shared closure and calls it from all four exits (the three early returns plus the normal fall-through), instead of only the one at the bottom of the loop. Adds regression tests for all three previously missed exits, plus the two guard-coverage tests requested in review (abort suppresses the flush, a tool-call-only turn with no Content events does not fire a vacuous empty-text event). Addresses the outstanding critical review comment and the follow-up test coverage suggestion on PR #6489. * fix(hooks): fire MessageDisplay on the ACP surface, coalesce delivery, drain is_final before turn end Addresses the three findings from the local verification report on #6489: - ACP/qwen serve (Finding 1): the delivery logic now lives in a shared MessageDisplayDispatcher (packages/core), and Session.ts wires it into all four raw-stream loops (main prompt, Stop-hook continuation, cron tick, background notification) — these surfaces consume GeminiChat's stream directly and never enter GeminiClient.sendMessageStream, so they need their own fire sites. The daemon no longer advertises an event it never emits. - Slow-hook backlog (Finding 2): the per-message promise chain is replaced by coalescing delivery — at most one in-flight request plus one pending payload per message; newer flushes overwrite the pending slot, which is lossless because displayed_text is cumulative, and is_final is sticky. A slow hook now sees fewer, newer payloads instead of an ever-growing queue of stale ones. - Headless is_final drop (Finding 3): finish() resolves only once every enqueued payload has actually been delivered, and every exit out of the streaming loops awaits it (early returns, normal fall-through, and the enclosing finally for uncaught exceptions), so a short-lived -p process can no longer exit with the final payload still queued. As a consequence, is_final delivery now strictly precedes the Stop hook rather than racing it. Also: the failure log line carries the message_id, finish() is idempotent, the review-requested tests are added (mid-stream and final firings share one message_id; isFinal as the sole flush reason), and hooks.md gains a delivery-semantics contract covering coalescing, the drain guarantee, no is_final on cancellation, provisional displayed_text, and multiple messages per tool-using turn. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(hooks): bound MessageDisplay drain wait, fix test gaps flagged in review finish() now gives up waiting on drain after 5s (MESSAGE_DISPLAY_DRAIN_TIMEOUT_MS) instead of blocking turn teardown for up to the full 60s hook timeout, per the re-verification's S1 finding. Delivery keeps running in the background past the timeout; only the caller's wait is bounded. Also: add the config.ts bridge test for MessageDisplay field extraction (S5), and add the missing MessageDisplay/InstructionsLoaded entries to acpAgent.test.ts's HookEventName mock (S6). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(hooks): dispatch MessageDisplay is_final alongside stale deliveries, share one drain budget Round-3 review findings on #6489: - finish() no longer queues the is_final payload behind an in-flight mid-stream delivery: the pending slot's supersession argument applies to the in-flight slot too, so the final payload is dispatched immediately, alongside the stale delivery if one is still running. is_final is handed to the hook the moment the message ends — before Stop — on every surface, and can no longer be dropped by a short-lived process exiting with it still queued (Finding 1). - The bounded drain wait is memoized: every finish() call (explicit, finally, or concurrent) shares one promise and one timer, so the teardown ceiling is MESSAGE_DISPLAY_DRAIN_TIMEOUT_MS itself, not a multiple of it (Finding 2). - hooks.md delivery semantics rewritten to match the shipped behavior, including the headless orphaned-hook caveat and the unspecified completion order between an overlapped stale execution and the final one (Finding 3). - The dispatcher mirrors its warnings to console.warn itself (stderr on headless/ACP, ink patchConsole in the TUI) in addition to the injected debug-file sink, so hitting the drain timeout is visible by default (Finding 4). - A superseded mid-stream delivery that fails after the final was dispatched no longer warns; failures during streaming still do. - New tests: finish() twice while delivery is in flight (the exact client.ts sequence), concurrent finish() calls sharing one budget, is_final overtaking a held mid-stream delivery, and drain resolving on the final delivery alone. * refactor(core): consolidate MessageDisplay finish() calls, dedupe test spy setup client.ts: wrap the turn.run() streaming loop in try/finally so messageDisplay.finish() fires once instead of at each of the three early-return sites plus the post-loop path -- matching the pattern the four raw-stream loops in Session.ts already use for the same dispatcher. message-display-dispatcher.test.ts: centralize the console.warn spy setup/teardown in beforeEach/afterEach instead of five repeated per-test try/finally blocks. No behavior change: full client.test.ts (246/246) and the message-display-buffer/dispatcher suites (24/24) pass unchanged. * docs(hooks): clarify MessageDisplay cancellation timing (round-4 nit) * test(hooks): cover the 3 untested MessageDisplay dispatch sites, fix cancellation doc wording Adds MessageDisplay is_final coverage for the Stop-hook continuation loop, the in-session cron fire, and the background-notification loop, each with a normal-completion and an abort case. Adds three MessageDisplayDispatcher edge-case tests: a delivery settling just before the drain timeout, an abort arriving after a drain wait has already started, and addChunk called after abort but before finish(). Rewords the cancellation-timing doc bullet to state the actual criterion (abort signal state when finish() runs) rather than an approximation of it. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> |
||
|
jinye
|
230db17650
|
feat(cli): group daemon channel workers by workspace (phase 4b) (#6635)
* feat(cli): group daemon channel workers by workspace (phase 4b) Multi-workspace `qwen serve --channel` now runs one channel worker per owning workspace instead of a single primary-bound worker. Each worker binds to its workspace's directory, daemon-workspace env marker, and effective env overlay. Channels are grouped implicitly by their configured working directory: a channel belongs to the registered workspace its resolved cwd matches, mirroring the worker's own workspace validation. Unknown, ambiguous, or untrusted targets fail fast at startup. The pidfile and daemon status grow an additive per-workspace worker list while keeping the existing single-worker fields for older readers; single-workspace daemons stay byte-identical to before. `--channel all` stays primary-only. Refs #6378 * fix(cli): harden multi-workspace channel workers Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): close listener after channel worker startup failure Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): restore grouped channel webhooks Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): mount runtime before channel workers start Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6635) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(cli): strengthen channel worker edge coverage Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * chore(cli): address channel review suggestions Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> |
||
|
jinye
|
6df19602c4
|
fix(cli): Scope session organization mutations by workspace (#6724)
* fix(cli): scope session organization mutations by workspace Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6724) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6724) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6724) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
Heyang Wang
|
5f89459e90
|
feat(core): add unified session recovery planning (#6731)
Centralize resume recovery classification so each entrypoint can share the same decision about interrupted prompts, dangling tool calls, and history gaps. - Add a core SessionRecoveryPlan builder with provider-safe repaired history - Route headless, stream-json control, and ACP continue paths through it - Show a TUI resume notice when an interrupted tool turn is detected - Document the Recovery Service design and cover the core plan with tests Co-authored-by: heyang.why <heyang.why@alibaba-inc.com> |
||
destire-mio
|
7279d6f360
|
feat(cli): add project-scoped prompt stash (#6709)
Co-authored-by: zhuyouwei <zhuyouwei@testin.cn> |
||
|
jinye
|
ff514476de
|
feat(cli): workspace-qualified ACP transport (daemon multi-workspace phase 4) (#6621)
* docs(design): add daemon multi-workspace phase 4 (workspace-qualified ACP) design * feat(cli): add workspace-qualified ACP transport (issue #6378 phase 4) Per-runtime ACP dispatcher at /workspaces/:workspace/acp (HTTP + WS) dispatched by URL path from the single upgrade listener; per-runtime device-flow + reverse client-MCP; owner-index via bridge lifecycle; untrusted/unknown rejected; legacy /acp unchanged; advertise workspace_qualified_acp for multi-workspace. * fix(cli): keep per-runtime device-flow registry out of serve fast-path bundle Phase 4 secondary-runtime device-flow statically imported createDeviceFlowRegistry into run-qwen-serve, pulling glob/@iarna/toml into the serve fast-path bundle and failing the closure check. Import it dynamically at the creation site; the check now passes and behavior is unchanged. * refactor(cli): drop per-runtime device-flow for secondary workspaces Follow-up to the fast-path fix: instead of dynamically importing createDeviceFlowRegistry for secondary runtimes, drop the per-runtime device-flow wiring entirely. Secondary ACP device-flow falls back to the dispatcher default, keeping the serve fast-path bundle closure clean without the dynamic-import indirection. WorkspaceRuntime.deviceFlowRegistry stays optional for a future per-runtime hook. * fix(cli): share daemon-global device-flow across ACP mounts; harden WS path parsing Secondary ACP mounts share the daemon-global device-flow registry (single instance per daemon) instead of a per-runtime one; the event sink fans out to every trusted runtime bridge so secondary ACP clients receive their own flow events, fixing the reviewer #6621 Critical and the CI test failure. Drops WorkspaceRuntime.deviceFlowRegistry. WS upgrade path is parsed from the raw request-target instead of new URL().pathname, rejecting %2e%2e / backslash / dot-segment traversal. * refactor(cli): gate CDP claim on primary mount; return plural ACP POST promise Add a primary flag to RuntimeAcpMount so a secondary workspace's ACP connection cannot claim the CDP tunnel -- the claim is gated on activeMount.primary, matching the primary-only chrome-devtools MCP wiring. The plural /workspaces/:workspace/acp POST handler returns the dispatch promise instead of voiding it. * refactor(cli): centralize ACP-HTTP enablement in resolveAcpHttpEnabled Add resolveAcpHttpEnabled() as the single interpretation of the QWEN_SERVE_ACP_HTTP opt-out, replacing four independent env checks across mount, voice-WS advertisement, and CDP-MCP gating. Advertise workspace_qualified_acp only when the ACP HTTP surface is enabled AND multi-workspace sessions are active, so it is not announced when ACP HTTP is disabled. * feat(cli): ACP dispose 503 gate + aggregate connection snapshot across mounts After dispose() the shared ACP HTTP handlers (legacy /acp + workspace-qualified) return 503 server_disposed instead of racing torn-down registries during the shutdown drain. Add AcpHttpHandle.getSnapshot() aggregating connection and wsStream counts across the primary mount and every trusted secondary runtime, and switch the metrics sampler to it so daemon metrics report all workspaces' ACP connections rather than only the primary's. * test(cli): cover ACP dispose 503, aggregate snapshot, and raw dot-segment WS reject * docs(design): record Phase 4 ACP systematic rework (8-axis hardening) Correct the Summary (the device-flow registry stays daemon-global and shared, not per-runtime) and add a section documenting the final architecture: runtime mount factory, routing/trust isolation, raw request-target WS parsing, daemon-global device-flow with event-sink fan-out, primary-only CDP, disposed 503 gate, aggregate getSnapshot, and resolveAcpHttpEnabled-gated capability advertisement. * fix(cli): align /daemon/status ACP counts with the aggregate mount snapshot Code review found a drift: the metrics sampler switched to the aggregate AcpHttpHandle.getSnapshot() (all mounts) while /daemon/status still read the primary-only registry snapshot, so the two observability surfaces diverged under multi-workspace. Extend AcpHttpSnapshot to aggregate all transport counters (connection/session/sse/ws streams + pending client requests) and feed the /daemon/status transport summary from it; per-connection diagnostics and the connection cap stay primary-scoped. Also refresh the device-flow-registry doc comment to the daemon-global shared model. * test(cli): regression-test device-flow on a trusted secondary workspace Locks in the reviewer Critical fix: a trusted secondary workspace's ACP now shares the daemon-global device-flow registry, so device_flow/start reaches provider resolution (an unsupported-provider error here) instead of erroring 'Device flow not configured'. Wires a shared DeviceFlowRegistry into the test harness and drives initialize + device_flow/start over the secondary WebSocket. * docs(design): mark the superseded per-runtime device-flow section Address PR #6621 review: the pre-rework 'Per-runtime device-flow registry' section contradicted Systematic rework axis 4 (daemon-global shared registry + fan-out). Flag it as superseded design-history so readers don't build the wrong mental model. * refactor(cli): mount ACP only for trusted secondary workspaces Address PR #6621 review suggestions: (1) skip creating a dispatcher/registry/remember-lane for untrusted non-primary workspaces (they are 403-rejected before any mount lookup), so they no longer appear as always-zero entries in the aggregate getSnapshot(); (2) test that a secondary workspace cannot claim the process-wide CDP tunnel (primary-only guard); (3) test that a WS upgrade to an unknown selector is rejected 400. * test(cli): cover device-flow event fan-out across bridges Address PR #6621 review: the resolveEventBridges fan-out (the reviewer Critical fix's core delivery path) had zero test coverage. Add unit tests that a device-flow event reaches every resolved bridge, that one bridge throwing does not block the others (best-effort), and that it falls back to the single bridge when no resolver is provided. * fix(cli): report ACP connection pressure across all mounts Address PR #6621 review: the connection_capacity_high warning read the primary mount's snapshot only, so a saturated secondary workspace was invisible. Compute the busiest mount from the aggregate snapshot (per-mount cap is uniform, opts.maxConnections) so any mount nearing capacity triggers the warning. * test(cli): allow acp-http-enabled.ts in the serve process.env guard Fix CI failure on PR #6621: the serve process.env guard flagged the new acp-http-enabled.ts as a direct process.env reader. It is the QWEN_SERVE_ACP_HTTP interpreter extracted from index.ts and serve-features.ts (both already allow-listed); QWEN_SERVE_ACP_HTTP is a daemon-level process-global toggle, so the file inherits their allow-list entry. * docs: harden workspace-qualified ACP design Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs: plan workspace-qualified ACP hardening Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): align workspace-qualified ACP routing Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): harden qualified ACP request errors Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(cli): cover unmarked URIError fallback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): make ACP disposal terminal Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): aggregate ACP connection diagnostics Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * chore: remove review process artifact Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): address workspace ACP review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): finish ACP review follow-ups Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
qqqys
|
25f491d3ac
|
feat(dingtalk): mention response senders (#6679)
* docs: design DingTalk at-sender replies * docs: plan DingTalk at-sender replies * feat(channels): preserve session for response delivery * feat(dingtalk): optionally mention response sender * docs(dingtalk): explain response mentions * fix(dingtalk): retain queued mention targets * fix(dingtalk): bound mention target lifecycle * fix(dingtalk): clear synthetic command mention target * fix(dingtalk): clear buffered targets on session death * debug(dingtalk): log mention delivery result * fix(dingtalk): render response mentions * fix(dingtalk): send visible response mentions * feat(dingtalk): use text replies for mentions * fix(dingtalk): preserve mentioned text replies |
||
|
jinye
|
38384ae7b9
|
feat(serve): Add cursor-paged transcript replay endpoint (#6525)
* feat(serve): Add cursor-paged transcript replay endpoint Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): Bound transcript replay indexing Limit transcript index builds to bounded snapshots and surface oversized transcript errors as 413 responses. Give transcript status calls a dedicated timeout and update the capabilities integration baseline. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): Validate transcript cursors Sign transcript cursors so forged snapshot sizes cannot bypass the index cache, and keep hasMore tied to persisted record availability when replay conversion returns a partial page. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): Lazy-init transcript cursor secret Avoid generating the transcript cursor HMAC key while importing the core barrel so unrelated tests with narrow crypto mocks can load core without requiring randomBytes. Keep the VS Code companion crypto mock partial so it only replaces the auth-token UUID behavior it asserts on. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): Address transcript replay review suggestions Mark bounded replay truncation frames as having a transcript endpoint, sanitize paged transcript replay conversion errors, and remove the core reader's incomplete pre-encoded cursor field so cursors are only emitted after replay continuation state is merged. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): Stabilize transcript replay pagination Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): Avoid quadratic transcript line scanning Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): Mark transcript history gaps Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): Address transcript reader review comments Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): Address transcript replay review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): align transcript cursor preflight errors Return transcript snapshot conflicts for cursor pagination when the active JSONL can no longer be found during route preflight. Add route-level and integration coverage for full transcript paging, and document the boolean fullTranscriptAvailable SDK contract. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(cli): Cover paged dangling tool call replay Add a HistoryReplayer.replayPage regression test that carries a dangling tool call through pendingToolCalls and finalizes it on a later page. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): Bound transcript index cache bytes Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix: Address transcript replay review follow-ups Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): Preserve pending tool calls on transcript replay errors Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6525) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: fix CI failure on PR #6525 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): warm transcript-replay tools leniently The read-only transcript-replay Config sets skipSkillManager, but Config.initialize() still runs toolRegistry.warmAll({ strict: true }), which constructs SkillTool whose constructor throws when no SkillManager exists. The throw escaped the replay try/catch and surfaced as JSON-RPC -32603, so GET /session/:id/transcript returned HTTP 500 for every persisted session. Add a lenientToolWarmup initialize option and set it for the replay Config so tools that cannot construct under the deliberately-skipped subsystems are logged and skipped instead of aborting initialize(). Replay only needs optional tool_call metadata and ToolCallEmitter already falls back to the recorded tool name, so buildable tools keep full title/kind. This supersedes the narrower excludeTools:[Skill] guard, which is removed. * fix(core): invalidate transcript index cache on in-place rewrites An in-place transcript rewrite that keeps the inode and byte length (e.g. rsync --inplace or a redaction pass) reused a stale cached index, because makeCacheKey() keyed only on path:dev:ino:size. readSegmentRecords then found each recorded offset parsing to a different uuid and dropped it, so GET /session/:id/transcript answered 200 with an empty events array instead of the documented 409. Include the file mtime in the index cache key so a fresh read after a same-size rewrite rebuilds the index, and raise SessionTranscriptSnapshotUnavailableError (-> 409) on a uuid mismatch or missing fragment instead of silently returning a short/empty transcript. Also make the qwen-serve docs explicit that at the default --channel-idle-timeout-ms 0 each page rebuilds the index (O(snapshotSize)). * codex: address PR review feedback (#6525) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * qwen: fix CI failure on PR #6525 The Run ESLint step failed on vitest/valid-expect in packages/acp-bridge/src/bridge.test.ts: the getSessionTranscriptPage timeout test stores expect(request).rejects.toBeInstanceOf(BridgeTimeoutError) and awaits it only after advancing the fake timers (a deliberate deferred await so the pending timeout rejection has a handler before it fires). Auto-fixing would add an inline await and deadlock the test, so scope-disable the rule on that assignment with a rationale. lint:ci and the affected test pass. * qwen: address PR review feedback (#6525) Withhold nextCursor on a mid-page transcript replay error. When collectHistoryReplayUpdatesPage catches a replayError partway through a page, records after the failed one are dropped and pendingToolCalls reflect partial state; still emitting nextCursor advanced the client past the dropped records and carried corrupted pendingToolCalls forward (phantom in-progress tool calls on later pages). Now nextCursor is withheld whenever replay.replayError is set — the page is already flagged partial + replayError, so the client stops instead of paginating with corrupted cursor state. Update the handler test to assert no cursor is issued on a replay error. * qwen: address PR review feedback (#6525) Log when parseTranscriptReplayState drops malformed pending tool calls from a replay cursor. Previously rawPending.filter(isPendingReplayToolCall) silently discarded entries that no longer matched the shape (e.g. a cursor from a newer daemon or corrupted in transit), turning a version-mismatch/corruption into a hard-to-diagnose 'tool never completed' artifact on later pages. Now emit a debug warning with the dropped/total counts; behavior is otherwise unchanged. * fix(serve): address transcript review feedback Dispose superseded replay configs, preserve structured resolution errors, sanitize multi-workspace failures, and expand transcript replay coverage across unit and real-daemon integration paths. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * qwen: address transcript review feedback (#6525) - [Critical] Map a missing transcript session to HTTP 404: the child throws a raw resourceNotFound (ENOENT without a cursor) that fell through sendBridgeError to 500. bridge.getSessionTranscriptPage now translates it to SessionNotFoundError, mirroring the load/resume path, with a bridge test. - Dedup the untrusted-session-owner 403 onto the shared sendUntrustedWorkspaceResponse so the response format/message stay consistent across session routes (route logging + context preserved). - Add coverage for parseTranscriptReplayState's non-object replay branch (cursor replay=garbage) -> empty pendingToolCalls + default cumulativeUsage. - Document that cursorHmacKeys are cached for the daemon lifetime (external key rotation requires a restart). * qwen: adopt transcript review suggestions (#6525) - Add a handler test that a mid-page replay error preserves already-emitted events (events>=1) alongside partial+replayError and withholds the cursor. - Add a two-call handler test for the cross-page cumulativeUsage round-trip: page 1 folds the bumped usage into the encoded cursor; page 2 decodes and propagates it into the replay context. - Log (not silently drop) a superseded structured error in the multi-workspace transcript resolution fallback. * qwen: clean up transcript test fixtures to fix no-AK CI flake (#6525) The transcript-paging integration suite wrote ~6 persisted chats/*.jsonl sessions into the daemon's project dir and never removed them. Because vitest runs a file's suites sequentially, those leftover sessions widened a pre-existing race in the later 'PATCH /session/:id/metadata > updates displayName' test (a freshly-created session can exist on disk but not yet appear in the listWorkspaceSessions page), making it fail deterministically in the no-AK smoke run. Add an afterAll to the transcript suite that removes the project chats/ dir, restoring a clean session list for subsequent suites. Verified: full no-AK suite now passes 43/43 across repeated runs. * qwen: harden transcript reader test timestamps + assert page fields (#6525) The record() helper derived the ISO timestamp seconds from text.length, producing invalid values (e.g. 00:00:013) once a record's text reached 10+ chars — harmless today only because no test asserted startTime. Replace it with a monotonic base+offset timestamp (always valid, strictly increasing). Also assert the previously-unchecked required SessionTranscriptRecordPage fields (sessionId, filePath, startTime, lastUpdated); the strict-ISO checks on startTime/lastUpdated guard against the timestamp-helper class of bug. --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> |
||
|
Shaojin Wen
|
51888210aa
|
feat(review): give every line of a large diff an accountable reviewer (#6612)
* feat(review): give every line of a large diff an accountable reviewer Review agents were handed the diff *command* and left to run it themselves. Shell tool output is capped at 30 000 characters and split head-1/5 / tail-4/5, so on a large changeset every agent received a few hundred lines off the top of the first file, the tail of the last file, and a truncation marker in place of everything between. Measured on a 211 000-character diff: 14.4% of the changeset, the same 14.4% for all ten agents. Nineteen of the twenty defects maintainers eventually confirmed on that PR lay in the hidden 85.6%. The ten-way dimension fan-out multiplied redundant reads of the visible sliver rather than adding coverage, and each review round sampled a different subset of the bugs depending on which files an agent happened to open on its own. The diff is now captured to a file and partitioned. `read_file` still caps a single read at ~25 000 characters, so writing the diff out is necessary but not sufficient — a whole-file read of that diff returns its first 611 lines. Chunks are therefore bounded by both a line budget (attention) and a character budget (what one un-truncated read returns), split on hunk boundaries, and never through the middle of a function. They tile the diff exactly, which is what makes the new coverage receipts checkable: past 500 diff lines each chunk gets one agent that owns it and must account for it, and a chunk with no receipt is re-reviewed before the run proceeds. "No blockers" can no longer be reported over code nobody read. Coverage alone did not close the gap. Chunk agents held every state-machine defect in that PR inside their assigned territory and reported none of them: the bugs were not inside any hunk but between new lines sitting two thousand lines apart, and what the agents lacked was not the lines but the question. A heavily rewritten file now also gets three whole-file agents that walk a fixed invariant checklist — mutable fields cleared on every exit path, timers cancelled on every close without discarding captured data, map inserts matched by deletes, retry counters incremented at every entry, status returns actually checked, error codes classified permanent versus transient, config honoured on every path, early returns that skip a required side effect. The checklist is split three ways deliberately: one agent asked to run all eight checks over a 2 400-line file runs one of them properly. Verification is sharded at eight findings per agent, because one verifier re-reading code for sixty findings degrades on the tail of its list. A verifier may now downgrade a Critical but never delete one — a rejected Critical is invisible to every later stage, a downgraded one still reaches a human. The reverse audit fans out per chunk instead of asking a single context-starved agent to re-read the whole diff, no longer skips verification, and stops after two consecutive dry rounds rather than one: on the PR that motivated this, the review reported "no blockers" twice and the next round surfaced five Criticals, three of them in code present since the first commit. * fix(review): keep small-diff reads inside the read_file cap Step 3A told every agent to read the whole diff in one call. `read_file` truncates a single call at ~25 000 characters, so a 500-line diff of long lines would come back short — the same blind spot the chunk plan removes, reintroduced at a smaller scale. Across the last 39 merged PRs that take the Step 3A path the largest diff is 23 570 characters, so this never fired in practice, but the margin is six percent. Step 3A now walks the chunk ranges, which are sized to fit one un-truncated read: one or two calls at this size. Derive a file's pre-change line count from the diff instead of measuring it with a second `git show` per file. `git show <base>:<newpath>` returns nothing for a renamed file, reporting zero pre-change lines and classifying a wholesale rewrite as light. The identity holds exactly for creations, deletions, renames and ordinary edits, and halves the process spawns. * fix(review): choose the topology from source lines, not diff lines Diff size is a bad proxy for review risk because test code dominates it. Across this repo's last 40 merged PRs the median diff is 41% test code and 14 of the 40 are more than half tests; PR #6457, which motivated the territory fan-out, is itself 58% tests. Gating on raw diff lines therefore carved small production changes into territories: a change of 173 source lines shipping 489 lines of new tests went to the chunked topology, where its production code ended up owned by a single agent, when the dimension fan-out would have read it through eight lenses. Territory fan-out is worth it when there is a lot of risky code to divide, not a lot of lines. The gate is now `srcDiffLines > 500`, with `diffLines > 2400` as a second clause — a delivery bound rather than a risk one, since past that point chunking uses fewer agents than the ten-lens topology anyway and reading a diff that large dilutes all ten. On the 40-PR sample six PRs move back to the dimension fan-out, for about 5% more agents in total across the sample. Paths are classified as source, test, or generated, and the per-kind line counts ship in the fetch report. Chunking is unchanged: the plan still tiles every line, tests and generated files included. What the gate decides is how many reviewers there are and what each is asked to do. Heaviness is likewise restricted to source files — the invariant checklist asks about fields, timers, collections, and error taxonomies, and a rewritten test file has none of those. * fix(review): decode C-quoted diff paths as bytes `git diff` C-quotes any path with a control character or a non-ASCII byte, so a file named `sub/中文文件.ts` arrives as `"b/sub/\344\270\255..."`. The chunk planner stripped the backslashes, turning it into `sub/344270255...ts` — a name that exists nowhere. Every downstream use of the path then failed silently: the line count came back zero, the file could never be classified as heavy, and the chunk agent was told it was reviewing a file that does not exist. Reuse core's `unquoteCStylePath`, which reassembles the octal escapes as UTF-8 bytes, rather than keeping a second, wrong decoder here. Coverage was never affected — line ranges stayed correct — but this repo has non-ASCII paths, so the mislabelling was reachable. Also correct two places that claimed hunks are never split. They are: a hunk larger than the chunk target is split at a top-level declaration, because a brand-new file arrives as one enormous hunk and treating it as atomic would hand a single agent a 50 000-character territory. * fix(review): make diff capture and header parsing robust to git config Four defects, all found in review of this branch. Diff capture obeyed whatever the user's git config said. With `color.diff=always` every `diff --git` line arrives wrapped in ANSI escapes, the parser recognises none of them, and the plan comes back with zero files and zero chunks — the coverage guarantee silently evaluates to nothing. `diff.mnemonicPrefix` renames the `a/`/`b/` prefixes to `i/`/`w/` and every path is then wrong; `diff.external` and textconv filters emit output that is not a unified diff at all. Capture now pins `--no-ext-diff --no-textconv --no-color --unified=3` and the two prefixes. The `diff --git` header was split with a greedy regex. Git separates the two paths with a space and does not quote a path merely for containing one, so `a/img with space.png b/img with space.png` split into `space.png`. Usually the `---`/`+++` headers disambiguate, but a binary or mode-only section has neither. For a non-rename both paths are the same string, so the split point is arithmetic; a rename states its new path outright in `rename to`. A chunk boundary could land on a `-` line. Those exist only on the old side, so the "starts at a top-level declaration" guarantee did not hold for the post-change file an invariant agent later reads. Split points are now restricted to lines present on the new side. An `oversized` chunk — one hunk with no safe interior boundary — can exceed what a single `read_file` returns. Chunks now carry their character count, and a chunk agent is told to page when a read reports truncation. A `Covered:` receipt for a range the agent only half read is worse than no receipt at all. * fix(review): split past a distant boundary, and stop probing GitHub for anchors Both defects surfaced running the new review against PR #6591. A 1431-line React component was emitted as a single 45 675-character chunk — nearly twice what one `read_file` returns — because the splitter looked for a safe boundary only inside the 400-line budget window, found none, and gave up on the entire remainder. Twenty-seven boundaries existed further along; the first sat 460 lines in. It now reaches past the window for the next one, so a single distant boundary can no longer collapse a whole file into one chunk. That PR goes from 15 chunks with one over the read cap to 18 with none. Step 7 validated comment anchors by trial. GitHub rejects an entire review with a 422 if any comment's line falls outside every hunk of its file, and the skill offered no cheap way to check, so a run against a real PR submitted five throwaway reviews carrying the bodies `Test`, `Test`, `t`, `t`, `t` to discover which anchors would stick. Those are permanent, public reviews on someone else's pull request. The fetch report now carries each file's hunks as new-side line ranges, which turns the check into a lookup, and the skill states plainly that a review is never submitted to test an anchor. * fix(review): stop reading hunk payload as metadata, and harden the plan Eleven defects from review of this branch. The worst two were silent. A unified diff emits a removed line whose content starts with `-- ` as `--- ...`, and an added line whose content starts with `++ ` as `+++ ...`. SQL, Lua and Haskell comments start with `-- `. The parser read those payload lines as file headers: the path was overwritten by the line's text, and the line vanished from the add/remove counts. A two-file diff — one SQL file losing a comment, one text file gaining a `++ ` line — came back with the second file named `plus line`. Metadata is now only recognised before a file's first hunk. The tiling invariant — every diff line belongs to exactly one chunk, which is what makes a missing coverage receipt mean something — was asserted only in tests. `buildDiffPlan` now checks it and refuses to return a plan with a hole. The rest: a split point could take a *deleted* blank line as evidence of the blank line before a declaration, though that blank exists only in the old file; whole-file invariant agents were pointed at `chunks[].files[]`, which merges hunks at lines 10 and 900 into one `10-902` span and would have had them report pre-existing defects as new; pure-deletion hunks were exported as the inclusive range `[N, N]`, so a right-side comment could be anchored where GitHub has no line and the 422 would sink the whole review; a deleted file could be marked heavy and send three agents to read a post-image that does not exist; a chunk holding a single line longer than one `read_file` can never be fully read by paging, and must now report itself uncoverable rather than receipt a lie; capture did not pin rename detection or `--no-relative`; `gitRaw` had no timeout, so a credential prompt on headless CI would hang forever; a failed base fetch was swallowed, leaving a stale merge-base and a structurally complete report describing the wrong diff; and local reviews still captured with a bare `git diff`, which `color.diff=always` alone renders unparseable. Adds an integration test that drives the real capture against a real repository under hostile git config, covering the paths synthetic fixtures cannot: renames and binaries and mode-only changes with spaces in their names, C-quoted non-ASCII names, and payload lines that impersonate headers. * fix(review): pin submodule output, and separate written lines from hunk spans Four defects from review of this branch. Diff capture left submodules to user config. `diff.ignoreSubmodules=all` hides a changed gitlink completely — a silent coverage hole in the file that is now the review's source of truth — and `diff.submodule=log` replaces the whole `diff --git` section with prose no parser can read. Both are pinned now, and the integration test asserts a bumped gitlink survives them. Whole-file invariant agents were handed `files[].hunks[]` as "the changed lines". A hunk spans the three context lines git prints either side of every change: on PR #6457's `QQChannel.ts` those spans cover 1 962 new-side lines of which only 1 403 were written. The agent would have reported defects in 559 lines that predate the PR. The report now also carries `addedRanges[]` — the exact lines the change wrote — and the skill gates invariant agents on those, keeping `hunks[]` for the one thing it is right for, GitHub anchor validation. `Uncoverable:` was introduced as a chunk agent's answer for a chunk holding a line longer than one read, but the receipt accounting still demanded a `Covered:` line from every chunk and relaunched any chunk lacking one — so an uncoverable chunk would have been retried forever. It is now a first-class terminal status: accepted by the accounting, carried into Step 6 under "Not reviewed", and it blocks an Approve verdict. Step 3A, which also walks the chunk plan, is covered by the same rule. The integration test built its fixture repository inside the developer's git environment, so a global `core.hooksPath` or `commit.gpgsign` ran during the test and `~/.gitconfig` decided what the "clean" baseline was. It now disables system and global config, hooks and signing, and sets the executable bit through the index rather than shelling out to `chmod`, which does nothing on Windows. * feat(review): plan any captured diff, and stop the report outgrowing one read Seven items from review of this branch. None blocking; two of them were the skill promising a topology it could not deliver. Step 3B's chunk agents are "one per entry in `chunks[]`", and only `fetch-pr` produced a chunk plan. A local-diff review, and a cross-repo review in lightweight mode, therefore routed into the territory fan-out with no chunk list, no receipts and no tiling guarantee. `qwen review plan-diff <diff-file>` now emits the same plan from any captured diff; redirecting `git diff` or `gh pr diff` to a file already sidesteps the shell's character cap, so all four review paths share one mechanism. A bare diff has no tree to read a post-image from, so it gets chunk agents but no invariant agents, and says so by omission. The fetch report is read with the same `read_file` that truncates at 25 000 characters — and for a seven-file PR it was already 28 056. The tail of `chunks[]` was being silently lost: the coverage hole this design closes, reappearing one level up. `addedRanges[]` now ships only on `heavy` files, its only consumer, which brings that report to 24 992; the skill says to page the read; and the command prints a note when the report exceeds one read. It stays pretty-printed on purpose — a compact one-line JSON cannot be paged by line. The tiling assertion threw inside `fetch-pr` after the worktree existed and before any report was written, so an unforeseen diff shape killed the review outright. It now degrades to the documented diff-less report with a loud warning, keeping both the loudness and the review. `gitOpt` and `git` had no timeout, and `resolveMergeBase` uses `gitOpt` for a network fetch — the exact path whose credential prompt the `gitRaw` timeout was added to survive. All three wrappers now share a deadline and `GIT_TERMINAL_PROMPT=0`. Markdown under `docs/` or at the repository root classifies as `docs` and stays out of `srcDiffLines`, so a translation PR does not trip the territory gate. Markdown inside a source tree stays `source` — the bundled skill prompts are behaviour, not prose. Also: the user docs stated the gate without its `diffLines > 2400` clause, and `READ_FILE_CHAR_CAP` was exported but never used. It now backs the report-size warning. * test(review): unit-test the merge-base and plan-report seams The last open review thread asked for `resolveMergeBase`, `fileMetrics` and `gitRaw` to be testable with git mocked out. Three of the four functions it named have since moved: `classifyHeavy` is a pure function with unit tests, `fileMetrics` became `buildPlanReport`, which already takes an injected post-image resolver, and `gitRaw`'s output path is exercised by the real-git integration test. `resolveMergeBase` was still private and untested. It now lives behind a three-method `GitProbe` — fetch, refExists, mergeBase — that `fetch-pr` fills from the real wrappers. Seven tests cover the branches that matter and that no end-to-end run reaches: the tracking ref preferred over the local branch, the fall-through when the tracking ref shares no history, and above all the dangerous one — a failed fetch that still resolves a merge-base from a stale local ref, which produces a structurally complete report describing a diff nobody wrote. `buildPlanReport` gains seven of its own: the injected resolver is asked once per file and never for a binary, a null resolver means "no tree, decide nothing" rather than a guess, `addedRanges` ship only where an invariant agent will read them, and a pure-deletion hunk never reaches the anchorable ranges. * fix(review): see deletions, survive suppressBlankEmpty, and stop approving unread code Seven findings from review of the merged head. Three of them were the design contradicting itself. `diff.suppressBlankEmpty` prints a blank context line as a physically empty record rather than a lone space, and there is no command-line flag to override it — only `-c`. The parser advanced its new-side cursor for space-prefixed context alone, so every `addedRanges` entry after the first blank line shifted up by one, and the split-point heuristic stopped recognising blank lines. The capture now pins the config, and the parser treats an empty hunk-body record as context regardless, because a diff from `gh pr diff` or a hand-captured file never passes through that pin. A whole-file invariant agent was given the post-change file and the ranges the PR wrote. A deletion appears in neither. Removing a `clearTimeout()`, a `Map.delete()`, or a retry-counter increment is exactly what the checklist hunts, and the text it was handed cannot show a line that is no longer there — telling it to "cite the surrounding hunk" pointed at data it never received. Heavy files now carry a `diffRange` into the report, and the agent reads its own slice of the diff, where the `-` lines are. The receipt accounting demanded exactly one per chunk and said it applied to Step 3A, where nine dimension agents each walk every chunk: literal execution yields nine receipts or none. Territory ownership is a Step 3B idea. What both paths share is the uncoverable rule, and that needs no agent — a chunk is uncoverable iff its `maxLineChars` exceeds the read cap, which the orchestrator reads out of the plan before launching anything. That rule was also never threaded into Step 7, so a green PR with an unread chunk could receive a public LGTM. Any uncoverable chunk now downgrades APPROVE to COMMENT and must be named in the body. Also: the capture recipes redirected into `.qwen/tmp` before anything created it; a file-path review of an unchanged file produced an empty plan that no agent could read, and the skill now branches to a full-file read instead; and the docs classifier called `website/src/App.tsx` prose while calling `packages/cua-driver/docs/*.md` source — it now matches prose extensions under a documentation directory at any depth. * fix(review): tell agents what a severity means before asking for one The severity definitions lived once, in Step 6 — after every severity had already been assigned. Step 3's finding format asked each agent for `Severity: Critical | Suggestion | Nice to have` and never said what the words meant. The agents that fill that field are separate subagents with separate priors and no shared definition between them, so each fell back on its own, and the priors disagree. Observed on a live review of PR #6635 — a run of the skill as it stands on main, whose Step 3 and Step 6 text this branch inherits unchanged. One review, CHANGES_REQUESTED, ten inline comments. Six were Critical, and four of those six were coverage gaps: "zero test coverage", "no references to `workers`", "no test exercises this". Two Suggestions in the same review were the identical class. The verdict is computed from Criticals alone, so that PR was blocked partly on the strength of findings its own reviewer had, elsewhere, called suggestions. The two genuine Criticals — a fail-fast that no longer fires before the daemon reports healthy, and a startup failure path that never closes the HTTP server — would have blocked it on their own. The definitions now sit in the finding format that every agent is handed, they are listed among the things every agent prompt must carry, and Step 6 points back at them rather than restating them. A missing test is a Suggestion: "this file has zero references to X" is a coverage statistic, not a defect. Two shapes stay Critical because something is genuinely wrong — a test asserting the opposite of the intended behaviour, and a test weakened or deleted in the diff so new behaviour passes. If a missing test would let a specific incorrect behaviour ship, report that behaviour and cite the gap as evidence. * fix(review): walk cross-file edges in both directions Cross-file impact analysis only ever asked "will the existing callers break?" Every bullet was about signature compatibility, and the budget rule told agents in so many words to "skip unchanged-signature modifications". A field added to an interface changes no signature and breaks no caller, so the analysis was blind to it by construction. The failure that exposed this, on PR #6621: the diff added `deviceFlowRegistry?` to WorkspaceRuntime and passed it into the dispatcher for every secondary ACP mount, and nothing anywhere assigned it. The reviewing agent saw the declaration, found no writer, wrote "intentionally deferred to a later milestone", and filed a Suggestion to fix the JSDoc. The reader was AcpDispatcher — a file the diff never touched — where `if (!this.deviceFlowRegistry)` turned `auth/device_flow/start` into an INTERNAL_ERROR and `auth/status` into an empty list on every non-primary workspace. Workspace-qualified ACP shipped its authentication dead, and the review called it a documentation nit. A second reviewer filed the same observation as Critical; the author fixed it with code and dropped the field. Reading cannot find this. The declaration, the pass-through, and the read sit in three different places, and the read is outside the diff, so no agent reaches it by paging through hunks. Only a grep for the read sites does. So: for every field, option, or optional parameter the diff adds, grep its read sites, including outside the diff, and ask what happens when it arrives undefined. Severity is decided at the read site, not the declaration. And an agent must not explain an unpopulated field with author intent it cannot observe — "reserved for future use" is a claim about a person, not about code, and reaching for one means filling a hole in your own field of view. * fix(review): pin the diff base, and make the review body checkable Three defects, all found by reading what live reviews actually posted. The diff base. Agents were handed a diff command and left to choose a base. `main..HEAD` and `main...HEAD` differ by one character and by the entire meaning of the review: a two-dot diff against a main that has moved shows main's later commits reversed, so main's fixes read as the branch's regressions. A review of PR #6626 approved the four files the PR actually changed, then warned the author publicly that their branch carried "typo regressions" in a file the PR never touched and should be rebased. main had corrected `compatability` to `compatibility` after the fork point. The branch had done nothing. Capture now resolves the base once and hands agents a file; they never see a ref name, and a finding in a file outside the report's `files[]` is not a finding about this PR. The review body. "A Suggestion never goes in body" is stated twice and was violated anyway, because a model holding a finding it cannot anchor would rather say it somewhere than drop it. On PR #6631 an unanchorable Suggestion about `session.ts:2048` — a line in no hunk — became a second paragraph of the public review body. So the rule stops being prose: for COMMENT the body is exactly one of three sentences plus the footer and nothing else, and you read what you are about to send and confirm it. A Suggestion that will not anchor is deleted; it is already in the terminal output and the Step 8 report. The downgrade sentence. On PR #6489 a review with three Suggestions and no Critical announced it had been "downgraded from Approve" — telling the author the PR would otherwise have been approved, which was false: a Suggestion-only review is COMMENT on its own. Decide the event from the findings first, apply the downgrade flag second, and write the sentence only if it changed the answer. * fix(review): decide the event by counting, not by weighing A review of PR #6584 filed three inline Suggestions and submitted APPROVE with an empty body. GitHub recorded it as an approval. The rule it broke has been in Step 7 all along --- APPROVE means no Critical *and* no Suggestion --- and so has the one about the body, which is empty only for REQUEST_CHANGES. Both were stated twice. Both were ignored. They are ignored because at submit time the model is reasoning about what it wants to say, and "these are only suggestions, the PR is fine" is a sentence it can talk itself into. Nothing in that sentence is a count. So the event and the body become arithmetic. Count the Criticals, count the Suggestions, read the row off a three-row table, and only then apply the downgrade flags --- which can turn APPROVE or REQUEST_CHANGES into COMMENT and nothing else. Then read back what you are about to send and confirm it matches the row. A body holding text the table does not authorise is a finding that failed to anchor; if it is a Suggestion, it gets deleted, not relocated into public prose that no line of code answers to. This subsumes the body-only invariant added in the previous commit, which the same submit-time reasoning had already defeated once, on PR #6631. * fix(review): stop the plan report outgrowing the read it must fit in The report tells an agent how to page everything else, so it has to be readable in one `read_file` — about 25 000 characters. Running the real `fetch-pr` against PR #6457 produced 25 070. Two constraints pull against each other. Compact JSON is a single enormous line, and `read_file` pages at line boundaries, so a report too big for one call could never be read at all. Indented JSON pages fine but spends four lines on `{ "start": 812, "end": 815 }`, and a heavily rewritten file contributes hundreds of them: `QQChannel.ts` alone carries 140 added ranges and 49 hunks. So indent the structure and inline the leaves. Same JSON, same keys, one range per line, still pageable — and 28% smaller. The #6457 report goes from 25 070 bytes to 18 042, and the "page it" warning that used to fire on a seven-file PR now stays quiet. The earlier attempt at this trimmed `addedRanges` to heavy files only and landed at 24 992 bytes on the same PR. Eight bytes of headroom was not a fix. Tests pin the three properties that matter: the collapsed text parses back to an identical object, no range spans two lines, and a path that literally spells a range is not mistaken for one — JSON escapes the quotes inside a string value, and the collapse patterns require unescaped ones. * fix(review): prune the worktree registration a deleted directory leaves behind `cleanStale` and `cleanup` both guarded `git worktree remove` behind `existsSync(path)`, and neither ever pruned. Delete the directory by hand — which is exactly what reclaiming disk with `rm -rf .qwen/tmp` does — and git keeps the worktree registered but missing. From then on `/review` on that PR cannot run: $ git worktree add .qwen/tmp/review-pr-6457 qwen-review/pr-6457 fatal: '...' is a missing but already registered worktree; use 'add -f' to override, or 'prune' or 'remove' to clear and the branch delete that `cleanStale` does next fails too, because the phantom worktree still has that branch checked out. Nothing in the review command surface ran `git worktree prune`, so nothing ever cleared it. This surfaced running the real skill: the orchestrator's first `fetch-pr` failed, it fell back to `qwen review cleanup`, and retried. The leak is not rare — three abandoned worktrees from May and June were still registered in this checkout, one per review that died before Step 9. `releaseWorktree` now does both halves in the order they depend on: remove the directory if it is there, prune the registration unconditionally (a no-op when nothing is stale), and only then let the caller delete the branch. Both callers share it. The tests drive real git. Deleting a worktree directory by hand and re-adding it throws "missing but already registered" without the prune, and `branch -D` throws "used by worktree" — both assertions fail if the prune is removed, which is the point of writing them. * fix(review): put the open comments where a truncated read will find them `read_file` returns the first `truncateToolOutputThreshold` characters — 25 000 by default — sets `isTruncated`, and pages by line. `pr-context` wrote "## Open inline comments (no replies yet — may still need attention)" last, so on a PR with a long history it was the first thing lost, and nothing read the flag that said so. On PR #5738 that section began at character 27 125 of a 31 220-character file. The review submitted "Reviewed — no blockers." Five Critical threads were unresolved; four had in fact been addressed, but the fifth — `clearCiEnv()` clearing only `CI*` while `writeTerminalTitle` branches on `TMUX`/`STY`/ `ZELLIJ`/`DVTM` — was live, in the diff, and never seen. Regenerating the context for ten PRs: four lost part or all of the section, and all four were the PRs with the most review rounds. Small PRs never trip it. - Emit the open threads before the already-discussed ones. The findings a round must answer outrank the ones already settled. - `pr-context` warns when the file exceeds the threshold, naming any headings past the cut, and says so plainly when the loss is inside the last section's body instead. - Step 2 of SKILL.md now tells the agent to read `isTruncated` and page the remainder before Step 3. Reordering buys headroom; it does not create it. A 40 000-character context still loses its tail, which is what the warning is for. * fix(review): load this repo's review rules, and re-check open Criticals before approving Two gaps the dogfood on live PRs surfaced, both invisible from reading the skill. `load-rules` looks for a `## Code Review` heading in AGENTS.md and QWEN.md. Neither had one, so it wrote an empty file on every run: every `/review` in this repo reviewed with zero project rules. Add the section, distilled from the conventions already scattered through AGENTS.md (ESM, no cross-package relative imports, kebab-case/PascalCase naming, collocated tests, comments-only-when-why), plus the two hard lessons below. The section loads from the base branch by design — a PR cannot inject its own review rules — so it takes effect once merged. The skill treated a zero-Critical outcome as a fallback rather than a claim. On one PR it published two Criticals citing code not present at the reviewed commit (a fabricated blocker on an already-approved PR); on another it submitted C=0 while a live, twice-filed Critical still stood (a dropped blocker). Add a step before the verdict: for each unresolved Critical on the PR, read the code at the reviewed commit and record still-stands / fixed-by-this-diff / cannot-tell. The event follows from the code, not from the finding count or the thread flags — `isResolved`/`isOutdated` track the anchored line, not whether the bug was fixed. - AGENTS.md: new `## Code Review` section. - load-rules.ts: export `extractCodeReviewSection`; load-rules.test.ts covers the boundary scan and asserts AGENTS.md's own section extracts non-empty, so deleting the heading fails the build. - SKILL.md: re-verification step ahead of the Verdict. |
||
|
tanzhenxin
|
fa7fdbca01
|
fix(core): clamp max_tokens to the context window; retire the output reservation (#6556)
* fix(core): clamp max_tokens to the context window; retire the output reservation Auto-compaction was firing far too early — a 200K-window session compacted at roughly half the window. The cause was not the compaction engine but that every request manufactured a large max_tokens, which forced a defensive reservation of that output budget out of the window before computing compaction thresholds. The reservation shrank the effective window, pulled the trigger down, and spawned a chain of band-aids. Size max_tokens to the room actually left in the window instead — the smaller of the model's output ceiling and (window − prompt − margin) — so an oversized request can never exceed the context limit. Once output is guaranteed to fit, the reservation is unnecessary and is removed; compaction gates on the full window again. Raise the default proportional threshold from 0.70 to 0.85, and replace the temporary half-window reservation cap with a flat 64K output ceiling. This resolves early compaction, the 400 "maximum context length" error on request, the "hard limit: 0" pre-send NOOP for env-configured models, and retires the half-window reservation cap, while keeping max_tokens on the wire for both OpenAI- and Anthropic-shaped providers. Fixes #5950 Fixes #6384 Claude-Session: https://claude.ai/code/session_014DW2TynKHLjsbRqTBSyQue * test(cli): update /context threshold expectations for 85% default The auto-compaction default moved from 70% to 85% and the output reservation was removed, so computeThresholds(200K) now yields warn=150K / auto=170K (was 147K / 167K). Update the /context command tests that hard-coded the old ladder. * fix(core): apply window clamp to samplingParams users who omit max_tokens Previously a samplingParams config without a max_tokens key sent no max_tokens on the wire (OpenAI path), so those users bypassed the prompt + max_tokens <= window clamp — inconsistent with the Anthropic path, which always injects the clamped value. Mirror the Anthropic fallback (reconcile ?? config ?? request) so the clamped maxOutputTokens is injected when samplingParams omits max_tokens. Guard the injection: when samplingParams targets a provider-specific output-budget key (max_completion_tokens for GPT-5/o-series, max_new_tokens), leave it verbatim — adding max_tokens alongside double-specifies the budget and those endpoints reject the pair. * fix(core): clamp provider output-budget keys to the window in samplingParams A samplingParams config carrying a provider-specific output-budget key (max_completion_tokens for GPT-5/o-series, max_new_tokens) but no max_tokens previously passed the key through verbatim, so its value escaped the prompt + output <= window clamp — e.g. max_completion_tokens: 200000 on a 200K window with a 150K prompt. Clamp the key's value in place to the remaining window (min with the request maxOutputTokens) instead of injecting a separate max_tokens: sending both keys double-specifies the output budget and o-series rejects the pair. The value only shrinks when the window is tight; when there is room it passes through unchanged, matching how max_tokens is already treated. * fix(core): compact on the window ceiling, not the max of the threshold ladder (#6583) * fix(core): compact on the window ceiling (min), not the max of the ladder computeThresholds combined the proportional term (pct*window) and the absolute term (effectiveWindow - AUTOCOMPACT_BUFFER) with Math.max, which pushed the auto-compaction trigger toward the top of the window on large windows — a 1M-token window compacted at ~97%, leaving ~33K headroom. The absolute term is structurally a ceiling ("compact before the prompt leaves too little room for the summarization side-query, which needs up to SUMMARY_RESERVE of output"), so it composes with Math.min, matching the claude-code reference (services/compact/autoCompact.ts, which uses Math.min and whose default trigger is the absolute term alone). auto = absoluteCeiling > 0 ? min(pct*window, absoluteCeiling) : pct*window warn = max(0, auto - WARN_BUFFER) // WARN_PCT_OFFSET retired hard = unchanged Effect: large windows compact at ~85% (the DEFAULT_PCT ceiling) instead of ~97%; small/mid windows keep room to run compaction (a 128K window's summary now provably fits); sub-33K windows are unchanged. A lower context.autoCompactThreshold now pulls compaction earlier on large windows, matching the reference's Math.min override semantics. Updates the threshold unit tests, the settings schema description, and the user docs to describe the setting as a ceiling on the trigger. * refactor(core): trim threshold doc comments; name the hard-edge term Post-review cleanup (no behavior change): - Collapse the duplicated regime explanation shared between the DEFAULT_PCT and computeThresholds doc comments into one canonical block; point the constant's doc at computeThresholds. - Rename rawHard -> hardEdge and note it is the window-edge ceiling, so the two roles of the hard tier (window edge vs. auto + HARD_BUFFER) are legible. - Shorten the context.autoCompactThreshold description in settings.md to the concise schema wording (also un-widens the docs table). * fix(core): clamp provider output-budget keys on every samplingParams exit A config carrying both max_tokens and a provider-specific output-budget key (max_completion_tokens / max_new_tokens) took the max_tokens early return, spreading the provider key onto the wire unclamped — on backends honoring the larger key, prompt + output could exceed the window. Collapse the two returns into a single exit that always runs the provider-key clamp, so no output-budget key escapes the window clamp regardless of which combination of keys is present. --------- Co-authored-by: 易良 <1204183885@qq.com> |
||
|
qqqys
|
7129cecba2
|
fix(channels): manage stale DingTalk Stream connections (#6675)
* fix(channels): manage stale DingTalk Stream connections * fix(channels): harden DingTalk connection lifecycle --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> |
||
|
qqqys
|
c84089ec48
|
fix(dingtalk): preserve markdown tables (#6673) | ||
|
qqqys
|
043c22cb4b
|
feat(channels): support webhook-triggered channel tasks (#6495)
* feat(daemon): add a2a settings and capabilities * docs(channels): design webhook-triggered tasks * feat(channels): add webhook task helpers * fix(channels): bound webhook prompt metadata * feat(channels): run webhook-triggered tasks * fix(channels): harden webhook task lifecycle * fix(channels): require yolo for webhook tasks * feat(channels): parse webhook configuration * fix(channels): validate webhook secrets * feat(channels): forward webhook tasks to channel worker * fix(channels): require webhook enqueue on supervisors * fix(channels): handle webhook IPC send failures safely * feat(serve): accept channel webhook tasks * fix(serve): stop webhook validation after first error * fix(webhooks): reject inherited target refs * docs(channels): document webhook-triggered tasks * docs(channels): fix webhook task example * docs(channels): refine webhook task docs * docs(channels): add webhook task implementation plan * fix(channels): restore webhook task context and chunks * fix(serve): classify worker webhook enqueue failures * fix(channels): address webhook review feedback * fix(serve): address channel webhook review blockers * fix(serve): satisfy channel webhook lint * fix(serve): harden channel webhook admission * fix(serve): narrow channel webhook source config * fix(serve): classify webhook session scope failures * fix(serve): harden webhook payload handling * fix(serve): authenticate webhook startup cheaply * fix(serve): keep deferred serve fast path lean * fix(serve): address deferred webhook review blockers * fix(channels): propagate webhook approval mode * fix(channels): harden webhook task admission * fix(acp): harden approval mode initialization * fix(channels): harden webhook shutdown and secrets * fix(channels): harden webhook review blockers * fix(serve): harden deferred webhook auth * test(channels): cover webhook target rejection * fix(channels): preserve webhook thread targets * fix(channels): address webhook review blockers * fix(channels): harden webhook review blockers * test(serve): align deferred webhook secret log assertion * fix(channels): isolate webhook thread sessions * fix(channels): harden webhook enqueue failures * fix(serve): classify disabled channel workers |
||
joeytoday
|
c0aeb7df5d
|
docs(channels): add setup screenshots to WeCom robot guide (#6648) | ||
|
nas
|
32ddd7ae77
|
docs: document tools.disabled and tools.visible settings (#6641)
Both settings are implemented and wired end to end (settingsSchema.ts, normalizeDisabledTools.ts, ToolRegistry registration gate) but were missing from the settings reference, while their deprecated siblings tools.core / tools.exclude / tools.allowed are documented. In particular, tools.disabled already answers a recurring user request: disabling enter_plan_mode entirely so the model can never switch into plan mode on its own (#5970). Documenting it makes that option discoverable. |
||
|
tanzhenxin
|
7a9ee09f49
|
fix(core): honor NO_PROXY for model requests (#6640) | ||
|
ytahdn
|
8522d43875
|
feat(daemon): expose session runtime status (#6645)
* feat(daemon): expose session runtime status * test(daemon): cover pending interaction mirrors --------- Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com> |
||
|
dreamWB
|
24bca9a718
|
feat(web-shell): add context mention customization (#6578)
* feat(web-shell): add context mention customization * fix(web-shell): address context mention review comments * fix(web-shell): harden custom context rendering * fix(web-shell): guard custom tag render fallbacks * fix(web-shell): harden custom context rendering --------- Co-authored-by: 易良 <1204183885@qq.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> |