Skip to content

fix(release): use relative postinstall patch dir - #5973

Merged
yiliang114 merged 1 commit into
QwenLM:mainfrom
yiliang114:codex/fix-release-postinstall-patchdir
Jun 29, 2026
Merged

fix(release): use relative postinstall patch dir#5973
yiliang114 merged 1 commit into
QwenLM:mainfrom
yiliang114:codex/fix-release-postinstall-patchdir

Conversation

@yiliang114

Copy link
Copy Markdown
Collaborator

What this PR does

This change makes the published CLI postinstall patch step pass a patch directory relative to the package install root instead of an absolute path.

Why it's needed

The release Docker sandbox build installs the packed CLI globally before running Docker integration tests. The new postinstall patching path passed an absolute directory to patch-package, but patch-package rejects absolute --patch-dir values. That caused the sandbox image build to fail before the Docker integration tests could start.

Reviewer Test Plan

How to verify

Run the package asset script test and confirm the postinstall regression case records --patch-dir patches while the full package asset suite passes.

Evidence (Before & After)

Before: the regression test failed because the generated postinstall command passed an absolute patch directory. After: npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/package-assets.test.js passes with 12 tests.

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

Environment (optional)

Node.js local script tests via Vitest.

Risk & Scope

  • Main risk or tradeoff: Low; the change only affects the generated postinstall patch invocation and keeps the working directory unchanged.
  • Not validated / out of scope: Full Docker sandbox image build was not run locally because the local Docker daemon is unavailable.
  • Breaking changes / migration notes: None.

Linked Issues

Fixes #5969

中文说明

What this PR does

这个改动让发布后的 CLI postinstall 补丁步骤传入相对于安装根目录的 patch 目录,而不是绝对路径。

Why it's needed

Release 的 Docker sandbox 构建会先全局安装打包后的 CLI,再运行 Docker 集成测试。新的 postinstall 补丁逻辑把绝对目录传给了 patch-package,但 patch-package 会拒绝绝对 --patch-dir,导致 sandbox 镜像构建在 Docker 集成测试开始前失败。

Reviewer Test Plan

How to verify

运行 package asset 脚本测试,确认 postinstall 回归用例记录到的是 --patch-dir patches,并且整个 package asset 测试文件通过。

Evidence (Before & After)

Before:回归测试失败,因为生成的 postinstall 命令传入了绝对 patch 目录。After:npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/package-assets.test.js 通过,共 12 个测试。

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

Environment (optional)

通过 Vitest 运行 Node.js 本地脚本测试。

Risk & Scope

  • Main risk or tradeoff:风险较低;改动只影响生成的 postinstall 补丁调用方式,并保持工作目录不变。
  • Not validated / out of scope:本地 Docker daemon 不可用,因此没有运行完整 Docker sandbox 镜像构建。
  • Breaking changes / migration notes:无。

Linked Issues

Fixes #5969

@yiliang114
yiliang114 marked this pull request as ready for review June 29, 2026 02:41
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR, @yiliang114!

Template looks good ✓ — all required sections present, bilingual body included, linked issue #5969.

On direction: this is a clear-cut release pipeline fix. The Docker sandbox build step failed because patch-package rejects absolute --patch-dir paths, and the postinstall script was passing one. Fixing this unblocks the nightly release. Aligned with the project's release infrastructure needs.

On approach: minimal and correct — one line to compute path.relative(installRoot, patchDir), one line to use it, one test update. The || '.' fallback is defensive but harmless (the patches subdir always exists inside installRoot, so path.relative will never return ''). No scope creep, no unrelated changes.

Moving on to code review. 🔍

中文说明

感谢贡献,@yiliang114

模板完整 ✓ — 所有必填部分齐全,包含中英双语,关联了 issue #5969

方向:这是一个明确的发布流水线修复。Docker sandbox 构建步骤因为 patch-package 拒绝绝对 --patch-dir 路径而失败,postinstall 脚本正好传了绝对路径。修复后 nightly release 可以正常运行。符合项目的发布基础设施需求。

方案:最小化且正确 — 一行计算 path.relative(installRoot, patchDir),一行使用它,一行测试更新。|| '.' 回退是防御性的但无害(patches 子目录始终在 installRoot 内,path.relative 不会返回 '')。没有范围蔓延,没有无关改动。

进入代码审查 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code Review

The fix is clean and correct. path.relative(installRoot, patchDir) computes the relative path from the install root to the patches directory — exactly what patch-package needs. The test is updated to match, expecting 'patches' instead of the absolute path. No issues found.

One observation: the || '.' fallback is defensive but unreachable in practice — patchDir is always <installRoot>/patches, so path.relative will always return 'patches', never ''. Not a concern, just noting it.

Testing

Before/After: generated postinstall argv

$ node -e "
const path = require('path');
const distDir = '/tmp/demo-dist';
const installRoot = distDir;
const patchDir = path.join(distDir, 'patches');
const relativePatchDir = path.relative(installRoot, patchDir) || '.';
console.log('BEFORE (main):', '--patch-dir', patchDir);
console.log('AFTER  (PR):  ', '--patch-dir', relativePatchDir);
"
BEFORE (main): --patch-dir /tmp/demo-dist/patches
AFTER  (PR):   --patch-dir patches

The before path is absolute (rejected by patch-package), the after path is relative (accepted).

Unit test results (PR branch)

 RUN  v3.2.4 /home/github-runner/actions-runner-8/_work/qwen-code/qwen-code/.qwen/worktrees/triage

 ✓ scripts/tests/package-assets.test.js (12 tests) 94ms

 Test Files  1 passed (1)
      Tests  12 passed (12)
   Start at  10:48:54
   Duration  331ms (transform 55ms, setup 21ms, collect 45ms, tests 94ms, environment 0ms, prepare 60ms)

All 12 tests pass, including the postinstall regression case that verifies --patch-dir patches (relative) is generated.

CI also green: Test (ubuntu-latest, Node 22.x) passed on the PR.

中文说明

代码审查

修复简洁正确。path.relative(installRoot, patchDir) 计算从安装根目录到 patches 目录的相对路径 — 正好是 patch-package 需要的。测试已更新匹配,期望 'patches' 而不是绝对路径。未发现问题。

一个观察:|| '.' 回退是防御性的但在实践中不可达 — patchDir 始终是 <installRoot>/patches,所以 path.relative 总是返回 'patches',不会返回 ''。不是问题,只是记录一下。

测试

所有 12 个测试通过,包括验证生成 --patch-dir patches(相对路径)的 postinstall 回归用例。CI (Ubuntu) 也已通过。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Straightforward fix, cleanly implemented. The release pipeline was broken — patch-package rejects absolute --patch-dir, and the postinstall script was passing one. The fix is the minimum needed: compute the relative path, use it. The path.relative() call correctly resolves to 'patches' since the patches dir always lives inside the install root.

Test updated to match, all 12 tests pass, CI green. The before/after evidence clearly shows the absolute-to-relative conversion.

If I had to maintain this in six months, I'd thank the author — one line of logic, obvious intent, no surprises. Approving. ✅

中文说明

简洁的修复,实现干净。发布流水线坏了 — patch-package 拒绝绝对 --patch-dir,而 postinstall 脚本正好传了绝对路径。修复是最小化的:计算相对路径,使用它。path.relative() 正确解析为 'patches',因为 patches 目录始终在安装根目录内。

测试已更新匹配,12 个测试全部通过,CI 绿色。前后对比清楚展示了绝对路径到相对路径的转换。

如果六个月后需要维护这段代码,我会感谢作者 — 一行逻辑,意图明确,没有意外。批准 ✅

Qwen Code · qwen3.7-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No review findings. Downgraded from Approve to Comment: CI still running.

— qwen3.7-max via Qwen Code /review

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found. LGTM! ✅

— qwen3.7-max via Qwen Code /review

@yiliang114
yiliang114 added this pull request to the merge queue Jun 29, 2026
Merged via the queue into QwenLM:main with commit 98dec4f Jun 29, 2026
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Release Failed for v0.19.3-nightly.20260629.ee375c6a4 on 2026-06-29

3 participants