fix(packages/agents): wire up overrides.onStepFinish in flow agents#9
fix(packages/agents): wire up overrides.onStepFinish in flow agents#9zrosenbauer merged 4 commits intomainfrom
Conversation
The FlowAgentOverrides type declared onStepFinish, but flow-agent.ts never passed it to createStepBuilder — only config.onStepFinish was used. This meant per-call override hooks were silently ignored. Add buildMergedStepFinishHook() that merges config and override hooks (config fires first, then override) via fireHooks/wrapHook, and use the merged hook in both generate() and stream(). Co-Authored-By: Claude <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: a412dee 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.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe changes introduce a merged hook mechanism in the flow agent that combines config-level and per-call override Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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. 📝 Coding Plan
Comment |
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/agents/flow/flow-agent.test.ts`:
- Around line 429-435: Replace the mutable shared array "order" and in-place
pushes with assertions that inspect the mocks' invocation order: make
configOnStepFinish and overrideOnStepFinish remain vi.fn() spies and after the
scenario assert that configOnStepFinish.mock.invocationCallOrder[0] <
overrideOnStepFinish.mock.invocationCallOrder[0] (or the equivalent invocation
order property available on the vi.fn mocks), removing any use of order.push and
avoiding in-place mutation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 38473985-b83e-48f2-b5f9-c69e3dab35d1
📒 Files selected for processing (2)
packages/agents/src/core/agents/flow/flow-agent.test.tspackages/agents/src/core/agents/flow/flow-agent.ts
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Merge main's prepareFlowAgent() refactor while preserving the buildMergedStepFinishHook fix inside prepareFlowAgent. Co-Authored-By: Claude <noreply@anthropic.com>
5167f00 to
a412dee
Compare
Summary
FlowAgentOverrides.onStepFinishwas accepted by the type system but silently dropped at runtime in flow agents — onlyconfig.onStepFinishwas passed tocreateStepBuilderbuildMergedStepFinishHook()that merges config and per-call override hooks (config fires first, then override) using the existingfireHooks/wrapHookutilitiesgenerate()andstream()pathsTest plan
onStepFinishhooks fire duringgenerate()