Fix docs safe-output base fallback - #19465
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 32c349b4-907d-42e9-aad8-2f0edc267779
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19465Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19465" |
github-actions
Bot
added
the
area-engineering-systems
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Adds a validated fallback for resolving documentation PR target branches from raw gh-aw safe-output metadata.
Changes:
- Extracts shared branch-resolution and validation logic.
- Integrates it into source and compiled workflows.
- Adds regression fixtures and comprehensive tests.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/Infrastructure.Tests/WorkflowScripts/PrDocsCheckWorkflowTests.cs |
Verifies workflow integration. |
.github/workflows/pr-docs-check/validate_outcome.py |
Uses the shared resolver. |
.github/workflows/pr-docs-check/test_validate_outcome.py |
Tests outcome fallback behavior. |
.github/workflows/pr-docs-check/test_resolve_safe_output_target.py |
Tests resolver validation. |
.github/workflows/pr-docs-check/resolve_safe_output_target.py |
Implements target resolution. |
.github/workflows/pr-docs-check/fixtures/run-32112079288-safeoutputs.jsonl |
Adds raw recovery fixture. |
.github/workflows/pr-docs-check/fixtures/run-32112079288-agent_output.json |
Adds canonical recovery fixture. |
.github/workflows/pr-docs-check.md |
Integrates resolver into workflow. |
.github/workflows/pr-docs-check.lock.yml |
Regenerates compiled workflow. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Report whether a notification mismatch came from canonical or raw server metadata, and keep drafted-PR validation source-neutral. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 32c349b4-907d-42e9-aad8-2f0edc267779
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
|
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. |
* Fix Issue 19708 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7e17da02-39b1-4fa1-ac25-144eb6fcb904 * Eliminate false positive Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Make the restore hermetic instead of dropping the project update Restores the project-update leg of the test and makes its restore hermetic via the install sidecar's `packages` field, rather than removing the step that fails. Without `packages`, the relaunched CLI derives its Aspire feed from the identity it just persisted (darc-pub-microsoft-aspire-<commit>). Once that staging build is promoted to GA the feed stops carrying a matching Aspire.AppHost.Sdk, which is the reported failure in #19708. Pinning `packages` at the harness hive removes that dependency, and InstallSidecarWriter.PrepareForSelfUpdate rewrites only channel/version/commit, so the field survives the self-update. This restores two behaviours the test was written to cover: that the persisted identity channel lands in a project's aspire.config.json, and the "AppHost predates the self-update" scenario users actually hit. The second `aspire update --self` is dropped because it cannot coexist with `packages`: once the sidecar's channel is staging, the synthesized local-hive channel replaces the same-named built-in channel (PackagingService.GetChannelsAsync) and a local hive has no CliDownloadBaseUrl, so self-download fails with "Channel 'staging' does not support CLI downloads". The project update asserts the same persistence more strongly, via the resulting config. Also drops the [QuarantinedTest] attribute added by #19733 while resolving the merge with main, so the fix actually runs in CI. Validated: 3 consecutive passing E2E runs against ASPIRE_E2E_ARCHIVE, with quarantined and outerloop tests excluded. Recording confirms the relaunched process is the published staging binary (13.5.3+b5f143315ffb6968ea939a9978797a5b20e4c688), not a local no-op. UpdateCommandTests theory: 4 passed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f2f02689-8514-4cf8-b38d-f2b62f506754 * Improve package path processing Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Mitch Denny <midenn@microsoft.com> Copilot-Session: 7e17da02-39b1-4fa1-ac25-144eb6fcb904 Copilot-Session: f2f02689-8514-4cf8-b38d-f2b62f506754
Tests selector3 / 99 PR test projects · 1 PR job · 0 advisory-only targets, from 11 changed files. Selected PR test projects (3 / 99)
Selected PR jobs (1)
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
The privileged cross-repository safe-output workflow was validated offline but not demonstrated through a post-change workflow dispatch.
Review details
- Files reviewed: 11/11 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. |
|
✅ No documentation update needed. Step 5 branch taken: docs_required → false-positive signal, nothing to document Triggered signals (1): All 11 changed files are internal engineering-systems artifacts: the |
|
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
Fixes the Documentation Check safe-output base resolver exposed by recovery run 32112079288. gh-aw v0.86.2 generated the correct patch against
release/13.5, but canonicalization stripped the server-injected base metadata and the apply-time resolver failed closed before safe outputs could create the draft documentation PR.This change extracts the resolver into a tested helper that:
The workflow still skips the resolver when no
create_pull_requestoutput exists. The generated lock file was compiled idempotently with the repository-pinned gh-aw v0.86.2.Validation:
python -m unittest discover -s .github\workflows\pr-docs-check -p "test_*.py" -v(102 passed)dotnet test --project tests\Infrastructure.Tests\Infrastructure.Tests.csproj --no-launch-profile -- --filter-class "*.PrDocsCheckWorkflowTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"(6 passed)gh aw compile pr-docs-check --validate --no-check-updateChecklist
<remarks />and<code />elements on your triple slash comments?