Fix Foundry Local lifecycle and remote endpoints - #19197
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: dc2ed36b-6e61-4271-8ceb-3de8338a20e1
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19197Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19197" |
github-actions
Bot
added
the
area-app-model
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Updates the Foundry hosting integration for modern daemonized CLI behavior and remote Foundry Local endpoints.
Changes:
- Supports legacy
serviceand modernserverCLI lifecycles. - Adds unmanaged remote endpoints and
LocalModelId. - Uses HTTP health checks and loads cached models before downloading.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
tests/PolyglotAppHosts/Aspire.Hosting.Foundry/TypeScript/apphost.mts |
Exercises new TypeScript APIs. |
tests/PolyglotAppHosts/Aspire.Hosting.Foundry/Python/apphost.py |
Exercises new Python APIs. |
tests/PolyglotAppHosts/Aspire.Hosting.Foundry/Java/AppHost.java |
Exercises new Java APIs. |
tests/PolyglotAppHosts/Aspire.Hosting.Foundry/Go/apphost.go |
Exercises new Go APIs. |
tests/Aspire.Hosting.Azure.Tests/FoundryExtensionsTests.cs |
Adds lifecycle, parsing, and endpoint tests. |
tests/Aspire.Hosting.Azure.Tests/FoundryDeploymentConnectionPropertiesTests.cs |
Verifies local model connection properties. |
src/Aspire.Hosting.Foundry/README.md |
Documents managed and remote usage. |
src/Aspire.Hosting.Foundry/LocalModelHealthCheck.cs |
Checks loaded models through HTTP. |
src/Aspire.Hosting.Foundry/FoundryResource.cs |
Tracks service management mode. |
src/Aspire.Hosting.Foundry/FoundryLocalService.cs |
Implements CLI lifecycle and model handling. |
src/Aspire.Hosting.Foundry/FoundryLocalHealthCheck.cs |
Probes Foundry through HTTP. |
src/Aspire.Hosting.Foundry/FoundryExtensions.cs |
Adds remote endpoint configuration and orchestration. |
src/Aspire.Hosting.Foundry/FoundryDeploymentResource.cs |
Exposes LocalModelId. |
Suppressed comments (2)
src/Aspire.Hosting.Foundry/FoundryLocalService.cs:355
- When
foundry server startexits nonzero without reporting an endpoint, this still waits for the full startup timeout beforeRunFoundryCommandAsynccan inspect the exit code. Users therefore get a delayed generic timeout instead of the CLI's immediate failure output. Only wait for the endpoint predicate after a successful parent exit; a nonzero exit should flow through to the existing detailed command error.
await process.WaitForExitAsync(startCancellation.Token).ConfigureAwait(false);
await outputCompletionSource.Task.WaitAsync(startCancellation.Token).ConfigureAwait(false);
src/Aspire.Hosting.Foundry/FoundryLocalService.cs:355
- The central daemonized-stream regression is not exercised by the added tests: the parser tests never enter this process path, and
RunAsFoundryLocal_SetsIsEmulatoracceptsFailedToStartwhen the CLI is unavailable. Add an automated process-level regression using a fakefoundrycommand whose parent exits while a child retains the redirected handles, and verify startup returns after the endpoint is observed rather than waiting for EOF.
// The modern "server start" command daemonizes, and the daemon inherits the CLI's
// redirected stream handles. Wait until the parent exits and reports its endpoint,
// then stop draining instead of waiting forever for EOF from the daemon.
await process.WaitForExitAsync(startCancellation.Token).ConfigureAwait(false);
await outputCompletionSource.Task.WaitAsync(startCancellation.Token).ConfigureAwait(false);
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: dc2ed36b-6e61-4271-8ceb-3de8338a20e1
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (4)
src/Aspire.Hosting.Foundry/FoundryLocalService.cs:114
- The new cross-generation loaded-model HTTP logic has no behavioral coverage; the added test only exercises the JSON parser. Add handler-backed tests for the modern
/models/loadedresponse, the legacy 404 fallback to/openai/loadedmodels, non-success responses, and case-insensitive model matching so either CLI generation cannot silently remain unhealthy.
public static async Task<bool> IsModelLoadedAsync(Uri endpoint, string modelId, HttpClient httpClient, CancellationToken cancellationToken)
{
foreach (var path in new[] { "models/loaded", "openai/loadedmodels" })
{
using var request = new HttpRequestMessage(HttpMethod.Get, new Uri(endpoint, path));
src/Aspire.Hosting.Foundry/README.md:69
- This new remote-endpoint workflow is only documented in C#, although both
RunAsFoundryLocal(endpoint)andLocalModelIdare exported to polyglot AppHosts. Add the equivalent TypeScript example so the hosting README covers every exported usage surface.
```csharp
var foundry = builder.AddFoundry("foundry")
.RunAsFoundryLocal("http://windows-host:5273");
src/Aspire.Hosting.Foundry/FoundryDeploymentResource.cs:44
- These new remarks conflict with the adjacent
DeploymentNamedocumentation, which still says thatDeploymentNameis the model ID for Foundry Local. The connection string now ignoresDeploymentNamein local mode and usesLocalModelId ?? ModelName, so update that remark to avoid contradictory IntelliSense.
/// <remarks>
/// Aspire resolves this value automatically when it manages Foundry Local. Set it explicitly
/// when connecting to an existing Foundry Local service and <see cref="ModelName"/> is an alias
/// rather than the identifier reported by the service.
src/Aspire.Hosting.Foundry/FoundryLocalService.cs:388
- The daemonized-stream regression is not covered by an automated test. The updated AppHost test can pass via
FailedToStartwhen Foundry is absent, while the parser tests never exercise a parent process exiting with a child still holding redirected pipes. Add a deterministic fake-CLI process test that verifies startup returns after the endpoint is emitted and shutdown completes without waiting for stream EOF.
if (stopReadingAfterProcessExit)
{
using var startCancellation = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
startCancellation.CancelAfter(s_serviceStartTimeout);
using var startCancellationRegistration = startCancellation.Token.Register(static state => KillProcess((Process)state!), process);
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
PR Testing ReportPR Information
Artifact Version Verification
The dogfood CLI and NuGet package hive were installed into an isolated temporary directory. Four fresh C# file-based AppHosts were generated from the PR hive, and each added Changes AnalyzedChange Categories
The PR changes Foundry Local process lifecycle management, cached-model reuse, HTTP health checks, unmanaged remote endpoints, Test Scenarios ExecutedScenario 1: PR artifact and fresh AppHost verificationObjective: Verify testing uses packages built from the latest PR commit rather than the workspace build. Coverage Type: Artifact/build validation Status: Passed Steps:
Evidence:
Scenario 2: Managed modern Foundry lifecycle with cached modelObjective: Verify Aspire manages Foundry CLI 0.10.1, loads an already-cached model, reports HTTP health, resolves the concrete local model ID, and stops the daemon. Coverage Type: Happy path Status: Passed Steps:
Evidence:
Scenario 3: Existing remote endpoint remains externally ownedObjective: Verify Aspire observes an existing Foundry Local endpoint with explicit Coverage Type: Happy path and lifecycle boundary Status: Passed Steps:
Evidence:
Scenario 4: Invalid existing endpointObjective: Verify unsupported endpoint schemes fail before AppHost startup. Coverage Type: Unhappy path Status: Passed Expected Outcome: A non-zero exit code and the documented absolute HTTP/HTTPS validation error. Observed Outcome: Evidence:
Scenario 5: Failed modern startup cleans up the daemonObjective: Verify a modern CLI that reports daemon startup and then fails is cleaned up without leaving the resource in a success-shaped state. Coverage Type: Unhappy path and cleanup recovery Status: Passed Steps:
Evidence:
Summary
Overall ResultPR VERIFIED All approved scenarios passed against the packaged PR artifact. No product issue was found. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: dc2ed36b-6e61-4271-8ceb-3de8338a20e1
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/Aspire.Hosting.Foundry/FoundryLocalHealthCheck.cs:20
- Legacy
foundry serviceinstances do not exposeGET /v1/models; the v0.8.94 REST contract usesGET /openai/statusfor service status. This probe therefore keeps an otherwise healthy legacy service unhealthy, despite this change explicitly retaining legacy CLI support. Add a 404 fallback to/openai/status(or choose the probe by CLI generation) and cover both endpoint generations.
using var request = new HttpRequestMessage(HttpMethod.Get, new Uri(resource.EmulatorServiceUri, "v1/models"));
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: dc2ed36b-6e61-4271-8ceb-3de8338a20e1
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/Aspire.Hosting.Foundry/FoundryLocalService.cs:395
- The hang regression is not exercised by the added tests: they cover output parsing, while the existing lifecycle test uses whatever
foundryinstallation happens to be present and therefore does not deterministically execute this daemonized-stream path in CI. A future change could wait for daemon-owned EOF again without failing any test. Add a process-runner regression test where the CLI parent writes an endpoint and exits while a child keeps stdout/stderr open, and assert startup returns within a bounded time.
await process.WaitForExitAsync(startCancellation.Token).ConfigureAwait(false);
await outputCompletionSource.Task.WaitAsync(startCancellation.Token).ConfigureAwait(false);
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
🔵 Needs a closer look
The new public endpoint overload is missing the required IntelliSense usage example.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/Aspire.Hosting.Foundry/FoundryExtensions.cs:181
- This public fluent API has two materially different ownership modes, but its IntelliSense documentation does not include a usage example for the new external-endpoint mode. Add an
<example>with a<code>block so callers can see the required call shape without relying on the package README.
- Files reviewed: 13/13 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: dc2ed36b-6e61-4271-8ceb-3de8338a20e1
|
Addressed the suppressed documentation recommendation from review 5115545563. The public |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
🟢 Approval recommended
The lifecycle, remote endpoint, health-check, connection-property, and polyglot paths have focused regression coverage.
Review details
- Files reviewed: 13/13 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: dc2ed36b-6e61-4271-8ceb-3de8338a20e1
Tests selector3 / 99 PR test projects · 3 PR jobs · 0 advisory-only targets, from 12 changed files. Selected PR test projects (3 / 99)
Selected PR jobs (3)
Advisory workflow impact (0)none How these were chosen — grouped by what changed🧪 🔧 🔧 🔧 🔧 🔧 🔧 🧪 Job reasons
Selection computed for commit |
There was a problem hiding this comment.
🔵 Needs a closer look
Cross-version external daemon ownership, networking, and shutdown behavior warrant final human validation despite strong focused coverage.
Review details
- Files reviewed: 13/13 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
Pull request created: #1621
|
|
📝 Documentation has been drafted in microsoft/aspire.dev#1621 targeting Documented the new Foundry Local remote endpoint mode (
Note This draft PR needs human review before merging. |
|
The CI build failed due to test failure(s) that appear unrelated to the PR changes. These may be flaky tests. Suspected flaky test(s): Suggested actions:
You can re-run the failed jobs from the workflow run page. |
Description
Foundry Local's newer CLI daemonizes through
foundry server, which caused Aspire to hang while reading inherited output streams and left cached models stuck in a downloading state. WSL2 and Linux AppHosts also had no supported way to connect to a Foundry Local service already running on another host.This change supports both legacy
foundry serviceand currentfoundry serverCLI generations, waits for structured startup output without waiting indefinitely for daemon-owned streams, serializes shutdown, and loads cached models before downloading. It also adds an unmanaged endpoint mode that observes an existing Foundry Local service without starting, stopping, downloading, or loading models on that host. Service and model health checks now use the Foundry HTTP APIs, including the loaded-model endpoint.User-facing usage
C# AppHost:
TypeScript AppHost:
Validation includes 38 targeted Foundry hosting tests, compilation of the TypeScript, Java, Python, and Go polyglot AppHosts, and an end-to-end lifecycle check with Foundry CLI 0.10.1 that confirmed the daemon starts, reaches readiness, and stops cleanly.
Fixes #10937
Fixes #12750
Checklist
<remarks />and<code />elements on your triple slash comments?