fix(sdk): stop emitting a duplicated hashbang in the serve-mcp bin - #10485
fix(sdk): stop emitting a duplicated hashbang in the serve-mcp bin#10485wenshao wants to merge 7 commits into
Conversation
esbuild already carries the entry point's own `#!/usr/bin/env node` into the bundle, so the extra `banner` stacked a second hashbang onto line 2 of `dist/daemon-mcp/serve-bridge/bin.js`. That is valid to esbuild and a `SyntaxError` to node, which makes the published `qwen-serve-mcp` bin fail to start through both `node <file>` and the shebang. The bin has never been runnable since it was introduced, and `@qwen-code/sdk@0.1.8` ships it broken. Drop the redundant banner and assert at build time that the bin starts with a hashbang and that `node --check` can parse it, so neither failure mode can be published again. Nothing else emits a hashbang banner in this repo; every other bin already relies on its entry point's own.
|
Thanks for the fix! Template looks good ✓ — all required sections present, bilingual. Problem: observed bug with hard evidence, not theoretical — #10484 reproduces straight from the published Direction: clearly aligned — this repairs a bin the SDK already ships as broken, and adds the build-time guard that would have caught it. No auth/sandbox/model/telemetry/release-pipeline surface touched. Size: not applicable — one file ( Approach: exactly the minimal change. Dropping the redundant Risk: no elevated risk signals — no high-risk paths touched (checked against the revert-history pattern list). Linked issue #10484 is open, so no duplicate/already-fixed exit applies. Moving on to code review. 🔍 中文说明感谢修复! 模板完整 ✓ —— 各必需小节齐全,中英双语。 问题:已观测到的 bug,证据确凿,不是理论问题 —— #10484 直接从已发布的 方向:明确对齐 —— 它修复了 SDK 已经发布但实际损坏的 bin,并补上了本可以拦住该问题的构建期守卫。不触及 auth/沙箱/模型/遥测/发布流水线相关面。 规模:不适用 —— 单文件( 方案:恰好是最小改动。去掉多余的 风险:无升级风险信号 —— 未触及高风险路径(已按 revert 历史模式列表核对)。关联 issue #10484 仍开放,不适用重复/已修复分支。 进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRe-run on the same commit — verdict on the code is unchanged: this is exactly the right change, and I found no blockers. My independent proposal for "bin ships a duplicated hashbang" was: drop the The two
Neither blocks this PR; the fix itself is minimal and correct. Test evidenceThis is an unattended CI run — no PR code was executed here; the evidence below is the PR's own CI read through the API, plus the sandboxed CI status on the reviewed commit at the time of this re-run (attempt 4 of Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Reading the history so nobody has to: attempt 2's red is pre-existing flake, not this PR. The failures are all web-shell DOM/timing tests ( Sandboxed verification for the behavioural claim already ran and passed (46/46 scripted assertions): A/B against the base build (base emits two hashbangs and fails both launch paths; head boots and answers a real MCP 中文说明代码审查同提交上的重跑 —— 对代码的结论不变:这正是正确的改动,未发现阻塞项。面对"发布产物出现重复 hashbang",我的独立方案是:去掉
两者都不阻塞本 PR;修复本身最小且正确。 测试证据本次为无人值守 CI 运行 —— 此处未执行任何 PR 代码;以下证据来自通过 API 读取的 PR 自身 CI,以及沙箱 审查提交上的 CI 状态: 把历史讲清楚,免得别人再查一遍:第 2 次尝试的红是既有抖动,不是本 PR 造成。失败全部是 web-shell DOM/时序测试( 行为性声明的沙箱验证已经跑过并通过(46/46 脚本断言):与 base 构建 A/B 对照(base 输出两行 hashbang、两种启动方式都失败;head 能启动并通过 stdio 应答真实 MCP — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean across every stage on the third pass; the two open items are the same non-blocking This re-run exists to answer one question: why hasn't the approval landed yet? The answer is mechanical, not substantive. My last pass left the approve-on-green instruction in place, but the finalize step only acts when every check on the reviewed commit is green — and the Linux unit suite never settled: attempts 1 and 3 were cancelled mid-suite by re-run triggers, and attempt 2's red was pre-existing web-shell timeout flake that fails identically on the merge base without this PR. Nothing in that history reflects on the change; the SDK's own 1699-test suite and the 46/46 sandboxed verification assertions stayed green throughout. On the substance my read is unchanged, and I re-verified rather than rubber-stamped: the problem is real and reproduced from the published 0.1.8 tarball; the fix is exactly the minimal one (drop the duplicated banner, keep the entry's own hashbang like every other bin in the repo); the guard is load-bearing by mutation-matrix evidence, not decoration; the diff carries nothing else. yiliang114's approval of this exact commit already stands; mine is the second vote 中文说明这次重跑只为回答一个问题:为什么批准还没有落下?答案是机制性的,不是实质性的。上一轮我已留下"变绿即批准"的指令,但 finalize 步骤只在被审查提交上的所有检查全绿时才执行 —— 而 Linux 单测套件始终没有落定:第 1、3 次尝试被重跑触发中途取消,第 2 次尝试的红是既有的 web-shell 超时抖动,在不含本 PR 的合并基上以同样形态失败。这些历史与改动本身无关;SDK 自身的 1699 测试套件与 46/46 沙箱验证断言全程保持绿色。 实质判断不变,且本轮是重新核实而非照单全收:问题真实存在,可直接从已发布的 0.1.8 tarball 复现;修复恰好是最小方案(去掉重复的 banner,像仓库里其他所有 bin 一样保留入口自带的 hashbang);守卫经突变矩阵逐行证明是承重的,不是摆设;diff 不夹带任何其他内容。yiliang114 对同一提交的批准已经生效;我的批准是 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 6a": Windows (cmd.exe) behavior of execSync( node --check ${JSON.stringify(filePath)} ) — no Windows runner available in this environment to verify empirically..
Test Plan (not a blocker): @qwen-code/sdk@0.1.8 — no such file or directory.
中文说明
已审查。 建议见行内评论。
未探索到全部深度(达到工具调用预算):"agent 6a":Windows (cmd.exe) behavior of execSync( node --check ${JSON.stringify(filePath)} ) — no Windows runner available in this environment to verify empirically.。
Test Plan(非阻断):@qwen-code/sdk@0.1.8 — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| throw new Error(`Bin ${filePath} must start with a hashbang line`); | ||
| } | ||
| try { | ||
| execSync(`node --check ${JSON.stringify(filePath)}`, { stdio: 'pipe' }); |
There was a problem hiding this comment.
[Suggestion] R1-1: The new assertExecutableBin guard invokes execSync(node --check ${JSON.stringify(filePath)}). execSync with a command string runs through /bin/sh -c, and JSON.stringify is JSON quoting, not shell quoting — inside double quotes, $(...), backticks, and $VAR remain live command substitution on POSIX. filePath is derived from the checkout location (rootDir from import.meta.url), so if the repo is cloned under a path whose directory components contain $(…), a backtick, or $VAR (e.g. a worktree or job directory named from branch or job metadata), the embedded command executes as the build user during npm run build, and the expansion can rewrite the argument so node --check targets a wrong path and the build fails with a misleading Bin ... does not parse error. This is the only execSync site in this file interpolating a dynamic value (the tsc / dts-bundle-generator calls are fully literal strings).
Witness:
probe: real assertExecutableBin extracted from HEAD,
filePath=/tmp/probe-qwe$(touch /tmp/probe-finding-a-marker)ry/bin.js
BUGGY ARM (the PR's exact construction):
marker before call: false
threw: Bin ... does not parse: Cannot find module '/tmp/probe-qwery/bin.js'
marker after call: true <- embedded $(touch ...) executed
FIXED ARM (execFileSync('node', ['--check', filePath], { stdio: 'pipe' })):
marker after call: false; literal path parses; guard returns clean
The fix spans two locations, so a regular code block instead of a suggestion:
// top of file: extend the existing import
import { execFileSync, execSync } from 'node:child_process';
// inside assertExecutableBin — the argv form needs no quoting at all
execFileSync('node', ['--check', filePath], { stdio: 'pipe' });中文说明
新增的 assertExecutableBin 守卫使用 execSync(node --check ${JSON.stringify(filePath)})。execSync 传入命令字符串时会经由 /bin/sh -c 执行,而 JSON.stringify 是 JSON 引号,不是 shell 引号——在双引号内部,$(...)、反引号和 $VAR 在 POSIX 上仍然是有效的命令替换。filePath 来自检出位置(由 import.meta.url 推导的 rootDir),因此如果仓库被克隆到目录组件中含有 $(…)、反引号或 $VAR 的路径下(例如以分支或任务元数据命名的 worktree 或任务目录),嵌入的命令就会在 npm run build 期间以构建用户身份执行,且展开可能改写参数,使 node --check 指向错误路径,构建会以误导性的 Bin ... does not parse 错误失败。这是该文件中唯一插入动态值的 execSync 调用点(tsc / dts-bundle-generator 调用都是纯字面量字符串)。
验证证据:
探针:从 HEAD 提取真实的 assertExecutableBin,
filePath=/tmp/probe-qwe$(touch /tmp/probe-finding-a-marker)ry/bin.js
BUGGY ARM(PR 中的原始写法):
调用前 marker 存在:false
抛出:Bin ... does not parse: Cannot find module '/tmp/probe-qwery/bin.js'
调用后 marker 存在:true <- 嵌入的 $(touch ...) 被执行了
FIXED ARM(execFileSync('node', ['--check', filePath], { stdio: 'pipe' })):
调用后 marker 存在:false;字面量路径正常解析;守卫正常返回
修复涉及两处,因此给出普通代码块而非 suggestion:
// 文件顶部:扩展现有 import
import { execFileSync, execSync } from 'node:child_process';
// assertExecutableBin 内部——argv 形式完全不需要引号处理
execFileSync('node', ['--check', filePath], { stdio: 'pipe' });— qwen3.8-max via Qwen Code /review (v0.22.3)
There was a problem hiding this comment.
Fixed in fec89f4a31 — confirmed, and taken as written.
execFileSync('node', ['--check', filePath], { stdio: 'pipe' }) now, with the reason recorded at the call site so the string form doesn't come back. I reproduced your witness against the real guard body extracted from HEAD, with filePath under a directory named qwen$(touch …)ry:
BUGGY execSync + JSON.stringify | marker created: true | threw: Bin … does not parse: …
FIXED execFileSync argv | marker created: false | returned clean
Both halves of your finding hold: the embedded command ran, and the expansion rewrote the argument so the guard failed the build with a misleading does not parse. The argv form runs the literal path and returns clean.
中文说明
已在 fec89f4a31 修复 —— 结论确认,按建议原样采纳。
现在是 execFileSync('node', ['--check', filePath], { stdio: 'pipe' }),并在调用点记录了原因,避免字符串形式回潮。我用从 HEAD 提取的真实守卫函数体复现了你的验证证据,filePath 位于名为 qwen$(touch …)ry 的目录下:
BUGGY execSync + JSON.stringify | marker created: true | threw: Bin … does not parse: …
FIXED execFileSync argv | marker created: false | returned clean
你这条发现的两半都成立:嵌入的命令确实执行了,并且展开改写了参数,使守卫以误导性的 does not parse 让构建失败。argv 形式传的是字面量路径,正常返回。
| assertExecutableBin( | ||
| join(rootDir, 'dist', 'daemon-mcp', 'serve-bridge', 'bin.js'), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R1-2: The new assertExecutableBin guard is the sole check pinning the duplicate-hashbang regression this PR fixes, and nothing in the suite exercises it — neither of its two failure branches (missing hashbang, node --check parse failure) is reachable from any test. A repo-wide sweep at HEAD confirms it: 0 test files reference assertExecutableBin, the built bin.js, or any hashbang assertion; test/unit/serve-bridge.test.ts imports source modules only, and the one integration test touching serve-bridge spawns the source via tsx — structurally blind to a build-time banner regression. A future edit neutralizing the guard (a refactor swallowing the error, converting a throw to a warning, or dropping the call) leaves every suite green; if the regression then recurs (banner re-added, or an esbuild config change that drops the entry hashbang), the published qwen-serve-mcp bin ships with #!/usr/bin/env node on line 2 and fails at startup with SyntaxError — exactly the break this PR fixes.
Witness:
repo-wide sweep at HEAD (node_modules excluded):
assertExecutableBin referenced in test files: 0
bin.js / hashbang assertions in packages/sdk-typescript/test/: 0
test/unit/serve-bridge.test.ts imports source modules only
integration-tests/cli/daemon-invocation-context.test.ts:165 spawns bin.ts via tsx (source, not the built artifact)
Suggested fix — add an artifact-level test that pins the shipped artifact rather than the guard (so it reds on a banner re-addition even if assertExecutableBin is later neutralized), e.g.:
// packages/sdk-typescript/test/unit/serve-bridge-bin.test.ts (run post-build)
const binPath = join(distDir, 'daemon-mcp/serve-bridge/bin.js');
const lines = readFileSync(binPath, 'utf8').split('\n');
expect(lines[0]).toBe('#!/usr/bin/env node');
expect(lines[1].startsWith('#!')).toBe(false);
expect(() => execFileSync('node', ['--check', binPath])).not.toThrow();Note the suite's shape when adding it: vitest.config.ts:29 sets include: ['test/**/*.test.ts'], so the test must live under test/ with a .test.ts suffix, and reading dist/ relies on the repo's built-dist precondition (the vitest globalSetup guard stops the run when dist/ is missing). If you add it, please prove it pins the regression: re-add the esbuild banner (or otherwise remove the fix) and confirm the new test goes red — it must fail when line 2 becomes #!... or the built output stops parsing, independent of whether assertExecutableBin still exists.
中文说明
新增的 assertExecutableBin 守卫是钉住本 PR 所修复的重复 hashbang 回归的唯一检查,但测试套件中没有任何内容覆盖它——它的两个失败分支(缺少 hashbang、node --check 解析失败)都无法被任何测试触达。在 HEAD 上的全仓库扫描确认:0 个测试文件引用 assertExecutableBin、构建产物 bin.js 或任何 hashbang 断言;test/unit/serve-bridge.test.ts 只导入源码模块,而唯一触及 serve-bridge 的集成测试通过 tsx 启动源码——在结构上无法看到构建期 banner 回归。未来任何使守卫失效的修改(重构吞掉错误、把 throw 变成警告、或删掉调用)都会让整个套件保持绿色;届时若回归复发(重新加回 banner,或 esbuild 配置变更丢弃了入口的 hashbang),发布出的 qwen-serve-mcp bin 将在第 2 行带着 #!/usr/bin/env node,并在启动时以 SyntaxError 失败——正是本 PR 要修复的问题。
验证证据:
HEAD 上的全仓库扫描(排除 node_modules):
测试文件中 assertExecutableBin 的引用数:0
packages/sdk-typescript/test/ 中 bin.js / hashbang 断言的引用数:0
test/unit/serve-bridge.test.ts 仅导入源码模块
integration-tests/cli/daemon-invocation-context.test.ts:165 通过 tsx 启动 bin.ts(源码,而非构建产物)
建议修复——添加一个钉住交付产物而非守卫本身的产物级测试(这样即使 assertExecutableBin 日后被失效,重新加回 banner 时它也会变红),例如:
// packages/sdk-typescript/test/unit/serve-bridge-bin.test.ts(构建后运行)
const binPath = join(distDir, 'daemon-mcp/serve-bridge/bin.js');
const lines = readFileSync(binPath, 'utf8').split('\n');
expect(lines[0]).toBe('#!/usr/bin/env node');
expect(lines[1].startsWith('#!')).toBe(false);
expect(() => execFileSync('node', ['--check', binPath])).not.toThrow();添加时请注意套件的约束:vitest.config.ts:29 设置了 include: ['test/**/*.test.ts'],因此测试必须位于 test/ 下且以 .test.ts 结尾;读取 dist/ 依赖仓库的已构建 dist 前置条件(vitest globalSetup 守卫会在 dist/ 缺失时中止运行)。如果添加了该测试,请证明它确实钉住了回归:重新加回 esbuild banner(或以其他方式移除修复),确认新测试变红——无论 assertExecutableBin 是否仍然存在,只要第 2 行变成 #!... 或构建产物不再可解析,它都必须失败。
— qwen3.8-max via Qwen Code /review (v0.22.3)
There was a problem hiding this comment.
Added in fec89f4a31 — packages/sdk-typescript/test/unit/serve-bridge-bin.test.ts, and it does red on a banner re-addition with assertExecutableBin gone. One deliberate deviation from the suggested shape, and one correction to its premise.
Deviation. A test that reads only dist/ is silent on any tree that hasn't been built, so I made the primary case build-independent: the bin's esbuild options moved into scripts/serve-bridge-bin-build-options.js, shared by the build script and the test, and the test builds the bin with those exact options into a temp dir (23 ms — the bundle externalizes @modelcontextprotocol/sdk) and asserts line 1 is #!/usr/bin/env node, line 2 is not a hashbang, and node --check parses. Your dist/ assertion is kept as a second case, skipIf the artifact is absent — it covers the one thing the options module cannot: a banner re-added at the call site as a spread override.
Correction. packages/sdk-typescript/vitest.config.ts has no globalSetup — nothing stops the run when dist/ is missing. What actually puts dist/ there in CI is the root prepare script: npm ci runs node scripts/prepare.js → npm run build, which builds every workspace in order. The test job never runs a build step of its own. That also means QWEN_SKIP_PREPARE=1 lanes (qwen-autofix.yml, e2e.yml, the review tooling's scratch trees) have no dist/ — which is why the second case skips rather than fails there, and why the first case doesn't depend on it at all.
Mutation proof, exactly the pin you asked for:
| mutant | build | test 1 (built from options) | test 2 (dist/) |
|---|---|---|---|
| none (as pushed) | exit 0 | ✅ | ✅ |
banner back in the shared options |
— | ❌ expected true to be false |
✅ |
banner at the call site + guard neutralized (if (filePath) return;) |
exit 0 — regression publishes | ✅ | ❌ |
| restored | exit 0 | ✅ | ✅ |
Row 3 is your scenario: the build goes green with #!/usr/bin/env node on line 2 of the shipped bin, and the suite is what catches it. Full SDK suite after the change: 38 files / 1707 tests, all green; eslint --max-warnings 0, prettier --check and tsc --noEmit clean.
中文说明
已在 fec89f4a31 添加 —— packages/sdk-typescript/test/unit/serve-bridge-bin.test.ts,并且确认它在"重新加回 banner + assertExecutableBin 已失效"时会变红。相对建议的形态有一处有意的偏离,另有一处前提需要更正。
偏离。 只读 dist/ 的测试在未构建的树上是静默的,所以我把主用例做成不依赖构建产物:bin 的 esbuild 选项抽到 scripts/serve-bridge-bin-build-options.js,由构建脚本与测试共享;测试用这份完全相同的选项把 bin 构建到临时目录(23 ms —— 该 bundle 把 @modelcontextprotocol/sdk 外部化),断言第 1 行是 #!/usr/bin/env node、第 2 行不是 hashbang、node --check 可解析。你建议的 dist/ 断言作为第二个用例保留,产物缺失时 skipIf —— 它覆盖了选项模块覆盖不到的一种情况:banner 以展开覆写的形式加在调用点上。
更正。 packages/sdk-typescript/vitest.config.ts 里没有 globalSetup —— dist/ 缺失时并没有什么会中止运行。CI 里真正把 dist/ 放到位的是根 prepare 脚本:npm ci 会执行 node scripts/prepare.js → npm run build,按依赖顺序构建所有 workspace。test job 自身从不跑构建步骤。这也意味着设置了 QWEN_SKIP_PREPARE=1 的 lane(qwen-autofix.yml、e2e.yml、review 工具的 scratch tree)没有 dist/ —— 所以第二个用例在那里是跳过而不是失败,而第一个用例根本不依赖它。
突变验证,正是你要求钉住的那一点:
| 突变 | 构建 | 用例 1(按选项构建) | 用例 2(dist/) |
|---|---|---|---|
| 无(即已推送版本) | exit 0 | ✅ | ✅ |
共享选项里加回 banner |
— | ❌ expected true to be false |
✅ |
调用点加 banner + 守卫失效(if (filePath) return;) |
exit 0 —— 回归会被发布 | ✅ | ❌ |
| 还原 | exit 0 | ✅ | ✅ |
第 3 行就是你说的场景:构建全绿,而发布出的 bin 第 2 行是 #!/usr/bin/env node,靠测试套件抓住。改动后 SDK 全量套件:38 文件 / 1707 测试全绿;eslint --max-warnings 0、prettier --check、tsc --noEmit 均干净。
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 46 passed · 0 failed · 46 total Flakiness gate: not applicable — no runnable changed test files (0 out-of-scope file(s) noted in the log) 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:46 通过 · 0 失败 · 46 总计 抖动门:不适用 — no runnable changed test files (0 out-of-scope file(s) noted in the log) Verification reportPR 10485 — fix(sdk): stop emitting a duplicated hashbang in the serve-mcp binVerdict: 中文摘要
Central claim + A/BCentral claim: the built Witness:
A/B assertions: 11/11 (every base-side red is an encoded expectation, i.e. a pass). Secondary claim 1 — the guard is load-bearing, both clauses pinned. Witness:
Matrix assertions: 13/13, no surviving mutants, no redundant defence (each hunk fails its own mutation). Secondary claim 2 — no sibling of the bug class remains. Witness:
Census + tarball assertions: 18/18. GatesWitness:
Gate assertions: 4/4. Findings
Not covered
MethodologyEnvironment: CI Flakiness gate logEvidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 77 passed · 0 failed · 77 total Flakiness gate: not applicable — no runnable changed test files (0 out-of-scope file(s) noted in the log) 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:77 通过 · 0 失败 · 77 总计 抖动门:不适用 — no runnable changed test files (0 out-of-scope file(s) noted in the log) Verification reportPR 10485 — fix(sdk): stop emitting a duplicated hashbang in the serve-mcp bin (round 2)Verdict: 中文摘要
Previous-finding status (follow-up round)This is a follow-up round. The head OID is byte-identical to the previous round (
Central claim + A/BCentral claim: the built Witness:
A/B assertions: 17/17 (every base-side red is an encoded expectation, i.e. a pass). Reviewer Test Plan, per step — all three steps were executable and pass:
Secondary claim 1 — the guard is load-bearing, both clauses pinned. Witness:
Matrix assertions: 20/20, no surviving mutants, no redundant defence — each hunk fails its own single-hunk mutation, and only the combination row reproduces the silent ship. Positive control: C1/C2 are the caught mutants; C0 is the unmutated green. Secondary claim 2 — no sibling of the bug class remains. Witness:
Census + tarball assertions: 23/23 and 11/11. GatesWitness:
Gate assertions: 6/6. Findings
No new findings this round. Not covered
MethodologyEnvironment: CI Flakiness gate logEvidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
CanReader
left a comment
There was a problem hiding this comment.
Reviewed the build change and reproduced the bug end to end. This is a real break, not a theoretical one, and the fix is right.
I rebuilt the serve-bridge entry both ways with the repo's own esbuild to confirm the mechanism:
# A) with banner:js — the config on main today
$ esbuild src/daemon-mcp/serve-bridge/bin.ts --bundle --platform=node --format=esm \
--banner:js='#!/usr/bin/env node' --external:@modelcontextprotocol/sdk
head -2:
#!/usr/bin/env node
#!/usr/bin/env node
$ node --check A.js
SyntaxError: Invalid or unexpected token
# B) without the banner — this PR
head -2:
#!/usr/bin/env node
var __defProp = Object.defineProperty;
$ node --check B.js -> exit 0
src/daemon-mcp/serve-bridge/bin.ts already starts with #!/usr/bin/env node, and esbuild carries an entry point's own hashbang into the output, so the banner really was stacking a second one on line 2. Since dist/daemon-mcp/serve-bridge/bin.js is the qwen-serve-mcp bin in package.json, the published binary could not start at all.
I also ran the new assertExecutableBin against both artifacts: it throws on A and passes on B, so the guard actually catches the regression it is there to catch. Putting it in the build script rather than a unit test is the right layer — this is a property of the emitted artifact, and neither type checking nor vitest would ever see it.
One suggestion, not a blocker: the guard hardcodes the one bin path. Since package.json already declares the bin map, driving the assertion off that map instead would cover any bin added later without someone remembering to add a second call. Today there is exactly one entry so the coverage is complete either way.
Two review follow-ups on the duplicated-hashbang fix. `assertExecutableBin` ran `node --check` through `execSync` with a command string, so the path went through `/bin/sh -c` — and `JSON.stringify` is JSON quoting, not shell quoting. A checkout under a directory containing `$(…)`, a backtick or `$VAR` executed the embedded command as the build user and then failed the build with a misleading `does not parse`. The argv form of `execFileSync` needs no quoting at all. Nothing in the suite covered the guard, so a later edit that swallows its error or drops the call would leave every test green while the next banner regression publishes. Extract the bin's esbuild options into a module the build script and a new test share, and assert on the emitted bytes: exactly one hashbang, on line 1, and `node --check` parses. That reds on a banner regression whatever happens to the guard. A second case pins the shipped `dist/` artifact, catching a banner re-added at the call site rather than in the shared options; `npm ci` builds it through the root `prepare` script, and it skips on a tree without a build rather than failing.
|
Both Review follow-ups
On the previous red ( Both failures are the same self-hosted-runner starvation, and neither reaches this diff:
This diff touches Local gates on the new head: 中文说明两条 评审跟进
关于上一次的红( 两处失败是同一个自建 runner 资源饥饿问题,都没有触及本 diff:
本 diff 只触及 新 head 的本地门禁: |
|
CI attempt 1 on
It failed in the The fixture writes a deliberately-CommonJS stub uploader into The A/B is decisive, because the helper tests are byte-identical across the two commits (this PR touches only
Same code, different host, different result. The stray
What attempt 1 does prove about this PR: 中文说明
失败发生在 该 fixture 会把一个刻意写成 CommonJS 的 stub 上传器写入 A/B 是决定性的,因为这些 helper 测试在两个提交间逐字节相同(本 PR 只触及
同样的代码,不同的宿主,不同的结果。
第 1 次尝试确实证明了本 PR 的一件事: |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Test Plan (not a blocker): test/unit/serve-bridge-bin.test.ts — no such file or directory; @qwen-code/sdk@0.1.8 — no such file or directory.
中文说明
已审查。 建议见行内评论。
Test Plan(非阻断):test/unit/serve-bridge-bin.test.ts — no such file or directory; @qwen-code/sdk@0.1.8 — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| expect(lines[0]).toBe('#!/usr/bin/env node'); | ||
| expect(lines[1]?.startsWith('#!')).toBe(false); | ||
| expect(() => | ||
| execFileSync('node', ['--check', outfile], { stdio: 'pipe' }), |
There was a problem hiding this comment.
[Suggestion] R2-1: The tmpdir node --check resolves the ESM parse goal only via Node's module-syntax detection — no package.json above os.tmpdir() declares "type": "module" — and that detection is default-enabled only from Node 22.7, while the package declares "engines": {"node": ">=22.0.0"}. A contributor running the SDK unit suite on any Node in that band below 22.7 (22.0–22.6) gets SyntaxError: Cannot use import statement outside a module and the test fails even though the built bin is correct. CI pins latest 22.x (.nvmrc = 22, 22.x in ci.yml), so it never surfaces there.
Witness:
Node v22.6.0, freshly esbuild-built tmp bin (the test's exact options and execFileSync call):
node --check -> SyntaxError: Cannot use import statement outside a module (bin.js:9), exit 1
Same Node, {"type": "module"} package.json written beside the file: exit 0
Control Node v22.23.2 on the same bare tmpdir: exit 0
Write a minimal package.json into the temp dir right after mkdtempSync so the module goal is explicit across the whole engines range:
writeFileSync(join(outDir, 'package.json'), '{"type": "module"}\n');The fix must keep the check valid across the whole declared "engines": {"node": ">=22.0.0"} range (packages/sdk-typescript/package.json), and the outfile must stay named bin.js — bin.qwen-serve-mcp maps to ./dist/daemon-mcp/serve-bridge/bin.js, so do not switch the temp outfile to .mjs. If you apply this, please confirm the mutation: 'emits exactly one hashbang, on line 1, and parses' should still pass under --no-experimental-detect-module with the fixture in place, and removing the fixture while detection is off should red it.
中文说明
R2-1:临时目录里的 node --check 只能依靠 Node 的模块语法检测来确定 ESM 解析目标 —— os.tmpdir() 上层没有任何 package.json 声明 "type": "module" —— 而该检测从 Node 22.7 起才默认开启,而包声明的是 "engines": {"node": ">=22.0.0"}。贡献者在低于 22.7 的 Node(22.0–22.6)上跑 SDK 单测时,会得到 SyntaxError: Cannot use import statement outside a module,测试失败,但构建出的 bin 本身完全正确。CI 固定使用最新的 22.x(.nvmrc = 22,ci.yml 用 22.x),所以那里永远看不到这个问题。
验证证据:在真实的 Node v22.6.0 二进制上执行 —— 用测试完全相同的选项和 execFileSync 调用在裸临时目录里新构建出的 bin,node --check 抛出上述 SyntaxError(bin.js:9),exit 1;在文件旁写入 {"type": "module"} 的 package.json 后:exit 0;对照用 Node v22.23.2 跑同一裸临时目录:exit 0。
修复方式:在 mkdtempSync 之后立即向临时目录写入一个最小的 package.json(writeFileSync(join(outDir, 'package.json'), '{"type": "module"}\n')),使模块目标在整个 engines 范围内都是显式确定的。
修复前提:该检查必须在声明的整个 "engines": {"node": ">=22.0.0"} 范围内有效(packages/sdk-typescript/package.json),且输出文件必须仍命名为 bin.js —— bin.qwen-serve-mcp 映射到 ./dist/daemon-mcp/serve-bridge/bin.js,因此不要把临时输出文件改成 .mjs。应用后请做一次突变确认:放入该文件后,即使加 --no-experimental-detect-module,'emits exactly one hashbang, on line 1, and parses' 仍应通过;在关闭检测时移除该文件则应使其变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
There was a problem hiding this comment.
Fixed in 4ebd40bd07 — confirmed, and taken as written: writeFileSync(join(outDir, 'package.json'), '{"type": "module"}\n') right after mkdtempSync, with the outfile still named bin.js so it keeps matching the shipped bin.qwen-serve-mcp mapping.
I reproduced your witness on v22.22.2 by driving the detection toggle directly, and ran the exact mutation you asked for:
ARM 1 patched, detection default 2 passed
ARM 2 patched, NODE_OPTIONS=--no-experimental-detect-module 2 passed
ARM 3 fixture removed, detection still off 'emits exactly one hashbang, on line 1, and parses' FAILS:
SyntaxError: Cannot use import statement outside a module
restored 2 passed
ARM 2 is the pin you asked for (the case passes with the fixture even with detection off), ARM 3 is the red without it. One scope note confirming your analysis: the tmpdir case was the only bare-goal site — the dist/ case and the build guard's own node --check both resolve under packages/sdk-typescript/package.json, which declares "type": "module", so neither needed a change. Full SDK suite after the fix and the main merge: 38 files / 1746 tests green.
中文说明
已在 4ebd40bd07 修复 —— 结论确认,按建议原样采纳:在 mkdtempSync 之后立即 writeFileSync(join(outDir, 'package.json'), '{"type": "module"}\n'),输出文件仍命名为 bin.js,与发布的 bin.qwen-serve-mcp 映射保持一致。
我在 v22.22.2 上直接驱动检测开关复现了你的验证证据,并完成了你要求的突变确认:
ARM 1 已修复,检测默认开启 2 通过
ARM 2 已修复,NODE_OPTIONS=--no-experimental-detect-module 2 通过
ARM 3 移除 fixture,检测保持关闭 'emits exactly one hashbang, on line 1, and parses' 变红:
SyntaxError: Cannot use import statement outside a module
还原 2 通过
ARM 2 正是你要求钉住的点(放入 fixture 后即使关闭检测该用例仍通过),ARM 3 是移除后的变红。一个范围性备注,印证你的分析:tmpdir 用例是唯一缺少显式 module goal 的位置 —— dist/ 用例与构建守卫自身的 node --check 都在 packages/sdk-typescript/package.json(声明了 "type": "module")之下解析,无需改动。修复并合并 main 后 SDK 全量套件:38 文件 / 1746 测试全绿。
The bin test's node --check ran against a bare temp dir, where the ESM
parse goal rests on Node's module-syntax detection — default-enabled only
from 22.7, while the package supports >=22.0.0. On Node 22.0-22.6 the
check dies with 'Cannot use import statement outside a module' even
though the built bin is correct. Write {"type": "module"} beside the
bundle so the goal is explicit across the whole engines range; the
outfile stays bin.js to match the shipped bin mapping.
Verified with --no-experimental-detect-module: the case passes with the
fixture and reds without it.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- Agent 7 probe finding (build.js:234 call-site hunk survives revert) — overlaps the settled R1-2 thread (comment 3886351052); the call-site regression lane is the dist case the test's comments document as the second line of defence
- Agent 7 probe finding (build.js:255 assertExecutableBin not exercised by any test) — overlaps the settled R1-2 thread (comment 3886351052); guard-independence of the suite is the test's documented design premise
Test Plan (not a blocker): test/unit/serve-bridge-bin.test.ts — no such file or directory; @qwen-code/sdk@0.1.8 — no such file or directory; 1707 tests pass — this review observed 1746, 503, 5297, 94 passed.
Convergence: round 3 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 1 (1 new). Findings keep coming back to the same files: packages/sdk-typescript/test/unit/serve-bridge-bin.test.ts (findings in round 2; 1 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
已审查——无阻断问题。 建议见行内评论。
本轮确认的 2 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
Test Plan(非阻断):test/unit/serve-bridge-bin.test.ts — no such file or directory; @qwen-code/sdk@0.1.8 — no such file or directory; 1707 tests pass — this review observed 1746, 503, 5297, 94 passed。
收敛情况:第 3 轮发布了 1 条行内评论,其中 1 条是首次提出;上一轮发布了 1 条(其中 1 条首次提出)。发现反复回到同一批文件:packages/sdk-typescript/test/unit/serve-bridge-bin.test.ts(第 2 轮已出过发现,本轮又有 1 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,或将本 PR 的评审降到 --severity-floor critical,可以避免循环反复推导同一组发现。本轮没有未决的 Critical,因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.3)
| const shippedBinPath = join( | ||
| rootDir, | ||
| 'dist', | ||
| 'daemon-mcp', | ||
| 'serve-bridge', | ||
| 'bin.js', | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R3-1: shippedBinPath restates the bin.qwen-serve-mcp mapping from package.json, and it.skipIf(!existsSync(shippedBinPath)) cannot tell a diverged path from a clean tree. If someone relocates or renames the bin output, updating bin in package.json and serveBridgeBinPath in scripts/build.js consistently but not this constant, existsSync(shippedBinPath) is false forever and 'ships that same shape in dist/' silently skips from then on — the only suite-level check of the actually shipped bytes is gone with no signal, and a banner later re-introduced at the build.js call site ships an unstartable bin exactly like @qwen-code/sdk@0.1.8 did.
Witness:
drift simulation (bin renamed in package.json + build.js, test constant untouched):
test/unit/serve-bridge-bin.test.ts (2 tests | 1 skipped)
Tests 1 passed | 1 skipped (2) <- dist case silently skipped, suite green
same drift state, path derived from package.json:
Tests 2 passed (2)
fix applied + dist/ removed (QWEN_SKIP_PREPARE lane):
Tests 1 passed | 1 skipped (2) <- skip-don't-fail semantics preserved
Derive the path from the manifest instead of restating it:
| const shippedBinPath = join( | |
| rootDir, | |
| 'dist', | |
| 'daemon-mcp', | |
| 'serve-bridge', | |
| 'bin.js', | |
| ); | |
| const pkg = JSON.parse(readFileSync(join(rootDir, 'package.json'), 'utf8')); | |
| const shippedBinPath = join(rootDir, pkg.bin['qwen-serve-mcp']); |
The fix must keep the skip-when-dist/-missing semantics: Dockerfile:30 installs with RUN QWEN_SKIP_PREPARE=1 npm ci (and scripts/prepare.js exits before building), so trees legitimately exist without a built dist/ where this case must skip, not fail. If applied, please confirm the mutation: after deriving the path from package.json, renaming the bin in package.json (with dist/ built) must leave 'ships that same shape in dist/' running, not skipping.
中文说明
R3-1:shippedBinPath 复述了 package.json 里的 bin.qwen-serve-mcp 映射,而 it.skipIf(!existsSync(shippedBinPath)) 无法区分「路径已漂移」与「干净树」。若将来有人移动或重命名 bin 产物,同步更新了 package.json 的 bin 与 scripts/build.js 里的 serveBridgeBinPath,却漏掉这个常量,existsSync(shippedBinPath) 将永远为假,'ships that same shape in dist/' 从此静默跳过——对实际发布字节的唯一套件级检查就在没有任何信号的情况下消失了;此后若有人在 build.js 调用点重新加回 banner,就会像 @qwen-code/sdk@0.1.8 一样再次发布无法启动的 bin。
验证证据(探针,临时树):漂移模拟(在 package.json 与 build.js 中重命名 bin,测试常量不动)→ Tests 1 passed | 1 skipped (2),dist 用例被静默跳过、套件全绿;同一漂移状态下改为从 package.json 推导路径 → Tests 2 passed (2);应用修复并删除 dist/(QWEN_SKIP_PREPARE 场景)→ Tests 1 passed | 1 skipped (2),「跳过而非失败」的语义得以保留。
修复方式:从 package.json 推导该路径,而不是再复述一遍(见上方 suggestion 代码块)。
修复前提:必须保留 dist/ 缺失时跳过的语义——Dockerfile:30 以 RUN QWEN_SKIP_PREPARE=1 npm ci 安装(scripts/prepare.js 会在构建前退出),确实存在没有构建产物的合法树,该用例在那里必须跳过而非失败。若应用此修复,请做一次突变确认:从 package.json 推导路径之后,重命名 package.json 中的 bin(在已构建 dist/ 的情况下),'ships that same shape in dist/' 必须仍在执行而不是跳过。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 92 passed · 0 failed · 92 total Flakiness gate: ✅ 1 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:92 通过 · 0 失败 · 92 总计 抖动门:✅ 1 changed test file(s) x 5 identical rounds, no divergence Verification reportPR 10485 — fix(sdk): stop emitting a duplicated hashbang in the serve-mcp bin (round 3)Verdict: 中文 — 判定:✅ 通过 · 可合入(agent 判定)
Previous-finding status (follow-up round)This is a follow-up round. The head moved from
Central claim + A/BCentral claim (unchanged from prior rounds, re-proven at the new head): the built Witness:
A/B assertions: 19/19 (every base-side red is an encoded expectation, i.e. a pass). Reviewer Test Plan, per step — all four steps were executable and passed:
Secondary claim 1 — the guard is load-bearing, both clauses pinned, including the new option-sharing surface. Witness:
Matrix assertions: 24/24, no surviving mutants. Positive controls: C1/C2 are caught mutants, C0 is the unmutated green. (Unscheduled corroboration: an earlier harness sequencing error left the tree with C5's broken dist, and the dist case immediately red'd against it before a clean rebuild — the case bites on exactly the artifact shape it exists to catch.) Secondary claim 2 — the
Shell-safety assertions: 7/7. Caveat stated plainly: the depth-2 checkout cannot reach Secondary claim 3 — the new test is load-bearing and correct across the engines range (commit
Test-flag assertions: 9/9. Sibling census + accept side (carried over). Witness:
Corrections
Findings
No new blocking findings this round. Not covered
MethodologyEnvironment: CI Flakiness gate logEvidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |














What this PR does
Removes the redundant hashbang
bannerfrom theqwen-serve-mcpesbuild step, and adds a build-time assertion that the produced bin starts with a hashbang and actually parses. Review follow-up (fec89f4a31): the assertion runsnode --checkin argv form rather than through a shell, and the bin's esbuild options moved to a module the build script and a new test share, so the regression is pinned by the suite and not only by the guard.Why it's needed
Fixes #10484.
scripts/build.jspassedbanner: { js: '#!/usr/bin/env node' }for the serve-bridge bin, but esbuild already carries the entry point's own hashbang fromsrc/daemon-mcp/serve-bridge/bin.tsinto the bundle. Stacking the banner on top of it emitted a second hashbang on line 2 ofdist/daemon-mcp/serve-bridge/bin.js. esbuild accepts that; node does not. Node strips only the first hashbang line, so the publishedqwen-serve-mcpbin died withSyntaxError: Invalid or unexpected tokenthrough bothnode <file>and the shebang — it has never been startable, and@qwen-code/sdk@0.1.8ships it broken.Dropping the banner is the smaller half of the fix. The larger half is that nothing in the pipeline ever started the bin:
tsc,eslintand the 1699-test SDK suite all run againstsrc, and the build's existing assertions cover only bundle byte budgets and browser-safety of the library entries. So this also addsassertExecutableBin, matching the existingassert*idiom in that file — it checks line 1 is a hashbang and shells out tonode --check, which is the only oracle that actually catches this class of break.No other build in the repo emits a hashbang banner; every other declared bin already relies on its entry point's own.
Two review findings on that guard are folded in. It ran
node --checkthroughexecSyncwith a command string, so a checkout path containing$(…), a backtick or$VARexpanded under/bin/sh -cand then failed the build with a misleadingdoes not parse— the argv form ofexecFileSyncneeds no quoting at all. And nothing in the suite exercised the guard, so a later edit swallowing its error would leave every test green while the next banner regression publishes.scripts/serve-bridge-bin-build-options.jsnow holds the bin's esbuild options for both the build script andtest/unit/serve-bridge-bin.test.ts, which builds with them and asserts the emitted bytes; a second case pins the shippeddist/artifact, catching a banner re-added at the call site instead.Reviewer Test Plan
How to verify
main, runnpm run build --workspace @qwen-code/sdkandhead -2 packages/sdk-typescript/dist/daemon-mcp/serve-bridge/bin.js. Confirm two hashbang lines, and that running the file fails withSyntaxError.initializeinto the bin over stdio and confirm it answers.banner: { js: '#!/usr/bin/env node' }back and confirm the build now fails withBin … does not parse; then delete the hashbang frombin.tsinstead and confirm it fails withBin … must start with a hashbang line.serveBridgeBinBuildOptionsand confirmvitest run test/unit/serve-bridge-bin.test.tsfails. Then neutralizeassertExecutableBin(if (filePath) return;) and spread a banner over the options at the call site: the build now exits 0 with#!/usr/bin/env nodeon line 2 of the shipped bin, and thedist/case is what reds.Evidence (Before & After)
Before (
origin/main, and the@qwen-code/sdk@0.1.8tarball from npm):After, driving a real MCP handshake over stdio against the built bin, run in place as an executable:
Both negative controls fail the build as intended, and the patch as submitted builds clean.
Tested on
Environment (optional)
Linux, Node v22.22.2.
npm run buildexit 0; full@qwen-code/sdksuite 38 files / 1707 tests pass;eslintat--max-warnings 0over the three changed files exit 0;tsc --noEmitandtsc --noEmit -p tsconfig.test-fence.jsonexit 0;prettier --checkclean.Risk & Scope
node --checkonce per build (a few ms), and the new test bundles the bin once into a temp dir (23 ms — it externalizes@modelcontextprotocol/sdk). If a future bin entry legitimately needs a banner, the guard forces it to keep the output parseable rather than silently shipping a broken file.dist/case skips rather than fails when the artifact is absent.npm cibuilds every workspace through the rootpreparescript, so it runs in CI; lanes that setQWEN_SKIP_PREPARE=1have nodist/, and the primary case does not depend on one.qwen-serve-mcpnow starts.Linked Issues
Fixes #10484
中文说明
本 PR 做了什么
移除
qwen-serve-mcpesbuild 步骤里多余的 hashbangbanner,并新增一条构建期断言:产出的 bin 必须以 hashbang 开头,并且真的能被解析。评审跟进(fec89f4a31):该断言改用 argv 形式执行node --check,不再经过 shell;bin 的 esbuild 选项抽成构建脚本与新测试共享的模块,使这个回归由测试套件钉住,而不只依赖构建期守卫。为什么需要它
修复 #10484。
scripts/build.js给 serve-bridge bin 传了banner: { js: '#!/usr/bin/env node' },但 esbuild 本来就会把入口src/daemon-mcp/serve-bridge/bin.ts自带的 hashbang 带进产物。banner 叠加在上面之后,dist/daemon-mcp/serve-bridge/bin.js的第 2 行就多出了第二个 hashbang。esbuild 接受它,node 不接受。node 只会剥掉第一行 hashbang,所以发布出去的qwen-serve-mcp无论是node <file>还是走 shebang,都会以SyntaxError: Invalid or unexpected token挂掉——它从来就没能启动过,而@qwen-code/sdk@0.1.8发布的就是坏的。去掉 banner 只是修复中较小的一半。更重要的一半是:整条流水线从来没有真的启动过这个 bin——
tsc、eslint和 1699 项 SDK 测试都只跑src,构建里已有的断言只覆盖 bundle 字节预算和库入口的浏览器安全性。所以这里同时加了assertExecutableBin,沿用该文件里已有的assert*写法:检查第 1 行是 hashbang,并调用node --check——这是唯一能抓到这类问题的判据。仓库里没有第二处构建使用 hashbang banner;其他已声明的 bin 都直接依赖各自入口文件自带的 hashbang。
针对该守卫的两条评审意见已一并处理。它此前用字符串形式的
execSync执行node --check,因此含有$(…)、反引号或$VAR的检出路径会在/bin/sh -c下展开,并让构建以误导性的does not parse失败——argv 形式的execFileSync完全不需要引号处理。另外测试套件从未覆盖这道守卫,日后若有改动吞掉它的错误,所有测试仍会全绿,而下一次 banner 回归照样会被发布。现在scripts/serve-bridge-bin-build-options.js承载 bin 的 esbuild 选项,由构建脚本与test/unit/serve-bridge-bin.test.ts共享;该测试用这份选项构建并断言产出的字节,另有一个用例钉住已构建的dist/产物,覆盖 banner 改加在调用点的情形。Reviewer 测试计划
如何验证
main上执行npm run build --workspace @qwen-code/sdk,然后head -2 packages/sdk-typescript/dist/daemon-mcp/serve-bridge/bin.js。确认有两行 hashbang,且运行该文件会报SyntaxError。initialize,确认它能回应。banner: { js: '#!/usr/bin/env node' }加回去,确认构建以Bin … does not parse失败;再改为删掉bin.ts的 hashbang,确认构建以Bin … must start with a hashbang line失败。serveBridgeBinBuildOptions,确认vitest run test/unit/serve-bridge-bin.test.ts失败。再让assertExecutableBin失效(if (filePath) return;),同时在调用点用展开覆写加上 banner:此时构建 exit 0,发布出的 bin 第 2 行是#!/usr/bin/env node,由dist/那个用例变红抓住。证据(修改前与修改后)
修改前(
origin/main,以及 npm 上的@qwen-code/sdk@0.1.8tarball):修改后,把构建出的 bin 原地当作可执行文件运行,跑一次真实的 MCP 握手:
两个反向对照都按预期让构建失败,提交的这版则构建通过。
测试平台
环境(可选)
Linux,Node v22.22.2。
npm run buildexit 0;@qwen-code/sdk全量测试 38 个文件 / 1707 项通过;对三个改动文件执行eslint --max-warnings 0exit 0;tsc --noEmit与tsc --noEmit -p tsconfig.test-fence.jsonexit 0;prettier --check干净。风险与范围
node --check(几毫秒),新测试会把 bin 额外打包到临时目录一次(23 ms —— 它把@modelcontextprotocol/sdk外部化)。如果将来某个 bin 入口确实需要 banner,这道守卫会强制它保证产物仍可解析,而不是悄悄发布一个坏文件。dist/那个用例在产物缺失时是跳过而非失败。npm ci会通过根prepare脚本构建所有 workspace,因此它在 CI 中会实际执行;设置了QWEN_SKIP_PREPARE=1的 lane 没有dist/,而主用例并不依赖构建产物。qwen-serve-mcp现在能启动了。关联 Issue
Fixes #10484