Extract syncVersions workflow shell into a script - #97598
Draft
mountiny wants to merge 2 commits into
Draft
Conversation
Draft
40 tasks
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
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.
Explanation of Change
Follow-up to the review feedback on Fix version sync between E/App and Mobile-Expensify: the main shell logic is moved out of
.github/workflows/syncVersions.ymland into.github/scripts/syncVersions.sh, leaving the workflow responsible only forworkflow_dispatch, checkout,validateActor,setupGitForOSBotify,setupNode, and thinrun:invocations.This is a behavior-preserving refactor. Every annotation string, commit message and git invocation is carried over unchanged.
Six
run:-bearing steps collapse into two:submoduleRemote,checkVersions./.github/scripts/syncVersions.sh checktargetVersion, full version sync, submodule-only bump,verifySync./.github/scripts/syncVersions.sh syncThe script uses two subcommands rather than a single always-run step so that
setupNodecan stay conditional — it is only needed for the full version sync path, and on macOS it runsnpm ci(which also runsnpm iinsideMobile-Expensifyviascripts/postInstall.sh, dirtying the submodule working tree). Forcing it on the submodule-only path would be both slow and a new failure mode.Notes on the details that matter for parity:
ACTUAL_SHAandNEED_FULL_VERSION_SYNCare threaded fromchecktosyncthrough step outputs rather than recomputed. RecomputingACTUAL_SHAwould turn the "did the submodule checkout move since we updated it?" guard into a comparison of a value with itself.syncdeliberately never re-runsgit submodule update --remote— Mobile-Expensifymaincan advance while Node is being set up between the two steps.git diff --staged --quiet; the full-sync path does none of that).${{ }}expression is interpolated into a shell body any more.TARGET_VERSIONnow reaches the script viaenv:, which removes a script-injection vector on a runner that holdsOS_BOTIFY_COMMIT_TOKENand 1Password credentials.The script gains one extra subcommand,
version-components, purely so the Android/iOS version math can be unit tested without a repo fixture.Two follow-ups intentionally left out of this PR so it stays a pure refactor:
git add Mobile-Expensify; git commitunconditionally. When the versions differ but the App gitlink already points at Mobile-Expensifymain, nothing is staged,git commitexits 1, and the job dies after the local version commit and before any push. The submodule-only path already guards this withgit diff --staged --quiet. Fixed in a follow-up PR stacked on this one.TARGET_VERSIONdispatch input is only usable when it equals the Mobile-Expensify version, since only the App side is rewritten and verification then compares the two. The inputdescription:is clarified here; the behavior is unchanged.Fixed Issues
$ #88233
PROPOSAL: N/A
Tests
This is a CI-only workflow change; there is no app-facing behavior to exercise. It is covered by new unit tests plus ShellCheck.
npm run shellcheckand verify it passes for all files, including the new.github/scripts/syncVersions.sh.npx jest tests/unit/SyncVersionsTest.tson macOS and verify all 13 tests pass. The suite builds throwaway git repos (a bare Mobile-Expensify remote, a bare App remote, and a working clone wiring them together as a submodule) underos.tmpdir()and runs the real script against them:version-componentsderives the correctSHORT_VERSION/BUILD_NUMBER/CF_VERSION/ANDROID_VERSION_CODE, including zero-padding, matchinggenerateAndroidVersionCodeinscripts/bumpVersion.ts.checkreports in-sync, full-version-sync-needed, and submodule-bump-needed for the three real-world states.syncin submodule-only mode commits and pushes the new pointer and verifies it, and fails with::error::(without pushing) when the submodule checkout moved sincecheck.syncin full-version mode rewritespackage.json,android/app/build.gradleand all threeInfo.plistfiles, pushes both commits, and fails verification when the target version does not match Mobile-Expensify.PlistBuddyand BSDsed -i '') and run on macOS.validateActor→setupGitForOSBotify→check→ conditionalsetupNode→sync→ Slack announce → Slack failure.Sync E/App and Mobile-Expensify versionsworkflow manually while App and Mobile-Expensify are already in sync, and verify the run exits early with::notice::✅ Versions and submodule are in syncand makes no commits, no pushes and no Slack post.Offline tests
Not applicable — this changes a GitHub Actions workflow and its supporting script. There is no client-side behavior and no network state to vary.
QA Steps
Not applicable — no user-facing behavior changes. The workflow is a manual
workflow_dispatchrestricted to mobile deployers and is validated by triggering it directly (see step 5 inTests), not on staging or production.PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
N/A — CI workflow change only.
Android: mWeb Chrome
N/A — CI workflow change only.
iOS: Native
N/A — CI workflow change only.
iOS: mWeb Safari
N/A — CI workflow change only.
MacOS: Chrome / Safari
N/A — CI workflow change only.