fix(adk): preserve A2A user identity before task resolution - #2483
fix(adk): preserve A2A user identity before task resolution#2483erauner12 wants to merge 7 commits into
Conversation
Signed-off-by: Evan Rauner <raunerevan@gmail.com>
Signed-off-by: Evan Rauner <raunerevan@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes an identity-propagation gap in the A2A runtimes by ensuring the authenticated x-user-id is available at the runtime boundary before task-store resolution/persistence callbacks run. This aligns the Go ADK interceptor and the Python task store/request-context plumbing so owner-scoped controller calls consistently execute under the initiating user rather than falling back to the runtime service identity.
Changes:
- Go: propagate
x-user-idinto the returnedcontext.Contextviaauth.WithUserID(...)in the A2A call interceptor. - Python: derive the effective user from
ServerCallContext, scope it around controller HTTP calls inKAgentTaskStore, and clear stale request identity on headerless requests. - Add focused regression tests covering scoping/restoration behavior and headerless-request clearing.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| python/packages/kagent-core/tests/test_task_store.py | Adds tests validating user scoping around task-store controller calls and restoration on success/failure. |
| python/packages/kagent-core/tests/test_request_context.py | Adds test ensuring a later headerless request clears previously set request identity. |
| python/packages/kagent-core/src/kagent/core/a2a/_task_store.py | Scopes the effective user (from ServerCallContext) around controller save/get/delete calls. |
| python/packages/kagent-core/src/kagent/core/a2a/_requests.py | Sets/clears request-scoped user identity at request-build time based on ServerCallContext headers. |
| python/packages/kagent-core/src/kagent/core/a2a/_context.py | Introduces helpers to extract x-user-id from ServerCallContext and a context manager to scope identity. |
| go/adk/pkg/a2a/executor.go | Fixes interceptor to return a context carrying the authenticated user ID. |
| go/adk/pkg/a2a/executor_test.go | Adds regression test asserting the interceptor populates both CallContext.User and outbound X-User-Id injection via context. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Coordination and forward-port note#2485 is the controller-side counterpart to this runtime-boundary change and should merge after this PR. #2484 complements both with persistent exact reads but is independently mergeable. A main-oriented prototype is available in erauner12#11. A forward-port should preserve this PR's identity-lifecycle invariants while adapting to main's newer boundaries: the Go runtime uses the Combined validation on a compatible v0.10 build completed a fresh-process |
supreme-gg-gg
left a comment
There was a problem hiding this comment.
Thanks for reporting this issue and opening this fix, this sounds reasonable to me, left one small comment. We also have a PR doing this for main branch
| @contextmanager | ||
| def scoped_request_user_id(user_id: str | None) -> Iterator[None]: | ||
| """Temporarily expose a scoped user to controller HTTP request hooks.""" | ||
| if not user_id: |
There was a problem hiding this comment.
Why does it preserve the existing context var when the current server call context has no user? I would expect the call context to be authoritative
There was a problem hiding this comment.
Agreed, the current ServerCallContext should be the source of truth inside the task-store callback.
The early return for a missing user leaves the outer ContextVar value in place, which could let a headerless request inherit stale identity. I’ll change the scope so it sets None for the callback and resets the token afterward.
I’ll also update the regression test to confirm that the headerless callback sees no user, and that the outer value is restored once the callback finishes.
I reviewed #2449 as well. Its Go change enforces the same contract on main; this PR is for release/v0.10.x and also covers the matching Python runtime boundary. I’ll keep the Go implementation aligned with #2449 while preserving the release-specific and Python coverage.
There was a problem hiding this comment.
Updated in d75a67f6.
scoped_request_user_id now always installs the current call-context value, including None. The regression test verifies that a headerless callback observes no user while the outer ContextVar value is restored after the scope exits.
Focused validation passed:
pytest packages/kagent-core/tests/test_request_context.py packages/kagent-core/tests/test_task_store.py— 4 passedgo test ./adk/pkg/a2a— passed
The branch was also updated from the latest release/v0.10.x without rewriting the reviewed commits.
| @contextmanager | ||
| def scoped_request_user_id(user_id: str | None) -> Iterator[None]: | ||
| """Temporarily expose a scoped user to controller HTTP request hooks.""" | ||
| if not user_id: |
There was a problem hiding this comment.
Agreed, the current ServerCallContext should be the source of truth inside the task-store callback.
The early return for a missing user leaves the outer ContextVar value in place, which could let a headerless request inherit stale identity. I’ll change the scope so it sets None for the callback and resets the token afterward.
I’ll also update the regression test to confirm that the headerless callback sees no user, and that the outer value is restored once the callback finishes.
I reviewed #2449 as well. Its Go change enforces the same contract on main; this PR is for release/v0.10.x and also covers the matching Python runtime boundary. I’ll keep the Go implementation aligned with #2449 while preserving the release-specific and Python coverage.
Signed-off-by: Evan Rauner <raunerevan@gmail.com>
Signed-off-by: Evan Rauner <raunerevan@gmail.com>
There was a problem hiding this comment.
Hi @erauner12 thanks for the change, this lgtm but can you remove the Go ADK changes given that #2507 is going to merge soon (which adds the Go part to 0.10.x)
|
@supreme-gg-gg , will do! |
Signed-off-by: erauner <erauner@medallia.com>
|
description still says go call interceptor changed, and test plan lists go test ./adk/pkg/a2a. last commit here removed the go changes as superseded. diff is python only now. probably just needs the description and test plan updated to match. |
Summary
Preserve the authenticated A2A user before task-store resolution in both the Go and Python runtimes on
release/v0.10.x.Fixes #2465.
Problem
A persisted continuation is a new A2A request and may be handled by a different process. Task resolution can run before executor-local identity setup, so task-store callbacks must receive the authenticated request identity at the runtime boundary.
Without that identity, owner-scoped reads can fall back to the runtime service account and report an existing task as not found.
Change
x-user-idvalue in bothCallContext.Userand the returnedcontext.Contextbefore task resolution.ServerCallContext, scope it around controller calls, restore prior context afterward, and clear stale request identity when a later request has no forwarded user.Trust boundary
This introduces no new identity source. Both runtimes consume the existing authenticated A2A service parameter; they do not trust arbitrary task metadata or invent an owner.
Tests
cd go && go test ./adk/pkg/a2acd python && uv run pytest packages/kagent-core/tests/test_task_store.py packages/kagent-core/tests/test_request_context.pyCoverage verifies pre-resolution visibility, scoped restoration after success and failure, that a headerless scoped callback observes no user while an outer ContextVar is restored afterward, and no identity leakage from a user-bearing request into a later headerless request.
Scope and ordering
This PR changes runtime identity ingestion only. Persistent exact
GetTaskremains the separate issue tracked in #2464.