Procházet zdrojové kódy

workflow: hook promises and hook failures are realm-built too

Codex code-review round 5: agent()/parallel()/pipeline() returned HOST Promise
objects into the script realm — Object.getPrototypeOf(agent('x')) reached host
Promise.prototype, contradicting the realm contract (correctness containment,
not the accepted sandbox stance). The rejection channel had the same leak one
hop away: a caught hook failure was a host WorkflowError (host Error.prototype
chain), and phase()/log() threw host errors synchronously.

All three surfaces are realm-built now:
- hook promises: the realm's own Promise.resolve (bound at context setup)
  assimilates the host promise, so the script-visible promise carries realm
  prototypes; the realm promise gets the same no-op rejection consumer as the
  host one (a script may drop it).
- hook failures: rejections and phase/log sync throws are translated at the
  boundary into realm-built clones (name/code/message/fatal via an in-realm
  factory); non-WorkflowError host failures become generic realm Errors
  carrying their describeThrown rendering.
- the combinators recognize FATAL clones structurally
  (isFatalWorkflowErrorClone: proxy-guarded descriptor reads), preserving the
  fatal-vs-null discipline across the boundary; a script forging the shape
  kills only its own run. drive() maps any post-cancel failure to 'cancelled'
  by run state (a CANCELLED clone deliberately fails the host instanceof).

Tests: realm-promise identity for all three hooks + host Promise.prototype
pollution unreachable; clone shape (instanceof realm Error, name/code/fatal/
message) with prototype-chain mutation staying realm-side; a rejecting
provider result crossing as a generic clone; phase/log sync-throw clones;
combinator catch branches (string throw, proxy throw, shape-miss forgery →
null; forged fatal → kills own run); existing fatal-propagation, cancellation,
and unhandled-rejection tests as canaries.
Tianyi Cui před 3 měsíci
rodič
revize
7234d41b91

+ 1 - 1
docs/rfc/implemented/feature/2026-07-05-dynamic-workflows.md

@@ -28,7 +28,7 @@ One deliberate strictness DIVERGENCE from CC: hook misuse — unknown or deferre
 
 **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 (`args`, `agent()` results) are rebuilt INSIDE the realm via the context's own `JSON.parse`, and `parallel`/`pipeline` resolve to realm-built arrays, so the script never holds a live host-prototype object. Realm functions (stages, thunks) are called, never materialized.
+**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.
 

+ 1 - 1
packages/workflow/workflow-vm/README.md

@@ -10,7 +10,7 @@ The first [`WorkflowService`](../workflow/README.md) implementation: an in-proce
 
 ## Realm discipline
 
-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 (`args`, `agent()` results) are rebuilt INSIDE the realm through the context's own `JSON.parse`, and the arrays `parallel`/`pipeline` resolve to are realm-built, so the script never holds an object whose prototype chain reaches host intrinsics.
+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.
 
 ## Limits, cancellation, disposal
 

+ 16 - 0
packages/workflow/workflow-vm/src/realm.ts

@@ -156,6 +156,22 @@ function ownDataProperty(value: object, key: string): unknown {
   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

+ 90 - 22
packages/workflow/workflow-vm/src/runtime.ts

@@ -9,17 +9,22 @@
  * 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 arrays `parallel`/`pipeline` resolve to are
- * realm-built for the same reason (their ELEMENTS are realm values already —
- * only the container needs rebuilding). Realm functions (pipeline stages,
- * parallel thunks) are called, not materialized — their values stay
- * realm-side.
+ * 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.
  *
  * Failure discipline: fatal {@link WorkflowError}s (bad hook arguments,
  * unsupported options/schemas, tripped caps, seam start failures,
- * cancellation) ALWAYS propagate through `parallel`/`pipeline`; 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
+ * 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.
@@ -34,14 +39,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, isFatalWorkflowError } from '@deepseek-ai/dsh-workflow'
+import { WorkflowError } from '@deepseek-ai/dsh-workflow'
 import type {
   WorkflowAgentEndInfo,
   WorkflowAgentInfo,
   WorkflowMeta,
   WorkflowResult,
 } from '@deepseek-ai/dsh-workflow'
-import { materializeFromRealm, MaterializeError, describeThrown, thrownRendering, REALM_THROWN_RENDERER_SOURCE } from './realm.ts'
+import { materializeFromRealm, MaterializeError, describeThrown, thrownRendering, isFatalWorkflowErrorClone, REALM_THROWN_RENDERER_SOURCE } from './realm.ts'
 
 /** The per-run knobs the engine resolves from its Config. */
 export interface ExecutionLimits {
@@ -121,6 +126,8 @@ export class WorkflowExecution {
   private readonly context: vm.Context
   private readonly realmJsonParse: (text: string) => unknown
   private readonly realmArrayFrom: (items: unknown[]) => unknown[]
+  private readonly realmPromiseResolve: (value: unknown) => Promise<unknown>
+  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<Promise<unknown>>()
@@ -159,16 +166,38 @@ export class WorkflowExecution {
     // 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, bound NOW so a script reassigning its
-    // globals later cannot swap it: combinator results must be realm arrays.
+    // 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<unknown>
+    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
 
     const globals: Record<string, unknown> = {
-      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) },
+      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),
     }
     for (const [key, value] of Object.entries(globals)) {
@@ -228,8 +257,12 @@ export class WorkflowExecution {
       const value = raw === undefined ? null : this.materializeResult(raw)
       return { value, stopReason: 'completed', agentsStarted: this.started }
     } catch (error: unknown) {
-      if (error instanceof WorkflowError && error.code === 'CANCELLED') {
-        return { value: null, stopReason: 'cancelled', error: error.message, agentsStarted: this.started }
+      // 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`.
+      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)
@@ -258,6 +291,37 @@ 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<unknown>): Promise<unknown> {
+    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
@@ -472,7 +536,10 @@ export class WorkflowExecution {
       try {
         return await thunk()
       } catch (error: unknown) {
-        if (isFatalWorkflowError(error)) throw error
+        // 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
         return null
       }
     }))
@@ -505,8 +572,9 @@ 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 kills the whole script.
-        if (isFatalWorkflowError(error)) throw error
+        // remaining stages; a fatal error (a realm clone — see parallel())
+        // kills the whole script.
+        if (isFatalWorkflowErrorClone(error)) throw error
         return null
       }
     }))

+ 95 - 1
packages/workflow/workflow-vm/tests/workflow-vm.spec.ts

@@ -288,9 +288,23 @@ describe('dsh-workflow-vm', () => {
           () => { throw new Error('boom') },
           () => agent('fine'),
           () => 'plain value',
+          () => { throw 'string throw' },
+          () => { throw new Proxy({ name: 'WorkflowError', fatal: true }, {}) },
+          () => { throw { name: 'WorkflowError', fatal: 'forged-but-not-true' } },
         ])
       `))
-      expect(result.value).toEqual([null, 'stub reply', 'plain value'])
+      // 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')
     })
 
     it('FATAL errors propagate through parallel AND pipeline instead of dissolving into null', async () => {
@@ -433,6 +447,86 @@ 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<string, unknown>).wfLeakProbe).toBeUndefined()
+      delete (Promise.prototype as unknown as Record<string, unknown>).wfLeakProbe
+    })
+
+    it('hook failures cross the boundary as realm-built WorkflowError clones', 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'
+          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 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<string, unknown>).wfErrLeakProbe).toBeUndefined()
+      expect((Error.prototype as unknown as Record<string, unknown>).wfErrLeakProbe).toBeUndefined()
+    })
+
+    it('a non-WorkflowError host failure (a rejecting provider result) crosses as a generic realm clone', async () => {
+      const ctx = new Context()
+      await ctx.plugin(SubagentService)
+      const provider: SubagentProvider = {
+        name: 'rejecting',
+        capabilities: { outputSchema: true, depthLimit: true, toolFilter: true },
+        start: () => ({
+          id: AgentId('reject-child'),
+          result: Promise.reject(new Error('backend exploded')),
+          cancel: () => { /* nothing in flight */ },
+          dispose: () => Promise.resolve(),
+        }),
+      }
+      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 } }
+      `))
+      expect(result.value).toMatchObject({ isRealmError: true, name: 'Error' })
+      expect((result.value as { message: string }).message).toContain('backend exploded')
+    })
+
+    it('phase()/log() synchronous throws cross as realm clones too', 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
+        }
+        try { log(3) } catch (e) {
+          return { isRealmError: e instanceof Error, name: e.name, message: e.message }
+        }
+      `))
+      expect(result.value).toMatchObject({ isRealmError: true, 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 () => {
       const { ctx, parent } = await setup()
       const result = await run(ctx, parent, script(`