Conversation
|
I think the implementation’s pretty much done. Let me know if you’d like any tweaks or changes. Could potentially add the same |
|
My initial thinking on this was actually to put the lifecycle hooks on each agent instead of at the root. Main use case I was thinking was an auth check or pulling something from the DB. Was thinking for the interface the beforeAgentRun hook would be optional. If it's there, it takes in the input and whatever it returns becomes the input for the actual agent run. I also think this would lay the foundations for running agents in a "waitUntil" and getting good resumability. Regardless, really appreciate this. Gonna review right now, I am down to have these basically be "middlewares" |
|
Tbh when making this I was tossing up between per agent hooks vs general application lifecycle, I dont think it would to much of a refactor if you wanted me to make this per-agent hooks instead? |
I'm actually down to have both. A middleware system like what you have for global logging/ratelimiting/etc. seems like a good thing to have. But I definitely want to have a more fine grained one for each agent as well |
|
This kinda devolved into my scratch work to think this through, curious what u think... IGNORE:
Writing that out it would mean that: agents have:
then middlewares have:
so the final order would end up being:
honestly feels a little complicated and would need a very good usecase for middlewares to go all the way with that. Like imagine trying to put that in docs and explain that entire lifecycle to a normal eng lmao. The agent lifecycle use cases I can think of:
onError:
ah but now as I'm writing this I forgot that like, onFinish:
For the middleware I feel like all it would be would be:
Honestly leaning towards axing the global lifecycles and just focusing on a really good "onStart()" (should actually be "beforeRun()") and "onFinish()" for the agents... curious what u think |
I actually think having both global lifecycle hooks and per agent hooks could be nice for DX. It would keep the door open for more flexible options. Having an option in middleware to control whether it runs before or after agent hooks could be a good middle ground too (with maybe an override option at agent level if a particular agent has no need or should not use a certain middleware). That way, the global "application-level" hooks can handle broader boilerplate tasks without having to repeat logic inside every agent hook. I like the idea of maybe renaming them similar to Vitest or (beforeAll, afterAll, etc.) to make the lifecycle more intuitive when reading. And the order you outlined makes sense but having the ability to configure the order would nice in cases where middleware should wrap or trail and agent's lifecycle. As for simplicity vs extensibility, I’m good with whatever direction you feel best fits the long-term design, this feels like one of those spots where it might be worth keeping the abstraction around, even if the docs take an extra paragraph to explain. Though overall I'm easy with any decision. |
|
Been thinking about it and what I want to do is this: For now just do the before/after hooks on the agents themselves to keep things simpler, mostly because one of the next things I need to do is an "adapter" system. Currently River only works in SvelteKit which is how I like it, but I know that 99% of people will just be using NextJS so it needs to be supported. Only having 2 server lifecycle functions will make doing the experimentation/work of getting all of that working WAY easier, then we can revisit the middleware stuff later. I'm gonna throw together a quick separate PR for the agent hooks (should be pretty easy, and I want to leave this one around as reference for the middleware stuff later) and then I have a flight tonight where I'll do some exploring on making a more generic adapter |
Yeah that sounds good, It didn't take me too long to throw together theses lifecycle hooks so fingers crossed it should be to different on the per agent level. I mean i'm a Svelte guy as well so the current setup doesn't bother me but yeah for any kind of wide spread use at least for now a NextJS adapter is the way to go. Is there anything you want me to work on in the meantime that would help you out? |
|
if you could do a pass on more robust error handling that would be awesome. Taking the "RiverError" and making it way better in a similar way to trpc's https://trpc.io/docs/server/error-handling. Would be awesome to have the errors the client gets with onError always be an error with a descriptive code and message (stream closed, server crashed, read failed, custom error, etc.) |
Yeah okay sounds good, i'll do a quick pass and hopefully have a draft PR open in the next hour. |
|
pushed in the basic lifecycles for agents, can see them in action here: https://github.com/bmdavis419/river-examples/tree/main/basic-aisdk-example going to react conf tonight so I'll be less active then normal this week, but I'll try and do some experimenting with adapters on the flight and when I have time |





This PR adds a server lifecycle hooks system to provide flexible, type-safe hooks with per-hook error handling.
Changes
Lifecycle hooks
LifecycleHooks<T extends AgentRouter>now supports each hook (beforeAgentRun,afterAgentRun,onAbort,onError) being either:try: the hook function.catch: an optional per-hook error handler.callServerHookexecutorWraps hook invocation and ensures:
catchis called if provided.onErrorfallback is called if the per-hook handler is absent.Server handler updates
All hook calls (
beforeAgentRun,afterAgentRun,onAbort,onError) now usecallServerHook:afterAgentRunruns even ifrunner.runAgentthrows.Example route updated
Demonstrates both simple hooks and hooks with custom per-hook error handling.
Why
Notes / Follow-ups