9608 commits

This branch

This branch

All branches

Author	SHA1	Message	Date
probe	1424ea0f9e	fix(web-shell): preserve continuation outcomes across replay	2026-09-11 13:16:44 +08:00
ytahdn	53e6196c6b	fix(web-shell): persist cancelled responses and elapsed time (#11608) ... * fix(web-shell): preserve cancelled responses and elapsed time * fix(core): retain cancelled text during reactive compression --------- Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com>	2026-09-11 04:30:09 +00:00
Shaojin Wen	d66ad3e3a2	feat(review): focus automatic static navigation reviews (#11456) ... * feat(review): focus automatic static navigation reviews * fix(review): address round-1 review of the docs-nav profile (#11456) - QWEN_REVIEW_AUTOMATIC is operator-only: added to PROJECT_ENV_HARDCODED_EXCLUSIONS and read through automaticReviewRequested(), which refuses file-sourced values - SKILL.md: the docs-nav profile passes the cumulative findings file to compose-review as findingsPath, so an unruled candidate caps at Comment instead of posting as a verified blocker - the classifier treats top-level key order as structure (a reorder keeps the full review); the design doc records the decision - assessResume reports a moved head as head-moved even on a profiled report; both profile guards read the shared DOCS_NAV_PROFILE constant - the mechanism-health note no longer reads the profile's by-design anchor withhold as a stopped chain - the coverage gap subject names the omitted full review and reverse audit instead of the review that ran - buildRoleBrief welds no context pointer for a malformed plan prNumber * fix(review): address round-2 review of the docs-nav profile (#11456) * fix(review): address round-3 review of the docs-nav profile (#11456) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): close focused navigation review gaps * fix(review): strengthen focused navigation contracts --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-09-11 03:59:55 +00:00
now-ing	0376c241fe	fix(serve): preserve child error detail on explicit-close and kill paths (#11190) ... The session/close notification-failure log shared by closeSession and killSession serialized the caught rejection with String(), which collapses a wire-shaped JSON-RPC error record (the child's close refusal, not an Error instance on the daemon side) to [object Object]. Route it through the existing extractErrorMessage helper, and apply the same fix to the branchSession live-state cleanup close that calls the same wire method. Refs #11123 Co-authored-by: mac <bishopapril850965@yahoo.com>	2026-09-11 03:43:55 +00:00
Xiong Wang	f649d65d1f	feat(live): add visual input, proactive assistance and memory (#11369) ... * feat(live): add visual input, proactive memory and desktop controls * fix(live): preserve upstream checks and finalize migration validation * feat(live): open active configuration from host settings * fix(live): address lifecycle and proactive review findings * feat(live): add subagent controls and diagnostics with review fixes --------- Co-authored-by: 菲兹 <feizi.wx@alibaba-inc.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>	2026-09-11 03:13:13 +00:00
Shaojin Wen	c6c276b05c	feat(web-shell): improve composer and context usage presentation (#11509) ... * feat(web-shell): improve composer and context usage presentation * fix(web-shell): address context presentation review feedback * test(web-shell): align brand fixtures with startup and layout * fix(web-shell): improve context readability and availability	2026-09-11 03:01:33 +00:00
jinye	2f426a64f4	feat(web-shell): enrich browser notifications and open target sessions (#11447) ... * feat(web-shell): enrich browser notifications and open target sessions Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * feat(web-shell): include current prompt in browser notifications Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * feat(web-shell): configure browser notification branding and defaults Allow hosts to configure notification names, icons, and the initial preference. Enable notifications by default in the built-in page while preserving saved user choices and browser permission requirements. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(web-shell): address browser notification review feedback Preserve code excerpts, bound pending notification text, and keep notification configuration changes from restarting session streams. Reconcile branding documentation and add regression coverage. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(web-shell): address notification follow-up review cases Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>	2026-09-11 02:27:39 +00:00
qqqys	ec743fd450	feat(web-shell): route scheduled runs by model and group (#11396) ... * feat(web-shell): route scheduled runs by model and group * fix(serve): harden scheduled task rollback * test(serve): align deletion generation expectation * fix(serve): scope the fail-closed sub-session model gate to scheduled-task runs * test(serve): pin the scheduled-task model-gate conjuncts The fail-closed model gate in create-sub-session and its standalone sibling had three conjuncts no test discriminated: widening the source predicate to sourceType === 'default' or dropping modelApplied === false left both suites green. Add a positive scheduled-task case with modelApplied: true, and give both non-scheduled negative cases real non-scheduled source coordinates so each conjunct has a witness. Also correct the updateGate.fail comment in cronScheduler.test.ts: the mock returns the input array, so updateCronTasks skips the tasks write before the injected error is thrown — it does not write through. * fix(serve): address scheduled-task routing review feedback (#11396) A dispatch-failed one-shot stayed inert when re-enabled: the PATCH re-seat pointed a date-pinned cron at a next occurrence up to a year out, and the missed-detection disk guard suppressed the retry signal forever. Re-enable of such a task now keeps its consumed anchors so the missed-one-shot pass delivers the retry (confirm-first), and the dead guard plus the restoredPerRunOneShots set it fed are removed. Also: PATCH answers 404 for a missing task id even when the patch names a missing group; the routing-id cap is shared with session creation at 256 and the create_sub_session tool validates it up front; an unreadable .deletions sidecar no longer vetoes the tasks write (observations skip, deletion writes rebuild); stray routing fields on a non-per-run task are normalized on read instead of failing the file; the rollback watchdog strictly outlasts the agent close timeout and force-kills plus cleans the transcript on the timeout path; and the workspace-unavailable form error is localized. --------- Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.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-code-dev-bot <qwen-code-dev@service.alibaba.com>	2026-09-11 02:24:50 +00:00
qwen-code-dev-bot	7556821e65	fix(ci): retry transient E2E build artifact upload failures once (#11364) (#11375) ... * fix(ci): retry transient E2E build artifact upload failures once (#11364) * test(ci): harden the e2e-build upload retry contract pins (#11364) Address review findings on the retry helper suite: scope the two-attempt assertions to the archive instead of every upload-artifact step and bind the attempts by step name, pin the pack-to-upload handoff (archive name and step order), pin the build job's absent job-level continue-on-error, pin the retry gate by whole expression so a failure() conjunct cannot silently disable it, compare the two with: blocks wholesale (minus the intentional overwrite asymmetry) plus the action pin, and compare downloaded names against every workflow upload instead of restating the four known legs. Also add the suite to the yamlSuites roster in ci-platform-lanes.test.js so it can never join the dependency-free fast lane undetected. * fix(ci): correct the upload-retry rationale and name-key its contract (#11364) Real-runner probes on the PR showed the comment's two reasons for the retry's overwrite do not reproduce: a stalled upload aborts before finalize and reserves nothing, and name uniqueness is scoped per run attempt so a manual re-run does not 409 either. Reword both comments to the window overwrite actually covers — a first attempt that finalized e2e-build and only then reported failure. The contract test picked "the archive" by position (uploads[0]), contradicting its own comment: an unrelated upload step placed before the archive upload reddened the suite with misleading messages. Key on the literal e2e-build name instead — already the pinned contract in scripts/tests/e2e-workflow.test.js — in both the filter and the download-leg assertion. Mutation probes: a decoy upload step before the archive now stays green, while decoy+retry-deleted, retry-deleted, retry-renamed, and overwrite-dropped all still fail. * test(ci): pin the pack-to-upload directory in the e2e retry contract (#11364) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(ci): pin the exact upload path in the e2e retry contract (#11364) Round 3's handoff pin closed the pack side but left the upload side pinned to a directory prefix, so moving only the two upload with.path values into a subdirectory of runner.temp kept the suite green while the runtime upload found no file and redded the build job. Compare first.with.path to the exact '${{ runner.temp }}/' + archive literal, reusing the derived archive binding rather than restating the name. Mutation probes: the upload-side drift now fails on the new assertion (16/1, expected-vs-actual), the workspace-escape and pack-side drift mutations stay red on their existing assertions, and the same upload-side mutation under the old prefix assertion stays green — the exact pin is the load-bearing one. Also reword the overwrite rationale in both the workflow comment and the test comment to the form the shipped action source supports: the record is created before any bytes move, so a failed first attempt can leave the e2e-build name taken, and overwrite is delete-then-upload and a no-op when nothing exists. The previous wording asserted a stall reserves nothing, which the create-then-upload lifecycle contradicts. And name the retry contract's actual ungated siblings (run-e2e-tests.sh's sandbox-image build retry, build-and-publish-image.yml's image build retry) instead of the budget-gated sandbox:none shard retry the comment over-claimed parity with. * fix(ci): pin the unpack archive name and restore the overwrite rationale (#11364) Round 4's handoff pin derives the archive name from the upload with.path, so renaming the tarball at the pack step and both upload paths while the four legs' unpack arguments stay put kept the suite green while every leg would fail closed on main (review R1-3). Pin each 'Unpack build artifact' run's trailing argument against the derived name; the legs download into runner.temp/e2e-build/, so the comparison is a suffix match, not the upload's full path. Mutation probe: a producer-only rename now fails the contract with 'a leg unpacks a different archive than the build job uploads' (16 pass / 1 fail), the unmutated workflow stays green (17/17), and the same mutation was green before this pin (17/17). Also restore the overwrite rationale that 03b921ba landed and 09deb2b rewrote: the real-runner harness on this PR showed a stalled attempt aborts before finalize and reserves nothing, and overwrite (ListArtifacts -> DeleteArtifact) cannot rescue a name an un-finalized attempt reserved, so the comments once again name the window overwrite actually covers — an attempt that finalized e2e-build and only then reported failure. * fix(ci): announce absorbed e2e-build upload stalls so recurrence stays countable (#11364) The upload retry absorbs a stalled first attempt behind continue-on-error, so the build job concludes success and the main-CI failure tracker — gated on workflow_run.conclusion == "failure" and selecting jobs with conclusion == "failure" — never records the stall the retry recovered from, freezing the recurrence count the #11364 ruling conditions on at one. Add a plain run step after the retry, gated on steps.upload-build.outcome == 'failure', that emits a :⚠️: annotation: the same check-run surface that named run 34208365262's "Upload progress stalled.", so the absorbed stall stays queryable without reddening the job or the four legs behind needs: ['build']. The implicit success() gate from its placement after the retry scopes it to the absorbed case; a double failure reddens the job directly and the tracker already sees that. Pin the step in the retry-contract test per the round-7 review's acceptance criterion (R4-1): exact gate expression, :⚠️: emission, no action and no failing directive, placed after the retry. Mutation probes: deleting the step, stripping :⚠️:, negating the gate, adding exit 1, moving the step before the retry, and giving it a uses: action each turn the suite red (16 pass / 1 fail); the unmutated workflow is green (17/17). --------- 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>	2026-09-11 02:05:02 +00:00
Stellar鱼	6a43348f7d	test(integration): assert compress telemetry success (#11259)	2026-09-11 01:15:17 +00:00
易良	38688f891f	fix(core): free an exited web terminal's PTY resources at exit time (#11572) ... WebTerminalRegistry released PTY resources only in release(). handleExit set `exited`/`exitCode` and notified the exit listeners, touching nothing else, and the browser route deliberately keeps an exited session alive for scrollback replay (finishExited leaves releaseAfterReplay at its false default). The client will not release earlier either: a live exit closes the socket with 4000, which is non-retryable, so only a tab close, a workspace drain, dispose() or the 15-minute idle reclaim ever freed the PTY. So every exited web terminal held node-pty's conout worker - and, upstream, its conhost.exe - for up to IDLE_RECLAIM_MS. Exited sessions are also excluded from the admission cap on purpose, so accumulation inside that window was unbounded. Extract the PTY-resource half of release() into releasePtyResources() and call it from handleExit, deferred one setImmediate so the trailing onData callbacks node-pty may still have queued reach the buffer first - the same race shellExecutionService drains before finalizing. The helper keeps the session's map entry and its buffer, so readSnapshot() replay and the route's releaseAfterReplay path are unaffected, and it never signals the pid: the shell is gone and its pid may be recycled, which is why #11313 added releaseHost instead of reusing kill(). A per-session flag keeps a later release() from disposing anything twice. release() now routes both arms through the helper. killPtyTree stays on the live arm only and still runs first, so releaseHost keeps seeing the close the wrapper noted. Everything between the old detach site and the new one is synchronous (spawnSync, process.kill, pty.kill), so no onData callback can interleave and the live path's observable behaviour is unchanged. Adds three tests: resources freed at exit time rather than at the reclaim, scrollback still replayable afterwards, and no double free when release() follows. Verified at the mechanism level on Linux with os.platform() mocked to win32; actual conout-worker and conhost.exe reclamation on a real Windows ConPTY is not verified here, and #11352's upstream close defect is untouched. Fixes #11353 Patrol-Run: qwen-issue-patrol/jmtvhcstvvk Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-09-11 01:13:53 +00:00
Dragon	8eb33a860a	perf(core): drop the redundant history clone in session recovery planning (#11595) ... Classifying a resumed session deep-cloned the rebuilt API history twice: once to keep a pristine copy for interruption detection, and once for the orphaned-tool-use repair to mutate in place. Detection only ever reads, so the pristine copy can alias the array the caller passed in — every caller already hands over a snapshot it owns, and the repair still runs on its own clone so that snapshot is never modified. On a synthetic 50k-record transcript this removes ~85 ms from a resume with no compression and ~15 ms from one that compressed, against a total resume cost dominated by the transcript parse.	2026-09-11 01:01:26 +00:00
qqqys	5a9c9b3406	feat(goal): stop a Goal at a turn or an active-time budget (#11457) ... * feat(goal): stop a Goal at a turn or an active-time budget The token budget is a runaway-spend guard, sized for that job: 30,000,000 tokens by default, which a healthy long run reaches late. It is not what a user reaches for to keep a Goal short -- people say "at most twenty turns" or "at most half an hour". Neither had any effect, and the objective template invites the first phrasing while the user doc has to explain that writing it configures nothing. Add two ceilings beside the token one, both off by default: `model.goalMaxTurns` counts finished Goal turns and `model.goalMaxActiveMinutes` counts the wall time the Goal spends active. The meters were already on the record; only the ceilings and the stop were missing. A Goal that reaches either takes the path a spent token budget already takes: one wind-down turn to hand off, then `usage_limited` with a new `limitKind` (`turn_budget` or `time_budget`) and a reason naming the budget. A resume authorizes another window on top of what was used and moves only the ceiling that ran out. Ceilings are read at the continuation boundary, so the turn that crosses one still finishes; a turn crossing several reports one reason, token first. - `spentBudget` replaces the runtime's direct token check, so the continuation gate, the settle, and the no-progress bound cannot disagree about whether a Goal is out of allowance. The no-progress pause yields to every spent budget, not just the token one, so a cadence stop still gets its hand-off. - The reducer's budget-resume branch keyed off the literal `token_budget`; it now keys off `isGoalBudgetLimitKind`, or a turn-stopped Goal would resume still rendering "ran its turn budget" as the reason it is active. - The wind-down line stops naming the token budget, since the hosts carry a plain `windDown` flag and cannot say which ceiling was reached. It points at the budget line above it, which grew to report turns against their ceiling and, when one is armed, active minutes against theirs. - Both new `limitKind` values reach the two whitelists outside core: the SDK's hand-copied union and the Web Shell's `getGoalState`, which rebuilds the record field by field and would otherwise drop them on the live path. Design: docs/design/2026-09-09-goal-turn-and-time-budget.md and its .zh-CN.md counterpart. * fix(goal): repair the assertions and mocks the budget change broke Six CLI assertions pinned the continuation prompt verbatim, and renaming the budget line's prefix and rewording the wind-down line moved that text out from under them. Two more CLI suites could not collect at all: they build their `@qwen-code/qwen-code-core` mock by enumerating exports, and `settingsSchema.ts` now reads two new caps at module scope. Neither was reachable from the verification steps this change shipped with, which ran only `runBudget.test.ts` and `config.test.ts` in the CLI package. `cd packages/cli && npx vitest run` reaches all eight. Also, while in these files: - `get_goal`'s `lastGoal` summary already reported `tokenBudget`, so a Goal stopped by a cadence ceiling could be inspected and show only the allowance that did not run out. It now reports all three, and the tool description says so. - Two validator doc comments said "-1 for no ceiling, or a positive integer"; the predicates accept `0` as the same opt-out, the way the token budget's sibling already documents it. - `settingsSchema.test.ts` now pins both new entries: integer, no default, bounded by their caps, no restart. * fix(config): reject signed zero for the Goal cadence settings, and say what the ceilings actually do The `excludedValues` guard added last round compared with `Object.is`, which tells `+0` from `-0`. `Number()` maps `-0`, `-0.0`, `-.0` and `-0e0` to `-0`, `Number.isInteger(-0)` is true and `-0 < -1` is false, so `/config model.goalMaxTurns=-0` walked past the guard, reported `Set ... = 0` because `String(-0)` is `"0"`, and `JSON.stringify` persisted the excluded value. Every later start in that scope then aborted on `must be > 0`, with `/config` unreachable to repair it -- the same lockout the guard was added to prevent. Compare numbers with `===` so signed zero matches, keeping identity for the string half of the field's type. The documentation of both ceilings overclaimed in three ways, each fixed wherever the claim is published -- the two schema descriptions and their generated mirror, the settings table, the user guide, both locales of the Web Shell placeholder, three places in the goal-draft skill, and the `propose_goal` objective example: - `-1` was described as running Goals with no ceiling. It only removes a ceiling a Goal has already spent, on the resume or edit that follows; a Goal still under its ceiling keeps it. - Neither ceiling is retrofitted onto a Goal already on the record, so the only way to bound one is to replace it. Nothing said so. - The minutes row read as a wall-clock cap. There is no timer: the ceiling is read between turns, so a turn already running is never interrupted and the window can be exceeded by that turn plus the hand-off. The design doc's "no behavioural change when off" bullet and its first acceptance criterion were also too strong, in both languages. Restoring an `active` Goal rebases `updatedAt` unconditionally, so the elapsed figure a restored Goal reports drops offline time for everyone, settings or not -- and drops with it any active time in an interrupted turn that no journal write had committed. Both are now stated rather than implied, with the reason the record cannot separate the two. Two promises the tests did not hold: - Three surfaces say user turns are still admitted at the ceiling, and the only case covering it admitted the turn one step below the limit. A case now reserves a user turn with the ceiling already spent and the hand-off in flight; adding a `spentBudget` gate to `beginTurn` reds it. - The active-time ceiling is the only one whose spent state can change between queueing a continuation and delivering it. A case now queues under the ceiling with no host and binds past it, asserting the hand-off; making `bindHost` flush directly reds it. * fix(goal): name what replacing a Goal costs, keep ceiling advice out of objective text Round-three review, four findings. Replacing a Goal is the only way to bound one already on the record, and the docs offered it without the cost. `/goal set` goes through `createGoal`: a new Goal at revision 1, turn, token and active-time meters at zero, and an evidence cursor at the replacement, so everything the old Goal recorded drops out of the citable catalog. Said so on every surface that offers the remedy -- the user guide, both settings rows and both schema descriptions, regenerated. The ceiling settings were pointed at from two opposite places: the goal-draft template slot said "outside the objective", while its exemplar, the user guide's Budget row and strong example, the `propose_goal` objective example and the Web Shell placeholder all wrote the pointer into objective text -- text re-sent every turn to a model that cannot act on it, advising a setting that by its own wording never applies to the Goal carrying it. One placement now: never in objective text. The rules-of-thumb bullet names the settings, the user guide's Budget row says to set them in settings, and the Web Shell pointer moves from the placeholder into the hint rendered under the same field. `SKILL.test.ts` pins both halves -- the settings are named in the skill body and absent from the template and the exemplar -- so they cannot drift apart. The strong example also gains the `On block:` clause the template asks for. The signed-zero guard is one `includes` call: SameValueZero already matches `-0` against a `[0]` exclusion, so the typeof-split ternary carried an arm no setting can reach. The `-0` assertion from last round still pins it. The settings table rows are trimmed to fit the widest existing row, so the table is not re-padded, and one shortening that would have misdescribed which values are rejected is reverted to the accurate list. * fix(goal): stop the ceiling docs promising more than the runtime does Round-four review, five findings, all documentation precision. Each one is fixed by narrowing a claim rather than qualifying it further, so the surface the next round can find fault with gets smaller. - Bounding a Goal already on the record: "replacing it with /goal set or clearing it, which starts a new Goal" said clearing starts a Goal. Clearing leaves none. Only the replacement starts a new Goal at revision 1; clearing is followed by starting again. Both settings rows and both schema descriptions now say so, matching the user guide. - "Time while the process is not running does not count" overclaimed. The only exclusion is the rebase a restart performs; a suspended process, such as a sleeping laptop, is charged. The claim now says exactly that. - The quantified overshoot, "that turn plus the hand-off turn", holds only when a turn is running as the window runs out. A window that runs out while no turn runs is not noticed until the next turn ends. The bound is dropped in favour of saying the Goal can run well past the window. - The Web Shell hint sent readers to settings for two keys that client's settings surface cannot reach. It now names `/config` in the chat, which runs in daemon sessions. - The second weak-to-strong exemplar still wrote a bare turn count into its Budget. It carries the advisory marker like the first, and the skill test now checks every exemplar row instead of one. --------- Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com>	2026-09-11 01:01:02 +00:00
qqqys	af4dece3a7	feat(sdk): add a peer endpoint so a program outside Qwen Code can join cross-session messaging (#11560) ... * feat(sdk): add a peer endpoint so a program outside Qwen Code can join cross-session messaging `@qwen-code/sdk/peer` is an implementation of the cross-session protocol for a program that is not itself a Qwen Code session — a voice front-end, a relay, a build watcher. `PeerEndpoint.start({ name })` binds an inbox and publishes a session record, so the program is listed by `qwen sessions ps` and by every session's `list_agents`; `list()` and `send()` address sessions by name; `onMessage` receives what they send; receipts are applied as state transitions and can be awaited. It is written from the protocol page with Node's own modules alone and shares no code with Qwen Code. That makes it the page's first independent implementation, and a conformance suite runs it against Qwen Code's own registry, inbox, directory and send path in both directions: listing and reaching each other, frames parsed and answered each way, a folded drop receipt settling a burst, a user-minted controller token recognised and a forged one not, identical verdicts on hostile lines, record files and names, and every published limit pinned to Qwen Code's constants. The endpoint takes the shared `<pid>.json` when it is free and a minted `<pid>-<8 hex>.json` when it is held, never replacing a record it cannot prove dead. Its inbox accepts only its own token, it never deletes records or sockets other processes wrote, and trust comes only from a controller token. On Linux it refuses to publish a record without a start token and PID namespace. Implementing the page found two rules it left implicit, now stated in "Writing your own record": `procStart` and `pidNs` are required on Linux, and a held `<pid>.json` means writing the minted name instead. Separate Node-only subpath, kept out of the browser-bundled entries; the build checks its size and that it carries no runtime dependency. * fix(sdk): present the controller token only on sends marked controller Review follow-ups on the peer endpoint. The controller token was presented on every send, so any process that published a record under a name the program then addressed read the plaintext of a credential Qwen Code only ever stores hashed. A send now presents it only when marked `controller: true`; unmarked sends — from an endpoint holding a token too — present the recipient's own token. On a marked send, two records for one session id and name are ambiguous rather than collapsed to the newer, so a copied record cannot take the grant. A malformed `controllerToken` (empty, no `qpc_` prefix) is rejected at start with `invalid-controller-token`, as is a marked send without one. Other fixes: - Two `start()` calls in one process raced for `<pid>.json` and one record was lost; registrations inside a process now run one at a time. - `send()` checks `closed` again after the directory read, so a send overlapping `close()` rejects instead of writing a dead reply address. - `ambiguous.matches` holds only addresses that select one session. - Receipt `reason` is flattened and bounded like every other label. - The local concurrent-send ceiling is marked `local` and described as this program's limit, not as a busy peer. - An explicit `socketPath` no longer re-permissions a directory the call did not create. - `probePeerSocket` is renamed `probePeerSocketVerdict`, matching core; the status list is the single source of the status type. - `flattenPeerLabel` matches control and format characters by Unicode category and caps in code points, in core as well, so the two implementations still agree. - The build checks every emitted peer declaration, from a module with tests of its own; the surface test is compiled by the typecheck and matches imports as path segments. Docs: `list_agents` and `send_message` need `agents.crossSessionMessaging` on the receiving session; delivery without review also follows a matching `fromMode`, and `agents.crossSessionInbound` outranks a controller grant; a controller send to another endpoint is dropped unread; the endpoint's inbox applies none of the protocol page's §6; the `socketPath` sibling fallback; and the protocol page gives separate reasons for `pidNs` and `procStart`. The design doc no longer says the default entry ships to browsers. Tests pin each of these, plus the paths the first round left unexercised: ledger and answered-id eviction past their bounds, start rollback, the Linux identity guard, registry directory tightening and read errors, the over-long line with the default deadline, the symlinked and over-long socket paths, prompt close, Windows, and a live PID with a mismatched start token.	2026-09-10 22:13:39 +00:00
Shaojin Wen	30a1624752	Merge branch 'main' into codex/web-shell-session-continue	2026-09-11 05:45:09 +08:00
Canberk Pitirli	c46cb85cf2	fix(vscode-ide-companion): resolve diff paths once in DiffManager (#10494) ... * fix(vscode-ide-companion): resolve workspace-relative paths in closeDiff * fix(vscode-ide-companion): resolve diff paths once in DiffManager The MCP openDiff/closeDiff pair stored and matched raw paths, so a diff opened with one form of a path could not be closed with the other. Resolving only on the close side, as this branch first did, made that worse: openDiff kept storing the relative key while closeDiff looked up the absolute one, so no close ever matched. Resolve once inside DiffManager instead, and split the two jobs the stored path was doing. originalFilePath stays exactly as the caller supplied it and is what goes back out in the notifications, because the CLI keys its pending openDiff promise by the string it sent. The new resolvedFilePath is what everything matches on: closing, deduping, focusing, and the active-editor check that compares against an always-absolute editor fsPath. This also fixes the open side for relative paths, which read old content and built the diff URIs from a path relative to the extension host's cwd. Drops the duplicated resolution blocks in showDiffCommand and FileMessageHandler in favour of the shared helper. * fix(vscode-ide-companion): stop closeDiff from cross-matching entries and mangling the echoed path Two separate bugs review found in the resolve-once consolidation: closeDiff echoed originalFilePath after it had been normalized, so any caller path that wasn't already normalize-stable (src/./foo.ts, a //, Windows forward slashes) came back as a different string than the CLI keyed its pending promise by. The promise never settled, disconnect() never released diffMutex, and every later openDiff hung forever. originalFilePath is now stored verbatim from the caller. closeDiff also matched purely on resolvedFilePath and took the first insertion-order hit. DiffManager is a per-window singleton shared by every session, so two entries can share a resolvedFilePath with different content (a webview permission preview and a second CLI session's proposed edit). Closing one could return the other session's content, which the scheduler then writes out as a "user edit" on the wrong file. closeDiff now prefers the entry whose originalFilePath matches the caller's form, falling back to the first resolvedFilePath match only when nothing matches exactly. Both are pinned with red-then-green tests. Claude-Session: https://claude.ai/code/session_01DE7wVFkA8f4EKdePsHm3ee --------- Co-authored-by: wenshao <shaojin.wensj@alibaba-inc.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>	2026-09-10 19:30:59 +00:00
Zhu Lei	92f575906a	fix(serve): load persisted MCP config after ACP preheat at startup (#11145) ... * fix(serve): load persisted MCP config after ACP preheat at startup Reconcile workspace MCP configuration once the primary runtime bridge finishes preheating so persisted mcpServers settings are available without a manual reload. Fixes QwenLM/qwen-code#7771 * fix(serve): start workspace MCP discovery after ACP preheat Use the non-throwing primary runtime lookup and coordinator.ensure() so startup discovery runs on the workspace Config without re-enabling bootstrap MCP spawn. * fix(serve): skip keep-alive preheat after startup ACP warm-up Boot discovery now prepares the already-live runtime without a second client-route keep-alive preheat, so an idle daemon is not forced to a 10-minute channel window. Warm-start failures are debug-logged. * fix(serve): make ensure skip-keep-alive semantics explicit Document EnsureOptions.skipKeepAlivePreheat and the ensure overload so object-form live-runtime skip is no longer implicit. Log the two boot early returns, and use skip-path wording when a live snapshot dies before status(). * test(serve): restore registry after boot skip-path debug coverage Avoid polling daemon status after clearing the primary generation so the skip-no-runtime debug assertion does not race a broken status read. * fix(serve): hold the ACP child through boot MCP discovery Boot ensure({}) skipped keep-alive, so default channelIdleTimeoutMs=0 reaped the warm child before initializeWorkspaceMcp. Arm the 10-minute window and load createDebugLogger from the debugLogger subpath so the serve fast-path bundle stays closed. * fix(serve): address round-3 review suggestions for boot MCP discovery Extend the preheat success guard to skip discovery when runtime startup already failed, route boot diagnostics through daemonLog (including cause on failure), and add tests that go red if discovery runs on preheat failure, targets non-primary workspaces, or races shutdown/startup failure. * fix(serve): address round-4 review on boot runtime ensure Annotate the channel-worker snapshot mock for typecheck, split shutdown vs startup-failure skip guards, unwrap ensure failure causes for daemon logs, skip warn on WorkspaceDrainingError, rename logs to workspace runtime ensure, and add tests for cause-less failures and the runtimeStartupError guard arm. * fix(serve): drop bridgeErrors import and harden boot ensure guards Remove the forbidden bridgeErrors static import and classify ensure .catch failures via shuttingDown/runtimeStartupError. Check startup failure before shutdown when skipping post-preheat ensure, gate primary runtime on active state, and add tests for deepestErrorMessage, inactive primary, and the runtimeStartupError guard. --------- 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>	2026-09-10 19:25:13 +00:00
qqqys	07b1cd033e	refactor(core): move the web_search DashScope client behind a backend interface (#11490) ... * refactor(core): split the web_search DashScope client into a backend and surface page titles The tool description tells the model to cite sources as [Title](URL), but the result it receives carries bare URLs: DashScope's search items expose only {type, url}, so the model either invents titles or pastes raw links. Ask the search side model to open its reply with a "Sources:" block of "title — url" lines, attach those titles to URLs the search actually returned, drop anything it did not, strip the block from the narration, and render a source as a markdown link when a title is known and as a bare URL otherwise — so a result without titles is byte-identical to today's. The block is requested first because a search that exhausts the request budget is salvaged from whatever streamed, and anything asked for last is what a slow search loses. Move the Responses client, stream parsing, retry and salvage logic out of the tool invocation into a WebSearchBackend implementation behind a small interface, leaving the tool with permissions, the result envelope, truncation and the citation policy. No error path, timeout, retry or display line changes, so another provider's server-side search can plug in beside this one. * fix(core): stop the web_search Sources parser from editing narration or trusting page text - Require a bullet for plain source entries and drop a matched block all-or-nothing, so narration that merely ends in a URL survives and no orphaned bullet tail is left behind; gate entry parsing on line length to bound the regex backtracking. - Mine titles only from narrated text: salvaged extractor page content must not author its own citation title. - Run response-declared titles through cleanTitle and strip URLs smuggled into relayed titles; accept parenthesized markdown destinations and the bullet glyphs models actually emit. - De-duplicate the evidence tiers by normalized URL, drop the unread instance-level backend kind member, and narrow the docs, interface contracts, and the web_fetch bullet to the guarantees the code keeps. * fix(core): escape backslashes in web_search link text A title ending in a backslash consumed the escape added for the closing bracket, so `[C:\path\](url)` reached the model as literal text and the citation it was meant to protect was lost. Escape backslashes along with the brackets. Also drop the now-redundant export on SIDE_REQUEST_INSTRUCTIONS — nothing outside the module reads it, and the instructions are pinned through the request the backend sends. * fix(core): keep web_search citations reachable in the CLI and stop capped pages vanishing Three findings from the review, all in how a result reaches the reader. Percent-encode markdown destinations instead of wrapping them in <...>. The wrap was meant to protect parenthesized URLs, but the CLI only turns a link into an OSC 8 hyperlink when it can read a bare scheme, so the wrapped form lost the hyperlink and showed literal angle brackets for exactly those URLs; its own link pattern already handles one level of balanced parens. Encoding keeps the scheme in front and also covers what wrapping never fixed: nested or unbalanced parens, and `<`/`>` corrupting the OSC 8 target. Stop escaping `)` in link text — it needs no escape there, and nothing unescapes what the CLI renders, so it only showed the user a stray backslash. Give the block stripper the set of pages the formatter will actually render. Both evidence tiers are capped at 25, so a page past the cap had its Sources line deleted from the narration and was never listed either — it left the result entirely while the prose still credited it. Those lines now stay, with their heading. Fall back to extracted page text after the strip when the side model's whole reply was the Sources block, so a successful result cannot arrive with no findings at all. The fallback runs on the stripped value, never before it, so raw page text is still never mined for titles. * refactor(core): take page-title extraction back out of the web_search backend split Two review rounds on the title feature grew from 16 findings to 29, seven of them induced by the previous round's fixes, and nearly all of them land on one mechanism: parsing a free-form Sources block out of the side model's narration and then editing that narration. That design needs deciding on its own rather than being patched one finding at a time, so this change returns the PR to what it is underneath — the DashScope client moved behind a backend interface, with no behavior change — and the title work moves to a separate issue with its implementation preserved on its own branch. Output is identical to the merge base. Across 28 mocked scenarios covering success, truncation, the per-tier caps, stream salvage, every error mapping, the no-search retry, request parameters and progress updates, the content the model receives, the display line, the error, the request body and the client options all match byte for byte. The merge-base test suite passes unchanged apart from the new `kind` discriminator on the resolved backend config. Also drops the re-export of WebSearchBackendConfig from web-search.ts, which nothing imported. --------- Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>	2026-09-10 17:08:36 +00:00
qwen-code-ci-bot	0bb84a4153	chore(release): sdk-typescript v0.1.12 (#11589) ... * chore(release): v0.23.3 * chore(release): sdk-typescript v0.1.12 --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>	2026-09-10 16:22:02 +00:00
probe	ed2791e6a2	fix(web-shell): preserve continuation across reconnects	2026-09-10 23:57:50 +08:00
qqqys	4815941476	feat(goal): start approved Web Shell proposals after their owning turn (#11360) ... * feat(goal): start approved Web Shell proposals after their owning turn * fix(goal): close proposal lifecycle gaps * fix(goal): address settlement review gaps * fix(goal): block proposal settlement when a guard continuation yields to a queued prompt (#11360) #runStopContinuation's skipped-send funnel returned a plain end_turn for the queued-prompt and unreliable-drain stops without marking the turn's settlementBlocked flag, so a parked proposal approved earlier in the turn was applied even though the turn ceded to the queued user prompt. Set the flag at those three stops, mirroring the loop-level exits, and pin each with a regression that fails without it. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(cli): deflake the ab-drive run-dir sweep counts against foreign temp dirs (#11360) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(goal): revoke proposal approval when entering Plan mode * fix(goal): give ACP proposal settlement one owner --------- Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-09-10 15:43:28 +00:00
qqqys	c3d9bed85b	fix(serve): restore configured channels after restart (#11472) ... * fix(serve): restore configured channels after restart * fix(serve): make configured channel restore fail soft * fix(serve): identify configured channel source * fix(serve): harden configured channel recovery * fix(serve): bound configured channel startup restoration * fix(serve): preserve fail-soft channel startup --------- Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com>	2026-09-10 15:40:24 +00:00
jinye	cbd2cbadec	fix(acp): Preserve submitted prompt provenance for auto recall (#11455) ... * fix(acp): Preserve submitted prompt provenance for auto recall Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(acp): Omit submitted_prompt on channel-classified turns (#11455) Channel turns are machine-relayed deliveries (loop jobs, webhook tasks, adapter-synthesized events), so nothing on them crossed a submission boundary. Since submitted_prompt presence alone gates Auto Recall's outbound provider search, emitting it there ran retrieval on unattended turns and could publish composed wrapper text as user provenance. Gate emission on a positive user-submission condition instead. Also reconcile the producer-set contract this PR changed: hooks.md, the Auto Recall READMEs, the in-product hook description with its locale mirrors, and the recording payload comment now state the actual rule, and the new design doc records the channel exclusion plus the strict-decoder rollout warning. * fix(acp): Require explicit submission provenance for auto recall Carry original Web Shell composer text separately through admission and queues; keep background dispatches and undeclared requests ineligible. Reconcile hook and integration contracts and add regression coverage. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>	2026-09-10 15:17:51 +00:00
Shaojin Wen	67fe20a838	fix(autofix): charge regressions to the brake and gate test weakening (#10188) ... * fix(autofix): charge regressions to the brake and gate test weakening Two holes let an autofix round ship a new problem for free. A round that pushed a fix and turned CI red counted as a SUCCESS: the consecutive-failure brake counts "rounds that pushed nothing", so the red it created reset the counter, came back as the next round's input, and was paid for out of the round budget. A PR could alternate regress/repair indefinitely while every brake read it as converging. Every acted round now stamps what it pushed and whether the head it pushed onto was fully green; the next round charges a regression only when the live head is exactly that sha, the marker says green, the window key matches, and the head is red now. The observing round records it in whichever report it posts, and the brake stops resetting on a regressing round. Attribution is conservative on every axis: a moved head, a re-arm, a pending check at push time, a cancelled run, or the loop's own lanes all drop the charge. The second hole: relaxing an existing assertion was invisible to the whole gate. Build/typecheck/lint never read assertions, the package tests run the weakened file, and the bite check reads only the tests a round ADDS. The SKILL already required content evidence for deleting or weakening a test; nothing enforced it. The gate now measures deleted test files, net assertion removal, and added skip/todo markers, and rejects the round unless each file is recorded in test-weakening.json with a reason. It judges that the claim exists, not that it is right — the reasons ride into the round report for a maintainer to read against the diff. Deletions get their own merge-freight discriminator: content equality reads "absent on both sides" as identical, so a round deleting a test the PR itself added looked like freight and escaped. The merge base separates the two cases. * fix(autofix): attribute test weakening to round commits, close marker escapes * fix(autofix): close weakening-gate merge/typechange escapes, own-lane charge leaks (#10188) * fix(autofix): close weakening-gate comment, attribute, and name-framing escapes (#10188) * fix(autofix): close weakening-gate masking escapes and delete/restore false charge (#10188) * fix(autofix): close weakening-gate lexer, marker-shape, and verdict escapes (#10188) - WEAKEN_ASSERT_RE: left-boundary class (console.assert( never fails a test) and a matcher requirement on added expect( lines (matcher-less expect passes for anything); the del side stays bare so a multi-line-formatted assertion removal still charges - skip markers: concurrent/sequential/shuffle chains, tagged-template each tails, computed accessors, and newline-split chains measured on a joined view; sd counts comment-stripped del lines - weaken_strip_comments gains string/template state (an in-string /* can no longer poison the strip); weaken_strip_code gains regex-literal and template-nesting state - modify/delete merge resolutions that KEEP the file apply no freight - the net-range deletion arm enumerates the explicit pre-round->tip pair and is judged even when per-commit measurement is UNAVAILABLE * fix(autofix): close weakening-gate member-assertion, marker-spelling, and lexer escapes (#10188) - WEAKEN_ASSERT_RE/ADD_RE: the member arm requires the CALL (an exported assert alias executes no assertion), and supertest's throwing member form .expect( counts on every arm - WEAKEN_SKIP_RE: backtick-quoted computed accessors, the ?. optional-chaining spellings (it?.skip( and it.skip?.(), and the reversed chain with an execution modifier behind the marker - weaken_strip_code: regex-vs-division is keyed on the last significant TOKEN (an operator including /, a regex-head keyword preserved across whitespace, or a control-head close paren tracked on a paren-kind stack), not the last output character - weaken_strip_code/weaken_strip_comments: a ${} hole closes on the brace matching its own ${ (per-hole brace depth), not the first } inside the hole (an object literal or arrow body) - the skip-marker net gains the whole-blob backstop twin of the assertion arm, so a genuine addition cannot net against an in-comment marker deletion or hide behind a block-comment span opened above the hunk - twelve witnessed fixtures (each guard flips its own test red under mutation); fixtureWrite emits single-quoted shell words so backtick/${ fixture content survives * fix(autofix): close weakening-gate merge coverage, freight phantom, and lexer escapes (#10188) * fix(autofix): close weakening-gate cross-commit, merge-discard, and skip-surface escapes (#10188) * fix(autofix): close weakening-gate matcher-credit, each-skip, and merge-recount escapes (#10188) * fix(autofix): close weakening-gate tail-drop, skip-netting, fast-forward, and JSX escapes (#10188) * fix(autofix): stop the weakening gate charging honest test edits (#10188) Three of the four measured defects were false rejects the gate published as machine-measured fact, and the fourth was test surface it never saw. The matcher-tail census counted every member call in a stripped blob, so deleting a spy and its restore read as three removed assertion lines; scope it to continuation-shaped lines, the only shape that arm exists to see. A fast-forwarded main recorded a recount base only for files it added, so main's own delta was charged to a round that synced and then renamed a symbol; record bases for modified files too and read the ledger instead of set membership. The body-call skip arm charged vitest's condition-valued dynamic skip, which is the runtime spelling of the .skipIf environment guard the doctrine already exempts. And `*_tests.rs` matched no pathspec arm, leaving two tracked Rust test files invisible. Each fix carries a witness that a mutation probe confirms goes red without it; the pinned guards for all four arms stay green. * fix(autofix): measure test weakening with the parser, not a text census (#10188) Replace the awk lexer, the four extended regexes, the matcher-tail census, the per-commit accumulators, the verdict-time recount and the netting skip with one instrument: count-test-surface.mjs parses the whole file with the TypeScript compiler and reports its declared test surface (statement-level assertion chains, test/describe registrations with their enabled state, bare early returns ahead of assertions). Each file's round delta is tip - pre-round - main's contribution, where main's contribution across a merge is measured on git's own auto-merge of main's side (git merge-file --ours) and across a fast-forwarded main commit on the commit itself, so the same weakening measures the same before, during or after a merge, and main's delta neither charges nor shields. Narrow the pathspec to test-NAMED files (a README under integration-tests/ is not a test deletion), read the enumeration producers' status instead of swallowing it, re-apply the reason bar when rendering, and state the declared non-goals in the gate header: reachability, condition-valued guards and by-reference options are runtime facts the runner-backed instruments judge. Bind the af-155 regression classifier to the commit its rollup describes (headRefOid read with the rollup; a moved head classifies none), exclude the loop's own dispatch-pending StatusContext by its exact context value, classify verdict-less checks (EXPECTED, STALE, STARTUP_FAILURE) as pending, stamp pre=none after a salvage-merged push, drop the unread CHECK_STATE from the failure report's env, and render the regression disclosure on every report shape. Drop the hand-rolled CommonMark fence model from workflow-size.test.js: the existing anchor/pointer/contents pins already prove navigation, and a text model of a renderer is the class of instrument this change removes. * chore: record the merged qwen-autofix.yml size * fix(autofix): stage the surface counter with the pre-merge absence guard (#10188) The stage step copies count-test-surface.mjs from the trusted base, but the script is new in this PR: pre-merge the base lacks it, and a bare cp under the step's -e kills every PR-event round whose workflow resolves from the PR's own ref — the deferral the reviewer kept re-reporting. Give the cp the heartbeat guard: rm -rf first so a planted leftover cannot survive a failed copy and be executed as trusted (af-149), then || true so absence skips; the gate already fails open on a missing counter (WEAKEN_MEASURED=false). Pin the pair and their order beside the heartbeat pins. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(autofix): close the weakening gate's fail-open paths and make green an allowlist Answers rounds 21-26 by retiring mechanisms rather than adding entrances. Head-state classifier (af-155): GREEN is now an allowlist, never the fall-through. A charge-enabling verdict requires every counted check to hold a state the classifier recognises as passing, so CANCELLED, NEUTRAL, SKIPPED and any state GitHub adds later read as pending instead of minting the premise. (R22-3) Test-surface counter: a run that exits 0 without printing a readable verdict is a measurement FAILURE, not a verdict. weaken_measure validates the shape before returning it, so the file takes the existing fail-closed arm ("test surface could not be measured") instead of being silently uncharged while the round still reports itself measured. (R26-2) Merge attribution: git merge-file's failure test is its exit status, not its output size. --ours resolves conflicts and exits 0, so a legitimately empty result (main emptying a test file) was never conflated with failure -- the line is replaced anyway so the intent no longer has to be re-derived from a size test, and pinned by a fixture that makes main's side zero bytes across a real merge commit. (R21-4) Report-post fallback: gated on the af-155 state the round AUTHORED, not on its outcome. A no-op round's observation about the PRIOR round was unrecoverable once its report failed to post, because the marker it read is superseded by the next push and the workflow's handoff comment excludes exactly the fixed/noop outcomes. (R22-1, R26-3) Instrument staging keeps the pre-merge absence guard it was given here and moves to its own step after npm ci, so the parser is a .cjs copy of the typescript the TRUSTED lockfile installed; both digests travel in expression context and are verified before the counter executes. A verbatim absence probe runs the step against a script-less tree with leftovers planted in both shapes. Boundaries stated where they can be read: --ours is the attribution model and its error deliberately lands on the charging side; the base axis is a documented residual with the same bound as the flake carve-out; the SKILL no longer promises assertion counting for the Python and Rust shapes, which are watched for deletion alone. 508/508 in scripts/tests; five mutants (green default, verdict validation, emptiness-as-failure, outcome-gated fallback, no-op event overwriting the baseline) all killed. * test(autofix): execute the paths the weakening gate's own guards protect A reverse audit of the previous commit found two guards that no test reached, both load-bearing in the fail-open direction, plus one hazard in the counter's CLI dispatch. git merge-file refuses a file carrying a NUL byte (exit 255, no output), and the branch's own side has to stand in for the auto-merge then. Nothing executed that arm: making the substitution unreachable left the whole suite green. Without it the refusal's empty output reads as main having emptied the file, which credits the round with the entire baseline surface and masks a removal of up to that size. New fixture `binary-merge` -- main appends a comment, the round drops an assertion, and the NUL makes every side of the merge binary -- charges `net 1 assertion(s) removed` and fails when the substitution is removed. The round rewrites the file rather than filtering it, because a line filter reads a NUL-bearing file as binary and would empty it instead. The staging step's present-counter branch -- the one that runs in production once this lands -- was pinned only by text. Swapping `mkdir -p` and `cp` inside it left the suite green while killing every round, because the step is unconditional and runs under `-e`. The probe now runs the step body the way GitHub runs a `run:` block (`bash --noprofile --norc -e -o pipefail`) in both shapes: the pre-merge tree that lacks the script, and a tree that has it with a parser planted where npm ci puts one, asserting the staged bytes and both digests. The counter's CLI dispatch ran at module scope, so importing it executed it against the importer's argv -- a positional the importing program owns lands in the unknown-mode arm and takes that process down with exit 2. It now runs only when the file is the program, compared as REAL paths: the gate runs it from RUNNER_TEMP, which on macOS is reached through /var -> /private/var, and a literal comparison silently disables the CLI and reports every file unmeasurable. Both halves are pinned, including a run through a symlink. Also: the head-state classifier keeps the status axis it had, so a check still in flight is pending even when it carries a conclusion from an earlier attempt; and the dialect table is exercised for every extension the pathspec selects, not only `.ts` -- deleting the JS entries was invisible to 52 tests while every tracked `*.test.js` file is measured by the gate. 509/509 in scripts/tests. Mutants killed: the merge-file substitution made unreachable, the staging order swapped, the main-module guard compared literally, the JS dialects deleted, plus the five from the previous commit. * fix(autofix): follow git's trivial-merge rule when merge-file refuses A reverse audit of the previous commit found that its new binary-merge fixture pinned THAT a substitution happens, not WHICH side it takes -- and the side it took was wrong half the time. When `git merge-file` refuses (a NUL byte makes the file binary, exit 255, no output), the previous commit substituted the branch's side unconditionally. That is right only when main's side is unchanged from the base. When MAIN is the side that moved, it reads main's own weakening as the round's and rejects a round whose only act was `git merge origin/main` -- the exact property the neighbouring main-empties fixture already guards, re-broken by a second trigger into the same substitution. The fallback now follows git's own trivial-merge rule: a side that did not move from the base contributes nothing, so the other side IS the result, and only a genuine two-sided change resolves for the branch the way `--ours` would have. Three fixtures pin the three shapes -- the round weakens (charged), main weakens (accepted), both move (charged for the round's own removal only) -- and both fixed-side choices now fail. The head-state classifier's status axis becomes an allowlist too. The denylist it had enumerated four in-flight states and missed REQUESTED, a real CheckStatusState member that four other sites in this same workflow already list; the axis is now `status == "COMPLETED"`, with an absent field reading as finished because a commit status carries no status field at all. Both axes are allowlists for the same reason a denylist forgets the member it was not told about -- which is how CANCELLED entered the green arm in the first place. The classifier's rationale moves to qwen-autofix.md#af-155, leaving the workflow 1024 bytes below the hard gate. 509/509 in scripts/tests. Mutants killed: always take the branch's side, always take main's side, the four-member status denylist restored, the status axis dropped entirely. * fix(autofix): bound main's contribution by what the merge actually took A third reverse audit found a false ACCEPT in the attribution model, in both lanes: the auto-merge is computed from the two sides and never consults the merge commit's own tree, so a merge that DISCARDED main's side still books main's delta as a contribution. A round can therefore merge main with `-s ours` and delete an assertion in the same breath: the credit for main's discarded weakening absorbs the round's own removal exactly, and the round ships uncharged. The previous commit widened the reach of this by extending the trivial-merge rule to the binary lane, where the same shape had been rejected before. Main's contribution is now bounded by what the tip took. When the merge result is the branch's side byte for byte, main contributed nothing to that file, whatever an auto-merge would have produced, and nothing is subtracted. A resolution that took main's version, or produced something new, still goes through the model unchanged -- including `merge-conflict- drop`, whose resolution commit differs from both sides. Two fixtures pin the shape in both lanes: `strategy-ours-hole` and `strategy-ours-hole-binary`, each a round that merges `-s ours` and removes an assertion. Deleting the bound leaves both accepted. 509/509 in scripts/tests. * fix(autofix): clamp main's credit by what the merge actually landed The previous commit's bound was byte-exact: it fired only when the merge result equalled the branch's side to the byte. A fourth reverse audit showed one added comment line inside the merge reinstates the whole hole, and the blast radius is main's ENTIRE modelled delta -- a round can strip every assertion from a pre-existing test file and ship it uncharged. Main's contribution is now the part the MODEL and the LANDED blob agree on, and only where they agree in sign. Equal, and the clamp is the identity: the ordinary merge measures as it always did. They diverge exactly when a resolution took neither side whole, and then the clamp cuts both ways -- a resolution that discarded main's side credits nothing, and a resolution that weakened the file itself is not main's contribution either. `weaken_auto_blob` emits three lines now, always: the modelled auto-merge, an optional override for the side to compare against, and the blob the merge commit actually holds. Presence is deliberately NOT clamped: the baseline is what the round had available to weaken, and letting the merge result define it would let a round discard a test main added and answer for nothing -- the one thing the deletion signal exists to catch. What the clamp cannot see is identity. Main removing one assertion while the resolution puts it back and drops a different one nets to zero, the same way an assertion moved within a file always has; that boundary is now stated in the header beside the model. Fixtures: `ours-hole-plus-comment` is the evasion the byte-exact bound missed, and `binary-resolution-takes-main` pins the binary fallback's branch-side arm -- which the clamp had quietly un-covered -- at its declared over-charging direction. 509/509 in scripts/tests. Mutants killed: the clamp returning the model, the clamp returning the landed delta, the landed blob never emitted, the binary fallback's else arm taking main's side. One survivor recorded: that arm taking the merge base, which the clamp makes equivalent in every shape a fixture can reach. * fix(autofix): clamp only main's removals, never its additions The clamp the previous commit added was sign-symmetric, and its two signs have opposite consequences. Capping a REMOVAL cuts the shield, which is what it was written for. Capping an ADDITION cuts the charge -- so a round could discard whatever main added to a test file mid-round and pay nothing. A fifth reverse audit demonstrated it through the real gate in three spellings, none of them needing a merge strategy flag: an ordinary merge resolution that drops the assertion main just landed, a delete/modify resolution that drops the test main appended, and the same resolution keeping that test registered `.skip`. The commit that added the clamp already left PRESENCE unclamped for exactly this reason, and said so; the reasoning was never carried into the deltas. It is now: main's additions raise the baseline whatever the merge kept, and only its removals are credited as far as they landed. It also contradicted a promise the gate header makes -- the same weakening arriving by fast-forward was charged while the one arriving through a merge was not. The event skip goes back to testing the model alone. Its second half was added to let a landed-only divergence through, but the clamp returns zero for every such event, so the only thing processing it could still do was latch `baselinePresent` off a side main never held: a file the round authored itself, touched while resolving a merge and dropped afterwards, was charged as a deleted pre-existing test. Six fixtures, all through the real gate: the three discard spellings (charged), the round's own file edited in a merge and dropped (accepted), main removing two where the resolution keeps one and the round then removes another (charged for its own), and main disabling a test the resolution keeps enabled before the round disables it (charged). 509/509 in scripts/tests. The four mutants that survived the previous commit are now killed: both clamp branches, and `clampBag` returning either bag unclamped. Presence clamped by the landed blob, and the skip processing every event, are killed too. * fix(autofix): measure main's contribution where main made it A sixth reverse audit found two more defects, and both trace to the same mechanism: main's contribution was read off a three-way SPLICE of the two sides (`git merge-file --ours`), which mixes the round's own edits into what is supposed to be main's delta. The splice can invent a surface neither side ever had. Witness: the round un-skips a suite, main adds a test inside that suite, and the resolution takes main's file verbatim -- so the tip is byte-identical to main's and nothing was weakened. The splice combined the round's un-skip with main's addition into an ENABLED registration that existed in no tree, and the round was charged for disabling it. The splice also decided PRESENCE, through an endpoint that is the branch's side in two arms. A path both sides added during the round, resolved to main's file and then deleted by the round, read as "the baseline never held it" and shipped uncharged -- while the gate's own `weaken_baseline_holds`, the definition its fail-closed arm uses, said the opposite. So the splice is gone. Main's contribution is main's OWN delta, measured on main's own side against the merge base, and presence follows main's own side directly. That makes the two arms one rule: a modify/delete resolved for main was already measured from the merge base. `git merge-file` leaves the gate entirely, and with it the binary-refusal fallback, its trivial-merge rule and the over-charge they carried -- a resolution that takes main's version now charges the round for nothing, where the model used to bill it for main's removal too. What the merge actually DID with main's contribution still clamps it, as before: removals credited only as far as they landed, additions never clamped. 509/509 in scripts/tests, with the gate section 60 lines shorter. Four new fixtures: the splice artifact (accepted), the add/add path dropped after the merge (charged), a resolution that removes more than main did (charged for the difference), and one that moves opposite to main (charged for the round's own removal only). Mutants killed: main's delta measured against the branch side, presence forced either way, removals credited from the landed blob, the landed guard dropped, additions clamped down. * fix(autofix): decide the baseline by what main LANDED, not by what it holds A seventh reverse audit found three defects in the new model. Two are false rejections with high reachability, one is a false accept that a git tie-break decides. Presence flipped true whenever main held the file at a merge, even when main's side was byte-identical to the merge base -- main landed nothing there. So a test file an EARLIER round removed, or renamed, is charged `test file deleted` again in every later round that merges main, until main also loses the file. The gate's own header promises a rename costs one ack entry naming the old path; it was costing one per round. Presence now moves only when main's side and the merge base DISAGREE about the file existing. Main adding it during the round puts it in the baseline, and the round answers for dropping it afterwards. Main deleting it takes it out only when the merge adopted that deletion. Main merely still holding a file it has always held says nothing. The "not an event at all" early return is gone with it. It dropped main's deletion from the running total while the NEXT merge's base already reflected that deletion, so main's re-add was credited from zero a second time: add, delete-with-the-round-keeping-its-copy, re-add was charged three assertions where one was removed. Every merge event is recorded now, and the clamp handles the resolution that kept the round's copy -- its landed delta is zero, so main's modelled deletion credits nothing. A criss-cross history has more than one equally valid merge base and git picks one without promising which. Since main's delta is measured against that base, the pick decided the verdict -- and one of them credits main with the ROUND's own removal, waiving a real weakening. The file is now refused rather than measured: `test surface could not be measured`, which one ack entry answers. 509/509 in scripts/tests. Three fixtures for the presence rule and the event chain; both presence mutants (latching main's hold unconditionally, and clearing on main's deletion whatever landed) are killed. The stale auto-merge prose in the counter's header, its clamp comment, an orphaned doc comment in the gate and two leftovers in the suite are corrected. * fix(autofix): chain main's events, and let the merge decide what it adopted An eighth reverse audit found that the previous commit's headline fix does not work and its fixture is vacuous, plus two false accepts it introduced. The removed early return was load-bearing. Its case -- main deleted the file, the merge kept the round's own copy -- is now recorded as an event whose `landed` blob IS the round's copy, so a copy weaker than the merge base credits MAIN with the round's own removal. The round guts a test file, main happens to delete it, the resolution keeps the gutted copy, and the round ships uncharged. The early return is back. What that removal was meant to fix is fixed properly: main's events CHAIN. Each event now measures main's side against main's side at the PREVIOUS event, not against a fresh merge base that already reflects it, so the events telescope to main's own net for the round. Add, delete-with-the- round-keeping-its-copy, re-add was charged three assertions where one was removed -- and it still was after the previous commit, because recording the deletion with a landed delta of zero is behaviourally identical to dropping it. The fixture missed it by re-adding byte-identical content, so the third merge was not an event at all; it re-adds a comment line now. Presence follows what main CONTRIBUTED, not what it holds. `mainHolds` alone exempted a whole path for the round whenever the pre-round ref lacked it -- including when main GREW it and the round restored main's file and gutted it. The rule is now: main holding the file and having moved it puts it in the baseline, whatever the resolution then did; main deleting it takes it out only when the merge adopted that deletion. The criss-cross refusal was charging honest rounds. More than one merge base only matters when the candidates DISAGREE about the file, which is now the test; when they hold the same blob the pick cannot change the verdict and the file is measured. Two fixtures pin both sides of that. 516/516 in scripts/tests. Six mutants killed, each the exact regression it names: the early return removed, the chain removed, presence from `mainHolds` alone, presence cleared without checking the base, the refusal made unconditional, and the refusal dropped. The stale presence comment the previous commit missed, and `weaken_baseline_holds`'s claim to be the same definition the counter reports, are corrected. * fix(autofix): drop the event whenever main holds no side and the file stays A ninth reverse audit found that the reinstated early return was too narrow, that presence read byte movement where it meant surface movement, and that the criss-cross guard compared the wrong identity. The early return required the merge base to hold the file. One merge later it does not: after main deletes a file the round kept, the NEXT merge sees no main side AND no base, the event is recorded, and the round's own copy becomes the landed blob that sets the clamp -- crediting main with exactly the round's removal, one merge after the guard that exists to prevent it. The base is not part of the test any more: main holding no side while the file stays in the round's hands is the same case however the history reached it. Presence asked whether main's side moved BYTES. Main appending a comment to a file an earlier round deleted contributes no coverage, and reading it as a contribution re-charged that deletion in every round main happened to touch the file -- and made the verdict depend on how many times the round merged main. It asks about the measured SURFACE now: the totals and both registration multisets. The criss-cross guard compared `git rev-parse <base>:<path>`, which returns a TREE oid for a directory while the measurement treats a tree as no blob at all. Two bases that agree for the measurement read as a disagreement, and an honest round was charged unmeasurable on a tie-break that could not have changed the verdict. It compares blob-or-absent identity now, the same question `weaken_blob` asks. One finding is answered with a stated precondition rather than code. The model reads main's side as MAIN's, which holds while origin/main does not contain the round's own commits -- true here, where pull requests land squashed and main's first-parent history carries no merge of a PR branch. A guard for it was written, found unreachable from every history these fixtures can build, and removed in favour of saying so in the header. 516/516 in scripts/tests. Three fixtures, and with them the two mutants the audit found surviving are killed: the caller's skip of a dropped event (previously indistinguishable, because no fixture had a prior main event for the file) and the early return's base condition. The clamp's `landedHolds` condition is now redundant for manifests the gate produces, and says so. * fix(autofix): read movement from the bytes where the surface is unmeasurable A tenth reverse audit found that the previous commit's surface test disabled the deletion arm for every non-JavaScript test file -- 45 Rust and 18 Python paths in this repository. `count()` reads no surface from those shapes, so two present blobs always compare equal and "main moved it" could never become true. Once the pre-round ref lacked such a path, nothing main did during the round put it back in the baseline, and the deletion arm -- the only arm those shapes have, and one the gate header promises never fails open -- was waived. Movement now falls back to the bytes when neither side is a dialect the instrument reads, because there "the surface did not move" is unmeasurable rather than false. The surface test also compared main's DISABLED registrations, which reach no signal: only the tip's do. Comparing them raised the baseline for a change the gate can never charge -- main adding an `it.skip` re-charged a deletion an earlier round had already answered for, which is the exact failure the surface test was written to remove. It compares the chargeable quantities now: the assertion totals and the enabled-registration multiset, of which `enabled` is a derived count. The criss-cross comparison goes through `weaken_blob`, the reader the measurement itself uses, instead of a second identity built from `rev-parse`. A tree, a gitlink and a missing path are "no blob" to all of them alike, and a mode-only difference is the same blob, so the guard refuses exactly the disagreements that could change a verdict. Two fixtures for the deletion arm: a Python file main grows while the round keeps its earlier deletion, and a file main adds whose measured surface is indistinguishable from an absent one. Both are charged; both fail if the fallback or the presence rule's existence test is removed. The `main-absorbs-round-commit` fixture is deleted. The audit showed it measured nothing -- the file is byte-identical on both sides of its merge, so the event filter drops it before attribution runs -- and its comment claimed a mechanism that was not the one making it pass. The precondition it stood for is stated in the header instead, with its justification corrected: a squash-merge of the round's own work into main mid-round is the spelling that breaks the model, not a merge, which is self-correcting because the merge base carries the same commit. 516/516 in scripts/tests. * fix(autofix): decide the byte fallback by what was measured, not by the extension An eleventh reverse audit found that the previous commit's fallback keyed on the file's EXTENSION rather than on whether the instrument read anything. A `.test.ts` that registers nothing measures exactly like a `.py` -- and this repository has one, a type-level fixture under the CUA driver -- so its deletion could still be waived after an earlier round removed it. The test is the measured surface now: no registration of either kind and no assertion, on both sides. The audit also showed the previous commit's narrowing of the surface comparison changed real verdicts with no coverage, in either direction. Main moving only DISABLED registrations is no coverage and reaches no signal, so it must not put a file an earlier round deleted back into the baseline; re-adding that comparison now fails a fixture, as does dropping any of the three fields that remain. The criss-cross guard's presence arm was equally uncovered: both shipped criss-cross fixtures disagree about the file's CONTENT, and the variable that tells "no blob" from "the first candidate" was free to be removed. A fixture whose candidate bases disagree about whether the file exists at all pins it. Three fixtures, three mutants killed: the fallback keyed on the extension, the surface comparison widened back to disabled registrations, and the criss-cross presence guard removed. The header's three claims are corrected. It said movement falls back to the bytes for "the Python and Rust shapes" (it is any file that measures to nothing, in any dialect), that presence follows main having moved the surface (main ADDING the path raises it with no movement at all), and that comparing merge bases through `weaken_blob` buys agreement on trees, gitlinks and mode-only changes (the identity it replaced already had all three; what it buys is one reader instead of two that can drift apart). The byte fallback's cost -- main touching such a file re-charges a deletion an earlier round answered for -- is stated rather than left to be discovered. 516/516 in scripts/tests. * fix(autofix): let a skips-only file reach the bytes, and pin every field A twelfth reverse audit proved the previous commit introduces no new false accept -- structurally, and over forty thousand generated histories -- and found one inconsistency and a coverage claim that was false. `isEmptySurface` counted a disabled registration as content while `sameSurface` cannot see one. A file that is NOTHING BUT disabled registrations therefore took the comparison branch, where the comparator is blind to the only thing in the file, so no rewrite of it could ever read as main contributing. This repository has one such file. It reaches the bytes now, like every other file the comparison cannot read, while main moving disabled registrations on a file that has other content is still not coverage arriving. Four fixtures, four mutants killed, and each of the comparison's three fields is load-bearing on its own for the first time: main planting a guard (the live total falls, the declared total stands), main deleting an assertion already behind one (the declared total falls, the live total cannot), and main renaming a test (both totals stand, the multiset moves). Those three survived every test in the file before. The previous commit's message claimed dropping any of the three fields fails a fixture. It did not; only the re-added disabled comparison was pinned. The claim is not repeated here, and the comparison's own header now says what it leaves out and where those files go instead. The criss-cross presence assertion also gains the path check its sibling already had: exiting 1 does not discriminate, because every guard mutant still exits 1 -- charging main's own deletion to the round, which is the hazard the guard exists for. 516/516 in scripts/tests. * fix(autofix): close the escape hatches the review found in the weakening gate - count-test-surface: see through type-only wrappers (`as`, `<T>x`, `satisfies`, `!`) in constant folding, options objects and call chains; fold comparison/equality/logical operators of two constants; treat template-literal returns as non-thenable - count-test-surface: an `if (true)` wrapper or a catch whose try holds an assertion no longer shelters a body skip; a registration callback handed by name resolves to its module-scope binding; a collector factory binding (it.skipIf / it.each / test.extend) registers nothing - af-155: charge a regression only when the run's window IS the live re-arm key, and never stamp pre=green when the push carries a merge past the head prepare classified - tests: exercise all five own-lane names in the charge classifier, and gate the verbatim staging witness on the host having sha256sum Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(autofix): arm-aware guard conditions, lane-list dedup, branch-side clamp (#10188) Holes the round-27/28 review kept re-asserting on top of a211bc0: - count-test-surface: the early-return silencer now fires only when the runner provably reaches the return on EVERY entry — underCondition is arm-aware for constant if/ternary/short-circuit operands (the executed arm is no condition; the skipped arm is dead code), so the environment-guard idiom `if (!cond) return;` keeps its assertions measured (R27-21, R27-27's sync/async asymmetry, R27-1's else-arm). HEAD's own contract.test.ts measured {assertions: 0} off that shape. - constant() folds arithmetic and bitwise operators with JavaScript's own semantics; opaque placeholders and bigints never enter the fold (R27-19's remaining arms). `fixme` joins the disabling set (R21-2's Playwright arm). - measure: a merge event's landed delta is measured against the branch's own side (new manifest `branch` field, emitted by the gate as weaken_emit's fifth line), so a resolution that discarded main's removal credits main with nothing (R27-20). - af-155: the loop-owned lane exclusion lives in ONE workflow env entry (LOOP_OWNED_LANES) referenced by all three head-state classifiers; the hand-copied lists had drifted six lanes short (R27-23). Tripling the list inline would have crossed the 470000-byte workflow start-runs gate (the file stood 115 bytes under it), so the af-149 kill-block prose moved to qwen-autofix.md per the file's own doctrine. Every guard change is pinned discriminatively and mutation-checked both ways; the gate-level fixtures re-spell their weakening shapes unconditionally, with the conditional spellings pinned as no-charge. * fix(autofix): Playwright-aware kinds, placeholder-safe folds, fallback numbering (#10188) Round-30/31/32 leftovers: - count-test-surface: the prefix arm no longer folds an operator over a placeholder value — `-[]` is -0 (falsy) at runtime, never the placeholder's -1 (R32-1); the object/bigint opacity guard is hoisted and shared by the unary and binary arms. - count-test-surface: Playwright's namespaced API is booked by kind — `test.describe` is a suite whose disabled state propagates, `test.beforeEach` is a hook, and the utility members (`test.step`, `test.use`, …) register no phantom test (R30-2). - push-and-report: the marker-only fallback now also carries the eval marker, so a fallback-posted round consumes its round number and the next round does not reuse it (R31-2's numbering arm). - review-verification: the weakening rejection's ack list rides its own workdir file and the remedy names it — the tail-bounded rejection document used to cut a long list from the front while telling the agent to ack "every file listed above" (R32-2). Each new guard is pinned discriminatively and mutation-checked; the Playwright row fails red without either the member gate or the kind override, and the placeholder row fails red without the prefix guard. * fix(autofix): measure a merge's landed delta by which side the resolution took (#10188) The R27-20 branch-side baseline was right for a resolution that KEPT the branch's side and wrong for one that took main's whole: the binary-resolution-takes-main gate fixture (caught on CI) charged the round for main's landed removal because `landed - branch` folds the round's own discarded edit into the delta. The clamp now reads the resolution per event: took main's side whole → main's delta landed exactly; kept the branch's side whole → none of it did (the R27-20 case); a mixed resolution is still measured against the branch's side. Mutation-checked in both directions, per the fixture family: forcing the old merge-base baseline leaves binary-resolution-takes-main green but reopens the R27-20 fixture red; dropping the kept-branch arm does the reverse. The shipped rule keeps both green. --------- Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: probe <probe@local>	2026-09-10 15:04:34 +00:00
Bob.qwencode	38513b591b	chore(release): v0.23.3 (#11583) ... * chore(release): v0.23.3 * docs(changelog): sync for v0.23.3 --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>	2026-09-10 15:03:19 +00:00
Shaojin Wen	910e0020a3	test(review): pin the fold invariant the per-line escape rests on (#11533) ... * test(review): pin the fold invariant the per-line escape rests on `escapeTagOpeners` models CommonMark INLINE structure only. That is correct exactly while every model-written channel folds its text to a line before it, and the fold lives at five sites, not one — `ingestEntryList` for the entry channels, the `\s+` normalisation for downgrade reasons, `collapseEntry` for the `Not reviewed:` disclosures, `scriptLintGate`'s own push, and compose's refusal of a line break in a ruling note's `by`. `scriptLintGate` is the proof that a sixth channel can appear and be missed: it joins `bodyCriticals` after `ingestEntryList` has run, and for a while it did not fold, so a fenced block in a side file the review agent can rewrite reached the escape as several lines and rendered a literal `&amp;lt;` to a reviewer. Nothing in the code enforced that. This adds the guard: one case drives every channel with a multi-line entry — LF, CRLF and a lone CR — and asserts the CONSEQUENCE, so a new channel that skips the fold reddens here instead of mangling a review. It also pins the two channels that REFUSE rather than fold (a quoted fence on either Critical list, and a ruling note's `by`), and the one that renders as a code span, where an escape would show the reader a literal `&lt;` instead. Removing the fold from `scriptLintGate` or from the disclosures reddens it. Two boundaries that reverse audits keep rediscovering are recorded rather than changed, both measured against cmark-gfm — the renderer GitHub runs — and an HTML parser: - A reference link's LABEL is ordinary inline text: a backtick inside `[a`b][c]`, `[x][a`b]`, `[a`b]` or a link reference definition's own label is a code-span delimiter like any other, and the tag it pairs over is code. Two audits reasoned the other way and neither could build a counterexample; the rows say so now. - `rendersAsNothing` is fold-blind and position-blind on purpose. An arm that deleted a `<details>`'s content would delete a quoted `` `<details>` `` too, which is the shape a review of that file writes — it loses a real finding to catch a readable one. And splitting the `<?` arm by position was tried and REVERTED: on a prose-weighted corpus of 30000 bodies the split alone traded 121 -> 116 bodies wrongly called empty for 1426 -> 1730 wrongly called visible, and teaching it the container prefixes then needs the container's EXTENT as well — without that, 121 -> 3594. Being right there needs the block structure the module deliberately does not model. * test(review): give the fold guard teeth, and say only what it measures Review found that the case added here, and the comments beside it, claimed more than they delivered. Four reverse-audit rounds then found the same class of defect four more times — each time in a sentence written as MEASURED that had not been. Everything below is now checked by a mutation that reddens it. The helper had three assertions and two of them could not fail. `body.split('\n')` does not split on a bare `\r`, and the filter keeps the entry's first physical line, so the line count read 1 whatever happened to the rest; and `&amp;lt;` can only appear if a model wrote those characters itself, since the escape emits `&lt;` and nothing in the source emits `&amp;`. Both are replaced by the invariant the fold actually promises: the entry's TAIL on the same line as its head, and no `\r` or `\n` left inside it. That is what a surviving break costs the reader — the per-line escape decides each half on its own and can post a tag the renderer holds live. The case now drives eight rows, not five. It gained the downgrade reasons, which need no fixture at all, and a Critical deferral's RELOCATION exit — a second producer that joins `bodyCriticals` after `ingestEntryList` has run, which a `Suggestion` row never reaches. Removing the fold at either site reddens the case; before, both were silently uncovered. The comments now name what the experiment shows: - Three rows are held by more than one fold — the deferral list, the cannot-tell leg, and the relocation exit, that last one three times over — so they cannot redden on a single-site regression, and the other five do. The earlier text said two and six, and credited the relocation exit to two holders that can both be broken with the row still folded. - The deferral LIST line never reaches `escapeTagOpeners`; it posts as `- ${mdField(entry)}`. The relocation line does reach it and comes back unchanged. Both are asserted inert as a code span, for different reasons. - `ingestEntryList`'s `\r\n?` normalisation folds nothing on its own; `collapseEntry` is what carries the entry and disclosure legs. - `rendersAsNothing` stays position-blind because `scanLines` discards `token.map` and returns a block-quote-only depth — not because a parse would cost too much. That path already runs one: `canProjectFooterMarker` opens on a `<`, which every body these arms judge carries by construction. The earlier text blamed cost and put the figure at the parser's own inline-pass note, which is off by two orders of magnitude. The corpus numbers behind the reverted `<?` split keep their conclusion and lose their false precision: the axis of the third figure is named, and both they and the parse comparison are marked directional rather than reproducible, because the absolute values swing more than twentyfold with the body's shape. No behaviour change: outside the tests, only comments move.	2026-09-10 15:02:38 +00:00
Shaojin Wen	dfafbf240e	fix(ci): widen the replayed supersede-cede timeline margin to deflake Scripts (#11581) ... Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-09-10 14:49:56 +00:00
qqqys	57e828a215	refactor(dingtalk): remove obsolete background response aggregation (#11570) ... Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com>	2026-09-10 13:59:31 +00:00
qqqys	b3ebe1e84b	feat(channels)!: remove message prefix filtering (#11571) ... BREAKING CHANGE: messagePrefix no longer filters or rewrites channel messages. Existing sender, group, mention and pairing policies still apply. Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com>	2026-09-10 13:59:17 +00:00
易良	1961e9744a	fix(acp): pass shell execution config to tool invocations (#11102) ... * fix(acp): pass the shell execution config through ACP tool dispatch The ACP tool dispatch called `invocation.execute(signal, onToolProgress)` with no third argument, so `ShellToolInvocation` fell back to `shellExecutionConfig ?? {}`. The PTY was then sized 80x30 from `shellExecutionService`'s own fallbacks — not even `Config`'s 80x24 default — and `pager`, `showColor` and `maxBufferedOutputBytes` were dropped for every ACP tool call. The TUI scheduler already passes `config.getShellExecutionConfig()` (coreToolScheduler); do the same here. This is how the process tree in #11303 was identified: the reporter's orphans were all `conhost.exe --headless --width 80 --height 30`, which is this fallback's signature and nothing else's. The leak itself is a separate defect, fixed in #11313. * fix(core): reap surviving hook process trees on Windows `terminateSurvivingHookProcessGroup` was an explicit no-op on win32, so when a parent-exit-surviving hook (MessageDisplay / StopFailure / SessionDelete) was cancelled, nothing reaped the shell the supervisor had started. Those hooks spawn a detached supervisor plus a shell child and `unref()` it, so the parent's own tree kill on the supervisor is not a reliable substitute: the supervisor may already have exited, which reparents the shell out of its tree, or its taskkill may fail. The result is the leftover `cmd.exe` processes reported in #11303. Implement the branch with `taskkill /f /t`, extracted from `terminateWindowsHookProcessTree` as a shared pid-only helper. Guarded by a `process.kill(pid, 0)` liveness probe first. Windows has no process group to signal, so an unguarded taskkill against a pid that has already exited could land on a recycled pid belonging to an unrelated application — the #6067 collateral-kill failure mode. The POSIX branch gets that safety for free by signalling the negated pid. * fix(vscode): shut the ACP CLI down gracefully instead of killing it `AcpConnection.disconnect()` did a bare `this.child.kill()`. On Windows that is `TerminateProcess`: the CLI's `process.on('exit')` handlers never run, so every PTY, ConPTY host, sleep inhibitor and tracked child process it was holding is orphaned. That is why the leaked processes in #11303 survived until the VS Code window itself was closed. Close the child's stdin instead. Ending the ndjson stream is the CLI's own shutdown path: `await connection.closed` returns, it fires SessionEnd hooks, drains the MCP pool, disposes its sessions and exits normally, so its exit cleanup actually runs. Force-kill only if that does not land within 5s — `taskkill /f /t` on Windows, since at that point the CLI is unresponsive and nothing else will reap the shells underneath it, and SIGKILL elsewhere. The timer is cleared as soon as the child exits. A late write error on a pipe whose reader is gone arrives as an 'error' event, and an unhandled one on an EventEmitter throws — in the extension host, not here — so stdin gets a one-shot error listener before it is ended. * fix(vscode): bind the ACP exit handler to the child that owns it `setupChildProcessHandlers` keyed its exit handler on `this.child`, not on the child it was installed for. A superseded child exiting after `connect()` installed its replacement therefore nulled out the *live* connection and reported it as disconnected. Latent before, because `disconnect()` force-killed the old child and it was usually gone before the replacement was assigned. The graceful shutdown in the previous commit widens that window to seconds, so bind the handlers to their own child. * fix(vscode): handle the ACP exit rejection before anything races it `processExitPromise` is created in `setupChildProcessHandlers` but its only consumer is the `Promise.race` in `initialize()`, which attaches much later. A child that exits in between — a failed startup, or a superseded child winding down after `disconnect()` — rejected it with no handler attached: an unhandled rejection in the extension host. Mark it handled at creation. The race still receives the original promise and still sees the rejection, so nothing else changes. Caught by CI on this branch: all 521 companion tests passed but vitest reported the suite as failed on the unhandled error, which is the same shape the extension host would have hit at runtime. * test(vscode): cover the Windows escalation branch, which CI never reaches `Test (windows-latest)` is skipped on PRs, so a test that branches on `process.platform` only ever exercises its POSIX half in CI — and the Windows half is the whole point here. Pin the platform on both escalation tests: one asserts SIGKILL on POSIX, the other asserts the `taskkill /f /t` tree kill on win32. `execFile` is now mocked alongside `spawn` so the Windows branch can be asserted without a real taskkill on the runner. * fix(vscode): let the CLI finish its own shutdown before force-killing it The 5s grace was shorter than the shutdown it was waiting for. On the ide_close path the CLI budgets 8s for the MCP pool drain (shutdownMcpPool(8_000), acpAgent.ts:2902) and 30s for the session drain (SESSION_DRAIN_TIMEOUT_MS, acpAgent.ts:496), so a single slow MCP server put the escalation in the middle of a wind-down that was progressing correctly, skipping the process.on('exit') cleanup this teardown exists to protect. Raise the grace to 40s to cover both stages, and pin the boundary in the escalation tests: nothing may be signalled one tick before the deadline, so a grace shorter than the CLI's own budget now reds them. No SIGTERM stage on POSIX: shutdownHandler is still attached during the wind-down (process.off('SIGTERM', ...) sits in the finally at acpAgent.ts:3119, after the awaits at 3112-3116), and `shuttingDown` only blocks a second signal, so a SIGTERM would run a second concurrent shutdown over the same pool and sessions. Also split the escalation log by branch: only win32 performs a tree kill, the POSIX branch signals the non-detached child alone, and the shared message claimed a tree kill on both. And move the two new constants above the class JSDoc, which they had been inserted between, detaching the AcpConnection doc comment from the class. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmts86p0gqr * fix(core): fall back to a direct kill when the surviving hook's taskkill fails terminateSurvivingHookProcessGroup discarded the boolean taskkillProcessTree was extracted to return, so a taskkill that errored or exceeded WINDOWS_TASKKILL_TIMEOUT_MS left the hook's cmd.exe tree running with nothing else able to reap it and only a debugLogger.warn as a trace. The sibling caller in terminateWindowsHookProcessTree already honours it. The fallback stays behind the isProcessAlive probe, so the #6067 recycled-pid guard is unchanged. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmts86p0gqr * fix: close Windows process cleanup review gaps * fix: reap CLI process group and gate superseded ACP callbacks Address qwen-code-ci-bot review findings on #11102: - Spawn the ACP child detached on POSIX so disconnect() can signal its process group (reaping the PTYs, ConPTY hosts and MCP children the CLI tracks) with a root-only fallback. Previously SIGKILL reached the CLI root process alone and orphaned the tree. - Gate the five SDK inbound callbacks on `this.child !== ownChild` so a superseded connection stops dispatching into callbacks that read `this.*` at call time. - Re-probe liveness before the Windows pid-based SIGKILL fallback in terminateSurvivingHookProcessGroup, so a pid taskkill already reported dead is never signalled directly (collateral-kill risk). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtspbz90ri * fix: guard retired ACP dispatch and pin teardown escalation tests Two gaps remained after d67635b77: - sendPrompt's onEndTurn tail ran unguarded after a stale prompt resolved post-disconnect, clearing the replacement session's streaming state. Bail out when the connection was retired (this.sdkConnection !== conn). - The five SDK inbound callback guards compared this.child !== ownChild, which is wrong across the shutdown grace window (this.child is nulled before the grace timer and initialize() re-runs on the still-current child). Compare against the wired connection instead. Pin the positive halves the first fix left unpinned: the surviving hook pid is still reaped after the supervisor exits, a live child exit clears the connection and fires onDisconnected, and the force-kill escalation still fires when stdin could not be ended. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtspbz90ri * fix: cover runExitCleanup in the ACP shutdown grace The ide_close path budgets 8s for the MCP pool drain and 30s for the session drain, then runs runExitCleanup() (up to OVERALL_CLEANUP_TIMEOUT_MS = 5s) in the finally wrapping runAcpAgent. The 40s grace fired 3s before that 43s bounded wind-down finished, cutting the CLI off mid-cleanup and dropping the recording flush and MCP disconnects. Bump to 45s to cover all three bounded stages. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtspbz90ri * fix(vscode-ide-companion): guard stale session writes after supersede newSession and loadSession wrote this.sessionId after awaiting the connection they captured, with no supersede guard. disconnect() now leaves the retired CLI alive for up to SHUTDOWN_GRACE_MS, so a late session/new or session/load response from that retired CLI can land after a replacement connection is installed and stamp the dead session's id back onto the field disconnect() just nulled. Gate both post-await writes on `this.sdkConnection === conn` (the same guard sendPrompt already carries), and correct the escalation comment and its test comment to name exactly what the POSIX group kill does and does not reach. Add tests pinning the detached-spawn premise and the stale-session guard. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtsxwmdgrw * fix(vscode-ide-companion): pin re-connect supersede gate and align spawn comment The spawn-option comment claimed the POSIX group signal reaps PTYs, ConPTY hosts and MCP children. The escalation comment (corrected by faab984f29) says the group kill only reaches the CLI root and its non-detached MCP stdio children — it does NOT reach descendants that call setsid() (detached hook supervisors/monitors, node-pty sessions). Align the spawn comment with that. Every supersede test drove the gate through disconnect(), which nulls this.child and this.sdkConnection together, so the captured-identity predicate and a weaker !this.child always agree. Add a re-connect test that installs a replacement child while the old connection is still winding down, pinning the stronger predicate. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmttcwqrask * fix(vscode-ide-companion): pin re-connect supersede gates for outbound session writes The outbound guards (newSession/loadSession/sendPrompt) key on `this.sdkConnection !== conn` (or `=== conn`) to detect a superseded connection. Every test drove the gate through disconnect(), which nulls this.child and this.sdkConnection together, so a weakened `!this.child` still bails and the predicate is never discriminated. Add re-connect cases that install a replacement child while the retired connection's promises are still in flight: a guard weakened to `!this.child` would then stamp the retired session id (newSession/loadSession) or fire onEndTurn (sendPrompt), turning the test red. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmttf1wj0so * fix(vscode-ide-companion): fail superseded newSession/loadSession instead of returning the retired CLI's payload The supersede guards added at newSession and loadSession correctly skipped the stale write to this.sessionId, but still returned the retired CLI's response. QwenAgentManager applies that payload unconditionally: applySessionStateFromResult (qwenAgentManager.ts:1283) and restoreBaselineSessionStateAfterLoad (:1066) write the dead CLI's model and mode state into the live webview's baselines, and createNewSession hands the same promise to every concurrent caller through sessionCreateInFlight (:1226-1234), so they all adopt it. Both now throw RequestError.internalError({ details: 'connection superseded' }), matching the convention the inbound callback guards in this same file already use. Callers already have error paths: createNewSession rethrows non-auth errors (isAuthenticationRequiredError matches AUTH_REQUIRED -32000, not INTERNAL_ERROR -32603) and its finally clears sessionCreateInFlight, so concurrent callers get the rejection instead of stale state; loadSessionViaAcp rethrows as well. loadSession's gate moves outside its catch, so a supersede is no longer logged as a request failure, and ahead of the success log, so a discarded load prints no unqualified success line. sendPrompt's guard is deliberately left returning: its only caller (qwenAgentManager.sendMessage:397) is Promise<void> and discards the response, so no payload is certified to anyone, and the guard's real job -- skipping onEndTurn so the replacement session's streaming state survives -- is already correct. Tests: both supersede cases now assert the rejection (code + data.details) rather than awaiting a resolution. Mutation-verified: reverting either guard to `return response` reds exactly those two cases ("promise resolved { sessionId: 'stale-from-retired-cli' } instead of rejecting"), 31 passed. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtu0hicwtk * fix(vscode-ide-companion): close the superseded session on session replace The agent keeps every session alive until told otherwise, and a retained session still fires autonomous model turns when its background tasks complete — each turn can spawn shells, which is what grew conhost.exe without bound in #11303 while the window stayed open. session/new and session/load now close the superseded session once the replacement is confirmed. Fire-and-forget: a refused or unsupported close (older CLI) must not block the user's new session, and a later session/load of the closed id simply re-reads the flushed transcript. * fix(acp): narrow shell execution follow-up Remove the risky hook-process and superseded-session cleanup changes, leaving only the ACP shell execution configuration passthrough. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-09-10 12:57:57 +00:00
Shaojin Wen	ac1edef974	fix(review): keep host-trusted state out of the container's writable surface (#9983) ... * fix(review): keep host-trusted state out of the container's writable surface Two instances of one root cause, both reachable on `main` today and neither opened by the sandbox — before it, the reviewed repository's commands ran as the host user with no filesystem restriction at all, so containment narrowed this reach rather than creating it. What it did not do is close it, and a `required` that claims containment should not leave host-trusted state in the directory it hands the container read-write. **Leases.** `cleanupReviewWorktreeLeases` matches a lease by session ids alone and then force-removes whatever worktree and deletes whatever branch it names. Those files lived in the review temp dir — the bind mount. Reviewed code that edits one makes another session's cleanup destroy the wrong tree; one it plants with a foreign session id is swept by nobody and wedges that PR on that machine until someone deletes the file by hand. They now live one directory over, in `.qwen/review-leases`, which nothing mounts. A lease left behind by an older build is deleted when the same target is leased again. **Gitfiles.** These cannot move — git requires `<tree>/.git` — so the check moves instead. The existing identity gates cannot see the attack and never could: the writer chooses both the gitfile's target and that target's backpointer, so the round-trip agrees; `--show-toplevel` still names the tree; no symlink is involved; and asking git for the common dir resolves THROUGH the rewritten gitfile, so that answer comes from the same hand. Location is the question that has an honest answer — a real admin entry lives under `<repo>/.git/worktrees/`, outside the mount, and a planted one has to be inside it. `restoreProbeTreeTracked` now refuses an entry that resolves inside the review temp dir. Reproduced end to end before fixing, and the reproduction is the reason the gate is where it is: a rewritten gitfile pointing at a planted admin entry whose own `commondir` names a planted repository, both under the mount, passes all three existing gates — and `git checkout --force` then runs that repository's `filter.evil.smudge` ON THE HOST. With the new gate the same shape is refused while the entry it replaced still passes. Three mutants, three reds: a gate that always admits, one that admits when the entry cannot be resolved, and one that refuses when there is no mount at all — the last would refuse every ordinary repository. * fix(review): close the shapes the first cut of this gate left open Six blockers from round 1, all verified before fixing. **The predicate failed open at both boundaries of its own question.** An entry whose realpath IS the mount root produced an empty relative path and was read as outside — while the root is exactly as writable as anything under it — and a child legitimately named `..evil-git` produced a relative path starting with the characters an escape would. Both are places a planted entry can sit. The hand-rolled arithmetic is gone; `isSubpath` already answers this correctly and is already tested. **Every gate sat behind `.git` being a file.** Reviewed code that replaces the gitfile with a `.git` DIRECTORY of its own — `rm .git && git init .` inside the mount — skipped the lot, and the restore then ran against the repository it had just made. That shape is now refused before the branch, and only under a mount: outside one, a `.git` directory is what every ordinary checkout has. **The gate was on the second host-side write, not the first.** The probe tree's own creation — `git worktree add` — resolves the repository through the REVIEW worktree's gitfile, a second rewritable pointer in the same mount, written after the build phase has already run the PR's code in the container. `worktree add` checks files out, so it executes whatever filter that pointer leads to, before anything inside the restore could fire. Both call sites now ask the same question through one helper rather than two inline copies. **The legacy-lease removal was a wedge.** `force` only swallows ENOENT, so a DIRECTORY at the old lease path threw EISDIR out of acquisition — and `mkdir .qwen/tmp/qwen-review-lease-pr-42.json` is one command inside the mount, after which every review of that PR on that machine failed, with nothing able to remove it: the rollback rethrows, the sweep skips the lease shape, and `rm -f` cannot remove a directory. **The move stranded the only automated orphan sweeper.** The review job's `if: always()` heal step swept the old path only, so a cancelled or timed-out run left a dead lease at the new one, on a runner whose workspace persists. It now sweeps both, recursively. Tests follow the same rule they were missing: the gate's production call sites are driven, not just its predicate. Every previous fixture put its tree under a bare `tmpdir()`, where `mountRootFor` answers null and the gate short-circuits before its logic runs — which is how deleting the wiring shipped green. The new fixtures sit under `.qwen/tmp` and plant a COHERENT admin entry, because an incoherent one fails earlier for a reason that has nothing to do with the gate. Seven mutants, seven reds — including passing the probe tree where the review worktree belongs. * fix(review): gate the third write, and stop breaking the lane that pins the sweep **The workflow edit broke its own pinning test, and this PR's required check with it.** `scripts/tests/review-worktree-cleanup-workflow.test.js` asserts the sweep line literally, derived from `paths.ts` so a rename there fails the build rather than silently no-op-ing the sweep on the runners. Adding `-r` broke the substring. The test now pins BOTH locations — the new one where this job's own runs leave leases, the old one where a persisted workspace can still hold one — which is what the test existed to enforce and what the move should have updated in the same breath. It went unnoticed because the commit was verified with vitest alone; `npm run test:scripts` is a separate lane of the same required job. **The revert phase was a third ungated write, reached exactly when the others refuse.** `git checkout base -- …` runs against the same mounted probe tree, and its only guard was a symlink-only lstat walk that a rewritten gitfile passes untouched. Worse, a restore refusal becomes `inconclusive` without throwing and the mutation phase's catch continues on purpose "so the revert probe below still runs" — so guarding the first two writes and not this one left the route open precisely where the other two closed it. **A claim in the gate's own rationale was false.** The comment called `worktree add` "the FIRST host-side git write of this phase"; `discardWorktree` above already runs `worktree remove --force` and `worktree unlock` with the same cwd. They materialise nothing, so no filter runs — but that distinction is the entire reason the gate can sit below them, and the looser sentence invites the next maintainer to add a checkout above it. Tests follow the same rule as the code. The ADMIT path had no coverage, so a mutation mangling the parsed gitdir survived: every refusal fixture would still refuse, because a mangled target fails to resolve and the location check fails closed — while in production the same mangling refuses every healthy tree. It is asserted now, relative spelling included, since git writes that pointer relative when it can. And the revert gate is driven through the phase itself, by a fake runner that rewrites `.git` from inside the probe tree — which is who does it in production. Nine mutants, nine reds. * test(review): gate the new cases off the platform where containment cannot exist The three tests added for this gate are deterministically red on Windows, and the reason is this PR's own sibling: `mountRootFor` refuses every absolute Windows path, because a drive letter is a colon and the `-v` grammar has one separator. Containment is unavailable there by design, so the gates never speak, `out.json` never carries the refusal, and the assertions fail for a reason that has nothing to do with what they test. The Windows lane runs only under `merge_group` and reports as skipped on the PR page, so green here was no evidence — the first red would have landed in the queue and stalled the batch. Gated with the same named predicate the sibling suite uses, so the reason is stated once rather than repeated three times. Two more from the same review. The fake runner embedded the planted path in a JS string literal by interpolation, which a Windows path's backslashes self-mangle; it is generated through `JSON.stringify` now. And both new tests had pasted the ~22-line fixture that `scaffoldModifiedPr` already provides, leaving three copies to evolve in lock-step — the helper takes the worktree location as a parameter instead, since the location is exactly what differs and exactly why the gates fire. The three gates were re-mutated after the refactor, one at a time: three mutants, three reds. * test(review): assert the property, not the sentence that explains it Both gate tests asserted a refusal MESSAGE, and `worktreeCreateFailureDetail` serializes that message identically whether the gate throws before the write or after it. So the mutant that matters — moving `worktree add` above the gate, which is exactly what the comment beside it warns a refactor might do — checked files out through the planted pointer, ran its filters on the host, and left both tests green. Creation: the probe tree not existing is what says the spawn never ran. Revert: the planted repository now carries a real `filter.evil.smudge`, and the assertion is that its canary was never written. That is the property itself rather than a proxy for it — with the gate removed the filter EXECUTES on the host inside the suite, which is what the test now catches. The fixture had to become coherent for this to mean anything: an empty planted directory fails the checkout for reasons unrelated to the gate, so it copies the real admin entry and points `commondir` at a planted common dir, the shape the reproduction in the PR description uses. * fix(review): ask git where the repository is, and gate the entrances that were left **Stop re-implementing git's resolver.** The gate parsed `gitdir:` and resolved it in Node while the write that followed resolved the same file in git, and the two disagree in ways that are individually small and collectively unbounded: JS `trim()` strips U+00A0 where git's `read_gitfile` trims only C-locale space, so `gitdir: <NBSP><real entry>` resolved outside the mount here — admitted — and to a planted entry inside it there; and `resolve()` is lexical where a spawned git resolves relative targets against the tree's physical path after chdir. Every divergence admits a pointer whose real referent only git sees, and enumerating them has no last corner. So the resolver that decides is now the resolver that acts: `rev-parse --absolute-git-dir`, run in the tree, with hooks and fsmonitor inert. What stays ours is the only judgment that was ever ours — WHERE the answer lives. The fixtures had to become real repositories for this to mean anything, which is an improvement on its own. **Three more entrances, all probed to host execution.** Round 2 recorded that per-call-site gating does not close the class, and I argued a shared layer would be noise. That was wrong, and the review paid for the answer: base-tree's rebuild had no defence at all; scratch-tree's screen matches `filter.*.smudge|clean` and not `filter.*.process`, which also executes; and `--resume` runs an index-refreshing `git status` through the same pointer, so a planted `core.fsmonitor` fires inside the very command that collects the ruling's evidence — the attack does not need the resume to succeed. All three now ask the same question, and `status` carries an inert fsmonitor besides. **Absence is the caller's question, not this one's.** The first cut refused a tree that does not exist, which is the state `--resume` legitimately asks about — 26 tests, measured. It returns null there now. * fix(review): the entrances round 6 found, and two limits stated rather than papered over Case, first: the verdict is a string comparison, and Node's JS `realpathSync` hands back the spelling it was asked for whenever the lookup succeeded — so on a case-insensitive filesystem (default macOS APFS, a host this pipeline supports) a planted entry spelled with different case resolves INSIDE the mount and compares OUTSIDE it. `realpathSync.native` asks the operating system for the stored name, so both sides arrive spelled as the filesystem holds them. Two more entrances. scratch-tree's REUSE path reaches a `checkout --force` of its own through the scratch tree's pointer, and fetch-pr's FRESH path checks out through the review worktree's. The reuse one is not a refusal, though: an unusable leftover is exactly what the rebuild exists for, and refusing there turned "rebuild over it" into "fail the command" — three tests said so. And `--resume` refused to CRASH where this file's own contract, the option's describe and the docs all promise refuse-to-FRESH. Throwing propagated out of `runFetchPr`, so `cleanStale` — the thing that removes the planted tree — was never reached. It returns a refusal reason now and the fresh path sweeps. The two new refusal tests were red on Windows, for the third time in this PR and for the same reason: `mountRootFor` refuses every absolute Windows path, so containment is unavailable there by design and the gate never speaks. Both gated off win32 like their siblings. Two limits I could not close, stated where they are rather than left looking proven. The `--resume` gate's wiring is pinned by no test: `fetch-pr.test.ts` mocks `node:fs` without `existsSync`, so the tree never exists there and the gate is inert — deleting it ships green. And the reuse-path gate is defence in depth rather than a demonstrated check: with both trees pointed at one planted common dir and a `filter.*.process` the existing screen misses, removing the gate still executes nothing, because the reset declines that tree for its own reasons first. * fix(review): stop editing git's answer, and gate the pointer that is actually used Two blockers, both inside the previous round's own fixes. `.trim()` on `rev-parse`'s stdout re-opened the exact divergence the paragraph above it closes. It strips U+00A0, U+FEFF and the U+2000 block, so an admin entry whose directory NAME ends in one is resolved by git with the character and judged here without it — and a twin of that name minus the character, symlinked outside the mount, is where the judgment then lands. Only the terminator `rev-parse` adds may be removed. And the fresh-path gate checked the wrong pointer. `git()` sets no cwd, so `worktree add` resolves the repository from `process.cwd()` — while the gate asked about `wt`, which `cleanStale` has just removed, and a tree that does not exist has no pointer to distrust. It was a no-op on every reachable path. It asks about the launch directory now, which is the one the nested geometry `mountRootFor` documents lets an outer review's containerized phase rewrite. * fix(review): let the launch directory resolve the way git does Last round's gate asked `untrustedGitfile` about `process.cwd()`, and that function demands `<tree>/.git` — right for the trees this pipeline builds, wrong for the directory a command was launched from. Any subdirectory of a checkout has no `.git` of its own; git walks up. So in the nested geometry the pipeline runs its own reviews in — cwd at `<review tree>/packages/cli` — the gate answered "has no .git to resolve" and fetch-pr aborted before the `worktree add` that would have succeeded. 77 tests in one suite, at the reviewer's HEAD. My checkout is not under `.qwen/tmp`, so `mountRootFor` returned null here and the gate never fired: the suite stayed green while the dogfood lane broke. That blind spot is now a test — a subdirectory of a mounted checkout, asserted to resolve. The launch directory gets its own question: ask git, from there, what repository `worktree add` will find, and judge only where that answer lives. No shape check, because the shape belongs to a tree and this is not one. * fix(review): three failures of my own, and the block gate that stops the fourth `realpathSync.native` is not always there. A suite that mocks `node:fs.realpathSync` as a bare `vi.fn` gives it no `.native`, and reaching through it threw a TypeError straight into the fail-closed catch — refusing every worktree creation in any checkout that happens to sit under `.qwen/tmp`, which is the geometry this pipeline reviews itself in. Falls back to the plain call, and a test asks that shape directly, because the suite that carries the mock never reaches this gate from an unmounted checkout. The real-git fixtures ran without `isolateHostGitConfig`, which every other real-git block in the same file installs. On a host with `commit.gpgsign=true` and no usable key the fixture commit fails and the whole block goes red for reasons unrelated to the gate — reproduced with a poisoned HOME. And one more case was left ungated for the Windows lane. Rather than gate it and wait for the fifth, the whole block is gated: every case in it builds a layout under `.qwen/tmp`, and on Windows neither the question nor the fixture exists. Worth recording why these three reached the branch at all: my checkout is not under `.qwen/tmp`, so `mountRootFor` answers null here and every one of these gates is inert on my machine. Green locally proves nothing about the geometry they are for. * fix(review): gate the reads and the launch directories, not only the writes The location gate this PR added was asked at the writes and nowhere else, which is how the same class kept resurfacing: a class closed one call site at a time re-opens at the next call site somebody adds. Four entrances stood open behind it, and two more of the same shape were a census away. `worktreeResidue` measured BEFORE the gates its two call sites carry, and a measurement is not a safe read: `status` refreshes the index, an index refresh runs the repository's clean filter, and every identity check the probe carries is honest about WHICH repository answers and blind to where that repository lives. A coherent plant writes both halves of the round trip, HEAD comes from the copied admin entry so the sha pin matches, and no symlink is involved. The gate now runs first, and a refusal is unmeasured-with-reason — the channel every caller already reads as "not clean". `fetch-pr` asked at step 4, after `cleanStale`'s force-remove and after the PR fetch, both of which discover their repository from `process.cwd()`. In the nested geometry `mountRootFor` documents, that launch directory is inside the outer review's read-write mount, and a fetch through a rewritten pointer loads the planted repository's transport config. The gate moves to the top of the command, before the lease is even read; the step-4 ask stays as the narrower window around the checkout `worktree add` performs. `captureLocalDiff` gets the same treatment for the local lane, before its first `rev-parse`. `comment-status` and `repo-context` read through the pointer without asking at all. No execution witness is claimed for `log`/`ls-tree`/`show` — what is at stake there is the answer: `changedSinceComment: false` sourced from a planted repository is this pipeline certifying that the code behind a blocker thread did not move, and `repo-context` exists precisely to take identity files from the merge base rather than from the PR head. And the containment predicate judged against the innermost `.qwen/tmp` only. The MOUNT has to stay innermost — the first occurrence would pull `<repo>/.git` and every sibling checkout into the container — but "which directory may a container have been given read-write" and "where may reviewed code already have written" are different questions, and in the nested geometry they have different answers. The judgement now takes the outermost layer. It costs the honest layouts nothing: `git worktree add` run from inside a linked worktree still writes its admin entry under the main repository's `.git/worktrees/`, outside every layer. `mountRootFor` moves from `sandboxed-exec` into `worktree` (re-exported from its old home) so the gates can default to it instead of taking it as an argument every new call site has to remember to pass. Tests: `host-execution.canary.test.ts` asks the property once for every route, with a live plant as its oracle — a rewritten gitfile naming an admin entry whose commondir names a planted repository carrying a filter git executes, plus a canary the filter writes on the host. Its first case asserts the canary IS written when the same command runs ungated, so a fixture that stops being an attack fails there rather than certifying the gates that walked around it. Six mutations, six distinct reds. The committed fixtures that wrote a `gitdir: `-prefixed backpointer now write the bare path git actually writes; the prefixed form failed the round-trip check first and masked every route behind it. * fix(review): gate the base tree's reuse fast path too The census that found `repo-context` and `captureLocalDiff` found one more, and this one returns before the gate rather than running beside it: the base-tree reuse branch answers "already built at <sha>" on two facts that both live inside the mount — a marker file in the base tree, and `rev-parse HEAD` resolved through that tree's own `.git`. A planted repository answers the right sha for a working tree holding anything at all, and the A/B's BASE side is then the reviewed code's own; every "the base behaves differently" verdict a shard reports would be its author's. Not a refusal: an unusable leftover is what the rebuild exists for. Falling through discards the tree — removing the plant with it — and creates a fresh one through the review worktree's pointer, which the gate before `worktree add` already checks. Same shape as `scratch-tree`'s reuse path, for the same reason. The canary suite gains the arm: with the plant in place the report must not say "reusing it". Removing the gate reds it, and the mutant's own note names the planted tree as an already-built base. * fix(review): ask the launch-directory question where all of it goes through Hoisting the gate inside `fetch-pr` fixed one command. The census that followed found the same shape in `load-rules` (this review's own rules, read with `show <base>:<path>`), `submit` (where a submission goes), `match-remote` (which remote gets fetched), `run`, `revert-hunk` and `cleanup` — none of which knows it is asking this question, and none of which should have to. They all share one thing: every wrapper in `lib/git` runs git with no `cwd`, so each discovers its repository from `process.cwd()`. That is the choke point, so the question moves there — four functions in one file instead of a gate per command, which is the pattern that kept re-opening this class. The throwing wrappers throw. `gitProbe` answers its own documented "could not be run at all" value instead, because that is a probe's contract and every `gitOpt` caller already handles it; the command does not run either way, which is the part that matters. `releaseWorktree` is the case that shows why this is the right split: through a poisoned launch directory its `worktree remove` is refused, and the `rmSync` behind it still clears the path — the remedy keeps working without git acting through the plant. Memoized per directory; outside a review temp dir it costs no syscall at all (`mountRootFor` returns null on a string scan). The cached admit is the same TOCTOU residual the per-tree gates already document and do not close. `captureLocalDiff`'s own gate goes: it now contributes nothing but a nicer message, and a duplicate that has to be kept in step with the real one is a liability. `fetch-pr` keeps its own, because that command changes state that is not a git call — the lease read and write — before it makes one, and the ordering test pins exactly that. Seven mutations, seven distinct red sets, baseline green. * refactor(review): let the gates default to the mount root everywhere Every production call site now asks `untrustedGitfile(tree)` and `untrustedRepositoryFrom(cwd)` rather than threading `mountRootFor` through by hand. That is the point of moving it into this module: the argument stays injectable for the fixtures that drive the judgement with their own root, but a new call site cannot get it wrong by forgetting to pass one — and two spellings of the same check in one file is the kind of thing a reader has to stop and reconcile. The test-only reset seam on the launch-directory verdict goes with it: the verdict is keyed on the directory, and every fixture stands its plant up in a fresh temp dir, so a cached answer can never be served for a different one. * fix(review): gate revert-hunk's tree reads, and reflow a comment The last tree-scoped command that asked nothing. `git apply` writes into the tree without running a filter, so no execution is claimed here — what the gate refuses is deciding a probe's outcome on answers that come out of the plant. `--show-toplevel` still prints this tree when the gitfile has been rewritten (that is the shape the location gate exists for), and `check-attr` and `ls-files` then report the planted repository's EOL regime and index: the two facts this function refuses a conversion on. A scratch tree is inside the directory the sandbox hands the reviewed code read-write, so that writer is the code under review. A harness fact like every other refusal there — nothing is claimed about the hunk, and the tree is untouched. That leaves one deliberate exception, stated rather than left to be found: `cleanup` still runs `worktree prune` and `branch -D` from the launch directory ungated, because it is the remedy — refusing there would leave a planted tree with nothing able to sweep it, and its `rmSync` clears the path whether or not git can act through the pointer. * fix(review): let a refusal say it refused, and re-ask where git resolves Six review Criticals with one theme: a gate that answers correctly, then reports through a channel its callers read as an answer. `gitProbe` returned the launch-dir refusal as `{out: null, status: null}` and every consumer used the status-blind `gitOpt`, so `releaseWorktree` deleted the directory with `rmSync`, reported `freed: true`, and left behind the registration and branch that wedge the next `worktree add`; `load-rules` wrote an empty rules file into every agent brief while printing "No review rules found". `status: null` already documented "the command could not be run at all" — git answers a path absent at a ref with 128 — so the fix keys on that and carries the refusal text beside it, rather than adding a third state. `releaseWorktree` now reports the release it could not make, and `load-rules` separates a source it could not read from one that is absent. The launch-dir verdict was memoized for the whole process, so a pointer rewritten after the run's first git call was served that first clean answer forever — and in the nested geometry the outer review's containerized build holds the directory read-write for the length of a command, which is a rewrite between calls, not between a check and its use. Only the negative is cached now, and only outside a review temp dir where it costs no syscall. fetch-pr's step-4 refusal was thrown inside the try whose catch rolled the fetched ref back with an ungated `branch -D`, so the gate's own answer executed the plant's reference-transaction hooks on the host and the run died as `Failed to create worktree at …`, indistinguishable from an infrastructure failure. The ask is hoisted out, and the five remaining spawns in fetch-pr and cleanup that resolved their repository from `process.cwd()` go through lib/git's gated wrappers. Cleanup still sweeps: the `rmSync` that clears the path is not a git call, and a measured ungated prune was clearing the PLANT's registrations, not the real one. A lease moved out of the mounted directory with no fallback read, so an older build's live lock was invisible to the gate and the next acquisition deleted it before sweeping its worktree mid-run — #9205, the incident the lease exists to prevent. The gate reads both locations for one release, and acquisition refuses a foreign legacy lease instead of removing it. base-tree's reuse fast path policied the tree's pointer but not its contents, and `rev-parse HEAD` does not move when working files change, so the reviewed build could overwrite the base checkout and the A/B compared the PR against a copy of itself — a regression it breaks then reads as pre-existing on both sides. Tracked dirt falls through to the rebuild; untracked build output still reuses, or the concurrent-shard clobber the fast path prevents comes back. Measured: `npm run build` here modifies no tracked file. revert-hunk decided where `--tree`'s repository resolves but never asked whether `--tree` IS that tree, so a symlink at the scratch path aimed `git apply -R` at another legitimate tree while the report certified the scratch path. The leaf is refused, and re-read immediately before the write. The bounded ancestor walk `resetScratchTree` also does is not added here: this command has no common dir to bound at, and an unbounded walk refuses every tree under a linked `/tmp`. Also one definition of the attack fixture instead of ten copies across five suites, which repairs the one copy whose admin-entry backpointer carried a `gitdir: ` prefix git never writes there. * fix(review): ask a pointer where it resolves to, and stamp the run that built a tree Four review Criticals with one theme: a gate that asked half the question, and a marker that recorded what it vouched for but not who vouched. A gitfile rewritten to `gitdir: <repo>/.git` names the repository's OWN common dir, which resolves OUTSIDE the mount, so every gate that asked only where the answer lives admitted it and then acted on the main repository through it — measured, `status` refreshed the main index and `rev-parse HEAD` answered the main head, so the reads a review treats as fact came from a tree nobody verified. Both questions are now asked in one place, so the tree gate, the launch-directory gate and the residue probe close together instead of one entrance at a time, and the two `rev-parse` calls share one sanitized spawn instead of duplicating its boilerplate. scratch-tree's reuse route asked about the scratch pointer only, never about the review worktree's — the one `headSha` and every comparison inside the reset resolve through — so an unpinned run answered `available: true, reused: true` with the main head and reset the verifier's tree to the user's own commit. Suspicion there falls through to the rebuild, whose gate refuses, rather than refusing on the spot. `mountRootFor` matched no temp dir for a process standing AT `.qwen/tmp`, because the marker ends in a separator: the launch-directory question went unpoliced there and `lib/git` memoized that negative as trusted. The base-tree reuse marker recorded a sha but not a run, and `cleanStale` never releases `-base`, so an earlier round's tree was certified as this run's A/B base after a whole containerized phase could drop untracked executables into it — `dist/cli.js` is what a host-side A/B runs, and `--untracked-files=no` cannot see it. Refusing any untracked file at all would disable every legitimate reuse and bring back the concurrent-shard clobber the fast path exists to prevent, so the marker carries the run epoch the deadline stamps and the session ledger already key on: same-run reuse survives, an earlier run's tree rebuilds. And the lease move left `isReviewPlumbing` naming its directories by hand, so a checkout holding a lease captured that churned lease JSON as the user's untracked change and an interleaved local round could never again report "no changes". The pattern is built from the `paths.ts` constants now, which is what keeps the next move from re-opening it. Two existing witnesses were repaired rather than left green for the wrong reason: the canary's base-tree fixture and the different-sha marker case both hand-wrote a one-line marker, which the epoch fence now turns away before the gate each exists to exercise. Both stamp the epoch, and a probe removing the reuse pointer gate reds the canary again. * fix(review): fail closed where a gate could not be run, and stop editing git's answer Seven review Criticals, all in code this pull request added: four ways a gate reported a success it had not earned, one that threw where its contract says it degrades, one lane-breaking test, and one regex that edited the very answer it exists to pass through unedited. `rev-parse` answered null for three different things — git ran and said this is no repository, git timed out, and git died on some other fatal — and two of the three gates read all three as "no objection". The launch-directory gate budgets 30s against the protected commands' 120s, and the residue probe's spawns carry no budget at all, so a config sized to parse between the two left the gate silent while the command it gates resolved through the pointer nobody had judged: measured, `status` refreshed the planted index and ran its clean filter on the host. The null is classified once, at the spawn that produces it, and both gates refuse the class they cannot judge. A genuine not-a-repository still passes through — that is the caller's own error path to own, and refusing it would answer a question nobody asked. Pinning `LC_ALL` is what makes that classification depend on git's behaviour rather than on the runner's locale. The same strip that removed git's record terminator also removed a `\r` that was the last byte of the path itself, which is precisely what the docstring above it forbids: a plant named `evil\r` was judged at its twin `evil`, symlinked outside the mount, and admitted while every gated command resolved through the plant. `gitProbe`'s launch-dir pre-check read `process.cwd()` outside its try, so a review whose worktree was swept out from under it threw ENOENT through `releaseWorktree`'s documented never-throws contract and aborted the sweep before the branch delete and the lease release — leaving the stale lease that refuses every later cleanup of that target. `cleanup` collapsed a refusal into each of its two benign answers: `refExists` said "no such branch", so the branch leg was skipped silently and the lease released over a surviving branch, and `pruneWorktrees` said "nothing to prune", so the symlink arm announced `Removed … link` over a registration that stayed. Both report the refusal and hold the lease now. A genuine prune failure stays swallowed, because it must not mask the error that got us here. And the rewritten `toContain('common dir')` assertion depended on the mount-relative gate answering first, which it never does on Windows — the same lane this pull request has broken three times before. Split by the file's own convention rather than branching one expectation on `process.platform`, so each lane asserts a single unconditional sentence. Every guard above has a witness that reds without it, including the admit arms: a genuine not-a-repository keeps passing, and the residue probe's `status` is shown never to run at all. * fix(review): fail cleanup's branch probe on any non-answer, survive a deleted cwd With --verify --quiet only exit 1 is genuine absence: a null status (spawn failure, timeout kill) or a 128 fatal used to read as "no such branch", so the delete was skipped, the lease released, and "Nothing to clean" printed over a surviving branch (R19-1). Name the non-answer and hold the lease instead. Also capture the launch directory at runCleanup's entry and hand it to the ancestor walks as an explicit stopAt: their default reads process.cwd() outside any try, so a deleted cwd threw uv_cwd out of the best-effort sweep before any degradation could run (the cleanup half of R19-4). The holder-skip note now names the path the lease was found at, which during the rollout can be the pre-move path (R20-1). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): walk --tree's ancestors for a planted symlink, not only its leaf The redirect gate checked the leaf of --tree alone, at entry and again before the mutating git apply. A symlink swapped in at an ancestor between the review temp root and the leaf — after the create/sweep checks ran — was invisible to every check in the chain, and git apply -R then reverse-applied into whichever tree the link names while the report certified the scratch path (R24-1). Walk the ancestors bounded at the outermost review temp root, at both checks; a --tree outside any review temp root keeps the leaf check so a linked /var refuses nothing. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): mirror new leases at the legacy path, bound the legacy read The dual-location lease read was one-directional: a pre-move build stays blind to leases written at the new path, so for the rollout window an old fetch-pr passes its gate over a live lease and force-removes the new session's worktree mid-run (R16-1). Mirror the lease at the legacy path after the new-path wx write wins; an honored foreign lease surfacing before the mirror backs the acquisition out instead of clobbering it. The legacy read also re-granted gate authority to the one directory reviewed code can write: a planted legacy lease naming a foreign session was a permanent denial of service (R24-2). A legacy lease is now honored only while its mtime predates LEGACY_LEASE_CUTOFF_MS (the release date of the first build carrying the move — MUST be set before shipping). Release and diagnostics see both locations (R20-1): clear removes the legacy path too (recursive, so a directory at the lease name cannot EISDIR-wedge), and the held-lease messages name the path the lease was actually found at via the new readReviewWorktreeLeaseAt. The finalizer sweeps the legacy directory only for leases content-identical to the new-path twin — the mirror is readable from inside the mount, so copied credentials prove nothing, and a plant must never drive a host-side removal. readLease lstats before opening, so a planted FIFO hangs no gate read (R24-4). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): fence base-tree reuse with a per-run secret kept outside the mount The reuse fence keyed on inputs the sandbox's read-write mount exposes: the run epoch was the plan's mtime and the plan lives inside the mount, so reviewed code could forge the stamp and have the pipeline certify an attacker-shaped tree as the A/B base (R15-4). The stamp's second line is now a random per-run nonce persisted beside REVIEW_LEASE_DIR — created atomically by the run's first host process, adopted by its shards, unreadable and unwritable from inside the mount. The failed marker carries the same fence: a planted sha-only marker no longer settles the A/B lane into "already failed" with no build attempted (R22-1). At marker write the untracked path set the build legitimately left is recorded host-side, and reuse requires the current set to be a subset — an untracked plant dropped after the stamp (dist/cli.js, what a host-side A/B executes as the BASE side) falls through to the rebuild that sweeps it (R25-1). And when a tree THIS run built fails a state check — legitimate codegen dirt, a concurrent shard's snapshot update — the command declines busy instead of discarding the live tree another shard may be mid-A/B in; discard-and-rebuild is kept for cross-run leftovers, forges, and plants (R26-1). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): fail the trust gates closed when the mount root is refused mountRootFor's null was overloaded: "outside any temp dir" and "inside one, but refused" were the same value, and every trust gate read a refusal as "nothing to police" — measured end-to-end with a symlinked .qwen/tmp, where a gated worktree add ran through the plant and its smudge filter wrote its canary on the host (R26-3). The gates now ask the containment question lexically (insideReviewTmpLexically, no filesystem) beside mountRootFor, and where the spelling says inside and the mount says null they refuse with a reason of their own — including a launch directory that no longer exists at its spelling, the rename attack's shape. The launch-dir gate's memo made that inversion permanent: a null produced while mountRootFor consulted the filesystem was cached TRUSTED for the process lifetime, so one from inside the outer mount switched the gate off and a planted filter.<x>.clean executed as the review user (R26-2). Only the pure no-marker string scan is memoized now; inside a review temp dir the question is re-asked on every call, as documented. Also: releaseWorktree's not-freed predicate keyed only on refusal, so a probe's {status: null} (spawn failure, timeout kill, deleted cwd) read as "no objection" and freed:true was certified over a surviving registration and branch — the next add met "missing but already registered" (R24-3); and its first statement read the cwd outside any try, so a deleted cwd threw uv_cwd through the never-throws contract (R19-4). comment-status evaluated the same trust gate twice per run and could certify a trust state its own threads[] contradicted; the verdict is now computed once and handed to makeGitProbe (R26-4). mountRootFor's walk bound moves from the checkout's parent to the geometry-aware root (a linked direct parent refused every macOS /tmp checkout, R19-3), and its Windows refusal covers the colon-less UNC shape as a class (R19-2). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): round-trip the admin entry's backpointer in the location gate untrustedPointer admitted a gitfile rewritten to a SIBLING worktree's legitimate admin entry: the entry is outside the mount and is not the common dir, so both location questions passed while every host-side command through it measured and mutated the sibling tree (R23-2). The gate now carries the same backpointer round-trip the write paths always have: the entry's gitdir file must resolve back to the tree it is asked about, or the tree is refused. Only a definitive mismatch is this arm's to name — an unreadable or dangling backpointer redirects nothing, is refused by the downstream identity checks with their own reasons, and the one writer who could corrupt a backpointer to duck the mismatch cannot reach an entry outside the mount at all. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): count a lossy decode of git's answer as no answer (R8-2) encoding: 'utf8' maps an undecodable byte in git's byte-exact rev-parse output to U+FFFD, so a gitfile target holding an invalid UTF-8 byte was judged at the U+FFFD spelling while git acted on the original bytes: a plant under the raw-byte name inside the mount and a decoy under the U+FFFD one, symlinked to the tree's real admin entry outside it, passed every question the location gate asks — the round-trip included — while the gated write resolved through the plant. No gate may judge a spelling git never printed: a U+FFFD anywhere in the answer now counts as the question not answered, which every caller already fails closed on. The witness plants the full shape (raw-byte admin entry, U+FFFD decoy) and is itself gated to Linux: NTFS is UTF-16 and APFS rejects invalid UTF-8 names with EILSEQ, so neither the fixture nor the attack can exist there. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): fail the prune arm when git could not be run at all (R23-8) The symlink arm's pruneWorktrees() surfaced only the probe's refusal, so the third shape {out: null, status: null, refusal: null} — a spawn failure, the timeout kill, a deleted cwd — read as a successful prune: the arm announced "Removed ... link", wrote no stderr line, and released the lease over a registration git never swept, and the next worktree add met "missing but already registered" with nobody told why. Same shape-conflation class as R19-1 at a second site. Only genuine non-zero exits stay swallowed. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(review): type the lease-read mocks with their parameters tsc --build (unlike the vitest run) rejects the readReviewWorktreeLeaseAt delegation calling a zero-argument vi.fn with two. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): close what the trust-artifact move's first pass missed Four follow-ups from an adversarial audit of the fence: The untracked record was git status's collapsed path set: dist/ and node_modules/ recorded as one entry each, so a plant dropped INSIDE a directory the build left — the exact executable a host-side A/B runs — changed no membership and passed the subset fence. The record is now file-level (ls-files --others, which never collapses), so an addition inside a recorded directory IS a membership change. The trust file was keyed by the plan's path and mtime, and the plan lives inside the mount: a utimensat backdate to an earlier run's exact stamp re-keyed this run to that run's file, adopting its nonce. The key now takes max(mtimeMs, ctimeMs) — ctime cannot be set from userland, so the identity cannot be rolled back. The busy arm over-generalized: a rewritten pointer or moved HEAD on a tree this run built has no benign cause, and declining left the plant standing for the run. Those arms now fall through to discard-and-rebuild — while tracked dirt AND untracked additions (an A/B's own snapshot --update or cache output is the benign shape) decline busy instead. And the discard itself could not rebuild a planted tree: worktree remove refuses a tree whose gitfile no longer points back, so the removal cleared the coherent plant's admin dir while the real registration survived and the next add met "missing but already registered" — the deferred refusal->rebuild wedge. When the remove fails, the reverse scan now also runs, narrowed to the tree's own basename so a tampered sibling entry is never deleted. Also pins the sweep's per-field equality with a same-target doctored twin (the worktreePath comparison had no witness) and gives readReviewWorktreeLeaseAt a legacy found-at-path witness. * fix(review): close the audit's second pass — witness gaps and one substitution route From the adversarial audit of the takeover batch: - releaseWorktree's symlink arm now has its own witness for the status-null prune (the main arm had one; the sibling arm was pinned by nothing — the mutation survived green). - The launch-dir memo's comment now names the rename attack's actual premise — Node's cachedCwd, not a kernel property, so a future in-process chdir is the thing that re-opens the window — and the canary pins the stale-spelling premise instead of assuming it. - scratch-tree's --standalone shape asks the location question before buildStandaloneTree: headSha was read through the review worktree's own gitfile, and with no --fetched-sha pin a planted HEAD would put the plant's content in the tree presented as the commit under review. - revert-hunk's outermost-bound arithmetic gains a nested-geometry witness (a swap at the OUTER layer), so a drift to lastIndexOf reds it. * fix(review): anchor the scratch sweep at the captured root (R30-6) The entry-captured repositoryRoot was handed to the ancestor walks but not to scratchWorktreesOf: its resolve(worktree) and redirectedAncestor default stop both read process.cwd() again, so a cwd deleted mid-run threw uv_cwd out of the sweep — past the entry guard that exists to catch exactly that. Both now anchor at the captured root. The witness is the call count: a full cleanup reads process.cwd() exactly once. * fix(review): the base-tree fence reads only host-side state Rounds 27-30 found the nonce-in-marker design's hole: the markers carried the run secret in cleartext inside the tree — a child of the read-write mount — so reviewed code read it and forged a settling failed-marker (the A/B lane suppressed as "infrastructure") or an ok marker over a tree it shaped. The fence now reads the trust record alone, kept outside the mount: per tree it carries the pinned base, the build's verdict, and the untracked inventory per file with its size+ctime pair — an in-place rewrite of a recorded file is as visible as an added one. Markers in the tree are informational baseSha notes, excluded from every fence decision. The trust file is one per plan path, rotated in place on identity drift so no earlier generation survives to adopt; the run identity is the worktree lease's mtime when one is held (host-side, mount-untouchable) and the plan mtime with the ledger's tolerance otherwise. A mid-run mergeBaseSha rewrite conflicts with the pin and declines rather than rebuilding at a sha named after the fact. Same-run trees with a torn or missing record decline busy instead of being discarded; pointer/HEAD rewrites still discard-and-rebuild. The record step re-asks the pointer question first, and the git helpers carry the inert-config pins and a 512 MiB buffer so a real tree's ignored listing cannot silently drop the record. * fix(review): the mount-null is three answers, and the gates split them Where containment cannot exist — a drive-letter colon, a UNC share, a colon-bearing POSIX checkout — the review never ran sandboxed, so the mount is no trust boundary and the gates' docstrings promise silence. The fail-closed arm read every null as a refusal and refused unconditionally there (measured: 43 red tests under the declared Windows model). mountRootFor now shares one implementation with a discriminated nullKind (outside / unmountable / refused); the gates fail closed only on a redirect or a root that vanished mid-check, and stay silent where containment cannot exist. The pointer gate's round-trip is now exact: the admin entry must own the tree the question is about (untrustedRepositoryFrom compares against git's own --show-toplevel, which a borrow cannot bring into agreement), an owner that cannot be resolved fails closed, and a redirect planted between the mount root and the owner is refused before anything resolves through it — the round trip must not agree with a link. The mount root is threaded through the questions rather than re-asked mid-gate, so a mid-gate flip of the mount cannot split the answer. The flat-case walk bound moves to .qwen: the checkout's own directory is the user's layout, not the pipeline's. releaseWorktree's not-freed predicate keys on the prune alone now: a remove that answered 0 already cleared the registration, and keying on the remove's null published freed:false over a release the prune had completed. Witnesses pin each arm, including the symlink arm that had none. * fix(review): the legacy lease is never believed, the mirror never fatal The mtime-bounded honored read is gone entirely: a pinned-to-the-past cutoff froze the honored population at release day while remaining a utimes-forgeable permanent DoS channel from inside the mount. A legacy-path file is residue now — the mirror still replaces it (loudly, naming the displaced session, when it parses as another session's lock) so pre-move builds keep seeing the lock, and only the new path answers the gate. The mirror write is best-effort (an obstruction in the mounted directory is mount weather, warned and skipped, never a failed acquisition) and replaces via tmp-file + renameSync, which never follows a planted symlink and never opens a readerless FIFO. The finalizer is driven solely by the trusted directory: one destructive pass per target, the legacy twin deleted only when content-identical — and never listed, so a poisoned mount cannot shape the sweep. The lease directory re-roots lexically to the outermost enclosing repository in the nested geometry, so a review launched from inside another review's worktree keeps its host-trusted state outside every layer's mount. fetch-pr's step 4 also asks the destination's ancestors: a symlink left standing by a declined releaseWorktree would otherwise have mkdirSync and worktree add create and check out the PR's code at the link's target — outside the review temp dir, unsandboxed. And the review job's lease sweep feeds both globs through remove_review_tree's repair ladder instead of a bare rm -rf, so a mode-000 wedge is reported rather than silently surviving. Tests that delete the process's own cwd or need a mount are gated to hosts where those exist. * fix(review): judge the launch directory where the kernel stands (R30-47) The launch-dir gate judged Node's cached process.cwd() spelling while every wrapper spawns with no cwd, inheriting the KERNEL cwd — and the two diverge exactly once an ancestor is renamed. A leaf rename inside the mount plus the spelling re-stood as a symlink to an honest sibling made the cached spelling pass every arm while git ran in the renamed plant. The gate now asks the kernel (/bin/pwd -P) where the process stands and judges that spelling; a divergence is the rename, refused. Windows keeps the cached-spelling judgement: no /bin/pwd there, and no containment to protect either. The canary witness drives the full leaf-rename shape and goes red with the kernel judgement disabled. * fix(review): the second audit round's findings on the batch itself - base-tree trust: key under the OUTERMOST repository in nested geometry — the lease re-roots there (R27-6) and the trust file must follow, or it lands inside the outer review's writable mount where the fence's record is the outer code's to forge; runIdentityMs then reads the lease where it actually lives. - redirectedAncestor: keep walking past a nonexistent component — a leaf ENOENT under a symlinked ancestor used to end the walk with the link itself un-lstat'd, which defeated the R27-9 gate when the link sat above an unresolvable leaf. - createReviewWorktreeLease: a same-session refresh preserves the lease mtime — the base-tree run identity keys on it, and a resumed run would otherwise rotate the trust state and discard the standing base tree. - untrackedInventory is a null-prototype map: a file literally named __proto__ would otherwise land in the prototype setter, invisible to the fence. - mirror/atomicWrite tmp files are removed on a failed rename; the standalone refusal no longer prescribes a remedy (--fetched-sha) that the same gate would refuse. * fix(review): the mirror's tmp cleanup must not throw either The mirror is documented never fatal, but the rename-failure catch called rmSync unguarded: an rm error there escaped createReviewWorktreeLease after the new-path lease had already won, rolling back the acquisition — the fatal outcome the contract exists to prevent, one failure deeper. Same wrap in the trust store's atomicWrite, where a failing cleanup would have masked the original write error. * fix(review): import rmSync in the trust store, type the rmSync mock's path esbuild-transpiled vitest runs don't typecheck, so the round-3 commit built red in CI: atomicWrite's new catch used rmSync without importing it, and the lease suite's rmSync mock typed the path PathOrFileDescriptor where the real signature takes PathLike. * refactor(review): split the base-tree reuse fence out of this PR The fence landed here on 2026-09-08 to answer R29-1, and it has been the cluster the review keeps returning to: 11 of the 18 open Criticals live in `base-tree.ts` / `lib/base-tree-trust.ts` or in code those two pulled into other files, against 22% of the diff. Round 33 said it plainly — a cluster that keeps producing siblings is one root cause being fixed instance by instance, and splitting it ends the loop faster than another round. The dependency runs one way: the fence uses the location gates from `lib/worktree.ts`, nothing in the gates uses the fence. So it lifts out whole, and it comes back as its own PR stacked on this one. What leaves: - `lib/base-tree-trust.ts` and its suite — the host-side trust store. - `base-tree.ts` and `base-tree.test.ts` return to their state before the fence commit. The location gates this PR is about (`untrustedGitfile` at the reuse fast path, at the worktree read and at the rebuild) predate the fence and stay; `GIT_NEUTRALIZE`, `gitOutZ` and the 512 MB `maxBuffer` were introduced for the fence's inventory listing and leave with it. - The canary's base-tree case goes back to certifying on the in-tree marker — which is what the command reads again — and the fence's own record-step canary leaves. - `createReviewWorktreeLease`'s mtime preservation: it exists only so the fence's run identity survives a resumed run's re-acquisition, and it is R33-1's subject. Its tmp-file cleanup on a failed mirror rename is unrelated and stays. `packages/cli` is green (120 files, 5997 tests), `tsc --noEmit` clean, and the one deterministically-red test on Linux (R33-2) leaves with the fence file it lived in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012vBGpv2LrLQ8QNEYu2gsNn * fix(review): the seven Criticals the split leaves behind Every one is mutation-probed: the guard is removed or negated, the case named beside it goes red, and the rest of the suite stays green. `releaseWorktree` (R32-2). The `refusal !== null` disjunct that forced `freed: false` was not gated on "neither arm cleared the registration", while `couldNotRun` beside it was. Each probe re-asks `launchDirRefusal()` from scratch, so a refusal on one arm negated a release the other had completed — the same miskeying R31-1 and R28-11 record for the null-status arm. A refusal is already inside `couldNotRun` for its own arm, so it now only ever supplies the REASON for that verdict, never makes one. `untrustedPointer`, question 3 (R32-10). The containment test that gates the `redirectedAncestor` walk was bounded at the DEEPEST review temp root while the location question two above is widened to the OUTERMOST. A sibling of the enclosing review's worktree, relinked at this tree, sat outside the walk entirely and the realpath round trip agreed with the link. Both questions ask against `outermostReviewTmpRoot` now. `untrustedPointer`, question 2 (R33-3). `resolvedCommonDir` dropped the `notARepository` half of the taxonomy, so a timeout, a spawn failure or a git too old for `--path-format` read as "no objection" — and question 2 is the only one of the three that sees `gitdir: <repo>/.git`. It carries the taxonomy now and refuses every no-answer that is not git's own. `dropWorktreeRegistration` (R32-12). git appends a counter to a worktree id whenever the basename is taken, so the reverse scan's `id === basename` narrowing could not match `wt1` — the entry the tree actually owns — and left the registration standing after `rmSync` took the directory. Matched against git's own id grammar instead; the gitdir-names-this-path and tree-is-gone narrowings still carry the "never a sibling's" half. `fetch-pr` step 4 (R32-11). The destination guard walked `dirname(wt)` and never `wt`, so a link AT the worktree path redirected the create and the checkout. The walk starts at the leaf; `redirectedAncestor` lstats its first component before any stop test, so the ancestors are unchanged. `clearReviewWorktreeLease` (R32-4). The legacy delete is the one mount-resident removal in the clear path and it ran unfenced, after the trusted lease was already released — a planted mode-500 directory or a Windows handle threw out of `runCleanup`, whose tests pin it as never throwing. Loud instead, like the mirror write. The mirror displacement warning (R32-3). It interpolated a session id read out of the writable mount verbatim; a newline in it forges a second stderr line and a raw ESC repaints the terminal. Through `inertPath` now, the flattener the rest of the repo already uses, and bounded — the warning names the displaced run, it does not carry its payload. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012vBGpv2LrLQ8QNEYu2gsNn --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: wenshao <nigolaschao777@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-10 12:30:35 +00:00
易良	779cfe913b	fix(cli): drain notification queue when a turn errors out (#11559) ... * fix(cli): drain notification queue when a turn errors out * test(cli): reconcile error-time drain expectation with the fix The daemon Todo Stop Guard test pinned the old invariant that an unarmed turn does not drain the automatic queues when it errors out. That behavior stranded mid-turn background completions and pinned activeWorkState, which is exactly the bug this fix removes. The test now asserts the completion is drained and delivered after the error.	2026-09-10 12:01:04 +00:00
jinye	dfcf07accc	feat(serve): support 256 workspaces by default (#11515) ... * feat(serve): support 256 workspaces by default Make workspace registration capacity configurable from 1 to 256 with a default of 256. Preserve independent channel and ACP child policies, bound persistent registrations, and keep the expanded global session default at 800. Refs #11386 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): correct capacity docs and diagnostics from review - docs/users/qwen-serve.md documented --channels all; the flag is --channel. - The promotion path lost its server-side diagnostic when an oversize store refused the write, while the sibling create path kept logging. - The startup overflow message labelled explicit --workspace values as Restored, sending operators to a store that holds fewer records. - Registration-capacity validation named the environment variable even when an embedder supplied an out-of-range option. - The channel-owner message asserted retained owners were counted and told operators to stop a worker, neither of which holds at boot. - Session-total and new-409 documentation still described pre-change behaviour, and the design table's line anchors drift on merge. - Tests read the registration cap from the ambient environment, and neither the environment-driven cap, the legacy session derivation, the injected route cap, nor the internal-runtime exemption had a witness. --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-09-10 11:57:10 +00:00
易良	8259daeed0	fix(vscode): preserve active file exclusion (#11568) ... Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-09-10 11:31:54 +00:00
Shaojin Wen	4c6e2adcea	feat(web-shell): render URLs in user messages as clickable links (#11464) ... * feat(web-shell): render URLs in user messages as clickable links User-typed URLs in the Web Shell transcript rendered as plain text, so they could not be opened by click. Split message text with a small tokenizer (explicit http(s) scheme only, CJK characters terminate a match, trailing sentence punctuation and unmatched closing brackets are trimmed, a bare scheme stays text) and render URL segments through a LinkifiedText component that mirrors MarkdownLink's safety check and desktop-shell external-open routing. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(web-shell): harden user-message URL boundaries from review Address review on #11464: - Invert the tokenizer's character class from a hand-maintained denylist to an ASCII URL-grammar allowlist. The denylist absorbed unlisted scripts into the href (emoji and Thai shifted the host via IDNA) and truncated URLs containing an apostrophe (en.wikipedia.org/wiki/L'Aquila linked to the unrelated /wiki/L page). The allowlist fails closed and matches remark-gfm's boundary behavior. - Trim markdown emphasis delimiters (*, _, backslash) from the match end so a pasted **url** does not request /docs**. - Rewrite trimTrailing to count each bracket pair's excess closers once and trim by index, making the trim O(n) instead of O(n^2) on inputs with long runs of unmatched closers. - Pin the host-parser parts path in UserMessage with a URL test; tag the Playwright spec @smoke so PR CI actually runs it; reuse Markdown's .link rule instead of copying it; fix the garbled backtick bullet in both design docs and record the raw-IRI prefix tradeoff. Mutation witnesses: reverting the allowlist reddens 9 tokenizer tests, dropping * _ backslash from TRAILING_PUNCT reddens the emphasis test, and reverting the parsed-parts branch reddens the new UserMessage case. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(web-shell): drop dead TRAILING_PUNCT entries and correct the gfm-parity claim Address round-2 review on #11464: - TRAILING_PUNCT held three entries the allowlist can never produce (double quote, backtick, backslash), so the round-1 commit message's backslash mutation witness was false. The set is now exactly the reachable trimmable chars, and the quote/backtick it.each cases are retargeted as termination tests (they pin the allowlist boundary, not the trimmer). - The design docs' decision row claimed the allowlist matches remark-gfm boundary behavior; measurement says otherwise in both directions (trailing ~ kept, no left-boundary rule, stricter on trailing backtick/backslash, balanced a[0] keeps its ]). The row now says inspired-by and lists the known deltas, in both language versions. Mutation witnesses: dropping * and _ from TRAILING_PUNCT reddens the emphasis test; adding double quote to the allowlist reddens the new termination test; the removed dead entries were verified to change no test outcome (that is why they are dead). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(web-shell): trim trailing tilde, normalize bare % in href, pin tokenizer coverage Address round-3 review on #11464: - Add GFM's strikethrough delimiter ~ to TRAILING_PUNCT so a pasted ~~https://github.com/QwenLM/qwen-code~~ does not 404 on click, while interior tildes (/~user) stay intact. The docs' delta list drops the now-fixed 'trailing ~ kept' entry and gains the measured character- reference divergence (&amp; kept verbatim where remark-gfm resolves it). - Normalize a bare % (not followed by two hex digits) to %25 in the rendered href only — byte-identical to the assistant markdown path's normalizeUri — keeping the visible text verbatim; the docs' 'the one case' superlative is replaced by the measured wrong-link shapes. - Pin the tokenizer axes the suite previously left green under mutation: interior membership for all 11 unpinned allowlist characters plus a host:port port fixture (dropping : now reddens two tests), termination for all seven excluded printable ASCII delimiters, the angle-bracket autolink form, and the no-left-boundary decision. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(web-shell): match normalizeUri's % rule and pin the desktop opener's href Address round-4 review on #11464: - The bare-% normalization used a hex-only lookahead, while the assistant path's normalizeUri (mdast-util-to-hast) skips encoding when two ASCII alphanumerics follow — so 50%off diverged between user and assistant bubbles. The lookahead is widened to [0-9A-Za-z]{2}, measured identical to the real normalizeUri on every reachable input (space cannot appear in a match). The comment now also names the remaining delta: raw [ ] { } stay verbatim where normalizeUri percent-encodes them, and both design docs record it; the English IRI row's inverted claim about the assistant path is corrected (it does normalize). - Pin that the desktop external opener receives the normalized href, not the raw matched text — the segment.value mutation survives all previous tests and now reddens the new desktop-click case. - Correct the interior-fixture comment in linkify.test.ts: only six of the eleven characters are trimmed by TRAILING_PUNCT at a trailing position; the other five are interior for fixture uniformity. Mutation witnesses: hex-only lookahead reddens the 50%off case; passing segment.value to the opener reddens the desktop-click % case; both stay green on the committed source. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-09-10 11:26:08 +00:00
Shaojin Wen	76635386e4	fix(review): dispatch review agents through parallel workflows (#11524) ... * fix(review): dispatch review agents through parallel workflows * fix(review): respect workflow preferences and validate loaded scripts * test(web-shell): align fixtures with branding and compact footer	2026-09-10 11:15:56 +00:00
易良	424e40cc1f	test(core): cover bundled ConPTY fallback and replies (#11555) ... Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-09-10 09:58:30 +00:00
易良	585589ac67	fix(dev): install hooks during worktree bootstrap (#11496) ... * fix(dev): install hooks during worktree bootstrap * fix(dev): keep worktree hook setup out of the shared git config Husky runs `git config core.hooksPath .husky/_` with no --worktree, so from a linked worktree the value lands in the config every worktree of the repository shares while `.husky/_` is created only in the checkout being bootstrapped. Skip the Husky step and report it when the key is unset and this checkout does not own the repository config, so a bootstrap can no longer repoint hook resolution for roots that never received the wrappers. A primary checkout still installs hooks, and an already-configured `core.hooksPath` is untouched. Also drop the caller's success exit, which `install()` made unreachable when it started exiting on every successful path, and bring the pnpm-worktree-bootstrap design doc in line with a hook step it still recorded as deliberately skipped. The new fixture runs the real script against a throwaway root whose `.git` is a file or a directory and whose config comes from a real `git init` repo, which makes both new branches reachable and pins the fail-closed guard: the injected `GIT_CONFIG_*` constant holds one value for the child's whole lifetime and cannot express the unset state that asks husky to write. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtuo29vduf * fix(dev): ask git which root owns the worktree hook config `ownsRepositoryConfig()` inferred repository-config ownership from the filesystem shape of `.git`, and the proxy is wrong at both ends. With no `.git` at all, `statSync(..., { throwIfNoEntry: false })` returns `undefined` and the predicate folded that absence into "owns the config", so a repository-less checkout ran husky into its `.git can't be found` soft failure (exit 0) and the fail-closed check then turned a successful dependency install into exit 1 blaming Husky. A `.git` file is not only a linked worktree either: `git clone --separate-git-dir` checkouts and submodules have one too and do own their config, so hooks were declined where they would have been correctly scoped. Ask git instead: `rev-parse --git-dir` differs from `--git-common-dir` only in a linked worktree, and a failed `rev-parse` names the no-repository state, so the skip notice stops asserting "linked worktree" and the bootstrap does not gain a hard git dependency. Also bind the fail-closed check to an artefact husky's own write produced, not only to the config value. husky 9.1.7 exits 0 on every soft-failure path (`index.js:16` git command not found, `index.js:17` refused `git config` write) before the `mkdirSync(_())` on line 19, and a linked worktree inherits `core.hooksPath` from the config it shares, so re-reading that value compared it against itself and passed exactly when husky had created nothing. Restore the registry-fallback case's hermeticity: `PATH` holds only the stub directory again, which now also pins that the retry needs no ambient git. Rebuild the ownership fixture from real git layouts, because `rev-parse` resolves nothing for a `mkdirSync`'d `.git` or a hand-written `gitdir:` file, and give the stub husky a failing mode so the exit code husky returns is pinned rather than the install result's. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtuys2rjuu * fix(dev): surface hooks-path read failures in worktree bootstrap - getHooksPath() no longer collapses a refused git config read (exit 128/2/3) into "unset": only an absent key (exit 1) or a missing git binary keeps the skip path; anything else fails the bootstrap with the read error instead of a green, hook-less worktree. - The linked-worktree skip notice now names the recovery path: re-run this script once the primary checkout has hooks installed. - Tests cover a git stub exiting 128, the real-world unset HUSKY state, and pin the recovery sentence in the skip notice. --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-09-10 09:44:47 +00:00
probe	0b018e23b1	fix(ci): sync main and complete continuation test fixture	2026-09-10 17:40:02 +08:00
易良	fe7cf68644	fix(ci): grant the packaging check the permissions its callee declares (#11532) ... The scheduled packaging check failed to start. A caller must grant every permission the called workflow's jobs declare, and the release workflow's publish job declares `contents: write` while the OSS mirror declares `actions: read` — the check granted only `contents: read`. That validation runs when the workflow file is loaded, before any `if` is evaluated, so the two jobs being unreachable in a dry run does not exempt their permissions. The run died in two seconds with no job and no annotation, which is what a startup failure looks like. Grant both. Nothing here writes: the check hardcodes `dry_run` to true, which skips the publish and the mirror job on their own conditions.	2026-09-10 09:02:21 +00:00
qqqys	0ef35351e9	feat(acp): register daemon-managed sessions in the session registry and let them send peer messages (#11488) ... * feat(acp): register daemon-managed sessions in the session registry and let them send peer messages Only the interactive UI registered, so a session `qwen serve` drives was invisible to `qwen sessions ps`, absent from every peer's `list_agents`, and unable to send at all: the send path reads this process's own record to learn its reply address, and with no record it reported the feature disabled. A user with a terminal open and a daemon session working in the same repository could not have the two speak to each other. The registry was keyed by PID, one record per process, which was right while a process meant a session. The `qwen --acp` child a daemon spawns hosts up to thirty-two at once, each with its own id, name and working directory, and one record cannot describe them. So a process hosting several writes one record per session, `<pid>-<8 hex>.json`. The suffix is minted at registration and never moves — deliberately not derived from the session id, because an id is swapped underneath a live session by a `/clear` or a session load, and a record that renamed itself would strand every reader holding the old name. The PID in front of the suffix is still what liveness, the sweep and the namespace guards read, so a minted record is reaped exactly like a shared one, and a reader too old to know the name does not see it rather than mistaking it for litter. Which record a caller means is a slot, threaded through patch, unregister and the own-record read. A Config learns its slot from the registration itself, so a session's `/clear`, its `/cd` and its exit each touch its own record and no sibling's. The process binds one inbox for all its sessions and writes that address into every record. Senders already pin `toSessionId` on every frame, so that is what tells the sessions apart; a frame naming a session this process does not hold — or naming none, which used to be deliverable because there was only one session it could have meant — is answered `misaddressed`. That sharing broke an assumption elsewhere. The send path and `list_agents` excluded "this session" by matching either its id or its reply address, the address being a proxy for the id that held while one process bound one socket. Siblings share an inbox, so the proxy stopped holding and every sibling of the sending session would have been filtered out as if it were the sender. Identity is the session id alone now; a twin under a second process is still excluded, because that is what the id catches. Inbound is refused rather than held. A hold is a question put to a person, and nobody is watching a held-message list on a daemon-managed session's behalf, so holding would leave every sender waiting out a five-minute expiry to learn nothing. Where a held message should surface for these sessions is a real question and separate work; the protocol page says so rather than leaving a reader to find out. Registration happens only while cross-session messaging is on. The interactive UI registers unconditionally because its record also answers "what is running right now"; a daemon-managed session's record exists to be addressed, and one with no inbox behind it would put a name in every peer's listing that can be addressed and never answered. The inbox is bound by the first session that needs one, and the transport is imported at that moment, so an ACP process that never messages a peer never loads it. Closing is registered as exit cleanup and also runs from both teardown methods, because a bare signal reaches neither. Whether to bind at all is read from the settings the process started with: the agent's `settings` field is re-pointed at whichever session is being handled, and this is a question about the process. Part of #10925 Unblocks #10118 (M3) * fix(acp): import the peer-messaging transport statically so the fast-path bundle closure stays clean The agent imported the transport and `registerSession` with dynamic `import()`s, on the theory that a process with messaging off should not load them. The theory bought nothing and cost something: every module behind both imports already sits in the agent's static closure, and a dynamic import of the core barrel turns that barrel into a code-splitting entry. esbuild then re-partitions the shared chunks, and one of the chunks the agent imports statically came to hold iconv-lite's encoding tables — a module the ACP fast path must not load, which is exactly what the startup bundle closure check exists to catch, and did. Both imports are static again, the catch-up loop that existed only to cover the import's latency is gone, and the design note says why the transport is loaded the way it is. Part of #10925 * fix(acp): judge hosted sessions by live ids and their own settings Review follow-ups on daemon-managed session registration: - ownsSessionId tested the frozen publication key, so a frame pinned to the id the record actually advertises after /clear was answered misaddressed; test each hosted Config's live id as well. - Registration and the inbox bind followed the process's startup settings; each session's own settings decide instead. - A failed inbox bind latched as started, so no session ever retried it; treat null/failure as not-started, and latch closed on teardown so a retry cannot resurrect the socket. - disposeSessions awaited the inbox drain before aborting generations; abort first so a running turn cannot keep executing tools after the client is gone. - The send-side mirror re-minted its bucket per addressed session id while the receiver meters per address, letting alternating siblings reset the allowance and duplicate window; keep one bucket per inbox address and stop the repeat refusal naming "that session". - The self-exclusion could be beaten by a re-id patch landing between the own-record read and the directory read; re-check a peer on this session's own inbox against a fresh read, on the send path and in list_agents alike. - The record filename check accepted a zero-padded pid on platforms without a namespace id; require the canonical decimal form. - Drop the dead sessionId parameter from the misaddressed-frame re-assert, document the two record filename forms and the one-sender consequence, and add the Chinese design doc. * fix(ipc): probe each inbox once, and keep a minted record nameable Two behaviour fixes from review, plus the tests that would have caught them and the cases the slot plumbing was missing. A process hosting several sessions writes one record per session, all pointing at the one inbox. The liveness pass dialled that socket once per record, so a daemon with 32 hosted sessions opened 32 simultaneous connections at one address every time a peer listed the directory — enough to trip the inbox backlog. Probe each distinct address once and apply the verdict to every record that names it. Re-registration deleted the captured path of a minted record before it knew whether the delete would happen. On a transient read error it returned early with the capture already gone, and a minted suffix cannot be derived a second time: the record stayed on disk under a name this process could no longer patch or remove, outliving the session as a directory entry pointing at a dead inbox. Keep the capture on that one branch. Tests: every registration fixture returned the shared slot, which is also the parameter default, so nothing could tell "passes the slot it registered under" from "passes the default" — config and send_message now register a minted slot and name it in the assertion. Also covers the unknown-slot path against a live shared record, a minted filename surviving an id swap, near-miss record filenames, probe-once, the ownsSessionId branches of the inbound gate, and an ACP host running two sessions through one inbox. * test(core): name the minted slot in the list_agents identity read --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>	2026-09-10 08:30:36 +00:00
probe	1740eaa653	feat(web-shell): continue interrupted sessions	2026-09-10 15:52:05 +08:00
qwen-code-dev-bot	2488d12d8b	fix(web-shell): realign two suites with shipped behavior to unbreak main CI (#11525) (#11530) ... The brand PR #11244 landed on top of #11470 and left two web-shell suites failing on main, redding the Test job: - The sidebar brand tests assert the version tooltip's title while mounting at the default 260px width, but #11470 hides the version label below the 344px compact footer breakpoint, so the query finds nothing. Mount those brand tests at 360px, matching the convention of the footer-version suite. - The workspace-session loading test pins the exact daemon requests issued on load, but the workspace provider now fetches GET /brand beside capabilities. Answer it with a 404 (an older daemon without the route, which settles the fetch instead of arming the retry timer) and expect the call, including the StrictMode remount double. Both are test-environment alignments; no production behavior changes. Fixes #11525 Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: Shaojin Wen <szujobs@gmail.com> Co-authored-by: 易良 <1204183885@qq.com>	2026-09-10 07:51:15 +00:00
qwen-code-dev-bot	b74592785b	fix(ci): record the qwen-autofix.yml size drift two in-allowance PRs accumulated (#11529) (#11537) ... The ratchet tolerates 4096 bytes of growth per PR without a baseline bump. Two consecutive qwen-autofix.yml PRs — #9940 (to 465612, +2522 over the recorded 463090) and #11516 (to 469259, +3647 more) — were each inside the allowance against their own base, so neither was forced to touch the recorded number, and the sum (6169) sails past it. Since #11516 landed, every strict gate run — a local run without a PR base SHA, the push run of the PR itself whose event.before predates the growth, and any future PR that touches qwen-autofix.yml — fails on growth it did not author, the red-wall shape #9904's leniency cannot cover for the file's own editors. Record the measured size (469259, wc -c on main) as the gate's own error message prescribes. The file now sits 741 bytes under the 470000-byte absolute gate, so the next real growth has to move prose into qwen-autofix.md instead of reaching for the ratchet again. Mutation probe: with the entry at 463090 the strict vitest mirror fails ("6169 bytes over its recorded 463090"); at 469259 the workflow-size suite passes 208/208 and check-workflow-size.sh exits 0.	2026-09-10 07:44:10 +00:00
易良	b3d022b357	fix(vscode): restore pre-cutover conversation history in the panel (#11495) ... * fix(vscode): surface pre-cutover conversations in the panel history The WebShell cutover (#9811) switched the companion's history list to the daemon catalog filtered by sourceType 'vscode'. Sessions recorded before creator attribution existed carry no source metadata, so upgrading to v0.23.x emptied the panel history even though every transcript is still on disk (#11489). Claim the panel's own legacy sessions back instead of widening the filter: the host ships its globalState conversation ids as an allowlist, the panel intersects it with the daemon's default (unattributed) catalog, and opening one of them lazily stamps the 'vscode' source through the existing restore-time attribution path. globalState stays untouched for downgrade safety, and unattributed CLI/browser sessions stay out of the panel. * fix(vscode): run the legacy session scan once per bootstrap Review feedback: the allowlist never converges for ids that can no longer match (other workspaces' conversations, deleted transcripts, sessions already stamped by a restore), so keying the scan to every history-dropdown open re-paged the default catalog indefinitely. Gate it on a per-bootstrap done flag instead; failures still retry on the next open. Also unpin the scan page size from the test. --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>	2026-09-10 07:27:15 +00:00
易良	11e629b3a1	perf(export): split the transcript renderer's embedded CSS into a versioned asset (#11485) ... * perf(export): split the transcript renderer's embedded CSS into a versioned asset The export renderer carried the web-shell component stylesheet as a ~2.3 MB string literal, so every reader parsed and compiled 4.1 MB of JS (56% of it dead CSS) before a transcript could render. Lift that literal out at export build time into a version-pinned, SRI-protected export-transcript-document.css served from unpkg and loaded via a nonce-bearing <link>, dropping the renderer JS to ~1.83 MB. The transform is an esbuild onLoad plugin in the web-templates export build that strips the injected CSS constant from web-shell's dist/transcript.js; web-shell source and runtime behavior are untouched. The document's fail-closed load-error path is extended to the stylesheet so a missing CSS asset fails the same way as a missing renderer. * fix(export): match the transcript CSS entry on Windows paths too esbuild hands plugin callbacks the platform-native absolute path, so the extract-transcript-css `onLoad` filter never matched on Windows: the callback did not run, `extractedTranscriptCss.css` stayed undefined, and the mandatory extraction guard below aborted the build. That build is not platform-gated — `scripts/prepare.js` runs it from `prepare`, so `npm ci` itself would fail on every Windows contributor and on the windows-latest legs of test_windows and desktop-release. Widen the separator to `[\\/]`, keeping the `transcript\.js$` tail so the barred `web-shell/dist/index.js` package root still does not match. The filter moves to transcript-css-entry.mjs because build.mjs is a top-level-await script with no harness — the same reason scripts/sdk-node-exporter-stub.js exists — so scripts/tests/transcript-css-entry-filter.test.js can pin both separators. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtufhms2u4 * fix(export): latch a stylesheet failure ahead of the parser block The `<link id="transcript-stylesheet">` sits in `<head>` while the `window` error listener that catches its failure is registered by an inline script in `<body>`. Chromium parser-blocks that script on the pending stylesheet, so when the CSS failure settles first the error event is dispatched with no listener to receive it: nothing marks the render as failed, both renderComplete guards in document-main.tsx pass, React mounts the transcript without any of the component CSS, and the requestAnimationFrame stamps `data-render-complete="true"`. The reviewer measured this fail-open above roughly 2.1 MB of document HTML (272 of the 1,000 permitted blocks) for a 404, an SRI rejection, a truncated body and a destroyed socket alike, and fail-closed for a *late* failure — so size, not failure kind, decides it. Latch the failure in `<head>` before the `<link>` is parsed and act on the latch from the existing body IIFE. The head script only records: `showLoadError()` writes `document.body.dataset` and `#app`, neither of which exists while the parser is still in `<head>`. It carries `nonce="__EXPORT_NONCE__"` because the document CSP allows no inline script, which is safe — `formatters/html.ts:53` replaces every occurrence. The listener is capture-phase because resource error events do not bubble. Not the `link.sheet === null` variant: the reviewer measured `sheet` non-null for a 404, a truncated body and a destroyed socket, so it only detects SRI rejection. scripts/tests/export-transcript-document-template.test.js pins the position, the nonce, the capture phase and the record-only shape; all five cases go red against the unpatched template. The behavioural witness (real Chromium, large document, instant CSS abort) belongs to the playwright transcript gate, which is out of budget on this host. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtufhms2u4 * fix(scripts): name the missing export renderer assets, pin the CSS gate The bundle copy became all-or-nothing over two artifacts but its `else` warning still named only the renderer, so the one new way to reach that branch — a tree built before the split, then `npm run bundle`d without rebuilding web-templates, which has the JS and no CSS — told the operator to go looking for a `export-transcript-document.js` that was sitting right there, and silently discarded it. List the paths that are actually absent, matching the sibling Web Shell warning twenty lines above. Stays warn-and-skip: prepare-package.js is the release gate. Also pin that release gate. Every fixture that reached `preparePackage` staged `dist/export-transcript-document.css` unconditionally, so deleting the new required-path entry left the whole test:scripts lane green; a release built with `npm ci --ignore-scripts` would then publish documents whose stylesheet 404s on unpkg for that version. `verifyBundleArtifacts` reports through console.error + process.exit(1) rather than a throw, so the new case stubs exit instead of copying the audio-capture sibling's `toThrow` idiom. Both cases were flip-checked: restoring the old warning text, and deleting the CSS line from prepare-package.js, each turn their case red. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtufhms2u4 * fix(export): pin the stylesheet-failure id contract and sync the design docs The <head> latch, the body listener and the <link> each spell 'transcript-stylesheet' independently and nothing compared them, so renaming either listener's id left the whole suite green while the latch recorded nothing - reinstating the fail-open the latch was added to close. Derive the id from the <link> and assert both listeners compare against it. Verified red under both mutations: latch id -> 'transcript-renderer' (1 failed | 5 passed), and the mirror with the body listener's id wrong and the latch intact (same). Both design docs still specified the two shapes the previous round replaced: the forward-slash-only onLoad filter that never matches on Windows, and the body-listener-only fail-closed extension. Section 1 now quotes the shipped TRANSCRIPT_CSS_ENTRY_FILTER and names transcript-css-entry.mjs, section 2 describes the <head> latch (position, nonce, capture phase, record-only), section 3 names the module-level render guard, and "Files affected" lists the three omitted files. EN and zh-CN are updated in the same commit. Also correct the shape-guard comment in build.mjs: the document nonces every <style> created through document.createElement, so the CSP would not block an un-stripped duplicate, and a 367-byte regrowth stays inside both byte budgets. That throw is the only guard on the duplicate-injection path. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtuo29vduf * fix(export): close out transcript CSS review comments * fix(export): tighten transcript CSS closeout Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-09-10 07:15:32 +00:00
qqqys	1097b9fe0a	fix(web-shell): move scheduled task row icon into trailing meta slot (#11356) ... * fix(web-shell): drop the per-row scheduled-task icon in the sidebar The row-level calendar icon sat left of the group header icon, so scheduled-task sessions read as outside their group; the group header already carries the calendar marker. * fix(web-shell): preserve scheduled task row interactions * fix(web-shell): keep touch taps on session rows * test(web-shell): cover scheduled task row statuses * fix(web-shell): keep session row actions reachable on touch and focus The hover-gated reveal left session actions unreachable on hoverless devices, and the meta-slot width reservation no longer matched the reveal conditions. Always reveal the overlay under (hover: none) with the tap-through guard intact, key the focus reveal and reservation on container focus instead of :focus-visible, and merge the forked marker hide list into one rule. Pin the stylesheet invariants in a css source test and the coloured-row marker slot in the workspace-removal suite. * fix(web-shell): keep session row markers visible on touch devices Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(web-shell): preserve touch markers after actions --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-09-10 06:55:52 +00:00
易良	b1ac3e2970	docs(daemon): add a REST integration entry point, guarded against the route surface (#11477) ... * docs(daemon): add a REST integration entry point The daemon REST surface is documented — 3.5k lines of route reference plus 21 deep-dive pages, all published. What is missing is the way in. An integrator lands on a 237-route surface with no signal about which routes are meant for them, and `users/overview.md` answers "how do I start using the CLI", not "I am putting this inside my product". Add one page that answers the second question: - Which of the six daemon integration paths to take, and what each one leaves the integrator owning. - The 25 routes a REST integration actually uses, grouped by job and linked to their reference sections — against 237 registered, the rest being Web Shell internals that change with that UI. - The minimal flow end to end: preflight, create, subscribe, prompt, answer permissions, close. - Two facts that are load-bearing at design time and currently buried: the daemon spawns `qwen --acp` children so the CLI must be on its host, and the auth model is single-operator with no per-end-user principal. This deliberately links rather than restates. The reference stays the authority; 17 of the 25 routes have a dedicated section there and the page says plainly which 8 do not, rather than papering over it (#11359). Refs #11427, #11359 * test(serve): guard the REST guide's promises against the route surface The guide tells integrators a specific 25-route subset is what they should build on, and links 17 of them into the protocol reference. Both claims are hand-maintained against a 237-route surface, so a rename upstream would leave the page promising a route that no longer answers, or pointing at an anchor that no longer exists, with nothing failing. Four checks, same shape as capabilities-docs-contract.test.ts: - every route the guide names is still registered somewhere under serve/ - every protocol-reference anchor the guide links resolves to a real heading - the 8 routes the guide marks "no dedicated section yet" are exactly the 8 that lack one, so writing a section forces the note to be removed - the source walker still sees >100 routes, so the first check cannot pass vacuously if registrations move to a style the regex misses Scope is the guide's own promises, not an inventory of all 237. A full coverage ratchet would need a ~174-entry "known undocumented" baseline; that is bulk in service of a problem nobody has reported, and can be added if the undocumented set starts moving. Verified statically against this tree: all four pass, and the walker sees 213 routes. Refs #11427, #11359 * docs(daemon): correct the process model and the memory flags Review on #11477 caught two accuracy defects and two smaller ones. All four verified against source before fixing. **The process model was stated backwards.** The page said each session's agent gets its own process and a crash is contained to one session. It is one child per *bridge*, and one bridge per *workspace runtime*: "At most one `qwen --acp` child per bridge… Multiple sessions multiplex onto the child via `connection.newSession()`" (bridge.ts), and the single `channel.exited` cleanup "tears down ALL multiplexed sessions" (ensureChannel). `daemon/00-index.md` and `users/qwen-serve.md` already said so. The container-sizing advice was wrong in the same direction, and "crash contained to one session" inverted the fault domain — it is the workspace. Rewritten, with the real lever named: run separate daemons, since `--max-sessions` caps concurrency, not blast radius. **The memory row read as enforcement.** Both flags are observe-only: `--memory-budget-mb` "does not change how any `qwen --acp` child is sized", and `--child-heap-mode` reports a modelled partition where "Nothing is applied: no child is sized from the budget and no spawn is refused". **Dropped the `stopReason` enum** rather than repeating it. Source has `end_turn`, `cancelled`, `error`, `max_tokens`; `refusal` and `max_turn_requests` also exist in packages/ but not in the protocol doc, and the `length` the protocol doc lists appears nowhere in source. Copying a list that is already wrong upstream would make a fourth copy of it — the page now links to the route section instead. The upstream enum still needs fixing (#11359). **Dropped the hardcoded "237 routes".** It disagreed with this PR's own test — 237 counts method+path pairs, the walker counts 213 distinct paths — and volatile totals in prose are what capabilities-docs-contract.test.ts exists to discourage. The point stands without a number. Contract assertions re-verified after the edits: 0 unregistered routes, 0 broken anchors, the undocumented set is still exactly 8, walker sees 213. * docs(daemon): correct the stopReason values in the protocol reference The list under `POST /session/:id/prompt` was wrong in both directions. It gave `end_turn`, `cancelled`, `max_tokens`, `error`, `length`. Against source: - `length` appears nowhere in `packages/`. It is not an ACP `StopReason` and the daemon never emits it. - `max_turn_requests` and `refusal` were missing. Both are in the ACP union (`@agentclientprotocol/sdk` `schema/types.gen.d.ts`: `StopReason = "end_turn" | "max_tokens" | "max_turn_requests" | "refusal" | "cancelled"`), so they arrive from the agent even though qwen-code never constructs them itself. - `error` is real but is ours, not ACP's — `create-sub-session.ts:695` and `live-session-coordinator.ts:1659` set it when a turn fails inside the daemon. - `reconstructed_from_transcript` was missing entirely (`prompt-terminal-ledger.ts:295`), for turns recovered from persisted history. Rewritten to separate the ACP values from the two the daemon originates, and to say plainly that the field is an open string — it is typed `string` on the wire (`bridgeTypes.ts:1258`), the ACP set can grow, and a client that exhaustively switches on it breaks on the next addition. That last part is the guidance the old list quietly worked against. Found while fixing #11477's own copy of this list, which is now a link rather than a fourth transcription of it. Refs #11359 * docs(daemon): correct six daemon claims in the REST guide and protocol reference Each claim below was checked against the code at this head and was stated the wrong way round. - qwen-serve-protocol.md: `turn_complete.data.stopReason` never carries `error` or `reconstructed_from_transcript`. A daemon-side failure publishes `turn_error` with its `code`/`errorKind`; the two `stopReason = 'error'` sites are SSE consumers' local variables (create-sub-session.ts:695, live-session-coordinator.ts:1659), and `reconstructed_from_transcript` is a ledger record (prompt-terminal-ledger.ts:295) that surfaces only as `promptTerminals[].stopReason` on POST /session/:id/load. The ACP five and the open-string instruction are kept. - SDK row: the Python SDK has no daemon transport at all (sdk-python.md:15-16), so the row names TS and Java and states the gap instead of implying it away. - Step 3: `model_switch_failed` is the only signal on the attach path; a fresh create also returns `modelApplied` in the 200 body (bridge.ts:5755-5816 -> session.ts:3563). The attach return object has no such field (bridge.ts:9828-9845) and swallows the switch deliberately (:9794-9806). - Step 5: the permission timeout defaults to 0 = wait indefinitely (bridge.ts:2726, commands/serve.ts:623-628); it does not "fire". - Step 5 + Permissions table: lead with the session-scoped vote route. POST /permission/:requestId is wired to primaryBridge only (routes/permission.ts:107-121, server.ts:3286-3287), so it 404s for a session owned by another registered runtime, with the same body as a lost vote. - Operations memory row: `--memory-budget-mb` has a live consumer — the adaptive live-journal growth pool (daemon-memory-budget.ts:19,37,38 -> run-qwen-serve.ts:3802-3812 -> bridge.ts:2912) — so it is not observe-only. The route/anchor contract in rest-integration-docs-contract.test.ts is unchanged: no route renamed, no anchor link added or removed, and the session-scoped permission route stays outside the protocol reference so the pinned undocumented eight still holds. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtufhms2u4 * docs(daemon): correct the turn_error, promptTerminals, modelApplied and approval-mode claims Four review findings, one defect class: a documented claim the code does not back. Docs only — no product code touched. - `turn_error`: `data` always carries `message`; `code` only when the daemon classified the failure, and `errorKind` on none of the three named paths. A prompt rejected because the ACP child died mid-request carries neither (bridge.ts:2119-2120 conditional spreads, :1949-1956 extractErrorCode needs a `code` property, :2065-2071 classifyTurnErrorKind matches only 'terminated', status.ts:100-107 BridgeChannelClosedError has none). Teardown flush and deadline do carry `code` (:2434, :10303). - `promptTerminals`: reconciliation appends two record shapes and the interrupted one has no `stopReason` key at all, so the reader is keyed on `terminal` instead (prompt-terminal-ledger.ts:283-297). The field is omitted entirely when the ledger holds no evidence (:374) and is attached only for `load` (routes/session.ts:4471-4478). - The deadline paragraph four lines below named `errorKind: "prompt_deadline_exceeded"`; the publisher sets that value as `code` (bridge.ts:10300-10306). - `modelApplied`: assigned only inside `if (modelServiceId)` and spread only when defined (bridge.ts:5755-5766, :5816), so step 2's bare create body yields a `200` with no such key. - `permission_request`: the daemon pins no approval mode at spawn (acp-child-extra-args.ts:15-20, spawnChannel.ts:484-492), the settings default is ApprovalMode.AUTO (settingsSchema.ts:2905, config.ts:1642), and AUTO's filter skips requestPermission for the allowed class (Session.ts:12346-12354) — so auto-approval is the default posture, not a decision. An untrusted folder is forced to DEFAULT (config.ts:1646-1655) and /capabilities reports the vote policy only (routes/capabilities.ts:101-104). Verified: packages/cli rest-integration-docs-contract.test.ts 4 passed, and prettier --check clean on both files. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtusclhbul * docs(daemon): correct the REST integration guide's route, token and sizing claims * docs(daemon): correct REST integration review findings Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(serve): check guide tables and documentation targets Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-09-10 06:49:42 +00:00
Shaojin Wen	f6540d1994	fix(web-shell): make the workspace provider guard self-diagnosing and reload on root retry (#11421) ... * fix(web-shell): make the workspace provider guard self-diagnosing and reload on root retry The strict useDaemonWorkspace guard fails closed with a message that cannot distinguish an absent provider from a duplicated module copy (two DaemonWorkspaceContext instances in one page, seen in dev when the module graph is refreshed under a live page), so the root boundary showed a dead-end fallback that only a manual reload could clear. The provider now registers a per-module-copy marker on first render, and the guard's error states which of the three cases it hit: no provider rendered, a provider rendered from a different module copy (with both copy ids), or a provider from this copy rendered and the consumer is outside its subtree. The standalone root boundary's retry now reloads the page, since re-mounting the same broken module graph would throw again. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(web-shell): restore spies in afterEach so a failing retry test cannot leak the console.error mock Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(web-shell): guard the root-fallback reload behind token survivability and sharpen copy diagnostics Review round on ca4cdfb7 found that the unconditional reload could strand the shell unauthenticated when the daemon token never reached storage, and that the diagnostics had four misreport shapes. Retry now reloads only when a reload can still authenticate (URL or per-tab persisted token) and falls back to an in-place reset otherwise, with the button label matching the action via a new retryMode prop on RootErrorFallback. The copy registry tracks rendered-vs-provided per module copy, prefers the duplicate-copies branch when foreign copies exist, carries the module URL in each copy id, and is capped so dev hot re-evaluation cannot grow it unbounded. Mutation checks performed: removing the reload-survivability guard, the label plumbing, the harness try/catch, the URL-bearing id, the helper-local root, the foreign-copy precedence, or the registry cap each turns the corresponding new test red; reverting restores green. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(web-shell): address review round 3 — tokenless reload, shared token parse, bounded-map util, wording honesty - main.tsx: the survivability gate now also passes when no token was resolved at boot (tokenless trusted loopback strands nothing), and the reload carries the live theme/language across, since session switches strip those one-shot URL params. - config/daemon.ts: the URL token grammar is parsed once in readTokenFromLocation(), shared by getDaemonToken() and the survivability predicate so the spellings cannot drift. - DaemonWorkspaceProvider.tsx: copy ids fall back to a module name when import.meta.url is lowered away (esbuild iife export documents); the registry insert goes through a new utils/bounded-map helper (the package's fourth copy of that loop, and the first with the correct exit test); both guard branches admit unmount/lost-client explicitly. - export-html build.mjs: silences the deliberate empty-import-meta warning and fails the build if it resurfaces. - Tests: boot-order replay pinning that the predicate never consults getDaemonToken()'s cache, a same-element re-render pinning 'provided' as terminal, a live-copy-eviction test that seeds foreign copies after the live one, the tokenless-reload and theme/language-carry cases in main.test.tsx, the zh-CN reload label, and bounded-map unit tests. Each new guard was exercised as a mutation first (removed or reverted) and confirmed to turn its test red. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(web-shell): address review round 4 — one-shot param strip, live-URL reload, a firing import.meta guard, finish the bounded-map migration - main.tsx: boot strips the one-shot theme/language/lang params once the initializers have read them, so the retry reload's URL carry cannot become a permanent override of stored preferences; the reload URL is built from the live location.href. - export-html build.mjs: the import.meta guard now keeps the warnings (logLevel: 'error') and fails only on reads outside the tolerated prebuilt transcript entry — the previous logOverride: 'silent' discarded them and made the check unreachable. - bounded-map: App.tsx's same-name private copy and the useSessionArtifacts tail migrate to the shared helper, deleting the drifted duplicates (their 'if (!oldest) break' exit would stop evicting on an empty-string key). - tests: the zh-CN x default-reset copy cell; the reload-carry test now stubs location after the last navigation and pins the live session path and workspace in the carried URL; scripts/tests gains an end-to-end case that drives the export build with an injected import.meta read and asserts the guard's failure. Mutation checks: removing the boot strip, reverting the guard to logOverride: silent, mutating the zh-CN retry copy, or building the reload URL from origin each turns the corresponding new case red. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(web-templates): extract the import.meta guard into a tested predicate and stop silencing the build Round 5 found three problems with the round-4 guard wiring: the e2e test drove the real build with an injected probe and left src/export-html/dist wiped on every green run (the build rm -rf's it up front and only repopulates on success), its existsSync bail-out reported a vacuous pass instead of a skip on trees without the prebuilt transcript, the file-path allowlist could not see a fourth import.meta read arriving through that same prebuilt bundle, and logLevel: 'error' suppressed every other warning class the build emits. The guard is now a pure predicate in import-meta-guard.mjs — file allowlist plus a count ratchet (the deliberate guarded ternary accounts for exactly three empty-import-meta warnings; a fourth fails) — called from build.mjs, unit-tested with fabricated warnings. The e2e half keeps driving the real build but snapshots and restores the dist directory, restores the probed file in finally, and uses describe.skipIf so a missing prebuilt transcript reports skipped, not passed. logLevel is gone: warnings print at the default level again while documentBuildResult.warnings stays populated for the guard. Mutation checks: removing the ratchet reds the fourth-read unit case and the transcript-probe e2e; disconnecting the guard call reds both e2e cases; reverting to logOverride: 'silent' reds the document-entry e2e. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-09-10 06:33:33 +00:00
易良	d8baa8730f	fix(core): release node-pty's conout worker after every PTY on Windows (#11313) ... * fix(core): release the ConPTY host after every PTY on Windows On Windows every completed tool call orphaned one headless `conhost.exe` (~8 MB) for the lifetime of the CLI process: #11303 measured 347 of them (~2.8 GB) under a VS Code Companion session after 12 h, growing 1:1 with tool shell commands. `taskkill` (`windowsKillPid`, `performCancelKill`) owns the *shell* process. It cannot reach the pseudo-console host, which is a separate process bound to the HPCON handle this process holds, not a descendant of the shell. Only `ptyProcess.kill()` releases it — it is the sole caller of node-pty's `WindowsPtyAgent.kill()` -> `conptyNative.kill()` -> `ClosePseudoConsole()`. On a natural shell exit node-pty runs `_$onProcessExit`, which only flushes buffered data and destroys its sockets, so the pseudo-console stays open. Three teardown paths dropped the PTY without that call: - `disposeForegroundPtyResources` — the healthy path. Its `isPtyActive` guard correctly skips taskkill when the shell exited cleanly, and that is exactly the per-tool-call leak. - the background-promote settle path — which has already removed the pid from `activePtys`, so the process-exit `cleanup()` cannot reclaim it either. - `WebTerminalRegistry.release()` — skips `killPtyTree` entirely for a session whose shell already exited. Release the host in all three. This is narrower than #5892, not wider: it closes only the HPCON we opened, so unlike a `taskkill /t` it can never reach a third-party application (the #6067 collateral-kill regression). win32-only, deliberately — there is no ConPTY host elsewhere, and `UnixTerminal.kill()` would signal an already-exited, possibly recycled pid. Fixes #11303 * fix(core): release the ConPTY host without killing a recycled pid Follow-up to the previous commit, correcting how the host is released. `ptyProcess.kill()` does three things on Windows, and only two of them are wanted here. Besides `ClosePseudoConsole` and disposing the conout worker, node-pty's `WindowsPtyAgent.kill()` forks a helper to run `GetConsoleProcessList` on the shell pid and then `process.kill()`s every pid it returns. On the healthy path the shell has already exited, so `AttachConsole` throws, the helper dies with an uncaught error, node-pty's 5s timeout falls back to `resolve([shellPid])` — and we `TerminateProcess` a pid `ClosePseudoConsole` just freed for reuse. That is exactly the #6067 collateral-kill failure mode, and the previous commit would have fired it on every tool call. Drive the two teardowns directly instead, extracted to `conpty-host.ts`. The shape is checked before use and the release degrades to a no-op if node-pty's internals ever change, so a dependency bump can only bring the leak back, never a kill we did not intend. Verified unchanged in 1.2.0-beta.15. This also fixes a second leak found while tracing the first: node-pty runs a `worker_threads` Worker per PTY to read the conout pipe, and `ConoutConnection.dispose()` is likewise reachable only from `kill()`. That accounts for the 353 threads the reporter measured against 347 orphaned conhosts — one leaked worker each. `WebTerminalRegistry` now goes through the same helper rather than `pty.kill()`, via a new optional `releaseHost()` on `WebTerminalPty`. `performCancelKill` deliberately keeps using `ptyProcess.kill()`: it early-returns unless the shell is still running, so the console process list is real there and killing it is the intended tree-kill fallback for when taskkill fails to launch. * fix(core): make the ConPTY host release idempotent node-pty's native `PtyKill` looks its baton up by pty id and calls `ClosePseudoConsole` without removing the entry from its handle list, so closing the same pseudo-console twice is a double-free on an already-closed HPCON — undefined behavior in-process, not a catchable throw. The cancel path reaches teardown twice: `performCancelKill` runs `ptyProcess.kill()` (which closes the host itself), and the finalizer then runs `disposeForegroundPtyResources`. Without a guard the release added here would have turned that into a crash. Track released PTYs in a WeakSet, and have the two sites that call node-pty's own `kill()` — `performCancelKill` and the process-exit `windowsStrategy.killPty` — record it, so a later release is a no-op. `performCancelKill` keeps using `kill()` on purpose: it early-returns unless the shell is still running, so node-pty's console-process-list lookup resolves for real there and killing it is the intended fallback for a taskkill that never launched. * test(core): cover the Windows release branch, which CI never reaches `Test (windows-latest)` is skipped on PRs, so the web-terminal release test only ever ran its POSIX half — the win32 assertions were dead code in CI, on the platform the whole fix is about. `web-terminal-registry.test.ts` now steers `os.platform()` (the only thing conpty-host reads; `killPtyTree` branches on `process.platform`, so it is untouched) and asserts both halves: the host and conout worker are released on win32, and nothing is touched elsewhere. The `os` mock passes everything else through, since Storage — reached via debugLogger — needs the real `homedir()`/`tmpdir()`. The shellExecutionService tests already mocked `os.platform`, so those were exercising the win32 paths on Linux runners. * docs(core): stop claiming the cancel-path kill() proves the shell is alive performCancelKill runs `spawnSync(taskkill /f /t)` immediately above this call, by design, so on the normal cancel the shell is already dead when kill() runs and node-pty's console-process-list lookup takes the 5 s `[innerPid]` fallback (windowsPtyAgent._getConsoleProcessList has only a message listener plus that timeout) instead of resolving for real. The comment asserted the opposite invariant, which would mislead the next maintainer on a file already flagged as revert-correlated. Comment only, no behaviour change. kill() stays as the fallback for a taskkill that never launched, and noteConPtyHostReleased stays paired with it so the finalizer does not close the same pseudo-console twice. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtrow8ztpw * test(core): pin the ConPTY release placement and the worker-dispose guard Two coverage gaps in the #11303 release path, both measured green before: - the release in firePostSettle sits above the `!postPromote?.onSettle` early return on purpose, but no test told the two placements apart: sliding the call one statement down kept the whole suite green while every promoted shell whose caller passes postPromote without onSettle leaked a conhost and a conout worker. The existing onData-only test now asserts the release fired. - the try/catch around `_conoutSocketWorker.dispose()` had no test at all. It is load-bearing because of where the release runs: in finalize()'s finally, immediately before activePtys.delete(pid) and after the result already settled. An escaping throw skips that delete and leaves a finished pid registered for the process-exit `taskkill /f /t`, against a pid Windows may have recycled. The new case drives dispose to throw and asserts a following cleanup() does not tree-kill that pid. Both run on Linux (the platform is mocked), so neither is a win32 skip. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtrow8ztpw * fix(core): close the two remaining ConPTY release gaps on Windows Three review findings, all on the release path this PR added: - Attach the post-promote settle listener unconditionally. It was gated on `postPromote`, so a background promote that passed no handlers dropped the pid from activePtys, disposed the foreground exit listener, and left nothing that could ever release the conout worker or the ConPTY host. Only the caller forwarding stays gated: firePostSettle early-returns on `!postPromote?.onSettle` after the reap and the release, so the PR-2 detach-everything contract still holds for callers that did not opt in. Attaching the 'error' listener unconditionally also keeps a post-promote pty error from being emitted on an EventEmitter with no listener. The PR-2.5 compat test now pins 2 onExit registrations and still asserts no data listener and no caller callback. - Stop recording a ConPTY release that never happened on the cancel path. WindowsTerminal.kill() runs its whole teardown through _deferNoArgs, which queues it until `_isReady` — set only on the conout socket's first data byte — so a cancel before the shell's first output byte queued a teardown that may never run while noteConPtyHostReleased permanently suppressed the finalizer's release. The note now sits inside the try, after kill(), and is skipped when the teardown was deferred; reading the optional `_isReady` degrades to the previous behavior if the field is ever renamed. - Pin the activePtys invariant directly in the dispose-throws test instead of only through the process-exit taskkill argv, so the conout dispose guard keeps a witness if windowsStrategy.killPty is ever refactored. Tests: 145 passed in src/services/shellExecutionService.test.ts. Each new or changed assertion was mutation-checked red against the unfixed source (restore of the `if (postPromote)` wrapper, unconditional note, and removal of the dispose guard in conpty-host.ts). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmts86p0gqr * docs(core): stop certifying a ConPTY host close that never happens Review found that `releaseConPtyHost` cannot close the pseudo-console on any of its three call sites, and the finding was confirmed independently against the exact pin. At `@lydell/node-pty` 1.2.0-beta.10, the native exit-watcher thread in `src/win/conpty.cc` erases the pty baton *before* it delivers the JS `onExit`, and `PtyKill` skips `ClosePseudoConsole` when `get_pty_baton` returns null — silently, so not even the warn fires. `struct pty_baton` has no destructor, so the erase leaks the HPCON rather than closing it. Every call site here runs strictly after `onExit`. So the split is: - `_conoutSocketWorker.dispose()` is pure JS and genuinely frees the worker thread. That is the 353-threads half of #11303, and it is real. - `_ptyNative.kill()` is a no-op. The `conhost.exe` half — the dominant term the reporter measured — is NOT fixed. The unit tests cannot see the difference, because `_ptyNative.kill` is a `vi.fn()`: the assertion passes on a call that does nothing on Windows. Comments and names only, no behaviour change. Corrected here: the doc block, both call-site comments, the `releasedHosts` rationale (which after this finding applies only to the cancel path, where the shell is still alive and the baton still exists), the version clearance (the JS field names were re-checked on beta.15, the native semantics were not and did change at beta.14), the warn text, the web-terminal comments, and the test names and describe block that read as certifying the host. The call itself stays: the call site is right, and it starts working unchanged the moment upstream closes the HPCON when the baton is erased. Until then the only mitigation for the host half is `tools.shell.enableInteractiveShell: false`. * fix(core): release the conout worker on the remaining Windows release paths Address the closeout findings on the ConPTY worker release: - conpty-host.ts: stop gating the conout worker dispose behind the native-kill shape check — a node-pty bump that renames _pty/_ptyNative must not also lose the only teardown that frees anything today. Correct the doc block's two overbroad claims (the 'error' entry point can reach a live baton, and enableInteractiveShell only gates the shell-tool path). - web-terminal-registry.ts: release the conout worker on the live branch too. killPtyTree's pty.kill() defers its whole teardown while _isReady is false, so a terminal released before its first output byte stranded the worker. The wrapper now notes the close only when kill() really ran, so release() never double-closes a pseudo-console kill() already closed. - shellExecutionService.ts: fix the settle-path comment (it released only the worker, not the host), name the settle-time taskkill reap the unconditional attach adds, correct the 'error'-listener rationale (node-pty's listeners('error').length < 2 throw), and qualify the three "PR-2 detach-everything" statements that the unconditional attach now falsifies. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtsxwmdgrw * fix(core): format web-terminal-registry.test.ts to satisfy prettier Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-conflict/jmtt2wnugs4 * fix(core): avoid double-close on web-terminal deferred live release The web-terminal live-release path called releaseConPtyHost even when node-pty's kill() was still queued in _deferreds (the shell had not yet emitted its first byte), closing the same HPCON a second time. Route that case through the new disposeConoutWorker so the queued kill() stays the single closer, and correct the two now-false invariants in the comments. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmttf1wj0so * docs(core): reattach the releaseConPtyHost contract block to its function Inserting disposeConoutWorker between the contract block and releaseConPtyHost orphaned it: the block closed with */ and a new /** started on the very next line, so TypeScript resolved the leading trivia to disposeConoutWorker and releaseConPtyHost documented as nothing. The export all three cross-file call sites invoke (shellExecutionService.ts:1871 and :2142, web-terminal-registry.ts:321) had no hover doc. Move disposeConoutWorker and its own JSDoc above the contract block, so the block sits immediately before releaseConPtyHost with a blank line on each side and neither reads as a continuation of the other. The block is relocated verbatim -- the caller enumeration in it is the text a resolved thread already ruled on. Leave a one-line pointer in the disposeConoutWorker doc for the #6067 and win32-only rationale rather than duplicating them. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtu0hicwtk * docs(core): make the releaseConPtyHost caller enumeration match the call graph The three-group enumeration omitted the web-terminal release of an already-exited session and filed the cancel path wholly among the note-suppressed sites. Verified against the real call graph: - release() reaches session.pty.releaseHost?.() from two arms -- :464 for !session.exited and :472 for session.exited -- and both land on the same releaseConPtyHost(spawned) at web-terminal-registry.ts:321. The exited arm runs strictly after onExit, records no note, so it is a member of the no-op group, not the note-suppressed one. It is the primary web-terminal path for #11303. - releaseHost branches only on _isReady (:317), never on session.exited, so the disposeConoutWorker route at :318 is reached from BOTH arms, not just the live one. - performCancelKill records the note only when _isReady !== false (shellExecutionService.ts:2418), so a cancel that lands before the shell's first output byte reaches the finalizer at :1871 with no note. - windowsKillPid's interactive-shell kill at :619-623 notes unconditionally and was unlisted. That is six distinct production states reaching releaseConPtyHost plus two routing to disposeConoutWorker; the previous text named six entries total and mis-classified two of them. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtu0hicwtk * test(core): witness the throw guard in disposeConoutWorker Its twin inside releaseConPtyHost already had a dedicated test, but this one had none: disposeConoutWorker's only call site is web-terminal-registry.ts:318 and the sole test reaching it used a bare vi.fn() for conoutDispose, so deleting the try/catch left the whole suite green. The guard is load-bearing because release() calls session.pty.releaseHost?.() bare (:464/:472), after the session is already deleted from the map (:445), and dispose()'s loop (:422-426) has no per-iteration guard -- an escaping throw would abort teardown for every session still queued behind it. Mutation-verified: with the try/catch deleted, 1 failed | 27 passed; intact, 28 passed. Runs on the Linux lane because the file mocks node:os platform, so this is not a skip-on-CI witness. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtu0hicwtk * docs(core): state both arms of the deferred releaseHost, and test the exited one The deferred branch's rationale was written only for the live release, where killPtyTree has just queued a kill() in node-pty's _deferreds. But release() reaches releaseHost from two arms -- :464 for !session.exited and :472 for session.exited -- and releaseHost branches on _isReady alone, never on session.exited. So a shell that exits before its first output byte (COMSPEC resolving to a binary that quits immediately, or a releaseWorkspace drain racing pwsh startup) lands in the same branch, where no kill() ran, nothing is queued, and the double-close argument does not apply -- the native baton is already erased by the exit-watcher, so a close would no-op rather than double-close. The worker is still the resource to free, which is what the else arm's own comment says it is there for. Split the comment into LIVE and EXITED so each arm carries its real reason. Add the missing case: _isReady = false, then onExit({ exitCode: 0 }), asserting conoutDispose once and kill/nativeKill/spawnSync never. Per conpty-host.ts the test asserts the worker dispose and the ABSENCE of the native kill, never that the conhost half was freed. Mutation-verified: deleting the _isReady === false branch turns the new case red (3 failed | 26 passed); intact, 29 passed. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtu0hicwtk --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>	2026-09-10 06:24:52 +00:00