Skip to content

feat: lifecycle hooks#1

Open
justsomelegs wants to merge 10 commits intodavis7dotsh:mainfrom
justsomelegs:feat/lifecycle-hooks
Open

feat: lifecycle hooks#1
justsomelegs wants to merge 10 commits intodavis7dotsh:mainfrom
justsomelegs:feat/lifecycle-hooks

Conversation

@justsomelegs
Copy link

@justsomelegs justsomelegs commented Oct 6, 2025

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:

    • A simple function for straightforward usage.
    • An object with:
      • try: the hook function.
      • catch: an optional per-hook error handler.
  • callServerHook executor
    Wraps hook invocation and ensures:

    • Errors in hooks are caught.
    • Per-hook catch is called if provided.
    • Global onError fallback is called if the per-hook handler is absent.
    • Default logging is used if no handler exists.
  • Server handler updates
    All hook calls (beforeAgentRun, afterAgentRun, onAbort, onError) now use callServerHook:

    • Consistent error handling across all hooks.
    • Guarantees that afterAgentRun runs even if runner.runAgent throws.
  • Example route updated
    Demonstrates both simple hooks and hooks with custom per-hook error handling.

Why

  • Allows users to define custom error handling for each lifecycle stage without boilerplate.
  • Ensures errors in hooks do not silently break the server.
  • Improves observability, debugging, and robustness of agent runs.
  • Maintains backward compatibility for existing hook usage.

Notes / Follow-ups

  • The API is still flexible; what data is passed to each hook can change in future revisions.
  • Not sure if you were already planning this, but I had some spare time and implemented per-hook error handling as well :)

@justsomelegs justsomelegs changed the title feature: lifecycle hooks feat: lifecycle hooks Oct 6, 2025
@justsomelegs justsomelegs marked this pull request as draft October 6, 2025 19:02
@justsomelegs justsomelegs marked this pull request as ready for review October 6, 2025 19:44
@justsomelegs
Copy link
Author

I think the implementation’s pretty much done. Let me know if you’d like any tweaks or changes.

Could potentially add the same HookWithError type to the client hooks i suppose, but I'll wait to hear what you want.

@bmdavis419
Copy link
Collaborator

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"

@justsomelegs
Copy link
Author

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?

@bmdavis419
Copy link
Collaborator

found an issue when testing, there isn't proper type narrowing on the input type based on the agentId
image
this is definitely some hellish generic nonsense that needs to get fixed, looking into it now...

@bmdavis419
Copy link
Collaborator

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

@justsomelegs
Copy link
Author

found an issue when testing, there isn't proper type narrowing on the input type based on the agentId image this is definitely some hellish generic nonsense that needs to get fixed, looking into it now...

I just got type narrowing working.

image

@bmdavis419
Copy link
Collaborator

oh nice, I had just done this really kinda dumb hack lol that's almost certainly better
image

@justsomelegs
Copy link
Author

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

would the middlewares run before the agent specific callbacks? would you wanna change this to a more modular system like;

image

or just keep them as they are currently?

@bmdavis419
Copy link
Collaborator

bmdavis419 commented Oct 6, 2025

This kinda devolved into my scratch work to think this through, curious what u think...

IGNORE:

~~Yea actually I like that more custom middle ware setup better. General flow in my head for what an agent run would look like:

  1. client calls it
  2. "beforeRun" MIDDLEWARE is fired
  3. "onStart" lifecycle method on the agent itself is run. this takes in the input type, then whatever it returns becomes the agent function's input type
  4. agent function is called
  5. any events that happen during the run (onAbort or onError) are called when they happen
  6. a final "onSuccess" MIDDLEWARE is fired if there were not any errors/aborts~~

Writing that out it would mean that:

agents have:

  • onStart
  • onError
  • onAbort
  • onFinish (would include a status string of "success" | "error" | "aborted"
  • agent (actual agent run function)

then middlewares have:

  • beforeRun
  • afterRun
  • onError
  • onAbort

so the final order would end up being:

  1. middleware.beforeRun()
  2. agent.onStart()
  3. agent.agent()
  4. WHEN ERROR: agent.onError(), then middleware.onError()
  5. WHEN ABORTED: agent.onAbort(), then middeware.onAbort()
  6. agent.onFinish()
  7. middleware.afterRun()

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:
onStart:

  • do an auth check
  • fetch requisite context from the DB
  • create and save an "agent_run_id"

onError:

  • good logging
  • update the status of a DB entry tracking the agent

ah but now as I'm writing this I forgot that like, streamText has onAbort and onError built into it, so adding our own might be pretty redundant...

onFinish:

  • save final status to DB
  • save metrics on how long the agent run took
  • maybe persist final outputs? (unsure on that one)

For the middleware I feel like all it would be would be:

  • auth checks for everything (don't love this)
  • uniform logging (not a bad thing)
    that's kinda it?

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

@justsomelegs
Copy link
Author

justsomelegs commented Oct 6, 2025

This kinda devolved into my scratch work to think this through, curious what u think...

IGNORE:

~~Yea actually I like that more custom middle ware setup better. General flow in my head for what an agent run would look like:

  1. client calls it
  2. "beforeRun" MIDDLEWARE is fired
  3. "onStart" lifecycle method on the agent itself is run. this takes in the input type, then whatever it returns becomes the agent function's input type
  4. agent function is called
  5. any events that happen during the run (onAbort or onError) are called when they happen
  6. a final "onSuccess" MIDDLEWARE is fired if there were not any errors/aborts~~

Writing that out it would mean that:

agents have:

* onStart

* onError

* onAbort

* onFinish (would include a status string of `"success" | "error" | "aborted"`

* agent (actual agent run function)

then middlewares have:

* beforeRun

* afterRun

* onError

* onAbort

so the final order would end up being:

1. middleware.beforeRun()

2. agent.onStart()

3. agent.agent()

4. WHEN ERROR: agent.onError(), then middleware.onError()

5. WHEN ABORTED: agent.onAbort(), then middeware.onAbort()

6. agent.onFinish()

7. middleware.afterRun()

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: onStart:

* do an auth check

* fetch requisite context from the DB

* create and save an "agent_run_id"

onError:

* good logging

* update the status of a DB entry tracking the agent

ah but now as I'm writing this I forgot that like, streamText has onAbort and onError built into it, so adding our own might be pretty redundant...

onFinish:

* save final status to DB

* save metrics on how long the agent run took

* maybe persist final outputs? (unsure on that one)

For the middleware I feel like all it would be would be:

* auth checks for everything (don't love this)

* uniform logging (not a bad thing)
  that's kinda it?

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.

@bmdavis419
Copy link
Collaborator

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

@justsomelegs
Copy link
Author

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?

@bmdavis419
Copy link
Collaborator

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.)

@justsomelegs
Copy link
Author

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 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.

@bmdavis419
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants