Jelajahi Sumber

workflow: harden the seam-contract tests ahead of the engine swap

Three engine-agnostic pins, landed BEFORE the worker-thread port so the
port commit demonstrates contract preservation against them:

- tool-workflow: the tool:<toolName> prompt-section registration was
  entirely unasserted — assemble() now pins the section present under
  the CONFIGURED name and gone after fiber dispose (the packages
  AGENTS.md dispose-and-assert-cleanup rule; tool-bash is the template).
- tool-workflow: drop the dead `??` re-defaulting of already-
  schemastery-resolved config (the hidden-fallback shape AGENTS.md
  bans) and the direct-apply test that existed only to cover those
  branches; both engines' `config as ResolvedConfig` is the pattern.
- workflow-vm: workflow/end was asserted only on completed runs — the
  cancelled path and the grace force-settle path now pin the event and
  its stopReason/error/agentsStarted payload (an observer's only death
  signal on those paths).
imccyu 2 bulan lalu
induk
melakukan
fbc9eb313c

+ 5 - 2
packages/workflow/tool-workflow/src/index.ts

@@ -49,6 +49,8 @@ export const Config: z<Config> = z.object({
   maxResultChars: z.natural().min(1).default(50_000),
 })
 
+type ResolvedConfig = Required<Config>
+
 /**
  * The script-authoring contract, embedded in the tool description. This IS the
  * model-facing spec: the meta block, the hooks and their exact semantics, and
@@ -120,8 +122,9 @@ function renderResult(run: WorkflowRun, result: WorkflowResult, maxChars: number
 }
 
 export function apply(ctx: Context, config: Config): void {
-  const maxResultChars = config.maxResultChars ?? 50_000
-  const toolName = config.toolName ?? 'workflow'
+  // schemastery (the exported Config schema) has already filled the defaulted
+  // fields; the assertion records that resolution, not a hidden fallback.
+  const { toolName, maxResultChars } = config as ResolvedConfig
   // Usage policy ships with the tool (the master convention: tool guidance
   // lives in tool plugins as prompt sections, not in the deployment persona).
   ctx.systemPrompt.section({

+ 8 - 11
packages/workflow/tool-workflow/tests/tool-workflow.spec.ts

@@ -130,17 +130,6 @@ describe('dsh-tool-workflow', () => {
     expect(engine.disposed).toBe(1)
   })
 
-  it('applies raw-config fallbacks when loaded without schemastery defaults (direct apply)', async () => {
-    const ctx = new Context()
-    await ctx.plugin(SystemPrompt)
-    await ctx.plugin(ToolRegistry)
-    await ctx.plugin(StubEngine)
-    // Direct apply with an empty RAW config: the `??` fallbacks resolve the
-    // tool name and render cap without schemastery having filled them.
-    toolWorkflow.apply(ctx, {})
-    expect(ctx.tools.get('workflow')).toBeDefined()
-  })
-
   it('a synchronous engine start throw (parse/meta failure) becomes an isError result', async () => {
     const { ctx, engine, parent } = await setup()
     engine.startError = new Error('script must begin with `export const meta = {...}`')
@@ -192,8 +181,16 @@ describe('dsh-tool-workflow', () => {
     const fiber = await ctx.plugin(toolWorkflow, { toolName: 'orchestrate' })
     expect(ctx.tools.get('orchestrate')).toBeDefined()
     expect(ctx.tools.get('workflow')).toBeUndefined()
+    // The usage-policy prompt section rides the same registration: present
+    // under the CONFIGURED name (its guidance names the tool it describes)…
+    const sections = (await ctx.systemPrompt.assemble()).sections
+    const section = sections.find(s => s.name === 'tool:orchestrate')
+    expect(section?.text).toContain('orchestrate')
+    expect(sections.some(s => s.name === 'tool:workflow')).toBe(false)
     await fiber.dispose()
     expect(ctx.tools.get('orchestrate')).toBeUndefined()
+    // …and gone with the fiber — a reload must not leak a stale section.
+    expect((await ctx.systemPrompt.assemble()).sections.some(s => s.name === 'tool:orchestrate')).toBe(false)
   })
 
   it('presents a generic pending card titled by the sniffed meta name, with the script as rawInput', async () => {

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

@@ -5,7 +5,7 @@ import { AgentId } from '@deepseek-ai/dsh-agent'
 import type { Agent } from '@deepseek-ai/dsh-agent'
 import SubagentService from '@deepseek-ai/dsh-subagent'
 import type { SubagentCapabilities, SubagentProvider, SubagentResult, SubagentRun, SubagentStartRequest } from '@deepseek-ai/dsh-subagent'
-import type { WorkflowResult, WorkflowRunInfo } from '@deepseek-ai/dsh-workflow'
+import type { WorkflowResult, WorkflowResultInfo, WorkflowRunInfo } from '@deepseek-ai/dsh-workflow'
 import * as vmEngineModule from '../src/index.ts'
 import VmWorkflowEngine, { type Config } from '../src/index.ts'
 
@@ -534,6 +534,8 @@ describe('dsh-workflow-vm', () => {
 
     it('cancel() aborts in-flight children and settles the run cancelled', async () => {
       const { ctx, parent, provider } = await setup({ manual: true })
+      const ends: WorkflowResultInfo[] = []
+      ctx.on('workflow/end', (_info, result) => { ends.push(result) })
       const handle = ctx.workflows.start({ script: script("return await agent('long job')"), parent })
       await vi.waitFor(() => { expect(provider.runs.length).toBe(1) })
       handle.cancel('user stopped it')
@@ -541,6 +543,9 @@ describe('dsh-workflow-vm', () => {
       expect(result.stopReason).toBe('cancelled')
       expect(result.error).toContain('user stopped it')
       expect(provider.runs[0]!.disposed).toBe(true)
+      // workflow/end is an observer's only death signal: it fires for a
+      // cancelled run too, mirroring the settled outcome data.
+      expect(ends).toEqual([{ stopReason: 'cancelled', error: result.error, agentsStarted: 1 }])
       await handle.dispose()
     })
 
@@ -787,6 +792,8 @@ describe('dsh-workflow-vm', () => {
 
     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 ends: WorkflowResultInfo[] = []
+      ctx.on('workflow/end', (_info, result) => { ends.push(result) })
       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.
@@ -797,6 +804,9 @@ describe('dsh-workflow-vm', () => {
       const result = await handle.result
       expect(result.stopReason).toBe('cancelled')
       expect(result.error).toContain('user aborted')
+      // The grace force-settle fires workflow/end exactly like an ordinary
+      // settlement — an abandoned script's death still reaches observers.
+      expect(ends).toEqual([{ stopReason: 'cancelled', error: result.error, agentsStarted: 0 }])
       await handle.dispose()
     })