chore(deps): clear remaining CVE audit findings (diff DoS, uuid bounds check) - #10985
chore(deps): clear remaining CVE audit findings (diff DoS, uuid bounds check)#10985now-ing wants to merge 5 commits into
Conversation
…s check) - diff ^7.0.0 -> ^8.0.2 in packages/core and packages/cli (GHSA-73rr-hh4g-fpgx: DoS in parsePatch/applyPatch, patched >= 8.0.3) - uuid ^9.0.1 -> ^11.1.1 in packages/core (GHSA-w5hq-g745-h8pq: missing buffer bounds check in v3/v5/v6, patched >= 11.1.1) - regenerate package-lock.json and NOTICES.txt fast-uri and qs were already cleared on main by QwenLM#10862. After this change 'npm audit --omit=dev' reports zero vulnerabilities. Fixes QwenLM#10850 Signed-off-by: mac <bishopapril850965@yahoo.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
@now-ing the security half of this is right and I could verify it, but the diff bump breaks the build — six checks are red on f6f0204e9e3a5392684179a966d1bb92f412c6f8, all from the same tsc failure.
What checks out
Dependency CVE audit→ success. The claim that the audit goes to zero holds.- I confirmed both advisories against GitHub's advisory API rather than taking the description's word: GHSA-73rr-hh4g-fpgx affects
diff >= 6.0.0, < 8.0.3and your lockfile resolves8.0.4; GHSA-w5hq-g745-h8pq affectsuuid < 11.1.1,12.0.0–12.0.1,13.0.0–13.0.1and your lockfile resolves11.1.1. Both genuinely patched. - Good catch on
mermaid/node_modules/uuid@14.0.0. #10850 lists it as vulnerable and it isn't —14.0.0falls outside every affected range. Worth saying so on the issue, since that's the one part of it that's still open. - The
uuidbump looks safe on its own:packages/core/src/utils/openaiLogger.tsis the only consumer andimport { v4 as uuidv4 }is unchanged from v9 through v11.
Blocker: diff 7 → 8 removes the types this repo builds against
Lint & Static, Test (ubuntu-latest, Node 22.x), Integration Tests (no-AK, No Sandbox), OpenTUI no-flicker gate, TUI parity snapshots (ink vs opentui) and Real daemon E2E / Java 11 all fail identically:
src/services/fileHistoryService.ts(19,43): error TS2305: Module '"diff"' has no exported member 'Hunk'.
src/tools/diffOptions.ts(10,41): error TS2694: Namespace '.../diff/libesm/index' has no exported member 'PatchOptions'.
src/tools/diffOptions.ts(62,9): error TS2694: ... has no exported member 'ParsedDiff'.
src/tools/diffOptions.ts(73,7): error TS18048: 'result' is possibly 'undefined'.
src/tools/diffOptions.ts(46,16): error TS2345: Argument of type 'undefined' is not assignable to parameter of type 'string'.
src/utils/gitDiff.ts(12,15): error TS2305: Module '"diff"' has no exported member 'Hunk'.
src/utils/gitDiff.test.ts(489,49): error TS7006: Parameter 'l' implicitly has an 'any' type. (×8)
Error: Command failed: tsc --build
The cause is upstream, not yours: jsdiff 8.0.0 rewrote the source in TypeScript and now ships its own type definitions (kpdecker/jsdiff#591). Their release notes say the switch from the DefinitelyTyped types "removes many exported types previously used for options arguments" and splits abortable calls — ones with a timeout or maxEditLength, which may return undefined — from non-abortable ones. That is precisely Hunk/PatchOptions/ParsedDiff vanishing and structuredPatch becoming possibly-undefined. So this is a type migration, not a redesign; the runtime functions you use are all still there.
Three things that will bite you while fixing it:
@types/diffis still declared.packages/core/package.json:131andpackages/cli/package.json:109both carry"@types/diff": "^7.0.2", and this PR doesn't touch them. Upstream explicitly says to drop that dependency when moving to v8 — leaving it means two competing type surfaces for the same module.- The blast radius is wider than the log.
packages/corefails first, sopackages/clinever gets typechecked. It has three moreimport type { Hunk } from 'diff'sites —src/ui/hooks/useDiffData.ts,src/ui/components/DiffDialog.tsx,src/ui/components/DiffDialog.test.tsx. Andpackages/core/src/utils/gitDiff.ts:17re-exportsGitDiffHunk = Hunk, so this sits on core's exported surface too. Fixing only the four files in the log won't get you green. ^8.0.2is one patch below the fix. The advisory's first patched version is8.0.3. Your lockfile pins8.0.4so the audit passes, but declaring^8.0.3would stop a fresh resolve from landing back on a vulnerable8.0.2.
Why your verification missed this
The description says npm run build exits 0 — CI says it doesn't, in the packages/core workspace, via scripts/prepare.js. The two test files you ran can't catch it either: vitest transpiles without typechecking, so a types-only break is invisible to it however many tests pass. npm run typecheck, or a full npm run build, is the check that actually pins a major-version dependency bump.
Template
The body is missing ## Risk & Scope, ## Linked Issues, and the 中文说明 <details> block from .github/pull_request_template.md. Risk & Scope is the one that matters here — "Breaking changes / migration notes" is exactly the box a two-major bump needs to fill in, and it's where the diff v8 type removals belonged.
Direction
Not asking you to abandon this — the premise is sound and the audit gate agrees. The uuid bump is clean and could land as-is. The diff bump needs the type migration carried along with it: either do it here, or split uuid out and take diff separately so the security fix isn't held hostage to a seven-file type cleanup.
中文说明
@now-ing 这个 PR 的安全部分是对的，我也验证过了，但 diff 的升级把构建搞坏了 —— 在 f6f0204e9e3a5392684179a966d1bb92f412c6f8 上有 6 个 check 是红的，全部源于同一个 tsc 失败。
已验证没问题的部分
Dependency CVE audit→ success，"audit 归零" 的说法成立。- 我没有直接采信 PR 描述，而是对着 GitHub advisory API 核了两个公告：GHSA-73rr-hh4g-fpgx 影响
diff >= 6.0.0, < 8.0.3，你的 lockfile 解析到8.0.4；GHSA-w5hq-g745-h8pq 影响uuid < 11.1.1、12.0.0–12.0.1、13.0.0–13.0.1，你的 lockfile 解析到11.1.1。两个都确实修好了。 mermaid/node_modules/uuid@14.0.0这个判断很准。#10850 把它列为受影响，但它并不在任何受影响区间内。建议去 issue 下面说明一下 —— 那是 #10850 里唯一还站得住的开放项。uuid单独看是安全的：唯一使用方是packages/core/src/utils/openaiLogger.ts，import { v4 as uuidv4 }从 v9 到 v11 没有变化。
阻塞项：diff 7 → 8 移除了本仓库依赖的类型
Lint & Static、Test (ubuntu-latest, Node 22.x)、Integration Tests (no-AK, No Sandbox)、OpenTUI no-flicker gate、TUI parity snapshots (ink vs opentui)、Real daemon E2E / Java 11 六个 check 报的是同一批错误（见上方英文部分的完整日志）：Hunk、PatchOptions、ParsedDiff 不再从 diff 导出，structuredPatch 的返回值变成可能为 undefined，createPatch 的 header 参数不再接受 undefined，最终 tsc --build 失败、packages/core 构建中断。
根因在上游而不是你的写法：jsdiff 8.0.0 用 TypeScript 重写了源码并自带类型定义（kpdecker/jsdiff#591）。其 release notes 明确说，从 DefinitelyTyped 类型切换过来 "移除了许多此前用于 options 参数的导出类型"，并且把可中断调用（带 timeout 或 maxEditLength、结果可能为 undefined）与不可中断调用的类型拆开了。所以这是一次类型迁移，不是重新设计 —— 你用到的运行时函数都还在。
修的时候有三点会绊到你：
@types/diff还留着。packages/core/package.json:131和packages/cli/package.json:109都还写着"@types/diff": "^7.0.2"，本 PR 没有动它们。上游明确说升级到 v8 时应当移除这个依赖 —— 留着就等于同一个模块有两套互相竞争的类型。- 影响面比日志显示的更大。
packages/core先失败，所以packages/cli根本没走到类型检查。它还有三处import type { Hunk } from 'diff'：src/ui/hooks/useDiffData.ts、src/ui/components/DiffDialog.tsx、src/ui/components/DiffDialog.test.tsx。另外packages/core/src/utils/gitDiff.ts:17把GitDiffHunk = Hunk再导出，所以它也在 core 的对外类型面上。只改日志里那四个文件是不够的。 ^8.0.2比修复版本低一个 patch。 该公告的首个修复版本是8.0.3。你的 lockfile 钉在8.0.4，所以 audit 能过；但声明成^8.0.3可以避免将来重新解析时又落回有漏洞的8.0.2。
为什么你的自测没发现
PR 描述里写 npm run build 退出码为 0，但 CI 显示它在 packages/core workspace 里经 scripts/prepare.js 失败了。你跑的那两个测试文件也抓不到这个问题：vitest 只转译不做类型检查，所以纯类型层面的破坏无论多少测试通过都看不出来。真正能钉住"大版本依赖升级"的检查是 npm run typecheck，或者完整的 npm run build。
模板
PR 正文缺少 .github/pull_request_template.md 里的 ## Risk & Scope、## Linked Issues 以及 中文说明 的 <details> 区块。其中 Risk & Scope 最关键 —— "Breaking changes / migration notes" 正是跨两个大版本升级该填的栏目，diff v8 的类型移除本应写在那里。
方向
不是要你放弃这个 PR —— 出发点是成立的，audit gate 也认可。uuid 那部分很干净，可以单独合。diff 这部分需要把类型迁移一起做完：要么在本 PR 里补上，要么把 uuid 拆出去先合、diff 单独走，免得安全修复被一次七文件的类型清理拖住。
— Qwen Code · qwen3.8-max-2026-09-02
diff@8 renamed/reshaped its type surface; the runtime API used here (diffLines/structuredPatch) is unchanged: - named type imports use 'StructuredPatchHunk as Hunk' aliases so downstream references stay untouched - Diff.ParsedDiff -> Diff.StructuredPatch - DEFAULT_DIFF_OPTIONS narrowed to Pick<StructuredPatchOptionsNonabortable, 'context' | 'ignoreWhitespace'> so it satisfies both structuredPatch and createPatch overloads Verified: core tsc --noEmit clean; 194 tests across fileHistoryService/gitDiff/diffOptions pass; cli tsc adds no new errors vs a clean tree. Signed-off-by: mac <bishopapril850965@yahoo.com>
Local verification report — PR #10985Verdict: blocking. The dependency bump itself is sound — the two advisories really are gone and the runtime behaviour of 1. EnvironmentTwo git worktrees off the same clone: 2. The blocking problem
18 errors, in 4 files, all rooted in one thing: jsdiff 8.0.0 rewrote itself in TypeScript and now ships its own type definitions, and those definitions do not export the three names this repo imports.
From the 8.0.0 release notes: "the The reason Downstream of the three missing names, overload resolution then falls through to the abortable overloads, which is where Consequences on this PR right now: Negative control: same worktree, same The fix (validated — build goes green, no behaviour change)--- a/packages/core/src/tools/diffOptions.ts
+++ b/packages/core/src/tools/diffOptions.ts
-export const DEFAULT_DIFF_OPTIONS: Diff.PatchOptions = {
+export const DEFAULT_DIFF_OPTIONS: Diff.CreatePatchOptionsNonabortable &
+ Diff.StructuredPatchOptionsNonabortable = {
context: 3,
ignoreWhitespace: true,
};
@@
-): Diff.ParsedDiff {
+): Diff.StructuredPatch {
@@
- const getStats = (patch: Diff.ParsedDiff) => {
+ const getStats = (patch: Diff.StructuredPatch) => {
@@
- patch.hunks.forEach((hunk: Diff.Hunk) => {
+ patch.hunks.forEach((hunk: Diff.StructuredPatchHunk) => {
--- a/packages/core/src/services/fileHistoryService.ts
-import { diffLines, structuredPatch, type Hunk } from 'diff';
+import {
+ diffLines,
+ structuredPatch,
+ type StructuredPatchHunk as Hunk,
+} from 'diff';
--- a/packages/core/src/utils/gitDiff.ts
--- a/packages/cli/src/ui/components/DiffDialog.tsx
--- a/packages/cli/src/ui/components/DiffDialog.test.tsx
--- a/packages/cli/src/ui/hooks/useDiffData.ts
-import type { Hunk } from 'diff';
+import type { StructuredPatchHunk as Hunk } from 'diff';The
While you are there, 3. What the PR does deliver — all of it reproduces
4. Runtime behaviour:
|
| 仓库里引用的 | jsdiff ≤ 7（经 @types/diff@7） |
jsdiff 8.0.4（自带类型） |
|---|---|---|
Diff.PatchOptions |
✅ | ❌ 换成 CreatePatchOptionsNonabortable / StructuredPatchOptionsNonabortable |
Diff.ParsedDiff |
✅ | ❌ 换成 StructuredPatch |
Hunk / Diff.Hunk |
✅ | ❌ 换成 StructuredPatchHunk |
8.0.0 的 release notes 原话："the diff package on npm now includes its own TypeScript type definitions. Users who previously used the @types/diff npm package from DefinitelyTyped should remove that dependency when upgrading JsDiff to v8. Note that the transition … removes many exported types previously used for options arguments."
为什么 @types/diff@7.0.2（仍然声明在 packages/cli 和 packages/core 的 devDependencies 里，这个 PR 没动它）兜不住：TypeScript 在每一层目录都先看 node_modules/<name> 再看 node_modules/@types/<name>，所以 packages/core/node_modules/diff/libesm/index.d.ts 胜出，@types/diff 根本不会被查到。报错信息里点名的正是这个路径。
三个名字缺失之后，重载解析会退到 abortable 那几个重载上，这就是 diffOptions.ts:46,47,50,73 那几个 TS2345/TS2322/TS18048 的来历；gitDiff.ts 里 GitDiffHunk 的再导出因此崩掉，又带出 gitDiff.test.ts 里 8 个 TS7006 implicitly has an 'any' type。
当前后果：Lint & Static、Test (ubuntu-latest)、Integration Tests (no-AK)、OpenTUI no-flicker gate、TUI parity snapshots、Real daemon E2E / Java 11、web-shell E2E Smoke 全红，无一例外都挂在同一处 tsc 失败上 —— 多数在 Install dependencies，OpenTUI 那道门在 Build cli。Dependency CVE audit 是绿的，因为它根本不构建。
反向对照：同一个 worktree、同一套 node_modules 安装路径、同一套工具链，把 PR head 换成基线提交 60161cb64a → npm ci + npm run build 退出码 0，0 个 TypeScript 错误。所以这是本 PR 引入的，不是 main 上原有的。
修复（已验证：构建转绿，且不改变行为）
具体 diff 见上方英文部分。as Hunk 别名让 GitDiffHunk、GitDiffFileHunks 以及下游所有签名保持不变。StructuredPatchHunk 和旧的 Hunk 描述的是同一个对象 —— 我对比过运行期的键：两边都正好是 oldStart / oldLines / newStart / newLines / lines。
顺手建议：@types/diff: ^7.0.2 应该从 packages/cli 和 packages/core 的 devDependencies 里删掉（上游明确这么说，而且它现在已经不可达）。根 package.json 里的 @types/uuid: ^10.0.0 处境相同 —— uuid 11 自带类型，而全仓库唯一的 uuid 使用方就是 openaiLogger.ts。
3. PR 承诺的部分 —— 全部复现成立
见上方英文部分图 2。
- advisory 清干净了。 基线树：2 条（
difflow GHSA-73rr-hh4g-fpgx，uuidmoderate GHSA-w5hq-g745-h8pq）。PR 树在真实npm ci之后：found 0 vulnerabilities。在装好的树上和--package-lock-only两种方式下都确认过。 - 关于
mermaid的说法成立。node_modules/mermaid/node_modules/uuid@14.0.0仍在 PR 树里，而 audit 依然干净，说明 14.0.0 确实不在 GHSA-w5hq-g745-h8pq 当前的受影响区间内。值得一提是因为 ci: Dependency CVE audit fails repo-wide on new fast-uri/qs/uuid advisories (main lockfile) #10850 当时假设的正好相反（"fix requires uuid@14.0.2 vianpm audit fix --force"）—— 现在的 advisory 数据已经不是这么说了。 node scripts/check-lockfile.js→Lockfile check passed.- lockfile 是稳定的。 在 PR head 上跑
npm install --package-lock-only逐字节复现出提交进来的 lockfile，所以 esbuild 各平台条目上那批"peer": true变动是 npm 的一次性归一化，不是漂移。 npm run generate:notices --workspace=qwen-code-vscode-ide-companion之后NOTICES.txt无改动 —— CI 里那道对比门会过。npm run bundle退出码 0，node dist/cli.js --version打印0.23.0；产出的 chunk 里能搜到 jsdiff 8 的 patch 代码和 uuid 11 的越界检查字符串，说明新版本确实进了发布产物。
4. 运行期行为：diff 7 → 8 对本仓库是空操作
这次跨了两个大版本，而这个库垫在 CLI 渲染的每一个 diff 底下，所以我没有想当然，而是实测了行为（见上方英文部分图 3）。
- 库层面。 一个 harness 把仓库自己的
createPatchSmart、structuredPatchSmart、getDiffStat（含ignoreWhitespace: true→false的回退分支）在两个版本上并排重放，语料是 300 个真实packages/core文件 × 10 种现实变异（中段改行、插入、删除、重新缩进、行尾空白、CRLF、追加尾行、去掉 EOF 换行、空行膨胀、恒等）：3000 个用例，patch 不一致 0，getDiffStat不一致 0，hunk 形状不一致 0，抛异常一致性偏差 0。 hunk 的键和StructuredPatch顶层键完全相同。 - 反向对照。 同一个 harness 把旧臂换成 jsdiff 5.2.0：600 个用例里 437 个 patch 不一致、202 个 stat 不一致。说明这个 harness 是能测出 jsdiff 真实行为变化的，上面的 0/3000 是结论而不是空跑。
- 构建产物层面。 把同样的比较抬高一层 ——
main构建出的packages/core/dist（jsdiff 7.0.0）对 PR 构建出的 dist（8.0.4）—— 900 个用例，0 处不一致。 - 穿过构建产物的端到端：在 8.0.4 下，
createPatchSmart对一次真实编辑、getDiffStat、以及对一个现建 git 仓库跑fetchGitDiffHunks，返回的 patch 和 hunk 都符合预期。 - uuid。 每臂 20 万个
v4()：0 个畸形，全部互不相同，格式一致。仓库里唯一的调用点是openaiLogger.ts里的uuidv4().slice(0, 8)，从不传buf，而buf正是 GHSA-w5hq-g745-h8pq 的全部内容 —— 所以 uuid 这一升在运行期完全没有暴露面。（不过 advisory 的修复是实打实的：传一个偏短的buf，9.0.1 会静默越界写，11.1.1 会抛UUID byte range 0:15 is out of buffer bounds。） - 测试。
packages/core：fileHistoryService.test.ts+openaiLogger.test.ts→ 91 通过；diffOptions.test.ts+gitDiff.test.ts→ 143 通过。packages/cli：DiffDialog.test.tsx→ 2 通过。全部跑在 jsdiff 8.0.4 / uuid 11.1.1 上。
5. 非阻塞项
N1 —— diff: ^8.0.2 仍然允许装到有漏洞的版本。 GHSA-73rr-hh4g-fpgx 的受影响区间是 6.0.0 - 8.0.2，修复落在 8.0.3。一棵满足本 PR 所声明区间的树 audit 依然是脏的：
$ cat package.json -> "diff": "^8.0.2"
$ npm install diff@8.0.2 && npm audit --omit=dev
diff 6.0.0 - 8.0.2
jsdiff has a Denial of Service vulnerability in parsePatch and applyPatch
1 low severity vulnerability
提交的 lockfile 钉的是 8.0.4，所以 CI 没问题；但 packages/core 和 packages/cli 是要发到 npm 的，下游按区间解析而不是按我们的 lockfile。写成 ^8.0.3（或者跟 NOTICES.txt 记录一致的 ^8.0.4）才把意图编码进去。
N2 —— 描述和实际树对不上。 "Reviewer Test Plan / How to verify" 第 3 步说 npm run build 退出 0，实际退出 1。第 2 步那 91 个测试确实过 —— 但它们在坏掉的 head 上也照样过，因为 vitest 只转译不做类型检查，这很可能就是这个 PR 看起来"已验证"的由来。另外 "Tested on: 🍏 macOS ✅" 对这样一个与平台完全无关的失败来说不可能成立。修好之后建议把正文一并订正。
N3 —— "zero vulnerabilities" 只是 prod 范围。 对 PR 的 lockfile 跑不带 --omit=dev 的 npm audit，仍有 10 条（1 low、3 moderate、4 high、2 critical），全部来自只在开发期用的依赖 —— vitest / @vitest/coverage-v8、lodash、postcss-selector-parser 等。这在 --omit=dev --audit-level=high 这道门的范围之外；而且本 PR 清掉的两条（low + moderate）本来就没有让那道门变红。PR 正文对"这是为收紧 --audit-level 提前还债"这点是诚实的，只是摘要那句话读起来比实际范围大。
N4 —— 只是观察，不需要动作。 diff 和 uuid 不再被提升到根 node_modules；lockfile 现在带的是 packages/cli/node_modules/diff@8.0.4、packages/core/node_modules/diff@8.0.4 和 packages/core/node_modules/uuid@11.1.1，diff 从一份变成两份。重新解析下它是稳定的，代价是几百 KB，所以知道就行 —— 顺带一提，第 2 节那条报错信息指向嵌套路径也正是因为它。
6. 未覆盖
- 只在 Linux 上跑。没有 macOS / Windows —— 不过失败发生在
tsc，平台在这里不是变量。 - 没有跑完整的
packages/cli或packages/web-shell套件；我跑的是碰到这两个被升级包的测试，加上仓库自己的 build、bundle、lockfile 和 NOTICES 各道门。 - 建议的修复在本地验证过（构建 0 错误、bundle 0、236 个测试通过），但没有推到任何地方 —— 采纳或另写都由你定。
🤖 Generated with Claude Code — Claude Opus 5 (1M context)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-1 leftover @types/diff@^7.0.2 declaration in packages/core and packages/cli after the diff v8 bump — already raised in review 5108848878, still standing at HEAD
- diff declared range ^8.0.2, one patch below the advisory's first patched version 8.0.3 — already raised in review 5108848878, still standing at HEAD
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): 91 tests pass — this review observed 28140, 23180, 504, 1919, 298, 1772, 5832, 94 passed.
— qwen3.8-max via Qwen Code /review (v0.23.0)
jsdiff v8 ships its own type definitions; the leftover @types/diff@7 declarations in core and cli declared a second, competing type surface that broke CI typechecking even after fddf582 migrated the imports. - remove @types/diff from packages/core and packages/cli devDependencies (npm install refreshed the lockfile; the root node_modules/@types/diff entry is gone) - raise the diff floor from ^8.0.2 to ^8.0.3 (first patched release per the DoS advisory) in both manifests; lockfile resolves 8.0.4 Verified: - npm run typecheck (all workspaces + integration-tests): exit 0 - core tsc --noEmit / cli tsc --noEmit: both clean - npm run build: exit 0 - 194 tests across gitDiff/diffOptions/fileHistoryService pass - cli DiffDialog.test.tsx passes - npm audit --omit=dev --audit-level=high: 0 vulnerabilities Signed-off-by: mac <bishopapril850965@yahoo.com>
|
Punch list done at 9e15d3d:
Verification (this time the CI-equivalent commands): root |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test (integration / E2E layer) — CI's verification matrix has not run at this head: the only checks present are review-automation jobs (review-pr pending, precheck-pr, authorize, assign, label, Remind on force-push), with no Lint & Static, Test, Integration Tests, TUI parity snapshots, OpenTUI no-flicker gate, web-shell E2E Smoke or Real daemon E2E job in existence at commit 9e15d3d. Those suites did not run locally either, so the end-to-end layer is uncovered. The build itself, and every unit suite that loads a bumped package, WERE executed locally and are green.
Test Plan (not a blocker): 91 tests pass — this review observed 28140, 23180, 1919, 298, 1772, 504, 5832, 94 passed.
— qwen3.8-max via Qwen Code /review (v0.23.0)
|
@qwen-code /resolve |
Both sides deleted a different adjacent entry in the @types block of the lockfile: this branch dropped @types/diff (diff v8 ships its own types) while main dropped @types/doctrine (its only consumer, react-docgen, went away with the webui retirement). Keep both deletions.
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge resolution: PR #10985 ← mainOnly Root cause. Main's Semantic, not just adjacent. The conflict's shared tail ( Load-bearing.
Unverified (no build/typecheck/tests; only the conflicted file edited).
中文说明唯一冲突文件 根因与性质：main 的 关键约束：两个 未验证（未跑构建/类型/测试，只改冲突文件）： |
|
Qwen Code review timed out. Qwen review timed out after 10800 seconds (of the 180-minute budget). For large PRs, retry with a longer timeout by commenting: |
|
/review --timeout=360 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- X3-1 root @types/uuid left unreachable by the uuid v11 bump — already reported (wenshao comment 5535310284)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent 6a": npm run typecheck across the remaining workspaces (acp-bridge, sdk-typescript, web-shell, vscode-ide-companion, live-host, chrome-extension) — the run aborted….
Not reviewed: reverse audit — stopped before round 1 by the review time budget.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/hooks/useDiffData.ts:8 — [review] D3-1 hand-aliased hunk type bypasses core's exported GitDiffHunk, which has zero importerspackages/core/package.json:133 — [review] D3-2 uuid retained for one 8-char id that node:crypto randomUUID already produces in-packagepackage-lock.json:27641 (+1 locations) — [review] D3-3 two byte-identical nested diff@8.0.4 copies with no hoisted root copy, a preserved artifact npm does not generate
— qwen3.8-max via Qwen Code /review (v0.23.0)
|
@qewn-code /triage |
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
APPROVE (verified at head 4b73848)
Historical blockers
Round 1's CHANGES_REQUESTED (f6f0204e) and the maintainer's initial blocking verification report (the two "additional" items beyond the bump) were both addressed in the intermediate pushes — @types/diff removed from both packages, and the remaining Suggestion-tier carry (@types/uuid root entry now unreachable after the bump — cleanup) is non-blocking by everyone's grading, including the final round which posted zero new findings.
My Critical-only scan
- CVE floor math checks out:
diffresolves 8.0.4 ≥ 8.0.3 patch level for GHSA-73rr-hh4g-fpgx;uuidpins exactly 11.1.1 ≥ the 11.1.1 floor for GHSA-w5hq-g745-h8pq. The out-of-scope nestedmermaid/uuid@14exclusion reasoning matches the advisory's affected ranges. No audit-regression risk introduced. - diff v8 breaking-type migration is complete: I enumerated every
diffimporter in core/cli at head — theHunk→StructuredPatchHunk,ParsedDiff→StructuredPatch, andPatchOptions→Pick<StructuredPatchOptionsNonabortable,'context'|'ignoreWhitespace'>renames cover all six sites (diffOptions, fileHistoryService, gitDiff, useDiffData, DiffDialog + test).DEFAULT_DIFF_OPTIONSis only consumed insidediffOptions.ts, at positions wherecontext/ignoreWhitespaceare legal for bothstructuredPatchanddiffLines/createPatch— the Pick tightening cannot drop a needed option. Runtime call shapes (diffLines,structuredPatch, hunk.lines) are unchanged between v7 and v8;@types/diffremoval is correct since v8 ships its own types, and typecheck/lint/test lanes are green on the bump. - uuid v9→v11 is API-safe for the single
v4usage; the deferred suggestion to replace it outright withnode:cryptois follow-up, not a defect. - Lockfile entries are consistent (nested
diff@8.0.4copies are the explained hoist-retention state; thebinary-pathuuidbin change v9→v11 matches the upstream layout). The unrelated-lookingScopeSelectStep.tsxchange is pure prettier line-wrapping with identical call semantics — consistent with the branch picking up the currently-unmerged formatting normalization on touched files.
CI at head
23 green, zero failures or cancellations; the flows' remaining deferrals are cleanup Suggestions tracked in-thread.
Local verification — PR #10985, round 2 (delta)Verdict: merge-ready. Round 1's blocker is gone, its N1 is closed exactly as asked, and the one genuinely new risk since then — a This is a delta pass on top of round 1 (head 1. EnvironmentTwo fresh worktrees off the same clone: 2. Where round 1's items stand
3. The blocker is gone, and the check that says so is discriminating
Green on its own proves little, so: reverting only Tests over every jsdiff/uuid consumer in the tree, not just the two files the PR body names — The migrated code paths, in the real TUINot strictly necessary for a dependency bump, but the six migrated files are exactly the ones that render diffs, so I drove the built bundle under a real pty.
4. The new risk: a lockfile resolved by hand
Structural delta over all 2256 entries, That is the whole delta. Every other entry is byte-identical; the only two entries changed in place are the Closure. A checker that walks every Reproducibility. The second copy of Repo gates, run locally: 5. The advisories, and the floor the manifests now declare
Base Advisory data as GitHub serves it today: GHSA-73rr-hh4g-fpgx ( What a downstream consumer of the published ranges gets, measured rather than assumed:
Does the floor behave like the pin? CI only ever exercises 8.0.4, so I replayed the repo's own call shapes — 6. Non-blockingR2-1 — the PR body now describes a different change than the one in the branch. Round 1's N2 is still open and has grown: the body says R2-2 — two of the changed files are not Prettier-formatted, and CI structurally cannot tell you. -export const DEFAULT_DIFF_OPTIONS: Pick<Diff.StructuredPatchOptionsNonabortable, 'context' | 'ignoreWhitespace'> = {
+export const DEFAULT_DIFF_OPTIONS: Pick<
+ Diff.StructuredPatchOptionsNonabortable,
+ 'context' | 'ignoreWhitespace'
+> = {
-import { diffLines, structuredPatch, type StructuredPatchHunk as Hunk } from 'diff';
+import {
+ diffLines,
+ structuredPatch,
+ type StructuredPatchHunk as Hunk,
+} from 'diff';R2-3 — root R2-4 — the three R2-5 — what is actually holding the merge button, for the record. One note on the automated round-3 review. Its deferred item D3-3 describes the two nested 7. Not covered
中文版本本地验证 —— PR #10985，第 2 轮（增量）结论：可以合入。 第 1 轮的阻塞项已经消失，N1 按要求彻底关闭；而这之后唯一真正新增的风险 —— 一个靠手工解决的 本轮是在第 1 轮（head 1. 环境同一个 clone 下的两个全新 worktree： 2. 第 1 轮各项的现状
3. 阻塞项确实没了，而且这个"绿"是有判别力的两棵树
单看绿色说明不了什么，所以做了反向对照：只把 测试覆盖了树里所有 jsdiff / uuid 使用方，而不只是 PR 正文点名的那两个文件 —— 迁移过的代码路径，在真实 TUI 里对一个依赖升级来说这不是必须的，但被迁移的 6 个文件恰好就是渲染 diff 的那几个，所以我把构建产物放进真实 pty 里跑了一遍。
4. 新增的风险：手工解决的 lockfile
相对 差异就这些。其余每一条都逐字节相同；唯二原地改动的是 闭包完整性。 用一个按 node 解析顺序遍历每条 可复现性。 在 PR 树里跑 第二份 仓库自带的门，本地全跑了： 5. 两条 advisory，以及现在声明的版本下限
基线 GitHub 当前提供的 advisory 数据：GHSA-73rr-hh4g-fpgx（ 下游消费方按已发布区间解析会得到什么，实测而非推断：
下限的行为和钉住的版本一致吗？ CI 永远只会跑到 8.0.4，所以我把仓库自己的调用形态 —— 6. 非阻塞项R2-1 —— PR 正文描述的已经不是分支里的这个改动了。 第 1 轮的 N2 仍未处理，而且范围变大：正文写 R2-2 —— 两个被改的文件不符合 Prettier 格式，而 CI 在结构上不可能告诉你。 -export const DEFAULT_DIFF_OPTIONS: Pick<Diff.StructuredPatchOptionsNonabortable, 'context' | 'ignoreWhitespace'> = {
+export const DEFAULT_DIFF_OPTIONS: Pick<
+ Diff.StructuredPatchOptionsNonabortable,
+ 'context' | 'ignoreWhitespace'
+> = {
-import { diffLines, structuredPatch, type StructuredPatchHunk as Hunk } from 'diff';
+import {
+ diffLines,
+ structuredPatch,
+ type StructuredPatchHunk as Hunk,
+} from 'diff';R2-3 —— 根目录的 R2-4 —— cli 那三处类型导入可以改成从 core 拿，这样 R2-5 —— 记录一下现在到底是什么卡着合并按钮。 关于自动评审 round 3 的一点补充。 它的 deferred 项 D3-3 把"两份嵌套的 7. 未覆盖
🤖 Generated with Claude Code — Claude Opus 5 (1M context) |
6 participants
What this PR does
Clears the two remaining
npm audit --omit=devfindings so the repo-wide audit reports zero vulnerabilities, finishing what #10862 started (that bump already cleared thefast-urihigh andqsadvisories on main):diff^7.0.0→^8.0.2inpackages/core/package.jsonandpackages/cli/package.json— GHSA-73rr-hh4g-fpgx (DoS inparsePatch/applyPatch, patched ≥ 8.0.3). All other copies in the tree were already safe (api-extractor 8.0.4, shadcn 8.0.3, @opentui/core 9.0.0).uuid^9.0.1→^11.1.1inpackages/core/package.json— GHSA-w5hq-g745-h8pq (missing buffer bounds check in v3/v5/v6 whenbufis provided, patched ≥ 11.1.1). The nestedmermaid/node_modules/uuid@14.0.0is not in any affected range of that advisory (affected: <11.1.1, 12.0.0–12.0.1, 13.0.0–13.0.1) and is left alone.package-lock.jsonregenerated;packages/vscode-ide-companion/NOTICES.txtupdated to match.Both are direct dependencies, and the consuming call sites use stable APIs only (
{ v4 }from uuid;diffLines/structuredPatch/Hunkfrom diff), verified below.Why it's needed
Fixes #10850. The
Dependency CVE auditCI gate (npm audit --omit=dev --audit-level=high) went green again after #10862, but the lockfile still carried two known advisories (diff low, uuid moderate). Clearing them now removes the debt before anyone tightens--audit-level, and keeps the audit output clean for the per-package lockfile audits in the same job.Reviewer Test Plan
How to verify
npm installthennpm audit --omit=dev→found 0 vulnerabilities.npx vitest run packages/core/src/services/fileHistoryService.test.ts packages/core/src/utils/openaiLogger.test.ts→ 91 tests pass (usesdiffLines/structuredPatch/Hunkand{ v4 }).npm run buildexits 0.Evidence (Before & After)
Before:
npm audit --omit=devreported 2 vulnerabilities (diff low, uuid moderate).After:
vulns: {}— zero findings. N/A for screenshots — dependency-only change.Tested on