Remove refresh-typescript-sdks workflow and script - #18288
Conversation
Co-authored-by: radical <1472+radical@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 18288Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18288" |
There was a problem hiding this comment.
Pull request overview
This PR fixes the refresh-typescript-sdks.yml CI workflow, which had been silently failing on every run because the underlying eng/refreshTypeScriptSdks.ps1 script still targeted a removed playground directory layout (playground/polyglot/TypeScript/<app>/ValidationAppHost/apphost.ts). The TypeScript playground was restructured into playground/TypeScriptAppHost and playground/TypeScriptApps/*, with entry points renamed from apphost.ts to apphost.mts.
Changes:
- Updated AppHost discovery to recursively search multiple playground roots (
TypeScriptAppHost,TypeScriptApps) forapphost.mts/apphost.tsentry points instead of using the removed fixed path. - Made SDK verification extension-aware: generated module file names (
.mtsvs.ts) are now derived from the entry-point extension. - Refactored per-app metadata into a
pscustomobjectwithDirectory,EntryPoint, andDisplayNameproperties for cleaner downstream usage.
The refresh script derived generated-file extensions per entry point (.mts vs legacy .ts) but hardcoded '.aspire/modules' as the output directory for both the pre-restore cleanup and the post-restore verification. The CLI writes a legacy 'apphost.ts' app's generated SDK to '.modules/' instead (ILanguageDiscovery.LegacyGeneratedFolderName), so a legacy app would have its real output left untouched during cleanup and then fail verification looking in the wrong directory. Add Get-GeneratedModulesDir to resolve the output directory from the entry-point extension the same way the file names are derived, and use it for both the cleanup and Assert-GeneratedSdkFiles. No playground app uses 'apphost.ts' today, so this fixes the latent legacy path rather than a current failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
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
Changes AnalyzedFiles Changed
Change Categories
This is an infra-only PR, so it was validated using the CI Infrastructure Validation (GitHub Actions track)What runs on this PR
Automated tests
gh-aw / lock files
Logic validation (the core of the fix)The end-to-end run ( Pre-condition confirmed (reproduces the bug the PR fixes): the old hard-coded root Harness result: 13 / 13 assertions passed.
Source verification of CLI-behavior assumptionsBecause the verification step (
This matches the script's entry-point→folder and entry-point→file-extension coupling exactly. The verification step will therefore target the directory and file names the CLI actually writes, for both modern and legacy AppHosts. Failure-modes scan
Residual CI-only coverage (not exercised)A real Summary
Overall Result✅ PR VERIFIED The fix correctly replaces the removed Recommendations
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
The generated AppHost SDK output is ignored by the repo-level .gitignore, so the scheduled refresh workflow could successfully regenerate files and still produce an empty PR. Stage those ignored SDK files explicitly before invoking the shared create-pull-request action. Co-authored-by: Copilot <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. |
|
I think we should actually delete this workflow, it must have been created at the very beginning of polyglot when we decided we should keep the generated code in the repos before we had |
|
Assuming we are ok with this explanation I will update this PR to delete the file instead |
The generated TypeScript AppHost SDKs under .aspire/modules and .modules are gitignored and regenerated on demand by 'aspire restore'/'aspire run'. The scheduled workflow had to force-add these ignored files to commit them, which only produced churn and drift against locally generated output. Remove the workflow and its script entirely. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sébastien Ros (sebastienros)
changed the title
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. Runs the full test matrix + all jobs (ALL) — a rule matching '.github/actions/enumerate-tests/action.yml' selects ALL Selection computed for commit |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Adam Ratzman (adamint)
left a comment
Adam Ratzman (adamint)
left a comment
There was a problem hiding this comment.
(automated review)
looks good to me now. I pushed the workflow staging fix for the ignored generated SDK outputs; the unrelated template timeout passed on rerun and CI is green.
|
Thank you Adam Ratzman (@adamint) and Sébastien Ros (@sebastienros) for fixing this! |
Address review findings on the failure-issue automation: - The monitor watch list still named refresh-typescript-sdks.yml and update-github-models.yml, but both workflows were removed on main (microsoft#18288 and microsoft#18405). monitor-scheduled-workflows.js looks each entry up via listWorkflowRuns({ workflow_id }), so every 2-hour run would 404 on these two and log a warning while monitoring nothing. Remove both from the config and from the watch-list prose in the doc. - monitor-scheduled-workflows.md linked internal-build-failure- notifications.md, which does not exist in the repo. Drop the dangling link, keep the sentence. - specialized-test-failure-issues.md described extractionFailed as true "only when every .trx was unreadable". The implementation is readErrors > 0 && failedTests.Count == 0 (at least one unreadable trx with no failures collected); the partial case is also flagged. Match the markdown to the code and its in-code comment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Removes the
Refresh TypeScript Playground SDKsworkflow (.github/workflows/refresh-typescript-sdks.yml) and its script (eng/refreshTypeScriptSdks.ps1).Why
The workflow regenerated the TypeScript AppHost SDK modules (
aspire.*,base.*,transport.*) under.aspire/modules/.modulesand opened a daily PR to commit them. But those generated files:.gitignoreignores.aspire/and.modules/),main, not on disk by default), andaspire restore/aspire run.To commit them at all, the workflow had to force past
.gitignore(git add -f, added incd0c171cb7specifically because the PR was otherwise a no-op). Committing ephemeral, per-build generated code only creates churn and drifts against whatever the local CLI generates. There's no value in maintaining a scheduled PR for it, so the workflow and script are removed entirely.No other files reference the workflow or script.