Repository navigation
Add action middleware hook to RuntimeConfig - #461
Conversation
AK2000
left a comment
There was a problem hiding this comment.
Thank's for the PR, it looks good! I had some small comments. Would you like me to go ahead and make changes and merge?
| terminate_override: bool | None = None | ||
|
|
||
|
|
||
| @dataclasses.dataclass(frozen=True) |
There was a problem hiding this comment.
I wonder if this whole block would be better off either in agent.py where we define other type related to actions, or maybe in it's own class.
| assert self._exchange_client is not None | ||
| context = ActionContext(source_id, self._exchange_client) | ||
| return await action_method(*args, context=context, **kwargs) | ||
| context = ActionContext(call.source_id, self._exchange_client) |
There was a problem hiding this comment.
I think we should inject the context into a place the middleware will have access to it instead of inside the nest of middleware calls. The idea behind the context was to give the action access to its caller. Also, a middleware might need other information from the exchange (i.e. ask the source for some information, or check the owner on the source)
| args: Tuple of positional arguments. | ||
| kwargs: Dictionary of keyword arguments. | ||
| tag: Unique tag of the request message that initiated this | ||
| invocation, if the action was initiated by a message. |
There was a problem hiding this comment.
is it reasonable to assume that an action always comes via a tagged message? in which case the optionality of this tagged value can go away and we can be confident that there is always a UUID here. otherwise, downstream, everyone has to deal with what happens if there is no tag.
I didn't dig into the codebase to see if this is true or not - @AK2000 maybe knows, or I'll look later.
There was a problem hiding this comment.
We always pass actions with a tag except in the tests. I think it becomes a compatibility question, because technically people could instantiate runtime on their own, generate an arbitrary source id and call runtime.action. I don't imagine there to be any useful reason to do this, but it is possible. Does adding a required tag then make this a breaking change?
There was a problem hiding this comment.
calling actions that are not mediated by the documented ways of invoking actions should not be in the public facing api.
…nent) Action middleware (academy-agents/academy#461 hook) and a logging-handler fallback that emit an Academy actor's action lifecycle as ARIA tool.call and journal.* events, validated against the canonical contracts before they leave the process. Tests run a real Academy Runtime; the demo reconstructs terminal state from the event stream alone. First of the adapter components from the ARIA-Academy gap analysis (next: session/token client, capability publisher, run wrapper).
|
A data point for review: this branch now has a real consumer. I built the first component of an ARIA conformance adapter against it — an event emitter that turns an actor's action lifecycle into governance events (one started + one succeeded/failed record per invocation, with the invocation tag as the correlation id). The middleware interface held up without modification: ActionCall carries exactly the fields the emitter needed, the exception path maps cleanly to failure events, and the picklability constraint was no burden for a module-level middleware class. Tests run against a real Runtime with the middleware installed via RuntimeConfig. Code and live tests: https://github.com/agents4science/agents4science.github.io/tree/main/academy-aria |
|
Please do — much appreciated, and your call entirely on where the types live. On the context injection: strongly agree, with a concrete data point. The next thing we plan to build on this hook is a token-scope/policy interceptor, which needs exchange access exactly as you describe (query the caller, check ownership on the source); the event emitter just didn't happen to need it, so the gap didn't show in the first consumer. On Ben's question: making the tag always-present would simplify consumers — our emitter carries a None-fallback branch solely for the direct runtime.action() path, which has no message behind it. Generating a UUID at dispatch when no message tag exists seems like the right resolution: every invocation gets a correlation identity, and downstream code stops branching. |
a932277 to
823ad11
Compare
Summary
Adds a small, general extension point to the runtime: an ordered chain of action middleware configured on
RuntimeConfig, applied around every action dispatch.Motivation. Governance-style integrations — per-action authorization, policy evaluation, budget/usage metering, audit-event emission, supervision telemetry — all need to intercept the same place: action dispatch in
Runtime.action. Today that requires subclassingRuntime, but a subclass cannot be injected throughManager.launch: the manager ships a_RunSpecto the executor and constructsRuntimeon the worker.RuntimeConfigis the thing that travels, so the hook lives there. One documented interception contract lets all of these integrations live outside Academy core (we are building an ARIA/GMP conformance adapter on exactly this hook, but the mechanism is deliberately not specific to it).Design notes.
ActionCall(frozen dataclass):action,source_id,agent_id,args,kwargs, and the request messagetagso external systems can correlate invocations with their own records.UserErrorResponse).AttributeErrorbefore any middleware runs — no behavior change for that path.Runtime.__init__; with no middleware configured, dispatch goes straight to the previous logic (now_invoke_action). No overhead and no behavior change for existing users.RuntimeConfig.Related Issues
None filed — happy to convert the design discussion to an issue first if you'd prefer; splitting was skipped to keep the proposal concrete.
Changes
Testing
Runtime.actionand the message path returningUserErrorResponse),ActionCallmetadata including messagetag, unknown-action short-circuit (middleware not invoked), andRuntimeConfigpicklability with middleware configured.pytest tests/unit: 627 passed, 3 skipped.ruff check/ruff format --checkclean on changed files;mypy academy/runtime.pyintroduces no new errors (the 6 reported are pre-existing inmessage.py/authenticate.pyunder current dep versions).Pull Request Checklist
pre-commit(e.g., ruff, mypy, etc.).RuntimeConfig,ActionCall,ActionMiddleware, andRuntime.action; happy to add a user-guide page if you'd like one.)Drafted with AI assistance (Claude); reviewed line-by-line by the author.
🤖 Generated with Claude Code