fix: flatten TaskPushNotificationConfig to match v1 spec

[darrelmiller]

Summary

Flattens TaskPushNotificationConfig to match the v1 A2A specification.

Fixes #416

Problem

The SDK used a nested two-class structure (TaskPushNotificationConfig wrapping PushNotificationConfig) that didn't match the v1 spec's flat model where url, token, and authentication are direct properties of TaskPushNotificationConfig.

Solution

• Moved url, token, and authentication directly onto TaskPushNotificationConfig
• Made Id and TaskId nullable (they are server-assigned, not always present when used inline in SendMessageConfiguration)
• Updated V0_3Compat converters for bidirectional conversion
• Updated AspNetCore handlers, CLI sample, and all tests
• Retained PushNotificationConfig type for HTTP+JSON REST endpoint body compatibility

Testing

• All 1,648 tests pass across all test projects
• ITK interoperability tests pass (4/5 scenarios)

[gemini-code-assist]

Code Review

This pull request flattens the TaskPushNotificationConfig structure in v1.0, moving properties like Url, Token, and Authentication directly into the class instead of nesting them, and updates the client, server processors, compatibility converters, and tests accordingly. The review feedback identifies several issues: an uninitialized TaskId in A2ACli.cs that should be replaced with the taskId parameter, a lack of validation for TaskId in A2AHttpJsonClient.cs which could lead to malformed URLs, and potential NullReferenceExceptions in V03TypeConverter.cs when accessing auth.Schemes.Count without null-safety checks.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[chopmob-cloud]

Verified this independently against current main and the v1 spec proto. The flattening is correct and the earlier bot findings are all addressed on 6075ee1. One asymmetry looks worth a second look before merge.

Spec conformance checks out

Checked the flattened model field by field against specification/a2a.proto:

message TaskPushNotificationConfig {
  string tenant = 1;
  string id = 2;
  string task_id = 3;
  string url = 4 [(google.api.field_behavior) = REQUIRED];
  string token = 5;
  AuthenticationInfo authentication = 6;
}

The new shape matches exactly, and relaxing Id and TaskId to nullable while keeping [JsonRequired] only on Url is right: url carries the sole REQUIRED behavior annotation. The SendMessageConfiguration property rename is also spec correct, not just a rename for symmetry, since the spec field is task_push_notification_config = 2.

Both PushNotificationConfig and TaskPushNotificationConfig are registered in A2AJsonContext, so the source generated path is covered for the new PostJsonAsync<TaskPushNotificationConfig, ...> call.

The three points from the automated review all read as resolved on the current head: TaskId = taskId is set in A2ACli.cs, ArgumentException.ThrowIfNullOrEmpty(request.Config.TaskId, ...) guards the URL build, and the Schemes access moved to the is { Schemes: { Count: > 0 } schemes } property pattern in all three conversion sites.

Tenant is silently dropped on HTTP+JSON but preserved on JSON-RPC

A2AHttpJsonClient.CreateTaskPushNotificationConfigAsync now posts the flat type:

return await PostJsonAsync<TaskPushNotificationConfig, TaskPushNotificationConfig>(
    $"/tasks/{Uri.EscapeDataString(request.Config.TaskId)}/pushNotificationConfigs",
    request.Config, "CreateTaskPushNotificationConfig", cancellationToken).ConfigureAwait(false);

but the REST endpoint still binds the nested type, unchanged by this PR:

internal static Task<IResult> CreatePushNotificationConfigRestAsync(
    IA2ARequestHandler requestHandler, ILogger logger, string taskId,
    PushNotificationConfig config, CancellationToken cancellationToken)

PushNotificationConfig has no Tenant member, and the options use JsonSerializerDefaults.Web without JsonUnmappedMemberHandling.Disallow, so tenant is skipped rather than rejected. taskId is dropped the same way, which is harmless since the route supplies it, but tenant has no other channel on this path.

Before this PR the two sides agreed: the client posted a PushNotificationConfig and the server bound a PushNotificationConfig, and neither could express tenant. Now the client can send it and the server cannot receive it, so the drop is newly reachable. It is also the one field the spec constrains relationally, "Must match the tenant value from the selected AgentInterface in the Agent Card when that field is set", so losing it silently on one transport seems worth avoiding.

Confirmed on 6075ee1 rather than reasoned about, with the real A2AJsonUtilities.DefaultOptions:

var sent = new TaskPushNotificationConfig
{
    Id = "cfg-1", TaskId = "t-1", Url = "http://callback", Tenant = "tenant-A",
};
var wire = JsonSerializer.Serialize(sent, A2AJsonUtilities.DefaultOptions);

// what the REST endpoint binds
var restBound = JsonSerializer.Deserialize<PushNotificationConfig>(wire, A2AJsonUtilities.DefaultOptions)!;
// no Tenant member to carry it

// what the JSON-RPC path binds
var rpcBound = JsonSerializer.Deserialize<TaskPushNotificationConfig>(wire, A2AJsonUtilities.DefaultOptions)!;
Assert.Equal("tenant-A", rpcBound.Tenant);   // passes

Passes on net10.0, so the same create call keeps tenant over JSON-RPC and loses it over HTTP+JSON.

Changing the REST parameter to TaskPushNotificationConfig would close it and match the client, though it changes what that endpoint accepts, so it may be deliberate to leave the REST surface on the v0.3 shape for now. If it is, a note on PushNotificationConfig saying tenant is not carried on the HTTP+JSON create path would save the next reader the round trip.

Minor, spec wording only

ToV1SendMessageRequest fills the config's task id from the message:

TaskPushNotificationConfig = cfg.PushNotification is { } pn
    ? ToV1TaskPushNotificationConfigFromV03PushNotification(pn, p.Message.TaskId ?? string.Empty)
    : null,

The spec comment on SendMessageConfiguration.task_push_notification_config says "Task id should be empty when sending this configuration in a SendMessage request." For a v0.3 follow up message on an existing task this populates it. Harmless as far as I can tell, and arguably more informative, just flagging it since the surrounding change is about matching the spec precisely.

[xu-shanshan]

this is the embedded SendMessageConfiguration.task_push_notification_config path, not the standalone CreateTaskPushNotificationConfig path.

[xu-shanshan]

For the standalone create API, the config is being added to an already-known task, so taskId identifies the parent task for that operation. But in SendMessage, the push config is attached while the message is being sent, and the server should associate it with the task created or selected by that SendMessage request.

[xu-shanshan]

TaskId = taskId says which task this config belongs to. In the embedded SendMessage path, that should come from the SendMessage task context, so I think this should be omitted.

[xu-shanshan]

This test is for SendMessageAsync, so it is exercising the embedded SendMessageConfiguration.taskPushNotificationConfig shape.

Because the spec says the embedded SendMessage config should not carry taskId, I think this fixture should avoid both identifiers here:

no TaskId, because the task association comes from the SendMessage task context
no Id, unless SendMessage explicitly supports client-assigned push-config ids

[xu-shanshan]

One REST behavior to clarify: this endpoint now binds the body as TaskPushNotificationConfig, which includes Tenant, then forwards the config after only overriding TaskId from the route.

That means a non-tenant REST call to /tasks/{id}/pushNotificationConfigs can still pass tenant in the body to the handler. But MapHttpA2A currently documents tenant REST variants as unsupported and says request Tenant fields are always null for REST calls.

Could we make this consistent one way or the other?

If tenant REST is out of scope, reject or clear config.Tenant on the non-tenant REST route and add a test for that behavior.
If body tenant is intentionally supported on the non-tenant route, update the REST limitation docs and add a route/body test so this is explicit.