feat(agents): add agentChain propagation and fix flow agent step hook forwarding#50
Conversation
… forwarding - Add AgentChainEntry type and agentChain field to StepInfo and StepFinishEvent for agent ancestry tracking across nested agents - Forward onStepStart/onStepFinish hooks from flow agent $.agent() to sub-agents, enabling full observability of nested agent steps - Thread agentChain internally through ParentAgentContext and StepBuilderOptions without exposing it on public GenerateParams - Add integration tests for chain propagation and hook forwarding Co-Authored-By: Claude Code <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: db8909e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an Changes
Sequence Diagram(s)sequenceDiagram
participant Root as Root Caller
participant Flow as Flow Agent
participant Builder as Step Builder
participant Sub as Sub-Agent
participant Emitter as Hook/Event Emitter
Root->>Flow: generate(params)
Flow->>Flow: extractAgentChain(params) -> incomingChain
Flow->>Flow: currentChain = incomingChain + {id: flow}
Flow->>Builder: create step builder (agentChain=currentChain)
Builder->>Sub: call $.agent(sub) with merged hooks + agentChain=currentChain
Sub->>Sub: extractAgentChain(params) -> incomingChain
Sub->>Sub: currentChain = incomingChain + {id: sub}
Sub->>Emitter: emit onStepStart/onStepFinish (agentChain=currentChain)
Emitter-->>Flow: parent hooks receive sub-agent events (with extended chain)
Flow->>Emitter: emit flow-level onStepFinish (agentChain=[{id: flow}])
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Move `extractAgentChain` from agent.ts and flow-agent.ts into base/utils.ts to eliminate duplication. Use a shared empty array constant to avoid per-call allocations for top-level agents. Co-Authored-By: Claude Code <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/src/core/types.ts`:
- Around line 175-185: Update the agentChain JSDoc to match emitted events:
explain that agentChain is a readonly array of AgentChainEntry representing the
ancestry from root to the producing agent and that even
top-level/directly-invoked agents will have a single-entry array (e.g., [{ id:
"<agent-name>" }]) because the base agent code appends { id: config.name },
rather than being undefined; reference the agentChain and AgentChainEntry types
and the base agent behavior when making this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 65a4ad69-5fbf-4cbf-bed1-2bf395a2bf23
📒 Files selected for processing (8)
.changeset/agent-chain-propagation.mdpackages/agents/src/core/agents/base/agent.tspackages/agents/src/core/agents/base/utils.tspackages/agents/src/core/agents/flow/flow-agent.tspackages/agents/src/core/agents/flow/steps/factory.tspackages/agents/src/core/types.tspackages/agents/src/index.tspackages/agents/src/integration/lifecycle.test.ts
Replace optional chaining and ternaries (disallowed by oxlint) with explicit nil checks via resolveParentHooks helper. Capitalize continuation comments. Co-Authored-By: Claude Code <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/src/core/agents/base/utils.ts`:
- Around line 432-443: Add an `@example` block to the JSDoc for the exported
function extractAgentChain showing typical usage: demonstrate calling
extractAgentChain(params) where params is a raw GenerateParams-like object that
may include an internal agentChain field and show the expected returned readonly
AgentChainEntry[] (or empty array) result; keep the example minimal, use a small
literal object (e.g., { agentChain: [...] }) to illustrate both presence and
absence cases, and ensure the tag appears alongside the existing `@param` and
`@returns` entries in the extractAgentChain comment.
In `@packages/agents/src/core/agents/flow/steps/factory.ts`:
- Around line 288-297: The forwarded parent hooks are overwriting
delegated-agent hooks because agentParams spreads ...config.config then
...forwardedHooks from resolveParentHooks(), which also returns keys even when
undefined; update the assembly in agentParams used by generate() and stream()
(and the similar block around the 603-623 region) to merge hook callbacks
instead of replacing them and only include defined hook properties: for example,
compute mergedOnStepStart and mergedOnStepFinish that call delegated hook then
parent hook (or vice versa) when both exist, and spread only the resulting
defined callbacks into agentParams; ensure resolveParentHooks() usage does not
inject undefined keys that clobber child-only hooks and that $.agent() receives
the merged callbacks rather than the plain forwardedHooks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 83c5bde8-05c0-45cf-817a-0b5256386000
📒 Files selected for processing (4)
packages/agents/src/core/agents/base/agent.tspackages/agents/src/core/agents/base/utils.tspackages/agents/src/core/agents/flow/flow-agent.tspackages/agents/src/core/agents/flow/steps/factory.ts
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/agents/src/core/agents/flow/steps/factory.ts (1)
291-300:⚠️ Potential issue | 🟠 MajorMerge delegated step hooks instead of overwriting them.
Line 298 spreads
forwardedHooksafterconfig.config, so any delegatedconfig.config.onStepStart/onStepFinishis replaced. Lines 623-624 also always materialize both keys, which means a parent that only definesonStepStartstill clears a child-onlyonStepFinishby overwriting it withundefined. That regresses$.agent()whenever local step hooks and forwarded parent hooks are used together.Please merge the callbacks and only include defined hook keys in the forwarded object.
Also applies to: 616-625
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agents/src/core/agents/flow/steps/factory.ts` around lines 291 - 300, The code currently spreads forwardedHooks after config.config into agentParams which overwrites any local step hooks (e.g., config.config.onStepStart / onStepFinish) and also always materializes both hook keys, clearing child-only hooks; change resolveParentHooks/forwardedHooks handling so you merge hook callbacks instead of overwriting and only add defined keys: when building agentParams, detect onStepStart and onStepFinish on both config.config and forwardedHooks and create composed functions (call parent then child or vice‑versa as intended) for each key that exists on either side, and only include those keys in the forwarded object passed into agentParams (do not set keys to undefined). Ensure the same merge logic is applied wherever resolveParentHooks was previously materializing both hook keys (e.g., the other occurrence that currently always sets both onStepStart/onStepFinish).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/agents/src/core/agents/flow/steps/factory.ts`:
- Around line 291-300: The code currently spreads forwardedHooks after
config.config into agentParams which overwrites any local step hooks (e.g.,
config.config.onStepStart / onStepFinish) and also always materializes both hook
keys, clearing child-only hooks; change resolveParentHooks/forwardedHooks
handling so you merge hook callbacks instead of overwriting and only add defined
keys: when building agentParams, detect onStepStart and onStepFinish on both
config.config and forwardedHooks and create composed functions (call parent then
child or vice‑versa as intended) for each key that exists on either side, and
only include those keys in the forwarded object passed into agentParams (do not
set keys to undefined). Ensure the same merge logic is applied wherever
resolveParentHooks was previously materializing both hook keys (e.g., the other
occurrence that currently always sets both onStepStart/onStepFinish).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4dae7391-0497-4422-b61b-98fc6c554025
📒 Files selected for processing (3)
packages/agents/src/core/agents/base/agent.tspackages/agents/src/core/agents/flow/steps/factory.tspackages/agents/src/integration/lifecycle.test.ts
- Fix agentChain JSDoc in types.ts to reflect that top-level agents get a single-entry chain (not undefined) - Add @example tag to extractAgentChain export in utils.ts - Replace resolveParentHooks with mergeStepHooks in factory.ts to merge parent + child step hooks instead of clobbering, and only include defined hook keys in the spread Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
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/flow/steps/factory.ts (1)
134-145:⚠️ Potential issue | 🟠 MajorClone
agentChainbefore exposing it to hooks or delegated agents.
agentChainis reused by reference inStepInfo/StepFinishEventand then forwarded unchanged into$.agent(). That makes ancestry tracking mutable across the whole run: one in-place change toevent.step.agentChainor anAgentChainEntry.idinside a hook will bleed into later step events and descendant agents.Suggested fix
+function cloneAgentChain( + chain: readonly AgentChainEntry[] | undefined, +): readonly AgentChainEntry[] | undefined { + if (isNil(chain)) { + return undefined; + } + return chain.map((entry) => ({ ...entry })); +} + async function executeStep<T>(params: { id: string; type: OperationType; execute: (args: { $: StepBuilder }) => Promise<T>; @@ - const stepInfo: StepInfo = { id, index: indexRef.current++, type, agentChain }; + const stepAgentChain = cloneAgentChain(agentChain); + const stepInfo: StepInfo = { + id, + index: indexRef.current++, + type, + agentChain: stepAgentChain, + }; @@ - const parentOnStepFinishHook = buildParentHookCallback(parentHooks, "onStepFinish", (fn) => - fn({ step: stepInfo, result: value, duration, agentChain }), - ); + const parentOnStepFinishHook = buildParentHookCallback(parentHooks, "onStepFinish", (fn) => + fn({ step: stepInfo, result: value, duration, agentChain: stepAgentChain }), + ); @@ - const parentOnStepFinishHook = buildParentHookCallback(parentHooks, "onStepFinish", (fn) => - fn({ step: stepInfo, result: undefined, duration, agentChain }), - ); + const parentOnStepFinishHook = buildParentHookCallback(parentHooks, "onStepFinish", (fn) => + fn({ step: stepInfo, result: undefined, duration, agentChain: stepAgentChain }), + ); @@ const agentParams = { ...config.config, input: config.input, signal: ctx.signal, logger: ctx.log.child({ stepId: config.id }), ...mergedHooks, - agentChain, + agentChain: cloneAgentChain(agentChain), };As per coding guidelines, "Immutable data only — no in-place mutation of shared state or function arguments."
Also applies to: 219-220, 272-273, 293-300
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agents/src/core/agents/flow/steps/factory.ts` around lines 134 - 145, The agentChain array is being passed by reference into StepInfo/StepFinishEvent and into delegated builders (e.g. createStepBuilderInternal and $.agent()), making ancestry mutable; before creating StepInfo, childCtx, or calling createStepBuilderInternal / $.agent(), create a shallow copy of agentChain (e.g., [...agentChain]) and use that copy when assigning to StepInfo.agentChain, StepFinishEvent.agentChain, childCtx and when passing into createStepBuilderInternal or $.agent() so hooks and descendant agents receive an immutable snapshot of the chain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/agents/src/core/agents/flow/steps/factory.ts`:
- Around line 134-145: The agentChain array is being passed by reference into
StepInfo/StepFinishEvent and into delegated builders (e.g.
createStepBuilderInternal and $.agent()), making ancestry mutable; before
creating StepInfo, childCtx, or calling createStepBuilderInternal / $.agent(),
create a shallow copy of agentChain (e.g., [...agentChain]) and use that copy
when assigning to StepInfo.agentChain, StepFinishEvent.agentChain, childCtx and
when passing into createStepBuilderInternal or $.agent() so hooks and descendant
agents receive an immutable snapshot of the chain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e8dbddd3-3de2-4761-91c7-f5cf70cd95ce
📒 Files selected for processing (1)
packages/agents/src/core/agents/flow/steps/factory.ts
- Add @funkai/utils package (private, workspace-only) with privateField utility for Symbol-keyed non-enumerable fields on plain objects - Migrate agentChain transport from string key to Symbol-keyed private field using _agentChainField, making it invisible to Object.keys(), JSON.stringify(), for...in, and object spread - Update extractAgentChain to read via private field accessor - Update buildAgentTool and factory.ts to stamp private field after spread (Symbol fields don't survive spread) Co-Authored-By: Claude <noreply@anthropic.com>
Simplify the privateField accessor — drop Object.freeze() on the returned accessor and remove the remove() method since neither is needed for the current use case. Co-Authored-By: Claude <noreply@anthropic.com>
Summary
AgentChainEntrytype andagentChainfield toStepInfoandStepFinishEvent— provides full agent ancestry tracking (e.g.,[{ id: "pipeline" }, { id: "writer" }]) on every step hook event$.agent()in flow agents to forwardonStepStart/onStepFinishhooks to sub-agents — sub-agent internal tool-loop steps are now observable from the root flow's hooksagentChaininternally throughParentAgentContextandStepBuilderOptionswithout exposing it on publicGenerateParamsorBaseGenerateParamsAgentChainEntryuses{ readonly id: string }(object, not string) for future extensibilityTest plan
pnpm typecheck— all packages compilepnpm build— build succeedspnpm test --filter=@funkai/agents— 662 tests pass (658 existing + 4 new)agentChainwith flow agent id$.agent()sub-agent internal steps carry extendedagentChainagentChain