fix(core): bound the workflow meta evaluation - #9136
Conversation
`extractAndStripMeta` evaluates the model-authored `export const meta = {...}`
literal inside a vm realm and then walks the result on the host. Neither half
is bounded, and each one hangs the process on its own:
{ name: (function () { while (true) {} })() } // spins during evaluation
{ get phases() { while (true) {} } } // spins during the walk
The loops are synchronous, so this is not merely slow. The event loop is
blocked outright: no timer fires, no signal handler runs, and nothing left in
the process can cancel it. Running the second literal under vitest does not
even produce a test timeout — the worker's own timeout timer cannot fire.
Run both halves inside the vm, each under a 250ms timeout, which is the only
mechanism that can interrupt synchronous JS from inside the process. The walk
moves into a second vm script that serialises the value to JSON for the host
to parse, so a getter or proxy trap now executes under the same bound as the
literal itself.
The two scripts stay separate programs deliberately. The serializer's source
is fixed and interpolates nothing, so the literal never shares a lexical scope
with it — a getter closes over its own program's scope, not the serializer's.
Folding the literal into the serializer's scope instead would let it read the
helpers and assign to the flag that records whether a thenable was found,
which is the check that keeps a stray rejected Promise from terminating the
host on the next tick. That check keeps its existing behaviour and error text;
it is now performed during the in-vm walk, so `rejectThenablesInMeta` and its
host-side traversal are no longer needed.
The serializer also carries a 64 KiB budget over the values it copies. A
250ms window is long enough to build a very large string, and the JSON round
trip would then copy it twice more.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Problem: real and demonstrated, not theoretical. The PR ships two self-contained reproductions — an evaluation-time loop and a getter that only loops at property-read time — and I confirmed against Direction: aligned. Bounding model-authored code that the workflow runtime already evaluates is squarely the runtime's job, and the direction (bound both halves, inside the vm) is what the #9097 review converged on before that branch failed to converge on implementation details. This PR restarts from Size: core paths touched — 208 production lines ( Approach: scope feels right. The obvious smaller fix (timeout on evaluation only) was already ruled out by the getter case, and the diff is focused — one implementation file, one test file, no drive-by changes at the structure level. The two-program structure (literal and serializer as separate vm scripts so they never share lexical scope) is the part that earns close review, and the PR calls that out itself. Risk: no elevated risk signals from the revert-history path check. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：真实且有论证，不是理论性的。PR 给出了两个可独立运行的复现——求值期死循环、以及只在属性读取时才死循环的 getter。我对照 方向：对齐。对 workflow 运行时已经在求值的模型生成代码加上资源边界，完全属于运行时自身的职责；方向（两个阶段都加边界、放进 vm）也正是 #9097 review 收敛到的结论（那个分支只是实现细节未能收敛）。本 PR 从 规模：触及核心路径——208 行生产代码（ 方案：范围合理。更小的修法（只给求值加超时）已经被 getter 场景排除；diff 也很聚焦——一个实现文件、一个测试文件，结构上没有顺手改动。两个程序的结构（字面量与序列化器分属不同的 vm 脚本、不共享词法作用域）是值得仔细看的地方，PR 自己也指出了这一点。 风险：revert 历史路径检查无升级风险信号。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewI formed my baseline before reading the diff: bound the literal's evaluation with vm's
Two non-blocking observations, neither worth holding the PR:
TestingUnattended CI run — PR code is never executed here; the evidence below is the PR's own CI, read through the API. At review time the primary unit suite was still running, so the table reflects a snapshot and the finalize job will update it when CI settles. What has completed is green (precheck, secret scan, dependency audit, both desktop-shell builds); the macOS/Windows/integration jobs are skipped for this fork PR, which is the normal gating. Not verified: runtime behavior on this commit — the unit job below is the signal for it, and it had not finished at review time. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The added tests assert elapsed-time bounds rather than relying on the runner's timeout, which is the right shape for this failure mode — and per the author's account reverting the fix makes the block hang rather than fail, so the suite structurally pins the change. That "without" half of the A/B is the one thing a single green CI run cannot show. Sandboxed verification would settle it: 中文说明代码审查我在读 diff 之前先形成了自己的基线方案：用 vm 的
两点不阻塞的观察，都不值得卡住这个 PR：
测试无人值守 CI 运行——这里从不执行 PR 代码；以下证据是通过 API 读取的 PR 自身 CI。审查时主单元测试 仍在运行，因此表格是一个快照，finalize 任务会在 CI 结束后更新它。已完成的检查均为绿色（precheck、密钥扫描、依赖审计、两个 desktop-shell 构建）；macOS/Windows/集成任务因 fork PR 门控被跳过，属正常。未验证：该提交上的运行时行为——下表的 unit 任务就是对应信号，审查时它尚未完成。 新增测试断言的是耗时上界而不是依赖测试框架的超时——对这种失败模式来说是正确的形态；按作者的说法，回退修复会让该测试块卡死而非失败，因此测试套件在结构上钉住了这个改动。A/B 中"无修复"的那一半是单次绿色 CI 无法展示的。沙箱验证可以定论： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — a clean, well-argued fix whose design I arrived at independently; the two nits are cosmetic, and the only thing standing between this and approval is the still-running unit suite. Stepping back: the independent proposal I wrote before reading the diff — vm The two reservations from the review are genuinely minor (error-envelope consistency for a contrived patched-stringify case; a slightly overstated comment) and not worth a review round. Approval is deferred only because the primary unit suite has not landed on the reviewed commit yet — approval deferred until CI lands green on 中文说明置信度：4/5 —— 一个干净、论证充分的修复，其设计与我独立推导的方案一致；两个小问题都是表面的，唯一挡在批准前面的，是尚未跑完的单元测试。 退一步看：我在读 diff 之前写下的独立方案——对求值加 vm 审查中的两点保留意见确实都是小问题（一个刻意场景下的错误封装一致性；一条略有夸大的注释），不值得再开一轮 review。批准之所以推迟，只是因为主单元测试尚未在受审提交上出结果。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "This PR bounds both halves of workflow meta extraction…": none — all checks above were completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — all checks above completed; ~9 of ~47 tool calls used.; "This PR bounds both halves of workflow meta extraction…": none — all planned checks completed. (Deliberately not reported as findings: the stale title of the "literal predefines the flag name" test, which the separate ….
Test Plan (not a blocker): 155 passed — this review observed 1429, 19656, 1555, 494, 3561, 560 passed; 641 passed — this review observed 1429, 19656, 1555, 494, 3561, 560 passed.
中文说明
未探索到全部深度（达到工具调用预算）："This PR bounds both halves of workflow meta extraction…"：none — all checks above were completed within budget.；"You are review agent reverse-audit — Reverse audit agent…"：none — all checks above completed; ~9 of ~47 tool calls used.；"This PR bounds both halves of workflow meta extraction…"：none — all planned checks completed. (Deliberately not reported as findings: the stale title of the "literal predefines the flag name" test, which the separate …。
Test Plan（非阻断）：155 passed — this review observed 1429, 19656, 1555, 494, 3561, 560 passed; 641 passed — this review observed 1429, 19656, 1555, 494, 3561, 560 passed。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const jsonStringify = JSON.stringify; | ||
| const push = Array.prototype.push; | ||
| const thenCall = Promise.prototype.then; |
There was a problem hiding this comment.
[Critical] Serializer intrinsics are captured one script too late. Script 2 reads JSON.stringify, Promise.prototype.then, Object.keys, WeakSet.prototype.* and Reflect.apply from the vm realm at its own start — after script 1, the model-authored literal, has run in the same realm and can replace any of them. The docstring claim "a literal that replaced a prototype method before this script ran cannot redirect the walk" is therefore false: capture only defends against mid-walk (getter-fired) tampering. Probe-verified against this commit: (1) Forge — a poisoned JSON.stringify returning a crafted envelope makes the function return attacker-chosen meta that bypasses both the hasThenable gate and validateMeta (a 200 KB forged meta.name was accepted); (2) Blind — a poisoned Object.keys hides a rejecting promise from the walk, the function returns accepted meta, and the dangling rejection then terminates the host process (the base tree survives the same input); (3) Disarm — a poisoned Promise.prototype.then turns the mark-handled call into a no-op. — Failure scenario: meta = { name: (Object.keys = () => ['name','description'], 'x'), description: 'd', extra: Promise.reject(new Error('boom')) } → extraction returns success, then Node's default --unhandled-rejections=throw kills the process, decoupled from the run. Reachable on the run path before the sandbox's 30s body timeout is armed, and from the follow-up callers this PR's test comment anticipates.
Fix: secure the intrinsics before model code runs — a fixed prelude script that Object.freezes JSON, Promise.prototype, WeakSet.prototype, Array.prototype, Object, Reflect in the fresh meta context (contract literals are plain data and need none of these mutable), or run the serializer in a second, fresh context whose intrinsics model code never touched. Correct the docstring claim either way, and add a regression test: a literal that swaps Object.keys and embeds Promise.reject(...) must throw the "must not be Promises" error and leave no unhandled rejection.
中文说明
序列化器的内置对象捕获晚了一个脚本。Script 2 在自身开头从 vm realm 读取 JSON.stringify、Promise.prototype.then、Object.keys、WeakSet.prototype.* 和 Reflect.apply——而此时 script 1（模型编写的字面量）已经在同一个 realm 里运行过，可以替换其中任何一个。因此 docstring 中"在本脚本运行之前替换了原型方法的字面量无法重定向遍历"的说法不成立：捕获只能防御遍历过程中（getter 触发的）篡改。已在本提交上探测验证：(1) 伪造——被污染的 JSON.stringify 返回精心构造的信封，使函数返回同时绕过 hasThenable 检查和 validateMeta 的攻击者自选 meta（一个 200 KB 的伪造 meta.name 被接受）；(2) 致盲——被污染的 Object.keys 使遍历看不到某个正在 reject 的 Promise，函数返回被接受的 meta，随后游离的 rejection 终结宿主进程（base 树在同样输入下存活）；(3) 解除武装——被污染的 Promise.prototype.then 使"标记为已处理"的调用变成空操作。— 故障场景：meta = { name: (Object.keys = () => ['name','description'], 'x'), description: 'd', extra: Promise.reject(new Error('boom')) } → 提取成功返回，随后 Node 默认的 --unhandled-rejections=throw 杀死进程，与本次运行脱钩。该路径在 sandbox 自身的 30s 脚本体超时装配之前即可触达，本 PR 测试注释中预期的后续调用方同样可达。
修复方向：在模型代码运行之前锁定内置对象——用一段固定的前置脚本对新 meta 上下文中的 JSON、Promise.prototype、WeakSet.prototype、Array.prototype、Object、Reflect 执行 Object.freeze（契约字面量是纯数据，不需要它们可变），或者让序列化器运行在第二个全新上下文中（其内置对象从未被模型代码触碰）。无论采用哪种，都请修正 docstring 的说法，并补充回归测试：替换 Object.keys 且内嵌 Promise.reject(...) 的字面量必须抛出 "must not be Promises" 错误且不留下未处理的 rejection。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复：meta 值改由未暴露给模型的 fresh VM realm 序列化，并新增内置对象污染与宿主 helper 暴露回归。验证：workflow-sandbox.test.ts 167/167 通过；npm run build、npm run typecheck 通过。
| ); | ||
| } | ||
|
|
||
| const walked = JSON.parse(String(serialized)) as { |
There was a problem hiding this comment.
[Critical] Host-side coercion of the serializer result can execute model code outside any timeout. JSON.parse(String(serialized)) runs outside the try/catch; with a poisoned JSON.stringify (previous comment) serialized can be an object whose model-authored toString runs during String() coercion on the host thread, where no vm timeout can reach — probe-verified: the process hung until externally killed, while the base tree returns promptly. This reintroduces exactly the unbounded host event-loop wedge this PR exists to remove, on a path before the 30s body timeout and wall-clock watchdog are armed. Non-JSON or null returns additionally escape the mapped error contract (raw SyntaxError / TypeError), and the parsed envelope is never shape-checked (walked.hasThenable on null throws). — Failure scenario: JSON.stringify = () => ({ toString() { while (true) {} } }) → permanent hang of the CLI process; JSON.stringify = () => '@' → raw SyntaxError instead of the contracted malformed-meta error.
Fix:
if (typeof serialized !== 'string') {
throw new Error(
'extractAndStripMeta: failed to evaluate meta object literal: unexpected serializer result',
);
}
const walked = JSON.parse(serialized) as {
hasThenable: boolean;
value: unknown;
};inside the existing try/catch (extend the try block), with a shape check on walked before use — typeof never invokes user code.
中文说明
宿主侧对序列化结果的强制转换可能在任何超时之外执行模型代码。JSON.parse(String(serialized)) 位于 try/catch 之外；在 JSON.stringify 被污染时（见上一条评论），serialized 可以是一个对象，其模型编写的 toString 会在宿主线程的 String() 强制转换中运行，而那里没有任何 vm 超时可达——探测验证：进程挂起直至被外部杀死，而 base 树立即正常返回。这在 30s 脚本体超时和墙钟看门狗装配之前的路径上，重新引入了本 PR 要消除的那类无界宿主事件循环卡死。非 JSON 或 null 返回值还会逃逸映射后的错误契约（裸 SyntaxError / TypeError），且解析出的信封从未做形状检查（对 null 读取 walked.hasThenable 会抛错）。— 故障场景：JSON.stringify = () => ({ toString() { while (true) {} } }) → CLI 进程永久挂起；JSON.stringify = () => '@' → 抛出裸 SyntaxError 而非契约约定的 meta 格式错误。
修复：如上代码，把类型守卫与解析移入现有 try/catch（扩展 try 块），并在使用前检查 walked 的形状——typeof 永远不会调用用户代码。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复：宿主只接受字符串结果，移除 String(serialized) 强制转换，并在 JSON.parse 后校验 envelope 形状。验证：workflow-sandbox.test.ts 167/167 通过；npm run build、npm run typecheck 通过。
| function spend(n) { | ||
| budget -= n; | ||
| if (budget < 0) throw new Error('meta literal is too large'); | ||
| } |
There was a problem hiding this comment.
[Critical] Budget/timeout abort paths leave thenables unmarked — a regression vs the removed walk. spend() throws (and the 250 ms timeout aborts) before the walk reaches later keys, so a thenable sitting after an over-budget field is never marked handled; its rejection then terminates a listener-less host process under Node's default policy. The removed rejectThenablesInMeta had no budget, so it always marked every thenable it could reach before throwing. Probe-verified A/B: this build throws the wrapped size error and then dies with the unhandled rejection (exit 1); the base tree throws "meta values must not be Promises" and survives. — Failure scenario: export const meta = { name: 'x'.repeat(200000), description: 'd', extra: Promise.reject(new Error('boom')) } → the size error surfaces, then the unmarked rejection kills the process, decoupled from the run. Today's sole production caller masks this while adoptionEscapeHook is registered; the follow-up callers named in this PR's own test comment (tool-confirmation dialog, saved-workflow palette) have no such listener.
Fix: on budget overflow, latch a tooLarge flag and keep walking (without accumulating payload) so every reachable thenable is still marked handled and hasThenable still surfaces — walk time is already bounded by META_EVAL_TIMEOUT_MS. Throw the size error host-side afterwards, preferring the thenable error when both apply.
中文说明
预算/超时中断路径会漏标 thenable——相对被删除的遍历是回归。spend() 抛错（或 250ms 超时中断）发生在遍历到达后面的键之前，因此位于超预算字段之后的 thenable 永远不会被标记为已处理；在 Node 默认策略下，它的 rejection 会终结没有监听器的宿主进程。被删除的 rejectThenablesInMeta 没有预算限制，总能在抛错前标记所有可达的 thenable。已通过 A/B 探测验证：本构建抛出包装后的体积错误，随后死于未处理的 rejection（exit 1）；base 树抛出 "meta values must not be Promises" 并存活。— 故障场景：export const meta = { name: 'x'.repeat(200000), description: 'd', extra: Promise.reject(new Error('boom')) } → 体积错误先暴露，随后未标记的 rejection 杀死进程，与本次运行脱钩。当前唯一的生产调用方在 adoptionEscapeHook 注册期间能掩盖该事件；而本 PR 测试注释中点名的后续调用方（工具确认对话框、已保存 workflow 面板）没有这类监听器。
修复：预算溢出时闩住一个 tooLarge 标志并继续遍历（不再累积载荷），使每个可达的 thenable 仍被标记为已处理、hasThenable 仍能上报——遍历耗时已由 META_EVAL_TIMEOUT_MS 框住。之后再在宿主侧抛出体积错误（两者同时成立时优先报 thenable 错误）。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复：大小超限改为锁存后继续扫描，descriptor 预扫描会先标记并处理原生 Promise，Promise 错误优先返回。验证：新增超限字段后置 Promise 回归；workflow-sandbox.test.ts 167/167 通过。
| // Cycles and shared subgraphs both terminate here. | ||
| if (apply(setHas, seen, [value])) return undefined; |
There was a problem hiding this comment.
[Critical] Shared-subgraph regression: a phase object aliased at two array positions was accepted before this diff and is now rejected. The seen-guard returns undefined for any revisited object; inside an array that becomes null, so validateMeta throws "meta.phases entries must be objects" even though every entry the model wrote is a valid object. Probe-verified A/B: the base tree returns {"phases":[{"title":"t"},{"title":"t"}]}; this commit throws. The IIFE-in-literal syntax needed to express this shape is already supported by the existing suite (the cyclic-spread tests use it). — Failure scenario: phases: (function(){ const s = { title: 't' }; return [s, s]; })() → a formerly-valid saved workflow now fails extraction with a misleading error message.
Fix: copy shared nodes again instead of dropping them — e.g. consult the seen-set only for cycle-breaking on the current ancestor path (a path set entered/exited around the recursive copy calls); the timeout and budget already bound the work. If the drop is intentional, skip the element instead of emitting null, and give a message that distinguishes this case.
中文说明
共享子图回归：在两个数组位置引用同一个 phase 对象的字面量，在本 diff 之前被接受，现在会被拒绝。seen 守卫对任何再次访问的对象返回 undefined；在数组中它变成 null，于是 validateMeta 抛出 "meta.phases entries must be objects"——即使模型写的每一项都是合法对象。A/B 探测验证：base 树返回 {"phases":[{"title":"t"},{"title":"t"}]}；本提交抛错。表达这种形状所需的"字面量内 IIFE"语法已被现有测试套件支持（循环引用 spread 测试就在使用）。— 故障场景：phases: (function(){ const s = { title: 't' }; return [s, s]; })() → 原本合法的已保存 workflow 现在提取失败，且错误信息具有误导性。
修复：对共享节点重新拷贝而不是丢弃——例如仅在祖先路径上判环时使用 seen 集合（用一个在递归 copy 调用前后进入/退出的路径集合）；超时和预算已经框住了工作量。如果丢弃是有意为之，请跳过该元素而不是输出 null，并给出能区分这种情况的错误信息。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复：循环检测改为当前递归路径集合，递归返回时删除对象，因此共享子图会在每个位置复制而循环仍被截断。验证：新增共享 phase 别名回归；workflow-sandbox.test.ts 167/167 通过。
| // Functions, symbols and bigints are not contract values; drop them the | ||
| // same way the previous host-side walk did. | ||
| if (type !== 'object') return undefined; |
There was a problem hiding this comment.
[Critical] Function/symbol/bigint values in optional string fields are now silently dropped where the old path threw. The serializer returns undefined for non-contract types, so whenToUse: function(){} or phases[].detail = () => {} disappear from the JSON and validateMeta passes — but the preserved docstring promises "Throws when meta is present but malformed: ... wrong field type", and the new comment's claim that the previous host-side walk dropped them is false (it passed values to validateMeta, which threw). Probe-verified A/B: the base tree throws "meta.whenToUse must be a string"; this commit returns accepted meta with the field silently missing. Required fields still throw (dropping makes them absent) and function-valued array entries still throw (they become null), which is how the divergence stayed hidden. — Failure scenario: a model-authored meta that puts code in whenToUse / phases[].detail / phases[].model used to fail the run with a clear, upstream-aligned error; now the workflow runs with the field silently missing and the author gets no feedback.
Fix: preserve the old strictness — track a dropped-non-contract flag alongside hasThenable (or emit a sentinel) and throw the original "must be a string" errors host-side. If JSON-drop semantics are the intended new contract, correct the comment and the docstring's throw guarantee instead.
中文说明
可选字符串字段中的 function/symbol/bigint 值，旧路径会抛错，现在被静默丢弃。序列化器对非契约类型返回 undefined，于是 whenToUse: function(){} 或 phases[].detail = () => {} 会从 JSON 中消失，validateMeta 就此通过——但保留的 docstring 承诺"meta 存在但格式错误时抛错：……字段类型错误"，且新注释声称"先前宿主侧遍历也是这样丢弃的"并不属实（旧遍历把值传给 validateMeta，由它抛错）。A/B 探测验证：base 树抛出 "meta.whenToUse must be a string"；本提交返回被接受的 meta，字段静默缺失。必填字段仍会抛错（丢弃使其缺失）、数组元素为函数时仍会抛错（变成 null），这正是该分歧未被暴露的原因。— 故障场景：模型把代码写进 whenToUse / phases[].detail / phases[].model 时，过去会以清晰的、与上游一致的错误使运行失败；现在 workflow 照常运行而字段静默缺失，作者得不到任何反馈。
修复：保留旧的严格性——在 hasThenable 旁增加一个"丢弃了非契约值"的标志（或输出哨兵值），在宿主侧抛出原有的 "must be a string" 错误。如果 JSON 丢弃语义就是新的契约，请改为修正注释和 docstring 中的抛错承诺。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复：function、symbol、bigint 不再被静默省略，改为保留可由宿主 shape 校验拒绝的值。验证：新增 whenToUse/detail/model 三类错误类型回归；workflow-sandbox.test.ts 167/167 通过。
| function copy(value) { | ||
| const type = typeof value; | ||
| if (value === null) return null; | ||
| if (type === 'string') { spend(value.length); return value; } |
There was a problem hiding this comment.
[Suggestion] spend() charges pre-escape length, but JSON escaping inflates the serialized payload up to 6x past the cap. Measured on this commit: a NUL-string meta at the exact budget boundary (65,512 chars, budget lands on exactly 0) serializes to 393,101 bytes against the documented 65,536 — ratio 6.00x (control characters and lone surrogates escape to \uXXXX; -Number.MAX_VALUE spends 8 but serializes to ~24 chars). The defense still holds — a hard, small bound — but the constant's comment ("Cap on the serialised meta payload. Bounds what a literal can force the host to retain and re-parse") overstates what actually reaches the host. — Concrete cost: anyone sizing downstream consumers or logs on "64 KiB max" would be wrong by nearly an order of magnitude.
Fix: enforce the cap on the real output — after jsonStringify in the serializer, throw 'meta literal is too large' if the produced string's length exceeds META_SERIALIZED_MAX_CHARS (the true length is known at that point); or charge spend a worst-case factor for strings.
中文说明
spend() 按转义前的长度计费，但 JSON 转义会把序列化载荷放大到上限的至多 6 倍。已在本提交上实测：恰好处于预算边界（65,512 个字符，预算恰好扣到 0）的 NUL 字符串 meta，序列化出 393,101 字节，而文档上限是 65,536——比值 6.00（控制字符和孤立代理项会转义为 \uXXXX；-Number.MAX_VALUE 计费 8 却序列化出约 24 个字符）。防线依然成立——仍是硬且小的上界——但常量的注释（"序列化后 meta 载荷的上限。限制字面量迫使宿主保留并重新解析的量"）夸大了实际到达宿主的量。— 具体代价：任何按"最大 64 KiB"来规划下游消费者或日志的人都会错近一个数量级。
修复：在真实输出上强制上限——在序列化器中 jsonStringify 之后，若生成的字符串长度超过 META_SERIALIZED_MAX_CHARS 则抛出 'meta literal is too large'（此时的真实长度已知）；或对字符串按最坏系数计费。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复：除遍历预算外，现对 JSON.stringify 的实际输出长度再次执行 64 KiB 上限检查。验证：新增 JSON 转义膨胀回归；workflow-sandbox.test.ts 167/167 通过。
| expect(meta?.name).toBe('undefined:undefined'); | ||
| }); | ||
|
|
||
| it('a thenable stays rejected even when the literal predefines the flag name', () => { |
There was a problem hiding this comment.
[Suggestion] The test title promises a scenario its fixture never sets up. 'a thenable stays rejected even when the literal predefines the flag name' contains no hasThenable field — it is a plain thenable case, so the regression its title claims to guard (a model-defined field shadowing the serializer's envelope flag) is untested. Mutant-probe verified: merging the walked value's fields into the envelope (jsonStringify({ hasThenable, ...value })) leaves this test green, while a corrected fixture fails through the mutant — the advertised guard is vacuous as written. — Concrete cost: a future change that lets a model-authored hasThenable: false shadow the envelope flag would silently accept a Promise-bearing meta (thenable field dropped, no error), and this test would pass green through the change.
Fix: put the claimed scenario in the fixture, still asserting /meta values must not be Promises/ — or rename the test to match its body:
const src = `export const meta = { name: 'x', description: 'd', hasThenable: false, phases: Promise.resolve(1) }\nreturn 1`;中文说明
测试标题承诺了其 fixture 并未构造的场景。'a thenable stays rejected even when the literal predefines the flag name' 中没有 hasThenable 字段——它只是一个普通的 thenable 用例，因此标题声称要守护的回归（模型自定义字段遮蔽序列化器信封标志）实际并未被测试。突变体验证：把被遍历值的字段并入信封（jsonStringify({ hasThenable, ...value })）时本测试仍然为绿，而修正后的 fixture 能穿透该突变体使其变红——按现状，这个自称的护栏是空的。— 具体代价：未来若某个改动允许模型编写的 hasThenable: false 遮蔽信封标志，携带 Promise 的 meta 会被静默接受（thenable 字段被丢弃、不报错），而本测试会一路绿灯地放行该改动。
修复：把标题声称的场景放进 fixture，仍然断言 /meta values must not be Promises/——或者把测试改名为与其内容一致。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复：测试 fixture 现显式设置 hasThenable: false，覆盖预定义标志仍不能隐藏 Promise 的场景。验证：workflow-sandbox.test.ts 167/167 通过。
| expect(timed(() => extractAndStripMeta(src))).toBeLessThan(BOUND_MS); | ||
| }); | ||
|
|
||
| it('bounds a getter nested inside phases', () => { |
There was a problem hiding this comment.
[Suggestion] Two wedge shapes named in superseded PR 9097's Critical review have no regression test, though this PR positions itself as the converged replacement of that feedback. Probe-verified both are bounded today: a get then() { while (true) {} } literal throws at ~252 ms via the vm timeout, and the weaponized Symbol.iterator phases payload is disarmed in ~1 ms because the serializer walks arrays by index — this is a missing-test gap, not a live defect. — Concrete cost: the iterator payload is disarmed only by the structural choice to iterate by index; a future edit reintroducing iterator-protocol traversal over vm-realm values (or moving any part of the walk back to the host) would silently restore the unrecoverable event-loop wedge for that shape, with no test to fail.
Fix: add two cases to the bounded evaluation block — one with get then() { while (true) {} } asserting the malformed-meta throw within BOUND_MS, and one with the weaponized-iterator phases payload asserting bounded behaviour (throw or successful extraction) within BOUND_MS.
中文说明
被取代的 PR 9097 的 Critical 评审中点名的两种卡死形态没有回归测试，尽管本 PR 将自己定位为针对该反馈的收敛替代方案。已探测验证两者目前都有界：get then() { while (true) {} } 字面量约 252ms 经 vm 超时抛出；武器化的 Symbol.iterator phases 载荷在约 1ms 内被化解（因为序列化器按索引遍历数组）——这是缺测试的缺口，不是现行缺陷。— 具体代价：迭代器载荷之所以被化解，仅仅依赖于"按索引遍历"这一结构选择；未来若有改动重新引入对 vm realm 值的迭代器协议遍历（或把任何一部分遍历挪回宿主），该形态的不可恢复事件循环卡死会被静默恢复，且没有测试会失败。
修复：在 bounded evaluation 块中补充两个用例——一个用 get then() { while (true) {} } 断言在 BOUND_MS 内抛出 meta 格式错误；一个用武器化迭代器的 phases 载荷断言在 BOUND_MS 内有界结束（抛错或成功提取）。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复：新增无限循环 then getter 的超时回归，以及 phases 自定义 Symbol.iterator 不被调用的回归。验证：workflow-sandbox.test.ts 167/167 通过。
| for (let i = 0; i < value.length; i++) { | ||
| spend(2); | ||
| apply(push, out, [copy(value[i])]); |
There was a problem hiding this comment.
[Suggestion] The per-element spend(2) and per-key spend(key.length + 4) accounting sites have no test; the serialized-size cap is only ever tripped through string length today. Mutant executed on this commit: deleting those two spend calls leaves the existing size-cap test passing while a string-free phases: Array.from({ length: 40000 }, () => ({})) payload (~120 KB, ~1.8x the cap) sails through the budget in 36 ms and is retained and host-parsed before validateMeta rejects its shape. — Concrete cost: that mutant ships a cap that only bounds strings, with no test failing.
Fix: add one string-free cap test, e.g.:
const src = `export const meta = { name: 'x', description: 'd', phases: Array.from({ length: 40000 }, () => ({})) }\nreturn 1`;
expect(() => extractAndStripMeta(src)).toThrow(
/failed to evaluate meta object literal/,
);中文说明
按元素的 spend(2) 与按键的 spend(key.length + 4) 计费点没有测试；目前序列化体积上限只会经由字符串长度被触发。已在本提交上执行突变体：删除这两处 spend 调用后，现有的体积上限测试仍然通过，而一个不含字符串的 phases: Array.from({ length: 40000 }, () => ({})) 载荷（约 120 KB，约为上限的 1.8 倍）在 36ms 内穿过预算，被宿主保留并解析，直到 validateMeta 才拒绝其形状。— 具体代价：该突变体若上线，体积上限就只约束字符串，而没有任何测试失败。
修复：补充一个不含字符串的体积上限测试（示例如上）。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复：容器节点同样计入预算，并新增 40,000 个无字符串对象的 phases 回归。验证：workflow-sandbox.test.ts 167/167 通过。
已被后续 commit 取代，当前 head 需重新 review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "This PR bounds both halves of workflow meta extraction —…": none significant — I did not run the full TS module via vitest (no node_modules in worktree); the harness mirrors the exact runInContext flow and copies the ser…; "This PR bounds both halves of workflow meta extraction —…": none — I did not need to run live probes; every candidate resolved by source inspection against the reviewed commit.; "This PR bounds both halves of workflow meta extraction —…": none — no check was left unfinished at the ceiling. I did not run the package's vitest suite itself, relying instead on direct execution of the same code paths …; "This PR bounds both halves of workflow meta extraction —…": none — all planned checks completed (~20 of ~52 calls used).; "This PR bounds both halves of workflow meta extraction —…": nothing — the walk is complete. Tool calls used: ~13..
Test Plan (not a blocker): 155 passed — this review observed 1429, 19656, 1555, 494, 3561, 560 passed; 641 passed — this review observed 1429, 19656, 1555, 494, 3561, 560 passed.
中文说明
未探索到全部深度（达到工具调用预算）："This PR bounds both halves of workflow meta extraction —…"：none significant — I did not run the full TS module via vitest (no node_modules in worktree); the harness mirrors the exact runInContext flow and copies the ser…；"This PR bounds both halves of workflow meta extraction —…"：none — I did not need to run live probes; every candidate resolved by source inspection against the reviewed commit.；"This PR bounds both halves of workflow meta extraction —…"：none — no check was left unfinished at the ceiling. I did not run the package's vitest suite itself, relying instead on direct execution of the same code paths …；"This PR bounds both halves of workflow meta extraction —…"：none — all planned checks completed (~20 of ~52 calls used).；"This PR bounds both halves of workflow meta extraction —…"：nothing — the walk is complete. Tool calls used: ~13.。
Test Plan（非阻断）：155 passed — this review observed 1429, 19656, 1555, 494, 3561, 560 passed; 641 passed — this review observed 1429, 19656, 1555, 494, 3561, 560 passed。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const source = globalThis[${JSON.stringify(META_SLOT)}]; | ||
| markPromises(source); | ||
| const value = copy(source); |
There was a problem hiding this comment.
[Critical] R2-1: Dangling-rejection escape — model-created rejecting Promises can escape the serializer's marking on the vm-timeout/abort path (and through hidden-thenable entrances), so after the bounded error surfaces, Node's default --unhandled-rejections=throw terminates the host process. This is the round-1 R1-3 failure class surviving on the timeout path: the fix (latch tooLarge + markPromises pre-scan) closes the budget abort, but a vm timeout still aborts the scripts before every reachable Promise is marked — contradicting the invariant comment above ("The serializer marks any thenable it reaches as handled inside the vm") and the PR's own contract ("must surface as an ordinary malformed-meta error"). — Failure scenario: probe-verified at this commit (every shape exits 1): (1) { name: (function () { Promise.reject(new Error("boom")); while (true) {} })(), description: "d" } — script 1 times out at 250ms, script 2 never runs, the rejection is unmarked, the process dies on the next tick (newly shaped by this PR — pre-PR it hung forever); (2) a Proxy getOwnPropertyDescriptor trap eats the serializer timeout inside markPromises before a sibling extra: Promise.reject(...) key is reached; (3) a getter materializes a rejection then throws/loops during copy — both variants kill the process. Hidden-thenable entrances inherited from the removed walk but re-asserted by the new comment are equally fatal: symbol-keyed and prototype-chain Promises (meta silently ACCEPTED, then dies), species sabotage and proxy-wrapped Promises (meta rejected correctly, host still dies). Today's sole caller survives only because adoptionEscapeHook happens to be registered on unhandledRejection; the follow-up callers this PR's own test comment anticipates (tool-confirmation dialog, saved-workflow palette) have no listener.
Fix direction (design decision needed): make the marking abort-resilient (catch per-property read errors in copy and continue the sweep; iterate Reflect.ownKeys in markPromises for symbol keys — noting it does not cover function-carried Promises, which early-return before key iteration), and/or install a scoped unhandledRejection safety net around extractAndStripMeta (removed in finally), or neutralise Promise in metaContext since the contract rejects Promises anyway. Add regressions asserting both the throw AND no unhandled rejection on the following tick.
中文说明
[Critical] R2-1：游离 rejection 逃逸——模型创建的 rejected Promise 可以在 vm 超时/中断路径上（以及一些隐藏的 thenable 入口）逃过序列化器的标记，于是在有界错误暴露之后，Node 默认的 --unhandled-rejections=throw 会终结宿主进程。这是第一轮 R1-3 故障类别在超时路径上的残留：修复（闩住 tooLarge + markPromises 预扫描）堵住了预算中断路径，但 vm 超时仍会在所有可达 Promise 被标记之前中断脚本——与上方的不变量注释（"序列化器会把它触达的任何 thenable 在 vm 内标记为已处理"）以及本 PR 自己的契约（"必须以常规的 meta 格式错误暴露"）相矛盾。— 故障场景：已在本提交上探测验证（每种形状都以 exit 1 终结）：(1) { name: (function () { Promise.reject(new Error("boom")); while (true) {} })(), description: "d" } —— script 1 在 250ms 超时，script 2 永不运行，rejection 未被标记，进程在下一个 tick 死亡（该形状由本 PR 新塑造——PR 之前是永久挂起）；(2) Proxy 的 getOwnPropertyDescriptor 陷阱在 markPromises 内部吃光序列化器超时，导致后面的 extra: Promise.reject(...) 键未被触达；(3) getter 在 copy 期间物化一个 rejection 然后抛错/死循环——两种变体都杀死进程。继承自被删除遍历、但被新注释重新断言的隐藏 thenable 入口同样致命：symbol 键与原型链上的 Promise（meta 被静默接受，随后进程死亡）、species 破坏与 proxy 包裹的 Promise（meta 被正确拒绝，宿主仍然死亡）。当前唯一调用方侥幸存活仅因为 adoptionEscapeHook 恰好注册在 unhandledRejection 上；本 PR 测试注释中预期的后续调用方（工具确认对话框、已保存 workflow 面板）没有这类监听器。
修复方向（需要设计决策）：让标记对中断有韧性（在 copy 中捕获逐属性读取错误并继续扫描；在 markPromises 中改用 Reflect.ownKeys 以覆盖 symbol 键——注意它仍不覆盖函数携带的 Promise，因为函数在键迭代前就提前返回），和/或在 extractAndStripMeta 周围安装一个作用域内的 unhandledRejection 安全网（在 finally 中移除），或者既然契约本来就拒绝 Promise，直接在 metaContext 中中和 Promise。补充回归测试：断言抛错的同时，下一个 tick 没有未处理的 rejection。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复。验证证据：workflow-sandbox 183/183 通过；5 个 abort-path 与 6 个隐藏 Promise 回归通过，移除 rejection 观察的变异会稳定 RED。
| typeof (parsed as { tooLarge?: unknown }).tooLarge !== 'boolean' || | ||
| !Object.hasOwn(parsed, 'value') |
There was a problem hiding this comment.
[Suggestion] R2-2: A top-level thenable-shaped meta literal trips this envelope shape check ("unexpected serializer result") instead of the contracted "meta values must not be Promises" error: copy() returns undefined for the root thenable, JSON.stringify drops the undefined-valued value key, so Object.hasOwn(parsed, 'value') fails before walked.hasThenable is consulted. The PR description promises the thenable check "keeps its existing behaviour and error text"; nested thenables still produce the correct error, only the top-level shape is affected. The meta is still rejected (fails closed) — this is a misleading-diagnostic defect, not an acceptance defect. — Failure scenario: probe-verified: export const meta = { then: () => {}, name: 'x', description: 'd' } (and the get then() variant) throws ...failed to evaluate meta object literal: unexpected serializer result at this commit; the base tree threw the specific Promise error.
| typeof (parsed as { tooLarge?: unknown }).tooLarge !== 'boolean' || | |
| !Object.hasOwn(parsed, 'value') | |
| typeof (parsed as { tooLarge?: unknown }).tooLarge !== 'boolean' |
(plus keep the value requirement, but check walked.hasThenable/tooLarge first — or normalize in the serializer: value: value === undefined ? null : value.)
中文说明
[Suggestion] R2-2：顶层 thenable 形状的 meta 字面量会触发这里的信封形状检查（"unexpected serializer result"），而不是契约约定的 "meta values must not be Promises" 错误：copy() 对顶层 thenable 返回 undefined，JSON.stringify 丢弃值为 undefined 的 value 键，于是 Object.hasOwn(parsed, 'value') 在检查 walked.hasThenable 之前就失败了。PR 描述承诺 thenable 检查"保持原有行为和错误文案"；嵌套 thenable 仍产生正确错误，只有顶层形状受影响。meta 仍会被拒绝（失败关闭）——这是误导性诊断缺陷，不是接受缺陷。— 故障场景：已探测验证：export const meta = { then: () => {}, name: 'x', description: 'd' }（以及 get then() 变体）在本提交上抛出 ...failed to evaluate meta object literal: unexpected serializer result；base 树抛出的是具体的 Promise 错误。
修复：先检查 walked.hasThenable/tooLarge 再要求 value 键；或在序列化器中归一化：value: value === undefined ? null : value。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复。验证证据：workflow-sandbox 183/183 通过；top-level thenable 现在命中 Promise 诊断回归。
| { timeout: META_EVAL_TIMEOUT_MS }, | ||
| ); | ||
| } catch (e) { | ||
| const msg = e instanceof Error ? e.message : 'unknown error'; |
There was a problem hiding this comment.
[Suggestion] R2-3: Errors thrown from a vm realm fail the host-realm e instanceof Error check, so this 'unknown error' fallback (which replaced the pre-diff String(e), here and in the second catch) erases the cause text of every vm-realm failure — including this PR's own timeout error and any model-thrown Error. — Failure scenario: probe-verified: the vm timeout error has .message === "Script execution timed out after 250ms" but instanceof Error === false host-side, and { name: doesNotExist, ... } surfaces ...: unknown error (the base tree produced ReferenceError: doesNotExist is not defined); a model-thrown new Error('helpful diagnostic') likewise vanishes. The diagnostic for the exact wedge case this PR exists to handle is lost, and the model's self-correction loop gets nothing to act on.
Fix: extract the message without executing model code on the host — a duck-typed .message read can run a model-authored accessor if the model throws an object with a getter message (verifier-observed), so extract inside the vm (a small fixed script under timeout) or use util.inspect. Do NOT restore bare String(e) — a model toString wedges the host thread, the hazard class this PR removes.
中文说明
[Suggestion] R2-3：从 vm realm 抛出的错误无法通过宿主 realm 的 e instanceof Error 检查，因此这里的 'unknown error' 兜底（取代了 diff 之前的 String(e)，第二处 catch 同样如此）会抹掉所有 vm realm 失败的成因文本——包括本 PR 自己的超时错误和模型抛出的任何 Error。— 故障场景：已探测验证：vm 超时错误的 .message === "Script execution timed out after 250ms"，但在宿主侧 instanceof Error === false；{ name: doesNotExist, ... } 显示为 ...: unknown error（base 树产生 ReferenceError: doesNotExist is not defined）；模型抛出的 new Error('helpful diagnostic') 同样消失。本 PR 要处理的那个卡死场景的诊断信息就此丢失，模型的自我纠错循环得不到任何可行动的信息。
修复：在不于宿主执行模型代码的前提下提取消息——鸭子类型读 .message 也可能运行模型编写的 accessor（若模型抛出带 getter message 的对象，验证者已观察到），因此应在 vm 内提取（一段固定的、受超时约束的小脚本）或使用 util.inspect。不要恢复裸 String(e)——模型的 toString 会卡死宿主线程，那正是本 PR 要消除的故障类别。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复。验证证据：workflow-sandbox 183/183 通过；跨 realm error message 回归断言精确错误文本。
| const serializerGlobal = arguments.callee.caller.constructor('return globalThis')(); | ||
| if (serializerGlobal.__qwenWorkflowMetaIsPromise) throw new Error('host helper exposed'); |
There was a problem hiding this comment.
[Suggestion] R2-4: This isolation probe checks __qwenWorkflowMetaIsPromise, an identifier that exists nowhere in the round-2 implementation — grep confirms the only __qwenWorkflowMeta* names are this test line and META_SLOT = '__qwenWorkflowMetaValue' (workflow-sandbox.ts:412) — so the detection branch is unreachable and the test passes regardless of what the serializer exposes. Probe-verified: the serializer globalThis IS reachable through this exact escape chain and the real slot IS visible there; the stale identifier is always undefined. A future change re-exposing serializer state on the serializer global (or making the slot writable/configurable) ships green. — Concrete cost: this reads as the security regression test for serializer-scope isolation, so an exposure regression would be certified by a green run of exactly this test. Note for the fix: accessor-shorthand getters cannot run this chain (their .caller access throws); a re-anchored probe must keep the function-expression getter shape.
| const serializerGlobal = arguments.callee.caller.constructor('return globalThis')(); | |
| if (serializerGlobal.__qwenWorkflowMetaIsPromise) throw new Error('host helper exposed'); | |
| if (typeof serializerGlobal.copy !== 'undefined' || typeof serializerGlobal.__qwenWorkflowMetaValue !== 'undefined') throw new Error('host helper exposed'); |
(or delete the stale probe if the adjacent scope-isolation tests are deemed sufficient.)
中文说明
[Suggestion] R2-4：这个隔离探测检查的是 __qwenWorkflowMetaIsPromise——该标识符在第二轮实现中任何地方都不存在——grep 确认仅有的两个 __qwenWorkflowMeta* 名称是本测试行和 META_SLOT = '__qwenWorkflowMetaValue'（workflow-sandbox.ts:412）——因此检测分支不可达，无论序列化器暴露什么，测试都会通过。已探测验证：序列化器的 globalThis 通过这条逃逸链确实可达，真实 slot 在那里可见；而这个过时的标识符始终是 undefined。未来若有改动重新把序列化器状态暴露到序列化器 global 上（或把 slot 变为可写/可配置），该测试仍会绿灯通过。— 具体代价：它看起来是序列化器作用域隔离的安全回归测试，因此一次暴露回归恰恰会以本测试的绿灯获得认证。修复注意：accessor 简写 getter 无法运行这条链（其 .caller 访问会抛错）；重新锚定的探测必须保留函数表达式 getter 形状。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复。验证证据：workflow-sandbox 183/183 通过；隔离探测现检查 serializer realm 的 copy helper。
| * Fixed source for the meta serializer. Interpolates NOTHING — the model's | ||
| * literal is evaluated by a separate program (see `extractAndStripMeta`), so |
There was a problem hiding this comment.
[Suggestion] R2-5: This comment claims the serializer source "Interpolates NOTHING", but the template literal below interpolates three times (${META_SERIALIZED_MAX_CHARS} twice, ${JSON.stringify(META_SLOT)} once); the extractAndStripMeta docstring repeats the same false claim ("the copy helper's source is fixed and interpolates nothing"). The load-bearing invariant is "no MODEL-AUTHORED source is ever interpolated". — Failure scenario: a maintainer auditing the isolation guarantee sees the stated invariant contradicted two lines down and can no longer tell which claim matters; a future edit interpolating a value derived from metaSource (an error detail, an option) would cross no alarm, because the stated invariant is already visibly false.
| * Fixed source for the meta serializer. Interpolates NOTHING — the model's | |
| * literal is evaluated by a separate program (see `extractAndStripMeta`), so | |
| * Fixed source for the meta serializer. Interpolates only fixed host constants | |
| * (the size cap and the slot name) — never model-authored source; that is what |
(apply the same rewording to the docstring.)
中文说明
[Suggestion] R2-5：该注释声称序列化器源码"不做任何插值"，但其下方的模板字面量实际插值了三次（${META_SERIALIZED_MAX_CHARS} 两次、${JSON.stringify(META_SLOT)} 一次）；extractAndStripMeta 的 docstring 重复了同样的不实说法（"拷贝辅助源码是固定的、不插值"）。真正承重的不变量是"永不插值模型编写的源码"。— 故障场景：审计隔离保证的维护者看到所声明的不变量在两行之下就被反驳，无法再判断哪个说法才承重；未来某次编辑插值一个源自 metaSource 的值（错误细节、选项）不会触发任何警报，因为所声明的不变量早已 visibly 不成立。
修复：改为"仅插值固定的宿主常量（体积上限与 slot 名）——绝不插值模型编写的源码；正是这一点让字面量远离本作用域"，并在 docstring 同步修改。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复。验证证据：workflow-sandbox 183/183 通过；两处注释已与固定 host 常量插值行为对齐。
| const key = keys[i]; | ||
| const keep = spend(key.length + 4); | ||
| const copied = copy(value[key]); |
There was a problem hiding this comment.
[Suggestion] R2-6: copy keeps recursing through the entire graph after the size budget is exhausted (spend becomes a no-op returning false but recursion continues), turning a clean "meta literal is too large" rejection into a 250ms script-2 vm timeout with a misleading error, and every oversized meta burns the full second 250ms budget. — Failure scenario: probe-verified: extra: new Array(1e8) (holey, cheap to build in script 1) surfaces "Script execution timed out after 250ms" (with R2-3's erasure, even that text is lost) instead of "meta literal is too large". The existing cap test passes only because its 40k-element graph finishes the post-exhaustion walk inside 250ms.
Fix (verifier self-checked on this commit): an early-return at the top of copy alone does NOT flip the holey-array case — the iteration loops must also bail: add if (tooLarge) break; in the array branch (and the object branch). With both, the probe flips to "meta literal is too large" at ~7ms, Promise-error precedence is preserved, and all existing tests stay green. This is safe because markPromises has already completed and getter-deferred Promises copy never reaches are never created.
中文说明
[Suggestion] R2-6：体积预算耗尽后 copy 仍继续递归遍历整个图（spend 变成返回 false 的空操作，但递归不停），把干净的 "meta literal is too large" 拒绝变成 250ms script-2 vm 超时和误导性错误，且每个超尺寸 meta 都烧满第二个 250ms 预算。— 故障场景：已探测验证：extra: new Array(1e8)（稀疏数组，在 script 1 中构造开销极低）暴露 "Script execution timed out after 250ms"（叠加 R2-3 的抹除，连这段文案都看不到）而非 "meta literal is too large"。现有上限测试通过仅因其 4 万元素的图能在 250ms 内走完耗尽后的遍历。
修复（验证者已在本提交上自检）：仅在 copy 顶部提前返回并不能翻转稀疏数组场景——迭代循环也必须退出：在数组分支（及对象分支）加 if (tooLarge) break;。两者都加后，探测翻转为 ~7ms 返回 "meta literal is too large"，Promise 错误优先级保持，现有测试全部仍绿。这样做是安全的：markPromises 已经跑完，而 copy 永远触达不到的 getter 延迟 Promise 也永远不会被创建。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复。验证证据：workflow-sandbox 183/183 通过；holey array 超预算回归通过，移除提前终止的变异会稳定 RED。
| const descriptor = descriptors[keys[i]]; | ||
| if ('value' in descriptor) markPromises(descriptor.value); |
There was a problem hiding this comment.
[Suggestion] R2-7: This sweep — the one thing that reaches Promises hidden in non-enumerable data properties, invisible to copy's enumerable objectKeys walk — has no test. The branch works today (probe: extra: Object.defineProperty({}, 'h', { value: Promise.reject(new Error('nonenum')) }) correctly throws "meta values must not be Promises"), but the executed mutation "align markPromises with copy's enumerable objectKeys walk" flips the probe to meta ACCEPTED + process exit 1 while the entire extractAndStripMeta suite (37 tests) stays green. — Failure scenario: that symmetric-looking refactor is plausible future cleanup; once shipped, a rejected Promise parked in a non-enumerable own property slips past both walkers, leaving the dangling rejection that terminates the host process — the P4a R3 crash reintroduced with no red test.
Fix: add a test — a meta literal with extra: Object.defineProperty({}, 'h', { value: Promise.reject(new Error('x')) }) asserting /meta values must not be Promises/.
中文说明
[Suggestion] R2-7：这条扫描——唯一能触达隐藏在不可枚举数据属性中的 Promise 的机制（copy 的可枚举 objectKeys 遍历看不到它们）——没有测试。该分支目前是有效的（探测：extra: Object.defineProperty({}, 'h', { value: Promise.reject(new Error('nonenum')) }) 正确抛出 "meta values must not be Promises"），但实际执行的变异"让 markPromises 与 copy 的可枚举 objectKeys 遍历对齐"会把探测翻转为 meta 被接受 + 进程 exit 1，而整个 extractAndStripMeta 套件（37 项）依然全绿。— 故障场景：这种看起来对称的重构是可信的未来清理；一旦上线，停放在不可枚举自有属性中的 rejected Promise 会同时躲过两个遍历器，留下终结宿主进程的游离 rejection——P4a R3 崩溃在没有任何红色测试的情况下被重新引入。
修复：补一个测试——meta 字面量含 extra: Object.defineProperty({}, 'h', { value: Promise.reject(new Error('x')) })，断言 /meta values must not be Promises/。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复。验证证据：workflow-sandbox 183/183 通过；non-enumerable、symbol、prototype 与 function property Promise 回归均通过。
| let serialized = jsonStringify({ | ||
| hasThenable: hasThenable, | ||
| tooLarge: tooLarge, | ||
| value: value, | ||
| }); |
There was a problem hiding this comment.
[Critical] R2-8: Envelope forgery — a model-authored getter invoked during the walk reaches the serializer realm's globalThis via the arguments.callee.caller.constructor('return globalThis')() chain (the same chain the test below this block exercises) and installs Object.prototype.toJSON, forging the envelope the host trusts. This defeats BOTH the thenable rejection (hasThenable) and the 64 KiB size budget (tooLarge), against the docstring's promise that the serializer realm's intrinsics "have never been exposed to the model-authored program". The captured jsonStringify binding is immune to reassignment but irrelevant — JSON.stringify consults toJSON through the prototype chain of the envelope literal being stringified; the post-stringify length re-check re-stringifies through the same polluted toJSON; and the host performs no length check before JSON.parse. — Failure scenario: probe-verified twice against this commit (with a distinguishing control): (1) thenable-rejection bypass — phases: Promise.resolve(1) plus a getter installing toJSON returning { hasThenable: false, tooLarge: false, value: { name: 'forged', description: 'forged' } } → meta ACCEPTED with fully attacker-fabricated content, where the contract demands "meta values must not be Promises"; (2) size-budget bypass — forged whenToUse = 'A'.repeat(10*1024*1024) → ACCEPTED, 10,485,760 chars against the 65,536 cap (160x), parsed and retained by the host in <100ms — exactly the memory exhaustion META_SERIALIZED_MAX_CHARS exists to prevent.
Fix (both, verifier flip-tested on this commit): (a) prepend Object.freeze(Object.prototype); Object.freeze(Array.prototype); to META_SERIALIZE_SOURCE — all attack arms flip to rejection and the full 167-test suite stays green (the serializer only creates null-prototyped objects and arrays, so freezing breaks nothing); (b) defense in depth — host-side serialized.length > META_SERIALIZED_MAX_CHARS check before JSON.parse (closes the size arm only; a small forged envelope passes any length check). Add a regression using the getter shape above asserting rejection and that no >64 KiB payload is ever accepted.
中文说明
[Critical] R2-8：信封伪造——遍历期间被调用的模型 getter 通过 arguments.callee.caller.constructor('return globalThis')() 链（与本代码块下方测试所用的同一条链）触达序列化器 realm 的 globalThis，并安装 Object.prototype.toJSON，伪造宿主信任的信封。这同时绕过了 thenable 拒绝（hasThenable）与 64 KiB 体积预算（tooLarge），与 docstring 中"序列化器 realm 的内置对象从未暴露给模型程序"的承诺相悖。捕获的 jsonStringify 绑定虽无法被重新赋值，但与此无关——JSON.stringify 会经由被序列化信封字面量的原型链查询 toJSON；字符串化之后的长度复检同样经由被污染的 toJSON 重新序列化；而宿主在 JSON.parse 之前没有任何长度检查。— 故障场景：已在本提交上两次探测验证（带区分性对照）：(1) thenable 拒绝绕过——phases: Promise.resolve(1) 加上一个安装 toJSON 返回 { hasThenable: false, tooLarge: false, value: { name: 'forged', description: 'forged' } } 的 getter → meta 被接受且内容完全由攻击者伪造，而契约要求抛出 "meta values must not be Promises"；(2) 体积预算绕过——伪造 whenToUse = 'A'.repeat(10*1024*1024) → 被接受，10,485,760 字符对 65,536 上限（160 倍），宿主在 100ms 内解析并驻留——正是 META_SERIALIZED_MAX_CHARS 要防止的内存耗尽。
修复（两者都要，验证者已在本提交上做翻转测试）：(a) 在 META_SERIALIZE_SOURCE 前置 Object.freeze(Object.prototype); Object.freeze(Array.prototype);——所有攻击臂翻转为拒绝，167 项测试全绿（序列化器只创建空原型对象与数组，冻结不破坏任何路径）；(b) 纵深防御——宿主在 JSON.parse 之前检查 serialized.length > META_SERIALIZED_MAX_CHARS（只堵住体积臂；小体积伪造信封能通过任何长度检查）。用上述 getter 形状补回归：断言被拒绝，且任何 >64 KiB 载荷都不被接受。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复。验证证据：workflow-sandbox 183/183 通过；envelope forgery 与 oversize 回归通过，移除 prototype 冻结的变异会稳定 RED。
已被后续 commit 取代，当前 head 需重新 review
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 Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "You are review agent reverse-audit — Reverse audit agent…": (none — all planned checks completed; ~14 of ~59 tool calls used); "Context: PR #9136 (QwenLM/qwen-code) moves both halves of…": none — all planned checks completed (~12 of ~56 calls)..
Test Plan (not a blocker): 155 passed — this review observed 1429, 19656, 1555, 494, 3561, 560 passed; 641 passed — this review observed 1429, 19656, 1555, 494, 3561, 560 passed.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："You are review agent reverse-audit — Reverse audit agent…"：(none — all planned checks completed; ~14 of ~59 tool calls used)；"Context: PR #9136 (QwenLM/qwen-code) moves both halves of…"：none — all planned checks completed (~12 of ~56 calls).。
Test Plan（非阻断）：155 passed — this review observed 1429, 19656, 1555, 494, 3561, 560 passed; 641 passed — this review observed 1429, 19656, 1555, 494, 3561, 560 passed。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| hook.enable(); | ||
| try { | ||
| return run(); | ||
| } finally { | ||
| hook.disable(); | ||
| } |
There was a problem hiding this comment.
[Critical] R3-1: Microtask-deferred rejection escape — observePromiseRejections only marks Promises created inside the synchronous hook window, but the vm realm's microtask queue drains only AFTER hook.disable() runs in this synchronous finally. A rejection whose Promise is created in a .then callback or await continuation escapes the handled-marking entirely. — Failure scenario: meta = { name: (Promise.resolve().then(() => { Promise.reject(new Error('boom')); }), 'x'), description: 'd' } (and the await variant) → extractAndStripMeta SUCCEEDS with valid meta, then the unmarked rejection terminates the host under Node's default --unhandled-rejections=throw on a later tick — probe-verified end-to-end for four deferred shapes (plain .then, await, chained .then, and one deferred during the serializer walk). Today's sole caller run() masks the kill only because adoptionEscapeHook happens to be registered; the follow-up callers this PR's own test comment names (tool-confirmation dialog, saved-workflow palette) have no listener. None of the 13 new dangling-rejection tests covers deferred creation. This is the R2-1 family's remaining sibling entrance: synchronous creations are now marked, microtask-deferred ones are not.
Fix direction (flip-tested): keep the hook enabled until the first macrotask boundary (disable via setImmediate) — but a shipping fix needs a single shared/refcounted hook: the naive per-call deferred-disable OOMed the vitest worker when consecutive synchronous extractions stacked independently-enabled hooks. Add regressions for both deferred shapes asserting the throw AND no unhandled rejection on the next tick.
中文说明
[Critical] R3-1：微任务延迟的 rejection 逃逸——observePromiseRejections 只标记在同步 hook 窗口内创建的 Promise，但 vm realm 的微任务队列只会在同步 finally 里的 hook.disable() 执行之后才排空。在 .then 回调或 await 续体中才创建的 Promise，其 rejection 完全逃过标记。— 故障场景：meta = { name: (Promise.resolve().then(() => { Promise.reject(new Error('boom')); }), 'x'), description: 'd' }（以及 await 变体）→ extractAndStripMeta 成功返回合法 meta，随后未标记的 rejection 在 Node 默认 --unhandled-rejections=throw 下于后续某个 tick 终结宿主进程——四种延迟形状（裸 .then、await、两级链式 .then、序列化遍历期间经 getter 延迟创建）均已端到端探测验证。当前唯一调用方 run() 能掩盖该进程杀死仅因 adoptionEscapeHook 恰好已注册；本 PR 测试注释中点名的后续调用方（工具确认对话框、已保存 workflow 面板）没有监听器。新增的 13 个游离 rejection 测试全部同步创建 rejection，无一覆盖延迟创建。这是 R2-1 故障家族残留的兄弟入口：同步创建已被标记，微任务延迟创建仍未被标记。
修复方向（已做翻转测试）：把 hook 保持启用到第一个宏任务边界（经 setImmediate 禁用）——但落地实现需要单一共享/引用计数的 hook：朴素的“每次调用各自延迟禁用”会在连续同步调用时叠出多个独立启用的 hook，vitest worker 因此 OOM。补两个延迟形状的回归：断言抛错且下一个 tick 无未处理 rejection。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复：VM 微任务在 hook 关闭前于有界 runInContext 内排空，延迟创建的 Promise 也会在创建时标记并拒绝。验证证据：then callback 与 await continuation 回归通过；workflow-sandbox.test.ts 190/190 通过。
| raw = new vm.Script(`(${metaSource})`).runInContext(metaContext, { | ||
| timeout: META_EVAL_TIMEOUT_MS, | ||
| }); |
There was a problem hiding this comment.
[Critical] R3-2: Microtask-deferred infinite loop escapes the vm timeout entirely — the vm watchdog only interrupts code executing INSIDE runInContext; a loop deferred into a .then callback or await continuation runs after runInContext returns, outside every bound this PR installs, and wedges the host event loop permanently while extraction ACCEPTS the meta. — Failure scenario: meta = { name: (Promise.resolve().then(() => { while (true) {} }), 'x'), description: 'd' } → runInContext returns in ~0ms, extractAndStripMeta returns {name:'x',description:'d'} successfully, then at the next microtask checkpoint the loop blocks the event loop forever — the probe process had to be killed at 12s, a registered setTimeout never firing. Verified across four shapes: plain .then, deferral from a getter walked by script 2, async/await, and a hostile message getter reaching the error-formatting script (even a FAILED extraction still wedges). Freezing Promise.prototype in the meta realm does not close it. The wedge lands before the run path's 30s body timeout and wall-clock watchdog are armed, and falsifies the PR's headline contract — "A meta literal that never returns now surfaces as the ordinary malformed-meta error instead of wedging the process" — for a reachable model-authored shape. Distinct from R3-1: no rejection is involved; R3-1's fix does not close this.
No in-process closure exists (the vm watchdog cannot reach code outside runInContext; no API drains queued microtasks). A real bound needs the extraction to run in a worker_threads worker / child process terminate()d on timeout. If the residual is accepted for now, the META_EVAL_TIMEOUT_MS rationale comment and the PR description must state explicitly that the bound covers synchronous work only — deferred work can still wedge.
中文说明
[Critical] R3-2：微任务延迟的无限循环完全逃过 vm 超时——vm 看门狗只能中断在 runInContext 内部执行的代码；推迟到 .then 回调或 await 续体里的循环会在 runInContext 返回之后才运行，处在本 PR 装配的一切边界之外，把宿主事件循环永久卡死，而提取接受了该 meta。— 故障场景：meta = { name: (Promise.resolve().then(() => { while (true) {} }), 'x'), description: 'd' } → runInContext 约 0ms 返回，extractAndStripMeta 成功返回 {name:'x',description:'d'}，随后在下一个微任务检查点循环阻塞事件循环直至永远——探测进程在 12s 时被强杀，已注册的 setTimeout 始终未触发。四种形状均已验证：裸 .then、script 2 遍历 getter 时的延迟、async/await，以及经恶意 message getter 触达错误格式化脚本（即使提取失败也照样卡死）。在 meta realm 里冻结 Promise.prototype 无法堵住该通道。卡死发生在运行路径的 30s 脚本体超时与墙钟看门狗装配之前，对一个可达的模型编写形状证伪了 PR 的标题契约——“永不返回的 meta 字面量现在会以常规的 meta 格式错误暴露，而不是卡死进程”。与 R3-1 相互独立：不涉及任何 rejection，R3-1 的修复堵不住它。
进程内不存在闭合手段（vm 看门狗无法触达 runInContext 之外的代码；也没有任何 API 能排空已排队的微任务）。真正的边界需要把提取放进一个可按超时 terminate() 的 worker_threads worker / 子进程。如果暂时接受该残留，META_EVAL_TIMEOUT_MS 的理由注释与 PR 描述必须明确写明：该边界只覆盖同步工作——延迟工作仍能卡死进程。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复：meta、serializer 与错误格式化 context 均启用 afterEvaluate，并显式排空 getter 写入源 realm 的微任务。验证证据：字面量、serializer getter、message getter 的微任务死循环均在 250ms VM 超时内收口；workflow-sandbox.test.ts 190/190 通过。
| function observePromiseRejections<T>(run: () => T): T { | ||
| const nativeThen = Promise.prototype.then; |
There was a problem hiding this comment.
[Suggestion] R3-3: The PR description contradicts the merged code on the very mechanism this round added — "without the promise-hook machinery — a thenable is detected structurally during the in-vm walk, as it was before" (Relationship to #9097) and "a Promise created during evaluation but not reachable from the returned value … is still not marked handled … unchanged here" (Risk & Scope) are both false at HEAD: this function is a scoped createHook PROMISE interceptor that marks every Promise created in the window handled at creation, reachable or not — the green 'contains rejected Promises when %s aborts evaluation' table proves it. Stale counts too: "All 148 pre-existing tests pass unchanged" (one expectation was edited — the unknown-identifier test now asserts /totallyUnknown is not defined/) and "155 passed"/"Seven tests" (the suite is now 183). — Concrete cost: the merged description becomes the durable record; a future maintainer reading "no promise-hook machinery" can conclude this function is redundant and delete it — re-opening exactly the R2-1 escape (probe-verified process kill); implementers of the follow-up callers named in the PR may build unhandledRejection listeners against a hole that no longer exists.
Fix: before merge, state in "Relationship to #9097" that a scoped creation-time async-hooks interceptor now backs the in-vm structural marking; replace the "still not marked handled" sentence with the current behaviour (every Promise created during evaluation is marked handled at creation, including the abort paths); refresh the test counts; declare the one edited expectation.
中文说明
[Suggestion] R3-3: PR 描述与合入代码在本轮新增的机制上自相矛盾——“不需要 promise hook 机制——thenable 仍像以前一样在 vm 内按结构识别”（与 #9097 的关系）与“求值期间创建但从返回值不可达的 Promise……仍然不会被标记为已处理……本 PR 未改变它”（风险与范围）在 HEAD 上都不成立：本函数就是一个作用域内的 createHook PROMISE 拦截器，会把窗口内创建的每一个 Promise 在创建时标记为已处理，无论其是否可从返回值触达——绿色的 'contains rejected Promises when %s aborts evaluation' 表即是证明。计数同样过期：“原有 148 项测试全部原样通过”（实际有一处期望被修改——未知标识符测试现断言 /totallyUnknown is not defined/），“155 项通过”/“新增七个测试”（套件现为 183 项）。— 具体代价：合入后的描述将成为持久记录；未来维护者读到“没有 promise hook 机制”可能认为本函数多余而将其删除——正好重新打开 R2-1 逃逸（探测验证过的进程杀死）；PR 中点名的后续调用方实现者可能针对一个已不存在的漏洞去构建 unhandledRejection 监听器。
修复：合入前，在“与 #9097 的关系”中说明现有一个作用域内的创建期 async-hooks 拦截器支撑 vm 内的结构化标记；把“仍然不会被标记为已处理”一句替换为当前行为（求值期间创建的每个 Promise 都在创建时被标记为已处理，含中断路径）；刷新测试计数；声明被修改的那一处期望。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复：PR 描述已同步 scoped async-hooks、VM 微任务排空、不可达 Promise 行为与当前 190 项测试证据。验证证据：描述中的机制与 e49a3b5 实现一致。
| // from the run that triggered it. The serializer marks any thenable it | ||
| // reaches as handled inside the vm; reject the meta literal up front. | ||
| if (walked.hasThenable) { |
There was a problem hiding this comment.
[Suggestion] R3-4: The diff deletes rejectThenablesInMeta but leaves two test-comment blocks describing it as current behaviour — workflow-sandbox.test.ts:646-655 ("The fix is to walk the eval result, neutralise any thenables with a .catch" — no host-side walk of the eval result exists after this diff) and workflow-sandbox.test.ts:692-702 ("the try/catch only wraps the vm-eval" — it now wraps the serializer too; and "A WeakSet bounds the recursion against cycles AND against future shapes where the same node is reached through multiple keys" — that now states the OPPOSITE of the serializer's ancestors add-on-entry/delete-on-exit behaviour, which the new test 'copies shared phase objects at each array position' pins). — Concrete cost: a maintainer trusting the comment could add a global seen-set to the serializer to "restore" the documented invariant and silently re-introduce the round-2 shared-subgraph regression. (This source-side comment was updated correctly in this round — the two test-side blocks look like oversights in the same cleanup.)
Fix: rewrite both blocks to describe the current mechanism — rejection-neutralisation via the serializer's handlePromise plus the observePromiseRejections async-hooks net, and cycle bounding via the ancestors set, which intentionally does NOT dedupe shared subgraphs.
中文说明
[Suggestion] R3-4: diff 删除了 rejectThenablesInMeta，但留下两处测试注释块仍在描述它的行为如同现状——workflow-sandbox.test.ts:646-655（“修复方式是遍历求值结果，用 .catch 中和所有 thenable”——本 diff 之后已不存在对求值结果的宿主侧遍历）与 workflow-sandbox.test.ts:692-702（“try/catch 只包裹 vm 求值”——现在也包裹了序列化器；以及“WeakSet 既防循环、也防同一节点经多个键被再次触达的形状”——这现在与序列化器 ancestors 集合“进入时加入/退出时删除”的行为相反，而新测试 'copies shared phase objects at each array position' 已把故意逐位置复制共享子图的行为钉住）。— 具体代价：信任该注释的维护者可能给序列化器加一个全局 seen 集合来“恢复”文档所述不变量，从而悄悄重新引入第二轮的共享子图回归。（源码侧注释本轮已正确更新——这两处测试侧注释看起来是同一次清理中的遗漏。）
修复：改写两处注释块以描述当前机制——rejection 中和经由序列化器的 handlePromise 加 observePromiseRejections async-hooks 安全网完成；循环边界经由 ancestors 集合完成，且该集合有意不去重共享子图。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复：两处测试注释已改为 serializer handlePromise + async-hooks observer，以及仅对当前递归路径生效的 ancestors 语义。验证证据：workflow-sandbox.test.ts 190/190 通过。
| } catch (e) { | ||
| throw new Error( | ||
| 'extractAndStripMeta: failed to evaluate meta object literal: ' + | ||
| formatMetaEvaluationError(e), | ||
| ); | ||
| } |
There was a problem hiding this comment.
[Suggestion] R3-5: A timeout in the serializer walk (script 2) is reported with the same message as a timeout in the literal evaluation (script 1) — this single catch wraps both runInContext calls, so both surface as failed to evaluate meta object literal: Script execution timed out after 250ms. — Failure scenario: { name: 'x', description: 'd', get phases() { while (true) {} } } evaluates instantly and hangs only when script 2 reads .phases; an operator diagnosing the timeout inspects the literal, sees no loop at evaluation position, and misdiagnoses (or concludes the timeout is spurious), because nothing indicates the hang was in the property-read walk. Probe-verified: both shapes yield byte-identical messages today; splitting the catches was flip-tested and distinguishes the stages.
Fix: wrap the serializer runInContext in its own try/catch throwing extractAndStripMeta: failed to serialize meta object literal: … (script 1 keeps the existing prefix), so the message distinguishes literal evaluation from the walk.
中文说明
[Suggestion] R3-5: 序列化遍历（script 2）的超时与字面量求值（script 1）的超时上报的是同一条错误信息——这个唯一的 catch 包裹了两处 runInContext 调用，两者都暴露为 failed to evaluate meta object literal: Script execution timed out after 250ms。— 故障场景：{ name: 'x', description: 'd', get phases() { while (true) {} } } 求值瞬间完成，只在 script 2 读取 .phases 时挂起；排障的操作者检查字面量，在求值位置看不到任何循环，于是误判（或认为超时是误报），因为没有任何信息表明挂起发生在属性读取遍历中。已探测验证：两种形状目前产生逐字节相同的信息；把 catch 拆分后做了翻转测试，可以区分两个阶段。
修复：给序列化器的 runInContext 单独包一层 try/catch，抛出 extractAndStripMeta: failed to serialize meta object literal: …（script 1 保留现有前缀），使错误信息能区分字面量求值与遍历。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复：求值与序列化分别捕获并使用 failed to evaluate / failed to serialize 前缀。验证证据：对应阶段断言与完整 workflow-sandbox.test.ts 190/190 通过。
| const message = new vm.Script(META_ERROR_SOURCE).runInContext(context, { | ||
| timeout: META_EVAL_TIMEOUT_MS, | ||
| }); |
There was a problem hiding this comment.
[Suggestion] R3-6: formatMetaEvaluationError is tested only via the ReferenceError happy path — two executed mutants both survive the 183-test suite. — Failure scenario: (a) deleting the typeof error === 'string' branch from META_ERROR_SOURCE degrades a thrown string (throw 'kapow' currently surfaces verbatim) to unknown error, and no test fails; (b) dropping the { timeout: META_EVAL_TIMEOUT_MS } option on this call leaves a hostile message getter — throw { get message() { while (true) {} } }, bounded today at ~252ms only by this option — to wedge the host thread forever: under the mutant the probe was killed at 20s. The exact failure class this PR removes, reintroduced on the error path and pinned by nothing.
Fix — two cases beside the existing unknown-identifier test:
it('surfaces a string thrown by the meta literal', () => {
const src = `export const meta = { name: (function(){ throw 'kapow'; })(), description: 'd' }\nreturn 1`;
expect(() => extractAndStripMeta(src)).toThrow(/kapow/);
});
it('bounds an error whose message getter loops', () => {
const src = `export const meta = { name: (function(){ throw { get message() { while (true) {} } }; })(), description: 'd' }\nreturn 1`;
expect(() => extractAndStripMeta(src)).toThrow(
/failed to evaluate meta object literal/,
);
expect(timed(() => extractAndStripMeta(src))).toBeLessThan(BOUND_MS);
});中文说明
[Suggestion] R3-6: formatMetaEvaluationError 只经由 ReferenceError 的常规路径被测——两个已执行的变异体都能在全套 183 项测试中存活。— 故障场景：(a) 删除 META_ERROR_SOURCE 中的 typeof error === 'string' 分支，会把抛出的字符串（throw 'kapow' 目前原样暴露）降级为 unknown error，且没有任何测试失败；(b) 去掉本调用的 { timeout: META_EVAL_TIMEOUT_MS } 选项，恶意的 message getter——throw { get message() { while (true) {} } }，目前仅靠该选项才在 ~252ms 内收口——将把宿主线程永久卡死：变异下探测进程在 20s 时被强杀。这正是本 PR 要消除的那类故障，被重新引入到错误路径上，且没有任何测试钉住。
修复——在现有未知标识符测试旁补两个用例（代码见上）。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复：新增抛出字符串、同步恶意 message getter 与微任务恶意 message getter 回归。验证证据：三条路径均通过，完整 workflow-sandbox.test.ts 190/190 通过。
已被后续 commit 取代，当前 head 需重新 review。
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "You are review agent reverse-audit — Reverse audit agent…": empirical probe of FinalizationRegistry/GC residual trigger (R4-3) — not retried (prior round ran 1,294 negative rounds).; "You are review agent reverse-audit — Reverse audit agent…": cross-platform validation (macOS/Windows vm timeout behavior) — Linux only.; "You are review agent reverse-audit — Reverse audit agent…": empirical re-probe of the R4-3 FinalizationRegistry trigger not re-attempted (prior round ran 1,294 negative forced-GC rounds; mechanism already confirmed there…; "You are review agent reverse-audit — Reverse audit agent…": cross-platform (macOS/Windows) vm-timeout behavior not exercised — Linux-only environment.; "PR #9136 (QwenLM/qwen-code) bounds both halves of workflow…": none — no check was left unfinished., and 3 more.
Test Plan (not a blocker): 190 passed — this review observed 20003, 1429, 19656, 1555, 494, 3561, 560 passed.
中文说明
未探索到全部深度（达到工具调用预算）："You are review agent reverse-audit — Reverse audit agent…"：empirical probe of FinalizationRegistry/GC residual trigger (R4-3) — not retried (prior round ran 1,294 negative rounds).；"You are review agent reverse-audit — Reverse audit agent…"：cross-platform validation (macOS/Windows vm timeout behavior) — Linux only.；"You are review agent reverse-audit — Reverse audit agent…"：empirical re-probe of the R4-3 FinalizationRegistry trigger not re-attempted (prior round ran 1,294 negative forced-GC rounds; mechanism already confirmed there…；"You are review agent reverse-audit — Reverse audit agent…"：cross-platform (macOS/Windows) vm-timeout behavior not exercised — Linux-only environment.；"PR #9136 (QwenLM/qwen-code) bounds both halves of workflow…"：none — no check was left unfinished.，另有 3 条。
Test Plan（非阻断）：190 passed — this review observed 20003, 1429, 19656, 1555, 494, 3561, 560 passed。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| if (apply(setHas, promiseScanSeen, [value])) return; | ||
| apply(setAdd, promiseScanSeen, [value]); | ||
| const descriptors = getOwnPropertyDescriptors(value); |
There was a problem hiding this comment.
[Critical] R4-1: markPromises walks the entire evaluated graph via getOwnPropertyDescriptors/Reflect.ownKeys with NO node/size budget — spend() only gates the later copy() — and that enumeration is a single native call the 250 ms vm timeout cannot interrupt promptly. — Failure scenario: a workflow file with phases: new Array(10_000_000).fill({}) evaluates fast in script 1 (native fill), then script 2's markPromises enumerates 10M sets of own-property descriptors: measured 21,241–23,380 ms end-to-end through extractAndStripMeta at this commit (~85× META_EVAL_TIMEOUT_MS, >4× this suite's own 5 s bound), the calling thread/event loop frozen the whole time and transient peak RSS up to ~3.2 GB, before failed to serialize ... timed out surfaces. A 1M-key object ({ ...new Array(1_000_000).fill(0) }) takes 839 ms. The existing holey-array test (new Array(1e8)) passes only because holey arrays have no own index descriptors — .fill() materializes them and defeats it. Scales with attacker-chosen size; reachable on the run path before the 30 s body timeout is armed. Flip-tested: a bounded array branch inside markPromises drops the wedge to ~300 ms.
Fix direction — give markPromises its own budget mirroring copy()'s accounting, e.g.:
// inside markPromises, before the descriptor enumeration:
if (!spend(4)) return; // the scan aborts once the budget is exhausted,
// same shape as copy()'s accounting(or iterate arrays by length + sparse indices instead of getOwnPropertyDescriptors), plus a .fill()-materialized regression.
中文说明
[Critical] R4-1：markPromises 通过 getOwnPropertyDescriptors/Reflect.ownKeys 遍历整个求值结果图，却没有任何节点/体积预算——spend() 只在后面的 copy() 里生效——而这段枚举是一次原生调用，250ms 的 vm 超时无法及时中断它。— 故障场景：phases: new Array(10_000_000).fill({}) 在 script 1 里经原生 fill 快速求值，随后 script 2 的 markPromises 枚举 1000 万组自有属性描述符：在本提交上经真实 extractAndStripMeta 实测 21,241–23,380 ms（约 85× META_EVAL_TIMEOUT_MS、超过本套件自身 5s 断言的 4 倍），期间调用线程/事件循环全程冻结，瞬时内存峰值最高约 3.2 GB，最后才以 failed to serialize ... timed out 暴露。100 万键对象（{ ...new Array(1_000_000).fill(0) }）耗时 839ms。现有 holey-array 测试（new Array(1e8)）能通过仅因为空洞数组没有自有索引描述符——.fill() 将其物化即可击穿该测试。耗时随攻击者可选的规模线性增长；该路径在运行路径的 30s 脚本体超时装配之前即可触达。已做翻转测试：在 markPromises 内加入有界的数组分支后，卡死从约 21s 降至约 300ms。
修复方向——给 markPromises 独立的预算，与 copy() 的计费同形（见上代码块），或改用 length + 稀疏索引的有界迭代代替对数组的 getOwnPropertyDescriptors，并补充 .fill() 物化形状的回归。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复：物化数组在 serializer 描述符枚举前按预算拒绝，meta 求值/序列化同时移入 2 秒超时、256 MB 堆上限的隔离子进程。验证：Node 22 独立探针中 1000 万元素物化数组 237.5 ms 返回 meta literal is too large；workflow-sandbox.test.ts 193/193、Core build/typecheck、ESLint、Prettier 通过。
| raw = new vm.Script(`(${metaSource})`).runInContext(metaContext, { | ||
| timeout: META_EVAL_TIMEOUT_MS, | ||
| }); |
There was a problem hiding this comment.
[Critical] R4-2: vm timeout only checks at JS interrupt points — a single long-running native builtin in the model-authored literal (or in microtasks drained by microtaskMode: 'afterEvaluate') runs far past the 250 ms bound while the event loop stays frozen. The constant's comment ("tight enough that a wedge is reported promptly") and the docstring ("the split between them is what makes that bound real") are disproven by measurement for native builtins, and this residual is documented nowhere. — Failure scenario (all measured end-to-end at this commit; the correct failed to evaluate ... timed out error surfaces, but ~43× past the declared bound and >2× the tests' 5 s assertion): (a) name: (new Array(2**26).fill(1).sort(), 'x') → 10,704–11,372 ms; (b) name: JSON.stringify(new Array(2**26).fill(1)) → 10,673–14,072 ms; (c) microtask-deferred Promise.resolve(new Array(2**26).fill(1)).then(function(a){ a.sort(); }) → 11,194–13,706 ms, charged to script 1's afterEvaluate drain. Scales with array size (2^27 is worse); a hostile workflow meta wedges the CLI main thread for tens of seconds on the run path (before the 30 s body timeout is armed) and on the future dialog/palette callers named in this PR's motivation.
This is inherent to vm timeouts (native builtins are mostly uninterruptible). Either move meta extraction into a worker thread / child process that can be terminate()d on timeout, or correct the rationale comment to state the residual, e.g.:
// Wall-clock bound on each vm script. Interrupts JS bytecode loops promptly;
// a single long native builtin call (large Array.sort / JSON.stringify) can
// still run past it for seconds before the next interrupt check.中文说明
[Critical] R4-2：vm 的 timeout 只在 JS 中断点检查——模型字面量里一次长时间运行的原生内置调用（或经 microtaskMode: 'afterEvaluate' 排空的微任务里的此类调用）会在事件循环被冻结的状态下远超 250ms 边界。常量的注释（"tight enough that a wedge is reported promptly"）与 docstring（"the split between them is what makes that bound real"）在原生内置调用上被实测证伪，且该残留没有在任何地方写明。— 故障场景（均在本提交上端到端实测：错误最终正确暴露为 failed to evaluate ... timed out，但耗时约为声明边界的 43 倍、超过测试 5s 断言的 2 倍）：(a) name: (new Array(2**26).fill(1).sort(), 'x') → 10,704–11,372 ms；(b) name: JSON.stringify(new Array(2**26).fill(1)) → 10,673–14,072 ms；(c) 微任务延迟的 Promise.resolve(new Array(2**26).fill(1)).then(function(a){ a.sort(); }) → 11,194–13,706 ms，计入 script 1 的 afterEvaluate 排空。耗时随数组规模增长（2^27 更久）；恶意 workflow meta 可在运行路径（30s 脚本体超时装配之前）以及本 PR 动机中点名的后续调用方（工具确认对话框/已保存 workflow 面板）上把 CLI 主线程卡死数十秒。
这是 vm 超时的固有限制（原生内置调用大多不可中断）。要么把 meta 提取放进可按超时 terminate() 的 worker 线程/子进程；要么按上面代码块修正理由注释，明确写出该残留。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复：meta 求值与序列化移入可强制终止的隔离子进程，原生 builtin 不再阻塞宿主主线程。验证：Node 22 独立探针 Array.sort 1362.9 ms、JSON.stringify 1329.2 ms、afterEvaluate sort 1313.7 ms 内有界失败，正常 meta 225.5 ms 成功；workflow-sandbox.test.ts 193/193、Core build/typecheck 通过。
| Object.freeze(Object.prototype); | ||
| Object.freeze(Array.prototype); |
There was a problem hiding this comment.
[Suggestion] R4-4: The Object.freeze(Array.prototype) defense line this diff adds has no guard test — mutation-tested at this commit: deleting line 548 leaves all 190 tests green, while the Object.prototype sibling is pinned by the envelope-forgery tests in this same diff. — Failure scenario: a model getter using the realm-reach chain arguments.callee.caller.constructor('return globalThis')() (proven working by two existing passing tests in this diff) sets the serializer realm's Array.prototype.toJSON = () => [{ title: 'forged' }]; copied arrays are realm [] literals, so JSON.stringify consults the poisoned toJSON and emits forged phases, while the envelope wrapper (Object.prototype still frozen) stays genuine, host checks pass, and validateMeta accepts content the literal never produced. Verified RED with the freeze line deleted and GREEN with it restored. The cost today is an unguarded security line this diff itself added — the forgery goes live the moment a refactor drops the freeze.
Mirror the Object.prototype forgery test for arrays, asserting the effect rather than the mechanism, e.g.:
it('rejects phases forgery through Array.prototype', () => {
const src = `export const meta = {
name: 'x',
description: 'd',
phases: [{ title: 'real' }],
extra: Object.defineProperty({}, 'value', { enumerable: true, get: function () {
const g = arguments.callee.caller.constructor('return globalThis')();
g.Array.prototype.toJSON = () => [{ title: 'forged' }];
return 'safe';
} }),
}`;
expect(extractAndStripMeta(src).meta?.phases).toEqual([{ title: 'real' }]);
});(Do not use a phases: Promise.resolve(1) variant — with no real array in the envelope the poison has nothing to act on and the mutant still passes.)
中文说明
[Suggestion] R4-4：本 diff 新增的 Object.freeze(Array.prototype) 防线没有护栏测试——在本提交上做变异测试：删除第 548 行后全部 190 项测试依旧通过，而同一 diff 中 Object.prototype 的姊妹行有信封伪造测试钉住。— 故障场景：模型 getter 经 arguments.callee.caller.constructor('return globalThis')() 链触达 serializer realm（该链已被本 diff 中两个现存的通过测试证明有效），设置 serializer realm 的 Array.prototype.toJSON = () => [{ title: 'forged' }]；被拷贝的数组是 realm 内的 [] 字面量，JSON.stringify 会查询被污染的 toJSON 并输出伪造的 phases，而信封包装对象（Object.prototype 仍被冻结）保持真实，宿主检查全部通过，validateMeta 接受了字面量从未产生的内容。已验证：删除冻结行测试变 RED，恢复后变 GREEN。当前代价是本 diff 自己新增的安全防线无人看守——一旦某次重构删掉该行，伪造即刻生效。
为数组补一个与 Object.prototype 伪造测试对称的回归，断言效果而非机制（见上代码块）。不要使用 phases: Promise.resolve(1) 变体——信封中没有真实数组时毒化无对象可作用，变异仍会通过。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复：补充 Array.prototype.toJSON 污染回归，固定 phases 保持原始 real 内容。验证：workflow-sandbox.test.ts 193/193、ESLint、Prettier 与 Core typecheck 通过。
| * 3. A SECOND fresh vm context walks that value and serialises it to JSON, | ||
| * and the host parses the result into host-realm plain objects. Its | ||
| * intrinsics have never been exposed to the model-authored program. |
There was a problem hiding this comment.
[Suggestion] R4-7: This docstring's security claim is false — probe-verified at this commit: a model-authored getter written as a function expression reaches the serializer realm via arguments.callee.caller.constructor('return globalThis')(), observed the serializer-only slot __qwenWorkflowMetaValue, and persisted a mutation on that realm's globalThis. Only Object.prototype/Array.prototype are frozen there; String/Number/Boolean/Function/Promise/WeakSet prototypes and JSON are poisonable. No exploit exists today — every intrinsic is captured before the walk and Reflect.apply bypasses Function.prototype (a walk-time overwrite of jsonStringify + envelope flags was attempted and had no effect) — but safety rests entirely on that un-documented capture discipline, and a maintainer trusting this sentence who adds serializer code reading a realm global after the walk starts inherits a poisonable realm: the envelope can then be forged (hasThenable:false, tooLarge:false, arbitrary value), letting a Promise-bearing or oversized meta pass extraction. Note the existing "does not expose host helpers" test passes either way, because copy is a closure local never on any globalThis; method-syntax getters hit the .caller poison pill while function-expression getters do not.
| * 3. A SECOND fresh vm context walks that value and serialises it to JSON, | |
| * and the host parses the result into host-realm plain objects. Its | |
| * intrinsics have never been exposed to the model-authored program. | |
| * 3. A SECOND fresh vm context walks that value and serialises it to JSON, | |
| * and the host parses the result into host-realm plain objects. Model | |
| * getters CAN still reach this realm (via the arguments.callee.caller | |
| * chain); safety rests on every intrinsic being captured below before | |
| * the walk starts, on Reflect.apply for all calls, and on freezing | |
| * Object.prototype / Array.prototype at the top of the script. |
Optionally freeze the remaining reachable prototypes (String, Number, Boolean, Function, Promise, WeakSet, JSON) as defense in depth — flip-tested: all 190 tests stay green with them frozen (reachability persists; the freezes mitigate poisoning but do not sever the caller-chain escape).
中文说明
[Suggestion] R4-7：该 docstring 的安全声明不成立——已在本提交上探测验证：写成函数表达式的模型 getter 可经 arguments.callee.caller.constructor('return globalThis')() 触达 serializer realm，观察到 serializer 专用槽 __qwenWorkflowMetaValue，并在该 realm 的 globalThis 上持久化了一个变更。该 realm 仅冻结了 Object.prototype/Array.prototype；String/Number/Boolean/Function/Promise/WeakSet 原型与 JSON 均可被污染。今天尚无可利用路径——所有内置对象都在遍历开始前捕获、Reflect.apply 绕过 Function.prototype（遍历期间覆写 jsonStringify 与信封标志的尝试已被实测为无效）——但安全性完全依赖这条未写入文档的"先捕获后使用"纪律；若维护者信任此句并在遍历开始后新增读取 realm 全局的 serializer 代码，将继承一个可被污染的 realm：信封随即可被伪造（hasThenable:false, tooLarge:false、任意 value），携带 Promise 或超大的 meta 将通过提取。注意现有"不暴露宿主 helper"测试两种情况下都能通过，因为 copy 是闭包局部量，从不在任何 globalThis 上；方法语法 getter 会触发 .caller 毒丸而函数表达式 getter 不会。
建议按上方 suggestion 块改写该条 docstring，写明真实不变量；可选地将剩余可达原型一并冻结作为纵深防御——已做翻转测试：冻结后 190 项测试全绿（可达性仍在，冻结只减轻毒化面，不能切断 caller 链逃逸）。
— qwen3.8-max via Qwen Code /review (v0.21.11)
There was a problem hiding this comment.
已修复：契约说明改为真实的隔离子进程边界，并明确由外层硬超时处理无法及时到达 node:vm interrupt point 的原生 builtin。验证：Node 22 四条性能探针均在 0.24–1.36 秒内完成或有界失败，正常 meta 成功；Core build/typecheck 通过。
已被后续 commit 908ea66 取代，当前 head 需重新 review
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 Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; the changed suite ran on Linux only (spawnSync timeout/SIGKILL semantics are platform-dependent).
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": none — the full chunk and all dependent host-side code were read within budget..
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Test Plan (not a blocker): 190 passed — this review observed 1429, 19656, 1555, 494, 3561, 560 passed.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; the changed suite ran on Linux only (spawnSync timeout/SIGKILL semantics are platform-dependent)。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：none — the full chunk and all dependent host-side code were read within budget.。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
Test Plan（非阻断）：190 passed — this review observed 1429, 19656, 1555, 494, 3561, 560 passed。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| const childEnv = { ...process.env }; | ||
| // The inline evaluator has no source coverage to collect; inherited V8 |
There was a problem hiding this comment.
[Critical] R5-4: The child inherits NODE_OPTIONS from {...process.env}; a module-mode value (--input-type=module or --experimental-default-type=module) flips the CommonJS --eval script into ESM, where its require('node:async_hooks') throws — so every meta extraction fails. The CLI's env scrub does not cover this path: the only production scrub that mutates process.env is gated on ACP-daemon mode, and the repo's own test pins that NODE_OPTIONS survives a non-ACP launch. A throwing/missing --require preload in NODE_OPTIONS is the same class. — Failure scenario: qwen runs under NODE_OPTIONS='--input-type=module' → at the first workflow with export const meta, the child dies with ReferenceError: require is not defined in ES module scope (exit 1, empty stdout) → every meta literal, including perfectly well-formed ones, throws failed to evaluate meta object literal: isolated evaluator exited without a result — a permanent, misleading, feature-wide failure for the session.
Witness (probe through the real extractAndStripMeta, Node 22):
input-type=module => THREW: ... isolated evaluator exited without a result
default-type=module => THREW: ... isolated evaluator exited without a result
require-preload => THREW: ... isolated evaluator exited without a result
clean => SUCCESS meta={"name":"w","description":"d","phases":[{"title":"One"}]}
Same literal; the env var is the only variable.
Fix: pin the eval input mode in the spawn argv — --input-type=commonjs on the command line overrides both NODE_OPTIONS variants — or delete childEnv['NODE_OPTIONS'] next to the NODE_V8_COVERAGE deletion:
[
`--max-old-space-size=${META_CHILD_MAX_OLD_SPACE_MB}`,
'--input-type=commonjs',
'--eval',
META_CHILD_SOURCE,
],中文说明
子进程通过 {...process.env} 继承 NODE_OPTIONS；模块模式的取值（--input-type=module 或 --experimental-default-type=module）会把 CommonJS 的 --eval 脚本翻转成 ESM，其中的 require('node:async_hooks') 会抛错——于是所有 meta 提取都会失败。CLI 的环境变量清理覆盖不到这条路径：唯一会改动 process.env 的生产清理逻辑只在 ACP 守护模式下生效，且仓库自己的测试固定了"非 ACP 启动时 NODE_OPTIONS 保留"这一行为。NODE_OPTIONS 中抛错/缺失的 --require 预加载属于同一类。— 故障场景：qwen 在 NODE_OPTIONS='--input-type=module' 下运行时，第一个包含 export const meta 的 workflow 就会使子进程以 ReferenceError: require is not defined in ES module scope 死亡（exit 1、stdout 为空）→ 所有 meta 字面量（包括完全合法的）都抛 failed to evaluate meta object literal: isolated evaluator exited without a result——整个会话内永久的、误导性的、功能级故障。
证据（经真实 extractAndStripMeta 探测，Node 22）：
input-type=module => THREW: ... isolated evaluator exited without a result
default-type=module => THREW: ... isolated evaluator exited without a result
require-preload => THREW: ... isolated evaluator exited without a result
clean => SUCCESS meta={"name":"w","description":"d","phases":[{"title":"One"}]}
同一字面量，唯一变量是该环境变量。
修复：在 spawn argv 中固定求值输入模式——命令行上的 --input-type=commonjs 可以覆盖 NODE_OPTIONS 的两种变体——或在删除 NODE_V8_COVERAGE 的位置旁 delete childEnv['NODE_OPTIONS']（见上代码块）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
There was a problem hiding this comment.
已修复。验证证据：提交 ab0ba69 清除隔离 evaluator 子进程继承的 NODE_OPTIONS；修复前真实入口在 --input-type=module 下合法 meta 失败，修复后 --input-type=module 与 --experimental-default-type=module 两条回归均通过。workflow-sandbox.test.ts 195/195、Core build/typecheck、ESLint、Prettier、git diff --check 均通过。
| describe('bounded evaluation', () => { | ||
| const BOUND_MS = 5_000; |
There was a problem hiding this comment.
[Suggestion] R5-1: BOUND_MS = 5_000 is looser than the child outer kill (META_CHILD_TIMEOUT_MS = 2_000), so no test pins the 250ms per-script vm timeout (META_EVAL_TIMEOUT_MS) — verified by mutation: with META_EVAL_TIMEOUT_MS = 1900 all 17 bounded-evaluation tests still pass, per-case durations swelling from ~565ms to 3,866–3,881ms. — Failure scenario: a future refactor drops or inflates timeout: META_EVAL_TIMEOUT_MS; every malformed meta literal then blocks the calling thread ~2–3.9s (outer SIGKILL + error-path drain) instead of ~0.25–0.6s on the run path, and CI stays green.
Fix: for the cases resolved by the vm timeout (all except the native-builtin case, which intentionally exercises the outer kill), assert a bound between the two timeouts, e.g. expect(timed(...)).toBeLessThan(1_500) — generous vs the production worst case (~2×250ms + spawn overhead ≈ 0.6s) but below META_CHILD_TIMEOUT_MS, so losing the per-script timeout turns the suite red.
中文说明
BOUND_MS = 5_000 比子进程外层强杀（META_CHILD_TIMEOUT_MS = 2_000）更宽松，因此没有任何测试固定 250ms 的单脚本 vm 超时（META_EVAL_TIMEOUT_MS）——变异验证：把 META_EVAL_TIMEOUT_MS 改为 1900 后，17 个 bounded-evaluation 测试全部通过，单例耗时从约 565ms 膨胀到 3,866–3,881ms。— 故障场景：未来重构删除或调大 timeout: META_EVAL_TIMEOUT_MS 后，每个恶意/畸形 meta 字面量都会在运行路径上阻塞调用线程约 2–3.9 秒（外层 SIGKILL + 错误路径排空），而不是约 0.25–0.6 秒，且 CI 保持绿色。
修复：对由 vm 超时收口的用例（除刻意验证外层强杀的 native builtin 用例外）断言一个介于两层超时之间的上界，例如 expect(timed(...)).toBeLessThan(1_500)——相对生产最坏情况（约 2×250ms + 启动开销 ≈ 0.6s）留有余量，又低于 META_CHILD_TIMEOUT_MS，这样单脚本超时一旦丢失套件即红。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| throw new Error( | ||
| `extractAndStripMeta: failed to ${stage} meta object literal: ` + | ||
| 'isolated evaluator exited without a result', | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R5-2: When the child dies with a non-zero exit and no parseable stdout, this error discards result.status / result.signal / stderr. result.error is only set for spawn failures and ETIMEDOUT, so an abnormal exit (e.g. V8 heap OOM inside the 256MB-capped child) reaches this branch and reports a generic message indistinguishable from a spawn failure or an OS OOM-kill. — Failure scenario: a meta literal that allocates past the child's 256MB heap makes V8 abort with FATAL ERROR ... heap out of memory on stderr and a non-zero exit; the user sees "isolated evaluator exited without a result" with no hint that the literal blew the heap cap this PR added, so debugging starts from a misleading message. Probe (same spawnSync options as this function): both process.abort() and a heap-OOM child return error: undefined, status: null, signal: 'SIGABRT', stdout: '', stderr carrying 481 / 2,383 bytes of diagnostics; end-to-end through extractAndStripMeta with an over-cap literal, the thrown message is exactly the generic one.
| throw new Error( | |
| `extractAndStripMeta: failed to ${stage} meta object literal: ` + | |
| 'isolated evaluator exited without a result', | |
| ); | |
| throw new Error( | |
| `extractAndStripMeta: failed to ${stage} meta object literal: ` + | |
| `isolated evaluator exited without a result ` + | |
| `(status=${result.status}, signal=${result.signal})`, | |
| ); |
中文说明
当子进程以非零退出且没有可解析的 stdout 时，这里的错误丢弃了 result.status / result.signal / stderr。result.error 只在 spawn 失败与 ETIMEDOUT 时被设置，因此异常退出（例如在 256MB 上限的子进程内发生 V8 堆 OOM）会落到这个分支，报出一条与 spawn 失败、OS OOM-kill 无法区分的泛化消息。— 故障场景：meta 字面量分配超过子进程 256MB 堆上限时，V8 以 FATAL ERROR ... heap out of memory 中止并向 stderr 输出、非零退出；用户只看到 "isolated evaluator exited without a result"，完全看不出字面量击穿了本 PR 新增的堆上限，调试从一开始就被误导。探测（与本函数相同的 spawnSync 选项）：process.abort() 与堆 OOM 子进程都返回 error: undefined, status: null, signal: 'SIGABRT', stdout: ''，stderr 分别携带 481 / 2,383 字节诊断信息；用超上限字面量端到端走 extractAndStripMeta，抛出的正是这条泛化消息。
修复：见上方 suggestion（附带 status/signal）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| * and the host parses the result into host-realm plain objects. Its | ||
| * intrinsics have never been exposed to the model-authored program. | ||
| * | ||
| * Both scripts run in a bounded child process. The model-authored literal can |
There was a problem hiding this comment.
[Suggestion] R5-3: The PR description was never synced to the round-5 mechanism: it still describes a purely in-process fix ("The fix runs both halves inside the vm, each under a 250ms timeout", EN and 中文 alike), never mentions the isolated subprocess, its 2s SIGKILL outer bound, or the 256MB heap cap this commit adds, and the test plan's "190 passed" is stale (this commit's suite has 193 tests). Verified against the live body: grep for child|spawn|subprocess|SIGKILL|256|2000|heap → zero hits. The R4-2 blocker explicitly required the description to state what the bound covers. — Failure scenario: a maintainer merging on, or a future caller tuning against, the description's contract (e.g. the follow-up saved-workflow palette that reads every workflow's meta at startup) reasons with the wrong worst-case latency (~250ms stated vs ~2s + spawn overhead real) and the wrong mechanism — the same description/code divergence class already fixed once in R3-3, re-introduced by the round-5 rewrite.
Fix: update the PR description (both languages) to state that evaluation and serialization run in a spawned child process (META_CHILD_TIMEOUT_MS = 2s, SIGKILL, 256MB heap) around the per-script 250ms vm timeouts, and refresh the test-plan count to 193.
中文说明
PR 描述从未同步到第 5 轮的机制：它仍然描述纯进程内修复（"The fix runs both halves inside the vm, each under a 250ms timeout"，中英文均如此），完全没有提到隔离子进程、2s SIGKILL 外层边界、本提交新增的 256MB 堆上限，且测试方案中的 "190 passed" 已过时（本提交的套件为 193 项）。已对线上描述验证：grep child|spawn|subprocess|SIGKILL|256|2000|heap → 零命中。R4-2 阻断项曾明确要求描述写明边界覆盖范围。— 故障场景：基于该描述合并的维护者、或按描述契约调参的后续调用方（例如启动时读取所有 workflow meta 的已保存 workflow 面板）会按错误的最坏延迟（描述称 ~250ms，实际 ~2s + 启动开销）和错误的机制推理——与 R3-3 已修复过一次的"描述/代码分歧"同类，被第 5 轮重写再次引入。
修复：更新 PR 描述（两种语言），写明求值与序列化运行在一个被 spawn 的子进程中（META_CHILD_TIMEOUT_MS = 2s、SIGKILL、256MB 堆），外层包住每脚本 250ms 的 vm 超时，并把测试方案的数字更新为 193。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| * 3. A SECOND fresh vm context walks that value and serialises it to JSON, | ||
| * and the host parses the result into host-realm plain objects. Its | ||
| * intrinsics have never been exposed to the model-authored program. |
There was a problem hiding this comment.
[Suggestion] R4-7: This docstring sentence remains false at this commit (carried forward from the round-4 ledger; ruled "still stands" after tracing). A model-authored getter invoked during the walk still reaches the serializer realm's globalThis via arguments.callee.caller.constructor('return globalThis')() — this PR's own scope-isolation tests (test lines ~611/628/646/659) exercise exactly that reach, and a probe through the real code confirms it (reachedDifferentRealm: true, with that realm's Object/JSON/Reflect reachable). The Object.prototype/Array.prototype freezes, captured intrinsics, and host-side re-checks keep the reach harmless — but the absolute claim "have never been exposed" is inaccurate as written. — Failure scenario: a future maintainer trusts the absolute claim and removes the prototype freezes or the host-side length/shape re-checks (believing the realm unreachable), reopening the R2-8 envelope-forgery class.
Fix: reword to state the real defenses, e.g. "Its intrinsics are frozen and captured before the walk runs, and the host re-validates the envelope independently — a model getter that reaches this realm during the walk can neither redirect the walk nor forge its output."
中文说明
该 docstring 句子在本提交上仍然不成立（自第 4 轮台账结转；追溯代码后判定"仍然成立"）。遍历期间被调用的模型 getter 仍能通过 arguments.callee.caller.constructor('return globalThis')() 触达序列化器 realm 的 globalThis——本 PR 自己的作用域隔离测试（测试文件 ~611/628/646/659 行）正是在验证这条可达链，经真实代码的探测也确认了它（reachedDifferentRealm: true，且该 realm 的 Object/JSON/Reflect 可达）。Object.prototype/Array.prototype 冻结、捕获的内置对象与宿主侧复检使这条可达链无害——但"从未暴露"这一绝对化表述按字面是不准确的。— 故障场景：未来维护者相信该绝对化表述，删除原型冻结或宿主侧的长度/形状复检（以为该 realm 不可达），从而重新打开 R2-8 信封伪造故障类。
修复：改写为陈述真实防线，例如："Its intrinsics are frozen and captured before the walk runs, and the host re-validates the envelope independently — a model getter that reaches this realm during the walk can neither redirect the walk nor forge its output."
— qwen3.8-max via Qwen Code /review (v0.21.12)
| }); | ||
| }); | ||
|
|
||
| it('rejects serializer-envelope forgery through Object.prototype', () => { |
There was a problem hiding this comment.
[Suggestion] R5-9: The only test aimed at pinning Object.freeze(Object.prototype) is masked by the async-hooks Promise-creation hook: the fixture's own phases: Promise.resolve(1) makes the child throw META_PROMISE_ERROR regardless of whether the toJSON poisoning succeeds. Mutation-verified: removing Object.freeze(Object.prototype); (keeping the Array freeze) leaves all three forgery/poisoning tests green — while with the Promise removed from the fixture, the same mutation makes extraction SUCCEED with a forged envelope (probe: RETURNED META: {"name":"forged","description":"forged"}). The control mutation (removing the Array freeze instead) fails the sibling test, so the pattern discriminates when unmasked. The second Object.prototype test ('rejects an oversized payload after attempted envelope forgery') pins nothing either — its shorthand getter is strict-mode and throws before the poisoning can execute. — Failure scenario: a future change drops or relocates the Object.prototype freeze and ships green; a Promise-free getter could then forge the envelope end-to-end (host length/shape re-checks and validateMeta bound the blast radius — a pinning gap, not a live exploit).
Fix: drop phases: Promise.resolve(1) from the fixture AND assert the returned meta instead of .toThrow() (mirroring the sibling Array-poisoning test): expect(extractAndStripMeta(src).meta).toEqual({ name: 'x', description: 'd' }) — removing only the Promise leaves .toThrow() failing in both states, so the assertion shape matters too.
中文说明
唯一意在固定 Object.freeze(Object.prototype) 的测试被 async-hooks 的 Promise 创建钩子掩盖：fixture 自带的 phases: Promise.resolve(1) 使子进程无论 toJSON 投毒是否成功都会抛 META_PROMISE_ERROR。变异验证：删除 Object.freeze(Object.prototype);（保留 Array 冻结）后，三个伪造/投毒测试全部保持绿色——而把 fixture 中的 Promise 去掉后，同样的变异会让提取成功并返回伪造信封（探测：RETURNED META: {"name":"forged","description":"forged"}）。对照变异（改删 Array 冻结）会使兄弟测试失败，说明解除掩盖后该模式具有区分力。第二个 Object.prototype 测试（'rejects an oversized payload after attempted envelope forgery'）同样什么也固定不了——其简写 getter 是严格模式，会在投毒执行前就抛错。— 故障场景：未来改动删除或移动 Object.prototype 冻结都能绿色通过；届时一个不含 Promise 的 getter 就能端到端伪造信封（宿主侧长度/形状复检与 validateMeta 限制了影响面——这是固定缺口，不是现役漏洞）。
修复：去掉 fixture 中的 phases: Promise.resolve(1)，并把断言从 .toThrow() 改为断言返回的 meta（仿照兄弟 Array 投毒测试）：expect(extractAndStripMeta(src).meta).toEqual({ name: 'x', description: 'd' })——只去掉 Promise 而不改断言形态的话，.toThrow() 在两种状态下都会失败，因此断言形态同样关键。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| // Prevent allocation-heavy literals from exhausting the host process before | ||
| // the child timeout can stop them. | ||
| const META_CHILD_MAX_OLD_SPACE_MB = 256; |
There was a problem hiding this comment.
[Suggestion] R5-10: The 256MB cap only limits V8 old-space; ArrayBuffer/TypedArray backing stores bypass it entirely, so this comment overclaims. Measured at this commit: node --max-old-space-size=256 evaluating (new Uint8Array(new ArrayBuffer(2**31 - 24)).fill(1), 1) → exit 0, fill 838ms, rssMB: 2090, heapUsedMB: 3, arrayBuffersMB: 2048; end-to-end through the real extractAndStripMeta the child peaks at RSS 2,143,232 KB and the vm timeout only fires at ~965ms — after the uninterruptible native fill. The evaluate and serialize phases each admit one such allocation (~4GiB structural ceiling), held for most of the 2s kill window; on a host with <~2–4GiB free the kernel OOM killer may take the parent CLI or a sibling process. (Filed Critical by the finder; verification ruled Suggestion — extraction still throws cleanly, the residual is bounded by SIGKILL, and the base tree is strictly worse for the identical literal.) — Failure scenario: a memory-constrained container or small dev machine evaluating a workflow meta with a multi-GB backing-store literal spikes RSS for up to ~2s and can trigger OOM-kills outside the child during that window.
| // Prevent allocation-heavy literals from exhausting the host process before | |
| // the child timeout can stop them. | |
| const META_CHILD_MAX_OLD_SPACE_MB = 256; | |
| // Prevent allocation-heavy literals from exhausting the host process before | |
| // the child timeout can stop them. Bounds V8 heap allocations only — | |
| // ArrayBuffer/TypedArray backing stores are limited only by the outer kill. | |
| const META_CHILD_MAX_OLD_SPACE_MB = 256; |
Optionally enforce an OS-level cap on the child (setrlimit/ulimit -v wrapper or a cgroup slice before spawnSync).
中文说明
256MB 上限只约束 V8 老生代；ArrayBuffer/TypedArray 的后备存储完全绕过它，因此这条注释过度声明。在本提交上实测：node --max-old-space-size=256 求值 (new Uint8Array(new ArrayBuffer(2**31 - 24)).fill(1), 1) → exit 0，fill 838ms，rssMB: 2090, heapUsedMB: 3, arrayBuffersMB: 2048；端到端走真实 extractAndStripMeta，子进程 RSS 峰值 2,143,232 KB，vm 超时直到约 965ms 才触发——在不可中断的原生 fill 完成之后。求值与序列化阶段各自允许一次这样的分配（结构性上限约 4GiB），并可持续占据 2s 强杀窗口的大部分时间；在可用内存不足 2–4GiB 的宿主上，内核 OOM killer 可能杀掉父 CLI 或兄弟进程。（finder 报为 Critical；验证后判定 Suggestion——提取仍然干净抛错，残留由 SIGKILL 收口，且 base 树对同一字面量严格更糟。）— 故障场景：内存受限的容器或小型开发机在求值含多 GB 后备存储字面量的 workflow meta 时，RSS 在最长约 2s 内飙升，该窗口内可能触发子进程之外的 OOM-kill。
修复：见上方 suggestion（修正注释范围）；可选地对子进程施加 OS 级上限（spawnSync 前用 setrlimit/ulimit -v 包装或 cgroup slice）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| expect(() => extractAndStripMeta(src)).toThrow(); | ||
| }); | ||
|
|
||
| it('prefers a Promise error after the size budget is exceeded', () => { |
There was a problem hiding this comment.
[Suggestion] R5-11: The only test meant to pin the parent-side hasThenable-before-tooLarge envelope precedence rides the child async-hooks path: the fixture creates a real Promise (extra: Promise.resolve(1)), so the child throws META_PROMISE_ERROR before any envelope is emitted and the parent re-throws verbatim without evaluating either check. Mutation-verified: swapping the two parent checks leaves the full suite green (193/193), while a probe with { name: 'x'.repeat(70000), description: 'd', extra: { then: () => {} } } flips — under the swap it receives the too-large error instead of the Promise diagnostic. — Failure scenario: after such a reorder, a plain thenable next to oversized content (which creates no Promise, so the hook cannot fire) reports meta literal is too large instead of the Promise diagnostic, steering the model/user to shrink the literal instead of removing the thenable.
Fix: change the fixture's Promise to a plain thenable so the envelope path is exercised — extra: { then: () => {} } — optionally keeping the current fixture as a separate case for the child-gate path.
中文说明
唯一意在固定宿主侧"hasThenable 先于 tooLarge"信封优先级的测试走的是子进程 async-hooks 路径：fixture 创建了真实 Promise（extra: Promise.resolve(1)），子进程在输出任何信封之前就抛 META_PROMISE_ERROR，宿主原样重抛，两个检查都没有被真正执行。变异验证：交换宿主两个检查的顺序后整个套件仍然全绿（193/193）；而用 { name: 'x'.repeat(70000), description: 'd', extra: { then: () => {} } } 探测则会翻转——交换后收到的是 too-large 错误而非 Promise 诊断。— 故障场景：这种重排发生后，超大内容旁的普通 thenable（不创建 Promise，钩子不会触发）会报 meta literal is too large 而不是 Promise 诊断，把模型/用户引向"缩小字面量"而非"移除 thenable"。
修复：把 fixture 的 Promise 改为普通 thenable，使信封路径真正被执行——extra: { then: () => {} }——可选地把现有 fixture 保留为子进程钩子路径的独立用例。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| expect(timed(() => extractAndStripMeta(src))).toBeLessThan(BOUND_MS); | ||
| }); | ||
|
|
||
| it('bounds descriptor scanning for a materialized array', () => { |
There was a problem hiding this comment.
[Suggestion] R5-12: This test never exercises descriptor scanning and cannot distinguish the fast length * 2 > budget heuristic from a child OOM/SIGKILL fallback: the shortcut fires before getOwnPropertyDescriptors is reached, and both fallback messages match the test's /failed to serialize meta object literal/ regex within its time bound. Mutation executed: with the shortcut removed the child dies at the 2s kill and the test still passes — BASE pristine: elapsed=84ms, message="... meta literal is too large" vs mutated: elapsed=2024ms, message="... isolated evaluator exceeded 2000ms" — while worst-case extraction cost for a materialized array regresses from ~instant to a full child death on the run path. — Failure scenario: deleting the shortcut ships green; every materialized-array meta then pays ~2s + spawn instead of instant rejection.
Fix: assert the specific envelope message — expect(() => extractAndStripMeta(src)).toThrow(/meta literal is too large/) — deterministic, and immune to the spawn cold-start concern that makes tightening the time bound risky (the sibling holey-array test already asserts this discriminating message).
中文说明
该测试从未真正执行描述符扫描，也无法区分快速的 length * 2 > budget 启发式与子进程 OOM/SIGKILL 兜底：启发式先于 getOwnPropertyDescriptors 触发，且两种兜底消息在其时间上界内都匹配测试的 /failed to serialize meta object literal/ 正则。已执行变异：删除该快捷路径后子进程死于 2s 强杀，测试仍然通过——BASE 原始：elapsed=84ms, message="... meta literal is too large" 对比 变异后：elapsed=2024ms, message="... isolated evaluator exceeded 2000ms"——而物化数组的最坏提取代价从近乎瞬时退化为运行路径上完整的子进程死亡。— 故障场景：删除快捷路径可以绿色通过；每个物化数组 meta 都要付出约 2s + 启动开销，而不是瞬时拒绝。
修复：断言具体的信封消息——expect(() => extractAndStripMeta(src)).toThrow(/meta literal is too large/)——确定性强，且避开收紧时间上界带来的 spawn 冷启动风险（兄弟 holey-array 测试已经在断言这条区分性消息）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| }); | ||
| }); | ||
|
|
||
| it('rejects Promises after the literal mutates serializer helpers', () => { |
There was a problem hiding this comment.
[Suggestion] R5-13: Because the fixture's hostile value is a real Promise (extra: Promise.resolve(1)), the child's async-hooks gate throws META_PROMISE_ERROR independently of the serializer, leaving the defense the test names — serializer detection surviving meta-realm helper mutation (Object.keys = ..., Promise.prototype.then = ...) — unpinned. Four executed arms: (A) all three serializer detection paths neutralized + original fixture → the test still passes; (B) same mutation + thenable fixture → fails expected [Function] to throw an error; (C) pristine + thenable fixture → passes; (D) gate disabled + pristine serializer + thenable fixture → passes (rejection comes from the serializer alone). Same masking shape as R5-11, a different test and a different unpinned defense. — Failure scenario: a mutation breaking the serializer's captured-intrinsic walk ships green while a thenable-hiding regression rots silently.
Fix: make the hostile value a non-Promise thenable so the hook never fires — extra: { then: () => {} } — detection must then come from the serializer's own walk, and the Object.keys override in the fixture becomes genuinely discriminating.
中文说明
由于 fixture 的恶意值是真实 Promise（extra: Promise.resolve(1)），子进程的 async-hooks 闸门会独立于序列化器抛出 META_PROMISE_ERROR，使测试名义上要固定的防线——序列化器在 meta realm 内置对象被篡改（Object.keys = ...、Promise.prototype.then = ...）后仍能检出——没有被固定。四组已执行实验：(A) 中和序列化器全部三条检出路径 + 原 fixture → 测试仍通过；(B) 同样变异 + thenable fixture → 失败 expected [Function] to throw an error；(C) 原始代码 + thenable fixture → 通过；(D) 关闭闸门 + 原始序列化器 + thenable fixture → 通过（拒绝仅来自序列化器）。与 R5-11 同一掩盖形态，但测试与未固定防线不同。— 故障场景：破坏序列化器捕获内置对象遍历的变异可以绿色通过，thenable 隐匿回归将无声腐烂。
修复：把恶意值改为非 Promise 的 thenable，使钩子永不触发——extra: { then: () => {} }——检出必须来自序列化器自己的遍历，fixture 中的 Object.keys 覆盖才真正具有区分力。
— qwen3.8-max via Qwen Code /review (v0.21.12)
已被后续 commit 取代，当前 head 需重新 review
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. Suggestions are inline.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; the changed suite ran on Linux only (spawnSync timeout/SIGKILL semantics are platform-dependent).
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Test Plan (not a blocker): 190 passed — this review observed 20008, 1429, 19656, 1555, 494, 3561, 560 passed.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; the changed suite ran on Linux only (spawnSync timeout/SIGKILL semantics are platform-dependent)。
未审查：reverse audit — did not converge within the reverse-audit round cap of 5。
Test Plan（非阻断）：190 passed — this review observed 20008, 1429, 19656, 1555, 494, 3561, 560 passed。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| } catch { | ||
| throw new Error( | ||
| `extractAndStripMeta: failed to ${stage} meta object literal: ` + | ||
| 'isolated evaluator exited without a result', | ||
| ); | ||
| } |
There was a problem hiding this comment.
[Suggestion] R5-2: When the child dies with a non-zero exit and no parseable stdout, this error discards result.status / result.signal / stderr. result.error is only set for spawn failures and ETIMEDOUT, so an abnormal exit (e.g. a heap-OOM abort inside the 256MB-capped child) surfaces as the generic "exited without a result" with none of the fields that would distinguish it. — Failure scenario: the child OOMs or crashes on a hostile literal → the author/debugger sees "isolated evaluator exited without a result" with no status/signal/stderr and cannot tell an environment kill from a protocol violation.
Include result.status/result.signal (and a trimmed stderr tail) in the thrown message when available.
中文说明
当子进程以非零退出且没有可解析的 stdout 时，这个错误丢弃了 result.status / result.signal / stderr。result.error 只在 spawn 失败和 ETIMEDOUT 时被设置，因此异常退出（例如 256MB 堆上限子进程内的堆 OOM 中止）会以泛化的 "exited without a result" 呈现，不带任何可用于区分的字段。— 故障场景：子进程在恶意字面量上 OOM 或崩溃 → 作者/调试者看到 "isolated evaluator exited without a result"，没有 status/signal/stderr，无法区分环境杀死与协议违例。建议：可用时在抛出的错误信息中包含 result.status/result.signal（以及截断的 stderr 尾部）。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| * Both scripts run in a bounded child process. The model-authored literal can | ||
| * defer arbitrary work to property-read time — `{ get phases() { while (true) | ||
| * {} } }` evaluates instantly and only spins when something reads `.phases`. |
There was a problem hiding this comment.
[Suggestion] R5-3: The PR description was never synced to the round-5 mechanism: it still describes a purely in-process fix ("The fix runs both halves inside the vm, each under a 250ms timeout", EN and 中文 alike), never mentioning the isolated child process, the 2s outer SIGKILL, the 256MB heap cap, or the NODE_OPTIONS/NODE_V8_COVERAGE scrub — the mechanisms this round's safety actually rests on. (The docstring WAS synced; the description is the remaining gap — anchored here where the docstring describes the real design.) — Failure scenario: a maintainer reading the PR reasons about a mechanism that no longer exists (in-vm-only bounding) and misses the child-kill/heap-cap contracts that actually bound native builtins.
中文说明
PR 描述从未同步到第 5 轮的机制：仍描述纯进程内修复（"修复方案是把两个阶段都放进 vm 执行，各自受 250ms 超时约束"，中英文皆然），从未提及隔离子进程、2 秒外层 SIGKILL、256MB 堆上限、NODE_OPTIONS/NODE_V8_COVERAGE 清除——而本轮的安全性实际依赖的正是这些机制。（docstring 已同步；描述是残留缺口——锚定在 docstring 描述真实设计处。）— 故障场景：维护者阅读 PR 时对一个已不存在的机制（仅 vm 内定时）做推理，漏掉真正框住原生 builtin 的子进程杀除/堆上限契约。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| it('rejects serializer-envelope forgery through Object.prototype', () => { | ||
| const src = `export const meta = { | ||
| name: 'x', | ||
| description: 'd', | ||
| phases: Promise.resolve(1), |
There was a problem hiding this comment.
[Suggestion] R5-9: This test is blind to the defense it names: its own phases: Promise.resolve(1) co-factor makes the child throw META_PROMISE_ERROR before any forged envelope can reach the host, so it passes identically with Object.freeze(Object.prototype) removed from META_SERIALIZE_SOURCE. Mutation-verified at this commit: freeze removed → this test still passes; a fixed fixture (Promise dropped, assert on returned meta) flips — forged meta {name:'forged',description:'forged'} observed with the freeze gone, real meta with it restored. The sibling oversize-forgery test is likewise insensitive (its throw is maxBuffer overflow), while the Array-poisoning control genuinely pins its freeze. — Failure scenario: a future change drops Object.freeze(Object.prototype); a model getter installs Object.prototype.toJSON, forging the envelope to a validated meta that silently surfaces — while this test still passes.
witness: freeze removed → test STILL PASSES; fixed fixture + freeze removed → {"name":"forged","description":"forged"}; freeze restored → {"name":"x","description":"d"}
Drop the Promise from the fixture and assert on the returned meta: expect(extractAndStripMeta(src).meta).toEqual({ name: 'x', description: 'd' }) — then removing the freeze fails the assertion exactly when the defense is gone (a bare toThrow() still wouldn't — the forged envelope validates cleanly).
中文说明
该测试对其命名的防线是盲的：fixture 自身的 phases: Promise.resolve(1) 协因会使子进程在任何伪造信封到达宿主之前抛出 META_PROMISE_ERROR，因此即便从 META_SERIALIZE_SOURCE 移除 Object.freeze(Object.prototype)，测试照样通过。已在提交上变异验证：移除 freeze → 本测试仍通过；修复后的 fixture（去掉 Promise、断言返回的 meta）发生翻转——移除 freeze 时观察到伪造 meta {name:'forged',description:'forged'}，恢复后为真实 meta。同组的超大伪造测试同样不敏感（其抛错来自 maxBuffer 溢出），而 Array 投毒对照测试确实固定了其 freeze。— 故障场景：未来改动移除 Object.freeze(Object.prototype)；模型 getter 安装 Object.prototype.toJSON，把信封伪造成能通过校验的 meta 并静默浮现——而本测试仍通过。修复：从 fixture 去掉 Promise 并断言返回的 meta：expect(extractAndStripMeta(src).meta).toEqual({ name: 'x', description: 'd' })——这样移除 freeze 时断言恰好在防线消失时失败。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| it('prefers a Promise error after the size budget is exceeded', () => { | ||
| const src = `export const meta = { | ||
| name: 'x'.repeat(200000), | ||
| description: 'd', | ||
| extra: Promise.resolve(1), |
There was a problem hiding this comment.
[Suggestion] R5-11: The only test meant to pin the parent-side hasThenable-before-tooLarge envelope precedence rides the child async-hooks path: the fixture creates a real Promise (extra: Promise.resolve(1)), so the child throws META_PROMISE_ERROR via the createdPromise gate independently of the host-side if (walked.hasThenable) ... if (walked.tooLarge) ordering — delete or swap that ordering and the test stays green. — Failure scenario: a future refactor swaps the host-side precedence (tooLarge before hasThenable); an oversize meta containing a thenable surfaces "meta literal is too large" instead of the Promise diagnostic, and no test notices. Fix direction: add a precedence case whose thenable is a plain non-Promise thenable (bypasses the async-hook gate) so the parent-side ordering is what decides the error.
中文说明
唯一旨在固定宿主侧 hasThenable 优先于 tooLarge 信封顺序的测试，实际依赖的是子进程 async-hooks 路径：fixture 创建了真实 Promise（extra: Promise.resolve(1)），子进程经 createdPromise 关卡抛出 META_PROMISE_ERROR，与宿主侧 if (walked.hasThenable) ... if (walked.tooLarge) 的顺序无关——删除或交换该顺序测试仍为绿。— 故障场景：未来重构交换宿主侧优先级（tooLarge 先于 hasThenable）；包含 thenable 的超大 meta 以 "meta literal is too large" 而非 Promise 诊断浮现，且无测试察觉。修复方向：新增一个使用非 Promise 的普通 thenable 的优先级用例（绕过 async-hooks 关卡），使宿主侧顺序真正决定错误。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| it('bounds descriptor scanning for a materialized array', () => { | ||
| const src = `export const meta = { name: 'x', description: 'd', phases: new Array(10_000_000).fill({}) }\nreturn 1`; |
There was a problem hiding this comment.
[Suggestion] R5-12: This test never exercises descriptor scanning and cannot distinguish the fast length * 2 > budget heuristic from a child OOM/SIGKILL fallback: the shortcut fires before getOwnPropertyDescriptors is reached (re-verified at this commit), so the scan its name claims to pin is not on the executed path. — Failure scenario: a future change breaks or removes the descriptor-scan budgeting (the expensive part); this test still passes via the length shortcut, and the regression only surfaces on shapes the shortcut misses. Fix direction: rename to reflect the length-shortcut path it actually pins, or add a case under the shortcut threshold whose descriptor enumeration is genuinely expensive.
中文说明
该测试从未执行描述符扫描，也无法区分快速的 length * 2 > budget 启发式与子进程 OOM/SIGKILL 兜底：快捷路径在到达 getOwnPropertyDescriptors 之前就已触发（本提交上复核确认），其名称所声称固定的扫描并不在执行路径上。— 故障场景：未来改动破坏或移除描述符扫描的预算控制（昂贵的部分）；本测试仍经 length 快捷路径通过，回归只在快捷路径漏掉的形状上浮现。修复方向：重命名以反映其实际固定的 length 快捷路径，或新增一个低于快捷阈值、但描述符枚举真正昂贵的用例。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| if (type !== 'object') return null; | ||
| if (handlePromise(value) || typeof value.then === 'function') { | ||
| hasThenable = true; |
There was a problem hiding this comment.
[Suggestion] R6-11: copy()'s thenable gate reads value.then unguarded, while markPromises() explicitly guards the identical read one function above (comment: "Keep scanning data properties after a hostile then getter throws") — the second traversal of the same value graph contradicts the first's documented resilience, so a throwing then getter aborts extraction with model-controlled text instead of the serializer's designed thenable verdict. Legitimate meta is unaffected (WorkflowMeta has no then member) and the failure direction is safe (reject) — but the asymmetry is involuntary: the author already decided this exact case should be survived, once. — Failure scenario: reproduced through the real child at this commit: { get then() { throw new Error('boom') }, name: 'x', description: 'y' } → failed to serialize meta object literal: boom (model-controlled text) instead of the actionable META_PROMISE_ERROR; nested variant likewise.
witness: PR code → {"ok":false,"error":"extractAndStripMeta: failed to serialize meta object literal: boom"}; with the guard below applied → both probes flip to 'meta values must not be Promises', full 195-test file green.
| if (type !== 'object') return null; | |
| if (handlePromise(value) || typeof value.then === 'function') { | |
| hasThenable = true; | |
| if (type !== 'object') return null; | |
| let thenable = handlePromise(value); | |
| if (!thenable) { | |
| try { | |
| thenable = typeof value.then === 'function'; | |
| } catch { | |
| thenable = true; // unreadable then: treat as hostile thenable | |
| } | |
| } | |
| if (thenable) { | |
| hasThenable = true; |
中文说明
copy() 的 thenable 关卡无防护地读取 value.then，而上方一个函数的 markPromises() 对同一读取做了显式防护（注释："Keep scanning data properties after a hostile then getter throws"）——对同一值图的第二次遍历违背了第一次遍历已文档化的韧性，于是抛错的 then getter 会以模型可控文本中止提取，而不是给出序列化器设计的 thenable 判定。合法 meta 不受影响（WorkflowMeta 没有 then 成员），失败方向安全（拒绝）——但该不对称是非自愿的：作者已经决定过这种情形应当被容忍。— 故障场景：本提交上经真实子进程复现：{ get then() { throw new Error('boom') }, name: 'x', description: 'y' } → failed to serialize meta object literal: boom（模型可控文本）而非可操作的 META_PROMISE_ERROR；嵌套变体同理。修复：镜像 markPromises 的防护（见上方 suggestion），让 catch 显式选择判定。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| ])('rejects a Promise hidden behind a %s', async (_name, src) => { | ||
| const unhandled: unknown[] = []; | ||
| const onUnhandled = (reason: unknown) => unhandled.push(reason); |
There was a problem hiding this comment.
[Suggestion] R6-12: All 13 rejection-containment tests smuggle real Promise.reject(...) values, which the child's async_hooks PROMISE init hook catches at creation regardless of the serializer walk — so no test pins markPromises's descriptor/ownKeys/prototype scan breadth, the ONLY detection path for non-Promise thenables (hook guard: if (type !== 'PROMISE' || attaching) return;); the only plain-thenable test is the top-level case at line 571, caught by the direct root read, not the deep scan. — Failure scenario: mutant probe at this commit: narrowed the scan to Object.keys enumeration (no descriptors/symbols/prototype) → all 13 existing tests still pass (13 passed | 182 skipped), while hidden non-enumerable and symbol-keyed plain thenables flip from rejected to SILENTLY ACCEPTED — violating the documented "reject the meta literal up front" contract with no diagnostic.
witness: mutant arm → Tests 13 passed | 182 skipped; both hidden thenables accepted {"accepted":{..."meta":{"name":"x","description":"d"}}}; pristine arm rejects both with META_PROMISE_ERROR.
Add hidden-placement plain-thenable cases to the family (non-enumerable Object.defineProperty({}, 'hidden', { value: { then: () => {} } }) and a symbol-key thenable), each expecting /meta values must not be Promises/.
中文说明
全部 13 个 rejection 收容测试走私的都是真实 Promise.reject(...) 值，子进程的 async_hooks PROMISE init 钩子在创建时就已捕获，与序列化器遍历无关——因此没有测试固定 markPromises 的描述符/ownKeys/原型链扫描广度，而那是非 Promise thenable 的唯一检测路径（钩子守卫：if (type !== 'PROMISE' || attaching) return;）；唯一的普通 thenable 测试是 571 行的顶层用例，由根部直接读取捕获，而非深层扫描。— 故障场景：本提交上变异探测：把扫描收窄为 Object.keys 枚举（无描述符/symbol/原型链）→ 现有 13 个测试全部仍通过（13 passed | 182 skipped），而隐藏的非枚举与 symbol 键普通 thenable 由被拒绝翻转为被静默接受——违背文档化的 "reject the meta literal up front" 契约且无任何诊断。修复：向该族新增隐藏位置的普通 thenable 用例（非枚举 Object.defineProperty({}, 'hidden', { value: { then: () => {} } }) 与 symbol 键 thenable），各期望 /meta values must not be Promises/。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| hook.enable(); | ||
| try { | ||
| const value = run(); | ||
| if (createdPromise) throw new Error(META_PROMISE_ERROR); |
There was a problem hiding this comment.
[Suggestion] R6-13: A meta literal that creates a Promise with a looping reaction leaves the child's async-hook stack imbalanced when V8's timeout interrupts the microtask drain, so the isolated-evaluator child crashes at script teardown with Error: async hook stack has become corrupted (actual: 4, expected: 1) (native trace from node::AsyncHooks::pop_async_context), exit status 1 — the sandbox child crashes on model-controlled input. The envelope is still delivered (written before teardown; the host ignores result.status/stderr), so the accept/reject decision stays correct (rejection), but the verdict is a timeout instead of META_PROMISE_ERROR, and envelope delivery relies on libuv flushing the stdout write before the teardown corruption check aborts the process — an ordering the code does not control. — Failure scenario: reproduced 3/3 at this commit through the real META_CHILD_SOURCE: input { name: (() => { Promise.resolve(0).then(() => { while (true) {} }); return 'n'; })(), description: 'd' } → child status 1, stderr 'async hook stack has become corrupted', host surfaces 'Script execution timed out after 250ms' instead of META_PROMISE_ERROR.
witness: child capture: status: 1, stderr Error: async hook stack has become corrupted (actual: 4, expected: 1) + pop_async_context trace; stdout {"ok":false,"error":"...Script execution timed out after 250ms"}.
Reconsider the async-hooks observer (the serializer's thenable walk plus the child's process.on('unhandledRejection') listener already contain reachable and dangling promises, and the hook is what makes the timeout interrupt leave the async stack imbalanced); if kept, tolerate the teardown crash explicitly rather than relying on it being host-invisible by accident.
中文说明
创建带循环反应 Promise 的 meta 字面量，会在 V8 超时中断微任务排空时使子进程的 async-hook 栈失衡，隔离求值子进程在脚本收尾时以 Error: async hook stack has become corrupted (actual: 4, expected: 1)（node::AsyncHooks::pop_async_context 的原生 trace）崩溃，退出状态 1——沙箱子进程在模型可控输入上崩溃。信封仍被送达（先于收尾写入；宿主忽略 result.status/stderr），故接受/拒绝决定仍正确（拒绝），但判定是超时而非 META_PROMISE_ERROR，且信封送达依赖 libuv 在收尾破坏性检查中止进程之前刷出 stdout 写入——这个顺序不受代码控制。— 故障场景：本提交上经真实 META_CHILD_SOURCE 复现 3/3：输入 { name: (() => { Promise.resolve(0).then(() => { while (true) {} }); return 'n'; })(), description: 'd' } → 子进程 status 1，stderr 'async hook stack has become corrupted'，宿主浮现 'Script execution timed out after 250ms' 而非 META_PROMISE_ERROR。修复方向：重新审视 async-hooks 观察器（序列化器的 thenable 遍历加子进程的 process.on('unhandledRejection') 监听器已能容接受可达与游离 Promise，而钩子正是让超时中断留下失衡 async 栈的原因）；若保留，请显式容忍收尾崩溃，而不是依赖它碰巧对宿主不可见。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| function createMetaContext() { | ||
| return vm.createContext(Object.create(null), { | ||
| microtaskMode: 'afterEvaluate', |
There was a problem hiding this comment.
[Suggestion] R6-14: With microtaskMode: 'afterEvaluate', a looping promise reaction scheduled by the literal is drained inside the evaluate runInContext, so when it consumes the 250ms budget the drain's timeout exception replaces the script's genuine pending exception — the real evaluation error is silently lost (the same mechanism also lets metaStageError's own drainMetaMicrotasks replace a formatted error with a drain-timeout error on other orderings). — Failure scenario: reproduced at this commit through the real child: { name: (x => { Promise.resolve(0).then(() => { while (true) {} }); throw new Error('real cause'); return x; })(1), description: 'd' } → envelope {"ok":false,"error":"...failed to evaluate meta object literal: Script execution timed out after 250ms"} — the author-visible error says "timed out" even though evaluation failed instantly with 'real cause'; a script author fixing a broken meta literal gets a timeout diagnostic pointing at nothing.
witness: PR child stdout for that input: ...failed to evaluate meta object literal: Script execution timed out after 250ms — 'real cause' lost.
Fix direction: drain microtasks explicitly under microtaskMode: 'manual' so the script's pending exception is surfaced before any reaction runs, or capture the evaluation exception before draining.
中文说明
在 microtaskMode: 'afterEvaluate' 下，字面量调度的循环 promise 反应会在 evaluate 的 runInContext 内被排空，当其耗尽 250ms 预算时，排空超时异常会替换脚本真正的待抛异常——真实求值错误被静默丢失（同一机制在其他顺序下也会让 metaStageError 自身的 drainMetaMicrotasks 用排空超时错误替换已格式化的错误）。— 故障场景：本提交上经真实子进程复现：{ name: (x => { Promise.resolve(0).then(() => { while (true) {} }); throw new Error('real cause'); return x; })(1), description: 'd' } → 信封 {"ok":false,"error":"...failed to evaluate meta object literal: Script execution timed out after 250ms"}——作者可见的错误说 "timed out"，尽管求值瞬间就以 'real cause' 失败；修复坏 meta 字面量的脚本作者得到一个什么都不指向的超时诊断。修复方向：在 microtaskMode: 'manual' 下显式排空微任务，使脚本的待抛异常先于任何反应运行而浮现，或在排空前捕获求值异常。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| const keys = ownKeys(descriptors); | ||
| for (let i = 0; i < keys.length; i++) { | ||
| const descriptor = descriptors[keys[i]]; | ||
| if ('value' in descriptor) markPromises(descriptor.value); |
There was a problem hiding this comment.
[Suggestion] R6-15: markPromises skips accessor descriptors entirely and copy only reads own enumerable keys, so a thenable hidden behind a non-enumerable ACCESSOR property is silently accepted (field dropped) while its two twin shapes are rejected — inconsistent with the "reject the meta literal up front" contract, and unpinned by any test. Security impact is nil (the getter is never invoked, nothing reaches the host); this is a detection-consistency/diagnostic gap in code newly added by this PR. — Failure scenario: reproduced at this commit: { extra: Object.defineProperty({}, 'hidden', { get: () => ({ then: () => {} }) }) } is ACCEPTED ({"name":"x","description":"d"}, extra silently dropped), while the non-enumerable DATA property and enumerable ACCESSOR twins are both REJECTED with META_PROMISE_ERROR.
witness: UNPATCHED: non-enumerable ACCESSOR → ACCEPTED; non-enumerable DATA → REJECTED; enumerable ACCESSOR → REJECTED. PATCHED (accessor descriptors flagged without invoking getter): non-enumerable ACCESSOR → REJECTED (flips).
Either extend detection to accessor descriptors in markPromises without invoking the getter (treating a non-enumerable accessor as a thenable suspect — flagging ALL accessors would break the accepted enumerable-accessor cases), or add a test documenting the accepted-drop as intended behavior for this shape.
中文说明
markPromises 完全跳过访问器描述符，且 copy 只读取自身的可枚举键，因此藏在非枚举访问器属性后的 thenable 会被静默接受（字段被丢弃），而它的两个孪生形状却被拒绝——与 "reject the meta literal up front" 契约不一致，且无任何测试固定。安全影响为零（getter 从未被调用，没有任何东西到达宿主）；这是本 PR 新增代码中的检测一致性/诊断缺口。— 故障场景：本提交上复现：{ extra: Object.defineProperty({}, 'hidden', { get: () => ({ then: () => {} }) }) } 被接受（{"name":"x","description":"d"}，extra 静默丢弃），而非枚举数据属性与可枚举访问器两个孪生形状均以 META_PROMISE_ERROR 被拒绝。修复：要么在 markPromises 中把检测扩展到访问器描述符且不调用 getter（把非枚举访问器视为 thenable 嫌疑——标记所有访问器会破坏被接受的可枚举访问器用例），要么新增测试把该形状的接受-丢弃记录为预期行为。
— qwen3.8-max via Qwen Code /review (v0.21.12)
Independent verification on a real local stack (macOS)I rebuilt both sides of this PR and ran them as the real CLI — production bundle, real Verdict: the hang is real, it is reachable from an ordinary model turn, and this PR removes it with no regression I could find on well-formed workflows. One small defect in the new code is worth fixing before merge (§5), plus a description/implementation drift worth correcting (§6). Setup (click to expand)
1. The hang reproduces through the product, not just through
|
| meta literal | main (6ebc79f) |
PR #9136 (ab0ba69) |
|---|---|---|
| well-formed control | ok 110 ms | ok 104 ms |
| loop during evaluation | HUNG → SIGKILL @15 s | err 344 ms · failed to evaluate … timed out after 250ms |
get phases() { while(true){} } |
HUNG → SIGKILL @15 s | err 348 ms · failed to serialize … |
looping getter nested in phases[] |
HUNG → SIGKILL @15 s | err 346 ms · failed to serialize … |
| microtask-deferred loop | returns ok in 31 ms, then the process wedges → SIGKILL @15 s | err 334 ms · failed to serialize … |
new Array(2**26).fill(1).sort() |
ok — after 11.8 s of blocked event loop | err 2047 ms · isolated evaluator exceeded 2000ms |
Promise value |
err (same text) | err (same text) |
200 000-char name |
ok | err · meta literal is too large ← newly refused (documented) |
CJK + emoji name/phases |
ok | ok (round-trips identically) |
300-entry phases[] |
ok | ok |
| cycle below top level | validation err | same validation err |
| literal throws | err …: Error: boom |
err …: boom ← class-name prefix dropped |
typeof process / require / args inside the literal |
undefined / undefined / undefined, 67 globals |
identical — the child does not widen the literal's reach |
The sort() row is the one that justifies the child process over a plain in-vm timeout: — V8 never reaches an interrupt point there, so on main it simply blocks for 11.8 s.
3. Well-formed workflows are untouched end to end
meta.name / description / phases[] survive the new child + JSON round trip, log() output and the script's return value are unchanged.
4. Cost of the child process (measured, not estimated)
extractAndStripMeta on a well-formed literal, 60 iterations:
| median | p95 | max | |
|---|---|---|---|
main |
0.22 ms | 0.33 ms | 0.69 ms |
| PR #9136 | 35.4 ms | 37.7 ms | 40.3 ms |
| PR #9136, machine at load avg 91 on 10 cores | 53.0 ms | 94.6 ms | 115.2 ms |
So this adds ~35 ms of synchronous event-loop block per workflow invocation (~160×, but small in absolute terms next to any real run), and a script without a meta block still costs 0 ms (no child is spawned). Worth noting: even at load average 91 the worst observed cost was 115 ms, i.e. a 17× margin below META_CHILD_TIMEOUT_MS — the 2 s ceiling does not look like a false-failure risk on a loaded machine. --max-old-space-size=256 and the macOS seatbelt profiles ((allow process-exec)) are both fine for the child.
5. Finding — a healthy workflow dies with a TypeError when the child cannot be spawned
evaluateMetaIsolated computes stage before it checks result.error:
const stage = result.stderr.includes(META_CHILD_SERIALIZE_MARKER) ? 'serialize' : 'evaluate';
if (result.error) { … }When the child cannot be spawned at all, Node leaves result.stdout/result.stderr undefined, so the line above throws before the carefully-written diagnostic can be produced:
TypeError: Cannot read properties of undefined (reading 'includes')
Reproduced three ways on the PR build: (a) through the compiled module with the node binary missing (upgrade/nvm switch mid-session), (b) through the compiled module under ulimit -u 25, where fork() returns EAGAIN — precisely the process pressure a large workflow fan-out creates — and (c) in the real TUI (screenshot above), where a perfectly well-formed workflow fails with that message.
error: EAGAIN / spawnSync …/bin/node EAGAIN
stdout: undefined stderr: undefined
Fix is one line — (result.stderr ?? ''), or move the stage computation below the result.error branch. Nothing in the 195 tests distinguishes the two versions (I applied the fix and re-ran: still 195 green), so a small spawnSync-mocked case would be worth adding alongside it.
6. Tests, and which guards are real
workflow-sandbox.test.ts: 195 passed on Node 24.18.1 and Node 22.23.2 (macOS).- Wider blast radius
src/agents/runtime/+src/tools/workflow/: 681 passed, 6 skipped, 16 files — no regression. - Mutation matrix (flip one line of the fix, run the PR's own tests):
| mutation | result |
|---|---|
| none | 195 pass |
drop timeout: META_CHILD_TIMEOUT_MS (i.e. in-vm timeout only) |
1 fail — bounds a long native builtin ✅ |
drop drainMetaMicrotasks(metaContext) |
1 fail ✅ |
drop if (createdPromise) throw … |
2 fail ✅ |
META_SERIALIZED_MAX_CHARS 64 KiB → 64 MiB |
2 fail ✅ |
drop timeout: META_EVAL_TIMEOUT_MS on the literal-evaluation script |
195 pass |
The last row is the only soft spot: with that timeout gone the outer 2 s child timeout still catches everything, so the suite stays green (the same cases just take ~2 s instead of ~350 ms). An assertion on elapsed time (< 1 s) or on the specific timed out after 250ms text would pin that layer.
7. Description drift (docs only)
The PR body still describes the earlier design — "The fix runs both halves inside the vm, each under a 250ms timeout" — and never mentions what ab0ba69 actually ships: a spawnSync child process with META_CHILD_TIMEOUT_MS = 2000, --max-old-space-size=256, NODE_OPTIONS / NODE_V8_COVERAGE scrubbing, and stage attribution over a stderr marker. A reviewer reading only the description reviews the wrong design, and the "Risk & Scope" section does not mention the new per-invocation process spawn. Worth a refresh before merge.
Not covered here
Windows (spawnSync + windowsHide), --sandbox docker mode, and the qwen serve daemon path — I only checked that the macOS seatbelt profiles permit process-exec.
中文版本
在本地真实环境上的独立验证（macOS）
我把这个 PR 的两侧都重新构建，并以真实 CLI 的形态跑起来——生产包、真实 Workflow 工具、tmux 下的真实 TUI，配一个记录型 mock OpenAI provider，让模型精确吐出描述里那个恶意 meta 字面量。Tested on 表里 macOS 是 N/A，这份报告把这一列补上。
结论：挂死是真实的、能从一次普通的模型回合直接触达，本 PR 确实消除了它，而且我没有发现良构 workflow 上的任何回归。 新代码里有一处小缺陷建议合并前修掉（§5），另外 PR 描述与实现已经脱节，值得更新（§6）。
环境
| Base | 6ebc79f（与 main 的 merge-base） |
| Head | ab0ba69（fix/workflow-meta-eval-bounds） |
| 方法 | 同一个 worktree，只替换 workflow-sandbox.ts，各自 npm run build + npm run bundle → 两个独立的 dist/cli.js（已核对：base 包内不含 __qwen_meta_serialize__，head 包内含） |
| 主机 | macOS 26.6，arm64，10 核，Node 24.18.1 与 22.23.2 |
| 驱动 | mock OpenAI SSE provider（带 usage 帧）→ 携带待测 meta 字面量的 workflow 工具调用；QWEN_CODE_ENABLE_WORKFLOWS=1、隔离 QWEN_HOME、YOLO 审批 |
1. 挂死能在产品层复现，而不只是 node -e
同样的提示词、同样的工具调用，两个构建（见上方第一张图）：
main 上 CLI 永久卡死：spinner 冻结在 0s（它自己的定时器根本无法触发），ESC、两次 Ctrl+C、/quit 以及后续输入全部无响应——前后抓取的 pane 逐字节相同——最终只有 SIGKILL 能结束它。对卡死进程做 sample(1)，1617/1617 个采样都落在 MicrotaskQueue::PerformCheckpoint → Runtime_ObjectValuesSkipFastPath → GetPropertyWithAccessor，也就是宿主侧 Object.values() 遍历在调用模型写的 get phases()。这正是本 PR 要挪进受限子进程的那段遍历——main 的故障模式在栈层面被证实，而不是推断。
同样的输入在本 PR 上变成一个普通的工具错误（348 ms），会话继续存活。
2. 行为矩阵——两个构建，每个字面量独立进程
挂死用例在 15 秒时被硬杀。
| meta 字面量 | main (6ebc79f) |
PR #9136 (ab0ba69) |
|---|---|---|
| 良构对照 | ok 110 ms | ok 104 ms |
| 求值期死循环 | 挂死 → 15s SIGKILL | err 344 ms · failed to evaluate … 250ms |
get phases() { while(true){} } |
挂死 → 15s SIGKILL | err 348 ms · failed to serialize … |
phases[] 内嵌套的死循环 getter |
挂死 → 15s SIGKILL | err 346 ms · failed to serialize … |
| 微任务延迟的死循环 | 31 ms 就正常返回，随后进程卡死 → 15s SIGKILL | err 334 ms · failed to serialize … |
new Array(2**26).fill(1).sort() |
ok——但事件循环被阻塞了 11.8 秒 | err 2047 ms · isolated evaluator exceeded 2000ms |
Promise 值 |
err（文案一致） | err（文案一致） |
20 万字符的 name |
ok | err · meta literal is too large ← 新增拒绝（已在描述中说明） |
中文 + emoji 的 name/phases |
ok | ok（往返完全一致） |
300 项 phases[] |
ok | ok |
| 非顶层的循环引用 | 校验错误 | 同样的校验错误 |
| 字面量抛错 | err …: Error: boom |
err …: boom ← 少了错误类名前缀 |
字面量内 typeof process / require / args |
undefined / undefined / undefined，67 个全局 |
完全相同——子进程没有扩大字面量的可达面 |
sort() 那一行正是「为什么需要子进程、而不是只加一个 vm timeout:」的证据：V8 在那里根本到不了中断点，所以 main 上就是实打实阻塞 11.8 秒。
3. 良构 workflow 端到端无变化
meta.name / description / phases[] 都完整穿过新的子进程 + JSON 往返，log() 输出与脚本返回值不变（见上方第二张图）。
4. 子进程的开销（实测，不是估算）
良构字面量上的 extractAndStripMeta，各 60 次：
| 中位数 | p95 | 最大 | |
|---|---|---|---|
main |
0.22 ms | 0.33 ms | 0.69 ms |
| PR #9136 | 35.4 ms | 37.7 ms | 40.3 ms |
| PR #9136（10 核机器负载 91 时） | 53.0 ms | 94.6 ms | 115.2 ms |
也就是每次 workflow 调用新增约 35 ms 的同步事件循环阻塞（约 160×，但相对任何真实 run 的绝对量很小）；没有 meta 块的脚本仍然是 0 ms（不会 spawn 子进程）。值得一提：即使负载 91，最坏也只有 115 ms，距离 META_CHILD_TIMEOUT_MS 还有 17 倍余量——2 秒上限在高负载机器上看起来不构成误杀风险。--max-old-space-size=256 和 macOS seatbelt 配置（(allow process-exec)）对子进程都没问题。
5. 发现——子进程起不来时，良构 workflow 会以 TypeError 失败
evaluateMetaIsolated 在检查 result.error 之前就算了 stage：
const stage = result.stderr.includes(META_CHILD_SERIALIZE_MARKER) ? 'serialize' : 'evaluate';
if (result.error) { … }当子进程根本起不来时，Node 会把 result.stdout/result.stderr 置为 undefined，于是上面这行先抛错，精心构造的诊断信息根本没机会产生：
TypeError: Cannot read properties of undefined (reading 'includes')
在 PR 构建上用三种方式复现：(a) 编译产物 + node 二进制消失（会话中途升级 / nvm 切换）；(b) 编译产物 + ulimit -u 25，此时 fork() 返回 EAGAIN——这恰恰是大规模 workflow 扇出会造出的进程压力；(c) 真实 TUI（上方第三张图），一个完全良构的 workflow 直接以该消息失败。
修复只需一行——(result.stderr ?? '')，或者把 stage 的计算挪到 result.error 分支之后。现有 195 个测试无法区分这两个版本（我把修复打上后重跑，依然 195 全绿），所以建议顺手补一个 mock spawnSync 的用例。
6. 测试，以及哪些护栏是真的
workflow-sandbox.test.ts：Node 24.18.1 与 Node 22.23.2（macOS）下均 195 通过。- 更大范围
src/agents/runtime/+src/tools/workflow/：681 通过、6 跳过、16 个文件——无回归。 - 变异矩阵（每次只改本 PR 的一行，再跑 PR 自己的测试）：
| 变异 | 结果 |
|---|---|
| 不变异 | 195 通过 |
去掉 timeout: META_CHILD_TIMEOUT_MS（即只剩 vm 内超时） |
1 失败——bounds a long native builtin ✅ |
去掉 drainMetaMicrotasks(metaContext) |
1 失败 ✅ |
去掉 if (createdPromise) throw … |
2 失败 ✅ |
META_SERIALIZED_MAX_CHARS 64 KiB → 64 MiB |
2 失败 ✅ |
去掉字面量求值脚本上的 timeout: META_EVAL_TIMEOUT_MS |
195 通过 |
最后一行是唯一的软肋：去掉它之后，外层 2 秒子进程超时仍然兜得住，所以测试全绿（只是那几个用例从 ~350 ms 变成 ~2 s）。加一条耗时断言（< 1 s）或断言 timed out after 250ms 文案，就能把这一层钉住。
7. 描述与实现脱节（仅文档）
PR 正文仍在描述更早的方案——「把两个阶段都放进 vm 执行，各自受 250ms 超时约束」——完全没有提到 ab0ba69 实际交付的东西：一个 spawnSync 子进程、META_CHILD_TIMEOUT_MS = 2000、--max-old-space-size=256、剥离 NODE_OPTIONS / NODE_V8_COVERAGE，以及用 stderr 标记做阶段归因。只读描述的评审者会去评审一个不存在的设计；「风险与范围」一节也没有提到新增的「每次调用 spawn 一个进程」。建议合并前刷新。
未覆盖
Windows（spawnSync + windowsHide）、--sandbox docker 模式，以及 qwen serve daemon 路径——我只核对了 macOS seatbelt 配置允许 process-exec。
|
Closing in favour of #9325. This branch was right about the direction and the review rounds here were right about the substance — each round found a real evaluation hazard the previous fix did not cover: the getter walked on the host, then the serializer sharing a lexical scope with the literal, then promise reactions under Six rounds in, the question stopped being "how do we bound this evaluation" and became "why are we evaluating it at all". Every field in the meta contract is a string, and upstream's own rule is that the meta object must be a pure literal — no variables, calls, spreads or interpolation. Given that contract, evaluation was the wrong mechanism from the start. #9325 parses the literal instead. A parser has no execution semantics, so the getter, the promise reaction, the proxy trap and the large allocation are not bounded — they cannot be written. The vm context, the timeout, the thenable walker and the child process all go away. It is a contract narrowing, so the surface was measured rather than assumed: all 42 meta literals in the existing suite were run through both paths — 19 identical, 13 rejected by both, 10 refused only by the parser (nine of them the hostile fixtures from these review rounds, the tenth a regex in a non-contract field), and zero cases where both accept and produce different values. Parsing is also ~70x cheaper than the vm path, which matters because this call is on the way to the confirmation dialog and saved-workflow enumeration. Thanks for the six rounds — the findings here are what made it clear the mechanism, not the bound, was the problem. 中文说明关闭本 PR，由 #9325 取代。 这个分支的方向是对的，这里几轮评审在实质上也都是对的——每一轮都找出了上一次修复没覆盖到的真实求值风险：先是在宿主侧遍历时触发的 getter，然后是序列化器与字面量共享词法作用域，接着是 到第六轮，问题已经不再是"怎么给这次求值加边界"，而是"我们究竟为什么要求值"。meta 契约里每个字段都是字符串，而上游自己的规则就是：meta 对象必须是纯字面量——不允许变量、调用、spread 或插值。既然契约如此，求值从一开始就是选错了机制。 #9325 改为解析该字面量。解析器没有执行语义，因此 getter、promise reaction、proxy trap、大块分配都不是"被框住"——它们写不出来。vm context、超时、thenable 遍历器、子进程，全部消失。 这是一次契约收窄，所以变更面是实测而非估计：现有测试套件里全部 42 个 meta 字面量都走了两条路径——19 个结果相同、13 个两者都拒绝、10 个仅被解析器拒绝（其中 9 个正是这几轮评审里的敌对样本，第 10 个是非契约字段里的一个正则），以及0 个"两者都接受但取值不同"。解析同时比 vm 路径便宜约 70 倍，这一点很重要，因为这个调用正通往确认对话框与已保存 workflow 的枚举。 感谢这六轮评审——正是这些发现让人看清：问题出在机制上，而不在边界上。 |
…ion (QwenLM#9340) * feat(review): say when the approach, not the patch, is the open question Every finding /review emits is anchored to a `file:line` in the current diff. That is what a finding is — and it means a review can report where an approach leaks, but never that a different approach would retire all of the leaks at once. Measured: one change to `extractAndStripMeta` took three attempts across two PRs. QwenLM#9097 (3 rounds, 18 findings) added a timeout to the vm call; QwenLM#9136 (6 rounds, 56 findings) moved the walk inside the vm and ended up spawning a child process per call, growing 228 -> 920 source diff lines. QwenLM#9325 landed it in one commit by not evaluating the literal at all. All 74 findings were individually correct, and every one of them went away with the mechanism. The signal was already there and filed as the wrong kind of thing: `did not converge within the reverse-audit round cap` appeared four times across the two PRs, as a coverage gap — "we did not finish looking" — rather than as a conclusion about the change. Nothing was responsible for reading it as "stop patching". Add one advisory paragraph, and one clause on the terminal verdict line, when a non-Approve round is past the round threshold AND its source diff has grown at least 3x since the review first measured it. This round's round-cap stop rides along as corroborating text when present; it is never a trigger on its own. It is deliberately not a finding. Findings are what the autofix loop consumes, and that loop patching each finding in turn is the pattern being interrupted — a finding here would be fixed rather than read. It addresses the human deciding what happens next, so it is a body paragraph and a verdict-line clause, it adds no cap, and it never moves the event. The baseline is a baseline, not the previous round's size: 228 -> 920 across six rounds is ~1.3x per round, which no per-round delta would notice, but 4.0x cumulatively. `Ledger.src0` records the first measurement and is carried forward unchanged, so a diff that later shrinks cannot rewrite its own baseline. It is the one marker field that survives truncation — the ruling that withholds an anchor from a partial finding list does not extend to a measurement of the diff. Known limits, documented rather than papered over: it cannot see across pull requests, so the three-attempt shape that motivated it would have fired only on a second forgeable persisted counter; and it is retroactively blank, staying silent until a PR has posted two rounds after this ships. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(review): suppress approach signal for downgraded approvals * fix(review): measure approach growth over full diff * fix(review): validate approach signal evidence * fix(review): pin approach-signal boundaries and validator coverage Round-5 review findings: boundary tests for the round threshold, growth factor, and source-diff floor; the round-cap corroborating clause and its zh rendering; src0 survival through the pr-context persist seam and the incremental marker carry-forward; artifact validator refusal/absence tests for approachSignal; design doc firing list names the pre-cap verdict. * fix(review): clamp the approach signal's round at the ledger cap (R9-1) The signal computed its displayed round with an unclamped `prevRound + 1` while the ledger marker stamp and the deferred-suggestions clause both clamp with `Math.min(prevRound + 1, LEDGER_MAX_ROUND)`. `parseLedger` accepts `round == LEDGER_MAX_ROUND`, so a side file at the cap is representable and carries forward: one composed body announced "⚠️ Round 10001" beside a marker stamping `"round":10000`, and the terminal verdict line printed 10001 too — the doc comment in this same diff claims all three consumers cannot disagree "at the cap included". The new test pins the cap for the third consumer, mirroring the existing deferred-clause cap test; mutation-verified that reverting the clamp turns it red with `round: 10001`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: qqqys <266654365+qqqys@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>
3 participants
What this PR does
Bounds both halves of workflow
metaextraction with a vm timeout, by moving the walk over the evaluated value into the vm alongside the evaluation itself. Ametaliteral that never returns now surfaces as the ordinary malformed-meta error instead of wedging the process.Supersedes #9097 — see "Relationship to #9097" below.
Why it's needed
extractAndStripMetaevaluates the model-authoredexport const meta = {...}object literal inside a fresh vm realm, then walks the resulting value on the host to reject stray Promises and copy it into host-realm objects. Neither half is bounded, and each one hangs the process on its own:The loops are synchronous, so this is not merely slow — the event loop is blocked outright. No timer fires, no signal handler runs, and nothing left in the process can cancel it. The second form is the more instructive one: a getter defers its work to property-read time, so the literal evaluates instantly and only spins when something reads the value. A timeout placed on the literal's evaluation alone never fires for it.
The literal is model-authored source, which makes a non-terminating expression a realistic accident, not only an adversarial construction. The path is reachable today:
extractAndStripMetaruns insidesandbox.runbefore the run path's own 30s body timeout and wall-clock watchdog are armed. It also blocks two natural follow-ups that would move this call somewhere strictly worse — rendering the workflow's name and phases in the tool-confirmation dialog (hanging the TUI before the user has consented to anything) and reading each saved workflow's description for the slash-command palette (hanging startup with no run to abort).The fix runs both halves inside the vm, each under a 250ms timeout. Each context uses
microtaskMode: 'afterEvaluate', and the source realm is explicitly drained after serializer getters run, so Promise continuations execute inside a boundedrunInContextcall instead of escaping to an unbounded host microtask checkpoint. The walk remains a second vm script that serialises the value to JSON for the host to parse.The two scripts stay separate programs deliberately, and this is the part worth reviewing closely. The serializer's source is fixed and interpolates nothing, so the model's literal never shares a lexical scope with it — a getter closes over its own program's scope, not the serializer's. Folding the literal into the serializer's scope instead would let it read the helper bindings and assign to the flag that records whether a thenable was found, which is precisely the check that keeps a stray rejected Promise from terminating the host on the next tick. That check keeps its existing behaviour and error text; it now runs during the in-vm walk, so the host-side
rejectThenablesInMetatraversal is no longer needed and is removed.The serializer additionally carries a 64 KiB budget over the values it copies: a 250ms window is long enough to build a very large string, and the JSON round trip would then copy it twice more.
Relationship to #9097
#9097 proposed the minimal form of this fix — a
timeouton the existingrunInContextcall. Review correctly found that it bounds only the literal's own evaluation, leaving the host-side walk unbounded, which the getter case above demonstrates. Subsequent automated fixes on that branch moved the walk into the vm (the right direction) but interpolated the literal into the serializer's own lexical scope, which review then flagged as critical, and added a process-globalpromiseHooksinterceptor. That branch did not converge across three review rounds.This PR restarts from
mainwith the two-program structure that makes scope isolation hold by construction rather than by careful escaping. Reachable thenables are detected structurally during the in-vm walk, while a scoped async-hooks observer marks every Promise created during bounded evaluation as handled, including abort paths and Promises outside the returned graph. VM microtasks are drained under the same timeout. #9097 is closed in favour of this.Reviewer Test Plan
How to verify
cd packages/core && npx vitest run src/agents/runtime/workflow-sandbox.test.ts— 190 passed on Node 22.17.0.The current round adds seven regressions for microtask-deferred loops and rejections, hostile error-message getters, thrown-string diagnostics, and evaluation-versus-serialization error attribution. Existing expectations were updated where serializer failures now intentionally use the
failed to serializeprefix.To confirm the tests are real guards, revert only
workflow-sandbox.tsand run thebounded evaluationblock. It does not fail — it hangs, and vitest's own--testTimeoutcannot rescue it, because the worker's event loop is blocked and the timeout timer never fires. That is the failure mode this PR removes, and it is why the added tests assert an elapsed-time bound rather than relying on the test runner's timeout.The two hangs can also be reproduced directly against
main:The second is the one that matters: it passes the 250ms timeout untouched and hangs on the host-side property read.
Evidence (Before & After)
N/A — no user-visible or TUI change. This is a robustness fix in the workflow sandbox.
Tested on
Environment (optional)
Unit tests only, Node 22.23.0 on Linux.
Risk & Scope
validateMetaaccepts, and it strengthens the host-realm guarantee the previous per-field copy provided by hand. A literal needing more than 250ms or producing more than 64 KiB is newly refused; both constants are named and commented.Linked Issues
Supersedes #9097.
中文说明
这个 PR 做了什么
把 workflow
meta提取的两个阶段都用 vm 超时框住——办法是把对求值结果的遍历也挪进 vm，与求值本身放在一起。一个永不返回的meta字面量现在会以常规的 meta 格式错误暴露出来，而不是把进程卡死。取代 #9097 —— 见下方"与 #9097 的关系"。
为什么需要
extractAndStripMeta会在一个全新的 vm realm 里对模型编写的export const meta = {...}对象字面量求值，然后在宿主侧遍历结果，以拒绝游离的 Promise 并把值拷贝成宿主 realm 的对象。这两个阶段都没有边界，而且各自都能单独卡死进程：这些循环是同步的，所以这不只是"慢"——事件循环被直接阻塞：定时器不触发，信号处理器不运行，进程内没有任何东西能取消它。第二种形式更说明问题：getter 把工作推迟到了属性读取时，所以字面量本身瞬间就返回了，只有当有人去读这个值时才开始空转。只给字面量求值加超时，对它永远不会触发。
字面量是模型编写的源码，因此不终止的表达式是现实中会发生的意外，而不仅是对抗性构造。这条路径今天就可达：
extractAndStripMeta在sandbox.run内部执行，早于运行路径自身的 30s 脚本体超时和墙钟看门狗被装上。它还挡住了两个自然的后续改动，而那两处的后果严格更糟——在工具确认对话框里渲染 workflow 的名称和阶段（会在用户尚未授权任何东西之前卡死 TUI），以及为斜杠命令面板读取每个已保存 workflow 的描述（会在没有任何 run 可中止的情况下卡死启动）。修复方案是把两个阶段都放进 vm 执行，各自受 250ms 超时约束。每个上下文都启用
microtaskMode: 'afterEvaluate'，序列化 getter 执行后还会显式排空源 realm 的微任务，因此 Promise 续体会在有超时的runInContext内执行，不会逃到宿主的无界微任务检查点。遍历仍是第二段 vm 脚本，把值序列化成 JSON 交给宿主解析。两段脚本是刻意分开的两个程序，这是最值得仔细审阅的部分。 序列化器的源码是固定的、不做任何插值，因此模型的字面量永远不与它共享词法作用域——getter 闭包在它自己那个程序的作用域上，而不是序列化器的。反过来，如果把字面量折叠进序列化器的作用域，它就能读到那些辅助绑定，并且能给"是否发现了 thenable"那个标志赋值——而这个检查正是阻止游离的 rejected Promise 在下一个 tick 终结宿主进程的那道防线。该检查保持原有行为和错误文案不变，只是改为在 vm 内的遍历过程中完成，因此宿主侧的
rejectThenablesInMeta遍历不再需要，已一并删除。序列化器另外带了一个 64 KiB 的拷贝预算：250ms 的窗口足够构造出一个非常大的字符串，而 JSON 往返还会再把它复制两遍。
与 #9097 的关系
#9097 提出的是本修复的最小形式——给已有的
runInContext调用加一个timeout。评审正确地指出：它只框住了字面量自身的求值，宿主侧的遍历仍然没有边界，上面的 getter 案例即是证明。该分支上后续的自动化修复把遍历挪进了 vm（方向是对的），但把字面量插值进了序列化器自己的词法作用域，随后被评审判为 critical，并且引入了进程级的promiseHooks拦截器。那个分支经过三轮评审仍未收敛。本 PR 从
main重新开始，采用两段式程序结构——作用域隔离是由结构保证的，而不是靠小心翼翼的转义。可达 thenable 仍在 vm 内按结构识别；同时使用作用域内的 async-hooks 观察器，在创建时处理有界求值期间产生的每个 Promise，包括中断路径和返回图之外的 Promise。VM 微任务也在同一超时内排空。#9097 将被关闭，由本 PR 取代。审阅者验证方案
如何验证
cd packages/core && npx vitest run src/agents/runtime/workflow-sandbox.test.ts——Node 22.17.0 下 190 项通过。本轮新增 7 个回归，覆盖微任务延迟的死循环与 rejection、恶意错误 message getter、抛出字符串的诊断，以及求值/序列化错误阶段区分。序列化失败现在会按预期使用
failed to serialize前缀，相关旧断言已同步更新。要确认这些测试是真正的护栏，只回退
workflow-sandbox.ts并运行bounded evaluation这一组。它不会失败——它会挂起，而且 vitest 自己的--testTimeout也救不了，因为 worker 的事件循环被阻塞，超时定时器根本无法触发。这正是本 PR 消除的故障模式，也是新增测试断言"耗时上界"而不是依赖测试框架超时的原因。这两个挂死也可以直接在
main上复现：第二条才是重点：它毫发无伤地穿过了 250ms 超时，然后在宿主侧的属性读取上挂死。
证据（前后对比）
N/A——没有用户可见或 TUI 层面的变化。这是 workflow sandbox 内部的健壮性修复。
测试环境
环境（可选）
仅单元测试，Linux 上的 Node 22.23.0。
风险与范围
validateMeta能接受的任何形状都是保形的，而且它比先前手写的逐字段拷贝更强地保证了宿主 realm 归属。需要超过 250ms 或产出超过 64 KiB 的字面量会被新拒绝；两个常量都已命名并加了注释。关联 Issue
取代 #9097。