fix(agents): type safety audit for lifecycle hooks and flow engine

[zrosenbauer]

Summary

• Remove unsafe generic hook forwarding — parent agents no longer forward onStart/onFinish/onError to sub-agents. These hooks are parameterized by TInput/TOutput which differ between parent and child, causing silent type-safety violations at runtime. Only fixed-type hooks (onStepStart, onStepFinish) are forwarded.
• Wrap buildMergedHook in fireHooks — merged hooks now swallow errors like all other hooks in the system, preventing hook failures from crashing agents.
• Fix config spread ordering — in flow agent steps, framework fields (input, signal, logger) can no longer be overwritten by user config.config spread.
• Thread TOutput through BaseGenerateParams — onFinish hooks now receive GenerateResult<TOutput> instead of untyped GenerateResult, enabling type-safe result inspection.
• Fix AnyHook contravariance — resolve 5 typecheck errors in flow engine by using a properly documented any escape hatch for the internal hook merge utility (function parameter contravariance makes strict typing impossible here).

Test plan

• All 656 existing tests pass
• Typecheck passes clean (pnpm typecheck --filter=@funkai/agents)
• Integration tests updated to assert new hook forwarding behavior
• Changeset included for patch release

[changeset-bot]

🦋 Changeset detected

Latest commit: ecdcd62

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@funkai/agents	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
funkai	Error		Mar 21, 2026 6:28am

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c158dee9-31c7-405e-9d90-0fb63c706056

📥 Commits

Reviewing files that changed from the base of the PR and between 3c8fba3 and ecdcd62.

📒 Files selected for processing (7)

• packages/agents/docs/core/hooks.md
• packages/agents/src/core/agents/base/agent.ts
• packages/agents/src/core/agents/base/utils.ts
• packages/agents/src/core/agents/flow/engine.ts
• packages/agents/src/core/agents/flow/types.ts
• packages/agents/src/core/agents/types.ts
• packages/agents/src/integration/lifecycle.test.ts

---

📝 Walkthrough

Walkthrough

Threads TOutput through agent params, adds ParentAgentContext to forward only fixed-typed lifecycle hooks and logger to sub-agents, moves merged-hook invocation into the shared fire path, and makes framework-provided fields (input, signal, logger) take precedence when building sub-agent params.

Changes

Cohort / File(s)	Summary
Types: GenerateParams / BaseGenerateParams packages/agents/src/core/agents/types.ts, packages/agents/src/core/agents/flow/types.ts	Add TOutput to BaseGenerateParams/GenerateParams; thread TOutput into onFinish and into public Agent/FlowAgent method signatures so per-call params carry output typing.
Parent Context & Hook Forwarding packages/agents/src/core/agents/base/utils.ts, packages/agents/src/core/agents/base/agent.ts	Introduce exported ParentAgentContext (logger, onStepStart, onStepFinish); extend buildAITools to accept parentCtx; add buildParentParams and forward only logger, onStepStart, onStepFinish to sub-agents; add buildMergedHook and wire merged onStepFinish into runtime firing path.
Flow Engine Hook Merging packages/agents/src/core/agents/flow/engine.ts	Replace previous generic merged-hook typing with internal AnyHook to unify differing hook shapes for internal merging, avoiding unsafe casts.
Flow Step Params & Signal Precedence packages/agents/src/core/agents/flow/steps/factory.ts, packages/agents/src/core/agents/flow/steps/agent.test.ts, packages/agents/src/core/agents/flow/steps/factory.test.ts	Reorder agentParams so ...config.config is spread first; explicitly set input, signal: ctx.signal, logger afterward so framework ctx.signal/logger override user config. Update tests to expect ctx.signal precedence.
Docs: Hook Forwarding packages/agents/docs/core/hooks.md	Add “Sub-Agent Hook Forwarding” section documenting that only fixed-shaped per-call items (onStepStart, onStepFinish, logger) are forwarded; generic hooks (onStart, onFinish, onError) remain at parent level.
Integration Tests packages/agents/src/integration/lifecycle.test.ts	Add end-to-end suite validating lifecycle hook propagation, merge ordering, signal/logger forwarding, streaming behavior, error vs finish semantics, and step-index ordering across nested flows and sub-agents.
Changeset .changeset/type-safety-audit.md	Document runtime hook handling and type-safety changes introduced in this patch.

Sequence Diagram

sequenceDiagram
    participant ParentAgent as Parent Agent (TInput,TOutput)
    participant Config as Config/Per-Call
    participant ParentCtx as ParentAgentContext
    participant SubAgent as Sub-Agent Tool (TInput',TOutput')
    participant Execution as Sub-Agent Execution

    ParentAgent->>Config: assemble hooks, logger, signal
    Config->>ParentCtx: build ParentAgentContext (logger, onStepStart, merged onStepFinish)
    ParentCtx->>SubAgent: forward logger, onStepStart, onStepFinish, signal (ctx.signal)
    Note over ParentCtx: drop generic hooks: onStart/onFinish/onError
    SubAgent->>Execution: generate(params with forwarded parent fields)
    Execution->>SubAgent: fire onStepStart -> run step -> fire onStepFinish
    SubAgent-->>ParentAgent: return result
    ParentAgent->>ParentAgent: handle parent onFinish/onError (typed to parent's TOutput)

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• fix(agents): type safety audit for lifecycle hooks and flow engine #46 — Strong overlap: threads TOutput, adds ParentAgentContext and enforces onStepStart/onStepFinish forwarding and merged-hook behavior.
• refactor(packages/agents): extract shared generate/stream setup #13 — Related changes to prepareGeneration / agent preparation and hook wiring that intersect with this PR.
• fix(agents): replace colon in sub-agent tool names with underscore #26 — Related modifications to buildAITools / base utils; both PRs touch parent-context/tool wiring.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.87% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main objective: fixing type safety issues in lifecycle hooks and the flow engine, which is the core purpose of the changeset.
Description check	✅ Passed	The description is directly related to the changeset, providing clear bullet points covering all major changes including hook forwarding, error handling, config ordering, type threading, and contravariance fixes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/agents/src/core/agents/types.ts (1)

300-345: ⚠️ Potential issue | 🟠 Major

Per-call output overrides make onFinish type-unsafe.

BaseGenerateParams types onFinish.result as GenerateResult<TOutput>, but GenerateParams allows output?: OutputParam to change the runtime result shape without updating the hook's type. An agent created with TOutput = string can call .generate({ output: Output.object({ schema }), onFinish: ({ result }) => result.output.foo }) and typecheck even though result.output is a string at the type-checker's understanding but an object at runtime.

The as TOutput cast on line 336 prevents TypeScript from catching this. Either make output override drive the hook's GenerateResult type, or disallow output overrides on the typed path.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/agents/src/core/agents/types.ts` around lines 300 - 345, The
onFinish hook's result type is unsound because BaseGenerateParams<TOutput> fixes
GenerateResult<TOutput> while GenerateParams allows a runtime output override
(OutputParam) that changes the actual result shape; fix by making the output
override drive the hook type or forbidding overrides when a concrete TOutput is
used — e.g. update GenerateParams to be generic over an OutputParam and derive
TOutputFromOutputParam to compute the correct GenerateResult type for onFinish
(replace the current unsafe `as TOutput` cast in BaseGenerateParams.onFinish),
or alternatively disallow the `output` property on the typed path (make
`output?: never` when TOutput is explicitly provided) and remove the cast so
TypeScript enforces consistency; refer to BaseGenerateParams, GenerateParams,
GenerateResult, OutputParam, and the onFinish declaration to locate and apply
this change.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/agents/docs/core/hooks.md`:
- Around line 109-138: The fenced diagram block starting with "Parent.generate({
input, onStepFinish })" in packages/agents/docs/core/hooks.md is missing a
language specifier; update the opening triple-backtick to include a language tag
such as "text" or "plaintext" (e.g., ```text) so the diagram block is properly
marked for markdownlint and consistent rendering.

In `@packages/agents/src/integration/lifecycle.test.ts`:
- Line 332: Replace the ternary expressions in this test (e.g., the expression
"return r.ok ? r.value : 0;" that uses variable r) with an if/else or
early-return style to satisfy the no-ternary rule; for example, check r.ok and
return r.value when true, otherwise return 0. Do the same for the other reported
locations in the file (around the occurrences at the ranges noted in the review)
so all ternaries are converted to explicit if/else or early returns.

---

Outside diff comments:
In `@packages/agents/src/core/agents/types.ts`:
- Around line 300-345: The onFinish hook's result type is unsound because
BaseGenerateParams<TOutput> fixes GenerateResult<TOutput> while GenerateParams
allows a runtime output override (OutputParam) that changes the actual result
shape; fix by making the output override drive the hook type or forbidding
overrides when a concrete TOutput is used — e.g. update GenerateParams to be
generic over an OutputParam and derive TOutputFromOutputParam to compute the
correct GenerateResult type for onFinish (replace the current unsafe `as
TOutput` cast in BaseGenerateParams.onFinish), or alternatively disallow the
`output` property on the typed path (make `output?: never` when TOutput is
explicitly provided) and remove the cast so TypeScript enforces consistency;
refer to BaseGenerateParams, GenerateParams, GenerateResult, OutputParam, and
the onFinish declaration to locate and apply this change.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f8af9c5c-63db-42da-930c-771a16f3840c

📥 Commits

Reviewing files that changed from the base of the PR and between abb42ba and b26cc21.

📒 Files selected for processing (11)

• .changeset/type-safety-audit.md
• packages/agents/docs/core/hooks.md
• packages/agents/src/core/agents/base/agent.ts
• packages/agents/src/core/agents/base/utils.ts
• packages/agents/src/core/agents/flow/engine.ts
• packages/agents/src/core/agents/flow/steps/agent.test.ts
• packages/agents/src/core/agents/flow/steps/factory.test.ts
• packages/agents/src/core/agents/flow/steps/factory.ts
• packages/agents/src/core/agents/flow/types.ts
• packages/agents/src/core/agents/types.ts
• packages/agents/src/integration/lifecycle.test.ts