Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/type-safety-audit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"@funkai/agents": patch
---

Fix type safety issues in agent lifecycle hooks and flow engine

- Remove unsafe generic hook forwarding from parent agents to sub-agents — only fixed-type hooks (`onStepStart`, `onStepFinish`) are forwarded; generic hooks (`onStart`, `onFinish`, `onError`) stay at the parent level where their `TInput`/`TOutput` types are correct
- Wrap `buildMergedHook` in `fireHooks` for error protection — merged hooks now swallow errors like all other hooks
- Fix config spread ordering in flow agent steps — framework fields (`input`, `signal`, `logger`) can no longer be overwritten by user config
- Thread `TOutput` through `BaseGenerateParams` so `onFinish` hooks receive `GenerateResult<TOutput>` instead of untyped `GenerateResult`
- Fix `AnyHook` contravariance in flow engine — use properly documented `any` escape hatch for internal hook merging
59 changes: 59 additions & 0 deletions packages/agents/docs/core/hooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,65 @@ base.onStepFinish -> overrides.onStepFinish

The `stepId` for agent tool-loop steps is counter-based: `agentName:0`, `agentName:1`, etc.

## Sub-Agent Hook Forwarding

When a parent agent has sub-agents (via the `agents` config), those sub-agents are wrapped as tools. The parent forwards a subset of its hooks to each sub-agent so internal activity is observable.

### What gets forwarded (safe — fixed event types)

| Hook | Event type | Why safe |
| -------------- | ----------------- | -------------------------------------- |
| `onStepStart` | `StepInfo` | Fixed type, same shape for every agent |
| `onStepFinish` | `StepFinishEvent` | Fixed type, same shape for every agent |
| `logger` | `Logger` | No event type, just a logger instance |

These hooks are passed directly into `child.generate()` as per-call hooks. The parent's `onStepFinish` is merged (config + per-call) before forwarding, so both the config-level and call-level hooks fire for sub-agent steps.

### What stays at the parent (not forwarded — generic event types)

| Hook | Event type | Why not forwarded |
| ---------- | -------------------------------------------------------------- | ----------------------------------------- |
| `onStart` | `{ input: TInput }` | `TInput` differs between parent and child |
| `onFinish` | `{ input: TInput, result: GenerateResult<TOutput>, duration }` | Both `TInput` and `TOutput` differ |
| `onError` | `{ input: TInput, error: Error }` | `TInput` differs between parent and child |

These hooks are parameterized by the agent's generic types (`TInput`, `TOutput`). A parent typed `Agent<{ userId: string }>` would have `onStart: (e: { input: { userId: string } }) => void`, but a sub-agent might expect `{ query: string }`. Forwarding the parent's hook to the child would cause the hook to receive the wrong event shape at runtime — the compiler cannot catch this because the type boundary is erased when hooks cross agent boundaries.

Sub-agent lifecycle activity is still observable at the parent level through `onStepFinish`, which fires for each tool-loop step including sub-agent tool calls and their results.

### Lifecycle diagram

```
Parent.generate({ input, onStepFinish })
├── Parent fires own onStart({ input }) ← parent's TInput, type-safe
├── generateText() runs tool loop
│ │
│ ├── Step 0: LLM calls sub-agent tool
│ │ │
│ │ │ Passed into child.generate():
│ │ │ logger → parent's logger
│ │ │ onStepStart → parent's onStepStart (StepInfo — fixed type)
│ │ │ onStepFinish → parent's merged onStepFinish (StepFinishEvent — fixed type)
│ │ │
│ │ │ NOT passed:
│ │ │ onStart, onFinish, onError (generic types — would break type safety)
│ │ │
│ │ ├── Child fires own onStart({ input }) ← child's TInput, type-safe
│ │ ├── Child runs tool loop
│ │ │ ├── Child step 0 → parent's onStepFinish fires (StepFinishEvent)
│ │ │ └── Child step 1 → parent's onStepFinish fires (StepFinishEvent)
│ │ ├── Child fires own onFinish(...) ← child's types, type-safe
│ │ └── Returns result to parent
│ │
│ └── Parent's onStepFinish fires for step 0 (includes sub-agent tool result)
├── Parent fires own onFinish({ input, result }) ← parent's TInput/TOutput, type-safe
└── Returns Result to caller
```

## Error Handling

All hooks are executed via `attemptEachAsync`, which:
Expand Down
50 changes: 43 additions & 7 deletions packages/agents/src/core/agents/base/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
buildPrompt,
toTokenUsage,
} from "@/core/agents/base/utils.js";
import type { ParentAgentContext } from "@/core/agents/base/utils.js";
import type {
Agent,
AgentConfig,
Expand Down Expand Up @@ -99,7 +100,7 @@ export function agent<
*
* @private
*/
function extractInput(params: GenerateParams<TInput, TTools, TSubAgents>): TInput {
function extractInput(params: GenerateParams<TInput, TTools, TSubAgents, TOutput>): TInput {
if (Object.hasOwn(params, "prompt") && !isNil(params.prompt)) {
return params.prompt as unknown as TInput;
}
Expand Down Expand Up @@ -171,7 +172,7 @@ export function agent<
* @private
*/
function resolveSignal(
params: GenerateParams<TInput, TTools, TSubAgents>,
params: GenerateParams<TInput, TTools, TSubAgents, TOutput>,
): AbortSignal | undefined {
const { timeout, signal } = params;
if (signal && isNotNil(timeout)) {
Expand All @@ -197,7 +198,7 @@ export function agent<
async function prepareGeneration(
input: TInput,
log: Logger,
params: GenerateParams<TInput, TTools, TSubAgents>,
params: GenerateParams<TInput, TTools, TSubAgents, TOutput>,
): Promise<PreparedGeneration> {
const resolvedModel = params.model ?? (await resolveValue(config.model, input));
const model = await withModelMiddleware({ model: resolvedModel });
Expand All @@ -210,9 +211,23 @@ export function agent<
const hasTools = Object.keys(mergedTools).length > 0;
const hasAgents = Object.keys(mergedAgents).length > 0;

// Only fixed-type hooks (onStepStart, onStepFinish) are forwarded to
// Sub-agents. Generic hooks (onStart, onFinish, onError) are NOT
// Forwarded because their event types are parameterized by TInput/TOutput
// — a sub-agent has different generics, so the parent's typed hook
// Would receive the wrong event shape at runtime. Sub-agent activity
// Is still observable via onStepFinish at the parent's tool-loop level.
// See packages/agents/docs/core/hooks.md for the full lifecycle.
const parentCtx: ParentAgentContext = {
log,
onStepStart: params.onStepStart,
onStepFinish: buildMergedHook(log, config.onStepFinish, params.onStepFinish),
};

const aiTools = buildAITools(
valueOrUndefined(hasTools, mergedTools),
valueOrUndefined(hasAgents, mergedAgents),
parentCtx,
);

// eslint-disable-next-line @typescript-eslint/no-explicit-any -- params.system is Resolver-shaped; safe to resolve
Expand Down Expand Up @@ -269,7 +284,7 @@ export function agent<
}

async function generate(
params: GenerateParams<TInput, TTools, TSubAgents>,
params: GenerateParams<TInput, TTools, TSubAgents, TOutput>,
): Promise<Result<GenerateResult<TOutput>>> {
const startedAt = Date.now();
let resolvedInput: TInput | undefined;
Expand Down Expand Up @@ -329,7 +344,7 @@ export function agent<
wrapHook(config.onFinish, { input, result: generateResult, duration }),
wrapHook(params.onFinish, {
input,
result: generateResult as GenerateResult,
result: generateResult,
duration,
}),
);
Expand Down Expand Up @@ -368,7 +383,7 @@ export function agent<
}

async function stream(
params: GenerateParams<TInput, TTools, TSubAgents>,
params: GenerateParams<TInput, TTools, TSubAgents, TOutput>,
): Promise<Result<StreamResult<TOutput>>> {
const startedAt = Date.now();
let resolvedInput: TInput | undefined;
Expand Down Expand Up @@ -454,7 +469,7 @@ export function agent<
wrapHook(config.onFinish, { input, result: generateResult, duration }),
wrapHook(params.onFinish, {
input,
result: generateResult as GenerateResult,
result: generateResult,
duration,
}),
);
Expand Down Expand Up @@ -658,3 +673,24 @@ function pickByOutput<T>(output: unknown, ifOutput: T, ifText: T): T {
}
return ifText;
}

/**
* Build a merged hook that fires config-level and per-call hooks sequentially.
*
* Returns `undefined` when both are absent so `buildParentParams` skips
* the field entirely and sub-agent defaults are preserved.
*
* @private
*/
function buildMergedHook<E>(
log: Logger,
configHook: ((event: E) => void | Promise<void>) | undefined,
callHook: ((event: E) => void | Promise<void>) | undefined,
): ((event: E) => void | Promise<void>) | undefined {
if (isNil(configHook) && isNil(callHook)) {
return undefined;
}
return async (event: E) => {
await fireHooks(log, wrapHook(configHook, event), wrapHook(callHook, event));
};
}
104 changes: 101 additions & 3 deletions packages/agents/src/core/agents/base/utils.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,75 @@
import type { LanguageModelUsage } from "ai";
import { tool } from "ai";
import { isFunction, isNil, isNotNil, isString } from "es-toolkit";
import { isFunction, isNil, isNotNil, isString, omitBy } from "es-toolkit";
import { match, P } from "ts-pattern";
import type { ZodType } from "zod";
import { z } from "zod";

import type { Agent, Message, Resolver } from "@/core/agents/types.js";
import type { Logger } from "@/core/logger.js";
import type { TokenUsage } from "@/core/provider/types.js";
import type { Tool } from "@/core/tool.js";
import type { StepFinishEvent, StepInfo } from "@/core/types.js";
import { RUNNABLE_META } from "@/lib/runnable.js";
import type { RunnableMeta } from "@/lib/runnable.js";

/**
* Context forwarded from a parent agent to sub-agents wrapped as tools.
*
* Includes the parent's logger and **fixed-type** lifecycle hooks so that
* sub-agent internal step activity is visible to the parent.
*
* ## Why only step hooks are forwarded
*
* `onStepStart` and `onStepFinish` use fixed event types (`StepInfo`,
* `StepFinishEvent`) that are the same for every agent — safe to pass
* from parent to child with no type mismatch.
*
* `onStart`, `onFinish`, and `onError` are **not** forwarded because their
* event types are generic over `TInput`/`TOutput`. A parent agent typed
* `Agent<{ userId: string }, ...>` would have `onStart: (e: { input: { userId: string } }) => void`,
* but the sub-agent's input is a completely different type (e.g.
* `{ query: string }`). Forwarding the parent's hook to the child would
* cause the hook to receive the wrong event shape at runtime — a silent
* type-safety violation that the compiler cannot catch.
*
* Sub-agent activity is still observable at the parent level through
* `onStepFinish`, which fires for each tool-loop step including sub-agent
* tool calls.
*
* ## Lifecycle (what gets passed down vs. what stays at the parent)
*
* ```
* Passed into child.generate() — fixed types, safe:
* log → child creates .child({ agentId }) from it
* onStepStart → StepInfo (same shape for all agents)
* onStepFinish → StepFinishEvent (same shape for all agents)
*
* NOT passed down — generic types, would break type safety:
* onStart → { input: TInput } (differs per agent)
* onFinish → { input: TInput, result: GenerateResult<TOutput> } (differs per agent)
* onError → { input: TInput, error: Error } (differs per agent)
* ```
*/
export interface ParentAgentContext {
/** Parent logger — sub-agent creates `.child({ agentId })` from it. */
log?: Logger;

/**
* Fires when a sub-agent step starts.
*
* Uses `StepInfo` — a fixed (non-generic) type, safe to forward.
*/
onStepStart?: (event: { step: StepInfo }) => void | Promise<void>;

/**
* Fires when a sub-agent step finishes.
*
* Uses `StepFinishEvent` — a fixed (non-generic) type, safe to forward.
*/
onStepFinish?: (event: StepFinishEvent) => void | Promise<void>;
}

/**
* Merge `Tool` records and wrap subagent `Runnable` objects into AI SDK
* tool format for `generateText` / `streamText`.
Expand All @@ -24,12 +83,15 @@ import type { RunnableMeta } from "@/lib/runnable.js";
*
* @param tools - Record of named tools to include.
* @param agents - Record of named sub-agents to wrap as tools.
* @param parentCtx - Parent context forwarded to sub-agent generate calls
* (logger, lifecycle hooks).
* @returns The merged tool set, or `undefined` when empty.
*/
export function buildAITools(
tools?: Record<string, Tool>,
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Agent generic params are contravariant; `unknown` breaks assignability
agents?: Record<string, Agent<any, any, any, any, any>>,
parentCtx?: ParentAgentContext,
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- ToolSet requires `any` values; `unknown` breaks assignability with AI SDK
): Record<string, any> | undefined {
const hasTools = isNotNil(tools) && Object.keys(tools).length > 0;
Expand All @@ -39,7 +101,7 @@ export function buildAITools(
return undefined;
}

const agentTools: Record<string, unknown> = buildAgentTools(agents, tools);
const agentTools: Record<string, unknown> = buildAgentTools(agents, tools, parentCtx);

return { ...tools, ...agentTools };
}
Expand Down Expand Up @@ -253,6 +315,7 @@ function buildAgentTools(
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Agent generic params are contravariant; `unknown` breaks assignability
agents: Record<string, Agent<any, any, any, any, any>> | undefined,
tools: Record<string, Tool> | undefined,
parentCtx: ParentAgentContext | undefined,
): Record<string, unknown> {
if (!agents) {
return {};
Expand All @@ -279,6 +342,7 @@ function buildAgentTools(
meta,
toolName,
tools,
parentCtx,
);

return [agentToolName, agentTool];
Expand All @@ -298,14 +362,22 @@ function buildAgentTool(
meta: RunnableMeta | undefined,
toolName: string,
tools: Record<string, Tool> | undefined,
parentCtx: ParentAgentContext | undefined,
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- ToolSet requires `any` values; `unknown` breaks assignability with AI SDK
): ReturnType<typeof tool<any, any>> {
const parentParams = buildParentParams(parentCtx);

if (isNotNil(meta) && isNotNil(meta.inputSchema)) {
return tool({
description: `Delegate to ${toolName}`,
inputSchema: meta.inputSchema,
execute: async (input, { abortSignal }) => {
const r = await runnable.generate({ input, signal: abortSignal, tools });
const r = await runnable.generate({
input,
signal: abortSignal,
tools,
...parentParams,
});
if (!r.ok) {
throw new Error(r.error.message);
}
Expand All @@ -321,6 +393,7 @@ function buildAgentTool(
prompt: input.prompt,
signal: abortSignal,
tools,
...parentParams,
});
if (!r.ok) {
throw new Error(r.error.message);
Expand All @@ -329,3 +402,28 @@ function buildAgentTool(
},
});
}

/**
* Build the per-call params to forward from parent context to sub-agent.
*
* Only forwards the parent logger and **fixed-type** step hooks.
* Generic hooks (`onStart`, `onFinish`, `onError`) are intentionally
* excluded — see {@link ParentAgentContext} for the rationale.
*
* Omits `undefined` values so they don't override sub-agent defaults.
*
* @private
*/
function buildParentParams(ctx: ParentAgentContext | undefined): Record<string, unknown> {
if (isNil(ctx)) {
return {};
}
return omitBy(
{
logger: ctx.log,
onStepStart: ctx.onStepStart,
onStepFinish: ctx.onStepFinish,
},
isNil,
);
}
Loading