8699 commits

This branch

This branch

All branches

Author	SHA1	Message	Date
yiliang114	b74c60a787	Merge remote-tracking branch 'origin/main' into fix/9434-hook-ask-diff	2026-08-20 21:37:20 +08:00
jinjing.zzj	57b6b900f8	fix(core): close round-8 ask-bounce findings (#9434) ... - R8-1: pass hookAskReason through the info branch of restrictWorkflowConfirmationDetails, mirroring edit/exec, so bubbled info approvals keep the hook reason in the leader UI (+ sentinel test). - R8-2: the ask bounce no longer awaits resolveDiffFromCli on the critical path (closeDiff is bounded only by the 10-minute IDE RPC timeout and takes no signal); the invalidation is fire-and-forget and an abort re-check precedes the status transition. Epoch invalidation still drops stale round-1 answers (+ regression test with a hung RPC).	2026-08-20 21:30:22 +08:00
Shaojin Wen	a20a724ec0	refactor(review): build the incremental scope from the PR's diff, not a check (#9267) ... * refactor(review): build the incremental scope from the PR's diff, not a check The containment oracle proved, after the fact, that a separately captured `anchor..head` carried no hunk the PR's own `base..head` diff lacked — because a comment anchored on a line GitHub does not display answers 422 and takes the whole all-or-nothing Create Review call with it. That proof was a hand-written match over two rendered unified diffs, and six review rounds each closed the reported entrances while the next found new ones: count-less headers, deletion junctions, lossy UTF-8 decodes that collapse distinct bytes onto U+FFFD, a budget spent across hunks, content matched without position. Every one was the same shape — something the delta carried that the PR's diff did not display, arriving through a gap in the match. The surface is unbounded because it is a match over arbitrary content. So the scope is no longer checked against the PR's diff; it is built from it. The delta is read only for the post-image ranges it touched, and the published text is assembled out of the full capture's own hunks. Every line the review sees is a line GitHub displays, by construction rather than by proof. The oracle goes, and with it the two refusal reasons that existed to report its verdicts: `hunks-outside-pr-diff` and `containment-unverified`. One reason replaces them, and it names a fact rather than a failed proof — `nothing-to-narrow`, when the PR's diff has no hunk left in the range that changed since the anchor. That is the "undo per feedback" round, where the commits since the anchor put lines back the way the base had them: there is genuinely nothing there to re-review, and the round keeps the full range, which is the review it would have done anyway. The invariant is asserted directly against real-git captures rather than argued per shape: every line of the narrowed output must appear in the full capture, checked over the whole output on each scenario, including the undo-per-feedback history that defeated the oracle six times. A mutant that assembles from the delta's bytes instead fails three of the four. Net effect on the tree: -255 lines of production code and the ~765-line battery that existed to pin it, against +29 and a four-case integration suite. * docs(review): retire the oracle's refusal reasons from the skill #9100 has landed, so the paragraphs this would have conflicted with are settled and the deferred half of this PR can go in. SKILL.md's recovery taxonomy still enumerated `hunks-outside-pr-diff` and `containment-unverified` — reasons the report can no longer carry, since the oracle that produced them is gone. It names `nothing-to-narrow` now, and says what actually produces it: an "undo per feedback" revert, which puts lines back the way the base had them so the PR's diff no longer shows that region, and a capture whose bytes do not survive a UTF-8 round trip. The retry classification moves with it. The old sentence said "the containment reasons re-rule identically"; the new one says the narrowing re-narrows identically, and why — the same two captures select the same hunks, and a capture that failed a round trip fails it again. Both remain deterministic for the same sha and outside the retryable set. Pinned in SKILL.test.ts beside its siblings: moving the reason into the retryable set fails that test rather than shipping green. * fix(review): keep narrowed rounds displayed-only across rename, mode, and huge-hunk shapes - Refuse to narrow when a delta path does not cross into the full capture's keys: git's rename detection can resolve differently across the two ranges, and the unmatched section is a displayed change that would silently drop from the published scope while the round still certified head. The round keeps the full range instead. - Emit a full section whole when the delta touches it without hunks: a since-anchor mode change, pure rename, or binary replacement lives in the section header, and the old guard dropped the whole section while reporting `effective: true`. - Weld `incremental.diffBase` to the merge base, not the anchor: the published hunks are byte-identical hunks of `mergeBase..head`, so Agent 7's test-efficacy probe must recompute that range; the anchor range can carry undo hunks the PR's diff does not display at all. - Assemble the narrowed text without spreading selected hunks into a single `push`: a hunk past the ~125k-line argument ceiling crashed the whole fetch-pr round with a RangeError instead of degrading. - Validate the full capture with a fatal UTF-8 decode instead of re-encoding a full-size copy to compare, removing ~N of peak memory on the large long-lived PR workload. Pinned by real-git integration scenarios for each shape, including the deletion-acceptance control and an unconditional null fallback; the producer→consumer weld test now asserts the merge base end to end. Each new test was verified to fail against the pre-round code. * fix(review): close the rename-key divergence in incremental narrowing - Fail closed on a rename the full capture does not key as the SAME rename: when round 1 rewrites a file below git's rename threshold and round 2 renames it, `base..head` nets the chain to an addition plus a deletion while `anchor..head` carries a 100%-similarity rename keyed on the new path. The path guard passed — the new path is in the full capture, as the addition — while the rename's deletion half sat under the old path and dropped from the published scope under `effective: true`. `parseDiff` now exposes `rename from`, and the narrowing join refuses unless both captures key the same rename; the round keeps the full range, which still displays the deletion. - Fatal-decode the delta symmetrically with the full capture: `narrowToDelta` takes the delta's raw bytes and decodes them itself. A lossily pre-decoded delta folded an invalid path byte onto U+FFFD, which could collide with a legitimate U+FFFD path in the full capture and publish an unchanged file's hunks. - Record the executable bit through git itself in the mode scenarios (`git update-index --chmod=...` beside the filesystem chmod): `chmodSync` is invisible to git on Windows — libuv cannot set the exec bit and `core.fileMode` is false — so the two mode tests failed on the Windows merge-queue leg. Verified against a Windows-git model (`core.fileMode=false`): 2 failed before, 16/16 after. - Assert the narrowing outright where the scenarios are constructed to narrow: the null guards on the undo-per-feedback and post-anchor-file tests let an all-or-nothing refusal ship green with zero assertions executed. - Pin the unpinned emission shapes: a whole-file deletion riding the `+0,0` clamp and inclusive `overlaps`, a mode-only full section the delta touches with content hunks, and both hunks of a two-region file surviving the join — each verified red against the corresponding mutant. - Enumerate all four null shapes under `nothing-to-narrow` in the report's union doc, the demotion arm, and the skill's reason bullet; the routing (deterministic, never retried) already held for all four. The rewrite-then-rename regression test fails against the pre-round code (the probe published [new.ts, other.ts] with the deletion absent); the mode scenarios fail under the Windows model before the index-native recording. * fix(review): keep header-level changes in the narrowed incremental scope A delta section whose hunks all miss the full capture used to be dropped whole, taking a post-anchor mode flip or rename out of the published scope while the round still reported effective. Track header-level delta changes per path and emit such sections whole, the hunk-less treatment. Also close the measured battery-power gaps: the rename guard's pass-through arm, rename-plus-hunks emission, a single delta hunk overlapping two full hunks, the multi-file section drop, the capture-failed disk assertion, the diffBase seam's division of labor, and the skill's reason taxonomy plus retryable set. (#9267) * test(review): close the narrowing battery's mutation holes (#9267) The headerTouched miss-branch tests never asserted the section's surviving content hunks, the delta-side UTF-8 refusal test stayed green with its guard removed, and the battery carried no no-trailing-newline marker pin and no binary delta section. Add the missing assertions and the two scenarios, and rebuild the delta-side refusal test around the U+FFFD collision its comment describes, so each shape now fails under the mutant it is meant to catch — all four mutants probed against the battery. Also name what the merge-base clamp actually prevents in the skill's `base-untrusted` clause, whose "those" lost its antecedent when the containment reasons were retired. * fix(review): carry position-divergent hunks through the narrowing join (#9267) Myers aligns a change inside a run of identical lines against whatever surrounds it, and the two captures' old sides differ — so the same post-anchor change can sit at disjoint head-side ranges in `base..head` and `anchor..head`. The range join then dropped a change the PR's diff displays while the round still reported `effective: true`, and the ledger certified head over it — the change never re-entered any later scope. Fail closed per hunk: a missed delta hunk whose changed lines the full section also changed emits the section whole — every line of it is displayed, and clean siblings still narrow. A netted-out undo contributes no line the full section displays, so the deliberate section drop is untouched. Covers both probe-confirmed shapes: the whole-section miss and the partial miss where a matched sibling hunk kept the file visible. * fix(review): fail closed when a delta hunk lacks a corroborating full hunk (#9267) * fix(review): key changed-line corroboration by new-side junction (#9267) * fix(review): narrow between files, not within them Four consecutive rounds reported the same class and each fix was defeated by the next round's entrance: whole-section miss, partial miss, then two shapes defeating the position-divergence guard's conjuncts, then content-only corroboration, then junction-keyed corroboration defeated by a delta hunk carrying two changes. The reviews were right about the cause each time and right about the pattern: matching hunks across the two captures is a heuristic over arbitrary content, which is what the containment oracle this file replaced also was. The two captures are independent Myers alignments over overlapping content, so which HUNK a change lands in is not stable between them — a run of identical lines lets the same edit be attributed to the run's front in one and its back in the other. Every guard here tried to recognise that divergence; none could, because it is a property of the alignment and not of the change. What IS stable is which FILE a change belongs to, and the path and rename guards already fail closed on the one way that could differ. So the unit of narrowing is the file: a section the delta touched is emitted whole, a section it did not touch is dropped. Nothing the delta performed can fall out of a section emitted entire, and the whole position-divergence family stops existing rather than being caught. That failure direction is the reason this could not stay as it was. A dropped hunk left the round reporting `effective: true`, and the ledger then certified head as the next anchor, so the change was never reviewed by any round. The deleted oracle failed toward more review; this failed toward silently less, recorded as complete. Cost, stated plainly: within a touched file the round now reviews all of that file's PR hunks, not only the ones that moved since the anchor. The saving incremental review exists for is the untouched files — a round touching 2 of 40 reviews 2 — and that is unaffected. Also: a base-free round no longer reports `capture-failed`. The fetch succeeded and `git merge-base` found no common ancestor, so nothing threw; naming an infrastructure fault put a deterministic state into the class the recovery flow retries. It reports `nothing-to-narrow`, and the two pins that asserted the old reason move with it. * fix(review): split a base-free round by why the base is missing The R11-1 fix keyed on `mergeBaseSha === null` alone, but that null has two causes and only one is deterministic. A base that could not be FETCHED — a fresh CI clone with no local base ref, hitting a transient fault — is infrastructure: something did fail, and the re-run re-runs exactly the component that failed. Reporting `nothing-to-narrow` there put a retryable state into the never-retried class and pointed operators at "nothing to narrow" instead of a fetch failure. The arms are split by `baseFetchFailed` now: fetch failed keeps `capture-failed`, no-common-ancestor keeps `nothing-to-narrow`. This is the same distinction SKILL.md's recovery paragraph already draws for a planless `partition-failed`, applied where the code makes the same choice. The pin that was supposed to cover this asserted `nothing-to-narrow` over a `{sha: null, baseFetchFailed: true}` fixture while its comment said "the fetch succeeded" — a fixture contradicting its own comment, which is what kept the gap invisible. It is one test over both fixtures now, each with the reason its cause implies. * fix(review): split merge-base surface failures from deterministic refusals (#9267) --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-08-20 12:52:54 +00:00
Shaojin Wen	d0cd5520d8	test(review): single-encode the diff-read fixture and pin the failed-read gate (#9539) ... * test(review): single-encode the diff-read fixture and pin the failed-read gate Two follow-ups to the diff-read pin added in #9484: - The fixture's launch line was `JSON.stringify(...)` before the trailing `.map((r) => JSON.stringify(r))` encoded it a second time, so `parseTranscript` parsed a bare string and dropped it — `launchPrompt` was silently `''`. Make it a plain object literal like its siblings, and assert `launchPrompt` so the encoding can't regress unseen. - Add a FAILED (`response: { error }`) read of the diff to the fixture and assert `diffToolCalls` stays 1 and `diffReads` stays `[[1, 40]]`. Hoisting the counter out of the `!isErrorPart` branch — which otherwise ships green across the whole suite — would credit a denied read as a diff read. * test(review): pin the failed-read gate on the evidence arg lists (#9539)	2026-08-20 12:22:50 +00:00
qqqys	2870eeedf9	feat(web-shell): adopt canonical Goal v3 controls (#9393) ... * feat(web-shell): adopt canonical Goal v3 controls Route WebShell Goal lifecycle through the canonical Goal v3 control plane instead of model-bound chat commands. Goals can be created before the first message, then inspected, edited, paused, resumed, replaced and cleared directly. - core: extend the Goal reducer/runtime and protocol with the v3 control actions and a revisioned snapshot. - serve/acp: expose typed Goal state and controls over the daemon HTTP routes and the ACP bridge, with a shared error taxonomy. - sdk/webui: surface the same revisioned snapshot and control actions so every consumer reads one source of truth. - web-shell: render the active Goal as a compact composer row sized to match the queued-message row, drop the clear confirmation, and hold ordinary messages in a local FIFO queue while a Goal runs — only the explicit Insert action enters the active turn. The TUI keeps its existing presentation and command flow. Token-budget UI and desktop-shell adoption are intentionally out of scope. * test(serve): count the two Goal routes in the telemetry guards The PR registers `POST /session/:id/goal` and `GET /session/:id/goal`, both `handler_resolved`, taking the legacy session telemetry catalog from 59 routes to 61 and the attribution split from 57/2 to 59/2. Three hardcoded totals in telemetry.test.ts and one in the drift guard still asserted the old counts, so the Test job failed even though the guard's real check — registered Express routes equal the catalog — passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(webui): keep the Goal snapshot authoritative across replace, stale reads, and load Three ordering defects let a stale Goal frame win over the daemon's actual state, each of which then flips `holdQueuedPromptsLocally`, the Goal strip, and the manual-run gate off a goal that is not really there. - `selectGoalState` recorded an ordering identity only for a cleared goal, and `goal-runtime` attaches `clearedGoal` to a clear but not to a replace — so a pre-replace frame of the replaced goal passed every guard and reinstalled it over its replacement. Carry a bounded ledger of superseded goal identities (cleared and replaced alike) forward onto each accepted snapshot and reject frames at or behind one. - `getGoal()` reconciled a bare-null READ snapshot against whatever state existed at resolution time. A read the daemon answered while goal-less can land after a concurrent create, and with no `clearedGoal` tombstone it read as "clear whatever is current", wiping the new goal. Stamp the read with the goal observed at issue time: a bare-null response may only clear that goal. - The session-load path installed its snapshot behind a reference-equality guard instead of `selectGoalState`, so a frame arriving inside the load window discarded the authoritative response — and when none arrived, the raw install registered no tombstone and a later stale frame resurrected a cleared goal. Reconcile instead, and keep the synthesized empty snapshot for a failed fetch only while no state is known. Each fix is pinned by a test that fails when the fix is reverted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(webui): pin the known-Goal-state branch of a failed session-load fetch Only the fresh-connection branch (synthesize an idle snapshot) was covered, so a simplification that always synthesizes on rejection would ship green and replace a live goal with idle on a transient `goal()` failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(web-shell): keep Goal controls and drafts owned by the surface that started them Four Goal-surface defects, all of the same shape — state applied to whatever form, latch, or composer exists when an operation settles rather than to the one that started it: - `GoalEditDialog` live-synced its textarea to the `objective` prop, so a concurrent edit from another client (or the refresh a failed save triggers) silently overwrote the user's typed draft — the only copy. Adopt prop refreshes only while the field is still pristine. - `GoalsDialog` left its form dismissible while a submit was in flight, so closing it mid-request handed that request's `resetForm()`/`setFormError` to the next goal's form. Pass `dismissible={!submitting}`, matching `GoalEditDialog`. - ChatPane's `/goal` branch ran before the broken-connection guard applied further down the same `handleSubmit`, so a control typed while the pane was disconnected was consumed, written to the transcript, and then failed with only a toast. Apply the guard inside the branch and keep the text. - ChatPane's goal-control busy latch was session-keyed, so a server-side goal replacement released it mid-operation and a second control could dispatch against the same expected revision (one loses with a 409). Key the latch to the operation and release it only from its owner. Each fix is pinned by a test that fails when the fix is reverted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(web-shell): fail Goal gates closed while hydrating, and stop losing held prompts - The `goalSnapshotRef` gates (`promptBlocked`, retry, language change, fast-model select) failed OPEN while `connection.goalState` was still hydrating: the session load clears `loadingTranscript` before its `goal()` fetch resolves, so a prompt typed in that window was submitted straight into a Goal the client had not learned about — while the queue-hold gate, `enqueueManualRun` and `tryFireBoundRun` all failed closed on the identical state. They now share one `isGoalGateBlocked()` predicate that treats an unknown Goal state on a real session as blocked. - Creating a Goal in an allocated session went through the workspace-scoped control, which — unlike `sessionActions.controlGoal` — never wrote `connection.goalState`; the only compensation was a conditional re-sync one round trip later. Until it landed the hold gate read false and no Goal strip rendered. A new `applyGoalSnapshot` session action installs the create response into the connection state, reconciled like any other snapshot. - Locally held Goal prompts were stashed under `(workspaceCwd, sessionId)` and could only be relocated when the session just left was the same one, so a workspace resolving while the user was on another session orphaned the stash under a key nothing looks up again — silently losing typed text. Any stash whose session half matches is now relocated and restored in queue order. Each fix is pinned by a test that fails when the fix is reverted. The App test harness now defaults to a hydrated (goal-less) snapshot, matching a loaded session; the tests that exercise the hydration window set it back to undefined. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(acp): publish turn_complete when a Goal turn ends Goal turns are driven inside the ACP child via its own prompt() call, so the daemon bridge never sees a session/prompt RPC boundary for them and published no turn terminal on SSE. Web Shell (and any SSE client) only settles its streaming state on turn_complete/turn_error/prompt_cancelled, so after a Goal turn the spinner spun forever and locally queued messages never drained — even though the daemon itself was already idle. The child now sends the existing `_qwencode/end_turn` notification with `source: 'goal'` and a turn-scoped promptId when a Goal turn settles, and the bridge translates that into a real `turn_complete` frame — the same contract the Web Shell goal e2e mock already assumed. * fix(web-shell): serialize held-prompt release, drop the dead insert abort registry - Releasing several locally held Goal prompts fired every submission at once, so a prompt awaiting media uploads could be overtaken by a later plain one and reach the daemon's queue out of order. `submitPendingPrompt` now returns its admission promise and the release chains them; the first release stays synchronous. - `explicitInsertAbortControllersRef` was populated and cleaned up but never read: nothing aborted its controllers, so the signal plumbing and the `abort.signal.aborted` recovery branches were unreachable. An explicit insert is meant to outlive an owner rotation and settle into the queue of the session it was started from (two tests pin exactly that), so the registry, the signal and the dead branches are gone rather than given a consumer. - Both mount sites rendered the Insert button enabled while streaming was idle and a Goal was active — precisely the state `insertQueuedPrompt` no-ops in. `canInsertMidTurn` now tracks the hook: the affordance appears only while a turn is running. Test coverage the review probes found missing, each verified by reverting the line it gates: the images and slash-command insert guards, the `isInserting` reset on both settle-unaccepted paths, the release order, the Insert affordance, and the held-prompt stash handoff (a prompt already dealt with must not come back from a stale owner key). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(web-shell): localize Goal command errors and stop Goal surfaces swallowing input - `parseWebShellGoalCommand` returned an English sentence that `formatError` prefers over any localized fallback. It now returns the offending keyword and both composers render `goals.error.requiresObjective` from the dictionaries; the manual-run rejection gets `scheduledTasks.error.goalActive` for the same reason. - `handleGoalSlashCommand` returned true — wiping the composer — before the preconditions it checks asynchronously held, so a `/goal clear` typed without a session lost its text to a toast. The cheap preconditions are checked first and refuse the submit instead. - `enqueueManualRun` treated an unknown Goal state as "Goal is active", which also fires when no session is attached and no Goal can exist, so Run now on a fresh workspace always failed. It shares the hydration-aware gate now. - ChatPane consulted the host slash handler after its `/goal` intercept, so a host override applied in the main composer but not in a pane; and a bare `/goal` in a pane without a Goals view consumed the text silently. The pane now matches the composer's ordering and refuses what it cannot open. - GoalsDialog offered Edit on a completed Goal, which the reducer rejects, and fell back to the stale snapshot when the edited session left the list — turning the friendly "no longer available" path into a raw conflict error. - The versioned control request is built by one shared helper instead of two copies that had already drifted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(web-shell): retire the dead Goal footer paths and restore the stranded-session rationale - `GOAL_STATUS_ACTIVE_EVENT` lost its only production listener in this PR while `GoalStatusMessage` kept dispatching it and two test files kept asserting the dispatch — a contract that looks live but is dead. The dispatch, the `activateFooter` prop, the `isLatest` plumbing that fed it, and the assertions are gone. - The built-in StatusBar now received `activeGoal={null}` permanently, so its goal pill and `onOpenGoals` wiring could never fire while StatusBar.test.tsx kept them green. The pill, its props, its elapsed-time ticker and that pill-only test file are removed; the composer status stack is the goal surface. Custom footers keep their `activeGoal` prop, which is still fed. - `/language ui` skipped its daemon sync when `promptBlocked`, silently switching the chrome while the agent kept answering in the old language for the rest of a Goal run. It now refuses with the same feedback the language picker gives for the identical condition. - The `@container` block in GoalStatusStrip styled `.root`, which cannot match its own container query, so half the responsive rule never applied. Trimmed to the descendant rules that do, with the reason recorded. - Restored the rationale comment above `strandedGoalSessionRef` — the mechanism it documents is unchanged and still live. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(sdk): expose limitKind, drop the unused expected-version type, cover the Goal wire seams - `GoalRecord` in the SDK omitted `limitKind`, the field that decides whether a stopped Goal can be resumed. The daemon sends it, the webui mapper now parses it (rejecting unknown values), and both Resume controls hide for an evidence-limited Goal instead of offering a click that always 409s. - `GoalExpectedVersion` was added to the published SDK with zero read sites — the request type inlines the two fields — so it is removed before release turns it into a shape we must support forever. - Tests for the wire seams the review found uncovered, each verified by reverting the line it gates: the goal error-kind → HTTP mapping and its `current` forwarding, `GET /goals` listing paused/blocked/usage_limited and filtering complete, the untrusted-workspace gate for every work-expanding action (not just create), and `bridge.controlSessionGoal`'s `{ request }` envelope — the only producer of the shape the agent's handler reads. - acpAgent's core mock now carries the real `GoalConflictError` / `GoalInvalidTransitionError`, without which every `instanceof` branch in `mapGoalControlError` throws before it can be asserted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(web-shell): close the Goal coverage gaps the review probes found Each test below fails when the line it gates is reverted (probe-verified): - App: the direct-submission `onSubmitBefore` rejection keeps the draft; a session-less stranded session is forgotten when the user leaves the Goals page; a create after a successful one starts a fresh session; the allocated create is dispatched only after the allocation completes; the Goal edit dialog closes when the same session's goal is replaced; a stale edit resolution does not install its snapshot over the session now displayed; the control busy latch stays owned by the session that started it; and the allocated-session create is observed through the strip the App renders rather than through the value the test's own mock wrote. - ChatPane: the control request is built from the freshly fetched Goal (both the pause payload and the `/goal set` → replace mapping); the edit dialog closes when the pane's goal changes; the "no longer available" message is pinned instead of `expect.any(Error)`; and a control dropped because the pane moved to another session no longer reports a failure toast — matching how the edit-save and main-composer paths already treat that race. - e2e/mock: the mock's `GET /session/:id/status` returns the flat `DaemonSessionSummary` the daemon really sends (the envelope left every field the client reads undefined), its clear response carries the `clearedGoal` tombstone so the anti-resurrection path is reachable, and the first Goal scenario re-checks that no prompt was sent after the create resolves. - Restored the one-shot run-now rationale comments in ScheduledTasksDialog and its tests; the behavior they explain is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(web-shell): drop the imports the retired Goal paths stranded `ed422f9a6c` removed the dead `GOAL_STATUS_ACTIVE_EVENT` contract and the StatusBar goal pill together with the tests that covered them, but left four imports behind with no remaining reference. `lint:ci` runs eslint with `--max-warnings 0`, so `@typescript-eslint/no-unused-vars` failed the required `Test (ubuntu-latest, Node 22.x)` gate on this head. - `StatusBar.tsx`: `useEffect`/`useState` fed only the goal pill's elapsed-time ticker, which went with the pill. - `SystemMessage.test.tsx`: `TranscriptRenderModeProvider` and `serializeGoalStatusMessage` were used only by the removed "goal status activation" describe block. No behavior change and no test weakened — the covered production paths were deleted in the same commit that stranded these imports. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(acp): surface Goal-turn activity in hasActivePrompt summaries Goal turns never flip the bridge's promptActive flag because they run inside the ACP child without a session/prompt RPC, so live-state reported hasActivePrompt: false while a Goal was executing and the Web Shell sidebar showed no activity indicator for that session. The child now sends `_qwencode/start_turn` (source `goal`) when a Goal turn begins; the bridge tracks it as a separate goalTurnActive flag and ORs it into every hasActivePrompt summary. The existing goal end_turn signal clears the flag, the start/end pair now lives in the goal queue drain so a prompt that rejects early cannot leak the flag, and an RPC prompt start self-heals a stale flag since the child serializes the two. * fix(acp): let a mid-turn insert reach a running Goal turn A Goal turn runs inside the ACP child via `prompt()` directly, so the bridge never sees a `session/prompt` RPC for it and `pendingPromptCount` stays 0 for its whole duration. `POST /session/:id/mid-turn-message` passes `rejectIfIdle: true` and the admission gate reads only that counter, so every explicit Insert during a Goal turn was answered `accepted: false` — while the Web Shell enables the affordance precisely because `c9597f6e` made a Goal turn non-idle in `hasActivePrompt`. The client returned the row to its hold and reported `insertFailed`; the media variant deleted the uploaded attachments with it. The e2e mock answers `accepted: true` unconditionally, so nothing caught it. The session is genuinely busy during a Goal turn — the child drains this same queue between tool batches from inside it — so admission now treats `goalTurnActive` as busy and the message is queued for that drain. A Goal turn owns no prompt slot, so nothing settled what its last drain missed: the goal `end_turn` signal now closes that window the way the prompt terminal already does (`queueOnly` callers get `onSettledWithoutDrain`, everything else is promoted). Promotion is the supported path while a Goal is still active — the child's `claimGoalTurn` makes the promoted prompt wait for the permit and run as the next Goal turn. Both tests fail when their line is reverted (probe-verified). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(webui): stamp the session-load Goal read the way `getGoal` does The R1-10 fix landed only in `getGoal()`: the session-load path still routed a fulfilled bare-null `goal()` read straight through `selectGoalState`. The load issues that read while the session is goal-less, so the answer carries no `clearedGoal` tombstone — and `selectGoalState` then derives the clear target from whatever the store holds when it is applied. A goal created inside the load window (the Web Shell allocates a session, then creates the goal on it, and the load's `Promise.allSettled` is gated on its slowest sibling request) was therefore accepted as the thing being cleared, and its identity was written into `clearedGoalOrder`. From there `isSupersededGoalFrame`'s `<=` tie rejected every later frame of that goal at the same revision — including the `refreshGoal()` that follows the create. The daemon held a live goal while the store reported goal-less, `holdQueuedPromptsLocally` read false so ordinary prompts bypassed the Goal queue, and the tombstone survived reloads. `getGoal`'s guard moves into a shared `selectGoalStateFromRead`, and the load path stamps its read with `goalStateAtLoadStart` — the goal it observed when the read was issued, which it already captures. The provider test mirrors actions.test.ts's stale-read case and fails when the stamp is replaced by an apply-time read (probe-verified). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(web-shell): close two Goal-queue leaks in the composer and queue Two Goal-queue leaks the review found on this head. `ChatPane`'s composer submit read `goalState?.goal?.status !== 'active'`, which is TRUE while the snapshot is still hydrating — the session load clears `loadingTranscript` before its `goal()` resolves, and the daemon has no server-side prompt gate for an active Goal, so a prompt typed in that window went straight to the daemon and bypassed the queue. Every other gate in the client already fails closed on the same window. The predicate now lives in `utils/goalGate`, shared by App's `isGoalGateBlocked` and both of ChatPane's gates, so they cannot drift apart again. The pane fixture gains the Goal snapshot a loaded session always carries — without it the fixture models a hydrating session, not a Goal-less one. `useQueuedPrompts` captured the stash owner key once when an explicit insert started, but the workspace half of that key can resolve mid-flight and the owner-change effect relocates the whole stash onto the new key, deleting the old one. The accepted `midTurnState: 'queued'` write then hit a key nobody holds and was silently dropped: the row came back from the stash still `isInserting`, and release/edit/delete/clear all skip such a row — bricked until a reload. The key is now resolved at use time by the session half (the same uniqueness invariant the relocation itself relies on), and the four inline owner-match copies collapse into one helper that compares that half rather than the whole key. Both new tests fail when the line they gate is reverted (probe-verified). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(web-shell): re-check the Goal hold on every release-chain link The chain that drains locally-held prompts is built synchronously when the hold lifts, but each link runs only after the previous admission settles, and `submitPendingPrompt` never consults the hold. Pausing a Goal (which starts the drain) and resuming it inside the chain-length x admission-latency window left the remaining links POSTing against an active Goal — the queue emptied against the user's change of mind, which is the contract this PR exists to hold. Each link now re-checks the hold (and the write block) and returns its row to held instead of sending it; the next inactive transition re-drains in order. The revert is inline rather than through `setQueuedPromptFlags` because that callback is declared below this effect, so naming it in the dep array would read it before its initializer. Also pins `mapGoalControlError` at the ext-method layer, which had no coverage: `sessionGoalControl`'s only tests were the success path and the untrusted gate, and the gate throws before the mapping is reachable. The new case drives real `GoalConflictError` / `GoalInvalidTransitionError` / persistence rejections through `extMethod` and asserts the code plus intact `data.errorKind` and `data.current` — the payload the client's 409 resync depends on. Both fail when the line they gate is reverted (probe-verified). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(web-shell): stamp GoalsDialog's CAS fields through the shared builder `versionedRequest` was a private second writer of `expectedGoalId` / `expectedRevision` — the exact drift `buildGoalControlRequest` was introduced to prevent. The dialog's two call sites now go through the utility; behaviour is unchanged for them (both always hold a goal, so the builder's `goalUnavailable` guard is unreachable there, and `edit` falls back to the goal's own objective as before). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(web-shell): stop offering Goal controls the reducer will reject Clears the three Critical findings from round 2 of the review on #9393. R2-1 / R2-2 — both resume gates (`GoalStatusStrip.tsx`, `GoalsDialog.tsx`) keyed off `goal.limitKind === undefined` alone, while core's `isEvidenceLimited` (`goal-reducer.ts`) also treats a Goal as evidence-limited when `lastReason` is one of the two sentinel strings. Those sentinels shipped before the `limitKind` field did, so a Goal persisted in that window restores as `usage_limited` with no `limitKind` at all: the UI offered a Resume button that `reduceGoalControl` is guaranteed to answer with `GoalInvalidTransitionError`. Both gates now go through one `canResumeGoal` in `utils/goalGate.ts` — the module whose documented invariant is that every client Goal gate runs through a shared predicate so none can drift. It states the reducer's rule directly (complete/active refuse; `usage_limited` refuses when evidence-limited) rather than approximating it, and it keeps the evidence check scoped to `usage_limited` exactly as the reducer does, so a `paused` Goal carrying stale sentinel prose is not stranded without a Resume control. The Web Shell client bundles for the browser and cannot import `@qwen-code/qwen-code-core`, so the two sentinel strings are duplicated. A comment asking the next person to keep them in sync is not a mechanism: `goalGate.test.ts` reads `packages/core/src/goals/goal-protocol.ts` and fails if either literal moves, or if core grows a third sentinel branch. R2-3 — a session switch mid-drain silently destroyed queued prompts. The serial release chain stamps the whole batch `serverState: 'submitting'` up front and then releases it one link at a time (a prompt carrying media awaits its uploads, so that window is seconds, not microseconds). Two things went wrong inside it: the chain carried no owner token, so remaining links called `submitPrompt` against the old session, which throws before POSTing and was swallowed by the chain's own `.catch`; and the owner-change stash only saves rows matching `isLocallyHeldPrompt` (`serverState === undefined`), so every chain-marked row was excluded from it. Both typed prompts were gone with nothing restored to the editor. Each link now bails when the owner token it was built for is no longer current, and a new `unreleasedPromptIdsRef` tracks exactly the rows the chain stamped but never handed to `submitPendingPrompt`. The owner-change effect stashes those alongside the genuinely held rows, dropping the optimistic stamp on the way in so the next drain re-releases them in order. Deliberately narrower than the finding's suggested fix on one point: the row whose admission is actually in flight is NOT stashed. Restoring it would flip the behaviour pinned by 'ignores an old submit response after an S1 to S2 to S1 owner change' and 'fences an old submit before the replacement owner rerenders', which deliberately fence and drop an in-flight submission across an owner change — that POST may well have landed, so resurrecting the row risks a duplicate. The rows the chain never POSTed have no such ambiguity: nothing exists for them on the daemon, so stashing them cannot duplicate anything. Reversing the in-flight contract is a design call for the reviewer, not a drive-by. Also fixes R2-12 in passing, since the new tests need it: `MockGoal`'s snapshot `status` union omitted `'complete'` (a TS2322 against this file's own later usage) and had no `limitKind`. Verification: `npx vitest run client/hooks/ client/utils/ client/components/GoalStatusStrip.test.tsx client/components/dialogs/GoalsDialog.test.tsx client/components/ChatPane.test.tsx` in `packages/web-shell` — 1011 passed (994 on the stashed tree, so all 17 new tests pass), with an identical 17 pre-existing failures in the untouched `hooks/useMessages.test.ts` on both trees. eslint clean on all eight touched files. Every fix is mutation-verified. Dropping the sentinel fallback, widening it to any `lastReason`, or unscoping it from `usage_limited` each turns a distinct test red at both the unit and the component level; removing the chain owner guard, the unreleased-row half of the stash filter, or the stamp reset each turns the new mid-drain test red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(web-shell): stop Goal controls and the release chain racing each other Four round-3 Criticals, each reproduced first and then mutation-verified. R3-4 / R3-19 — `goalControlBusy` was a shared latch with no owner. A create in an allocated session released it unconditionally in `finally`, and the composer `/goal` path never consulted it at all, so two controls could read the same snapshot, stamp the same expectedGoalId/expectedRevision, and let the daemon reject the loser with a 409. Ported ChatPane's twin pattern — `goalControlOpSeqRef`/`goalControlOwnerRef`, released only by the operation that still owns it — to both `controlCurrentGoal` and `createGoalForAllocatedSession`, and made `handleGoalSlashCommand` refuse (keeping the composer text) while a control is in flight. R3-24 — a chain link deleted its id from `unreleasedPromptIdsRef` before the owner check. The owner token is replaced in the render body while the stash is a passive effect flushed after commit, so a link firing in that window left a row that was neither locally held nor unreleased, and the stash dropped it: the typed prompt vanished with no POST, no abort, no toast. Delete moved below the owner check, as the review suggested. R3-20 — a queue clear mid-drain aborts only the in-flight link's controller; the chain's pending links have no controller yet, so they went on to POST prompts the user had explicitly cleared. The link now bails when its row has left the queue. Verification: 775 passing across App / useQueuedPrompts.dom / useQueuedPrompts.midTurnReconcile / ChatPane / GoalStatusStrip / GoalsDialog; full web-shell 3906 passing with the same 18 failures the branch has without this change (useMessages background-agent reconciliation, build-artifact — confirmed by stash-and-rerun). tsc unchanged at 66 pre-existing errors from unbuilt workspace deps. ESLint and Prettier clean on the touched files. Mutation-verified: each of the four fixes reverted in isolation turns exactly its own new test red. * test(cli): align merged replay expectations * fix(web-shell): keep a prompt typed mid-drain behind the release chain R3-2: the serial release chain preserved order only inside the batch it drained. It exists because the prompt at its head may await media uploads for seconds; a prompt typed inside that window went straight through `submitPendingPrompt`, so the daemon admitted it ahead of the older held rows it was typed after -- and while link 1's upload was still running it could overtake link 1 itself and start the turn. The chain is now published on `releaseChainRef` (owner-pinned, retired once its newest tail settles), and `enqueuePrompt`'s ordinary path appends to that tail instead of POSTing past it. The waiting row is registered in `unreleasedPromptIdsRef`, so it is stamped-but-not-POSTed exactly like the chain's own undrained rows and the owner-change stash saves its text rather than losing it. The per-link guards (owner pinned at build time, row-still-present, hold/write-block revert) move into a shared `releaseChainedPrompt` so both the drain and the appended send carry identical semantics. A re-drain for a live owner now extends the existing chain instead of racing it. Test: `holds a prompt typed mid-drain behind the release chain` -- two held prompts with link 1's admission hung, a third typed mid-drain; asserts it does not POST while the chain is in flight and that final order is ['first with media', 'second plain', 'typed during drain']. Mutation-verified: disabling the append arm turns the test red with the reported symptom (the mid-drain prompt POSTs immediately, order inverted). Verification: `npx tsc --noEmit` clean; `npx vitest run` in packages/web-shell -> 190 files / 3986 tests passed. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com>	2026-08-20 12:19:05 +00:00
callmeYe	4839935e55	feat: register toggle-only Qwen reasoning (#9574)	2026-08-20 11:31:35 +00:00
ytahdn	bf0dbb1ae1	feat(web-shell): support mid-turn file attachments (#9570) ... Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com>	2026-08-20 10:58:12 +00:00
Shaojin Wen	5832c49035	feat(review): rule on contract documentation, and matrix layered guards (#9448) ... * feat(review): rule on contract documentation, and matrix layered guards Add a path-scoped checklist for consumer-facing contract documentation — wire protocols, API references, SDK guides — that reads the prose a diff adds and asks whether the code makes it true. Every existing dimension reads the code and asks whether the code is right; the documentation item that already exists asks whether a doc is present. Nothing checked the document a change ships against the behaviour it ships, and for a wire contract that document is what an integrator builds against without ever seeing the implementation. The rule is deliberately narrow. Proposals, plans and RFCs are excluded because describing behaviour the tree does not have yet is the point of the genre; user guides and changelogs are excluded because a rule that fires on every documentation change is a rule that gets skimmed. Only reference material an outside consumer builds against is governed, only prose the diff adds or falsifies is in scope, and wording, silence and roadmap text are never findings. Also correct the deep-verification mutation matrix for defence in depth. A one-row-per-guard matrix cannot see two hunks that close one hazard from different directions: reverting either alone leaves the other holding the line, so both rows read as survivors and the report claims two coverage gaps that do not exist. The matrix now carries a combination row for such a set and a third survivor class for the guards it explains. The positive control a survivor already owed is tightened to the file it was measured in — a control that turns a test red somewhere else proves the runner runs, not that the chosen command collects anything exercising the mutated file. * fix(review): take contract-doc governance from location, not from a name Review found the filename branch both unbounded and unbounding. It backtracked quadratically — a failing extension scan once per keyword occurrence, 403 ms on a 96 kB path git accepts, synchronously inside every agent-brief build, which is the shape the Java checklist in the same file grades a denial-of-service hole. And because a keyword in a name fires anywhere in the tree, it also governed a user guide, a changelog entry and a flat-layout RFC, each of which would then have been read under a checklist whose blockers are Critical. Adding exclusions did not converge; every audit round found another form. Both problems have one cause, so both get one fix: governance now comes only from where a document lives — a reference section or an SDK package — and the keyword branch is gone. The genre exclusion is built once from a shared list and applied in both its directory and its filename form, so a changelog directory and an `rfc-0001.md` are excluded by the same rule that already excluded `rfcs/` and `CHANGELOG.md`. The recall this costs is deliberate and now stated as a test: a wire reference kept outside a documentation or SDK tree is not governed, and a project that wants the checklist there has its own review rules. Every alternative of both the location branches and the genre list is pinned in the matcher table in both spellings, the changelog exclusion is pinned by paths where it is the deciding clause rather than by ones no branch reaches anyway, and the timing block gains four arms that end in a documentation extension — every existing arm short-circuits at the extension gate, which is how an unbounded matcher shipped past it. * fix(review): make the contract-doc pins and timing arms measure what they name Round two found the arms and the completeness claims weaker than they read. The two timing arms matched their target expression at the first anchor, so neither ever reached the failing scan it was named for: two quadratic mutants — one in the genre exclusion, one in the location branch, both behaviour-identical on newline-free paths, so no behavioural test could see them — shipped with the whole suite green. Every arm is now a near miss that matches the prefix at each anchor and fails on the last character, and both mutants die on them. The genre-silence test had the same shape one level up: none of its fixtures sat inside a governed location, so location alone silenced them and the test stayed green with the entire genre exclusion deleted. It now carries fixtures where the exclusion is the deciding clause. The matcher itself was asymmetric or under-pinned on eight axes that each shipped green as a one-edit narrowing — section-name anchoring, the SDK suffix quantifier and its character class, two separator members, the shared genre list, and the case-insensitivity of three separate expressions. Each now has a row that flips. Four behaviour changes came with them. Section names are plural-tolerant on all four words rather than two. A genre is a whole token wherever it sits, so a numbered or dated genre filename is the genre its name says it is, while a word that merely begins with those letters is not. Per-version release notes join changelogs as history rather than contract — this repository ships one inside a governed location. The markdown extension family is admitted in full, since governance is by location and not by which spelling a file uses. Two entrances that contradicted the location rationale are closed: the SDK branch is anchored to a package root, because an `sdk` segment at any depth also governed user guides and fixture trees, and a closed set of repository-meta and agent-context filenames is excluded whole. What remains — non-contract prose inside a reference tree that no closed set describes — is accepted and pinned rather than enumerated: measured here, 44 of the 55 tracked documents under the developer tree are integrator references, and deciding document kind inside a reference tree is the shape that took the keyword branch out last round. * fix(review): pin the contract-doc members and boundaries that flip nothing Round three found the completeness claim still short in three directions, each a one-edit change that moved real paths with the whole suite green. Members: the genre list's newest entry was pinned only in the singular, and seven of the eleven repository-meta filenames plus the SDK branch's `libs` prefix had no row at all. Every alternative now has one, in both numbers where the expression allows both. Boundaries: the trailing separator that makes each location branch match a section directory was pinned by nothing, so dropping it revived governance by filename — the entrance the keyword branch was withdrawn for two rounds ago, returning through the terminator rather than the alternation. Rows now pin both sides. The same was true of the SDK branch's case-insensitivity and of the space in the genre separator class. Ownership: the section branches matched at any depth, which is right for a package-local reference section and wrong for a vendored library or a doc-shaped fixture that carries one. Anchoring the branch would have dropped both, so the distinction is drawn where it belongs — a closed set of trees whose prose the diff's author does not own, alongside the meta filenames it sits next to. Findings against third-party documentation are noise the author cannot act on. Also taken, though the review deferred them as non-blocking: the assertion on the diff-scoped blocker stopped one word short of the negation it exists to pin, and two of the five never-a-finding bullets had no assertion at all. * fix(review): drop the contract-doc table row the ownership block duplicated * fix(review): exclude separator-less thirdparty trees, and pin the NOT_OURS set --------- Co-authored-by: wenshao <nigolaschao777@gmail.com>	2026-08-20 10:56:18 +00:00
ytahdn	5f3165f17e	fix(daemon): avoid custom attachment upload header (#9567) ... Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com>	2026-08-20 09:18:45 +00:00
jinye	50d2700797	fix(web-shell): Stop repeated session title catalog refreshes (#9563) ... Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-08-20 09:03:32 +00:00
Shaojin Wen	313f191150	fix(autofix): make the brake's BLOCKED handoff a first-class round outcome (#9297) ... * fix(autofix): make the brake's BLOCKED handoff a first-class round outcome When the growth brake fires, feedback.md tells the address agent to stop BLOCKED with a handoff — but the output contract only accepted address-summary.md or no-action.md, so a round that followed the instruction died as 'finished without required output file(s)', the brake's decision text was buried under a generic failure.md, the report said 'could not produce a passing fix', and the job left a red review-address check that the next scan counts as new feedback. Observed on #9222 rounds 6/7. The handoff becomes a first-class verdict end to end: run-agent.mjs honors an agent-written handoff.md (with no fix verdict) as a graceful exit the way it already honors failure.md, and shields it from the API-error retry reclassification; the verification gate reports outcome=handoff for a no-commit round with a handoff and no failure.md; finalize lets handoff pass without failing the job; the report step runs for this outcome, posts the handoff note with the eval marker (watermark advances — the feedback is consumed as evaluated), and names the stop honestly instead of reporting it as a failed fix. The skill now tells the agent exactly which file to write when the brake fires. A coexisting spec output still outranks the handoff, and failure.md coexistence keeps the failed classification, so crash paths are unchanged. * fix(autofix): align the handoff outcome's consumers and pins with its contract (#9297) Review found the new handoff outcome breaking two pinned helper tests (stale breaker-headline wording, unclassified headline in the fleet-shepherd contract test), misreporting handoff rounds in the status-comment finalize step, and leaving the whole handoff chain unpinned against mutation. - Update the breaker headline pin to the PR's reworded headline. - Classify the handoff headline as transient in the shepherd contract test and drop its "AutoFix stopped" prefix so the shepherd's terminal-only REASON regex cannot capture a transient stop (the shepherd workflow itself stays outside this round's footprint). - Include handoff in the Finalize-status published-report branch. - Give deliberate stops their own takeover-digest census bucket instead of the residual crash/infra bucket (EN + ZH). - Neutralize :: workflow commands at the two new handoff echo sites. - Use the runner's non-empty missing() convention for handoff.md so an empty file cannot read as a verdict in one layer and not the other. - Correct the run-agent.mjs precedence comment: when a handoff coexists with a spec output, the gate (handoff branch first) decides the round, matching the documented "handoff + no-action -> handoff" contract. - Pin the handoff chain where its siblings are pinned: finalize replay, POST_HANDOFF replay, mark/headline replays, the gate's no-commit decision table, the stub-runner handoff/empty/API-error cases, the report-step if-clause, and the census needle-to-emit cross-pins. * fix(autofix): classify a no-commit handoff before the gate's structural checks (#9297) Review proved the new handoff classification unreachable exactly where the brake fires: the structural pre-checks (core rebuild, settings schema, contracts) judge the PR's own diff and reject before the no-commit fork, and the growth brake fires on precisely the red PRs whose diff trips them. A compliant handoff (no commit, only handoff.md) then classified as a retryable failure, so the repair pass deleted handoff.md and could commit against the brake's explicit stop — the self-feeding loop the handoff exists to prevent. Reproduced with the real gate script: schema-check-fail + no-commit handoff exited 1 with no outcome=handoff. Move the no-commit handoff classification above the structural checks (right after the failure.md exits, which keep their precedence). A handoff claims nothing — acted=false, deferred to a human — so the checks' false-no-action rationale does not apply, and the retryable/ repair machinery must never engage on a round the brake told to stop. The no-op fork reverts to no-action-only classification. - Add a gate test: stale schema + no commit + handoff.md classifies outcome=handoff, exit 0, no retryable (fails on the pre-fix gate). - Pin the handoff-note :: workflow-command neutralization in both layers (the gate's sed and the runner's replaceAll), which review showed were surviving mutations. * fix(autofix): reject a no-commit handoff written over a dirty workspace (#9297) * fix(autofix): report a dirty-handoff rejection honestly, not as a failed fix (#9297) * fix(autofix): reject a handoff written beside a round commit, non-retryably (#9297) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ci): classify the committed handoff shape as its own non-retryable outcome R7-1 on this PR: a round that HAS a commit beside handoff.md skipped both brake-violation guards (clean tree misses the dirty guard; committed ref misses the no-commit branch) and fell through to the structural checks, where reject_fix defaults to retryable and the repair pass deletes handoff.md and may commit again against the brake's stop. Classify it before the structural checks under its own outcome committed_handoff, sibling of dirty_handoff: non-retryable, its own honest report headline (reusing dirty_handoff's wording would claim nothing was committed when a commit exists), listed among the report-publishing outcomes in the status classifier, and never routed through finalize's pass list. Pins updated in the same pass: the committedWithHandoff gate case now expects committed_handoff with no retryable, the shepherd contract test classifies the new headline as transient (loop stays engaged), the status-classifier pin names all five outcomes, and the handoff-contract gate test gets an explicit subprocess budget (eight fixture arms outgrew the 5s default). * fix(ci): count committed-handoff rounds in the milestone census rejected bucket (#9297) * fix(autofix): publish brake violations green and preserve handoffs across crashes (#9297) Two Critical review findings on the handoff output contract. Brake-violation rounds (dirty_handoff / committed_handoff) ended with a red review-address check: the eval marker stamps ts=NEWEST, strictly before the check completes, and the scan counts failed checks completed after the watermark — including this workflow's own review-address checks — as new feedback. The next scan re-selected the PR and burned a full agent round on the item the posted headline promised not to retry, once per violation. Admit both outcomes to the green finalize arm the way the clean handoff already is (the diff's own comment names this self-feeding loop as the reason handoff went green), and key the report step's routing and POST_HANDOFF trigger on the outcomes themselves so the green rounds still publish their honest headline, handoff note, and eval marker instead of going silent. A crash, budget kill, or loop guard after the agent wrote handoff.md synthesized a failure.md that shadowed the note: the gate reads failure.md first (outcome=failed), the report preferred it, and the timeout sentinel re-handed the item the brake stopped. Preserve the agent-written handoff in the crash branch (exit 0, mirroring the agent-written-failure.md arm), and never let writeHandoff overwrite a non-empty agent verdict. Both findings reproduced against this commit's verbatim code before fixing: the case/jq replay showed the violation check red and counted as new feedback, and a stub run showed the synthesized failure.md shadowing the handoff. New behavioral tests fail pre-fix and pass post-fix. * test(ci): give four subprocess-heavy replays explicit budgets The milestone digest, stale-duplicate revalidation, deny-by-default footprint, and recoverable-API-render tests spawn multiple bash replays of the real workflow/gate scripts each; the files those replays parse grew with this PR's handoff chain, and all four outgrew the 5s default (each verified to pass with an explicit 30s budget, matching the suite's convention for subprocess-heavy tests). * fix(ci): mirror the handoff outcome consumers into the recovery clone (#9297) --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-08-20 08:32:43 +00:00
易良	0baaec2b32	chore(ci): Drop NPM_TOKEN in favor of npm Trusted Publishing (#9552) ... * chore(ci): Drop NPM_TOKEN in favor of npm Trusted Publishing * chore(ci): Pin npm 11 for Trusted Publishing in release jobs * test(ci): Cover Trusted Publishing requirements	2026-08-20 08:31:41 +00:00
易良	a07f52acfe	fix(tests): retry acp-cron cleanup rm on transient ENOTEMPTY (#9559) ... The recurring cron job re-fires every ~5s under QWEN_CODE_TEST_CRON_FAST. A second fire can race the cleanup's recursive rmSync and drop a fresh file into the fake HOME after the walk has drained it, making the final rmdir fail with ENOTEMPTY. Add maxRetries/retryDelay so the transient write settles before the removal gives up.	2026-08-20 08:26:34 +00:00
jinye	2e6151aa15	fix(serve): Harden standalone conversation primitives (#9512) ... * fix(cli): pass ConversationDirectoryIdentityError cause through native Error options * fix(cli): re-inspect raced standalone directories and report first creation as created * fix(core): measure JSONL head integrity against a line budget * fix(core): Preserve plain JSONL record budgets Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-08-20 08:09:44 +00:00
Shaojin Wen	a8a855914b	fix(review): run verifier probes in a private scratch worktree (#9207) (#9221) ... * fix(review): run verifier probes in a private scratch worktree (#9207) Step 4's verifier is the review's one writing agent: it writes a probe, runs it, applies the one-line fix its flip-check needs, and restores. All of that landed in the shared review worktree — the tree `working_dir` pins every other agent to — and the pipelined loop launches a round's verifiers alongside the NEXT round's reverse auditors, so those writes are live exactly while the auditors read. "Leave the tree as you found it", which the brief has always said and verifiers do obey, cannot close that window: the exposure is *during* the probe. Measured on a live run, a round-5 auditor read a probe's mutant plus a leftover probe test and came within a step of filing a Critical against code no commit contains; it recovered only by improvising `git show HEAD:`, a fallback no brief mentions. Three parts, because isolation alone is a guarantee one regression away from being false: - `qwen review scratch-tree` gives each verifier shard a throwaway sibling worktree at the commit under review, with the review worktree's node_modules linked in so a unit harness starts without an install. Every call hands back a pristine tree (a previous finding's mutant surviving into the next probe would be a wrong verdict carrying a deterministic source tag); the label is the shard's record key, because the shards of one round run concurrently; and a tree it cannot create makes the probe inconclusive rather than falling back to the shared worktree. This is the isolation the test-efficacy probe has had since #6832 and the A/B's base tree has, extended to the last step that writes. - Every code-reading brief — dimension agents, chunk agents, reverse auditors and the verifier itself — now carries the rule that auditor had to invent: the worktree is shared, code that is not in the diff and not in the commit is not a finding, and anything surprising is judged against `git show HEAD:`. - `agent-prompt` reads the tree once per call, and every wave of agents is built by it immediately before launch: residue is named in each brief it builds and warned about on stderr, so a contaminated tree is announced to the agents about to read it instead of being discovered as a phantom Critical. `exposeDependencies` moves to `lib/worktree.ts` beside the other disposable-tree machinery, and now farms each workspace member's own node_modules as well as the root's. Measured on this repo, a tree with 1560 root packages linked still could not resolve `@testing-library/react` for a UI probe, because npm could not hoist that copy out of `packages/cli` — which also silently cost the efficacy probe the same class of test. `cleanup` sweeps the scratch family by prefix; the label half is the shard's key and cannot be reconstructed by the sweeper. * fix(review): address the scratch-tree review — two criticals and the honesty gaps Both Criticals were real, and both had the same shape: a claim the code made that one machine class or one leftover state falsified. - The welded `scratch-tree` command interpolated `--worktree` unquoted, so on any checkout under a path with a space or an apostrophe every shard's isolation was silently unavailable and every probe fell back to a reading. Quoted with `shellQuotePath`, like every other path this file prints into a command. - The reuse gate checked only that the scratch path EXISTS. A bare directory there — the leftover of a crashed `worktree add`, or of a cleanup whose `rmSync` failed — has no `.git`, so git walked up and ran `checkout --force --detach` against the user's own checkout: their uncommitted work discarded, their HEAD detached onto the PR's commit, and `rev-parse HEAD` then returning the sha that made the reset report success. Gated on the tree being a registered worktree, with the regression pinned by a test that fails on the un-gated code. The rest are the same defect class at lower stakes — something stated more broadly than it holds: - The residue probe hand-parsed porcelain's rendered form, so a path with a space, a non-ASCII byte, or a literal ` -> ` came back as a name matching nothing on disk, `--untracked-files=normal` collapsed a whole probe directory to one unactionable `dir/` entry, the 1 MB `maxBuffer` default answered the dirtiest trees with "clean", and the cap truncated silently while both renderers presented the list as complete. Now `-z --untracked-files=all` with a 64 MB buffer, and a `{paths, total}` result both renderers disclose. - `git checkout -- <path>` restores from the INDEX, so the advised recovery left staged residue in the tree; it now says `git checkout HEAD --`. - `git show HEAD:<path>` cannot produce an untracked path — the prototypical residue — so the rule now says what that answer means instead of handing the reader a command that exits 128. - Agent 8's `--whole-diff` block reads the same shared worktree and got neither the rule nor the residue paths; it is built outside `buildLaunch`, which is exactly how it was missed. - The stderr warning claimed every agent had been told; the block is gated on `reviewsCode`, and Agent 7 — which builds and tests that tree — is not. - A label that flattens to no path-safe character fell back to a shared `agent` tree, `git clean -fd` left a nested repo standing while the report said the tree was pristine, `dependencies: null` said "no node_modules" about a farm that had failed to link, `{0,0}` read as "already in place" for a `node_modules` holding nothing linkable, and `--out` was validated after the tree and its farm already existed. - The docs stated the isolation unconditionally: local-diff and file-path reviews have no worktree and no scratch tree, and SKILL.md/DESIGN.md and the user page now say so. Tests moved with the code (`exposeDependencies` and `worktreeCreateFailureDetail` now live beside `lib/worktree.ts`), and the plumbing that was pinned at both ends but not in the middle — that a verify shard's recorded brief carries ITS record key as the scratch label — is pinned by a test that fails when the key is dropped. * fix(review): close the round-2 review — Windows guard, Agent 7's blind spot, unpinned wording The Critical is a test that would only fail where this PR's CI does not look: the new chmod-based case guards on `process.getuid`, which is undefined on Windows, and `chmodSync` on a directory there sets a read-only attribute that does not stop `git worktree add` from creating a subdirectory — so the merge_group-only Windows leg would go red for every PR carrying the file. Sixteen of the seventeen chmod-permission tests in this repo already skip win32; this one now does too. The rest are the same class as round 1 — a claim wider than the code: - **Agent 7 had no protection for residue that PREDATES the round.** The exclusion's justification ("its own commands are the writes it sees") is only true for residue a round creates. A tree that starts dirty reaches Agent 7's compile and test run, where a `[build]`/`[test]` finding is pre-confirmed and skips verification — a merge-blocking phantom Critical of exactly the class this machinery exists to prevent. The residue paragraph now goes to EVERY brief (with "a defect confined to these paths is not a finding"); only the reader rule stays scoped to the roles that review code. The stderr warning says to restore before launching the wave, not before the next round. - **Residue paths reached two sinks unflattened.** `inertPath` moved to `lib/paths.ts` and now covers the scratch-tree note and the orchestrator's stderr line as well as the briefs — the `-z` format this PR introduced is precisely what lets a control byte in a filename arrive intact. - **The recovery wording could not clear two shapes the probe reports.** A path staged as NEW and a rename destination are in the index but not in HEAD, so `git checkout HEAD --` cannot match either; both renderers now name `git rm --cached` for those and `rm -rf` for untracked residue (including the nested- repo directory entry `--untracked-files=all` still cannot expand). - **The "full set" command was the one this PR calls unusable.** Both notes now say `git status --porcelain --untracked-files=all`, since the default collapses the probe directory whose files the count came from. - **`alreadyPresent` believed an empty farm dir.** The dir a previous call creates when the source holds nothing linkable is gitignored, so the reset spares it, and the second call flipped "no harness will start here" into "already in place" with nothing changed. It now requires a non-empty farm. - Doc corrections: `resetScratchTree`'s header still said `clean -fd`, and the residue probe's comment claimed `--untracked-files=all` ends directory-shaped entries — it does not for a directory holding its own `.git`. The invalid- UTF-8 limit (`encoding: 'utf8'` maps a bad byte to U+FFFD, and no string form of that name resolves) is documented rather than papered over. Pinned, each verified by reverting the fix and watching the test go red: the apostrophe ESCAPE in the welded command (the fixture had no apostrophe, so a naive `'…'` wrap passed), the `git checkout HEAD --` wording in both places, the capped-note arithmetic, the farm-failure note, stdout-before-side-file ordering and the exit-1 arm it also covers, and `sharedTreeResidueTotal` on the creation-failure return. * fix(review): close the round-3 review — hooks, hidden mutants, planted farms, suppression after restore Four of the seven Criticals are the same discovery from four directions: a scratch tree is a LINKED worktree and a `clean`/`checkout` reset is not the guarantee it reads as. - **Hooks resolve to the user's repository.** `git worktree add` and `checkout --force` both fire `post-checkout` from the common dir — the user's own `.git/hooks` — so creating or resetting a scratch tree executed whatever that repository holds. Every git call this command makes now runs with `core.hooksPath` pointed at a path holding no hooks, and the report says plainly that hooks, config and refs are shared rather than isolated. - **skip-worktree hid a mutant through the reset.** `checkout --force` silently skips a file carrying the bit and `clean` never touches tracked files, so a probe that set it (directly or via `sparse-checkout`) left a mutant that survived with `git status` reading empty and the sha still matching. The reset now refuses when `ls-files -v` still shows a hidden entry, which routes the caller to discard-and-rebuild. - **The farm was certified by existence.** `clean -ffd` spares ignored paths to keep the dependency farm — and equally spares whatever a probe installed or planted there. `node_modules` is now the one ignored path a reuse does not inherit: it is cleared and re-linked, and `exposeDependencies` marks the farms it builds so a directory it did not build is never certified as one. - **A broken leftover could not be rebuilt over.** A `.git` gitfile whose admin entry survives makes `worktree remove` fail and the next `worktree add` refuse "missing but already registered"; `discardWorktree` now prunes. The fifth is about the instruction rather than the tree: the residue paragraph is baked into every brief at build time, so restoring the paths and then launching the already-built wave tells every agent to drop findings in a file that is by then exactly the PR's code. Both the stderr warning and SKILL.md now say to rebuild the wave after restoring. The remaining two are the three new real-git fixtures missing `isolateHostGitConfig()`, which a polluted host gitconfig (`commit.gpgsign` with no key, a `core.hooksPath` hook) turns into suites that fail for reasons the branch never touched. The suggestions, in one line each: a rename now reports BOTH of its names (the restore needs the one that is gone); a `git status` that dies is reported as UNMEASURED rather than clean, in both renderers; `inertPath` covers `\p{Cf}` and `\p{Zl}`/`\p{Zp}` — bidi overrides and zero-width characters passed through the sanitizer that exists to defuse hostile filenames; residue paths and tree paths are shell-quoted where the notes prescribe commands over them (`rm -rf my probe.ts` deleted `probe.ts`); the verifier brief no longer says "then move on" after applying a candidate fix, because one tree serves every finding in a shard; `scratchLabel` strips a leading dash, which yargs read as a flag; the member-farm loop guards per member rather than around the loop; the unguarded `mkdirSync`/`readdirSync` in the farm now count as failures instead of throwing out of a best-effort contract; cleanup discloses a `readdir` failure instead of reporting "nothing to clean", and sweeps a dangling symlink `releaseWorktree` cannot see; the briefs' restore recipe gained the staged-only branch the scratch-tree note already had; and both "pristine" claims now say that gitignored paths survive. Also corrected: "the one agent whose job requires writing" — Agent 7's efficacy probe writes too, and has had its own tree since #6832. New tests, each verified by reverting its fix: the hooks suppression, the skip-worktree refusal, the planted-farm replacement, the prune, staged-residue detection, the unmeasured state, `inertPath`'s character class, and the residue reaching every launch class rather than the one role the earlier test inspected. * fix(review): close the round-4 review — untrusted workspace paths, member farms, locked leftovers The sharpest two are about treating the reviewed PR's own manifest as data rather than as input, in code that DELETES: - `exposeDependencies` fed workspace dirs from the root manifest of the code under review straight into `join()` and then into the farm's opening `rmSync`. A PR setting `"workspaces": [".."]` — or a committed SYMLINK at a workspace path, which `readWorkspacePackages` follows deliberately because npm does — pointed that delete at a directory outside both trees; in this pipeline's layout, at the reviewer's own checkout. Every member is now resolved through `realpathSync` and required to be contained in the tree it belongs to, which closes the string and the symlink vector together, and a member that escapes is counted as failed rather than silently skipped. - The reuse path wiped only the ROOT farm, so `<tree>/packages/<member>/ node_modules` survived with its marker and was certified as-is — the same hole round 3 closed at the root, one level down, where Node resolves a member's imports FIRST. `exposeDependencies` now takes `rebuild`, and the reuse path distrusts every farm rather than the top one. Two more that would have wedged a review: - `git worktree prune` never drops a LOCKED admin entry, and probe code has a shell inside these trees: one `touch` in the admin dir the tree's own gitfile names, and every later `worktree add` for that PR fatals "missing but locked" — permanently, since cleanup prunes too. `discardWorktree` now unlocks and retries with the second `--force`. - `ls-files -v` and `clean` ran under `spawnSync`'s default 1 MiB buffer, which a large repo passes; Node kills the child, the reset reads that as failure, and every reuse rebuilds forever. And a Windows one of the same class as round 2's: the new fixture wrote a file named `a -> b.ts`, and `>` is reserved on NTFS — the merge-queue-only Windows leg would have failed at fixture setup. Split, with the arrow half skipped there (the shape it pins cannot exist on NTFS). The rest: the residue restore recipe was WRONG for a staged rename's original name (`git rm --cached` stages a deletion there; `git checkout HEAD --` is what clears it) and the test that certified it never staged anything — both fixed, and the test now builds all four real shapes; the reader rule was gated on `reviewsCode`, which left Agent 0 and the test matrix reading worktree source without it (the gate is now "every role that judges code", i.e. all but Agent 7); the stderr warning did not actually carry the rebuild-after-restore instruction SKILL.md attributes to it; `ScratchTreeReport` dropped the `unmeasured` state, so a failed `git status` read as clean to a script; the scoped-package branch linked non-directory entries the top-level branch skips; `farmDependencies`' catch and its note branch were unreachable once round 3 guarded the fs calls, so they are gone rather than pretending to be a net; cleanup's family-read failure now fails the run instead of letting it announce "Nothing to clean", and its dangling-symlink branch no longer swallows a throwing `rmSync`; the residue note says the names are flattened for display and where the exact bytes are. Disclosed rather than enforced, with the reason in the code: the farm's links are read-write and point into the shared worktree, so a probe that writes THROUGH one (an `npm rebuild`, a package that writes into its own directory) lands outside its tree where the residue check cannot see it. Copying the farm would cost the minutes it exists to save; the verifier's block now says to replace a link with a copy before modifying a dependency. New coverage: workspace escape and symlinked-member containment, the self-farm guard, rebuild-on-reuse at member level, assume-unchanged beside skip-worktree, the unmeasured state through its renderers, the dangling-symlink sweep, the family-read failure, and the cleanup mock gaps that made two of those branches unreachable by construction. * fix(review): close the round-5 review — scoped prune, real pristine, submodules, symlink gadgets The findings that hold without assuming an attacker already has a shell: - **The prune added in round 4 was repo-wide.** `git worktree prune` drops any admin entry whose directory is momentarily absent — another shard's `worktree add` mid-flight (this pipeline runs discards and adds concurrently against one common dir), or the user's own worktree on a volume that happens to be unmounted. It now removes the one entry whose `gitdir` file names this path, and nothing else. - **"Pristine" spared ignored paths, and a probe's state lives there.** Its own `node_modules` at any depth, a `.tsbuildinfo`, a `dist/` it built and then mutated — all survived a reset the report called pristine, and the farm-level wipe could not reach them (a member farm whose source has none was skipped before the target was touched; the root rebuild was skipped entirely when the review worktree had no `node_modules`). The reset is now `clean -ffdx` and the farm is re-linked, so pristine means pristine. - **An initialized submodule was untouched by all of it**: `checkout --force` without `--recurse-submodules` leaves its working tree, `clean` never touches a tracked gitlink, and `rev-parse HEAD` is the superproject's. A tree holding one is now rebuilt rather than reset — a fresh `worktree add` leaves submodules uninitialized, so the rebuild is both correct and cheap. - **test-efficacy reuses one probe tree across every suite run**, and the code running in it is the PR's own test code. Round 4 answered this with the positive control; the control only covers the green-forcing direction, so the farm is now re-linked before each run instead — about a second against a 540-second budget. The remaining Criticals all presuppose code execution that this pipeline grants before any of this code runs (Agent 7 installs and tests the PR; a verifier's probe is arbitrary code by design), so they do not change what an attacker can do — but three of them describe gadgets that cost nothing to remove, and they are removed: - `cleanup` no longer hands ANY symlink at a family path to `git worktree remove`, which would follow it and delete whichever registered worktree it points at. It unlinks the link and reports that. - `resetScratchTree` validates that the tree IS the tree — not a symlink, and `rev-parse --show-toplevel` resolving to itself — before running a reset that would otherwise land wherever the path resolves. - `farmNodeModules` lstats its SOURCE root, so a symlink at the review worktree's `node_modules` cannot redirect every probe tree's dependencies. Also: the `dependencies: null` contract now says what it means (no `node_modules` to link from; a link failure arrives counted, not as null), the `alreadyPresent` branch of the note is gone with the reuse path that could reach it, and the residue note carries the flattened-names disclosure its sibling renderer already had. New tests: the symlinked scratch path, ignored-state clearing, the submodule rebuild, and the unmeasured pass-through through the command's own report. * fix(review): close the round-6 review — committed farms, foreign repos, deinit'd submodules One finding crosses the line the round-5 reply drew, and it is the important one: `node_modules` is gitignored by convention, not by rule, so a pull request can force-add `node_modules/.qwen-review-farm` beside its own module stubs and `git worktree add` checks both out. The fresh-create path then found a marker and certified the PR's own modules as the farm this code built — reachable from PR CONTENT, with nothing executed. Two changes: the marker records the dependency root it was built from and is only believed when it names this one, and both scratch-tree paths (fresh and reuse) now rebuild rather than trust anything found at that path. The other three that hold on their own: - **The identity check added in round 5 was incomplete.** `rev-parse --show-toplevel` prints the directory the `.git` file sits in, whatever that file points at — so a gitfile naming another repository, or a whole repo planted at the predictable scratch path, passed it while every command below would have run against someone else's objects, refs, hooks and config. The tree must now share a common dir with the review worktree. - **`git submodule deinit` walked through the round-5 submodule gate**: the status line goes back to `-` while the submodule's gitdir — its hooks, its config, its objects — survives under the common dir and is resurrected by the next `update --init`. The gate now reads the commit's gitlinks instead of the submodule's state, so a repo with submodules rebuilds rather than resets. - **The round-5 anti-planting rebuild never reached the positive control or the hunk probes.** They defaulted `dependencyRoot` to the probe tree, where `exposeDependencies` returns before clearing anything — so the control, the run that decides whether ANY mutant verdict is trusted, resolved through whatever the baseline's test code had planted. Smaller, from the same review: a dangling symlink at the target skipped the rebuild wipe (`existsSync` follows links); a dangling top-level link in the source was dropped without counting; a stray file under a scope directory was linked as a package where the top-level branch skips one. Disclosed rather than fixed, with the count now in the report: the farm mirrors npm workspace SELF-links, so an import that goes through a package NAME resolves back into the review worktree's copy and a mutation made in the disposable tree is invisible to it. Re-pointing them would break resolution outright for any package whose entry point is a build artifact a fresh checkout does not have, so `DependencyFarm.selfLinked` carries the number instead of the layout carrying it silently. Not fixed, and this is the third round of stating it: the remaining findings need a prior arbitrary-code-execution foothold (a background process racing the containment check between resolve and use). The pipeline grants that capability several steps earlier and by other code — Agent 7 runs `npm ci` and the PR's test suite — so hardening this command changes which line appears in the trace, not what is possible. The place to change it is sandboxing the review's command execution, which is its own design. Tests: the locked-leftover recovery, the scoped registration drop (a sibling worktree whose directory is absent survives it), the foreign-repository gitfile, the deinit-proof submodule gate, and the scope-directory stray file. The foreign-repo fixture is a CLONE on purpose — with an unrelated repo the reset fails for the wrong reason and the test passes without the guard it pins. * fix(review): close the round-7 review — walk-up escapes, identity gates, farm wipe Five Criticals, each probe-verified on real git before the fix and mutation-checked after: - worktreeResidue verifies the path IS a worktree before measuring: with the .git file gone, git discovery walks up into the user's checkout and answers with the user's own dirty state — fail closed instead (R7-4). - runScratchTree applies the same --show-toplevel check to the trusted --worktree argument before resolving HEAD (R7-3). - The reuse identity gate rejects a tree whose gitdir equals the common dir: a .git symlinked or hand-edited to name it passed every prior check while checkout --force detached the user's main HEAD onto the PR sha and rewrote the main index (R7-2). - farmNodeModules detects the rebuild target with lstatSync: a dangling symlink a PR committed as node_modules read as absent, skipped the wipe, and died EEXIST on every rebuild (R7-1). - The residue probe's node_modules/dist exclusion is enforced by a pipeline-controlled excludes file rather than borrowed from the commit's .gitignore (R7-5). Plus the production-shaped UNMEASURED fixture (R7-19) and the three remaining stale contract texts on the reuse reset (R6-7). * fix(review): close the round-8 review — tripwire identity, GIT_DIR redirects, symlink writes Round-8 Criticals, each probe-confirmed and pinned by a test that goes red when the fix is reverted: - The residue tripwire certified a CLEAN status for a repository planted over the contamination (`rm .git && git init && commit`): no local check can tell a planted repo from the tree it replaced. It now fails closed unless `.git` is a gitfile, and reports unmeasured for what `git status` cannot see inside committed gitlinks (a non-empty submodule directory) instead of clean. - An inherited GIT_DIR (or GIT_WORK_TREE/GIT_INDEX_FILE/...) redirected every identity check at once — both sides of each comparison see the same override, so none could detect it. Every git call in the scratch-tree lifecycle and the tripwire now drops the redirect variables. - The scratch-tree reuse gate accepted a planted gitfile naming a SIBLING worktree's admin entry and reset the sibling: the gate now requires the admin entry's gitdir backpointer to resolve to this tree. - Probe writes followed PR-committed leaf symlinks (mode 120000) into the shared review worktree — the positive-control injection, every mutant and every hunk restore. Each write site refuses a symlinked target as inconclusive. - discardWorktree handed a symlink at the tree path to `git worktree remove --force`, which resolved it and force-deleted the registered victim it pointed at; a symlink is unlinked instead. Plus the record-key comments (chunk id separates --all-chunks shards) and the orphaned marksOurFarm doc block. * fix(review): close the round-9 review — gitlink residue, env redirects, probe escapes Round-9 Criticals, each probe-confirmed on real git before the fix and pinned by a test that goes red when the fix is reverted: - worktreeResidue's gitlink blind set was parsed from RENDERED `ls-files -s`: default core.quotepath quotes a non-ASCII gitlink name into a spelling that never resolves on disk, the entry dropped from `blind`, and a contaminated gitlink was certified CLEAN. It now reads `-z` (R9-1). - An exported GIT_DIR redirected every inheriting git call at once: test-efficacy's head-sha read, probe resets and revert checkout, and base-tree's add and reuse check. Every git spawn in both files now drops the redirect variables (R9-16). - cleanup's symlink guard covered only the scratch family; the three named family paths still reached releaseWorktree, whose existsSync follows a LIVE link and whose `worktree remove --force` resolves it — deleting the registered victim while reporting success, and silently skipping a dangling link. The guard moved into the shared report path all four families go through (R9-18). - exposeDependencies' rebuild wiped only the farm-owned node_modules; anything planted at another path — packages/node_modules resolves before the root farm in a packages/* workspace — survived between probe runs and decided their verdicts. Rebuild now wipes every node_modules the farm does not recreate, following no links (R9-37). - NO_HOOKS covered hooks only, not config-driven content FILTERS: a planted `filter.<name>.smudge|clean` plus one attributes line — both writable into the common dir — executed on every reset and rebuild checkout. runScratchTree detects repo-local filter config and refuses rather than run it (R9-38). - The harness never WRITES probe test files, so no write guard ever saw one committed as mode 120000 — vitest collected it through the link and scored mutants against code the tree never mutated. Probe files are checked against the committed index and dropped as not-run before any suite collects them (R9-40). - The probe write guards lstat-checked the LEAF only; lstat resolves intermediate components, so a relinked ancestor in a reused tree read ordinary and every later write followed it out of the tree. The guard walks every component (as safeRmWithin does for deletes), and each restore write re-validates immediately before writing — a mid-run relink stops the phase instead of writing through the link (R9-17). * fix(review): close the round-10 Criticals — env config injection, sweep scope, probe-set holds Taken back from the autofix loop, and scoped to Criticals from here: the findings that hold without assuming a foothold, plus the ones whose fix is a few lines and removes a step from a chain. Reachable from the environment or from PR content, with nothing executed: - **`sanitizedGitEnv` dropped only the discovery redirects.** `GIT_CONFIG_COUNT` with `GIT_CONFIG_KEY_<n>`/`GIT_CONFIG_VALUE_<n>` sets any config key for the run — `core.fsmonitor` and the `filter.*` pair are command execution — and `GIT_CONFIG_GLOBAL`/`GIT_CONFIG_SYSTEM`/`GIT_CONFIG_PARAMETERS` reach the same place by other routes. A gate on the front door with the window open. - **`lib/git.ts` never had the gate at all**: `gitOpts()` spread `process.env` into every shared helper, `releaseWorktree`'s `worktree remove --force` included — the commands that run against the user's own repository, so they need it more than the disposable-tree ones that got it first. - **The residue probe could be steered by the tree it measures**: `core.fsmonitor` runs a command on `status`, so the tripwire was an executor. Emptied for that invocation, the same discipline the checkouts' `core.hooksPath` already had. - **`probeTargetEscapes` treated a backslash as a separator on POSIX**, where it is an ordinary name character: `x\y.test.ts` split into phantom components, the first `lstat` died ENOENT, and the catch reported "no escape" for a leaf nobody looked at. It also never checked the tree ROOT, which defeats every per-component check at once. - **The symlink drop reassigned the probe list before the collocated-test hold ran**, so a mutant whose own test was dropped answered "there is no such test" and was scored `survived` on the strength of the other probes passing — the hold exists to refuse exactly that. - **The rebuild sweep matched `node_modules` case-sensitively** on the two filesystems this file already special-cases: `Node_Modules` resolves like the real one on APFS and NTFS and survived every rebuild. - **A tracked path in the residue list suppressed real findings.** The brief said "none of it is the PR's code, so a defect confined to those paths is not a finding" — false for a file the commit contains and this PR changes: only the uncommitted DIFFERENCE is foreign. It now separates the two, and a defect present in the committed version stays a finding. Cheap chain removal, though each needs a prior foothold: - The excludes file moved from a constant name in the shared temp dir to a `mkdtemp` directory written `wx` — git re-reads its CONTENTS on every later `status`, so a writable fixed path could blind the tripwire. - `discardWorktree` now clears the registration named by the TREE's own `.git` pointer, read before anything is removed, instead of scanning `<common>/worktrees/<id>/gitdir` files — which any same-user process can rewrite to aim this cleanup at a live sibling. The scan remains only as the fallback for a tree whose pointer is unreadable, which is the case the step exists for, and it now refuses entries whose tree still stands. - The scratch sweep refuses to run at all when its parent is not a directory: a symlinked `.qwen/tmp` redirects every delete below it, and no per-leaf check can see an ancestor. - `removeUnownedNodeModules` counts a wipe it could not perform, instead of letting a planted directory survive every rebuild undisclosed. Left as recorded decisions, not re-litigated: R4-5, R6-2 and the R6-5/R9-14/ R9-38/R9-40/R10-25/R10-27 class, which need probe code already running as the user — a capability Agent 7's install and test step grants several steps earlier. The remaining Suggestions stay on the deferred queue for a follow-up, per the triage gate's own call that seven rounds was the moment to land this. * fix(review): close the round-11 Criticals — the half-fixes from round 10 Five of the eight are residue from the previous round's own fixes, which is the honest reason to take them: - **The fsmonitor neutralization covered one spawn of two.** `git status` got `-c core.fsmonitor=`; the sibling `git ls-files -s -z` that builds the gitlink blind set did not, so the same planted command still fired during the measurement. - **The gitlink blind-set filter failed OPEN.** An absent directory is the clean shape `worktree add` leaves; an UNREADABLE one is a place neither `git status` nor this probe can see, and both answered "nothing to hide there". Only ENOENT is clean now. - **The wipe walk skipped symlinked directories entirely**, so a `node_modules` behind a PR-COMMITTED symlink directory survived every rebuild with nothing said. The walk still refuses to follow a link — the farm's own entries point out of the tree — but a link resolving back INSIDE the tree that hides dependencies is now counted as a failure the caller reports. - **The sweep's ancestor check stopped at its own argument.** `lstat` refuses to dereference only the last component, so a symlink one hop above (`.qwen` over `.qwen/tmp`) resolved silently and redirected the same deletes. Every ancestor up to the filesystem root is checked. - **Detection was followed by the damage it detected.** When the mutation phase aborts because the probe tree was relinked mid-run, the revert phase ran anyway — `git checkout base -- …` and `safeRmWithin` with a cwd resolving through the link into the shared review worktree. The phase now re-validates the tree first, and `safeRmWithin` lstats its own ROOT, which its docstring always promised and its loop never did (a root link resolves the whole prefix in the kernel, so every component below it looks ordinary). Left as recorded decisions: R11-1 (`refs/replace` in the shared common dir), R11-2's residual fallback path, and R11-3 (content filters during `status`) — each needs probe code already running as the user, the capability Agent 7's install and test step grants several steps earlier, and the scan fallback exists precisely for the corrupt-pointer recovery the earlier rounds pinned. * fix(review): close the round-12 Criticals — sanitizer gaps, sweep scope, false farm failures Nine of the twenty-four hold without a foothold, or are defects in the previous rounds' own fixes: - **`residue.unmeasured` rode raw into two sinks** while the paths beside it went through `inertPath` — and that string is built from `ls-files -z` gitlink names, so a PR that commits a gitlink with ESC bytes puts a control sequence on the orchestrator's terminal and in the verifier's note. - **The branch delete never got the env the check did.** `refExists` resolves the real repository through the sanitized helpers, while `git branch -D` in cleanup and fetch-pr still inherited `process.env`: with `GIT_DIR` exported the pipeline verified a branch in one repository and deleted it in another. - **The ancestor refusal guarded one sweep of three.** It announced that `.qwen/tmp` hangs off a symlink and the same function went on to delete the base-tree lock and every side file underneath it. The check moved to the top of `runCleanup` and now refuses the whole clean. - **A family symlink that would not unlink released the lease**, unlike the three sibling failures that hold it, leaving the next `fetch-pr` to pass a gate over an occupied path. - **The symlink disclosure added last round produced FALSE failures**: it never consulted the `owned` set, so a PR-committed `alias → .` counted a failure for the farm this very call had just re-linked, and it judged before the walk had wiped the target it was complaining about. It now runs after the walk and skips owned farms. - **Probe names were passed to `git ls-files` as raw pathspecs**, so a probe committed as `:(literal)x.test.ts` — a legal filename — was parsed as magic and its symlink never found. Every pathspec is `:(literal)`-prefixed now, and a refused pathspec drops every probe rather than answering "no symlinks". - **The residue oracle could be blinded by index bits.** `skip-worktree` and `assume-unchanged` make `status` answer clean for an edited tracked file — the hazard the scratch tree's reset already refuses to certify around, missing from the reader-side probe that tells auditors the tree is pristine. - The excludes file is created 0600 rather than 0644. Left on the recorded boundary: the filter-gate coverage findings (R12-6, R12-24), the hardlink and runtime-relink shapes (R12-31, R12-32, R12-72). Each needs the PR's own test code running as the user — the capability Agent 7's install and test step grants several steps earlier — and the last three are properties of executing untrusted tests at all, not of this command. * fix(review): close the round-13 Criticals — spelling, empty probe sets, raw error text Five hold without a foothold, and four of them are defects in the previous round's own fixes: - **`owned` mixes path spellings by construction** — the tree root as the caller spelled it, each member as `containedIn` resolved it — and last round's disclosure compared only one of them, so a farm reached through a link counted a failure for a directory that same call had just re-linked. Both spellings are asked now, through one helper the wipe and the disclosure share. - **A suite run with an EMPTY probe list runs vitest with no filter**, which collects whatever the repository holds and scores it as this probe's evidence. After the committed-symlink drop empties the set, the mutation and revert gates now skip rather than run. - **Git's own stderr rode raw into the verifier-facing note** through `Error.message`, as did the filter KEY NAMES in the refusal (a git subsection name carries any byte but newline and NUL). Same sink class as last round's `unmeasured`, flattened the same way. - **Every refusal that fires before the residue is measured answered with the empty list a measured-clean tree produces.** A consumer could not tell "the tree is clean" from "this call never looked"; the refusals now carry the unmeasured reason. Left on the recorded boundary, re-checked and unchanged: R12-6, R12-22, R12-24 (config planting), and the R13-9/R13-10/R13-13 family, which need probe code already running as the user. * fix(review): close the round-14 Criticals — farm containment, residue blinds, release guard Five hold without a foothold, and three of them are channels a pull request controls outright — no same-user foothold anywhere: - **Farm entries were mirrored link-or-not.** A committed symlink under `node_modules` (force-add defeats gitignore) pointing out of it became a write channel from the disposable tree to wherever it points — into the shared worktree's tracked files — recreated on every rebuild. Entries now resolve through realpath before linking; only a borrowed `node_modules` or an npm workspace self-link passes, everything else is counted and disclosed. (R10-18) - **The residue probe's untracked view came from `status` alone**, which honors ignore rules the contaminator controls: a committed whitelist-form `.gitignore` (`*` with `!`-negations) blinded it to contamination. The probe now merges `ls-files --others` without `--exclude-standard` and filters the pipeline's build artifacts in code. (R10-19) - **A gitlink named in bytes UTF-8 cannot decode dropped from the blind set**: the mangled spelling never resolves on disk, the readdir read absent, and a contaminated gitlink certified clean. Such names now fail closed into unmeasured. (R11-4, second entrance) - **The `owned` set the rebuild disclosure asks held one spelling** while the disclosure loop presented another — on a host whose tree path carries a symlinked ancestor (macOS's `/var` vs `/private/var`) every rebuild counted a phantom failure for the farm the same call had just re-linked. The set is normalized once, at build time. (R13-1 remainder) - **`releaseWorktree` followed a symlink standing at the path**: `existsSync` resolved it and `git worktree remove --force` deleted whichever registered worktree it named — and a dangling one wedged the next `worktree add` while invisible. The lstat-first guard cleanup's family sweep applies to every path now lives at this choke point, which is where fetch-pr's `cleanStale` releases. (R13-3) Pinned against real git and real filesystems; all seven new tests fail on the pre-round sources. * fix(review): skip the invalid-UTF-8 gitlink fixture off Linux APFS and NTFS both refuse a filename that is not valid UTF-8, so the fixture cannot be created there — `mkdir` fails ENOENT on macOS, and the round-14 test reddened for every maintainer on a Mac while the repo's `pull_request` CI, which runs the unit legs on Linux only, stayed green. The shape it pins cannot exist on those filesystems either, so the skip loses no coverage. Same class as the round-2 and round-6 guards (`chmod` semantics, NTFS reserved characters): a fixture that cannot exist on a platform is skipped there rather than left to fail in a leg nobody watches. * fix(review): close the round-15 Criticals — a tripwire that was firing on every healthy run The important one is a regression this PR introduced two rounds ago, and it is measurable rather than hypothetical. Round 14 answered "a `.gitignore` can hide a probe's leftovers from `status`" by merging the ignore-INDEPENDENT untracked listing into the residue set. Measured on a healthy review worktree of this repo, after the `npm ci` and build the pipeline itself runs there: `git status` reported NOTHING and the tripwire reported 3 957 paths — coverage HTML, `.tsbuildinfo`, husky's installed hooks. Every one of them reached every verifier and every reverse auditor as contamination, with `rm` and `git checkout` recipes attached, and real residue would have been three lines inside that. A tripwire that fires on every healthy run is not read. What separates the two sets is not the pattern, which is unbounded, but WHO WROTE THE RULE. An ignore file the commit under review carries is ordinary repo hygiene that every reader of that tree sees identically; a rule from anywhere else — `info/exclude`, a `.gitignore` written after the checkout — is the plant. `check-ignore -v` names the source file per path, so the reconciliation asks git that question instead of inferring it from the name. One exception, because it is the whitelist form's whole trick: a rule from the commit that hides EVERYTHING (`*` with `!`-negations) vouches for nothing and is not believed. Measured again through the real function afterwards: 3 957 → 0, with the two files that were genuinely modified at that moment still named. Also: - **The index-bit oracle failed OPEN.** Its guard asked for `status === 0`, and a spawn that dies leaves `status` null — so the one tree it could not read the skip-worktree/assume-unchanged bits of was certified clean, which is precisely the tree that can be carrying a mutant `status` cannot see. It now fails closed like its three siblings, and takes `-z` so `core.quotepath` cannot octal-expand a non-ASCII name past the buffer. - **The efficacy probe tree carried tracked writes between runs.** Re-linking `node_modules` before each run (round 5) covers half of what a run leaves behind; tracked files are the other half and the more direct one — a suite that rewrites a probe file after vitest has collected it stays green for its own run and hands every later run a file of its choosing, and the verdict that buys is `killed`: "a test catches this", asserted for statements no test covers. Every run now opens by putting the tree back to its commit, guarded by the same identity gate the residue probe uses so a directory that is not a checkout is skipped rather than having the enclosing repository checked out into it. Two fixtures had to change with it, and both were relying on the hole. The control-setup test made its point by having the fake runner DELETE the probe file; a deleted probe file now comes straight back, so it relinks it instead (the read-failure path itself is pinned directly in the unit suite). The wrong-line-mutant test wrote its mismatched content without committing it, which is not a state a probe tree can be in — a probe tree is a detached checkout — so it commits it. Each fix was verified by reverting it and watching the new test go red: the provenance filter, its trust direction, the catch-all exception, the `.husky/_` footprint entry, the fail-closed oracle, and the tracked-file restore. * chore: keep main's package-lock.json A local `npm install` during the merge dropped six `"peer": true` markers from the lock. Unrelated to this branch, so it goes back to main's copy. * fix(review): close the round-16 Criticals — catch-alls, the exec env family, untracked plants Two of these are holes in last round's own fix, which is the right way round for them to be found: - **`hidesEverything` recognised only the `*`/`**` spellings.** `?` matches any single character, so `?*` is `*` with extra steps — a committed `.gitignore` whitelist written that way vouched for everything it hid, with no execution anywhere: it is committed content. The check is now shape-based rather than a list of spellings: every segment wildcard-only, at least one `*`. Enumerating the spellings one at a time is how the hole got made. - **The probe tree's pre-run restore failed OPEN on an unresolvable `.git`.** The gate skips a directory with no `.git` because there is no commit to restore FROM — but `.git` is untracked, so nothing ever restores IT, and a guest that overwrites it once bought "proceed, nothing to put back" from every later phase. Present-but-unresolvable is now a failure, which is the same discipline round 15 applied to the residue probe's index-bit oracle. And three that stand on their own: - **`safeRmWithin` split the PR-controlled path on backslashes unconditionally.** On POSIX a backslash is an ordinary filename character, so ONE committed name — `\..\review-pr-42\package.json`, legal on POSIX — became three components, two of them `..`, which `join` normalises away silently. The revert phase's delete then reached the sibling shared review worktree. `probeTargetEscapes` already split platform-dependently with a comment naming this hazard: the write side had the fix, the delete side did not. Both now, and a `..` component is refused outright rather than resolved. - **`sanitizedGitEnv` closed redirection and config injection and left execution open.** `GIT_SSH_COMMAND` and `GIT_EXTERNAL_DIFF` are a command, `GIT_EXEC_PATH` moves git's own subcommand and remote-helper lookup, `GIT_TEMPLATE_DIR` plants hooks for the next `init`. Not a new judgement call: `config/shared-env-keys.ts` blocks exactly this family for session subprocesses, with the rationale written out there. The setter need not be an attacker — a reviewer's shell profile exporting `GIT_EXEC_PATH` silently changes which `git-remote-https` every fetch in this pipeline runs. - **Nothing removed UNTRACKED files between probe runs.** Round 15 restored the tracked half; a `vitest.config.ts` — untracked, because no zero-config project commits one — is what a suite reaches for to decide the next run's collection. `clean -fd` and not `-fdx`, so the borrowed farm and the ignored build output the probes need survive. Also, the live-symlink test in `git.integration.test.ts` compared git's CANONICAL worktree path against an un-canonicalized `mkdtemp` path. It passes on macOS by accident — `/private/var/…` contains `/var/…` as a substring, so `toContain` succeeds — and would not on a fixture reached through a symlinked ancestor. It now realpaths, so it passes on purpose. Each fix was verified by reverting it alone and watching its test go red. * fix(review): close the round-17 Criticals — provenance, absent .git, ignored plants, ancestor links Three are holes in the two previous rounds' own fixes: - **"Tracked" is not "unchanged".** The round-15 provenance test asked `ls-files` whether the ignore file's PATH is in the index, so a `.gitignore` the commit carries went on vouching for rules appended to it after the checkout. The status set the same function already computed answers this: a source that appears there has been edited away from the commit, and its rules are the writer's. - **An ABSENT `.git` was read as "nothing to restore".** Round 16 made the probe tree's restore fail closed on a `.git` it cannot resolve and left the cheaper state open — `.git` is an untracked pointer file inside the tree the PR's own suite runs in, so one `rm` bought "proceed" from every later phase. Running the restore anyway is not the alternative: with no `.git`, discovery walks UP and checks the enclosing repository out into the tree. Refusing is the only answer that is neither. - **The between-run sweep honored the ignore rules.** `clean -fd` skips what the commit's own `.gitignore` names, and those rules are the PR's to write, so a plant named to match one survived every restore. It is `-ffdx` now, with `-e node_modules`: the borrowed farm is the one ignored thing in that tree the probes cannot run without, and everything else ignored goes. The two restore spawns also empty `core.fsmonitor`, which both of them execute. And four that stand on their own: - **`releaseWorktree`'s symlink guard was leaf-only.** `lstatSync` dereferences every component except the last, so a link at `.qwen/tmp` left every path under it looking ordinary while `git worktree remove --force` landed in whatever checkout it named. `runCleanup` refuses its whole sweep for this; `cleanStale` releases with no guard of its own, so the refusal now lives at the choke point every caller inherits. - **`runCleanup`'s own ancestor guard ran before a network-bound audit** and nothing re-checked it afterwards, though the lease condition beside it gets exactly that re-check for exactly that window. - **The scratch tree's filter screen read the wrong tree's config.** It runs against the review worktree, while the checkout it authorises runs in the SCRATCH tree, whose own `config.worktree` is honored once `extensions.worktreeConfig` is on. The screen now reads every entry under the common dir's `worktrees/`. - **`runOneMutant` and `runControlMutant` had no pre-write escape re-check**, while `runOneHunkProbe` — in this same diff — carries one with a comment explaining the threat. `redirectedAncestor` is now one shared function rather than two, and its walk STOPS at the checkout instead of climbing to `/`: `/var` is a symlink on every macOS box, so the unbounded version refused every sweep there while reporting that it had found a redirect. Two fixtures became real checkouts, because a probe tree is one in production and a bare `mkdtemp` no longer reaches the behaviour they pin. Each fix with a behavioural test was verified by reverting it alone and watching that test go red. The pre-mutation re-checks are window-narrowing on a check-then-use race and are not pinned by one. * docs(review): drop the JSDoc left stranded when redirectedAncestor moved Round 17 moved `redirectedAncestor` into `lib/worktree.ts` and deleted the copy here, but not the block above it — which then sat on `scratchWorktreesOf`, describing a function this file no longer has and, worse, describing the OLD behaviour: "the walk stops at the filesystem root" is exactly what round 17 changed, because `/var` is a symlink on macOS and an unbounded walk refuses every sweep there. * fix(review): close the round-19 Criticals — PR-content channels, and the guards this PR wrote for itself Two are reachable from a PR's committed content with nothing executed, which is the line this branch draws everywhere else: - **A committed `vendor -> ../stash` was never counted.** The rebuild walk discloses a `node_modules` symlink resolving back INSIDE the disposable tree and silently skipped one resolving OUT of it — so the farm reported `{linked: n, failed: 0}` while a `node_modules` under the link's target survived every rebuild. Node realpaths the importing file, so imports under the link resolve in the stash and decide later verdicts. Outside state cannot be wiped from here; it is COUNTED, which is what the contract promises. - **`workspaces: ["."]` widened the self-link whitelist to the whole shared worktree.** npm accepts a root manifest declaring itself a workspace and creates the self-link itself, `containedIn(root, '.')` answers the root, and every `node_modules` link resolving anywhere inside the review worktree then passed the verdict and was mirrored into the disposable tree as a read-write channel back. A member that resolves to the dependency root is not a member. Four more are this PR's own guards not being applied to the code beside them: - **`resetScratchTree` lstat'd only the immediate parent**, on the stated premise that `.qwen/tmp` is the one component above the leaf anything here can replace. False one hop higher: a link at `.qwen` redirects the whole path, and then every check in that gate agrees with every other because they all resolve through it — toplevel self-equality, common-dir equality, gitdir ≠ commondir, and even the backpointer round-trip. It walks every ancestor now, bounded at the repository the common dir belongs to. - **`restoreProbeTreeTracked` had none of the gates its sibling reset has.** A rewritten gitfile pointing at another repository passed `--show-toplevel` self-equality and the "restore" wrote that repository's content in; a skip-worktree bit made `checkout --force` skip a tampered file silently while the function answered "as the commit left it". Both are checks `scratch-tree` documents for the identical reset. The backpointer and ancestor checks apply to the linked-worktree shape a probe tree actually has — a standalone checkout has no admin entry to round-trip and IS the repository root. - **The revert phase collected probes screened once, from the index, before the baseline.** Every run since executes the PR's own test code, which can replace a probe with a link; the mutation writers re-check immediately before they write and this collection had no equivalent, so a relinked probe was collected THROUGH the link and scored against code the revert never touched. - **`runCleanup`'s symlink branch never pruned.** It returns before `releaseWorktree`, which is where the pipeline's only other prune lives — so the family paths were unlinked and reported swept while their registrations stayed behind to wedge the next `worktree add`. And two cheap ones with no argument against them: `sanitizedGitEnv` deleted by exact case, which removes nothing on Windows where env lookup is case-insensitive (the `shared-env-keys.ts` list it is modelled on folds case for this reason); and it now sets `GIT_NO_REPLACE_OBJECTS=1`, because one `git replace <sha> <evil>` in the common dir makes every `checkout --detach <sha>` here materialise someone else's tree while `rev-parse <sha>` still answers the original. Each fix is pinned by a test that goes red when that fix alone is reverted. * fix(review): close the round-20 Criticals — an empty probe list, a dropped refusal, a symlinked root The first one is a bug this branch shipped yesterday, and the worst kind: it turns a screen into a wider run. - **The revert phase could call `runProbeSuite` with an EMPTY list.** Round 19 added a screen that drops probes relinked out of the tree after the baseline; the `probes.length > 0` gate the phase opens with was taken before that screen could empty the list, and `vitest run` with no file argument collects the WHOLE suite — so "every probe was tampered with" became "score everything", with the verdicts attributed to files the phase never selected. It now stops the phase and says which of the two happened, once rather than per file. - **`cleanStale` discarded `releaseWorktree`'s refusal.** The guard added in round 17 declines to release through an ancestor symlink and reports why; `fetch-pr` called it as a bare statement, so the sweep looked successful and the next `worktree add` wedged at a path nobody was told about. The result is read and the reason printed, like every other failure on that path. - **The probe tree's own root was never lstat'd.** The ancestor walk added in round 19 starts above the leaf, and every identity comparison realpaths both sides — so a probe tree that IS a symlink into the shared review worktree agrees with itself all the way down, and the restore's `checkout --force` and `clean -ffdx` would have run in the tree every other agent is reading. The first is pinned end-to-end by the fixture that already relinks its probes mid-run: before round 19 it scored that relinked probe `inert` — a fabricated verdict read through the link — and the assertion now names the phase-level refusal instead. Reverting the guard alone turns it red. --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>	2026-08-20 07:50:44 +00:00
ChiGao	02d303f849	feat(serve): persist prompt terminal ledger for cold-load reconciliation (#9426) ... * feat(serve): persist prompt terminal ledger for cold-load reconciliation Turn terminal events (turn_complete / turn_error) were synthesized by the ACP bridge and published over SSE only, so a prompt that was in flight when the daemon died could never be resolved after a restart: the cold load replay emits transcript chunks and carries no terminal evidence, leaving promptId-keyed orchestrators stuck on "unknown". Each session now owns an append-only sidecar ledger next to its transcript. The bridge appends one in_flight record at prompt admission and one terminal record at the single publishPromptTerminal exit (covering the close/kill/channel-crash/daemon-shutdown flushes) through an injected synchronous sink. Ledger writes are best-effort and never block prompt execution or teardown, and records carry only ids, states, and timestamps — no prompt text, user content, or paths. On a cold session load the serve layer reconciles prompts left dangling by a dead daemon: it classifies the transcript tail with the existing turn-interruption detector and appends a completed (stop reason reconstructed_from_transcript) or interrupted (code daemon_lost) verdict, guarded by an attribution check so an unattributable tail stays unknown (fail-closed). The load response gains an optional promptTerminals field with the trailing 64 terminal records, omitted entirely when the ledger holds no terminal evidence, and archive/unarchive move the sidecar alongside the transcript so evidence survives storage lifecycle. Design: docs/design/2026-08-19-prompt-terminal-ledger-design.md * fix(serve): tighten ledger reconciliation fail-closed semantics and complete sidecar lifecycle Address review findings on the prompt terminal ledger: - reconcile: fail closed on multiple dangling prompts (no synthesized terminal for the newest either); attribute the oldest dangling prompt only when the attribution guard skips settled admissions (fixes the [A if, B if, B cancelled] misattribution veto), the transcript's last write postdates the admission (temporal evidence), and a clean verdict is upgraded to interrupted when the model tail holds any functionCall part, id or not (id-less tool-call guard covering the detectTurnInterruption wire-pairing blind spot) - lifecycle: removeSessionFiles deletes the ledger in both archive states; archive/unarchive move it through a single getPromptLedgerPathForState helper with merge semantics when the destination already exists (append-and-unlink instead of a permanent split); move warnings carry full source and destination paths in both directions - scans: DataProcessor.scanChatFiles and usageHistoryService.rebuildFromSessionJsonl exclude .ledger.jsonl sidecars (the ledger is not a transcript) - writer: appendPromptLedgerRecord seals a torn tail before appending so a torn fragment cannot fuse with (and destroy) the next record - tests: pin the new behavior across multi-dangling fail-closed, settled-then-queued attribution, valid interleave migration, temporal veto, id-less tool-call guard, sidecar lifecycle (move/merge/warn-only delete), torn-tail sealing, queued-admission flush on shutdown, active-prompt and resume load contracts, and ledger exclusion from insight scans - docs: sync the design doc's reconciliation algorithm, lifecycle, and fail-closed invariants * perf(serve): read only the ledger tail for load-response promptTerminals readRecentPromptTerminals ran on every POST /session/:id/load (including attached hot loads) and synchronously read and JSON-parsed the entire ledger — a multi-megabyte event-loop stall for long sessions on the per-request hot path. Add a tailBytes option to readPromptLedgerRecords that reads a trailing byte window (the first window line is always dropped: the window start can tear a line in half). The load path now reads a 256 KiB window, which holds hundreds of ~150-byte records against the 64-terminal response cap; sessions whose ledger outgrows the window return a best-effort trailing subset, which the response contract already allows. * fix(serve): close wrong-terminal attribution classes in cold-load reconciliation Strengthen the reconcile attribution evidence per review round 2: measure the temporal evidence on the same api-history projection the verdict uses, fail closed on a compression checkpoint written after the target's admission, and require the visible tail to postdate every other prompt's settled terminal (FIFO evidence). Also fix a TS18048 narrowing gap in the window test, make the seal test assert the raw file layout, and restructure the window test so the call-site tailBytes wiring is actually observable. * fix(serve): keep ChatRecord import inline so lint-staged cannot merge it into a type-only import * fix(serve): fail-closed reconciliation on millisecond clock equality and deadline-overlapped turns * fix(serve): TOCTOU fence before ledger append and documented residual attribution risk * test(serve): pin the ledger race fixture on the transcript timeline * feat(serve): bind cold-load evidence to the admission via a dispatch marker * test(core): pin the ledger sidecar exclusion in usage rebuild * fix(serve): create ledger sidecar owner-only and fence marker-era compression by position Round-7 review Criticals: - appendPromptLedgerRecord created the sidecar with umask-default permissions (0o644) while the adjacent transcript is owner-only; the ledger now follows the 0o600 convention at creation time. - Marker-bearing admissions fence post-admission compression by marker position instead of wall clock, so a backward clock step cannot hide a compression reset that voids the evidence chain. - Design doc: the residual-risk claim is corrected — the dispatch marker binds ordering, not ownership; the two ownership classes that survive it (recordless predecessor with continued writes, ledger-less cross-client writer) are documented, pending writer identity on transcript records (#9483). --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>	2026-08-20 07:27:49 +00:00
Shaojin Wen	43aea24550	fix(cli): give the case-variant delete test the attachments mock (#9551) ... `deleteDaemonSessions` takes `bridge: Pick<AcpSessionBridge, 'closeSession' | 'deleteSessionAttachments'>` since #9477, which updated every mock that existed when it was written. #9341 landed in parallel and added one more — "collapses case-variant spellings in one batch to a single delete" — with a `{ closeSession }` bridge. Each PR was green on its own merge ref; main is red combined, so `npm ci` fails the build for every branch cut from it: src/serve/server/session-archive.test.ts(1069,7): error TS2741: Property 'deleteSessionAttachments' is missing in type '{ closeSession: Mock<Procedure> }' Adds the same `vi.fn().mockResolvedValue(undefined)` its neighbours already pass. The test asserts on the delete result, not on the spy, so its meaning is unchanged: 48/48 still pass.	2026-08-20 06:57:15 +00:00
易良	48b30647d0	refactor: centralize cross-package contracts (#9497) ... * refactor: centralize cross-package contracts * fix(build): harden cross-package contract checks * docs(core): clarify sub-session prompt limit scope	2026-08-20 06:24:41 +00:00
ytahdn	50a0d2a761	fix(web-shell): keep sidebar sessions synchronized (#9533) ... Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com>	2026-08-20 06:21:03 +00:00
ytahdn	a41d5ec058	feat(web-shell): unify file uploads and references (#9477) ... * feat(web-shell): unify file upload and reference flow * fix(web-shell): address attachment review feedback * fix(web-shell): address attachment review feedback * fix(web-shell): address attachment review feedback * fix(webui): restore optimistic text prompts --------- Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com>	2026-08-20 06:18:34 +00:00
jinye	a659539bc7	feat(cli): Add standalone conversation isolation primitives (#9341) ... * docs: finalize standalone PR2 core design Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * feat(cli): Add standalone conversation isolation primitives Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): block mixed-case standalone restore Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): fail closed on corrupt session metadata Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(cli): repair PR2A CI doubles and identity replacement cases The ubuntu Test job failed on ten PR2A cases that pass on macOS: - Export SessionIdCaseConflictError from the worktree test's core mock and give its SessionService double findSessionIdIgnoringCase, since loadSession now resolves persisted spelling before reading metadata. - Add readCreationMetadataIfReadable to the Live task fake and mirror it onto the three server lifecycle spies so the fail-closed store interface sees the same fixture metadata as the legacy tolerant readCreationMetadata path it replaced. - Pin the original inode via rename in the two same-path replacement cases. ext4/overlayfs recycle a freed inode immediately, so rm+mkdir at the same path could satisfy the recorded device+inode identity on Linux runners and make a real replacement look valid. * fix(serve): key restore shared guard on persisted session id spelling The restore handlers resolved the persisted (possibly uppercase) spelling of a session id only inside the shared coordinator guard, while batch delete locks its exclusive guard on the raw caller ids. A restore of the normalized request id therefore raced a concurrent batch delete of the persisted-spelled id on case-sensitive volumes. Resolve the persisted spelling before acquiring the shared guard and key runSharedMany on the resolved id so both sides contend on the same key, in both the REST and ACP restore handlers. Regression tests assert the guard key at both transports. * fix(serve): unify persisted-session conflict contract across restore surfaces - SessionIdCaseConflictError now carries an optional candidateSessionId with a shape-aware message, so a same-spelling active+archived conflict names the persisted spelling instead of blaming the request-case id. - REST/ACP-HTTP conversion re-checks the candidate spelling before mapping to SessionConflictError; ACP child surfaces both shapes as INTERNAL_ERROR + errorKind 'session_conflict', and the reserved-source rejection carries errorKind 'reserved_session_source'. - Pin the previously ungated guards from review: lineage validity conjuncts and archived-state reads (session-source), persisted-spelling adoption in ACP load/resume, ensureStandaloneDirectory EEXIST raced re-inspection, and the trailing root revalidation inside inspectConversationDirectoryIdentity (fs-interception seam). * docs: sync standalone PR2 plan and design docs with R1 review - Plan doc: ship the readCreationMetadataIfReadable signature in the store snippet, record jsonl-utils/error-response/readCreationMetadata in the PR2A checklist with the second-core-file re-audit outcome, name the launcher as child-UUID generator in both sections, extend the PR2A vitest and prettier gates to every PR-touched file, and stop claiming junction/Windows coverage the matrix cannot run. - Design doc: reconcile the initial-prompt ordering clause with the strict create schema (prompt admitted by createWithInitialPrompt after the create transaction commits), replace the stale "before the UUID can be released" wording with the terminal-reservation model, and enumerate all seven deny categories in the acceptance matrix. * fix(cli): normalize session-map lookups in restore failure cleanup guards The sessions map is keyed by normalizeSessionIdForLookup-folded ids, but the three cleanupAfterRequestFailure guards read it with the raw config.getSessionId(). After a restore adopts a non-canonical persisted spelling (uppercase legacy transcript), those reads always miss and the guard would treat a still-stored session as unstored, shutting its config down in the double-cleanup-failure window. Normalize at the read sites; no behavior change for canonical or non-UUID ids. The adoption test now also pins that caller-case follow-up operations (cancel) still reach the adopted session through the normalized key. * fix(cli): snapshot standalone dir entries after final identity inspect prepareStandaloneDirectory read the child entries before the trailing identity re-inspection, so a same-uid entry appearing between the two steps would not flip the not_empty verdict. Read entries after the final inspect so the emptiness check runs on the freshest snapshot the identity machinery can guarantee. Addresses yiliang114's review on PR #9341. * fix(cli): lock restore guard on both request and persisted session id spellings Restore keyed its shared SessionArchiveCoordinator guard only on the resolved persisted spelling while batch delete/archive/unarchive lock on raw caller ids, so on a case-insensitive filesystem a delete carrying the request-case spelling never collided and could unlink the transcript mid-restore (R2-1). Lock both spellings on the REST and ACP-HTTP restore surfaces. Also from R2 review: - Pin the pre-guard/in-guard conflict-conversion stages in the both-states restore tests (R2-8: call-count + guard-not-entered assertions; mutation witness supplied by the reviewer). - Add the toRpcError SessionIdCaseConflictError producer case to dispatch-error.test.ts and list the suite in the PR2A verification block; extend the PR2B block with the collocated suites its checklist modifies (R2-5). - Correct the plan checklist label for the pre-existing shared jsonl-utils module from Create to Modify (R2-7). * fix(cli): canonicalize session-archive coordinator lock keys The two-spelling restore guard from the previous commit closed only the enumerated spellings: any third case variant of a caller id took an exclusive key that collided with no held guard, and on a case-insensitive filesystem could unlink the transcript mid-restore — including the common case where request and persisted spellings coincide and the lock set collapses to one key (R3 review, probe- verified on a case-insensitive mount). Canonicalize lock keys with normalizeSessionIdForLookup at the coordinator boundary (runSharedMany / runExclusiveMany / assertNotTransitioning) so every case variant of a session id contends on one key, and revert the restore-side spelling enumeration it makes redundant. Add a coordinator-level regression test for the case-fold collision, and fix a misleading comment above the in-guard call-count assertion (R3-1). * test(cli): pin workspace ordering/race propagation and align PR2 plan From the R4 review: - Pin that prepareStandaloneDirectory reads entries after the final identity re-inspection, via an interposed inspect that plants an entry mid-sequence (R4-3; mutant-verified to flip). - Pin that ensureStandaloneDirectory propagates a raced 'compromised' inspection verbatim instead of collapsing it to identity_changed (R4-7; mutant-verified to flip). - Plan: daemon bridge live-entry lookups (including getSessionEventEpoch) use the canonical ID — acp-bridge byId.get is exact-match with no id normalization — while the storage spelling is confined to SessionService filename/directory-hash/ACP-child storage operations (R4-1). - Plan: declare the session-archive coordinator lock-key canonicalization in the PR2A inventory and run session-archive.test.ts in the PR2A block (R4-2); add dispatch-error.test.ts to the PR2B block (R4-5). * fix(core): make case-insensitive resolver conflict decisions content-based The resolver threw SessionIdCaseConflictError on filename enumeration alone, before any content validation, and silently dropped a single candidate whose head recovered no records. Two probe-verified failure modes from the R5 review: - A present-but-unreadable case-variant transcript (torn/empty/ foreign-project head) resolved to undefined, so create admission admitted the canonical spelling and materialized a case-only twin; every later resolve then threw on the duplicate, permanently locking out the just-created session (R5-1). - A valid session with an unreadable same-spelling twin in the other state directory threw on enumeration while getSessionLocation cleanly reported one readable copy — listed as loadable, but every restore 409'd (R5-2). Conflict arms now consult getSessionLocation: exactly one readable spelling wins; conflict is thrown when two or more are genuinely readable (or a single candidate is conflicted across states); a candidate whose head fails validation still occupies the id when its file is on disk, while one that raced away mid-resolution resolves to undefined. Admission already maps the thrown conflict to persisted-true, so no admission change is needed. * fix(cli): make caller-supplied sessionId create admission case-aware The argv['sessionId'] branch (reached by raw stdio ACP session/new with a requestedSessionId, without any daemon reserveCreate) checked occupancy with exact-spelled sessionExistsInAnyState only, so a legacy mixed-case transcript did not block the create and the daemon persisted a case-only twin — which the resolver's conflict semantics then make permanently unrestorable on every surface (R5-2). Route the check through the case-insensitive resolver, treating its conflict throw as occupancy. Also pin that the ACP restore path hands the resolver-adopted storage spelling to assertSessionLoadable: archived uppercase transcript restored via the canonical lowercase id must surface errorKind 'session_archived' (R5-3; the request-spelling mutant skips the error on case-sensitive filesystems). * fix(cli): narrow reserved-source restore gate to internal runtimes Two items from the maintainer's round-2 live verification: - N1: the restore-side reserved-standalone-source gate fired for every runtime, so a transcript persisted on main with the client-supplied sourceType "standalone" became permanently unloadable while still listed. The create side already blocks new reserved-source transcripts, so any such file on an ordinary store predates the gate — keep it loadable there and hide only on the internal Conversations runtime (REST) / isolated ACP surface, where genuine standalone sessions will live. Generic-arm tests on both surfaces flipped to pin the compat restore; the internal-arm 404 pin is unchanged. - N2: the R5 occupancy throw reused the both-states message for a single unreadable transcript. SessionIdCaseConflictError gains a 'unreadable_transcript' reason with a truthful message, used by both occupancy arms; the case_conflict shape is unchanged. * test(cli): flip the remaining generic-surface reserved-source test The exact-spelling variant was missed in the N1 narrowing commit and failed CI on ubuntu (session/load + session/resume expected the old generic-surface hide). Flip it to pin the compat restore like its mixed-case sibling. * docs: sync PR2A plan with the shipped create-admission resolver consumer The round-6 triage deferred note flagged the plan as desynced: the R5-2 fix made loadCliConfig's caller-supplied sessionId branch a sixth findSessionIdIgnoringCase consumer. Declare it in the per-file checklist, count it in the consumer inventory, and add config.test.ts to the PR2A vitest block. * fix(core): narrow the session id case resolver's occupancy arms Three shapes were classified as permanent occupancy, regressing paths that loaded or created fine before the resolver replaced the exact-existence check: - Candidates were enumerated by case-insensitive filename match without the pattern gate that getSessionLocation applies, so an agent-suffixed id (which the CLI admits and writes under the raw session id) resolved to an unreadable-head conflict. Skip names the classifier would reject. - On a case-insensitive filesystem every spelling opens the same physical transcript, so a readable copy plus a torn case twin reported two readable candidates and raised a conflict for a session with one loadable copy. Collapse spellings that share a device/inode and resolve to the one whose own directory entry backs the file. - An unreadable head under the requested spelling itself is a case-only twin of nothing, yet it refused the id with no listing entry to delete or unarchive. Report it absent, matching getSessionLocation, so a first run that crashed before its first record can reuse its own 0-byte transcript. The twin-minting protection still applies when the persisted spelling differs from the requested one, and genuinely distinct readable spellings still conflict. Also pin the explicit-standalone child branch's sourceId guard and its documented lineage behaviour, which no case covered. * fix(cli): key the private conversation directory on the canonical id Restore derived the directory hash from the persisted spelling while the seven other materialize/discard call sites derived it from the lowercased live id, so restoring a legacy mixed-case transcript produced one directory and every later Live or task call produced a second, empty one — either orphaning what the first held or failing the call because the session sat outside its isolated directory. Rollback then inspected the other hash and leaked the first directory permanently. The directory belongs to the live entry, which the bridge registers under the canonical id alongside the lifecycle locks and in-flight maps, so both restore paths now derive it from that id too. This also keeps directories that pre-date the change reachable: restore used the lowercased request id before, so every one already on disk is canonical-keyed. Storage-facing operations — transcript filenames, metadata reads and the ACP child's own session storage — keep the authoritative spelling. The design doc and PR2 plan are corrected to scope the spelling rule accordingly. * test(cli): follow the canonical private-directory key in the Live restore case The internal-restore case pinned the directory hash to the persisted spelling, which the canonical-key change inverted. It now asserts the canonical id for the directory and bridge cwd, and keeps the original intent explicit by asserting that the creation-metadata read still uses the persisted spelling. * fix(cli): report proven parent lineage from the loadable-session reader The exported reader returned one verdict for two different situations: a child whose parent lineage it had verified, and an explicit standalone child whose parent it never read. PR2B is being built on that reader, so the ambiguity mattered even though no caller was affected yet. The verdict now carries the parent's own classification. An explicit standalone child whose parent is still readable must have a standalone top-level parent, which also rejects a grandchild or a lineage cycle because neither parent classifies as top-level. A parent that has been archived away or deleted keeps the child loadable — it is self-describing, and its own transcript is the evidence that a valid parent existed when it was created — but `parentSource` is then absent, so a caller that needs proven lineage rejects on that rather than guessing from `kind`. The compatibility adapter reads the new field instead of re-reading the store to re-derive the same classification, so its behaviour is unchanged while a duplicated location lookup and metadata read disappear from every legacy standalone child restore. Adapter output is identical for every input: explicit standalone was already filtered out before the parent check, so no current caller changes behaviour. * fix(core): stop the alias resolver from turning I/O and missing inodes into conflicts Two defects in the case-variant collapse added earlier in this branch. The stat guard was statically dead: `statSync` without `{ bigint: true }` always returns numbers, so the `typeof` test could never fire. The hazard it was meant to cover is a filesystem that exposes no inodes — FAT/exFAT and some SMB mounts report `ino === 0` for every file — where `dev:ino` collapses genuinely distinct transcripts onto one identity and a real two-transcript conflict silently resolves to one spelling. That is a fail-open on a correctness decision, so it now uses the existing `hasVerifiableInode()` helper, whose docblock describes exactly this case. The resolver also swallowed every `statSync` failure into `undefined`, which its caller reads as positive proof of a conflict. A transient EACCES or EMFILE therefore surfaced as `409 session_conflict` — a permanent-looking answer for a blip that succeeds on retry. Only ENOENT is now treated as meaningful: a transcript that raced away is no longer a competing spelling. Every other error propagates. * fix(core): let a crashed first run resume its transcript past a case twin The self-escape added for an unreadable transcript under the requested spelling only covered the single-candidate arm. Once any case twin was enumerated, resolution took the all-unreadable arm instead, where presence was computed over every candidate including the requested spelling's own file — so the documented crash recovery vanished the moment a stale twin existed, and neither file could be deleted because both classify as nonexistent. Reusing an id whose file is already on disk mints no case-only twin, so that arm now takes the same escape. A twin under a different spelling still occupies the id, because minting the requested spelling beside it is what would make both unrestorable. The disappearance test was vacuous: its candidate spelling equalled the request, so it returned at the self-escape and never reached the race loop it named — deleting that loop left it green. It now uses a differing spelling with `existsSync` false, and forcing the loop to throw unconditionally kills it. The all-unreadable rejection test likewise needed two spellings that are both distinct from the request to exercise twin-minting protection. * fix(cli): fail closed when a filesystem cannot prove directory identity The conversation-directory checks compared `dev`/`ino` directly, so on a filesystem that exposes no inodes — FAT/exFAT and some SMB mounts, where Node reports `ino === 0` for every entry — every directory compared equal. The root pin, the two anti-swap re-probes around `realpath`, and the expected identity check would all confirm a directory that had in fact been replaced, which is the swap those probes exist to catch. They now require a verifiable inode on both sides before treating a match as proof, reusing the `hasVerifiableInode()` helper already written for this in core and exporting it from the package surface. An unverifiable inode reads as a changed identity rather than as a match. The regression test pins a root whose inode is also 0, so a plain `===` comparison still matches and only the verifiability guard can fail it. * fix(cli): keep caller-supplied session-id admission fail-closed on I/O errors Swapping the existence check for the case-insensitive resolver narrowed the catch to `SessionIdCaseConflictError` and rethrew everything else, but the resolver deliberately propagates non-ENOENT `readdir` and transcript-read failures. An unreadable chats directory therefore killed startup with a raw EACCES or ENOTDIR instead of the guarded message, and bypassed the `throwOnSessionIdConflict` contract the ACP path depends on. The previous check answered "occupied" for any read failure. Restoring that keeps an unprovable id on the guarded path; distinguishing "cannot determine" from "occupied" would be a new response shape and is left alone here. * fix(cli): keep the directory identity module out of the core package barrel Importing `hasVerifiableInode()` from the core package barrel pulled core's whole module graph into the serve pre-listen bundle closure, so `check:serve-fast-path-bundle` reported glob, chokidar, fzf, @iarna/toml and the core shell tool runtime as statically reachable from `run-qwen-serve`. This module is deliberately dependency-free for that reason. The predicate is restated locally with a comment recording why it is not imported, since core has no subpath export for it. The barrel export added for that import is reverted so the package surface is unchanged. * fix: correct three defects introduced by the previous review round **The inode guard made a directory fail to equal itself.** `createConversationRootIdentity()` compares `before`/`after` of the same path, so requiring a verifiable inode threw `identity_changed` on the very first root establishment and, because the workspace clears its cached root on failure, Conversations never started on exFAT/FAT or an inode-less SMB mount. "Cannot prove unchanged" is not "changed": the root is now established with `inodeVerifiable: false` recorded, comparisons fall back to device, canonical path and stat shape, and the weaker guarantee is explicit on the identity for callers to surface. Where inodes exist they are still required to match. **The occupancy escape was placed to discard a real twin.** It returned early for the whole arm whenever the requested spelling was enumerated, so a present-but-unreadable twin stopped occupying the id — the case-only twin the surrounding comment exists to prevent. The escape belongs per candidate, not per arm: the requested spelling's own file never counts as occupancy, every other spelling still does. **The private directory was still a caller obligation.** The comment claimed the bridge registers live entries canonically and every materialize derives from that id, but the bridge echoes whatever the caller passed, and `LiveTaskService.ensureResident()` passes an id that originates in a tool argument. `ConversationWorkspace` now canonicalizes before hashing, so one session resolves to one directory by construction. Also unifies the resolver's two arms, which were the same algorithm written twice — that duplication is why the escape landed in only one copy. * fix(cli): Collapse case-variant session ids in batch lifecycle and CLI create Batch delete/archive/unarchive locked on canonical keys but still deduped raw spellings, so two case variants of one id deadlocked the batch. CLI --session-id now stores the lowercase spelling so new mixed-case transcripts stop accumulating. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Cursor <cursoragent@cursor.com>	2026-08-20 06:15:52 +00:00
易良	4b750b5b13	fix(cli): suppress Homebrew update notification when brew has nothing newer (#9493) (#9502) ... * fix(cli): suppress Homebrew update notification when brew has nothing newer (#9493) * test(cli): strengthen homebrew update coverage * docs(cli): clarify Homebrew metadata scope	2026-08-20 06:10:32 +00:00
jinjing.zzj	c2bfc3b2d7	fix(core): invalidate round-1 IDE diff on PreToolUse ask bounce (#9434) ... A round-1 openDiff resolver that survives into a bounced round (sibling auto-approval never closes the diff) could answer the bounced confirmation through the status-only awaiting_approval guard, bypassing the bounced onConfirm protections (hideModify, dropBounceModifyPayload, claim guard). Bounce now bumps a per-call confirmation epoch and resolves/closes the outstanding round-1 IDE diff the same way the CLI's handleConfirm does; openIdeDiffIfEnabled drops its answer when the epoch has moved on. Adds accept/reject arm regression tests. (#9434 review R7-1)	2026-08-20 13:29:25 +08:00
易良	b219e3a716	chore(ci): Add --provenance to npm publish and id-token permission (#9532) ... * chore(ci): Add --provenance to npm publish and id-token permission * test(scripts): expect --provenance in npm publish step assertion PR #9532 adds --provenance to every npm publish in the release pipeline. Update the workflow-pinning test to match the new command so the helper test suite stays green.	2026-08-20 05:21:02 +00:00
Shaojin Wen	d4b54a46ff	refactor(review): route the certification path atoms through one needle (#9484) ... * refactor(review): route the certification path atoms through one needle `openedBrief`, `readBrief` and `readFindingsPointer` each inlined the same two lines — `JSON.stringify(path)` then `args.some(a => a.includes(...))` — which is the single mechanism enforcing the bar's "exact path, not a `${path}.bak` look-alike" guarantee. Three copies is the exact drift class this module was created to remove, turned on the module itself: a fix to the match (a serialization edge, path normalization, a stricter compare) would have to land in three places in lockstep or leave one atom silently weaker than its siblings. Extract a private `namesPath(args, path)` and have all three route through it. No behavior change — the existing `.bak`-trap pins in `certification.test.ts` and `check-coverage.test.ts` cover each atom and pass unchanged. * refactor(review): finish the needle consolidation, and keep its name its own Two findings on the first round, both about the helper this PR introduced. The consolidation stopped one copy short. `parseTranscript` still inlined the same exact-path predicate for the diff-read half of the bar, so the count went from four copies to two — while the new comment claimed a fix to the match reaches all of them at once. The match now lives in `transcripts.ts`, beside the code that serializes the args, and both halves call it: the diff-read half directly, the three brief/findings atoms through a wrapper that spreads it over a record's call list. `certification.ts` already imported from `transcripts.ts`, so this needs no layering inversion. The helper also took the name of a module-private `namesPath` in `utils/findings.ts` whose semantics are deliberately different — boundary matching over PROSE, which credits `rm /plan/chunk-3.brief.md` for naming the brief. Unifying the two the obvious way would make `openedBrief` credit an agent for deleting a file it never opened. They keep distinct names now (`serializedArgsNamePath`, `argsNameExactPath`), and both doc comments say why, so a grep for "the path matcher" cannot land a reader on the wrong one. Tests: the needle's own cases (whole JSON string value, no `.bak` sibling, no shell mention) and the `rm <brief>` shape at the `openedBrief` atom. Relaxing the needle to a bare substring now reddens BOTH suites from one edit — the property the consolidation exists to buy, and the one the second copy denied. * test(review): pin the shared needle's two untested arms, restore parseTranscript's doc Three review Suggestions on the needle consolidation: - `serializedArgsNamePath` had been inserted between `parseTranscript`'s JSDoc and its declaration, orphaning the doc onto the wrong function. Move it above the JSDoc so each doc sits with its own function. - The diff-read half (`parseTranscript` with a `diffPath`) was exercised by no test — every `readTranscripts` call site omits `diffPath` and nothing set `diffToolCalls` non-zero, so an arg swap at the call site shipped green. Pin it: the exact diff read counted, a `.bak` sibling and a shell command that only names the diff refused, and `diffToolCalls: 0` without a `diffPath`. - `argsNameExactPath`'s existential (`some`) was only ever run on 0/1-element arrays, so a first-element-only regression shipped green. Pin a match in the second position for `openedBrief` and `readBrief`. * test(review): pin the diff read's RANGE, not only its count The new diff-read fixture asserts `diffToolCalls`, which a mutation dropping the `range: namedTheDiff ? rangeOf(args) : null` wiring survives — the count stays right while every chunk-coverage ruling, which reads the lines rather than the tally, is handed an empty list. The fixture's exact read now carries an `offset`/`limit` and the test asserts `diffReads` equals `[[1, 40]]`; dropping the wiring reddens it.	2026-08-20 04:06:49 +00:00
qqqys	63fe7c174a	feat(core): expose workflow execution state (#9034) ... * feat(core): expose workflow execution state * fix(cli): initialize workflow snapshot state * fix(core): preserve workflow client gating * fix(core): snapshot workflows before journal drain * fix(core): sanitize workflow replay logs * fix(core): preserve workflow failure trace integrity * fix(core): validate workflow snapshot run IDs * test(core): cover workflow event boundaries * fix(core): isolate workflow run callbacks * fix(core): align workflow log projections and dependency tails (#9034) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): keep persisted workflow projections in agreement (#9034) --------- Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-08-20 03:41:40 +00:00
qqqys	36c77ff803	fix(dingtalk): attach media from quoted messages (#9347) ... * fix(dingtalk): attach media from quoted messages * fix(dingtalk): keep the reply text when attaching quoted media R1-1: `attachMedia`'s placeholder cleanup was written for the direct-media path, where `extractContent` generates `(audio)` / `(video)` / `(file: name)` itself. This PR made the quoted-media path reach it, and there `envelope.text` is the user's own reply — so a reply reading exactly like one of those placeholders was blanked and the agent got an attachment with no prompt. A group `@Bot (audio)` arrives here as exactly `(audio)`, the mention having been stripped upstream. `attachMedia` now takes the placeholder to erase as a parameter; only the direct-media call site passes one. R1-2: the same path newly routes text-only replies through the unguarded `mkdirSync`/`writeFileSync`/`basename` block. Those are synchronous throw sites — ENOSPC on a write of up to 50 MB, ENAMETOOLONG from a quoted fileName over 255 bytes (`basename` does not truncate), a TypeError from a truthy non-string fileName. An escape rejects `processMessage`, whose catch sends the generic error reply and never calls `handleInbound`; the msgId is already in `seenMessages`, so DingTalk's retry is deduped and the prompt is lost for good. The block now degrades the way a failed download already does: log, skip the attachment, deliver the text. This also covers the pre-existing direct-media path. Verified: dingtalk 310/310. Both mutation-checked — restoring the caller-blind cleanup fails 3 tests, letting the fs block throw fails 1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(dingtalk): address quoted-media review findings Unify the msgType->mediaType mapping in a shared helper, make extractContent the single source of truth for the placeholder text cleaned on attach, and remove the store directory when a media write fails so failed stores no longer leak into tmpdir. Merge the stacked attachMedia JSDoc blocks, document quoted media downloads, and pin the previously uncovered paths: unmapped quoted msgTypes with a downloadCode, own-media + quoted-media combinations, direct placeholder cleaning, and the degraded-store attachment shapes. * fix(dingtalk): file-back a quoted image colliding with the own image * fix(dingtalk): give generated media store names a mime-derived extension (#9347) --------- Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot[bot]@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>	2026-08-20 03:18:32 +00:00
Zqc	bba2e1a184	feat(cli): Add agent view PTY workers (#7800) ... * feat(cli): add agent view supervisor runtime * feat(cli): add agent view pty workers * fix(cli): harden agent view supervisor runtime * fix(cli): harden agent view pty workers * fix(cli): surface supervisor startup failures * fix(cli): handle supervisor subscription failures * fix(cli): address review feedback on agent-view supervisor (#7799) - Narrow AgentViewSupervisorSubscriptionOptions to omit timeoutMs, which the subscription never reads - Throw when DEV=true with a .ts entrypoint but tsx is missing, instead of silently spawning a process that will crash - Add test for the qwen fallback path when argv[1] is undefined - Add test for worker sideband auth bypass (workerEvent without token) * fix(cli): address review feedback on agent-view supervisor (#7799) * fix(cli): address review feedback on agent-view supervisor (#7799) * fix(cli): address review feedback on agent-view supervisor (#7799) * fix(cli): bind streaming supervisor handlers and keep attach stream 8-bit clean (#7799) * fix(cli): pause attach socket during leftover write and harden error handlers (#7799) * fix(cli): harden supervisor shutdown tests and clean up signal listeners (#7799) Pass the auth token to expectSupervisorUnreachable so it distinguishes a live auth-gated supervisor from a closed socket, remove SIGTERM/SIGINT listeners on the non-signal shutdown path, exercise prepareSocketPath's mkdir via a nested socket path in the permission test, and add coverage for the 1 MB request-line size guard. * fix(cli): guard fire-and-forget promises and add supervisor timeout test (#7799) * fix(cli): normalize socket path test prefix and force credential file mode (#7799) * fix(cli): address agent view runtime review feedback * test(cli): cover agent view bridge edge cases * test(core): isolate worktree symlink traversal fixture * Revert "test(core): isolate worktree symlink traversal fixture" This reverts commit 09f6d4404b. * fix(cli): harden agent view supervisor store, auth, and streaming (#7799) * fix(cli): harden agent view store joins, writes, and coverage (#7799) * fix(cli): harden agent view credential writes and subscription handshake (#7799) * fix(cli): harden agent view store normalization, roster safety, and bridge backpressure (#7799) * fix(cli): guard response serialization in supervisor server (#7799) * fix(cli): harden agent view pty host teardown * fix(cli): fail fast on pty host auth rejection * test(cli): cover pty host remote exit polling * fix(cli): address agent view pty worker review nits * fix(cli): harden pty host socket fallback * fix(cli): harden agent view pty workers * fix(cli): harden agent view pty workers * test(cli): cover pty host spawn contract * fix(cli): guard agent view pty host socket takeover and races * fix(cli): fit agent view pty logs under wire cap, strip host token after merge * fix(cli): harden pty host fallback selection * fix(cli): address agent view worker review blockers * fix(cli): bound attach lease ttl and surface pty host input/listen failures * fix(cli): serialize pty host socket ownership and keep attach transport byte-transparent * fix(cli): escalate pty host shutdown and harden socket path fallback * fix(cli): harden win32 kill, decoder reset, and exit tracker - kill/shutdown pass no signal on Windows (WindowsTerminal.kill throws for any signal string; argument-less kill terminates conpty tree) - input StringDecoder is reset at attach boundaries so incomplete UTF-8 bytes from a prior session cannot leak into the next - createRemoteExitTracker requires two consecutive probe failures before resolving exited, preventing false death on a single transient timeout - defaultSpawnPtyHost pairs detached with windowsHide: true - stringifySandbox object branch test, shutdownHost else-branch test, win32 kill guard test, decoder reset test * fix(cli): skip POSIX signal tests on Windows and reclaim empty lock files R9-1: 'passes kill signals' and 'gracefully shuts down with SIGTERM' assert POSIX signal pass-through, but the R8-36 fix made Windows pass undefined. Skip both on win32 to prevent merge-queue CI regression. R9-2: A zero-byte lock (process killed between writeFile open and pid write) yielded parseInt('') = NaN, which failed Number.isInteger and never entered the reclaim branch. Flip the condition so non-integer/empty locks are also treated as stale. * fix(cli): remove abort listener on delay timeout path R10-1: delay() added an abort listener with { once: true } but only removed it when abort fired. When the timer resolved first, the listener stayed registered on the shared AbortSignal. Extract onAbort into a named function and removeEventListener on the timeout path. * fix(cli): don't optimistically resolve exited on SIGINT for remote handles wenshao's local verification (probe P19) showed that kill('SIGINT') on a reconnected handle (child === undefined) resolves exitTracker immediately, even though the worker can trap SIGINT and survive. Only resolve for terminal signals (SIGKILL, SIGTERM, default); let createRemoteExitTracker's status poller observe whether the worker actually exits after SIGINT. * test(cli): add SIGINT carve-out test for remote handle kill Exercise the 'allowedSignal !== SIGINT' guard: connect a childless handle, kill('SIGINT'), assert exited does not settle within 100ms, then kill('SIGTERM') and assert it resolves. Mutation-verified: deleting the guard makes this test fail. * fix(cli): Only resolve exited on SIGKILL for remote handle kill SIGINT/SIGTERM can be trapped and survived; the server-side kill op has no SIGKILL escalation, so optimistically resolving exited for them could report a still-alive worker as dead. Only SIGKILL is untrappable and guarantees the worker is gone, so resolve immediately for it alone and defer other signals to the remote exit poller. * fix(cli): harden agent view PTY host socket lock and kill default The pid lockfile reclaim path could double-acquire: two concurrent reclaimers of a stale lock could both proceed (the later rm deleting the rival's fresh lock), and a rival could reclaim a live writer's lockfile during the O_EXCL create-then-write window — letting two hosts race through prepare→listen and orphan the displaced one. Re-read before removing a stale lock, only release a lock that still contains this pid, and re-verify ownership after listen, failing closed with EADDRINUSE when displaced so at most one host serves the socket. Also pin the signal-less kill op to SIGTERM instead of node-pty's POSIX SIGHUP fallback, which bypasses ALLOWED_KILL_SIGNALS and is commonly ignored. * fix(cli): fix agent view PTY host dispose signal, ring trim, and lock loop dispose() used node-pty's signal-less kill, which falls back to SIGHUP on POSIX and is ignored by nohup-style workers; align it with shutdown() by passing SIGTERM. BoundedOutputRing.trim() ran the leading-continuation-byte re-sync unconditionally, discarding leading bytes (and inflating droppedBytes) when the window never overflowed; gate it on a size trim actually running. The socket lock loop's attempt bound made a successful reclaim on the final iteration throw EADDRINUSE instead of retrying the freed lockfile; loop until the create wins or a live holder is confirmed. Also consume the required-but-unused ACTIVE_CWD sideband field as the state report's cwd fallback, which cannot throw ENOENT like process.cwd(). * fix(cli): settle remote pty host exit only after the RPC lands * fix(cli): optional-call shutdown in the lost-RPC regression test * fix(cli): wait for PTY host endpoint shutdown --------- Co-authored-by: 俊良 <zzj542558@alibaba-inc.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: yiliang114 <effortyiliang@gmail.com>	2026-08-20 03:11:57 +00:00
Shaojin Wen	3b3818db87	fix(ci): keep qwen-autofix.yml under GitHub's 500 KB start-runs limit (#9517) ... GitHub does not start runs for a workflow file larger than 500 KB (512,000 bytes) and reports nothing when it stops. qwen-autofix.yml crossed that line on 2026-08-19 at 512,782 bytes: schedule ticks stopped firing, every workflow_dispatch sat "queued" forever with zero jobs and could not be cancelled, and issues/issue_comment went quiet — while pull_request_review runs kept succeeding, because a PR event resolves the workflow from the PR's own branch and those carry older, smaller copies of this file. The loop therefore looked half-alive and stayed dark for a day. Move 75 long comment blocks (1,326 lines) verbatim into a sibling design record, .github/workflows/qwen-autofix.md, leaving each block's opening lines plus a `qwen-autofix.md#af-NNN` pointer where it sat: 518,055 -> 426,437 bytes. No executable line changes — the YAML parses to an identical document outside `run:`, every `run:` script still passes `bash -n`, and the only lines removed anywhere are comments. Steps that are duplicated verbatim across jobs share one pointer so they stay byte-identical. Add .github/scripts/check-workflow-size.sh (gate at 470,000 bytes), wired into CI on every profile: a .github-only PR classifies as `github_ci_only` and skips the `full`-only checks, which is exactly the PR that can trip this. Tests pin the gate, every workflow's size, and pointer/section symmetry. Delete qwen-autofix-recovery.yml. It was cloned during the incident on the theory that the workflow ENTITY was wedged, but it carried the same oversized file, so its dispatches queued identically and its schedule never fired.	2026-08-20 01:56:45 +00:00
jinjing.zzj	611d9959c0	fix(core): close round-6 ask-bounce findings (#9434) ... R6-1: a bounced re-execution skips the PreToolUse hook, so the confirmation surfaces while bounced must not change what runs: - the IDE diff is no longer opened on a bounce; it answered through the pre-wrap details, bypassing dropBounceModifyPayload - ModifyWithEditor is rejected while bounced (the call stays in awaiting_approval; Cancel/Proceed still recover) - the hook-reviewed request.args snapshot is restored before the bounced re-execution, closing the stream-json responder channel that mutates request.args directly before answering R6-2: the bounced plan-shell branches now use the ordinary confirmation phase's planShellResponseClaimed guard (checked and set synchronously before the validation await), so racing answers to the same bounced call collapse to one handleConfirmationResponse.	2026-08-20 09:28:45 +08:00
Shaojin Wen	f3cb3a11ab	feat(review): add runtime-axis, table-sweep and isolation witness forms (#9445) ... * feat(review): add runtime-axis, table-sweep and isolation witness forms The witness rule's forms all answer "what does this input produce": probe runs a unit harness, base-tree runs one input on two trees, extract-step executes a lifted run: body, the sweep iterates a population. Three common claims fall outside that shape. A forward-compatibility claim is unfalsifiable on the one runtime the harness happens to be running, and a green matrix is only evidence about the versions in it. base-tree's version axis is git; this adds the runtime axis, capped at one other version and priced at a tarball rather than an install and a build. A hardcoded table mirroring another system's namespace looks like data rather than logic, so it gets checked by reading it against a list somebody retypes — which is the mirrored oracle the sweep already rejects, wearing a disguise. Parsing the literal out of the source and diffing it against the authority at runtime makes both directions visible, and the direction that ships is the one no test written against the table can see. A claim about an aggregate invites a per-component dump, after which the verdict rests on code the review itself wrote. Removing one contributor and re-reading leaves both numbers coming out of unmodified code, and settles sum-versus-maximum in a single pair of readings. None of the three needs a new CLI surface: the witness field already carries free text, so the witness rule's enumeration of forms is the only other line that had to move. * fix(review): teach the three new witness forms to the verify brief Round 1 review, R1-1: the Step 4 verifier never reads SKILL.md — its whole instruction set is the brief agent-briefs.ts builds — and every pre-existing form in the witness rule's enumeration has a capability paragraph there, including the impact sweep (phrased "sweep the real population", which is why a search for the SKILL.md name missed it). The three new forms had none, so the only agent that produces witnesses could not reach them. They now sit next to their kin: the table sweep after the population sweep, the version axis after the base-tree A/B, elimination after the drive block, and all three in the brief's own witness-form list. Also from round 1: - The subtraction rule was wrong for the aggregate the worked example actually is. A difference is the removed contributor's value only under a sum; under a maximum, removing a non-holder moves nothing and removing the holder exposes the next-largest, so the stated rule would have had a verifier quote 0.2 MB as a child's heap. Both files now read the pair for the combining rule, and name reducing the population to one contributor as what yields a value outright (R1-10). - Whether a silent contributor is skipped or counted as zero is invisible under a sum and a maximum alike; it shows only where a zero moves the figure. Scoped to counts, denominators and averages (R1-11). - The two-child reading is quoted the same way in both files now, and no longer claims to prove more than it does: flat under addition excludes a sum, not every alternative (R1-8, R1-9). - Node 22 exposes eight old-generation names, not ten; the ten-name set is the classifier's, and the two Node 24 adds are named (R1-7). - One name for the capability, "version axis", matching its defining paragraph and the DESIGN.md heading (R1-3), and the witness rule no longer points below for a form that is above it (R1-2). - The enumeration trap is conditional on an unbounded entrance space, and most of these tables are bounded — the lens's own carve-out (R1-5). - "Installs the other version" against "no install and no build" read as a contradiction; the cost is a download, no dependency install (R1-6). - The one-version cap and "every version the repo supports" contradicted each other. Both now say: the versions the claim names, which for a support range is the floor and the newest (R1-4). --------- Co-authored-by: wenshao <nigolaschao777@gmail.com>	2026-08-20 01:16:36 +00:00
jinjing.zzj	2e64af9dd0	fix(core): close round-5 ask-bounce findings (#9434)	2026-08-20 04:09:34 +08:00
jinjing.zzj	60db2599ab	fix(core): close round-4 ask-bounce findings (#9434) ... Carry the confirmation-phase plan-shell decision into the execution phase via a per-callId map so the PreToolUse ask bounce compiles and re-applies decoratePlanModeShellConfirmation / validatePlanModeShell Approval (R4-1, R3-2). Re-enter the captured invocation context around the bounced onConfirm so the tool-invocation guard sees the invocation's identity, not the responder's (R4-4). Collapse the two inline cancel-before-execution copies into cancelWithSyntheticResponse (R3-5) and correct the bounce docstring (R3-4). shell: keep a prepared/confirmed sed edit authoritative when a re-entrant preview fails instead of flipping to raw execution (R4-3), and present the retained confirmed content in the re-confirmation view so it matches what approval writes (R4-2). cli: render hookAskReason on the stream-json permission suggestions and the ACP permission request content (R3-3). Tests: hook-reason cap on short terminals (R3-7), exec-branch hookAskReason passthrough (R3-8), abort-resolves and reason-fallback bounce variants (R3-9), sed retention across a re-entrant preview (R4-5), structured-bounce decline (R4-6), plan-shell bounce through the scheduler (R4-7).	2026-08-20 00:05:26 +08:00
Zqc	39fc769d3a	feat(cli): show loaded context files alongside the first prompt (#8855) ... * feat(cli): show loaded context files alongside the first prompt When context files (QWEN.md / context.fileName) are attached to the system prompt, surface a one-shot INFO line above the user's first prompt listing exactly which files were loaded, so users can verify discovery (e.g., catch typos in context.fileName) without digging into debug logs. Also shorten display paths for files under the user home to `~/...` in both the announcement and the /context detail breakdown. Fixes #5267 * fix(cli): address review feedback on context file visibility - Resolve memory-marker paths against the session working directory instead of process.cwd() in /context detail (ACP/daemon sessions) - Sanitize display paths with stripAnsiAndControl before they reach the terminal - Drop empty context files from the announced list to match concatenateInstructions' empty-content filter - Delegate home-dir shortening to tildeifyPath (with an optional home override for tests) instead of a second prefix-check implementation - Align the one-shot announcement guard with downstream input classification (trim, /btw, shell mode) - Add tests for the announcement latch, parseMemoryFiles ~ shortening, CWD-under-home and home-prefix-collision cases; use shared-volume fixtures so tests hold on Windows * fix(cli): address second-round review feedback on context file visibility - Skip the announcement latch for blank submissions (dropped by the message queue before reaching the model) - Treat Windows cross-drive relative results (absolute paths) as outside the CWD tree so home-dir files still get `~` shortening - Extract the hasAttachedContent predicate shared by concatenateInstructions and contextFilePaths so "displayed = attached" holds by construction - Thread the loader's resolved userHomePath through formatContextFileDisplayPath into tildeifyPath so discovery and display agree on the home directory - Pin ordering (announcement precedes submission admission), add a whitespace-only-file filter test, exercise workingDir != cwd in the /context detail test, and cover the /directory add reload wiring * fix(cli): address third-round review feedback on context file visibility - Consume the one-shot latch on model-invocable slash commands: skills and MCP prompts are expanded into a submit_prompt that reaches the model, so excluding all slash commands deferred the announcement to a later plain prompt (or hid it entirely in skill-only sessions). - Build contextFilePaths from every attached file, not just memory-named ones, so the announcement matches what concatenateInstructions injects (extension context files with custom basenames were attached but unannounced while /context detail listed them). - Correct the contextFilePaths JSDoc: entries are display paths (CWD-relative or ~/... shortcuts), not paths to resolve against the CWD. - Tests: model-invocable skill first turn consumes the latch; shell-mode submissions do not; extension files with custom basenames are announced. * fix(cli): address fourth-round review feedback on context file visibility - Replace the order-dependent shell-mode test with a pure predicate (consumesContextAnnouncementLatch) and hermetic unit tests: the old test failed deterministically when run in isolation because the React harness state toggle depended on earlier tests' side effects. - Keep latch consumption correct while shell mode is active for slash commands, which route before the shell intercept: model-invocable ones (skills, MCP prompts) still consume it. - Consume the latch only when something was actually announced, so files attached later in the session still get their one-shot notice. - Skip latch consumption for queued (deferUntilIdle) submissions, which are admitted only after the queue drains. - Re-arm the latch on same-process session switches (/clear) via an effect keyed on the session id. - Classify CWD containment with isSubpath instead of startsWith('..'), which misclassified in-tree directories named like '..cfg'. - Document the deliberate exclusions: baseline rules are not announced (see ruleCount), and the Windows cross-drive arm is consciously untested on POSIX CI. * fix(cli): address fifth-round review feedback on context file visibility - Revert the deferUntilIdle exclusion (R4-8): it silently broke the announcement for queue-only submissions, which never pass back through handleFinalSubmit once the drain admits them. The latch is now consumed at queue time; emitting at the drain admission choke point is a deeper refactor deferred for this feature. - Re-arm the latch on Ctrl-L clear-screen, which wipes the emitted INFO item without a session switch (completes the screen-clear case R4-13 named). - Soften the predicate docstring and the guard comment to be honest about the heuristic: btw is deliberately exempt (a side question that doesn't advance the main conversation, even though it may fork a model call), not "bypassing the model"; consumption is a prediction, not an admission guarantee. - Correct the ANSI-stripping test comment to the real mechanism (stripVTControlCharacters matches the ESC[2Jb…BEL run as one BEL-terminated sequence), so a future maintainer doesn't misdiagnose. * fix(cli): re-arm context-file announcement latch on conversation rewind - Rewind (/rewind / double-Esc) filters history to before the target turn via loadHistory, wiping the emitted INFO item without a session switch. Add a conditional re-arm in handleRewindConfirm: the latch re-arms iff the rewound history no longer contains the announcement, so rewinding past it re-announces on the next prompt while rewinding to a later turn doesn't duplicate it. Completes the wipe-path coverage alongside /clear (session id effect) and Ctrl-L (handleClearScreen). * fix(cli): correct rewind latch polarity and cover Ctrl-L re-arm - R6-1 (Critical): the rewind latch re-arm shipped inverted. Latch semantics are true=consumed / false=armed; the correct assignment is truncatedUi.some(announcement) — true (stay consumed) when the INFO survives the rewind, false (re-arm) when it was filtered out. The negation inverted both branches: rewinding past the announcement left the latch consumed (no re-announce), rewinding to a later turn armed it (duplicate). Drop the negation. - R6-2: add 're-arms the latch after Ctrl-L wipes the INFO' — submit, handleClearScreen, submit again, assert two announcements. Locks the R5-8 Ctrl-L re-arm; removing that line now fails the suite. * fix(cli): address R8 review on context-file announcement - AppContainer.tsx: performMemoryRefresh anchors on getWorkingDir() not process.cwd(), matching the /context read site (R1) and ACP/daemon under skipProcessChdir (R8-5). - commandUtils.ts: docstring drops the false "MCP prompts" claim (McpPromptLoader has no modelInvocable field) and the inline comment names the disableModelInvocation / description-less exemptions instead of a false exclusivity (R8-4, R8-7). - AppContainer.test.tsx: renderRewindHarness takes optional history / contextFilePaths and gains two rewind tests — past the announcement re-arms (re-announces), retaining it stays consumed (no dup); each fails under its !some/some mutation, covering the R6-1 inversion (R8-1). - directoryCommand.test.tsx: strengthen the loadServerHierarchicalMemory assertion to pin the getWorkingDir() anchor (R8-6). R8-2 (defensive reset) and R8-9 (/cd plumbing) declined: the reset is on a path that can't hold stale values (safe-mode early return); /cd needs cross-file plumbing, same class as the deferred /restore — both are subsumed by a self-healing latch follow-up. * fix(cli): address R9 review on context-file announcement - commandUtils.ts: narrow the btw exemption to /btw only. ?btw is not a slash command and goes to the main model as a plain query, so it must consume the latch. Updated 2 tests that pinned the old ?btw exemption (R9-2). - AppContainer.tsx: document the /cd latch gap in the consume-point comment — relocateWorkingDirectory swaps the file set without re-arming, deferred to the self-healing latch follow-up (R8-9). - AppContainer.test.tsx: add performMemoryRefresh anchor test that mocks loadHierarchicalGeminiMemory, captures the callback from useGeminiStream mock args, and asserts the first arg is config.getWorkingDir() (not process.cwd()) and setContextFilePaths received the loader's paths (R9-1). * fix(cli): address R10 review — shared constant, project-local marker test, latch-consuming rewind test, gap comment fix - Extract CONTEXT_FILES_ANNOUNCEMENT_PREFIX constant + isContextFilesAnnouncement predicate in commandUtils.ts; emission site, rewind matcher, and all test fixtures/helpers now reference them, eliminating exact-spelling coupling - Add project-local marker test (QWEN.md, docs/QWEN.md) with getWorkingDir != process.cwd() to kill anchor-divergence mutation - Rewind re-arm test now submits before rewinding to consume the latch, asserting 2 total announcements so the deletion mutation is killed - Correct consume-point comment and add /directory add + performMemoryRefresh to the known-gap list alongside /cd * fix(cli): widen isContextFilesAnnouncement param type to string for tsc --build HistoryItem union includes variants whose type is not in the MessageType enum (e.g. 'about', 'stats'), so { type: MessageType } is structurally incompatible. Widen to { type: string } so the predicate accepts all HistoryItem variants; the runtime check (=== MessageType.INFO) is unchanged. * fix(cli): address R11 review — stale mock params, type-check test, ordering assertion, positional pinning - Correct loadServerHierarchicalMemory mock parameter list in config.test.ts to match real signature (extensionContextFilePaths is slot 4, not slot 5) - Add 3 unit tests for isContextFilesAnnouncement type discriminant (non-INFO item with prefix must not match) - Replace invocationCallOrder[0] with findIndex-based assertion to isolate the announcement's own call index - Pin folderTrust slot to true in directoryCommand.test.tsx - Pin extensionContextFilePaths and contextRuleExcludes with distinct sentinels in performMemoryRefresh test to catch same-typed swap * fix(cli): add /resume to known-gap list in latch comment /resume of the current session wipes UI history (announcement INFO not persisted) without re-arming the latch — same class as /cd, /directory add, and performMemoryRefresh. Documented as a known gap; self-healing latch follow-up covers all centrally. * fix(cli): reconcile announcement latch on any history replacement Wrap loadHistory with latch reconciliation: after rewind, /restore, or same-id /resume, the latch is set from whether the restored history contains a context-files announcement. Fixes the /restore duplicate announcement and the same-id /resume missing-announcement gap; replaces the rewind path's explicit hunk with the same shape. Also document the intentional fileCount vs contextFilePaths criteria difference in the loader, and fix a pre-existing test bug: the remount-only refresh test's mock destructuring was missing the history parameter, so it never actually captured refreshStatic. * fix(cli): route interactive /resume through latch wrapper and sanitize markers useResumeCommand now accepts an optional loadHistory override; AppContainer passes the latch-reconciling wrapper so interactive /resume (including same-id) reconciles the latch instead of leaving it consumed with no announcement in the rebuilt history. The wrapper also destructures loadHistory so its useCallback deps hold the stable function reference rather than the per-mutation historyManager identity, keeping history out of commandContext's rebuild path. concatenateInstructions sanitizes the marker displayPath with stripAnsiAndControl: newline/control characters in directory names could previously forge or hide entries in the /context parser, contradicting the sanitized announcement surface. Documented that contextFilePaths lists top-level files only (@import content is inlined into its importer). Added tests for the sessionId re-arm effect, the startup-resume armed latch, and the resume override. * test(cli): stub initialize in sessionId re-arm test to stop unhandled rejection --------- Co-authored-by: 俊良 <zzj542558@alibaba-inc.com>	2026-08-19 15:53:48 +00:00
Shaojin Wen	c1b8f1a11f	refactor(review): define each certification-bar atom exactly once (#9473) ... The two-author certification bar — a CLI-built prompt delivered verbatim, plus the agent demonstrably opening its brief, the diff, or the findings list its prompt named — was re-implemented in four places: the coverage walk, the layer-audit gate, retirement, and the resume recovery command. Re-implementing a bar means drifting from it, and each copy had drifted in its own direction. Extract the atoms into `lib/certification.ts` — `chunkOfKey`, `declaresOwnUncoverable`, `openedBrief`, `readBrief`, `readFindingsPointer` — and have coverage, layer-audit-gate, retirement and `recover-findings` compose the same functions. No behavior change: the atoms are the exact predicates the live pipeline already ran, now spelled once so a future edit to the bar reaches every consumer.	2026-08-19 15:06:04 +00:00
jinye	d3202c685a	feat(web-shell): Consume live-state session activity timestamps (#9476) ... * feat(web-shell): Consume live-state session activity timestamps * qwen: address PR review feedback (#9476)	2026-08-19 15:03:12 +00:00
Shaojin Wen	b604da3191	fix(review): keep the blocker in a COMMENT body every softening path reaches (#9416) ... * fix(review): keep the blocker in a COMMENT body every softening path reaches `compose-review` renders the body copy of an unanchorable blocker on a COMMENT only when that COMMENT stands where a REQUEST_CHANGES would have been — the body copy is its ONLY copy, and softening the event must never erase it. The condition listing which softenings qualify was written as an enumeration of the two flags known at the time: if (downgradedFrom === 'Request changes' || criticalsUnverified) A third path shipped past it. The findings-file `— [unverified]` tag softens a Request changes at the event line, and it sets neither flag: not `downgradedFrom` (only the presubmit carve-out sets that) and not `criticalsUnverified` (only the verification-delivery gate sets that). So a run whose coverage was PROVEN and whose verifier ran posted a 239-character body — the opener, the tag disclosure, the footer — with the blocker nowhere in it, while the verdict line and the artifact both counted it. The condition is derived now rather than enumerated: `baseEvent` is the row before every cap and downgrade, so `baseEvent === 'REQUEST_CHANGES' && event === 'COMMENT'` asks the question the clause is actually about, and answers it for softening paths that do not exist yet. Same closure the module applied to the deferral channel after #9095. The regression test builds the isolating shape — covered plan, verifier on record, findings file still carrying a tag — and asserts both flags the old condition read are unset while the blocker rides the body. Reverting to the enumeration reddens it; so does dropping the clause. * fix(review): keep the presubmit downgrade reasons on every softened Request changes * fix(review): keep the presubmit downgrade reasons on every softened Approve	2026-08-19 15:02:57 +00:00
Shaojin Wen	4154bd7457	fix(ci): no-op touch to re-register the autofix workflow triggers (#9479) ... Since ~2026-08-19 00:00 UTC the workflow's schedule, issue_comment, and pull_request triggers stopped creating runs while pull_request_review kept working; dispatch runs were created but never expanded into jobs (three sat queued with zero jobs for 4-11 hours). The on: block was unchanged throughout and a disable/enable cycle did not restore dispatch, consistent with a stale trigger registration on the Actions backend. Any content change forces a re-parse; this commit is that change (one comment line).	2026-08-19 14:56:57 +00:00
Shaojin Wen	133cf8bfcf	refactor(cli): consolidate shared helpers ahead of the legacy audit skill (#9345) ... * refactor(cli): consolidate shared helpers ahead of the legacy audit skill Move the pieces the upcoming /audit skill needs out of command-group ownership so no skill imports across command groups: - the findings schema moves from commands/review/ to cli/src/utils/ as-is; every review consumer imports it from the new home, and the stale-bundle digest, bundle-asset list, and artifact comment track the move - safeTarget (traversal-safe slug) and tokenizeArgs (quoted argument splitting) lift to cli/src/utils/paths.ts and shell-args.ts, with review's copies re-exporting/redirecting - the two private git check-ignore copies (review test-plan, team memory) consolidate into one fresh-by-default helper in core utils; the memo stays caller-side so a remedy re-check observes the flip Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): restore review ledger machinery dropped by the consolidation The shared-helper consolidation silently reverted three behaviors PR #9175 added to compose-review, against the PR's stated "no behavior change" intent: the unreviewed-dimension anchor exemption (scopeUnproven / dimensionGapsAreDepthOnly / isNonDiffDimensionGap), the LEDGER_MAX_ROUND stamp clamp, and the bilingual budget-stop phrase splice. Restore them with the tests that pin them; SKILL.md, ledger.ts, and deadline.ts still document all three. Also harden the new helper tests: - safeTarget: the deep-path fixtures now share a flattened prefix longer than the kept window, so a truncation-only slug (no digest) collides instead of shipping green. - isGitIgnored: each GIT_* scrub arm now carries a discriminating fixture (three arms previously passed with their scrub line deleted), and every foreign git init scrubs ambient repo-placement selectors. * fix(core): scrub git config-injection channels from the ignore probe Ambient GIT_CONFIG_COUNT (inline KEY/VALUE injection) and GIT_CONFIG_GLOBAL/GIT_CONFIG_SYSTEM (config-file redirects) can aim core.excludesFile at a foreign rules file and flip the probe's verdict for the -C worktree — the same leak class the existing selector scrubs close. Measured on the pristine probe: both channels turn a not-ignored path into an ignored one. Also pin the guards the review found unpinned: the two config channels, the `--` separator for dash-leading paths, the timeoutMs wiring, safeTarget's hash-of-original-target property, and tree-existence of the lifted review helpers. * fix(core): scrub pathspec-magic env channels from the ignore probe * fix(core): drop the whole GIT_* env family from the ignore probe Two more leak channels surfaced on the probe's per-variable scrub list: GIT_ICASE_PATHSPECS (the fourth pathspec modifier — ambient, makes check-ignore reject every pathspec with exit 128, which the catch reads as not-ignored) and GIT_CONFIG_PARAMETERS (the inline -c channel git itself uses to propagate config to children — ambient, can aim core.excludesFile at a foreign rules file). Measured through the real function: the first flips a genuinely ignored path to not-ignored, the second flips a not-ignored path to ignored. Since the channel list grew by one leak per review round, drop the whole GIT_* family instead of enumerating, and close the system config tier explicitly (GIT_CONFIG_NOSYSTEM=1) so host policy in /etc/gitconfig can no longer answer for the -C worktree — that ambient dependency also made the config-redirect arm red on any host whose system config matches the probe path. Pin the icase member, the PARAMETERS channel, the default 5 s deadline (previously unpinned), and add a lower timing bound to the caller-deadline arm so it cannot pass vacuously when the shim is not executable. * fix(cli): make the safeTarget slug space prefix-free at the dash boundary Review's cleanup sweeps .qwen/tmp/ by qwen-review-<slug>- prefix. A slug that itself carried '-' — natively (pr-6771 vs pr) or via the truncation join — could extend a shorter slug, letting one target's cleanup delete a DISTINCT target's artifacts (R8-7). The truncation branch this PR carries newly lands deep targets inside the cap instead of dying ENAMETOOLONG, which turned the latent collision live. Drop '-' from the slug alphabet entirely (dashes flatten like separators) and join the truncation digest with '_': with '-' out of every slug, the qwen-review-<slug>- boundary is unambiguous by construction — no slug can start with another slug plus '-'. prev-ledger side files keep their hardcoded dashed name on both writer and reader, untouched by the slug. Tests pin the prefix-free property (short-vs-short, short-vs-truncated) and the fixture names follow the new slugs. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * revert(cli): restore safeTarget byte-identical to the pre-lift behavior Reverts the slug alphabet change (539495226d) and drops the deep-target truncation branch, per review round findings R12-1 and R8-7: - R12-1: flattening dashes renamed only one side of review's two-sided artifact-naming contract — bundled-skill templates, composed names, prev-ledger and brief/report producers hardcode the dash spelling, so the bypass-audit tripwire would silently skip and the cleanup sweep would leak ~15 dash-form artifacts per review. - R8-7: the prefix-sweep hazard only exists because of the truncation branch this lift carried; main's safeTarget has no truncation, so a behavior-preserving lift must not add it. Deep-target support belongs in a follow-up paired with the sweep-side structural fix it needs. safeTarget is now byte-identical to the pre-lift implementation (moved, not modified); tests pin the dash spelling and the leading-strip rule as they behave on main. Full review suite green (5022). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci@service.alibaba.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>	2026-08-19 14:53:44 +00:00
Shaojin Wen	fd2dcd3fe0	fix(ci): clone qwen-autofix into a recovery workflow entity (#9482) ... The Actions backend wedged the original qwen-autofix.yml workflow entity on 2026-08-19: runs stick "queued" with zero jobs and cannot be cancelled or deleted via API, schedule ticks stopped being created, and event- triggered runs are dropped. Same-repo control workflows run normally, so the failure is bound to that one workflow entity. A byte-identical copy under a new path registers as a fresh entity and resumes the loop; the original file stays untouched so the revert is deleting this one file.	2026-08-19 14:46:05 +00:00
易良	b5577b7d11	fix(sdk): route unrecognized diagnostics onto a bounded transcript sidechannel (#9202) ... * fix(sdk): route unrecognized diagnostics onto a bounded transcript sidechannel Normalizer-classified unrecognized_event / unrecognized_session_update debug events no longer enter transcript blocks[]: they are mirrored onto a capped unrecognizedDiagnostics sidechannel instead. This stops them from finalizing a streaming assistant/thought block (which dropped a following assistant.usage frame) and from consuming the maxBlocks budget (which let repeated noise evict real conversation content). malformed_payload diagnostics and client-dispatched debug events keep their existing block semantics. * fix(sdk): align browser bundle budget * fix(sdk): close the sidechannel review round (#8823) - export the sidechannel API through the daemon barrel (selectUnrecognizedDiagnostics, UNRECOGNIZED_DIAGNOSTICS_LIMIT, DAEMON_UI_UNRECOGNIZED_DIAGNOSTIC_REASONS + types) and pin the reachability in daemon-public-surface.test.ts - restore the MAX_TEXT_BLOCK_LENGTH cap on sidechannel text, mirroring truncateText exactly (suffix fits within the cap) - ship the unrecognized reason subset as a runtime const array and route by membership, so a new reason cannot fall through to appendStatusBlock - copy the correlation fields createBase stamps (promptId, sourceRecordIds, branchRecordId, originatorClientId) onto sidechannel entries; drop the dead source/data switches - un-fuse the budget-history comment chain in scripts/build.js - update docs/developers/daemon-ui for the split routing - tests: full entry shape, text cap, block-path debugReason counterpart, and a webui malformed_payload interleave sibling so the #7012 flush-before-guard keeps a discriminating stimulus * fix(sdk): address round-2 sidechannel review for #8823 - build.js: bump daemon browser bundle budget 191KB -> 192KB (195,591 bytes measured > 195,584 cap; build failed at head) - webui: narrow the observer-mode debug guard so unrecognized_* diagnostics reach the reducer sidechannel; only block-path debug events are dropped - webui: merge history-store unrecognizedDiagnostics in applyTranscriptHistory so paged-back sessions keep diagnostics - transcript: extract truncateTextAtLimit shared by the block and sidechannel truncation paths - transcript: reset unrecognizedDiagnostics on rewind alongside the sibling per-turn state resets - types: rename DaemonUnrecognizedDiagnostic.receivedAt to clientReceivedAt (matches the sibling block projection) - tests: reason-prefix conformance pin, rewind reset, narrowed guard, history pagination merge * fix(webui): avoid flushing sidechannel diagnostics * fix(sdk): preserve diagnostics across rewind * fix(webui): dedupe sidechannel history records * fix(webui): align the paging sidechannel test with the normalizer keys The paging test added in e6b40e5c failed deterministically (webui suite red, CI Test job red) for two reasons: 1. The fixtures stamped only _meta['qwen.session.recordId'], but the SDK normalizer's extractSourceRecordIds reads _meta.qwenTranscript.sourceRecordIds — no sidechannel entry ever carried sourceRecordIds, so the dedupe assertion could not pass and the new displayedRecordIds loop was never exercised by a passing test. Stamp BOTH keys, matching production replay frames (acp-bridge buildUpdateMeta) and the sibling dedupe test. 2. Cap arithmetic: LIMIT-1 live entries + 2 fresh history entries = LIMIT+1, so the newest-wins slice evicted record-old-1 which the test asserted present. Emit LIMIT-2 live events so the post-merge total lands exactly on the cap. Also correct the post-merge index assertions: history entries come first (old-1, old-2), then the deduped-once live overlap, then the first live mystery event. Suite 506/506, eslint + prettier clean. * fix(sdk): raise diagnostic sidechannel bundle budget * fix(sdk): raise the daemon browser bundle budget to 198KB and pin the diagnostics selector - The sidechannel routing + selector cost ~1037 B over the 197KB cap (bundle measured 201893 B), failing the browser-bundle size gate; bump MAX_DAEMON_BROWSER_BUNDLE_BYTES to 198 * 1024. - Fold the rebase-residue 190→191→192 KB ledger entries into the accurate 190→195→196→197→198 lineage so the next bump has one canonical history. - Add a behavioral pin for selectUnrecognizedDiagnostics: it must return the routed sidechannel itself (toBe), discriminating a `return []` or shallow-copy regression that the typeof-only surface test cannot see; flip-verified. * fix(sdk): reset the user pointer on sidechanneled diagnostics, share the routing predicate appendUnrecognizedDiagnostic left activeUserBlockId untouched while the replaced appendStatusBlock path reset it for every non-user block; a later mergeable user.text.delta with no promptId stamp (e.g. a peer client's $ <cmd> echo) then appended onto the earlier user block across the diagnostic, collapsing two user turns into one and skewing rewindTranscriptToUserTurn's kind==='user' turn indexing. Keep the reset (assistant/thought pointers stay untouched, the point of the sidechannel); witness test flip-verified red without the one-line reset. Also export isUnrecognizedDiagnosticReason from types.ts next to DAEMON_UI_UNRECOGNIZED_DIAGNOSTIC_REASONS and call it at all three routing-guard sites (reducer, provider flush condition, provider drop filter) so the #7012/#8823 guard pair classifies every debug event against one source instead of three hand-written copies. * fix(ci): prevent bite harness SIGPIPE --------- Co-authored-by: yiliang114 <yiliang114@users.noreply.github.com>	2026-08-19 14:38:00 +00:00
易良	517a2bfc28	fix(triage): compute the flake-gate diff before the env -i re-exec (#9468) ... * test(triage): pin the parent-computed diff and the child copy Update the record-step pins to the RUNNER_TEMP-staged file and add a pin asserting the scrubbed child copies it rather than re-running git. * fix(triage): compute the flake-gate diff before the env -i re-exec The scrubbed (env -i) child cannot read the shallow merge-ref objects: git global safe.directory lives under HOME, which env -i strips, so git refuses to read the base commit and the diff fails with "Could not access <base-oid>". Compute the NUL-delimited diff in the parent (normal environment, no PR code executes) and stage it under RUNNER_TEMP; the clean child copies it into the root-only gate home. * fix(triage): rm -f before the parent diff redirect, slash-path the parent commands * test(triage): pin diff-before-re-exec ordering and slash-pathed commands * fix(triage): harden the flake-record staging path against directory and symlink plants	2026-08-19 14:37:44 +00:00
Heyang Wang	5003ab3c7f	feat(web-shell): add transcript contract prevalidation (#9388) ... * test(web-shell): add transcript contract prevalidation Freeze reproducible evidence for current transcript paths before any VS Code or HTML export production migration. - Add versioned fixtures, closed export schema, and capability gates - Probe direct-daemon and ACP identity under partial history prepend - Preserve raw adapter semantics and full write_file Turn Output diffs - Document the two-MR architecture, security constraints, and blockers * fix(web-shell): harden transcript prevalidation gates Make the evidence-only contract suite enforce the review assumptions it documents while preserving the existing runtime transcript behavior. - Run the contract suite in the required no-AK integration job - Fail closed on ambiguous identity probes and deduplicate gate kinds - Enforce manifest, hash, export safety, and renderer version boundaries - Cover visible transcript text and stable Desktop packaging semantics - Record the complete PR comment evaluation and verification outcome * fix(web-shell): close transcript prevalidation gaps * fix(web-shell): remove brittle Desktop wiring probe Keep transcript contract prevalidation at the evidence level it can actually prove. The previous source-text assertion could both reject equivalent formatting and pass unreachable packaging code. - Remove the Desktop script parser and its false behavioral claim - Mark installed-artifact verification as deferred to Desktop smoke tests - Clarify MR1 matrix, CI wiring, and provenance evidence boundaries - Refresh the hash-locked capability matrix fixture Note: This does not change Web Shell or Desktop production behavior. --------- Co-authored-by: heyang.why <heyang.why@alibaba-inc.com>	2026-08-19 14:13:12 +00:00
ChiGao	d96f264de7	feat(telemetry): link daemon HTTP request spans to inbound W3C traceparent (#9391) ... * feat(telemetry): link daemon HTTP request spans to inbound W3C traceparent The daemon HTTP surface records a request span per request, but every span starts a new trace: a caller forwarding the standard W3C traceparent header (OTel-instrumented clients, proxies, gateways) gets no linkage back to its own trace. Extract traceparent/tracestate from inbound request headers in the daemon telemetry middleware and parent the request span to that remote context. Extraction reuses the same path as the existing JSON-RPC _meta extraction (global propagator first, strict manual fallback so behavior is identical without a registered SDK) and fails closed: requests without a valid header keep the exact current span shape. * fix(telemetry): guard inbound traceparent sampling and align W3C fallback - Force TraceFlags.SAMPLED on inbound HTTP parents via the existing shouldForceSampled() matrix: an unsampled remote parent under the default parentbased_always_on sampler silently dropped the request span, the whole next() subtree, and the session-subprocess spans forwarded via _meta (review C1). - Replace the hand-rolled manual fallback parser with a direct W3CTraceContextPropagator instance so acceptance rules (future versions, tracestate, all-zero ids, version-00 extension field) match the registered path with or without an initialized SDK. - Gate middleware extraction behind isTelemetrySdkInitialized() to skip the hot-path parse when telemetry is off, and emit a debug daemon log when a present-but-invalid traceparent header is rejected. - Re-export DaemonRequestSpanOptions from the core barrel and add a type-level guard so the parentContext field cannot silently disappear (vitest alone cannot catch its removal). * chore(vscode): regenerate companion NOTICES.txt for @opentelemetry/core * fix(telemetry): lazy-load OTel core fallback propagator behind SDK init Address review feedback on the inbound traceparent linkage: - Keep @opentelemetry/core out of the static graph. The module-level W3CTraceContextPropagator in daemon-tracing.ts pulled the CJS barrel (bot-measured +65,046 bytes) into every closure loading that module, including telemetry-off deployments. daemon-tracing.ts now keeps only a holder + setter (setDaemonFallbackPropagator, typed against @opentelemetry/api — type imports stay free at runtime); the lazy sdk-impl.ts chunk, whose closure already contains @opentelemetry/core via sdk-node/resources, constructs and injects the W3C instance on the successful SDK assembly path. Until injection, extraction returns no parent context: the HTTP edge is already gated on isTelemetrySdkInitialized (nothing changes when telemetry is off), and the _meta edge's consumers (withDaemonSpan / withInteractionSpan) short-circuit on the same flag, so an unresolved pre-init parent never had an observable effect. - Add the mutation-verified fail-closed test for the header-extraction try/catch in daemonTelemetryMiddleware: a throwing extractor leaves the request settling normally (recordDaemonHttpRequest still fires once) with no parentContext on the span options. - Record the rejected traceparent value (truncated to 128 chars) as http.request.header.traceparent on the invalid-header breadcrumb — traceparent only carries trace-id/span-id/flags, so this is privacy-safe and makes broken cross-service joins diagnosable. Also document why the _meta extraction path deliberately skips shouldForceSampled (trusted in-process bridge vs external HTTP input). * feat(telemetry): carry inbound trace id into daemon access log with telemetry off Telemetry off (the default) left daemon logs without any trace id: with no request span, the log trace prefix never fires, so a caller forwarding W3C traceparent could not be joined to its daemon log lines. The middleware now parses the header with a plain regex (extractInboundTraceId — same shape/all-zero/ff rejections as the W3C propagator, no OTel machinery) and stores the trace id on the per-response telemetry context. The access log emits it as the camelCase traceId field of "request completed", keeping the log-based join alive with no telemetry config and no trace backend. With telemetry on nothing changes: the request span already carries the caller's trace id into the log prefix. * fix(telemetry): unify _meta/HTTP sampling and repair build export - Export extractInboundTraceId from the core barrel: the previous commit exported it from daemon-tracing.ts only, so downstream package builds failed with TS2305. - extractDaemonTraceContext now applies the same shouldForceSampled() matrix as the HTTP edge: the _meta path is also reachable from direct ACP clients (acpAgent newSession/loadSession/unstable_resumeSession and Session.prompt pass caller-controlled _meta), so an external sampled=0 parent no longer silences daemon spans there either. The in-process bridge is unaffected (its injected values are already SAMPLED). - The rejected-header breadcrumb now goes through sanitizeLogText so a crafted traceparent cannot forge log line structure with control characters. - Add the sdk-impl wiring test: after initializeTelemetry the injected W3C fallback propagator resolves inbound HTTP parents. * fix(telemetry): align log-path traceparent parsing and emit traceId in both modes - extractInboundTraceId now mirrors the vendored W3C propagator's acceptance exactly: single optional leading/trailing whitespace and trailing extension fields above version 00 (version 00 must stay four fields). Previously the strict four-field anchor made the two paths disagree on the same forward-compatible header, silently dropping the access-log traceId for exactly the callers the propagator path supports. - The camelCase traceId access-log field is now captured whenever a valid header parses, regardless of telemetry mode, so one saved log query / alert shape works for every deployment; with telemetry on the snake_case span prefix carries the same id redundantly. * fix(telemetry): move inbound trace id getter out of the middleware module 52d572c0f2 made the access log statically import the telemetry middleware module to read the captured inbound trace id. The access log sits inside the serve fast-path pre-listen closure (run-qwen-serve imports it directly), so the middleware's core-barrel import graph came along for the ride and check-serve-fast-path-bundle started failing: the 5.6MB core chunk (shell tool, glob, chokidar, @iarna/toml, fzf) became statically reachable from run-qwen-serve. Move the response-context symbol, its type, and the getDaemonTelemetryInboundTraceId getter into a new import-light telemetry-context.ts; the middleware imports the symbol from there and re-exports the getter, so the access log no longer links against the telemetry module at all. * fix(telemetry): capture inbound trace id pre-auth under a dedicated symbol * test(telemetry): pin the trace id seam through the context module getter --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>	2026-08-19 12:21:49 +00:00
jinye	4b77a6e472	fix(core): treat duplicate provider tool-call ids as replays only when arguments match (#9436) ... * fix(core): treat duplicate provider tool-call ids as replays only on matching args The duplicate provider tool-call guard (#5038/#5657) keyed on the id alone, so models whose ids are only unique within a single response — e.g. Kimi emits {name}_{index} and the index can restart at 0 on any round — had fresh calls misclassified as replays: the second collision got a synthetic duplicate error and the third tripped the circuit breaker, killing every turn by round three. A handled id now maps to a (name, canonical args) fingerprint — the same sha256 repeat key the loop guards use, moved to a leaf module so toolCallIdUtils can share it without an import cycle. An incoming call is a replay only when its fingerprint matches the call that first executed under that provider id; id collisions with different args execute normally under the unique suffixed id that normalization already assigns. Exact same-args replays keep the unchanged #5014 suppression and #5657 breaker behavior at all four entry points (AgentCore, TUI stream, non-interactive CLI, ACP daemon session). The synthetic duplicate message now tells the model to re-issue with a fresh tool-call id when a new invocation was intended, giving id-emitting models a recovery path. * qwen: address PR review feedback (#9436) - Fingerprint each incoming call once per carrier object: the WeakMap cache now keys on any stable carrier (FunctionCall part or ToolCallRequestInfo), and the replay predicate / recording helpers take the precomputed fingerprint instead of rehashing (name, args) on every breaker scan, admission pass, and record. - Move the getToolCallRepeatKey tests next to the extracted leaf module instead of exercising it through the loop detection service's compatibility re-export. - Restore the assertion pinning that runToolCalls never mutates the history accessor's returned fingerprint map, now that the defensive copy is load-bearing. * qwen: address PR review feedback (#9436) Criticals: - Canonicalize onto a null-prototype object so a literal __proto__ own key (preserved by JSON.parse) stays a data property instead of vanishing through the inherited setter — two calls differing only in __proto__ no longer collide on one repeat key, which the replay oracle would have turned into a wrongly suppressed execution. - Clone request args at scheduler intake: callers pass args that can alias the model-emitted functionCall part stored in chat history, and the executor rewrites PATH_ARG_KEYS on request.args in place (a persistence the post-'ask' bounce re-execution relies on). Without the clone those rewrites leak into history and skew the replay fingerprints derived from it, letting genuine replays of path-carrying calls re-execute in multi-round agent runtimes. Suggestions: - Complete the duplicate message with the different-arguments recovery path required by design decision D3, for models whose provider assigns ids. - Fix the copy-convention comments (the accessor returns a fresh map per call; copies are future-proofing) and align all four entry points on copying the accessor result. - Pin the untested branches: history first-occurrence-wins for reused ids and orphan response-id exclusion, the fingerprint cache-hit path, the __proto__ distinction, the caller-args no-mutation invariant, and a cross-round runtime test that a replay of the original call stays suppressed after an id-colliding execution.	2026-08-19 12:02:46 +00:00
易良	872dd614f7	fix(core): isolate image payload eviction state (#9423) ... * fix(core): isolate image payload eviction state * fix(core): clone history parts for background agents before in-place image eviction * fix(core): protect image history snapshots * fix(core): bound referenced image reattachment * fix(core): anchor image-marker matching to the full eviction marker	2026-08-19 11:33:36 +00:00
易良	bb45a09c06	fix(core): reject upstream fail-fast placeholder responses (#8938) ... * feat(core): reject upstream fail-fast placeholder responses Upstream endpoints occasionally fail fast with an HTTP 200 whose entire body is the placeholder text '(request timeout)'. The response passes all existing stream validation and gets persisted as a normal assistant reply, polluting subsequent request context and surfacing to users as a lost reply. - Throw InvalidStreamError('UPSTREAM_DEGRADED_RESPONSE') at stream end when the whole response body is exactly the placeholder, reusing the transient retry budget so the turn is rolled back and re-attempted. - Treat placeholder-only model turns as invalid in extractCuratedHistory so already-polluted sessions (including --resume) stop replaying them. Both checks are exact whole-turn matches to avoid false positives on legitimate mentions of the text. Fixes #8916 * fix(core): prevent placeholder text from reaching display before retry (#8938) - Defer the first chunk with a finishReason in non-continuation turns so post-stream validation (placeholder check, empty-text check) can reject it before consumers see the content. Without this a single-chunk fail-fast placeholder (text + finishReason: 'STOP') was yielded to the TUI / headless adapter before the throw, and neither consumer rolled back the committed output. - Add exhaustion test for UPSTREAM_DEGRADED_RESPONSE retry budget. - Add curation test for whitespace-padded placeholder (.trim() guard). - Add curation test for placeholder turn carrying a functionCall. - Assert the placeholder text is absent from emitted stream events. * fix(core): block degraded continuation leaks * fix(core): catch split degraded placeholders * fix(core): withhold degraded placeholder chunks * fix(core): preserve current image payloads after placeholder curation * fix(core): preserve deferred stream chunks safely * fix(core): fold held placeholder-prefix text into continuation accounting When a stream is cut while degraded-placeholder-prefix chunks are held back (never yielded), the send loop's delivery accounting saw nothing delivered and replayed fresh — letting a placeholder split across the cut ('(request ' + 'timeout)') slip through as the harmless-looking fragment. Stage the held text on the pipeline's error path and fold it in the attempt's catch so the continuation gate resumes from the prefix and the reassembled text is still rejected as a completed placeholder. A completed placeholder is the exception: there is nothing honest to resume from, so that recovery stays a fresh replay. Fixes the split-placeholder regression test; all 332 geminiChat tests green. * fix(core): latch deferredFirstChunk so duplicate finishReason chunks are not dropped A second finishReason-bearing chunk arriving before any yield overwrote deferredFirstChunk (plain assignment at both deferral sites), so only the slot's FINAL value was yielded post-stream while every overwritten chunk's parts still landed in allModelParts -> history/JSONL: text persisted but never displayed, or a functionCall persisted but never dispatched. Latch the first deferred chunk with ??= at both sites (R8-1). * fix(core): judge deferred chunk against accumulated text on stream error The error-path escape check judged the deferred chunk by its own parts only. A placeholder split into a held prefix and a finishReason-bearing tail ('(request ' + 'timeout)') failed that per-chunk prefix check on the tail alone: the fragment leaked to consumers while the held prefix was staged after it, scrambling the continuation handoff. Judge the deferred chunk against the accumulated attempt text — the in-loop hold's own expression — and hand the combined text off in delivery order. Also drop dead stores: the yieldedAnyChunk write after the loop's last read and the trailing local resets after the final yields. * fix(core): deliver withheld placeholder-defense chunks in arrival order, drop the invisible continuation handoff Three review findings on the placeholder-defense machinery: 1. The deferred first chunk reached consumers out of arrival order — the post-loop flush yielded held chunks first, and any inline-yielded chunk preceded it, so displayed text diverged from arrival-order history/JSONL (and consolidation's in-place part merge rewrote the shared part the deferred chunk carried, duplicating text on delivery). 2. The continuation handoff staged never-delivered held text into the transport-continuation buffer; a continuation that diverged from completing the placeholder persisted the invisible prefix into history/JSONL while the UI showed only the remainder. 3. The staged handoff survived the turn through the MAX_TOKENS recovery wrapper (which rethrows non-InvalidStreamError without the folding catch) and could be folded into a later turn's continuation buffer. Changes: - Unify the deferred chunk and held placeholder-prefix chunks into one arrival-ordered pendingPreValidationChunks list; every flush (in-loop divergence release, stream-error delivery, post-loop flush) yields it in arrival order, so deferral can no longer reorder delivery. - Snapshot the pending chunks with detachChunkParts before history consolidation merges adjacent text parts in place, so post-consolidation flushes deliver what actually arrived instead of rewritten text. - On a stream error, deliver the pending chunks (mirrored by the send loop into the continuation buffer, keeping delivery and persistence aligned) unless they complete the placeholder — a completed placeholder is still dropped silently for a clean fresh replay. This replaces the pendingHeldPlaceholderText field, its staging, the send-loop fold and the entry reset: never-delivered text no longer enters the continuation buffer, which also closes the cross-turn leak. Tests: arrival-order delivery of a deferred chunk + later held chunks; in-order delivery of a diverged placeholder prefix without retry; a lone placeholder-prefix chunk delivered and persisted without rejection; delivery/persistence alignment when a held-prefix continuation diverges. Existing split-placeholder continuation coverage still green (340/340). * fix(core): propagate image eviction to durable history, harden placeholder gates Round-10 review findings: - replaceImagePayloadsInPlace now rewrites the shared Part object itself (part.text = reference; part.inlineData = undefined) instead of only replacing the content.parts[i] slot: when placeholder curation makes two user turns adjacent, appendCuratedContent merges them into a fresh Content whose parts array reuses the durable history's Part objects, and a slot-only swap left the inline base64 payload alive in history — the eviction pass re-ran every request (countAllInlineImages stayed at the threshold) and store.put() re-hashed the multi-MB payload each time, so the memory this mechanism exists to shed never left history. Nested functionResponse parts get the same treatment (getFunctionResponseParts returns the live array). Regression test pins the durable side. - Extract nonThoughtText(parts) and use it at both placeholder gates (the per-chunk hold and the stream-error completed-placeholder drop) so the two decisions cannot drift when the text-accumulation semantics change. - Log the one deliberate discard: the completed-placeholder drop on the stream-error path now emits a debugLogger.warn naming the withheld chunk count, so a benign transport-cut recovery is distinguishable from lost output after the fact. - Comment at the eviction call site names skipParts as the load-bearing protection for the current message's images (the identity skipEntry find misses whenever curation merged the user turns into a fresh object). * fix(core): preserve deferred stream metadata * fix(core): close round-12 findings on degraded-placeholder defense - XML tool-call recovery now aligns the withheld pre-validation chunks with the rewritten turn: their plain text is rewritten to the stripped remainder so consumers no longer receive the raw `<invoke>` markup while history persists the recovered shape (fixes the deterministic 'retains a short text prefix' failure red-lining CI) - completed-placeholder detection is now cross-attempt: a placeholder that completes across a transport cut (prefix before the cut, remainder before the error) is discarded instead of persisting via the continuation-prefix fold, and the send loop refuses to continue from a delivered text that equals the completed placeholder — it routes through the invalid-stream budget and retries fresh - mid-stream flush of held chunks yields detached copies so post-stream consolidation cannot rewrite already-delivered part objects - image reattach now resolves `Image #<id>` references from the store even below the eviction threshold, so fallback/recovery rebuilds after an in-place eviction pass don't carry markers without pixels * fix(core): close round-13 findings on degraded-placeholder defense R13-1/R13-4: the completed-placeholder conversion no longer throws from inside the send loop's catch (which escaped the loop and skipped the invalid-stream budget — zero retries, no telemetry, no fallback chain). Both conversion sites now fall through to the budget handler: the send-loop guard converts when the folded buffer completes the placeholder, and the stream-error discard converts before rethrowing, so a placeholder completed across a transport cut retries fresh with the continuation reset instead of continuing from the garbage prefix. R13-2: the fork boundary (copyHistoryContainers) now shallow-clones Part objects — including nested functionResponse.parts — so a forked chat's in-place image eviction can no longer strip payloads out of the MAIN conversation's durable history (the snapshot chain shares part objects by reference; the ids would exist only in the discarded fork's store). The stale contract test is updated to pin the new isolation. R13-3: the cross-attempt discard merges the UNTRIMMED attempt text (trim only after the merge), matching the persistence merge so a remainder opening with whitespace cannot slip past the discard. R13-8: the below-threshold reference scan is skipped entirely when the image payload store is empty (the common steady state). R13-10: reattach candidates exclude ids already carried inline in the same request (content-hash ids would otherwise ship the same payload twice per send). R13-15: the per-chunk placeholder hold compares the MERGED delivered text when a continuation prefix is in flight, so a remainder that finishes the placeholder over several chunks is withheld instead of yielded inline before the post-stream gate throws. Tests: double-cut regressions for both remainder shapes (with/without finish frame), e2e below-threshold reattach, fork-boundary isolation (top-level + nested), inline-id dedupe, split-tail converted to the retry path, plus the R13-7/9/11/12/13/14 assertion strengthenings. * test(core): pin full history in the second tool-result continuation test R13-14 follow-up: the 'deliver every deferred chunk in a tool result continuation' test also pinned history only via at(-1); replace with the full expected history so a drifted partial-turn pop orphaning the functionResponse turn cannot hide. * fix(core): read the converted placeholder error type in budget telemetry Round-13 follow-up: the invalid-stream budget handler's debug log and ContentRetryEvent still read (error as InvalidStreamError).type, but on the converted-placeholder path `error` is the raw transport error and the InvalidStreamError lives in budgetError — the retry log printed [undefined] and the telemetry event carried a wrong type. Read budgetError.type at both sites. * fix(core): exempt last-referenced images from reattach cap + harden tests Round-14 review of the degraded-placeholder defense (#8938). Critical fix (image-payload-references): buildReattachParts ranked ids referenced in the user's LAST message by their original eviction-marker position, so under the maxRecentImages cap an explicitly-referenced older image lost its slot to a stale, never-requested marker — the reference-resolution feature silently failed in its primary scenario. Ids referenced in the last content are now collected separately and reattached unconditionally, outside the recency cap, mirroring prepareImagePayloadsForRequest's unconditional referencedIds reattach. Doc (forkedAgent): the CacheSafeParams.history contract now states the shallow-clone behavior — consumers may rewrite part objects in place, but deeper payload objects (inlineData, functionResponse.response) remain shared and must not be mutated. Test-efficacy hardening (all mutation- or trace-verified gaps from the review): nested functionResponse inline-twin dedupe; cap-exempt referenced-image regression; marker-present + placeholder-absent pins on the eviction test; below-threshold e2e cap-binding, all-images eviction, post-send-2 marker state, exactly-once reattach; mid-history and consecutive-run placeholder curation semantics; shape-A double-cut record/history agreement + corrected mechanism comment; diverged continuation second-half delivery; plain-RETRY-after-rejection pin; UPSTREAM_DEGRADED_RESPONSE routing pin on cut-before-finish; deferred-chunk ORDER pin (finish chunk must be last); retry-exhaustion no-CHUNK-to-consumer pin (via an expectStreamExhaustion collector); and a hold-liveness test pinning that non-placeholder text is released inline before stream end. * refactor(core): dedupe placeholder-error and reattach-append sites Ponytail review follow-ups on the degraded-placeholder defense (#8938): - Extract a degradedPlaceholderError() factory: four call sites (send-loop catch-convert, stream-error completed-placeholder discard, and the two post-stream gates) constructed the identical InvalidStreamError literal; the factory keeps the message/type strings from drifting between sites. - Extract appendReattachParts(): both request-history branches ended in the same 7-line "append onto last user turn, else push a fresh one" block; the shared helper keeps the append shape from drifting. * fix(core): gate placeholder conversion on !streamYieldedFunctionCall (#8938) - The send-loop placeholder->InvalidStreamError conversion was the only placeholder-retry producer that could fire after a functionCall chunk had been delivered: a folded '(request timeout)' completed on the same attempt that emitted a tool call would schedule a fresh retry after the consumer received the call, orphaning the tool_use/tool_result pairing. Add the same point-of-no-return guard every sibling path enforces; the original transport error now propagates instead. - Regression test: folded placeholder + delivered functionCall => no third attempt, stream terminates, last chunk is the tool call. - Test pins from review: budget-routing assertions (mockLogContentRetry with UPSTREAM_DEGRADED_RESPONSE) on both double-cut tests, a non-vacuous retry guard on the prefix+remainder rejection test, and the throwing structuredClone spy on the getHistoryLength no-clone contract (matching the sibling walk-only accessors). * test(core): attach the rejection handler before timer flush (#8938) The folded-placeholder/functionCall regression test rejects the consumer stream mid timer-advance; attaching the expect().rejects handler only after advancing left a handlerless rejection at the timer checkpoint, which vitest's unhandled-error gate turns into a red run under parallel CI load (all 20094 core tests passed; exit 1 came from the single unhandled rejection). Attach the expectation before advancing timers so the handler is on the promise regardless of when the send loop rejects. * test(core): satisfy vitest/valid-expect on the pre-timer rejection handler (#8938) The previous fix assigned expect().rejects to a variable, which eslint's vitest/valid-expect rule rejects (async assertions must be awaited or returned) — CI lint failed before the suite even ran. Keep the unhandled-rejection protection by registering a plain catch handler before advancing timers, and leave the awaited expect().rejects assertion as the lint-compliant pin. * fix(core): scope placeholder run invalidation to the degraded turns (#8938) R16-1: extractCuratedHistory dropped the WHOLE consecutive model run when any turn was a degraded placeholder, so a sibling functionCall turn was dropped while the following user(functionResponse) survived — the outgoing request carried an orphaned functionResponse, exactly the invalid pairing the continuation gate and the MAX_TOKENS hasFunctionCall check exist to avoid (pre-PR the run stayed whole and provider-valid). - curation now drops only the placeholder turns of a run and keeps valid siblings (invalid-content runs keep the pre-existing whole-run semantics) - repairOrphanedToolUseTurns scan no longer lets placeholder turns split the model<->user pairing adjacency: an fr right after a placeholder sibling counts as adjacent (the mirror run [functionCall, placeholder] + fr previously synthesized a duplicate error-fr because the placeholder broke adjacency) - the whole-run drop test is rewritten to the revised per-turn semantics; new regression tests pin both run directions through the public API (mutation-verified: the old whole-run drop fails them). 350/350 green, tsc + eslint clean. * refactor(core): narrow degraded placeholder defense * fix(core): preserve placeholder-aware tool adjacency --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: yiliang114 <yiliang114@users.noreply.github.com>	2026-08-19 10:34:52 +00:00
易良	8b2593d79b	docs(tools): clarify list_agents excludes Agent Team teammates (#9432) ... The empty list_agents result ("No background agents are available in this session") is true for the ordinary background-subagent roster but reads as team status while named Agent Team teammates are active, which can trigger duplicate launches or a false "no workers running" conclusion. Make the control-plane boundary explicit: - tool description states that named teammates are NOT listed, deliver their final reports automatically, and that list_agents/task_list polling must not be used to wait for them; - empty result names the roster it reports on and the teammate exclusion. The registries stay separate; this is a tool-contract wording fix only. Fixes #9431	2026-08-19 09:20:19 +00:00
callmeYe	e4f5504e9f	feat(extensions): support authenticated HTTPS Git installs (#9458) ... * feat(extensions): support authenticated HTTPS Git installs * test(serve): update capability integration baseline	2026-08-19 09:06:54 +00:00
Shaojin Wen	e352037078	fix(review): clamp the posting volume at its origin, not only where it is written (#9460) ... * fix(review): clamp the posting volume at its origin, not only where it is written Follow-up to #9413, from a maintainer's independent pass. The terminal `VOLUME:` line was the one read site of the round's count that skipped the shared volume reader: the serializer, the parser and the side-file recovery all clamp, but `result.postedInline` carried the raw drafted- comment length, so in the defensive over-cap case the line would print an uncapped number beside a marker recording the capped one — the two- outputs-disagree failure that reader's own docstring exists to prevent. Clamping once where the count is derived puts every surface on the same value. Not reachable with any real producer (no round drafts 100k inline comments), so this closes a consistency gap rather than a live bug. Also corrects a fixture comment that survived the round-4 semantics reversal: `postedInline`'s absence is preserved, not defaulted to zero, and the comment beside it still claimed the opposite of what the validator, the type and two tests all say. * docs(review): re-anchor the bodyTrim fixture comment to its own semantics The rewritten `postedInline` comment broke what the line below it refers to. The fixture's comments used to read as one chain — every field non-default because absence defaults to something — so `bodyTrim`'s "for the same reason" resolved correctly. With `postedInline` now documenting the OPPOSITE reason (its absence is preserved, not defaulted), the nearest antecedent teaches a maintainer that `bodyTrim` is preserved too, while the validator defaults it and always emits it. The comment now states its own side of the split instead of pointing at whichever comment happens to precede it.	2026-08-19 08:55:42 +00:00