Skip to content

Fix source links for external API references - #1610

Open
David Pine (IEvangelist) wants to merge 1 commit into
microsoft:mainfrom
IEvangelist:ievangelist-aws-api-source-links
Open

Fix source links for external API references#1610
David Pine (IEvangelist) wants to merge 1 commit into
microsoft:mainfrom
IEvangelist:ievangelist-aws-api-source-links

Conversation

@IEvangelist

Copy link
Copy Markdown
Member

Summary

  • resolve Aspire.Hosting.AWS package versions to the immutable commit behind the matching AWS GitHub release
  • normalize embedded PDB document paths to safe repository-relative paths and suppress confirmed missing inferred files
  • authenticate the updater's GitHub reads with bounded retries

Validation

  • dotnet test tests/PackageJsonGenerator.Tests/PackageJsonGenerator.Tests.csproj --configuration Release --no-restore (48 passed)
  • generated Aspire.Hosting.AWS 13.7.2 and Aspire.Hosting.DocumentDB 0.114.1 with zero rooted paths and zero missing repository files
  • generated the AWS TypeScript API data and confirmed it inherited commit 653c103df4e342a927f7ac0fd533b350ef772c34

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings September 2, 2026 15:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new GitHub release resolver only queries the first 100 releases, which can cause the automation to fail for older versions once the upstream repo exceeds that page size.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR improves how the PackageJsonGenerator determines and emits source links for externally sourced API references by (1) resolving immutable source commits for certain external packages and (2) hardening PDB-derived source path handling so generated links remain safe and valid.

Changes:

  • Add PDB document-path normalization to emit safe repository-relative paths (and fail fast on unmappable absolute paths).
  • Treat confirmed 404 source lookups as “missing file” and suppress broken file/line links for inferred filenames.
  • Resolve Aspire.Hosting.AWS package versions to the commit behind the matching upstream GitHub release, and plumb GITHUB_TOKEN into the updater workflow.
File summaries
File Description
tests/PackageJsonGenerator.Tests/PackageJsonGeneratorHelperTests.cs Adds unit tests for PDB source-path normalization behavior (safe mapping + rejection cases).
src/tools/PackageJsonGenerator/README.md Documents the new external release resolver and PDB path normalization behavior.
src/tools/PackageJsonGenerator/PackageJsonGenerator.cs Suppresses broken links when a source file is confirmed missing (404) at the resolved commit.
src/tools/PackageJsonGenerator/Helpers/PdbSourceReader.cs Normalizes PDB document names into safe repo-relative paths; ignores empty doc names.
src/tools/PackageJsonGenerator/generate-package-json.ps1 Adds AWS external-release commit resolution and GitHub-authenticated reads with retries.
.github/workflows/update-integration-data.yml Exposes an app token as GITHUB_TOKEN for the updater script to increase GitHub API reliability.
Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +400 to +418
$releases = $null
for ($attempt = 1; $attempt -le 3; $attempt++) {
try {
$releases = Invoke-RestMethod -Uri $source.ReleasesApi -Headers $headers
break
}
catch {
if ($attempt -eq 3) {
throw
}
Start-Sleep -Seconds ([Math]::Pow(2, $attempt - 1))
}
}

$headingPattern = "(?m)^###\s+$([regex]::Escape($PackageId))\s+\($([regex]::Escape($Version))\)\s*$"
$matchingReleases = @($releases | Where-Object {
-not [string]::IsNullOrWhiteSpace($_.body) -and $_.body -match $headingPattern
})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants