Fix terminal API surface - #18978
Conversation
Keep terminal orchestration implementation types internal and mark TerminalOptions with the same experimental diagnostic as WithTerminal. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ae2398ed-a4d8-423d-bf69-3a04bbf6c54f
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 18978Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18978" |
Tests selector (audit mode)The full test matrix and all jobs still run in audit mode. The tests and jobs below are what selective CI would run under enforcement. 48 / 100 test projects · 4 jobs, from 5 changed files. Selected test projects (48 / 100)
Selected jobs (4)
How these were chosen — grouped by what changed
🔧 show 43
🧪 📦 affected project Job reasons
Selection computed for commit |
There was a problem hiding this comment.
Pull request overview
Keeps terminal implementation details internal while retaining the experimental public terminal configuration API.
Changes:
- Internalizes terminal annotation, host resource, and socket-layout types.
- Marks
TerminalOptionswithASPIRETERMINAL001. - Updates XML documentation and adds API-surface tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/Aspire.Hosting/ApplicationModel/TerminalAnnotation.cs |
Internalizes the annotation and marks options experimental. |
src/Aspire.Hosting/ApplicationModel/TerminalHostLayout.cs |
Internalizes terminal socket-layout details. |
src/Aspire.Hosting/ApplicationModel/TerminalHostResource.cs |
Internalizes the terminal host resource. |
src/Aspire.Hosting/TerminalResourceBuilderExtensions.cs |
Removes public documentation links to internal types. |
tests/Aspire.Hosting.Tests/WithTerminalTests.cs |
Verifies visibility and experimental metadata. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Agree. I think it is good to keep this internal for now. I have a few other fixes to merge on top of this one. |
Description
The terminal API review found that orchestration and Unix domain socket implementation types were leaking into the public
Aspire.Hostingsurface. This change keepsTerminalAnnotation,TerminalHostResource, andTerminalHostLayoutinternal while preserving terminal configuration throughWithTerminalandTerminalOptions.TerminalOptionsnow carries the sameASPIRETERMINAL001experimental diagnostic asWithTerminal, so direct use of the supporting configuration type is guarded consistently. Public XML documentation no longer links to the internal terminal host resource, and focused tests cover the intended visibility and diagnostic metadata.User-facing usage
Terminal configuration remains available through the existing experimental API:
Validation:
dotnet test --project tests/Aspire.Hosting.Tests/Aspire.Hosting.Tests.csproj --no-launch-profile -- --filter-class "*.WithTerminalTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"(23 passed)Fixes # (issue)
Checklist
<remarks />and<code />elements on your triple slash comments?