diff --git a/packages/workflow-engine/src/__tests__/hooks.test.ts b/packages/workflow-engine/src/__tests__/hooks.test.ts index 9b58b81d1..539595bc2 100644 --- a/packages/workflow-engine/src/__tests__/hooks.test.ts +++ b/packages/workflow-engine/src/__tests__/hooks.test.ts @@ -14,11 +14,19 @@ import type { ProgressEvent, } from '../types.js' +const STRUCTURED_SCHEMA = { + type: 'object', + required: ['count'], + properties: { count: { type: 'number' } }, + additionalProperties: false, +} + type CtxOverrides = Partial<{ agentResults: Map runner: (params: AgentRunParams) => Promise pending: { kind: 'skip' | 'retry' } | null journal: JournalEntry[] + appended: JournalEntry[] budgetTotal: number | null signal: AbortSignal truncated: string[] @@ -67,7 +75,9 @@ function buildCtx(overrides: CtxOverrides = {}): { }, journalStore: { read: async () => [], - append: async () => {}, + append: async (_id: string, entry: JournalEntry) => { + overrides.appended?.push(entry) + }, truncate: async (id: string) => { overrides.truncated?.push(id) }, @@ -156,6 +166,25 @@ test('agent dead → retry still dead → final null (dead stays dead)', async ( expect(calls).toBe(2) }) +test('agent dead → retry throws without schema → exactly two attempts and final runagent-threw', async () => { + let calls = 0 + const { ctx, hooks } = buildCtx({ + runner: async () => { + calls++ + if (calls === 1) return { kind: 'dead' as const } + throw new Error('retry failed') + }, + loggerWarn: () => {}, + }) + + expect(await hooks.agent('p')).toBeNull() + expect(calls).toBe(2) + const final = ctx.journal[0]!.result + expect(final.kind === 'dead' ? final.reason : undefined).toBe( + 'runagent-threw', + ) +}) + test('agent non-abort throw → retry once succeeds → ok', async () => { let calls = 0 const { hooks } = buildCtx({ @@ -227,6 +256,163 @@ test('agent skipped → no retry (user actively skips, no retry)', async () => { expect(calls).toBe(1) }) +test('structured output invalid once → retry succeeds → only valid output is charged and journaled', async () => { + let calls = 0 + const { ctx, hooks } = buildCtx({ + runner: async () => { + calls++ + return calls === 1 + ? { + kind: 'ok' as const, + output: { count: 'wrong' }, + usage: { outputTokens: 99 }, + } + : { + kind: 'ok' as const, + output: { count: 2 }, + usage: { outputTokens: 3 }, + } + }, + loggerWarn: () => {}, + }) + + expect(await hooks.agent('p', { schema: STRUCTURED_SCHEMA })).toEqual({ + count: 2, + }) + expect(calls).toBe(2) + expect(ctx.resources.budget.spent()).toBe(3) + expect(ctx.journal).toHaveLength(1) + expect(ctx.journal[0]!.result).toEqual({ + kind: 'ok', + output: { count: 2 }, + usage: { outputTokens: 3 }, + }) +}) + +test('valid structured output succeeds on the first attempt without retry', async () => { + let calls = 0 + const { hooks } = buildCtx({ + runner: async () => { + calls++ + return { + kind: 'ok' as const, + output: { count: 1 }, + usage: { outputTokens: 2 }, + } + }, + }) + + expect(await hooks.agent('p', { schema: STRUCTURED_SCHEMA })).toEqual({ + count: 1, + }) + expect(calls).toBe(1) +}) + +test('structured output invalid twice → final dead is journaled without charging tokens', async () => { + let calls = 0 + const appended: JournalEntry[] = [] + const { ctx, events, hooks } = buildCtx({ + runner: async () => { + calls++ + return { + kind: 'ok' as const, + output: { count: 'wrong' }, + usage: { outputTokens: 99 }, + } + }, + appended, + loggerWarn: () => {}, + }) + + expect(await hooks.agent('p', { schema: STRUCTURED_SCHEMA })).toBeNull() + expect(calls).toBe(2) + expect(ctx.resources.budget.spent()).toBe(0) + expect(ctx.journal).toHaveLength(1) + const final = ctx.journal[0]!.result + expect(final.kind).toBe('dead') + expect(final.kind === 'dead' ? final.reason : undefined).toBe( + 'invalid-structured-output', + ) + expect(final.kind === 'dead' ? final.detail : undefined).toBe( + '/count must be number', + ) + expect(appended).toHaveLength(1) + expect(appended[0]!.result).toEqual(final) + expect( + events.some( + event => + event.type === 'agent_done' && + event.result.kind === 'dead' && + event.result.reason === 'invalid-structured-output', + ), + ).toBe(true) +}) + +test('invalid JSON Schema fails before backend execution and is not retried or journaled', async () => { + let calls = 0 + const { ctx, events, hooks } = buildCtx({ + runner: async () => { + calls++ + return { kind: 'ok', output: {}, usage: { outputTokens: 1 } } + }, + }) + + await expect( + hooks.agent('p', { + schema: { type: 'definitely-not-a-json-schema-type' }, + }), + ).rejects.toThrow(/schema/i) + expect(calls).toBe(0) + expect(ctx.journal).toHaveLength(0) + expect(events.some(event => event.type === 'agent_started')).toBe(false) +}) + +test('structured output invalid then retry throws → exactly two attempts and final runagent-threw', async () => { + let calls = 0 + const { ctx, hooks } = buildCtx({ + runner: async () => { + calls++ + if (calls === 2) throw new Error('retry failed') + return { + kind: 'ok' as const, + output: { count: 'wrong' }, + usage: { outputTokens: 1 }, + } + }, + loggerWarn: () => {}, + }) + + expect(await hooks.agent('p', { schema: STRUCTURED_SCHEMA })).toBeNull() + expect(calls).toBe(2) + const final = ctx.journal[0]!.result + expect(final.kind === 'dead' ? final.reason : undefined).toBe( + 'runagent-threw', + ) +}) + +test('backend throws then retry returns invalid structured output → exactly two attempts and final validation dead', async () => { + let calls = 0 + const { ctx, hooks } = buildCtx({ + runner: async () => { + calls++ + if (calls === 1) throw new Error('first failed') + return { + kind: 'ok' as const, + output: { count: 'wrong' }, + usage: { outputTokens: 1 }, + } + }, + loggerWarn: () => {}, + }) + + expect(await hooks.agent('p', { schema: STRUCTURED_SCHEMA })).toBeNull() + expect(calls).toBe(2) + const final = ctx.journal[0]!.result + expect(final.kind === 'dead' ? final.reason : undefined).toBe( + 'invalid-structured-output', + ) +}) + test('agent journal hit does not call runner', async () => { let called = 0 const { emitter } = createBufferingEmitter() @@ -280,6 +466,125 @@ test('agent journal hit does not call runner', async () => { expect(called).toBe(0) }) +test('valid structured output journal hit is revalidated and skips runner', async () => { + let calls = 0 + const params: AgentRunParams = { + prompt: 'p', + schema: STRUCTURED_SCHEMA, + } + const { hooks } = buildCtx({ + runner: async () => { + calls++ + return { + kind: 'ok', + output: { count: 2 }, + usage: { outputTokens: 1 }, + } + }, + journal: [ + { + key: agentCallKey('p', params), + seq: 0, + result: { + kind: 'ok', + output: { count: 1 }, + usage: { outputTokens: 1 }, + }, + }, + ], + }) + + expect(await hooks.agent('p', { schema: STRUCTURED_SCHEMA })).toEqual({ + count: 1, + }) + expect(calls).toBe(0) +}) + +test('invalid legacy structured output journal hit is invalidated and rerun live', async () => { + let calls = 0 + const truncated: string[] = [] + const warnings: string[] = [] + const params: AgentRunParams = { + prompt: 'p', + schema: STRUCTURED_SCHEMA, + } + const { ctx, hooks } = buildCtx({ + runner: async () => { + calls++ + return { + kind: 'ok', + output: { count: 2 }, + usage: { outputTokens: 1 }, + } + }, + journal: [ + { + key: agentCallKey('p', params), + seq: 0, + result: { + kind: 'ok', + output: { count: 'stale-invalid' }, + usage: { outputTokens: 10 }, + }, + }, + ], + truncated, + loggerWarn: message => { + warnings.push(message) + }, + }) + + expect(await hooks.agent('p', { schema: STRUCTURED_SCHEMA })).toEqual({ + count: 2, + }) + expect(calls).toBe(1) + expect(truncated).toEqual(['r1']) + expect(ctx.journalInvalidated).toBe(true) + expect( + warnings.some(message => + message.includes('does not match its structured output schema'), + ), + ).toBe(true) + expect(ctx.journal).toHaveLength(1) + expect(ctx.journal[0]!.result).toEqual({ + kind: 'ok', + output: { count: 2 }, + usage: { outputTokens: 1 }, + }) +}) + +test('journaled invalid-structured-output dead replays null without rerunning', async () => { + let calls = 0 + const params: AgentRunParams = { + prompt: 'p', + schema: STRUCTURED_SCHEMA, + } + const { hooks } = buildCtx({ + runner: async () => { + calls++ + return { + kind: 'ok', + output: { count: 2 }, + usage: { outputTokens: 1 }, + } + }, + journal: [ + { + key: agentCallKey('p', params), + seq: 0, + result: { + kind: 'dead', + reason: 'invalid-structured-output', + detail: "must have required property 'count'", + }, + }, + ], + }) + + expect(await hooks.agent('p', { schema: STRUCTURED_SCHEMA })).toBeNull() + expect(calls).toBe(0) +}) + test('agent exceeding total cap throws', async () => { const { hooks, ctx } = buildCtx() ctx.resources.agentCountBox.value = 1000 @@ -530,6 +835,45 @@ test('agentAdapterRegistry takes priority over agentRunner (dispatched to adapte expect(called).toEqual(['adapter']) }) +test('agentAdapterRegistry result is validated at the same engine boundary', async () => { + let adapterCalls = 0 + let runnerCalls = 0 + const registry = new AgentAdapterRegistry() + .register({ + id: 'ad', + capabilities: { structuredOutput: true }, + async run() { + adapterCalls++ + return { + kind: 'ok', + output: { count: 'wrong' }, + usage: { outputTokens: 1 }, + } + }, + }) + .default('ad') + const { ctx, hooks } = buildCtx({ + agentAdapterRegistry: registry, + runner: async () => { + runnerCalls++ + return { + kind: 'ok', + output: { count: 1 }, + usage: { outputTokens: 1 }, + } + }, + loggerWarn: () => {}, + }) + + expect(await hooks.agent('p', { schema: STRUCTURED_SCHEMA })).toBeNull() + expect(adapterCalls).toBe(2) + expect(runnerCalls).toBe(0) + const final = ctx.journal[0]!.result + expect(final.kind === 'dead' ? final.reason : undefined).toBe( + 'invalid-structured-output', + ) +}) + test('agentAdapterRegistry resolve throws → agent rethrows (workflow failed)', async () => { const registry = new AgentAdapterRegistry().default('missing') // not registered const { hooks } = buildCtx({ diff --git a/packages/workflow-engine/src/__tests__/structuredOutput.test.ts b/packages/workflow-engine/src/__tests__/structuredOutput.test.ts index 71c760041..94d0bd03b 100644 --- a/packages/workflow-engine/src/__tests__/structuredOutput.test.ts +++ b/packages/workflow-engine/src/__tests__/structuredOutput.test.ts @@ -1,5 +1,8 @@ import { expect, test } from 'bun:test' -import { validateAgainstSchema } from '../engine/structuredOutput.js' +import { + assertValidJsonSchema, + validateAgainstSchema, +} from '../engine/structuredOutput.js' const schema = { type: 'object', @@ -27,8 +30,12 @@ test('missing field fails', () => { }) test('type error fails', () => { - const { valid } = validateAgainstSchema({ name: 'a', count: 'x' }, schema) + const { valid, errors } = validateAgainstSchema( + { name: 'a', count: 'x' }, + schema, + ) expect(valid).toBe(false) + expect(errors).toContain('/count must be number') }) test('same schema reuses cache', () => { @@ -38,3 +45,19 @@ test('same schema reuses cache', () => { true, ) }) + +test('assertValidJsonSchema compiles a valid schema without validating a value', () => { + expect(() => assertValidJsonSchema({ ...schema })).not.toThrow() +}) + +test('assertValidJsonSchema rejects an invalid schema', () => { + expect(() => + assertValidJsonSchema({ type: 'definitely-not-a-json-schema-type' }), + ).toThrow(/schema/i) +}) + +test('assertValidJsonSchema rejects async schemas unsupported by the synchronous validator', () => { + expect(() => assertValidJsonSchema({ $async: true, type: 'object' })).toThrow( + /async json schemas are not supported/i, + ) +}) diff --git a/packages/workflow-engine/src/__tests__/types.test.ts b/packages/workflow-engine/src/__tests__/types.test.ts index e1c7d9f94..f1ab410f9 100644 --- a/packages/workflow-engine/src/__tests__/types.test.ts +++ b/packages/workflow-engine/src/__tests__/types.test.ts @@ -1,4 +1,5 @@ import { expect, test } from 'bun:test' +import type { AgentRunResult } from '../types.js' // Directly construct type shapes to verify JSON round-trip (core requirement for resume persistence). test('AgentRunResult ok branch can JSON round-trip', () => { @@ -32,6 +33,17 @@ test('AgentRunResult dead with reason/detail can JSON round-trip', () => { expect(round.reason).toBe('no-structured-output') }) +test('AgentRunResult invalid-structured-output reason can JSON round-trip', () => { + const dead: AgentRunResult = { + kind: 'dead', + reason: 'invalid-structured-output', + detail: "must have required property 'count'", + } + const round = JSON.parse(JSON.stringify(dead)) + expect(round).toEqual(dead) + expect(round.reason).toBe('invalid-structured-output') +}) + // Backward compatible with old journals: reason/detail both optional, missing is still valid dead. test('AgentRunResult dead without reason is still valid (backward compatible with old journal)', () => { const legacy = { kind: 'dead' as const } diff --git a/packages/workflow-engine/src/engine/hooks.ts b/packages/workflow-engine/src/engine/hooks.ts index 1e1f380bd..0e9670337 100644 --- a/packages/workflow-engine/src/engine/hooks.ts +++ b/packages/workflow-engine/src/engine/hooks.ts @@ -10,6 +10,10 @@ import type { EngineContext } from './context.js' import { WorkflowAbortedError, WorkflowError } from './errors.js' import { agentCallKey } from './journal.js' import type { WorkflowHooks } from './script.js' +import { + assertValidJsonSchema, + validateAgainstSchema, +} from './structuredOutput.js' /** Sub-workflow executor for the workflow() hook (injected by runWorkflow to avoid circular dependencies). */ export type SubWorkflowRunner = (opts: { @@ -64,29 +68,49 @@ export function makeHooks( const agentId = r.agentIdSeq.value++ const params: AgentRunParams = { prompt, ...opts } + // Compile before consulting the journal or invoking a backend. An invalid schema is a workflow + // configuration error, not a transient agent failure, so it must fail directly without retry. + if (params.schema) assertValidJsonSchema(params.schema) const key = agentCallKey(prompt, params) const label = opts.label as string | undefined const phase = (opts.phase as string | undefined) ?? ctx.currentPhase ?? undefined - // Journal hit -> return cached result directly + const invalidateJournal = async (): Promise => { + ctx.journalInvalidated = true + ctx.journal = ctx.journal.slice(0, ctx.journalIndex) + await ctx.ports.journalStore.truncate(ctx.runId) + } + + // Journal hit -> return a still-valid cached result directly. Old journal entries predate the + // engine-level validation boundary, so validate structured output again before replaying it. if (!ctx.journalInvalidated && ctx.journalIndex < ctx.journal.length) { const entry = ctx.journal[ctx.journalIndex]! if (entry.key === key) { - ctx.journalIndex++ - emit({ - type: 'agent_done', - agentId, - label, - phase, - result: entry.result, - }) - return resultToOutput(entry.result) + const cachedResult = validateStructuredResult( + entry.result, + params.schema, + ) + if (entry.result.kind === 'ok' && cachedResult.kind === 'dead') { + ctx.ports.logger.warn?.( + `cached agent result for "${label ?? `#${agentId}`}" does not match its structured output schema; rerunning`, + ) + await invalidateJournal() + } else { + ctx.journalIndex++ + emit({ + type: 'agent_done', + agentId, + label, + phase, + result: entry.result, + }) + return resultToOutput(entry.result) + } + } else { + // Divergence: discard subsequent journal entries; everything from here on runs live + await invalidateJournal() } - // Divergence: discard subsequent journal entries; everything from here on runs live - ctx.journalInvalidated = true - ctx.journal = ctx.journal.slice(0, ctx.journalIndex) - await ctx.ports.journalStore.truncate(ctx.runId) } let release: () => void @@ -157,50 +181,69 @@ export function makeHooks( // resolve is outside the try: configuration errors (e.g. AdapterNotFoundError) propagate directly without retry — // this is a workflow configuration problem, not a transient backend failure; retrying is meaningless and would mask the bug. const adapter = registry ? registry.resolve(params) : null - const invokeBackend = (): Promise => - adapter + const invokeBackend = async (): Promise => { + const rawResult = adapter ? adapter.run(params, adapterCtx!) : ctx.ports.agentRunner.runAgentToResult(params, ctx.host) + return validateStructuredResult(await rawResult, params.schema) + } // Auto-retry once on failure: dead (terminal API error after retries) or a non-abort throw // both get one retry chance; WorkflowAbortedError (kill) is not retried — it is the user's intent. // If retry still fails: dead stays dead; a throw degrades to dead (one agent must not take down the workflow). // budget is not double-charged: dead does not call addOutputTokens; retry-ok charges once (at the final ok). - // dead.reason is passed through to the log: no-structured-output (the agent's final text block did not produce plain-object JSON) - // is a high-frequency cause of death; logging detail lets you immediately see what the agent last said. + // dead.reason is passed through to the log: no-structured-output and invalid-structured-output + // are high-frequency causes of death; logging detail makes the failed boundary visible immediately. // detail is wrapped with String() defensively: old journals or third-party adapters may write non-strings (corrupted data), // and calling .slice directly would throw a TypeError that pierces the logging path. + type BackendAttempt = + | { kind: 'result'; value: AgentRunResult } + | { kind: 'error'; error: unknown } + const attemptBackend = async (): Promise => { + try { + return { kind: 'result', value: await invokeBackend() } + } catch (error) { + if (error instanceof WorkflowAbortedError) throw error + return { kind: 'error', error } + } + } + + const first = await attemptBackend() let result: AgentRunResult - try { - result = await invokeBackend() - if (result.kind === 'dead') { - const detailStr = - typeof result.detail === 'string' ? result.detail : '' + if (first.kind === 'result' && first.value.kind !== 'dead') { + result = first.value + } else { + if (first.kind === 'error') { + const errorMessage = + first.error instanceof Error + ? first.error.message + : String(first.error) + ctx.ports.logger.warn?.( + `agent "${label ?? `#${agentId}`}" threw (${errorMessage}); retrying once`, + ) + } else if (first.value.kind === 'dead') { + const detail = + typeof first.value.detail === 'string' ? first.value.detail : '' ctx.ports.logger.warn?.( `agent "${label ?? `#${agentId}`}" returned dead` + - (result.reason ? ` (${result.reason})` : '') + - (detailStr ? `: ${detailStr.slice(0, 150)}` : '') + + (first.value.reason ? ` (${first.value.reason})` : '') + + (detail ? `: ${detail.slice(0, 150)}` : '') + '; retrying once', ) - result = await invokeBackend() - } - } catch (e) { - if (e instanceof WorkflowAbortedError) throw e - const eMsg = e instanceof Error ? e.message : String(e) - ctx.ports.logger.warn?.( - `agent "${label ?? `#${agentId}`}" threw (${eMsg}); retrying once`, - ) - try { - result = await invokeBackend() - } catch (e2) { - if (e2 instanceof WorkflowAbortedError) throw e2 - // Retry still threw: degrade to dead (keep the workflow going; hooks.agent returns null) - result = { - kind: 'dead', - reason: 'runagent-threw', - detail: e2 instanceof Error ? e2.message : String(e2), - } } + + const retry = await attemptBackend() + result = + retry.kind === 'result' + ? retry.value + : { + kind: 'dead', + reason: 'runagent-threw', + detail: + retry.error instanceof Error + ? retry.error.message + : String(retry.error), + } } if (result.kind === 'ok') { ctx.resources.budget.addOutputTokens(result.usage.outputTokens) @@ -298,3 +341,23 @@ export function makeHooks( function resultToOutput(result: AgentRunResult): unknown { return result.kind === 'ok' ? result.output : null } + +/** Enforce the caller-provided schema at the engine boundary for every adapter/runner implementation. */ +function validateStructuredResult( + result: AgentRunResult, + schema?: object, +): AgentRunResult { + if (!schema || result.kind !== 'ok') return result + + const { valid, errors } = validateAgainstSchema(result.output, schema) + if (valid) return result + + return { + kind: 'dead', + reason: 'invalid-structured-output', + detail: + errors.length > 0 + ? errors.join('; ') + : 'structured output does not match schema', + } +} diff --git a/packages/workflow-engine/src/engine/structuredOutput.ts b/packages/workflow-engine/src/engine/structuredOutput.ts index 6cb4abb9f..e2d283823 100644 --- a/packages/workflow-engine/src/engine/structuredOutput.ts +++ b/packages/workflow-engine/src/engine/structuredOutput.ts @@ -2,6 +2,26 @@ import { Ajv, type ValidateFunction } from 'ajv' const cache = new WeakMap() +function getValidator(schema: object): ValidateFunction { + const cached = cache.get(schema) + if (cached) return cached + + const ajv = new Ajv({ allErrors: true, strict: false }) + const validate = ajv.compile(schema) as ValidateFunction & { + $async?: boolean + } + if (validate.$async) { + throw new Error('Async JSON schemas are not supported') + } + cache.set(schema, validate) + return validate +} + +/** Compile a JSON Schema up front so configuration errors fail before journal replay or backend execution. */ +export function assertValidJsonSchema(schema: object): void { + getValidator(schema) +} + /** * Validate agent output against a JSON Schema (Ajv, compilation result cached by schema object). * The engine performs secondary validation on the schema result returned by the adapter, and uses it for tests. @@ -10,17 +30,15 @@ export function validateAgainstSchema( value: unknown, schema: object, ): { valid: boolean; errors: string[] } { - let validate = cache.get(schema) - if (!validate) { - const ajv = new Ajv({ allErrors: true, strict: false }) - validate = ajv.compile(schema) as ValidateFunction - cache.set(schema, validate) - } + const validate = getValidator(schema) const valid = validate(value) as boolean return { valid, errors: valid ? [] - : (validate.errors ?? []).map(e => e.message ?? 'validation error'), + : (validate.errors ?? []).map(e => { + const message = e.message ?? 'validation error' + return e.instancePath ? `${e.instancePath} ${message}` : message + }), } } diff --git a/packages/workflow-engine/src/index.ts b/packages/workflow-engine/src/index.ts index cb04ade31..962ddb26a 100644 --- a/packages/workflow-engine/src/index.ts +++ b/packages/workflow-engine/src/index.ts @@ -9,7 +9,7 @@ export * from './engine/concurrency.js' export * from './engine/script.js' export * from './engine/journal.js' export * from './engine/budget.js' -export * from './engine/structuredOutput.js' +export { validateAgainstSchema } from './engine/structuredOutput.js' export * from './engine/namedWorkflows.js' export * from './engine/errors.js' export * from './engine/context.js' diff --git a/packages/workflow-engine/src/types.ts b/packages/workflow-engine/src/types.ts index 638a87df7..63593cfdd 100644 --- a/packages/workflow-engine/src/types.ts +++ b/packages/workflow-engine/src/types.ts @@ -58,12 +58,14 @@ export type AgentRunResult = /** * Cause-of-death classification for log aggregation / post-hoc auditing. Optional for backward compatibility with old journals. * - no-structured-output: agent finished but finalize content has no StructuredOutput (neither called tools nor produced JSON in text) + * - invalid-structured-output: adapter returned structured output that did not match the caller-provided JSON Schema * - runagent-threw: runAgent threw a non-abort error (API failure / context overflow / runtime error) * - worktree-failed: isolation:'worktree' creation failed (fail-closed degradation) * - unknown: unclassified (compatible with old backends / third-party adapters) */ reason?: | 'no-structured-output' + | 'invalid-structured-output' | 'runagent-threw' | 'worktree-failed' | 'unknown'