Skip to content

fix(a2a): serve exact task reads from the persistent store - #2484

Merged
EItanya merged 6 commits into
kagent-dev:release/v0.10.xfrom
erauner12:fix/v0.10-a2a-persistent-get-task
Aug 22, 2026
Merged

fix(a2a): serve exact task reads from the persistent store#2484
EItanya merged 6 commits into
kagent-dev:release/v0.10.xfrom
erauner12:fix/v0.10-a2a-persistent-get-task

Conversation

@erauner12

@erauner12 erauner12 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Serve exact A2A GetTask requests from the controller's authoritative, owner-scoped persistent task store on release/v0.10.x without delegating to the managed runtime.

Fixes #2464.

Problem

The controller already uses persistent storage for ListTasks, but exact GetTask continued through the passthrough runtime. A persisted task could therefore appear in ListTasks while exact retrieval returned task_not_found.

Change

storeTaskQueryHandler.GetTask now:

  • queries the persistent store with the authenticated caller identity, or an empty identity when none is available;
  • returns a shaped copy with the requested recent-history bound and retained artifacts;
  • maps caller-scoped database.ErrNotFound to A2A ErrTaskNotFound without delegation; and
  • propagates other storage failures for the A2A handler to return with internal-error semantics.

Authorization boundary

Exact GetTask remains scoped to the authenticated caller. It does not substitute ShareContext.UserID: a task-ID-only request cannot prove that the requested task belongs to the shared session. Missing identity and caller-scoped misses return task_not_found without invoking the runtime, preserving the store's authorization boundary.

Why the controller store

The same persistent store already serves task listing and receives runtime persistence callbacks. Making it fully authoritative for exact reads keeps persisted task existence consistent across the A2A query surface and avoids a redundant runtime lookup against the same database.

Tests

  • cd go && go test ./core/internal/a2a
  • cd go && go test -race ./core/internal/a2a -run '^(TestGetTask_|TestWire_GetTaskStoreErrorSemantics)'

Coverage includes persistent hits, absent identity, other-owner and not-found results, backend failures, dual-wire A2A error mapping, requested history shaping, artifact retention, non-mutation, and the share-context boundary above. Every exact-read path asserts that the delegate is not invoked.

Validation

An independent A2A client against a compatible v0.10 deployment retrieved the same persisted task through exact GetTask on both supported 0.3 and 1.0 wires without a ListTasks fallback.

This PR fixes exact reads only. Runtime identity propagation is tracked separately in #2465.

Signed-off-by: Evan Rauner <raunerevan@gmail.com>
(cherry picked from commit fb5da78)
(cherry picked from commit a41e24bd6f7a1d38489c9cad50a49fc2251ae069)
@github-actions github-actions Bot added the bug Something isn't working label Aug 18, 2026
@erauner12
erauner12 marked this pull request as ready for review August 18, 2026 22:03
@erauner12
erauner12 requested a review from a team as a code owner August 18, 2026 22:03
Copilot AI lite review requested due to automatic review settings August 18, 2026 22:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request fixes an inconsistency in the legacy controller A2A query path by serving exact GetTask reads from the controller’s owner-scoped persistent task store (when an authenticated session principal is present) before falling back to the managed runtime, aligning GetTask behavior with the already store-backed ListTasks.

Changes:

  • Added a store-first GetTask implementation in storeTaskQueryHandler, with delegation on absent identity or database.ErrNotFound, and propagation of other store errors.
  • Extended the task-store interface and updated the Postgres client to support owner-scoped GetTask reads.
  • Added regression tests covering persistent hits (history shaping + artifact retention), owner isolation, share-context boundary behavior, fallback paths, and error propagation.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
go/core/internal/a2a/task_query_store.go Implements store-first GetTask and updates task-store interface to support exact reads from persistence.
go/core/internal/a2a/task_query_store_test.go Adds focused unit tests locking in authorization boundary and fallback/error behaviors for GetTask.
go/core/internal/a2a/a2a_handler_mux.go Updates handler-construction docs to reflect that task queries are store-backed when a task store is configured.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines 60 to +64
// handler for one agent. kagent persists tasks and is their source of truth,
// so with a store ListTasks is served from it instead of proxying to the agent
// runtime (whose legacy 0.3 transport returns ErrUnsupportedOperation for it);
// every other method, GetTask included, still delegates to the passthrough
// proxy. v0 has no native tasks/list, so the legacy handler is wrapped to
// serve that method from the store too (lowercase TaskState). Without a store
// both wires keep their native behavior, including v0's method-not-found for
// tasks/list.
// so with a store GetTask and ListTasks are served from it instead of proxying
// to the agent runtime. v0 has no native tasks/list, so the legacy handler is
// wrapped to serve that method from the store too (lowercase TaskState).
// Without a store both wires keep their native behavior, including v0's
Comment on lines +33 to +35
// storeTaskQueryHandler answers GetTask and ListTasks from kagent's task
// store, which is the source of truth for persisted tasks. Every other method
// is delegated to the embedded handler unchanged.
@erauner12

Copy link
Copy Markdown
Contributor Author

Related work and forward-port note

#2483 and #2485 repair identity propagation across the managed-runtime path. This PR is independent: authoritative store-backed exact reads remain useful even when runtime forwarding is functioning.

A main-oriented prototype is available in erauner12#12. The store-first/delegate-on-miss behavior transfers directly, but main's handler-mux structure differs from the v0.10 dual native/legacy JSON-RPC wiring. A forward-port should also retain this PR's authenticated-caller and share-context regression coverage rather than deriving read authority from ShareContext.UserID.

@supreme-gg-gg supreme-gg-gg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the new GetTask method makes sense to me, but I left a question about fallback behaviour


func (h *storeTaskQueryHandler) GetTask(ctx context.Context, req *a2atype.GetTaskRequest) (*a2atype.Task, error) {
userID := callerUserID(ctx)
if userID == "" {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the point of falling back to delegating to the runtime here and below in ErrNotFound. Why not treat the DB as authoritative as ListTask method below? The runtime will eventually read from the DB again which gives you the same result, this also weakens authorization boundary

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. The delegate-on-miss behavior was meant as a compatibility path for tasks not found in the controller store, but in this setup the persistent store is already the source of truth, and the runtime resolves through that same store.

If identity is missing, or if the caller-scoped lookup returns ErrNotFound, delegating doesn’t help and can only weaken the authorization boundary.

I’ll make GetTask match ListTasks: query the persistent store with the authenticated caller and return the store result or error directly, with no delegation. I’ll update the tests, comments, and PR rationale to match.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in 85333a83.

GetTask now treats the persistent store as authoritative: absent identity and caller-scoped database.ErrNotFound no longer delegate to the runtime, and not-found is mapped to A2A ErrTaskNotFound. The focused tests assert zero delegate calls for hits, misses, other-owner/share-context reads, absent identity, and backend failures, including 0.3 and 1.0 wire error semantics.

Validation passed:

  • go test ./core/internal/a2a
  • go test -race ./core/internal/a2a -run '^(TestGetTask_|TestWire_GetTaskStoreErrorSemantics)'

The PR description, source comments, and #2464 expectations were updated to match the authoritative-store behavior.

@erauner12 erauner12 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left response


func (h *storeTaskQueryHandler) GetTask(ctx context.Context, req *a2atype.GetTaskRequest) (*a2atype.Task, error) {
userID := callerUserID(ctx)
if userID == "" {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. The delegate-on-miss behavior was meant as a compatibility path for tasks not found in the controller store, but in this setup the persistent store is already the source of truth, and the runtime resolves through that same store.

If identity is missing, or if the caller-scoped lookup returns ErrNotFound, delegating doesn’t help and can only weaken the authorization boundary.

I’ll make GetTask match ListTasks: query the persistent store with the authenticated caller and return the store result or error directly, with no delegation. I’ll update the tests, comments, and PR rationale to match.

@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels Aug 20, 2026
…0-a2a-persistent-get-task

Signed-off-by: Evan Rauner <raunerevan@gmail.com>
Signed-off-by: Evan Rauner <raunerevan@gmail.com>
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels Aug 20, 2026
}

func (h *storeTaskQueryHandler) GetTask(ctx context.Context, req *a2atype.GetTaskRequest) (*a2atype.Task, error) {
task, err := h.store.GetTask(ctx, string(req.ID), callerUserID(ctx))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we do the userid check here like in the last version before this commit, so check the userID == "" first and return an error immediately instead of calling the DB

@erauner12 erauner12 Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in e8b4b1ce. GetTask now checks callerUserID(ctx) first and returns ErrTaskNotFound immediately when it is empty, before calling the persistent store. The absent-identity test now asserts that neither the store nor delegate is invoked.

Validation passed:

  • go test ./core/internal/a2a
  • go test -race ./core/internal/a2a -run '^(TestGetTask_|TestWire_GetTaskStoreErrorSemantics)'

Signed-off-by: Evan Rauner <raunerevan@gmail.com>
@erauner12
erauner12 force-pushed the fix/v0.10-a2a-persistent-get-task branch from 7ebc3a5 to e8b4b1c Compare August 20, 2026 23:41
@EItanya
EItanya merged commit 03e950f into kagent-dev:release/v0.10.x Aug 22, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants