3845 commits

Author	SHA1	Message	Date
qwen-code-dev-bot	d707217f16	Merge branch 'main' into feat/summary-export-path	2026-08-02 02:43:07 +08:00
Shaojin Wen	06ead8f4ff	fix(core): unblock history pagination on oversized transcript turns (#8335) ... * fix(core): ride indivisible transcript pages over the read budget Backward history pagination dead-ended with HTTP 413 whenever a single turn exceeded the 4 MiB page budget: a turn cannot be split across pages, so the reader threw SessionTranscriptPageTooLargeError and the Web Shell latched a permanent pagination error banner. Take at least one indivisible unit per page (one aggregate record forward, one turn backward) so pagination always makes progress; the 32 MiB response serialization cap remains the hard ceiling. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * feat(web-shell): add retry button for history pagination error A non-retryable transcript page failure (4xx, partial replay) latched paginationError with no in-UI recovery short of reloading the session. The banner now offers a retry that force-clears the latch and refetches the same page, whose cursor was never advanced by the failed attempt. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-08-01 18:39:05 +00:00
qwen-code-ci-bot	893a97057b	chore(release): v0.21.3 (#8338) ... * chore(release): v0.21.3 * docs(changelog): sync for v0.21.3 --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>	2026-08-01 18:14:12 +00:00
Shaojin Wen	63359f6de4	Merge branch 'main' into feat/summary-export-path	2026-08-02 01:34:37 +08:00
qwen-code-ci-bot	812e233ca3	fix(cli): address review feedback on /summary custom path (#8116)	2026-08-01 16:53:13 +00:00
Shaojin Wen	92e184a3d0	feat(review): mined disciplines — effective-diff guard, positive control, seven lenses (#8261) ... * feat(review): borrowed-verification trio — test-plan check, base-tree A/B, per-hunk probes - qwen review test-plan: rule on the PR Test Plan's checkable claims (paths, npm scripts, test counts) against the reviewed tree; contradictions and differing counts are disclosed via compose-review, never capping. - qwen review base-tree: build the merge base in a sibling worktree so the verifier can A/B a comparative claim instead of reading it; swept by cleanup. - test-efficacy: third probe kind — reverse-apply one hunk at a time and re-run the affected tests, attributing a still-green suite to the specific change nothing gates; shares the mutants' budget window, runs last. * fix(review): survive real runner output — ANSI-laced and trimmed-away summaries Both measured on a live /review of QwenLM/qwen-code#8176 with the built CLI: - test-plan's observedTestCounts strips SGR sequences before matching; a color-enabled pipe interleaves them BETWEEN tokens, and the count claim fell to 'unchecked' with the summary right there in the report. - build-test's trimOutput rescues runner summary lines from the omitted middle (like module-resolution errors): a failing suite's tail is all failure details and npm epilogue, which pushed the one-line summary out of the kept text entirely. * fix(review): address the eight findings from live review of this branch All measured in the review (QwenLM/qwen-code#8215 review comment): - test-plan: linear-time bold-heading scan (the old pattern backtracked catastrophically on an unclosed ** line an untrusted PR body controls); a flag preceding the npm script yields no claim instead of a false 'no package defines this script'. - test-efficacy: a hunk probe's restore recreates the parent directory a reverse-applied 'new file' hunk removed (the ENOENT from finally lost the verdict and marked every remaining hunk inconclusive); hunks get their own skippedForBaseline instead of mislabeling a red baseline as a budget skip; splitDiffIntoHunks re-captures the file header at every diff --git boundary; a hunk-survived finding notes when it restates an inert file-level revert at hunk granularity. - base-tree: idempotent fast path keyed on a build marker + HEAD check — concurrent verifier shards reuse one built tree instead of sweeping it out from under each other mid-A/B (a fabricated base-side difference with a deterministic source tag was the worst case); cost wording is now 'an install and a build' everywhere it was 'one extra build'. * fix(cli): never score a hunk survived when its own test left the baseline (#8215) A per-hunk probe reported `survived` whenever the green baseline probes still passed with the hunk reverted. When the hunk's own collocated test dropped out of the baseline (a probe-tree import error collects nothing), the remaining green probes prove only that THEY do not cover the hunk, so the verdict is now `inconclusive` — the same dropped-test asymmetry the mutants already hold. Also scope the hunk-survived cross-reference note to the hunk's own collocated test, and let test-plan match a workspace-scoped run of the plan's bare command instead of falling through to the manifest on an exact-string miss. * fix(review): silence-bias hardening from four live review rounds of this branch The two blocking findings, reproduced on this PR's own Test Plan: - test-plan files no false contradicted notes: npm rulings move from a four-verb denylist to an allowlist (the run form + npm's script aliases — the ~fifty other builtins each used to become 'no package defines this script'); a slash token is claimed as a repo path only with evidence (an extension or ./ prefix), never when it is a flag's value (--repo owner/repo) or under the review's own temp root; HEADING_LINE_RE drops the same quadratic shape its bold sibling was rewritten to remove. - base-tree gets a real mutual-exclusion lock around sweep+add+build (mkdirSync test-and-set; the loser returns busy instead of deleting the tree the winner is mid-install in), and a failed build writes a settled marker so later shards stop re-paying the install to relearn 'unavailable'. Also: Agent 7's brief now names hunk-survived and the hunks.* counters (it is the report's only consumer, and the finding class was invisible); hunk findings anchor at the first ADDED line instead of up to three context lines above the change. * feat(review): four round-2 borrowings — measured failure attribution, round ledger, richer mutants, doc parity Shaped by a live round-2 maintainer re-verification (QwenLM/qwen-code#7818): - qwen review test-delta: rerun the PR side's failed test commands on the built merge base and diff the failing FILE SETS — netNew is the PR's own failure by measurement (whatever files the diff touches), shared is pre-existing by measurement. Counts are never compared: a flaky suite fails different test names between runs of the same tree. An unfinished base rerun attributes nothing. - Round ledger: the incremental review cache persists confirmed findings under round-scoped ids (R1-2); a high-effort re-review rules on each (fixed / still stands / cannot tell) and opens its report with the table, the way a human round-2 comment opens with 'M1 is fixed'. - Three replacement mutation operators in test-efficacy: drop '?? fallback', force a comparison-bearing guard condition to 'true', drop a '+ CONST' term. Each survivor maps to one crisp untested-protection sentence. A line whose raw text and literal-blanked code view disagree yields no candidate — an edit index computed across the two views once spliced 'iftrue 0)' into a guard, and a mangled mutant reads as inconclusive while quietly spending a cap slot. - Quality brief: documentation-parity lens — a new user-facing surface whose siblings are documented is a Suggestion naming the sibling precedent; no documented sibling, no finding. * feat(review): render adjudication, workflow step extraction, three verification lenses Shaped by a live sanitizer-PR verification (QwenLM/qwen-code#8147): - Render-adjudication capability (opt-in): with QWEN_REVIEW_SCRATCH_REPO set, the verifier may post a minimal payload to that user-designated repo and rule on GitHub's own rendered HTML — the measured case being an @ -> &#64; defusal every local reading called sound while GitHub's real renderer registered the mention and fired the notification. Absent the setting, rendering claims honestly cap at low confidence / cannot tell. Step 7's write ban names the carve-out explicitly. - qwen review extract-step: lift one workflow step's run: script verbatim into an executable, with env (as comments, never half-substituted exports), every ${{ }} site listed unevaluated, and a heuristic invoked- command list as the stubbing starting point. With base-tree, both arms of a by-hand workflow A/B become two invocations. yaml declared as a cli dependency (previously resolved only via hoisting). - Three brief lenses: a borrowed protection idiom missing what made it work at home (the code ancestor did the protecting; only the entity was copied); a second parser for an authoritatively-parsed format is a divergence hunt; tests that pin the mechanism instead of the effect, and oracles that mirror the implementation's own model. * feat(review): sibling-entrance discipline for the fixed verdict From round 6 of the live sanitizer verification (QwenLM/qwen-code#8147): the fix closed the fence-shaped entrance into a raw-HTML block, and the code-span entrance beside it — same divergence, adjacent syntax — stayed open. A re-check that tests only the reported input rules 'fixed' over a hole one backtick away. Both fixed-verdict sites in SKILL Step 6 (the open-Criticals re-check and the round ledger) now require enumerating a divergence-class defect's sibling entrances before ruling fixed; a still-open sibling is a NEW finding, never a reason to withhold the original's fixed — the two rulings stay separate so the second hole cannot ship unreviewed. * feat(review): three measured-verification lenses from live rounds 8037/8005 - Threshold-boundary scan: when a fix is a ratio/length guard, hold the issue's own variables fixed and binary-search the boundary where the behaviour flips; put the number next to what the issue reports. Live case: a prose-ratio guard covered the edit/write_file half of its issue and silently declined the run_shell_command half (~473-char boundary). - Delimiter self-injection named as the first parser-differential probe: a no-escaping extractor fed its own close tag truncates silently. - Shared-gate state enumeration: a deliberate-design defence extends only to the states it argues — an input-hold argued for 'active' silently froze three idle states sharing the same gate. The sibling-entrance rule, applied to a state machine instead of a syntax. * feat(review): carry the round ledger in the posted review body The ledger shipped as a local cache file and its first multi-round live use exposed the flaw: four model-comparison rounds re-reviewed the same two PRs and every round opened from scratch — medium never reads the cache, and from CI or another clone there is no cache at all. The one artifact every environment can see, the posted review, carried nothing machine-readable. - compose-review embeds an HTML-comment marker (invisible on GitHub) carrying this round's findings and round number; the round comes from the side file pr-context recovered, +1 — never from the model. - pr-context recovers the reviewing account's latest marker, renders it as a 'Previous /review round (machine ledger)' work-list section, and writes it beside the context file for compose-review's round counter. Own-account only, latest round wins, fail-quiet on any malformation — the marker is data the next round rules on, never authority it obeys. - Step 6's previous-round ruling now runs at medium too (recovery is free; medium still writes no cache and posts nothing); the local cache is demoted to fallback for rounds that never posted. * fix(review): address review feedback — false-positive hardening, binary diff guard, error convention (#8215) * fix(review): address review feedback — base-tree availability gate, test-plan false positives, hunk-probe ranges (#8215) - base-tree: only stamp a base tree available when runBuildTest actually compiled something (ok AND npm toolchain AND a non-empty build). An `unsupported` handoff or an empty npm scope returns ok:true having built nothing; marking that tree available let an A/B read the absence of a build as a behavioural difference. - cleanup: sweep the stale base-tree build lock a killed builder leaves behind. - test-plan: read the root manifest's scripts directly so a root-only script survives when the root defines no build/test; bail on the inline --root=./dir rebasing form; stop treating a positional after an inline --flag=value as the flag's value; prefer a failed scoped run when ruling a bare command; anchor the npm script alias to a full token so `yarn test:unit` is not truncated. - test-efficacy: exclude `\ No newline at end of file` from the startLine offset count; compute the mutant-overlap range from the header's new-side span so it no longer overshoots into a closely following hunk. * feat(review): mined disciplines — effective-diff guard, positive control, seven lenses From a full sweep of 108 maintainer verification comments (2026-07-31): - fetch-pr detects an EMPTY merge-base diff (work already landed — the skill stops and recommends close-as-superseded) and a diff collapsed to a residual by overlapping merged PRs (disclosed; body claims about the rest are description-of-history). - test-efficacy runs a POSITIVE CONTROL before spending the mutant window: an injected always-failing test must turn the runner red, or every would-be survivor is re-classed inconclusive — a dead runner cannot kill, and its survivors were the false gap-reports measured live. - Brief lenses: tested-fix + contamination statement; race verdicts as rates with load amplification (lower idle rate without structural change is luck); triangulate-and-label for unreachable authorities; arm-identity and comparator negative-control for A/B; deleted code's incidental effects; self-defeating fallbacks (trigger set within cannot-succeed set); reachability adjudication (fix vs scaffolding); flipped-assertion audit; shape-vs-provenance authorization probing. * fix(review): four fixes from the stack's own round-2 reviews - test-delta (Critical, #8218 review): a base rerun that fails with ZERO parseable failing files (install/toolchain failure) no longer reads as 'base green' — it attributes nothing, like a timeout, instead of promoting every PR-side failure to net-new. - extract-step (Critical, #8225 review): every line of a multi-line env value is comment-prefixed — an unprefixed block-scalar second line sat in the emitted script as an executable line. - extract-step: expressionsOf no longer misses expressions containing their own brace (format('{0}')). - cleanup (#8215 review): sweeps an orphaned base-tree build lock, which otherwise wedges the A/B for that PR permanently. * fix(review): close the remaining round-2 findings across the stack - selectMutants: replacement operators get a 3-slot sub-cap inside the shared cap — measured 24x pool inflation (215 vs 9 candidates over 40 real commits) was draining the time window hunk probes draw from last, silently un-shipping the hunk-survived finding class; sub-cap drops are counted in skippedForCap. - test-delta: whole-command budget (540s, mirroring test-efficacy) so three failed commands cannot blow the 600s tool ceiling and lose the report; run() regains build-test's three deliberate properties (stdin ignored, error.code/SIGTERM timeout detection, trimmed output). - test-plan: pasted unified diffs inside Evidence blocks shed no path claims (+++ b/<path> once ruled contradicted on a correct body); gitignored paths rule unchecked — absent at the reviewed commit by construction, the .qwen/ reasoning generalized. - agent-prompt: the Agent 7 efficacy block now enumerates all FOUR finding kinds including hunk-survived, the hunks.* skip counters, and harnessValidated — the brief and the block in the same prompt no longer present two different complete sets (the sibling-enumeration lesson, applied to our own prompt). * fix(review): address review feedback — diff-header false positives, stale prompt enumeration, added-file hunk probes (#8215) * fix(review): close the round-3 findings on the stack #8261 review: - the dead-harness re-class now runs BEFORE findings are built (a failed positive control could still file survivor findings — the Critical); it also covers the file-level revert probe's inert verdicts, the control pays for its run out of the shared budget (no more silently eaten skippedForBudget slot), a failed control stops the window instead of manufacturing survivors to re-class, and its note names all three causes that share the green-control shape. - emptyDiff no longer fires on a FAILED diff capture (close-as-superseded off an infrastructure error would close a live PR); countDiffChangedLines counts --x/++x body lines. #8215 review: - the cd target itself is no longer claimed as a path (a bare dir carries no evidence; only the file tokens it resolves are); - 'Test Files N passed' is a FILE count — variable-length lookbehind keeps it out of the test-count claims; - base-tree's handler gains the pipeline's catch-and-exit-1 convention; - probeCreateFailureDetail folded into the shared worktreeCreateFailureDetail. * fix(review): address review feedback — cd-base exclusion, Test Files count guard, base-tree error handling, probe delegation (#8215) * fix(review): port the collocated-dropout test to the post-#8050 runner seam Merging main brought #8050's Windows-portability refactor, which resolves the probe runner through vitest/package.json's bin — a node_modules/.bin fake is dead weight it never reads. The 8215-only collocated-dropout test still installed the old .bin fake, so the REAL vitest ran its fixtures, price.test.ts genuinely passed, and the hunk scored survived. The test now overrides the fake package's vitest.mjs like every post-refactor test. * fix(review): close the round-4 findings from both stack reviews From the #8215 review: trimOutput's summary rescue is capped at 40 lines (uncapped, 40k lines of 'Test <n>: …' prose made the whole trim a no-op — 1.6MB in, 1.6MB out); a spaceless # line is prose per the ATX rule GitHub applies (a pasted #!/bin/bash outside a fence once ended the Test Plan mid-body), with the bare-#-run crash on the closing scan guarded; a base-tree build lock older than 30 minutes is a corpse and is swept rather than reporting busy for the rest of the review. From the #8261 review: a pasted diff's BODY lines shed no path claims (-packages/old/gone.ts matched PATH_RE and ruled a false contradicted — the syntax-line filter alone was vacuous against the real failure mode); a gitignored file that nonetheless EXISTS rules reproduces (the ignore guard only ever downgrades a would-be contradiction); test-delta's note now explains each unattributed command (timeout vs toolchain failure) instead of leaving the reader to infer why it is in neither list. * fix(review): bound the summary rescue, apply the ATX heading rule, sweep stale build locks The three 8215-layer findings from the latest review, fixed at this layer (they were first patched further up the stack, where the reviewer of THIS PR cannot see them): - trimOutput's summary rescue is capped at 40 lines — uncapped, 40k lines of 'Test <n>: …' prose voided the trim entirely (measured 1.6MB in, 1.6MB out) and the bounded-output contract is the whole point. - A '#' with no following whitespace is prose, not a heading (the ATX rule GitHub applies): '#8176', '#tag', an unfenced '#!/bin/bash' no longer end the Test Plan section mid-body; the bare-#-run crash on the closing scan is guarded. - A base-tree build lock older than 30 minutes is a corpse left by a killed builder — swept and rebuilt instead of reporting busy for the rest of the review. * fix(review): EEXIST-only lock busy, bun test alias, chained cd bail, fence backreference Four live findings from the latest inline review round (the rest of the round was already fixed upstream by the takeover bot - verified by probing head behavior rather than re-reading the threads): - base-tree's lock catch distinguishes EEXIST (a concurrent builder, busy) from EPERM/EROFS/ENOSPC (this run's own failure, reported as such, not as a busy that will never clear). - "bun test" is bun's built-in runner, not a package-script alias: it runs whether or not any manifest defines test, so ruling it against the scripts table filed a false contradicted. - A chained cd matches the leading-cd shape but the single-hop resolver joined file tokens against the FIRST directory; it now bails like the exotic-cd case. - codeSpans' fence regex closes on its own marker via backreference; a tilde fence line inside a backtick block ended the span early and lines after it were lost to extraction. * fix(review): make the ledger marker actually reach the posted body Three Criticals from the #8255 review, all real: - The marker was appended in the CLI handler, AFTER composeReview() returned, so it only ever reached the composed JSON on disk. submit posts what the PURE function returns, so no posted review carried a marker, latestOwnLedger always returned null, and the whole feature was inert end to end. The append now lives inside composeReview, fed by a draftedComments seam that both CLI boundaries fill from the same array they count (and both strip from a model-written state JSON). Three end-to-end tests now assert on the body composeReview returns. - pr-context's side-file write preceded the mkdir that creates its directory and was unguarded: an ENOENT failed the whole command over a best-effort carry-forward. mkdir first, write guarded. - CANONICAL_LGTM_RE is anchored, so a trailing marker made every no-op round "worth showing" and prior rounds started rendering in full. The marker is now stripped before the filter and the count walk, not only before rendering. Plus the three thread findings: ledger cells are escaped before landing in the context table (a title with a pipe could forge rows), parseLedger takes the LAST marker and type-checks/normalises what it recovers, and the stray verdict doc comment is reattached to verdictLine. * fix(review): close the ten open findings on this PR - guard-true tested for a comparison anywhere after `if (`, including the then-body, so `if (ready) emit(a !== b);` admitted a mutant on the comparison-less condition the gate exists to exclude. It now tests the condition span only. - The `survived` detail said "when it changes" for legacy DELETION mutants too; it now matches the operator. - test-delta's `unparsed` required both sides to parse zero files, so a PR-side failure whose FAIL lines the trim scattered was silently dropped whenever the base rerun happened to parse. netNew/shared come from the PR side, so the PR side alone decides. - failingFilesOf now matches Windows path shapes (backslashes, C:) - a missed parse is an unattributed failure, not a loud error. - The replacement branch of runOneMutant (write-file -> run-probe -> classify) had no end-to-end test; one now drives a coalesce operator through the real handler and asserts the mutated line, the verdict, the operator-specific wording, and that the shared tree is untouched. - Two tests were vacuous with respect to what they promised: the baseline-dir test never asserted the cwd (its helper swallowed the argument - fixed at the helper), and the one-candidate-per-line test used an input that never triggered the replacement path, so the `continue` under test was not load-bearing. - Reattached the orphaned selectMutants JSDoc; reworded the SKILL line. * fix(review): the test helper's cwd parameter is required, not optional CI's `tsc --build` failed on test-delta.test.ts: the exec seam always passes a cwd, but the helper's signature marked it optional, so pushing it into a string[] was `string | undefined`. Missed locally because vitest runs through esbuild, which strips types without checking them - the suite was green while the build was red. The gate to run before pushing a type-level change is `npx tsc --build`, not the test suite. * fix(review): a base rerun that could not RUN attributes nothing Two Criticals from the latest review, both reachable on the brief's own happy path: - baseUnusable covered only timeouts. Every other way the base side can fail to run - an unbuilt base tree, a missing install, a workspace the PR ADDED (npm test --workspace cannot resolve on base), an ENOBUFS truncation - exits non-zero with zero FAIL lines, which this code read as a green base. Every PR-side failure then became netNew: the strongest evidence the command emits, manufactured from a base that never ran a test. It now attributes nothing and says why. - Timeout detection was the weaker substring form the sibling explicitly rejects; an external SIGTERM (container stop, cancelled job) set neither an ETIMEDOUT message nor an exit code and fed straight into the above. build-test now exports spawnTimedOut and test-delta asks the same question rather than re-deriving it. Also: the base output is trimmed (it precedes the verdict fields in the report the agent reads, so an untrimmed megabyte truncates exactly what the command produces); the guard-true gate no longer reads an arrow function's => as a comparison (every predicate guard was a candidate - the if (ready) noise the gate exists to exclude); the term-drop message no longer calls a string concatenation a reserve term; the unparsed note describes its own PR-side-only condition; and the ledger's Step 6/Step 8 now agree that a still-standing finding keeps its id. * fix(review): extract-step resolves all three env/defaults levels, and comments every env line Two silent-wrongness defects in a command whose whole value is fidelity. `env:`, `shell:` and `working-directory:` are three-level settings on GitHub — workflow, job, step, nearest wins — and only the step level appears in the step's own text. Reading step-level alone reproduced by machine the exact transcription error this command exists to remove: measured, a step under a job-level `NODE_ENV: production` and a workflow-level `GLOBAL_FLAG` extracted with `env: { LOCAL: '1' }` and `workingDirectory: undefined`, so the emitted script ran with both unset and nothing said so. Not a contrived shape: this repo carries workflow-level `env:` in 7 workflows, job-level `env:` in 10, and job-level `defaults.run` in qwen-triage.yml — the workflow the command's own test plan names. The three levels now merge with the runner's precedence, and `envSources` records which level each key came from, so an inherited value is visible rather than indistinguishable from the step's own. The env block was commented per ENTRY, not per LINE. A YAML block scalar (qwen-autofix.yml's `SETTINGS_JSON: |-`) reaches the header as a multi-line string, so its continuation lines landed in command position — and under the `set -e` the header itself emits, the extracted step died in its own preamble before its `run:` body ran. Every line is commented now. Tests pin the effect, not the mechanism: `executableLines()` asserts nothing but the `run:` body ever reaches command position, plus a `bash -n` parse check. Verified to flip — all five new assertions fail against the pre-fix implementation (`{ LOCAL: '1' }`, `undefined` working directory, three executable lines instead of one, and a real `bash -n` syntax error). * fix(review): compare failing files by a normalised, project-keyed identity Critical: the two sides run in DIFFERENT roots (the PR worktree and the base tree), and netNew/shared compared the parsed paths verbatim - so an absolute-path runner turned every pre-existing failure into a fabricated Critical, with the authority of a measurement behind it. Paths are now normalised against each run's own root (and backslashes to /, so a Windows path compares with its POSIX-printed twin), which is why test-delta gained --pr-worktree. The identity also keeps the vitest project token: dropping it collapsed same-named test files across workspaces, so a PR-caused failure in one package could read as pre-existing because another package has a file by the same name - the worse failure direction. Also from the same review, all of them reachable on the brief's own path: - The base rerun now inherits build-test's stdio: ['ignore','pipe','pipe'] ("a build that asks a question is a build that hangs until the deadline") and its trimOutput, which matters because entries[].base precedes the verdict fields in the report the agent reads. - The brief gates on base-tree's `available`, not just its `path`: a tree that was created but did not build populates path too, and measuring against it turns an infrastructure failure into Criticals. - A programmatic caller omitting `timeout` no longer sends NaN into spawnSync. - MutantCandidate is a discriminated union, so an operator without its replacement line - which would delete a line while reporting "with its ?? fallback dropped" - is unrepresentable. - The comparison class no longer requires a trailing space (if (a<b) is the same guard, just unformatted) and matches a brace-less else if. - DeltaEntry.unparsed's doc now describes the PR-side-only condition it actually implements. * fix(review): restore the whole-command budget, keep generics out of guard-true Round-1 findings from a fresh review of this PR: - test-delta had no aggregate deadline: --timeout is PER command and defaults to 300s, so three failed commands is 900s against Agent 7's 600s ceiling - killed with NO report at all, discarding the base-tree install and build just paid for. TOTAL_BUDGET_MS mirrors the one test-efficacy reserves; commands it cannot fit are disclosed. - guard-true matched generic calls: `if (isRecord<string>(v))` produced a mutant, and a type-guard predicate is exactly the `if (ready)` shape whose survivors the gate calls noise. The trailing \s is required, not an accidental asymmetry with [!=]== - telling `a<b` from `fn<T>(x)` needs a parser, and the gate is silence-biased by design. - --pr-worktree had no contract test, and its failure mode is the worst here: arriving undefined, root stripping silently stops and every pre-existing failure becomes a fabricated netNew. The new test feeds parseSync's output straight into runTestDelta and asserts an attribution only reachable when both roots were stripped (verified red against the snake_case field shape that shipped once already). - Merged the two consecutive doc comments on prWorktree. * fix(review): a budget-shortened deadline is not the same fact as a slow rerun Round-2 finding on the budget just restored: `Math.min(perCommandMs, remaining)` can hand a rerun far less than --timeout, and if it dies there the note said only "timed out - infrastructure, not evidence". True, but it sends the reader hunting a hang that is really an exhausted budget - and unlike a real timeout, a rerun with budget to spare would still measure it. The note now names those commands separately and says so. Verified red against removing the tracking line. * fix(review): brace-tolerant stub list, pipefail fidelity, and extract-step in the briefs Round-3 findings on this PR, fixed. `expressionsOf` matched `[^}]*`, so any expression containing a brace — `format('refs/pull/{0}/head', …)`, `fromJSON('{"a":1}')` — was not mis-listed but DROPPED. A stub list reads as "these are all the values to supply", so a silent omission is a value that never gets stubbed. It now scans forward to the closing `}}`, and reports nothing for an unterminated site rather than swallowing the rest of the text. Declaring `shell: bash` is not the runner's default `bash`. The default is `bash -e {0}`; a declared `bash` (at any level) is `bash --noprofile --norc -eo pipefail {0}`, and a pipeline whose middle stage fails aborts under one and not the other. The header now carries `set -eo pipefail` or `set -e` accordingly — 163 of this repo's 434 `run:` steps are under a declared bash and were getting the weaker one. A `shell:` value is also a command template (`perl {0}`), so only its first word goes in the shebang and the whole template is recorded beside it. `extract-step` was registered on the CLI and mentioned in DESIGN.md, and nowhere in SKILL.md or the agent briefs — the runtime prompts. The capability was unreachable by the agents it was written for. The verifier's brief now carries it next to the A/B paragraph it composes with, and Step 4 summarises it. Also: env ordered nearest-first (measured on qwen-autofix.yml:route:0, merge order put 20 inherited entries ahead of the step's own 26 in a 49-line header); a valueless `FOO:` renders as the empty string, not `"null"`, and a non-scalar as JSON rather than `[object Object]`; a missing file no longer reports as a parse failure; DESIGN.md's lens count matches its list. The test oracle is rebuilt around the property instead of a filter: the file is the header plus the body verbatim, and every line before the body is a comment or a directive the test names. The old helper dropped `set -e` unconditionally, so it could not tell the header's from one the body legitimately contains — and would have gone green on a header that leaked exactly that line. 434 real `run:` steps swept: 0 non-verbatim bodies, 0 live header lines, 0 missed expression sites, 0 out-of-order env, 0 `bash -n` failures. * fix(review): restore the replacement sub-cap, stranded on a downstream branch Round-3 finding, and the third instance of one class: an 8218-layer fix committed on the 8261 branch, four PRs above the code it belongs to. Measured over 40 real commits, the replacement operators produce ~24x the deletion pool (215 vs 9 candidates; guard-true drives it). Every mutant run drains the same window hunk probes draw from LAST, so uncapped, most diffs with any replacement candidates leave hunk probing zero runs - the hunk-survived finding class silently stops firing and nothing says so. Three slots, and what the sub-cap drops is counted in skippedForCap rather than lost. Also swept the other direction: diffed every review file against its 8261 copy to confirm nothing else 8218-layer is stranded up there. The remaining divergence is 8261's own (the positive control, its lenses). * fix(review): one severity predicate, carried ids, and a lossless marker Round-1 review of the ledger marker, six findings. Two contradicted contracts this package already states elsewhere: - `buildLedger` restated the severity predicate as a bare `startsWith` while `severityOf` — what `countInlineFindings` and the unmarked-comment gate both use — trims first. A Critical whose body opened with a newline was counted, was posted, blocked the merge, and was silently absent from the ledger, shifting the id of every finding after it. It classifies through `severityOf` now, and strips the marker with the shared prefixes instead of a second copy of the literals. - Ids were assigned by array position every round, so the report saying `R1-2 still stands` shipped beside a marker recording that claim as `R3-4` — and `LedgerFinding.id` documented itself "stable across re-reports". Step 6 now mandates writing the original id into the re-report (`**[Critical]** R1-2: …`), `buildLedger` reads it back, and a duplicate or absent id falls back to the next free id of this round. Four smaller ones: - `stripLedgerMarker` removed only the FIRST marker while `parseLedger` reads the LAST, so a body holding two leaked the trusted marker into model-facing prose and un-filtered the canonical LGTM it was appended to. - The `--` guard rewrote data to an em dash: a finding about `--comment` reached the next round as `—comment`. It escapes at the JSON layer now, which is comment-safe and lossless. - `file` was capped on read (200) but not on write; the cap is one constant binding both halves. - `submit`'s path — the one GitHub's payload is built from — had no test that the marker reaches the posted body. Plus: tie-break `latestOwnLedger` on review id when `submitted_at` ties, neutralise a backtick in the location code span, skip the `currentUser()` round-trip when the PR has no reviews, and say why a stale side file is deliberately left in place. * docs(review): complete the "delta cannot rule" enumeration in both places Round-4 finding. The brief and SKILL.md each listed three cases where test-delta attributes nothing - unparsed, timed-out base, no merge base - but the code has five: the later rounds of this PR added "a base rerun that failed without naming any failing file" (it did not measure the base) and "a command the whole-command budget could not fit". Two enumerations of the same set with different membership, in the two places an agent reads. That is the sibling-enumeration lesson this skill teaches, applied to its own prose for the second time: the fix is not just adding the missing members but saying that the report names each case with its own reason rather than folding them into one. * fix(review): the invokes list was mostly prose, not commands Round-4 finding on this PR. `invokes` is documented as a heuristic starting point, and imprecision is fine — but measured over this repo's 434 real `run:` steps it was reporting 435 distinct "commands", 267 of them appearing exactly once, with a worst case of 63 entries made up of words like `CI`, `Evidence`, `PR` and `and`. A list that size, mostly prose, is not a starting point. Three causes, each measured: - A `${{ … }}` expression is not shell, and it routinely contains `||`. Splitting on that as a pipeline separator reported both operands as commands (`matrix.arch`, `github.event.inputs.version`). Expressions are now masked to an opaque token before the split; one sitting in command position contributes nothing, which is honest — what it expands to is unknown here by design. - A heredoc body is input to a command, not a list of them. Its lines were scanned as commands, terminator included. 12 steps in this repo carry one. - The `name=value` skip stepped over the prefix and took the NEXT word as the command — but for a quoted value with spaces that word is inside the value: `EVIDENCE_SECTION=$'### Evidence images'` reported `Evidence`. Quoted spans are now blanked out, with the quote carried across lines so a multi-line string's continuation lines are data too. Command substitutions are read first, so `body="$(sanitize < "$REPORT")"` still reports `sanitize`. A `#` preceded by whitespace ends the live part of a line, so an apostrophe in a trailing comment cannot open a span and eat the rest of the script. Measured after: 435 distinct commands to 187, singletons 267 to 104, worst case 63 to 27 — and the worst case is now real commands (`awk cat chmod curl git jq mktemp pkill tar tee timeout`) plus the script's own shell functions. Also added the builtins a stub could not intercept anyway (`eval`, `exec`, `source`, `unset`, `command`, …) to the keyword set. 434 steps re-swept: 0 non-verbatim bodies, 0 live header lines, 0 missed expression sites, 0 out-of-order env, 0 `bash -n` failures. * docs(review): the rationale named only one of the ways base goes unmeasured Third and last copy of the enumeration the previous commit fixed. The bullet's headline already generalised - "base attributes nothing it did not finish" - but its body named only the timeout, so a reader learning the contract from the rationale would conclude timeouts are the only unusable case. Name the set, and say why the report keeps the reasons apart: "we could not measure" and "we measured nothing" are different facts to the author, and only one of them is about their PR. * fix(review): a quoted `<<EOF` is not a heredoc, and a continued line is one command Round-5 findings, both in the previous commit's own scanner. A heredoc opener was matched over the whole line, so one inside a string started heredoc mode: `echo "write <<EOF for a heredoc"` made every later line wait for a terminator that never arrives. The failure is not a missing entry but a missing REST — measured, a three-line script returned `[]` instead of `[curl, jq]`, empty and entirely plausible. Opener detection moved inside the quote walk, where it only fires outside quotes; the quoted forms (`<<'EOF'`) are consumed by the match, so their quotes never open a span either. A backslash-continued command was scanned as several lines, which puts the next ARGUMENT in command position — this is where `apt-get install -y \` / ` libx11-dev` reported the package as an invoked command. Continuations are now joined into one logical line before scanning. Measured after: 187 distinct commands to 185, and the singleton tail is now dominated by real ones — PowerShell cmdlets, macOS tooling, and the scripts' own shell functions. 434 real steps re-swept: 0 non-verbatim bodies, 0 live header lines, 0 missed expression sites, 0 out-of-order env, 0 `bash -n` failures. * fix(review): case labels, a second heredoc, and an expression in command position Round-6 findings, from running the scanner against adversarial shapes rather than reading it again. Two are UNDER-reports, which is the worse direction: a command missing from the list is a stub the verifier never writes, so the extraction reaches the real network. - A `case` pattern label stopped the scan on its own line: `blocked) gh api x` reported nothing, losing `gh`. The label is now stepped over like a `name=` prefix. - Only the first heredoc opener on a line was tracked, so `cat <<A <<B` left the second body and its terminator read as commands (`B`, `y`). Openers are queued and consumed in order. - Masking an expression to a QUOTED token let the quote-stripper delete it entirely, so `${{ steps.x.outputs.cmd }} arg` reported `arg` as the command. The token now survives stripping and cannot match a command word, so an expression in command position contributes nothing. Ten further adversarial shapes were already correct and are pinned as regression guards: nested `$( )` in quotes, subshells, function definitions, indented heredoc terminators, backticks, bare redirects, adjacent and empty `${{ }}` sites, and a JSON literal inside an expression. Verified to flip — exactly the three above fail against the previous commit, the other ten pass. 434 real steps re-swept: 0 non-verbatim bodies, 0 live header lines, 0 missed expression sites, 0 out-of-order env, 0 `bash -n` failures. * fix(review): annotate the continuation accumulator so tsc can type it `invokedCommandsOf`'s backslash-continuation loop failed to compile with TS7022: the narrowed type of `pending` at the join line is the union of the loop-entry value and the back edge, and the back edge is computed from the join itself. The declaration's own annotation does not break that cycle - control-flow narrowing runs after it - so the checker gives up and calls the result `any`. Caught only on a forced rebuild. `tsc --build` had been reporting this tree clean off a stale .tsbuildinfo, which is the same shape of gap that put a type error into CI last week: the test runner strips types, so the suite stayed green either way. Gate with --force. * fix(review): a file-count label stops counting at the end of its line `Test Files 45 passed` filing its 45 as a differing TEST count was fixed once, with a lookbehind on the bare-count pattern. That only ever rejected the all-green shape. The moment any file fails, the runner prints `Test Files 1 failed | 44 passed (45)`, the label is no longer adjacent to the number, and 44 comes through as a test count - so the note reads "claimed 44, observed 1323" on exactly the runs whose summary someone would paste. Adjacency was the wrong invariant; the line is. Masking from the label to end-of-line is distance-independent and picks up jest's `Test Suites: 1 failed, 44 passed, 45 total` at no cost. The label keeps its `Test` word on purpose: a first cut matched a bare `files` and blanked the line in "expect all four files and 471 tests to pass", silencing a real claim. An existing test caught that, which is the argument for the rule being as narrow as it is - anything that suppresses claims is worth exactly its narrowness. Also drops the now-dead lookbehind, which a reviewer had separately (and wrongly) called a JS syntax error; variable-length lookbehind is legal in V8, and the module parsed fine. It goes because the line rule subsumes it, not because it was broken. * fix(review): address round-1 review of the mined-disciplines batch The positive control could state a verdict it never measured, and its own failure path shipped untested. Ten findings from the review of #8261: - `runControlMutant` returns `boolean | null`. An unreadable probe file injected no test and ran nothing, but returned `false` — which the caller reads as "the injected always-failing test stayed green", re-classes every survivor with that sentence, and discards the whole mutant/hunk window over an I/O error. `null` is the file's own third-outcome rule; the window stays spendable and the note says the harness was not validated. - `mutants.skippedForControl` / `hunks.skippedForControl`. A control that came back red stopped the run with candidates unprobed while every `skipped*` counter read 0 — the silent cap the brief teaching agents to read those counters rules out. Their own reason, not the budget's. - Cover the control's failure half: a dead-runner integration case (survivors re-classed, revert probe's `inert` re-classed, findings empty, nothing spent) and a could-not-set-up case pinning that `null` does NOT discard the window. Both branches, and the re-class block, were previously unreachable by any test. - `rulePath`: one existence check, not two. The second was unreachable behind the first and its note — "exists at the reviewed commit (the diff does not change it)" — silently retired, collapsing tracked state and this run's build output onto one sentence. The ignore status now picks the wording. - `expressionsOf`: forbid the body from crossing another `${{`. A malformed site above a real one swallowed it into the blob, so `${{ github.event.comment.body }}` stopped being enumerated as an injection site — the one direction this helper must not fail in. - `countDiffChangedLines`: count by hunk position, not prefix shape. A deleted line whose content starts `--` arrives as `--- …` (markdown rules, YAML markers, SQL comments), and every dropped line pushes the ratio toward a false `collapsedFromUpstream`. - `emptyDiff` is gated on `baseFetchFailed` too. A stale local base ref that already contains the head commits diffs to empty, and the skill acts on that by recommending close-as-superseded. - `test-delta`: price the budget slot against the command's own measured duration instead of a flat 5s floor, which admitted commands that could only time out and then disclosed them as "infrastructure, not evidence". `skippedForBudget` becomes a structured field; `now` becomes a seam so the test stops reassigning the global `Date.now`. - Drop `probeCreateFailureDetail` (no production caller after the swap to `worktreeCreateFailureDetail`) and the duplicated base-lock sweep in `cleanup.ts`, ten lines above the identical pre-existing one. - `--` before the path in `check-ignore`, and the comment says plainly that no `-`-leading claim survives extraction today. * fix(review): parse the base rerun before its output is trimmed `trimOutput` rescues module errors and runner summaries out of the omitted middle, not the per-file FAIL lines this command reads. A base suite whose failure section overruns the tail budget therefore lost failing files into the gap - and a SHORT base set is the dangerous direction, because netNew is the PR side minus the base side. Every file the trim hid came back as a Critical attributed to this PR by "measurement". Parse the raw text, report the bounded one. The PR side cannot be fixed here: it is read out of build-test's stored output, trimmed before this command existed. That loss runs the other way - it understates `shared`, never invents a netNew - so the entry carries `prTruncated` and the note says the list may be partial. A silence-biased gap is still a gap the author should hear about. Also names both selection caps in the mutant-skip diagnostic. The count accumulates replacement sub-cap drops, and with 2 deletions and 6 replacements the total is exactly MAX_MUTANTS: the main cap never fires, yet 3 are dropped, and the message sent the reader looking for a pool of 11 candidates that never existed. * test(review): pin the `sh` set-line to exactly what the runner uses A reviewer asked for `expect(script).not.toContain('set -e')` on a `shell: sh` step. The opposite is correct - GitHub runs that step as `sh -e {0}` - but the thread was right that nothing pinned it either way. Assert both halves: `set -e` is present, `pipefail` is not. Dropping the line makes an extracted `sh` step run past a failure the runner stops on; adding pipefail claims a bash feature `sh` does not have. * fix(review): rerun only the command shapes build-test emits This command reads a report off disk and then executes the strings in it with `shell: true`, in the base worktree. Nothing else in the pipeline re-executes a value it read back from a file, so nothing else has to care where that value came from - this does, and the provenance is worse than it looks: the command is `npm test --workspace="<dir>"`, the workspace token is a directory, and a directory is a name a pull request chooses. `packages/x";curl …|sh;"` is a legal path in git and on Linux and it round-trips through the report into a shell. Restricting to the emitter's own grammar costs nothing real, since that grammar is the two shapes build-test produces. A command outside it is skipped and disclosed, the same treatment everything else this command cannot do already gets, so a future shape degrades to "judge it by the diff" rather than to arbitrary execution. * test(review): pin the empty-diff and collapse detectors Both lived inline in a large object literal with no test, and one of them is the most expensive disclosure this command emits: the SKILL acts on `emptyDiff` by recommending the PR be closed as superseded. Dropping the 200-line floor, flipping the 4x comparison, or losing either empty-diff guard turned nothing red. Extracted as two pure predicates and pinned at the boundaries, including the two guards that are not about the diff at all - a capture that threw leaves diffText empty, and a merge base resolved from a stale local ref can already contain the head commits. Either would have closed a live PR on an infrastructure error. Checked by mutation rather than by reading: all four edits above now fail exactly one assertion each. * test(review): pin the 30s budget floor, and stop keeping its arithmetic twice Two findings from the same review round, both about budget math nobody was checking. The 30s floor separates "skipped, judge it by the diff" from "started, timed out, disclosed as infrastructure". Every existing budget test left it non-binding - two sit hundreds of seconds from the boundary and the third is capped by the per-command deadline - so regressing it to the old 5s turned nothing red. Verified by doing exactly that: 26 green before the new test, one red after. A one-second command admitted into a twenty-second window comes back labelled `budgetClamped`, which is the mislabelling the priced floor exists to prevent. The other is the same duplication lesson this skill keeps arriving at, in its own tests: one test carried the per-run budget arithmetic in a block comment AND at the `now:` argument, and when the per-run figure changed only one copy moved. The block also predated the positive control, so it omitted a whole suite run. Deleted the copy rather than correcting it - two agreeing copies today are two disagreeing copies later. * fix(review): the working directory is a setting the extraction was losing Two findings, both of them this file failing its own stated argument. The stub list dropped it. `expressions` covered the script and the env and nothing else, so a `working-directory: ${{ github.workspace }}/x` produced an empty list and a summary line reading "0 ${{ }} site(s) to stub" - and `expressionsOf`'s own comment says why that is the failure this list cannot afford: the caller reads it as "these are all the values to supply". Widened to every setting the command carries, the `shell:` template included. The emitted script never mentioned it. The env block is commented into the header precisely so a reader of the script alone can see it; the working directory changes what the script does just as much and was in the metadata only. The argument for reading all three levels, written in this file, is that a step run "in the wrong directory, and nothing says so" is the transcription error the command exists to remove - which is exactly what the header did. It is a comment rather than a `cd` for the same reason env is comments, not exports: the value may hold `${{ … }}` and this command substitutes nothing. Both pinned, and both checked by deleting the fix: each mutation fails exactly one of the two new tests. * fix(review): $(( )) is arithmetic, and a heredoc's form decides where it ends Three fixes and one deliberate non-fix, all measured against this repo's own 434 `run:` steps rather than argued. `$(( ))` was read as a command substitution, so `N=$((N + 1))` reported `N` as a command to stub. It was the single largest source of junk in the list: 196 distinct "commands" across the corpus, 165 without it. A plain `<<WORD` heredoc ends only on a line that is exactly WORD; the loose match ended it on an indented `EOF` inside the body and then read the body as commands, which is how `rm` got reported for a script that never runs it. `<<-` stays looser than bash (any leading whitespace, not just tabs) because looser can only end a body early, and this file's priority is that an under-report is the worse direction. `[^()]*` matched only the innermost `$( )`, so `X=$(gh api $(u))` lost `gh` - a missed stub, and the extraction reaches the network. Depth counted now, and the assignment-prefix skip no longer steps over an unclosed `X=$(gh` into reading `api` as the command. The non-fix is recorded where the next reader will hit it: the quote walk is flat while shell quoting nests, and over ~300 lines the drift reports fragments of jq source as commands. Inserting a separator where a blanked span was removes nine of those, but it splits `a"X"b`, which is one word to the shell, and the minimal reproducer for the difference is 296 lines - nothing short enough to pin it. A scanner nobody can pin costs more than the junk it removes. * fix(review): refuse an ambiguous step name, and report errors like the siblings Two findings, both about a message the caller never gets. A job may legally hold two steps with the same name. The selector took the first and said nothing, which is the failure this file's own header names - "picks the same-named step from the wrong job" - and it is worst in the use the command exists for: A/B extraction runs it once per tree, so a PR that adds or reorders a duplicate leaves the two sides comparing different steps while reporting on one. Refused out loud now, naming the indices; the index is always available and never ambiguous. The handler also let every throw propagate, so five carefully separated messages - cannot read vs cannot parse vs no job vs no step vs no `run:` - all arrived as "An unexpected critical error occurred" under a stack trace. `base-tree` and `test-plan` in the same directory already catch, write the message, and set exit 1. Matched. The separation between "the path is wrong" and "the YAML is wrong" only pays if the caller sees it. * docs(review): the brief's list of limits was one short "Two limits worth knowing before you spend the step" became three when the selector started refusing an ambiguous step name, and the count went stale in the same commit that made it wrong. This is the enumeration drift the skill teaches, in the text that teaches it. The added entry says what to do rather than only what happens: pass the index, which is what an A/B wants regardless — the two trees have to select the same step, and a name that moved between them is exactly how they stop doing that. * chore(review): drop a scratch probe file that reached the branch `packages/cli/inert.mjs` was a throwaway harness for sweeping the repo's own workflows; its cleanup ran with a relative path from the wrong directory and it got committed by the next `git add -A`. It failed CI and not the local hook because the two lint different sets: lint-staged filters to `*.{js,jsx,ts,tsx}`, which does not include `.mjs`, while CI's flat config picks the file up regardless of `--ext`. Verified with CI's own command this time, not the hook's. * fix(review): the collapse disclosure needs the guard its sibling already has Three findings from the round on this branch, all accepted. `isEmptyDiff` refuses to rule when the merge base came from a possibly stale local ref, because such a base can already contain the head commits and diff to empty. The PARTIAL form of that cause lands on `isCollapsedFromUpstream` instead - a stale ref holding most of the head commits shrinks the recomputed diff past the 4x ratio - and the flag then tells Agent 0 to read the body as description-of-history when the body may be perfectly current and the real cause is a fetch failure. Two predicates over the same evidence, one guarded and one not. Guarded now, and pinned: removing the guard fails exactly one test. The pre-set of `mutantsSkippedForBudget`/`hunksSkippedForBudget` in the no-budget-for-control branch was dead in every path and worse than dead in one: both loops run with `harnessValidated` still null, re-check the same budget, and set their own counters - and the hunk loop's figure excludes the collocated-probe inconclusives it pushes first, which the pre-set did not. Dropped, with the reason where the assignment was. `runControlMutant`'s bound is now stated in its doc: it validates ONE file, so a collector that silently drops a different probe file passes the control while that file's survivors stand. The per-file baseline gate bounds what is left. A `true` there is read as covering the run, so what it does not cover belongs next to it. * fix(review): bound the whole ledger marker, not just each of its fields The per-field caps leave the total unbounded: fifty findings at full width serialize to just under 17,000 characters. The module's own paragraph calls the marker "a footnote, never a payload" and reasons about GitHub's 65,536-character body limit, so the total is the number that claim is about and nothing was computing it. Set the budget from measurement rather than from the limit. Across every review this pipeline has posted on its own stack (n=66) the body runs a median of 721 characters, p90 2,178, max 3,925 - the limit has ~61 KiB of headroom, and an over-long marker was never going to fail a post. The real cost is proportion: 17,000 characters of invisible payload on a 3,925-character review is four times more marker than review. 8 KiB holds fifty findings at realistic widths without dropping any, and the truncation path is reached only by a ledger no round has produced. What it drops travels with it. A `dropped` count is the difference between a list the next round reads as complete and one that says it is not - the same distinction this module already draws everywhere else. * fix(review): a partial ledger must not render as a complete one The size cap added in the previous commit can drop entries, and the rendered section still opened with "**Every entry below is owed a this-round ruling**" and said nothing about what was missing. The `dropped` count existed and reached the only place a reader sees the list as nothing at all — which is the failure it was added to prevent, committed one step after adding the field. It now says the list is partial, how many are missing, and that absence is not evidence a finding was fixed. Second, the table escaping escaped `|` but not `\`. `\|` in a title became `\\|`, which markdown reads as an escaped backslash followed by a LIVE separator: the forged row the escaping exists to prevent, produced by the escaping. Backslash first now, and the test counts separators the way markdown does rather than by pattern, because a lookbehind counter is fooled by exactly the input under test. * fix(review): an empty ledger title is a cap the next round cannot lift A comment that is nothing but its severity marker produced a ledger entry with an empty title. That does not merely degrade the entry, it jams the review: the next round is told every ledger entry is owed a this-round ruling, has no claim text to rule on, answers `cannot tell`, and `cannot-tell-existing-critical` is one of the cap reasons. Nothing between rounds changes, so the cap comes back every round. Dropping the entry would hide a Critical that really was posted, so it keeps its place and gets the one handle there is - the location it was filed at, or "the review body" for a body-level one. The field's stated job is "enough for the next round to re-locate the claim", and that is what a text-less finding still has. Found by probing `buildLedger` with ten malformed comment shapes; the other nine - carried ids, id collisions, two comments carrying the same id, `R2D2` not being an id, a leading newline before the marker - all behaved correctly and needed no change. * fix(review): count both caps as dropped, not just the byte one Found by running the three previous fixes together on one round: 51 findings in, 24 kept, and the marker said 26 were missing. `LEDGER_MAX_FINDINGS` truncates before the byte cap ever runs, and measuring `dropped` against the already-sliced list left the count cap's share silent - so the field added two commits ago to stop a truncated list reading as complete was itself under-reporting the truncation. Measured against what came IN now, and pinned across all four combinations: count cap alone, byte cap alone, both, and neither. Kept plus dropped equals given, in every one. * fix(review): the ignore check was the one git spawn without a deadline `isGitIgnored` reached for a bare `execFileSync` while every other git invocation in these commands goes through the package helper, which carries `GIT_TIMEOUT_MS` and `GIT_TERMINAL_PROMPT=0`. That constant's own comment is the argument: "a hang must still end". This one ran against a worktree the review does not control and could not. No behavioural delta for any input that does not hang - all 80 test-plan tests pass unchanged - so it ships without a new test rather than with a mocked-spawn assertion that would pin the call shape instead of the behaviour. * docs(review): a control killed mid-run did not "never run at all" SKILL's `harnessValidated` paragraph enumerated four ways the value lands on `null` - no green baseline, no candidates, no budget, an unreadable probe file - and all four are "the control never ran". There is a fifth: `runControlMutant` throws when its deadline kills the run or the runner cannot be spawned, the outer catch swallows it, and the value stays `null` rather than becoming a fabricated `false`. That is the path a slow machine takes. The ruling is unchanged - neither validated nor refuted, a survivor stands but unconfirmed - so this is about what the terminal says, not what it decides. An agent quoting "the control never ran at all" for a control that ran and was killed states something untrue about the run, and the enumeration is what it quotes from. --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Code <qwen-code@users.noreply.github.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>	2026-08-01 16:35:29 +00:00
Shaojin Wen	05260afc31	feat(review): carry the round ledger in the posted review body (#8255) ... * feat(review): borrowed-verification trio — test-plan check, base-tree A/B, per-hunk probes - qwen review test-plan: rule on the PR Test Plan's checkable claims (paths, npm scripts, test counts) against the reviewed tree; contradictions and differing counts are disclosed via compose-review, never capping. - qwen review base-tree: build the merge base in a sibling worktree so the verifier can A/B a comparative claim instead of reading it; swept by cleanup. - test-efficacy: third probe kind — reverse-apply one hunk at a time and re-run the affected tests, attributing a still-green suite to the specific change nothing gates; shares the mutants' budget window, runs last. * fix(review): survive real runner output — ANSI-laced and trimmed-away summaries Both measured on a live /review of QwenLM/qwen-code#8176 with the built CLI: - test-plan's observedTestCounts strips SGR sequences before matching; a color-enabled pipe interleaves them BETWEEN tokens, and the count claim fell to 'unchecked' with the summary right there in the report. - build-test's trimOutput rescues runner summary lines from the omitted middle (like module-resolution errors): a failing suite's tail is all failure details and npm epilogue, which pushed the one-line summary out of the kept text entirely. * fix(review): address the eight findings from live review of this branch All measured in the review (QwenLM/qwen-code#8215 review comment): - test-plan: linear-time bold-heading scan (the old pattern backtracked catastrophically on an unclosed ** line an untrusted PR body controls); a flag preceding the npm script yields no claim instead of a false 'no package defines this script'. - test-efficacy: a hunk probe's restore recreates the parent directory a reverse-applied 'new file' hunk removed (the ENOENT from finally lost the verdict and marked every remaining hunk inconclusive); hunks get their own skippedForBaseline instead of mislabeling a red baseline as a budget skip; splitDiffIntoHunks re-captures the file header at every diff --git boundary; a hunk-survived finding notes when it restates an inert file-level revert at hunk granularity. - base-tree: idempotent fast path keyed on a build marker + HEAD check — concurrent verifier shards reuse one built tree instead of sweeping it out from under each other mid-A/B (a fabricated base-side difference with a deterministic source tag was the worst case); cost wording is now 'an install and a build' everywhere it was 'one extra build'. * fix(cli): never score a hunk survived when its own test left the baseline (#8215) A per-hunk probe reported `survived` whenever the green baseline probes still passed with the hunk reverted. When the hunk's own collocated test dropped out of the baseline (a probe-tree import error collects nothing), the remaining green probes prove only that THEY do not cover the hunk, so the verdict is now `inconclusive` — the same dropped-test asymmetry the mutants already hold. Also scope the hunk-survived cross-reference note to the hunk's own collocated test, and let test-plan match a workspace-scoped run of the plan's bare command instead of falling through to the manifest on an exact-string miss. * fix(review): silence-bias hardening from four live review rounds of this branch The two blocking findings, reproduced on this PR's own Test Plan: - test-plan files no false contradicted notes: npm rulings move from a four-verb denylist to an allowlist (the run form + npm's script aliases — the ~fifty other builtins each used to become 'no package defines this script'); a slash token is claimed as a repo path only with evidence (an extension or ./ prefix), never when it is a flag's value (--repo owner/repo) or under the review's own temp root; HEADING_LINE_RE drops the same quadratic shape its bold sibling was rewritten to remove. - base-tree gets a real mutual-exclusion lock around sweep+add+build (mkdirSync test-and-set; the loser returns busy instead of deleting the tree the winner is mid-install in), and a failed build writes a settled marker so later shards stop re-paying the install to relearn 'unavailable'. Also: Agent 7's brief now names hunk-survived and the hunks.* counters (it is the report's only consumer, and the finding class was invisible); hunk findings anchor at the first ADDED line instead of up to three context lines above the change. * feat(review): four round-2 borrowings — measured failure attribution, round ledger, richer mutants, doc parity Shaped by a live round-2 maintainer re-verification (QwenLM/qwen-code#7818): - qwen review test-delta: rerun the PR side's failed test commands on the built merge base and diff the failing FILE SETS — netNew is the PR's own failure by measurement (whatever files the diff touches), shared is pre-existing by measurement. Counts are never compared: a flaky suite fails different test names between runs of the same tree. An unfinished base rerun attributes nothing. - Round ledger: the incremental review cache persists confirmed findings under round-scoped ids (R1-2); a high-effort re-review rules on each (fixed / still stands / cannot tell) and opens its report with the table, the way a human round-2 comment opens with 'M1 is fixed'. - Three replacement mutation operators in test-efficacy: drop '?? fallback', force a comparison-bearing guard condition to 'true', drop a '+ CONST' term. Each survivor maps to one crisp untested-protection sentence. A line whose raw text and literal-blanked code view disagree yields no candidate — an edit index computed across the two views once spliced 'iftrue 0)' into a guard, and a mangled mutant reads as inconclusive while quietly spending a cap slot. - Quality brief: documentation-parity lens — a new user-facing surface whose siblings are documented is a Suggestion naming the sibling precedent; no documented sibling, no finding. * feat(review): render adjudication, workflow step extraction, three verification lenses Shaped by a live sanitizer-PR verification (QwenLM/qwen-code#8147): - Render-adjudication capability (opt-in): with QWEN_REVIEW_SCRATCH_REPO set, the verifier may post a minimal payload to that user-designated repo and rule on GitHub's own rendered HTML — the measured case being an @ -> &#64; defusal every local reading called sound while GitHub's real renderer registered the mention and fired the notification. Absent the setting, rendering claims honestly cap at low confidence / cannot tell. Step 7's write ban names the carve-out explicitly. - qwen review extract-step: lift one workflow step's run: script verbatim into an executable, with env (as comments, never half-substituted exports), every ${{ }} site listed unevaluated, and a heuristic invoked- command list as the stubbing starting point. With base-tree, both arms of a by-hand workflow A/B become two invocations. yaml declared as a cli dependency (previously resolved only via hoisting). - Three brief lenses: a borrowed protection idiom missing what made it work at home (the code ancestor did the protecting; only the entity was copied); a second parser for an authoritatively-parsed format is a divergence hunt; tests that pin the mechanism instead of the effect, and oracles that mirror the implementation's own model. * feat(review): sibling-entrance discipline for the fixed verdict From round 6 of the live sanitizer verification (QwenLM/qwen-code#8147): the fix closed the fence-shaped entrance into a raw-HTML block, and the code-span entrance beside it — same divergence, adjacent syntax — stayed open. A re-check that tests only the reported input rules 'fixed' over a hole one backtick away. Both fixed-verdict sites in SKILL Step 6 (the open-Criticals re-check and the round ledger) now require enumerating a divergence-class defect's sibling entrances before ruling fixed; a still-open sibling is a NEW finding, never a reason to withhold the original's fixed — the two rulings stay separate so the second hole cannot ship unreviewed. * feat(review): three measured-verification lenses from live rounds 8037/8005 - Threshold-boundary scan: when a fix is a ratio/length guard, hold the issue's own variables fixed and binary-search the boundary where the behaviour flips; put the number next to what the issue reports. Live case: a prose-ratio guard covered the edit/write_file half of its issue and silently declined the run_shell_command half (~473-char boundary). - Delimiter self-injection named as the first parser-differential probe: a no-escaping extractor fed its own close tag truncates silently. - Shared-gate state enumeration: a deliberate-design defence extends only to the states it argues — an input-hold argued for 'active' silently froze three idle states sharing the same gate. The sibling-entrance rule, applied to a state machine instead of a syntax. * feat(review): carry the round ledger in the posted review body The ledger shipped as a local cache file and its first multi-round live use exposed the flaw: four model-comparison rounds re-reviewed the same two PRs and every round opened from scratch — medium never reads the cache, and from CI or another clone there is no cache at all. The one artifact every environment can see, the posted review, carried nothing machine-readable. - compose-review embeds an HTML-comment marker (invisible on GitHub) carrying this round's findings and round number; the round comes from the side file pr-context recovered, +1 — never from the model. - pr-context recovers the reviewing account's latest marker, renders it as a 'Previous /review round (machine ledger)' work-list section, and writes it beside the context file for compose-review's round counter. Own-account only, latest round wins, fail-quiet on any malformation — the marker is data the next round rules on, never authority it obeys. - Step 6's previous-round ruling now runs at medium too (recovery is free; medium still writes no cache and posts nothing); the local cache is demoted to fallback for rounds that never posted. * fix(review): address review feedback — false-positive hardening, binary diff guard, error convention (#8215) * fix(review): address review feedback — base-tree availability gate, test-plan false positives, hunk-probe ranges (#8215) - base-tree: only stamp a base tree available when runBuildTest actually compiled something (ok AND npm toolchain AND a non-empty build). An `unsupported` handoff or an empty npm scope returns ok:true having built nothing; marking that tree available let an A/B read the absence of a build as a behavioural difference. - cleanup: sweep the stale base-tree build lock a killed builder leaves behind. - test-plan: read the root manifest's scripts directly so a root-only script survives when the root defines no build/test; bail on the inline --root=./dir rebasing form; stop treating a positional after an inline --flag=value as the flag's value; prefer a failed scoped run when ruling a bare command; anchor the npm script alias to a full token so `yarn test:unit` is not truncated. - test-efficacy: exclude `\ No newline at end of file` from the startLine offset count; compute the mutant-overlap range from the header's new-side span so it no longer overshoots into a closely following hunk. * fix(review): address review feedback — diff-header false positives, stale prompt enumeration, added-file hunk probes (#8215) * fix(review): address review feedback — cd-base exclusion, Test Files count guard, base-tree error handling, probe delegation (#8215) * fix(review): port the collocated-dropout test to the post-#8050 runner seam Merging main brought #8050's Windows-portability refactor, which resolves the probe runner through vitest/package.json's bin — a node_modules/.bin fake is dead weight it never reads. The 8215-only collocated-dropout test still installed the old .bin fake, so the REAL vitest ran its fixtures, price.test.ts genuinely passed, and the hunk scored survived. The test now overrides the fake package's vitest.mjs like every post-refactor test. * fix(review): bound the summary rescue, apply the ATX heading rule, sweep stale build locks The three 8215-layer findings from the latest review, fixed at this layer (they were first patched further up the stack, where the reviewer of THIS PR cannot see them): - trimOutput's summary rescue is capped at 40 lines — uncapped, 40k lines of 'Test <n>: …' prose voided the trim entirely (measured 1.6MB in, 1.6MB out) and the bounded-output contract is the whole point. - A '#' with no following whitespace is prose, not a heading (the ATX rule GitHub applies): '#8176', '#tag', an unfenced '#!/bin/bash' no longer end the Test Plan section mid-body; the bare-#-run crash on the closing scan is guarded. - A base-tree build lock older than 30 minutes is a corpse left by a killed builder — swept and rebuilt instead of reporting busy for the rest of the review. * fix(review): EEXIST-only lock busy, bun test alias, chained cd bail, fence backreference Four live findings from the latest inline review round (the rest of the round was already fixed upstream by the takeover bot - verified by probing head behavior rather than re-reading the threads): - base-tree's lock catch distinguishes EEXIST (a concurrent builder, busy) from EPERM/EROFS/ENOSPC (this run's own failure, reported as such, not as a busy that will never clear). - "bun test" is bun's built-in runner, not a package-script alias: it runs whether or not any manifest defines test, so ruling it against the scripts table filed a false contradicted. - A chained cd matches the leading-cd shape but the single-hop resolver joined file tokens against the FIRST directory; it now bails like the exotic-cd case. - codeSpans' fence regex closes on its own marker via backreference; a tilde fence line inside a backtick block ended the span early and lines after it were lost to extraction. * fix(review): make the ledger marker actually reach the posted body Three Criticals from the #8255 review, all real: - The marker was appended in the CLI handler, AFTER composeReview() returned, so it only ever reached the composed JSON on disk. submit posts what the PURE function returns, so no posted review carried a marker, latestOwnLedger always returned null, and the whole feature was inert end to end. The append now lives inside composeReview, fed by a draftedComments seam that both CLI boundaries fill from the same array they count (and both strip from a model-written state JSON). Three end-to-end tests now assert on the body composeReview returns. - pr-context's side-file write preceded the mkdir that creates its directory and was unguarded: an ENOENT failed the whole command over a best-effort carry-forward. mkdir first, write guarded. - CANONICAL_LGTM_RE is anchored, so a trailing marker made every no-op round "worth showing" and prior rounds started rendering in full. The marker is now stripped before the filter and the count walk, not only before rendering. Plus the three thread findings: ledger cells are escaped before landing in the context table (a title with a pipe could forge rows), parseLedger takes the LAST marker and type-checks/normalises what it recovers, and the stray verdict doc comment is reattached to verdictLine. * fix(review): close the ten open findings on this PR - guard-true tested for a comparison anywhere after `if (`, including the then-body, so `if (ready) emit(a !== b);` admitted a mutant on the comparison-less condition the gate exists to exclude. It now tests the condition span only. - The `survived` detail said "when it changes" for legacy DELETION mutants too; it now matches the operator. - test-delta's `unparsed` required both sides to parse zero files, so a PR-side failure whose FAIL lines the trim scattered was silently dropped whenever the base rerun happened to parse. netNew/shared come from the PR side, so the PR side alone decides. - failingFilesOf now matches Windows path shapes (backslashes, C:) - a missed parse is an unattributed failure, not a loud error. - The replacement branch of runOneMutant (write-file -> run-probe -> classify) had no end-to-end test; one now drives a coalesce operator through the real handler and asserts the mutated line, the verdict, the operator-specific wording, and that the shared tree is untouched. - Two tests were vacuous with respect to what they promised: the baseline-dir test never asserted the cwd (its helper swallowed the argument - fixed at the helper), and the one-candidate-per-line test used an input that never triggered the replacement path, so the `continue` under test was not load-bearing. - Reattached the orphaned selectMutants JSDoc; reworded the SKILL line. * fix(review): the test helper's cwd parameter is required, not optional CI's `tsc --build` failed on test-delta.test.ts: the exec seam always passes a cwd, but the helper's signature marked it optional, so pushing it into a string[] was `string | undefined`. Missed locally because vitest runs through esbuild, which strips types without checking them - the suite was green while the build was red. The gate to run before pushing a type-level change is `npx tsc --build`, not the test suite. * fix(review): a base rerun that could not RUN attributes nothing Two Criticals from the latest review, both reachable on the brief's own happy path: - baseUnusable covered only timeouts. Every other way the base side can fail to run - an unbuilt base tree, a missing install, a workspace the PR ADDED (npm test --workspace cannot resolve on base), an ENOBUFS truncation - exits non-zero with zero FAIL lines, which this code read as a green base. Every PR-side failure then became netNew: the strongest evidence the command emits, manufactured from a base that never ran a test. It now attributes nothing and says why. - Timeout detection was the weaker substring form the sibling explicitly rejects; an external SIGTERM (container stop, cancelled job) set neither an ETIMEDOUT message nor an exit code and fed straight into the above. build-test now exports spawnTimedOut and test-delta asks the same question rather than re-deriving it. Also: the base output is trimmed (it precedes the verdict fields in the report the agent reads, so an untrimmed megabyte truncates exactly what the command produces); the guard-true gate no longer reads an arrow function's => as a comparison (every predicate guard was a candidate - the if (ready) noise the gate exists to exclude); the term-drop message no longer calls a string concatenation a reserve term; the unparsed note describes its own PR-side-only condition; and the ledger's Step 6/Step 8 now agree that a still-standing finding keeps its id. * fix(review): extract-step resolves all three env/defaults levels, and comments every env line Two silent-wrongness defects in a command whose whole value is fidelity. `env:`, `shell:` and `working-directory:` are three-level settings on GitHub — workflow, job, step, nearest wins — and only the step level appears in the step's own text. Reading step-level alone reproduced by machine the exact transcription error this command exists to remove: measured, a step under a job-level `NODE_ENV: production` and a workflow-level `GLOBAL_FLAG` extracted with `env: { LOCAL: '1' }` and `workingDirectory: undefined`, so the emitted script ran with both unset and nothing said so. Not a contrived shape: this repo carries workflow-level `env:` in 7 workflows, job-level `env:` in 10, and job-level `defaults.run` in qwen-triage.yml — the workflow the command's own test plan names. The three levels now merge with the runner's precedence, and `envSources` records which level each key came from, so an inherited value is visible rather than indistinguishable from the step's own. The env block was commented per ENTRY, not per LINE. A YAML block scalar (qwen-autofix.yml's `SETTINGS_JSON: |-`) reaches the header as a multi-line string, so its continuation lines landed in command position — and under the `set -e` the header itself emits, the extracted step died in its own preamble before its `run:` body ran. Every line is commented now. Tests pin the effect, not the mechanism: `executableLines()` asserts nothing but the `run:` body ever reaches command position, plus a `bash -n` parse check. Verified to flip — all five new assertions fail against the pre-fix implementation (`{ LOCAL: '1' }`, `undefined` working directory, three executable lines instead of one, and a real `bash -n` syntax error). * fix(review): compare failing files by a normalised, project-keyed identity Critical: the two sides run in DIFFERENT roots (the PR worktree and the base tree), and netNew/shared compared the parsed paths verbatim - so an absolute-path runner turned every pre-existing failure into a fabricated Critical, with the authority of a measurement behind it. Paths are now normalised against each run's own root (and backslashes to /, so a Windows path compares with its POSIX-printed twin), which is why test-delta gained --pr-worktree. The identity also keeps the vitest project token: dropping it collapsed same-named test files across workspaces, so a PR-caused failure in one package could read as pre-existing because another package has a file by the same name - the worse failure direction. Also from the same review, all of them reachable on the brief's own path: - The base rerun now inherits build-test's stdio: ['ignore','pipe','pipe'] ("a build that asks a question is a build that hangs until the deadline") and its trimOutput, which matters because entries[].base precedes the verdict fields in the report the agent reads. - The brief gates on base-tree's `available`, not just its `path`: a tree that was created but did not build populates path too, and measuring against it turns an infrastructure failure into Criticals. - A programmatic caller omitting `timeout` no longer sends NaN into spawnSync. - MutantCandidate is a discriminated union, so an operator without its replacement line - which would delete a line while reporting "with its ?? fallback dropped" - is unrepresentable. - The comparison class no longer requires a trailing space (if (a<b) is the same guard, just unformatted) and matches a brace-less else if. - DeltaEntry.unparsed's doc now describes the PR-side-only condition it actually implements. * fix(review): restore the whole-command budget, keep generics out of guard-true Round-1 findings from a fresh review of this PR: - test-delta had no aggregate deadline: --timeout is PER command and defaults to 300s, so three failed commands is 900s against Agent 7's 600s ceiling - killed with NO report at all, discarding the base-tree install and build just paid for. TOTAL_BUDGET_MS mirrors the one test-efficacy reserves; commands it cannot fit are disclosed. - guard-true matched generic calls: `if (isRecord<string>(v))` produced a mutant, and a type-guard predicate is exactly the `if (ready)` shape whose survivors the gate calls noise. The trailing \s is required, not an accidental asymmetry with [!=]== - telling `a<b` from `fn<T>(x)` needs a parser, and the gate is silence-biased by design. - --pr-worktree had no contract test, and its failure mode is the worst here: arriving undefined, root stripping silently stops and every pre-existing failure becomes a fabricated netNew. The new test feeds parseSync's output straight into runTestDelta and asserts an attribution only reachable when both roots were stripped (verified red against the snake_case field shape that shipped once already). - Merged the two consecutive doc comments on prWorktree. * fix(review): a budget-shortened deadline is not the same fact as a slow rerun Round-2 finding on the budget just restored: `Math.min(perCommandMs, remaining)` can hand a rerun far less than --timeout, and if it dies there the note said only "timed out - infrastructure, not evidence". True, but it sends the reader hunting a hang that is really an exhausted budget - and unlike a real timeout, a rerun with budget to spare would still measure it. The note now names those commands separately and says so. Verified red against removing the tracking line. * fix(review): brace-tolerant stub list, pipefail fidelity, and extract-step in the briefs Round-3 findings on this PR, fixed. `expressionsOf` matched `[^}]*`, so any expression containing a brace — `format('refs/pull/{0}/head', …)`, `fromJSON('{"a":1}')` — was not mis-listed but DROPPED. A stub list reads as "these are all the values to supply", so a silent omission is a value that never gets stubbed. It now scans forward to the closing `}}`, and reports nothing for an unterminated site rather than swallowing the rest of the text. Declaring `shell: bash` is not the runner's default `bash`. The default is `bash -e {0}`; a declared `bash` (at any level) is `bash --noprofile --norc -eo pipefail {0}`, and a pipeline whose middle stage fails aborts under one and not the other. The header now carries `set -eo pipefail` or `set -e` accordingly — 163 of this repo's 434 `run:` steps are under a declared bash and were getting the weaker one. A `shell:` value is also a command template (`perl {0}`), so only its first word goes in the shebang and the whole template is recorded beside it. `extract-step` was registered on the CLI and mentioned in DESIGN.md, and nowhere in SKILL.md or the agent briefs — the runtime prompts. The capability was unreachable by the agents it was written for. The verifier's brief now carries it next to the A/B paragraph it composes with, and Step 4 summarises it. Also: env ordered nearest-first (measured on qwen-autofix.yml:route:0, merge order put 20 inherited entries ahead of the step's own 26 in a 49-line header); a valueless `FOO:` renders as the empty string, not `"null"`, and a non-scalar as JSON rather than `[object Object]`; a missing file no longer reports as a parse failure; DESIGN.md's lens count matches its list. The test oracle is rebuilt around the property instead of a filter: the file is the header plus the body verbatim, and every line before the body is a comment or a directive the test names. The old helper dropped `set -e` unconditionally, so it could not tell the header's from one the body legitimately contains — and would have gone green on a header that leaked exactly that line. 434 real `run:` steps swept: 0 non-verbatim bodies, 0 live header lines, 0 missed expression sites, 0 out-of-order env, 0 `bash -n` failures. * fix(review): restore the replacement sub-cap, stranded on a downstream branch Round-3 finding, and the third instance of one class: an 8218-layer fix committed on the 8261 branch, four PRs above the code it belongs to. Measured over 40 real commits, the replacement operators produce ~24x the deletion pool (215 vs 9 candidates; guard-true drives it). Every mutant run drains the same window hunk probes draw from LAST, so uncapped, most diffs with any replacement candidates leave hunk probing zero runs - the hunk-survived finding class silently stops firing and nothing says so. Three slots, and what the sub-cap drops is counted in skippedForCap rather than lost. Also swept the other direction: diffed every review file against its 8261 copy to confirm nothing else 8218-layer is stranded up there. The remaining divergence is 8261's own (the positive control, its lenses). * fix(review): one severity predicate, carried ids, and a lossless marker Round-1 review of the ledger marker, six findings. Two contradicted contracts this package already states elsewhere: - `buildLedger` restated the severity predicate as a bare `startsWith` while `severityOf` — what `countInlineFindings` and the unmarked-comment gate both use — trims first. A Critical whose body opened with a newline was counted, was posted, blocked the merge, and was silently absent from the ledger, shifting the id of every finding after it. It classifies through `severityOf` now, and strips the marker with the shared prefixes instead of a second copy of the literals. - Ids were assigned by array position every round, so the report saying `R1-2 still stands` shipped beside a marker recording that claim as `R3-4` — and `LedgerFinding.id` documented itself "stable across re-reports". Step 6 now mandates writing the original id into the re-report (`**[Critical]** R1-2: …`), `buildLedger` reads it back, and a duplicate or absent id falls back to the next free id of this round. Four smaller ones: - `stripLedgerMarker` removed only the FIRST marker while `parseLedger` reads the LAST, so a body holding two leaked the trusted marker into model-facing prose and un-filtered the canonical LGTM it was appended to. - The `--` guard rewrote data to an em dash: a finding about `--comment` reached the next round as `—comment`. It escapes at the JSON layer now, which is comment-safe and lossless. - `file` was capped on read (200) but not on write; the cap is one constant binding both halves. - `submit`'s path — the one GitHub's payload is built from — had no test that the marker reaches the posted body. Plus: tie-break `latestOwnLedger` on review id when `submitted_at` ties, neutralise a backtick in the location code span, skip the `currentUser()` round-trip when the PR has no reviews, and say why a stale side file is deliberately left in place. * docs(review): complete the "delta cannot rule" enumeration in both places Round-4 finding. The brief and SKILL.md each listed three cases where test-delta attributes nothing - unparsed, timed-out base, no merge base - but the code has five: the later rounds of this PR added "a base rerun that failed without naming any failing file" (it did not measure the base) and "a command the whole-command budget could not fit". Two enumerations of the same set with different membership, in the two places an agent reads. That is the sibling-enumeration lesson this skill teaches, applied to its own prose for the second time: the fix is not just adding the missing members but saying that the report names each case with its own reason rather than folding them into one. * fix(review): the invokes list was mostly prose, not commands Round-4 finding on this PR. `invokes` is documented as a heuristic starting point, and imprecision is fine — but measured over this repo's 434 real `run:` steps it was reporting 435 distinct "commands", 267 of them appearing exactly once, with a worst case of 63 entries made up of words like `CI`, `Evidence`, `PR` and `and`. A list that size, mostly prose, is not a starting point. Three causes, each measured: - A `${{ … }}` expression is not shell, and it routinely contains `||`. Splitting on that as a pipeline separator reported both operands as commands (`matrix.arch`, `github.event.inputs.version`). Expressions are now masked to an opaque token before the split; one sitting in command position contributes nothing, which is honest — what it expands to is unknown here by design. - A heredoc body is input to a command, not a list of them. Its lines were scanned as commands, terminator included. 12 steps in this repo carry one. - The `name=value` skip stepped over the prefix and took the NEXT word as the command — but for a quoted value with spaces that word is inside the value: `EVIDENCE_SECTION=$'### Evidence images'` reported `Evidence`. Quoted spans are now blanked out, with the quote carried across lines so a multi-line string's continuation lines are data too. Command substitutions are read first, so `body="$(sanitize < "$REPORT")"` still reports `sanitize`. A `#` preceded by whitespace ends the live part of a line, so an apostrophe in a trailing comment cannot open a span and eat the rest of the script. Measured after: 435 distinct commands to 187, singletons 267 to 104, worst case 63 to 27 — and the worst case is now real commands (`awk cat chmod curl git jq mktemp pkill tar tee timeout`) plus the script's own shell functions. Also added the builtins a stub could not intercept anyway (`eval`, `exec`, `source`, `unset`, `command`, …) to the keyword set. 434 steps re-swept: 0 non-verbatim bodies, 0 live header lines, 0 missed expression sites, 0 out-of-order env, 0 `bash -n` failures. * docs(review): the rationale named only one of the ways base goes unmeasured Third and last copy of the enumeration the previous commit fixed. The bullet's headline already generalised - "base attributes nothing it did not finish" - but its body named only the timeout, so a reader learning the contract from the rationale would conclude timeouts are the only unusable case. Name the set, and say why the report keeps the reasons apart: "we could not measure" and "we measured nothing" are different facts to the author, and only one of them is about their PR. * fix(review): a quoted `<<EOF` is not a heredoc, and a continued line is one command Round-5 findings, both in the previous commit's own scanner. A heredoc opener was matched over the whole line, so one inside a string started heredoc mode: `echo "write <<EOF for a heredoc"` made every later line wait for a terminator that never arrives. The failure is not a missing entry but a missing REST — measured, a three-line script returned `[]` instead of `[curl, jq]`, empty and entirely plausible. Opener detection moved inside the quote walk, where it only fires outside quotes; the quoted forms (`<<'EOF'`) are consumed by the match, so their quotes never open a span either. A backslash-continued command was scanned as several lines, which puts the next ARGUMENT in command position — this is where `apt-get install -y \` / ` libx11-dev` reported the package as an invoked command. Continuations are now joined into one logical line before scanning. Measured after: 187 distinct commands to 185, and the singleton tail is now dominated by real ones — PowerShell cmdlets, macOS tooling, and the scripts' own shell functions. 434 real steps re-swept: 0 non-verbatim bodies, 0 live header lines, 0 missed expression sites, 0 out-of-order env, 0 `bash -n` failures. * fix(review): case labels, a second heredoc, and an expression in command position Round-6 findings, from running the scanner against adversarial shapes rather than reading it again. Two are UNDER-reports, which is the worse direction: a command missing from the list is a stub the verifier never writes, so the extraction reaches the real network. - A `case` pattern label stopped the scan on its own line: `blocked) gh api x` reported nothing, losing `gh`. The label is now stepped over like a `name=` prefix. - Only the first heredoc opener on a line was tracked, so `cat <<A <<B` left the second body and its terminator read as commands (`B`, `y`). Openers are queued and consumed in order. - Masking an expression to a QUOTED token let the quote-stripper delete it entirely, so `${{ steps.x.outputs.cmd }} arg` reported `arg` as the command. The token now survives stripping and cannot match a command word, so an expression in command position contributes nothing. Ten further adversarial shapes were already correct and are pinned as regression guards: nested `$( )` in quotes, subshells, function definitions, indented heredoc terminators, backticks, bare redirects, adjacent and empty `${{ }}` sites, and a JSON literal inside an expression. Verified to flip — exactly the three above fail against the previous commit, the other ten pass. 434 real steps re-swept: 0 non-verbatim bodies, 0 live header lines, 0 missed expression sites, 0 out-of-order env, 0 `bash -n` failures. * fix(review): annotate the continuation accumulator so tsc can type it `invokedCommandsOf`'s backslash-continuation loop failed to compile with TS7022: the narrowed type of `pending` at the join line is the union of the loop-entry value and the back edge, and the back edge is computed from the join itself. The declaration's own annotation does not break that cycle - control-flow narrowing runs after it - so the checker gives up and calls the result `any`. Caught only on a forced rebuild. `tsc --build` had been reporting this tree clean off a stale .tsbuildinfo, which is the same shape of gap that put a type error into CI last week: the test runner strips types, so the suite stayed green either way. Gate with --force. * fix(review): a file-count label stops counting at the end of its line `Test Files 45 passed` filing its 45 as a differing TEST count was fixed once, with a lookbehind on the bare-count pattern. That only ever rejected the all-green shape. The moment any file fails, the runner prints `Test Files 1 failed | 44 passed (45)`, the label is no longer adjacent to the number, and 44 comes through as a test count - so the note reads "claimed 44, observed 1323" on exactly the runs whose summary someone would paste. Adjacency was the wrong invariant; the line is. Masking from the label to end-of-line is distance-independent and picks up jest's `Test Suites: 1 failed, 44 passed, 45 total` at no cost. The label keeps its `Test` word on purpose: a first cut matched a bare `files` and blanked the line in "expect all four files and 471 tests to pass", silencing a real claim. An existing test caught that, which is the argument for the rule being as narrow as it is - anything that suppresses claims is worth exactly its narrowness. Also drops the now-dead lookbehind, which a reviewer had separately (and wrongly) called a JS syntax error; variable-length lookbehind is legal in V8, and the module parsed fine. It goes because the line rule subsumes it, not because it was broken. * fix(review): parse the base rerun before its output is trimmed `trimOutput` rescues module errors and runner summaries out of the omitted middle, not the per-file FAIL lines this command reads. A base suite whose failure section overruns the tail budget therefore lost failing files into the gap - and a SHORT base set is the dangerous direction, because netNew is the PR side minus the base side. Every file the trim hid came back as a Critical attributed to this PR by "measurement". Parse the raw text, report the bounded one. The PR side cannot be fixed here: it is read out of build-test's stored output, trimmed before this command existed. That loss runs the other way - it understates `shared`, never invents a netNew - so the entry carries `prTruncated` and the note says the list may be partial. A silence-biased gap is still a gap the author should hear about. Also names both selection caps in the mutant-skip diagnostic. The count accumulates replacement sub-cap drops, and with 2 deletions and 6 replacements the total is exactly MAX_MUTANTS: the main cap never fires, yet 3 are dropped, and the message sent the reader looking for a pool of 11 candidates that never existed. * test(review): pin the `sh` set-line to exactly what the runner uses A reviewer asked for `expect(script).not.toContain('set -e')` on a `shell: sh` step. The opposite is correct - GitHub runs that step as `sh -e {0}` - but the thread was right that nothing pinned it either way. Assert both halves: `set -e` is present, `pipefail` is not. Dropping the line makes an extracted `sh` step run past a failure the runner stops on; adding pipefail claims a bash feature `sh` does not have. * fix(review): rerun only the command shapes build-test emits This command reads a report off disk and then executes the strings in it with `shell: true`, in the base worktree. Nothing else in the pipeline re-executes a value it read back from a file, so nothing else has to care where that value came from - this does, and the provenance is worse than it looks: the command is `npm test --workspace="<dir>"`, the workspace token is a directory, and a directory is a name a pull request chooses. `packages/x";curl …|sh;"` is a legal path in git and on Linux and it round-trips through the report into a shell. Restricting to the emitter's own grammar costs nothing real, since that grammar is the two shapes build-test produces. A command outside it is skipped and disclosed, the same treatment everything else this command cannot do already gets, so a future shape degrades to "judge it by the diff" rather than to arbitrary execution. * fix(review): the working directory is a setting the extraction was losing Two findings, both of them this file failing its own stated argument. The stub list dropped it. `expressions` covered the script and the env and nothing else, so a `working-directory: ${{ github.workspace }}/x` produced an empty list and a summary line reading "0 ${{ }} site(s) to stub" - and `expressionsOf`'s own comment says why that is the failure this list cannot afford: the caller reads it as "these are all the values to supply". Widened to every setting the command carries, the `shell:` template included. The emitted script never mentioned it. The env block is commented into the header precisely so a reader of the script alone can see it; the working directory changes what the script does just as much and was in the metadata only. The argument for reading all three levels, written in this file, is that a step run "in the wrong directory, and nothing says so" is the transcription error the command exists to remove - which is exactly what the header did. It is a comment rather than a `cd` for the same reason env is comments, not exports: the value may hold `${{ … }}` and this command substitutes nothing. Both pinned, and both checked by deleting the fix: each mutation fails exactly one of the two new tests. * fix(review): $(( )) is arithmetic, and a heredoc's form decides where it ends Three fixes and one deliberate non-fix, all measured against this repo's own 434 `run:` steps rather than argued. `$(( ))` was read as a command substitution, so `N=$((N + 1))` reported `N` as a command to stub. It was the single largest source of junk in the list: 196 distinct "commands" across the corpus, 165 without it. A plain `<<WORD` heredoc ends only on a line that is exactly WORD; the loose match ended it on an indented `EOF` inside the body and then read the body as commands, which is how `rm` got reported for a script that never runs it. `<<-` stays looser than bash (any leading whitespace, not just tabs) because looser can only end a body early, and this file's priority is that an under-report is the worse direction. `[^()]*` matched only the innermost `$( )`, so `X=$(gh api $(u))` lost `gh` - a missed stub, and the extraction reaches the network. Depth counted now, and the assignment-prefix skip no longer steps over an unclosed `X=$(gh` into reading `api` as the command. The non-fix is recorded where the next reader will hit it: the quote walk is flat while shell quoting nests, and over ~300 lines the drift reports fragments of jq source as commands. Inserting a separator where a blanked span was removes nine of those, but it splits `a"X"b`, which is one word to the shell, and the minimal reproducer for the difference is 296 lines - nothing short enough to pin it. A scanner nobody can pin costs more than the junk it removes. * fix(review): refuse an ambiguous step name, and report errors like the siblings Two findings, both about a message the caller never gets. A job may legally hold two steps with the same name. The selector took the first and said nothing, which is the failure this file's own header names - "picks the same-named step from the wrong job" - and it is worst in the use the command exists for: A/B extraction runs it once per tree, so a PR that adds or reorders a duplicate leaves the two sides comparing different steps while reporting on one. Refused out loud now, naming the indices; the index is always available and never ambiguous. The handler also let every throw propagate, so five carefully separated messages - cannot read vs cannot parse vs no job vs no step vs no `run:` - all arrived as "An unexpected critical error occurred" under a stack trace. `base-tree` and `test-plan` in the same directory already catch, write the message, and set exit 1. Matched. The separation between "the path is wrong" and "the YAML is wrong" only pays if the caller sees it. * docs(review): the brief's list of limits was one short "Two limits worth knowing before you spend the step" became three when the selector started refusing an ambiguous step name, and the count went stale in the same commit that made it wrong. This is the enumeration drift the skill teaches, in the text that teaches it. The added entry says what to do rather than only what happens: pass the index, which is what an A/B wants regardless — the two trees have to select the same step, and a name that moved between them is exactly how they stop doing that. * chore(review): drop a scratch probe file that reached the branch `packages/cli/inert.mjs` was a throwaway harness for sweeping the repo's own workflows; its cleanup ran with a relative path from the wrong directory and it got committed by the next `git add -A`. It failed CI and not the local hook because the two lint different sets: lint-staged filters to `*.{js,jsx,ts,tsx}`, which does not include `.mjs`, while CI's flat config picks the file up regardless of `--ext`. Verified with CI's own command this time, not the hook's. * fix(review): bound the whole ledger marker, not just each of its fields The per-field caps leave the total unbounded: fifty findings at full width serialize to just under 17,000 characters. The module's own paragraph calls the marker "a footnote, never a payload" and reasons about GitHub's 65,536-character body limit, so the total is the number that claim is about and nothing was computing it. Set the budget from measurement rather than from the limit. Across every review this pipeline has posted on its own stack (n=66) the body runs a median of 721 characters, p90 2,178, max 3,925 - the limit has ~61 KiB of headroom, and an over-long marker was never going to fail a post. The real cost is proportion: 17,000 characters of invisible payload on a 3,925-character review is four times more marker than review. 8 KiB holds fifty findings at realistic widths without dropping any, and the truncation path is reached only by a ledger no round has produced. What it drops travels with it. A `dropped` count is the difference between a list the next round reads as complete and one that says it is not - the same distinction this module already draws everywhere else. * fix(review): a partial ledger must not render as a complete one The size cap added in the previous commit can drop entries, and the rendered section still opened with "**Every entry below is owed a this-round ruling**" and said nothing about what was missing. The `dropped` count existed and reached the only place a reader sees the list as nothing at all — which is the failure it was added to prevent, committed one step after adding the field. It now says the list is partial, how many are missing, and that absence is not evidence a finding was fixed. Second, the table escaping escaped `|` but not `\`. `\|` in a title became `\\|`, which markdown reads as an escaped backslash followed by a LIVE separator: the forged row the escaping exists to prevent, produced by the escaping. Backslash first now, and the test counts separators the way markdown does rather than by pattern, because a lookbehind counter is fooled by exactly the input under test. * fix(review): an empty ledger title is a cap the next round cannot lift A comment that is nothing but its severity marker produced a ledger entry with an empty title. That does not merely degrade the entry, it jams the review: the next round is told every ledger entry is owed a this-round ruling, has no claim text to rule on, answers `cannot tell`, and `cannot-tell-existing-critical` is one of the cap reasons. Nothing between rounds changes, so the cap comes back every round. Dropping the entry would hide a Critical that really was posted, so it keeps its place and gets the one handle there is - the location it was filed at, or "the review body" for a body-level one. The field's stated job is "enough for the next round to re-locate the claim", and that is what a text-less finding still has. Found by probing `buildLedger` with ten malformed comment shapes; the other nine - carried ids, id collisions, two comments carrying the same id, `R2D2` not being an id, a leading newline before the marker - all behaved correctly and needed no change. * fix(review): count both caps as dropped, not just the byte one Found by running the three previous fixes together on one round: 51 findings in, 24 kept, and the marker said 26 were missing. `LEDGER_MAX_FINDINGS` truncates before the byte cap ever runs, and measuring `dropped` against the already-sliced list left the count cap's share silent - so the field added two commits ago to stop a truncated list reading as complete was itself under-reporting the truncation. Measured against what came IN now, and pinned across all four combinations: count cap alone, byte cap alone, both, and neither. Kept plus dropped equals given, in every one. --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Code <qwen-code@users.noreply.github.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>	2026-08-01 16:18:12 +00:00
qwen-code-dev-bot	4c32ef3e05	Merge branch 'main' into feat/summary-export-path	2026-08-01 23:32:50 +08:00
Shaojin Wen	dbb0349351	feat(review): borrow recall, a fix loop, and a size-derived budget from Claude /review (#8315) ... * feat(review): borrow recall, a fix loop, and size-derived budget from Claude /review Three changes, from a comparison of this skill against Claude Code's `/code-review`. The orchestration half of that comparison went the other way — nothing there has the worktree isolation, the transcript-backed coverage gate, or the single computed verdict — so what is borrowed is what it does better: how much it surfaces, what a finding *is*, and what a small diff costs. Recall ------ The Exclusion Criteria are a filter on what KIND of thing is a finding. Read as a confidence bar — which is how an agent under "silence is better than noise" reads them — they license dropping anything half-believed, and that drop is invisible: no later stage sees a candidate that was never filed. Every stage after the finders removes wrong findings; none can add a missing one. Each finder brief now carries the counterweight explicitly, and the Step 4 verifier deliberately does not get it. Code quality was one agent holding six unrelated checks — the shape this skill already refuses for invariant agents, on measured evidence (PR #6457: one agent with an eight-item checklist found 1 of 5 defects; the same model split three ways found all 5). Split into 3a reuse/duplication, 3b altitude/abstraction fit, 3c consistency/clarity. 12 -> 14 agents in 3A. Low was one undirected pass capped at 8, and its only alternative was a nine-subagent fan-out. It is now an angle rotation in one context — line-by-line, removed behaviour, language pitfalls, wrapper routing, reuse/dead code, sibling consistency, then a gap sweep — dedup-only, no re-judging, cap 10. Still zero subagents. --fix and findings as data -------------------------- `--fix` is `--comment` reflected and gated on the opposite target: `--comment` writes to a pull request, `--fix` writes to a working tree, so a PR review (whose tree is the ephemeral worktree Step 9 deletes) ignores it with a warning. An effective `--fix` floors the effort at medium — editing the user's files on an unverified finding is the same mistake as posting one. New `qwen review findings` canonicalizes the findings into a JSON artifact the terminal report, the saved report and the review JSON all read, instead of three transcriptions of one list. With `--outcomes` it merges the fixer's ledger and REFUSES one that does not account for every finding: a fixer that applies six of nine and reports six has not lied about any of them, it has silently shortened the list. Size-derived budget ------------------- New `plan.budget`, computed from srcDiffLines the way the topology gate is and recorded in the plan rather than passed as a flag, so every reader sees one number. Scopes the low tier's angle count and sweep, the Agent 8 ceiling (0 below 80 source lines — "one domain dominates" is a judgement, and one made about forty lines finds a dominant domain every time), and the verify shard width. It never scales a dimension away: that is the roster's answer and the roster reads effort. Not included: per-model prompt routing. Claude's table exists because it was measured per model family; shipping an invented mapping into this skill is the kind of change its own review would flag. Tests: 39 files, 1215 assertions. * docs(review): align counts and level descriptions left stale by the 3a/3b/3c split Round-1 self-review findings on this branch: - SKILL.md medium tier still named 'quality (Agent 3)'; the Step-1 low bullet and Step 3C heading hardcoded six angles though plan.budget scales them 3-6; and no fallback was stated for a plan written by an older CLI that carries no budget field (falls back to the flat pre-budget behaviour — more coverage, never less). - DESIGN.md still labelled the 12-agent roster '(current)', kept the ten-lens / crosses-twelve topology arithmetic beside the updated fourteen-agent copy, keyed the re-gating cost row to the 12-agent roster, and described low as one pass ≤8 and medium as unverified inline angles — both contradicting the SKILL.md this PR ships. The LLM-call-budget and Fork-Subagent sections were still summed for 12 agents. - findings.ts: validateFindings accepted outcome but dropped outcomeNote, so the canonical artifact did not round-trip — a skipped finding fed back through --input kept its outcome and lost its reason. Tests: 40 review files green, including two new round-trip cases. * docs(review): one id per finding across the cache ledger and the findings artifact The rebase onto #8218 left two id schemes for one finding: the incremental cache's cross-round ledger names findings R<round>-<n>, while the findings artifact accepted any unique id. Same defect, two names, and the outcome ledger and next round's report could no longer be joined. The artifact now uses the R-ids whenever the run writes the cache ledger. Conflict resolutions from the rebase itself: review.ts keeps both new subcommands (test-delta from main, findings from this branch); the documentation-parity check #8218 added to the old Agent 3 brief lands in 3c, the consistency slice that owns sibling-parity checks. * test(cli): add findings to the pinned review subcommand surface review.test.ts pins the exact subcommand list and sits one directory above the review/ glob the branch's local runs used, so the new findings subcommand never met it until CI. Ubuntu was the only matrix leg that ran. --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-08-01 15:26:58 +00:00
Shaojin Wen	1be199ae82	feat(review): render adjudication, workflow step extraction, three verification lenses (#8225) ... * feat(review): borrowed-verification trio — test-plan check, base-tree A/B, per-hunk probes - qwen review test-plan: rule on the PR Test Plan's checkable claims (paths, npm scripts, test counts) against the reviewed tree; contradictions and differing counts are disclosed via compose-review, never capping. - qwen review base-tree: build the merge base in a sibling worktree so the verifier can A/B a comparative claim instead of reading it; swept by cleanup. - test-efficacy: third probe kind — reverse-apply one hunk at a time and re-run the affected tests, attributing a still-green suite to the specific change nothing gates; shares the mutants' budget window, runs last. * fix(review): survive real runner output — ANSI-laced and trimmed-away summaries Both measured on a live /review of QwenLM/qwen-code#8176 with the built CLI: - test-plan's observedTestCounts strips SGR sequences before matching; a color-enabled pipe interleaves them BETWEEN tokens, and the count claim fell to 'unchecked' with the summary right there in the report. - build-test's trimOutput rescues runner summary lines from the omitted middle (like module-resolution errors): a failing suite's tail is all failure details and npm epilogue, which pushed the one-line summary out of the kept text entirely. * fix(review): address the eight findings from live review of this branch All measured in the review (QwenLM/qwen-code#8215 review comment): - test-plan: linear-time bold-heading scan (the old pattern backtracked catastrophically on an unclosed ** line an untrusted PR body controls); a flag preceding the npm script yields no claim instead of a false 'no package defines this script'. - test-efficacy: a hunk probe's restore recreates the parent directory a reverse-applied 'new file' hunk removed (the ENOENT from finally lost the verdict and marked every remaining hunk inconclusive); hunks get their own skippedForBaseline instead of mislabeling a red baseline as a budget skip; splitDiffIntoHunks re-captures the file header at every diff --git boundary; a hunk-survived finding notes when it restates an inert file-level revert at hunk granularity. - base-tree: idempotent fast path keyed on a build marker + HEAD check — concurrent verifier shards reuse one built tree instead of sweeping it out from under each other mid-A/B (a fabricated base-side difference with a deterministic source tag was the worst case); cost wording is now 'an install and a build' everywhere it was 'one extra build'. * fix(cli): never score a hunk survived when its own test left the baseline (#8215) A per-hunk probe reported `survived` whenever the green baseline probes still passed with the hunk reverted. When the hunk's own collocated test dropped out of the baseline (a probe-tree import error collects nothing), the remaining green probes prove only that THEY do not cover the hunk, so the verdict is now `inconclusive` — the same dropped-test asymmetry the mutants already hold. Also scope the hunk-survived cross-reference note to the hunk's own collocated test, and let test-plan match a workspace-scoped run of the plan's bare command instead of falling through to the manifest on an exact-string miss. * fix(review): silence-bias hardening from four live review rounds of this branch The two blocking findings, reproduced on this PR's own Test Plan: - test-plan files no false contradicted notes: npm rulings move from a four-verb denylist to an allowlist (the run form + npm's script aliases — the ~fifty other builtins each used to become 'no package defines this script'); a slash token is claimed as a repo path only with evidence (an extension or ./ prefix), never when it is a flag's value (--repo owner/repo) or under the review's own temp root; HEADING_LINE_RE drops the same quadratic shape its bold sibling was rewritten to remove. - base-tree gets a real mutual-exclusion lock around sweep+add+build (mkdirSync test-and-set; the loser returns busy instead of deleting the tree the winner is mid-install in), and a failed build writes a settled marker so later shards stop re-paying the install to relearn 'unavailable'. Also: Agent 7's brief now names hunk-survived and the hunks.* counters (it is the report's only consumer, and the finding class was invisible); hunk findings anchor at the first ADDED line instead of up to three context lines above the change. * feat(review): four round-2 borrowings — measured failure attribution, round ledger, richer mutants, doc parity Shaped by a live round-2 maintainer re-verification (QwenLM/qwen-code#7818): - qwen review test-delta: rerun the PR side's failed test commands on the built merge base and diff the failing FILE SETS — netNew is the PR's own failure by measurement (whatever files the diff touches), shared is pre-existing by measurement. Counts are never compared: a flaky suite fails different test names between runs of the same tree. An unfinished base rerun attributes nothing. - Round ledger: the incremental review cache persists confirmed findings under round-scoped ids (R1-2); a high-effort re-review rules on each (fixed / still stands / cannot tell) and opens its report with the table, the way a human round-2 comment opens with 'M1 is fixed'. - Three replacement mutation operators in test-efficacy: drop '?? fallback', force a comparison-bearing guard condition to 'true', drop a '+ CONST' term. Each survivor maps to one crisp untested-protection sentence. A line whose raw text and literal-blanked code view disagree yields no candidate — an edit index computed across the two views once spliced 'iftrue 0)' into a guard, and a mangled mutant reads as inconclusive while quietly spending a cap slot. - Quality brief: documentation-parity lens — a new user-facing surface whose siblings are documented is a Suggestion naming the sibling precedent; no documented sibling, no finding. * feat(review): render adjudication, workflow step extraction, three verification lenses Shaped by a live sanitizer-PR verification (QwenLM/qwen-code#8147): - Render-adjudication capability (opt-in): with QWEN_REVIEW_SCRATCH_REPO set, the verifier may post a minimal payload to that user-designated repo and rule on GitHub's own rendered HTML — the measured case being an @ -> &#64; defusal every local reading called sound while GitHub's real renderer registered the mention and fired the notification. Absent the setting, rendering claims honestly cap at low confidence / cannot tell. Step 7's write ban names the carve-out explicitly. - qwen review extract-step: lift one workflow step's run: script verbatim into an executable, with env (as comments, never half-substituted exports), every ${{ }} site listed unevaluated, and a heuristic invoked- command list as the stubbing starting point. With base-tree, both arms of a by-hand workflow A/B become two invocations. yaml declared as a cli dependency (previously resolved only via hoisting). - Three brief lenses: a borrowed protection idiom missing what made it work at home (the code ancestor did the protecting; only the entity was copied); a second parser for an authoritatively-parsed format is a divergence hunt; tests that pin the mechanism instead of the effect, and oracles that mirror the implementation's own model. * feat(review): sibling-entrance discipline for the fixed verdict From round 6 of the live sanitizer verification (QwenLM/qwen-code#8147): the fix closed the fence-shaped entrance into a raw-HTML block, and the code-span entrance beside it — same divergence, adjacent syntax — stayed open. A re-check that tests only the reported input rules 'fixed' over a hole one backtick away. Both fixed-verdict sites in SKILL Step 6 (the open-Criticals re-check and the round ledger) now require enumerating a divergence-class defect's sibling entrances before ruling fixed; a still-open sibling is a NEW finding, never a reason to withhold the original's fixed — the two rulings stay separate so the second hole cannot ship unreviewed. * feat(review): three measured-verification lenses from live rounds 8037/8005 - Threshold-boundary scan: when a fix is a ratio/length guard, hold the issue's own variables fixed and binary-search the boundary where the behaviour flips; put the number next to what the issue reports. Live case: a prose-ratio guard covered the edit/write_file half of its issue and silently declined the run_shell_command half (~473-char boundary). - Delimiter self-injection named as the first parser-differential probe: a no-escaping extractor fed its own close tag truncates silently. - Shared-gate state enumeration: a deliberate-design defence extends only to the states it argues — an input-hold argued for 'active' silently froze three idle states sharing the same gate. The sibling-entrance rule, applied to a state machine instead of a syntax. * fix(review): address review feedback — false-positive hardening, binary diff guard, error convention (#8215) * fix(review): address review feedback — base-tree availability gate, test-plan false positives, hunk-probe ranges (#8215) - base-tree: only stamp a base tree available when runBuildTest actually compiled something (ok AND npm toolchain AND a non-empty build). An `unsupported` handoff or an empty npm scope returns ok:true having built nothing; marking that tree available let an A/B read the absence of a build as a behavioural difference. - cleanup: sweep the stale base-tree build lock a killed builder leaves behind. - test-plan: read the root manifest's scripts directly so a root-only script survives when the root defines no build/test; bail on the inline --root=./dir rebasing form; stop treating a positional after an inline --flag=value as the flag's value; prefer a failed scoped run when ruling a bare command; anchor the npm script alias to a full token so `yarn test:unit` is not truncated. - test-efficacy: exclude `\ No newline at end of file` from the startLine offset count; compute the mutant-overlap range from the header's new-side span so it no longer overshoots into a closely following hunk. * fix(review): address review feedback — diff-header false positives, stale prompt enumeration, added-file hunk probes (#8215) * fix(review): address review feedback — cd-base exclusion, Test Files count guard, base-tree error handling, probe delegation (#8215) * fix(review): port the collocated-dropout test to the post-#8050 runner seam Merging main brought #8050's Windows-portability refactor, which resolves the probe runner through vitest/package.json's bin — a node_modules/.bin fake is dead weight it never reads. The 8215-only collocated-dropout test still installed the old .bin fake, so the REAL vitest ran its fixtures, price.test.ts genuinely passed, and the hunk scored survived. The test now overrides the fake package's vitest.mjs like every post-refactor test. * fix(review): bound the summary rescue, apply the ATX heading rule, sweep stale build locks The three 8215-layer findings from the latest review, fixed at this layer (they were first patched further up the stack, where the reviewer of THIS PR cannot see them): - trimOutput's summary rescue is capped at 40 lines — uncapped, 40k lines of 'Test <n>: …' prose voided the trim entirely (measured 1.6MB in, 1.6MB out) and the bounded-output contract is the whole point. - A '#' with no following whitespace is prose, not a heading (the ATX rule GitHub applies): '#8176', '#tag', an unfenced '#!/bin/bash' no longer end the Test Plan section mid-body; the bare-#-run crash on the closing scan is guarded. - A base-tree build lock older than 30 minutes is a corpse left by a killed builder — swept and rebuilt instead of reporting busy for the rest of the review. * fix(review): EEXIST-only lock busy, bun test alias, chained cd bail, fence backreference Four live findings from the latest inline review round (the rest of the round was already fixed upstream by the takeover bot - verified by probing head behavior rather than re-reading the threads): - base-tree's lock catch distinguishes EEXIST (a concurrent builder, busy) from EPERM/EROFS/ENOSPC (this run's own failure, reported as such, not as a busy that will never clear). - "bun test" is bun's built-in runner, not a package-script alias: it runs whether or not any manifest defines test, so ruling it against the scripts table filed a false contradicted. - A chained cd matches the leading-cd shape but the single-hop resolver joined file tokens against the FIRST directory; it now bails like the exotic-cd case. - codeSpans' fence regex closes on its own marker via backreference; a tilde fence line inside a backtick block ended the span early and lines after it were lost to extraction. * fix(review): close the ten open findings on this PR - guard-true tested for a comparison anywhere after `if (`, including the then-body, so `if (ready) emit(a !== b);` admitted a mutant on the comparison-less condition the gate exists to exclude. It now tests the condition span only. - The `survived` detail said "when it changes" for legacy DELETION mutants too; it now matches the operator. - test-delta's `unparsed` required both sides to parse zero files, so a PR-side failure whose FAIL lines the trim scattered was silently dropped whenever the base rerun happened to parse. netNew/shared come from the PR side, so the PR side alone decides. - failingFilesOf now matches Windows path shapes (backslashes, C:) - a missed parse is an unattributed failure, not a loud error. - The replacement branch of runOneMutant (write-file -> run-probe -> classify) had no end-to-end test; one now drives a coalesce operator through the real handler and asserts the mutated line, the verdict, the operator-specific wording, and that the shared tree is untouched. - Two tests were vacuous with respect to what they promised: the baseline-dir test never asserted the cwd (its helper swallowed the argument - fixed at the helper), and the one-candidate-per-line test used an input that never triggered the replacement path, so the `continue` under test was not load-bearing. - Reattached the orphaned selectMutants JSDoc; reworded the SKILL line. * fix(review): the test helper's cwd parameter is required, not optional CI's `tsc --build` failed on test-delta.test.ts: the exec seam always passes a cwd, but the helper's signature marked it optional, so pushing it into a string[] was `string | undefined`. Missed locally because vitest runs through esbuild, which strips types without checking them - the suite was green while the build was red. The gate to run before pushing a type-level change is `npx tsc --build`, not the test suite. * fix(review): a base rerun that could not RUN attributes nothing Two Criticals from the latest review, both reachable on the brief's own happy path: - baseUnusable covered only timeouts. Every other way the base side can fail to run - an unbuilt base tree, a missing install, a workspace the PR ADDED (npm test --workspace cannot resolve on base), an ENOBUFS truncation - exits non-zero with zero FAIL lines, which this code read as a green base. Every PR-side failure then became netNew: the strongest evidence the command emits, manufactured from a base that never ran a test. It now attributes nothing and says why. - Timeout detection was the weaker substring form the sibling explicitly rejects; an external SIGTERM (container stop, cancelled job) set neither an ETIMEDOUT message nor an exit code and fed straight into the above. build-test now exports spawnTimedOut and test-delta asks the same question rather than re-deriving it. Also: the base output is trimmed (it precedes the verdict fields in the report the agent reads, so an untrimmed megabyte truncates exactly what the command produces); the guard-true gate no longer reads an arrow function's => as a comparison (every predicate guard was a candidate - the if (ready) noise the gate exists to exclude); the term-drop message no longer calls a string concatenation a reserve term; the unparsed note describes its own PR-side-only condition; and the ledger's Step 6/Step 8 now agree that a still-standing finding keeps its id. * fix(review): extract-step resolves all three env/defaults levels, and comments every env line Two silent-wrongness defects in a command whose whole value is fidelity. `env:`, `shell:` and `working-directory:` are three-level settings on GitHub — workflow, job, step, nearest wins — and only the step level appears in the step's own text. Reading step-level alone reproduced by machine the exact transcription error this command exists to remove: measured, a step under a job-level `NODE_ENV: production` and a workflow-level `GLOBAL_FLAG` extracted with `env: { LOCAL: '1' }` and `workingDirectory: undefined`, so the emitted script ran with both unset and nothing said so. Not a contrived shape: this repo carries workflow-level `env:` in 7 workflows, job-level `env:` in 10, and job-level `defaults.run` in qwen-triage.yml — the workflow the command's own test plan names. The three levels now merge with the runner's precedence, and `envSources` records which level each key came from, so an inherited value is visible rather than indistinguishable from the step's own. The env block was commented per ENTRY, not per LINE. A YAML block scalar (qwen-autofix.yml's `SETTINGS_JSON: |-`) reaches the header as a multi-line string, so its continuation lines landed in command position — and under the `set -e` the header itself emits, the extracted step died in its own preamble before its `run:` body ran. Every line is commented now. Tests pin the effect, not the mechanism: `executableLines()` asserts nothing but the `run:` body ever reaches command position, plus a `bash -n` parse check. Verified to flip — all five new assertions fail against the pre-fix implementation (`{ LOCAL: '1' }`, `undefined` working directory, three executable lines instead of one, and a real `bash -n` syntax error). * fix(review): compare failing files by a normalised, project-keyed identity Critical: the two sides run in DIFFERENT roots (the PR worktree and the base tree), and netNew/shared compared the parsed paths verbatim - so an absolute-path runner turned every pre-existing failure into a fabricated Critical, with the authority of a measurement behind it. Paths are now normalised against each run's own root (and backslashes to /, so a Windows path compares with its POSIX-printed twin), which is why test-delta gained --pr-worktree. The identity also keeps the vitest project token: dropping it collapsed same-named test files across workspaces, so a PR-caused failure in one package could read as pre-existing because another package has a file by the same name - the worse failure direction. Also from the same review, all of them reachable on the brief's own path: - The base rerun now inherits build-test's stdio: ['ignore','pipe','pipe'] ("a build that asks a question is a build that hangs until the deadline") and its trimOutput, which matters because entries[].base precedes the verdict fields in the report the agent reads. - The brief gates on base-tree's `available`, not just its `path`: a tree that was created but did not build populates path too, and measuring against it turns an infrastructure failure into Criticals. - A programmatic caller omitting `timeout` no longer sends NaN into spawnSync. - MutantCandidate is a discriminated union, so an operator without its replacement line - which would delete a line while reporting "with its ?? fallback dropped" - is unrepresentable. - The comparison class no longer requires a trailing space (if (a<b) is the same guard, just unformatted) and matches a brace-less else if. - DeltaEntry.unparsed's doc now describes the PR-side-only condition it actually implements. * fix(review): restore the whole-command budget, keep generics out of guard-true Round-1 findings from a fresh review of this PR: - test-delta had no aggregate deadline: --timeout is PER command and defaults to 300s, so three failed commands is 900s against Agent 7's 600s ceiling - killed with NO report at all, discarding the base-tree install and build just paid for. TOTAL_BUDGET_MS mirrors the one test-efficacy reserves; commands it cannot fit are disclosed. - guard-true matched generic calls: `if (isRecord<string>(v))` produced a mutant, and a type-guard predicate is exactly the `if (ready)` shape whose survivors the gate calls noise. The trailing \s is required, not an accidental asymmetry with [!=]== - telling `a<b` from `fn<T>(x)` needs a parser, and the gate is silence-biased by design. - --pr-worktree had no contract test, and its failure mode is the worst here: arriving undefined, root stripping silently stops and every pre-existing failure becomes a fabricated netNew. The new test feeds parseSync's output straight into runTestDelta and asserts an attribution only reachable when both roots were stripped (verified red against the snake_case field shape that shipped once already). - Merged the two consecutive doc comments on prWorktree. * fix(review): a budget-shortened deadline is not the same fact as a slow rerun Round-2 finding on the budget just restored: `Math.min(perCommandMs, remaining)` can hand a rerun far less than --timeout, and if it dies there the note said only "timed out - infrastructure, not evidence". True, but it sends the reader hunting a hang that is really an exhausted budget - and unlike a real timeout, a rerun with budget to spare would still measure it. The note now names those commands separately and says so. Verified red against removing the tracking line. * fix(review): brace-tolerant stub list, pipefail fidelity, and extract-step in the briefs Round-3 findings on this PR, fixed. `expressionsOf` matched `[^}]*`, so any expression containing a brace — `format('refs/pull/{0}/head', …)`, `fromJSON('{"a":1}')` — was not mis-listed but DROPPED. A stub list reads as "these are all the values to supply", so a silent omission is a value that never gets stubbed. It now scans forward to the closing `}}`, and reports nothing for an unterminated site rather than swallowing the rest of the text. Declaring `shell: bash` is not the runner's default `bash`. The default is `bash -e {0}`; a declared `bash` (at any level) is `bash --noprofile --norc -eo pipefail {0}`, and a pipeline whose middle stage fails aborts under one and not the other. The header now carries `set -eo pipefail` or `set -e` accordingly — 163 of this repo's 434 `run:` steps are under a declared bash and were getting the weaker one. A `shell:` value is also a command template (`perl {0}`), so only its first word goes in the shebang and the whole template is recorded beside it. `extract-step` was registered on the CLI and mentioned in DESIGN.md, and nowhere in SKILL.md or the agent briefs — the runtime prompts. The capability was unreachable by the agents it was written for. The verifier's brief now carries it next to the A/B paragraph it composes with, and Step 4 summarises it. Also: env ordered nearest-first (measured on qwen-autofix.yml:route:0, merge order put 20 inherited entries ahead of the step's own 26 in a 49-line header); a valueless `FOO:` renders as the empty string, not `"null"`, and a non-scalar as JSON rather than `[object Object]`; a missing file no longer reports as a parse failure; DESIGN.md's lens count matches its list. The test oracle is rebuilt around the property instead of a filter: the file is the header plus the body verbatim, and every line before the body is a comment or a directive the test names. The old helper dropped `set -e` unconditionally, so it could not tell the header's from one the body legitimately contains — and would have gone green on a header that leaked exactly that line. 434 real `run:` steps swept: 0 non-verbatim bodies, 0 live header lines, 0 missed expression sites, 0 out-of-order env, 0 `bash -n` failures. * fix(review): restore the replacement sub-cap, stranded on a downstream branch Round-3 finding, and the third instance of one class: an 8218-layer fix committed on the 8261 branch, four PRs above the code it belongs to. Measured over 40 real commits, the replacement operators produce ~24x the deletion pool (215 vs 9 candidates; guard-true drives it). Every mutant run drains the same window hunk probes draw from LAST, so uncapped, most diffs with any replacement candidates leave hunk probing zero runs - the hunk-survived finding class silently stops firing and nothing says so. Three slots, and what the sub-cap drops is counted in skippedForCap rather than lost. Also swept the other direction: diffed every review file against its 8261 copy to confirm nothing else 8218-layer is stranded up there. The remaining divergence is 8261's own (the positive control, its lenses). * docs(review): complete the "delta cannot rule" enumeration in both places Round-4 finding. The brief and SKILL.md each listed three cases where test-delta attributes nothing - unparsed, timed-out base, no merge base - but the code has five: the later rounds of this PR added "a base rerun that failed without naming any failing file" (it did not measure the base) and "a command the whole-command budget could not fit". Two enumerations of the same set with different membership, in the two places an agent reads. That is the sibling-enumeration lesson this skill teaches, applied to its own prose for the second time: the fix is not just adding the missing members but saying that the report names each case with its own reason rather than folding them into one. * fix(review): the invokes list was mostly prose, not commands Round-4 finding on this PR. `invokes` is documented as a heuristic starting point, and imprecision is fine — but measured over this repo's 434 real `run:` steps it was reporting 435 distinct "commands", 267 of them appearing exactly once, with a worst case of 63 entries made up of words like `CI`, `Evidence`, `PR` and `and`. A list that size, mostly prose, is not a starting point. Three causes, each measured: - A `${{ … }}` expression is not shell, and it routinely contains `||`. Splitting on that as a pipeline separator reported both operands as commands (`matrix.arch`, `github.event.inputs.version`). Expressions are now masked to an opaque token before the split; one sitting in command position contributes nothing, which is honest — what it expands to is unknown here by design. - A heredoc body is input to a command, not a list of them. Its lines were scanned as commands, terminator included. 12 steps in this repo carry one. - The `name=value` skip stepped over the prefix and took the NEXT word as the command — but for a quoted value with spaces that word is inside the value: `EVIDENCE_SECTION=$'### Evidence images'` reported `Evidence`. Quoted spans are now blanked out, with the quote carried across lines so a multi-line string's continuation lines are data too. Command substitutions are read first, so `body="$(sanitize < "$REPORT")"` still reports `sanitize`. A `#` preceded by whitespace ends the live part of a line, so an apostrophe in a trailing comment cannot open a span and eat the rest of the script. Measured after: 435 distinct commands to 187, singletons 267 to 104, worst case 63 to 27 — and the worst case is now real commands (`awk cat chmod curl git jq mktemp pkill tar tee timeout`) plus the script's own shell functions. Also added the builtins a stub could not intercept anyway (`eval`, `exec`, `source`, `unset`, `command`, …) to the keyword set. 434 steps re-swept: 0 non-verbatim bodies, 0 live header lines, 0 missed expression sites, 0 out-of-order env, 0 `bash -n` failures. * docs(review): the rationale named only one of the ways base goes unmeasured Third and last copy of the enumeration the previous commit fixed. The bullet's headline already generalised - "base attributes nothing it did not finish" - but its body named only the timeout, so a reader learning the contract from the rationale would conclude timeouts are the only unusable case. Name the set, and say why the report keeps the reasons apart: "we could not measure" and "we measured nothing" are different facts to the author, and only one of them is about their PR. * fix(review): a quoted `<<EOF` is not a heredoc, and a continued line is one command Round-5 findings, both in the previous commit's own scanner. A heredoc opener was matched over the whole line, so one inside a string started heredoc mode: `echo "write <<EOF for a heredoc"` made every later line wait for a terminator that never arrives. The failure is not a missing entry but a missing REST — measured, a three-line script returned `[]` instead of `[curl, jq]`, empty and entirely plausible. Opener detection moved inside the quote walk, where it only fires outside quotes; the quoted forms (`<<'EOF'`) are consumed by the match, so their quotes never open a span either. A backslash-continued command was scanned as several lines, which puts the next ARGUMENT in command position — this is where `apt-get install -y \` / ` libx11-dev` reported the package as an invoked command. Continuations are now joined into one logical line before scanning. Measured after: 187 distinct commands to 185, and the singleton tail is now dominated by real ones — PowerShell cmdlets, macOS tooling, and the scripts' own shell functions. 434 real steps re-swept: 0 non-verbatim bodies, 0 live header lines, 0 missed expression sites, 0 out-of-order env, 0 `bash -n` failures. * fix(review): case labels, a second heredoc, and an expression in command position Round-6 findings, from running the scanner against adversarial shapes rather than reading it again. Two are UNDER-reports, which is the worse direction: a command missing from the list is a stub the verifier never writes, so the extraction reaches the real network. - A `case` pattern label stopped the scan on its own line: `blocked) gh api x` reported nothing, losing `gh`. The label is now stepped over like a `name=` prefix. - Only the first heredoc opener on a line was tracked, so `cat <<A <<B` left the second body and its terminator read as commands (`B`, `y`). Openers are queued and consumed in order. - Masking an expression to a QUOTED token let the quote-stripper delete it entirely, so `${{ steps.x.outputs.cmd }} arg` reported `arg` as the command. The token now survives stripping and cannot match a command word, so an expression in command position contributes nothing. Ten further adversarial shapes were already correct and are pinned as regression guards: nested `$( )` in quotes, subshells, function definitions, indented heredoc terminators, backticks, bare redirects, adjacent and empty `${{ }}` sites, and a JSON literal inside an expression. Verified to flip — exactly the three above fail against the previous commit, the other ten pass. 434 real steps re-swept: 0 non-verbatim bodies, 0 live header lines, 0 missed expression sites, 0 out-of-order env, 0 `bash -n` failures. * fix(review): annotate the continuation accumulator so tsc can type it `invokedCommandsOf`'s backslash-continuation loop failed to compile with TS7022: the narrowed type of `pending` at the join line is the union of the loop-entry value and the back edge, and the back edge is computed from the join itself. The declaration's own annotation does not break that cycle - control-flow narrowing runs after it - so the checker gives up and calls the result `any`. Caught only on a forced rebuild. `tsc --build` had been reporting this tree clean off a stale .tsbuildinfo, which is the same shape of gap that put a type error into CI last week: the test runner strips types, so the suite stayed green either way. Gate with --force. * fix(review): a file-count label stops counting at the end of its line `Test Files 45 passed` filing its 45 as a differing TEST count was fixed once, with a lookbehind on the bare-count pattern. That only ever rejected the all-green shape. The moment any file fails, the runner prints `Test Files 1 failed | 44 passed (45)`, the label is no longer adjacent to the number, and 44 comes through as a test count - so the note reads "claimed 44, observed 1323" on exactly the runs whose summary someone would paste. Adjacency was the wrong invariant; the line is. Masking from the label to end-of-line is distance-independent and picks up jest's `Test Suites: 1 failed, 44 passed, 45 total` at no cost. The label keeps its `Test` word on purpose: a first cut matched a bare `files` and blanked the line in "expect all four files and 471 tests to pass", silencing a real claim. An existing test caught that, which is the argument for the rule being as narrow as it is - anything that suppresses claims is worth exactly its narrowness. Also drops the now-dead lookbehind, which a reviewer had separately (and wrongly) called a JS syntax error; variable-length lookbehind is legal in V8, and the module parsed fine. It goes because the line rule subsumes it, not because it was broken. * fix(review): parse the base rerun before its output is trimmed `trimOutput` rescues module errors and runner summaries out of the omitted middle, not the per-file FAIL lines this command reads. A base suite whose failure section overruns the tail budget therefore lost failing files into the gap - and a SHORT base set is the dangerous direction, because netNew is the PR side minus the base side. Every file the trim hid came back as a Critical attributed to this PR by "measurement". Parse the raw text, report the bounded one. The PR side cannot be fixed here: it is read out of build-test's stored output, trimmed before this command existed. That loss runs the other way - it understates `shared`, never invents a netNew - so the entry carries `prTruncated` and the note says the list may be partial. A silence-biased gap is still a gap the author should hear about. Also names both selection caps in the mutant-skip diagnostic. The count accumulates replacement sub-cap drops, and with 2 deletions and 6 replacements the total is exactly MAX_MUTANTS: the main cap never fires, yet 3 are dropped, and the message sent the reader looking for a pool of 11 candidates that never existed. * test(review): pin the `sh` set-line to exactly what the runner uses A reviewer asked for `expect(script).not.toContain('set -e')` on a `shell: sh` step. The opposite is correct - GitHub runs that step as `sh -e {0}` - but the thread was right that nothing pinned it either way. Assert both halves: `set -e` is present, `pipefail` is not. Dropping the line makes an extracted `sh` step run past a failure the runner stops on; adding pipefail claims a bash feature `sh` does not have. * fix(review): rerun only the command shapes build-test emits This command reads a report off disk and then executes the strings in it with `shell: true`, in the base worktree. Nothing else in the pipeline re-executes a value it read back from a file, so nothing else has to care where that value came from - this does, and the provenance is worse than it looks: the command is `npm test --workspace="<dir>"`, the workspace token is a directory, and a directory is a name a pull request chooses. `packages/x";curl …|sh;"` is a legal path in git and on Linux and it round-trips through the report into a shell. Restricting to the emitter's own grammar costs nothing real, since that grammar is the two shapes build-test produces. A command outside it is skipped and disclosed, the same treatment everything else this command cannot do already gets, so a future shape degrades to "judge it by the diff" rather than to arbitrary execution. * fix(review): the working directory is a setting the extraction was losing Two findings, both of them this file failing its own stated argument. The stub list dropped it. `expressions` covered the script and the env and nothing else, so a `working-directory: ${{ github.workspace }}/x` produced an empty list and a summary line reading "0 ${{ }} site(s) to stub" - and `expressionsOf`'s own comment says why that is the failure this list cannot afford: the caller reads it as "these are all the values to supply". Widened to every setting the command carries, the `shell:` template included. The emitted script never mentioned it. The env block is commented into the header precisely so a reader of the script alone can see it; the working directory changes what the script does just as much and was in the metadata only. The argument for reading all three levels, written in this file, is that a step run "in the wrong directory, and nothing says so" is the transcription error the command exists to remove - which is exactly what the header did. It is a comment rather than a `cd` for the same reason env is comments, not exports: the value may hold `${{ … }}` and this command substitutes nothing. Both pinned, and both checked by deleting the fix: each mutation fails exactly one of the two new tests. * fix(review): $(( )) is arithmetic, and a heredoc's form decides where it ends Three fixes and one deliberate non-fix, all measured against this repo's own 434 `run:` steps rather than argued. `$(( ))` was read as a command substitution, so `N=$((N + 1))` reported `N` as a command to stub. It was the single largest source of junk in the list: 196 distinct "commands" across the corpus, 165 without it. A plain `<<WORD` heredoc ends only on a line that is exactly WORD; the loose match ended it on an indented `EOF` inside the body and then read the body as commands, which is how `rm` got reported for a script that never runs it. `<<-` stays looser than bash (any leading whitespace, not just tabs) because looser can only end a body early, and this file's priority is that an under-report is the worse direction. `[^()]*` matched only the innermost `$( )`, so `X=$(gh api $(u))` lost `gh` - a missed stub, and the extraction reaches the network. Depth counted now, and the assignment-prefix skip no longer steps over an unclosed `X=$(gh` into reading `api` as the command. The non-fix is recorded where the next reader will hit it: the quote walk is flat while shell quoting nests, and over ~300 lines the drift reports fragments of jq source as commands. Inserting a separator where a blanked span was removes nine of those, but it splits `a"X"b`, which is one word to the shell, and the minimal reproducer for the difference is 296 lines - nothing short enough to pin it. A scanner nobody can pin costs more than the junk it removes. * fix(review): refuse an ambiguous step name, and report errors like the siblings Two findings, both about a message the caller never gets. A job may legally hold two steps with the same name. The selector took the first and said nothing, which is the failure this file's own header names - "picks the same-named step from the wrong job" - and it is worst in the use the command exists for: A/B extraction runs it once per tree, so a PR that adds or reorders a duplicate leaves the two sides comparing different steps while reporting on one. Refused out loud now, naming the indices; the index is always available and never ambiguous. The handler also let every throw propagate, so five carefully separated messages - cannot read vs cannot parse vs no job vs no step vs no `run:` - all arrived as "An unexpected critical error occurred" under a stack trace. `base-tree` and `test-plan` in the same directory already catch, write the message, and set exit 1. Matched. The separation between "the path is wrong" and "the YAML is wrong" only pays if the caller sees it. * docs(review): the brief's list of limits was one short "Two limits worth knowing before you spend the step" became three when the selector started refusing an ambiguous step name, and the count went stale in the same commit that made it wrong. This is the enumeration drift the skill teaches, in the text that teaches it. The added entry says what to do rather than only what happens: pass the index, which is what an A/B wants regardless — the two trees have to select the same step, and a name that moved between them is exactly how they stop doing that. * chore(review): drop a scratch probe file that reached the branch `packages/cli/inert.mjs` was a throwaway harness for sweeping the repo's own workflows; its cleanup ran with a relative path from the wrong directory and it got committed by the next `git add -A`. It failed CI and not the local hook because the two lint different sets: lint-staged filters to `*.{js,jsx,ts,tsx}`, which does not include `.mjs`, while CI's flat config picks the file up regardless of `--ext`. Verified with CI's own command this time, not the hook's. --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Code <qwen-code@users.noreply.github.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>	2026-08-01 15:13:10 +00:00
qwen-code-dev-bot	c4c268a4a5	Merge branch 'main' into feat/summary-export-path	2026-08-01 22:19:58 +08:00
Woong Bi	e903b28870	fix(cli): resolve symlinked entrypoint path in main module guard (#8309) ... Co-authored-by: Wchoi189 <wchoi189@users.noreply.github.com>	2026-08-01 14:17:00 +00:00
qwen-code-dev-bot	ac4799feca	fix(cli): add ui.mouseTracking setting to restore right-click and URL clicks (#8198) ... * fix(cli): add ui.mouseTracking setting to restore right-click and URL clicks VP mode (default since 0.21.1) enables SGR mouse tracking (?1002h) which captures ALL mouse events including right-click and left-click on OSC 8 hyperlinks. The terminal can no longer handle these natively — right-click context menus and clickable URL links stop working. Add a `ui.mouseTracking` boolean setting (default: true) that gates SGR mouse tracking in useMouseEvents. When set to false, mouse tracking is disabled entirely, restoring terminal-native right-click and OSC 8 hyperlink handling. The trade-off is that mouse wheel scrolling and scrollbar dragging in the VP viewport stop working; keyboard scrolling (Up/Down/PgUp/PgDn) still works. Users who need right-click or URL clicking can set: "ui": { "mouseTracking": false } Fixes the regression reported in #8003 (right-click broken) and the follow-up report (URL link clicks broken) after 0.21.1. * fix(cli): regenerate mouseTracking schema and clarify setting scope (#8198) * fix(cli): scope ui.mouseTracking to the TUI and document it (#8198) * fix(cli): handle hyperlink clicks in VP mode via onContentClick When SGR mouse tracking is active (VP mode default), the terminal cannot handle OSC 8 hyperlink clicks natively. Add an onContentClick callback to ScrollableList that fires on left-press in non-scrollbar areas. MainContent implements this callback by extracting hyperlink regions from the clicked item's text (parsing markdown links and bare URLs with visual column tracking) and opening the URL at the click position via the platform's open/xdg-open/start command. This allows URL clicks to work in VP mode without disabling mouse tracking, so mouse wheel scrolling and scrollbar dragging continue to work simultaneously. New file: packages/cli/src/ui/utils/hyperlink-click.ts - extractHyperlinkRegions(): parses markdown links and bare URLs from raw text, computing visual column positions - findUrlAtColumn(): maps a click column to a URL - openUrl(): opens a URL via the platform's default handler * fix(cli): remove broken hyperlink-click feature from VP mode (#8198) Remove the hyperlink-click feature (hyperlink-click.ts, handleContentClick, onContentClick) which had three critical defects identified in review: 1. Command injection on Windows: openUrl used child_process.exec with shell string interpolation, where single-quote escaping is ineffective against cmd.exe metacharacters (&, |, ^, ;). 2. Non-functional coordinate mapping: handleContentClick used raw 1-based SGR terminal screen coordinates directly as item indices into allVirtualItems, without the scroll-offset and frame-anchor transforms that every other mouse consumer applies. 3. Wrong visual-column accounting: extractHyperlinkRegions counted raw markdown characters (including **bold**, *italic*, ~~strikethrough~~ markers) that InlineMarkdownRenderer strips, causing column drift. The ui.mouseTracking setting (the PR's core purpose) is unaffected and remains fully functional. A properly designed click-to-open feature can be added in a follow-up with correct coordinate transforms, renderer-aware column parsing, and openBrowserSecurely for safe URL launching. * docs(cli): cross-reference ui.mouseTracking in useTerminalBuffer descriptions (#8198) * fix(cli): document mouseTracking tradeoffs and hide showScrollbar in serve mode (#8198) * docs(cli): clarify mouseTracking tradeoffs in shortcuts and settings (#8198) Address review feedback: note that mouse-wheel scrolling requires ui.mouseTracking in the keyboard-shortcuts table, the tmux trackpad section, and troubleshooting; add the missing ui.showScrollbar row to the settings reference; and pin the mouseTracking schema fields (requiresRestart: false) in settingsSchema.test.ts. * fix(cli): make mouseTracking restart-scoped and gate mouse affordances (#8198) ui.mouseTracking declared requiresRestart: false but nothing propagates a settings write to the refcounted SGR mouse mode: the SettingsContext value is a single LoadedSettings created outside React with no change emitter, so a subscriber that never re-renders (e.g. a memoized completed thought block) keeps ?1002h active and right-click stays broken even with the setting off. Mark it requiresRestart: true, matching ui.useTerminalBuffer, and drop the runtime-toggle test whose own comment admitted it did not prove the flip lands. Add a shared useMouseTrackingEnabled hook and route the useMouseEvents gate and the mouse-dependent affordances through it — the history-item "click to expand" hint, the selection-list hover controller, and prompt click-to-position — so they read one source and no longer advertise a dead interaction when mouseTracking is disabled. * test(cli): add component-level mouseTracking affordance gate tests (#8198) * fix(cli): address review feedback on mouseTracking PR (#8198) - Rewrite BaseSelectionList mouse test to mock RowMouseController and assert mount/unmount directly, pinning the component-level gate instead of the downstream useMouseEvents escape writes - Add troubleshooting entry for right-click/links/selection symptoms - Add useTerminalBuffer pairing advice to settings.md mouseTracking row --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>	2026-08-01 14:15:42 +00:00
Shaojin Wen	0ee995d3bd	Merge branch 'main' into feat/summary-export-path	2026-08-01 21:36:07 +08:00
Dragon	e9db70a072	feat(cli): add TUI image display tool (#8217) ... * feat(cli): add TUI image display tool * fix(cli): report terminal image rendering failures * fix(cli): disable Kitty placeholders in Warp * fix(cli): constrain terminal image previews * fix(core): preserve fork image tool cache prefix * fix(cli): address display_image review feedback (#8217) Add the missing DisplayImage zh/zh-TW locale entries (and en for parity) that broke the i18n guard test, detect chafa via a PATH lookup instead of a synchronous probe render during display_image execution, and reject truncated PNGs by reading the full 24-byte header before reporting success. * fix(web-shell): add display_image to tool display contract (#8217) * test(cli): cover display image validation edges * fix(cli): harden chafa executable lookup (#8217) Resolve chafa through the mermaid renderer's hardened findExecutable and spawn the resolved path, so a project-local node_modules/.bin/chafa is never executed unless the user opted in. display_image is a non-prompting Read tool, so the previous bare-name PATH resolution allowed arbitrary code execution from a malicious repository. Also add review-requested coverage: isTerminalImageDisplay routing, the truncated-PNG rejection branch, the chafa stderr fallback, and the Kitty re-emit dedup guard. * fix(cli): address TUI image display review feedback (#8217) - Cache rendered terminal images (bounded LRU keyed on path, mtime, size, shape, and renderer) so a terminal resize or a restored session no longer re-reads the file or re-spawns chafa for every visible image. - Reject cmd.exe metacharacters in the model-supplied path before spawning chafa through a shell, closing a command-injection surface on Windows .cmd/.bat shims. - Bound a failing chafa's stderr to a capped first line before rendering it into permanent scrollback. - Enforce the main-agent-only display_image ban inside the tool execution (isInForkExecution) and fail resolveForkExecutionAllowedTools closed when display_image is advertised without a concrete allowlist. - Move the fileName computation to its use site, drop misleading awaits on the synchronous renderer, and add Kitty/Ghostty detection plus renderer cache, truncation, and shell-metacharacter tests. * fix(cli): preserve color in chafa image fallback * feat(cli): render images directly in Warp * fix(cli): disable native images in Warp * test(cli): cover chafa shell path guard * fix(cli): skip re-transmitting Kitty image payload on remount (#8217) --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> 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 Autofix <qwen-code-autofix@users.noreply.github.com>	2026-08-01 12:39:52 +00:00
Shaojin Wen	9bfe608be9	feat(review): measured failure attribution, round ledger, richer mutants, doc parity (#8218) ... * feat(review): borrowed-verification trio — test-plan check, base-tree A/B, per-hunk probes - qwen review test-plan: rule on the PR Test Plan's checkable claims (paths, npm scripts, test counts) against the reviewed tree; contradictions and differing counts are disclosed via compose-review, never capping. - qwen review base-tree: build the merge base in a sibling worktree so the verifier can A/B a comparative claim instead of reading it; swept by cleanup. - test-efficacy: third probe kind — reverse-apply one hunk at a time and re-run the affected tests, attributing a still-green suite to the specific change nothing gates; shares the mutants' budget window, runs last. * fix(review): survive real runner output — ANSI-laced and trimmed-away summaries Both measured on a live /review of QwenLM/qwen-code#8176 with the built CLI: - test-plan's observedTestCounts strips SGR sequences before matching; a color-enabled pipe interleaves them BETWEEN tokens, and the count claim fell to 'unchecked' with the summary right there in the report. - build-test's trimOutput rescues runner summary lines from the omitted middle (like module-resolution errors): a failing suite's tail is all failure details and npm epilogue, which pushed the one-line summary out of the kept text entirely. * fix(review): address the eight findings from live review of this branch All measured in the review (QwenLM/qwen-code#8215 review comment): - test-plan: linear-time bold-heading scan (the old pattern backtracked catastrophically on an unclosed ** line an untrusted PR body controls); a flag preceding the npm script yields no claim instead of a false 'no package defines this script'. - test-efficacy: a hunk probe's restore recreates the parent directory a reverse-applied 'new file' hunk removed (the ENOENT from finally lost the verdict and marked every remaining hunk inconclusive); hunks get their own skippedForBaseline instead of mislabeling a red baseline as a budget skip; splitDiffIntoHunks re-captures the file header at every diff --git boundary; a hunk-survived finding notes when it restates an inert file-level revert at hunk granularity. - base-tree: idempotent fast path keyed on a build marker + HEAD check — concurrent verifier shards reuse one built tree instead of sweeping it out from under each other mid-A/B (a fabricated base-side difference with a deterministic source tag was the worst case); cost wording is now 'an install and a build' everywhere it was 'one extra build'. * fix(cli): never score a hunk survived when its own test left the baseline (#8215) A per-hunk probe reported `survived` whenever the green baseline probes still passed with the hunk reverted. When the hunk's own collocated test dropped out of the baseline (a probe-tree import error collects nothing), the remaining green probes prove only that THEY do not cover the hunk, so the verdict is now `inconclusive` — the same dropped-test asymmetry the mutants already hold. Also scope the hunk-survived cross-reference note to the hunk's own collocated test, and let test-plan match a workspace-scoped run of the plan's bare command instead of falling through to the manifest on an exact-string miss. * fix(review): silence-bias hardening from four live review rounds of this branch The two blocking findings, reproduced on this PR's own Test Plan: - test-plan files no false contradicted notes: npm rulings move from a four-verb denylist to an allowlist (the run form + npm's script aliases — the ~fifty other builtins each used to become 'no package defines this script'); a slash token is claimed as a repo path only with evidence (an extension or ./ prefix), never when it is a flag's value (--repo owner/repo) or under the review's own temp root; HEADING_LINE_RE drops the same quadratic shape its bold sibling was rewritten to remove. - base-tree gets a real mutual-exclusion lock around sweep+add+build (mkdirSync test-and-set; the loser returns busy instead of deleting the tree the winner is mid-install in), and a failed build writes a settled marker so later shards stop re-paying the install to relearn 'unavailable'. Also: Agent 7's brief now names hunk-survived and the hunks.* counters (it is the report's only consumer, and the finding class was invisible); hunk findings anchor at the first ADDED line instead of up to three context lines above the change. * feat(review): four round-2 borrowings — measured failure attribution, round ledger, richer mutants, doc parity Shaped by a live round-2 maintainer re-verification (QwenLM/qwen-code#7818): - qwen review test-delta: rerun the PR side's failed test commands on the built merge base and diff the failing FILE SETS — netNew is the PR's own failure by measurement (whatever files the diff touches), shared is pre-existing by measurement. Counts are never compared: a flaky suite fails different test names between runs of the same tree. An unfinished base rerun attributes nothing. - Round ledger: the incremental review cache persists confirmed findings under round-scoped ids (R1-2); a high-effort re-review rules on each (fixed / still stands / cannot tell) and opens its report with the table, the way a human round-2 comment opens with 'M1 is fixed'. - Three replacement mutation operators in test-efficacy: drop '?? fallback', force a comparison-bearing guard condition to 'true', drop a '+ CONST' term. Each survivor maps to one crisp untested-protection sentence. A line whose raw text and literal-blanked code view disagree yields no candidate — an edit index computed across the two views once spliced 'iftrue 0)' into a guard, and a mangled mutant reads as inconclusive while quietly spending a cap slot. - Quality brief: documentation-parity lens — a new user-facing surface whose siblings are documented is a Suggestion naming the sibling precedent; no documented sibling, no finding. * fix(review): address review feedback — false-positive hardening, binary diff guard, error convention (#8215) * fix(review): address review feedback — base-tree availability gate, test-plan false positives, hunk-probe ranges (#8215) - base-tree: only stamp a base tree available when runBuildTest actually compiled something (ok AND npm toolchain AND a non-empty build). An `unsupported` handoff or an empty npm scope returns ok:true having built nothing; marking that tree available let an A/B read the absence of a build as a behavioural difference. - cleanup: sweep the stale base-tree build lock a killed builder leaves behind. - test-plan: read the root manifest's scripts directly so a root-only script survives when the root defines no build/test; bail on the inline --root=./dir rebasing form; stop treating a positional after an inline --flag=value as the flag's value; prefer a failed scoped run when ruling a bare command; anchor the npm script alias to a full token so `yarn test:unit` is not truncated. - test-efficacy: exclude `\ No newline at end of file` from the startLine offset count; compute the mutant-overlap range from the header's new-side span so it no longer overshoots into a closely following hunk. * fix(review): address review feedback — diff-header false positives, stale prompt enumeration, added-file hunk probes (#8215) * fix(review): address review feedback — cd-base exclusion, Test Files count guard, base-tree error handling, probe delegation (#8215) * fix(review): port the collocated-dropout test to the post-#8050 runner seam Merging main brought #8050's Windows-portability refactor, which resolves the probe runner through vitest/package.json's bin — a node_modules/.bin fake is dead weight it never reads. The 8215-only collocated-dropout test still installed the old .bin fake, so the REAL vitest ran its fixtures, price.test.ts genuinely passed, and the hunk scored survived. The test now overrides the fake package's vitest.mjs like every post-refactor test. * fix(review): bound the summary rescue, apply the ATX heading rule, sweep stale build locks The three 8215-layer findings from the latest review, fixed at this layer (they were first patched further up the stack, where the reviewer of THIS PR cannot see them): - trimOutput's summary rescue is capped at 40 lines — uncapped, 40k lines of 'Test <n>: …' prose voided the trim entirely (measured 1.6MB in, 1.6MB out) and the bounded-output contract is the whole point. - A '#' with no following whitespace is prose, not a heading (the ATX rule GitHub applies): '#8176', '#tag', an unfenced '#!/bin/bash' no longer end the Test Plan section mid-body; the bare-#-run crash on the closing scan is guarded. - A base-tree build lock older than 30 minutes is a corpse left by a killed builder — swept and rebuilt instead of reporting busy for the rest of the review. * fix(review): EEXIST-only lock busy, bun test alias, chained cd bail, fence backreference Four live findings from the latest inline review round (the rest of the round was already fixed upstream by the takeover bot - verified by probing head behavior rather than re-reading the threads): - base-tree's lock catch distinguishes EEXIST (a concurrent builder, busy) from EPERM/EROFS/ENOSPC (this run's own failure, reported as such, not as a busy that will never clear). - "bun test" is bun's built-in runner, not a package-script alias: it runs whether or not any manifest defines test, so ruling it against the scripts table filed a false contradicted. - A chained cd matches the leading-cd shape but the single-hop resolver joined file tokens against the FIRST directory; it now bails like the exotic-cd case. - codeSpans' fence regex closes on its own marker via backreference; a tilde fence line inside a backtick block ended the span early and lines after it were lost to extraction. * fix(review): close the ten open findings on this PR - guard-true tested for a comparison anywhere after `if (`, including the then-body, so `if (ready) emit(a !== b);` admitted a mutant on the comparison-less condition the gate exists to exclude. It now tests the condition span only. - The `survived` detail said "when it changes" for legacy DELETION mutants too; it now matches the operator. - test-delta's `unparsed` required both sides to parse zero files, so a PR-side failure whose FAIL lines the trim scattered was silently dropped whenever the base rerun happened to parse. netNew/shared come from the PR side, so the PR side alone decides. - failingFilesOf now matches Windows path shapes (backslashes, C:) - a missed parse is an unattributed failure, not a loud error. - The replacement branch of runOneMutant (write-file -> run-probe -> classify) had no end-to-end test; one now drives a coalesce operator through the real handler and asserts the mutated line, the verdict, the operator-specific wording, and that the shared tree is untouched. - Two tests were vacuous with respect to what they promised: the baseline-dir test never asserted the cwd (its helper swallowed the argument - fixed at the helper), and the one-candidate-per-line test used an input that never triggered the replacement path, so the `continue` under test was not load-bearing. - Reattached the orphaned selectMutants JSDoc; reworded the SKILL line. * fix(review): the test helper's cwd parameter is required, not optional CI's `tsc --build` failed on test-delta.test.ts: the exec seam always passes a cwd, but the helper's signature marked it optional, so pushing it into a string[] was `string | undefined`. Missed locally because vitest runs through esbuild, which strips types without checking them - the suite was green while the build was red. The gate to run before pushing a type-level change is `npx tsc --build`, not the test suite. * fix(review): a base rerun that could not RUN attributes nothing Two Criticals from the latest review, both reachable on the brief's own happy path: - baseUnusable covered only timeouts. Every other way the base side can fail to run - an unbuilt base tree, a missing install, a workspace the PR ADDED (npm test --workspace cannot resolve on base), an ENOBUFS truncation - exits non-zero with zero FAIL lines, which this code read as a green base. Every PR-side failure then became netNew: the strongest evidence the command emits, manufactured from a base that never ran a test. It now attributes nothing and says why. - Timeout detection was the weaker substring form the sibling explicitly rejects; an external SIGTERM (container stop, cancelled job) set neither an ETIMEDOUT message nor an exit code and fed straight into the above. build-test now exports spawnTimedOut and test-delta asks the same question rather than re-deriving it. Also: the base output is trimmed (it precedes the verdict fields in the report the agent reads, so an untrimmed megabyte truncates exactly what the command produces); the guard-true gate no longer reads an arrow function's => as a comparison (every predicate guard was a candidate - the if (ready) noise the gate exists to exclude); the term-drop message no longer calls a string concatenation a reserve term; the unparsed note describes its own PR-side-only condition; and the ledger's Step 6/Step 8 now agree that a still-standing finding keeps its id. * fix(review): compare failing files by a normalised, project-keyed identity Critical: the two sides run in DIFFERENT roots (the PR worktree and the base tree), and netNew/shared compared the parsed paths verbatim - so an absolute-path runner turned every pre-existing failure into a fabricated Critical, with the authority of a measurement behind it. Paths are now normalised against each run's own root (and backslashes to /, so a Windows path compares with its POSIX-printed twin), which is why test-delta gained --pr-worktree. The identity also keeps the vitest project token: dropping it collapsed same-named test files across workspaces, so a PR-caused failure in one package could read as pre-existing because another package has a file by the same name - the worse failure direction. Also from the same review, all of them reachable on the brief's own path: - The base rerun now inherits build-test's stdio: ['ignore','pipe','pipe'] ("a build that asks a question is a build that hangs until the deadline") and its trimOutput, which matters because entries[].base precedes the verdict fields in the report the agent reads. - The brief gates on base-tree's `available`, not just its `path`: a tree that was created but did not build populates path too, and measuring against it turns an infrastructure failure into Criticals. - A programmatic caller omitting `timeout` no longer sends NaN into spawnSync. - MutantCandidate is a discriminated union, so an operator without its replacement line - which would delete a line while reporting "with its ?? fallback dropped" - is unrepresentable. - The comparison class no longer requires a trailing space (if (a<b) is the same guard, just unformatted) and matches a brace-less else if. - DeltaEntry.unparsed's doc now describes the PR-side-only condition it actually implements. * fix(review): restore the whole-command budget, keep generics out of guard-true Round-1 findings from a fresh review of this PR: - test-delta had no aggregate deadline: --timeout is PER command and defaults to 300s, so three failed commands is 900s against Agent 7's 600s ceiling - killed with NO report at all, discarding the base-tree install and build just paid for. TOTAL_BUDGET_MS mirrors the one test-efficacy reserves; commands it cannot fit are disclosed. - guard-true matched generic calls: `if (isRecord<string>(v))` produced a mutant, and a type-guard predicate is exactly the `if (ready)` shape whose survivors the gate calls noise. The trailing \s is required, not an accidental asymmetry with [!=]== - telling `a<b` from `fn<T>(x)` needs a parser, and the gate is silence-biased by design. - --pr-worktree had no contract test, and its failure mode is the worst here: arriving undefined, root stripping silently stops and every pre-existing failure becomes a fabricated netNew. The new test feeds parseSync's output straight into runTestDelta and asserts an attribution only reachable when both roots were stripped (verified red against the snake_case field shape that shipped once already). - Merged the two consecutive doc comments on prWorktree. * fix(review): a budget-shortened deadline is not the same fact as a slow rerun Round-2 finding on the budget just restored: `Math.min(perCommandMs, remaining)` can hand a rerun far less than --timeout, and if it dies there the note said only "timed out - infrastructure, not evidence". True, but it sends the reader hunting a hang that is really an exhausted budget - and unlike a real timeout, a rerun with budget to spare would still measure it. The note now names those commands separately and says so. Verified red against removing the tracking line. * fix(review): restore the replacement sub-cap, stranded on a downstream branch Round-3 finding, and the third instance of one class: an 8218-layer fix committed on the 8261 branch, four PRs above the code it belongs to. Measured over 40 real commits, the replacement operators produce ~24x the deletion pool (215 vs 9 candidates; guard-true drives it). Every mutant run drains the same window hunk probes draw from LAST, so uncapped, most diffs with any replacement candidates leave hunk probing zero runs - the hunk-survived finding class silently stops firing and nothing says so. Three slots, and what the sub-cap drops is counted in skippedForCap rather than lost. Also swept the other direction: diffed every review file against its 8261 copy to confirm nothing else 8218-layer is stranded up there. The remaining divergence is 8261's own (the positive control, its lenses). * docs(review): complete the "delta cannot rule" enumeration in both places Round-4 finding. The brief and SKILL.md each listed three cases where test-delta attributes nothing - unparsed, timed-out base, no merge base - but the code has five: the later rounds of this PR added "a base rerun that failed without naming any failing file" (it did not measure the base) and "a command the whole-command budget could not fit". Two enumerations of the same set with different membership, in the two places an agent reads. That is the sibling-enumeration lesson this skill teaches, applied to its own prose for the second time: the fix is not just adding the missing members but saying that the report names each case with its own reason rather than folding them into one. * docs(review): the rationale named only one of the ways base goes unmeasured Third and last copy of the enumeration the previous commit fixed. The bullet's headline already generalised - "base attributes nothing it did not finish" - but its body named only the timeout, so a reader learning the contract from the rationale would conclude timeouts are the only unusable case. Name the set, and say why the report keeps the reasons apart: "we could not measure" and "we measured nothing" are different facts to the author, and only one of them is about their PR. * fix(review): a file-count label stops counting at the end of its line `Test Files 45 passed` filing its 45 as a differing TEST count was fixed once, with a lookbehind on the bare-count pattern. That only ever rejected the all-green shape. The moment any file fails, the runner prints `Test Files 1 failed | 44 passed (45)`, the label is no longer adjacent to the number, and 44 comes through as a test count - so the note reads "claimed 44, observed 1323" on exactly the runs whose summary someone would paste. Adjacency was the wrong invariant; the line is. Masking from the label to end-of-line is distance-independent and picks up jest's `Test Suites: 1 failed, 44 passed, 45 total` at no cost. The label keeps its `Test` word on purpose: a first cut matched a bare `files` and blanked the line in "expect all four files and 471 tests to pass", silencing a real claim. An existing test caught that, which is the argument for the rule being as narrow as it is - anything that suppresses claims is worth exactly its narrowness. Also drops the now-dead lookbehind, which a reviewer had separately (and wrongly) called a JS syntax error; variable-length lookbehind is legal in V8, and the module parsed fine. It goes because the line rule subsumes it, not because it was broken. * fix(review): parse the base rerun before its output is trimmed `trimOutput` rescues module errors and runner summaries out of the omitted middle, not the per-file FAIL lines this command reads. A base suite whose failure section overruns the tail budget therefore lost failing files into the gap - and a SHORT base set is the dangerous direction, because netNew is the PR side minus the base side. Every file the trim hid came back as a Critical attributed to this PR by "measurement". Parse the raw text, report the bounded one. The PR side cannot be fixed here: it is read out of build-test's stored output, trimmed before this command existed. That loss runs the other way - it understates `shared`, never invents a netNew - so the entry carries `prTruncated` and the note says the list may be partial. A silence-biased gap is still a gap the author should hear about. Also names both selection caps in the mutant-skip diagnostic. The count accumulates replacement sub-cap drops, and with 2 deletions and 6 replacements the total is exactly MAX_MUTANTS: the main cap never fires, yet 3 are dropped, and the message sent the reader looking for a pool of 11 candidates that never existed. * fix(review): rerun only the command shapes build-test emits This command reads a report off disk and then executes the strings in it with `shell: true`, in the base worktree. Nothing else in the pipeline re-executes a value it read back from a file, so nothing else has to care where that value came from - this does, and the provenance is worse than it looks: the command is `npm test --workspace="<dir>"`, the workspace token is a directory, and a directory is a name a pull request chooses. `packages/x";curl …|sh;"` is a legal path in git and on Linux and it round-trips through the report into a shell. Restricting to the emitter's own grammar costs nothing real, since that grammar is the two shapes build-test produces. A command outside it is skipped and disclosed, the same treatment everything else this command cannot do already gets, so a future shape degrades to "judge it by the diff" rather than to arbitrary execution. --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Code <qwen-code@users.noreply.github.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>	2026-08-01 12:34:30 +00:00
qqqys	36698c5669	feat(workflows): support opt-in background runs (#8303) ... * feat(workflows): support opt-in background runs * test(cli): assert workflow completion chain routing --------- Co-authored-by: qwen-code-dev-bot <269191875+qwen-code-dev-bot@users.noreply.github.com>	2026-08-01 12:18:48 +00:00
qwen-code-dev-bot	9d6d5568cd	Merge branch 'main' into feat/summary-export-path	2026-08-01 19:47:20 +08:00
qqqys	4aa7d1ec0b	feat(cli): adopt Goal v3 in interactive TUI (#8005) ... * feat(cli): adopt Goal v3 in interactive TUI * fix(cli): restore Goal CI contracts * fix(cli): stop a queued message from stalling the Goal loop (#8005) Reserve the next Goal turn only for a user batch the queue will actually release: peekNextUserBatchKey now mirrors the two-lane drain gate, so an active Goal turn no longer binds a permit to a held plain message and the loop keeps running. Also release the Goal turn binding on the background-capacity early return, drop a redundant getSteerInput spread, recognise the legacy /goal clear aliases in interactive mode, and restore the rollback assertion for loadPausedBackgroundAgents. * fix(cli): address review feedback on Goal queue and retry hint (#8005) * fix(cli): restore Goal trust gate and address review feedback (#8005) * fix(cli): address Goal review feedback on cancel release, icons, claim gate (#8005) * fix(cli): address Goal review feedback on non-interactive guard, icons, and drain retry (#8005) * fix(cli): address review feedback on Goal v3 TUI (#8005) - Strip 'set' keyword before forwarding to legacy path in non-interactive /goal set, preventing the keyword from leaking into the goal condition - Add goalTerminalErrorRef to prevent post-stream cleanup from wiping Goal turn terminal errors - Add 50-turn continuation budget to Goal runtime, matching the legacy MAX_GOAL_ITERATIONS cap - Display verifier rejection status cards (verifier_reject cause) - Clear stale lastReason when editing a goal objective * fix(test): align client-goal turn count with continuation budget (#8005) * fix(cli): address Goal set-clear and delivery-error review feedback (#8005) Non-interactive `/goal set <clear-keyword>` now sets a literal objective instead of clearing the active goal, by bypassing the clear-keyword check for explicit set operations. Goal-turn stream errors now fire onDeliveryFailed rather than onDelivered by including goalTerminalErrorRef in the post-stream delivery dispatch. * fix(cli): rename misleading missingActiveGoalContext variable (#8005) * fix(cli): stop Goal queue from stranding input and orphaning prompts (#8005) Only an active Goal turn holds ordinary input now; paused, blocked and usage_limited states drain the queue normally so a user whose Goal is merely paused (e.g. via Escape) is never stranded waiting for /goal clear. A cancelled Goal continuation turn also strips its synthetic "no new real user input" prompt from the chat history. Previously the auto-restore branch bailed before its orphan strip ran (Goal turns add no UI user item), so the preamble survived and appendCuratedContent merged the user's next real message into it. * fix(core): address Goal resume-budget and mid-turn clear review feedback (#8005) Resuming a Goal that exhausted its 50-turn continuation budget was accepted and reported as `active`, then immediately re-transitioned to `usage_limited` without running a turn: the resume branch kept the exhausted `turnCount`, which `queueContinuation` re-checks. Resume now resets `turnCount` so an explicit resume grants a fresh continuation budget and the reported outcome matches the settled one. A mid-turn `/goal clear` with no active Goal was silently swallowed because its causeless `goal_control` result rendered only when idle. The handler now renders any causeless result (a `status` read or a no-Goal `clear`), which never broadcasts and so cannot double-render. * test(cli): cover Goal tool-batch fail-close guards (#8005) * fix(core,cli): guard budget-exhaustion identity and pass goalContext to tool-result recording (#8005) The budget-exhaustion callback in queueContinuation only checked goal status, not identity. A replace dispatched during the journal append could create a fresh goal that the stale callback then incorrectly usage-limited. Capture goalId/revision at enqueue time and return early on mismatch, matching the existing identity-guard pattern used by handleStartFailure and recordVerificationOutcome. The TUI recorded tool results without goalContext, so the evidence catalog never admitted tool-result records and any Goal whose objective depended on external state could not be verified. Pass request.goalContext at both recordToolResult call sites in useGeminiStream, tagging get_goal/update_goal results as goal_runtime to match the CoreToolScheduler pattern. --------- Co-authored-by: qwen-code-dev-bot <269191875+qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>	2026-08-01 11:21:53 +00:00
Aleks-0	933d78a089	feat(cli): add /model --compaction for configurable chat compression model (#7818) ... * feat(cli): add /model --compaction for configurable chat compression model * fix(cli): regenerate settings.schema.json with compactionModel field * fix(cli): address review findings — fix tab-completion, add i18n, add tests for compaction model * fix(cli): add missing i18n keys for compaction model check-i18n * fix(cli): address review #4787388264 — Left arrow, resolveModelId, clear path, filters - Add isCompactionModelMode to Left arrow close handler in ModelDialog - Use resolveModelId() for preferredCompactionModelEntry lookup - Implement /model --compaction clear path (persistSetting undefined) - Add !m.fastOnly to --compaction inline filter - Add !m.visionOnly to --fast inline filter - Replace duplicate compaction test with authType:modelId selector test QwenCode QwenLLM * fix(cli): address review #4789474041 — add tests and i18n keys - Add test for /model --compaction clear path - Add test for isCompactionModelMode selection handler - Add i18n key 'Compaction model override cleared' to all locales - Change clear syntax from trailing space to explicit 'clear' keyword QwenCode QwenLLM * fix(cli): address review #4791100466 and #4791461620 - Add imageModelMode guard to setIsCompactionModelMode (mutual exclusivity) - Add !m.fastOnly to getCompactionModel() availability filter - Add fallback chain test (compactionModel -> fastModel) in chatCompressionService - Add completion test for --compaction flag in modelCommand QwenCode QwenLLM * fix: add visionOnly test and handle missing settings on compaction clear * fix(cli): remove dead disjunct and add visionOnly to compaction completion test * fix(cli): add visionOnly filter to --voice handler and test compaction-model dialog dispatch * fix(core): remove fastModel fallback from compaction chain and add context window guard - Remove getFastModel() from getCompactionModel() fallback: compactionModel -> main model only. Prevents silent behavior change for users with fastModel configured. - Add context window guard in ChatCompressionService: if the slimmed payload exceeds the compaction model's context window, fall back to the main model for that compression and emit a warning. - Thread warning through ChatCompressionInfo -> UI (TUI + ACP Session). - Update config.test.ts and chatCompressionService.test.ts for new behavior. QwenCode QwenLLM * fix(core): address review findings — stale i18n, undefined leak, guard estimate, compress warning, vacuous test - Update all i18n locales (en/zh/zh-TW), settingsSchema, settings.schema.json, config JSDoc: remove fastModel fallback references - Coalesce getCompactionModel() to main model so undefined never leaks to fastModel via resolveDefaultModel - Add COMPACT_MAX_OUTPUT_TOKENS to guard estimate (providers count output against same window) - Surface warning in compressCommand.ts (acp, interactive, non-interactive) - Fix vacuous completion test: rename decoys to qw-prefix so exclusion filter is exercised QwenCode QwenLLM * fix(core): address review round 2 — guard skip for main model, undefined fallback test, visionOnly + warning tests - Skip context window guard when effective model is the main model (no confusing warning when nothing configured) - Fix fallback test to mock getCompactionModel → undefined, exercising the ?? coalesce - Add visionOnly propagation and conflict tests in modelRegistry.test.ts - Add warning display test in compressCommand.test.ts QwenCode QwenLLM * fix(core): count system prompt in guard estimate and add mutation-gap tests - I1: hoist buildCompressionSystemPrompt() above the guard, include its token estimate (systemInstruction.length / CHARS_PER_TOKEN) in slimmedTokenEstimate — closes the band where the guard passed but the provider returned 400 - Add 3 mutation-gap tests: large window (model used, no warning), undefined coalesce (main model not fastModel), guard skip for main model (no confusing warning) QwenCode QwenLLM --------- Co-authored-by: QwenCode <qwen-code@qwen.ai>	2026-08-01 11:19:19 +00:00
qwen-code-dev-bot	f18300f53c	test(cli): cover post-LLM symlink re-check and interactive error UI (#8116)	2026-08-01 10:52:56 +00:00
易良	bd85599139	feat: visualize ordinary-session plan execution (#7580) ... * feat: visualize ordinary session plan execution * fix(web-shell): complete plan workflow rendering * fix: preserve authoritative plan state * fix(core): isolate teammate todo state * fix: reject ambiguous empty plan snapshots * test(web-shell): cover workflow dialog wiring * feat(web-shell): preview workflow before execution * feat(web-shell): inspect workflow step details * feat(web-shell): preserve session workflow history * test: align failing CI tests with plan-isolation behavior SubAgentTracker: the emitter now guards subagent TodoWrite results (tool-call-emitter emitResult early-returns on subagentMeta), so a subagent todo no longer promotes into a session-level plan. Flip the stale assertion to expect no plan emission, mirroring the dedicated guard test in tool-call-emitter.test.ts. HistoricalPlanExecution: the pagination fixture's onLoadOlderHistory returned Promise<void>, but PlanExecutionHistoryProvider requires Promise<boolean> and throws 'Unable to load earlier session history' on a falsy resolution. Production wires loadOlderHistory (resolves true after layout); return true in the fixture to model a successful load. * refactor: simplify session plan execution workflow * fix(web-shell): preserve split plan approval workflow --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>	2026-08-01 10:48:30 +00:00
ytahdn	554c5e44ba	feat(web-shell): support mutable default mid-turn messages (#8229) ... * feat(web-shell): support mutable default mid-turn messages * fix(serve): register mid-turn removal telemetry route * test(serve): update telemetry route totals * fix(test): add session_mid_turn_message_mutation to expected features list * fix(webui): forward clientId on cross-session mid-turn removal (#8229) - Forward the session clientId in the cross-session removeMidTurnMessage branch so the bridge's exact-originator match can succeed; without it the removal resolved to an undefined originator and could never remove the message stamped at enqueue. - Strip a misaligned/malformed messageIds from mid_turn_message_injected in asKnownDaemonEvent instead of rejecting the whole event, mirroring the sidechannel parser so a buggy daemon can't silently lose the injection signal. - Log a mid-turn removal miss in the bridge like the enqueue/pending-removal siblings, to make removal races diagnosable from daemon logs. * fix(web-shell): exclude annotations from mid-turn path and harden idle cleanup (#8229) * fix(web-shell): add container-type to .queuedPrompts so @container query applies (#8229) * fix(web-shell): harden mid-turn dedupe and capability gate per review (#8229) - removeInjectedFromQueue now matches by id first (position-independent) and falls back to text only when no id match exists, so two same-text sends can't remove the wrong row and double-deliver. - Thread canMutateMidTurn into useQueuedPrompts and gate the mid-turn delete/edit mutation on it, so the keyboard path can't hit a DELETE route the daemon doesn't advertise. - asMidTurnMessageInjectedData omits a malformed messageIds key instead of leaving a present undefined, matching the sidechannel parser. - Narrow MidTurnQueueItem.midTurnState, document the load-bearing effect order, and make clearQueuedPrompts return false on a no-op clear. * fix: harden mid-turn removal per review (log escape, cross-session client id) (#8229) - Escape the caller-controlled messageId (and sessionId) in the mid-turn removal-miss stderr line to prevent log injection (CWE-117). - Forward the target session's persisted client id on cross-session mid-turn removal so the bridge's exact-originator match no longer rejects valid removals after a session switch with per-session client ids. - Strengthen tests: distinct-id independence for two queued messages, deferred removal proving the composer waits for daemon removal, and the active-turn delete failed-action flag. --------- Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>	2026-08-01 10:41:29 +00:00
qwen-code-dev-bot	543b1206c2	Merge branch 'main' into feat/summary-export-path	2026-08-01 17:41:16 +08:00
Shaojin Wen	6f8ad2b4a5	feat(review): Include CLI version in attribution (#8294) ... * feat(review): Include CLI version in attribution Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(review): Decouple footer test from package version Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-08-01 09:40:57 +00:00
Zqc	75f487c238	feat(cli): Add agent view supervisor runtime (#7799) ... * feat(cli): add agent view supervisor runtime * fix(cli): harden agent view supervisor runtime * 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) --------- 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>	2026-08-01 08:37:37 +00:00
Qwen Code Bot	504492104e	fix(cli): address review feedback on /summary custom path (#8116)	2026-08-01 08:19:22 +00:00
Shaojin Wen	e6e59fbd0c	feat(review): Test Plan claim check, base-tree A/B harness, per-hunk probes (#8215) ... * feat(review): borrowed-verification trio — test-plan check, base-tree A/B, per-hunk probes - qwen review test-plan: rule on the PR Test Plan's checkable claims (paths, npm scripts, test counts) against the reviewed tree; contradictions and differing counts are disclosed via compose-review, never capping. - qwen review base-tree: build the merge base in a sibling worktree so the verifier can A/B a comparative claim instead of reading it; swept by cleanup. - test-efficacy: third probe kind — reverse-apply one hunk at a time and re-run the affected tests, attributing a still-green suite to the specific change nothing gates; shares the mutants' budget window, runs last. * fix(review): survive real runner output — ANSI-laced and trimmed-away summaries Both measured on a live /review of QwenLM/qwen-code#8176 with the built CLI: - test-plan's observedTestCounts strips SGR sequences before matching; a color-enabled pipe interleaves them BETWEEN tokens, and the count claim fell to 'unchecked' with the summary right there in the report. - build-test's trimOutput rescues runner summary lines from the omitted middle (like module-resolution errors): a failing suite's tail is all failure details and npm epilogue, which pushed the one-line summary out of the kept text entirely. * fix(review): address the eight findings from live review of this branch All measured in the review (QwenLM/qwen-code#8215 review comment): - test-plan: linear-time bold-heading scan (the old pattern backtracked catastrophically on an unclosed ** line an untrusted PR body controls); a flag preceding the npm script yields no claim instead of a false 'no package defines this script'. - test-efficacy: a hunk probe's restore recreates the parent directory a reverse-applied 'new file' hunk removed (the ENOENT from finally lost the verdict and marked every remaining hunk inconclusive); hunks get their own skippedForBaseline instead of mislabeling a red baseline as a budget skip; splitDiffIntoHunks re-captures the file header at every diff --git boundary; a hunk-survived finding notes when it restates an inert file-level revert at hunk granularity. - base-tree: idempotent fast path keyed on a build marker + HEAD check — concurrent verifier shards reuse one built tree instead of sweeping it out from under each other mid-A/B (a fabricated base-side difference with a deterministic source tag was the worst case); cost wording is now 'an install and a build' everywhere it was 'one extra build'. * fix(cli): never score a hunk survived when its own test left the baseline (#8215) A per-hunk probe reported `survived` whenever the green baseline probes still passed with the hunk reverted. When the hunk's own collocated test dropped out of the baseline (a probe-tree import error collects nothing), the remaining green probes prove only that THEY do not cover the hunk, so the verdict is now `inconclusive` — the same dropped-test asymmetry the mutants already hold. Also scope the hunk-survived cross-reference note to the hunk's own collocated test, and let test-plan match a workspace-scoped run of the plan's bare command instead of falling through to the manifest on an exact-string miss. * fix(review): silence-bias hardening from four live review rounds of this branch The two blocking findings, reproduced on this PR's own Test Plan: - test-plan files no false contradicted notes: npm rulings move from a four-verb denylist to an allowlist (the run form + npm's script aliases — the ~fifty other builtins each used to become 'no package defines this script'); a slash token is claimed as a repo path only with evidence (an extension or ./ prefix), never when it is a flag's value (--repo owner/repo) or under the review's own temp root; HEADING_LINE_RE drops the same quadratic shape its bold sibling was rewritten to remove. - base-tree gets a real mutual-exclusion lock around sweep+add+build (mkdirSync test-and-set; the loser returns busy instead of deleting the tree the winner is mid-install in), and a failed build writes a settled marker so later shards stop re-paying the install to relearn 'unavailable'. Also: Agent 7's brief now names hunk-survived and the hunks.* counters (it is the report's only consumer, and the finding class was invisible); hunk findings anchor at the first ADDED line instead of up to three context lines above the change. * fix(review): address review feedback — false-positive hardening, binary diff guard, error convention (#8215) * fix(review): address review feedback — base-tree availability gate, test-plan false positives, hunk-probe ranges (#8215) - base-tree: only stamp a base tree available when runBuildTest actually compiled something (ok AND npm toolchain AND a non-empty build). An `unsupported` handoff or an empty npm scope returns ok:true having built nothing; marking that tree available let an A/B read the absence of a build as a behavioural difference. - cleanup: sweep the stale base-tree build lock a killed builder leaves behind. - test-plan: read the root manifest's scripts directly so a root-only script survives when the root defines no build/test; bail on the inline --root=./dir rebasing form; stop treating a positional after an inline --flag=value as the flag's value; prefer a failed scoped run when ruling a bare command; anchor the npm script alias to a full token so `yarn test:unit` is not truncated. - test-efficacy: exclude `\ No newline at end of file` from the startLine offset count; compute the mutant-overlap range from the header's new-side span so it no longer overshoots into a closely following hunk. * fix(review): address review feedback — diff-header false positives, stale prompt enumeration, added-file hunk probes (#8215) * fix(review): address review feedback — cd-base exclusion, Test Files count guard, base-tree error handling, probe delegation (#8215) * fix(review): port the collocated-dropout test to the post-#8050 runner seam Merging main brought #8050's Windows-portability refactor, which resolves the probe runner through vitest/package.json's bin — a node_modules/.bin fake is dead weight it never reads. The 8215-only collocated-dropout test still installed the old .bin fake, so the REAL vitest ran its fixtures, price.test.ts genuinely passed, and the hunk scored survived. The test now overrides the fake package's vitest.mjs like every post-refactor test. * fix(review): bound the summary rescue, apply the ATX heading rule, sweep stale build locks The three 8215-layer findings from the latest review, fixed at this layer (they were first patched further up the stack, where the reviewer of THIS PR cannot see them): - trimOutput's summary rescue is capped at 40 lines — uncapped, 40k lines of 'Test <n>: …' prose voided the trim entirely (measured 1.6MB in, 1.6MB out) and the bounded-output contract is the whole point. - A '#' with no following whitespace is prose, not a heading (the ATX rule GitHub applies): '#8176', '#tag', an unfenced '#!/bin/bash' no longer end the Test Plan section mid-body; the bare-#-run crash on the closing scan is guarded. - A base-tree build lock older than 30 minutes is a corpse left by a killed builder — swept and rebuilt instead of reporting busy for the rest of the review. * fix(review): EEXIST-only lock busy, bun test alias, chained cd bail, fence backreference Four live findings from the latest inline review round (the rest of the round was already fixed upstream by the takeover bot - verified by probing head behavior rather than re-reading the threads): - base-tree's lock catch distinguishes EEXIST (a concurrent builder, busy) from EPERM/EROFS/ENOSPC (this run's own failure, reported as such, not as a busy that will never clear). - "bun test" is bun's built-in runner, not a package-script alias: it runs whether or not any manifest defines test, so ruling it against the scripts table filed a false contradicted. - A chained cd matches the leading-cd shape but the single-hop resolver joined file tokens against the FIRST directory; it now bails like the exotic-cd case. - codeSpans' fence regex closes on its own marker via backreference; a tilde fence line inside a backtick block ended the span early and lines after it were lost to extraction. --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Code <qwen-code@users.noreply.github.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>	2026-08-01 07:58:00 +00:00
Dragon	e569734a1e	feat(skills): add auto-skill curator (#7846) ... * feat(skills): add auto-skill curator * fix(i18n): translate /curator command descriptions for zh and zh-TW The strict-parity locale coverage test failed because the new /curator command and its status/run/restore subcommands fell back to English descriptions in zh-CN and zh-TW. Add Simplified and Traditional Chinese translations for the curator command descriptions and its user-facing output strings. * fix(i18n): add English source keys for /curator command The strict-parity i18n check failed because zh and zh-TW defined the new /curator command keys while en.js (the source of truth) did not, producing extra-key parity errors. Add the matching English source entries so all supported locales share the same key set. * fix(skills): align curator lifecycle safeguards * fix(skills): harden curator trust and name guards * fix(skills): guard curator mutations by workspace trust * test(skills): cover curator stale-to-active reactivation path * test(skills): cover curator rollback and restore-collision paths Add coverage for two previously untested error paths in the auto-skill curator: - restoreArchivedAutoSkill refusing to overwrite an existing active directory, leaving both the reused directory and the archived copy intact. - runAutoSkillCurator rolling back an archive rename when the post-move state persistence fails, returning the skill to the live library and leaving nothing stranded in the archive (new isolated test file that mocks atomicWriteJSON to fail once). * test(cli): cover curator command errors and stacked auto-skill usage - Add mockRejectedValue error-path tests for the /curator status, run, restore, and pin commands, asserting each surfaces messageType 'error' and that a failed run/restore skips skill-discovery refresh. - Add positive stacked auto-skill tests to both the non-interactive and interactive slash-command paths, asserting recordAutoSkillUsage is called once per successful stacked skill carrying project skillDetail. * fix(skills): reject control-byte auto-skill directory names isManagedDirectoryName only checked the auto-skill- prefix and basename, so a crafted directory whose name embeds ANSI/control bytes was treated as a managed skill and its name printed verbatim by the non-interactive /curator output (which, unlike the TUI, does not run escapeAnsiCtrlCodes), enabling terminal control-sequence injection from a cloned repo. Require the directory name to match SKILL_NAME_PATTERN. A managed dir is always auto-skill-<name> where <name> passes validateSkillName and the prefix chars are within the same charset, so this never rejects a legitimately generated directory (including Unicode skill names, which an ASCII-only guard would wrongly drop) while excluding ESC/control bytes. Add a regression test covering a crafted directory with a valid manifest name so only the directory-name guard can exclude it. * fix(skills): guard curator state reads and clarify restore errors Align the curator state read path with the noFollow/lstat guards every write already uses: refuse a symlinked or non-regular-file state file (which could otherwise be followed to an external path, /dev/zero, or a FIFO, causing OOM or a boot hang in untrusted workspaces) and cap the read size. Also distinguish a present-but-ineligible archived skill from a missing one in restore error messages. * fix(skills): close curator TOCTOU reads and preserve seeding baseline Address review feedback on the auto-skill curator: - readManagedSkill previously read the manifest via Promise.all([lstat, lstat, readFile]); the readFile ran concurrently with the lstat guards, so a symlinked SKILL.md pointing at /dev/zero could start an unbounded read before the guard rejected it. Read the manifest with O_NOFOLLOW + an fstat size bound instead (shared readRegularFileNoFollow helper), refusing symlinks atomically and bounding the read. - readState had the same lstat->readFile TOCTOU window; the O_NOFOLLOW read closes it while keeping the existing friendly error messages. - First-run seeding overwrote firstSeenAt/lastActivityAt with now even when recordAutoSkillUsage had already created a record, resetting the inactivity clock. Preserve an existing baseline (like useCount/pinned/ lastUsedAt), while a brand-new skill still gets a fresh now baseline. Adds regression tests for the seeding-baseline preservation and for refusing a symlinked manifest. * fix(core): harden auto-skill curator per review feedback (#7846) - Resolve node:fs constants lazily so importing the curator does not crash tests that mock node:fs without a constants export. - Preserve the original error via cause when a rollback also fails. - Apply the skill-name charset guard to archived directory names reserved in the review-agent task prompt. * test(core): cover curator restore rollback and re-read guard (#7846) * fix(core): record auto-skill usage on re-invocation (#7846) * fix(skills): preserve curator read failures * fix(skills): harden curator lifecycle guards * test(skills): cover curator safety paths * fix(skills): address curator review findings (#7846) - Ignore future manifest mtimes in lastActivityMs so a bogus timestamp cannot make a skill permanently un-curatable - Skip archived status entries whose directory is also live, preventing contradictory double-listing in /curator status - Check the weekly interval before acquiring the cross-process lock in maybeRunAutoSkillCurator so most boots skip the lock entirely - Use handle.readFile() instead of a single handle.read() to avoid silent truncation on short reads * fix(cli): localize curator messages * test(skills): cover curator usage safeguards * fix(skills): address curator review feedback (#7846) - Isolate per-skill rename failures so a transient error no longer aborts the whole pass and prevents state persistence (boot-loop fix) - Make usage recording fire-and-forget (void instead of await) since it is already best-effort and nothing consumes the result - Skip state file creation when no auto-skills exist - Prune dead records whose directory exists in neither root - Sanitize user-supplied directory names in error messages (JSON.stringify) to close the ANSI control-sequence echo path - Split reserved skill names into active/archived lists in the review-agent task prompt - Make collision output actionable with remediation guidance - Fix rollbackMoves mutating its argument (moved.reverse → copy) - Add null guard to isMissing for non-object rejections - Add locale keys for skippedErrors output (9 locales) --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: Qwen Autofix <qwen-autofix@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>	2026-08-01 07:37:20 +00:00
Qwen Code Bot	523cf481f7	fix(cli): address review feedback on /summary custom path (#8116) ... - Skip symlink-escape check for the default .qwen/ target so a symlinked .qwen/ directory (shared team config, overlay mounts) keeps working, and the check no longer runs after the LLM call - Re-run the overwrite guard immediately before writing to close the TOCTOU window across the slow generation step - Determine isDefaultTarget by comparing the resolved path against the default so `/summary .qwen/` gets the same 0o700 permissions - Only chmod 0o600 on file creation; preserve existing permissions on regeneration - Return empty content in interactive-mode errors to avoid double rendering (failInteractive already adds the error to history) - Tighten the overwrite-guard regex to require `**Update time**: ` after the Summary Metadata heading, preventing false positives - Fix the realpathNearestExisting comment to document the missing containment-during-walk guard vs export/stats copies - Add tests: symlink cycle, default target with symlinked .qwen, TOCTOU overwrite guard, explicit .qwen/ permissions, chmod preservation, interactive error content, regex false-positive	2026-08-01 05:39:01 +00:00
易良	07c832ce37	refactor(cli): remove ACP private serve dependencies (#8141) ... * refactor(cli): move ACP contracts out of serve * fix(cli): guard invalid CPU core counts * style(cli): remove stray double blank line in daemon-metrics-ring (#8141) --------- Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>	2026-08-01 05:29:53 +00:00
qqqys	f62fc76533	feat(workflows): bubble workflow agent approvals (#8240) ... * feat(workflows): bubble workflow agent approvals * fix(cli): forward host deny message on workflow approval (#8240) * fix(cli): advance permission queue on abort and add approval observability (#8240) * fix(cli): address review feedback on workflow approval PR (#8240) - Remove abort listener leak on shared turn signal in Session.ts by holding the listener reference and cleaning up via .finally() - Guard stale permissionResponse in WebViewProvider after auto-resolve - Add stream-json defensive guard to handleWorkflowApproval - Add regression test for normal diff-based edit prompts in PermissionDrawer - Document seenSources dedup safety in workflow-run-registry --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>	2026-08-01 05:26:31 +00:00
易良	a3b5b4d856	fix: make the test suite portable on Windows (#8050) ... * ci: add temporary Windows compatibility validation * fix(acp): resolve sandbox mounts with POSIX semantics * test: normalize Windows runner environment * ci: fix Windows validation environment setup * fix: make path handling portable on Windows * test: make suites portable across Windows * ci: normalize Windows validation environment * fix: finish Windows test portability * test: finish Windows runner validation * test: stub Windows efficacy probes deterministically * fix(review): launch npx correctly on Windows * fix(test): harden review probe portability * test(cli): keep bridge spawn assertions strict * fix: address remaining Windows-portability review feedback - Split cursor integer validation: relax dev/ino to Number.isInteger for Windows 64-bit file indices (which exceed 2^53) while keeping snapshotSize/position safe-integer, since those are arithmetic operands. - Derive session-mention unescaping from the shared SHELL_SPECIAL_CHARS set via a new unescapeShellSpecials helper, so the escape set no longer diverges between POSIX and Windows. - Restore test teeth: a distinguishable realpath mapping in the weixin send test, and exact-basename assertions in the review paths test. - Nits: report POSIX-only skips via it.skipIf instead of if-wraps, inline path.posix in installationInfo, name the win32 traversal assumption in permission-manager test titles, explain the NUL-byte transcript trick, and use a single LC_ALL=C.UTF-8 in the Windows runner workflow. * fix: address Windows-portability review feedback - Run the Windows runner env-config step under pwsh so Out-File writes UTF-8 (no BOM) into GITHUB_ENV; Windows PowerShell 5.1 wrote UTF-16LE, which the runner parsed as UTF-8 and silently left TEMP/TMP unset. Also echo TEMP in the verify step so a regression is visible in the logs. - Restore the not.toHaveProperty('sessionScope') assertion in the serve bridge test: toEqual ignores undefined-valued keys, so on its own it no longer proved the sessionScope key is truly absent. - Drop the now-dead chmodSync on the fake vitest runner, which is invoked through process.execPath so the executable bit is irrelevant. * fix: address follow-up Windows-portability review feedback - Resolve the probe's vitest entry via createRequire + the package bin field instead of a hard-coded path, with self-explaining errors - Normalise result-path separators only on win32 so a POSIX backslash filename cannot false-match a probe - Add a literal V1 projection test, a vitest-entry canary, and a TEMP-redirection assert; clarify intent comments * test(cli): fix session-scoped symlink test setup * ci: use Windows PowerShell for runner setup * test(cli): respect localized extension list output * ci: limit Windows validation to workspace tests * docs(ci): clarify Windows validation scope * test(cli): create session-scoped parent in symlink fixture (#8050) * test(cli): cover vitest bin resolution errors * fix(cli): preserve runtime session cleanup after merge * fix(test): cover Windows portability edge cases * test(core): cover session profiler without nofollow * fix(review): expose dependencies to efficacy probes * fix(review): isolate efficacy probe caches * fix(review): preserve probe dependency bins * fix(review): ignore dependency metadata files * fix(review): address probe symlink assertion and doc comment placement (#8050) * test(cli): tighten dependency exposure assertions * fix(cli): use file URL for ESM import specifier in probe test (#8050) On Windows, join() produces backslash paths that break ESM import specifiers (\\x is parsed as a hex escape). Convert to a file URL via pathToFileURL so the generated vitest.mjs is valid on all platforms. * fix(cli): skip broken probe dependencies * test(cli): cover Windows review edge cases * fix(windows): platform-aware archive path, symlink error handling, probe normalization * fix(test): align archive-path assertions with mocked join and retry Windows cleanup sessionService.test.ts mocks path.join to always join with '/', so the readdir dir checks must match '/'-separated paths; using path.sep broke them on Windows where sep stays '\' under the automock. The channel webhook tests remove temp trees that Windows can still hold briefly (extension-store watchers), turning rmdir into ENOTEMPTY; give fsp.rm the standard maxRetries/retryDelay backoff. * docs(review): correct probe spawn failure comment * fix(review): address probe-runner and profiler review feedback Distinguish a present-but-hidden vitest from a missing one in findVitestBin, guard the remaining per-entry calls in exposeDependencies and disclose a partial dependency farm instead of failing silently, fold case when matching probe paths on Windows only, key the profiler's O_NOFOLLOW trade on the platform rather than the flag's presence (with a test pinning the symlink pre-check as the Windows guard), and normalise the platform-skip test idioms. --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com> Co-authored-by: Qwen Autofix <autofix@qwen-code.bot> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>	2026-08-01 05:16:58 +00:00
Qwen Code Bot	578baa1409	fix(cli): address review feedback on /summary custom path (#8116) ... - Fix CRLF false-negative in overwrite guard by normalizing line endings - Allow overwriting empty pre-created files (zero-length bypass) - Detect trailing separator on existing file and report clearly - Log chmod failures via debugLogger matching exportCommand convention - Add comment explaining mkdir mode asymmetry - Update docs: /summary usage table and custom-path welcome-back note - Add i18n key for trailing-separator error in all 9 locales - Add tests for CRLF, empty file, and trailing separator cases	2026-08-01 04:01:32 +00:00
易良	2bbd82f14b	fix(cli): complete image routing across entry points (#7206) ... * fix(cli): complete image routing across entry points resolve local image @ references consistently in TUI, ACP, and non-interactive entry points validate aliases and canonical targets against workspace, ignore, MIME, and file-identity boundaries route image turns through the full-turn agent or vision bridge while failing closed on unsupported/fallback paths integrate with tool-result vision bridge from #7484: @ image routing and tool-result image routing coexist with \0 suffix priority convention Refs #6988 * test(cli): complete session config fixture * fix(cli): close image routing review gaps * test(core): fix readManyFiles config mock cast * fix(cli): address image routing review feedback * fix(cli): address image routing review comments * test(core): cover validated file read safeguards * chore: trim review followup scope * fix(core): preserve custom fs text reads * fix(core): harden validated file snapshots * fix(core): resolve validated attachment review findings * fix(core): share file display label with errors * fix(core): unify snapshot cleanup through finally block (#7206) * fix(cli): accept repeated full-turn model override * test(cli): align drain prompt id expectation * fix(cli): continue directory loop on workspace/ignore check failure (#7206) The multi-root workspace directory loop used break on workspace-boundary and canonical-ignore check failures, preventing resolution from remaining directories. Switch to continue and defer the ignoredByReason push until after the loop so a path that resolves from a later directory does not leave a stale entry in the ignored-paths report. * fix(cli): cover validated image routing edge cases * test(core): type snapshot growth read mock * fix(core): bound validated text reads * test(cli): align large at-file read expectation * fix(readManyFiles): key FileReadCache by canonical path, add size pre-check * test(cli): deflake InstalledTab activation warning render sync (#7206) * test(cli): cover full-turn model override rejection * fix(core): keep unbounded validated text reads handle-bound * fix(core): per-file error boundary and scan budget for validated reads (#7206) * fix(core): surface size errors for validated files too large to snapshot (#7206) --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>	2026-08-01 02:40:44 +00:00
ChiGao	907c7dea70	fix(cli): stabilize thinking block height, replace transcript overlay with inline Ctrl+O toggle (#8077) ... * fix(cli): hide streaming thinking preview, rebind Ctrl+O to inline fullDetail toggle The streaming thinking block showed a 4-line preview that varied in height due to empty lines in the model's reasoning output, causing constant page reflow and flicker during generation. Changes: - ThinkBody now renders nothing when collapsed (both streaming and committed), keeping the block at a stable 1-line header height. - Ctrl+O now toggles inline fullDetail mode (like Claude Code): all thinking blocks, tool groups, and tool results expand/collapse in the main conversation view — no alternate-screen overlay. - Alt+T preserved as hidden shortcut (same toggle, not shown in UI). - MainContent passes fullDetail to HistoryItemDisplay via the existing ThoughtExpandedContext, so the toggle works in both VP and Static rendering paths. - Removed TranscriptView overlay rendering, transcriptItems memo, StreamingContext import, and EMPTY_HISTORY_ITEMS constant. - Removed dead code: tailVisualLines, grow-only height tracker, MAX_STREAMING_THINKING_VISUAL_LINES, openTranscript callback. * refactor(cli): remove orphaned transcript full-detail infrastructure (#8077) * fix(cli): update Ctrl+O help text and add thinking-expansion integration test (#8077) * fix(cli): update docs, help text, and remove transcript dead code (#8077) * fix(cli): strengthen Ctrl+O full-detail tests and refresh stale docs (#8077) * fix(cli): address review feedback on test isolation, dead i18n keys, and missing negative case (#8077) * fix(cli): add M1 mutation-killing assertion to Ctrl+O test (#8077) The existing test only asserted clearTerminal (a refreshStatic side effect) and never verified the setThoughtExpanded state flip. Under the vi.mock('ink') harness the mocked App never re-renders from a directly-called handler, so a behavioural allExpanded assertion is not possible. Add a structural guard on the handler source that fails when setThoughtExpanded is removed (mutation M1 verified). * fix(cli): strengthen Ctrl+O toggle assertion and document non-VP redraw (#8077) Tighten the structural M1 assertion from .toContain to a regex that matches the (prev) => !prev updater pattern, catching mutations like (prev) => true that the old check would miss. Document the non-VP scrollback redraw in keyboard-shortcuts.md per maintainer request. --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>	2026-08-01 02:39:53 +00:00
qwen-code-dev-bot	cae22cd411	Merge branch 'main' into feat/summary-export-path	2026-08-01 10:32:06 +08:00
zhangxy-zju	de022664dc	feat(skills): add disabled skill levels (#8057) ... * feat(skills): add disabled skill levels * fix(core): guard getDisabledSkillLevels for partial Config shims (#8057) * test(cli): cover disabledLevels safe/bare guard and daemon wiring (#8057) * fix(cli): guard daemon disabledLevels for safe mode consistency (#8057) * test(core): add regression guard for missing getDisabledSkillLevels (#8057) * test(core): pin partial config skill discovery (#8057) --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>	2026-08-01 02:22:47 +00:00
qwen-code-dev-bot	854704153d	fix(cli): filter SGR mouse escape sequences in early input capture (#8268) ... classifyEscapeSequence only filtered ESC[? (DEC private mode) and ESC[> (DA2) terminal responses, missing ESC[< (SGR mouse events). During startup, accumulated mouse bytes were replayed as user input, leaking raw escape sequences like ^[[<65;68;23M into the input box. Add 0x3c ('<') to the CSI third-byte check so SGR mouse sequences are classified as terminal responses and filtered out. Fixes #8267 Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>	2026-08-01 02:17:11 +00:00
jinye	a6aad41ace	refactor(core): thread the descriptor instead of forking text-read helpers (#7967) ... * fix(serve): allow bounded reads of large text files * fix(serve): bound large-text reads by scan cost, not by which knob was set Follow-up to the bounded large-text read path. Three changes: Gate on any explicit window argument, not on `limit`. Gating on `limit` had the cost model backwards in both directions: `{ line: 900_000_000, limit: 20 }` was admitted despite walking the whole file, while `{ maxBytes: 4096 }` — satisfiable from the first 4 KiB — was refused. A read with no window argument at all still fails, since a caller that believes it holds the whole file may write it back truncated. Add MAX_TEXT_SCAN_BYTES (8 MiB). MAX_READ_BYTES caps what a read returns; nothing capped what it cost. Line offsets are resolved by scanning from byte 0, so a query param could turn into an uninterruptible multi-second scan of an arbitrarily large file — and on Windows hold a read handle for that span, blocking renames and deletes. Past the budget the read is refused with `file_too_large` pointing at readBytes, which reaches any offset in O(1). Tolerate appends on streamed windows. Requiring whole-file size/mtime stability after reading a prefix rejected reads whose returned bytes were still valid, and the case it rejected — tailing a live log — is the one this path exists for. Streamed windows now assert inode identity plus "did not shrink"; truncation and replacement are still rejected. Also: non-UTF-8 large text now returns `binary_file` rather than `file_too_large`, so a client retrying on 413 with a smaller window can't loop forever; and `readFileWithLineAndLimit` throws instead of silently ignoring a caller-supplied `fileHandle` on the by-path fallback. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(core): thread the descriptor instead of forking text-read helpers PR #7947 pinned large-text reads to one inode by threading a caller-owned FileHandle into readTextRange as an optional field, plus a second field, forceStreaming, to suppress the buffering fast path. Two optional fields produced four combinations: one meaningful, one used by a single test, one unreachable, and — in readFileWithLineAndLimit — one that silently fell through to a by-path read, defeating the reason the caller opened a handle. Unify the two encoding detectors. detectFileEncoding now takes a path or a borrowed handle, so detectFileHandleEncoding is deleted along with the message discrepancy between them: an encoding iconv-lite cannot load now raises LargeNonUtf8TextError naming that encoding rather than deferring to the decoder's generic invalid-utf8 variant. Both still refuse the file, and the Serve boundary maps both to binary_file. Split the reader into readTextRange (path) and readTextRangeFromHandle (always streams, both byte bounds required). The unreachable combination and its untested readFileHandleBuffer are gone, and with no fileHandle parameter left for readFileWithLineAndLimit to ignore, the RangeError guarding that fallthrough is deleted too — the trap can no longer be expressed. CoreReadTextFileHandleRequest drops its required stats field. Nothing downstream read it, and because the ACP request type it extends permits extra properties, TypeScript accepted the dead argument silently. readFileHandleChunks becomes chunksFromHandle(fh, from) — the one seam byte-cursor text paging needs. No observable change at the Serve boundary: its 222 tests pass unmodified. Two fileSystemService tests were deleted rather than repaired; they asserted the arguments readFileWithLineAndLimit received, which is nothing once the handle path stops calling it. Their coverage lives in read-text-range.test.ts against real files and in workspace-file-system.test.ts at the real boundary. 258 production lines in core, net -71 overall. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(core): make CoreReadTextFileHandleRequest standalone Self-audit follow-up to f55c867a. Two fields survived the reshape that the handle path never reads: - `stats` was documented as required ("must pass the Stats captured from that handle") and nothing downstream read it. The handle path always streams, so it never needs a size to choose a strategy, and the encoding probe does its own fstat. - `path` became dead once readTextRangeFromHandle replaced the path-plus-handle call. Errors are labelled with the path by the Serve boundary that owns it. Neither was caught by the compiler: the ACP ReadTextFileRequest the type derived from permits extra properties, so the CLI kept passing both silently. That is the argument for declaring the type standalone rather than Omit-ing four of six inherited fields and quietly re-admitting the rest. Also record the second behaviour delta of the detector merge in the design doc: detectFileEncoding catches I/O errors and falls back to 'utf-8', where detectFileHandleEncoding let them propagate. The failure is not lost — a handle that fails the 8 KiB probe fails the streaming read immediately after — but a different call now reports it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(serve): address review feedback on decoded-output cap, lineEnding, line validation, and docs (#7967) * fix(serve): scope PR to descriptor threading, revert #7947 behavior reversals (#7967) Maintainer review adjudicated the pending design-reversal Critical: the descriptor-threading refactor is sound and boundary-neutral, but this PR also carried two undisclosed reversals of #7947 that must not ride in on a refactor. Revert both, keeping only the refactor and the disclosed non-UTF-8 mapping: - Restore didFileVersionChange (size + mtime + ctime equality before and after the streamed read) in place of assertDidNotShrink. The size-only guard let a same-size in-place overwrite return torn content and a concurrent append run past the snapshot it opened against; the equality check rejects both with hash_mismatch. Main's three snapshot tests are restored unchanged. - Restore #7947's limit-only large-text admission and drop MAX_TEXT_SCAN_BYTES / maxScanBytes / TextScanBudgetExceededError. The broadened wantsWindow admission and scan budget are a real contract change that belongs in its own PR against #7947, not here. Kept: the descriptor-threading refactor (one detectFileEncoding, two entry points, readTextFileFromHandle/readTextRangeFromHandle, no mode flags) and the disclosed mapping of large non-UTF-8 windows to binary_file rather than file_too_large, which the maintainer explicitly approved. * test(core): port deep-read and encoding-probe tests to the handle path (#7967) * test(core): pin handle read-to-EOF and unlimited-limit contracts (#7967) * docs(serve): disclose handle-read behaviour deltas and add boundary tests (#7967) Address maintainer re-verification (R2). The refactor's code is unchanged; this makes the documentation describe the actual diff and pins the remaining untested handle-path contracts. - Rewrite the design doc to match what the diff does against current main (one detector, two entry points, chunksFromHandle, standalone CoreReadTextFileHandleRequest) instead of the superseded intermediate state, and add a Behaviour deltas section disclosing the read-to-EOF chunk reader, the fresh-buffer-per-chunk change, the limit: Infinity admission, and the binary_file mapping for undecodable large text. - State the handle-layer omitted/infinite limit contract in qwen-serve-protocol.md, reconciled with the unchanged GET /file admission. - Add handle-bound abort tests (pre-aborted and mid-stream) to read-text-range.test.ts and an HTTP-layer test asserting a large non-UTF-8 file requested with a finite limit returns 422 binary_file. * test(core): add positive handle-read test for Infinity and omitted limit (#7967) * fix(core): satisfy ReadTextRangeResult type in handle-read test mock (#7967) * fix(core): bound handle reads to stat size and reject Infinity limit (#7967) * fix(core): satisfy ReadTextRangeResult type in fileSystemService test mocks (#7967) * refactor(core): reuse detectLineEndingFromContent in whole-file fast path (#7967) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): export ReadTextRangeResult type and cover CRLF detection (#7967) * test(core): cover whole-file lineEnding and truncatedByBytes shape (#7967) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: qwen-autofix[bot] <qwen-autofix[bot]@users.noreply.github.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-08-01 02:07:37 +00:00
易良	ddd672ced3	fix(channel): recover ACP bridge after wake (#8211) ... * fix(channel): recover ACP bridge after wake * test(channel): cover bridge recovery gates * fix(channel): close the 5-10min wake misclassification window and dedupe recovery - Lower the default event-loop suspend threshold to 5 minutes so any low-CPU sleep gap is filtered before it reaches the AcpBridge stall-kill threshold; pin the invariant with a cross-package test. - Extract the duplicated startSingle/startAll bridge recovery into a shared createBridgeRecovery helper. * test(channel): use observe in observedContacts recovery-gate mock The mock provided a record method but ChannelBaseOptions.observedContacts expects observe, so the slow-contact-recording path was never exercised. * fix(channel): harden wake recovery * test(channel): align wake recovery threshold checks * fix(channel): clear recovery-aborted loop runs and strengthen gate tests (#8211) * fix(channel): scope recovery suppression to prompts actually aborted (#8211) * fix(channel): carry low-CPU gap across ticks and address review feedback (#8211) --------- Co-authored-by: qwen-code-bot <qwen-code-bot@users.noreply.github.com>	2026-08-01 02:04:52 +00:00
Qwen Code Bot	610984025d	fix(cli): harden /summary symlink chain walk, file mode, and overwrite guard (#8116)	2026-08-01 01:27:15 +00:00
jinye	1d1bc49473	fix(external-context): harden MCP dependencies (#8206) ... * fix(external-context): harden MCP dependencies Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(external-context): drop no-op MCP override, raise SDK floors (#8206) Remove the `@modelcontextprotocol/sdk@^1.30.0` -> `@hono/node-server` override: resolution with and without it is byte-identical because the SDK's own `^1.19.9 || ^2.0.5` range already selects 2.0.12, and mobile-mcp's exact SDK pin (not this entry) keeps its nested Hono 1. Raise the `@modelcontextprotocol/sdk` floor to `^1.30.0` on the packages that actually instantiate the Hono-backed transport (cli, core, sdk-typescript, vscode-ide-companion) so the hardened constraint lives in the manifests where the code runs, matching the PR's stated intent. --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-08-01 01:04:16 +00:00
Qwen Code Autofix	ddee5164a9	fix(cli): re-validate appended default filename for symlink escape in /summary (#8116)	2026-07-31 19:36:55 +00:00
qwen-code-dev-bot	bc463835cf	fix(cli): guard /summary overwrite and expand tilde in path (#8116)	2026-07-31 18:30:06 +00:00
Shaojin Wen	77d8a27eda	feat(daemon): raise default max sessions from 20 to 32 (#8235) ... * feat(daemon): raise default max sessions from 20 to 32 * fix(daemon): update test assertion and docs for new default max sessions (32) * fix(daemon): sync default max sessions Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-07-31 17:47:51 +00:00
qwen-code-ci-bot	f6467bfd38	fix(cli): reject broken symlinks escaping project root in /summary (#8116)	2026-07-31 17:27:57 +00:00
qwen-code-dev-bot	afac1c1de7	Merge branch 'main' into feat/summary-export-path	2026-07-31 23:31:32 +08:00
qqqys	7189a68334	fix(serve): isolate managed memory by selected workspace (#8056) ... * fix(serve): isolate managed memory by workspace * feat(serve): add memory project scope option * test(serve): fix clean build type assertion * test(serve): cover untrusted workspace memory tasks * test(memory): cover remaining workspace paths * test(serve): cover unavailable memory lanes * test(memory): isolate default scope * fix(serve): address review feedback on workspace memory isolation (#8056) - Create secondary ACP mounts on demand for dynamically-registered workspaces so the qualified memory routes work beyond boot-time runtimes (rename getWorkspaceRememberLane → ensureWorkspaceRememberLane) - Remove the symlink-alias machinery from getAutoMemoryRoot: it had no producer, relaxed a documented invariant, and could throw on the per-turn hot path; workspace mode now uses the same plain path.join as git-root mode - Move memoryProjectScope validation into the pre-listen block - Use the shared sendWorkspaceRuntimeUnavailable helper in server.ts - Revert an unrelated test mock change; fix misleading 'compatibility fallback' wording in ServeOptions - Add workspace_qualified_memory capability tag, docs for the new flag and env var * docs(serve): add workspace_qualified_memory to conditional features table (#8056) * fix(serve): address follow-up review feedback on workspace memory isolation (#8056) * fix(serve): address follow-up review feedback on workspace memory isolation (#8056) Extract MEMORY_PROJECT_SCOPES const and MemoryProjectScope type in core so yargs choices, ServeOptions, ServeArgs, and the runQwenServe guard share one source of truth (reduces drift risk from five to three edit points; the fast-path guard keeps inline comparisons because an import boundary test forbids core imports on the lightweight startup path). Document memory-project-scope caveats in the user-facing docs: daemon vs standalone CLI split-brain, sanitizeCwd punctuation collisions, and flag vs env normalization differences. Add the per-lane MAX_PENDING resource note to the developer configuration reference. * fix(cli): add missing sessionRuntimeBaseDir to late-add workspace test (#8056) * test(serve): cover untrusted forget/dream and no-lane memory poll (#8056) * fix(core): extract MEMORY_PROJECT_SCOPES into zero-import leaf module (#8056) Importing the constant as a value from the core barrel turned it into a real static edge that pulled the entire 5.6 MB barrel into the serve pre-listen bundle closure, breaking the fast-path gate. Move MEMORY_PROJECT_SCOPES and MemoryProjectScope into a new memory/scopes.ts with no imports of its own, re-export from paths.ts so the barrel surface is unchanged, add a ./memoryScopes subpath export, and switch run-qwen-serve.ts to the narrow import. Also derive the unknown-scope guard in resolveWorkspaceProjectScope() from the constant instead of hardcoding 'git-root'. * test(cli): pin non-allocating contract in workspace memory poll test (#8056) --------- Co-authored-by: qwen-code-dev-bot <269191875+qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Autofix <qwen-autofix@users.noreply.github.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>	2026-07-31 15:06:38 +00:00
Kagura	2e04bde02f	fix(ui): suppress stale mouse events during dialog transitions (#8089) ... When a dialog opens, ScrollableList's hasFocus prop transitions to false, but the useEffect cleanup (which unsubscribes from mouse events) runs after paint. During this micro-window, any incoming wheel event still reaches the handler and scrolls the viewport behind the dialog. Add an enabledRef that tracks the computed enabled state synchronously. The mouseCallback checks the ref before forwarding, eliminating events that arrive between re-render and effect cleanup. Fixes #8086	2026-07-31 13:49:02 +00:00