feat(runtime_context): pass Request to extension point (EVO-1626 followup)#13
Open
marcelogorutuba wants to merge 5 commits into
Open
Conversation
…owup) Previously runtime_context.current_context_id was called with the metadata dict, which is always None on the chat/a2a code paths. As a result, an enterprise consumer that resolves tenant from the X-Evo-Tenant-Id header could never observe it. Thread an optional `request` argument from the chat and a2a routes down through agent_runner -> StandardRunner.run_agent and prefer it over metadata when present at the runtime_context call site. Falls back to metadata when no request is provided (e.g. internal callers), so the default community impl that ignores the argument is unaffected. Refs: EVO-1626
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThreads an optional request object from the chat and A2A API routes through the agent runner into the StandardRunner, and uses it as the primary input to the runtime_context extension point, falling back to metadata when no request is provided. Sequence diagram for passing request to runtime_context extension pointsequenceDiagram
actor Client
participant ChatRoutes as chat_routes
participant A2aRoutes as a2a_routes
participant AgentRunner as agent_runner
participant StandardRunner as StandardRunner
participant RuntimeContext as runtime_context
Client->>ChatRoutes: chat(request)
ChatRoutes->>AgentRunner: run_agent(db, session_id, files, metadata, user_id, request)
AgentRunner->>StandardRunner: run_agent(db, session_id, files, metadata, user_id, request)
StandardRunner->>RuntimeContext: current_context_id(request)
Client->>A2aRoutes: handle_message_send(request)
A2aRoutes->>AgentRunner: run_agent(db, session_id, files, metadata, user_id, request)
AgentRunner->>StandardRunner: run_agent(db, session_id, files, metadata, user_id, request)
alt request is None
StandardRunner->>RuntimeContext: current_context_id(metadata)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider tightening the type for the new
requestparameter (e.g.,RequestorOptional[Request]) instead ofAnyinrun_agentandStandardRunner.run_agentso downstream consumers get better type safety and editor support. - To reflect that
requestcan be omitted, it may be clearer to type it asOptional[...] = Nonerather thanAny = Nonein both the function signatures and any related call sites.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider tightening the type for the new `request` parameter (e.g., `Request` or `Optional[Request]`) instead of `Any` in `run_agent` and `StandardRunner.run_agent` so downstream consumers get better type safety and editor support.
- To reflect that `request` can be omitted, it may be clearer to type it as `Optional[...] = None` rather than `Any = None` in both the function signatures and any related call sites.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…ardRunner (EVO-1626) Code review follow-up on PR #13: with_context is sync by contract, so it cannot keep the consumer's tenant binding alive across the awaits between the FastAPI handler and the next SQLAlchemy transaction. Add a companion async-only sibling helper so consumers can scope the binding per request. - EP 1.0.0 -> 1.1.0: new optional bind_context(context_id) -> AsyncContextManager[None] surfaced on the runtime_context module. Community default is a no-op async CM. Consumer impls participate by exposing a bind_context method (duck-typed); when absent the shim returns the no-op. - StandardRunner.run_agent wraps the request body in async with runtime_context.bind_context(context_id) if context_id else nullcontext(). Community path is unchanged (no-op CM); enterprise path keeps app.current_tenant_id set across every await so the consumer's before begin listener can issue set_config(...) on each transaction. - EXTENSION_POINTS.md: documents the new optional method, explains why it has to be async, and bumps the changelog. No enterprise code lives in community — the shim is a pure extension point default, mirroring how with_context, current_context_id and the other EPs already work. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Round-2 review M2-1: extend the runner-hook mirror with 4 tests covering the new async with bind_context wrap in StandardRunner. - test_default_bind_is_noop_async_cm: community default works as async CM (no consumer registered). - test_null_branch_when_context_id_is_none: nullcontext() supports async with on Python 3.10+. - test_consumer_bind_context_is_invoked: when a consumer exposes bind_context, the shim delegates and enter/exit are tracked. - test_consumer_bind_context_resets_on_exception: exception in body still drives __aexit__ → finally cleanup. 10 passed (4 new). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ity (EVO-1626) Community must remain neutral about what consumers do with the runtime context EP. Replace tenant/enterprise-specific wording in comments and docs with generic context-binding language. No behavior change; the EP is still a generic per-request context primitive whose meaning is defined by whichever consumer is registered. - runtime_context.py: docstrings now talk about "context binding" / "consumers", not "tenant" / "enterprise". - standard_runner.py: wrap comment talks about "operational context", no more app.current_tenant_id reference (that was implementation detail of one specific consumer). - EXTENSION_POINTS.md: "Why bind_context is async-only" rationale reworded to be consumer-agnostic. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Round-3 polish: - L3-1: remove "for EVO-1626" from TestBindContextWrap docstring. Ticket refs in code rot — context belongs in PR descriptions, not source. - L3-2: declare bind_context() -> AbstractAsyncContextManager[None] so type checkers (mypy/pyright) can validate async with callers, and to match the return-type annotation style of with_context and current_context_id in the same module. 10 passed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
requestargument torun_agent/StandardRunner.run_agentand threads it from the chat/a2a routes down to theruntime_contextextension-point call site.requestovermetadataso consumers that resolve the tenant fromX-Evo-Tenant-Idactually see the header. Falls back tometadatawhen no request is provided (internal callers, tests).Why
Before this patch,
runtime_context.current_context_id(metadata)was called withmetadata=Noneon every chat/a2a request, so the enterprise consumer inevo-enterprise-licensing-python(EVO-1626) had no way to observeX-Evo-Tenant-Idend-to-end.Validation
ast.parse).src/services/adk/runners/standard_runner.py:101) keeps a single-line behavior change behind the existing log line.Changed Files
src/services/adk/runners/standard_runner.pysrc/services/adk/agent_runner.pysrc/api/chat_routes.pysrc/api/a2a_routes.pyLinked Issue
Summary by Sourcery
Thread the incoming request object through agent execution so the runtime context extension point can use it when resolving context IDs.
Bug Fixes:
Enhancements: