[MOBILE-3815] Review follow-up: upgrade data loss, Kotlin downgrade, live activity models, and release automation - #142
Merged
Conversation
…ive activity models Six blocking defects from the PR #137 review follow-up. Editor: - AirshipUpdater migrated drawables only from urbanairship-plugin-lib/res, but the 9.x updater had already moved them into urbanairship-resources.androidlib/res -- which this release adds to obsoleteDirectories. Upgrading deleted the customer's notification icons. Migrate from both legacy roots, merge into an existing destination instead of failing, and refuse to delete a directory that still holds drawables. - AirshipAndroidGradlePostProcess rewrote the Kotlin plugin version unconditionally, downgrading customers already on a newer Kotlin and breaking their build. Treat the requirement as a floor: only ever raise, leave unparseable versions (Gradle variables, unsubstituted placeholders) alone. Release: - Both matrix legs uploaded an identical package under one artifact name (409 on upload-artifact@v4) and every path still said urbanairship-* after the gradle output was renamed to airship-*. Build the platform-independent package once in its own job, fix the globs, and set if-no-files-found/fail_on_unmatched_files so a future rename fails loudly instead of publishing an empty release. Native bridges: - trackScreen's Kotlin parameter was non-null, so the documented TrackScreen(null) call to stop tracking threw on Android while iOS handled it. - The Android event pump consumed pending events before Unity registered its listener, dropping the first channel-created/push-token event of a cold start. Leave them pending and replay from setListener. Also collapses the per-EventType processPending walk into one pass. Live Activity / Live Update models: - LiveActivityInfo.attributes, LiveActivityContent.state and LiveUpdate.content are arbitrary JSON objects on the proxy side; JsonUtility silently left all three empty. Flatten them into parallel key/value arrays natively and pair them back in C#, matching the existing PushMessage.Extras approach. - Outbound Live Update requests sent dismissTimestamp, but the proxy reads dismissalTimestamp, so the value never arrived. Found while verifying the models against the proxy source; beyond the listed scope but a one-word fix in the same records. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…review items The non-blocking half of the PR #137 review follow-up. - EnumExtensions was a global-namespace static class with a `this Enum` extension. A customer with their own EnumExtensions got a compile error on upgrade. Moved into AirshipSDK; every consumer already lives there, so nothing else changed. - Exposed InboxMessage.listIconUrl. Both natives already put it on the wire (Kotlin sets it directly, the iOS proxy maps listIconURL -> "listIconUrl") and C# dropped it. - Made InternalInboxMessage and the InboxMessage(InternalInboxMessage) constructor internal. It is the wire shape, not something callers should build, and the name already said so. Visible to the test assembly via InternalsVisibleTo. - Inbox extras now go through AirshipUtils.PairFlattenedObject rather than a hand-rolled loop, which also drops a latent IndexOutOfRange when the values array came back shorter than the keys array. - AirshipPostBuild added its shell script phase unconditionally, so an Append build stacked another copy step on every build. Skip when the phase already exists. - editChannelSubscriptionLists was the only iOS editor case using an inline strict "values" guard instead of the tolerant helper the siblings use. Not a bug today -- JsonArray.ToJson does emit the wrapper -- but it made the payload shape load-bearing in one spot and not the others. - Documented OnAuthorizedSettingsChanged as iOS only. Authorized notification settings have no Android equivalent, so the event cannot fire there. - Dropped the unused System.Linq import in AttributeEditor and the unreachable nullable-enum branch in AirshipUtils (boxing a Nullable<T> yields T, so it never ran). - Added tests for the new wire contracts: listIconUrl, live update content, live activity attributes and content state, and PairFlattenedObject's edge cases. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The packaging step could not run on a hosted runner: build.gradle drives Unity directly for -importPackage/-exportPackage, and getUnityExePath() resolves the editor from $UNITY_EXE or a macOS Unity Hub path, so on ubuntu-latest it threw "Unable to find Unity executable". The tag-triggered release has therefore never produced the package; 9.x assets were attached by hand. Run the packaging job inside the game-ci editor image, which is the same image the existing player-build job already pulls, so no new infrastructure is involved: - A small `editor` job reads m_EditorVersion from ProjectSettings/ProjectVersion.txt -- the file unity-builder itself reads -- and derives the image tag from it, so bumping Unity cannot leave CI on a stale editor. Tag format and the rolling version default come from unity-builder's ImageTag. - Serial activation and license return mirror game-ci's own activate.sh: both need a -projectPath, and the seat is returned with if: always() because serial seats are limited. A failed return is not swallowed -- a leaked seat is worse than a red job. - The android image exports JAVA_HOME/ANDROID_HOME/ANDROID_SDK_ROOT only through the unity-editor wrapper's env fragments, which gradle does not go through, so the step sources them before invoking gradle. - UNITY_EXE points at /usr/bin/unity-editor rather than /opt/unity/Editor/Unity so the editor calls run under xvfb, as they do everywhere else in game-ci. Not verified: this cannot be exercised offline. Whether the resolved image tag is published for this editor version, and whether headless serial activation succeeds with this account's license type, will only be known from a real tag run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Simplifies the previous commit and writes down what is still unverified, so a red tag run is debuggable from the file rather than from memory. - Dropped the `editor` job and pinned package.container.image to a literal tag. The job existed only to avoid hardcoding the tag, and it put the riskiest assumption in the file on the critical path: if `container.image` does not accept a `needs` expression the workflow fails to parse, which is worse than any staleness it prevented. - Kept the protection it was there for, more cheaply and more correctly. The game-ci image records its editor version at $UNITY_PATH/version, so a first step compares the real container against ProjectSettings/ProjectVersion.txt. That catches drift in either direction with no duplicated version literal. - Restored publish-docs' `needs: [release, plugin]`. Narrowing it to [release] was behaviour-preserving but unrequested churn. - Added a header runbook covering the failure modes that can only show up on a real run: unpublished image tag, editor drift, Personal-vs-Pro license activation, a leaked serial seat, the JDK/Android SDK env fragments, and the two guards that stop an empty release. Plus the safe way to exercise it -- a prerelease tag runs `plugin` and `package` and then fails Verify Version before publishing anything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Docker Hub was reachable after all: unityci/editor:ubuntu-6000.2.8f1-android-3 exists (pushed 2026-03-19, linux/amd64, active), as does the -ios-3 sibling the player leg uses. Replaced the "may not be published" caveat with the curl that lists available tags, and recorded that the trailing -3 is game-ci's rolling image revision -- an alias over their 3.2.x builds -- so the editor segment is the only part needing maintenance. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fixes the CI compile failure in both matrix legs: CS0246 at AirshipLiveActivityManager.cs 202/211/219. Splitting the live activity content model into inbound and outbound halves, I renamed the type to LiveActivityContentInfo by replacing the original rather than adding alongside it, leaving LiveActivityStartRequest/UpdateRequest/EndRequest referencing a type that no longer existed. The doc comment on the new type even cross-referenced the one I had deleted. LiveActivityContent is back as the outbound type, keeping Dictionary<string, object> state: outbound payloads go through AirshipUtils.Serialize, which handles dictionaries, so only the inbound path needs the parallel-array workaround. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
On a tag push, `package` ran alongside both `plugin` matrix legs, so the release needed three simultaneous Unity license activations where CI has only ever demonstrated two. A serial has a fixed seat count and each Unity job holds one for its duration, so the extra job could fail activation with an error that reads as a broken workflow rather than a capacity limit. needs: [plugin] puts peak concurrency back at two, matching what CI already proves works. It also stops the run spending packaging time when the players do not build. Costs wall-clock: packaging now waits for both player legs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ulrico972
approved these changes
Aug 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #137, stacked on #141. Addresses the defects still open after that merge, plus the release pipeline.
Base is
MOBILE-3815rather thanmainso the diff is only these five commits.Blocking fixes
AirshipUpdatermigrates drawables from both legacy rootsurbanairship-resources.androidlib/res, which this release adds toobsoleteDirectories. Upgrading deleted customer icons. Also merges into an existing destination instead of throwing, and refuses to delete a directory still holding drawables.AirshipAndroidGradlePostProcessrewrote the version unconditionally, downgrading customers already on newer Kotlin and breaking their builds. Unparseable versions ($kotlin_version, unsubstituted placeholders) are now left alone.trackScreen(screenName: String?)AirshipAnalytics.TrackScreendocuments passingnullto stop tracking; iOS handled it, Android threw on Kotlin's null check. Closes the inline thread.setListener. Also oneprocessPendingpass instead of one perEventType.LiveActivityInfo.attributes,LiveActivityContent.stateandLiveUpdate.contentare arbitrary JSON objects on the proxy side;JsonUtilitysilently left all three empty. Flattened natively into parallel key/value arrays and paired back in C#, matching the existingPushMessage.Extrasapproach.Release pipeline
The tag-triggered release has never worked.
mainhad no packaging step at all (9.x assets were attached by hand), and the step added in #137 could not resolve a Unity editor on a hosted runner —getUnityExePath()reads$UNITY_EXEor a macOS Unity Hub path, soubuntu-latestthrewUnable to find Unity executable. On top of that both matrix legs uploaded an identical package under one artifact name (409 onupload-artifact@v4) and every path still saidurbanairship-*after the gradle output was renamed toairship-*.Now: the package is built once in its own job inside the game-ci editor image (the same image the Android player leg already pulls, verified published on Docker Hub), with serial activation and license return mirroring game-ci's own scripts.
if-no-files-found: errorandfail_on_unmatched_files: truemean a future rename fails loudly rather than publishing an empty release.The file carries a header runbook for the failure modes that can only surface on a real run, and a safe way to exercise it: push a prerelease tag such as
10.0.0-rc1—pluginandpackageboth run, thenVerify Versionfailsreleaseon the version mismatch before anything ships.Non-blocking
EnumExtensionsmoved out of the global namespace intoAirshipSDK. As a global static with athis Enumextension it broke the compile of any customer with their ownEnumExtensions.InboxMessage.listIconUrlexposed. Both natives already put it on the wire; C# dropped it.InternalInboxMessageandInboxMessage(InternalInboxMessage)made internal — it is the wire shape, and the name already said so.AirshipUtils.PairFlattenedObject, which also removes a latentIndexOutOfRangewhen the values array came back shorter than the keys array.AirshipPostBuildskips its shell-script phase when already present; Append builds were stacking a duplicate copy step every build.OnAuthorizedSettingsChangeddocumented as iOS-only in the migration guide.System.Linqimport and an unreachable nullable-enum branch removed.Review threads
Fixed: trackScreen, Live Activity state, event race. Documented:
OnAuthorizedSettingsChanged— no Android equivalent exists, so it cannot fire there.Two threads are false positives and need no code change. Kotlin
.optMap().opt("values")and iOSeditChannelSubscriptionListsboth assume the C# side sends a bare JSON array. It doesn't:TagEditor/TagGroupEditor/SubscriptionListEditor/ScopedSubscriptionListEditorall useJsonArray<T>.ToJson(), which isJsonUtility.ToJson(this)and emits{"values":[…]}.JsonArray.csis byte-identical tomainapart from the namespace, andmain's shipped Java did the sameopt("values")unwrap. The wrapper is the established contract. I did normalizeeditChannelSubscriptionListsonto the tolerant helper its siblings use, since it was the only case where the payload shape was load-bearing.Still open, needs a decision rather than a patch: JNI concurrency — add a lock defensively, or close it as investigated.
Beyond the agreed scope
One extra fix: outbound Live Update requests sent
dismissTimestamp, butLiveUpdateRequest.fromJsonreadsdismissalTimestamp, so the value never arrived on Android. Found while verifying the models against the proxy source; a one-word change in records I was already rewriting. The inbound model correctly keepsdismissTimestamp— that asymmetry is the proxy's.Deliberately skipped: deduplicating the Kotlin version between
build.gradleandairship.properties. Gradle'splugins {}block can't read an external property, so every workaround restructuressettings.gradle— low value, real risk of breaking the Android build.Build coverage
CI compiles the C#, and the first run on this branch caught a real error I introduced: splitting the live activity content model, I renamed
LiveActivityContentinstead of adding alongside it, leaving the three outbound request records pointing at a deleted type (CS0246 in both matrix legs). Restored inff0cdae.What CI does and does not cover, so the remaining gaps are explicit:
AirshipSDK.dll)unity-plugin)unity-plugingradle module is never built by theBuildstepAssets/Plugins/iOS)So the Kotlin event-pump and live-update changes and the Swift flattening changes are still unexercised by anything. Before merging, worth an Android and an iOS build from a dev machine, running the test suite in the editor, and a
10.0.0-rc1tag to exercise the release workflow without publishing.🤖 Generated with Claude Code