fix: run workflows on the shipped binary through a QuickJS sandbox - #187
Merged
Merged
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
QuickJS deferred promises are not disposed after normal host-call settlement, causing a critical handle leak.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
What changed in this PR
Migrates workflow execution from isolated-vm to a bundled QuickJS WebAssembly sandbox for consistent runtime support.
Changes:
- Consolidates sandbox execution and lifecycle management.
- Removes native runtime dependencies and gating.
- Updates tests, build metadata, and documentation.
| File | Summary |
|---|---|
scripts/build-coding-agent-bundle.mjs |
Removes native VM externalization. |
pnpm-lock.yaml |
Updates dependency lock data. |
packages/coding-agent/test/workflow-vm.test.ts |
Adds QuickJS sandbox coverage. |
packages/coding-agent/test/workflow-vm-conformance.test.ts |
Removes obsolete dual-runtime tests. |
packages/coding-agent/test/workflow-ultraloop-opt-in.test.ts |
Updates opt-in coverage. |
packages/coding-agent/test/workflow-registration.test.ts |
Updates registration tests. |
packages/coding-agent/test/workflow-extension.test.ts |
Enables workflow tests across runtimes. |
packages/coding-agent/src/features/workflow/vm.ts |
Implements the QuickJS sandbox. |
packages/coding-agent/src/features/workflow/vm-quickjs.ts |
Removes the duplicate executor. |
packages/coding-agent/src/features/workflow/step-workflow.ts |
Updates workflow executor wiring. |
packages/coding-agent/src/features/workflow/runtime.ts |
Uses QuickJS by default. |
packages/coding-agent/src/features/workflow/registration-gate.ts |
Removes runtime-dependent gating. |
packages/coding-agent/src/features/workflow/index.ts |
Updates workflow exports. |
packages/coding-agent/package.json |
Replaces the native dependency. |
packages/coding-agent/docs/development.md |
Documents QuickJS constraints. |
docs/orchestration-lifecycle.md |
Updates workflow runtime documentation. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+317
to
+321
| hostCalls.live.delete(deferred); | ||
| hostCalls.pending = Math.max(0, hostCalls.pending - 1); | ||
| if (hostCalls.closed) return; | ||
| hostCalls.refreshDeadline?.(); | ||
| hostCalls.onSettled?.(); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
workflow(ultracode)收敛为单一 QuickJS-WASM 执行器
背景
#178 已将 workflow 沙箱从 isolated-vm 换成 QuickJS-WASM，解决了正式版二进制上 workflow 与 ultracode 完全不可用的问题。但作为过渡形态，遗留三件事：
runtime.ts按宿主能否加载原生扩展在 isolated-vm 与 QuickJS 间选择，导致开发机与正式版跑两套引擎，语义差异只在发布版暴露——正是 fix: restore workflow and ultracode on the released binary #178 所修 bug 的成因类型。同时--ignore-scripts装不上原生扩展、CI node-gyp 编译、Node 版本 ABI 匹配等问题仍在。vm-quickjs.ts:104），而该豁免唯一生效场景恰是必须终止的场景；循环运行在 WebAssembly 调用内堵死宿主事件循环，外层兜底定时器也无法执行，实测进程挂死。JS_FreeRuntime处 abort 整个 WASM 实例，随后对已释放 context 结清抛出 use-after-free 杀进程；模块按进程缓存还会毒化后续所有运行。修改（16 文件，+710 / −1152）
收敛为单一执行器
vm-quickjs.ts合并进vm.ts并删除；vm.ts成为唯一沙箱，兼持契约类型与引擎无关纯函数runtime.ts：vmExecutor默认runInQuickJs，移除按宿主选择的defaultWorkflowVmExecutor；显式注入仍优先index.ts：导出面收敛为runInQuickJs+WORKFLOW_MAX_SCRIPT_BYTES移除 isolated-vm
package.jsonoptionalDependencies、pnpm-lock.yaml、bundle 脚本 externals 白名单门禁简化
registration-gate.ts：决策收敛为 enabled / not-enabled / disabled-by-env；移除 vm-unavailable、vm-unsupported-runtime 与WORKFLOW_VM_UNAVAILABLE_WARNINGstep-workflow.ts：删掉发出该告警的 session_start 分支；执行器类型改为typeof runInQuickJs缺陷 1 修复
timeoutMs的扇出会在过期 deadline 上恢复续体并被中断，parallel()会把该分支报成裸 null 而非错误，即误判超时变成静默错误结果。宿主侧定时器保留 pending 豁免。缺陷 2 修复
HostCallTracker增加未结清 deferred 集合与closed标志：teardown 时先置位 closed、释放全部残留 deferred，再释放 context 与 runtime；此后到达的结清直接返回，不再触碰 VM。测试
workflow-vm.test.ts重写为单执行器契约套件（26 例），吸收原 conformance 全部用例，新增覆盖上述两缺陷及「较窄中断器修法会打破」的扇出场景workflow-vm-conformance.test.ts（已并入）workflow-extension.test.ts、workflow-ultraloop-opt-in.test.ts去掉skipIf(!isIsolatedVmAvailable())门控workflow-registration.test.ts按新门禁重写文档
development.md、orchestration-lifecycle.md：更正原生扩展必需的错误描述，记录两条编辑约束（必须用 singlefile 变体；handle 与 context/runtime 释放顺序）行为变化与边界
timeoutMs会被终止（与原 isolated-vm 按墙钟一致）；交错短突发不受影响，每次宿主调用结清都会重新计时；等待宿主调用的时间不计入超时。Intl、structuredClone、Array.fromAsync；其余 ES 特性（含 ES2023toSorted/with、ES2024Object.groupBy）可用。pnpm install --ignore-scripts开箱即得可用 workflow，README 无需修改。验证
测试与门禁
tsgo --noEmit通过biome check --error-on-warnings .（全仓）通过--version、--list-models）