Skip to content

[patch][engg]: Migrate MSAL visionOS validation to governed ACES macOS template - #3065

Open
mipetriu wants to merge 5 commits into
devfrom
mipetriu-msal-visionos-aces-migration
Open

[patch][engg]: Migrate MSAL visionOS validation to governed ACES macOS template#3065
mipetriu wants to merge 5 commits into
devfrom
mipetriu-msal-visionos-aces-migration

Conversation

@mipetriu

Copy link
Copy Markdown
Contributor

Summary

Migrates azure_pipelines/visionos-validation.yml off the Microsoft-hosted macOS-14 vmImage onto the governed shared ACES macOS job template, keeping visionOS validation build-only. This is the MSAL leg of the org-wide ACES migration (common/IdentityCore and Broker are already done; mirrors Broker PR #2576 and this repo's already-migrated pr-validation.yml).

What changed

  • Added pipelinesShared git resource (IDDP/MSAL-ObjC-Pipelines, ref: refs/heads/main) and converted all three jobs to - template: Pipeline YAMLs/shared/aces-macos-job.yml@pipelinesShared. Pool, image, Xcode selection, and Ruby/xcbeautify tooling are now inherited from that single source of truth. (TODO comment left in-file to switch the ref once ameyapat/common-aces-config merges.)
  • Added azure_pipelines/templates/download-visionos-sdk.yml — ACES-compatible visionOS SDK + Apple Vision Pro simulator provisioning with retry logic and failOnStderr: false. No xcode-select (Xcode is owned by the ACES template). Included by all three jobs.
  • Dropped sudo xcode-select -s /Applications/Xcode_*.app steps, the inline xcodebuild -downloadPlatform visionOS blocks, and the manual gem install xcpretty/slather/bundler installs (all owned by / unnecessary under the ACES template).
  • Kept the existing github resource endpoint 'MSAL ObjC Service Connection' (this repo's convention) for the Broker/WPJ resources.

⚠️ Build-only conversion of Validate_visionOS_Framework

This job previously ran the visionOS XCTest host and published JUnit results. It is now build-only:

  • PublishTestResults@2 was removed (build-only produces no reports).
  • Build command unchanged: ./build.py --target visionOSFrameworkbuild.py already defines the visionOSFramework target as build-only (operations: ["build"]), so no build.py change was needed.

Root cause of the build-only guardrail: visionOS xcodebuild test builds fine then hangs at test-host launch (exit 124) because the simulator's backboardd / RealityKit+PHASE spatial-audio subsystem segfaults (SIGSEGV) on the ACES agent image (ADO run 1670903). This is a proven agent-image/runtime defect, not our code — the fix belongs to the ACES macOS agent-pool team.

Validate_SPM_Integration_visionOS — no hang risk

Investigated spm-integration-test.sh: with --include-visionos --skip-sample-app it only runs xcodebuild ... archive for the xrsimulator/xros SDKs, builds an xcframework, and packages/pushes. It never boots a visionOS simulator or runs an XCTest host, so it is not affected by the exit-124 hang. Migrated the pool to ACES only; functional behavior (temp-branch create/run/cleanup, script flags) is unchanged.

Validate_Broker_visionOS — already build-only

Kept ./build.py --show-build-settings --target vision_library (Broker sub-build, build-only). Migrated pool/Xcode/SDK handling to the ACES pattern; preserved all checkout/auth/submodule logic (AzureCLI aadToken, NGC submodules with MSAzureToken_encoded, WPJ + openssl-msft submodules, Broker checkout path layout, UsePythonVersion@0).

Preserved exactly

  • Triggers (pr autoCancel/drafts → main/release/*; trigger push → main/release/*) — when the pipeline runs is unchanged.
  • All auth/token/submodule/checkout logic for job 3.

Validation

  • Both YAML files parse cleanly with yaml.safe_load; steps under each job's template parameters is confirmed a non-null list (5 / 6 / 12 steps respectively). @pipelinesShared / ${{ }} tokens are valid YAML scalars.
  • build.py was not modified.
  • Real verification is a manual ADO queue against this scratch branch (org identitydivision, project IDDP). Expect a one-time "pipeline needs permission to access a resource" authorization prompt for the governed pipelinesShared template on first run. This does not need to be queued to open the PR.

…S template

Migrate azure_pipelines/visionos-validation.yml off the Microsoft-hosted
macOS-14 vmImage onto the governed shared ACES macOS job template, keeping
visionOS validation build-only.

- Add pipelinesShared git resource (IDDP/MSAL-ObjC-Pipelines, refs/heads/main)
  and convert all three jobs to the shared aces-macos-job.yml template.
- Add azure_pipelines/templates/download-visionos-sdk.yml (ACES-compatible
  visionOS SDK + Apple Vision Pro simulator provisioning with retry logic,
  no xcode-select).
- Validate_visionOS_Framework converted to build-only: drop PublishTestResults@2
  (visionOS XCTest host hangs at launch, exit 124, due to backboardd /
  RealityKit+PHASE SIGSEGV on the ACES agent image; ADO run 1670903).
- Drop xcode-select, inline xcodebuild -downloadPlatform visionOS blocks, and
  manual gem installs (all owned by the ACES template).
- Preserve triggers and all auth/token/submodule/checkout logic.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8518b07f-0bc8-4213-a55f-8fdc0d8c5969
Copilot AI review requested due to automatic review settings July 28, 2026 18:34
@mipetriu
mipetriu requested review from a team as code owners July 28, 2026 18:34
@mipetriu
mipetriu requested review from sidudiki and swasti29 July 28, 2026 18:34
Comment thread azure_pipelines/templates/download-visionos-sdk.yml Outdated
Match Broker #2576 convention: no PR trigger (pr: none) and push
trigger restricted to release/* (dropped main). Updated top-of-file
comment to reflect release-only + no PR trigger.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8518b07f-0bc8-4213-a55f-8fdc0d8c5969

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.

Pull request overview

This PR migrates the visionOS validation Azure Pipelines definition to the governed ACES macOS job template, centralizing pool/Xcode/toolchain configuration while keeping the visionOS leg build-only.

Changes:

  • Switched all three visionOS validation jobs to use Pipeline YAMLs/shared/aces-macos-job.yml@pipelinesShared.
  • Added a reusable template to provision the visionOS SDK + Vision Pro simulator with retries.
  • Removed inline Xcode selection / gem installs / inline xcodebuild -downloadPlatform blocks in favor of shared-template ownership.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
azure_pipelines/visionos-validation.yml Migrates jobs to the governed ACES macOS template and wires in the shared visionOS SDK provisioning template.
azure_pipelines/templates/download-visionos-sdk.yml Adds a shared step template to download the visionOS platform (with retries) and provision a Vision Pro simulator device.

Comment thread azure_pipelines/visionos-validation.yml
Comment thread azure_pipelines/visionos-validation.yml
Comment thread azure_pipelines/templates/download-visionos-sdk.yml Outdated
Copilot AI review requested due to automatic review settings July 28, 2026 18:39

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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

azure_pipelines/visionos-validation.yml:5

  • The pipeline trigger configuration contradicts both the PR description (“Triggers … unchanged”) and the existing repo convention that visionOS validation runs on PRs to main/release (see azure_pipelines/pr-validation.yml:34 comment). With pr: none and trigger limited to release/*, this pipeline will not run on PRs to main/release/*, which can break branch protection expectations and reduce validation coverage.

Recommendation: restore the PR trigger for main and release/* and re-add main to the push trigger (and update the header comment accordingly).

# Pipeline for visionOS validation - runs only on pushes to release branches (no PR trigger)
# This pipeline is separate to avoid slowing down regular PR validation
# Configure GitHub branch protection rules to require this pipeline for release branches
#
# Pool + Xcode + Ruby/Python tooling are provided by the shared governed ACES macOS

@kaisong1990 kaisong1990 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.

One issue in the new SDK template.

Comment thread azure_pipelines/templates/download-visionos-sdk.yml Outdated
- Trigger visionOS validation on master + release/* pushes (keep pr: none).
  visionOS telemetry usage is minimal so per-PR runs aren't worth it, but
  master/release integration coverage is; rationale added to header comment.
- Remove stale TODO about switching pipelinesShared ref once
  ameyapat/common-aces-config merges — it already pins refs/heads/main.
- Pin SPM-integration cleanup to $(Build.SourcesDirectory) instead of the
  brittle 'cd ../..', mirroring pr-validation.yml.
- Harden download-visionos-sdk.yml: gate the expensive/flaky platform
  download purely on SDK presence (a missing simulator device alone no longer
  forces a redundant re-download); create the sim device only if missing; fail
  fast (logissue error + exit 1) when the device type/runtime can't be resolved
  or the visionOS SDK is still absent after setup. Keep failOnStderr: false.

Stays strictly build-only (no test host, no PublishTestResults). No build.py
changes.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8518b07f-0bc8-4213-a55f-8fdc0d8c5969
Copilot AI review requested due to automatic review settings July 29, 2026 22:47
@mipetriu

Copy link
Copy Markdown
Contributor Author

Re: the low-confidence reviewer note about the trigger configuration (visionos-validation.yml) — the trigger is intentional. This pipeline runs on pushes to master and release/* with pr: none (no per-PR trigger) by design: visionOS telemetry usage is minimal, so running full visionOS validation on every PR isn't worth the queue time, whereas master/release integration coverage is valuable. The header comment now documents this rationale. This matches the convention applied to the Broker and common/IdentityCore repos in the same org-wide effort.

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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread azure_pipelines/visionos-validation.yml Outdated
Comment thread azure_pipelines/templates/download-visionos-sdk.yml Outdated
Comment thread azure_pipelines/templates/download-visionos-sdk.yml Outdated
…late)

Trigger visionos-validation.yml on PRs to `main` and on pushes to
`main` + `release/*`. MSAL cuts releases as a PR from a release/* branch
into `main`, so `main` is the release-integration branch; this replaces
the earlier master/`pr: none` convention. Header comment updated to
explain the rationale.

download-visionos-sdk.yml:
- Return-check `xcrun simctl create` (guarded by `if !`) so a failed
  device create fails fast with logissue + exit 1 instead of falling
  through to "created successfully" and exiting 0.
- Extract the visionOS runtime id by matching the
  com.apple.CoreSimulator.SimRuntime.* token instead of `awk '{print $NF}'`,
  which could capture a trailing "(available)" marker.

Stays strictly build-only. No build.py changes. pipelinesShared ref
untouched (refs/heads/main).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8518b07f-0bc8-4213-a55f-8fdc0d8c5969
Copilot AI review requested due to automatic review settings July 31, 2026 22:52

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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

azure_pipelines/visionos-validation.yml:25

  • Issue: The PR description says PR triggers were preserved for main + release/* and that pr.autoCancel/pr.drafts are kept, but this pipeline now only triggers PRs targeting main and omits autoCancel/drafts.

Impact: If someone opens a PR targeting a release/* branch (or if branch protection expects this status check on release PRs), this pipeline won’t run and required checks can’t complete. It also diverges from the stated intent of the PR.

Recommendation: Either update the PR description/header comment to match the new trigger behavior, or (preferred, to match the description and the existing branch-protection note) restore release/* in the PR trigger and add back autoCancel: true + drafts: true.

# Trigger on PRs targeting main (release PRs land here)
pr:
  branches:
    include:
    - main

Copilot AI review requested due to automatic review settings July 31, 2026 23:40

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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

azure_pipelines/visionos-validation.yml:26

  • The PR description says the trigger behavior was preserved (including pr.autoCancel and pr.drafts), but the updated YAML omits both. This is inconsistent with the stated intent and can change (or at least obscure) PR trigger behavior.

Recommendation: either restore autoCancel: true / drafts: true under pr: (matching the prior pipeline), or update the PR description to reflect the intentional change.

pr:
  branches:
    include:
    - main

@kaisong1990 kaisong1990 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.

Reviewed the ACES migration end to end. Two things below; the rest looks correct and my earlier SDK-script comments are addressed.

jobName: Validate_Broker_visionOS
displayName: Validate Broker visionOS Build
timeoutInMinutes: 30
selectXcode: false

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.

The Broker job was pinned to Xcode 15.4 before this change (both the 'Select Xcode version' step and the SDK download used 15.4), while the other two jobs used 16.2. With selectXcode: false and the explicit xcode-select steps removed, this job now builds vision_library on whatever Xcode the ACES image defaults to. That is a toolchain change for the Broker build, not just a pool migration, and it is not called out in the description. Please confirm the Broker visionOS build has actually been run green on the ACES default Xcode, or pass the Xcode selection parameter for this job.

# triggers give post-merge integration coverage on `main` and `release/*`. This replaces the earlier
# master/`pr: none` convention (used in the Broker/WPJ repos) because MSAL releases PR into `main`.
# This pipeline is separate to avoid slowing down regular PR validation.
# Configure GitHub branch protection rules to require this pipeline for main/release branches

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.

This line is now stale: the pr trigger only includes main, so this pipeline never runs on PRs targeting release/* and cannot be a required check there. Same for the note in pr-validation.yml (�isionOS validation moved to visionos-validation.yml (runs on PRs to main/release only)). Not re-opening the trigger decision, just please update both comments so they match the new behavior.

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.

3 participants