diff --git a/docs/core-data-structures/workflow.md b/docs/core-data-structures/workflow.md index 11f216f39e..8c7916a6c3 100644 --- a/docs/core-data-structures/workflow.md +++ b/docs/core-data-structures/workflow.md @@ -47,7 +47,7 @@ interface WorkflowResult { ## A live run: `WorkflowRun` -The handle the consumer holds while a script executes. The consumer awaits `result`, may `cancel` mid-flight, and MUST `dispose` on every path. `result` does NOT reject — a script failure resolves with `stopReason: 'error'` — so the consumer maps a non-`completed` reason to an `isError` result. `dispose()` cancels, waits a bounded grace for the script to settle AND its children to finish disposing, then abandons whatever is left (the engine documents the abandonment semantics); it never hangs on a stuck script. +The handle the consumer holds while a script executes. The consumer awaits `result`, may `cancel` mid-flight, and MUST `dispose` on every path. `result` does NOT reject — a script failure resolves with `stopReason: 'error'` — and once the run is cancelled it SETTLES within the engine's bounded grace even if the script itself never settles (the engine abandons the script and reports `cancelled`), so a consumer awaiting `result` is never wedged past a cancellation. `dispose()` = cancel + that bounded settle + child quiescence (the engine documents what abandonment leaves behind); it never hangs on a stuck script. ```ts type-equiv interface WorkflowRun { 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 4d7061335e..f39f12f0ed 100644 --- a/docs/rfc/implemented/feature/2026-07-05-dynamic-workflows.md +++ b/docs/rfc/implemented/feature/2026-07-05-dynamic-workflows.md @@ -24,13 +24,13 @@ 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 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. +**Trust premise (governs every engine decision below)**: workflow scripts are MODEL-WRITTEN — the same trust level as the model's existing bash access — so the engine defends against BUGGY scripts, never hostile ones. In scope: `result` never rejects, no unhandled rejections from dropped hook promises, loud rejection of values JSON cannot carry, fatal-vs-null hook discipline, cancellation that always frees the caller. Out of scope, deliberately: adversarial values (throwing/spinning accessors, proxies with hostile traps, prototype forgery, `prepareStackTrace` hijack) — host code MAY run script code while reading script values, and that is accepted, because a hostile script can already occupy the event loop forever with a synchronous spin past its first await; containing its error VALUES while conceding it the event loop would be cost without a threat model. Genuine hardening is an engine swap behind the seam (worker/isolated-vm gets value isolation by serialization for free), not incremental host-side defenses. + +**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. Under the trust premise, in-process is enough. Accepted, documented limitations: `start()` blocks the caller for the script's initial synchronous slice (bounded by the vm timeout); that timeout covers ONLY the initial slice, so a synchronous spin past it (an await continuation, 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 — or script code the host runs while rendering a thrown value) 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. -**Realm boundary**: values entering the host (meta, hook options, schemas, the return value) go through `materializeFromRealm` — a descriptor walk that NEVER invokes accessors (the repo's `isJsonValue` is prototype-strict and getter-invoking, so it cannot run first; it would reject every cross-realm object and let realm code run outside the timed window) and rejects loud everything JSON cannot carry, proxies included (the trap-free `util.types.isProxy`, checked before any inspection, so realm-side traps never run on the host stack), copying via `Object.defineProperty` so a `"__proto__"` key becomes a data property, never a prototype mutation. Values entering the realm are realm-built throughout, so the script never holds a live host-prototype object: `args` and `agent()` results via the context's own `JSON.parse`, combinator result arrays via its `Array.from`, hook promises via its `Promise.resolve`, and hook failures as realm-built clones (name/code/message/fatal — the combinators recognize fatal clones structurally). Realm functions (stages, thunks) are called, never materialized. - -**Containment**: every hook-returned promise carries a no-op rejection consumer, so a script that drops a promise cannot surface an unhandled rejection when cancellation rejects it — `dsh-app-boot` exits the process on unhandled rejections. Thrown script/meta values are pre-rendered to a string by a realm-side catch compiled into the wrapper (rendering runs inside the realm's own execution window, so a hostile `stack` getter dies by the vm sync-slice timeout like any other script code — host-side formatting of realm errors is unfixable in general, since V8 stack formatting invokes script-controllable `name`/`prepareStackTrace` hooks); the host catch descriptor-reads that string or falls back to `describeThrown` (fixed labels, own-data reads, an identity-verified host-native stack getter), so `result` cannot reject. Caps (`maxConcurrentAgents` auto = `min(16, cores - 2)`, `maxTotalAgents` 1000, `maxItemsPerCall` 4096) and timeouts are validated Config, not literals. +**Value boundary**: values entering the host (meta, hook options, schemas, the return value) go through `materializeFromRealm` — a plain recursive walk that rejects loud everything JSON cannot carry (exotic prototypes, functions, symbols, cycles, sparse arrays, non-finite numbers, nested `undefined`), copying via `Object.defineProperty` so a `"__proto__"` key becomes a data property, never a prototype mutation; getters are read ordinarily and their RESULT crosses (a throwing read fails loud). Values entering the realm (`args`, `agent()` results, hook promises and failures, combinator arrays) are handed over directly as host values — the script is trusted, so host prototypes are not a leak; `args` is host-`structuredClone`d once so a script cannot mutate the caller's object. Hook failures are host `WorkflowError`s: the combinators recognize fatality by host `instanceof` (unforgeable from the realm), and the script-visible consequence — in-script `instanceof Error` is `false` for hook errors; branch on `e.name`/`e.code` — is documented in the engine README. Realm functions (stages, thunks) are called, never materialized. Thrown script values are rendered by a total host-side renderer (stack → message → `String()`, fixed label if rendering throws), so `result` cannot reject. Caps (`maxConcurrentAgents` auto = `min(16, cores - 2)`, `maxTotalAgents` 1000, `maxItemsPerCall` 4096) and timeouts are validated Config, not literals. ### The consumer (dsh-tool-workflow) @@ -42,6 +42,7 @@ A `workflow` tool mirroring `dsh-tool-subagent`'s synchronous shape: start, awai ## What was rejected +- **Hostile-value containment in the host** (trap-free proxy rejection, accessor-never-invoked descriptor walks, realm-side pre-rendering of thrown values, realm-built promises/arrays/error clones with structural fatal recognition): an earlier revision built all of it, and review showed the cost was real while the threat model was not — every one of those defenses guards against an author the premise already trusts, who retains an accepted unkillable event-loop spin regardless. Removed in favor of the plain boundary above; the hardened engine deletes such machinery anyway (serialization by construction). - **Background execution as the default** (CC's shape): deferred; foreground-synchronous matches `dsh-tool-subagent`'s cut, and background semantics should be designed ONCE across bash/subagent/workflow rather than per-tool. - **Workflow-layer JSON parsing for `agent({schema})`**: duplicating a seam concern at one consumer while the seam's capability flag stayed dishonestly `false`. - **Meta as tool parameters instead of `export const meta`**: zero parsing, but scripts stop being self-contained artifacts and CC-authored scripts stop being drop-in. diff --git a/packages/workflow/tool-workflow/package.json b/packages/workflow/tool-workflow/package.json index 9c3f819cb5..416b6a814f 100644 --- a/packages/workflow/tool-workflow/package.json +++ b/packages/workflow/tool-workflow/package.json @@ -35,9 +35,11 @@ "@deepseek-ai/dsh-agent": "workspace:^", "@deepseek-ai/dsh-llm": "workspace:^", "@deepseek-ai/dsh-session": "workspace:^", + "@deepseek-ai/dsh-subagent": "workspace:^", "@deepseek-ai/dsh-system-prompt": "workspace:^", "@deepseek-ai/dsh-tools": "workspace:^", "@deepseek-ai/dsh-workflow": "workspace:^", + "@deepseek-ai/dsh-workflow-vm": "workspace:^", "cordis": "^4.0.0-rc.6" } } diff --git a/packages/workflow/tool-workflow/tests/tool-workflow.spec.ts b/packages/workflow/tool-workflow/tests/tool-workflow.spec.ts index 012fae4961..677cbbf7a9 100644 --- a/packages/workflow/tool-workflow/tests/tool-workflow.spec.ts +++ b/packages/workflow/tool-workflow/tests/tool-workflow.spec.ts @@ -9,6 +9,8 @@ import type { Agent } from '@deepseek-ai/dsh-agent' import { WorkflowRunId, WorkflowService } from '@deepseek-ai/dsh-workflow' import type { WorkflowResult, WorkflowRun, WorkflowStartRequest } from '@deepseek-ai/dsh-workflow' import { CallId } from '@deepseek-ai/dsh-llm' +import SubagentService from '@deepseek-ai/dsh-subagent' +import VmWorkflowEngine from '@deepseek-ai/dsh-workflow-vm' import * as toolWorkflow from '../src/index.ts' /** A controllable engine standing in behind ctx.workflows (the tool's only seam). */ @@ -221,4 +223,33 @@ describe('dsh-tool-workflow', () => { expect(unwrapped).toBe(toolWorkflow) expect(typeof unwrapped.apply).toBe('function') }) + + describe('composition with the REAL vm engine (the mock above must stay honest)', () => { + it('an abort releases the tool even when the script parks on a promise no hook owns', async () => { + // Regression for the review-found turn wedge: the tool awaits + // run.result BEFORE its disposing finally, the registry and the loop + // await the tool — so if cancellation could not settle result (a script + // parked on `await new Promise(() => {})`), an aborted turn stayed + // wedged forever. The seam now guarantees result settles within the + // grace of cancel(); this drives that guarantee through the real + // registry + real tool + real engine. + const ctx = new Context() + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(SubagentService) + await ctx.plugin(VmWorkflowEngine, { disposeGraceMs: 30 }) + await ctx.plugin(toolWorkflow, {}) + const parent = { id: AgentId('caller'), options: {} } as unknown as Agent + const controller = new AbortController() + const pending = execute(ctx, { + script: "export const meta = { name: 'stuck', description: 'parks forever' }\nawait new Promise(() => {})\nreturn 1", + }, { agent: parent, signal: controller.signal }) + // Give the run a beat to start (past its synchronous slice), then abort. + await new Promise(resolve => setTimeout(resolve, 20)) + controller.abort('user abort') + const result = await pending + expect(result.isError).toBe(true) + expect((result.content[0] as { text: string }).text).toContain('cancelled') + }) + }) }) diff --git a/packages/workflow/workflow-vm/README.md b/packages/workflow/workflow-vm/README.md index b5d8817735..0ec1116cb7 100644 --- a/packages/workflow/workflow-vm/README.md +++ b/packages/workflow/workflow-vm/README.md @@ -2,21 +2,25 @@ The first [`WorkflowService`](../workflow/README.md) implementation: an in-process **`node:vm` engine**. It parses the Claude Code-format script (`export const meta = {...}` + plain-JS body), runs the body in a fresh vm context with the workflow hooks injected, and fans `agent()` calls out to [`ctx.subagents`](../../subagent/README.md). +## Trust premise + +Workflow scripts are **model-written** — the same trust level as the model's existing bash access — so this engine defends against **buggy** scripts, never hostile ones. vm is NOT a security boundary and no attempt is made to contain adversarial values: property reads on script values may run script code (a getter, a `toString`, a proxy trap) on the host stack, and a script determined to hang the process can simply spin past its first await (see the limitations below). What the engine DOES guarantee, because benign scripts hit these constantly: `result` never rejects, a dropped hook promise never becomes an unhandled rejection (the app boot layer exits the process on those), values that JSON cannot carry are rejected **loud** instead of silently mangled, and hook misuse is fatal instead of dissolving into a per-item `null`. Genuine sandboxing is an engine swap behind the seam (worker-thread/isolated-vm, where the boundary is serialization by construction), not incremental host-side defenses here. + ## The script contract it executes - **Meta extraction** (`extractMeta`): a string/comment-aware brace scanner finds the leading `export const meta` literal (template interpolation rejected — the literal must be pure), evaluates it ALONE in an empty timed vm context, materializes the result to plain JSON data, validates the shape (`name`/`description` required; unknown fields rejected loud), and blanks the statement line-preservingly so error stacks keep the script's own line numbers. - **Hooks**: `agent(prompt, {label, phase, schema, model})` (schema = the [structured-output subset](../../core/tools/README.md), forwarded as `outputSchema`; result = validated object, or final text without a schema; a failed child resolves `null`), `parallel(thunks)`, `pipeline(items, ...stages)` with NO cross-stage barrier and `(prev, item, index)` stage callbacks, `phase(title)`, `log(message)`, and the `args` global. Anything else — `effort`/`isolation`/`agentType`, unknown options, malformed arguments, schemas outside the subset — throws a FATAL `WorkflowError` that `parallel`/`pipeline` re-throw rather than nulling (see the seam README's failure discipline). - **Determinism bans**: `Date.now()`, `Math.random()`, and argless `new Date()` throw (kept even though resume is deferred, so scripts stay resume-compatible); no timers, filesystem, or Node APIs exist in the context. -## Realm discipline +## The value boundary -Values ENTERING the host (the meta literal, hook options/schemas, the script's return) are materialized by `materializeFromRealm`: a descriptor walk that never invokes accessors and rejects loud everything JSON cannot carry (accessors, exotic prototypes, functions, symbols, cycles, sparse arrays, non-finite numbers, nested `undefined`, and proxies — rejected via the trap-free `util.types.isProxy` BEFORE any inspection could run a realm-side trap on the host stack), copying into host containers via `defineProperty` so a `"__proto__"` key becomes a data property, never a prototype mutation. Values ENTERING the realm are realm-built throughout, so the script never holds an object whose prototype chain reaches host intrinsics: `args` and `agent()` results are rebuilt through the context's own `JSON.parse`, combinator result arrays through its `Array.from`, hook promises through its `Promise.resolve`, and a hook failure (rejection or synchronous `phase`/`log` throw) crosses as a realm-built clone carrying name/code/message/fatal — the combinators recognize fatal clones structurally, so the fatal-vs-null discipline survives the boundary. +Values ENTERING the host (the meta literal, hook options/schemas, the script's return) are materialized by `materializeFromRealm`: a plain recursive walk that rejects loud everything JSON cannot carry (exotic prototypes, functions, symbols, cycles, sparse arrays, non-finite numbers, nested `undefined`), copying into host containers via `defineProperty` so a `"__proto__"` key becomes a data property, never a prototype mutation. Getters are read ordinarily — the RESULT is what crosses; a read that throws fails loud. Values ENTERING the realm (`args`, `agent()` results, hook promises and their failures, combinator arrays) are handed over directly as host values — the script is trusted, so host prototypes are not a leak; `args` is `structuredClone`d once at start so a script scribbling on it cannot mutate the caller's object. One script-visible consequence: an error thrown by a hook is a HOST error, so `e instanceof Error` inside the script is `false` — branch on `e.name`/`e.code` instead (the combinators recognize fatality by host `instanceof`, which a script-built object can never pass, so fatal-vs-null cannot be forged or dissolved). ## Limits, cancellation, disposal -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. +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, and a script that STILL has not settled `disposeGraceMs` after the cancel (parked on a promise no hook owns, like `await new Promise(() => {})`) is ABANDONED with `result` force-settling `cancelled` — a consumer awaiting `result` is never wedged past a cancellation. 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; thrown script values are rendered by a total host-side renderer (stack, then message, then `String()`, with a fixed label if rendering itself throws) — `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 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. +**Documented limitations** (the accepted cost of the in-process mechanism; the seam exists so a worker-thread/isolated-vm engine can swap in): `start()` runs the script's initial synchronous slice inline, so the caller blocks until the first await or the vm `timeout`; that `timeout` covers ONLY the initial slice, so a synchronous spin past it (an await continuation, a thenable's `then` invoked by promise resolution, or script code the host runs while rendering a thrown value) 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 value-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 d0c8d8abfc..9b8c646aa9 100644 --- a/packages/workflow/workflow-vm/src/index.ts +++ b/packages/workflow/workflow-vm/src/index.ts @@ -4,25 +4,33 @@ * body in a fresh in-process vm context with the workflow hooks injected, and * fans `agent()` calls out to `ctx.subagents`. * - * Engine limitations, documented as the accepted cost of the in-process - * mechanism (the interface/implementation seam exists precisely so a - * worker-thread or isolated-vm engine can swap in if these ever matter): + * TRUST PREMISE: scripts are MODEL-WRITTEN — the same trust level as the + * model's existing bash access — so this engine defends against BUGGY + * scripts, never hostile ones. vm is NOT a security boundary and no attempt + * is made to contain adversarial values (see ./realm.ts); genuine sandboxing + * is an engine swap behind the seam (worker-thread/isolated-vm), not + * incremental host-side defenses here. * - * - vm is NOT a security boundary. Scripts are model-written — the same trust - * 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; - * 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. + * Engine limitations, documented as the accepted cost of the in-process + * mechanism: + * + * - `start()` runs the script's initial SYNCHRONOUS slice inline, so the + * CALLER blocks on the host event loop until the script's first await (or + * the vm `timeout` kills the slice); the meta-literal evaluation has its + * own timeout budget on the same call. + * - The vm `timeout` covers only that initial slice; realm code running past + * it — 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 synchronous spin there cannot be killed in-process, and neither can + * script code the host invokes while rendering a failure (a getter on a + * thrown value). `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`). @@ -55,7 +63,11 @@ export interface Config { maxItemsPerCall?: number /** vm timeout for the script's initial synchronous slice AND the meta-literal evaluation (default 5000 ms). */ syncTimeoutMs?: number - /** How long `dispose()` waits for a cancelled script to settle before abandoning it (default 5000 ms). */ + /** + * How long after a cancellation an unsettled script may keep running before + * it is abandoned and `result` force-settles `cancelled` (default 5000 ms); + * also bounds `dispose()`. + */ disposeGraceMs?: number } @@ -110,6 +122,7 @@ export class VmWorkflowEngine extends WorkflowService { maxTotalAgents: this.config.maxTotalAgents, maxItemsPerCall: this.config.maxItemsPerCall, syncTimeoutMs: this.config.syncTimeoutMs, + disposeGraceMs: this.config.disposeGraceMs, } const execution = new WorkflowExecution( this.ctx, @@ -149,9 +162,11 @@ export class VmWorkflowEngine extends WorkflowService { }, dispose: (): Promise => { // Idempotent: cancel, then wait min(settle + child quiescence, grace). - // `result` and `quiesce()` never reject, so the race needs no - // rejection handling; a script or child still unsettled past the grace - // is abandoned per the module contract. + // The cancel itself bounds `result` (the execution abandons a script + // still unsettled `disposeGraceMs` later), so this outer race exists + // for CHILD quiescence: a slow-disposing child must not hold dispose + // past the grace. `result` and `quiesce()` never reject, so the race + // needs no rejection handling. disposed ??= (async () => { execution.cancel('workflow disposed') await Promise.race([ diff --git a/packages/workflow/workflow-vm/src/meta.ts b/packages/workflow/workflow-vm/src/meta.ts index bcb891e85e..cfe3e4d381 100644 --- a/packages/workflow/workflow-vm/src/meta.ts +++ b/packages/workflow/workflow-vm/src/meta.ts @@ -20,7 +20,7 @@ import * as vm from 'node:vm' import { WorkflowError } from '@deepseek-ai/dsh-workflow' import type { WorkflowMeta, WorkflowPhase } from '@deepseek-ai/dsh-workflow' -import { materializeFromRealm, MaterializeError, describeThrown, thrownRendering, REALM_THROWN_RENDERER_SOURCE } from './realm.ts' +import { materializeFromRealm, MaterializeError, renderThrown } from './realm.ts' /** The result of {@link extractMeta}: the validated meta and the runnable body. */ export interface ExtractedScript { @@ -171,18 +171,11 @@ export function extractMeta(script: string, evalTimeoutMs: number): ExtractedScr // An EMPTY context: any non-literal reference (a variable, a call) throws // here. The result — data only — is what the contract checks; a getter or // IIFE can still run, which is why the timeout and the materialization - // below are part of the same boundary. A thrown value is pre-rendered by - // the realm-side catch INSIDE the timed window, so a hostile - // stack/message/toString can neither run on the host catch path nor - // outlive the timeout. - evaluated = vm.runInNewContext( - `(() => { try { return (${literal}) } catch (e) { throw (${REALM_THROWN_RENDERER_SOURCE})(e) } })()`, - undefined, - { timeout: evalTimeoutMs }, - ) + // below are part of the same boundary. + evaluated = vm.runInNewContext(`(${literal})`, undefined, { timeout: evalTimeoutMs }) } catch (error: unknown) { throw new WorkflowError( - `meta block failed to evaluate as a pure literal: ${thrownRendering(error) ?? describeThrown(error)}`, + `meta block failed to evaluate as a pure literal: ${renderThrown(error)}`, 'META_INVALID', { cause: error }, ) diff --git a/packages/workflow/workflow-vm/src/realm.ts b/packages/workflow/workflow-vm/src/realm.ts index c1f1ff7c5c..f5a2665626 100644 --- a/packages/workflow/workflow-vm/src/realm.ts +++ b/packages/workflow/workflow-vm/src/realm.ts @@ -1,48 +1,34 @@ /** - * Realm-boundary materialization for the vm engine. + * The vm engine's value boundary: copy script-realm values into plain host + * JSON data — loud about everything JSON cannot carry — and render thrown + * script values to failure text. * - * Values produced INSIDE the script realm (the meta literal, hook arguments, - * the script's return value) must become plain host-realm JSON data before the - * host touches them. The repo's `isJsonValue` guard cannot run first: it is - * prototype-strict (any cross-realm object fails it) and it INVOKES getters - * (letting realm code run outside the vm's timed window). So this module walks - * own-property DESCRIPTORS — never invoking accessors — and copies data into - * host containers, rejecting loud everything JSON cannot carry: - * accessor properties, non-plain prototypes, functions, symbols (keys or - * values), bigints, non-finite numbers, `undefined` values, cycles, sparse - * arrays, arrays with non-index own properties, and proxies. Proxies are - * rejected via the trap-free native `util.types.isProxy` check BEFORE any - * other inspection — a descriptor walk over a proxy would otherwise run its - * realm-side traps (`ownKeys`, `getOwnPropertyDescriptor`, `getPrototypeOf`) - * on the host stack, outside the vm's timed window, and a throwing trap would - * escape as a raw realm error instead of a {@link MaterializeError}. The same - * check guards the PROTOTYPE position (an object whose prototype is a proxy). + * TRUST PREMISE (everything in this module hangs on it): workflow scripts are + * MODEL-WRITTEN, the same trust level as the model's existing bash access, so + * this boundary guards against BUGGY scripts, not hostile ones. It rejects + * loud what JSON would silently mangle — functions, symbols, bigints, + * non-finite numbers, nested `undefined`, cycles, sparse arrays, exotic + * prototypes — because accepted-then-ignored is this repo's banned failure + * mode. It does NOT defend against adversarial values: the walk reads + * properties ordinarily (a getter runs, and whatever it returns is what + * crosses), {@link renderThrown} reads `stack`/`message`/`String()` directly, + * and a proxy is walked through its traps. A hostile script gains nothing + * worth defending here — it can already occupy the event loop forever with a + * synchronous spin past the first await (the engine's documented, accepted + * limitation) — so host-side hostile-value containment would be cost without + * a threat model; genuine hardening is an ENGINE SWAP (worker/isolated-vm, + * where the boundary is serialization by construction), not incremental + * defenses here. * - * Host objects are built with `Object.defineProperty` into a fresh `{}` — - * never plain `target[key] =` assignment, which a `"__proto__"` key would turn - * into prototype mutation instead of a data property. - * - * The host→realm direction deliberately does NOT live here: a host object - * handed into the realm would expose host intrinsics through its prototype - * chain, so the engine rebuilds inbound values INSIDE the realm via the - * context's own `JSON.parse` (see the runtime). - * - * {@link REALM_THROWN_RENDERER_SOURCE}, {@link thrownRendering}, and - * {@link describeThrown} are the same discipline for the one place realm - * values reach the host WITHOUT materialization: a thrown value crossing into - * a host catch block. The renderer runs INSIDE the realm's own execution - * window (compiled into the script wrapper), so reading a hostile - * accessor/`toString` there is subject to the vm sync-slice timeout exactly - * like any other script code; the host side only descriptor-reads the - * pre-rendered string, or falls back to {@link describeThrown}, which invokes - * no getter whose function identity is not the host realm's own native stack - * getter. + * The host→realm direction needs no machinery at all: hooks hand the script + * plain host values, host prototypes included — the script is trusted. One + * consequence is documented in the engine README: an error thrown by a hook + * is a HOST error, so an in-script `instanceof Error` check is false; read + * `name`/`code`/`message` instead. * * @module @deepseek-ai/dsh-workflow-vm/realm */ -import { types } from 'node:util' - /** Thrown by {@link materializeFromRealm}; the caller wraps it into the right `WorkflowError` code. */ export class MaterializeError extends Error { constructor(public readonly path: string, public readonly reason: string) { @@ -52,154 +38,63 @@ export class MaterializeError extends Error { } /** - * Realm-SOURCE text (an arrow-function expression) the engine compiles into - * its script wrappers: `throw (RENDERER)(e)` inside a catch around the whole - * body/literal. It renders the thrown value to a string INSIDE the realm's - * own execution window — a hostile `stack`/`message` accessor or `toString` - * invoked here is subject to the vm sync-slice timeout like any other script - * code (and post-await it is the engine's accepted spin limitation, identical - * to a script reading `e.stack` in its own catch). Host `WorkflowError`s - * thrown by hooks pass through unwrapped (duck-checked by name — a realm - * forgery fails the host's `instanceof` and merely renders data-only); - * everything else becomes `{ __wfThrown: }`, whose only consumer is - * {@link thrownRendering}. Every read is individually contained, so the - * renderer itself never throws. - */ -export const REALM_THROWN_RENDERER_SOURCE = `(e) => { - try { if (e && e.name === 'WorkflowError') return e } catch { /* hostile name getter: fall through to rendering */ } - const rendered = (() => { - try { if (e && typeof e.stack === 'string' && e.stack.length > 0) return e.stack } catch { /* hostile stack getter */ } - try { if (e && typeof e.message === 'string') return e.message } catch { /* hostile message getter */ } - try { return String(e) } catch { /* hostile toString/Symbol.toPrimitive */ } - return '[unrenderable thrown value]' - })() - return { __wfThrown: rendered } -}` - -/** - * The pre-rendered failure text carried by a realm-catch wrapper object - * (`{ __wfThrown: string }` from {@link REALM_THROWN_RENDERER_SOURCE}), or - * `undefined` when `error` is not such a wrapper. Descriptor-read and - * proxy-guarded: never invokes user code. - * @param error - the value a host catch received from script execution. - * @returns the realm-rendered string, or `undefined` to fall back to - * {@link describeThrown}. - */ -export function thrownRendering(error: unknown): string | undefined { - if (typeof error !== 'object' || error === null || types.isProxy(error)) return undefined - const value = ownDataProperty(error, '__wfThrown') - return typeof value === 'string' ? value : undefined -} - -/** - * The host realm's own native `stack` getter (modern V8 makes `stack` an own - * ACCESSOR on Errors); `undefined` where it is a data property. Typed through - * a structural view of the descriptor — it is only ever identity-compared or - * `.call`ed on an explicit receiver, never invoked unbound. - */ -const HOST_STACK_GETTER: unknown = (Object.getOwnPropertyDescriptor(new Error(), 'stack') as { get?: unknown } | undefined)?.get - -/** - * Render a thrown value HOST-SIDE without ever throwing and without running - * any code the host does not own: proxies become a fixed label (trap-free - * `isProxy` before any inspection); `stack` is read as an own data descriptor, - * or through its getter ONLY when that getter's function identity is the host - * realm's own native stack getter (an unforgeable check — realm code cannot - * hold that identity, and the host realm's `prepareStackTrace` is the host's - * own trust domain); `message` is an own-data read; anything else - * object-shaped renders as `[object Object]` untouched; only primitives - * (which cannot carry code) reach `String()`. Used for host-thrown errors - * (vm timeouts, `WorkflowError`s) and as the fallback for adversarial values - * that bypassed the realm-side renderer (e.g. a hostile thenable rejection); - * ordinary script failures arrive pre-rendered via {@link thrownRendering}. + * Render a thrown value to failure text without ever throwing: prefer the + * `stack` (host or realm — a realm error's `stack` is a plain string read), + * fall back to `message`, then `String()`. Reading those properties MAY run + * script code (a getter, `toString`) — accepted under the module's trust + * premise; if that code itself throws, a fixed label is returned instead. * @param error - the thrown value, of any shape and any realm. * @returns human-readable text for the failure report; prefers the stack. */ -export function describeThrown(error: unknown): string { - switch (typeof error) { - case 'object': - break - case 'function': - return '[thrown function]' - default: - // Primitives (string/number/boolean/bigint/symbol/undefined): String() - // cannot reach user code on these. - return String(error) +export function renderThrown(error: unknown): string { + try { + const stack = (error as { stack?: unknown } | null | undefined)?.stack + if (typeof stack === 'string' && stack.length > 0) return stack + const message = (error as { message?: unknown } | null | undefined)?.message + if (typeof message === 'string' && message.length > 0) return message + return String(error) + } catch { + // A throwing accessor/toString on the thrown value — rendering must be + // total (drive()'s never-reject contract), so fall back to a fixed label. + return '[unrenderable thrown value]' } - if (error === null) return 'null' - if (types.isProxy(error)) return '[thrown proxy]' - const stack = readStack(error) - if (typeof stack === 'string' && stack.length > 0) return stack - const message = ownDataProperty(error, 'message') - if (typeof message === 'string') return message - return '[object Object]' -} - -/** - * Read `error.stack` without running foreign code: an own DATA descriptor is - * read directly; an accessor is invoked only on function identity with - * {@link HOST_STACK_GETTER} (never a realm or user function). The native - * getter returns `undefined` on a non-Error receiver rather than throwing. - */ -function readStack(error: object): unknown { - const descriptor = Object.getOwnPropertyDescriptor(error, 'stack') - if (descriptor === undefined) return undefined - if ('value' in descriptor) return descriptor.value - if (typeof descriptor.get !== 'function') return undefined - if (descriptor.get !== HOST_STACK_GETTER) return undefined - return descriptor.get.call(error) -} - -/** An own DATA property's value (`undefined` for absent or accessor); never invokes user code on a non-proxy object. */ -function ownDataProperty(value: object, key: string): unknown { - const descriptor = Object.getOwnPropertyDescriptor(value, key) - return descriptor !== undefined && 'value' in descriptor ? descriptor.value : undefined -} - -/** - * Whether `error` is a FATAL realm-built `WorkflowError` clone — the shape the - * engine's hooks reject with (host errors are translated at the realm boundary - * so the script never holds host prototypes), duck-checked because a realm - * object cannot be an `instanceof` the host class. Proxy-guarded and - * descriptor-read, so a forged object cannot run code here; a script forging - * the shape only kills its own run (self-sabotage). Combinators use this to - * decide re-throw vs per-item `null`. - * @param error - the value a combinator caught from a realm thunk/stage. - * @returns `true` when the error must propagate and kill the script. - */ -export function isFatalWorkflowErrorClone(error: unknown): boolean { - if (typeof error !== 'object' || error === null || types.isProxy(error)) return false - return ownDataProperty(error, 'name') === 'WorkflowError' && ownDataProperty(error, 'fatal') === true } /** * Whether an object's prototype chain is data-shaped: `null`, or a prototype * whose own prototype is `null` (the realm's `Object.prototype` — which we * cannot compare by identity across realms). A `Date`/`Map`/class instance - * has a longer chain and is rejected, as is a proxy sitting in the prototype - * position (checked trap-free BEFORE its own prototype is dereferenced). + * has a longer chain and is rejected. */ function hasPlainPrototype(value: object): boolean { const proto: unknown = Object.getPrototypeOf(value) if (proto === null) return true - if (types.isProxy(proto)) return false return Object.getPrototypeOf(proto) === null } /** * Copy `value` (typically from the vm realm) into plain host JSON data. * Throws {@link MaterializeError} naming the offending path for anything JSON - * cannot carry losslessly. Accessors are detected via descriptors and NEVER - * invoked. `undefined` is accepted only at the ROOT (a script with no - * `return` value) — the caller decides what it means; an `undefined` nested - * INSIDE a container is a violation. + * cannot carry losslessly. Properties are read ordinarily — a getter runs and + * its RESULT is materialized; a read that throws surfaces as a + * {@link MaterializeError} carrying the rendered failure. `undefined` is + * accepted only at the ROOT (a script with no `return` value) — the caller + * decides what it means; an `undefined` nested INSIDE a container is a + * violation. * @param value - the realm value to materialize. * @param root - the path label for the root value (error messages). * @returns the host-realm copy (plain objects/arrays/scalars only). */ export function materializeFromRealm(value: unknown, root = 'value'): unknown { if (value === undefined) return undefined - return materialize(value, root, new Set()) + try { + return materialize(value, root, new Set()) + } catch (error: unknown) { + if (error instanceof MaterializeError) throw error + // A property read ran script code that threw; total-ize it so callers can + // keep the narrow MaterializeError contract. + throw new MaterializeError(root, `reading the value threw: ${renderThrown(error)}`) + } } function materialize(value: unknown, path: string, seen: Set): unknown { @@ -214,20 +109,15 @@ function materialize(value: unknown, path: string, seen: Set): unknown { case 'bigint': throw new MaterializeError(path, 'bigints are not JSON data') case 'function': - throw new MaterializeError(path, 'functions cannot cross the workflow realm boundary') + throw new MaterializeError(path, 'functions cannot cross the workflow value boundary') case 'symbol': - throw new MaterializeError(path, 'symbols cannot cross the workflow realm boundary') + throw new MaterializeError(path, 'symbols cannot cross the workflow value boundary') case 'undefined': throw new MaterializeError(path, 'undefined is not JSON data') case 'object': break } if (value === null) return null - // BEFORE anything else touches the object: every inspection below — - // Array.isArray aside — can trigger a proxy trap, running realm code on the - // host stack (module doc). isProxy is a native internal-slot check (no - // traps, catches revoked proxies, realm-agnostic). - if (types.isProxy(value)) throw new MaterializeError(path, 'proxies cannot cross the workflow realm boundary') const objectValue: object = value if (seen.has(objectValue)) throw new MaterializeError(path, 'circular references are not JSON data') seen.add(objectValue) @@ -242,10 +132,8 @@ function materialize(value: unknown, path: string, seen: Set): unknown { function materializeArray(value: unknown[], path: string, seen: Set): unknown[] { const out: unknown[] = [] for (let index = 0; index < value.length; index++) { - const descriptor = Object.getOwnPropertyDescriptor(value, index) - if (descriptor === undefined) throw new MaterializeError(`${path}[${index}]`, 'sparse arrays are not JSON data') - if (!('value' in descriptor)) throw new MaterializeError(`${path}[${index}]`, 'accessor properties cannot cross the workflow realm boundary') - out.push(materialize(descriptor.value, `${path}[${index}]`, seen)) + if (!(index in value)) throw new MaterializeError(`${path}[${index}]`, 'sparse arrays are not JSON data') + out.push(materialize(value[index], `${path}[${index}]`, seen)) } // Own enumerable props beyond the indices (e.g. `arr.total = 3`) would be // silently dropped by JSON — reject them instead. @@ -256,7 +144,7 @@ function materializeArray(value: unknown[], path: string, seen: Set): un } } if (Object.getOwnPropertySymbols(value).length > 0) { - throw new MaterializeError(path, 'symbol-keyed properties cannot cross the workflow realm boundary') + throw new MaterializeError(path, 'symbol-keyed properties cannot cross the workflow value boundary') } return out } @@ -266,20 +154,16 @@ function materializeObject(value: object, path: string, seen: Set): Reco throw new MaterializeError(path, 'only plain objects and arrays are JSON data (exotic prototype)') } if (Object.getOwnPropertySymbols(value).length > 0) { - throw new MaterializeError(path, 'symbol-keyed properties cannot cross the workflow realm boundary') + throw new MaterializeError(path, 'symbol-keyed properties cannot cross the workflow value boundary') } const out: Record = {} - for (const [key, descriptor] of Object.entries(Object.getOwnPropertyDescriptors(value))) { - // Non-enumerable own props never reach JSON output — skip them, matching - // JSON.stringify's contract exactly (documented in the module doc). - if (!descriptor.enumerable) continue - if (!('value' in descriptor)) { - throw new MaterializeError(`${path}.${key}`, 'accessor properties cannot cross the workflow realm boundary') - } + // Object.keys = own enumerable string keys, matching JSON.stringify's + // property selection exactly (non-enumerable props never reach JSON output). + for (const key of Object.keys(value)) { // defineProperty, never assignment: a "__proto__" key must become an OWN // data property of the copy, not a prototype mutation. Object.defineProperty(out, key, { - value: materialize(descriptor.value, `${path}.${key}`, seen), + value: materialize((value as Record)[key], `${path}.${key}`, seen), enumerable: true, writable: true, configurable: true, diff --git a/packages/workflow/workflow-vm/src/runtime.ts b/packages/workflow/workflow-vm/src/runtime.ts index 955be94a87..7748cb2f2e 100644 --- a/packages/workflow/workflow-vm/src/runtime.ts +++ b/packages/workflow/workflow-vm/src/runtime.ts @@ -4,30 +4,27 @@ * concurrency semaphore and caps, cancellation, and the drive loop that turns * a script settlement into a {@link WorkflowResult}. * - * Realm discipline (see also ./realm.ts): values ENTERING the host from the - * script (hook options, schemas, the return value) are materialized via - * descriptor walks; values ENTERING the realm from the host (`args`, agent() - * results) are rebuilt INSIDE the realm through the context's own - * `JSON.parse`, so the script never holds an object whose prototype chain - * reaches host intrinsics. The same rule covers every other value a hook - * hands the script: the promises `agent`/`parallel`/`pipeline` return are - * realm promises (the realm's own `Promise.resolve` over the host promise), - * the arrays the combinators resolve to are realm-built (their ELEMENTS are - * realm values already — only the container needs rebuilding), and a hook - * failure — rejection or synchronous `phase`/`log` throw — crosses as a - * realm-built clone carrying name/code/message/fatal. Realm functions - * (pipeline stages, parallel thunks) are called, not materialized — their - * values stay realm-side. + * Value boundary (the trust premise lives in ./realm.ts): values ENTERING the + * host from the script (hook options, schemas, the return value) are + * materialized by `materializeFromRealm` — a plain walk that rejects loud + * everything JSON cannot carry. Values ENTERING the realm (`args`, `agent()` + * results, hook promises and their failures, combinator arrays) are handed + * over DIRECTLY as host values: the script is model-written and trusted, so + * host prototypes are not a leak. `args` is host-side `structuredClone`d once + * at start so a script scribbling on it cannot mutate the caller's object — + * that is a benign-bug guard, not isolation. Realm functions (pipeline + * stages, parallel thunks) are called, not materialized — their values stay + * realm-side until they cross through a hook or the final return. * * Failure discipline: fatal {@link WorkflowError}s (bad hook arguments, * unsupported options/schemas, tripped caps, seam start failures, - * cancellation) ALWAYS propagate through `parallel`/`pipeline` — they cross - * the realm boundary as fatal clones, recognized structurally — and the - * per-item `null` is reserved for child-run failures and ordinary in-stage - * script errors. Every hook-returned promise gets a no-op rejection consumer - * attached, so a script that drops a promise (fires an `agent()` without - * awaiting it) cannot surface an unhandled rejection when cancellation - * rejects it — the app boot layer exits the process on unhandled rejections. + * cancellation) ALWAYS propagate through `parallel`/`pipeline` — recognized + * by host `instanceof`, which a script cannot forge — and the per-item `null` + * is reserved for child-run failures and ordinary in-stage script errors. + * Every hook-returned promise gets a no-op rejection consumer attached, so a + * script that drops a promise (fires an `agent()` without awaiting it) cannot + * surface an unhandled rejection when cancellation rejects it — the app boot + * layer exits the process on unhandled rejections. * * @module @deepseek-ai/dsh-workflow-vm/runtime */ @@ -39,14 +36,14 @@ import type { Agent } from '@deepseek-ai/dsh-agent' import type {} from '@deepseek-ai/dsh-subagent' import { assertSupportedOutputSchema, OutputSchemaError } from '@deepseek-ai/dsh-tools' import type { StructuredOutputSchema } from '@deepseek-ai/dsh-tools' -import { WorkflowError } from '@deepseek-ai/dsh-workflow' +import { isFatalWorkflowError, WorkflowError } from '@deepseek-ai/dsh-workflow' import type { WorkflowAgentEndInfo, WorkflowAgentInfo, WorkflowMeta, WorkflowResult, } from '@deepseek-ai/dsh-workflow' -import { materializeFromRealm, MaterializeError, describeThrown, thrownRendering, isFatalWorkflowErrorClone, REALM_THROWN_RENDERER_SOURCE } from './realm.ts' +import { materializeFromRealm, MaterializeError, renderThrown } from './realm.ts' /** The per-run knobs the engine resolves from its Config. */ export interface ExecutionLimits { @@ -60,6 +57,8 @@ export interface ExecutionLimits { maxItemsPerCall: number /** vm timeout for the script's initial synchronous slice. */ syncTimeoutMs: number + /** How long after `cancel()` a still-unsettled script is abandoned (result force-settles `cancelled`). */ + disposeGraceMs: number } /** The engine-side observers the execution reports progress through. */ @@ -124,13 +123,24 @@ export class WorkflowExecution { private readonly controller = new AbortController() private currentPhase: string | undefined private readonly context: vm.Context - private readonly realmJsonParse: (text: string) => unknown - private readonly realmArrayFrom: (items: unknown[]) => unknown[] - private readonly realmPromiseResolve: (value: unknown) => Promise - private readonly realmErrorClone: (name: string, code: string | undefined, message: string, fatal: boolean) => unknown private readonly compiled: vm.Script /** Every live `agent()` call promise — awaited or stray — for {@link quiesce}. */ private readonly inFlightAgents = new Set>() + /** Fires {@link abandoned}; assigned by the promise executor at field initialization. */ + private declareAbandoned!: () => void + private abandonTimer: NodeJS.Timeout | undefined + /** + * Rejects `disposeGraceMs` after {@link cancel} if the script has not + * settled by then. `drive()` races the script against it, so `result` + * ALWAYS settles within the grace of a cancellation — even when the script + * is parked on a promise no hook owns (`await new Promise(() => {})`), which + * cancellation cannot reject. Without this, a consumer awaiting `result` + * before disposing (the tool's shape) would hang forever on such a script, + * wedging its caller past any abort. + */ + private readonly abandoned = new Promise((_, reject) => { + this.declareAbandoned = () => { reject(new WorkflowError('workflow script abandoned after the cancellation grace', 'CANCELLED')) } + }) constructor( private readonly ctx: Context, @@ -144,61 +154,34 @@ export class WorkflowExecution { ) { // Compile FIRST: a body syntax error must throw out of the constructor // (the engine maps it to SCRIPT_PARSE) before any realm state exists. - // The body is wrapped in a realm-side catch that pre-renders any thrown - // value to a string (see REALM_THROWN_RENDERER_SOURCE) — rendering happens - // inside the realm's own execution window, never on a host catch path. // lineOffset compensates for the wrapper line, so stack traces carry the // script's own line numbers (the meta statement was blanked, not removed). try { - this.compiled = new vm.Script( - `(async () => { try {\n${body}\n} catch (e) { throw (${REALM_THROWN_RENDERER_SOURCE})(e) } })()`, - { - filename: `workflow:${meta.name}`, - lineOffset: -1, - }, - ) + this.compiled = new vm.Script(`(async () => {\n${body}\n})()`, { + filename: `workflow:${meta.name}`, + lineOffset: -1, + }) } catch (error: unknown) { throw new WorkflowError(`workflow script does not parse: ${String(error)}`, 'SCRIPT_PARSE', { cause: error }) } this.context = vm.createContext({}, { name: `workflow:${meta.name}` }) vm.runInContext(DETERMINISM_PRELUDE, this.context) - // The realm's own JSON.parse — the host→realm rebuild channel. - const realmJson = vm.runInContext('JSON', this.context) as { parse(text: string): unknown } - this.realmJsonParse = (text: string) => realmJson.parse(text) - // The realm's own Array.from / Promise.resolve / an error factory, bound - // NOW so a script reassigning its globals later cannot swap them: - // combinator results must be realm arrays, hook promises realm promises, - // and hook failures realm-built clones. - this.realmArrayFrom = vm.runInContext('Array.from.bind(Array)', this.context) as (items: unknown[]) => unknown[] - this.realmPromiseResolve = vm.runInContext('Promise.resolve.bind(Promise)', this.context) as (value: unknown) => Promise - this.realmErrorClone = vm.runInContext(`(name, code, message, fatal) => { - const error = new Error(message) - error.name = name - if (code !== undefined) error.code = code - error.fatal = fatal - return error - }`, this.context) as (name: string, code: string | undefined, message: string, fatal: boolean) => unknown + // A run that settles without ever being abandoned leaves `abandoned` + // permanently pending or rejecting into the void — consume it so a late + // grace timer cannot surface an unhandled rejection. + void this.contain(this.abandoned) const globals: Record = { - agent: (prompt: unknown, opts?: unknown) => this.realmFacing(this.track(this.agent(prompt, opts))), - parallel: (thunks: unknown) => this.realmFacing(this.parallel(thunks)), - pipeline: (items: unknown, ...stages: unknown[]) => this.realmFacing(this.pipeline(items, stages)), - phase: (title: unknown) => { - try { - this.phase(title) - } catch (error: unknown) { - throw this.toRealmError(error) - } - }, - log: (message: unknown) => { - try { - this.log(message) - } catch (error: unknown) { - throw this.toRealmError(error) - } - }, - args: this.toRealm(args), + agent: (prompt: unknown, opts?: unknown) => this.contain(this.track(this.agent(prompt, opts))), + parallel: (thunks: unknown) => this.contain(this.parallel(thunks)), + pipeline: (items: unknown, ...stages: unknown[]) => this.contain(this.pipeline(items, stages)), + phase: (title: unknown) => { this.phase(title) }, + log: (message: unknown) => { this.log(message) }, + // Host-side clone: a script scribbling on args must not mutate the + // caller's object (a benign-bug guard; args is plain JSON by the seam + // contract, so structuredClone is total here and throws loud otherwise). + args: args === undefined ? undefined : structuredClone(args), } for (const [key, value] of Object.entries(globals)) { // Data properties on the contextified global; frozen shape not required — @@ -226,7 +209,10 @@ export class WorkflowExecution { /** * Cancel the run: children abort (the shared signal), waiting `agent()` * slots reject, and every future hook call throws `CANCELLED` — the script - * dies at its next await. Idempotent; the first reason wins. + * dies at its next await. A script that STILL has not settled after + * `disposeGraceMs` (parked on a promise no hook owns) is abandoned so + * `result` settles regardless (see {@link abandoned}). Idempotent; the + * first reason wins. */ cancel(reason?: string): void { if (this.cancelReason !== undefined) return @@ -234,14 +220,18 @@ export class WorkflowExecution { this.cancelError = new WorkflowError(`workflow run cancelled: ${this.cancelReason}`, 'CANCELLED') this.controller.abort(this.cancelReason) for (const waiter of this.slotWaiters.splice(0)) waiter.reject(this.cancelledError()) + this.abandonTimer = setTimeout(() => { this.declareAbandoned() }, this.limits.disposeGraceMs) + // unref'd: an armed grace timer must never hold the process open. + this.abandonTimer.unref() } /** * Run the script to settlement. Resolves — never rejects — with the run's * {@link WorkflowResult}: the materialized return value on `completed`, the * failure message on `error`, and `cancelled` when the script died of - * cancellation. After settlement, any stray children a script fired without - * awaiting are aborted (their `agent()` wrappers dispose them). + * cancellation (or outlived its post-cancel grace and was abandoned — see + * {@link abandoned}). After settlement, any stray children a script fired + * without awaiting are aborted (their `agent()` wrappers dispose them). */ async drive(): Promise { try { @@ -249,7 +239,9 @@ export class WorkflowExecution { // the script must not execute at all, let alone report `completed`. if (this.isCancelled()) throw this.cancelledError() const scriptPromise = this.compiled.runInContext(this.context, { timeout: this.limits.syncTimeoutMs }) as Promise - const raw: unknown = await this.contain(Promise.resolve(scriptPromise)) + // The race is the result-settles-after-cancel guarantee: a parked + // script loses to the abandon channel once the grace expires. + const raw: unknown = await Promise.race([this.contain(Promise.resolve(scriptPromise)), this.abandoned]) // Cancelled while the body ran: a script that settled without touching // another hook (or without any) must still report `cancelled` — the // holder asked for cancellation and `completed` would be a lie. @@ -258,25 +250,23 @@ export class WorkflowExecution { return { value, stopReason: 'completed', agentsStarted: this.started } } catch (error: unknown) { // Any failure after cancel() reports `cancelled` with the canonical - // reason — the reject path mirrors the resolve path's post-settle - // check, and a hook CANCELLED failure crosses the realm boundary as a - // clone that deliberately fails the host `instanceof`. + // reason — the reject path mirrors the resolve path's post-settle check. if (this.isCancelled()) { return { value: null, stopReason: 'cancelled', error: this.cancelledError().message, agentsStarted: this.started } } - // Ordinary script failures arrive pre-rendered by the realm-side catch - // (thrownRendering); host-thrown errors (a vm timeout, a WorkflowError) - // and adversarial values that bypassed the wrapper (e.g. a hostile - // thenable rejection) render via the total, host-code-only - // describeThrown. Neither path can throw — drive() resolving is the - // `result` never-rejects seam contract. - return { value: null, stopReason: 'error', error: thrownRendering(error) ?? describeThrown(error), agentsStarted: this.started } + // renderThrown is total (host- and realm-thrown values alike), so this + // arm cannot throw — drive() resolving is the `result` never-rejects + // seam contract. + return { value: null, stopReason: 'error', error: renderThrown(error), agentsStarted: this.started } } finally { // Reap strays: a script that fired agent() calls without awaiting them // leaves live children behind after settlement — abort them all. (The // per-call wrappers dispose each child; the contain() consumer keeps // their rejections from going unhandled.) if (this.cancelReason === undefined) this.cancel('workflow settled') + // drive() settling means nothing is left to abandon — including the + // timer the self-cancel above just armed. + if (this.abandonTimer !== undefined) clearTimeout(this.abandonTimer) } } @@ -291,37 +281,6 @@ export class WorkflowExecution { return promise } - /** - * Hand a hook's host promise to the script as a REALM promise (the realm's - * own `Promise.resolve` assimilates it) whose failure reason is a - * realm-built clone — the script must never hold host prototypes, and both - * the promise object and a caught rejection would otherwise expose them - * (module doc). The realm promise gets the same no-op rejection consumer as - * {@link contain}, since the script may drop it; the intermediate host - * promises are handled by the assimilation chain itself. - */ - private realmFacing(hostPromise: Promise): Promise { - const translated = hostPromise.catch((error: unknown) => { - throw this.toRealmError(error) - }) - const realmPromise = this.realmPromiseResolve(translated) - realmPromise.catch(() => { /* consumed: a script-dropped realm promise must not surface an unhandled rejection (see contain) */ }) - return realmPromise - } - - /** - * Rebuild a host failure as a realm-built error clone: a `WorkflowError` - * keeps its name/code/message/fatal (the combinators recognize the shape - * via {@link isFatalWorkflowErrorClone}); anything else becomes a generic - * realm `Error` carrying its {@link describeThrown} rendering. - */ - private toRealmError(error: unknown): unknown { - if (error instanceof WorkflowError) { - return this.realmErrorClone('WorkflowError', error.code, error.message, error.fatal) - } - return this.realmErrorClone('Error', undefined, describeThrown(error), false) - } - /** * Register one `agent()` call promise for {@link quiesce} tracking; the * entry drops when the call fully settles (which is AFTER its child's @@ -354,14 +313,6 @@ export class WorkflowExecution { return this.cancelError ?? new WorkflowError('workflow run cancelled', 'CANCELLED') } - /** Rebuild a host value inside the script realm (via the realm's own JSON.parse). */ - private toRealm(value: unknown): unknown { - if (value === undefined) return undefined - if (value === null) return null - if (typeof value === 'string' || typeof value === 'number' || typeof value === 'boolean') return value - return this.realmJsonParse(JSON.stringify(value)) - } - /** Materialize the script's return value; violations become RESULT_UNSERIALIZABLE. */ private materializeResult(raw: unknown): unknown { try { @@ -453,7 +404,7 @@ export class WorkflowExecution { return null } this.observer.agentEnd({ ...info, outcome: 'completed' }) - return this.toRealm(result.structured) + return result.structured } this.observer.agentEnd({ ...info, outcome: 'completed' }) return outputText(result.output) @@ -532,20 +483,17 @@ export class WorkflowExecution { } return thunk as () => unknown }) - const settled = await Promise.all(thunks.map(async (thunk) => { + return Promise.all(thunks.map(async (thunk) => { try { return await thunk() } catch (error: unknown) { - // Hooks translate host errors at the realm boundary, so a fatal error - // reaches a thunk catch only as a realm clone (a script forging the - // shape merely kills its own run). - if (isFatalWorkflowErrorClone(error)) throw error + // Hook failures are host WorkflowErrors; a fatal one is recognized by + // host `instanceof` — a script-built object can never pass it, so + // fatality cannot be forged (nor accidentally dissolved). + if (isFatalWorkflowError(error)) throw error return null } })) - // The container must be a REALM array (module doc); the elements are - // realm values already. - return this.realmArrayFrom(settled) } /** The `pipeline(items, ...stages)` hook: per-item stage chains, NO cross-stage barrier. */ @@ -563,7 +511,7 @@ export class WorkflowExecution { } return stage as (previous: unknown, item: unknown, index: number) => unknown }) - const settled = await Promise.all(rawItems.map(async (item: unknown, index) => { + return Promise.all(rawItems.map(async (item: unknown, index) => { let value: unknown = item try { for (const stage of stages) { @@ -572,15 +520,12 @@ export class WorkflowExecution { return value } catch (error: unknown) { // An ordinary stage throw drops the ITEM to null and skips its - // remaining stages; a fatal error (a realm clone — see parallel()) - // kills the whole script. - if (isFatalWorkflowErrorClone(error)) throw error + // remaining stages; a fatal host WorkflowError (see parallel()) kills + // the whole script. + if (isFatalWorkflowError(error)) throw error return null } })) - // The container must be a REALM array (module doc); the elements are - // realm values already. - return this.realmArrayFrom(settled) } private assertItemCap(length: number, hook: string): void { diff --git a/packages/workflow/workflow-vm/tests/meta.spec.ts b/packages/workflow/workflow-vm/tests/meta.spec.ts index a957b0ae05..02daa02940 100644 --- a/packages/workflow/workflow-vm/tests/meta.spec.ts +++ b/packages/workflow/workflow-vm/tests/meta.spec.ts @@ -103,29 +103,19 @@ return 2` }) it('rejects a literal evaluating to non-JSON data (META_INVALID via materialization)', () => { - const error = bad('export const meta = { name: "x", description: "d", phases: [{ get title() { return "t" } }] }') + const error = bad('export const meta = { name: "x", description: "d", whenToUse: () => 1 }') expect(error.code).toBe('META_INVALID') expect(error.message).toContain('JSON data') }) - it('rejects a meta literal containing a proxy as META_INVALID — its traps never run', () => { - // bad() rethrows anything that is not a WorkflowError, so a trap firing - // ('trap ran') would fail this test instead of mapping to META_INVALID. - const error = bad('export const meta = { name: "x", description: "d", phases: new Proxy([], { getPrototypeOf() { throw new Error("trap ran") } }) }') - expect(error.code).toBe('META_INVALID') - expect(error.message).toContain('proxies cannot cross') - }) - - it('a meta expression THROWING a hostile value maps to META_INVALID — rendering stays realm-side', () => { - // bad() rethrows anything that is not a WorkflowError, so a hostile value - // escaping the realm-side renderer raw would fail this test. - const error = bad('export const meta = { name: (() => { throw { get stack() { throw new Error("boom") }, toString() { throw new Error("boom") } } })(), description: "d" }\nreturn 1') + it('a meta expression that THROWS maps to META_INVALID carrying the rendered value', () => { + const error = bad('export const meta = { name: (() => { throw "nope" })(), description: "d" }\nreturn 1') expect(error.code).toBe('META_INVALID') expect(error.message).toContain('pure literal') - expect(error.message).toContain('[unrenderable thrown value]') + expect(error.message).toContain('nope') }) - it('a spinning meta expression (even inside a thrown stack getter) dies by the eval timeout', () => { + it('a spinning meta expression dies by the eval timeout', () => { try { extractMeta('export const meta = { name: (() => { while (true) {} })(), description: "d" }', 50) throw new Error('expected the extraction to time out') diff --git a/packages/workflow/workflow-vm/tests/realm.spec.ts b/packages/workflow/workflow-vm/tests/realm.spec.ts index c1bdeb33d6..769a19aa71 100644 --- a/packages/workflow/workflow-vm/tests/realm.spec.ts +++ b/packages/workflow/workflow-vm/tests/realm.spec.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest' import * as vm from 'node:vm' -import { materializeFromRealm, MaterializeError, describeThrown, thrownRendering } from '../src/realm.ts' +import { materializeFromRealm, MaterializeError, renderThrown } from '../src/realm.ts' /** Evaluate an expression inside a fresh vm realm and hand back the raw realm value. */ function inRealm(expression: string): unknown { @@ -35,17 +35,21 @@ describe('materializeFromRealm', () => { expect(rejection(inRealm('{ a: undefined }'))).toContain('value.a') }) - it('never invokes accessors: a counting getter is rejected, not read', () => { + it('invokes getters ordinarily — the getter RESULT is what crosses (trust premise)', () => { const counter = inRealm(` (() => { globalThis.reads = 0 - return { get x() { globalThis.reads += 1; return 1 } } + return { get x() { globalThis.reads += 1; return globalThis.reads } } })() `) - expect(rejection(counter)).toContain('accessor properties cannot cross') - // The getter body never ran — descriptor inspection only. - expect((counter as { x?: unknown }).x).toBe(1) // sanity: reading DOES run it… - expect(rejection(counter)).toContain('accessor') // …but materialization still never did + expect(materializeFromRealm(counter)).toEqual({ x: 1 }) + }) + + it('a getter that THROWS surfaces as a MaterializeError carrying the rendered failure', () => { + const hostile = inRealm("{ get x() { throw new Error('read failed') } }") + const message = rejection(hostile) + expect(message).toContain('reading the value threw') + expect(message).toContain('read failed') }) it('a "__proto__" key becomes an OWN data property of the copy, never a prototype mutation', () => { @@ -81,39 +85,18 @@ describe('materializeFromRealm', () => { expect(materializeFromRealm(inRealm('Object.assign(Object.create(null), { a: 1 })'))).toEqual({ a: 1 }) }) - it('rejects proxies (root, nested, revoked, host-realm) WITHOUT running any trap', () => { - const trapped = inRealm(`new Proxy({ a: 1 }, { - ownKeys() { throw new Error('trap ran') }, - getOwnPropertyDescriptor() { throw new Error('trap ran') }, - getPrototypeOf() { throw new Error('trap ran') }, - })`) - // A trap firing would surface 'trap ran' (a non-MaterializeError) instead. - expect(rejection(trapped)).toContain('proxies cannot cross') - expect(rejection(inRealm('{ nested: new Proxy([], {}) }'))).toContain('value.nested') - const revoked = inRealm('(() => { const r = Proxy.revocable({}, {}); r.revoke(); return r.proxy })()') - expect(rejection(revoked)).toContain('proxies cannot cross') - expect(rejection(new Proxy({}, {}))).toContain('proxies cannot cross') - }) - - it('rejects an object whose PROTOTYPE is a proxy without dereferencing through it', () => { - const value = inRealm(`Object.create(new Proxy({}, { - getPrototypeOf() { throw new Error('trap ran') }, - }))`) - expect(rejection(value)).toContain('exotic prototype') - }) - it('rejects cycles and accepts the same object reused as a sibling (a DAG)', () => { expect(rejection(inRealm('(() => { const o = {}; o.self = o; return o })()'))).toContain('circular') const dag = inRealm('(() => { const leaf = { v: 1 }; return { a: leaf, b: leaf } })()') expect(materializeFromRealm(dag)).toEqual({ a: { v: 1 }, b: { v: 1 } }) }) - it('rejects sparse arrays, accessor elements, and non-index array properties', () => { + it('rejects sparse arrays and non-index array properties; an array getter element materializes its value', () => { expect(rejection(inRealm('[1, , 3]'))).toContain('sparse') - expect(rejection(inRealm('(() => { const a = [1]; Object.defineProperty(a, 0, { get: () => 1 }); return a })()'))) - .toContain('accessor') expect(rejection(inRealm('(() => { const a = [1]; a.total = 3; return a })()'))) .toContain('non-index') + expect(materializeFromRealm(inRealm('(() => { const a = [1]; Object.defineProperty(a, 0, { get: () => 7, enumerable: true }); return a })()'))) + .toEqual([7]) }) it('skips non-enumerable own properties (matching JSON.stringify exactly)', () => { @@ -134,45 +117,29 @@ describe('materializeFromRealm', () => { }) }) -describe('describeThrown (host-side thrown-value rendering)', () => { - it('renders a HOST Error via its identity-verified native stack getter', () => { - const error = new Error('host failure') - const rendered = describeThrown(error) - expect(rendered).toContain('host failure') - expect(rendered).toContain('at ') // a real stack, not just the message - }) - - it('never invokes a REALM error stack getter (identity mismatch) — message renders instead', () => { +describe('renderThrown', () => { + it('prefers the stack, for host and realm errors alike', () => { + const host = renderThrown(new Error('host failure')) + expect(host).toContain('host failure') + expect(host).toContain('at ') // a real stack, not just the message const realmError: unknown = vm.runInNewContext('(() => { try { throw new Error("realm failure") } catch (e) { return e } })()') - expect(describeThrown(realmError)).toBe('realm failure') + expect(renderThrown(realmError)).toContain('realm failure') }) - it('reads a data-property stack directly and falls through a setter-only accessor', () => { - expect(describeThrown({ stack: 'data stack' })).toBe('data stack') - const setterOnly = { message: 'via message' } - Object.defineProperty(setterOnly, 'stack', { set() { /* swallow */ } }) - expect(describeThrown(setterOnly)).toBe('via message') + it('falls back from stack to message to String()', () => { + expect(renderThrown({ stack: 'custom data stack' })).toBe('custom data stack') + const stackless = new Error('stackless failure') + delete stackless.stack + expect(renderThrown(stackless)).toBe('stackless failure') + expect(renderThrown({ code: 42 })).toBe('[object Object]') + expect(renderThrown('plain')).toBe('plain') + expect(renderThrown(42)).toBe('42') + expect(renderThrown(undefined)).toBe('undefined') + expect(renderThrown(null)).toBe('null') }) - it('labels proxies and functions without touching them; primitives stringify', () => { - expect(describeThrown(new Proxy({}, { getOwnPropertyDescriptor() { throw new Error('trap ran') } }))).toBe('[thrown proxy]') - expect(describeThrown(() => 1)).toBe('[thrown function]') - expect(describeThrown('plain')).toBe('plain') - expect(describeThrown(42)).toBe('42') - expect(describeThrown(undefined)).toBe('undefined') - expect(describeThrown(null)).toBe('null') - expect(describeThrown({ code: 42 })).toBe('[object Object]') - }) -}) - -describe('thrownRendering (the realm-catch wrapper reader)', () => { - it('extracts the pre-rendered string from a wrapper and nothing else', () => { - expect(thrownRendering({ __wfThrown: 'rendered text' })).toBe('rendered text') - expect(thrownRendering({ __wfThrown: 42 })).toBeUndefined() - expect(thrownRendering({ other: 'x' })).toBeUndefined() - expect(thrownRendering(new Error('plain'))).toBeUndefined() - expect(thrownRendering('string')).toBeUndefined() - expect(thrownRendering(null)).toBeUndefined() - expect(thrownRendering(new Proxy({ __wfThrown: 'forged' }, { getOwnPropertyDescriptor() { throw new Error('trap ran') } }))).toBeUndefined() + it('is total: a value whose accessors/toString throw renders as a fixed label', () => { + expect(renderThrown({ get stack() { throw new Error('nope') } })).toBe('[unrenderable thrown value]') + expect(renderThrown({ [Symbol.toPrimitive]() { throw new Error('nope') } })).toBe('[unrenderable thrown value]') }) }) diff --git a/packages/workflow/workflow-vm/tests/workflow-vm.spec.ts b/packages/workflow/workflow-vm/tests/workflow-vm.spec.ts index d87d571295..e362ed6ef0 100644 --- a/packages/workflow/workflow-vm/tests/workflow-vm.spec.ts +++ b/packages/workflow/workflow-vm/tests/workflow-vm.spec.ts @@ -289,22 +289,13 @@ describe('dsh-workflow-vm', () => { () => agent('fine'), () => 'plain value', () => { throw 'string throw' }, - () => { throw new Proxy({ name: 'WorkflowError', fatal: true }, {}) }, - () => { throw { name: 'WorkflowError', fatal: 'forged-but-not-true' } }, + () => { throw { name: 'WorkflowError', fatal: true, message: 'forged fatal' } }, ]) `)) - // The last three probe the fatal-clone recognition: a non-object, a - // proxy (never inspected), and a shape miss are all ordinary nulls. - expect(result.value).toEqual([null, 'stub reply', 'plain value', null, null, null]) - }) - - it('a script forging a fatal clone kills only its own run (self-sabotage, not a bypass)', async () => { - const { ctx, parent } = await setup() - const result = await run(ctx, parent, script(` - return await parallel([() => { throw { name: 'WorkflowError', fatal: true, message: 'forged fatal' } }]) - `)) - expect(result.stopReason).toBe('error') - expect(result.error).toContain('forged fatal') + // The last entry probes fatality: it is recognized by host instanceof, + // which a script-built object can never pass — a WorkflowError-SHAPED + // throw is an ordinary null, and real fatality cannot be forged. + expect(result.value).toEqual([null, 'stub reply', 'plain value', null, null]) }) it('FATAL errors propagate through parallel AND pipeline instead of dissolving into null', async () => { @@ -386,11 +377,12 @@ describe('dsh-workflow-vm', () => { expect((await run(ctx, parent, script("return await agent('p', { effort: 'high' })"))).error).toContain('"effort" is deferred') }) - it('rejects options that are not plain JSON data (an accessor smuggled into opts)', async () => { + it('rejects options whose property reads throw (materialization is loud, not silent)', async () => { const { ctx, parent } = await setup() - const result = await run(ctx, parent, script("return await agent('p', { get label() { return 'x' } })")) + const result = await run(ctx, parent, script("return await agent('p', { get label() { throw new Error('read failed') } })")) expect(result.stopReason).toBe('error') expect(result.error).toContain('options must be plain JSON data') + expect(result.error).toContain('read failed') }) it('validates phase() and log() arguments loudly', async () => { @@ -416,7 +408,7 @@ describe('dsh-workflow-vm', () => { }) }) - describe('determinism bans and realm isolation', () => { + describe('determinism bans and the value boundary', () => { it('Date.now, Math.random, and argless new Date throw; parameterized Date stays usable', async () => { const { ctx, parent } = await setup() expect((await run(ctx, parent, script('return Date.now()'))).error).toContain('Date.now() is not available') @@ -426,18 +418,16 @@ describe('dsh-workflow-vm', () => { expect(ok.value).toBe(0) }) - it('args cross into the realm as data: mutating them (or their prototype chain) cannot reach host intrinsics', async () => { + it('args are cloned at start: a script scribbling on them cannot mutate the caller\'s object', async () => { const { ctx, parent } = await setup() const hostArgs = { files: ['a.ts'], nested: { deep: [1, 2] } } const result = await run(ctx, parent, script(` args.files.push('b.ts') - Object.getPrototypeOf(args).polluted = 'realm-only' return { count: args.files.length, deep: args.nested.deep[1] } `), hostArgs) expect(result.value).toEqual({ count: 2, deep: 2 }) - // The host copy is untouched, and the HOST Object.prototype was never reachable. + // The caller's object is untouched (the engine cloned args host-side). expect(hostArgs.files).toEqual(['a.ts']) - expect(({} as Record).polluted).toBeUndefined() }) it('scalar/null args pass through directly; absent args leave the global undefined', async () => { @@ -447,51 +437,24 @@ describe('dsh-workflow-vm', () => { expect((await run(ctx, parent, script('return typeof args'))).value).toBe('undefined') }) - it('hook promises are REALM promises: instanceof holds in-script, host Promise.prototype stays unreachable', async () => { - const { ctx, parent } = await setup() - const result = await run(ctx, parent, script(` - const p = agent('x') - const par = parallel([() => 'v']) - const pipe = pipeline([1], (n) => n) - Object.getPrototypeOf(p).wfLeakProbe = 'realm-only' - return { - agentIsRealmPromise: p instanceof Promise, - parallelIsRealmPromise: par instanceof Promise, - pipelineIsRealmPromise: pipe instanceof Promise, - value: await p, - } - `)) - expect(result.stopReason).toBe('completed') - expect(result.value).toEqual({ - agentIsRealmPromise: true, - parallelIsRealmPromise: true, - pipelineIsRealmPromise: true, - value: 'stub reply', - }) - expect((Promise.prototype as unknown as Record).wfLeakProbe).toBeUndefined() - delete (Promise.prototype as unknown as Record).wfLeakProbe - }) - - it('hook failures cross the boundary as realm-built WorkflowError clones', async () => { + it('hook failures reach the script as HOST WorkflowErrors: fields readable, in-realm instanceof Error is false', async () => { const { ctx, parent } = await setup() const result = await run(ctx, parent, script(` try { await agent('p', { bogus: true }) return 'unreachable' } catch (e) { - Object.getPrototypeOf(Object.getPrototypeOf(e)).wfErrLeakProbe = 'realm-only' + // The documented consequence of the trust premise: hook errors are + // host objects, so realm instanceof is false — read the fields. return { isRealmError: e instanceof Error, name: e.name, code: e.code, fatal: e.fatal, message: e.message } } `)) expect(result.stopReason).toBe('completed') - expect(result.value).toMatchObject({ isRealmError: true, name: 'WorkflowError', code: 'UNSUPPORTED_OPTION', fatal: true }) + expect(result.value).toMatchObject({ isRealmError: false, name: 'WorkflowError', code: 'UNSUPPORTED_OPTION', fatal: true }) expect((result.value as { message: string }).message).toContain('"bogus" is not recognized') - // The script mutated its error's prototype CHAIN — host intrinsics untouched. - expect((Object.prototype as unknown as Record).wfErrLeakProbe).toBeUndefined() - expect((Error.prototype as unknown as Record).wfErrLeakProbe).toBeUndefined() }) - it('a non-WorkflowError host failure (a rejecting provider result) crosses as a generic realm clone', async () => { + it('a non-WorkflowError host failure (a rejecting provider result) reaches the script raw', async () => { const ctx = new Context() await ctx.plugin(SubagentService) const provider: SubagentProvider = { @@ -507,68 +470,34 @@ describe('dsh-workflow-vm', () => { ctx.subagents.registerProvider(provider) await ctx.plugin(VmWorkflowEngine, { provider: 'rejecting' }) const result = await run(ctx, fakeParent(), script(` - try { await agent('p'); return 'unreachable' } catch (e) { return { isRealmError: e instanceof Error, name: e.name, message: e.message } } + try { await agent('p'); return 'unreachable' } catch (e) { return { name: e.name, message: e.message } } `)) - expect(result.value).toMatchObject({ isRealmError: true, name: 'Error' }) + expect(result.value).toMatchObject({ name: 'Error' }) expect((result.value as { message: string }).message).toContain('backend exploded') }) - it('phase()/log() synchronous throws cross as realm clones too', async () => { + it('phase()/log() throw host WorkflowErrors synchronously on misuse', async () => { const { ctx, parent } = await setup() const result = await run(ctx, parent, script(` try { phase(3) } catch (e) { - if (!(e instanceof Error) || e.name !== 'WorkflowError') throw e + if (e.name !== 'WorkflowError') throw e } try { log(3) } catch (e) { - return { isRealmError: e instanceof Error, name: e.name, message: e.message } + return { name: e.name, message: e.message } } `)) - expect(result.value).toMatchObject({ isRealmError: true, name: 'WorkflowError' }) + expect(result.value).toMatchObject({ name: 'WorkflowError' }) expect((result.value as { message: string }).message).toContain('log() requires') }) - it('parallel/pipeline resolve to REALM arrays: instanceof holds in-script, host intrinsics stay unreachable', async () => { + it('a returned value whose property reads throw fails loud as RESULT_UNSERIALIZABLE', async () => { const { ctx, parent } = await setup() const result = await run(ctx, parent, script(` - const fromParallel = await parallel([() => agent('a'), () => 'plain']) - const fromPipeline = await pipeline([1], (prev) => prev + 1) - Object.getPrototypeOf(fromParallel).polluted = 'realm-only' - return { - parallelIsRealmArray: fromParallel instanceof Array, - pipelineIsRealmArray: fromPipeline instanceof Array, - values: [fromParallel[1], fromPipeline[0]], - } - `)) - expect(result.stopReason).toBe('completed') - expect(result.value).toEqual({ - parallelIsRealmArray: true, - pipelineIsRealmArray: true, - values: ['plain', 2], - }) - // The script's prototype mutation stayed realm-side: the HOST - // Array.prototype was never reachable through a combinator result. - expect(([] as unknown as Record).polluted).toBeUndefined() - }) - - it('a returned proxy is rejected as RESULT_UNSERIALIZABLE without running its traps', async () => { - const { ctx, parent } = await setup() - const result = await run(ctx, parent, script(` - return new Proxy({ a: 1 }, { ownKeys() { throw new Error('trap ran') } }) + return { get a() { throw new Error('read failed') } } `)) expect(result.stopReason).toBe('error') expect(result.error).toContain('not plain JSON data') - expect(result.error).toContain('proxies cannot cross') - expect(result.error).not.toContain('trap ran') - }) - - it('agent() options passed as a proxy are rejected loudly, traps never invoked', async () => { - const { ctx, parent } = await setup() - const result = await run(ctx, parent, script(` - return await agent('p', new Proxy({}, { ownKeys() { throw new Error('trap ran') } })) - `)) - expect(result.stopReason).toBe('error') - expect(result.error).toContain('options must be plain JSON data') - expect(result.error).not.toContain('trap ran') + expect(result.error).toContain('read failed') }) it('a non-JSON return value fails loud as RESULT_UNSERIALIZABLE', async () => { @@ -696,60 +625,6 @@ describe('dsh-workflow-vm', () => { expect(result.error).toBe('[object Object]') }) - it('hostile thrown values render realm-side: result NEVER rejects, no unhandled rejection', async () => { - const unhandled: unknown[] = [] - const onUnhandled = (reason: unknown): void => { unhandled.push(reason) } - process.on('unhandledRejection', onUnhandled) - try { - const { ctx, parent } = await setup() - // Each thrown value runs code (or throws) when rendered — the realm - // wrapper renders it INSIDE script execution, and the host catch only - // ever descriptor-reads the pre-rendered string. - const cases: [string, string][] = [ - ["throw { get stack() { throw new Error('stack getter threw') } }", '[object Object]'], - ["throw { get stack() { throw new Error('x') }, message: 'getter threw, message renders' }", 'getter threw, message renders'], - ["throw { get message() { throw new Error('message getter threw') } }", '[object Object]'], - ["throw { stack: 'custom data stack' }", 'custom data stack'], - ["throw (() => { const o = { message: 'setter-only stack' }; Object.defineProperty(o, 'stack', { set() {} }); return o })()", 'setter-only stack'], - ["throw new Proxy({}, { getOwnPropertyDescriptor() { throw new Error('gopd trap threw') } })", '[object Object]'], - ["throw { [Symbol.toPrimitive]() { throw new Error('toPrimitive threw') } }", '[unrenderable thrown value]'], - ['throw () => 1', '() => 1'], - ['throw null', 'null'], - ] - for (const [body, rendered] of cases) { - const result = await run(ctx, parent, script(body)) - expect(result.stopReason).toBe('error') - expect(result.error).toBe(rendered) - } - // Let any stray rejection reach the process hook before asserting. - await new Promise(resolve => setTimeout(resolve, 20)) - expect(unhandled).toEqual([]) - } finally { - process.off('unhandledRejection', onUnhandled) - } - }) - - it('a synchronous spin hidden in a thrown stack getter dies by the vm timeout, not on the host', async () => { - const { ctx, parent } = await setup({ config: { provider: 'stub', syncTimeoutMs: 50 } }) - // The realm-side renderer reads e.stack INSIDE the timed sync slice, so - // the spin is killed exactly like a plain `while (true) {}` body. - const result = await run(ctx, parent, script('throw { get stack() { while (true) {} } }')) - expect(result.stopReason).toBe('error') - expect(result.error?.toLowerCase()).toContain('timed out') - }) - - it('a hostile thenable rejection that bypasses the realm wrapper renders host-side, data-only', async () => { - const { ctx, parent } = await setup() - // Returning a thenable makes the host unwrap it AFTER the script - // settled — its rejection value skips the realm catch entirely and hits - // drive()'s catch raw. The proxy must be labelled, its traps never run. - const result = await run(ctx, parent, script(` - return { then(_resolve, reject) { reject(new Proxy({}, { getOwnPropertyDescriptor() { throw new Error('trap ran') } })) } } - `)) - expect(result.stopReason).toBe('error') - expect(result.error).toBe('[thrown proxy]') - }) - it('falls back to the message for an Error whose stack was stripped', async () => { const { ctx, parent } = await setup() const result = await run(ctx, parent, script(` @@ -803,18 +678,43 @@ describe('dsh-workflow-vm', () => { } }) - it('dispose() abandons a stuck script after the grace instead of hanging (result stays pending)', async () => { + it('cancel() force-settles the result of a script parked on a promise no hook owns', async () => { + const { ctx, parent } = await setup({ config: { provider: 'stub', disposeGraceMs: 30 } }) + const handle = ctx.workflows.start({ + // No hooks involved: an unsettleable await cancellation cannot reject + // — the abandon grace is the only thing that can settle this run. + script: script("await new Promise(() => {})\nreturn 'unreachable'"), + parent, + }) + handle.cancel('user aborted') + const result = await handle.result + expect(result.stopReason).toBe('cancelled') + expect(result.error).toContain('user aborted') + await handle.dispose() + }) + + it('a never-settling returned thenable is abandoned the same way', async () => { + const { ctx, parent } = await setup({ config: { provider: 'stub', disposeGraceMs: 30 } }) + const handle = ctx.workflows.start({ script: script('return { then() {} }'), parent }) + handle.cancel() + expect((await handle.result).stopReason).toBe('cancelled') + await handle.dispose() + }) + + it('dispose() abandons a stuck script after the grace instead of hanging (result settles cancelled)', async () => { const { ctx, parent } = await setup({ config: { provider: 'stub', disposeGraceMs: 30 } }) const handle = ctx.workflows.start({ - // No hooks involved: an unsettleable await the engine cannot reject. script: script("await new Promise(() => {})\nreturn 'unreachable'"), parent, }) const before = Date.now() await handle.dispose() expect(Date.now() - before).toBeLessThan(1000) - const settled = await Promise.race([handle.result.then(() => 'settled'), Promise.resolve('pending')]) - expect(settled).toBe('pending') + // The abandon that freed dispose() also settled result — a consumer + // still awaiting it (the tool does, before its disposing finally) is + // released rather than wedged forever. + const result = await handle.result + expect(result.stopReason).toBe('cancelled') }) it('dispose() is idempotent and settles cleanly after a completed run', async () => { diff --git a/packages/workflow/workflow/README.md b/packages/workflow/workflow/README.md index 46efa2f2d0..076f5443ac 100644 --- a/packages/workflow/workflow/README.md +++ b/packages/workflow/workflow/README.md @@ -4,7 +4,7 @@ The **workflow seam** (`ctx.workflows`): an abstract service defining WHAT a wor ## Service: `WorkflowService` (abstract) -`start(request: WorkflowStartRequest): WorkflowRun` — parse and execute a script. Throws synchronously (`SCRIPT_PARSE`/`META_INVALID`) for a script that cannot begin; once a run is returned, its `result` NEVER rejects — every failure resolves with `stopReason: 'error'` (or `'cancelled'`). `dispose()` must reach quiescence within a bounded grace (cancel → wait for the script to settle and its children to finish disposing → abandon), never hanging its caller. +`start(request: WorkflowStartRequest): WorkflowRun` — parse and execute a script. Throws synchronously (`SCRIPT_PARSE`/`META_INVALID`) for a script that cannot begin; once a run is returned, its `result` NEVER rejects — every failure resolves with `stopReason: 'error'` (or `'cancelled'`) — and once the run is cancelled, `result` settles within the implementation's bounded grace even if the script itself never settles (a consumer awaiting `result` must never be wedged past a cancellation). `dispose()` must reach quiescence within a bounded grace (cancel → wait for the script to settle and its children to finish disposing → abandon), never hanging its caller. The protected `emitWorkflowEvent` helper dispatches the `workflow/*` events with PER-LISTENER containment and PER-LISTENER payload snapshots (a throwing subscriber is logged, never propagated, and cannot starve later listeners; each subscriber gets its own clone of the payload, so mutating it corrupts neither the engine nor other listeners) — the same containment guarantee as the subagent seam's lifecycle emits. diff --git a/packages/workflow/workflow/src/index.ts b/packages/workflow/workflow/src/index.ts index 5f340e2f93..83b1560563 100644 --- a/packages/workflow/workflow/src/index.ts +++ b/packages/workflow/workflow/src/index.ts @@ -127,8 +127,8 @@ export type WorkflowEventName = * subset (see dsh-tools). * - `AGENT_CAP` / `ITEM_CAP` — the run/agent caps tripped. * - `AGENT_START` — the subagent seam refused to start a child. - * - `RESULT_UNSERIALIZABLE` — a value crossing the realm boundary is not - * plain JSON data. + * - `RESULT_UNSERIALIZABLE` — a value crossing the script/host value boundary + * is not plain JSON data. * - `CANCELLED` — the run was cancelled; pending and future hooks reject * with this (the script-kill mechanism). */ @@ -179,7 +179,10 @@ export function isFatalWorkflowError(error: unknown): boolean { * - {@link start} throws synchronously for a request that cannot begin (an * unparseable script, an invalid meta block). Once it returns a * {@link WorkflowRun}, `result` NEVER rejects — every failure resolves with - * `stopReason: 'error'` (or `'cancelled'`). + * `stopReason: 'error'` (or `'cancelled'`) — and once the run is cancelled, + * `result` SETTLES within the implementation's bounded grace even if the + * script itself never settles (a consumer awaiting `result` must never be + * wedged past a cancellation). * - The `workflow/*` events fire through {@link emitWorkflowEvent} (data * snapshots, per-listener containment); `workflow/end` fires exactly once * per started run, after `result` is settled or as it settles. diff --git a/packages/workflow/workflow/src/types.ts b/packages/workflow/workflow/src/types.ts index 32377ee46b..01e5d08bf0 100644 --- a/packages/workflow/workflow/src/types.ts +++ b/packages/workflow/workflow/src/types.ts @@ -85,7 +85,7 @@ export interface WorkflowResult { stopReason: WorkflowStopReason /** The failure message (present iff `stopReason` is not `completed`). */ error?: string - /** How many `agent()` calls the run started (across its whole lifetime). */ + /** How many `agent()` calls the run accepted (whole lifetime, including calls still queued for a slot when the run was cancelled). */ agentsStarted: number } @@ -93,17 +93,19 @@ export interface WorkflowResult { * The handle the consumer holds while a script executes. The consumer awaits * `result`, may `cancel` mid-flight, and MUST `dispose` on every path. * `result` does NOT reject — a script failure resolves with `stopReason: - * 'error'` — so the consumer maps a non-`completed` reason to an `isError` - * result. `dispose()` cancels, then waits a bounded grace for the script to - * settle before abandoning it (the engine documents the abandonment - * semantics); it never hangs on a stuck script. + * 'error'` — and once the run is cancelled it SETTLES within the engine's + * bounded grace even if the script itself never settles (the engine abandons + * the script and reports `cancelled`), so a consumer awaiting `result` is + * never wedged past a cancellation. `dispose()` = cancel + that bounded + * settle + child quiescence; it never hangs on a stuck script and is safe to + * call on every path (idempotent). */ export interface WorkflowRun { readonly id: WorkflowRunId /** The validated meta block (available before the body runs). */ readonly meta: WorkflowMeta readonly result: Promise - /** Cancel the run: children abort, pending hooks reject, the script dies at its next await. */ + /** Cancel the run: children abort, pending hooks reject, the script dies at its next await (or is abandoned at the grace). */ cancel(reason?: string): void /** Cancel + bounded-grace settle; safe to call on every path (idempotent). */ dispose(): Promise @@ -149,6 +151,6 @@ export interface WorkflowResultInfo { stopReason: WorkflowStopReason /** The failure message (present iff `stopReason` is not `completed`). */ error?: string - /** How many `agent()` calls the run started. */ + /** How many `agent()` calls the run accepted (see {@link WorkflowResult.agentsStarted}). */ agentsStarted: number } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7877dc9fad..093e982930 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1060,6 +1060,9 @@ importers: '@deepseek-ai/dsh-session': specifier: workspace:^ version: link:../../core/session + '@deepseek-ai/dsh-subagent': + specifier: workspace:^ + version: link:../../subagent/subagent '@deepseek-ai/dsh-system-prompt': specifier: workspace:^ version: link:../../core/system-prompt @@ -1069,6 +1072,9 @@ importers: '@deepseek-ai/dsh-workflow': specifier: workspace:^ version: link:../workflow + '@deepseek-ai/dsh-workflow-vm': + specifier: workspace:^ + version: link:../workflow-vm cordis: specifier: ^4.0.0-rc.6 version: 4.0.0-rc.6(@cordisjs/plugin-include@1.0.4)(@cordisjs/plugin-loader@1.0.0-rc.4)