diff --git a/docs/rfc/implemented/feature/2026-07-05-dynamic-workflows.md b/docs/rfc/implemented/feature/2026-07-05-dynamic-workflows.md index 6f8678ecea..f44e1fad89 100644 --- a/docs/rfc/implemented/feature/2026-07-05-dynamic-workflows.md +++ b/docs/rfc/implemented/feature/2026-07-05-dynamic-workflows.md @@ -24,7 +24,7 @@ One deliberate strictness DIVERGENCE from CC: hook misuse — unknown or deferre ### The engine (dsh-workflow-vm): in-process node:vm -**Why node:vm and not isolated-vm/worker threads**: isolated-vm is in maintenance mode, needs `--no-node-snapshot` on EVERY consumer process (including the published bins) on Node ≥ 20, and falls back to node-gyp source builds; a worker-thread engine turns every hook into RPC and complicates the per-file coverage gate. Scripts are model-written — the same trust level as the model's existing bash access — so genuine sandboxing is not the current requirement. The interface/implementation split exists precisely so a hardened engine can swap in later. Accepted, documented limitations: vm is not a security boundary, and the vm timeout covers only the initial synchronous slice — a pathological synchronous spin after the first await cannot be killed in-process; `dispose()` cancels, waits a bounded grace for the script to settle and its children to finish disposing, then abandons. +**Why node:vm and not isolated-vm/worker threads**: isolated-vm is in maintenance mode, needs `--no-node-snapshot` on EVERY consumer process (including the published bins) on Node ≥ 20, and falls back to node-gyp source builds; a worker-thread engine turns every hook into RPC and complicates the per-file coverage gate. Scripts are model-written — the same trust level as the model's existing bash access — so genuine sandboxing is not the current requirement. The interface/implementation split exists precisely so a hardened engine can swap in later. Accepted, documented limitations: vm is not a security boundary, and the vm timeout covers only the initial synchronous slice — a pathological synchronous spin in realm code past that slice (an await continuation, or a thenable's `then` invoked by promise resolution — a returned thenable resolves per JavaScript semantics, which is what makes an un-awaited `return agent('x')` work) cannot be killed in-process; `dispose()` cancels, waits a bounded grace for the script to settle and its children to finish disposing, then abandons. **Meta extraction**: a string/comment-aware brace scanner (template interpolation rejected) finds the literal; it is evaluated ALONE in an empty, timed vm context; the result must materialize to plain JSON data and pass shape validation (unknown fields rejected loud); the statement is blanked line-preservingly so stacks keep script line numbers. diff --git a/packages/workflow/workflow-vm/README.md b/packages/workflow/workflow-vm/README.md index e519ce7c0e..05a8e252d3 100644 --- a/packages/workflow/workflow-vm/README.md +++ b/packages/workflow/workflow-vm/README.md @@ -16,7 +16,7 @@ Values ENTERING the host (the meta literal, hook options/schemas, the script's r Per-run: a concurrency semaphore (`maxConcurrentAgents`), a total-`agent()` cap (`maxTotalAgents`), and a per-call item cap (`maxItemsPerCall`), all config. `cancel()` aborts every child (a shared `AbortSignal`), rejects waiting `agent()` slots, and makes every future hook call throw `CANCELLED` — the script dies at its next await and the run settles `cancelled`; a cancellation that lands before the body runs (or before it settles) reports `cancelled` even if the script itself needed no hooks. Once a run settles, stray children a script fired without awaiting are aborted too, and `dispose()` waits for those children to finish disposing (bounded by the grace) before returning. Every hook-returned promise carries a no-op rejection consumer, so a dropped promise cannot surface an unhandled rejection (the app boot layer exits the process on those). Thrown script values are pre-rendered to a string INSIDE the realm's execution window (the body is compiled into a realm-side catch), so a hostile `stack` getter is subject to the vm sync-slice timeout like any other script code; the host catch only descriptor-reads that string, falling back to `describeThrown` (fixed labels, own-data reads, an identity-verified host-native stack getter) — `result` cannot reject. -**Documented limitations** (the accepted cost of the in-process mechanism; the seam exists so a worker-thread/isolated-vm engine can swap in): vm is NOT a security boundary — scripts are model-written, the same trust level as the model's bash access — and the vm `timeout` covers only the initial synchronous slice, so a pathological synchronous spin after the first await cannot be killed; `dispose()` waits `disposeGraceMs` then ABANDONS such a script (its settlement stays contained, but an abandoned spin would still occupy the event loop). +**Documented limitations** (the accepted cost of the in-process mechanism; the seam exists so a worker-thread/isolated-vm engine can swap in): vm is NOT a security boundary — scripts are model-written, the same trust level as the model's bash access — and the vm `timeout` covers only the initial synchronous slice, so a pathological synchronous spin in realm code past that slice (an await continuation, or a thenable's `then` invoked by promise resolution) cannot be killed; `dispose()` waits `disposeGraceMs` then ABANDONS such a script (its settlement stays contained, but an abandoned spin would still occupy the event loop). A returned promise or thenable resolves per JavaScript semantics BEFORE materialization — that is what makes an un-awaited `return agent('x')` work — and the realm-boundary guard applies to the resolution. ## Config diff --git a/packages/workflow/workflow-vm/src/index.ts b/packages/workflow/workflow-vm/src/index.ts index 4f6a506329..d0c8d8abfc 100644 --- a/packages/workflow/workflow-vm/src/index.ts +++ b/packages/workflow/workflow-vm/src/index.ts @@ -12,13 +12,17 @@ * level as the model's bash access — and the realm-boundary materialization * is correctness containment, not a sandbox. * - The vm `timeout` covers only the initial SYNCHRONOUS slice of the script; - * a pathological synchronous spin after the first await cannot be killed - * in-process. `dispose()` waits a bounded grace for the script to settle - * AND its children (stray `agent()` calls included) to finish disposing, - * then ABANDONS whatever is left: pending hook promises are already - * rejected and the script's settlement is contained (no unhandled - * rejection), but an abandoned synchronous spin would still occupy the - * event loop. + * realm code that runs past that slice — an await continuation, a + * thenable's `then` invoked by promise resolution (including one the script + * RETURNS: a returned thenable resolves per JavaScript semantics before + * materialization, which is what makes an un-awaited `return agent('x')` + * work) — is beyond the timeout, so a pathological synchronous spin there + * cannot be killed in-process. `dispose()` waits a bounded grace for the + * script to settle AND its children (stray `agent()` calls included) to + * finish disposing, then ABANDONS whatever is left: pending hook promises + * are already rejected and the script's settlement is contained (no + * unhandled rejection), but an abandoned synchronous spin would still + * occupy the event loop. * * Plugin export shape: a default-exported {@link WorkflowService} subclass * (the class-based service form, like `dsh-bash-local`). diff --git a/packages/workflow/workflow-vm/tests/workflow-vm.spec.ts b/packages/workflow/workflow-vm/tests/workflow-vm.spec.ts index 00a9438575..28c67dda14 100644 --- a/packages/workflow/workflow-vm/tests/workflow-vm.spec.ts +++ b/packages/workflow/workflow-vm/tests/workflow-vm.spec.ts @@ -217,6 +217,19 @@ describe('dsh-workflow-vm', () => { expect(result.stopReason).toBe('completed') expect(result.value).toBeNull() }) + + it('a returned promise/thenable resolves per async-JS semantics before materialization', async () => { + const { ctx, parent } = await setup() + // Load-bearing ergonomics: forgetting await on the final hook call works. + expect((await run(ctx, parent, script("return agent('x')"))).value).toBe('stub reply') + // A hand-built thenable is assimilated by the async return — the + // RESOLUTION is the script's return value (standard JavaScript), and the + // realm-boundary guard applies to that resolution, not the thenable. + expect((await run(ctx, parent, script('return { value: 1, then(resolve) { resolve({ ok: true }) } }'))).value).toEqual({ ok: true }) + const nonJson = await run(ctx, parent, script('return { then(resolve) { resolve({ bad: new Date(0) }) } }')) + expect(nonJson.stopReason).toBe('error') + expect(nonJson.error).toContain('not plain JSON data') + }) }) describe('combinator semantics', () => {