Skip to content

Commit 0a53130

Browse files
rogerbarretorogerbarreto
committed
Address PR #5940 design feedback (Q-A through Q-F)
Q-A: poll vector store til status leaves InProgress before return. Exp backoff 250ms-2s. Honor cancel. Q-B: try/catch upload loop. Mid-fail = best-effort DeleteFileAsync on already-uploaded ids. Swallow cleanup errors. Q-C: pinned AgentReference.Version uses GetAgentVersionAsync. Empty/whitespace/'latest' = GetLatest path. Q-D: HostedAgentUserAgentPolicy detects existing combined 'foundry-hosting/...' segment. No double prefix. Q-E: mode-3 vector-store test uses fake transport. No DNS to example.com. Q-F: no shim. Class always [Experimental] (since 8015e00, before dotnet-1.0.0). No compat contract. Callers rename to AIProjectClientExtensions. Rebase onto origin/main reconciliation: aad20c2 added public AsAIAgent(this AIProjectClient, Uri agentEndpoint, ...) extension that calls an internal FoundryAgent(AIProjectClient, Uri, ...) ctor. Reintroduced that ctor + a new FoundryChatClient(AIProjectClient, Uri, ProjectOpenAIClientOptions?) overload that reuses the supplied AIProjectClient's pipeline (via GetProjectResponsesClientForAgentEndpoint) instead of stamping a fresh credential. Verified: 346/346 net10 + 284/284 net472 Foundry unit, 230/230 Foundry.Hosting unit, format clean.
1 parent 5677e13 commit 0a53130

7 files changed

Lines changed: 521 additions & 20 deletions

File tree

‎dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/HostedAgentUserAgentPolicy.cs‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ internal sealed class HostedAgentUserAgentPolicy : PipelinePolicy
4343
/// <summary>Bare segment stamped by <c>AgentFrameworkUserAgentPolicy</c> in the non-hosted scenario; this policy upgrades it in-place when both run.</summary>
4444
private const string BareAgentFrameworkPrefix = "agent-framework-dotnet/";
4545

46+
/// <summary>Combined hosted segment that this policy emits. Recognized in-place so callers whose pipelines already carry a (possibly different-version) combined segment get it replaced rather than double-prefixed (Q-D fix).</summary>
47+
private const string CombinedHostedPrefix = "foundry-hosting/agent-framework-dotnet/";
48+
4649
public override void Process(PipelineMessage message, IReadOnlyList<PipelinePolicy> pipeline, int currentIndex)
4750
{
4851
AppendHeader(message);
@@ -66,12 +69,32 @@ private static void AppendHeader(PipelineMessage message)
6669
return;
6770
}
6871

72+
// Combined-form check first: if the caller's pipeline already has
73+
// `foundry-hosting/agent-framework-dotnet/{version}` (with a version that differs
74+
// from ours — otherwise the .Contains above would have returned early), replace the
75+
// entire combined span in place. Without this, the bare-prefix search below would
76+
// match `agent-framework-dotnet/` *inside* the combined segment and produce a
77+
// malformed `foundry-hosting/foundry-hosting/agent-framework-dotnet/...` value.
78+
var combinedIdx = existing.IndexOf(CombinedHostedPrefix, StringComparison.Ordinal);
79+
if (combinedIdx >= 0)
80+
{
81+
var combinedEnd = existing.IndexOf(' ', combinedIdx);
82+
if (combinedEnd < 0)
83+
{
84+
combinedEnd = existing.Length;
85+
}
86+
87+
var replacedCombined = string.Concat(existing.AsSpan(0, combinedIdx), s_supplementValue.AsSpan(), existing.AsSpan(combinedEnd));
88+
message.Request.Headers.Set("User-Agent", replacedCombined);
89+
return;
90+
}
91+
6992
// If the bare agent-framework segment is present (stamped by
7093
// AgentFrameworkUserAgentPolicy when not hosted), upgrade it in place to the
7194
// combined hosted form so the wire never carries both segments simultaneously.
7295
// Mirrors Python where get_user_agent() returns a single combined string when the
7396
// hosted prefix is registered.
74-
var idx = existing!.IndexOf(BareAgentFrameworkPrefix, StringComparison.Ordinal);
97+
var idx = existing.IndexOf(BareAgentFrameworkPrefix, StringComparison.Ordinal);
7598
if (idx >= 0)
7699
{
77100
var end = existing.IndexOf(' ', idx);

‎dotnet/src/Microsoft.Agents.AI.Foundry/FoundryAgent.cs‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,23 @@ public FoundryAgent(
113113
{
114114
}
115115

116+
/// <summary>
117+
/// Internal constructor used by the <c>AsAIAgent(this AIProjectClient, Uri, ...)</c>
118+
/// extension where the caller already has an <see cref="AIProjectClient"/> and the agent
119+
/// endpoint URI. Reuses the supplied client's pipeline (no new credential or transport is
120+
/// stamped) and surfaces the agent through a <see cref="FoundryChatClient"/> just like the
121+
/// public agent-endpoint ctor.
122+
/// </summary>
123+
internal FoundryAgent(
124+
AIProjectClient aiProjectClient,
125+
Uri agentEndpoint,
126+
IList<AITool>? tools = null,
127+
Func<IChatClient, IChatClient>? clientFactory = null,
128+
IServiceProvider? services = null)
129+
: base(CreateInnerAgentFromAgentEndpointReusingProjectClient(aiProjectClient, agentEndpoint, tools, clientFactory, services))
130+
{
131+
}
132+
116133
/// <summary>
117134
/// Internal constructor used by <c>AsAIAgent</c> extension methods that already have an <see cref="AIProjectClient"/> and a configured <see cref="ChatClientAgent"/>.
118135
/// </summary>
@@ -287,6 +304,39 @@ private static AIAgent CreateInnerAgentFromAgentEndpoint(
287304
return WireClientHeaders(new ChatClientAgent(chatClient, agentOptions, services: services));
288305
}
289306

307+
/// <summary>
308+
/// Variant of <see cref="CreateInnerAgentFromAgentEndpoint"/> that reuses an existing
309+
/// <see cref="AIProjectClient"/>'s pipeline instead of stamping a fresh credential. Used by
310+
/// the <c>AsAIAgent(AIProjectClient, Uri agentEndpoint, ...)</c> extension overload.
311+
/// </summary>
312+
private static AIAgent CreateInnerAgentFromAgentEndpointReusingProjectClient(
313+
AIProjectClient aiProjectClient,
314+
Uri agentEndpoint,
315+
IList<AITool>? tools,
316+
Func<IChatClient, IChatClient>? clientFactory,
317+
IServiceProvider? services)
318+
{
319+
Throw.IfNull(aiProjectClient);
320+
Throw.IfNull(agentEndpoint);
321+
322+
IChatClient chatClient = new FoundryChatClient(aiProjectClient, agentEndpoint, clientOptions: null);
323+
var agentName = ((FoundryChatClient)chatClient).AgentName!;
324+
325+
if (clientFactory is not null)
326+
{
327+
chatClient = clientFactory(chatClient);
328+
}
329+
330+
ChatClientAgentOptions agentOptions = new()
331+
{
332+
Id = agentName,
333+
Name = agentName,
334+
ChatOptions = new() { Tools = tools },
335+
};
336+
337+
return WireClientHeaders(new ChatClientAgent(chatClient, agentOptions, services: services));
338+
}
339+
290340
/// <summary>
291341
/// Parses an agent endpoint URI. Delegates to <see cref="FoundryChatClient.ParseAgentEndpoint(Uri)"/>
292342
/// so the chat client and the agent share a single source of truth for the URL shape.

‎dotnet/src/Microsoft.Agents.AI.Foundry/FoundryChatClient.cs‎

Lines changed: 121 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,21 @@ internal FoundryChatClient(Uri agentEndpoint, AuthenticationTokenProvider creden
139139
{
140140
}
141141

142+
/// <summary>
143+
/// Initializes a new instance for the Agent Endpoint mode (Mode 3) by reusing an existing
144+
/// <see cref="AIProjectClient"/>'s pipeline. Equivalent to the
145+
/// <see cref="FoundryChatClient(Uri, AuthenticationTokenProvider, ProjectOpenAIClientOptions?)"/>
146+
/// constructor but skips building a fresh per-agent pipeline: the project-level
147+
/// <see cref="ProjectOpenAIClient"/> on <paramref name="aiProjectClient"/> is used directly.
148+
/// </summary>
149+
/// <param name="aiProjectClient">The project client already configured at the project root containing <paramref name="agentEndpoint"/>.</param>
150+
/// <param name="agentEndpoint">The per-agent endpoint URI. Same shape constraints as the other agent-endpoint ctor.</param>
151+
/// <param name="clientOptions">Optional per-agent client options applied to the per-agent <c>GetProjectResponsesClientForAgentEndpoint</c> call.</param>
152+
internal FoundryChatClient(AIProjectClient aiProjectClient, Uri agentEndpoint, ProjectOpenAIClientOptions? clientOptions)
153+
: this(BuildAgentEndpointInnerFromProjectClient(aiProjectClient, agentEndpoint, clientOptions))
154+
{
155+
}
156+
142157
private FoundryChatClient(AgentEndpointInner inner)
143158
: base(inner.ChatClient)
144159
{
@@ -258,25 +273,53 @@ public async Task<FileDeletionResult> DeleteFileAsync(string fileId, Cancellatio
258273

259274
/// <summary>
260275
/// Uploads the supplied files, creates a vector store containing them, waits until the
261-
/// store is fully ready, and returns the <see cref="VectorStore"/>. Mirrors Python's
276+
/// store finishes ingesting its files (status leaves <see cref="VectorStoreStatus.InProgress"/>),
277+
/// and returns the <see cref="VectorStore"/>. Mirrors Python's
262278
/// <c>foundry_chat_client.create_vector_store(name, files, expires_after_days)</c>.
263279
/// </summary>
264280
/// <param name="name">The vector store name.</param>
265281
/// <param name="filePaths">Paths to files to upload and attach to the store.</param>
266282
/// <param name="expiresAfter">Optional last-active-at expiration window. When supplied, the vector store expires this many days after its last use.</param>
267283
/// <param name="cancellationToken">A token that can cancel the orchestration.</param>
268-
/// <returns>The created and fully-ready <see cref="VectorStore"/>.</returns>
284+
/// <returns>The created and fully-ready <see cref="VectorStore"/>. The returned instance reflects the state observed after polling completes; it may be in <see cref="VectorStoreStatus.Completed"/> (typical), <see cref="VectorStoreStatus.Expired"/>, or any other terminal status returned by the service. Only <see cref="VectorStoreStatus.InProgress"/> is polled.</returns>
285+
/// <remarks>
286+
/// <para>
287+
/// File-upload semantics are best-effort: when one of the per-file uploads throws, this method
288+
/// makes a best-effort attempt to delete the files it has already uploaded so they do not
289+
/// accumulate as orphaned resources on the project, then rethrows the original exception. The
290+
/// cleanup itself does not throw — its failures are silently ignored because the caller is
291+
/// already receiving a more meaningful exception from the original upload failure.
292+
/// </para>
293+
/// <para>
294+
/// Cancellation aborts the polling loop with an <see cref="OperationCanceledException"/>; any
295+
/// already-uploaded files and the partially-created vector store remain on the project and are
296+
/// the caller's responsibility to clean up.
297+
/// </para>
298+
/// </remarks>
269299
/// <exception cref="ArgumentException"><paramref name="name"/> is <see langword="null"/> or whitespace, or <paramref name="filePaths"/> is <see langword="null"/>.</exception>
270300
public async Task<VectorStore> CreateVectorStoreAsync(string name, IEnumerable<string> filePaths, TimeSpan? expiresAfter = null, CancellationToken cancellationToken = default)
271301
{
272302
Throw.IfNullOrWhitespace(name);
273303
Throw.IfNull(filePaths);
274304

275305
var fileIds = new List<string>();
276-
foreach (var path in filePaths)
306+
try
277307
{
278-
var uploaded = await this.UploadFileAsync(path, FileUploadPurpose.Assistants, cancellationToken).ConfigureAwait(false);
279-
fileIds.Add(uploaded.Id);
308+
foreach (var path in filePaths)
309+
{
310+
cancellationToken.ThrowIfCancellationRequested();
311+
var uploaded = await this.UploadFileAsync(path, FileUploadPurpose.Assistants, cancellationToken).ConfigureAwait(false);
312+
fileIds.Add(uploaded.Id);
313+
}
314+
}
315+
catch
316+
{
317+
// Q-B: best-effort cleanup of files already uploaded before the mid-loop failure so
318+
// they do not accumulate as orphaned resources on the project. Swallow cleanup
319+
// exceptions — the caller is already going to see the original upload exception, and
320+
// there is nothing useful we can do with a secondary delete failure.
321+
await BestEffortDeleteFilesAsync(fileIds).ConfigureAwait(false);
322+
throw;
280323
}
281324

282325
var options = new VectorStoreCreationOptions
@@ -293,8 +336,57 @@ public async Task<VectorStore> CreateVectorStoreAsync(string name, IEnumerable<s
293336
}
294337

295338
var vectorStoreClient = this.GetVectorStoreClient();
296-
var result = await vectorStoreClient.CreateVectorStoreAsync(options, cancellationToken).ConfigureAwait(false);
297-
return result.Value;
339+
var createResult = await vectorStoreClient.CreateVectorStoreAsync(options, cancellationToken).ConfigureAwait(false);
340+
var created = createResult.Value;
341+
342+
// Q-A: poll until the vector store leaves the in-progress state. Without this the helper
343+
// hands the caller a vector store whose file ingestion may still be running, defeating
344+
// the purpose of the one-call wrapper.
345+
return await WaitForVectorStoreReadyAsync(vectorStoreClient, created, cancellationToken).ConfigureAwait(false);
346+
}
347+
348+
private async Task BestEffortDeleteFilesAsync(IEnumerable<string> fileIds)
349+
{
350+
foreach (var id in fileIds)
351+
{
352+
try
353+
{
354+
// Pass CancellationToken.None: cleanup runs in the catch path; the caller's
355+
// token may already be cancelled and we still want to do our best to free
356+
// orphaned resources before propagating the original exception.
357+
await this.DeleteFileAsync(id, CancellationToken.None).ConfigureAwait(false);
358+
}
359+
catch
360+
{
361+
// Silently ignore cleanup failures; see XML doc on CreateVectorStoreAsync.
362+
}
363+
}
364+
}
365+
366+
private static async Task<VectorStore> WaitForVectorStoreReadyAsync(VectorStoreClient client, VectorStore initial, CancellationToken cancellationToken)
367+
{
368+
if (initial.Status != VectorStoreStatus.InProgress)
369+
{
370+
return initial;
371+
}
372+
373+
var delay = TimeSpan.FromMilliseconds(250);
374+
var maxDelay = TimeSpan.FromSeconds(2);
375+
var current = initial;
376+
while (current.Status == VectorStoreStatus.InProgress)
377+
{
378+
await Task.Delay(delay, cancellationToken).ConfigureAwait(false);
379+
var refreshed = await client.GetVectorStoreAsync(current.Id, cancellationToken).ConfigureAwait(false);
380+
current = refreshed.Value;
381+
382+
if (delay < maxDelay)
383+
{
384+
var next = TimeSpan.FromMilliseconds(delay.TotalMilliseconds * 2);
385+
delay = next < maxDelay ? next : maxDelay;
386+
}
387+
}
388+
389+
return current;
298390
}
299391

300392
/// <summary>Deletes a vector store. The associated files (if any) are not deleted by this method; call <see cref="DeleteFileAsync(string, CancellationToken)"/> separately to clean them up.</summary>
@@ -485,6 +577,28 @@ private static AgentEndpointInner BuildAgentEndpointInner(
485577
return new AgentEndpointInner(chatClient, aiProjectClient, agentName);
486578
}
487579

580+
private static AgentEndpointInner BuildAgentEndpointInnerFromProjectClient(
581+
AIProjectClient aiProjectClient,
582+
Uri agentEndpoint,
583+
ProjectOpenAIClientOptions? clientOptions)
584+
{
585+
Throw.IfNull(aiProjectClient);
586+
Throw.IfNull(agentEndpoint);
587+
588+
var (agentName, _) = ParseAgentEndpoint(agentEndpoint);
589+
590+
var perAgentOptions = clientOptions ?? new ProjectOpenAIClientOptions();
591+
perAgentOptions.Endpoint = agentEndpoint;
592+
perAgentOptions.AgentName = agentName;
593+
594+
var chatClient = aiProjectClient.GetProjectOpenAIClient()
595+
.GetProjectResponsesClientForAgentEndpoint(agentName, options: perAgentOptions)
596+
.AsIChatClient();
597+
598+
// Reuse the caller's AIProjectClient verbatim — no new pipeline is materialized.
599+
return new AgentEndpointInner(chatClient, aiProjectClient, agentName);
600+
}
601+
488602
/// <summary>Best-effort registration of <see cref="AgentFrameworkUserAgentPolicy"/> via the MEAI <see cref="OpenAIRequestPolicies"/> hook with at-most-once dedup per pipeline.</summary>
489603
private static void TryRegisterAgentFrameworkUserAgentPolicy(IChatClient? innerClient)
490604
{

‎dotnet/src/Microsoft.Agents.AI.Foundry/FoundryPromptAgentConverter.cs‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,23 @@ public static async Task<ProjectsAgentDefinition> ConvertAsync(IChatClient chatC
5858
return cachedVersion.Definition;
5959
}
6060

61-
// Prompt Agent (Mode 2) AgentReference-only: fetch the latest version from the service.
61+
// Prompt Agent (Mode 2) AgentReference-only: fetch the agent definition from the service.
62+
// Honor a pinned AgentReference.Version when present (Q-C fix); fall back to the latest
63+
// version only when the reference is unpinned ("", null, or "latest").
6264
if (foundryChatClient.GetService<AgentReference>() is { } agentReference)
6365
{
6466
var aiProjectClient = foundryChatClient.GetService<AIProjectClient>()
6567
?? throw new InvalidOperationException(
66-
"Cannot fetch the latest agent version because the FoundryChatClient does not expose an AIProjectClient.");
68+
"Cannot fetch the agent version because the FoundryChatClient does not expose an AIProjectClient.");
69+
70+
if (!string.IsNullOrWhiteSpace(agentReference.Version)
71+
&& !string.Equals(agentReference.Version, "latest", StringComparison.OrdinalIgnoreCase))
72+
{
73+
var pinnedVersion = await aiProjectClient.AgentAdministrationClient
74+
.GetAgentVersionAsync(agentReference.Name, agentReference.Version, cancellationToken)
75+
.ConfigureAwait(false);
76+
return pinnedVersion.Value.Definition;
77+
}
6778

6879
var record = await aiProjectClient.AgentAdministrationClient
6980
.GetAgentAsync(agentReference.Name, cancellationToken)

‎dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/HostedOutboundUserAgentTests.cs‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,46 @@ public async Task HostedAgentUserAgentPolicy_IsIdempotent_WhenCombinedSegmentAlr
308308
Assert.Equal(-1, second);
309309
}
310310

311+
[Fact]
312+
public async Task HostedAgentUserAgentPolicy_ReplacesDifferentVersionCombinedSegment_InPlaceAsync()
313+
{
314+
// Q-D regression: when the User-Agent already carries the COMBINED hosted form with a
315+
// different version (e.g. an older registration or caller-supplied baseline), the policy
316+
// must replace the entire combined span — not just the bare suffix — so we never emit
317+
// the malformed `foundry-hosting/foundry-hosting/agent-framework-dotnet/...` shape.
318+
using var handler = new InspectingHandler();
319+
#pragma warning disable CA5399
320+
using var httpClient = new HttpClient(handler);
321+
#pragma warning restore CA5399
322+
323+
var pipeline = ClientPipeline.Create(
324+
new ClientPipelineOptions { Transport = new HttpClientPipelineTransport(httpClient) },
325+
perCallPolicies: [new SetUserAgentPolicy("foundry-hosting/agent-framework-dotnet/0.0.1 MEAI/10.5.1"), HostedAgentUserAgentPolicy.Instance],
326+
perTryPolicies: default,
327+
beforeTransportPolicies: default);
328+
329+
// Act
330+
var message = pipeline.CreateMessage();
331+
message.Request.Method = "POST";
332+
message.Request.Uri = new Uri("https://example.test/anything");
333+
await pipeline.SendAsync(message);
334+
335+
// Assert: no doubled foundry-hosting/ prefix.
336+
Assert.NotNull(handler.LastUserAgent);
337+
Assert.DoesNotContain("foundry-hosting/foundry-hosting/", handler.LastUserAgent, StringComparison.Ordinal);
338+
339+
// The combined segment must appear exactly once, and the trailing MEAI segment must be
340+
// preserved in place (i.e. the policy only rewrote the combined span, not anything after it).
341+
var firstCombined = handler.LastUserAgent!.IndexOf("foundry-hosting/agent-framework-dotnet/", StringComparison.Ordinal);
342+
Assert.True(firstCombined >= 0);
343+
var secondCombined = handler.LastUserAgent.IndexOf("foundry-hosting/agent-framework-dotnet/", firstCombined + 1, StringComparison.Ordinal);
344+
Assert.Equal(-1, secondCombined);
345+
Assert.Contains(" MEAI/10.5.1", handler.LastUserAgent, StringComparison.Ordinal);
346+
347+
// And the version that survives must be the runtime supplement value's version, not 0.0.1.
348+
Assert.DoesNotContain("foundry-hosting/agent-framework-dotnet/0.0.1", handler.LastUserAgent, StringComparison.Ordinal);
349+
}
350+
311351
private sealed class InspectingHandler : HttpClientHandler
312352
{
313353
public string? LastUserAgent { get; private set; }

0 commit comments

Comments
 (0)