Skip to content

[patch] [engg]: Remove obsolete availability checks - #1919

Open
josephpab wants to merge 5 commits into
devfrom
josephpab-commoncore-availability-cleanup
Open

[patch] [engg]: Remove obsolete availability checks#1919
josephpab wants to merge 5 commits into
devfrom
josephpab-commoncore-availability-cleanup

Conversation

@josephpab

@josephpab josephpab commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

PR Checklist (must be completed before review)

  • All tests pass locally
  • PR size is <= 500 LOC per PR Size Check policy
  • PR is independently mergeable (no hidden dependencies)
  • Appropriate reviewers are assigned
  • PR reviewed by code owner (required if Copilot-generated)
  • SME or Senior IC assigned where required

PR Title Format

Required Format: [Keyword1] [Keyword2]: Description

  • Keyword1: major, minor, or patch (case-insensitive)
  • Keyword2: feature, bugfix, engg, or tests (case-insensitive)

Examples:

  • [MAJOR] [Feature]: new API
  • [minor] [bugfix]: fix crash
  • [PATCH][tests]:add coverage

Proposed changes

Remove first-party runtime OS checks and compile-time SDK-version guards that are dead under CommonCore's supported deployment targets and build toolchains:

  • Use the macOS 12+ serial-number and XPC provider paths directly.
  • Configure SSO request UI behavior and map ASAuthorizationErrorNotInteractive directly.
  • Remove pre-iOS-13 UI automation fallbacks.
  • Remove iOS 15/16 and macOS 12/13/14 *_VERSION_MAX_ALLOWED wrappers, keeping every runtime @available gate so behavior on macOS 12 and other supported OS versions is unchanged.
  • Run the interaction-required and Platform SSO tests on every supported SDK where their platform applies.
  • Update the changelog.

Changed areas: MSIDDeviceId, MSIDXpcProviderCache, SSO extension request/provider handling, MSIDExternalSSOContext, URL host validation, related unit tests, iOS automation helpers, and changelog.txt.

Type of change

  • Feature work
  • Bug fix
  • Documentation
  • Engineering change
  • Test
  • Logging/Telemetry

Risk

  • High – Errors could cause MAJOR regression of many scenarios. (Example: new large features or high level infrastructure changes)
  • Medium – Errors could cause regression of 1 or more scenarios. (Example: somewhat complex bug fixes, small new features)
  • Small – No issues are expected. (Example: Very small bug fixes, string changes, or configuration settings changes)

Additional information

Deployment targets

Current dev sets IPHONEOS_DEPLOYMENT_TARGET = 16.0 and MACOSX_DEPLOYMENT_TARGET = 12.0. Runtime @available gates are preserved everywhere, so runtime behavior on macOS 12 is unchanged.

Compile-time SDK floor

Removing a *_VERSION_MAX_ALLOWED wrapper makes the enclosed symbol a compile-time requirement. The SDK each removal actually requires:

Symbol First declared in SDK Min Xcode
userInterfaceEnabled iOS 15.0 / macOS 12.0 13.0
ASAuthorizationErrorNotInteractive iOS 15.0 / macOS 12.0 13.0
NSURLComponents.percentEncodedHost iOS 7.0 / macOS 10.9 none (no new symbol)
ASAuthorizationProviderExtensionLoginManager macOS 13.0 14.1
ASAuthorizationProviderExtensionKeyTypeCurrentDeviceSigning macOS 14.0 15.0

The highest requirement introduced here is the macOS 14.0 SDK (Xcode 15.0). The percentEncodedHost guard never gated a symbol at all — that API dates to macOS 10.9 / iOS 7, and the guard only wrapped a behavioral difference, so its removal has no SDK implication.

This does not establish a new floor, because dev already requires a higher SDK than any of the above. These are used today with a runtime @available gate and no *_MAX_ALLOWED guard:

Pre-existing on dev First declared in SDK Min Xcode
ASWebAuthenticationSession.additionalHeaderFields (MSIDASWebAuthenticationSessionHandler.m) iOS 17.4 / macOS 14.4 15.3
WKWebView.inspectable (MSIDOAuth2EmbeddedWebviewController.m) iOS 16.4 / macOS 13.3 14.3

So the effective floor on dev is already Xcode 15.3, above the Xcode 15.0 maximum this PR requires.

Toolchain pinning

Every build path pins an Xcode version at or above that floor:

  • CommonCore visionOS validation selects /Applications/Xcode_16.2.app explicitly.
  • CommonCore, MSAL, and Broker iOS/macOS jobs take pool + Xcode from the shared Pipeline YAMLs/shared/aces-macos-job.yml@pipelinesShared template, documented in-repo as the source of truth for toolchain setup.
  • Broker's hosted visionOS stage pins Xcode 16.4 via select_xcode.sh.
  • MSAL C++ subtree checks pin Xcode 16.4 on the ACES pool and 15.4 on the hosted macOS-14 fallback — the lowest pin anywhere in this repo, and still above the Xcode 15.3 floor.

Guards intentionally retained

  • __MAC_OS_X_VERSION_MAX_ALLOWED >= 260000 for the macOS 26-only isSetupAssistantFlow declaration — above every pinned toolchain, so it is a genuine future-SDK guard.
  • Four #ifdef __MAC_OS_X_VERSION_MAX_ALLOWED checks in keychain code. These are platform discriminators (the macro is undefined for iOS/visionOS), not version thresholds, and preserve macOS-only data-protection-keychain attributes.
  • No first-party tvOS, watchOS, or visionOS *_VERSION_MAX_ALLOWED checks were present. There are no first-party sample paths or nested submodules.

Newer runtime checks remain intact, including macOS 13/14 XPC and Platform SSO paths, iOS 16.4/macOS 13.3 web inspection, iOS 17/macOS 14 URL parsing tests, and iOS 18/macOS 15/visionOS 2 web-authentication APIs.

Validation

Completed with local code signing disabled:

  • xcodebuild test -workspace IdentityCore.xcworkspace -scheme 'IdentityCore iOS' -configuration Debug -sdk iphonesimulator -destination 'platform=iOS Simulator,id=F0DB19C7-8299-4AB3-AEB7-027FE3AC41D0' -only-testing:'IdentityCoreTests iOS/MSIDSSOExtensionRequestDelegateTests' -only-testing:'IdentityCoreTests iOS/NSURLExtensionsTests' -parallel-testing-enabled NO CODE_SIGN_IDENTITY='' CODE_SIGNING_REQUIRED=NO CODE_SIGNING_ALLOWED=NO -quiet — passed on iOS 16.4.
  • xcodebuild test -workspace IdentityCore.xcworkspace -scheme 'IdentityCore Mac' -configuration Debug -destination 'platform=macOS' -only-testing:'IdentityCoreTests Mac/MSIDSSOExtensionRequestDelegateTests' -only-testing:'IdentityCoreTests Mac/NSURLExtensionsTests' -only-testing:'IdentityCoreTests Mac/MSIDDeviceInfoTests' -parallel-testing-enabled NO CODE_SIGN_IDENTITY='' CODE_SIGNING_REQUIRED=NO CODE_SIGNING_ALLOWED=NO -quiet — passed.
  • xcodebuild build-for-testing -workspace IdentityCore.xcworkspace -scheme 'IdentityCore iOS' -configuration Debug -sdk xrsimulator -destination 'platform=visionOS Simulator,id=33A7323A-038C-41D4-ADBC-19E99C863C17' CODE_SIGN_IDENTITY='' CODE_SIGNING_REQUIRED=NO CODE_SIGNING_ALLOWED=NO -quiet — passed on visionOS 2.5.
  • xcodebuild build -project IdentityCore/IdentityCore.xcodeproj -target 'IdentityAutomationTestLib iOS' -configuration Debug -sdk iphonesimulator CODE_SIGN_IDENTITY='' CODE_SIGNING_REQUIRED=NO CODE_SIGNING_ALLOWED=NO -quiet — passed.
  • xcodebuild build -project IdentityCore/IdentityCore.xcodeproj -target 'IdentityAutomationTestLib Mac' -configuration Debug CODE_SIGN_IDENTITY='' CODE_SIGNING_REQUIRED=NO CODE_SIGNING_ALLOWED=NO -quiet — passed.
  • CODE_SIGNING_ALLOWED=NO CODE_SIGNING_REQUIRED=NO ./build.py --targets mac_library --no-xcpretty --no-clean — full macOS build and test suite passed.
  • PR CI: [Common Core] PR Validation (iOS + macOS) and [MSAL] Common core submodule check v2 (iOS + macOS) passed.

Remove runtime OS gates and fallback branches made unreachable by the iOS 16 and macOS 12 deployment floors while retaining newer-version and SDK guards.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d4af6210-550f-424a-86c5-e8619d0b0178
@josephpab
josephpab requested a review from a team as a code owner July 24, 2026 19:02
@josephpab
josephpab requested review from Yong Zeng (jasoncoolmax) and mipetriu and a lite review from Copilot July 24, 2026 19:02

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

Removes runtime OS availability branches that are now unreachable due to the project’s updated deployment floors (iOS 16+, macOS 12+), simplifying SSO/UI behavior and macOS utility code while keeping compile-time SDK guards intact.

Changes:

  • Eliminates redundant @available(...) runtime checks in SSO request/delegate and macOS utility paths.
  • Simplifies UI automation helpers by removing pre–iOS 13 fallback behaviors.
  • Updates integration test execution path and records the cleanup in the changelog.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
IdentityCore/tests/integration/MSIDSSOExtensionRequestDelegateTests.m Runs the interaction-required error mapping test unconditionally on supported SDKs (still behind MAX_ALLOWED guards).
IdentityCore/tests/automation/ui_tests_lib/XCUIElement+CrossPlat.m Removes legacy iOS version branching in text selection helper; marks unused app parameter.
IdentityCore/tests/automation/ui_tests_lib/MSIDBaseUITest.m Removes pre–iOS 13 keyboard-tap loop; marks unused app parameter.
IdentityCore/src/util/mac/MSIDXpcProviderCache.m Removes macOS 12 availability gating around XPC provider existence check; simplifies control flow.
IdentityCore/src/util/ASAuthorizationSingleSignOnProvider+MSIDExtensions.m Sets userInterfaceEnabled directly under compile-time guards (no runtime availability check).
IdentityCore/src/requests/broker/MSIDSSOExtensionRequestDelegate.m Maps ASAuthorizationErrorNotInteractive directly under compile-time guards (no runtime availability check).
IdentityCore/src/MSIDDeviceId.m Uses the macOS 12+ kIOMainPortDefault path directly for serial-number retrieval.
changelog.txt Adds an entry documenting removal of redundant runtime availability checks.

Comment thread IdentityCore/src/util/mac/MSIDXpcProviderCache.m
Use the documented Xcode 16.2 baseline to remove compile-time SDK checks that are guaranteed true, while retaining runtime availability, platform, and future-SDK guards.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d4af6210-550f-424a-86c5-e8619d0b0178
Copilot AI review requested due to automatic review settings July 24, 2026 19:51

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 12 out of 12 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

IdentityCore/src/util/mac/MSIDXpcProviderCache.m:207

  • Local variable name folderConstrats appears to be a typo ("Constrats" vs "Constraints"), which makes the intent of the substring check harder to read. Since this is a local variable, it can be safely renamed for clarity.
#if DEBUG
    NSString *folderConstrats = @"DerivedData";
#else
    NSString *folderConstrats = @"Applications";
#endif

Comment thread IdentityCore/src/util/NSURL+MSIDExtensions.m
@josephpab
josephpab enabled auto-merge (squash) July 28, 2026 23:58
@mipetriu

Copy link
Copy Markdown
Contributor

This change hinges on every build using an SDK new enough that these symbols are declared, since removing the *_VERSION_MAX_ALLOWED wrappers keeps only the runtime @available gate. That's fine at runtime, but it turns this into a compile-time dependency on the Xcode 16.2 / macOS 15.2 SDK across the board — e.g. userInterfaceEnabled (ASAuthorizationSingleSignOnProvider+MSIDExtensions.m), ASAuthorizationErrorNotInteractive (MSIDSSOExtensionRequestDelegate.m), the NSURLComponents.percentEncodedHost path (NSURL+MSIDExtensions.m), and the macOS 13 loginManager properties (MSIDExternalSSOContext.h/.m) will no longer compile under an older SDK.

Do we pin the toolchain everywhere this is consumed — CI templates and any downstream MSAL/broker builds — so nobody can build this on an older Xcode? If the 16.2 floor is real and enforced, this is fine and the description just needs to match (the comment on the userInterfaceEnabled line already caught the description drift on that one file). If it isn't enforced, the *_MAX_ALLOWED guards for these are the ones I'd keep. Could you confirm which it is?

@josephpab

Copy link
Copy Markdown
Contributor Author

Good catch on the description — it was overstating the baseline, and I have rewritten that section. Short answer: the floor is real and enforced, but it is not Xcode 16.2, and this PR is not what establishes it.

Per-symbol requirements. Checking each symbol you named against the SDK headers:

Symbol First declared in SDK Min Xcode
userInterfaceEnabled iOS 15.0 / macOS 12.0 13.0
ASAuthorizationErrorNotInteractive iOS 15.0 / macOS 12.0 13.0
NSURLComponents.percentEncodedHost iOS 7.0 / macOS 10.9 none
ASAuthorizationProviderExtensionLoginManager macOS 13.0 14.1
ASAuthorizationProviderExtensionKeyTypeCurrentDeviceSigning macOS 14.0 15.0

So the highest requirement introduced is the macOS 14.0 SDK (Xcode 15.0), not 16.2/15.2.

One correction on percentEncodedHost: that guard never gated a symbol. percentEncodedHost dates to macOS 10.9 / iOS 7, and nothing else inside the block is new API — the #if only wrapped a behavioral difference (macOS 13 stopped throwing on an invalid host). Removing it has no SDK implication at all; the runtime @available still does the real work.

The floor already exists on dev, independent of this PR. Two APIs are used today with a runtime @available gate and no *_MAX_ALLOWED guard:

  • ASWebAuthenticationSession.additionalHeaderFields in MSIDASWebAuthenticationSessionHandler.m — iOS 17.4 / macOS 14.4 SDK, so Xcode 15.3+
  • WKWebView.inspectable in MSIDOAuth2EmbeddedWebviewController.m — iOS 16.4 / macOS 13.3 SDK, so Xcode 14.3+

Anyone on an SDK older than macOS 14.4 / iOS 17.4 already fails to compile dev before this change. Since my maximum requirement is Xcode 15.0, these removals sit strictly below the existing floor and cannot be the thing that breaks an older-SDK build.

Pinning. Every build path pins at or above that:

  • CommonCore visionOS validation: /Applications/Xcode_16.2.app, explicit.
  • Broker hosted visionOS stage: 16.4 via select_xcode.sh.
  • MSAL C++ subtree checks: 16.4 on the ACES pool, 15.4 on the hosted macOS-14 fallback — the lowest pin in the repo, still above the 15.3 floor.
  • CommonCore / MSAL / Broker iOS+macOS jobs: pool + Xcode come from Pipeline YAMLs/shared/aces-macos-job.yml@pipelinesShared, which our YAML documents as the source of truth for toolchain setup.

Caveat worth flagging: that shared ACES template lives in IDDP/MSAL-ObjC-Pipelines and I could not read it from here (access-restricted), so I am relying on its documented role rather than quoting a pinned version. If you want that nailed down before merging, someone with access can confirm the exact xcode-select in it — though it does not change the conclusion, since the binding constraint is the pre-existing Xcode 15.3 requirement on dev, not anything in this diff.

On the two bot nits: the userInterfaceEnabled one was the description drift, now fixed. I left the folderConstrats typo alone as a pre-existing rename unrelated to this cleanup, and left @available(macOS 13.0, *) as-is — the * already covers iOS/visionOS, and with an iOS 16.0 deployment target adding iOS 16.0 would be a no-op. Happy to fold either in if you would rather see them here.

Comment thread IdentityCore/tests/automation/ui_tests_lib/MSIDBaseUITest.m
Perform the tap in the single-argument method so it no longer allocates an XCUIApplication solely to pass into an overload that ignores it. The app: overload is retained as a thin wrapper because it is public API of this shared test library and MSAL automation still calls it.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d4af6210-550f-424a-86c5-e8619d0b0178
Copilot AI review requested due to automatic review settings August 4, 2026 20:25

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 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (2)

IdentityCore/src/util/NSURL+MSIDExtensions.m:136

  • The availability check only gates macOS 13+, but the comment and behavior change also apply to iOS 16+. As written, the host-validation block may be skipped on iOS builds. Consider using a multi-platform availability check so the validation runs on both iOS 16+ and macOS 13+.
        if (@available(macOS 13.0, *))

IdentityCore/src/util/mac/MSIDXpcProviderCache.m:207

  • Typo in the local variable name (folderConstrats) makes the code harder to read/grep and looks accidental. Rename it to folderConstraints (and update the single use) to avoid propagating the misspelling.
#if DEBUG
    NSString *folderConstrats = @"DerivedData";
#else
    NSString *folderConstrats = @"Applications";
#endif

@josephpab

Copy link
Copy Markdown
Contributor Author

Copilot resolve the merge conflicts in this pull request

Co-authored-by: josephpab <175344915+josephpab@users.noreply.github.com>
auto-merge was automatically disabled August 8, 2026 19:51

Head branch was pushed to by a user without write access

Copilot AI commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Done in bb0b3c6 — merged dev into the branch (one "update from dev" merge commit, no rebase).

changelog.txt was the only conflict: dev added the macOS CBA preferred-identity entry under TBD at the same position as this branch's availability-cleanup entry. Kept both, dev's first.

No semantic conflicts — the 11 source/test files in this PR are disjoint from the files dev touched since the merge base, so the availability-cleanup diff is unchanged (git diff origin/dev...HEAD still shows the same 11 files plus the changelog line).

Comment thread changelog.txt Outdated
Drop the overstated Xcode 16.2 reference from the changelog entry. The highest SDK any removal in this PR requires is macOS 14.0 (Xcode 15.0), and dev already had an effective Xcode 15.3 floor, so the shipping note now cites the deployment targets and supported toolchains without a version figure that would go stale.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d4af6210-550f-424a-86c5-e8619d0b0178
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.

5 participants