feat(installer): keep hosted entrypoint and verify release assets#3828
feat(installer): keep hosted entrypoint and verify release assets#3828yiliang114 wants to merge 52 commits into
Conversation
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Requesting changes due to a deterministic script test failure.
— gpt-5.5 via Qwen Code /review
|
@copilot pls review for me |
# Conflicts: # scripts/build-standalone-release.js # scripts/create-standalone-package.js # scripts/installation/INSTALLATION_GUIDE.md # scripts/installation/install-qwen-with-source.bat # scripts/installation/install-qwen-with-source.sh # scripts/tests/install-script.test.js
yiliang114
changed the title
|
Updated after #3776 was merged into
Local validation after the base change: npm run test:scripts
npx eslint scripts/build-standalone-release.js scripts/create-standalone-package.js scripts/release-script-utils.js scripts/release-asset-config.js scripts/tests/install-script.test.js
npx prettier --check scripts/build-standalone-release.js scripts/create-standalone-package.js scripts/release-script-utils.js scripts/release-asset-config.js scripts/tests/install-script.test.js scripts/installation/INSTALLATION_GUIDE.md .github/workflows/release.yml package.json
bash -n scripts/installation/install-qwen-with-source.sh
git diff --check origin/main...HEADResult: |
|
Pushed an additional follow-up commit (
I intentionally did not fold Aliyun OSS/CDN archive upload automation into this PR. That belongs to the later release-publishing step because it requires the actual upload path/credentials and would expand this PR beyond the hosted-entrypoint/release-asset cleanup scope. Local validation after this commit: npm run test:scripts
npx eslint scripts/build-standalone-release.js scripts/create-standalone-package.js scripts/release-script-utils.js scripts/release-asset-config.js scripts/tests/install-script.test.js
npx prettier --check scripts/build-standalone-release.js scripts/create-standalone-package.js scripts/release-script-utils.js scripts/release-asset-config.js scripts/tests/install-script.test.js scripts/installation/INSTALLATION_GUIDE.md .github/workflows/release.yml package.json
bash -n scripts/installation/install-qwen-with-source.sh
git diff --checkResult: |
yiliang114
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
@copilot pls review pr |
Reviewed. Here's a summary: All critical issues from prior rounds are resolved:
Structural quality:
One minor observation: Tests: 54 tests, 47 pass on macOS (7 Windows-only skipped). ESLint, Prettier, shell syntax, and |
The verify-installation-release script was duplicating the archive name derivation logic with a hardcoded ternary instead of reusing the standaloneArchiveName helper from build-standalone-release. Export the helper and import it so the extension mapping lives in one place.
|
|
||
| const match = /^([0-9a-fA-F]{64})\s+\*?(.+)$/.exec(trimmed); | ||
| if (!match) { | ||
| fail(`Malformed SHA256SUMS line ${index + 1}: ${trimmed}`); |
There was a problem hiding this comment.
[Critical] parseSha256Sums throws on malformed/unrecognized lines (fail()), replacing the old tolerant parsing that silently skipped unparseable lines.
This function is used to parse upstream SHASUMS256.txt from nodejs.org (via parseChecksums in build-standalone-release.js:106). If nodejs.org adds comments or tweaks the format, the release pipeline breaks entirely — while the old code would just use the subset of entries it could parse.
| fail(`Malformed SHA256SUMS line ${index + 1}: ${trimmed}`); | |
| function parseSha256Sums(content, { tolerant = false } = {}) { | |
| const checksums = new Map(); | |
| for (const [index, line] of content.split(/\r?\n/).entries()) { | |
| const trimmed = line.trim(); | |
| if (!trimmed) continue; | |
| const match = /^([0-9a-fA-F]{64})\s+\*?(.+)$/.exec(trimmed); | |
| if (!match) { | |
| if (tolerant) continue; | |
| fail(`Malformed SHA256SUMS line ${index + 1}: ${trimmed}`); | |
| } | |
| checksums.set(match[2], match[1].toLowerCase()); | |
| } | |
| return checksums; | |
| } |
Then use parseSha256Sums(content, { tolerant: true }) when parsing upstream Node.js SHASUMS, and keep strict mode for self-generated checksums.
— DeepSeek/deepseek-v4-pro via Qwen Code /review
| const { fetchImpl = fetch } = options; | ||
| const normalizedBaseUrl = normalizeHttpsBaseUrl(baseUrl); | ||
| const checksumUrl = new URL('SHA256SUMS', normalizedBaseUrl).toString(); | ||
| const checksums = parseSha256Sums(await fetchText(checksumUrl, fetchImpl)); |
There was a problem hiding this comment.
[Suggestion] verifyReleaseBaseUrl makes fetch calls without a timeout. If a remote server (GitHub Releases, OSS) accepts the TCP connection but never sends data, the new npm run verify:installation-release -- --base-url ... CI step hangs indefinitely until the job timeout (potentially hours).
| const checksums = parseSha256Sums(await fetchText(checksumUrl, fetchImpl)); | |
| const response = await fetchImpl(checksumUrl, { | |
| signal: AbortSignal.timeout(30_000), | |
| }); |
Apply to all fetch calls: fetchText (line 239), assertRemoteAssetAvailable (lines 210, 220), and here.
— DeepSeek/deepseek-v4-pro via Qwen Code /review
| if %ERRORLEVEL% EQU 0 exit /b 0 | ||
| echo(!VERSION!| findstr /R /C:"^[0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*[A-Za-z0-9.-]*$" >nul | ||
| if !ERRORLEVEL! EQU 0 exit /b 0 | ||
| echo(!VERSION!| findstr /R /C:"^v[0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*[A-Za-z0-9.-]*$" >nul |
There was a problem hiding this comment.
[Suggestion] The .bat version validation regex ([A-Za-z0-9.-]*$) is looser than the .sh counterpart (([.-][A-Za-z0-9]+)*$). It allows multiple consecutive separators, trailing dots/dashes, and empty pre-release segments — e.g. v1.2.3., v1.2.3--rc1, v1.2.3..beta pass validation but then fail later with confusing 404/"archive not found" errors. The .sh installer correctly rejects these with a clear "must be a semver string" message.
Tighten the suffix portion of the regex to match the .sh semantics, or use a PowerShell-based semver check for consistency across platforms.
— DeepSeek/deepseek-v4-pro via Qwen Code /review
Summary
Scope
This PR intentionally does not publish
install-qwen.shorinstall-qwen.batas GitHub Release assets. Those installer scripts are staged for the hosted installation endpoint, while each GitHub Release publishes the platform-specific standalone archives andSHA256SUMS.The hosted install flow remains:
Validation
Commands run after folding the hosted staging and release verification PRs into this branch:
Observed locally:
npm run test:scriptsreports 120 passed and 7 Windows-only tests skipped on macOS. eslint, prettier, shell syntax, whitespace checks, hosted asset staging smoke, and verifier help smoke are clean.Scope / Risk
SHA256SUMS.--version/QWEN_INSTALL_VERSIONfor pinned installs from the hosted entrypoint.Testing Matrix
Testing matrix notes:
Linked Issues / Bugs
Related #3728